From d7af5241f230a87d2bda0b2cfe33c03acea855da Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:00:42 +0000 Subject: [PATCH 1/2] test(app-shell): pin ActionParam to one authority before converging the shadows Written first and observed RED against the unconverged tree: the census names ActionDefaultInspector.tsx:266 and ActionPreview.tsx:47, and the import half names both files. The census is the reverse-verified half on purpose. A module-local declaration is invisible from outside its module (objectui#5899), which is also why scripts/__tests__/one-authority-per-exported-name-6273.test.ts is green on all three sites: its matcher requires `export`. The key-set, strict-schema and I18nLabel assertions are direction guards on the published authority, which this card does not change and which therefore cannot fail before it. Refs #6329 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .../ActionParam.one-authority.test.ts | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 packages/app-shell/src/views/metadata-admin/ActionParam.one-authority.test.ts diff --git a/packages/app-shell/src/views/metadata-admin/ActionParam.one-authority.test.ts b/packages/app-shell/src/views/metadata-admin/ActionParam.one-authority.test.ts new file mode 100644 index 000000000..cbdab5932 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ActionParam.one-authority.test.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * **`ActionParam` has one authority, and app-shell does not redeclare it** + * (objectui#6329). + * + * The name was declared THREE times, not twice as the card first counted: + * + * 1. `packages/types/src/ui-action.ts` — `export interface ActionParam`, + * derived from the spec's `ActionParamSchema` input and re-exported from + * the package barrel. This is the authority, and it already carries its + * own parity suite (`packages/types/src/__tests__/spec-derived-unions.test.ts` + * and `page-nav-misc-spec-parity.test.ts`). + * 2. `views/metadata-admin/inspectors/ActionDefaultInspector.tsx` — a + * module-local `interface`, seven members plus `[k: string]: unknown`. + * 3. `views/metadata-admin/previews/ActionPreview.tsx` — a module-local + * `interface`, ten members, no index signature. + * + * app-shell already imports the published name correctly elsewhere + * (`src/utils/resolveActionParams.test.ts`), so 2 and 3 were shadows of a name + * their own package reads by reference. Under the 2026-08-25 family ruling + * 甲A1 — every exported name has exactly one authority — they are deleted, not + * reconciled against each other. + * + * ## Why the census, and not a key-set assertion + * + * The obvious pin — "the type accepts exactly these keys" — is a BLIND + * INSTRUMENT against declaration 2. `[k: string]: unknown` makes every string + * a member, so `keyof` on that type is `string` and a key-set comparison + * cannot fail whatever the file does. That is the same trap + * `FlowNodeInspector.specKeys.test.tsx` records for `FlowNodeLike`. + * + * Here the index signature GOES AWAY in the convergence rather than being + * worked around, so the key-set half becomes live — but only against the + * published type, which is unchanged by this card and therefore cannot fail + * before it. The half that can fail before and pass after is the CENSUS: a + * module-local declaration is invisible from outside its module (that is the + * instrument hole objectui#5899 is about, and the reason + * `scripts/__tests__/one-authority-per-exported-name-6273.test.ts` — whose + * matcher requires `export` — is green on all three sites). Only source can + * see it. So the census is the reverse-verified pin; the key-set, schema and + * `I18nLabel` assertions below are DIRECTION guards, pinning that the + * authority still holds every member the two shadows carried. + * + * ## Why an AST and not a grep + * + * This very file names `interface ActionParam` in prose, and the two converged + * files each keep a comment saying why their local copy is gone. A text scan + * reports violations that do not exist; the compiler sees a comment. The + * comment-blindness control below proves the discriminator rather than + * asserting it. + */ + +import { describe, it, expect } from 'vitest'; +import ts from 'typescript'; +import { readFileSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { ActionParamSchema } from '@objectstack/spec/ui'; +import type { ActionParam } from '@object-ui/types'; + +const THIS_DIR = path.dirname(fileURLToPath(import.meta.url)); +/** `packages/app-shell/src` — this file sits at `src/views/metadata-admin/`. */ +const APP_SHELL_SRC = path.resolve(THIS_DIR, '../..'); +const REPO_ROOT = path.resolve(APP_SHELL_SRC, '../../..'); +const PUBLISHED_AUTHORITY = path.join(REPO_ROOT, 'packages/types/src/ui-action.ts'); + +const NAME = 'ActionParam'; + +/** + * Tests declare throwaway shapes and quote real declarations as fixture text + * on purpose — this file does both. They are out of the population; they stay + * fair game as matcher fixtures. + */ +const NOT_POPULATION = /(?:^|[\\/])__tests__[\\/]|\.(?:test|spec|stories)\.[cm]?tsx?$/; +const SOURCE_SUFFIX = /\.(?:[cm]?ts|tsx)$/; + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) walk(full, out); + else if (SOURCE_SUFFIX.test(entry.name) && !NOT_POPULATION.test(full)) out.push(full); + } + return out; +} + +interface Site { + readonly rel: string; + readonly line: number; + readonly what: string; +} + +const parse = (file: string, source: string): ts.SourceFile => + ts.createSourceFile(file, source, ts.ScriptTarget.Latest, /* setParentNodes */ true, ts.ScriptKind.TSX); + +/** + * Every site in `source` that DECLARES the name — `interface X`, `type X`, + * `enum X`, exported or not. Import and export clauses are not declarations + * and never match: they are how one authority reaches many files. + */ +function declarationSites(source: string, rel = ''): Site[] { + const sourceFile = parse(rel, source); + const sites: Site[] = []; + const visit = (node: ts.Node): void => { + const isDeclaration = + (ts.isInterfaceDeclaration(node) || ts.isTypeAliasDeclaration(node) || ts.isEnumDeclaration(node)) && + node.name.text === NAME; + if (isDeclaration) { + const kind = ts.isInterfaceDeclaration(node) + ? 'interface' + : ts.isTypeAliasDeclaration(node) + ? 'type' + : 'enum'; + sites.push({ + rel, + line: sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1, + what: `${kind} ${NAME}`, + }); + } + ts.forEachChild(node, visit); + }; + ts.forEachChild(sourceFile, visit); + return sites; +} + +/** Files that name `ActionParam` at all — the prefilter the AST walk runs on. */ +function mentioningFiles(): { file: string; rel: string; source: string }[] { + return walk(APP_SHELL_SRC) + .map((file) => ({ file, rel: path.relative(REPO_ROOT, file), source: readFileSync(file, 'utf8') })) + .filter(({ source }) => source.includes(NAME)) + .sort((a, b) => (a.rel < b.rel ? -1 : 1)); +} + +/** Type-only or value imports of `ActionParam` from `@object-ui/types`. */ +function importsFromTypes(source: string, rel: string): boolean { + const sourceFile = parse(rel, source); + let found = false; + ts.forEachChild(sourceFile, (node) => { + if (!ts.isImportDeclaration(node)) return; + if (!ts.isStringLiteral(node.moduleSpecifier) || node.moduleSpecifier.text !== '@object-ui/types') return; + const bindings = node.importClause?.namedBindings; + if (bindings === undefined || !ts.isNamedImports(bindings)) return; + for (const element of bindings.elements) { + if (element.name.text === NAME) found = true; + } + }); + return found; +} + +const CONVERGED = [ + 'packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx', + 'packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx', +]; + +describe('ActionParam — one authority (objectui#6329)', () => { + // ── The census: the half that fails before the convergence ──────────────── + + it('no file under app-shell/src declares ActionParam locally', () => { + const sites = mentioningFiles().flatMap(({ rel, source }) => declarationSites(source, rel)); + expect( + sites.map((s) => `${s.rel}:${s.line} — ${s.what}`), + 'app-shell must READ the published `ActionParam` from `@object-ui/types`, never redeclare it (objectui#6329)', + ).toEqual([]); + }); + + it('both converged files import ActionParam from @object-ui/types', () => { + const missing = CONVERGED.filter((rel) => { + const source = readFileSync(path.join(REPO_ROOT, rel), 'utf8'); + return !importsFromTypes(source, rel); + }); + expect(missing, 'deleting the local copy is only half the fix — the published name has to arrive').toEqual([]); + }); + + // ── Controls on the instrument itself ───────────────────────────────────── + + it('finds the one declaration that SHOULD exist (non-vacuity)', () => { + const sites = declarationSites(readFileSync(PUBLISHED_AUTHORITY, 'utf8'), 'packages/types/src/ui-action.ts'); + expect(sites.map((s) => s.what)).toEqual([`interface ${NAME}`]); + }); + + it('is blind to prose and to re-exports, and not to declarations', () => { + const quoted = [ + '/**', ' * The local copy is gone:', ' * interface ActionParam { name?: string }', ' */', + "import type { ActionParam } from '@object-ui/types';", + "export type { ActionParam } from '@object-ui/types';", + "const s = 'interface ActionParam {}';", + ].join('\n'); + expect(declarationSites(quoted)).toEqual([]); + expect(declarationSites('interface ActionParam { name?: string }').map((s) => s.what)).toEqual([ + `interface ${NAME}`, + ]); + expect(declarationSites('type ActionParam = { name?: string }').map((s) => s.what)).toEqual([`type ${NAME}`]); + }); + + it('the prefilter is not hiding the population', () => { + const rels = mentioningFiles().map((f) => f.rel); + for (const rel of CONVERGED) expect(rels).toContain(rel); + }); + + // ── Direction guards on the surviving authority ─────────────────────────── + + it('carries every member the two deleted shadows declared, with no index signature', () => { + // `[k: string]: unknown` would make `string extends keyof ActionParam` + // true, and every key-set assertion below vacuous. This is the blind + // instrument the inspector's copy was, asserted away. + const noIndexSignature: string extends keyof ActionParam ? false : true = true; + expect(noIndexSignature).toBe(true); + + type ShadowMembers = + | 'name' | 'field' | 'label' | 'type' | 'required' + | 'options' | 'placeholder' | 'helpText' | 'defaultValue' | 'defaultFromRow'; + type Missing = Exclude; + const noMemberLost: [Missing] extends [never] ? true : false = true; + expect(noMemberLost).toBe(true); + }); + + it('refuses a key the inspector shadow admitted through its index signature', () => { + // `referenceTo` is the resolved-side spelling `ui-action.ts` documents as + // the silent authoring error: `[k: string]: unknown` typed it `unknown` + // and let it through, while the strict schema rejects it BY NAME. + // @ts-expect-error — not an authorable `ActionParam` key + const refused: ActionParam = { name: 'account_id', referenceTo: 'account' }; + expect(refused.name).toBe('account_id'); + + const parsed = ActionParamSchema.safeParse({ name: 'account_id', referenceTo: 'account' }); + expect(parsed.success).toBe(false); + expect(JSON.stringify(parsed.error?.issues ?? [])).toContain('referenceTo'); + }); + + it('what the shadows described still parses clean through the real strict schema', () => { + const param = { + name: 'reason', + field: 'reason', + label: 'Reason', + type: 'select', + required: true, + options: [{ label: 'Duplicate', value: 'dup' }], + placeholder: 'Why?', + helpText: 'Shown under the input.', + defaultValue: 'dup', + defaultFromRow: false, + } satisfies ActionParam; + const parsed = ActionParamSchema.safeParse(param); + expect(JSON.stringify(parsed.error?.issues ?? [])).toBe('[]'); + expect(parsed.success).toBe(true); + }); + + it('accepts BOTH authorized I18nLabel forms — the preview shadow admitted only `en`', () => { + // The preview's copy said `string | { en?: string }`. Its own `localize` + // helper has always read `Object.values(o)[0]`, so an inline locale map + // keyed by any tag already rendered; only the declaration was narrower + // than the code. Converging widens the TYPE onto what the runtime does, + // it does not widen what the runtime accepts. + const plain = { name: 'p', label: 'Reason' } satisfies ActionParam; + const localeMap = { name: 'p', label: { en: 'Reason', 'fr-FR': 'Motif' } } satisfies ActionParam; + for (const param of [plain, localeMap]) { + expect(ActionParamSchema.safeParse(param).success).toBe(true); + } + }); +}); From e660fed22f05b8da3eb4847b7f0da228c941c6d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:16:22 +0000 Subject: [PATCH 2/2] refactor(app-shell): converge both ActionParam shadows onto the published type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActionParam` was declared three times, not twice: `@object-ui/types` publishes it (derived from the spec's `ActionParamSchema` input, with its own parity suite), and ActionDefaultInspector.tsx and ActionPreview.tsx each carried a module-local `interface` of the same name. app-shell already read the published one elsewhere, so both locals were shadows. Deleted, not reconciled against each other (family ruling 甲A1, 2026-08-25). Neither shadow needed a member the published type lacks, so the published surface is untouched. What they got wrong was the declaration: - the inspector's `[k: string]: unknown` typed every key `unknown`, so a commit of a key `ActionParamSchema` rejects BY NAME type-checked and failed on save — and it made the two copies look compatible while they described different authoring surfaces; - the preview's `label?: string | { en?: string }` admitted the `en` tag and no other, while its own `localize` has always read `Object.values(o)[0]`. Two consequences of withdrawing the local `type?: string` in favour of `ResolvableParamFieldType`: `renderFieldMock` loses its `long_text` / `integer` branches (both belong to other vocabularies — the console form-builder dialect and JSON Schema — so a param spelled either way is a parse rejection and could never reach the preview), and the inspector narrows its dropdown commit through the runtime witnesses `@object-ui/types` exports rather than writing the raw DOM string. Fixes #6329 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .changeset/6329-actionparam-one-authority.md | 42 ++++++++++++ .../inspectors/ActionDefaultInspector.tsx | 65 +++++++++++++++---- .../metadata-admin/previews/ActionPreview.tsx | 39 +++++++---- 3 files changed, 120 insertions(+), 26 deletions(-) create mode 100644 .changeset/6329-actionparam-one-authority.md diff --git a/.changeset/6329-actionparam-one-authority.md b/.changeset/6329-actionparam-one-authority.md new file mode 100644 index 000000000..ac3ed137e --- /dev/null +++ b/.changeset/6329-actionparam-one-authority.md @@ -0,0 +1,42 @@ +--- +'@object-ui/app-shell': patch +--- + +`ActionParam` has one authority again (objectui#6329). The name was declared three times, +not twice as the card counted: `@object-ui/types` publishes it — derived from the spec's +`ActionParamSchema` input, with its own parity suite — and `ActionDefaultInspector.tsx` and +`ActionPreview.tsx` each carried a module-local `interface` of the same name. app-shell +already read the published one elsewhere (`utils/resolveActionParams.test.ts`), so both +locals were shadows. They are deleted, not reconciled against each other, under the +2026-08-25 family ruling 甲A1. + +Neither shadow needed a member the published type lacks, so nothing was added to the +published surface. What the shadows got wrong was the DECLARATION, in the direction that +lets wrong metadata compile: + +- The inspector's copy carried `[k: string]: unknown`. An index signature admits every key + at type `unknown`, so a commit of a key `ActionParamSchema` rejects by name — `.strict()`, + and `referenceTo` is listed in its alias map — type-checked here and failed on save. It + also made the two copies look compatible when they were describing different authoring + surfaces: `options` / `helpText` / `defaultValue` were declared outright on one side and + swallowed as `unknown` on the other. +- The preview's copy declared `label?: string | { en?: string }`, admitting the `en` tag and + no other, while its own `localize` helper has always read `Object.values(o)[0]`. An inline + locale map keyed `fr-FR` rendered correctly and failed `tsc`. The published `I18nLabel` + admits both authorized forms, so the type now matches what the code already did — this + widens the declaration, not the runtime's acceptance. + +Two behaviour-visible consequences, both of them the local `type?: string` being withdrawn +in favour of the published `ResolvableParamFieldType` (the spec's 49-member `FieldType` plus +objectui's three declared param aliases): + +- `ActionPreview.renderFieldMock` no longer branches on `long_text` or `integer`. Neither is + in that vocabulary — `long_text` belongs to the console form-builder dialect and `integer` + to JSON Schema — so a param spelled either way is a parse rejection on the server and + could never have reached the preview. The two comparisons compiled only because the local + copy typed `type` as `string`. +- The inspector's param-type dropdown narrows its commit through the runtime witnesses + `@object-ui/types` exports (`ACTION_PARAM_FIELD_TYPES` + `OBJECTUI_LOCAL_PARAM_FIELD_TYPES`) + rather than writing the raw DOM string. An unrecognised spelling clears the key instead of + being written into metadata the server would refuse; the eight offered spellings are + unaffected, and are now checked against the vocabulary at compile time. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx index 76876afc4..09e4f1715 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx @@ -31,6 +31,12 @@ import * as React from 'react'; import { Plus, Trash2 } from 'lucide-react'; import type { ActionLocation } from '@objectstack/spec/ui'; +import { + ACTION_PARAM_FIELD_TYPES, + OBJECTUI_LOCAL_PARAM_FIELD_TYPES, + type ActionParam, + type ResolvableParamFieldType, +} from '@object-ui/types'; import { Button, Label, Textarea, } from '@object-ui/components'; @@ -98,6 +104,14 @@ const BODY_LANG_OPTS = [ { value: 'js', label: 'Sandboxed JS (L2)' }, ]; +/* + * `satisfies`, not a bare literal: every spelling this dropdown offers must be + * one the published `ActionParam.type` admits. The eight below are spec + * `FieldType` members, and the check is what stops a ninth from being added in + * a dialect the server's `.strict()` `ActionParamSchema` would reject on save — + * the way `long_text` reached `ActionPreview` while the local `type?: string` + * was still in force (objectui#6329). + */ const PARAM_TYPE_OPTS = [ { value: 'text', label: 'Text' }, { value: 'textarea', label: 'Long text' }, @@ -107,7 +121,30 @@ const PARAM_TYPE_OPTS = [ { value: 'date', label: 'Date' }, { value: 'datetime', label: 'Date/time' }, { value: 'lookup', label: 'Lookup' }, -]; +] satisfies { value: ResolvableParamFieldType; label: string }[]; + +/** + * Every `type` spelling an authored param may carry, as a runtime set — the + * spec's `FieldType` members plus objectui's three declared param aliases, + * both taken BY REFERENCE from the witnesses `@object-ui/types` exports for + * exactly this (a hand-listed copy is what the drift guard in that package + * fails on). + */ +const RESOLVABLE_PARAM_TYPES: ReadonlySet = new Set([ + ...ACTION_PARAM_FIELD_TYPES, + ...OBJECTUI_LOCAL_PARAM_FIELD_TYPES, +]); + +/** + * Narrow a dropdown commit — a DOM string — onto the published vocabulary. + * + * Returns `undefined` rather than coercing, so an unrecognised spelling clears + * the key instead of being written into metadata the server would refuse. This + * is a boundary check, not a lenient fallback: nothing off-spec gets accepted. + */ +function asParamFieldType(value: string): ResolvableParamFieldType | undefined { + return RESOLVABLE_PARAM_TYPES.has(value) ? (value as ResolvableParamFieldType) : undefined; +} /** * Friendly labels for the spec's action locations. @@ -263,16 +300,20 @@ function FieldPicker({ label, objectName, value, onCommit, disabled }: { return ; } -interface ActionParam { - name?: string; - field?: string; - label?: unknown; - type?: string; - required?: boolean; - placeholder?: string; - defaultFromRow?: boolean; - [k: string]: unknown; -} +/* + * No local `ActionParam` here (objectui#6329). `@object-ui/types` publishes the + * authoring shape, derived from the spec's `ActionParamSchema` input; this + * panel WRITES that shape, so it reads the authority by reference. + * + * The copy that used to sit here carried `[k: string]: unknown`, which is the + * member that mattered: an index signature admits every key at type `unknown`, + * so `patchParam(i, { referenceTo: 'account' })` type-checked while + * `ActionParamSchema` — `.strict()` — rejects that key BY NAME on save. It is + * the same defect `FlowNodeInspector.specKeys.test.tsx` records for + * `description`, and it also made the copy look compatible with the preview's + * (which declared `options` / `helpText` / `defaultValue` outright) when the + * two were simply describing different authoring surfaces. + */ /* ─────────────── inspector ─────────────── */ @@ -453,7 +494,7 @@ export function ActionDefaultInspector({ )} patchParam(i, { label: v })} disabled={readOnly} /> {!p.field && ( - patchParam(i, { type: v })} disabled={readOnly} /> + patchParam(i, { type: asParamFieldType(v) })} disabled={readOnly} /> )} patchParam(i, { placeholder: v })} disabled={readOnly} />
diff --git a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx index cbbd6f400..614802f36 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx @@ -41,21 +41,23 @@ import { Workflow, icons as lucideIcons, } from 'lucide-react'; +import type { ActionParam } from '@object-ui/types'; import type { MetadataPreviewProps } from '../preview-registry.js'; import { PreviewShell, PreviewMessage, PreviewErrorBoundary } from './PreviewShell.js'; -interface ActionParam { - name?: string; - field?: string; - label?: string | { en?: string }; - type?: string; - required?: boolean; - options?: Array<{ label: string | { en?: string }; value: string }>; - placeholder?: string; - helpText?: string; - defaultValue?: unknown; - defaultFromRow?: boolean; -} +/* + * No local `ActionParam` here (objectui#6329). `@object-ui/types` publishes the + * authoring shape, derived from the spec's `ActionParamSchema` input, and this + * package already read it by reference elsewhere. The copy that used to sit + * here restated ten of its members and got two of them wrong in a way that + * only ever narrowed the DECLARATION, never the code: + * + * - `label?: string | { en?: string }` admitted the `en` tag and no other, + * while `localize` below has always read `Object.values(o)[0]`. An inline + * locale map keyed `fr-FR` rendered fine and failed `tsc`. + * - `type?: string` admitted every string, which is how the two dead + * branches in `renderFieldMock` survived — see the note there. + */ interface ResultDialogField { path: string; @@ -389,12 +391,21 @@ function renderFieldMock(p: ActionParam): React.ReactElement { ); } - if (p.type === 'textarea' || p.type === 'html' || p.type === 'long_text') { + // No `long_text` / `integer` branches (objectui#6329). Both are spellings + // from OTHER vocabularies — `long_text` from the console's form-builder + // dialect (`apps/console/src/components/FormPage.tsx`), `integer` from JSON + // Schema (`ToolPreview.tsx`, `json-schema-to-fields.ts`) — and neither is in + // `ResolvableParamFieldType`, which is the spec's 49-member `FieldType` plus + // objectui's three declared param aliases. `ActionParamSchema` is `.strict()` + // with a `FieldType` enum on `type`, so a param spelled either way is a parse + // rejection on the server and can never reach this preview. The local + // `type?: string` was the only thing that made the comparisons compile. + if (p.type === 'textarea' || p.type === 'html') { return