feat(agent-manager): run project scripts in the selected terminal

[marius-kilocode]

Agent Manager Run scripts previously executed only as VS Code Tasks with TaskRevealKind.Always, which forced the bottom terminal panel open even when the Agent Manager terminal was selected.

This change makes the existing terminal destination dropdown the single source of truth for both terminal opening and Run execution:

• Agent Manager panel executes the platform-specific Run script through the CLI backend PTY service and renders it in a named Run tab in the Agent Manager side terminal.
• VS Code terminal preserves the existing VS Code Task behavior in the integrated bottom terminal.

The panel-local dropdown choice travels with every Run request, so no second setting or cross-window preference is needed. The legacy integrated adapter remains isolated in run/task.ts, and the routing decision remains one pickRunStart branch. Once the embedded terminal is the only supported target, the dropdown option, adapter file, and routing branch can be deleted together.

Behavior of the embedded path:

• Run executes the script as an explicit executable plus argv through the canonical client.v2.pty API. Linux and macOS use sh; Windows uses PowerShell or CMD based on the existing script precedence. No shell string construction and no command injection through terminal input.
• The Run tab streams live output, accepts input for interactive scripts, and replays retained output after natural exit and after Agent Manager webview reloads.
• Stop terminates the full process tree: descendant enumeration plus process group and per-process signaling on POSIX, taskkill /f /t on Windows. Worktree deletion, provider shutdown, and tab close route through the same removal path.
• Natural exits reconcile through both pty.exited events and status reads, so fast scripts and SSE reconnects cannot strand Run/Stop status.
• Script execution is blocked in untrusted workspaces, matching VS Code Tasks.

PTY hardening in packages/core, kept behind narrow kilocode_change markers:

• Explicit command plus args no longer receives an implicit login-shell -l argument.
• Canonical /api/pty attachments can replay retained output after exit. Legacy /pty behavior is unchanged.
• PTY removal waits for process-tree termination and is uninterruptible.

Setup scripts remain VS Code Tasks and are intentionally unchanged here. Their migration is tracked in #12649.\n\nFollow-up to the original Run-script feature in #7526.

Screenshots

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This incremental commit (a282a149e6) is a pure mechanical refactor: it extracts the script-terminal manager wiring and Run controller construction out of AgentManagerProvider.ts into a new script-terminal-runtime.ts file, keeping the provider file below the repo's size cap. No behavior changed — event subscription/unsubscription ordering, dispose sequencing, and the pickRunStart routing logic are all preserved as-is, just relocated. All call sites (this.scripts.manager.*, this.scripts.dispose()) were updated consistently, and agent-manager-arch.test.ts was updated to assert against the new file. No new issues found in the changed code.

Files Reviewed (incremental — 3 files)

• packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts - delegates script-terminal/run wiring to the new runtime factory, no behavior change
• packages/kilo-vscode/src/agent-manager/script-terminal-runtime.ts - new file, extracted createScriptTerminalRuntime and createRunController factories
• packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts - updated assertions to match the extracted file and new this.scripts.manager.* call sites

Note: Prior WARNING findings on files not touched in this increment (ScriptTerminalManager.ts, packages/core/src/pty.ts, packages/core/src/kilocode/pty/termination.ts) are unchanged by this commit and were already addressed per the previous review; see existing inline threads.

Previous Review Summaries (4 snapshots, latest commit a6f6655)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit a6f6655)

Status: No Issues Found | Recommendation: Merge

Overview

This incremental commit addresses all 7 WARNING findings from the previous review (double stop/dispose race in RunScriptManager, missing "already gone" recovery in ScriptTerminalManager.removeEntry, statsPoller.skipWorktree ordering race, unfiltered onEvent PTY subscription, notifyEnd clobbering the exit code on teardown, unconditional allowExited: true on /api/pty connect, and no timeout on the ps/taskkill child process spawns) plus the setScripts no-op-guard SUGGESTION. Each fix is backed by a new or updated unit test that exercises the real implementation (race between start/remove, 404-on-remove recovery, replayExited query param, exit-code preservation on teardown). No new issues found in the changed code.

Files Reviewed (incremental — 14 files)

• packages/core/src/kilocode/pty/termination.ts - added spawn timeout, resolved prior WARNING
• packages/core/src/pty.ts - preserved exit code in notifyEnd, resolved prior WARNING
• packages/core/test/kilocode/pty-termination.test.ts - updated for timeout option
• packages/core/test/pty/pty-session.test.ts - added coverage for exit-code preservation
• packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts - filtered PTY event subscription and reordered skipWorktree, resolved 2 prior WARNINGs
• packages/kilo-vscode/src/agent-manager/GitStatsPoller.ts - added unskipWorktree rollback
• packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts - added "already gone" recovery path, resolved prior WARNING
• packages/kilo-vscode/src/agent-manager/run/manager.ts - guarded stop/dispose against races, resolved prior WARNING
• packages/kilo-vscode/src/agent-manager/script-terminal-url.ts - added replayExited query param
• packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts - added ordering assertion
• packages/kilo-vscode/tests/unit/run-script-manager.test.ts - added race test
• packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts - added recovery + replayExited tests
• packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts - added no-op guard to setScripts, resolved prior SUGGESTION
• packages/server/src/groups/pty.ts / packages/server/src/handlers/pty.ts - gated allowExited behind explicit query param, resolved prior WARNING

Previous review (commit 3a829f5)

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	7
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/kilo-vscode/src/agent-manager/run/manager.ts	68	stop()/dispose() can run twice on the same handle if remove() races with an in-flight start()
packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts	305	removeEntry has no "already gone" recovery path, unlike reconcile() — can leave a worktree permanently stuck
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	1064	Awaiting run/script teardown before statsPoller.skipWorktree reopens the race the adjacent comment documents
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	133	Unfiltered onEvent subscription delivers PTY events for every Agent Manager panel, not just this one
packages/core/src/pty.ts	333	teardown()'s notifyEnd(session, {}) can clobber the exit code stored for an allowExited replay subscriber
packages/server/src/handlers/pty.ts	181	allowExited: true applied unconditionally to all /api/pty connect clients, not just Agent Manager's new terminals
packages/core/src/kilocode/pty/termination.ts	87	No timeout on the ps/taskkill child process spawns — a hang can block uninterruptible PTY removal indefinitely

SUGGESTION

File	Line	Issue
packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts	380	setScripts skips the no-op-guard pattern used by every other setter in this file, causing avoidable re-renders

Files Reviewed (incremental — 1 file)

• .kilo/plans/agent-manager-script-terminals.md - removed (implementation plan doc, no code change)

This incremental commit only deletes the implementation plan doc .kilo/plans/agent-manager-script-terminals.md; no code changed. All 8 previously reported findings remain on unchanged code and are still current (verified against live PR comment line numbers). The prior SUGGESTION about a brittle comment-wording assertion in agent-manager-arch.test.ts is no longer anchored to a resolvable diff line and has been dropped from this summary.

Fix these issues in Kilo Cloud

Previous review (commit 4016919)

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	7
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/kilo-vscode/src/agent-manager/run/manager.ts	68	stop()/dispose() can run twice on the same handle if remove() races with an in-flight start()
packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts	305	removeEntry has no "already gone" recovery path, unlike reconcile() — can leave a worktree permanently stuck
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	1064	Awaiting run/script teardown before statsPoller.skipWorktree reopens the race the adjacent comment documents
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	133	Unfiltered onEvent subscription delivers PTY events for every Agent Manager panel, not just this one
packages/core/src/pty.ts	333	teardown()'s notifyEnd(session, {}) can clobber the exit code stored for an allowExited replay subscriber
packages/server/src/handlers/pty.ts	181	allowExited: true applied unconditionally to all /api/pty connect clients, not just Agent Manager's new terminals
packages/core/src/kilocode/pty/termination.ts	87	No timeout on the ps/taskkill child process spawns — a hang can block uninterruptible PTY removal indefinitely

SUGGESTION

File	Line	Issue
packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts	380	setScripts skips the no-op-guard pattern used by every other setter in this file, causing avoidable re-renders

Files Reviewed (37 files)

• .changeset/calm-run-terminals.md
• .kilo/plans/agent-manager-script-terminals.md
• packages/core/src/kilocode/pty/termination.ts - 1 issue
• packages/core/src/pty.ts - 1 issue
• packages/core/test/kilocode/pty-termination.test.ts
• packages/core/test/pty/pty-session.test.ts
• packages/kilo-docs/pages/automate/agent-manager.md
• packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts - 2 issues
• packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts - 1 issue
• packages/kilo-vscode/src/agent-manager/__tests__/AgentManagerProvider.spec.ts
• packages/kilo-vscode/src/agent-manager/host.ts
• packages/kilo-vscode/src/agent-manager/run/controller.ts
• packages/kilo-vscode/src/agent-manager/run/destination.ts
• packages/kilo-vscode/src/agent-manager/run/manager.ts - 1 issue
• packages/kilo-vscode/src/agent-manager/run/message.ts
• packages/kilo-vscode/src/agent-manager/run/task.ts
• packages/kilo-vscode/src/agent-manager/script-terminal-url.ts
• packages/kilo-vscode/src/agent-manager/types.ts
• packages/kilo-vscode/src/agent-manager/vscode-host.ts
• packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts
• packages/kilo-vscode/tests/unit/agent-manager-terminal-chrome.test.ts
• packages/kilo-vscode/tests/unit/agent-manager-terminal-side.test.ts
• packages/kilo-vscode/tests/unit/agent-manager-terminal-state.test.ts
• packages/kilo-vscode/tests/unit/run-message.test.ts
• packages/kilo-vscode/tests/unit/run-script-manager.test.ts
• packages/kilo-vscode/tests/unit/run-terminal-destination.test.ts
• packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts
• packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx
• packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css
• packages/kilo-vscode/webview-ui/agent-manager/terminal/SideTerminalPanel.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/SortableTerminalTab.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/chrome.ts
• packages/kilo-vscode/webview-ui/agent-manager/terminal/index.ts
• packages/kilo-vscode/webview-ui/agent-manager/terminal/render.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/side.ts
• packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts - 1 issue
• packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts
• packages/kilo-vscode/webview-ui/src/types/messages/webview-messages.ts
• packages/server/src/handlers/pty.ts - 1 issue

The new commit (4016919238) moves the Run destination from a global VS Code setting to the panel-local terminal dropdown: RunController.run now takes an explicit RunTerminalDestination argument threaded from the webview's agentManager.runScript message through RunTaskConfig.destination, and pickRunStart was simplified to a plain generic picker. The wiring is consistent end-to-end (types, provider, webview dispatch, and new/updated tests for run/message.ts, agent-manager-terminal-side.test.ts, and run-terminal-destination.test.ts), and the previously-flagged readRunTerminalDestination/setting-based path was fully removed along with the dead kilo-code.new.agentManager.runTerminalDestination config entry and its changeset wording. No new bugs, injection risks, or fork-hygiene issues found in this incremental diff. The agent-manager-arch.test.ts brittle-comment-wording suggestion from the previous review no longer maps to a current diff line and has been dropped from this summary; all other previously reported findings are on files untouched by this commit and remain outstanding.

Fix these issues in Kilo Cloud

Previous review (commit 22d1a50)

Status: 9 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	7
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/kilo-vscode/src/agent-manager/run/manager.ts	68	stop()/dispose() can run twice on the same handle if remove() races with an in-flight start()
packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts	305	removeEntry has no "already gone" recovery path, unlike reconcile() — can leave a worktree permanently stuck
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	1064	Awaiting run/script teardown before statsPoller.skipWorktree reopens the race the adjacent comment documents
packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts	133	Unfiltered onEvent subscription delivers PTY events for every Agent Manager panel, not just this one
packages/core/src/pty.ts	333	teardown()'s notifyEnd(session, {}) can clobber the exit code stored for an allowExited replay subscriber
packages/server/src/handlers/pty.ts	181	allowExited: true applied unconditionally to all /api/pty connect clients, not just Agent Manager's new terminals
packages/core/src/kilocode/pty/termination.ts	87	No timeout on the ps/taskkill child process spawns — a hang can block uninterruptible PTY removal indefinitely

SUGGESTION

File	Line	Issue
packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts	380	setScripts skips the no-op-guard pattern used by every other setter in this file, causing avoidable re-renders
packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts	484	Brittle assertion on the literal wording of a source comment

Files Reviewed (32 files)

• .changeset/calm-run-terminals.md
• .kilo/plans/agent-manager-script-terminals.md
• packages/core/src/kilocode/pty/termination.ts - 1 issue
• packages/core/src/pty.ts - 1 issue
• packages/core/test/kilocode/pty-termination.test.ts
• packages/core/test/pty/pty-session.test.ts
• packages/kilo-docs/pages/automate/agent-manager.md
• packages/kilo-vscode/package.json
• packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts - 2 issues
• packages/kilo-vscode/src/agent-manager/ScriptTerminalManager.ts - 1 issue
• packages/kilo-vscode/src/agent-manager/__tests__/AgentManagerProvider.spec.ts
• packages/kilo-vscode/src/agent-manager/host.ts
• packages/kilo-vscode/src/agent-manager/run/controller.ts
• packages/kilo-vscode/src/agent-manager/run/destination.ts
• packages/kilo-vscode/src/agent-manager/run/manager.ts - 1 issue
• packages/kilo-vscode/src/agent-manager/run/task.ts
• packages/kilo-vscode/src/agent-manager/script-terminal-url.ts
• packages/kilo-vscode/src/agent-manager/terminal-destination.ts
• packages/kilo-vscode/src/agent-manager/types.ts
• packages/kilo-vscode/src/agent-manager/vscode-host.ts
• packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts - 1 issue
• packages/kilo-vscode/tests/unit/agent-manager-terminal-chrome.test.ts
• packages/kilo-vscode/tests/unit/agent-manager-terminal-state.test.ts
• packages/kilo-vscode/tests/unit/run-script-manager.test.ts
• packages/kilo-vscode/tests/unit/run-terminal-destination.test.ts
• packages/kilo-vscode/tests/unit/script-terminal-manager.test.ts
• packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx
• packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css
• packages/kilo-vscode/webview-ui/agent-manager/terminal/SideTerminalPanel.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/SortableTerminalTab.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/chrome.ts
• packages/kilo-vscode/webview-ui/agent-manager/terminal/render.tsx
• packages/kilo-vscode/webview-ui/agent-manager/terminal/state.ts - 1 issue
• packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts
• packages/server/src/handlers/pty.ts - 1 issue

Command injection claims in the PR description hold up: client.v2.pty.create/vscode.ProcessExecution both pass command/args as separate values, no shell string construction found in the reviewed embedded or legacy paths. Test coverage for the new ScriptTerminalManager/RunScriptManager lifecycle is solid and exercises real implementations without excessive mocking. The findings above focus on process/PTY lifecycle races and error-handling gaps introduced by this change.

Fix these issues in Kilo Cloud

---

_{Reviewed by claude-sonnet-5 · Input: 24 · Output: 6.8K · Cached: 510.7K}

_{Review guidance: REVIEW.md from base branch main}

[marius-kilocode]

@kilocode-bot can you do a quick security review of this? Is there any difference security wise that represent major blockers?

[kilo-code-bot]

Security review (focused)

Verdict: no blockers. The core execution path is sound — Run still resolves an allowlisted .kilo/run-script* file (run/service.ts ALLOWED + validated()), and the embedded path passes an explicit command + args[] array to client.v2.pty.create with no shell string interpolation, so there is no new command-injection vector through terminal input, script names, env values, or worktree/branch names. Terminal keystrokes go through PtyProtocol.decodeInput into the PTY fd, never into a shell command line. Two items are worth addressing (both warning), the rest are hardening/informational.

Findings

1. warning — NEW RISK: Windows cmd argv is pre-quoted for VS Code ProcessExecution, now fed to node-pty.
   buildRunTaskCommand() returns ["/d","/s","/c", "\"C:\\...\\run-script.cmd\""] (run/service.ts). VS Code tasks and child_process keep that quoting intact; node-pty on Windows re-serialises args[] into a command line with its own escaping (packages/core/src/pty/pty.node.ts:7 → argvToCommandLine), so the embedded path's effective command line is not the one this quoting was designed for. With a workspace path containing "-adjacent characters (&, ^, % are legal in Windows paths) the resulting cmd.exe line is attacker-influenced. Please verify on Windows with a repo path containing a space, &, and %VAR%, and prefer passing the unquoted path (letting node-pty quote).

2. warning — NEW RISK: PID-reuse window in tree termination.
   packages/core/src/kilocode/pty/termination.ts:165-176 enumerates PIDs via ps, sleeps GRACE_MS, then signals raw PIDs and PGIDs (:80-82 sends to both -pid and pid). The root PID comes from the stale proc handle rather than from a live check, and up to ~400 ms elapses between snapshot and SIGKILL. If the target exits in that window and the OS recycles the PID (realistic in containers with a low pid_max), we SIGKILL an unrelated same-uid process — and via -pid, its whole process group. Previous behaviour (session.process.kill() in the old pty.ts teardown) only ever signalled the PTY handle, so this is genuinely new blast radius. Suggest re-verifying the root is still the expected process (e.g. re-reading its ppid/start time) before the SIGKILL pass.

3. informational — NEW RISK (cheap hardening): no pid > 1 guard before signalling.
   termination.ts:74-87 signals whatever descendants() produced. kill(-1, …) targets every process the user can signal and kill(0, …) the caller's own group; :112 only checks Number.isSafeInteger, so 0 and 1 pass the row filter. Not reachable from normal ps output, but a pid > 1 && parent >= 0 filter is a one-liner given the blast radius.

4. informational — PRE-EXISTING/EQUIVALENT pattern: ps / taskkill resolved by name, not absolute path.
   termination.ts:95 and :155 spawn "ps" / "taskkill" without a path. On Windows, libuv's path search includes the CWD, and the backend's CWD is the workspace root (services/cli-backend/server-manager.ts:114), so a malicious repo shipping taskkill.exe could be executed on Stop. Mitigating context: this is the established pattern across the repo (packages/core/src/shell.ts:38, kilocode/background-process/index.ts:751, util/process.ts:157), so the PR does not introduce the pattern — but new code could use %SystemRoot%\System32\taskkill.exe and /bin/ps. No shell is used and no argument is attacker-controlled (String(proc.pid) only), so there is no injection into these commands.

5. informational — NEW RISK, correctly implemented: untrusted-workspace gate.
   Previously trust enforcement was implicit — VS Code refuses tasks.executeTask in Restricted Mode. The embedded PTY path bypasses VS Code entirely (the CLI backend has no trust concept), so the explicit check at script-terminal-runtime.ts:69 is now the only gate. It is correctly placed: it runs inside the RunController.start adapter before either branch of pickRunStart, reads vscode.workspace.isTrusted live (vscode-host.ts isTrusted()) rather than caching it, and ScriptTerminalManager.start has no other caller (intercept() only handles close/resize). No bypass found. Nit: getShellEnvironment() (login shell + env) runs in run/controller.ts before the trust check — it's user-profile-only and workspace-independent, so not exploitable, but moving the check earlier would be tidier.

6. informational — PRE-EXISTING/EQUIVALENT: /api/pty replayExited=1 access control.
   packages/server/src/handlers/pty.ts:186 + packages/core/src/pty.ts:324,330. Replay is gated by the same auth as any connect (loopback bind + rotating basic/auth_token credential, or a single-use ptyID-scoped ticket) and by knowing the PtyID; there is no per-client ownership model, but that is pre-existing for live attaches, and any holder of the credential can already create arbitrary PTYs (RCE-equivalent). No cross-session leakage: attach only ever replays that session's buffer, subscriber.end is seeded from the same session's exit code, and the retained buffer is dropped on remove (drop() in ScriptTerminalManager, plus worktree delete/remove now force clear("run", …)). Legacy /pty keeps the ExitedError. The one widened window: exited-session output stays readable until explicit removal — acceptable, but Run output now survives longer than before.

7. informational — NEW RISK, bounded: uninterruptible removal.
   packages/core/src/pty.ts:182-192 wraps teardown in Effect.uninterruptible and teardown awaits terminate(). The bound is real (GRACE_MS×2 + up to 2×SPAWN_TIMEOUT_MS for ps ≈ 10.4 s worst case POSIX; ~5 s Windows since taskkill uses the spawn timeout), and every spawn resolves on error/close, so no unbounded hang and no DoS via a hanging script — but a wedged ps can stall removal/interpreter shutdown ~10 s per session with no way to cancel. An explicit Effect.timeout around terminate() in teardown would make the ceiling declarative.
   Related: terminate() does not verify the tree actually died, so pty.remove returns success even when SIGKILL fails (root-owned child via sudo, D-state). A Run script can therefore survive "Stop"/worktree deletion while AgentManagerProvider.deleteWorktree trusts the success result — same class of gap as the old task-based stop, so equivalent rather than a regression.

8. informational — no new IPC/authorization surface. No new server endpoints (only the replayExited query param on the existing canonical connect route). The new agentManager.scriptTerminals / agentManager.runScript{destination} messages travel over the existing Agent Manager webview channel — other extensions cannot post to it, and destination is untrusted-but-harmless (both branches execute the same allowlisted resolved script; pickRunStart defaults to embedded). resize cols/rows are unvalidated in intercept() but the server schema enforces PositiveInt (groups/pty.ts:71-72). The webview receives the loopback credential inside wsUrl (script-terminal-url.ts:9-16) — identical to the pre-existing terminal-routing.ts:227 builder, and the new one is strictly better hygiene (URLSearchParams encoding vs manual concat). Run PTYs also inherit the KILO_SERVER_PASSWORD/USERNAME tombstones from pty.ts, so the script itself does not see backend credentials. Env is passed as a map (never logged in pty.create), not interpolated.

9. informational — positive: the Shell.login change reduces surface. packages/core/src/pty.ts:217 now appends -l only for the implicit shell, so an explicit sh <script> no longer runs as a login shell — less profile code executed for script runs.

Path traversal / executable selection

No issues. resolveScript() iterates a fixed candidate list, re-checks membership in ALLOWED, joins against <root>/.kilo, and validated() rejects non-regular files and symlinks escaping .kilo (realpath().startsWith(dir + sep)). cwd is validated absolute + isDirectory() in run/controller.ts, and ptyID is encodeURIComponent-ed in the WS URL. The sh / PowerShell (-NoProfile -ExecutionPolicy Bypass -File) / CMD selection is unchanged from the task path — -ExecutionPolicy Bypass is pre-existing and inherent to running a user-authored .ps1.

[kilo-code-bot]

Security review already posted here: #12680 (comment)

Verdict: no blockers. Core execution path is sound — allowlisted .kilo/run-script* resolution unchanged, and the embedded path uses explicit command+args[] into client.v2.pty.create with no shell-string construction, so no new injection vector via terminal input, script names, env, or worktree/branch names.

Two warning-level (non-blocking) new risks worth a follow-up:

1. Windows CMD quoting mismatch — buildRunTaskCommand() pre-quotes the path for VS Code's ProcessExecution, but node-pty re-serializes args[] with its own escaping, so the effective cmd.exe line is unverified when workspace paths contain &, ^, %.
2. PID-reuse window in termination.ts:165-176 — root PID is read from a stale handle with a ~400ms gap before SIGKILL, and the signal now hits the whole process group (-pid) instead of just the PTY handle as before — larger blast radius than prior behavior.

Plus minor hardening notes (no pid > 1 guard, name-based ps/taskkill resolution, uninterruptible removal up to ~10s, no post-terminate() death verification) — none blocking.

Nothing further needed unless you want me to spawn a follow-up agent to fix the two warnings.