Skip to content

fix(bin): chain teardown and backlog completion after a PR merge - #2854

Open
pramendra wants to merge 3 commits into
kunchenguid:mainfrom
pramendra:fm/fm-pr-merge-atomic-teardown
Open

fix(bin): chain teardown and backlog completion after a PR merge#2854
pramendra wants to merge 3 commits into
kunchenguid:mainfrom
pramendra:fm/fm-pr-merge-atomic-teardown

Conversation

@pramendra

Copy link
Copy Markdown

Intent

bin/fm-pr-merge.sh should automatically tear down and record done after a successful merge, instead of leaving that to three separately-remembered manual steps (fm-pr-merge.sh, fm-teardown.sh, tasks-axi done). Under load, firstmate dropped the last two steps twice in one session even though the merges were real and green, leaving the worktree leased and the backlog showing the task as in-flight until a manual sweep caught it. This is a process gap (a sequence someone has to remember), not a data-format problem.

Acceptance criteria:

  1. On a successful merge, fm-pr-merge.sh automatically runs the equivalent of bin/fm-teardown.sh and tasks-axi done --pr - one command produces all three effects, or clearly shows why one step didn't (e.g. teardown correctly refusing on unlanded work elsewhere).
  2. Never mask a real failure: if the merge succeeds but teardown legitimately refuses (its own hard-won guard against discarding unlanded work), that refusal must still surface clearly - never silently swallowed, never treated as fully done when it wasn't. Nothing about fm-teardown.sh's own safety logic changes.
  3. Preserve every existing caller of fm-pr-merge.sh working exactly as before for the merge step itself - this is additive automation after a successful merge, not a rewrite of the merge logic.
  4. Provide an explicit opt-out flag for a caller that genuinely needs merge-without-teardown (e.g. testing, or its own reason to defer cleanup), rather than making the new automatic behavior unconditional.
  5. Prove the fix can fail: demonstrate the OLD behavior (a successful merge leaving stale state behind) failing to clean up, then the NEW behavior cleaning up automatically - not just an assertion that it works.
  6. Update the script's own header comment to describe the new atomic behavior, since it previously only described the merge step.

Do not build a bigger control plane, sync layer, or new state-tracking mechanism. The fix is mechanical: chain three already-correct, already-tested operations so a human or firstmate cannot forget the last two.

This is firstmate's own shared tracked material (bin/), so firstmate-coding-guidelines' style rules apply (one sentence per line in prose/comments, plain dash, no agent co-author, shellcheck-clean).

The PR body must state plainly: what the three-step sequence used to require, exactly what is now automatic, and the one thing that still correctly stops automation (teardown's unlanded-work refusal) and why that refusal must never be bypassed.

What Changed

  • bin/fm-pr-merge.sh now chains bin/fm-teardown.sh <task-id> and then tasks-axi done <task-id> --pr <url> automatically after a successful merge, so one command produces all three effects instead of three separately-remembered steps. Previously it stopped after recording PR metadata and merging.
  • Added a --no-teardown flag (accepted anywhere before the optional -- separator) that preserves the old merge-only behavior for callers with their own reason to defer cleanup; existing callers that don't pass it now get the automatic chain.
  • A teardown refusal (its hard-won guard against discarding unlanded work) is reported verbatim and exits non-zero before tasks-axi done ever runs — the merge landing does not bypass or silence that refusal. tasks-axi done itself only runs when a compatible backend is available (via fm-tasks-axi-lib.sh's existing availability check); on a manual backend the operator still gets teardown's own printed reminder, unchanged.
  • Updated the script's header comment, AGENTS.md, docs/architecture.md, and docs/scripts.md to describe the new atomic teardown-and-completion behavior, and updated tests/fm-pr-check-security.test.sh and tests/fm-pr-merge.test.sh to cover the new chaining (including --no-teardown opt-out) and to keep unrelated merge/URL-derivation tests isolated from it.

Note: this branch also includes unrelated .serena/.gitignore and .serena/project.yml additions (editor tooling config), which are not part of this behavioral change.

Risk Assessment

✅ Low: The change is a well-bounded, additive chain (merge → teardown → tasks-axi done) with a correct opt-out flag, proper non-masking propagation of teardown's refusal exit code, reuse of an existing availability gate to avoid double-invoking tasks-axi, and thorough new tests (success, refusal-surfaces, opt-out) plus updated header/doc comments consistent with every acceptance criterion.

Testing

test

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • test
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Your Name added 2 commits August 23, 2026 18:39
fm-pr-merge.sh required three separately-remembered commands to fully
close out a task; the last two were dropped twice in one session while
the merges themselves were real and green. A successful merge now
automatically runs fm-teardown.sh and tasks-axi done, surfacing any
teardown refusal instead of swallowing it, with --no-teardown for
callers that need the old merge-only behavior.
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the position-independent opt-out parser now covers the previously reported ordering case, and unavailable non-manual tasks-axi backends now fail visibly after teardown.

Reviews (2): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile

Comment thread bin/fm-pr-merge.sh Outdated
Comment thread bin/fm-pr-merge.sh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant