Skip to content

PAN-3668 - #3670

Closed
eltmon wants to merge 60 commits into
mainfrom
feature/pan-3668
Closed

eltmon wants to merge 60 commits into
mainfrom
feature/pan-3668

Conversation

@eltmon

@eltmon eltmon commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Issue: #3668

Acceptance Criteria

  • Add prime-agent to the canonical Harness and RuntimeName unions
  • Add Prime discriminator literals to the harness-behavior unions
  • Define PRIME_AGENT_BEHAVIOR and register it in the behavior map and lookup
  • Add prime-agent to flywheel, artifacts, telemetry, and context-preview enumerations
  • Add prime-agent to CLI --harness option help and regenerate the composer manifest
  • Add the prime-agent harness literal and primeAgent section to the config schema
  • Add the table-driven Overdeck-to-Prime provider and model mapping module
  • Resolve the prime-agent executable with actionable missing-binary errors
  • Add Prime Agent doctor checks for binary, version, RPC capability, and credentials
  • Implement strict LF-delimited JSONL framing for the Prime RPC protocol
  • Implement the persistent Prime RPC client: spawn, correlation, events, timeouts
  • Add the Prime runtime adapter and register it in the global runtime registry
  • Resume Prime sessions by verified session ID and fail loudly on mismatch
  • Enforce the managed-session policy that keeps Cloister the lifecycle authority
  • Launch managed work agents through the Prime RPC runtime
  • Launch supervised conversations through the Prime RPC runtime
  • Map Overdeck delivery, steering, abort, and kill to Prime RPC commands
  • Add the Prime transcript adapter and register it in the transcript registry
  • Map Prime session statistics into Overdeck cost and activity schemas
  • Project Prime sessions through dashboard contracts, routes, and the read model
  • Render Prime Agent in dashboard selection, badges, filters, and conversation views
  • Render and inject the prime-agent-global.md context artifact
  • Document Prime Agent setup, security boundary, and operational limits
  • Add the mechanical Prime Agent no-loss audit
  • Run full quality gates, throwaway dashboard boot, and the live Prime smoke test

panopticon-agent[bot] added 30 commits August 12, 2026 13:53
panopticon-agent[bot] added 3 commits August 17, 2026 09:35
panopticon-agent[bot] and others added 4 commits September 9, 2026 09:05
Resolves 46 conflicts between PAN-3668 (Prime Agent harness) and main's
OpenCode harness work (PAN-3783) plus the ~50 commits that followed.

Every harness enumeration is resolved as a union: opencode (main) and
prime-agent (this branch) both land in Harness, RuntimeName,
KNOWN_HARNESSES, HarnessMarker, the context-layer/telemetry/flywheel
literal sets, CLI --harness help, and the Settings/ModelPicker lists.

Where the two sides refactored the same code, main's newer structure wins
and prime-agent is re-added in main's idiom:

- sync.ts: keeps main's writeContextArtifactSync and the removal of
  project CLAUDE.md/AGENTS.md writing; prime-agent global render re-added
  in that idiom.
- conversation-runtime.ts: keeps main's Kimi resume contract, codex prompt
  files and managedStateKey; prime-agent fields folded into each branch.
- spawn-planning-session.ts: keeps main's delegation to
  claudeSystemPromptFiles, typed RuntimeName so prime-agent is accepted.
- system-prerequisites/harness-binary: keeps main's ExecutableResolution
  and the WSL-interop guard (PAN-3827) alongside the prime-agent probe.
- branding/ModelPicker tests: keep main's stronger icon-render assertion
  and HARNESS_OPTIONS-derived count, extended to prime-agent.

Conflicts where this branch had already replaced an inline harness union
with RuntimeName/Harness keep that refactor, which subsumes main's
opencode addition via KNOWN_HARNESSES.

normalizeResolution is exported so the prime-agent doctor consumes main's
widened resolver result instead of duplicating the rule.

Verified: typecheck, lint, build, slash-command regeneration, the Prime
no-loss audit, contracts, and the branding/ModelPicker suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The origin/main merge resolution adopted main's writeContextArtifactSync
for every rendered global layer, including the new prime-agent artifact.
That left writeRenderedGlobalContext with no callers anywhere in the tree
— it duplicated the same change-detect-then-write rule.

Removing it keeps one writer for Overdeck-owned context artifacts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
panopticon-agent[bot] and others added 4 commits September 18, 2026 04:05
The test-skip verification gate (PAN-3847) landed on main on 2026-09-17,
after this branch wrote the Prime Agent smoke on 2026-08-12. Its
`describe.skipIf(!runLive)` guard is exactly the shape the new gate
forbids, so merging main in turned a legitimate file into a required-gate
failure.

Rather than work around the gate, use the mechanism vitest.config.ts
already provides for opt-in suites: rename the file to
`prime-agent-smoke.slow.test.ts`. The slow lane is excluded from
`npm test`, from CI, and from verification runs unless
VITEST_INCLUDE_SLOW=1, so the suite needs no skip guard at all and the
gate has nothing to flag.

The env guard is replaced by a beforeAll that calls
requireHarnessBinary('prime-agent'). An enabled run without the binary
now fails immediately with the installation guidance instead of spawning
a shell that never produces a host and timing out 120s later inside
waitUntil().

docs/prime-agent-verification.md is updated to the new path, the new run
recipe, and the fail-fast behavior. It also drops
OVERDECK_PRIME_AGENT_PROVIDER, which the doc claimed but no source file
has ever read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
src/lib/cloister/verification-artifact.ts writes each gate run to
`.overdeck/verification/<timestamp>-<sha>.json`, but .gitignore only
covered `.overdeck/verification-latest.json`. Every workspace that runs
the verification gate is therefore left with an untracked directory that
`pan done` sees as a dirty tree.

The sibling artifact one line above is already ignored and the block's
own comment says workspace runtime stays local, so this is the same rule
applied to the directory form.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the suite in the slow lane surfaced the failure mode the skip
guard used to hide: the host does not come up, waitUntil burns its full
120s, and throws "Timed out waiting for Prime Agent production host"
with nothing to act on. The host was spawned with stdio: 'inherit', so
whatever it printed about the cause went to a stream vitest discards.

Spawn it through a helper that pipes and records stdout/stderr, and
include the tail of that output in the timeout error. The spawned
process is `dist/prime-agent-host.js`, whose RPC channel is its own
child pipe and its HTTP interface, so piping the wrapper's own streams
does not touch the protocol.

Removing the skip guard is only an improvement if an enabled run
explains itself; this is the other half of that change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s audit

The PAN-3668 branch widened the `--changed origin/main` gate set, and two
host-dependent failures came with it.

`makeDbLive` opened `overdeck.db` without running the init migration, and
`openDatabase` creates the file when it is missing. Building the layer
against a not-yet-created path therefore left a table-less database behind;
the next read-only open of that same path failed its schema audit with
"overdeck.db schema is incompatible", surfacing inside whatever unrelated
code read next (resources routes, `pan start`). It now runs the same
idempotent `runOverdeckMigrationSync` as the sync door.

The liveness no-loss audit already mocked `getDashboardApiUrlSync` so the
no-resume probe could not reach a live dashboard, but the probe
short-circuits on `OVERDECK_NO_RESUME` before it ever gets there. The
verification gate inherits that variable from a dashboard booted with
--no-resume, so every agent row carried gatingReason "Boot --no-resume" and
the fixture comparison became machine-dependent. The test now clears and
restores the variable alongside HOME.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@eltmon

eltmon commented Sep 18, 2026

Copy link
Copy Markdown
Owner Author

Review CHANGES REQUESTED for PAN-3668

Review — PAN-3668

Verdict: CHANGES REQUESTED — PrimeAgentRuntimeSync.killAgent skips process-tree termination whenever the abort RPC fails or hangs, so Cloister's automated kill leaves a Prime agent running while recording it as killed.

Context

  • Run ID: agent-pan-3668-review-37430f70
  • Manifest: /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668/.pan/review/agent-pan-3668-review-37430f70/context.json
  • Branch / workspace: feature/pan-3668 / /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668
  • HEAD reviewed: 37430f704da297143c81773384e64ef60fa49eb7
  • Base / prior cycle SHA: merge-base 38c9da1ce2a06531479edc27541c505c577f7e56; prior cycle be1bc0bb (approved, delta-only)
  • Spec: ${OVERDECK_HOME}/state/panopticon-cli/specs/2026-08-12-PAN-3668-add-prime-agent-as-a-managed-harness.xbrief.json (25 items, 58 acceptance criteria). The dispatch manifest carried an empty acceptanceCriteria array; criteria were read from the canonical xBRIEF.
  • Sole reviewer, all four dimensions, no convoy (per dispatch).

The integration is broad and, in most places, careful: strict LF framing that deliberately avoids readline, id-correlated RPC with bounded pending map, a table-driven provider map that refuses to invent a fallback model, loud-failing resume, a managed-session policy denylist, and complete operator documentation. One lifecycle path did not get the same care.

Blocking Findings

[correctness] killAgent never terminates the process tree when the abort RPC fails or hangs — src/lib/runtimes/prime-agent.ts:146

  • Severity: ! MUST

  • Classification / requirement scope: implementation defect; in_pr_scope

  • Trigger and changed-code connection:

    async killAgent(agentId: string): Promise<void> {
      await this.controller.abort(agentId);      // rejects -> function exits here
      await this.controller.terminate(agentId);  // never reached
    }

    productionController.abort is postPrimeAgentHost(agentId, { op: 'abort' })
    (src/lib/runtimes/prime-agent.ts:68). That helper rejects whenever the unix socket is
    missing (src/lib/prime-agent/session-controller.ts:57), the request errors (:69), or
    the host answers non-2xx (:65), and it sets no request timeout. Those split into
    two cases, and only the second leaks:

    1. Host already dead (socket missing, ECONNREFUSED). tmuxCreateSession runs
      bash -lc <command> with no keep-alive (src/lib/runtimes/tmux-cli.ts), so when the
      host process exits the pane and the tmux session are already gone. killAgent rejects,
      but there is nothing left to terminate. Consequence here is limited to the unhandled
      rejection at src/lib/cloister/service.ts:623 and a spurious Failed to kill log.
    2. Host alive, abort fails or times out — the Prime child is wedged, so the host's own
      RPC request times out (requestTimeoutMs is bound to --startup-timeout-ms, 30 s by
      default, host.ts:88) and answers 500. This is the leak. The tmux session, the host
      process, and the prime-agent child all survive, and terminate never runs.
  • Expected / observed / impact:

    • Expected (spec): runtime-adapter.ac3 — "killAgent aborts generation before terminating the process tree"; message-delivery.ac3 — "Kill issues abort, waits the bounded grace period, then terminates the owned process tree." Neither the bounded grace period nor an unconditional terminate exists on the path production takes.
    • Observed: abort rejects → terminate (the tmux kill) is skipped → in case 2 the session and child keep running.
    • Impact: src/lib/cloister/service-crash.ts:211 swallows the rejection with a console.error while host.emit({ type: 'killed_agent', agentId }) fires on the next line regardless. Cloister therefore records the agent as killed while it is still alive and still burning provider tokens — an unkillable zombie agent for the new harness. Secondarily, src/lib/cloister/service.ts:623 calls runtime.killAgent(agent.id) unawaited in the bulk-kill loop, so the same rejection surfaces there as an unhandled promise rejection inside the dashboard server.
    • ? Amplifier the author should check first, in one RPC call: does Prime's abort
      return success: false when nothing is currently streaming? rpc-client.route:100
      rejects on any success !== true, which the host turns into a 500. If abort on an idle
      session is not a success, then every kill of an idle Prime agent takes the
      skipped-terminate path — this stops being a wedged-child edge case and becomes "the
      default kill is broken." I could not check it without spending a live session; it is the
      single fact that most changes the severity picture.
  • Evidence: isolated probe in a gitignored .tmp/ directory, driving the production PrimeAgentRuntimeSync with an injected controller whose abort rejects the way postPrimeAgentHost does:

    abort OK      -> ["abort","terminate"]
    abort FAILS   -> ["abort"] | threw: MessageDeliveryFailed: Prime Agent host is unavailable for agent-x
    

    Evidence limit: static trace plus the adapter-level probe. I did not drive a live
    prime-agent session to observe the surviving tmux session directly, because doing so
    spends provider credits and touches live operator state; the reachability above is read
    from unchanged callers.

  • Fix: put the abort in a try/finally (or .catch(() => undefined)), insert the bounded grace period the AC names, and always terminate:

    async killAgent(agentId: string): Promise<void> {
      try { await this.controller.abort(agentId); } catch { /* host already gone */ }
      finally {
        await new Promise(resolve => setTimeout(resolve, PRIME_AGENT_KILL_GRACE_MS));
        await this.controller.terminate(agentId);
      }
    }

    killPrimeAgentSession in src/lib/prime-agent/session-controller.ts:74 already has the
    correct abort → grace → terminate shape; the adapter should use that shape rather than
    leaving it stranded (see non-blocking finding N1). Give postPrimeAgentHost a request
    timeout at the same time, so a wedged host cannot hang the kill.

Non-blocking Findings

N1 ~ The only code implementing the AC's abort→grace→terminate sequence is unreachable — src/lib/prime-agent/session-controller.ts:17

registerPrimeAgentSession, getPrimeAgentSession, hasPrimeAgentSession, and
killPrimeAgentSession are never called from production code — grep -rn across src/
finds only delivery.ts:315 (hasPrimeAgentSession) and the test file. The sessions
map is therefore always empty at runtime: hasPrimeAgentSession is permanently false,
and killPrimeAgentSession is dead code. The test that proves message-delivery.ac3
(tests/unit/lib/prime-agent/delivery.test.ts:51, "aborts, waits a bounded grace period,
and terminates") registers its own session into that empty map, so it verifies a code path
production never takes — which is why the blocker above went unnoticed. deliverPrimeAgentMessage
also always takes its postPrimeAgentHost branch (:43) rather than the in-process one.
Either wire the registry at host launch or delete it and move the grace-period logic into
the adapter, and retarget the test at PrimeAgentRuntimeSync.killAgent.

N2 ~ One malformed stdout line kills the Prime host with an uncaught exception — src/lib/prime-agent/host.ts:94

child.stdout.on('data', chunk => client.acceptStdout(chunk)) has no try/catch.
acceptStdout → framer.push throws PrimeAgentJsonlError on malformed JSON
(jsonl-framing.ts:69) or an oversized record (:74), and rpc-client.route throws on a
non-object record (rpc-client.ts:86) or a response missing its id (:92). A synchronous
throw inside a stream 'data' listener becomes an uncaught exception: the host process
dies, cleanup() never runs, the prime-agent child is orphaned, and no
prime-agent-launch-error file is written (that write lives only in main().catch,
host.ts:165). Any upstream banner or warning printed to stdout triggers it.

The framer also cannot recover even if the throw were caught: push throws from line 43
before the this.chunks = [] reset on line 44, so the buffered prefix and start offset
are left stale and every later record is corrupted. That is the opposite of
rpc-framing.ac3 — "a malformed JSON record yields a per-record error while later
records still parse
." The test (jsonl-framing.test.ts:32) only asserts that push
throws, so it does not cover the "later records still parse" half of the criterion.
Fix: have push skip and report the bad record after resetting state, and wrap
acceptStdout so a framing error closes the client cleanly instead of killing the process.

N3 ~ Keyed (at-most-once) deliveries to a Prime agent bypass the dedup cascade — src/lib/agents/delivery.ts:315

The Prime branch sits above the if (dedupKey !== undefined) branch at :346, so a keyed
delivery to a prime-agent target never reaches deliverKeyedAgentMessage and never
returns deduplicated. The key is silently ignored, so a retried keyed message (a verdict
hand-back, for instance) is delivered twice. Move the Prime branch below the keyed branch,
or teach deliverKeyedAgentMessage about the Prime tier.

N4 ~ The provider table and the credential gate disagree, and the two launch paths disagree with each other — src/lib/prime-agent/provider-map.ts:12

API_KEY_PROVIDER_IDS maps twelve providers (anthropic, openai, google, kimi, minimax,
openrouter, zai, mimo, xai, groq, cerebras, mistral), but getProviderAuthMode
(src/lib/agents/provider-auth.ts:8) returns a mode only for anthropic, openai, and
google — undefined for the other nine. Work-agent spawn treats undefined as fatal
(src/lib/runtimes/prime-agent.ts:45, "Prime Agent provider credentials are unavailable"),
so nine mapped providers can never launch a work agent. The conversation path takes the
opposite branch: getPrimeAgentBaseCommand (src/lib/agents/runtime-command.ts:36)
defaults ?? 'api-key' and launches them without any credential check. Same model, two
different answers depending on whether it is a work agent or a conversation. Also,
provider-map.ac2 ("a missing credential produces an error naming the missing variable
or auth file and the command to configure it
") is not implemented anywhere: the map does
no credential check at all, and the doctor's fix text is generic ("Configure credentials
for the provider used by the selected Prime Agent model", doctor-prime-agent.ts:77).

Relatedly, harnessOptionsFor (ProviderManagementSection.tsx:88) offers prime-agent
for every provider including nous and dashscope, which have no entry in the map at all.
Those fail loudly at launch with a typed PrimeAgentProviderMappingError, which satisfies
provider-map.ac1/ac3, so this is a UX rough edge rather than a silent wrong result.

N5 ~ Documentation promises a resume check that the code does not perform — configuration/harnesses.mdx

The new section states: "A detached session resumes only when its recorded model,
provider, and workspace
match." resumePrimeAgentSession
(src/lib/prime-agent/session-resume.ts:24) verifies the session file is readable and that
get_state returns the same session id — nothing more. Neither model, provider, nor
workspace is compared. Either narrow the sentence to the session-identity check that exists
(which is what session-resume-recovery.ac1/ac2 actually require) or add the comparison.

N6 ≉ Prime Agent and oh-my-pi share a badge glyph — src/dashboard/frontend/src/components/shared/branding/index.tsx:132

'prime-agent': { …, Icon: PiHarnessIcon } reuses the oh-my-pi icon, so the two harnesses
are visually indistinguishable in every badge surface. dashboard-frontend.ac1 asks for "a
correct harness badge"; the label text is right, the glyph is another harness's. A
lettermark or a distinct glyph would settle it.

N7 ≉ The new dashboard projection adds a synchronous directory scan per agent row, for every harness — src/dashboard/server/services/runtime-session-projection.ts:10

projectRuntimeSession was spliced into both the session tree
(routes/projects.ts:295) and the activity payload (routes/command-deck.ts:383) for all
harnesses, not just Prime. Only the Prime adapter implements getSessionMetrics, so every
other harness falls through to runtime.getLastActivity(agentId). For claude-code that is
getSessionPath → getActiveSessionId → getMostRecentJSONL → getSessionFilesSync
(src/lib/cost-parsers/jsonl-parser.ts:103), a readdirSync plus a sort comparator that
calls statSync twice per comparison — O(N log N) blocking syscalls on the dashboard event
loop. getActiveSessionId always returns null here: no sessions-index.json exists in any
of the 187 directories under ~/.claude/projects/, so the expensive fallback is the only
path taken.

Measured on this host (warm cache): 0.46 ms per call for a typical 36-file workspace
project dir, 9.1 ms for a 510-file dir. At a handful of rows per issue that is a few
milliseconds — bounded, and I have not demonstrated material impact, so this is advisory.
It becomes material for agents whose project dir holds hundreds of transcripts. Gating the
call on harness === 'prime-agent', or implementing getSessionMetrics for the other
adapters, removes it. No key collision: neither call site sets lastActivity,
tokenUsage, or cost earlier in its object literal, so the spread only adds fields.

N8 ≉ Two holes in the mechanical no-loss audit — tests/unit/lib/prime-agent-no-loss-audit.test.ts:6

const ALL = ['claude-code', 'ohmypi', 'codex', 'acp', 'kimi-code', 'muse', 'prime-agent']
omits 'opencode', a pre-existing literal present in every surface the test checks, so its
disappearance would not fail the audit — exactly what no-loss-audit.ac3 exists to catch.
Separately, row S9 ("Cost source … prime_agent source … [x]") passes on
expect(source('src/lib/overdeck/cost.ts')).toContain("'prime_agent'"), a string match the
type-union addition satisfies on its own. The reconcile sweep short-circuits for that
source — if (source !== 'ohmypi' && source !== 'codex') return empty;
(src/lib/overdeck/cost.ts:628) — so POST /costs/reconcile {source:'prime_agent'} is
accepted and silently does nothing, and Prime sessions never reach the durable cost ledger.
acp sits in the same union with the same inert behavior, so this follows existing
precedent rather than introducing a new gap, and the live projection path does surface
Prime cost to the dashboard; it is the audit row that overstates what is wired.

N9 ? Prerequisite version probing changed for every tool, not just Prime — src/lib/system-prerequisites.ts:224

defaultProbe now returns stdout || stderr instead of stdout. Harmless where stdout is
populated, but a prerequisite that prints nothing to stdout and an error to stderr will now
report "found" with the error text as its version string. Worth a deliberate confirmation
that no existing prerequisite behaves that way.

N10 ? Session id and session path are conflated — src/lib/prime-agent/host.ts:150

sessionId = String(state.data?.sessionFile ?? state.data?.sessionId ?? agentId) writes the
session file path into prime-agent-session-id, and the resume path then passes the same
value as both sessionId and sessionPath (:101-102). It works today only because
sessionFile wins in both expressions. A Prime build that reports sessionId without
sessionFile fails the launch at :156 ("did not report a durable session path"), which is
a loud failure rather than a wrong result — but the two concepts should not share a variable.

N11 ? file-size-allowlist.txt accumulated six rows for src/lib/config-yaml/schema.ts and three for launcher-generator.ts across merge resolutions, including a 900 entry inserted below higher values. Cosmetic if the guard takes the maximum; worth tidying.

Coverage

All 122 changed files were accounted for. Ledger by group:

Group Files Disposition
New Prime core (src/lib/prime-agent/*: jsonl-framing, rpc-client, host, host-http, policy, provider-map, session-resume, session-controller, launch-command) 9 Reviewed line by line — sources of the blocker and N1–N5, N10
Runtime adapter + registry (src/lib/runtimes/prime-agent.ts, index.ts, types.ts) 3 Reviewed — source of the blocker, N7
Contracts (types.ts, harness-behavior.ts, flywheel.ts, artifacts.ts, telemetry.ts, context-layers.ts, composer-commands.generated.ts) 7 Reviewed — additive, every pre-existing literal preserved
Config (schema.ts, defaults.ts, merge.ts, roles.ts) 4 Reviewed — primeAgent normalized with a validated positive-integer timeout and no model field
Delivery / conversation / transcript / cost wiring 10 Reviewed — source of N3, N8
Context layers + sync 4 Reviewed — prime-agent-global.md rendered and injected via --append-system-prompt; nothing writes ~/.prime/agent
Dashboard server routes + services 7 Reviewed — source of N7
Dashboard frontend 13 Reviewed — source of N6
CLI (index.ts, doctor.ts, doctor-prime-agent.ts, strike.ts, artifacts.ts, flywheel.ts) 6 Reviewed
Harness binary / prerequisites / launcher generator 3 Reviewed — source of N9
Tests (unit, integration, contracts) 31 Reviewed — source of N1, N2, N8
Docs + evidence (harnesses.mdx, context-layers.mdx, MODEL_ROUTING.md, HARNESSES.md, TELEMETRY.md, no-loss audit, verification, screenshot) 8 Reviewed — source of N5
Build / infra (tsdown.config.ts, .gitignore, guard-agent-dir-removal.sh, allowlists, baselines) 5 Reviewed — prime-agent-host entry added; guard allowlist scoped to the two file removals
Type-widening-only touches (RuntimeName substituted for inline harness unions across cloister, planning, specialists, lifecycle) 12 Reviewed — mechanical, no behavior change

Four dimensions

  • Correctness — one blocker (kill path) plus N1–N3, N10. Framing, id correlation, pending-map bound, process-exit rejection, and resume verification are otherwise sound. resolveAllowedHarness widening to KNOWN_HARNESSES adds exactly one literal (prime-agent) over the old inline allowlist; no unintended harness became selectable.
  • Security — no blocking finding. The host binds a unix socket under ~/.overdeck/sockets with 0700 dirs and a per-launch 32-byte token in a 0600 file, checks the token before acting, caps request bodies at 1 MiB, and caps concurrency at 8. The child is spawned with an argument array, never a shell string (host.ts:86), satisfying rpc-client.ac4. quoteShell/shellQuote in launch-command.ts:13 and prime-agent.ts:76 implement the standard POSIX '\'' escape correctly for the launcher script. Token comparison is non-constant-time, which is within Overdeck's documented trusted-local threat model and not a finding. No secret is logged: framing errors report byte counts, not record contents.
  • Performance — N7 only, advisory with measurements. The RPC client bounds its pending map (128) and record size (8 MiB); host.ts debounces stats writes at 250 ms and serializes them through a promise chain. No execSync was added to any server-reachable path. The spawn readiness loop polls with awaited 100 ms sleeps, not busy-waiting.
  • Requirements / UX — one AC failure (blocking), plus partial misses in N2 (rpc-framing.ac3), N4 (provider-map.ac2), N6 (dashboard-frontend.ac1), N8 (no-loss-audit.ac3). Every other criterion I could check statically or by test is met.

AC-to-evidence matrix (by plan item)

Item Status Evidence
contracts-harness-literal met types.ts union + KNOWN_HARNESSES; RuntimeName widened; typecheck green
contracts-behavior-discriminators met all six discriminator unions carry their Prime literal
contracts-prime-behavior-record met PRIME_AGENT_BEHAVIOR uses only Prime values; registered in BEHAVIORS and getHarnessBehavior
contracts-enumerations met flywheel, artifacts, telemetry, context-preview all include prime-agent; prior literals intact
cli-harness-flag-help met --harness help updated on plan/start/strike; composer manifest regenerated
config-schema-prime-agent met primeAgent section parses binaryPath/rpcStartupTimeoutMs, no model field; merge test covers default, override, and rejection
provider-map partially met ac1/ac3 met; ac2 unmet (no credential check names a variable or command) — N4
harness-binary-resolution met HARNESS_BINARY_BY_RUNTIME + configuredHarnessBinaryPath + "Prime Agent" label in requireHarnessBinary
doctor-prime-checks met binary, semver range, --mode rpc probe, credential check; results are warn not error, consistent with the optional-harness precedent
rpc-framing partially met ac1 (real U+2028/U+2029 bytes in the fixture) and ac2 met; ac3 half-unmet — N2
rpc-client met correlation, event channel, process-exit rejection, fake-timer timeouts, pending bound, argument-array spawn
runtime-adapter ac3 unmet registry + heartbeat met; killAgent — blocking finding
session-resume-recovery met missing/unreadable file and id mismatch both stop the process and raise PrimeAgentResumeError
managed-session-policy met all five policy rules present; 16 daemon commands denied at rpc-client.request; checkpoint recorded as policy-enforced
work-agent-launch met spawn-prime-agent.test.ts asserts explicit provider/model/session-dir and no claude-code fallback
conversation-launch met resolveAllowedHarness accepts it; preparePrimeAgentConversationLaunch injects the policy
message-delivery ac3 unmet ac1/ac2 met via get_state.isStreaming; ac3 tested only against the unreachable registry — blocker + N1
transcript-adapter-prime met 'prime-agent': primeAgentAdapter registered; fixture renders user/assistant/thinking/tool/compaction/error
cost-usage-mapping met parser omits absent fields rather than substituting; reconcile inertness noted in N8
dashboard-api-projection met both payloads project through the runtime read door; no route parses Prime session files directly
dashboard-frontend partially met picker, filters, labels, token/cost surfaces present; badge glyph duplicated — N6
context-renderer-prime met prime-agent-global.md rendered by pan sync, injected via --append-system-prompt, preview enumerated; nothing touches ~/.prime/agent
docs-setup-reference partially met install, pinning, binaryPath, auth, mapping, recovery, security boundary, disabled automation, uninstall, subscription-billing caveat all present; one overstated resume claim — N5
no-loss-audit partially met ac1/ac2 met; ac3 has a hole — N8
verification-gauntlet unverified at this HEAD docs/prime-agent-verification.md records a full gate run, a throwaway Node 22 dashboard boot, a live smoke, and a Playwright screenshot — dated 2026-08-12, five commits before this HEAD. Not reused as exact-HEAD evidence.

Verification

Run at HEAD 37430f70:

Check Result
npm run typecheck Passed (exit 0). Dashboard guard: 26 known errors, none new. Frontend guard: 0 known, none new.
npm run lint:effect-diagnostics Passed — 308 known findings, no NEW: diagnostics.
Focused Prime suites (15 files: src/lib/prime-agent/__tests__/, tests/unit/lib/prime-agent/, no-loss audit, runtime adapter, spawn, conversation-runtime, transcript, cost parser, doctor, context-layers, runtime-session-projection) Passed — 68/68 in 115.8 s
killAgent abort-failure probe (isolated, gitignored .tmp/, removed afterwards) Reproduced the blocker: ["abort"] with no terminate
getSessionFilesSync timing probe against real ~/.claude/projects dirs 0.46 ms (36 files) / 9.1 ms (510 files) per call; 0 of 187 dirs carry a sessions-index.json

Not run, and why:

  • npm test (full suite) — the pipeline's verification stage owns it; the role's budget says not to repeat it. Focused suites covered every Prime-touching test file.
  • npm run build, npm run lint — no exact-HEAD result available; deferred to the verification stage.
  • tests/integration/prime-agent-smoke.slow.test.ts — opt-in slow lane (VITEST_INCLUDE_SLOW=1); it drives a real binary against a real provider, which spends credits. Recorded as unverified at this HEAD; the 2026-08-12 evidence in docs/prime-agent-verification.md predates the last five commits.
  • Live Prime session to observe the surviving tmux session for the blocker — same reason; the adapter probe plus the caller trace carry the finding.

Scope Note

Four REVIEWER_READY signals arrived mid-review (correctness, performance, requirements,
security) pointing at .pan/review/agent-pan-3668-review-be1bc0bb/ — a previous cycle's
run directory (commit be1bc0bb, 2026-09-09). This dispatch names run
agent-pan-3668-review-37430f70 and states there is no convoy and that I review every
dimension myself, so those lanes are stale and did not feed this verdict. I read the stale
correctness.md for context: it is a delta-only report that approves on the grounds that
everything outside the last few commits was "unchanged from the prior clean review" — it
never traced the kill path, which is how a blocker that has been present since
5c51e210b0d ("feat: register Prime Agent runtime adapter") survived several approving
cycles. Prior approval is not evidence of safety here; the verdict-routing drift between
be1bc0bb and 37430f70 is worth an operator look.

Source: /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668/.pan/review/agent-pan-3668-review-37430f70/review.md

Required action

Fix every blocking review finding, commit the fixes, then re-request review with:

pan review request PAN-3668 -m "Fixed review issues"

panopticon-agent[bot] and others added 3 commits September 18, 2026 05:08
…ew findings

Blocking: `PrimeAgentRuntimeSync.killAgent` awaited `controller.abort` before
`controller.terminate`, so any abort rejection skipped the terminate entirely.
`postPrimeAgentHost` rejects when the host socket is missing, when the host
answers non-2xx, and (previously, with no request deadline) never at all when
the Prime child is wedged. In the wedged case the tmux session, the host
process, and the Prime child all survived while `service-crash.ts` swallowed the
rejection and emitted `killed_agent` — an agent recorded as killed that kept
burning provider tokens. The kill now runs abort → bounded grace → terminate,
with the abort best-effort and the terminate unconditional, and
`postPrimeAgentHost` carries a request timeout so a wedged host cannot hang the
kill it is the subject of.

The in-process session registry that held the only correct abort → grace →
terminate sequence was never populated in production, so `killPrimeAgentSession`
was dead code and `hasPrimeAgentSession` was permanently false — and the test
proving that AC exercised a path production never takes, which is how the kill
defect survived several approving review cycles. The registry is gone;
`deliverPrimeAgentMessage` is now the host POST it always was at runtime, and
the tests drive `PrimeAgentRuntimeSync.killAgent` and a real host socket.

Also from the review:

- A malformed stdout record killed the host with an uncaught exception, and the
  framer reset its buffer *after* parsing, so every later record was corrupted.
  `push` now resets state before parsing and reports per-record errors, and
  `acceptStdout` never throws — the "later records still parse" half of
  rpc-framing.ac3.
- A keyed delivery to a Prime agent fell past the dedup branch with the key
  silently dropped, so a retried keyed message was delivered twice. It now
  refuses, like the ACP tier, because one host POST cannot enforce at-most-once.
- Nine mapped providers could never launch a work agent (`getProviderAuthMode`
  returning undefined was read as "no credentials") while the conversation path
  launched them with no check at all. Both paths now default to api-key and run
  one credential gate that names the missing variable or auth file and where to
  set it (provider-map.ac2). Its test file also moved into `__tests__/`, where
  vitest actually collects it — it had never run.
- The stderr version fallback is now opt-in per prerequisite instead of applying
  to all thirteen.
- Prime Agent gets its own badge glyph instead of oh-my-pi's.
- The runtime projection no longer calls `getLastActivity` for harnesses without
  cached metrics, which put claude-code's readdir + statSync sort on the
  dashboard event loop once per agent row.
- The no-loss audit's harness list was missing `opencode`; the docs claimed a
  resume check on model, provider, and workspace that the code does not perform;
  the file-size allowlist carried nine stale duplicate rows (the guard takes the
  last row per path, not the highest).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tderr version

Three gaps left by the review-fix commit:

- The keyed-delivery refusal for a Prime target had no test. Added alongside the
  ACP and Channels refusals it is modelled on.
- `delivery.test.ts` pointed OVERDECK_HOME at a directory its own afterEach had
  just deleted, leaving the shared per-worker home stale for every test file
  after it in that worker. It now saves and restores.
- `doctor-prime-agent` passes its own probe, which read stdout only — so with
  the stderr fallback now opt-in per prerequisite, the doctor would have
  reported "did not return a semantic version" for a healthy Prime install. It
  honours the same option, and its credential fix text now names where a
  credential can live instead of saying "configure credentials".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The opt-in stderr fallback passed an options object on every probe call, which
changed the call shape for all thirteen prerequisites and broke the Kimi probe
assertions in `src/lib/acp/__tests__/kimi.test.ts` and
`tests/integration/cli/doctor.test.ts`. Only the prerequisite that needs the
fallback passes it now; every other call is byte-for-byte what it was.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@eltmon

eltmon commented Sep 18, 2026

Copy link
Copy Markdown
Owner Author

Review CHANGES REQUESTED for PAN-3668

Review — PAN-3668

Verdict: CHANGES REQUESTED — the new harness reaches canUseHarnessSync's legacy fallthrough, so prime-agent + Anthropic model + subscription auth is allowed and offered unlocked in the pickers — the exact cell the same file blocks for ohmypi on Claude Code subscription Terms of Service grounds.

Context

  • Run ID: agent-pan-3668-review-93cd6317
  • Manifest: /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668/.pan/review/agent-pan-3668-review-93cd6317/context.json
  • Branch / workspace: feature/pan-3668 / /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668
  • HEAD reviewed: 93cd6317d3430311f889c127a66d47af34cf6a1e (= PR PAN-3668 #3670 head, confirmed via gh pr view)
  • Base: merge-base 38c9da1ce2a06531479edc27541c505c577f7e56. Prior cycle: 37430f70 (CHANGES REQUESTED, kill path)
  • Delta reviewed this cycle: 4 commits, 27 files, +675/-184 (325bb233, 26be13e9, 959a437c, 93cd6317)
  • Spec: ${OVERDECK_HOME}/state/panopticon-cli/specs/2026-08-12-PAN-3668-add-prime-agent-as-a-managed-harness.xbrief.json (25 items, 58 acceptance criteria, nested as items[].items[] with kind: acceptance_criterion). The dispatch manifest again carried an empty acceptanceCriteria array; criteria were read from the canonical xBRIEF.
  • Sole reviewer, all four dimensions, no convoy (per dispatch).

Every prior-cycle finding was addressed, and addressed well. The kill-path blocker is fixed at the adapter with the abort made best-effort, a bounded grace period, and an unconditional terminate; the unreachable session registry is gone rather than papered over; the framer now reports per-record errors instead of killing the host; the provider table and the credential gate now give the same model the same answer on both launch paths. The blocker below is one the previous cycles — mine included — did not look for: the harness-policy gate is the one canonical enumeration this PR never extended, and it is the one that encodes a Terms of Service rule.

Blocking Findings

[requirements/security-policy] prime-agent has no harness-policy decision, so Anthropic + subscription is allowed where ohmypi is blocked on ToS grounds — src/lib/harness-policy.ts:150

  • Severity: ⊗ MUST NOT

  • Classification / requirement scope: validation gap (missing policy decision) plus a specification defect; in_pr_scope

  • Trigger and changed-code connection:

    canUseHarnessSync branches for opencode, muse, claude-code, codex, acp, kimi-code, and ohmypi. It has no prime-agent branch, so the new runtime falls through to the terminal // harness === 'pi' (legacy) return ALLOWED. The file's own header calls itself the "Single source of truth for 'is this {harness, model, authMode} combination allowed?'" and states that "Every spawn entry point and every harness/model picker UI MUST call canUseHarness()". Rule 2 there reads: "ohmypi running an Anthropic model under Anthropic subscription auth is blocked (Claude Code subscription terms forbid using the Anthropic subscription with non-Anthropic harnesses)."

    This PR does not merely leave that cell undecided — its new code builds the path through it:

    • SUBSCRIPTION_PROVIDER_IDS.anthropic = 'anthropic' (src/lib/prime-agent/provider-map.ts:69) gives Anthropic a Prime subscription provider ID.
    • SUBSCRIPTION_CREDENTIALS.anthropic = { authFile: '~/.claude/.credentials.json', command: 'claude /login' } (:50, added this cycle) makes the Claude Code OAuth credential file the thing the launch gate checks, and tells the operator to run claude /login to supply it.
    • getProviderAuthMode (src/lib/agents/provider-auth.ts:10) returns 'subscription' for any Anthropic model whenever the operator is logged into Claude Code, and both launch paths now pass that through (src/lib/runtimes/prime-agent.ts:50, src/lib/agents/runtime-command.ts:38). No operator opt-in is involved — subscription is the automatic answer on a Claude-subscription host.

    The picker half is missing too: buildHarnessPolicyDecisions (src/lib/harness-policy-decisions.ts:78) enumerates seven harnesses by hand and omits prime-agent, and the map is typed Record<string, Record<string, …>> so typecheck cannot catch the omission. ModelPicker.tsx:47 resolves a missing key as ?? { allowed: true }. That file's header documents this exact failure mode from PAN-2528: "the previous in-route implementation emitted a 'pi' decision key while the pickers queried 'ohmypi', so the ohmypi + Anthropic + subscription ToS block was silently dropped before the picker UI could read it." The same drop has happened again for the new harness. harnessOptionsFor (ProviderManagementSection.tsx:88) also offers prime-agent for every provider, Anthropic included.

  • Expected / observed / impact:

    • Expected: a new harness gets an explicit decision in the policy gate, and the Anthropic + subscription cell resolves the same way it does for every other non-claude-code harness — or the operator makes an explicit, documented decision that Prime is different.
    • Observed: canUseHarnessSync('prime-agent', 'claude-sonnet-5', 'subscription') returns { allowed: true } with no reason, the pickers receive no decision and default to allowed, and the launch assembles prime-agent --provider anthropic --model claude-sonnet-5 against the Claude Code OAuth credentials.
    • Impact (picker half, independent of the ToS question): the missing decision key drops every decision the gate already makes for prime-agent, not just the Anthropic one. canUseHarnessSync('prime-agent', 'gpt-5.5', 'api-key') reaches canUseModelWithAuthSync before any harness branch and returns SUBSCRIPTION_ONLY_MODEL_BLOCK; a kimi-code/* id returns KIMI_NATIVE_ID_FOREIGN_HARNESS_BLOCK. The spawn gate (conversation-runtime.ts:358) refuses both, but the picker receives no decision and offers those rows unlocked, so the operator picks a combination that then fails at launch. That is a demonstrated offer-then-refuse defect regardless of how the ToS question is decided, which is why fix (2) below is unconditional.
    • Impact: Overdeck ships a default-path way to run Anthropic models under the Claude Code subscription through a third-party binary — the thing the repo blocks mechanically for ohmypi and documents in configuration/harnesses.mdx:528 ("only the claude-code binary may invoke Anthropic models when you're authenticated via the Claude Code OAuth subscription"), while the same doc's harness table says claude-code is "Required for Anthropic-subscription users". This is a compliance decision that belongs to the operator, and the code currently makes it silently.
  • Evidence: runtime probe at this HEAD, in a gitignored .tmp/ script driving the production functions, removed afterwards (tree clean):

    canUseHarnessSync(claude-code,  claude-sonnet-5, subscription) -> true
    canUseHarnessSync(ohmypi,       claude-sonnet-5, subscription) -> false | Claude Code subscription Terms of Service restrict Anthropic…
    canUseHarnessSync(prime-agent,  claude-sonnet-5, subscription) -> true
    policy map keys served to pickers: [claude-code, ohmypi, codex, acp, opencode, kimi-code, muse]
    picker lookup for prime-agent: (undefined -> ModelPicker.tsx:47 defaults to { allowed: true })
    
    getProviderAuthMode(claude-sonnet-5) -> subscription
    route(subscription) -> { provider: 'anthropic', model: 'claude-sonnet-5' }
    ~/.claude/.credentials.json present: true
    credential gate: PASS (launch proceeds)
    

    Evidence limit: I did not spawn a live Prime session (that spends provider credits and, if the ToS reading is what the repo says, would be the very act in question). Every step above is the production function on this host's real auth state; only the final prime-agent process execution is unobserved.

  • Fix — the mechanical half is small, the policy half is the operator's:

    1. Add an explicit prime-agent branch to canUseHarnessSync rather than letting it reach the legacy fallthrough. Mirroring ohmypi keeps the repo self-consistent: provider.name === 'anthropic' && authMode === 'subscription' → blocked with a reason naming the ToS rule and the API-key alternative.
    2. Add 'prime-agent': canUseHarnessSync('prime-agent', model, authMode) to buildHarnessPolicyDecisions, so the pickers can lock and explain the row. A Record<RuntimeName, …> type there would have made this omission a typecheck failure — worth doing while the file is open.
    3. Drop the anthropic entry from SUBSCRIPTION_CREDENTIALS and SUBSCRIPTION_PROVIDER_IDS, leaving openai/openai-codex as the "one subscription-auth path" the plan actually asked for, and correct the docs/MODEL_ROUTING.md sentence accordingly.
    4. Specification correction. The plan's provider-map narrative asks for "at minimum OpenAI API key, Anthropic API key, Prime Inference API key, and one subscription-auth path", and docs-setup-reference.ac3 requires "the caveat that Claude Pro/Max subscription use through Prime bills as extra usage, not plan quota". The implementation followed both faithfully; the spec is what never reconciled a Claude-subscription path with the repo's ToS gate. AC3's sentence should be rewritten to say the combination is refused by harness policy (or, if the operator decides Prime is genuinely different from ohmypi here, that decision needs to be explicit in harness-policy.ts and in the docs, with the reason — not an implicit fallthrough).

Non-blocking Findings

N1 ~ The subscription credential gate re-derives auth presence from one of the two known locations, so a macOS keychain login reads as "not signed in" — src/lib/prime-agent/provider-map.ts:50

assertPrimeAgentCredentialAvailable tests existsSync('~/.claude/.credentials.json'). getClaudeAuthStatus — the resolver that produced the 'subscription' answer in the first place — checks that file and then the macOS Keychain (src/lib/claude-auth.ts:86, platform !== 'darwin' guard; :118 for the file), because per its own comment the flat file is "Linux, older Claude Code versions" while Claude Code ≥2.x on macOS stores credentials in the Keychain. On such a host loggedIn is true, the file is absent, and the new gate throws "~/.claude/.credentials.json is missing. Run claude /login to sign in" at an operator who is already signed in. src/lib/remote/fly-provider.ts:495-506 shows the codebase's established file-then-keychain pattern. This is moot if fix (3) above removes the Anthropic subscription entry; if that entry stays for any reason, the gate must consult getClaudeAuthStatus instead of existsSync. Evidence limit: static trace plus the repo's own documented platform split; I have no macOS host. The OpenAI subscription path has no such divergence — getOpenAIAuthStatus reads exactly ~/.codex/auth.json (src/lib/openai-auth.ts:53).

N2 ~ A wedged host stretches kill to ~31 s while Cloister has already emitted killed_agent

killAgent now always terminates, which was the point — but postPrimeAgentHost defaults to PRIME_AGENT_HOST_REQUEST_TIMEOUT_MS = 30_000 (session-controller.ts:34) and the grace period adds 1 s, so the wedged-child case takes ~31 s to bring the tree down. service-crash.ts:211 emits killed_agent immediately either way, so the dashboard shows the agent killed for half a minute while it still runs. The kill path explicitly tolerates abort failure, so it can afford a much shorter deadline than a normal message POST: pass an explicit few-second timeout for the { op: 'abort' } call.

N3 ≉ The credential message points at settings keys the Settings UI cannot write — src/lib/prime-agent/provider-map.ts:118

The fix text says set "api_keys.<provider>" in <settings file>, but ApiKeysConfig (src/lib/settings.ts:64) declares no anthropic, groq, cerebras, or mistral field, and settings-api.ts:989 does not serve them either, so those four are reachable only by hand-editing settings.json. Validation is permissive (settings.ts:201) and loadSettingsSync's deep merge preserves unknown keys, so a hand-edited value is honoured and the env-var half of the message always works — the advice is incomplete rather than wrong.

N4 ? The credential test reads the host's real ~/.overdeck/settings.json — src/lib/prime-agent/__tests__/provider-map.test.ts:48

"names the missing variable and where to set it" deletes MINIMAX_API_KEY from the environment but assertPrimeAgentCredentialAvailable then calls loadSettingsSync(), which reads the operator's real settings file. It passes here because this host has none; on a host with api_keys.minimax configured the assertion inverts. Point OVERDECK_HOME at a temp dir in beforeEach, or inject the settings reader.

N5 ≉ Carried, unchanged: harnessOptionsFor (ProviderManagementSection.tsx:88) still offers prime-agent for nous and dashscope, which have no provider-map entry. They fail loudly at launch with PrimeAgentProviderMappingError, which satisfies provider-map.ac1/ac3; it remains a UX rough edge.

N6 ≉ Carried, unchanged and consistent with precedent: POST /costs/reconcile { source: 'prime_agent' } is accepted and silently does nothing, because the sweep short-circuits for anything but ohmypi/codex (src/lib/overdeck/cost.ts:628). acp behaves identically. The live projection still surfaces Prime cost to the dashboard; it is audit row S9 that overstates what is wired.

N7 ? Carried, now documented deliberately: host.ts:164-166 still resolves the session id and the session path from the same state.data.sessionFile. The new comment explains why untangling needs a live Prime binary and confines the change to two lines. Fine as a recorded limitation.

N8 ? The unterminated-oversize error reports the wrong byte count — src/lib/prime-agent/jsonl-framing.ts:82

When a tail past the cap is dropped, the error carries tail.byteLength (the last chunk) rather than the accumulated bufferedBytes it just discarded, so a record blown past 8 MiB by a 1 KiB chunk reports "1024 bytes received". Cosmetic: the message is diagnostic only, and the reset/resynchronise behaviour is correct.

Coverage

Delta ledger (this cycle, all 27 files)

File Disposition
src/lib/runtimes/prime-agent.ts Reviewed — prior blocker fixed: abort in try/catch, bounded grace, unconditional terminate; authMode ?? 'api-key' resolves the two-answers split (N4 of prior cycle)
src/lib/prime-agent/session-controller.ts Reviewed — dead in-process registry deleted (prior N1); request timeout added; four removed exports have zero remaining references (grep across src/ + tests/)
src/lib/agents/delivery.ts Reviewed — keyed deliveries to a Prime target now refuse, matching the ACP tier verbatim (delivery.ts:554); prior N3 closed
src/lib/prime-agent/jsonl-framing.ts Reviewed — per-record errors, state reset before parse, bounded resynchronise; source of N8
src/lib/prime-agent/rpc-client.ts Reviewed — acceptStdout cannot throw; onRecordError channel
src/lib/prime-agent/host.ts Reviewed — onRecordError → stderr, so a banner line no longer kills the host and orphans the child (prior N2); session id/path split into two expressions (N7)
src/lib/prime-agent/provider-map.ts Reviewed — credential gate naming variable + settings key + auth file + login command (provider-map.ac2 now met); source of the blocker and N1, N3
src/lib/prime-agent/launch-command.ts Reviewed — single gate point shared by both launch paths
src/lib/system-prerequisites.ts Reviewed — stderr version fallback is now opt-in per prerequisite (versionFromStderr), closing prior N9 for every other tool
src/cli/commands/doctor-prime-agent.ts Reviewed — probe honours the opt-in; fix text names both credential homes
src/dashboard/server/services/runtime-session-projection.ts Reviewed — gated on getSessionMetrics, so no harness but Prime pays a blocking directory scan (prior N7); no field regression versus main, which carried none of these fields
src/dashboard/frontend/.../branding/index.tsx Reviewed — own lettermark, distinct from oh-my-pi (prior N6)
src/lib/overdeck/infra.ts Reviewed — makeDbLive applies the schema like the sync door; idempotent (runOverdeckMigrationSync short-circuits on an existing agents table). Callers are terminal-issues.ts and one integration test, not the dashboard boot path. See Scope Note
configuration/harnesses.mdx Reviewed — resume claim narrowed to the check that exists (prior N5). Its harness table and ToS section are what the blocker contradicts
scripts/file-size-allowlist.txt Reviewed — six stale schema.ts rows and three launcher-generator.ts rows consolidated into one accurate row each (prior N11). Guard verified green at HEAD and in tree mode
Tests (11 files: delivery, jsonl-framing, rpc-client, provider-map, deliver-agent-message, spawn-prime-agent, conversation-runtime-prime, infra, runtime-session-projection, liveness-no-loss-audit, prime-agent-no-loss-audit) Reviewed — fake timers used correctly for the grace period; the kill test now drives PrimeAgentRuntimeSync.killAgent (the production path) instead of the deleted registry; opencode added to the audit's ALL, which now matches KNOWN_HARNESSES exactly (8 literals); the two launch tests mock getOpenAIAuthStatus to loggedIn: false and pin providerAuth.openai: 'api-key', so the new gate takes the env-var branch and stays host-independent. Source of N4

Full-PR coverage

All 126 changed files are accounted for: the 27 above at this HEAD, and the remaining 99 in the prior cycle's file-by-file ledger (.pan/review/agent-pan-3668-review-37430f70/review.md), which I wrote and re-checked here for the paths the delta touches. Nothing in the delta invalidates those dispositions.

Four dimensions

  • Correctness — no blocking correctness defect remains. The kill sequence, framing recovery, RPC routing, credential resolution, and the two launch paths' agreement all verified. Advisories N2, N8.
  • Security / policy — the blocker is a policy-gate omission with a Terms of Service consequence, not a memory-safety or injection issue. The host's socket hygiene (0700 dirs, per-launch 32-byte 0600 token, 1 MiB body cap, concurrency 8, argument-array spawn, POSIX-correct shellQuote) is unchanged from the prior cycle's clean assessment. Nothing in the delta logs a secret: framing errors report byte counts, and the new credential errors name variable names, never values.
  • Performance — improved by the delta: the projection no longer puts readdirSync + a statSync-comparator sort on the dashboard event loop for non-Prime harnesses (measured last cycle at 0.46 ms / 9.1 ms per call). makeDbLive adds one sqlite_master lookup per layer build. No execSync on any server-reachable path. Advisory N2 is a latency-of-kill observation, not throughput.
  • Requirements / UX — one AC family fails (see matrix). runtime-adapter.ac3, message-delivery.ac3, rpc-framing.ac3, provider-map.ac2, dashboard-frontend.ac1, and no-loss-audit.ac3 all moved from unmet/partial to met this cycle. docs-setup-reference.ac3 is met as written but the requirement itself is what needs correcting.

AC-to-evidence matrix (changes since prior cycle only; all others unchanged and met)

AC Prior Now Evidence
runtime-adapter.ac3 unmet met prime-agent.ts:151-171; delivery.test.ts drives the production killAgent for abort-ok, abort-rejects, and host-gone
message-delivery.ac3 unmet met Same; the grace period is a named constant advanced with fake timers
rpc-framing.ac3 half-unmet met jsonl-framing.ts:40-95; tests cover malformed-then-later-records, split-chunk malformed, oversize completed and buffered, and resynchronise
provider-map.ac2 unmet met assertPrimeAgentCredentialAvailable names env var + settings key (api-key) and auth file + login command (subscription); called from the one shared builder
dashboard-frontend.ac1 partial met (badge) PrimeAgentHarnessIcon lettermark. Note: the picker offers the row unlocked in a cell policy should lock — that is the blocker, not this AC
no-loss-audit.ac3 hole met ALL now equals KNOWN_HARNESSES (8 literals, opencode restored)
docs-setup-reference.ac1 partial met Resume sentence now matches resumePrimeAgentSession's actual session-identity check
docs-setup-reference.ac3 met met, spec defect The required caveat is present and accurate about billing; the requirement never reconciled the ToS gate — see blocker fix (4)
verification-gauntlet unverified unverified at this HEAD docs/prime-agent-verification.md is dated 2026-08-12, nine commits back. Not reused as exact-HEAD evidence

Verification

Run at HEAD 93cd6317:

Check Result
npm run typecheck Passed (exit 0). Dashboard guard: 26 known errors, none new. Frontend guard: 0 known, none new.
npm run lint:effect-diagnostics Passed — 308 known findings, no NEW: diagnostics
Focused suites (20 files: src/lib/prime-agent/__tests__, tests/unit/lib/prime-agent, no-loss audits, runtime adapter, spawn, conversation-runtime, transcript, cost parser, doctor, context-layers, projection, deliver-agent-message, system-prerequisites, harness-binary, infra) Passed — 177/177 in 24.9 s
bash scripts/lint-file-size.sh and --at HEAD Passed both modes — the allowlist consolidation is safe
Harness-policy probe (isolated, gitignored .tmp/, removed; tree clean) Reproduced the blocker end to end — see the block above
GitHub CI at exactly 93cd6317 (gh pr checks 3670) All green: build (22), lint, test, guard, Clean install + server smoke test, flake lane, prompt-trailer gate. Head SHA confirmed via gh pr view 3670 --json headRefOid

Reused as exact-HEAD evidence rather than re-run: npm test, npm run build, npm run lint — CI's test, build (22), and lint jobs passed on this SHA, so the role's verification budget says not to repeat them locally.

Not run, and why:

  • tests/integration/prime-agent-smoke.slow.test.ts — opt-in slow lane (VITEST_INCLUDE_SLOW=1); it drives a real binary against a real provider and spends credits. Unverified at this HEAD.
  • A live Prime session — same reason, and for the blocker's cell specifically, launching it is the act under question.
  • macOS keychain reproduction for N1 — no macOS host available.

Scope Note

  1. Commit 325bb2333b7 is outside every xBRIEF files_scope. It changes src/lib/overdeck/infra.ts (makeDbLive now applies the schema) and de-hosts tests/unit/lib/agents/liveness-no-loss-audit.test.ts (clearing OVERDECK_NO_RESUME so the fixture comparison stops depending on how the host dashboard was booted). Both are sound, tested, and narrow — classic fix-forward for failures this work surfaced — but they belong to the substrate/liveness area, not Prime Agent. Flagging so the operator sees them rather than discovering them in a later archaeology pass.
  2. The blocker's policy half needs an operator decision, not an agent one. Whether Prime Agent may run Anthropic models under a Claude Code OAuth subscription is a Terms of Service reading. The repo already answers it one way for ohmypi; this PR answers it the other way for prime-agent by omission. An implementer should not resolve that silently in either direction — the mechanical fixes (explicit policy branch, picker decision key, typed decision record) are unambiguous and should land regardless.
  3. The no-loss audit does not cover harness-policy.ts or harness-policy-decisions.ts. no-loss-audit.ac2 enumerates the surfaces it checks — Harness union, behavior map, config literal, transcript registry, telemetry, artifacts, flywheel, context previews — and the policy gate is not among them, which is why nine commits and several review cycles missed this. Adding those two files to the audit's enumeration list would make the next harness's omission a test failure instead of a review finding.

Source: /home/eltmon/Projects/overdeck/workspaces/feature-pan-3668/.pan/review/agent-pan-3668-review-93cd6317/review.md

Required action

Fix every blocking review finding, commit the fixes, then re-request review with:

pan review request PAN-3668 -m "Fixed review issues"

@eltmon

eltmon commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

Closing this PR, not the issue. #3668 stays open.

Why: main has moved about 534 commits since the last clean rebase on 9/18. The branch now conflicts in 22 files (roughly 30 hunks; 5 of them are tests main deleted), and 65 call sites use …Sync names that #4057 (PAN-4002) renamed. All 7 Overdeck review rounds requested changes. A revive is about 1–2 agent-days of rework plus review, and it would need to land the same day it's rebased.

Where the work is: branch feature/pan-3668 keeps every commit. The last review round's uncommitted fixes are committed as 76a5f838bdc on the local branch in workspaces/feature-pan-3668. That covers:

  • the prime-agent block for Anthropic models on subscription auth,
  • provider-map routing cleanup,
  • a 3s kill-abort timeout,
  • the framer byte-count fix.

It isn't pushed: the pre-push file-size guard rejects the stale branch.

Worth porting regardless of Prime: type HarnessPolicyDecisionMap as Record<RuntimeName, …> so an unlisted harness can't slip past the ToS block. That's being handled separately.

@eltmon eltmon closed this Sep 24, 2026
@eltmon
eltmon deleted the feature/pan-3668 branch September 25, 2026 05:40
eltmon pushed a commit that referenced this pull request Sep 27, 2026
PAN-3668's merge was refused because merge-ops preferred the persisted
merge_set_repos.artifact_url (#3670, the first, closed PR) over the open
PR ensurePRExists resolved (#4251). Both the monorepo and remote paths now
take the fresh URL and its number via freshMergeArtifact (new
merge-artifact.ts, kept out of the capped merge-ops.ts), which rewrites a
differing stored row and logs the replacement.

The clean-direct and server-rebase tests' forge mocks now expose the real
parseArtifactRef, which the new module needs.

Item: merge-lands-fresh-pr

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
overdeck-agent Bot pushed a commit that referenced this pull request Sep 27, 2026
* chore(plan): complete planning for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(forge): gh/glab branch lookup throws on failure, null only on absence

The gh path of getExistingGitHubArtifact swallowed every gh error with
`2>/dev/null || true`, so a GraphQL rate limit read as "no PR" and review
verdicts were refused. It now returns null only for gh's "no pull requests
found for branch" answer or a non-OPEN PR, throws a timed-out message when
the exec timeout kills gh, and throws gh's stderr otherwise. glab mr list
gets the same split (empty list = absence). createReviewArtifact falls back
to the number in the created URL when the follow-up lookup fails or finds
nothing, so a created PR never loses its id.

Item: lookup-failure-not-absence

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(review): select the open PR for a head branch, not prs[0]

The GitHub REST pulls list sorts by creation date, so prs[0] could be a
newer closed PR while an older one is still open. selectPullRequestForHead
ranks open (most recently updated) first and, when asked, then merged and
closed. The forge App path now uses it open-only, so discoverArtifact never
returns a closed PR. listPullRequestsForHead maps updated_at to updatedAt.

Item: open-first-selector

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(review): move selectPullRequestForHead into github-pr-selection

github-app.ts is a capped god file; the selector added in the previous
commit pushed it to 1088 lines and the file-size guard refused the push.
The selector and its tests now live in src/lib/github-pr-selection.ts and
tests/unit/lib/github-pr-selection.test.ts (the PRD named github-app.ts).
github-app.ts keeps only the updatedAt mapping (+3 lines, allowlisted).

Item: open-first-selector

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(review): retry transient forge failures in discoverArtifact

New src/lib/forge-transient.ts: isTransientForgeError classifies rate
limits, network errors, timeouts and 502/503/504 (anchored to the
clients' "failed: 503" / "HTTP 502" formats so a /pull/504 URL never
matches); retryTransientForgeOp retries only those, after 2 s then 8 s.
Both adapters' discoverArtifact run through it. Tests use fake timers.

Item: discover-transient-retry

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(review): merge-gate branch lookup ranks open above merged above closed

lookupPullRequestForBranch took prs[0] on the App path and `--limit 1` on
the gh path; both lists sort by creation, so a newer closed PR beat an
older open one. Both paths now list the branch's PRs (gh: --limit 20 with
updatedAt) and pick with selectPullRequestForHead({ includeClosed: true }).

Item: gate-lookup-selector

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(review): merge lands the freshly resolved PR, not a stale stored URL

PAN-3668's merge was refused because merge-ops preferred the persisted
merge_set_repos.artifact_url (#3670, the first, closed PR) over the open
PR ensurePRExists resolved (#4251). Both the monorepo and remote paths now
take the fresh URL and its number via freshMergeArtifact (new
merge-artifact.ts, kept out of the capped merge-ops.ts), which rewrites a
differing stored row and logs the replacement.

The clean-direct and server-rebase tests' forge mocks now expose the real
parseArtifactRef, which the new module needs.

Item: merge-lands-fresh-pr

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(specialists): specialists done separates forge failure from absence

A failed artifact lookup now prints "Couldn't reach <forge> to find the
review artifact for <branch>: <reason>" and exits 1; only a true absence
prints "No open review artifact". For review verdicts the caller identity
is checked before any forge call, so a non-review agent is refused without
touching the forge. A review verdict that hits a transient forge failure
(lookup or post) is journaled as review.verdict-deferred with status,
runId, byte-capped notes, callerId (null for an operator) and reason,
through the new src/lib/cloister/deferred-verdict.ts. The journal type
union gains review.verdict-deferred and review.verdict-replay-gave-up.
The run-id fallback moved ahead of discovery so the deferral carries it.

Item: specialists-done-honest-error

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* fix(cloister): deacon-lite replays deferred review verdicts

New src/lib/cloister/deferred-verdict-replay.ts. When a
review.verdict-deferred entry is the journal tail, recoverStalledReviews
(after the issue-pause hold) replays it once the entry is >= 10 minutes
old, through `pan admin specialists done review` under the original
caller (OVERDECK_AGENT_ID = callerId; agent vars removed for an operator
verdict), so reviewVerdictRefusal runs again in full. The child's own
appended journal entries decide the outcome: review.verdict ends it, a
fresh deferral is the next cooldown, anything else journals
review.verdict-replay-gave-up {reason:'failed'}. A newer reviewRunId
gives up as 'superseded'; seven deferrals of one run give up as 'cap'
and warn in activity. An in-process set stops overlapping ticks.
`pan show` summarizes the two new entry types.

Deviation from D10: the outcome is read from entries appended during the
replay, not from a deferral count keyed on runId, so a child that
resolves a different runId (or records the verdict then fails later)
is not mis-read as a failure.

Implementation checkpoint: on this host the deacon child (pid 1868537)
-> dashboard server (1867603) -> user systemd (1917, environ unreadable,
walk stops) carry no OVERDECK_AGENT_ID, so readAncestorAgentIds() is []
in a replay child and the caller resolves from the env it is given.

Item: deferred-verdict-replay

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* docs(review): document forge retry, deferred verdicts and the merged PR

docs/PIPELINE-GATES.md: under "Verdict feedback routing", how
discoverArtifact retries transient failures (2 s, 8 s), how a failed
lookup is reported, and how a review verdict is deferred and replayed;
entry-type rows for review.verdict-deferred and
review.verdict-replay-gave-up; deacon-lite routine 5 names the replay;
the merge gate section says which PR a branch lookup picks and that the
merge lands the freshly resolved PR, overwriting a stale stored URL.

Also commits the planner's concerns.md entry on forge lookup failure vs
absence and stale stored artifact URLs, unchanged: its "Before PAN-4263"
wording already reads correctly now that the fix has landed.

Item: docs-pipeline-gates

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* test(review): merge leaves a stored row alone when it names the resolved PR

Covers merge-lands-fresh-pr.ac4: a stored artifact_url equal to the
resolved PR (trailing slash ignored) is not rewritten.

Item: merge-lands-fresh-pr

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(review): say a stale stored artifact_url is never merged

Matches docs-pipeline-gates.ac4 wording.

Item: docs-pipeline-gates

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): record PAN-4263 plan as running

The pipeline stamped the xBRIEF status proposed -> running at work start.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(review): acceptance criteria verified for PAN-4263

Each acceptance criterion is verified by its parent item's tests:
forge.test.ts (lookup-failure-not-absence), github-pr-selection.test.ts
and forge.test.ts (open-first-selector), forge-transient.test.ts
(discover-transient-retry), github-pr-lookup.test.ts (gate-lookup-selector),
merge-ops-stale-artifact.test.ts (merge-lands-fresh-pr),
specialists-done-forge-lookup.test.ts (specialists-done-honest-error),
deferred-verdict-replay.test.ts and deacon-lite-recover-stalled-reviews.test.ts
(deferred-verdict-replay), and docs/PIPELINE-GATES.md (docs-pipeline-gates).

Item: lookup-failure-not-absence.ac1
Item: lookup-failure-not-absence.ac2
Item: lookup-failure-not-absence.ac3
Item: lookup-failure-not-absence.ac4
Item: lookup-failure-not-absence.ac5
Item: lookup-failure-not-absence.ac6
Item: open-first-selector.ac1
Item: open-first-selector.ac2
Item: open-first-selector.ac3
Item: open-first-selector.ac4
Item: discover-transient-retry.ac1
Item: discover-transient-retry.ac2
Item: discover-transient-retry.ac3
Item: discover-transient-retry.ac4
Item: discover-transient-retry.ac5
Item: gate-lookup-selector.ac1
Item: gate-lookup-selector.ac2
Item: gate-lookup-selector.ac3
Item: merge-lands-fresh-pr.ac1
Item: merge-lands-fresh-pr.ac2
Item: merge-lands-fresh-pr.ac3
Item: merge-lands-fresh-pr.ac4
Item: specialists-done-honest-error.ac1
Item: specialists-done-honest-error.ac2
Item: specialists-done-honest-error.ac3
Item: specialists-done-honest-error.ac4
Item: specialists-done-honest-error.ac5
Item: deferred-verdict-replay.ac1
Item: deferred-verdict-replay.ac2
Item: deferred-verdict-replay.ac3
Item: deferred-verdict-replay.ac4
Item: deferred-verdict-replay.ac5
Item: docs-pipeline-gates.ac1
Item: docs-pipeline-gates.ac2
Item: docs-pipeline-gates.ac3
Item: docs-pipeline-gates.ac4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

* chore(workspace): plan artifacts for PAN-4263

---------

Co-authored-by: overdeck-agent[bot] <4205044+overdeck-agent[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant