diff --git a/.changeset/cli-json-error-envelope-adr-0112-code.md b/.changeset/cli-json-error-envelope-adr-0112-code.md new file mode 100644 index 0000000000..2b261902ac --- /dev/null +++ b/.changeset/cli-json-error-envelope-adr-0112-code.md @@ -0,0 +1,81 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli): `--format json` failure envelopes carry the ADR-0112 `code` and `httpStatus` (#13347) + +Every machine-readable failure this CLI emits was built the same way — 48 sites +under `packages/cli/src/commands/`: + +```ts +await emitJson({ success: false, error: error.message }); +``` + +The payload carried the human sentence and nothing else. The error reaching +those `catch` blocks from `@objectstack/client` is not a bare `Error`: the SDK's +`fetch` wrapper attaches `err.code` (the semantic ADR-0112 string, normalized to +the same spelling across the flat `@objectstack/rest` envelope and the wrapped +runtime-dispatcher one) and `err.httpStatus`. Both were discarded at the CLI +boundary, so the one outcome a script most needs to branch on — *someone else +edited it, re-read and retry* vs *you are not allowed* vs *the server is down* — +was separable only by substring-matching an English sentence that no contract +pins. + +A stale-pin refusal from `os meta delete --if-match` used to read: + +```json +{ + "success": false, + "error": "[metadata_conflict] view/race_probe has been modified since you loaded it. …" +} +``` + +and now reads: + +```json +{ + "success": false, + "error": "[metadata_conflict] view/race_probe has been modified since you loaded it. …", + "code": "METADATA_CONFLICT", + "httpStatus": 409 +} +``` + +Maintainer ruling 2026-08-30 (option **A** of three): + +- The payload stays **FLAT**. Nesting into `{ error: { code, message, httpStatus } }` + was considered and declined as breaking. +- `success` and `error` keep their current meaning **and spelling**. +- The two keys are emitted **only when the thrown error carries them**, and are + **absent** — not `undefined` — otherwise. No fallback code is invented for a + locally-thrown plain `Error`: this CLI's own input refusals get no code, + deliberately, because ADR-0112's ledger is the authority on who may mint one. + +One value-space note, stated so the declaration matches what ships: `code` is a +**pass-through**, never minted or filtered. On wire failures it is the semantic +ADR-0112 string the SDK attached; on local I/O failures inside the same `try` +(`os validate` reading a `src/docs` that is a file, say) it is the Node errno +(`ENOENT`, `ENOTDIR`, …). The two vocabularies are disjoint — errnos are +`E`-prefixed OS names — so a consumer branching on ADR-0112 codes cannot +false-match an errno, but not every emitted `code` is ledger-owned. + +Human (`table`) output is untouched. + +**Why `minor` and not `patch`.** Maintainer-set, and it overrides the obvious +reading: this is a shape change to an **already-published error envelope**, and +that makes it minor even though it is purely additive. + +**Migration.** Nothing is required — no key is removed, renamed or re-typed, and +every payload this CLI emitted before is still emitted, byte-for-byte, minus the +two new keys. Two things are worth knowing before you rely on the new ones: + +- **The envelope is polymorphic, by design.** `code` and `httpStatus` are absent + whenever the failure did not carry them, which a consumer cannot distinguish + from an older CLI. Branch on presence (`if (payload.code === 'METADATA_CONFLICT')`), + never on absence meaning "success" or "unsupported version". This cost was + weighed against breaking every existing consumer, and the non-breaking side won. +- **Stop substring-matching the sentence.** `error` is prose and no contract pins + its wording; the bracketed `[metadata_conflict]` tag some messages carry today + is a property of one producer, not a contract, and a separate card argues for + removing it. Code that reads the sentence to classify a failure should move to + `code` (with `httpStatus` as the coarse fallback). diff --git a/packages/cli/src/commands/cloud/login.ts b/packages/cli/src/commands/cloud/login.ts index 28da4201ab..a0a994db30 100644 --- a/packages/cli/src/commands/cloud/login.ts +++ b/packages/cli/src/commands/cloud/login.ts @@ -73,7 +73,7 @@ import * as readline from 'node:readline/promises'; import { stdin as input, stdout as output } from 'node:process'; import { Command, Flags } from '@oclif/core'; import type { CliExitCode } from '../../utils/format.js'; -import { printHeader, printKV, printSuccess, printError, emitJson } from '../../utils/format.js'; +import { printHeader, printKV, printSuccess, printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { loginWithBrowser, loginWithPassword } from '../../utils/auth-flows.js'; import { DEFAULT_CLOUD_URL, readCloudConfig, writeCloudConfig } from '../../utils/cloud-config.js'; @@ -246,7 +246,7 @@ export default class CloudLogin extends Command { // written (an expired code, a denied approval, a poll failure), so an // indented payload here would recreate a two-document stream on the // path a consumer is least able to recover from. - await emitRecord({ success: false, error: error.message }); + await emitRecord({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/cloud/logout.ts b/packages/cli/src/commands/cloud/logout.ts index 8e964ecaf4..9349f965c7 100644 --- a/packages/cli/src/commands/cloud/logout.ts +++ b/packages/cli/src/commands/cloud/logout.ts @@ -8,7 +8,7 @@ import { Command, Flags } from '@oclif/core'; import { ObjectStackClient } from '@objectstack/client'; -import { printHeader, printSuccess, printError, emitJson } from '../../utils/format.js'; +import { printHeader, printSuccess, printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { deleteCloudConfig, tryReadCloudConfig } from '../../utils/cloud-config.js'; export default class CloudLogout extends Command { @@ -46,7 +46,7 @@ export default class CloudLogout extends Command { } } catch (error: any) { if (flags.json) { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/cloud/whoami.ts b/packages/cli/src/commands/cloud/whoami.ts index 1a29f5db52..70dbe6167e 100644 --- a/packages/cli/src/commands/cloud/whoami.ts +++ b/packages/cli/src/commands/cloud/whoami.ts @@ -7,7 +7,7 @@ */ import { Command, Flags } from '@oclif/core'; -import { printHeader, printKV, printSuccess, printError, emitJson, isExitSignal } from '../../utils/format.js'; +import { printHeader, printKV, printSuccess, printError, emitJson, isExitSignal, errorCodeFields } from '../../utils/format.js'; import { tryReadCloudConfig } from '../../utils/cloud-config.js'; export default class CloudWhoami extends Command { @@ -72,7 +72,7 @@ export default class CloudWhoami extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 2dd9f9278d..b843fb6563 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -39,6 +39,7 @@ import { printMetadataStats, emitJson, isExitSignal, + errorCodeFields, } from '../utils/format.js'; import { checkSpecVersionGap } from '../utils/spec-version.js'; @@ -720,7 +721,7 @@ export default class Compile extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ success: false, error: error.message, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error), warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); this.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/data/create.ts b/packages/cli/src/commands/data/create.ts index 75c3d0c0cd..cc427e61f1 100644 --- a/packages/cli/src/commands/data/create.ts +++ b/packages/cli/src/commands/data/create.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, printSuccess, emitJson } from '../../utils/format.js'; +import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -100,6 +100,7 @@ export default class DataCreate extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/data/delete.ts b/packages/cli/src/commands/data/delete.ts index 8a0086653f..2b17c869fa 100644 --- a/packages/cli/src/commands/data/delete.ts +++ b/packages/cli/src/commands/data/delete.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, printSuccess, emitJson } from '../../utils/format.js'; +import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -83,6 +83,7 @@ export default class DataDelete extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/data/get.ts b/packages/cli/src/commands/data/get.ts index 05583d2389..5f3a8a85db 100644 --- a/packages/cli/src/commands/data/get.ts +++ b/packages/cli/src/commands/data/get.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -74,6 +74,7 @@ export default class DataGet extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/data/query.ts b/packages/cli/src/commands/data/query.ts index 9e1065dfcc..2abe1d8185 100644 --- a/packages/cli/src/commands/data/query.ts +++ b/packages/cli/src/commands/data/query.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -117,6 +117,7 @@ export default class DataQuery extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/data/update.ts b/packages/cli/src/commands/data/update.ts index 2efcacdad1..57becc5123 100644 --- a/packages/cli/src/commands/data/update.ts +++ b/packages/cli/src/commands/data/update.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, printSuccess, emitJson } from '../../utils/format.js'; +import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -104,6 +104,7 @@ export default class DataUpdate extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 4f1771daad..89307dc468 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -12,6 +12,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../utils/format.js'; // ─── Types ────────────────────────────────────────────────────────── @@ -284,7 +285,7 @@ export default class Diff extends Command { } catch (error: any) { if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/environments/bind.ts b/packages/cli/src/commands/environments/bind.ts index 2788822623..67fb7d85c2 100644 --- a/packages/cli/src/commands/environments/bind.ts +++ b/packages/cli/src/commands/environments/bind.ts @@ -4,7 +4,7 @@ import { Command, Flags, Args } from '@oclif/core'; import path from 'node:path'; import fs from 'node:fs/promises'; import { spawnSync } from 'node:child_process'; -import { printError, printStep, printKV, emitJson, isExitSignal } from '../../utils/format.js'; +import { printError, printStep, printKV, emitJson, isExitSignal, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -147,7 +147,7 @@ export default class EnvironmentsBind extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.format === 'json') { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/environments/create.ts b/packages/cli/src/commands/environments/create.ts index f652d36072..4cab5f7df9 100644 --- a/packages/cli/src/commands/environments/create.ts +++ b/packages/cli/src/commands/environments/create.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Command, Flags } from '@oclif/core'; -import { printError, emitJson, isExitSignal } from '../../utils/format.js'; +import { printError, emitJson, isExitSignal, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; import { readAuthConfig, writeAuthConfig } from '../../utils/auth-config.js'; @@ -127,7 +127,7 @@ export default class EnvironmentsCreate extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.format === 'json') { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/environments/list.ts b/packages/cli/src/commands/environments/list.ts index 8f71fa58b5..8b46e560ca 100644 --- a/packages/cli/src/commands/environments/list.ts +++ b/packages/cli/src/commands/environments/list.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -76,7 +76,7 @@ export default class EnvironmentsList extends Command { } } catch (error: any) { if (flags.format === 'json') { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/environments/show.ts b/packages/cli/src/commands/environments/show.ts index f956828cd8..0f37f01d85 100644 --- a/packages/cli/src/commands/environments/show.ts +++ b/packages/cli/src/commands/environments/show.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -67,7 +67,7 @@ export default class EnvironmentsShow extends Command { } } catch (error: any) { if (flags.format === 'json') { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/i18n/check.ts b/packages/cli/src/commands/i18n/check.ts index 8a89d29d3e..afcb3bcc9b 100644 --- a/packages/cli/src/commands/i18n/check.ts +++ b/packages/cli/src/commands/i18n/check.ts @@ -14,6 +14,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { computeI18nCoverage } from '../../utils/i18n-coverage.js'; @@ -158,7 +159,7 @@ export default class I18nCheck extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/i18n/extract.ts b/packages/cli/src/commands/i18n/extract.ts index 185158f930..28a77c4739 100644 --- a/packages/cli/src/commands/i18n/extract.ts +++ b/packages/cli/src/commands/i18n/extract.ts @@ -15,6 +15,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { extractTranslations, @@ -371,7 +372,7 @@ export default class I18nExtract extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index 8ecaea3301..2d473a9f10 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -14,6 +14,7 @@ import { collectMetadataStats, printMetadataStats, emitJson, + errorCodeFields, } from '../utils/format.js'; export default class Info extends Command { @@ -115,7 +116,7 @@ export default class Info extends Command { } catch (error: any) { if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 907af1be6c..74fddc7657 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -23,6 +23,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../utils/format.js'; // ─── Types ────────────────────────────────────────────────────────── @@ -636,7 +637,7 @@ export default class Lint extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); process.exit(1); } console.log(''); diff --git a/packages/cli/src/commands/login.ts b/packages/cli/src/commands/login.ts index 1cae2c2ee9..9f6f9b056e 100644 --- a/packages/cli/src/commands/login.ts +++ b/packages/cli/src/commands/login.ts @@ -95,7 +95,7 @@ import { Command, Flags } from '@oclif/core'; import type { CliExitCode } from '../utils/format.js'; -import { printHeader, printSuccess, printError, printKV, emitJson } from '../utils/format.js'; +import { printHeader, printSuccess, printError, printKV, emitJson, errorCodeFields } from '../utils/format.js'; import { writeAuthConfig, readAuthConfig } from '../utils/auth-config.js'; import { ObjectStackClient } from '@objectstack/client'; import * as readline from 'node:readline/promises'; @@ -367,7 +367,7 @@ export default class AuthLogin extends Command { // written (an expired code, a denied approval, a poll failure), so an // indented payload here recreated the exact two-document stream #6531 // is about — on the path a consumer is least able to recover from. - await emitRecord({ success: false, error: error.message }); + await emitRecord({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/logout.ts b/packages/cli/src/commands/logout.ts index 74f19a9778..d89d31319a 100644 --- a/packages/cli/src/commands/logout.ts +++ b/packages/cli/src/commands/logout.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Command, Flags } from '@oclif/core'; -import { printHeader, printSuccess, printError, emitJson } from '../utils/format.js'; +import { printHeader, printSuccess, printError, emitJson, errorCodeFields } from '../utils/format.js'; import { deleteAuthConfig, readAuthConfig } from '../utils/auth-config.js'; import { ObjectStackClient } from '@objectstack/client'; @@ -53,6 +53,7 @@ export default class Logout extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/meta/delete-json-error-code.test.ts b/packages/cli/src/commands/meta/delete-json-error-code.test.ts new file mode 100644 index 0000000000..353556956d --- /dev/null +++ b/packages/cli/src/commands/meta/delete-json-error-code.test.ts @@ -0,0 +1,236 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13347] The ADR-0112 carriers survive the whole way to a `--format json` + * failure envelope — and are ABSENT when the failure never carried them. + * + * ## Why this file drives the real command and the real SDK + * + * The unit pin (`utils/format.error-code-fields.test.ts`) asserts what + * `errorCodeFields` does with an error SHAPED like the SDK's. That is a claim + * about the builder, and it stays green even if the shape is a fiction — if + * `err.code` never actually arrives populated in one of the 48 `catch` blocks, + * the card's whole premise is dead and every unit case still passes. + * + * So the cases below start from a REAL HTTP response body and run the REAL + * `@objectstack/client` `fetch` wrapper (the frame that builds the error), the + * REAL oclif command, and the REAL `emitJson`. One seam is stubbed, and it is + * the credential boundary rather than any part of the mechanism: + * `createApiClient` — "the operator is logged in, and here is the server" — + * replaced with a real `ObjectStackClient` whose `fetch` returns the response + * instead of opening a socket. + * + * ## Both server dialects, because the code's SPELLING is the thing at risk + * + * `@objectstack/rest` answers flat (`{ error, code }`); the runtime dispatcher + * answers wrapped (`{ success: false, error: { code, message, httpStatus } }`). + * #3842 / #4007 made `err.code` the same semantic STRING on both. A pin that + * exercised only one dialect would go green against a CLI that publishes the + * numeric status under `code` on the other. + * + * ## The omission arm gets a control from the same population + * + * `os meta delete --if-match ''` is refused by `metaDeleteOptions` with a plain + * `Error`, inside the same `try`, before a client exists. That is this CLI's + * own input refusal — the case option **B** would have minted a code for, and + * option **A** deliberately does not. The case asserts the emitted BYTES carry + * neither key, and asserts `createApiClient` was never called, so "no code" + * is measured on a genuinely local throw rather than on a network path that + * happened not to run. + * + * ## Why every case carries an explicit 60s budget + * + * Not a slow test being papered over — the budget is wall-clock only and no + * assertion moves with it. Each case drives a real `Command.run` against the + * real oclif root, which resolves the plugin/manifest surface before the + * command body runs; measured at ~0.8s per case on an idle box, and measured + * TIMING OUT at vitest's 5s default when this file ran inside the package's + * full 220-file suite on a shared container. A default-budget case here is a + * test whose verdict depends on what else the box happens to be doing. The + * neighbouring `delete-reset-carriers.test.ts` reaches the same conclusion for + * the same reason and spells it the same way. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ObjectStackClient } from '@objectstack/client'; +import MetaDelete, { EMPTY_IF_MATCH_REFUSAL } from './delete.js'; + +const stub = vi.hoisted(() => ({ + client: undefined as any, + token: 'test-token' as string | undefined, + createCalls: 0, +})); + +vi.mock('../../utils/api-client.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createApiClient: async () => { + stub.createCalls += 1; + return { client: stub.client, token: stub.token }; + }, + }; +}); + +/** `packages/cli` — the oclif root the command is loaded against. */ +const CLI_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); + +/** + * The escape is written as `\x1b`, never as the byte itself: one raw control + * character makes grep treat the whole file as binary. + */ +const SGR = /\x1b\[[0-9;]*m/g; +const plain = (s: string) => s.replace(SGR, ''); + +interface CliRun { + out: string; + exitCode: number; +} + +async function runCli(argv: string[]): Promise { + const chunks: string[] = []; + const logSpy = vi + .spyOn(console, 'log') + .mockImplementation((...args: unknown[]) => { chunks.push(args.map(String).join(' ')); }); + // `emitText` awaits `stdout.write`'s callback — a stub that swallows it + // HANGS rather than failing, and the case dies at its own timeout. + const writeSpy = vi + .spyOn(process.stdout, 'write') + .mockImplementation(((chunk: unknown, encodingOrCb?: unknown, maybeCb?: unknown) => { + chunks.push(String(chunk)); + const done = typeof encodingOrCb === 'function' ? encodingOrCb : maybeCb; + if (typeof done === 'function') (done as (err?: Error | null) => void)(null); + return true; + }) as never); + const savedExitCode = process.exitCode; + let exitCode = 0; + try { + await MetaDelete.run(argv, { root: CLI_ROOT }); + } catch (error: unknown) { + const oclif = (error as { oclif?: { exit?: number } })?.oclif; + exitCode = typeof oclif?.exit === 'number' ? oclif.exit : 1; + } finally { + logSpy.mockRestore(); + writeSpy.mockRestore(); + process.exitCode = savedExitCode; + } + return { out: plain(chunks.join('\n')), exitCode }; +} + +const CONFLICT_SENTENCE = + '[metadata_conflict] view/race_probe has been modified since you loaded it. ' + + 'Expected parent sha256:aaa but current is sha256:bbb'; + +/** A client whose `fetch` answers with one canned response, body and status verbatim. */ +function clientAnswering(status: number, body: unknown): ObjectStackClient { + return new ObjectStackClient({ + baseUrl: 'https://door.test', + token: 'test-token', + fetch: async () => ({ + ok: status >= 200 && status < 300, + status, + statusText: 'Conflict', + json: async () => body, + headers: new Headers(), + }) as any, + }); +} + +afterEach(() => { + stub.client = undefined; + stub.token = 'test-token'; + stub.createCalls = 0; +}); + +describe('[#13347] `os meta delete --format json` publishes the code it was handed', () => { + it('POSITIVE CONTROL — the FLAT `@objectstack/rest` 409 arrives with `code` populated', async () => { + // One frame earlier than the command: what the SDK's wrapper actually + // builds. If this is empty the card's premise is dead, and no assertion + // about the envelope below would be worth reading. + const client = clientAnswering(409, { error: CONFLICT_SENTENCE, code: 'METADATA_CONFLICT' }); + const thrown = await client.meta + .deleteItem('view', 'race_probe') + .then(() => undefined, (e: any) => e); + expect(thrown).toBeInstanceOf(Error); + expect(thrown.code).toBe('METADATA_CONFLICT'); + expect(thrown.httpStatus).toBe(409); + expect(thrown.message).toBe(CONFLICT_SENTENCE); + }, 60_000); + + it('the FLAT dialect reaches the envelope as `code` + `httpStatus` beside `error`', async () => { + stub.client = clientAnswering(409, { error: CONFLICT_SENTENCE, code: 'METADATA_CONFLICT' }); + const run = await runCli(['view', 'race_probe', '--format', 'json']); + + expect(run.exitCode).toBe(1); + const payload = JSON.parse(run.out); + expect(payload).toEqual({ + success: false, + error: CONFLICT_SENTENCE, + code: 'METADATA_CONFLICT', + httpStatus: 409, + }); + // The whole point: the branch a script needs is readable WITHOUT + // substring-matching the English sentence. + expect(payload.code).toBe('METADATA_CONFLICT'); + // …and the payload is FLAT — option C was declined as breaking. + expect(typeof payload.error).toBe('string'); + }, 60_000); + + it('the WRAPPED dispatcher dialect lands on the SAME spelling, not the numeric status', async () => { + stub.client = clientAnswering(409, { + success: false, + error: { code: 'METADATA_CONFLICT', message: CONFLICT_SENTENCE, httpStatus: 409 }, + }); + const run = await runCli(['view', 'race_probe', '--format', 'json']); + + expect(run.exitCode).toBe(1); + const payload = JSON.parse(run.out); + expect(payload.code).toBe('METADATA_CONFLICT'); + expect(payload.httpStatus).toBe(409); + // The regression #3842 fixed at the producer, pinned at this boundary: a + // NUMBER under `code` would make the branch our docs teach never match. + expect(typeof payload.code).toBe('string'); + expect(payload.code).not.toBe('409'); + }, 60_000); + + it('a status with no code publishes the status alone, not an invented code', async () => { + stub.client = clientAnswering(503, { message: 'upstream unavailable' }); + const run = await runCli(['view', 'race_probe', '--format', 'json']); + + expect(run.exitCode).toBe(1); + const payload = JSON.parse(run.out); + expect(payload).toEqual({ success: false, error: 'upstream unavailable', httpStatus: 503 }); + expect(Object.keys(payload)).not.toContain('code'); + }, 60_000); + + it("OMIT ARM — the CLI's own local refusal emits NEITHER key, in the BYTES", async () => { + // `metaDeleteOptions` throws a plain `Error` before a client exists. + const run = await runCli(['view', 'race_probe', '--format', 'json', '--if-match', '']); + + expect(run.exitCode).toBe(1); + // Genuinely local: nothing reached the network, so "no code" is not an + // accident of a path that happened not to run. + expect(stub.createCalls).toBe(0); + + const payload = JSON.parse(run.out); + expect(payload).toEqual({ success: false, error: EMPTY_IF_MATCH_REFUSAL }); + // Asserted on the emitted TEXT, because `{ code: undefined }` would be + // byte-identical through `JSON.stringify` at the object level. + expect(run.out).not.toContain('"code"'); + expect(run.out).not.toContain('"httpStatus"'); + expect(Object.keys(payload).sort()).toEqual(['error', 'success']); + }, 60_000); + + it('human `table` output is UNTOUCHED — same bytes, no carriers', async () => { + // Explicitly out of scope for this card: the prose stays prose. + stub.client = clientAnswering(409, { error: CONFLICT_SENTENCE, code: 'METADATA_CONFLICT' }); + const run = await runCli(['view', 'race_probe']); + + expect(run.exitCode).toBe(1); + expect(run.out).toContain(CONFLICT_SENTENCE); + expect(run.out).not.toContain('httpStatus'); + expect(run.out).not.toContain('METADATA_CONFLICT'); + }, 60_000); +}); diff --git a/packages/cli/src/commands/meta/delete.ts b/packages/cli/src/commands/meta/delete.ts index 1d762e6708..f054f64475 100644 --- a/packages/cli/src/commands/meta/delete.ts +++ b/packages/cli/src/commands/meta/delete.ts @@ -2,7 +2,7 @@ import { Args, Command, Flags } from '@oclif/core'; import type { DeleteMetaItemOptions } from '@objectstack/client'; -import { printError, printSuccess, emitJson } from '../../utils/format.js'; +import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -183,6 +183,7 @@ export default class MetaDelete extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/meta/get.ts b/packages/cli/src/commands/meta/get.ts index 029cb9a650..9c4cae2956 100644 --- a/packages/cli/src/commands/meta/get.ts +++ b/packages/cli/src/commands/meta/get.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -63,6 +63,7 @@ export default class MetaGet extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/meta/list.ts b/packages/cli/src/commands/meta/list.ts index 521be90f06..2752733304 100644 --- a/packages/cli/src/commands/meta/list.ts +++ b/packages/cli/src/commands/meta/list.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, emitJson } from '../../utils/format.js'; +import { printError, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -95,6 +95,7 @@ export default class MetaList extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/meta/register.ts b/packages/cli/src/commands/meta/register.ts index ac4c867d53..a919acd766 100644 --- a/packages/cli/src/commands/meta/register.ts +++ b/packages/cli/src/commands/meta/register.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Args, Command, Flags } from '@oclif/core'; -import { printError, printSuccess, emitJson } from '../../utils/format.js'; +import { printError, printSuccess, emitJson, errorCodeFields } from '../../utils/format.js'; import { createApiClient, requireAuth } from '../../utils/api-client.js'; import { formatOutput } from '../../utils/output-formatter.js'; @@ -87,6 +87,7 @@ export default class MetaRegister extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/commands/meta/resync.ts b/packages/cli/src/commands/meta/resync.ts index 14340c10a3..5e6316b70f 100644 --- a/packages/cli/src/commands/meta/resync.ts +++ b/packages/cli/src/commands/meta/resync.ts @@ -12,6 +12,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { bootstrapPlatformAdmin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; @@ -130,7 +131,7 @@ export default class MetaResync extends Command { try { stack = await bootSchemaStack({ jsonOutput: flags.json, databaseUrl: flags['database-url'] }); } catch (error: any) { - if (flags.json) await emitJson({ error: error.message }, 0, { compact: true }); + if (flags.json) await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); else printError(error.message || String(error)); process.exit(1); } @@ -215,7 +216,7 @@ export default class MetaResync extends Command { console.log(''); } catch (error: any) { exitCode = 1; - if (flags.json) await emitJson({ error: error.message }, 0, { compact: true }); + if (flags.json) await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); else printError(error.message || String(error)); } finally { await stack.shutdown(); diff --git a/packages/cli/src/commands/migrate/apply.ts b/packages/cli/src/commands/migrate/apply.ts index 407fa0a670..31275cc5d3 100644 --- a/packages/cli/src/commands/migrate/apply.ts +++ b/packages/cli/src/commands/migrate/apply.ts @@ -12,6 +12,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack, @@ -180,7 +181,7 @@ export default class MigrateApply extends Command { composeHostStack: true, }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -371,7 +372,7 @@ export default class MigrateApply extends Command { console.log(chalk.dim(` ${timer.display()}`)); console.log(''); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/migrate/files-to-references.ts b/packages/cli/src/commands/migrate/files-to-references.ts index a7d02bf8f2..26fd337e9a 100644 --- a/packages/cli/src/commands/migrate/files-to-references.ts +++ b/packages/cli/src/commands/migrate/files-to-references.ts @@ -13,6 +13,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; @@ -166,7 +167,7 @@ export default class MigrateFilesToReferences extends Command { extraPlugins: await buildDataMigrationPlugins({ storage: true }), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -295,7 +296,7 @@ export default class MigrateFilesToReferences extends Command { if (!result.gatePassed) this.exit(1); } catch (error: any) { if (isExitSignal(error)) throw error; - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/migrate/meta.ts b/packages/cli/src/commands/migrate/meta.ts index aa3dbea298..4510e5e949 100644 --- a/packages/cli/src/commands/migrate/meta.ts +++ b/packages/cli/src/commands/migrate/meta.ts @@ -25,6 +25,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -418,7 +419,7 @@ export default class MigrateMeta extends Command { return; } if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); @@ -538,7 +539,7 @@ export default class MigrateMeta extends Command { extraPlugins: await buildDataMigrationPlugins({ automation: true }), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); return; } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); return; } printError(error.message || String(error)); this.exit(1); return; @@ -612,7 +613,7 @@ export default class MigrateMeta extends Command { } } catch (error: any) { exitCode = 1; - if (flags.json) await emitJson({ error: error.message }, 0, { compact: true }); + if (flags.json) await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); else printError(error.message || String(error)); } finally { await stack.shutdown(); diff --git a/packages/cli/src/commands/migrate/multi-value-columns.ts b/packages/cli/src/commands/migrate/multi-value-columns.ts index f85643f536..5abef79e17 100644 --- a/packages/cli/src/commands/migrate/multi-value-columns.ts +++ b/packages/cli/src/commands/migrate/multi-value-columns.ts @@ -13,6 +13,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; @@ -432,7 +433,7 @@ export default class MigrateMultiValueColumns extends Command { ...(apply ? {} : { readOnlyProbe: true }), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -578,7 +579,7 @@ export default class MigrateMultiValueColumns extends Command { if (failed.length > 0 || verified === false) this.exit(1); } catch (error: any) { if (isExitSignal(error)) throw error; - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 0c29d0926e..511c8e8171 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -11,6 +11,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack, @@ -153,7 +154,7 @@ export default class MigratePlan extends Command { composeHostStack: true, }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -322,7 +323,7 @@ export default class MigratePlan extends Command { console.log(chalk.dim(` ${timer.display()}`)); console.log(''); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/migrate/recorded-by.ts b/packages/cli/src/commands/migrate/recorded-by.ts index cef253c566..77288dc4b3 100644 --- a/packages/cli/src/commands/migrate/recorded-by.ts +++ b/packages/cli/src/commands/migrate/recorded-by.ts @@ -24,6 +24,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -100,7 +101,7 @@ export default class MigrateRecordedBy extends Command { extraPlugins: await buildDataMigrationPlugins(), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; diff --git a/packages/cli/src/commands/migrate/resume.ts b/packages/cli/src/commands/migrate/resume.ts index 4e55742f44..3e047fd8a9 100644 --- a/packages/cli/src/commands/migrate/resume.ts +++ b/packages/cli/src/commands/migrate/resume.ts @@ -22,6 +22,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -110,7 +111,7 @@ export default class MigrateResume extends Command { extraPlugins: await buildDataMigrationPlugins(), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; diff --git a/packages/cli/src/commands/migrate/summary-nulls.ts b/packages/cli/src/commands/migrate/summary-nulls.ts index 693fc5ccc1..65a70cb04e 100644 --- a/packages/cli/src/commands/migrate/summary-nulls.ts +++ b/packages/cli/src/commands/migrate/summary-nulls.ts @@ -13,6 +13,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; @@ -172,7 +173,7 @@ export default class MigrateSummaryNulls extends Command { extraPlugins: await buildDataMigrationPlugins(), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -239,7 +240,7 @@ export default class MigrateSummaryNulls extends Command { if (report.failures.length > 0) this.exit(1); } catch (error: any) { if (isExitSignal(error)) throw error; - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/migrate/value-shapes.ts b/packages/cli/src/commands/migrate/value-shapes.ts index 6f0e01fd96..f3c679f18d 100644 --- a/packages/cli/src/commands/migrate/value-shapes.ts +++ b/packages/cli/src/commands/migrate/value-shapes.ts @@ -12,6 +12,7 @@ import { printStep, createTimer, emitJson, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -139,7 +140,7 @@ export default class MigrateValueShapes extends Command { extraPlugins: await buildDataMigrationPlugins(), }); } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); return; @@ -239,7 +240,7 @@ export default class MigrateValueShapes extends Command { this.exit(1); } } catch (error: any) { - if (flags.json) { await emitJson({ error: error.message }, 0, { compact: true }); this.exit(1); } + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); this.exit(1); } finally { diff --git a/packages/cli/src/commands/register.ts b/packages/cli/src/commands/register.ts index 5e84c66588..90ac1b68ac 100644 --- a/packages/cli/src/commands/register.ts +++ b/packages/cli/src/commands/register.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Command, Flags } from '@oclif/core'; -import { printHeader, printSuccess, printError, printKV, emitJson } from '../utils/format.js'; +import { printHeader, printSuccess, printError, printKV, emitJson, errorCodeFields } from '../utils/format.js'; import { writeAuthConfig } from '../utils/auth-config.js'; import { ObjectStackClient } from '@objectstack/client'; import * as readline from 'node:readline/promises'; @@ -152,7 +152,7 @@ export default class Register extends Command { } } catch (error: any) { if (flags.json) { - await emitJson({ success: false, error: error.message }); + await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/storage/orphans.ts b/packages/cli/src/commands/storage/orphans.ts index 6cb0f42b0d..ca477812c4 100644 --- a/packages/cli/src/commands/storage/orphans.ts +++ b/packages/cli/src/commands/storage/orphans.ts @@ -12,6 +12,7 @@ import { createTimer, emitJson, isExitSignal, + errorCodeFields, } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; @@ -106,7 +107,7 @@ export default class StorageOrphans extends Command { }); } catch (error: any) { if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); @@ -178,7 +179,7 @@ export default class StorageOrphans extends Command { } catch (error: any) { if (isExitSignal(error)) throw error; if (flags.json) { - await emitJson({ error: error.message }, 0, { compact: true }); + await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } printError(error.message || String(error)); diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 2253413c8e..690ab8202f 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -32,6 +32,7 @@ import { printMetadataStats, emitJson, isExitSignal, + errorCodeFields, } from '../utils/format.js'; import { checkSpecVersionGap } from '../utils/spec-version.js'; @@ -519,6 +520,7 @@ export default class Validate extends Command { await emitJson({ valid: false, error: error.message, + ...errorCodeFields(error), // [#12047] Whatever the run had reached before the throw. A config // that dies in `loadConfig` reports `[]` here honestly — nothing was // computed yet — while a throw from a later step (a `src/docs` that diff --git a/packages/cli/src/commands/whoami.ts b/packages/cli/src/commands/whoami.ts index 9cd05cc9c4..9616c00e6f 100644 --- a/packages/cli/src/commands/whoami.ts +++ b/packages/cli/src/commands/whoami.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { Command, Flags } from '@oclif/core'; -import { printHeader, printError, printKV, emitJson } from '../utils/format.js'; +import { printHeader, printError, printKV, emitJson, errorCodeFields } from '../utils/format.js'; import { createApiClient, requireAuth } from '../utils/api-client.js'; import { formatOutput } from '../utils/output-formatter.js'; @@ -75,6 +75,7 @@ export default class Whoami extends Command { await emitJson({ success: false, error: error.message, + ...errorCodeFields(error), }); this.exit(1); } diff --git a/packages/cli/src/utils/format.error-code-fields.test.ts b/packages/cli/src/utils/format.error-code-fields.test.ts new file mode 100644 index 0000000000..188f7af865 --- /dev/null +++ b/packages/cli/src/utils/format.error-code-fields.test.ts @@ -0,0 +1,237 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13347] `errorCodeFields` — the ADR-0112 carriers a `--format json` failure + * envelope adds beside its `error` sentence, and the arm that adds NOTHING. + * + * ## What is being pinned, and why it is pinned on BYTES + * + * Ruled 2026-08-30 (option **A**): when the thrown error carries `code` / + * `httpStatus`, emit them alongside `error`; when it does not, OMIT them. + * `success` and `error` keep their meaning and spelling, so the change is + * additive and no existing consumer breaks. The payload stays FLAT — option C + * (nesting into `{ error: { code, message, httpStatus } }`) was declined as + * breaking — and no fallback code is invented for a locally-thrown plain + * `Error` (option B, not chosen). + * + * The omit arm is the half that ships broken silently, because the WRONG + * implementation of it — `{ ...payload, code: error.code }`, leaving + * `code: undefined` on the object — is byte-identical to the right one through + * `JSON.stringify`, and byte-identical through `yaml.stringify` as well (both + * measured below). It stops being identical on `formatOutput`'s `table` + * branch, which walks `Object.entries` and prints `code: null` for a + * present-but-undefined key. So every omission case here asserts the emitted + * TEXT, and one case asserts the wrong implementation is visibly different — + * a negative control, without which "the bytes have no `code`" would pass + * against an implementation that leaks the key. + */ + +import { describe, it, expect, vi } from 'vitest'; +import yaml from 'yaml'; + +import { errorCodeFields, emitJson, isExitSignal } from './format.js'; +import { formatOutput } from './output-formatter.js'; + +/** Capture everything a payload emitter writes, `console.log` and stdout alike. */ +async function captureEmit(run: () => Promise): Promise { + const chunks: string[] = []; + const logSpy = vi + .spyOn(console, 'log') + .mockImplementation((...args: unknown[]) => { chunks.push(args.map(String).join(' ')); }); + // `emitText` writes through `writeStdoutDirect`, which calls + // `stdout.write(text, callback)` and AWAITS the callback. A stub that + // swallows the callback does not fail, it HANGS — invoke it. + const writeSpy = vi + .spyOn(process.stdout, 'write') + .mockImplementation(((chunk: unknown, encodingOrCb?: unknown, maybeCb?: unknown) => { + chunks.push(String(chunk)); + const done = typeof encodingOrCb === 'function' ? encodingOrCb : maybeCb; + if (typeof done === 'function') (done as (err?: Error | null) => void)(null); + return true; + }) as never); + const savedExitCode = process.exitCode; + try { + await run(); + } finally { + logSpy.mockRestore(); + writeSpy.mockRestore(); + process.exitCode = savedExitCode; + } + return chunks.join('\n'); +} + +/** + * An error shaped the way `@objectstack/client`'s `fetch` wrapper shapes one: + * a real `Error` with the ADR-0112 string on `code` and the numeric status on + * `httpStatus`. The end-to-end proof that the SDK really produces this — from + * a real 409 body, through the real wrapper, into a real command's `catch` — + * lives in `commands/meta/delete-json-error-code.test.ts`; this file pins what + * the builder does with it. + */ +function sdkError(message: string, code: string | undefined, httpStatus: number): Error { + const e = new Error(message) as Error & { code?: string; httpStatus?: number }; + if (code !== undefined) e.code = code; + e.httpStatus = httpStatus; + return e; +} + +describe('[#13347] errorCodeFields — what counts as "carrying" a code', () => { + it('adds BOTH carriers when the error carries both (the SDK shape)', () => { + const err = sdkError('[metadata_conflict] view/race_probe has been modified', 'METADATA_CONFLICT', 409); + expect(errorCodeFields(err)).toEqual({ code: 'METADATA_CONFLICT', httpStatus: 409 }); + }); + + it('adds NEITHER for a locally-thrown plain Error — no fallback is invented', () => { + // Option B was NOT chosen: the CLI's own input refusals get no code, and + // ADR-0112's ledger stays the authority on who may mint one. + expect(errorCodeFields(new Error('--if-match needs the metadata version'))).toEqual({}); + }); + + it('decides the two keys INDEPENDENTLY — a status with no code still ships', () => { + // Not a style choice: the SDK sets `error.httpStatus = res.status` on EVERY + // non-2xx, while `error.code` is `undefined` whenever the server sent none. + // "Emit both or neither" would discard a status that is in hand, on exactly + // the responses whose envelope is thinnest. + expect(errorCodeFields(sdkError('Bad Request', undefined, 400))).toEqual({ httpStatus: 400 }); + // …and the mirror: a code with no status. + const coded = new Error('boom') as Error & { code?: string }; + coded.code = 'VALIDATION_FAILED'; + expect(errorCodeFields(coded)).toEqual({ code: 'VALIDATION_FAILED' }); + }); + + it('REJECTS a numeric `code` rather than coercing it', () => { + // The pre-#3842 wrapped envelope parked the HTTP STATUS in `error.code`. + // Re-publishing a number under the name the semantic vocabulary uses would + // reintroduce that confusion at this boundary; `httpStatus` still ships. + const legacy = new Error('boom') as Error & { code?: unknown; httpStatus?: number }; + legacy.code = 400; + legacy.httpStatus = 400; + expect(errorCodeFields(legacy)).toEqual({ httpStatus: 400 }); + }); + + it('rejects an empty-string code and a non-integer status', () => { + const blank = new Error('boom') as Error & { code?: string; httpStatus?: number }; + blank.code = ''; + blank.httpStatus = Number.NaN; + expect(errorCodeFields(blank)).toEqual({}); + const fractional = new Error('boom') as Error & { httpStatus?: number }; + fractional.httpStatus = 409.5; + expect(errorCodeFields(fractional)).toEqual({}); + }); + + it("never publishes oclif's EEXIT control signal as an error code", () => { + // `this.exit(n)` THROWS an ExitError whose `code` is the string 'EEXIT', + // and several of the 48 catch blocks do not re-throw it first (`os migrate + // meta` carries a comment about the bare "EEXIT: 1" it would report). It is + // our own stack unwinding, not a vocabulary a consumer may branch on. + const exitError = new Error('EEXIT: 1') as Error & { code?: string; oclif?: { exit: number } }; + exitError.code = 'EEXIT'; + exitError.oclif = { exit: 1 }; + expect(isExitSignal(exitError)).toBe(true); + expect(errorCodeFields(exitError)).toEqual({}); + + // The other spelling of the same signal — `oclif.exit` with no `code`. + const bare = new Error('exit') as Error & { oclif?: { exit: number } }; + bare.oclif = { exit: 2 }; + expect(errorCodeFields(bare)).toEqual({}); + }); + + it('survives a non-object throw without inventing anything', () => { + // `throw 'a string'` and `throw null` both reach these catch blocks as + // `error`, and a builder that indexed them blindly would take the command + // down on the failure path. + for (const thrown of [undefined, null, 'a string', 42, { nope: true }]) { + expect(errorCodeFields(thrown)).toEqual({}); + } + }); + + it('returns a FRESH object each call — the spread must not alias shared state', () => { + const a = errorCodeFields(sdkError('x', 'A_CODE', 400)); + const b = errorCodeFields(new Error('plain')); + expect(a).not.toBe(b); + expect(errorCodeFields(new Error('plain'))).toEqual({}); + }); +}); + +describe('[#13347] the emitted BYTES — additive when present, absent when not', () => { + it('emitJson: the carriers appear beside `error`, and `success`/`error` keep their spelling', async () => { + const err = sdkError('[metadata_conflict] view/json_probe has been modified', 'METADATA_CONFLICT', 409); + const out = await captureEmit(() => + emitJson({ success: false, error: err.message, ...errorCodeFields(err) }), + ); + const payload = JSON.parse(out); + expect(payload).toEqual({ + success: false, + error: '[metadata_conflict] view/json_probe has been modified', + code: 'METADATA_CONFLICT', + httpStatus: 409, + }); + // FLAT, not nested — option C was declined as breaking. + expect(typeof payload.error).toBe('string'); + // The two keys existing consumers read are untouched, by name and by value. + expect(Object.keys(payload).slice(0, 2)).toEqual(['success', 'error']); + }); + + it('emitJson: a plain Error emits the SAME BYTES it emitted before this card', async () => { + const err = new Error('--if-match needs the metadata version to pin the reset to'); + const before = await captureEmit(() => emitJson({ success: false, error: err.message })); + const after = await captureEmit(() => + emitJson({ success: false, error: err.message, ...errorCodeFields(err) }), + ); + expect(after).toBe(before); + expect(after).not.toContain('code'); + expect(after).not.toContain('httpStatus'); + }); + + it('NEGATIVE CONTROL: `code: undefined` is what this must NOT build', async () => { + // Without this case, the assertion above passes against the WRONG + // implementation on two of the three emitters. Here is the measurement of + // where each one hides it and where it does not. + const leaky = { success: false, error: 'boom', code: undefined as string | undefined }; + const clean = { success: false, error: 'boom' }; + + // JSON: identical — the trap. + expect(JSON.stringify(leaky)).toBe(JSON.stringify(clean)); + // YAML: also identical — `yaml.stringify` drops undefined too. + expect(yaml.stringify(leaky)).toBe(yaml.stringify(clean)); + // The object itself is NOT identical, and that is what leaks one emitter over. + expect(Object.keys(leaky)).toEqual(['success', 'error', 'code']); + expect(Object.keys(clean)).toEqual(['success', 'error']); + + // `formatOutput`'s human branch is where the difference becomes visible. + const leakyTable = await captureEmit(() => formatOutput(leaky, 'table')); + const cleanTable = await captureEmit(() => formatOutput(clean, 'table')); + expect(leakyTable).toContain('code'); + expect(cleanTable).not.toContain('code'); + + // What the builder actually returns has no such key to leak. + expect(Object.keys({ ...clean, ...errorCodeFields(new Error('boom')) })).toEqual(['success', 'error']); + }); + + it('formatOutput yaml/table: the omit arm adds no key on any emitter', async () => { + const payload = { success: false, error: 'boom', ...errorCodeFields(new Error('boom')) }; + const asYaml = await captureEmit(() => formatOutput(payload, 'yaml')); + expect(asYaml).not.toContain('code'); + expect(asYaml).not.toContain('httpStatus'); + const asTable = await captureEmit(() => formatOutput(payload, 'table')); + expect(asTable).not.toContain('code'); + expect(asTable).not.toContain('httpStatus'); + }); + + it('the compact NDJSON writers stay one line with the carriers added', async () => { + // `os login` / `os cloud login` declare NDJSON: one record per line is the + // contract, and a payload that wrapped would break every consumer reading + // stdout a line at a time. + const err = sdkError('device code expired', 'EXPIRED_TOKEN', 401); + const out = await captureEmit(() => + emitJson({ success: false, error: err.message, ...errorCodeFields(err) }, 0, { compact: true }), + ); + expect(out.trimEnd().split('\n')).toHaveLength(1); + expect(JSON.parse(out)).toEqual({ + success: false, + error: 'device code expired', + code: 'EXPIRED_TOKEN', + httpStatus: 401, + }); + }); +}); diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index eb214450f7..167200f413 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -131,6 +131,129 @@ export function isExitSignal(error: unknown): boolean { return e?.code === 'EEXIT' || typeof e?.oclif?.exit === 'number'; } +/** + * [#13347] The ADR-0112 carriers a `--format json` failure envelope adds + * beside its `error` sentence — `{ code, httpStatus }`, and only the ones the + * thrown error actually carries. + * + * ## The defect this closes + * + * Every machine-readable failure this CLI emits was built the same way: + * + * await emitJson({ success: false, error: error.message }); + * + * — 48 sites under `commands/`, and the payload carried the human sentence and + * nothing else. The error reaching those `catch` blocks from + * `@objectstack/client` is not a bare `Error`: the SDK's `fetch` wrapper + * attaches `err.code` (the semantic ADR-0112 string, normalized to the SAME + * spelling across the flat `@objectstack/rest` envelope and the wrapped + * runtime-dispatcher one — #3842 / #4007) and `err.httpStatus`. Both were + * dropped at the CLI boundary, so the one outcome a script most needs to + * branch on — *someone else edited it, re-read and retry* vs *you are not + * allowed* vs *the server is down* — was separable only by substring-matching + * an English sentence that no contract pins. + * + * ## Why the keys are OMITTED rather than emitted as `undefined` + * + * `JSON.stringify` drops an `undefined` value, so `{ code: undefined }` and an + * absent `code` are byte-identical on `emitJson`'s own output — which is + * exactly why building the former is unsafe: the difference is invisible where + * an author would look for it, and NOT invisible everywhere. + * + * Measured, rather than assumed. `yaml.stringify` drops it too (checked: both + * `{ success, error, code: undefined }` and `{ success, error }` serialize to + * `success: false\nerror: boom\n`). `formatOutput`'s `table` branch does not: + * `printKeyValue` walks `Object.entries`, which yields the key, and its + * `value === undefined` arm prints `code: null` outright; `printTable` derives + * its columns from `Object.keys` and grows an empty `code` column the same way. + * A payload meaning "this failure carried no code" would there assert a code + * whose value is null. + * + * So this returns a partial object with the keys ABSENT — the spread adds + * nothing at all when there is nothing to add — and the pin asserts the + * emitted BYTES rather than the object, because on two of the three emitters + * the object's own shape is what the bytes cannot show. + * + * ## What counts as "carrying" one — per key, not as a pair + * + * The two keys are decided INDEPENDENTLY, and that is a measurement rather + * than a preference: the SDK sets `error.httpStatus = res.status` on every + * non-2xx, while `error.code` comes from `asSemanticCode(...)` and is + * `undefined` whenever the server sent no code. Coupling them ("emit both or + * neither") would therefore discard a status that IS in hand, on exactly the + * responses whose envelope is thinnest. + * + * - `code` — a non-empty STRING. A numeric `code` is deliberately rejected + * rather than coerced: the pre-#3842 wrapped envelope parked the HTTP + * STATUS in `error.code`, and re-publishing a number under the name the + * semantic vocabulary uses would reintroduce, at this boundary, the exact + * confusion that producer-side fix removed. + * - `httpStatus` — a finite integer. `NaN` and fractional values are not + * statuses. + * + * ⛔ No value is INVENTED. A locally-thrown plain `Error` — every one of this + * CLI's own input refusals — carries neither key and gets neither key. That + * was option **B** of the card and it was not chosen: ADR-0112's ledger is the + * authority on who may mint a code, and this card mints nothing. The accepted + * cost, recorded so it is not re-opened as a defect: the payload is + * POLYMORPHIC — a consumer cannot distinguish "this failure carried no code" + * from "an older CLI". That was weighed against breaking every existing + * consumer and the non-breaking side won. + * + * ⛔ Nor is the value FILTERED against a catalog. The codes actually in flight + * here are broader than `StandardErrorCode`'s enum — `METADATA_CONFLICT`, + * `FORBIDDEN` and `VALIDATION_FAILED` are all absent from it — so a membership + * check would drop precisely the code this card exists to surface. Passing a + * producer's code through is not minting one; narrowing the field to a + * vocabulary no ledger declares would be. + * + * ## The one exclusion, and why it is not a filter + * + * oclif's `this.exit(n)` THROWS an `ExitError` whose `code` is the string + * `'EEXIT'`. It is a control-flow signal from our own code, not a failure with + * a vocabulary, and several of these `catch` blocks do not re-throw it first + * (`os migrate meta` carries a comment about the "EEXIT: 1" it would otherwise + * report). Publishing `code: "EEXIT"` into a machine-readable envelope would + * hand consumers a branch on our own stack unwinding. {@link isExitSignal} is + * reused rather than re-spelled so the judgement "this is a signal, not an + * error" stays single-sourced. + * + * @example + * } catch (error: any) { + * if (flags.format === 'json') { + * await emitJson({ success: false, error: error.message, ...errorCodeFields(error) }); + * this.exit(1); + * } + */ +export interface ErrorCodeFields { + /** + * The machine code the failure carried, when it carried one — passed + * through, never minted or filtered. For wire failures this is the semantic + * ADR-0112 string the SDK attached (`METADATA_CONFLICT`, `FORBIDDEN`, …); + * for local I/O failures it is the Node errno (`ENOENT`, `ENOTDIR`, …), + * which `os validate` / `os compile` / `os lint` really do throw from + * inside the same `try`. The two vocabularies are disjoint (errnos are + * `E`-prefixed OS names), so an ADR-0112 branch cannot false-match an + * errno — but a consumer must not assume every `code` is ledger-owned. + * Declared honestly here per the 2026-08-31 contract review: narrowing + * this field to ADR-0112-only later would be breaking. + */ + code?: string; + /** The HTTP status the failure carried, when it carried one. */ + httpStatus?: number; +} + +export function errorCodeFields(error: unknown): ErrorCodeFields { + const fields: ErrorCodeFields = {}; + if (isExitSignal(error)) return fields; + const e = error as { code?: unknown; httpStatus?: unknown } | null | undefined; + if (typeof e?.code === 'string' && e.code !== '') fields.code = e.code; + if (typeof e?.httpStatus === 'number' && Number.isInteger(e.httpStatus)) { + fields.httpStatus = e.httpStatus; + } + return fields; +} + /** * The drain-aware write `emitJson` is built on, for machine payloads that are * not JSON — `formatOutput`'s `--format yaml` truncates on a pipe exactly like