mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
docs: correct and tighten comments on the session capability filter
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
51591905c5
commit
3816f617d2
@@ -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.
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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<SessionAc
|
||||
|
||||
// windmill-api/src/drafts.rs `require_can_write_path`: `authed.is_admin` returns Ok
|
||||
// BEFORE the operator branch, so an admin who is also an operator may still save
|
||||
// drafts. Every chat write tool funnels through the draft lifecycle, which is why
|
||||
// this single capability covers scripts, flows, apps, resources, variables,
|
||||
// schedules and triggers alike.
|
||||
// drafts.
|
||||
if (isAuthedAdmin(me) || !me.operator) {
|
||||
capabilities.add('write_draft')
|
||||
}
|
||||
|
||||
// windmill-api/src/jobs.rs `run_preview_script` / `run_dynamic_select`: the operator
|
||||
// windmill-api/src/jobs.rs `run_preview_script` / `run_preview_flow_job`: the operator
|
||||
// check comes first and has no admin escape — the opposite precedence to drafts.
|
||||
if (!me.operator) {
|
||||
capabilities.add('run_preview')
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// The toolset pulls in the script/flow editor tools, hence monaco. Same stand-ins
|
||||
// as global/core.test.ts — this suite only ever reads `def.function.name`.
|
||||
// The toolset pulls in the script/flow editor tools, hence monaco. Same stand-ins as
|
||||
// global/core.test.ts; nothing here executes a tool, so bare shapes are enough.
|
||||
vi.mock('monaco-editor', () => ({
|
||||
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)
|
||||
|
||||
@@ -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<string, SessionToolPolicy> = {
|
||||
// ── Reads, docs and conversation ────────────────────────────────────────
|
||||
@@ -60,8 +59,9 @@ export const SESSION_TOOL_POLICIES: Record<string, SessionToolPolicy> = {
|
||||
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<string, SessionToolPolicy> = {
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user