fix: show the settings card, and make an edit actually take effect
`mode` and `extraRoots` carried no `volatile()` marker, so the framework's
form projection dropped the whole entry from `settings.describe` and the plugin
had no card under Plugins at all. The marker is what creates the card.
It is also what turns each field into a reference rather than a value: the
resolved config hands `apply` a `{get(), [write]}` per volatile field, which is
how a settings write lands in a running plugin without re-applying it. So the
policy and the guard now read both fields at the point of use instead of
snapshotting them at mount, and `sectionOrder`/`contextOrder` stay unmarked
because a prompt section cannot be re-ordered without a remount.
A section is also registered once as a plain string, so the rule could not
follow a live `mode` without freezing at load. The mode-specific sentence moved
into the binding, which is a callback.
This commit is contained in:
1 parent
c41c555400
commit
28f20a8aac
8 files changed
+283
-59
No files matched your search
@@ -115,6 +115,23 @@ The host row's config is where this deployment's facts belong — the profile's
|
|||||||
- ../../shared # relative to each session's own directory
|
- ../../shared # relative to each session's own directory
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Both fields are also on the plugin's card under **Plugins** in the Web UI, which
|
||||||
|
is the same layer reached a different way: a settings write rewrites this entry in
|
||||||
|
the profile patch rather than storing anything new. An edit there is live — the
|
||||||
|
guard reads `mode` and `extraRoots` on every call, so a change decides the next
|
||||||
|
file call without a restart.
|
||||||
|
|
||||||
|
The one exception is `off`. Choosing it while a session is running stops the guard
|
||||||
|
immediately and empties the binding on the next request, because the binding is a
|
||||||
|
callback. The rule is not: it was registered at load as a plain string, and
|
||||||
|
nothing unregisters it, so the prompt would go on stating a rule that nothing
|
||||||
|
enforces. An `off` *deployment* is therefore settled at load — set it in the
|
||||||
|
patch. `ask` and `deny` swap freely in both directions.
|
||||||
|
|
||||||
|
`sectionOrder` and `contextOrder` are deliberately absent from the card. They
|
||||||
|
place the rule and the binding among the other prompt sections, which is decided
|
||||||
|
once at load, and no section is re-ordered without a remount.
|
||||||
|
|
||||||
`extraRoots` is for code a module legitimately always depends on. Anything you
|
`extraRoots` is for code a module legitimately always depends on. Anything you
|
||||||
grant by hand is per-conversation and is never persisted — "this session may
|
grant by hand is per-conversation and is never persisted — "this session may
|
||||||
also see `src/shared`" is a statement about one conversation, and writing it down
|
also see `src/shared`" is a statement about one conversation, and writing it down
|
||||||
@@ -171,7 +188,7 @@ Honest boundaries, because each one is a decision rather than an oversight:
|
|||||||
|
|
||||||
Verified:
|
Verified:
|
||||||
|
|
||||||
- 42 tests (`npm test`) over the scope algebra, the policy state, and the guard —
|
- 45 tests (`npm test`) over the scope algebra, the policy state, and the guard —
|
||||||
including that containment is decided on canonical targets (a symlink inside
|
including that containment is decided on canonical targets (a symlink inside
|
||||||
the facet pointing outside it is refused), that a prefix is not a parent
|
the facet pointing outside it is refused), that a prefix is not a parent
|
||||||
(`/a/room` does not contain `/a/roommate`), that a malformed tool call never
|
(`/a/room` does not contain `/a/roommate`), that a malformed tool call never
|
||||||
@@ -251,7 +268,7 @@ lib/scope.js the pure half — argument extraction, text, reason
|
|||||||
lib/guard.js preset row: rule, binding, tool, pre-execute guard
|
lib/guard.js preset row: rule, binding, tool, pre-execute guard
|
||||||
presets/facet.patch.yml the `facet` agent preset
|
presets/facet.patch.yml the `facet` agent preset
|
||||||
presets/standard-compaction.patch.yml optional: the same budget for `standard`
|
presets/standard-compaction.patch.yml optional: the same budget for `standard`
|
||||||
test/ 42 tests
|
test/ 45 tests
|
||||||
```
|
```
|
||||||
|
|
||||||
## Also in this repository
|
## Also in this repository
|
||||||
|
|||||||
@@ -16,10 +16,12 @@
|
|||||||
* published once here and consumed there, which is also why the preset can be
|
* published once here and consumed there, which is also why the preset can be
|
||||||
* swapped without touching any of this.
|
* swapped without touching any of this.
|
||||||
*
|
*
|
||||||
* There is deliberately no `settings` namespace. A facet grant is a statement
|
* There is deliberately no `settings` *namespace* of our own: a facet grant is a
|
||||||
* about one conversation, so the only thing worth persisting is the deployment's
|
* statement about one conversation, so the only thing worth persisting is the
|
||||||
* standing grants, and those belong in the profile patch — the layer where this
|
* deployment's standing grants, and those belong in the profile patch — the
|
||||||
* machine's facts already live.
|
* layer where this machine's facts already live. What the framework projects
|
||||||
|
* onto this row is the {@link Config} schema below, which is the same patch
|
||||||
|
* layer reached through the Web UI rather than a second place to store things.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import Schema from '@deepseek-ai/schemastery'
|
import Schema from '@deepseek-ai/schemastery'
|
||||||
@@ -28,21 +30,42 @@ import { createFacetPolicy } from './lib/policy.js'
|
|||||||
|
|
||||||
export const name = 'dsh-facet'
|
export const name = 'dsh-facet'
|
||||||
|
|
||||||
/** Every configuration field this plugin exposes. */
|
/**
|
||||||
|
* Every configuration field this plugin exposes.
|
||||||
|
*
|
||||||
|
* `mode` and `extraRoots` are marked `volatile()`, and that marker is doing two
|
||||||
|
* jobs rather than one. It is what puts them on the settings page at all — the
|
||||||
|
* framework's form projection keeps only fields under a volatile ancestor, and
|
||||||
|
* drops an entry with no such field from `settings.describe` entirely, so
|
||||||
|
* without it this row has no card. It is also what makes an edit *live*: a
|
||||||
|
* volatile field is handed to {@link apply} as a reference rather than a value,
|
||||||
|
* and the framework commits an edit into that reference without re-applying the
|
||||||
|
* plugin.
|
||||||
|
*
|
||||||
|
* The second job is why every reader of these two fields goes through
|
||||||
|
* `configValue()` in `./lib/policy.js` and reads at the point of use. A plugin
|
||||||
|
* that takes the reference for a string gets a settings card that appears to
|
||||||
|
* work and changes nothing.
|
||||||
|
*
|
||||||
|
* `sectionOrder` and `contextOrder` are deliberately NOT volatile. They place
|
||||||
|
* the rule and the binding among the other prompt sections, which is settled
|
||||||
|
* once at load; making them volatile would put two knobs nobody should touch on
|
||||||
|
* the card, and a section cannot be re-ordered without a remount anyway.
|
||||||
|
*/
|
||||||
export const Config = Schema.object({
|
export const Config = Schema.object({
|
||||||
/**
|
/**
|
||||||
* How a call outside the facet is handled. `ask` questions the user, `deny`
|
* How a call outside the facet is handled. `ask` questions the user, `deny`
|
||||||
* refuses outright, `off` disables enforcement while leaving the preset in
|
* refuses outright, `off` disables enforcement while leaving the preset in
|
||||||
* place — useful for telling a scoping problem from an unrelated one.
|
* place — useful for telling a scoping problem from an unrelated one.
|
||||||
*/
|
*/
|
||||||
mode: Schema.union(['ask', 'deny', 'off']).default('ask'),
|
mode: Schema.union(['ask', 'deny', 'off']).default('ask').volatile(),
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Paths always in scope, for code a module legitimately depends on — shared
|
* Paths always in scope, for code a module legitimately depends on — shared
|
||||||
* types, a common test harness. Relative paths resolve against each session's
|
* types, a common test harness. Relative paths resolve against each session's
|
||||||
* own directory, so the same list is meaningful in every workspace.
|
* own directory, so the same list is meaningful in every workspace.
|
||||||
*/
|
*/
|
||||||
extraRoots: Schema.array(String).default([]),
|
extraRoots: Schema.array(String).default([]).volatile(),
|
||||||
|
|
||||||
/** Placement of the invariant rule in the system prompt. */
|
/** Placement of the invariant rule in the system prompt. */
|
||||||
sectionOrder: Schema.number().default(100),
|
sectionOrder: Schema.number().default(100),
|
||||||
|
|||||||
+25
-10
@@ -64,10 +64,19 @@ export const Config = Schema.object({
|
|||||||
*/
|
*/
|
||||||
export function apply(ctx, config) {
|
export function apply(ctx, config) {
|
||||||
const policy = ctx.facetPolicy
|
const policy = ctx.facetPolicy
|
||||||
/** @type {'ask'|'deny'|'off'} */
|
|
||||||
const mode = policy.mode
|
|
||||||
|
|
||||||
if (mode === 'off') return
|
// Read at the point of use, never captured. `mode` is a volatile field, so a
|
||||||
|
// settings edit updates the policy's reference in place and deliberately does
|
||||||
|
// not re-apply this plugin; a `const mode` here would answer with the value
|
||||||
|
// the session was loaded with, for the life of the session.
|
||||||
|
const modeNow = () => policy.mode
|
||||||
|
|
||||||
|
// `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
|
||||||
|
// a reload, because a volatile-only edit never re-runs this function. `ask`
|
||||||
|
// and `deny` swap freely in both directions.
|
||||||
|
if (modeNow() === 'off') return
|
||||||
|
|
||||||
const sectionOrder = policy.sectionOrder ?? 100
|
const sectionOrder = policy.sectionOrder ?? 100
|
||||||
const contextOrder = policy.contextOrder ?? 105
|
const contextOrder = policy.contextOrder ?? 105
|
||||||
@@ -129,6 +138,9 @@ export function apply(ctx, config) {
|
|||||||
const classify = async (exec) => {
|
const classify = async (exec) => {
|
||||||
if (exec.signal?.aborted) return undefined
|
if (exec.signal?.aborted) return undefined
|
||||||
|
|
||||||
|
const mode = modeNow()
|
||||||
|
if (mode === 'off') return undefined
|
||||||
|
|
||||||
// Widening is the one act that always needs the user, whatever the mode:
|
// Widening is the one act that always needs the user, whatever the mode:
|
||||||
// it is explicit, deliberate, and the only thing standing between a scoped
|
// it is explicit, deliberate, and the only thing standing between a scoped
|
||||||
// agent and an unscoped one.
|
// agent and an unscoped one.
|
||||||
@@ -155,11 +167,13 @@ export function apply(ctx, config) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// The rule. `interpolate: false` because every value in it is already
|
// The rule. `interpolate: false` because every value in it is already
|
||||||
// literal; there is nothing here for the harness to substitute.
|
// literal; there is nothing here for the harness to substitute. It carries no
|
||||||
|
// `mode`, because a section is registered once and cannot follow a live edit —
|
||||||
|
// see `facetRuleText`.
|
||||||
ctx.systemPrompt.section({
|
ctx.systemPrompt.section({
|
||||||
name: 'facet-scope',
|
name: 'facet-scope',
|
||||||
order: Number(sectionOrder),
|
order: Number(sectionOrder),
|
||||||
text: facetRuleText({ mode }),
|
text: facetRuleText(),
|
||||||
interpolate: false,
|
interpolate: false,
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -173,11 +187,11 @@ export function apply(ctx, config) {
|
|||||||
text: (context) => {
|
text: (context) => {
|
||||||
const scope = policy.scopeFor(context?.agent)
|
const scope = policy.scopeFor(context?.agent)
|
||||||
if (scope === undefined) return ''
|
if (scope === undefined) return ''
|
||||||
return facetContextText({ cwd: scope.cwd, root: scope.root, extras: scope.extras, mode })
|
return facetContextText({ cwd: scope.cwd, root: scope.root, extras: scope.extras, mode: modeNow() })
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|
||||||
if (config.tool) ctx.tools.register(facetTool(ctx, policy, mode))
|
if (config.tool) ctx.tools.register(facetTool(ctx, policy))
|
||||||
|
|
||||||
// `next()` first, so an earlier policy plugin keeps its say: this listener
|
// `next()` first, so an earlier policy plugin keeps its say: this listener
|
||||||
// only ever adds a question or a refusal to a call that was otherwise allowed.
|
// only ever adds a question or a refusal to a call that was otherwise allowed.
|
||||||
@@ -206,9 +220,8 @@ function requestedPaths(args) {
|
|||||||
*
|
*
|
||||||
* @param {import('@deepseek-ai/cordis').Context} ctx - the guard's context.
|
* @param {import('@deepseek-ai/cordis').Context} ctx - the guard's context.
|
||||||
* @param {object} policy - the `facetPolicy` service.
|
* @param {object} policy - the `facetPolicy` service.
|
||||||
* @param {'ask'|'deny'|'off'} mode - the deployment's enforcement mode.
|
|
||||||
*/
|
*/
|
||||||
function facetTool(ctx, policy, mode) {
|
function facetTool(ctx, policy) {
|
||||||
return defineTool({
|
return defineTool({
|
||||||
name: FACET_TOOL,
|
name: FACET_TOOL,
|
||||||
description:
|
description:
|
||||||
@@ -276,7 +289,9 @@ function facetTool(ctx, policy, mode) {
|
|||||||
ok: true,
|
ok: true,
|
||||||
root: scope.root,
|
root: scope.root,
|
||||||
facet: scope.root,
|
facet: scope.root,
|
||||||
mode,
|
// Read here rather than captured at mount, so `status` reports the mode
|
||||||
|
// in force now instead of the one this session started under.
|
||||||
|
mode: String(policy.mode),
|
||||||
standing: policy.standing,
|
standing: policy.standing,
|
||||||
granted,
|
granted,
|
||||||
scope: scope.extras,
|
scope: scope.extras,
|
||||||
|
|||||||
+64
-6
@@ -18,6 +18,38 @@
|
|||||||
|
|
||||||
import { isAbsolute, resolve } from 'node:path'
|
import { isAbsolute, resolve } from 'node:path'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The symbol cosmokit uses to mark a volatile config reference.
|
||||||
|
*
|
||||||
|
* Looked up by symbol rather than by importing cosmokit, because the ESM and CJS
|
||||||
|
* copies of that library are distinct module instances and `Symbol.for` is the
|
||||||
|
* only identity they share.
|
||||||
|
*/
|
||||||
|
const VOLATILE_WRITE = Symbol.for('cosmokit.volatile.write')
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read one configuration field.
|
||||||
|
*
|
||||||
|
* A `volatile()` field is a *reference* in the running config, not a plain
|
||||||
|
* value: the framework resolves the schema through `Schema.resolve`, which wraps
|
||||||
|
* each volatile field in `createVolatile(...)` — `{get(), [write]}`. A
|
||||||
|
* non-volatile field, or a config object a test handed straight to
|
||||||
|
* {@link createFacetPolicy}, is the value itself. Both are accepted, so one code
|
||||||
|
* path serves a real profile and a direct call.
|
||||||
|
*
|
||||||
|
* Every read has to happen at the point of use rather than once at construction.
|
||||||
|
* A settings write commits into these references in place and deliberately does
|
||||||
|
* *not* re-apply the plugin, so anything captured at construction would never
|
||||||
|
* see the change — which is how a plugin ends up with a settings card that does
|
||||||
|
* nothing.
|
||||||
|
*
|
||||||
|
* @param {unknown} value - a field of the resolved config.
|
||||||
|
* @returns {unknown} the current value.
|
||||||
|
*/
|
||||||
|
export function configValue(value) {
|
||||||
|
return typeof value === 'object' && value !== null && VOLATILE_WRITE in value ? value.get() : value
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @typedef {object} FacetScope
|
* @typedef {object} FacetScope
|
||||||
* @property {string} cwd - the session working directory, absolute.
|
* @property {string} cwd - the session working directory, absolute.
|
||||||
@@ -28,6 +60,13 @@ import { isAbsolute, resolve } from 'node:path'
|
|||||||
/**
|
/**
|
||||||
* Build the policy for one deployment.
|
* Build the policy for one deployment.
|
||||||
*
|
*
|
||||||
|
* `config` is the *resolved* composition config, so its volatile fields are
|
||||||
|
* references rather than values; every read below goes through
|
||||||
|
* {@link configValue} and happens per use. Holding the config and reading it lazily
|
||||||
|
* is what makes the settings card honest: `mode` and `extraRoots` are edited live
|
||||||
|
* in the Web UI, and a policy that snapshotted them here would keep answering
|
||||||
|
* with the values it was loaded with.
|
||||||
|
*
|
||||||
* @param {object} config
|
* @param {object} config
|
||||||
* @param {'ask'|'deny'|'off'} config.mode - how an out-of-scope call is handled.
|
* @param {'ask'|'deny'|'off'} config.mode - how an out-of-scope call is handled.
|
||||||
* @param {readonly string[]} [config.extraRoots] - standing grants, resolved against each session's cwd.
|
* @param {readonly string[]} [config.extraRoots] - standing grants, resolved against each session's cwd.
|
||||||
@@ -36,7 +75,20 @@ import { isAbsolute, resolve } from 'node:path'
|
|||||||
export function createFacetPolicy(config) {
|
export function createFacetPolicy(config) {
|
||||||
/** @type {Map<string, string[]>} session id → granted paths, in the order granted. */
|
/** @type {Map<string, string[]>} session id → granted paths, in the order granted. */
|
||||||
const grants = new Map()
|
const grants = new Map()
|
||||||
const standing = Array.isArray(config.extraRoots) ? config.extraRoots.filter(isPath) : []
|
|
||||||
|
/**
|
||||||
|
* The deployment's standing grants, read fresh on every call.
|
||||||
|
*
|
||||||
|
* Not filtered once at construction: `extraRoots` is volatile, so an edit in
|
||||||
|
* the settings UI replaces the reference's contents without remounting this
|
||||||
|
* plugin, and the next scope resolution is expected to see it.
|
||||||
|
*
|
||||||
|
* @returns {string[]} configured paths, blanks dropped.
|
||||||
|
*/
|
||||||
|
function standingRoots() {
|
||||||
|
const configured = configValue(config.extraRoots)
|
||||||
|
return Array.isArray(configured) ? configured.filter(isPath) : []
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The scope of one agent, or `undefined` when it has none.
|
* The scope of one agent, or `undefined` when it has none.
|
||||||
@@ -66,7 +118,7 @@ export function createFacetPolicy(config) {
|
|||||||
function extrasFor(agent, cwd) {
|
function extrasFor(agent, cwd) {
|
||||||
const sessionId = typeof agent?.session?.id === 'string' ? agent.session.id : undefined
|
const sessionId = typeof agent?.session?.id === 'string' ? agent.session.id : undefined
|
||||||
const own = sessionId === undefined ? [] : grants.get(sessionId) ?? []
|
const own = sessionId === undefined ? [] : grants.get(sessionId) ?? []
|
||||||
const all = [...standing, ...own].map((path) => (isAbsolute(path) ? resolve(path) : resolve(cwd, path)))
|
const all = [...standingRoots(), ...own].map((path) => (isAbsolute(path) ? resolve(path) : resolve(cwd, path)))
|
||||||
return [...new Set(all)]
|
return [...new Set(all)]
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -124,11 +176,17 @@ export function createFacetPolicy(config) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
mode: config.mode,
|
/** The enforcement mode. A getter, so a settings edit is answered on the next call. */
|
||||||
|
get mode() {
|
||||||
|
return configValue(config.mode)
|
||||||
|
},
|
||||||
/** Prompt placements, carried here so the preset row needs no configuration of its own. */
|
/** Prompt placements, carried here so the preset row needs no configuration of its own. */
|
||||||
sectionOrder: config.sectionOrder ?? 100,
|
sectionOrder: configValue(config.sectionOrder) ?? 100,
|
||||||
contextOrder: config.contextOrder ?? 105,
|
contextOrder: configValue(config.contextOrder) ?? 105,
|
||||||
standing: [...standing],
|
/** The standing grants. A getter for the same reason as {@link mode}. */
|
||||||
|
get standing() {
|
||||||
|
return standingRoots()
|
||||||
|
},
|
||||||
scopeFor,
|
scopeFor,
|
||||||
grant,
|
grant,
|
||||||
revoke,
|
revoke,
|
||||||
|
|||||||
+28
-18
@@ -118,25 +118,24 @@ export function isWideningCall(toolName, args) {
|
|||||||
* treatment the sandbox policy and approval policy already get, for the same
|
* treatment the sandbox policy and approval policy already get, for the same
|
||||||
* reason.
|
* reason.
|
||||||
*
|
*
|
||||||
* @param {object} input
|
* It takes no configuration, and that is deliberate rather than an oversight.
|
||||||
* @param {'ask'|'deny'|'off'} input.mode - how an out-of-scope call is handled.
|
* A system-prompt section is a plain string registered once; there is no
|
||||||
* @returns {string} prompt section text, or `''` to contribute nothing.
|
* callback to re-render it, and a settings write to a `volatile()` field
|
||||||
|
* updates the config reference *without* re-applying the plugin. So anything
|
||||||
|
* this function interpolated would freeze at whatever the value was at load —
|
||||||
|
* the prompt would go on describing the old `mode` while the guard enforced the
|
||||||
|
* new one. Everything that depends on `mode` therefore lives in the binding,
|
||||||
|
* which is a callback and therefore live.
|
||||||
|
*
|
||||||
|
* @returns {string} prompt section text.
|
||||||
*/
|
*/
|
||||||
export function facetRuleText(input) {
|
export function facetRuleText() {
|
||||||
const { mode } = input
|
|
||||||
if (mode === 'off') return ''
|
|
||||||
|
|
||||||
const reach =
|
|
||||||
mode === 'deny'
|
|
||||||
? 'A call outside it is refused. `facet action=add` asks the user to grant a path, and is the only way to widen the scope.'
|
|
||||||
: 'A call outside it is not refused: it is put to the user as a question, so say in one line what you need and why. If they agree, record it with `facet action=add` so the rest of the session does not have to ask again.'
|
|
||||||
|
|
||||||
return [
|
return [
|
||||||
'# Facet scope',
|
'# Facet scope',
|
||||||
'',
|
'',
|
||||||
'This session is scoped to one facet of the project — one module rather than the whole system. Your runtime context states which.',
|
'This session is scoped to one facet of the project — one module rather than the whole system. Your runtime context states which, and says how a call outside it is treated.',
|
||||||
'',
|
'',
|
||||||
`Work inside the facet. ${reach}`,
|
'Work inside the facet.',
|
||||||
'',
|
'',
|
||||||
'Never widen the scope on your own initiative: a grant is the user\u2019s decision, and an unrequested one is a bug, not initiative. Do not add paths you merely expect to need — ask when you actually need them.',
|
'Never widen the scope on your own initiative: a grant is the user\u2019s decision, and an unrequested one is a bug, not initiative. Do not add paths you merely expect to need — ask when you actually need them.',
|
||||||
'',
|
'',
|
||||||
@@ -164,9 +163,12 @@ export function displayPath(cwd, path) {
|
|||||||
/**
|
/**
|
||||||
* The mutable half of the facet, for the runtime-context snapshot.
|
* The mutable half of the facet, for the runtime-context snapshot.
|
||||||
*
|
*
|
||||||
* Kept to three short lines because it is re-sent as part of the conversation
|
* Kept short because it is re-sent as part of the conversation whenever it
|
||||||
* whenever it changes, and a scope statement that costs more than the scope is
|
* changes, and a scope statement that costs more than the scope is worth is a
|
||||||
* worth is a scope that does not pay for itself.
|
* scope that does not pay for itself. It carries one sentence more than the bare
|
||||||
|
* binding used to, because `mode` now belongs here: this is the only half that
|
||||||
|
* is re-rendered per request, so it is the only half that can follow a settings
|
||||||
|
* change. See {@link facetRuleText}.
|
||||||
*
|
*
|
||||||
* @param {object} input
|
* @param {object} input
|
||||||
* @param {string} input.cwd - the session working directory.
|
* @param {string} input.cwd - the session working directory.
|
||||||
@@ -179,13 +181,21 @@ export function facetContextText(input) {
|
|||||||
const { cwd, root, extras = [], mode } = input
|
const { cwd, root, extras = [], mode } = input
|
||||||
if (mode === 'off' || typeof root !== 'string' || root === '') return ''
|
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'
|
||||||
|
? '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'
|
||||||
|
|
||||||
// The root is stated absolutely even though it is the session's own directory
|
// The root is stated absolutely even though it is the session's own directory
|
||||||
// and a relative form would be shorter. `facet: .` is true and useless: the
|
// and a relative form would be shorter. `facet: .` is true and useless: the
|
||||||
// model needs a path it can reason about, and the user reading an approval
|
// model needs a path it can reason about, and the user reading an approval
|
||||||
// prompt needs to see which folder they are being asked about.
|
// prompt needs to see which folder they are being asked about.
|
||||||
const lines = [`facet: ${root}`]
|
const lines = [`facet: ${root}`]
|
||||||
if (extras.length > 0) lines.push(`granted outside it: ${extras.map((path) => displayPath(cwd, path)).join(', ')}`)
|
if (extras.length > 0) lines.push(`granted outside it: ${extras.map((path) => displayPath(cwd, path)).join(', ')}`)
|
||||||
lines.push(`outside the facet: ${mode === 'deny' ? 'refused' : 'ask the user'}`)
|
lines.push(`outside the facet: ${reach}`)
|
||||||
return lines.join('\n')
|
return lines.join('\n')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+30
-2
@@ -18,6 +18,15 @@ import { after, before, test } from 'node:test'
|
|||||||
|
|
||||||
import { apply as guard } from '../lib/guard.js'
|
import { apply as guard } from '../lib/guard.js'
|
||||||
import { createFacetPolicy } from '../lib/policy.js'
|
import { createFacetPolicy } from '../lib/policy.js'
|
||||||
|
import { Config } from '../index.js'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The symbol cosmokit marks a volatile config reference with. A settings write
|
||||||
|
* reaches a running plugin as `target[write](value)` — that is literally what
|
||||||
|
* `updateVolatile` does — so a test can commit an edit the way the framework
|
||||||
|
* does, without a settings service.
|
||||||
|
*/
|
||||||
|
const VOLATILE_WRITE = Symbol.for('cosmokit.volatile.write')
|
||||||
|
|
||||||
/** A filesystem provider with `dsh-fs-local`'s two relevant behaviours. */
|
/** A filesystem provider with `dsh-fs-local`'s two relevant behaviours. */
|
||||||
const fsLikeLocal = {
|
const fsLikeLocal = {
|
||||||
@@ -50,8 +59,8 @@ const fsLikeLocal = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Capture what `apply` registered, so the test can drive it. */
|
/** Capture what `apply` registered, so the test can drive it. */
|
||||||
function harness({ mode = 'ask', extraRoots = [], tool = true } = {}) {
|
function harness({ mode = 'ask', extraRoots = [], tool = true, config } = {}) {
|
||||||
const policy = createFacetPolicy({ mode, extraRoots })
|
const policy = createFacetPolicy(config ?? { mode, extraRoots })
|
||||||
const captured = { section: undefined, context: undefined, tool: undefined, preExecute: undefined }
|
const captured = { section: undefined, context: undefined, tool: undefined, preExecute: undefined }
|
||||||
const ctx = {
|
const ctx = {
|
||||||
facetPolicy: policy,
|
facetPolicy: policy,
|
||||||
@@ -217,6 +226,25 @@ test('off mode registers nothing', () => {
|
|||||||
assert.equal(captured.tool, undefined)
|
assert.equal(captured.tool, undefined)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('a live mode edit decides the next call, without the guard being remounted', async () => {
|
||||||
|
// The loader hands the guard the schema-resolved config, so `mode` arrives as
|
||||||
|
// a volatile reference rather than a string. This drives that exact shape.
|
||||||
|
const config = Config({ mode: 'ask' })
|
||||||
|
const { captured } = harness({ config })
|
||||||
|
const outside = { name: 'read', args: { file_path: '../../shared/types.ts' }, agent: agent() }
|
||||||
|
|
||||||
|
assert.equal((await dispatch(captured, outside)).kind, 'ask')
|
||||||
|
|
||||||
|
// What a settings write does: commit into the reference in place, and
|
||||||
|
// deliberately do not re-apply the plugin. One mount, three answers.
|
||||||
|
config.mode[VOLATILE_WRITE]('deny')
|
||||||
|
assert.equal((await dispatch(captured, outside)).kind, 'deny')
|
||||||
|
|
||||||
|
// `off` silences the guard entirely, which is the whole meaning of the mode.
|
||||||
|
config.mode[VOLATILE_WRITE]('off')
|
||||||
|
assert.equal((await dispatch(captured, outside)).kind, 'allow')
|
||||||
|
})
|
||||||
|
|
||||||
test('the guard adds to a decision, never overrides one', async () => {
|
test('the guard adds to a decision, never overrides one', async () => {
|
||||||
const { captured } = harness()
|
const { captured } = harness()
|
||||||
const denied = await dispatch(captured, {
|
const denied = await dispatch(captured, {
|
||||||
|
|||||||
+61
-2
@@ -10,6 +10,18 @@ import assert from 'node:assert/strict'
|
|||||||
import { test } from 'node:test'
|
import { test } from 'node:test'
|
||||||
|
|
||||||
import { apply, Config } from '../index.js'
|
import { apply, Config } from '../index.js'
|
||||||
|
import { configValue } from '../lib/policy.js'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The symbol cosmokit marks a volatile config reference with, and the writer it
|
||||||
|
* carries. A settings write reaches a running plugin through exactly this
|
||||||
|
* function — `updateVolatile(target, source)` is `target[write](source.get())` —
|
||||||
|
* so a test can commit an edit the same way the framework does.
|
||||||
|
*/
|
||||||
|
const VOLATILE_WRITE = Symbol.for('cosmokit.volatile.write')
|
||||||
|
|
||||||
|
/** Whether a field arrived as a live reference rather than a plain value. */
|
||||||
|
const isReference = (value) => typeof value === 'object' && value !== null && VOLATILE_WRITE in value
|
||||||
|
|
||||||
/** The smallest context this row touches. */
|
/** The smallest context this row touches. */
|
||||||
function harness() {
|
function harness() {
|
||||||
@@ -28,12 +40,59 @@ function harness() {
|
|||||||
|
|
||||||
test('the schema fills every default, so a bare row is a working row', () => {
|
test('the schema fills every default, so a bare row is a working row', () => {
|
||||||
const config = Config({})
|
const config = Config({})
|
||||||
assert.equal(config.mode, 'ask')
|
assert.equal(configValue(config.mode), 'ask')
|
||||||
assert.deepEqual(config.extraRoots, [])
|
assert.deepEqual(configValue(config.extraRoots), [])
|
||||||
assert.equal(config.sectionOrder, 100)
|
assert.equal(config.sectionOrder, 100)
|
||||||
assert.equal(config.contextOrder, 105)
|
assert.equal(config.contextOrder, 105)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('the editable fields are volatile, which is the only reason they have a settings card', () => {
|
||||||
|
const config = Config({})
|
||||||
|
// The framework's form projection keeps only fields under a volatile ancestor
|
||||||
|
// and drops an entry with no such field from `settings.describe` entirely, so
|
||||||
|
// dropping either of these markers silently removes the card from the UI.
|
||||||
|
assert.equal(isReference(config.mode), true)
|
||||||
|
assert.equal(isReference(config.extraRoots), true)
|
||||||
|
|
||||||
|
// Placement is settled once at load: not settings anyone edits live, and not
|
||||||
|
// on the card.
|
||||||
|
assert.equal(isReference(config.sectionOrder), false)
|
||||||
|
assert.equal(isReference(config.contextOrder), false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('a settings edit is answered on the next call, without the plugin being re-applied', () => {
|
||||||
|
const provided = []
|
||||||
|
let applies = 0
|
||||||
|
const config = Config({ mode: 'ask', extraRoots: ['/w/src/shared'] })
|
||||||
|
apply(
|
||||||
|
{
|
||||||
|
provide: (name, value) => {
|
||||||
|
applies += 1
|
||||||
|
provided.push({ name, value })
|
||||||
|
},
|
||||||
|
on: () => {},
|
||||||
|
},
|
||||||
|
config,
|
||||||
|
)
|
||||||
|
const policy = provided[0].value
|
||||||
|
assert.equal(policy.mode, 'ask')
|
||||||
|
assert.deepEqual(policy.standing, ['/w/src/shared'])
|
||||||
|
|
||||||
|
// What the framework does on a settings write: it commits into the running
|
||||||
|
// references and deliberately does NOT re-apply the plugin. Everything the
|
||||||
|
// policy reports has to follow from the reference alone.
|
||||||
|
config.mode[VOLATILE_WRITE]('deny')
|
||||||
|
config.extraRoots[VOLATILE_WRITE](['/w/src/shared', '/w/src/types'])
|
||||||
|
|
||||||
|
assert.equal(applies, 1)
|
||||||
|
assert.equal(policy.mode, 'deny')
|
||||||
|
assert.deepEqual(policy.standing, ['/w/src/shared', '/w/src/types'])
|
||||||
|
assert.deepEqual(
|
||||||
|
policy.scopeFor({ session: { id: 's1', header: { cwd: '/w/src/room' } } }).extras,
|
||||||
|
['/w/src/shared', '/w/src/types'],
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
test('a bad mode fails loudly at load rather than at the first file call', () => {
|
test('a bad mode fails loudly at load rather than at the first file call', () => {
|
||||||
assert.throws(() => Config({ mode: 'maybe' }))
|
assert.throws(() => Config({ mode: 'maybe' }))
|
||||||
})
|
})
|
||||||
|
|||||||
+26
-12
@@ -51,33 +51,47 @@ test('isWideningCall is true only for the one act that needs the user', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test('facetRuleText states the rule and never the binding', () => {
|
test('facetRuleText states the rule and never the binding', () => {
|
||||||
const ask = facetRuleText({ mode: 'ask' })
|
const rule = facetRuleText()
|
||||||
assert.match(ask, /put to the user as a question/)
|
|
||||||
// The two modes must not read alike: ask must not claim the call is refused.
|
|
||||||
assert.doesNotMatch(ask, /A call outside it is refused/)
|
|
||||||
|
|
||||||
const deny = facetRuleText({ mode: 'deny' })
|
// Mode-neutral by construction. A prompt section is registered once as a plain
|
||||||
assert.match(deny, /A call outside it is refused/)
|
// string, and a `volatile()` edit updates the config reference without
|
||||||
|
// re-applying the plugin — so a `mode` interpolated here would freeze at load
|
||||||
|
// while the guard went on enforcing the new one. The mode-specific sentence
|
||||||
|
// lives in the binding, which is a callback.
|
||||||
|
assert.match(rule, /Work inside the facet\./)
|
||||||
|
assert.doesNotMatch(rule, /refused|put to the user as a question/)
|
||||||
|
assert.match(rule, /Never widen the scope on your own initiative/)
|
||||||
|
|
||||||
// The rule must not name a path: a changing system prompt invalidates the
|
// The rule must not name a path: a changing system prompt invalidates the
|
||||||
// prompt cache for the whole conversation.
|
// prompt cache for the whole conversation.
|
||||||
assert.doesNotMatch(ask, /\/|src\//)
|
assert.doesNotMatch(rule, /\/|src\//)
|
||||||
|
|
||||||
assert.equal(facetRuleText({ mode: 'off' }), '')
|
|
||||||
assert.equal(facetContextText({ cwd: '/a', root: '/a/b', extras: [], mode: 'off' }), '')
|
assert.equal(facetContextText({ cwd: '/a', root: '/a/b', extras: [], mode: 'off' }), '')
|
||||||
})
|
})
|
||||||
|
|
||||||
test('facetContextText carries the binding that does change', () => {
|
test('facetContextText carries the binding that does change', () => {
|
||||||
|
const ask = facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode: 'ask' })
|
||||||
assert.equal(
|
assert.equal(
|
||||||
facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode: 'ask' }),
|
ask,
|
||||||
'facet: /w/src/room\noutside the facet: ask the user',
|
'facet: /w/src/room\n' +
|
||||||
|
'outside the facet: 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',
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const deny = facetContextText({ cwd: '/w', root: '/w/src/room', extras: ['/w/src/shared'], mode: 'deny' })
|
||||||
assert.equal(
|
assert.equal(
|
||||||
facetContextText({ cwd: '/w', root: '/w/src/room', extras: ['/w/src/shared'], mode: 'deny' }),
|
deny,
|
||||||
'facet: /w/src/room\ngranted outside it: src/shared\noutside the facet: refused',
|
'facet: /w/src/room\n' +
|
||||||
|
'granted outside it: src/shared\n' +
|
||||||
|
'outside the facet: refused — `facet action=add` asks the user to grant a path, and is the only way to widen the scope',
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// The binding is the only half that can follow a live edit, so the two modes
|
||||||
|
// have to read differently here — they no longer differ in the rule.
|
||||||
|
assert.notEqual(ask, deny)
|
||||||
|
|
||||||
// Unscoped sessions contribute nothing rather than an empty heading.
|
// Unscoped sessions contribute nothing rather than an empty heading.
|
||||||
assert.equal(facetContextText({ cwd: '/w', root: undefined, extras: [], mode: 'ask' }), '')
|
assert.equal(facetContextText({ cwd: '/w', root: undefined, extras: [], mode: 'ask' }), '')
|
||||||
|
assert.equal(facetContextText({ cwd: '/w', root: '/w/src/room', extras: [], mode: 'off' }), '')
|
||||||
})
|
})
|
||||||
|
|
||||||
test('displayPath prefers the shortest unambiguous form', () => {
|
test('displayPath prefers the shortest unambiguous form', () => {
|
||||||
|
|||||||
Reference in new issue
Block a user