Skip to content

Bug: Convoy landing merge never triggers — updateConvoyProgress not called after MR closes #962

@jrf0110

Description

@jrf0110

Parent

Part of #204 (Phase 3: Multi-Rig + Scaling)

Problem

Convoy landing merges (feature branch → main) are never triggered for review-then-land convoys. The ready_to_land flag is never set, so processConvoyLandings() in the alarm loop has nothing to process. Similarly, review-and-merge convoys never auto-close after the last bead's MR completes.

This affects all convoys. The final landing merge must be triggered manually or never happens.

Root Cause

Two interlocking issues in the bead/review lifecycle prevent updateConvoyProgress() from re-evaluating after MR beads close:

1. completeReview() closes MR beads via raw SQL, bypassing updateBeadStatus()

File: src/dos/town/review-queue.ts:220-238

completeReview() runs a raw UPDATE beads SET status = 'closed' query. This does not call updateBeadStatus(), which means updateConvoyProgress() is never triggered when the MR bead closes.

2. closeBead() on the already-closed source bead is a no-op

File: src/dos/town/beads.ts:187

completeReviewWithResult() at review-queue.ts:276 calls closeBead(entry.bead_id) on the source bead. But the polecat already closed this bead when it called gt_done. The guard if (bead.status === status) return bead at beads.ts:187 prevents re-entry, so updateConvoyProgress() is never called.

The sequence

Step What happens Convoy progress effect
Polecat calls gt_done Source bead closed → updateConvoyProgress fires MR bead is still open → NOT EXISTS guard excludes this bead from closedCount → convoy not ready
Refinery finishes review completeReview() closes MR bead via raw SQL (no updateBeadStatus) No recount. updateConvoyProgress never fires.
completeReviewWithResult() calls closeBead() on source bead Source bead already closed → no-op at line 187 No recount. updateConvoyProgress never fires.
Alarm runs processConvoyLandings() Queries for ready_to_land = 1 Finds nothing. The flag was never set.

The NOT EXISTS guard at beads.ts:274-282 is correct — it prevents premature ready_to_land while reviews are in flight. But there's no mechanism to re-evaluate convoy progress after the MR bead resolves.

Fix

After completeReviewWithResult() closes the MR bead and the source bead, explicitly re-trigger updateConvoyProgress() for the source bead. This requires one of:

Option A (simplest): In completeReviewWithResult(), after the existing closeBead() call, call updateConvoyProgress(sql, sourceBeadId, timestamp) directly. The no-op guard on closeBead doesn't matter — we're calling the convoy recount explicitly. The MR bead is now closed, so the NOT EXISTS guard passes, and the bead counts toward closedCount.

Option B: Change completeReview() to use updateBeadStatus() instead of raw SQL for closing MR beads. This makes MR bead closures trigger convoy progress automatically. More correct long-term but higher blast radius.

Option C: Make processConvoyLandings() self-healing — instead of relying on the ready_to_land flag, query for convoys where all tracked beads are closed with no pending MR beads. Adds a query per alarm tick but makes the system resilient to missed recounts.

Option A is the lowest-risk fix.

Acceptance Criteria

  • review-then-land convoys set ready_to_land after the last MR bead closes
  • processConvoyLandings() creates a landing MR bead (feature branch → main) and dispatches the refinery
  • Refinery receives the "Final Landing" convoy context in its system prompt
  • review-and-merge convoys auto-close after the last MR bead closes
  • No regression: premature ready_to_land is still prevented while MRs are in flight

Notes

  • No data migration needed — cloud Gastown hasn't deployed to production
  • processConvoyLandings() itself is robust — dedup guard, error handling with retry, correct target branch resolution. The function works; it just never has anything to process.
  • There's a secondary issue: when processConvoyLandings() creates the landing MR, getConvoyForBead(sql, convoyId) returns null because the dependency direction is reversed (source beads track the convoy, not the other way around). This means the refinery gets no convoyContext for the final landing — the "Final Landing" prompt section is never included. This should be fixed as part of this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingkilo-auto-fixAuto-generated label by Kilokilo-triagedAuto-generated label by Kilo

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions