From 9e890dba67e9befcb66580c5161e69de7dd6de24 Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:23:56 +0200 Subject: [PATCH 1/6] feat(odd): make the feature document the verbatim spec subagents read by reference Handoffs paraphrased the user's request, the feature document summarized it, and verify never saw it (gentle-shell#1713). The document now has a fixed Specs/Tasks/Log order with verbatim quotes, handoffs reference it, verify returns a verdict per spec, and user-reported failures are reproduced first. The compact contract lives in the always-on ODD steps because the lazy assets were never read in the failing run. --- assets/agents/gentle-ai-explore.md | 2 +- assets/agents/gentle-ai-verify.md | 2 + assets/agents/gentle-ai-worker.md | 2 +- assets/orchestrator-delegation.md | 4 +- assets/orchestrator-memory.md | 13 +++++-- extensions/gentle-ai.ts | 4 +- odd/tasks/spec-by-reference-handoffs.md | 38 +++++++++++++++++++ tests/odd-routing-contract.test.ts | 50 ++++++++++++++++++++++--- tests/orchestrator-budget.test.ts | 2 +- 9 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 odd/tasks/spec-by-reference-handoffs.md diff --git a/assets/agents/gentle-ai-explore.md b/assets/agents/gentle-ai-explore.md index 4e2622778..9f64f625d 100644 --- a/assets/agents/gentle-ai-explore.md +++ b/assets/agents/gentle-ai-explore.md @@ -10,7 +10,7 @@ tools: You are the read-only explorer for generic ODD work. -Map relevant files, symbols, relationships, and uncertainty within the parent-provided scope. +Map relevant files, symbols, relationships, and uncertainty within the parent-provided scope. When the parent supplies an ODD feature document, read it until `## Log` for the requirements. - For structural questions, use the cwd-scoped `codegraph` tool before broad filesystem searches. Initialize the workspace index with `operation: "init"` when it is absent, then use `query` or `explore`; never ask it to target another path. - `codegraph` may create or update only the current workspace `.codegraph/` index. This is the sole permitted mutation; all tracked files, source files, and other project content remain read-only. diff --git a/assets/agents/gentle-ai-verify.md b/assets/agents/gentle-ai-verify.md index 405ccb8aa..f0ed5a164 100644 --- a/assets/agents/gentle-ai-verify.md +++ b/assets/agents/gentle-ai-verify.md @@ -14,6 +14,8 @@ Inspect relevant evidence and execute only exact test, build, or lint commands e For behavior changes with applicable runnable deterministic tests and a clear expected outcome, test-first is the default: assess observed RED before implementation, observed GREEN afterward, and focused checks after refactor. Do not infer RED from a test file existing or claim GREEN without observed execution. For passive documentation, non-testable changes, an unavailable runner, or no meaningful RED, assess the stated exception and proportionate ordinary functional or structural verification. Test presence alone is not applicability; never demand a TUI or chat toggle, invent lifecycle evidence, or skip checks. +When the parent supplies an ODD feature document, read all of it, including the verbatim user entries in `## Log`, and return a verdict per `S#`: met, unmet, or unverified, with evidence. Run the spec's example commands when the parent authorized them; when an example mutates state, run it only against isolated state the parent named (for example a temporary data file), then compare the exact output and error text with the spec. Passing tests never prove an `S#` whose example was not executed. + - Do not edit, write, or fix findings. - Do not run unapproved commands, alter an authorized command, install dependencies, or mutate repository state. Authorized commands may create only outputs the parent explicitly identified as expected. - Treat every unexpected mutation as a blocker: report it, but do not clean it up or fix it. diff --git a/assets/agents/gentle-ai-worker.md b/assets/agents/gentle-ai-worker.md index a4fd794ca..e42df1664 100644 --- a/assets/agents/gentle-ai-worker.md +++ b/assets/agents/gentle-ai-worker.md @@ -25,7 +25,7 @@ Before repository work: 1. Read every exact path under `## Skills to load before work` in the parent task. Do not rediscover the skill registry. 2. Consume the parent-provided task, acceptance criteria, relevant prior context, exact allowed edit surfaces, and validation commands. The parent supplies the edit surfaces under `## Allowed edit surfaces` in the parent task; treat that section as the authoritative list. - Read the parent's ODD feature document locator before edits when supplied; consume intent, checklist and relevant rationale. Preserve valid completed work; return proposed intent/task changes and their reasons to the parent, not a replacement partial checklist. Findings do not authorize scope expansion. + Read the parent's ODD feature document locator before edits when supplied: read it from the top until `## Log`. Its `## Specs` are authoritative over any summary in the handoff, and their quoted strings, error messages, and examples are exact. Report in `summary` which `S#` the change covers and any linked `S#` it could not satisfy. Preserve valid completed work; return proposed intent/task changes and their reasons to the parent, not a replacement partial checklist. Findings do not authorize scope expansion. 3. Inspect the working tree and preserve pre-existing changes. Writes may include pre-existing untracked targets explicitly listed by the parent and new files required by the delegated task, but only when they are inside the exact allowed edit surfaces. 4. Preserve every unrelated tracked or untracked file. Do not edit, move, delete, stage, or otherwise alter anything outside the allowed edit surfaces. 5. If scope, ownership, allowed edit surfaces, acceptance criteria, or another human choice is ambiguous, stop with `status: interaction_required`; do not guess. Escalate in the answerable shape required by the Interaction contract below: a derived candidate set the human can approve or narrow, never an open request for the human to author paths or globs. diff --git a/assets/orchestrator-delegation.md b/assets/orchestrator-delegation.md index 46a3fcc95..13277f58a 100644 --- a/assets/orchestrator-delegation.md +++ b/assets/orchestrator-delegation.md @@ -57,7 +57,7 @@ The sections below bind generic delegation rules to Pi's concrete runtime. They ## Language Boundary — subagent-facing English + exceptions -Subagent-facing prompts should be written in English by default, even when the user speaks Spanish. Translate the user's request into concise English before delegation. This keeps token usage lower and gives built-in/project subagents a consistent operating language without changing the user-facing persona. +Subagent-facing prompts should be written in English by default, even when the user speaks Spanish. Write the handoff instructions in English, but never translate, condense, or paraphrase the user's requirements: pass them by reference to the feature document's `## Specs` (`Spec: odd/tasks/.md (read until "## Log"). Do T#; S#.`), or include the user's request verbatim when no feature document exists. This keeps orchestrator output small and gives built-in/project subagents a consistent operating language without changing the user-facing persona or losing exact requirements. Exceptions: @@ -72,7 +72,7 @@ These instructions apply to all development work. Preserve the existing direct/d Investigation, explanation, review, comparison, and proposal-only requests remain read-only: no writer, apply, or implementation artifacts. Ambiguous or conditional change intent requires one clarification; stop and wait. Explore existing code and requirements proportionately first. Research findings and automatic execution pace never authorize mutations. -Small, understood work creates no durable task artifacts. Substantial means coordinated steps or progress worth recovering, not a line threshold. For substantial authorized implementation, automatically create the feature document after exploration, without a task or storage permission prompt. Follow `orchestrator-memory.md` for the file and full Engram recovery copy, automatic updates, truthful checkoffs, and resume. Keep the parent responsible for reconciliation; bounded workers receive the document locator, relevant intent, task IDs, authorized scope and checks, not authority over unrelated tasks; they read the document before edits. +Small, understood work creates no durable task artifacts. Substantial means coordinated steps or progress worth recovering, not a line threshold. For substantial authorized implementation, automatically create the feature document after exploration, without a task or storage permission prompt. Follow `orchestrator-memory.md` for the file and full Engram recovery copy, automatic updates, truthful checkoffs, and resume. Keep the parent responsible for reconciliation; bounded workers receive the document locator, task IDs, linked `S#`, authorized scope and checks, not authority over unrelated tasks; they read the document until `## Log` before edits and report which `S#` they covered. A rejected or resumed handoff is repaired and resent with the same content, never shortened. Reproduce a user-reported failure by running the reported example before deciding it already works; reading code is not reproduction. Use about 400 authored changed lines per ODD task only as a planning heuristic, counting additions plus deletions. Keep the smallest coherent behavior with its tests and docs. This is not a task acceptance criterion, hard cap, counter-trigger, automatic stop, forced split, or RDD trigger. If the correct clear solution naturally exceeds it, briefly explain why and continue without size-only rework loops. Never delete spaces, blank lines, or comments for cosmetic line savings; never omit tests, minify, add gratuitous abstractions, or split artificially to fit the heuristic. Forward this same advisory-only instruction when delegating tasks to subagents. The delivery budget below reads the accumulated branch, not this per-task heuristic. Existing PR size gates remain unchanged; follow repository policy. diff --git a/assets/orchestrator-memory.md b/assets/orchestrator-memory.md index 819fa6fa6..367bcfbf1 100644 --- a/assets/orchestrator-memory.md +++ b/assets/orchestrator-memory.md @@ -4,15 +4,22 @@ Bind this to the parent Pi session only, on organic progress/recovery. Not alway ### Organic feature continuity -For substantial authorized organic implementation, the parent maintains `odd/tasks/.md` and an Engram recovery copy under topic `odd//tasks`, scoped to the current project. Use a descriptive filename-safe feature name, reuse the same identity, and never overwrite another feature. Keep one feature document, not a separate plan file or topic: objective, problem, why, scope, constraints, actionable checklist with stable task IDs, acceptance criteria and checks, progress, verification evidence, and next step. Include concise rationale for meaningful accepted changes. Routine corrections stay with their tasks; no exhaustive decision journal. Mirror the full current document and repository-relative file locator, not only a summary or completion notice. +For substantial authorized organic implementation, the parent maintains `odd/tasks/.md` and an Engram recovery copy under topic `odd//tasks`, scoped to the current project. Use a descriptive filename-safe feature name, reuse the same identity, and never overwrite another feature. Keep one feature document, not a separate plan file or topic. It is the specification subagents read by reference, so keep this fixed order, stable content first and the growing log last: -Accepted user, review, or verification changes automatically update affected intent and TODOs: preserve valid completed and unrelated work; add genuinely new tasks or reopen invalidated items with a reason, and revise their checks. Findings alone never authorize scope expansion or automatic acceptance. New business scope still requires user authorization. Check off only observed outcomes with applicable proof; record failed, unavailable, skipped, or pending checks honestly. The parent merges bounded worker results rather than replacing the entire feature with one worker's partial view. +1. A header of two or three lines: objective, branch, delivery strategy, and test runner. +2. `## Specs`: numbered `S1`..`Sn` requirements, constraints, and acceptance criteria with their checks. Quote the user's exact strings, error messages, commands, and examples verbatim, in the user's language; you may number and order them, but never summarize or reword those fragments. Do not add requirements the user never asked for; mark assumptions as assumptions. +3. `## Tasks`: one line per task, using stable task IDs, with its linked `S#`, route (inline or delegated), status, and commit identity or RED/GREEN evidence. +4. `## Log`: the logbook. `L1` holds the user's original request verbatim; later user corrections are logged verbatim with their date. Problem analysis, rationale for meaningful accepted changes, verification evidence, progress, and next step go here. Routine corrections stay brief; no exhaustive decision journal. + +Mirror the full current document and repository-relative file locator, not only a summary or completion notice. + +Accepted user, review, or verification changes automatically update affected intent and TODOs, unlike a process that regenerates every artifact: a requirement change appends its verbatim `## Log` entry, rewrites only the affected `S#`, and reopens only its linked task; preserve valid completed and unrelated work; add genuinely new tasks or reopen invalidated items with a reason, and revise their checks. Findings alone never authorize scope expansion or automatic acceptance. New business scope still requires user authorization. Check off only observed outcomes with applicable proof; record failed, unavailable, skipped, or pending checks honestly. The parent merges bounded worker results rather than replacing the entire feature with one worker's partial view. Persist local progress first, then mirror through the existing injected Engram save tool. Read back both writes; they are not atomic. If Engram is unavailable, preserve local progress and explicitly mark the mirror pending; do not claim persistence succeeded or block unrelated safe work. Resynchronize when available. If a file write is unsafe or unavailable, preserve existing state and report the limitation. Preserve both versions on irreconcilable edits and ask only about the real conflict; never silently prefer a newer timestamp. On resume, use `mem_context`, then project/feature-scoped `mem_search`, and `mem_get_observation` for the full saved document; read the actual task file. Do not infer active work from the newest global memory. Reconcile current requirements, code, and proof before continuing the next unfinished task. Preserve pending mirrors and conflicting edits; a missing copy is not permission to overwrite surviving progress. Use the injected equivalents of these existing memory tools, never invent availability. -Before implementation or resume, the parent reads both the actual file and full observation, reconciles them, and passes the locator and relevant context; workers read the document before edits. Small work without a document still receives its authorized scope and checks. +Before implementation or resume, the parent reads both the actual file and full observation, reconciles them, and passes the locator, task IDs, and linked `S#`; workers read the document until `## Log` before edits. Small work without a document still receives its authorized scope and checks. The existing `todo` tool is the required session/UI projection for substantial ODD, not a third authority. After reconciling and writing the durable file and Engram copy, create or rebuild the visible `todo` list from the same feature tasks before the first source write; after every task transition and material plan change, update both durable copies and the visible projection in the same turn; its replay or completed-list clearing must not delete or replace the durable file or Engram copy. If the projection is unavailable, record that limitation without pretending it is synchronized. Small/read-only work does not acquire an ODD artifact or todo list merely because the UI can display tasks. diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 028ce2242..743bd2252 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -1255,8 +1255,8 @@ Organic Driven Development (ODD) is the predefined workflow of this orchestrator 2. **Explore.** Explore existing code and requirements first, proportionately to the request, before proposing or writing anything. 3. **Resolve uncertainty.** Recommend optional research only for a named uncertainty; ask one focused user question only for a real unresolved product decision, then stop and wait; use at most one scoped read-only assumption challenge for a high-consequence unproven premise. 4. **Classify.** The work is substantial when exploration yields two or more meaningful implementation steps, or progress worth recovering after an interruption. Small, understood work stays small and creates no durable task artifacts. -5. **Track before the first write.** For substantial authorized implementation, create \`odd/tasks/.md\` and its Engram mirror \`odd//tasks\` automatically, then create or rebuild the visible \`todo\` list from the reconciled feature tasks, all before the first source write and without asking permission for tasks or storage. Tell the user in one line which feature document was created and how many tasks it holds. -6. **Implement task by task.** Route each task through the orchestrator's Work Routing Ladder, honoring its mandatory delegation triggers, with applicable test-first development and checks. These triggers are mandatory, not advisory: executing past a fired trigger inline is a routing defect even if the work succeeds. Check an item off only after its outcome and checks were observed; update the file, mirror, and visible \`todo\` projection after every task transition and material plan change. Every task closes with at least one work-unit commit on the feature branch, branch first when on the default branch, with tests and docs alongside the behavior, using a Conventional Commit message; record the commit identity in the feature document as evidence. Work-unit commits on the feature branch are part of authorized substantial ODD implementation; push, pull request creation, and merge remain the user's decisions. +5. **Track before the first write.** For substantial authorized implementation, create \`odd/tasks/.md\` and its Engram mirror \`odd//tasks\` automatically, then create or rebuild the visible \`todo\` list from the reconciled feature tasks, all before the first source write and without asking permission for tasks or storage. Tell the user in one line which feature document was created and how many tasks it holds. The document is the specification subagents read, in this order: a two- or three-line header; \`## Specs\` with numbered \`S#\` that quote the user's exact strings, error messages, and examples verbatim, never summarized and never adding unrequested requirements; \`## Tasks\` with one line per task (ID, linked \`S#\`, route, commit); \`## Log\` last, where \`L1\` is the user's original request verbatim and later user corrections, evidence, and decisions are appended. A requirement change appends its verbatim Log entry, rewrites only the affected \`S#\`, and reopens only its task. +6. **Implement task by task.** Hand off by reference, never by paraphrase: \`Spec: odd/tasks/.md (read until \`## Log\`). Do T#; S#.\` and ask which \`S#\` were covered. Without a feature document, include the user's request verbatim. Verify reads the whole document, runs the spec's examples, and returns a verdict per \`S#\`. When the user reports a failure, reproduce it before deciding it already works. Route each task through the orchestrator's Work Routing Ladder, honoring its mandatory delegation triggers, with applicable test-first development and checks. These triggers are mandatory, not advisory: executing past a fired trigger inline is a routing defect even if the work succeeds. Check an item off only after its outcome and checks were observed; update the file, mirror, and visible \`todo\` projection after every task transition and material plan change. Every task closes with at least one work-unit commit on the feature branch, branch first when on the default branch, with tests and docs alongside the behavior, using a Conventional Commit message; record the commit identity in the feature document as evidence. Work-unit commits on the feature branch are part of authorized substantial ODD implementation; push, pull request creation, and merge remain the user's decisions. 7. **Close.** Report the verified outcome, every failed, skipped, or pending check, and the next step. The native review candidate is a work-unit commit or a PR slice, never a TODO checkbox and never the accumulated feature branch; native review runs only under the user-owned RDD switch. Phase reporting: the Gentle Shell prompt label is inferred automatically from the primary session's tool activity (reads show \`exploring\`, edits \`implementing\`, test runs \`checking\`, user questions \`deciding\`). When the \`gentle_odd_phase\` tool is available, use it to refine that label with phases tools cannot show (\`authorizing\`, \`researching\`, \`deciding\`, \`closing\`): call \`gentle_odd_phase\` only when the primary session's ODD phase actually changes, never per tool call or on a fixed cadence, and never from a subagent. It drives the Gentle Shell prompt label only. Resume an interrupted feature with \`mem_context\`, then project- and feature-scoped \`mem_search\`, then \`mem_get_observation\` for the full document, then the task file itself; reconcile before continuing the next unfinished task. Detail for steps 3–7: \`orchestrator-delegation.md\` and \`orchestrator-memory.md\`. diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md new file mode 100644 index 000000000..1f3ff8a0d --- /dev/null +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -0,0 +1,38 @@ +# Spec-by-reference ODD handoffs (gentle-shell#1713) +Branch: `fix/1713-spec-by-reference` (base `origin/main` cf3012f7) · Delivery: ask-on-risk · Runner: `node --experimental-strip-types --test `; full `npm test`, `npm run typecheck` +Engram mirror: `odd/spec-by-reference-handoffs/tasks` · Route: inline for every task (user: "para la tarea de hoy no quiero que delegues nada") + +## Specs +S1. The ODD feature doc is the reference: "el doc .md de ODD se genera cuando realmente se necesita, además de eso, la idea es que sea una referencia. Este tendria que tener las specs ahi mismo y funciona estilo bitacora." + Fixed order: header (2-3 lines) → `## Specs` → `## Tasks` → `## Log`. Stable content on top, growing log last. +S2. Specs quote, never summarize: exact strings, error messages and examples from the user stay literal, in the user's language. Specs may number and order, never rewrite those fragments. +S3. Change = only the affected spec and task: "A diferencia de SDD, un cambio no re hace todo, solo re hace esa spec y la tarea asociada". A change appends a verbatim user Log entry, rewrites only the affected S#, reopens only its T#. +S4. Log is the logbook: first entry L1 holds the original request verbatim; user corrections are logged verbatim; evidence and decisions go here, not in Tasks. Tasks stay one line each (S# links, route, commit, RED→GREEN). +S5. Handoffs pass a reference, not a paraphrase (goal: "minimizar el consumo de tokens y a la vez mantener la guia y calidad"): + Spec: odd/tasks/x.md (read until "## Log"). Do T2 → S3, S4. + Report covered S# and any S# you could not satisfy. + Workers read until `## Log`; when no doc exists, the handoff carries the user's request verbatim. No "translate/condense the request" step. +S6. Verify is grounded in the specs: reads the whole doc including verbatim user Log entries, returns a per-S# verdict, executes the spec's `$` examples the parent authorized (isolated when they mutate state), compares exact output. +S7. User-reported failures are reproduced before the orchestrator decides they already work. +S8. Handoff friction never shrinks content (#1713 cause 5): writer-surface rejection names the offending line and says to resend the same task unchanged except the section; `subagent_continue` of a writer inherits the original task's surfaces when the follow-up has none; an unknown task id lists recent valid ids. + +## Tasks +- [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · commit: see L9 +- [ ] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests +- [ ] T3 (S1-S6) pending user decision · port the doc format to the gentle-ai canon (`internal/components/agentguidance/routing.go`) and regenerate `fixtures/odd-routing-canonical.md` + +## Log +L1 2026-10-03 user (verbatim): > quiero que hagamos esto, pero pimero lo analicemos super bien https://github.com/Gentleman-Programming/gentle-shell/issues/1713 +L2 2026-10-03 user (verbatim): > espera para la tarea de hoy no quiero que delegues nada +L3 2026-10-03 user (verbatim): > es que el doc .md de ODD se genera cuando realmente se necesita, además de eso, la idea es que sea una referencia. Este tendria que tener las specs ahi mismo y funciona estilo bitacora. A diferencia de SDD, un cambio no re hace todo, solo re hace esa spec y la tarea asociada +L4 2026-10-03 user (verbatim): > mira como es la estructura ahora, dame la mejor referncia para minimizar el consumo de tokens y a la vez mantener la guia y calidad +L5 2026-10-03 user (verbatim): > me gsuta! vamos con esa +L6 2026-10-03 analysis: paraphrase rule at assets/orchestrator-delegation.md:60 (gentle-pi only); doc format at assets/orchestrator-memory.md:7 mirrored from gentle-ai routing.go; verify agent never sees requirements; no reproduce-first rule; bench B-luna handoffs 283/1786→1542→963/358/285 chars; existing docs median 6.6k, max 39.7k chars, dominated by logs. +L7 2026-10-03 forecast: ~250 authored changed lines across T1+T2 (under the ~400 budget); T3 lives in another repository. +L8 2026-10-03 user (verbatim): > orchestrator-memory.md es algo que recien se llama al hacer odd con archivo .md? + finding: orchestrator-memory.md and orchestrator-delegation.md are lazy (only named in assets/orchestrator.md:44,66); the B-luna orchestrator never read either. T1 must put the compact doc format and handoff contract in the always-on ODD steps (extensions/gentle-ai.ts:1253-1262) and keep detail in the lazy assets. +L9 2026-10-03 T1 evidence (risk: medium, prompt-contract change; checks: writer self-verification inline, no delegation per L2): + RED: tests/odd-routing-contract.test.ts new "feature document is the verbatim specification..." failed on missing `## Specs`; doc-format test failed on missing "verification evidence, progress, and next step". + GREEN: 20 prompt-contract test files 589/589; `npm run typecheck` no regressions; `npm test` all stages passed. + Decision: fixture line 19 (LB2 "Translate the user's request into concise English") moved to `replaced` in tests/orchestrator-budget.test.ts; its first sentence stays pinned by tests/persona-single-channel.test.ts. + Always-on prompt grew by the compact contract in extensions/gentle-ai.ts steps 5-6 (orchestrator.md budget untouched). diff --git a/tests/odd-routing-contract.test.ts b/tests/odd-routing-contract.test.ts index 611d94335..384cd751e 100644 --- a/tests/odd-routing-contract.test.ts +++ b/tests/odd-routing-contract.test.ts @@ -195,16 +195,15 @@ test("user documentation shows recovery and candidate-level consent without clai test("one feature document carries intent, accepted rationale and worker context", () => { containsAll(memory, [ "one feature document, not a separate plan file or topic", - "objective, problem, why, scope, constraints", - "progress, verification evidence, and next step", - "concise rationale for meaningful accepted changes", - "Routine corrections stay with their tasks; no exhaustive decision journal", + "verification evidence, progress, and next step", + "rationale for meaningful accepted changes", + "Routine corrections stay brief; no exhaustive decision journal", "Accepted user, review, or verification changes", "automatically update affected intent and TODOs", "add genuinely new tasks or reopen invalidated items with a reason", "Findings alone never authorize scope expansion or automatic acceptance", "Before implementation or resume, the parent reads both the actual file and full observation", - "passes the locator and relevant context; workers read the document before edits", + "passes the locator, task IDs, and linked `S#`; workers read the document until `## Log` before edits", ]); containsAll(read("assets/agents/gentle-ai-worker.md"), [ "Read the parent's ODD feature document locator before edits", @@ -212,6 +211,47 @@ test("one feature document carries intent, accepted rationale and worker context ]); }); +// gentle-shell#1713: handoffs paraphrased the user's request, the feature +// document summarized it, and verify never saw it. The document is now the +// specification subagents read by reference, in a fixed token-cheap order. +test("feature document is the verbatim specification subagents read by reference", () => { + const always = [ + "`## Specs`", + "`## Tasks`", + "`## Log`", + "exact strings, error messages, and examples verbatim", + "`L1` is the user's original request verbatim", + "rewrites only the affected `S#`, and reopens only its task", + "Hand off by reference, never by paraphrase", + "read until `## Log`", + "Without a feature document, include the user's request verbatim", + "a verdict per `S#`", + "reproduce it before deciding it already works", + ]; + for (const persona of ["gentleman", "neutral"] as const) containsAll(__testing.buildGentlePrompt(persona), always); + containsAll(memory, [ + "specification subagents read by reference", + "stable content first and the growing log last", + "never summarize or reword those fragments", + "Do not add requirements the user never asked for", + "`L1` holds the user's original request verbatim", + "rewrites only the affected `S#`, and reopens only its linked task", + ]); + assert.doesNotMatch(delegation, /Translate the user's request into concise English/); + containsAll(delegation, [ + "never translate, condense, or paraphrase the user's requirements", + "Reproduce a user-reported failure", + ]); + containsAll(read("assets/agents/gentle-ai-worker.md"), ["until `## Log`", "which `S#` the change covers"]); + containsAll(read("assets/agents/gentle-ai-verify.md"), [ + "verbatim user entries in `## Log`", + "verdict per `S#`", + "compare the exact output and error text", + "isolated state", + ]); + containsAll(read("assets/agents/gentle-ai-explore.md"), ["until `## Log`"]); +}); + test("ODD defaults to applicable test-first without chat or TUI activation", () => { const worker = read("assets/agents/gentle-ai-worker.md"); const verify = read("assets/agents/gentle-ai-verify.md"); diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 02aaccc30..f0a873a37 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -197,7 +197,7 @@ const DISPOSITION_MAP: DispositionRange[] = [ { lines: [9, 13], target: "core", label: "Core Role" }, { lines: [15, 15], target: "core", label: "Language Boundary heading" }, { lines: [17, 17], target: "core", label: "Language Boundary LB1 pointer" }, - { lines: [19, 19], target: "delegation", label: "Language Boundary LB2 (subagent-English)" }, + { lines: [19, 19], target: "replaced", label: "Language Boundary LB2: request translation replaced by spec-by-reference handoffs (gentle-shell#1713)" }, { lines: [21, 21], target: "replaced", label: "Language Boundary LB3: retired artifact types" }, { lines: [23, 23], target: "core", label: "Language Boundary LB4 (public comment language)" }, { lines: [25, 28], target: "delegation", label: "Language Boundary LB5 (exceptions)" }, From b160c3b8c802c31013d2d62a559466f88e0de161 Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:28:10 +0200 Subject: [PATCH 2/6] fix(agents): keep rejected and resumed handoffs from shrinking Writer-surface rejections now name the offending line and ask for the same task unchanged, generic writer continuations inherit the surfaces their launch was admitted with, and unknown task ids list recent ids. Each of these failures made the orchestrator resend shorter, re-summarized handoffs that dropped requirements (gentle-shell#1713). --- extensions/gentle-agents.ts | 23 ++++++++++----- lib/bounded-writer-admission.ts | 37 +++++++++++++++++++++---- odd/tasks/spec-by-reference-handoffs.md | 9 ++++-- tests/bounded-writer-admission.test.ts | 19 ++++++++++++- tests/gentle-agents.test.ts | 2 ++ tests/writer-edit-surface-scope.test.ts | 27 +++++++++++++++++- 6 files changed, 100 insertions(+), 17 deletions(-) diff --git a/extensions/gentle-agents.ts b/extensions/gentle-agents.ts index dc6bb7f69..0a2f90625 100644 --- a/extensions/gentle-agents.ts +++ b/extensions/gentle-agents.ts @@ -42,7 +42,7 @@ import { CARD_TONE, renderCard } from "../lib/shell-card.ts"; import { openInExternalEditor } from "./gentle-shell.ts"; import { resolveGentlePiAgentHome, gentlePiConfigHome } from "../lib/agent-home.ts"; import { resolveProfilePin, resolveUnversionedProjectProfile } from "../lib/agent-profile-pin.ts"; -import { allowedEditSurfaces, isDevelopmentSurface, isGenericBoundedWriter, prepareBoundSessionRepository, rejectUnscopedBoundedWriterDispatch, safeBootstrapDirectory, sessionRepositoryAuthority } from "../lib/bounded-writer-admission.ts"; +import { allowedEditSurfaces, inheritAllowedEditSurfaces, isDevelopmentSurface, isGenericBoundedWriter, prepareBoundSessionRepository, rejectUnscopedBoundedWriterDispatch, safeBootstrapDirectory, sessionRepositoryAuthority } from "../lib/bounded-writer-admission.ts"; import { CHILD_METRICS_EVENT, CHILD_METRICS_REVOKED, childEvent, launchSelection, type LaunchSelection } from "../lib/runtime-metrics-children.ts"; import { runtimeMetricsEnvAllows, type RuntimeMetricsPolicyDeps } from "../lib/runtime-metrics-policy.ts"; @@ -1117,6 +1117,14 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = return stored?.task; }; + // A guessed id ("1") leads back to real ids instead of a dead end, so the + // parent retries the same call rather than re-summarizing it (gentle-shell#1713). + const unknownTask = (id: unknown, ctx: ExtensionContext) => { + const recent = [...store.list(ctx.sessionManager.getSessionId() ?? "")].sort((a, b) => b.createdAt - a.createdAt).slice(0, 5); + const hint = recent.length ? ` Recent task ids: ${recent.map((task) => `${task.id} (${task.agent})`).join(", ")}.` : ""; + return text(`Error: no task ${String(id)}.${hint}`, { error: "unknown task" }); + }; + // Restores this exact session's own finished subagents as visible history // on an explicit resume, or on startup into a session that already has // entries -- never on new, fork, or reload (see the session_start handler's @@ -1636,14 +1644,14 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = }, ); - tool("status", "Report the status of one subagent task.", { required: ["task_id"], properties: { task_id: { type: "string" } } }, async (params) => { + tool("status", "Report the status of one subagent task.", { required: ["task_id"], properties: { task_id: { type: "string" } } }, async (params, ctx) => { const task = await resolveTask(String(params.task_id)); - return task ? text(describeTask(task), taskDetails(task)) : text(`Error: no task ${String(params.task_id)}`, { error: "unknown task" }); + return task ? text(describeTask(task), taskDetails(task)) : unknownTask(params.task_id, ctx); }); - tool("result", "Return the final answer of a finished subagent task, or its current state if it is still running.", { required: ["task_id"], properties: { task_id: { type: "string" } } }, async (params) => { + tool("result", "Return the final answer of a finished subagent task, or its current state if it is still running.", { required: ["task_id"], properties: { task_id: { type: "string" } } }, async (params, ctx) => { const task = await resolveTask(String(params.task_id)); - if (!task) return text(`Error: no task ${String(params.task_id)}`, { error: "unknown task" }); + if (!task) return unknownTask(params.task_id, ctx); // The parent just pulled a finished result; its pending completion must // never be replayed on top of it. if (isFinished(task.status)) { @@ -1683,7 +1691,7 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = async (params, ctx, signal) => { if (Object.hasOwn(params, "sdd_change") || Object.hasOwn(params, "remediation") || Object.hasOwn(params, "research_selection")) return text("Error: retired SDD delegation is not supported.", { error: "retired SDD delegation" }); const previous = await resolveTask(String(params.task_id)); - if (!previous) return text(`Error: no task ${String(params.task_id)}`, { error: "unknown task" }); + if (!previous) return unknownTask(params.task_id, ctx); if (retiredSddAgent(previous.agent)) return text("Error: retired SDD agents cannot be continued.", { error: "retired SDD delegation" }); if (!isFinished(previous.status) || !previous.sessionPath) return text(`Error: task ${previous.id} cannot be continued yet (${previous.status}).`, { error: "not continuable" }); // Continuing acts on the previous result, so any pending completion for @@ -1694,7 +1702,8 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = if (!agent) return text(`Error: subagent "${previous.agent}" is no longer defined.`, { error: "unknown agent" }); const mode = (params.mode as AgentMode | undefined) ?? (previous.mode as AgentMode); const foreignContinuation = foreignTasks.has(previous.id); - return launch(ctx, await buildRequest(ctx, agent, String(params.prompt ?? ""), typeof params.label === "string" ? params.label : undefined, typeof params.context === "string" ? params.context : undefined, mode, previous.sessionPath, foreignContinuation ? undefined : previous.cwd, signal, foreignContinuation ? previous.cwd : undefined), signal); + const prompt = inheritAllowedEditSurfaces(previous.agent, String(params.prompt ?? ""), params.context, previous.prompt); + return launch(ctx, await buildRequest(ctx, agent, prompt, typeof params.label === "string" ? params.label : undefined, typeof params.context === "string" ? params.context : undefined, mode, previous.sessionPath, foreignContinuation ? undefined : previous.cwd, signal, foreignContinuation ? previous.cwd : undefined), signal); }, ); diff --git a/lib/bounded-writer-admission.ts b/lib/bounded-writer-admission.ts index 00d02d128..7336a3f63 100644 --- a/lib/bounded-writer-admission.ts +++ b/lib/bounded-writer-admission.ts @@ -20,7 +20,9 @@ function isTaskScopedRepositoryRelativePath(value: string, backticked: boolean): // This is the same canonical parser used by both the tool hook and executor. // Prose cannot close the section, and repeated sections must agree exactly. -export function allowedEditSurfaces(...values: unknown[]): string[] | undefined { +// A rejection carries the concrete problem so the caller repairs only the +// section instead of re-summarizing the whole task (gentle-shell#1713). +function parseAllowedEditSurfaces(values: readonly unknown[]): { paths: string[] } | { problem: string } { let expected: string[] | undefined; for (const value of values) { if (typeof value !== "string") continue; @@ -28,29 +30,52 @@ export function allowedEditSurfaces(...values: unknown[]): string[] | undefined const lines = value.slice((heading.index ?? 0) + heading[0].length).split(/\r?\n/); const end = lines.findIndex(line => MARKDOWN_HEADING_LINE.test(line)); const entries = (end === -1 ? lines : lines.slice(0, end)).map(line => line.replace(/ +$/g, "")).filter(Boolean); - if (!entries.length) return undefined; + if (!entries.length) return { problem: "The section has no entries." }; const paths: string[] = []; for (const source of entries) { const line = source.replace(/^ {0,3}/, ""); const unlisted = line.replace(MARKDOWN_LIST_MARKER, ""); const quoted = unlisted.match(/^`([^`]+)`$/); const path = quoted?.[1] ?? unlisted; - if (/^(?:[-*+]|\d+[.)])$/.test(line) || (unlisted.includes("`") && !quoted) || /\p{Cc}|\p{Zl}|\p{Zp}/u.test(source) || !isTaskScopedRepositoryRelativePath(path, !!quoted)) return undefined; + if (/^(?:[-*+]|\d+[.)])$/.test(line) || (unlisted.includes("`") && !quoted) || /\p{Cc}|\p{Zl}|\p{Zp}/u.test(source) || !isTaskScopedRepositoryRelativePath(path, !!quoted)) { + const shown = source.trim().replace(/\p{Cc}|\p{Zl}|\p{Zp}/gu, " "); + return { problem: `Line "${shown.length > 120 ? `${shown.slice(0, 120)}...` : shown}" is not a valid surface entry; move prose under a following Markdown heading.` }; + } paths.push(path); } const unique = [...new Set(paths)].sort(); - if (expected && (expected.length !== unique.length || expected.some((path, index) => path !== unique[index]))) return undefined; + if (expected && (expected.length !== unique.length || expected.some((path, index) => path !== unique[index]))) return { problem: "Repeated sections list different surfaces." }; expected = unique; } } - return expected; + return expected ? { paths: expected } : { problem: "No `## Allowed edit surfaces` heading was found." }; +} + +export function allowedEditSurfaces(...values: unknown[]): string[] | undefined { + const parsed = parseAllowedEditSurfaces(values); + return "paths" in parsed ? parsed.paths : undefined; } export function rejectUnscopedBoundedWriterDispatch(input: unknown): { block: true; reason: string } | undefined { if (!input || typeof input !== "object" || Array.isArray(input)) return undefined; const record = input as Record; if (typeof record.agent !== "string" || !WRITER_NAMES.includes(record.agent)) return undefined; - return allowedEditSurfaces(record.task, record.context) ? undefined : { block: true, reason: WRITER_EDIT_SURFACE_REJECTION }; + const parsed = parseAllowedEditSurfaces([record.task, record.context]); + if ("paths" in parsed) return undefined; + return { block: true, reason: `${WRITER_EDIT_SURFACE_REJECTION} ${parsed.problem} Resend the same task text unchanged except for that section; never shorten or re-summarize it.` }; +} + +// A continuation resumes the same delegated task, so a writer follow-up that +// carries no section of its own inherits the surfaces its original launch was +// admitted with. A follow-up that carries the heading is validated as written, +// never merged, and nothing is inherited when the original had no valid section. +// Judgment Day fix batches keep their own exact dispatch protocol. +export function inheritAllowedEditSurfaces(agent: string, followUp: string, context: unknown, originalPrompt: string): string { + const hasHeading = (value: unknown) => typeof value === "string" && /^## Allowed edit surfaces[ \t]*$/im.test(value); + if (!isGenericBoundedWriter(agent) || hasHeading(followUp) || hasHeading(context)) return followUp; + const inherited = allowedEditSurfaces(originalPrompt); + if (!inherited) return followUp; + return `${followUp}\n\n## Allowed edit surfaces\n${inherited.map(path => /\s/u.test(path) ? `\`${path}\`` : path).join("\n")}\n`; } export function isGenericBoundedWriter(name: string): boolean { diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md index 1f3ff8a0d..b9e500793 100644 --- a/odd/tasks/spec-by-reference-handoffs.md +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -17,8 +17,8 @@ S7. User-reported failures are reproduced before the orchestrator decides they a S8. Handoff friction never shrinks content (#1713 cause 5): writer-surface rejection names the offending line and says to resend the same task unchanged except the section; `subagent_continue` of a writer inherits the original task's surfaces when the follow-up has none; an unknown task id lists recent valid ids. ## Tasks -- [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · commit: see L9 -- [ ] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests +- [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · 9e890dba +- [x] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests · RED→GREEN · commit: see L10 - [ ] T3 (S1-S6) pending user decision · port the doc format to the gentle-ai canon (`internal/components/agentguidance/routing.go`) and regenerate `fixtures/odd-routing-canonical.md` ## Log @@ -36,3 +36,8 @@ L9 2026-10-03 T1 evidence (risk: medium, prompt-contract change; checks: writer GREEN: 20 prompt-contract test files 589/589; `npm run typecheck` no regressions; `npm test` all stages passed. Decision: fixture line 19 (LB2 "Translate the user's request into concise English") moved to `replaced` in tests/orchestrator-budget.test.ts; its first sentence stays pinned by tests/persona-single-channel.test.ts. Always-on prompt grew by the compact contract in extensions/gentle-ai.ts steps 5-6 (orchestrator.md budget untouched). +L10 2026-10-03 T2 evidence (risk: HIGH, touches writer admission; independent verifier not run because the user forbade delegation (L2); RDD assess unavailable: `gentle_review` not exposed in this session): + RED: rejection-detail test (writer-edit-surface-scope), missing `inheritAllowedEditSurfaces` export (bounded-writer-admission), "Recent task ids" assertion (gentle-agents) all failed first. + GREEN: writer-edit-surface-scope 14/14, bounded-writer-admission 8/8, gentle-agents 190/190; `npm run typecheck` no regressions; `npm test` all stages passed. + Decisions: inheritance only for generic writers (gentle-ai-worker, worker), never jd-fix-agent; only when the follow-up and its context carry no heading; inherited surfaces come from the original task prompt (surfaces passed only via `context` are not inherited and still reject). Rejection keeps the canonical text as prefix and appends the concrete problem plus "Resend the same task text unchanged...". + Gap: continuation inheritance is unit-tested on the helper; the one-line wiring in subagent_continue has no end-to-end writer test. diff --git a/tests/bounded-writer-admission.test.ts b/tests/bounded-writer-admission.test.ts index 6dae91c76..c0bf70e21 100644 --- a/tests/bounded-writer-admission.test.ts +++ b/tests/bounded-writer-admission.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; import { __testing } from "../extensions/gentle-ai.ts"; -import { bindSessionRepositoryPreparation, boundSessionRepositoryAuthorityCurrent, captureBoundSessionRepositoryAuthority, isDevelopmentSurface, prepareBoundSessionRepository, sessionRepositoryAuthority } from "../lib/bounded-writer-admission.ts"; +import { allowedEditSurfaces, inheritAllowedEditSurfaces, bindSessionRepositoryPreparation, boundSessionRepositoryAuthorityCurrent, captureBoundSessionRepositoryAuthority, isDevelopmentSurface, prepareBoundSessionRepository, sessionRepositoryAuthority } from "../lib/bounded-writer-admission.ts"; import type { NativeReviewCli } from "../lib/native-review-cli.ts"; test("development admission excludes sensitive, config and bookkeeping surfaces structurally", () => { @@ -93,3 +93,20 @@ test("explicit review protects canonical HOME aliases, filesystem root and sensi } assert.equal(calls, 0, "no native STATUS against protected directories"); }); + +// gentle-shell#1713: a writer continuation without its own section was +// rejected, and every retry shortened the follow-up. A continuation resumes +// the same delegated task, so it inherits the surfaces that launch admitted. +test("writer continuations inherit the original surfaces only when they carry none", () => { + const original = "Implement it.\n\n## Allowed edit surfaces\n- src/model.ts\n- `docs/with space.md`\n\n## Return\nReport"; + const inherited = inheritAllowedEditSurfaces("gentle-ai-worker", "Continue with the remaining specs.", undefined, original); + assert.deepEqual(allowedEditSurfaces(inherited), ["docs/with space.md", "src/model.ts"]); + assert.ok(inherited.startsWith("Continue with the remaining specs.")); + const own = "Continue.\n## Allowed edit surfaces\nsrc/other.ts"; + assert.equal(inheritAllowedEditSurfaces("gentle-ai-worker", own, undefined, original), own, "a follow-up section is validated as written, never merged"); + const viaContext = "## Allowed edit surfaces\nsrc/other.ts"; + assert.equal(inheritAllowedEditSurfaces("worker", "Continue.", viaContext, original), "Continue."); + assert.equal(inheritAllowedEditSurfaces("gentle-ai-explore", "Continue.", undefined, original), "Continue.", "non-writers are untouched"); + assert.equal(inheritAllowedEditSurfaces("jd-fix-agent", "Continue.", undefined, original), "Continue.", "Judgment Day fix batches keep their exact protocol"); + assert.equal(inheritAllowedEditSurfaces("gentle-ai-worker", "Continue.", undefined, "No surfaces here."), "Continue.", "nothing to inherit stays rejectable"); +}); diff --git a/tests/gentle-agents.test.ts b/tests/gentle-agents.test.ts index 5e1838a78..51bfbb941 100644 --- a/tests/gentle-agents.test.ts +++ b/tests/gentle-agents.test.ts @@ -3148,6 +3148,8 @@ test("background runs return at once; status, result, send_message, cancel, and assert.equal((await resumed).content[0].text, "Summary."); assert.match((await tools.get("subagent_cancel")!.execute("c9", { task_id: id }, undefined, undefined, ctx)).content[0].text, /not running/); assert.match((await tools.get("subagent_status")!.execute("c10", { task_id: "nope" }, undefined, undefined, ctx)).content[0].text, /Error: no task nope/); + // gentle-shell#1713: a guessed id ("1") must point back to real ids. + assert.match((await tools.get("subagent_continue")!.execute("c10b", { task_id: "1", prompt: "more" }, undefined, undefined, ctx)).content[0].text, new RegExp(`^Error: no task 1\\. Recent task ids: .*${id} \\(explore\\)`)); assert.match((await tools.get("subagent_run")!.execute("c11", { agent: "ghost", task: "x" }, undefined, undefined, ctx)).content[0].text, /no subagent named "ghost"\. Known: explore/); }); diff --git a/tests/writer-edit-surface-scope.test.ts b/tests/writer-edit-surface-scope.test.ts index 295aeb32d..c6a680e64 100644 --- a/tests/writer-edit-surface-scope.test.ts +++ b/tests/writer-edit-surface-scope.test.ts @@ -66,9 +66,34 @@ async function assertAccepted(input: Record, message: string) { } async function assertRejected(input: Record, message: string) { - assert.deepEqual(await dispatchWriter(input), { block: true, reason: REJECTION }, message); + const result = await dispatchWriter(input); + assert.equal(result?.block, true, message); + assert.ok(result.reason.startsWith(REJECTION), message); } +// gentle-shell#1713: an unexplained rejection made the orchestrator resend +// shorter, re-summarized tasks (1786 -> 1542 chars) and lose requirements. +test("rejection names the offending line and asks for the same task unchanged", async () => { + const result = await dispatchWriter({ + agent: "gentle-ai-worker", + mode: "task", + task: [ + "Implement split transactions.", + "", + "## Allowed edit surfaces", + "- src/model.ts", + "- test/**/*.test.ts", + "", + "Requirements: one parent transaction with ordered allocations.", + ].join("\n"), + }); + assert.equal(result?.block, true); + assert.match(result.reason, /Line "Requirements: one parent transaction with ordered allocations\." is not a valid surface entry/); + assert.match(result.reason, /Resend the same task text unchanged except for that section; never shorten or re-summarize it\./); + const missing = await dispatchWriter({ agent: "gentle-ai-worker", mode: "task", task: "Implement it." }); + assert.match(missing?.reason ?? "", /No `## Allowed edit surfaces` heading was found\./); +}); + test("task-scoped surfaces are accepted ahead of a deeper heading", async () => { await assertAccepted({ agent: "gentle-ai-worker", From 6c9ebdd78a9efad3d8e4fcb92ba62c3ae98474fa Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:28:18 +0200 Subject: [PATCH 3/6] docs(odd): record spec-by-reference task evidence --- odd/tasks/spec-by-reference-handoffs.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md index b9e500793..7121718cc 100644 --- a/odd/tasks/spec-by-reference-handoffs.md +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -18,7 +18,7 @@ S8. Handoff friction never shrinks content (#1713 cause 5): writer-surface rejec ## Tasks - [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · 9e890dba -- [x] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests · RED→GREEN · commit: see L10 +- [x] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests · RED→GREEN · b160c3b8 - [ ] T3 (S1-S6) pending user decision · port the doc format to the gentle-ai canon (`internal/components/agentguidance/routing.go`) and regenerate `fixtures/odd-routing-canonical.md` ## Log @@ -41,3 +41,4 @@ L10 2026-10-03 T2 evidence (risk: HIGH, touches writer admission; independent ve GREEN: writer-edit-surface-scope 14/14, bounded-writer-admission 8/8, gentle-agents 190/190; `npm run typecheck` no regressions; `npm test` all stages passed. Decisions: inheritance only for generic writers (gentle-ai-worker, worker), never jd-fix-agent; only when the follow-up and its context carry no heading; inherited surfaces come from the original task prompt (surfaces passed only via `context` are not inherited and still reject). Rejection keeps the canonical text as prefix and appends the concrete problem plus "Resend the same task text unchanged...". Gap: continuation inheritance is unit-tested on the helper; the one-line wiring in subagent_continue has no end-to-end writer test. +L11 2026-10-03 next: T3 needs a user decision (gentle-ai canon port lives in another repository); push/PR remain user decisions. Running authored delta: T1 64+/15-, T2 ~95 lines. From 2397fdfc361e2b6d5aaa01d49c9e0f04d67b3912 Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:31:24 +0200 Subject: [PATCH 4/6] docs(odd): record review outcome for spec-by-reference handoffs --- odd/tasks/spec-by-reference-handoffs.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md index 7121718cc..2f6224ac1 100644 --- a/odd/tasks/spec-by-reference-handoffs.md +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -42,3 +42,5 @@ L10 2026-10-03 T2 evidence (risk: HIGH, touches writer admission; independent ve Decisions: inheritance only for generic writers (gentle-ai-worker, worker), never jd-fix-agent; only when the follow-up and its context carry no heading; inherited surfaces come from the original task prompt (surfaces passed only via `context` are not inherited and still reject). Rejection keeps the canonical text as prefix and appends the concrete problem plus "Resend the same task text unchanged...". Gap: continuation inheritance is unit-tested on the helper; the one-line wiring in subagent_continue has no end-to-end writer test. L11 2026-10-03 next: T3 needs a user decision (gentle-ai canon port lives in another repository); push/PR remain user decisions. Running authored delta: T1 64+/15-, T2 ~95 lines. +L12 2026-10-03 RDD: user granted review of 9e890dba..6c9ebdd7 (risk medium, 14 files, 231 lines) and allowed one reviewer subagent as the only exception to L2. Lineage review-7744719648a3cec2, lens review-reliability: approved; acknowledged (authority burned). + Non-blocking follow-ups: R3-continue-wiring-unproved (WARNING, no end-to-end continue test); R3-inherit-reserialization (WARNING, inherited entries are backticked only for whitespace, so a quoted path such as one starting with a list marker could round-trip differently; fix: always backtick inherited entries); R3-heading-detection-divergence (SUGGESTION, reuse the canonical heading matcher); R3-rejection-problem-coverage (SUGGESTION, assert empty/repeated-section messages, truncation, status/result hints). From d97718203301863469203ed971364768acb120b7 Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:42:16 +0200 Subject: [PATCH 5/6] fix(agents): make inherited surfaces round-trip and rejections precise Inherited writer surfaces are always backticked so quoted-only entries round-trip, rejections tell invalid paths apart from prose, every rejection is asserted exactly, and the subagent_continue wiring is proven end to end (review follow-ups for gentle-shell#1713). --- lib/bounded-writer-admission.ts | 14 ++++++++--- odd/tasks/spec-by-reference-handoffs.md | 10 ++++++++ tests/bounded-writer-admission.test.ts | 4 ++++ tests/gentle-agents.test.ts | 32 +++++++++++++++++++++++++ tests/writer-edit-surface-scope.test.ts | 29 +++++++++++++++++++++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/lib/bounded-writer-admission.ts b/lib/bounded-writer-admission.ts index 7336a3f63..b8842fba9 100644 --- a/lib/bounded-writer-admission.ts +++ b/lib/bounded-writer-admission.ts @@ -38,8 +38,14 @@ function parseAllowedEditSurfaces(values: readonly unknown[]): { paths: string[] const quoted = unlisted.match(/^`([^`]+)`$/); const path = quoted?.[1] ?? unlisted; if (/^(?:[-*+]|\d+[.)])$/.test(line) || (unlisted.includes("`") && !quoted) || /\p{Cc}|\p{Zl}|\p{Zp}/u.test(source) || !isTaskScopedRepositoryRelativePath(path, !!quoted)) { - const shown = source.trim().replace(/\p{Cc}|\p{Zl}|\p{Zp}/gu, " "); - return { problem: `Line "${shown.length > 120 ? `${shown.slice(0, 120)}...` : shown}" is not a valid surface entry; move prose under a following Markdown heading.` }; + // Prose and a bad path need different repairs: moving a real surface + // out of the section would silently narrow the writer's scope. + const prose = /^(?:[-*+]|\d+[.)])$/.test(line) || (!quoted && /\p{White_Space}/u.test(unlisted)); + const shown = (prose ? source.trim() : path).replace(/\p{Cc}|\p{Zl}|\p{Zp}/gu, " "); + const bounded = shown.length > 120 ? `${shown.slice(0, 120)}...` : shown; + return { problem: prose + ? `Line "${bounded}" is not a valid surface entry; move prose under a following Markdown heading.` + : `Entry "${bounded}" is not a narrow repository-relative path; remove absolute paths, \`..\` segments, root globs, and stray backticks.` }; } paths.push(path); } @@ -75,7 +81,9 @@ export function inheritAllowedEditSurfaces(agent: string, followUp: string, cont if (!isGenericBoundedWriter(agent) || hasHeading(followUp) || hasHeading(context)) return followUp; const inherited = allowedEditSurfaces(originalPrompt); if (!inherited) return followUp; - return `${followUp}\n\n## Allowed edit surfaces\n${inherited.map(path => /\s/u.test(path) ? `\`${path}\`` : path).join("\n")}\n`; + // Every inherited entry is backticked: some entries are admitted only when + // quoted, and quoting never changes a valid path (review R3-002). + return `${followUp}\n\n## Allowed edit surfaces\n${inherited.map(path => `\`${path}\``).join("\n")}\n`; } export function isGenericBoundedWriter(name: string): boolean { diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md index 2f6224ac1..3888abadf 100644 --- a/odd/tasks/spec-by-reference-handoffs.md +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -19,6 +19,7 @@ S8. Handoff friction never shrinks content (#1713 cause 5): writer-surface rejec ## Tasks - [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · 9e890dba - [x] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests · RED→GREEN · b160c3b8 +- [x] T4 (S8) inline · review follow-ups R3-001/002/004/005: e2e continue test, always-backticked inherited surfaces, path-vs-prose rejection advice, exact rejection assertions · RED→GREEN · commit: this work unit (see git log) - [ ] T3 (S1-S6) pending user decision · port the doc format to the gentle-ai canon (`internal/components/agentguidance/routing.go`) and regenerate `fixtures/odd-routing-canonical.md` ## Log @@ -44,3 +45,12 @@ L10 2026-10-03 T2 evidence (risk: HIGH, touches writer admission; independent ve L11 2026-10-03 next: T3 needs a user decision (gentle-ai canon port lives in another repository); push/PR remain user decisions. Running authored delta: T1 64+/15-, T2 ~95 lines. L12 2026-10-03 RDD: user granted review of 9e890dba..6c9ebdd7 (risk medium, 14 files, 231 lines) and allowed one reviewer subagent as the only exception to L2. Lineage review-7744719648a3cec2, lens review-reliability: approved; acknowledged (authority burned). Non-blocking follow-ups: R3-continue-wiring-unproved (WARNING, no end-to-end continue test); R3-inherit-reserialization (WARNING, inherited entries are backticked only for whitespace, so a quoted path such as one starting with a list marker could round-trip differently; fix: always backtick inherited entries); R3-heading-detection-divergence (SUGGESTION, reuse the canonical heading matcher); R3-rejection-problem-coverage (SUGGESTION, assert empty/repeated-section messages, truncation, status/result hints). +L13 2026-10-03 RDD: doc-only L12 commit re-opened the whole branch range (target f913925b…); user granted review and re-confirmed the reviewer exception. Lineage review-52b799f6e8561bc4: approved; acknowledged. Findings R3-001 (WARNING, no e2e continue test), R3-002 (WARNING, always backtick inherited entries), R3-003 (SUGGESTION, reuse canonical heading matcher), R3-004 (SUGGESTION, rejection advice wrong for invalid paths), R3-005 (SUGGESTION, prefix-only rejection asserts; unasserted messages/truncation/hints). + Learned: RDD reviews the whole branch base-diff, so post-review doc commits re-open a full review; record outcomes in the next work unit. +L14 2026-10-03 user (verbatim): > dale + (accepting: fix R3-001, R3-002, R3-004, R3-005 and record L13 as T4; R3-003 stays a follow-up.) +L15 2026-10-03 T4 evidence (risk: HIGH, writer admission; reviewer exception only per L12/L13): + RED: e2e continue test failed only on unquoted `src/app.ts` (R3-002); quoted-only `-` round-trip failed; invalid paths reported as prose (R3-004). Status/result unknown-id hints already passed (coverage only). + Mutation: replacing the continue wiring with the raw prompt makes the e2e test fail with the exact rejection; restored, file unchanged vs HEAD. + GREEN: bounded-writer-admission 8/8, writer-edit-surface-scope 15/15, gentle-agents 191/191; `npm run typecheck` no regressions; `npm test` all stages passed. + Kept as follow-up: R3-003 (reuse canonical heading matcher). diff --git a/tests/bounded-writer-admission.test.ts b/tests/bounded-writer-admission.test.ts index c0bf70e21..230408b3d 100644 --- a/tests/bounded-writer-admission.test.ts +++ b/tests/bounded-writer-admission.test.ts @@ -109,4 +109,8 @@ test("writer continuations inherit the original surfaces only when they carry no assert.equal(inheritAllowedEditSurfaces("gentle-ai-explore", "Continue.", undefined, original), "Continue.", "non-writers are untouched"); assert.equal(inheritAllowedEditSurfaces("jd-fix-agent", "Continue.", undefined, original), "Continue.", "Judgment Day fix batches keep their exact protocol"); assert.equal(inheritAllowedEditSurfaces("gentle-ai-worker", "Continue.", undefined, "No surfaces here."), "Continue.", "nothing to inherit stays rejectable"); + // Review R3-002: an entry admitted only when quoted must round-trip quoted. + const quotedOnly = "## Allowed edit surfaces\n- `-`\n- src/model.ts"; + assert.deepEqual(allowedEditSurfaces(quotedOnly), ["-", "src/model.ts"]); + assert.deepEqual(allowedEditSurfaces(inheritAllowedEditSurfaces("worker", "Continue.", undefined, quotedOnly)), ["-", "src/model.ts"]); }); diff --git a/tests/gentle-agents.test.ts b/tests/gentle-agents.test.ts index 51bfbb941..2f8349201 100644 --- a/tests/gentle-agents.test.ts +++ b/tests/gentle-agents.test.ts @@ -3149,6 +3149,7 @@ test("background runs return at once; status, result, send_message, cancel, and assert.match((await tools.get("subagent_cancel")!.execute("c9", { task_id: id }, undefined, undefined, ctx)).content[0].text, /not running/); assert.match((await tools.get("subagent_status")!.execute("c10", { task_id: "nope" }, undefined, undefined, ctx)).content[0].text, /Error: no task nope/); // gentle-shell#1713: a guessed id ("1") must point back to real ids. + for (const name of ["subagent_status", "subagent_result"]) assert.match((await tools.get(name)!.execute("c10a", { task_id: "1" }, undefined, undefined, ctx)).content[0].text, new RegExp(`^Error: no task 1\\. Recent task ids: .*${id} \\(explore\\)`), name); assert.match((await tools.get("subagent_continue")!.execute("c10b", { task_id: "1", prompt: "more" }, undefined, undefined, ctx)).content[0].text, new RegExp(`^Error: no task 1\\. Recent task ids: .*${id} \\(explore\\)`)); assert.match((await tools.get("subagent_run")!.execute("c11", { agent: "ghost", task: "x" }, undefined, undefined, ctx)).content[0].text, /no subagent named "ghost"\. Known: explore/); }); @@ -4855,3 +4856,34 @@ test("children receive context and safety extensions, and missing files are omit } } }); + +// gentle-shell#1713 (review R3-001): prove the subagent_continue wiring end to +// end, not only the helper. A writer follow-up without its own section reaches +// the child with the surfaces its original launch was admitted with. +test("a writer continuation without its own section inherits the admitted surfaces end to end", async () => { + const h = fakePi(); + const runtime = deps(); + const profile = mkdtempSync(join(tmpdir(), "gentle-agents-continue-")); + mkdirSync(join(profile, "agents"), { recursive: true }); + writeFileSync(join(profile, "agents", "gentle-ai-worker.md"), readFileSync(new URL("../assets/agents/gentle-ai-worker.md", import.meta.url))); + writeFileSync(join(profile, "subagents.json"), JSON.stringify({ model_profiles: { "gentle-ai-worker": { model: "openai/gpt-4o", effort: "high" } } })); + const env: NodeJS.ProcessEnv = {}; + gentleAgents(h.pi, env, { ...runtime.deps, env, agentHome: profile }); + const { ctx } = fakeContext(); + await h.fire("session_start", ctx); + const launched = h.tools.get("subagent_run")!.execute("run", { agent: "gentle-ai-worker", task: "Do T2.\n\n## Allowed edit surfaces\n- src/app.ts\n- `docs/with space.md`\n\n## Return\nReport", mode: "task" }, undefined, undefined, ctx); + await tick(); + runtime.children[0].emit({ type: "agent_end", messages: [{ role: "assistant", content: [{ type: "text", text: "partial" }] }] }); + runtime.children[0].emit({ type: "agent_settled" }); + const taskId = ((await launched).details.gentleAgents as { taskId: string }).taskId; + const continued = h.tools.get("subagent_continue")!.execute("follow", { task_id: taskId, prompt: "Continue with the remaining specs.", mode: "task" }, undefined, undefined, ctx); + await tick(); + assert.equal(runtime.children.length, 2, "the continuation is admitted instead of rejected"); + const prompt = String(runtime.children[1].written.find(frame => typeof frame.message === "string")?.message); + assert.match(prompt, /^Continue with the remaining specs\.\n\n## Allowed edit surfaces\n`docs\/with space\.md`\n`src\/app\.ts`\n/); + runtime.children[1].emit({ type: "agent_end", messages: [{ role: "assistant", content: [{ type: "text", text: "done" }] }] }); + runtime.children[1].emit({ type: "agent_settled" }); + await continued; + await h.fire("session_shutdown", ctx); + rmSync(profile, { recursive: true, force: true }); +}); diff --git a/tests/writer-edit-surface-scope.test.ts b/tests/writer-edit-surface-scope.test.ts index c6a680e64..02dd74d2b 100644 --- a/tests/writer-edit-surface-scope.test.ts +++ b/tests/writer-edit-surface-scope.test.ts @@ -65,10 +65,23 @@ async function assertAccepted(input: Record, message: string) { assert.equal(await dispatchWriter(input), undefined, message); } +const RESEND = "Resend the same task text unchanged except for that section; never shorten or re-summarize it."; +const PROBLEM = /^(?:No `## Allowed edit surfaces` heading was found\.|The section has no entries\.|Repeated sections list different surfaces\.|Line ".+" is not a valid surface entry; move prose under a following Markdown heading\.|Entry ".+" is not a narrow repository-relative path; remove absolute paths, `\.\.` segments, root globs, and stray backticks\.)$/; + +// Every rejection is the canonical text, exactly one concrete problem, and the +// resend instruction; nothing else (review R3-005). +function rejectionProblem(reason: string): string { + assert.ok(reason.startsWith(`${REJECTION} `), "canonical rejection prefix"); + assert.ok(reason.endsWith(` ${RESEND}`), "resend instruction suffix"); + const problem = reason.slice(REJECTION.length + 1, reason.length - RESEND.length - 1); + assert.match(problem, PROBLEM); + return problem; +} + async function assertRejected(input: Record, message: string) { const result = await dispatchWriter(input); assert.equal(result?.block, true, message); - assert.ok(result.reason.startsWith(REJECTION), message); + rejectionProblem(result.reason); } // gentle-shell#1713: an unexplained rejection made the orchestrator resend @@ -94,6 +107,20 @@ test("rejection names the offending line and asks for the same task unchanged", assert.match(missing?.reason ?? "", /No `## Allowed edit surfaces` heading was found\./); }); +test("each rejection names its concrete problem, and invalid paths are not called prose", async () => { + const problem = async (input: Record) => rejectionProblem((await dispatchWriter({ agent: "gentle-ai-worker", mode: "task", ...input }))!.reason); + assert.equal(await problem({ task: "## Allowed edit surfaces\n\n## Return\nReport" }), "The section has no entries."); + assert.equal(await problem({ task: "## Allowed edit surfaces\nsrc/a.ts", context: "## Allowed edit surfaces\nsrc/b.ts" }), "Repeated sections list different surfaces."); + for (const path of ["/etc/passwd", "../outside.ts", "src/../../x.ts", "**/*.ts", "src/`odd.ts"]) { + assert.equal(await problem({ task: `## Allowed edit surfaces\n- ${path}` }), `Entry "${path}" is not a narrow repository-relative path; remove absolute paths, \`..\` segments, root globs, and stray backticks.`, path); + } + const long = `Then ${"verify every requirement ".repeat(8)}carefully.`; + assert.equal(await problem({ task: `## Allowed edit surfaces\nsrc/a.ts\n${long}` }), `Line "${long.slice(0, 120)}..." is not a valid surface entry; move prose under a following Markdown heading.`); + const control = await problem({ task: "## Allowed edit surfaces\nsrc/a\u0007.ts" }); + assert.doesNotMatch(control, /\u0007/); + assert.match(control, /^Entry "src\/a \.ts" is not a narrow repository-relative path/); +}); + test("task-scoped surfaces are accepted ahead of a deeper heading", async () => { await assertAccepted({ agent: "gentle-ai-worker", From 7c89cca48d7912656ac7e0edde6539bcaef3a8fa Mon Sep 17 00:00:00 2001 From: Alan-TheGentleman Date: Sat, 3 Oct 2026 18:52:11 +0200 Subject: [PATCH 6/6] fix(agents): never steer a quotable path out of the surfaces section An unquoted surface entry with whitespace that is valid when quoted now gets both repairs (backticks or move prose) instead of prose-only advice, which could narrow the writer's scope. Unknown task ids no longer require a context, and inheritance reuses the canonical heading matcher. --- extensions/gentle-agents.ts | 4 ++-- lib/bounded-writer-admission.ts | 18 +++++++++++++----- odd/tasks/spec-by-reference-handoffs.md | 7 ++++++- tests/gentle-agents.test.ts | 2 ++ tests/writer-edit-surface-scope.test.ts | 9 +++++++-- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/extensions/gentle-agents.ts b/extensions/gentle-agents.ts index 0a2f90625..5dd7c8d64 100644 --- a/extensions/gentle-agents.ts +++ b/extensions/gentle-agents.ts @@ -1119,8 +1119,8 @@ export default function gentleAgents(pi: ExtensionAPI, env: NodeJS.ProcessEnv = // A guessed id ("1") leads back to real ids instead of a dead end, so the // parent retries the same call rather than re-summarizing it (gentle-shell#1713). - const unknownTask = (id: unknown, ctx: ExtensionContext) => { - const recent = [...store.list(ctx.sessionManager.getSessionId() ?? "")].sort((a, b) => b.createdAt - a.createdAt).slice(0, 5); + const unknownTask = (id: unknown, ctx: ExtensionContext | undefined) => { + const recent = [...store.list(ctx?.sessionManager?.getSessionId() ?? "")].sort((a, b) => b.createdAt - a.createdAt).slice(0, 5); const hint = recent.length ? ` Recent task ids: ${recent.map((task) => `${task.id} (${task.agent})`).join(", ")}.` : ""; return text(`Error: no task ${String(id)}.${hint}`, { error: "unknown task" }); }; diff --git a/lib/bounded-writer-admission.ts b/lib/bounded-writer-admission.ts index b8842fba9..d50395695 100644 --- a/lib/bounded-writer-admission.ts +++ b/lib/bounded-writer-admission.ts @@ -6,7 +6,10 @@ import { resolveSessionWorktree, type WorktreeResolver } from "./session-worktre const WRITER_NAMES = ["gentle-ai-worker", "worker", "jd-fix-agent"]; export const WRITER_EDIT_SURFACE_REJECTION = "Writer tasks must include the exact Markdown heading `## Allowed edit surfaces` with narrow repository-relative paths or narrow globs, one per line. Every non-empty line belongs to the section until the next canonical Markdown heading and must be a valid surface entry. Paths containing whitespace require whole-entry backticks; begin explanatory prose under the next Markdown heading. The parent must derive or map that canonical block from the delegated task and relaunch the writer; do not accept aliases, and do not ask the human to author paths or globs."; -const ALLOWED_EDIT_SURFACES_HEADING = /^## Allowed edit surfaces[ \t]*$/gim; +// One heading matcher: the parser scans with the global form, and continuation +// inheritance tests presence with the same source (review R3-003). +const ALLOWED_EDIT_SURFACES_HEADING_LINE = /^## Allowed edit surfaces[ \t]*$/im; +const ALLOWED_EDIT_SURFACES_HEADING = new RegExp(ALLOWED_EDIT_SURFACES_HEADING_LINE.source, "gim"); const MARKDOWN_HEADING_LINE = /^ {0,3}#{1,6} /; const MARKDOWN_LIST_MARKER = /^(?:[-*+]|\d+[.)]) +/; @@ -40,12 +43,17 @@ function parseAllowedEditSurfaces(values: readonly unknown[]): { paths: string[] if (/^(?:[-*+]|\d+[.)])$/.test(line) || (unlisted.includes("`") && !quoted) || /\p{Cc}|\p{Zl}|\p{Zp}/u.test(source) || !isTaskScopedRepositoryRelativePath(path, !!quoted)) { // Prose and a bad path need different repairs: moving a real surface // out of the section would silently narrow the writer's scope. + // An unquoted entry with whitespace that would be valid when quoted is + // ambiguous: name both repairs instead of guessing prose. const prose = /^(?:[-*+]|\d+[.)])$/.test(line) || (!quoted && /\p{White_Space}/u.test(unlisted)); + const quotable = prose && !quoted && !unlisted.includes("`") && !/\p{Cc}|\p{Zl}|\p{Zp}/u.test(source) && isTaskScopedRepositoryRelativePath(unlisted, true); const shown = (prose ? source.trim() : path).replace(/\p{Cc}|\p{Zl}|\p{Zp}/gu, " "); const bounded = shown.length > 120 ? `${shown.slice(0, 120)}...` : shown; - return { problem: prose - ? `Line "${bounded}" is not a valid surface entry; move prose under a following Markdown heading.` - : `Entry "${bounded}" is not a narrow repository-relative path; remove absolute paths, \`..\` segments, root globs, and stray backticks.` }; + return { problem: quotable + ? `Line "${bounded}" is not a valid surface entry; if it is a path, wrap the whole entry in backticks, otherwise move it under a following Markdown heading.` + : prose + ? `Line "${bounded}" is not a valid surface entry; move prose under a following Markdown heading.` + : `Entry "${bounded}" is not a narrow repository-relative path; remove absolute paths, \`..\` segments, root globs, and stray backticks.` }; } paths.push(path); } @@ -77,7 +85,7 @@ export function rejectUnscopedBoundedWriterDispatch(input: unknown): { block: tr // never merged, and nothing is inherited when the original had no valid section. // Judgment Day fix batches keep their own exact dispatch protocol. export function inheritAllowedEditSurfaces(agent: string, followUp: string, context: unknown, originalPrompt: string): string { - const hasHeading = (value: unknown) => typeof value === "string" && /^## Allowed edit surfaces[ \t]*$/im.test(value); + const hasHeading = (value: unknown) => typeof value === "string" && ALLOWED_EDIT_SURFACES_HEADING_LINE.test(value); if (!isGenericBoundedWriter(agent) || hasHeading(followUp) || hasHeading(context)) return followUp; const inherited = allowedEditSurfaces(originalPrompt); if (!inherited) return followUp; diff --git a/odd/tasks/spec-by-reference-handoffs.md b/odd/tasks/spec-by-reference-handoffs.md index 3888abadf..0d5b40fc6 100644 --- a/odd/tasks/spec-by-reference-handoffs.md +++ b/odd/tasks/spec-by-reference-handoffs.md @@ -19,7 +19,8 @@ S8. Handoff friction never shrinks content (#1713 cause 5): writer-surface rejec ## Tasks - [x] T1 (S1-S7) inline · doc format + handoff/verify/reproduce contract in assets, agents, extension ODD step, contract tests · RED→GREEN · 9e890dba - [x] T2 (S8) inline · runtime friction in lib/bounded-writer-admission.ts and extensions/gentle-agents.ts + tests · RED→GREEN · b160c3b8 -- [x] T4 (S8) inline · review follow-ups R3-001/002/004/005: e2e continue test, always-backticked inherited surfaces, path-vs-prose rejection advice, exact rejection assertions · RED→GREEN · commit: this work unit (see git log) +- [x] T4 (S8) inline · review follow-ups R3-001/002/004/005: e2e continue test, always-backticked inherited surfaces, path-vs-prose rejection advice, exact rejection assertions · RED→GREEN · d9771820 +- [x] T5 (S8) inline · review follow-ups of lineage review-b5d3d60f29e54b69: ambiguous whitespace-entry advice, ctx-safe unknown-id error, one heading matcher (R3-003) · RED→GREEN · this work unit - [ ] T3 (S1-S6) pending user decision · port the doc format to the gentle-ai canon (`internal/components/agentguidance/routing.go`) and regenerate `fixtures/odd-routing-canonical.md` ## Log @@ -54,3 +55,7 @@ L15 2026-10-03 T4 evidence (risk: HIGH, writer admission; reviewer exception onl Mutation: replacing the continue wiring with the raw prompt makes the e2e test fail with the exact rejection; restored, file unchanged vs HEAD. GREEN: bounded-writer-admission 8/8, writer-edit-surface-scope 15/15, gentle-agents 191/191; `npm run typecheck` no regressions; `npm test` all stages passed. Kept as follow-up: R3-003 (reuse canonical heading matcher). +L16 2026-10-03 RDD: lineage review-b5d3d60f29e54b69 (target 53d44815…, through d9771820) approved; acknowledged. Findings: R3-001 (WARNING) a real unquoted path with a space got the prose advice; R3-002 (SUGGESTION) unknownTask dereferenced ctx unconditionally. R3-003 from the earlier review was still open. +L17 2026-10-03 user (verbatim): > pr y merge +L18 2026-10-03 user (verbatim): > no seria mejor arreglar los hallazgos ? + T5 evidence (risk: HIGH, writer admission; reviewers allowed per user): RED: ambiguous-entry test (writer-edit-surface-scope) and `TypeError ... reading 'sessionManager'` on a ctx-less subagent_status (gentle-agents). GREEN after fix. An unquoted whitespace entry that is valid when quoted now names both repairs (backticks or move prose); entries invalid even when quoted keep the prose advice. R3-003 is a pure refactor (one regex source), covered by existing inheritance tests. diff --git a/tests/gentle-agents.test.ts b/tests/gentle-agents.test.ts index 2f8349201..dfd4dd896 100644 --- a/tests/gentle-agents.test.ts +++ b/tests/gentle-agents.test.ts @@ -3149,6 +3149,8 @@ test("background runs return at once; status, result, send_message, cancel, and assert.match((await tools.get("subagent_cancel")!.execute("c9", { task_id: id }, undefined, undefined, ctx)).content[0].text, /not running/); assert.match((await tools.get("subagent_status")!.execute("c10", { task_id: "nope" }, undefined, undefined, ctx)).content[0].text, /Error: no task nope/); // gentle-shell#1713: a guessed id ("1") must point back to real ids. + // Review follow-up: a call without a context still gets the structured error. + assert.match((await tools.get("subagent_status")!.execute("c10c", { task_id: "1" }, undefined, undefined, undefined as never)).content[0].text, /^Error: no task 1\./); for (const name of ["subagent_status", "subagent_result"]) assert.match((await tools.get(name)!.execute("c10a", { task_id: "1" }, undefined, undefined, ctx)).content[0].text, new RegExp(`^Error: no task 1\\. Recent task ids: .*${id} \\(explore\\)`), name); assert.match((await tools.get("subagent_continue")!.execute("c10b", { task_id: "1", prompt: "more" }, undefined, undefined, ctx)).content[0].text, new RegExp(`^Error: no task 1\\. Recent task ids: .*${id} \\(explore\\)`)); assert.match((await tools.get("subagent_run")!.execute("c11", { agent: "ghost", task: "x" }, undefined, undefined, ctx)).content[0].text, /no subagent named "ghost"\. Known: explore/); diff --git a/tests/writer-edit-surface-scope.test.ts b/tests/writer-edit-surface-scope.test.ts index 02dd74d2b..4c2966273 100644 --- a/tests/writer-edit-surface-scope.test.ts +++ b/tests/writer-edit-surface-scope.test.ts @@ -66,7 +66,7 @@ async function assertAccepted(input: Record, message: string) { } const RESEND = "Resend the same task text unchanged except for that section; never shorten or re-summarize it."; -const PROBLEM = /^(?:No `## Allowed edit surfaces` heading was found\.|The section has no entries\.|Repeated sections list different surfaces\.|Line ".+" is not a valid surface entry; move prose under a following Markdown heading\.|Entry ".+" is not a narrow repository-relative path; remove absolute paths, `\.\.` segments, root globs, and stray backticks\.)$/; +const PROBLEM = /^(?:No `## Allowed edit surfaces` heading was found\.|The section has no entries\.|Repeated sections list different surfaces\.|Line ".+" is not a valid surface entry; move prose under a following Markdown heading\.|Line ".+" is not a valid surface entry; if it is a path, wrap the whole entry in backticks, otherwise move it under a following Markdown heading\.|Entry ".+" is not a narrow repository-relative path; remove absolute paths, `\.\.` segments, root globs, and stray backticks\.)$/; // Every rejection is the canonical text, exactly one concrete problem, and the // resend instruction; nothing else (review R3-005). @@ -115,7 +115,12 @@ test("each rejection names its concrete problem, and invalid paths are not calle assert.equal(await problem({ task: `## Allowed edit surfaces\n- ${path}` }), `Entry "${path}" is not a narrow repository-relative path; remove absolute paths, \`..\` segments, root globs, and stray backticks.`, path); } const long = `Then ${"verify every requirement ".repeat(8)}carefully.`; - assert.equal(await problem({ task: `## Allowed edit surfaces\nsrc/a.ts\n${long}` }), `Line "${long.slice(0, 120)}..." is not a valid surface entry; move prose under a following Markdown heading.`); + const ambiguous = (shown: string) => `Line "${shown}" is not a valid surface entry; if it is a path, wrap the whole entry in backticks, otherwise move it under a following Markdown heading.`; + assert.equal(await problem({ task: `## Allowed edit surfaces\nsrc/a.ts\n${long}` }), ambiguous(`${long.slice(0, 120)}...`)); + // A real path with a space must not be steered out of the section. + assert.equal(await problem({ task: "## Allowed edit surfaces\n- docs/with space.md" }), ambiguous("- docs/with space.md")); + // Whitespace that stays invalid even when quoted is plain prose. + assert.equal(await problem({ task: "## Allowed edit surfaces\n- ~ notes for later" }), `Line "- ~ notes for later" is not a valid surface entry; move prose under a following Markdown heading.`); const control = await problem({ task: "## Allowed edit surfaces\nsrc/a\u0007.ts" }); assert.doesNotMatch(control, /\u0007/); assert.match(control, /^Entry "src\/a \.ts" is not a narrow repository-relative path/);