Conversation
Stop headless grok --resume from the turn-end adapter (zombie hang path). Ensure a detached watcher and write state/.supervision-gap instead. Quote-aware seatbelt fail-closed so diagnostic shells mentioning arm paths as data are allowed; real unquoted if/for arm bodies still deny. Document the standalone arm-only re-arm shape and update unit tests.
…g composer borders
Light breadcrumb when a prior Grok blind turn left a gap marker, so the pull-based guard points operators at the durable record.
…sure-lock and log hardening
… stripping to seatbelt tools
|
Thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for firstmate#461 at |
kunchenguid
added
the
wheelhouse:pending-contributor-action
|
Automated reminder: this PR still looks blocked on a rebase or merge conflict fix. If you are still interested, please rebase onto the current base branch, resolve the conflict, and push. If I do not hear back, I may close this as inactive. |
|
I am closing this because it has been waiting on a rebase or merge-conflict fix since 2026-07-13, and I have not seen a comment or push since then. If you still want to keep working on this, please reopen it or open a new PR and mention this one. Happy to take another look when there is an update. |
2 participants
Intent
Production readiness for Grok primary supervision reliability after repeated silent fleet gaps (2026-07-11).
Goals:
Push target is the korallis/firstmate fork (write access); open PR to kunchenguid/firstmate. Do not merge.
What Changed
grok --resumewith a mechanical guard (bin/fm-turnend-guard-grok.sh): on a blind turn it writes a durablestate/.supervision-gapmarker and directly ensures a detachedbin/fm-watch.sh, using an ensure lock taken only aftermkdirwith atomic stale-age recovery and a size-capped ensure log; the legacyGROK_TURNEND_GUARD_ACTIVEenv is kept as a loop-guard no-op.bin/fm-arm-command-policy.mjs) to fail closed on quoting: a proper quote scanner eliminates the apostrophe cross-quote false-allow, quoted commands in execution position (including interpreter-cquoted exec) are denied, and--commandargument stripping is scoped to seatbelt tools only.bin/fm-guard.shnow surfacesstate/.supervision-gapin the watcher-down banner and retires the marker once supervision is healthy, with docs (docs/turnend-guard.md, supervision protocol and architecture docs) synced to the new lock, log, and re-arm behavior; note the branch base also carries prior local-main commits not yet pushed to origin, so the raw diff is wider than the supervision work itself.Risk Assessment
✅ Low: The delta since the last review round is a 14-line, test-backed fix that I verified empirically closes both reported quoted-exec bypass shapes without regressing legitimate allow cases; only contrived defense-in-depth edges remain.
Testing
Baseline full suite was already green; re-ran the three branch-focused suites (turn-end guard, arm pretool seatbelt, watcher lock) green, then demonstrated the change end-to-end with the real hooks: the Grok Stop hook wrote the durable supervision-gap marker and mechanically ensured a real detached watcher without ever spawning headless grok --resume, fm-guard surfaced the gap in its watcher-down banner and retired it once healthy, the ensure lock/log-cap and legacy loop-guard behaved as documented, and the quote-aware seatbelt allowed previously false-denied diagnostic commands while denying quoted-exec, apostrophe-mispair, interpreter -c, and foreign --command executions. No screenshot artifacts because the change has no rendered UI surface — the terminal hook transcripts are the end-user surface and are captured as evidence.
Evidence: Grok turn-end guard E2E transcript (gap marker, watcher ensure, banner, retirement, loop-guard, log cap)
Evidence: Quote-aware seatbelt E2E transcript (10 allow/deny cases through the real PreToolUse transport)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
.agents/skills/afk/SKILL.md- branch carries 59 commit(s) that exist on your local main branch but were never pushed to origin/main; rebasing would bundle this unrelated work (147 file(s)) into the PR:Push main to origin, or rebase your branch onto origin/main, before gating.
🔧 Fix applied.
1 warning still open:
.agents/skills/afk/SKILL.md- branch carries 59 commit(s) that exist on your local main branch but were never pushed to origin/main; rebasing would bundle this unrelated work (147 file(s)) into the PR:Push main to origin, or rebase your branch onto origin/main, before gating.
bin/fm-arm-command-policy.mjs:70- The new unsupported-grammar fallback strips quoted regions as data unless they open command position, so a quoted protected path executed through an interpreter is false-allowed:if true; then bash -c "bin/fm-watch-arm.sh"; fireturns allow (verified with node), while the identicalbash -c "bin/fm-watch-arm.sh"outside the if denies as watcher-nested, and the pre-change fallback (rawMentionsProtected) denied any mention. Same gap via stripCommandFlagArgs for tools whose --command flag executes its argument (e.g.if true; then su root --command bin/fm-watch-arm.sh; fiallows). Broad-kill shapes remain denied, so the destructive cases are still covered, but this is a real execution shape the seatbelt used to catch. Consider keeping quoted content visible when the preceding word is an interpreter -c/-lc style flag, or restricting --command stripping to fm-arm-pretool-check.sh invocations.bin/fm-turnend-guard-grok.sh:86- The ensure-lock steal is not fully atomic despite the comment: two racers that both pass the age>=60 check can interleave so racer B'smvsteals racer A's freshly recreated lock (age-check TOCTOU), and both proceed; A's EXIT cleanup then rmdir's B's lock. Consequence is bounded - at worst two detached fm-watch.sh spawns, and the watcher's own singleton lock self-evicts the duplicate - so this is acceptable, but the "two racers can never both claim it" comment overclaims.bin/fm-turnend-guard-grok.sh:101- Log rotation replaces the inode (tail > tmp && mv), but a previously detached watcher still holds an open append fd on the old unlinked inode; its subsequent output bypasses the size cap invisibly until that watcher exits, and the visible log misses it. Debug-only log with bounded practical growth, so acceptable as-is; appending rotation (truncate-in-place) would avoid it if it ever matters.bin/fm-guard.sh:75- The gap-marker retire keys on the loose beacon-freshness predicate (fm_supervision_status FM_SUP_WATCHER_FRESH), while the writer (fm-turnend-guard-grok.sh via fm_watcher_healthy) requires an identity-matched lock. A fresh beacon with a mismatched lock identity would retire a gap the turn-end guard still considers blind; self-correcting because the marker is rewritten on the next blind turn, so worst case is a briefly missing banner line.🔧 Fix: deny interpreter -c quoted exec; scope --command stripping to seatbelt tools
1 info still open:
bin/fm-arm-command-policy.mjs:68- The interpreter -c detection requires the shell name immediately before the -c flag, soif true; then bash -o errexit -c "bin/fm-watch-arm.sh"; fistill false-allows in the unsupported-grammar fallback, and non-shell interpreters (python3 -c 'os.system("bin/fm-watch-arm.sh")') allow on both the main and fallback paths (verified with node). Contrived shapes for a seatbelt guarding against accidental unsafe commands; the realistic sh/bash/zsh -c shapes from the round-1 finding are now covered with tests. Acceptable as-is.✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline configured command (all tests/*.test.sh under tmux) ran green before this sessionbash tests/fm-turnend-guard.test.sh(38 checks: predicate, shared hook, grok adapter gap/ensure/lock/loop-guard, per-harness hook wiring)bash tests/fm-arm-pretool-check.test.sh(matrix + direct policy contract incl. new quoted-data allow, quote-mispair deny, quoted-exec deny, interpreter -c deny, --command scoping)bash tests/fm-watcher-lock.test.sh(incl. new gap-marker surfaced-in-banner and retired-only-when-healthy assertions)Manual E2E: piped Grok Stop payload through the real bin/fm-turnend-guard-grok.sh in a sandbox primary home with a live tmux task window and a decoy grok on PATH; verified gap marker content, real detached fm-watch.sh with singleton lock + fresh beacon, zero grok invocations, banner surfacing + healthy-path retirement via bin/fm-guard.sh, legacy GROK_TURNEND_GUARD_ACTIVE no-op, and ensure-log 500-line trim of a 288KB logManual E2E: submitted 10 representative commands through the real bin/fm-arm-pretool-check.sh stdin transport (Grok toolInput schema); all ALLOW/DENY outcomes and Grok-shaped decision objects matched expectationsConfirmed no stray demo processes/tmux sessions and a clean worktree afterward✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.