Skip to content

feat(metadata-core,metadata): warn when a pre-current-era artifact carries fault-open form-view predicates - #12989

Merged
hotlong merged 4 commits into
mainfrom
claude/issue-12915-bare-root-predicate-notice
Aug 28, 2026
Merged

feat(metadata-core,metadata): warn when a pre-current-era artifact carries fault-open form-view predicates#12989
hotlong merged 4 commits into
mainfrom
claude/issue-12915-bare-root-predicate-notice

Conversation

@hotlong

Copy link
Copy Markdown
Contributor

Part of #12915 — scope C only. The card stays open: scope A (the expression-rewriting ADR-0087 conversion) is deferred by the maintainer ruling recorded on the card and is not implemented here.

What this does

A form-view predicate binds record (+ previous, parent) in runtime record forms, or data in metadata-editing forms. The contract states the failure mode beside the vocabulary (packages/spec/src/ui/view.zod.ts, FormFieldSchema.visibleWhen / FormSectionSchema.visibleWhen): a bare identifier is unbound, the predicate faults, and visibleWhen's fault fallback is true — so a field the predicate was authored to hide renders for everyone.

That is quiet alone and lethal in combination with the authoring pattern it serves. On the reported deployment an artifact built by released @objectstack/cli 17.1.0 authors { field: 'disqualification_reason', required: true, visibleWhen: 'status == "unqualified"' } — the era's working spelling. On a 17.2 runtime the predicate faults open, the conditionally hidden field renders, and its unconditional required: true blocks every record creation through the console, while the same payload POSTs 201 through REST. Nothing refused and nothing logged, so the operator — the only person who can rebuild the artifact — had no signal at all.

The framework artifact door now emits one deduped warn line per artifact naming the authored engines.protocol floor and the runtime spec version, how many predicates on which views (first path as anchor), the fault-open consequence, and the remedy (os build).

  • packages/metadata-core/src/form-predicate-root-policy.ts — the detection policy, exported (detectUnboundFormViewPredicateRoots, BOUND_FORM_VIEW_PREDICATE_ROOTS) so a composed door can reuse one policy rather than fork it.
  • packages/metadata/src/plugin.ts — wiring in _convertArtifactForward, reusing the existing artifactConversionWarned dedupe set so the HMR replay does not re-announce.

No behaviour change: no refusal, no rewrite, no packages/spec or packages/metadata-protocol edit. A pin asserts the registered view still carries the authored predicate sources verbatim, bare root and all.

Scoped to legacy artifacts by construction. The notice fires only inside the versioned window the forward conversion already opens, and it reads that pass's own verdict rather than recomputing the comparison, so the two can never disagree about which artifacts are old. Verified against the window code rather than assumed: converted-forward (declared floor below the running spec) and converted-undeclared (no declared range, grandfathered as data at rest) open it; authored-current and runtime-version-unknown do not. An artifact declaring the current or a newer floor gets zero notices even when it carries bare roots.

Precision — the notice must never cry wolf

A notice that fires on a healthy current artifact is worse than no notice, so every judgement call resolves toward silence: string literals are stripped before the scan; only root position counts (record.status names one root); a name followed by ( is a call, not a scope root; a comprehension macro (.exists(, .all(, .map(, .filter(, .exists_one() binds its own iteration variable and is therefore skipped whole; AST-only envelopes pass.

A tokenizer rather than a CEL parse is the established shape for this exact question here — the spec's own enforced checkFormViewPredicateFeaturesRoot scans the source string the same way. A real CEL parse lives in packages/formula, which metadata-core cannot reach without breaking its spec-only dependency edge; spec itself exposes no parse. Measured, not assumed.

One premise correction

The dispatch brief expected the form-view predicate key set to be visibleWhen / requiredWhen / readonlyWhen / visibleOn. Read off the schema instead: a form-view field entry and a section each carry exactly visibleWhen plus the deprecated visibleOn alias. requiredWhen / readonlyWhen are declared on the OBJECT field schema (packages/spec/src/data/field.zod.ts), a different surface with its own binding scope, and are not scanned here. Both keys are read because this scan runs before the parse that folds the alias — same reason the legacy groups section bucket is read alongside sections.

Per-option visibleWhen is deliberately out of scope: options are evaluated by a different evaluator that also binds current_user, so this vocabulary would be the wrong yardstick there.

Verification

Union re-run on the final commit 8ee7e0793.

  • pnpm --filter @objectstack/metadata-core --filter @objectstack/metadata test — 15/15 files, 255/255 tests; 37/37 files, 672/672 tests.
  • pnpm --filter @objectstack/metadata-core typecheck — exit 0. --listFiles confirms tsconfig.test.json really reaches both new files (1 hit each), so the green is not a green over source nothing read.
  • packages/metadata declares no typecheck script — it is a DEBT ledger entry whose dts build is the type gate. pnpm --filter @objectstack/metadata build exit 0, and check-type-check-coverage --re-measure holds its entry at exactly its recorded 89 (surplus: none — every entry sits exactly at its measurement), which is what covers the new test file there.
  • node scripts/check-type-check-coverage.mjs --re-measure under NODE_OPTIONS=--max-old-space-size=4096OK — 31 ledger entr(ies) re-measured, 1570 raw tsc error(s) total, none above its recorded number.
  • Gate families derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (not from a hand-built path list): all 21 path-matched families plus the 6 convention-triggered ones (two new test files) exit 0, including check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:stack-collection-maps, check:durability-log-level, check:test-source-alias, check:nul-bytes. check:dispatcher-error-vocabulary also exit 0 (no new SCREAMING_SNAKE code literal is introduced).
  • pnpm lint — full repo, eslint . --no-inline-config, exit 0. No narrowing claimed on this axis.
  • One family was not measured: a bare node scripts/pm/check-half-states.mjs exited 3 with PREREQUISITE NOT MET — the anonymous API rate limit is exhausted for this egress IP. That is a transport refusal, not a red gate; the form CI runs (check:pm-half-states = --self-test) exits 0.

Ablation.packages/metadata/vitest.config.ts carries no alias for @objectstack/metadata-core, so the door suite resolves it through dist/ — both legs therefore rebuilt and proved what reached dist/. Predicted direction stated before running: the two notice-asserting pins go red, the five silence pins stay green. Observed exactly that — 2 failed | 5 passed, both failures expected [] to have a length of 1. The mutation was confirmed on disk by counting the injected marker (1) and the anchor (still 1) before the rebuild, then by scripts/ablation-dist-preflight.mjs reporting it present in 2 built files. Restore leg: git checkout HEAD -- PATH, git diff HEAD empty, restored blob byte-identical to the HEAD blob (0d828bb8f3fe8d36ca2e3bb6b4885e5ab522e991), rebuild, preflight --absent clean, suite green again at 7/7.

A first attempt at this ablation was a void run and is reported rather than quietly retried: the marker was written as a comment, esbuild stripped it, and it reached only the sourcemap — the preflight refused to count it and the run was discarded before any conclusion was drawn from it.

Declared narrowing — verification ran UNLOCKED.scripts/pm/os-verify-lock.sh could not take the shared verify lock on this host: no usable flock. The shared verify lock is declared Linux-only (flock is util-linux, and a stock macOS does not ship it), so the commands were run directly, without the lock — a declared narrowing, not a silent one. No serialization guarantee held for these runs, nor for any sibling agent in this container while they ran.

Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/metadata-core, @objectstack/metadata, touching 23 documentable anchor(s). ⚠️1 changed file(s) yielded no anchor (packages/metadata-core/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ai/skills-reference.mdx(via visibleWhen (literal))
  • content/docs/api/client-sdk.mdx(via MetadataPlugin (symbol))
  • content/docs/api/index.mdx(via MetadataPlugin (symbol))
  • content/docs/automation/flows.mdx(via MetadataPlugin (symbol), visibleWhen (literal))
  • content/docs/data-modeling/field-types.mdx(via visibleWhen (literal))
  • content/docs/data-modeling/fields.mdx(via visibleWhen (literal))
  • content/docs/data-modeling/formulas.mdx(via visibleWhen (literal))
  • content/docs/data-modeling/validation-rules.mdx(via visibleWhen (literal))
  • content/docs/deployment/environment-variables.mdx(via visibleWhen (literal))
  • content/docs/kernel/services-checklist.mdx(via MetadataPlugin (symbol))
  • content/docs/protocol/kernel/metadata-service.mdx(via MetadataPlugin (symbol))
  • content/docs/protocol/objectui/layout-dsl.mdx(via visibleOn (literal), visibleWhen (literal))
  • content/docs/ui/pages.mdx(via visibleWhen (literal))
  • content/docs/ui/views.mdx(via visibleOn (literal), visibleWhen (literal))

5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx(via MetadataPlugin (symbol), visibleWhen (literal))
  • content/docs/releases/v12.mdx(via visibleWhen (literal))
  • content/docs/releases/v15.mdx(via visibleOn (literal), visibleWhen (literal))
  • content/docs/releases/v16.mdx(via visibleWhen (literal))
  • content/docs/releases/v17.mdx(via visibleWhen (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/metadata-core/src/index.ts) — pages documenting those are invisible to this run
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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 — 11 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 2b4178aa53ca62089f43e2cfae0b7838cf340dd1packageMentionDocs.

Which tree this was computed on

This run read content/docs from 890d15a88e8c8558801b0af3aae733007c1a4bf8 — the merge of head 8ee7e0793a67e9c4a57210cfe84d698bd5bc5964 into base 2b4178aa53ca62089f43e2cfae0b7838cf340dd1, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 890d15a88e8c8558801b0af3aae733007c1a4bf8 && git checkout 890d15a88e8c8558801b0af3aae733007c1a4bf8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2b4178aa53ca62089f43e2cfae0b7838cf340dd1 8ee7e0793a67e9c4a57210cfe84d698bd5bc5964 && git checkout -B drift-repro 2b4178aa53ca62089f43e2cfae0b7838cf340dd1 && git merge --no-ff 8ee7e0793a67e9c4a57210cfe84d698bd5bc5964
node scripts/docs-audit/affected-docs.mjs --json 2b4178aa53ca62089f43e2cfae0b7838cf340dd1

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2b4178aa53ca62089f43e2cfae0b7838cf340dd1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@hotlong