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

Merge PR #8424: fix(cli): allow extension tools in --tools allowlists (@roboomp)

Browse Source
This commit is contained in:
can1357
2026-08-13 16:57:14 +02:00
parent 05eb8beebc 3749478239
commit bc77dcabb6
8 changed files with 68 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -6,6 +6,7 @@
- Fixed the status-line git branch display freezing on the previous branch after the first branch switch, caused by the HEAD watcher binding to a file inode that git unlinks on its atomic HEAD rename ([#8412](https://github.com/can1357/oh-my-pi/issues/8412)).
- Fixed Pi extension contexts omitting the runtime `mode`, which made documented TUI guards silently disable extension UI ([#8419](https://github.com/can1357/oh-my-pi/issues/8419)).
- Fixed extension-registered tool names being rejected by `--tools` before extension discovery, preventing least-privilege sessions from allowlisting plugin tools ([#8421](https://github.com/can1357/oh-my-pi/issues/8421)).
## [17.3.0] - 2026-08-13
packages/coding-agent/src/cli/args.ts
+14 -3
View File
@@ -6,7 +6,7 @@ import { $env, APP_NAME, logger } from "@oh-my-pi/pi-utils";
import chalk from "@oh-my-pi/pi-utils/chalk";
import type { ServiceTierOpenAISettingValue } from "../config/service-tier";
import { CLI_THINKING_LEVELS, type ConfiguredThinkingLevel, parseCliThinkingLevel } from "../thinking";
import { BUILTIN_TOOL_NAMES, HIDDEN_TOOL_NAMES, normalizeToolNames } from "../tools/builtin-names";
import { normalizeToolNames } from "../tools/builtin-names";
import {
OPTIONAL_FLAGS,
OPTIONAL_VALUE_FLAGS,
@@ -108,7 +108,6 @@ export interface Args {
const PARSE_DEPS: ParseDeps = {
logger,
parseThinking: parseCliThinkingLevel,
builtinToolNames: [...BUILTIN_TOOL_NAMES, ...HIDDEN_TOOL_NAMES],
normalizeToolNames,
thinkingEfforts: CLI_THINKING_LEVELS,
};
@@ -148,6 +147,7 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map<string, { ty
// reparse in `runRootCommand` parses it a second time). Mutating the input
// would corrupt that later parse, so never touch the caller's array.
const args = [...inputArgs];
const parseDeps = PARSE_DEPS;
const result: Args = {
messages: [],
fileArgs: [],
@@ -214,7 +214,7 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map<string, { ty
if (i + 1 < args.length && args[i + 1] !== PROFILE_BOOTSTRAP_BOUNDARY_ARG) {
const consumed = consumeBuiltInStringValue(arg, args, i + 1);
i = consumed.index;
STRING_SETTERS[arg](result, consumed.value, PARSE_DEPS);
STRING_SETTERS[arg](result, consumed.value, parseDeps);
}
} else if (OPTIONAL_VALUE_FLAGS.has(arg)) {
const config = OPTIONAL_FLAGS[arg];
@@ -332,6 +332,17 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map<string, { ty
return result;
}
/** Reject requested tool names absent from the fully discovered session registry. */
export function validateToolNames(requested: readonly string[] | undefined, known: readonly string[]): void {
if (!requested) return;
const knownNames = new Set(known);
const unknown = requested.filter(name => !knownNames.has(name));
if (unknown.length === 0) return;
throw new CliUsageError(
`Unknown tool${unknown.length === 1 ? "" : "s"} in --tools: ${unknown.join(", ")}. Valid tools: ${known.join(", ")}.`,
);
}
/**
* Emit a stderr error listing the unrecognized flags and return `true` when
* there were any. Caller is expected to exit with a non-zero status. Splitting
packages/coding-agent/src/cli/extension-flags.ts
+6 -10
View File
@@ -29,18 +29,14 @@ export interface ExtensionFlagSink {
* semantics and surfaces in `unknownFlags` — without consuming the following
* message or overwriting the built-in field. No built-in name list to maintain.
*
* Returns `null` when there is no sink or no registered extension flags, in
* which case the caller keeps its original startup parse (an extension-aware
* re-parse would be identical anyway).
* Returns `null` only when there is no sink. Once extensions have loaded, the
* reparse always runs so `--tools` can be validated against their registered
* tools even when no extension registered CLI flags.
*/
export function applyExtensionFlags(runner: ExtensionFlagSink | undefined, rawArgs: string[]): Args | null {
const extensionFlags = runner?.getFlags();
if (!runner || !extensionFlags || extensionFlags.size === 0) {
return null;
}
const parsed = parseArgs(rawArgs, extensionFlags);
// `parseArgs` only records registered extension flags in `unknownFlags`, so
// every entry here is a flag this runner owns that was actually passed.
if (!runner) return null;
const parsed = parseArgs(rawArgs, runner.getFlags());
// `parseArgs` records extension flag values in `unknownFlags`.
for (const [name, value] of parsed.unknownFlags) {
runner.setFlagValue(name, value);
}
packages/coding-agent/src/cli/flag-tables.ts
+2 -10
View File
@@ -47,7 +47,6 @@ import { CliUsageError } from "./usage-error";
export interface ParseDeps {
logger: { warn: (message: string, meta?: Record<string, unknown>) => void };
parseThinking: (value: string | null | undefined) => ConfiguredThinkingLevel | undefined;
builtinToolNames: readonly string[];
normalizeToolNames: (values: Iterable<string>) => string[];
thinkingEfforts: readonly string[];
}
@@ -189,15 +188,8 @@ export const STRING_SETTERS: Record<string, StringSetter> = {
.map(s => s.trim())
.filter(Boolean),
);
// An unknown name silently narrowing the toolset is worse than a failed
// launch: scripts keep running believing the tool is available (e.g. a
// stale `--tools bash,ssh` after the ssh tool's removal).
const unknown = names.filter(name => !deps.builtinToolNames.includes(name));
if (unknown.length > 0) {
throw new CliUsageError(
`Unknown tool${unknown.length === 1 ? "" : "s"} in --tools: ${unknown.join(", ")}. Valid tools: ${deps.builtinToolNames.join(", ")}.`,
);
}
// Validation runs after session tool discovery. At this point extension,
// custom, plugin-manifest, and MCP tools are not all known yet.
result.tools = names;
},
"--thinking": (result, value, deps) => {
packages/coding-agent/src/main.ts
+20 -2
View File
@@ -23,7 +23,7 @@ import {
} from "@oh-my-pi/pi-utils";
import chalk from "@oh-my-pi/pi-utils/chalk";
import { reset as resetCapabilities } from "./capability";
import { type Args, reportUnrecognizedFlags } from "./cli/args";
import { type Args, reportUnrecognizedFlags, validateToolNames } from "./cli/args";
import { applyExtensionFlags, type ExtensionFlagSink } from "./cli/extension-flags";
import { processFileArguments } from "./cli/file-processor";
import { buildInitialMessage } from "./cli/initial-message";
@@ -425,7 +425,18 @@ export function createAcpSessionFactory(args: AcpSessionFactoryOptions): AcpSess
if (args.parsedArgs.apiKey && !args.baseOptions.model && nextSession.model) {
args.authStorage.setRuntimeApiKey(nextSession.model.provider, args.parsedArgs.apiKey);
}
applyExtensionFlags(nextSession.extensionRunner, args.rawArgs);
const runner = nextSession.extensionRunner;
applyExtensionFlags(
runner
? {
getFlags: () => runner.getFlags(),
setFlagValue: (name, value) => {
runner.setFlagValue(name, value);
},
}
: undefined,
args.rawArgs,
);
return nextSession;
};
}
@@ -1685,6 +1696,13 @@ export async function runRootCommand(
preloadedExtensions: extensionsResult,
});
try {
validateToolNames(initialArgs.tools, session.getAllToolNames());
} catch (error) {
await session.dispose();
throw error;
}
// Cold-revive support: a `parked` subagent ref restored from disk (Agent Hub
// scan, collab mirror, resumed process) has a sessionFile but no in-memory
// reviver, so `ensureLive` (IRC sends, hub focus) would refuse it. Install a
packages/coding-agent/test/extension-flag-dispatch.test.ts
+4
View File
@@ -15,6 +15,10 @@ class FakeExtensionFlagSink implements ExtensionFlagSink {
]);
}
getToolNames(): readonly string[] {
return [];
}
setFlagValue(name: string, value: boolean | string): void {
this.#values.set(name, value);
}
packages/coding-agent/test/extension-flag-initial-message.test.ts
+2 -2
View File
@@ -221,8 +221,8 @@ describe("applyExtensionFlags (single-parser flag resolution)", () => {
it("returns null when there is no runner", () => {
expect(applyExtensionFlags(undefined, ["--spawn-peer", "x", "task"])).toBeNull();
});
it("returns null when the runner registered no flags", () => {
expect(applyExtensionFlags(fakeRunner({}), ["--whatever", "task"])).toBeNull();
it("reparses with an empty extension registry so unknown flags remain visible", () => {
expect(applyExtensionFlags(fakeRunner({}), ["--whatever", "task"])?.unrecognizedFlags).toEqual(["--whatever"]);
});
it("applies and strips a string flag in space form", () => {
const runner = fakeRunner({ "spawn-peer": "string" });
packages/coding-agent/test/flag-tables.test.ts
+19 -8
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from "bun:test";
import { parseArgs } from "../src/cli/args";
import { parseArgs, validateToolNames } from "../src/cli/args";
import { OPTIONAL_VALUE_FLAGS, STRING_VALUE_FLAGS } from "../src/cli/flag-tables";
import { CliUsageError } from "../src/cli/usage-error";
@@ -85,19 +85,30 @@ describe("--session-dir", () => {
});
});
describe("--tools legacy aliases", () => {
describe("--tools validation", () => {
it("maps search and find to grep and glob", () => {
const result = parseArgs(["--tools", "search,find,grep"]);
expect(result.tools).toEqual(["grep", "glob"]);
});
it("rejects unknown tool names instead of silently narrowing the toolset", () => {
// Removed tools (ssh, job, irc, launch, search_tool_bm25) used to be
// dropped with only a log-file warning, so `--tools bash,ssh` ran with
// just bash and no visible notice.
expect(() => parseArgs(["--tools", "bash,ssh"])).toThrow(CliUsageError);
expect(() => parseArgs(["--tools", "bash,ssh"])).toThrow(/Unknown tool in --tools: ssh/);
it("defers unknown-name validation until all session tools are discovered", () => {
expect(parseArgs(["--tools", "bash,intercom"]).tools).toEqual(["bash", "intercom"]);
expect(parseArgs(["--tools", "read,custom_tool"], new Map()).tools).toEqual(["read", "custom_tool"]);
});
});
describe("--tools discovered-registry validation", () => {
it("accepts extension and custom tools after they enter the session registry", () => {
expect(() =>
validateToolNames(["read", "intercom", "custom_tool"], ["read", "intercom", "custom_tool"]),
).not.toThrow();
});
it("rejects names absent from the final registry", () => {
expect(() => validateToolNames(["read", "missing"], ["read", "intercom", "custom_tool"])).toThrow(
/Unknown tool in --tools: missing/,
);
});
});