smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: store slash commands in input history (#3148)

Browse Source 
Previously only 4 commands (/plan, /goal, /mcp, /ssh) stored their text
in history via per-handler addToHistory calls. All other built-in slash
commands were silently skipped because executeBuiltinSlashCommand returned
true before the input controller's addToHistory was reached.

- Centralize history recording in the input controller after successful
  slash command dispatch, for both Enter and Ctrl+Enter submit paths.
- Remove all 10 per-command addToHistory calls from slash command handlers
  to prevent duplicates.
- Add shouldSkipHistory() security filter to exclude commands that may
  carry secrets: /login <url> (OAuth callback with code=/state= params)
  and /mcp add --token <token> (bearer token).
- Add regression tests for the security filter (8 cases).
- Update 7 existing test files to remove handler-level addToHistory
  assertions (now the input controller's responsibility).
This commit is contained in:
oldschoola
2026-06-23 14:11:12 -07:00
parent 93d730c01d
commit 715eb0792c
11 changed files with 79 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+2
View File
@@ -4,7 +4,9 @@
### Fixed
- Slash commands are now recorded in input history (Up Arrow recall). Previously only 4 commands (`/plan`, `/goal`, `/mcp`, `/ssh`) stored their text; all other built-in slash commands were silently skipped because `executeBuiltinSlashCommand` returned `true` before `addToHistory` was called. History is now centralized in the input controller after successful command dispatch. Commands that may carry secrets (`/login <url>` with OAuth callback params, `/mcp add --token <token>`) are excluded from history to prevent credential leakage ([#3148](https://github.com/can1357/oh-my-pi/issues/3148))
- Fixed all extension loading silently failing on the cross-compiled `omp-darwin-arm64` release binary (downloaded directly or via a Homebrew tap wrapper) because `__computeBunfsPackageRoot` mis-handled `import.meta.dir = "//root/omp-darwin-arm64"`. Bun 1.3.14 reports `<bunfs-root>/<binary-name>` for the compiled entry's `import.meta.dir`, but the pre-fix function joined `metaDir + "packages"` and produced `/root/omp-darwin-arm64/packages` — the binary basename was baked into every bunfs path, so the TypeBox/legacy-pi shims and every `@oh-my-pi/pi-*` package-root override failed `existsSync` validation and `resolveCanonicalPiSpecifier` fell through to a bunfs `Bun.resolveSync` that also could not find the module. The function now detects the bunfs-root + binary-basename shape (`path.basename(path.dirname(metaDir)) === "root"`) and strips the trailing binary segment by slicing the original `metaDir`; the production bunfs shim join path also preserves Bun's bunfs-native `//root` / `B:\~BUN\root` prefix that `path.join` would otherwise collapse. ([#3329](https://github.com/can1357/oh-my-pi/issues/3329))
## [16.1.16] - 2026-06-23
### Breaking Changes
packages/coding-agent/src/modes/controllers/input-controller.ts
+23
View File
@@ -27,6 +27,27 @@ import { ensureSupportedImageInput, ImageInputTooLargeError, loadImageInput } fr
import { resizeImage } from "../../utils/image-resize";
import { generateSessionTitle, setSessionTerminalTitle } from "../../utils/title-generator";
/**
* Slash commands that may carry secrets in their arguments should never be
* persisted to history. /login <url> receives an OAuth callback URL containing
* code= and state= params. /mcp add --token <token> receives a bearer token.
*/
export function shouldSkipHistory(slashText: string): boolean {
if (!slashText.startsWith("/")) return false;
const name = slashText.slice(1).split(/\s+/, 1)[0];
// /login <url> — the redirect URL carries OAuth code= and state= params.
// /login <provider> (e.g. "anthropic") is safe — it just opens the selector.
if (name === "login" && slashText.length > "/login".length) {
const arg = slashText.slice("/login".length).trim();
return arg.includes("://") || arg.startsWith("http");
}
if (name === "mcp") {
const args = slashText.slice("/mcp".length).trim();
return args.startsWith("add") && /--token\s/.test(args);
}
return false;
}
interface Expandable {
setExpanded(expanded: boolean): void;
}
@@ -572,6 +593,7 @@ export class InputController {
ctx: this.ctx,
});
if (slashResult === true) {
if (!shouldSkipHistory(text)) this.ctx.editor.addToHistory(text);
return;
}
if (typeof slashResult === "string") {
@@ -1011,6 +1033,7 @@ export class InputController {
ctx: this.ctx,
});
if (slashResult === true) {
if (!shouldSkipHistory(text)) this.ctx.editor.addToHistory(text);
return;
}
if (typeof slashResult === "string") {
packages/coding-agent/src/slash-commands/builtin-registry.ts
-22
View File
@@ -239,11 +239,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
return "Plan: off";
},
handleTui: async (command, runtime) => {
const hadArgs = !!command.args;
await runtime.ctx.handlePlanModeCommand(command.args || undefined);
if (hadArgs) {
runtime.ctx.editor.addToHistory(command.text);
}
runtime.ctx.editor.setText("");
},
},
@@ -277,11 +273,7 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
return state ? `Goal: ${state.goal.status} (${shortDetail(state.goal.objective)})` : "Goal: off";
},
handleTui: async (command, runtime) => {
const hadArgs = !!command.args;
await runtime.ctx.handleGoalModeCommand(command.args || undefined);
if (hadArgs) {
runtime.ctx.editor.addToHistory(command.text);
}
runtime.ctx.editor.setText("");
},
},
@@ -1270,7 +1262,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
allowArgs: true,
handle: handleMcpAcp,
handleTui: async (command, runtime) => {
runtime.ctx.editor.addToHistory(command.text);
runtime.ctx.editor.setText("");
await runtime.ctx.handleMCPCommand(command.text);
},
@@ -1293,7 +1284,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
allowArgs: true,
handle: handleSshAcp,
handleTui: async (command, runtime) => {
runtime.ctx.editor.addToHistory(command.text);
runtime.ctx.editor.setText("");
await runtime.ctx.handleSSHCommand(command.text);
},
@@ -1436,9 +1426,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
allowArgs: true,
handleTui: async (command, runtime) => {
const question = command.text.slice(`/${command.name}`.length).trim();
if (question) {
runtime.ctx.editor.addToHistory(command.text);
}
runtime.ctx.editor.setText("");
await runtime.ctx.handleBtwCommand(question);
},
@@ -1450,9 +1437,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
allowArgs: true,
handleTui: async (command, runtime) => {
const work = command.text.slice(`/${command.name}`.length).trim();
if (work) {
runtime.ctx.editor.addToHistory(command.text);
}
runtime.ctx.editor.setText("");
await runtime.ctx.handleTanCommand(work);
},
@@ -1464,9 +1448,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
allowArgs: true,
handleTui: async (command, runtime) => {
const complaint = command.text.slice(`/${command.name}`.length).trim();
if (complaint) {
runtime.ctx.editor.addToHistory(command.text);
}
runtime.ctx.editor.setText("");
await runtime.ctx.handleOmfgCommand(complaint);
},
@@ -1558,7 +1539,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
}
},
handleTui: async (command, runtime) => {
runtime.ctx.editor.addToHistory(command.text);
runtime.ctx.editor.setText("");
await runtime.ctx.handleMemoryCommand(command.text);
},
@@ -1586,7 +1566,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
runtime.ctx.editor.setText("");
return;
}
runtime.ctx.editor.addToHistory(command.text);
runtime.ctx.editor.setText("");
await runtime.ctx.handleRenameCommand(title);
},
@@ -1629,7 +1608,6 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
runtime.ctx.editor.setText("");
return;
}
runtime.ctx.editor.addToHistory(command.text);
runtime.ctx.editor.setText("");
await runtime.ctx.handleMoveCommand(targetPath);
},
packages/coding-agent/test/slash-commands/btw.test.ts
+1 -4
View File
@@ -26,7 +26,6 @@ describe("/btw slash command", () => {
const handled = await executeBuiltinSlashCommand("/btw why is it doing that?", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/btw why is it doing that?");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleBtwCommand).toHaveBeenCalledWith("why is it doing that?");
});
@@ -40,17 +39,15 @@ describe("/btw slash command", () => {
);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/btw explain why the cache reuse matters here");
expect(harness.handleBtwCommand).toHaveBeenCalledWith("explain why the cache reuse matters here");
});
it("does not add a blank /btw invocation to history", async () => {
it("handles a blank /btw invocation without error", async () => {
const harness = createRuntime();
const handled = await executeBuiltinSlashCommand("/btw ", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).not.toHaveBeenCalled();
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleBtwCommand).toHaveBeenCalledWith("");
});
packages/coding-agent/test/slash-commands/history-security.test.ts
+41
View File
@@ -0,0 +1,41 @@
import { describe, expect, it } from "bun:test";
import { shouldSkipHistory } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller";
describe("shouldSkipHistory — security filter for slash command history", () => {
it("skips /login with a redirect URL argument (contains OAuth code/state)", () => {
expect(shouldSkipHistory("/login http://localhost:1455/auth/callback?code=abc&state=xyz")).toBe(true);
});
it("does not skip /login without arguments (triggers provider selector)", () => {
expect(shouldSkipHistory("/login")).toBe(false);
});
it("does not skip /login with a provider name only", () => {
expect(shouldSkipHistory("/login anthropic")).toBe(false);
});
it("skips /mcp add with --token flag (contains bearer token)", () => {
expect(shouldSkipHistory("/mcp add myserver --url http://x --token sk-secret123")).toBe(true);
});
it("does not skip /mcp add without --token", () => {
expect(shouldSkipHistory("/mcp add myserver --url http://x")).toBe(false);
});
it("does not skip /mcp without add subcommand", () => {
expect(shouldSkipHistory("/mcp list")).toBe(false);
expect(shouldSkipHistory("/mcp reload")).toBe(false);
});
it("does not skip ordinary slash commands", () => {
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);
});
it("returns false for non-slash text", () => {
expect(shouldSkipHistory("just a prompt")).toBe(false);
expect(shouldSkipHistory("")).toBe(false);
});
});
packages/coding-agent/test/slash-commands/memory.test.ts
-2
View File
@@ -26,7 +26,6 @@ describe("/memory slash command", () => {
const handled = await executeBuiltinSlashCommand("/memory view", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/memory view");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleMemoryCommand).toHaveBeenCalledWith("/memory view");
});
@@ -37,7 +36,6 @@ describe("/memory slash command", () => {
const handled = await executeBuiltinSlashCommand("/memory stats", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/memory stats");
expect(harness.handleMemoryCommand).toHaveBeenCalledWith("/memory stats");
});
});
packages/coding-agent/test/slash-commands/move.test.ts
+1 -3
View File
@@ -29,18 +29,16 @@ describe("/move slash command", () => {
const handled = await executeBuiltinSlashCommand("/move /tmp/project", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/move /tmp/project");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleMoveCommand).toHaveBeenCalledWith("/tmp/project");
});
it("does not add a blank /move invocation to history", async () => {
it("handles a blank /move invocation without error", async () => {
const harness = createRuntime();
const handled = await executeBuiltinSlashCommand("/move ", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).not.toHaveBeenCalled();
expect(harness.showError).toHaveBeenCalledWith("Usage: /move <path>");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleMoveCommand).not.toHaveBeenCalled();
packages/coding-agent/test/slash-commands/omfg.test.ts
+1 -4
View File
@@ -26,7 +26,6 @@ describe("/omfg slash command", () => {
const handled = await executeBuiltinSlashCommand("/omfg This guy used any again....", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/omfg This guy used any again....");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleOmfgCommand).toHaveBeenCalledWith("This guy used any again....");
});
@@ -40,17 +39,15 @@ describe("/omfg slash command", () => {
);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/omfg stop making unchecked casts in generated TypeScript");
expect(harness.handleOmfgCommand).toHaveBeenCalledWith("stop making unchecked casts in generated TypeScript");
});
it("does not add a blank /omfg invocation to history", async () => {
it("handles a blank /omfg invocation without error", async () => {
const harness = createRuntime();
const handled = await executeBuiltinSlashCommand("/omfg ", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).not.toHaveBeenCalled();
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleOmfgCommand).toHaveBeenCalledWith("");
});
packages/coding-agent/test/slash-commands/plan-history.test.ts
+8 -15
View File
@@ -61,8 +61,8 @@ function createGoalHarness(opts: { goalModeEnabled: boolean; dropOnCall: boolean
};
}
describe("/plan history preservation when already active", () => {
it("preserves typed text in history when user confirms exit", async () => {
describe("/plan handler when already active", () => {
it("exits plan mode when user confirms exit", async () => {
const h = createPlanHarness({ planModeEnabled: true, confirmExit: true });
const handled = await executeBuiltinSlashCommand("/plan hello world", h.runtime);
@@ -70,12 +70,10 @@ describe("/plan history preservation when already active", () => {
expect(handled).toBe(true);
// Sanity check: exit was confirmed, so plan mode is now off.
expect(h.state.planModeEnabled).toBe(false);
// The typed command must still be recoverable via Up Arrow.
expect(h.addToHistory).toHaveBeenCalledWith("/plan hello world");
expect(h.setText).toHaveBeenCalledWith("");
});
it("preserves typed text in history when user cancels exit", async () => {
it("keeps plan mode active when user cancels exit", async () => {
const h = createPlanHarness({ planModeEnabled: true, confirmExit: false });
const handled = await executeBuiltinSlashCommand("/plan hello world", h.runtime);
@@ -83,23 +81,21 @@ describe("/plan history preservation when already active", () => {
expect(handled).toBe(true);
// Cancel: plan mode stays active.
expect(h.state.planModeEnabled).toBe(true);
expect(h.addToHistory).toHaveBeenCalledWith("/plan hello world");
expect(h.setText).toHaveBeenCalledWith("");
});
it("preserves typed text in history when entering plan mode for the first time", async () => {
it("enters plan mode when invoked for the first time", async () => {
const h = createPlanHarness({ planModeEnabled: false, confirmExit: false });
await executeBuiltinSlashCommand("/plan hello world", h.runtime);
expect(h.state.planModeEnabled).toBe(true);
expect(h.addToHistory).toHaveBeenCalledWith("/plan hello world");
expect(h.setText).toHaveBeenCalledWith("");
});
});
describe("/goal history preservation when already active", () => {
it("preserves typed text in history even if the handler clears goal mode", async () => {
describe("/goal handler when already active", () => {
it("drops goal mode when handler clears it", async () => {
// Simulates the user invoking a drop path that turns goal mode off
// inside handleGoalModeCommand. Without capturing state up-front,
// the post-call check would miss this case.
@@ -109,24 +105,21 @@ describe("/goal history preservation when already active", () => {
expect(handled).toBe(true);
expect(h.state.goalModeEnabled).toBe(false);
expect(h.addToHistory).toHaveBeenCalledWith("/goal new objective");
});
it("preserves typed text in history when goal mode stays active", async () => {
it("keeps goal mode active when handler does not clear it", async () => {
const h = createGoalHarness({ goalModeEnabled: true, dropOnCall: false });
await executeBuiltinSlashCommand("/goal new objective", h.runtime);
expect(h.state.goalModeEnabled).toBe(true);
expect(h.addToHistory).toHaveBeenCalledWith("/goal new objective");
});
it("preserves typed text in history when entering goal mode for the first time", async () => {
it("enters goal mode when invoked for the first time", async () => {
const h = createGoalHarness({ goalModeEnabled: false, dropOnCall: false });
await executeBuiltinSlashCommand("/goal new objective", h.runtime);
expect(h.state.goalModeEnabled).toBe(true);
expect(h.addToHistory).toHaveBeenCalledWith("/goal new objective");
});
});
packages/coding-agent/test/slash-commands/rename.test.ts
+1 -3
View File
@@ -29,18 +29,16 @@ describe("/rename slash command", () => {
const handled = await executeBuiltinSlashCommand("/rename my session", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/rename my session");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleRenameCommand).toHaveBeenCalledWith("my session");
});
it("does not add a blank /rename invocation to history", async () => {
it("handles a blank /rename invocation without error", async () => {
const harness = createRuntime();
const handled = await executeBuiltinSlashCommand("/rename ", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).not.toHaveBeenCalled();
expect(harness.showError).toHaveBeenCalledWith("Usage: /rename <title>");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleRenameCommand).not.toHaveBeenCalled();
packages/coding-agent/test/slash-commands/tan.test.ts
+1 -4
View File
@@ -26,7 +26,6 @@ describe("/tan slash command", () => {
const handled = await executeBuiltinSlashCommand("/tan add a changelog note", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/tan add a changelog note");
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleTanCommand).toHaveBeenCalledWith("add a changelog note");
});
@@ -40,17 +39,15 @@ describe("/tan slash command", () => {
);
expect(handled).toBe(true);
expect(harness.addToHistory).toHaveBeenCalledWith("/tan investigate why prompt cache reuse matters here");
expect(harness.handleTanCommand).toHaveBeenCalledWith("investigate why prompt cache reuse matters here");
});
it("does not add a blank /tan invocation to history", async () => {
it("handles a blank /tan invocation without error", async () => {
const harness = createRuntime();
const handled = await executeBuiltinSlashCommand("/tan ", harness.runtime);
expect(handled).toBe(true);
expect(harness.addToHistory).not.toHaveBeenCalled();
expect(harness.setText).toHaveBeenCalledWith("");
expect(harness.handleTanCommand).toHaveBeenCalledWith("");
});