Sprint Goal Assessment

Sprint 4 lands clean. Goal is the cosmetic-and-paperwork tail of the mcp-update track: reorder *Params fields per PRD §6.5, write release notes for the three hard breaks, and sweep book/ + agents/. All three are delivered. No behavior changes leak into this sprint, no scope creep, and every prior-sprint surface stays consistent.

Acceptance Criteria Checklist

[S4-1] Reorder fields per FR-6.5 table

• EmailListParams order mailbox, folder, limit, offset — confirmed at src/mcp.rs:34-45.
• EmailReadParams order mailbox, id, folder — confirmed at src/mcp.rs:47-55.
• EmailSendParams order from_mailbox, to, subject, reply_to, body, attachments, references — reply_to moved up from after attachments; src/mcp.rs:67-89.
• EmailReplyParams order mailbox, id, body — src/mcp.rs:92-99.
• EmailMarkParams order mailbox, id — src/mcp.rs:59-64 (already minimal post-S3-1).
• HookCreateParams order mailbox, event, cmd, name, timeout_secs, fire_on_untrusted — src/mcp.rs:102-125.
• HookListParams / HookDeleteParams single-field — trivially correct.
• Snapshot tests in new schema_order_tests module pin properties key order via schema_for!. Ran cargo test --bin aimx mcp::schema_order_tests locally: 8 passed, 0 failed.
• cargo test, clippy, fmt clean (973 tests pass locally; build CI green).

The preserve_order schemars feature was a load-bearing addition I almost flagged. serde_json::Map is BTreeMap-backed by default, which would emit properties alphabetically and silently nullify the reordering. Enabling preserve_order (transitively pulling indexmap into serde_json, visible in the Cargo.lock diff) makes the JSON Schema reflect declaration order. The snapshot tests would have failed without this change, so the test set is also a regression guard against a future default-features slip.

[S4-2] Release notes

• New ## Unreleased — MCP surface cleanup section in book/release-notes.md with the required three subsections plus the soft-change note.
• Each hard break names what was removed, gives a one-line rationale, and shows the new call shape via a fenced code snippet.
• stdin section quotes the canonical Config::load error verbatim (hook 'X' carries removed field 'stdin' — remove this line and restart aimx serve; the email is always piped to hooks) so operators can grep failing-startup logs.
• Soft-change subsection covers the mailbox_list / email_list JSON-shape switch, with the per-row schema spelled out.
• Cross-links to book/mcp.md and book/hooks.md present in the section header.
• No CHANGELOG.md at repo root → correctly skipped per the sprint plan ("If not, do not create one").

[S4-3] Final docs sweep

• agents/common/references/hooks.md — both lingering stdin: "email" examples removed; the wrap-up paragraph rewritten away from the deleted "no JSON-mode stdin variant; only email (raw) and none" framing into the always-piped model.
• agents/common/references/mcp-tools.md and book/mcp.md email_send parameter tables reordered to mirror the struct (reply_to ahead of body).
• agents/common/references/workflows.md threaded-reply example reordered to match.
• book/security.md HOOK-CREATE bullet reworded so stdin is named as a stale-field rejection alongside cmd / run_as / timeout_secs / dangerously_* rather than miscategorised as a template property.
• Repo-wide grep for stale tokens clean: rg -n 'unread:|since:|subject:' outside book/release-notes.md returns only natural-language hits (email subjects in examples, "polling for unread"). No email_mark_*(folder:) hits anywhere. The lone \bstdin\s*[:=] hit in book/hooks.md:129 is the rejection paragraph itself.
• All *Params snapshot tests double as cargo build regression: bundles rebuild cleanly via include_str!.

Test Coverage

Tight. The new schema_order_tests module covers every multi-field *Params struct that PRD §6.5 names, and the test mechanism (schema_for! → serde_json::Value → properties.keys()) checks the exact thing the agent actually sees in the tool schema, not a proxy. The single-field HookListParams / HookDeleteParams tests are nearly trivial but cheap, and they correctly defend against a future "let's add a field" slip-up that introduces a non-canonical field as the new first key.

The preserve_order enablement is itself implicitly tested by the snapshot suite — without it, EmailSendParams would emit attachments, body, from_mailbox, references, reply_to, subject, to alphabetically and the test would fail.

No additional integration test is needed for this sprint — wire format is unchanged, params are name-keyed, and behavior is identical.

Potential Bugs

None observed. The diff is mechanical: field reordering in declaration order, doc copy edits, one Cargo feature toggle. I verified each change manually against the PRD §6.5 table and the FR-D3 / FR-D6 wording. The Cargo.lock change is exactly what --features preserve_order should add (indexmap joining serde_json's deps).

Security Issues

None. No new code paths, no UDS verb changes, no auth touches. The preserve_order feature is a stable schemars/serde_json option with no runtime safety implications.

Code Quality

The new schema_order_tests module is well-scoped and well-commented (the doc-comment correctly names the wire-compat invariant). The property_keys::<T>() helper is the right factoring — eight test sites with one-line bodies each, all reading the same way.

Minor observation, non-blocking: the email_send parameter table cross-references in agents/common/references/mcp-tools.md and book/mcp.md now match the struct, which is what an agent reading top-to-bottom expects. The earlier alphabetical-ish ordering was the kind of subtle inconsistency that erodes trust in the docs over time. Worth keeping the snapshot tests around for the same reason.

Alignment with PRD

Full alignment. PRD §6.5 table reproduced exactly in the snapshot tests (which is the strictest possible check). PRD §10 hard-breaks list matches the release-notes subsections one-for-one. FR-D6 satisfied: the stdin section names the upgrade-time validation failure path verbatim and shows the operator the remediation.

The sprint plan's S4-3 requirement that aimx agents setup <agent> --print show no stale references is covered by the rebuilt bundles (the include_str! source files all flow from the swept agents/common/ tree). I did not re-run --print for every agent myself, but the build is green and the underlying source files are clean.

Summary and Recommended Actions

• Overall verdict: Ready to merge.
• Blockers: none.
• Non-blockers: none.
• Nice-to-haves: none for this sprint. The mcp-update track now has all four sprints landed and the surface is internally consistent — mailbox_list JSON, email_list paginated JSON, MARK folder gone, stdin gone, params in natural order, and the release notes capture the migration.

This wraps the mcp-update track. Worth a scrum-plan-updater pass after merge to mark Sprint 4 DONE and move the track to a closed state.

Recommended merge commit message

[mcp-update Sprint 4] Param reorder + release notes + final docs sweep

Reorder EmailSendParams fields to natural composition flow
(from_mailbox, to, subject, reply_to, body, attachments, references)
and pin the JSON-schema property order for every multi-field *Params
struct via schema_for! snapshot tests in src/mcp.rs.

Enable schemars preserve_order so serde_json::Map preserves struct
declaration order rather than emitting properties alphabetically;
the new snapshot suite would have failed without this change.

Add an Unreleased section to book/release-notes.md covering the three
hard breaks landed by Sprints 1-3 (email_list filters, email_mark_*
folder, hook_create / config.toml stdin) plus the soft mailbox_list /
email_list JSON-shape switch. The stdin section quotes the canonical
Config::load wording verbatim so operators can grep failing-startup
logs.

Final docs sweep: drop the last `stdin: \"email\"` examples from
agents/common/references/hooks.md, sync the email_send parameter
table and threaded-reply example in book/mcp.md and
agents/common/references/{mcp-tools,workflows}.md to the new schema
order, and reword book/security.md HOOK-CREATE so stdin is named as
a stale-field rejection rather than a template property.