From 95d883b1bee21f39ebd99e6601127c086f2084ba Mon Sep 17 00:00:00 2001 From: Joachim Rosskopf Date: Sat, 15 Aug 2026 09:45:15 +0200 Subject: [PATCH] feat(client): typed CAS/approve guards, expand guard fields, list_instances cursor, as_of/scenario plumbing Contract-parity fixes: the typed crates lagged the server wire surface, so typed consumers could not use documented features. - UpdatePageRequest gains the optional-with-meaning write guards the server has accepted since #246/#354: base_version, require_exact_base, base_sha256 (Some("") = approve-create sentinel, serialized as the explicit empty string; None = unguarded, omitted from the wire) and the provenance passthrough. UpdatePageResponse gains auto_merged + head_version/head_sha256/head_content so a typed caller can act on a conflict refusal. - ExpandResponse gains version + content_sha256 (serde-default, absent-tolerant), closing the read->hash->guarded-write loop end to end through escurel-client. - ListInstancesRequest gains cursor; the client forwards it and the documented "only an absent next_cursor means done" resume works. - The client stopped dropping as_of/scenario on expand / neighbours / list_instances (and scenario on resolve) - the typed requests carried them all along but they never reached the wire. Skill sync (same PR): references/05-consume-from-rust.md documents the guarded-write recipe + cursor loop; skill CHANGELOG 0.6.27 + VERSION. Co-Authored-By: Claude Fable 5 --- .claude/skills/escurel-platform/CHANGELOG.md | 18 + .claude/skills/escurel-platform/VERSION | 2 +- .../references/05-consume-from-rust.md | 34 +- crates/escurel-cli/src/agent.rs | 6 +- crates/escurel-cli/src/workflow.rs | 2 + crates/escurel-client/src/lib.rs | 69 ++- .../escurel-client/tests/client_roundtrip.rs | 1 + .../escurel-client/tests/contract_parity.rs | 451 ++++++++++++++++++ .../escurel-server/tests/suite/meta_skill.rs | 2 + .../tests/suite/page_write_events.rs | 2 + crates/escurel-types/src/agent.rs | 80 +++- crates/escurel-types/tests/wire_contract.rs | 140 ++++++ 12 files changed, 791 insertions(+), 16 deletions(-) create mode 100644 crates/escurel-client/tests/contract_parity.rs diff --git a/.claude/skills/escurel-platform/CHANGELOG.md b/.claude/skills/escurel-platform/CHANGELOG.md index 7195d92b..74264f3b 100644 --- a/.claude/skills/escurel-platform/CHANGELOG.md +++ b/.claude/skills/escurel-platform/CHANGELOG.md @@ -4,6 +4,24 @@ The skill version tracks the consumer-facing contract, not the Escurel binary version. The Escurel repo's checked-out git ref is the true version pin (see `SKILL.md` → "How this skill is installed"). +## 0.6.29 — the typed Rust client catches up with the wire guards + +- `escurel-client` / `escurel-types` contract parity + (`references/05-consume-from-rust.md`): `UpdatePageRequest` now + carries the CAS/approve guards (`base_version`, `require_exact_base`, + `base_sha256` — `Some("")` = approve-create — and the `provenance` + passthrough); `UpdatePageResponse` gained `auto_merged` + + `head_version`/`head_sha256`/`head_content`; `ExpandResponse` gained + `version` + `content_sha256`, so the full read→hash→guarded-write + loop is typed end to end. Absent guard = unguarded, byte-identical to + the old client's wire traffic. +- `ListInstancesRequest.cursor` + client plumbing: `list_instances` + now resumes from `next_cursor` (only its absence means done). +- The client stopped dropping `as_of`/`scenario` on + `expand`/`neighbours`/`list_instances` (and `scenario` on `resolve`) + — they were on the typed requests all along but never reached the + wire, silently returning current/base state. + ## 0.6.28 — chat cursor errors are typed; reader event tools over the lake - `list_messages` with an undecodable `cursor` now answers diff --git a/.claude/skills/escurel-platform/VERSION b/.claude/skills/escurel-platform/VERSION index a918a2aa..c1e4fc80 100644 --- a/.claude/skills/escurel-platform/VERSION +++ b/.claude/skills/escurel-platform/VERSION @@ -1 +1 @@ -0.6.0 +0.6.27 diff --git a/.claude/skills/escurel-platform/references/05-consume-from-rust.md b/.claude/skills/escurel-platform/references/05-consume-from-rust.md index e6e020c7..5c0bb149 100644 --- a/.claude/skills/escurel-platform/references/05-consume-from-rust.md +++ b/.claude/skills/escurel-platform/references/05-consume-from-rust.md @@ -51,7 +51,7 @@ client.list_instances(ListInstancesRequest { skill: "customer".into(), ..Default // provenance_path(...) still exists but calls provenance_ancestry with // `to_page` (ProvenanceAncestryRequest gained that field) under the hood. client.provenance_report(ProvenanceReportRequest { kind: "drift".into(), ..Default::default() }).await?; -client.update_page(UpdatePageRequest { page_id, content }).await?; +client.update_page(UpdatePageRequest { page_id, content, ..Default::default() }).await?; // Chat history (M-Chat, issue #63): append-mostly log keyed by an // opaque chat_group_id. See `references/02` §Chat tools. client.append_message(AppendMessageRequest { @@ -69,6 +69,38 @@ client.list_messages(ListMessagesRequest { }).await?; ``` +The read methods forward the optional-with-meaning wire fields: `as_of` +(time-travel cut) and `scenario` (overlay) on +`expand`/`neighbours`/`list_instances`/`search` (+ `scenario` on +`resolve`) are sent when non-empty — empty = current base state. +`list_instances` paginates: pass `cursor` from the previous response's +`next_cursor`; **only an absent `next_cursor` means done** (a string +always means more rows, even on a short page). + +### Guarded writes (the read→hash→approve loop) + +`expand` (plain reads only) returns `content_sha256` — the hash of the +stored markdown bytes — and, on a live-CRDT gateway, `version`. +`UpdatePageRequest` carries the matching guards; all default to absent = +unguarded, so plain upserts are unchanged: + +```rust +let read = client.expand(ExpandRequest { page_id: page_id.clone(), ..Default::default() }).await?; +let resp = client.update_page(UpdatePageRequest { + page_id, + content: redraft, + base_sha256: read.content_sha256, // content-hash CAS (#354): works on EVERY gateway + // base_version: read.version, // version CAS (#246): live-CRDT only; stale → auto-merge + // require_exact_base: true, // strict: stale base → conflict, never a merge (approvals) + // base_sha256: Some(String::new()), // Some("") = approve-create ("I expect no page yet") + ..Default::default() +}).await?; +if !resp.ok && resp.issues.iter().any(|i| i.code == "conflict") { + // resp.head_sha256 / resp.head_content / resp.head_version: re-diff + // against head and retry. resp.auto_merged reports a landed merge. +} +``` + Field names follow the wire contract (`q`/`k`, not `query`/`top_k`); JSON-bearing fields (`frontmatter`, `rows`, `params`) are typed `serde_json::Value`s — real JSON, not encoded strings. See `crates/escurel-types/src/` and diff --git a/crates/escurel-cli/src/agent.rs b/crates/escurel-cli/src/agent.rs index 7b0da760..37cd6dba 100644 --- a/crates/escurel-cli/src/agent.rs +++ b/crates/escurel-cli/src/agent.rs @@ -561,7 +561,11 @@ async fn validate(client: &Client, page_id: String) -> Result { async fn update_page(client: &Client, page_id: String) -> Result { let content = read_stdin("page body")?; let resp = client - .update_page(UpdatePageRequest { page_id, content }) + .update_page(UpdatePageRequest { + page_id, + content, + ..Default::default() + }) .await?; Ok(json!({ "ok": resp.ok, diff --git a/crates/escurel-cli/src/workflow.rs b/crates/escurel-cli/src/workflow.rs index 356f3a6b..2c9904af 100644 --- a/crates/escurel-cli/src/workflow.rs +++ b/crates/escurel-cli/src/workflow.rs @@ -76,6 +76,7 @@ async fn invoke(client: &Client, a: RunArgs) -> Result { .update_page(UpdatePageRequest { page_id: page.clone(), content: board_markdown(&run_id, &a.skill, "running"), + ..Default::default() }) .await .context("create the run board")?; @@ -192,6 +193,7 @@ async fn stop(client: &Client, run_id: &str) -> Result { .update_page(UpdatePageRequest { page_id: page.clone(), content: board_markdown(run_id, &wf_skill, "stopped"), + ..Default::default() }) .await .context("mark the run stopped")?; diff --git a/crates/escurel-client/src/lib.rs b/crates/escurel-client/src/lib.rs index 86cf9ba2..51d36c6a 100644 --- a/crates/escurel-client/src/lib.rs +++ b/crates/escurel-client/src/lib.rs @@ -166,12 +166,18 @@ impl Client { /// Parse a `[[wikilink]]` and look up its target page. pub async fn resolve(&self, req: ResolveRequest) -> Result { - self.transport - .call_typed("resolve", json!({ "wikilink": req.wikilink })) - .await + let mut args = json!({ "wikilink": req.wikilink }); + if !req.scenario.is_empty() { + args["scenario"] = json!(req.scenario); + } + self.transport.call_typed("resolve", args).await } /// Fetch a page's frontmatter, body, and outbound wikilinks. + /// + /// A plain read (no `as_of`/`scenario`) also returns the guard pair + /// for the read→hash→guarded-write loop: `content_sha256` (always, + /// on a server ≥ #408) and `version` (live-CRDT gateways). pub async fn expand(&self, req: ExpandRequest) -> Result { let mut args = json!({ "page_id": req.page_id }); if !req.anchor.is_empty() { @@ -180,6 +186,14 @@ impl Client { if !req.version.is_empty() { args["version"] = json!(req.version); } + // Time-travel cut and scenario overlay — optional-with-meaning: + // omitting them silently read the current base state. + if !req.as_of.is_empty() { + args["as_of"] = json!(req.as_of); + } + if !req.scenario.is_empty() { + args["scenario"] = json!(req.scenario); + } if req.full { args["full"] = json!(true); } @@ -195,6 +209,12 @@ impl Client { if !req.link_skill.is_empty() { args["link_skill"] = json!(req.link_skill); } + if !req.as_of.is_empty() { + args["as_of"] = json!(req.as_of); + } + if !req.scenario.is_empty() { + args["scenario"] = json!(req.scenario); + } self.transport.call_typed("neighbours", args).await } @@ -275,6 +295,19 @@ impl Client { args["frontmatter_key"] = json!(req.frontmatter_key); args["frontmatter_value"] = json!(req.frontmatter_value); } + if !req.as_of.is_empty() { + args["as_of"] = json!(req.as_of); + } + if !req.scenario.is_empty() { + args["scenario"] = json!(req.scenario); + } + // Resume cursor from the previous response's `next_cursor`. + // Only an absent/null `next_cursor` means done — a string always + // means more rows (ACL filtering may shorten a page below + // `limit` with rows still to come). + if !req.cursor.is_empty() { + args["cursor"] = json!(req.cursor); + } self.transport.call_typed("list_instances", args).await } @@ -309,13 +342,31 @@ impl Client { } /// Upsert a markdown page (the public write path). + /// + /// The optional guards on [`UpdatePageRequest`] make this the write + /// half of the read→hash→guarded-write loop: `base_version` (#246) + /// is the optimistic-concurrency CAS with CRDT auto-merge, + /// `require_exact_base` makes it strict (approvals), and + /// `base_sha256` (#354) is the content-hash CAS that works on every + /// gateway — `Some("")` approves a create. All default to absent = + /// unguarded, so existing callers are unchanged. pub async fn update_page(&self, req: UpdatePageRequest) -> Result { - self.transport - .call_typed( - "update_page", - json!({ "page_id": req.page_id, "content": req.content }), - ) - .await + let mut args = json!({ "page_id": req.page_id, "content": req.content }); + if let Some(v) = &req.base_version { + args["base_version"] = json!(v); + } + if req.require_exact_base { + args["require_exact_base"] = json!(true); + } + // `Some("")` is the approve-create sentinel and MUST reach the + // wire as the explicit empty string; only `None` is omitted. + if let Some(h) = &req.base_sha256 { + args["base_sha256"] = json!(h); + } + if let Some(p) = &req.provenance { + args["provenance"] = p.clone(); + } + self.transport.call_typed("update_page", args).await } /// Soft-delete (archive) a markdown page (#300). Retracts it from diff --git a/crates/escurel-client/tests/client_roundtrip.rs b/crates/escurel-client/tests/client_roundtrip.rs index a0de0e9e..ee3141c7 100644 --- a/crates/escurel-client/tests/client_roundtrip.rs +++ b/crates/escurel-client/tests/client_roundtrip.rs @@ -234,6 +234,7 @@ name: Globex\n\ .update_page(UpdatePageRequest { page_id: "markdown/instances/customer/globex.md".to_owned(), content: body.to_owned(), + ..Default::default() }) .await .unwrap(); diff --git a/crates/escurel-client/tests/contract_parity.rs b/crates/escurel-client/tests/contract_parity.rs new file mode 100644 index 00000000..37bc5bde --- /dev/null +++ b/crates/escurel-client/tests/contract_parity.rs @@ -0,0 +1,451 @@ +//! Contract-parity tests: the typed client must carry every +//! optional-with-meaning field the server's tools accept and emit. +//! +//! Real gateway via `escurel-test-support` (real DuckDB, real MCP-over-HTTP, +//! real OIDC test issuer) — no mocks at the boundary (CLAUDE principle 2). +//! +//! These pin the drift findings where `escurel-types` / `escurel-client` +//! lagged the wire surface: dropped `as_of`/`scenario` on reads, the +//! missing `list_instances` cursor, and the missing CAS/approve guard on +//! `update_page` (`base_version` / `require_exact_base` / `base_sha256`) +//! plus the guard fields `expand` publishes (`version`, `content_sha256`). + +use escurel_client::{ + Client, ExpandRequest, ListInstancesRequest, NeighboursRequest, ResolveRequest, SearchRequest, + SecretString, UpdatePageRequest, +}; +use escurel_test_support::{AuthMode, ConfigOverrides, EscurelProcess, FixtureBuilder, Opts, Role}; + +const TENANT: &str = "acme"; + +const CUSTOMER_SKILL: &str = "---\n\ +type: skill\n\ +id: customer\n\ +description: A buying organisation.\n\ +required_frontmatter: [id, name]\n\ +optional_frontmatter: [tier, at]\n\ +---\n\ +# customer\n"; + +// The instances carry an `at:` timestamp because the `as_of` time-travel +// cut keys on it (`at_ts <= as_of`; UNTIMED pages always remain — they +// are not events on the timeline). A cut before `at` hides them. +const ACME_INSTANCE: &str = "---\n\ +type: instance\n\ +skill: customer\n\ +id: acme\n\ +name: Acme Corp\n\ +tier: gold\n\ +at: 2024-06-01T00:00:00Z\n\ +---\n\ +# Acme Corp\n\nKey account. See [[customer::initech]].\n"; + +const INITECH_INSTANCE: &str = "---\n\ +type: instance\n\ +skill: customer\n\ +id: initech\n\ +name: Initech\n\ +at: 2024-06-01T00:00:00Z\n\ +---\n\ +# Initech\n"; + +/// An RFC 3339 cut that predates the fixtures' `at:` timestamps. +const BEFORE_EVERYTHING: &str = "2000-01-01T00:00:00Z"; + +fn fixtures() -> FixtureBuilder { + FixtureBuilder::new() + .tenant(TENANT) + .skill("customer", CUSTOMER_SKILL) + .instance("customer", "acme", ACME_INSTANCE) + .instance("customer", "initech", INITECH_INSTANCE) + .done() +} + +async fn start() -> EscurelProcess { + EscurelProcess::spawn(Opts { + auth: AuthMode::TestIssuer, + fixtures: Some(fixtures()), + config_overrides: ConfigOverrides::default(), + }) + .await +} + +/// A gateway with the real DuckDB CRDT backend, the way the binary always +/// runs — required for the monotonic-version half of the guard loop. +async fn start_live() -> EscurelProcess { + EscurelProcess::spawn(Opts { + auth: AuthMode::TestIssuer, + fixtures: Some(fixtures()), + config_overrides: ConfigOverrides { + live_crdt: true, + ..Default::default() + }, + }) + .await +} + +async fn authed_client(p: &EscurelProcess) -> Client { + let token = p.mint_token(TENANT, Role::Agent); + Client::connect(p.base_url(), SecretString::from(token)) + .await + .unwrap() +} + +async fn resolve_page_id(client: &Client, wikilink: &str) -> String { + client + .resolve(ResolveRequest { + wikilink: wikilink.to_owned(), + ..Default::default() + }) + .await + .unwrap() + .page + .expect("page present") + .page_id +} + +// ── item 4: dropped optional-with-meaning fields on reads ───────── + +/// `expand` honours `as_of` server-side (a page born after the cut reads +/// as absent). The typed request has carried `as_of` all along — this +/// pins that the client actually SENDS it. +#[tokio::test] +async fn expand_forwards_as_of() { + let p = start().await; + let client = authed_client(&p).await; + let page_id = resolve_page_id(&client, "[[customer::acme]]").await; + + // Plain read: present. + let now = client + .expand(ExpandRequest { + page_id: page_id.clone(), + ..Default::default() + }) + .await + .unwrap(); + assert!(now.page.is_some(), "plain expand finds the page"); + + // Time-travel to before the fixture was written: absent. + let past = client + .expand(ExpandRequest { + page_id, + as_of: BEFORE_EVERYTHING.to_owned(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + past.page.is_none(), + "expand with as_of predating the write must resolve to no page — \ + the client dropped `as_of` on the floor" + ); + p.shutdown().await; +} + +/// Same plumbing pin for `neighbours`: edges from sources born after the +/// `as_of` cut are hidden server-side. +#[tokio::test] +async fn neighbours_forwards_as_of() { + let p = start().await; + let client = authed_client(&p).await; + let page_id = resolve_page_id(&client, "[[customer::acme]]").await; + + let now = client + .neighbours(NeighboursRequest { + page_id: page_id.clone(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + !now.edges.is_empty(), + "acme links to initech — plain neighbours sees the edge" + ); + + let past = client + .neighbours(NeighboursRequest { + page_id, + as_of: BEFORE_EVERYTHING.to_owned(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + past.edges.is_empty(), + "neighbours with as_of predating the writes must see no edges — \ + the client dropped `as_of` on the floor" + ); + p.shutdown().await; +} + +/// Same plumbing pin for `list_instances`. +#[tokio::test] +async fn list_instances_forwards_as_of() { + let p = start().await; + let client = authed_client(&p).await; + + let now = client + .list_instances(ListInstancesRequest { + skill: "customer".to_owned(), + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(now.instances.len(), 2, "both seeded instances visible"); + + let past = client + .list_instances(ListInstancesRequest { + skill: "customer".to_owned(), + as_of: BEFORE_EVERYTHING.to_owned(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + past.instances.is_empty(), + "list_instances with as_of predating the writes must be empty — \ + the client dropped `as_of` on the floor" + ); + p.shutdown().await; +} + +/// `search` already forwards `as_of` (fixed earlier) — pinned here so the +/// read surface stays uniform while the sibling methods catch up. +#[tokio::test] +async fn search_forwards_as_of() { + let p = start().await; + let client = authed_client(&p).await; + // Scoped to instance pages: every seeded instance is timed, so the + // pre-dating cut leaves no candidate blocks. (Unscoped search would + // still return untimed SKILL blocks from the vector side.) + let past = client + .search(SearchRequest { + q: "acme".to_owned(), + page_type: "instance".to_owned(), + as_of: BEFORE_EVERYTHING.to_owned(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + past.hits.is_empty(), + "as_of cut hides every timed instance block: {:?}", + past.hits + ); + p.shutdown().await; +} + +// ── item 3: list_instances cursor pagination ────────────────────── + +/// Paginate a real seeded instance list to exhaustion through the typed +/// client: request cursor + response next_cursor (only absent means done). +#[tokio::test] +async fn list_instances_paginates_to_exhaustion() { + let mut fx = FixtureBuilder::new() + .tenant(TENANT) + .skill("customer", CUSTOMER_SKILL); + for i in 0..5 { + let body = format!( + "---\ntype: instance\nskill: customer\nid: c{i}\nname: Customer {i}\n---\n# C{i}\n" + ); + fx = fx.instance("customer", &format!("c{i}"), body.as_str()); + } + let p = EscurelProcess::spawn(Opts { + auth: AuthMode::TestIssuer, + fixtures: Some(fx.done()), + config_overrides: ConfigOverrides::default(), + }) + .await; + let client = authed_client(&p).await; + + let mut seen = Vec::new(); + let mut cursor = String::new(); + let mut pages = 0; + loop { + let resp = client + .list_instances(ListInstancesRequest { + skill: "customer".to_owned(), + limit: 2, + cursor: cursor.clone(), + ..Default::default() + }) + .await + .unwrap(); + assert!( + resp.instances.len() <= 2, + "page respects the limit (got {})", + resp.instances.len() + ); + seen.extend(resp.instances.into_iter().map(|i| i.page_id)); + pages += 1; + assert!(pages <= 10, "cursor loop must terminate"); + match resp.next_cursor { + Some(c) => cursor = c, + None => break, + } + } + seen.sort(); + seen.dedup(); + assert_eq!( + seen.len(), + 5, + "every seeded instance appears exactly once across pages: {seen:?}" + ); + assert!(pages >= 3, "limit 2 over 5 rows takes at least 3 pages"); + p.shutdown().await; +} + +// ── items 1 + 2: the read→hash→guarded-write loop ───────────────── + +/// Full approve loop through the typed client: `expand` publishes +/// `content_sha256` (and, with a CRDT backend, `version`); a guarded +/// `update_page` carrying that hash succeeds; re-sending the now-stale +/// hash conflicts with `head_sha256`/`head_content` on the typed response. +#[tokio::test] +async fn expand_hash_feeds_guarded_update_and_stale_hash_conflicts() { + let p = start_live().await; + let client = authed_client(&p).await; + let page_id = resolve_page_id(&client, "[[customer::acme]]").await; + + let read = client + .expand(ExpandRequest { + page_id: page_id.clone(), + ..Default::default() + }) + .await + .unwrap(); + let sha = read + .content_sha256 + .clone() + .expect("plain expand publishes content_sha256 (#354/#408)"); + assert_eq!(sha.len(), 64, "hex sha256"); + assert!( + read.version.is_some(), + "live-CRDT gateway publishes the monotonic version on expand (#246)" + ); + + // Guarded write against the hash we just read: succeeds. + let updated = "---\n\ +type: instance\n\ +skill: customer\n\ +id: acme\n\ +name: Acme Corp\n\ +tier: platinum\n\ +---\n\ +# Acme Corp\n\nKey account. See [[customer::initech]].\n"; + let ok = client + .update_page(UpdatePageRequest { + page_id: page_id.clone(), + content: updated.to_owned(), + base_sha256: Some(sha.clone()), + ..Default::default() + }) + .await + .unwrap(); + assert!( + ok.ok, + "guarded write with the fresh hash lands: {:?}", + ok.issues + ); + + // Same (now stale) hash again: refused as a typed conflict, with the + // head hash + content for the approver to re-diff against. + let stale = client + .update_page(UpdatePageRequest { + page_id: page_id.clone(), + content: + "---\ntype: instance\nskill: customer\nid: acme\nname: Acme Corp\n---\n# stale\n" + .to_owned(), + base_sha256: Some(sha), + ..Default::default() + }) + .await + .unwrap(); + assert!(!stale.ok, "stale hash must not write"); + assert_eq!(stale.issues[0].code, "conflict"); + let head_sha = stale.head_sha256.expect("conflict carries head_sha256"); + assert_eq!(head_sha.len(), 64); + let head = stale.head_content.expect("conflict carries head_content"); + assert!( + head.contains("platinum"), + "head_content is the landed write" + ); + + // The published hash tracks the head: reading again yields the hash the + // conflict reported, closing the loop. + let reread = client + .expand(ExpandRequest { + page_id, + ..Default::default() + }) + .await + .unwrap(); + assert_eq!(reread.content_sha256.as_deref(), Some(head_sha.as_str())); + p.shutdown().await; +} + +/// The version-CAS variant (#246): a strict (`require_exact_base`) write +/// against a stale `base_version` conflicts instead of auto-merging, and +/// the typed response carries `head_version`. +#[tokio::test] +async fn stale_base_version_with_require_exact_base_conflicts() { + let p = start_live().await; + let client = authed_client(&p).await; + let page_id = resolve_page_id(&client, "[[customer::initech]]").await; + + let read = client + .expand(ExpandRequest { + page_id: page_id.clone(), + ..Default::default() + }) + .await + .unwrap(); + let base = read.version.expect("live-CRDT gateway publishes version"); + + // Advance the head past the base we hold. + let advance = "---\ntype: instance\nskill: customer\nid: initech\nname: Initech\ntier: silver\n---\n# Initech\n"; + let first = client + .update_page(UpdatePageRequest { + page_id: page_id.clone(), + content: advance.to_owned(), + base_version: Some(base.clone()), + ..Default::default() + }) + .await + .unwrap(); + assert!(first.ok, "clean CAS write lands: {:?}", first.issues); + assert_ne!(first.new_version, base, "version advanced"); + + // Strict write against the stale base: conflict, nothing persisted. + let stale = client + .update_page(UpdatePageRequest { + page_id: page_id.clone(), + content: "---\ntype: instance\nskill: customer\nid: initech\nname: Initech\n---\n# strict loser\n".to_owned(), + base_version: Some(base), + require_exact_base: true, + ..Default::default() + }) + .await + .unwrap(); + assert!(!stale.ok, "strict stale base must conflict, never merge"); + assert_eq!(stale.issues[0].code, "conflict"); + assert_eq!( + stale.head_version.as_deref(), + Some(first.new_version.as_str()), + "conflict reports the head the caller must re-read" + ); + assert!(!stale.auto_merged, "strict path never auto-merges"); + + let reread = client + .expand(ExpandRequest { + page_id, + ..Default::default() + }) + .await + .unwrap(); + assert!( + reread.body.contains("Initech") && !reread.body.contains("strict loser"), + "the refused draft was not persisted" + ); + p.shutdown().await; +} diff --git a/crates/escurel-server/tests/suite/meta_skill.rs b/crates/escurel-server/tests/suite/meta_skill.rs index dbdc5018..96fefe1f 100644 --- a/crates/escurel-server/tests/suite/meta_skill.rs +++ b/crates/escurel-server/tests/suite/meta_skill.rs @@ -65,6 +65,7 @@ async fn removing_a_standard_section_is_rejected() { .update_page(UpdatePageRequest { page_id: META_PAGE_ID.to_owned(), content: mangled, + ..Default::default() }) .await .expect("update_page"); @@ -86,6 +87,7 @@ async fn appending_tenant_guidance_is_accepted() { .update_page(UpdatePageRequest { page_id: META_PAGE_ID.to_owned(), content: extended, + ..Default::default() }) .await .expect("update_page"); diff --git a/crates/escurel-server/tests/suite/page_write_events.rs b/crates/escurel-server/tests/suite/page_write_events.rs index 49188900..0039ce9e 100644 --- a/crates/escurel-server/tests/suite/page_write_events.rs +++ b/crates/escurel-server/tests/suite/page_write_events.rs @@ -76,6 +76,7 @@ async fn a_write_notifies_the_webhook_but_never_the_inbox() { c.update_page(UpdatePageRequest { page_id: PAGE.to_owned(), content: BODY.to_owned(), + ..Default::default() }) .await .expect("update_page"); @@ -133,6 +134,7 @@ async fn a_skill_page_write_is_not_announced() { page_id: "markdown/skills/customer.md".to_owned(), content: "---\ntype: skill\nid: customer\ndescription: Customers.\n---\n# customer\n" .to_owned(), + ..Default::default() }) .await; diff --git a/crates/escurel-types/src/agent.rs b/crates/escurel-types/src/agent.rs index 512d54ed..83d10d53 100644 --- a/crates/escurel-types/src/agent.rs +++ b/crates/escurel-types/src/agent.rs @@ -130,6 +130,20 @@ pub struct ExpandResponse { /// (additive; old servers never emit it). #[serde(default, skip_serializing_if = "Option::is_none")] pub shadow: Option, + /// The page's current monotonic version (`v`, #246) — the value + /// to send back as [`UpdatePageRequest::base_version`] in the + /// read→edit→guarded-write cycle. Emitted only by a gateway with a + /// live CRDT backend; absent otherwise. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub version: Option, + /// Hex sha256 of the STORED markdown bytes (#354/#408) — exactly what + /// [`UpdatePageRequest::base_sha256`] compares against, closing the + /// read→hash→guarded-write approve loop without a write-probe. + /// Published on **plain reads only**: absent under `as_of`/`scenario` + /// (a historical/overlaid body is not the current stored bytes) and + /// on old servers. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub content_sha256: Option, } // ── neighbours ──────────────────────────────────────────────────── @@ -443,6 +457,10 @@ pub struct ListSkillsResponse { pub struct ListInstancesRequest { #[serde(rename = "skill_id")] pub skill: String, + /// Resume cursor from a previous response's + /// [`ListInstancesResponse::next_cursor`]. Empty = start from the top. + #[serde(skip_serializing_if = "String::is_empty")] + pub cursor: String, pub order_by_at: String, pub limit: u32, pub frontmatter_key: String, @@ -470,8 +488,10 @@ pub struct InstanceInfo { #[serde(default)] pub struct ListInstancesResponse { pub instances: Vec, - /// MCP wire emits `next_cursor` (null today); pagination is not - /// yet implemented server-side. + /// Resume cursor: pass it back as [`ListInstancesRequest::cursor`] to + /// fetch the next page. The wire keeps the key present (`null` on the + /// last page) — **only absence/null means done**; an ACL filter may + /// legitimately shorten a page below `limit` with more rows to come. #[serde(skip_serializing_if = "Option::is_none")] pub next_cursor: Option, } @@ -545,21 +565,73 @@ pub struct ValidateResponse { // ── update / live ───────────────────────────────────────────────── -/// `update_page` arguments. MCP wire keys: `page_id`, `content`. +/// `update_page` arguments. MCP wire keys: `page_id`, `content`, plus the +/// optional concurrency/approve guards `base_version`, +/// `require_exact_base`, `base_sha256`, and the `provenance` passthrough. +/// +/// Every guard is optional-with-meaning: **absent means unguarded** (the +/// wire semantics), so all of them are omitted from the serialized +/// arguments unless explicitly set. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] #[serde(default)] pub struct UpdatePageRequest { pub page_id: String, pub content: String, + /// Optimistic-concurrency guard (#246): the page `version` the client + /// last read (published by `expand` on a live-CRDT gateway, and by + /// this tool's own `new_version`). Stale base → CRDT three-way + /// auto-merge, or a typed `conflict` when unmergeable. On a gateway + /// with no CRDT backend the guard refuses (`versioning_unavailable`) + /// rather than being silently dropped. + #[serde(skip_serializing_if = "Option::is_none")] + pub base_version: Option, + /// Strict compare-and-swap: with a stale `base_version`, conflict + /// outright instead of attempting the auto-merge — the + /// human-in-the-loop approval shape. Requires `base_version` + /// (the server rejects the flag without one). + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub require_exact_base: bool, + /// Content-hash compare-and-swap (#354) — the approve guard that works + /// on EVERY gateway: the hex sha256 of the stored markdown the held + /// write was drafted against, as published by `expand`'s + /// `content_sha256`. `Some("")` is the **approve-create sentinel** + /// ("I expect no page yet") and is serialized as the empty string; + /// `None` means unguarded and is omitted from the wire. A mismatch + /// refuses with `code: conflict` + `head_sha256` + `head_content`. + #[serde(skip_serializing_if = "Option::is_none")] + pub base_sha256: Option, + /// Provenance passthrough (#246): a runner-orchestrated write carries + /// its `provenance.workflow`/`runner` block, which suppresses the + /// opt-in `page-edited` event for that write. + #[serde(skip_serializing_if = "Option::is_none")] + pub provenance: Option, } -/// MCP wire keys: `ok`, `issues`, `new_version`. +/// MCP wire keys: `ok`, `issues`, `new_version`, `auto_merged`, and — on a +/// `conflict` refusal — `head_version` / `head_sha256` / `head_content`. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Default)] #[serde(default)] pub struct UpdatePageResponse { pub ok: bool, pub issues: Vec, pub new_version: String, + /// The write landed as a CRDT three-way auto-merge of a stale + /// `base_version` draft with the concurrent head (never `true` under + /// `require_exact_base`). Absent on old servers ⇒ `false`. + pub auto_merged: bool, + /// On a `base_version` conflict: the head version the caller must + /// re-read before re-drafting. + #[serde(skip_serializing_if = "Option::is_none")] + pub head_version: Option, + /// On a `base_sha256` conflict: the hash of the stored markdown at + /// head (`""` when no page exists yet). + #[serde(skip_serializing_if = "Option::is_none")] + pub head_sha256: Option, + /// On a conflict: the stored markdown at head, for the caller to + /// re-diff / re-draft against. The wire may carry an explicit `null` + /// (page absent), which decodes to `None`. + #[serde(skip_serializing_if = "Option::is_none")] + pub head_content: Option, } /// `delete_page` arguments (#300). MCP wire keys: `page_id`, optional diff --git a/crates/escurel-types/tests/wire_contract.rs b/crates/escurel-types/tests/wire_contract.rs index c5accc43..dc06f208 100644 --- a/crates/escurel-types/tests/wire_contract.rs +++ b/crates/escurel-types/tests/wire_contract.rs @@ -440,6 +440,8 @@ fn roundtrip_agent() { }], wikilinks_out: vec![WikilinkParsed::default()], shadow: Some(json!({ "base_page_id": "markdown/base/p/skills/s.md" })), + version: Some("v7".into()), + content_sha256: Some("ab".repeat(32)), }); rt(ResolveResponse { parsed: Some(WikilinkParsed::default()), @@ -473,6 +475,10 @@ fn roundtrip_agent() { ok: true, issues: vec![], new_version: "v1".into(), + auto_merged: false, + head_version: None, + head_sha256: None, + head_content: None, }); rt(ListSkillsResponse { skills: vec![Skill { @@ -608,3 +614,137 @@ fn roundtrip_admin() { bytes_reclaimed: 4096, }); } + +// ── update_page guard wire semantics (contract-parity) ──────────── + +/// Absent guards stay absent on the wire (absent means UNGUARDED), and +/// `base_sha256: Some("")` — the approve-create sentinel — serializes as +/// the explicit empty string, never dropped. +#[test] +fn update_page_request_guard_wire_semantics() { + // Unguarded request: no guard key leaks onto the wire. + let bare = UpdatePageRequest { + page_id: "p".into(), + content: "c".into(), + ..Default::default() + }; + let v = serde_json::to_value(&bare).unwrap(); + let obj = v.as_object().unwrap(); + for k in [ + "base_version", + "require_exact_base", + "base_sha256", + "provenance", + ] { + assert!(!obj.contains_key(k), "unset guard `{k}` must be omitted"); + } + + // Fully guarded request: every field serializes under its wire key. + let guarded = UpdatePageRequest { + page_id: "p".into(), + content: "c".into(), + base_version: Some("v9".into()), + require_exact_base: true, + base_sha256: Some(String::new()), // approve-create sentinel + provenance: Some(json!({ "workflow": "w" })), + }; + let v = serde_json::to_value(&guarded).unwrap(); + assert_eq!(v["base_version"], "v9"); + assert_eq!(v["require_exact_base"], true); + assert_eq!( + v["base_sha256"], "", + "Some(\"\") serializes as the sentinel" + ); + assert_eq!(v["provenance"]["workflow"], "w"); + // ...and round-trips. + let back: UpdatePageRequest = serde_json::from_value(v).unwrap(); + assert_eq!(back, guarded); +} + +/// Mirrors `tool_update_page`'s `base_sha256` conflict refusal. +#[test] +fn update_page_conflict_response_wire_shape() { + let wire = json!({ + "ok": false, + "issues": [{ + "severity": "error", + "code": "conflict", + "location": "base_sha256", + "message": "stale", + }], + "head_sha256": "ab".repeat(32), + "head_content": "---\n---\n# head\n", + }); + let resp: UpdatePageResponse = serde_json::from_value(wire).unwrap(); + assert!(!resp.ok); + assert_eq!(resp.issues[0].code, "conflict"); + assert_eq!(resp.head_sha256.as_deref(), Some("ab".repeat(32).as_str())); + assert_eq!(resp.head_content.as_deref(), Some("---\n---\n# head\n")); + assert!(!resp.auto_merged, "absent auto_merged decodes to false"); + assert!(resp.head_version.is_none()); +} + +/// Mirrors `tool_expand`'s guard-field emission (#246/#354/#408): the +/// typed response carries `version` + `content_sha256` when present and +/// tolerates their absence (old server / `as_of` read). +#[test] +fn expand_response_guard_fields_wire_shape() { + let wire = json!({ + "page": { + "page_id": "p", "slug": "s", "skill": "sk", + "page_type": "instance", "last_written_by": null, + }, + "frontmatter": {}, + "body": "b", + "blocks": [], + "wikilinks_out": [], + "version": "v12", + "content_sha256": "cd".repeat(32), + }); + let resp: ExpandResponse = serde_json::from_value(wire).unwrap(); + assert_eq!(resp.version.as_deref(), Some("v12")); + assert_eq!( + resp.content_sha256.as_deref(), + Some("cd".repeat(32).as_str()) + ); + // Absence (historical read / old server) decodes to None. + let old: ExpandResponse = serde_json::from_value(json!({ + "page": null, + })) + .unwrap(); + assert!(old.version.is_none()); + assert!(old.content_sha256.is_none()); +} + +/// `list_instances` cursor plumbing: the request omits an empty cursor +/// and carries a set one; `next_cursor: null` (last page) decodes `None`. +#[test] +fn list_instances_cursor_wire_shape() { + let bare = ListInstancesRequest { + skill: "customer".into(), + ..Default::default() + }; + let v = serde_json::to_value(&bare).unwrap(); + assert!(!v.as_object().unwrap().contains_key("cursor")); + + let resumed = ListInstancesRequest { + skill: "customer".into(), + cursor: "tok".into(), + ..Default::default() + }; + let v = serde_json::to_value(&resumed).unwrap(); + assert_eq!(v["cursor"], "tok"); + + let last: ListInstancesResponse = serde_json::from_value(json!({ + "instances": [], + "next_cursor": null, + })) + .unwrap(); + assert!(last.next_cursor.is_none(), "only null/absent means done"); + let more: ListInstancesResponse = serde_json::from_value(json!({ + "instances": [], + "next_cursor": "tok", + })) + .unwrap(); + assert_eq!(more.next_cursor.as_deref(), Some("tok")); +}