fix(daemon): suppress same-window refill double-injection and bound watcher stop - #5
Merged
Merged
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Away-mode Scenario B double-injected U+2063 when a done/failed/blocked/ needs-decision status already forced capacity re-evaluation and a same- window refill wake fired a second escalate. Cover the refill for one TTL- bounded shot, keep later refill-only wakes escalating, and align gotmp fixtures with teardown's remote-job lib sources.
…uite Arm signal handling and --restart waited unboundedly on a child watcher whose EXIT cleanup can block on recovery-marker locks. Escalate TERM to KILL after a short bound, and make wait_for_exit do the same so bare waits after intentional interrupts cannot hang the full portable suite.
…including replacement, ordering, and runner retirement
…ctives for sourced libs
jokim1
force-pushed
the
fm/fm-restore-green-main
branch
from
August 17, 2026 20:52
b20f281 to
036c6cd
Compare
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
Restore green CI on firstmate fork main (jokim1/firstmate) by fixing adopted-stack test regressions that block every firstmate PR. Captain ordered 2026-08-15 fix main. Named in-scope failures include afk-inject Scenario B duplicate U+2063 marker (real regression from cf26324/kunchenguid#2051; fix double-injection at source with regression assertion), pi-watch hung-successor, remote-job ready, and any other real adopted-stack regressions confirmed by local reproduction - fix code when behavior regressed, update tests only when expectation is stale against intentionally correct behavior; never weaken tests to hide regressions. Include fm-gotmp-fixture-lag only if not already on main. Shellcheck-clean; follow firstmate-coding-guidelines for shared tracked material. PR target origin jokim1/firstmate ONLY - never push/PR/touch upstream kunchenguid/firstmate. Full suite must pass locally and fork CI must go green; capture green CI run URL in PR body. Prior pipeline work on this branch (daemon refill suppression, watch-arm bounded stop, watcher-lock/fixture stabilizations through cb8cb58) must be preserved. Fix agent must NOT be codex (hangs on security-adjacent material); use non-codex agent (claude applied globally because grok is not a native no-mistakes agent and only codex/claude neutralize firstmate project instructions).
What Changed
fm-supervise-daemon.shso a status-driven capacity re-evaluation followed by a refill wake no longer double-injects the away-mode U+2063 marker; the refill is covered for one TTL-bounded shot while later refill-only wakes still escalate.fm-watch-arm.sh(andwait_for_exit) by escalating TERM to KILL after a short wait, so signal handling and--restartcannot hang indefinitely on a child watcher whose EXIT cleanup blocks on recovery-marker locks; the arm re-arms fresh and reclaims any stale lock.fm-watch-checkpoint.sh,fm-teardown.sh,fm-timeout-lib.sh, andfm-playbot-reconcile.mjs.Risk Assessment
✅ Low: The change is well-bounded and overwhelmingly test-stabilization; the few source edits are narrow, defensive, intent-aligned, and covered by strengthened (not weakened) tests, with no upstream references introduced.
Testing
Ran the four named in-scope regression tests plus the slow bearings-snapshot test individually through the canonical
bin/fm-test-run.shrunner; all passed (exit=0) with the afk-inject Scenario B duplicate-U+2063 assertion, pi-watch hung-successor fallback, remote-job ready bounds, and gotmp fixture all green. Inspected the test-file diffs to confirm the stabilizations replace fixed-count polling with wall-clock deadlines while keeping assertions intact rather than weakening them. The remaining changed-test batch (13 additional stabilized scripts, several multi-minute) was still running when I was required to return; nothing that executed failed. The changes are CLI/test-harness behavior with no rendered UI surface, so evidence is CLI transcripts (per-testok -lines and FM_TEST_END exit=0 markers) rather than screenshots — appropriate for a shell toolbelt. Full local suite green and fork CI green-URL capture in the PR body remain the author's responsibility and were not verifiable in this isolated worktree.Evidence: afk-inject + pi-watch named-regression transcript
ok - Scenario B: swallowed Enter produces exactly one clean digest ok - Pi hung successor falls back to one typed actionable wake ok - OpenCode hung successor falls back to one typed actionable wake FM_TEST_END tests/fm-afk-inject-e2e.test.sh exit=0 FM_TEST_END tests/fm-pi-watch-extension.test.sh exit=0Evidence: remote-job + gotmp transcript
FM_TEST_END tests/fm-remote-job.test.sh exit=0 duration_ms=91095 gate_skip=false FM_TEST_END tests/fm-gotmp.test.sh exit=0 duration_ms=16439 gate_skip=false FM_TEST_SUMMARY total=2 failed=0 skipped_gate=0Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 2 issues found → auto-fixed ✅
bin/fm-supervise-daemon.sh- merge conflict rebasing onto origin/lila-maintests/fm-daemon.test.sh- merge conflict rebasing onto origin/lila-main🔧 Fix applied.
✅ Re-checked - no issues remain.
bin/fm-watch-arm.sh:438---restartbehavior changed: a TERM-resistant but healthy watcher holding the watch lock is now force-killed viastop_pid_bounded(TERM → 5s wait → KILL) and the arm re-arms fresh, surfacing durable recovery; previously the restart attached to that live peer instead. The rewritten/renamed testtest_watch_restart_bounds_term_resistant_healthy_peercodifies the new contract. This is the intended 'watch-arm bounded stop' work (commit 44e1132) and is intent-aligned, but it is a real semantics shift in a core supervisor path: a healthy-but-slow-to-TERM watcher can now be KILLed during an explicit restart, leaving a stale lock the fresh watcher must reclaim.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-afk-inject-e2e.test.sh— Scenario A/B/C all ok; Scenario B swallowed-Enter produces exactly one clean U+2063 digest (exit=0)bin/fm-test-run.sh tests/fm-pi-watch-extension.test.sh— Pi + OpenCode hung-successor fallback and 30 lifecycle assertions ok (exit=0)bin/fm-test-run.sh tests/fm-remote-job.test.sh— remote-job ready/queue bounds ok (exit=0)bin/fm-test-run.sh tests/fm-gotmp.test.sh— session-bootstrap fixture ok (exit=0)bin/fm-test-run.sh tests/fm-bearings-snapshot.test.sh— snapshot-bearings ok (exit=0, 382s)Reviewedgit diffof tests/fm-pi-watch-extension.test.sh confirming assertions preserved under deadline-based waitForLaunched remaining changed tests (calm-pi-extension, daemon, inactive-reconcile, pr-check-security, procevent, secondmate-safety, spawn-worktree-settle, watch-arm, watch-checkpoint, watch-triage, watcher-lock, herdr-lab, remote-secondmate-lifecycle-e2e) — still executing (calm-pi-extension in progress, no failures) when finalization was required✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: fm-watch-checkpoint.sh: add shellcheck source directives for sourced libs
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.
Green CI
Fork CI green: https://github.com/jokim1/firstmate/actions/runs/32068154155
Head:
036c6cdb27f3842f5f68b21963ab3d576501b784Fixer: global
agent: claudein~/.no-mistakes/config.yaml(grok is not a native no-mistakes agent; firstmatedisable_project_settingsonly neutralizes codex/claude).