From 87aa90858b884e93995603e2ddc95dca4056e8e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 17:02:38 +0000 Subject: [PATCH 1/4] fix(app-shell): a position decision output lists the position registry and commits position names (objectui#7611) The Approve/Reject dialog rendered a position decision output as a sys_position lookup. A fresh database holds no sys_position rows, so the picker offered nothing, and a row it did offer committed a row id where the spec (objectstack#22710) and the engine's resolveAs position read a machine name. decisionOutputParams moves position from the record-lookup table to a catalog table, and renders it as a select over the registry items the caller read through the console's metadata store (ensureType), the reader RecipientPickerField lists the same registry through. DeclaredActionsBar reads them at click time. No reader degrades to free text; an empty registry is still a picker. Claude-Session: https://claude.ai/code/session_01Rerax7QTjKMPCUZxQUtPFR Co-authored-by: Claude --- .../7611-decision-output-position-registry.md | 11 ++ apps/console/src/services/approvalsApi.ts | 7 +- .../src/utils/decisionOutputParams.test.ts | 122 +++++++++++++++++- .../src/utils/decisionOutputParams.ts | 108 +++++++++++++++- .../src/views/DeclaredActionsBar.tsx | 28 +++- .../__tests__/DeclaredActionsBar.test.tsx | 65 +++++++++- 6 files changed, 324 insertions(+), 17 deletions(-) create mode 100644 .changeset/7611-decision-output-position-registry.md diff --git a/.changeset/7611-decision-output-position-registry.md b/.changeset/7611-decision-output-position-registry.md new file mode 100644 index 0000000000..ad934bc41d --- /dev/null +++ b/.changeset/7611-decision-output-position-registry.md @@ -0,0 +1,11 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/console': minor +--- + +A `position` decision output lists the position registry and commits position names (objectui#7611, objectstack ADR-0131 D3/D4). + +- **What the approver sees.** In the Approve and Reject dialogs (the Approval Center and the record page both decide through `DeclaredActionsBar`), a `position` output is a select over the `position` items the registry serves, read through the console's metadata store (`ensureType('position')`), the same reader the sharing-rule recipient picker lists positions through. Each option is labelled by the position's `label`, else its name, and the list is sorted by name. It used to be a `sys_position` lookup, which offered nothing on a database without `sys_position` rows. +- **What the flow receives.** The decide body carries the picked positions' machine names (an array when the output is `multiple`). That is what the spec says the output collects (objectstack#22710), and what the engine's `resolveAs: 'position'` matches against `sys_user_position.position`. The lookup committed `sys_position` row ids instead. +- **Without a metadata store.** A surface rendered outside a `` has no registry reader, so a `position` output there is the free-text box an untyped output gets, the same fallback the recipient picker takes. An empty registry is not that case: it renders the select with no options. +- `department` and `team` outputs are unchanged: they are still record lookups and commit record ids. diff --git a/apps/console/src/services/approvalsApi.ts b/apps/console/src/services/approvalsApi.ts index 412a2ce00b..a74f6b8bb5 100644 --- a/apps/console/src/services/approvalsApi.ts +++ b/apps/console/src/services/approvalsApi.ts @@ -135,9 +135,10 @@ export interface ApprovalRequestRow { /** * framework#3447 follow-up: the normalized TYPED declarations behind * `decision_outputs` — `{ key, label?, type?, multiple? }` — so the decide - * dialog renders a record picker (user / department / position / team; id - * values, `multiple` → id array) instead of free text. Prefer this and fall - * back to the bare key list (older backend). + * dialog renders a picker instead of free text: user / department / team + * pick records and commit ids, position lists the position registry and + * commits machine names (objectui#7611); `multiple` → an array of them. + * Prefer this and fall back to the bare key list (older backend). */ decision_output_defs?: Array<{ key: string; diff --git a/packages/app-shell/src/utils/decisionOutputParams.test.ts b/packages/app-shell/src/utils/decisionOutputParams.test.ts index 893a219f55..e6198dab72 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.test.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.test.ts @@ -21,11 +21,12 @@ * object looked right), so the round trip through both stages is the pin. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { decisionOutputDefs, decisionOutputParams, foldDecisionOutputs, + readDecisionOutputRegistry, } from './decisionOutputParams'; import { resolveActionParams } from '@object-ui/fields'; import { paramToField } from './paramToField'; @@ -82,6 +83,7 @@ describe('decisionOutputParams — declaration → param', () => { const [param] = decisionOutputParams( [{ key: 'parallel_positions', label: '并行会审岗位', type: 'position', multiple: true }], t, + { registry: { position: [] } }, ); expect(param).toMatchObject({ label: '并行会审岗位', multiple: true }); }); @@ -127,7 +129,6 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955) // union, and a bare table widens it to `string` (objectui#4040). it.each([ ['department', 'sys_business_unit'], - ['position', 'sys_position'], ['team', 'sys_team'], ] as const)('a %s output reaches the widget as a %s picker', (type, referenceTo) => { const [param] = decisionOutputParams([{ key: 'k', type, multiple: true }], t); @@ -151,7 +152,7 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955) // were dropped in resolution the dialog would submit a blank value and let // the server 400 instead. const [param] = decisionOutputParams( - [{ key: 'k', type: 'position', required: true }], t, { decision: 'approve' }, + [{ key: 'k', type: 'team', required: true }], t, { decision: 'approve' }, ); expect(widgetFor(param)).toMatchObject({ type: 'lookup', required: true }); }); @@ -164,13 +165,126 @@ describe('decisionOutputParams — the params survive resolution (objectui#2955) it('never degrades a typed picker to a record-id text box', () => { // The #2955 symptom, stated as the invariant: no declared record kind may // arrive at the widget as `text` (that is the "paste a UUID" fallback). + // `position` is a catalog kind, so it is judged with a registry read in + // hand — an empty one, the state a fresh environment can be in. for (const type of ['user', 'department', 'position', 'team'] as const) { - const [param] = decisionOutputParams([{ key: 'k', type }], t); + const [param] = decisionOutputParams([{ key: 'k', type }], t, { registry: { position: [] } }); expect(widgetFor(param).type).not.toBe('text'); } }); }); +/** + * objectui#7611 (ADR-0131 D3/D4) — a `position` output lists the position + * REGISTRY and commits the position's machine name, which is what the spec + * says the output collects (objectstack#22710) and what the engine's + * `resolveAs: 'position'` matches against `sys_user_position.position`. It used + * to be a `sys_position` lookup: a fresh database holds no `sys_position` rows, + * so the picker offered nothing, and a row it did offer committed a row id. + */ +describe('decisionOutputParams — a position output lists the registry (objectui#7611)', () => { + const POSITIONS = [ + { name: 'manager', label: 'Project Manager' }, + { name: 'auditor', label: 'Auditor' }, + { name: 'exec' }, + { label: 'nameless — not a catalog item' }, + ]; + + it('offers the registry items by name, sorted by name, labelled by `label`', () => { + const [param] = decisionOutputParams( + [{ key: 'parallel_positions', type: 'position', multiple: true }], + t, + { registry: { position: POSITIONS } }, + ); + expect(param).toMatchObject({ type: 'select', multiple: true }); + expect(param.options).toEqual([ + { value: 'auditor', label: 'Auditor' }, + { value: 'exec', label: 'exec' }, + { value: 'manager', label: 'Project Manager' }, + ]); + // Nothing names the retired record source. + expect(param.reference).toBeUndefined(); + expect(JSON.stringify(param)).not.toContain('sys_position'); + }); + + it('reaches the widget as a select whose values are the machine names', () => { + const [param] = decisionOutputParams( + [{ key: 'parallel_positions', type: 'position', multiple: true }], + t, + { registry: { position: POSITIONS } }, + ); + const field = widgetFor(param); + expect(field).toMatchObject({ type: 'select', multiple: true }); + expect((field.options as Array<{ value: string }>).map((o) => o.value)).toEqual([ + 'auditor', + 'exec', + 'manager', + ]); + }); + + it('an empty registry is still a picker, with nothing to offer', () => { + const [param] = decisionOutputParams([{ key: 'p', type: 'position' }], t, { + registry: { position: [] }, + }); + expect(widgetFor(param)).toMatchObject({ type: 'select', options: [] }); + }); + + it('no registry reader degrades to the free-text box an untyped output gets', () => { + // An absent reader is a different fact from an empty registry — the same + // split `RecipientPickerField` draws on the same registry. + const [param] = decisionOutputParams([{ key: 'p', type: 'position' }], t); + expect(param).toMatchObject({ type: 'text', helpText: 't:actions.decisionOutput.helpMultiValue' }); + }); + + it('keeps `required` on approve, so a blank registry pick blocks Confirm', () => { + const [param] = decisionOutputParams( + [{ key: 'p', type: 'position', required: true }], + t, + { decision: 'approve', registry: { position: POSITIONS } }, + ); + expect(widgetFor(param)).toMatchObject({ type: 'select', required: true }); + }); + + it('folds the picked names into the decide body unchanged', () => { + expect(foldDecisionOutputs({ 'outputs.parallel_positions': ['auditor', 'manager'] })).toEqual({ + rest: {}, + outputs: { parallel_positions: ['auditor', 'manager'] }, + }); + }); +}); + +describe('readDecisionOutputRegistry (objectui#7611)', () => { + it('reads each catalog type the outputs list, once, and nothing else', async () => { + const ensureType = vi.fn(async (type: string) => (type === 'position' ? [{ name: 'ops' }] : [])); + const registry = await readDecisionOutputRegistry( + [ + { key: 'a', type: 'position' }, + { key: 'b', type: 'position', multiple: true }, + { key: 'c', type: 'team' }, + { key: 'd', type: 'user' }, + { key: 'e' }, + ], + { ensureType }, + ); + expect(ensureType.mock.calls).toEqual([['position']]); + expect(registry).toEqual({ position: [{ name: 'ops' }] }); + }); + + it('reads nothing when no output is a catalog kind', async () => { + const ensureType = vi.fn(async () => []); + expect(await readDecisionOutputRegistry([{ key: 'c', type: 'team' }], { ensureType })).toEqual({}); + expect(ensureType).not.toHaveBeenCalled(); + }); + + it('answers `{}` without a reader, and `[]` for a read that fails', async () => { + expect(await readDecisionOutputRegistry([{ key: 'a', type: 'position' }], null)).toEqual({}); + const failing = { ensureType: vi.fn(async () => { throw new Error('HTTP_500'); }) }; + expect(await readDecisionOutputRegistry([{ key: 'a', type: 'position' }], failing)).toEqual({ + position: [], + }); + }); +}); + describe('foldDecisionOutputs', () => { it('folds the dotted params into the nested decide body', () => { expect( diff --git a/packages/app-shell/src/utils/decisionOutputParams.ts b/packages/app-shell/src/utils/decisionOutputParams.ts index e728dee349..a45553aed6 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.ts @@ -70,10 +70,96 @@ export const DECISION_OUTPUT_PARAM_PREFIX = 'outputs.'; */ const OUTPUT_LOOKUP_OBJECTS: Record = { department: 'sys_business_unit', - position: 'sys_position', team: 'sys_team', }; +/** + * Catalog kinds: the output's candidates are the items of one metadata type in + * the environment registry, and the value handed to the flow is the item's + * machine name (objectui#7611). + * + * `position` lives here and not in {@link OUTPUT_LOOKUP_OBJECTS}: ADR-0131 D3 + * gives positions one home, the registry, and D4 references a declared item by + * name. The spec says the same of this output (objectstack#22682, landed as + * objectstack#22710): a `position` decision output collects position machine + * names, which is what the engine's `resolveAs: 'position'` matches against + * `sys_user_position.position`. The `sys_position` lookup this entry replaces + * listed rows a fresh database no longer has, so the picker offered nothing. + * + * The read is the one `RecipientPickerField`'s registry kinds make: the + * console's metadata store, `ensureType()`, the same cached reader that + * serves apps and objects — see {@link readDecisionOutputRegistry}. + */ +const OUTPUT_REGISTRY_TYPES: Record = { + position: 'position', +}; + +/** + * The registry items a decision surface read for the catalog kinds above, + * keyed by metadata type. + * + * A type that is ABSENT and a type that maps to `[]` are different facts, and + * {@link decisionOutputParams} keeps them apart: `[]` is an empty registry, so + * the output renders a picker with nothing to offer; an absent entry means no + * reader was available (no metadata store), and only that degrades the output + * to the free-text box an untyped output gets. + */ +export type DecisionOutputRegistry = Readonly>; + +/** The one member of the metadata store this module reads. */ +export interface DecisionOutputRegistryReader { + ensureType: (type: string) => Promise; +} + +/** + * Read the registry items the given outputs list, once per metadata type. + * + * `null` reader (a surface outside a ``) → `{}`: no type is + * read, and each catalog kind degrades as {@link DecisionOutputRegistry} says. + * A type whose read fails maps to `[]`, the same answer `RecipientPickerField` + * gives a failed registry read. + */ +export async function readDecisionOutputRegistry( + defs: DecisionOutputDef[], + reader: DecisionOutputRegistryReader | null | undefined, +): Promise { + if (!reader) return {}; + const types = new Set(); + for (const d of defs) { + const type = d?.type ? OUTPUT_REGISTRY_TYPES[d.type] : undefined; + if (type) types.add(type); + } + const out: Record = {}; + for (const type of types) { + try { + const items = await reader.ensureType(type); + out[type] = Array.isArray(items) ? items : []; + } catch { + out[type] = []; + } + } + return out; +} + +/** + * Registry items → select options: the value is the item's machine name and + * the label its `label`, else its name. Items without a name are dropped, and + * the list is sorted by name — the same reading `RecipientPickerField` makes + * of the same registry. + */ +function registryOptions(items: readonly unknown[]): Array<{ value: string; label: string }> { + const named = items.filter( + (it): it is { name: string; label?: unknown } => + it != null && typeof (it as { name?: unknown }).name === 'string' && !!(it as { name: string }).name, + ); + return [...named] + .sort((a, b) => a.name.localeCompare(b.name)) + .map((it) => ({ + value: it.name, + label: typeof it.label === 'string' && it.label ? it.label : it.name, + })); +} + /** * Read the declared outputs off an approval request row. Prefers the typed * declarations; falls back to the bare key list an older backend sends. @@ -115,11 +201,18 @@ function humanizeKey(key: string): string { * nothing reads the outputs, so demanding routing data to say "no" would trap * the rejection). Marking it here is what turns the server's 400 into a * blocked Confirm button with the field flagged, instead of a round trip. + * + * A catalog kind (`position`) renders a `select` over the registry items the + * caller read with {@link readDecisionOutputRegistry} and passes as + * `registry`; its option values are machine names, so the decide body carries + * the names the engine routes by. `select` is a declared param type whose + * options travel through `resolveActionParams()` untouched, so no new param + * key is needed to get them to the widget. */ export function decisionOutputParams( defs: DecisionOutputDef[], t: (key: string) => string, - opts: { decision?: 'approve' | 'reject' } = {}, + opts: { decision?: 'approve' | 'reject'; registry?: DecisionOutputRegistry } = {}, ): Array> { return defs.filter((d) => d && d.key).map((d) => { const base = { @@ -135,6 +228,17 @@ export function decisionOutputParams( helpText: t('actions.decisionOutput.help'), }; } + const registryType = d.type ? OUTPUT_REGISTRY_TYPES[d.type] : undefined; + const registryItems = registryType ? opts.registry?.[registryType] : undefined; + if (registryItems) { + return { + ...base, + type: 'select', + options: registryOptions(registryItems), + multiple: d.multiple === true, + helpText: t('actions.decisionOutput.help'), + }; + } const lookupObject = d.type ? OUTPUT_LOOKUP_OBJECTS[d.type] : undefined; if (lookupObject) { return { diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index 95b4cbbff9..fd04563de0 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -43,6 +43,7 @@ import { } from '@object-ui/components'; import { ActionProvider, + MetadataCtx, useAction, useCapabilityGate, useCondition, @@ -66,7 +67,11 @@ import { useAdapter } from '../providers/AdapterProvider.js'; // 'authFetchSpy' before initialization`, the side effect running inside the // hoisted `@object-ui/auth` mock factory before the spy existed. import { useMetadataItem } from '@object-ui/react'; -import { decisionOutputDefs, decisionOutputParams } from '../utils/decisionOutputParams.js'; +import { + decisionOutputDefs, + decisionOutputParams, + readDecisionOutputRegistry, +} from '../utils/decisionOutputParams.js'; import { getIcon } from '../utils/getIcon.js'; import { isOverrideDecision, bypassedApproverNames } from '../utils/approvalOverride.js'; import { @@ -142,6 +147,17 @@ const DeclaredActionButton: React.FC<{ // literal IS what renders, which is how English help text survived in a zh-CN // workspace (objectui#2762 P0-3). const { t } = useObjectTranslation(); + // The console's metadata store, or `null` outside a `` — + // the reader a catalog-kind decision output (`position`) lists the registry + // through (objectui#7611). Read raw rather than through `useMetadata()`, + // whose no-provider fallback answers every type with `[]`: an empty registry + // and an absent reader are different facts (`DecisionOutputRegistry`). Held + // in a ref and read at click time, so no callback keys on the store's + // context identity, which is rebuilt whenever any type it caches settles + // (AGENTS.md #10). + const metadataStore = React.useContext(MetadataCtx); + const metadataStoreRef = React.useRef(metadataStore); + metadataStoreRef.current = metadataStore; const recordData = record != null && typeof record === 'object' ? (record as Record) : {}; /** @@ -285,8 +301,16 @@ const DeclaredActionButton: React.FC<{ // Widget mapping (typed picker vs free text) lives in the shared helper, // so the record header's Approve/Reject renders the same controls // (objectui#2955). + // + // A catalog kind (`position`) lists the registry, so its items are read + // before the params are built — once per type, from the store's + // session cache after the first read (objectui#7611). + const outputDefs = decision ? decisionOutputDefs(recordData) : []; const outputParams = decision - ? decisionOutputParams(decisionOutputDefs(recordData), t, { decision }) + ? decisionOutputParams(outputDefs, t, { + decision, + registry: await readDecisionOutputRegistry(outputDefs, metadataStoreRef.current), + }) : []; // Typed as the SEAM's contract, not as `any` and not cast to `ActionDef` // on the way out (objectui#5611). `overrideNotice` below is host-composed diff --git a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx index 8b80a00f39..7b7c9ce7c9 100644 --- a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx +++ b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx @@ -99,7 +99,7 @@ import { DeclaredActionsBar } from '../DeclaredActionsBar'; // The real provider — the `@object-ui/react` double above spreads `actual`, so // the ambient predicate scope the objectui#4080 no-row case needs is the // shipped one, not a stand-in for it. -import { PredicateScopeProvider } from '@object-ui/react'; +import { MetadataCtx, PredicateScopeProvider } from '@object-ui/react'; const REQUEST = { id: 'req_1', status: 'pending', record_id: 'proj_9' }; @@ -337,7 +337,9 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { // `reference`. Spelled `referenceTo` the target was dropped, and the // Approval Center rendered a "paste a record id" text box where a // sys_position multi-select belonged. The full round trip is pinned in - // `utils/decisionOutputParams.test.ts`; this pins what the BAR emits. + // `utils/decisionOutputParams.test.ts`; this pins what the BAR emits for a + // record kind (a `position` output lists the registry since objectui#7611 — + // pinned below). const decideAction = { name: 'approval_approve', type: 'api', @@ -351,7 +353,7 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { record={{ ...REQUEST, decision_output_defs: [ - { key: 'parallel_positions', label: '并行会审岗位', type: 'position', multiple: true }, + { key: 'owning_teams', label: '负责团队', type: 'team', multiple: true }, ], }} location="record_section" @@ -365,15 +367,66 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { const params = executeSpy.mock.calls[0][0].actionParams as Array>; expect(params).toHaveLength(1); expect(params[0]).toMatchObject({ - name: 'outputs.parallel_positions', - label: '并行会审岗位', + name: 'outputs.owning_teams', + label: '负责团队', type: 'lookup', - reference: 'sys_position', + reference: 'sys_team', multiple: true, }); expect(params[0].referenceTo).toBeUndefined(); }); + it('lists the position registry for a position output and offers names (objectui#7611)', async () => { + // ADR-0131 D3/D4: positions live in the environment registry and are + // referenced by machine name. The bar reads the registry through the + // console's metadata store — the reader `RecipientPickerField` lists the + // same registry through — and hands the dialog a select whose values are + // the names the engine routes by. A fresh database has no `sys_position` + // rows, so the lookup this replaces offered nothing. + const ensureType = vi.fn(async (type: string) => + type === 'position' + ? [{ name: 'manager', label: 'Project Manager' }, { name: 'auditor', label: 'Auditor' }] + : [], + ); + const store = { ensureType } as unknown as React.ContextType; + render( + + + , + ); + + fireEvent.click(screen.getByText('Approve')); + await waitFor(() => expect(executeSpy).toHaveBeenCalled()); + + expect(ensureType.mock.calls).toEqual([['position']]); + const params = executeSpy.mock.calls[0][0].actionParams as Array>; + expect(params).toHaveLength(1); + expect(params[0]).toMatchObject({ + name: 'outputs.parallel_positions', + label: '并行会审岗位', + type: 'select', + multiple: true, + options: [ + { value: 'auditor', label: 'Auditor' }, + { value: 'manager', label: 'Project Manager' }, + ], + }); + expect(params[0].reference).toBeUndefined(); + }); + it('localizes the decision-output help text', async () => { const decideAction = { name: 'approval_approve', From 4686fb7dd526bab6da3d2b9b584addbaa1e78df1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 17:14:52 +0000 Subject: [PATCH 2/4] test(app-shell): the registry fixture carries an empty-name item, so the name guard is pinned (objectui#7611) The first ablation of the empty-name clause in registryOptions stayed green: the fixture held no item whose name is an empty string. Claude-Session: https://claude.ai/code/session_01Rerax7QTjKMPCUZxQUtPFR Co-authored-by: Claude --- packages/app-shell/src/utils/decisionOutputParams.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/app-shell/src/utils/decisionOutputParams.test.ts b/packages/app-shell/src/utils/decisionOutputParams.test.ts index e6198dab72..74988314e5 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.test.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.test.ts @@ -188,6 +188,7 @@ describe('decisionOutputParams — a position output lists the registry (objectu { name: 'auditor', label: 'Auditor' }, { name: 'exec' }, { label: 'nameless — not a catalog item' }, + { name: '', label: 'empty name — not a catalog item' }, ]; it('offers the registry items by name, sorted by name, labelled by `label`', () => { From 4223b14c0ed5cf80f8ccf1d1a3a8628b482d80b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 17:18:16 +0000 Subject: [PATCH 3/4] refactor(app-shell): the bar syncs its metadata-store ref after commit, not during render (objectui#7611) Writing the ref during render raised react-hooks/refs on DeclaredActionsBar. A click always lands after a commit, so an effect with no dependency list keeps the ref current without keying on the store's context identity. Claude-Session: https://claude.ai/code/session_01Rerax7QTjKMPCUZxQUtPFR Co-authored-by: Claude --- packages/app-shell/src/views/DeclaredActionsBar.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index fd04563de0..2354f9ac4d 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -154,10 +154,13 @@ const DeclaredActionButton: React.FC<{ // and an absent reader are different facts (`DecisionOutputRegistry`). Held // in a ref and read at click time, so no callback keys on the store's // context identity, which is rebuilt whenever any type it caches settles - // (AGENTS.md #10). + // (AGENTS.md #10). The ref is synced after each commit rather than during + // render; a click always lands after one. const metadataStore = React.useContext(MetadataCtx); const metadataStoreRef = React.useRef(metadataStore); - metadataStoreRef.current = metadataStore; + React.useEffect(() => { + metadataStoreRef.current = metadataStore; + }); const recordData = record != null && typeof record === 'object' ? (record as Record) : {}; /** From 6eb495c67d25b26732ac83dc9955322fe5660a6c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 17:49:11 +0000 Subject: [PATCH 4/4] fix(app-shell): only a decision with a catalog output awaits the registry read (objectui#7611) Awaiting the registry read on every approve/reject moved the dispatch out of the click's tick, and DeclaredActionsBar.overrideAffordance's poll, which reads the dispatch synchronously after its click, timed out three times. A decision with no catalog output now dispatches synchronously, as before; decisionOutputsReadRegistry is the one predicate the bar asks, and a store with no catalog output to serve is never read. Claude-Session: https://claude.ai/code/session_01Rerax7QTjKMPCUZxQUtPFR Co-authored-by: Claude --- .../src/utils/decisionOutputParams.test.ts | 17 ++++++++++++ .../src/utils/decisionOutputParams.ts | 12 +++++++++ .../src/views/DeclaredActionsBar.tsx | 14 ++++++---- .../__tests__/DeclaredActionsBar.test.tsx | 26 +++++++++++++++++++ 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/packages/app-shell/src/utils/decisionOutputParams.test.ts b/packages/app-shell/src/utils/decisionOutputParams.test.ts index 74988314e5..1b2ccd96c1 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.test.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.test.ts @@ -25,6 +25,7 @@ import { describe, it, expect, vi } from 'vitest'; import { decisionOutputDefs, decisionOutputParams, + decisionOutputsReadRegistry, foldDecisionOutputs, readDecisionOutputRegistry, } from './decisionOutputParams'; @@ -254,6 +255,22 @@ describe('decisionOutputParams — a position output lists the registry (objectu }); }); +describe('decisionOutputsReadRegistry (objectui#7611)', () => { + it('is true only when an output is a catalog kind', () => { + expect(decisionOutputsReadRegistry([{ key: 'a', type: 'team' }, { key: 'b', type: 'position' }])).toBe(true); + expect( + decisionOutputsReadRegistry([ + { key: 'a', type: 'user' }, + { key: 'b', type: 'department' }, + { key: 'c', type: 'team' }, + { key: 'd', type: 'text' }, + { key: 'e' }, + ]), + ).toBe(false); + expect(decisionOutputsReadRegistry([])).toBe(false); + }); +}); + describe('readDecisionOutputRegistry (objectui#7611)', () => { it('reads each catalog type the outputs list, once, and nothing else', async () => { const ensureType = vi.fn(async (type: string) => (type === 'position' ? [{ name: 'ops' }] : [])); diff --git a/packages/app-shell/src/utils/decisionOutputParams.ts b/packages/app-shell/src/utils/decisionOutputParams.ts index a45553aed6..217b7f8a5f 100644 --- a/packages/app-shell/src/utils/decisionOutputParams.ts +++ b/packages/app-shell/src/utils/decisionOutputParams.ts @@ -111,6 +111,18 @@ export interface DecisionOutputRegistryReader { ensureType: (type: string) => Promise; } +/** + * Does any of these outputs list a registry? + * + * A decision surface asks this before it awaits + * {@link readDecisionOutputRegistry}, so a decision with no catalog output + * still dispatches in the click's own tick, as it did before catalog kinds + * existed; only a decision that has a registry to read waits for it. + */ +export function decisionOutputsReadRegistry(defs: DecisionOutputDef[]): boolean { + return defs.some((d) => !!(d?.type && OUTPUT_REGISTRY_TYPES[d.type])); +} + /** * Read the registry items the given outputs list, once per metadata type. * diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index 2354f9ac4d..be58bb51f9 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -70,6 +70,7 @@ import { useMetadataItem } from '@object-ui/react'; import { decisionOutputDefs, decisionOutputParams, + decisionOutputsReadRegistry, readDecisionOutputRegistry, } from '../utils/decisionOutputParams.js'; import { getIcon } from '../utils/getIcon.js'; @@ -307,13 +308,16 @@ const DeclaredActionButton: React.FC<{ // // A catalog kind (`position`) lists the registry, so its items are read // before the params are built — once per type, from the store's - // session cache after the first read (objectui#7611). + // session cache after the first read (objectui#7611). Only a decision + // that HAS a catalog output awaits that read; every other decision + // dispatches in the click's own tick, as before. const outputDefs = decision ? decisionOutputDefs(recordData) : []; + const metadataStoreNow = metadataStoreRef.current; + const registry = metadataStoreNow && decisionOutputsReadRegistry(outputDefs) + ? await readDecisionOutputRegistry(outputDefs, metadataStoreNow) + : undefined; const outputParams = decision - ? decisionOutputParams(outputDefs, t, { - decision, - registry: await readDecisionOutputRegistry(outputDefs, metadataStoreRef.current), - }) + ? decisionOutputParams(outputDefs, t, { decision, registry }) : []; // Typed as the SEAM's contract, not as `any` and not cast to `ActionDef` // on the way out (objectui#5611). `overrideNotice` below is host-composed diff --git a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx index 7b7c9ce7c9..f8d664e0b5 100644 --- a/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx +++ b/packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx @@ -427,6 +427,32 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => { expect(params[0].reference).toBeUndefined(); }); + it('dispatches in the click\'s own tick when no output lists the registry (objectui#7611)', () => { + // Only a decision with a catalog output waits for the registry read; every + // other decision dispatches synchronously, as it did before — a store is + // mounted here, so it is the OUTPUTS that decide, not the store's absence. + const ensureType = vi.fn(async () => []); + const store = { ensureType } as unknown as React.ContextType; + render( + + + , + ); + + fireEvent.click(screen.getByText('Approve')); + + expect(executeSpy).toHaveBeenCalledTimes(1); + expect(ensureType).not.toHaveBeenCalled(); + }); + it('localizes the decision-output help text', async () => { const decideAction = { name: 'approval_approve',