Write the record to disk as a bundle a stranger can verify - #650
Merged
Conversation
Seven steps built an exporter that produced structures in memory and never an artifact. The Definition of Done was "a rehearsal runs, the exporter writes a bundle, the standalone verifier passes on it in a subprocess" and nothing wrote the bundle, so nothing had ever verified end to end. The layout is the one the design fixed: manifest.json, streams.jsonl, logbooks/<kind>.jsonl. JSON Lines because a reviewer can stream, grep and diff it per row. The trap that carries is that the line format is NOT what the hashes cover, so a verifier must REASSEMBLE the body before hashing. Reassembly therefore exists twice, once in the package and once in the standalone script, and the duplication is the point: a checker importing CORA's reassembly would confirm CORA's own idea of what the bundle says. That is not a check. A new fitness test pins the script's zero-import property, because the duplication is a standing invitation to deduplicate it away and its failure mode is silent. The gap that only appears end to end: a bundle whose files are each individually well-formed can still be missing a whole logbook kind. Per-file verification passes; only the reassembled body's hash catches it. Both suites assert exactly that, alongside tamper and line-reorder sensitivity, because order is part of the record rather than presentation. H3 lands here too, as step 7 asked, not as an afterthought: the published record's own hash, the one a paper prints beside its locator and the only one covering bytes a reader can hold. Its payload type is deliberately distinct from H1's, so an export whose redaction happened to drop nothing still hashes differently as a published record than as a full one. They must never collide. The manifest carries it as optional, and None means "not a published projection" rather than "redaction found nothing": the absence is the signal, and the verifier refuses --published against a bundle lacking it rather than falling back to H1. Writing into a non-empty directory refuses, and the manifest is written last, so an interrupted export leaves something read_bundle_body rejects rather than a complete-looking bundle with truncated rows. Found while writing the end-to-end test and NOT fixed here, because loosening a fail-closed check is a security decision rather than a bundle writer's business: ensure_all_clearances_fired refuses unless every declared activity/payload clearance fires, and those three keys live on different step kinds, two of them optional. redact_record therefore refuses any export whose activities lack a setpoint-with-units AND an action AND a check. It fails closed, so nothing leaks, but the first small rehearsal bundle would hit it and read as a config error. Step 6's own fixture already works around it with a comment reasoning that a real export "would plausibly exercise every step kind" -- an assumption about data holding up a hard refusal. Documented at both sites; owed a decision. Refs: project_record_export_v3.md F4/F5, project_record_export_build_brief.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`redact_record` used to raise unless every declared activity/payload clearance (channel, action_name, units) fired. Those three keys live on different step kinds and two are optional, so no narrow export -- most likely the first rehearsal bundle -- reliably fires all three. The check reasoned from a denylist's threat model (an unfired rule that should have hidden something is a leak) and applied it backwards to tier 2's allowlist, where an unfired rule means a field was published LESS than the profile permits, never more. There is no mechanism by which that leaks anything. unfired_clearances now returns the (kind, column, pointer) triples instead of raising, and RedactionResult / build_manifest thread the result onto the manifest as a completeness fact a reader can see, not a reason to refuse. A new regression test seeds the exact shape that used to abort (one setpoint, no units, no action, no check) and asserts it now redacts, verifies against H3, and the manifest names the two clearances the export was too narrow to exercise. The genuine worry the old check was reaching for -- a misspelled pointer in TIER2_JSONB_CLEARED_POINTERS that can never fire against any real payload -- is a fact about this file's code, not about any one export, and now has its own build-time check: test_tier2_jsonb_ clearances_are_real_keys.py. outcome.measurements gets a real introspection check against Measurement's actual dataclass fields; activity.payload has no typed contract, so it hand-encodes the known keys from append_activities/route.py's docstring plus the envelope keys conductor.py's _append_step merges into every kind. Both checks verified to fire on an injected typo and restore clean. Writing that hand-encoded key list surfaced two things beyond the completeness check's own scope, recorded in its docstring rather than acted on here: `STEP_KIND_VALUES` has five members and the payload docstring documents three (capture/compute are undocumented), and `result` -- a string closed in practice to exactly three constants (_RESULT_OK/_RESULT_FAILED/_RESULT_IN_FLIGHT, verified by grepping every call site) -- is written on every conductor-driven activity row and is not in the clearance set, so the published record cannot currently distinguish a step that succeeded from one that failed. Not added here: changing what gets published is a content decision, not a typo check. Refs: project_record_export_v3.md F5, project_record_export_build_brief.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits finishing the record exporter's Definition of Done. Seven
merged steps produced structures in memory and never an artifact, so
nothing had ever verified end to end.
Bundle writer + H3 (
5699dfd293). Writesmanifest.json,streams.jsonl,logbooks/<kind>.jsonl. The line format is NOT whatthe hashes cover, so a verifier must reassemble the body before
hashing; that reassembly deliberately exists twice, once in the package
and once in
scripts/verify_record_hash.py, because a checkerimporting CORA's reassembly would only confirm CORA's own idea of what
the bundle says. A fitness test pins the script's zero-import property.
H3, the published record's own hash, lands here with a payload type
distinct from H1's so the two can never collide;
Nonemeans "not apublished projection", and
--publishedrefuses rather than fallingback to H1. Writing into a non-empty directory refuses, and the
manifest is written last so an interrupted export leaves something
read_bundle_bodyrejects.Unfired tier-2 clearances (
9144168891).redact_recordused toraise unless every declared activity/payload clearance fired. Those
keys live on different step kinds and two are optional, so no narrow
export reliably fires all three. The check applied a denylist's threat
model to tier 2's allowlist, where an unfired rule means a field was
published LESS than the profile permits, never more. It now reports the
(kind, column, pointer)triples onto the manifest as a completenessfact. The genuine worry underneath, a misspelled pointer that can never
fire, gets its own build-time check instead.
Ready for the first live rehearsal against the 2-BM record.
Refs:
project_record_export_v3.mdF5,project_record_export_build_brief.md🤖 Generated with Claude Code