diff --git a/.changeset/hand-written-messages-stderr.md b/.changeset/hand-written-messages-stderr.md new file mode 100644 index 000000000..ea84a4c81 --- /dev/null +++ b/.changeset/hand-written-messages-stderr.md @@ -0,0 +1,16 @@ +--- +"cf": minor +--- + +Write hand-written command messages to stderr + +Breaking change during the beta: `cf auth login`, `cf auth logout`, the auth +profile commands, `cf cli telemetry enable` / `disable` and `cf migrate` now +print their notices, warnings and success messages on stderr, so redirected or +piped stdout holds only command output, as with generated commands. The auth +and telemetry commands use clack's log lines and the `✓` success line, which, +as with generated commands, appears only when stderr is a terminal. A command +to run next is printed on its own line so it copies cleanly. `cf auth login` +with `CLOUDFLARE_API_TOKEN` set now warns once that the token takes +precedence, and `cf auth logout` no longer reports removing OAuth tokens that +it keeps while `CLOUDFLARE_API_TOKEN` is set. diff --git a/AGENTS.md b/AGENTS.md index 850c5afb9..aed4fe580 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -248,7 +248,7 @@ the complete generated and hand-written `_meta/commands.json` catalogue (see | `request-headers.ts` | User-Agent plus `X-CF-CLI-Mode` / detected-agent headers; authentication is owned by the API client/raw-fetch/auth layers | | `dry-run.ts` | `--dry-run` body-preview formatting (`bodyKind: 'json' \| 'multipart' \| 'octet-stream' \| 'none'`) | | `errors.ts` | `handleError`; APIError box; status-level hints (no per-code knowledge in `src/`) | -| `output.ts` | `formatOutput` (silent on null; `✓ ` on TTY stderr); `--quiet` knob | +| `output.ts` | `formatOutput` (silent on null; `✓ ` on TTY stderr via `printSuccess`); `--quiet` knob | | `progress.ts` | `withProgress(label, task)` clack braille/timer spinner; active only with color + stdout TTY and no `CF_QUIET=1` | | `prompt.ts` | required text/enum/acknowledgement prompts; sensitivity is explicit OpenAPI/forge metadata, not a name regex; secret-only stdin ingestion; destructive confirmation | | `session.ts` | `openSession(version)` — idempotent once-per-process banner emission at interactive invocation startup; prompt helpers call it defensively | @@ -269,7 +269,8 @@ the complete generated and hand-written `_meta/commands.json` catalogue (see | `ui/blocks.ts` | `errorBlock(...)` left-gutter / clack-style block rendering (replaced the old boxen-based `ui/box.ts`); `wrapPlain` per-line wrap | | `ui/banner.ts` | slim `renderPromptIntro(version)` headline and `formatCommand`; the old gradient/ASCII splash is removed | | `ui/theme.ts` | background-neutral semantic colour palette and Chalk wrapper | -| `ui/format.ts` | one-line `success`, `warning`, `info`, and `hint` helpers | +| `ui/format.ts` | one-line `success`, `warning`, and `hint` helpers | +| `ui/log.ts` | stderr messages: clack `log.info` / `log.warn` and `printCommands` (copyable next steps); not re-exported from `ui/index.ts` | | `input-validation.ts` | yargs-time arg coercion + `resolveFileToken(value, fieldName, format)` for `@` ingestion; runtime body/path validation | | `resolve.ts` | URL templating (path-param substitution), `isId`, `isValidDomainName` | @@ -651,6 +652,12 @@ These have stabilised this repo's voice; preserve them. - **Secret prompt:** clack.password (masks input). - **Success line:** `✓ ` on stderr; null/undefined API results suppress stdout output entirely. +- **Messages:** stdout carries only command output; every message goes to + stderr. Use `lib/ui/log.ts` for notices and warnings and `printSuccess` + for success lines. A next step or fix goes on its own line + (`printCommands`); other commands in a message are styled as code. + Prompts, spinners and the init, build and deploy flows still write to + stdout until they move. - **Splash screen:** bare `cf` prints the headline, a short getting-started list, and the docs link. Yargs owns root, group, and command help rendering. ## Global Flags diff --git a/packages/cli/src/__tests__/commands/auth/device-login.test.ts b/packages/cli/src/__tests__/commands/auth/device-login.test.ts index e015cbec8..dbfc51401 100644 --- a/packages/cli/src/__tests__/commands/auth/device-login.test.ts +++ b/packages/cli/src/__tests__/commands/auth/device-login.test.ts @@ -1,14 +1,14 @@ import { writeFile } from "node:fs/promises"; -import { - mockConsoleMethods, - runInTempDir, -} from "@cloudflare/workers-utils/test-helpers"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { mockOutputStreams } from "../../helpers/capture-output.js"; import { runCf } from "../../helpers/run-cf.js"; const sharedAuth = vi.hoisted(() => ({ activeProfile: "default", - credentials: undefined as { scopes: string[] } | undefined, + credentials: undefined as + | { oauth_token?: string; scopes: string[] } + | undefined, login: vi.fn(), })); @@ -25,6 +25,8 @@ vi.mock("@cloudflare/workers-auth/cf", async (importOriginal) => ({ getActiveProfile: () => sharedAuth.activeProfile, getCredentialStore: () => ({ path: () => "/test/auth.json" }), readAuthCredentials: () => sharedAuth.credentials, + // Stored tokens never refresh, so they read as invalid or expired. + getOAuthTokenFromLocalState: async () => undefined, login: sharedAuth.login, }), })); @@ -40,7 +42,7 @@ vi.mock("../../../lib/telemetry/dispatcher.js", () => ({ describe("cf auth device login", () => { runInTempDir(); - const std = mockConsoleMethods(); + const std = mockOutputStreams(); beforeEach(() => { sharedAuth.activeProfile = "default"; @@ -99,15 +101,14 @@ describe("cf auth device login", () => { it("loads file-sourced Cloudflare variables for login", async () => { await writeFile(".env", "CLOUDFLARE_API_TOKEN=file-token"); - let observedToken: string | undefined; - sharedAuth.login.mockImplementationOnce(async () => { - observedToken = process.env.CLOUDFLARE_API_TOKEN; - return false; - }); await runCf(["auth", "login", "--no-browser"]); - expect(observedToken).toBe("file-token"); + expect(sharedAuth.login).not.toHaveBeenCalled(); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"▲ CLOUDFLARE_API_TOKEN is set and takes precedence. Unset it to log in via OAuth."` + ); expect(process.env.CLOUDFLARE_API_TOKEN).toBeUndefined(); }); @@ -158,6 +159,18 @@ describe("cf auth device login", () => { ); }); + it("confirms a new named profile and shows how to activate it", async () => { + std.setStderrTTY(true); + + await runCf(["auth", "create", "work", "--no-browser"]); + + expect(std.out).toBe(""); + expect(std.err).toContain('✓ Profile "work" created.'); + expect(std.err).toContain( + "● To use this profile in a directory, run:\n\n cf auth activate work" + ); + }); + it("can use the localhost callback flow for named profiles", async () => { await runCf([ "auth", @@ -213,8 +226,9 @@ describe("cf auth device login", () => { await runCf(["auth", "login", "--no-browser"]); - expect(std.out).toContain("Scopes: one, two, three, four, five"); - expect(std.out).not.toContain("cf auth whoami"); + expect(std.out).toBe(""); + expect(std.err).toContain("Scopes: one, two, three, four, five"); + expect(std.err).not.toContain("cf auth whoami"); }); it("summarizes more than five selected scopes after login", async () => { @@ -224,8 +238,41 @@ describe("cf auth device login", () => { await runCf(["auth", "login", "--no-browser"]); - expect(std.out).toContain("Scopes: 6 selected"); - expect(std.out).toContain("Run cf auth whoami to see the full list."); - expect(std.out).not.toContain("one, two, three, four, five, six"); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "● Starting OAuth login flow... + ● Config file: /test/auth.json + │ Scopes: 6 selected. Run cf auth whoami to see the full list." + `); + }); + + it("starts a fresh login when the stored tokens are invalid", async () => { + sharedAuth.credentials = { oauth_token: "expired", scopes: ["one"] }; + + await runCf(["auth", "login", "--no-browser"]); + + expect(sharedAuth.login).toHaveBeenCalledOnce(); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "▲ Existing tokens are invalid or expired. Starting fresh login... + ● Starting OAuth login flow... + ● Config file: /test/auth.json + │ Scopes: one" + `); + }); + + it("re-authenticates with --force when already logged in", async () => { + sharedAuth.credentials = { oauth_token: "token", scopes: ["one"] }; + + await runCf(["auth", "login", "--force", "--no-browser"]); + + expect(sharedAuth.login).toHaveBeenCalledOnce(); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "● Re-authenticating... + ● Starting OAuth login flow... + ● Config file: /test/auth.json + │ Scopes: one" + `); }); }); diff --git a/packages/cli/src/__tests__/commands/auth/keyring-login.test.ts b/packages/cli/src/__tests__/commands/auth/keyring-login.test.ts index a15c54264..cfe97909c 100644 --- a/packages/cli/src/__tests__/commands/auth/keyring-login.test.ts +++ b/packages/cli/src/__tests__/commands/auth/keyring-login.test.ts @@ -2,11 +2,9 @@ import { readUserPreferences, updateUserPreferences, } from "@cloudflare/workers-auth/cf"; -import { - mockConsoleMethods, - runInTempDir, -} from "@cloudflare/workers-utils/test-helpers"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { mockOutputStreams } from "../../helpers/capture-output.js"; import { runCf } from "../../helpers/run-cf.js"; const sharedAuth = vi.hoisted(() => ({ @@ -46,7 +44,7 @@ vi.mock("../../../lib/telemetry/dispatcher.js", () => ({ describe("cf auth login keyring preference", () => { runInTempDir(); - const std = mockConsoleMethods(); + const std = mockOutputStreams(); const credentials = { oauth_token: "test-token" }; const encryptedStore = { kind: "encrypted-file", path: () => "auth.enc" }; @@ -80,7 +78,8 @@ describe("cf auth login keyring preference", () => { expect(readUserPreferences().keyring_enabled).toBe(true); expect(sharedAuth.login).not.toHaveBeenCalled(); - expect(std.out).toContain("You are already logged in."); + expect(std.out).toBe(""); + expect(std.err).toContain("You are already logged in."); } ); diff --git a/packages/cli/src/__tests__/commands/auth/logout.test.ts b/packages/cli/src/__tests__/commands/auth/logout.test.ts new file mode 100644 index 000000000..ee10957d7 --- /dev/null +++ b/packages/cli/src/__tests__/commands/auth/logout.test.ts @@ -0,0 +1,78 @@ +import { + readAuthConfigFile, + writeAuthConfigFile, +} from "@cloudflare/workers-auth/cf"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; +import { http, HttpResponse } from "msw/http"; +import { beforeEach, describe, expect, it } from "vite-plus/test"; +import { mockOutputStreams } from "../../helpers/capture-output.js"; +import { server, setupMsw } from "../../helpers/msw.js"; +import { runCf } from "../../helpers/run-cf.js"; + +function createDefaultProfile(): void { + writeAuthConfigFile( + { oauth_token: "token", expiration_time: "2999-01-01T00:00:00.000Z" }, + "default" + ); +} + +describe("cf auth logout", () => { + runInTempDir(); + setupMsw(); + const std = mockOutputStreams(); + + beforeEach(() => { + server.use( + http.post( + "https://dash.cloudflare.com/oauth2/revoke", + () => new HttpResponse("") + ) + ); + }); + + it("explains how to log in when not logged in", async () => { + await runCf(["auth", "logout"], { CLOUDFLARE_API_TOKEN: undefined }); + + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "● You are not currently logged in. To authenticate, run: + + cf auth login" + `); + }); + + it("points at the environment token when only that is set", async () => { + await runCf(["auth", "logout"], { CLOUDFLARE_API_TOKEN: "env-token" }); + + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"● No cf OAuth credentials found, but CLOUDFLARE_API_TOKEN is set. Unset it to log out."` + ); + }); + + it("confirms which file the OAuth tokens were removed from", async () => { + createDefaultProfile(); + std.setStderrTTY(true); + + await runCf(["auth", "logout"], { CLOUDFLARE_API_TOKEN: undefined }); + + expect(std.out).toBe(""); + expect(std.err).toContain( + "✓ Removed OAuth tokens from /home/.config/cloudflare/config/default.json" + ); + }); + + it("keeps the OAuth tokens while the environment token is set", async () => { + createDefaultProfile(); + std.setStderrTTY(true); + + await runCf(["auth", "logout"], { CLOUDFLARE_API_TOKEN: "env-token" }); + + expect(readAuthConfigFile("default")?.oauth_token).toBe("token"); + expect(std.out).toBe(""); + expect(std.err).toContain( + "Unset the CLOUDFLARE_API_TOKEN in the environment to log out." + ); + expect(std.err).not.toContain("✓"); + }); +}); diff --git a/packages/cli/src/__tests__/commands/auth/profiles.test.ts b/packages/cli/src/__tests__/commands/auth/profiles.test.ts index 0e7c8b97c..3bce6dd62 100644 --- a/packages/cli/src/__tests__/commands/auth/profiles.test.ts +++ b/packages/cli/src/__tests__/commands/auth/profiles.test.ts @@ -3,19 +3,13 @@ import { createCfProfileStore, writeAuthConfigFile, } from "@cloudflare/workers-auth/cf"; -import { - mockConsoleMethods, - runInTempDir, -} from "@cloudflare/workers-utils/test-helpers"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; import { http, HttpResponse } from "msw/http"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { mockOutputStreams } from "../../helpers/capture-output.js"; import { server, setupMsw, TEST_BASE_URL } from "../../helpers/msw.js"; import { runCf } from "../../helpers/run-cf.js"; -function normalizeProfileOutput(output: string): string { - return output.replace(/^(i|ℹ) /gm, "ℹ ").replace(/^(‼|⚠) /gm, "⚠ "); -} - function createProfile(name: string, token = `${name}-token`): void { writeAuthConfigFile( { oauth_token: token, expiration_time: "2999-01-01T00:00:00.000Z" }, @@ -26,7 +20,7 @@ function createProfile(name: string, token = `${name}-token`): void { describe("cf auth profiles", () => { runInTempDir(); setupMsw(); - const std = mockConsoleMethods(); + const std = mockOutputStreams(); const store = createCfProfileStore({ logger: { debug: () => {}, @@ -50,11 +44,13 @@ describe("cf auth profiles", () => { const cwd = process.cwd(); createProfile("work"); + std.setStderrTTY(true); await runCf(["auth", "activate", "work"]); expect(store.bindings.getProfileForDirectory(cwd)).toBe("work"); - expect(normalizeProfileOutput(std.getAndClearOut())).toMatchInlineSnapshot( - `"ℹ Profile "work" activated for ""."` + expect(std.getAndClearOut()).toBe(""); + expect(std.getAndClearErr()).toContain( + '✓ Profile "work" activated for "".' ); await runCf(["auth", "list"]); @@ -64,12 +60,14 @@ describe("cf auth profiles", () => { JSON.parse(String(vi.mocked(console.log).mock.lastCall?.[0])) ).toEqual([{ name: "work", boundDirectories: [cwd] }]); std.getAndClearOut(); + std.getAndClearErr(); await runCf(["auth", "deactivate"]); expect(store.bindings.getProfileForDirectory(cwd)).toBeUndefined(); - expect(normalizeProfileOutput(std.getAndClearOut())).toMatchInlineSnapshot(` - "ℹ Profile "work" deactivated from "". - → Run cf auth login to set up the default profile, or cf auth create to create a named profile." + expect(std.getAndClearOut()).toBe(""); + expect(std.getAndClearErr()).toMatchInlineSnapshot(` + "✓ Profile "work" deactivated from "". + ● Run cf auth login to set up the default profile, or cf auth create to create a named profile." `); }); @@ -126,18 +124,50 @@ describe("cf auth profiles", () => { expect(store.configs.exists("work")).toBe(false); expect(store.bindings.getBindingsForProfile("work")).toEqual([]); - expect(normalizeProfileOutput(std.out)).toMatchInlineSnapshot(` - "ℹ Removed directory bindings: - - ℹ Profile "work" deleted. - → No active profile for this directory. Run cf auth login to set up the default profile, or cf auth create to create a named profile." + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "● Removed directory bindings: + │ + Successfully logged out. + ● No active profile for this directory. Run cf auth login to set up the default profile, or cf auth create to create a named profile." `); }); + it("keeps the directory's active profile when deleting another", async () => { + createProfile("work"); + createProfile("other"); + store.bindings.activate("work", process.cwd()); + + await runCf(["auth", "delete", "other"]); + + expect(store.configs.exists("other")).toBe(false); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot(` + "Successfully logged out. + ● Active profile for this directory: work." + `); + }); + + it("falls back to the default profile after deactivating", async () => { + createProfile("default"); + createProfile("work"); + store.bindings.activate("work", process.cwd()); + + await runCf(["auth", "deactivate"]); + + expect( + store.bindings.getProfileForDirectory(process.cwd()) + ).toBeUndefined(); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"● This directory now uses the default profile."` + ); + }); + it("does not duplicate the active profile in whoami output", async () => { await runCf(["auth", "whoami", "--profile", "work"]); - expect(normalizeProfileOutput(std.out)).toMatchInlineSnapshot(` + expect(std.out).toMatchInlineSnapshot(` "{ "authenticated": false, "error": "Not logged in" @@ -181,21 +211,26 @@ describe("cf auth profiles", () => { store.bindings.activate("work", process.cwd()); await runCf(["auth", "login"]); - expect(normalizeProfileOutput(std.getAndClearOut())).toMatchInlineSnapshot(` - "⚠ This directory has profile "work" active. \`cf auth login\` updates the default profile, not "work". - To re-authenticate "work", run \`cf auth create work\`. - ℹ You are already logged in. - Config file: /home/.config/cloudflare/config/default.json - Token expires: 1/1/2999, 12:00:00 AM - → Run cf auth login --force to re-authenticate, or cf auth logout to log out." + expect(std.getAndClearOut()).toBe(""); + expect(std.getAndClearErr()).toMatchInlineSnapshot(` + "▲ This directory has profile "work" active. cf auth login updates the default profile, not "work". To re-authenticate "work", run: + + cf auth create work + + ● You are already logged in. + │ Config file: /home/.config/cloudflare/config/default.json + │ Token expires: 1/1/2999, 12:00:00 AM + │ Run cf auth login --force to re-authenticate, or cf auth logout to log out." `); await runCf(["auth", "logout"]); - expect(normalizeProfileOutput(std.getAndClearOut())).toMatchInlineSnapshot(` - "⚠ This directory has profile "work" active. \`cf auth logout\` removes the default profile's token, not "work". - To delete "work", run \`cf auth delete work\`. - ℹ Logging out... - Removed: OAuth tokens from /home/.config/cloudflare/config/default.json" + expect(std.getAndClearOut()).toBe(""); + expect(std.getAndClearErr()).toMatchInlineSnapshot(` + "▲ This directory has profile "work" active. cf auth logout removes the default profile's token, not "work". To delete "work", run: + + cf auth delete work + + Successfully logged out." `); }); }); diff --git a/packages/cli/src/__tests__/commands/migrate.test.ts b/packages/cli/src/__tests__/commands/migrate.test.ts index abd2bf9ed..831a8f2cc 100644 --- a/packages/cli/src/__tests__/commands/migrate.test.ts +++ b/packages/cli/src/__tests__/commands/migrate.test.ts @@ -1,14 +1,11 @@ import path from "node:path"; -import { - mockConsoleMethods, - runInTempDir, - seed, -} from "@cloudflare/workers-utils/test-helpers"; +import { runInTempDir, seed } from "@cloudflare/workers-utils/test-helpers"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { findWranglerConfig, maybeMigrateWranglerProject, } from "../../lib/wrangler-migration.js"; +import { mockOutputStreams } from "../helpers/capture-output.js"; import { runCf } from "../helpers/run-cf.js"; const { migrateWranglerToCf } = vi.hoisted(() => ({ @@ -22,7 +19,7 @@ vi.mock("@cloudflare/codemods", () => ({ describe("cf migrate", () => { runInTempDir(); - const std = mockConsoleMethods(); + const std = mockOutputStreams(); beforeEach(() => { migrateWranglerToCf.mockReset(); @@ -43,6 +40,7 @@ describe("cf migrate", () => { const result = await runCf(["migrate"]); expect(result.exitCode).toBe(0); + expect(std.out).toBe(""); expect(migrateWranglerToCf).toHaveBeenCalledWith( path.join(process.cwd(), configFile), { @@ -52,12 +50,12 @@ describe("cf migrate", () => { installDependencies: true, } ); - expect(std.out).toContain( + expect(std.err).toContain( "Using the Wrangler bundler because @cloudflare/vite-plugin is not declared." ); - expect(std.out).toContain("Updated 1 file(s):"); - expect(std.out).toContain("cloudflare.config.ts"); - expect(std.out).toContain("Migration complete."); + expect(std.err).toContain("Updated 1 file(s):"); + expect(std.err).toContain("cloudflare.config.ts"); + expect(std.err).toContain("Migration complete."); }); it.each(["dependencies", "devDependencies"])( @@ -76,7 +74,7 @@ describe("cf migrate", () => { path.join(process.cwd(), "wrangler.jsonc"), expect.objectContaining({ bundler: "vite" }) ); - expect(std.out).toContain( + expect(std.err).toContain( "Using the Vite bundler because @cloudflare/vite-plugin is declared." ); } @@ -96,7 +94,7 @@ describe("cf migrate", () => { path.join(process.cwd(), "config/wrangler.toml"), expect.objectContaining({ bundler: "vite" }) ); - expect(std.out).toContain( + expect(std.err).toContain( "Using the Vite bundler because @cloudflare/vite-plugin is declared." ); }); @@ -110,7 +108,7 @@ describe("cf migrate", () => { path.join(process.cwd(), "wrangler.json"), expect.objectContaining({ bundler: "vite" }) ); - expect(std.out).not.toContain("Using the"); + expect(std.err).not.toContain("Using the"); }); it("accepts an explicit config path and migration options", async () => { @@ -140,10 +138,11 @@ describe("cf migrate", () => { installDependencies: false, } ); - expect(std.out).toContain( + expect(std.out).toBe(""); + expect(std.err).toContain( "Would update 2 file(s):\n├─ config/cloudflare.config.ts\n└─ config/package.json" ); - expect(std.out).toContain("Migration preview complete."); + expect(std.err).toContain("Migration preview complete."); }); it("requires an explicit path when no Wrangler config is found", async () => { @@ -298,9 +297,10 @@ describe("cf migrate", () => { const result = await runCf(["migrate"]); expect(result.exitCode).toBe(1); - expect(std.out).toContain( + expect(std.out).toBe(""); + expect(std.err).toContain( "Follow-up work:\n└─ [required] durable_objects.migrations: Migrate this setting manually.\n └─ https://developers.cloudflare.com/workers/" ); - expect(std.out).toContain("Migration requires follow-up work."); + expect(std.err).toContain("Migration requires follow-up work."); }); }); diff --git a/packages/cli/src/__tests__/commands/telemetry.test.ts b/packages/cli/src/__tests__/commands/telemetry.test.ts index 0e7e3829e..51bba32e1 100644 --- a/packages/cli/src/__tests__/commands/telemetry.test.ts +++ b/packages/cli/src/__tests__/commands/telemetry.test.ts @@ -1,28 +1,49 @@ -import { - mockConsoleMethods, - runInTempDir, -} from "@cloudflare/workers-utils/test-helpers"; +import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; import { describe, expect, it } from "vite-plus/test"; +import { mockOutputStreams } from "../helpers/capture-output.js"; import { runCf } from "../helpers/run-cf.js"; describe("cf cli telemetry", () => { runInTempDir(); - const std = mockConsoleMethods(); + const std = mockOutputStreams(); it("reports the opt-out default and updates the stored preference", async () => { await runCf(["cli", "telemetry", "status"]); expect(std.getAndClearOut()).toContain("Status: Enabled"); await runCf(["cli", "telemetry", "disable"]); - expect(std.getAndClearOut()).toContain("Status: Disabled"); - await runCf(["cli", "telemetry", "status"]); expect(std.getAndClearOut()).toContain("Status: Disabled"); await runCf(["cli", "telemetry", "enable"]); + await runCf(["cli", "telemetry", "status"]); expect(std.getAndClearOut()).toContain("Status: Enabled"); }); + it("confirms each change on stderr and keeps stdout empty", async () => { + std.setStderrTTY(true); + + await runCf(["cli", "telemetry", "disable"]); + expect(std.out).toBe(""); + expect(std.getAndClearErr()).toContain( + "✓ cf is no longer collecting usage telemetry." + ); + + await runCf(["cli", "telemetry", "enable"]); + expect(std.out).toBe(""); + expect(std.getAndClearErr()).toContain( + "✓ cf is now collecting anonymous usage telemetry. Thank you for helping make cf better." + ); + }); + + it("prints nothing when a change succeeds outside a terminal", async () => { + await runCf(["cli", "telemetry", "disable"]); + await runCf(["cli", "telemetry", "enable"]); + + expect(std.out).toBe(""); + expect(std.err).toBe(""); + }); + it("shows when the environment overrides stored state", async () => { await runCf(["cli", "telemetry", "status"], { CF_SEND_TELEMETRY: "false", @@ -32,29 +53,32 @@ describe("cf cli telemetry", () => { ); }); - it("reports an environment override after enabling telemetry", async () => { + it("warns when the environment overrides enabling telemetry", async () => { await runCf(["cli", "telemetry", "enable"], { CF_SEND_TELEMETRY: "false", }); - const output = std.getAndClearOut(); - expect(output).toContain("Status: Disabled (set by CF_SEND_TELEMETRY)"); - expect(output).toContain("CF_SEND_TELEMETRY=false overrides this setting"); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"▲ cf is not collecting usage telemetry because CF_SEND_TELEMETRY=false overrides this setting."` + ); }); - it("reports an environment override after disabling telemetry", async () => { + it("warns when the environment overrides disabling telemetry", async () => { await runCf(["cli", "telemetry", "disable"], { CF_SEND_TELEMETRY: "true", }); - const output = std.getAndClearOut(); - expect(output).toContain("Status: Enabled (set by CF_SEND_TELEMETRY)"); - expect(output).toContain("CF_SEND_TELEMETRY=true overrides this setting"); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"▲ cf is collecting usage telemetry because CF_SEND_TELEMETRY=true overrides this setting."` + ); }); - it("shows DO_NOT_TRACK as the effective source after enabling", async () => { + it("names DO_NOT_TRACK when it overrides enabling telemetry", async () => { await runCf(["cli", "telemetry", "enable"], { DO_NOT_TRACK: "1" }); - const output = std.getAndClearOut(); - expect(output).toContain("Status: Disabled (set by DO_NOT_TRACK)"); - expect(output).toContain("DO_NOT_TRACK overrides this setting"); + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"▲ cf is not collecting usage telemetry because DO_NOT_TRACK overrides this setting."` + ); }); it("shows the Wrangler environment alias as the source", async () => { @@ -66,14 +90,13 @@ describe("cf cli telemetry", () => { ); }); - it("reports the alias when it overrides enabling telemetry", async () => { + it("names the alias when it overrides enabling telemetry", async () => { await runCf(["cli", "telemetry", "enable"], { WRANGLER_SEND_METRICS: "false", }); - const output = std.getAndClearOut(); - expect(output).toContain("Status: Disabled (set by WRANGLER_SEND_METRICS)"); - expect(output).toContain( - "WRANGLER_SEND_METRICS=false overrides this setting" + expect(std.out).toBe(""); + expect(std.err).toMatchInlineSnapshot( + `"▲ cf is not collecting usage telemetry because WRANGLER_SEND_METRICS=false overrides this setting."` ); }); }); diff --git a/packages/cli/src/__tests__/helpers/capture-output.ts b/packages/cli/src/__tests__/helpers/capture-output.ts index 4bd4c33f0..fb38536ac 100644 --- a/packages/cli/src/__tests__/helpers/capture-output.ts +++ b/packages/cli/src/__tests__/helpers/capture-output.ts @@ -1,4 +1,6 @@ -import { vi } from "vite-plus/test"; +import { format } from "node:util"; +import { normalizeString } from "@cloudflare/workers-utils/test-helpers"; +import { afterEach, beforeEach, vi } from "vite-plus/test"; /** * Capture a command's output streams. @@ -27,3 +29,95 @@ export function captureOutput(): { }, }; } + +/** + * Capture everything written to each stream, like `mockConsoleMethods`. + * + * Console calls and direct stream writes (clack lines, `✓` success lines, + * raw payloads) share one buffer per stream, so a test can assert that + * stdout holds only the command's output. Call at `describe` level; output + * is normalized the same way as `mockConsoleMethods`. + * + * stderr is not a terminal unless a test calls `setStderrTTY(true)`, so + * terminal-only output does not depend on how the suite was launched. + */ +export function mockOutputStreams(): { + readonly out: string; + readonly err: string; + getAndClearOut: () => string; + getAndClearErr: () => string; + setStderrTTY: (isTTY: boolean) => void; +} { + let out = ""; + let err = ""; + let spies: { mockRestore: () => void }[] = []; + let originalStderrTTY: PropertyDescriptor | undefined; + const setStderrTTY = (isTTY: boolean) => { + Object.defineProperty(process.stderr, "isTTY", { + configurable: true, + value: isTTY, + }); + }; + const toOut = (...args: unknown[]) => { + out += `${format(...args)}\n`; + }; + const toErr = (...args: unknown[]) => { + err += `${format(...args)}\n`; + }; + + beforeEach(() => { + out = ""; + err = ""; + originalStderrTTY = Object.getOwnPropertyDescriptor( + process.stderr, + "isTTY" + ); + setStderrTTY(false); + spies = [ + vi.spyOn(console, "log").mockImplementation(toOut), + vi.spyOn(console, "info").mockImplementation(toOut), + vi.spyOn(console, "debug").mockImplementation(toOut), + vi.spyOn(console, "warn").mockImplementation(toErr), + vi.spyOn(console, "error").mockImplementation(toErr), + vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { + out += String(chunk); + return true; + }), + vi.spyOn(process.stderr, "write").mockImplementation((chunk) => { + err += String(chunk); + return true; + }), + ]; + }); + afterEach(() => { + for (const spy of spies) { + spy.mockRestore(); + } + if (originalStderrTTY) { + Object.defineProperty(process.stderr, "isTTY", originalStderrTTY); + } else { + Reflect.deleteProperty(process.stderr, "isTTY"); + } + }); + + const read = (buffer: string) => normalizeString(buffer).replace(/\n+$/, ""); + return { + get out() { + return read(out); + }, + get err() { + return read(err); + }, + getAndClearOut: () => { + const value = read(out); + out = ""; + return value; + }, + getAndClearErr: () => { + const value = read(err); + err = ""; + return value; + }, + setStderrTTY, + }; +} diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 23262b385..b26111c93 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -1,3 +1,4 @@ +import { getAuthFromEnv } from "@cloudflare/workers-auth"; import { getActiveProfile, getConfigPath, @@ -8,7 +9,8 @@ import { setKeyringPreference, setProfile, } from "../../lib/oauth/index.js"; -import { hint, info, theme, warning } from "../../lib/ui/index.js"; +import { theme } from "../../lib/ui/index.js"; +import { log, printCommands } from "../../lib/ui/log.js"; import { assertNoProfileFlag } from "./profiles.js"; import type { CommonYargsOptions } from "../../lib/cli-types.js"; import type { ArgClassification } from "../../lib/telemetry/index.js"; @@ -77,12 +79,10 @@ const loginCommand: CommandModule = { const activeProfile = getActiveProfile(); if (activeProfile !== "default") { - console.log( - warning( - `This directory has profile "${activeProfile}" active. \`cf auth login\` updates the default profile, not "${activeProfile}".\n` + - `To re-authenticate "${activeProfile}", run \`cf auth create ${activeProfile}\`.` - ) + log.warn( + `This directory has profile "${activeProfile}" active. ${theme.code("cf auth login")} updates the default profile, not "${activeProfile}". To re-authenticate "${activeProfile}", run:` ); + printCommands(`cf auth create ${activeProfile}`); } setProfile("default"); @@ -93,41 +93,47 @@ const loginCommand: CommandModule = { setKeyringPreference(argv.useKeyring); } + // Env credentials take precedence over OAuth, and the OAuth flow refuses + // to start while they are set. + if (getAuthFromEnv({ allowGlobalAuthKey: false }) !== undefined) { + log.warn( + `${theme.code("CLOUDFLARE_API_TOKEN")} is set and takes precedence. Unset it to log in via OAuth.` + ); + return; + } + if (!argv.force && isOAuthLoggedIn()) { // Verify the stored tokens actually work, refreshing if needed. const validToken = await getValidToken(); if (validToken) { const state = readAuthCredentials(); - console.log(info("You are already logged in.")); - console.log( - `${theme.info("Config file:")} ${theme.muted(getConfigPath())}` - ); + const lines = [ + "You are already logged in.", + `${theme.info("Config file:")} ${theme.muted(getConfigPath())}`, + ]; if (state?.expiration_time) { - console.log( + lines.push( `${theme.info("Token expires:")} ${new Date( state.expiration_time ).toLocaleString()}` ); } - console.log( - hint( - `Run ${theme.code("cf auth login --force")} to re-authenticate, or ${theme.code("cf auth logout")} to log out.` - ) + lines.push( + `Run ${theme.code("cf auth login --force")} to re-authenticate, or ${theme.code("cf auth logout")} to log out.` ); + log.info(lines.join("\n")); return; } // Token refresh failed — proceed with a fresh login. - console.log( - warning( - "Existing tokens are invalid or expired. Starting fresh login...\n" - ) + log.warn( + "Existing tokens are invalid or expired. Starting fresh login..." ); } else if (argv.force && isOAuthLoggedIn()) { - console.log(info("Re-authenticating...\n")); + log.info("Re-authenticating..."); } - console.log(info("Starting OAuth login flow...\n")); + log.info("Starting OAuth login flow..."); const ok = await login({ browser: argv.browser, @@ -136,36 +142,28 @@ const loginCommand: CommandModule = { profile: "default", }); if (!ok) { - // Env credentials are present; the OAuth flow refuses to start. - console.log( - warning( - "CLOUDFLARE_API_TOKEN is set and takes precedence. Unset it to log in via OAuth." - ) - ); return; } const state = readAuthCredentials(); - console.log( - `${theme.info("Config file:")} ${theme.muted(getConfigPath())}` - ); + const lines = [ + `${theme.info("Config file:")} ${theme.muted(getConfigPath())}`, + ]; if (state?.scopes) { - if (state.scopes.length > MAX_INLINE_SCOPES) { - console.log(`${theme.info("Scopes:")} ${state.scopes.length} selected`); - console.log( - hint(`Run ${theme.code("cf auth whoami")} to see the full list.`) - ); - } else { - console.log(`${theme.info("Scopes:")} ${state.scopes.join(", ")}`); - } + lines.push( + state.scopes.length > MAX_INLINE_SCOPES + ? `${theme.info("Scopes:")} ${state.scopes.length} selected. Run ${theme.code("cf auth whoami")} to see the full list.` + : `${theme.info("Scopes:")} ${state.scopes.join(", ")}` + ); } if (state?.expiration_time) { - console.log( + lines.push( `${theme.info("Token expires:")} ${new Date( state.expiration_time ).toLocaleString()}` ); } + log.info(lines.join("\n")); }, }; diff --git a/packages/cli/src/commands/auth/logout.ts b/packages/cli/src/commands/auth/logout.ts index 2514fc06d..8a3a2f3de 100644 --- a/packages/cli/src/commands/auth/logout.ts +++ b/packages/cli/src/commands/auth/logout.ts @@ -1,3 +1,4 @@ +import { getAuthFromEnv } from "@cloudflare/workers-auth"; import { getActiveProfile, getConfigPath, @@ -5,7 +6,9 @@ import { logout, setProfile, } from "../../lib/oauth/index.js"; -import { hint, info, theme, warning } from "../../lib/ui/index.js"; +import { printSuccess } from "../../lib/output.js"; +import { theme } from "../../lib/ui/index.js"; +import { log, printCommands } from "../../lib/ui/log.js"; import { assertNoProfileFlag } from "./profiles.js"; import type { CommonYargsOptions } from "../../lib/cli-types.js"; import type { CommandModule } from "yargs"; @@ -24,55 +27,39 @@ const logoutCommand: CommandModule = { const activeProfile = getActiveProfile(); if (activeProfile !== "default") { - console.log( - warning( - `This directory has profile "${activeProfile}" active. \`cf auth logout\` removes the default profile's token, not "${activeProfile}".\n` + - `To delete "${activeProfile}", run \`cf auth delete ${activeProfile}\`.` - ) + log.warn( + `This directory has profile "${activeProfile}" active. ${theme.code("cf auth logout")} removes the default profile's token, not "${activeProfile}". To delete "${activeProfile}", run:` ); + printCommands(`cf auth delete ${activeProfile}`); } setProfile("default"); const hasOAuthTokens = isOAuthLoggedIn(); - const hasEnvToken = !!process.env.CLOUDFLARE_API_TOKEN; + const hasEnvToken = + getAuthFromEnv({ allowGlobalAuthKey: false }) !== undefined; if (!hasOAuthTokens) { if (hasEnvToken) { - console.log( - info( - "No cf OAuth credentials found, but CLOUDFLARE_API_TOKEN environment variable is set." - ) + log.info( + `No cf OAuth credentials found, but ${theme.code("CLOUDFLARE_API_TOKEN")} is set. Unset it to log out.` ); - console.log(info("Unset it with: unset CLOUDFLARE_API_TOKEN")); } else { - console.log(info("You are not currently logged in.")); - console.log( - hint(`Run ${theme.code("cf auth login")} to authenticate.`) - ); + log.info("You are not currently logged in. To authenticate, run:"); + printCommands("cf auth login"); } return; } - console.log(info("Logging out...")); - await logout("default"); - console.log( - `${theme.info("Removed:")} OAuth tokens from ${theme.muted(getConfigPath())}` - ); - + // workers-auth keeps the stored tokens, and says why, while env + // credentials are set. if (hasEnvToken) { - console.log(""); - console.log( - warning("CLOUDFLARE_API_TOKEN environment variable is still set.") - ); - console.log( - info( - "You will remain authenticated via the env var. Unset it with: unset CLOUDFLARE_API_TOKEN" - ) - ); + return; } + + printSuccess(`Removed OAuth tokens from ${theme.muted(getConfigPath())}`); }, }; diff --git a/packages/cli/src/commands/auth/profiles.ts b/packages/cli/src/commands/auth/profiles.ts index 668d7ef45..9fa4704de 100644 --- a/packages/cli/src/commands/auth/profiles.ts +++ b/packages/cli/src/commands/auth/profiles.ts @@ -1,8 +1,9 @@ import path from "node:path"; import { getAuthFromEnv, validateProfileName } from "@cloudflare/workers-auth"; import { getProfileStore, login, logout } from "../../lib/oauth/index.js"; -import { formatOutput } from "../../lib/output.js"; -import { hint, info, theme } from "../../lib/ui/index.js"; +import { formatOutput, printSuccess } from "../../lib/output.js"; +import { theme } from "../../lib/ui/index.js"; +import { log, printCommands } from "../../lib/ui/log.js"; import type { CommonYargsOptions } from "../../lib/cli-types.js"; import type { ArgumentsCamelCase, Argv, CommandModule } from "yargs"; @@ -98,18 +99,13 @@ export const createCommand: CommandModule = { profile: argv.name, }); - console.log( - info( - isUpdate - ? `Profile "${argv.name}" re-authenticated.` - : `Profile "${argv.name}" created.` - ) - ); - console.log( - hint( - `Run ${theme.code(`cf auth activate ${argv.name}`)} to use this profile in a directory.` - ) + printSuccess( + isUpdate + ? `Profile "${argv.name}" re-authenticated.` + : `Profile "${argv.name}" created.` ); + log.info("To use this profile in a directory, run:"); + printCommands(`cf auth activate ${argv.name}`); }, }; @@ -140,30 +136,28 @@ export const deleteCommand: CommandModule = { argv.name ); if (removedBindings.length > 0) { - console.log(info("Removed directory bindings:")); - for (const dir of removedBindings) { - console.log(` ${dir}`); - } + log.info( + [ + "Removed directory bindings:", + ...removedBindings.map((dir) => ` ${dir}`), + ].join("\n") + ); } await logout(argv.name); profiles.configs.delete(argv.name); - console.log(info(`Profile "${argv.name}" deleted.`)); + printSuccess(`Profile "${argv.name}" deleted.`); const currentProfile = profiles.bindings.getProfileForDirectory( process.cwd() ); if (currentProfile) { - console.log( - info(`Active profile for this directory: ${currentProfile}.`) - ); + log.info(`Active profile for this directory: ${currentProfile}.`); } else if (profiles.configs.exists("default")) { - console.log(info("This directory now uses the default profile.")); + log.info("This directory now uses the default profile."); } else { - console.log( - hint( - `No active profile for this directory. Run ${theme.code("cf auth login")} to set up the default profile, or ${theme.code("cf auth create ")} to create a named profile.` - ) + log.info( + `No active profile for this directory. Run ${theme.code("cf auth login")} to set up the default profile, or ${theme.code("cf auth create ")} to create a named profile.` ); } }, @@ -204,8 +198,8 @@ export const activateCommand: CommandModule = { const targetDir = argv.dir ?? process.cwd(); profiles.bindings.activate(argv.name, targetDir); - console.log( - info(`Profile "${argv.name}" activated for "${path.resolve(targetDir)}".`) + printSuccess( + `Profile "${argv.name}" activated for "${path.resolve(targetDir)}".` ); }, }; @@ -230,22 +224,18 @@ export const deactivateCommand: CommandModule = { const { removedProfile, newResolution } = profiles.bindings.deactivate(targetDir); - console.log( - info( - `Profile "${removedProfile}" deactivated from "${path.resolve(targetDir)}".` - ) + printSuccess( + `Profile "${removedProfile}" deactivated from "${path.resolve(targetDir)}".` ); if (newResolution.profile === undefined) { - console.log( - hint( - `Run ${theme.code("cf auth login")} to set up the default profile, or ${theme.code("cf auth create ")} to create a named profile.` - ) + log.info( + `Run ${theme.code("cf auth login")} to set up the default profile, or ${theme.code("cf auth create ")} to create a named profile.` ); } else if (newResolution.profile === "default") { - console.log(info("This directory now uses the default profile.")); + log.info("This directory now uses the default profile."); } else { - console.log( - info(`Now using: ${newResolution.profile} (${newResolution.source}).`) + log.info( + `Now using: ${newResolution.profile} (${newResolution.source}).` ); } }, diff --git a/packages/cli/src/commands/login/index.ts b/packages/cli/src/commands/login/index.ts index e24867d9a..4f7c50390 100644 --- a/packages/cli/src/commands/login/index.ts +++ b/packages/cli/src/commands/login/index.ts @@ -1,5 +1,6 @@ import { withTelemetry } from "../../lib/telemetry/index.js"; -import { info, theme } from "../../lib/ui/index.js"; +import { theme } from "../../lib/ui/index.js"; +import { log } from "../../lib/ui/log.js"; import loginCommand, { loginTelemetryClassification } from "../auth/login.js"; /** @@ -11,10 +12,8 @@ const loginAliasCommand: typeof loginCommand = { describe: false, handler: async (argv) => { if (!argv.quiet) { - console.error( - info( - `${theme.code("cf login")} is an alias for ${theme.code("cf auth login")}. Authentication commands are under ${theme.code("cf auth")}.` - ) + log.info( + `${theme.code("cf login")} is an alias for ${theme.code("cf auth login")}. Authentication commands are under ${theme.code("cf auth")}.` ); } await loginCommand.handler(argv); diff --git a/packages/cli/src/commands/migrate/index.ts b/packages/cli/src/commands/migrate/index.ts index 69dccbb8b..5553e8dfa 100644 --- a/packages/cli/src/commands/migrate/index.ts +++ b/packages/cli/src/commands/migrate/index.ts @@ -38,7 +38,7 @@ type MigrateArgs = InferArgs; type MigrationBundler = NonNullable; function printDetectedBundler(bundler: MigrationBundler): void { - console.log( + console.error( theme.muted( bundler === "vite" ? "Using the Vite bundler because @cloudflare/vite-plugin is declared. Pass --bundler wrangler to override." @@ -63,6 +63,7 @@ const migrateCommand: CommandModule = { dryRun: args["dry-run"], force: args.force, installDependencies: args.install, + output: "stderr", }); }, }; diff --git a/packages/cli/src/commands/telemetry/disable.ts b/packages/cli/src/commands/telemetry/disable.ts index ebeb66f98..370d296ed 100644 --- a/packages/cli/src/commands/telemetry/disable.ts +++ b/packages/cli/src/commands/telemetry/disable.ts @@ -1,9 +1,11 @@ +import { printSuccess } from "../../lib/output.js"; import { getTelemetrySourceLabel, resolveTelemetry, setTelemetryPermission, } from "../../lib/telemetry/config.js"; -import { telemetryStatusLine } from "./shared.js"; +import { theme } from "../../lib/ui/index.js"; +import { log } from "../../lib/ui/log.js"; import type { CommandModule } from "yargs"; const command: CommandModule = { @@ -12,12 +14,14 @@ const command: CommandModule = { handler: () => { setTelemetryPermission(false); const { enabled, source } = resolveTelemetry(); - console.log(telemetryStatusLine({ enabled, source })); - console.log( - enabled - ? `cf is collecting usage telemetry because ${getTelemetrySourceLabel(source)}=true overrides this setting.\n` - : "cf is no longer collecting usage telemetry.\n" - ); + if (enabled) { + const override = `${getTelemetrySourceLabel(source)}=true`; + log.warn( + `cf is collecting usage telemetry because ${theme.code(override)} overrides this setting.` + ); + } else { + printSuccess("cf is no longer collecting usage telemetry."); + } }, }; diff --git a/packages/cli/src/commands/telemetry/enable.ts b/packages/cli/src/commands/telemetry/enable.ts index e712e56f8..7b1b774b8 100644 --- a/packages/cli/src/commands/telemetry/enable.ts +++ b/packages/cli/src/commands/telemetry/enable.ts @@ -1,9 +1,11 @@ +import { printSuccess } from "../../lib/output.js"; import { getTelemetrySourceLabel, resolveTelemetry, setTelemetryPermission, } from "../../lib/telemetry/config.js"; -import { telemetryStatusLine } from "./shared.js"; +import { theme } from "../../lib/ui/index.js"; +import { log } from "../../lib/ui/log.js"; import type { CommandModule } from "yargs"; const command: CommandModule = { @@ -12,12 +14,16 @@ const command: CommandModule = { handler: () => { setTelemetryPermission(true); const { enabled, source } = resolveTelemetry(); - console.log(telemetryStatusLine({ enabled, source })); - console.log( - enabled - ? "cf is now collecting anonymous usage telemetry. Thank you for helping make cf better.\n" - : `cf is not collecting usage telemetry because ${getTelemetrySourceLabel(source)}${source === "env" || source === "wrangler-env" ? "=false" : ""} overrides this setting.\n` - ); + if (enabled) { + printSuccess( + "cf is now collecting anonymous usage telemetry. Thank you for helping make cf better." + ); + } else { + const override = `${getTelemetrySourceLabel(source)}${source === "env" || source === "wrangler-env" ? "=false" : ""}`; + log.warn( + `cf is not collecting usage telemetry because ${theme.code(override)} overrides this setting.` + ); + } }, }; diff --git a/packages/cli/src/lib/oauth/account-hint.ts b/packages/cli/src/lib/oauth/account-hint.ts index e467f027f..acede0883 100644 --- a/packages/cli/src/lib/oauth/account-hint.ts +++ b/packages/cli/src/lib/oauth/account-hint.ts @@ -10,12 +10,10 @@ import { findCloudflareConfig, } from "../project-settings.js"; import { wrapPlain } from "../ui/blocks.js"; +import { GUIDE_WIDTH } from "../ui/log.js"; import { theme } from "../ui/theme.js"; import type { Writable } from "node:stream"; -/** clack prefixes message lines with a glyph and two spaces (`│ `). */ -const GUIDE_WIDTH = 3; - export function printAccountConfigHint( accountId: string, options: { diff --git a/packages/cli/src/lib/output.ts b/packages/cli/src/lib/output.ts index 13fbe0aa6..80f6b3a61 100644 --- a/packages/cli/src/lib/output.ts +++ b/packages/cli/src/lib/output.ts @@ -23,6 +23,16 @@ function outputPayload(data: unknown): unknown { return data; } +/** + * The `✓` line confirming that a command finished. It prints only when + * stderr is a terminal; scripts read the exit code instead. + */ +export function printSuccess(message: string): void { + if (process.stderr.isTTY) { + process.stderr.write(`${theme.success("✓")} ${message}\n`); + } +} + /** * Format and output data as JSON. * @@ -46,8 +56,8 @@ export function formatOutput(data: unknown, options: OutputOptions = {}): void { // the literal string 'null'), and — on TTYs only — write a brief ✓ // line to stderr so humans know it worked. if (data === null || data === undefined) { - if (options.successLabel && process.stderr.isTTY) { - process.stderr.write(`${theme.success("✓")} ${options.successLabel}\n`); + if (options.successLabel) { + printSuccess(options.successLabel); } return; } diff --git a/packages/cli/src/lib/ui/format.ts b/packages/cli/src/lib/ui/format.ts index d56d9ca0b..6aa604b06 100644 --- a/packages/cli/src/lib/ui/format.ts +++ b/packages/cli/src/lib/ui/format.ts @@ -33,17 +33,6 @@ export function warning(message: string): string { return `${theme.warning(symbols.warning)} ${message}`; } -/** - * Format an info message with info symbol. - * - * @example - * console.log(info('Use --help for more options')); - * // ℹ Use --help for more options - */ -export function info(message: string): string { - return `${theme.info(symbols.info)} ${message}`; -} - /** * Format a hint/tip message with arrow prefix. * diff --git a/packages/cli/src/lib/ui/index.ts b/packages/cli/src/lib/ui/index.ts index 6ae6c2e1e..ebc10bf49 100644 --- a/packages/cli/src/lib/ui/index.ts +++ b/packages/cli/src/lib/ui/index.ts @@ -8,5 +8,5 @@ export { formatCommand, renderPromptIntro } from "./banner.js"; export { errorBlock } from "./blocks.js"; -export { hint, info, success, warning } from "./format.js"; +export { hint, success, warning } from "./format.js"; export { colorPalette, supportsColor, symbols, theme } from "./theme.js"; diff --git a/packages/cli/src/lib/ui/log.ts b/packages/cli/src/lib/ui/log.ts new file mode 100644 index 000000000..1c6eb62df --- /dev/null +++ b/packages/cli/src/lib/ui/log.ts @@ -0,0 +1,42 @@ +/** + * Messages for people. + * + * stdout carries only a command's output (JSON or a raw payload), so every + * message goes to stderr: clack log lines for notices and warnings, and + * commands for the user to run next. The `✓` success line is `printSuccess` + * in `../output.ts`. + * + * Not re-exported from `./index.ts`: the entry point imports that barrel, and + * loading clack there would slow every invocation. + */ + +import * as clack from "@clack/prompts"; +import { theme } from "./theme.js"; +import type { LogMessageOptions } from "@clack/prompts"; + +/** clack prefixes message lines with a glyph and two spaces (`│ `). */ +export const GUIDE_WIDTH = 3; + +function toStderr(options?: LogMessageOptions): LogMessageOptions { + return { output: process.stderr, spacing: 0, ...options }; +} + +/** clack log lines on stderr, without a spacer line above by default. */ +export const log = { + info: (message: string, options?: LogMessageOptions) => + clack.log.info(message, toStderr(options)), + warn: (message: string, options?: LogMessageOptions) => + clack.log.warn(message, toStderr(options)), +}; + +/** + * Commands for the user to run next, each on its own line. They are indented + * to the guide's text column without its glyphs, so copying one from the + * terminal yields exactly the command. + */ +export function printCommands(...commands: string[]): void { + const indent = " ".repeat(GUIDE_WIDTH); + process.stderr.write( + `\n${commands.map((command) => indent + theme.code(command)).join("\n")}\n\n` + ); +} diff --git a/packages/wrangler-tests/src/__tests__/logout.test.ts b/packages/wrangler-tests/src/__tests__/logout.test.ts index 2772b5f8b..f9726313e 100644 --- a/packages/wrangler-tests/src/__tests__/logout.test.ts +++ b/packages/wrangler-tests/src/__tests__/logout.test.ts @@ -27,6 +27,8 @@ function cfAuthFilePath(): string { describe("logout", () => { runInTempDir(); const std = mockConsoleMethods(); + // The clack mock drops log lines, so cf's own messages can't be asserted + // here. The cf package's auth tests check them with real clack. // cf's `loadTokens()` / `loadApiToken()` now resolve their paths via // `homedir()` at call time, so `runInTempDir()`'s HOME stub correctly @@ -35,19 +37,16 @@ describe("logout", () => { expect, }) => { await runWrangler("auth logout", { CLOUDFLARE_API_TOKEN: undefined }); - expect(std.out).toContain("You are not currently logged in."); + expect(std.out).toBe(""); }); // Env `CLOUDFLARE_API_TOKEN` set with no on-disk credentials in the - // stubbed HOME: cf prints the env-var hint and exits cleanly. + // stubbed HOME: cf prints the env-var hint on stderr and exits cleanly. it("should exit with a message stating the user logged in via API token", async ({ expect, }) => { await runWrangler("auth logout", { CLOUDFLARE_API_TOKEN: "DUMMY_TOKEN" }); - expect(std.out).toContain( - "No cf OAuth credentials found, but CLOUDFLARE_API_TOKEN environment variable is set." - ); - expect(std.out).toContain("Unset it with: unset CLOUDFLARE_API_TOKEN"); + expect(std.out).toBe(""); }); // Ported now that token paths resolve via `homedir()` at call time @@ -88,8 +87,8 @@ describe("logout", () => { await runWrangler("auth logout", { CLOUDFLARE_API_TOKEN: undefined }); - // cf reports the credential kind and path that it removed. - expect(std.out).toContain("Removed: OAuth tokens from"); + expect(std.out).toBe(""); + expect(std.err).toContain("Successfully logged out."); // The refresh token was sent to the revocation endpoint. expect(revoked).toBe(true); // And the on-disk token file was removed. diff --git a/packages/wrangler-tests/src/__tests__/profiles.test.ts b/packages/wrangler-tests/src/__tests__/profiles.test.ts index f5075b058..5fc5779c3 100644 --- a/packages/wrangler-tests/src/__tests__/profiles.test.ts +++ b/packages/wrangler-tests/src/__tests__/profiles.test.ts @@ -42,6 +42,8 @@ function createDeprecatedProfileFile(name: string): void { describe("Profiles", () => { runInTempDir(); const std = mockConsoleMethods(); + // The clack mock drops log lines, so cf's own messages can't be asserted + // here. The cf package's auth tests check them with real clack. beforeEach(() => { // Profile management is unavailable when environment credentials are @@ -208,9 +210,7 @@ describe("Profiles", () => { expect(profiles().configs.exists("client-a")).toBe(false); expect(profiles().bindings.getBindingsForProfile("client-a")).toEqual([]); - expect(std.out).toContain("Removed directory bindings:"); - expect(std.out).toContain("projects/client-a"); - expect(std.out).toContain("projects/client-a-v2"); + expect(std.out).toBe(""); }); it("deletes a profile file when logout returns before clearing storage", async ({ @@ -219,7 +219,7 @@ describe("Profiles", () => { createDeprecatedProfileFile("client-a"); await runWrangler("auth delete client-a"); expect(profiles().configs.exists("client-a")).toBe(false); - expect(std.out).toContain('Profile "client-a" deleted.'); + expect(std.out).toBe(""); }); it("falls back to ancestor profile after deleting a child profile", async ({ @@ -340,7 +340,7 @@ describe("Profiles", () => { expect( profiles().bindings.getProfileForDirectory(process.cwd()) ).toBeUndefined(); - expect(std.out).toContain("This directory now uses the default profile."); + expect(std.out).toBe(""); }); it("shows no active profile when logged out globally", async ({ @@ -349,9 +349,11 @@ describe("Profiles", () => { createProfileFile("client-a"); profiles().bindings.activate("client-a", process.cwd()); await runWrangler("auth deactivate"); - expect(std.out).toContain( - "Run cf auth login to set up the default profile" - ); + expect( + profiles().bindings.getProfileForDirectory(process.cwd()) + ).toBeUndefined(); + expect(profiles().configs.exists("default")).toBe(false); + expect(std.out).toBe(""); }); });