diff --git a/docs/gentle-shell.md b/docs/gentle-shell.md index ae2179fd3..d70957fd8 100644 --- a/docs/gentle-shell.md +++ b/docs/gentle-shell.md @@ -217,5 +217,9 @@ Three things keep the list current, which a static tool description cannot: A finished list stays on screen for the turn it finished in and clears at the next. `ctrl+shift+t` collapses the card to the task in progress (`GENTLE_PI_TODO_KEY` rebinds it, `off` disables it); `GENTLE_PI_TODO=0` disables the tool and the card. +### Bridge providers + +The Gentle AI harness (ODD workflow, identity, review contract) and the open-tasks block are appended to `before_agent_start`'s `systemPromptOptions.appendSystemPrompt` instead of being returned as a replacement `systemPrompt` (gentle-shell#1485). Provider bridges such as `pi-claude-bridge` forward only those structured sections after their own preset and drop a returned `systemPrompt`, so this route reaches every provider, bridged or not. + Set `GENTLE_PI_SHELL=0` to keep pi's built-in footer and editor. diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 25de797bc..dbae1730b 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -1,3 +1,4 @@ +import { appendSystemPromptOnce } from "../lib/append-system-prompt.ts"; import { consumeReviewMutation, pendingReviewMutation, recordReviewMutation } from "../lib/review-reminder-receipt.ts"; import { isOddPhase, oddPhaseRegistry, ODD_PHASES } from "../lib/odd-phase.ts"; import { resolveSessionWorktree } from "../lib/session-worktree-registry.ts"; @@ -1252,6 +1253,7 @@ Organic Driven Development (ODD) is the predefined workflow of this orchestrator 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. 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: when the \`gentle_odd_phase\` tool is available, call \`gentle_odd_phase\` only when the primary session's ODD phase actually changes (\`authorizing\`, \`exploring\`, \`researching\`/\`deciding\`, \`planning\`, \`implementing\`, \`checking\`/\`closing\`), 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\`. Harness principles: @@ -9244,9 +9246,11 @@ function createGentleAiExtensionForTesting( return fragment === null ? "" : `\n\n${fragment}`; })() : ""; - return { - systemPrompt: `${event.systemPrompt}${gentlePrompt}${reviewContractPrompt}`, - }; + // gentle-shell#1485: pi-claude-bridge drops a handler-returned systemPrompt + // and forwards only systemPromptOptions, so the harness is delivered + // through the mutable appendSystemPrompt section instead of a replacement. + appendSystemPromptOnce(event.systemPromptOptions, `${gentlePrompt}${reviewContractPrompt}`); + return undefined; }); // gentle-pi#556 / gentle-ai#4051: with RDD enabled, the agent could finish diff --git a/extensions/gentle-todo.ts b/extensions/gentle-todo.ts index 969fe4cba..6f627072f 100644 --- a/extensions/gentle-todo.ts +++ b/extensions/gentle-todo.ts @@ -1,5 +1,6 @@ import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; import { Text, type Component, type TUI } from "@earendil-works/pi-tui"; +import { appendSystemPromptOnce } from "../lib/append-system-prompt.ts"; import { NativePointerRegion } from "../lib/native-pointer-region.ts"; import { sidebarPart } from "../lib/shell-sidebar.ts"; import { invalidateSidebar } from "../lib/shell-sidebar-layout.ts"; @@ -236,7 +237,10 @@ export default function gentleTodo(pi: ExtensionAPI, env: NodeJS.ProcessEnv = pr } const block = todoPromptBlock(current.state, staleTurns(current.state, current.turn)); if (!block) return undefined; - return { systemPrompt: `${event.systemPrompt}\n\n${block}` }; + // gentle-shell#1485: pi-claude-bridge drops a handler-returned + // systemPrompt, so the open-tasks block goes through appendSystemPrompt. + appendSystemPromptOnce(event.systemPromptOptions, block); + return undefined; }); pi.on("tool_execution_end", (event, ctx) => { diff --git a/lib/append-system-prompt.ts b/lib/append-system-prompt.ts new file mode 100644 index 000000000..0e4f98b78 --- /dev/null +++ b/lib/append-system-prompt.ts @@ -0,0 +1,21 @@ +// gentle-shell#1485: pi-claude-bridge only forwards the structured +// systemPromptOptions parts of before_agent_start, dropping any +// handler-returned replacement systemPrompt. Extensions mutate +// options.appendSystemPrompt instead so the harness reaches every provider. +export interface AppendableSystemPromptOptions { + appendSystemPrompt: string; +} + +// Safe to call more than once with the same options object and the same +// text: a text already present is a no-op, so a handler that runs more than +// once against the same systemPromptOptions never duplicates its own block. +export function appendSystemPromptOnce( + options: AppendableSystemPromptOptions | null | undefined, + text: string, +): void { + const normalized = text.replace(/^\n+/, ""); + if (!options || !normalized) return; + const current = options.appendSystemPrompt ?? ""; + if (current.includes(normalized)) return; + options.appendSystemPrompt = current ? `${current}\n\n${normalized}` : normalized; +} diff --git a/odd/tasks/fix-1485-append-system-prompt.md b/odd/tasks/fix-1485-append-system-prompt.md new file mode 100644 index 000000000..d9cd43eb8 --- /dev/null +++ b/odd/tasks/fix-1485-append-system-prompt.md @@ -0,0 +1,81 @@ +# Fix #1485: deliver the harness through systemPromptOptions + +## Objective + +Make the Gentle Pi harness (ODD workflow, identity, persona, review contract, RDD status, research block) and the Gentle Todo open-tasks block reach the model on every provider, including `pi-claude-bridge`. + +## Problem + +`extensions/gentle-ai.ts` and `extensions/gentle-todo.ts` inject text by returning `{ systemPrompt: event.systemPrompt + ... }` from `before_agent_start`. `pi-claude-bridge` 0.8.0 forwards only the structured `systemPromptOptions` parts (context files, skills, `customPrompt`, `appendSystemPrompt`) after the Claude Code preset, so the returned prompt is silently dropped. Observed: the visible TODO is never updated, ODD phase labels never appear, and RDD consent is relayed as chat text (gentle-shell #1485). + +## Why + +Pi documents this route (`docs/extensions.md:101`: "Prefer changing prompt sections ... Returning `systemPrompt` ... replaces the whole prompt"). After `before_agent_start`, Pi rebuilds the prompt from the mutated `result.systemPromptOptions` (`agent-session.js` `_preparePromptAndToolLoadout`). A probe in #1485 shows text appended to `systemPromptOptions.appendSystemPrompt` reaches Claude Code through the bridge. + +## Scope + +- Move the gentle-ai injection from the returned `systemPrompt` to `event.systemPromptOptions.appendSystemPrompt`, idempotently. +- Same for the gentle-todo open-tasks block. +- Tests and docs. + +## Constraints + +- Behavior for non-bridge providers must stay equivalent: same text, same primary-session scoping, no duplication across turns or handlers. +- Do not break other `before_agent_start` handlers or existing prompt tests. +- Technical artifacts in English. +- Out of scope: the superseded `feat/bridge-instructions` branch (kept, unpublished); the stale `gentle-ai` skill (#1085); the dangling `APPEND_SYSTEM.md` symlink left by a gentle-ai test. + +## Tasks + +- [x] T1 gentle-ai harness through `appendSystemPrompt`, with tests. Route: delegated (writer; 4+ files to understand). Shared idempotent helper `lib/append-system-prompt.ts`. `tests/telemetry-trigger.test.ts` and `tests/runtime-harness.mjs` asserted the removed return shape and were updated to the new contract (scope extended by the parent; required consequence, not new behavior). Commit `13d6a3d24`. +- [x] T2 gentle-todo block through `appendSystemPrompt`, with tests, plus a cross-extension ordering test. Route: delegated (same writer). Commit `c1589327c`. +- [x] T3 Docs (`docs/gentle-shell.md`; `docs/review-integration.md` is a byte-pinned contract artifact and was restored after CI `verify` caught the drift). Route: delegated (same writer). Commit: the docs commit that carries this document update. +- [x] T4 Live verification under `claude-bridge`. Route: inline (parent). See evidence. + +## Acceptance criteria + +- The harness and the todo block appear in `systemPromptOptions.appendSystemPrompt` for primary sessions and reach a `claude-bridge` model. +- Non-bridge providers still receive the same harness exactly once per turn. +- No handler returns a whole replacement `systemPrompt` for this purpose any more. + +## Checks + +- Focused tests, unit stage of `node scripts/run-test-suite.mjs`, `node scripts/check-provider-contract.mjs`, `node --experimental-strip-types tests/runtime-harness.mjs`, `node scripts/check-types.mjs`. + +## Delivery + +- Strategy: `ask-on-risk`. Forecast: about 200–400 authored changed lines. +- RDD: on (global). +- PR: `Closes #1485`, `type:bug`. + +## Progress + +- Worktree: `../gentle-pi-worktrees/1485-append-system-prompt`, branch `fix/1485-bridge-append-system-prompt` from `origin/main` (cedc69e08). + +## Runner semantics (confirmed by the writer) + +- `dist/core/extensions/runner.js` `emitBeforeAgentStart` builds one normalized `systemPromptOptions` per emission and passes the same object to every handler in registration order, so later handlers see earlier appends. +- `dist/core/agent-session.js` emits from the stable `_baseSystemPromptOptions`, so appends never accumulate across turns. +- `dist/core/system-prompt.js` renders `appendSystemPrompt` as the final `addendum` section. +- No other package handler returns a replacement `systemPrompt`. + +## Verification evidence + +- `node --experimental-strip-types --test tests/*.test.ts`: 3876 pass, 0 fail, 43 skipped (pre-existing Windows-native skips). +- `node --experimental-strip-types tests/runtime-harness.mjs`: exit 0 (writer and parent). +- Focused files (helper, route, review contract prompt, todo, telemetry): 42 pass, 0 fail (parent re-run). +- `node scripts/check-provider-contract.mjs`: pass. `node scripts/check-types.mjs`: no regressions. +- Live, `gentle-shell -p --no-session --model claude-bridge/claude-opus-5-5`, asked whether the instructions contain "Default workflow: Organic Driven Development": this branch answered yes; the installed package answered no. With `openai-codex/gpt-5.5` on this branch, the phrase is present exactly once. + +## Review + +- `cedc69e08..c85dc1362`: medium, 483 lines; lineage `review-c6da780bb242bf6e`, one lens (reliability), approved and acknowledged. Advisory findings: non-bridge acceptance not proven live (addressed afterwards by the Codex probe), tautological ordering assertion and overclaiming route test, silent no-op when `systemPromptOptions` is missing, substring dedupe, weak todo idempotency assertion. + +## Follow-ups + +- Done in this PR (commit `1a5cbc094`, user-approved scope addition): the `gentle_odd_phase` reporting instruction lived only in `assets/orchestrator-delegation.md`; one "Phase reporting" line now follows step 7 in the harness, covered by `tests/odd-routing-contract.test.ts` (RED then GREEN). Live under `claude-bridge`, the model now states when to call `gentle_odd_phase`. +- Superseded `feat/bridge-instructions` branch: deleted (never published). + +## Next step + +Pull request with `Closes #1485`; merge is the user's decision. diff --git a/tests/append-system-prompt-route.test.ts b/tests/append-system-prompt-route.test.ts new file mode 100644 index 000000000..61cde9d4b --- /dev/null +++ b/tests/append-system-prompt-route.test.ts @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { createGentleAiExtension } from "../extensions/gentle-ai.ts"; +import gentleTodo from "../extensions/gentle-todo.ts"; + +// gentle-shell#1485: pi-claude-bridge forwards only the structured +// systemPromptOptions parts of before_agent_start (contextFiles, skills, +// customPrompt, appendSystemPrompt) after Claude Code's own preset, so a +// handler-returned replacement systemPrompt never reaches the model on that +// provider. These tests exercise both extensions against one shared +// systemPromptOptions object, the same object every before_agent_start +// handler observes within a single pi emission (packages/coding-agent +// extensions/runner.js emitBeforeAgentStart: one `currentOptions` per call). + +type Handler = (event: unknown, ctx: ExtensionContext) => unknown; + +function gentleAiHandlers(): Map { + const handlers = new Map(); + const pi = { + on(name: string, handler: Handler) { + handlers.set(name, handler); + }, + events: { emit() {} }, + registerCommand() {}, + registerTool() {}, + } as unknown as ExtensionAPI; + createGentleAiExtension({ nativeReviewCli: null })(pi); + return handlers; +} + +function gentleTodoHandlers(): { handlers: Map; tools: Map Promise }> } { + const handlers = new Map(); + const tools = new Map Promise; name: string }>(); + const pi = { + on(event: string, handler: Handler) { + handlers.set(event, [...(handlers.get(event) ?? []), handler]); + }, + registerTool(tool: { execute: (...args: unknown[]) => Promise; name: string }) { + tools.set(tool.name, tool); + }, + registerShortcut() {}, + } as unknown as ExtensionAPI; + gentleTodo(pi, {}); + return { handlers, tools }; +} + +function ctx(): ExtensionContext { + return { + cwd: process.cwd(), + hasUI: true, + ui: { notify() {}, setWidget() {} }, + sessionManager: { getSessionId: () => "append-route-session", getBranch: () => [] }, + } as unknown as ExtensionContext; +} + +test("both extensions land their block in appendSystemPrompt on one shared options object, and neither returns a replacement systemPrompt", async () => { + const aiHandlers = gentleAiHandlers(); + const { handlers: todoHandlers, tools } = gentleTodoHandlers(); + const session = ctx(); + + for (const handler of todoHandlers.get("session_start") ?? []) await handler({}, session); + await tools.get("todo")!.execute("c1", { action: "write", tasks: [{ title: "Fix the bug" }] }, undefined, undefined, session); + for (const handler of todoHandlers.get("tool_execution_end") ?? []) await handler({ toolName: "todo" }, session); + + const event = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + + // Every handler registered for before_agent_start observes the same + // systemPromptOptions object within one emission, exactly as pi's runner + // does for the real event. + const aiResult = await aiHandlers.get("before_agent_start")!(event, session); + let todoResult: unknown; + for (const handler of todoHandlers.get("before_agent_start") ?? []) todoResult = await handler(event, session); + + assert.equal(aiResult, undefined, "gentle-ai must not return a replacement systemPrompt"); + assert.equal(todoResult, undefined, "gentle-todo must not return a replacement systemPrompt"); + + const appended = event.systemPromptOptions.appendSystemPrompt; + assert.match(appended, /el Gentleman Identity and Harness/); + assert.match(appended, /## Todo list/); + assert.match(appended, /1\. \[pending\] Fix the bug/); + assert.ok( + appended.indexOf("el Gentleman Identity and Harness") < appended.indexOf("## Todo list"), + "gentle-ai's block must precede gentle-todo's, matching handler registration order", + ); +}); + +test("re-running both handlers on the same already-populated options object does not duplicate either block", async () => { + const aiHandlers = gentleAiHandlers(); + const { handlers: todoHandlers, tools } = gentleTodoHandlers(); + const session = ctx(); + + for (const handler of todoHandlers.get("session_start") ?? []) await handler({}, session); + await tools.get("todo")!.execute("c1", { action: "write", tasks: [{ title: "Fix the bug" }] }, undefined, undefined, session); + for (const handler of todoHandlers.get("tool_execution_end") ?? []) await handler({ toolName: "todo" }, session); + + const event = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + await aiHandlers.get("before_agent_start")!(event, session); + const gentleAiOccurrencesAfterFirstRun = event.systemPromptOptions.appendSystemPrompt.split("el Gentleman Identity and Harness").length - 1; + assert.equal(gentleAiOccurrencesAfterFirstRun, 1); + + // A defensive re-run of gentle-ai's own handler against an options object + // that already carries its exact block must not append it again. + await aiHandlers.get("before_agent_start")!(event, session); + const gentleAiOccurrencesAfterSecondRun = event.systemPromptOptions.appendSystemPrompt.split("el Gentleman Identity and Harness").length - 1; + assert.equal(gentleAiOccurrencesAfterSecondRun, 1, "a second gentle-ai run on the same options object must not duplicate the harness"); +}); diff --git a/tests/append-system-prompt.test.ts b/tests/append-system-prompt.test.ts new file mode 100644 index 000000000..70798b9f8 --- /dev/null +++ b/tests/append-system-prompt.test.ts @@ -0,0 +1,46 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { appendSystemPromptOnce } from "../lib/append-system-prompt.ts"; + +// gentle-shell#1485: pi-claude-bridge forwards only the structured +// systemPromptOptions parts of before_agent_start, so extensions must mutate +// options.appendSystemPrompt instead of returning a replacement systemPrompt. + +test("appends to an empty appendSystemPrompt, stripped of a leading blank line", () => { + const options = { appendSystemPrompt: "" }; + appendSystemPromptOnce(options, "\n\nHarness block"); + assert.equal(options.appendSystemPrompt, "Harness block"); +}); + +test("appends after existing content with a blank-line separator", () => { + const options = { appendSystemPrompt: "user APPEND_SYSTEM.md content" }; + appendSystemPromptOnce(options, "\n\nHarness block"); + assert.equal(options.appendSystemPrompt, "user APPEND_SYSTEM.md content\n\nHarness block"); +}); + +test("is idempotent: the same text is never appended twice to the same options object", () => { + const options = { appendSystemPrompt: "" }; + appendSystemPromptOnce(options, "\n\nHarness block"); + appendSystemPromptOnce(options, "\n\nHarness block"); + assert.equal(options.appendSystemPrompt, "Harness block"); + assert.equal(options.appendSystemPrompt.split("Harness block").length - 1, 1); +}); + +test("two different callers accumulate without erasing each other", () => { + const options = { appendSystemPrompt: "" }; + appendSystemPromptOnce(options, "\n\nGentle AI harness"); + appendSystemPromptOnce(options, "\n\nTodo list block"); + assert.equal(options.appendSystemPrompt, "Gentle AI harness\n\nTodo list block"); +}); + +test("empty or blank text is a no-op", () => { + const options = { appendSystemPrompt: "kept" }; + appendSystemPromptOnce(options, ""); + appendSystemPromptOnce(options, "\n\n"); + assert.equal(options.appendSystemPrompt, "kept"); +}); + +test("a missing options object never throws", () => { + assert.doesNotThrow(() => appendSystemPromptOnce(undefined, "\n\nHarness block")); + assert.doesNotThrow(() => appendSystemPromptOnce(null, "\n\nHarness block")); +}); diff --git a/tests/gentle-todo.test.ts b/tests/gentle-todo.test.ts index fb6d044f4..01974ca59 100644 --- a/tests/gentle-todo.test.ts +++ b/tests/gentle-todo.test.ts @@ -160,22 +160,29 @@ test("the Todo header paints the shared hover role while hovered, and clears it assert.deepEqual(component.handleMouse?.(move(0)), { handled: true }); }); -test("every turn carries the open tasks in the system prompt and the card goes stale after two silent turns", async () => { +function promptEvent(): { systemPrompt: string; systemPromptOptions: { appendSystemPrompt: string } } { + return { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; +} + +test("every turn carries the open tasks in appendSystemPrompt (never a returned systemPrompt) and the card goes stale after two silent turns", async () => { const { pi, tools, fire } = fakePi(); gentleTodo(pi, {}); const { ctx, widget } = fakeContext(); await fire("session_start", ctx); - await fire("before_agent_start", ctx, { systemPrompt: "base" }); + await fire("before_agent_start", ctx, promptEvent()); await tools.get("todo")!.execute("c1", { action: "write", tasks: [{ title: "Fix the bug" }] }, undefined, undefined, ctx); await fire("tool_execution_end", ctx, { toolName: "todo" }); - const withTasks = (await fire("before_agent_start", ctx, { systemPrompt: "base" })) as { systemPrompt: string }; - assert.match(withTasks.systemPrompt, /^base\n\n## Todo list/); - assert.match(withTasks.systemPrompt, /1\. \[pending\] Fix the bug/); + const withTasksEvent = promptEvent(); + const withTasksResult = await fire("before_agent_start", ctx, withTasksEvent); + assert.equal(withTasksResult, undefined, "the handler must not return a replacement systemPrompt"); + assert.match(withTasksEvent.systemPromptOptions.appendSystemPrompt, /^## Todo list/); + assert.match(withTasksEvent.systemPromptOptions.appendSystemPrompt, /1\. \[pending\] Fix the bug/); assert.doesNotMatch(widget()![0], /stale/); - const stale = (await fire("before_agent_start", ctx, { systemPrompt: "base" })) as { systemPrompt: string }; - assert.match(stale.systemPrompt, /stale: 2 turns without an update/); + const staleEvent = promptEvent(); + await fire("before_agent_start", ctx, staleEvent); + assert.match(staleEvent.systemPromptOptions.appendSystemPrompt, /stale: 2 turns without an update/); assert.match(widget()![0], /ctrl\+shift\+t collapse/); assert.match(widget()![1], /stale · 2 turns/); @@ -184,6 +191,32 @@ test("every turn carries the open tasks in the system prompt and the card goes s assert.doesNotMatch(widget()![0], /stale/); }); +test("before_agent_start is idempotent: a todo block already present in appendSystemPrompt is not duplicated", async () => { + const { pi, tools, fire } = fakePi(); + gentleTodo(pi, {}); + + // Capture the exact block a fresh turn produces (no staleness yet). + const probe = fakeContext(); + await fire("session_start", probe.ctx); + await tools.get("todo")!.execute("c1", { action: "write", tasks: [{ title: "Fix the bug" }] }, undefined, undefined, probe.ctx); + await fire("tool_execution_end", probe.ctx, { toolName: "todo" }); + const probeEvent = promptEvent(); + await fire("before_agent_start", probe.ctx, probeEvent); + const block = probeEvent.systemPromptOptions.appendSystemPrompt; + assert.match(block, /^## Todo list/); + + // A fresh session reaching the identical block, but whose options object + // already carries that exact text (e.g. a retried emission), must not + // duplicate it. + const { ctx } = fakeContext(); + await fire("session_start", ctx); + await tools.get("todo")!.execute("c1", { action: "write", tasks: [{ title: "Fix the bug" }] }, undefined, undefined, ctx); + await fire("tool_execution_end", ctx, { toolName: "todo" }); + const seededEvent = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: block } }; + await fire("before_agent_start", ctx, seededEvent); + assert.equal(seededEvent.systemPromptOptions.appendSystemPrompt, block, "a block already present must not be appended again"); +}); + test("a finished list stays for its turn and clears at the next, and the collapse key folds the card", async () => { const { pi, tools, shortcuts, fire } = fakePi(); gentleTodo(pi, {}); diff --git a/tests/odd-routing-contract.test.ts b/tests/odd-routing-contract.test.ts index 21e16a2f7..3b50b1e6d 100644 --- a/tests/odd-routing-contract.test.ts +++ b/tests/odd-routing-contract.test.ts @@ -277,6 +277,7 @@ test("ODD protocol is always-on in the rendered system prompt and runs by defaul "Tell the user in one line which feature document was created and how many tasks it holds", "6. **Implement task by task.**", "7. **Close.**", + "call `gentle_odd_phase` only when the primary session's ODD phase actually changes", "Harness principles:", "# el Gentleman Orchestrator", ]; diff --git a/tests/review-contract-prompt.test.ts b/tests/review-contract-prompt.test.ts index 5a4c25552..f6c75c9a3 100644 --- a/tests/review-contract-prompt.test.ts +++ b/tests/review-contract-prompt.test.ts @@ -16,8 +16,9 @@ import type { NativeReviewCli } from "../lib/native-review-cli.ts"; // These tests exercise the real committed mirror (contracts/review-provider-contract-mirror/) // rather than a fake one: the mirror IS the package under test. -type BeforeAgentStartResult = { systemPrompt: string }; +type BeforeAgentStartResult = undefined; type BeforeAgentStartHandler = (event: unknown, ctx: ExtensionContext) => Promise; +type MutableEvent = { agentName?: string; systemPrompt: string; systemPromptOptions: { appendSystemPrompt: string } }; const REPO_ROOT = join(import.meta.dirname, ".."); const MIRROR_LOCK_PATH = join(REPO_ROOT, "contracts", "review-provider-contract-mirror", "provider-contract.lock.json"); @@ -56,30 +57,35 @@ function ctx(overrides: Record = {}): ExtensionContext { } as unknown as ExtensionContext; } -const primaryEvent = { systemPrompt: "base" }; +function primaryEvent(overrides: Partial = {}): MutableEvent { + return { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" }, ...overrides }; +} -test("before_agent_start injects the mirrored review execution contract for the primary session", async () => { +test("before_agent_start injects the mirrored review execution contract for the primary session through appendSystemPrompt, never a returned systemPrompt", async () => { const { beforeAgentStart } = harness({} as NativeReviewCli); - const result = await beforeAgentStart(primaryEvent, ctx()); + const event = primaryEvent(); + const result = await beforeAgentStart(event, ctx()); + assert.equal(result, undefined, "the handler must not return a replacement systemPrompt"); + const appended = event.systemPromptOptions.appendSystemPrompt; const expected = mirroredPiOrchestrationText(); - assert.match(result.systemPrompt, /Substantial authorized work: use ODD/); - assert.match(result.systemPrompt, /For behavior changes with applicable runnable deterministic tests and a clear expected outcome, use test-first by default: observe RED, GREEN, then refactor with focused checks/); - assert.match(result.systemPrompt, /Test presence alone does not establish applicability; no chat or TUI toggle activates it/); - assert.match(result.systemPrompt, /no meaningful RED, explain why and run proportionate ordinary functional or structural verification/); - assert.match(result.systemPrompt, /Never invent lifecycle evidence or skip checks/); - assert.doesNotMatch(result.systemPrompt, /If tests exist, use strict TDD/); - assert.match(result.systemPrompt, /ODD \(Default Workflow, harness section above\) is mandatory on every request/); - assert.doesNotMatch(result.systemPrompt, /Prefer SDD\/OpenSpec artifacts/); - assert.match(result.systemPrompt, /## Gentle AI review execution contract \(mirrored provider bundle 1\.2\.0\)/); - assert.ok(result.systemPrompt.includes(expected), "the mirrored orchestration/pi.md text must appear verbatim"); - assert.match(result.systemPrompt, /call `gentle_review` with {"operation":"inspect"}/); - assert.match(result.systemPrompt, /call `gentle_review` with operation `status`, the exact retained `lineageId`, and `workspaceRoot`/); - assert.match(result.systemPrompt, /Use `gentle_review_capture` for one current returned slot or `gentle_review_capture_group` for the complete current reviewer group/); - assert.match(result.systemPrompt, /An eligible interactive Pi host may resolve `gentle-ai\.review-integration\.consent\/v3` before the envelope reaches the model/); - assert.match(result.systemPrompt, /If `gentle_review` returns the envelope unresolved, it is still the original provider-owned two-choice contract/); - assert.match(result.systemPrompt, /Never add the host action to a decoded or relayed provider envelope/); - assert.match(result.systemPrompt, /An approved capture awaits acknowledgement; it is not burned\. On `approved`, use bound facade STATUS to obtain or replay the exact provider-issued `acknowledge-approved` continuation, then execute it unchanged\. Only its successful returned envelope burns authority; do not issue STATUS after that burn\./); - let previousLifecycleIndex = result.systemPrompt.indexOf("## Gentle AI review execution contract"); + assert.match(appended, /Substantial authorized work: use ODD/); + assert.match(appended, /For behavior changes with applicable runnable deterministic tests and a clear expected outcome, use test-first by default: observe RED, GREEN, then refactor with focused checks/); + assert.match(appended, /Test presence alone does not establish applicability; no chat or TUI toggle activates it/); + assert.match(appended, /no meaningful RED, explain why and run proportionate ordinary functional or structural verification/); + assert.match(appended, /Never invent lifecycle evidence or skip checks/); + assert.doesNotMatch(appended, /If tests exist, use strict TDD/); + assert.match(appended, /ODD \(Default Workflow, harness section above\) is mandatory on every request/); + assert.doesNotMatch(appended, /Prefer SDD\/OpenSpec artifacts/); + assert.match(appended, /## Gentle AI review execution contract \(mirrored provider bundle 1\.2\.0\)/); + assert.ok(appended.includes(expected), "the mirrored orchestration/pi.md text must appear verbatim"); + assert.match(appended, /call `gentle_review` with {"operation":"inspect"}/); + assert.match(appended, /call `gentle_review` with operation `status`, the exact retained `lineageId`, and `workspaceRoot`/); + assert.match(appended, /Use `gentle_review_capture` for one current returned slot or `gentle_review_capture_group` for the complete current reviewer group/); + assert.match(appended, /An eligible interactive Pi host may resolve `gentle-ai\.review-integration\.consent\/v3` before the envelope reaches the model/); + assert.match(appended, /If `gentle_review` returns the envelope unresolved, it is still the original provider-owned two-choice contract/); + assert.match(appended, /Never add the host action to a decoded or relayed provider envelope/); + assert.match(appended, /An approved capture awaits acknowledgement; it is not burned\. On `approved`, use bound facade STATUS to obtain or replay the exact provider-issued `acknowledge-approved` continuation, then execute it unchanged\. Only its successful returned envelope burns authority; do not issue STATUS after that burn\./); + let previousLifecycleIndex = appended.indexOf("## Gentle AI review execution contract"); for (const marker of [ 'call `gentle_review` with {"operation":"inspect"}', "2. **Freeze once.**", @@ -87,48 +93,61 @@ test("before_agent_start injects the mirrored review execution contract for the "Use `gentle_review_capture` for one current returned slot", "5. **Acknowledge exactly.**", ]) { - const markerIndex = result.systemPrompt.indexOf(marker, previousLifecycleIndex + 1); + const markerIndex = appended.indexOf(marker, previousLifecycleIndex + 1); assert.ok(markerIndex > previousLifecycleIndex, `${marker} must follow the previous lifecycle step`); previousLifecycleIndex = markerIndex; } - assert.doesNotMatch(result.systemPrompt, /authority is already burned/); - assert.doesNotMatch(result.systemPrompt, /gentle-ai review status\b.*--agent pi/); + assert.doesNotMatch(appended, /authority is already burned/); + assert.doesNotMatch(appended, /gentle-ai review status\b.*--agent pi/); }); test("before_agent_start does not inject the review execution contract for a named agent session", async () => { const { beforeAgentStart } = harness({} as NativeReviewCli); - const result = await beforeAgentStart({ agentName: "review-readability", systemPrompt: "base" }, ctx()); - assert.doesNotMatch(result.systemPrompt, /Gentle AI review execution contract/); + const event = primaryEvent({ agentName: "review-readability" }); + await beforeAgentStart(event, ctx()); + assert.doesNotMatch(event.systemPromptOptions.appendSystemPrompt, /Gentle AI review execution contract/); }); test("before_agent_start does not inject the review execution contract for gentle-ai-worker", async () => { const { beforeAgentStart } = harness({} as NativeReviewCli); - const result = await beforeAgentStart({ agentName: "gentle-ai-worker", systemPrompt: "base" }, ctx()); - assert.equal(result.systemPrompt, "base"); - assert.doesNotMatch(result.systemPrompt, /Substantial authorized work: use ODD/); - assert.doesNotMatch(result.systemPrompt, /Gentle AI review execution contract/); + const event = primaryEvent({ agentName: "gentle-ai-worker" }); + await beforeAgentStart(event, ctx()); + assert.equal(event.systemPromptOptions.appendSystemPrompt, "", "a named agent gets nothing appended"); }); test("before_agent_start does not inject the review execution contract for jd-fix-agent", async () => { const { beforeAgentStart } = harness({} as NativeReviewCli); - const result = await beforeAgentStart({ agentName: "jd-fix-agent", systemPrompt: "base" }, ctx()); - assert.equal(result.systemPrompt, "base"); - assert.doesNotMatch(result.systemPrompt, /Substantial authorized work: use ODD/); - assert.doesNotMatch(result.systemPrompt, /Gentle AI review execution contract/); + const event = primaryEvent({ agentName: "jd-fix-agent" }); + await beforeAgentStart(event, ctx()); + assert.equal(event.systemPromptOptions.appendSystemPrompt, "", "a named agent gets nothing appended"); }); test("before_agent_start does not let legacy prompt text bypass primary ODD and review injection", async () => { const { beforeAgentStart } = harness({} as NativeReviewCli); - const result = await beforeAgentStart({ systemPrompt: "SDD apply executor body" }, ctx()); - assert.match(result.systemPrompt, /Substantial authorized work: use ODD/); - assert.match(result.systemPrompt, /Gentle AI review execution contract/); - assert.doesNotMatch(result.systemPrompt, /### 3\. SDD \(optional\)/); + const event = primaryEvent({ systemPrompt: "SDD apply executor body" }); + await beforeAgentStart(event, ctx()); + const appended = event.systemPromptOptions.appendSystemPrompt; + assert.match(appended, /Substantial authorized work: use ODD/); + assert.match(appended, /Gentle AI review execution contract/); + assert.doesNotMatch(appended, /### 3\. SDD \(optional\)/); }); test("before_agent_start injects nothing when nativeReviewCli is null", async () => { const { beforeAgentStart } = harness(null); - const result = await beforeAgentStart(primaryEvent, ctx()); - assert.doesNotMatch(result.systemPrompt, /Gentle AI review execution contract/); + const event = primaryEvent(); + await beforeAgentStart(event, ctx()); + assert.doesNotMatch(event.systemPromptOptions.appendSystemPrompt, /Gentle AI review execution contract/); +}); + +test("before_agent_start is idempotent: running twice on the same systemPromptOptions never duplicates the harness", async () => { + const { beforeAgentStart } = harness({} as NativeReviewCli); + const event = primaryEvent(); + await beforeAgentStart(event, ctx()); + const firstLength = event.systemPromptOptions.appendSystemPrompt.length; + await beforeAgentStart(event, ctx()); + assert.equal(event.systemPromptOptions.appendSystemPrompt.length, firstLength, "a second run on the same options object must not append again"); + const occurrences = event.systemPromptOptions.appendSystemPrompt.split("Gentle AI review execution contract").length - 1; + assert.equal(occurrences, 1); }); // gentle-ai R1/R3: a tampered mirrored orchestration/pi.md must never be spliced into the system prompt. diff --git a/tests/runtime-harness.mjs b/tests/runtime-harness.mjs index 37a0ea714..93ebacca2 100644 --- a/tests/runtime-harness.mjs +++ b/tests/runtime-harness.mjs @@ -394,23 +394,29 @@ async function run() { const promptCwd = await tempWorkspace(); try { const promptHook = hooks.get("before_agent_start")[0]; - const promptResult = await promptHook({ systemPrompt: "base" }, createCtx(promptCwd)); - assert.match(promptResult.systemPrompt, /base/); - assert.match(promptResult.systemPrompt, /el Gentleman/); - assert.match(promptResult.systemPrompt, /Organic Driven Development/); - assert.doesNotMatch(promptResult.systemPrompt, /## SDD Research Capabilities/); - assert.match(promptResult.systemPrompt, /review execution contract/); + // gentle-shell#1485: pi-claude-bridge drops a handler-returned systemPrompt + // and forwards only structured systemPromptOptions, so the harness must + // land in appendSystemPrompt and the hook must never return a replacement. + const promptEvent = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + const promptResult = await promptHook(promptEvent, createCtx(promptCwd)); + assert.equal(promptResult, undefined, "before_agent_start must not return a replacement systemPrompt"); + assert.equal(promptEvent.systemPrompt, "base", "the original systemPrompt field must be left untouched"); + const promptAppended = promptEvent.systemPromptOptions.appendSystemPrompt; + assert.match(promptAppended, /el Gentleman/); + assert.match(promptAppended, /Organic Driven Development/); + assert.doesNotMatch(promptAppended, /## SDD Research Capabilities/); + assert.match(promptAppended, /review execution contract/); assert.doesNotMatch(await readFile(join(ROOT, "extensions", "gentle-ai.ts"), "utf8"), /readCommandSddStatus/); - assert.match(promptResult.systemPrompt + delegationDetail, /do not pass the `model` parameter by default/); - assert.doesNotMatch(promptResult.systemPrompt, /Every Agent tool call MUST include `model`/); + assert.match(promptAppended + delegationDetail, /do not pass the `model` parameter by default/); + assert.doesNotMatch(promptAppended, /Every Agent tool call MUST include `model`/); assert.ok( - promptResult.systemPrompt.includes( + promptAppended.includes( `Package assets root: \`${join(ROOT, "assets")}\`. Lazy asset paths below are relative to this root.`, ), "parent prompt must declare the one absolute root for relative lazy asset paths", ); assert.doesNotMatch( - promptResult.systemPrompt, + promptAppended, new RegExp(ambientTestAssetsDir.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")), "normal runtime must ignore ambient GENTLE_PI_TEST_ASSETS_DIR", ); @@ -420,26 +426,30 @@ async function run() { join(globalConfigHome, "persona.json"), '{"mode":"neutral"}\n', ); - const neutralPromptResult = await promptHook({ systemPrompt: "base" }, createCtx(promptCwd)); - assert.match(neutralPromptResult.systemPrompt, /Do not use slang or regional expressions/); + const neutralPromptEvent = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + const neutralPromptResult = await promptHook(neutralPromptEvent, createCtx(promptCwd)); + assert.equal(neutralPromptResult, undefined, "before_agent_start must not return a replacement systemPrompt"); + const neutralAppended = neutralPromptEvent.systemPromptOptions.appendSystemPrompt; + assert.match(neutralAppended, /Do not use slang or regional expressions/); assert.doesNotMatch( - neutralPromptResult.systemPrompt, + neutralAppended, /When the user writes Spanish, answer in natural Rioplatense Spanish with voseo/, "neutral persona prompt must not include unconditional voseo instructions after reload", ); - const subagentPromptResult = await promptHook( - { agentName: "worker", systemPrompt: "worker base" }, - createCtx(promptCwd), - ); - assert.equal(subagentPromptResult.systemPrompt, "worker base"); + const subagentEvent = { agentName: "worker", systemPrompt: "worker base", systemPromptOptions: { appendSystemPrompt: "" } }; + const subagentPromptResult = await promptHook(subagentEvent, createCtx(promptCwd)); + assert.equal(subagentPromptResult, undefined, "before_agent_start must not return a replacement systemPrompt"); + assert.equal(subagentEvent.systemPromptOptions.appendSystemPrompt, "", "a named agent gets nothing appended"); await mkdir(join(promptCwd, ".pi", "gentle-ai"), { recursive: true }); await writeFile( join(promptCwd, ".pi", "gentle-ai", "persona.json"), '{"mode":"gentleman"}\n', ); - const localOverridePromptResult = await promptHook({ systemPrompt: "base" }, createCtx(promptCwd)); + const localOverrideEvent = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + const localOverridePromptResult = await promptHook(localOverrideEvent, createCtx(promptCwd)); + assert.equal(localOverridePromptResult, undefined, "before_agent_start must not return a replacement systemPrompt"); assert.match( - localOverridePromptResult.systemPrompt, + localOverrideEvent.systemPromptOptions.appendSystemPrompt, /When the user writes Spanish, answer in natural Rioplatense Spanish with voseo/, ); const personaCtx = createCtx(promptCwd, true); diff --git a/tests/telemetry-trigger.test.ts b/tests/telemetry-trigger.test.ts index a259907a6..649008749 100644 --- a/tests/telemetry-trigger.test.ts +++ b/tests/telemetry-trigger.test.ts @@ -247,10 +247,13 @@ test("activation: a missing binary or spawn error never affects activation", asy const notifications: Array<{ message: string; severity: string }> = []; const ctx = fakeContext("/work/project", notifications); - // Must resolve cleanly and produce the ordinary orchestrator prompt fields, - // never throw or notify about the missing binary. - const outcome = await beforeAgentStart!({ systemPrompt: "base" }, ctx); - assert.equal(typeof outcome, "object"); + // Must resolve cleanly and produce the ordinary orchestrator prompt fields + // in appendSystemPrompt (never a returned systemPrompt), and never throw + // or notify about the missing binary. + const event = { systemPrompt: "base", systemPromptOptions: { appendSystemPrompt: "" } }; + const outcome = await beforeAgentStart!(event, ctx); + assert.equal(outcome, undefined, "the handler must not return a replacement systemPrompt"); + assert.match(event.systemPromptOptions.appendSystemPrompt, /el Gentleman/); assert.equal(notifications.length, 0); });