mirror of
https://github.com/wassname/pi-goals.git
synced 2026-09-12 12:50:58 +08:00
Fix solo recovery review boundaries and peer detachment status
This commit is contained in:
wassname
@@ -58,14 +58,14 @@ If the worker model is unavailable or fails after Pi's automatic recovery, work
|
||||
|
||||
**Supervisor failure falls back automatically, but never silently.** On reload/disconnect or a not-ready peer, goal work pauses while the existing Intercom readiness window allows up to five minutes for recovery. A returning ready peer keeps supervised mode. A timeout, explicit supervisor startup/readiness failure, or terminal supervisor model error after Pi's own retries ends that wait and switches an approved working plan to **UNSUPERVISED**. The visible warning and saved session message state the exact reported reason (or readiness timeout, not an invented root cause), the mode transition, preserved plan/evidence, unavailable supervisor sign-off, and `/goals restart` recovery. A continuation message tells the worker to keep implementing and save verification evidence. Ordinary supervisor tool errors and recoverable manual-compaction failures are not by themselves terminal peer failures.
|
||||
|
||||
`/goals solo` explicitly chooses the same mode for an already-approved working plan. Solo mode/reason persist across reload, resume and compaction; the tracked supervisor pane stays available for inspection but its binding is detached. No late peer can silently restore supervision. `CompleteGoal` is unavailable even with a previous approval checkpoint, and manually checked goals remain unreviewed claims. Solo does not auto-complete the plan or erase evidence. Use `/goals restart` for a fresh supervisor and new approval binding; use `/goals clear` to disconnect the plan when appropriate.
|
||||
`/goals solo` explicitly chooses the same mode for an already-approved working plan. Solo mode/reason persist across reload, resume and compaction; the tracked supervisor pane stays available for inspection but its binding is detached. A reachable supervisor receives the detachment reason, shows paused, and rejects further steering on that pairing. No late peer can silently restore supervision. `CompleteGoal` is unavailable even with a previous approval checkpoint, and manually checked goals remain unreviewed claims. Solo does not auto-complete the plan or erase evidence. Use `/goals restart` for a fresh supervisor and new approval binding; use `/goals clear` to disconnect the plan when appropriate.
|
||||
|
||||
An initial **Ready** selection also authorizes fallback on supervisor launch/readiness failure, but only if the exact displayed plan still matches after all waits and the worker model restores successfully. Cancellation, changed content, unapproved drafts, repository/session preflight errors, and worker-model failures never authorize fallback. Recovery does not turn a planning draft into approved work.
|
||||
|
||||
Recovery commands:
|
||||
|
||||
- `/goals reconnect` retries the remembered role model and existing supervisor binding. Worker readiness/reconnect waits allow five minutes, including an ordinary 60-second supervisor compaction, and never replace a slow or missing pane automatically. A peer returning within that window clears the connection pause automatically; an established active worker pairing publishes a fresh current view so supervisor-only reload can resume review even when its previous view was already accepted.
|
||||
- `/goals restart` explicitly closes only the tracked supervisor pane and starts a replacement for a working plan, preserving its file/version but invalidating old approvals. During planning it clears the failed pane so Ready can launch again.
|
||||
- `/goals restart` explicitly closes only the tracked supervisor pane and starts a replacement for a working plan, preserving its file/version but invalidating old approvals. During planning it clears the failed pane so Ready can launch again. If closing a healthy supervisor pane fails, the existing pairing is preserved and the close error is reported; that local error does not authorize solo fallback.
|
||||
- In the supervisor pane, use `/model` then `/goals supervise` (or `/goals reconnect`) to recover an unavailable supervisor model. Startup failure is reported to the waiting worker; it need not wait for the timeout to learn the cause and enter the announced solo fallback.
|
||||
|
||||
Both sessions must load the updated transport for the request/reply reconnect fix; mixed-version peers are not a supported recovery configuration. Ready announces worker readiness only after its model is restored. Plan content is rechecked across startup/model-restore waits; changed content returns to review using the existing pane instead of starting different work. Clearing or leaving planning cancels its pending Ready attempt. `CompleteGoal` checks cancellation and the original binding/version after its asynchronous status lookup and before recording completion.
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# Independent review: 79ec5f3 "Continue approved goals with loud solo recovery on supervisor failure"
|
||||
|
||||
Base: `15dd7f0`. Reviewed diff, full `src/index.ts`, `src/intercom.ts`, `src/supervisor-session.ts`, `src/herdr.ts`, `src/command-help.ts`, `src/prompts.ts`, test changes, and `slop/reviews/solo-recovery/review.md` + validation files as claims to verify. Read-only repo; no panes, research sessions, installs, journal, commits, or edits. Reproductions ran in an isolated `/tmp` copy with `PI_SUBAGENT_CHILD`/`PI_GOALS_ROLE` unset and `PI_GOALS_EVIDENCE_DIR` redirected to `/tmp`. The unrelated dirty `slop/reviews/review-fixes-native/*.jsonl` logs were preserved (sha256 matched `context.txt` before and after). This is not real Herdr acceptance.
|
||||
|
||||
## Verified claims (reproduced)
|
||||
|
||||
- `env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE PI_GOALS_EVIDENCE_DIR=/tmp/... npx vitest run`: **22 files / 138 tests pass**, matching `validation.txt`. The `fatal: not a git repository` stderr is indeed from the negative preflight regression.
|
||||
- `getArgumentCompletions` / `AutocompleteItem.description` are real Pi 0.85.1 API (`node_modules/@earendil-works/pi-coding-agent/dist/core/extensions/types.d.ts:895`) and are wired into interactive-mode slash commands. The per-verb worker/supervisor completion split works and returns `null` (no interference) for objectives.
|
||||
- Authorization rules hold in code and tests: solo fallback after Ready only fires when the exact displayed plan still matches after all waits *and* the worker model restores; changed plan, `noplan`/cancel, unapproved draft, repository preflight failure, and worker-model failure all return to planning/notify instead (`src/index.ts` Ready branch; test `never uses initial supervisor failure to approve a %s Ready attempt`). No approval is inferred from draft, cancellation, changed plan, or model error.
|
||||
- Persisted `mode`/`soloReason` restore on `session_start`; legacy entries without `mode` default to `supervised`. `CompleteGoal` rejects in solo (including an in-flight call via the post-await binding/version re-check). `/goals reconnect` in solo restores only the worker model and stays solo. `/goals restart` restores `supervised` only after successful close+launch+readiness; a failed replacement stays loudly solo. A late old-peer hello after detach is dropped (binding mismatch) and cannot silently restore supervision. Terminal worker-model errors after Pi recovery pause work and never enter solo. Supervisor-side settled model errors call `failReady` only after Pi retries; transient `agent_end` errors alone do not demote. Continuation after solo entry is real: a persisted display message plus a `followUp` user message resume the worker.
|
||||
- The five-minute window behaves as claimed at startup/resume/disconnect boundaries, including transient-reconnect-within-window staying supervised (test `shows a missing resumed supervisor...` updated expectations).
|
||||
|
||||
## Findings
|
||||
|
||||
### F1 — Medium-low (reproduced): a Herdr close failure during `/goals restart` is misclassified as a supervisor failure and demotes a *healthy* supervised pairing to solo
|
||||
|
||||
`src/index.ts:496` wraps `stopSupervisor()` failure in `SupervisorFailure`, and the catch at `src/index.ts:518` routes any `SupervisorFailure` in a working phase to `enterSolo(...)`. This contradicts the class's own contract (`src/index.ts:87`: "Only launch/readiness failures authorize fallback, not local model, plan or repository errors") — a Herdr socket/close error is a local environment failure, not a supervisor launch/readiness failure.
|
||||
|
||||
Reproduced in the isolated copy with the repo's own flow-test harness (`closeSupervisorPane.mockRejectedValueOnce(...)` on a healthy connected pairing, then `/goals restart`): resulting state is `{ mode: "solo", approvalId: null, supervisorPaneId: "owned-pane" }` with the "UNSUPERVISED WORKER ... Could not close the tracked supervisor pane" notification. Consequences: the live binding is detached while the still-healthy supervisor pane remains open and abandoned; `CompleteGoal` is blocked until a further restart succeeds; each retry must first succeed at the same close that just failed. Before this commit the same failure was a plain error with a notify and no mode change.
|
||||
|
||||
Minimal fix: at line 496 throw a plain `Error` (as before) instead of `SupervisorFailure`, so the catch notifies "Goal recovery failed ... use /goals reconnect to retry" without entering solo. If solo-on-failed-replacement is desired only for genuine launch/readiness failures, that behavior is unchanged since `startSupervisor`/`waitSupervisor` still throw `SupervisorFailure`.
|
||||
|
||||
### F2 — Low (inferred from code): the abandoned supervisor pane keeps showing "supervising" and `SteerWorker` silently no-ops after the worker enters solo
|
||||
|
||||
`enterSolo` (`src/index.ts:201-213`) calls `intercom.detach()`, and `detach()` clears `this.binding` *before* calling `this.hello()`, so no final message is published on the old binding. The old supervisor session retains the stale `peer`/`peerReady`; its `connected` getter is channel-level, so its new status line reads "supervising" and `SteerWorker` publishes without error while the worker drops every message on the binding-mismatch check in `intercom.ts` `receive`. Loudness is worker-side only; the visible supervisor pane misrepresents the pairing until `/goals restart`/`clear` closes it. Minimal fix: in `enterSolo`, before `intercom.detach()`, publish one final `hello` (or dedicated message) on the old binding carrying a failure/reason such as "worker entered solo mode; pairing detached" so the pane flips to paused and steering errors surface.
|
||||
|
||||
### F3 — Low (code-read, cosmetic): solo widget still says "awaiting supervisor review" for claimed goals
|
||||
|
||||
`src/index.ts:431` renders manually ticked goals as `? claimed complete; awaiting supervisor review: ...` even in solo mode, where no review can arrive; only the unshifted UNSUPERVISED line (line 432) contradicts it. Minimal fix: in solo, render these as `? claimed complete; unreviewed (solo): ...`.
|
||||
|
||||
### F4 — Low (inferred): every `/goals` invocation overwrites the command token and silently cancels an in-flight reconnect/restart
|
||||
|
||||
`commandAttempt = command` runs at the top of the handler (`src/index.ts:448`) for *all* verbs, including no-ops (`/goals supervise` in the worker, `/goals solo` during planning, `/goals work` with no pairing). The in-flight reconnect/restart's `current()` then fails and it returns silently — no notification that the recovery was cancelled. Realistic scenario: user mistypes `/goals supervis` while a restart is inside its five-minute readiness wait; the restart dies quietly and the pairing stays paused. Minimal fix: assign `commandAttempt` only inside the reconnect/restart branch (and other branches that intentionally supersede), or notify when an in-flight recovery is cancelled.
|
||||
|
||||
### F5 — Informational
|
||||
|
||||
- `supervisor-session.ts:98` calls `statusContext?.ui.setStatus(...)` during `session_shutdown`; whether Pi tolerates `setStatus` after UI teardown is unverified (mocked in tests). Low risk, unflagged as a bug.
|
||||
- Solo reasons built with `String(error)` carry an `Error: ` prefix (`src/index.ts:223`); cosmetic.
|
||||
- Design consequence (documented in README, within the stated authorization): any transport-level disconnect not recovered within five minutes converts to solo even if the supervisor process is healthy, and an explicit peer `failure` hello ends the wait with zero recovery window. Both are loud and reasoned; flagging only so the trade-off is conscious.
|
||||
- Unnecessary-complexity note: three overlapping cancellation tokens (`readyAttempt`, `recoveryAttempt`, `recoveryCommand`/`commandAttempt`) guard adjacent async spans; F4 is the concrete cost of the third one. `intercom.onSteer`'s new `state.mode === "solo"` guard is redundant (the detached binding already drops steers) but harmless.
|
||||
|
||||
## Remaining functional gaps (agree with review.md's own boundary)
|
||||
|
||||
- No real-Herdr, real-model acceptance is claimed here or by the commit: rendered loud fallback, per-verb completion display, actual useful worker continuation quality after solo, both reload orders, restart-back-to-supervised, and a real ApproveGoal → CompleteGoal cycle in parent-owned test panes remain unverified.
|
||||
- The five-minute window is not a liveness watchdog: an alive-but-hung supervisor that keeps reporting ready still requires explicit `/goals solo` or `/goals restart`.
|
||||
- Continuation delivery relies on Pi's messaging API; no exactly-once guarantee (acknowledged).
|
||||
- `native/` evidence covers the pre-existing fork/resume path only; it is not evidence for the new fallback behavior.
|
||||
@@ -0,0 +1,226 @@
|
||||
|
||||
RUN v4.1.9 /tmp/pi-goals-solo-review-baseline-BoWZ0j
|
||||
|
||||
❯ test/goals-flow.test.ts (50 tests | 10 failed | 40 skipped) 235ms
|
||||
× keeps a healthy pairing ready when restart cannot close its pane 48ms
|
||||
× labels solo completion claims unreviewed without implying a supervisor will review them 20ms
|
||||
× does not let no-op supervise cancel an in-flight restart 23ms
|
||||
× does not let no-op noplan cancel an in-flight restart 24ms
|
||||
× does not let no-op model cancel an in-flight restart 22ms
|
||||
× does not let no-op busy reconnect cancel an in-flight restart 21ms
|
||||
× does not let no-op solo cancel an in-flight restart 21ms
|
||||
× does not let no-op work cancel an in-flight restart 19ms
|
||||
× announces explicit solo to the old supervisor without a reciprocal failure loop 19ms
|
||||
× announces terminal failure to the old supervisor without a reciprocal failure loop 16ms
|
||||
❯ test/supervisor-session.test.ts (14 tests | 1 failed | 13 skipped) 13ms
|
||||
× shows paused with the worker's solo detachment reason and refuses steering or approval 12ms
|
||||
|
||||
⎯⎯⎯⎯⎯⎯ Failed Tests 11 ⎯⎯⎯⎯⎯⎯⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > keeps a healthy pairing ready when restart cannot close its pane
|
||||
AssertionError: expected { Object (phase, mode, ...) } to match object { phase: 'working', …(3) }
|
||||
(6 matching properties omitted from actual)
|
||||
|
||||
- Expected
|
||||
+ Received
|
||||
|
||||
{
|
||||
- "approvalId": "restored-binding",
|
||||
- "mode": "supervised",
|
||||
+ "approvalId": null,
|
||||
+ "mode": "solo",
|
||||
"phase": "working",
|
||||
"supervisorPaneId": "owned-pane",
|
||||
}
|
||||
|
||||
❯ test/goals-flow.test.ts:820:37
|
||||
818| closeSupervisorPane.mockRejectedValueOnce(new Error("Herdr close una…
|
||||
819| await flow.commands.get("goals").handler("restart", flow.ctx);
|
||||
820| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
| ^
|
||||
821| expect(readFileSync(path, "utf8")).toBe(before);
|
||||
822| expect(openSupervisorPane).not.toHaveBeenCalled();
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > labels solo completion claims unreviewed without implying a supervisor will review them
|
||||
AssertionError: expected last "vi.fn()" call to have been called with [ 'pi-goals-widget', …(1) ]
|
||||
|
||||
- Expected
|
||||
+ Received
|
||||
|
||||
[
|
||||
"pi-goals-widget",
|
||||
[
|
||||
"UNSUPERVISED: You explicitly selected /goals solo. Supervisor sign-off unavailable. /goals restart",
|
||||
- "? claimed complete; unreviewed (solo): make the file",
|
||||
+ "? claimed complete; awaiting supervisor review: make the file",
|
||||
],
|
||||
]
|
||||
|
||||
❯ test/goals-flow.test.ts:839:33
|
||||
837| writeFileSync(path, readFileSync(path, "utf8").replace("[ ] goal:", …
|
||||
838| await flow.commands.get("goals").handler("solo", flow.ctx);
|
||||
839| expect(flow.ctx.ui.setWidget).toHaveBeenLastCalledWith("pi-goals-wid…
|
||||
| ^
|
||||
840| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
841| expect(flow.entries.at(-1)?.data).toMatchObject({ signedOffGoals: []…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op supervise cancel an in-flight restart
|
||||
AssertionError: expected 'This is the worker session. Run /goal…' to be 'Goal supervision reconnected; the cur…' // Object.is equality
|
||||
|
||||
Expected: "Goal supervision reconnected; the current plan is unchanged."
|
||||
Received: "This is the worker session. Run /goals supervise in the saved supervisor session; no new pairing was created."
|
||||
|
||||
❯ test/goals-flow.test.ts:863:37
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
| ^
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
865| expect(await flow.hooks.get("tool_call")({ toolName: "write", input:…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op noplan cancel an in-flight restart
|
||||
AssertionError: expected 'Not in planning mode; the current pla…' to be 'Goal supervision reconnected; the cur…' // Object.is equality
|
||||
|
||||
Expected: "Goal supervision reconnected; the current plan is unchanged."
|
||||
Received: "Not in planning mode; the current plan is unchanged."
|
||||
|
||||
❯ test/goals-flow.test.ts:863:37
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
| ^
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
865| expect(await flow.hooks.get("tool_call")({ toolName: "write", input:…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[4/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op model cancel an in-flight restart
|
||||
AssertionError: expected 'Run /goals clear before changing the …' to be 'Goal supervision reconnected; the cur…' // Object.is equality
|
||||
|
||||
Expected: "Goal supervision reconnected; the current plan is unchanged."
|
||||
Received: "Run /goals clear before changing the active supervisor model."
|
||||
|
||||
❯ test/goals-flow.test.ts:863:37
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
| ^
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
865| expect(await flow.hooks.get("tool_call")({ toolName: "write", input:…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[5/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op busy reconnect cancel an in-flight restart
|
||||
AssertionError: expected 'Stop the current turn before recoveri…' to be 'Goal supervision reconnected; the cur…' // Object.is equality
|
||||
|
||||
Expected: "Goal supervision reconnected; the current plan is unchanged."
|
||||
Received: "Stop the current turn before recovering goal supervision."
|
||||
|
||||
❯ test/goals-flow.test.ts:863:37
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
| ^
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
865| expect(await flow.hooks.get("tool_call")({ toolName: "write", input:…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[6/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op solo cancel an in-flight restart
|
||||
AssertionError: expected { Object (phase, mode, ...) } to match object { phase: 'working', …(2) }
|
||||
(7 matching properties omitted from actual)
|
||||
|
||||
- Expected
|
||||
+ Received
|
||||
|
||||
{
|
||||
"approvalId": "e03bc7aa-7265-49de-8b0c-edc9a99faf15",
|
||||
- "mode": "supervised",
|
||||
+ "mode": "solo",
|
||||
"phase": "working",
|
||||
}
|
||||
|
||||
❯ test/goals-flow.test.ts:862:37
|
||||
860| flow.transport.receive({ binding, role: "supervisor", kind: "hello",…
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
| ^
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[7/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > does not let no-op work cancel an in-flight restart
|
||||
AssertionError: expected { Object (phase, mode, ...) } to match object { phase: 'working', …(2) }
|
||||
(7 matching properties omitted from actual)
|
||||
|
||||
- Expected
|
||||
+ Received
|
||||
|
||||
{
|
||||
"approvalId": "f9dd5633-c0a1-4242-868f-128375f0753a",
|
||||
- "mode": "supervised",
|
||||
+ "mode": "solo",
|
||||
"phase": "working",
|
||||
}
|
||||
|
||||
❯ test/goals-flow.test.ts:862:37
|
||||
860| flow.transport.receive({ binding, role: "supervisor", kind: "hello",…
|
||||
861| await restarting;
|
||||
862| expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", …
|
||||
| ^
|
||||
863| expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected…
|
||||
864| expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", e…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[8/11]⎯
|
||||
|
||||
FAIL test/goals-flow.test.ts > announces explicit solo to the old supervisor without a reciprocal failure loop
|
||||
FAIL test/goals-flow.test.ts > announces terminal failure to the old supervisor without a reciprocal failure loop
|
||||
AssertionError: expected "vi.fn()" to be called with arguments: [ StringContaining{…}, 'error' ]
|
||||
|
||||
Number of calls: 0
|
||||
|
||||
❯ test/goals-flow.test.ts:884:35
|
||||
882| await new Promise(resolve => setImmediate(resolve));
|
||||
883| expect(supervisor.connected).toBe(false);
|
||||
884| expect(supervisorCtx.ui.notify).toHaveBeenCalledWith(expect.stringCo…
|
||||
| ^
|
||||
885| expect(() => supervisor.steer("Obsolete advice.")).toThrow("pairing …
|
||||
886| expect(flow.entries.at(-1)?.data).toMatchObject({ mode: "solo", appr…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[9/11]⎯
|
||||
|
||||
FAIL test/supervisor-session.test.ts > shows paused with the worker's solo detachment reason and refuses steering or approval
|
||||
AssertionError: expected last "vi.fn()" call to have been called with [ 'pi-goals', 'supervisor · paused' ]
|
||||
|
||||
- Expected
|
||||
+ Received
|
||||
|
||||
[
|
||||
"pi-goals",
|
||||
- "supervisor · paused",
|
||||
+ "supervisor · starting/reconnecting",
|
||||
]
|
||||
|
||||
❯ test/supervisor-session.test.ts:377:36
|
||||
375| runtime.transport.receive({ binding: "approval-1", role: "worker", k…
|
||||
376| expect(runtime.ctx.ui.notify).toHaveBeenCalledWith(reason, "error");
|
||||
377| expect(runtime.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals"…
|
||||
| ^
|
||||
378| await expect(runtime.tools.get("SteerWorker").execute("id", { instru…
|
||||
379| expect(runtime.transport.sent.filter(message => message.kind === "st…
|
||||
|
||||
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[10/11]⎯
|
||||
|
||||
|
||||
Test Files 2 failed (2)
|
||||
Tests 11 failed | 53 skipped (64)
|
||||
Start at 19:54:44
|
||||
Duration 1.09s (transform 369ms, setup 0ms, import 1.40s, tests 248ms, environment 0ms)
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
Base: 79ec5f350ad5c369aa51a48212a196cdddf2c174
|
||||
Task: agreed F1-F4 only; no push, panes or broader reconnect investigation.
|
||||
Pi: 0.85.1
|
||||
Unrelated dirty logs before validation:
|
||||
a8f1facd16fd6ff2938195a7adbdc3b4b1e1dea61533e90a39e5eb18d201e190 slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
de5bb7171e71508c40e19f3929303a63f1fa0f2ea7789a2bcc8728ea038310d2 slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
Initial status:
|
||||
M slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
M slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
?? docs/human_journal.md (not read or edited)
|
||||
Baseline isolated copy: /tmp/pi-goals-solo-review-baseline-BoWZ0j
|
||||
@@ -0,0 +1,34 @@
|
||||
# Independent review fixes: F1–F4
|
||||
|
||||
Base: `79ec5f3` (Continue approved goals with loud solo recovery on supervisor failure).
|
||||
|
||||
## Attribution and source confirmation
|
||||
|
||||
`../independent-review-79ec5f3.md` is a verbatim copy of the parent-supplied independent reviewer artifact, not this implementation worker's review. Source: `/home/code/.pi/agent/sessions/--home-code-.pi-agent-git-github.com-wassname-pi-goals--/subagent-artifacts/outputs/8ec5792f-53d8-4210-b79b-c027713186c8/solo-recovery-independent-review.md`.
|
||||
|
||||
Read the review, applicable AGENTS.md, README, prior implementation evidence and affected source/tests before changes. No panes, research sessions or journal contents were accessed. Unrelated dirty native logs were preserved byte-for-byte; see context.txt and preservation.txt.
|
||||
|
||||
## Dispositions
|
||||
|
||||
- **F1 confirmed and fixed.** Failed `stopSupervisor()` was classified as `SupervisorFailure`, authorizing solo for a healthy peer after a local Herdr close error. It now raises an ordinary error. A plain-error-only fix would have left worker readiness false after model restoration, so a previously connected pairing also restores its local readiness after the failed close. Plan, binding and tracked pane remain unchanged, no replacement starts, and the error remains visible. Regression verifies supervised status, allowed implementation and delivered steering afterward. Genuine launch/readiness failure still uses the approved loud fallback.
|
||||
- **F2 partially confirmed; corrected diagnosis and fixed remaining gap.** Contrary to the review's code description, base `79ec5f3` already sent a not-ready hello **before** clearing the binding, and `connected` included both readiness flags. Thus delivery of that hello already disconnected the peer and blocked steering. The actual missing pieces were an explicit terminal detachment reason and distinguishing detached from starting/reconnecting in supervisor status. Solo now supplies that reason to `detach`, using the existing final hello, not a new protocol. The supervisor's readiness-status getter includes the peer-reported failure; rejected steering includes that reason. No received peer failure is copied into the local failure field or echoed back as one. Paired real GoalIntercom adapter tests cover explicit solo and terminal-supervisor-error fallback, a single solo announcement, rejected stale steering and a settled message count without reciprocal failure loops. The supervisor role test checks paused status and rejected steering/approval. Delivery of this final notice requires a reachable transport; no durable detachment-ack protocol or broader reconnect behavior was added.
|
||||
- **F3 confirmed and fixed.** Solo manual completion claims now say `unreviewed (solo)`, retaining zero supervised sign-offs. Supervised claims retain `awaiting supervisor review`.
|
||||
- **F4 confirmed and fixed.** The command cancellation token is now changed only after a command's no-op/rejection checks, in branches that intentionally change the plan/mode or initiate recovery. Six regression cases hold restart at its readiness wait, issue no-op supervise/noplan/model/busy-reconnect/already-solo/work-in-solo commands, then deliver peer readiness; recovery still completes. Existing clear-during-recovery cancellation tests remain passing. Valid plan-changing commands still supersede recovery; no new command or recovery framework was introduced.
|
||||
|
||||
## Observed validation
|
||||
|
||||
- New regressions against isolated **base source**: 11 failed, 53 skipped, expected exit 1. `baseline-regressions.txt` and the isolated-copy location in `context.txt` preserve the reproduction. This is intentional red-test evidence, not a final validation failure.
|
||||
- Focused current-source run: 3 files / 71 tests passed (`focused.txt`).
|
||||
- Full current-source `npm test`: 22 files / **149 tests passed** (`validation.txt`). This includes existing deterministic real Pi native/RPC lifecycle tests.
|
||||
- `npm run typecheck`, `npm run lint`, `npm run build`, `git diff --check`: passed.
|
||||
- Tests unset inherited `PI_SUBAGENT_CHILD` and `PI_GOALS_ROLE` only for test processes and use the fresh explicit `review-fixes/native` evidence directory. Other commands retain their environment.
|
||||
- The expected `fatal: not a git repository` stderr comes from the existing negative Ready preflight regression, which passes by rejecting unauthorized fallback.
|
||||
- Fresh `native/` recordings establish the existing deterministic fork/resume path still runs, **not** real-model functional proof of the new review fixes.
|
||||
|
||||
## Remaining gaps / boundaries
|
||||
|
||||
The user reports reconnect often works only after reloading the worker and then reconnecting. **Unresolved, user-reported, not reproduced or fixed here**; broader reconnect investigation was explicitly deferred. Approved five-minute recovery and loud automatic solo fallback policy are unchanged.
|
||||
|
||||
Real-model Herdr rendered status/notice, useful autonomous continuation, both reload orders, restart-back-to-supervised and ApproveGoal → CompleteGoal acceptance remain outstanding. No real-session robustness success is claimed. Hung-but-ready peers, transient delay/credit policy, and message delivery guarantees are unchanged. The final detachment notice is best-effort on the current binding; offline peers cannot be notified by it.
|
||||
|
||||
Scoped fixes are committed locally for parent/reviewer follow-up; **not pushed**. Independent re-review of these fixes remains required before acceptance/push.
|
||||
@@ -0,0 +1,10 @@
|
||||
|
||||
RUN v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
|
||||
|
||||
fatal: not a git repository (or any of the parent directories): .git
|
||||
|
||||
Test Files 3 passed (3)
|
||||
Tests 71 passed (71)
|
||||
Start at 19:54:08
|
||||
Duration 2.34s (transform 544ms, setup 0ms, import 1.77s, tests 1.90s, environment 0ms)
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -0,0 +1,32 @@
|
||||
{"id":"planning","type":"response","command":"prompt","success":true}
|
||||
{"type":"agent_start"}
|
||||
{"type":"turn_start"}
|
||||
{"type":"message_start","message":{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788954906871}}
|
||||
{"type":"message_end","message":{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788954906871}}
|
||||
{"type":"message_start","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"totalTokens":0,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"pending","timestamp":1788954906889}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_start","contentIndex":0}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_delta","contentIndex":0,"delta":"Test context retained. Actual outputs still need inspection."}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_end","contentIndex":0,"content":"Test context retained. Actual outputs still need inspection."}}
|
||||
{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954906889,"rawStopReason":"stop"}}
|
||||
{"type":"turn_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954906889,"rawStopReason":"stop"},"toolResults":[]}
|
||||
{"type":"agent_end","messages":[{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788954906871},{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954906889,"rawStopReason":"stop"}],"willRetry":false}
|
||||
{"type":"agent_settled"}
|
||||
{"id":"worker-state","type":"response","command":"get_state","success":true,"data":{"model":{"id":"test","name":"Offline test model","reasoning":false,"input":["text"],"contextWindow":16000,"maxTokens":1000,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0},"api":"openai-completions","provider":"offline","baseUrl":"http://127.0.0.1:38459"},"thinkingLevel":"off","isStreaming":false,"isCompacting":false,"steeringMode":"one-at-a-time","followUpMode":"one-at-a-time","sessionFile":"/tmp/goals-native-pair-j8cvKU/agent/sessions/--tmp-goals-native-pair-j8cvKU--/2026-09-09T11-55-06-433Z_01a08605-8f40-7409-85cc-57eacee84a8a.jsonl","sessionId":"01a08605-8f40-7409-85cc-57eacee84a8a","autoCompactionEnabled":true,"messageCount":2,"pendingMessageCount":0}}
|
||||
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"out","message":{"binding":"native-pair-test","role":"worker","kind":"view","id":"4bab328d-099f-4466-b1eb-3b6dfca1c9d7","text":"The worker stopped.\n\nInspect the results and judge whether the agreed goal is actually achieved. If unfinished, investigate why the worker stopped and use SteerWorker to send the next useful instruction and resume work. If a verified dependency prevents progress, establish what will resume it and how that will be observed. Do not treat stopping as completion. Consider ApproveGoal only after the results satisfy the goal.\n\nreview trigger: settled\nsource session: /tmp/goals-native-pair-j8cvKU/agent/sessions/--tmp-goals-native-pair-j8cvKU--/2026-09-09T11-55-06-433Z_01a08605-8f40-7409-85cc-57eacee84a8a.jsonl\nworker model: offline/test\nlatest human direction:\nInspect actual outputs.\ntool calls with no result: none\ntracked background work: No tracked work in this fixture.\n\nnew worker overview (initial or reset view) (VCC algorithmic compression; local # refs index new messages; tool-result bodies omitted; inspect source for evidence):\n[Session Goal]\n- Retain this planning context for the supervisor fork.\n\n[user]\nRetain this planning context for the supervisor fork.\n\n[assistant]\nTest context retained. Actual outputs still need inspection. (#1)\n\nworker view id: 4bab328d-099f-4466-b1eb-3b6dfca1c9d7","reason":"settled","backgroundQuiet":true}},"id":"e7ccf046","parentId":"43529cce","timestamp":"2026-09-09T11:55:07.663Z"}}
|
||||
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"ack","message":{"binding":"native-pair-test","role":"supervisor","kind":"received","id":"4bab328d-099f-4466-b1eb-3b6dfca1c9d7"}},"id":"824e0f4c","parentId":"e7ccf046","timestamp":"2026-09-09T11:55:07.668Z"}}
|
||||
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"ack","message":{"binding":"native-pair-test","role":"supervisor","kind":"received","id":"4bab328d-099f-4466-b1eb-3b6dfca1c9d7"}},"id":"bce8079f","parentId":"824e0f4c","timestamp":"2026-09-09T11:55:07.698Z"}}
|
||||
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"queued","message":{"binding":"native-pair-test","role":"supervisor","kind":"steer","id":"20f27a3b-b816-4c06-a336-bc7f949c5660","text":"Read the real outputs before declaring completion."}},"id":"f1ed434e","parentId":"bce8079f","timestamp":"2026-09-09T11:55:07.713Z"}}
|
||||
{"type":"agent_start"}
|
||||
{"type":"turn_start"}
|
||||
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"in","message":{"binding":"native-pair-test","role":"supervisor","kind":"steer","id":"20f27a3b-b816-4c06-a336-bc7f949c5660","text":"Read the real outputs before declaring completion."}},"id":"cd6ab1de","parentId":"f1ed434e","timestamp":"2026-09-09T11:55:07.715Z"}}
|
||||
{"type":"message_start","message":{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788954907715}}
|
||||
{"type":"message_end","message":{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788954907715}}
|
||||
{"type":"message_start","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"totalTokens":0,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"pending","timestamp":1788954907716}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_start","contentIndex":0}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_delta","contentIndex":0,"delta":"Test context retained. Actual outputs still need inspection."}}
|
||||
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_end","contentIndex":0,"content":"Test context retained. Actual outputs still need inspection."}}
|
||||
{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954907716,"rawStopReason":"stop"}}
|
||||
{"type":"turn_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954907716,"rawStopReason":"stop"},"toolResults":[]}
|
||||
{"type":"agent_end","messages":[{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788954907715},{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788954907716,"rawStopReason":"stop"}],"willRetry":false}
|
||||
{"type":"agent_settled"}
|
||||
{"type":"extension_ui_request","id":"5734fb74-37a3-44cd-9e44-64623a19ce09","method":"notify","message":"Goal supervision peer disconnected; reconnect the existing session.","notifyType":"warning"}
|
||||
@@ -0,0 +1,7 @@
|
||||
Unrelated dirty logs after validation:
|
||||
a8f1facd16fd6ff2938195a7adbdc3b4b1e1dea61533e90a39e5eb18d201e190 slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
de5bb7171e71508c40e19f3929303a63f1fa0f2ea7789a2bcc8728ea038310d2 slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
Expected hashes:
|
||||
a8f1facd16fd6ff2938195a7adbdc3b4b1e1dea61533e90a39e5eb18d201e190 slop/reviews/review-fixes-native/supervisor-events.jsonl
|
||||
de5bb7171e71508c40e19f3929303a63f1fa0f2ea7789a2bcc8728ea038310d2 slop/reviews/review-fixes-native/worker-events.jsonl
|
||||
Staged files before scoped commit:
|
||||
@@ -0,0 +1,27 @@
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 test
|
||||
> vitest run
|
||||
|
||||
|
||||
RUN v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
|
||||
|
||||
fatal: not a git repository (or any of the parent directories): .git
|
||||
|
||||
Test Files 22 passed (22)
|
||||
Tests 149 passed (149)
|
||||
Start at 19:55:05
|
||||
Duration 5.30s (transform 3.81s, setup 0ms, import 8.86s, tests 12.31s, environment 3ms)
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 typecheck
|
||||
> tsc --noEmit
|
||||
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 lint
|
||||
> biome check src/ test/
|
||||
|
||||
Checked 42 files in 49ms. No fixes applied.
|
||||
|
||||
> @wassname2/pi-goals@0.2.2 build
|
||||
> tsc
|
||||
|
||||
+15
-4
@@ -204,7 +204,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
state = { ...state, mode: "solo", soloReason: reason, approvalId: null };
|
||||
persist();
|
||||
stopWorkerTimers();
|
||||
intercom.detach();
|
||||
intercom.detach(`Worker entered solo mode; this pairing is detached. ${reason} Restore supervision with /goals restart in the worker session.`);
|
||||
const message = `UNSUPERVISED WORKER: ${reason} Continuing in solo mode with the same approved plan (${planRel(ctx)}) and evidence preserved. Supervisor sign-off is unavailable; do not call CompleteGoal or claim supervised completion. Continue useful implementation and save verification evidence. Use /goals restart to restore supervision.`;
|
||||
ctx.ui.notify(message, "warning");
|
||||
pi.sendMessage({ customType: "pi-goals-mode", content: message, display: true });
|
||||
@@ -428,7 +428,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
// goal also shows its open subtasks: this file is the task list, so the widget is the task list.
|
||||
// No path line: the session id makes it too long to be useful in the widget.
|
||||
const plan = readPlan(ctx);
|
||||
const lines: string[] = claimed.map(g => `? claimed complete; awaiting supervisor review: ${g.subject}`);
|
||||
const lines: string[] = claimed.map(g => `? claimed complete; ${state.mode === "solo" ? "unreviewed (solo)" : "awaiting supervisor review"}: ${g.subject}`);
|
||||
if (state.phase === "working" && state.mode === "solo") lines.unshift(`UNSUPERVISED: ${state.soloReason} Supervisor sign-off unavailable. /goals restart`);
|
||||
else if (liveGoals.length === 0 && claimed.length === 0) lines.push("✔ complete");
|
||||
for (const g of liveGoals) {
|
||||
@@ -445,12 +445,12 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
getArgumentCompletions: prefix => goalCommandCompletions(prefix, "worker"),
|
||||
handler: async (args, ctx) => {
|
||||
const command = {};
|
||||
commandAttempt = command;
|
||||
let arg = args.trim();
|
||||
if (arg === "solo") {
|
||||
if (state.phase !== "working") { ctx.ui.notify("Solo requires an already-approved plan. A draft still needs Ready.", "warning"); return; }
|
||||
if (modelError) { ctx.ui.notify(`Cannot enter solo: ${pauseReason()}`, "warning"); return; }
|
||||
if (state.mode === "solo") { ctx.ui.notify("Already UNSUPERVISED; plan preserved, supervisor sign-off unavailable. /goals restart restores supervision.", "warning"); return; }
|
||||
commandAttempt = command;
|
||||
readyAttempt = undefined;
|
||||
enterSolo(ctx, "You explicitly selected /goals solo.");
|
||||
return;
|
||||
@@ -463,6 +463,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
}
|
||||
if (arg === "noplan") {
|
||||
if (state.phase !== "planning") { ctx.ui.notify("Not in planning mode; the current plan is unchanged.", "info"); return; }
|
||||
commandAttempt = command;
|
||||
readyAttempt = undefined;
|
||||
planningContextPending = false;
|
||||
resyncReason = null;
|
||||
@@ -478,6 +479,8 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
if (arg === "reconnect" || arg === "restart") {
|
||||
if (!state.phase) { ctx.ui.notify("No active plan to recover.", "info"); return; }
|
||||
if (!ctx.isIdle()) { ctx.ui.notify("Stop the current turn before recovering goal supervision.", "warning"); return; }
|
||||
commandAttempt = command;
|
||||
const connectedBeforeRecovery = intercom.connected;
|
||||
readyAttempt = undefined;
|
||||
recoveryAttempt = undefined;
|
||||
const version = state.planVersion;
|
||||
@@ -493,8 +496,13 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
return;
|
||||
}
|
||||
if (arg === "restart") {
|
||||
if (!(await stopSupervisor())) throw new SupervisorFailure("Could not close the tracked supervisor pane; no replacement was opened.");
|
||||
const stopped = await stopSupervisor();
|
||||
if (!current()) return;
|
||||
if (!stopped) {
|
||||
// Model restoration paused our readiness, but a failed close did not end the pairing.
|
||||
if (connectedBeforeRecovery) intercom.markReady();
|
||||
throw new Error("Could not close the tracked supervisor pane; no replacement was opened.");
|
||||
}
|
||||
state = { ...state, supervisorPaneId: null, approvalId: null };
|
||||
persist();
|
||||
}
|
||||
@@ -526,6 +534,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
ctx.ui.notify("No active plan to disconnect.", "info");
|
||||
return;
|
||||
}
|
||||
commandAttempt = command;
|
||||
const currentPlan = planRel(ctx);
|
||||
if (!(await stopSupervisor())) {
|
||||
ctx.ui.notify("Could not close the visible supervisor; the plan remains connected.", "warning");
|
||||
@@ -544,6 +553,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
ctx.ui.notify("Run /goals clear before changing the active supervisor model.", "warning");
|
||||
return;
|
||||
}
|
||||
commandAttempt = command;
|
||||
if (!(await stopSupervisor())) {
|
||||
ctx.ui.notify("Could not close the visible supervisor; its model was not changed.", "warning");
|
||||
return;
|
||||
@@ -554,6 +564,7 @@ export function registerWorker(pi: ExtensionAPI): void {
|
||||
ctx.ui.notify(`Goal-supervisor model ${ref ? `set to ${ref}` : "reset to the remembered supervisor model"}.`, "info");
|
||||
return;
|
||||
}
|
||||
commandAttempt = command;
|
||||
if (!(await stopSupervisor())) {
|
||||
ctx.ui.notify("Could not close the visible supervisor; no new plan was started.", "warning");
|
||||
return;
|
||||
|
||||
+5
-3
@@ -106,12 +106,14 @@ export class GoalIntercom {
|
||||
}
|
||||
|
||||
// End this plan's binding without disposing the session's transport.
|
||||
detach(): void {
|
||||
detach(reason?: string): void {
|
||||
if (this.deliveryTimer) clearTimeout(this.deliveryTimer);
|
||||
this.deliveryTimer = undefined;
|
||||
this.inbox.clear();
|
||||
this.delivering = undefined;
|
||||
this.ready = false;
|
||||
// Announce on the old binding; a peer failure is never echoed as our own failure.
|
||||
this.failure = reason;
|
||||
this.hello();
|
||||
this.binding = "";
|
||||
this.peer = undefined;
|
||||
@@ -131,7 +133,7 @@ export class GoalIntercom {
|
||||
this.hello();
|
||||
if (this.ctx) this.onConnectionChange(this.ctx);
|
||||
}
|
||||
get readinessFailure(): string | undefined { return this.failure; }
|
||||
get readinessFailure(): string | undefined { return this.failure ?? this.peerFailure; }
|
||||
get ended(): boolean { return this.stopped; }
|
||||
get bound(): boolean { return !this.stopped && Boolean(this.binding); }
|
||||
get peerPresent(): boolean { return Boolean(this.bound && this.peer && this.channel?.snapshot().connected); }
|
||||
@@ -167,7 +169,7 @@ export class GoalIntercom {
|
||||
}
|
||||
|
||||
steer(text: string): string {
|
||||
if (!this.connected) throw new Error("Worker is disconnected; no instruction was sent.");
|
||||
if (!this.connected) throw new Error(this.peerFailure ? `Worker is disconnected: ${this.peerFailure} No instruction was sent.` : "Worker is disconnected; no instruction was sent.");
|
||||
const message: Message = { binding: this.binding, role: this.role, kind: "steer", id: randomUUID(), text };
|
||||
this.record("out", message);
|
||||
this.pending.set(message.id, message);
|
||||
|
||||
@@ -808,3 +808,87 @@ it("keeps worker runtime errors paused after Pi retries, rather than treating th
|
||||
expect((await flow.hooks.get("tool_call")({ toolName: "write", input: { path: "result.txt" } }, flow.ctx)).block).toBe(true);
|
||||
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("keeps a healthy pairing ready when restart cannot close its pane", async () => {
|
||||
const flow = setup([]);
|
||||
try {
|
||||
const path = restoredPlan(flow);
|
||||
const before = readFileSync(path, "utf8");
|
||||
await flow.hooks.get("session_start")({}, flow.ctx);
|
||||
closeSupervisorPane.mockRejectedValueOnce(new Error("Herdr close unavailable"));
|
||||
await flow.commands.get("goals").handler("restart", flow.ctx);
|
||||
expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", mode: "supervised", approvalId: "restored-binding", supervisorPaneId: "owned-pane" });
|
||||
expect(readFileSync(path, "utf8")).toBe(before);
|
||||
expect(openSupervisorPane).not.toHaveBeenCalled();
|
||||
expect(flow.notifications.at(-1)).toContain("Could not close the tracked supervisor pane");
|
||||
expect(flow.notifications.some(text => text.includes("UNSUPERVISED WORKER"))).toBe(false);
|
||||
expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", expect.stringContaining("supervised worker"));
|
||||
expect(await flow.hooks.get("tool_call")({ toolName: "write", input: { path: "result.txt" } }, flow.ctx)).toBeUndefined();
|
||||
flow.transport.receive({ binding: "restored-binding", role: "supervisor", kind: "steer", id: "still-paired", text: "Inspect the output." });
|
||||
expect(flow.messages.at(-1)?.content).toBe("[supervisor] Inspect the output.");
|
||||
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("labels solo completion claims unreviewed without implying a supervisor will review them", async () => {
|
||||
const flow = setup([]);
|
||||
try {
|
||||
const path = restoredPlan(flow);
|
||||
await flow.hooks.get("session_start")({}, flow.ctx);
|
||||
writeFileSync(path, readFileSync(path, "utf8").replace("[ ] goal:", "[x] goal:"));
|
||||
await flow.commands.get("goals").handler("solo", flow.ctx);
|
||||
expect(flow.ctx.ui.setWidget).toHaveBeenLastCalledWith("pi-goals-widget", [expect.stringContaining("UNSUPERVISED"), "? claimed complete; unreviewed (solo): make the file"]);
|
||||
expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", expect.stringContaining("0/1 goals · UNSUPERVISED"));
|
||||
expect(flow.entries.at(-1)?.data).toMatchObject({ signedOffGoals: [] });
|
||||
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it.each(["supervise", "noplan", "model", "busy reconnect", "solo", "work"])("does not let no-op %s cancel an in-flight restart", async noop => {
|
||||
const flow = setup([]);
|
||||
try {
|
||||
restoredPlan(flow);
|
||||
await flow.hooks.get("session_start")({}, flow.ctx);
|
||||
if (["solo", "work"].includes(noop)) await flow.commands.get("goals").handler("solo", flow.ctx);
|
||||
flow.transport.replyToHello(false);
|
||||
const restarting = flow.commands.get("goals").handler("restart", flow.ctx);
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
const binding = (flow.entries.at(-1)?.data as any).approvalId;
|
||||
expect(binding).toBeTruthy();
|
||||
expect(binding).not.toBe("restored-binding");
|
||||
if (noop === "busy reconnect") flow.ctx.isIdle.mockReturnValue(false);
|
||||
await flow.commands.get("goals").handler(noop === "busy reconnect" ? "reconnect" : noop, flow.ctx);
|
||||
flow.ctx.isIdle.mockReturnValue(true);
|
||||
flow.transport.receive({ binding, role: "supervisor", kind: "hello", id: "hello", ready: true, reply: true });
|
||||
await restarting;
|
||||
expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", mode: "supervised", approvalId: binding });
|
||||
expect(flow.notifications.at(-1)).toBe("Goal supervision reconnected; the current plan is unchanged.");
|
||||
expect(flow.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", expect.stringContaining("supervised worker"));
|
||||
expect(await flow.hooks.get("tool_call")({ toolName: "write", input: { path: "result.txt" } }, flow.ctx)).toBeUndefined();
|
||||
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it.each(["explicit solo", "terminal failure"])("announces %s to the old supervisor without a reciprocal failure loop", async cause => {
|
||||
const wire = pairedIntercomFixture();
|
||||
const flow = setup([], [], wire.worker.events as ExtensionAPI["events"]);
|
||||
const supervisor = new GoalIntercom({ events: wire.supervisor.events, on: () => {}, appendEntry: () => {} } as unknown as ExtensionAPI);
|
||||
const supervisorCtx = { sessionManager: { getEntries: () => [] }, ui: { notify: vi.fn() } };
|
||||
try {
|
||||
restoredPlan(flow);
|
||||
supervisor.configure("restored-binding", "supervisor", supervisorCtx as any, true);
|
||||
await flow.hooks.get("session_start")({}, flow.ctx);
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
expect(supervisor.connected).toBe(true);
|
||||
if (cause === "explicit solo") await flow.commands.get("goals").handler("solo", flow.ctx);
|
||||
else supervisor.failReady("Supervisor quota exceeded");
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
expect(supervisor.connected).toBe(false);
|
||||
expect(supervisorCtx.ui.notify).toHaveBeenCalledWith(expect.stringContaining("Worker entered solo mode; this pairing is detached"), "error");
|
||||
expect(() => supervisor.steer("Obsolete advice.")).toThrow("pairing is detached");
|
||||
expect(flow.entries.at(-1)?.data).toMatchObject({ mode: "solo", approvalId: null });
|
||||
expect(flow.messages.filter(message => message.display && message.content.startsWith("UNSUPERVISED WORKER"))).toHaveLength(1);
|
||||
expect(wire.worker.sent.filter(message => message.failure?.startsWith("Worker entered solo"))).toHaveLength(1);
|
||||
expect(wire.supervisor.sent.some(message => message.failure?.startsWith("Worker entered solo"))).toBe(false);
|
||||
const count = wire.worker.sent.length + wire.supervisor.sent.length;
|
||||
await new Promise(resolve => setImmediate(resolve));
|
||||
expect(wire.worker.sent.length + wire.supervisor.sent.length).toBe(count);
|
||||
} finally { supervisor.detach(); rmSync(flow.cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
@@ -363,3 +363,23 @@ it("shows the supervisor role, waits through Pi retries, then reports a settled
|
||||
expect(runtime.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", undefined);
|
||||
} finally { rmSync(cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it("shows paused with the worker's solo detachment reason and refuses steering or approval", async () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "pi-goals-supervisor-detached-"));
|
||||
try {
|
||||
const runtime = setup(cwd, join(cwd, "plan.md"));
|
||||
await runtime.start();
|
||||
expect(runtime.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", "supervising");
|
||||
runtime.transport.replyToHello(false);
|
||||
const reason = "Worker entered solo mode; this pairing is detached. Restore supervision with /goals restart in the worker session.";
|
||||
runtime.transport.receive({ binding: "approval-1", role: "worker", kind: "hello", id: "hello", ready: false, failure: reason });
|
||||
expect(runtime.ctx.ui.notify).toHaveBeenCalledWith(reason, "error");
|
||||
expect(runtime.ctx.ui.setStatus).toHaveBeenLastCalledWith("pi-goals", "supervisor · paused");
|
||||
await expect(runtime.tools.get("SteerWorker").execute("id", { instruction: "Must not send." })).rejects.toThrow(reason);
|
||||
expect(runtime.transport.sent.filter(message => message.kind === "steer")).toHaveLength(0);
|
||||
const approval = await runtime.tools.get("ApproveGoal").execute("approve", { goal: "make the file", verifyOutputPath: "verify.txt" }, undefined, undefined, runtime.ctx);
|
||||
expect(approval.isError).toBe(true);
|
||||
expect(approval.content[0].text).toContain("disconnected or not ready");
|
||||
expect(runtime.transport.sent.at(-1)).toMatchObject({ kind: "hello", reply: true, failure: undefined });
|
||||
} finally { rmSync(cwd, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user