Sprint 1 Review — MAILBOX-LIST UDS verb + non-root mailbox_list

Sprint Goal Assessment

Sprint 1's goal is met. The MCP mailbox_list tool no longer touches /etc/aimx/config.toml; it ships an AIMX/1 MAILBOX-LIST frame and the daemon answers with a SO_PEERCRED-filtered JSON array. Non-root agents enumerate their own mailboxes without sudo. Codec discipline (no headers on MAILBOX-LIST) and the new JsonAckResponse shape are clean and well-tested. Build, clippy, fmt, and the new unit + integration tests all pass locally.

Acceptance Criteria Checklist

[S1-1] Add MAILBOX-LIST UDS verb to the AIMX/1 codec

• Request::MailboxList variant with no payload — src/send_protocol.rs:182.
• parse_request_frame recognizes MAILBOX-LIST — src/send_protocol.rs:402.
• Round-trip + header-rejection unit tests cover Owner: and Content-Length: — mailbox_list_codec_tests, all 5 tests pass.
• No daemon handler in this story — handler lands in S1-2.
• cargo test / cargo clippy --all-targets -- -D warnings / cargo fmt -- --check clean.

[S1-2] Daemon-side handler — JSON response with SO_PEERCRED owner filter

• handle_mailbox_list in src/mailbox_list_handler.rs (the sprint plan explicitly allowed a new file).
• Per-row struct serializes the FR-A3 fields exactly: name, inbox_path, sent_path, total, unread, sent_count, registered.
• Root sees all; non-root uses mailbox::caller_owns with stray-dir uid fallback via inbox_owner_uid.
• Stray dirs visible to root and to the user whose uid owns the inbox dir.
• Empty result is the JSON literal [] (verified by stranger_sees_empty_array).
• Dispatch in src/serve.rs:861 writes the JSON ack response.
• Four unit tests cover root, owner, stranger, and on-disk count paths.
• Lints clean.

[S1-3] MCP mailbox_list becomes a thin UDS client

• mailbox_list no longer calls self.load_config or self.authorize_mailbox — it goes through submit_mailbox_list_via_daemon.
• Daemon-not-running message matches email_send / hook_create exactly.
• list_mailboxes_with_unread deleted from src/mcp.rs.
• New integration test mcp_mailbox_list_returns_caller_owned spawns aimx serve + aimx mcp in a temp data dir and asserts the JSON shape.
• Old text-format assertion rewritten; new test exists for the missing-socket path (mcp_mailbox_list_without_daemon_reports_missing_socket).
• Lints clean.

[S1-4] Primer FR-F1 lead + mcp-tools.md mailbox_list row rewrite

• agents/common/aimx-primer.md "Two access surfaces" leads with the exact FR-F1 phrasing verbatim.
• agents/common/references/mcp-tools.md mailbox_list row rewritten with the JSON field table, an example response, and a "next step" pointing the agent at <inbox_path>/<id>.md.
• No reference to aimx mailboxes list introduced in the primer.
• cargo build rebuilds bundles cleanly.
• Manual smoke (aimx agents setup claude --print) — not explicitly evidenced in the PR description, but the bundle is built from the same include_str!/include_dir! source so the rebuild covers it.

Test Coverage

Coverage is solid. The codec layer has 5 round-trip + rejection tests, the handler has 4 unit tests across the visibility matrix, and there are 2 integration tests that exercise the real binary end-to-end (with-daemon and without-daemon paths). Two minor gaps worth flagging — both Non-blocker:

• Stray on-disk dir visibility for non-root caller is unit-tested only implicitly. The handler's stray-dir branch (is_visible_to falling through to inbox_owner_uid) is not exercised by a dedicated test. The non_root_owner_sees_only_owned_mailboxes test seeds dirs but the mailboxes are registered, so the fallback path never fires. Adding a test that creates a stray inbox dir (no config entry) chowned to the caller would pin FR-A3's "stray dirs appear with registered: false" guarantee for non-root callers.
• is_marked_read cheap-parser divergence from count_with_unread. The handler uses a line-scan that recognizes read = true literally; mailbox::count_with_unread (used by aimx mailboxes show) does a full TOML decode. The two will agree on every well-formed frontmatter aimx writes today, but a hand-edited read = "true" (string) would be reported as unread by the new handler and as read by the show command. No test pins this — agree-by-construction with count_with_unread would be a useful invariant test.

Potential Bugs

None of the issues below are blockers — Sprint 1 is internally consistent and the code paths are correct on every input the handler can encounter from aimx-written frontmatter. Calling them out as Non-blocker for the implementer's awareness.

• Cheap read = … parser is permissive about quoted values. src/mailbox_list_handler.rs:295-298 parses read = "true" (string) as false because it compares the trimmed value against the literal "true" (without quotes). aimx never writes a quoted bool, so this is unreachable from the daemon's own writers, but it is a divergence from the full TOML decoder used elsewhere. Non-blocker — no production frontmatter would hit this; raised for consistency with count_with_unread.
• splitn(3, "+++") breaks on a +++ token inside the frontmatter body. Same file; same function. The TOML frontmatter contract forbids +++ inside the block, but the cheap parser is more brittle than the TOML decoder. Non-blocker, same reason.
• is_visible_to for catchall. The PRD wording is "non-root callers never see the catchall — preserves today's behavior" but the sprint plan and the implementation both treat catchall identically to other mailboxes (visible to root or to whoever owns it via aimx-catchall). The implementation matches the sprint plan and matches today's caller_owns semantics, so this is correct — flagging only because the PRD reads slightly more restrictive at first glance. Not a bug.

Security Issues

None observed. The MAILBOX-LIST codec rejects Owner: and Content-Length: headers on the request frame, so a forged client cannot smuggle an owner hint past the SO_PEERCRED filter. caller.uid is sourced from the kernel-validated peer credential. No user-supplied input feeds into a path or shell.

Code Quality

• submit_mailbox_list_via_daemon reuses is_socket_missing (which catches NotFound | ConnectionRefused | PermissionDenied) while older sibling helpers (submit_via_daemon, submit_mark_via_daemon) still hand-check ErrorKind::NotFound only. The new code is the better pattern; consider migrating the older helpers in a follow-up so all daemon-not-running surfaces behave identically when the socket exists but is unreachable. Non-blocker, follow-up.
• decode_mailbox_list_response reimplements basic AIMX/1 header parsing (Content-Length read, ERR split). The pattern is broadly the same as parse_response but specialised for the JSON-bodied OK frame. Acceptable for the first JSON ack consumer; if Sprint 2's email_list ships another JSON ack reader, a shared parse_json_ack_response helper would prevent drift.
• JsonAckResponse::Err writer is byte-identical to AckResponse::Err. The unit test pins this, which is good. As long as the contract holds, clients can use one ERR parser for all ack-style verbs.

Alignment with PRD

Aligned. FR-A1 through FR-A6 and FR-F1/F2 are all addressed in this sprint. The other FRs (B/C/D/F3/F4) belong to later sprints and are correctly out of scope here.

Human Review Comments

No human comments on the PR; no linked issue.

Summary and Recommended Actions

• Overall verdict: Ready to merge.
• Blockers: None.
• Non-blockers (worth a follow-up but not a re-spin):
  • Add a unit test that exercises the stray-dir visibility branch for a non-root caller (seed a stray inbox dir with the caller's uid; assert it appears with registered: false).
  • Add an invariant test that the cheap is_marked_read parser agrees with the full-TOML decoder on every shape aimx itself emits (or replace the cheap parser with the full decoder if perf is not load-bearing here).
  • Migrate submit_via_daemon and submit_mark_via_daemon to use is_socket_missing so all daemon-not-running surfaces behave identically.
• Nice-to-haves: Factor the JSON-ack response decoder into a shared helper before Sprint 2 adds a second consumer in email_list.

[Sprint 1] MAILBOX-LIST UDS verb + non-root mailbox_list

Move mailbox_list off the root-owned config.toml: add an
AIMX/1 MAILBOX-LIST verb the daemon answers with a JSON array
filtered by SO_PEERCRED, and turn the MCP tool into a thin UDS
client. New JsonAckResponse shape; codec rejects Owner: /
Content-Length: headers on MAILBOX-LIST so a forged owner hint
cannot defeat peer-cred filtering. Primer + mcp-tools.md lead
with "read .md files directly when you don't need a mutation"
and document the new JSON shape with a worked next-step example.