diff --git a/.changeset/browse-skill-nudge.md b/.changeset/browse-skill-nudge.md new file mode 100644 index 000000000..30ef072cb --- /dev/null +++ b/.changeset/browse-skill-nudge.md @@ -0,0 +1,7 @@ +--- +"browse": patch +--- + +Surface the browse skill at runtime with two static touchpoints. Root help (`browse` / `browse --help`) now always leads with a "Start here (for AI agents)" banner pointing to `browse skills install`. Separately, the first regular command on a fresh install prints a one-time stderr hint (never stdout) when the canonical skill dir (`~/.agents/skills/browse`) is absent — gated by a once-per-install marker file in the CLI cache dir, skipped on `help`/`skills` commands and in CI/tests, and disabled with `BROWSE_DISABLE_SKILL_NUDGE=1`. No agent detection, session keys, or time windows are involved; the only check is one canonical-path lookup. Command telemetry includes a `skill_present` property driven by the same check so skill adoption is measurable. + +Also stop the "Update available" notice from printing on every command. The update check still refreshes its cache silently in the background, but the notice is now shown only on the human-facing surfaces — `browse` / `browse --help` and `browse doctor` — so it no longer spams scripts and agent command loops. diff --git a/packages/cli/package.json b/packages/cli/package.json index 2db6085ba..5f8e3d6aa 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -24,6 +24,7 @@ "bin": "browse", "dirname": "browse", "commands": "./dist/commands", + "helpClass": "./dist/lib/help", "hooks": { "init": "./dist/hooks/init.js", "prerun": "./dist/hooks/prerun.js", diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 497096aed..7b7b8a296 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -1,3 +1,5 @@ +import { join } from "node:path"; + import { Flags } from "@oclif/core"; import { BrowseCommand } from "../base.js"; @@ -8,6 +10,7 @@ import { import { buildDoctorReport, renderDoctorReport } from "../lib/driver/doctor.js"; import { sessionName } from "../lib/driver/flags.js"; import { outputJson } from "../lib/output.js"; +import { getUpdateNotice } from "../lib/update.js"; export default class Doctor extends BrowseCommand { static override description = @@ -42,6 +45,20 @@ export default class Doctor extends BrowseCommand { } this.log(renderDoctorReport(report)); + await this.writeUpdateNotice(); if (report.verdict === "fail") this.exit(1); } + + private async writeUpdateNotice(): Promise { + try { + const notice = await getUpdateNotice(this.config.version, process.env, { + cacheFile: join(this.config.cacheDir, "update-check.json"), + }); + if (notice) { + process.stderr.write(`\n${notice}`); + } + } catch { + // Best-effort update notice should never affect doctor output. + } + } } diff --git a/packages/cli/src/hooks/init.ts b/packages/cli/src/hooks/init.ts index 3010eb7d3..efb430e6b 100644 --- a/packages/cli/src/hooks/init.ts +++ b/packages/cli/src/hooks/init.ts @@ -3,9 +3,12 @@ import { join } from "node:path"; import type { Hook } from "@oclif/core"; import { startTelemetryInvocation } from "../lib/telemetry.js"; -import { maybeAutoUpdateCli } from "../lib/update.js"; +import { + scheduleBackgroundUpdateCheck, + takeUpdateNotice, +} from "../lib/update.js"; -const hook: Hook.Init = async function ({ config }) { +const hook: Hook.Init = async function ({ config, id }) { try { startTelemetryInvocation(); } catch { @@ -13,12 +16,28 @@ const hook: Hook.Init = async function ({ config }) { } try { - await maybeAutoUpdateCli(config.version, process.env, { + // Silent: refresh the cached latest version when stale, but never print. + await scheduleBackgroundUpdateCheck(process.env, { cacheFile: join(config.cacheDir, "update-check.json"), }); } catch { // Best-effort update checks should never affect CLI behavior. } + + try { + // Remind until upgraded, at most once per interval; help and doctor render + // it themselves, so skip those surfaces to avoid double-printing. + if (id && id !== "help" && id !== "doctor") { + const notice = await takeUpdateNotice(config.version, process.env, { + cacheFile: join(config.cacheDir, "update-check.json"), + }); + if (notice) { + process.stderr.write(`\n${notice}`); + } + } + } catch { + // Best-effort update notices should never affect CLI behavior. + } }; export default hook; diff --git a/packages/cli/src/lib/driver/daemon/client.ts b/packages/cli/src/lib/driver/daemon/client.ts index 353e05bc4..0c11d34f3 100644 --- a/packages/cli/src/lib/driver/daemon/client.ts +++ b/packages/cli/src/lib/driver/daemon/client.ts @@ -3,6 +3,7 @@ import { promises as fs } from "node:fs"; import net from "node:net"; import { fail } from "../../errors.js"; +import { maybeNudgeInstallSkill } from "../../skill-nudge.js"; import type { DriverCommandName } from "../commands/types.js"; import { targetsCompatible } from "../mode.js"; import type { ConnectionTarget, DriverStatus, OpenResult } from "../types.js"; @@ -56,6 +57,12 @@ export async function ensureDriverDaemon({ } spawnDaemon(session, target); await waitForSocketReady(getSocketPath(session), 30_000); + try { + // A fresh daemon marks the start of a browser session. + await maybeNudgeInstallSkill(); + } catch { + // Best-effort nudge must never affect daemon startup. + } } finally { await releaseLock(session); } diff --git a/packages/cli/src/lib/help.ts b/packages/cli/src/lib/help.ts new file mode 100644 index 000000000..129b0c19f --- /dev/null +++ b/packages/cli/src/lib/help.ts @@ -0,0 +1,39 @@ +import { join } from "node:path"; + +import { Help } from "@oclif/core"; + +import { getUpdateNotice } from "./update.js"; + +const AGENT_START_HERE = `Start here (for AI agents): + Run \`browse skills install\` to load the browse skill into your coding agent + (Claude Code, Codex, Cursor, Gemini, …), then prefer \`browse\` for browser + automation. +`; + +/** + * Root-help override that leads with an agent-targeted "Start here" pointer to + * the browse skill — static help text, always shown on bare `browse` and + * `browse --help` (both route through showRootHelp). Also surfaces the update + * notice here and in `doctor` — the only human-facing surfaces that show it, + * so it never spams commands. + */ +export default class BrowseHelp extends Help { + public override async showRootHelp(): Promise { + this.log(AGENT_START_HERE); + await super.showRootHelp(); + await this.writeUpdateNotice(); + } + + private async writeUpdateNotice(): Promise { + try { + const notice = await getUpdateNotice(this.config.version, process.env, { + cacheFile: join(this.config.cacheDir, "update-check.json"), + }); + if (notice) { + process.stderr.write(`\n${notice}`); + } + } catch { + // Best-effort update notice should never affect help output. + } + } +} diff --git a/packages/cli/src/lib/skill-nudge.ts b/packages/cli/src/lib/skill-nudge.ts new file mode 100644 index 000000000..f523d74cd --- /dev/null +++ b/packages/cli/src/lib/skill-nudge.ts @@ -0,0 +1,61 @@ +import { isBrowseSkillInstalled } from "./skill-presence.js"; + +/** + * Stderr tip suggesting `browse skills install`, shown when a new browser + * session's daemon starts and the bundled skill is not installed. The daemon + * spawn is the session boundary, so the tip appears once per session until + * the skill is installed — no marker files or time windows. Best-effort and + * stderr-only: it can never affect machine-readable stdout. + */ +export async function maybeNudgeInstallSkill( + env: NodeJS.ProcessEnv = process.env, +): Promise { + if (isNudgeDisabled(env)) { + return; + } + + if (await isBrowseSkillInstalled()) { + return; + } + + writeNudge(); +} + +function writeNudge(): void { + process.stderr.write( + [ + "Tip: browse works best with its skill loaded into your agent.", + "Run:", + " browse skills install", + "", + ].join("\n"), + ); +} + +function isNudgeDisabled(env: NodeJS.ProcessEnv): boolean { + if ( + env.BROWSE_DISABLE_SKILL_NUDGE === "1" || + env.BB_DISABLE_SKILL_NUDGE === "1" + ) { + return true; + } + if (env.NODE_ENV === "test") { + return true; + } + return isCiEnvironment(env); +} + +function isCiEnvironment(env: NodeJS.ProcessEnv): boolean { + const value = env.CI; + if (!value) { + return false; + } + const normalized = value.trim().toLowerCase(); + return !( + normalized === "" || + normalized === "0" || + normalized === "false" || + normalized === "no" || + normalized === "off" + ); +} diff --git a/packages/cli/src/lib/skill-presence.ts b/packages/cli/src/lib/skill-presence.ts new file mode 100644 index 000000000..1a7dec7e2 --- /dev/null +++ b/packages/cli/src/lib/skill-presence.ts @@ -0,0 +1,36 @@ +import { access } from "node:fs/promises"; +import { homedir } from "node:os"; +import { join } from "node:path"; + +/** + * Check the canonical install locations for the bundled browse skill, matching + * the `skills` CLI's own two scopes (its source resolves + * `join(global ? homedir() : cwd, ".agents", "skills")`): + * + * - global: `~/.agents/skills/browse` — what `browse skills install` + * itself writes (`npx skills add --global --agent '*'`) + * - project: `/.agents/skills/browse` — `skills add` without `-g` + * + * Agent-specific dirs are symlinks/copies alongside the canonical dir, so two + * filesystem checks cover every agent at both scopes. + */ +export async function isBrowseSkillInstalled( + home: string = homedir(), + cwd: string = process.cwd(), +): Promise { + const candidates = [ + join(home, ".agents", "skills", "browse"), + join(cwd, ".agents", "skills", "browse"), + ]; + + for (const candidate of candidates) { + try { + await access(candidate); + return true; + } catch { + // Keep checking the remaining scopes. + } + } + + return false; +} diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index 8b0f04024..0e836599f 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -7,6 +7,7 @@ import type { Command } from "@oclif/core"; import { detectAgent } from "./agent.js"; import { getRunTelemetry, resetRunTelemetry } from "./run-telemetry.js"; +import { isBrowseSkillInstalled } from "./skill-presence.js"; import type { CommandFailureTelemetry } from "./errors.js"; const browserbaseTelemetrySource = "cli"; @@ -141,6 +142,9 @@ function createCliTelemetry(options: CreateCliTelemetryOptions): CliTelemetry { ? resolveAnonymousInstallId(env, options.sessionId) : Promise.resolve(""); const agentPromise = telemetryEnabled ? detectAgent() : Promise.resolve(null); + const skillPresentPromise: Promise = telemetryEnabled + ? isBrowseSkillInstalled().catch(() => null) + : Promise.resolve(null); const baseProperties: TelemetryProperties = { source: browserbaseTelemetrySource, @@ -157,9 +161,10 @@ function createCliTelemetry(options: CreateCliTelemetryOptions): CliTelemetry { return; } - const [distinctId, agent] = await Promise.all([ + const [distinctId, agent, skillPresent] = await Promise.all([ distinctIdPromise, agentPromise, + skillPresentPromise, ]); await posthogCapture(transport, { @@ -170,6 +175,7 @@ function createCliTelemetry(options: CreateCliTelemetryOptions): CliTelemetry { properties: { ...baseProperties, agent, + skill_present: skillPresent, ...properties, }, }); diff --git a/packages/cli/src/lib/update.ts b/packages/cli/src/lib/update.ts index 84040bcdd..f07b0941d 100644 --- a/packages/cli/src/lib/update.ts +++ b/packages/cli/src/lib/update.ts @@ -8,26 +8,113 @@ import semver from "semver"; const CLI_PACKAGE_NAME = "browse"; const DEFAULT_NPM_REGISTRY_BASE_URL = "https://registry.npmjs.org/"; const UPDATE_CACHE_TTL_MS = 24 * 60 * 60 * 1000; +// Codex-parity cadence (codex-rs/tui/src/updates.rs uses 20h): remind on +// regular commands until the user upgrades, at most once per interval. +const UPDATE_NOTIFY_INTERVAL_MS = 20 * 60 * 60 * 1000; const UPDATE_TIMEOUT_MS = 1500; interface UpdateCheckCache { checkedAt: string; version: string; + /** When the user was last shown the update notice (any surface). */ + lastNotifiedAt?: string; } interface UpdateCheckOptions { cacheFile?: string; } -export async function maybeAutoUpdateCli( +/** + * Read-only check for a newer published version. Returns the formatted notice + * text when a fresh cache shows an update is available, otherwise null. Never + * prints and never hits the network — call from human-facing surfaces (root + * help, `doctor`) rather than on every command so we don't spam automation. + */ +export async function getUpdateNotice( currentVersion: string, env: NodeJS.ProcessEnv = process.env, options: UpdateCheckOptions = {}, -): Promise { +): Promise { + if (isUpdateCheckDisabled(env)) { + return null; + } + + const cachePath = resolveUpdateCheckPath(env, options); + if (!cachePath) { + return null; + } + + const cache = await readFreshUpdateCheckCache(cachePath); + if (!cache || !isVersionNewer(currentVersion, cache.version)) { + return null; + } + + // Pull surfaces always render; marking is best-effort so the push notice + // does not immediately repeat what the user has already seen. + await writeUpdateCheckCache(cachePath, { + ...cache, + lastNotifiedAt: new Date().toISOString(), + }); + + return formatUpdateNotice(currentVersion, cache.version); +} + +/** + * Push notice for regular commands: reminds until the user upgrades, at most + * once per UPDATE_NOTIFY_INTERVAL_MS (Codex shows its upgrade banner every + * session until upgraded; the interval is the one-shot-CLI analog of "once + * per session"). The notice is only returned when recording lastNotifiedAt + * succeeds, so a read-only cache dir can never cause repeated printing. + */ +export async function takeUpdateNotice( + currentVersion: string, + env: NodeJS.ProcessEnv = process.env, + options: UpdateCheckOptions = {}, +): Promise { + if (isUpdateCheckDisabled(env)) { + return null; + } + + const cachePath = resolveUpdateCheckPath(env, options); + if (!cachePath) { + return null; + } + + const cache = await readFreshUpdateCheckCache(cachePath); + if (!cache || !isVersionNewer(currentVersion, cache.version)) { + return null; + } + + const lastNotifiedMs = cache.lastNotifiedAt + ? Date.parse(cache.lastNotifiedAt) + : Number.NaN; if ( - env.BROWSE_DISABLE_UPDATE_CHECK === "1" || - env.BB_DISABLE_UPDATE_CHECK === "1" + Number.isFinite(lastNotifiedMs) && + Date.now() - lastNotifiedMs < UPDATE_NOTIFY_INTERVAL_MS ) { + return null; + } + + const recorded = await writeUpdateCheckCache(cachePath, { + ...cache, + lastNotifiedAt: new Date().toISOString(), + }); + if (!recorded) { + return null; + } + + return formatUpdateNotice(currentVersion, cache.version); +} + +/** + * Refresh the cached "latest version" in the background when it is stale, so + * the surfaces that show the notice have fresh data. Silent: never prints. + */ +export async function scheduleBackgroundUpdateCheck( + env: NodeJS.ProcessEnv = process.env, + options: UpdateCheckOptions = {}, +): Promise { + if (isUpdateCheckDisabled(env)) { return; } @@ -38,15 +125,19 @@ export async function maybeAutoUpdateCli( const cache = await readFreshUpdateCheckCache(cachePath); if (cache) { - if (isVersionNewer(currentVersion, cache.version)) { - writeUpdateNotice(currentVersion, cache.version); - } return; } spawnBackgroundUpdateCheck(env, cachePath); } +function isUpdateCheckDisabled(env: NodeJS.ProcessEnv): boolean { + return ( + env.BROWSE_DISABLE_UPDATE_CHECK === "1" || + env.BB_DISABLE_UPDATE_CHECK === "1" + ); +} + export async function refreshUpdateCheckCache( env: NodeJS.ProcessEnv = process.env, options: UpdateCheckOptions = {}, @@ -95,6 +186,7 @@ async function readUpdateCheckCache( const parsed = JSON.parse(contents) as { checkedAt?: unknown; version?: unknown; + lastNotifiedAt?: unknown; }; if (typeof parsed.version !== "string" || parsed.version.length === 0) { @@ -108,6 +200,10 @@ async function readUpdateCheckCache( return { checkedAt: parsed.checkedAt, version: parsed.version, + ...(typeof parsed.lastNotifiedAt === "string" && + parsed.lastNotifiedAt.length > 0 + ? { lastNotifiedAt: parsed.lastNotifiedAt } + : {}), }; } catch { return null; @@ -117,12 +213,14 @@ async function readUpdateCheckCache( async function writeUpdateCheckCache( cachePath: string, cache: UpdateCheckCache, -): Promise { +): Promise { try { await mkdir(dirname(cachePath), { recursive: true }); await writeFile(cachePath, `${JSON.stringify(cache)}\n`, "utf8"); + return true; } catch { // Best-effort cache writes should never affect CLI behavior. + return false; } } @@ -166,18 +264,16 @@ function resolveUpdateCheckWorkerPath(): string { ); } -function writeUpdateNotice( +function formatUpdateNotice( currentVersion: string, latestVersion: string, -): void { - process.stderr.write( - [ - `Update available: ${currentVersion} -> ${latestVersion}.`, - "Run:", - ` npm i -g ${CLI_PACKAGE_NAME}@latest`, - "", - ].join("\n"), - ); +): string { + return [ + `Update available: ${currentVersion} -> ${latestVersion}.`, + "Run:", + ` npm i -g ${CLI_PACKAGE_NAME}@latest`, + "", + ].join("\n"); } async function fetchLatestCliVersion(): Promise { diff --git a/packages/cli/tests/cli-surface.test.ts b/packages/cli/tests/cli-surface.test.ts index 41878e0b4..118880151 100644 --- a/packages/cli/tests/cli-surface.test.ts +++ b/packages/cli/tests/cli-surface.test.ts @@ -1,7 +1,28 @@ -import { describe, expect, it } from "vitest"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; import { runCli } from "./helpers/run-cli.js"; +const surfaceCleanup: string[] = []; + +async function homeWithoutSkill(): Promise { + const home = await mkdtemp(join(tmpdir(), "browse-banner-noskill-")); + surfaceCleanup.push(home); + return home; +} + +async function homeWithSkill(): Promise { + const home = await mkdtemp(join(tmpdir(), "browse-banner-skill-")); + surfaceCleanup.push(home); + const dir = join(home, ".agents", "skills", "browse"); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, "SKILL.md"), "---\nname: browse\n---\n", "utf8"); + return home; +} + const cloudCommandsWithExamples = [ ["cloud", "projects", "list"], ["cloud", "projects", "get"], @@ -46,6 +67,13 @@ const skillsCommandsWithExamples = [ ]; describe("CLI surface", () => { + afterEach(async () => { + while (surfaceCleanup.length > 0) { + const path = surfaceCleanup.pop(); + if (path) await rm(path, { recursive: true, force: true }); + } + }); + it("prints browse root help", async () => { const result = await runCli(["--help"]); expect(result.exitCode).toBe(0); @@ -57,6 +85,23 @@ describe("CLI surface", () => { expect(result.stdout).toContain("skills"); }); + it("shows the skill banner on root help when the skill is not installed", async () => { + const result = await runCli(["--help"], { + env: { HOME: await homeWithoutSkill() }, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("Start here (for AI agents)"); + }); + + it("shows the skill banner on root help even when the skill is installed", async () => { + const result = await runCli(["--help"], { + env: { HOME: await homeWithSkill() }, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("Start here (for AI agents)"); + expect(result.stdout).toContain("Unified Browserbase CLI"); + }); + it("prints cloud topic help", async () => { const result = await runCli(["cloud", "--help"]); expect(result.exitCode).toBe(0); diff --git a/packages/cli/tests/cli-update.test.ts b/packages/cli/tests/cli-update.test.ts index af5169de2..37e9ffee1 100644 --- a/packages/cli/tests/cli-update.test.ts +++ b/packages/cli/tests/cli-update.test.ts @@ -7,8 +7,9 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { runCli } from "./helpers/run-cli.js"; import { - maybeAutoUpdateCli, + getUpdateNotice, refreshUpdateCheckCache, + takeUpdateNotice, } from "../src/lib/update.js"; const require = createRequire(import.meta.url); @@ -30,7 +31,7 @@ afterEach(async () => { }); describe("CLI auto-update", () => { - it("uses a fresh cache to print an update notice without hitting the network", async () => { + it("shows the update notice on root help from a fresh cache without hitting the network", async () => { const cacheDir = await createTempDir("browse-update-cache-"); const cachePath = join(cacheDir, "update-check.json"); await writeUpdateCache(cachePath, { @@ -38,7 +39,7 @@ describe("CLI auto-update", () => { version: "99.0.0", }); - const result = await runCli(["status"], { + const result = await runCli(["--help"], { env: { BROWSE_CACHE_DIR: cacheDir, BROWSE_DISABLE_UPDATE_CHECK: "0", @@ -47,17 +48,107 @@ describe("CLI auto-update", () => { }); expect(result.exitCode).toBe(0); - expect(JSON.parse(result.stdout)).toMatchObject({ + expect(result.stderr).toContain( + `Update available: ${cliVersion} -> 99.0.0.`, + ); + expect(result.stderr).toContain("Run:\n npm i -g browse@latest"); + }); + + it("shows the update notice on regular commands, deduped within the notify interval", async () => { + const cacheDir = await createTempDir("browse-update-once-"); + const cachePath = join(cacheDir, "update-check.json"); + await writeUpdateCache(cachePath, { + checkedAt: new Date().toISOString(), + version: "99.0.0", + }); + const env = { + BROWSE_CACHE_DIR: cacheDir, + BROWSE_DISABLE_UPDATE_CHECK: "0", + BROWSE_DAEMON_DIR: cacheDir, + }; + + const first = await runCli(["status"], { env }); + expect(first.exitCode).toBe(0); + expect(JSON.parse(first.stdout)).toMatchObject({ browserConnected: false, session: "default", }); - expect(result.stderr).toContain( + expect(first.stderr).toContain( `Update available: ${cliVersion} -> 99.0.0.`, ); - expect(result.stderr).toContain("Run:\n npm i -g browse@latest"); + + const second = await runCli(["status"], { env }); + expect(second.exitCode).toBe(0); + expect(second.stderr).not.toContain("Update available:"); }); - it("compares prerelease identifiers with ASCII ordering", async () => { + it("reminds again after the notify interval until the user upgrades", async () => { + const cacheDir = await createTempDir("browse-update-renotify-"); + const cachePath = join(cacheDir, "update-check.json"); + const env = { + ...process.env, + BROWSE_DISABLE_UPDATE_CHECK: "0", + BROWSE_UPDATE_CHECK_FILE: cachePath, + }; + + await writeUpdateCache(cachePath, { + checkedAt: new Date().toISOString(), + version: "99.0.0", + }); + expect(await takeUpdateNotice("1.0.0", env)).toContain("99.0.0"); + expect(await takeUpdateNotice("1.0.0", env)).toBeNull(); + + // 21h-old lastNotifiedAt (past the 20h interval) -> reminds again. + await writeUpdateCache(cachePath, { + checkedAt: new Date().toISOString(), + lastNotifiedAt: new Date(Date.now() - 21 * 60 * 60 * 1000).toISOString(), + version: "99.0.0", + }); + expect(await takeUpdateNotice("1.0.0", env)).toContain("99.0.0"); + expect(await takeUpdateNotice("1.0.0", env)).toBeNull(); + + // Upgraded -> silence even past the interval. + await writeUpdateCache(cachePath, { + checkedAt: new Date().toISOString(), + lastNotifiedAt: new Date(Date.now() - 21 * 60 * 60 * 1000).toISOString(), + version: "99.0.0", + }); + expect(await takeUpdateNotice("99.0.0", env)).toBeNull(); + }); + + it("never repeats the push notice when the cache is unwritable", async () => { + const cacheDir = await createTempDir("browse-update-unwritable-"); + const cachePath = join(cacheDir, "update-check.json", "nested.json"); + + const env = { + ...process.env, + BROWSE_DISABLE_UPDATE_CHECK: "0", + BROWSE_UPDATE_CHECK_FILE: cachePath, + }; + + // A file standing in for the parent directory makes the path unwritable. + await writeFile(join(cacheDir, "update-check.json"), "not a directory"); + expect(await takeUpdateNotice("1.0.0", env)).toBeNull(); + }); + + it("does not repeat the push notice after help already showed it", async () => { + const cacheDir = await createTempDir("browse-update-pullmark-"); + const cachePath = join(cacheDir, "update-check.json"); + const env = { + ...process.env, + BROWSE_DISABLE_UPDATE_CHECK: "0", + BROWSE_UPDATE_CHECK_FILE: cachePath, + }; + + await writeUpdateCache(cachePath, { + checkedAt: new Date().toISOString(), + version: "99.0.0", + }); + expect(await getUpdateNotice("1.0.0", env)).toContain("99.0.0"); + expect(await takeUpdateNotice("1.0.0", env)).toBeNull(); + }); + + it("returns no notice for prerelease identifiers with ASCII ordering", async () => { const cacheDir = await createTempDir("browse-update-prerelease-"); const cachePath = join(cacheDir, "update-check.json"); await writeUpdateCache(cachePath, { @@ -65,21 +156,13 @@ describe("CLI auto-update", () => { version: "1.0.0-beta.B", }); - const stderrSpy = vi - .spyOn(process.stderr, "write") - .mockImplementation(() => true); - - try { - await maybeAutoUpdateCli("1.0.0-beta.b", { - ...process.env, - BROWSE_DISABLE_UPDATE_CHECK: "0", - BROWSE_UPDATE_CHECK_FILE: cachePath, - }); + const notice = await getUpdateNotice("1.0.0-beta.b", { + ...process.env, + BROWSE_DISABLE_UPDATE_CHECK: "0", + BROWSE_UPDATE_CHECK_FILE: cachePath, + }); - expect(stderrSpy).not.toHaveBeenCalled(); - } finally { - stderrSpy.mockRestore(); - } + expect(notice).toBeNull(); }); it("refreshes the update cache from the npm registry", async () => { @@ -115,7 +198,7 @@ describe("CLI auto-update", () => { }); }); - it("treats stale cache entries as refreshes instead of notifying immediately", async () => { + it("does not notify from a stale cache even on root help", async () => { const cacheDir = await createTempDir("browse-update-stale-"); const cachePath = join(cacheDir, "update-check.json"); await writeUpdateCache(cachePath, { @@ -123,7 +206,7 @@ describe("CLI auto-update", () => { version: "98.0.0", }); - const result = await runCli(["status"], { + const result = await runCli(["--help"], { env: { BROWSE_CACHE_DIR: cacheDir, BROWSE_DISABLE_UPDATE_CHECK: "0", @@ -132,10 +215,6 @@ describe("CLI auto-update", () => { }); expect(result.exitCode).toBe(0); - expect(JSON.parse(result.stdout)).toMatchObject({ - browserConnected: false, - session: "default", - }); expect(result.stderr).not.toContain("Update available:"); }); }); diff --git a/packages/cli/tests/skill-nudge.test.ts b/packages/cli/tests/skill-nudge.test.ts new file mode 100644 index 000000000..e746ba8eb --- /dev/null +++ b/packages/cli/tests/skill-nudge.test.ts @@ -0,0 +1,86 @@ +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { maybeNudgeInstallSkill } from "../src/lib/skill-nudge.js"; + +const cleanupPaths: string[] = []; +let stderrSpy: ReturnType; + +beforeEach(() => { + stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); +}); + +afterEach(async () => { + stderrSpy.mockRestore(); + vi.unstubAllEnvs(); + + while (cleanupPaths.length > 0) { + const path = cleanupPaths.pop(); + if (!path) continue; + await rm(path, { recursive: true, force: true }); + } +}); + +async function createTempHome(withSkill: boolean): Promise { + const home = await mkdtemp(join(tmpdir(), "browse-nudge-home-")); + cleanupPaths.push(home); + if (withSkill) { + await mkdir(join(home, ".agents", "skills", "browse"), { + recursive: true, + }); + } + return home; +} + +function nudgeEnv(overrides: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { + return { + BROWSE_DISABLE_SKILL_NUDGE: "0", + CI: "0", + NODE_ENV: "production", + ...overrides, + }; +} + +function stderrText(): string { + return stderrSpy.mock.calls.map((call) => String(call[0])).join(""); +} + +describe("maybeNudgeInstallSkill (session start)", () => { + it("nudges when the skill is absent", async () => { + const home = await createTempHome(false); + vi.stubEnv("HOME", home); + await maybeNudgeInstallSkill(nudgeEnv()); + expect(stderrText()).toContain("browse skills install"); + }); + + it("nudges on every call while the skill is absent (one per session start)", async () => { + const home = await createTempHome(false); + vi.stubEnv("HOME", home); + await maybeNudgeInstallSkill(nudgeEnv()); + await maybeNudgeInstallSkill(nudgeEnv()); + const matches = stderrText().match(/browse skills install/g) ?? []; + expect(matches).toHaveLength(2); + }); + + it("stays silent when the skill is installed", async () => { + const home = await createTempHome(true); + vi.stubEnv("HOME", home); + await maybeNudgeInstallSkill(nudgeEnv()); + expect(stderrText()).not.toContain("browse skills install"); + }); + + it.each([ + ["BROWSE_DISABLE_SKILL_NUDGE", { BROWSE_DISABLE_SKILL_NUDGE: "1" }], + ["BB_DISABLE_SKILL_NUDGE", { BB_DISABLE_SKILL_NUDGE: "1" }], + ["CI", { CI: "true" }], + ["NODE_ENV=test", { NODE_ENV: "test" }], + ])("stays silent under %s", async (_label, overrides) => { + const home = await createTempHome(false); + vi.stubEnv("HOME", home); + await maybeNudgeInstallSkill(nudgeEnv(overrides)); + expect(stderrText()).not.toContain("browse skills install"); + }); +}); diff --git a/packages/cli/tests/skill-presence.test.ts b/packages/cli/tests/skill-presence.test.ts new file mode 100644 index 000000000..322ca9403 --- /dev/null +++ b/packages/cli/tests/skill-presence.test.ts @@ -0,0 +1,49 @@ +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { isBrowseSkillInstalled } from "../src/lib/skill-presence.js"; + +const cleanupPaths: string[] = []; + +afterEach(async () => { + while (cleanupPaths.length > 0) { + const path = cleanupPaths.pop(); + if (!path) continue; + await rm(path, { recursive: true, force: true }); + } +}); + +async function createTempHome(): Promise { + const home = await mkdtemp(join(tmpdir(), "browse-skill-home-")); + cleanupPaths.push(home); + return home; +} + +describe("isBrowseSkillInstalled", () => { + it("returns false when both canonical scopes are absent", async () => { + const home = await createTempHome(); + const project = await createTempHome(); + expect(await isBrowseSkillInstalled(home, project)).toBe(false); + }); + + it("returns true when the global ~/.agents/skills/browse exists", async () => { + const home = await createTempHome(); + const project = await createTempHome(); + await mkdir(join(home, ".agents", "skills", "browse"), { + recursive: true, + }); + expect(await isBrowseSkillInstalled(home, project)).toBe(true); + }); + + it("returns true for a project-scoped /.agents/skills/browse", async () => { + const home = await createTempHome(); + const project = await createTempHome(); + await mkdir(join(project, ".agents", "skills", "browse"), { + recursive: true, + }); + expect(await isBrowseSkillInstalled(home, project)).toBe(true); + }); +});