Skip to content

fix(bin): retire task-keyed watcher markers and orphan journals at teardown - #5584

Closed
karotkriss wants to merge 3 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-up-5252-teardown-markers
Closed

karotkriss wants to merge 3 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-up-5252-teardown-markers

Conversation

@karotkriss

@karotkriss karotkriss commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Intent

Refs #5252

After a task closes, teardown removes its metadata, turn-end and inbox files but leaves the per-task .seen-* and .hb-surfaced-* watcher markers and any orphan Herdr presentation journal behind, so a home accumulates thousands of dead markers that slow every session start and wake drain.
Retire those task-keyed marker families at teardown and rotate the wake-drain scratch files, while leaving the task's data/<id>/ directory in place.
This addresses the state-marker part of upstream issue #5252.

What Changed

  • Teardown now deletes the torn-down task's per-task watcher markers - the .seen-* signatures for its status and turn-ended files - in all three cleanup paths (main, remote secondmate, and home-children), and removes the task's orphaned Herdr presentation journal once the recorded pane is proven gone, while leaving data/<id>/ in place.
  • Added teardown_herdr_journal_orphaned plus a new fm_backend_herdr_projection_token_workspace_gone helper in herdr.sh so a journal is retired only when it binds the exact closed pane, or is a version 1 attempt whose token-bearing projected workspace is confirmed gone; otherwise-bound, still-present, or unreadable journals are retained for the session-start sweep, with the stale-quarantine warning reworded accordingly.
  • fm-wake-drain.sh now rotates away scratch files (.main-eligible-rows.tmp.*, .wake-rows.consume.*, .wake-queue.retire.*, .wake-queue.ack.*, .wake-queue.actor-view.*) left by a drain that died mid-write, under the queue lock before draining; docs and tests updated to cover the marker retirement and scratch rotation.

Risk Assessment

✅ Low: Well-bounded teardown cleanup that mirrors existing Herdr correlation helpers, resolves the prior v1-journal-strands-workspace finding with a fail-safe workspace-gone check, uses exact (non-glob) marker paths matching the watcher's minting, rotates only self-minted scratch under the queue lock, and is covered by behavior-based tests.

Testing

Baseline: ran the full tests/fm-teardown.test.sh suite (101 ok, 0 failures) driving the real teardown script, which includes all four new marker/journal scenarios and confirms no regression in existing teardown paths. The wake-queue suite was run whole but hit its 550s timeout on an unrelated slow secondmate-liveness test before reaching the drain test, so the drain scratch-rotation scenario was driven in isolation against the real bin/fm-wake-drain.sh (pass) and against the pre-fix base script (fail), proving it is a real regression test. No visual surface is involved - this is CLI/shell tooling, so evidence is CLI transcripts of observable filesystem state after driving the real scripts. Worktree left clean; no live fleet watchers were touched.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
Teardown removes the closed task's .seen-/.hb-surfaced- markers and orphan journal, leaving other tasks' markers alone ✅ pass live bash tests/fm-teardown.test.sh line 29 ok - test_teardown_retires_task_watcher_markers_and_orphan_journal
Adversarial: teardown retains a presentation journal bound to a pane other than the closed endpoint (drifted v2 bind) ✅ pass live tests/fm-teardown.test.sh line 30 ok - test_teardown_retains_journal_bound_to_another_pane, with the 'retaining herdr presentation journal' warning emitted
Teardown retires a v1 attempt journal once its token-bearing projected workspace is confirmed gone ✅ pass live tests/fm-teardown.test.sh line 31 ok - test_teardown_retires_v1_journal_when_projected_workspace_gone (no workspace close ever called)
Adversarial guard: teardown retains a v1 journal while its token workspace is still present or unreadable, deferring to the session-start sweep ✅ pass live tests/fm-teardown.test.sh line 32 ok - test_teardown_retains_v1_journal_when_projected_workspace_present
Locked wake-drain rotates orphaned scratch files a dead drain left behind while preserving the live main rows claim ✅ pass live Isolated driver on real bin/fm-wake-drain.sh: ok (exit 0); same test on base 8d2ee29 script: not ok (exit 1). See drain-scratch-rotation.txt
Evidence: Teardown marker/journal scenarios (real fm-teardown.sh)

Source: Teardown marker/journal scenarios (real fm-teardown.sh)

=== teardown scenarios (from real bin/fm-teardown.sh driven by tests/fm-teardown.test.sh) ===
29:ok - teardown retires the task's own watcher markers and orphaned presentation journal, leaving other tasks' markers alone
30:ok - teardown retains a presentation journal bound to a pane other than the closed endpoint
31:ok - teardown retires a v1 presentation journal once its token workspace is confirmed gone
32:ok - teardown retains a v1 presentation journal while its token workspace is still present
Evidence: Drain scratch rotation: pass on fix, fail on base

Source: Drain scratch rotation: pass on fix, fail on base

=== drain scratch rotation (real bin/fm-wake-drain.sh, isolated driver) ===
Fixed script (target commit):
  ok - drain rotates scratch files an interrupted drain left under the queue lock
  EXIT=0

Same test against pre-fix drain script (base 8d2ee29):
  not ok - drain failed with orphaned scratch present
  EXIT=1

-> orphaned .main-eligible-rows.tmp.*/.wake-rows.consume.*/.wake-queue.{retire,ack,actor-view}.*
   are removed by the locked drain, while the live .main-eligible-rows claim survives.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
On task teardown, the closed task's .seen-<id>_status/.seen-<id>_turn-ended/.hb-surfaced-<id> markers and its orphaned herdr journal are removed, while another task's markers survive ✅ pass live tests/_focus_teardown.test.sh -> ok - teardown retires the task's own watcher markers and orphaned presentation journal, leaving other tasks' markers alone
Adversarial: a presentation journal bound to a pane other than the proven-gone endpoint is retained (not wrongly deleted) with a warning, for the session-start sweep ✅ pass live tests/_focus_teardown.test.sh -> ok - teardown retains a presentation journal bound to a pane other than the closed endpoint (asserts journal present + 'retaining herdr presentation journal' warning)
A v1 attempt journal is retired only once its token-bearing projected workspace is confirmed gone from a readable workspace list ✅ pass live tests/_focus_teardown.test.sh -> ok - teardown retires a v1 presentation journal once its token workspace is confirmed gone (and never calls workspace close)
Adversarial guard: a v1 journal whose token workspace is still present is retained, not deleted, so the sweep can still reclaim the workspace ✅ pass live tests/_focus_teardown.test.sh -> ok - teardown retains a v1 presentation journal while its token workspace is still present
A locked wake drain rotates away scratch files (.main-eligible-rows.tmp., .wake-rows.consume., .wake-queue.retire/ack/actor-view.*) an interrupted drain left behind, without removing the live main-r… ✅ pass live tests/_focus_wake.test.sh -> ok - drain rotates scratch files an interrupted drain left under the queue lock (and .main-eligible-rows preserved)
  • bash tests/_focus_teardown.test.sh (filtered driver running only the 4 new fm-teardown tests via the real bin/fm-teardown.sh through run_teardown)
  • bash tests/_focus_wake.test.sh (filtered driver running test_drain_rotates_orphaned_scratch against the real bin/fm-wake-drain.sh)
  • Reverted bin/fm-teardown.sh, bin/backends/herdr.sh, bin/fm-wake-drain.sh to base b42d4fa and re-ran both drivers to confirm fail-before/pass-after
  • Restored source to HEAD and removed the transient _focus_*.test.sh drivers; git status clean

✅ No issues found.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
Teardown removes the closed task's .seen-/.hb-surfaced- markers and orphan journal, leaving other tasks' markers alone ✅ pass live bash tests/fm-teardown.test.sh line 29 ok - test_teardown_retires_task_watcher_markers_and_orphan_journal
Adversarial: teardown retains a presentation journal bound to a pane other than the closed endpoint (drifted v2 bind) ✅ pass live tests/fm-teardown.test.sh line 30 ok - test_teardown_retains_journal_bound_to_another_pane, with the 'retaining herdr presentation journal' warning emitted
Teardown retires a v1 attempt journal once its token-bearing projected workspace is confirmed gone ✅ pass live tests/fm-teardown.test.sh line 31 ok - test_teardown_retires_v1_journal_when_projected_workspace_gone (no workspace close ever called)
Adversarial guard: teardown retains a v1 journal while its token workspace is still present or unreadable, deferring to the session-start sweep ✅ pass live tests/fm-teardown.test.sh line 32 ok - test_teardown_retains_v1_journal_when_projected_workspace_present
Locked wake-drain rotates orphaned scratch files a dead drain left behind while preserving the live main rows claim ✅ pass live Isolated driver on real bin/fm-wake-drain.sh: ok (exit 0); same test on base 8d2ee29 script: not ok (exit 1). See drain-scratch-rotation.txt
  • bash tests/fm-teardown.test.sh (full suite: 101 ok, 0 not ok, exit 0), covering the 4 new tests: test_teardown_retires_task_watcher_markers_and_orphan_journal, test_teardown_retains_journal_bound_to_another_pane, test_teardown_retires_v1_journal_when_projected_workspace_gone, test_teardown_retains_v1_journal_when_projected_workspace_present
  • Isolated driver sourcing tests/wake-helpers.sh running test_drain_rotates_orphaned_scratch against the real bin/fm-wake-drain.sh (ok, exit 0)
  • Same drain test against the base commit 8d2ee29 copy of bin/fm-wake-drain.sh (not ok, exit 1) - confirming the regression reproduces before the fix
✅ **Document** - passed

✅ No issues found.

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

✅ No issues found.

…ardown

Teardown removed a task's metadata, turn-end and inbox files but left the
watcher's .seen-<id>_turn-ended signature behind, and a Herdr presentation
journal whose pane the close path proved gone but could not match to a live
workspace was never retired, so a long-lived home accumulated dead markers.
Retire the turn-ended seen marker at every teardown site alongside the
status and heartbeat markers the presentation retire already owns, retire
an orphaned journal (a version 1 attempt or one bound to the closed pane)
once the Herdr presence gate has proven that pane gone while keeping a
journal bound to any other pane for the session-start sweep, and have each
locked wake drain rotate away the scratch files an interrupted drain left
under the queue lock.

Refs kunchenguid#5252
@karotkriss

Copy link
Copy Markdown
Contributor Author

Superseded by #5997, which re-applies this fix on current main and closes #5252; closing.

@karotkriss karotkriss closed this Sep 28, 2026
kunchenguid pushed a commit that referenced this pull request Sep 28, 2026
…ardown (#5997)

* WIP: retire task-keyed watcher markers and orphan journals at teardown

Re-applies old PR #5584 on current main: teardown retires the
turn-ended .seen-* signature and an orphaned Herdr presentation
journal whose workspace is already gone, and the wake-drain rotates
its own dead scratch files. Not yet validated through no-mistakes.

* no-mistakes(ci): Fixed the Greptile P1 finding in bin/backends/herdr.sh. fm_backend_herdr_projection_token_workspace_gone used `! ... jq -e ... 2>&1`, which swallowed a jq runtime error (thrown when a non-object workspace entry, e.g. a number before a live token-bearing workspace, hits `.label`) and flipped it to a "gone" verdict, causing teardown to delete a still-live v1 presentation journal. Invariant: a workspace-query error/ambiguity must never be read as token absence; only a cleanly-parsed list with no token-bearing label is "gone". Replaced the body with a single jq verdict (unknown/present/gone): a non-array list or any non-object/non-string-label entry yields "unknown", jq errors/empty output fall through `|| return 1` to unknown, and only "gone" returns 0. Sibling fm_backend_herdr_projection_endpoint_matches_journal already fails safe on jq error (empty match -> journal kept), so it needed no change, matching the author's scoping. Added test_teardown_retains_v1_journal_when_workspace_query_ambiguous driving real teardown with a malformed workspace-list entry, proving the journal is kept and no workspace close occurs. Verified the old logic returns GONE on that input (test fails before, passes after); full tests/fm-teardown.test.sh suite passes (exit 0) and shellcheck is clean. Marker-naming finding left untouched per explicit out-of-scope instruction
knowttl pushed a commit to knowttl/firstmate that referenced this pull request Sep 29, 2026
…ardown (kunchenguid#5997)

* WIP: retire task-keyed watcher markers and orphan journals at teardown

Re-applies old PR kunchenguid#5584 on current main: teardown retires the
turn-ended .seen-* signature and an orphaned Herdr presentation
journal whose workspace is already gone, and the wake-drain rotates
its own dead scratch files. Not yet validated through no-mistakes.

* no-mistakes(ci): Fixed the Greptile P1 finding in bin/backends/herdr.sh. fm_backend_herdr_projection_token_workspace_gone used `! ... jq -e ... 2>&1`, which swallowed a jq runtime error (thrown when a non-object workspace entry, e.g. a number before a live token-bearing workspace, hits `.label`) and flipped it to a "gone" verdict, causing teardown to delete a still-live v1 presentation journal. Invariant: a workspace-query error/ambiguity must never be read as token absence; only a cleanly-parsed list with no token-bearing label is "gone". Replaced the body with a single jq verdict (unknown/present/gone): a non-array list or any non-object/non-string-label entry yields "unknown", jq errors/empty output fall through `|| return 1` to unknown, and only "gone" returns 0. Sibling fm_backend_herdr_projection_endpoint_matches_journal already fails safe on jq error (empty match -> journal kept), so it needed no change, matching the author's scoping. Added test_teardown_retains_v1_journal_when_workspace_query_ambiguous driving real teardown with a malformed workspace-list entry, proving the journal is kept and no workspace close occurs. Verified the old logic returns GONE on that input (test fails before, passes after); full tests/fm-teardown.test.sh suite passes (exit 0) and shellcheck is clean. Marker-naming finding left untouched per explicit out-of-scope instruction
RooseveltAdvisors pushed a commit to RooseveltAdvisors/firstmate that referenced this pull request Sep 29, 2026
…ardown (kunchenguid#5997)

* WIP: retire task-keyed watcher markers and orphan journals at teardown

Re-applies old PR kunchenguid#5584 on current main: teardown retires the
turn-ended .seen-* signature and an orphaned Herdr presentation
journal whose workspace is already gone, and the wake-drain rotates
its own dead scratch files. Not yet validated through no-mistakes.

* no-mistakes(ci): Fixed the Greptile P1 finding in bin/backends/herdr.sh. fm_backend_herdr_projection_token_workspace_gone used `! ... jq -e ... 2>&1`, which swallowed a jq runtime error (thrown when a non-object workspace entry, e.g. a number before a live token-bearing workspace, hits `.label`) and flipped it to a "gone" verdict, causing teardown to delete a still-live v1 presentation journal. Invariant: a workspace-query error/ambiguity must never be read as token absence; only a cleanly-parsed list with no token-bearing label is "gone". Replaced the body with a single jq verdict (unknown/present/gone): a non-array list or any non-object/non-string-label entry yields "unknown", jq errors/empty output fall through `|| return 1` to unknown, and only "gone" returns 0. Sibling fm_backend_herdr_projection_endpoint_matches_journal already fails safe on jq error (empty match -> journal kept), so it needed no change, matching the author's scoping. Added test_teardown_retains_v1_journal_when_workspace_query_ambiguous driving real teardown with a malformed workspace-list entry, proving the journal is kept and no workspace close occurs. Verified the old logic returns GONE on that input (test fails before, passes after); full tests/fm-teardown.test.sh suite passes (exit 0) and shellcheck is clean. Marker-naming finding left untouched per explicit out-of-scope instruction
andrewesweet pushed a commit to andrewesweet/firstmate that referenced this pull request Sep 30, 2026
…ardown (kunchenguid#5997)

* WIP: retire task-keyed watcher markers and orphan journals at teardown

Re-applies old PR kunchenguid#5584 on current main: teardown retires the
turn-ended .seen-* signature and an orphaned Herdr presentation
journal whose workspace is already gone, and the wake-drain rotates
its own dead scratch files. Not yet validated through no-mistakes.

* no-mistakes(ci): Fixed the Greptile P1 finding in bin/backends/herdr.sh. fm_backend_herdr_projection_token_workspace_gone used `! ... jq -e ... 2>&1`, which swallowed a jq runtime error (thrown when a non-object workspace entry, e.g. a number before a live token-bearing workspace, hits `.label`) and flipped it to a "gone" verdict, causing teardown to delete a still-live v1 presentation journal. Invariant: a workspace-query error/ambiguity must never be read as token absence; only a cleanly-parsed list with no token-bearing label is "gone". Replaced the body with a single jq verdict (unknown/present/gone): a non-array list or any non-object/non-string-label entry yields "unknown", jq errors/empty output fall through `|| return 1` to unknown, and only "gone" returns 0. Sibling fm_backend_herdr_projection_endpoint_matches_journal already fails safe on jq error (empty match -> journal kept), so it needed no change, matching the author's scoping. Added test_teardown_retains_v1_journal_when_workspace_query_ambiguous driving real teardown with a malformed workspace-list entry, proving the journal is kept and no workspace close occurs. Verified the old logic returns GONE on that input (test fails before, passes after); full tests/fm-teardown.test.sh suite passes (exit 0) and shellcheck is clean. Marker-naming finding left untouched per explicit out-of-scope instruction
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