diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index c5739a0b4c..0f728df41f 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -1288,10 +1288,11 @@ export class AIChatManager { typeof this.systemMessage.content === 'string' ? this.systemMessage.content.length / tokenPerCharacter : 0 + // The filtered set, matching what the request actually carries: a restricted + // session ships fewer definitions than `this.tools` holds. + const tools = filterSessionTools(this.tools, this.sessionAccess) const toolTokens = - this.tools.length > 0 - ? JSON.stringify(this.tools.map((t) => t.def)).length / tokenPerCharacter - : 0 + tools.length > 0 ? JSON.stringify(tools.map((t) => t.def)).length / tokenPerCharacter : 0 return systemTokens + toolTokens } @@ -2175,8 +2176,14 @@ export class AIChatManager { } const pipeline = this.pipelineAiChatHelpers const mcpTools = createMcpTools(this.mcpServers) + // Every tool source assembled below needs an entry in SESSION_TOOL_POLICIES: the + // session filter fails closed, so a source added here without one is withheld from + // restricted sessions. sessionToolset.test.ts enumerates these sources to catch it. if (pipeline) { - systemMessage.content += getPipelinePromptSection(pipeline.getPipelineContext()) + systemMessage.content += getPipelinePromptSection( + pipeline.getPipelineContext(), + this.sessionAccess + ) this.tools = [ ...globalToolsFor({ sessionPreview: this.isSessionChat }), ...pipelineTools, @@ -2301,7 +2308,10 @@ export class AIChatManager { } const pipeline = this.pipelineAiChatHelpers if (pipeline) { - systemMessage.content += getPipelinePromptSection(pipeline.getPipelineContext()) + systemMessage.content += getPipelinePromptSection( + pipeline.getPipelineContext(), + this.sessionAccess + ) } this.systemMessage = systemMessage } diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 0ff9668570..31e24157b6 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1301,7 +1301,7 @@ The current user's workspace username is "${username}".${instanceLine} ${ canWriteDraft ? 'Use tools to inspect workspace items and create per-user drafts (saved server-side, visible only to this user — not deployed) for scripts, flows, schedules, triggers, resources, variables, and raw apps.' - : "Use tools to inspect workspace items and the workspace's run history. You have no tools to create or change anything here — this user's role does not allow it — so when they ask for a change, say plainly that you cannot make it rather than describing steps as if you had." + : "Use tools to inspect workspace items and the workspace's run history, and to run items that are already deployed. You cannot create, edit or deploy anything here — this user's role does not allow it — so when they ask for a change, say plainly that you cannot make it rather than describing steps as if you had." }${when( // Every line here is about choosing a path for something NEW, down to the folder // guidance; with nothing to create, the whole block is context tax. @@ -1392,10 +1392,7 @@ ${ } - open_page opens its page as a tab in the side-panel preview next to the chat — the only way to show one of these pages there (open_preview only handles editable items). Changing filters on a page already open updates that same tab; only pass new_tab when the user explicitly asks for a separate tab. - create_artifact saves a persistent markdown document (a planning doc, design write-up, spec, or other longer structured output) shown in the session preview panel. Prefer it over a long inline reply for content the user will revisit; keep brief answers inline. To revise one, call list_artifacts then read_artifact for the current content, then update_artifact to overwrite it — never create a second artifact for the same document. Each content change is saved as a version, keeping the most recent ones: use list_artifact_versions and read_artifact's version argument to recover earlier wording the user asks to go back to, rather than rewriting it from memory. list_artifact_versions is the source of truth for what is still available — do not assume a version that is not listed. -- The artifact whose \`role\` is \`plan\` is this session's plan document — one per session, surviving \`/clear\` — and \`approvedVersion\` is the version the user signed off. Below \`version\` means the current text is a proposal they have not agreed to, usually one they turned down; absent means nothing here was ever approved. In either case never describe the current text as agreed or build from it: ask what they want changed, or read the version they did agree to with read_artifact. An agreed plan is an ordinary artifact: revise it the same way${when( - canWriteDraft, - ', and do not call exit_plan_mode to amend it — that tool only exists while plan mode is active' - )}. Update it when the work parts ways with it (a step turns out unnecessary, an approach has to change, scope grows), not after every step you complete. Never quietly rewrite it to describe what you already built: say in your reply how the work now differs from the approved plan, then update the document. Updating it asks the user to approve nothing, so that sentence in your reply is their only chance to object before you keep building.` +- The artifact whose \`role\` is \`plan\` is this session's plan document — one per session, surviving \`/clear\` — and \`approvedVersion\` is the version the user signed off. Below \`version\` means the current text is a proposal they have not agreed to, usually one they turned down; absent means nothing here was ever approved. In either case never describe the current text as agreed or build from it: ask what they want changed, or read the version they did agree to with read_artifact. An agreed plan is an ordinary artifact: revise it the same way, and do not call exit_plan_mode to amend it — that tool only exists while plan mode is active. Update it when the work parts ways with it (a step turns out unnecessary, an approach has to change, scope grows), not after every step you complete. Never quietly rewrite it to describe what you already built: say in your reply how the work now differs from the approved plan, then update the document. Updating it asks the user to approve nothing, so that sentence in your reply is their only chance to object before you keep building.` : '' } diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts index 4aa95bf37d..9f0252083f 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts @@ -1,12 +1,19 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { whoami, deployPermission } = vi.hoisted(() => ({ +const { whoami, deployPermission, protectionRules } = vi.hoisted(() => ({ whoami: vi.fn(), - deployPermission: vi.fn() + deployPermission: vi.fn(), + protectionRules: vi.fn() })) vi.mock('$lib/gen', () => ({ UserService: { whoami } })) vi.mock('$lib/utils_workspace_deploy', () => ({ checkDeployPermission: deployPermission })) +// Only the fetch is stubbed — the bypass evaluation is the real one, so the test +// exercises the rule semantics rather than a restatement of them. +vi.mock('$lib/workspaceProtectionRules.svelte', async (importOriginal) => ({ + ...(await importOriginal()), + fetchProtectionRulesForWorkspace: protectionRules +})) import { resolveSessionAccess } from './sessionAccess' @@ -39,6 +46,7 @@ describe('resolveSessionAccess', () => { beforeEach(() => { vi.clearAllMocks() deployPermission.mockResolvedValue({ ok: true }) + protectionRules.mockResolvedValue([]) }) it('gives a developer every capability', async () => { @@ -86,7 +94,33 @@ describe('resolveSessionAccess', () => { it('presents a superadmin as admin to the deploy check', async () => { whoami.mockResolvedValueOnce(user({ is_super_admin: true })) await resolveSessionAccess('ws') - expect(deployPermission).toHaveBeenCalledWith('ws', expect.objectContaining({ is_admin: true })) + expect(deployPermission).toHaveBeenCalledWith( + 'ws', + expect.objectContaining({ is_admin: true }), + // An admin bypasses every ruleset, so none are fetched to hand over. + undefined + ) + }) + + // `checkDeployPermission` covers only the operator and RestrictDeployToDeployers halves + // of the gate, but the endpoints the deploy tools call run the backend's + // `check_deploy_rules`, which blocks on DisableDirectDeployment too. Drafting is + // untouched by that rule — it is the deploy that the workspace refuses. + it('withholds deploy under DisableDirectDeployment without a bypass', async () => { + protectionRules.mockResolvedValue([ + { name: 'lock', rules: ['DisableDirectDeployment'], bypass_users: [], bypass_groups: [] } + ]) + const caps = await capabilitiesFor({}) + expect(caps.has('deploy')).toBe(false) + expect(caps.has('write_draft')).toBe(true) + }) + + it('keeps deploy under DisableDirectDeployment for a bypass user', async () => { + protectionRules.mockResolvedValue([ + { name: 'lock', rules: ['DisableDirectDeployment'], bypass_users: ['u'], bypass_groups: [] } + ]) + const caps = await capabilitiesFor({}) + expect(caps.has('deploy')).toBe(true) }) // Fail open, matching checkDeployPermission: a transient whoami failure must not diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts index 11b1c73cc5..d38ff3da21 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts @@ -1,5 +1,9 @@ import { UserService, type User } from '$lib/gen' import { checkDeployPermission } from '$lib/utils_workspace_deploy' +import { + canUserBypassRuleKindInRulesets, + fetchProtectionRulesForWorkspace +} from '$lib/workspaceProtectionRules.svelte' /** * What a user may do in ONE workspace, as the AI session toolset needs to know it. @@ -80,7 +84,24 @@ export async function resolveSessionAccess(workspace: string): Promise { // produces invented calls and promises the chat cannot keep. // // Asserted over the ASSEMBLED message, not `prepareGlobalSystemMessage` alone: what - // actually ships is that plus the session-state section and plan mode's decoration, - // each appended by a different caller, and gating only the first looks correct while - // the other two still name withheld tools. Both axes are swept — every reachable - // profile, and every tool from the policy table — so neither a new tool nor a new - // capability combination slips past. + // actually ships is that plus the session-state section, the pipeline-editor section + // and plan mode's decoration, each appended by a different caller, and gating only the + // first looks correct while the others still name withheld tools. Both axes are swept — + // every reachable profile, and every tool from the policy table — so neither a new tool + // nor a new capability combination slips past. it.each([ ['read-only', []], ['drafts, no deploy', ['write_draft', 'run_preview']], @@ -139,7 +139,13 @@ describe('session tool policies', () => { }) msg = { ...msg, - content: (msg.content as string) + getSessionContextPromptSection(ctx, access) + content: + (msg.content as string) + + getSessionContextPromptSection(ctx, access) + + getPipelinePromptSection( + { folder: 'my_pipeline', mode: 'edit', nodes: [], assets: [] }, + access + ) } // Both decoration variants: the escalation one adds its own tool mentions. for (const blocks of [0, 9]) { diff --git a/frontend/src/lib/components/copilot/chat/pipeline/core.ts b/frontend/src/lib/components/copilot/chat/pipeline/core.ts index f696b1c869..f89fc7a38f 100644 --- a/frontend/src/lib/components/copilot/chat/pipeline/core.ts +++ b/frontend/src/lib/components/copilot/chat/pipeline/core.ts @@ -2,6 +2,7 @@ import { z } from 'zod' import { $ScriptLang } from '$lib/gen/schemas.gen' import type { ScriptLang } from '$lib/gen' import { createToolDef, executeTestRun, findAndReplace, type Tool } from '../shared' +import type { SessionAccess } from '../global/sessionAccess' import type { PipelineOutputKind } from '$lib/components/assets/AssetGraph/pipelineTemplates' // ============================================================================ @@ -331,8 +332,14 @@ export const pipelineTools: Tool[] = [ * /pipeline editor is open. Describes the annotation model and the direct-draft * workflow so the model uses the pipeline tools rather than the generic * write_script draft tools. + * + * `access` is the session's resolved capabilities (undefined outside a session, or + * before they resolve). The node-authoring guidance is dropped without `write_draft`, + * since the tools it names are withheld from that session; the annotation model stays, + * as it is what lets the model read and explain an existing pipeline. */ -export function getPipelinePromptSection(ctx: PipelineContext): string { +export function getPipelinePromptSection(ctx: PipelineContext, access?: SessionAccess): string { + const canWriteDraft = !access || access.capabilities.has('write_draft') return ` Data Pipeline editor (ACTIVE): @@ -342,8 +349,12 @@ Data Pipeline editor (ACTIVE): - \`materialize\` (the managed output): a managed \`// materialize ducklake:///\` means the runtime writes the node's output table FOR you — write the body as a single SELECT and the runtime wraps it in the create/replace, so do NOT also write your own CREATE TABLE / INSERT. The \`dbt://\` target below is the opposite: the node writes its own DDL and none of the write strategies apply to it. IMPORTANT: a MANAGED \`// materialize\` is **DuckDB-only** and its target MUST be a DuckLake table (\`ducklake:///
\`) — deploy rejects a \`ducklake://\` target on any other language. For a \`python3\`/\`bun\`/\`postgresql\` node writing the lake, do NOT use \`// materialize\`; write the output via the SDK instead (e.g. \`wmill.writeS3File(...)\`, a \`CREATE TABLE\` in postgresql, or \`wmill.databaseUrlFromResource\`/ducklake helpers) and let the output be inferred. Reach for \`duckdb\` when a node should materialize a DuckLake table. The one target any language BUT DBT'S OWN may declare (a dbt project's writes come from its manifest, so \`// materialize\` on a dbt script is rejected at deploy) is a WAREHOUSE RELATION: \`// materialize manual dbt:////\`, with \`\` a warehouse the workspace configures under Settings → dbt. \`manual\` is its only mode — nothing generates warehouse DDL, so the node issues its own write and the annotation records the outcome. Use it on an ingestion node a dbt project reads as a \`source\`: the declared relation and the dbt model become ONE graph node, and a downstream \`// on dbt:////\` fires when that node completes. Write strategy: with no option it REPLACES the whole table each run (full refresh; the only mode whose output columns may change); \`// materialize append\` INSERT-appends rows (incremental); \`// materialize key=\` merges/upserts on \`\`. \`// materialize manual \` opts OUT of managed writes — the script writes its own DDL and the annotation only records the output asset for lineage. \`materialize\` is paired with partitioning for incremental pipelines: a \`// partitioned \` node runs once per partition (append/merge into a fixed-schema table), and the \`{partition}\` token — usable in any asset URI AND in the body SQL — is substituted with the current partition's IDENTITY string at run time. To filter the source to the active slice on a time grain, use the runtime-injected macro: \`WHERE wm_partition() = {partition}\`. \`wm_partition(ts)\` buckets a timestamp with the exact identity format the runtime used (daily/hourly/weekly/monthly), so it always matches and you never hand-write a \`strftime\` format. Do NOT write \`= TIMESTAMP {partition}\`: the identity string is not a valid timestamp literal for hourly/weekly/monthly and errors at runtime. For \`dynamic\` partitioning the identity is your caller-supplied key (not a timestamp, no macro), so filter on it directly: \`WHERE = {partition}\`. \`materialize\` is an output DECLARATION on the node — it is not a command; there is no "materialize run". - \`measure\` / \`dimension\` (declared metrics): on a node that materializes a DuckLake table, \`// measure = [where ]\` names the canonical way to aggregate that table (e.g. \`// measure revenue = sum(amount) where not is_refund\`), and \`// dimension = \` names a way to slice it (e.g. \`// dimension region = region\`, \`// dimension month = date_trunc('month', ordered_at)\`). They execute nothing: they are catalogued at deploy so the editor and other agents can reuse the definition instead of re-deriving it and silently disagreeing. Keep the predicate in the \`where\` clause rather than folding it into the aggregate: it is rendered as \` FILTER (WHERE )\`, which is what lets two measures with different predicates sit under one GROUP BY. DuckLake-only, and only meaningful next to \`// materialize\`. Declare one when a number carries a judgement call someone else would get wrong (refunds excluded, test rows dropped, which column is the amount); do NOT blanket every table with measures, an obvious \`count(*)\` earns nothing. To USE a metric another node declares, read that node with read_pipeline_node and reuse its exact expression rather than guessing it. - Use get_pipeline_graph to see the current nodes/assets/triggers, and read_pipeline_node before editing one. -- Every node of this pipeline lives at \`f/${ctx.folder}/\` — \`${ctx.folder}\` is the folder name and \`f/\` is the owner prefix every workspace path carries, so write it exactly once (never \`f/f/…\`, and never a bare \`\`). +- Every node of this pipeline lives at \`f/${ctx.folder}/\` — \`${ctx.folder}\` is the folder name and \`f/\` is the owner prefix every workspace path carries, so write it exactly once (never \`f/f/…\`, and never a bare \`\`).${ + canWriteDraft + ? ` - Build new nodes with build_pipeline_node and edit existing ones with edit_pipeline_node. These apply directly as unsaved drafts on the canvas (like the flow/script editor applies AI edits) — they DO NOT deploy. There is no separate Accept/Reject step. Prefer these over the generic write_script/edit_script draft tools while a pipeline is open. - Reuse existing asset paths from the graph when wiring a downstream node to an upstream one (read the upstream's write asset, then \`// on\` that same URI). - Only deploy when the user explicitly asks; the user deploys drafts from the canvas.` + : '' + }` }