fix(coding-agent): harden profile bootstrap and aliases
This commit is contained in:
@@ -8,6 +8,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed profile bootstrap parsing so stripped `--profile`/`--alias` values no longer make optional or extension flags consume following prompt text, preserved standalone `--` as end-of-options after extension string flags, and made profile aliases respect `ZDOTDIR` while rejecting shell reserved words.
|
||||
- Made native user-level config discovery follow the active profile. Skills, rules, slash commands, prompts, instructions, hooks, tools, settings, extensions, MCP servers, and the top-level `SYSTEM.md`/`RULES.md`/`AGENTS.md` now resolve the user scope through `getAgentDir()`, so a named profile sees only its own `~/.omp/profiles/<name>/agent` config instead of the default profile's `~/.omp/agent` leaking into every profile. This matches the `/mcp` config writer and `getMCPConfigPath("user")`.
|
||||
- Fixed symlinked extension directories being skipped by native auto-discovery. The glob walker runs with `follow_links=false`, so a symlinked directory under `extensions/` was yielded as a symlink but never descended into — its `index.{ts,js}`/`package.json` stayed invisible while real directories loaded normally. `discoverExtensionModulePaths` now detects top-level symlinked directories and resolves their entry points, so an extension shared across profiles via a symlink loads like a real directory (symlinked extension *files* were already handled).
|
||||
## [15.9.1] - 2026-06-04
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
OPTIONAL_FLAGS,
|
||||
OPTIONAL_VALUE_FLAGS,
|
||||
type ParseDeps,
|
||||
PROFILE_BOOTSTRAP_BOUNDARY_ARG,
|
||||
STRING_SETTERS,
|
||||
STRING_VALUE_FLAGS,
|
||||
} from "./flag-tables";
|
||||
@@ -104,6 +105,9 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map<string, { ty
|
||||
passThrough = true;
|
||||
continue;
|
||||
}
|
||||
if (arg === PROFILE_BOOTSTRAP_BOUNDARY_ARG) {
|
||||
continue;
|
||||
}
|
||||
const flagIndex = i;
|
||||
|
||||
// Support --flag=value syntax (e.g. --tools=ask,read). The value is
|
||||
@@ -131,11 +135,10 @@ export function parseArgs(inputArgs: string[], extensionFlags?: Map<string, { ty
|
||||
if (extFlag.type === "boolean") {
|
||||
result.unknownFlags.set(flagName, true);
|
||||
} else if (extFlag.type === "string" && i + 1 < args.length) {
|
||||
// Consume the value in `--flag=value` form, when the next token is not
|
||||
// flag-looking, or when the next token is the end-of-options marker itself
|
||||
// (valid as a string flag value). Pass other flag-looking values as
|
||||
// `--flag=value`.
|
||||
if (equalsValueIndex !== -1 || args[i + 1] === "--" || !args[i + 1].startsWith("-")) {
|
||||
// Consume the value in `--flag=value` form or when the next token is not
|
||||
// flag-looking. A standalone `--` remains the end-of-options marker; use
|
||||
// `--flag=--` when an extension needs a literal "--" string value.
|
||||
if (equalsValueIndex !== -1 || !args[i + 1].startsWith("-")) {
|
||||
result.unknownFlags.set(flagName, args[++i]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -224,6 +224,13 @@ export const STRING_VALUE_FLAGS: ReadonlySet<string> = new Set(Object.keys(STRIN
|
||||
* {@link STRING_VALUE_FLAGS}.
|
||||
*/
|
||||
export const OPTIONAL_VALUE_FLAGS: ReadonlySet<string> = new Set(Object.keys(OPTIONAL_FLAGS));
|
||||
/**
|
||||
* Internal marker inserted by the profile bootstrap when removing `--profile`
|
||||
* or `--alias` would otherwise make the following value-like token become the
|
||||
* value of a preceding optional/extension flag. `parseArgs` ignores it, but its
|
||||
* flag-looking shape preserves argv boundaries during the second parse.
|
||||
*/
|
||||
export const PROFILE_BOOTSTRAP_BOUNDARY_ARG = "--omp-profile-boundary";
|
||||
|
||||
/**
|
||||
* Long-form launch flags that take NO value (booleans). The bootstrap pre-parser
|
||||
|
||||
@@ -48,6 +48,80 @@ export interface ProfileAliasInstallResult {
|
||||
}
|
||||
|
||||
const ALIAS_NAME_RE = /^[A-Za-z_][A-Za-z0-9_-]{0,63}$/;
|
||||
const POSIX_RESERVED_ALIAS_NAMES: ReadonlySet<string> = new Set([
|
||||
"case",
|
||||
"coproc",
|
||||
"do",
|
||||
"done",
|
||||
"elif",
|
||||
"else",
|
||||
"esac",
|
||||
"fi",
|
||||
"for",
|
||||
"function",
|
||||
"if",
|
||||
"in",
|
||||
"select",
|
||||
"then",
|
||||
"time",
|
||||
"until",
|
||||
"while",
|
||||
]);
|
||||
const FISH_RESERVED_ALIAS_NAMES: ReadonlySet<string> = new Set([
|
||||
"and",
|
||||
"begin",
|
||||
"break",
|
||||
"builtin",
|
||||
"case",
|
||||
"command",
|
||||
"continue",
|
||||
"else",
|
||||
"end",
|
||||
"exec",
|
||||
"for",
|
||||
"function",
|
||||
"if",
|
||||
"not",
|
||||
"or",
|
||||
"return",
|
||||
"switch",
|
||||
"while",
|
||||
]);
|
||||
const POWERSHELL_RESERVED_ALIAS_NAMES: ReadonlySet<string> = new Set([
|
||||
"begin",
|
||||
"break",
|
||||
"catch",
|
||||
"class",
|
||||
"continue",
|
||||
"data",
|
||||
"do",
|
||||
"dynamicparam",
|
||||
"else",
|
||||
"elseif",
|
||||
"end",
|
||||
"enum",
|
||||
"exit",
|
||||
"filter",
|
||||
"finally",
|
||||
"for",
|
||||
"foreach",
|
||||
"from",
|
||||
"function",
|
||||
"if",
|
||||
"in",
|
||||
"param",
|
||||
"process",
|
||||
"return",
|
||||
"switch",
|
||||
"throw",
|
||||
"trap",
|
||||
"try",
|
||||
"until",
|
||||
"using",
|
||||
"var",
|
||||
"while",
|
||||
"workflow",
|
||||
]);
|
||||
|
||||
// Keep local: importing the pi-utils root here would eagerly load env before
|
||||
// cli.ts has applied --profile, regressing profile-specific .env loading.
|
||||
@@ -55,7 +129,20 @@ function isEnoentError(error: unknown): boolean {
|
||||
return typeof error === "object" && error !== null && (error as { code?: unknown }).code === "ENOENT";
|
||||
}
|
||||
|
||||
function validateAliasName(aliasName: string): string {
|
||||
function getReservedAliasNames(shell: ProfileAliasShell): ReadonlySet<string> {
|
||||
switch (shell) {
|
||||
case "bash":
|
||||
case "zsh":
|
||||
return POSIX_RESERVED_ALIAS_NAMES;
|
||||
case "fish":
|
||||
return FISH_RESERVED_ALIAS_NAMES;
|
||||
case "powershell":
|
||||
case "pwsh":
|
||||
return POWERSHELL_RESERVED_ALIAS_NAMES;
|
||||
}
|
||||
}
|
||||
|
||||
function validateAliasName(aliasName: string, shell: ProfileAliasShell): string {
|
||||
const normalized = aliasName.trim();
|
||||
if (!ALIAS_NAME_RE.test(normalized)) {
|
||||
throw new Error(`Invalid alias "${aliasName}". Alias names must match ${ALIAS_NAME_RE.source}.`);
|
||||
@@ -63,6 +150,9 @@ function validateAliasName(aliasName: string): string {
|
||||
if (normalized.toLowerCase() === "omp") {
|
||||
throw new Error('Invalid alias "omp". Refusing to shadow the base omp command.');
|
||||
}
|
||||
if (getReservedAliasNames(shell).has(normalized.toLowerCase())) {
|
||||
throw new Error(`Invalid alias "${aliasName}". Refusing to create a ${shell} reserved word.`);
|
||||
}
|
||||
return normalized;
|
||||
}
|
||||
|
||||
@@ -125,7 +215,7 @@ function resolveShellConfigPath(
|
||||
): string {
|
||||
switch (shell) {
|
||||
case "zsh":
|
||||
return path.join(homeDir, ".zshrc");
|
||||
return path.join(env.ZDOTDIR || homeDir, ".zshrc");
|
||||
case "bash":
|
||||
return platform === "darwin" ? path.join(homeDir, ".bash_profile") : path.join(homeDir, ".bashrc");
|
||||
case "fish": {
|
||||
@@ -215,11 +305,11 @@ export async function installProfileAlias(options: ProfileAliasInstallOptions):
|
||||
if (!profile) {
|
||||
throw new Error("--alias requires a named --profile value.");
|
||||
}
|
||||
const aliasName = validateAliasName(options.aliasName);
|
||||
const platform = options.platform ?? process.platform;
|
||||
const homeDir = options.homeDir ?? os.homedir();
|
||||
const env = options.env ?? process.env;
|
||||
const shell = normalizeShellName(options.shellPath ?? env.SHELL, platform, env);
|
||||
const aliasName = validateAliasName(options.aliasName, shell);
|
||||
const configPath = resolveShellConfigPath(shell, homeDir, platform, env);
|
||||
const { block, command } = renderAliasBlock(shell, aliasName, profile, options.command ?? DEFAULT_ALIAS_COMMAND);
|
||||
const readFile = options.readFile ?? readProfileAliasConfigFile;
|
||||
|
||||
@@ -33,12 +33,33 @@
|
||||
*/
|
||||
|
||||
import { isSubcommand } from "../cli-commands";
|
||||
import { OPTIONAL_FLAGS, OPTIONAL_VALUE_FLAGS, STRING_VALUE_FLAGS, VALUELESS_FLAGS } from "./flag-tables";
|
||||
import {
|
||||
OPTIONAL_FLAGS,
|
||||
OPTIONAL_VALUE_FLAGS,
|
||||
PROFILE_BOOTSTRAP_BOUNDARY_ARG,
|
||||
STRING_VALUE_FLAGS,
|
||||
VALUELESS_FLAGS,
|
||||
} from "./flag-tables";
|
||||
|
||||
function isProfileBootstrapSubcommand(arg: string): boolean {
|
||||
return arg === "launch" || arg === "acp";
|
||||
}
|
||||
|
||||
function isUnknownLongValueCandidate(arg: string): boolean {
|
||||
return (
|
||||
arg.startsWith("--") &&
|
||||
!arg.includes("=") &&
|
||||
!STRING_VALUE_FLAGS.has(arg) &&
|
||||
!OPTIONAL_VALUE_FLAGS.has(arg) &&
|
||||
!VALUELESS_FLAGS.has(arg)
|
||||
);
|
||||
}
|
||||
|
||||
function needsBoundaryAfterGlobalStrip(stripped: readonly string[]): boolean {
|
||||
const previous = stripped[stripped.length - 1];
|
||||
return previous !== undefined && (OPTIONAL_VALUE_FLAGS.has(previous) || isUnknownLongValueCandidate(previous));
|
||||
}
|
||||
|
||||
export interface ProfileBootstrapResult {
|
||||
argv: string[];
|
||||
profile?: string;
|
||||
@@ -67,7 +88,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
let passThrough = false;
|
||||
let sawSubcommand = false;
|
||||
let canDispatchSubcommand = true;
|
||||
|
||||
let insertBoundaryBeforeNextValue = false;
|
||||
for (let index = 0; index < argv.length; index += 1) {
|
||||
const arg = argv[index];
|
||||
|
||||
@@ -76,6 +97,13 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
continue;
|
||||
}
|
||||
|
||||
if (insertBoundaryBeforeNextValue) {
|
||||
if (!arg.startsWith("-")) {
|
||||
stripped.push(PROFILE_BOOTSTRAP_BOUNDARY_ARG);
|
||||
}
|
||||
insertBoundaryBeforeNextValue = false;
|
||||
}
|
||||
|
||||
// `--` ends option processing. Anything that follows is forwarded verbatim
|
||||
// so users can pass arbitrary tokens (including a literal `--profile`) to
|
||||
// downstream tools without the bootstrap stealing them.
|
||||
@@ -91,6 +119,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
throw new Error("--profile requires a profile name");
|
||||
}
|
||||
profile = value;
|
||||
insertBoundaryBeforeNextValue = needsBoundaryAfterGlobalStrip(stripped);
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
@@ -100,6 +129,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
throw new Error("--profile requires a profile name");
|
||||
}
|
||||
profile = value;
|
||||
insertBoundaryBeforeNextValue = needsBoundaryAfterGlobalStrip(stripped);
|
||||
continue;
|
||||
}
|
||||
if (arg === "--alias") {
|
||||
@@ -108,6 +138,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
throw new Error("--alias requires a command name");
|
||||
}
|
||||
aliasName = value;
|
||||
insertBoundaryBeforeNextValue = needsBoundaryAfterGlobalStrip(stripped);
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
@@ -117,6 +148,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
throw new Error("--alias requires a command name");
|
||||
}
|
||||
aliasName = value;
|
||||
insertBoundaryBeforeNextValue = needsBoundaryAfterGlobalStrip(stripped);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -167,7 +199,7 @@ export function extractProfileFlags(argv: readonly string[]): ProfileBootstrapRe
|
||||
// single, consistent meaning instead of being swallowed as a flag value.
|
||||
// Known value-less launch flags are exempt so a trailing profile still
|
||||
// activates (`omp --print --profile work`).
|
||||
if (arg.startsWith("--") && !arg.includes("=") && !VALUELESS_FLAGS.has(arg)) {
|
||||
if (isUnknownLongValueCandidate(arg)) {
|
||||
canDispatchSubcommand = false;
|
||||
stripped.push(arg);
|
||||
const next = argv[index + 1];
|
||||
|
||||
@@ -30,13 +30,13 @@ describe("extension flag dispatch", () => {
|
||||
expect(args?.messages).toEqual(["--foo", "bar"]);
|
||||
});
|
||||
|
||||
it("still allows -- to be the value of a string extension flag", () => {
|
||||
it("keeps -- as end-of-options after a string extension flag", () => {
|
||||
const sink = new FakeExtensionFlagSink();
|
||||
|
||||
const args = applyExtensionFlags(sink, ["--bar", "--"]);
|
||||
const args = applyExtensionFlags(sink, ["--bar", "--", "--foo", "bar"]);
|
||||
|
||||
expect(sink.values.get("bar")).toBe("--");
|
||||
expect(sink.values.size).toBe(1);
|
||||
expect(args?.messages).toEqual([]);
|
||||
expect(sink.values.has("bar")).toBe(false);
|
||||
expect(sink.values.size).toBe(0);
|
||||
expect(args?.messages).toEqual(["--foo", "bar"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -54,6 +54,18 @@ describe("extension flags vs initial message", () => {
|
||||
expect(parsed.print).toBeUndefined();
|
||||
expect(parsed.messages).toEqual(["hello"]);
|
||||
});
|
||||
it("keeps standalone -- as end-of-options after a string extension flag", () => {
|
||||
const parsed = parseArgs(["--spawn-peer", "--", "--model", "opus", "hello"], extFlags);
|
||||
expect(parsed.unknownFlags.has("spawn-peer")).toBe(false);
|
||||
expect(parsed.model).toBeUndefined();
|
||||
expect(parsed.messages).toEqual(["--model", "opus", "hello"]);
|
||||
});
|
||||
it("consumes literal -- string values only in equals form", () => {
|
||||
const parsed = parseArgs(["--spawn-peer=--", "--model", "opus", "hello"], extFlags);
|
||||
expect(parsed.unknownFlags.get("spawn-peer")).toBe("--");
|
||||
expect(parsed.model).toBe("opus");
|
||||
expect(parsed.messages).toEqual(["hello"]);
|
||||
});
|
||||
it("treats an @-prefixed string value as the flag's value, not a file arg (P1#1)", () => {
|
||||
const parsed = parseArgs(["--spawn-peer", "@notes.md", "hello"], extFlags);
|
||||
expect(parsed.unknownFlags.get("spawn-peer")).toBe("@notes.md");
|
||||
|
||||
@@ -64,6 +64,26 @@ describe("profile alias installer", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("installs the zsh alias under ZDOTDIR when set", async () => {
|
||||
const files = new Map<string, string>();
|
||||
|
||||
const result = await installProfileAlias({
|
||||
profile: "work",
|
||||
aliasName: "omp-work",
|
||||
shellPath: "/bin/zsh",
|
||||
platform: "darwin",
|
||||
homeDir: "/Users/me",
|
||||
env: { ZDOTDIR: "/Users/me/.config/zsh" },
|
||||
readFile: async filePath => files.get(filePath) ?? "",
|
||||
writeFile: async (filePath, content) => {
|
||||
files.set(filePath, content);
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.configPath).toBe("/Users/me/.config/zsh/.zshrc");
|
||||
expect(files.get(result.configPath)).toContain("omp-work() {");
|
||||
});
|
||||
|
||||
it("writes a fish function that forwards argv", async () => {
|
||||
const files = new Map<string, string>();
|
||||
|
||||
@@ -263,6 +283,24 @@ describe("profile alias installer", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects shell reserved words before rendering alias functions", async () => {
|
||||
for (const { aliasName, shellPath } of [
|
||||
{ aliasName: "if", shellPath: "/bin/bash" },
|
||||
{ aliasName: "end", shellPath: "/opt/homebrew/bin/fish" },
|
||||
{ aliasName: "foreach", shellPath: "pwsh.exe" },
|
||||
]) {
|
||||
await expect(
|
||||
installProfileAlias({
|
||||
profile: "work",
|
||||
aliasName,
|
||||
shellPath,
|
||||
platform: shellPath === "pwsh.exe" ? "win32" : "linux",
|
||||
homeDir: "/home/me",
|
||||
}),
|
||||
).rejects.toThrow("reserved word");
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects POSIX sh because it does not read bash config files", async () => {
|
||||
await expect(
|
||||
installProfileAlias({
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { parseArgs } from "../src/cli/args";
|
||||
import { PROFILE_BOOTSTRAP_BOUNDARY_ARG } from "../src/cli/flag-tables";
|
||||
import { extractProfileFlags } from "../src/cli/profile-bootstrap";
|
||||
|
||||
describe("extractProfileFlags", () => {
|
||||
@@ -61,6 +63,32 @@ describe("extractProfileFlags", () => {
|
||||
expect(filePrefixed.profile).toBe("work");
|
||||
});
|
||||
|
||||
it("preserves optional-flag boundaries when stripping a profile before prompt text", () => {
|
||||
const extracted = extractProfileFlags(["--resume", "--profile", "work", "follow up"]);
|
||||
expect(extracted).toEqual({
|
||||
argv: ["--resume", PROFILE_BOOTSTRAP_BOUNDARY_ARG, "follow up"],
|
||||
profile: "work",
|
||||
aliasName: undefined,
|
||||
});
|
||||
|
||||
const parsed = parseArgs(extracted.argv);
|
||||
expect(parsed.resume).toBe(true);
|
||||
expect(parsed.messages).toEqual(["follow up"]);
|
||||
});
|
||||
|
||||
it("preserves extension-flag boundaries when stripping a profile before prompt text", () => {
|
||||
const extracted = extractProfileFlags(["--some-ext-flag", "--profile", "work", "follow up"]);
|
||||
expect(extracted).toEqual({
|
||||
argv: ["--some-ext-flag", PROFILE_BOOTSTRAP_BOUNDARY_ARG, "follow up"],
|
||||
profile: "work",
|
||||
aliasName: undefined,
|
||||
});
|
||||
|
||||
const parsed = parseArgs(extracted.argv, new Map([["some-ext-flag", { type: "string" }]]));
|
||||
expect(parsed.unknownFlags.has("some-ext-flag")).toBe(false);
|
||||
expect(parsed.messages).toEqual(["follow up"]);
|
||||
});
|
||||
|
||||
it("does not consume empty-string resume values before a trailing profile", () => {
|
||||
// Shared OPTIONAL_FLAGS metadata drives the bootstrap too. Empty string is
|
||||
// "no value" for resume/session aliases, so the bootstrap must release it
|
||||
@@ -212,10 +240,9 @@ describe("extractProfileFlags", () => {
|
||||
});
|
||||
|
||||
it("treats a `--` successor of an unknown flag as end-of-options, not a protected value", () => {
|
||||
// `--` is ambiguous under the parser (a string extension flag consumes it,
|
||||
// a boolean one does not), so the bootstrap keeps `--` a single consistent
|
||||
// meaning: end-of-options. Everything after is forwarded verbatim and no
|
||||
// profile is extracted, so a `--profile` fenced behind `--` never silently
|
||||
// `--` is the parser's end-of-options marker even after a string extension
|
||||
// flag. The bootstrap keeps that single meaning: everything after is
|
||||
// forwarded verbatim, so a `--profile` fenced behind `--` never silently
|
||||
// activates.
|
||||
expect(extractProfileFlags(["--some-ext-flag", "--", "--profile", "work"])).toEqual({
|
||||
argv: ["--some-ext-flag", "--", "--profile", "work"],
|
||||
|
||||
Reference in New Issue
Block a user