mirror of
https://github.com/wassname/pi-goals.git
synced 2026-09-12 12:50:58 +08:00
Allow reasoned approval of an exact dirty worktree state
Force overrides only cleanliness. Bind approval to Git HEAD/tree, index and dirty/untracked content fingerprints; recheck at CompleteGoal. Require investigative read-only supervision instead of accepting gate errors as experiment blockers.
This commit is contained in:
wassname
@@ -11,7 +11,7 @@ Plan in one Pi session, then do the work there while a stronger visible Pi sessi
|
||||
5. The fork becomes a read-only supervisor. Worker views and supervisor instructions travel over pi-intercom's extension channel, scoped to this plan pairing.
|
||||
6. Ready waits for the supervisor's Intercom readiness message; the worker does not begin before the fork has compacted and started.
|
||||
7. The supervisor compacts again when its context reaches 100k tokens.
|
||||
8. The supervisor records a private approval only after it sees a stopped worker, no active work, a clean commit, evidence, and saved verification output. `CompleteGoal` checks that approval against the exact plan block and Git tree before it ticks `[x]`.
|
||||
8. The supervisor records a private approval only after it sees a stopped worker, no active work, a clean worktree (or an explicit inspected-state override), evidence, and saved verification output. `CompleteGoal` checks that approval against the exact plan block and Git tree before it ticks `[x]`.
|
||||
|
||||
The two Pi sessions are visible. You can switch to the supervisor pane and talk to it directly. Supervisor instructions are shown in full, including in collapsed tool rows; ordinary messages and emitted thinking use Pi's display settings. The supervisor is prompted to give brief progress assessments and use judgment about when to intervene.
|
||||
|
||||
@@ -56,6 +56,14 @@ A new supervisor may still need up to five minutes for initial compaction. Recov
|
||||
|
||||
Model choices are remembered per project and role in `.pi/pi-goals/models/`. Use `/model` in planning, worker, or supervisor sessions to change that role's choice. Ready restores the worker choice after the planning fork is ready. An unavailable saved model stops the transition instead of substituting another. `/goals model <model>` explicitly overrides the supervisor choice for launch. -- Pi/OpenAI
|
||||
|
||||
## Inspected dirty-worktree approval
|
||||
|
||||
The supervisor can call `ApproveGoal` with `force: true` and a nonempty `reason` when preserved unrelated changes would otherwise prevent sign-off. It must inspect the changes first, not commit, reset or delete someone else's work. Force bypasses **only** cleanliness, never evidence, the current stopped view, active/unknown work, or exact goal/HEAD/tree checks.
|
||||
|
||||
The approval JSON stores the reason, NUL-delimited Git status, an index SHA-256 digest and per-dirty/untracked-file content SHA-256 digests (including modes, symlink targets and deletions). `CompleteGoal` requires the same state; even editing an already-dirty file without changing its status invalidates approval. Normal clean approvals behave as before. Git-ignored files and pi-goals' private plan/approval/model paths remain excluded. Dirty submodule/nested-repository directories or other unhashable paths fail closed; there is no recursive submodule override. Fingerprinting reads all included dirty/untracked bytes and can be expensive for large outputs; it does not lock concurrent writers.
|
||||
|
||||
A gate rejection is not automatically an experiment failure or a dependency of other authorized work. The read-only supervisor should inspect the exact error and implementation, distinguish causes with a cheap check, and steer repairs plus safe independent progress instead of repeating an unproductive status check. -- Pi/OpenAI
|
||||
|
||||
## Plan format
|
||||
|
||||
Current goals belong above `## Log`; goal-shaped historical checklists below it are ignored by the widget, approval matching and sign-off. A goal is a checkbox line whose text starts with `goal:`:
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
# Dirty-worktree approval override — Pi/OpenAI
|
||||
|
||||
Baseline: cb4790a. User requests ApproveGoal force and active, investigative supervision. Parent-approved scope: force overrides only the dirty-worktree restriction, with a nonempty reason and unchanged content-bound Git state at CompleteGoal. Never commit unrelated changes automatically, bypass evidence/runtime checks, or operate user panes.
|
||||
|
||||
- [x] goal: explicit force approval permits an inspected dirty state, not arbitrary later changes
|
||||
- Decision: optional force:true plus reason; checkpoint retains existing HEAD/tree/goal checks and records reason, exact porcelain status, index digest and per-dirty/untracked-path content digests/modes. Existing private plan/approval/model exclusions and ignored-file policy remain unchanged.
|
||||
- Decision: no speculative submodule crawler; an unhashable dirty path must fail closed with an inspection error rather than grant an unbound override.
|
||||
- UAT: call real ApproveGoal and CompleteGoal tool handlers against an isolated Git repository. Preserved unrelated tracked edits and untracked outputs pass unchanged; same-status content changes, added/deleted/staged paths, HEAD or goal changes invalidate. Default dirty rejection and force-without-reason rejection remain. Force cannot bypass evidence, current stopped view or active/unknown jobs.
|
||||
- [x] goal: instruct the supervisor to investigate excuses and direct authorized recovery/progress
|
||||
- Decision: exact error and source before inference; competing causes and a cheap discriminating check; read-only supervisor directs worker repairs. Sign-off restriction is not automatically experiment failure or a dependency of separately authorized work. No new authority, spending or mutation tool.
|
||||
- UAT: prompt contract regression plus documented manual scenario (dirty gate mistaken for active jobs); prompt tests do not establish autonomous judgment.
|
||||
|
||||
## Validation/provenance
|
||||
|
||||
Read AGENTS.md and installed Pi extension custom-tool/schema documentation. Tests must unset PI_SUBAGENT_CHILD, PI_GOALS_ROLE and PI_GOALS_EVIDENCE_DIR, or explicitly set a new evidence directory. Two pre-existing dirty review-fixes-native logs must remain untouched. Save complete validation output; commit scoped changes locally only. Independent review is parent-owned and still required.
|
||||
|
||||
## Result
|
||||
|
||||
Implemented and checked: [85 passing tests + typecheck/lint/build](../reviews/20260908-force-validation.txt). Force flow tests call both production tool handlers over two real transport adapters, with mocked Pi host APIs and isolated real Git repositories. They do not touch Herdr panes. Same-status tracked/untracked byte changes and same-status staged-index byte changes invalidate; ordinary clean approval still succeeds. Prompt assertions check the specified reasoning/authority instructions, not actual model behavior.
|
||||
|
||||
Initial focused run passed 45 tests/typecheck but failed two lint rules; both were corrected before the full successful run. [Initial output](../reviews/20260908-force-initial-validation.txt) is retained, not counted as a pass. Before/after checksums in final output prove the two pre-existing dirty logs were unchanged by validation.
|
||||
|
||||
Manual behavioral UAT remains open: present a dirty-worktree rejection alongside a misleading active-job explanation. Require the supervisor to cite the actual loaded check and raw status, inspect the preserved changes, direct a safe authorized fix or justified force approval, and identify independently authorized work without inventing dependencies. No useful-judgment claim from prompt tests.
|
||||
|
||||
No push, no nested review loop, no /goals supervise or noplan work. Parent's w8:p4T functional pane at pinned cb4790a was not touched. Force fingerprints do not lock concurrent writers; ignored/private paths retain prior exclusions; unhashable paths fail closed. Existing unrelated receipt/stale-tool/lifecycle limitations are unchanged.
|
||||
@@ -0,0 +1,105 @@
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 test
|
||||
> vitest run --run test/force-approval-flow.test.ts test/approval.test.ts test/supervisor-session.test.ts test/goals-flow.test.ts --reporter=verbose
|
||||
|
||||
|
||||
RUN v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
|
||||
|
||||
✓ test/approval.test.ts > hashes only the current goal, excluding the log, interview, and their historical goal text 6ms
|
||||
stdout | test/force-approval-flow.test.ts > force ApproveGoal -> CompleteGoal accepts only the reviewed dirty state without committing or modifying it
|
||||
Force UAT: paired real handlers accepted unchanged tracked + untracked dirty content; Git HEAD and user files stayed unchanged.
|
||||
|
||||
✓ test/force-approval-flow.test.ts > force ApproveGoal -> CompleteGoal accepts only the reviewed dirty state without committing or modifying it 108ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > reports actual idle state, invalidates stopped views on start, and stops completed plans 41ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > preserves drafts, records the interview, and keeps planning read-only 20ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > forks a visible supervisor on Ready and keeps the main session as worker 35ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed tracked content 99ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > starts work only after the supervisor launcher resolves 24ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > delivers an Intercom instruction to the worker 25ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > closes the supervisor on clear but keeps the plan file 23ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > accepts only an approval for the exact clean commit and goal block 64ms
|
||||
✓ test/goals-flow.test.ts > process role > keeps subagent children and visible supervisors out of the worker extension 0ms
|
||||
✓ test/goals-flow.test.ts > restores working linkage even when its remembered model is unavailable, and supports explicit recovery 24ms
|
||||
✓ test/goals-flow.test.ts > restores planning linkage even when its remembered model is unavailable, and supports explicit recovery 23ms
|
||||
✓ test/goals-flow.test.ts > shows a missing resumed supervisor, pauses writes, and automatically unpauses when that peer returns 25ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed untracked content 82ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed new untracked 83ms
|
||||
✓ test/goals-flow.test.ts > times out stale Ready retries in five seconds, without replacing the pane automatically 35ms
|
||||
✓ test/goals-flow.test.ts > explicitly restarts only the tracked pane, keeps the plan, and invalidates old approval binding 24ms
|
||||
✓ test/goals-flow.test.ts > does not persist startup results or launch work after session shutdown 21ms
|
||||
✓ test/goals-flow.test.ts > keeps a failed Ready model not-ready and recovers the same real supervisor binding 26ms
|
||||
✓ test/goals-flow.test.ts > points a present-but-paused peer recovery at the supervisor pane 17ms
|
||||
✓ test/goals-flow.test.ts > clear during the initial Ready wait cancels immediately and cannot resurrect the plan 21ms
|
||||
✓ test/goals-flow.test.ts > clear before the launcher resolves rejects late pane callbacks without restoring the binding 21ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed deleted untracked 62ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed index only 65ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed rename 74ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed HEAD 80ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed goal 79ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the evidence gate 44ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > restores monitoring and read-only tools without replaying persisted views 10ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > renders all advice in real Pi tool rows, including collapsed and restored rows 43ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > asks for judgment and useful recaps without inventing instructions 1ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > writes readiness only after removing writing tools 7ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > compacts a large planning fork before writing readiness 2ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > does not become ready when initial compaction fails 2ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > writes a durable worker instruction 2ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the verification gate 47ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the stopped view gate 29ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the tool call gate 21ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the unknown tracker gate 27ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the active tracker gate 28ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > records approval only from a stopped view with evidence and no active work 98ms
|
||||
✓ test/supervisor-session.test.ts > blocks the general intercom actuator even if enabled after startup 2ms
|
||||
✓ test/supervisor-session.test.ts > keeps a supervisor unready after model restoration failure, then recovers explicitly without substituting a model 2ms
|
||||
✓ test/supervisor-session.test.ts > warns once on unavailable usage but stays quiet for Pi's post-compaction null token sample 3ms
|
||||
|
||||
Test Files 4 passed (4)
|
||||
Tests 45 passed (45)
|
||||
Start at 06:14:52
|
||||
Duration 1.54s (transform 492ms, setup 0ms, import 2.27s, tests 1.58s, environment 0ms)
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 typecheck
|
||||
> tsc --noEmit
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 lint
|
||||
> biome check src/ test/
|
||||
|
||||
src/approval.ts:42:7 lint/suspicious/noImplicitAnyLet ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
|
||||
× This variable implicitly has the any type.
|
||||
|
||||
40 │ const files = paths.map((path): WorktreeSnapshot["files"][number] => {
|
||||
41 │ const fullPath = join(repoRoot, path);
|
||||
> 42 │ let stat;
|
||||
│ ^^^^
|
||||
43 │ try { stat = lstatSync(fullPath); }
|
||||
44 │ catch (error) {
|
||||
|
||||
i Variable declarations without type annotation and initialization implicitly have the any type. Declare a type or initialize the variable with some value.
|
||||
|
||||
|
||||
src/approval.ts:56:12 lint/suspicious/noAssignInExpressions ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
|
||||
× The assignment should not be in an expression.
|
||||
|
||||
54 │ const buffer = Buffer.alloc(256 * 1024);
|
||||
55 │ let bytes: number;
|
||||
> 56 │ while ((bytes = readSync(fd, buffer, 0, buffer.length, null)) > 0) hash.update(buffer.subarray(0, bytes));
|
||||
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
57 │ } finally { closeSync(fd); }
|
||||
58 │ return { path, kind: "file", mode, contentHash: hash.digest("hex") };
|
||||
|
||||
i The use of assignments in expressions is confusing.
|
||||
Expressions are often considered as side-effect free.
|
||||
|
||||
|
||||
Checked 33 files in 39ms. No fixes applied.
|
||||
Found 2 errors.
|
||||
check ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
|
||||
|
||||
× Some errors were emitted while running checks.
|
||||
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
Before validation: pre-existing dirty evidence checksums
|
||||
a8f1facd16fd6ff2938195a7adbdc3b4b1e1dea61533e90a39e5eb18d201e190 slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
de5bb7171e71508c40e19f3929303a63f1fa0f2ea7789a2bcc8728ea038310d2 slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
|
||||
Command: env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE -u PI_GOALS_EVIDENCE_DIR npm test -- --reporter=verbose
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 test
|
||||
> vitest run --reporter=verbose
|
||||
|
||||
|
||||
RUN v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
|
||||
|
||||
✓ test/package-agent.test.ts > package manifest > includes the extension without registering a packaged subagent 3ms
|
||||
✓ test/prompts.test.ts > planning prompt > requires fact finding or a focused question before a goal 2ms
|
||||
✓ test/prompts.test.ts > planning prompt > restores the same rule after compaction 1ms
|
||||
✓ test/prompts.test.ts > planning prompt > anchors work and sign-off to the user-visible result 0ms
|
||||
✓ test/worker-view.test.ts > keeps human direction and source location while sending only new messages 3ms
|
||||
✓ test/worker-view.test.ts > restarts after compaction and does not report historical tool calls as active 1ms
|
||||
✓ test/worker-view.test.ts > bounds serialized Unicode and quoted logs while marking omissions 3ms
|
||||
stdout | test/role-models.test.ts > remembers each role without automatic switching overwriting another role
|
||||
Role preferences restored: planning=planner, worker=small-worker, supervisor=astra; restore events did not overwrite the choice.
|
||||
|
||||
✓ test/role-models.test.ts > remembers each role without automatic switching overwriting another role 9ms
|
||||
✓ test/role-models.test.ts > fails on an unavailable remembered model without replacing the choice 4ms
|
||||
✓ test/intercom.test.ts > pi-intercom transport > receives exact advice once, acknowledges it and rejects unrelated peers 15ms
|
||||
✓ test/intercom.test.ts > pi-intercom transport > restores an unacknowledged steer on reconnect and stops replay after acknowledgment 5ms
|
||||
✓ test/intercom.test.ts > pi-intercom transport > advances the incremental overview only after acknowledgment 1ms
|
||||
✓ test/intercom.test.ts > pi-intercom transport > cancels a readiness wait on shutdown 2ms
|
||||
✓ test/intercom.test.ts > does not acknowledge a synchronous handoff failure, and retries the instruction 1ms
|
||||
✓ test/intercom.test.ts > detaches a completed binding and ignores its late advice without replay errors or false acceptance 1ms
|
||||
✓ test/background.test.ts > reports tracked running work, rather than equating idle agent with finished jobs 110ms
|
||||
✓ test/background.test.ts > distinguishes missing providers from an unavailable installed tracker 4ms
|
||||
✓ test/intercom-handshake.test.ts > re-handshakes unchanged peers in either direction without hello ping-pong or lost advice 11ms
|
||||
✓ test/intercom-handshake.test.ts > replays pending advice and views across either role's own readiness transition 1ms
|
||||
✓ test/intercom-handshake.test.ts > cancels pending waits immediately on detach or reconfiguration 2ms
|
||||
✓ test/herdr.test.ts > supervisor pane command > forks the planning session with pi-goals owning its Intercom dependency 8ms
|
||||
✓ test/herdr.test.ts > supervisor pane command > accepts Herdr's text version output and stale pane cleanup 55ms
|
||||
✓ test/approval.test.ts > fingerprints literal unusual paths, binary bytes, symlink targets, modes and deletions 177ms
|
||||
✓ test/approval.test.ts > hashes only the current goal, excluding the log, interview, and their historical goal text 1ms
|
||||
✓ test/fold.test.ts > foldPlan (current goals are above ## Log; durable memory is below it) > keeps the title, user voice and goals 1ms
|
||||
✓ test/fold.test.ts > foldPlan (current goals are above ## Log; durable memory is below it) > drops the log, the learnings and the unlimited appendix 0ms
|
||||
✓ test/fold.test.ts > foldPlan (current goals are above ## Log; durable memory is below it) > returns the whole plan when there is no ## Log yet (a fresh draft) 0ms
|
||||
✓ test/fold.test.ts > openSubtasks (the widget shows the next action, so the plan IS the task list) > lists the active goal's open and in-progress subtasks, stopping at the next goal 1ms
|
||||
✓ test/fold.test.ts > openSubtasks (the widget shows the next action, so the plan IS the task list) > does not leak subtasks from the goal below 1ms
|
||||
✓ test/fold.test.ts > does not show historical Log subtasks under the last active goal 0ms
|
||||
✓ test/append-log.test.ts > appendLog (the extension's only plan-file write) > creates ## Log at EOF when absent 2ms
|
||||
✓ test/append-log.test.ts > appendLog (the extension's only plan-file write) > appends after the last existing log line, before any following header 1ms
|
||||
✓ test/tick-goal.test.ts > tickGoal (sign-off ticks the goal; agent only ticks on wording drift) > ticks the exact-matching goal line, case-insensitive, leaving subtasks alone 2ms
|
||||
✓ test/tick-goal.test.ts > tickGoal (sign-off ticks the goal; agent only ticks on wording drift) > returns null on wording drift (fuzzy matching is the judge's job, not TypeScript's) 0ms
|
||||
✓ test/tick-goal.test.ts > tickGoal (sign-off ticks the goal; agent only ticks on wording drift) > returns null when the subject matches more than one goal line 0ms
|
||||
✓ test/tick-goal.test.ts > tickGoal (sign-off ticks the goal; agent only ticks on wording drift) > ignores a historical duplicate below the Log and leaves it unchanged 0ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > reports actual idle state, invalidates stopped views on start, and stops completed plans 51ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > preserves drafts, records the interview, and keeps planning read-only 25ms
|
||||
stdout | test/intercom-broker.test.ts > exchanges readiness, views and exact advice over a real isolated pi-intercom broker
|
||||
Intercom broker: readiness confirmed; exact worker view and supervisor advice received.
|
||||
|
||||
stdout | test/force-approval-flow.test.ts > force ApproveGoal -> CompleteGoal accepts only the reviewed dirty state without committing or modifying it
|
||||
Force UAT: paired real handlers accepted unchanged tracked + untracked dirty content; Git HEAD and user files stayed unchanged.
|
||||
|
||||
✓ test/force-approval-flow.test.ts > force ApproveGoal -> CompleteGoal accepts only the reviewed dirty state without committing or modifying it 124ms
|
||||
✓ test/intercom-broker.test.ts > exchanges readiness, views and exact advice over a real isolated pi-intercom broker 425ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > forks a visible supervisor on Ready and keeps the main session as worker 27ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > starts work only after the supervisor launcher resolves 28ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > delivers an Intercom instruction to the worker 20ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > closes the supervisor on clear but keeps the plan file 22ms
|
||||
✓ test/force-approval-flow.test.ts > keeps ordinary clean approval unchanged and rechecks runtime state at forced completion 104ms
|
||||
✓ test/goals-flow.test.ts > /goals flow > accepts only an approval for the exact clean commit and goal block 74ms
|
||||
✓ test/goals-flow.test.ts > process role > keeps subagent children and visible supervisors out of the worker extension 0ms
|
||||
✓ test/goals-flow.test.ts > restores working linkage even when its remembered model is unavailable, and supports explicit recovery 27ms
|
||||
✓ test/goals-flow.test.ts > restores planning linkage even when its remembered model is unavailable, and supports explicit recovery 26ms
|
||||
✓ test/goals-flow.test.ts > shows a missing resumed supervisor, pauses writes, and automatically unpauses when that peer returns 25ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed tracked content 78ms
|
||||
✓ test/goals-flow.test.ts > times out stale Ready retries in five seconds, without replacing the pane automatically 32ms
|
||||
✓ test/goals-flow.test.ts > explicitly restarts only the tracked pane, keeps the plan, and invalidates old approval binding 31ms
|
||||
✓ test/goals-flow.test.ts > does not persist startup results or launch work after session shutdown 27ms
|
||||
✓ test/goals-flow.test.ts > keeps a failed Ready model not-ready and recovers the same real supervisor binding 32ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed untracked content 76ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed new untracked 70ms
|
||||
✓ test/goals-flow.test.ts > points a present-but-paused peer recovery at the supervisor pane 21ms
|
||||
✓ test/goals-flow.test.ts > clear during the initial Ready wait cancels immediately and cannot resurrect the plan 27ms
|
||||
✓ test/goals-flow.test.ts > clear before the launcher resolves rejects late pane callbacks without restoring the binding 19ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed deleted untracked 61ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed index only 63ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed index contents 122ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed rename 73ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed HEAD 67ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > restores monitoring and read-only tools without replaying persisted views 9ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > renders all advice in real Pi tool rows, including collapsed and restored rows 26ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > asks for judgment and useful recaps without inventing instructions 1ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > writes readiness only after removing writing tools 2ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > compacts a large planning fork before writing readiness 1ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > does not become ready when initial compaction fails 2ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > writes a durable worker instruction 1ms
|
||||
✓ test/supervisor-session.test.ts > visible supervisor session > records approval only from a stopped view with evidence and no active work 77ms
|
||||
✓ test/supervisor-session.test.ts > blocks the general intercom actuator even if enabled after startup 2ms
|
||||
✓ test/supervisor-session.test.ts > keeps a supervisor unready after model restoration failure, then recovers explicitly without substituting a model 2ms
|
||||
✓ test/supervisor-session.test.ts > warns once on unavailable usage but stays quiet for Pi's post-compaction null token sample 3ms
|
||||
✓ test/force-approval-flow.test.ts > invalidates forced approval after changed goal 86ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the evidence gate 51ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the verification gate 42ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the stopped view gate 24ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the tool call gate 25ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the unknown tracker gate 25ms
|
||||
✓ test/force-approval-flow.test.ts > force does not bypass the active tracker gate 27ms
|
||||
✓ test/rpc-review.test.ts > RPC review flow > opens Refine's editor before it starts the revision turn 1856ms
|
||||
stdout | test/native-intercom.test.ts > runs a forked Pi supervisor and receives its exact instruction in another Pi session
|
||||
Native Pi pair: fork retained planning context; SteerWorker delivered exactly: Read the real outputs before declaring completion.
|
||||
|
||||
✓ test/native-intercom.test.ts > runs a forked Pi supervisor and receives its exact instruction in another Pi session 2941ms
|
||||
|
||||
Test Files 18 passed (18)
|
||||
Tests 85 passed (85)
|
||||
Start at 06:18:03
|
||||
Duration 3.48s (transform 3.77s, setup 0ms, import 7.89s, tests 7.45s, environment 2ms)
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 typecheck
|
||||
> tsc --noEmit
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 lint
|
||||
> biome check src/ test/
|
||||
|
||||
Checked 33 files in 68ms. No fixes applied.
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 build
|
||||
> tsc
|
||||
|
||||
|
||||
After validation: pre-existing dirty evidence checksums
|
||||
a8f1facd16fd6ff2938195a7adbdc3b4b1e1dea61533e90a39e5eb18d201e190 slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
de5bb7171e71508c40e19f3929303a63f1fa0f2ea7789a2bcc8728ea038310d2 slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
+53
-9
@@ -1,10 +1,16 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { createHash } from "node:crypto";
|
||||
import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs";
|
||||
import { closeSync, existsSync, lstatSync, mkdirSync, openSync, readFileSync, readlinkSync, readSync, renameSync, rmSync, type Stats, statSync, writeFileSync } from "node:fs";
|
||||
import { dirname, join, relative, resolve } from "node:path";
|
||||
|
||||
import { foldPlan, GOAL_LINE } from "./plan.js";
|
||||
|
||||
export interface WorktreeSnapshot {
|
||||
status: string;
|
||||
indexHash: string;
|
||||
files: Array<{ path: string; kind: "file" | "symlink" | "missing"; mode?: number; contentHash?: string }>;
|
||||
}
|
||||
|
||||
export interface ApprovalRecord {
|
||||
version: 3;
|
||||
verdict: "accept";
|
||||
@@ -15,7 +21,8 @@ export interface ApprovalRecord {
|
||||
repoRoot: string;
|
||||
head: string;
|
||||
tree: string;
|
||||
cleanWorktree: true;
|
||||
cleanWorktree: boolean;
|
||||
force?: { reason: string; worktree: WorktreeSnapshot };
|
||||
inspected: { plan: true; repository: true; evidence: true; verifyOutput: true };
|
||||
verifyOutputPath: string;
|
||||
supervisor: { sessionId: string; runId: string | null };
|
||||
@@ -26,19 +33,52 @@ function command(repoRoot: string, args: string[]): string {
|
||||
return execFileSync("git", args, { cwd: repoRoot, encoding: "utf8" }).trim();
|
||||
}
|
||||
|
||||
export function repositoryState(cwd: string): { repoRoot: string; head: string; tree: string; cleanWorktree: boolean } {
|
||||
// Capture bytes, not status flags: an edited file can change again while Git still reports M or ??.
|
||||
function worktreeSnapshot(repoRoot: string, status: string, pathspec: string[]): WorktreeSnapshot {
|
||||
const index = execFileSync("git", ["ls-files", "--stage", "-z", "--", ...pathspec], { cwd: repoRoot });
|
||||
const paths = status.split("\0").filter(Boolean).map(entry => entry.slice(3)).sort();
|
||||
const files = paths.map((path): WorktreeSnapshot["files"][number] => {
|
||||
const fullPath = join(repoRoot, path);
|
||||
let stat: Stats;
|
||||
try { stat = lstatSync(fullPath); }
|
||||
catch (error) {
|
||||
if (["ENOENT", "ENOTDIR"].includes((error as NodeJS.ErrnoException).code ?? "")) return { path, kind: "missing" };
|
||||
throw error;
|
||||
}
|
||||
const mode = stat.mode & 0o777;
|
||||
const hash = createHash("sha256");
|
||||
if (stat.isSymbolicLink()) return { path, kind: "symlink", mode, contentHash: hash.update(readlinkSync(fullPath, { encoding: "buffer" })).digest("hex") };
|
||||
if (!stat.isFile()) throw new Error(`Cannot fingerprint dirty path ${path}: only regular files and symlinks are supported.`);
|
||||
const fd = openSync(fullPath, "r");
|
||||
try {
|
||||
const buffer = Buffer.alloc(256 * 1024);
|
||||
for (;;) {
|
||||
const bytes = readSync(fd, buffer, 0, buffer.length, null);
|
||||
if (!bytes) break;
|
||||
hash.update(buffer.subarray(0, bytes));
|
||||
}
|
||||
} finally { closeSync(fd); }
|
||||
return { path, kind: "file", mode, contentHash: hash.digest("hex") };
|
||||
});
|
||||
return { status, indexHash: createHash("sha256").update(index).digest("hex"), files };
|
||||
}
|
||||
|
||||
export function repositoryState(cwd: string, captureWorktree = false): { repoRoot: string; head: string; tree: string; cleanWorktree: boolean; worktree?: WorktreeSnapshot } {
|
||||
const repoRoot = command(cwd, ["rev-parse", "--show-toplevel"]);
|
||||
const head = command(repoRoot, ["rev-parse", "HEAD"]);
|
||||
const tree = command(repoRoot, ["rev-parse", "HEAD^{tree}"]);
|
||||
const prefix = relative(repoRoot, resolve(cwd)).replaceAll("\\", "/");
|
||||
const owned = prefix ? `${prefix}/.pi` : ".pi";
|
||||
const cleanWorktree = command(repoRoot, [
|
||||
"status", "--porcelain=v1", "--untracked-files=all", "--", ".",
|
||||
const pathspec = [".",
|
||||
`:(exclude,glob)${owned}/plan/*.md`,
|
||||
`:(exclude,glob)${owned}/pi-goals/approvals/*`,
|
||||
`:(exclude,glob)${owned}/pi-goals/models/*`,
|
||||
]) === "";
|
||||
return { repoRoot, head, tree, cleanWorktree };
|
||||
];
|
||||
// NUL delimiters and no rename folding preserve whitespace/newlines and both sides of renames.
|
||||
const raw = execFileSync("git", ["status", "--porcelain=v1", "-z", "--no-renames", ...(captureWorktree ? ["--ignore-submodules=none"] : []), "--untracked-files=all", "--", ...pathspec], { cwd: repoRoot });
|
||||
const status = raw.toString("utf8");
|
||||
if (captureWorktree && !raw.equals(Buffer.from(status))) throw new Error("Cannot fingerprint non-UTF-8 Git paths.");
|
||||
return { repoRoot, head, tree, cleanWorktree: status === "", ...(captureWorktree ? { worktree: worktreeSnapshot(repoRoot, status, pathspec) } : {}) };
|
||||
}
|
||||
|
||||
export function goalBlock(plan: string, goal: string): string | null {
|
||||
@@ -112,6 +152,7 @@ export function approvalMatches(record: ApprovalRecord | null, input: {
|
||||
head: string;
|
||||
tree: string;
|
||||
cleanWorktree: boolean;
|
||||
worktree?: WorktreeSnapshot;
|
||||
}): boolean {
|
||||
return record?.version === 3
|
||||
&& record.verdict === "accept"
|
||||
@@ -122,8 +163,11 @@ export function approvalMatches(record: ApprovalRecord | null, input: {
|
||||
&& resolve(record.repoRoot) === resolve(input.repoRoot)
|
||||
&& record.head === input.head
|
||||
&& record.tree === input.tree
|
||||
&& record.cleanWorktree === true
|
||||
&& input.cleanWorktree
|
||||
&& (record.force
|
||||
? Boolean(record.force.reason?.trim()) && Boolean(record.force.worktree) && Boolean(input.worktree)
|
||||
&& record.cleanWorktree === input.cleanWorktree
|
||||
&& JSON.stringify(record.force.worktree) === JSON.stringify(input.worktree)
|
||||
: record.cleanWorktree === true && input.cleanWorktree)
|
||||
&& record.inspected.plan === true
|
||||
&& record.inspected.repository === true
|
||||
&& record.inspected.evidence === true
|
||||
|
||||
+5
-4
@@ -407,7 +407,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
if (paused) return { systemPrompt: `${ctx.getSystemPrompt()}\n\nGoal work is paused: ${paused} Do not implement or sign off goals. Human input and read-only diagnosis remain available; wait for recovery before resuming autonomous work.` };
|
||||
if (state.phase === "working") {
|
||||
return {
|
||||
systemPrompt: `${ctx.getSystemPrompt()}\n\nYou are the implementation worker for ${planRel(ctx)}. Keep the full conversation and do the work directly. A stronger read-only supervisor watches this session through pi-intercom and can steer you. Commit clean evidence before asking for sign-off. Stop when a goal appears complete so the supervisor can inspect a settled worker view. Call CompleteGoal only after the supervisor says it recorded approval. -- PI[Kimi K3]`,
|
||||
systemPrompt: `${ctx.getSystemPrompt()}\n\nYou are the implementation worker for ${planRel(ctx)}. Keep the full conversation and do the work directly. A stronger read-only supervisor watches this session through pi-intercom and can steer you. Commit your evidence before asking for sign-off; never commit or discard unrelated changes to satisfy the clean-worktree gate. The supervisor can explicitly accept an inspected unchanged dirty state with ApproveGoal force and a reason. Stop when a goal appears complete so the supervisor can inspect a settled worker view. Call CompleteGoal only after the supervisor says it recorded approval. -- PI[Kimi K3]`,
|
||||
};
|
||||
}
|
||||
if (!planningContextPending) return;
|
||||
@@ -611,14 +611,14 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
if (!plan.trim()) return result(`No plan file at ${planRel(ctx)}. Run /goals to draft one.`, true);
|
||||
const block = goalBlock(plan, params.goal);
|
||||
if (!block) return result(`No unique open goal line matched "${params.goal}" in ${planRel(ctx)}.`, true);
|
||||
const approval = readApproval(approvalPath(ctx.cwd, ctx.sessionManager.getSessionId(), params.goal));
|
||||
let repository: ReturnType<typeof repositoryState>;
|
||||
try {
|
||||
repository = repositoryState(ctx.cwd);
|
||||
repository = repositoryState(ctx.cwd, Boolean(approval?.force));
|
||||
} catch (error) {
|
||||
return result(`Goal sign-off could not inspect the repository: ${error instanceof Error ? error.message : String(error)}`, true);
|
||||
}
|
||||
if (!repository.cleanWorktree) return result("Goal sign-off blocked: worktree is dirty.", true);
|
||||
const approval = readApproval(approvalPath(ctx.cwd, ctx.sessionManager.getSessionId(), params.goal));
|
||||
if (!repository.cleanWorktree && !approval?.force) return result("Goal sign-off blocked: worktree is dirty. Request supervisor inspection, not an unrelated cleanup commit.", true);
|
||||
if (!approvalMatches(approval, {
|
||||
approvalId: state.approvalId,
|
||||
goal: params.goal,
|
||||
@@ -628,6 +628,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
|
||||
head: repository.head,
|
||||
tree: repository.tree,
|
||||
cleanWorktree: repository.cleanWorktree,
|
||||
worktree: repository.worktree,
|
||||
})) return result("Goal sign-off blocked: no matching supervisor approval checkpoint. Request a fresh supervisor review.", true);
|
||||
const ticked = tickGoal(plan, params.goal);
|
||||
if (!ticked) return result(`No unique exact goal line matched "${params.goal}" in ${planRel(ctx)}.`, true);
|
||||
|
||||
@@ -74,6 +74,8 @@ Your job is diligent, autonomous supervision: use independent judgment to help t
|
||||
|
||||
Treat "blocked", "waiting", "impossible", and "already done" as claims to verify. Identify the actual dependency and check whether it applies to this task. Read the relevant evidence yourself, or steer the worker to obtain it. Consider a mistaken assumption, a code or harness bug, or another authorized route before accepting a stopping reason. For example, a paused shared-local-GPU queue need not block a Modal remote-GPU job. Check the command's resource use and existing launch status; redirect unstarted remote work without unpausing the shared queue, duplicating a paid job, or exceeding the approved budget.
|
||||
|
||||
When a gate rejects an action, obtain the exact tool error, loaded implementation/version, and relevant source or runtime records before naming the blocker. Distinguish a sign-off failure from an experiment failure: dirty Git state is not evidence of active jobs, and a missing transcript result is not proof a tool is running. Compare plausible causes and request a cheap discriminating check with predicted outcomes. Do not accept a worker's excuse at face value or repeat interval/status checks that cannot change the state. Stay read-only: use SteerWorker to direct a concrete authorized repair and its verification. Identify independent work that can proceed safely in parallel; do not assume formal sign-off blocks the next already-authorized experiment unless the plan or user actually requires that dependency. Do not duplicate running jobs or exceed scope, permissions or budget.
|
||||
|
||||
Keep authorized work moving. Resolve technical choices within the agreed scope yourself. If idle with unfinished goals, use SteerWorker for a concrete next step or diagnostic check. If useful work is running, do not invent work or repeat an instruction already awaiting execution. Waiting is warranted when a verified dependency remains; identify what event will resume progress and how it will be observed. Escalate only a specific unresolved human decision, permission, credential, or spending need after checking what is already authorized. Do not dismiss genuine limits or expand scope to avoid reporting a blocker.
|
||||
|
||||
At each review, give a brief visible recap of how work is tracking against the goal: what the evidence shows and your judgment about the next step. Add perspective rather than repeating status. Distinguish observations from guesses. Keep routine recaps short, but do not suppress useful explanation or thinking. Do not edit files or execute the worker's work.
|
||||
@@ -82,7 +84,7 @@ Ground consequential judgments in verbatim evidence with a source path or link a
|
||||
|
||||
For a surprising result or stalled investigation, compare plausible explanations, including an implementation or evaluation bug and a confound. Choose a cheap check whose outcomes distinguish them, and state the predictions before requesting it. For ML results, inspect actual inputs and full outputs alongside metrics and relevant baselines or controls; a passing smoke test does not establish scientific validity. One failed implementation does not refute the idea. Ask the worker to improve checks in the real pipeline rather than build a separate diagnostic implementation. Use ml-debug and varglight for deeper investigation when available; routine reviews need only the decisive evidence and next action.
|
||||
|
||||
Before approving a goal, inspect its exact plan block, repository state, cited evidence, and a saved nonempty verification-output file. Challenge success claims as carefully as blocker claims: check that the artifact demonstrates the discriminator rather than merely existing or repeating a claimed pass. A stopped view means Pi is idle, not that background jobs have finished. Inspect saved job status when work was delegated or launched in the background; withhold approval if its state is unknown. When the discriminator is positively satisfied and no work is active, call ApproveGoal with that repository-relative path. Then call SteerWorker and tell the worker to call CompleteGoal with the exact goal text. When every goal is completed or cancelled, give a short final assessment and stop issuing instructions. -- Pi/OpenAI`;
|
||||
Before approving a goal, inspect its exact plan block, repository state, cited evidence, and a saved nonempty verification-output file. Challenge success claims as carefully as blocker claims: check that the artifact demonstrates the discriminator rather than merely existing or repeating a claimed pass. A stopped view means Pi is idle, not that background jobs have finished. Inspect saved job status when work was delegated or launched in the background; withhold approval if its state is unknown. When the discriminator is positively satisfied and no work is active, call ApproveGoal with that repository-relative path. If only unrelated preserved worktree changes prevent sign-off, inspect their actual diff/content and provenance instead of committing, deleting or resetting them. ApproveGoal(force: true, reason: "...") overrides only cleanliness: explain why accepting this exact dirty state is justified. It does not bypass evidence, stopped-worker, active-work, HEAD/tree or goal checks. The checkpoint records the reason and content fingerprints; any later change needs a fresh review. Then call SteerWorker and tell the worker to call CompleteGoal with the exact goal text. When every goal is completed or cancelled, give a short final assessment and stop issuing instructions. -- Pi/OpenAI`;
|
||||
}
|
||||
|
||||
export function isVisibleSupervisor(): boolean {
|
||||
@@ -214,13 +216,18 @@ export function registerVisibleSupervisor(pi: ExtensionAPI): void {
|
||||
name: "ApproveGoal",
|
||||
label: "Approve goal",
|
||||
executionMode: "sequential",
|
||||
description: "Record approval after inspecting the current goal, repository, evidence, and a saved nonempty verification-output file, with a stopped worker view and no active work.",
|
||||
description: "Record approval after inspecting the current goal, repository, evidence, and a saved nonempty verification-output file, with a stopped worker view and no active work. force overrides only dirty-worktree rejection and requires a reason; later Git/content changes invalidate it.",
|
||||
parameters: Type.Object({
|
||||
goal: Type.String({ description: "Exact text after goal: in the plan." }),
|
||||
verifyOutputPath: Type.String({ description: "Nonempty repository-relative file containing the verification output you inspected." }),
|
||||
force: Type.Optional(Type.Boolean({ description: "Accept this exact inspected dirty worktree, without bypassing any other approval gate." })),
|
||||
reason: Type.Optional(Type.String({ description: "Required with force:true. Why accepting these inspected worktree changes is justified." })),
|
||||
}),
|
||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||
if (modelError) return result(`Supervisor paused: ${modelError} Use /model, then /goals reconnect.`, true);
|
||||
const force = params.force === true;
|
||||
const reason = params.reason?.trim();
|
||||
if (force && !reason) return result("Cannot force approval without an explicit nonempty reason for accepting this worktree state.", true);
|
||||
const view = latestWorkerView(ctx);
|
||||
const newest = intercom.latestView;
|
||||
if (!intercom.connected || !newest || view !== newest.text) return result("Cannot approve without inspecting the latest worker view.", true);
|
||||
@@ -233,11 +240,11 @@ export function registerVisibleSupervisor(pi: ExtensionAPI): void {
|
||||
let repository: ReturnType<typeof repositoryState>;
|
||||
try {
|
||||
plan = readFileSync(settings.planPath, "utf8");
|
||||
repository = repositoryState(ctx.cwd);
|
||||
repository = repositoryState(ctx.cwd, force);
|
||||
} catch (error) {
|
||||
return result(`Cannot inspect approval inputs: ${error instanceof Error ? error.message : String(error)}`, true);
|
||||
}
|
||||
if (!repository.cleanWorktree) return result("Cannot approve with a dirty worktree. Commit the worker changes first.", true);
|
||||
if (!repository.cleanWorktree && !force) return result("Cannot approve with a dirty worktree. Commit only the worker's changes, or inspect preserved changes and use force:true with a reason. Do not commit unrelated changes to satisfy this gate.", true);
|
||||
const block = goalBlock(plan, params.goal);
|
||||
if (!block) return result(`Cannot approve: no unique open goal matches "${params.goal}".`, true);
|
||||
if (!hasEvidenceEntry(block)) return result("Cannot approve without a nonblank evidence entry in the goal block.", true);
|
||||
@@ -247,10 +254,10 @@ export function registerVisibleSupervisor(pi: ExtensionAPI): void {
|
||||
writeApproval(path, {
|
||||
version: 3, verdict: "accept", approvalId: settings.approvalId, goal: params.goal, planPath: settings.planPath,
|
||||
goalBlockHash: hashGoalBlock(block), repoRoot: repository.repoRoot, head: repository.head, tree: repository.tree,
|
||||
cleanWorktree: true, inspected: { plan: true, repository: true, evidence: true, verifyOutput: true }, verifyOutputPath: verifiedOutput,
|
||||
cleanWorktree: repository.cleanWorktree, ...(force ? { force: { reason: reason!, worktree: repository.worktree! } } : {}), inspected: { plan: true, repository: true, evidence: true, verifyOutput: true }, verifyOutputPath: verifiedOutput,
|
||||
supervisor: { sessionId: ctx.sessionManager.getSessionId(), runId: null }, timestamp: new Date().toISOString(),
|
||||
});
|
||||
return result(`Approval recorded for "${params.goal}". Now steer the worker to call CompleteGoal.`);
|
||||
return result(`Approval recorded for "${params.goal}".${force ? ` Forced worktree acceptance: ${reason}. Exact status and content fingerprints saved in ${path}; changes require fresh review.` : ""} Now steer the worker to call CompleteGoal.`);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
+31
-1
@@ -1,5 +1,35 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { chmodSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { expect, it } from "vitest";
|
||||
import { goalBlock, hashGoalBlock } from "../src/approval.js";
|
||||
import { goalBlock, hashGoalBlock, repositoryState } from "../src/approval.js";
|
||||
|
||||
it("fingerprints literal unusual paths, binary bytes, symlink targets, modes and deletions", () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "goals-fingerprint-"));
|
||||
const git = (...args: string[]) => execFileSync("git", args, { cwd });
|
||||
try {
|
||||
git("init", "-q");
|
||||
writeFileSync(join(cwd, "tracked"), "original"); git("add", ".");
|
||||
git("-c", "user.name=test", "-c", "user.email=test@example.com", "commit", "-qm", "base");
|
||||
const path = " white space\nname ";
|
||||
writeFileSync(join(cwd, path), Buffer.from([0, 255, 42]));
|
||||
symlinkSync("absent target", join(cwd, "link"));
|
||||
const first = repositoryState(cwd, true).worktree!;
|
||||
expect(first.files.map(file => file.path)).toContain(path);
|
||||
expect(first.files.find(file => file.path === "link")?.kind).toBe("symlink");
|
||||
writeFileSync(join(cwd, path), Buffer.from([0, 254, 42]));
|
||||
const changed = repositoryState(cwd, true).worktree!;
|
||||
expect(changed.status).toBe(first.status);
|
||||
expect(changed.files).not.toEqual(first.files);
|
||||
chmodSync(join(cwd, path), 0o700);
|
||||
expect(repositoryState(cwd, true).worktree?.files).not.toEqual(changed.files);
|
||||
rmSync(join(cwd, "link")); symlinkSync("other target", join(cwd, "link"));
|
||||
expect(repositoryState(cwd, true).worktree?.files.find(file => file.path === "link")?.contentHash).not.toBe(first.files.find(file => file.path === "link")?.contentHash);
|
||||
rmSync(join(cwd, "tracked"));
|
||||
expect(repositoryState(cwd, true).worktree?.files).toContainEqual({ path: "tracked", kind: "missing" });
|
||||
} finally { rmSync(cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("hashes only the current goal, excluding the log, interview, and their historical goal text", () => {
|
||||
const goal = "1. [ ] goal: output\n - evidence: output.txt\n";
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { approvalPath, readApproval } from "../src/approval.js";
|
||||
import goals from "../src/index.js";
|
||||
import { registerVisibleSupervisor } from "../src/supervisor-session.js";
|
||||
import { pairedIntercomFixture } from "./paired-intercom-fixture.js";
|
||||
|
||||
const goal = "inspect outputs";
|
||||
const reason = "Inspected preserved notebook edits and historical outputs; unrelated to this goal.";
|
||||
const settle = () => new Promise(resolve => setImmediate(resolve));
|
||||
|
||||
async function setup() {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "goals-force-approval-"));
|
||||
const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim();
|
||||
writeFileSync(join(cwd, "verify.txt"), "PASS: actual output checked\n");
|
||||
writeFileSync(join(cwd, "notebook.py"), "original\n");
|
||||
git("init", "-q"); git("add", ".");
|
||||
git("-c", "user.name=test", "-c", "user.email=test@example.com", "commit", "-qm", "verified output");
|
||||
const head = git("rev-parse", "HEAD");
|
||||
const planPath = join(cwd, ".pi/plan/worker-v1.md");
|
||||
mkdirSync(join(cwd, ".pi/plan"), { recursive: true });
|
||||
const plan = `1. [ ] goal: ${goal}\n - evidence: verify.txt\n\n## Log\n`;
|
||||
writeFileSync(planPath, plan);
|
||||
const wire = pairedIntercomFixture();
|
||||
function runtime(role: "worker" | "supervisor") {
|
||||
const entries: any[] = role === "worker" ? [{ type: "custom", customType: "pi-goals-state", data: { phase: "working", approvalId: "force-binding", planVersion: 1, supervisorPaneId: "fixture-only" } }] : [];
|
||||
const branch: any[] = [];
|
||||
const hooks = new Map<string, any>();
|
||||
const tools = new Map<string, any>();
|
||||
const ctx = {
|
||||
cwd, hasUI: true, isIdle: vi.fn(() => true), model: { provider: "test", id: "model" },
|
||||
modelRegistry: { find: (provider: string, id: string) => ({ provider, id }) },
|
||||
getSystemPrompt: () => "base", getContextUsage: () => ({ tokens: 10 }),
|
||||
sessionManager: { getSessionId: () => role, getSessionFile: () => join(cwd, `${role}.jsonl`), getEntries: () => entries, getBranch: () => branch },
|
||||
ui: { notify: vi.fn(), setWidget: vi.fn(), setStatus: vi.fn(), theme: { fg: (_kind: string, text: string) => text } },
|
||||
};
|
||||
const pi = {
|
||||
events: wire[role].events, getAllTools: vi.fn((): any[] => []), getActiveTools: () => ["read", "ApproveGoal", "SteerWorker"], setActiveTools: () => {},
|
||||
on: (name: string, fn: any) => { const prior = hooks.get(name); hooks.set(name, async (...args: any[]) => { await prior?.(...args); return fn(...args); }); },
|
||||
registerTool: (tool: any) => tools.set(tool.name, tool), registerCommand: () => {}, setModel: async () => true,
|
||||
appendEntry: (customType: string, data: unknown) => entries.push({ type: "custom", customType, data }),
|
||||
sendUserMessage: (text: string) => branch.push({ type: "message", message: { role: "user", content: [{ type: "text", text }] } }),
|
||||
};
|
||||
(role === "worker" ? goals : registerVisibleSupervisor)(pi as unknown as ExtensionAPI);
|
||||
return { pi, ctx, hooks, branch, tools };
|
||||
}
|
||||
const worker = runtime("worker");
|
||||
vi.stubEnv("PI_GOALS_WORKER_ID", "worker"); vi.stubEnv("PI_GOALS_OWNER_SESSION_ID", "worker");
|
||||
vi.stubEnv("PI_GOALS_APPROVAL_ID", "force-binding"); vi.stubEnv("PI_GOALS_PLAN_PATH", planPath);
|
||||
const supervisor = runtime("supervisor");
|
||||
await worker.hooks.get("session_start")({}, worker.ctx);
|
||||
await supervisor.hooks.get("session_start")({}, supervisor.ctx);
|
||||
await settle(); await settle();
|
||||
writeFileSync(join(cwd, "notebook.py"), "preserved user edit\n");
|
||||
writeFileSync(join(cwd, "historical output.txt"), "historical result\n");
|
||||
const approve = (params: object = { force: true, reason }) => supervisor.tools.get("ApproveGoal").execute("approve", { goal, verifyOutputPath: "verify.txt", ...params }, undefined, undefined, supervisor.ctx);
|
||||
const complete = () => worker.tools.get("CompleteGoal").execute("complete", { goal }, undefined, undefined, worker.ctx);
|
||||
return {
|
||||
cwd, git, head, planPath, plan, worker, supervisor, approve, complete,
|
||||
checkpoint: approvalPath(cwd, "worker", goal),
|
||||
close: async () => { await worker.hooks.get("session_shutdown")(); await supervisor.hooks.get("session_shutdown")(); vi.unstubAllEnvs(); rmSync(cwd, { recursive: true, force: true }); },
|
||||
};
|
||||
}
|
||||
|
||||
it("force ApproveGoal -> CompleteGoal accepts only the reviewed dirty state without committing or modifying it", async () => {
|
||||
const flow = await setup();
|
||||
try {
|
||||
expect((await flow.approve({})).isError).toBe(true);
|
||||
for (const reason of [undefined, "", " "]) expect((await flow.approve({ force: true, reason })).content[0].text).toContain("nonempty reason");
|
||||
expect(existsSync(flow.checkpoint)).toBe(false);
|
||||
const status = flow.git("status", "--porcelain=v1");
|
||||
expect((await flow.approve()).isError).toBe(false);
|
||||
const record = readApproval(flow.checkpoint)!;
|
||||
expect(record.cleanWorktree).toBe(false);
|
||||
expect(record.force?.reason).toBe(reason);
|
||||
expect(record.force?.worktree.status).toContain("?? historical output.txt\0");
|
||||
expect(record.force?.worktree.files).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ path: "notebook.py", kind: "file", contentHash: expect.stringMatching(/^[a-f0-9]{64}$/) }),
|
||||
expect.objectContaining({ path: "historical output.txt", kind: "file", contentHash: expect.stringMatching(/^[a-f0-9]{64}$/) }),
|
||||
]));
|
||||
expect((await flow.complete()).isError).toBe(false);
|
||||
expect(readFileSync(flow.planPath, "utf8")).toContain(`[x] goal: ${goal}`);
|
||||
expect(readFileSync(join(flow.cwd, "notebook.py"), "utf8")).toBe("preserved user edit\n");
|
||||
expect(readFileSync(join(flow.cwd, "historical output.txt"), "utf8")).toBe("historical result\n");
|
||||
expect(flow.git("rev-parse", "HEAD")).toBe(flow.head);
|
||||
expect(flow.git("status", "--porcelain=v1")).toBe(status);
|
||||
console.log("Force UAT: paired real handlers accepted unchanged tracked + untracked dirty content; Git HEAD and user files stayed unchanged.");
|
||||
} finally { await flow.close(); }
|
||||
});
|
||||
|
||||
it("keeps ordinary clean approval unchanged and rechecks runtime state at forced completion", async () => {
|
||||
const flow = await setup();
|
||||
try {
|
||||
expect((await flow.approve()).isError).toBe(false);
|
||||
flow.worker.pi.getAllTools.mockReturnValue([{ name: "process" }]);
|
||||
expect((await flow.complete()).isError).toBe(true); // Installed tracker is now unavailable.
|
||||
flow.worker.pi.getAllTools.mockReturnValue([]);
|
||||
writeFileSync(join(flow.cwd, "notebook.py"), "original\n");
|
||||
rmSync(join(flow.cwd, "historical output.txt"));
|
||||
expect((await flow.complete()).isError).toBe(true); // Cleaning up also changes the accepted state.
|
||||
expect((await flow.approve({})).isError).toBe(false);
|
||||
expect(readApproval(flow.checkpoint)?.force).toBeUndefined();
|
||||
expect((await flow.complete()).isError).toBe(false);
|
||||
} finally { await flow.close(); }
|
||||
});
|
||||
|
||||
it.each(["tracked content", "untracked content", "new untracked", "deleted untracked", "index only", "index contents", "rename", "HEAD", "goal"])("invalidates forced approval after changed %s", async change => {
|
||||
const flow = await setup();
|
||||
try {
|
||||
expect((await flow.approve()).isError).toBe(false);
|
||||
if (change === "index contents") {
|
||||
flow.git("add", "notebook.py");
|
||||
writeFileSync(join(flow.cwd, "notebook.py"), "separate worktree\n");
|
||||
expect((await flow.approve()).isError).toBe(false);
|
||||
}
|
||||
const status = flow.git("status", "--porcelain=v1");
|
||||
if (change === "tracked content") writeFileSync(join(flow.cwd, "notebook.py"), "changed user edits\n");
|
||||
if (change === "untracked content") writeFileSync(join(flow.cwd, "historical output.txt"), "changed old output\n");
|
||||
if (change === "new untracked") writeFileSync(join(flow.cwd, "another.txt"), "new");
|
||||
if (change === "deleted untracked") rmSync(join(flow.cwd, "historical output.txt"));
|
||||
if (change === "index only") flow.git("add", "notebook.py");
|
||||
if (change === "index contents") {
|
||||
writeFileSync(join(flow.cwd, "notebook.py"), "different staged bytes\n");
|
||||
flow.git("add", "notebook.py");
|
||||
writeFileSync(join(flow.cwd, "notebook.py"), "separate worktree\n");
|
||||
expect(flow.git("status", "--porcelain=v1")).toBe(status);
|
||||
}
|
||||
if (change === "rename") renameSync(join(flow.cwd, "notebook.py"), join(flow.cwd, "renamed.py"));
|
||||
if (change === "HEAD") flow.git("-c", "user.name=test", "-c", "user.email=test@example.com", "commit", "--allow-empty", "-qm", "new head");
|
||||
if (change === "goal") writeFileSync(flow.planPath, flow.plan.replace("evidence: verify.txt", "evidence: different.txt"));
|
||||
if (change.endsWith(" content")) expect(flow.git("status", "--porcelain=v1")).toBe(status);
|
||||
expect((await flow.complete()).isError).toBe(true);
|
||||
expect(readFileSync(flow.planPath, "utf8")).toContain(`[ ] goal: ${goal}`);
|
||||
} finally { await flow.close(); }
|
||||
});
|
||||
|
||||
it.each(["evidence", "verification", "stopped view", "tool call", "unknown tracker", "active tracker"])("force does not bypass the %s gate", async gate => {
|
||||
const flow = await setup();
|
||||
try {
|
||||
if (gate === "evidence") writeFileSync(flow.planPath, flow.plan.replace("evidence: verify.txt", "evidence: (empty until sign-off)"));
|
||||
if (gate === "verification") writeFileSync(join(flow.cwd, "verify.txt"), "");
|
||||
if (gate === "stopped view") { flow.worker.ctx.isIdle.mockReturnValue(false); await flow.worker.hooks.get("agent_start")({}, flow.worker.ctx); }
|
||||
if (gate === "tool call") flow.worker.branch.push({ type: "message", message: { role: "assistant", content: [{ type: "toolCall", id: "pending", name: "edit" }] } });
|
||||
if (gate.endsWith("tracker")) {
|
||||
flow.worker.pi.getAllTools.mockReturnValue([{ name: "process" }]);
|
||||
if (gate === "active tracker") {
|
||||
const emit = flow.worker.pi.events.emit;
|
||||
flow.worker.pi.events.emit = (name, request) => { if (name === "processes:request:list") { request.reply([{ status: "running", name: "existing-job" }]); return true; } return emit(name, request); };
|
||||
}
|
||||
}
|
||||
await flow.worker.hooks.get("agent_settled")({}, flow.worker.ctx); await settle();
|
||||
const response = await flow.approve();
|
||||
expect(response.isError).toBe(true);
|
||||
expect(existsSync(flow.checkpoint)).toBe(false);
|
||||
if (gate.endsWith("tracker")) expect(response.content[0].text).toContain("background work");
|
||||
if (gate === "tool call") expect(response.content[0].text).toContain("work is active: edit");
|
||||
} finally { await flow.close(); }
|
||||
});
|
||||
@@ -147,6 +147,14 @@ describe("visible supervisor session", () => {
|
||||
expect(systemPrompt).toContain("not independent evidence");
|
||||
expect(systemPrompt).toContain("outcomes distinguish them");
|
||||
expect(systemPrompt).toContain("One failed implementation does not refute the idea");
|
||||
expect(systemPrompt).toContain("exact tool error, loaded implementation/version");
|
||||
expect(systemPrompt).toContain("cheap discriminating check with predicted outcomes");
|
||||
expect(systemPrompt).toContain("Do not accept a worker's excuse at face value");
|
||||
expect(systemPrompt).toContain("Stay read-only: use SteerWorker");
|
||||
expect(systemPrompt).toContain("Distinguish a sign-off failure from an experiment failure");
|
||||
expect(systemPrompt).toContain("do not assume formal sign-off blocks the next already-authorized experiment");
|
||||
expect(systemPrompt).toContain("instead of committing, deleting or resetting them");
|
||||
expect(systemPrompt).toContain("overrides only cleanliness");
|
||||
expect(systemPrompt).toContain("do not invent work");
|
||||
expect(systemPrompt).toContain("stop issuing instructions");
|
||||
} finally { rmSync(cwd, { recursive: true, force: true }); }
|
||||
|
||||
Reference in New Issue
Block a user