Skip to content

fix(cli): the one-definition-of-multi-valued invariant reaches os generate — the CLI half of #17469 - #18392

Merged
os-support-ai merged 4 commits into
mainfrom
claude/issue-18199-multi-valued-invariant-cli
Sep 16, 2026
Merged

os-support-ai merged 4 commits into
mainfrom
claude/issue-18199-multi-valued-invariant-cli

Conversation

@os-support-ai

@os-support-ai os-support-ai commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Fixes #18199

Clause-②: no — this PR puts no new key on a published payload. It makes os generate follow the already-ruled isMultiValueField definition (#17469, director batch #128 item 5, option 1′) — pulling a diverged emitter back to a declared contract, which narrows rather than widens. Declared by the dispatching seat in claim 5693774731.

The maintainer ruling of 2026-09-13 (director batch #128 item 5, option 1′) gives "is this field multi-valued" one definitionisMultiValueField in packages/spec — and storage follows it. #17469 landed the driver-sql half and stopped at the package boundary. This is the ruling card for the packages/cli + packages/runtime half; the packages/objectql (11 files) and packages/drivers/driver-turso (1) sites are a separate domain:engine child card and are not touched here.

Per-site verdict

The premise is the card's own, and it is falsifiable per site: does this site read field.multiple to mean "is this value multi-valued"? It held for 18 of 21 sites in driver-sql and failed for 3. Here it holds for 5 and fails for 1.

site (on origin/main @ 8b81ab60) reading verdict
packages/cli/src/commands/generate.ts:2617 if (fieldDef.multiple) { table.jsonb(...) } decides the generated migration's column type covered
packages/cli/src/commands/generate.ts:2443 fieldTypeToSql(fType, !!fieldDef.multiple, …) same decision, --format sql covered
packages/cli/src/commands/generate.ts:1706 declaredColumnDefaultif (declaring?.multiple) return { kind: 'none' } mirrors createColumn's multi-value short-circuit, which returns before the DEFAULT line covered
packages/cli/src/commands/generate.ts:757 fieldTypeToTs(fType, !!fieldDef.multiple)os generate types decides whether the generated property type is an array — the ADR-0104 D1 value question verbatim covered
packages/cli/src/commands/generate.ts:1203 same call — os generate client same covered
packages/runtime/src/action-execution.ts:1279 multiple: p?.multiple ?? field?.multiple inherits an author's declaration onto a descriptor; decides nothing excluded

Why the two fieldTypeToTs sites are in, although the card's table names only three

The card's table is illustrative — it names 2 of objectql's 11 files too. The #14829 pin in this package states the invariant as "one authored multiple: true field, three surfaces, ONE answer": generated TS type, --format sql, --format typescript. Aligning the two migration surfaces and leaving fieldTypeToTs on the raw flag would have given a flagged text field string[] from os generate types and TEXT from the migration generated in the same run — #14829 re-created inside the CLI. Landing three of five sites was not available.

Why action-execution.ts:1279 is excluded — the reading

Both triage comments flagged it as a candidate for the same exclusion that removed 3 of 18 driver-sql sites, and the measurement agrees. resolveDeclaredActionParams copies multiple from the referenced field onto a ResolvedActionParam; the descriptor is then handed to validateActionParams, which builds the param's schema with valueSchemaFor({ type, multiple, options }) — whose last line is

return isMultiValueField(def) ? z.array(element) : element;

So the runtime already follows the one definition at this seam, through the spec's own consumer, and the read here is the ADR-0104 D2 inheritance the function's docblock declares. Pre-computing the verdict into the field would also break that contract: the key would stop carrying what the author wrote, and the predicate would then be applied to its own output. The reading is recorded at the site, so the next grep does not align it.

The declared divergence is gone

Three comments in the tree declared the losing authority, and the card is class (b) because of them. Each is rewritten rather than left standing:

  • generate.ts:1332 — "⛔ the DRIVER is the authority for which column exists, never the spec's isMultiValueField VALUE predicate". The first clause is still true; the second stopped being true when the driver's own multi-value half became that predicate.
  • generate.ts:2286 — "WARNING: this is deliberately NOT the spec's isMultiValueField … the column question belongs to the driver, and the driver's answer is the flag alone." The premise held and the conclusion inverted; the paragraph now records the reversal.
  • generate.ts:2615-2616 — "See fieldTypeToSql for why the authority is the driver's flag rule and not the spec's isMultiValueField value predicate."

One more, in a test: generate-field-type-vocabulary.pin.test.ts:90 carried the same sentence.

Measured — origin/main @ 8b81ab60 vs this branch

One config through all three generators, the same probe both times (the pre-fix leg was taken by checking generate.ts out at the base commit and restoring it to a byte-identical blob, be774af8…):

declaration os generate types --format sql --format typescript
{ type: 'text', multiple: true } string[]string JSONBTEXT table.jsonbtable.text
{ type: 'text', multiple: true, defaultValue: 'x' } TEXT with no DEFAULT → TEXT DEFAULT 'x' .defaultTo("x")
{ type: 'multiselect', multiple: true } string[][]string[] JSONB (unchanged) table.jsonb (unchanged)
{ type: 'lookup', multiple: true } string[] JSONB table.jsonb — all unchanged

The last two rows are the same raw read reaching two shapes the card did not name. multiple: true is accepted redundantly on the inherently-multi option types, so FIELD_TYPE_MAP's own string[] entry was being wrapped a second time; and the short-circuit was withholding a DEFAULT that the driver emits. Both close with the same alignment.

generate-multiple-json-column.pin.test.ts — re-targeted deliberately

It is #14829's pin, it went red when #17469 landed, and it pins the behaviour this PR changes. It was not skipped, disabled or quarantined. Before the re-target it failed exactly three arms, all on text, every MULTI_CAPABLE_TYPES member still green:

× control — the SAME type without the flag still gets its scalar column
    AssertionError: expected 'TEXT' not to be 'TEXT'
× text + multiple:true — array TS type AND a JSON column in both migrations
    AssertionError: expected 'string' to match /\[\]$/
× the flag decides before the type in the GENERATORS — a type outside MULTI_CAPABLE_TYPES too
    AssertionError: expected 'TEXT' to be 'JSONB'
Tests  3 failed | 12 passed (15)

That red is the narrowness measurement: the only declaration whose answer moves is the one where the flag and the one definition disagree. What changed in the file:

  • the JSONB arm for flagged text is inverted, not deleted — it is the only row that can tell which predicate generate.ts reads, so it now asserts the scalar answer and says so in its own failure text;
  • the discriminating control moves from single_text vs multi_text (now the same column, which is the fix) to single_lookup vs multi_lookup, which still straddles the predicate;
  • the swept roster is split by the predicate itself (FLAGGED_TYPES.filter(t => isMultiValueField({ type: t, multiple: true }))) rather than by a second hand-written list, with a control pinning both arms non-empty — SINGLE_VALUED_WHEN_FLAGGED must be exactly ['text'];
  • two new arms: the flag adds no second array level on an inherently-multi option type, and all three CLI surfaces agree with isMultiValueField across the whole sweep, with both outcomes required to occur so "they agree" is not "they are all the same answer".

The two source-read pins against driver-sql are untouched and still green.

Verification

All legs re-run on the final head after merging origin/maingit rev-parse --short HEAD = 0ee83a647. The merge brought packages/lint and .github/workflows/lint.yml, neither of which this diff touches; re-deriving the gate families on the merged tree added none.

leg command verdict
① dependency closure pnpm --filter '@objectstack/cli^...' build --concurrency=2 VERDICT command-exit 0
① package build pnpm --filter @objectstack/cli build VERDICT command-exit 0
② typecheck pnpm --filter @objectstack/cli typecheck exit 0 — check:test-typecheck: OK
② unit tier pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2 208 passed (208) / 2976 passed (2976)
③ gate families node scripts/pm/dispatch-gates.mjs derivation, reconciled with --ran 93 derived, 93 accounted, 0 UNRUN — 91 run, 2 NOT MEASURED (recorded exit 3)
③ lane addition pnpm lint (whole repo, not narrowed) exit 0

packages/cli's integration tier is declared to CI: the diff touches no integration-layer file, no bin/ entry and no test/helpers/serve-process.ts, so no local run of it is owed.

Not measured, with the reason — three gates refused their own prerequisite on this checkout, which is neither a red nor a pass:

  • pnpm check:dual-build-cjs-loads — exit 3, PREREQUISITE NOT MET, 12 packages have no dist. Its own text: "⛔ This is NOT a pass: nothing was measured."
  • pnpm check:i18n-coverage — exit 3, COULD NOT MEASURE: 1 of 13 configs failed to lint against an unbuilt @objectstack/connector-mcp.
  • pnpm --filter @objectstack/spec check:skill-examples — exit 1 on packages/client-react/dist holds no .d.ts declarations, the same class.

All three are covered by CI, which builds first; Lint & Repo Gates is green on this branch.

Acceptance notes

  • ⚠️ pnpm check:cross-package-test-inputs exits 1 on this branch, and it is not caused by this diff — it reproduces on origin/main @ 8b81ab60d. Control taken in a comparison worktree at that commit: with packages/spec unbuilt it exits 0; after pnpm --filter @objectstack/spec build, with one packages/cli path modified so the @objectstack/cli scan is pulled in, it exits 1 with the identical finding — packages/cli/test/init-created-files-summary.e2e.test.ts descends packages/spec/dist/, which no declared glob reaches. So it is latent on main and surfaces only on a built checkout, which is the main is red for every PR that touches packages/spec: #7769 gave sys_api_key update without bulk, and the conformance scan that catches it lives in a package #7769 never touched #7802 blind spot the gate's own text describes ("red on main while every PR reports green"). Reported for filing rather than fixed here: it is a different defect class from this card's, in a file this diff does not touch.
  • packages/cli/src/commands/migrate/{meta,summary-nulls,multi-value-columns,files-to-references,value-shapes}.ts all contain multiple: true, and every one of them is an oclif Flags.string({ multiple: true }) — a repeatable CLI flag, a different key with a different subject. Not covered, noted so the next grep does not have to re-derive it.
  • packages/cli/src/commands/explain.ts:70 describes the multiple field prop in a metadata-key listing. It echoes an author's vocabulary and decides nothing — the same shape as the 3 interpolation sites driver-sql isJsonField and spec isMultiValueField disagree for master_detail / tree / text + multiple: true, so a consumer using the spec predicate to shape a query gets a 400 #17469 excluded in driver-sql.
  • os migrate multi-value-columns reads driver-sql's own manual_column_type_change drift report and never asks the question itself, so it inherits the one definition for free.
  • Nothing above is filed as a card: none is a reproducible defect, a contract violation or a metadata-authoring trap.

Generated by Claude Code

…gration and type generators

The maintainer ruling of 2026-09-13 (decision batch #128 item 5, option 1')
gives "is this field multi-valued" ONE definition -- `isMultiValueField` in
`packages/spec` -- and storage follows it. #17469 landed the driver-sql half;
`packages/cli`'s generators were left reading `field.multiple` raw, and said so
in three code comments.

All five reads in `generate.ts` now go through one local seam,
`declaredMultiValued`, which calls the spec predicate:

  - `fieldTypeToTs` call sites (os generate types, os generate client)
  - `declaredColumnDefault`'s multi-value short-circuit
  - `fieldTypeToSql` call site (os generate migration --format sql)
  - `generateMigrationTs`'s JSON short-circuit (--format typescript)

The three comments that declared the losing authority are rewritten rather than
left in place.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
…rd the runtime site as excluded

The #14829 pin held "one authored `multiple: true` field, three surfaces, ONE
answer". #17469 split it on the retired shapes and the pin recorded the split
in a header paragraph rather than papering over it. The split is closed, so:

  - the arm that asserted JSONB for a flagged `text` field is INVERTED, not
    deleted -- it is the only row where the flag and the one definition
    disagree, so it is the only row that can tell which one is being read;
  - the discriminating control moves from `text` to `lookup`, which still
    straddles the predicate;
  - the swept roster is split by the predicate itself rather than by a second
    hand-written list, with both arms pinned non-empty;
  - two new arms: the flag is redundant (not a second array level) on an
    inherently-multi option type, and all three CLI surfaces agree with
    `isMultiValueField` across the whole sweep.

`packages/runtime/src/action-execution.ts` was measured against the same
per-site premise and EXCLUDED: it inherits an author's declaration onto a
descriptor, and the one definition is asked of that descriptor one frame down
in `valueSchemaFor`. The reading is recorded at the site so the next grep does
not align it.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 16, 2026
@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cli, @objectstack/runtime, touching 9 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/runtime/src/action-execution.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/kernel/lifecycle.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/objectql/types.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-4.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/runtime/src/action-execution.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 41 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 37af6533be1af36671e52eefebfd7740e24d56f1packageMentionDocs.

Which tree this was computed on

This run read content/docs from eafe648039a4d2660b8ac1ff2eb67b37f53ffbe8 — the merge of head 0ee83a64705f64da5258722a51a6cc3957389f62 into base 37af6533be1af36671e52eefebfd7740e24d56f1, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin eafe648039a4d2660b8ac1ff2eb67b37f53ffbe8 && git checkout eafe648039a4d2660b8ac1ff2eb67b37f53ffbe8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 37af6533be1af36671e52eefebfd7740e24d56f1 0ee83a64705f64da5258722a51a6cc3957389f62 && git checkout -B drift-repro 37af6533be1af36671e52eefebfd7740e24d56f1 && git merge --no-ff 0ee83a64705f64da5258722a51a6cc3957389f62

node scripts/docs-audit/affected-docs.mjs --json 37af6533be1af36671e52eefebfd7740e24d56f1

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 37af6533be1af36671e52eefebfd7740e24d56f1 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…ued, not the raw flag

Two statements on `content/docs/protocol/objectql/types.mdx` were falsified by
the surface this branch changes, in the #11430 shape the drift bot names: the
page states the rule by the ObjectQL type names it maps FROM, so it shares no
identifier with the emitter and was not listed.

  - "Any field flagged `multiple: true` becomes a `JSON` column regardless of
    its type" -- untrue since the maintainer ruling of 2026-09-13 gave
    "multi-valued" one definition and storage followed it. The column follows
    `isMultiValueField`, and the flag on a type outside the multi-capable and
    multi-option classes is refused at parse.
  - "set `multiple: true` on a scalar/`lookup` field" -- a scalar field is
    exactly where that declaration is now refused.

Factual corrections only; no new narrative, and `content/docs/releases/` is
untouched.

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

Copy link
Copy Markdown
Collaborator Author

CI state on the CURRENT head — one red, and it needs a body edit, ⛔ not a push

domain:cli execution PM seat, session session_01DvvamiacK328idtBYJBxV3, 2026-09-16T08:23Z.

⚠️ Two of the three failure events on this PR are already SUPERSEDED

Events arrived for TypeScript Type Check and Dogfood Regression Gate, both carrying head add55443aed3e245ed4eb80e71caa7f57e7f86ae. That is no longer this PR's head. Re-read from the PR object rather than trusted from the event payload, per 「信任 suite/check 事件前先重读 PR 对象取 head:检查读数绑定该 head,⛔ 不绑事件带的 SHA」:

current head: 6280fdc53d13b7de6097a146eb90c68ccc256490   (3 commits, 6 files, +332/-78)

Check runs on that head: 32 total — 1 failure · 11 success · 2 skipped · 18 still in progress.

⇒ ⛔ Those two reds are ⛔ not this head's state and ⛔ are not diagnosed here; acting on them would have been a diagnosis of a commit that no longer exists on this branch. ⭐ The 18 in_progress rows are an honest reading, ⛔ not a green one — this PR is ⛔ not cleared yet.

The one live red: Check Changeset (job 104719379696)

The PR body carries no line-initial Clause-②: declaration — measured on the stored body just now: 0 such lines. The gate reads only line-initial; a mention inside prose does ⛔ not count (its sibling PR #18389 proved that, having exactly one prose mention and going red anyway).

The gate's own remedy, verbatim:

DECLARE IT. One line, at the START of a line in the PR BODY (a - , > or ** prefix is read too)
And the line is read from the body on the next edited event (pr-automation.yml subscribes to it), so this red clears with no push and no re-run.

no push and no re-run are required, and none was spent. The declaration was already made by this seat at claim time (5693774731, Clause-②: no) and rests on #17469's ruling — this diff pulls a diverged emitter back to the isMultiValueField definition, which narrows rather than widens. The exact line is with the delivering dev.

The remedy is the declaration, never the deletion — the gate names dropping the changeset or regrading the package as the wrong fix, and this seat endorses that.

domain:cli execution PM seat · #6024 · session session_01DvvamiacK328idtBYJBxV3 · R76 · CI state · reading time 2026-09-16T08:23Z


Generated by Claude Code

@os-support-ai
os-support-ai marked this pull request as ready for review September 16, 2026 09:12
@os-support-ai
os-support-ai added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 7c8d6d9 Sep 16, 2026
44 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-18199-multi-valued-invariant-cli branch September 16, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants