Skip to content

Commit 88b530e

Browse files
committed
fixes
1 parent b5944c7 commit 88b530e

7 files changed

Lines changed: 126 additions & 20 deletions

File tree

CLAUDE.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ Adding a new field to `.bootstrap.json`:
126126

127127
- Update the `Bootstrap` schema in `tests/e2e/bootstrap-data.ts`. The writer in `setup/bootstrap.ts` consumes the same schema for its parameter type — drift is mechanically prevented.
128128

129+
Running e2e — the suite is slow (~3–5 minutes for a full run, ~hundreds of ms to seconds per test):
130+
131+
- **Plan for failures on the first run.** Capture the full output end-to-end the first time so every failure is in hand before deciding next steps. Pipe to a file when the buffer is unreliable: `bun run test:e2e 2>&1 | tee /tmp/e2e.log`. Vitest's per-file failure summary at the end is the authoritative list — read it before rerunning. Rerunning the full suite "to see what failed" is a 3-minute round-trip; don't do it.
132+
- **Iterate on one file.** During development, scope to the file under change: `bun run test:e2e tests/e2e/<noun>.e2e.test.ts` (seconds, not minutes). Run the full suite only as the closing pre-merge check, not during back-and-forth.
133+
- **The stack persists.** `e2e:up` is idempotent and the bootstrap reuses `.bootstrap.json` when the stored key still authenticates, so iteration cost is dominated by test execution, not setup. Don't `e2e:down` between iterations.
134+
- **Bail-by-default is off.** Vitest runs all e2e files even after a failure, on purpose — the full failure inventory is more valuable than fast-fail when each rerun costs minutes. Don't pass `--bail=1` unless you genuinely want to abort early; you'll lose signal you'd have to rerun to recover.
135+
129136
## Don't
130137

131138
- Add a color library or any colored output.

README.md

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,18 +1202,18 @@ By default `start` returns once the bundle has been consumed by the child (`stat
12021202

12031203
When `--repo <host-path>` is passed, the CLI bind-mounts the host directory at `/mnt/repo` inside the container and injects three settings into the workspace's `config.yml` so the child boots already wired to the repo: `remote-sync-url=file:///mnt/repo`, `remote-sync-branch=<branch>` (defaults to the current branch of the host repo, read via `git -C <path> symbolic-ref --short HEAD`; override with `--repo-branch`), and `remote-sync-type=<mode>` (defaults to `read-write`; override with `--repo-mode read-only`, which also makes the bind mount read-only). The bind mount is set at container-create time only — to add or change it after the fact, run `start --force` again with the new flags. The host path must be an existing directory; the CLI does not create or `git init` it for you.
12041204

1205-
| Flag | Description |
1206-
| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------- |
1207-
| `--port <n>` | Host port (default: 3000; auto-shifts up to 100 ports if taken). |
1208-
| `--image <ref>` | Docker image (default: `metabase/metabase-dev:feature-workspaces-v2`). |
1209-
| `--wait` | Block until `/api/health` is ready. Default: return as soon as consumed. |
1205+
| Flag | Description |
1206+
| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
1207+
| `--port <n>` | Host port (default: 3000; auto-shifts up to 100 ports if taken). |
1208+
| `--image <ref>` | Docker image (default: `metabase/metabase-dev:feature-workspaces-v2`). |
1209+
| `--wait` | Block until `/api/health` is ready. Default: return as soon as consumed. |
12101210
| `--timeout <ms>` | Per-phase readiness deadline (default: 240000). Covers post-create config consumption, (with `--wait`) the `/api/health` probe, and (with `--metadata`) the metadata-import status poll. |
1211-
| `--no-pull` | Skip `docker pull` (useful if the image is already present). |
1212-
| `--no-metadata` | Skip the warehouse metadata export. |
1213-
| `--force` | If a container for this workspace already exists, remove it before starting. |
1214-
| `--repo <host-path>` | Bind-mount a host directory at `/mnt/repo` and set `remote-sync-url=file:///mnt/repo` in `config.yml`. |
1215-
| `--repo-branch <name>` | `remote-sync-branch` value (default: current branch of the host repo). |
1216-
| `--repo-mode <mode>` | `remote-sync-type`: `read-write` (default) or `read-only`. Read-only also makes the bind mount read-only. |
1211+
| `--no-pull` | Skip `docker pull` (useful if the image is already present). |
1212+
| `--no-metadata` | Skip the warehouse metadata export. |
1213+
| `--force` | If a container for this workspace already exists, remove it before starting. |
1214+
| `--repo <host-path>` | Bind-mount a host directory at `/mnt/repo` and set `remote-sync-url=file:///mnt/repo` in `config.yml`. |
1215+
| `--repo-branch <name>` | `remote-sync-branch` value (default: current branch of the host repo). |
1216+
| `--repo-mode <mode>` | `remote-sync-type`: `read-write` (default) or `read-only`. Read-only also makes the bind mount read-only. |
12171217

12181218
### `metabase workspace stop <id>`
12191219

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@metabase/cli",
3-
"version": "0.1.1",
3+
"version": "0.1.2",
44
"description": "Metabase CLI",
55
"license": "AGPL-3.0",
66
"repository": {

src/commands/auth/login.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { z } from "zod";
22

33
import { clearRejection, recordRejection } from "../../core/auth/rejection";
4-
import { writeProfile } from "../../core/auth/storage";
4+
import { DEFAULT_PROFILE, writeProfile } from "../../core/auth/storage";
55
import { verifyCredentials } from "../../core/auth/verify";
6-
import { readEnvCredentials, resolveProfileName } from "../../core/config";
6+
import { explicitProfileName, readEnvCredentials } from "../../core/config";
77
import { ConfigError, errorMessage } from "../../core/errors";
88
import { normalizeUrl } from "../../core/url";
99
import type { ResourceView } from "../../domain/view";
@@ -26,9 +26,13 @@ const loginView: ResourceView<LoginResultJson> = {
2626
compactPick: LoginResult,
2727
tableColumns: [
2828
{ key: "profile", label: "Profile" },
29-
{ key: "url", label: "URL" },
30-
{ key: "authenticated", label: "Authenticated" },
31-
{ key: "email", label: "Email" },
29+
{ key: "url", label: "Metabase URL" },
30+
{
31+
key: "authenticated",
32+
label: "Status",
33+
format: (value) => (value === true ? "credentials verified" : "saved without verification"),
34+
},
35+
{ key: "email", label: "Logged in as" },
3236
],
3337
};
3438

@@ -51,7 +55,7 @@ export default defineMetabaseCommand({
5155
"metabase auth login --profile staging --url https://staging.example.com",
5256
],
5357
async run({ args, ctx }) {
54-
const profileName = resolveProfileName(args.profile);
58+
const profileName = await resolveLoginProfile(args.profile);
5559
const env = readEnvCredentials();
5660

5761
if (args.apiKey) {
@@ -87,6 +91,22 @@ export default defineMetabaseCommand({
8791
},
8892
});
8993

94+
async function resolveLoginProfile(flagProfile: string | undefined): Promise<string> {
95+
const explicit = explicitProfileName(flagProfile);
96+
if (explicit !== null) {
97+
return explicit;
98+
}
99+
if (!process.stdin.isTTY) {
100+
return DEFAULT_PROFILE;
101+
}
102+
return promptText({
103+
message: "Profile name",
104+
placeholder: DEFAULT_PROFILE,
105+
initialValue: DEFAULT_PROFILE,
106+
validate: (input) => (input ? undefined : "Profile name is required"),
107+
});
108+
}
109+
90110
async function resolveUrl(flagUrl: string | undefined, envUrl: string | null): Promise<string> {
91111
if (flagUrl) {
92112
return normalizeUrl(flagUrl);

src/core/config.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ vi.mock("@napi-rs/keyring", async () => {
1313
import { recordRejection } from "./auth/rejection";
1414
import { writeProfile } from "./auth/storage";
1515
import { setupTempConfigHome, type TempConfigHome } from "./auth/temp-config-home";
16-
import { resolveConfig, resolveProfileName } from "./config";
16+
import { explicitProfileName, resolveConfig, resolveProfileName } from "./config";
1717
import { ConfigError } from "./errors";
1818

1919
describe("resolveConfig", () => {
@@ -207,3 +207,29 @@ describe("resolveProfileName", () => {
207207
expect(resolveProfileName(undefined)).toBe("default");
208208
});
209209
});
210+
211+
describe("explicitProfileName", () => {
212+
const originalEnv = { ...process.env };
213+
214+
beforeEach(() => {
215+
delete process.env["METABASE_PROFILE"];
216+
});
217+
218+
afterEach(() => {
219+
process.env = { ...originalEnv };
220+
});
221+
222+
it("returns the flag value when provided", () => {
223+
process.env["METABASE_PROFILE"] = "env-profile";
224+
expect(explicitProfileName("flag-profile")).toBe("flag-profile");
225+
});
226+
227+
it("returns METABASE_PROFILE when no flag", () => {
228+
process.env["METABASE_PROFILE"] = "env-profile";
229+
expect(explicitProfileName(undefined)).toBe("env-profile");
230+
});
231+
232+
it("returns null when neither flag nor env is set", () => {
233+
expect(explicitProfileName(undefined)).toBeNull();
234+
});
235+
});

src/core/config.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ interface FieldResolution {
3838
}
3939

4040
export function resolveProfileName(profileFlag: string | undefined): string {
41-
return profileFlag || process.env[ENV_PROFILE] || DEFAULT_PROFILE;
41+
return explicitProfileName(profileFlag) ?? DEFAULT_PROFILE;
42+
}
43+
44+
export function explicitProfileName(profileFlag: string | undefined): string | null {
45+
return profileFlag || process.env[ENV_PROFILE] || null;
4246
}
4347

4448
export function readEnvCredentials(): EnvCredentials {

tests/e2e/auth.e2e.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,55 @@ describe("auth e2e", () => {
217217
});
218218
});
219219

220+
it("login routes through METABASE_PROFILE when no --profile flag is passed", async () => {
221+
const configHome = await makeIsolatedConfigHome();
222+
223+
const login = await runCli({
224+
args: [
225+
"auth",
226+
"login",
227+
"--url",
228+
bootstrap.baseUrl,
229+
"--api-key",
230+
bootstrap.adminApiKey,
231+
"--json",
232+
],
233+
configHome,
234+
env: { METABASE_PROFILE: "env_routed" },
235+
});
236+
237+
expect(login.exitCode, login.stderr).toBe(0);
238+
expect(parseJson(login.stdout, LoginResult)).toEqual({
239+
profile: "env_routed",
240+
url: bootstrap.baseUrl,
241+
authenticated: true,
242+
email: bootstrap.adminApiKeyEmail,
243+
});
244+
245+
const defaultStatus = await runCli({
246+
args: ["auth", "status", "--json"],
247+
configHome,
248+
});
249+
expect(defaultStatus.exitCode, defaultStatus.stderr).toBe(0);
250+
expect(parseJson(defaultStatus.stdout, AuthStatus)).toEqual({
251+
profile: "default",
252+
present: false,
253+
url: null,
254+
});
255+
256+
const envStatus = await runCli({
257+
args: ["auth", "status", "--json"],
258+
configHome,
259+
env: { METABASE_PROFILE: "env_routed" },
260+
});
261+
expect(envStatus.exitCode, envStatus.stderr).toBe(0);
262+
expect(parseJson(envStatus.stdout, AuthStatus)).toEqual({
263+
profile: "env_routed",
264+
present: true,
265+
url: bootstrap.baseUrl,
266+
});
267+
});
268+
220269
it("logout proceeds without --yes when stdin is not a TTY (non-interactive auto-confirm)", async () => {
221270
const configHome = await makeIsolatedConfigHome();
222271

0 commit comments

Comments
 (0)