Repository navigation
self-merge wake re-presented on a later drain after the merging turn already reported it #1
Description
Summary
Investigated as candidate "shape 5" for kunchenguid#3285 (fewer no-op model turns for the Claude primary). The observed symptom - a check: merge landed: <id> <url> wake surfacing again after the same turn already reported the merge (seen tonight for one PR in another project and two fork PRs) - is not a duplicate-detection bug in bin/fm-merge-outcome-lib.sh/bin/fm-watch.sh's merge-poll dedup, which already works correctly. It is the ONE intended durable wake from a main home's own self-merge surviving, unacknowledged, into a later wake-drain, where it gets presented again as if new even though the merging turn already told the captain the outcome directly.
Evidence this is not a duplicate-detection bug
fm_merge_outcome_report (bin/fm-merge-outcome-lib.sh) is idempotent per PR identity via the pr-poll-merge-notified marker (bin/fm-pr-lib.sh): a second call with the same task/provider/host/path/number returns FM_MERGE_OUTCOME_ALREADY_RECORDED=true and queues nothing. bin/fm-watch.sh's poll loop (is_pr_poll=1 && out=merged branch) checks that flag and continues past the generic wake-append when it is true, after retiring the poll.
Verified two ways, unmodified code:
- A direct unit-level repro calling
fm_merge_outcome_reporttwice (origin=self, then origin=poll) for the same PR identity in a main home (no parent-channel destination): the wake queue holds exactly one row for that PR both before and after the second call. - The full existing
tests/fm-pr-check-security.test.shsuite (56 tests, unmodified) passes, includingtest_self_merge_and_poll_publish_one_outcome, which exercises exactly this self-then-poll interleaving (in a secondmate home).
I also tried gating the local wake to origin = poll only (dropping the || [ -z "$destination" ] clause that lets a main-home self-merge queue its own local wake) - this is the fix that would make "no wake row survives a self-merge" literally true, but it broke tests/fm-pr-merge.test.sh's test_main_home_merge_leaves_a_durable_wake, which explicitly requires a main home's own self-merge to leave exactly one durable wake naming the PR. That test's rationale (inferred, not stated in this pass): crash-durability - if the session dies between the merge call and reporting to the captain in chat, the wake queue is the only trace that survives. Reverted; no production code changed in this pass.
The actual mechanism (best-supported hypothesis, not fixed here)
bin/fm-pr-merge.sh is only ever invoked by the main session itself, synchronously (bin/fm-lease-lib.sh refuses a branch actor's attempt to call it), so a self-merge is always known to its caller in the same turn - AGENTS.md section 7 already has that caller report the outcome to the captain directly ("After an autonomous merge, give the captain a one-line full-URL or local-main outcome"). But fm_merge_outcome_report's wake-queue row for that self-merge is never acknowledged/dequeued by that same turn. It sits in .wake-queue until some later drain reads it - which could be the very next turn - and gets presented again as a check: wake, which the supervision protocol (AGENTS.md section 8, item 3) always escalates, forcing a turn that has nothing new to do.
Candidate designs (neither implemented)
- Ack-at-report:
bin/fm-pr-merge.shdequeues its own just-queued wake row immediately after a successful self-merge, on the theory that the calling session's direct chat report (mandated by AGENTS.md section 7) is the durable-enough record, backed by thepr-poll-merge-notifiedmarker and the task's own meta/status. Simplest, but removes the crash-recovery tracetest_main_home_merge_leaves_a_durable_wakecurrently locks in; that test's expectation would need to change too, and whether the marker + task status are actually sufficient crash-recovery replacements needs a real answer, not an assumption (does anything durable point back to "this task's PR merged" without the wake row, for a session that crashed before reporting?). - Durable "reported" marker the drain reads: leave the wake queued (keeping the crash-durability trace as-is), but have the same turn that performs the self-merge also write a marker (or reuse
notice_emitted/a new field on the terminal-outcome-style record) that a later wake-drain checks before re-presenting amerged-<id>-<url>row it finds still queued - i.e., "this exact row was already delivered synchronously in the turn that created it; drain it silently instead of escalating." Keeps the safety net for the crash case (nothing to acknowledge => still surfaces) at the cost of a second bookkeeping mechanism the drain path has to consult.
Repro scripts
Both ad hoc scripts used for the evidence above (not part of the tracked test suite) are described here for whoever picks this up; happy to hand over the exact scripts on request rather than pasting throwaway paths from a session scratch directory.
Not in scope for this issue
- No code changes are proposed here.
- The two already-shipped fixes for Claude primary: routine wakes force a visible "Captain, shipshape." turn each - ~20 no-op model turns/hour on a busy fleet kunchenguid/firstmate#3285 (fork PR merging a declared pause outranking a failed run-step underneath it, and the composed inactive-outcome regression covering the same scenario through
fm-inactive-reconcile.sh) are unaffected and unrelated to this finding.
Activity