diff --git a/plugins/tracing/src/skills.ts b/plugins/tracing/src/skills.ts index 7fe3dc6..9f3acad 100644 --- a/plugins/tracing/src/skills.ts +++ b/plugins/tracing/src/skills.ts @@ -19,6 +19,81 @@ const SKILL_PROMPT_FRAGMENT = /\s*([^<\n]+)<\/name>/g; const MAX_SKILL_NAME_LENGTH = 64; +// Commands that read or execute the path they are given. Commands that only +// stage or move paths (git add, cp, rm, ...) are deliberately absent: a skill +// path in `git add -- .agents/skills/x/SKILL.md` is not a skill invocation. +const SKILL_READ_COMMANDS = new Set([ + "awk", + "bat", + "bash", + "bun", + "bunx", + "cargo", + "cat", + "code", + "deno", + "dotnet", + "fish", + "go", + "grep", + "head", + "java", + "less", + "more", + "node", + "npm", + "npx", + "open", + "perl", + "php", + "pnpm", + "powershell", + "pwsh", + "python", + "python3", + "rg", + "ruby", + "sed", + "sh", + "source", + "tail", + "uv", + "uvx", + "yarn", + "zsh", +]); +const SHELL_INTERPRETERS = new Set(["bash", "fish", "powershell", "pwsh", "sh", "zsh"]); +const GIT_READ_SUBCOMMANDS = new Set(["blame", "cat-file", "diff", "grep", "log", "show"]); +const SHELL_WRAPPERS = new Set([ + "builtin", + "command", + "env", + "nice", + "nohup", + "stdbuf", + "sudo", + "time", + "timeout", + "xargs", +]); +const WRAPPER_OPTIONS_WITH_VALUE = new Set([ + "-g", + "--group", + "-k", + "--kill-after", + "-n", + "--adjustment", + "-p", + "--prompt", + "-s", + "--signal", + "-u", + "--user", +]); + +const NESTED_COMMAND = + /(?:cmd|command)\s*:\s*(?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)'|`((?:[^`\\]|\\.)*)`)/g; + function skillAccessText(tc: ToolCall): string | undefined { if (!SKILL_ACCESS_TOOLS.has(tc.name)) return undefined; if (typeof tc.args === "string") return tc.args; @@ -50,10 +125,115 @@ function collect(text: string, patterns: RegExp[]): string[] { return names; } +function baseName(token: string): string { + return token.split(/[\\/]/).pop() ?? token; +} + +function tokenizeShell(segment: string): string[] { + const tokens: string[] = []; + const pattern = /"([^"]*)"|'([^']*)'|`([^`]*)`|([^\s]+)/g; + for (const match of segment.matchAll(pattern)) { + const token = match[1] ?? match[2] ?? match[3] ?? match[4]; + if (token) tokens.push(token); + } + return tokens; +} + +function shellSegments(text: string): string[] { + return text.split(/\s*(?:&&|\|\||;|\n|\|)\s*/).filter((segment) => segment.trim().length > 0); +} + +function unwrapShellTokens(tokens: string[]): string[] { + let index = 0; + while (index < tokens.length) { + if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(tokens[index])) { + index += 1; + continue; + } + const command = baseName(tokens[index]); + if (!SHELL_WRAPPERS.has(command)) break; + index += 1; + while (index < tokens.length && tokens[index].startsWith("-")) { + index += WRAPPER_OPTIONS_WITH_VALUE.has(tokens[index]) ? 2 : 1; + } + if (command === "timeout" && index < tokens.length && /^\d/.test(tokens[index])) { + index += 1; + } + } + return tokens.slice(index); +} + +function isSkillPathToken(token: string): boolean { + return /\/SKILL\.md$/.test(token) || /\/skills\/(?:\.[^/]+\/)?[^/]+\/scripts\//.test(token); +} + +function commandReadsSkillPath(tokens: string[]): boolean { + const commandTokens = unwrapShellTokens(tokens); + if (commandTokens.length === 0) return false; + // Codex sometimes records a bare script path (no interpreter). That is an + // execution, unlike a path passed to a command that only stages it. + if (commandTokens.length === 1) return isSkillPathToken(commandTokens[0]); + + const command = baseName(commandTokens[0]); + if (command === "git") { + const subcommand = commandTokens.slice(1).find((token) => !token.startsWith("-")); + return subcommand !== undefined && GIT_READ_SUBCOMMANDS.has(subcommand); + } + if (SHELL_INTERPRETERS.has(command)) { + const commandFlag = commandTokens.indexOf("-c"); + if (commandFlag >= 0 && commandTokens[commandFlag + 1]) { + return shellSegments(commandTokens[commandFlag + 1]).some((segment) => + commandReadsSkillPath(tokenizeShell(segment)), + ); + } + return true; + } + return SKILL_READ_COMMANDS.has(command); +} + +function nestedShellCommands(text: string): string[] { + const commands: string[] = []; + for (const match of text.matchAll(NESTED_COMMAND)) { + const command = match[1] ?? match[2] ?? match[3]; + if (command) commands.push(command); + } + return commands; +} + +function skillsFromShellText(text: string): string[] { + const names: string[] = []; + for (const segment of shellSegments(text)) { + if (!commandReadsSkillPath(tokenizeShell(segment))) continue; + for (const name of collect(segment, [SKILL_DOC, SKILL_SCRIPT])) { + if (!names.includes(name)) names.push(name); + } + } + return names; +} + +function uniqueNames(groups: string[][]): string[] { + const names: string[] = []; + for (const group of groups) { + for (const name of group) { + if (!names.includes(name)) names.push(name); + } + } + return names; +} + /** Skills a tool call picked up by reading a `SKILL.md` or running a skill script. */ export function skillsForToolCall(tc: ToolCall): string[] { const text = skillAccessText(tc); - return text ? collect(text, [SKILL_DOC, SKILL_SCRIPT]) : []; + if (!text) return []; + if (tc.name === "read_mcp_resource") return collect(text, [SKILL_DOC, SKILL_SCRIPT]); + + // Code-mode calls carry a nested `exec_command({cmd: "..."})`; classify the + // nested command instead of trusting the surrounding program text. + const nestedCommands = nestedShellCommands(text); + if (nestedCommands.length > 0) { + return uniqueNames(nestedCommands.map(skillsFromShellText)); + } + return skillsFromShellText(text); } /** Skills explicitly invoked with the prompt, from Codex's injected `` fragments. */ diff --git a/plugins/tracing/test/skills.test.ts b/plugins/tracing/test/skills.test.ts index e4719e7..fc7f5de 100644 --- a/plugins/tracing/test/skills.test.ts +++ b/plugins/tracing/test/skills.test.ts @@ -10,7 +10,11 @@ function toolCall(name: string, args: unknown): ToolCall { } function exec(cmd: string): ToolCall { - return toolCall("exec_command", { cmd, workdir: "/repo", max_output_tokens: 10_000 }); + return toolCall("exec_command", { + cmd, + workdir: "/repo", + max_output_tokens: 10_000, + }); } function turnWith(toolCalls: ToolCall[], promptSkills: string[] = []): Turn { @@ -78,6 +82,55 @@ describe("skillsForToolCall", () => { ), ).toEqual([]); }); + + it("ignores skill paths that a command only stages, copies, or prints", () => { + expect(skillsForToolCall(exec("git add -- .agents/skills/not-loaded/SKILL.md"))).toEqual([]); + expect( + skillsForToolCall(exec("git add -- .agents/skills/not-loaded/scripts/check.py")), + ).toEqual([]); + expect(skillsForToolCall(exec("git status -- .agents/skills/not-loaded/SKILL.md"))).toEqual([]); + expect( + skillsForToolCall(exec("cp .agents/skills/not-loaded/SKILL.md /tmp/not-loaded.md")), + ).toEqual([]); + expect(skillsForToolCall(exec("echo .agents/skills/not-loaded/SKILL.md"))).toEqual([]); + }); + + it("keeps reads behind wrappers, shell -c, and git read subcommands", () => { + expect( + skillsForToolCall(exec("sudo -u root cat .agents/skills/security-review/SKILL.md")), + ).toEqual(["security-review"]); + expect( + skillsForToolCall(exec("env FOO=1 cat .agents/skills/security-review/SKILL.md")), + ).toEqual(["security-review"]); + expect( + skillsForToolCall(exec('bash -c "cat .agents/skills/security-review/SKILL.md"')), + ).toEqual(["security-review"]); + expect(skillsForToolCall(exec('bash -c "git add .agents/skills/not-loaded/SKILL.md"'))).toEqual( + [], + ); + expect( + skillsForToolCall(exec("git show HEAD:.agents/skills/security-review/SKILL.md")), + ).toEqual(["security-review"]); + }); + + it("classifies nested code-mode commands instead of the surrounding program", () => { + expect( + skillsForToolCall( + toolCall( + "exec", + 'await tools.exec_command({cmd:"git add .agents/skills/not-loaded/SKILL.md"});', + ), + ), + ).toEqual([]); + expect( + skillsForToolCall( + toolCall( + "exec", + 'await tools.exec_command({cmd:"cat .agents/skills/security-review/SKILL.md"});', + ), + ), + ).toEqual(["security-review"]); + }); }); describe("skillsForPrompt", () => {