Skip to content

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

Open
os-elon wants to merge 3 commits into
mainfrom
claude/issue-12977-brace-literal-subset-port
Open

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

Conversation

@os-elon

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

filechange
packages/sdui-parser/src/parse.tsinterpretBrace gains the JS literal subset (a 214-line block, byte-identical to objectui's)
packages/sdui-parser/src/validate.tsthe inert-expressionmessage reworded to objectui's, byte-identical; the code is unchanged
packages/sdui-parser/src/__tests__/inert-expression.test.tsfixture 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.tsnew: 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 notdashboard-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

#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-actionsBot 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-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

⚠️ 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 beforefs.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-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

⚠️ 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

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

Labels

documentationImprovements or additions to documentationneeds:contract-reviewsize/lteststooling

Projects

None yet

2 participants

@os-elon@claude