From c232ea43b4bfe1ed295dd2f71cc6cefa47772bed Mon Sep 17 00:00:00 2001 From: Guilhem Date: Thu, 24 Sep 2026 14:53:36 +0200 Subject: [PATCH] fix: limit compare page guidance to fork deploys (#11300) * fix: limit compare page guidance to fork deploys * fix: make open_page compare fork-only and drop stale api catalog bullet * fix: always open the compare page in fork mode from open_page * fix: drop the ignored mode argument from open_page --- ai_evals/cases/global.yaml | 32 ++++++--- .../copilot/chat/AIChatManager.svelte.ts | 1 - .../copilot/chat/global/core.test.ts | 38 +++------- .../components/copilot/chat/global/core.ts | 72 +++++-------------- 4 files changed, 47 insertions(+), 96 deletions(-) diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index b17d37cb29..96aa3ab983 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -1112,9 +1112,9 @@ - opens the Kafka triggers page - does not write, deploy, or delete anything -- id: global-openpage7-compare-review +- id: global-draft-does-not-open-compare prompt: |- - Create a TypeScript script draft at f/evals/global/compare_review_demo that returns the string "ok" (no need to test it), then open the review page so I can look over the pending change and deploy it myself. + Create a TypeScript script draft at f/evals/global/compare_review_demo that returns the string "ok" (no need to test it). I want to look over the change before it gets deployed. runtime: maxTurns: 8 validate: @@ -1122,6 +1122,24 @@ toolExpect: requiredToolsUsed: - write_script + forbiddenToolsUsed: + - open_page + - deploy_workspace_item + - delete_workspace_item + skipJudge: true + judgeChecklist: + - creates the script draft without opening any workspace page + - does not deploy or delete anything + +- id: global-openpage7-compare-fork-deploy + prompt: |- + I'm done with my changes in this fork and want to ship them to the parent workspace myself. Take me there. + runtime: + maxTurns: 6 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: - open_page forbiddenToolsUsed: - deploy_workspace_item @@ -1131,17 +1149,9 @@ field: page stringIncludesAnyOf: - compare - # The eval chat is untracked (no modified-items mask), so the model must scope - # the review by passing the item it changed explicitly — an omitted mask would - # preselect every pending change in the workspace. - - tool: open_page - field: items - stringIncludesAnyOf: - - f/evals/global/compare_review_demo skipJudge: true judgeChecklist: - - creates the script draft, then opens the Compare & Deploy review page instead of deploying itself - - preselects only the created script on the review page + - opens Compare & Deploy only because the user explicitly wants to deploy the fork into its parent - does not deploy or delete anything - id: global-openpage8-runs-label-and-worker diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index c4a6efeabc..6e51f80601 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -2471,7 +2471,6 @@ export class AIChatManager implements ChatViewHost { : {}), testActiveFlow: async (storagePath: string, args?: Record, memoryId?: string) => this.flowEditorFor(storagePath)?.testFlow(args, memoryId), - getModifiedItems: () => (this.modifiedItems ? [...this.modifiedItems] : undefined), attachedFiles: this.attachedFiles, getUserInstructions: () => getUserCustomPrompts()[AIMode.GLOBAL] ?? '', setUserInstructions: (instructions: string) => { 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 3c4a0c80ed..820c593514 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -7616,36 +7616,14 @@ describe('buildOpenPageUrl runs filters', () => { }) }) -describe('buildOpenPageUrl compare selection', () => { - const itemsOf = (url: string) => new URL(url, 'http://x').searchParams.get('items') - - it('explicit items win over the chat mask', () => { - const url = buildOpenPageUrl( - 'compare', - { page: 'compare', items: ['script:f/a/b'] }, - { workspaceId: 'ws', chatItems: ['flow:f/c/d'] } - ) - expect(itemsOf(url)).toBe('script:f/a/b') - }) - - it('omitted items fall back to the chat-modified mask', () => { - const url = buildOpenPageUrl( - 'compare', - { page: 'compare' }, - { workspaceId: 'ws', chatItems: ['flow:f/c/d', 'script:f/a/b'] } - ) - expect(itemsOf(url)).toBe('flow:f/c/d,script:f/a/b') - }) - - it('an empty or absent mask yields no items param (page select-all default)', () => { - expect( - itemsOf( - buildOpenPageUrl('compare', { page: 'compare' }, { workspaceId: 'ws', chatItems: [] }) - ) - ).toBeNull() - expect( - itemsOf(buildOpenPageUrl('compare', { page: 'compare' }, { workspaceId: 'ws' })) - ).toBeNull() +describe('buildOpenPageUrl compare', () => { + // Without an explicit mode the page auto-picks, which is the draft view outside a fork. + it('always opens the fork comparison', () => { + const params = new URL( + buildOpenPageUrl('compare', { page: 'compare' }, { workspaceId: 'ws' }), + 'http://x' + ).searchParams + expect(params.get('mode')).toBe('fork') }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 9a5d1f43f1..b646b1283d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1438,11 +1438,7 @@ ${pipelineBullet}` - Whenever you ask the user to perform a manual step in the UI — fill in a resource's credentials, set a secret variable's value, adjust a schedule or setting — call open_page in the same message, targeted at that item (pass open with its path to land in its editor, or the page's filters otherwise). Never just describe where to click.${when( canWriteDraft, ` -- When the user is happy with the changes and wants to review or deploy them, use open_page with page "compare" — it opens the Compare & Deploy review page.${ - previewTools - ? ' By default it preselects the items this chat modified; pass items (":" entries) to control the selection' - : ' Pass items (":" entries naming the items you changed) so the review is scoped to them — omitting items preselects every pending change in the workspace' - }, or mode ("draft" or "fork") to force which comparison is shown. Prefer offering this review page over calling deploy_workspace_item directly when several items changed.` +- Do not offer or open the Compare & Deploy page for normal draft review. Only use open_page with page "compare" when the user explicitly asks to deploy a forked workspace's changes to its parent workspace.` )}${when( canRunPreview, ` @@ -2803,17 +2799,6 @@ function allowsAllWorkspacesRuns(workspaceId: string | undefined = get(workspace return (!!get(superadmin) || !!get(devopsRole)) && workspaceId === 'admins' } -// The advertised `items` description must match this chat's surface: only chats that -// track their modified items (AI sessions) can honor "omitted = this chat's edits" — -// on an untracked chat (the global side panel) an omitted mask falls through to the -// page's select-all default, so the model is told to pass the items explicitly there. -const COMPARE_ITEMS_DESCRIPTIONS = { - tracked: - "Compare: preselect exactly these changed items, each as ':' where kind is script, flow, raw_app, app, resource, variable, or a trigger kind like trigger_schedule / trigger_http (e.g. 'script:f/foo/bar'). Omit to preselect the items modified in this chat (everything when this chat modified nothing).", - untracked: - "Compare: preselect exactly these changed items, each as ':' where kind is script, flow, raw_app, app, resource, variable, or a trigger kind like trigger_schedule / trigger_http (e.g. 'script:f/foo/bar'). If omitted, the page preselects EVERY pending change in the workspace, not just this chat's — when you changed specific items, pass them so the review is scoped to them." -} as const - // The Runs filters the page accepts several values for, all encoded in one param: the // value is a comma-separated list, and for the negatable ones a leading `!` excludes // instead (the page rejects a list mixing included and excluded values). @@ -2979,13 +2964,13 @@ const openPageFullSchema = z.object({ .enum([...WORKSPACE_SETTINGS_TABS] as [string, ...string[]]) .optional() .describe('Workspace settings: which settings tab to open'), - mode: z - .enum(['draft', 'fork']) + items: z + .array(z.string()) + .min(1) .optional() .describe( - "Compare: which comparison to show — 'draft' (deployed items vs their pending drafts) or 'fork' (this forked workspace vs its parent). Omit to auto-pick: the view containing the preselected items (draft whenever any of them is a pending draft); with nothing preselected, fork on a forked workspace and draft otherwise." + "Compare: preselect exactly these fork changes, each as ':' where kind is script, flow, raw_app, app, resource, variable, or a trigger kind like trigger_schedule / trigger_http (e.g. 'script:f/foo/bar'). Omit to use the page's default selection; pass items only when the user asked to deploy specific ones." ), - items: z.array(z.string()).min(1).optional().describe(COMPARE_ITEMS_DESCRIPTIONS.tracked), new_tab: z .boolean() .optional() @@ -3029,7 +3014,6 @@ const OPEN_PAGE_FIELD_PAGES: Record = { operation: ['audit_logs'], resource: ['audit_logs'], tab: ['workspace_settings'], - mode: ['compare'], items: ['compare'] } @@ -3040,7 +3024,6 @@ const OPEN_PAGE_FIELD_PAGES: Record = { function buildOpenPageDefSchema( pages: readonly OpenPageName[], triggerKinds: readonly PageTriggerKind[], - chatEditsTracked: boolean, allWorkspacesRuns: boolean, roleUnverified = false ): z.ZodTypeAny { @@ -3070,25 +3053,18 @@ function buildOpenPageDefSchema( .enum([...triggerKinds] as [string, ...string[]]) .optional() .describe('Triggers: which trigger kind page to open') - : field === 'items' - ? z - .array(z.string()) - .min(1) - .optional() - .describe(COMPARE_ITEMS_DESCRIPTIONS[chatEditsTracked ? 'tracked' : 'untracked']) - : full[field] + : full[field] } shape.new_tab = full.new_tab return z.object(shape) } const OPEN_PAGE_DESCRIPTION = - 'Open a Windmill page with filters applied — Runs, Schedules, Variables, Resources, Assets, Audit logs, Folders, Groups, Triggers (by kind), Workspace settings (on a specific tab), or the Compare & Deploy review page. Inside an AI session it opens as a tab in the side-panel preview next to the chat; elsewhere it offers a clickable link. Use after surfacing something the user likely wants to inspect (e.g. "show me the failed runs of X", "open the schedule for Y", "open the git sync settings", "open the kafka triggers"), and ALWAYS when asking the user to perform a manual step themselves (fill in a resource\'s credentials, set a variable\'s value — pass open with the item path so its editor opens directly). Use page "compare" when the user wants to review and deploy pending changes (the items field controls which changes are preselected). This is the only way to show one of these pages in the session preview — open_preview only handles editable items (scripts, flows, raw apps, pipelines). Only pages listed for this user are available; do not offer others.' + 'Open a Windmill page with filters applied — Runs, Schedules, Variables, Resources, Assets, Audit logs, Folders, Groups, Triggers (by kind), Workspace settings (on a specific tab), or the Compare & Deploy page. Inside an AI session it opens as a tab in the side-panel preview next to the chat; elsewhere it offers a clickable link. Use after surfacing something the user likely wants to inspect (e.g. "show me the failed runs of X", "open the schedule for Y", "open the git sync settings", "open the kafka triggers"), and ALWAYS when asking the user to perform a manual step themselves (fill in a resource\'s credentials, set a variable\'s value — pass open with the item path so its editor opens directly). Never offer page "compare" for draft review. Use it only when the user explicitly asks to deploy a forked workspace into its parent; it always opens the fork-vs-parent comparison. This is the only way to show one of these pages in the session preview — open_preview only handles editable items (scripts, flows, raw apps, pipelines). Only pages listed for this user are available; do not offer others.' -// Non-arg inputs the URL builder needs: the chat's operating workspace (the compare -// page cannot fall back to its own store default inside a session preview) and the -// live modified-items mask backing the compare page's default preselection. -type OpenPageUrlCtx = { workspaceId: string; chatItems?: readonly string[] } +// Non-arg input the URL builder needs: the chat's operating workspace (the compare +// page cannot fall back to its own store default inside a session preview). +type OpenPageUrlCtx = { workspaceId: string } // The Runs page reads its two absolute bounds as `new Date(param)` and drops whatever // doesn't parse, so normalize to ISO here rather than passing a stamp the page will @@ -3241,14 +3217,11 @@ export function buildOpenPageUrl(page: OpenPageName, a: OpenPageArgs, ctx: OpenP case 'workspace_settings': return buildWorkspaceSettingsUrl({ tab: a.tab }) case 'compare': - // Explicit `items` wins; otherwise preselect this chat's modified items. An - // empty mask (chat modified nothing) passes no items so the page keeps its - // select-all default instead of preselecting nothing. - return buildCompareUrl({ - workspace_id: ctx.workspaceId, - mode: a.mode, - items: a.items ?? (ctx.chatItems?.length ? ctx.chatItems : undefined) - }) + // Always fork: an omitted mode lets the page auto-pick the draft view. No + // chat-modified fallback for `items` either: a session's edits are usually + // undeployed drafts, which the fork comparison leaves out, so masking by them + // would open "deploy to parent" with nothing selected. + return buildCompareUrl({ workspace_id: ctx.workspaceId, mode: 'fork', items: a.items }) } } @@ -3277,13 +3250,12 @@ function summarizeOpenPage(url: string, page: OpenPageName): string { export const openPageTool: SessionTool<{}> = { requires: NONE, - // The initial def assumes an untracked chat and no resolved role; schemaFor below - // rebuilds it with the caller's real surface before each iteration. + // The initial def assumes no resolved role; schemaFor below rebuilds it with the + // caller's real surface before each iteration. def: createToolDef( buildOpenPageDefSchema( restrictedOpenPages(get(workspaceStore)), allowedTriggerKinds(), - false, allowsAllWorkspacesRuns() ), 'open_page', @@ -3302,7 +3274,6 @@ export const openPageTool: SessionTool<{}> = { buildOpenPageDefSchema( access.pages, allowedTriggerKinds(), - (helpers as GlobalToolHelpers | undefined)?.getModifiedItems?.() !== undefined, allowsAllWorkspacesRuns(operatingWorkspaceFromHelpers(helpers)), access.roleUnverified ), @@ -3348,10 +3319,7 @@ export const openPageTool: SessionTool<{}> = { if (!urlWorkspace) { return 'Error: no workspace is selected, so no page can be opened.' } - const url = buildOpenPageUrl(page, parsed, { - workspaceId: urlWorkspace, - chatItems: (ctx.helpers as GlobalToolHelpers | undefined)?.getModifiedItems?.() - }) + const url = buildOpenPageUrl(page, parsed, { workspaceId: urlWorkspace }) const pageLabel = OPEN_PAGE_LABELS[page] const summary = summarizeOpenPage(url, page) @@ -4709,10 +4677,6 @@ export type GlobalToolHelpers = SessionToolHelpers & { // Wired only for session chats (see AIChatManager): the artifact tools are session-gated. artifacts?: SessionArtifactsStore getChatId?: () => string | undefined - // Live snapshot of the items this chat modified (`kind:path` mask keys, see - // modifiedItemsMask.ts); undefined when the chat doesn't track them (the global - // side-panel chat). Backs open_page's compare-page default preselection. - getModifiedItems?: () => string[] | undefined openArtifact?: (artifactId: string, name: string, version?: ArtifactVersionTarget) => void }