fix(watch): detect and handle crewmates parked at permission dialogs#419
Open
shgnplaatjies wants to merge 8 commits into
Open
fix(watch): detect and handle crewmates parked at permission dialogs#419shgnplaatjies wants to merge 8 commits into
shgnplaatjies wants to merge 8 commits into
Conversation
3 tasks
shgnplaatjies
force-pushed
the
fm/crew-rmrf-fix-q3
branch
2 times, most recently
from
395bbcb to
b06ec97
Compare
A crewmate's global settings.json keeps rm -rf in ask as a deliberate safety net, and that still prompts under --dangerously-skip-permissions. Routine cleanup (node_modules, .terraform, scratch traces) inside a crewmate's own worktree was hitting that prompt with nobody to answer it, stranding crewmates mid-turn with uncommitted work that was later lost when the worktree was reclaimed. fm-spawn.sh now writes a worktree-scoped rm -rf allow entry into each claude crewmate's own .claude/settings.local.json. Verified end to end with a real spawn: a scoped rm -rf inside the worktree now runs with no prompt, while one outside the worktree still correctly prompts. Ship and scout briefs now direct crewmates to do scratch/verification work inside their own worktree (using its absolute path) rather than an external temp directory, since only an absolute worktree-scoped command can match the pre-approved pattern.
…wmates" This reverts commit bcebab4fee4cfbe9e54b53d4685747aeb4556659.
Claude Code's ask-listed permission patterns (e.g. Bash(rm -rf *)) still prompt under --dangerously-skip-permissions, and nothing available to a per-task config file can suppress that: a worktree-scoped settings.local.json allow rule, and a PreToolUse hook forcing permissionDecision:allow, were both tried and empirically ruled out against a matching global ask rule. The only mechanism that works is narrowing the global ~/.claude/settings.json itself, outside any repo - a captain decision, not something this repo can generate. So this redirects the fix from suppression to detection and handling: a crewmate parked at one of these dialogs is not busy (no busy footer renders), so it was previously indistinguishable from routine idleness and could sit unattended, wedge, or lose uncommitted work if the session crashed before anyone noticed. bin/fm-tmux-lib.sh and bin/fm-crew-state.sh now recognize the dialog's "Do you want to proceed?" signature and report it as a distinct blocked/pane state instead of falling through to a stale status log or unknown. bin/fm-watch.sh tags the stale wake reason with a permission-dialog marker whenever this is why. The stuck-crewmate-recovery skill answers the dialog directly (the option number as literal text via fm-send.sh, not --key, which only supports Escape/Enter/C-c) ahead of its generic escalation ladder, since interrupting or relaunching would just recreate the same wedge - and notes a verified false-negative quirk in fm-send.sh's composer-clear check right after a dialog closes, so a firstmate following this guidance confirms via a peek rather than trusting a reported failure and retrying blind.
…dialog response guidance
…ssion-dialog detection
fm_backend_cmux_surface_exists piped the CLI call straight into jq -e without checking the CLI's own exit status. jq -e on completely empty stdin (e.g. because the underlying CLI call failed) exits 0, so a failed call read as "surface exists" instead of propagating the failure - masking a target-absent condition as a false positive. Unrelated to the permission-dialog work on this branch; found by no-mistakes' own test step (it runs the full suite) blocking on a pre-existing failure while validating that unrelated change.
Two more pre-existing, unrelated test failures root-caused after tool upgrades (git 2.34.1 -> 2.54.0, tasks-axi 0.1.2 -> 0.2.2) fixed the other two known gaps: fm-backend.test.sh's spawn conformance test pinned its "old" baseline via resolve_base_ref(), which preferred local `main` over origin/main. This repo is a treehouse pool of crewmate worktrees sharing one object store, and local `main` has no freshness guarantee (unlike PROJECT clones, fleet-sync does not keep it current) - it had drifted 68 commits behind origin/main, predating the backend-abstraction refactor entirely. That pinned a pre-refactor fm-spawn.sh as the comparison baseline, producing a false "tmux command log differs old vs new" diff unrelated to any real behavior change. Now prefers a remote- tracking ref, falling back to local main only when no remote exists. fm-session-start.test.sh's pi-harness tests set FM_FAKE_HARNESS=pi and mock `ps` to simulate process-ancestry detection, but bin/fm-harness.sh checks verified env markers (CLAUDECODE, PI_CODING_AGENT, GROK_AGENT) before ever reaching that fallback. Running the suite from inside a real claude session (as this task's own crewmate does) leaves CLAUDECODE=1 set in the ambient environment, which wins before the mock is ever consulted - detecting claude regardless of the fake, independent of which harness the test intends to simulate. Now clears all three verified markers before invoking fm-session-start.sh. Both found via no-mistakes' own test step (it runs the full suite) while validating the unrelated permission-dialog work on this branch.
…ion env vars and behavior
shgnplaatjies
force-pushed
the
fm/crew-rmrf-fix-q3
branch
from
b06ec97 to
6f910d7
Compare
Owner
|
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#419 at |
kunchenguid
added
the
wheelhouse:pending-contributor-action
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
2 participants
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.
What Changed
bin/fm-watch.shandbin/fm-tmux-lib.shadd compound-signature detection for panes parked at interactive permission-confirmation dialogs, surfacing a distinctstale: permission-dialogwake reason instead of generic staleness.bin/fm-crew-state.shreportsstate: blocked · source: panefor a detected permission dialog, with corrected guidance text for how firstmate should respond.bin/backends/cmux.shfixessurface_existsto check the CLI call's own exit status rather than a stale prior status..agents/skills/stuck-crewmate-recovery/SKILL.mdand.agents/skills/harness-adapters/SKILL.md, plusdocs/architecture.md,docs/configuration.md, anddocs/scripts.md, document the new detection env vars and response behavior; test suite updates resolve stale-baseline and env-leak false failures uncovered while adding coverage for the new detection path.Risk Assessment
✅ Low: The branch adds a well-tested, narrowly-scoped permission-dialog detection feature (compound signature to avoid false positives, correctly placed in the fallback path so it never overrides run-step or busy-pane precedence), fixes a genuine bug in cmux's surface_exists (previously swallowed the CLI's own exit status via a pipe, now checked explicitly), cleanly reverts an abandoned approach with no leftover artifacts, and includes matching regression tests and documentation updates; no correctness, security, or behavioral risks were found.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **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"✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.