chore(spec): classify what a restriction-shaped property's EMPTY value means (#3896 follow-up) - #3945
Merged
Merged
Conversation
…e means (#3896) #3929 fixed one field. `sys_sharing_rule.criteria_json` was optional, and its absence evaluated to `find(object, { filter: {} })` — every record of the object, granted to the recipient. Its description said so out loud: "leave empty to share every record." That sentence is the part worth generalising. For a platform whose premise is that agents author metadata, a field description is not documentation about the contract, it IS the contract the next author reads — and omission is the commonest error a model makes, precisely because it raises none. When omission also lands on the widest grant, the likeliest mistake is the most dangerous outcome, silently. Sweeping the spec found the same syntactic shape — an optional list or predicate that "restricts" something — carrying opposite meanings when empty: object.apiMethods undefined = unrestricted, [] = deny-all allowedSources "empty = all allowed" sharing condition nothing is shared (#3929) Nothing marked which was which. A maintainer knows by memory; a model cannot. New gate `check:empty-state`, wired into the Spec Liveness Check workflow: it scans the schema surface for statements declaring an empty state permissive and requires each to be classified in empty-state-registry.mts as `scope`, `closed`, `open` or `output`. Access-gate classifications must cite where the posture is enforced, resolved against the checkout so a rotted pointer is reported. 20 statements across 214 schema files are classified; a new unclassified one fails. Detection matches the STATEMENT, not field names — names would be a guess, and the liveness README is blunt about where that ends up ("a permanently-noisy check is a check nobody reads"). Negated tokens are ignored, so the ⚠ that object.zod.ts prints to warn an empty whitelist is DENY-ALL is not itself flagged as permissive. Statements resolving to no property are narrative and reported as non-failing notes. One correction: `DynamicLoadingConfig.allowedSources`, a supply-chain gate, documented [] as admitting every source. It now states the apiMethods three-state — undefined = any source, [] = deny-all, a subset = those types. The empty ARRAY is closed; only ABSENCE is open. Collapsing the two is what makes an allow-list vacuous. The field has no runtime consumer (the whole block is declared-but-unenforced, ADR-0049 false compliance, not addressed here), which is why the wording mattered: an unimplemented property's description is the spec whoever implements it builds to. It now carries an [EXPERIMENTAL] marker. Also registers the sharing-rule-criteria-required dogfood proof added by #3929, which had been declaring a @proof: tag the registry did not know about. No runtime behaviour changes. spec: 263 files / 6858 tests green, tsc clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/xl
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 105 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…urces contract The `check:docs` step of the "TypeScript Type Check" job regenerates content/docs/references/ from the spec and diffs it. Correcting the `allowedSources` describe text left the generated page stale. The regenerated line is the point of the change, not a side effect: the reference table now states the three-state contract — undefined = any source, [] = deny-all, a subset = exactly those types — where an author reads it, instead of "Restrict which source types are permitted", which said nothing about what emptiness does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QuViRSR1j6GJjf9qGbnqFX
os-zhuang
marked this pull request as ready for review
July 29, 2026 12:22
os-zhuang
added a commit
that referenced
this pull request
Jul 29, 2026
…config that never existed (#3950) Follow-up to #3896/#3945. Found while building the empty-state gate: `allowedSources` documented `[]` as admitting every source, and checking who enforced that turned up nobody. `DynamicLoadingConfigSchema`, `PluginDiscoveryConfigSchema` and `PluginDiscoverySourceSchema` promised sandboxing (`defaultSandbox`), integrity verification (`requireIntegrity`), source allow-listing (`allowedSources`) and admin approval (`discovery.requireApproval`) for dynamically loaded plugins. None of it was ever wired to anything — the three schemas were an island, referenced only by their own round-trip tests, yet published into json-schema/ and the authorable key surface where an author reads them as capabilities the platform has. That is ADR-0049 false compliance in its security shape, and the precedent (`tool.requiresConfirmation`, #3715) is to remove rather than mark dead. No tombstones: nothing ever parsed these schemas, so a retiredKey() prescription could never be delivered — the manifest and authorable-surface baselines are dropped as the deliberate removal both files document, with a block comment in the source recording the reasoning. Breaking in the narrow sense: the types were importable via the published ./kernel subpath. Nothing imported them; folds into the unreleased 17.0.0. `ActivationEventSchema` (7 real consumers) stays. spec: 263 files / 6856 tests green, tsc clean, all six spec gates green, reference docs regenerated.
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…ates (#3966) Follow-up to #3896/#3945. That gate required any "empty = permissive" statement in the spec to be classified on purpose — and its scan scope had a hole big enough to miss the bug it was built for. The sentence that shipped #3896, "leave empty to share every record", was the description of sys_sharing_rule.criteria_json, which lives in plugin-sharing. The gate could not see its own crime scene. Now scans **/*.object.ts anywhere under packages/ — plugins, platform-objects, metadata-core, and the create-objectstack templates, since a starter file is the highest-leverage thing a model copies from. 214 → 290 files. It immediately found a real one. sys_user_permission_set.organization_id declares "NULL = applies in every org context": a user↔permission-set grant with no org scope applies everywhere. Deliberate and load-bearing rather than an oversight — ADR-0095 D3 / ADR-0068 D2 derive the platform_admin posture from an UNSCOPED admin_full_access grant specifically, and an org-scoped grant of the same set must not confer it. The empty state is not merely wider; it is the distinguishing input to the highest privilege in the system. Registered `open` with that rationale and both enforcement sites cited. Three fixes the new surface forced: repudiated prose no longer fires (#3929's own "Deliberately NOT ..." comment was being flagged, with the escape kept narrow so a genuine claim carrying an unrelated negation is still caught); the owning property is resolved by nesting rather than a name list (skipping `description` answered `required`, the sibling above it — indentation is what separates a field from its config); and the property-search window is per-surface, since widening it globally would mis-attribute .zod.ts narrative to a distant property. Evidence is now parsed with the liveness ledger's own checkEvidence, so prose and multiple paths work — the README already promised that, and a raw-path existsSync quietly did not. 6 new unit tests (32 total). spec: 263 files / 6862 tests green, tsc clean, both gates green. No runtime behaviour changes.
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
2 participants
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.
Follow-up to #3929 (merged). That PR fixed one field; this one governs the class.
The generalisable part of #3896
sys_sharing_rule.criteria_jsonwas optional, and its absence evaluated tofind(object, { filter: {} })— every record of the object, granted to the recipient. The field description said so out loud: "leave empty to share every record."That sentence is what's worth generalising. For a platform whose premise is that agents author metadata, a field description is not documentation about the contract, it is the contract the next author reads. And omission is the commonest error a model makes, precisely because it raises none — nothing is missing from the artifact, the mistake is in what was never written. When omission also lands on the widest grant, the likeliest error is the most dangerous outcome, silently.
What the sweep found
The same syntactic shape — an optional list or predicate that "restricts" something — carries opposite meanings when empty, with nothing marking which is which:
object.apiMethodsundefined= unrestricted,[]= deny-allplugin-runtime.allowedSourcesconditionA maintainer knows by memory. A model authoring metadata cannot, and guessing wrong is silent and permissive.
The gate
pnpm --filter @objectstack/spec check:empty-state, wired as a second step into the existing Spec Liveness Check workflow (it already triggers onpackages/spec/**).It scans
packages/spec/src/**/*.zod.tsfor statements declaring an empty state to be permissive, and requires each to be classified inscripts/liveness/empty-state-registry.mts:semanticsscopeclosedopenoutputclosed/openmust cite where the posture is enforced; the path resolves against the checkout, so a rotted pointer is reported rather than trusted. 20 statements across 214 schema files are now classified (1 closed, 2 open, 4 output, 9 scope); adding an unclassified one fails CI.Why the liveness ledger's machinery and not a new lint rule. The ledger asks "does this property do anything?" —
criteria_jsonpassed that test at every layer. This asks the question it doesn't: what does its empty value mean? Same ratchet philosophy, sameevidenceresolution, same "no new undeclared surface" contract, one class at a time (ADR-0054 §3).Why prose and not field names. Names would be a guess; the statement is exact. The liveness README is blunt about where a guessy check ends up — "a permanently-noisy check is a check nobody reads" — so the detector:
object.zod.tsprints precisely to warn that an empty whitelist isDENY-ALLwould be flagged as if it were permissive, inverting the gate's meaning;metadata-persistence.environmentId: "new writes leave unset") stays quiet;:as a separator — every schema line has one;.describe()argument sits below it (searching one way mis-attributedexplain.zod.ts's statements to the next property down the file);One behavioural correction
DynamicLoadingConfig.allowedSources— a supply-chain gate — documented[]as admitting every source. It now states theapiMethodsthree-state:undefined= any source,[]= deny-all, a subset = exactly those types.The empty ARRAY is closed; only ABSENCE is open. Collapsing the two is what makes an allow-list vacuous — the one value an author reaches by mistake is also the widest grant.
The field has no runtime consumer: the whole
DynamicLoadingConfigblock (requireIntegrity,defaultSandbox,allowedSources) is declared and unenforced — the ADR-0049 false-compliance shape — and is not addressed here. That absence is exactly why the wording mattered: an unimplemented property's description is the specification whoever implements it will build to. It now carries an[EXPERIMENTAL — not enforced]marker so authors are not misled meanwhile.Also
Registers the
sharing-rule-criteria-requireddogfood proof added by #3929, which was declaring a@proof:tagproof-registry.mtsdid not know about — the liveness gate had started warning about the orphan. Unbound, for the same reason asshowcase-bu-hierarchy-sharing: sharing rules are authored at stack level, so there is no governed per-type ledger entry to ratchet.Verification
POST /data/sharing/rules绕过SharingRuleSchema:criteria缺失或拼错静默变成"共享该对象全部记录",与 ADR-0049 "never seeded as a permissive match-all" 直接冲突 #3896 shipped is caught;const empty = {},DENY-ALL, bare "leave unset", and:-as-separator are not.packages/spec: 263 files, 6858 tests, all passing.tsc --noEmitclean.check:authorable-surface,check:spec-changes,check:upgrade-guide,check:skill-docs) produce no diff.Not in this PR
DynamicLoadingConfigblock (above) — a real ADR-0049 enforce-or-remove item, security-shaped, deserving its own change.knowledge-source.mimeTypesis registeredscopebut is the vacuous-allow-list shape.storage.zod.tsalready models the identical concept the better way — an explicitmode: 'whitelist'discriminator plus.min(1), so an empty whitelist cannot be expressed at all. Preferred for new allow-lists; changing this one is a breaking change to an authorable field.required(write-time) fromNOT NULL(storage), which is what forced fix(sharing)!: a rule with no criteria shares NOTHING, not every record (#3896) #3929's invariant into imperative write guards rather than a declarativerequired: true. Still worth doing, but it is ergonomics — this PR covers the safety boundary.Generated by Claude Code