Uh oh!
There was an error while loading. Please reload this page.
test(packages): gate that a package door's field allowlist cannot silently drop a stamped or declared key - #14830
Open
os-trump wants to merge 6 commits into
Open
Conversation
…drop a stamped key
Both `/packages` doors project the installed-package record through a
hand-written field allowlist. The trade that bought was explicit: drift
shows up as a missing field, never a 500. Within one day of it landing,
an ADR-0070 D2 `writable` verdict started reaching both doors and both
would have dropped it — a 200 with the field simply absent. One door was
saved by someone reading a sibling pin, the other by a merge conflict.
The field-specific pins that followed cover `writable` and nothing else.
These two gates cover the general case, deriving both sides from real
code rather than a hand-kept key list:
- REST door: `served ⊇ getMetaItems({type:'package'}) keys`, measured
through the real `ObjectStackProtocolImplementation` over a real
`SchemaRegistry`.
- runtime door: `served ⊇ record keys`, plus a set-equality register
for the keys the door stamps AFTER the projection — measured as
`served − record`, so a reorder empties it and reds.
The two doors solved the near-miss differently (the REST allowlist
contains `writable`; the runtime one deliberately does not and orders
instead), so the invariants are stated separately rather than assumed
symmetric. Only hand-kept artifacts are the annotated exclusion and
stamp registers, both compared loudly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza…esence `SchemaRegistry.installPackage` seats the optional record fields as own properties holding `undefined`, so `'settings' in record` is true for a package installed without settings. Both doors omit undefined-valued fields deliberately and JSON.stringify drops them anyway, so counting them made the gate red on every package for a key no consumer could have observed — measured on the first run, six false drops. Also drops the fixture's `findOne` double: `getMetaItems` never reaches that verb, and `check:engine-double-contract` is right that a fake looser than ObjectQL.findOne is worth refusing rather than stubbing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
… all of them The ablation caught this and it is the more useful of the two findings: deleting `installedVersion` from EITHER door's allowlist left both gates GREEN. `installPackage` leaves 8 of the 12 declared fields as own properties holding `undefined`, and a key the wire cannot carry is correctly invisible to the detector — so the coverage assertion was exercising 4 fields while reading as if it covered the record. `seatDeclaredFields` fills every own key whose value is `undefined`, derived from the record's own key set rather than from a list of field names, so a field added tomorrow is seated without an edit. A control assertion now fails if any declared slot is unobservable again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…Schema The first ablation's most useful reading: deleting `installedVersion` from either allowlist left both gates GREEN. Six of the twelve declared record fields are ABSENT from a freshly installed record (installPackage writes only what an install can know), so a gate watching only the producer's live output cannot see them dropped. The card asks for `served ⊇ the producer's stamped/DECLARED key set`, so the declared half is derived from the record schema — in the TEST's expectation, never in the production allowlist, which stays hand-written and untouched. That distinction is the one the originating card ruled on and it is spelled out at DECLARED_RECORD_KEYS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
…ckage-door-field-allowlist-gate
os-trump
marked this pull request as ready for review
September 3, 2026 05:19
os-trump
enabled auto-merge
September 3, 2026 05:19
os-project-manager
disabled auto-merge
September 3, 2026 06:34
os-project-manager
enabled auto-merge
September 3, 2026 07:48
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#14642
A gate that goes red when a
/packagesdoor's field allowlist drops a key theproducer stamps or the record declares. Both sides of the comparison are derived
by running real code; the only hand-kept artifacts are two annotated registers,
both compared loudly.
Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
File face
Two files, both new, both tests:
packages/rest/src/package-door-producer-key-carry.test.tspackages/runtime/src/domains/package-door-producer-key-carry.test.tsNo production file is modified. In particular
packages/rest/src/error-response.tsis not touched — PR #14713 is in flight on it and the gate's honest landing
site was never that file. No CI job added, no CI filter root widened.
Base refreshed by merging
origin/main(not rebasing, no force-push) at47181632f2. That merge matters: PR #14785 landedpackages/runtime/tsconfig.test.jsonwhile this branch was open, so the runtime pin is now compiled by a real tsc
program. See Verification.
Is the near-miss still reachable on today's
main?writableitself: no. Both doors carry it now, by two differentmechanisms, and both are pinned by name (
package-list-writable-carry.test.ts,packages-serializable-response.test.ts).one field. The ablation below reintroduces the drop four different ways and
every one of them shipped a 200 with a field silently absent before this PR.
So the card's premise stands, with the triage comment's sharpening: the gap is
the next stamped or declared key, not
writable.The two doors do not share an invariant, and the gate does not pretend they do
getMetaItems({type:'package'})registry.getAllPackages()writablecomes fromAsserting "the allowlist contains every stamped key" at the runtime door would
red on a correct door. So the runtime pin measures the stamp set as
served − record— never a list — and compares it for set equality againstan annotated register. A reorder empties that measured set and reds; a new stamp
grows it and reds until the decision is written down. That register is hand-kept
on purpose: swapping a silent list for a loud one is the mechanism this card
asked for.
Both gate readings
Four ablation legs, each mutating one production line, each restored and proven
restored. The mutation is proven on disk by an anchor count before/after, never
by the editor's exit code. Re-run in full after the merge; every number below
is identical before and after, so the merge moved nothing.
'writable'fromREGISTRY_PACKAGE_RESPONSE_FIELDSlist/detail PKG: writable, 6 drops across 3 packages'installedVersion'from the same allowlistlist/detail PKG: installedVersion, 6 dropstoPackageResponse(withWritableVerdict(...))stamp set for com.example.showcase: expected [] to deeply equal [ 'writable' ]'installedVersion'fromINSTALLED_PACKAGE_RESPONSE_FIELDSlist/detail PKG: installedVersion, 6 dropsLeg 1 is the exact near-miss the card is about. Leg 3 is the exact regression the
runtime door's ordering comment warns about. Legs 2 and 4 are the generality
claim: a plain declared field, with no field-specific pin anywhere.
No rebuild leg is involved and none is owed. Both mutated files are resolved
by vitest as local source — the REST pin imports
./package-routes.js, theruntime pin reaches
./domains/packages.jsthrough../http-dispatcher.js— sodist/is not on the resolution path for either subject and there is nothing topreflight. The restore leg is proven by
git diff HEADbeing empty for bothdoors after every leg, not by the restore command's exit code.
Two findings the ablation produced, both now fixed in this PR
Recording these because in both cases the first version of the gate was green
for a bad reason, and a reviewer should see that the readings above are not the
first ones taken.
Object.keyswas the wrong instrument.installPackageseatssettingsas an own property holding
undefined. Both doors omit undefined-valuedfields deliberately, and
JSON.stringifydrops them anyway, so the first runreported six false drops for a key no consumer could ever have observed. The
detector now measures defined-valued keys.
The fixture could not see two thirds of the record. Deleting
installedVersionfrom either allowlist left both gates green on thefirst ablation:
installPackagewrites only the fields an install can know,so six of the twelve declared fields are simply absent from a freshly
installed record.
seatDeclaredFieldsnow seats every field the recordschema declares, and a control assertion fails if any declared slot becomes
unobservable again.
On
packages/specThe card rules out deriving the production allowlist from
packages/spec,and this PR does not: both allowlists stay hand-written and are untouched. What
finding 2 derives from the schema is the test's expectation, which is the
card's own wording for the detector — "the projected key set ⊇ the producer's
stamped/declared key set minus an explicit, annotated exclusion list".
Deriving the expectation is precisely what turns a newly declared field into an
explicit decision at each door instead of a silent omission. It adds no import
edge either:
@objectstack/specis already a runtime dependency of bothpackages. The rationale is spelled out at
DECLARED_RECORD_KEYSin both files soit cannot be mistaken for the rejected design.
Why two pins rather than one detector
Measured, not assumed. A single file would have to reach the other package's
door:
packages/runtime,@objectstack/restresolves to itsdist/(novitest alias maps it to source), which would make the gate's verdict a function
of build state. It would also grow
KNOWN_UNALIASED_TEST_IMPORTSinscripts/check-test-source-alias.mjs, a shrink-only registry that lists no@objectstack/restentry for that package today.@objectstack/restcannot import@objectstack/runtimeat all — thedependency runs the other way.
So: two pins, one shape. Each imports its own door as source, which is also
what lets a reviewer ablate one allowlist and watch exactly one gate go red.
What I did NOT do, and why
the detector, not an edit to the lists.
packages/rest/src/error-response.ts(hot file, PR fix(rest): consult the bespoke structured arms before the declared-status passthrough, so both error doors answer one refusal with one body #14713).package publishes a test file, so nothing user-visible ships.
skip-changesetapplied.
packages/runtime/test-typecheck-debt.json, and didnot need to — see Verification. That ledger is byte-identical to
origin/mainin this branch.
any,@ts-expect-error, oreslint-disablewas added anywhere, and no assertion was weakened. Theload-bearing comparison — set equality on the stamp set — runs over
Set(string)andstring[], notany.Verification
All readings from
47181632f2, the merge commit at the head of this branch.The dependency closure was built first (
pnpm --filter '@objectstack/runtime^...' build),because an unbuilt closure turns every workspace import into TS2307 and makes a
typecheck verdict a statement about a different world.
The runtime test-layer typecheck, which the merge newly applies to this PR
pnpm --filter @objectstack/runtime typecheck— EXIT=0. It now chainscheck:test-typecheck, whose own verdict line is:Three separate confirmations that the new runtime pin is genuinely covered and
carries no debt:
tsc --noEmit --project tsconfig.test.json --listFilesshows 1 hit forsrc/domains/package-door-producer-key-carry.test.ts— it is in the program.(This replaces the pre-merge reading of 0 hits, which was true on the old base
and is now false.)
the ledgered ones in the other 27 files.
packages/runtime/test-typecheck-debt.jsonis byte-identical toorigin/main's copy — nothing added, no number moved.Other green
packages/restgate — 4/4;packages/runtimegate — 5/5.package-registry-item-projection,package-list-writable-carry,packages-serializable-response,packages-writable-verdict— 3 files /14 tests and 3 files / 23 tests, both EXIT=0.
pnpm --filter @objectstack/rest typecheck— EXIT=0, with--listFilesconfirming 1 hit for the new REST test file.
scripts/pm/dispatch-gates.mjs --commands— exit 0, re-derived and re-run on the merged head (the family list is
unchanged, and the change set is still exactly these two files).
check:engine-double-contractwas RED early on (my fixture'sfindOnedouble)and is green after removing that verb, which the producer path never calls.
Not measured, with the reason
check:dual-build-cjs-loadsandcheck:type-check-debt --re-measure— exit3, "PREREQUISITE NOT MET": both need the whole workspace built
(
turbo run build --filter='./packages/*' --filter='./packages/*/*'), not justone closure. Exit 3 is neither a pass nor a finding. The sub-measurement that
this diff could actually have moved — the runtime test-layer debt — is answered
directly above by
check:test-typecheckat EXIT=0 with the ledger unchanged.And
check:dual-build-cjs-loadsreads built output, which a diff adding only*.test.tsfiles cannot reach.check-test-completeness— exit 3; it needs a savedturbo run testlog,and its own failure text says to record it NOT MEASURED when run locally.
Narrowed, declared
The two packages' FULL suites were not run. Nine attempts on the shared verify
lock across this branch never got a long enough turn — holders included one run
at 917s and another at 1704s — and
@objectstack/runtime's whole suite exceedswhat fits inside the container's foreground cap on a shared box. Narrowed to the
two new files plus the four sibling pins on the same two doors, all green above.
The diff modifies no production file, no config, and no turbo/vitest setting, so
the only way a sibling test's verdict could move is cross-file interference
inside a vitest worker; CI runs the full sharded suite on this PR regardless.
Generated by Claude Code