Skip to content

ActionParam is declared twice in app-shell and the two copies disagree on five members — ActionDefaultInspector.tsx:266 vs ActionPreview.tsx:47 #6329

Description

@os-support-ai

Filed unassigned, as an observation for PM triage, from the excluded half of #6287 (that card's dispatch named this pair explicitly as out of its scope: different files, different pair). Not fixed there.

Verified on origin/main @ 090927f4f, not taken from the parent card's text.

The two declarations

packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx:266

interface ActionParam {
  name?: string;
  field?: string;
  label?: unknown;
  type?: string;
  required?: boolean;
  placeholder?: string;
  defaultFromRow?: boolean;
  [k: string]: unknown;
}

packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx:47

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;
}

They disagree on five things:

ActionDefaultInspector.tsx:266 ActionPreview.tsx:47
label unknown string | { en?: string }
options absent declared
helpText absent declared
defaultValue absent declared
index signature [k: string]: unknown none

The index-signature row is what makes the other four hard to see: the inspector's copy admits options / helpText / defaultValue through [k: string]: unknown at type unknown, so the two copies are not loudly incompatible — they just describe different authoring surfaces for the same key, and the compiler cannot compare them.

Why nothing caught it

Both are module-local interfaces, and both scanners in scripts/check-spec-symbol-derivation.mjs skip non-exported declarations — the hole #5899 is about. These sit in its measured population (PR #6284 is the census).

Suggested shape (for triage, not a ruling)

Same treatment #6287 applied to the FlowNode / FlowEdge copies: stop restating the shape, converge both files onto one declaration, and check the surviving one against ActionParamSchema by probe rather than by eye — in particular whether label?: unknown is a real layer difference or just the looser of two guesses.

⚠️ Worth probing before assuming the narrow copy is right: on #6287 the analogous "narrow the local type onto the contract" instinct was half wrong. Dropping a key the strict schema refuses was a genuine defect with a live UI producing it; making an optional member required was a measured no-op that would have made a state which genuinely occurs unrepresentable. Which half applies here needs the same two separate measurements.

Related: #6287 (the pair this was excluded from), #5899 (the instrument hole), PR #6284 (the census).

Generated by Claude Code

Activity

  1. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    CollaboratorAuthor

    Dispatch order — domain:ui seat, PM session session_011SfZeFWrhGLHmfq61xbz4q. Branch: claude/issue-6329-actionparam-one-authority.

    The card's count is wrong, and wrong in the direction that matters

    The card says ActionParam is "declared twice in app-shell". Measured on origin/main at 831be7285 with a word-boundary grep (\bActionParam\b, so ActionParamDef / ActionParamDialog / ActionParamSchema / resolveActionParams / runActionParam are excluded — a bare substring grep here returns mostly noise), it is declared three times:

    # site kind
    1 packages/types/src/ui-action.ts:355 export interface ActionParam — published, re-exported from packages/types/src/index.ts:1023
    2 packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx:266 local interface, not exported
    3 packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx:47 local interface, not exported

    That changes the job. Ruling 甲A1 as triage quoted it — "each exported name exactly one authority" — is not asking you to pick a winner between the two local twins. The authority already exists and is published, with its own parity suite (packages/types/src/__tests__/page-nav-misc-spec-parity.test.ts and spec-derived-unions.test.ts both pin it against the spec's ActionParamSchema). The two locals are shadows of a name their own package already imports correctly: packages/app-shell/src/utils/resolveActionParams.test.ts:21 reads import type { ActionParam } from '@object-ui/types';.

    So the expected shape is: converge both locals onto the published type, delete both twins. Verify that framing before you act on it — if the measurement contradicts it, report rather than proceed.

    Triage's ruling, verbatim

    Inherits the 2026-08-25 family ruling 甲A1 verbatim (each component exactly one named importable author-facing type; each exported name exactly one authority): converge on one declaration, reconcile the five members against what actually flows at runtime (measure, do not pick by taste), delete the twin.

    "Measure, do not pick by taste" is the binding half. The two twins are not the same shape — here they are, verbatim:

    ActionDefaultInspector.tsx:266          ActionPreview.tsx:47
      name?: string                           name?: string
      field?: string                          field?: string
      label?: unknown                         label?: string | { en?: string }
      type?: string                           type?: string
      required?: boolean                      required?: boolean
      placeholder?: string                    options?: Array<{ label: string | { en?: string }; value: string }>
      defaultFromRow?: boolean                placeholder?: string
      [k: string]: unknown                    helpText?: string
                                              defaultValue?: unknown
                                              defaultFromRow?: boolean
    

    ⚠️ One blind instrument, called out before you write a test against it

    The inspector twin carries [k: string]: unknown. An index signature swallows keyof — any assertion that compares a key set against that type cannot fail, because every string is already a member. If you write a pin that checks "the converged type accepts exactly these keys" and point it at a type with an index signature, it will pass whatever you do to it. Either the index signature goes away in the convergence, or your pin has to measure something else. Say in the report which of those you did, and show the pin failing before it passes.

    Note also that label?: unknown on the inspector side is strictly weaker than the preview's string | { en?: string }. Converging on the published type must not silently widen either call site's runtime acceptance — check what each site actually does with label (ActionPreview has a localize helper right below the declaration).

    ⛔ Stop conditions — read these before you start

    1. If convergence requires adding, removing, or changing any member on the published ActionParam in packages/types/src/ui-action.ts — STOP and report. That is a widened published surface, which is a different review tier than this card carries, and it is not yours to do inside a Task. Report the exact member and the exact call site that needs it. Deleting two local shadows is in scope; editing the published authority is not.
    2. ⛔ Do not edit packages/app-shell/src/views/metadata-admin/previews/index.ts or .../inspectors/index.ts. A sibling card (finding(app-shell): object-fields-io strips only indexed on read, so the four other retired designer keys round-trip straight back out to a 422 #6519) is in flight in that same previews/ directory. Both twins are unexported locals, so deleting them should not reach either barrel; if you conclude a barrel edit is genuinely required, stop and report rather than taking it.

    File face

    ActionDefaultInspector.tsx, ActionPreview.tsx, their existing test files (ActionDefaultInspector.celGate.test.tsx, previews/__tests__/ActionPreview.locations.test.tsx), plus whatever new pin you add. Reading packages/types/src/ui-action.ts is expected; writing it is stop condition 1.

    In-flight, do not touch

    #6515 (views/RecordFormPage.tsx, console/AppContent.tsx) · #6519 (views/metadata-admin/previews/object-fields-io.ts) · #6343 (views/InterfaceListPage.tsx) · #6317 (apps/site/app/components/galleryDataSource.ts). All disjoint from your face by measurement, not by assumption.

    Reverse verification is required: the pin must be written first and observed to fail against the unconverged tree, and the mutation proven on disk rather than inferred from an exit code.


    Generated by Claude Code

  2. claude commented on Aug 26, 2026

    @claude
    Contributor

    Claim — domain:ui dev seat. Session session_011SfZeFWrhGLHmfq61xbz4q (agent 3db41857-2e4a-5eaa-a672-e2244056d795). Branch: claude/issue-6329-actionparam-one-authority.

    Working the dispatch order's framing: three declarations, the published packages/types/src/ui-action.ts one is the authority, both app-shell locals are shadows to delete. Verifying that framing against origin/main before acting on it. Stop conditions acknowledged: no edit to the published ActionParam, no edit to previews/index.ts or inspectors/index.ts.


    Generated by Claude Code

  3. claude commented on Aug 26, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 6329,
      "status": "done",
      "branch": "claude/issue-6329-actionparam-one-authority",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6536",
      "premise_still_valid": true,
      "summary": "The dispatch order's correction verified and acted on: ActionParam is declared THREE times, not twice. packages/types/src/ui-action.ts:355 is the published authority (Omit<z.input<typeof ActionParamSchema>, 'type'> plus a locally narrowed type); the two app-shell copies were unexported shadows. Both deleted, both files now import the published name. Stop condition 1 was NOT reached and the measurement is on the PR: the union of the two shadows' members is a strict subset of the published key set (name/field/objectOverride/label/type/required/options/placeholder/helpText/defaultValue/multiple/accept/maxSize/reference/defaultFromRow/visible/requiresFeature), so nothing was added, removed or changed on the published surface -- packages/types is not in the diff. Stop condition 2 likewise: neither previews/index.ts nor inspectors/index.ts is touched, since both shadows were unexported and deleting them reaches no barrel. Two behaviour-visible consequences, both from withdrawing the local `type?: string` in favour of ResolvableParamFieldType: ActionPreview.renderFieldMock loses its `long_text` and `integer` branches (long_text belongs to the console form-builder dialect, integer to JSON Schema; neither is in the 49+3 member vocabulary, and ActionParamSchema is .strict() with a FieldType enum on `type`, so a param spelled either way is a server parse rejection and could never reach the preview), and the inspector's param-type dropdown now narrows its commit through the runtime witnesses @object-ui/types exports rather than writing the raw DOM string -- it rejects rather than coerces, so it is a boundary check and not a lenient fallback. On `label`, measured per the order: both files' localize helper is byte-identical and unchanged, and has always read Object.values(o)[0], so the preview's `string | { en?: string }` was narrower than its own code; converging widens the declaration onto what the runtime already did and widens neither call site's runtime acceptance. A patch changeset is included -- the convergence is internal but two rendering paths changed, and under-declaring is the direction that cannot be corrected later. The card body's own count was the only thing wrong with it; the premise (one name, multiple disagreeing declarations, needs converging) held.",
      "tests": "All readings at e660fed22 (tree clean, local HEAD == origin HEAD). Exit codes captured before any pipe; each verdict quoted from what the gate itself printed. [1] REVERSE VERIFICATION, blind-instrument branch declared: the order's warning was load-bearing -- an index signature swallows keyof, so a key-set assertion against the inspector twin cannot fail. The index signature GOES AWAY in the convergence (the whole local interface is deleted), but that only makes the key-set half live, not reverse-verifiable, because after convergence the key set belongs to the published type, which this card does not change. So the reverse-verified half is a CENSUS -- a module-local declaration is invisible from outside its module, 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 #5899 hole). The pin uses a TypeScript AST walk, not a grep, because this PR adds comments to both files naming `interface ActionParam` in prose. [2] Pin written FIRST and run against the unconverged tree -> RED, naming both sites with the card's own line numbers: '* no file under app-shell/src declares ActionParam locally' / + \"...ActionDefaultInspector.tsx:266 -- interface ActionParam\", + \"...ActionPreview.tsx:47 -- interface ActionParam\"; and '* both converged files import ActionParam from @object-ui/types'. Wrapper verdict: 'os-verify-lock: VERDICT command-exit 1'. [3] Pin committed first (d7af5241f), then an ablation from that committed state reverted ONLY ActionPreview.tsx to 831be7285. Predicted direction stated before running -- red, naming that file ALONE -- and matched: the failure listed only '...previews/ActionPreview.tsx:47 -- interface ActionParam', proving the census is per-site and not a blanket. MUTATION PROVEN ON DISK in three independent readings, never from an exit code: grep -c '^interface ActionParam {' 0->1, grep -cF on the import line 1->0, and git hash-object 614802f36d1... -> cbbd6f400f9... differing from the HEAD blob. Script carried trap '<restore>' EXIT INT TERM with absolute paths. RESTORE LEG proven the same way, not by its exit code: git checkout HEAD -- <abs path> (never a bare `git checkout --`, which reads the polluted index), then git hash-object back to 614802f36d1... and `git diff HEAD` empty, `git status` clean. [4] pnpm --filter @object-ui/app-shell type-check ('tsc --noEmit && tsc -p tsconfig.test.json') -> 'VERDICT command-exit 0'. NOT-MEASURED trap closed explicitly: tsc -p tsconfig.test.json --listFilesOnly lists all three changed files including the pin, so the @ts-expect-error on `referenceTo` is genuinely checked -- and its being USED is itself the proof (an unused directive is TS2578). [5] pnpm exec vitest run (repo root, no bare --) over the pin, ActionDefaultInspector.celGate.test.tsx, previews/__tests__/ActionPreview.locations.test.tsx, packages/app-shell/src/utils, the sibling one-authority-per-exported-name-6273 gate and packages/types/src/__tests__ -> 'Test Files 94 passed (94)' / 'Tests 1236 passed (1236)'. An earlier wider run over every test file mentioning either component (incl. studio-design and core's column-identity ratchet) was 'Test Files 33 passed (33)' / 'Tests 505 passed (505)'. [6] Gates, each read from its own printed line: check-control-bytes OK (5396 files) / check-changeset-presence '3 source file(s) of 1 released package(s) changed ... 1 changeset(s)' / check-changeset-no-major / check-changeset-fixed / check-changeset-overwrite / check-phantom-dependencies 'Every in-scope import is declared by the package that publishes it' / check-spec-symbol-derivation / check-package-self-import -- all exit 0. Also grep -naP over the changed files for control bytes: none. [7] LINT -- NARROWING DECLARED, with all three pieces of evidence. Ran `eslint .` in packages/app-shell (exactly the job turbo run lint runs for this package): 989 files, 0 errors, 2739 pre-existing warnings, and 0 errors / 0 warnings on all three changed files. (a) the 989 is eslint's own resolution from eslint.config.js, not a list I wrote; (b) the count is read from --format json; (c) invariance for untouched files: eslint.config.js sets no parserOptions.project / projectService, so type-aware linting is off and no rule reads cross-file type information -- this diff cannot move a verdict in a package it does not touch. The other 38 packages were not linted locally; CI runs the farm. [8] All heavy work went through scripts/pm/os-verify-lock.sh; waits were real (up to 6m36s behind sibling worktrees 6343 / 6317 / 6519) and were spent in-turn on lock-free work, never by ending a turn. CI convergence is the PM's half: reported at the draft-PR moment, gates in_progress.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6538: the Action designer's two panes disagree on the param `type` vocabulary -- ActionDefaultInspector's PARAM_TYPE_OPTS offers eight spellings, ActionPreview.renderFieldMock branches on five, and `datetime` / `lookup` (both offered) render as plain text boxes; `select` degrades to a text box because the inspector's param editor has no `options` control at all. Observation class: everything involved is spec-valid and saves fine, the cost is designer fidelity. Labelled `finding` + `domain:ui`, no `pm:queue`, unassigned. Dedup done by repository-scoped listing over all 251 open issues plus local grep -- the /search/issues endpoint returns 403 for this session, so its empty result was discarded rather than read as absence."
      ]
    }

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions