Skip to content

feat(mail): per-message MailClient delivery with a live in_progress intent and completion-only finalize (cli#617 slice b) - #641

Open
tps-anvil wants to merge 1 commit into
mainfrom
feat/617-slice-b
Open

tps-anvil wants to merge 1 commit into
mainfrom
feat/617-slice-b

Conversation

@tps-anvil

Copy link
Copy Markdown
Collaborator

Refs #617 (slice (b) of the approved design rev 2)

What this slice does

Adds the mechanism only, inert at runtime. MailClient gains a per-message
delivery API (deliverNext() / completeDelivery()) that commits one message
and leaves its placement intent in the in_progress phase, removed by an
explicit completion. A committed message is re-presented until then, bounded by
the liveness bound (after five presentations without completion, or 24 hours
after the first, the intent goes terminal failed, the payload is kept, and one
log line names the mailbox, message ID and presentation count). Recovery is
completion-gated: a placing intent is finished to in_progress and presented,
an in_progress intent is re-presented, and unknown is skipped.

runtime/event-loop.ts and runtime/agent.ts are untouched and the batch
checkNewMail() the event loop calls keeps its current behaviour, so merging
this does not change the running agent's delivery; slice (c) wires the event
loop.

recoverPromoted now reads the placement intent's phase and withholds a record
that resolves to anything but placing, with a retryable busy refusal. That
covers every direct caller the slice (a) PR deferred: the bridge startup sweep,
the OpenClaw plugin startup sweep, verifyMailAction and the plugin's
internalInbound.

The at-most-once assertions in mail-interrupted-recovery.test.ts are rewritten
to the "re-present until completed" contract, driving the delivery API.

The operator command that clears failed intents and tombstones is a later,
smaller PR; it is not in this slice.

Design items → code / test

Slice (b) mechanism:

  • per-message commit + in_progress writer — MailClient.deliverNext,
    MailClient.commitToCur(..., keepInProgress) and
    ReplayStore.advancePresentation in packages/agent/src/lib/mailbox-policy.ts.
  • completion-only finalize — MailClient.completeDelivery and
    ReplayStore.completePlacement.
  • liveness bound — advancePlacementPresentation (MAX_DELIVERY_PRESENTATIONS,
    DELIVERY_PRESENTATION_TTL_MS) plus the log line in MailClient.presentUnderLock.
  • in_progress re-presentation in recovery — MailClient.recoverDelivery /
    presentUnderLock.
  • completion-gated placing recovery — presentUnderLock moves placing to
    in_progress.
  • "later messages are still in new/" — commitNextForDelivery commits one
    message.
  • the four direct recoverPromoted presenters — the phase check in
    recoverPromoted (packages/cli/src/utils/mail.ts).

Rev-2 precision items:

  • the stray source left by a crash between the link and the source unlink: the
    source is a hard link to cur/, so a later pass removes it through the
    existing same-inode "already-delivered" path, under the relay stripe. Covered
    by the kill-before-source-removal case.
  • a terminal failed intent does not keep the per-file gate closed:
    advancePlacementPresentation writes failed; pendingPlacementEntries
    omits it and hasPendingFile reads it as not live. Asserted in the liveness
    tests.
  • the consumed append on placing recovery stays guarded by isConsumed
    (presentUnderLock).
  • the CLI recovery site withholds presentation: the slice (a) lease sweep
    covers the sweep; this slice's recoverPromoted check covers the four direct
    callers.
  • commit-time verify mechanics are unchanged: MailClient verifies before the
    lock, and commitToCur re-reads under the lock.

Evidence

Tests (real signed records through the real parser and signer; real child-process
kills for the crash boundaries):

  • packages/agent/test/mail-delivery-at-least-once.test.ts — 14 tests: one
    commit at a time with later mail left in new/; completion-only finalize;
    in_progress re-presentation; completion-gated placing recovery; duplicate
    envelope refused as replay; the presentation-count and TTL bounds; the
    terminal intent; the preserved unknown phase; kills before intent write,
    link, source removal and ledger commit; a kill after the intent becomes
    in_progress.
  • packages/agent/test/mail-interrupted-recovery.test.ts — 10 tests, rewritten
    to the delivery API's "re-present until completed" contract.
  • packages/cli/test/mail-slice-b-presenters.test.ts — 4 tests:
    recoverPromoted withholds in_progress, failed and unknown, and
    verifyMailAction refuses.

Mutation checks, one per gate (edit, run, observe the behaviour assertion fail,
restore, observe green):

  • commit finalizes instead of leaving in_progress → the "re-present until
    completed" and "later messages stay in new/" assertions fail.
  • recovery skips in_progress → the re-presentation assertions fail.
  • the liveness bound never trips → the two bound tests fail.
  • the recoverPromoted phase check removed → the three presenter tests fail.
  • isLivePlacement treats unknown as not live → the unknown-phase gate
    assertion fails.

Checks: tsc for packages/agent and packages/cli clean; bun run lint:ci
clean; node scripts/changelog-fragments.mjs check clean.

Suite counts, measured on 0ab2328 (main 78d295e):

  • agent: 254 pass / 0 fail across 23 files (baseline on main: 240 pass / 0 fail
    across 22 files) — the difference is the new 14-test file.
  • cli: 3274 pass / 7 skip / 0 fail across 3281 tests in 211 files (baseline on
    main: 3270 pass / 7 skip / 0 fail across 3277 tests in 210 files) — the
    difference is the new 4-test file.

…ntent and completion-only finalize (cli#617 slice b)

Adds the delivery mechanism only, inert at runtime. MailClient gains
deliverNext()/completeDelivery(): a commit leaves its placement intent in the
in_progress phase, recovered messages are re-presented until completed, and a
liveness bound (five presentations or 24 h) moves a stuck intent to terminal
failed. recoverPromoted reads the intent phase and withholds a record that
resolves to anything but placing, covering the four direct callers deferred by
slice (a). runtime/event-loop.ts and runtime/agent.ts are untouched.

Refs #617
@tps-anvil
tps-anvil requested a review from a team as a code owner October 13, 2026 23:58
@tps-anvil

Copy link
Copy Markdown
Collaborator Author

617 slice (b) — prose sweep (deliverable, not committed)

Scope: every sentence this PR adds or changes — changelog fragment, PR body,
added source comments, test titles. Checked against the code for three claim
types: (a) outcomes/status codes on every branch; (b) scope words
(every/all/only/never/none/always); (c) coverage claims (does the named test
assert it).

Changelog fragment .changelog/unreleased/changed-617-slice-b-delivery-api.md

Sentence (compressed) Claim type Call
"MailClient gains a per-message delivery API; a committed message stays re-presentable until completed" coverage ok — deliverNext leaves in_progress; completeDelivery removes it; asserted by the delivery tests.
"Nothing in the agent runtime calls the new API yet, so delivery is unchanged until slice (c)" scope (unchanged) ok — git diff touches neither runtime/event-loop.ts nor runtime/agent.ts; checkNewMail unchanged.
"commits one message and leaves its placement intent in the in_progress phase, removed by an explicit completion" outcome ok — commitToCur(..., true) → advancePresentation; removed by completePlacement. (Dropped "only" here: the guarantee word added no fact.)
"after five presentations without completion, or 24 hours after the first, the intent goes terminal failed, the payload is kept and one log line names the mailbox, message ID and presentation count" outcome + scope ok — MAX_DELIVERY_PRESENTATIONS=5, DELIVERY_PRESENTATION_TTL_MS=24h; failed written, payload untouched, one console.warn.
"recoverPromoted now reads the placement intent's phase and withholds a record that resolves to anything but placing with a retryable busy refusal, so tps mail ack and nack and the bridge and plugin startup sweeps do not present another consumer's in-flight delivery" outcome + scope ok — the check returns {ok:false,class:"busy"} before presentation; busy is in RETRYABLE_REJECT_CLASSES.

PR body

Reuses the fragment's claims plus the item→code/test mapping and the evidence
numbers. The mapping sentences name functions/tests that exist; the counts name
the commit they were measured on. No outcome/scope/coverage claim beyond the
table above.

Added source comments

Comment (compressed) Claim type Call
io/mail.ts "an in_progress re-presentation … the liveness bound turns a message that is never completed into a terminal failed intent" outcome ok — advancePlacementPresentation failed branch.
io/mail.ts "The caller finalizes with completeDelivery() after the handler completes" scope (softened) ok — directive, not a description of shipped behaviour; reworded from "only after".
io/mail.ts "recoverDelivery … unknown is skipped (never presented or finished)" scope ok — if (phase === "unknown") continue; and presentUnderLock returns null for unknown.
io/mail.ts "finish only a placing intent whose cur/ copy is absent" scope + outcome ok — finishUnlinkedPlacement re-checks resumableOnRecovery.
mailbox-policy.ts "Deletes only an in_progress intent for this message" scope + outcome ok — completeInProgressPlacement gates on phase + messageId.
mailbox-policy.ts "a failed or unknown intent, or another message's intent, is never replaced" scope ok — writePlacementPhase refuses unless phase is placing/in_progress and the messageId matches.
utils/mail.ts "the same bar now covers every direct recoverPromoted caller" scope (every) ok — the check is inside recoverPromoted, so every caller passes through it.
utils/mail.ts "a failed read never reads as absent" scope (never) ok — a placementPhase throw returns busy, not a pass.

Test titles / describe strings

Every added describe/test title states an outcome the corresponding
assertion measures. Spot-checked: "later messages stay in new/ until the
in-flight one completes"; "re-presents a committed message until it is
completed"; "finished to in_progress and presented"; "dead-lettered as replay
once committed"; "intent goes terminal, a line is logged, and other mail
flows"; "past the presentation TTL is quarantined"; "unknown … gate stays
closed"; "recoverPromoted withholds a … intent". All true against the code
and asserted by the test body.

Verdict

No failing sentence found. One guarantee word removed at review ("only"). No
new claims were added during the sweep.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 53798320-704a-4763-ad13-75065ff2d395
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

2 participants