From 78d069a6282c2ea4347e44f9bab42dc7685c0e52 Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Fri, 25 Sep 2026 17:43:19 +0200 Subject: [PATCH] refactor: derive the nameable kinds from the probes they need MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-6 review finding. The set of kinds that can be stored under another name and the map of deployed-existence probes had to hold the same three kinds, with nothing tying them together: adding a nameable kind without its probe would make the deployed-item check fall through, letting a draft staged under a deployed item's name win that name — the inverse of the rule the resolution is built on, with no type error to catch it. The set is now the probe map's keys. Also adds the guard for the branch that makes probe-first ordering safe: a delete whose deployed-existence check fails while a draft is staged under that name stops instead of falling through to the namesake. Co-Authored-By: Claude Opus 5 (1M context) --- .../copilot/chat/global/core.test.ts | 20 +++++++++++++++++++ .../copilot/chat/global/userDraftAdapter.ts | 20 +++++++++++-------- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index cd71a547a4..2d4eb2bec5 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -2671,6 +2671,26 @@ describe('global AI tools', () => { } }) + // Whether something is deployed at the path is what says whether a draft staged under + // that name is the one addressed. Unanswered, a delete would fall through to the + // namesake and remove an unrelated draft. + it('stops a delete when it cannot tell whether the path is a deployed item', async () => { + seedBackendDraft( + 'raw_app', + 'u/admin/draft_unreachable_probe', + { summary: 'Namesake', draft_path: 'f/sales/flaky_app', files: {}, runnables: {} }, + { workspace: WORKSPACE } + ) + vi.mocked(AppService.existsApp).mockRejectedValue(new Error('server error')) + + await expect( + callGlobalTool('delete_workspace_item', { type: 'app', path: 'f/sales/flaky_app' }) + ).rejects.toThrow(/Could not tell whether/) + expect( + getBackendDraft('raw_app', 'u/admin/draft_unreachable_probe', { workspace: WORKSPACE }) + ).toBeDefined() + }) + it('refuses a draft_path that two drafts are staged under', async () => { for (const storage of ['u/admin/draft_a', 'u/admin/draft_b']) { seedBackendDraft( diff --git a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts index c52ec98b21..62b06d09fd 100644 --- a/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts +++ b/frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts @@ -314,15 +314,15 @@ export function chosenDraftName(itemKind: UserDraftItemKind, value: unknown): st } /** - * The kinds an editor can stage a name for: a flow and a raw app write `draft_path`, a - * script renames the value's own `path`. A resource, variable, schedule or trigger draft - * is only ever stored at its own path, so a failed drafts listing withholds nothing a - * write of one needed. + * Deployed-existence probe per kind that can be stored under a name other than its path: a + * flow and a raw app stage one in `draft_path`, a script renames the value's own `path`. A + * resource, variable, schedule or trigger draft only ever lives at its own path. + * + * Being nameable and having a probe is the same property — a draft can only be staged under + * a name some deployed item might also hold — so the set below is derived from these keys + * rather than repeated. Listing a kind here without a probe would let a draft staged under a + * deployed item's name win that name, inverting the rule the resolution is built on. */ -const NAMEABLE_DRAFT_KINDS: ReadonlySet = new Set(['script', 'flow', 'raw_app']) - -/** Deployed-existence probe per nameable kind — the same three kinds, since only an item - * that can be deployed can leave a draft staged under its old name. */ const deployedExists: Partial< Record Promise> > = { @@ -331,6 +331,10 @@ const deployedExists: Partial< raw_app: (workspace, path) => AppService.existsApp({ workspace, path }) } +const NAMEABLE_DRAFT_KINDS: ReadonlySet = new Set( + Object.keys(deployedExists) as UserDraftItemKind[] +) + /** What a path resolved to: where the draft is stored, and its value when reaching it * already fetched one, so a read does not ask for the same draft twice. */ type ResolvedDraft = { storagePath: string; value?: unknown }