Skip to content

Commit 63d603e

Browse files
Elon Muskclaude
andauthored
fix(cli): read the declared response envelope in os datasource commands (#10675) (#10761)
`list-tables`, `introspect` and `validate` read the pre-#3843 flat shape (`body.tables` / `body.draft` / `body.results`, `error` as a string) while the server emits `{ success, data }` / `{ success:false, error:{code,message} }`. Every payload read `undefined`, and each command reported that as an empty result — most dangerously `validate`, which printed "No federated objects to validate." and exited 0 against drift the server had flagged `missing_column … severity:error`. All three now go through one CLI-side reader for the declared envelope, with payload types taken from `@objectstack/spec/contracts` rather than re-transcribed. The reader is strict: a body that is not the envelope is a loud failure, never an empty payload, so "nothing found" is reachable only from a server that really said so. The error path passes `error.message` — a string — instead of handing the error object to oclif's `this.error()`. Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6936d07 commit 63d603e

8 files changed

Lines changed: 612 additions & 22 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
**Fix:** `os datasource list-tables`, `os datasource introspect` and
6+
`os datasource validate` now read the response envelope the server actually
7+
emits, so all three work against a live server for the first time (#10675).
8+
9+
The three commands read the pre-#3843 **flat** shape — `body.tables`,
10+
`body.draft`, `body.results`, and `body.error` as a string — while every REST
11+
body the platform sends is the declared envelope written by `sendOk` /
12+
`sendError`: `{ success: true, data: { … } }` or
13+
`{ success: false, error: { code, message } }`. Nothing failed loudly, because
14+
each payload simply read `undefined` and every command reported that as an
15+
ordinary empty result:
16+
17+
- `list-tables` printed `No remote tables found.` while the server was
18+
returning two tables.
19+
- `introspect` printed `Failed to generate draft` for drafts the server had
20+
generated.
21+
- `validate` printed `No federated objects to validate.` and exited **0**
22+
against drift the server had flagged `missing_column … severity:error` — a
23+
schema gate green-lighting a CI-breaking condition it had never read.
24+
- An unknown datasource crashed with `TypeError: first argument must be a
25+
string or instance of Error`, because the error **object** was handed to
26+
oclif's `this.error()` instead of `error.message`.
27+
28+
`validate`'s exit code is the behaviour change to note: a datasource whose
29+
federated objects have drifted now exits **1** where it previously exited 0. If
30+
you have a pipeline that treats this command as advisory, it starts failing on
31+
drift that was always there.
32+
33+
A body that is **not** the declared envelope is now a loud failure rather than
34+
an empty payload. That distinction is the point: "nothing found" is reachable
35+
only from a server that really said so, never from a response the CLI could not
36+
read. The legacy flat shape is deliberately *not* also accepted — a
37+
consumer-side fallback would re-create the divergence as a second de-facto
38+
contract.
Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The three `os datasource` subcommands, driven against the WRAPPED response
5+
* envelope the server really emits (#10675).
6+
*
7+
* This is these commands' first functional coverage: they shipped reading the
8+
* pre-#3843 flat shape (`body.tables` / `body.draft` / `body.results`, `error`
9+
* as a string) and never worked against the current envelope. Nothing was red
10+
* — every payload read `undefined`, which each command reported as an ordinary
11+
* empty result.
12+
*
13+
* ## What these tests are really pinning
14+
*
15+
* Not "the happy path parses". The severe failure mode was `validate` printing
16+
* `No federated objects to validate.` and exiting **0** against drift the
17+
* server had flagged `missing_column region severity:error` — a schema gate
18+
* green-lighting a CI-breaking condition it had never read. A test proving the
19+
* happy path now parses would not have caught it, because the happy path was
20+
* never what made it dangerous. So the drift case is first, and it asserts the
21+
* silent-pass sentence is ABSENT as well as asserting the failure.
22+
*
23+
* ## Why the bodies come from `sendOk` / `sendError`
24+
*
25+
* Those two functions in `@objectstack/types` are the one writer of the
26+
* declared envelope, so a fixture built through them is the server's shape by
27+
* construction. Typing this file's payloads out by hand would repeat the exact
28+
* mistake under repair: a copy of a server shape that stays self-consistent
29+
* while the server moves.
30+
*/
31+
32+
import { beforeAll, afterEach, describe, expect, it, vi } from 'vitest';
33+
import { Config } from '@oclif/core';
34+
import type { Command } from '@oclif/core';
35+
import { sendError, sendOk } from '@objectstack/types';
36+
import type { RemoteTable, SchemaValidationResult } from '@objectstack/spec/contracts';
37+
import { dirname, resolve } from 'node:path';
38+
import { fileURLToPath } from 'node:url';
39+
import { serverBody } from '../../utils/__tests__/server-body.js';
40+
import DatasourceIntrospect from './introspect.js';
41+
import DatasourceListTables from './list-tables.js';
42+
import DatasourceValidate from './validate.js';
43+
44+
const HERE = dirname(fileURLToPath(import.meta.url));
45+
/** This package's own root — `packages/cli`, never outside it. */
46+
const CLI_ROOT = resolve(HERE, '../../..');
47+
48+
const DS = 'showcase_external';
49+
const SERVER = 'http://127.0.0.1:39999';
50+
51+
/** The two tables the card's live-server oracle returned. */
52+
const REMOTE_TABLES: RemoteTable[] = [
53+
{ name: 'customers', columnCount: 7 },
54+
{ name: 'orders', columnCount: 7 },
55+
];
56+
57+
/**
58+
* The induced drift from the card: the fixture DB's `customers.region` was
59+
* renamed, and the server answered `ok:false … missing_column region`.
60+
*/
61+
const DRIFT_RESULT: SchemaValidationResult = {
62+
ok: false,
63+
datasource: DS,
64+
object: 'showcase_customers',
65+
diffs: [{ kind: 'missing_column', remoteName: 'customers', column: 'region', severity: 'error' }],
66+
};
67+
68+
const CLEAN_RESULT: SchemaValidationResult = {
69+
ok: true,
70+
datasource: DS,
71+
object: 'showcase_customers',
72+
diffs: [],
73+
};
74+
75+
/** The silent pass this card exists to make impossible. */
76+
const SILENT_PASS = 'No federated objects to validate.';
77+
78+
let config: Config;
79+
80+
beforeAll(async () => {
81+
config = await Config.load({ root: CLI_ROOT });
82+
});
83+
84+
afterEach(() => {
85+
vi.unstubAllGlobals();
86+
});
87+
88+
interface Driven {
89+
logs: string[];
90+
warns: string[];
91+
/** The URL the command actually requested — the route the oracle drove. */
92+
url?: string;
93+
/** Present when the command exited non-zero. */
94+
failure?: { message: string; exit?: number };
95+
}
96+
97+
type CommandCtor = new (argv: string[], config: Config) => Command;
98+
99+
/**
100+
* Run one command in-process against a fixed HTTP response, capturing what a
101+
* user would see. `run()` is driven directly rather than through oclif's
102+
* dispatcher so the assertions are about THIS class, not about command lookup.
103+
*/
104+
async function drive(
105+
Cmd: CommandCtor,
106+
argv: string[],
107+
response: { status: number; body: unknown },
108+
): Promise<Driven> {
109+
const logs: string[] = [];
110+
const warns: string[] = [];
111+
let url: string | undefined;
112+
113+
vi.stubGlobal('fetch', async (input: unknown) => {
114+
url = String(input);
115+
return {
116+
status: response.status,
117+
ok: response.status < 400,
118+
json: async () => response.body,
119+
};
120+
});
121+
122+
const cmd = new Cmd(argv, config);
123+
Object.assign(cmd, {
124+
log: (message?: string) => {
125+
logs.push(String(message ?? ''));
126+
},
127+
warn: (message: string | Error) => {
128+
warns.push(message instanceof Error ? message.message : String(message));
129+
return message;
130+
},
131+
});
132+
133+
try {
134+
await cmd.run();
135+
return { logs, warns, url };
136+
} catch (err) {
137+
const exit = (err as { oclif?: { exit?: number } }).oclif?.exit;
138+
return { logs, warns, url, failure: { message: (err as Error).message, exit } };
139+
}
140+
}
141+
142+
const target = [DS, '--url', SERVER, '--token', 'tok'];
143+
144+
describe('os datasource validate — the gate must not pass on drift it never read', () => {
145+
it('fails on induced drift the server flagged, instead of reporting nothing to validate', async () => {
146+
const run = await drive(DatasourceValidate, target, {
147+
status: 200,
148+
body: serverBody((res) => sendOk(res, { ok: false, results: [DRIFT_RESULT] })),
149+
});
150+
151+
// The defect: this sentence, with exit 0, against the body below it.
152+
expect(run.logs).not.toContain(SILENT_PASS);
153+
expect(run.logs.join('\n')).toContain('✗ missing_column: showcase_customers.region');
154+
expect(run.failure?.message).toBe('External schema validation failed.');
155+
expect(run.failure?.exit).toBe(1);
156+
expect(run.url).toBe(`${SERVER}/api/v1/datasources/${DS}/external/validate`);
157+
});
158+
159+
it('passes when the server reports every federated object matching', async () => {
160+
const run = await drive(DatasourceValidate, target, {
161+
status: 200,
162+
body: serverBody((res) => sendOk(res, { ok: true, results: [CLEAN_RESULT] })),
163+
});
164+
165+
expect(run.failure).toBeUndefined();
166+
expect(run.logs.join('\n')).toContain('✓ showcase_customers matches');
167+
expect(run.logs).not.toContain(SILENT_PASS);
168+
});
169+
170+
it('keeps "nothing to validate" reachable only from a server that really said so', async () => {
171+
const run = await drive(DatasourceValidate, target, {
172+
status: 200,
173+
body: serverBody((res) => sendOk(res, { ok: true, results: [] })),
174+
});
175+
176+
expect(run.failure).toBeUndefined();
177+
expect(run.logs).toContain(SILENT_PASS);
178+
});
179+
180+
it('refuses a body it cannot read rather than reporting it as zero results', async () => {
181+
// The pre-#3843 flat shape — i.e. any response that is not the declared
182+
// envelope. Reading it as "no results" is precisely the silent pass; a
183+
// consumer-side fallback that accepted it would be the second de-facto
184+
// contract Prime Directive #12 forbids.
185+
const run = await drive(DatasourceValidate, target, {
186+
status: 200,
187+
body: { ok: false, results: [DRIFT_RESULT] },
188+
});
189+
190+
expect(run.logs).not.toContain(SILENT_PASS);
191+
expect(run.failure?.message).toContain('envelope');
192+
});
193+
194+
it('prints the server error text for an unknown datasource instead of crashing on the error object', async () => {
195+
const run = await drive(DatasourceValidate, ['nope', '--url', SERVER, '--token', 'tok'], {
196+
status: 400,
197+
body: serverBody((res) =>
198+
sendError(res, 400, 'EXTERNAL_DATASOURCE_ERROR', "Datasource 'nope' is not configured."),
199+
),
200+
});
201+
202+
expect(run.failure?.message).toBe("Datasource 'nope' is not configured.");
203+
// The pre-fix crash — `this.error(<the error OBJECT>)`.
204+
expect(run.failure?.message).not.toContain('first argument must be a string');
205+
});
206+
});
207+
208+
describe('os datasource list-tables', () => {
209+
it('lists the tables the server returned, instead of reporting none found', async () => {
210+
const run = await drive(DatasourceListTables, target, {
211+
status: 200,
212+
body: serverBody((res) => sendOk(res, { tables: REMOTE_TABLES })),
213+
});
214+
215+
expect(run.failure).toBeUndefined();
216+
expect(run.logs).not.toContain('No remote tables found.');
217+
expect(run.logs.join('\n')).toContain('customers (7 cols)');
218+
expect(run.logs.join('\n')).toContain('orders (7 cols)');
219+
expect(run.url).toBe(`${SERVER}/api/v1/datasources/${DS}/external/tables`);
220+
});
221+
222+
it('keeps "no remote tables" reachable from an empty server list', async () => {
223+
const run = await drive(DatasourceListTables, target, {
224+
status: 200,
225+
body: serverBody((res) => sendOk(res, { tables: [] })),
226+
});
227+
228+
expect(run.failure).toBeUndefined();
229+
expect(run.logs).toContain('No remote tables found.');
230+
});
231+
232+
it('prints the server error text for an unknown datasource', async () => {
233+
const run = await drive(DatasourceListTables, ['nope', '--url', SERVER], {
234+
status: 400,
235+
body: serverBody((res) =>
236+
sendError(res, 400, 'EXTERNAL_DATASOURCE_ERROR', "Datasource 'nope' is not configured."),
237+
),
238+
});
239+
240+
expect(run.failure?.message).toBe("Datasource 'nope' is not configured.");
241+
expect(run.failure?.message).not.toContain('first argument must be a string');
242+
});
243+
});
244+
245+
describe('os datasource introspect', () => {
246+
// An opaque marker, deliberately: what this asserts is that the CLI emits the
247+
// source the SERVER produced. The draft's contents are the server's business
248+
// (#10712 fixes the namespace prefix / sharingModel gap, #10676 the primary
249+
// key), and pinning today's draft text here would block those fixes.
250+
const DRAFT_SOURCE = '/* draft source, verbatim from the server */';
251+
252+
it('emits the draft the server generated, instead of "Failed to generate draft"', async () => {
253+
const run = await drive(DatasourceIntrospect, [...target, '--table', 'customers'], {
254+
status: 200,
255+
body: serverBody((res) =>
256+
sendOk(res, {
257+
draft: {
258+
name: 'customers',
259+
datasource: DS,
260+
definition: {},
261+
source: DRAFT_SOURCE,
262+
review: [{ column: 'region', remoteType: 'jsonb', note: 'unmapped remote type' }],
263+
},
264+
}),
265+
),
266+
});
267+
268+
expect(run.failure).toBeUndefined();
269+
expect(run.logs).toContain(DRAFT_SOURCE);
270+
expect(run.warns.join('\n')).toContain("REVIEW: column 'region' — unmapped remote type");
271+
expect(run.url).toBe(`${SERVER}/api/v1/datasources/${DS}/external/tables/customers/draft`);
272+
});
273+
274+
it('still reports a genuinely absent draft', async () => {
275+
const run = await drive(DatasourceIntrospect, [...target, '--table', 'customers'], {
276+
status: 200,
277+
body: serverBody((res) => sendOk(res, {})),
278+
});
279+
280+
expect(run.failure?.message).toBe(`Failed to generate draft for 'customers' on '${DS}'.`);
281+
});
282+
283+
it('prints the server error text when the remote table does not exist', async () => {
284+
const run = await drive(DatasourceIntrospect, [...target, '--table', 'ghost'], {
285+
status: 400,
286+
body: serverBody((res) =>
287+
sendError(res, 400, 'EXTERNAL_DATASOURCE_ERROR', "Remote table 'ghost' not found."),
288+
),
289+
});
290+
291+
expect(run.failure?.message).toBe("Remote table 'ghost' not found.");
292+
expect(run.failure?.message).not.toContain('first argument must be a string');
293+
});
294+
});

‎packages/cli/src/commands/datasource/introspect.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { Args, Command, Flags } from '@oclif/core';
4+
import type { ObjectDraft } from '@objectstack/spec/contracts';
5+
import { readEnvelopeFrom } from '../../utils/response-envelope.js';
46
import { writeFile } from 'node:fs/promises';
57
import { resolve, isAbsolute } from 'node:path';
68

@@ -50,13 +52,16 @@ export default class DatasourceIntrospect extends Command {
5052
body: '{}',
5153
},
5254
);
53-
const body = (await res.json()) as {
54-
draft?: { source?: string; review?: Array<{ column: string; note: string }> };
55-
error?: string;
56-
};
57-
if (body.error) this.error(body.error);
55+
// `data.draft`, not `body.draft`: the server wraps every payload in the
56+
// declared envelope, so the flat read produced `undefined` and reported
57+
// "Failed to generate draft" for drafts the server had generated (#10675).
58+
const envelope = await readEnvelopeFrom<{ draft?: ObjectDraft }>(res);
59+
if (!envelope.ok) {
60+
this.error(envelope.message);
61+
return;
62+
}
5863

59-
const draft = body.draft;
64+
const draft = envelope.data.draft;
6065
if (!draft?.source) {
6166
this.error(`Failed to generate draft for '${flags.table}' on '${args.name}'.`);
6267
return;

‎packages/cli/src/commands/datasource/list-tables.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { Args, Command, Flags } from '@oclif/core';
4+
import type { RemoteTable } from '@objectstack/spec/contracts';
5+
import { readEnvelopeFrom } from '../../utils/response-envelope.js';
46

57
/** Resolve server URL + token from flags then env (mirrors createApiClient). */
68
function resolveTarget(flags: { url?: string; token?: string }): { url: string; token?: string } {
@@ -39,13 +41,17 @@ export default class DatasourceListTables extends Command {
3941
const res = await fetch(`${url}/api/v1/datasources/${args.name}/external/tables${qs}`, {
4042
headers: token ? { authorization: `Bearer ${token}` } : {},
4143
});
42-
const body = (await res.json()) as {
43-
tables?: Array<{ schema?: string; name: string; columnCount: number; rowCountEstimate?: number }>;
44-
error?: string;
45-
};
46-
if (body.error) this.error(body.error);
44+
// The payload lives under `data` in the declared envelope, and its shape is
45+
// the service contract's own `RemoteTable` rather than a transcription of
46+
// it — a copy is what silently reported "No remote tables found." against a
47+
// server that had listed two (#10675).
48+
const envelope = await readEnvelopeFrom<{ tables?: RemoteTable[] }>(res);
49+
if (!envelope.ok) {
50+
this.error(envelope.message);
51+
return;
52+
}
4753

48-
const tables = body.tables ?? [];
54+
const tables = envelope.data.tables ?? [];
4955
if (tables.length === 0) {
5056
this.log('No remote tables found.');
5157
return;

0 commit comments

Comments
 (0)