refactor(core): remove shell description input (#32823)
This commit is contained in:
parent
fbf889db83
commit
d29f5eba92
27 changed files with 124 additions and 161 deletions
|
|
@ -24,23 +24,6 @@ exports[`tool parameters JSON Schema (wire shape) bash 1`] = `
|
|||
"description": "The command to execute",
|
||||
"type": "string",
|
||||
},
|
||||
"description": {
|
||||
"description":
|
||||
"Clear, concise description of what this command does in 5-10 words. Examples:
|
||||
Input: ls
|
||||
Output: Lists files in current directory
|
||||
|
||||
Input: git status
|
||||
Output: Shows working tree status
|
||||
|
||||
Input: npm install
|
||||
Output: Installs package dependencies
|
||||
|
||||
Input: mkdir foo
|
||||
Output: Creates directory 'foo'"
|
||||
,
|
||||
"type": "string",
|
||||
},
|
||||
"timeout": {
|
||||
"description": "Optional timeout in milliseconds",
|
||||
"exclusiveMinimum": 0,
|
||||
|
|
@ -55,7 +38,6 @@ Output: Creates directory 'foo'"
|
|||
},
|
||||
"required": [
|
||||
"command",
|
||||
"description",
|
||||
],
|
||||
"type": "object",
|
||||
}
|
||||
|
|
|
|||
|
|
@ -106,19 +106,16 @@ describe("tool parameters", () => {
|
|||
})
|
||||
|
||||
describe("shell", () => {
|
||||
test("accepts minimum: command + description", () => {
|
||||
expect(parse(Shell, { command: "ls", description: "list" })).toEqual({ command: "ls", description: "list" })
|
||||
test("accepts command", () => {
|
||||
expect(parse(Shell, { command: "ls" })).toEqual({ command: "ls" })
|
||||
})
|
||||
test("accepts optional timeout + workdir", () => {
|
||||
const parsed = parse(Shell, { command: "ls", description: "list", timeout: 5000, workdir: "/tmp" })
|
||||
const parsed = parse(Shell, { command: "ls", timeout: 5000, workdir: "/tmp" })
|
||||
expect(parsed.timeout).toBe(5000)
|
||||
expect(parsed.workdir).toBe("/tmp")
|
||||
})
|
||||
test("rejects missing description", () => {
|
||||
expect(accepts(Shell, { command: "ls" })).toBe(false)
|
||||
})
|
||||
test("rejects missing command", () => {
|
||||
expect(accepts(Shell, { description: "list" })).toBe(false)
|
||||
expect(accepts(Shell, {})).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -182,7 +182,6 @@ describe("tool.shell", () => {
|
|||
Effect.gen(function* () {
|
||||
const result = yield* run({
|
||||
command: "echo test",
|
||||
description: "Echo test message",
|
||||
})
|
||||
expect(result.metadata.exit).toBe(0)
|
||||
expect(result.metadata.output).toContain("test")
|
||||
|
|
@ -204,7 +203,6 @@ describe("tool.shell", () => {
|
|||
const result = yield* bash.execute(
|
||||
{
|
||||
command: "echo fallback",
|
||||
description: "Echo fallback text",
|
||||
},
|
||||
ctx,
|
||||
)
|
||||
|
|
@ -227,7 +225,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "echo hello",
|
||||
description: "Echo hello",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -249,7 +246,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "echo foo && echo bar",
|
||||
description: "Echo twice",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -273,7 +269,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "Write-Host foo; if ($?) { Write-Host bar }",
|
||||
description: "Check PowerShell conditional",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -303,7 +298,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: "Remove-Item -Recurse tmp",
|
||||
description: "Remove a temp directory",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -331,7 +325,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: `cat ${file}`,
|
||||
description: "Read wildcard path",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -359,7 +352,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: `echo $(cat "${file}")`,
|
||||
description: "Read nested bash file",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -389,7 +381,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: `Copy-Item -PassThru "${process.env.WINDIR!.replaceAll("\\", "/")}/win.ini" ./out`,
|
||||
description: "Copy Windows ini",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -415,7 +406,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: `Write-Output $(Get-Content ${file})`,
|
||||
description: "Read nested PowerShell file",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -446,7 +436,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: 'Get-Content "C:../outside.txt"',
|
||||
description: "Read drive-relative file",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -474,7 +463,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: 'Get-Content "$HOME/.ssh/config"',
|
||||
description: "Read home config",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -503,7 +491,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: 'Get-Content "$PWD/../outside.txt"',
|
||||
description: "Read pwd-relative file",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -531,7 +518,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: 'Get-Content "$PSHOME/outside.txt"',
|
||||
description: "Read pshome file",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -567,7 +553,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: `Get-Content -Path "${root}$env:${key}\\Windows\\win.ini"`,
|
||||
description: "Read Windows ini with missing env",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -598,7 +583,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "Get-Content $env:WINDIR/win.ini",
|
||||
description: "Read Windows ini from env",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -626,7 +610,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: `Get-Content -Path FileSystem::${process.env.WINDIR!.replaceAll("\\", "/")}/win.ini`,
|
||||
description: "Read Windows ini from FileSystem provider",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -655,7 +638,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: "Get-Content ${env:WINDIR}/win.ini",
|
||||
description: "Read Windows ini from braced env",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -682,7 +664,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "Set-Location C:/Windows",
|
||||
description: "Change location",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -710,7 +691,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "Write-Output ('a' * 3)",
|
||||
description: "Write repeated text",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -736,7 +716,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: `TYPE "${path.join(process.env.WINDIR!, "win.ini")}"`,
|
||||
description: "Read Windows ini with cmd",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -761,7 +740,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: "cd ../",
|
||||
description: "Change to parent directory",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -786,7 +764,6 @@ describe("tool.shell permissions", () => {
|
|||
{
|
||||
command: "echo ok",
|
||||
workdir: os.tmpdir(),
|
||||
description: "Echo from temp dir",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -817,7 +794,6 @@ describe("tool.shell permissions", () => {
|
|||
{
|
||||
command: "echo ok",
|
||||
workdir: dir,
|
||||
description: "Echo from external dir",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -850,7 +826,6 @@ describe("tool.shell permissions", () => {
|
|||
{
|
||||
command: "echo ok",
|
||||
workdir: "/tmp",
|
||||
description: "Echo from Git Bash tmp",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -878,7 +853,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: "cat /tmp/opencode-does-not-exist",
|
||||
description: "Read Git Bash tmp file",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -910,7 +884,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* fail(
|
||||
{
|
||||
command: `cat ${filepath}`,
|
||||
description: "Read external file",
|
||||
},
|
||||
capture(requests, err),
|
||||
),
|
||||
|
|
@ -922,7 +895,6 @@ describe("tool.shell permissions", () => {
|
|||
expect(extDirReq!.always).toContain(expected)
|
||||
expect(extDirReq!.metadata).toMatchObject({
|
||||
command: `cat ${filepath}`,
|
||||
description: "Read external file",
|
||||
directories: [outerTmp],
|
||||
patterns: [expected],
|
||||
})
|
||||
|
|
@ -942,7 +914,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: `rm -rf ${path.join(tmp, "nested")}`,
|
||||
description: "Remove nested dir",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -963,7 +934,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "git log --oneline -5",
|
||||
description: "Git log",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -985,7 +955,6 @@ describe("tool.shell permissions", () => {
|
|||
yield* run(
|
||||
{
|
||||
command: "cd .",
|
||||
description: "Stay in current directory",
|
||||
},
|
||||
capture(requests),
|
||||
)
|
||||
|
|
@ -1006,7 +975,7 @@ describe("tool.shell permissions", () => {
|
|||
const requests: Array<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
|
||||
expect(
|
||||
yield* fail(
|
||||
{ command: "echo test > output.txt", description: "Redirect test output" },
|
||||
{ command: "echo test > output.txt" },
|
||||
capture(requests, err),
|
||||
),
|
||||
).toMatchObject({ message: err.message })
|
||||
|
|
@ -1025,7 +994,7 @@ describe("tool.shell permissions", () => {
|
|||
tmp,
|
||||
Effect.gen(function* () {
|
||||
const requests: Array<Omit<PermissionV1.Request, "id" | "sessionID" | "tool">> = []
|
||||
yield* run({ command: "ls -la", description: "List" }, capture(requests))
|
||||
yield* run({ command: "ls -la" }, capture(requests))
|
||||
const bashReq = requests.find((r) => r.permission === "bash")
|
||||
expect(bashReq).toBeDefined()
|
||||
expect(bashReq!.always[0]).toBe("ls *")
|
||||
|
|
@ -1047,7 +1016,6 @@ describe("tool.shell abort", () => {
|
|||
const res = yield* run(
|
||||
{
|
||||
command: `echo before && sleep 30`,
|
||||
description: "Long running command",
|
||||
},
|
||||
{
|
||||
...ctx,
|
||||
|
|
@ -1078,7 +1046,6 @@ describe("tool.shell abort", () => {
|
|||
Effect.gen(function* () {
|
||||
const result = yield* run({
|
||||
command: `sleep 60`,
|
||||
description: "Timeout test",
|
||||
timeout: 500,
|
||||
})
|
||||
expect(result.output).toContain("shell tool terminated command after exceeding timeout")
|
||||
|
|
@ -1099,7 +1066,6 @@ describe("tool.shell abort", () => {
|
|||
const result = yield* tool.execute(
|
||||
{
|
||||
command: `sleep 60`,
|
||||
description: "Default timeout test",
|
||||
},
|
||||
ctx,
|
||||
)
|
||||
|
|
@ -1116,7 +1082,6 @@ describe("tool.shell abort", () => {
|
|||
Effect.gen(function* () {
|
||||
const result = yield* run({
|
||||
command: `echo stdout_msg && echo stderr_msg >&2`,
|
||||
description: "Stderr test",
|
||||
})
|
||||
expect(result.output).toContain("stdout_msg")
|
||||
expect(result.output).toContain("stderr_msg")
|
||||
|
|
@ -1132,7 +1097,6 @@ describe("tool.shell abort", () => {
|
|||
Effect.gen(function* () {
|
||||
const result = yield* run({
|
||||
command: `exit 42`,
|
||||
description: "Non-zero exit",
|
||||
})
|
||||
expect(result.metadata.exit).toBe(42)
|
||||
}),
|
||||
|
|
@ -1147,7 +1111,6 @@ describe("tool.shell abort", () => {
|
|||
const result = yield* run(
|
||||
{
|
||||
command: `echo first && sleep 0.1 && echo second`,
|
||||
description: "Streaming test",
|
||||
},
|
||||
{
|
||||
...ctx,
|
||||
|
|
@ -1174,7 +1137,6 @@ describe("tool.shell truncation", () => {
|
|||
const lineCount = Truncate.MAX_LINES + 500
|
||||
const result = yield* run({
|
||||
command: fill("lines", lineCount),
|
||||
description: "Generate lines exceeding limit",
|
||||
})
|
||||
mustTruncate(result)
|
||||
expect(result.output).toMatch(/\.\.\.output truncated\.\.\./)
|
||||
|
|
@ -1190,7 +1152,6 @@ describe("tool.shell truncation", () => {
|
|||
const byteCount = Truncate.MAX_BYTES + 10000
|
||||
const result = yield* run({
|
||||
command: fill("bytes", byteCount),
|
||||
description: "Generate bytes exceeding limit",
|
||||
})
|
||||
mustTruncate(result)
|
||||
expect(result.output).toMatch(/\.\.\.output truncated\.\.\./)
|
||||
|
|
@ -1205,7 +1166,6 @@ describe("tool.shell truncation", () => {
|
|||
Effect.gen(function* () {
|
||||
const result = yield* run({
|
||||
command: fill("lines", 1),
|
||||
description: "Generate one line",
|
||||
})
|
||||
expect((result.metadata as { truncated?: boolean }).truncated).toBe(false)
|
||||
expect(result.output).toContain("1")
|
||||
|
|
@ -1220,7 +1180,6 @@ describe("tool.shell truncation", () => {
|
|||
const lineCount = Truncate.MAX_LINES + 100
|
||||
const result = yield* run({
|
||||
command: fill("lines", lineCount),
|
||||
description: "Generate lines for file check",
|
||||
})
|
||||
mustTruncate(result)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue