Changes Overview

Bundles four mechanical cleanup items from the user-mailbox track's Non-blocking Review Backlog into a single PR: (R1) a 4-line comment explaining intentional non-memoization in a test resolver, (R2) deletes dead require_root / enforce_root + their three direct unit tests from src/uds_authz.rs, (R3) renames the wire-protocol type names MailboxCrudRequest -> MailboxLifecycleRequest, Request::MailboxCrud -> Request::MailboxLifecycle, and MailboxCrudFallback -> MailboxLifecycleFallback across 5 files, and (R4) derives the format_auth_error rendering verb in mcp::authorize_mailbox from the auth::Action variant (MailboxCreate -> "create", MailboxDelete -> "delete", else None) instead of hardcoding Some("create"). Strict net diff: +88 / -173.

Scope Alignment

All four in-tree items match the PR description exactly. R5 (marking the four backlog bullets [x] in docs/user-mailbox-sprint.md) does not appear in the diff because docs/ is a symlink to a path outside the repo — this is structural and called out in the PR description, not a gap.

No scope creep observed: wire-protocol verb strings (MAILBOX-CREATE / MAILBOX-DELETE / MAILBOX-LIST), auth::Action::Mailbox* variants, and format_auth_error itself are all untouched. Test scaffolds in mailbox_handler.rs::tests were updated to the new struct name with no semantic changes.

Independent Verification

• git grep "MailboxCrud" 052db5db -- src/ services/ -> nothing. Closes the S3-6 spot-check strictly. (PR's claim verified.)
• git grep "require_root\|enforce_root" 052db5db -- src/ services/ -> nothing. R2 deletion is safe; no production caller remains.
• The remaining consumers of uds_authz are Caller, peer_username, log_decision, enforce_mailbox_owner_or_root — none touch the deleted helpers.
• R4 verb-derivation match is exhaustive enough: explicit arms for MailboxCreate { .. } / MailboxDelete { .. }, fallback _ => None. format_auth_error defaults verb to "perform" when None, so OwnerMismatch from a hypothetical future non-create predicate will render as "cannot perform a mailbox owned by another user" — sensible. No panic! fallback.
• No existing test asserts on the previous "create" wording from the MCP surface. auth.rs::tests exercises format_auth_error directly with explicit verb values and is unaffected.
• Doc-comment housekeeping is consistent: the MCP comment correctly drops the stale "/ verb" mention now that verb is passed; MailboxLifecycleRequest's doc-comment block correctly updated; write_mailbox_crud_request's doc paragraph now references the new struct type.
• On the PR head SHA 052db5db: cargo fmt -- --check clean; cargo clippy --all-targets -- -D warnings clean; cargo test --bin aimx 1026/1026; cargo test --test integration 97/97; cargo test --tests (uds_authz default lane) 1/1; verifier cargo test 43/43. All matches the PR's claimed numbers.

Potential Bugs

None.

Security Issues

None. R2 deletion does not weaken the authz surface — the deleted helpers had zero callers; mailbox-CRUD authorization runs through enforce_mailbox_owner_or_root and the auth::authorize predicate, both untouched.

Test Coverage

Adequate. The three deleted unit tests (require_root_rejects_non_root, require_root_accepts_root, enforce_root_logs_reject) exercised only the deleted helpers — removing them with the helpers is correct, not a regression. R4 has no new test scaffold (the PR description acknowledges this; mcp.rs has no tests module covering authorize_mailbox directly today and the verb derivation is one match arm). All other changes are mechanical type renames covered by the existing test suite that uses the renamed types.

Code Quality

One minor observation, non-blocker, out of scope per the literal R3 wording:

The R3 rename only touched CamelCase type identifiers. The underlying snake_case function names still carry the old mailbox_crud prefix:

• submit_mailbox_crud_via_daemon (src/mcp.rs)
• submit_mailbox_crud_request (src/mcp.rs)
• handle_mailbox_crud (src/mailbox_handler.rs)
• parse_mailbox_crud_headers (src/send_protocol.rs)
• write_mailbox_crud_request (src/send_protocol.rs)
• the mailbox_crud_create_then_mark_works_on_same_mailbox test name

This is consistent with the PR's stated scope ("rename pre-existing wire-protocol type names") and the strict spot-check (grep "MailboxCrud" — CamelCase) passes. But it leaves a stylistic mismatch between the type names and the function names that handle them. Worth tracking as a follow-up if the operator wants full nomenclature consistency, but absolutely not blocking.

Summary and Recommended Actions

• Overall verdict: Ready to merge
• Blockers: none
• Non-blockers: none
• Nice-to-haves: optional follow-up to rename the snake_case mailbox_crud function names to mailbox_lifecycle for full nomenclature consistency

Recommended merge commit message

chore: user-mailbox track backlog cleanup

Bundles the four Non-blocking Review Backlog items from the user-mailbox
track:

- Comment in `install_tester_resolver_with_current_user` documenting why
  per-call `lookup_username` is intentional (test isolation requirement).
- Delete dead `require_root` / `enforce_root` from `src/uds_authz.rs`
  and their three direct unit tests; no production callers remained
  after Sprint 1 retired the root-only mailbox-CRUD path.
- Rename wire-protocol type names off the `MailboxCrud` prefix:
  `MailboxCrudRequest` -> `MailboxLifecycleRequest`,
  `Request::MailboxCrud` -> `Request::MailboxLifecycle`,
  `MailboxCrudFallback` -> `MailboxLifecycleFallback`. Wire-protocol
  verb strings (`MAILBOX-CREATE` / `MAILBOX-DELETE`) unchanged.
- Derive `format_auth_error` rendering verb from the `auth::Action`
  variant in `mcp::authorize_mailbox` instead of hardcoding
  `Some("create")`, so a future predicate that emits `OwnerMismatch`
  from a non-create action renders correctly.

Net: +88 / -173. fmt + clippy clean; 1124 default-lane tests pass;
21/21 ignored uds_authz integration tests pass under sudo.