fix(cli): one column width for the file family in both os generate migration formats - #18014
Conversation
…mats `os generate migration` answered one file-family field with two widths depending on `--format`: the sql format states `VARCHAR(2048)` while the typescript format spelled a bare `table.string(name)`, knex's varchar(255). The typescript half now reads its width off the sql vocabulary instead of riding the reference arm, so the two formats of one command cannot disagree. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
…mats The new pin measures the two formats against each other AND against the driver's own `MEDIA_ID_VARCHAR_CHARS`, so two halves that both drifted to knex's default could not satisfy it. Controls: `f_text`, the non-file family the generator documents, and the reference arm the family used to ride. The vocabulary pin's `tsColumn` reader stopped at the column name, so a sized call read as no column at all; it now captures the argument list. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8ebbd1c8bc0c72c521e898690b5b9275238bd966 && git checkout 8ebbd1c8bc0c72c521e898690b5b9275238bd966
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 60b99552a2f88ee8b06f28f8bec3caa952e60f3c 78adc8ea86a61980dff05cb29651c2c4b4acf035 && git checkout -B drift-repro 60b99552a2f88ee8b06f28f8bec3caa952e60f3c && git merge --no-ff 78adc8ea86a61980dff05cb29651c2c4b4acf035
node scripts/docs-audit/affected-docs.mjs --json 60b99552a2f88ee8b06f28f8bec3caa952e60f3c
|
Contract reviewHead reviewed: Verdict: PASS. Independence: SELF-REVIEW. I dispatched this card's dev, so this review shares my dispatch order's assumptions — including, as it turned out, a wrong one. That is disclosed here, not smoothed over: the item below that this PR falsified is mine, and I would not have caught it if the dev had simply obeyed the order. ⛔ My acceptance item 4 was wrong, and obeying it would have written a false statement into the fileMy order named That is the STRING family's measured three-producer row — a different fork, a different card. The dev measured the item, refused it, and said so in The paragraph that really was stale about this family is What the diff actually doesThe production change inside those 85 added lines is small: the five file-family cases leave the case 'image': case 'file': case 'avatar': case 'video': case 'audio':
colMethod = `table.string('${fieldName}', ${fileReferenceVarcharChars(fType)})`;— and ⛔ The five Docs drift — checked by hand, because the checker says it cannot see this classThe advisory listed
⇒ This diff falsifies nothing on that page. The other three listed pages restate no width at all ( ⛔ Release-owned guardrail held, and it was measured, not assumed: Changeset — owed, present, and at the right levelThe dev did not take my order's default here either; it measured the criterion. Verification I am relying on, and what it does not coverThe dev's evidence is unusually load-bearing and I checked its shape rather than only its conclusions:
⛔ Not covered, stated rather than papered over: no dialect was exercised — there is no live PostgreSQL or MySQL cell in that container, so nothing here is a DDL measurement. The dialect answer comes from CI's Acceptance notes I am accepting as filed, with judgement
What this review does not do⛔ No approving review, ⛔ no merge — this is a managed-surface PR and neither is mine to give. Next: ③ green on Reviewed-by: os-sales — Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 34756944488 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
The queue build failed, and the failure is not this PR'sRead at 2026-09-13T12:38:49Z – 12:40:20Z. Standing down rather than re-queueing, with the reason measured rather than asserted. What failed: It is an assertion on product behaviour, not on a duration or a timestamp, so by the triage comment's own rule it is a real behaviour change and ⛔ not something a re-queue may be used to wish away. Why it is not this PR's. The assertion runs list: { …, sort: [{ field: 'name', direction: 'desc' }] }This PR touches zero What the queue build actually contains. The queue branch is ⇒ #17914 is in this build, ahead of this PR, and is a breaking change to the very clause the failing specimen is made of — the list-view Controls, so this is not one reading standing alone:
What I have not measured: I did not execute What I am doing about it. Nothing to this PR. There is no fix to port: the seam is #17914's to carry — either its own diff updates the CLI specimen to the retired clause's replacement, or the ⛔ No re-queue yet — a re-queue now rebuilds the same collision and burns a full-queue cycle for everything behind it. The re-queue becomes correct once #17914 has landed or left the queue, and it is the one re-run this failure is entitled to. This PR stays watched until it is green and merged. Generated by Claude Code |
⛔ Correction — my previous comment claimed more than the evidence supportedIn The control I missed. #17914 had its own merge-queue entry, built and tested alone, and the very shard that failed on this PR's build passed there: I compared only against A second control, since taken: What is still establishedThe mechanism is read from the diff, not inferred from timing: What is NOT established, and I am not going to assert itWhy the same shard is green on #17914 alone and on What that changes about the next step — nothing, and here is whyEither way the same action is correct and it is also the decisive experiment: this PR's base is now stale (it was validated on
⛔ Still no re-queue on the stale head, ⛔ no re-run used as a substitute for an answer, ⛔ nothing skipped, disabled or quarantined, and ⛔ no empty commit. This PR stays watched until it is green and merged. Generated by Claude Code |
Resolved by measurement:
|
| tree | head | vitest run test/format-zod-union.test.ts |
|---|---|---|
| PR head (main + this PR's 4 files) | c1e5ce24c697 |
exit 1 — 2 failed / 11 passed |
origin/main alone, this diff absent |
1e20f816ea81 |
exit 1 — 2 failed / 11 passed, byte-identical failures |
| before #17914 | f52bfa90f471 |
exit 0 — 13 passed |
The third leg is what makes this a control rather than a blind reading: it can go green, and does. ⇒ Red on both sides of this PR's diff. origin/main is already broken; this PR's four files do not participate — they are packages/cli/src/commands/generate*.ts plus a changeset, while the failing test exercises packages/cli/src/utils/format.ts and @objectstack/spec's view schema.
Two corrections to my own record
My earlier correction over-corrected. In 5653470881 I wrote that if the test failed on the rebased head then "the seam is real and reaches this PR". It fails there — and it also fails on main without this PR, which is the reading that decides. The stand-down in 5653324611 reached the right conclusion; what was wrong with it was the basis, not the verdict. I have now been wrong in both directions on this attribution, once by asserting past my evidence and once by retreating past it. Both are recorded rather than quietly reconciled.
My stated mechanism was wrong on the second issue. I predicted "order is missing and required". Measured against the built spec on each side:
AFTER #17914 (1e20f816e) — 2 issues:
[0] code=invalid_value path=views.0.list.sort.0.order
"Invalid option: expected one of \"asc\"|\"desc\""
[1] code=unrecognized_keys path=views.0.list.sort.0 keys=["direction"]
BEFORE #17914 (f52bfa90f) — 1 issue:
[0] code=invalid_union path=views.0.list.sort "Invalid input"
[0] is invalid_value from z.enum(['asc','desc']) — not an invalid_type/required report. A patch author matching on code or message would have matched the wrong thing.
The break is deeper than the count, and one green is now vacuous
The length assertion masks a second, independent break. The next line of the block is expect(issues[0]!.code).toBe('invalid_union'), and after #17914 views[].list.sort is no longer a union at all — the bare-string arm was the union's other member. The subject of that describe block is a rejection reaching the terminal from behind a z.union, and that shape no longer exists at this door. It is also why the block's third test still passes: it asserts the prescription reaches the terminal, and it does — by the ordinary non-union path the file was never about. ⛔ So the minimal patch is not 1 → 2; it is re-pointing the specimen at a door that is still a z.union with a strict branch carrying a curated prescription, or re-authoring the block. That is an authoring decision, not a mechanical one, and it was deliberately not made here.
File face of the repair: packages/cli/test/format-zod-union.test.ts — test-only. ⛔ Not packages/spec/**: #17914 is the ruled end-state (#17053, ADR-0049) and editing it would reverse a landed ruling.
Sequencing, and why nothing changes on this PR
The repair does not belong in this PR. This PR's card is the file-family column width; folding an unrelated consumer repair into it would widen the PR and would make a red that main already owns read as this PR's. The consequence that matters for sequencing: this PR cannot reach green through the merge queue until main is repaired, because the queue rebuilds it onto current main. ⛔ No re-queue until then — it would fail identically and burn a full-queue cycle for everything behind it.
⚠️ A second finding, stated as open because it is not closed
CI reports Test Core (3/6) success on 1e20f816e — the same commit where this test measures red. Partial evidence, no conclusion drawn:
- Those
mainjobs did real work:Run this shard's testsat 834s and 817s, not a cache hit. - But
Test Core (5/6)on that same commit ran its tests in 2 seconds, which is not a test run. turbo.json's explicitinputsfor@objectstack/cli#testpin several cross-package files by name but notpackages/spec/src/ui/view.zod.ts, and the CI slice runs with--only.
That points at a scheduling or cache artifact rather than a passing test, but ⛔ it is not closed: the Actions log download 403s from these containers (redirect to blob storage, denied by the egress proxy), and the local partition does not reproduce CI's slicing. The experiment that settles it: read step 9 (Compute this shard's package set, which prints the shard's item list) of jobs 103727415113 (5/6) and 103727415143 (6/6), from a seat that can reach Actions logs. If it holds, a required gate has been reporting green over a test it never ran — which is worth more than this one repair.
This PR stays watched. ⛔ Nothing skipped, disabled or quarantined; ⛔ no empty commit; ⛔ no re-queue while main is red.
Generated by Claude Code
Contract reviewRe-anchored to the current head Verdict: PASS, carried forward unchanged from The head advanced twice since that review (
⭐ Control that can actually fail, so four "identical" rows are not a vacuous comparison — a file the base merges did change must differ: | That control is the #18043 repair ( ③ on this head34 checks, enumerated ( ⛔ Note the acceptance standard this does not rest on: earlier in this card's history a green Reviewed-by: os-sales — Generated by Claude Code |
Landing record — ledger 50⛔ This belongs on card #17883. It is here because #17883 has become unreachable. Measured 2026-09-13T16:22:06Z – 16:24:33Z, with controls:
The same shape holds for #17853 (ledger 49's card). Both are cards closed by a ⇒ The residue disposal this record would normally announce cannot be performed and is now moot — there is no card left to carry
Enqueued Reading 1 — shape and ancestryTwo fields ⇒ one parent ⇒ squash, which is what the queue does whatever Negative control that can actually fail — after a squash the PR head must NOT be an ancestor of Reading 2 — content, with three controls
What landed4 files, +321/−19. The production change inside them is small: the five Changeset: Contract review of record: The detour, recorded because it is the part worth rememberingThis PR was green, armed and queued at 12:22Z, and was ejected at 12:51:14Z — not by anything in its own diff. #17914 landed the retirement of the bare-string list-view My own record through it, kept here rather than tidied away: I first asserted the failure was "not this PR's" without taking the one control that could have falsified it (#17914's own queue build, which was green on the same shard), then over-corrected past the evidence in the other direction when the rebased head also went red. The three-tree measurement settled it — ⛔ Through all of it: no test was skipped, disabled or quarantined; no empty commit; no close-and-reopen; and the one re-queue this PR used was spent only after the cause was on Disposal⛔ Not performed, and not skippable-by-choice: see the header — the card is unreachable, so there is nothing left to clear. Seven-for-seven on residue surviving auto-close stands as today's count; this one is unmeasurable rather than clean. Generated by Claude Code |
Fixes #17883
Clause-②: no
os generate migrationgave the file family —file/image/avatar/video/audio— two different column widths depending on--format. The typescript format emitted a baretable.string(name), whichgenerate.tsitself states is knex'svarchar(255), while--format sqlemittedVARCHAR(2048)for the same field.The stop condition, measured first
Triage ruled the direction (the two halves must agree) with a hard fence: if 255 turns out to be deliberate, stop and route the card to
needs-user-decisioninstead of forcing the alignment.generate.ts:1456saysVARCHAR(50)andVARCHAR(2048)"were widths this file invented", and invented numbers have history. Five readings, taken on this branch's base611795e40:driver-sqlputs file/image/avatar/video/audio inJSON_COLUMN_TYPES,packages/cligenerate.ts gives themVARCHAR(2048)— and neither side is obviously the one that should move #15041 verbatim: "The driver is the side that moves; the generator'sVARCHAR(2048)already states the ruled end-state and stands." Its column section adds the operative criterion: the driver's width mirror "answers for the family what the generator answers, so the pin that sweeps the other string classes can sweep this one — a width the two sides disagree on would be this fork again, one level down."driver-sql's own constant header (MEDIA_ID_VARCHAR_CHARS = 2048) states it as a rule, not a taste: "A driver that created knex's defaultvarchar(255)here would open a fresh divergence between the two producers of the same column."media-column-move.ts:101—MEDIA_ID_MOVE_WIDTH = 2048, used in thetype varchar(2048) using (…)retype. Read as a control only; not touched.git log -Son the arm shows the family joined the typescript switch's REFERENCE arm in431979e67(fix(cli): give every FieldType member a generate.ts vocabulary entry, and make the next gap loud #14834), a card about giving everyFieldTypemember an entry — a presence fix. That arm's derivation is the target row'sidcolumn (table.string('id').primary()), which was never the file family's derivation. The 255 was inherited by sharing an arm, never chosen for this family.MAX_VARCHAR_CHARS = 16383, and both the SQL format and the driver already ship 2048 for this family everywhere.No reading makes 255 deliberate, so the alignment stands and the card does not move to
needs-user-decision. The typescript half is the side that was out of step.The change
The family leaves the reference arm and takes one of its own. The width is read from the SQL format's own entry rather than retyped beside it, so the two formats cannot drift apart again — a second literal
2048would be the same defect one line over.⛔ The SQL half did not move. 2048 is the already-shipped target; the repair is the typescript half joining it, never the two halves meeting in the middle. The new pin asserts the agreed width against
driver-sql's own constant precisely so that two halves that had both drifted to knex's default could not satisfy it.Acceptance — both directions, five types, with the control the file documents
One probe object, one field per type, both formats, before and after the fix. The whole delta is five lines:
--format sqlgivesVARCHAR(2048)and the typescript format givestable.string(name, 2048)forfile,image,avatar,videoandaudio. One verified is not five, so the pin sweepsFILE_REFERENCE_TYPESimported from the spec rather than a list written out.f_text— the non-file family the file itself documents atgenerate.ts:1403-:1405, where both halves already agree — is byte-identical before and after, and so is every other non-family column and the entire--format sqlemission (the diff above has no hunk on the SQL side at all). ⛔ No control was invented; the documented one was used.Reverse verification. With the fix committed, the pre-fix
generate.tswas put back on disk and the new pin run against it: 5 of 8 tests red, one per family member, with the three control/anti-vacuity tests staying green — the discriminating direction. On-disk proof of the mutation rather than an editor's exit code: the injected resolver went 4 occurrences ⇒ 0, the blob hash moved8d8c34a0⇒1fccbeff. Restore verified by state, not by exit code: blob back to8d8c34a0,git diff HEADempty. The subject resolves through a relative intra-package import (./generate.js), never through a dependency'sexports, so nodistsits between the edit and the verdict.Premise falsified — acceptance item 4 pointed at the wrong table
The dispatch order asked for the self-describing table at
generate.ts:1461to be updated, on the reading that it "still records the pre-#15989 three-way reading (driver varchar(255)) and is wrong on its first column". Re-derived on this branch, that is not so:Those two rows are about
phoneandurl— the STRING family, not the file family. #15989 moved the file family to 2048; it did not touchurl, whose driver column is stilldeclaredVarcharLength(field)=DEFAULT_STRING_VARCHAR_CHARS= 255 with no declaredmaxLength. The rows are the measured pre-#16091 reading kept deliberately as the rationale for the entries that replaced them ("VARCHAR(50)andVARCHAR(2048)were widths this file invented"). Rewritingdriver varchar(255)to 2048 there would have introduced an error, so those lines are byte-identical in this PR.The stale text about the file family is a different paragraph —
generate.ts:1375-:1381, which held the family out of scope on the ground that it "is in the driver'sJSON_COLUMN_TYPEStoday", untrue since #15989 landed. That paragraph is rewritten, which serves what acceptance item 4 actually asked for: no table left out of date the moment the fix lands. The same stale sentence ingenerate-field-type-vocabulary.pin.test.ts's header is corrected with it.Verification
Base
611795e40; readings taken at3b3ba8a35. Exit codes captured before any pipe.pnpm --filter '@objectstack/cli^...' buildVERDICT command-exit 0(dependency closure)pnpm --filter @objectstack/cli exec vitest run --project unitVERDICT command-exit 0— 206 files, 2958 tests passedpnpm --filter @objectstack/cli typecheckVERDICT command-exit 0(tsc --noEmit+check:test-typecheck, ledger held)scripts/pm/dispatch-gates.mjs --commandseslint . --no-inline-config --format jsonBoth typecheck programs were shown to contain the changed files (
tsc -p tsconfig.json --listFiles: 1532 files,commands/generate.tsand the new pin both present), so the green is about them and not around them.Three runs needed a prerequisite before they measured anything, and each was re-run to a real verdict rather than recorded as a pass:
check:dual-build-cjs-loadsandcheck:i18n-coverageboth exited 3 (PREREQUISITE NOT MET— built output missing) until the build they prescribe was run, then exit 0;check:type-check-debtexited 124 at a 240s budget and then 3 under--max-old-space-size=4096(the childtscOOM'd inside my own cap), and exit 0 at 6144 MB, which the gate names as its own CI-shaped ceiling.The
integrationtier is declared to CI: this diff touches no spawn entry, nobin/, notest/helpers/serve-process.tsand no driver or kernel boot path.Changeset — measured, not assumed
patchon@objectstack/cli.packages/cliis published (not private),files[]isdist README.md CHANGELOG.md, andtsconfig.build.jsoncompilessrc. Afterpnpm --filter @objectstack/cli build, the changed subject is present in the shipped path:packages/cli/dist/commands/generate.jscarries the new resolver (4 occurrences) and the sized emission (1). Positive control, a known shipped symbol in the same file: 3 hits. Negative control, a nonsense token: 0 hits. Something published moves, so a changeset is owed; a bug fix in a released package takespatch.Acceptance notes
media-column-move.tsis read as a control and not edited; theos migrate files-to-referencesretype statement is untouched; no width outside the file family moved;packages/specis not touched;content/docs/releases/**is not touched. No export was added to a published entry point.noted, not filed: on MySQL an index over avarchar(2048)utf8mb4 column exceeds InnoDB's 3072-byte index key limit, and since [finding]os generate migrationemits no declared index at all — a generated table carries none of the object'suniqueconstraints, while driver-sql creates them #16317 both formats emit declareduniqueindexes. This is not created here — the SQL format and the driver already build the column at 2048 — and it is unmeasured (no MySQL cell in this container), so it is an observation rather than a card. Successor: thedriver-sqlseat, on the next card that touches the media column width; on a per-dialect pin it would surface inTemporal Conformance (live PG + MySQL).noted, not filed:generate-string-family-width.pin.test.tsexcludesFILE_REFERENCE_TYPESfrom the character-catch-all sweep and gives a per-deployment reason that is now only half true (the driver's arm is per-deployment; the generator's width is not). The exclusion is still correct and the file family now has its own pin, so nothing is uncovered. Successor: whoever next edits that pin's derivation.packages/cli/src/commands/generate-file-reference-width.pin.test.ts, beside its seven siblinggenerate-*.pin.test.tsfiles, rather than underpackages/cli/test/**as the claim comment's declared file face spelled it. Same package, unit tier, andcheck:tier-file-adoption/check:cross-package-test-inputsboth pass.Generated by Claude Code