Skip to content

feat(bin): add native Shuvcode supervisor entrypoint with Herdr presentation - #62

Open
shuv1337 wants to merge 3 commits into
mainfrom
native-herdr
Open

shuv1337 wants to merge 3 commits into
mainfrom
native-herdr

Conversation

@shuv1337

@shuv1337 shuv1337 commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Adds an explicit native ShuvBro entrypoint for new managed Shuvcode homes. Shuvcode owns durable work and execution; ShuvBro supplies orchestration policy and places exact, persistent Session views in Herdr.

Requires companion Shuvcode #440 and Herdr #16. The integration contract and joint evidence describe the three-repository boundary.

Behavior

  • fm-native.mjs initializes new homes with the tracked ShuvBro profile, runs the native supervisor, forwards typed work commands, and reconciles Herdr views. Existing homes are refused.
  • Lead, secondmate and worker views use exact home/Session/location identities and credential-free attachment arguments. Creation preserves parent placement and focus. The journal records display provenance and pending outcomes; Shuvcode retains workflow authority.
  • watch maintains status leases. Display close, restart, cleanup and reopen do not start or cancel execution. Cleanup requires positive settlement and closes only the recorded pane; reopen creates a new binding generation for the same Session.
  • Lost or ambiguous replies retain their pending state. Exact readback reconciles bindings and reports; uncertain creation or launch requires operator inspection. Presentation belongs to the destination host.

Validation

  • Required no-mistakes pipeline reached checks-passed at 089d2b7: 12 targeted public-CLI tests, an isolated CLI evaluation using a fake native service and fake Herdr socket, and fresh full CI passed. Coverage includes two workers, lost replies, authority conflicts, independent restarts, stale locks and guarded cleanup. These checks do not access live/default services.
  • Separate real three-repository lab: compiled Linux Shuvcode, actual Herdr PTYs, a local deterministic Responses provider, Git commits and verified result receipts. Verified exact bindings, no-focus placement, display close while work continues, restart without additional model requests, and status expiry.
  • Final adapter 089d2b7, SHA-256 e63cfe03b39a15f6382e146df952b50ee6c3a42306a0d72682161a2b3d3e0c05: cleanup and reopen work while watch stays active. The Session remains unchanged, the new binding receives later reports, neighboring panes from both generations and parent focus survive, and model requests remain 4→4.
  • Pipeline review fixed per-entry failure isolation, closure of absent entries, title validation before pending creation, watch lock handoff and stale-owner reclamation, and test teardown. Profile tests now check the emitted semantic contract without treating prompt-source text as behavioral proof.

Initial scope is new managed homes. Existing-home migration, real two-host networking, SSH disconnect behavior and production load remain unqualified. Production services and installed binaries were not changed.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ bin/fm-native.mjs:246 - sync reconciles entries one after another and stops at the first error, so one uncertain or conflicting entry blocks every entry after it. Example: the lead's workspace.create reply is lost, leaving entry-0 in create_pending. Every later sync, up or watch pass then throws at line 264 before it reaches the workers. Their views are never created, and existing ready entries are never re-reported, so their 5 s leases expire to unknown for good. The test at tests/fm-native-supervisor.test.mjs:128 (creates === 1) confirms that the two worker views are never created. A foreign binding on one entry (line 239) has the same effect. Fix: catch errors per entry inside the sync loop, keep reconciling and reporting the rest, then throw a combined error so the exit code stays non-zero.
  • ⚠️ bin/fm-native.mjs:256 - The second sync loop sends an unknown report for journal entries that have left the projection but are still ready, and it does not catch pane_not_found/not_found the way the ready branch of reconcile does (line 315). Example: a native entry drops out of the projection, then its pane is closed. From then on every sync/up exits 1 and watch logs an error every 2 s. Cleanup is also impossible, because fact is undefined and fails the settlement check. Fix: apply the same not_found → closed transition here.
  • ⚠️ bin/fm-native.mjs:268 - The entry is saved as create_pending before the label is computed with fact.title.slice(0, 100), and presentation() never checks that title is a string. If a ship or scout fact has no title, this throws a TypeError before workspace.create is ever sent. The entry is then stuck in create_pending ('Uncertain creation'), even though nothing was created, and there is no CLI way to recover it. Fix: validate title in presentation() and compute the label before saving create_pending.
  • ⚠️ bin/fm-native.mjs:129 - watch holds the home lock for its entire life. The docs say to keep watch running and also to use explicit cleanup and reopen, but both fail with 'Native display adapter is already running' while watch holds the lock. To clean up, the operator has to stop watch, which lets every lease expire to unknown. Fix: take the lock for each reconcile pass and release it between passes, or document that watch must be stopped first.
  • ℹ️ bin/fm-native.mjs:264 - Nothing in the CLI resolves an entry stuck in create_pending or launch_pending. cleanup and reopen both refuse it, and the docs only say to 'inspect the journal and Herdr'. In practice that means editing native-display.json by hand. The intent ('remains visibly pending without blind duplicate allocation') is met, so the open question is only whether an explicit resolve or abandon operation is wanted. Adding one would extend the scope.
  • ⚠️ bin/fm-native.mjs:231 - The --host-id option and the attachment() rewrite replace the native attachment's host_id: "local" with hostname() or a user-chosen string before binding. The intent requires binding the 'exact existing native Session/home/host/location identity'. A host field that differs from the native value, plus an option to override it, goes beyond what the intent requires. Remedy: remove the --host-id option and bind the native attachment unchanged, unless Herdr explicitly needs a concrete host name.
  • ℹ️ bin/fm-native.mjs:226 - Attach-argv validation accepts three prefix shapes: a bare executable, interpreter plus script, and --no-env-file --preload X script. Nothing in the intent requires the five-item preload/dev-runner form. If the managed Shuvcode only emits one shape, accept just that shape.

🔧 Fix applied.
3 issues (1 warning, 2 infos) still open:

  • ⚠️ bin/fm-native.mjs:169 - The fix round changed watch to take the lock on every pass and made waiters retry every 100 ms, so lock handoff now happens constantly. Two parts of the new lock code are not atomic. (1) Release calls rm(directory, {recursive:true}), which deletes pid first and then the directory. On Linux, rename() of a directory onto an existing empty directory succeeds. So a waiter (for example cleanup polling while watch releases) can rename its staging dir into place in that gap. The releaser's rmdir then hits ENOTEMPTY. Depending on Node's rm implementation, it either deletes the waiter's fresh lock (the waiter now runs with no lock held) or throws from finally, failing a pass that otherwise succeeded. (2) Stale reclaim (line 176) does check-then-remove. Stopping watch with Ctrl-C during a pass leaves a dead-pid lock. If a restarted watch and a cleanup both try to recover it, B's rm(force) can remove A's just-acquired lock, and both run passes at once. Either way two processes hold the lock together, and a stale watch journal can overwrite cleanup/reopen results, which is the overwrite F4 was meant to prevent. Fix: make release atomic by renaming the lock dir to a unique tombstone and then removing it, or use an O_EXCL lock file (writeFile(..., {flag:'wx'})) released with unlink. Reclaim a stale lock by renaming it to a unique tombstone and re-checking ownership, not by removing it in place.
  • ℹ️ tests/fm-native-supervisor.test.mjs:77 - start() registers its kill hook with t.after after the fixture's hook. node:test runs after-hooks in registration order (FIFO, checked locally). So in the watch+cleanup test, the fixture closes the fake Herdr server and runs rm(directory, {recursive:true}) while the watch child is still running. That child can still be appending native-calls.jsonl, writing journal temp files, or creating lock staging dirs in that directory, so cleanup can race and the test can flake. Fix: kill the watch child and await closed inside the test body before it returns, or make the fixture's teardown run last.
  • ℹ️ bin/fm-native.mjs:235 - As the user directed for F3, title validation now happens at the projection boundary, so a single worker without a title fails the whole presentation(). The lead and the other workers are then neither created nor reported, and the new test asserts creates === 0. This is consistent with how other contract fields are already validated, but it is a deliberate exception to the F1 rule of independent per-entry reconciliation. Noted only so the tradeoff is visible.

🔧 Fix applied.
1 info still open:

  • ℹ️ bin/fm-native.mjs:175 - If a process dies after claim(guard) but before unlink(guard) (a window of a few microseconds during stale reclaim), .native-display-lock.reclaim is left owned by a dead pid. From then on, every sync, cleanup and reopen call, and every watch pass, fails with 'Ambiguous native display lock recovery' until someone deletes it by hand. docs/native-supervisor.md does not name this file. This fails closed, which matches the intent's 'retain for inspection' semantics. Noted only so the manual recovery step is known.
✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 10 of 13 scenarios driven live against the product
Scenario Result Live Evidence
Operator inits a NEW native home and runs up; a lead and two workers get views without focus change, placed from @parent and bound by exact readback, with the neighbor pane untouched ✅ pass live transcript-main.txt: workspace.create focus=false source=@parent, bind_runtime→get_runtime readback, creates/launches 3/3, neighbor pane untouched; Herdr and Shuvcode were hermetic fakes
Typed control passes work operations to native Shuvcode and refuses attach/init through control ✅ pass live transcript-main.txt: control -- task steer forwarded with --home; control -- attach exit 1
Repeated sync is idempotent with an increasing status sequence and no duplicate creates or attach launches ✅ pass live transcript-main.txt: seq 1→2, creates/launches stay 3/3
Authority conflict: a foreign binding on a worker pane fails closed, other entries still report, and cleanup refuses even when settled ✅ pass live transcript-main.txt: 'entry-1: Recorded presentation runtime binding changed', lead/worker B reported seq=3, pane still present
Cleanup is refused for state=done without settlement; a lost pane.close reply stays close_pending and a retry confirms by readback, closing only the owned pane with no workspace.close or cancellation ✅ pass live transcript-main.txt: done-only exit 1; close_pending then exit 0; remaining panes are the neighbor plus the other two views
The user closes a worker view while it keeps working: the entry becomes closed, is not recreated, and only presentation is queried from native ✅ pass live transcript-main.txt: entry-2 closed, native calls during sync ['presentation'], creates stay 3
Explicit reopen creates a fresh binding generation; reopening a retired entry is refused ✅ pass live transcript-main.txt: entry-2 new pane @native-4 and new bindingID; retired entry-1 reopen exit 1
A lost workspace.create reply leaves the lead visibly create_pending without a duplicate, while both workers are created and keep reporting ✅ pass live transcript-lost.txt: entry-0 create_pending on both passes, creates stay 3, workers seq 1→2
The watch adapter stopped and restarted independently resumes from the journal with no duplicate views and keeps reporting new state ✅ pass live transcript-restart.txt: same bindings and panes, creates/launches 3/3, seq 2→4, worker state now blocked; status runs while watch runs
Fail-closed guards: default Herdr session, credential argv, and old Herdr without runtime methods (refused before native init); existing or legacy homes are never taken over ✅ pass live transcript-guards.txt: every attempt exits 1, native CLI calls [] for old Herdr, legacy home contents unchanged
Lock handoff and stale-lock reclaim under concurrent passes, plus watch running alongside cleanup ⏸️ untested no The prior payload did not establish a live result: this scenario was covered only by unit test cases in fm-test-run-native-supervisor.log, not driven against the running CLI. Driving it live would nee…
Behaviour against real Herdr runtime-attachment methods and the real Shuvcode supervisor, including Herdr-side lease expiry to unknown ⏸️ untested no The runbook excludes the real-herdr-gated and live-harness-optin families and forbids driving host Herdr/Shuvcode; the Herdr companion changes are out of scope. The root agent validates this in a priv…
Remote, destination-owned presentation across two hosts ⏸️ untested no Needs a second host with its own Herdr/Shuvcode; the intent forbids claiming two-host behavior from local fixtures.
  • bin/fm-test-run.sh --exclude-family live-harness-optin --exclude-family real-herdr-gated tests/fm-native-supervisor.test.sh (12/12 node:test cases pass)
  • bin/fm-test-run.sh --check-coverage (new test script is covered as unclassified; guard ok)
  • node drive-native-cli.mjs bin/fm-native.mjs main: --help, init, up, control task steer, control attach refusal, idempotent sync, foreign-binding conflict, done-but-unsettled cleanup refusal, lost pane.close then retried cleanup, user-closed view during work, explicit reopen, retired reopen refusal
  • node drive-native-cli.mjs bin/fm-native.mjs guards: default session, credential argv, old Herdr without runtime methods, re-init and legacy home refusal, sync on non-native home
  • node drive-native-cli.mjs bin/fm-native.mjs lost: lost workspace.create reply for lead while two workers proceed
  • node drive-native-cli.mjs bin/fm-native.mjs restart: watch started, stopped and restarted independently; status while watch runs
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

shuv1337 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

🔍 Automated Cursor review is underway. Please hold off on merging until the review comment lands here (usually 10–20 minutes).

@cursor

cursor Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review complete:

High

The non-default Herdr session guard does not constrain which server is used. bin/fm-native.mjs:49 rejects the session names default and main, and :53 stores herdr.session, but that field is never sent. herdr() at :219 and :224 dials only journal.herdr.socket and writes {id, method, params} with no session. negotiate() at :243-245 checks attachment methods and not the server's session name. Named and default Herdr sessions use different sockets (bin/backends/herdr.sh:3467-3470: the default session is ~/.config/herdr/herdr.sock). Impact: --herdr-session native-drive together with the default socket still creates, binds, and closes panes on the live default session. The PR's "default session refused" case only passes the name default, which returns before any connection. Fix: require the peer to report its session (ping or an equivalent) and refuse unless it equals the configured non-default name; also refuse the well-known default socket path.

Medium

Cleanup is an emptying close with no focus protection, and a create result can adopt the parent pane. Each entry is a new workspace (bin/fm-native.mjs:309). Cleanup then calls pane.close on that root pane (:124-125) and sets phase to closed on any success, with no pane.get readback. bin/backends/herdr.sh:1042-1047 documents that an explicit close of a workspace's last pane removes the workspace and, before Herdr 0.8.0, moves focus to a neighbor. The existing adapter snapshots focus, refuses the active tab (bin/backends/herdr.sh:983-986), and restores focus. This path does none of that. :310-311 also accepts any create result whose pane, tab, and workspace ids merely agree with each other, including the parent workspace. Impact: cleanup can steal focus or, if create echoes an existing pane, close a pane this adapter does not own. A close ack that leaves the pane in place is forgotten, so a later reopen allocates a second view. Fix: reject a create whose workspace id is the parent or whose pane already exists; snapshot the focused tab, refuse the active tab, confirm pane_not_found before marking closed, and restore focus.

A lost close cannot be finished once the native entry leaves the projection. close_pending with the pane still present falls through to bin/fm-native.mjs:120, which requires fact.settled. sync then skips that entry at :291 because the phase is not ready. Impact: if the close reply is lost and the settled entry drops out of the projection while the pane remains, later cleanup keeps failing and watch exits 0, so the pane stays up with no recorded owner. The not-found path at :110-117 does not cover a pane that is still there. Fix: a close_pending retry should confirm the pane is gone or re-issue pane.close for the recorded pane without requiring the projection entry to still be present.

Binding host identity is rewritten, and --host-id can stamp an arbitrary host. :253 requires the native attachment host_id to be local, then :264 replaces it with journal.hostID (hostname() or --host-id from :53) before pane.bind_runtime. Impact: the Herdr binding is not the exact native host identity the intent asks for, and a caller-supplied --host-id publishes some other machine's identity. Fix: bind the native attachment unchanged, or if Herdr needs a concrete host, substitute only this machine's hostname and drop --host-id.

A lost bind is retried instead of adopted by readback. :315-320 always calls pane.bind_runtime again while bind_pending. owned() runs only after that call returns. docs/native-supervisor.md:36 says lost binding replies reconcile through exact readback. Impact: if the first bind was applied and the reply was lost, the next pass issues a second bind. A non-idempotent server can conflict or reset the lease and leave the entry stuck in bind_pending while the pane is already bound. Fix: call pane.get_runtime first, and advance to bound when the binding id and attachment already match.

One invalid projection entry blocks heartbeats for the whole home. presentation() (bin/fm-native.mjs:248-260) throws before the per-entry loop in sync. docs/native-supervisor.md:19 says one entry's failure does not stop the others from being created or reported. Impact: a single worker with a missing title, bad argv, or nonlocal host drops reports for the lead and every other worker, so their 5s leases expire to unknown. The per-entry catch at :288 never runs. Fix: validate and isolate entries inside the loop; keep reporting entries that still match the contract.

A successful attach is marked ready without the argv check the lost-reply path requires. :334-335 sets ready as soon as pane.send_input returns. The argv readback exists only for launch_pending recovery (:339-344) and for an already-running process (:325-328). docs/native-supervisor.md:37-38 say a launch is adopted only when the exact foreground argv is observable, and that input is sent only into an idle shell. Impact: if the shell accepts the input but does not exec that argv, later passes only report status and never retry, so the view is not the attach TUI. Fix: leave the entry in launch_pending until process_info shows the exact argv, including after a successful send_input.

Low

A crashed stale-lock reclaim bricks the home. If the process dies after claim(guard) and before unlink(guard) (bin/fm-native.mjs:170-172), :175 throws Ambiguous native display lock recovery on every later sync, cleanup, reopen, and watch pass until .native-display-lock.reclaim is deleted by hand. docs/native-supervisor.md does not name that file. This fails closed, which matches the lost-reply rule, but there is no documented recovery. Fix: name the file and the delete step in the doc, or reclaim a dead guard the same way as a dead lock (content check, then unlink) instead of failing permanently.

Tests

bin/fm-test-run.sh --exclude-family live-harness-optin --exclude-family real-herdr-gated tests/fm-native-supervisor.test.sh — 12 passed, 0 failed. FM_LIVE was not set. The fake socket never checks session identity, always returns a fresh workspace id, and does not move focus on pane.close, so the cases above are untested. I did not run live-harness-optin or real-herdr-gated.

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