From 4bc4296494bc66dfd2e5bd7c0ea96f7f60673234 Mon Sep 17 00:00:00 2001 From: NotExpectedYet Date: Mon, 19 Oct 2026 09:56:26 +0100 Subject: [PATCH] fix: stop the facet preset naming a prompt variable the harness dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preset's persona row copied the deployment's suffix, "Your working directory is {{cwd}}". The harness substitutes only registered prompt variables and throws on an unregistered one during prompt assembly — so when a harness release stopped registering `cwd` (only `provider` and `model` remain), every session on this preset failed its first turn with unknown prompt variable "{{cwd}}" in section "deployment:persona-suffix" and chat was dead rather than degraded. The row now carries the prefix only. The folder is stated where it can follow the session instead: the facet binding already names the facet, and now names the session folder as well when the two differ — which is every conversation that started higher up and was narrowed, and exactly the case where relative tool paths need saying out loud. `test/presets.test.js` makes this class of failure impossible to reintroduce silently: it scans every shipped preset for a variable reference and requires the name to be one the harness registers, and holds this bundle's own rule and binding text to no references at all. Restoring the old suffix fails it. --- README.md | 31 ++++++++++++-- lib/scope.js | 12 ++++++ presets/facet.patch.yml | 22 +++++++--- presets/standard-compaction.patch.yml | 15 +++++-- test/presets.test.js | 61 +++++++++++++++++++++++++++ test/scope.test.js | 2 + 6 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 test/presets.test.js 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', )