Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 43 additions & 39 deletions packages/coding-agent/CHANGELOG.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/coding-agent/src/config/model-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ interface ProviderOverride {
* Without explicit transport propagation, an openrouter (or any) entry
* marked `transport: pi-native` in models.yml silently reverts to the
* default openai-completions transport after the background catalog
* refresh — so the first `/model` switch after boot hits the raw OpenAI
* refresh — so the first `/models` switch after boot hits the raw OpenAI
* chat-completions URL instead of the gateway's `/v1/pi/stream` (#2555).
* See `xiaomi-tp-discovery-merge.test.ts` and the `refresh()` baseUrl-override
* regression in `model-registry.test.ts`.
Expand Down
2 changes: 1 addition & 1 deletion packages/coding-agent/src/config/model-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ export async function resolveAllowedModels(
/**
* Synchronous subset of {@link resolveAllowedModels} for contexts where async is unavailable
* (e.g. `getAvailableModels()` which is called from the ACP model-list advertisement, RPC
* `get_available_models`, and the `/model` slash command). Uses the same effective
* `get_available_models`, and the `/models` slash command). Uses the same effective
* `enabledModels` scope semantics as startup resolution:
*
* - Glob selectors match `provider/modelId` and bare model id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { ExtensionModelQuery } from "./types";

/**
* Build the `ctx.models` facade. `getModel` is read lazily so `current()` always
* reflects the live session model (it can change mid-session via `/model`).
* reflects the live session model (it can change mid-session via `/models`).
*/
export function createExtensionModelQuery(
modelRegistry: ModelRegistry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*
* Each screen is backed by a proven primitive — {@link SelectList} (list / detail
* / tools / thinking), {@link Input} (name), {@link ModelSelectorComponent} (the
* same rich `/model` picker, in direct-select mode), and {@link HookEditorComponent}
* same rich `/models` picker, in direct-select mode), and {@link HookEditorComponent}
* (multiline instructions; Ctrl+G opens `$EDITOR`). The overlay edits an in-memory
* {@link WatchdogConfigDoc} and only touches disk + the live advisors via the host
* `save` callback.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export interface SettingsRuntimeContext {
thinkingLevel: ThinkingLevel | undefined;
/** Available themes */
availableThemes: string[];
/** Provider/source ids shown in /model. */
/** Provider/source ids shown in /models. */
providers: string[];
/** Working directory for plugins tab */
cwd: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ export class SelectorController {
this.ctx.updateEditorBorderColor();
const roleSelectorHint = this.ctx.keybindings.getKeys("app.model.select")[0] ?? "Alt+M";
this.ctx.showStatus(
`Session-only model: ${selector ?? model.id}. Use ${roleSelectorHint} or /model for roles.`,
`Session-only model: ${selector ?? model.id}. Use ${roleSelectorHint} or /models for roles.`,
);
done();
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion packages/coding-agent/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2140,7 +2140,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
modelFallbackMessage =
patterns && patterns.length > 0
? `No model available matching enabledModels (${patterns.join(", ")}) with usable credentials. Configure auth for an allowed provider or adjust enabledModels.`
: "No models available. Use /login or set an API key environment variable. Then use /model to select a model.";
: "No models available. Use /login or set an API key environment variable. Then use /models to select a model.";
}
}

Expand Down
10 changes: 5 additions & 5 deletions packages/coding-agent/src/session/agent-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7849,7 +7849,7 @@ export class AgentSession {
throw new Error(
"No model selected.\n\n" +
`Use /login, set an API key environment variable, or create ${getAgentDbPath()}\n\n` +
"Then use /model to select a model.",
"Then use /models to select a model.",
);
}

Expand Down Expand Up @@ -9170,7 +9170,7 @@ export class AgentSession {

// Trust the recorded role only while its resolved model still IS the
// active model. A model switch through another surface (alt+m, retry
// fallback, /model) or a role re-configuration leaves the recorded role
// fallback, /models) or a role re-configuration leaves the recorded role
// pointing at a model the session no longer runs; cycling from that
// stale slot lands on the wrong neighbor and reads as a skipped entry.
const lastRole = this.sessionManager.getLastModelChangeRole();
Expand Down Expand Up @@ -10312,7 +10312,7 @@ export class AgentSession {
return stream.result();
},
telemetry: resolveTelemetry(this.agent.telemetry, this.sessionId),
// Honor the user's /model thinking selection on the handoff path.
// Honor the user's /models thinking selection on the handoff path.
// Clamped per-model inside generateHandoffFromContext via
// resolveCompactionEffort so unsupported-effort models don't trip
// requireSupportedEffort.
Expand Down Expand Up @@ -12219,7 +12219,7 @@ export class AgentSession {
metadata: this.agent.metadataForProvider(candidate.provider),
convertToLlm: messages => this.#convertToLlmForSideRequest(messages),
telemetry,
// Honor the user's /model thinking selection (incl. `off`) on
// Honor the user's /models thinking selection (incl. `off`) on
// the manual `/compact` path. Clamped per-model inside compact()
// via resolveCompactionEffort so unsupported-effort models
// (xai-oauth/grok-build) don't trip requireSupportedEffort.
Expand Down Expand Up @@ -12926,7 +12926,7 @@ export class AgentSession {
initiatorOverride: "agent",
convertToLlm: messages => this.#convertToLlmForSideRequest(messages),
telemetry,
// Honor the user's /model thinking selection on the
// Honor the user's /models thinking selection on the
// auto-compaction path — the most-fired compaction
// site. Clamped per-model inside compact() via
// resolveCompactionEffort.
Expand Down
6 changes: 3 additions & 3 deletions packages/coding-agent/src/slash-commands/acp-builtins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export type { AcpBuiltinSlashCommandResult } from "./types";
/**
* All names (primary + aliases) that are reserved by ACP builtins. Used to
* filter out extension commands that would shadow a builtin or its alias at
* dispatch time (e.g. `models` is an alias for `/model`, so an extension
* dispatch time (e.g. `/models` is the canonical builtin name, so an extension
* registering `models` would appear in the palette but execute the builtin).
*/
export const ACP_BUILTIN_RESERVED_NAMES: ReadonlySet<string> = new Set(
Expand All @@ -19,8 +19,8 @@ export const ACP_BUILTIN_RESERVED_NAMES: ReadonlySet<string> = new Set(
* Whether an extension command named `name` would be captured by ACP builtin
* dispatch before reaching the extension handler. Beyond exact name/alias
* collisions, `parseSlashCommand` treats `:` as a name/args separator, so a
* colon-namespaced name whose prefix is a handled builtin (e.g. `model:foo`)
* executes the `/model` builtin with `foo` as args. Such names must not be
* colon-namespaced name whose prefix is a handled builtin (e.g. `models:foo`)
* executes the `/models` builtin with `foo` as args. Such names must not be
* advertised to ACP clients.
*/
export function isAcpBuiltinShadowedName(name: string): boolean {
Expand Down
5 changes: 2 additions & 3 deletions packages/coding-agent/src/slash-commands/builtin-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
},
},
{
name: "model",
aliases: ["models"],
name: "models",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: This drops the only /model registration (BUILTIN_SLASH_COMMAND_LOOKUP is populated only from name plus aliases at lines 2296-2300), so existing TUI and ACP invocations of /model now fall through. The repo bar says default-behavior changes need explicit maintainer sign-off; absent that, make /models canonical but keep aliases: ["model"] so existing user scripts/docs keep working.

description: "Switch model for this session",
acpDescription: "Show current model selection",
getTuiAutocompleteDescription: runtime => {
Expand All @@ -353,7 +352,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
);
if (!match) {
return usage(
`Unknown model: ${modelId}. Use ACP \`session/setModel\` for picker-driven selection or list available models with /model.`,
`Unknown model: ${modelId}. Use ACP \`session/setModel\` for picker-driven selection or list available models with /models.`,
runtime,
);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/coding-agent/test/acp-agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1570,13 +1570,13 @@ describe("ACP agent", () => {
];
// Extension runner: unique command + one colliding with an ACP builtin
// ("fast") + a colon-namespaced one whose prefix is a builtin
// ("model:foo" parses as builtin `/model` with args `foo` at dispatch).
// ("models:foo" parses as builtin `/models` with args `foo` at dispatch).
(session as unknown as { extensionRunner: unknown }).extensionRunner = {
getRegisteredCommands(reserved?: Set<string>) {
return [
{ name: "my-ext-cmd", description: "Extension command", handler: async () => {} },
{ name: "fast", description: "Would shadow builtin", handler: async () => {} },
{ name: "model:foo", description: "Colon-shadowed by /model", handler: async () => {} },
{ name: "models:foo", description: "Colon-shadowed by /models", handler: async () => {} },
].filter(cmd => !reserved?.has(cmd.name));
},
};
Expand Down Expand Up @@ -1611,8 +1611,8 @@ describe("ACP agent", () => {
// ACP builtin "fast" appears exactly once (reserved-set exclusion, no duplicate from extension).
expect(names.filter(n => n === "fast").length).toBe(1);
// Colon-namespaced collision with a builtin prefix is not advertised:
// ACP would dispatch `/model:foo` to the `/model` builtin, not the extension.
expect(names).not.toContain("model:foo");
// ACP would dispatch `/models:foo` to the `/models` builtin, not the extension.
expect(names).not.toContain("models:foo");

harness.abortController.abort();
await Bun.sleep(0);
Expand Down
34 changes: 17 additions & 17 deletions packages/coding-agent/test/acp-builtins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,36 +414,36 @@ describe("ACP builtin slash commands", () => {
expect(output[0]).toContain("No messages");
});

// /model
it("model: returns current model when set", async () => {
// /models
it("models: returns current model when set", async () => {
const { output, runtime } = createRuntime();
runtime.session.model = { provider: "anthropic", id: "claude-opus-4-5" } as never;

const result = await executeAcpBuiltinSlashCommand("/model", runtime);
const result = await executeAcpBuiltinSlashCommand("/models", runtime);

expect(result).toEqual({ consumed: true });
expect(output[0]).toContain("anthropic/claude-opus-4-5");
});

it("model: returns no-selection message when undefined", async () => {
it("models: returns no-selection message when undefined", async () => {
const { output, runtime } = createRuntime();

const result = await executeAcpBuiltinSlashCommand("/model", runtime);
const result = await executeAcpBuiltinSlashCommand("/models", runtime);

expect(result).toEqual({ consumed: true });
expect(output[0]).toContain("No model");
});

it("model: returns ACP usage message when args provided", async () => {
it("models: returns ACP usage message when args provided", async () => {
const { output, runtime } = createRuntime();

const result = await executeAcpBuiltinSlashCommand("/model claude-3-5-sonnet", runtime);
const result = await executeAcpBuiltinSlashCommand("/models claude-3-5-sonnet", runtime);

expect(result).toEqual({ consumed: true });
expect(output[0]?.toLowerCase()).toContain("acp");
});

it("model: applies known id and emits both title + config change notifications", async () => {
it("models: applies known id and emits both title + config change notifications", async () => {
const { output, runtime, session } = createRuntime();
const available = [{ provider: "anthropic", id: "claude-3-5-sonnet", contextWindow: 200_000 }];
session.getAvailableModels = () => available;
Expand All @@ -457,7 +457,7 @@ describe("ACP builtin slash commands", () => {
};
const setModelSpy = spyOn(session, "setModel").mockResolvedValue(undefined);

const result = await executeAcpBuiltinSlashCommand("/model claude-3-5-sonnet", runtime);
const result = await executeAcpBuiltinSlashCommand("/models claude-3-5-sonnet", runtime);

expect(result).toEqual({ consumed: true });
expect(setModelSpy).toHaveBeenCalledWith(available[0]);
Expand All @@ -466,14 +466,14 @@ describe("ACP builtin slash commands", () => {
expect(configNotified).toBe(1);
});

it("model: does not emit config change when id is unknown", async () => {
it("models: does not emit config change when id is unknown", async () => {
const { runtime } = createRuntime();
let configNotified = 0;
runtime.notifyConfigChanged = () => {
configNotified++;
};

await executeAcpBuiltinSlashCommand("/model nonexistent", runtime);
await executeAcpBuiltinSlashCommand("/models nonexistent", runtime);

expect(configNotified).toBe(0);
});
Expand Down Expand Up @@ -1035,23 +1035,23 @@ describe("wave 5 — adapters and polish", () => {
}
});

// /model with unknown id
it("/model gpt-fake-9000: returns unknown-model message", async () => {
// /models with unknown id
it("/models gpt-fake-9000: returns unknown-model message", async () => {
const { output, runtime } = createRuntime();
const result = await executeAcpBuiltinSlashCommand("/model gpt-fake-9000", runtime);
const result = await executeAcpBuiltinSlashCommand("/models gpt-fake-9000", runtime);
expect(result).toEqual({ consumed: true });
expect(output[0]).toContain("Unknown model");
});

// /model with known id (fake registry)
it("/model known-id: reports model set and triggers notifyTitleChanged", async () => {
// /models with known id (fake registry)
it("/models known-id: reports model set and triggers notifyTitleChanged", async () => {
const { output, session, runtime } = createRuntime();
session.getAvailableModels = () => [{ provider: "anthropic", id: "claude-sonnet-test" }];
let titleChanged = false;
runtime.notifyTitleChanged = () => {
titleChanged = true;
};
const result = await executeAcpBuiltinSlashCommand("/model claude-sonnet-test", runtime);
const result = await executeAcpBuiltinSlashCommand("/models claude-sonnet-test", runtime);
expect(result).toEqual({ consumed: true });
expect(output[0]).toContain("Model set to anthropic/claude-sonnet-test.");
expect(titleChanged).toBe(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { TempDir } from "@oh-my-pi/pi-utils";
// Regression: `#closeProviderSessionsForModelSwitch` historically only handled
// the `openai-codex-responses` / `openai-responses` keys and left
// `openai-completions:<provider>:<baseUrl>:<modelId>` entries behind on a
// /model switch. The cached strict-tools disable scopes and reasoning-effort
// /models switch. The cached strict-tools disable scopes and reasoning-effort
// fallbacks for the old backend then survived indefinitely — repro reported
// in #3260 (PR #3236).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ describe("AgentSession role model thinking behavior", () => {
// Record a model_change for the "slow" role WITHOUT switching the
// active model — the session still runs the default model. This is the
// stale state left behind when the model is changed through another
// surface (alt+m, temporary model, /model) after a role cycle.
// surface (alt+m, temporary model, /models) after a role cycle.
session.sessionManager.appendModelChange(`${slowModel.provider}/${slowModel.id}`, "slow");
expect(session.sessionManager.getLastModelChangeRole()).toBe("slow");
expect(session.model?.id).toBe(defaultModel.id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe("InteractiveMode prompt-template autocomplete (#2462)", () => {
expect(matches.filter(name => name === "exit")).toHaveLength(1);
});

it("does not duplicate templates whose names collide with builtin slash command aliases", async () => {
it("does not duplicate templates whose names collide with builtin slash command names", async () => {
const created = createHarness([
{
name: "models",
Expand All @@ -217,7 +217,7 @@ describe("InteractiveMode prompt-template autocomplete (#2462)", () => {
const provider = slot.current;
expect(provider).toBeDefined();
const matches = await fetchSlashSuggestions(provider!, "/models");
// Builtin `/model` owns the `/models` alias. The colliding template is filtered
// Builtin `/models` is the canonical command name. The colliding template is filtered
// out so autocomplete follows the interactive slash-command resolution path.
expect(matches.filter(name => name === "models")).toHaveLength(1);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/coding-agent/test/model-discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,7 @@ describe("ModelRegistry runtime discovery", () => {
});

test("llama.cpp router preset refresh honors --ctx-size when the child hasn't been loaded yet", async () => {
// Reporter's workflow: `/model` picks a preset. On its very first switch
// Reporter's workflow: `/models` picks a preset. On its very first switch
// the child hasn't been spawned yet (meta.n_ctx absent), but the
// configured window is still what the user wants surfaced.
writeModelCache(
Expand Down
2 changes: 1 addition & 1 deletion packages/coding-agent/test/model-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ describe("ModelRegistry", () => {

test("refresh keeps transport override on built-in provider (#2555 openrouter gateway)", async () => {
// Reporter ran `omp` with the auth-gateway broker proxying OpenRouter.
// Default model worked; switching via `/model` produced
// Default model worked; switching via `/models` produced
// `404 No route: POST /chat/completions` until restart. Root cause:
// background discovery refresh re-fetched the openrouter catalog and
// `mergeDiscoveredModel` dropped `transport: pi-native` (raw catalog
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe("internal-url-autocomplete", () => {

it("rejects non-url prefixes", () => {
expect(isInternalUrlPrefix("@src/foo")).toBe(false);
expect(isInternalUrlPrefix("/model")).toBe(false);
expect(isInternalUrlPrefix("/models")).toBe(false);
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/coding-agent/test/prompt-action-autocomplete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ describe("prompt action autocomplete", () => {

it("delegates trySyncSlashCompletion to CombinedAutocompleteProvider", () => {
const provider = createPromptActionAutocompleteProvider({
commands: [{ name: "model", description: "Switch AI model" }],
commands: [{ name: "models", description: "Switch AI model" }],
basePath: "/tmp",
keybindings: AppKeybindingsManager.inMemory(),
copyCurrentLine: () => {},
Expand All @@ -201,12 +201,12 @@ describe("prompt action autocomplete", () => {

const result = provider.trySyncSlashCompletion("/mo");
expect(result).not.toBeNull();
expect(result!.items.map(i => i.value)).toContain("model");
expect(result!.items.map(i => i.value)).toContain("models");
});

it("returns null from trySyncSlashCompletion for non-slash text", () => {
const provider = createPromptActionAutocompleteProvider({
commands: [{ name: "model", description: "Switch AI model" }],
commands: [{ name: "models", description: "Switch AI model" }],
basePath: "/tmp",
keybindings: AppKeybindingsManager.inMemory(),
copyCurrentLine: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("shouldSkipHistory — security filter for slash command history", () =
expect(shouldSkipHistory("/plan do something")).toBe(false);
expect(shouldSkipHistory("/settings")).toBe(false);
expect(shouldSkipHistory("/btw what is this")).toBe(false);
expect(shouldSkipHistory("/model claude")).toBe(false);
expect(shouldSkipHistory("/models claude")).toBe(false);
});

it("returns false for non-slash text", () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/coding-agent/test/slash-commands/switch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ function createRuntime() {
};
}

describe("/model slash command", () => {
describe("/models slash command", () => {
it("opens the model setup picker for role and thinking assignment", async () => {
const harness = createRuntime();

const handled = await executeBuiltinSlashCommand("/model", harness.runtime);
const handled = await executeBuiltinSlashCommand("/models", harness.runtime);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should-fix: This only proves the new /models spelling works. If removing /model is intentional, add a contract test for executeBuiltinSlashCommand("/model", ...) returning false (and the ACP equivalent) so the break is explicit; if the alias is kept, assert /model still opens the same picker.


expect(handled).toBe(true);
expect(harness.showModelSelector.mock.calls).toEqual([[]]);
Expand Down
Loading