starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-10-13 00:03:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf: settle a deployed path without listing the workspace's drafts

Browse Source 
Round-5 review findings.

The deployed-item check now runs before the drafts listing again, but a failed
probe is remembered rather than swallowed: it only matters once something is staged
under the name, and there the call stops instead of guessing which draft it means.
That keeps the cheap short-circuit for a path that is simply where something is
deployed — one indexed row, no listing — without the exposure that made the first
ordering unsafe for a delete or a deploy.

A kind no editor stages a name for skips the lookup entirely: those reach neither
the probe nor the listing, where before they always paid a workspace-wide read that
checks write access per row. The trigger test that pinned name routing for such a
kind now pins what the declaration is actually for — finding the path nested inside
`config` — since the state it set up, an http trigger draft carrying `draft_path`,
is one no editor or drawer produces.

An empty path reached `draftTargetOf`, whose message blamed the tool definition for
what is a malformed argument, and the model reads that string. It names the cause.

Measured on the running instance: a read of a deployed script makes 0 listings and
1 exists; a variable write makes 0 listings; a script write to a fresh path still
lists, since nothing is deployed there to settle it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
2026-09-25 17:24:43 +02:00
co-authored by Claude Opus 5
parent 7ce85a4fde
commit c2053bcbfc
3 changed files with 39 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/components/copilot/chat/global/core.test.ts
+9 -11
View File
@@ -3157,14 +3157,14 @@ describe('global AI tools', () => {
})
// write_trigger is the one tool whose path is nested inside another argument, so its
// target declaration has to reach into `config` to find the name to resolve.
it('routes a trigger write nested in config to the draft staged under that name', async () => {
// target declaration has to reach into `config` for it. Without that the call resolves
// nothing and never reaches the draft it means to update.
it('resolves a trigger write through the path nested in config', async () => {
seedBackendDraft(
'trigger_http',
'u/admin/draft_http_0001',
{
path: 'u/admin/draft_http_0001',
draft_path: 'u/admin/staged_route',
script_path: 'f/scripts/handler',
is_flow: false,
route_path: 'api/staged',
@@ -3178,7 +3178,7 @@ describe('global AI tools', () => {
await callGlobalTool('write_trigger', {
kind: 'http',
config: {
path: 'u/admin/staged_route',
path: 'u/admin/draft_http_0001',
script_path: 'f/scripts/handler',
is_flow: false,
route_path: 'api/renamed',
@@ -3188,13 +3188,11 @@ describe('global AI tools', () => {
}
})
expect(
getBackendDraft<any>('trigger_http', 'u/admin/draft_http_0001', { workspace: WORKSPACE })
?.route_path
).toBe('api/renamed')
expect(
getBackendDraft('trigger_http', 'u/admin/staged_route', { workspace: WORKSPACE })
).toBeUndefined()
const draft = getBackendDraft<any>('trigger_http', 'u/admin/draft_http_0001', {
workspace: WORKSPACE
})
expect(draft?.route_path).toBe('api/renamed')
expect(draft?.script_path).toBe('f/scripts/handler')
})
// A script draft's chosen name IS `value.path`, and the deployer deploys at the path
frontend/src/lib/components/copilot/chat/global/core.ts
+4 -3
View File
@@ -4750,11 +4750,12 @@ function draftTarget(
}
}
/** The draft this call addresses. Absent means the tool did not declare `draftTarget`;
* failing here beats writing to a path that was never resolved to a storage key. */
/** The draft this call addresses. Failing here beats writing to a path that was never
* resolved to a storage key. Reachable when the call named an empty path, which the schemas
* accept — so the message names that rather than the declaration a reader cannot fix. */
function draftTargetOf(ctx: WriteDraftCtx): ResolvedDraftTarget {
if (!ctx.target) {
throw new Error('Internal error: this tool did not declare the draft it addresses.')
throw new Error('This call named no item to act on. Pass the path of the item.')
}
return ctx.target
}
frontend/src/lib/components/copilot/chat/global/userDraftAdapter.ts
+26 -13
View File
@@ -368,6 +368,10 @@ async function storagePathForChosenName(
path: string,
opts: { forWrite?: boolean }
): Promise<string> {
// Nothing stages a name for this kind, so the path is already the key and there is no
// listing worth reading.
if (!NAMEABLE_DRAFT_KINDS.has(itemKind)) return path
const staged: StagedDraft[] = []
const add = (storagePath: string, summary: string | undefined) => {
// A new script or flow editor stores under '', which no tool call can address.
@@ -375,7 +379,19 @@ async function storagePathForChosenName(
if (staged.some((s) => s.storagePath === storagePath)) return
staged.push({ storagePath, summary })
}
// A deployed item keeps its own path: drafts staged under that name belong to other
// items and are reached by their storage path. Asked before the listing, which is
// workspace-wide and checks write access per row — one indexed lookup settles the common
// case, where the path is simply where something is deployed.
const exists = deployedExists[itemKind]
let probeUnknown = false
try {
if (exists && (await exists(workspace, path))) return path
} catch {
// Unknown, not "nothing deployed": remembered, so a staged name below is never taken
// for this path's draft on the strength of a question that went unanswered.
probeUnknown = true
}
let rows: Awaited<ReturnType<typeof DraftService.listDrafts>>
try {
rows = await DraftService.listDrafts({ workspace })
@@ -383,14 +399,7 @@ async function storagePathForChosenName(
// Nothing is stored at this path, so it may be a name. Without the listing that
// cannot be told, and a write that guessed the path itself would create a second
// draft; a read of whatever is deployed there is harmless.
//
// For a kind no editor stages a name for, the path cannot be one, so there is
// nothing the listing would have told a write either — refusing it would fail a
// write that has no ambiguity to resolve.
if (!opts.forWrite || !NAMEABLE_DRAFT_KINDS.has(itemKind)) return path
// A deployed item keeps its own path whatever is staged, so the listing was not
// needed after all. Only refuse when that cannot be established either.
if (exists && (await exists(workspace, path).catch(() => false))) return path
if (!opts.forWrite) return path
throw new Error(
`Could not load this workspace's drafts, so "${path}" cannot be matched to the draft ` +
`it may name: ${e instanceof Error ? e.message : String(e)}. Try again.`
@@ -406,11 +415,15 @@ async function storagePathForChosenName(
}
}
if (staged.length === 0) return path
// A deployed item keeps its own path: drafts staged under that name belong to other
// items and are reached by their storage path. Consulted only once something is staged,
// so the common path costs no request and a failure here — which would leave a delete or
// a deploy pointing at a namesake draft — stops the call rather than being guessed past.
if (exists && (await exists(workspace, path))) return path
if (probeUnknown) {
// Something is staged under this name, and whether the path is also a deployed item's
// own is what says which draft the call means. Answering it either way could delete or
// deploy an unrelated draft.
throw new Error(
`Could not tell whether something is already deployed at "${path}", so the draft staged ` +
`under that name cannot be identified. Try again.`
)
}
if (staged.length > 1) {
// Refused rather than guessed: nothing checks a name for uniqueness before deploy.
const listed = staged