Skip to content

test(plugin-security,spec): pin the shared identifier schemas to the storage columns that bound them - #12584

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-12144-identifier-bounds-pin
Aug 26, 2026
Merged

test(plugin-security,spec): pin the shared identifier schemas to the storage columns that bound them#12584
huangyiirene merged 3 commits into
mainfrom
claude/issue-12144-identifier-bounds-pin

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #12144

Route taken: the PIN route (triage comment 5414530579's freely dispatchable half)

The shared identifier schemas (SystemIdentifierSchema, SnakeCaseIdentifierSchema, EventNameSchema) declare a floor and a grammar and no .max(), while every column that stores an identifier is bounded. This PR links the two surfaces so they cannot drift, the PR #12143 idiom: the pin reads the column widths off the registration surface (SecurityPlugin.init() → the manifest register({ objects }) call, and the SysMetadataObject declaration imported from @objectstack/metadata-core) rather than restating them, and probes the spec schemas against those live widths.

Accept/reject behaviour: UNCHANGED (Clause-② statement). The final diff changes no accepted or rejected value anywhere: the spec edit is TSDoc only (no .describe(), no validator change), the new file is a test, the docs page is regenerated from the TSDoc, and the changeset is prose. The pin itself asserts acceptance is unchanged at every measured width, and the ablation below shows it turning red under exactly the narrowing the triage fenced.

Why no per-surface .max() (the conditional limb, measured and declined)

The conditional limb required each surface to carry a .max() matching its own enforced ceiling exactly. Measured against origin/main:

Storing column Owner (registration surface) Enforced width Spec schema typing the value
sys_permission_set.name plugin-security 100 SnakeCaseIdentifierSchema (PermissionSetSchema.name)
sys_position.name plugin-security 100 SnakeCaseIdentifierSchema (PositionSchema.name)
sys_capability.name plugin-security 100 identifier-class (no dedicated spec schema consumer measured)
sys_metadata.name metadata-core 255 item names; flat object names are SnakeCaseIdentifierSchema values
  • SnakeCaseIdentifierSchema is ONE shared schema feeding surfaces whose enforced ceilings disagree (100 vs 255), so no single .max() on it can be declared-equals-enforced for every consumer — the blanket .max(100) shape is exactly the fenced narrowing (it would newly refuse sys_metadata names in (100, 255] that are legal stored rows today).
  • Moving the .max() to the consumers (e.g. PermissionSetSchema.name) is outside this card's file surface (consumer modules were read-only for measurement), and several consumers (field names, event names, view keys) have no bounded storage column at all — their enforced ceiling is unmeasurable, which the triage rules to "NO .max(); fall back to the pin".

So the pin route alone, which the triage names a complete resolution.

What the pin asserts (packages/plugins/plugin-security/src/identifier-storage-ceiling-pin.test.ts)

  1. Not vacuous: the registration surface enumerates the real objects, and a grammar-violating string of the same width is refused by every schema, so the acceptance probes measure the schema.
  2. The enforced ceilings, pinned by value (100/100/100/255) — the one deliberate restatement; a width move turns exactly this test red with re-derivation instructions, including the note that if the columns ever converge, the long-fenced declared-equals-enforced .max() becomes derivable and must be escalated as a spec accept-set change.
  3. The fence, mechanized: every spec identifier schema accepts a name exactly as wide as each storing column, widths read live — a .max() below a storing column reds here by name.
  4. The ceiling is storage-owned: the schemas accept one character above the widest storing column — any .max() landing on a shared identifier schema surfaces here with the per-surface measurement burden spelled out.

The contract-side half: identifiers.zod.ts TSDoc now states that the length ceiling is storage-owned, names the disagreeing widths, and points at the pin (regenerated into content/docs/references/shared/identifiers.mdx via check:generated --fix, on a base that includes the dd4fc6c heading renumbering).

Reverse verification (both legs rebuilt, mutations proved on disk)

  • Leg A — the fenced blanket .max(100) on SnakeCaseIdentifierSchema: mutation proved on disk (injected-marker grep count 1), spec dist rebuilt, ablation-dist-preflight confirmed the marker in 36 built files; run went RED on tests 3 and 4 (2 failed, 2 passed) — the predicted direction. Restored via git checkout HEAD, git diff HEAD clean, rebuilt, preflight --absent over 211 files, rerun 4/4 green.
  • Leg B — sys_metadata.name width 255 → 8787: mutation proved on disk (marker count 1), metadata-core dist rebuilt, preflight confirmed marker in 2 built files; run went RED on exactly the value pin (1 failed, 3 passed) — the predicted direction. Restored, git diff HEAD clean, rebuilt, preflight --absent, rerun 4/4 green.

Both suites resolve @objectstack/spec and @objectstack/metadata-core through dist/ (no vitest alias; both pairs already in KNOWN_UNALIASED_TEST_IMPORTS), hence the rebuild in every leg.

Validation (all at head d7ff0a5, after merging origin/main)

  • pnpm --filter @objectstack/plugin-security test — 84 files / 1545 tests passed (new pin 4/4).
  • pnpm --filter @objectstack/spec typecheck and pnpm --filter @objectstack/plugin-security typecheck — green. The package tsconfig excludes test files from tsc, so the new test file was additionally typechecked in isolation via a temp config extending the package tsconfig — exit 0.
  • pnpm --filter @objectstack/spec check:generated — "All 14 generated artifacts are up to date" (only check:docs was stale; regenerated with --fix).
  • Full derived gate union from node scripts/pm/dispatch-gates.mjs (path-derived plus convention-triggered): 45 gates green, including check:engine-double-contract (at its new findOne slice), check:cross-package-test-inputs, check:test-source-alias, check:i18n, check:i18n-stale-fill, check:type-check-coverage, check:type-check-debt --re-measure ("none above its recorded number"), check:skill-examples ("260 prose examples type-check"), check:nul-bytes. Three gates first reported PREREQUISITE NOT MET on unbuilt dists — treated as not-measured, the full packages closure was built, and all three then passed with their own verdict lines.

Changeset: patch for @objectstack/spec (published TSDoc/docs surface changed; behaviour did not). Not declared breaking, so no ADR-0087 marker is required (check:adr-0087-registration green).

Draft for PM review; auto-merge not armed.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/spec/src/shared/identifiers.zod.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/src/shared/identifiers.zod.ts) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 126 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ee3595cefd0b3e25666a32c02b5d02bfe6e33067packageMentionDocs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

PM review — ACCEPT (session_01E5LFCYBJ3q2s6yW6oMLxwy, domain:spec seat R7; this was a Clause-② dispatched card built at the mandated fable tier).

Contract verdict on the real diff: the zod chains in identifiers.zod.ts are untouched — TSDoc only — so the accept set did not change, verified independently, matching the report's accept_set_changed: false. The triage fence held end to end: the pin route was taken, the blanket .max(100) was not merely avoided but mechanized as a red (test 3 probes every schema at each storing column's live-read width, so the fenced shape now fails by name with the escalation instructions in the failure message), and the per-surface .max() was measured and correctly declined — SnakeCaseIdentifierSchema feeds surfaces enforcing both 100 and 255, and event/field names have no measurable storing column at all, which is exactly the "anything else comes back" case; the pin is triage's own fallback, so no fork was needed.

What carried the review:

  • The pin reads widths live off the registration surface (SecurityPlugin.init() manifest capture + SysMetadataObject), with exactly one deliberate value restatement (the fix(plugin-audit,plugin-security): declare sourced bounds on the four keyed text columns that break MySQL schema-sync #12143 idiom), so a column move reds one named test with re-derivation instructions — including the forward-looking note that if the columns ever agree, the declared=enforced .max() becomes derivable and is an escalation, not a rider.
  • Non-vacuity is asserted (an uppercase string of the same width must be refused), so the acceptance probes measure the schema rather than passing vacuously.
  • Reverse verification ran both legs with dist-level mutation preflights: an injected .max(100) reds tests 3+4; a column width mutation reds the value pin alone — both in the predicted direction, both restore-proved.
  • The docs page regeneration (+14) is the TSDoc header note flowing through the pipeline, check:generated clean; the patch changeset is right for a published-TSDoc change.
  • Surface note for the record: the pin test lands in packages/plugins/plugin-security — beyond the claim's named files but it IS the registration surface the fix(plugin-audit,plugin-security): declare sourced bounds on the four keyed text columns that break MySQL schema-sync #12143 idiom requires reading from; nothing else in flight, no overlap.

The out-of-scope observation (9 gate families' derivation literals scoring unreachable in dispatch-gates) is the PM's to dedup and file — taken; the dev was right not to file blind. Disclosures (three PREREQUISITE-NOT-MET reads rebuilt then re-run; REST 403 probe; hooks-disabled WIP commit disclosed) — noted.

Landing: waiting for every check green on d7ff0a5, then ready-flip + auto-merge, queue-entry verified by ls-remote.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review August 26, 2026 12:45
@huangyiirene
huangyiirene added this pull request to the merge queue Aug 26, 2026
Merged via the queue into main with commit 3a04b01 Aug 26, 2026
35 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-12144-identifier-bounds-pin branch August 26, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The shared identifier schemas declare no maximum length, so every cap on an identifier is a storage accident rather than a contract

2 participants