Skip to content

fix(spec): hook condition row declares expression, not javascript - #20475

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20439-hook-condition-expression-row
Sep 28, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20439-hook-condition-expression-row

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20439

Clause-②: no

What

packages/spec/src/data/hook.form.ts's condition row declared language: 'javascript' over HookSchema.condition, which is EvaluatedExpressionInputSchema — a CEL predicate, not a script (hook.zod.ts: 'Predicate (CEL); hook runs only when TRUE …'). Every sibling predicate row (field.form.ts / object.form.ts's visibleWhen / readonlyWhen / requiredWhen, and the formula expression row) already declares language: 'expression'. A consumer keyed on the row's declared language (objectui#10963's CodeWidget fix) could not tell this row apart from a real script row (body.source, action.source, both genuinely 'javascript').

  • condition row now declares language: 'expression', helpText: 'CEL predicate — the hook runs only when TRUE' (triage's wording, matching the sibling rows' phrasing).
  • Pinned in packages/spec/src/system/metadata-form-declared-rows.pin.test.ts (no existing row-language assertion was found, so the pin was added beside that file's existing form-row pins): asserts hook.condition declares type: 'code' / language: 'expression', with a control against field.visibleWhen (a sibling predicate row already correct) so the assertion is shown to actually discriminate.
  • en.metadata-forms.generated.ts regenerated (node scripts/check-i18n-bundles.mjs --write) to pick up the new helpText. zh-CN / ja-JP / es-ES kept their existing translated values under the tool's merge behaviour (now stale relative to the new English source — the tool does not auto-translate).
  • Changeset: @objectstack/spec patch.

Why this landing site

Matches the constraint's expected surface exactly: hook.form.ts, the pin, the regenerated translation bundles, and the changeset. No other producer or consumer needed a change.

Tests

  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/system/metadata-form-declared-rows.pin.test.ts src/system/metadata-form-zod-reconciliation.test.ts src/data/hook.test.ts src/data/hook-body.test.ts — 173 passed.
  • pnpm --filter @objectstack/platform-objects exec vitest run --maxWorkers=2 src/apps/translations — 399 passed (21 files), including metadata-forms-vocabulary.test.ts and hook-execution-panel-echo-decisions.test.ts.
  • pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec check:generated — all 15 generated artifacts up to date.
  • pnpm check:i18n — all 9 packages in sync, no undeclared authoring keys.
  • pnpm --filter @objectstack/spec typecheck — clean.
  • Gate derivation: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands → 85 derived families, all run and reconciled (--ran): 84 green, 1 honestly NOT-MEASURED — pnpm check:dual-build-cjs-loads exits its own PREREQUISITE NOT MET (3) because this worktree never ran a full pnpm build across every package in the monorepo (studio, client-react, several connectors/plugins/services have no dist/); that whole-workspace build is disproportionate to this one-row change and is CI's own "Build Core" job to run. Nothing about this diff is implicated in that gate.

Acceptance notes

None — no out-of-scope findings surfaced while making this change.


Generated by Claude Code

HookSchema.condition is a CEL predicate (EvaluatedExpressionInputSchema),
but hook.form.ts declared its `condition` row `language: 'javascript'`,
same as the real script rows (body.source, action.source). A consumer
keyed on the declared language cannot tell the predicate apart from a
script. Change the row to `language: 'expression'`, matching every
sibling predicate row, and pin it beside the existing form-row pins.

---
_Generated by [Claude Code](https://claude.ai/code)_

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
en.metadata-forms.generated.ts picks up the new condition helpText
(node scripts/check-i18n-bundles.mjs --write). The other 8 packages'
bundles regenerated byte-identical; zh-CN/ja-JP/es-ES kept their
existing translated-locale values under merge mode (now stale relative
to the new English source, per the tool's documented behaviour).

---
_Generated by [Claude Code](https://claude.ai/code)_

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 2 changed package(s)), so this run has no opinion about the docs.

What this run could not see

Coarse fallback — 137 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9bf5e67affab69ce740037f33b003f5faf45d205 → packageMentionDocs.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: ce6d3e9666a1153b68255452b5092a73c4a8e2d3
Local-runs: none

① Derived judgments

Inputs: card #20439 (body; triage grade 5871322775; claim 5872433320; os-dev-report 5874241898), PR #20475 (body; its one comment 5874198509, the docs-drift check; file list; net diff origin/main...refs/os-seat2/pr20475 — 4 files, +47/-2, three commits over merge-base acd009521e6e), and the 33 check-runs on the head as read at 2026-09-28T16:41Z.

  • Row change packages/spec/src/data/hook.form.ts:77 — the condition row: language: 'javascript' to 'expression', helpText to CEL predicate — the hook runs only when TRUE. RIGHT. HookSchema.condition is EvaluatedExpressionInputSchema (hook.zod.ts:293, described "Predicate (CEL); hook runs only when TRUE"), the same slot shape as the seven sibling predicate rows (field.form.ts:331-333, object.form.ts:302,347-349), every one declaring 'expression'. The two 'javascript' rows left (hook.form.ts:43 body.source, action.form.ts:60 source) edit a plain { language, source } body, not an envelope, so they are correctly untouched. The helpText is triage's wording verbatim.
  • Is 'expression' a value the form DSL declares? FormFieldSchema.language (packages/spec/src/ui/view.zod.ts:3273-3274) is z.string().optional() — a free string whose docstring lists 'javascript', 'sql', 'json', 'typescript', 'expression', 'cel' as examples. Declared by docstring and by seven live rows, not enforced by an enum: this diff moves no parse verdict and no accept-set. The changeset's "no key is added, removed, narrowed or widened, and no parse verdict changes" is accurate. RIGHT.
  • Public surface implied: the exported hookForm value in @objectstack/spec, and through packages/metadata-protocol/src/protocol.ts:201 (TYPE_TO_FORM = METADATA_FORM_REGISTRY, served on /meta/types) the form the Studio renders. A value change only — no DTS or type movement; Type Check · source gates (api-surface) is green on the head.
  • Does any consumer in THIS repo branch on a code row's language? NO. Swept every non-test .ts/.tsx under packages/, services/, apps/, examples/ at the head. The registry's readers outside spec are metadata-protocol/src/protocol.ts (verbatim pass-through), packages/lint/src/validate-predicate-path-refs.ts (reads rows' own visibleWhen predicates, never language), packages/cli/src/commands/lint.ts and packages/cli/src/utils/i18n-extract.ts / i18n-coverage.ts (extract labels and helpText, never language). The only language === 'expression' branches in the repo (packages/runtime/src/sandbox/quickjs-runner.ts:129, script-runner.ts:520) read a hook BODY's language — a different field. So the behavioural consumer is objectui's CodeWidget alone; in this repo the change is served data plus the en bundle. RIGHT that no other producer or consumer needed a change.
  • packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts:770 regenerated to the new helpText — RIGHT; Lint & Repo Gates (the job that runs pnpm check:i18n) concluded success on this head. No copy of the old text survives anywhere in the head tree except the changeset's FROM/TO prose (grepped).
  • Pin packages/spec/src/system/metadata-form-declared-rows.pin.test.ts asserts hook.condition is exactly one top-level row with type: 'code' and language: 'expression'. condition IS a section top-level row (Execution section), so rowFor reaches it. The dev's "no existing row-language assertion" holds: the card's pointer hook-body.test.ts:15,22,132 asserts HookBodySchema / ExpressionBodySchema parsing, not a form row, and field.test.ts:1584 is a field schema — so a new pin beside the finding(spec): field.relatedListFilter and object.validations are DECLARED by the served schema and omitted by METADATA_FORM_REGISTRY's forms — the generic metadata form never renders them, so an author's only door is the Source tab #19085 form-row pins is the right landing. The CONTROL is a second lit reading (a sibling 'expression' row), not a dark one; its comment's claim that it catches a helper that stopped reading language is slightly off (that failure mode reds the main assertion itself), but the report's reverse-verification (revert to 'javascript', re-run: exactly the new assertion RED, the other 7 green) supplies the discrimination evidence. Acceptable as is.
  • Docs: content/docs/references/data/hook.mdx:39, content/docs/automation/hooks.mdx:107 and content/docs/automation/hook-bodies.mdx:74 already say the condition is CEL, so no page is falsified; the Docs Drift Check had no opinion (generic names). RIGHT that no docs edit was owed.
  • Scope: the four touched files are exactly the claim's file surface (hook.form.ts, the pin, the regenerated bundle, .changeset/20439-*.md); no breach; no governed path (Governed Surface Queue Guard green); examples/app-showcase/src/data/hooks/index.ts's two authored conditions are untouched and unaffected (the form is an editing surface, not a parser).

② Semver level

.changeset/20439-hook-condition-expression-row.md declares @objectstack/spec: patch. RIGHT — a bug fix in a released package (17.4.0): a declared-row value change with no accept-set movement is patch, never skip-changeset. @objectstack/platform-objects (also 17.4.0, public) carries the regenerated en bundle; both packages sit in the single fixed group of .changeset/config.json, so the spec changeset versions it in lockstep and no second changeset is owed. Clause-②: no, no arm, in both the PR body and the changeset body — RIGHT: language is a free z.string(), nothing widens or narrows. Check Changeset green.

③ Boundary flags

  • Dev flag (PR body and report): zh-CN / ja-JP / es-ES metadata-forms bundles kept their old translated helpText, "now stale relative to the new English source". ANSWERED — that IS the bundle tool's documented behaviour, and no gate or test treats it as a defect. packages/cli/src/commands/i18n/extract.ts:294-303: "Merge (the default) never overwrites an existing non-default-locale entry … a present-but-stale string is not a gap, only a missing one is. This is deliberate, not an oversight"; the generated bundle header says the same (packages/cli/src/utils/i18n-extract.ts:2244); packages/cli/test/i18n-extract.test.ts:414 (check-i18n-bundles merge mode never updates an EXISTING field description, so editing one leaves the en bundle silently stale (and the gate green) #8543) pins that translated locales keep merge while en tracks the source. The --source-hashes companion this package opts into (packages/platform-objects/scripts/i18n-extract.config.ts) records only leaves that are copies of the source, carries zero metadataForms.* entries in all three locales, and its header says a path with no entry is legacy-trusted and never reported stale; source-hash.test.ts records the ruling: staleness is a SERVING rule, not a gate (Option C — fail the build until every locale is re-translated — was rejected). The three old strings still describe the slot correctly (skip when false is the same rule as run only when TRUE), so no author is misled; re-translation is a translator's in-place edit, not this card's.
  • Dev flag: check:dual-build-cjs-loads NOT MEASURED locally (prerequisite: a whole-workspace build). ANSWERED by the head: Build Core and Type Check · consumer gates concluded success.
  • Test Core (3/6) concluded failure — NOT this diff's. Job 109025393814 log, read: the @objectstack/spec#build DTS step died ERR_WORKER_OUT_OF_MEMORY (JS heap) before any test ran — check-test-completeness: 0 of 13 scheduled package(s) reported, 13 never reached — and the shard's 13 packages include neither @objectstack/spec nor @objectstack/platform-objects. The identical spec#build task succeeded on this same head in Build Core, Test Core (2/6), Test Core (4/6) and all three Dogfood Regression Gate shards, and the diff changes one string literal, one test file and one generated bundle — nothing that alters DTS emission. A runner-memory flake; the owning seat re-runs the shard before enqueue, it does not touch the verdict on the diff.
  • Check-runs NOT yet concluded at that reading (2026-09-28T16:41Z): Test Core (1/6), Test Core (5/6), Test Core (6/6), Type Check · workspace. This record does not presume them green; the seat reads them before landing.
  • open_questions: [] — nothing to answer. out_of_scope_findings: [] — I looked for one: hook.form.ts:43 body.source stays 'javascript' even when body.language is 'expression' (an L1 body is JS-highlighted). That row's wire is a plain string, so 'expression' there would route it through the envelope wrongly — not a defect, no card owed.
  • Nothing ESCALATED.

Implemented-by: claude/issue-20439-hook-condition-expression-row
Reviewed-by: session_014EJ1ED8X4MMrT18BhVx4tx

VERDICT: PASS


Generated by Claude Code

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

Projects

None yet

2 participants