feat(bin): add omp-as-primary harness support and land archive-span captain-hold fix - #2971
feat(bin): add omp-as-primary harness support and land archive-span captain-hold fix#2971erwin-wee wants to merge 11 commits into
Conversation
omp (Oh My Pi, @oh-my-pi/pi-coding-agent) sets both OMPCODE=1 and CLAUDECODE=1 on every bash-tool child process, so checking CLAUDECODE first silently misidentified omp sessions as claude. Test OMPCODE first, same precedence-fix pattern already used for the Cursor marker collision in this file, and add ancestry-based detection as a second layer. Also record the empirically verified (but partial) Oh My Pi adapter facts in harness-adapters so dispatch continues to fail closed until a launch template and busy-state contract land in a follow-up. (cherry picked from commit 12e8fad585c58e8d3b063f05e5779c76a203c93e)
…-dispatch.json Verified live 2026-08-24 on omp v18.0.4: interrupt (single Escape), exit (/exit), and a full real crew dispatch smoke test (spawn, run, report, exit, teardown) on the openai-codex/gpt-5.6-luna model. - bin/fm-spawn.sh: omp launch template (--approval-mode yolo, --model, --thinking), model/effort flag wiring, and an explicit --secondmate refusal on omp (no primary supervision protocol yet, same as muse). - bin/fm-control-lib.sh: omp control-plane mechanics (interrupt key, exit command, kind restriction) mirroring the verified facts. - .agents/skills/harness-adapters/SKILL.md: omp moved from partially-verified to VERIFIED CREWMATE/SCOUT, with the live evidence and the model-role-mapping rationale. - AGENTS.md: omp added to the verified-harness list, crewmate/scout only. config/crew-harness and config/crew-dispatch.json (local, gitignored) now route crew dispatch to omp, with each dispatch rule's model selected from omp's own configured modelRoles (~/.omp/agent/config.yml) per captain instruction, superseding the 2026-08-21 claude-only restriction for omp dispatch. (cherry picked from commit 16cc445d95fa677ee6a6dd5b647fe51eedd6e60e)
omp had no working busy-state hook and no composer mapping, so Herdr's agent panel and fm-crew-state.sh could never show a real state for an omp crewmate/scout once idle. Both gaps are closed: - bin/fm-spawn.sh: omp's launch template now loads a per-task state/<id>.omp-ext.ts extension via an explicit -e path, mirroring the pi-ext pattern. omp's extension API is Pi-compatible but its settle event is agent_end (not agent_settled) and carries its own willContinue flag; the extension goes idle only when willContinue is falsy AND ctx.isIdle() also confirms it, the same double-check Pi's extension uses. Verified live end-to-end against the real omp v18.0.4 binary: busy mid-turn, idle only at genuine settle, turn_end staying a notification-only touch. - bin/fm-busy-lib.sh: fm_busy_sources_for_harness now trusts the omp-ext source for harness=omp. - bin/fm-composer-lib.sh: taught the shared screen classifier omp's real composer shape (composer.shape=box, live-verified): a top row that permanently embeds a status/title line and a bottom row that is simultaneously the closing rule and the last content row. This needed a new "capped" shape alongside the existing bordered/bare/left-bar/ separated catalogue, since the generic bordered shape's content_rows>0 gate always misses the common zero-interior-row case. Also registered omp's busy-indicator delivery regex (a stable trailing "esc" cancel-hint token; the spinner's working-message text is dynamic and never stable enough to match). - bin/fm-teardown.sh: clean up the per-task omp-ext.ts file alongside the other per-adapter artifacts. - tests/fm-busy-adapter-wiring.test.sh: drive the real fm-spawn- generated omp extension through a plain Node host and the real fm-busy-event.sh writer (agent_start, willContinue, ctx.isIdle(), turn_end, ordering, stale-incarnation rejection). - tests/fm-composer-lib.test.sh: matrix coverage for the new capped shape (idle, typed, wrapped, extraction fidelity). - harness-adapters skill: omp's Busy state row and the "remaining work" note updated to match the verified reality, with the exact verification boundary disclosed (a full nested fm-spawn dispatch from inside this task's own isolated worktree was not run, to avoid mutating the shared firstmate home; the live binary round-trip and the automated real-extension-file tests are what was actually done). Full existing composer-lib, composer-ghost, busy-state, busy-adapter-wiring, backend-herdr, tmux-submit-busy, and spawn-dispatch-profile suites re-run with zero regressions. (cherry picked from commit bed42b8fe5b00260bfcde2b30141e92804670ffe)
Add a line to both the ship and scout scaffold's status-reporting rules preferring direct, unpiped tool invocations over piping through tail/head: the environment's automatic output-compaction hook only rewrites simple invocations, so a manual pipe defeats it. (cherry picked from commit 24e85643c7f7cc9c1f003f33ba1fc1fb95a7dff0)
(cherry picked from commit 7e01c77fd97cf94ee295deb3a5745801c3c3546f)
verify and complete's per-entry check (verify_hold_durable, via resolve_entry) read the active backlog only. Once tasks-axi retention (prune, or done --keep) swept an answered, closed captain-held task into data/done-archive.md, both refused it as absent - even though the archive still carried the captain's recorded answer. Live-reproduced against this exact fm-captain-hold.sh (post-collapse PR kunchenguid#2728) with a throwaway fixture. task_show_durable/task_show_archived/absent_task_message now read the archive through bin/fm-tasks-axi-lib.sh's fm_tasks_axi_archive_show (already present on this branch), keeping a genuine miss (both files searched) distinct from an unreadable archive (absence never established). Wired into verify_hold_durable, resolve_entry, and origin_exists_here - the read-only lookups; every mutation path still resolves through task_show alone, since tasks-axi cannot rewrite an archived row. Adds a regression test that forces retention to archive an answered captain-held task before calling complete/verify; confirmed it reproduces the bug against the pre-fix script and passes against the fix. Local-only landing, matching this home's established pattern for firstmate- repo changes: applied and validated locally, not pushed/PR'd upstream. (cherry picked from commit 4d5edc0b9d68d60711f8f722139d83ee945af353)
Prevents the serena MCP tool's local .serena/ scratch directory from being accidentally committed, the same hygiene local main converged on.
Re-derives the closed-unmerged slimming pass (PR kunchenguid#2676) against current AGENTS.md rather than replaying its stale patch, cutting the two largest inline one-owner violations to pointers per firstmate-coding-guidelines' knowledge-placement tree: - Section 2 collapses the exhaustive per-file config/ and state/ inventory into a short tree plus two summary paragraphs. Section 2 already names docs/configuration.md as the single owner of the home layout and config schemas, and its "Operational home layout and state" section covers every file (including the steering inbox, captain inbox, pending replies, and Relay artifacts origin added since the closed PR). The safety facts stay inline: state internals are never hand-edited and supervision is repaired only through section 8's home-scoped owner path. - Section 4 collapses the inline TOON/spendPriority/eligibility/granularity procedure into the quota-array-dispatch skill it already names as the single owner, keeping the candidate-transparency contract inline. Scoped deliberately to these two unambiguous cuts; the closed PR's other micro-edits target section 3 and section 7 prose origin has since evolved (durable inboxes, record-divergence), where re-cutting would risk reverting origin's work rather than removing duplication.
…e tasks-axi-lib's new archive-read role
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "fix(omp): clear restoring flag before as..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (confirmed.ok) { | ||
| recovery = successorRecovery; | ||
| failure = ""; | ||
| break; |
There was a problem hiding this comment.
When the replacement watcher passes handling confirmation and then exits before predecessor wake delivery finishes, its close handler sees owner.restoring and returns without scheduling another arm. This leaves OMP watcher supervision down until a later turn-end guard or manual repair detects the missing cycle.
owner.restoring stayed true through the await sendWake(...) call after a restore attempt concluded (success or exhausted retries), so if the confirmed successor watcher exited during that window its own close handler saw restoring still true and silently dropped the failure, leaving watcher supervision down with no scheduled retry. Clear the flag as soon as the retry loop's outcome is decided, before the async notification, so a close event racing with wake delivery is handled normally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Speaking as Kun's firstmate: Scheduled 11:10am PT 8/24 pass. VISION.md read in full from current main VISION (inspected
Class for the whole PR: default-behavior. Mentally split: archive-span is corrective; omp-as-primary is product. Land-eligible only if the whole PR is corrective or opt-in — it is not. Do not auto-merge a mixed default-behavior PR. Security: none. No workflow-file / secret / injection. OMP live e2e is itself opt-in ( Overlap / HOLD: CI / NM: HEAD Workflows approved this pass: CI 32758007142, Require no-mistakes 32758007146. Not green at comment time. Land-eligible rec: NO (whole PR is mixed default-behavior; spawn/teardown/lock hold overlap; NM mismatch). Captain-flag NOW: no. Captain-decision for omp-as-primary product. Archive-span could land on its own corrective PR; this mixed PR will not. Also waiting-on-author for a matching NM attestation if the PR is split later. |
|
Speaking as Kun's firstmate: first look. class=default-behavior. Promoting omp as a primary harness plus spawn/teardown/lock wiring is a default fleet surface, even if the captain-hold archive-span slice of #2920 is corrective. The PR is not split. Never auto-eligible. VISION.md: captain-hold archive-span aligns as corrective honesty. New primary harness does not align as auto (authority / vendor / spine). This HEAD: Waiting on author for HEAD-matching attestation. Do not flag the captain until this is otherwise ready except the harness-as-primary decision. |
2 participants
Intent
Reconcile this firstmate home's local main, whose history had genuinely diverged from origin/main since merge-base 1cb900c, so /updatefirstmate no longer reports 'skipped: diverged', while preserving every real feature from both lineages (nothing silently dropped) and keeping the archive-retention captain-hold fix intact.
The branch is rebuilt as a clean linear descendant of origin/main (origin/main + 8 commits): it brings only the genuinely-unique local work and drops redundant re-derivations origin already carries in evolved form. Investigation established that origin independently re-derived, since the fork, the collapse-of-decisions-into-captain-holds feature (#2728), the record-divergence guard (#2744), and the bearings close controls (#2707) - origin's bin/fm-captain-hold.sh is byte-identical to local's pre-archive-fix version - so local's d1ebe75/f228bd84/b6d28cd were redundant reimplementations and were intentionally dropped (their features survive via origin's 99b21d8/d3342dc/b96dba1). The 9-commit archive-retention decision-hold branch (PR #2672, closed unmerged upstream) was likewise dropped as superseded.
Kept genuinely-unique local work, resolving every conflict to preserve origin's evolution: omp-as-primary support (harness detection ordered ahead of Claude's marker; omp crewmate/scout dispatch; per-task busy-state hook and composer shape; omp primary supervision with new .omp/extensions watch and turn-end guard extensions); the archive-span captain-hold fix (4d5edc0), which also required bringing the fm-tasks-axi-lib.sh archive-read helper definitions it depends on (originally added by the dropped decision-hold branch, so 4d5edc0 alone was an incomplete re-land); a .serena/ gitignore entry; and a re-derived AGENTS.md dedup cutting the section-2 config/state inventory to its docs/configuration.md owner and the section-4 quota procedure to the quota-array-dispatch skill.
Latent bugs fixed while landing omp on origin's base: dropping the redundant collapse restored an omp mention it had reverted from AGENTS.md's verified-harness list; the pi-primary guard tests leaked the host's ambient OMPCODE (this home runs under omp) and were hardened to pin the pi extension family (FM_NATIVE_EXTENSION_FAMILY=pi and -u OMPCODE); a duplicate omp) case in fm-harness.sh's ancestry walk was removed; and the new docs/supervision-protocols/omp.md was classified in documentation-audiences.json.
The AGENTS.md slimming was scoped deliberately to those two unambiguous one-owner cuts rather than mechanically replaying the closed PR #2676 patch, because origin has since evolved the other sections that patch targeted (durable inboxes, record-divergence).
What Changed
.omp/extensions/fm-primary-omp-watch.ts/fm-primary-turnend-guard.tsextensions with accompanying supervision docs and tests.bin/fm-captain-hold.sh), bringing along thefm-tasks-axi-lib.sharchive-read helper definitions it depends on, and fixed related latent bugs: restored the omp mention in AGENTS.md's verified-harness list, hardened the pi-primary guard tests against ambientOMPCODE/host harness leakage, removed a duplicateomp)case infm-harness.sh's ancestry walk, and classified the new omp supervision doc indocs/documentation-audiences.json.docs/configuration.mdowner and the quota procedure section into the quota-array-dispatch skill; added a.serena/gitignore entry.Risk Assessment
✅ Low: All areas of the diff were independently and thoroughly verified — harness detection, omp-primary extensions cross-checked against the real SDK types, composer/spawn harness dispatch, the archive-span captain-hold fix empirically validated against the real tasks-axi binary, and the AGENTS.md/docs slimming checked against its stated two-cut scope and the omp verified-harness restoration — with no correctness bugs, silent-fallback issues, or intent contradictions found anywhere.
Testing
Ran the full set of tests changed or added by this branch (guard-stale-banner, busy-adapter-wiring, captain-hold-lifecycle, composer-lib, omp-primary-extensions, session-start, supervision-instructions) plus manual CLI transcripts of bin/fm-harness.sh under forced CLAUDECODE/OMPCODE env combinations; all assertions pass, the OMPCODE-hardened guard test stays green even with ambient OMPCODE forced on, the archived-answer captain-hold regression test passes, and the omp-vs-claude harness precedence and de-duplicated ancestry-walk case behave exactly as the stated intent describes. One suite (fm-captain-hold-lifecycle.test.sh) hangs after its later, unrelated cases; I reproduced the identical hang against the pre-change base commit in a scratch worktree, confirming it is a pre-existing environment issue unrelated to this branch's changes, so no fix was needed and it is not reported as a finding.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-guard-stale-banner.test.sh (all 27 cases pass, including the pi-primary routing case that pins FM_NATIVE_EXTENSION_FAMILY=pi/-u OMPCODE)OMPCODE=1 bash tests/fm-guard-stale-banner.test.sh (re-run with ambient OMPCODE forced on, confirming the hardening fix keeps the suite green under an omp host — the scenario the fix was written for)bash tests/fm-busy-adapter-wiring.test.shtimeout 60 bash tests/fm-captain-hold-lifecycle.test.sh (7 cases pass including 'the completion gate reads an archived recorded answer instead of reporting it absent', the archive-span regression test; suite hangs afterward on an unrelated later case)bash tests/fm-captain-hold-lifecycle.test.sh against base commit 038d0f7 in a scratch worktree (same hang reproduces further along, proving it predates this branch and is not a regression it introduced)bash tests/fm-composer-lib.test.shbash tests/fm-omp-primary-extensions.test.shbash tests/fm-session-start.test.shbash tests/fm-supervision-instructions.test.shbash tests/fm-omp-primary-live-e2e.test.sh (correctly skips: opt-in live e2e gated on FM_OMP_PRIMARY_LIVE_E2E=1)env -u CLAUDECODE OMPCODE=1 bash bin/fm-harness.sh -> omp; env -u OMPCODE CLAUDECODE=1 bash bin/fm-harness.sh -> claude; env -u CLAUDECODE OMPCODE=1 CLAUDECODE=1 bash bin/fm-harness.sh -> omp (manual CLI transcript demonstrating omp is detected ahead of Claude's marker as the intent requires)grep -c 'omp)' bin/fm-harness.sh (confirms only one omp) case remains in the ancestry walk, i.e. the duplicate-case bug was actually removed)grep for 'omp' in AGENTS.md's verified-harness line (confirms the reverted omp mention was restored) and for docs/configuration.md / quota-array-dispatch references (confirm both dedup-target owners exist and are named correctly)grep '.serena/' .gitignore (confirms the gitignore entry landed)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.