feat(bin): recurring automated review sweep across the fleet - #39
Closed
meisbokai wants to merge 9 commits into
Closed
feat(bin): recurring automated review sweep across the fleet#39meisbokai wants to merge 9 commits into
meisbokai wants to merge 9 commits into
Conversation
A headless dispatch (cron sweep, or any spawn without a live supervisor watching the pane) must not depend on manual rescue. Two verified races broke the existing path on this host (default shell fish): 1. PATH TRANSIENT: for ~100ms after `new-window -c PROJ_ABS`, tmux reports pane_current_path as $HOME before fish's slow startup applies the -c directory. The old "path != PROJ_ABS" check broke on that transient and captured $HOME (e.g. /home/boks), so meta's worktree= was truncated and hooks/launch targeted the wrong directory. 2. FOREGROUND RACE: while `treehouse get` runs it is the foreground process; the worktree subshell is its child. A launch command sent before the subshell is foreground lands in the wrong context and is dropped, so the agent never starts and the pane is left at a shell prompt. Fix: - Detection now requires a real treehouse worktree path (under ~/.treehouse/) that is NOT the project dir, an interactive shell foreground (subshell ready), AND stable across two consecutive polls — defeating both the $HOME blip and any intermediate worktree path as treehouse selects one. - Post-launch verify: confirm pane_current_command flips to the harness binary within a grace window; resend the launch once and re-check if not. Exit non-zero if the harness still fails to start. - Add `--approve` to the pi launch template so pi runs fully unattended.
bin/fm-review-sweep.sh runs unattended from cron (every 8h) to review every open fleet PR. Each run: - Enumerates open PRs across all fleet repos (resolved from data/projects.md). - Excludes drafts and already-approved PRs (reviewDecision=APPROVED). - Fetches CI status per PR; failing-CI PRs are KEPT and their review brief adds an 'investigate the CI failure root cause' instruction with a ## CI Failure section in the posted comment. - Dispatches a review-rectify-pi review (--push, REVIEW-ONLY: no code edits or fixes) per kept PR as a firstmate crewmate via bin/fm-spawn.sh, bounded to FM_SWEEP_CONCURRENCY (3) concurrent reviews via a spawn/reap pool. - After each review posts its comment, parses the recommendation; on a clean APPROVE (not CONDITIONAL APPROVE) transitions the PR's MILE- key to 'In Review' via 'jira issue move'. - Logs a run summary to state/sweep.log; a single PR failure never aborts the sweep (set -uo pipefail, no -e; best-effort per-PR). Guarded by flock (no overlapping runs). --dry-run enumerates+filters+prints the plan and dispatches nothing. The exact crontab line is documented in the header for firstmate to install after merge.
AGENTS.md: add a 'Recurring review sweep (cron)' subsection under the supervision protocol covering the cadence, fleet scope, draft/approved filters, failing-CI handling, review-only contract, bounded concurrency, and the clean-APPROVE -> Jira 'In Review' tie-in. Includes the crontab line for firstmate to install after merge. README.md: add fm-review-sweep.sh to the bin/ toolbelt table and document the FM_SWEEP_CONCURRENCY / FM_SWEEP_TASK_TIMEOUT tunables.
Wrap the sweep's main body in main() guarded by a BASH_SOURCE check so the helper functions can be sourced by tests without side effects (no flock, mkdir, or gh calls on source). Add tests/fm-review-sweep.test.sh covering the side-effect-free logic: - resolve_fleet: parses owner/name + clone path from registry bullets, strips trailing .git - ci_status: FAILURE => fail, SUCCESS/SKIPPED => pass, PENDING/STARTED => not fail, empty => unknown - parse_recommendation: clean APPROVE vs CONDITIONAL APPROVE (the Jira tie-in must key on APPROVE only), plus BLOCK/REQUEST CHANGES/CAUTION - jira_key_for: MILE- key from title and body, empty when absent gh is stubbed on PATH; no network. All bin/ and tests/ remain shellcheck-clean.
…anup, resend safety
…x failing-CI detection
…e trust, and headless reliability pillars
…fail window leak, precise worktree return, dry-run header
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
eyevanovich
added a commit
to eyevanovich/firstmate
that referenced
this pull request
Jul 23, 2026
## Intent
Fix the live GitLab note-verification regression blocking completion of
KissCut issue 146 after the guarded GitLab workflow changes. Preserve
idempotence so an existing note is reused without duplicates, and retain
trusted-project, positive-resource-ID, authenticated-author, exact-body,
non-system-note, regular-file, and noteable-resource guards. Use the
current GitLab work-item-backed issue note response shape, audit the
shared issue and merge-request note
creation/list/read-back/pagination/body-encoding identity path together,
and batch only directly implied note defects rather than broadening
scope. Add deterministic live-shape fixture coverage and validate
focused forge behavior, lint, and the portable suite. Do not mutate
KissCut issue 146 from this fix task.
## What Changed
- Accept GitLab work-item-backed issue notes while verifying the trusted
project, noteable resource ID and type, author, exact body, and
non-system status.
- Preserve idempotent issue and merge-request note creation by matching
existing notes against their underlying resource identity before
posting.
- Add a deterministic live-shape fixture and focused coverage for
work-item responses, body encoding, pagination, and duplicate
suppression.
## Risk Assessment
✅ Low: Captain, the change is well-bounded and satisfies the
authoritative note-verification intent; the only finding is a
non-functional simplification opportunity.
## Testing
The already-passing portable baseline was supplemented with focused
GitLab forge mutation testing and an end-to-end CLI trace;
work-item-shaped issue notes verify successfully, preserve exact encoded
content, reuse the existing note without duplicates, and retain the
shared issue/MR safety guards, with no live mutation of KissCut issue
146.
<details>
<summary>Evidence: Focused forge mutation test transcript</summary>
```text
ok - issue claim sends explicit JSON media type and preserves array encoding
ok - live work-item note shape verifies exact resource and suppresses duplicates
ok - issue creation validates labels, optional claim, identity, and canonical read-back
ok - issue claim, status, note, close, reopen, and release converge safely
ok - already-correct issue claims, statuses, and label deltas are no-ops
ok - issue release can return self-owned work to the ready queue without clobbering labels
ok - issue mutations cannot steal or bypass exact ownership
ok - missing, archived, and malformed label metadata blocks mutations
ok - malformed targets, labels, usernames, and non-regular note files are rejected
ok - issue API identity rejects foreign, mismatched, and malformed work-item URLs
ok - untrusted projects and API failures never report a successful mutation
ok - issue and note mutations require deterministic matching read-back
ok - merge-request claim, status, labels, notes, lifecycle, and release converge safely
ok - merge-request workflow labels require status and release commands
ok - merge-request mutations preserve project, author, branch, head, and API guards
ok - issue creation and comments surface API and verification failures
```
</details>
<details>
<summary>Evidence: End-to-end GitLab note CLI trace showing first
creation, exact body preservation, reuse on retry, pagination, and a
single POST</summary>
```text
+ 6: set -u
++ 9: dirname tests/fm-forge-mutations.test.sh
+ 9: . tests/lib.sh
++ 23: '[' -n '' ']'
++ 26: FM_TEST_LIB_SOURCED=1
++ 35: export FM_GATE_REFUSE_BYPASS=1
++ 35: FM_GATE_REFUSE_BYPASS=1
++++ 40: dirname tests/lib.sh
+++ 40: cd tests/..
+++ 40: pwd
++ 40: ROOT=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4
++ 45: unset GIT_CONFIG_COUNT GIT_CONFIG_PARAMETERS
++ 46: IFS=
++ 46: read -r variable
+++ 48: compgen -A variable GIT_CONFIG_KEY_
+++ 48: compgen -A variable GIT_CONFIG_VALUE_
++ 49: export GIT_CONFIG_GLOBAL=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixture.gitconfig
++ 49: GIT_CONFIG_GLOBAL=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixture.gitconfig
++ 69: FM_TEST_CLEANUP_DIRS=()
++ 11: fm_test_tmproot fm-forge-mutations-tests
++ 79: local prefix=fm-forge-mutations-tests root
+++ 80: mktemp -d /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.XXXXXX
++ 80: root=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB
++ 81: '[' 0 -eq 0 ']'
++ 82: trap fm_test_cleanup EXIT
++ 84: FM_TEST_CLEANUP_DIRS+=("$root")
++ 85: printf '%s\n' /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB
++ 1: fm_test_cleanup
++ 72: local d
++ 73: for d in "${FM_TEST_CLEANUP_DIRS[@]:-}"
++ 74: '[' -n /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB ']'
++ 74: rm -rf /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB
+ 11: TMP=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB
+ 12: ADAPTER=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh
+ 13: REPO=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo
++ 14: fm_fakebin /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB
++ 95: local dir=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB fakebin=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin
++ 96: mkdir -p /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin
++ 97: printf '%s\n' /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin
+ 14: FAKEBIN=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin
+ 15: LOG=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log
+ 16: ISSUE_STATE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json
+ 17: MR_STATE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json
+ 18: ISSUE_NOTES=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json
+ 19: MR_NOTES=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json
+ 20: WORK_ITEM_NOTE_FIXTURE=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixtures/gitlab-work-item-note.json
+ 21: MUTATED=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated
+ 23: fm_git_init_commit /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo
+ 125: local dir=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo
+ 126: mkdir -p /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo
+ 127: git -C /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo init -q
++ 128: basename /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo
+ 128: printf '# %s\n' repo
+ 129: git -C /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo add README.md
+ 130: git -C /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo -c 'user.name=Firstmate Tests' -c user.email=tests@example.invalid commit -qm initial
+ 24: git -C /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo remote add origin git@gitlab.com:kisscut-museum/kisscut-platform.git
+ 25: git -C /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo checkout -q -b fm/fix
+ 27: cat
+ 347: chmod +x /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin/glab
+ 797: test_live_415_regression_claim_uses_json_media_type
+ 375: local out
+ 376: reset_case
+ 365: :
+ 366: rm -f /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated
++ 377: FM_FAKE_ISSUE_OWNER=none
++ 377: run_adapter issue-claim /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo 7
++ 350: PATH=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin:/Users/ipiesh/.codex/tmp/arg0/codex-arg0wM5SxM:/nix/store/ygxqin6ydzjfawywqpp5pal8wv6sf5bh-python3-3.13.13/bin:/nix/store/yggg7hbh9bi0p4c44npy2mdyjbj5d37h-grc-1.13/bin:/Users/ipiesh/.local/bin:/Users/ipiesh/go/bin:/Users/ipiesh/.cargo/bin:/opt/homebrew/bin:/Users/ipiesh/.nix-profile/bin:/etc/profiles/per-user/ipiesh/bin:/run/current-system/sw/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/ipiesh/bin:/opt/homebrew/sbin:/usr/local/sbin
++ 350: FM_FAKE_GLAB_LOG=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log
++ 350: FM_FAKE_ISSUE_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json
++ 350: FM_FAKE_MR_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json
++ 350: FM_FAKE_ISSUE_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json
++ 350: FM_FAKE_MR_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json
++ 350: FM_FAKE_WORK_ITEM_NOTE_FIXTURE=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixtures/gitlab-work-item-note.json
++ 350: FM_FAKE_MUTATED_MARKER=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated
++ 350: FM_FORGE_HOSTS_FILE=
++ 350: GITLAB_TOKEN=AMBIENT
++ 350: GITLAB_ACCESS_TOKEN=AMBIENT
++ 350: OAUTH_TOKEN=AMBIENT
++ 350: GLAB_ENABLE_CI_AUTOLOGIN=true
++ 350: CI_JOB_TOKEN=AMBIENT
++ 350: /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh issue-claim /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo 7
+ 377: out='{"forge":"gitlab","host":"gitlab.com","project":"kisscut-museum/kisscut-platform","issue":{"iid":7,"title":"Fix cutter","state":"opened","url":"https://gitlab.com/kisscut-museum/kisscut-platform/-/work_items/7","description":"Issue body","labels":["bug","status::in-progress"],"author":"ivan","assignees":["mate"],"updated_at":"2026-07-18T00:00:00Z"}}'
+ 379: jq -e '.issue.assignees == ["mate"]'
+ 381: assert_grep 'issues/7 --hostname gitlab.com --method PUT --input - --header Content-Type: application/json' /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log 'issue claim JSON media type'
+ 208: grep -F -- 'issues/7 --hostname gitlab.com --method PUT --input - --header Content-Type: application/json' /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log
+ 384: assert_grep 'input={"assignee_ids":[42]' /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log 'issue claim array
... [243589 bytes truncated] ...
S_FILE=\n++ 350: GITLAB_TOKEN=AMBIENT\n++ 350: GITLAB_ACCESS_TOKEN=AMBIENT\n++ 350: OAUTH_TOKEN=AMBIENT\n++ 350: GLAB_ENABLE_CI_AUTOLOGIN=true\n++ 350: CI_JOB_TOKEN=AMBIENT\n++ 350: /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh issue-create /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo --title \'Create mismatch\' --body-file /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo/create-body.md --label bug\nerror: issue identity does not match trusted repository\nerror: created issue identity could not be verified'
+ 784: rc=1
+ 785: expect_code 1 1 'created issue identity mismatch'
+ 201: local expected=1 actual=1 'label=created issue identity mismatch'
+ 202: '[' 1 = 1 ']'
+ 786: assert_contains $'++ 350: PATH=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin:/Users/ipiesh/.codex/tmp/arg0/codex-arg0wM5SxM:/nix/store/ygxqin6ydzjfawywqpp5pal8wv6sf5bh-python3-3.13.13/bin:/nix/store/yggg7hbh9bi0p4c44npy2mdyjbj5d37h-grc-1.13/bin:/Users/ipiesh/.local/bin:/Users/ipiesh/go/bin:/Users/ipiesh/.cargo/bin:/opt/homebrew/bin:/Users/ipiesh/.nix-profile/bin:/etc/profiles/per-user/ipiesh/bin:/run/current-system/sw/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/ipiesh/bin:/opt/homebrew/sbin:/usr/local/sbin\n++ 350: FM_FAKE_GLAB_LOG=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log\n++ 350: FM_FAKE_ISSUE_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json\n++ 350: FM_FAKE_MR_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json\n++ 350: FM_FAKE_ISSUE_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json\n++ 350: FM_FAKE_MR_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json\n++ 350: FM_FAKE_WORK_ITEM_NOTE_FIXTURE=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixtures/gitlab-work-item-note.json\n++ 350: FM_FAKE_MUTATED_MARKER=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated\n++ 350: FM_FORGE_HOSTS_FILE=\n++ 350: GITLAB_TOKEN=AMBIENT\n++ 350: GITLAB_ACCESS_TOKEN=AMBIENT\n++ 350: OAUTH_TOKEN=AMBIENT\n++ 350: GLAB_ENABLE_CI_AUTOLOGIN=true\n++ 350: CI_JOB_TOKEN=AMBIENT\n++ 350: /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh issue-create /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo --title \'Create mismatch\' --body-file /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo/create-body.md --label bug\nerror: issue identity does not match trusted repository\nerror: created issue identity could not be verified' identity 'created issue verification refusal'
+ 185: case "$1" in
+ 186: :
+ 788: reset_case
+ 365: :
+ 366: rm -f /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated
++ 790: FM_FAKE_API_FAIL=issue-create
++ 790: run_adapter issue-create /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo --title 'Create failure' --label bug
+ 790: out=$'++ 350: PATH=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin:/Users/ipiesh/.codex/tmp/arg0/codex-arg0wM5SxM:/nix/store/ygxqin6ydzjfawywqpp5pal8wv6sf5bh-python3-3.13.13/bin:/nix/store/yggg7hbh9bi0p4c44npy2mdyjbj5d37h-grc-1.13/bin:/Users/ipiesh/.local/bin:/Users/ipiesh/go/bin:/Users/ipiesh/.cargo/bin:/opt/homebrew/bin:/Users/ipiesh/.nix-profile/bin:/etc/profiles/per-user/ipiesh/bin:/run/current-system/sw/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/ipiesh/bin:/opt/homebrew/sbin:/usr/local/sbin\n++ 350: FM_FAKE_GLAB_LOG=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log\n++ 350: FM_FAKE_ISSUE_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json\n++ 350: FM_FAKE_MR_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json\n++ 350: FM_FAKE_ISSUE_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json\n++ 350: FM_FAKE_MR_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json\n++ 350: FM_FAKE_WORK_ITEM_NOTE_FIXTURE=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixtures/gitlab-work-item-note.json\n++ 350: FM_FAKE_MUTATED_MARKER=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated\n++ 350: FM_FORGE_HOSTS_FILE=\n++ 350: GITLAB_TOKEN=AMBIENT\n++ 350: GITLAB_ACCESS_TOKEN=AMBIENT\n++ 350: OAUTH_TOKEN=AMBIENT\n++ 350: GLAB_ENABLE_CI_AUTOLOGIN=true\n++ 350: CI_JOB_TOKEN=AMBIENT\n++ 350: /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh issue-create /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo --title \'Create failure\' --label bug\nerror: issue creation failed'
+ 791: rc=1
+ 792: expect_code 1 1 'issue creation API failure'
+ 201: local expected=1 actual=1 'label=issue creation API failure'
+ 202: '[' 1 = 1 ']'
+ 793: assert_contains $'++ 350: PATH=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/fakebin:/Users/ipiesh/.codex/tmp/arg0/codex-arg0wM5SxM:/nix/store/ygxqin6ydzjfawywqpp5pal8wv6sf5bh-python3-3.13.13/bin:/nix/store/yggg7hbh9bi0p4c44npy2mdyjbj5d37h-grc-1.13/bin:/Users/ipiesh/.local/bin:/Users/ipiesh/go/bin:/Users/ipiesh/.cargo/bin:/opt/homebrew/bin:/Users/ipiesh/.nix-profile/bin:/etc/profiles/per-user/ipiesh/bin:/run/current-system/sw/bin:/nix/var/nix/profiles/default/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/ipiesh/bin:/opt/homebrew/sbin:/usr/local/sbin\n++ 350: FM_FAKE_GLAB_LOG=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/glab.log\n++ 350: FM_FAKE_ISSUE_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue.json\n++ 350: FM_FAKE_MR_STATE_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr.json\n++ 350: FM_FAKE_ISSUE_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/issue-notes.json\n++ 350: FM_FAKE_MR_NOTES_FILE=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mr-notes.json\n++ 350: FM_FAKE_WORK_ITEM_NOTE_FIXTURE=/Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/tests/fixtures/gitlab-work-item-note.json\n++ 350: FM_FAKE_MUTATED_MARKER=/var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/mutated\n++ 350: FM_FORGE_HOSTS_FILE=\n++ 350: GITLAB_TOKEN=AMBIENT\n++ 350: GITLAB_ACCESS_TOKEN=AMBIENT\n++ 350: OAUTH_TOKEN=AMBIENT\n++ 350: GLAB_ENABLE_CI_AUTOLOGIN=true\n++ 350: CI_JOB_TOKEN=AMBIENT\n++ 350: /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY67HH2JGNYDV9VCR0WDWXX4/bin/fm-forge.sh issue-create /var/folders/j_/g4b3__rs5j95fq6xf9fb17sc0000gn/T//fm-forge-mutations-tests.eHx8KB/repo --title \'Create failure\' --label bug\nerror: issue creation failed' 'issue creation failed' 'issue creation failure report'
+ 185: case "$1" in
+ 186: :
+ 794: pass 'issue creation and comments surface API and verification failures'
+ 59: printf 'ok - %s\n' 'issue creation and comments surface API and verification failures'
ok - issue creation and comments surface API and verification failures
```
</details>
## Pipeline
Updates from [git push
no-mistakes](https://github.com/kunchenguid/no-mistakes)
<details>
<summary>✅ **intent** - passed</summary>
✅ No issues found.
</details>
<details>
<summary>✅ **Rebase** - passed</summary>
✅ No issues found.
</details>
<details>
<summary>⚠️ **Review** - 1 info</summary>
- ℹ️ `bin/fm-forge.sh:586` - The existing-note selector duplicates the
full trusted note-identity predicate from `note_identity_valid()`.
Extract a shared jq predicate/filter so list-time idempotence matching
and read-back verification cannot drift as guards evolve.
</details>
<details>
<summary>✅ **Test** - passed</summary>
✅ 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"`
- <code>Reviewed `git diff --unified=100
f68d7c9..4bab937`
against the authoritative intent.</code>
- <code>Accepted the previously successful configured portable baseline:
`command -v tmux >/dev/null || { echo &kunchenguid#34;tmux is required for e2e
tests&kunchenguid#34; >&2; exit 1; }; tmux -V; rc=0; for t in
tests/*.test.sh; do echo &kunchenguid#34;== $t ==&kunchenguid#34;; bash &kunchenguid#34;$t&kunchenguid#34; || rc=1;
done; exit &kunchenguid#34;$rc&kunchenguid#34;`.</code>
- <code>Ran `bash tests/fm-forge-mutations.test.sh` against the
deterministic local GitLab fixture.</code>
- <code>Ran `PS4=&kunchenguid#39;+ ${LINENO}: &kunchenguid#39; bash -x
tests/fm-forge-mutations.test.sh` to capture end-user CLI JSON and
API-call evidence.</code>
- <code>Verified `git status --short` remained empty and no
`.fm-forge-body.*` transient snapshots remained.</code>
</details>
<details>
<summary>✅ **Document** - passed</summary>
✅ No issues found.
</details>
<details>
<summary>✅ **Lint** - passed</summary>
✅ No issues found.
</details>
<details>
<summary>✅ **Push** - passed</summary>
✅ No issues found.
</details>
Co-authored-by: Ivan Miles Piesh <ivan@ivanpiesh.info>
eyevanovich
added a commit
to eyevanovich/firstmate
that referenced
this pull request
Jul 23, 2026
## Intent Autofix the remaining live GitLab merge-request creation verification bug exposed by KissCut issue 146. Identify and handle the exact GitLab API response semantics: create/update requests use remove_source_branch, while read responses distinguish the MR-specific should_remove_source_branch intent from force_remove_source_branch project policy and remove_source_branch_after_merge project defaults. Accept successful create or idempotent reuse only when title, exact description intent (allowing only null/empty normalization), draft state, source, target, head SHA, authenticated authorship, and requested source-branch deletion semantics truly match. Preserve ambiguous-success no-retry safety and never create a duplicate; conflicting existing state must refuse unless an explicitly safe guarded convergence path exists. Diagnostics must disclose only mismatched field names, never credentials or full untrusted MR bodies. Keep all fixes focused on GitLab MR create/reuse response semantics and field-only diagnostics, and add a live-shape regression fixture. The captain explicitly authorized per-task AUTOFIX via --yes for ordinary findings, but not destructive, irreversible, credential, or security-sensitive choices. ## What Changed - Verify GitLab merge-request creation and reuse against MR-specific source-branch removal intent, exact description semantics, draft state, head SHA, and authenticated authorship. - Fail closed on missing or conflicting response fields without retrying creation, and report only mismatched field names without exposing response data or usernames. - Expand GitLab live-response fixtures and regression coverage for project policy/default distinctions, malformed descriptions, authorship mismatches, and duplicate-prevention behavior. ## Risk Assessment⚠️ Medium: Captain, the required verification behavior is now correct, but the unused project-default parser adds avoidable coupling and failure surface that is safe to remove mechanically. ## Testing The previously successful full baseline was supplemented by the focused GitLab forge suite and an end-to-end fake-API CLI run; exact live-shape reuse avoided duplicate creation, conflicting MR deletion intent refused with field-only diagnostics, and new draft creation preserved its body and deletion request, with a clean worktree afterward. <details> <summary>Evidence: GitLab MR create/reuse end-to-end transcript</summary> ```text LIVE-SHAPE IDEMPOTENT REUSE (force=false, should=true, project-default=false) /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY6J004VW1DFDW8RPC50CCPR/bin/fm-forge.sh mr-create <test-worktree>/repo --title 'Ship fix' --source fm/fix --remove-source-branch out='{"forge":"gitlab","host":"gitlab.com","project":"kisscut-museum/kisscut-platform","mr":{"iid":5,"title":"Ship fix","state":"opened","url":"https://gitlab.com/kisscut-museum/kisscut-platform/-/merge_requests/5","source_branch":"fm/fix","target_branch":"main","draft":false,"merge_status":"can_be_merged","detailed_merge_status":"mergeable","sha":"e33d09c7d53e53dc466e7169c616c8c184001cf4","merge_commit_sha":null,"labels":["backend"],"author":"mate","assignees":[],"pipeline":{"id":9,"status":"success","sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","url":"https://gitlab.com/p/9"},"already":true}}' assert_no_grep '--method POST' <test-worktree>/glab.log 'live-shape reuse created another merge request' CONFLICTING MR-SPECIFIC DELETION INTENT run_adapter mr-create <test-worktree>/repo --title 'Ship fix' --source fm/fix --remove-source-branch error: matching merge request does not match requested head and metadata mismatch (should_remove_source_branch) assert_no_grep '--method POST' <test-worktree>/glab.log 'conflicting remove-source retry created another merge request' NEW DRAFT MR CREATION WITH EXACT BODY AND DELETION REQUEST /Users/ipiesh/.no-mistakes/worktrees/2814d4e34fac/01KY6J004VW1DFDW8RPC50CCPR/bin/fm-forge.sh mr-create <test-worktree>/repo --title 'Detailed fix' --source fm/fix --body-file <test-worktree>/repo/mr-create-body.md --draft --remove-source-branch out='{"forge":"gitlab","host":"gitlab.com","project":"kisscut-museum/kisscut-platform","mr":{"iid":5,"title":"Draft: Detailed fix","state":"opened","url":"https://gitlab.com/kisscut-museum/kisscut-platform/-/merge_requests/5","source_branch":"fm/fix","target_branch":"main","draft":true,"merge_status":"can_be_merged","detailed_merge_status":"mergeable","sha":"e33d09c7d53e53dc466e7169c616c8c184001cf4","merge_commit_sha":null,"labels":["backend"],"author":"mate","assignees":[],"pipeline":{"id":9,"status":"success","sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","url":"https://gitlab.com/p/9"},"already":false}}' assert_grep '"description":"Requested body\n\n"' <test-worktree>/glab.log 'merge-request POST did not preserve trailing body newlines' ``` </details> ## Pipeline Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes) <details> <summary>✅ **intent** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Rebase** - passed</summary> ✅ No issues found. </details> <details> <summary>⚠️ **Review** - 1 warning</summary> - 🚨 `bin/fm-forge.sh:672` - The required distinction between MR intent and project policy is violated: when `should_remove_source_branch` is unavailable, this fallback infers MR intent by comparing `force_remove_source_branch` with `remove_source_branch_after_merge`. Those are independent project-policy/default fields, so their inequality cannot prove the MR-specific choice and may falsely accept conflicting state. Require a boolean `should_remove_source_branch`; otherwise fail closed. - 🚨 `bin/fm-forge.sh:680` - The required “exact description intent (allowing only null/empty normalization)” is not enforced. jq&kunchenguid#39;s `//` also converts boolean `false` and a missing member to `&kunchenguid#34;&kunchenguid#34;`, allowing a malformed response to match an empty requested description. Require `description` to be a string or explicit null, and normalize only null. - 🚨 `bin/fm-forge.sh:1350` - The forbidden diagnostic behavior remains: authorship mismatch messages interpolate the authenticated username instead of disclosing only mismatched field names. Report fields such as `author.id,author.username` without including their values; the same issue also affects created-MR verification at line 1382. 🔧 Fix: Harden GitLab merge-request response verification 1 warning still open: -⚠️ `bin/fm-forge.sh:167` - `FM_GITLAB_PROJECT_REMOVE_SOURCE_DEFAULT` has no remaining consumer after verification switched exclusively to `should_remove_source_branch`. Parsing it adds dead state and can make every `load_trusted_project` caller fail on an irrelevant malformed project-default field. Remove this parsing block and the global; the fixture can retain the field as part of the live response shape. </details> <details> <summary>✅ **Test** - passed</summary> ✅ 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"` - <code>Baseline (already successful): `command -v tmux >/dev/null || { echo &kunchenguid#34;tmux is required for e2e tests&kunchenguid#34; >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo &kunchenguid#34;== $t ==&kunchenguid#34;; bash &kunchenguid#34;$t&kunchenguid#34; || rc=1; done; exit &kunchenguid#34;$rc&kunchenguid#34;`</code> - <code>Focused regression suite: `bash tests/fm-forge.test.sh`</code> - <code>Evidence run: `PS4=&kunchenguid#39;+${LINENO}: &kunchenguid#39; bash -x tests/fm-forge.test.sh`</code> - `Reviewed the evidence transcript for live-shape idempotent reuse, conflicting deletion-intent refusal without POST retry, field-only diagnostics, and successful draft MR creation preserving the exact body and deletion request` - <code>Verified the worktree remained clean with `git status --short`</code> </details> <details> <summary>✅ **Document** - passed</summary> ✅ No issues found. </details> <details> <summary>🔧 **Lint** - 1 issue found → auto-fixed ✅</summary> -⚠️ linter found issues (exit code 1) 🔧 Fix: Remove unused GitLab project deletion default ✅ Re-checked - no issues remain. </details> <details> <summary>✅ **Push** - passed</summary> ✅ No issues found. </details> --------- Co-authored-by: Ivan Miles Piesh <ivan@ivanpiesh.info>
eyevanovich
added a commit
to eyevanovich/firstmate
that referenced
this pull request
Jul 28, 2026
## Intent Port the five captain-approved upstream Firstmate safety fixes without importing unrelated upstream work: require two identical non-primary path reads before recording a spawned isolated copy; use Linux proc starttime plus complete command-line bytes for watcher PID identity while preserving the non-Linux locale-stable fallback; give PR-description opened and edited events independent concurrency identities while preserving the exact required check name, bot exemptions, read-only pull_request security model, and fork workflow; bound repeated notifications for exited workers parked on external waits without hiding live decisions, unknown liveness, away-mode ownership, or secondmate idle behavior; and add strictly bounded validated historical status context only after durable queue consumption and lock release. Preserve the fork's GitLab adapter, signing bridge, no-mistakes observer, local-only delivery, secondmate isolation, away mode, X mode, and all five runtime backends. Retain all fork tests and add counterfactual coverage for transient/stable/timeout/symlink/Orca worktree paths, Linux clock/PID/cmdline/malformed-proc/lock identity, fixed-head PR edits, parked-worker liveness and cadence, and wake annotation crash boundaries, unsafe files, lock release, and caps. ## What Changed - Require two stable worktree-path reads after spawn, and identify Linux watcher processes using proc start time plus complete command-line bytes with a locale-stable fallback elsewhere. - Give PR-body opened and edited events independent concurrency identities, and bound repeated stale notifications for exited workers parked on external waits while preserving live, unknown, away-mode, and secondmate handling. - Add validated, size-capped wake status context only after durable queue consumption and lock release, rejecting unsafe annotation files and covering the new safety boundaries with counterfactual tests. ## Risk Assessment⚠️ Medium: The five requested safety fixes appear correctly implemented and well covered by source tests, but the spawn-path change lacks the repository-required empirical backend verification evidence. ## Testing The supplied full baseline had already passed; five focused integration suites and a direct workflow-contract inspection then demonstrated all authoritative safety behaviors end-to-end, with reviewer-visible transcripts captured and no failures or residual worktree artifacts. <details> <summary>Evidence: Focused safety validation transcript</summary> ```text COMMAND: bash tests/fm-spawn-worktree-settle.test.sh ok - a single transient stale pane_current_path read is not accepted as the worktree ok - an already-settled pane confirms via the existing inter-poll sleep, not an extra full cycle ok - an unsettled pane times out without recording worktree metadata ok - a symlinked project root is compared by physical path before settling ok - Orca remains excluded because it owns worktree creation directly # all fm-spawn-worktree-settle tests passed COMMAND: bash tests/fm-watcher-lock.test.sh ok - simultaneous watcher starts leave exactly one live process ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME ok - Linux process identity ignores wall-clock changes and detects PID reuse, command changes, and malformed proc data ok - watcher lock ownership is bound to Linux starttime and complete command-line bytes ok - killed watcher stale lock is reclaimed ok - live watcher lock with stale heartbeat is actionable ok - guard banner leads when down with pending wakes (re-arm-after-drain) and stays silent when fresh ok - concurrent fm_lock_try_acquire yields exactly one winner ok - dead-pid stale lock is reclaimed by a single acquirer ok - concurrent stale-lock steal yields exactly one winner ok - live steal mutex is not reclaimed ok - live-held lock is not stolen ok - empty mid-acquire lock keeps a minimum grace ok - late original claimant cannot claim a recreated lock ok - paused mid-acquire claimant backs off to active stealer ok - watch restart refuses to signal a reused pid ok - watch restart reports a healthy peer without attaching to it ok - watcher self-evicts when the lock pid no longer names it ok - arm attaches to a live fresh watcher and exits only when that cycle ends ok - arm starts+confirms a fresh watcher on a clean lock and self-heals a dead-pid lock (never healthy off a dead pid) ok - arm cleans child watcher and temp output on HUP ok - arm propagates an immediate watcher wake before confirmation ok - arm attaches to a peer watcher after child stands down and exits when peer dies watcher: lock held by live pid 76413 but heartbeat is stale for 838509312s (>300s); inspect or stop that watcher before re-arming. ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed COMMAND: bash tests/no-mistakes-required-workflow.test.sh ok - fixed-head signed opened, unsigned edited, signed edited yields 0/1/0 ok - body event groups are distinct while head changes remain coalesced ok - run names expose monotonic numbers and immutable IDs ok - fork, permission, check-name, marker, and bot-exemption contracts are preserved COMMAND: bash tests/fm-watch-triage.test.sh ok - signal_reason_is_actionable: benign absorbed, captain verbs and coalesced batches surfaced ok - stale_is_terminal: terminal status surfaces, non-terminal and no-status are benign ok - scan_captain_relevant_statuses lists only captain-relevant statuses ok - classifier primitives: keyed decisions and activity phases, captain relevance, window-to-task, and overrides ok - crew_is_provably_working: only working+run-step/pane is provable; idle/finished/parked/failed/unknown surface ok - status_is_paused: only the leading paused verb matches, and paused is not captain-relevant ok - crew_absorb_class: working/paused/none from one read; crew_is_paused and crew_is_provably_working agree ok - signal_crew_provably_working: benign only when every referenced crew is provably working ok - a no-verb signal whose crew is provably working is absorbed (no exit, no queue, suppressor advanced, beacon present) ok - a bare turn-end whose crew is provably working (busy pane) is absorbed ok - a bare turn-end whose crew is not provably working is surfaced (the swallowed-finish fix) ok - a no-verb working: note whose crew is idle with no running pipeline is surfaced ok - captain-relevant signal is surfaced (queue + exit) and marked surfaced ok - a stale pane sitting on a terminal status is surfaced (queue + exit) ok - a stale terminal-looking status is overridden and absorbed while a run is actively working, then wedge-escalated ok - provably-working non-terminal stale is absorbed on first sight, then wedge-escalated past the threshold ok - consecutive wedge escalations on the same pane accumulate and demand deep inspection at the threshold ok - a pane becoming active again resets the consecutive wedge-escalation counter ok - a not-provably-working non-terminal stale is surfaced immediately (never left to wait out the timer) ok - a declared pause is absorbed on first sight, then re-surfaced as a recheck past the threshold, never wedge-escalated ok - exited declared-pause and captain-held panes use bounded pause cadence while a live decision gate still surfaces once ok - unknown endpoint liveness remains visible instead of using dead-agent pause suppression ok - a declared paused secondmate re-surfaces on the bounded normal-mode cadence ok - a non-paused secondmate retains normal stale suppression ok - a resumed secondmate clears pause and stale tracking before stale exemption ok - unchanged stale hashes reclassify when a crew enters or leaves pause ok - a declared pause is periodically rechecked against authoritative active-run state ok - a paused status overridden by authoritative working preserves its wedge timer and escalates ok - matching non-terminal stale suppressors repair missing or corrupt stale-since timers ok - triage log capping handles wc byte counts with leading spaces ok - a heartbeat with no captain-relevant change is absorbed and backs off the cadence ok - heartbeat backstop fail-safe surfaces a captain-relevant status the per-wake path missed ok - the liveness beacon stays fresh while the watcher absorbs benign wakes (fm-guard never false-alarms) ok - with .afk present the watcher reverts to one-shot so the daemon owns triage (no double-triage) ok - AFK changed paused panes hand off plain stale identities for daemon-owned pause triage COMMAND: bash tests/fm-wake-queue.test.sh ok - concurrent append plus drain preserves queue records ok - signal written while no watcher runs is caught on next run ok - stale wake is queued before suppressor state is advanced ok - a not-provably-working stale wake is queued before its suppressor is advanced ok - registered custom check output is queued before cadence suppression ok - two atomic drains cannot consume the same records twice ok - drain collapses obvious duplicate heartbeat and signal records ok - drain asserts watcher liveness: warns on a lapse, stays silent right after a fire ok - structural signal enrichment is separate, deduped, home-local, and ignores unsafe status files ok - bounded reads and per-item/global caps fail open with explicit truncation and omission markers ok - slow annotation releases the append lock and a deleted status file fails open ok - interruptions restore before commitment and never replay after raw commitment ``` </details> <details> <summary>Evidence: Actual PR workflow security and concurrency contract</summary> ```text Actual deployed workflow contract (.github/workflows/no-mistakes-required.yml) name: Require no-mistakes run-name: "PR #${{ github.event.pull_request.number }} body compliance - ${{ github.event.action }} - event ${{ github.run_number }} (run ${{ github.run_id }})" on: pull_request: types: [opened, edited, synchronize, reopened] branches: - main permissions: contents: read # GitHub concurrency groups retain at most one pending run, replacing older # pending runs even when cancel-in-progress is false. Give body-bearing events # an immutable per-event group so first-time-fork approvals can never collapse # opened/edited checks. Keep synchronize/reopened coalescing as before. concurrency: group: no-mistakes-required-${{ github.event.pull_request.number }}-${{ (github.event.action == 'opened' || github.event.action == 'edited') && github.run_id || 'head-change' }} cancel-in-progress: true jobs: check: name: PR must be raised via no-mistakes runs-on: ubuntu-latest if: >- github.event.pull_request.user.login != 'github-actions[bot]' && github.event.pull_request.user.login != 'dependabot[bot]' steps: - name: Verify no-mistakes signature in PR body env: PR_BODY: ${{ github.event.pull_request.body }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} PR_NUMBER: ${{ github.event.pull_request.number }} run: | set -eu marker='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' if printf '%s' "${PR_BODY:-}" | grep -qF -- "$marker"; then echo "Found no-mistakes signature in PR #${PR_NUMBER} body." exit 0 fi { echo "::error::This PR was not raised through no-mistakes." echo echo "Contributions to this repository must be submitted via 'git push no-mistakes'." echo "That pipeline runs the required review/test/lint/CI steps and writes a" echo "deterministic '## Pipeline' section into the PR body containing:" echo echo " $marker" echo echo "See CONTRIBUTING.md for setup and the full workflow." echo echo "PR author: ${PR_AUTHOR}" } >&2 exit 1 ``` </details> ## Pipeline Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes) <details> <summary>✅ **intent** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Rebase** - passed</summary> ✅ No issues found. </details> <details> <summary>⚠️ **Review** - 1 warning</summary> -⚠️ `bin/fm-spawn.sh:858` - The new two-read settling logic changes launch behavior shared by tmux, herdr, zellij, and cmux, but the range adds only stubbed counterfactual tests and no real-adapter verification notes. CONTRIBUTING.md requires empirical verification for `fm-spawn.sh` launch mechanics and corresponding backend evidence. Confirm/document that verification before merging, or explicitly approve this backend-neutral change as exempt. </details> <details> <summary>✅ **Test** - passed</summary> ✅ 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"` - <code>Confirmed the supplied baseline had already passed: `command -v tmux >/dev/null || { echo &kunchenguid#34;tmux is required for e2e tests&kunchenguid#34; >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo &kunchenguid#34;== $t ==&kunchenguid#34;; bash &kunchenguid#34;$t&kunchenguid#34; || rc=1; done; exit &kunchenguid#34;$rc&kunchenguid#34;`</code> - <code>Inspected the target diff with `git diff --stat b75c898..b783156` and mapped changed paths to the authoritative intent</code> - <code>`bash tests/fm-spawn-worktree-settle.test.sh` — transient/stable/timeout/symlink/Orca worktree behavior</code> - <code>`bash tests/fm-watcher-lock.test.sh` — Linux starttime, full cmdline bytes, malformed proc data, locale fallback, PID reuse, and lock identity</code> - <code>`bash tests/no-mistakes-required-workflow.test.sh` — opened/edited concurrency identities, fixed-head edits, exact check name, bot exemptions, permissions, and fork behavior</code> - <code>`bash tests/fm-watch-triage.test.sh` — exited parked-worker cadence, live decisions, unknown liveness, away-mode ownership, and secondmate behavior</code> - <code>`bash tests/fm-wake-queue.test.sh` — post-consumption annotation, unsafe files, lock release, crash boundaries, and bounded caps</code> - <code>Captured the deployed workflow contract with `sed -n &kunchenguid#39;1,80p&kunchenguid#39; .github/workflows/no-mistakes-required.yml`</code> - <code>Confirmed cleanup with `git status --short`</code> </details> <details> <summary>✅ **Document** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Lint** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Push** - passed</summary> ✅ No issues found. </details> --------- Co-authored-by: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Co-authored-by: Freudator86 <94322668+Freudator86@users.noreply.github.com> Co-authored-by: Freudator86 <tim@allesknut.de> Co-authored-by: vvizlan <steve.rule.wilson@gmail.com> Co-authored-by: Ivan Miles Piesh <ivan@ivanpiesh.info>
ivannovak
added a commit
to ivannovak/firstmate
that referenced
this pull request
Aug 3, 2026
Work-in-progress checkpoint of bin/fm-fleet-board.sh and its colocated test. Two known defects remain: the candidate_prs heading renders a bare #number instead of a full URL, and the bare-#number assertion also matches &kunchenguid#39; HTML entities so it can go red for the wrong reason.
ivannovak
added a commit
to ivannovak/firstmate
that referenced
this pull request
Aug 3, 2026
The open-pull-request rows identified a PR as "<repo> #<num>", which is exactly the bare shorthand the board forbids everywhere else. Identify the row by its repo and the task it belongs to, and let the full https URL below it be the only reference to the pull request itself. The colocated bare-reference scan could not have caught this reliably: jq's @html escapes apostrophes in fleet text to &kunchenguid#39;, so the scan matched every escaped apostrophe and went red for a reason unrelated to pull requests. Strip character references before scanning, and check the scan against a positive control before trusting it. Also replace the inline-decision assertions, which read the artifact's embedded JavaScript as text, with a probe that RUNS that script over a minimal DOM and inspects the payload the board actually hands to Lavish. A negative control tampers with the rendered artifact and requires the probe to go red, so the green case is backed by a proven instrument.
ivannovak
added a commit
to ivannovak/firstmate
that referenced
this pull request
Aug 3, 2026
Work-in-progress checkpoint of bin/fm-fleet-board.sh and its colocated test. Two known defects remain: the candidate_prs heading renders a bare #number instead of a full URL, and the bare-#number assertion also matches &kunchenguid#39; HTML entities so it can go red for the wrong reason.
ivannovak
added a commit
to ivannovak/firstmate
that referenced
this pull request
Aug 3, 2026
The open-pull-request rows identified a PR as "<repo> #<num>", which is exactly the bare shorthand the board forbids everywhere else. Identify the row by its repo and the task it belongs to, and let the full https URL below it be the only reference to the pull request itself. The colocated bare-reference scan could not have caught this reliably: jq's @html escapes apostrophes in fleet text to &kunchenguid#39;, so the scan matched every escaped apostrophe and went red for a reason unrelated to pull requests. Strip character references before scanning, and check the scan against a positive control before trusting it. Also replace the inline-decision assertions, which read the artifact's embedded JavaScript as text, with a probe that RUNS that script over a minimal DOM and inspects the payload the board actually hands to Lavish. A negative control tampers with the rendered artifact and requires the probe to go red, so the green case is backed by a proven instrument.
This was referenced Aug 7, 2026
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
Build a recurring, automated review sweep for firstmate (cron, every 8h) that
reviews every open fleet PR with review-rectify-pi --push (review-only), excluding
drafts and already-approved PRs, keeping failing-CI PRs (with a ## CI Failure
investigation), bounded to 3 concurrent, with a clean-APPROVE -> Jira In-Review
tie-in. Includes bin/fm-review-sweep.sh, the headless fm-spawn.sh launch fix
(stable worktree detection + post-launch verify + --approve), AGENTS.md/README.md
docs, and behavior tests. This re-run pushes via the meisbokai/firstmate fork
(contributor has no push access to kunchenguid/firstmate; CONTRIBUTING.md fork
workflow). Branch HEAD already passed review/test/lint in the prior run; only the
push failed on permissions, now resolved by the fork.
What Changed
bin/fm-review-sweep.sh, a cron-driven sweep that enumerates every open fleet PR, drops drafts and already-approved PRs (reviewDecision=APPROVED), keeps failing-CI PRs (adding a CI-failure investigation to the review), and dispatchesreview-rectify-piin review-only--pushmode bounded toFM_SWEEP_CONCURRENCY(3); on a clean APPROVE it moves the PR's linkedMILE-\d+Jira ticket to "In Review".bin/fm-spawn.sh: adds--approveto the pi launch template, replaces the naive cwd check with treehouse-path + interactive-shell + two-poll stability detection (defeats the$HOMEpath transient and thetreehouse getforeground race), and adds a post-launch verify that resends once if the harness binary isn't seen within 40s.tests/fm-review-sweep.test.shand documents the sweep, its state files, and the headless-launch reliability inAGENTS.mdandREADME.md.Risk Assessment
✅ Low: This is a focused fix-up commit that correctly resolves all 5 prior findings with minimal, targeted changes consistent with existing firstmate conventions, and introduces no new material risks.
Testing
Exercised the review sweep's real cron code path (enumerate→filter→CI-classify→plan→brief→Jira→flock) against a stubbed-gh fleet via the real main() with redirected paths, and verified the headless fm-spawn.sh detection/verify logic against scripted tmux sequences that replay the documented race conditions. All 25 sweep e2e checks, all 9 spawn-logic checks, and the project's 12 behavior tests pass; the dry-run plan, sweep.log, and both briefs are captured as artifacts showing the intended review-only + failing-CI-kept + clean-APPROVE→Jira behavior. One coverage gap (not a defect): a fully live headless spawn (real treehouse pool + real pi/claude binary) was not run because warming the pool and launching real harnesses would write worktrees/processes outside the worktree boundary and risk interfering with the no-mistakes gate's own treehouse machinery; the spawn fix's per-step logic is instead proven via verbatim-copied predicates driven by scripted pane outputs.
Evidence: Sweep --dry-run plan (what cron prints: candidates with CI flags; draft #2 & approved #3 filtered)
==== DRY RUN PLAN ==== concurrency=3 task_timeout=1800s repo #num cifail title veridianlab/racgoon #1 pass https://github.com/veridianlab/racgoon/pull/1 veridianlab/racgoon #4 fail https://github.com/veridianlab/racgoon/pull/4 veridianlab/infra #10 pass https://github.com/veridianlab/infra/pull/10 ==== END DRY RUN (dispatched nothing) ====Evidence: state/sweep.log cron run log (filter reasons, plan count, Jira moves + no-key composition)
==== sweep start 2026-06-22T12:49:43Z (dry-run=1, concurrency=3) ==== filter veridianlab/racgoon#2 draft filter veridianlab/racgoon#3 approved plan 3 candidate PR(s) across 2 repo(s) (after draft+approved filters) ==== sweep end (dry-run, 3 candidates) ==== jira veridianlab/racgoon#1 APPROVE -> moved MILE-1001 to In Review jira veridianlab/racgoon#99 APPROVE but no MILE- key in title/body; skipping transitionEvidence: Red-PR review brief: dedicated ## CI failure root-cause investigation section (gh pr checks / gh run view --log-failed)
Evidence: Green-PR review brief: REVIEW-ONLY contract (review-rectify-pi --push; never edit/commit/push/merge)
Evidence: --one REPO run: sweep restricted to a single repo
sweep: enumerating fleet PRs... repo veridianlab/infra (projects/racgoon-infra) ==== DRY RUN PLAN ==== repo #num cifail title veridianlab/infra #10 pass https://github.com/veridianlab/infra/pull/10 ==== END DRY RUN (dispatched nothing) ====Evidence: Sweep e2e results (25/25 checks pass)
PASS :: candidate #1 (pass CI) kept in plan PASS :: candidate #4 (fail CI) kept in plan PASS :: candidate infra#10 (pending CI -> pass) kept PASS :: draft #2 filtered out PASS :: already-approved #3 filtered out PASS :: FAILING-CI #4 is KEPT, not skipped PASS :: second run refused with the lock message PASS :: green brief states REVIEW-ONLY PASS :: green brief uses review-rectify-pi PASS :: green brief runs --push PASS :: green brief has never-push rule PASS :: no red-only CI-investigation section on a green PR PASS :: red brief has dedicated ## CI Failure section PASS :: red brief mandates investigate root-cause PASS :: red brief instructs reading the failing job log PASS :: red brief instructs listing the checks PASS :: exactly one Jira transition (APPROVE fired, CONDITIONAL suppressed) PASS :: transition targets MILE-1001 -> In Review PASS :: no transition when PR has no MILE key PASS :: no-key case logged (composes with standing rule) PASS :: APPROVE parses to APPROVE PASS :: CONDITIONAL APPROVE parses to CONDITIONAL (not APPROVE) PASS :: BLOCK parses to BLOCK PASS :: --one ran only the named repo PASS :: --one excluded the other repo -- totals: 25 passed, 0 failed -- ALL E2E CHECKS PASSEDEvidence: Headless spawn-logic results (9/9: detection + verify + --approve)
PASS :: transient HOME is skipped, worktree captured after it settles PASS :: captured path is the real treehouse worktree (not HOME) PASS :: waits past the treehouse-foreground race, then captures the subshell PASS :: no launch keystrokes sent during the foreground race PASS :: empty WT when only HOME is ever reported (would trigger the error+exit) PASS :: harness detected once pane_current_command flips to the harness binary PASS :: first 40 polls at a shell do NOT falsely verify (resend-once path engages) PASS :: pi launch template passes --approve (clears project-trust gate for headless dispatch) PASS :: brief still passed as the single positional argumentEvidence: End-to-end sweep harness script (reproducible: stubs gh/jira, runs real main())
Evidence: Spawn-logic harness script (verbatim predicates driven by scripted tmux)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 5 issues found → auto-fixed ✅
bin/fm-review-sweep.sh:301---onewithout a following repo argument hangs forever.ONLY_REPO="${2:-}"tolerates the missing arg, butshift 2fails when only one arg remains (returns non-zero, does not shift) andset -eis deliberately off, so$1stays--oneand thewhile [ $# -gt 0 ]loop re-matches it indefinitely. Verified:fm-review-sweep.sh --oneloops without progress. Fix: guard withshift 2 || { echo 'error: --one requires a repo argument' >&2; exit 2; }(or check$# -ge 2).bin/fm-review-sweep.sh:24- The header doc (line 24) promises an overrun review is 'abandoned (window left for inspection)', but on timeouttry_reap(line ~411) callsteardown_task, which sends/quit,kill-window(line 287), andtreehouse prune(line 289). The window and worktree are destroyed, so nothing is left for inspection — defeating post-mortem debugging of an unattended cron run. Either the doc or the behavior is wrong; preserving the pane/worktree on timeout (or at least skipping the kill) better matches the stated intent.bin/fm-review-sweep.sh:221- Whenfm-spawn.shreturns non-zero, the failure cleanup onlyrm -fs the meta/status andrm -rfs data (rm -f "$FM_ROOT/state/$id.meta" "$status_file"; rm -rf "$FM_ROOT/data/$id"), but fm-spawn may already have created the tmux windowfm-$id(it creates the window before the worktree-detection and post-launch-verify waits that can fail). The window is never killed, so it leaks. Over repeated sweep runs these accumulate. Addtmux kill-window -t "fm-$id" 2>/dev/null || trueto the failure path.bin/fm-review-sweep.sh:289-teardown_taskcallstreehouse prune --yes, a GLOBAL prune of all orphaned worktrees, whereas the establishedbin/fm-teardown.shreturns the specific worktree withtreehouse return --force "$WT"(readingworktree=from meta). The sweep never reads its task's worktree path, so it falls back to the imprecise global prune.pruneis safe if treehouse only targets truly-orphaned worktrees, but running it from cron while a live firstmate session has active crewmates relies on treehouse correctly distinguishing active vs orphaned worktrees. Returning the specific worktree (like fm-teardown) would be deterministic and consistent.bin/fm-review-sweep.sh:356- The dry-run headerprintf 'repo\t#cifail\ttitle\turl\n'does not match the data rowsprintf '%s\t#%s\t%s\t%s\n' "$repo" "$num" "$cifail" "$title". The header shows a nonsensical#cifailcolumn and listsurl, while the data printsrepo,#<num>,<cifail>,<title>— so the columns are mislabeled andurlis promised but never printed. The header should beprintf 'repo\t#num\tcifail\ttitle\n'.🔧 Fix: fix sweep --one loop, timeout inspection, spawn-fail window leak, precise worktree return, dry-run header
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-review-sweep.test.sh(project's own behavior tests: resolve_fleet, ci_status, parse_recommendation clean-vs-CONDITIONAL, jira_key_for — 12/12 pass)e2e_sweep.sh: sourced the real main() and ranmain --dry-runagainst a stubbed gh fleet of 5 PRs across 2 repos — verified draft #2 and approved #3 are filtered, candidates #1/#4/#10 kept, failing-CI #4 KEPT, and the dry-run plan + sweep.log show ite2e_sweep.sh§4: held the flock from a background fixture then ranmain --dry-runagain — verified graceful refusal with exit 0 and the lock messagee2e_sweep.sh§5-6: calledwrite_brieffor a green PR and a red PR — verified the review-only contract (review-rectify-pi/--push/never-push) and that only the red PR brief contains the dedicated## CI failure (this PR's CI is RED)root-cause investigation section (gh pr checks / gh run view --log-failed)e2e_sweep.sh§7: calledmaybe_transition_jirawith APPROVE then CONDITIONAL against a stubbed jira CLI — verified exactly oneissue move MILE-1001 In Review(CONDITIONAL suppressed), plus the no-MILE-key suppression+log pathe2e_sweep.sh§8: calledparse_recommendationfor APPROVE/CONDITIONAL APPROVE/BLOCK/REQUEST CHANGES/CAUTION — verified clean APPROVE→APPROVE and CONDITIONAL APPROVE→CONDITIONALe2e_sweep.sh§9: ranmain --one veridianlab/infra --dry-run— verified it ran only the named repo and excluded the othere2e_spawn_logic.sh: drove verbatim-copied is_treehouse_path/is_interactive_shell + the worktree-detection loop with a scripted tmux replaying the documented ~100ms $HOME path transient (skipped), the treehouse foreground race (waits, no keystrokes sent), and a never-settling pane (empty WT→error path)e2e_spawn_logic.sh§D-E: drove the verbatim post-launch verify loop — harness detected when pane_current_command flips to the harness binary; 40 polls at a shell do NOT falsely verify (engages the resend-once path)e2e_spawn_logic.sh§F: extracted the live pi launch_template from bin/fm-spawn.sh — verified it carries--approveand still passes the brief as a single positional argbash -n bin/fm-review-sweep.sh bin/fm-spawn.sh tests/fm-review-sweep.test.sh(syntax)Worktree cleanliness verified before/after (git status clean; removed transient state/sweep.log + state/sweep created by an early subprocess run)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.