[STG-2192] fix(cli): make browse skills add failures diagnosable + fail cleanly on unknown skill

[shrey150]

Summary

browse skills add <domain>/<task> fails for a large share of users with an opaque EEXIT 1 and no diagnostic detail. Root cause: when the catalog file API returns 404, the CLI silently fell back to git-cloning the entire browse.sh repo, found no matching skill, printed "No matching skills found" and exited 1 — and because the child was spawned with stdio: "inherit", the real error text was never captured, so telemetry recorded a bare numeric exit code.

This PR makes those failures measurable and friendlier:

• Capture child output. spawnPassthrough now pipes + echoes the npx skills add child's stdout/stderr to the terminal (live UX preserved) while buffering the last ~2KB. On a nonzero exit, that tail flows into the error message and telemetry instead of a bare exit code.
• Fail cleanly on unknown skills. A definitive 404 for a non-generated id now fails fast with an actionable message (Skill "<id>" not found in the catalog. Run \browse skills find ` ...`) — no more cloning the whole repo. Suffix-shaped (generated) ids and unavailable-catalog cases keep the browse.sh clone fallback (still covered by existing tests).
• Distinct telemetry result codes: skill_not_found, invalid_skill_id, npx_missing, skill_install_failed.
• Better missing-arg UX. browse skills add with no arg now prints guidance pointing at the <domain>/<task> form and browse skills find, instead of oclif's bare "Missing 1 required arg".

result_code was already wired through base.ts → recordCommandError → commandCompletedProperties; the gap was that the failing paths never set a meaningful code (they returned a numeric exit that surfaced as a generic oclif usage error). This PR routes each failure through fail(msg, exitCode, { resultCode }) so the codes are populated.

Linear: https://linear.app/browserbase/issue/STG-2192

E2E Test Matrix

Run against the locally built browse (node packages/cli/bin/run.js) with a local PostHog capture server (BROWSERBASE_TELEMETRY_HOST=http://127.0.0.1:<port>). result_code values are the real captured cli.command_completed properties.

Command / flow	Observed output	Confidence / sufficiency
browse skills add amazon.com/buy-something-fake (hits real browse.sh, 404)	exit 1; stderr: Skill "amazon.com/buy-something-fake" not found in the catalog. Run \browse skills find amazon.com` ...; telemetry result_code=skill_not_found, error_type=runtime`; no git clone	Proves the core fix: clean message + distinct code, no opaque clone. Hits the live API.
browse skills add (no arg)	exit 2; stderr: Missing skill id. Pass a skill in the form <domain>/<task>, e.g. \browse skills add yelp.com/extract-reviews`. Run `browse skills find ` ...; telemetry result_code=invalid_skill_id`	Proves the improved missing-arg UX + code.
browse skills add ../bad	exit 1; stderr: Invalid skill id "../bad". Use <domain>/<task>.; telemetry result_code=invalid_skill_id	Proves parse-failure code wiring.
browse skills add yelp.com/extract-reviews-2ikb22 with stub npx exiting 7	exit 7; live stderr npx says: simulated registry boom passed through; fail() message Could not install skill: npx says: simulated registry boom; telemetry result_code=skill_install_failed, success=false	Proves child output is captured (not discarded) AND flows into telemetry with the correct code while exit code is preserved.
browse skills add yelp.com/extract-reviews-2ikb22 (real npx, isolated HOME/XDG_CONFIG_HOME)	Downloaded 1 skill file to <temp>/browserbase/skills/yelp.com/extract-reviews-2ikb22 (SKILL.md present); npx skills add <localpath> then failed only due to this machine's unreachable npm registry mirror (socket-firewall... ENOTFOUND) — surfaced via the new skill_install_failed path	Proves the real browse.sh download stage works end-to-end against the live API; the npx skills add <localpath> step is independently covered by the passing vitest stub test.
pnpm --filter browse test (vitest)	210 passed (15 files), incl. new skill_not_found and missing-arg tests	Full suite green, no happy-path regression (download test still asserts --yes skills add <installPath>).
pnpm --filter browse lint (prettier + eslint + tsc)	clean	Supporting: format/lint/typecheck pass.

Changeset

.changeset/skills-add-failure-telemetry.md → "browse": patch (consumed by the dedicated release-cli.yml CLI release flow, which scans for "browse" changesets).

🤖 Generated with Claude Code

---

Summary by cubic

Improves browse skills add <domain>/<task> with clear, actionable errors and measurable telemetry, and stops cloning the entire repo on unknown skills. Addresses Linear STG-2192.

• Bug Fixes
  • Streams and buffers the last ~2KB of npx skills add output; on nonzero exit or spawn errors, includes the tail/OS error in the message and telemetry as skill_install_failed.
  • Treats catalog 404s for non-generated IDs as a clean failure with guidance (e.g., use browse skills find <domain>), avoiding cloning browserbase/browse.sh.
  • Keeps GitHub fallback for suffix-shaped IDs or when the catalog API is unavailable.
  • Adds distinct telemetry result codes: skill_not_found, invalid_skill_id, npx_missing, skill_install_failed.
  • Replaces the generic missing-arg error with guidance on <domain>/<task> and browse skills find.

^{Written for commit 84d9040. Summary will update on new commits.}

[changeset-bot]

🦋 Changeset detected

Latest commit: 84d9040

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[cubic-dev-ai]

1 issue found across 5 files

Confidence score: 4/5

• This PR is likely safe to merge with minimal risk: the reported issue is moderate (5/10) and appears limited to error classification rather than core install execution.
• In packages/cli/src/lib/skills/install.ts, errors thrown by spawnPassthrough can bypass fail(...), so failed installs may not be tagged as skill_install_failed, which can reduce consistency of downstream handling and diagnostics.
• Given the high confidence (8/10), this is worth a follow-up fix, but it does not strongly indicate a broad functional break across the reviewed changes.
• Pay close attention to packages/cli/src/lib/skills/install.ts - ensure spawnPassthrough failures are routed through fail(...) for consistent skill_install_failed classification.

Architecture diagram

sequenceDiagram
    participant User as CLI User
    participant Cmd as SkillsAdd Command
    participant Install as installSkill()
    participant Parse as parseSkillId()
    participant FetchAPI as fetchSkillFilesFromApi()
    participant GitHub as browse.sh Repo
    participant Npx as npx skills add
    participant Telemetry as Telemetry (PostHog)

    User->>Cmd: browse skills add <rawId>
    
    alt No argument
        Cmd->>Cmd: validate args
        Cmd->>Cmd: fail("Missing skill id...", 2, invalid_skill_id)
        Cmd->>Telemetry: commandCompleted (result_code=invalid_skill_id)
    end

    Cmd->>Install: installSkill(rawSkillId)
    
    Install->>Parse: parseSkillId(rawSkillId)
    
    alt Invalid format (e.g. "../bad")
        Parse->>Parse: fail("Invalid skill id...", 1, invalid_skill_id)
        Parse->>Telemetry: commandCompleted (result_code=invalid_skill_id)
    end

    alt npx not found
        Install->>Install: findExecutable("npx")
        note over Install: Checks PATH
        alt Not found
            Install->>Install: fail("npx not installed...", 1, npx_missing)
            Install->>Telemetry: commandCompleted (result_code=npx_missing)
        end
    end

    Install->>FetchAPI: fetchSkillFiles(skillId)
    
    alt API returns 404 AND id is non-generated (no suffix)
        FetchAPI->>FetchAPI: return { status: "not_found" }
        Install->>Install: fail("Skill not found...", 1, skill_not_found)
        Install->>Telemetry: commandCompleted (result_code=skill_not_found)
    else API returns 404 AND id has generated suffix
        FetchAPI->>GitHub: check directBlobSkillExists()
        alt Blob exists
            note over FetchAPI: Fallback to direct blob URL
            FetchAPI->>Install: return { status: "found", files }
        else Blob missing
            FetchAPI->>Install: return { status: "fallback" }
            Install->>Npx: runSkillsInstall(npx, [clone browse.sh --skill])
        end
    else API returns found files
        FetchAPI->>Install: return { status: "found", files }
        Install->>Install: downloadBlobSkill()
        Install->>Npx: runSkillsInstall(npx, ["--yes", "skills", "add", <localPath>])
    else API unavailable
        FetchAPI->>FetchAPI: return { status: "unavailable" }
        alt Id has generated suffix
            FetchAPI->>GitHub: check directBlobSkillExists()
            note over GitHub: Same path as API 404 + suffix
        else Non-generated id
            FetchAPI->>Install: return { status: "fallback" }
        end
    end

    Npx->>Npx: spawnPassthrough() — pipes stdout/stderr live + buffers tail
    alt npx exits 0
        Npx->>Cmd: return { exitCode: 0 }
        Cmd->>User: "Downloaded N skill file(s) to..."
        Cmd->>Telemetry: commandCompleted (success=true)
    else npx exits nonzero
        Npx->>Install: return { exitCode: N, output: "...tail..." }
        Install->>Install: fail("Could not install skill: ...", exitCode, skill_install_failed)
        Install->>User: stderr with captured detail
        Install->>Telemetry: commandCompleted (result_code=skill_install_failed)
    end

Loading

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>