diff --git a/lib/guard.js b/lib/guard.js index 12404df..cd34ae5 100644 --- a/lib/guard.js +++ b/lib/guard.js @@ -71,6 +71,35 @@ export function apply(ctx, config) { // the session was loaded with, for the life of the session. const modeNow = () => policy.mode + /** + * Whether an approval request from this session can reach a person. + * + * `ask` mode is a promise that an out-of-facet call is *put to the user*, and + * that promise needs a channel. A session can be composed without one: the + * `danger-full-access` permission preset sets `approval: never`, under which + * the approval service's `decide()` returns "rejected" before consulting any + * answerer. An ask is then a denial, and `facet action=add` — itself an ask — + * is denied with it, so the scope can never be widened while the model is told + * only `the user rejected tool "…"`. Naming that is the binding's job; see + * `facetContextText`. + * + * A missing service is a definite no (the tool pipeline denies an ask it + * cannot route). An unreadable policy is treated as a yes, so a transient + * failure does not tell a user to go and edit their configuration. + * + * @param {{session?: object}|undefined} agent - the agent whose session supplies the policy. + * @returns {boolean} whether an ask can reach a person. + */ + const canAsk = (agent) => { + const approval = ctx.get('approval') + if (approval === undefined || agent?.session === undefined) return false + try { + return approval.effectivePolicy(agent.session) !== 'never' + } catch { + return true + } + } + // `off` is the one part that stays a mount-time decision. Deciding it here // means an `off` deployment contributes no rule, no binding and no tool — // which is the point of the mode. The cost is that turning it *on* later needs @@ -187,7 +216,13 @@ export function apply(ctx, config) { text: (context) => { const scope = policy.scopeFor(context?.agent) if (scope === undefined) return '' - return facetContextText({ cwd: scope.cwd, root: scope.root, extras: scope.extras, mode: modeNow() }) + return facetContextText({ + cwd: scope.cwd, + root: scope.root, + extras: scope.extras, + mode: modeNow(), + canAsk: canAsk(context?.agent), + }) }, }) diff --git a/lib/scope.js b/lib/scope.js index fe368c8..dd60943 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -175,17 +175,31 @@ export function displayPath(cwd, path) { * @param {string|undefined} input.root - the facet root, or undefined when unscoped. * @param {readonly string[]} input.extras - absolute granted paths, outside the root. * @param {'ask'|'deny'|'off'} input.mode - how an out-of-scope call is handled. + * @param {boolean} [input.canAsk] - whether an approval request can reach the user. False means every ask is refused before anyone is consulted. * @returns {string} context text, or `''` to contribute nothing. */ export function facetContextText(input) { - const { cwd, root, extras = [], mode } = input + const { cwd, root, extras = [], mode, canAsk = true } = input if (mode === 'off' || typeof root !== 'string' || root === '') return '' // Stated in terms of what the model should *do*, because the same sentence is // what the user reads in an approval prompt. A denial that does not say how to // proceed is a denial the agent works around with `bash`. - const reach = - mode === 'deny' + // + // `canAsk` comes first because it collapses the other two. A session can be + // composed without an approval channel — the `danger-full-access` permission + // preset sets `approval: never`, and `decide()` then returns "rejected" before + // any answerer is consulted. There, an out-of-facet call is refused whatever + // `mode` says, and `facet action=add` is refused with it, because widening is + // itself a question. Telling the agent to ask would send it into a loop, and + // the reason this guard writes never reaches it: `serviceAsk` replaces a + // rejected ask with `the user rejected tool "…"`. So the binding is the one + // channel that can say what is true, and it names the mechanism that still + // works — `extraRoots` is configuration applied by the profile, not a grant + // asked for per conversation. + const reach = !canAsk + ? 'refused — approval prompts are off in this session, so `facet action=add` cannot be approved either; only the user can widen this, by adding the path to `extraRoots`' + : mode === 'deny' ? 'refused — `facet action=add` asks the user to grant a path, and is the only way to widen the scope' : 'put to the user as a question — say in one line what you need and why, then record it with `facet action=add` so the rest of the session does not have to ask again' diff --git a/test/guard.test.js b/test/guard.test.js index 012f4ab..65cd711 100644 --- a/test/guard.test.js +++ b/test/guard.test.js @@ -58,13 +58,22 @@ const fsLikeLocal = { }, } -/** Capture what `apply` registered, so the test can drive it. */ -function harness({ mode = 'ask', extraRoots = [], tool = true, config } = {}) { +/** + * Capture what `apply` registered, so the test can drive it. + * + * `approval` stands in for the approval service named by its policy: `'ask'` + * and `'never'` are the two the schema admits, and leaving it undefined models a + * deployment that composed no approval service at all — a case the tool pipeline + * denies outright, so the guard must treat it as no channel. + */ +function harness({ mode = 'ask', extraRoots = [], tool = true, config, approval } = {}) { const policy = createFacetPolicy(config ?? { mode, extraRoots }) const captured = { section: undefined, context: undefined, tool: undefined, preExecute: undefined } const ctx = { facetPolicy: policy, fs: fsLikeLocal, + get: (name) => + name === 'approval' && approval !== undefined ? { effectivePolicy: () => approval } : undefined, systemPrompt: { section: (value) => { captured.section = value @@ -275,6 +284,40 @@ test('the binding follows the agent, and is empty without one', () => { assert.equal(captured.context.text(undefined), '') }) +test('the binding refuses honestly when the session has no approval channel', () => { + // `danger-full-access` sets approval: never, so `decide()` returns "rejected" + // before any answerer is consulted — and `facet action=add`, itself an ask, is + // refused with it. The reason the guard writes never reaches the model on that + // path (`serviceAsk` replaces it with `the user rejected tool "…"`), so the + // binding is the only place the truth can be said. + const { captured } = harness({ mode: 'ask', approval: 'never' }) + const text = captured.context.text({ agent: agent() }) + + assert.match(text, /approval prompts are off in this session/) + assert.match(text, /cannot be approved either/) + assert.match(text, /`extraRoots`/) + + // The promise `ask` makes cannot be kept here, so it must not be made. + assert.doesNotMatch(text, /put to the user as a question/) +}) + +test('an approval channel keeps the promise, and a missing service counts as none', () => { + const withChannel = harness({ mode: 'ask', approval: 'ask' }) + assert.match(withChannel.captured.context.text({ agent: agent() }), /put to the user as a question/) + + // No approval service at all: the tool pipeline denies an ask it cannot route, + // so the binding must not offer one. + const without = harness({ mode: 'ask' }) + assert.match(without.captured.context.text({ agent: agent() }), /approval prompts are off in this session/) +}) + +test('deny mode still names the way that works when there is no channel', () => { + const { captured } = harness({ mode: 'deny', approval: 'never' }) + const text = captured.context.text({ agent: agent() }) + assert.match(text, /refused/) + assert.match(text, /`extraRoots`/) +}) + test('the facet tool reports the scope', async () => { const { captured } = harness() const value = await captured.tool.execute({ action: 'status' }, { agent: agent() }) diff --git a/test/scope.test.js b/test/scope.test.js index e41cade..bddaca3 100644 --- a/test/scope.test.js +++ b/test/scope.test.js @@ -94,6 +94,28 @@ test('facetContextText carries the binding that does change', () => { assert.equal(facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode: 'off' }), '') }) +test('a session with no approval channel says so instead of promising a question', () => { + // `approval: never` refuses every ask before an answerer is consulted, so the + // mode stops mattering: `ask` and `deny` both end in a refusal, and + // `facet action=add` cannot be approved either. `canAsk: false` is the one + // input that collapses them, and it has to name the mechanism that still + // works — `extraRoots` is configuration, not a per-conversation grant. + for (const mode of ['ask', 'deny']) { + const text = facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode, canAsk: false }) + assert.match(text, /outside the facet: refused — approval prompts are off in this session/) + assert.match(text, /`facet action=add` cannot be approved either/) + assert.match(text, /`extraRoots`/) + assert.doesNotMatch(text, /put to the user as a question/) + } + + // The default stays a promise: a caller that knows nothing about approval + // channels must not silently change what the binding says. + assert.match( + facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode: 'ask' }), + /put to the user as a question/, + ) +}) + test('displayPath prefers the shortest unambiguous form', () => { assert.equal(displayPath('/w', '/w/src/a.ts'), 'src/a.ts') assert.equal(displayPath('/w', '/w'), '.')