-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[STG-2276] feat(cli): once-per-install open nudge after cloud search/fetch #2247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
shrey150
wants to merge
2
commits into
main
Choose a base branch
from
shrey/cloud-open-nudge
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "browse": patch | ||
| --- | ||
|
|
||
| Nudge cloud search/fetch users toward `browse open`. After a successful `browse cloud search` or `browse cloud fetch`, the CLI prints a one-line, once-per-install tip to stderr pointing at `browse open <url>` — stdout stays machine-clean, the tip never fires on failures, and it can be disabled with `BROWSE_DISABLE_OPEN_NUDGE=1` (also skipped in CI and tests). The once-per-install marker lives in the CLI cache dir (`open-nudge.json`), mirroring the existing update-check/skill-nudge cache-file pattern. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| import { constants } from "node:fs"; | ||
| import { access, mkdir, writeFile } from "node:fs/promises"; | ||
| import { dirname, join } from "node:path"; | ||
|
|
||
| export const OPEN_NUDGE_HINT = | ||
| "Tip: open any of these in a live browser — browse open <url> (no API key needed locally)."; | ||
|
|
||
| const OPEN_NUDGE_MARKER_FILE = "open-nudge.json"; | ||
|
|
||
| interface OpenNudgeOptions { | ||
| cacheFile?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Once-per-install nudge from a successful `cloud search`/`cloud fetch` | ||
| * toward `browse open`. Returns the hint the first time it fires — the caller | ||
| * prints it to stderr so machine-readable stdout stays clean — and null once | ||
| * the install marker exists. Best-effort: any failure yields null so it can | ||
| * never affect CLI behavior. | ||
| */ | ||
| export async function maybeNudgeOpen( | ||
| options: OpenNudgeOptions = {}, | ||
| env: NodeJS.ProcessEnv = process.env, | ||
| ): Promise<string | null> { | ||
| if (isNudgeDisabled(env)) { | ||
| return null; | ||
| } | ||
|
|
||
| const cachePath = options.cacheFile; | ||
| if (!cachePath) { | ||
| return null; | ||
| } | ||
|
|
||
| if (await markerExists(cachePath)) { | ||
| return null; | ||
| } | ||
|
|
||
| await writeNudgeMarker(cachePath); | ||
| return OPEN_NUDGE_HINT; | ||
| } | ||
|
|
||
| /** | ||
| * Print the once-per-install `browse open` hint to stderr, keyed on a marker | ||
| * file in the CLI cache dir. Called by cloud commands after successful output. | ||
| */ | ||
| export async function writeOpenNudge( | ||
| cacheDir: string, | ||
| env: NodeJS.ProcessEnv = process.env, | ||
| ): Promise<void> { | ||
| try { | ||
| const hint = await maybeNudgeOpen( | ||
| { cacheFile: join(cacheDir, OPEN_NUDGE_MARKER_FILE) }, | ||
| env, | ||
| ); | ||
| if (hint) { | ||
| process.stderr.write(`\n${hint}\n`); | ||
| } | ||
| } catch { | ||
| // Best-effort nudges should never affect command output. | ||
| } | ||
| } | ||
|
|
||
| function isNudgeDisabled(env: NodeJS.ProcessEnv): boolean { | ||
| if ( | ||
| env.BROWSE_DISABLE_OPEN_NUDGE === "1" || | ||
| env.BB_DISABLE_OPEN_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" | ||
| ); | ||
| } | ||
|
|
||
| async function markerExists(cachePath: string): Promise<boolean> { | ||
| try { | ||
| await access(cachePath, constants.F_OK); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| async function writeNudgeMarker(cachePath: string): Promise<void> { | ||
| try { | ||
| await mkdir(dirname(cachePath), { recursive: true }); | ||
| await writeFile( | ||
| cachePath, | ||
| `${JSON.stringify({ shownAt: new Date().toISOString() })}\n`, | ||
| "utf8", | ||
| ); | ||
| } catch { | ||
| // Best-effort marker writes should never affect CLI behavior. | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { access } from "node:fs/promises"; | ||
| import { mkdtemp, rm } from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
|
|
||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| import { maybeNudgeOpen, OPEN_NUDGE_HINT } from "../src/lib/open-nudge.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 freshCacheFile(): Promise<string> { | ||
| const dir = await mkdtemp(join(tmpdir(), "browse-open-nudge-")); | ||
| cleanupPaths.push(dir); | ||
| return join(dir, "open-nudge.json"); | ||
| } | ||
|
|
||
| const enabledEnv: NodeJS.ProcessEnv = { NODE_ENV: "development", CI: "" }; | ||
|
|
||
| describe("maybeNudgeOpen", () => { | ||
| it("returns the hint once, then honors the install marker", async () => { | ||
| const cacheFile = await freshCacheFile(); | ||
| expect(await maybeNudgeOpen({ cacheFile }, enabledEnv)).toBe( | ||
| OPEN_NUDGE_HINT, | ||
| ); | ||
| await expect(access(cacheFile)).resolves.toBeUndefined(); | ||
| expect(await maybeNudgeOpen({ cacheFile }, enabledEnv)).toBeNull(); | ||
| }); | ||
|
|
||
| it("respects BROWSE_DISABLE_OPEN_NUDGE and BB_DISABLE_OPEN_NUDGE", async () => { | ||
| const cacheFile = await freshCacheFile(); | ||
| expect( | ||
| await maybeNudgeOpen( | ||
| { cacheFile }, | ||
| { ...enabledEnv, BROWSE_DISABLE_OPEN_NUDGE: "1" }, | ||
| ), | ||
| ).toBeNull(); | ||
| expect( | ||
| await maybeNudgeOpen( | ||
| { cacheFile }, | ||
| { ...enabledEnv, BB_DISABLE_OPEN_NUDGE: "1" }, | ||
| ), | ||
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it("does not nudge in CI or test environments", async () => { | ||
| const cacheFile = await freshCacheFile(); | ||
| expect( | ||
| await maybeNudgeOpen( | ||
| { cacheFile }, | ||
| { NODE_ENV: "development", CI: "true" }, | ||
| ), | ||
| ).toBeNull(); | ||
| expect( | ||
| await maybeNudgeOpen({ cacheFile }, { NODE_ENV: "test", CI: "" }), | ||
| ).toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when no cache file is configured", async () => { | ||
| expect(await maybeNudgeOpen({}, enabledEnv)).toBeNull(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.