fix: say so when a session has no approval channel to ask through
Release / release (push) Successful in 6s
Release / release (push) Successful in 6s
`ask` mode promises 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`, and the approval service's `decide()` then returns "rejected" before consulting any answerer. The call is refused, `facet action=add` is refused with it — widening is itself a question — and the scope can never be widened. The model is told none of that. `serviceAsk` replaces a rejected ask with `the user rejected tool \"…\"`, discarding the reason this guard wrote, so the binding is the only channel left that can state what is true. It now reads the session's effective approval policy and, when there is no channel, says the call was refused rather than asked and names the mechanism that still works: `extraRoots` is configuration applied by the profile, not a grant asked for per conversation. A missing approval service counts as no channel (the tool pipeline denies an ask it cannot route); an unreadable policy counts as a channel, so a transient failure never tells a user to go and edit their configuration.
This commit is contained in:
1 parent
6e696f10a2
commit
a1c8ecb1e8
4 files changed
+120
-6
No files matched your search
+36
-1
@@ -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),
|
||||
})
|
||||
},
|
||||
})
|
||||
|
||||
|
||||
+17
-3
@@ -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'
|
||||
|
||||
|
||||
+45
-2
@@ -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() })
|
||||
|
||||
@@ -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'), '.')
|
||||
|
||||
Reference in new issue
Block a user