recipes: a silent observation is legacy, and a call is judged by its own session - #27
Conversation
… own session Three defects in the code v0.4.0 shipped, all found by automated review of #25. 1. An observation that stated no contract_recipe skipped the comparability guard, which only fired when a recipe was stated. A ledger written before 0.4.0 states none, so comparing an intact legacy ledger against a recipe-2 declaration reported contract_mutated against every observation it held - the tool accusing honest evidence of drift. Silence is now read the way a silent declaration is read, as recipe 1, and the comparison is refused with a recipe_mismatch finding that names the one-line change which judges that ledger under the recipe that produced it. 2. The summary reported only the declaration's recipe, so a ledger containing a recipe-1 hash was summarised as ["2"]. Every recipe present in the evidence is now listed. 3. Per-call contract hashes were derived from a separate tools/list session's manifest rather than the declaration the call session served, and the call session's manifest was not kept, so the claim could not be checked after the fact. capture_calls now persists its own session's manifest and build_pair derives from it; a capture that cannot supply one stops rather than borrowing another session's declaration. The filesystem capture was re-taken from the real server. Every contract hash it yields is unchanged from v0.4.0, so what moved is the provenance of the evidence, not the evidence. Signed-off-by: Empire Labs Pty Ltd <narko4u@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 163ffb27bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # before 0.4.0 was hashed under. Treating silence as "unknown" skipped | ||
| # this guard entirely and reported the two hashes as contract drift, | ||
| # which is a false accusation against an intact ledger. | ||
| observed_recipe = check_recipe(stated_recipe or CONTRACT_RECIPE_LEGACY) |
There was a problem hiding this comment.
When a known-tool observation omits contract_hash—a supported shape used by scope-only observations—the new fallback still assigns recipe 1. Against a recipe-2 declaration this emits a high-severity recipe_mismatch claiming that a nonexistent hash was computed under recipe 1, whereas the previous logic correctly skipped the contract comparison. Only infer or compare the observation recipe when a contract hash is actually present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed: #28, released as v0.4.2.
Reproduced on the released v0.4.1 code before changing anything:
declaration recipe: 2 observation states: none contract_hash: absent
findings: [('recipe_mismatch', 'high')]
summary.contract_recipes: ['1', '2']
Two false statements in one finding, both worse than a missing check: the detail asserted that a hash the evidence does not carry had been "computed under recipe 1", and the summary counted a recipe the ledger does not hold. The scope-only shape was never this guard's business either way — check 3 has always been what judges those observations.
The inference now stops at the hash:
findings: [] (clean)
summary.contract_recipes: ['2']
A recipe exists to interpret a hash, so with no hash there is nothing to infer and nothing to refuse. Control: the same observation carrying an argument outside the declared schema still returns scope_violation, so dropping the recipe claim did not leave scope-only observations unpoliced.
Rule 3 in README.md and docs/DESIGN.md now carries the condition — "silence means legacy, not unknown where there is a hash to interpret" — because the unqualified wording is what produced the wrong inference in the first place.
| # this guard entirely and reported the two hashes as contract drift, | ||
| # which is a false accusation against an intact ledger. | ||
| observed_recipe = check_recipe(stated_recipe or CONTRACT_RECIPE_LEGACY) | ||
| recipes_in_use.add(observed_recipe) |
There was a problem hiding this comment.
Recipe collection occurs only after the undeclared-tool early return, so an unknown-tool observation carrying a recipe-1 contract hash under a recipe-2 declaration is summarized as contract_recipes: ["2"]. This contradicts the new guarantee that the summary lists every recipe held by the evidence and also lets an unknown recipe on such an observation bypass check_recipe; parse and collect the observation recipe before the tool lookup can continue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed: #28, released as v0.4.2.
Both halves reproduced against v0.4.1:
undeclared tool, contract_recipe "1", vs a recipe-2 declaration
before: contract_recipes ['2'] the evidence holds 1 and 2
after: contract_recipes ['1', '2']
undeclared tool, contract_recipe "9"
before: accepted -- check_recipe bypassed entirely
after: ValueError: unknown contract recipe '9'
Recipe parsing now runs before the tool lookup, so a stated recipe is validated and counted wherever it appears rather than only on tools the declaration happens to carry. The summary guarantee added in v0.4.1 was untrue for that path, and a summary that quietly under-describes its evidence is the one kind of statement this tool should not be making. It is now rule 4 in README.md and docs/DESIGN.md.
Noted on the pairing: both defects came from the same shape — a guard placed after an early return, and a default applied without asking what it was defaulting about. That is what the new tests pin, each with a control, rather than just the two symptoms.
1 participant
Closes out the three findings from the automated review of #25. Each was reproduced against the shipped v0.4.0 code before anything was changed.
1. P1 — a silent observation produced a false drift verdict
validator.pyread an observation's recipe asobs.get("contract_recipe"), which isNonefor anything written before v0.4.0, and the comparability guard only fired when a recipe was stated. So the guard was unarmed for exactly the artifacts most likely to hit it: a pre-v0.4.0 ledger compared against a current declaration.Reproduced on
main@1574a84with a pre-0.4.0 observation (a recipe-1 hash and no recipe field):Three
contract_mutatedfindings — "annotation bound to contract that has changed since declaration" — against a ledger nothing had touched. That is the failure mode this tool exists to prevent, pointed the wrong way.Fix: silence is read the way a silent declaration is read, as recipe 1 (
check_recipe(stated or CONTRACT_RECIPE_LEGACY)), so the comparison is refused rather than guessed:The finding states the remedy, because a refusal with no way out is only half a fix: recipe 1 on the declaration compares the ledger under the recipe that produced it. Backward compatibility is unchanged — a legacy observation against a legacy declaration still validates clean (asserted as a control).
2. P1 — per-call hashes came from a different session than the call
build_pairrecomputed each call's hash fromobserved_raw, the manifest captured by a separatetools/listsession, and the call session's own manifest was never persisted. Against a server that varies its declaration per session, the pair could assert a contract that call never ran under.Verified rather than assumed: the committed capture diverges 0/3 — the two sessions agreed for every called tool, so the fixture was not wrong. What was wrong is that nothing recorded the call session's manifest, so the claim could not be checked at all.
Fix:
capture_callspersists the manifest its own session served (calls.jsonnow carriestools),build_pairderives per-call hashes from it, and a capture that cannot supply one stops (SystemExit) instead of borrowing another session's declaration. The capture script also warns at capture time if a called tool's declaration differs between the two sessions, since that gap is a finding, not a reason to pick the convenient session.3. P2 — the summary under-reported the recipes in the evidence
recipes_in_usecollected the manifest and per-tool declaration recipes but never the observation's, so a ledger holding a recipe-1 hash was summarised["2"]— andverifyprints that summary. Each validated observation's recipe is now added, so the ledger above reports["1", "2"], and the assertion that pinned the old behaviour is inverted.Capture re-take
The filesystem capture was re-taken from the real
@modelcontextprotocol/server-filesystem(2026.1.14declared,2026.8.31observed) so the committed evidence carries the manifest its session served. Every contract hash is identical to v0.4.0:Only timestamps moved. What changed is the provenance of the evidence, not the evidence.
Tests
46 passing (
pytest), with three new: a silent observation is read as the legacy recipe (including the legacy-declaration control), the call-session manifest agrees with the tools/list session (and the stored call-time hash equals both), and a capture without its call-session manifest is refused.ruff check src tests,bandit -q -r src -c pyproject.toml,rebuild_pair.py --check, and the README's documented commands were run on this tree; the README's shown output is unchanged.Docs updated where they were now wrong rather than merely incomplete: README and
docs/DESIGN.mdrecipe rules are three (they said two), and the recipe-mismatch row covers the silent case.