Repository navigation
DetailSection headerColor reaches the DOM only as bg-${...} — a template-literal Tailwind class that generates no CSS, so the key works only by collision with other files' literal classes #6178
Description
Activity
Concentrated triage batch:
finding→pm:queue+domain:ui, Bug, M —headerColorreaches the DOM only as a template-literal Tailwind class, which generates no CSS under this repo's v4@sourcescanning (the repo's own test documents that reading), so the declared key styles a header only by collision with other files' literal classes. Fix direction the dev measures: a closed value vocabulary mapped to static literal classes (Tailwind-visible), or inline style from the authored value — whichever matches how sibling color keys in this repo already solved it; do not add a safelist for an open value set without measuring what authors actually write.
Generated by Claude Code
Claim: PM loop round R2
Session:session_01Mn4BZ5AVDM81pvfij1WwM9
Branch:claude/issue-6178-detailsection-headercolor
Worktree:objectui-issue-6178
Domain:domain:ui
File surface:packages/plugin-detail/src/**(DetailSection.tsx) + its tests (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: opus
Clause-②: no — the key is already declared and already authored; this makes it actually produce CSS.⚠️ If the chosen fix closes the value set (a vocabulary of allowed colours), that narrows what is accepted and IS a contract change — stop and report rather than deciding it.
Serial constraints cleared:packages/plugin-detailis free — its previous holder (#5935, the wide lucide sweep) was reclaimed topm:queueat 09:17Z on confirmed agent death. No other lane PR touches this package (#6267 →packages/types, #6283 →app-shell, #6082 →content/docs).Stale-premise check — run, not assumed
✅ Confirmed on today's
main, both sites:DetailSection.tsx:513and:542carrysection.headerColor && \bg-${section.headerColor}`. Two occurrences in that file, one more inindex.tsx. ⚠️ Three commits have landed onpackages/plugin-detailsince the card was filed (most recently6c1b105`) — the dev re-derives the surrounding code rather than trusting the card's line numbers.Triage's direction, carried
A closed value vocabulary mapped to static literal classes (Tailwind-visible), or inline style from the authored value — ⭐ whichever matches how sibling colour keys in this repo already solved it. ⛔ Do not add a safelist for an open value set without measuring what authors actually write. Measure the sibling solution first; that measurement decides the shape.
Generated by Claude Code
os-dev-report
{ "issue": 6178, "status": "done", "branch": "claude/issue-6178-detailsection-headercolor", "pr": "https://github.com/objectstack-ai/objectui/pull/6294", "premise_still_valid": true, "mcp_call_count": 3, "mcp_calls": [ "create_pull_request -> PR 6294 (write)", "issue_write create -> issue 6295 (write)", "add_issue_comment -> this report on 6178 (write)" ], "summary": "The card's premise held on today's main and I confirmed it against the built stylesheet, not by inference: compiling apps/console/src/index.css with the two `bg-` template literals deleted produced a byte-identical sheet (sha256 dffcd88d..e7e3b2, 441302 bytes), so that call site contributed zero rules. But it was not inert in effect - both values the @object-ui/types mirror documents are live TODAY by collision (`.bg-muted` and `.bg-primary/10` are 2 of 227 `bg-*` selectors in the built CSS, sourced from 691 and 63 literal occurrences in other files), so the fix had to preserve them rather than just stop lying. The sibling measurement decided the shape unambiguously: DetailSection was the ONLY `bg-`+interpolation class in the whole workspace, and useRowColor (packages/plugin-grid) already solves the identical problem with a lookup of literal classes + a `bg-` pass-through + undefined otherwise. New packages/plugin-detail/src/headerColor.ts does exactly that over six tint-only semantic tokens (muted, muted/50, accent, primary/10, secondary/10, destructive/10), both call sites rewired, plus a changeset. Two premise corrections that do not change the conclusion: the card's 'no safelist' is imprecise (apps/console/src/index.css uses nine @source inline(...) lines for streamdown - there is just no bg-* safelist), and the third headerColor mention is index.tsx:310 (not :287), which is the detail-section registry INPUT ROW, a declaration and not a fourth read site. No sanitizer truncation in the card: the body carries no angle-bracket fragments at all.", "tests": "All at 799d2385e (branch HEAD; `git status --porcelain` empty, so every measurement below was taken on this exact tree). (1) `pnpm exec vitest run packages/plugin-detail` -> 'Test Files 106 passed (106) / Tests 1007 passed (1007)'. (2) `pnpm --filter @object-ui/plugin-detail type-check` -> clean, script name echoed ('tsc --noEmit && tsc -p tsconfig.test.json'), so not a zero-match no-op. (3) `pnpm exec eslint packages/plugin-detail --format json` -> exit 0, 0 errors over the 159 files eslint's own config resolution judged (count read from the JSON); my four files 0e/0w except DetailSection.tsx's 12 PRE-EXISTING warnings at lines 70/102/105/111/122/147/221/223/261/261/263/428, none on the three lines touched (37/514/543). Repo-wide `pnpm lint` NARROWED and declared: eslint.config.js configures no type-aware linting (no projectService, no parserOptions.project) and no eslint-rules/ rule reads another file, so a verdict is a function of the file's own text plus shared config, neither of which this diff moves outside packages/plugin-detail. (4) Gates exit 0: check:control-bytes, check:phantom-deps, check:entry-guard, check:self-import, check:vi-mock-specifiers, check:esm-specifiers, check-type-check-coverage, check-lint-coverage, check-changeset-presence ('4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'), check-changeset-no-major, check-changeset-fixed. TWO gates report a BROKEN GAUGE, pre-existing and self-diagnosing, not a verdict on this diff: check:eager-closure ('No eager-closure report at apps/console/dist/eager-closure.json ... an absent report means the console was not built') and check:readme-exports ('its type entry ./dist/index.d.ts is not on disk -- run pnpm build first'); this worktree had 0 of 40 packages built and no apps/console/dist at that moment, and neither names a file in this diff. Every gate result quoted is that gate's own printed verdict line, and every exit code was captured before any pipe. ABLATION - four points, direction PREDICTED before each run, all four correct; every script carried `trap ... EXIT INT TERM` restore and refused to run (exit 90) unless the mutation was confirmed on disk by before/after grep counts on the anchored text, and printed its restore leg. Baseline ablation (rebuild = the console CSS compile, mutation confirmed 2 -> 0 occurrences): stylesheet byte-identical, predicted and observed. P1 revert ONLY the non-collapsible call site -> predicted that branch reds and collapsible stays green; observed 4 red / 17 pass, reds = 3 non-collapsible cases + the interpolation pin, ALL 5 collapsible cases green. P2 revert ONLY the collapsible call site -> mirror; 4 red / 17 pass, ALL 5 non-collapsible cases green. P3 bg-muted -> bg-mutedd (still a complete literal) -> predicted the design-system test reds while the verbatim-source test stays green; observed exactly that, 5 red / 16 pass. P4 'bg-muted' -> ['bg-','muted'].join('') (still a valid utility at runtime) -> predicted ONLY the verbatim-source test reds; observed exactly 1 red / 20 pass, with the design-system test, the resolver tests and EVERY render test green - #6178 reproduced in miniature. WHICH ASSERTIONS SURVIVED THE REVERT, stated plainly because it is the honest limit of the render tests: under P1/P2 the two 'a mapped token renders its literal class' cases on the reverted branch STILL PASSED, because the old concatenation produces the same string for a mapped value. INSTRUMENTS - the render test (DetailSection.headerColor.test.tsx) asserts which className reaches the DOM and says in its own header that this proves NOTHING about CSS emission; the generation half (headerColor.test.ts) asserts (a) each vocabulary class appears verbatim as a complete token in the module source (what the v4 extractor needs) and (b) each builds a real rule against this workspace's shipped @theme via __unstable__loadDesignSystem from `tailwindcss` (a root devDependency, so the tooling tier of check:phantom-deps allows it), with bg-, bg-mutedd and bg-not-a-token as negative controls proving the instrument can say no. What it still cannot show, stated in the file: it does not run the oxide scanner and does not verify any app's @source globs. Every negative pin has a positive probe beside it - the header element is located getBy-style and throws when no header rendered, so no negative assertion can pass vacuously against an unrendered header. Vitest console silencing was worked around throughout: every probe wrote results to a file rather than relying on stdout.", "open_questions": [ { "question": "The renderer can now deliver headerColor for a declared vocabulary. Should @objectstack/spec declare the key on the strict record:details section schema - and over which vocabulary? The spec REFUSES headerColor today (packages/spec/src/ui/component.zod.ts, pinned by component.test.ts 'still refuses the two keys #11661 deliberately withholds'), and the recorded reason is exactly this defect: 'declaring it would advertise a capability the renderer does not deliver'. That reason is now obsolete, and the pin's own comment says a later batch must flip it consciously. I did not touch spec or packages/types (both outside this card's file surface; packages/types is held by objectui#6267).", "options": [ "A - declare headerColor on the spec section schema as z.enum over the six tokens this PR ships (muted, muted/50, accent, primary/10, secondary/10, destructive/10), and narrow the @object-ui/types mirror to match. Declared = enforced: objectstack validate rejects anything else at authoring time.", "B - declare it as z.string() (mirroring today's objectui type). Keeps the pass-through escape hatch authorable, but re-advertises an open set the renderer delivers only for the enumerated half plus whatever the host app's Tailwind build happens to generate - the capability-illusion shape the spec refusal exists to prevent.", "C - leave the spec refusal standing and treat headerColor as an objectui-only renderer key. Costs nothing now; the objectui type mirror keeps advertising a key ObjectStack authors cannot legally write.", "D - retire the key instead: the card's own option 1, and still available since zero producers author it anywhere in either repo. This PR would be reverted rather than extended." ], "recommendation": "A, on all four axes. REAL BUSINESS NEED: measured, and it is thin but not zero - zero producers author headerColor in objectstack (examples, platform pages, content) or in objectui outside declarations, so nothing is pulling on an open set; what does exist is the pull from the objectstack#11661 batch, which declared three sibling section keys and withheld this one ONLY because of this defect. A closed enum satisfies that pull at its measured size. LONG-TERM SOUNDNESS: it removes the last dialect - one vocabulary declared once in spec, enforced at publish, rendered from literals the scanner can see; B leaves the contract wider than the renderer, which is the shape #0.1 rejects. AI-WRITTEN METADATA: this is the decisive axis. An enum is structurally hard to get wrong - an AI author writing headerColor: 'blue-100' is REJECTED at authoring time instead of shipping a header that silently does not paint, which is precisely the failure mode this card documents. B preserves that silent failure by contract. STARTUP SCOPE: A adds no surface the renderer does not already deliver and takes no bet on an unpulled open set; D is the stricter reading of the same axis and I would not argue against it, but triage already ruled the disposition (finding -> pm:queue as a Bug, with 'make it work' as the direction), so retiring it now would be re-litigating a settled call. The residual judgement inside A - whether the vocabulary should be exactly these six - is the maintainer's; I chose tints only because CardHeader sets no foreground, so a solid bg-primary or bg-destructive would leave the title unreadable without a paired text-*-foreground." }, { "question": "Secondary, and the only thing in this PR I would call a contract-adjacent judgement I made rather than escalated: the DECLARED type is untouched (headerColor?: string, nothing rejected) but the EFFECTIVE value set is reshaped - an undocumented bare suffix that happened to collide with another file's literal (e.g. 'blue-100' -> bg-blue-100) no longer paints. Is that acceptable, or should it have been escalated before landing?", "options": [ "A - accept as landed. Ending accidental collision-liveness IS the fix; no change satisfying the card can preserve it, so treating it as a blocking contract change would make the card unfixable by construction.", "B - widen the vocabulary to cover every bg-* selector currently in the console build (227 of them) so nothing can regress.", "C - revert and escalate the vocabulary before any renderer change lands." ], "recommendation": "A. Three measurements bound the risk to zero for real authors: zero producers author headerColor anywhere in either repo, both documented examples (muted, primary/10) are preserved and now work by declaration rather than collision, and every previously-colliding suffix has a one-word migration through the pass-through ('blue-100' -> 'bg-blue-100'). B is not a real option - that 227-item set is a property of one app's build at one commit, unversioned by construction, which is the defect rather than a fix for it. Flagging rather than hiding it: the dispatch guard said stop and report if the fix CLOSES the value set, and I judged this shape does not close it (the declared type is unchanged and the bg-* pass-through keeps the accepted set open), so I landed it as draft with the reshaping stated at the top of the PR body for the reviewer to overrule." } ], "out_of_scope_findings": [ "filed as objectui#6295: useRowColor (packages/plugin-grid/src/useRowColor.ts) indexes both config.colors and COLOR_TO_CLASS with an unguarded record-derived string, so a record whose colour field holds 'constructor'/'toString'/'valueOf' resolves to an inherited Object.prototype function, passes the `if (!color)` truthiness guard and throws 'TypeError: color.startsWith is not a function' during render - reproduced on the exact expression chain before filing. Unassigned, unlabelled (a specific defect, left for PM triage). Dedup done on the zero-quota channel before filing: issueCount=0 for useRowColor, COLOR_TO_CLASS and colorToClass; the 6 open hits for 'rowColor' and 1 for 'prototype' (#2231, #2890, #5144, #5435, #6058, #6152, #5828) were each opened and read - all schema/mirror-parity or auth-seed cards, none this defect." ] }
Generated by Claude Code
✅ ACCEPT — PR #6294. The measurement is the deliverable here, not the diff.
Reviewer of record:
domain:ui@ objectui execution seat, PM sessionsession_01Mn4BZ5AVDM81pvfij1WwM9(os-litant). Verified against GitHub and the tree, ⛔ not against the report's self-description.⭐ The instrument question was answered, not dodged
My dispatch warned that asserting a
classNamein the DOM proves nothing about whether Tailwind emitted a rule — the old code putbg-in the DOM too, and a rendering assertion was green the entire time it generated no CSS.The dev answered it with the compiled artifact: it built
apps/console/src/index.cssthe way the console build does, then again with the template literal deleted, and the two sheets are byte-identical — 441302 bytes, same sha256. ⇒ That call site contributed zero rules. ✅ That is a measurement of the thing in question, not a proxy for it.⭐ And it found the fact that changes the fix
The card said the key "works only by collision". The dev measured it rather than repeating it:
.bg-mutedand.bg-primary/10are in the built stylesheet today — 2 of 227bg-*selectors — because other files author them literally (691 and 63 occurrences elsewhere).⇒ The key was not inert; it was live by accident. So the fix had to preserve both documented examples, not merely stop lying.
⚠️ A fix written from the card's wording alone would likely have broken them.⭐ The ablation, and one entry that is the defect in miniature
Four points, direction predicted before each run, all four correct; every script refused to run (exit 90) unless the mutation was confirmed on disk, and printed its restore leg.
P4 replaced
'bg-muted'with['bg-','muted'].join('')— still a valid utility at runtime. Predicted: only the verbatim-source test reds. Observed: exactly 1 red / 20 pass — the design-system test, the resolver tests and every render test stayed green.That is #6178 reproduced in miniature: a class valid at runtime but unextractable from source, with every instrument except the source-text one reporting green. ⭐ It demonstrates the new guard catches the original defect class, which is a stronger claim than "the fix works".
✅ The honest limit was stated without being asked: under P1/P2 the two "a mapped token renders its literal class" cases on the reverted branch still passed, because the old concatenation produces the same string for a mapped value. That is the render tests' real blind spot, declared.
Independently checked by me
- Prototype guard is real:
Object.prototype.hasOwnProperty.call(...), not a bare index —headerColor: 'constructor'returnsundefined, and there is a test for it. - The regression pin is not vacuous: it asserts no interpolated utility class in either the module or the call sites, and that
headerColorClass(section.headerColor)still appears exactly twice — so the pin cannot pass becauseheaderColorstopped being read altogether. ⭐ That second half is the part people forget. - The generation test carries negative controls that can say no —
bg-,bg-mutedd,bg-not-a-tokenall build nothing;bg-mutedbuildsbackground-color. So a green there means "emitted", not "instrument inert". - Path face is clean:
.changeset/+packages/plugin-detail/src/**. ⛔ No governed surface — normal landing path. - Two gates reporting a broken gauge were correctly classified, not treated as failures:
check:eager-closureandcheck:readme-exportsboth self-diagnose an unbuilt worktree (0 of 40 packages built), and neither names a file in this diff. ✅ CI builds first.
On the guard it had to judge
My dispatch said: if the fix closes the value set, stop and report. The dev judged this shape does not close it — the declared type is unchanged, nothing is rejected, and the
bg-*pass-through keeps the accepted set open — and landed it as draft with the reshaping stated at the top of the PR body for the reviewer to overrule.✅ That is the correct reading and the correct handling. What changed is not what the key accepts but which values paint, and ending accidental collision-liveness is the fix — no change satisfying this card can preserve it. The risk is bounded by three measurements: zero producers author
headerColoranywhere in either repo, both documented examples are preserved, and every previously-colliding suffix has a one-word migration ('blue-100'→'bg-blue-100'), which the changeset states.⚠️ One reviewer note, not a blocker: the changeset ispatch. A runtime behaviour change on a published package could argue forminor. The declared type does not move and no in-repo producer exists, sopatchis defensible and the bump gates passed — recording the observation rather than blocking on it.Landing
mergeable_statewasbehind; a base merge is in progress. ⛔ Not enqueuing until it re-readscleanwith every check green.Routed onward, ⛔ not decided here
- objectui#6295 — the dev's out-of-scope find:
useRowColorindexes a record-derived string unguarded, so a colour field holdingconstructor/toStringresolves to anObject.prototypefunction, passes theif (!color)truthiness guard, and throwscolor.startsWith is not a functionduring render. ⭐ Reproduced on the exact expression chain before filing, and deduped on the zero-quota channel first. Givenfinding; ⛔domain:*and grading are triage's. - The spec question —
@objectstack/specrefusesheaderColoron the strictrecord:detailsschema, and the recorded reason is this exact defect ("declaring it would advertise a capability the renderer does not deliver"). That reason is now obsolete. ⛔packages/specbelongs to thedomain:speclane and this seat does not touch it — routing it there rather than deciding it.
Generated by Claude Code
- Prototype guard is real:
Measured at
190fbd01(the current objectstack.objectui-shapin) while working objectstack-ai/objectstack#11661 (declaring renderer-honouredrecord:detailssection keys):packages/plugin-detail/src/DetailSection.tsx:513and:542are the only read points:section.headerColor &&+ a template-literal class of the formbg-+${section.headerColor}insidecn(...).tailwindcss ^4.3.3) with@sourcescanning over source files and no safelist —apps/console/src/__tests__/sdui-preview-page-source-styling.test.tsdocuments this repo's own reading: "a utility class authored in real page metadata produces no CSS". A template-literal class is never a complete token in scanned source, so this call site generates no CSS at all; an authored value styles the header only when some other source file happens to use the identicalbg-<value>class literally (accidental, unversioned liveness).headerColoranywhere in objectstack (examples, platform pages, content) or in objectui outside the@object-ui/typesmirror and thedetail-sectionregistry input row (packages/plugin-detail/src/index.tsx:287) — both declaration-only surfaces, which is whycheck:react-declaration-paritynever saw it (it compares two declarations, never a declaration against a renderer).Consequence: the key is unsafe-by-construction — advertised by the registry input and the
@object-ui/typesmirror (views.tsdocuments'muted','primary/10'examples) while the renderer cannot deliver it for arbitrary values. objectstack#11661 therefore deliberately does not declareheaderColoron the strict spec section schema (declaring a dead key is the capability-illusion shape).Fix directions (either, plus dropping the registry input row and the mirror examples):
headerColorfromDetailSection.tsx, thedetail-sectionregistry inputs, and the@object-ui/typesmirror) — smallest honest surface; matches the spec-side refusal.bg-muted/bg-primary/10strings), then propose the enum for spec declaration.Filed as a finding (observation, unassigned) — the disposition is a triage call.
Back-link: objectstack-ai/objectstack#11661.