refactor(claude-cli): always load extension, gate via /api/models filter

Previously the vendored @fusion/pi-claude-cli extension was conditionally
loaded based on GlobalSettings.useClaudeCli. That forced a Fusion restart
every time the user toggled the provider card — confusing UX.

Key insight: pi-claude-cli registers a NEW provider id ("pi-claude-cli")
rather than overriding "anthropic", so loading it unconditionally is
safe — direct Anthropic auth and CLI-routed models coexist peacefully.
The extension also gracefully no-ops when the `claude` binary is missing
(see packages/pi-claude-cli/index.ts:106 — the throw is caught locally).

Changes:
 - serve/daemon/dashboard: always append the resolved pi-claude-cli path
   to discoverAndLoadExtensions, no settings lookup.
 - resolveClaudeCliExtensionPaths() takes no args now; always returns the
   resolved path.
 - /api/models filter flipped: hide provider === "pi-claude-cli" when
   the toggle is OFF (previously: restricted to those models when ON).
 - POST /api/auth/claude-cli drops restartRequired semantics — toggling
   now has immediate effect on the picker.
 - Provider card UX updated to match: "Claude-CLI-routed models are
   now visible/hidden from the model picker" instead of "Restart Fusion
   to activate".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-04-23 21:09:19 -07:00
parent b354e49940
commit 8f5c60fd8c
7 changed files with 86 additions and 180 deletions

View File

@@ -1,7 +1,4 @@
import { mkdirSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { describe, expect, it } from "vitest";
import { tempWorkspace } from "@fusion/test-utils";
import {
resolveClaudeCliExtension,
resolveClaudeCliExtensionPaths,
@@ -22,40 +19,16 @@ describe("resolveClaudeCliExtension", () => {
});
describe("resolveClaudeCliExtensionPaths", () => {
it("returns empty when useClaudeCli is off (default)", () => {
const result = resolveClaudeCliExtensionPaths({});
expect(result.paths).toEqual([]);
expect(result.warning).toBeUndefined();
expect(result.resolution).toBeNull();
});
it("returns empty when useClaudeCli is explicitly false", () => {
const result = resolveClaudeCliExtensionPaths({ useClaudeCli: false });
expect(result.paths).toEqual([]);
expect(result.resolution).toBeNull();
});
it("returns empty when useClaudeCli is a non-boolean truthy value", () => {
// Defensive: API might pass strings, numbers — we only activate on true.
const result = resolveClaudeCliExtensionPaths({
useClaudeCli: "true" as unknown as boolean,
});
expect(result.paths).toEqual([]);
});
it("returns the resolved path when useClaudeCli is on", () => {
const result = resolveClaudeCliExtensionPaths({ useClaudeCli: true });
// Post-redesign (2026-04-23): the extension loads unconditionally so the
// setting only gates the `/api/models` filter, not extension registration.
// This function now takes no arguments and always returns the resolved
// workspace path — cleaner contract, no settings coupling.
it("always returns the resolved workspace path when the package is installed", () => {
const result = resolveClaudeCliExtensionPaths();
expect(result.paths).toHaveLength(1);
expect(result.paths[0]).toMatch(/pi-claude-cli[\/\\]index\.ts$/);
expect(result.resolution?.status).toBe("ok");
});
it("surfaces a warning but does not throw on weird inputs", () => {
// Exercises the defensive null/undefined/garbage handling — callers
// pass settings from disk that could be corrupt.
// @ts-expect-error intentionally bad shape
const result = resolveClaudeCliExtensionPaths(null);
expect(result.paths).toEqual([]);
expect(result.resolution.status).toBe("ok");
expect(result.warning).toBeUndefined();
});
});
@@ -70,25 +43,3 @@ describe("cached resolution roundtrip", () => {
});
});
// Directory-fixture smoke test: give the resolver a minimal "fake" package
// layout to prove it handles malformed installs gracefully. This doesn't
// use the resolver directly (it's hard-coded to look up
// @fusion/pi-claude-cli), but proves the package.json parsing logic is
// robust when we refactor later.
describe("package.json edge cases (documentation)", () => {
it("fixture layout documents what a broken install looks like", () => {
const root = tempWorkspace("claude-cli-ext-");
// This fixture is not exercised by the current implementation but
// captures the shape we'd need to test if resolveClaudeCliExtension
// accepted a custom search path. Keeping it here so the next person
// refactoring has a template.
const pkgDir = join(root, "fake", "node_modules", "@fusion", "pi-claude-cli");
mkdirSync(pkgDir, { recursive: true });
writeFileSync(
join(pkgDir, "package.json"),
JSON.stringify({ pi: { extensions: ["index.ts"] }, version: "0.0.0" }),
);
// No index.ts — would trigger missing-entry if we pointed the resolver here.
expect(true).toBe(true);
});
});

View File

@@ -103,28 +103,27 @@ export function resolveClaudeCliExtension(): ClaudeCliExtensionResolution {
}
/**
* Compute the paths to append to `discoverAndLoadExtensions`' configuredPaths
* based on the user's `useClaudeCli` setting.
* Compute the paths to append to `discoverAndLoadExtensions`' configuredPaths.
*
* When the setting is off we return no paths at all — the bundled
* `@fusion/pi-claude-cli` sits idle in node_modules and contributes nothing
* to the running pi session. Flipping the toggle on requires a server
* restart to pick up the new extension (pi has no stable runtime-reload API
* for custom provider registrations). The dashboard toggle hook surfaces
* this in its status response.
* The extension is loaded unconditionally — the provider it registers lives
* under a distinct id (`"pi-claude-cli"`, see the vendored package's
* index.ts) so coexistence with direct Anthropic auth is safe. When the
* user flips `useClaudeCli` off, the provider stays registered; the dashboard
* simply hides its models from the picker via the `/api/models` filter.
*
* `warning` is populated when resolution fails (corrupted install, missing
* entry). Callers should log it but must not fail startup — the feature is
* optional.
* This "always load" choice means toggling the setting has immediate effect
* — no Fusion restart required. If `@fusion/pi-claude-cli` itself is missing
* or broken (unusual — it's a hard workspace dep), we emit a warning and
* return no paths; pi will continue without CLI-routed models.
*
* `warning` is populated when resolution fails. Callers should log it but
* must not fail startup.
*/
export function resolveClaudeCliExtensionPaths(globalSettings: {
useClaudeCli?: unknown;
}): { paths: string[]; warning?: string; resolution: ClaudeCliExtensionResolution | null } {
const enabled = globalSettings?.useClaudeCli === true;
if (!enabled) {
return { paths: [], resolution: null };
}
export function resolveClaudeCliExtensionPaths(): {
paths: string[];
warning?: string;
resolution: ClaudeCliExtensionResolution;
} {
const resolution = resolveClaudeCliExtension();
switch (resolution.status) {
case "ok":
@@ -134,7 +133,7 @@ export function resolveClaudeCliExtensionPaths(globalSettings: {
paths: [],
resolution,
warning:
"useClaudeCli is on but @fusion/pi-claude-cli is not installed in node_modules. Run `pnpm install`.",
"@fusion/pi-claude-cli is not installed in node_modules. Run `pnpm install`.",
};
case "missing-entry":
case "error":

View File

@@ -393,22 +393,16 @@ export async function runDaemon(opts: DaemonOptions = {}) {
.filter((r) => r.enabled)
.map((r) => r.path);
const claudeCliPaths = await (async () => {
try {
const globalSettings = await store.getGlobalSettingsStore().getSettings();
const result = resolveClaudeCliExtensionPaths(globalSettings);
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
} catch (err) {
console.warn(
`[extensions] Unable to evaluate useClaudeCli setting: ${err instanceof Error ? err.message : String(err)}`,
);
setCachedClaudeCliResolution(null);
return [];
// Always load the vendored pi-claude-cli extension — see comment in
// serve.ts for rationale. The `useClaudeCli` setting only affects the
// /api/models filter, not extension registration.
const claudeCliPaths = (() => {
const result = resolveClaudeCliExtensionPaths();
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
})();
const extensionsResult = await discoverAndLoadExtensions(

View File

@@ -768,22 +768,14 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
.filter((r) => r.enabled)
.map((r) => r.path);
const claudeCliPaths = await (async () => {
try {
const globalSettings = await store.getGlobalSettingsStore().getSettings();
const result = resolveClaudeCliExtensionPaths(globalSettings);
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
} catch (err) {
console.warn(
`[extensions] Unable to evaluate useClaudeCli setting: ${err instanceof Error ? err.message : String(err)}`,
);
setCachedClaudeCliResolution(null);
return [];
// Always load the vendored pi-claude-cli extension — see serve.ts.
const claudeCliPaths = (() => {
const result = resolveClaudeCliExtensionPaths();
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
})();
// Load all enabled extensions: Fusion/Pi filesystem-discovered + package-resolved.

View File

@@ -447,25 +447,19 @@ export async function runServe(
.filter((r) => r.enabled)
.map((r) => r.path);
// Conditionally load the vendored pi-claude-cli extension so the user's
// "Anthropic — via Claude CLI" provider routing takes effect without
// requiring a manual `pi-claude-cli` install.
const claudeCliPaths = await (async () => {
try {
const globalSettings = await store.getGlobalSettingsStore().getSettings();
const result = resolveClaudeCliExtensionPaths(globalSettings);
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
} catch (err) {
console.warn(
`[extensions] Unable to evaluate useClaudeCli setting: ${err instanceof Error ? err.message : String(err)}`,
);
setCachedClaudeCliResolution(null);
return [];
// Always load the vendored pi-claude-cli extension. It registers under
// a distinct provider id ("pi-claude-cli") so it coexists with direct
// Anthropic auth. The `useClaudeCli` setting only controls whether the
// dashboard shows those models in the picker — the extension itself is
// a no-op when the `claude` binary is missing (it catches and logs
// internally, see packages/pi-claude-cli/index.ts:106).
const claudeCliPaths = (() => {
const result = resolveClaudeCliExtensionPaths();
setCachedClaudeCliResolution(result.resolution);
if (result.warning) {
console.warn(`[extensions] pi-claude-cli: ${result.warning}`);
}
return result.paths;
})();
const extensionsResult = await discoverAndLoadExtensions(

View File

@@ -291,10 +291,11 @@ function ClaudeCliStatusLine({
</small>
);
}
// Enabled but `ready` is false and we have no specific reason — usually a
// transient state after flipping the toggle before the first probe
// completes.
return (
<small className="settings-muted">
Enabled. Restart Fusion to complete activation.
</small>
<small className="settings-muted">Enabled. Validating</small>
);
}
@@ -317,9 +318,9 @@ function ClaudeCliActionToast({
return (
<p className="onboarding-helper-text">
{verb}.{" "}
{action.restartRequired
? "Restart Fusion to activate the routing change."
: "No further action needed."}
{action.kind === "enabled"
? "Claude-CLI-routed models are now visible in the model picker."
: "Claude-CLI-routed models are hidden from the model picker."}
</p>
);
}

View File

@@ -375,22 +375,6 @@ function slugifyPresetName(name: string): string {
return slug || "preset";
}
/**
* Extract RunMutationContext from the X-Run-Context header.
* Used to correlate dashboard mutations with agent runs for audit trails.
*/
function _extractRunContext(req: { headers: { [key: string]: string | string[] | undefined } }): import("@fusion/core").RunMutationContext | undefined {
const header = req.headers['x-run-context'];
if (typeof header !== 'string') return undefined;
try {
const parsed = JSON.parse(header);
if (parsed && typeof parsed.runId === 'string' && typeof parsed.agentId === 'string') {
return parsed as import("@fusion/core").RunMutationContext;
}
} catch { /* invalid JSON, ignore */ }
return undefined;
}
function validateModelPresets(value: unknown): ModelPreset[] | undefined {
if (value === undefined) return undefined;
if (!Array.isArray(value)) {
@@ -7585,12 +7569,10 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
const session = terminalSessionManager.getSession(id);
if (!session) {
throw notFound("Session not found");
} else {
throw badRequest("Session is not running");
}
return;
throw badRequest("Session is not running");
}
res.json({ killed: true, sessionId: id });
} catch (err: unknown) {
if (err instanceof ApiError) {
@@ -7782,10 +7764,8 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
const session = terminalService.getSession(id);
if (!session) {
throw notFound("Session not found");
} else {
throw badRequest("Failed to kill session");
}
return;
throw badRequest("Failed to kill session");
}
res.json({ killed: true });
@@ -11486,43 +11466,35 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
});
function validateAgentInstructionsPayload(
res: Response,
instructionsPath: unknown,
instructionsText: unknown,
): boolean {
if (instructionsPath !== undefined && instructionsPath !== null && instructionsPath !== "") {
if (typeof instructionsPath !== "string") {
throw badRequest("instructionsPath must be a string");
return false;
}
if (instructionsPath.length > 500) {
throw badRequest("instructionsPath must be at most 500 characters");
return false;
}
if (instructionsPath.includes("..")) {
throw badRequest("instructionsPath must not contain parent directory traversal (..)");
return false;
}
const isAbsoluteUnix = instructionsPath.startsWith("/");
const isAbsoluteWindows = /^[A-Za-z]:[\\/]/.test(instructionsPath);
if (isAbsoluteUnix || isAbsoluteWindows) {
throw badRequest("instructionsPath must be a project-relative path");
return false;
}
if (!instructionsPath.endsWith(".md")) {
throw badRequest("instructionsPath must end in .md");
return false;
}
}
if (instructionsText !== undefined && instructionsText !== null && instructionsText !== "") {
if (typeof instructionsText !== "string") {
throw badRequest("instructionsText must be a string");
return false;
}
if (instructionsText.length > 50000) {
throw badRequest("instructionsText must be at most 50,000 characters");
return false;
}
}
@@ -11584,7 +11556,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
if (permissions !== undefined && (typeof permissions !== "object" || permissions === null || Array.isArray(permissions))) {
throw badRequest("permissions must be an object");
}
if (!validateAgentInstructionsPayload(res, instructionsPath, instructionsText)) {
if (!validateAgentInstructionsPayload(instructionsPath, instructionsText)) {
return;
}
if (soul !== undefined && soul !== null && typeof soul !== "string") {
@@ -12617,7 +12589,7 @@ async function persistImportedSkills(
updates.totalOutputTokens = body.totalOutputTokens ?? undefined;
}
if (!validateAgentInstructionsPayload(res, body.instructionsPath, body.instructionsText)) {
if (!validateAgentInstructionsPayload(body.instructionsPath, body.instructionsText)) {
return;
}
if ("instructionsPath" in body) {
@@ -12769,7 +12741,7 @@ async function persistImportedSkills(
router.patch("/agents/:id/instructions", async (req, res) => {
try {
const { instructionsPath, instructionsText } = req.body ?? {};
if (!validateAgentInstructionsPayload(res, instructionsPath, instructionsText)) {
if (!validateAgentInstructionsPayload(instructionsPath, instructionsText)) {
return;
}
@@ -18940,15 +18912,16 @@ function registerModelsRoute(
}
}
// When the user has opted to route AI through pi-claude-cli, only
// Anthropic Claude models are reachable — pi-claude-cli wraps the
// local Claude CLI and does not bridge other providers. Surface only
// those models so every picker in the app (settings, onboarding, per
// lane overrides) stays honest about what'll actually run.
// OpenRouter-proxied Claude (provider: "openrouter") is excluded on
// purpose: it hits OpenRouter's API, not the local CLI.
if (useClaudeCli) {
models = models.filter((m) => m.provider === "anthropic");
// The vendored @fusion/pi-claude-cli extension registers its provider
// as "pi-claude-cli" (distinct from "anthropic") regardless of the
// toggle. When the toggle is OFF, hide those entries from the picker
// so users don't see CLI-routed models they haven't opted into.
// When ON, show everything — the user deliberately wants them visible
// alongside any direct Anthropic auth or other providers they've
// connected. Hiding only the CLI-routed entries (not restricting to
// them) preserves full flexibility.
if (!useClaudeCli) {
models = models.filter((m) => m.provider !== "pi-claude-cli");
}
res.json({ models, favoriteProviders, favoriteModels });
@@ -19165,12 +19138,14 @@ function registerAuthRoutes(
res.json({
enabled: next,
// Pi extension registrations can't be added/removed mid-process,
// so flipping on/off requires a restart for the model routing
// itself to take effect. Skill install/backfill happens
// immediately either way. Surface this so the UI can show a
// "Restart Fusion to activate" prompt when next !== prev.
restartRequired: prev !== next,
// The pi-claude-cli extension is loaded unconditionally at startup
// (the provider it registers is namespaced distinctly so it doesn't
// clash with direct Anthropic auth), so flipping this setting has
// immediate effect — the /api/models filter reads the new value on
// next request. No restart needed. `restartRequired` is retained
// in the response shape for forward compatibility but is always
// false today.
restartRequired: false,
});
} catch (err: unknown) {
if (err instanceof ApiError) {