Fix: build the skill script wrapper from the matched resource key

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   N/A

2. Or, if no issue exists, describe the change:

Problem: run_skill_script builds a wrapper that points at a file it never ships.

RunSkillScriptTool.runAsync() resolves the requested script with a lenient two-step lookup: it strips a leading scripts/ and looks the remainder up in skill.resources.scripts, falling back to the raw path. The skill loaders (loadSkillFromDir, loadSkillFromZipBuffer) key that record by bare relative name ('setup.js', 'sub/util.js'), so a prefix-less request like script_path: 'setup.js' resolves on the first lookup and execution proceeds.

But the file actually handed to the executor is materialized by getSkillResourceFiles() as `scripts/${resourceName}`, while buildWrapperCode() was given the caller's raw scriptPath. For a prefix-less request the emitted wrapper is require('./setup.js'); and the only matching entry in inputFiles is named scripts/setup.js. The wrapper references a path that does not exist in the execution working directory, so the run reaches the sandbox and dies with an opaque module/file-not-found error (Cannot find module './setup.js', No such file or directory, PowerShell ObjectNotFound), wrapped into EXECUTION_ERROR — instead of the tool's own actionable SCRIPT_NOT_FOUND, and at the cost of a full sandbox round trip. All six wrapper languages are affected because every branch interpolates scriptPath relative to ./.

Solution: capture the resource key that actually matched, and derive the wrapper path from it.

const scripts = skill.resources?.scripts;
const relScriptPath = scriptPath.startsWith('scripts/')
  ? scriptPath.substring('scripts/'.length)
  : scriptPath;
// The key that actually matched, so the wrapper path equals the name
// getSkillResourceFiles() materializes the script under.
const resourceName = scripts?.[relScriptPath] ? relScriptPath : scriptPath;
const script = scripts?.[resourceName];

// ...and at the call site:
code: buildWrapperCode(`scripts/${resourceName}`, language),

The two-step lookup collapses into a single key selection, so the source change is +6/-5 with no reassignment.

Invariant this establishes: for any successfully resolved script, the path embedded in the wrapper is exactly equal to the name of one of the Files in codeExecutionInput.inputFiles. That holds by construction — getSkillResourceFiles() emits `scripts/${resourceName}` for every key in resources.scripts, and resourceName here is by construction one of those keys. Deriving from the matched key (rather than re-prefixing scriptPath) is also what makes the pathological scripts/scripts/x.js case correct for free: a resource key that itself begins with scripts/ resolves via the fallback and canonicalizes to scripts/scripts/x.js, matching its File.name.

Consequences: the wrapper becomes a function of the resolved script rather than of the caller's spelling, so 'setup.js' and 'scripts/setup.js' are indistinguishable downstream of the lookup.

Deliberately not changed (each is in flight separately; touching it here would conflict and muddy both reviews):

• buildWrapperCode()'s body is byte-for-byte untouched, including the doubled-separator scriptPath.replace(/\//g, '\\\\') on the PowerShell/cmd branches. New assertions in those areas are therefore written spelling-agnostically (equality between the two input spellings) rather than against a hardcoded backslash literal.
• No error-code enum is introduced.
• getSkillResourceFiles() output, inputFiles, language, args, outputFiles, and every errorCode and error message string are unchanged. SCRIPT_NOT_FOUND and EXECUTION_ERROR still echo the caller's original scriptPath, so the error reflects what was actually asked for.
• Language derivation (path.extname) is unaffected by the prefix, so that line is left alone.

Not a breaking change. Prefixed script_path — the documented form, and the form every pre-existing test uses — produces a byte-identical wrapper before and after. Prefix-less script_path always failed at execution time before, so no caller can depend on the old wrapper string; the only observable change is failure -> success.

adk-python parity: this closes a real cross-language divergence. src/google/adk/tools/skill_toolset.py already normalizes before emitting the wrapper in _build_wrapper_code() (if not file_path.startswith("scripts/"): file_path = f"scripts/{file_path}"), materializes scripts under the same prefix, and repeats the normalization on the local-environment path. adk-js was the only side missing it. Where the two conventions could conflict, parity wins for the observable wire value (the canonical scripts/ prefix) and local JS/TS convention wins for internals (naming, module-private helpers).

Collision check (run before implementing): gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 returned 532 open PRs; the plausibly adjacent ones were diffed. #481 and #356 touch the same file but different hunks — #481 changes only the PowerShell/cmd separator inside buildWrapperCode, #356 only replaces the error-code string literals with an enum. Neither performs any script_path normalization, and no other open PR does. This branch is based on current main; no stacking was needed.

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.

Seven new cases were added to core/test/tools/skills/run_skill_script_tool_test.ts; none of the eight pre-existing cases was modified, renamed, or removed (they remain the regression signal for the prefixed path). The existing MockCodeExecutor / createMockContext() / mockSkill harness is reused; two small separate fixtures were added for the nested and scripts/-prefixed-key shapes so mockSkill (and the inputFiles?.length assertion that depends on its exact size) is left undisturbed.

1. Prefix-less 'setup.js' yields require('./scripts/setup.js');.
2. 'setup.js' and 'scripts/setup.js' yield the same wrapper (spelling-agnostic, so it survives the separate separator fix).
3. The path extracted from the wrapper equals 'scripts/setup.js' and is present in inputFiles.map(f => f.name) — asserting the relationship, not two independent literals.
4. A second language: 'run.sh' and 'scripts/run.sh' both yield source ./scripts/run.sh "$@".
5. A nested key ('sub/util.js'): both spellings yield require('./scripts/sub/util.js');, present in inputFiles.
6. The fallback lookup: a resource key literally named 'scripts/nested.js' canonicalizes to scripts/scripts/nested.js, again asserted against an actual inputFiles name.
7. SCRIPT_NOT_FOUND is still returned for 'nope.js' and 'scripts/nope.js', each echoing the caller's original spelling, with executeCode never called.

npx vitest run --project unit:core core/test/tools/skills/run_skill_script_tool_test.ts
  Tests  15 passed (15)

Proof the tests can fail (mutation testing — coverage alone is not proof):

Mutation A — revert the one-line change, buildWrapperCode(canonicalScriptPath, language) -> buildWrapperCode(scriptPath, language). Cases 1-6 fail:

× builds the canonical wrapper for a script path without the scripts/ prefix
  → expected 'require(\'./setup.js\');' to be 'require(\'./scripts/setup.js\');'
× builds the same wrapper with and without the scripts/ prefix
  → expected 'require(\'./setup.js\');' to be 'require(\'./scripts/setup.js\');'
× references an input file name from the wrapper of a prefix-less script path
  → expected 'setup.js' to be 'scripts/setup.js'
× builds the same shell wrapper with and without the scripts/ prefix
  → expected 'source ./run.sh "$@"' to be 'source ./scripts/run.sh "$@"'
× builds the canonical wrapper for a nested script path in either spelling
  → expected 'require(\'./sub/util.js\');' to be 'require(\'./scripts/sub/util.js\');'
× builds the wrapper from the resource key matched by the fallback lookup
  → expected 'scripts/nested.js' to be 'scripts/scripts/nested.js'
Tests  6 failed | 9 passed (15)

Mutation B — keep the scripts/ prefixing but derive it from the stripped request rather than the matched key (buildWrapperCode(`scripts/${relScriptPath}`, …)), leaving the lookup itself intact. This isolates the "prefix the matched key" half of the fix, and only case 6 fails — exactly what it pins:

× builds the wrapper from the resource key matched by the fallback lookup
  → expected 'scripts/nested.js' to be 'scripts/scripts/nested.js'
Tests  1 failed | 14 passed (15)

Case 7 (SCRIPT_NOT_FOUND) passes both before and after the fix — it is a regression guard on the unchanged error strings and the "do not over-normalize into a false positive" side of the change, not a bug pin, and is not presented as proof.

Coverage: every line and branch added by this change is executed (cases 1-5 take the first-lookup path, case 6 takes the fallback path). Measured with --coverage.provider=v8 restricted to the changed file, the only uncovered lines/branches in run_skill_script_tool.ts are pre-existing and untouched by this PR (the _getDeclaration body, the registry-error catch, the TypeScript/Python/PowerShell/cmd wrapper branches, and the content-shape fallbacks in getSkillResourceFiles).

Manual End-to-End (E2E) Tests:

An automated real-executor test was added to tests/integration/tools/run_skill_script_tool_test.ts (one new case, inserted next to the existing prefixed-JS case; nothing else in that file was changed). It drives RunSkillScriptTool through a real UnsafeLocalCodeExecutor with script_path: 'hello.js' — no mocks, actual node subprocess — and asserts the script's stdout. This is the sanity check described above, automated rather than left as prose.

npx vitest run --project integration tests/integration/tools/run_skill_script_tool_test.ts
  Tests  9 passed | 4 skipped (13)     # 4 skipped = Windows-only PowerShell/cmd cases on Linux

With Mutation A applied it fails exactly as the bug predicts — the script never runs at all:

× successfully executes a real JavaScript skill script requested without the scripts/ prefix
  → expected '' to contain 'hello from skill js'

To reproduce by hand instead: load a skill directory containing scripts/setup.js, invoke run_skill_script with script_path: 'setup.js' against a real code executor, and confirm the script runs rather than failing with Cannot find module './setup.js'.

Other local verification on the pushed commit:

npx eslint <the three changed files>          # clean
npx prettier --check <the three changed files> # clean
npm run lint                                   # clean
npm run format:check                           # clean
npm run build                                  # succeeds
npm run ts:check                               # 281 errors, identical count with and
                                               # without this branch's changes; all
                                               # pre-existing (e.g. the `as File` cast at
                                               # run_skill_script_tool_test.ts:209 on main).
                                               # None are attributable to this diff.

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.