From 738019efdbff7bb0cc89458a71d24fe998a44879 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 11:37:44 +0000 Subject: [PATCH 1/2] fix(cli): judge --code before the probe, prompt and boot in os migrate record-currency-codes A code outside ISO 4217 is refused with the backfill's INVALID_FIELD / 400 envelope before the occupancy probe, the --apply prompt and bootSchemaStack. The engine-side check is unchanged. Document the command in cli.mdx and the record-mode switch (409 destructive_change, dry run, --apply) in currency.mdx. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QUs3BvyNBMezKJUd7aydyG --- .../28168-record-currency-code-before-boot.md | 10 ++ content/docs/data-modeling/currency.mdx | 22 ++++ content/docs/deployment/cli.mdx | 38 +++++++ .../migrate/record-currency-codes.test.ts | 106 +++++++++++++++--- .../commands/migrate/record-currency-codes.ts | 49 +++++++- 5 files changed, 206 insertions(+), 19 deletions(-) create mode 100644 .changeset/28168-record-currency-code-before-boot.md diff --git a/.changeset/28168-record-currency-code-before-boot.md b/.changeset/28168-record-currency-code-before-boot.md new file mode 100644 index 00000000000..576c008c8cc --- /dev/null +++ b/.changeset/28168-record-currency-code-before-boot.md @@ -0,0 +1,10 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate record-currency-codes` refuses a `--code` outside ISO 4217 before it probes the database, prompts or boots + +Clause-②: no +Metadata-protocol: none + +A code that is not an uppercase ISO 4217 code (`usd`, `EURO`) used to be refused only by the backfill, after the app had booted, and an `--apply` boot creates missing tables and columns before that refusal. The command now judges `--code` first, with the same predicate the backfill uses, and answers with the same `INVALID_FIELD` / 400 envelope (`code` and `httpStatus` under `--json`) and exit 1. Nothing is probed, prompted or booted for a bad code. The backfill in `@objectstack/objectql` keeps its own check unchanged. diff --git a/content/docs/data-modeling/currency.mdx b/content/docs/data-modeling/currency.mdx index e489f45c61e..126c510cec8 100644 --- a/content/docs/data-modeling/currency.mdx +++ b/content/docs/data-modeling/currency.mdx @@ -122,6 +122,28 @@ as an explicit value. A `defaultValue` you declare on the code field yourself is ordinary field default: the engine applies it before the check, so it satisfies the rule, because you wrote it. +### Switching a field that already holds rows + +An existing `dynamic` or `fixed` amount can be switched to `currencyMode: 'record'`, +but every row already stored then holds an amount beside a blank code. Such a row is +not refused for resting, but it shows as a plain number, falls into the `null` group of +a grouped aggregate, and sits outside every filter on the pair until it holds a code. + +1. **The save answers 409 `destructive_change`.** Saving the switched object reports + the finding `currency_mode_to_record` on the amount field, and the message names the + command to run. Re-submit the save with `force` (`?force=true` on + `PUT /api/v1/meta/:type/:name`) to make the switch. +2. **Dry run.** `os migrate record-currency-codes --object opportunity --field amount --code USD` + counts the rows holding an amount and no code, and writes nothing. +3. **`--apply`.** The same command with `--apply` writes the code into those rows. It + is idempotent: re-run it until the report says zero. + +The code is the one you give with `--code`, and it is never derived: not from the +`defaultCurrency` the field had under `fixed`, not from the organization's currency. +One run writes one code, so if the stored amounts are in more than one currency, split +those rows with your own writes first. See +[`os migrate record-currency-codes`](/docs/deployment/cli#os-migrate-record-currency-codes). + ### Aggregating a record-mode amount Amounts in different currencies do not add. A `sum`, `avg`, `min` or `max` over a diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index bdda74dc7bf..aba22abbe8b 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1302,6 +1302,7 @@ where the data lives. | `os migrate files-to-references` | Convert legacy file-field values to `sys_file` references, verify the ownership ledger, and record the deployment's migration flag | | `os migrate value-shapes` | Scan stored reference and structured-JSON field values against the platform's value contract, and record the deployment's migration flag when clean | | `os migrate summary-nulls` | Backfill roll-up `count` / `sum` columns still stored as `NULL` on parent rows created before the insert-time seed. Repairs values; no flag, nothing depends on it having run | +| `os migrate record-currency-codes` | Write the currency code you give into the rows of a record-mode currency field that hold an amount and no code, the rows stored before the field was switched to `currencyMode: 'record'`. The code is never derived. No flag; idempotent. See [below](#os-migrate-record-currency-codes) | | `os migrate comment-reactions` | Carry the emoji reactions a 17.x deployment stored in the retired `sys_comment.reactions` column into `sys_comment_reaction` records, list every stored value that does not read, and record the verified run that lets `os migrate apply --allow-destructive` drop the column | | `os migrate organization-ownership` | The v18 ownership ceremony (ADR-0131 D10): write a plan of what each table's rows will become, then `--apply` that reviewed plan. It upgrades a database 17.x or an 18.0 pre-release (`18.0.0-next.*`) created; a v18 server refuses to start on one until it has run. See [below](#os-migrate-organization-ownership) | | `os migrate activity-changed-fields` | Stamp the two read-rule columns (`changed_fields`, `milestone_key`) on the `sys_activity` rows written before they existed. Until it runs, those rows are withheld from every reader but the system, so run it at once after upgrading. No flag; idempotent | @@ -1504,6 +1505,43 @@ A deployment whose database was seeded fresh on this version has nothing to do here: its parents were created with the value already in place, and the run reports zero. +#### `os migrate record-currency-codes` + +Switching an existing currency field to `currencyMode: 'record'` leaves every row +already stored holding an amount beside a blank code. Nothing refuses such a row for +resting, but until it holds a code it shows as a plain number, falls into the `null` +group of a grouped aggregate, and sits outside every filter on the amount and its +code. The save that makes the switch answers 409 `destructive_change` naming this +command ([Currency fields](/docs/data-modeling/currency#switching-a-field-that-already-holds-rows) +covers the switch). This command writes the code into those rows. + +```bash +os migrate record-currency-codes --object opportunity --field amount --code USD # Dry run: counts the rows, writes none +os migrate record-currency-codes --object opportunity --field amount --code USD --apply # Write the code (prompts) +os migrate record-currency-codes --object opportunity --field amount --code USD --apply --yes --json # CI / scripts +``` + +**The code is yours, never derived.** `--object`, `--field` and `--code` are all +required. One run writes the one code you give into every row of the field whose +amount is set and whose code is blank. It never reads a code from a `defaultCurrency` +the field had before, from the organization's `localization.currency`, or from +anything else. A row that holds a code is never touched, whatever the code, and a row +with no amount owes none. If the stored amounts are in more than one currency, split +them with your own writes first: nothing here can tell those rows apart. + +**A bad `--code` is refused before anything is opened.** A code that is not an +uppercase ISO 4217 code (`usd`, `EURO`) is refused with `INVALID_FIELD` / 400 and +exit 1 before the database is probed, the `--apply` prompt is shown or the app is +booted. An object your deployment does not declare (`OBJECT_NOT_FOUND`), or a field +that is not a record-mode currency field (`INVALID_FIELD`), is refused after the boot +and before any row is read. + +Each row is written through the engine like any other update, so the record-mode +write rule and the code field's `valueDomain` judge it, and it is audited. Idempotent: +every write turns a blank code into a code, so a second run finds nothing to write. +Re-running until the report says zero *is* the verification, which is why this +command records no flag. + #### `os migrate comment-reactions` v18 retired `sys_comment.reactions`, the JSON column that held every emoji diff --git a/packages/cli/src/commands/migrate/record-currency-codes.test.ts b/packages/cli/src/commands/migrate/record-currency-codes.test.ts index 77700dd35a4..bd965d96c7c 100644 --- a/packages/cli/src/commands/migrate/record-currency-codes.test.ts +++ b/packages/cli/src/commands/migrate/record-currency-codes.test.ts @@ -8,8 +8,10 @@ * `record-currency-backfill.test.ts`. What is pinned here is what a unit test * of the backfill cannot see: the command is dry-run-by-default, its three * naming flags are required, they reach `backfillRecordCurrencyCodes` as given, - * a refusal reaches the `--json` envelope with its code, and the human face's - * dry-run line is followed by the journal-mode note on a SQLite target only. + * a refusal reaches the `--json` envelope with its code, a code outside ISO + * 4217 is refused before the occupancy probe, the `--apply` prompt and the boot + * (card #28168), and the human face's dry-run line is followed by the + * journal-mode note on a SQLite target only. * The seams that would boot a database or walk a real engine are replaced; the * command's own parse and control flow run for real, and the report's formatter * and the refusal are the real ones. @@ -31,6 +33,19 @@ vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ probeMigrationTarget: vi.fn(), })); vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: vi.fn(async () => []) })); +// The `--apply` confirmation: a prompt opened at all is recorded, and answered +// no, so a case that should never reach it can say so instead of hanging. +const prompt = vi.hoisted(() => ({ opened: 0 })); +vi.mock('node:readline', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createInterface: (() => { + prompt.opened++; + return { question: (_q: string, answer: (a: string) => void) => answer('n'), close: () => {} }; + }) as unknown as typeof real.createInterface, + }; +}); vi.mock('@objectstack/objectql', async (importOriginal) => { const real = await importOriginal(); return { @@ -134,29 +149,88 @@ describe('os migrate record-currency-codes', () => { expect(payload).toMatchObject({ code: 'OBJECT_NOT_FOUND' }); }, RUN_TIMEOUT); - it('a refused code reaches the --json envelope with its code and httpStatus, and exits 1', async () => { + it('a refusal from the backfill reaches the --json envelope with its code and httpStatus, and exits 1', async () => { const real = await vi.importActual('@objectstack/objectql'); - const recordEngine = { - ...engine, - getObject: () => ({ - fields: { amount: { type: 'currency', currencyConfig: { currencyMode: 'record', currencyField: 'currency_code' } } }, - }), - }; - const refusal = await real.backfillRecordCurrencyCodes(recordEngine, { info() {}, warn() {} }, { - object: 'opportunity', field: 'amount', code: 'usd', - }).then(() => { throw new Error('expected the code to be refused'); }, (e: unknown) => e as Error); + // A member code on a field that is not record-mode: the refusal only the + // booted engine can make, since it reads the object's declaration. + const plainEngine = { ...engine, getObject: () => ({ fields: { amount: { type: 'currency' } } }) }; + const refusal = await real.backfillRecordCurrencyCodes(plainEngine, { info() {}, warn() {} }, { + object: 'opportunity', field: 'amount', code: 'USD', + }).then(() => { throw new Error('expected the field to be refused'); }, (e: unknown) => e as Error); vi.mocked(backfillRecordCurrencyCodes).mockRejectedValue(refusal); - const err = await MigrateRecordCurrencyCodes.run( - ['--json', '--object', 'opportunity', '--field', 'amount', '--code', 'usd'], - { root: CLI_ROOT }, - ).catch((e: unknown) => e); + const err = await MigrateRecordCurrencyCodes.run(['--json', ...NAMING], { root: CLI_ROOT }).catch((e: unknown) => e); expect(isExitSignal(err)).toBe(true); expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1); const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join('')); expect(payload).toMatchObject({ code: 'INVALID_FIELD', httpStatus: 400, error: refusal.message }); }, RUN_TIMEOUT); + describe('a code outside ISO 4217 is refused before the probe, the prompt and the boot', () => { + /** The engine's own refusal of the same code, for the envelope and wording it is held to. */ + const engineRefusal = async (code: string): Promise => { + const real = await vi.importActual('@objectstack/objectql'); + const recordEngine = { + ...engine, + getObject: () => ({ + fields: { amount: { type: 'currency', currencyConfig: { currencyMode: 'record', currencyField: 'currency_code' } } }, + }), + }; + return real.backfillRecordCurrencyCodes(recordEngine, { info() {}, warn() {} }, { + object: 'opportunity', field: 'amount', code, + }).then(() => { throw new Error('expected the code to be refused'); }, (e: unknown) => e as Error); + }; + /** Up to the clause naming the field, which only the booted engine can name. */ + const leadingClause = (message: string): string => message.slice(0, message.indexOf(', so ')); + + beforeEach(() => { + vi.mocked(bootSchemaStack).mockClear(); + vi.mocked(probeMigrationTarget).mockClear(); + prompt.opened = 0; + }); + + it('--json --apply: the engine\'s INVALID_FIELD / 400 envelope, never confirmation_required, and nothing opened', async () => { + const err = await MigrateRecordCurrencyCodes.run( + ['--json', '--apply', '--object', 'opportunity', '--field', 'amount', '--code', 'usd'], + { root: CLI_ROOT }, + ).catch((e: unknown) => e); + expect(isExitSignal(err)).toBe(true); + expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1); + const payload = JSON.parse(stdout.mock.calls.map((c: unknown[]) => String(c[0])).join('')); + const refusal = await engineRefusal('usd'); + expect(payload).toMatchObject({ code: refusal.code, httpStatus: refusal.httpStatus }); + expect(payload).toMatchObject({ code: 'INVALID_FIELD', httpStatus: 400 }); + expect(leadingClause(payload.error)).toBe(leadingClause(refusal.message)); + expect(vi.mocked(bootSchemaStack)).not.toHaveBeenCalled(); + expect(vi.mocked(probeMigrationTarget)).not.toHaveBeenCalled(); + expect(vi.mocked(backfillRecordCurrencyCodes)).not.toHaveBeenCalled(); + }, RUN_TIMEOUT); + + it('human face, --apply on a terminal: the refusal is printed and no prompt opens', async () => { + const wasTTY = process.stdin.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + try { + const err = await MigrateRecordCurrencyCodes.run( + ['--apply', '--object', 'opportunity', '--field', 'amount', '--code', 'EURO'], + { root: CLI_ROOT }, + ).catch((e: unknown) => e); + expect((err as { oclif?: { exit?: number } }).oclif?.exit).toBe(1); + } finally { + Object.defineProperty(process.stdin, 'isTTY', { value: wasTTY, configurable: true }); + } + expect(printedLines().join('\n')).toContain(leadingClause((await engineRefusal('EURO')).message)); + expect(prompt.opened).toBe(0); + expect(vi.mocked(bootSchemaStack)).not.toHaveBeenCalled(); + expect(vi.mocked(probeMigrationTarget)).not.toHaveBeenCalled(); + }, RUN_TIMEOUT); + + it('a member code still reaches the boot (the control)', async () => { + await MigrateRecordCurrencyCodes.run(['--json', ...NAMING], { root: CLI_ROOT }); + expect(vi.mocked(bootSchemaStack)).toHaveBeenCalledTimes(1); + expect(vi.mocked(backfillRecordCurrencyCodes)).toHaveBeenCalledTimes(1); + }, RUN_TIMEOUT); + }); + it('on a SQLite file the dry-run line claims no row or table and names the journal-mode switch', async () => { boot(SQLITE_CONFIG); await MigrateRecordCurrencyCodes.run(NAMING, { root: CLI_ROOT }); diff --git a/packages/cli/src/commands/migrate/record-currency-codes.ts b/packages/cli/src/commands/migrate/record-currency-codes.ts index d730ad9feee..6bf55f78803 100644 --- a/packages/cli/src/commands/migrate/record-currency-codes.ts +++ b/packages/cli/src/commands/migrate/record-currency-codes.ts @@ -3,6 +3,7 @@ import { Command, Flags } from '@oclif/core'; import chalk from 'chalk'; import { createInterface } from 'node:readline'; +import { isValueDomainMember } from '@objectstack/spec/shared'; import { printHeader, printSuccess, @@ -24,6 +25,33 @@ import { refuseUndeclaredObjects } from '../../utils/migrate-object-scope.js'; // Type-only, so the heavy engine package is still loaded lazily below. import type { RecordCurrencyBackfillEngine } from '@objectstack/objectql'; +/** + * The `--code` refusal, judged before the occupancy probe, the `--apply` + * prompt and the boot: a code the code field's `valueDomain` would refuse + * fails the same way on every database, so nothing about the database needs + * opening to say so, and an `--apply` boot may create tables and columns. + * + * The predicate is the one `backfillRecordCurrencyCodes` applies, and the + * error is its envelope (`INVALID_FIELD` / 400, `httpStatus` for the `--json` + * face) and its wording, so the answer is the one the engine gave when this + * was judged after the boot. The code field's name is in the object's + * declaration, which is not loaded yet, so the message names the amount field + * the code is owed by. The engine keeps its own check for every other caller. + */ +function refuseNonMemberCurrencyCode(object: string, field: string, code: string): void { + if (isValueDomainMember('iso_4217_currency', code)) return; + const err = new Error( + `[record-currency-backfill] '${code}' is not an ISO 4217 currency code, so the currency code field of ` + + `'${object}.${field}' would refuse it: give the uppercase alphabetic code of the currency these amounts ` + + 'are in (for example USD). Refused before the database was opened or any row was read.', + ) as Error & { code: string; status: number; httpStatus: number; field: string }; + err.code = 'INVALID_FIELD'; + err.status = 400; + err.httpStatus = 400; + err.field = field; + throw err; +} + async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes const rl = createInterface({ input: process.stdin, output: process.stdout }); @@ -78,7 +106,7 @@ export default class MigrateRecordCurrencyCodes extends Command { code: Flags.string({ description: 'The ISO 4217 code to write, uppercase (e.g. USD). Written into every row with an amount and a blank code; ' + - 'a row that holds a code is never touched. A code that is not ISO 4217 is refused before any row is read', + 'a row that holds a code is never touched. A code that is not ISO 4217 is refused before the database is opened', required: true, }), apply: Flags.boolean({ @@ -103,6 +131,20 @@ export default class MigrateRecordCurrencyCodes extends Command { if (!flags.json) printHeader('Migrate · record-currency-codes'); + // `--code` first: before the occupancy probe, the `--apply` prompt and the boot. + try { + refuseNonMemberCurrencyCode(flags.object, flags.field, flags.code); + } catch (error: any) { + if (flags.json) { + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); + this.exit(1); + return; + } + printError(error.message); + this.exit(1); + return; + } + // Occupancy gate, as `summary-nulls`: probed before boot and before the prompt. const occupancy = await probeMigrationTarget(flags['database-url']); if (occupancy.status === 'busy' && apply && !flags.force) { @@ -244,8 +286,9 @@ export default class MigrateRecordCurrencyCodes extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } - // `INVALID_FIELD` is the backfill refusing the field or the code before - // its walk ("Refused before the walk read or wrote any row."). The boot + // `INVALID_FIELD` is the backfill refusing the field before its walk + // ("Refused before the walk read or wrote any row."; the code was judged + // before the boot, so it is refused there only if that judge drifts). The boot // above ran first, so on a SQLite file the note naming a journal-mode // switch follows, as on `summary-nulls`. const journalNote = error?.code === 'INVALID_FIELD' From 2a029310c284aae891814fa8a823d28d190624cb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 13:03:09 +0000 Subject: [PATCH 2/2] docs(cli): state the fixed condition beside defaultCurrency in the record-currency-codes section The currency-mode closure pin (rule A) reds a defaultCurrency sentence that does not carry the fixed condition. Name the field's defaultCurrency as the one it had under fixed. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01QUs3BvyNBMezKJUd7aydyG --- content/docs/deployment/cli.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index aba22abbe8b..9ca675fd7ce 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1523,8 +1523,8 @@ os migrate record-currency-codes --object opportunity --field amount --code USD **The code is yours, never derived.** `--object`, `--field` and `--code` are all required. One run writes the one code you give into every row of the field whose -amount is set and whose code is blank. It never reads a code from a `defaultCurrency` -the field had before, from the organization's `localization.currency`, or from +amount is set and whose code is blank. It never reads a code from the `defaultCurrency` +the field had under `fixed`, from the organization's `localization.currency`, or from anything else. A row that holds a code is never touched, whatever the code, and a row with no amount owes none. If the stored amounts are in more than one currency, split them with your own writes first: nothing here can tell those rows apart.