diff --git a/src/AGENTS.md b/src/AGENTS.md index 7575324..3debb99 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -36,7 +36,7 @@ Source domain for the Pi extension implementation. Route to narrower AGENTS.md f ### Domain Details - `index.ts` → Pi extension entry point; registers commands, tools, and events, and wires `guard/tool-policy.ts` in front of every tool call before it reaches `runtime/`. -- `index.tool-visibility.ts` → computes which tools are visible/hidden to the model for the current state (separate from whether a tool call is *allowed*, which `guard/tool-policy.ts` decides); read by `index.ts` when building the tool list for a turn. +- `index.tool-visibility.ts` → computes which tools are visible/hidden to the model for the current state (separate from whether a tool call is *allowed*, which `guard/tool-policy.ts` decides); read by `index.ts` when building the tool list for a turn. **It states only the planner's own invariants — never the whole world.** `pi.setActiveTools` is a global setter with no notion of whose tools are whose, and other extensions write it too; rebuilding the list from `getAllTools()` resurrected everything they had just hidden and rewrote the head of the prompt mid-turn, discarding the prefix cache. `computePlannerActiveTools` is therefore a subtraction — `(active now ∪ what we claim) \ what we hide`, walked in registry order because order is part of those bytes — and it converges, so a stable decision means a stable head. The contract gate is the one exception and is a *sandbox*: project reads must be unreachable, so the world is remembered on the way in and restored on the way out. `plannerLastSetToolNames()` exposes our own footprint for `runtime/prefix-watch.ts`. - `constants.ts` → two package-wide constants, `EXTENSION_NAME` and `SCHEMA_VERSION`; bump `SCHEMA_VERSION` only alongside a real persisted-schema migration in `storage/schema.ts`. - `public-api.ts` → barrel re-export of the full public surface (types + functions) across every domain — git, guard, instructions, project-local, runtime, session, settings, storage, worktree. Adding an export here without a corresponding domain change is a smell; every new domain export usually needs an entry added here too. - Most extension work starts in `src/index.ts`, then moves to a runtime/storage helper once behavior needs tests. diff --git a/src/guard/tool-matrix.test.ts b/src/guard/tool-matrix.test.ts new file mode 100644 index 0000000..7ca5a3f --- /dev/null +++ b/src/guard/tool-matrix.test.ts @@ -0,0 +1,632 @@ +import { describe, expect, it } from "vitest"; +import { PLANNER_STAGE_STEPS } from "../storage/schema"; +import { + ALL_PLANNER_TOOL_NAMES, + getAllowedPlannerWrapperTools, + PLANNER_LIFECYCLE_TRANSITION_TOOLS, + PLANNER_WRAPPER_TOOLS, + type PlannerWrapperTool, +} from "./tool-policy"; + +/** + * The tool matrix, pinned. + * + * `getAllowedPlannerWrapperTools` is the call-time gate: it decides which + * semantic wrapper tools the model may use at a given planner position. Two + * separate things depend on it being exactly right, and both fail quietly: + * + * - **Reachability.** A tool dropped from a step's list is not an error the + * model can report — it just gets refused and has no way forward. The step + * deadlocks and the only symptom is a stuck run. + * - **Prompt shape.** If the active tool list is ever narrowed to this set (so + * the schemas leave the request), then this matrix *is* the head of the + * prompt, and a change here re-reads the whole prefix on every backend that + * caches one. Order is part of the bytes, so the expectations below pin the + * order too, not just the membership. + * + * So the matrix is written out in full rather than recomputed: a test that + * derives the answer from the same table it checks proves nothing. Regenerate + * by hand when a step's tools genuinely change, and read the diff — every line + * that moves is a tool the model gains or loses. + * + * The *visibility* layer (plan active, contract gate, recovery-report unlock) + * is a different mechanism and is covered in `index.tool-visibility.test.ts`. + */ + +const STAGE_STEP_MATRIX: Record< + string, + Record +> = { + init: { + check_project: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + ], + check_git: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_git_init", + ], + prepare_storage: ["planner_status", "planner_artifact_read"], + choose_worktree_location: ["planner_status", "planner_artifact_read"], + create_plan_record: ["planner_status", "planner_artifact_read"], + create_plan_worktree: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + ], + enter_intake: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + ], + }, + intake: { + draft_goal: [ + "planner_status", + "planner_artifact_read", + "planner_goal_submit", + ], + await_goal_approval: [ + "planner_status", + "planner_artifact_read", + "planner_goal_decide", + ], + }, + discovery: { + scan_project_structure: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_discovery_submit", + "planner_contract_scan", + "planner_contract_route", + "planner_contract_read", + "planner_contract_upsert", + "planner_elenchus_check", + "planner_reason", + "planner_git_commit", + "planner_exec", + ], + write_questions: [ + "planner_status", + "planner_artifact_read", + "planner_questions_submit", + "planner_questions_resolve", + "planner_contract_scan", + "planner_contract_route", + "planner_contract_read", + "planner_exec", + ], + enter_planning: ["planner_status", "planner_artifact_read"], + }, + spec: { + draft_requirements: [ + "planner_status", + "planner_artifact_read", + "planner_spec_submit", + "planner_contract_route", + "planner_contract_read", + ], + elicit_gaps: [ + "planner_status", + "planner_artifact_read", + "planner_questions_submit", + "planner_questions_resolve", + "planner_spec_submit", + ], + verify_spec: [ + "planner_status", + "planner_artifact_read", + "planner_gate_check", + "planner_spec_submit", + ], + finish_spec: ["planner_status", "planner_artifact_read"], + }, + planning: { + read_context: [ + "planner_status", + "planner_artifact_read", + "planner_contract_route", + "planner_contract_read", + ], + draft_plan: [ + "planner_status", + "planner_artifact_read", + "planner_plan_submit", + "planner_contract_route", + "planner_contract_read", + ], + split_tasks: [ + "planner_status", + "planner_artifact_read", + "planner_contract_route", + "planner_contract_read", + ], + write_task_files: [ + "planner_status", + "planner_artifact_read", + "planner_task_upsert", + "planner_contract_route", + "planner_contract_read", + ], + verify_plan: ["planner_status", "planner_artifact_read"], + consistency_check: [ + "planner_status", + "planner_artifact_read", + "planner_gate_check", + "planner_elenchus_check", + "planner_reason", + "planner_task_upsert", + "planner_contract_route", + "planner_contract_read", + ], + enter_execution: ["planner_status", "planner_artifact_read"], + }, + execution: { + prepare_task: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_git_create_task_branch", + ], + write_tdd_plan: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_behavior_upsert", + "planner_gate_check", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_elenchus_check", + "planner_reason", + "planner_exec", + ], + write_tests: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_behavior_upsert", + "planner_gate_check", + "planner_git_commit", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + run_failing_tests: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_behavior_upsert", + "planner_gate_check", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + implement_task: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_git_commit", + "planner_contract_route", + "planner_contract_read", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + contract_check: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_git_commit", + "planner_contract_route", + "planner_contract_read", + "planner_contract_check", + "planner_contract_upsert", + "planner_elenchus_check", + "planner_reason", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + refactor_task: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_refactor_review", + "planner_git_commit", + "planner_contract_route", + "planner_contract_read", + "planner_contract_check", + "planner_contract_upsert", + "planner_git_create_refactor_branch", + "planner_git_merge_refactor_to_task", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + run_final_tests: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_behavior_upsert", + "planner_gate_check", + "planner_git_commit", + "planner_report_stuck", + "planner_skill_create", + "planner_skill_update", + "planner_exec", + ], + capture_skill: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_skill_create", + "planner_skill_update", + "planner_git_discard_changes", + ], + merge_task_to_plan: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_tdd_submit", + "planner_git_merge_task_to_plan", + ], + select_next_task: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + ], + }, + finalize: { + verify_plan_branch: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_contract_route", + "planner_contract_read", + "planner_git_discard_changes", + ], + compact_before_doubt: ["planner_status", "planner_artifact_read"], + doubt_review: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_doubt_review", + "planner_elenchus_check", + "planner_reason", + "planner_skill_create", + "planner_skill_update", + "planner_contract_route", + "planner_contract_read", + "planner_contract_check", + "planner_contract_upsert", + "planner_git_discard_changes", + ], + write_final_summary: [ + "planner_status", + "planner_artifact_read", + "planner_summary_submit", + "planner_skill_create", + "planner_skill_update", + "planner_contract_route", + "planner_contract_read", + "planner_contract_check", + "planner_contract_upsert", + "planner_git_discard_changes", + ], + enter_done: ["planner_status", "planner_artifact_read"], + }, + done: { + present_result: [ + "planner_status", + "planner_artifact_read", + "planner_skill_create", + "planner_skill_update", + "planner_contract_decide", + "planner_git_discard_changes", + ], + await_user_acceptance: [ + "planner_status", + "planner_artifact_read", + "planner_contract_decide", + "planner_git_discard_changes", + ], + handle_change_request: [ + "planner_status", + "planner_artifact_read", + "planner_plan_submit", + "planner_discovery_submit", + ], + prepare_output_branch: ["planner_status", "planner_artifact_read"], + merge_or_export_result: ["planner_status", "planner_artifact_read"], + cleanup_worktree: ["planner_status", "planner_artifact_read"], + mark_done: ["planner_status", "planner_artifact_read"], + cleanup_plan_files: ["planner_status", "planner_artifact_read"], + }, + recovery: { + read_state: [ + "planner_status", + "planner_artifact_read", + "planner_recovery_inspect", + ], + inspect_git: [ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_recovery_inspect", + ], + compare_expected_actual: [ + "planner_status", + "planner_artifact_read", + "planner_recovery_inspect", + ], + classify_recovery: [ + "planner_status", + "planner_artifact_read", + "planner_recovery_inspect", + ], + ask_user_if_destructive: [ + "planner_status", + "planner_artifact_read", + "planner_recovery_inspect", + ], + repair_or_resume: [ + "planner_status", + "planner_artifact_read", + "planner_recovery_inspect", + "planner_recovery_resume", + "planner_git_inspect", + "planner_elenchus_check", + "planner_reason", + ], + }, +}; + +/** A running, healthy state at the given position — the matrix baseline. */ +function runningAt(stage: string, step: string) { + return { + stage, + step, + broken: false, + requiresUserDecision: false, + requiresCompact: false, + debugArtifactsDir: null, + } as never; +} + +const DEBUG_TOOLS = [ + "planner_debug_strategy", + "planner_debug_probe", + "planner_debug_result", + "planner_debug_cleanup", +] as const; + +// Read-only and cross-stage: refusing these can never protect the state machine, +// and losing them blinds the model everywhere, including in recovery. +const ALWAYS = ["planner_status", "planner_artifact_read"] as const; + +describe("stage/step tool matrix", () => { + it("covers every stage and step the state machine can be in", () => { + expect(Object.keys(STAGE_STEP_MATRIX).sort()).toEqual( + Object.keys(PLANNER_STAGE_STEPS).sort(), + ); + for (const [stage, steps] of Object.entries(PLANNER_STAGE_STEPS)) { + expect(Object.keys(STAGE_STEP_MATRIX[stage])).toEqual([...steps]); + } + }); + + for (const [stage, steps] of Object.entries(STAGE_STEP_MATRIX)) { + for (const [step, expected] of Object.entries(steps)) { + it(`${stage}/${step} offers exactly ${expected.length} tools, in order`, () => { + expect(getAllowedPlannerWrapperTools(runningAt(stage, step))).toEqual( + expected, + ); + }); + } + } + + it("offers the read-only pair at every single position", () => { + for (const [stage, steps] of Object.entries(STAGE_STEP_MATRIX)) { + for (const step of Object.keys(steps)) { + const tools = getAllowedPlannerWrapperTools(runningAt(stage, step)); + expect(tools.slice(0, ALWAYS.length)).toEqual([...ALWAYS]); + } + } + }); +}); + +describe("tool reachability", () => { + it("leaves no wrapper tool unreachable from a normal run", () => { + const reachable = new Set(); + for (const [stage, steps] of Object.entries(STAGE_STEP_MATRIX)) { + for (const step of Object.keys(steps)) { + for (const tool of getAllowedPlannerWrapperTools( + runningAt(stage, step), + )) { + reachable.add(tool); + } + } + } + // Opened only by planner_report_stuck, so they are absent from the baseline. + for (const tool of DEBUG_TOOLS) reachable.add(tool); + // Reached through the preflight decision before a plan exists + // (STATUS_ONLY_TOOLS in runtime/planner-runtime.ts), never through a step. + reachable.add("planner_create_plan"); + // Offered only while the run is broken / awaiting a user decision. + reachable.add("planner_recovery_inspect"); + reachable.add("planner_recovery_resume"); + + expect(PLANNER_WRAPPER_TOOLS.filter((t) => !reachable.has(t))).toEqual([]); + }); + + it("keeps lifecycle transitions out of the wrapper gate entirely", () => { + // They are gated by their own exit conditions, not by this allowlist; if one + // ever appeared here it could be refused and the machine could not advance. + for (const tool of PLANNER_LIFECYCLE_TRANSITION_TOOLS) { + expect(PLANNER_WRAPPER_TOOLS as readonly string[]).not.toContain(tool); + expect(ALL_PLANNER_TOOL_NAMES as readonly string[]).toContain(tool); + } + }); +}); + +describe("tools that appear and disappear with run state", () => { + // planner_report_stuck opens a debug session (sets debugArtifactsDir); the four + // debug wrappers appear then, and planner_debug_cleanup closes it again. This is + // the one place where the offered set changes without the step changing. + it("adds the debug wrappers when a stuck debug session is open", () => { + const closed = getAllowedPlannerWrapperTools( + runningAt("execution", "implement_task"), + ); + const open = getAllowedPlannerWrapperTools({ + ...runningAt("execution", "implement_task"), + debugArtifactsDir: "/w/.pi/pi-code-planner/debug/task/sess", + } as never); + + expect(closed).not.toContain("planner_debug_probe"); + expect( + open.filter((t) => (DEBUG_TOOLS as readonly string[]).includes(t)), + ).toEqual([...DEBUG_TOOLS]); + // Opening a debug session only adds; nothing the model had is taken away. + expect( + open.filter((t) => !(DEBUG_TOOLS as readonly string[]).includes(t)), + ).toEqual([...closed]); + }); + + it("removes them again once the debug session is cleaned up", () => { + const after = getAllowedPlannerWrapperTools( + runningAt("execution", "implement_task"), + ); + for (const tool of DEBUG_TOOLS) expect(after).not.toContain(tool); + }); + + // The debug wrappers are derived, not listed per step: they are offered exactly + // when a session is open AND the step could have opened one (it allows + // planner_report_stuck). Pinned as an equality over the whole stage so the two + // halves of the stuck flow cannot drift apart again — they did once, and + // contract_check ended up able to open a session it could not close. + const STUCK_STEPS = [ + "write_tdd_plan", + "write_tests", + "run_failing_tests", + "implement_task", + "contract_check", + "refactor_task", + "run_final_tests", + ] as const; + + it("allows planner_report_stuck on exactly these execution steps", () => { + const actual = Object.entries(STAGE_STEP_MATRIX.execution) + .filter(([, tools]) => tools.includes("planner_report_stuck")) + .map(([step]) => step); + expect(actual).toEqual([...STUCK_STEPS]); + }); + + it("drives the debug loop on exactly the steps that can report stuck", () => { + const withDebug = (step: string) => + getAllowedPlannerWrapperTools({ + ...runningAt("execution", step), + debugArtifactsDir: "/w/dbg", + } as never); + const actual = Object.keys(STAGE_STEP_MATRIX.execution).filter((step) => + withDebug(step).includes("planner_debug_probe"), + ); + expect(actual).toEqual([...STUCK_STEPS]); + }); + + it("can always close a session it was able to open", () => { + // The failure this replaces: report stuck at contract_check, then find + // planner_git_commit blocked by the leftover artifacts and planner_debug_cleanup + // refused by the step. + for (const step of STUCK_STEPS) { + const open = getAllowedPlannerWrapperTools({ + ...runningAt("execution", step), + debugArtifactsDir: "/w/dbg", + } as never); + expect(open).toContain("planner_debug_cleanup"); + } + }); + + it("offers no debug wrapper on a step that cannot report stuck", () => { + const open = getAllowedPlannerWrapperTools({ + ...runningAt("execution", "prepare_task"), + debugArtifactsDir: "/w/dbg", + } as never); + for (const tool of DEBUG_TOOLS) expect(open).not.toContain(tool); + }); + + it("collapses to recovery tools while broken", () => { + expect( + getAllowedPlannerWrapperTools({ + ...runningAt("execution", "implement_task"), + broken: true, + } as never), + ).toEqual([ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_recovery_inspect", + "planner_recovery_resume", + ]); + }); + + it("collapses to the same set while awaiting a user decision", () => { + expect( + getAllowedPlannerWrapperTools({ + ...runningAt("done", "await_user_acceptance"), + requiresUserDecision: true, + } as never), + ).toEqual([ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_recovery_inspect", + "planner_recovery_resume", + ]); + }); + + it("adds the consistency checker at the repair decision, and only there", () => { + const repair = getAllowedPlannerWrapperTools({ + ...runningAt("recovery", "repair_or_resume"), + broken: true, + } as never); + expect(repair).toEqual([ + "planner_status", + "planner_artifact_read", + "planner_git_inspect", + "planner_recovery_inspect", + "planner_recovery_resume", + "planner_elenchus_check", + "planner_reason", + ]); + const otherRecoveryStep = getAllowedPlannerWrapperTools({ + ...runningAt("recovery", "classify_recovery"), + broken: true, + } as never); + expect(otherRecoveryStep).not.toContain("planner_elenchus_check"); + }); + + it("collapses to the read-only pair while a compact boundary is pending", () => { + expect( + getAllowedPlannerWrapperTools({ + ...runningAt("finalize", "compact_before_doubt"), + requiresCompact: true, + } as never), + ).toEqual([...ALWAYS]); + }); +}); diff --git a/src/guard/tool-policy.ts b/src/guard/tool-policy.ts index ec031f8..3fb47c7 100644 --- a/src/guard/tool-policy.ts +++ b/src/guard/tool-policy.ts @@ -206,10 +206,6 @@ const STEP_ALLOWED_TOOLS = { "planner_skill_update", "planner_elenchus_check", "planner_reason", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], write_tests: [ @@ -221,10 +217,6 @@ const STEP_ALLOWED_TOOLS = { "planner_report_stuck", "planner_skill_create", "planner_skill_update", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], run_failing_tests: [ @@ -235,10 +227,6 @@ const STEP_ALLOWED_TOOLS = { "planner_report_stuck", "planner_skill_create", "planner_skill_update", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], implement_task: [ @@ -250,10 +238,6 @@ const STEP_ALLOWED_TOOLS = { "planner_report_stuck", "planner_skill_create", "planner_skill_update", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], contract_check: [ @@ -285,10 +269,6 @@ const STEP_ALLOWED_TOOLS = { "planner_report_stuck", "planner_skill_create", "planner_skill_update", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], run_final_tests: [ @@ -300,10 +280,6 @@ const STEP_ALLOWED_TOOLS = { "planner_report_stuck", "planner_skill_create", "planner_skill_update", - "planner_debug_strategy", - "planner_debug_probe", - "planner_debug_result", - "planner_debug_cleanup", "planner_exec", ], capture_skill: [ @@ -422,7 +398,10 @@ export function getAllowedPlannerWrapperTools( Record > = STEP_ALLOWED_TOOLS[state.stage]; const stepRules = stageRules[state.step] ?? []; - return withAlwaysAllowed(filterDebugToolsForState(stepRules, state)); + return withAlwaysAllowed([ + ...stepRules, + ...debugToolsForState(stepRules, state), + ]); } /** @@ -547,18 +526,27 @@ function withAlwaysAllowed( ); } -// Debug tools stay listed in STEP_ALLOWED_TOOLS for execution steps but are -// hidden unless a debug session is actually open (debugArtifactsDir set), so -// the model isn't offered debug wrappers it can't yet call. -function filterDebugToolsForState( - tools: readonly PlannerWrapperTool[], +/** + * The debug wrappers a step may use right now. + * + * They are DERIVED, not listed per step, because they are the second half of one + * flow: `planner_report_stuck` unconditionally opens a debug session + * (`initializePlannerDebugSession` in `runtime/debug-tools.ts`), and that session + * can only be driven — and closed — with these four. Maintaining the two lists by + * hand let them drift: `contract_check` allowed the stuck report but not the + * wrappers, so a model that reported stuck there opened a session it could not + * touch, and `planner_git_commit` stayed blocked by + * `assertNoPlannerDebugArtifactsBeforeCommit` while telling it to run a + * `planner_debug_cleanup` the step would refuse. + * + * So the rule is the invariant, and the lists cannot drift again: offer them + * exactly when a session is open AND this step could have opened one. + */ +function debugToolsForState( + stepTools: readonly PlannerWrapperTool[], state: Pick, ): readonly PlannerWrapperTool[] { - if (state.debugArtifactsDir) { - return tools; - } - return tools.filter( - (tool) => - !(DEBUG_WRAPPER_TOOLS as readonly PlannerWrapperTool[]).includes(tool), - ); + if (!state.debugArtifactsDir) return []; + if (!stepTools.includes("planner_report_stuck")) return []; + return DEBUG_WRAPPER_TOOLS; } diff --git a/src/index.tool-visibility.test.ts b/src/index.tool-visibility.test.ts index 53846be..e27d2d9 100644 --- a/src/index.tool-visibility.test.ts +++ b/src/index.tool-visibility.test.ts @@ -6,6 +6,7 @@ import { } from "./guard/tool-policy"; import { activatePlannerToolVisibility, + computePlannerActiveTools, filterPlannerTools, persistPlannerToolVisibilityActive, persistPlannerToolVisibilityActiveToSession, @@ -170,6 +171,7 @@ describe("updateToolVisibility", () => { const activeTools: string[][] = []; const mockPi = { getAllTools: () => mockTools, + getActiveTools: () => mockTools.map((t) => t.name), setActiveTools: (names: string[]) => { activeTools.push(names); }, @@ -184,6 +186,7 @@ describe("updateToolVisibility", () => { const activeTools: string[][] = []; const mockPi = { getAllTools: () => mockTools, + getActiveTools: () => mockTools.map((t) => t.name), setActiveTools: (names: string[]) => { activeTools.push(names); }, @@ -201,6 +204,7 @@ describe("updateToolVisibility", () => { const handlers: Record Promise> = {}; const mockPi = { getAllTools: () => mockTools, + getActiveTools: () => mockTools.map((t) => t.name), setActiveTools: (names: string[]) => { activeTools.push(names); }, @@ -228,6 +232,7 @@ describe("updateToolVisibility", () => { > = {}; const mockPi = { getAllTools: () => mockTools, + getActiveTools: () => mockTools.map((t) => t.name), setActiveTools: (names: string[]) => { activeTools.push(names); }, @@ -308,6 +313,7 @@ describe("contract gate tool visibility", () => { let captured: string[] = []; const mockPi = { getAllTools: () => tools, + getActiveTools: () => tools.map((t) => t.name), setActiveTools: (names: string[]) => { captured = names; }, @@ -363,6 +369,7 @@ describe("recovery report tool visibility", () => { let captured: string[] = []; const mockPi = { getAllTools: () => tools, + getActiveTools: () => tools.map((t) => t.name), setActiveTools: (names: string[]) => { captured = names; }, @@ -401,3 +408,153 @@ describe("recovery report tool visibility", () => { expect(setRecoveryReportUnlocked(true)).toBe(false); }); }); + +describe("computePlannerActiveTools", () => { + // Registry order, and it does not move — the result is built by walking it, so + // a stable decision produces byte-identical tool schemas at the head of the + // prompt. `manager_reply` stands in for another extension's tool. + const ALL = [ + "bash", + "read", + "manager_reply", + "planner_status", + "planner_contract_scan", + "planner_git_commit", + "planner_recovery_report", + "planner_finish_step", + ]; + + const compute = ( + over: Partial[0]>, + ) => + computePlannerActiveTools({ + allToolNames: ALL, + activeNow: ALL, + planActive: true, + contractGate: false, + recoveryReportUnlocked: false, + ...over, + }); + + it("leaves another extension's hidden tool hidden", () => { + // The bug this replaces: we rebuilt the list from getAllTools(), which + // resurrected every tool anyone else had just hidden — and changed the head + // of the prompt between two calls of one turn. + const activeNow = ALL.filter((name) => name !== "manager_reply"); + expect(compute({ activeNow })).not.toContain("manager_reply"); + }); + + it("hides every planner tool when no plan is active, and touches nothing else", () => { + expect(compute({ planActive: false })).toEqual([ + "bash", + "read", + "manager_reply", + ]); + }); + + it("claims planner tools back even when they are not currently active", () => { + // Ours must be reachable whatever anyone else decided the world should be. + const activeNow = ["bash"]; + const result = compute({ activeNow }); + expect(result).toContain("planner_status"); + expect(result).toContain("planner_finish_step"); + }); + + it("keeps the recovery report hidden until stuck-detection unlocks it", () => { + expect(compute({})).not.toContain("planner_recovery_report"); + expect(compute({ recoveryReportUnlocked: true })).toContain( + "planner_recovery_report", + ); + }); + + it("is a fixed point: recomputing over its own output changes nothing", () => { + // A stable set of tools is a stable head, which is a prompt the backend does + // not have to read twice. Convergence is the property that buys that. + const once = compute({}); + expect(compute({ activeNow: once })).toEqual(once); + }); + + it("preserves registry order, since order is part of the head's bytes", () => { + const result = compute({ activeNow: [...ALL].reverse() }); + expect(result).toEqual(ALL.filter((n) => n !== "planner_recovery_report")); + }); + + it("closes the world down to the allowlist while the contract gate is up", () => { + // A sandbox, not a subtraction: project reads must be unreachable, so other + // extensions' tools go too. + expect(compute({ contractGate: true })).toEqual([ + "planner_status", + "planner_contract_scan", + "planner_finish_step", + ]); + }); +}); + +describe("contract-gate sandbox entry and exit", () => { + const ALL = [ + "bash", + "manager_reply", + "planner_status", + "planner_contract_scan", + "planner_git_commit", + "planner_finish_step", + ]; + + function driver() { + let world = ALL.filter((name) => name !== "manager_reply"); + const mockPi = { + getAllTools: () => ALL.map((name) => ({ name })), + getActiveTools: () => world, + setActiveTools: (names: string[]) => { + world = names; + }, + } as unknown as ExtensionAPI; + return { + run: () => { + updateToolVisibility(mockPi); + return world; + }, + }; + } + + beforeEach(() => { + setPlanActive(true); + setContractGateActive(false); + setRecoveryReportUnlocked(false); + }); + + afterEach(() => { + setContractGateActive(false); + setPlanActive(false); + }); + + it("restores the pre-sandbox world on the way out, not the sandbox", () => { + // Subtracting from the live list on the way out would leave everyone else's + // tools hidden for good: while the gate is up the live list IS the sandbox, + // so there is nothing left to subtract from. + const { run } = driver(); + run(); + setContractGateActive(true); + expect(run()).toEqual([ + "planner_status", + "planner_contract_scan", + "planner_finish_step", + ]); + setContractGateActive(false); + const after = run(); + expect(after).toContain("bash"); + expect(after).toContain("planner_git_commit"); + // Still respected: another extension hid this before the gate went up. + expect(after).not.toContain("manager_reply"); + }); + + it("remembers the world once, not on every refresh inside the gate", () => { + const { run } = driver(); + run(); + setContractGateActive(true); + run(); + run(); + setContractGateActive(false); + expect(run()).toContain("bash"); + }); +}); diff --git a/src/index.tool-visibility.ts b/src/index.tool-visibility.ts index 04c4192..484127a 100644 --- a/src/index.tool-visibility.ts +++ b/src/index.tool-visibility.ts @@ -161,25 +161,111 @@ export function resetPlanActiveCache(pi: ExtensionAPI): void { updateToolVisibility(pi); } +/** + * The active tool list to apply, as a SUBTRACTION from what is already active + * rather than a fresh statement of the whole world. + * + * `setActiveTools` is a global setter with no notion of whose tools are whose: + * whoever writes last wins the entire list. We used to rebuild it from + * `getAllTools()` on every provider request — and so does at least one other + * extension in the same session. Two extensions each declaring the world, on + * every request, resurrect each other's hidden tools and change the head of the + * prompt between two calls of one turn. The head is where the tool schemas live, + * and a prefix cache reuses exactly one thing: bytes it has already read. Change + * byte zero and the whole prompt is re-read. + * + * So we state only our own invariants and leave everyone else's decisions alone: + * + * next = (active now ∪ what we claim) \ what we hide + * + * This converges: our claim/hide sets are a pure function of planner state, so + * repeating the computation yields the same list — a stable set of tools, which + * is a stable head. Order is part of those bytes, so the result is built by + * walking the registry, whose order does not move. + * + * The contract gate is the exception, and deliberately so: it is a SANDBOX, not + * a subtraction. While it is up the model must not reach project reads at all, + * so the list is exactly the allowlist and other extensions' tools go with it. + */ +export function computePlannerActiveTools(input: { + allToolNames: readonly string[]; + activeNow: readonly string[]; + planActive: boolean; + contractGate: boolean; + recoveryReportUnlocked: boolean; +}): string[] { + if (input.planActive && input.contractGate) { + return input.allToolNames.filter((name) => CONTRACT_GATE_ALLOWED.has(name)); + } + + const claimed = new Set(); + const hidden = new Set(); + if (!input.planActive) { + // No plan: none of our tools belong in the prompt. + for (const name of plannerNames) hidden.add(name); + } else { + for (const name of plannerNames) claimed.add(name); + if (!input.recoveryReportUnlocked) { + claimed.delete(PLANNER_RECOVERY_REPORT_TOOL_NAME); + hidden.add(PLANNER_RECOVERY_REPORT_TOOL_NAME); + } + } + + const active = new Set(input.activeNow); + return input.allToolNames.filter( + (name) => (active.has(name) || claimed.has(name)) && !hidden.has(name), + ); +} + +/** + * The world as it was before the contract-gate sandbox went up. + * + * While the sandbox is up the live list IS the sandbox, so "subtract from what is + * active" would have almost nothing to subtract from: come out of the gate + * reading the live list and every other extension's tools would stay hidden for + * good. So the world is remembered on the way in and restored on the way out. + */ +let worldBeforeContractGate: string[] | null = null; + +/** + * The exact list we last wrote, or `null` before the first refresh — our own + * footprint on a global setter. + * + * `setActiveTools` has no notion of whose tools are whose, so a tool list + * arriving at the provider is not self-evidently ours: another extension may have + * written after us. Only the writer can answer "is this what WE decided?", and + * `runtime/prefix-watch.ts` needs the answer to tell a head we rewrote from a + * head somebody else did. + */ +let lastSetToolNames: string[] | null = null; + +export function plannerLastSetToolNames(): readonly string[] | null { + return lastSetToolNames; +} + /** Update the list of active tools in the Pi extension API. */ export function updateToolVisibility(pi: ExtensionAPI): void { - const allTools = pi.getAllTools(); - let toolNames: string[]; - if (!planActiveCache) { - toolNames = filterPlannerTools(allTools).map((t) => t.name); - } else if (contractGateActive) { - toolNames = allTools - .map((t) => t.name) - .filter((name) => CONTRACT_GATE_ALLOWED.has(name)); - } else { - toolNames = allTools - .map((t) => t.name) - .filter( - (name) => - name !== PLANNER_RECOVERY_REPORT_TOOL_NAME || recoveryReportUnlocked, - ); + const allToolNames = pi.getAllTools().map((tool) => tool.name); + const sandboxed = planActiveCache && contractGateActive; + if (sandboxed) { + worldBeforeContractGate ??= pi.getActiveTools(); + } + const activeNow = sandboxed + ? [] + : (worldBeforeContractGate ?? pi.getActiveTools()); + if (!sandboxed) { + worldBeforeContractGate = null; } - pi.setActiveTools(toolNames); + + const next = computePlannerActiveTools({ + allToolNames, + activeNow, + planActive: planActiveCache, + contractGate: contractGateActive, + recoveryReportUnlocked, + }); + lastSetToolNames = next; + pi.setActiveTools(next); } function restorePlannerToolVisibilityFromSession(ctx?: ExtensionContext): void { diff --git a/src/index.ts b/src/index.ts index b752032..0af23d9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ import { isPlanActive, markPlannerToolVisibilityActive, persistPlannerToolVisibilityActiveToSession, + plannerLastSetToolNames, registerPlannerToolVisibility, resetPlanActiveCache, setContractGateActive, @@ -158,6 +159,7 @@ import { PLANNER_PLAN_TOOL_NAMES, type PlannerPlanToolName, } from "./runtime/plan-tools"; +import { formatHeadChurnWarning, PrefixWatch } from "./runtime/prefix-watch"; import { runPlannerPreflight } from "./runtime/preflight"; import { executePlannerQuestionTool, @@ -549,6 +551,16 @@ const CONTRACT_CHECK_TOOL_PARAMETERS = { description: "Nearest meaningful AGENTS.md path when action is upsert_existing or create_new.", }, + coverageCursor: { + type: "number", + description: + "First directory of the coverage listing to show, 0-based. The result names the next cursor when more remain. Re-send the same fields; paging does not record a second check.", + }, + coveragePath: { + type: "string", + description: + "Show only one place in the coverage listing: a directory, or a file (the directory holding it is shown). Use instead of paging when only one domain matters.", + }, }, required: [ "action", @@ -1426,6 +1438,45 @@ export default function piCodePlannerExtension(pi: ExtensionAPI): void { registerPlannerSkillResources(pi); registerInstructionDefaultsSync(pi); registerPlannerToolVisibility(pi); + registerPlannerPrefixWatch(pi); +} + +/** + * Watch the head of every provider request and name it when something rewrites it + * mid-run. + * + * The tool schemas sit at the front of the prompt, and a prefix cache reuses one + * thing: bytes it has already read. Between runs a changed head is expected; a + * head rewritten between two calls of ONE run means the backend threw away + * everything it had read for nothing. This is measurement first — narrowing the + * active tool list per stage is a deliberate head change, and deciding that + * without numbers is how the previous context mechanism here was built and then + * had to be removed. + * + * Local only: `ctx.ui.notify`, nothing is ever sent anywhere. One notice per run, + * so an alarm firing on every call cannot drown the session it is reporting on. + */ +/** One watcher per extension load; a reload starts a fresh history. */ +const plannerPrefixWatch = new PrefixWatch(); + +function registerPlannerPrefixWatch(pi: ExtensionAPI): void { + let notifiedThisRun = false; + pi.on("agent_start", async () => { + plannerPrefixWatch.runStarted(); + notifiedThisRun = false; + }); + pi.on("before_provider_request", async (event, ctx) => { + const churn = plannerPrefixWatch.record( + event.payload, + plannerLastSetToolNames(), + ); + // Only a mid-run change we did not make is a defect: our own deliberate + // moves are a cost chosen with open eyes, and an alarm that cannot + // recognise its owner's footsteps is one nobody keeps armed. + if (!churn?.midRun || !churn.foreign || notifiedThisRun) return; + notifiedThisRun = true; + ctx.ui.notify(formatHeadChurnWarning(churn), "warning"); + }); } type RegisterToolFn = ExtensionAPI["registerTool"]; diff --git a/src/runtime/AGENTS.md b/src/runtime/AGENTS.md index ae9e005..00af7a8 100644 --- a/src/runtime/AGENTS.md +++ b/src/runtime/AGENTS.md @@ -42,7 +42,7 @@ Runtime domain for planner stages, model-facing status, tool wrappers, timers, r - `planner-runtime.ts` → `evaluatePlannerRuntimeReality`: decides `allow_stage_machine` vs `require_recovery`/`require_user_decision`/`require_compact`/`no_active_plan` from `ActivePlanContextStatus` + `PlannerGitReality`. Feeds `lifecycle.ts`. - `preflight.ts` → `runPlannerPreflight`/`checkPlannerPreflightToolAllowed`: assembles `PlannerPreflightResult` (active plan context, git reality, instruction routing, runtime decision) — the single read pass every tool depends on before deciding anything. -**Contracts / DOX** (`contracts.ts`) — implements the AGENTS.md contract flow: scans AGENTS.md/context files → routes chains → reads → upserts → validates (`parsePlannerContractMarkdown`, `formatPlannerContractBlock`, `validateContractReferences`). Contract state (summaries, chains, touchedFiles) lives in `PlannerContractsState` inside `state.json`. `workflow-tools.ts` calls `validateContractCheckCompleted`/`validateDiscoveryContractRouting` from here to gate step completion, and `status.ts` calls `formatPlannerContractsStatus` to surface it to the model. +**Contracts / DOX** (`contracts.ts`) — implements the AGENTS.md contract flow: scans AGENTS.md/context files → routes chains → reads → upserts → validates (`parsePlannerContractMarkdown`, `formatPlannerContractBlock`, `validateContractReferences`). Contract state (summaries, chains, touchedFiles) lives in `PlannerContractsState` inside `state.json`. `workflow-tools.ts` calls `validateContractCheckCompleted`/`validateDiscoveryContractRouting` from here to gate step completion, and `status.ts` calls `formatPlannerContractsStatus` to surface it to the model. The coverage listing `planner_contract_check` returns is **paged** (`formatCoverageMapPage`), not whole: it is one line per file in the HEAD commit, so a repository that committed its build output produced a single 79 802-character tool result — 31.5% of all tool output in one measured session. The page names its own `coverageCursor` and offers `coveragePath` for one directory or the directory holding one file, so the model chooses how much of it to read. Because a page turn re-sends the same required fields, `contractCheck` compares them against `state.contracts.lastCheck` and skips the tdd.md append when they match — otherwise paging would write the same check twice. **Plan & task lifecycle tools** — model-facing tool handlers that read/write `PlanRecord`/`TaskRecord`/`PlanStateRecord`. - `plan-tools.ts` → create/improve a plan: syncs bundled instruction defaults, creates the plan branch (`git/branches.ts`), initializes plan files/state (`storage/plan-store.ts`, `storage/schema.ts` `createInitialPlanState`). @@ -108,6 +108,7 @@ Runtime domain for planner stages, model-facing status, tool wrappers, timers, r **Misc** - `compact.ts` → builds the system-instructions bundle injected after a context compact (`PLANNER_COMPACT_MARKER`/`PLANNER_SYSTEM_INSTRUCTIONS_HEADER`), pulling section content from `instructions/manager.ts`. - **Window management belongs to Pi, not to the planner.** The extension no longer decides *when* to compact for context relief: the window-management compact steps (`compact_discovery`/`spec`/`planning`/`task` and `compact_finalize`) were removed from `PLANNER_STAGE_STEPS`, and the proactive `turn_end` monitor that replaced them (an output-aware token floor) has now been removed too. Reason: the floor was derived from `model.maxTokens` because an extension cannot read Pi's `reserveTokens`, so the two thresholds land wherever the user's settings happen to put them. On a real session (window 131072, `reserveTokens` 24576) our floor sat at 106823 against Pi's 106496 — 0.25% of the window apart, i.e. a guaranteed race instead of the intended separation, and a second compaction started right after Pi's leaves nothing to summarize (`prepareCompaction` measures only what accumulated since the previous cut point, so it throws "Nothing to compact (session too small)" while the window is 96% full). Pi's own auto-compaction sees `reserveTokens` and cannot race itself, so it is the only trigger now. The one surviving compact step is `compact_before_doubt` — a deliberate confidence reset before the doubt audit, not window relief — so `isPlannerCompactEnabled` returns true for it unconditionally and `planner_request_compact` runs it with no token gate at all. Everything that reacts *after* a compaction is untouched and works for Pi's compactions exactly as it did for ours: the `session_before_compact` indicator, the `session_compact` handler, and the post-compact `[SYSTEM_INSTRUCTIONS]` bundle. A legacy `state.json` parked at a removed step is remapped forward by `normalizePlanState` (`storage/state-store.ts`). The `compactBoundaries` (`compact.stage`/`compact.task`) setting and the `state.compactBoundaries` field were removed with the boundaries they gated — a legacy `settings.json` with a `compact` block gets an "unknown key" note, and a legacy `state.json` with a `compactBoundaries` field just ignores it. +- `prefix-watch.ts` → pure probe over the actual provider payload (`before_provider_request`). Tool schemas render INTO the system message, so they sit at the very front of the prompt, and every serving backend reuses exactly one thing: a prefix of bytes it already read. `describePayload` reduces a payload to `headChars`/`toolNames`/`toolChars`/`headKey`; `comparePayloads` reports what the head gained and lost; `PrefixWatch` holds ONE previous shape — no history, no defect log, nothing to report from later — and separates *when* (between runs is expected; between two calls of ONE run the prefix was discarded for nothing) from *who* (`setActiveTools` is a global setter and this extension is not the only writer, so `record()` takes the list WE last set — `plannerLastSetToolNames()`). Only mid-run AND foreign earns an alarm; wired in `index.ts` `registerPlannerPrefixWatch`, one `ctx.ui.notify` per run, nothing sent anywhere. It stays armed because the foreign writer is real, not hypothetical: the same session runs pi-telegram-manager, which answers `session_before_compact` with its own compaction. Ported from that extension's `core/payload-probe.ts`. - `compact-eta.ts` → pure empirical ETA for the compaction indicator. `estimateCompactionDuration` fits `T(x)=a+b·x` (weighted least squares, recency- and model-weighted; falls back to a through-origin rate model) over `storage/compact-timing-store.ts` history and reports a point ETA + band + variance (`cv` → `stable`/`noisy`/`single`/`none`). `compactionProgressFraction` drives an honest asymptotic bar that fills to a confidence-scaled target at the ETA and never reaches 100% before the real completion event. Wired in `index.ts` `registerPlannerCompactEvents`, gated by `isPlanActive`; the SDK does not stream summary generation, so this learned model is the only real progress signal. - `skill-library.ts` → `planner_skill_create`/`planner_skill_update`: persists reusable skill markdown + a JSON index (`storage/json.ts`) keyed by source kind (`stuck`/`debug`/`doubt_review`/…); `status.ts` lists active skills from here. - `question-tools.ts` → `planner_questions_submit`/`planner_questions_resolve`, the user-clarification loop; writes state via `storage/state-store.ts`. diff --git a/src/runtime/contracts.test.ts b/src/runtime/contracts.test.ts index be6a681..de0c838 100644 --- a/src/runtime/contracts.test.ts +++ b/src/runtime/contracts.test.ts @@ -25,7 +25,9 @@ import { initializePlanState } from "../storage/state-store"; import { MockPlannerFs } from "../test/mock-fs"; import { buildDirCoverageMap, + type DirectoryCoverageEntry, executePlannerContractTool, + formatCoverageMapPage, formatPlannerContractBlock, formatPlannerContractsStatus, initialWritableContractRequired, @@ -98,6 +100,130 @@ describe("buildDirCoverageMap gitignore filtering", () => { }); }); +describe("coverage map paging", () => { + const root = "/repo/app"; + const entry = ( + dir: string, + fileCount: number, + level: DirectoryCoverageEntry["agentsMdLevel"] = "none", + ): DirectoryCoverageEntry => ({ + dir: join(root, dir), + files: Array.from({ length: fileCount }, (_, i) => + join(root, dir, `f${i}.rs`), + ), + agentsMdPath: level === "none" ? null : join(root, dir, "AGENTS.md"), + agentsMdLevel: level, + }); + const many = (count: number) => + Array.from({ length: count }, (_, i) => entry(`d${i}`, 1)); + + it("says so when the commit touched nothing", () => { + const view = formatCoverageMapPage({ entries: [], root }); + expect(view.lines).toEqual(["No committed files detected in HEAD."]); + expect(view.page.nextCursor).toBeNull(); + }); + + it("shows everything and offers no next page when the map is small", () => { + const view = formatCoverageMapPage({ entries: many(3), root }); + expect(view.page).toMatchObject({ shown: 3, total: 3, nextCursor: null }); + expect(view.lines.join("\n")).not.toContain("coverageCursor"); + }); + + it("caps a large map and names the cursor that continues it", () => { + const view = formatCoverageMapPage({ entries: many(39), root }); + expect(view.page).toMatchObject({ cursor: 0, shown: 8, nextCursor: 8 }); + const text = view.lines.join("\n"); + expect(text).toContain("39 directories, 39 files"); + expect(text).toContain("Showing directories 1-8 of 39"); + expect(text).toContain("coverageCursor: 8"); + expect(text).toContain("31 more directories are not shown"); + }); + + it("keeps the whole listing far below one uncapped result", () => { + // The defect this exists for: a committed build directory produced a single + // 79 802-character result. Same shape, paged. + const huge = Array.from({ length: 39 }, (_, i) => entry(`d${i}`, 15)); + const uncapped = huge.reduce((sum, e) => sum + e.files.length, 0); + const view = formatCoverageMapPage({ entries: huge, root }); + expect(uncapped).toBe(585); + expect(view.lines.join("\n").length).toBeLessThan(4000); + }); + + it("continues from a cursor and ends without offering another page", () => { + const view = formatCoverageMapPage({ entries: many(10), root, cursor: 8 }); + expect(view.page).toMatchObject({ cursor: 8, shown: 2, nextCursor: null }); + expect(view.lines.join("\n")).toContain("Showing directories 9-10 of 10"); + }); + + it("truncates the files inside one directory and says how to see them", () => { + const view = formatCoverageMapPage({ entries: [entry("src", 25)], root }); + const text = view.lines.join("\n"); + expect(text).toContain("… 15 more files here"); + expect(text).toContain('coveragePath "src/"'); + }); + + it("selects a single directory by path", () => { + const entries = [...many(20), entry("src", 25, "exact")]; + const view = formatCoverageMapPage({ + entries, + root, + selectPath: "src", + }); + expect(view.page).toMatchObject({ + shown: 1, + selected: "src", + nextCursor: null, + }); + const text = view.lines.join("\n"); + expect(text).toContain("[AGENTS.md ✓]"); + // The selected directory earns the larger budget, so nothing is hidden. + expect(text).not.toContain("more files here"); + }); + + it("selects the directory holding a named file", () => { + const entries = [...many(20), entry("src", 3)]; + const view = formatCoverageMapPage({ + entries, + root, + selectPath: "src/f1.rs", + }); + expect(view.page.selected).toBe("src"); + }); + + it("accepts an absolute path as well as a root-relative one", () => { + const entries = [...many(20), entry("src", 3)]; + const view = formatCoverageMapPage({ + entries, + root, + selectPath: join(root, "src"), + }); + expect(view.page.selected).toBe("src"); + }); + + it("resolves a path buried in trailing separators", () => { + // The separators are stripped by scanning, not by /[/\\]+$/ — the pattern + // backtracks quadratically on exactly this shape, and the path comes in as a + // tool argument. + const entries = [entry("src", 3)]; + const view = formatCoverageMapPage({ + entries, + root, + selectPath: `src${"/".repeat(2000)}`, + }); + expect(view.page.selected).toBe("src"); + }); + + it("falls back to the first page when the path matches nothing", () => { + const view = formatCoverageMapPage({ + entries: many(39), + root, + selectPath: "nowhere/", + }); + expect(view.page).toMatchObject({ selected: null, cursor: 0, shown: 8 }); + expect(view.lines[0]).toContain("No touched directory matches"); + }); +}); + describe("planner local contracts parser", () => { const root = "/repo/app"; const path = "/repo/app/src/AGENTS.md"; diff --git a/src/runtime/contracts.ts b/src/runtime/contracts.ts index 57d13e3..1c3780b 100644 --- a/src/runtime/contracts.ts +++ b/src/runtime/contracts.ts @@ -464,7 +464,7 @@ async function contractRead( ); } -interface DirectoryCoverageEntry { +export interface DirectoryCoverageEntry { dir: string; files: string[]; agentsMdPath: string | null; @@ -542,33 +542,200 @@ export async function buildDirCoverageMap(input: { return entries; } -function formatCoverageMap( - entries: DirectoryCoverageEntry[], +/** + * How much of the coverage map one tool result may carry. + * + * The listing is unbounded by nature — it is one line per file in the HEAD + * commit — and a repository that committed its build output turns that into a + * single tool result larger than everything else the model reads. One measured + * session: 8 calls, 215 211 characters, 31.5% of all tool output, the worst of + * them 79 802 characters (39 directories, 580 files, nearly all of them + * `target/`). A result of that size cannot be re-read, cannot be summarized + * away, and dominates the next compaction's cut point. + * + * So the map is paged instead of truncated silently: the model is told what it + * is not seeing and given the exact call that shows it. Paging is the model's + * decision — a listing this large is usually noise, and the one directory it + * actually needs is reachable directly through `coveragePath`. + */ +const COVERAGE_PAGE_DIRS = 8; +const COVERAGE_DIR_FILES = 10; +/** A single selected directory earns a larger budget, but not an unbounded one. */ +const COVERAGE_SELECTED_DIR_FILES = 200; + +export interface CoverageMapPage { + /** Index of the first directory listed, 0-based. */ + cursor: number; + /** Cursor for the next page, or null when the listing ends here. */ + nextCursor: number | null; + /** Directories listed in this page. */ + shown: number; + /** Directories in the whole map. */ + total: number; + /** The directory the caller selected, relative to root, or null. */ + selected: string | null; +} + +export interface CoverageMapView { + lines: string[]; + page: CoverageMapPage; +} + +function coverageBadge(entry: DirectoryCoverageEntry, root: string): string { + if (entry.agentsMdLevel === "exact") return "[AGENTS.md ✓]"; + if (entry.agentsMdLevel === "parent") { + const parent = relative(root, dirname(entry.agentsMdPath ?? "")) || "."; + return `[AGENTS.md at parent: ${parent}]`; + } + return "[NO AGENTS.md]"; +} + +function coverageRule(entry: DirectoryCoverageEntry): string { + if (entry.agentsMdLevel === "exact") { + return "→ default: upsert_existing (prove no_update if nothing durable changed)"; + } + if (entry.agentsMdLevel === "parent") { + return "→ default: upsert_existing or create_new at this dir (prove no_update if subdomain is trivial)"; + } + return "→ default: create_new (prove no_update if change is fully self-contained)"; +} + +/** + * Drop trailing path separators, by scanning rather than by pattern. + * + * `/[/\\]+$/` says the same thing and is the obvious way to write it, but a + * repeated character class anchored at the end has to backtrack from every + * position when the match fails, so a path that is mostly separators costs + * quadratic time. `coveragePath` arrives from a tool argument, which is exactly + * the input this must not be quadratic in. + */ +function stripTrailingSeparators(value: string): string { + let end = value.length; + while (end > 0 && (value[end - 1] === "/" || value[end - 1] === "\\")) { + end -= 1; + } + return value.slice(0, end); +} + +/** + * Find the entry a caller's `coveragePath` points at. A directory selects + * itself; a file selects the directory holding it, because that is the unit the + * map is keyed by and the unit an AGENTS.md covers. Absolute and root-relative + * paths both work — the model has seen both shapes in earlier results. + */ +function selectCoverageEntry( + entries: readonly DirectoryCoverageEntry[], root: string, -): string[] { - if (entries.length === 0) return ["No committed files detected in HEAD."]; - const lines: string[] = ["Touched directories (from git HEAD commit):"]; - for (const entry of entries) { + path: string, +): DirectoryCoverageEntry | null { + const absolute = isAbsolute(path) ? normalize(path) : join(root, path); + const trimmed = stripTrailingSeparators(absolute); + const byDir = entries.find( + (entry) => stripTrailingSeparators(entry.dir) === trimmed, + ); + if (byDir) return byDir; + return ( + entries.find((entry) => entry.files.some((file) => file === trimmed)) ?? + null + ); +} + +/** + * The coverage map as one page of lines plus the cursor that reaches the rest. + * + * Pure: it takes the whole map and returns the slice, so the paging rules are + * testable without git, a filesystem, or a plan. + */ +export function formatCoverageMapPage(input: { + entries: readonly DirectoryCoverageEntry[]; + root: string; + cursor?: number; + selectPath?: string | null; +}): CoverageMapView { + const { entries, root } = input; + const total = entries.length; + if (total === 0) { + return { + lines: ["No committed files detected in HEAD."], + page: { cursor: 0, nextCursor: null, shown: 0, total: 0, selected: null }, + }; + } + + const totalFiles = entries.reduce( + (sum, entry) => sum + entry.files.length, + 0, + ); + const selected = input.selectPath + ? selectCoverageEntry(entries, root, input.selectPath) + : null; + const missSelection = Boolean(input.selectPath) && selected === null; + + const cursor = missSelection ? 0 : Math.min(input.cursor ?? 0, total); + const page = selected + ? [selected] + : entries.slice(cursor, cursor + COVERAGE_PAGE_DIRS); + const nextCursor = + selected || cursor + page.length >= total ? null : cursor + page.length; + const fileBudget = selected + ? COVERAGE_SELECTED_DIR_FILES + : COVERAGE_DIR_FILES; + + const lines: string[] = []; + if (missSelection) { + lines.push( + `No touched directory matches coveragePath "${input.selectPath}" — showing the first page instead.`, + ); + } + lines.push( + `Touched directories (from git HEAD commit): ${total} directories, ${totalFiles} files.`, + ); + lines.push( + selected + ? `Showing the directory selected by coveragePath.` + : `Showing directories ${cursor + 1}-${cursor + page.length} of ${total}.`, + ); + + for (const entry of page) { const relDir = relative(root, entry.dir) || "."; - const badge = - entry.agentsMdLevel === "exact" - ? "[AGENTS.md ✓]" - : entry.agentsMdLevel === "parent" - ? `[AGENTS.md at parent: ${relative(root, dirname(entry.agentsMdPath ?? ""))}]` - : "[NO AGENTS.md]"; - lines.push(` ${relDir}/ ${badge}`); - for (const f of entry.files) { - lines.push(` - ${relative(root, f)}`); + lines.push(` ${relDir}/ ${coverageBadge(entry, root)}`); + for (const file of entry.files.slice(0, fileBudget)) { + lines.push(` - ${relative(root, file)}`); + } + const hidden = entry.files.length - fileBudget; + if (hidden > 0) { + lines.push( + ` … ${hidden} more files here — pass coveragePath "${relDir}/" to list this directory alone.`, + ); } - const rule = - entry.agentsMdLevel === "exact" - ? "→ default: upsert_existing (prove no_update if nothing durable changed)" - : entry.agentsMdLevel === "parent" - ? "→ default: upsert_existing or create_new at this dir (prove no_update if subdomain is trivial)" - : "→ default: create_new (prove no_update if change is fully self-contained)"; - lines.push(` ${rule}`); + lines.push(` ${coverageRule(entry)}`); } - return lines; + + if (nextCursor !== null) { + lines.push(""); + lines.push( + `${total - (cursor + page.length)} more directories are not shown. This listing is paged so one commit cannot fill the context.`, + ); + lines.push( + `- Next page: call planner_contract_check again with the same fields plus coverageCursor: ${nextCursor}.`, + ); + lines.push( + `- One place instead: pass coveragePath with a directory ("src/") or a file ("src/main.rs" — the directory holding it is shown).`, + ); + lines.push( + "- Paging does not record a second check: the recorded action is the one already stored.", + ); + } + + return { + lines, + page: { + cursor, + nextCursor, + shown: page.length, + total, + selected: selected ? relative(root, selected.dir) || "." : null, + }, + }; } async function contractCheck(input: { @@ -599,6 +766,22 @@ async function contractCheck(input: { const domainImpact = requiredString(params, "domainImpact"); const evidence = stringArray(params.evidence, "evidence", true); const recommendedPath = optionalString(params.recommendedPath); + const coverageCursor = positiveIntegerOrUndefined( + params.coverageCursor, + "coverageCursor", + ); + const coveragePath = optionalString(params.coveragePath); + // A page turn re-sends the same fields, because the guard requires them and + // the model already holds them. That makes it indistinguishable from a real + // second check except by what is already stored, so the stored check decides: + // identical task, action and summary means this call is a page of the check + // that was already recorded, and appending it to tdd.md again would write the + // same evidence twice under the same heading. + const alreadyRecorded = + state.contracts.lastCheck !== null && + state.contracts.lastCheck.taskId === taskId && + state.contracts.lastCheck.action === action && + state.contracts.lastCheck.summary === outcomeSummary; const initialContractReason = initialWritableContractRequired({ state, action, @@ -631,17 +814,19 @@ async function contractCheck(input: { reason: outcomeSummary, taskId, }; - await appendContractCheckToTask({ - fs: input.fs, - planPaths, - taskId, - action, - outcomeSummary, - domainImpact, - changedFiles, - evidence, - recommendedPath: pendingUpsert?.path ?? null, - }); + if (!alreadyRecorded) { + await appendContractCheckToTask({ + fs: input.fs, + planPaths, + taskId, + action, + outcomeSummary, + domainImpact, + changedFiles, + evidence, + recommendedPath: pendingUpsert?.path ?? null, + }); + } const root = requireWorktreePath( state, "Planner contract tools require state.worktreePath.", @@ -667,21 +852,28 @@ async function contractCheck(input: { }, }, })); - const coverageLines = formatCoverageMap(coverageEntries, root); + const coverage = formatCoverageMapPage({ + entries: coverageEntries, + root, + cursor: coverageCursor, + selectPath: coveragePath, + }); return applied( input.toolName, [ - `Planner contract check recorded for task: ${taskId ?? "(none)"}.`, + alreadyRecorded + ? `Planner contract check already recorded for task: ${taskId ?? "(none)"} — this call only pages the coverage map.` + : `Planner contract check recorded for task: ${taskId ?? "(none)"}.`, `Action: ${action}`, pendingUpsert ? `Contract update required: call planner_contract_upsert for ${pendingUpsert.path}.` : "No contract update is required for this task.", "", - ...coverageLines, + ...coverage.lines, "", "Call planner_status before choosing the next planner action.", ].join("\n"), - { taskId, action, pendingUpsert, coverageEntries }, + { taskId, action, pendingUpsert, coverage: coverage.page }, ); } @@ -2145,7 +2337,7 @@ function positiveIntegerOrUndefined( key: string, ): number | undefined { if (value === undefined) return undefined; - const minimum = key === "cursor" ? 0 : 1; + const minimum = key.toLowerCase().endsWith("cursor") ? 0 : 1; if ( typeof value !== "number" || !Number.isInteger(value) || diff --git a/src/runtime/prefix-watch.test.ts b/src/runtime/prefix-watch.test.ts new file mode 100644 index 0000000..8a10083 --- /dev/null +++ b/src/runtime/prefix-watch.test.ts @@ -0,0 +1,198 @@ +import { describe, expect, it } from "vitest"; +import { + comparePayloads, + describePayload, + fingerprintHead, + formatHeadChurnWarning, + PrefixWatch, +} from "./prefix-watch"; + +// An openai-completions payload, which is what a llama.cpp server speaks: the +// system prompt is the leading message and the tool schemas ride alongside. +function payload(input: { tools?: string[]; system?: string; turns?: number }) { + return { + messages: [ + { role: "system", content: input.system ?? "you are a planner" }, + ...Array.from({ length: input.turns ?? 1 }, (_, i) => ({ + role: "user", + content: `turn ${i}`, + })), + ], + tools: (input.tools ?? ["planner_status"]).map((name) => ({ + type: "function", + function: { name, parameters: {} }, + })), + }; +} + +describe("describePayload", () => { + it("counts the system prompt and the tool schemas as the head", () => { + const shape = describePayload(payload({ tools: ["a", "b"] })); + expect(shape.toolNames).toEqual(["a", "b"]); + expect(shape.toolChars).toBeGreaterThan(0); + // The head is the serialized system message plus the serialized schemas — + // what the backend actually reads, not just the prose inside them. + const systemChars = JSON.stringify({ + role: "system", + content: "you are a planner", + }).length; + expect(shape.headChars).toBe(systemChars + shape.toolChars); + // The conversation is not the head — it comes after it. + expect(shape.messages).toBe(1); + expect(shape.messageChars).toBeGreaterThan(0); + }); + + it("reads an Anthropic-style system field as well as a leading message", () => { + const shape = describePayload({ + system: "abc", + messages: [{ role: "user", content: "hi" }], + tools: [], + }); + expect(shape.headChars).toBe(3); + expect(shape.messages).toBe(1); + }); + + it("charges nothing for an empty tool list", () => { + // Faithful to the payload, which omits the key, and to the prompt, where an + // empty list renders to nothing. + expect(describePayload({ messages: [], tools: [] }).toolChars).toBe(0); + }); + + it("survives a payload that is not an object", () => { + expect(describePayload(null).headChars).toBe(0); + expect(describePayload("nonsense").toolNames).toEqual([]); + }); + + it("names tools in both provider shapes", () => { + const shape = describePayload({ + messages: [], + tools: [{ function: { name: "wrapped" } }, { name: "bare" }, 42], + }); + expect(shape.toolNames).toEqual(["wrapped", "bare", "?"]); + }); +}); + +describe("head fingerprint", () => { + it("is equal for equal bytes and different for a one-character change", () => { + expect(fingerprintHead("abc")).toBe(fingerprintHead("abc")); + expect(fingerprintHead("abc")).not.toBe(fingerprintHead("abd")); + }); + + it("separates the same tools offered in a different order", () => { + // Order is part of the bytes: a reordered tool list is a different head, and + // a backend re-reads the whole prompt for it. + const a = describePayload(payload({ tools: ["x", "y"] })); + const b = describePayload(payload({ tools: ["y", "x"] })); + expect(a.headKey).not.toBe(b.headKey); + expect(comparePayloads(a, b).headStable).toBe(false); + }); + + it("is unmoved by a growing conversation", () => { + // The whole point: messages accumulate, the head does not, and the prefix + // stays reusable. + const a = describePayload(payload({ turns: 1 })); + const b = describePayload(payload({ turns: 9 })); + expect(comparePayloads(a, b).headStable).toBe(true); + }); +}); + +describe("comparePayloads", () => { + it("reports which tools were gained and lost, and by how much", () => { + const a = describePayload(payload({ tools: ["keep", "drop"] })); + const b = describePayload(payload({ tools: ["keep", "add"] })); + const delta = comparePayloads(a, b); + expect(delta.headStable).toBe(false); + expect(delta.toolsAdded).toEqual(["add"]); + expect(delta.toolsRemoved).toEqual(["drop"]); + expect(delta.headCharsDelta).not.toBe(Number.NaN); + }); +}); + +describe("PrefixWatch", () => { + const OURS = ["planner_status"]; + + it("says nothing about the first request of a session", () => { + const watch = new PrefixWatch(); + expect(watch.record(payload({}), OURS)).toBeNull(); + }); + + it("says nothing while the head holds still", () => { + const watch = new PrefixWatch(); + watch.record(payload({ turns: 1 }), OURS); + expect(watch.record(payload({ turns: 2 }), OURS)).toBeNull(); + }); + + it("marks a change within one run as mid-run", () => { + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + const churn = watch.record(payload({ tools: ["a", "b"] }), ["a", "b"]); + expect(churn?.midRun).toBe(true); + }); + + it("does not blame the first request after a run boundary", () => { + // Between runs the tool set may legitimately differ; that is not the defect. + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + watch.runStarted(); + const churn = watch.record(payload({ tools: ["a", "b"] }), ["a", "b"]); + expect(churn?.midRun).toBe(false); + }); + + it("recognises its own footsteps and does not report them as foreign", () => { + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + const churn = watch.record(payload({ tools: ["a", "b"] }), ["a", "b"]); + expect(churn?.foreign).toBe(false); + }); + + it("reports a mid-run head somebody else wrote", () => { + // The bug this exists to name: another extension rebuilt the whole active + // tool list on a request of ours, and the backend re-read the prompt. + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + const churn = watch.record(payload({ tools: ["a", "stranger"] }), ["a"]); + expect(churn).toMatchObject({ midRun: true, foreign: true }); + }); + + it("accuses nobody when it does not know what it last set", () => { + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), null); + const churn = watch.record(payload({ tools: ["b"] }), null); + expect(churn?.foreign).toBe(false); + }); + + it("compares each request against the one before it, not against the first", () => { + // Nothing accumulates: the watch holds one shape, so a churn is always the + // step just taken and never a replay of an older one. + const watch = new PrefixWatch(() => 0); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + watch.record(payload({ tools: ["b"] }), ["b"]); + const churn = watch.record(payload({ tools: ["c"] }), ["c"]); + expect(churn?.delta.toolsRemoved).toEqual(["b"]); + expect(churn?.delta.toolsAdded).toEqual(["c"]); + }); +}); + +describe("formatHeadChurnWarning", () => { + it("says what moved, by how much, and that nothing left the machine", () => { + const watch = new PrefixWatch(); + watch.runStarted(); + watch.record(payload({ tools: ["a"] }), ["a"]); + const churn = watch.record(payload({ tools: ["a", "stranger"] }), ["a"]); + const text = formatHeadChurnWarning( + churn ?? + (() => { + throw new Error("expected a churn"); + })(), + ); + expect(text).toContain("mid-run"); + expect(text).toContain("stranger"); + expect(text).toContain("nothing was sent anywhere"); + }); +}); diff --git a/src/runtime/prefix-watch.ts b/src/runtime/prefix-watch.ts new file mode 100644 index 0000000..e743541 --- /dev/null +++ b/src/runtime/prefix-watch.ts @@ -0,0 +1,254 @@ +/** + * What actually went to the provider — and whether its HEAD changed since last time. + * + * The planner can measure the messages it hands Pi. It cannot see the other half of a + * prompt: the system prompt Pi builds and the tool schemas it sends. On a local backend + * those are not a footnote — the chat template renders the tool list INTO the system + * message, so the tools sit at the very front of the prompt, ahead of every word anyone + * has said. Measured on a real planner session: a 32 197-character system prompt plus + * 61 023 characters of schemas for 65 tools, 52 015 of them the planner's own 54 — about + * 23 300 tokens, ~18% of a 131072 window that no compaction can ever reclaim. + * + * Which makes the head the one thing that must not change without a reason. Every serving + * backend (llama.cpp, vLLM, SGLang, Anthropic's cache) reuses exactly one thing: a prefix + * of bytes it has already read. Change byte zero and the whole prompt is re-read — the + * tokens are the same, the seconds are not. + * + * A head change between RUNS is expected: the tool set legitimately differs. Between two + * calls of ONE run, nothing about the model's situation changed and the backend threw away + * everything it had read; that is the defect this names. + * + * It stays armed because `setActiveTools` is a global setter and this extension is not the + * only one writing it — the same session runs pi-telegram-manager, confirmed by four + * compactions it answered through `session_before_compact`. Ported from that extension's + * `core/payload-probe.ts`, which found the class of bug this one was on the wrong side of: + * two extensions each rebuilding the whole active tool list on every request, resurrecting + * each other's hidden tools and rewriting the head mid-turn. + * + * Structural typing, no SDK import: a payload is whatever the provider was handed. Every + * function here is pure, so the whole probe is testable with plain objects. + */ + +/** The parts of a provider payload we can compare. Provider-agnostic by shape. */ +interface PayloadLike { + messages?: unknown; + tools?: unknown; + system?: unknown; +} + +/** One request, reduced to what decides whether a backend can reuse it. */ +export interface PayloadShape { + /** Characters in the head: the system prompt plus the serialized tool schemas. */ + headChars: number; + /** The tools the model was offered, in the order it was offered them. */ + toolNames: string[]; + /** Characters in the tool schemas alone — usually most of the head. */ + toolChars: number; + /** Characters in the conversation. */ + messageChars: number; + messages: number; + /** A cheap fingerprint of the head. Equal heads → equal fingerprints. */ + headKey: string; +} + +/** How this request compares with the one before it. */ +export interface PayloadDelta { + /** The head is byte-identical to the previous request's. This is the good case. */ + headStable: boolean; + /** Tools the model gained since the previous request. */ + toolsAdded: string[]; + /** Tools it lost. */ + toolsRemoved: string[]; + /** Characters the head grew (negative: shrank). */ + headCharsDelta: number; +} + +/** A head that changed, and everything needed to say who changed it. */ +export interface HeadChurn { + at: number; + /** True when the head changed BETWEEN CALLS OF ONE RUN. */ + midRun: boolean; + /** + * The tools the model was offered are NOT the ones we last set: the list was + * rewritten by something outside this extension. + */ + foreign: boolean; + delta: PayloadDelta; + shape: PayloadShape; +} + +function textOf(value: unknown): string { + if (typeof value === "string") return value; + if (value === undefined || value === null) return ""; + return JSON.stringify(value); +} + +/** + * The name of a tool as the provider was given it. OpenAI-style wraps it + * (`{type:"function", function:{name}}`); Anthropic-style does not (`{name}`). + */ +function toolName(tool: unknown): string { + if (typeof tool !== "object" || tool === null) return "?"; + const record = tool as { name?: unknown; function?: { name?: unknown } }; + const name = record.function?.name ?? record.name; + return typeof name === "string" ? name : "?"; +} + +/** + * Whether a message is the system prompt. OpenAI-completions carries it as the + * first message (`role: "system"`); Anthropic carries it in a `system` field of + * its own, which {@link describePayload} reads separately. + */ +function isSystemMessage(message: unknown): boolean { + if (typeof message !== "object" || message === null) return false; + const role = (message as { role?: unknown }).role; + return role === "system" || role === "developer"; +} + +/** + * A stable fingerprint of a string. Not cryptographic and does not need to be: it + * answers one question — "are these the same bytes?" — for a value we already hold + * in full. + */ +export function fingerprintHead(text: string): string { + let h1 = 0x811c9dc5; + let h2 = 0x01000193; + for (let i = 0; i < text.length; i += 1) { + const c = text.charCodeAt(i); + h1 = Math.imul(h1 ^ c, 0x01000193); + h2 = Math.imul(h2 + c, 0x85ebca6b) ^ (h2 >>> 13); + } + const a = (h1 >>> 0).toString(16).padStart(8, "0"); + const b = (h2 >>> 0).toString(16).padStart(8, "0"); + return `${a}${b}:${text.length}`; +} + +/** Reduce a provider payload to the shape a prefix cache cares about. */ +export function describePayload(payload: unknown): PayloadShape { + const body = ( + typeof payload === "object" && payload !== null ? payload : {} + ) as PayloadLike; + const messages = Array.isArray(body.messages) ? body.messages : []; + const tools = Array.isArray(body.tools) ? body.tools : []; + + const leading = messages.filter(isSystemMessage); + const systemText = textOf(body.system) + leading.map(textOf).join(""); + // No tools → no tool bytes. Faithful to the payload, which omits the key + // entirely when the list is empty, and to the prompt, where it renders to + // nothing. + const toolsText = tools.length > 0 ? JSON.stringify(tools) : ""; + const conversation = messages.filter((message) => !isSystemMessage(message)); + const messageChars = conversation.reduce( + (sum, message) => sum + textOf(message).length, + 0, + ); + + return { + headChars: systemText.length + toolsText.length, + toolNames: tools.map(toolName), + toolChars: toolsText.length, + messageChars, + messages: conversation.length, + headKey: fingerprintHead(`${systemText} ${toolsText}`), + }; +} + +/** Compare a request with the one before it. */ +export function comparePayloads( + previous: PayloadShape, + next: PayloadShape, +): PayloadDelta { + const before = new Set(previous.toolNames); + const after = new Set(next.toolNames); + return { + headStable: previous.headKey === next.headKey, + toolsAdded: next.toolNames.filter((name) => !before.has(name)), + toolsRemoved: previous.toolNames.filter((name) => !after.has(name)), + headCharsDelta: next.headChars - previous.headChars, + }; +} + +/** Whether two tool lists hold the same names (order is not the question here). */ +function sameTools(a: readonly string[], b: readonly string[]): boolean { + if (a.length !== b.length) return false; + const seen = new Set(a); + return b.every((name) => seen.has(name)); +} + +/** + * Watches every provider request and reports the ones whose head changed. + * + * Two questions decide whether a head change is worth reporting, and they are + * different questions: + * + * - **When?** Between runs is expected. Between two calls of ONE run, nothing + * about the model's situation changed and the prefix was thrown away. + * - **Who?** `setActiveTools` is a global setter with no notion of whose tools + * are whose, and this extension is not the only one writing it. A mid-run + * change may be a stranger's — the thing worth naming — or it may be ours, + * which is a cost we chose with our eyes open, not news. + * + * Only a change we did not make is worth an alarm, and an alarm that cannot + * recognise its owner's footsteps is one nobody keeps armed — so {@link record} + * takes the list WE set and reports both answers to the caller, which decides. + * + * It keeps only the previous shape. Nothing here accumulates a history: a + * session-long log of head changes would be a reporting surface, and this + * module has exactly one caller, which reads each churn once and then notifies. + */ +export class PrefixWatch { + private last: PayloadShape | null = null; + /** Requests seen since the current run began — 0 means the next one opens a run. */ + private requestsThisRun = 0; + + constructor(private readonly now: () => number = Date.now) {} + + /** A run started: the next request is its first, so a change there is not mid-run. */ + runStarted(): void { + this.requestsThisRun = 0; + } + + /** + * Record one provider request. Returns the churn it caused, if any. + * + * `ours` is the tool list we last wrote, or `null` when we have not written one + * — the only way to tell a head we rewrote from a head somebody else did. + * Unknown ownership is not an accusation: we cannot claim it and we will not + * blame anyone for it. + */ + record(payload: unknown, ours?: readonly string[] | null): HeadChurn | null { + const shape = describePayload(payload); + const previous = this.last; + this.last = shape; + const midRun = this.requestsThisRun > 0; + this.requestsThisRun += 1; + if (!previous) return null; + const delta = comparePayloads(previous, shape); + if (delta.headStable) return null; + const foreign = ours != null && !sameTools(shape.toolNames, ours); + return { at: this.now(), midRun, foreign, delta, shape }; + } +} + +/** + * The one-line report for a mid-run foreign churn. Local only — the caller passes + * it to `ctx.ui.notify`; nothing is ever sent anywhere. + */ +export function formatHeadChurnWarning(churn: HeadChurn): string { + const moved = + churn.delta.headCharsDelta > 0 + ? `grew ${churn.delta.headCharsDelta}` + : `shrank ${-churn.delta.headCharsDelta}`; + const parts = [ + `Prompt head rewritten mid-run: ${moved} chars (~${Math.round(Math.abs(churn.delta.headCharsDelta) / 4)} tokens).`, + "The prefix cache was discarded and the whole prompt re-read.", + ]; + if (churn.delta.toolsAdded.length > 0) { + parts.push(`Tools added: ${churn.delta.toolsAdded.join(", ")}.`); + } + if (churn.delta.toolsRemoved.length > 0) { + parts.push(`Tools removed: ${churn.delta.toolsRemoved.join(", ")}.`); + } + parts.push("This is a local check — nothing was sent anywhere."); + return parts.join(" "); +} diff --git a/src/runtime/stage-behavior.ts b/src/runtime/stage-behavior.ts index 5ad8347..a83e70a 100644 --- a/src/runtime/stage-behavior.ts +++ b/src/runtime/stage-behavior.ts @@ -486,6 +486,15 @@ export const PLANNER_STAGE_BEHAVIOR = { "planner_reason", "planner_git_commit", "planner_report_stuck", + // Listed wherever planner_report_stuck is: the stuck report opens a debug + // session unconditionally, and without these the step can open one it + // cannot drive or close — which also leaves planner_git_commit blocked by + // assertNoPlannerDebugArtifactsBeforeCommit. Kept in sync by + // runtime/tool-gating-invariant.test.ts. + "planner_debug_strategy", + "planner_debug_probe", + "planner_debug_result", + "planner_debug_cleanup", "planner_skill_create", "planner_skill_update", "planner_exec",