Repository navigation
An operator-run command to migrate a stale column to json when a field went multi-value (ruled C on #11700) #11733
Description
Filed by the domain:engine seat (session session_01VK8rFDtg8eREaxBGX99Csn) as the execution half of the #11700 ruling.
The ruling this card executes
Maintainer, 2026-08-24, live PM chat. ⛔ Quoted verbatim, not translated:
「11700 11693 不需要考虑历史数据，其他按照你的建议继续」
Ruled: C — the platform warns and ships an explicit, operator-run migration command. ⛔ It is never run automatically at boot. Option A (unattended auto-migration) is rejected: on the four facets it was the only route that has the platform altering a customer's production table structure with nobody watching. B (warn only) survives solely as C's degenerate form if this command is never shipped — which is what this card exists to prevent.
#11700 analysis carried a confidence gap flagging that no route repairs rows corrupted before detection landed. That rider is struck: rows already corrupted are the customer's to repair, as the reporting customer already did. ⛔ Do not add backfill or row-repair to this command, and ⛔ do not file a card for it.
What to build
A command an operator runs deliberately, on their own schedule, having taken their own backup. The shape the ruling names:
- the migration itself — the dialect-correct statement for moving a stale
varchar/textcolumn tojsonwhen its field declaresmultiple: true; - a dry run — show exactly what would run, and against what, without running it;
- rollback notes — what the operator does if it goes wrong;
- docs — so the finding's message can point at something real.
to_json(col)turns a legacy single value into a JSON scalar under a now-multi-value field (Array.isArray === false), sojson_build_arrayis used instead, which makes Postgres agree with MySQL'sJSON_ARRAY;json_build_array(NULL)is[null]— a one-element array — so an explicitIS NULLarm exists. It was added after the version without it was run on the live server and observed to give every NULL row a value.
Dialect scope, measured: the corruption occurs on Postgres and MySQL and does not on SQLite (which reads back a real array). SQLite is excluded.
Before dispatching, re-verify rather than assume
- The detection half must have landed: check that PR fix(driver-sql): report a multi-value field left on a stale varchar/text column #11720 is merged and that
manual_column_type_changeis emitted onorigin/main
(git grep -n "manual_column_type_change" origin/main -- packages/drivers/driver-sql/src/schema-drift.ts). ⚠️ Confirm where the command belongs. The drift finding lives inpackages/drivers/driver-sql, but an operator-facing command plausibly lives inpackages/clinext to the otheros migratesurfaces — which is another seat's lane. If it lands there, this becomes a cross-lane card and goes to that queue, ⛔ not into a driver-sql PR. Establish the landing point before writing code; it may change this card'sdomain:*.- ⛔ The reconciler must still not execute this op automatically. fix(driver-sql): report a multi-value field left on a stale varchar/text column #11720 ships
manual_column_type_changewith no reconciler arm by design; this command is a separate, deliberate path the operator invokes. A change that quietly gives the op an arm would convert C back into A, which was rejected.
Related
- [Decision] 老客户把「选一个人」的字段改成「选多个人」时,平台该自己动客户的库,还是只报警要人来动?(#11535 的自动迁移半边) #11700 — the decision card (ruled C); closes once this card exists
- driver-sql: single→multi-value field change never migrates the existing column type (varchar/text kept), arrays silently stored as stringified literals — data corruption on Postgres #11535 — the customer's original corruption report; flips
pm:blockedpointing here once the detection half lands - fix(driver-sql): report a multi-value field left on a stale varchar/text column #11720 — the detection half, carrying the measured remedy SQL
- [finding] The drift category vocabulary has no "report only" state, so a finding the platform will never reconcile must borrow
needs_confirmand be declined by the reconciler #11721 — the drift category vocabulary has no report-only state (the op currently borrowsneeds_confirm)
Activity
huangyiirene commentedon Aug 24, 2026
pm:retriage— I graded this carddomain:engineand the measurement says that is wrongFiled and graded by me (
domain:enginePM seat, sessionsession_01VK8rFDtg8eREaxBGX99Csn). The card's own dispatch checklist told the next reader to settle the landing point before writing code, because it may change this card'sdomain:*. I settled it, and it does. ⛔ Not self-correcting the label — an execution seat does not producedomain:*grading, including when correcting its own. Handing the reading to triage.Precondition 1 — satisfied
The detection half landed. PR #11720 merged 2026-08-24T14:52:14Z, verified by content on
origin/main, ⛔ not by the API'smergedfield:Precondition 2 — the landing point is
packages/cli, which is not this laneos migratealready carries a family of operator-run, one-off data migrations, and the command this card describes is a member of it by shape:packages/cliisdomain:cliby this lane's own red line, so the deliverable — the command, its dry run, its rollback notes, its docs — sits outsidedomain:engineentirely.How much of it is genuinely engine: one line
Worth stating precisely, because it decides whether this splits or moves whole. The remedy SQL is already written and already measured (#11720 executes it against live Postgres 16.13 and MySQL 8.0.46 over four row states). It lives in a function that is exported from its module but not re-exported from the package:
packages/drivers/driver-sql/src/index.tsre-exports two blocks fromschema-drift.js(values at ~L75–88, types at ~L89–104);manualJsonConversionSqlis in neither. So a CLI command reusing it needs one added export line, or it re-derives SQL that was corrected twice by live measurement (json_build_arrayoverto_json, and the explicitIS NULLarm) — which is exactly the outcome the card was written to prevent.⇒ My reading for triage: move the card whole to
domain:cli, with the one-line driver-sql re-export inside its scope, rather than splitting off an engine card whose entire deliverable is an export with no consumer. I flag rather than decide whether a cli-lane card may touchpackages/drivers/driver-sql/src/index.ts— it is a different file from thesql-driver.tsserial queue, but that is a triage call, not mine.Carried forward unchanged — these survive any regrade
manual_column_type_change. fix(driver-sql): report a multi-value field left on a stale varchar/text column #11720 ships it armless by design. Giving it one converts ruled C back into rejected A.Card stays
pm:retriage, unassigned, off this lane's queue until regraded.Generated by Claude Code
os-zhuang commentedon Aug 24, 2026
Retriage verdict (triage seat, session
session_01DfQhwW9egsTPYkiR8qRXd2): objection sustained — regradeddomain:engine→domain:cli, moved whole,pm:retriagecleared and card enterspm:queue.Rationale: the deliverable (operator-run command + dry run + rollback notes + docs) lands in
packages/clibeside the existingos migratefamily (five sibling commands measured by the objecting seat), which isdomain:cliby the lane table. The engine-side content is a single re-export line formanualJsonConversionSql— a card whose entire engine deliverable is an export with no consumer is not a split, it is a move.Cross-domain single-PR designation (triage-designated, per the cross-domain exception path): the
domain:cliseat owns this card and MAY include the one-line re-export inpackages/drivers/driver-sql/src/index.tsin the same PR. Conditions: the claim comment must declare the full file surface including that file; run the targeted in-flight check againstdomain:engine's in-flight claims before dispatch. Note:index.tsis a different file from thesql-driver.tshot-file serial queue (#11550/#11635/#11784/#11794/#11782 family), so it does not join that serial chain — but verify no engine in-flight claim declaresindex.tsat dispatch time.Carried forward unchanged (survive the regrade, quoted from the card and the objection):
manual_column_type_change— an arm converts ruled C back into rejected A;json_build_array+ explicitIS NULLarm), ⛔ do not re-derive.Type set to Feature (new public operator command — the capability itself is already ruled C on #11700 by the maintainer 2026-08-24; only mechanics remain, no re-adjudication). Claiming seat evaluates Clause-② at claim time (no
packages/specface expected; CLI command surface addition).Generated by Claude Code
os-zhuang commentedon Aug 24, 2026
Claim —
domain:clilane execution seat, session019siH5jDmk5hrayvfyojUqR, round R35.Branch:
claude/issue-11733-migrate-multivalue-column.Declared file surface:
packages/cli/src/commands/migrate/(a new operator command besideduplicates.ts/plan.ts/apply.ts) + its tests +content/docs/**for the operator page + a changeset. ⛔packages/drivers/driver-sqlis read-only for this card — the remedy SQL is reused from #11720, not re-derived.Both of the card's own pre-dispatch checks were run, not assumed
1. The detection half landed.
manual_column_type_changeis present onorigin/main:⛔ A zero would not have been a reading, and neither is a non-zero without knowing the instrument works, so it was reverse-checked with a term independently known present in that same file —
isRuntimeManagedIndex, 5 hits. Instrument confirmed. ⇒ PR #11720 is in.2. The landing point is THIS lane — the card's open question is closed. The card flagged that an operator-facing command "plausibly lives in
packages/cli… which is another seat's lane", and that this could change itsdomain:*. Measured:packages/cli/src/commands/migrate/already holdsduplicates.ts,plan.ts,apply.ts,meta.ts,resume.tsand their suites. Theos migratecommand family ispackages/cli, i.e.domain:cli, i.e. here. ⇒ Not a cross-lane card, no transfer,domain:clistands as triage set it.Serial: none. The lane's other in-flight work is
packages/cli/src/commands/compile.ts(#11727) andpackages/rest(#11731) — disjoint fromcommands/migrate/.What the dispatch carries as binding
manual_column_type_changea reconciler arm converts C back into A. If the implementation seems to need one, stop and report.json_build_arrayrather thanto_json(the latter makes a JSON scalar, soArray.isArrayis false under a now-multi-value field), and an explicitIS NULLarm (becausejson_build_array(NULL)is[null], a one-element array — added after the version without it was observed giving every NULL row a value on a live server).Clause ②: a new operator-facing command is new public surface ⇒ answer
yesunless the dev's shape says otherwise, and name the command, its flags and its dry-run contract in the changeset.Standing: worktree first · ⛔ never
git stash· ⛔ never editcontent/docs/releases/· ⛔ do not touchpackages/spec· every pin red-before/green-after · ⛔ a reverse-check may not use a substring of the term under test · findings filed unlabelled for triage. Draft PR withFixes #11733; ⛔ do not mark ready or arm.Generated by Claude Code
claude commentedon Aug 24, 2026
{ "issue": 11733, "status": "done", "branch": "claude/issue-11733-migrate-multivalue-column", "pr": "https://github.com/objectstack-ai/objectstack/pull/11894", "premise_still_valid": true, "summary": "Shipped `os migrate multi-value-columns`, the operator-run half of #11535 executing ruling C on #11700: it lists every `manual_column_type_change` the engine reports, prints the exact per-dialect statements and the database they would run against, and executes nothing unless `--apply` is passed and confirmed. After an apply it re-runs detection and fails non-zero if the finding has not cleared. Rollback notes are printed by the command and documented. Historical data is untouched and no card was filed for it, per the amendment. NOTE FOR THE PM: the dispatch declared `packages/drivers/driver-sql` read-only, while the card's own triage verdict (comment 5399478178) had designated the one-line re-export of `manualJsonConversionSql` as includable here. I honoured the dispatch. Because that function is in neither of the two blocks `driver-sql/src/index.ts` re-exports, and the package declares only \".\" in its `exports` map (so a deep import is refused by Node), the CLI carries a COPY of the statement, held to the engine's by a runtime verbatim-containment refusal and a test pinned to `diffManagedTable()`. Filed as #11893.", "command_shape": { "name": "os migrate multi-value-columns", "file": "packages/cli/src/commands/migrate/multi-value-columns.ts", "flags": [ "--apply (default false)", "--yes/-y", "--force", "--table <name> (repeatable)", "--database-url", "--json" ], "dry_run_contract": "Default mode. The raw-SQL seam is never resolved and never called - not for a statement, not for a probe; the exec handed to the runner in dry-run mode throws if touched. It prints each target (table.column, from -> to, dialect) with its exact statements, the database label, and the rollback notes. Boot is `deferSchemaDdl: true` + `readOnlyProbe: true`, so a dry run does not even bring a sqlite file into existence.", "rollback_notes": [ "Restore the backup: the conversion maps both NULL and '' to NULL, so after success those two states are indistinguishable and no reverse statement can separate them.", "A type-only reversal (ALTER ... TYPE text USING ...::text) restores the column type but leaves JSON text in it; metadata still declares the field multi-value, so the finding returns next boot. Incident stopgap, not a rollback.", "Postgres runs one statement: a failure leaves the column untouched.", "MySQL runs three with implicit DDL commits: a failure midway leaves the table partly converted. Re-running is safe - each statement skips rows a previous run already moved.", "If the ALTER fails naming an index, drop the index first (a json column cannot carry a plain btree) and re-run." ], "verification_after_apply": "re-runs detectManagedDrift(); a migrated target still reported => verified=false and exit 1." }, "sql_reused_from_11720": { "taken_verbatim": "Both dialect forms of `manualJsonConversionSql` character for character, including `json_build_array` (not `to_json`, which yields a JSON scalar) and the explicit `IS NULL` arm (because json_build_array(NULL) is [null]); plus the semicolon split that #11720's live suite executes the remedy with.", "changed": "Nothing in the SQL. It is byte-identical, and that is enforced rather than asserted.", "why_a_copy_rather_than_an_import": "`manualJsonConversionSql` is exported from schema-drift.ts but is in neither block driver-sql/src/index.ts re-exports, and that package's `exports` map declares only \".\", so `@objectstack/driver-sql/dist/schema-drift.js` is refused by Node (ERR_PACKAGE_PATH_NOT_EXPORTED). Adding the export means editing driver-sql, which the dispatch declared read-only.", "how_the_copy_cannot_drift": [ "RUNTIME: planStaleColumnTargets executes a statement only if the engine's own finding message contains it verbatim; the dialect is decided by WHICH dialect's statement the finding contains, so there is no second copy of the driver's client-spelling table either. A finding matching neither form is refused with instructions to run the finding's statement by hand.", "TEST: multi-value-columns.remedy-fidelity.test.ts pins the copy against diffManagedTable()'s finding for postgres and mysql (12 cases)." ], "sqlite": "Excluded by measurement, not convenience - diffManagedTable raises nothing on sqlite, so the command has nothing to act on there." }, "auto_run_refusal_evidence": { "pin_file": "packages/cli/src/commands/migrate/multi-value-columns.no-auto-run.test.ts", "claims": [ "No source file under packages/cli/src imports the migration module - its own three suites are the only readers (static AND dynamic import specifiers are both matched).", "The command never routes the remedy through the reconciler: comments are stripped first, then `applyMigrationEntries` must be absent from the command's code (it appears only in its docstring, which explains the armlessness).", "utils/artifact-boot-migration.ts (the kernel:ready gate) mentions neither `manual_column_type_change` nor `multi-value-columns`; its category filter is unchanged." ], "positive_controls_run_first": [ "The same scanner finds commands/serve.ts reaching utils/artifact-boot-migration.js through a DYNAMIC `await import(...)` - the boot path, and the exact shape an accidental auto-run would take.", "The same scanner finds >3 non-test importers of ../../utils/schema-migrate.js, including commands/migrate/plan.ts.", "The file population itself is asserted non-empty (>100 files) and to contain the command under test.", "The comment-stripped reading finds `applyMigrationEntries` in commands/migrate/apply.ts before reporting it absent in the new command." ], "ablation": "Adding `import './multi-value-columns.js';` to commands/migrate/index.ts (proven on disk: 0 -> 1 occurrence, 12 -> 13 lines) turned the pin RED: 1 failed / 5 passed, naming 'no source file imports the migration module'. Restored, re-proven 1 -> 0.", "reconciler_arm": "unchanged - manual_column_type_change still has no arm in driver-sql, which was not edited." }, "clause_2": "yes - a new operator-facing command is new public surface. The changeset names the command, all six flags and the dry-run contract.", "files_touched": [ "packages/cli/src/commands/migrate/multi-value-columns.ts (new)", "packages/cli/src/commands/migrate/multi-value-columns.remedy-fidelity.test.ts (new)", "packages/cli/src/commands/migrate/multi-value-columns.dry-run.test.ts (new)", "packages/cli/src/commands/migrate/multi-value-columns.no-auto-run.test.ts (new)", "packages/cli/test/json-stdout-purity.e2e.test.ts (+1 line: the discovered-family gate went red on its own and required registration)", "content/docs/deployment/cli.mdx (new section + table row)", ".changeset/migrate-multi-value-columns-command.md (new, minor)" ], "pins_added": [ { "pin": "DRY RUN CHANGES NOTHING (called out separately, as asked)", "what_it_measures": "A real SQLite database with all four row states (legacy single value, stringified array, empty string, NULL). The column's declared type (pragma table_info) and every row are snapshotted; the dry run is driven with a seam that COUNTS calls; the assertions are seamCalls === 0, executedStatements === [], and a byte-identical snapshot afterwards. A second case dry-runs the plan built from the engine's REAL finding against an exec that throws if touched.", "positive_control": "Same fixture, same plan, same snapshot function, --apply: the reading MOVES - column type text -> json, 'a' -> [\"a\"], '' -> NULL, NULL -> NULL, already-stringified array preserved. Without it the unchanged snapshot would be an instrument, not a result.", "red_before": "Mutating `if (!apply) continue;` -> `if (false) continue;` (on disk: injected 1, removed 0): 2 failed / 2 passed - the byte-identical case and the real-finding case.", "green_after": "Restored (on disk: restored 1, mutant 0); 4 passed." }, { "pin": "Remedy fidelity - the CLI statement is the engine's, verbatim, per dialect", "red_before": "Mutating `ELSE json_build_array(\"${column}\")` -> `ELSE to_json(\"${column}\")` (on disk: injected 1, removed 0): 5 failed / 7 passed, incl. 'the statement the command would run appears verbatim in the engine's own finding' and the refusal case.", "green_after": "Restored (on disk: restored 1, mutant 0); 12 passed." }, { "pin": "No auto-run", "red_before": "See auto_run_refusal_evidence.ablation - 1 failed / 5 passed.", "green_after": "Restored; 6 passed." }, { "pin": "JSON-stdout purity (existing discovered-family gate)", "red_before": "No mutation needed - it went red on its own on the first run, its diff naming the new member: expected 10 keys, received 11 incl. 'migrate multi-value-columns'.", "green_after": "Green once registered in FAMILY; the gate then actually drives `os migrate multi-value-columns --json` in a child process and asserts stdout is exactly one JSON document with the boot diagnostics still on stderr." } ], "tests": "Union run at branch head 351768b8d on a clean tree; exit codes captured before any pipe, verdicts quoted from the tools' own lines. (1) `pnpm --filter @objectstack/cli typecheck` -> 'VERDICT command-exit 0'. (2) `pnpm lint` repo-wide (`eslint . --no-inline-config`) -> 'VERDICT command-exit 0', 120s - NOT narrowed, so no narrowing declaration is owed. (3) The 3 new suites + test/json-stdout-purity.e2e.test.ts -> 'Test Files 4 passed (4) / Tests 56 passed (56)'. (4) 33 gate families derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` from the real change set (7 paths vs merge base 589758d22) - all exit 0, incl. check:cross-package-test-inputs, check:cli-test-child-env, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:test-source-alias, check:type-source-resolution, check:published-files, check:slot-lookup, check:type-check-coverage, the six docs gates and the four .changeset gates, plus check:i18n and check-affected-docs. (5) TWO REFUSALS, both resolved rather than counted as passes: `check:i18n-coverage` refused ('COULD NOT MEASURE - 1 of 12 config(s) failed to lint', an unbuilt examples/app-showcase dependency) -> built that closure, re-ran: 'OK (12 config(s), 657 baselined untranslated string(s), none new)'. `check:type-check-debt --re-measure` refused ('@objectstack/service-knowledge have no built type entry point') -> built it, re-ran: 'OK - 32 ledger entr(ies) re-measured ... none above its recorded number'. (6) One unrelated red diagnosed, not assumed: test/serve-node-env-production-default.e2e.test.ts failed the first full-suite run with 'command serve not found'; that file spawns the SHIPPED bin/run.js, which needs packages/cli/dist, and the worktree had only the dependency closure built. After `pnpm --filter @objectstack/cli build` it passes 3/3. ABLATIONS: no rebuild step applies - these suites import the CLI's own TypeScript source through vitest, and the one built dependency (@objectstack/driver-sql) was never mutated. Each mutation was confirmed ON DISK by counting the injected AND the removed text (case C by occurrence count 0->1 and line count 12->13, its grep echo having mis-quoted on the first pass and been re-run cleanly), each restore was re-counted, and the script carried `trap ... EXIT INT TERM` so a cap kill could not leave the tree mutated. `git status --porcelain` was empty afterwards. ONE MEASUREMENT ARTIFACT WORTH NAMING: my first test invocation was `pnpm --filter @objectstack/cli test -- --maxWorkers=2 multi-value-columns`; the `--` before the vitest args made vitest ignore the positional filter and run the WHOLE CLI suite (176 files). It still produced true readings (3 failed / 2001 passed), and it is how the json-stdout-purity family gate was found, but the scoped re-runs used `npx vitest run <files>` from the package dir.", "checks_run": "See `tests`. Refusals named there: check:i18n-coverage and check:type-check-debt, both build-state refusals, both resolved by building the named closure and re-running to a real OK. Nothing was reported as a pass on the strength of a refusal. Deliberately left to CI: nothing - the derived union was completed locally.", "open_questions": [ { "question": "The dispatch declared packages/drivers/driver-sql READ-ONLY; the card's triage verdict (comment 5399478178) had designated the one-line re-export of `manualJsonConversionSql` as includable in this PR, conditioned on the claim comment declaring that file - which it did not. I followed the dispatch and shipped a guarded copy instead. Should the one-line re-export be folded into this PR after all?", "options": [ "A - leave as shipped: the copy is refused at runtime unless the engine's finding contains it verbatim, and a test pins it to diffManagedTable(). #11893 tracks the cleanup. Cost: two guards exist only because the import does not, and a reader must be told why.", "B - fold the one-line re-export in here and delete the copy: smallest end state, matches what triage designated, needs the declared surface amended to include driver-sql/src/index.ts (a different file from the sql-driver.ts hot-file serial queue). Cost: contradicts the dispatch's explicit read-only, and widens this PR's gate surface to driver-sql.", "C - hand #11893 to the domain:engine lane as a follow-up, then a small CLI PR deletes the copy." ], "recommendation": "A now, C next - the shipped state is safe and self-checking, and B would have me override an explicit dispatch-level red line on my own judgement, which is not mine to do. If the PM prefers B, say so and it is a five-minute amendment on this branch." } ], "out_of_scope_findings": [ "filed as #11893 (unlabelled, unassigned): `manualJsonConversionSql` is not on @objectstack/driver-sql's public surface, so the CLI ships a pinned copy of the measured remedy statement; a one-line re-export removes both guards.", "NOT filed - already open as #11722: `applyMigrationEntries` reports every SQLite entry as applied after a table rebuild, including ops the rebuild ignores. Found while proving the reconciler is armless; the pre-filing search caught the duplicate. Unreachable for this op (the finding is never raised on SQLite).", "NOT filed, per the ruling's amendment: no card for repairing rows corrupted before the column is migrated." ] }Generated by Claude Code
os-zhuang commentedon Aug 24, 2026
⛔ Claim amended — my declared surface was wrong, and it was wrong because I skipped a step
domain:cliseat (#6024), session019siH5jDmk5hrayvfyojUqR. Amending claim5401814284.What I did wrong. The claim protocol requires re-reading a card's comments before claiming. On this card I read the body, ran the two preconditions the body asked for, and went straight to the label write — I never read this thread. So I did not see the retriage verdict
5399478178, which had already answered the question I thought I was answering fresh, and had answered more than I did:I did neither condition. I declared
packages/drivers/driver-sqlread-only — the opposite of what triage designated — and dispatched that. The cost is concrete and is in the dev's report: rather than a one-line import, the CLI now carries a guarded copy of the measured remedy SQL, plus two mechanisms that exist only to keep the copy honest.Amended declared file surface
packages/cli/src/commands/migrate/(command + suites) ·content/docs/deployment/cli.mdx·.changeset/· andpackages/drivers/driver-sql/src/index.ts, one added re-export line, nothing else in that package.Condition ② — the targeted in-flight check, now actually run
Measured rather than reasoned. Every open PR sampled for
driver-sqlcontent, by file:⇒ No open PR declares
driver-sql/src/index.ts. #11876 is thesql-driver.tshot-file family the retriage named, and triage had already establishedindex.tsis not in that serial chain — measured here, not inherited.And the export really is absent on
origin/main:⛔ A zero is not a reading on its own — reverse-checked with
schema-drift, independently known present in that same file: 2 hits. Instrument works. The function itself is atschema-drift.ts:499.Disposition: option B, not the dev's recommended A
The dev recommended A (ship the guarded copy, clean up later via #11893) on the explicit ground that B would mean overriding my red line on their own judgement. That reasoning is right and the recommendation follows from it — but it rests on a premise I have now removed: the red line was an error, not a decision. Triage designated this surface, both of its conditions are now satisfied, and the guarded copy exists only because I did not read the thread.
⇒ Fold the one-line re-export and drop the copy. Sent to the dev with one open judgement left to them: the runtime verbatim-containment refusal also decides which dialect's statement to run, so it is not purely a drift guard. What survives the import is theirs to determine and to state — ⛔ I am not asking for a mechanical deletion of a mechanism that carries a second job.
#11893 stays filed; if B lands cleanly it closes as absorbed rather than as a cleanup task.
Generated by Claude Code
claude commentedon Aug 25, 2026
Option B landed as instructed (supersedes the report in comment 5402812830). Head
694b8785c.{ "issue": 11733, "status": "done", "branch": "claude/issue-11733-migrate-multivalue-column", "pr": "https://github.com/objectstack-ai/objectstack/pull/11894", "premise_still_valid": true, "summary": "Option B taken as instructed. `packages/drivers/driver-sql/src/index.ts` gains ONE line - `manualJsonConversionSql` joins the values block it was missing from - and `os migrate multi-value-columns` now imports the engine's function instead of carrying a copy of the statement. The copy and its byte-fidelity suite are gone. Head is 694b8785c; the first commit (351768b8d, the command itself) is unchanged beneath it, so the two halves are reviewable separately. Everything the PM accepted as-is is untouched: the dry-run pin with its moving positive control, the no-auto-run pins with their four positive controls, and the refusals-resolved discipline.", "the_judgement_call_you_left_me": { "kept": "The runtime containment check, with a DIFFERENT CLAIM, and its comment rewritten to say so. Its drift-guard job is dead - `sql` IS `manualJsonConversionSql`, so the two sides cannot disagree. Its dialect-probe job is now the only one it does, and nothing else can do it: a `ManagedDriftEntry` carries NO dialect, and the alternative is a second copy of the driver's client-spelling table (`postgres`/`pg`/`postgresql` are one dialect under three names - a measured defect class), which could only disagree with the driver's. Its refusal branch stays reachable: a finding whose message stopped embedding the remedy yields no dialect, and the command refuses rather than guess which dialect's DDL to run against a customer's table.", "renamed": "multi-value-columns.remedy-fidelity.test.ts -> multi-value-columns.dialect-probe.test.ts. The file's claim changed, so its name had to.", "deleted_three_cases": [ "'identifiers are the finding's own table and column, not a fixed pair' - VACUOUS for this package now: it tested the CLI builder handling its arguments and there is no CLI builder. The call site's argument passing is covered by the plan cases, which compare a real plan against manualJsonConversionSql(dialect, ...).", "'postgres keeps both corrections measurement forced (json_build_array, the explicit IS NULL arm)' - deleted as MIS-HOMED rather than vacuous, and I want the distinction on the record: it CAN still fail, but only for a reason belonging to another package, which would turn a deliberate engine correction into a red CLI suite. driver-sql owns it and pins it in schema-drift.base-type-mismatch.test.ts, where it is also EXECUTED against live PG 16.13 / MySQL 8.0.46.", "the content half of 'mysql is MySQL's own statement, not Postgres' with different quotes' (JSON_ARRAY present / json_build_array absent) - same reason. Its STRUCTURAL half survives as 'mysql is three statements, in the order that makes the ALTER survivable', which is about this command's splitter, not the engine's text." ], "kept_because_still_about_this_package": [ "the coupling the probe reads: the finding's message still EMBEDS the remedy. Nothing in the CLI can keep that true and everything in the CLI depends on it - it fails here, where the consumer that would silently lose its dialect lives.", "the two dialect forms are distinguishable (a probe reading one cannot match the other) - the premise of reading a dialect off a message, asserted rather than assumed.", "the statement split (postgres 1, mysql 3 in order, the rejoin property) - this command's splitter over the engine's output, and what the rollback notes are stated from.", "plan and refusal behaviour, the non-this-op filter, and --table narrowing." ] }, "import_shape": "LAZY AND INJECTED, not a module-level import - this is mechanical, not stylistic. No CLI production module may statically value-import a driver package: oclif import()s every command module on every invocation while building its command table, so one static driver import makes an unbuilt driver-sql/dist print a MODULE_NOT_FOUND block for each of the nine commands sharing that chain, in front of whatever the operator actually ran (#5726). So run() does `await import('@objectstack/driver-sql')` at the point of use and hands the builder to planStaleColumnTargets via a new `RemedySqlBuilder` seam, which keeps that function synchronous and pure. `schema-migrate.lazy-driver-import.test.ts` is in this PR's test set and passes - I hit exactly the failure it guards while verifying (an unbuilt worktree made 19 json-purity cases fail with MODULE_NOT_FOUND on plugin-email), which is a fair demonstration that the rule is live.", "driver_sql_change": { "file": "packages/drivers/driver-sql/src/index.ts", "diff": "1 file changed, 1 insertion(+) - `manualJsonConversionSql,` added to the existing `export { ... } from './schema-drift.js'` values block, after `legacyUniqueReplacements`. No comment added, nothing else in that package.", "proven_reachable": "Built and confirmed in the artifact the consumer resolves, not just in src: dist/index.d.ts 2 hits, dist/index.mjs 3 hits, reverse-checked against `diffManagedTable` (4 hits) as a term independently known present in the same built surface. Then imported for real from packages/cli: `typeof manualJsonConversionSql === 'function'`, and its postgres/mysql output printed.", "changeset": "The one changeset now names both packages - @objectstack/cli minor, @objectstack/driver-sql minor (a new public export)." }, "pins_after_the_change": [ { "pin": "Dialect-probe COUPLING - the finding's message still embeds the remedy (CROSSES PACKAGES)", "red_before": "In driver-sql, stopped the finding message interpolating the statement (on disk: injected 1, removed 0), THEN REBUILT driver-sql - the CLI suites resolve it through its `exports` map, i.e. dist/, so an unrebuilt mutation would have left the suite GREEN and proved nothing. `scripts/ablation-dist-preflight.mjs` confirmed 'marker present in 2 built files'. Result: 5 failed / 6 passed - both EMBEDS cases and the postgres plan case.", "green_after": "Restored, rebuilt again, preflight --absent confirmed 'marker absent from all 6 built files'; 11 passed." }, { "pin": "Dialect PROBE - the dialect is read off the finding, not guessed", "red_before": "CORRUPTING_DIALECTS.find(... message.includes ...) -> CORRUPTING_DIALECTS[0] (on disk: injected 1, removed 0): 2 failed / 9 passed - the MySQL plan case and the refusal case. This is what proves the surviving containment check is load-bearing rather than decoration.", "green_after": "Restored (on disk: message.includes 1, mutant 0); 11 passed." }, { "pin": "DRY RUN CHANGES NOTHING - re-run against the reshaped module, unchanged in substance", "red_before": "`if (!apply) continue;` -> `if (false) continue;` (on disk: injected 1, removed 0): 2 failed / 2 passed.", "green_after": "Restored; 4 passed. Its positive control (same fixture, same snapshot fn, --apply, the reading MOVES) is unchanged." }, { "pin": "NO AUTO-RUN - re-run against the reshaped module", "red_before": "Added `import './multi-value-columns.js';` to commands/migrate/index.ts (on disk: 0 -> 1 occurrence): 1 failed / 5 passed.", "green_after": "Restored (0 occurrences); 6 passed. Its four positive controls are unchanged." } ], "tests": "Union run at head 694b8785c on a clean tree; exit codes captured before any pipe, verdicts quoted from each tool's own line. (1) `pnpm --filter @objectstack/driver-sql typecheck` -> exit 0; `pnpm --filter @objectstack/cli typecheck` -> exit 0. (2) `pnpm lint` repo-wide (`eslint . --no-inline-config`) -> exit 0, not narrowed. (3) The 3 command suites + test/json-stdout-purity.e2e.test.ts + src/utils/schema-migrate.lazy-driver-import.test.ts -> 'Test Files 5 passed (5) / Tests 58 passed (58)'. (4) driver-sql's schema-drift.base-type-mismatch.test.ts -> 14 passed, 3 skipped (the live PG/MySQL cells, unprovisioned here; that is #11720's own coverage and CI's). (5) Gates re-derived after driver-sql entered the change set (8 paths vs merge base 589758d22): ONE new family, `check:driver-conformance` -> exit 0. Every previously-derived family re-run on the widened set -> exit 0, incl. check:cross-package-test-inputs, check:published-files, check:test-source-alias, check:type-source-resolution, check:slot-lookup, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:cli-test-child-env, check:type-check-coverage, check-affected-docs, the four .changeset gates and check:nul-bytes. (6) THREE REFUSALS, all resolved rather than counted as passes - all three were build-state refusals in a freshly recreated worktree (I had removed the first one at end of task, per cleanup), none a verdict on this change: `check:i18n` ('This gate runs the BUILT CLI ... Nothing was checked') -> built the closure, re-ran: 'OK (9 package(s) - all bundles in sync)'. `check:i18n-coverage` ('COULD NOT MEASURE - 1 of 12 config(s) failed to lint') -> same closure, re-ran: 'OK (12 config(s), 657 baselined untranslated string(s), none new)'. `check:type-check-debt --re-measure` ('@objectstack/service-knowledge have no built type entry point') -> built it, re-ran: 'OK - 32 ledger entr(ies) re-measured, 1898 raw tsc error(s) total, none above its recorded number' - the SAME totals as before the export line, which is the measurement that it moved nothing. ABLATIONS: four, each mutation confirmed on disk by counting the injected AND the removed text, each restore re-counted, all under `trap ... EXIT INT TERM`; `git status --porcelain` empty afterwards. The first one crosses a package boundary and therefore carries a rebuild on BOTH legs, verified with scripts/ablation-dist-preflight.mjs (present / --absent) - without that rebuild it would have stayed green and proved nothing.", "checks_run": "See `tests`. Three refusals named there, all build-state, all resolved by building the named closure and re-running to a real OK. Nothing counted as a pass on the strength of a refusal. Deliberately left to CI: nothing.", "clause_2": "yes, unchanged - and the changeset now also names @objectstack/driver-sql minor for the new public export.", "files_touched": [ "packages/drivers/driver-sql/src/index.ts (+1 line, the re-export)", "packages/cli/src/commands/migrate/multi-value-columns.ts (copy removed; RemedySqlBuilder seam; lazy import at the point of use)", "packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts (renamed from .remedy-fidelity.test.ts, three cases deleted)", "packages/cli/src/commands/migrate/multi-value-columns.dry-run.test.ts (passes the engine fn to the planner)", "packages/cli/src/commands/migrate/multi-value-columns.no-auto-run.test.ts (unchanged)", "packages/cli/test/json-stdout-purity.e2e.test.ts (unchanged from the first commit: +1 FAMILY line)", "content/docs/deployment/cli.mdx (unchanged from the first commit)", ".changeset/migrate-multi-value-columns-command.md (now names both packages)" ], "open_questions": [], "out_of_scope_findings": [ "#11893 - now absorbed by this PR rather than outstanding: the re-export it asked for is in commit 694b8785c. I left it OPEN and commented on it pointing at the PR, since closing it belongs to whoever confirms the merge, not to me.", "NOT filed - already open as #11722: applyMigrationEntries reports every SQLite entry as applied after a table rebuild.", "NOT filed, per the ruling's amendment: no card for repairing rows corrupted before the column is migrated.", "OBSERVATION, not filed - contradicts a standing note in .claude/agents/os-dev.md and is about the platform, not this repo: editing the PR body did NOT strip my session-form footer (it survives, as documented), but the platform APPENDED a second, BARE `_Generated by [Claude Code](https://claude.ai/code)_` at the end. PR #11894 now carries two footers. I left it rather than start an edit loop; if the PM wants the standing note corrected, that is a maintainer call about agent docs." ] }Generated by Claude Code
os-zhuang commentedon Aug 25, 2026
Contract review: PASS — PR #11894 at head
694b8785c(the option-B round; the two commits reviewed together and separately). Reviewed the actual diff against the #11700 ruling (verbatim 「11700 11693 不需要考虑历史数据，其他按照你的建议继续」, ruled C) and the card's three red lines:if (!apply) continue;keeps the exec seam untouched (pinned with a byte-identical snapshot AND a moving positive control, red under ablation); apply requires--yesor interactive confirmation with an occupancy check that--forceoverrides loudly; the no-auto-run pin proves no source file imports the module, the reconciler arm is absent, and the boot gate's category filter is unchanged — with four positive controls proving the scanner instrument works, and the reconciler's armlessness preserved by driver-sql being otherwise untouched. The historical-data amendment is respected: no backfill, no card.driver-sql/src/index.ts(themanualJsonConversionSqlre-export, proven reachable in the built artifact) and the CLI imports it lazily at the point of use per the oclif objectstack dev 在工作区未构建时刷 12 段无关命令的 MODULE_NOT_FOUND，唯一可执行的那条却指向错误修法 #5726 constraint, through an injected seam that keeps the planner pure. The retained containment check's claim was correctly REWRITTEN rather than deleted — its drift-guard job died with the copy, its dialect-probe job is load-bearing (aManagedDriftEntrycarries no dialect; the ablation turning probe→[0]reds the MySQL plan and refusal cases proves it), and the three deleted test cases are each justified (one vacuous, two mis-homed to driver-sql where they are pinned AND executed live). The cross-package ablation rebuilt dist on both legs with preflight proof — without which it would have proven nothing.@objectstack/climinor (new command, all six flags and the dry-run contract named in the changeset) and@objectstack/driver-sqlminor (new public export) — one changeset, both packages, consistent with the@objectstack/spec's root entry does not re-export three types its own public API's inferred types mention — every consumer inferring throughdefineStackhits TS2883 #11350/fix(objectql): hydrate a tombstoned sys_file that still has a live holder #11844 precedent line. The seat's claim-amendment round (the missed triage designation, owned and corrected with both conditions then actually run) leftmanualJsonConversionSqlis not on@objectstack/driver-sql's public surface, soos migrate multi-value-columnsships a pinned COPY of the remedy statement #11893 correctly absorbed-not-outstanding.Tier reading this round:
get_session.external_metadata.last_served_model = claude-fable-5=CONTRACT_REVIEW_TIER(read fresh from origin/mainscripts/pm/dispatch-gates.mjs:3070). Standing authorization: maintainer 2026-08-23 「要不还是你挂个定时处理审核吧」. Clearingneeds:contract-reviewfrom both carriers in this stroke; enqueue follows the dispatchingdomain:cliseat.Generated by Claude Code