Uh oh!
There was an error while loading. Please reload this page.
fix(components): page:header resolves declared action ids - #7180
Merged
Conversation
`@objectstack/spec`'s `PageHeaderProps.actions` is `z.array(z.string())` — "Action IDs to show in header" — but the canonical `page:header` renderer read the array as ActionDef objects and resolved nothing, so metadata satisfying the published contract rendered zero header buttons. Resolve ids against the object's own metadata at the top of the actions pipeline, through the same `useMetadataItem` entry `record:quick_actions` uses, so the existing placement / capability / visible / order chain runs unchanged over uniformly-shaped defs. Inline objects keep working as renderer tolerance for the transition, per element, so a half-migrated array resolves. An id that resolves to nothing warns once and renders nothing, naming the object's declared action names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
Document the contract the renderer now honours: the header names actions by id and resolves them against the object's own metadata, with the inline-object form named as migration tolerance rather than a second declared shape. Adds the changeset and types the console-warning assertions in the new pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
os-dev-report Same report as #6252 (comment 5492213936); posted here too because the dispatch asked for it on the PR. {
"issue": 6252,
"status": "done",
"branch": "claude/issue-6252-page-header-action-ids",
"pr": "https://github.com/objectstack-ai/objectui/pull/7180",
"premise_still_valid": true,
"summary": "The canonical page:header renderer (packages/components/src/renderers/layout/containers.tsx) now resolves each element of `actions` as a declared action id against the object's own metadata, through the same useMetadataItem entry record:quick_actions uses — no second resolver. Normalisation sits at the TOP of the actions pipeline, so the existing single filter chain (actionRendersAt placement, requiredPermissions capability gate, visible/hidden, order, inline/overflow split) runs unchanged over uniformly-shaped defs; that is what makes the equivalence claim measurable. Inline ActionDef objects keep working per element, so a half-migrated ['convert', {def}] array resolves the id and passes the object through — renderer tolerance for the migration, still undeclared. An id that resolves to nothing renders nothing and warns ONCE naming the object's declared action names (suppressed while the metadata read is in flight, so a correct page never warns on first paint); silently dropping it would reproduce the silent-loss class this seat keeps filing. Clause 2 fence honoured: the spec type and its zod mirror are untouched, actions stays z.array(z.string()). Docs updated (content/docs/guide/slotted-pages.md taught only the object form) plus a minor changeset. ZONE 2 VERDICTS, all re-measured on this repo's head: A2.1 CONFIRMED (the pre-fix chain read .locations/.requiredPermissions/.visible/.order straight off each array element via actionRendersAt, no lookup anywhere). A2.2 CONFIRMED — exactly ONE canonical page:header registration, ComponentRegistry.register('header', PageHeaderRenderer, {namespace:'page', skipFallback:true}) in containers.tsx; packages/layout/src/index.ts deliberately does NOT re-register it and says so, and the legacy `page-header` alias already resolves ids by delegating to record:quick_actions — so the multi-spelling hazard the PM flagged does not fire here, and the sibling that already implements the ruling is the alias, not a second canonical mount. A2.3 CONFIRMED, cheap. A2.4 CONFIRMED — nothing pinned the rendered result of a string-authored page:header; the only two id-shaped fixtures in the repo pin VALIDATION only (examples/schema-catalog pins that actions:['export'] validates clean, which supports ids), so nothing had to be updated or deleted. A2.5 CONFIRMED with no seam: @object-ui/components already depends on @object-ui/react and already imports eight hooks from it, so useMetadataItem was reachable with no new dependency and no second context scope.",
"tests": "All runs at final commit af40cf5c2 with a clean working tree (git status --short empty), so every figure below read exactly that tree. Heavy runs went through the container's shared verify lock; verdicts read from its VERDICT line, exit codes captured before any pipe. (1) NEW PIN packages/components/src/__tests__/page-header-action-ids.test.tsx, registered in vitest.config.mts heavyDomTests (required — it renders through the ComponentRegistry): 'pnpm exec vitest run --project dom-heavy ...' gives Test Files 1 passed (1), Tests 8 passed (8). Shape of the proof: the SAME action metadata authored twice, once as ids and once as inline objects, both renders compared on the ordered button-name list AND a normalised innerHTML projection; the object-shape render is the LIVE CONTROL and is asserted non-empty first, so an id-side green cannot come from two empty headers agreeing. The population makes one equivalence assertion cover every filter the acceptance criterion names: list_item-only must not render, record_more must route to the overflow menu and never inline, a requiredPermissions the user lacks is denied, a CEL visible false for the record hides, and qualify (order 1) renders BEFORE convert (order 2) — the reverse of the order both authorings list them in. (2) ABLATION, direction predicted before running (red on the id path, green on the object-shape control). Mutation = feeding rawHeaderActions back into the filter chain. Proven on disk by BOTH marker count and blob hash, never by an editor's exit code: PRE target-count=1 inject-count=0 PRE_HASH=3dafeb94 equal to the HEAD blob; POST target-count=0 inject-count=1 POST_HASH=f87a37c9. Result: Tests 7 failed | 1 passed (8) — passing count asserted alongside the red, and 7/1 matched the prediction exactly (the one immovable case is 'does not warn while the metadata lookup is still in flight', true with or without resolution, so this is a targeted break and not a collapsed population). Restore proven by STATE not exit code: restored blob 3dafeb94 equals the HEAD blob, and git diff HEAD / git diff --cached / git status --short are all empty; the script carried an EXIT/INT/TERM trap with an absolute path. No rebuild needed on either leg and this was measured, not assumed: the root vitest.config.mts aliases @object-ui/components to packages/components/src, so the suite resolves SOURCE, never dist. (3) ACCEPTANCE CRITERION 3, measured against the BUILT ARTIFACT as the card demands. pnpm --filter @object-ui/components build, then a run mounting the built packages/components/dist/index.js by explicit relative import (the workspace alias would otherwise redirect to src) with an id-authored header whose resolved action carries body:{language:'js',source:'...OS6252_DIST_BODY_MARKER...'}: the id resolved through the BUILT renderer, the marker appears nowhere in the rendered DOM, and the authored node's JSON.stringify is byte-identical before and after render. LIVE CONTROL: with that one import removed the same file fails 'expected undefined to be truthy' (page:header is unregistered in the light dom project), which proves the passing run measured the built bundle and nothing else. That measurement is deliberately NOT COMMITTED — turbo's test task is dependsOn ['^build'], dependencies only, so a committed dist-importing test would be NOT MEASURED in CI rather than a pin; filed as objectui#7183. The committed half is the pin 'never writes a resolved def — and so no body.source — back onto the authored node', with the object-shape authoring as its live control: expect(JSON.stringify(objectAuthored)).toContain(BODY_MARKER) — the inline form DOES drag the handler body into what it serializes, which is what proves the id-side assertion can fail at all. Built-bundle greps with controls: 'did not resolve'=1 and 'page:header'=7 present, 'OS6252_HANDLER_BODY_MARKER'=0. (4) REGRESSION SURFACE: 'vitest run packages/components scripts/__tests__/vitest-invocation-guard.test.ts' gives 224 files, 2070 passed; all page-header* plus RecordDetailView.header*/predicate suites give 16 files, 184 passed; console registry/contract parity (registry-inputs-spec-parity, public-contract, ga-honoured-inputs-author-reach, component-input-union-specimens, record-block-record-reach) gives 156 passed. (5) TYPES: 'pnpm --filter @object-ui/components run type-check' exit 0 — note the script is spelled type-check (hyphen) in this repo — and it demonstrably COVERS the new test file: tsc -p tsconfig.test.json reported four TS7006 errors inside it before they were fixed, so this is not a typecheck that excludes tests. (6) GATES: check:control-bytes OK (5945 tracked text files); check:doc-types OK; check:doc-fences OK; check:doc-snippets OK, 272/272 blocks judged 0 failed, after building the closure it names (its first run was PRECONDITION NOT MET exit 2 — recorded as NOT MEASURED, then re-run, never as a pass); check:sdui-registration-pins OK, all 16 registrations present, after building the console plus its dependency closure (its first two runs were exit 2 'no console build to weigh' and a TS2307 unbuilt-closure error — both NOT MEASURED, not results). (7) LINT: eslint over all three changed source files, 0 errors (5 no-explicit-any WARNINGS in the new test matching its sibling page-header tests; lint.yml deliberately sets no --max-warnings). The narrowing is measured, not assumed, all three pieces: population is the diff itself via git diff --name-only; the count comes from --format json (3 files linted); and the config is not type-aware — eslint.config.js has zero projectService / parserOptions / project: occurrences against a live control of 10 'rules' hits in the same file — so with no config file in the diff no untouched file's verdict can move. The repo-wide farm is CI's run either way. CI convergence not awaited, per the standing dispatch contract.",
"mcp_calls": "12 — issue_read get + get_comments, create_pull_request, pull_request_read (body read-back), search_issues x2 (the second was the required control and it FAILED to hit, see open_questions), issue_write create x2, and this report's comments plus one read-back. Bulk reads went to git and to the zero-quota web payload channel instead.",
"open_questions": [
{
"question": "MECHANISM, not a code decision — the MCP `search_issues` channel appears blind in this container, and this seat's dedupe had to route around it. A targeted search for this card's own near-verbatim title returned total_count 0, i.e. the known-must-hit CONTROL failed, so its earlier empty result for my dedupe query was not a reading. Unauthenticated REST is also 403 here (both /repos/.../issues and /search/issues). Dedupe was completed on the zero-quota GitHub web payload channel with a live control that DID hit (query 'action-id lookup' returned 8 issue numbers including 6252; the two dedupe queries returned disjoint populations with no duplicate of either finding), so both findings are filed properly — but the next seat should know that search_issues answering 0 here means nothing.",
"options": [
"A — treat MCP search_issues as unavailable for this seat/container and make the web payload channel the documented dedupe route, control word mandatory",
"B — investigate why search_issues returns 0 for a query matching an issue it can read directly via issue_read (indexing lag vs. a scope/permission difference between the read and search paths)",
"C — do nothing; each dev re-discovers it per run"
],
"recommendation": "A plus B. A is cheap and immediate — the web channel is measured working here, costs no GraphQL quota, and the control discipline the dispatch already mandates is exactly what caught this. B matters because a silently-empty search is the worst failure shape available to a dedupe step: it looks like 'no duplicate exists' and it is unfalsifiable without a control, so a seat that skips the control files duplicates and never learns."
}
],
"out_of_scope_findings": [
"filed as objectui#7182: the action-id lookup now exists twice (record-quick-actions.tsx and containers.tsx) and the two copies DISAGREE on mixed arrays — quick_actions switches on the whole array (every element a string), page:header normalises per element — so during the migration this card opens, one authored array means two things depending on which renderer draws it. Not fixed here: unifying them changes record:quick_actions behaviour on a renderer this card does not own, and wants a ruling on whether a mixed array is a supported migration state.",
"filed as objectui#7183: no committed test can observe its own package's BUILT artifact — the root vitest alias sends every workspace specifier to src, and turbo's test task is dependsOn ['^build'] (dependencies only), so a dist-importing test is MODULE_NOT_FOUND / NOT MEASURED in CI. This card's acceptance criterion 3 explicitly asks for a built-artifact measurement; it was run by hand with a live control and passes, but it cannot be left behind as a standing pin."
]
}Generated by Claude Code |
os-warren
commented
Sep 1, 2026
CollaboratorAuthor
PM review — |
This was referenced Sep 1, 2026
os-warren
marked this pull request as ready for review
September 1, 2026 10:05
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6252
Implements the objectstack#11592 ruling (maintainer, 2026-08-25, on recommendation B): the canonical
page:headerrenderer resolvesPageHeaderProps.actionsas action ids.The defect
@objectstack/specdeclaresactions: z.array(z.string()).optional().describe('Action IDs to show in header')(verified on the installed pin,@objectstack/spec@17.2.0,src/ui/component.zod.ts). The renderer read that array asActionDefobjects and resolved nothing, so a header authored the way the published contract declares rendered zero buttons — satisfying the schema deleted the header.Two sibling surfaces already read it as ids:
record:quick_actions(packages/plugin-detail/src/renderers/record-quick-actions.tsx) resolves a string-valuedactionsout of the object's metadata, andlayout:page-headerreaches the same resolver by delegating itsactionsinto arecord:quick_actionsnode (packages/layout/src/PageHeader.tsx:196-208). So one authoring key meant two different things depending on which renderer drew the header.What changed
packages/components/src/renderers/layout/containers.tsx:actionsis normalised at the top of the pipeline — a string is resolved against the object's ownactionsmetadata, an object passes through. Resolution goes through the sameuseMetadataItementryrecord:quick_actionsuses; no second resolver, no second lookup path.actionRendersAtplacement (record_header/record_more), therequiredPermissionscapability gate,visible/hidden,order, and the inline/overflow split are untouched by this change. That is what makes the equivalence claim testable at all.actionsinput description now says ids.The one deliberate difference from
record:quick_actions: that renderer switches on the whole array (rawActions.every(a => typeof a === 'string')); this one normalises per element, so a half-migrated['convert', { … }]resolves the id and passes the object through. Same mechanism, wider arity — a page mid-migration is exactly the state this card creates.Clause ② is not engaged. The declared type and its zod mirror are untouched —
actionsstaysz.array(z.string()). The object shape survives as renderer tolerance, exactly as it does today, undeclared. No accept/reject behaviour of the contract moves.Docs:
content/docs/guide/slotted-pages.mdnow teaches the ids contract on the canonical header (it previously showed only the object form), with the inline-object form named as migration tolerance.Acceptance criteria — evidence
Run at
af40cf5c2(working tree clean at that commit, so every run below read exactly this tree).1. An id-authored header renders the same buttons as the object-shape authoring.
The same action metadata is authored twice and the two renders compared. The object-shape render is the live control: each equivalence case asserts it is non-empty first, so an id-side green can never come from two empty headers agreeing.
The population covers every filter the criterion names, so one equivalence assertion measures all of them:
list_only(list_item— must not render),archive(record_more— must land in the ⋯ menu, not inline),gated(requiredPermissionsthe user lacks),closed_only(a CELvisiblefalse for this record), andqualify(order: 1) rendering beforeconvert(order: 2), which is the reverse of the order both authorings list them in. Both the ordered button-name list and a normalisedinnerHTMLprojection are asserted equal.2. The renderer's tests gain the id-shaped cases.
packages/components/src/__tests__/page-header-action-ids.test.tsx— 8 cases (equivalence, the filter chain,record_morerouting, unresolved-id warn-once with a resolvable sibling as control, no-warn-while-loading, mixed arrays, theproperties.actionsbridge spelling, and criterion 3).Ablation — direction predicted before running: red on the id path, green on the object-shape control. Mutation = feeding
rawHeaderActionsback into the filter chain (the minimal deletion of the fix). Proven on disk by marker count and blob hash, never by an editor's exit code:7 failed / 1 passed matched the prediction exactly: the only case that cannot move is "does not warn while the metadata lookup is still in flight", which is true with or without resolution. Restore proven by state, not exit code: restored blob
3dafeb94…equals the HEAD blob, andgit diff HEAD,git diff --cached,git status --shortare all empty. The mutation and restore legs need no rebuild here: the rootvitest.config.mtsaliases@object-ui/componentstopackages/components/src, so this suite resolves source, notdist.3. The id path carries no
body.sourceinto the built artifact.Measured two ways, and the built-artifact one is the load-bearing half.
Against the built artifact.
pnpm --filter @object-ui/components build, then a run that mounts the builtpackages/components/dist/index.js(an explicit relative import — the workspace alias would otherwise redirect to src) with an id-authored header whose resolved action carriesbody: { language: 'js', source: '…OS6252_DIST_BODY_MARKER…' }. The id resolved through the built renderer, the marker appears nowhere in the rendered DOM, and the authored node'sJSON.stringifyis byte-identical before and after render. Live control: with that one import removed the same file failsexpected undefined to be truthy—page:headeris unregistered in the lightdomproject, which proves the passing run measured the built bundle and nothing else. This measurement is not committed: turbo'stesttask isdependsOn: ["^build"](dependencies only, not the package's own build), so a committeddist-importing test would beNOT MEASUREDin CI rather than a pin. Filed as a follow-up.As a committed pin.
never writes a resolved def — and so no body.source — back onto the authored nodeasserts the authored node is unchanged by rendering and the marker is absent from the DOM, with the object-shape authoring as the live control:expect(JSON.stringify(objectAuthored)).toContain(BODY_MARKER)— the inline form does drag the handler body into what it serializes, which is what proves this assertion can fail at all.Grep of the built bundle, with controls:
did not resolve= 1 andpage:header= 7 (present),OS6252_HANDLER_BODY_MARKER= 0.Other verification
vitest run packages/components scripts/__tests__/vitest-invocation-guard.test.tspage-header*+RecordDetailView.header*/ predicate suitesregistry-inputs-spec-parity,public-contract)pnpm --filter @object-ui/components run type-checktsc -p tsconfig.test.jsonreported errors in it before they were fixed)pnpm check:control-bytespnpm check:doc-typespnpm check:doc-fencespnpm check:doc-snippetspnpm check:sdui-registration-pinseslinton all three changed source files: 0 errors (5 pre-existing-styleno-explicit-anywarnings in the new test, matching its sibling page-header tests;--max-warningsis deliberately unset in this repo). The lint run is narrowed and the narrowing is measured, not assumed: the population is the diff itself (git diff --name-only), the count comes from--format json(3 files linted), and the config is not type-aware —eslint.config.jscontains zeroprojectService/parserOptions/project:occurrences against a live control of 10ruleshits in the same file — so no untouched file's verdict can move under this diff, which touches no config. The repo-wide farm is CI's run either way.Not in this PR
hotcrm's migration of its 16 inline
record_headerobjects to id references is downstream of this landing, as the card records. Nothing here touches it.Generated by Claude Code