Repository navigation
[Feat] Tell the model which shell syntax execute_command uses (cmd.exe / PowerShell) #2115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { describeShellDialect } from "../shellDialect" | ||
|
|
||
| describe("describeShellDialect", () => { | ||
| it.each([ | ||
| { shell: "C:\\Windows\\System32\\cmd.exe", expected: "cmd.exe" }, | ||
| { shell: "C:\\Windows\\System32\\CMD.EXE", expected: "cmd.exe" }, | ||
| { shell: "C:\\Program Files\\PowerShell\\7\\pwsh.exe", expected: "PowerShell" }, | ||
| { shell: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", expected: "PowerShell" }, | ||
| ])("names the dialect for $shell", ({ shell, expected }) => { | ||
| expect(describeShellDialect(shell)).toContain(expected) | ||
| }) | ||
|
|
||
| it("warns that ';' does not chain commands in cmd.exe", () => { | ||
| expect(describeShellDialect("C:\\Windows\\System32\\cmd.exe")).toContain("`;` is not a separator") | ||
| }) | ||
|
|
||
| it.each(["/bin/bash", "/bin/zsh", "C:\\Program Files\\Git\\bin\\bash.exe", "C:\\Windows\\System32\\wsl.exe"])( | ||
| "adds nothing for POSIX shell %s", | ||
| (shell) => { | ||
| expect(describeShellDialect(shell)).toBeUndefined() | ||
| }, | ||
| ) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /** | ||
| * Describes the command syntax of Windows-native shells for the system prompt. | ||
| * | ||
| * Rules and habits written for bash silently do the wrong thing under these shells | ||
| * (e.g. `;` is not a command separator in cmd.exe), so the model is told which | ||
| * dialect it is writing for. Returns undefined for POSIX shells, where the shell | ||
| * path already names the dialect. | ||
| */ | ||
| export function describeShellDialect(shellPath: string): string | undefined { | ||
| // path.basename follows the host platform; split on both separators so Windows paths work anywhere. | ||
| const name = (shellPath.split(/[\\/]/).pop() ?? "").toLowerCase() | ||
|
Check warning on line 11 in src/utils/shellDialect.ts
|
||
|
|
||
| if (name === "cmd.exe" || name === "cmd") { | ||
|
Check warning on line 13 in src/utils/shellDialect.ts
|
||
| return ( | ||
| "Commands run through Windows cmd.exe, not bash. Use cmd syntax: `&` or `&&` to chain commands " + | ||
| "(`;` is not a separator), `%VAR%` for variables, `set X=Y`, `%ERRORLEVEL%` for exit codes, " + | ||
| "and `dir`, `type`, `findstr`. POSIX tools may be missing or shadowed by other binaries on PATH." | ||
| ) | ||
| } | ||
|
|
||
| if (name === "powershell.exe" || name === "pwsh.exe" || name === "powershell" || name === "pwsh") { | ||
|
Check warning on line 21 in src/utils/shellDialect.ts
|
||
| return ( | ||
| "Commands run through PowerShell, not bash. Use PowerShell syntax: `;` to chain commands, " + | ||
| "`$env:VAR` for environment variables, `$LASTEXITCODE` for exit codes, and cmdlets such as " + | ||
| "`Get-ChildItem` and `Select-String`." | ||
|
Comment on lines
+24
to
+25
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Distinguish cmdlet success from native exit codes. If a generated command runs As per path instructions, “Verify PR claims against implementation and contracts.” 🧰 Tools🪛 GitHub Check: mutation-diff[warning] 25-25: Mutation test advisory [warning] 24-24: Mutation test advisory 🤖 Prompt for AI AgentsSource: Path instructions |
||
| ) | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 2479
Assert syntax tokens for each dialect.
The name assertion can pass when the syntax guidance is removed. The existing
cmd.exetest covers only the semicolon warning. Add assertions for command chaining, variables, and exit-code guidance for both dialects.Suggested test assertions
🤖 Prompt for AI Agents