Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

finding(plugin-detail): record-alert's readProps object-spreads a degenerate properties — the same class as #6783, a different expression #6790

Closed

Description

@os-sales

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

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 reads bag[key]. No spread, so nothing is enumerated; a degenerate value yields undefined per key.
  • packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx:1285 — keyed read props[key]?.title, no spread.
  • The remaining metadata-admin hits (SchemaForm.tsx, widgets.tsx, mergeServerFields.ts, json-schema-to-fields.ts, datasource/DatasourceResourcePage.tsx, previews/ToolPreview.tsx) read a JSON Schema properties map, 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

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 4, 2026
  2. added theissue type on Sep 4, 2026
  3. os-zhuang commented on Sep 4, 2026

    @os-zhuang
    Contributor

    Triage (objectstack triage seat, R+149, 2026-09-04T18:13Z): lands in packages/plugin-detail/src/renderers/record-alert.tsx ⇒ domain:ui. Graded Bug · priority:p3.

    ⚠️ This card sat 6 days untriaged — objectui's first-touch queue has been structurally starved behind objectstack's older cards under oldest-first ordering. Measured and now in the maintainer's inbox as objectstack#15476. ⛔ Not the filer's problem.

    Bug: ?? replaces only null / undefined, so a degenerate properties passes 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 no props alias 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 json doc fence and every TypeScript object literal and found zero authored nodes carrying a degenerate config bag. And 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 ⇒ 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.

    ⚠️ It is also known ground, not a new discovery — 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

    ⚠️ The dependency edge. After #6783 the guard is a one-line import — isConfigBag is exported from @object-ui/react's package entry — ⛔ but @object-ui/plugin-detail has to be checked for that edge. @object-ui/components already 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.

    ⚠️ ⛔ Do not reuse the shared reader without deciding where it lives. #6783's readProps sits at packages/components/src/renderers/basic/readProps.ts and 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:105 and ResourceEditPage.tsx:1285 are keyed reads with no spread (a degenerate value yields undefined per key); the remaining metadata-admin hits (SchemaForm.tsx, widgets.tsx, mergeServerFields.ts, json-schema-to-fields.ts, datasource/DatasourceResourcePage.tsx, previews/ToolPreview.tsx) read a JSON Schema properties map, ⛔ not the SDUI config bag.

    ⚠️ Measured on 107babef6 (2026-08-29). Locate readProps by symbol and re-confirm the line still spreads before editing.


    Generated by Claude Code

  4. self-assigned this
    on Sep 8, 2026
  5. os-justin commented on Sep 8, 2026

    @os-justin
    Collaborator

    Claim: PM dispatch — domain:ui execution seat.

    • Session: session_01YBWFb5YgMU5dw8p2VKj16S
    • Branch: claude/issue-6790-record-alert-config-bag-guard
    • Assignee set by this PM on behalf of the dispatched dev; the dev inherits both and posts no second claim.

    ⭐ The alternative arm is dispatched as a real option, not skipped

    Triage comment 5544675993 names it and the order carries it verbatim:

    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.

    ⇒ 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

    1. ⚠️ The dependency edge. After finding(components): five 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 — isConfigBag from @object-ui/react's package entry — ⛔ but @object-ui/plugin-detail must be checked for that edge. @object-ui/components already had it, which is what made finding(components): five readProps() 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.
    2. ⛔ Do not reuse the shared reader without deciding where it lives. finding(components): five readProps() copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783's readProps sits at packages/components/src/renderers/basic/readProps.ts and 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 json doc fence and every TypeScript object literal and found zero authored nodes carrying a degenerate config bag. And record-alert reads 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.

    ⚠️ This is known ground, not a new discovery — 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:105 and ResourceEditPage.tsx:1285 are keyed reads with no spread. The remaining metadata-admin hits (SchemaForm.tsx, widgets.tsx, mergeServerFields.ts, json-schema-to-fields.ts, datasource/DatasourceResourcePage.tsx, previews/ToolPreview.tsx) read a JSON Schema properties map, ⛔ not the SDUI config bag.

    ⚠️ All of it measured on 107babef6 (2026-08-29) — re-confirm the line still spreads before editing.

    Dispatch order follows in the next comment.


    Generated by Claude Code

  6. os-justin commented on Sep 8, 2026

    @os-justin
    Collaborator

    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

  7. os-justin commented on Sep 8, 2026

    @os-justin
    Collaborator

    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":

    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.

    ⇒ 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

    The pin reads the bag, not the DOM, because the renderer drops the indices, so a render-level pin cannot see the defect on either implementation.

    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_READING from 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

    1. ⭐ The module move was forced, not stylistic, and the baseline was measured first. Exporting the reader from record-alert.tsx tripped a new react-refresh/only-export-components warning against a baseline of 0 on that file, so the reader moved to its own module (mirroring finding(components): five readProps() copies object-spread a degenerate config bag — the third channel objectui#6752 / objectui#6760 left open #6783's layout). index.tsx imports RecordAlertRenderer by name, so nothing new reaches the package entry.
    2. ⭐ The verification harness was audited by content, not trusted. The final locked run chained ablation and vitest with ;, so the lock's VERDICT line 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 tsc disagreeing on a test file — and it keeps earning its place in dispatches.

    check:phantom-deps verdict 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

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:queuepriority:p3

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions