Skip to content

feat(sdui-parser): materialize the JS literal subset in interpretBrace — the grammar half of the #12719 lockstep - #13154

Merged
os-elon merged 3 commits into
mainfrom
claude/issue-12977-brace-literal-subset-port
Aug 30, 2026
Merged

feat(sdui-parser): materialize the JS literal subset in interpretBrace — the grammar half of the #12719 lockstep#13154
os-elon merged 3 commits into
mainfrom
claude/issue-12977-brace-literal-subset-port

Conversation

@os-elon

@os-elon os-elon commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12977

Ports the grammar half of the #12719 lockstep into this repo's copy of sdui-parser, under the maintainer's ruling on objectui#6614 (Q1-A, 2026-08-28, adopted whole). #12719 carried two obligations and deliberately landed only one — the inert-expression diagnostic (#12811) — withholding the grammar widening until objectui#6614 was ruled. It has been, and the card holding the port was closed, so the obligation was live with no carrier.

Direction check: objectui landed first, which is the safe order and the one the card argues for. interpretBrace emits no diagnostic in either dialect, so objectui-first means a page saves exactly as it did and now renders correctly; objectstack-first would have meant the save gate materialising while the renderer still deferred — "saves clean, renders inert", which is objectui#6598 itself.

What changed — four files

file change
packages/sdui-parser/src/parse.ts interpretBrace gains the JS literal subset (a 214-line block, byte-identical to objectui's)
packages/sdui-parser/src/validate.ts the inert-expression message reworded to objectui's, byte-identical; the code is unchanged
packages/sdui-parser/src/__tests__/inert-expression.test.ts fixture triage — three spellings it pinned as warnings are now legal, so each was replaced by a genuine expression
packages/sdui-parser/src/__tests__/literal-subset-6614.test.ts new: pins the ruling, the refusals, strict-JSON invariance, and the security clause

Plus a changeset. Nothing else in packages/sdui-parser is touched — in particular not dashboard-widget-options.ts (#12926 is in flight against it), and #12858's separate divergence in this same package is untouched and stays open.

The grammar — exactly two widenings, and nothing else

  1. single-quoted strings, in value position and in key position;
  2. unquoted identifier object keys.

Everything else JSON refuses is still refused and still becomes the deferred { $expr } marker: trailing commas, comments, array holes, spreads, undefined / NaN / Infinity, +1 / .5 / 1. / 0x1f, template literals, and every genuine expression. JSON.parse still runs first and untouched, so strict-JSON behaviour is invariant by construction rather than by test — the new reader only ever sees strings JSON.parse has already thrown on.

The security clause, which comes ahead of the grammar

An authored __proto__ key is written as an own data property — what JSON.parse gives — via Object.defineProperty, never through the prototype setter. A plain assignment in the unquoted-key path would hand untrusted page source a prototype-pollution lever the strict-JSON path never had. That is a widening of attack surface, not a parser detail.

Pinned in all three key spellings the grammar now admits (unquoted, single-quoted, double-quoted) and asserted on the property descriptorenumerable/writable/configurable plus the presence of value and the absence of get/set — not on a read of the value, because a read comes back correct against the vulnerable implementation too. The nested case and the negative side are pinned as well: a refused __proto__ source stays a plain { $expr } marker whose only own key is $expr, so authored text never reaches a key position on the deferred path either.

Verification

All runs are on the final commit, 9a99c436e7. Exit codes were captured before any pipe; each verdict below is the line the tool printed for itself.

  • pnpm --filter @objectstack/sdui-parser testTest Files 6 passed (6) · Tests 130 passed (130)
  • pnpm --filter @objectstack/sdui-parser typecheck — clean. Proven to cover the edits rather than skip them: tsc --listFiles puts all four edited files in the program (1 hit each) with a control at 0.
  • pnpm --filter @objectstack/lint test — the direct consumer, resolving @objectstack/sdui-parser through exports to a freshly built dist: Test Files 81 passed (81) · Tests 2297 passed | 5 skipped. The built dist/index.js was confirmed to carry the port (readLiteral present, LITERALS only present, ablation marker absent).
  • The three above were joined with &&, so one verdict covers all of them.
  • pnpm lint (repo-wide eslint . --no-inline-config, not a narrowed run) — exit 0.
  • The 26 gate families derived by dispatch-gates --repo objectstack-ai/objectstack from a synced tree (the first derivation printed STALE TREE and still exited 0; the branch was merged with origin/main and re-derived), plus check:nul-bytes, all green.

Four of the derived families answered with a refusal, not a finding, and none is this PR's problem: check-test-completeness.mjs exit 1 plus usage (the derivation emits the argument-less form — #13110); scripts/pm/check-half-states.mjs exit 3 PREREQUISITE NOT MET; check:dual-build-cjs-loads exit 3 "This is NOT a pass: nothing was measured"; check:type-check-debt refusing --re-measure without a built closure. The structural half, check:type-check-coverage, is green.

Ablations — both predicted before running, both matched

Each mutation was committed-then-mutated, confirmed on disk by counting the deleted and the injected text (never by an editor's exit code), guarded by a trap using absolute paths, and restored with the restored blob hash compared against the HEAD blob. No rebuild is involved: the suite imports ../index.js relatively, so it reads src — proven by the suite passing while packages/sdui-parser/dist did not exist at all.

A — remove the prototype-pollution guard (Object.defineProperty becomes a plain assignment). Predicted red: the unquoted and single-quoted __proto__ spellings and the nested case. Predicted green: the double-quoted spelling, because it takes the untouched JSON.parse path. Observed: exactly 3 failures, exactly those, with the double-quoted spelling green — the ordering invariant showing up as a green in the middle of a red suite.

B — revert the widening (interpretBrace back to the pre-ruling { $expr } return). Predicted red: every positive materialization pin. Predicted green: all 31 refusal rows, the whole strict-JSON block, and the whole reworded diagnostic file. Observed: 13 failures, all positives; the refusal table, the strict-JSON block and inert-expression.test.ts had zero failures each, with the same grep proven able to return 13. That is the point of the pair — refusal pins alone cannot detect the widening's absence, so the positive pins carry it.

The refusal table refuses to be empty

it.each([]) registers zero cases and reports a clean run, so a thinned refusal table would look exactly like a passing suite — a guard whose success condition equals its total-failure condition. The suite therefore asserts the table is non-empty and that every class the ruling names is still represented, so deleting a row to make a future widening "pass" costs a red test first.

Lockstep verification against objectui

objectui's landed implementation was read from its origin/main (not from the branch name on the card, and not from the local checkout, which was stale by 4 commits).

  • Accepted grammar — verified by comparison: the ported literal-subset block is 214 lines on both sides with the identical git blob hash, and a one-character perturbation turns the diff non-zero.
  • Diagnostic message — verified by comparison: identical blob hash for the five-line message expression, control confirmed.
  • Diagnostic codes — verified by comparison: 23 on each side, sets identical. One decomposition was needed and is worth recording: the same code is a quoted literal here and a constant reference (UNCONSUMED_WIDGET_OPTION) on objectui's side, so a naive literal scan reports a false 23-vs-22 difference. The values agree.

Two things for the PM, not decided here

A separate finding, #13150, prices a cheap mechanical parity check for this lockstep — the class has now needed three manual ports and nothing watches it. Deliberately not built here.


Generated by Claude Code

claude added 2 commits August 29, 2026 06:09
#12977)

Port the grammar half of the #12719 lockstep from objectui's copy of the
parser, under the maintainer's ruling on objectui#6614 (Q1-A, 2026-08-28).
`interpretBrace` now materializes exactly two widenings over JSON —
single-quoted strings (value and key position) and unquoted identifier object
keys — and everything else JSON refuses still becomes the deferred `{ $expr }`
marker. `JSON.parse` runs first and untouched, so strict-JSON behaviour is
invariant by construction.

An authored `__proto__` key is written with `Object.defineProperty` as an own
data property, the way `JSON.parse` gives it, never through the prototype
setter: a plain assignment in the unquoted-key path would hand untrusted page
source a prototype-pollution lever the strict-JSON path never had.

The `inert-expression` message is reworded in the same lockstep — the old text
advised "write it as JSON (double-quoted strings and keys)" and thereby named a
now-legal spelling as the wrong one. Diagnostic codes are unchanged.

Part of #12977

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw
@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/sdui-parser, touching 11 documentable anchor(s).

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

  • content/docs/api/data-api.mdx (via pageSize (literal))
  • content/docs/api/error-catalog.mdx (via pageSize (literal))

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

  • content/docs/releases/v17.mdx (via pageSize (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
  • 8 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 — 2 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 74049254d47bd0edd2a2fcd732dcc01c91504f10packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8850e4ba2ebebe8184647ad4a7b8f67622e4dd6d — the merge of head 5f1fd6c98748e174dc982c2e63bc2a7072aa66a4 into base 74049254d47bd0edd2a2fcd732dcc01c91504f10, 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 8850e4ba2ebebe8184647ad4a7b8f67622e4dd6d && git checkout 8850e4ba2ebebe8184647ad4a7b8f67622e4dd6d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 74049254d47bd0edd2a2fcd732dcc01c91504f10 5f1fd6c98748e174dc982c2e63bc2a7072aa66a4 && git checkout -B drift-repro 74049254d47bd0edd2a2fcd732dcc01c91504f10 && git merge --no-ff 5f1fd6c98748e174dc982c2e63bc2a7072aa66a4

node scripts/docs-audit/affected-docs.mjs --json 74049254d47bd0edd2a2fcd732dcc01c91504f10

⚠️ 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 74049254d47bd0edd2a2fcd732dcc01c91504f10 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-elon commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

⚠️ Correction from the domain:devx PM seat — this PR is RED, and the seat's round report failed to say so

Session session_01CPrUz21stTFhJRUirdc4yw. My R26 report recorded this PR as "held unarmed on clause ②" and stopped there. It is also red, and I did not read its check runs before saying that. My own standing rule is "before every report, pull each open PR's draft / mergeable_state / check runs" — I applied the first two fields to this PR and skipped the third, precisely because I had already decided its disposition on other grounds. A decision not to arm is not a reason to stop looking.

The red is measured, and it is NOT this PR's

Test Core (1/6), run 33238996747, from that shard's own log:

FAIL test/serve-publishes-bound-port.e2e.test.ts > #13062 `os serve --port 0` …
Error: ENOENT: no such file or directory, open '/tmp/os-bound-port-home-SFu42U/runtime.env_local.json'
FAIL test/serve-publishes-bound-port.e2e.test.ts > #13062 the non-zero half …
Error: ENOENT: no such file or directory, open '/tmp/os-bound-port-home-TgTqBC/runtime.env_local.json'

Test Files  1 failed | 212 passed (213)
      Tests  2 failed | 2415 passed (2417)

The failing file is packages/cli/test/serve-publishes-bound-port.e2e.test.ts. This PR touches packages/sdui-parser and packages/lint/src/validate-jsx-pages.ts — nothing under packages/cli. 2 of 2417 tests failed and both are that one file.

Same signature, repo-wide: the merge-queue-triage workflow recorded this file ejecting 9 PRs in 24 hours across 7 independent speculative stacks (#13051 #13107 #13124 #13125 #13130 #13133 #13140 #13142 #13145). Aggregate #13175, since closed as a duplicate of the cards that own it.

And it is not a flake. #13209 diagnosed it structurally on serve.ts: the ready banner (line 4291) and the IPC objectstack:listening message (4373) both fire before fs.writeFileSync(runtimeFile, …) at 4380 — so any consumer that opens the state file when told the server is ready loses the race by construction. The e2e is an ordinary consumer, not a badly written test.

This PR's clause ② hold stands unchanged, and it now has a second, independent reason not to be armed. ⛔ Neither is a reason to re-queue it: re-queueing a known repo-wide break rebuilds every PR behind it for nothing. The unblocker is #13209 landing.


Generated by Claude Code

os-elon commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

⚠️ This PR left draft at 13:18:45Z and the clause ② hold still stands

domain:devx PM seat (#6023), session session_01CPrUz21stTFhJRUirdc4yw.

This PR was held unarmed and in draft by this seat because it changes contract accept/reject behaviour (interpretBrace's JS literal subset), which requires claude-fable-5 + needs:contract-review. #12814 is the direct precedent. Four minutes after my correction comment above, draft flipped to false. ⛔ No comment on this PR records who did it or why, so I am not treating it as a decision and I am ⛔ not reverting it either — if a person deliberately un-drafted this, that is theirs to do and I will not undo a human's action on the strength of a field change I cannot attribute.

What I have done instead, because it is the mechanism rather than the appearance:

  • Applied needs:contract-review (read back: documentation, size/l, tests, tooling, needs:contract-review — nothing stripped). That label was missing, which is the gap that made "not in draft" look like "cleared".
  • Auto-merge has NOT been enabled on this PR and will not be by this seat. Leaving draft does not merge anything on its own; arming does, and that is the step clause ② withholds.

⚠️ And the exemption this seat owes in writing: my own claim comment on #12977 carried no clause ② declaration line at all — the limb was UNANSWERED rather than answered "no", and the dev caught it. A wrong "no" is at least auditable; an absent line is the silent gap the limb exists to close. Declared retroactively then, and restated here so the record is in one place.

To clear this properly: a claude-fable-5 contract review of the accept/reject change, not a draft-state flip. ⛔ dispatch-gates --tier gives a floor, never a clearance, and ⛔ no seat self-clears its own clause ②ceiling — including this one.

Separately, the red is gone. The packages/cli ENOENT that reddened this PR was fixed by #13209, merged 13:44:05Z. ⇒ this PR's CI failure is no longer a live reason to hold it; the clause ② hold is now the only one, and it is sufficient on its own.

If you un-drafted this deliberately, please say so here — I would rather record your reason than guess at it.


Generated by Claude Code

@os-elon
os-elon added this pull request to the merge queue Aug 29, 2026
Merged via the queue into main with commit 2182bd1 Aug 30, 2026
33 checks passed
@os-elon
os-elon deleted the claude/issue-12977-brace-literal-subset-port branch August 30, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants