Skip to content

fix(mail): MailClient derives the promotion lock, mailbox paths and replay state from one root resolved per operation - #640

Open
tps-anvil wants to merge 1 commit into
mainfrom
fix/636-one-mail-root
Open

tps-anvil wants to merge 1 commit into
mainfrom
fix/636-one-mail-root

Conversation

@tps-anvil

@tps-anvil tps-anvil commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #636.

What changed

MailClient pinned its promotion lock root at construction, while its mailbox file paths and replay-state path followed the configured mail directory at each use. If the mail root was re-pointed or moved while a long-running client was alive, the promotion lock stayed under the old root while the files and the replay state followed the new one: a second process holding the new root's lock did not exclude promotion, so a delivery could repeat.

The mailbox root is now resolved once per operation, and the promotion lock path, the mailbox file paths and the replay-state path all derive from that one value. A re-pointed root moves all three together.

Evidence

Measured on a0833a0 (main 78d295e).

  • Acceptance test packages/agent/test/mail-client-repoint-root.test.ts (real filesystem, a real second process holding the new root's mailbox lock): red on main — it fails the assertion that the client does not promote under the old root's lock — and green at this head.
  • Mutation (lock root resolved at construction again): the same assertion fails; restored, green.
  • Agent suite (23 files): 241 pass / 0 fail. Baseline on main (22 files): 240 pass / 0 fail.
  • CLI suite (210 files): 3270 pass / 7 skip / 0 fail; identical on main.
  • node scripts/changelog-fragments.mjs check passes; bun run lint:ci exits 0; tsc builds packages/agent and packages/cli.

…ck and the paths it guards

Refs #636.

MailClient pinned its promotion lock root at construction while its mailbox
file paths and replay-state path were joined from the configured mail
directory at each use. If the mail root was re-pointed or moved while a
long-running client was alive, the promotion lock stayed under the old root
while the files and the replay state followed the new one, so a second
process holding the new root's lock did not exclude promotion and a delivery
could repeat.

Resolve the mailbox root once per operation and derive the promotion lock
path, the mailbox file paths and the replay-state path from that one value,
so a re-pointed root moves all three together.

Test: a real-filesystem case re-points the mail root under a live client
while a real second process holds the new root's lock; the client does not
promote under the old root's lock, and the one envelope is delivered once.
@tps-anvil
tps-anvil requested a review from a team as a code owner October 13, 2026 23:12
@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: fb680e58-618b-427a-b2ac-379ef3ba4797
  • 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.

@tps-anvil

Copy link
Copy Markdown
Collaborator Author

Prose sweep (cli#636, measured on a0833a0). Every sentence this PR adds or changes, checked against the code for (a) outcomes on every branch, (b) scope words, (c) coverage claims. Call per sentence.

Source comments — packages/agent/src/io/mail.ts

  1. "create the maildir for the root configured now." — construction calls mailboxRoot(), which creates the subdirectories. Pass.
  2. "Every operation derives its own root instead" — checkNewMail, sendMail and deliverOutbox each call mailboxRoot(); the private helpers receive the operation's root. Pass.
  3. "dir creation is not a construction-time pin." — the constructed root is not stored; operations resolve their own. Pass.
  4. "the mailbox root for the current operation, resolved once … with its maildir subdirectories created." — mailboxRoot() resolves via mailboxLockRoot (real path) and creates new/cur/dlq/outbox. Pass.
  5. "The promotion lock, the mailbox file paths and the replay-state path all derive from this one value" — commitToCur locks root, derives join(root,"cur",file) and mailboxReplayStore(root); recovery likewise. Pass.
  6. "a mail root re-pointed or moved while the client is alive moves all three together." — each operation resolves the root afresh. Pass.
  7. "It is never pinned at construction." — no root is retained at construction. Pass.
  8. "the resolved real path comes from mail-lock-namespace.ts so promotion and placement recovery reach one lock." — both commitToCur and the recovery helpers lock the same resolved root. Pass.

New test — packages/agent/test/mail-client-repoint-root.test.ts
9. "derive from one root resolved per operation, never pinned at construction." — true of the code under test. Pass.
10. "The client must not promote under the OLD root's lock, and … the one envelope is delivered exactly once." — asserted: checkNewMail returns [] while the lock is held, then ["r.json"] once, then []. Pass.
11. "The new root is a real mailbox: its maildir exists, and one record is seeded in new/." — the test creates new/cur/dlq/outbox and writes one record. Pass.
12. "The replay gate refuses the same envelope again." — the third checkNewMail returns []. Pass.
13. "Hold a mailbox lock in a real second process and release it on stdin." — the helper spawns bun and releases on stdin data. Pass.
14. Test title "a live client does not promote under the old root's lock, and delivers the envelope once" — both clauses asserted. Pass.

Changelog fragment — .changelog/unreleased/fixed-mailclient-one-mail-root.md
15. "MailClient derives its promotion lock, mailbox paths and replay state from one root resolved per operation." — true. Pass.
16. "A mail root re-pointed or moved while a client is alive now moves all three together: the promotion lock is taken under the root in force, not one resolved at construction." — true; the lock is taken on the per-operation root. Pass.

Edited tests — mail-interrupted-recovery.test.ts, mail-promote-guard.test.ts: the changes add the resolved-root argument to the two direct private-method calls and their import; no new prose claims.

No failing sentences.

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.

MailClient: the promotion lock and the mailbox paths it guards use one resolved root

2 participants