Sprint Goal Assessment

The sprint set out to land two parallel surface narrows — drop Folder: from MARK-READ/MARK-UNREAD (delete MarkFolder), and drop stdin from the hook_create surface (delete HookStdin, reject the field at config load) — both as hard breaks with no shim. The implementation is largely there: the codec rejects an incoming Folder: header, state_handler hard-codes the inbox path, MarkFolder and HookStdin are deleted from src/, Config::load peeks at the parsed toml::Value and rejects stdin with the canonical FR-D3 wording, and the docs sweep (book/, agents/, per-agent SKILL headers) is comprehensive. There is one acceptance-criterion miss that breaks an integration test fixture under sudo (see S3-1 below).

Acceptance Criteria Checklist

S3-1: Drop Folder: from MARK wire and delete MarkFolder

• MarkFolder enum + MarkRequest::folder deleted; parse / as_str impls gone
• Codec writer no longer emits Folder:; parser rejects Folder: with ParseError::Malformed; round-trip + rejection tests added
• state_handler::handle_mark reduced to inbox-only via the new inbox_dir; sent-folder branch and its mark_read_in_sent_folder test deleted
• MCP-side: EmailMarkParams.folder deleted; submit_mark_via_daemon signature trimmed; tool descriptions updated
• NOT MET: grep -n "MarkFolder\|Folder:\b" src/ tests/ returns zero hits — tests/uds_authz.rs:107 (raw_mark_frame) still synthesises Folder: inbox on the MARK frame. See Potential Bugs.
• book/mcp.md email_mark_* sections drop the folder parameter

S3-2: Delete HookStdin + Hook.stdin + executor unconditional pipe

• HookStdin, is_default_stdin, Hook.stdin deleted from src/hook.rs
• build_stdin collapsed; the call site is SandboxStdin::Email(read_stdin_source(stdin_source)). SandboxStdin::None is retained because platform::run_fallback tests still construct it (correct audit).
• HookCreateParams.stdin and HookCreateBody::stdin deleted; CLI --stdin flag deleted; build_hook_create_body no longer takes the param
• Wire body parser's legacy-field pre-screen rejects any stdin value with the unified "always piped" hint (Protocol err code)
• stdin = "none" test (execute_on_receive_with_stdin_none_pipes_empty_stdin) deleted; remaining tests updated to drop the field
• hooks list table column removed; hooks delete --yes confirmation no longer prints stdin

S3-3: Config::load rejects stdin with hook-named error

• New reject_removed_stdin_field peek runs after reject_legacy_schema and before from_value. Error wording matches FR-D3 verbatim ("hook 'X' carries removed field 'stdin' — remove this line and restart aimx serve; the email is always piped to hooks").
• Hook name resolution: explicit name wins; otherwise derive_hook_name_from_toml rebuilds the sha256 effective name from the raw TOML — matches effective_hook_name for the unnamed case.
• Old email_json-only rejection is gone; the unified path covers none / email / email_json / anything else.
• Tests pin the wording for all four values plus the derived-name case; load_accepts_stdin_none was deleted as required.

S3-4: Primer FR-F4 — drop stdin docs, add selectivity guidance + book sync

• agents/common/aimx-primer.md "Creating hooks" drops stdin, adds the verbatim selectivity guidance
• agents/common/references/hooks.md, mcp-tools.md hook_create row updated; per-agent SKILL.md.headers (claude-code, codex, gemini, goose, hermes, opencode) drop stdin
• book/hooks.md, book/hook-recipes.md, book/configuration.md, book/cli.md, book/troubleshooting.md updated; book/mcp.md email_mark_* rows drop folder (the docs counterpart to S3-1)
• rg 'stdin\s*=' book/ agents/ returns the one expected hit (the removal documentation in book/hooks.md)

Potential Bugs

Blocker 1: tests/uds_authz.rs::raw_mark_frame still injects Folder: inbox

File: tests/uds_authz.rs:105-109

fn raw_mark_frame(verb: &str, mailbox: &str, id: &str) -> String {
    format!(
        "AIMX/1 {verb}\r\nMailbox: {mailbox}\r\nId: {id}\r\nFolder: inbox\r\nContent-Length: 0\r\n\r\n"
    )
}

After this PR, the codec rejects Folder: on MARK with ParseError::Malformed — that's the hardening S3-1 explicitly added (mark_rejects_folder_header test in send_protocol.rs). So every MARK fixture in uds_authz.rs that goes through send_frame_as now receives a parse-error response instead of the expected authz outcome.

Concrete consequences when the integration suite runs (gated by AIMX_INTEGRATION_SUDO=1 + the two test uids):

• mark_read_as_other_forbidden and mark_unread_as_other_forbidden will FAIL — they assert the response contains EACCES, but the daemon returns a parse error before authz runs.
• mark_read_as_owner_ok_passes_authz, mark_read_as_root_any_ok, mark_unread_as_owner_ok_passes_authz, mark_unread_as_root_any_ok will pass, but only by accident — they assert not contains EACCES and a parse error happens to satisfy that. They are no longer testing authz at all.

This violates the explicit acceptance criterion grep -n "MarkFolder\|Folder:\b" src/ tests/ returns zero hits. The criterion was set up precisely to prevent this — the same grep run today still surfaces this hit.

Fix: drop Folder: inbox\r\n from raw_mark_frame. The Content-Length: 0 line keeps the frame well-formed.

These tests are #[ignore]-gated for normal cargo test runs, but the integration sweep is part of the project's release validation. Treating "the test is ignored" as license to ship a broken fixture is the kind of rot CI does not catch but a release does.

Non-blocker: EmailMarkParams silently drops a stale folder argument

File: src/mcp.rs:57-63

EmailMarkParams is #[derive(Serialize, Deserialize, schemars::JsonSchema)] without #[serde(deny_unknown_fields)]. An agent that still sends {"mailbox": "...", "id": "...", "folder": "sent"} will have folder silently ignored and the inbox copy will be marked instead. The PRD frames this as a hard break ("sent-mail mark is not supported"), so an explicit rejection ("folder is not a recognized parameter; sent-mail mark was removed") is more honest than silent ignore. Not blocking — the new tool description tells agents not to pass it, and the JSON Schema no longer advertises it — but worth tightening for symmetry with how stdin is handled on hook_create.

Test Coverage

• round_trip_mark_request_omits_folder and mark_rejects_folder_header pin both directions of the wire change. Good.
• load_rejects_stdin_field_for_every_value covers none / email / email_json against a named hook and asserts the wording invariants ("named_hook", "stdin", "always piped"). load_rejects_stdin_field_with_derived_name_when_unnamed independently pins the derived-name path. Together they cover the FR-D3 wording requirement well.
• create_rejects_stdin_field loops over none / email / email_json / bogus — solid coverage of the UDS pre-screen.
• hook_toml_with_stdin_field_is_rejected confirms the deny_unknown_fields lower-level shape kicks in if Config::load's pre-screen is ever bypassed.

Test gap (Non-blocker): there is no test that confirms MarkFolder removal in tests/integration.rs against a real daemon (the codec round-trip is unit-level, the integration coverage is in uds_authz.rs which is broken — see Blocker 1). After fixing raw_mark_frame, also consider one positive integration test that round-trips a MARK over the actual UDS so the wire change is covered end-to-end.

Code Quality

• derive_hook_name_from_toml is best-effort by design (missing fields fall back to defaults). The doc comment calls this out, which is correct — at this stage the input is already a parse-failure candidate, so deriving a sha256 against partial fields is a reasonable trade for a greppable error name. No issue.
• reject_removed_stdin_field returns Result<(), String> while the rest of Config::load's error path is Box<dyn Error> — the caller does .map_err(|e| -> Box<dyn std::error::Error> { e.into() })?, which is fine but a touch awkward. Consider returning the boxed error directly for symmetry. Cosmetic.
• SandboxStdin::EmailJson survives in src/platform.rs (#[allow(dead_code)]) even though no construction site exists after this PR. The doc says "Retained for the sprint-rework but not constructed today" — fine for now, but worth a follow-up to delete once the dust settles. Not for this sprint.

Alignment with PRD

The implementation tracks PRD §6.3 (FR-C3, FR-C4, FR-C5) and §6.4 (FR-D1 through FR-D5) faithfully. The unified rejection wording matches FR-D3 verbatim. FR-F4 selectivity guidance is mirrored to the operator-facing chapter (book/hooks.md, book/configuration.md) as well as the agent primer, which is more thorough than FR-F4 strictly required.

The email_mark_* JSON Schema cleanly drops folder. The hook surface is now cmd, name?, timeout_secs?, fire_on_untrusted? — matches the PRD's intended shape.

Summary and Recommended Actions

Overall verdict: Needs minor fixes.

Blockers (must fix before merge):

1. tests/uds_authz.rs::raw_mark_frame still emits Folder: inbox. Drop the Folder: line. Without this, the integration suite (under AIMX_INTEGRATION_SUDO=1) regresses two MARK authz tests and silently neuters four others, and the explicit S3-1 acceptance criterion (grep -n "Folder:\b" src/ tests/ returns zero hits) is unmet.

Non-blockers (should fix but not blocking):

1. Add #[serde(deny_unknown_fields)] to EmailMarkParams so a stale folder arg fails loudly instead of being silently dropped.
2. Add an integration test for MARK round-trip over UDS once the fixture is fixed, so the wire-shape change has end-to-end coverage rather than only unit coverage.

Nice-to-haves:

• Track SandboxStdin::EmailJson deletion as a follow-up — the variant is dead now.
• Consider returning a boxed error directly from reject_removed_stdin_field to drop the explicit .map_err(...) at the call site.

Re-review — APPROVED

All previously flagged issues are resolved. No new issues introduced.

Resolved

Blocker (fixed): tests/uds_authz.rs:106 no longer emits Folder: inbox on the MARK fixture. The acceptance grep rg -n 'MarkFolder|Folder:\b' src/ tests/ returns zero hits — verified locally. The integration matrix can now legitimately exercise authz for mark_*_as_other_forbidden (returns EACCES, not ParseError) and mark_*_as_owner_ok / _as_root_any_ok (passes authz on a valid frame).

Non-blocker (fixed): EmailMarkParams now carries #[serde(deny_unknown_fields)]. A stale folder arg from an unrefreshed agent surfaces as an unknown field parse error instead of being silently dropped. Two unit tests added (email_mark_params_rejects_stale_folder_arg, email_mark_params_accepts_canonical_shape) — both pass and pin the canonical shape and the rejection path. This is symmetric with how hook_create rejects stdin. Good cleanup.

Tabled with valid rationale

Integration round-trip test for MARK over UDS. Implementer pointed at tests/integration.rs::mcp_email_mark_read_unread, which spawns the real daemon, drives email_mark_read / email_mark_unread over stdio MCP routing through UDS, and asserts the on-disk frontmatter read = true / read = false toggle round-trips. Confirmed end-to-end: that test does cover the new wire shape via the actual daemon. No new fixture needed. Accepted.

Nice-to-haves still open (not blocking)

• SandboxStdin::EmailJson deletion in src/platform.rs — dead variant, deferred.
• reject_removed_stdin_field boxed-error tidy-up — cosmetic.

Both are appropriate to defer; they were nice-to-haves in the original review.

Verification

• rg -n 'MarkFolder|Folder:\b' src/ tests/ → zero hits.
• cargo test → 1057 tests pass (965 binary + 91 integration + suites).
• cargo test --bin aimx email_mark_params → both new unit tests pass.
• cargo test --test integration mcp_email_mark_read_unread → pass.
• cargo clippy --all-targets -- -D warnings → clean.
• cargo fmt -- --check → clean.
• Planning-doc cross-reference scan → zero hits outside docs/.

The fixup commit is exactly as advertised — the diff is the two-line fixture change plus the deny_unknown_fields annotation and the symmetric tests. No collateral damage.

Recommended merge commit message

[Sprint 3] MARK folder removal + hook_create stdin removal (#180)

Two parallel surface narrows that go down to the codec layer with no
shim:

MARK folder removal: delete `MarkFolder` enum and `MarkRequest.folder`;
codec stops emitting `Folder:` and rejects the header with
`ParseError::Malformed`. `state_handler` hard-codes the inbox path. MCP
`EmailMarkParams` loses `folder` and gains `#[serde(deny_unknown_fields)]`
so a stale arg fails loudly. Tool descriptions note sent-mail mark is
unsupported.

hook_create stdin removal: delete `HookStdin` enum and `Hook.stdin`;
`build_stdin` collapses to always-pipe-the-md. CLI `--stdin`, MCP
`HookCreateParams.stdin`, and the UDS body parser's `stdin` key are
gone. UDS pre-screen rejects any `stdin` value with a unified "always
piped" hint.

Config::load: peek the parsed `toml::Value` before `from_value` and
reject any `[[mailboxes.<name>.hooks]]` block carrying `stdin` with the
canonical wording, naming the hook by explicit `name` if set or by the
derived sha256 effective name.

Docs sweep: agent primer, references/hooks.md, mcp-tools.md, every
per-agent SKILL.md.header, and book chapters (hooks, hook-recipes,
configuration, cli, troubleshooting, mcp) drop `stdin` and `folder`;
the primer adds the verbatim selectivity guidance, mirrored to
book/hooks.md and book/configuration.md.