Sprint 45 — Strict Outbound + MCP Writes via Daemon — Review

Sprint Goal Assessment

The implementation fully achieves the stated sprint goal. All four stories (S45-1 through S45-4) land the privilege-separation and correctness fixes described in the plan: aimx send no longer reads config.toml, outbound is strictly gated on configured-domain + non-wildcard mailbox, and MCP write ops route through new MARK-READ / MARK-UNREAD UDS verbs protected by per-mailbox async locks. Findings #4, #5, and #8 from the 2026-04-17 manual test run are closed.

Acceptance Criteria Checklist

[S45-1] aimx send stops reading config.toml; daemon resolves From mailbox

• From-Mailbox: removed from encoder/parser — met (send_protocol.rs)
• Daemon parses From: from body and resolves against in-memory Config — met (send_handler.rs:123-165)
• Client deletes Config::load / resolve_from_mailbox call path — met (send.rs::build_request now just composes + wraps)
• main.rs dispatches Command::Send before config load — met (main.rs:68 hoisted out of dispatch_with_config)
• Unit tests moved to send_handler.rs — met
• book/mailboxes.md + CLAUDE.md updated — met
• "New integration test: run aimx send as a non-root user against a 0640 config; assert the Permission denied error no longer reproduces" — PARTIAL: the PR description notes this wasn't implemented as a standalone test. The new behavior is covered transitively by uds_send_listener_* integration tests that exercise the daemon path end-to-end with no config access from the client. Acceptable because aimx send now contains zero config:: references, making the scenario structurally impossible rather than just behaviorally tested.

[S45-2] Strict outbound — concrete mailbox + configured domain only

• Wildcard fallback deleted — met (resolve_concrete_mailbox skips entries where address.starts_with('*'))
• Domain mismatch → ERR DOMAIN — met (send_handler.rs:143-151)
• Mailbox miss → ERR MAILBOX with aimx mailbox create guidance — met
• Case-insensitive domain match — met + test
• Display-name form, concrete-mailbox acceptance, catchall rejection, foreign domain — all covered by unit tests

[S45-3] UDS protocol scaffolding — MARK-READ and MARK-UNREAD

• Dispatcher recognizes SEND + MARK-READ + MARK-UNREAD; unknown → ParseError::UnknownVerb mapped to ERR PROTOCOL — met
• write_mark_request + typed MarkRequest — met
• New codes PROTOCOL, NOTFOUND, IO added — met
• Codec unit tests for happy-path, malformed headers, missing headers, unknown folder, non-zero Content-Length rejection — all present (32 send_protocol::tests)
• File rename deferred — acceptable per sprint plan ("judgment call for the implementer")

[S45-4] MCP write ops route through daemon; per-mailbox concurrency guard

• email_mark_read/email_mark_unread become thin UDS clients — met (mcp.rs:225-247, submit_mark_via_daemon)
• Helpful "aimx daemon not running" hint on ENOENT — met
• state_handler.rs with StateContext + handle_mark — met
• Per-mailbox tokio::sync::RwLock<()> lazily inserted — met (StateContext::lock_for)
• ERR paths for mailbox-missing, id-missing, invalid folder, write failure, path traversal — all covered
• Integration test mcp_email_mark_read_unread (success) + mcp_email_mark_without_daemon_reports_missing_socket (fallback) — met
• Integration test mcp_mark_read_concurrent_with_inbound_ingest — met
• book/mcp.md updated — met

Potential Bugs

1. (Non-blocker, Documentation) State handler's concurrency comment overstates what it guarantees.
state_handler.rs:102-106 says "inbound takes the same lock shape via INGEST_WRITE_LOCK today" and the PR description's "Technical Decisions" section acknowledges this. In reality, ingest.rs uses a process-scoped std::sync::Mutex while state_handler.rs uses a per-mailbox tokio::sync::RwLock<HashMap<String, _>>. They are completely independent — MARK-READ and ingest on the same mailbox do NOT serialize against each other. The concurrency-safety claim relies instead on the fact that inbound ingest writes a freshly-allocated filename while MARK-READ targets an existing file, so they touch disjoint paths in practice. The integration test mcp_mark_read_concurrent_with_inbound_ingest exercises that disjoint-path case, not a true shared lock. Since the sprint explicitly defers unification to Sprint 46 and the practical behavior is safe, this isn't a correctness blocker, but the code comment should be softened so the next reader doesn't assume serialization they don't have.

2. (Non-blocker) state_handler::resolve_email_path is duplicated from mcp::resolve_email_path.
Both have identical flat-then-bundle lookup logic. Not a bug; just duplicated maintenance surface. Fine to ignore; Sprint 46 will touch the same area.

3. (Non-blocker, Minor) parse_ack_response accepts AIMX/1 OK <anything>.
mcp.rs:496 accepts both rest == "OK" and rest.starts_with("OK "). For the MARK verbs the daemon only ever writes AIMX/1 OK\n (see write_ack_response), but the client's looser acceptance of AIMX/1 OK <stuff> would silently swallow unexpected trailing content. Not exploitable — it only affects whether a malformed daemon reply is classified as Ok vs Malformed. Defensive tightening to rest == "OK" would be more honest about what the MARK verb protocol says.

Security Issues

No new security concerns.

• The UDS socket remains world-writable per FR-18b; authorization is explicitly out of scope in v0.2. That is documented in the primer and this sprint does not alter the posture.
• validate_id on the daemon side (state_handler.rs:65-78) blocks path-traversal (.., /, \, \0). Mirrors mcp::validate_email_id so a malicious local client that bypasses the MCP layer still gets rejected. Good defense in depth.
• The From-Mailbox: header is silently ignored (not trusted). The daemon re-derives the mailbox from the body's From:. Correct — the header was untrusted even before this sprint.
• Sanitization of Mailbox: / Id: / reason strings via sanitize_inline strips CR/LF so malicious mailbox/id values cannot inject extra response lines.

Test Coverage

Strong coverage across all four stories:

• 29 unit tests in send_handler::tests (S45-1 + S45-2), covering all FR-18c error codes, display-name/bare/angle-only From, foreign domain, case-insensitive match, catchall rejection, synthesized Message-ID, persist-on-delivery vs skip-on-TEMP, sign-failure → SIGN code.
• 32 unit tests in send_protocol::tests (S45-3), covering SEND + MARK verb happy paths, all malformed shapes (missing headers, wrong folder, non-zero Content-Length on MARK, etc.), and legacy_from_mailbox_header_is_silently_ignored.
• 7 unit tests in state_handler::tests, covering toggle to read and back, unknown mailbox, missing email, path traversal, bundle layout, sent folder.
• Integration tests spawning a real aimx serve subprocess: mcp_email_mark_read_unread, mcp_email_mark_without_daemon_reports_missing_socket, mcp_mark_read_concurrent_with_inbound_ingest.

Minor gaps (Non-blocker):

• The concurrent ingest + MARK test operates on different files (one pre-seeded, one freshly ingested). A test where MARK-READ and ingest actually target the same filename would have to construct an adversarial timestamp collision, which is contrived — the current test is the practical case.
• No test exercises malformed frontmatter in the target file (e.g., truncated +++ delimiters). The code path maps it to ErrCode::Io, which is reasonable; a test would nail the contract down.

Code Quality

• handle_uds_connection_with_timeout in serve.rs is now a single dispatcher for both SEND and MARK verbs with shared timeout + parse-failure drain logic. Clean shape.
• Reply::Send / Reply::Ack enum wrapping the two response types keeps the write path single-pass. Readable.
• SendContext and StateContext are cleanly separated — DKIM key lives only in the former; mailbox set as HashSet<String> lives only in the latter. No overlap of responsibilities.

Minor:

• resolve_concrete_mailbox has a subtle O(n) scan plus a fallback by local-part lookup. For v0.2 mailbox counts this is fine; flagged only for future awareness.
• state_handler::handle_mark reconstructs the file using manual "+++\n" / "+++" concatenation. This matches the existing mcp::set_read_status pattern. Using format_frontmatter would be more DRY but the type shapes differ (InboundFrontmatter vs OutboundFrontmatter for sent-folder case), so the current approach is pragmatic.

Alignment with PRD

• FR-18c code set: implementation adds three new codes (PROTOCOL, NOTFOUND, IO) alongside the existing MAILBOX | DOMAIN | SIGN | DELIVERY | TEMP | MALFORMED. Sprint plan explicitly called for this. OK.
• FR-18d (tightened): "the From mailbox must resolve to a configured non-wildcard mailbox whose address is under config.domain" — correctly enforced. The PRD's catchall-is-inbound-only semantics is now matched by the code.
• FR-18b (world-writable socket, UID logging for diagnostics only, no authorization) — preserved unchanged.
• No PRD P0 requirements appear unaddressed by this sprint.

Summary and Recommended Actions

• Overall verdict: Ready to merge (with one recommended comment-only cleanup)

• Blockers: none

• Non-blockers:

  1. Soften the concurrency comment in state_handler.rs:102-106 so the next reader doesn't assume MARK and ingest share a lock (they don't; safety comes from disjoint file targets).

• Nice-to-haves:

  1. Consider tightening parse_ack_response to reject AIMX/1 OK <trailing> for MARK verbs (currently accepted).
  2. Add an integration test for MARK-READ against a file with malformed frontmatter (maps to ErrCode::Io).
  3. Deduplicate resolve_email_path between mcp.rs and state_handler.rs — tracking for Sprint 46.

Sprint 45: strict outbound + MCP writes via daemon

- `aimx send` no longer reads /etc/aimx/config.toml; daemon parses From:
  from the submitted body and resolves the sender mailbox against its
  in-memory Config.
- Outbound rejects both foreign-domain From (ERR DOMAIN) and any From
  whose local part does not match an explicitly configured non-wildcard
  mailbox (ERR MAILBOX). Catchall is inbound-only per FR-18d (tightened).
- New AIMX/1 MARK-READ / MARK-UNREAD UDS verbs with typed codec, per-
  mailbox tokio RwLock in a StateContext, and client wiring in `aimx mcp`
  so email_mark_read / email_mark_unread work without root write access
  to mailbox files.
- New error codes PROTOCOL / NOTFOUND / IO added; legacy From-Mailbox:
  header silently ignored for forward-compatibility.

Closes findings #4, #5, #8 from the 2026-04-17 manual test run.

Sprint 45 — Re-Review (Cycle 2)

All four items from the Cycle 1 review are resolved in commit bdb0885. Verified by reading the diff and running the new tests.

Resolved

1. src/state_handler.rs:95-102 concurrency comment — rewritten correctly. The new wording explicitly states that (a) the per-mailbox tokio::sync::RwLock only serializes MARK calls against other MARK calls on the same mailbox, (b) it is independent of the process-wide ingest::INGEST_WRITE_LOCK (a blocking std::sync::Mutex), and (c) safety today rests on disjoint file targets (ingest writes freshly-allocated filenames; MARK rewrites existing ones). Sprint 46 is called out as the follow-up to unify both writers. The previous misleading "inbound takes the same lock shape via INGEST_WRITE_LOCK today" phrasing is gone.

2. src/mcp.rs::parse_ack_response — tightened. AIMX/1 OK is now the only form accepted as MarkOutcome::Ok. AIMX/1 OK <anything> returns MarkOutcome::Malformed("unexpected payload after OK: …"). Good defensive posture — the MARK ack is bodyless by contract, and a drifting daemon will now surface at the client instead of being silently swallowed.

3. src/state_handler.rs malformed-frontmatter tests — both added:

   • mark_rejects_truncated_frontmatter_as_io_error — writes a file with only the opening +++ and asserts ErrCode::Io with a reason containing "malformed frontmatter". Verified the splitn(3, "+++") path: length 2 < 3 triggers the correct branch.
   • mark_rejects_unparseable_frontmatter_toml_as_io_error — delimiters present but TOML body is this is = not valid = toml; asserts ErrCode::Io from the toml::from_str failure branch.

4. resolve_email_path dedup — the private copy is deleted from state_handler.rs; it now imports crate::mcp::resolve_email_path (which is pub at mcp.rs:614). One implementation, one maintenance surface.

New findings

None. The fix commit is tightly scoped (2 files, 125 insertions, 23 deletions) and introduces no regressions.

Verification

• cargo build --tests: clean.
• cargo clippy --all-targets -- -D warnings: clean.
• cargo test --bin aimx state_handler::: 9/9 pass (including the two new malformed-frontmatter tests).
• cargo test --bin aimx mcp::tests::parse_ack_response: 3/3 pass (bare-OK accept, trailing-payload reject, ERR code parse).

Verdict

Ready to merge.

Sprint 45: strict outbound + MCP writes via daemon

- `aimx send` no longer reads /etc/aimx/config.toml; daemon parses From:
  from the submitted body and resolves the sender mailbox against its
  in-memory Config.
- Outbound rejects both foreign-domain From (ERR DOMAIN) and any From
  whose local part does not match an explicitly configured non-wildcard
  mailbox (ERR MAILBOX). Catchall is inbound-only per FR-18d (tightened).
- New AIMX/1 MARK-READ / MARK-UNREAD UDS verbs with typed codec, per-
  mailbox tokio RwLock in a StateContext, and client wiring in `aimx mcp`
  so email_mark_read / email_mark_unread work without root write access
  to mailbox files.
- New error codes PROTOCOL / NOTFOUND / IO added; legacy From-Mailbox:
  header silently ignored for forward-compatibility.

Closes findings #4, #5, #8 from the 2026-04-17 manual test run.