diff --git a/src/agentMode/backends/shared/agentSystemPrompt.test.ts b/src/agentMode/backends/shared/agentSystemPrompt.test.ts index 805b88d4..d5735a9a 100644 --- a/src/agentMode/backends/shared/agentSystemPrompt.test.ts +++ b/src/agentMode/backends/shared/agentSystemPrompt.test.ts @@ -100,11 +100,17 @@ describe("buildAgentSystemPrompt", () => { const nonPlus = buildAgentSystemPrompt(); expect(nonPlus).toContain(COPILOT_PLUS_TOOLS_STEERING); expect(nonPlus).toContain("copilot-web-search"); + expect(nonPlus).toContain("copilot-web-fetch"); expect(nonPlus).toContain("copilot-read-pdf"); expect(nonPlus).toContain("copilot-youtube-transcript"); expect(nonPlus).toContain("copilot-fetch-x"); - // Fallback clause so a missing/unlicensed skill never dead-ends. - expect(nonPlus).toMatch(/fall back to whatever equivalent capability/i); + // Fallback clause so a missing/unlicensed skill never dead-ends or blocks + // a free user — it routes the agent to its own equivalent tool instead. + expect(nonPlus).toMatch(/silently fall back to your own equivalent tool/i); + expect(nonPlus).toMatch(/never refuse and never block the user/i); + // Fallback also covers a skill that runs but fails for this request (e.g. a + // page the relay can't fetch), so a single bad input doesn't dead-end it. + expect(nonPlus).toMatch(/fails for this particular request/i); // A Plus user gets the same steering. updateSetting("isPlusUser", true); diff --git a/src/agentMode/backends/shared/agentSystemPrompt.ts b/src/agentMode/backends/shared/agentSystemPrompt.ts index 4dfb9807..3c7e9426 100644 --- a/src/agentMode/backends/shared/agentSystemPrompt.ts +++ b/src/agentMode/backends/shared/agentSystemPrompt.ts @@ -44,24 +44,25 @@ import { getSettings } from "@/settings/model"; import { getDisableBuiltinSystemPrompt } from "@/system-prompts/state"; import { getEffectiveUserPrompt } from "@/system-prompts/systemPromptBuilder"; /** - * Steers the agent toward the bundled Copilot Plus skills for the four relay + * Steers the agent toward the bundled Copilot Plus skills for the relay * capabilities (see `skills/builtin/builtinSkills.ts`) instead of its own * built-in web/fetch tools, with an explicit fallback so the request never * dead-ends. Sent to every user regardless of Plus status: if a skill can't - * run (no license, self-host, disabled, missing), its script exits with the - * upgrade notice and the fallback clause routes the agent back to its own - * tools — so steering is safe for everyone. + * run (no license, self-host, disabled, missing), its script exits telling the + * agent to use its own equivalent tools, and the fallback clause routes it + * there — so steering never blocks free users and is safe for everyone. */ export const COPILOT_PLUS_TOOLS_STEERING = `## Copilot Plus tools For these requests, prefer the bundled Copilot skill over any built-in tool of your own: - Searching the web → the \`copilot-web-search\` skill +- Fetching or reading a specific web page → the \`copilot-web-fetch\` skill - Reading a PDF file → the \`copilot-read-pdf\` skill - Getting a YouTube video's transcript → the \`copilot-youtube-transcript\` skill - Fetching an X (Twitter) post → the \`copilot-fetch-x\` skill Each skill ships both a \`.sh\` and a \`.mjs\` script. Run the \`.sh\` with \`sh\` first; if the platform can't run \`sh\` (for example, Windows without Git Bash), run the matching \`.mjs\` with \`node\` instead. If neither \`sh\` nor \`node\` is available, tell the user to install Node.js from https://nodejs.org and try again. -If the matching skill is missing, disabled, or reports that it needs an active Copilot Plus license, fall back to whatever equivalent capability you have (or tell the user it's unavailable) — don't refuse the request.`; +If a skill is missing, disabled, reports that Copilot Plus is not active, or fails for this particular request (for example a page it can't fetch or any other relay error), silently fall back to your own equivalent tool (or, if you have none for that task, tell the user it's unavailable) and complete the request — never refuse and never block the user on upgrading. Only pass along an upgrade or renewal note when the skill's own message explicitly invites it, and keep any such mention brief and occasional.`; /** * Steers the agent toward the bundled `miyo-search` skill for vault search. A @@ -137,8 +138,8 @@ export function buildAgentSystemPrompt(): string { // Always steer toward the builtin Copilot Plus skills, regardless of Plus // status. Gating on `isPlusUser` would be wrong anyway — valid self-host // mode is Plus-enabled but reports `isPlusUser: false` — and if a skill - // can't run, its script exits with the upgrade notice and the fallback - // clause routes the agent back to its own tools. Safe for everyone. + // can't run, its script exits telling the agent to use its own equivalent + // tools and the fallback clause routes it there. Never blocks free users. parts.push(COPILOT_PLUS_TOOLS_STEERING); // Miyo steering is gated on the same `shouldUseMiyo` check that seeds the // skill, so we only point the agent at `miyo-search` when it's actually diff --git a/src/agentMode/skills/builtin/builtinSkills.test.ts b/src/agentMode/skills/builtin/builtinSkills.test.ts index cb77dd91..5ca2c548 100644 --- a/src/agentMode/skills/builtin/builtinSkills.test.ts +++ b/src/agentMode/skills/builtin/builtinSkills.test.ts @@ -10,9 +10,10 @@ function scriptOf(name: string, ext: ".sh" | ".mjs" = ".sh"): string { } describe("builtin Copilot Plus skills", () => { - it("ships the four Copilot-branded skills, each fanned out to all three agents", () => { + it("ships the five Copilot-branded skills, each fanned out to all three agents", () => { expect(BUILTIN_SKILLS.map((s) => s.name)).toEqual([ "copilot-web-search", + "copilot-web-fetch", "copilot-read-pdf", "copilot-youtube-transcript", "copilot-fetch-x", @@ -53,8 +54,8 @@ describe("builtin Copilot Plus skills", () => { // Auth flows through the env var, not a literal embedded key. expect(sh).toContain("Authorization: Bearer $KEY"); expect(sh).toContain("X-Client-Version: $CLIENT_VERSION"); - // Guard + upgrade prompt when the license/relay config is absent. - expect(sh).toContain('[ -n "$KEY" ] && [ -n "$BASE" ] || die "$UPGRADE"'); + // Guard + soft fallback when the license/relay config is absent. + expect(sh).toContain('[ -n "$KEY" ] && [ -n "$BASE" ] || no_license'); expect(sh).toContain("Copilot Plus"); const mjs = scriptOf(skill.name, ".mjs"); @@ -64,11 +65,53 @@ describe("builtin Copilot Plus skills", () => { expect(mjs).toContain('Authorization: "Bearer " + KEY'); expect(mjs).toContain('"X-Client-Version": CLIENT_VERSION'); // Same license guard as the shell script. - expect(mjs).toContain("if (!KEY || !BASE) die(UPGRADE);"); + expect(mjs).toContain("if (!KEY || !BASE) noLicense();"); expect(mjs).toContain("Copilot Plus"); } }); + it("falls back to the agent's own tools instead of blocking when Plus is absent", () => { + for (const skill of BUILTIN_SKILLS) { + const sh = scriptOf(skill.name, ".sh"); + // No license: tell the agent to use its own equivalent tools, never + // refuse, and only append the upsell occasionally (gated on the pid). The + // fallback wording is generic (not web-specific) so it suits the PDF skill + // too, which shares this message. + expect(sh).toContain("your own equivalent built-in tools"); + expect(sh).not.toContain("web tools"); + expect(sh).toContain("never refuse"); + expect(sh).toContain("$(( $$ % 4 ))"); + // The upsell carries the actionable instruction to obtain a license key. + expect(sh).toContain("get a license key at https://www.obsidiancopilot.com"); + // The invalid/expired-license (401/403) path is distinct and warrants a + // renewal note, but still falls back rather than refusing. + expect(sh).toContain('401|403) die "$LICENSE_INVALID"'); + expect(sh).toContain("renew their Copilot Plus license"); + // The old hard "requires Copilot Plus / upgrade" block is gone. + expect(sh).not.toContain("require Copilot Plus"); + + // A non-license relay failure (unreachable, or a non-2xx that isn't + // 401/403 — e.g. a page that can't be fetched) still routes the agent to + // its own tool rather than dead-ending the request. + expect(sh).toContain("$RELAY_FAILED_FALLBACK"); + expect(sh).toContain("your own equivalent built-in tool for this"); + + const mjs = scriptOf(skill.name, ".mjs"); + expect(mjs).toContain("your own equivalent built-in tools"); + expect(mjs).not.toContain("web tools"); + expect(mjs).toContain("process.pid % 4 === 0"); + expect(mjs).toContain("die(LICENSE_INVALID)"); + expect(mjs).toContain("RELAY_FAILED_FALLBACK"); + } + }); + + it("includes the firecrawl-backed web-fetch skill targeting /url4llm", () => { + expect(scriptOf("copilot-web-fetch", ".sh")).toContain('relay "/url4llm"'); + expect(scriptOf("copilot-web-fetch", ".sh")).toContain('\\"url\\"'); + expect(scriptOf("copilot-web-fetch", ".mjs")).toContain('await relay("/url4llm"'); + expect(scriptOf("copilot-web-fetch", ".mjs")).toContain("url: ARG, user_id: USER_ID"); + }); + it("maps each relay tool to its endpoint and request body (both scripts)", () => { expect(scriptOf("copilot-web-search", ".sh")).toContain('relay "/websearch"'); expect(scriptOf("copilot-web-search", ".sh")).toContain('\\"query\\"'); diff --git a/src/agentMode/skills/builtin/builtinSkills.ts b/src/agentMode/skills/builtin/builtinSkills.ts index 1bdb1d84..b3c0a548 100644 --- a/src/agentMode/skills/builtin/builtinSkills.ts +++ b/src/agentMode/skills/builtin/builtinSkills.ts @@ -2,17 +2,19 @@ import type { BackendId } from "@/agentMode/session/types"; /** * Plugin-shipped ("builtin") Agent Mode skills that wrap Copilot Plus relay - * capabilities (web search, PDF, YouTube, X). Unlike user-authored skills, - * these are seeded into the canonical skills folder by the plugin (see + * capabilities (web search, web fetch, PDF, YouTube, X). Unlike user-authored + * skills, these are seeded into the canonical skills folder by the plugin (see * `seedBuiltinSkills`) and refreshed when `version` bumps. * * Each skill ships a `SKILL.md` (instructions the agent reads) plus two * runnable scripts: a POSIX `sh` script (run with `curl`) and an equivalent * Node `.mjs` script. Both read the Copilot Plus license + relay base URL from * env vars the plugin injects at spawn time (see `buildCopilotPlusEnv`) and call - * the Brevilabs relay directly — no key is embedded in the skill files. A - * missing/invalid license makes the script exit non-zero with an upgrade prompt - * the agent relays to the user. + * the Brevilabs relay directly — no key is embedded in the skill files. When no + * license is configured (free user) or the relay rejects it, the script exits + * non-zero with a message that tells the agent to fall back to its own + * equivalent built-in capability — never to block the user — with only an + * occasional, gentle upsell. * * Why ship both an `sh` and a Node script: `sh` + `curl` is preferred because * the script runs in the *agent's* shell, where `sh`, `curl`, `sed`, and @@ -48,8 +50,42 @@ export const PLUS_ENV = { clientVersion: "COPILOT_CLIENT_VERSION", } as const; -const UPGRADE_MESSAGE = - "This is a Copilot Plus feature and needs an active license. Tell the user that web/PDF/YouTube/X tools require Copilot Plus, and to upgrade or renew at https://www.obsidiancopilot.com (then add their license key in Settings → Copilot Plus)."; +/** + * No Copilot Plus license is configured — the free-user case (a non-Plus user + * gets an empty env from `buildCopilotPlusEnv`, so `KEY`/`BASE` are absent). + * The skill simply isn't available; we must NOT block the user. The message + * tells the agent to quietly use its own equivalent capability and never refuse + * or push an upgrade. It stays generic about the fallback (the same message is + * shared by the web, PDF, YouTube, and X skills, so it must not assume a "web" + * recovery path). The upsell lives in a separate sentence the scripts append + * only some of the time (see `no_license`/`noLicense`) so the nudge stays + * occasional. + */ +const NO_LICENSE_MESSAGE = + "Copilot Plus is not active, so this skill is unavailable — do not retry it. Complete the request using your own equivalent built-in tools if you have them; otherwise tell the user it's unavailable. Never refuse or tell the user they are blocked."; + +/** Optional, occasional upsell appended to {@link NO_LICENSE_MESSAGE}. */ +const NO_LICENSE_UPSELL = + "You may also add one short, friendly note that Copilot Plus offers higher-quality web, PDF, YouTube, and X tools — get a license key at https://www.obsidiancopilot.com to access them."; + +/** + * The relay rejected the license (HTTP 401/403): the user HAD a key that is no + * longer valid (lapsed/expired Plus). Same graceful fallback as the free case, + * but here a renewal nudge is warranted because they were a paying user. + */ +const LICENSE_INVALID_MESSAGE = + "Your Copilot Plus license is inactive or expired, so this skill is unavailable — do not retry it. Complete the request using your own equivalent built-in tools if you have them; otherwise tell the user it's unavailable, and never refuse. You may briefly let the user know they can renew their Copilot Plus license at https://www.obsidiancopilot.com to restore the higher-quality versions of these tools."; + +/** + * The license is valid but the relay couldn't complete THIS request — the relay + * was unreachable, or it returned a non-2xx that isn't a 401/403 (e.g. the page + * a fetch targets is blocked, a video has no transcript, a transient 5xx). Since + * the steering routes these tasks away from the agent's own tools, the error + * must invite a fallback so a single bad URL/input doesn't dead-end a request + * the native tool could still complete. Appended after the concrete HTTP detail. + */ +const RELAY_FAILED_FALLBACK = + "If you have your own equivalent built-in tool for this, use it to complete the request; otherwise tell the user it could not be completed."; /** Wrap a string as a single-quoted shell literal (safe for embedding in `sh`). */ function shSingleQuote(value: string): string { @@ -58,9 +94,10 @@ function shSingleQuote(value: string): string { /** * Shared preamble every script uses: resolves env, defines the relay caller, - * and exits with the upgrade prompt when the license/relay config is absent. - * Kept inline in each `.sh` (scripts can't share an import once symlinked into - * agent dirs). + * and — when the license/relay config is absent — exits non-zero telling the + * agent to fall back to its own equivalent capability (with an occasional + * gentle upsell) rather than blocking the user. Kept inline in each `.sh` + * (scripts can't share an import once symlinked into agent dirs). * * `json_escape` covers single-line string values (backslash + double quote); * queries, URLs, and file paths never contain raw newlines, so this is enough @@ -76,14 +113,26 @@ BASE="\${${PLUS_ENV.baseUrl}:-}" KEY="\${${PLUS_ENV.licenseKey}:-}" USER_ID="\${${PLUS_ENV.userId}:-}" CLIENT_VERSION="\${${PLUS_ENV.clientVersion}:-}" -UPGRADE=${shSingleQuote(UPGRADE_MESSAGE)} +NO_LICENSE=${shSingleQuote(NO_LICENSE_MESSAGE)} +NO_LICENSE_UPSELL=${shSingleQuote(NO_LICENSE_UPSELL)} +LICENSE_INVALID=${shSingleQuote(LICENSE_INVALID_MESSAGE)} +RELAY_FAILED_FALLBACK=${shSingleQuote(RELAY_FAILED_FALLBACK)} die() { printf '%s\\n' "$1" >&2 exit "\${2:-2}" } -[ -n "$KEY" ] && [ -n "$BASE" ] || die "$UPGRADE" +# No Copilot Plus license configured (free user). Don't block them: tell the +# agent to use its own equivalent tools, appending the upsell only ~1 in 4 runs (keyed +# off the process id) so the nudge stays occasional instead of firing every call. +no_license() { + msg="$NO_LICENSE" + [ $(( $$ % 4 )) -eq 0 ] && msg="$msg $NO_LICENSE_UPSELL" + die "$msg" +} + +[ -n "$KEY" ] && [ -n "$BASE" ] || no_license # JSON-escape a single-line string: backslash first, then double quote. json_escape() { @@ -98,13 +147,13 @@ relay() { -H "Authorization: Bearer $KEY" \\ -H "X-Client-Version: $CLIENT_VERSION" \\ --data-binary @-) - [ $? -eq 0 ] || die "Could not reach the Copilot relay." 1 + [ $? -eq 0 ] || die "Could not reach the Copilot relay. $RELAY_FAILED_FALLBACK" 1 code=$(printf '%s' "$resp" | tail -n1) out=$(printf '%s' "$resp" | sed '$d') case "$code" in - 401|403) die "$UPGRADE" ;; + 401|403) die "$LICENSE_INVALID" ;; 2*) printf '%s\\n' "$out" ;; - *) die "Request failed (HTTP $code): $out" 1 ;; + *) die "Request failed (HTTP $code): $out. $RELAY_FAILED_FALLBACK" 1 ;; esac } `; @@ -119,9 +168,9 @@ relay() { * per-skill tail use top-level `await`. * * Behaviour mirrors the shell script exactly: same env vars, same relay call, - * same 401/403 → upgrade-prompt mapping, same non-zero exits. If the runtime is - * older than Node 18 (`fetch` undefined) it exits with a "update Node" message - * rather than failing obscurely. + * same no-license / 401/403 → fall-back-to-your-own-tools mapping, same + * non-zero exits. If the runtime is older than Node 18 (`fetch` undefined) it + * exits with a "update Node" message rather than failing obscurely. */ function nodeScriptPreamble(): string { return `#!/usr/bin/env node @@ -133,14 +182,26 @@ const BASE = process.env.${PLUS_ENV.baseUrl} || ""; const KEY = process.env.${PLUS_ENV.licenseKey} || ""; const USER_ID = process.env.${PLUS_ENV.userId} || ""; const CLIENT_VERSION = process.env.${PLUS_ENV.clientVersion} || ""; -const UPGRADE = ${JSON.stringify(UPGRADE_MESSAGE)}; +const NO_LICENSE = ${JSON.stringify(NO_LICENSE_MESSAGE)}; +const NO_LICENSE_UPSELL = ${JSON.stringify(NO_LICENSE_UPSELL)}; +const LICENSE_INVALID = ${JSON.stringify(LICENSE_INVALID_MESSAGE)}; +const RELAY_FAILED_FALLBACK = ${JSON.stringify(RELAY_FAILED_FALLBACK)}; function die(message, code = 2) { process.stderr.write(String(message) + "\\n"); process.exit(code); } -if (!KEY || !BASE) die(UPGRADE); +// No Copilot Plus license configured (free user). Don't block them: tell the +// agent to use its own equivalent tools, appending the upsell only ~1 in 4 runs (keyed +// off the process id) so the nudge stays occasional instead of firing every call. +function noLicense() { + let msg = NO_LICENSE; + if (process.pid % 4 === 0) msg += " " + NO_LICENSE_UPSELL; + die(msg); +} + +if (!KEY || !BASE) noLicense(); // relay(endpoint, body) -> prints the response body, mapping HTTP status. async function relay(endpoint, body) { @@ -159,14 +220,14 @@ async function relay(endpoint, body) { body: JSON.stringify(body), }); } catch { - die("Could not reach the Copilot relay.", 1); + die("Could not reach the Copilot relay. " + RELAY_FAILED_FALLBACK, 1); } const out = await resp.text(); - if (resp.status === 401 || resp.status === 403) die(UPGRADE); + if (resp.status === 401 || resp.status === 403) die(LICENSE_INVALID); if (resp.status >= 200 && resp.status < 300) { process.stdout.write(out + "\\n"); } else { - die("Request failed (HTTP " + resp.status + "): " + out, 1); + die("Request failed (HTTP " + resp.status + "): " + out + ". " + RELAY_FAILED_FALLBACK, 1); } } `; @@ -207,11 +268,27 @@ from https://nodejs.org and run the command again.${opts.extraNote ? ` ${opts.ex Both scripts print the result to stdout.`; } +/** + * The SKILL.md "if Copilot Plus isn't active" section shared by every builtin + * relay skill. Mirrors the scripts' runtime behaviour: when Plus is unavailable + * the agent must fall back to its own tools rather than block the user, and the + * upgrade/renewal nudge is gentle and occasional (driven by the script message, + * not repeated on the agent's own initiative). + */ +const LICENSE_PROBLEM_SECTION = `## If Copilot Plus is not active + +If the script exits saying Copilot Plus is unavailable, do NOT retry it. Do what +the message says: fall back to your own equivalent built-in capability to handle +the request when you have one (otherwise tell the user it's unavailable) — never +refuse or block the user. Only mention upgrading or renewing Copilot Plus when +the script's message explicitly invites it, and keep any such note short and +friendly.`; + /** * Build a skill that maps a single positional argument onto one relay - * endpoint (the web/YouTube/X tools are identical apart from the endpoint, - * argument name, and copy). PDF is hand-written below because it reads a - * local file and base64-encodes it before calling the relay. + * endpoint (the web search/fetch, YouTube, and X tools are identical apart from + * the endpoint, argument name, and copy). PDF is hand-written below because it + * reads a local file and base64-encodes it before calling the relay. */ function relaySkill(opts: { name: string; @@ -226,7 +303,7 @@ function relaySkill(opts: { }): BuiltinSkill { const [argKey, argPlaceholder] = opts.arg; const nodeScriptFile = opts.scriptFile.replace(/\.sh$/, ".mjs"); - const version = 3; + const version = 4; return { name: opts.name, version, @@ -246,11 +323,7 @@ ${opts.intro} ${howToRunSection({ shFile: opts.scriptFile, nodeFile: nodeScriptFile, argPlaceholder })} -## If it reports a license problem - -If the script exits with a message about Copilot Plus, do NOT retry. Tell the -user this capability needs an active Copilot Plus license and where to upgrade -or renew — then continue without it. +${LICENSE_PROBLEM_SECTION} `, files: [ { @@ -284,7 +357,18 @@ const WEB_SEARCH = relaySkill({ scriptFile: "web-search.sh", }); -const READ_PDF_VERSION = 4; +const WEB_FETCH = relaySkill({ + name: "copilot-web-fetch", + description: + "Fetch and read the full contents of a specific web page (URL) as clean Markdown using Copilot Plus. Use when the user shares a link or asks you to open, read, or summarize a particular page — not for an open-ended web search. Requires an active Copilot Plus license; without it, use your own fetch tool instead.", + heading: "Copilot web fetch", + intro: "Fetch a web page's contents as Markdown through Copilot Plus.", + endpoint: "/url4llm", + arg: ["url", ""], + scriptFile: "web-fetch.sh", +}); + +const READ_PDF_VERSION = 5; const READ_PDF: BuiltinSkill = { name: "copilot-read-pdf", version: READ_PDF_VERSION, @@ -310,11 +394,7 @@ ${howToRunSection({ extraNote: "Pass an absolute path to the PDF file.", })} -## If it reports a license problem - -If the script exits with a message about Copilot Plus, do NOT retry. Tell the -user this capability needs an active Copilot Plus license and where to upgrade -or renew — then continue without it. +${LICENSE_PROBLEM_SECTION} `, files: [ { @@ -373,6 +453,7 @@ const FETCH_X = relaySkill({ /** All plugin-shipped Copilot Plus relay skills, in display order. */ export const BUILTIN_SKILLS: readonly BuiltinSkill[] = [ WEB_SEARCH, + WEB_FETCH, READ_PDF, YOUTUBE_TRANSCRIPT, FETCH_X,