From 938602a6c2bbff81e807b7db82e57f70586740d6 Mon Sep 17 00:00:00 2001 From: macanderson Date: Sun, 26 Jul 2026 11:39:50 -0700 Subject: [PATCH 1/2] fix: hold the spec to its own schema, and its own conformance claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, all the same shape: a normative claim nothing checked. 1. SPEC.md §9's `verify` example was invalid against the schema SPEC.md ships. Both envelopes carried `"id": "v1"`, but §3.2 grants an `id` only to `query`/`frames`/`error`, `Envelope::Verify`/`Verified` have no such field, and the schema is `additionalProperties: false`. Removed — verify correlates by full frame identity, not by envelope id. Root cause: validate-examples.py checked `examples/` but never SPEC.md, so the one example surface with no machine check was the one that drifted. It now validates every fenced jsonc block in SPEC.md (comments and documented placeholders normalized; structure checked), so CI's existing `schema` job catches this class. Verified by re-introducing the `id`: the harness fails both envelopes and exits non-zero. 2. An ordinary ContextQuery was un-representable in conformant JSON. `kinds` and `anchors` were `required` while both are skip_serializing_if = "Vec::is_empty", so an unfiltered, unanchored query failed validation — the bug PR #44 fixed for ContextFrame, one type over. A test now asserts an ordinary query satisfies the schema's own `required` array (reads the schema, so it cannot drift), and a cross-audit of all 16 shared types confirms no other type demands a field its serializer elides. 3. §G2 claimed "Verified by frame-validity" while `target_uri` appeared nowhere in the conformance or validation code — the self-attestation §11.1 exists to reject. Implemented rather than downgraded: check_frames now rejects an edge with an empty target_uri. Auditing the other ten rules citing frame-validity found §D1 unenforced too: the frame's own content_digest was never format-checked, and the evidence string claimed "well-formed digests" while accepting `sha256:abc` — the very placeholder validate.rs exists to reject. Also fixed; the remaining nine were confirmed enforced. Both new checks are mutation-tested: disabling either makes its test fail. Also corrects contextgraph-host::wire, which said correlation is "negotiated by observation, not by a capability flag" — contradicting §3.2 and the shipped Capabilities::correlation, and inverting a MUST NOT. cargo test --workspace: 243 passed, 0 failed. fmt, clippy -D warnings, validate-examples.py, and conformance-{green,red}/host all pass. --- CHANGELOG.md | 32 +++++ SPEC.md | 4 +- contextgraph-conformance/src/lib.rs | 23 +++- .../tests/golden_fixtures.rs | 95 +++++++++++++- .../tests/ingest_conformance.rs | 59 ++++++++- contextgraph-host/src/wire.rs | 18 ++- contextgraph-types/src/frame.rs | 75 +++++++++++ schema/contextgraph-envelope.schema.json | 3 +- schema/validate-examples.py | 117 +++++++++++++++++- 9 files changed, 410 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db5f74..6f26f89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,38 @@ which. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1 host. Wire-compatible; Rust API breaking (#5, #6, #11). ### Fixed +- **`SPEC.md` §9's `verify` example no longer fails the schema `SPEC.md` ships.** + Both envelopes carried `"id": "v1"`, but §3.2 grants an `id` only to + `query`/`frames`/`error`, the reference `Envelope::Verify`/`Verified` have no + such field, and the schema is `additionalProperties: false` — the example was + invalid against the protocol's own definition. The `id`s are removed; + `verify` correlates by full frame identity, not by envelope id. Root cause: + `schema/validate-examples.py` checked `examples/` but never `SPEC.md`, so the + one example surface with no machine check was the one that drifted. It now + validates every fenced `jsonc` block in `SPEC.md` too (comments and documented + placeholders normalized away, structure checked), and CI's existing `schema` + job therefore catches this class of drift. +- **JSON Schema: an ordinary `ContextQuery` was un-representable.** `kinds` and + `anchors` were listed as `required` while both are + `skip_serializing_if = "Vec::is_empty"` in the reference type, so an + unfiltered, unanchored query — what a host sends when it wants anything + relevant — failed schema validation. Exactly the bug fixed for `ContextFrame` + below, one type over. A test now asserts an ordinary query satisfies the + schema's own `required` array, and a cross-audit of all 16 shared types + confirms no other type demands a field its serializer elides. +- **§G2 and §D1 are now actually verified, not merely asserted.** Both named + `frame-validity` as their verifier while neither `target_uri` nor the frame's + own `content_digest` was read by any check — the self-attestation §11.1 + exists to rule out. `check_frames` now rejects a relation with an empty + `target_uri` (§G2) and a present-but-malformed `content_digest` (§D1); its + evidence string had claimed "well-formed digests" while accepting + `sha256:abc`. §D1 was found by auditing the other ten rules that cite + `frame-validity` after §G2 turned out to be unenforced; the remaining nine + were confirmed enforced. +- **`contextgraph-host::wire` docs no longer invert a MUST NOT.** The module + said concurrency is "negotiated by observation, not by a capability flag", + contradicting `SPEC.md` §3.2 and the shipped `Capabilities::correlation`: a + host **MUST NOT** send an `id` to a provider that did not declare correlation. - JSON Schema: a `ContextFrame`'s `required` is now exactly what the reference serializer always emits (`id`, `kind`, `title`, `score`, `token_cost`). `provenance` and `relations` were listed as globally required but are diff --git a/SPEC.md b/SPEC.md index c2b5833..a1b9783 100644 --- a/SPEC.md +++ b/SPEC.md @@ -441,14 +441,14 @@ a notification-shaped 1.x addition (§13) and is not defined here. ```jsonc // host → provider -{ "type": "verify", "id": "v1", +{ "type": "verify", "request": { "frames": [ { "provider_id": "code-graph", "frame_id": "frm_retry", "content_digest": "sha256:<64 hex>" } ] } } // provider → host -{ "type": "verified", "id": "v1", +{ "type": "verified", "response": { "verdicts": [ { "frame": { "provider_id": "code-graph", "frame_id": "frm_retry", "content_digest": "sha256:<64 hex>" }, diff --git a/contextgraph-conformance/src/lib.rs b/contextgraph-conformance/src/lib.rs index c5d1d0e..fd98b14 100644 --- a/contextgraph-conformance/src/lib.rs +++ b/contextgraph-conformance/src/lib.rs @@ -714,6 +714,16 @@ pub fn check_frames(result: &ContextQueryResult) -> (bool, String) { "frame[{i}] field `{field}` is not an RFC 3339 UTC timestamp (§F4)" )); } + // §D1: the frame's own content_digest, when present, must be in the + // protocol's digest form. Like §G2 this was listed as verified here and + // read by nothing — so the digest that anchors deterministic + // composition, usage reports and `context/verify` was held to a looser + // standard than the §F5 provenance digests immediately below it. + if !frame.has_usable_content_digest() { + problems.push(format!( + "frame[{i}] content_digest is present but not `sha256:<64 lowercase hex>` (§D1)" + )); + } // §F5: file provenance must carry a well-formed digest, since that is // the only provenance a host can independently re-read and verify. for index in frame.provenance_with_unusable_digests() { @@ -721,7 +731,10 @@ pub fn check_frames(result: &ContextQueryResult) -> (bool, String) { "frame[{i}] provenance[{index}] addresses a file but its digest is missing or not `sha256:<64 lowercase hex>` (§F5)" )); } - // §G1: a graph edge must be citable by a human label. + // §G1/§G2: a graph edge must be citable by a human label, and must + // actually point somewhere. G2 was listed as "Verified by + // frame-validity" while no code read `target_uri` at all — the exact + // self-attestation §11.1 rejects. It is verified here now. for (edge_index, edge) in frame.relations.iter().enumerate() { if !edge.has_display_name() { problems.push(format!( @@ -729,6 +742,12 @@ pub fn check_frames(result: &ContextQueryResult) -> (bool, String) { edge.rel )); } + if !edge.has_target_uri() { + problems.push(format!( + "frame[{i}] relation[{edge_index}] `{}` has an empty target_uri (§G2 — an edge to nowhere is not an edge)", + edge.rel + )); + } } } @@ -736,7 +755,7 @@ pub fn check_frames(result: &ContextQueryResult) -> (bool, String) { ( true, format!( - "{} frame(s) — scores in [0,1], titles, citation labels, honest representations, RFC 3339 timestamps, well-formed digests, labelled relations", + "{} frame(s) — scores in [0,1], titles, citation labels, honest representations, RFC 3339 timestamps, well-formed digests, labelled and targeted relations", result.frames.len() ), ) diff --git a/contextgraph-conformance/tests/golden_fixtures.rs b/contextgraph-conformance/tests/golden_fixtures.rs index d59861b..ea6a583 100644 --- a/contextgraph-conformance/tests/golden_fixtures.rs +++ b/contextgraph-conformance/tests/golden_fixtures.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use contextgraph_conformance::check_frames; use contextgraph_types::{ - ContextFrame, ContextQuery, ContextQueryResult, PROTOCOL_VERSION, Representation, + ContextFrame, ContextQuery, ContextQueryResult, PROTOCOL_VERSION, Relation, Representation, }; use serde::Deserialize; use serde_json::{Map, Value}; @@ -293,6 +293,99 @@ fn assert_rejected_citation(frame: ContextFrame, label: &str) { ); } +/// §G2 — `target_uri` **MUST** be a non-empty URI. +/// +/// The spec listed this as "Verified by `frame-validity`" while `target_uri` +/// appeared nowhere in the conformance or validation code: the rule was +/// asserted, not checked, which is precisely what §11.1 says a conformance +/// suite must not do. This test is the check that makes the claim true, written +/// adversarially — a suite that only ever passes proves nothing about its +/// ability to catch a broken provider. +#[test] +fn frame_validity_catches_a_relation_that_points_nowhere() { + let base: ContextFrame = read_fixture("context-frame.compact.valid.json"); + + let with_edge = |target_uri: &str| { + let mut frame = base.clone(); + frame.relations = vec![Relation { + rel: "doc.documents".into(), + target_uri: target_uri.into(), + display_name: Some("Retry policy".into()), + }]; + ContextQueryResult { + frames: vec![frame], + truncated: false, + dropped_estimate: None, + } + }; + + // Control: the fixture is conformant, and stays so with a real edge — so a + // failure below is attributable to `target_uri` and nothing else. + let (passed, evidence) = check_frames(&with_edge("file:///repo/docs/retry.md")); + assert!(passed, "a well-formed edge was rejected: {evidence}"); + + // The breach: an edge that is labelled (§G1 satisfied) but dangling. + for empty in ["", " "] { + let (passed, evidence) = check_frames(&with_edge(empty)); + assert!( + !passed, + "a relation with target_uri {empty:?} passed frame-validity — §G2 is unenforced again" + ); + assert!( + evidence.contains("target_uri") && evidence.contains("§G2"), + "evidence must name the field and the rule, got: {evidence}" + ); + } +} + +/// §D1 — `content_digest`, when present, **MUST** match `sha256:<64 lowercase +/// hex>`. +/// +/// Found by auditing the other ten rules that name `frame-validity` as their +/// verifier after §G2 turned out to be unenforced. This one was unenforced too, +/// and the evidence string `check_frames` returned claimed "well-formed +/// digests" while accepting `sha256:abc` — the very placeholder +/// `validate::tests::rejects_the_placeholder_digest_the_repo_used_in_examples` +/// exists to reject. +#[test] +fn frame_validity_catches_a_malformed_content_digest() { + let base: ContextFrame = read_fixture("context-frame.compact.valid.json"); + let with_digest = |digest: Option<&str>| { + let mut frame = base.clone(); + frame.content_digest = digest.map(str::to_string); + ContextQueryResult { + frames: vec![frame], + truncated: false, + dropped_estimate: None, + } + }; + + // Control: the fixture's real digest passes, so a failure below is + // attributable to the digest form and nothing else. + let (passed, evidence) = check_frames(&with_digest(base.content_digest.as_deref())); + assert!( + passed, + "a well-formed content_digest was rejected: {evidence}" + ); + + for malformed in [ + "sha256:abc".to_string(), // the repo's own placeholder + format!("sha256:{}", "A".repeat(64)), // uppercase hex — a false-negative compare + format!("md5:{}", "a".repeat(64)), // undeclared algorithm + "a".repeat(64), // no algorithm prefix + ] { + let (passed, evidence) = check_frames(&with_digest(Some(&malformed))); + assert!( + !passed, + "content_digest {malformed:?} passed frame-validity — §D1 is unenforced again" + ); + assert!( + evidence.contains("content_digest") && evidence.contains("§D1"), + "evidence must name the field and the rule, got: {evidence}" + ); + } +} + #[test] fn manifest_has_strict_coverage_and_correct_file_hashes() { let directory = fixture_dir(); diff --git a/contextgraph-conformance/tests/ingest_conformance.rs b/contextgraph-conformance/tests/ingest_conformance.rs index 054badd..fe57feb 100644 --- a/contextgraph-conformance/tests/ingest_conformance.rs +++ b/contextgraph-conformance/tests/ingest_conformance.rs @@ -20,10 +20,10 @@ use contextgraph_host::wire::{Envelope, decode_line, encode_line}; use contextgraph_host::{ContextProvider, IngestConfig, PasteIngest, ingest_paste}; use contextgraph_types::{ContextQuery, Representation}; -/// The exact `required` key set the JSON Schema declares for a `ContextFrame`, -/// read from the schema in the repo so the test tracks the schema rather than a +/// The exact `required` key set the JSON Schema declares for `definition`, read +/// from the schema in the repo so the test tracks the schema rather than a /// hard-coded copy of it. -fn schema_required_frame_keys() -> Vec { +fn schema_required_keys(definition: &str) -> Vec { let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) .parent() .expect("workspace root") @@ -32,14 +32,63 @@ fn schema_required_frame_keys() -> Vec { let raw = std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())); let schema: serde_json::Value = serde_json::from_str(&raw).expect("schema is valid JSON"); - schema["$defs"]["ContextFrame"]["required"] + schema["$defs"][definition]["required"] .as_array() - .expect("$defs.ContextFrame.required is an array") + .unwrap_or_else(|| panic!("$defs.{definition}.required is an array")) .iter() .map(|v| v.as_str().expect("a required key is a string").to_string()) .collect() } +fn schema_required_frame_keys() -> Vec { + schema_required_keys("ContextFrame") +} + +/// The most ordinary query there is — no kind filter, no anchors — must be +/// representable in conformant JSON. +/// +/// The same bug this module's header describes for `ContextFrame` was still +/// live one type over: `kinds` and `anchors` are `skip_serializing_if = +/// "Vec::is_empty"`, so the reference serializer omits them, while the schema +/// listed both as `required`. An unfiltered, unanchored query — what a host +/// sends when it wants anything relevant — therefore failed schema validation. +/// +/// Asserting against the schema's own `required` array rather than a literal +/// list is what makes this a guard instead of a snapshot: re-adding either key +/// to `required` fails here immediately. +#[test] +fn an_unfiltered_unanchored_query_satisfies_the_schemas_required_keys() { + let query = ContextQuery { + goal: "why does the retry loop give up".into(), + query_text: None, + embedding: None, + kinds: Vec::new(), + anchors: Vec::new(), + max_frames: 8, + max_tokens: 2000, + as_of: None, + representation_preferences: Vec::new(), + }; + + let value = serde_json::to_value(&query).expect("query serializes"); + let object = value.as_object().expect("a query serializes to an object"); + + // Precondition: the elision this guards is real, not incidental. + assert!( + !object.contains_key("kinds") && !object.contains_key("anchors"), + "the reference serializer no longer elides empty kinds/anchors — \ + this guard's premise changed: {value}" + ); + + for key in schema_required_keys("ContextQuery") { + assert!( + object.contains_key(&key), + "an ordinary query omits schema-required key `{key}`, making it \ + un-representable in conformant JSON: {value}" + ); + } +} + /// The ADR's motivating paste: a noisy log, a data table, an attached note, and /// a directory reference — every evidence kind plus an anchor. fn motivating_paste() -> PasteIngest { diff --git a/contextgraph-host/src/wire.rs b/contextgraph-host/src/wire.rs index d5b4450..b09058f 100644 --- a/contextgraph-host/src/wire.rs +++ b/contextgraph-host/src/wire.rs @@ -36,9 +36,21 @@ //! (`docs/sketches/push-invalidation.md`). //! //! `id` is optional so that a provider written against an earlier revision -//! stays conformant: a host that has not seen a provider echo an `id` **MUST** -//! fall back to lock-step. Concurrency is negotiated by observation, not by a -//! capability flag. +//! stays conformant: it is queried in lock-step and is fully conformant. +//! +//! Correlation is negotiated **explicitly**, via +//! [`Capabilities::correlation`](contextgraph_types::Capabilities::correlation) +//! — a host **MUST NOT** send an `id` to a provider that did not declare it +//! (`SPEC.md` §3.2). This paragraph previously said the opposite ("negotiated +//! by observation, not by a capability flag"), which predates the capability +//! and inverts a MUST NOT: a reply carrying no `id` is ambiguous between "does +//! not implement correlation" and "implements it incorrectly", and a guarantee +//! whose violation is indistinguishable from legitimate behaviour cannot be +//! checked. +//! +//! Note that `verify`/`verified` carry no `id` at all: a verdict echoes the +//! frame identity it answers *in full*, so those exchanges correlate by +//! matching rather than by envelope id (`SPEC.md` §9). use contextgraph_types::{ Capabilities, ContextQuery, ContextQueryResult, ErrorCode, ProviderInfo, VerifyRequest, diff --git a/contextgraph-types/src/frame.rs b/contextgraph-types/src/frame.rs index b814e5f..ebedddc 100644 --- a/contextgraph-types/src/frame.rs +++ b/contextgraph-types/src/frame.rs @@ -209,6 +209,17 @@ impl Relation { .is_some_and(|name| !name.trim().is_empty()) } + /// Whether this edge points somewhere (`SPEC.md` §G2). + /// + /// `target_uri` is a required field, so serde guarantees it is *present* — + /// but `""` deserializes happily, and an edge to nowhere is not an edge. + /// The schema has carried `minLength: 1` here from the start; §G2 claimed + /// `frame-validity` verified it and no code did, which is the + /// self-attestation §11.1 exists to rule out. + pub fn has_target_uri(&self) -> bool { + !self.target_uri.trim().is_empty() + } + /// Whether `rel` uses the recommended vocabulary. Advisory only — a `false` /// here is a hint for a provider author, never a conformance failure. pub fn uses_recommended_vocabulary(&self) -> bool { @@ -448,6 +459,22 @@ impl ContextFrame { .collect() } + /// Whether this frame's own `content_digest`, if it carries one, is in the + /// protocol's digest form (`SPEC.md` §D1). + /// + /// Absent is fine — §D1 binds the digest only "when present". What is not + /// fine is a *present* digest that no host can compare against, which is + /// what `sha256:abc` is. §D1 named `frame-validity` as its verifier while + /// nothing read this field: the digest that anchors deterministic + /// composition, usage reports, and `context/verify` was the one digest in + /// the protocol that went unchecked, and §F5 held provenance to a stricter + /// standard than the frame's own identity. + pub fn has_usable_content_digest(&self) -> bool { + self.content_digest + .as_deref() + .is_none_or(is_well_formed_digest) + } + /// Whether this frame's fields satisfy the invariants of its declared /// [`representation`](Self::representation). Providers emit conforming /// frames; hosts reject a frame that lies about its shape (e.g. a @@ -661,6 +688,54 @@ mod tests { assert!(!edge.has_display_name()); } + #[test] + fn a_present_content_digest_must_be_usable_but_an_absent_one_is_fine() { + // §D1 binds the digest only "when present": a frame that carries none + // is conformant, a frame that carries `sha256:abc` is not. + let mut frame = sample_frame(); + frame.content_digest = None; + assert!(frame.has_usable_content_digest(), "absent is permitted"); + + frame.content_digest = Some(format!("sha256:{}", "a".repeat(64))); + assert!(frame.has_usable_content_digest()); + + for malformed in ["sha256:abc", &format!("sha256:{}", "A".repeat(64))] { + frame.content_digest = Some(malformed.to_string()); + assert!( + !frame.has_usable_content_digest(), + "{malformed} is not a comparable digest" + ); + } + } + + #[test] + fn an_edge_pointing_nowhere_does_not_satisfy_g2() { + // §G2 says `target_uri` MUST be a non-empty URI and claimed + // `frame-validity` checked it; nothing read the field at all. serde + // guarantees presence (it is not an Option), so the reachable breach is + // the empty — or whitespace-only — string, which the schema's + // `minLength: 1` rejects and the Rust side silently accepted. + let labelled_but_dangling = Relation { + rel: rel::DOC_DOCUMENTS.into(), + target_uri: String::new(), + display_name: Some("Net docs".into()), + }; + assert!(labelled_but_dangling.has_display_name(), "§G1 is satisfied"); + assert!(!labelled_but_dangling.has_target_uri(), "but §G2 is not"); + + let whitespace = Relation { + target_uri: " ".into(), + ..labelled_but_dangling.clone() + }; + assert!(!whitespace.has_target_uri()); + + let real = Relation { + target_uri: "file:///docs/net.md".into(), + ..labelled_but_dangling + }; + assert!(real.has_target_uri()); + } + #[test] fn optional_fields_are_omitted_when_absent() { let frame = sample_frame(); diff --git a/schema/contextgraph-envelope.schema.json b/schema/contextgraph-envelope.schema.json index f58ded4..5df8b93 100644 --- a/schema/contextgraph-envelope.schema.json +++ b/schema/contextgraph-envelope.schema.json @@ -438,11 +438,10 @@ }, "required": [ "goal", - "kinds", - "anchors", "max_frames", "max_tokens" ], + "$comment": "Required is exactly what the reference serializer (contextgraph-types ContextQuery) always emits: goal, max_frames, max_tokens. kinds and anchors are skip_serializing_if=Vec::is_empty, so an unfiltered, unanchored query — the most ordinary query there is — omits them; listing them as required made that query un-representable in conformant JSON. Same bug class as the ContextFrame required list below (PR #44): the schema must not demand fields the canonical serializer elides.", "additionalProperties": false }, "ContextQueryResult": { diff --git a/schema/validate-examples.py b/schema/validate-examples.py index a14910d..32c7cb7 100755 --- a/schema/validate-examples.py +++ b/schema/validate-examples.py @@ -5,10 +5,18 @@ Usage: python3 schema/validate-examples.py -Exits 0 if every message in examples/ is valid under schema/, and 1 otherwise. +Exits 0 if every message in examples/ AND every fenced example in SPEC.md is +valid under schema/, and 1 otherwise. No third-party dependencies beyond `jsonschema` (pip install jsonschema). + +SPEC.md is checked because it was the one example surface nothing validated, and +it drifted: the §9 `verify` example carried an `id` member that §3.2 grants only +to `query`/`frames`/`error`, that the reference envelope has no field for, and +that the schema's `additionalProperties: false` rejects. The spec's own examples +are now held to the spec's own schema. """ import json +import re import sys from pathlib import Path @@ -56,5 +64,112 @@ def check(label: str, ok: bool) -> None: continue check(f"{ref_path.name} message {i} ({msg['type']})", True) +# 3. SPEC.md's own fenced examples. +# +# These are jsonc — `//` comments, and prose placeholders standing in for values +# whose real form is uninteresting to a reader (`"…"`, `sha256:<64 hex>`). Both +# are normalized away before validation, because what is being checked here is +# STRUCTURE: member names, required members, types, and above all +# `additionalProperties` — the class of error that put an `id` on a `verify`. +# Substituting a placeholder never invents a member, so it cannot mask that. +ENVELOPE_TYPES = { + SCHEMA["$defs"][ref["$ref"].rsplit("/", 1)[-1]]["properties"]["type"]["const"] + for ref in SCHEMA["oneOf"] +} + +PLACEHOLDERS = [ + (re.compile(r"sha256:<64 hex>"), "sha256:" + "a" * 64), # §F5 digest form + (re.compile(r'"…"'), '"placeholder"'), # elided string value +] + + +def strip_line_comments(text: str) -> str: + """Remove `//` comments, leaving `//` inside JSON strings alone.""" + out = [] + for line in text.splitlines(): + in_string = escaped = False + cut = None + for i, ch in enumerate(line): + if escaped: + escaped = False + elif ch == "\\" and in_string: + escaped = True + elif ch == '"': + in_string = not in_string + elif ch == "/" and not in_string and line[i + 1 : i + 2] == "/": + cut = i + break + out.append(line if cut is None else line[:cut]) + return "\n".join(out) + + +def json_values(text: str): + """Yield each successive JSON value in a block (a block may hold a pair).""" + decoder = json.JSONDecoder() + index = 0 + while (index := _skip_ws(text, index)) < len(text): + value, index = decoder.raw_decode(text, index) + yield value + + +def _skip_ws(text: str, index: int) -> int: + while index < len(text) and text[index] in " \t\r\n": + index += 1 + return index + + +spec_path = ROOT / "SPEC.md" +spec_lines = spec_path.read_text().splitlines() +blocks, current, start = [], None, 0 +for number, line in enumerate(spec_lines, 1): + if current is None: + if line.rstrip() == "```jsonc": + current, start = [], number + elif line.rstrip() == "```": + blocks.append((start, "\n".join(current))) + current = None + else: + current.append(line) + +if not blocks: + check("SPEC.md contains fenced jsonc examples", False) + print(" no ```jsonc blocks found — did the fence style change?") + +for start, block in blocks: + text = strip_line_comments(block) + for pattern, replacement in PLACEHOLDERS: + text = pattern.sub(replacement, text) + + try: + values = list(json_values(text)) + except json.JSONDecodeError as e: + check(f"SPEC.md:{start} parses as JSON", False) + print(f" {e}") + continue + + for offset, value in enumerate(values): + # An envelope validates against the root schema; a bare payload example + # (§6's frame anatomy) validates against its own $def. Naming the target + # keeps this auditable rather than magical. + if isinstance(value, dict) and value.get("type") in ENVELOPE_TYPES: + target, schema = f"envelope {value['type']}", SCHEMA + else: + target = "ContextFrame" + schema = { + "$schema": SCHEMA["$schema"], + "$ref": "#/$defs/ContextFrame", + "$defs": SCHEMA["$defs"], + } + label = f"SPEC.md:{start}" + if len(values) > 1: + label += f" [{offset + 1}/{len(values)}]" + try: + jsonschema.validate(value, schema) + except jsonschema.ValidationError as e: + check(f"{label} ({target})", False) + print(f" {e.message}") + continue + check(f"{label} ({target})", True) + print(f"\n{'OK — all examples validate' if failures == 0 else f'{failures} failure(s)'}") sys.exit(1 if failures else 0) From 2ae2f4c9d6e5c96720d6e18eab4028c010aa9884 Mon Sep 17 00:00:00 2001 From: macanderson Date: Sun, 26 Jul 2026 11:58:55 -0700 Subject: [PATCH 2/2] fix: restore the two validate-examples.py checks the merge dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving the conflict with #63 kept this branch's version of `schema/validate-examples.py` wholesale, silently discarding both checks #63 had added to it on main: * the reference-SERIALIZED vector validation (issue #54) — the half that catches schema/serializer disagreement, which curated examples cannot; * the `$id` + served-copy check, asserting `$id` names the live domain and `site/public/schema/` is byte-identical to the source. Neither loss was visible in CI: both are additive validations, so deleting them removes coverage without failing anything. The PR was green with the checks gone. The second loss was not merely theoretical. This branch had edited the ContextQuery `$comment`, so the served copy under `site/public/schema/` had already drifted from the source — the exact "stale schema that still resolves" failure #63's comment warns about. The merge introduced the drift and deleted the detector in one move. Restoring the check fails immediately on it, which is how the drift was found. Also drops this branch's ContextQuery `required` edit in favour of main's. #63 fixed that bug independently and identically; its `$comment` is the better of the two (it names the conformance suite's own sample_query and records that absence and mean the same thing). `schema/` is now byte-identical to main, and the CHANGELOG entry is reworded to claim only what this branch still contributes: the regression test and the cross-audit, not the schema change. Section numbering reconciled: 1-2 examples/, 3 reference vectors, 4 SPEC.md, 5 schema identity. validate-examples.py: 39 checks pass (24 before the merge, 0 dropped). cargo test --workspace: 292 passed, 0 failed. fmt, clippy -D warnings, and conformance-{green,red}/host all pass. --- CHANGELOG.md | 16 ++-- schema/contextgraph-envelope.schema.json | 2 +- schema/validate-examples.py | 93 ++++++++++++++++++++---- 3 files changed, 88 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f26f89..33f156b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,14 +107,14 @@ which. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1 validates every fenced `jsonc` block in `SPEC.md` too (comments and documented placeholders normalized away, structure checked), and CI's existing `schema` job therefore catches this class of drift. -- **JSON Schema: an ordinary `ContextQuery` was un-representable.** `kinds` and - `anchors` were listed as `required` while both are - `skip_serializing_if = "Vec::is_empty"` in the reference type, so an - unfiltered, unanchored query — what a host sends when it wants anything - relevant — failed schema validation. Exactly the bug fixed for `ContextFrame` - below, one type over. A test now asserts an ordinary query satisfies the - schema's own `required` array, and a cross-audit of all 16 shared types - confirms no other type demands a field its serializer elides. +- **Regression guard for the `ContextQuery` `required` fix.** The schema change + itself landed independently in #63; this adds the test that keeps it fixed — + an ordinary unfiltered, unanchored query must satisfy the schema's *own* + `required` array (read from the schema, so it cannot drift into a stale + snapshot). A cross-audit of all 16 shared types confirms no other type demands + a field its serializer elides — this bug class has now recurred twice + (`ContextFrame` in PR #44, `ContextQuery` in #63), so it is worth a standing + check rather than another one-off fix. - **§G2 and §D1 are now actually verified, not merely asserted.** Both named `frame-validity` as their verifier while neither `target_uri` nor the frame's own `content_digest` was read by any check — the self-attestation §11.1 diff --git a/schema/contextgraph-envelope.schema.json b/schema/contextgraph-envelope.schema.json index 512095e..488739a 100644 --- a/schema/contextgraph-envelope.schema.json +++ b/schema/contextgraph-envelope.schema.json @@ -441,7 +441,7 @@ "max_frames", "max_tokens" ], - "$comment": "Required is exactly what the reference serializer (contextgraph-types ContextQuery) always emits: goal, max_frames, max_tokens. kinds and anchors are skip_serializing_if=Vec::is_empty, so an unfiltered, unanchored query — the most ordinary query there is — omits them; listing them as required made that query un-representable in conformant JSON. Same bug class as the ContextFrame required list below (PR #44): the schema must not demand fields the canonical serializer elides.", + "$comment": "Required is exactly what the reference serializer (contextgraph-types ContextQuery) always emits: goal, max_frames, max_tokens. `kinds` and `anchors` are skip_serializing_if=Vec::is_empty in the reference type, so an unfiltered, unanchored query — the most common query there is, and the one the conformance suite's own sample_query() sends — serializes without them. Listing them as globally required made that query un-representable in conformant JSON, the same class of bug ADR 0006 surfaced on ContextFrame. An empty array remains valid; absence and [] mean the same thing (no filter).", "additionalProperties": false }, "ContextQueryResult": { diff --git a/schema/validate-examples.py b/schema/validate-examples.py index 32c7cb7..3e11a23 100755 --- a/schema/validate-examples.py +++ b/schema/validate-examples.py @@ -5,15 +5,26 @@ Usage: python3 schema/validate-examples.py -Exits 0 if every message in examples/ AND every fenced example in SPEC.md is -valid under schema/, and 1 otherwise. +Exits 0 if every message in examples/, every reference-serialized vector, and +every fenced example in SPEC.md is valid under schema/ — and the schema's `$id` +resolves to a byte-identical served copy. Exits 1 otherwise. No third-party dependencies beyond `jsonschema` (pip install jsonschema). -SPEC.md is checked because it was the one example surface nothing validated, and -it drifted: the §9 `verify` example carried an `id` member that §3.2 grants only -to `query`/`frames`/`error`, that the reference envelope has no field for, and -that the schema's `additionalProperties: false` rejects. The spec's own examples -are now held to the spec's own schema. +The four example surfaces are deliberately different in kind: + + * `examples/` is hand-authored — it proves the schema accepts what a human + writes, and is what a provider author diffs against. + * `reference-vectors.ndjson` is machine-serialized from the reference Rust + types — it proves the schema accepts what the canonical serializer actually + emits (issue #54). Curated examples cannot catch that class. + * `SPEC.md` is the normative document. It is checked because it was the one + example surface nothing validated, and it drifted: the §9 `verify` example + carried an `id` member that §3.2 grants only to `query`/`frames`/`error`, + that the reference envelope has no field for, and that the schema's + `additionalProperties: false` rejects. The spec's own examples are now held + to the spec's own schema. + * the served `$id` copy is the schema's public identity — checked because a + stale schema that still resolves is worse than one that 404s. """ import json import re @@ -64,14 +75,40 @@ def check(label: str, ok: bool) -> None: continue check(f"{ref_path.name} message {i} ({msg['type']})", True) -# 3. SPEC.md's own fenced examples. +# 3. Reference-SERIALIZED vectors — one envelope per line, generated by +# `contextgraph-conformance/tests/reference_vectors.rs` from the reference +# Rust types rather than authored by hand. # -# These are jsonc — `//` comments, and prose placeholders standing in for values -# whose real form is uninteresting to a reader (`"…"`, `sha256:<64 hex>`). Both -# are normalized away before validation, because what is being checked here is -# STRUCTURE: member names, required members, types, and above all -# `additionalProperties` — the class of error that put an `id` on a `verify`. -# Substituting a placeholder never invents a member, so it cannot mask that. +# This is the half that catches schema/serializer disagreement (issue #54). +# Curated examples only ever prove the schema accepts what a human wrote; a +# field marked `required` here but omitted by `skip_serializing_if` in the +# Rust type stays invisible until some downstream serializes a real value and +# gets rejected. Validating the serializer's own minimal output — empty vecs, +# `None` options — makes that class of bug a red build instead of a +# downstream bug report. +vectors_path = ROOT / "schema" / "reference-vectors.ndjson" +vector_lines = [l for l in vectors_path.read_text().splitlines() if l.strip()] +if not vector_lines: + sys.exit(f"error: {vectors_path.name} is empty — regenerate it " + "(REGENERATE_VECTORS=1 cargo test -p contextgraph-conformance --test reference_vectors)") +for i, line in enumerate(vector_lines, 1): + try: + jsonschema.validate(json.loads(line), SCHEMA) + except (json.JSONDecodeError, jsonschema.ValidationError) as e: + check(f"{vectors_path.name} line {i}", False) + print(f" {e}") + continue + check(f"{vectors_path.name} line {i} ({json.loads(line)['type']})", True) + +# 4. SPEC.md's own fenced examples. +# +# These are jsonc — `//` comments, and prose placeholders standing in for +# values whose real form is uninteresting to a reader (`"…"`, +# `sha256:<64 hex>`). Both are normalized away before validation, because +# what is being checked here is STRUCTURE: member names, required members, +# types, and above all `additionalProperties` — the class of error that put +# an `id` on a `verify`. Substituting a placeholder never invents a member, +# so it cannot mask that. ENVELOPE_TYPES = { SCHEMA["$defs"][ref["$ref"].rsplit("/", 1)[-1]]["properties"]["type"]["const"] for ref in SCHEMA["oneOf"] @@ -171,5 +208,33 @@ def _skip_ws(text: str, index: int) -> int: continue check(f"{label} ({target})", True) +# 5. The schema's `$id` must dereference to this exact schema. +# +# `$id` is the schema's public identity — the URL third parties resolve and +# quote. It pointed at `context-graph-protocol.org`, a hyphenated host that +# was never registered and returned a DNS failure, so every consumer that +# tried to fetch it got nothing. Now it names the live domain, and the site +# serves the file from `site/public/schema/`. +# +# A served copy can drift from the source of truth, which would be worse than +# a 404: a stale schema that still resolves is one that silently validates +# the wrong thing. So the copy is asserted byte-identical here rather than +# trusted to be refreshed by hand. +SCHEMA_SOURCE = ROOT / "schema" / "contextgraph-envelope.schema.json" +SCHEMA_SERVED = ROOT / "site" / "public" / "schema" / "contextgraph-envelope.schema.json" +expected_id = f"https://contextgraphprotocol.org/schema/{SCHEMA_SOURCE.name}" + +check(f"$id is {expected_id}", SCHEMA.get("$id") == expected_id) + +if not SCHEMA_SERVED.exists(): + check(f"site serves the schema at {SCHEMA_SERVED.relative_to(ROOT)}", False) + print(f" missing — copy it: cp {SCHEMA_SOURCE.relative_to(ROOT)} {SCHEMA_SERVED.relative_to(ROOT)}") + failures += 1 +else: + identical = SCHEMA_SERVED.read_bytes() == SCHEMA_SOURCE.read_bytes() + check("the served schema copy is byte-identical to the source", identical) + if not identical: + print(f" refresh it: cp {SCHEMA_SOURCE.relative_to(ROOT)} {SCHEMA_SERVED.relative_to(ROOT)}") + print(f"\n{'OK — all examples validate' if failures == 0 else f'{failures} failure(s)'}") sys.exit(1 if failures else 0)