diff --git a/README.md b/README.md index ff4a451..2c65ed8 100644 --- a/README.md +++ b/README.md @@ -328,11 +328,11 @@ mb transform-tag delete 5 --yes ## Databases -Read warehouse metadata from `/api/database`. The `db` group exposes the full database list, the per-database record, schema and table inspection, the two manual-sync triggers, and (rarely useful) full-warehouse rollup endpoints. +Read warehouse metadata from `/api/database`. The `db` group exposes the full database list, the per-database record with optional table/field hydration, schema and table inspection, and the two manual-sync triggers. `db` is aliased to `database`. -> **Agent traversal:** prefer the granular path — `db list` → `db schemas ` → `db schema-tables ` → `table get --include fields`. On a real warehouse (dozens of schemas, hundreds of tables, dozens of fields per table) the rollup commands (`db metadata`, `db get --include tables.fields`, `db list --include tables`) return megabytes of JSON and exhaust the agent context. Reach for them only on small/dev warehouses where you know the size up front. +> **Agent traversal — the hydration ladder:** start with `db get --include tables`, the compact table map (id, name, schema, description per table) — one call that fits most databases. Pick the relevant tables, then fetch fields per table with `table fields ` (bounded: a table has at most a few hundred fields). `--include tables.fields` is the full rollup — small databases only. When output outgrows the `--max-bytes` cap, the error message names the next command down the ladder. On warehouses with hundreds of tables, traverse by schema (`db schemas ` → `db schema-tables `) or find tables by name (`mb search --models table --db-id `). ### `mb db list` @@ -340,34 +340,26 @@ Read warehouse metadata from `/api/database`. The `db` group exposes the full da mb db list mb db list --json mb db list --saved --json -mb db list --include tables --full --json # rollup: every db with its full table list +mb db list --include tables --json # every db with its compact table map ``` -| Flag | Description | -| ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--include ` | Hydrate related entities. Currently only `tables` is supported (each database is returned with its `tables`). On real warehouses this returns hundreds of table records per db — use the granular traversal instead. | -| `--saved` | Include the Saved Questions virtual database in the list. The virtual db has id `-1337` and no `engine`. | +| Flag | Description | +| ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--include ` | Hydrate related entities. Currently only `tables` is supported (each database is returned with its compact `tables`). To map a single warehouse, prefer `db get --include tables`. | +| `--saved` | Include the Saved Questions virtual database in the list. The virtual db has id `-1337` and no `engine`. | ### `mb db get ` ```sh mb db get 1 mb db get 1 --json -mb db get 1 --include tables --full --json # rollup: db + every table (compact) -mb db get 1 --include tables.fields --full --json # rollup: db + every table + every field +mb db get 1 --include tables --json # + compact table map (fits most databases) +mb db get 1 --include tables.fields --json # + every field of every table (small databases only) ``` -| Flag | Description | -| ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--include ` | Hydrate related entities. One of `tables` or `tables.fields`. `tables.fields` returns every column of every table in the database in one response — only safe on small/dev warehouses. For a real warehouse use `db schemas` → `db schema-tables` → `table get --include fields`. | - -### `mb db metadata ` - -Equivalent to `GET /api/database/:id/metadata`: a single database with all its tables and fields rolled up in one response. This is the largest read in the `db` group — on a real warehouse the response will exceed the agent context. Use only when you know the database is small (a seeded dev instance, a sample db, a freshly-bootstrapped test fixture). For agent-driven introspection on a real warehouse, walk `db schemas` → `db schema-tables` → `table get --include fields` instead. - -```sh -mb db metadata 1 --json --full --max-bytes 0 -``` +| Flag | Description | +| ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--include ` | Hydrate related entities. `tables` is the compact table map — the recommended first call for schema discovery. `tables.fields` is the full rollup and fits only small databases; on anything larger, take the map and fetch fields per table with `table fields `, or traverse by schema (see above). | ### `mb db schemas ` @@ -430,7 +422,7 @@ mb table list --db-id 1 --json ### `mb table get ` -Returns the basic table record (no fields). Pass `--include fields` to route through `/api/table/:id/query_metadata` so the response carries the table's columns compact-projected as `fields` — this is the default agent path for field introspection. Use `mb table fields ` if you only want the fields as a list envelope, or `mb table metadata ` when you also need FKs and dimensions hydrated. +Returns the basic table record (no fields). Pass `--include fields` to route through `/api/table/:id/query_metadata` so the response carries the table's columns compact-projected as `fields` — this is the default agent path for field introspection (the response also carries FK targets and dimensions under `--full`). Use `mb table fields ` if you only want the fields as a list envelope. ```sh mb table get 42 @@ -451,14 +443,6 @@ mb table fields 42 mb table fields 42 --json ``` -### `mb table metadata ` - -`GET /api/table/:id/query_metadata`: the table with its fields, FKs, dimensions, segments, and measures all hydrated. Heavier than `table get --include fields` — reach for it only when you actually need the FK / dimension / segment / measure data. - -```sh -mb table metadata 42 --json --full --max-bytes 0 -``` - ### `mb table update ` Patch a table (`PUT /api/table/:id`). Body fields: `display_name`, `description`, `caveats`, `points_of_interest`, `entity_type`, `visibility_type`, `field_order`, `show_in_getting_started`. Pass the body via `--body`, `--file`, or stdin (exactly one). @@ -1525,7 +1509,7 @@ mb skills path # absolute paths for direct Read mb skills path core # one path ``` -`mb skills get` honors the shared `--max-bytes` list cap. With the default 65 536 cap, `--all` will return only the first skill and emit a truncation notice — pass `--max-bytes 0` to dump every skill in one envelope. +`mb skills get` honors the shared `--max-bytes` list cap. With the default 24 576 cap, `--all` will return only the first skill and emit a truncation notice — pass `--max-bytes 0` to dump every skill in one envelope. Bundled skills: diff --git a/package.json b/package.json index 3689e18..cbbe443 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metabase/cli", - "version": "0.1.18", + "version": "0.2.0", "description": "Metabase CLI", "license": "AGPL-3.0", "repository": { diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index 44e32e4..2a654f3 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -59,22 +59,25 @@ Every list/get verb supports the same output flags: - `--json` — emit the full JSON envelope, safe for `jq`. Default is human-readable text. - `--full` — include every field (the compact projection is the default, and is the agent-facing contract). - `--fields a,b.c.d` — project specific dot-paths. Mutually exclusive with `--full`. **Paths are relative to each `data[]` item on list verbs, and to the root on single-item verbs.** So it's `--fields id,name` on `… list` / `database schema-tables` (`data.id` and `data[].id` both fail with `unknown field path: "data.id"`), and `--fields id,name,display` on `card get`, `--fields data.rows` on `mb query` (whose `data` is an object). -- `--max-bytes ` — cap **list** output size (drops trailing items, sets `truncated`). Default 65536; `0` disables. Single-item commands (`get`, `metadata`) never truncate — when their output exceeds the cap they throw a `ConfigError` (exit 2: "output is N bytes, over the M-byte --max-bytes cap…"); raise `--max-bytes` or narrow with `--fields`. +- `--max-bytes ` — cap **list** output size (drops trailing items, sets `truncated`). Default 24576 (sized to fit under agent-harness tool-output limits); `0` disables. Single-item commands (`get`) never truncate — when their output exceeds the cap they throw a `ConfigError` (exit 2: "output is N bytes, over the M-byte --max-bytes cap; …") whose tail names the remedy: on schema-shaped commands it is the exact narrower command to run instead — follow it rather than raising the cap. +- JSON output is a single line when stdout is piped (pretty-printed only at a TTY) — always parse it, never scrape by line position. -List envelope shape: +List envelope shape (metadata first, so counts and the truncation marker survive if a consumer cuts the tail): ```json { - "data": [ - /* items */ - ], "returned": 10, "total": 42, "limit": 50, - "truncated": false + "truncated": { "reason": "max_bytes", "bytes": 123456 }, + "data": [ + /* items */ + ] } ``` +`truncated` is present only when the cap dropped trailing items. + `total` is best-effort and may be `null` (empty / permissions-filtered collections, or `--limit` early-stop); use `returned` for the count you got and `data.length` for the rendered slice. ## Body input (create / update / run) @@ -118,8 +121,8 @@ mb transform --help --json | jq -r '.commands[].command' # verbs under "transfo Routine verb shapes (list / get / create / update), every flag, and output schemas live in each command's `--help` (add `--json` for output schemas). Below is only what help does _not_ tell you: footguns and non-obvious behaviors. -- **db traversal vs. rollup.** Default to granular: `database list` → `database schemas ` → `database schema-tables ` → `table get --include fields`. The rollup endpoints (`database get --include tables.fields`, `database metadata `) pull megabytes and blow the context window on any real warehouse — use them only on a small/dev db. `sync-schema` / `rescan-values` queue async work and return `{status:"ok"}` immediately; `sync-schema --wait` blocks until `initial_sync_status: complete`. -- **table fields.** `table get` never returns fields on its own — pass `--include fields` (compact) or use `table fields ` (list envelope). `table metadata ` adds FKs + dimensions (heavier). `table update` patches table-level metadata only; physical columns aren't editable. +- **db traversal: the hydration ladder.** Start with `database get --include tables` — the compact table map (id, name, schema, description per table), one call that fits most databases. Pick the relevant tables, then `table fields ` per table (bounded: fields are per-table). `--include tables.fields` is the full rollup — small databases only; when either outgrows the cap the error message names the next command down the ladder. Hundreds of tables? Traverse by schema (`database schemas ` → `database schema-tables `) or look tables up by name (`search --models table --db-id --limit 10`). `sync-schema` / `rescan-values` queue async work and return `{status:"ok"}` immediately; `sync-schema --wait` blocks until `initial_sync_status: complete`. +- **table fields.** `table get` never returns fields on its own — pass `--include fields` (compact; the underlying query_metadata response also carries FK targets and dimensions, visible under `--full`) or use `table fields ` (list envelope). `table update` patches table-level metadata only; physical columns aren't editable. - **field has no `list`.** Fields are per-table — get them via `table get --include fields`. Never enumerate fields across a whole db (context blow-up). `field summary` is live cardinality `{field_id, count, distincts}`; `field values` is the cached distinct set (`has_more_values: true` ⇒ truncated cache). `field update` patches metadata only (`base_type` isn't editable) — this is where you set a column's `semantic_type` or foreign-key target. - **upload (CSV → tables).** `upload csv --file ` creates a new table + model (prints `{model_id, table_id}`); `upload append ` / `upload replace --file ` add to / overwrite a table **previously created by upload** (columns must match). The destination db+schema is admin-configured, not per-call — check with `mb setting get uploads-settings --json` (`db_id: null` ⇒ uploads off/unconfigured; needs admin to read). `--collection ` only sets the model's collection. Max 50 MB. Errors: **"The uploads database is not configured."** = no db has uploads enabled; **"Uploads are not enabled."** = the append/replace target isn't an uploaded table. - **card.** `dataset_query` is the **flat** `mbql/query` value, not a legacy `{type:"query",query:…}` envelope (→ `mbql`). `--export-format csv|xlsx` streams the raw export (pipe to a file), bypassing the JSON envelope. `archive` is the only delete; unarchive with `update --body '{"archived":false}'`. `visualization_settings` keys are scoped by `display` and aren't pre-flighted — see `visualization`. diff --git a/skill-data/data-workflow/SKILL.md b/skill-data/data-workflow/SKILL.md index 88ed003..a1bb067 100644 --- a/skill-data/data-workflow/SKILL.md +++ b/skill-data/data-workflow/SKILL.md @@ -92,7 +92,7 @@ Don't make the user name a _stage_ — but do find out _where their data lives_ **Ask before you crawl.** If you don't already know which database/schema/table the user means, ask — one plain question short-circuits a dozen tool calls. The asymmetry: if they name a **database**, ask which **schema**; if they name a **table**, ask which **database**. "If you don't know, no problem — I'll look" is the fallback, not the opener. -**When you do crawl,** use `core`'s cheap, narrowest-first ladder (never whole-warehouse rollups): `mb db list` → `db schemas ` → `db schema-tables ` → `table list [--db-id]` → `table fields ` (or `table metadata ` for FK targets and dimensions — heavier). Have a _name_ rather than a tree to walk? `mb search [--models] [--db-id]`. Need to know what's in a column? `mb field summary ` (counts) and `field values ` (sample values). If a database looks freshly connected or an expected table is missing, offer `mb db sync-schema --wait` before concluding it doesn't exist. +**When you do crawl,** use `core`'s cheap, narrowest-first ladder (never whole-warehouse rollups): `mb db list` → `db schemas ` → `db schema-tables ` → `table list [--db-id]` → `table fields ` (or `table get --include fields --full` for FK targets and dimensions). Have a _name_ rather than a tree to walk? `mb search [--models] [--db-id]`. Need to know what's in a column? `mb field summary ` (counts) and `field values ` (sample values). If a database looks freshly connected or an expected table is missing, offer `mb db sync-schema --wait` before concluding it doesn't exist. **Read the shape to pick a stage.** Raw, normalized, SaaS-synced tables (many tables, coded columns, `*_field`/`*_choice` lookups)? → **build clean tables** first. Already wide, clean, human-readable ones? Then it depends on the goal: diff --git a/src/commands/context.test.ts b/src/commands/context.test.ts index 72360b5..5744c08 100644 --- a/src/commands/context.test.ts +++ b/src/commands/context.test.ts @@ -107,8 +107,8 @@ describe("resolveCommonFlags — fields CSV parsing", () => { }); describe("resolveCommonFlags — maxBytes parsing", () => { - it("uses default 65536 when omitted", () => { - expect(resolveCommonFlags({}, { isTty: true }).maxBytes).toBe(65536); + it("uses default 24576 when omitted", () => { + expect(resolveCommonFlags({}, { isTty: true }).maxBytes).toBe(24576); }); it("parses '0' as 0 (disables cap)", () => { @@ -144,7 +144,7 @@ describe("resolveCommonFlags — full result shape", () => { format: "text", full: false, fields: undefined, - maxBytes: 65536, + maxBytes: 24576, url: undefined, apiKey: undefined, profile: undefined, diff --git a/src/commands/dashboard/preflight.test.ts b/src/commands/dashboard/preflight.test.ts index fd62b19..0cfe9a8 100644 --- a/src/commands/dashboard/preflight.test.ts +++ b/src/commands/dashboard/preflight.test.ts @@ -87,6 +87,7 @@ describe("preflightDashcardCardReferences", () => { beforeEach(() => { captured = []; + process.stdout.isTTY = false; vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { captured.push(String(chunk)); return true; @@ -149,11 +150,10 @@ describe("preflightDashcardCardReferences", () => { "dashboard card-reference pre-flight failed: 1 error(s) — fix the dashcard card_id values listed above", ); expect(captured.join("")).toBe( - `${JSON.stringify( - { ok: false, errors: [{ path: "/dashcards/0/card_id", message: "card 134 is archived" }] }, - null, - 2, - )}\n`, + `${JSON.stringify({ + ok: false, + errors: [{ path: "/dashcards/0/card_id", message: "card 134 is archived" }], + })}\n`, ); }); @@ -170,17 +170,13 @@ describe("preflightDashcardCardReferences", () => { "dashboard card-reference pre-flight failed: 2 error(s) — fix the dashcard card_id values listed above", ); expect(captured.join("")).toBe( - `${JSON.stringify( - { - ok: false, - errors: [ - { path: "/dashcards/0/card_id", message: "card 134 is archived" }, - { path: "/dashcards/1/card_id", message: "card 134 is archived" }, - ], - }, - null, - 2, - )}\n`, + `${JSON.stringify({ + ok: false, + errors: [ + { path: "/dashcards/0/card_id", message: "card 134 is archived" }, + { path: "/dashcards/1/card_id", message: "card 134 is archived" }, + ], + })}\n`, ); }); @@ -202,11 +198,10 @@ describe("preflightDashcardCardReferences", () => { "dashboard card-reference pre-flight failed: 1 error(s) — fix the dashcard card_id values listed above", ); expect(captured.join("")).toBe( - `${JSON.stringify( - { ok: false, errors: [{ path: "/dashcards/0/card_id", message: "card 9999 not found" }] }, - null, - 2, - )}\n`, + `${JSON.stringify({ + ok: false, + errors: [{ path: "/dashcards/0/card_id", message: "card 9999 not found" }], + })}\n`, ); }); @@ -228,19 +223,15 @@ describe("preflightDashcardCardReferences", () => { "dashboard card-reference pre-flight failed: 1 error(s) — fix the dashcard card_id values listed above", ); expect(captured.join("")).toBe( - `${JSON.stringify( - { - ok: false, - errors: [ - { - path: "/dashcards/0/card_id", - message: "card 55 is not readable: You do not have permissions to do that.", - }, - ], - }, - null, - 2, - )}\n`, + `${JSON.stringify({ + ok: false, + errors: [ + { + path: "/dashcards/0/card_id", + message: "card 55 is not readable: You do not have permissions to do that.", + }, + ], + })}\n`, ); }); diff --git a/src/commands/db/get.ts b/src/commands/db/get.ts index 6821739..94c1a3d 100644 --- a/src/commands/db/get.ts +++ b/src/commands/db/get.ts @@ -7,10 +7,27 @@ import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseId } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; +import { fullRollupOversizeHint, tableMapOversizeHint } from "./hints"; + const DatabaseGetInclude = z.enum(["tables", "tables.fields"]); +function includeOversizeHint( + include: z.infer | undefined, + id: number, +): string | undefined { + if (include === "tables.fields") { + return fullRollupOversizeHint(id); + } + if (include === "tables") { + return tableMapOversizeHint(id); + } + return undefined; +} + export default defineMetabaseCommand({ meta: { name: "get", description: "Get a database by id" }, + details: + "`--include tables` returns the table map (id, name, schema, description per table) — one call that fits most databases. `--include tables.fields` is the full rollup; on all but small databases prefer the map plus `mb table fields ` per table of interest.", capabilities: { minVersion: 58 }, args: { ...outputFlags, @@ -18,12 +35,17 @@ export default defineMetabaseCommand({ ...connectionFlags, include: { type: "string", - description: `Hydrate related entities: ${DatabaseGetInclude.options.join("|")}. tables.fields pulls every table and field — megabytes on a real warehouse; prefer db schemas / db schema-tables / table get --include fields`, + description: `Hydrate related entities: ${DatabaseGetInclude.options.join("|")}. tables is the compact table map; tables.fields adds every field — fine for small databases, use the map plus table fields for large ones`, }, id: { type: "positional", description: "Database id", required: true }, }, outputSchema: Database, - examples: ["mb db get 1", "mb db get 1 --json", "mb db get 1 --include tables.fields --json"], + examples: [ + "mb db get 1", + "mb db get 1 --json", + "mb db get 1 --include tables --json", + "mb db get 1 --include tables.fields --json", + ], async run({ args, ctx, getClient }) { const id = parseId(args.id); const include = parseEnum(args.include, DatabaseGetInclude, "--include"); @@ -31,6 +53,6 @@ export default defineMetabaseCommand({ const database = await client.requestParsed(Database, `/api/database/${id}`, { query: { include }, }); - renderItem(database, databaseView, ctx); + renderItem(database, databaseView, { ...ctx, oversizeHint: includeOversizeHint(include, id) }); }, }); diff --git a/src/commands/db/hints.ts b/src/commands/db/hints.ts new file mode 100644 index 0000000..1c69e10 --- /dev/null +++ b/src/commands/db/hints.ts @@ -0,0 +1,15 @@ +export function fullRollupOversizeHint(dbId: number): string { + return ( + `this database is too large for a full rollup — get the table map with ` + + `\`mb db get ${dbId} --include tables\`, then fetch fields per table of interest with ` + + `\`mb table fields \`` + ); +} + +export function tableMapOversizeHint(dbId: number): string { + return ( + `this database has too many tables for one map — traverse by schema with ` + + `\`mb db schemas ${dbId}\` + \`mb db schema-tables ${dbId} \`, or find tables ` + + `by name with \`mb search --models table --db-id ${dbId}\`` + ); +} diff --git a/src/commands/db/index.ts b/src/commands/db/index.ts index 3f40aef..deeae65 100644 --- a/src/commands/db/index.ts +++ b/src/commands/db/index.ts @@ -11,7 +11,6 @@ export default defineCommandGroup({ subCommands: { list: () => import("./list").then((m) => m.default), get: () => import("./get").then((m) => m.default), - metadata: () => import("./metadata").then((m) => m.default), schemas: () => import("./schemas").then((m) => m.default), "schema-tables": () => import("./schema-tables").then((m) => m.default), "sync-schema": () => import("./sync-schema").then((m) => m.default), diff --git a/src/commands/db/metadata.ts b/src/commands/db/metadata.ts deleted file mode 100644 index 8e6490e..0000000 --- a/src/commands/db/metadata.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Database, databaseView } from "../../domain/database"; -import { renderItem } from "../../output/render"; -import { connectionFlags, outputFlags, profileFlag } from "../flags"; -import { parseId } from "../parse-id"; -import { defineMetabaseCommand } from "../runtime"; - -export default defineMetabaseCommand({ - meta: { - name: "metadata", - description: "Get a database with its tables and fields hydrated", - }, - details: - "Hydrates every table and field — megabytes on a real warehouse. For targeted reads use `db schemas`, `db schema-tables`, then `table get --include fields`.", - capabilities: { minVersion: 58 }, - args: { - ...outputFlags, - ...profileFlag, - ...connectionFlags, - id: { type: "positional", description: "Database id", required: true }, - }, - outputSchema: Database, - examples: ["mb db metadata 1", "mb db metadata 1 --json"], - async run({ args, ctx, getClient }) { - const id = parseId(args.id); - const client = await getClient(); - const database = await client.requestParsed(Database, `/api/database/${id}/metadata`); - renderItem(database, databaseView, ctx); - }, -}); diff --git a/src/commands/table/get.ts b/src/commands/table/get.ts index d7d8738..edec2a8 100644 --- a/src/commands/table/get.ts +++ b/src/commands/table/get.ts @@ -7,6 +7,8 @@ import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseId } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; +import { tableFieldsOversizeHint } from "./hints"; + const TableGetInclude = z.enum(["fields"]); export default defineMetabaseCommand({ @@ -40,7 +42,7 @@ export default defineMetabaseCommand({ TableQueryMetadata, `/api/table/${id}/query_metadata`, ); - renderItem(table, tableView, ctx); + renderItem(table, tableView, { ...ctx, oversizeHint: tableFieldsOversizeHint(id) }); return; } const table = await client.requestParsed(Table, `/api/table/${id}`); diff --git a/src/commands/table/hints.ts b/src/commands/table/hints.ts new file mode 100644 index 0000000..3ef7e5f --- /dev/null +++ b/src/commands/table/hints.ts @@ -0,0 +1,3 @@ +export function tableFieldsOversizeHint(tableId: number): string { + return `use \`mb table fields ${tableId}\` — list output truncates gracefully instead of failing`; +} diff --git a/src/commands/table/index.ts b/src/commands/table/index.ts index adc2a78..1ba2e3b 100644 --- a/src/commands/table/index.ts +++ b/src/commands/table/index.ts @@ -7,7 +7,6 @@ export default defineCommandGroup({ subCommands: { list: () => import("./list").then((m) => m.default), get: () => import("./get").then((m) => m.default), - metadata: () => import("./metadata").then((m) => m.default), fields: () => import("./fields").then((m) => m.default), update: () => import("./update").then((m) => m.default), }, diff --git a/src/commands/table/metadata.ts b/src/commands/table/metadata.ts deleted file mode 100644 index 45105b1..0000000 --- a/src/commands/table/metadata.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { TableQueryMetadata, tableView } from "../../domain/table"; -import { renderItem } from "../../output/render"; -import { connectionFlags, outputFlags, profileFlag } from "../flags"; -import { parseId } from "../parse-id"; -import { defineMetabaseCommand } from "../runtime"; - -export default defineMetabaseCommand({ - meta: { - name: "metadata", - description: "Get a table with its fields, FKs, and dimensions hydrated", - }, - capabilities: { minVersion: 58 }, - args: { - ...outputFlags, - ...profileFlag, - ...connectionFlags, - id: { type: "positional", description: "Table id", required: true }, - }, - outputSchema: TableQueryMetadata, - examples: ["mb table metadata 42", "mb table metadata 42 --json"], - async run({ args, ctx, getClient }) { - const id = parseId(args.id); - const client = await getClient(); - const table = await client.requestParsed(TableQueryMetadata, `/api/table/${id}/query_metadata`); - renderItem(table, tableView, ctx); - }, -}); diff --git a/src/domain/database.ts b/src/domain/database.ts index 3127510..83c68c7 100644 --- a/src/domain/database.ts +++ b/src/domain/database.ts @@ -1,6 +1,6 @@ import { z } from "zod"; -import { Table } from "./table"; +import { Table, TableCompact } from "./table"; import type { ResourceView } from "./view"; export const Database = z @@ -20,7 +20,11 @@ export const DatabaseCompact = Database.pick({ name: true, engine: true, is_saved_questions: true, -}).strip(); +}) + .strip() + .extend({ + tables: z.array(TableCompact).optional(), + }); export type DatabaseCompact = z.infer; export const databaseView: ResourceView = { diff --git a/src/output/error.test.ts b/src/output/error.test.ts index 74488f8..5b797ae 100644 --- a/src/output/error.test.ts +++ b/src/output/error.test.ts @@ -20,6 +20,7 @@ const originalVerbose = process.env["MB_VERBOSE"]; beforeEach(() => { streams = { stderr: "" }; + process.stderr.isTTY = false; vi.spyOn(process.stderr, "write").mockImplementation((chunk) => { streams.stderr += String(chunk); return true; @@ -67,7 +68,7 @@ describe("reportError", () => { process.env["MB_VERBOSE"] = "1"; reportError(new UnknownError({ originalMessage: "boom", stack: "trace" })); expect(streams.stderr).toBe( - "boom\n" + JSON.stringify({ originalMessage: "boom", stack: "trace" }, null, 2) + "\n", + "boom\n" + JSON.stringify({ originalMessage: "boom", stack: "trace" }) + "\n", ); }); @@ -130,11 +131,10 @@ describe("reportError", () => { it("emits a JSON error envelope to stderr (no detail) when format is json", () => { reportError(new ConfigError("missing TTY"), "json"); const expected = - JSON.stringify( - { ok: false, error: { category: "config", message: "missing TTY", exitCode: 2 } }, - null, - 2, - ) + "\n"; + JSON.stringify({ + ok: false, + error: { category: "config", message: "missing TTY", exitCode: 2 }, + }) + "\n"; expect(streams.stderr).toBe(expected); expect(process.exitCode).toBe(2); }); @@ -142,11 +142,10 @@ describe("reportError", () => { it("omits detail from the JSON error envelope when MB_VERBOSE is unset", () => { reportError(new UnknownError({ originalMessage: "boom", stack: "trace" }), "json"); const expected = - JSON.stringify( - { ok: false, error: { category: "unknown", message: "boom", exitCode: 1 } }, - null, - 2, - ) + "\n"; + JSON.stringify({ + ok: false, + error: { category: "unknown", message: "boom", exitCode: 1 }, + }) + "\n"; expect(streams.stderr).toBe(expected); expect(process.exitCode).toBe(1); }); @@ -155,19 +154,15 @@ describe("reportError", () => { process.env["MB_VERBOSE"] = "1"; reportError(new UnknownError({ originalMessage: "boom", stack: "trace" }), "json"); const expected = - JSON.stringify( - { - ok: false, - error: { - category: "unknown", - message: "boom", - exitCode: 1, - detail: { originalMessage: "boom", stack: "trace" }, - }, + JSON.stringify({ + ok: false, + error: { + category: "unknown", + message: "boom", + exitCode: 1, + detail: { originalMessage: "boom", stack: "trace" }, }, - null, - 2, - ) + "\n"; + }) + "\n"; expect(streams.stderr).toBe(expected); }); }); diff --git a/src/output/error.ts b/src/output/error.ts index 3ec1095..561c30e 100644 --- a/src/output/error.ts +++ b/src/output/error.ts @@ -3,6 +3,7 @@ import { toMetabaseError } from "../core/errors"; import type { ErrorCategory, MetabaseError } from "../core/errors"; import { warn } from "./notice"; +import { serializeJson } from "./render"; import type { Format } from "./types"; const VERBOSE_BREADCRUMB = "(rerun with MB_VERBOSE=1 for details)"; @@ -39,12 +40,16 @@ function writeTextError(handled: MetabaseError, verbose: boolean): void { return; } if (verbose) { - process.stderr.write(JSON.stringify(handled.developerDetail, null, 2) + "\n"); + process.stderr.write(serializeJson(handled.developerDetail, stderrPretty()) + "\n"); } else { process.stderr.write(VERBOSE_BREADCRUMB + "\n"); } } +function stderrPretty(): boolean { + return process.stderr.isTTY === true; +} + function writeJsonError(handled: MetabaseError, verbose: boolean): void { const payload: JsonErrorPayload = { category: handled.category, @@ -55,5 +60,5 @@ function writeJsonError(handled: MetabaseError, verbose: boolean): void { payload.detail = handled.developerDetail; } const envelope: JsonErrorEnvelope = { ok: false, error: payload }; - process.stderr.write(JSON.stringify(envelope, null, 2) + "\n"); + process.stderr.write(serializeJson(envelope, stderrPretty()) + "\n"); } diff --git a/src/output/notice.test.ts b/src/output/notice.test.ts index 789af72..915e36b 100644 --- a/src/output/notice.test.ts +++ b/src/output/notice.test.ts @@ -25,14 +25,22 @@ describe("warn", () => { describe("listTruncationNotice", () => { it("formats the byte count and a recovery hint", () => { - expect(listTruncationNotice(2048)).toBe("… cut at 2048 bytes; rerun with --max-bytes 0"); + expect(listTruncationNotice(2048)).toBe( + "… cut at 2048 bytes; narrow the selection or raise --max-bytes", + ); }); }); describe("itemOversizeMessage", () => { it("names both the actual byte count and the configured cap alongside the knobs", () => { expect(itemOversizeMessage(4096, 1024)).toBe( - "output is 4096 bytes, over the 1024-byte --max-bytes cap; narrow with --fields, or pass --max-bytes 0 to disable", + "output is 4096 bytes, over the 1024-byte --max-bytes cap; narrow with --fields or raise the cap with --max-bytes ", + ); + }); + + it("replaces the generic remedy with the command-specific hint when one is supplied", () => { + expect(itemOversizeMessage(4096, 1024, "use `mb db get 1 --include tables`")).toBe( + "output is 4096 bytes, over the 1024-byte --max-bytes cap; use `mb db get 1 --include tables`", ); }); }); diff --git a/src/output/notice.ts b/src/output/notice.ts index 5ab53cd..7504ae6 100644 --- a/src/output/notice.ts +++ b/src/output/notice.ts @@ -3,9 +3,11 @@ export function warn(message: string): void { } export function listTruncationNotice(bytes: number): string { - return `… cut at ${bytes} bytes; rerun with --max-bytes 0`; + return `… cut at ${bytes} bytes; narrow the selection or raise --max-bytes`; } -export function itemOversizeMessage(bytes: number, maxBytes: number): string { - return `output is ${bytes} bytes, over the ${maxBytes}-byte --max-bytes cap; narrow with --fields, or pass --max-bytes 0 to disable`; +const ITEM_OVERSIZE_REMEDY = "narrow with --fields or raise the cap with --max-bytes "; + +export function itemOversizeMessage(bytes: number, maxBytes: number, hint?: string): string { + return `output is ${bytes} bytes, over the ${maxBytes}-byte --max-bytes cap; ${hint ?? ITEM_OVERSIZE_REMEDY}`; } diff --git a/src/output/render.test.ts b/src/output/render.test.ts index be9a1e9..ef20360 100644 --- a/src/output/render.test.ts +++ b/src/output/render.test.ts @@ -6,7 +6,7 @@ import type { ResourceView } from "../domain/view"; import { parseJson } from "../runtime/json"; import { capListEnvelope } from "./cap"; import { renderSummary, renderItem, renderList, writeJson, writeText } from "./render"; -import type { ListEnvelope, RenderOptions } from "./types"; +import { DEFAULT_MAX_BYTES, type ListEnvelope, type RenderOptions } from "./types"; const Card = z.object({ id: z.number().int(), @@ -27,7 +27,7 @@ const baseOpts: RenderOptions = { format: "json", full: false, fields: undefined, - maxBytes: 65536, + maxBytes: DEFAULT_MAX_BYTES, }; const TruncatedEnvelope = z.object({ @@ -59,6 +59,7 @@ let streams: Streams; beforeEach(() => { streams = { stdout: "", stderr: "" }; + process.stdout.isTTY = false; vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { streams.stdout += String(chunk); return true; @@ -124,7 +125,7 @@ describe("renderItem", () => { it("throws ConfigError and writes nothing when a single item exceeds maxBytes", () => { const longName = "x".repeat(200); const item: Card = { id: 1, name: longName, archived: false }; - const expectedBytes = Buffer.byteLength(JSON.stringify(item, null, 2) + "\n", "utf8"); + const expectedBytes = Buffer.byteLength(JSON.stringify(item) + "\n", "utf8"); const error = (() => { try { renderItem(item, cardView, { ...baseOpts, full: true, maxBytes: 50 }); @@ -136,16 +137,40 @@ describe("renderItem", () => { expect(error).toBeInstanceOf(ConfigError); assert(error instanceof ConfigError, "expected ConfigError"); expect(error.message).toBe( - `output is ${expectedBytes} bytes, over the 50-byte --max-bytes cap; narrow with --fields, or pass --max-bytes 0 to disable`, + `output is ${expectedBytes} bytes, over the 50-byte --max-bytes cap; narrow with --fields or raise the cap with --max-bytes `, ); expect(error.exitCode).toBe(2); expect(streams.stdout).toBe(""); }); + it("surfaces the command-supplied oversize hint in the ConfigError message", () => { + const longName = "x".repeat(200); + const item: Card = { id: 1, name: longName, archived: false }; + const expectedBytes = Buffer.byteLength(JSON.stringify(item) + "\n", "utf8"); + const error = (() => { + try { + renderItem(item, cardView, { + ...baseOpts, + full: true, + maxBytes: 50, + oversizeHint: "use `mb card list` instead", + }); + } catch (caught: unknown) { + return caught; + } + throw new Error("expected renderItem to throw"); + })(); + expect(error).toBeInstanceOf(ConfigError); + assert(error instanceof ConfigError, "expected ConfigError"); + expect(error.message).toBe( + `output is ${expectedBytes} bytes, over the 50-byte --max-bytes cap; use \`mb card list\` instead`, + ); + }); + it("does not throw when item fits inside maxBytes", () => { renderItem({ id: 1, name: "Sales", archived: false }, cardView, { ...baseOpts, - maxBytes: 65536, + maxBytes: DEFAULT_MAX_BYTES, }); expect(streams.stderr).toBe(""); }); @@ -256,9 +281,30 @@ describe("renderList — JSON format", () => { expect(parseJson(streams.stdout, TruncatedEnvelope)).toEqual(expectedCapped); expect(streams.stderr).toBe( - `… cut at ${expectedCapped.truncated.bytes} bytes; rerun with --max-bytes 0\n`, + `… cut at ${expectedCapped.truncated.bytes} bytes; narrow the selection or raise --max-bytes\n`, ); }); + + it("serializes envelope metadata before data so a tail cut cannot hide truncation", () => { + const items: Card[] = Array.from({ length: 50 }, (_, index) => ({ + id: index, + name: `card-${"x".repeat(40)}-${index}`, + archived: false, + })); + renderList({ data: items, returned: items.length, total: items.length }, cardView, { + ...baseOpts, + maxBytes: 500, + }); + + const projectedItems = items.map(({ id, name }) => ({ id, name })); + const expectedCapped = capListEnvelope( + { data: projectedItems, returned: items.length, total: items.length }, + 500, + ); + assert(expectedCapped.truncated !== undefined, "fixture should produce truncation"); + const prefix = `{"returned":${expectedCapped.returned},"total":${expectedCapped.total},"truncated":{"reason":"max_bytes","bytes":${expectedCapped.truncated.bytes}},"data":[`; + expect(streams.stdout.slice(0, prefix.length)).toBe(prefix); + }); }); describe("renderList — text format", () => { @@ -348,7 +394,7 @@ describe("renderList — text format", () => { assert(expectedCapped.truncated !== undefined, "fixture should produce truncation"); expect(streams.stdout).toContain("ID"); expect(streams.stderr).toBe( - `… cut at ${expectedCapped.truncated.bytes} bytes; rerun with --max-bytes 0\n`, + `… cut at ${expectedCapped.truncated.bytes} bytes; narrow the selection or raise --max-bytes\n`, ); }); }); @@ -388,8 +434,18 @@ describe("renderList — --fields path errors", () => { }); describe("writeJson", () => { - it("emits the value pretty-printed with a trailing newline", () => { + it("emits single-line JSON with a trailing newline when stdout is not a TTY", () => { writeJson({ a: 1, b: ["x", "y"] }); + expect(streams.stdout).toBe('{"a":1,"b":["x","y"]}\n'); + }); + + it("pretty-prints when stdout is a TTY", () => { + process.stdout.isTTY = true; + try { + writeJson({ a: 1, b: ["x", "y"] }); + } finally { + process.stdout.isTTY = false; + } expect(streams.stdout).toBe('{\n "a": 1,\n "b": [\n "x",\n "y"\n ]\n}\n'); }); }); diff --git a/src/output/render.ts b/src/output/render.ts index bbf5d5e..e29b6a5 100644 --- a/src/output/render.ts +++ b/src/output/render.ts @@ -9,8 +9,18 @@ import type { ListEnvelope, RenderOptions } from "./types"; export { formatScalar } from "./table"; +// Pretty-printing is pure token overhead for the machine consumers on the other side of a +// pipe (~40% of the bytes), so indentation is reserved for humans at a terminal. +export function serializeJson(value: unknown, pretty: boolean): string { + return JSON.stringify(value, null, pretty ? 2 : undefined); +} + +function stdoutPretty(): boolean { + return process.stdout.isTTY === true; +} + export function jsonLine(value: unknown): string { - return JSON.stringify(value, null, 2) + "\n"; + return serializeJson(value, stdoutPretty()) + "\n"; } export function writeJson(value: unknown): void { @@ -26,7 +36,7 @@ type KeyValuePair = readonly [label: string, value: string]; export function renderItem(item: T, view: ResourceView, opts: RenderOptions): void { const projected = applyProjection(item, view, opts.full, opts.fields); const body = renderItemBody(item, view, projected, opts) + "\n"; - assertItemWithinMaxBytes(body, opts.maxBytes); + assertItemWithinMaxBytes(body, opts); process.stdout.write(body); } @@ -48,7 +58,7 @@ export function renderSummary( return; } const body = (typeof summaryText === "function" ? summaryText() : summaryText) + "\n"; - assertItemWithinMaxBytes(body, opts.maxBytes); + assertItemWithinMaxBytes(body, opts); process.stdout.write(body); } @@ -139,7 +149,16 @@ function renderJsonEnvelope( const projectedItems = projectListItems(envelope.data, view, opts.full, opts.fields); const projectedEnvelope: ListEnvelope = { ...envelope, data: projectedItems }; const capped = capListEnvelope(projectedEnvelope, opts.maxBytes); - process.stdout.write(JSON.stringify(capped, null, 2) + "\n"); + // Metadata precedes `data` so counts and the truncation marker survive when a downstream + // consumer (an agent harness, a pager) cuts the tail of the output. + const ordered: ListEnvelope = { + returned: capped.returned, + total: capped.total, + limit: capped.limit, + truncated: capped.truncated, + data: capped.data, + }; + process.stdout.write(serializeJson(ordered, stdoutPretty()) + "\n"); if (capped.truncated !== undefined) { warn(listTruncationNotice(capped.truncated.bytes)); } @@ -152,7 +171,7 @@ function renderItemBody( opts: RenderOptions, ): string { if (opts.format === "json") { - return JSON.stringify(projected, null, 2); + return serializeJson(projected, stdoutPretty()); } if (opts.fields !== undefined || opts.full) { return renderKeyValueLines(objectPairs(projected)); @@ -180,13 +199,13 @@ function renderKeyValueLines(pairs: ReadonlyArray): string { return pairs.map(([label, value]) => `${label.padEnd(padding)} ${value}`).join("\n"); } -function assertItemWithinMaxBytes(body: string, maxBytes: number): void { - if (maxBytes <= 0) { +function assertItemWithinMaxBytes(body: string, opts: RenderOptions): void { + if (opts.maxBytes <= 0) { return; } const bytes = Buffer.byteLength(body, "utf8"); - if (bytes <= maxBytes) { + if (bytes <= opts.maxBytes) { return; } - throw new ConfigError(itemOversizeMessage(bytes, maxBytes)); + throw new ConfigError(itemOversizeMessage(bytes, opts.maxBytes, opts.oversizeHint)); } diff --git a/src/output/skill-list.test.ts b/src/output/skill-list.test.ts index b57dd1f..196eacd 100644 --- a/src/output/skill-list.test.ts +++ b/src/output/skill-list.test.ts @@ -67,7 +67,7 @@ describe("renderSkillList", () => { expect(streams.stdout).toBe("core\n Drive a Metabase instance from the\n terminal.\n\n"); expect(streams.stderr).toBe( - `… cut at ${Buffer.byteLength(full, "utf8")} bytes; rerun with --max-bytes 0\n`, + `… cut at ${Buffer.byteLength(full, "utf8")} bytes; narrow the selection or raise --max-bytes\n`, ); }); }); diff --git a/src/output/types.ts b/src/output/types.ts index 3ddc3f2..2932d31 100644 --- a/src/output/types.ts +++ b/src/output/types.ts @@ -2,7 +2,10 @@ import { z, type ZodType } from "zod"; export type Format = "json" | "text"; -export const DEFAULT_MAX_BYTES = 65536; +// Sized to stay under agent-harness tool-output truncation (~30k chars in Claude Code's Bash +// tool): the CLI's own cap must fire, with its teaching error, before the transport silently +// cuts JSON mid-object. +export const DEFAULT_MAX_BYTES = 24576; export interface TruncationInfo { reason: "max_bytes"; @@ -41,4 +44,5 @@ export interface RenderOptions { full: boolean; fields: string[] | undefined; maxBytes: number; + oversizeHint?: string | undefined; } diff --git a/src/runtime/command-help.test.ts b/src/runtime/command-help.test.ts index b1ff5b5..b7c5b94 100644 --- a/src/runtime/command-help.test.ts +++ b/src/runtime/command-help.test.ts @@ -249,14 +249,12 @@ const ALL_COMMANDS = [ "auth logout", "db list", "db get", - "db metadata", "db schemas", "db schema-tables", "db sync-schema", "db rescan-values", "table list", "table get", - "table metadata", "table fields", "table update", "field get", diff --git a/tests/e2e/cli-error.ts b/tests/e2e/cli-error.ts index 3fe856c..7e4fbff 100644 --- a/tests/e2e/cli-error.ts +++ b/tests/e2e/cli-error.ts @@ -18,10 +18,10 @@ type CliErrorEnvelope = z.infer; // The e2e harness is never a TTY, so the CLI renders errors as a JSON envelope on stderr (the // agent-facing path); in a TTY it renders plain text. Any leading warn() lines (e.g. preflight -// notices) precede the envelope, which always opens with a bare "{" line. +// notices) precede the envelope, whose first line starts with "{". function parseEnvelope(stderr: string): CliErrorEnvelope | null { const lines = stderr.split("\n"); - const start = lines.indexOf("{"); + const start = lines.findIndex((line) => line.startsWith("{")); if (start === -1) { return null; } diff --git a/tests/e2e/db.e2e.test.ts b/tests/e2e/db.e2e.test.ts index 13299b4..d9b965d 100644 --- a/tests/e2e/db.e2e.test.ts +++ b/tests/e2e/db.e2e.test.ts @@ -1,11 +1,12 @@ import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { fullRollupOversizeHint, tableMapOversizeHint } from "../../src/commands/db/hints"; import { DatabaseListEnvelope } from "../../src/commands/db/list"; import { DatabaseSchemaListEnvelope } from "../../src/commands/db/schemas"; import { DatabaseSchemaTablesEnvelope } from "../../src/commands/db/schema-tables"; import { Database, DatabaseCompact, DatabaseSyncResult } from "../../src/domain/database"; import { TableCompact } from "../../src/domain/table"; -import { listEnvelopeSchema } from "../../src/output/types"; +import { DEFAULT_MAX_BYTES, listEnvelopeSchema } from "../../src/output/types"; import { parseJson } from "../../src/runtime/json"; import { readBootstrap, type E2EBootstrap } from "./bootstrap-data"; @@ -90,6 +91,11 @@ const ANALYTICS_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ }, ]; +const ALL_WAREHOUSE_TABLES_SORTED_BY_ID: TableCompact[] = [ + ...PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME, + ...ANALYTICS_TABLES_SORTED_BY_DISPLAY_NAME, +].toSorted((a, b) => a.id - b.id); + describe("db e2e", () => { let bootstrap: E2EBootstrap; const tempDirs: string[] = []; @@ -139,17 +145,16 @@ describe("db e2e", () => { expect(result.exitCode, result.stderr).toBe(0); const parsed = parseJson(result.stdout, listEnvelopeSchema(Database)); - expect(parsed.data.length).toBe(1); const warehouse = parsed.data[0]; - expect(warehouse?.id).toBe(SEEDED.warehouseDbId); - const tableIds = (warehouse?.tables ?? []).map((table) => table.id).toSorted(); - const expectedIds = [ - ...PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME, - ...ANALYTICS_TABLES_SORTED_BY_DISPLAY_NAME, - ] - .map((table) => table.id) - .toSorted(); - expect(tableIds).toEqual(expectedIds); + expect({ + count: parsed.data.length, + dbId: warehouse?.id, + tableIds: (warehouse?.tables ?? []).map((table) => table.id).toSorted(), + }).toEqual({ + count: 1, + dbId: SEEDED.warehouseDbId, + tableIds: ALL_WAREHOUSE_TABLES_SORTED_BY_ID.map((table) => table.id), + }); }); it("list --saved includes the Saved Questions virtual database", async () => { @@ -202,92 +207,106 @@ describe("db e2e", () => { }); }); - it("get --include tables.fields hydrates tables and their fields", async () => { + it("get --include tables returns the compact table map under the default cap", async () => { const result = await runCli({ - args: [ - "db", - "get", - String(SEEDED.warehouseDbId), - "--include", - "tables.fields", - "--full", - "--json", - "--max-bytes", - "0", - ], + args: ["db", "get", String(SEEDED.warehouseDbId), "--include", "tables", "--json"], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); expect(result.exitCode, result.stderr).toBe(0); - const parsed = parseJson(result.stdout, Database); - expect(parsed.id).toBe(SEEDED.warehouseDbId); - const customers = (parsed.tables ?? []).find((table) => table.id === SEEDED.tables.customers); - expect(customers).toBeDefined(); - expect(Array.isArray(customers?.fields)).toBe(true); - expect((customers?.fields ?? []).length).toBeGreaterThan(0); + const parsed = parseJson(result.stdout, DatabaseCompact); + expect({ ...parsed, tables: (parsed.tables ?? []).toSorted((a, b) => a.id - b.id) }).toEqual({ + id: SEEDED.warehouseDbId, + name: "Warehouse", + engine: "postgres", + tables: ALL_WAREHOUSE_TABLES_SORTED_BY_ID, + }); }); - it("get with a non-integer id fails fast with ConfigError", async () => { + it("get --include tables.fields returns compact tables with compact fields under the default cap", async () => { const result = await runCli({ - args: ["db", "get", "abc", "--json"], + args: ["db", "get", String(SEEDED.warehouseDbId), "--include", "tables.fields", "--json"], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); - expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toContain('invalid id: "abc" (expected integer)'); - expect(result.stdout).toBe(""); + expect(result.exitCode, result.stderr).toBe(0); + const parsed = parseJson(result.stdout, DatabaseCompact); + expect({ + tableIds: (parsed.tables ?? []).map((table) => table.id).toSorted(), + everyTableHasFields: (parsed.tables ?? []).every((table) => (table.fields ?? []).length > 0), + }).toEqual({ + tableIds: ALL_WAREHOUSE_TABLES_SORTED_BY_ID.map((table) => table.id), + everyTableHasFields: true, + }); }); - it("get against a missing database id surfaces a resource-missing 404 with the exact path", async () => { + it("get --include tables.fields --full over the cap exits 2 and points down the hydration ladder", async () => { const result = await runCli({ - args: ["db", "get", "9999999", "--json"], + args: [ + "db", + "get", + String(SEEDED.warehouseDbId), + "--include", + "tables.fields", + "--full", + "--json", + ], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); - expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Not found: GET /api/database/9999999."); + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain( + `over the ${DEFAULT_MAX_BYTES}-byte --max-bytes cap; ${fullRollupOversizeHint(SEEDED.warehouseDbId)}`, + ); }); - it("metadata returns the warehouse with its tables hydrated", async () => { + it("get --include tables over a tiny cap exits 2 and points at schema traversal and search", async () => { + const tinyCap = 512; const result = await runCli({ args: [ "db", - "metadata", + "get", String(SEEDED.warehouseDbId), - "--full", + "--include", + "tables", "--json", "--max-bytes", - "0", + String(tinyCap), ], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); - expect(result.exitCode, result.stderr).toBe(0); - const parsed = parseJson(result.stdout, Database); - expect(parsed.id).toBe(SEEDED.warehouseDbId); - const tableIds = (parsed.tables ?? []).map((table) => table.id).toSorted(); - const expectedIds = [ - ...PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME, - ...ANALYTICS_TABLES_SORTED_BY_DISPLAY_NAME, - ] - .map((table) => table.id) - .toSorted(); - expect(tableIds).toEqual(expectedIds); + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain( + `over the ${tinyCap}-byte --max-bytes cap; ${tableMapOversizeHint(SEEDED.warehouseDbId)}`, + ); }); - it("metadata against a missing database id surfaces a 404 HttpError", async () => { + it("get with a non-integer id fails fast with ConfigError", async () => { const result = await runCli({ - args: ["db", "metadata", "9999999", "--json"], + args: ["db", "get", "abc", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain('invalid id: "abc" (expected integer)'); + expect(result.stdout).toBe(""); + }); + + it("get against a missing database id surfaces a resource-missing 404 with the exact path", async () => { + const result = await runCli({ + args: ["db", "get", "9999999", "--json"], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Not found: GET /api/database/9999999/metadata."); + expect(result.stderr).toContain("Not found: GET /api/database/9999999."); }); it("schemas lists the seeded warehouse schemas alphabetically", async () => { diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts index 837f5e2..d860df3 100644 --- a/tests/e2e/skills.e2e.test.ts +++ b/tests/e2e/skills.e2e.test.ts @@ -107,7 +107,7 @@ describe("skills e2e", () => { it("get accepts comma-separated names", async () => { const result = await runCli({ - args: ["skills", "get", "git-sync,transform", "--json"], + args: ["skills", "get", "git-sync,transform", "--json", "--max-bytes", "0"], configHome: await makeIsolatedConfigHome(), }); diff --git a/tests/e2e/table.e2e.test.ts b/tests/e2e/table.e2e.test.ts index e2b1d46..827d57c 100644 --- a/tests/e2e/table.e2e.test.ts +++ b/tests/e2e/table.e2e.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { FieldListEnvelope } from "../../src/commands/table/fields"; +import { tableFieldsOversizeHint } from "../../src/commands/table/hints"; import { TableListEnvelope } from "../../src/commands/table/list"; import { Table, TableCompact } from "../../src/domain/table"; import { parseJson } from "../../src/runtime/json"; @@ -189,6 +190,29 @@ describe("table e2e", () => { }); }); + it("get --include fields over a tiny cap exits 2 and points at table fields", async () => { + const tinyCap = 256; + const result = await runCli({ + args: [ + "table", + "get", + String(SEEDED.tables.customers), + "--include", + "fields", + "--json", + "--max-bytes", + String(tinyCap), + ], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toContain( + `over the ${tinyCap}-byte --max-bytes cap; ${tableFieldsOversizeHint(SEEDED.tables.customers)}`, + ); + }); + it("get rejects an unknown --include value with ConfigError", async () => { const result = await runCli({ args: ["table", "get", String(SEEDED.tables.customers), "--include", "everything", "--json"], @@ -227,43 +251,6 @@ describe("table e2e", () => { expect(result.stderr).toContain("Not found: GET /api/table/9999999."); }); - it("metadata returns the table with hydrated fields", async () => { - const result = await runCli({ - args: [ - "table", - "metadata", - String(SEEDED.tables.customers), - "--json", - "--full", - "--max-bytes", - "0", - ], - configHome: await makeIsolatedConfigHome(), - env: authEnv(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - const parsed = parseJson(result.stdout, TableCompact); - const { fields, ...tableBody } = parsed; - const fieldNames = (fields ?? []).map((field) => field.name).toSorted(); - - expect({ tableBody, fieldNames }).toEqual({ - tableBody: CUSTOMERS_COMPACT, - fieldNames: CUSTOMERS_FIELD_NAMES, - }); - }); - - it("metadata against a missing table id surfaces a 404 HttpError", async () => { - const result = await runCli({ - args: ["table", "metadata", "9999999", "--json"], - configHome: await makeIsolatedConfigHome(), - env: authEnv(), - }); - - expect(result.exitCode).toBe(1); - expect(result.stderr).toContain("Not found: GET /api/table/9999999/query_metadata."); - }); - it("fields lists every field on the table in compact form", async () => { const result = await runCli({ args: ["table", "fields", String(SEEDED.tables.customers), "--json", "--max-bytes", "0"],