fix(bin): identify herdr lab viewers by kernel start time so stop works on WSL2 - #11
Merged
Merged
Conversation
…ask left behind fm-herdr-lab.sh viewer stop compared a recorded `ps -o lstart` string with a fresh reading. On WSL2 ps derives lstart from a wall-clock boot time that re-renders one to two seconds apart for the same live process, so the helper disowned its own viewer and teardown refused while it stayed attached. The viewer record now stores a kernel identity: /proc/<pid>/stat starttime in clock ticks, qualified by the boot id, with the locale-pinned lstart kept as the exact-match fallback on hosts without that /proc. The Python launcher takes its identity values from the same helper function, so one function owns the format. A lab server outlives the shell that provisioned it, so a lab whose owner died before its EXIT trap ran stayed registered after its task ended. Provision now records the owning task id and working directory when FM_TASK_ID is set, and the new `reap-task` command tears down exactly those labs through the guarded teardown path. fm-teardown.sh runs it before its worktree process reap, which would otherwise kill only a lab server started from the worktree and leave the session registered.
dardant
added a commit
that referenced
this pull request
Sep 25, 2026
Resolve the Herdr lab viewer overlap in favor of main's kernel start-time identity (#11), which supersedes this branch's own WSL2 viewer fix; every other part of this branch is kept.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
bin/fm-herdr-lab.sh viewer stop refuses to stop its own lab viewer on WSL2 because the recorded
ps -o lstartstart time and a later reading differ by one to two seconds for the same process, so lab teardown refuses while attached.Context: the captain approved starting the queued Firstmate fixes, each as a PR for his merge call. This hit three workers on 2026-09-24/25 (fm-afk-inject-wedge, fm-done-worker-stale-noise twice): each had to verify the pid, parent chain and
--session <lab>cmdline by hand and kill its own viewer before the guarded stop and teardown succeeded. A leftover lab session from an earlier task was also seen running, which suggests some teardown path may leak labs.What Changed
bin/fm-herdr-lab.shreplacesfm_herdr_lab_process_start(theps -o lstartvalue) withfm_herdr_lab_process_identity. When/procis readable, the identity is/proc/<pid>/statfield 22 (kernel starttime), prefixed with the kernel boot id if one can be read. Hosts without that/procstill fall back to the locale-pinnedlstart.viewer stopcompares the recorded identity against the current one, so a live viewer whoselstartdrifts by a second or two (as it does on WSL2) is no longer disowned.bin/fm-herdr-lab-viewer.pygets its values from that same shell function and writeslauncher_identity/viewer_identityin place oflauncher_start/viewer_startin the viewer pidfile, so the recorder and the stop guard can't disagree on the format.tests/fm-herdr-lab.test.shadds two tests. One checks thatviewer stopstill stops the viewer whenlstartdrifts. The other checks that the guard still refuses a reused PID whose kernel start time doesn't match the record.Risk Assessment
✅ Low: The change now only replaces the viewer record's lstart identity with /proc stat starttime plus boot_id (falling back to exact lstart when /proc is unavailable), which addresses the WSL2 drift the intent describes while keeping PID-reuse protection. The fix round fully removed the lab-leak backstop the user asked to drop, and the new tests exercise the public stop path: under a simulated lstart drift the viewer is still signalled, and with reused PIDs (mismatched starttime, boot id or lstart) nothing is signalled.
Testing
I drove four throwaway fm-lab-* Herdr sessions through bin/fm-herdr-lab.sh on this WSL2 host, each with a real attached pty viewer. On HEAD, the viewer record carriesboot=<id> starttime=<ticks>identities that match /proc, and viewer stop plus teardown succeed both normally and with a 1-second lstart drift injected through apsshim. Under the same drift, the pre-fix base code leaves the viewer alive and refuses teardown, which reproduces the reported bug. A forged starttime in the record still leaves the viewer unsignalled and teardown refused. Natural drift did not occur during a 15-second probe here, so the drift was simulated rather than produced by the host clock. The focused fm-herdr-lab suite and the real-Herdr attached-viewer e2e both pass. Every lab I created was torn down, and the default session and other agents' labs were never touched. This change has no visual UI surface, so the CLI transcript is the evidence.Evidence: Live lab transcript: HEAD happy path, HEAD under drift, base under drift (bug repro), forged-identity refusal
Source: Live lab transcript: HEAD happy path, HEAD under drift, base under drift (bug repro), forged-identity refusal
Evidence: WSL2 host probe of btime, lstart and /proc starttime
Source: WSL2 host probe of btime, lstart and /proc starttime
Evidence: Base vs HEAD under identical 1s lstart drift
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-herdr-lab.sh:601- This change adds more than the required fix. It adds a per-lab.ownerfile written from FM_TASK_ID during provision (bin/fm-herdr-lab.sh:80-98, 156-159), a new publicreap-task <task-id> <worktree>command (bin/fm-herdr-lab.sh:601-631, 679-682), and a new 'Fix 4' teardown step in bin/fm-teardown.sh:3460-3464 that runs on every non-secondmate task teardown. The intent requires one thing:viewer stopmust stop recognising its own viewer as someone else's when WSL2 lstart drifts. The leaked lab is mentioned only as an observation ('suggests some teardown path may leak labs'). The change does not name the teardown path that leaked. It assumes the owner was killed before its EXIT trap ran and adds a backstop. Nothing in the intent requires the new durable state, the new command or the new teardown step. Recommended remedy: remove the owner record, reap-task and Fix 4 from this PR, and follow up separately once the leaking path is known. The captain can instead confirm that the backstop is wanted here.bin/fm-herdr-lab.sh:81- The owner record depends on FM_TASK_ID being set when the lab is provisioned. tests/lib.sh:54 runsunset FM_TASK_ID, and many real-Herdr suites source tests/lib.sh before they provision a lab: fm-herdr-attached-viewer-live-e2e (sources it at line 32, provisions at line 55), fm-control-claude-exit-dialog-live-e2e, fm-herdr-submit-confirm-live-e2e, fm-herdr-version-floor-live-e2e, fm-spawn-claude-transcript-live-e2e, fm-backend-herdr-presentation-e2e, and others. Example: a worker with FM_TASK_ID=T runsbin/fm-test-run.sh tests/fm-herdr-attached-viewer-live-e2e.test.shfrom its worktree and the suite is killed before its EXIT trap. The lab stays registered with no.ownerfile, andfm-teardown.sh Tfinds nothing to reap, so the lab leaks exactly as in the reported incident. That makes the backstop silently partial for the test labs it is meant to catch. This defect is inside the reap-task component flagged above, so the smallest honest remedy is to remove that component. If the component stays, ownership needs a task marker that tests/lib.sh does not clear, which extends the change and needs the author's sign-off.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bin/fm-herdr-lab.sh provision|viewer start|viewer stop|teardown fm-lab-lstartA-*against real herdr 0.9.0 on WSL2 kernel 6.18.33.2 (no drift)The same lifecycle with apsshim on PATH that renders lstart +1s, for HEADviewer stop/teardown(fm-lab-lstartB-*)The same injected drift against the base-commit copy of bin/fm-herdr-lab.sh plus fm-herdr-lab-viewer.py (fm-lab-lstartC-*), then cleanup without driftAdversarial: live viewer record rewritten with starttime+1 on HEAD, thenviewer stopandteardown(fm-lab-lstartD-*), then the genuine record restored and cleaned upHost probe sampling /proc/stat btime, PID 1ps -o lstartand /proc starttime over 15sbash tests/fm-herdr-lab.test.sh(focused suite incl. test_viewer_stop_survives_lstart_drift and test_viewer_stop_refuses_a_reused_pid)bash tests/fm-herdr-attached-viewer-live-e2e.test.sh(existing real-Herdr attached viewer e2e)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.