Post the connector's own lifecycle messages exactly once, or not at all

[jorgemanrubia]

Stacked on Dispatch admitted mentions to Claude Code workers; do not merge before it.

The connector can dispatch a mention to a worker, but it has no voice of its own. When the worker is slow to start, nobody sees that the request was heard. When a project has no route, the mention gets no answer. When a worker fails, is lost, runs out its deadline or is stopped by a shutdown, no one is told which events need a person. Worse, anything the connector did post would be at risk of posting twice after a crash, because a request can reach Basecamp and the process can die before it learns the message id.

Originally tracked in Lifecycle messages and the outbox.

The worker still acknowledges and replies in its own words. That instruction is the dispatch prompt's first step before any work, and the worker reports it through ack_dispatch. The connector adds only fixed-form messages for moments when no worker can speak. They're built from ledger records: ids, states, stop reasons and times. No content, no model, no free text. A missing message costs less than a doubled one, so every lifecycle message goes through one outbox that never guesses.

The messages

• Guard acknowledgement: a boost on the recording that asked. It arms for a mention or an assignment, including one queued behind a live task on the same conversation — that event still asked for something. Triggers that ask for nothing (subscribed, completed) never arm it. It's written at admission for mentions and assignments and sent if no worker calls get_dispatch within 30 seconds. get_dispatch cancels it in its own transaction. A guard that already went out is reported back to the worker as guard_acknowledged, even when the task was created or the follow-up joined afterwards.
• Holding reply: posted at the reply destination when a mention or assignment is blocked because the project has no route.
• Still-running: one notice per attempt and occurrence, and never sent once its attempt has ended, so the connector's last word on finished work is not "still working on this".
• Completion notice: one per attempt, built from the stop reason and each event's outcome. It's posted when any event failed, is unknown, succeeded without a reply, or could not be started a second time. It names the failed, unknown and unstartable ones with the basecamp connect redispatch command they need; an event that succeeded without reporting a reply is only named, since the work behind it was reported done. An attempt where every event succeeded with a reply gets no notice.

The outbox, and the invariants it holds (each has a test that fails without it, bar one noted below; the DB enforces them where SQL can):

1. An intent is written in the transaction of the transition that calls for it: the admission verdict, the attempt's end, the still-running tick. The two commit or roll back together.
2. There is one intent per thing answered for: guard and holding reply per event, completion per attempt, still-running per attempt and occurrence.
3. Nothing is sent without a durable sending row. Claiming the intent (committed) is the only way to a request.
4. Nothing sending is ever sent again automatically. A failed or unanswered request is reconciled by listing the destination, never by posting. That happens once the request has had time to land, and a failing listing backs off before it gives up as indeterminate. A single flush also refuses to claim one intent twice.
5. Reconciliation adopts a message only when exactly one of the agent's messages at the destination since sending time matches the body, no other intent owns it, and no other unfinished intent there could claim it. Anything else becomes indeterminate, for a person to resolve as sent, abandoned or resend.
6. A receipt belongs to exactly one intent and never changes.
7. States only move along pending → sending | canceled, sending → sent | indeterminate, sending → canceled when Basecamp refused the request (which creates nothing), and indeterminate → sent | abandoned | pending. Those last three, and resending a refused intent, happen only by a person's decision.
8. get_dispatch cancels a pending guard in its own transaction, and a guard that already went out is reported to every worker as acknowledged, including tasks created and follow-ups joined afterwards.
9. A guard is reported fired from the moment it is claimed, and that is final, because Serve a connector worker its dispatch over MCP #736 settles a guard once. A worker that asks while the request is in flight doesn't acknowledge a second time. If Basecamp then refuses the request, the intent is canceled, since nothing was created, but that task's workers still don't acknowledge. The acknowledgement goes missing rather than doubled, which is the spec's own preference. A later task for the event has its guard armed, and its worker acknowledges itself: the intent is canceled, so nothing remains to fire that guard. This is the spec's rule applied: a missing acknowledgement costs less than a double one. A refusal here means Basecamp refusing the connector's own lifecycle request, and it is recorded on the outbox row. A worker's permission refusals are a separate matter, handled in the PR below this one.
10. Reconciliation never holds up sending for long. A running connector sends a batch, then lists at most one due destination. Each listing is time-bounded, and each failure backs off, doubling. After ten failures, recorded as such, the intent becomes indeterminate. A flush never claims the same intent twice.

On start, before intake, admission or dispatch run, the outbox reconciles every sending intent a previous process left, then sends what is due. If the ledger can't read or settle an intent, the connector does not start. A listing that fails backs off and does not block the start. The whole step is time-bounded, and sending stops at the first request that may not have landed, so a slow Basecamp delays the start instead of holding it up; the running outbox carries on from there. One wait is deliberate: an intent that went sending less than a minute before the restart may still be landing. That is the normal case when a supervisor restarts a crashed connector. Listing it at once could only mark it indeterminate for want of patience, so the running outbox reconciles it as soon as it is a minute old.

The adopted-reply rule keeps the connector's own notices out by reading the destination itself: a notice with a receipt is left out by its id, and one still without a receipt by its words at that destination. Nothing else is needed, so the dispatcher gets no id-only predicate beside it.

Posting uses the agent's own SDK client. The SDK makes one attempt at a non-idempotent create, and a test holds that. --shadow installs no hooks and posts nothing, so a promoted shadow ledger carries nothing to send. A future hold marker has a seam to pause sending while reconciliation continues. basecamp connect runs the sender beside the dispatcher and flushes the notices of attempts stopped at shutdown, within a bound.

One invariant has no test that fails without it: the claim's check that the row it moved was still pending. The select and the update share one immediate transaction, so it cannot fire; it is there so a later refactor that separates them cannot quietly send twice.

Evidence

• A kill between sending and the receipt is tested with a real child process and SIGKILL. When the request had reached the server, the restart adopts exactly one message. When it hadn't, the intent becomes indeterminate. In both cases the restart makes no request.
• Every rule above was reverted in turn and its test went red; the list is below.
• Live, as the bot-user agent on the scratch project 48929974, re-run on this head:
  • The worker's own acknowledgement (a boost) landed 7.5 s after the mention, on card 20: worker acknowledgement (4).
  • The holding reply landed 0.9 s after the mention, on card 20: holding reply (4).
  • A SIGTERM with a worker running posted the shutdown completion notice about a second later, on card 20: completion notice on shutdown (4).
  • With a worker slow to start, the guard boost landed 31.1 s after the mention and the worker did not acknowledge again, on card 20: guard acknowledgement.
• Opus adversarial reviews on each head found no path that sends twice, none that sends without a durable sending row, and no guard double-acknowledgement. Two blocking findings came out of them and are fixed: a failing reconciliation listing was retried every tick with no backoff, and the check that keeps the connector's own notices out of the adopted-reply rule applied to the whole ledger instead of the destination. Fourteen smaller findings are fixed too.

Fail-first proofs (80 reverts, each red)

Hook error swallowed; no key conflict handling; claim that does not commit sending; claim that also picks sending; post error returning the intent to pending; a flush claiming twice; stale age ignored; any match adopted; ownership unchecked; rivals unchecked; body ignored in matching; a failed listing settling; receipt index dropped; receipt made mutable; edge trigger off; resolve from any state; resolve without who; get_dispatch cancel trigger off; fired-before-task trigger off; claim not marking fired; guard not delayed; guard for non-requests; guard for a discarded record; hold ignored; succeeded-without-reply silent; blocked silent; notice when all succeeded; returned events named; blocked not read back from records; holding reply for other blocks; tag stripping off; other people's messages listed; a short Campfire listing accepted; restart resending sending (the kill test); a sent notice not recognized by its receipt, and an unreceipted one not recognized by its words; no reconciliation backoff; never giving up; retrying an unlistable destination; reconciling a young sending intent at start; rivals without indeterminate or abandoned; unreceipted notices adoptable; a post ignoring the flush deadline; duplicate Campfire lines; a 404 retried.

[Copilot]

🟡 Changes recommended

Operator recovery is not exposed, advertised remediation is unavailable or inaccurate, and resend reconciliation state is not reset.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Introduces a durable lifecycle-message outbox for the connector, providing guarded posting, reconciliation, and crash recovery.

Changes:

• Adds transactional lifecycle-message creation and resolution.
• Implements Basecamp posting, reconciliation, and exactly-once safeguards.
• Integrates outbox processing with connector startup and shutdown.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File summaries

File	Description
internal/connector/outbox.go	Defines outbox schema, states, queries, and resolution.
internal/connector/outbox_run.go	Implements sending and reconciliation.
internal/connector/outbox_kill_unix_test.go	Tests SIGKILL recovery behavior.
internal/connector/outbox_invariants_test.go	Tests outbox invariants and failure handling.
internal/connector/outbox_fakes_test.go	Provides outbox test fixtures.
internal/connector/outbox_basecamp.go	Implements SDK-backed posting and listing.
internal/connector/outbox_basecamp_test.go	Tests Basecamp poster behavior.
internal/connector/lifecycle.go	Renders lifecycle notices and installs hooks.
internal/connector/lifecycle_test.go	Tests lifecycle templates and rules.
internal/connector/ledger.go	Registers the outbox migration.
internal/commands/connect_run.go	Runs and flushes the outbox with the connector.

Review details

Suppressed comments (2)

internal/connector/lifecycle.go:75

• These notices direct users to basecamp connect redispatch, but NewConnectCmd registers only the setup subcommand (internal/commands/connect.go:28-64) and there is no redispatch implementation in the repository. Failed/unknown events therefore advertise a recovery action users cannot perform. Add and register the command, or do not emit it as remediation.

	redispatch := " Needs a person: basecamp connect redispatch " + id

internal/connector/lifecycle.go:86

• The PR description says every event named by a completion notice—including one that succeeded without a reported reply—includes its redispatch command, but this branch omits it for this case. Once the advertised redispatch command exists, append the shared redispatch suffix here as well.

	case e.Outcome == OutcomeSucceeded && e.ReplyID == nil:
		return "Event " + id + ": succeeded, with no reply reported."

• Files reviewed: 11/11 changed files
• Comments generated: 4
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Reconciliation can be starved, partial database scans can permit incorrect reply adoption, and one completion case contradicts the documented contract.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🟡 Changes recommended

Project scoping is not wired into the dispatcher, and cancellation can interrupt a subsequent Claude turn.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 40/40 changed files
• Comments generated: 2
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🔵 Needs a closer look

Durable state transitions, crash recovery, and non-idempotent external writes warrant final human review despite comprehensive tests.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🟡 Changes recommended

A definitively refused guard post is still reported to workers as acknowledged, preventing their fallback acknowledgement.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🟡 Changes recommended

Guard-refusal concurrency and unbounded reconciliation passes can suppress acknowledgements or significantly delay pending lifecycle messages.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/connector/outbox_run.go:408

• This drains the entire stale backlog while holding o.mu, and each Poster.List may consume the 30-second AdoptionScanTimeout. A burst of uncertain sends can therefore block all pending guard and holding messages for backlog × 30s (hundreds of rows can accumulate during the one-minute reconcile delay), despite each individual listing being bounded. Reconcile a bounded batch or time slice per run loop, as sending already does, so posting and reconciliation cannot starve each other.

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🟡 Changes recommended

Canceled outbox intents can incorrectly suppress legitimate worker replies during adoption.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🔵 Needs a closer look

Startup can emit stale progress notices and suppress hard ledger failures, while reply filtering adds an unbounded redundant query path.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

internal/commands/connect_run.go:381

• A pending still-running intent from a crashed process is flushed here before Dispatcher.Run gets a chance to recover that process's live attempts. Because claimIntent only checks the ledger's attempts.state, it still sees the orphan as live and can post “Still working on this”; immediately afterward dispatcher recovery ends the attempt as lost and emits a completion notice. Reorder startup so old sending intents are reconciled first, dispatcher recovery settles/verifies orphaned attempts next, and only then pending intents are flushed.

internal/connector/outbox_run.go:205

• A hard send-side ledger error is lost if the startup deadline expires concurrently. sendNext deliberately uses WithoutCancel when recording a receipt/refusal, so that write/readback can fail after ctx.Err() becomes non-nil; this condition then returns success and lets the connector start despite an unsettled ledger. Suppress only errors caused by the caller's cancellation, and propagate unrelated errors regardless of the current context state.

	if err := o.flushSome(ctx, 0, true); err != nil && ctx.Err() == nil {
		return fmt.Errorf("connector: send lifecycle messages on start: %w", err)
	}

internal/commands/connect_run.go:310

• LifecycleFilteredReplies has already removed lifecycle messages with one destination-scoped ledger scan, but this predicate performs two additional ledger queries for every remaining reply. Those queries use context.Background(), so a busy ledger can add up to the retry timeout per reply and run past the dispatcher's AdoptionBudget. Do not install the redundant predicate when using LifecycleFilteredReplies, or make filtering return/reuse its receipt set under the caller's context.

			Replies:            connector.LifecycleFilteredReplies{Lister: poster, Ledger: ledger},
			IsLifecycleMessage: outbox.IsLifecycleMessage,
			Lines:              lines, Logger: logger,

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🔵 Needs a closer look

A transition can invalidate a still-running or holding intent after its claim but before its post begins.

Review details

Suppressed comments (1)

internal/connector/outbox_run.go:293

• The eligibility checks in claimIntent are released before this request starts. A dispatcher can commit EndAttempt in that gap (or a redispatch can move a no_route event), after which this call still posts the already-claimed “still working” or holding message. In the successful-attempt case no completion notice follows, so “still working” can become the connector’s final message after work ended. Coordinate attempt/admission transitions with request initiation (or add a durable handshake those transitions honor), and add a test that pauses after claim but before Post.

	receipt, postErr := o.opts.Poster.Post(postCtx, intent.Destination, intent.Body)

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🔵 Needs a closer look

Canceled, never-posted intents can incorrectly suppress legitimate worker replies.

Review details

Suppressed comments (1)

internal/connector/outbox_run.go:846

• This also records bodies from canceled intents as possibly posted. Every canceled path proves no message exists (canceled before claim, or ErrNotPosted after claim), so a legitimate worker reply with the same normalized text is incorrectly filtered out forever. Exclude canceled rows from the body fallback; a refused intent will become eligible again automatically if a person resolves it back to pending.

		rows, err := r.Ledger.db.QueryContext(ctx, `
SELECT receipt_id, body FROM outbox WHERE message_kind = ? AND recording_id = ?`, string(messageKind), recordingID)

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🔵 Needs a closer look

Attempt settlement can race with still-running posts, and redundant lifecycle lookups can exceed adoption and shutdown bounds.

Review details

Suppressed comments (2)

internal/connector/outbox_run.go:407

• This liveness check happens only while the intent is claimed. After the transaction commits sending, the dispatcher can end the attempt before Poster.Post runs, so the connector can post “Still working” after the attempt ended. If every event succeeded with a reply, no completion intent is created, leaving this stale notice as the connector's last word. Coordinate attempt settlement with an in-flight still-running intent, or have settlement enqueue a terminal notice whenever one is already sending.

			switch err := tx.QueryRowContext(ctx, `SELECT state <> 'ended' FROM attempts WHERE id = ?`, in.AttemptID).Scan(&live); {
			case errors.Is(err, sql.ErrNoRows):
				live = false
			case err != nil:
				return fmt.Errorf("connector: outbox claim still-running %d: %w", in.ID, err)

internal/commands/connect_run.go:309

• LifecycleFilteredReplies already removes lifecycle receipts and unreceipted bodies with one destination-scoped query. Passing this callback as well makes AdoptableReply perform up to two extra SQLite queries per remaining reply, and those queries use context.Background(), so a busy ledger or high-volume conversation can overrun AdoptionBudget and delay shutdown. Leave this callback nil when using the filtered lister.

			IsLifecycleMessage: outbox.IsLifecycleMessage,

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[Copilot]

🟡 Changes recommended

Reply filtering can misclassify cross-kind IDs and suppress valid replies using canceled intents.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

internal/connector/outbox_run.go:849

• Canceled intents are included in unreceipted, although every cancellation path proves that no lifecycle message exists (get_dispatch/no-longer-called-for cancels before posting, and ErrNotPosted means Basecamp created nothing). Consequently, a genuine worker reply with the same normalized text is removed from adoption forever. Exclude canceled rows from this lookup; indeterminate/abandoned rows should remain because their messages may exist.

SELECT receipt_id, body FROM outbox WHERE message_kind = ? AND recording_id = ?`, string(messageKind), recordingID)

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

[jorgemanrubia]

🤖 Landed in #748, 6d0079ca — the six of us as one commit. Closing this as superseded, not abandoned: every commit here is in that merge.

Combining them is what found the defects none of us could see alone — a redispatch that blocked a conversation permanently once another branch's trigger landed beside it, a read-only ledger open that dropped the POSIX locks a writer held, doctor rejecting the configurations the same release adds, a recorded process losing its kernel start time so status called every live worker unverified, and four branches each writing their own migration 8.