Fix: refuse a skill script whose language the tool cannot launch - #880
Open
AmaadMartin wants to merge 3 commits into
Open
Fix: refuse a skill script whose language the tool cannot launch#880AmaadMartin wants to merge 3 commits into
AmaadMartin wants to merge 3 commits into
Conversation
added 3 commits
August 9, 2026 10:48
RunSkillScriptTool mapped a .ts script to CodeExecutionLanguage.TYPESCRIPT and emitted a ts-node wrapper that no code executor accepts. UnsafeLocalCodeExecutor answered with a success-shaped result, so the model read the failure as a script that ran and printed nothing. buildWrapperCode now returns undefined for a language it has no arm for, and the tool returns UNSUPPORTED_SCRIPT_LANGUAGE before it calls the executor. The switch stays the single source of truth for what the tool can launch.
Unit cases pin both spellings of a .ts path, an extension with no language, and that the executor is never called. Two cases pin the Python and Shell wrappers so the buildWrapperCode signature change stays behaviour-preserving. The integration case runs the refusal against a real UnsafeLocalCodeExecutor.
The resolved CodeExecutionLanguage is an internal value: an unmapped extension reported 'unspecified' in a sentence that then lists extensions. The message now names the extension the caller wrote. Also drops a test-only interface in the integration case that no assertion consulted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A
Problem:
RunSkillScriptToolmaps a.tsscript toCodeExecutionLanguage.TYPESCRIPTand emits arequire('ts-node/register')wrapper that no code executor in this repo accepts.UnsafeLocalCodeExecutoranswers with{stdout: '', stderr: 'Unsupported language: typescript', outputFiles: []}, which the tool forwards unchanged, so the model reads a failure as a script that ran and printed nothing.AgentEngineSandboxCodeExecutorthrows instead, so the same user mistake produces two different shapes. An extension with no language mapping is a third shape:buildWrapperCodethrows and thecatchmislabels itEXECUTION_ERROR.Solution:
buildWrapperCodenow returnsundefinedfor a language it has no arm for, and the tool returnsUNSUPPORTED_SCRIPT_LANGUAGEbefore it calls the executor. Theswitchalone decides what the tool can launch;SUPPORTED_SCRIPT_EXTENSIONSis message text only and must be kept in step with theswitchby hand. The dead TypeScript arm is gone. The model-facingscript_pathdescription now names the accepted extensions.The refusal message names the extension the caller wrote, not the resolved
CodeExecutionLanguage:Script 'scripts/hello.ts' has unsupported extension '.ts'. Skill scripts must be one of: .js, .py, .sh, .ps1, .bat, .cmd.An unmapped extension would otherwise report the internal enum value'unspecified'in a sentence that then lists extensions.Stacked on #356. The collision check found #356 (
feat/run-skill-script-error-code-enum), which adds theRunSkillScriptErrorCodeenum this change needs an eighth member of, so this PR branches from it instead ofmainand adds only the guard. I scanned all 764 open PRs on the fork: #634 and #729 also editrun_skill_script_tool.ts, but in different hunks (the resource-key lookup and the not-found counter), and no open PR adds this guard.Behaviour change on two already-broken paths. A
.tsscript moves from a silent empty success to a typed error. An unknown extension moves fromEXECUTION_ERRORtoUNSUPPORTED_SCRIPT_LANGUAGE; no test pinned the old value..js,.py,.sh,.ps1,.batand.cmdproduce byte-identical wrappers.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
CI did not run on this PR:
.github/workflows/validation.yamltriggers onpull_request: branches: [main], and this PR targetsfeat/run-skill-script-error-code-enum. The commands above were run locally on the pushed commit.I proved each new test can fail. Three mutations of the source, each reverted afterwards:
case CodeExecutionLanguage.TYPESCRIPTand deleted the guard. The three refusal unit cases and the integration case failed. The integration failure is the original bug: expected"errorCode": "UNSUPPORTED_SCRIPT_LANGUAGE", received{"outputFiles": [], "stderr": "Unsupported language: typescript", "stdout": ""}.UNSUPPORTED_SCRIPT_LANGUAGEto'UNSUPPORTED_LANGUAGE'. The enum-pinning case failed:expected 'UNSUPPORTED_LANGUAGE' to be 'UNSUPPORTED_SCRIPT_LANGUAGE'.PYTHONarm ofbuildWrapperCode. The Python wrapper case failed:expected undefined to be 'import runpy\nrunpy.run_path(...)'.Coverage of the added lines is 100% (v8, measured on
core/src/tools/skill/run_skill_script_tool.tswith the two suites above): both guard outcomes, the new enum member, the newdefault: return undefined, and the new declaration text. The lines still uncovered in that file are all pre-existing: the registry-errorcatch, the agent-executor fallback, the PowerShell and cmd wrapper arms (Windows-only tests), and twocontinuebranches ingetSkillResourceFiles.Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
Before:
{stdout: '', stderr: 'Unsupported language: typescript', outputFiles: []}. After:{error: "Script 'scripts/hello.ts' has unsupported extension '.ts'. Skill scripts must be one of: .js, .py, .sh, .ps1, .bat, .cmd.", errorCode: 'UNSUPPORTED_SCRIPT_LANGUAGE'}. Swap the path toscripts/hello.jsand the script still runs and prints.Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.