Repository navigation
feat: bind records to stable input and candidate snapshots (E03) - #420
Conversation
core/engineering/snapshots.ts turns what a host observed into the identity E01 defined. Pure: the host walks the tree, this normalizes, excludes, orders, and judges coverage. The rule that shapes it: identity must not silently overstate what was observed. - coverage is inside the digest, so a capture that quietly stopped covering a path is not byte-identical to one where the path was read and unchanged. - An unreadable or host-skipped file becomes an uncovered_relevant_input rather than an omission, and a non-empty list blocks acceptance: the collector knows it did not look at something relevant, so an unchanged digest cannot mean an unchanged tree. - A secret file is excluded by name and its digest is never recorded (a digest of a credential file is still an oracle for it); the exclusion is disclosed so partial coverage is visible. - stability stays outside the digest, so re-capturing an unchanged tree does not look like an edit. - A duplicate path is refused outright; a symlink carries its link text and is never followed, including one that escapes the repository. - Same HEAD with changed working bytes is a different identity, so dirty-tree evidence cannot be replaced by a revision. diffSnapshots explains what moved, including coverage drift a manifest diff would hide. It complements checkCandidateFreshness rather than replacing it. Registered snapshots.ts in E01's module-graph test at the same layer as validation, and widened its pure-import allowlist to ../secrets.ts so both surfaces withhold the same files; secrets.ts is itself pure. Verified: 14/14 focused (RED first: 13 failures before the barrel export), 987/987 full suite, build exit 0, diff --check clean. Eight mutation checks each turn the suite red: dropping coverage from the identity (3), digesting secrets, dropping unreadable paths, un-blocking uncovered inputs, tolerating duplicates, unsorting the manifest (2), ignoring mode changes, ignoring coverage drift.
An independent adversarial review found that the module validated paths and inputs AFTER deciding what to do with them, and that its own acceptance gate repeated the mistake E01 was corrected for twice. - A path was matched against exclusions and isSecretFile before being checked. So "/home/x/.netrc", "../../.npmrc", and a NUL-bearing path all returned ok:true with the raw string recorded in coverage.excluded — and coverage is inside the digest. Paths are now validated first. - candidateMayBindAcceptance read coverage?.uncovered_relevant_inputs ?? [], so omitting coverage entirely, or passing null/""/5, returned ok:true. A degradation that RAISES trust is the exact defect E01's reviews removed; an unreadable coverage now blocks. - Duplicate detection ran after the unreadable branch, so file+unreadable for one path produced a snapshot claiming that path was both fingerprinted and not covered. One path now means one observation, in any combination. - patternCovers silently returned false for *.log and **/*.log, so those exclusions were disclosed in the digest while every matching file stayed in the manifest — disclosed coverage narrower than actual, inverting the point of coverage. Supported shapes are now matched, and an unsupported pattern is refused rather than recorded. - Only the secret case was deduped, so declaring one exclusion twice changed the identity of an identical tree. - repository was digested verbatim: arbitrary keys, a string dirty, or a float head all reached the identity. It is now shape-checked. Verified: 20/20 focused, 993/993 full suite, build exit 0, diff --check clean. Six new mutation checks each turn the suite red (moving the path check back, making coverage optional, dropping duplicate detection, accepting unsupported patterns, removing exclusion dedup, removing repository validation).
A second adversarial review (Fable 5.1) targeted the fixes from the first
review and found three more ways two different trees reach one digest, plus
three spurious-drift bugs.
Collisions:
- executable was coerced: `entry.executable === true` turned 1, "true" and
{} into false, so a script WITH the executable bit shared the digest of one
without it. Observed: executable:1 and executable:false both 8d7b8e822812.
Non-booleans are now refused. This is the same coercion class as the
repository fix in a63fa75 \u2014 the first pass fixed the instance, not the rule.
- compareUtf8 encodes to UTF-8, which maps every unpaired surrogate to U+FFFD,
so "a\uD800" and "a\uFFFD" compared EQUAL. A non-total comparator made the
manifest sort order-dependent: the same tree in two enumeration orders gave
c2ce6fbcf849 and 3f3c2eba3f2d. Such paths and patterns are now refused.
- String(repo.head) accepted ["a".repeat(40)], which reached the digest as an
array; a non-plain repository prototype threw out of canonicalJson instead
of returning ok:false. Both are now shape-checked. Fix 6 was incomplete.
Spurious drift and unapplied disclosures:
- Exclusions were deduped by pattern+reason, so re-declaring a built-in rule
under reason "host-declared" produced a second coverage entry and a second
identity for an identical tree. Deduped by pattern; a conflicting reason is
now an error, because one pattern has one meaning.
- `**` was a supported exclusion: it empties the manifest, so two unrelated
trees both digested 1b912bb41278 and the gate still passed. Refused.
- Two shapes of finding 4 survived: a bare directory name was disclosed but
matched nothing inside it, and `*.log` vs `**/*.log` disagreed about a file
named `.log`. Both spellings now require a non-empty stem and agree.
Also strengthened four tests the review showed were weaker than their titles:
the secret test now greps the ENTIRE serialized result including `dropped`
rather than only `coverage`, and the exclusion test asserts a matching file
really leaves the manifest.
Verified: 26/26 focused, 999/999 full suite, build exit 0, diff --check clean.
Seven new mutation checks bite. An eighth candidate guard turned out to be
unreachable (isRepoRelativePath already refuses `*`) and was removed rather
than shipped as dead code that looks load-bearing.
Second adversarial review (Fable 5.1) — three more collisions, all closed in
|
| Finding | Observed | Fix |
|---|---|---|
executable coerced |
executable: 1 and executable: false both digested 8d7b8e822812 — a script with the executable bit was indistinguishable from one without |
non-booleans refused |
| Non-total sort order | compareUtf8 encodes to UTF-8, mapping unpaired surrogates to U+FFFD, so "a\uD800" and "a\uFFFD" compared equal; same tree in two enumeration orders → c2ce6fbcf849 vs 3f3c2eba3f2d |
such paths/patterns refused |
String(head) |
head: ["a".repeat(40)] accepted and digested as an array; non-plain prototype threw instead of returning ok:false |
shape-checked |
| Exclusion dedup by pattern +reason | re-declaring a built-in rule under host-declared gave a second coverage entry and a second identity for an identical tree |
deduped by pattern; conflicting reason is an error |
** supported |
empties the manifest — two unrelated trees both digested 1b912bb41278, gate still passed |
refused |
| Finding 4 survived in two shapes | bare directory name disclosed but matched nothing inside it; *.log and **/*.log disagreed on a file named .log |
both spellings require a non-empty stem and agree |
The executable coercion is the same class as the repository fix in a63fa75 — the first pass fixed the instance, not the rule. That's the lesson worth keeping: when a review finds a coercion, sweep every coercion.
Test honesty: four tests were weaker than their titles. The secret test now greps the entire serialized result including dropped (not just coverage), and the exclusion test asserts a matching file really leaves the manifest.
Verification: 26/26 focused, 999/999 full suite, build exit 0, git diff --check clean. Seven new mutation checks bite. An eighth candidate guard turned out to be unreachable — isRepoRelativePath already refuses * — and was removed rather than shipped as dead code that looks load-bearing.
Closes #401.
core/engineering/snapshots.tsturns what a host observed into the snapshot identity E01 defined. Pure: the host walks the tree, this normalizes, excludes, orders, and judges coverage.The rule that shapes it: identity must not silently overstate what was observed.
coverageis inside the digest, so a capture that quietly narrowed cannot look identical to a fuller one.uncovered_relevant_inputrather than an omission, and a non-empty list blocks acceptance — the collector knows it did not look at something relevant, so an unchanged digest cannot mean an unchanged tree.stabilitystays outside the digest, so re-capturing an unchanged tree is not mistaken for an edit.diffSnapshotsexplains what moved, including coverage drift a manifest diff would hide. It complementscheckCandidateFreshnessrather than replacing it.Second commit: six bypasses found in review
An independent adversarial review (
a63fa75) found the module validated paths and inputs after deciding what to do with them, and that its own gate repeated the defect E01 was corrected for twice:/home/x/.netrc,../../.npmrc, NUL paths recorded raw inside the digestcoverage?.… ?? []in the gateok:true— a degradation that raised trustpatternCoversmissed*.log,**/*.logrepositorypassed throughdirtyreached the identityAll six closed and regression-tested.
Verification
RED first (13 failures before the barrel export). 20/20 focused, 993/993 full suite, build exit 0,
git diff --checkclean. Fourteen mutation checks each turn the suite red.Registered
snapshots.tsin E01's module-graph test at the same layer asvalidation.ts, and widened its pure-import allowlist to../secrets.tsso both surfaces withhold the same files;secrets.tsis itself pure.