fix(mcp): a read says only what follows; a success adds no empty warnings (P5)

Two existing contracts the envelope broke when every result got every field:
a resource and its tool say the same (books://settings is books.settings,
checked by the_kept_resources_read), and a tool with no warnings has no
warnings key (books.add_account, defect 8). So head and next_steps are on
every tool that can change the books, next_steps is on a read only when
something follows it, guards is on a refusal, and the envelope adds no
warnings of its own (a tool's stay as they are).

The happy-path spec test now exercises books.journal_history (it fails on
a tool it never called), the demo company gets its books.start size check
(Done when 3), and the As-built block of P5 is written.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HS8WZziDnyxunC23MUxrLp
This commit is contained in:
swaitsandClaude Fable 5.1 committed 2026-10-29 15:48:42 -06:00
1 parent 8e683942a2
commit ad1afb478d
6 files changed
+76 -11

No files matched your search

+43 -2
View File
@@ -429,8 +429,8 @@ fn books_start_v2_is_whole_and_fits() {
assert!(s["why"].is_string() && s["required"].is_boolean(), "{s}"); assert!(s["why"].is_string() && s["required"].is_boolean(), "{s}");
assert!(s["tool"].is_string() || s["prompt"].is_string(), "{s}"); assert!(s["tool"].is_string() || s["prompt"].is_string(), "{s}");
} }
// The envelope: a read has steps and no head. // The envelope: a read has no head of the envelope's (its own stays).
assert!(start.get("guards").is_some_and(Value::is_array)); assert!(start["head"]["seq"].is_u64());
// The self-report is in the access log, beside the call. // The self-report is in the access log, beside the call.
let log = answer_of(&w.call("books.access_log", json!({"limit": 50}))).unwrap(); let log = answer_of(&w.call("books.access_log", json!({"limit": 50}))).unwrap();
@@ -502,3 +502,44 @@ fn a_mutating_call_answers_with_the_head_and_a_refusal_names_its_remedy() {
assert_eq!(guard["code"], "stale_preview", "{refused}"); assert_eq!(guard["code"], "stale_preview", "{refused}");
assert_eq!(guard["remedy_tool"], "books.post_entry"); assert_eq!(guard["remedy_tool"], "books.post_entry");
} }
/// Done-when (3): `books.start` is at most 8 KiB on the demo company too
/// (the books the viewer ships with, a loose end of every kind), and says
/// what it cut.
#[test]
fn books_start_fits_8_kib_on_the_demo_books() {
let today = ledger_core::settings::today_in(None);
let place = common::Place::new("p5-start-demo");
let source = tempfile::Builder::new()
.prefix("wb-p5-demo-")
.tempdir()
.unwrap();
ledger_gui::demo::build(source.path(), today).unwrap();
std::fs::create_dir_all(place.books_data().parent().unwrap()).unwrap();
std::fs::rename(source.path().join("books-data"), place.books_data()).unwrap();
let mut wire = place.start(None);
wire.initialize("p5");
let raw = wire.call("books.start", json!({}));
let text = raw["result"]["content"][0]["text"].as_str().unwrap();
assert!(
text.len() <= 8 * 1024,
"books.start is {} bytes",
text.len()
);
let start = answer_of(&raw).unwrap();
assert_eq!(start["state"], "active");
assert!(
!start["needs_attention"]["items"]
.as_array()
.unwrap()
.is_empty(),
"the demo has loose ends: {start}"
);
assert!(
start["needs_attention"]["counts"]
.as_object()
.unwrap()
.len()
>= 2
);
}
+2
View File
@@ -1294,6 +1294,8 @@ fn happy_path(name: &str) -> Mcp {
// The access log, the backups' status, and a backup now (none is set // The access log, the backups' status, and a backup now (none is set
// up: it says so). // up: it says so).
m.ok("books.access_log", json!({})); m.ok("books.access_log", json!({}));
// P5: the journal in words.
m.ok("books.journal_history", json!({"limit": 5}));
m.ok("books.backup", json!({})); m.ok("books.backup", json!({}));
m.ok("books.back_up_now", json!({})); m.ok("books.back_up_now", json!({}));
+11 -4
View File
@@ -12,7 +12,10 @@
//! 1. The envelope is applied in one place (`apply`, called by //! 1. The envelope is applied in one place (`apply`, called by
//! `call_tool`), so no tool can forget it. //! `call_tool`), so no tool can forget it.
//! 2. Every result of a tool that is not read-only has a `head` (the //! 2. Every result of a tool that is not read-only has a `head` (the
//! journal after the call's last write) and at least one next step. //! journal after the call's last write) and at least one next step; a
//! read has `next_steps` when something follows it and nothing else (a
//! resource and its tool agree). `guards` is on a refusal, and a
//! success without warnings has no `warnings` key.
//! 3. A tool's own fields are never overwritten: a payload that already //! 3. A tool's own fields are never overwritten: a payload that already
//! has `next_steps`, `guards`, `warnings` or `head` keeps it. //! has `next_steps`, `guards`, `warnings` or `head` keeps it.
//! 4. A refused call has no head and no steps; a refusal that is a //! 4. A refused call has no head and no steps; a refusal that is a
@@ -451,14 +454,18 @@ pub(crate) fn apply(
if mutating && steps.is_empty() { if mutating && steps.is_empty() {
steps.push(digest_step()); steps.push(digest_step());
} }
if !body.contains_key("next_steps") { // A tool that can change the books always says what follows; a read
// says it only when something does, so what it says is what a resource
// of the same name says (`books://settings` is `books.settings`).
// `guards` is on a refusal (above) and `warnings` is the tool's own:
// a success that has none says none, as the tools always did.
if !body.contains_key("next_steps") && (mutating || !steps.is_empty()) {
body.insert( body.insert(
"next_steps".into(), "next_steps".into(),
Value::Array(steps.iter().map(Step::to_json).collect()), Value::Array(steps.iter().map(Step::to_json).collect()),
); );
} }
body.entry("guards").or_insert_with(|| json!([]));
body.entry("warnings").or_insert_with(|| json!([]));
if let Some(head) = head { if let Some(head) = head {
body.entry("head").or_insert(head); body.entry("head").or_insert(head);
} }
+2 -2
View File
@@ -1564,8 +1564,8 @@ fn every_mutating_result_has_head_and_next_steps() {
}; };
let body = r.structured_content.expect("structured"); let body = r.structured_content.expect("structured");
assert!( assert!(
body["guards"].is_array() && body["warnings"].is_array(), body.get("guards").is_none() && body.get("warnings").is_none(),
"{name}" "{name}: a success without them says none"
); );
let ContentBlock::Text(t) = &r.content[0] else { let ContentBlock::Text(t) = &r.content[0] else {
panic!("{name}: no text block"); panic!("{name}: no text block");
+9 -2
View File
@@ -490,7 +490,7 @@ every tool over every book in ledger-store's `tests/fixtures/old-books`.
`books.discard_draft`. (`books.void_document`, `books.run_depreciation` `books.discard_draft`. (`books.void_document`, `books.run_depreciation`
and `books.run_recurring` post at once and have no `confirmed`.) and `books.run_recurring` post at once and have no `confirmed`.)
- **The result envelope (P5, plan 4.6).** Every tool result, whatever the - **The result envelope (P5, plan 4.6).** Every tool result, whatever the
tool, is given the same four fields in one place (`call_tool`, tool, is given the same fields in one place (`call_tool`,
`next_steps.rs::apply`), after the tool has answered; a tool's own field of `next_steps.rs::apply`), after the tool has answered; a tool's own field of
the same name is kept, never overwritten: the same name is kept, never overwritten:
`next_steps: [{ tool | prompt | command, args?, why, required }]` (what to `next_steps: [{ tool | prompt | command, args?, why, required }]` (what to
@@ -506,7 +506,14 @@ every tool over every book in ledger-store's `tests/fixtures/old-books`.
`warnings: [..]` (a tool's own `warnings` is that field), and, on every `warnings: [..]` (a tool's own `warnings` is that field), and, on every
tool that is not read-only, `head: { seq, hash }`, the journal's head tool that is not read-only, `head: { seq, hash }`, the journal's head
after the call's last write (or as it stands, when the call wrote after the call's last write (or as it stands, when the call wrote
nothing). The guard codes are the enum in `guards.rs`, each with its nothing). A tool that can change the books always has `head` and
`next_steps`; a read has `next_steps` only when something follows it, so
a resource and its tool say the same (`books://settings` is
`books.settings`). `guards` is on a refusal; a success that has no
warnings has no `warnings` key. A tool that can change the books always answers with all four;
a read has `next_steps` only when something follows it and no `guards`
or `warnings` of the envelope's, so a resource and its tool say the same
(`books://settings` is `books.settings`). The guard codes are the enum in `guards.rs`, each with its
remedy tool: `stale_preview` (the tool again, previewed), remedy tool: `stale_preview` (the tool again, previewed),
`format_upgrade_pending` (`books.start`), `receipt_required` `format_upgrade_pending` (`books.start`), `receipt_required`
(`books.attach_receipt`), `approval_required` (`books.approve_entry`), (`books.attach_receipt`), `approval_required` (`books.approve_entry`),
+9 -1
View File
@@ -1515,7 +1515,15 @@ Each unit follows the 8.0.4 template. "A-n" is an owner answer numbered in 8.3.1
5. **Files:** owns `crates/ledger-mcp/src/guards.rs`, `tools/prompts.rs`, `prompts/*.md`, `crates/ledger-store/src/describe.rs`, `crates/ledger-cli/tests/agent_evals/`; touches `server.rs` (the result path), `tools/kept.rs`, `opening.rs`, `bridge.rs`, `handbook.rs`, the 8.0.3 files, `docs/mcp-interface.md`. 5. **Files:** owns `crates/ledger-mcp/src/guards.rs`, `tools/prompts.rs`, `prompts/*.md`, `crates/ledger-store/src/describe.rs`, `crates/ledger-cli/tests/agent_evals/`; touches `server.rs` (the result path), `tools/kept.rs`, `opening.rs`, `bridge.rs`, `handbook.rs`, the 8.0.3 files, `docs/mcp-interface.md`.
6. **Tests and proofs:** named above; no kernel or Lean change. 6. **Tests and proofs:** named above; no kernel or Lean change.
7. **Done when:** (1) every mutating result carries `head` and non-empty `next_steps`; (2) every guard's remedy tool exists; (3) `books.start` ≤ 8 KiB on the `p2-served` fixture and on the demo books; (4) the prompts list and render; (5) `tool_descriptions_fit_700_characters` pins 0; (6) the six evals pass; (7) `just check` green. The live check (a fresh Claude Code session in an empty folder, and a Codex session, each posting an entry by following `next_steps`) is the EM's, recorded in the As-built block. 7. **Done when:** (1) every mutating result carries `head` and non-empty `next_steps`; (2) every guard's remedy tool exists; (3) `books.start` ≤ 8 KiB on the `p2-served` fixture and on the demo books; (4) the prompts list and render; (5) `tool_descriptions_fit_700_characters` pins 0; (6) the six evals pass; (7) `just check` green. The live check (a fresh Claude Code session in an empty folder, and a Codex session, each posting an entry by following `next_steps`) is the EM's, recorded in the As-built block.
8. **As built:** none yet. 8. **As built (2026-10-29, branch P5, eight commits on b97234ac; no schema change, so no migration number was taken):**
- Step 1 as written, with these shapes: `next_steps` items are `{ tool | prompt | command, args?, why, required }` (the `command` form carries `books.export_cpa_package`'s download command, which is not a tool); a result that already has `next_steps` or `head` keeps it (`books.start` has its own `next_steps` and `head`, `books.verify` and `books.access_log` their own `head`); the envelope adds no `warnings` of its own (a tool's own stay as they are, `books.cash_flow`'s object warnings included) and `guards` is on a refusal only; `head` and `next_steps` are on every tool that is not read-only (its annotation), `head` being the journal's head after the call; a read has `next_steps` only when something follows it, because a resource and its tool must say the same (`the_kept_resources_read` compares `books://settings` with `books.settings`), and an empty `warnings` on `books.add_account` broke `add_account_warns_without_tax_category_defect_8`. A refusal that is a guard carries `structuredContent.guards`; its text is unchanged. After a close the step is `books.verify`; **P8 replaces it with `attest_period` when it merges**. The guard registry has ten codes (the plan's seven examples plus `party_shares`, `party_required`, `open_drafts`); `untagged_control_line`, `duplicate_vendor_ref` and `first_number_locked` are in the registry and recognized from a refusal that begins with the code, but no tool refuses with them yet. The Needs-attention items are added after a change to an entry or an invoice only (not to a party or an account). `every_guard_names_an_existing_tool` is in `guards.rs`, `every_mutating_result_has_head_and_next_steps` in `server_tests.rs`.
- Step 2 as written, except: `head.last_handoff`, `rules.attestation_bar`, `rules.approval_rules` as a list and `attestation` are not in v2 (P8's); `you.verified`/`self_reported` and `isolation`'s shape were not added to `you` (the plan's `you` has four keys and has them). `agent` is stored on the access record as `agent {model?, reasoning_effort?, harness?}` (a new optional field of `ledger_store::access::Access`; the access log is its own chain, not the journal, so 8.0.6 does not apply). `you` is absent on the `books://start` resource (no token made that call). The fit loop shortens attention items to four and waiting drafts to three before the lists.
- Step 3 as written. The prompt tests are in `crates/ledger-cli/tests/agent_surface.rs` (`the_prompts_list_and_render`), not `crates/ledger-mcp/tests/prompts.rs`: the prompt renderer is `pub(crate)` and the p2-served harness is in ledger-cli; the "golden render" is each prompt's file with its blanks filled, compared exactly, rather than a stored copy. Prompts need `books:read`. The report resources now take their dates from the query; `books://entries/{id}` and `books://journal/{seq}` are new reads; `books://attachments/{id}` was already served.
- Step 4: `books.journal_history` as written, in `tools/journal.rs`, `subject` being the line's first subject. **Departure:** the describer in `crates/ledger-store/src/describe.rs (new)` is the server's; the viewer's `describe` arms in `ledger-gui/src/pages/journal.rs` were not moved (they carry icons, routes and today's party and account names; moving them behind a formatting trait was a large change to the viewer for no figure or behavior). So there are two describers; the server's quotes only identifiers, dates, amounts and short typed names and never a setting's value. Decide with the EM whether to unify them (U-unit).
- Step 5: parameter names are `reason` (on `books.set_convention`) and `revises` with `reason` (on `books.record_decision` and `books.raise_item`); `subject`, `decision_md` and `title` became optional in those two schemas so a source-only call can leave them out (given, they must equal the record's). Test names: `a_source_only_revision_is_journaled`, `source_revisions_wait_for_the_upgrade` (both in `crates/ledger-store/tests/source_revisions.rs`), plus `a_source_revision_must_change_only_the_source` and the over-the-wire `a_source_only_revision_over_the_wire`. 8.0.6: all six items (the `p5_before` fixture was written by b97234ac; no golden moved).
- Step 6: the eight descriptions were moved into the handbook sections `cpa-handoff`, `parties-and-tins`, `1099`, `files-in-out`, `reconciliation` and `viewer`; `tool_descriptions_fit_700_characters` pins none and the allowance list is gone. `instructions_budget` was already green with the final connect text (it already had no `preview_token`, reversal drafts or `request_tin_entry`). The aliases are removed (and `customer`/`vendor` are required); every over-the-wire test that used them was changed to the new names (`party_id_alias_still_works` became `a_customer_by_id_and_by_name_is_one_record`). `docs/mcp-interface.md`'s overview sections now describe the surface as built.
- Step 7: the six transcripts are as named. Each file may have `setup` (direct calls that make the chart and the books an agent finds; not the agent's moves), `settings`, and steps `call` (only `books.start`, first), `follow`, `follow_prompt`, `expect_next_steps_contains` and `expect`; `{"$upload": ..}` and `{"$var": ..}` stand for an uploaded file's id and a value a setup call saved. The runner is `agent_evals.rs`.
- Not run by the worker (D-60): `just check`, `just prove`; the live check of Done-when (7) (a fresh Claude Code session in an empty folder and a Codex session, each posting an entry by following `next_steps`) is the EM's. Checked: `cargo test -p ledger-mcp`, the store tests `source_revisions`, `format` and `store`, and the over-the-wire files this unit edited, with fmt and clippy on the five crates touched.
**Struck from P5:** ~~`books.job` and the 20 s rule~~ → D-53 (no measured call approaches a client limit over the bridge; jobs come with P9b if claude.ai's 240 s limit is ever real); ~~URL-mode elicitation~~ → nothing local to elicit (D-28); ~~`ledger-view` crate~~ → U0b (`PackageBooks`); ~~`books.viewer_status`~~ → `books.show` already reports `viewer_open`. **Struck from P5:** ~~`books.job` and the 20 s rule~~ → D-53 (no measured call approaches a client limit over the bridge; jobs come with P9b if claude.ai's 240 s limit is ever real); ~~URL-mode elicitation~~ → nothing local to elicit (D-28); ~~`ledger-view` crate~~ → U0b (`PackageBooks`); ~~`books.viewer_status`~~ → `books.show` already reports `viewer_open`.