From 6a066a1a0dff66a0223d2a8a98548005fe7b5457 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 11:09:23 -0400 Subject: [PATCH 1/7] Fix CLI issues found while trialling the getting-started guide - Scope keychain secrets by config directory, so same-named profiles in two config homes no longer overwrite each other's key; keys stored under the bare service stay readable and are copied into the scope. - `mb auth login --api-key -` reads the key from piped stdin; other commands refuse `--api-key -` instead of sending it as a key. - `mb git-sync export --branch ` refuses a branch missing from the remote before the export, naming `mb git-sync create-branch `. - Add `mb db set-data-actions ` for the database's `database-enable-actions` setting. - Show `dashboard_id` in the compact card view. - data-action skill: the toggle is Data actions and `mb` can set it, model-less actions go in a data actions collection, and template-tag ids are UUIDs from `mb uuid`. Co-Authored-By: Claude Opus 5.5 --- README.md | 20 ++++-- packages/cli/skill-data/data-action/SKILL.md | 25 +++++-- packages/cli/src/commands/auth/list.test.ts | 4 +- packages/cli/src/commands/auth/login.ts | 23 ++++++- .../cli/src/commands/data-action/create.ts | 2 +- packages/cli/src/commands/db/index.ts | 3 +- .../cli/src/commands/db/set-data-actions.ts | 51 +++++++++++++++ packages/cli/src/commands/git-sync/export.ts | 18 ++++- packages/cli/src/core/auth/storage.test.ts | 65 +++++++++++++++---- packages/cli/src/core/auth/storage.ts | 59 ++++++++++++----- packages/cli/src/core/config.test.ts | 15 ++++- packages/cli/src/core/config.ts | 6 ++ packages/cli/src/output/views/database.ts | 11 ++++ packages/cli/src/runtime/command-help.test.ts | 1 + packages/cli/src/runtime/input.ts | 4 +- packages/client/src/domain/card.ts | 1 + packages/client/src/domain/database.ts | 29 +++++++++ packages/client/src/resources/database.ts | 17 ++++- packages/client/src/version/requirements.ts | 1 + tests/e2e/auth.e2e.test.ts | 36 ++++++++++ tests/e2e/card.e2e.test.ts | 17 +++++ tests/e2e/dashboard.e2e.test.ts | 1 + tests/e2e/db.e2e.test.ts | 55 +++++++++++++++- 23 files changed, 410 insertions(+), 54 deletions(-) create mode 100644 packages/cli/src/commands/db/set-data-actions.ts diff --git a/README.md b/README.md index 6bcd26ff..f5d75ca5 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ On success the server is probed once — the rendered output shows the user, rol | Flag | Description | | ------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | | `--url ` | Metabase URL, including any subpath if the instance is hosted under one (`https://my.org.com/metabase`). Falls back to `MB_URL`, then prompts. | -| `--api-key ` | API key. Skips the browser flow. Visible in shell history — pipe on stdin instead. | +| `--api-key ` | API key. Skips the browser flow. Visible in shell history — pipe on stdin instead; `--api-key -` reads the key from piped stdin. | | `--client-id ` | Pre-registered OAuth client id (only needed when dynamic client registration is disabled on the server). | | `--profile `, `-p` | Profile to write to (default: `default`). | | `--skip-verify` | Save without contacting the server (no probe, no cache). | @@ -122,6 +122,7 @@ Non-interactive (non-TTY) login requires an API key; resolution order: `--api-ke mb auth login # interactive: browser or API key echo "$MB_KEY" | mb auth login --url https://m.example.com mb auth login --url https://m.example.com < key.txt +mb auth login --url https://m.example.com --api-key - < key.txt ``` ### `mb auth status` @@ -141,7 +142,7 @@ mb auth status --profile staging ### `mb auth list` -List configured authentication profiles. All profile metadata (URL, auth method, last successful probe, last failure) lives in `/profiles.json` at mode `0600`; the secrets (API key, or OAuth access/refresh tokens) sit in the OS keychain when available, or inline in the same file when the keychain is unavailable. +List configured authentication profiles. All profile metadata (URL, auth method, last successful probe, last failure) lives in `/profiles.json` at mode `0600`; the secrets (API key, or OAuth access/refresh tokens) sit in the OS keychain when available (service `metabase-cli:`, so each config directory keeps its own), or inline in the same file when the keychain is unavailable. `auth list` re-probes every profile, one at a time — a probe can refresh and rewrite an expired OAuth token, so probes are serialized to avoid racing on the shared `profiles.json`. On success it refreshes `lastProbe` (Metabase version, token features, user identity) and clears `lastFailure`; on failure it updates `lastFailure` and leaves the prior `lastProbe`/`url`/credential untouched. Rendered columns: `Profile | URL | Auth | Status | Role | Version | Skew | Last probed`; `--json` rows carry the same derived `edition`, `skew`, `knownRange` and `features` as `auth status`. Failed rows append a one-line footer pointing at `mb auth login --profile `. @@ -539,6 +540,15 @@ mb db rescan-values 1 mb db rescan-values 1 --json ``` +### `mb db set-data-actions ` + +Turn data actions on or off for a database (`PUT /api/database/:id` with `settings: {"database-enable-actions": …}`), the Data actions toggle of Admin → Databases. While it is off, data actions on the database can be neither created nor run. Needs an admin, and a driver that lists `actions` in its `features`. Returns the database with its `settings`; a state other than `on` or `off` is refused before the request (exit 2). + +```sh +mb db set-data-actions 2 on +mb db set-data-actions 2 off --json +``` + ## Tables Inspect and edit warehouse tables via `/api/table`. For agent-driven field introspection, `table get --include fields` is the default — it returns the table plus its columns in a single bounded response. @@ -1139,7 +1149,7 @@ mb snippet archive 1 --json ## Data actions -CRUD on `/api/action` plus `execute`. A data action is a parameterized native SQL write (`INSERT`, `UPDATE`, `DELETE`) filed in a collection. Data actions are off by default: an admin must enable them on the target database first. `mb data-action create` authors data actions only — no `model_id`, no implicit data actions — and needs a server whose data actions do not require a model. +CRUD on `/api/action` plus `execute`. A data action is a parameterized native SQL write (`INSERT`, `UPDATE`, `DELETE`) filed in a collection. Data actions are off by default: an admin must enable them on the target database first (`mb db set-data-actions on`). `mb data-action create` authors data actions only — no `model_id`, no implicit data actions — and needs a server whose data actions do not require a model. ### `mb data-action list` @@ -1169,7 +1179,7 @@ mb data-action create --file data-action.json --skip-validate | `--file ` | Path to JSON body file. | | `--skip-validate` | Skip the local MBQL 5 pre-flight validation; let the server be the authority. Use only when the bundled schema disagrees with what the server accepts. | -Body fields: `name` (required), `type` (required, `"query"`), `database_id` (required), `dataset_query` (required native query whose `{{tag}}` placeholders are its inputs), `parameters` (one per template tag), `collection_id` (optional; the root when omitted), `description`, `visualization_settings`. +Body fields: `name` (required), `type` (required, `"query"`), `database_id` (required), `dataset_query` (required native query whose `{{tag}}` placeholders are its inputs), `parameters` (one per template tag), `collection_id` (optional; a collection in the `data-actions` or `data-apps` namespace, such as one made by `mb collection create --namespace data-actions`, or the data actions root when omitted — a regular collection is refused), `description`, `visualization_settings`. ### `mb data-action update ` @@ -2180,7 +2190,7 @@ The import also asserts which branch git-sync tracks, read from the session prop ### `mb git-sync export` -Export Metabase changes back to the configured git remote (Metabase → repo). Auto-polls by default. The export targets the branch git-sync tracks; to push to a new branch, `stash` or `create-branch` first. When the remote has moved past the last sync, a plain export ends in a `conflict` task on Metabase v63 and newer, and is refused with a 400 on older servers. Such a task names the divergence in its text output and error, and may leave the remote's commit counted as synced, as an import conflict may: never answer it with a retry. +Export Metabase changes back to the configured git remote (Metabase → repo). Auto-polls by default. The export targets the branch git-sync tracks; to push to a new branch, `stash` or `create-branch` first. A `--branch` other than the tracked one is looked up in `git-sync branches` first, and one missing from the remote is refused with exit 2, naming `mb git-sync create-branch `. When the remote has moved past the last sync, a plain export ends in a `conflict` task on Metabase v63 and newer, and is refused with a 400 on older servers. Such a task names the divergence in its text output and error, and may leave the remote's commit counted as synced, as an import conflict may: never answer it with a retry. ```sh mb git-sync export -m "update dashboards" diff --git a/packages/cli/skill-data/data-action/SKILL.md b/packages/cli/skill-data/data-action/SKILL.md index 54f18fb2..3b3c45ec 100644 --- a/packages/cli/skill-data/data-action/SKILL.md +++ b/packages/cli/skill-data/data-action/SKILL.md @@ -1,6 +1,6 @@ --- name: data-action -description: Author and run Metabase data actions with the `mb` CLI — saved, parameterized native SQL writes (INSERT, UPDATE, DELETE) filed in a collection and run with values. Covers enabling them on a database, the body, mapping template tags to parameters, running, and the lifecycle. Triggers — "add a data action", "let users update a row", "a button that inserts a record", "run a write query", "a form that saves to the database", "delete rows from Metabase". +description: Author and run Metabase data actions with the `mb` CLI — saved, parameterized native SQL writes (INSERT, UPDATE, DELETE) filed in a collection and run with values. Covers enabling them on a database, the collection they go in, the body, mapping template tags to parameters, running, and the lifecycle. Triggers — "add a data action", "let users update a row", "a button that inserts a record", "run a write query", "a form that saves to the database", "delete rows from Metabase". allowed-tools: Read, Write, Edit, Bash, AskUserQuestion requires: [dataActionsWithoutModel] --- @@ -13,17 +13,27 @@ Flag conventions and `./.scratch` are in `core`; the native query and its templa ## Check the database first -Data actions are off on every database until an admin turns on **Model actions** for it (Admin → Databases → the database). `mb` cannot change that setting. +Data actions are off on every database until an admin turns on **Data actions** for it ("Allow data actions that use this database to be run", Admin → Databases → the database). That toggle is the database setting `database-enable-actions`. ```bash mb db get --full --json ``` - `features` must contain `"actions"` — otherwise the driver cannot run writes; pick another database. -- `settings["database-enable-actions"]` must be `true`. If it is not, ask the user to have an admin enable it. Create and execute fail with `Actions are not enabled.` until then. +- `settings["database-enable-actions"]` must be `true`. Create and execute fail with `Actions are not enabled.` until then. An admin profile turns it on with `mb db set-data-actions on`; anyone else asks an admin to. Turning it on lets anyone with access to the database's actions write to it, so confirm with the user first. + +## Pick the collection + +A data action goes in a data actions collection, a data app collection, or the data actions root (omit `collection_id`). A regular collection is refused with `An action without a model can only go in a data actions or data app collection.` Make one with: + +```bash +mb collection create --namespace data-actions --body '{"name":"Order actions"}' --json +``` ## The body +Mint one template-tag `id` per tag with `mb uuid --count --format text`. + ```json { "name": "Set order note", @@ -39,14 +49,14 @@ mb db get --full --json "native": "UPDATE orders SET note = {{note}} WHERE id = {{order_id}}", "template-tags": { "order_id": { - "id": "order_id", + "id": "6f1c9a4e-2b7d-4e0a-9c3f-1d2e3f4a5b6c", "name": "order_id", "display-name": "Order ID", "type": "number", "required": true }, "note": { - "id": "note", + "id": "0b8d7c6e-5f4a-4b3c-8d2e-1f0a9b8c7d6e", "name": "note", "display-name": "Note", "type": "text", @@ -78,7 +88,8 @@ mb db get --full --json ``` - `type` is always `"query"`. Never set `model_id` or `type: "implicit"`; `mb data-action create` refuses both. -- `database_id` and `dataset_query.database` are the same id (`mb db list`). Omit `collection_id` for the root collection (`mb collection list`). +- `database_id` and `dataset_query.database` are the same id (`mb db list`). `collection_id` is the data actions collection (here `12`); omit it for the data actions root. +- A template tag's `id` is a UUID, unique among the tags: the representation schema requires one. - One statement, no `;`. `create` and `update` validate `dataset_query` as `native-sql` does. ## Parameters: one per template tag @@ -92,7 +103,7 @@ Each `{{name}}` needs a `template-tags` entry and a `parameters` entry, all usin | `date` | `date/single` | | `boolean` | `boolean/=` | -- Parameter `id` and `slug` are the tag name; `target` is `["variable", ["template-tag", ""]]`. +- Parameter `id` and `slug` are the tag name (not the tag's UUID `id`); `target` is `["variable", ["template-tag", ""]]`. - Use raw variables only. A field filter (`type: "dimension"`), snippet or card reference does not work in a write — write the comparison yourself (`WHERE id = {{order_id}}`). - Set `required: true` on every input the statement cannot run without (`WHERE` keys, `NOT NULL` columns). - Wrap an optional input in `[[ … ]]` with `required: false`; the clause drops out when no value is given. This makes one action do partial updates: `UPDATE orders SET updated_at = now() [[, note = {{note}}]] [[, status = {{status}}]] WHERE id = {{order_id}}`. diff --git a/packages/cli/src/commands/auth/list.test.ts b/packages/cli/src/commands/auth/list.test.ts index f4a650b2..a734fbd5 100644 --- a/packages/cli/src/commands/auth/list.test.ts +++ b/packages/cli/src/commands/auth/list.test.ts @@ -37,7 +37,7 @@ vi.mock("../../core/auth/verify", () => ({ })); import authListCommand, { AuthProfileListEnvelope } from "./list"; -import { writeProfile, readProfileRecord } from "../../core/auth/storage"; +import { keyringService, readProfileRecord, writeProfile } from "../../core/auth/storage"; import { setupTempConfigHome, type TempConfigHome } from "../../core/auth/temp-config-home"; interface CapturedStdout { @@ -313,6 +313,6 @@ describe("auth list command", () => { reason: "Could not reach Metabase: getaddrinfo ENOTFOUND", }, }); - expect(hoisted.store.get("metabase-cli:profile:stable:apiKey")).toBe("good"); + expect(hoisted.store.get(`${keyringService()}:profile:stable:apiKey`)).toBe("good"); }); }); diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index e09b83d8..5b7a0a64 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -30,7 +30,7 @@ import { EMPTY_CELL } from "../../output/table"; import type { ResourceView } from "../../output/view"; import { interruptSignal } from "../../runtime/interrupt"; import { openBrowser } from "../../runtime/process"; -import { readInput } from "../../runtime/input"; +import { readInput, STDIN_ARG } from "../../runtime/input"; import type { CommonContext } from "../context"; import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { defineMetabaseCommand } from "../runtime"; @@ -71,7 +71,7 @@ const loginView: ResourceView = { export default defineMetabaseCommand({ meta: { name: "login", description: "Log in to a Metabase instance for a profile" }, details: - "Interactive login offers browser OAuth (recommended; Metabase v63+) or an API key — older servers fall back to the API key prompt automatically. Browser login opens Metabase, you sign in (password or SSO) and approve, and the CLI stores a refreshing access token. For CI/non-interactive use, supply an API key via --api-key, piped stdin, or $MB_API_KEY (first non-empty wins); any of these skips the browser flow, even on a TTY. The URL comes from --url or $MB_URL, prompted when stdin is a TTY.", + "Interactive login offers browser OAuth (recommended; Metabase v63+) or an API key — older servers fall back to the API key prompt automatically. Browser login opens Metabase, you sign in (password or SSO) and approve, and the CLI stores a refreshing access token. For CI/non-interactive use, supply an API key via --api-key (`--api-key -` reads it from stdin), piped stdin, or $MB_API_KEY (first non-empty wins); any of these skips the browser flow, even on a TTY. The URL comes from --url or $MB_URL, prompted when stdin is a TTY.", requires: ["user.current"], args: { ...outputFlags, @@ -92,13 +92,14 @@ export default defineMetabaseCommand({ examples: [ "mb auth login --url https://metabase.example.com", "echo $MB_API_KEY | mb auth login --url https://metabase.example.com", + "pass show metabase | mb auth login --url https://metabase.example.com --api-key -", "mb auth login --profile staging --url https://staging.example.com", ], async run({ args, ctx }) { const profileName = await resolveLoginProfile(args.profile); const env = readEnvCredentials(); - if (args.apiKey) { + if (args.apiKey && args.apiKey !== STDIN_ARG) { warn( "warning: --api-key is visible in shell history and process listings — pipe the key on stdin or set MB_API_KEY instead", ); @@ -354,6 +355,9 @@ async function nonInteractiveApiKey( flagKey: string | undefined, envKey: string | null, ): Promise { + if (flagKey === STDIN_ARG) { + return await apiKeyFromStdin(); + } if (flagKey) { return flagKey; } @@ -372,6 +376,19 @@ async function nonInteractiveApiKey( return null; } +async function apiKeyFromStdin(): Promise { + if (process.stdin.isTTY) { + throw new ConfigError( + "--api-key - reads the key from piped stdin, but stdin is a terminal; pipe the key in or omit --api-key to be prompted", + ); + } + const piped = (await readInput({ file: STDIN_ARG })).trim(); + if (piped === "") { + throw new ConfigError("--api-key - read nothing from stdin; pipe the API key in"); + } + return piped; +} + async function promptForUrl(): Promise { if (!process.stdin.isTTY) { throw new ConfigError( diff --git a/packages/cli/src/commands/data-action/create.ts b/packages/cli/src/commands/data-action/create.ts index f4993266..48ed039e 100644 --- a/packages/cli/src/commands/data-action/create.ts +++ b/packages/cli/src/commands/data-action/create.ts @@ -14,7 +14,7 @@ import { export default defineMetabaseCommand({ meta: { name: "create", description: "Create a data action from JSON" }, details: - 'A data action is a parameterized native query that writes to a database, filed in a collection. The JSON body needs `name`, `type: "query"`, `database_id`, a native `dataset_query` whose `{{tag}}` placeholders are its inputs, and `parameters` targeting those tags; `collection_id` picks the collection (omit for the root). Data actions must be enabled on the database. An MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.', + 'A data action is a parameterized native query that writes to a database, filed in a collection. The JSON body needs `name`, `type: "query"`, `database_id`, a native `dataset_query` whose `{{tag}}` placeholders are its inputs, and `parameters` targeting those tags; `collection_id` picks a collection in the `data-actions` or `data-apps` namespace (omit for the data actions root; a regular collection is refused). Data actions must be enabled on the database (`mb db set-data-actions on`). An MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.', skills: [ { skill: "data-action", purpose: "author a data action and its parameters" }, { skill: "native-sql", purpose: "the native query and its template tags" }, diff --git a/packages/cli/src/commands/db/index.ts b/packages/cli/src/commands/db/index.ts index deeae657..c7d23dd1 100644 --- a/packages/cli/src/commands/db/index.ts +++ b/packages/cli/src/commands/db/index.ts @@ -2,7 +2,7 @@ import { defineCommandGroup } from "../group"; export default defineCommandGroup({ name: "db", - description: "Inspect and sync Metabase databases", + description: "Inspect and sync Metabase databases, and turn their data actions on or off", alias: "database", skills: [ { skill: "core", purpose: "the database traversal ladder and schema sync" }, @@ -15,5 +15,6 @@ export default defineCommandGroup({ "schema-tables": () => import("./schema-tables").then((m) => m.default), "sync-schema": () => import("./sync-schema").then((m) => m.default), "rescan-values": () => import("./rescan-values").then((m) => m.default), + "set-data-actions": () => import("./set-data-actions").then((m) => m.default), }, }); diff --git a/packages/cli/src/commands/db/set-data-actions.ts b/packages/cli/src/commands/db/set-data-actions.ts new file mode 100644 index 00000000..6452159b --- /dev/null +++ b/packages/cli/src/commands/db/set-data-actions.ts @@ -0,0 +1,51 @@ +import { z } from "zod"; + +import { DatabaseWithSettings } from "@metabase/client/domain/database"; + +import { renderSummary } from "../../output/render"; +import { databaseDataActionsView } from "../../output/views/database"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseEnumFlag } from "../parse-enum"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +const DataActionsState = z.enum(["on", "off"]); + +export default defineMetabaseCommand({ + meta: { + name: "set-data-actions", + description: "Turn data actions on or off for a database", + }, + details: + "The Data actions toggle of Admin → Databases (the `database-enable-actions` database setting): while it is off, data actions on the database can be neither created nor run. Needs an admin. The driver must support writes: `mb db get --full --json` lists `actions` in `features`. Answers the database with its `settings`.", + skills: [{ skill: "data-action", purpose: "author and run data actions on the database" }], + requires: ["database.update"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Database id", required: true }, + state: { + type: "positional", + description: `Data actions state (${DataActionsState.options.join(" | ")})`, + required: true, + }, + }, + outputSchema: DatabaseWithSettings, + examples: ["mb db set-data-actions 2 on", "mb db set-data-actions 2 off --json"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const enabled = parseEnumFlag(args.state, DataActionsState, "state") === "on"; + const client = await getClient(); + const database = await client.database.update(id, { + settings: { "database-enable-actions": enabled }, + }); + const state = enabled ? "on" : "off"; + renderSummary( + database, + databaseDataActionsView, + `Turned data actions ${state} for database ${database.id} "${database.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/git-sync/export.ts b/packages/cli/src/commands/git-sync/export.ts index dafac1bf..76755070 100644 --- a/packages/cli/src/commands/git-sync/export.ts +++ b/packages/cli/src/commands/git-sync/export.ts @@ -19,8 +19,8 @@ export default defineMetabaseCommand({ description: "Export Metabase changes back to the configured git remote", }, details: - "The export targets the branch git-sync tracks. On Metabase 63+ `--branch` defaults to it and must name it (the server answers 409 for any other branch); older servers export to the branch `--branch` names and switch git-sync to it, refusing with 400 when that branch's tip is not the last synced commit unless --force is given. To push to a new branch, use `git-sync stash` or `git-sync create-branch` first. When the remote has moved past the last sync, a plain export ends in a `conflict` task on 63+ and is refused with 400 on older servers. --merge (63+) folds the remote's changes in by a three-way merge (entities changed on both sides still end in `conflict`), --force overwrites them. `git-sync export-preflight` previews which applies. A task that ends in `conflict` may make the server count the remote commit it saw as synced, so a retry, --merge included, may no longer see the remote's changes. Never answer a conflict with a retry: keep the remote's side with `import --force`, Metabase's with `export --force`, or both through a reviewed PR from a new branch (`mb skills get git-sync`, \"After a conflict task\").", - requires: ["gitSync.export"], + "The export targets the branch git-sync tracks. On Metabase 63+ `--branch` defaults to it and must name it (the server answers 409 for any other branch); older servers export to the branch `--branch` names and switch git-sync to it, refusing with 400 when that branch's tip is not the last synced commit unless --force is given. To push to a new branch, use `git-sync stash` or `git-sync create-branch` first; a `--branch` missing from the remote is refused before the export with a pointer to `create-branch`. When the remote has moved past the last sync, a plain export ends in a `conflict` task on 63+ and is refused with 400 on older servers. --merge (63+) folds the remote's changes in by a three-way merge (entities changed on both sides still end in `conflict`), --force overwrites them. `git-sync export-preflight` previews which applies. A task that ends in `conflict` may make the server count the remote commit it saw as synced, so a retry, --merge included, may no longer see the remote's changes. Never answer a conflict with a retry: keep the remote's side with `import --force`, Metabase's with `export --force`, or both through a reviewed PR from a new branch (`mb skills get git-sync`, \"After a conflict task\").", + requires: ["gitSync.branch", "gitSync.branches", "gitSync.export"], args: { ...outputFlags, ...profileFlag, @@ -80,6 +80,14 @@ export default defineMetabaseCommand({ } const mb = await getClient(); + // A guarded server answers an export to a branch missing from the remote with a 409 about the + // tracked branch having moved, which points away from the fix. + if (branch !== null && (await mb.gitSync.branch()) !== branch) { + const remote = await mb.gitSync.branches(); + if (!remote.data.includes(branch)) { + throw missingBranchError(branch); + } + } const result = await mb.gitSync.export(params); if (!wait.enabled) { @@ -95,6 +103,12 @@ export default defineMetabaseCommand({ }, }); +function missingBranchError(branch: string): ConfigError { + return new ConfigError( + `branch "${branch}" does not exist on the remote; run \`mb git-sync create-branch ${branch}\` to create it and switch git-sync to it, then export without --branch`, + ); +} + function emitRealignHint(ctx: CommonContext): void { if (ctx.format !== "text") { return; diff --git a/packages/cli/src/core/auth/storage.test.ts b/packages/cli/src/core/auth/storage.test.ts index 9f3f7c94..5f82b269 100644 --- a/packages/cli/src/core/auth/storage.test.ts +++ b/packages/cli/src/core/auth/storage.test.ts @@ -22,6 +22,7 @@ import * as storage from "./storage"; const { clearProfile, + keyringService, consumeKeychainResidualWarning, consumeKeyringDowngradeWarning, consumeLegacyStorageWarning, @@ -92,10 +93,10 @@ describe("profiles (keyring backend)", () => { const location = await writeProfile({ url: "https://m.example.com", apiKey: "secret" }); expect(location).toEqual({ backend: "keyring", - service: "metabase-cli", + service: keyringService(), account: "profile:default:apiKey", }); - expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBe("secret"); + expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBe("secret"); const file = parseJson(readFileSync(profilesFilePath(), "utf8"), ProfilesFile); expect(file).toEqual({ @@ -134,6 +135,44 @@ describe("profiles (keyring backend)", () => { }); }); + it("keeps same-named profiles in different config directories apart", async () => { + await writeProfile({ url: "https://first.example.com", apiKey: "first-key" }); + const second = setupTempConfigHome(); + try { + await writeProfile({ url: "https://second.example.com", apiKey: "second-key" }); + expect(await readProfileCredential()).toEqual({ + url: "https://second.example.com", + credential: { kind: "apiKey", apiKey: "second-key" }, + }); + } finally { + second.cleanup(); + } + expect(await readProfileCredential()).toEqual({ + url: "https://first.example.com", + credential: { kind: "apiKey", apiKey: "first-key" }, + }); + }); + + it("reads a key stored under the unscoped service and copies it into this directory's scope", async () => { + await writeProfile({ url: "https://m.example.com", apiKey: "old-key" }); + hoisted.store.delete(`${keyringService()}:profile:default:apiKey`); + hoisted.store.set("metabase-cli:profile:default:apiKey", "old-key"); + + expect(await readProfileCredential()).toEqual({ + url: "https://m.example.com", + credential: { kind: "apiKey", apiKey: "old-key" }, + }); + expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBe("old-key"); + }); + + it("clearProfile also removes the key stored under the unscoped service", async () => { + await writeProfile({ url: "https://m.example.com", apiKey: "k" }); + hoisted.store.set("metabase-cli:profile:default:apiKey", "old-key"); + + expect(await clearProfile()).toBe(true); + expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBeUndefined(); + }); + it("preserves user add order (no sort) and overwrites existing entries in place", async () => { await writeProfile({ url: "https://1.example.com", apiKey: "k1" }, "zeta"); await writeProfile({ url: "https://2.example.com", apiKey: "k2" }, "alpha"); @@ -152,7 +191,7 @@ describe("profiles (keyring backend)", () => { expect(await clearProfile("a")).toBe(true); expect(await readProfileCredential("a")).toBeNull(); - expect(hoisted.store.get("metabase-cli:profile:a:apiKey")).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:a:apiKey`)).toBeUndefined(); expect(await readProfileCredential("b")).toEqual({ url: "https://b.example.com", credential: { kind: "apiKey", apiKey: "b" }, @@ -347,7 +386,7 @@ describe("writeProbeResult and writeProbeFailure", () => { lastProbe: ALICE_PROBE, lastFailure: { at: FAILED_AT, kind: "auth", reason: "Invalid or unauthorized API key" }, }); - expect(hoisted.store.get("metabase-cli:profile:p:apiKey")).toBe("k"); + expect(hoisted.store.get(`${keyringService()}:profile:p:apiKey`)).toBe("k"); }); }); @@ -383,7 +422,7 @@ describe("MB_CLI_DISABLE_KEYRING", () => { const location = await writeProfile({ url: "https://m.example.com", apiKey: "secret" }); expect(location).toEqual({ backend: "keyring", - service: "metabase-cli", + service: keyringService(), account: "profile:default:apiKey", }); }); @@ -494,8 +533,8 @@ describe("OAuth profiles (keyring backend)", () => { lastProbe: null, lastFailure: null, }); - expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBe("access-1"); - expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBe("refresh-1"); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBe("access-1"); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBe("refresh-1"); }); it("switching an OAuth profile to an API key clears the OAuth tokens", async () => { @@ -506,8 +545,8 @@ describe("OAuth profiles (keyring backend)", () => { credential: { kind: "apiKey", apiKey: "k" }, }); expect(await readProfileRecord()).toEqual(defaultRecord(null)); - expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBeUndefined(); - expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBeUndefined(); }); it("switching an API key profile to OAuth clears the API key", async () => { @@ -530,14 +569,14 @@ describe("OAuth profiles (keyring backend)", () => { lastProbe: null, lastFailure: null, }); - expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBeUndefined(); }); it("clearProfile removes the OAuth tokens from the keyring", async () => { await writeOAuthProfile("https://m.example.com", OAUTH); expect(await clearProfile()).toBe(true); - expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBeUndefined(); - expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBeUndefined(); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBeUndefined(); expect(await readProfileCredential()).toBeNull(); }); @@ -561,7 +600,7 @@ describe("OAuth profiles (keyring backend)", () => { } // The recovered keyring still holds the pre-rotation tokens; the inline file copy is // authoritative, so the stale keyring entry must not shadow it. - expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBe("refresh-1"); + expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBe("refresh-1"); expect(await readProfileCredential()).toEqual({ url: "https://m.example.com", credential: rotated, diff --git a/packages/cli/src/core/auth/storage.ts b/packages/cli/src/core/auth/storage.ts index 4682a3ce..5b080f01 100644 --- a/packages/cli/src/core/auth/storage.ts +++ b/packages/cli/src/core/auth/storage.ts @@ -19,7 +19,7 @@ import { type ProfileRecord, } from "./profile-record"; -const KEYRING_SERVICE = "metabase-cli"; +const LEGACY_KEYRING_SERVICE = "metabase-cli"; const PROFILES_FILE = "profiles.json"; const LEGACY_CREDENTIALS_FILE = "credentials.json"; const LEGACY_REJECTIONS_FILE = "rejections.json"; @@ -118,6 +118,11 @@ export function consumeKeyringDowngradeWarning(): string | null { return message; } +// Scoped by config directory so two config homes holding a same-named profile keep separate secrets. +export function keyringService(): string { + return `${LEGACY_KEYRING_SERVICE}:${configDir()}`; +} + function keyringEnabled(): boolean { return readEnv(ENV_DISABLE_KEYRING) !== "1"; } @@ -128,24 +133,24 @@ function keyringEnabled(): boolean { // tell a recoverable backend issue from a real bug, and the design choice is // graceful degradation to the file backend either way — so every throw routes // to the fallback, deliberately. -function trySetKeyring(key: CredentialAccount, value: string): boolean { +function trySetKeyring(service: string, key: CredentialAccount, value: string): boolean { if (!keyringEnabled()) { return false; } try { - new Entry(KEYRING_SERVICE, key).setPassword(value); + new Entry(service, key).setPassword(value); return true; } catch { return false; } } -function tryReadKeyring(key: CredentialAccount): string | null | undefined { +function tryReadKeyring(service: string, key: CredentialAccount): string | null | undefined { if (!keyringEnabled()) { return undefined; } try { - return new Entry(KEYRING_SERVICE, key).getPassword(); + return new Entry(service, key).getPassword(); } catch { return undefined; } @@ -155,17 +160,21 @@ function tryReadKeyring(key: CredentialAccount): string | null | undefined { // existed or not); "failed" — the backend threw, so we cannot confirm the secret is gone. type KeyringRemoval = "skipped" | "removed" | "absent" | "failed"; -function removeKeyringEntry(key: CredentialAccount): KeyringRemoval { +function removeKeyringEntryIn(service: string, key: CredentialAccount): KeyringRemoval { if (!keyringEnabled()) { return "skipped"; } try { - return new Entry(KEYRING_SERVICE, key).deletePassword() ? "removed" : "absent"; + return new Entry(service, key).deletePassword() ? "removed" : "absent"; } catch { return "failed"; } } +function removeKeyringEntry(key: CredentialAccount): KeyringRemoval { + return removeKeyringEntryIn(keyringService(), key); +} + async function readProfilesFile(): Promise { const path = profilesFilePath(); let raw: string; @@ -304,8 +313,8 @@ export function keyringFallbackWarning(location: FileLocation): string { } function persistSecret(key: CredentialAccount, value: string): CredentialLocation { - if (trySetKeyring(key, value)) { - return { backend: "keyring", service: KEYRING_SERVICE, account: key }; + if (trySetKeyring(keyringService(), key, value)) { + return { backend: "keyring", service: keyringService(), account: key }; } // Falling back to the plaintext file (keyring unavailable): drop any stale keyring entry so a // recovered vault can't later shadow the file copy with an out-of-date secret. @@ -319,7 +328,23 @@ function resolveSecret(key: CredentialAccount, inline: string | null): string | if (inline !== null) { return inline; } - return tryReadKeyring(key) ?? null; + const scoped = tryReadKeyring(keyringService(), key); + if (scoped !== null && scoped !== undefined) { + return scoped; + } + return adoptLegacySecret(key); +} + +// Secrets stored before the service was scoped by config directory live under the bare service; +// copying one into this directory's scope keeps it readable after another directory's logout +// removes the shared entry. +function adoptLegacySecret(key: CredentialAccount): string | null { + const legacy = tryReadKeyring(LEGACY_KEYRING_SERVICE, key); + if (legacy === null || legacy === undefined) { + return null; + } + trySetKeyring(keyringService(), key, legacy); + return legacy; } export async function readProfileCredential( @@ -460,7 +485,7 @@ export async function writeOAuthProfile( await upsertRecord(file, name, updated); return onFile ? fileLocation(accessKey) - : { backend: "keyring", service: KEYRING_SERVICE, account: accessKey }; + : { backend: "keyring", service: keyringService(), account: accessKey }; }); } @@ -523,11 +548,15 @@ export async function clearProfile(name: string = DEFAULT_PROFILE): Promise { const file = await readProfilesFile(); const existing = findRecord(file, name); - const removals = [ - removeKeyringEntry(account.profileApiKey(name)), - removeKeyringEntry(account.profileOAuthAccess(name)), - removeKeyringEntry(account.profileOAuthRefresh(name)), + const accounts = [ + account.profileApiKey(name), + account.profileOAuthAccess(name), + account.profileOAuthRefresh(name), ]; + const removals = accounts.flatMap((key) => [ + removeKeyringEntry(key), + removeKeyringEntryIn(LEGACY_KEYRING_SERVICE, key), + ]); if (existing !== null) { flagResidualIfUnconfirmed(existing, existing.oauth !== null ? "oauth" : "apiKey", removals); } diff --git a/packages/cli/src/core/config.test.ts b/packages/cli/src/core/config.test.ts index 0f0d3e0c..90f18a05 100644 --- a/packages/cli/src/core/config.test.ts +++ b/packages/cli/src/core/config.test.ts @@ -40,6 +40,7 @@ import type { OAuthCredential } from "@metabase/client/auth/credential"; import { ConfigError } from "@metabase/client/errors"; import { + keyringService, readProfileCredential, writeOAuthProfile, writeProbeFailure, @@ -202,6 +203,16 @@ describe("resolveConfig", () => { ); }); + it("refuses --api-key - outside auth login rather than sending it as a key", async () => { + const error = await resolveConfig({ url: "https://m.example.com", apiKey: "-" }).catch( + (thrown: unknown) => thrown, + ); + assert(error instanceof ConfigError, "expected ConfigError"); + expect(error.message).toBe( + "--api-key - reads stdin only for `mb auth login`; log in a profile or set MB_API_KEY instead", + ); + }); + it("throws ConfigError when nothing is configured", async () => { const error = await resolveConfig({}).catch((thrown: unknown) => thrown); expect(error).toBeInstanceOf(ConfigError); @@ -232,7 +243,7 @@ describe("resolveConfig", () => { kind: "auth", reason: "Invalid or unauthorized API key", }); - hoisted.store.delete("metabase-cli:profile:lost:apiKey"); + hoisted.store.delete(`${keyringService()}:profile:lost:apiKey`); const error = await resolveConfig({ profile: "lost" }).catch((thrown: unknown) => thrown); expect(error).toBeInstanceOf(ConfigError); @@ -258,7 +269,7 @@ describe("resolveConfig", () => { tokenFeatures: null, }, }); - hoisted.store.delete("metabase-cli:profile:recovers:apiKey"); + hoisted.store.delete(`${keyringService()}:profile:recovers:apiKey`); const error = await resolveConfig({ profile: "recovers" }).catch((thrown: unknown) => thrown); expect(error).toBeInstanceOf(ConfigError); diff --git a/packages/cli/src/core/config.ts b/packages/cli/src/core/config.ts index 1b58f3a4..6cc7b920 100644 --- a/packages/cli/src/core/config.ts +++ b/packages/cli/src/core/config.ts @@ -8,6 +8,7 @@ import { refreshOAuthCredential } from "@metabase/client/auth/oauth-session"; import { ConfigError } from "@metabase/client/errors"; import { normalizeUrl } from "@metabase/client/url"; +import { STDIN_ARG } from "../runtime/input"; import { USER_AGENT } from "./user-agent"; import { DEFAULT_PROFILE, @@ -63,6 +64,11 @@ export function readEnvCredentials(): EnvCredentials { } export async function resolveConfig(flags: ConfigFlags): Promise { + if (flags.apiKey === STDIN_ARG) { + throw new ConfigError( + `--api-key - reads stdin only for \`mb auth login\`; log in a profile or set ${ENV_API_KEY} instead`, + ); + } const profile = resolveProfileName(flags.profile); const env = readEnvCredentials(); const hasUrl = Boolean(flags.url ?? env.url); diff --git a/packages/cli/src/output/views/database.ts b/packages/cli/src/output/views/database.ts index e09a5a51..8f0fcefa 100644 --- a/packages/cli/src/output/views/database.ts +++ b/packages/cli/src/output/views/database.ts @@ -2,6 +2,8 @@ import { type Database, DatabaseCompact, DatabaseSyncResult, + type DatabaseWithSettings, + DatabaseWithSettingsCompact, } from "@metabase/client/domain/database"; import type { ResourceView } from "../view"; @@ -23,3 +25,12 @@ export const databaseSyncResultView: ResourceView = { { key: "initial_sync_status", label: "Initial sync" }, ], }; + +export const databaseDataActionsView: ResourceView = { + compactPick: DatabaseWithSettingsCompact, + tableColumns: [ + { key: "id", label: "ID" }, + { key: "name", label: "Name" }, + { key: "engine", label: "Engine" }, + ], +}; diff --git a/packages/cli/src/runtime/command-help.test.ts b/packages/cli/src/runtime/command-help.test.ts index d645c834..e723ff24 100644 --- a/packages/cli/src/runtime/command-help.test.ts +++ b/packages/cli/src/runtime/command-help.test.ts @@ -312,6 +312,7 @@ const ALL_COMMANDS = [ "db schema-tables", "db sync-schema", "db rescan-values", + "db set-data-actions", "table list", "table get", "table fields", diff --git a/packages/cli/src/runtime/input.ts b/packages/cli/src/runtime/input.ts index a2f90bc8..77497d7b 100644 --- a/packages/cli/src/runtime/input.ts +++ b/packages/cli/src/runtime/input.ts @@ -12,6 +12,8 @@ export interface InputSources { export const DEFAULT_FLAG_NAME = "--body"; +export const STDIN_ARG = "-"; + const SOURCE_LIST = `${DEFAULT_FLAG_NAME}, --file, stdin, or a positional argument`; /** The value of a string flag, or `null` when it was left out or passed empty. */ @@ -46,7 +48,7 @@ export async function readInput(sources: InputSources): Promise { } async function readFileSource(path: string): Promise { - if (path === "-") { + if (path === STDIN_ARG) { return await drainStdin(); } try { diff --git a/packages/client/src/domain/card.ts b/packages/client/src/domain/card.ts index 84b6dbd0..bbf73450 100644 --- a/packages/client/src/domain/card.ts +++ b/packages/client/src/domain/card.ts @@ -53,6 +53,7 @@ export const CardCompact = Card.pick({ archived: true, database_id: true, collection_id: true, + dashboard_id: true, description: true, }).strip(); export type CardCompact = z.infer; diff --git a/packages/client/src/domain/database.ts b/packages/client/src/domain/database.ts index 757b1c7e..306193d7 100644 --- a/packages/client/src/domain/database.ts +++ b/packages/client/src/domain/database.ts @@ -63,6 +63,35 @@ export type DatabaseListInclude = z.infer; export const DatabaseGetInclude = z.enum(["tables", "tables.fields"]); export type DatabaseGetInclude = z.infer; +// The server merges a database's settings key by key, so an update names only the ones it changes. +const DATA_ACTIONS_SETTING = "database-enable-actions"; + +export const DatabaseSettings = z + .object({ [DATA_ACTIONS_SETTING]: z.boolean().optional() }) + .loose(); +export type DatabaseSettings = z.infer; + +export const DatabaseWithSettings = Database.extend({ settings: DatabaseSettings.nullable() }); +export type DatabaseWithSettings = z.infer; + +export const DatabaseWithSettingsCompact = DatabaseWithSettings.pick({ + id: true, + name: true, + engine: true, +}) + .strip() + .extend({ + settings: DatabaseSettings.pick({ [DATA_ACTIONS_SETTING]: true }) + .strip() + .nullable(), + }); +export type DatabaseWithSettingsCompact = z.infer; + +export const DatabaseUpdateInput = z + .object({ settings: z.object({ [DATA_ACTIONS_SETTING]: z.boolean() }).strict() }) + .strict(); +export type DatabaseUpdateInput = z.infer; + export const DatabaseSyncResult = z.object({ id: z.number().int(), status: z.literal("ok"), diff --git a/packages/client/src/resources/database.ts b/packages/client/src/resources/database.ts index 43ec4372..69ab7d8a 100644 --- a/packages/client/src/resources/database.ts +++ b/packages/client/src/resources/database.ts @@ -5,6 +5,8 @@ import { type DatabaseGetInclude, type DatabaseListInclude, type DatabaseSyncResult, + type DatabaseUpdateInput, + DatabaseWithSettings, } from "../domain/database"; import { Table } from "../domain/table"; import type { RequestOptions, Transport } from "../http/transport"; @@ -71,6 +73,19 @@ export function databaseResource(transport: Transport) { }); } + /** Update a database's settings by id; the server merges them into the ones it holds. */ + async function update( + id: number, + params: DatabaseUpdateInput, + options: RequestOptions = {}, + ): Promise { + return transport.requestParsed(DatabaseWithSettings, `/api/database/${id}`, { + ...options, + method: "PUT", + body: params, + }); + } + /** List the schema names in a database. */ async function schemas(id: number, options: RequestOptions = {}): Promise> { const data = await transport.requestParsed( @@ -134,5 +149,5 @@ export function databaseResource(transport: Transport) { return { id, status: ack.status }; } - return { list, get, schemas, schemaTables, syncSchema, rescanValues }; + return { list, get, update, schemas, schemaTables, syncSchema, rescanValues }; } diff --git a/packages/client/src/version/requirements.ts b/packages/client/src/version/requirements.ts index db4f598d..f13d865b 100644 --- a/packages/client/src/version/requirements.ts +++ b/packages/client/src/version/requirements.ts @@ -41,6 +41,7 @@ export const METHOD_REQUIREMENTS = { "dataAction.execute": [], "database.list": [], "database.get": [], + "database.update": [], "database.schemas": [], "database.schemaTables": [], "database.syncSchema": [], diff --git a/tests/e2e/auth.e2e.test.ts b/tests/e2e/auth.e2e.test.ts index daade6f9..11f95b00 100644 --- a/tests/e2e/auth.e2e.test.ts +++ b/tests/e2e/auth.e2e.test.ts @@ -598,6 +598,42 @@ describe("auth e2e", () => { }); }); + it("login with --api-key - reads the key from piped stdin", async () => { + const configHome = await makeIsolatedConfigHome(); + + const login = await runCli({ + args: ["auth", "login", "--url", bootstrap.baseUrl, "--api-key", "-", "--json"], + configHome, + stdin: `${bootstrap.adminApiKey}\n`, + }); + + expect(login.exitCode, login.stderr).toBe(0); + expect(login.stderr).not.toContain("--api-key is visible in shell history"); + const payload = parseJson(login.stdout, LoginResult); + expect(payload).toEqual({ + profile: "default", + url: bootstrap.baseUrl, + authenticated: true, + user: userOf(payload), + ...summarizeServer(bootstrap.server), + }); + }); + + it("login with --api-key - and an empty stdin fails naming the flag", async () => { + const configHome = await makeIsolatedConfigHome(); + + const login = await runCli({ + args: ["auth", "login", "--url", bootstrap.baseUrl, "--api-key", "-", "--json"], + configHome, + stdin: "", + }); + + expect(login.exitCode).toBe(2); + expect(cliErrorMessage(login.stderr)).toBe( + "--api-key - read nothing from stdin; pipe the API key in", + ); + }); + it("logout proceeds without --yes when stdin is not a TTY (non-interactive auto-confirm)", async () => { const configHome = await makeIsolatedConfigHome(); diff --git a/tests/e2e/card.e2e.test.ts b/tests/e2e/card.e2e.test.ts index 96979b2d..8e5ad5bc 100644 --- a/tests/e2e/card.e2e.test.ts +++ b/tests/e2e/card.e2e.test.ts @@ -28,6 +28,7 @@ const ORDERS_BY_STATUS_COMPACT = { archived: false, database_id: SEEDED.warehouseDbId, collection_id: SEEDED.defaultCollectionId, + dashboard_id: null, description: null, } as const; @@ -38,6 +39,7 @@ const NEW_CARD_COMPACT = { archived: false, database_id: SEEDED.warehouseDbId, collection_id: SEEDED.defaultCollectionId, + dashboard_id: null, description: null, } as const; @@ -326,6 +328,21 @@ describe("card e2e", () => { expect(archived).toEqual({ ...NEW_CARD_COMPACT, id: created.id, archived: true }); }); + it("update moving a card into a dashboard shows its dashboard_id in the compact view", async () => { + const result = await runCli({ + args: ["card", "update", String(SEEDED.ordersCardId), "--json"], + stdin: JSON.stringify({ dashboard_id: SEEDED.ordersDashboardId }), + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode, result.stderr).toBe(0); + expect(parseJson(result.stdout, CardCompact)).toEqual({ + ...ORDERS_BY_STATUS_COMPACT, + dashboard_id: SEEDED.ordersDashboardId, + }); + }); + it("create with a body missing required fields fails on Zod validation", async () => { const result = await runCli({ args: ["card", "create", "--json"], diff --git a/tests/e2e/dashboard.e2e.test.ts b/tests/e2e/dashboard.e2e.test.ts index d20451b9..623b9dde 100644 --- a/tests/e2e/dashboard.e2e.test.ts +++ b/tests/e2e/dashboard.e2e.test.ts @@ -1023,6 +1023,7 @@ describe("dashboard e2e", () => { archived: false, database_id: SEEDED.warehouseDbId, collection_id: SEEDED.defaultCollectionId, + dashboard_id: null, description: null, }); }); diff --git a/tests/e2e/db.e2e.test.ts b/tests/e2e/db.e2e.test.ts index 9070aaac..c4a7e414 100644 --- a/tests/e2e/db.e2e.test.ts +++ b/tests/e2e/db.e2e.test.ts @@ -1,7 +1,13 @@ import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { z } from "zod"; -import { Database, DatabaseCompact, DatabaseSyncResult } from "@metabase/client/domain/database"; +import { + Database, + DatabaseCompact, + DatabaseSyncResult, + DatabaseWithSettings, + DatabaseWithSettingsCompact, +} from "@metabase/client/domain/database"; import { TableCompact } from "@metabase/client/domain/table"; import { parseJson } from "@metabase/client/json"; @@ -459,4 +465,51 @@ describe("db e2e", () => { expect(result.exitCode).toBe(2); expect(cliErrorMessage(result.stderr)).toContain('invalid id: "abc" (expected integer)'); }); + + it("set-data-actions turns the database setting on and off, and get reads it back", async () => { + const id = String(SEEDED.warehouseDbId); + const on = await runCli({ + args: ["db", "set-data-actions", id, "on", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(on.exitCode, on.stderr).toBe(0); + expect(parseJson(on.stdout, DatabaseWithSettingsCompact)).toEqual({ + id: SEEDED.warehouseDbId, + name: "Warehouse", + engine: "postgres", + settings: { "database-enable-actions": true }, + }); + + const off = await runCli({ + args: ["db", "set-data-actions", id, "off", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(off.exitCode, off.stderr).toBe(0); + expect(parseJson(off.stdout, DatabaseWithSettingsCompact).settings).toEqual({ + "database-enable-actions": false, + }); + + const get = await runCli({ + args: ["db", "get", id, "--json", "--full"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(get.exitCode, get.stderr).toBe(0); + expect(parseJson(get.stdout, DatabaseWithSettings).settings?.["database-enable-actions"]).toBe( + false, + ); + }); + + it("set-data-actions refuses a state other than on or off before any request", async () => { + const result = await runCli({ + args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "yes", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toBe('invalid state: "yes" (expected one of: on, off)'); + }); }); From 787c6aed9d9a96153f1a2b1e47e7f0c1179c8ab6 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 12:04:43 -0400 Subject: [PATCH 2/7] Show namespace, visibility and mappings in compact output, list collections by namespace, name the export's branch - Compact views drop no field the trials acted on: a collection's `namespace`, a table's `visibility_type`, a dashcard's `parameter_mappings`. - `mb collection list --namespace ` lists one namespace's collections. - `mb git-sync export` reports the target `branch` in its JSON and ends its text output with it. Co-Authored-By: Claude Opus 5.5 --- README.md | 10 ++-- packages/cli/skill-data/transform/SKILL.md | 2 +- packages/cli/src/commands/collection/list.ts | 19 ++++++- packages/cli/src/commands/git-sync/export.ts | 10 +++- packages/cli/src/output/views/git-sync.ts | 1 + packages/client/src/domain/collection.ts | 1 + packages/client/src/domain/dashboard.ts | 1 + packages/client/src/domain/git-sync.ts | 3 ++ packages/client/src/domain/table.ts | 1 + packages/client/src/resources/collection.ts | 10 +++- .../client/src/resources/git-sync.test.ts | 8 ++- packages/client/src/resources/git-sync.ts | 11 ++-- tests/e2e/collection.e2e.test.ts | 54 +++++++++++++++++++ tests/e2e/dashboard.e2e.test.ts | 36 +++++++++++++ tests/e2e/db.e2e.test.ts | 7 +++ tests/e2e/library.e2e.test.ts | 1 + tests/e2e/table.e2e.test.ts | 38 +++++++++++-- 17 files changed, 190 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index f5d75ca5..ede29a99 100644 --- a/README.md +++ b/README.md @@ -1738,11 +1738,13 @@ Read and edit collections on `/api/collection`. Collections are the folders that mb collection list mb collection list --json mb collection list --filter archived --json +mb collection list --namespace transforms --json ``` -| Flag | Description | -| ------------------- | --------------------------------------------------------------------------------------------------------------- | -| `--filter ` | One of `all` (default), `archived` (returns the trash collection only), `personal` (only personal collections). | +| Flag | Description | +| ------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| `--filter ` | One of `all` (default), `archived` (returns the trash collection only), `personal` (only personal collections). | +| `--namespace ` | List one namespace's collections (`transforms`, `snippets`, `data-actions`, …), with its root, instead of normal ones. | ### `mb collection get ` @@ -2190,7 +2192,7 @@ The import also asserts which branch git-sync tracks, read from the session prop ### `mb git-sync export` -Export Metabase changes back to the configured git remote (Metabase → repo). Auto-polls by default. The export targets the branch git-sync tracks; to push to a new branch, `stash` or `create-branch` first. A `--branch` other than the tracked one is looked up in `git-sync branches` first, and one missing from the remote is refused with exit 2, naming `mb git-sync create-branch `. When the remote has moved past the last sync, a plain export ends in a `conflict` task on Metabase v63 and newer, and is refused with a 400 on older servers. Such a task names the divergence in its text output and error, and may leave the remote's commit counted as synced, as an import conflict may: never answer it with a retry. +Export Metabase changes back to the configured git remote (Metabase → repo). Auto-polls by default. The export targets the branch git-sync tracks; to push to a new branch, `stash` or `create-branch` first. The result names the target `branch` (`null` only where no `--branch` was given and the server, older than v63, exports to the one it tracks), and the text output ends with it. A `--branch` other than the tracked one is looked up in `git-sync branches` first, and one missing from the remote is refused with exit 2, naming `mb git-sync create-branch `. When the remote has moved past the last sync, a plain export ends in a `conflict` task on Metabase v63 and newer, and is refused with a 400 on older servers. Such a task names the divergence in its text output and error, and may leave the remote's commit counted as synced, as an import conflict may: never answer it with a retry. ```sh mb git-sync export -m "update dashboards" diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index bf8d9662..d833a997 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -51,7 +51,7 @@ mb transform run "$TRANSFORM_ID" --wait --profile --json - The `--json` envelope is shape-stable: `{message, run_id, final}` (plus `target_table_id` under `--sync` — a number, or `null` if the table didn't register before the timeout). `final` is `null` when `--wait` is omitted or the run never started, otherwise a full `TransformRun` with `status` and `message`. On a failed run (`final.status` ∈ {`failed`, `timeout`, `canceled`}) the CLI exits 1 and writes a one-line `transform run failed` to stderr; the failure detail lives only in `final.message` on stdout, so `jq -r '.final.message'` is where to look. - `transform create --json` returns the agent-facing compact projection: `{id, name, description, source_type, target: {type, database, schema, name}, target_db_id}`. Read `target.schema`/`target.name` directly off it — no follow-up `transform get`. - If a transform with the same `name` already has a YAML representation on disk under the configured remote-sync repo, `create` mints a `_2` suffix on the exported filename (the new transform gets a fresh `entity_id`; the prior one isn't touched). For "iterate on the same concept", prefer `transform update ` — see "Iterating on a failing transform". -- **`collection_id` only accepts a collection in the `:transforms` namespace.** Transforms aren't filed next to cards and dashboards — a normal analytics collection id fails create/update with `collection_id: A Transform can only go in Collections in the :transforms namespace.` Omit `collection_id` to leave the transform uncollected (the common case), or provision one with `mb collection create --body '{"name":"…"}' --namespace transforms --json` (see `core`) and pass the returned `id`. Cards and dashboards you build **on top of** the output table go in ordinary collections — so "put the transform and its dashboard in collection X" means _X holds the dashboard + cards; the transform stays in the transforms namespace._ +- **`collection_id` only accepts a collection in the `:transforms` namespace.** Transforms aren't filed next to cards and dashboards — a normal analytics collection id fails create/update with `collection_id: A Transform can only go in Collections in the :transforms namespace.` Omit `collection_id` to leave the transform uncollected (the common case), or reuse one from `mb collection list --namespace transforms --json`, or provision one with `mb collection create --body '{"name":"…"}' --namespace transforms --json` (see `core`) and pass the returned `id`. Cards and dashboards you build **on top of** the output table go in ordinary collections — so "put the transform and its dashboard in collection X" means _X holds the dashboard + cards; the transform stays in the transforms namespace._ ## Inspect diff --git a/packages/cli/src/commands/collection/list.ts b/packages/cli/src/commands/collection/list.ts index faf54008..dca13f06 100644 --- a/packages/cli/src/commands/collection/list.ts +++ b/packages/cli/src/commands/collection/list.ts @@ -1,4 +1,8 @@ -import { CollectionCompact, CollectionListFilter } from "@metabase/client/domain/collection"; +import { + CollectionCompact, + CollectionListFilter, + CollectionNamespace, +} from "@metabase/client/domain/collection"; import { collectionView } from "../../output/views/collection"; import { renderList } from "../../output/render"; @@ -6,6 +10,7 @@ import { listEnvelopeSchema } from "../../output/types"; import { windowList } from "../../output/window"; import { connectionFlags, listFlags, outputFlags, profileFlag } from "../flags"; import { parseEnumFlag } from "../parse-enum"; +import { givenValue } from "../../runtime/input"; import { defineMetabaseCommand } from "../runtime"; export const CollectionListEnvelope = listEnvelopeSchema(CollectionCompact); @@ -23,17 +28,27 @@ export default defineMetabaseCommand({ description: `Filter preset: ${CollectionListFilter.options.join("|")}`, default: "all", }, + namespace: { + type: "string", + description: `List one namespace's collections: ${CollectionNamespace.options.join("|")} (omit for normal collections)`, + }, }, outputSchema: CollectionListEnvelope, examples: [ "mb collection list", "mb collection list --json", "mb collection list --filter archived --json", + "mb collection list --namespace transforms --json", ], async run({ args, ctx, getClient }) { const filter = parseEnumFlag(args.filter, CollectionListFilter, "filter"); + const namespaceFlag = givenValue(args.namespace); + const namespace = + namespaceFlag === null + ? undefined + : parseEnumFlag(namespaceFlag, CollectionNamespace, "namespace"); const client = await getClient(); - const collections = await client.collection.list({ filter }); + const collections = await client.collection.list({ filter, namespace }); renderList(windowList(collections.data, ctx.range), collectionView, ctx); }, }); diff --git a/packages/cli/src/commands/git-sync/export.ts b/packages/cli/src/commands/git-sync/export.ts index 76755070..083040bb 100644 --- a/packages/cli/src/commands/git-sync/export.ts +++ b/packages/cli/src/commands/git-sync/export.ts @@ -90,13 +90,19 @@ export default defineMetabaseCommand({ } const result = await mb.gitSync.export(params); + const target = result.branch === null ? "" : ` Branch: ${result.branch}.`; if (!wait.enabled) { - renderSummary(result, syncExportView, `Started export task #${result.task_id}.`, ctx); + renderSummary( + result, + syncExportView, + `Started export task #${result.task_id}.${target}`, + ctx, + ); } else { const final = result.final ?? null; const text = final === null ? `Export task #${result.task_id} finished.` : formatSyncTask(final); - renderSummary(result, syncExportView, text, ctx); + renderSummary(result, syncExportView, `${text}${target}`, ctx); throwIfFailedTask(final, "export"); } emitRealignHint(ctx); diff --git a/packages/cli/src/output/views/git-sync.ts b/packages/cli/src/output/views/git-sync.ts index 7852f0e6..ac86b3f4 100644 --- a/packages/cli/src/output/views/git-sync.ts +++ b/packages/cli/src/output/views/git-sync.ts @@ -76,6 +76,7 @@ export const syncExportView: ResourceView = { compactPick: SyncExportResult, tableColumns: [ { key: "task_id", label: "Task ID" }, + { key: "branch", label: "Branch" }, { key: "message", label: "Message" }, ], }; diff --git a/packages/client/src/domain/collection.ts b/packages/client/src/domain/collection.ts index 8cae3c19..b18aaf84 100644 --- a/packages/client/src/domain/collection.ts +++ b/packages/client/src/domain/collection.ts @@ -97,6 +97,7 @@ export const CollectionCompact = Collection.pick({ location: true, parent_id: true, type: true, + namespace: true, authority_level: true, is_personal: true, is_remote_synced: true, diff --git a/packages/client/src/domain/dashboard.ts b/packages/client/src/domain/dashboard.ts index 63468a5d..de408b08 100644 --- a/packages/client/src/domain/dashboard.ts +++ b/packages/client/src/domain/dashboard.ts @@ -36,6 +36,7 @@ export const DashcardCompact = Dashcard.pick({ col: true, size_x: true, size_y: true, + parameter_mappings: true, }).strip(); export type DashcardCompact = z.infer; diff --git a/packages/client/src/domain/git-sync.ts b/packages/client/src/domain/git-sync.ts index aabf3e88..4ff54eaf 100644 --- a/packages/client/src/domain/git-sync.ts +++ b/packages/client/src/domain/git-sync.ts @@ -151,9 +151,12 @@ export const SyncImportResult = z.object({ }); export type SyncImportResult = z.infer; +// `branch` is the branch the export targets, null where the request named none and a server without +// the branch guard exports to the one it tracks. export const SyncExportResult = z.object({ message: z.string(), task_id: z.number().int().positive(), + branch: z.string().nullable(), final: SyncTask.nullable().optional(), }); export type SyncExportResult = z.infer; diff --git a/packages/client/src/domain/table.ts b/packages/client/src/domain/table.ts index ccec4b71..03dc6468 100644 --- a/packages/client/src/domain/table.ts +++ b/packages/client/src/domain/table.ts @@ -51,6 +51,7 @@ export const TableCompact = Table.pick({ db_id: true, schema: true, entity_type: true, + visibility_type: true, active: true, is_published: true, }) diff --git a/packages/client/src/resources/collection.ts b/packages/client/src/resources/collection.ts index 94d3627d..02c2a4ba 100644 --- a/packages/client/src/resources/collection.ts +++ b/packages/client/src/resources/collection.ts @@ -7,6 +7,7 @@ import { CollectionItem, type CollectionItemFilterModel, type CollectionListFilter, + type CollectionNamespace, type CollectionPinnedState, CollectionTreeNode, type CollectionUpdateInput, @@ -33,6 +34,7 @@ const DEFAULT_LIST_FILTER = "all"; export interface CollectionListParams { filter?: CollectionListFilter | undefined; + namespace?: CollectionNamespace | undefined; } export interface CollectionItemListParams { @@ -73,7 +75,8 @@ export async function listCollectionsAs( export function collectionResource(transport: Transport) { /** * List collections. `filter` picks a server-side preset: everything, archived, or personal. Every - * preset includes the Library collections that match it. + * preset includes the Library collections that match it. `namespace` lists that namespace's + * collections, with its root, instead of the default namespace's. */ async function list( params: CollectionListParams = {}, @@ -81,7 +84,10 @@ export function collectionResource(transport: Transport) { ): Promise> { const data = await transport.requestParsed(CollectionApiList, "/api/collection", { ...options, - query: COLLECTION_LIST_QUERY[params.filter ?? DEFAULT_LIST_FILTER], + query: { + ...COLLECTION_LIST_QUERY[params.filter ?? DEFAULT_LIST_FILTER], + namespace: params.namespace, + }, }); return { data, total: null }; } diff --git a/packages/client/src/resources/git-sync.test.ts b/packages/client/src/resources/git-sync.test.ts index 0d6baaa3..67260d42 100644 --- a/packages/client/src/resources/git-sync.test.ts +++ b/packages/client/src/resources/git-sync.test.ts @@ -539,8 +539,11 @@ describe("git-sync resource wire requests", () => { jsonResponse({ message: "Export queued", task_id: 8 }), ]); - await mb.gitSync.export({ message: "update dashboards" }); - + expect(await mb.gitSync.export({ message: "update dashboards" })).toEqual({ + message: "Export queued", + task_id: 8, + branch: "main", + }); expect(capture.calls.map((call) => call.body)).toEqual([ null, '{"branch":"main","message":"update dashboards"}', @@ -557,6 +560,7 @@ describe("git-sync resource wire requests", () => { expect(await mb.gitSync.export({ wait: IMMEDIATE_POLL })).toEqual({ message: "Export queued", task_id: 8, + branch: "main", final: SETTLED_TASK, }); }); diff --git a/packages/client/src/resources/git-sync.ts b/packages/client/src/resources/git-sync.ts index 5d73bf9b..8bc83e07 100644 --- a/packages/client/src/resources/git-sync.ts +++ b/packages/client/src/resources/git-sync.ts @@ -286,14 +286,15 @@ export function gitSyncResource(transport: Transport) { merge: params.merge, }, }); - if (params.wait === undefined) { - return { message: started.message, task_id: started.task_id }; - } - return { + const accepted = { message: started.message, task_id: started.task_id, - final: await settle(params.wait, options), + branch: branchName ?? null, }; + if (params.wait === undefined) { + return accepted; + } + return { ...accepted, final: await settle(params.wait, options) }; } /** diff --git a/tests/e2e/collection.e2e.test.ts b/tests/e2e/collection.e2e.test.ts index 3e42cf17..0bda141d 100644 --- a/tests/e2e/collection.e2e.test.ts +++ b/tests/e2e/collection.e2e.test.ts @@ -6,6 +6,7 @@ import { Collection, CollectionCompact, type CollectionId, + CollectionNamespace, } from "@metabase/client/domain/collection"; import { parseJson } from "@metabase/client/json"; @@ -28,6 +29,7 @@ const DEFAULT_COMPACT = { location: "/", parent_id: null, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -50,6 +52,7 @@ const ADMIN_PERSONAL_COMPACT = { location: "/", parent_id: null, type: null, + namespace: null, authority_level: null, is_personal: true, is_remote_synced: false, @@ -63,6 +66,7 @@ const TRASH_COMPACT = { location: "/", parent_id: null, type: "trash", + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -598,12 +602,57 @@ describe("collection e2e", () => { location: `/${SEEDED.defaultCollectionId}/`, parent_id: SEEDED.defaultCollectionId, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, }); }); + it("create --namespace shows the namespace, and list --namespace lists that namespace alone", async () => { + const createResult = await runCli({ + args: ["collection", "create", "--namespace", "transforms", "--json"], + stdin: JSON.stringify({ name: "e2e_transforms_collection" }), + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(createResult.exitCode, createResult.stderr).toBe(0); + const created = parseJson(createResult.stdout, CollectionCompact); + expect(created).toEqual({ + id: created.id, + name: "e2e_transforms_collection", + description: null, + archived: false, + location: "/", + type: null, + namespace: "transforms", + authority_level: null, + is_remote_synced: false, + }); + + const listResult = await runCli({ + args: ["collection", "list", "--namespace", "transforms", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(listResult.exitCode, listResult.stderr).toBe(0); + const ids = parseJson(listResult.stdout, CollectionListEnvelope).data.map((row) => row.id); + expect(ids).toEqual(["root", created.id]); + }); + + it("list --namespace refuses an unknown namespace before any request", async () => { + const result = await runCli({ + args: ["collection", "list", "--namespace", "widgets", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(cliErrorMessage(result.stderr)).toBe( + `invalid namespace: "widgets" (expected one of: ${CollectionNamespace.options.join(", ")})`, + ); + }); + it("create with a body missing the required name field fails on Zod validation", async () => { const result = await runCli({ args: ["collection", "create", "--json"], @@ -666,6 +715,7 @@ describe("collection e2e", () => { location: "/", parent_id: null, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -698,6 +748,7 @@ describe("collection e2e", () => { location: `/${SEEDED.defaultCollectionId}/`, parent_id: SEEDED.defaultCollectionId, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -717,6 +768,7 @@ describe("collection e2e", () => { location: "/", parent_id: null, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -742,6 +794,7 @@ describe("collection e2e", () => { location: `/${SEEDED.defaultCollectionId}/`, parent_id: SEEDED.defaultCollectionId, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, @@ -779,6 +832,7 @@ describe("collection e2e", () => { location: "/", parent_id: null, type: null, + namespace: null, authority_level: null, is_personal: false, is_remote_synced: false, diff --git a/tests/e2e/dashboard.e2e.test.ts b/tests/e2e/dashboard.e2e.test.ts index 623b9dde..ddaf1dae 100644 --- a/tests/e2e/dashboard.e2e.test.ts +++ b/tests/e2e/dashboard.e2e.test.ts @@ -43,6 +43,7 @@ const ORDERS_OVERVIEW_FIRST_DASHCARD_COMPACT = { col: 0, size_x: 12, size_y: 6, + parameter_mappings: [], } as const; const ORDERS_OVERVIEW_DETAIL_COMPACT = { @@ -338,6 +339,7 @@ describe("dashboard e2e", () => { col: 0, size_x: 12, size_y: 6, + parameter_mappings: [], }, ], returned: 1, @@ -348,6 +350,35 @@ describe("dashboard e2e", () => { }); }); + it("cards shows each dashcard's parameter mappings in compact form", async () => { + const mapping = { + parameter_id: "order_id", + card_id: SEEDED.ordersCardId, + target: ["dimension", ["field", SEEDED.fields.ordersId, null], { "stage-number": 0 }], + }; + const created = await runCli({ + args: ["dashboard", "create", "--json"], + stdin: JSON.stringify({ + name: "e2e_dashboard_with_mapping", + collection_id: SEEDED.defaultCollectionId, + parameters: [{ id: "order_id", slug: "order_id", name: "Order ID", type: "id" }], + dashcards: [ + { + ...singleDashcardBody("unused", SEEDED.ordersCardId).dashcards[0], + parameter_mappings: [mapping], + }, + ], + }), + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + expect(created.exitCode, created.stderr).toBe(0); + const dashboardId = parseJson(created.stdout, DashboardCompact).id; + + const envelope = await dashcardsOf(dashboardId); + expect(envelope.data.map((dashcard) => dashcard.parameter_mappings)).toEqual([[mapping]]); + }); + it("cards with a non-integer id fails fast with ConfigError", async () => { const result = await runCli({ args: ["dashboard", "cards", "abc", "--json"], @@ -494,6 +525,7 @@ describe("dashboard e2e", () => { col: 0, size_x: 12, size_y: 6, + parameter_mappings: [], }, ], }); @@ -685,6 +717,7 @@ describe("dashboard e2e", () => { col: 2, size_x: 10, size_y: 5, + parameter_mappings: [], }); }); @@ -821,6 +854,7 @@ describe("dashboard e2e", () => { col: 1, size_x: 12, size_y: 6, + parameter_mappings: [], }); expect(untouched).toEqual({ id: secondDashcard.id, @@ -831,6 +865,7 @@ describe("dashboard e2e", () => { col: 0, size_x: 6, size_y: 3, + parameter_mappings: [], }); }); @@ -970,6 +1005,7 @@ describe("dashboard e2e", () => { col: 0, size_x: 12, size_y: 6, + parameter_mappings: [], }, ], returned: 1, diff --git a/tests/e2e/db.e2e.test.ts b/tests/e2e/db.e2e.test.ts index c4a7e414..77c32b3f 100644 --- a/tests/e2e/db.e2e.test.ts +++ b/tests/e2e/db.e2e.test.ts @@ -40,6 +40,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/GenericTable", + visibility_type: null, active: true, is_published: false, }, @@ -51,6 +52,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -62,6 +64,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -73,6 +76,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -84,6 +88,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/ProductTable", + visibility_type: null, active: true, is_published: false, }, @@ -95,6 +100,7 @@ const PUBLIC_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/GenericTable", + visibility_type: null, active: true, is_published: false, }, @@ -109,6 +115,7 @@ const ANALYTICS_TABLES_SORTED_BY_DISPLAY_NAME: TableCompact[] = [ db_id: SEEDED.warehouseDbId, schema: "analytics", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, diff --git a/tests/e2e/library.e2e.test.ts b/tests/e2e/library.e2e.test.ts index 268e55cb..f2d32882 100644 --- a/tests/e2e/library.e2e.test.ts +++ b/tests/e2e/library.e2e.test.ts @@ -28,6 +28,7 @@ const REVIEWS_COMPACT = { db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/GenericTable", + visibility_type: null, active: true, is_published: false, }; diff --git a/tests/e2e/table.e2e.test.ts b/tests/e2e/table.e2e.test.ts index 9065482b..4cc6ee27 100644 --- a/tests/e2e/table.e2e.test.ts +++ b/tests/e2e/table.e2e.test.ts @@ -23,7 +23,7 @@ import { seedProbedProfile, UNREACHABLE_SEED_MESSAGE } from "./seed-profile"; import { SEEDED } from "./seed/seeded"; import { serverHas } from "./server-gate"; -const CUSTOMERS_COMPACT = { +const CUSTOMERS_COMPACT: TableCompact = { id: SEEDED.tables.customers, name: "customers", display_name: "Customers", @@ -31,11 +31,12 @@ const CUSTOMERS_COMPACT = { db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/GenericTable", + visibility_type: null, active: true, is_published: false, }; -const REVIEWS_COMPACT = { +const REVIEWS_COMPACT: TableCompact = { id: SEEDED.tables.reviews, name: "reviews", display_name: "Reviews", @@ -43,11 +44,12 @@ const REVIEWS_COMPACT = { db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/GenericTable", + visibility_type: null, active: true, is_published: false, }; -const SEEDED_WAREHOUSE_TABLES = [ +const SEEDED_WAREHOUSE_TABLES: TableCompact[] = [ CUSTOMERS_COMPACT, { id: SEEDED.tables.dailySales, @@ -57,6 +59,7 @@ const SEEDED_WAREHOUSE_TABLES = [ db_id: SEEDED.warehouseDbId, schema: "analytics", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -68,6 +71,7 @@ const SEEDED_WAREHOUSE_TABLES = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -79,6 +83,7 @@ const SEEDED_WAREHOUSE_TABLES = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -90,6 +95,7 @@ const SEEDED_WAREHOUSE_TABLES = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/TransactionTable", + visibility_type: null, active: true, is_published: false, }, @@ -101,6 +107,7 @@ const SEEDED_WAREHOUSE_TABLES = [ db_id: SEEDED.warehouseDbId, schema: "public", entity_type: "entity/ProductTable", + visibility_type: null, active: true, is_published: false, }, @@ -119,7 +126,7 @@ function bulkEditRefusal(serverTag: string | undefined): string { return `This operation requires Metabase v59+ (this server is ${serverTag}). Upgrade Metabase to use it.\n${DOWNGRADE_REMEDY}`; } -function warehouseEnvelope(data: typeof SEEDED_WAREHOUSE_TABLES) { +function warehouseEnvelope(data: TableCompact[]) { return { data, returned: data.length, @@ -279,7 +286,7 @@ describe("table e2e", () => { expect(result.exitCode, result.stderr).toBe(0); expect(parseJson(result.stdout, TableListEnvelope)).toEqual( - warehouseEnvelope([REVIEWS_COMPACT]), + warehouseEnvelope([{ ...REVIEWS_COMPACT, visibility_type: "hidden" }]), ); }); @@ -697,6 +704,27 @@ describe("table e2e", () => { expect(parseJson(restore.stdout, Table).description).toBeNull(); }); + it("update shows the visibility it set in the compact view", async () => { + const result = await runCli({ + args: [ + "table", + "update", + String(SEEDED.tables.reviews), + "--body", + JSON.stringify({ visibility_type: "hidden" }), + "--json", + ], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode, result.stderr).toBe(0); + expect(parseJson(result.stdout, TableCompact)).toEqual({ + ...REVIEWS_COMPACT, + visibility_type: "hidden", + }); + }); + it("update rejects multiple body sources", async () => { const result = await runCli({ args: [ From fc4064e0efd4c882308569f9284b790ec093cfb1 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 12:43:18 -0400 Subject: [PATCH 3/7] Accept search rows without a description, refuse a missing positional as an argv error - A document or indexed-entity search row carries no `description`, which failed every search whose results held one. - A missing required positional is refused with exit 2 naming it (or the unknown flag standing in its place) instead of citty printing the command's help, which `--json` turned into JSON help. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli.ts | 45 ++++++++++++++++++++++++-- packages/cli/src/commands/argv.test.ts | 26 ++++++++++++++- packages/cli/src/commands/argv.ts | 17 ++++++++++ packages/client/src/domain/search.ts | 3 +- tests/e2e/db.e2e.test.ts | 26 +++++++++++++++ tests/e2e/search.e2e.test.ts | 37 +++++++++++++++++++++ 6 files changed, 150 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index ad5ab33b..e49807d3 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,10 +1,12 @@ #!/usr/bin/env node -import { runMain } from "citty"; +import { parseArgs, runMain } from "citty"; import type { ArgsDef, CommandDef } from "citty"; import { ConfigError } from "@metabase/client/errors"; -import { separatePositionals } from "./commands/argv"; +import { assertRequiredPositionals, separatePositionals } from "./commands/argv"; +import { resolveOutputFormat } from "./commands/context"; +import type { Format } from "./output/types"; import { hoistGlobalFlags } from "./commands/global-flags"; import { trustSystemCa } from "./core/system-ca"; import main from "./main"; @@ -20,6 +22,7 @@ import { installInterruptHandler } from "./runtime/interrupt"; const HELP_FLAGS: ReadonlySet = new Set(["--help", "-h"]); const JSON_HELP_FLAG = "--json"; +const OUTPUT_FORMAT_FLAGS = ["json", "format"] as const; async function run(): Promise { installInterruptHandler((code) => process.exit(code)); @@ -49,10 +52,48 @@ async function run(): Promise { reportError(new ConfigError(`unknown command: ${unknown}`)); return; } + if (await refusedLeafArgv(rawArgs)) { + return; + } } await runMain(main, { showUsage: showUsageWithBreadcrumb, rawArgs }); } +async function refusedLeafArgv(rawArgs: readonly string[]): Promise { + const leaf = await resolveLeafArgv(main, rawArgs); + if (leaf === null) { + return false; + } + const leafArgs = rawArgs.slice(leaf.argsStart); + try { + assertRequiredPositionals(leafArgs, leaf.argsDef); + return false; + } catch (error) { + reportError(error, reportFormat(leafArgs, leaf.argsDef)); + return true; + } +} + +// Parsed against the output flags alone: citty's parse of the whole definition throws on the very +// positional that is missing. A `--format` value the command would itself refuse leaves the argv +// error in plain text. +function reportFormat(leafArgs: readonly string[], argsDef: ArgsDef): Format | undefined { + const outputFlags = Object.fromEntries( + OUTPUT_FORMAT_FLAGS.flatMap((key) => { + const def = argsDef[key]; + return def === undefined ? [] : [[key, def]]; + }), + ); + const parsed = parseArgs([...leafArgs], outputFlags); + const format = parsed["format"]; + const json = parsed["json"] === true; + try { + return resolveOutputFormat(typeof format === "string" ? { json, format } : { json }); + } catch { + return undefined; + } +} + async function normalizeArgv(argv: readonly string[]): Promise { const hoisted = hoistGlobalFlags(argv); const leaf = await resolveLeafArgv(main, hoisted); diff --git a/packages/cli/src/commands/argv.test.ts b/packages/cli/src/commands/argv.test.ts index 9d3da318..0ceb7275 100644 --- a/packages/cli/src/commands/argv.test.ts +++ b/packages/cli/src/commands/argv.test.ts @@ -3,7 +3,7 @@ import { assert, describe, expect, it } from "vitest"; import { ConfigError } from "@metabase/client/errors"; -import { assertArgv } from "./argv"; +import { assertArgv, assertRequiredPositionals } from "./argv"; const ARGS: ArgsDef = { format: { type: "string", default: "auto" }, @@ -80,3 +80,27 @@ describe("assertArgv", () => { expect(() => assertArgv(["--version"], ARGS)).not.toThrow(); }); }); + +const TWO_POSITIONALS: ArgsDef = { + json: { type: "boolean" }, + id: { type: "positional", required: true }, + state: { type: "positional", required: true }, +}; + +describe("assertRequiredPositionals", () => { + it("accepts argv that gives every required positional", () => { + expect(() => assertRequiredPositionals(["1", "on", "--json"], TWO_POSITIONALS)).not.toThrow(); + }); + + it("names the missing positional and the command's positionals", () => { + const error = thrownBy(() => assertRequiredPositionals(["1", "--json"], TWO_POSITIONALS)); + assert(error instanceof ConfigError, "expected ConfigError"); + expect(error.message).toBe("missing argument: (this command takes )"); + }); + + it("reports an unknown flag standing where the positional belongs as the unknown flag", () => { + const error = thrownBy(() => assertRequiredPositionals(["1", "--on"], TWO_POSITIONALS)); + assert(error instanceof ConfigError, "expected ConfigError"); + expect(error.message).toBe("unknown flag: --on"); + }); +}); diff --git a/packages/cli/src/commands/argv.ts b/packages/cli/src/commands/argv.ts index ef931cf2..6e1b47b6 100644 --- a/packages/cli/src/commands/argv.ts +++ b/packages/cli/src/commands/argv.ts @@ -102,6 +102,23 @@ function readValueFlag(item: FlagItem, spellings: FlagSpellings): ValueFlag | nu return { name, flag, value: readFlagValue(item, spellings) }; } +// citty refuses a missing required positional before a command runs, by printing the command's +// help (its JSON help under `--json`) and a bare message, so a mistyped flag standing where the +// positional belongs reads as a help request. Checked ahead of citty, it is the argv error it is. +export function assertRequiredPositionals(rawArgs: readonly string[], argsDef: ArgsDef): void { + const given = [...readArgv(rawArgs, commandSpellings(argsDef))].filter( + (item) => item.kind === "positional", + ).length; + const declared = Object.entries(argsDef).filter(([, def]) => def.type === "positional"); + const missing = declared.slice(given).find(([, def]) => def.required === true); + if (missing === undefined) { + return; + } + assertArgv(rawArgs, argsDef); + const usage = declared.map(([name]) => `<${name}>`).join(" "); + throw new ConfigError(`missing argument: <${missing[0]}> (this command takes ${usage})`); +} + function assertPositionalCount(positionals: readonly string[], argsDef: ArgsDef): void { const declared = Object.entries(argsDef) .filter(([, def]) => def.type === "positional") diff --git a/packages/client/src/domain/search.ts b/packages/client/src/domain/search.ts index 3e3385a4..ed5e549e 100644 --- a/packages/client/src/domain/search.ts +++ b/packages/client/src/domain/search.ts @@ -34,7 +34,8 @@ export const SearchResult = z id: z.union([z.number().int(), z.string()]), name: z.string(), model: SearchModel, - description: z.string().nullable(), + // A model whose search spec carries no description (documents, indexed entities) leaves the key out. + description: z.string().nullable().optional(), archived: z.boolean().nullable(), collection: SearchResultCollection.nullable(), result_metadata: z.array(z.unknown()).nullable().optional(), diff --git a/tests/e2e/db.e2e.test.ts b/tests/e2e/db.e2e.test.ts index 77c32b3f..32b7fde6 100644 --- a/tests/e2e/db.e2e.test.ts +++ b/tests/e2e/db.e2e.test.ts @@ -519,4 +519,30 @@ describe("db e2e", () => { expect(result.exitCode).toBe(2); expect(cliErrorMessage(result.stderr)).toBe('invalid state: "yes" (expected one of: on, off)'); }); + + it("set-data-actions with a flag where the state belongs refuses the flag instead of printing help", async () => { + const result = await runCli({ + args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "--on", "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(result.stdout).toBe(""); + expect(cliErrorMessage(result.stderr)).toBe("unknown flag: --on"); + }); + + it("set-data-actions without the state names the missing argument", async () => { + const result = await runCli({ + args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "--json"], + configHome: await makeIsolatedConfigHome(), + env: authEnv(), + }); + + expect(result.exitCode).toBe(2); + expect(result.stdout).toBe(""); + expect(cliErrorMessage(result.stderr)).toBe( + "missing argument: (this command takes )", + ); + }); }); diff --git a/tests/e2e/search.e2e.test.ts b/tests/e2e/search.e2e.test.ts index ac2fb06c..3291cb4d 100644 --- a/tests/e2e/search.e2e.test.ts +++ b/tests/e2e/search.e2e.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { z } from "zod"; import { Card, CardCompact, type CardCreateInput } from "@metabase/client/domain/card"; +import { DocumentCompact } from "@metabase/client/domain/document"; import { SEARCH_MODELS, SearchResult } from "@metabase/client/domain/search"; import { parseJson } from "@metabase/client/json"; import { pollUntil } from "@metabase/client/poll"; @@ -357,6 +358,42 @@ describe("search e2e", () => { }); }); + it("--models across cards, dashboards, metrics and documents answers a document row, which carries no description", async () => { + const configHome = await makeIsolatedConfigHome(); + const created = await runCli({ + args: ["document", "create", "--json"], + stdin: JSON.stringify({ + name: "Orders memo", + collection_id: SEEDED.defaultCollectionId, + document: { type: "doc", content: [] }, + }), + configHome, + env: authEnv(), + }); + expect(created.exitCode, created.stderr).toBe(0); + const documentId = parseJson(created.stdout, DocumentCompact).id; + + const envelope = await pollUntil( + async () => { + const result = await runCli({ + args: ["search", "Orders", "--models", "card,dashboard,metric,document", "--json"], + configHome, + env: authEnv(), + }); + expect(result.exitCode, result.stderr).toBe(0); + return parseJson(result.stdout, SearchListEnvelope); + }, + (candidate) => candidate.data.some((row) => row.model === "document"), + SEARCH_INDEX_POLL, + ); + + expect(envelope.data.toSorted((a, b) => a.model.localeCompare(b.model))).toEqual([ + ORDERS_BY_STATUS_COMPACT, + ORDERS_OVERVIEW_COMPACT, + { id: documentId, name: "Orders memo", model: "document" }, + ]); + }); + it("--include-metadata attaches the card's result columns to its full row", async () => { const configHome = await makeIsolatedConfigHome(); const full = await runCli({ From ddc91b85d361d999d6f91c3cc4c1a33d24b10c89 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 13:15:55 -0400 Subject: [PATCH 4/7] Keep the keychain service unscoped Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- packages/cli/src/commands/auth/list.test.ts | 4 +- packages/cli/src/core/auth/storage.test.ts | 65 +++++---------------- packages/cli/src/core/auth/storage.ts | 59 +++++-------------- packages/cli/src/core/config.test.ts | 5 +- 5 files changed, 33 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index ede29a99..91b5f12d 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,7 @@ mb auth status --profile staging ### `mb auth list` -List configured authentication profiles. All profile metadata (URL, auth method, last successful probe, last failure) lives in `/profiles.json` at mode `0600`; the secrets (API key, or OAuth access/refresh tokens) sit in the OS keychain when available (service `metabase-cli:`, so each config directory keeps its own), or inline in the same file when the keychain is unavailable. +List configured authentication profiles. All profile metadata (URL, auth method, last successful probe, last failure) lives in `/profiles.json` at mode `0600`; the secrets (API key, or OAuth access/refresh tokens) sit in the OS keychain when available, or inline in the same file when the keychain is unavailable. `auth list` re-probes every profile, one at a time — a probe can refresh and rewrite an expired OAuth token, so probes are serialized to avoid racing on the shared `profiles.json`. On success it refreshes `lastProbe` (Metabase version, token features, user identity) and clears `lastFailure`; on failure it updates `lastFailure` and leaves the prior `lastProbe`/`url`/credential untouched. Rendered columns: `Profile | URL | Auth | Status | Role | Version | Skew | Last probed`; `--json` rows carry the same derived `edition`, `skew`, `knownRange` and `features` as `auth status`. Failed rows append a one-line footer pointing at `mb auth login --profile `. diff --git a/packages/cli/src/commands/auth/list.test.ts b/packages/cli/src/commands/auth/list.test.ts index a734fbd5..f4a650b2 100644 --- a/packages/cli/src/commands/auth/list.test.ts +++ b/packages/cli/src/commands/auth/list.test.ts @@ -37,7 +37,7 @@ vi.mock("../../core/auth/verify", () => ({ })); import authListCommand, { AuthProfileListEnvelope } from "./list"; -import { keyringService, readProfileRecord, writeProfile } from "../../core/auth/storage"; +import { writeProfile, readProfileRecord } from "../../core/auth/storage"; import { setupTempConfigHome, type TempConfigHome } from "../../core/auth/temp-config-home"; interface CapturedStdout { @@ -313,6 +313,6 @@ describe("auth list command", () => { reason: "Could not reach Metabase: getaddrinfo ENOTFOUND", }, }); - expect(hoisted.store.get(`${keyringService()}:profile:stable:apiKey`)).toBe("good"); + expect(hoisted.store.get("metabase-cli:profile:stable:apiKey")).toBe("good"); }); }); diff --git a/packages/cli/src/core/auth/storage.test.ts b/packages/cli/src/core/auth/storage.test.ts index 5f82b269..9f3f7c94 100644 --- a/packages/cli/src/core/auth/storage.test.ts +++ b/packages/cli/src/core/auth/storage.test.ts @@ -22,7 +22,6 @@ import * as storage from "./storage"; const { clearProfile, - keyringService, consumeKeychainResidualWarning, consumeKeyringDowngradeWarning, consumeLegacyStorageWarning, @@ -93,10 +92,10 @@ describe("profiles (keyring backend)", () => { const location = await writeProfile({ url: "https://m.example.com", apiKey: "secret" }); expect(location).toEqual({ backend: "keyring", - service: keyringService(), + service: "metabase-cli", account: "profile:default:apiKey", }); - expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBe("secret"); + expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBe("secret"); const file = parseJson(readFileSync(profilesFilePath(), "utf8"), ProfilesFile); expect(file).toEqual({ @@ -135,44 +134,6 @@ describe("profiles (keyring backend)", () => { }); }); - it("keeps same-named profiles in different config directories apart", async () => { - await writeProfile({ url: "https://first.example.com", apiKey: "first-key" }); - const second = setupTempConfigHome(); - try { - await writeProfile({ url: "https://second.example.com", apiKey: "second-key" }); - expect(await readProfileCredential()).toEqual({ - url: "https://second.example.com", - credential: { kind: "apiKey", apiKey: "second-key" }, - }); - } finally { - second.cleanup(); - } - expect(await readProfileCredential()).toEqual({ - url: "https://first.example.com", - credential: { kind: "apiKey", apiKey: "first-key" }, - }); - }); - - it("reads a key stored under the unscoped service and copies it into this directory's scope", async () => { - await writeProfile({ url: "https://m.example.com", apiKey: "old-key" }); - hoisted.store.delete(`${keyringService()}:profile:default:apiKey`); - hoisted.store.set("metabase-cli:profile:default:apiKey", "old-key"); - - expect(await readProfileCredential()).toEqual({ - url: "https://m.example.com", - credential: { kind: "apiKey", apiKey: "old-key" }, - }); - expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBe("old-key"); - }); - - it("clearProfile also removes the key stored under the unscoped service", async () => { - await writeProfile({ url: "https://m.example.com", apiKey: "k" }); - hoisted.store.set("metabase-cli:profile:default:apiKey", "old-key"); - - expect(await clearProfile()).toBe(true); - expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBeUndefined(); - }); - it("preserves user add order (no sort) and overwrites existing entries in place", async () => { await writeProfile({ url: "https://1.example.com", apiKey: "k1" }, "zeta"); await writeProfile({ url: "https://2.example.com", apiKey: "k2" }, "alpha"); @@ -191,7 +152,7 @@ describe("profiles (keyring backend)", () => { expect(await clearProfile("a")).toBe(true); expect(await readProfileCredential("a")).toBeNull(); - expect(hoisted.store.get(`${keyringService()}:profile:a:apiKey`)).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:a:apiKey")).toBeUndefined(); expect(await readProfileCredential("b")).toEqual({ url: "https://b.example.com", credential: { kind: "apiKey", apiKey: "b" }, @@ -386,7 +347,7 @@ describe("writeProbeResult and writeProbeFailure", () => { lastProbe: ALICE_PROBE, lastFailure: { at: FAILED_AT, kind: "auth", reason: "Invalid or unauthorized API key" }, }); - expect(hoisted.store.get(`${keyringService()}:profile:p:apiKey`)).toBe("k"); + expect(hoisted.store.get("metabase-cli:profile:p:apiKey")).toBe("k"); }); }); @@ -422,7 +383,7 @@ describe("MB_CLI_DISABLE_KEYRING", () => { const location = await writeProfile({ url: "https://m.example.com", apiKey: "secret" }); expect(location).toEqual({ backend: "keyring", - service: keyringService(), + service: "metabase-cli", account: "profile:default:apiKey", }); }); @@ -533,8 +494,8 @@ describe("OAuth profiles (keyring backend)", () => { lastProbe: null, lastFailure: null, }); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBe("access-1"); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBe("refresh-1"); + expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBe("access-1"); + expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBe("refresh-1"); }); it("switching an OAuth profile to an API key clears the OAuth tokens", async () => { @@ -545,8 +506,8 @@ describe("OAuth profiles (keyring backend)", () => { credential: { kind: "apiKey", apiKey: "k" }, }); expect(await readProfileRecord()).toEqual(defaultRecord(null)); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBeUndefined(); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBeUndefined(); }); it("switching an API key profile to OAuth clears the API key", async () => { @@ -569,14 +530,14 @@ describe("OAuth profiles (keyring backend)", () => { lastProbe: null, lastFailure: null, }); - expect(hoisted.store.get(`${keyringService()}:profile:default:apiKey`)).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:default:apiKey")).toBeUndefined(); }); it("clearProfile removes the OAuth tokens from the keyring", async () => { await writeOAuthProfile("https://m.example.com", OAUTH); expect(await clearProfile()).toBe(true); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthAccess`)).toBeUndefined(); - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:default:oauthAccess")).toBeUndefined(); + expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBeUndefined(); expect(await readProfileCredential()).toBeNull(); }); @@ -600,7 +561,7 @@ describe("OAuth profiles (keyring backend)", () => { } // The recovered keyring still holds the pre-rotation tokens; the inline file copy is // authoritative, so the stale keyring entry must not shadow it. - expect(hoisted.store.get(`${keyringService()}:profile:default:oauthRefresh`)).toBe("refresh-1"); + expect(hoisted.store.get("metabase-cli:profile:default:oauthRefresh")).toBe("refresh-1"); expect(await readProfileCredential()).toEqual({ url: "https://m.example.com", credential: rotated, diff --git a/packages/cli/src/core/auth/storage.ts b/packages/cli/src/core/auth/storage.ts index 5b080f01..4682a3ce 100644 --- a/packages/cli/src/core/auth/storage.ts +++ b/packages/cli/src/core/auth/storage.ts @@ -19,7 +19,7 @@ import { type ProfileRecord, } from "./profile-record"; -const LEGACY_KEYRING_SERVICE = "metabase-cli"; +const KEYRING_SERVICE = "metabase-cli"; const PROFILES_FILE = "profiles.json"; const LEGACY_CREDENTIALS_FILE = "credentials.json"; const LEGACY_REJECTIONS_FILE = "rejections.json"; @@ -118,11 +118,6 @@ export function consumeKeyringDowngradeWarning(): string | null { return message; } -// Scoped by config directory so two config homes holding a same-named profile keep separate secrets. -export function keyringService(): string { - return `${LEGACY_KEYRING_SERVICE}:${configDir()}`; -} - function keyringEnabled(): boolean { return readEnv(ENV_DISABLE_KEYRING) !== "1"; } @@ -133,24 +128,24 @@ function keyringEnabled(): boolean { // tell a recoverable backend issue from a real bug, and the design choice is // graceful degradation to the file backend either way — so every throw routes // to the fallback, deliberately. -function trySetKeyring(service: string, key: CredentialAccount, value: string): boolean { +function trySetKeyring(key: CredentialAccount, value: string): boolean { if (!keyringEnabled()) { return false; } try { - new Entry(service, key).setPassword(value); + new Entry(KEYRING_SERVICE, key).setPassword(value); return true; } catch { return false; } } -function tryReadKeyring(service: string, key: CredentialAccount): string | null | undefined { +function tryReadKeyring(key: CredentialAccount): string | null | undefined { if (!keyringEnabled()) { return undefined; } try { - return new Entry(service, key).getPassword(); + return new Entry(KEYRING_SERVICE, key).getPassword(); } catch { return undefined; } @@ -160,21 +155,17 @@ function tryReadKeyring(service: string, key: CredentialAccount): string | null // existed or not); "failed" — the backend threw, so we cannot confirm the secret is gone. type KeyringRemoval = "skipped" | "removed" | "absent" | "failed"; -function removeKeyringEntryIn(service: string, key: CredentialAccount): KeyringRemoval { +function removeKeyringEntry(key: CredentialAccount): KeyringRemoval { if (!keyringEnabled()) { return "skipped"; } try { - return new Entry(service, key).deletePassword() ? "removed" : "absent"; + return new Entry(KEYRING_SERVICE, key).deletePassword() ? "removed" : "absent"; } catch { return "failed"; } } -function removeKeyringEntry(key: CredentialAccount): KeyringRemoval { - return removeKeyringEntryIn(keyringService(), key); -} - async function readProfilesFile(): Promise { const path = profilesFilePath(); let raw: string; @@ -313,8 +304,8 @@ export function keyringFallbackWarning(location: FileLocation): string { } function persistSecret(key: CredentialAccount, value: string): CredentialLocation { - if (trySetKeyring(keyringService(), key, value)) { - return { backend: "keyring", service: keyringService(), account: key }; + if (trySetKeyring(key, value)) { + return { backend: "keyring", service: KEYRING_SERVICE, account: key }; } // Falling back to the plaintext file (keyring unavailable): drop any stale keyring entry so a // recovered vault can't later shadow the file copy with an out-of-date secret. @@ -328,23 +319,7 @@ function resolveSecret(key: CredentialAccount, inline: string | null): string | if (inline !== null) { return inline; } - const scoped = tryReadKeyring(keyringService(), key); - if (scoped !== null && scoped !== undefined) { - return scoped; - } - return adoptLegacySecret(key); -} - -// Secrets stored before the service was scoped by config directory live under the bare service; -// copying one into this directory's scope keeps it readable after another directory's logout -// removes the shared entry. -function adoptLegacySecret(key: CredentialAccount): string | null { - const legacy = tryReadKeyring(LEGACY_KEYRING_SERVICE, key); - if (legacy === null || legacy === undefined) { - return null; - } - trySetKeyring(keyringService(), key, legacy); - return legacy; + return tryReadKeyring(key) ?? null; } export async function readProfileCredential( @@ -485,7 +460,7 @@ export async function writeOAuthProfile( await upsertRecord(file, name, updated); return onFile ? fileLocation(accessKey) - : { backend: "keyring", service: keyringService(), account: accessKey }; + : { backend: "keyring", service: KEYRING_SERVICE, account: accessKey }; }); } @@ -548,15 +523,11 @@ export async function clearProfile(name: string = DEFAULT_PROFILE): Promise { const file = await readProfilesFile(); const existing = findRecord(file, name); - const accounts = [ - account.profileApiKey(name), - account.profileOAuthAccess(name), - account.profileOAuthRefresh(name), + const removals = [ + removeKeyringEntry(account.profileApiKey(name)), + removeKeyringEntry(account.profileOAuthAccess(name)), + removeKeyringEntry(account.profileOAuthRefresh(name)), ]; - const removals = accounts.flatMap((key) => [ - removeKeyringEntry(key), - removeKeyringEntryIn(LEGACY_KEYRING_SERVICE, key), - ]); if (existing !== null) { flagResidualIfUnconfirmed(existing, existing.oauth !== null ? "oauth" : "apiKey", removals); } diff --git a/packages/cli/src/core/config.test.ts b/packages/cli/src/core/config.test.ts index 90f18a05..2b78553f 100644 --- a/packages/cli/src/core/config.test.ts +++ b/packages/cli/src/core/config.test.ts @@ -40,7 +40,6 @@ import type { OAuthCredential } from "@metabase/client/auth/credential"; import { ConfigError } from "@metabase/client/errors"; import { - keyringService, readProfileCredential, writeOAuthProfile, writeProbeFailure, @@ -243,7 +242,7 @@ describe("resolveConfig", () => { kind: "auth", reason: "Invalid or unauthorized API key", }); - hoisted.store.delete(`${keyringService()}:profile:lost:apiKey`); + hoisted.store.delete("metabase-cli:profile:lost:apiKey"); const error = await resolveConfig({ profile: "lost" }).catch((thrown: unknown) => thrown); expect(error).toBeInstanceOf(ConfigError); @@ -269,7 +268,7 @@ describe("resolveConfig", () => { tokenFeatures: null, }, }); - hoisted.store.delete(`${keyringService()}:profile:recovers:apiKey`); + hoisted.store.delete("metabase-cli:profile:recovers:apiKey"); const error = await resolveConfig({ profile: "recovers" }).catch((thrown: unknown) => thrown); expect(error).toBeInstanceOf(ConfigError); From 95ad77bfa471367fbf687401f17f408fba8180e5 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 13:26:46 -0400 Subject: [PATCH 5/7] Accept setting keys ending in a question mark Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/commands/setting/key.test.ts | 4 ++++ packages/cli/src/commands/setting/key.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/setting/key.test.ts b/packages/cli/src/commands/setting/key.test.ts index 81ce92f9..63d686a8 100644 --- a/packages/cli/src/commands/setting/key.test.ts +++ b/packages/cli/src/commands/setting/key.test.ts @@ -18,6 +18,10 @@ describe("parseSettingKey", () => { expect(parseSettingKey(" remote-sync-branch ")).toBe("remote-sync-branch"); }); + it("accepts a key ending in a question mark, as Metabase's boolean status settings do", () => { + expect(parseSettingKey("email-configured?")).toBe("email-configured?"); + }); + it("rejects a key that is not a kebab-case identifier", () => { const error = thrownBy(() => parseSettingKey("not a key!")); expect(error).toBeInstanceOf(ConfigError); diff --git a/packages/cli/src/commands/setting/key.ts b/packages/cli/src/commands/setting/key.ts index 1d0164e9..f22d3e4d 100644 --- a/packages/cli/src/commands/setting/key.ts +++ b/packages/cli/src/commands/setting/key.ts @@ -1,6 +1,6 @@ import { ConfigError } from "@metabase/client/errors"; -const SETTING_KEY_PATTERN = /^[A-Za-z][A-Za-z0-9_-]*$/; +const SETTING_KEY_PATTERN = /^[A-Za-z][A-Za-z0-9_-]*\??$/; export function parseSettingKey(value: string): string { const trimmed = value.trim(); From d92c75a92f1e8036c5987a828d1e242e1f624422 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 14:32:13 -0400 Subject: [PATCH 6/7] Take true or false in db set-data-actions, like transform-job set-active Co-Authored-By: Claude Opus 5.5 --- README.md | 10 +++++----- packages/cli/skill-data/data-action/SKILL.md | 2 +- packages/cli/src/commands/argv.test.ts | 2 +- .../cli/src/commands/data-action/create.ts | 2 +- .../cli/src/commands/db/set-data-actions.ts | 15 +++++++-------- tests/e2e/db.e2e.test.ts | 18 ++++++++++-------- 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 91b5f12d..1dadbf64 100644 --- a/README.md +++ b/README.md @@ -540,13 +540,13 @@ mb db rescan-values 1 mb db rescan-values 1 --json ``` -### `mb db set-data-actions ` +### `mb db set-data-actions ` -Turn data actions on or off for a database (`PUT /api/database/:id` with `settings: {"database-enable-actions": …}`), the Data actions toggle of Admin → Databases. While it is off, data actions on the database can be neither created nor run. Needs an admin, and a driver that lists `actions` in its `features`. Returns the database with its `settings`; a state other than `on` or `off` is refused before the request (exit 2). +Enable or disable data actions for a database (`PUT /api/database/:id` with `settings: {"database-enable-actions": …}`), the Data actions toggle of Admin → Databases. While it is off, data actions on the database can be neither created nor run. Needs an admin, and a driver that lists `actions` in its `features`. Returns the database with its `settings`; a value other than `true` or `false` is refused before the request (exit 2). ```sh -mb db set-data-actions 2 on -mb db set-data-actions 2 off --json +mb db set-data-actions 2 true +mb db set-data-actions 2 false --json ``` ## Tables @@ -1149,7 +1149,7 @@ mb snippet archive 1 --json ## Data actions -CRUD on `/api/action` plus `execute`. A data action is a parameterized native SQL write (`INSERT`, `UPDATE`, `DELETE`) filed in a collection. Data actions are off by default: an admin must enable them on the target database first (`mb db set-data-actions on`). `mb data-action create` authors data actions only — no `model_id`, no implicit data actions — and needs a server whose data actions do not require a model. +CRUD on `/api/action` plus `execute`. A data action is a parameterized native SQL write (`INSERT`, `UPDATE`, `DELETE`) filed in a collection. Data actions are off by default: an admin must enable them on the target database first (`mb db set-data-actions true`). `mb data-action create` authors data actions only — no `model_id`, no implicit data actions — and needs a server whose data actions do not require a model. ### `mb data-action list` diff --git a/packages/cli/skill-data/data-action/SKILL.md b/packages/cli/skill-data/data-action/SKILL.md index 3b3c45ec..fb124b36 100644 --- a/packages/cli/skill-data/data-action/SKILL.md +++ b/packages/cli/skill-data/data-action/SKILL.md @@ -20,7 +20,7 @@ mb db get --full --json ``` - `features` must contain `"actions"` — otherwise the driver cannot run writes; pick another database. -- `settings["database-enable-actions"]` must be `true`. Create and execute fail with `Actions are not enabled.` until then. An admin profile turns it on with `mb db set-data-actions on`; anyone else asks an admin to. Turning it on lets anyone with access to the database's actions write to it, so confirm with the user first. +- `settings["database-enable-actions"]` must be `true`. Create and execute fail with `Actions are not enabled.` until then. An admin profile turns it on with `mb db set-data-actions true`; anyone else asks an admin to. Turning it on lets anyone with access to the database's actions write to it, so confirm with the user first. ## Pick the collection diff --git a/packages/cli/src/commands/argv.test.ts b/packages/cli/src/commands/argv.test.ts index 0ceb7275..00b81ae7 100644 --- a/packages/cli/src/commands/argv.test.ts +++ b/packages/cli/src/commands/argv.test.ts @@ -89,7 +89,7 @@ const TWO_POSITIONALS: ArgsDef = { describe("assertRequiredPositionals", () => { it("accepts argv that gives every required positional", () => { - expect(() => assertRequiredPositionals(["1", "on", "--json"], TWO_POSITIONALS)).not.toThrow(); + expect(() => assertRequiredPositionals(["1", "true", "--json"], TWO_POSITIONALS)).not.toThrow(); }); it("names the missing positional and the command's positionals", () => { diff --git a/packages/cli/src/commands/data-action/create.ts b/packages/cli/src/commands/data-action/create.ts index 48ed039e..ca8a289d 100644 --- a/packages/cli/src/commands/data-action/create.ts +++ b/packages/cli/src/commands/data-action/create.ts @@ -14,7 +14,7 @@ import { export default defineMetabaseCommand({ meta: { name: "create", description: "Create a data action from JSON" }, details: - 'A data action is a parameterized native query that writes to a database, filed in a collection. The JSON body needs `name`, `type: "query"`, `database_id`, a native `dataset_query` whose `{{tag}}` placeholders are its inputs, and `parameters` targeting those tags; `collection_id` picks a collection in the `data-actions` or `data-apps` namespace (omit for the data actions root; a regular collection is refused). Data actions must be enabled on the database (`mb db set-data-actions on`). An MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.', + 'A data action is a parameterized native query that writes to a database, filed in a collection. The JSON body needs `name`, `type: "query"`, `database_id`, a native `dataset_query` whose `{{tag}}` placeholders are its inputs, and `parameters` targeting those tags; `collection_id` picks a collection in the `data-actions` or `data-apps` namespace (omit for the data actions root; a regular collection is refused). Data actions must be enabled on the database (`mb db set-data-actions true`). An MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.', skills: [ { skill: "data-action", purpose: "author a data action and its parameters" }, { skill: "native-sql", purpose: "the native query and its template tags" }, diff --git a/packages/cli/src/commands/db/set-data-actions.ts b/packages/cli/src/commands/db/set-data-actions.ts index 6452159b..5568f333 100644 --- a/packages/cli/src/commands/db/set-data-actions.ts +++ b/packages/cli/src/commands/db/set-data-actions.ts @@ -9,12 +9,12 @@ import { parseEnumFlag } from "../parse-enum"; import { parseId } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; -const DataActionsState = z.enum(["on", "off"]); +const EnabledWord = z.enum(["true", "false"]); export default defineMetabaseCommand({ meta: { name: "set-data-actions", - description: "Turn data actions on or off for a database", + description: "Enable or disable data actions for a database", }, details: "The Data actions toggle of Admin → Databases (the `database-enable-actions` database setting): while it is off, data actions on the database can be neither created nor run. Needs an admin. The driver must support writes: `mb db get --full --json` lists `actions` in `features`. Answers the database with its `settings`.", @@ -25,26 +25,25 @@ export default defineMetabaseCommand({ ...profileFlag, ...connectionFlags, id: { type: "positional", description: "Database id", required: true }, - state: { + enabled: { type: "positional", - description: `Data actions state (${DataActionsState.options.join(" | ")})`, + description: "true to enable data actions, false to disable", required: true, }, }, outputSchema: DatabaseWithSettings, - examples: ["mb db set-data-actions 2 on", "mb db set-data-actions 2 off --json"], + examples: ["mb db set-data-actions 2 true", "mb db set-data-actions 2 false --json"], async run({ args, ctx, getClient }) { const id = parseId(args.id); - const enabled = parseEnumFlag(args.state, DataActionsState, "state") === "on"; + const enabled = parseEnumFlag(args.enabled, EnabledWord, "enabled") === "true"; const client = await getClient(); const database = await client.database.update(id, { settings: { "database-enable-actions": enabled }, }); - const state = enabled ? "on" : "off"; renderSummary( database, databaseDataActionsView, - `Turned data actions ${state} for database ${database.id} "${database.name}".`, + `${enabled ? "Enabled" : "Disabled"} data actions for database ${database.id} "${database.name}".`, ctx, ); }, diff --git a/tests/e2e/db.e2e.test.ts b/tests/e2e/db.e2e.test.ts index 32b7fde6..b0701377 100644 --- a/tests/e2e/db.e2e.test.ts +++ b/tests/e2e/db.e2e.test.ts @@ -473,10 +473,10 @@ describe("db e2e", () => { expect(cliErrorMessage(result.stderr)).toContain('invalid id: "abc" (expected integer)'); }); - it("set-data-actions turns the database setting on and off, and get reads it back", async () => { + it("set-data-actions enables and disables the database setting, and get reads it back", async () => { const id = String(SEEDED.warehouseDbId); const on = await runCli({ - args: ["db", "set-data-actions", id, "on", "--json"], + args: ["db", "set-data-actions", id, "true", "--json"], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); @@ -489,7 +489,7 @@ describe("db e2e", () => { }); const off = await runCli({ - args: ["db", "set-data-actions", id, "off", "--json"], + args: ["db", "set-data-actions", id, "false", "--json"], configHome: await makeIsolatedConfigHome(), env: authEnv(), }); @@ -509,7 +509,7 @@ describe("db e2e", () => { ); }); - it("set-data-actions refuses a state other than on or off before any request", async () => { + it("set-data-actions refuses a value other than true or false before any request", async () => { const result = await runCli({ args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "yes", "--json"], configHome: await makeIsolatedConfigHome(), @@ -517,10 +517,12 @@ describe("db e2e", () => { }); expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toBe('invalid state: "yes" (expected one of: on, off)'); + expect(cliErrorMessage(result.stderr)).toBe( + 'invalid enabled: "yes" (expected one of: true, false)', + ); }); - it("set-data-actions with a flag where the state belongs refuses the flag instead of printing help", async () => { + it("set-data-actions with a flag where the value belongs refuses the flag instead of printing help", async () => { const result = await runCli({ args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "--on", "--json"], configHome: await makeIsolatedConfigHome(), @@ -532,7 +534,7 @@ describe("db e2e", () => { expect(cliErrorMessage(result.stderr)).toBe("unknown flag: --on"); }); - it("set-data-actions without the state names the missing argument", async () => { + it("set-data-actions without the value names the missing argument", async () => { const result = await runCli({ args: ["db", "set-data-actions", String(SEEDED.warehouseDbId), "--json"], configHome: await makeIsolatedConfigHome(), @@ -542,7 +544,7 @@ describe("db e2e", () => { expect(result.exitCode).toBe(2); expect(result.stdout).toBe(""); expect(cliErrorMessage(result.stderr)).toBe( - "missing argument: (this command takes )", + "missing argument: (this command takes )", ); }); }); From 68af2801c532d8549df8b975a8287b90db789d4d Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Sat, 10 Oct 2026 15:10:30 -0400 Subject: [PATCH 7/7] Bump the CLI to 0.5.2 Co-Authored-By: Claude Opus 5.5 --- packages/cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 53629262..a2c0c4d1 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@metabase/cli", - "version": "0.5.1", + "version": "0.5.2", "description": "Metabase CLI", "license": "AGPL-3.0", "repository": {