diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index d3a9d2b831..c5739a0b4c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -634,10 +634,8 @@ export class AIChatManager { // needs the preview pane; the global side-panel chat leaves it false. Reactive because // `planModeAvailable` derives from it. isSessionChat = $state(false) - // What the user may do in this session's operating workspace, resolved in the - // send pre-flight (see `resolveSessionAccessForSend`) and applied to the toolset - // handed to the chat loop. Undefined until the first send resolves it, and on the - // global side-panel chat, where it stays unfiltered. + // What the user may do in this session's operating workspace. Undefined until the + // first send resolves it — see `resolveSessionAccessForSend`. private sessionAccess: SessionAccess | undefined = undefined private sessionAccessGeneration = 0 autoAcceptEditsAvailable = $derived(supportsAutoAcceptEdits(this.mode)) @@ -2129,8 +2127,9 @@ export class AIChatManager { // stale resolves so workspace changes cannot overwrite newer skills. // Build the global-mode system message, tools, and helpers, layering on the // pipeline surface when a /pipeline editor has registered helpers. Centralized - // so changeMode, refreshGlobalSkills, and setPipelineHelpers stay consistent — - // each rebuild would otherwise drop the pipeline augmentation the others added. + // so changeMode, refreshGlobalSkills, setPipelineHelpers and the send pre-flight stay + // consistent — each rebuild would otherwise drop the pipeline augmentation the + // others added. // // Public because it is purely local, unlike `changeMode(GLOBAL)`, which also // fires the three network refreshes. diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index cabb6f763a..0ff9668570 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -2368,9 +2368,9 @@ export function getSessionContextPromptSection( ctx: SessionPromptContext, access?: SessionAccess ): string { - // This section is concatenated onto an already capability-gated prompt, so it has to - // honour the same profile: naming a tool the toolset withheld is what makes the model - // invent calls. Each branch keeps its "where work lands" fact either way. + // Concatenated onto an already capability-gated prompt, so it has to honour the same + // profile rather than assume the gating happened upstream. Each branch keeps its + // "where work lands" fact either way. const canDeploy = !access || access.capabilities.has('deploy') const canWriteDraft = !access || access.capabilities.has('write_draft') const canRunPreview = !access || access.capabilities.has('run_preview') diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts index 5c5bc3fbb8..11b1c73cc5 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts @@ -9,10 +9,9 @@ import { checkDeployPermission } from '$lib/utils_workspace_deploy' * point (the token is); this exists so the model is never handed a tool whose every * invocation would 401. * - * The rules below are NOT a role ladder. The backend's precedence between "admin" - * and "operator" differs per capability, and each rule cites the site it mirrors — - * copying the ladder instead would get `write_draft` wrong for a superadmin whose - * workspace role is operator. + * The rules below are NOT a role ladder: the backend's precedence between "admin" + * and "operator" differs per capability. Collapsing them into one ordering would get + * `write_draft` wrong for a superadmin whose workspace role is operator. */ export type SessionCapability = 'write_draft' | 'run_preview' | 'deploy' @@ -61,14 +60,12 @@ export async function resolveSessionAccess(workspace: string): Promise ({ editor: {}, languages: {}, @@ -109,8 +109,6 @@ describe('session tool policies', () => { expect(sessionToolAllowed('search_docs', readOnly)).toBe(true) }) - // Draft writes survive without `deploy`, and vice versa: the two are separate - // backend gates (drafts.rs vs. the deploy protection rules), not one ladder. // The prompt is documentation OF the toolset, so it must never name a tool the same // profile withheld — an instruction to call a tool the model was not given is what // produces invented calls and promises the chat cannot keep. @@ -143,7 +141,7 @@ describe('session tool policies', () => { ...msg, content: (msg.content as string) + getSessionContextPromptSection(ctx, access) } - // The escalation variant names exit_plan_mode a second time. + // Both decoration variants: the escalation one adds its own tool mentions. for (const blocks of [0, 9]) { const full = appendPlanModeInstructions(msg, blocks).content as string expect(withheld.filter((n) => full.includes(n))).toEqual([]) @@ -153,8 +151,9 @@ describe('session tool policies', () => { } ) - // Full access must reproduce the pre-capabilities prompt exactly, or every - // existing session's cached prefix and the ai_evals baseline move underneath us. + // A full-access profile must gate nothing at all: the text has to match the ungated + // build byte for byte, or every session's cached prefix and the ai_evals baseline + // move underneath us. it('builds an unchanged prompt when every capability is present', () => { const user = { username: 'alex', folders: ['shared'], folders_read: ['shared'] } for (const previewTools of [false, true]) { @@ -168,6 +167,8 @@ describe('session tool policies', () => { } }) + // Draft writes survive without `deploy`, and vice versa: the two are separate + // backend gates (drafts.rs vs. the deploy protection rules), not one ladder. it('treats write_draft and deploy as independent', () => { const draftsOnly = accessWith(['write_draft']) expect(sessionToolAllowed('write_script', draftsOnly)).toBe(true) diff --git a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts index 7a4cc40cdd..b91728d1eb 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts @@ -27,8 +27,7 @@ const RUN_PREVIEW: SessionToolPolicy = { requires: ['run_preview'] } * built by factories in other modules: a field on `globalTools` alone would look * exhaustive while silently missing them. * - * Completeness is enforced by a test that assembles the full session toolset and - * asserts every name resolves here — add the entry with the tool, not after. + * A tool with no entry here is withheld, so add the entry with the tool. */ export const SESSION_TOOL_POLICIES: Record = { // ── Reads, docs and conversation ──────────────────────────────────────── @@ -60,8 +59,9 @@ export const SESSION_TOOL_POLICIES: Record = { exec_datatable_sql: RUN_PREVIEW, // ── API catalog and MCP ───────────────────────────────────────────────── - // The wrapper tools themselves need nothing; the endpoint a call names is - // policed per-endpoint at call time (see apiCatalogTools). + // No capability needed: every endpoint these can reach is a read or a run-by-path. + // The authoring and delete endpoints are refused for everyone by COVERED_ENDPOINTS + // in apiCatalogTools, so there is no per-role cut left to make here. search_api_endpoints: NONE, call_api_get: NONE, call_api_endpoint: NONE, @@ -147,9 +147,8 @@ export const SESSION_TOOL_POLICIES: Record = { export function sessionToolAllowed(name: string, access: SessionAccess): boolean { const policy = SESSION_TOOL_POLICIES[name] - // An unregistered tool is withheld rather than advertised: the completeness - // test is what keeps this branch unreachable, so reaching it means a tool - // shipped without anyone deciding what it needs. + // Fails closed, so a tool that ships without a policy disappears from restricted + // sessions rather than leaking into them. if (!policy) return false if (policy.relevance === 'authoring' && !access.capabilities.has('write_draft')) { return false