fix: review round — failed bundle save fails the deploy, removing a draft records no change

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Diego Imbert
2026-10-01 10:33:42 +02:00
co-authored by Claude Opus 5.5
parent 26c08e9e95
commit 83ef6ce56d
6 changed files with 38 additions and 8 deletions
@@ -451,8 +451,8 @@
fromServer = true
}
}
// A folder retarget during the await (the route-page header switcher, or a
// session retarget) changed the target and reset hydratedFromDb. Don't
// A folder switch during the await (the route page's header switcher)
// changed the target and reset hydratedFromDb. Don't
// apply this now-stale folder's bundle or mark the new folder hydrated —
// otherwise the stale result blocks the new folder's hydrate and its
// drafts bleed across folders.
@@ -198,6 +198,22 @@ describe('pipeline tools', () => {
).rejects.toThrow(/No pipeline editor is open/)
})
it('remove_pipeline_node records no change: undoing a draft leaves nothing to deploy', async () => {
const { helpers } = makeHelpers()
const modified: string[] = []
await toolByName('remove_pipeline_node').fn({
args: { path: 'f/analytics/clean' },
workspace: 'w',
helpers,
toolCallbacks: {
...noopCallbacks(),
onItemModified: (kind, path) => modified.push(`${kind}:${path}`)
},
toolId: 't'
})
expect(modified).toEqual([])
})
it('test_pipeline_node requires confirmation', () => {
expect(toolByName('test_pipeline_node').requiresConfirmation).toBe(true)
})
@@ -366,8 +366,9 @@ export const pipelineTools: SessionTool<PipelineToolHelpers>[] = [
const { path } = removePipelineNodeSchema.parse(args)
const pipeline = pipelineForPath(helpers, path)
toolCallbacks.setToolStatus(toolId, { content: `Discarding draft '${path}'...` })
// Not recorded: undoing a draft changes nothing to deploy, and the last
// one gone would leave the pipeline listed as this chat's change.
await pipeline.removeProposedNode(path)
recordPipelineModified(toolCallbacks, pipeline)
toolCallbacks.setToolStatus(toolId, {
content: `Discarded draft '${path}'`,
result: 'Success'
@@ -354,12 +354,22 @@
// The session's changes list re-reads the draft once this resolves, so
// what is left of it has to be saved by then.
await tick()
await UserDraftDbSyncer.flush({
const draft = {
workspace: workspaceId,
itemKind: PIPELINE_DRAFT_KIND,
path: pipelineBundlePath(path)
})
}
await UserDraftDbSyncer.flush(draft)
deployErrors = errors
// A flush resolves even when the save failed: the server would keep the
// deployed drafts, and deploying them again would recreate their triggers.
const sync = UserDraftDbSyncer.getState(draft)
if (UserDraftDbSyncer.getConflict(draft).conflict || sync.state === 'failed') {
return {
success: false,
error: `Deployed, but the remaining drafts could not be saved: ${sync.failureMessage ?? 'they conflict with a newer version'}.`
}
}
if (errors.size > 0) {
deployErrorsOpen = true
return {
@@ -48,8 +48,9 @@ export function forkDiffKindToUserDraftKind(kind: ForkDiffKind): UserDraftItemKi
// A pipeline the chat edited is masked as its folder's draft bundle, and each
// script that bundle deploys is masked too, so a fork review selects exactly
// those. The session's changes list shows them as the pipeline's one row, not
// as rows of their own.
// those. The session's changes list shows deployed scripts of a masked
// pipeline's folder as that pipeline's one row — any of them, since a script's
// mask entry does not say whether a pipeline deploy or the chat put it there.
export function foldedIntoPipelineRow(
mask: ReadonlySet<string>,
kind: UserDraftItemKind,
@@ -540,7 +540,9 @@
}
const sync = UserDraftDbSyncer.getState(draft)
if (sync.state === 'failed') {
throw new Error(`The pipeline drafts could not be saved: ${sync.failureMessage ?? 'unknown error'}`)
throw new Error(
`The pipeline drafts could not be saved: ${sync.failureMessage ?? 'unknown error'}`
)
}
// The drafts go with the pipeline into the session, so leaving for it is
// not leaving them behind.