diff --git a/README.md b/README.md index 4fe3852..b3f7a73 100644 --- a/README.md +++ b/README.md @@ -237,6 +237,27 @@ replaces the whole plugin list rather than merging into it, so the file is a frozen copy of `standard` that will not follow the harness when `standard` changes. +### Prompt variables, and why the preset's persona has no suffix + +Both preset files used to copy the deployment's persona line, *Your working +directory is {{cwd}}*. The harness registers prompt variables per section and an +unregistered reference **throws during prompt assembly**, which fails the entire +turn — not a warning, not a literal left in the text. That is not a hypothetical: +a harness release stopped registering `cwd` (only `provider` and `model` remain), +and every session on this bundle's preset failed with +`unknown prompt variable "{{cwd}}" in section "deployment:persona-suffix"`. + +So neither preset carries that suffix now, and the folder is stated where it can +follow the session instead: `facet-guard`'s runtime binding names the facet, and +names the session folder too when the two differ — which is exactly the case +where relative tool paths need saying out loud. `test/presets.test.js` scans both +files for `{{…}}` and fails on any name outside `provider, model`, and the same +test holds this bundle's own rule and binding text to no references at all. + +If a *deployment* still fails with that message on a non-facet preset, the +reference is in the deployment's own `system-prompt` config (`personaSuffix`) or +in a copy of the shipped `standard` patch, not in this bundle. + ## What it does not cover Honest boundaries, because each one is a decision rather than an oversight: @@ -264,7 +285,7 @@ Honest boundaries, because each one is a decision rather than an oversight: Verified: -- 93 tests (`npm test`) over the scope algebra, the policy state, the guard, and +- 95 tests (`npm test`) over the scope algebra, the policy state, the guard, and the browser half — 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 (`/a/room` does not contain `/a/roommate`), that a malformed tool @@ -275,8 +296,10 @@ Verified: is refused when the directory it defaults to is outside the facet, that a delegated session inherits its parent's root at any depth and is not a way back out of a self-narrowing, that both listings normalise into one picker level and - that a file listing the Host confines still leaves a browsable one, and that - every `facet` result satisfies the tool's own declared output schema. + that a file listing the Host confines still leaves a browsable one, that no + preset or contributed prompt text references a prompt variable the harness + does not register, and that every `facet` result satisfies the tool's own + declared output schema. - The bundle's patch layers parse, merge and land correctly against the real profile: `dsh --profile web --patch ./cordis.patch.yml --patch ./presets/facet.patch.yml --dump-config` shows `facet-policy`, `preset-facet`, @@ -354,7 +377,7 @@ lib/scope.js the pure half — argument extraction, text, reason lib/guard.js preset row: rule, binding, tool, pre-execute guard presets/facet.patch.yml the `facet` agent preset presets/standard-compaction.patch.yml optional: the same budget for `standard` -test/ 93 tests +test/ 95 tests ``` `client/client.js` is a browser half rather than a module the host imports, and diff --git a/lib/scope.js b/lib/scope.js index db22043..b5da628 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -233,6 +233,18 @@ export function facetContextText(input) { // 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. const lines = [`facet: ${root}`] + // The session folder is named only when it is not the facet — the case wherever + // the user narrowed a conversation that started higher up — because that is when + // the two differ and relative paths in tool calls need saying out loud. + // + // It used to be the deployment persona's suffix that stated this, through the + // `cwd` prompt variable. The harness stopped registering that variable, and an + // unknown one now throws during prompt assembly instead of rendering as text, so + // a static template cannot carry a per-session fact any more. This can: it is + // rebuilt for each request from the same header the tool calls resolve against. + if (typeof cwd === 'string' && cwd !== '' && cwd !== root) { + lines.push(`session folder: ${cwd} — relative paths in tool calls resolve here`) + } if (extras.length > 0) lines.push(`granted outside it: ${extras.map((path) => displayPath(cwd, path)).join(', ')}`) lines.push(`outside the facet: ${reach}`) return lines.join('\n') diff --git a/presets/facet.patch.yml b/presets/facet.patch.yml index 7fe1193..ddadfd2 100644 --- a/presets/facet.patch.yml +++ b/presets/facet.patch.yml @@ -1,21 +1,27 @@ # Agent preset facet: one `@deepseek-ai/dsh-agent-preset` declaration inserted # after the web patch. # -# The composition is `standard` with two changes, and this file is the only place -# either of them is decided: +# The composition is `standard` with three changes, and this file is the only +# place any of them is decided: # # * compaction is configured (see the `compaction` row below) so the pressure # trigger is a budget rather than a formality. The shipped ratio is 0.80 of a # 1,000,000-token window, i.e. 800k, and on this deployment nothing has ever # come within 1.5% of it — compaction had never once run; # -# * `facet-guard` is mounted, which confines the session to its own folder. +# * `facet-guard` is mounted, which confines the session to its own folder; +# +# * the persona row carries no suffix. The shipped one states the working +# directory through a prompt variable the harness no longer registers, and +# an unregistered variable fails the whole turn — see the note on that row. # # Everything else is copied verbatim from the shipped `standard` patch, and that # is deliberate: a preset declaration replaces the whole plugin list rather than # merging into it, so a clone that drifts from the original silently loses # whatever it drifted past. When the harness next changes `standard`, re-copy -# this file and re-apply the two changes — do not hand-patch around it. +# this file and re-apply the three changes — do not hand-patch around it. The +# persona row must be re-checked by hand: re-copying it from `standard` restores +# the suffix, and the harness's variable set is what decides whether it composes. - insert: - id: preset-facet name: '@deepseek-ai/dsh-agent-preset' @@ -25,10 +31,16 @@ description: One module of the system, not all of it — the session's own folder, plus the files you grant. order: 5 plugins: + # No suffix. It used to state the working directory through the `cwd` + # prompt variable, which the harness does not register — and an unknown + # variable is not rendered as text: assembly throws and the whole turn + # fails with it. Only `provider` and `model` are registered now, so the + # prefix keeps its model reference and the folder is stated per session + # by `facet-guard`'s runtime binding instead, which follows the user + # when they narrow it. - id: persona name: '@deepseek-ai/dsh-persona' config: - suffix: Your working directory is {{cwd}}. prefix: You are a coding agent powered by the {{model}} model. - id: agent-instructions name: '@deepseek-ai/dsh-agent-instructions' diff --git a/presets/standard-compaction.patch.yml b/presets/standard-compaction.patch.yml index f524c41..b862199 100644 --- a/presets/standard-compaction.patch.yml +++ b/presets/standard-compaction.patch.yml @@ -11,13 +11,17 @@ # It is deliberately not applied for you, and the reason is a real hazard rather # than caution: a preset declaration replaces the whole plugin list, never merges # into it, so this file is a frozen copy of the shipped `standard` composition -# plus one change. When the harness next updates `standard` — a new tool, a +# plus two changes. When the harness next updates `standard` — a new tool, a # renamed row — this copy will not follow it, and the sessions on this preset # will silently lose whatever was added. Use it if you are prepared to re-copy # and re-apply when that happens; otherwise take the compaction settings into # whichever preset you actually run. # -# The only change from the shipped file is the `compaction-basic` row's config. +# The two changes from the shipped file are the `compaction-basic` row's config, +# and the persona row's missing suffix: the shipped one names the working +# directory with a prompt variable the harness no longer registers, and an +# unregistered variable fails the whole turn instead of rendering. Re-copying +# this file from a newer `standard` reintroduces that suffix — re-apply both. - insert: - id: preset-standard name: '@deepseek-ai/dsh-agent-preset' @@ -25,10 +29,15 @@ id: standard order: 1 plugins: + # No suffix, for the reason in `facet.patch.yml`: the harness registers + # only `provider` and `model` as prompt variables, and an unregistered + # one now throws instead of rendering. Nothing in this preset states the + # session's working directory any more — a static prompt template cannot + # name a folder that changes per session, so a deployment that needs it + # belongs in the runtime context rather than here. - id: persona name: '@deepseek-ai/dsh-persona' config: - suffix: Your working directory is {{cwd}}. prefix: You are a coding agent powered by the {{model}} model. - id: agent-instructions name: '@deepseek-ai/dsh-agent-instructions' diff --git a/test/presets.test.js b/test/presets.test.js new file mode 100644 index 0000000..2f6c586 --- /dev/null +++ b/test/presets.test.js @@ -0,0 +1,61 @@ +import assert from 'node:assert/strict' +import { readFileSync, readdirSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' + +import { facetContextText, facetRuleText } from '../lib/scope.js' + +const PRESETS = join(dirname(fileURLToPath(import.meta.url)), '..', 'presets') + +/** + * The prompt variables the harness registers for a section. + * + * `dsh-agent-loop` puts each of these into the prompt registry, and an assembly + * substitutes only what is registered. An unknown reference is not rendered as + * text — interpolation throws, and the throw takes the entire turn with it, so + * the user sees "This turn failed" and nothing else. A harness release that + * stopped registering `cwd` is exactly how this bundle's preset stopped + * composing sessions: the persona suffix it copied from the deployment said + * "Your working directory is …" with that variable in it. + */ +const REGISTERED = ['provider', 'model'] + +/** Every `{{name}}` reference in one text, as the name alone. */ +function referencesIn(text) { + return [...text.matchAll(/\{\{([^{}]*)\}\}/g)].map((match) => match[1]) +} + +test('no preset references a prompt variable the harness does not register', () => { + const files = readdirSync(PRESETS).filter((name) => name.endsWith('.yml')) + assert.ok(files.length > 0, 'the bundle ships at least one preset') + + const found = [] + for (const file of files) { + for (const name of referencesIn(readFileSync(join(PRESETS, file), 'utf8'))) found.push([file, name]) + } + + // The model reference is the one that has to keep working, so the guard cannot + // be satisfied by a preset that references nothing at all. + assert.ok( + found.some(([, name]) => name === 'model'), + 'a preset still names the model', + ) + for (const [file, name] of found) { + assert.ok( + REGISTERED.includes(name), + `${file} references {{${name}}}; the harness registers only ${REGISTERED.join(', ')}, and an unregistered reference fails the whole turn`, + ) + } +}) + +test('the text this bundle contributes to the prompt holds no variable references', () => { + // The rule and the binding are rebuilt from real values on every request, so a + // placeholder in either would be a reference the harness never resolves — the + // same failure, this time from the bundle's own hand. + const rule = facetRuleText() + const binding = facetContextText({ cwd: '/w', root: '/w/src', extras: ['/w/shared'], mode: 'ask' }) + assert.ok(rule.length > 0 && binding.length > 0, 'both halves contribute text') + assert.deepEqual(referencesIn(rule), []) + assert.deepEqual(referencesIn(binding), []) +}) diff --git a/test/scope.test.js b/test/scope.test.js index fd82b41..f5bab34 100644 --- a/test/scope.test.js +++ b/test/scope.test.js @@ -74,6 +74,7 @@ test('facetContextText carries the binding that does change', () => { assert.equal( ask, 'facet: /w/src/room\n' + + 'session folder: /w — relative paths in tool calls resolve here\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', ) @@ -81,6 +82,7 @@ test('facetContextText carries the binding that does change', () => { assert.equal( deny, 'facet: /w/src/room\n' + + 'session folder: /w — relative paths in tool calls resolve here\n' + 'granted outside it: src/shared\n' + 'outside the facet: refused — ask the user to widen the facet from its context panel, or with `facet action=add` where that can be granted', )