POK-47: Support Windows autopilot host bash

[HYT-koota]

Summary

Implements the POK-47 Windows opt-in behavior for bash execution.

• On native Windows, default/sandboxed bash now requires [runtime] autopilot = true; without that opt-in the tool returns a clear recoverable error explaining the host full-access trust model.
• When native Windows autopilot is enabled, bash runs directly on the Windows host via full-access execution instead of trying sandbox-runtime.
• Startup now fails clearly on native Windows without autopilot, and skips sandbox-runtime initialization when native Windows autopilot is enabled because sandbox-runtime does not support Windows.
• Doctor/configuration docs now call out the required opt-in and the lack of Linux sandbox isolation.

Validation

Passed:

• pnpm build
• pnpm exec biome check src/runtime/host-platform.ts src/main.ts src/tools/bash.ts tests/tools/bash.test.ts tests/agent/bash-approval.test.ts
• pnpm test -- tests/tools/bash.test.ts tests/agent/bash-approval.test.ts tests/agent/yolo-autopilot.test.ts
• bash scripts/doctor.sh

Attempted:

• pnpm preflight

pnpm preflight still fails on this Windows machine because of existing repo-wide Windows issues unrelated to this change: POSIX path expectations, symlink permission failures, Windows file-mode differences, and sandbox/security tests that assume POSIX/Linux behavior. The command also triggered full-repo Biome formatting churn on Windows; unrelated tracked churn was reverted before committing.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 381a62c1e3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Kill Windows host bash process trees on timeout

When native Windows autopilot runs a normal bash command that times out while a foreground child is running, this new branch sends it through executeUnsandboxedBash; that implementation's Windows timeout path can only call child.kill() on the bash wrapper, unlike the POSIX process-group kill in src/security/sandbox.ts, so the child process can continue running with full host access after Pokoclaw reports a timeout. Use a Windows process-tree termination strategy/job object for this host-exec path before enabling it by default.

Useful? React with 👍 / 👎.

[HYT-koota]

@claude The Windows host-bash timeout cleanup fix is now on this PR branch, including process-tree termination via taskkill /t /f. I re-ran pnpm test -- tests/security/sandbox.windows-timeout.test.ts tests/tools/bash.test.ts tests/agent/bash-approval.test.ts and pnpm build; both passed. Please review again.

[HYT-koota]

@claude Following up on the Windows host-bash timeout cleanup fix now on this PR branch. The targeted tests and pnpm build are still passing. Please take another look when you can.

[HYT-koota]

@claude This PR is now showing as conflicted with the latest upstream main. Please resolve the branch conflict and then review the Windows autopilot host-bash behavior again, especially the timeout/process-tree cleanup path.

[danielwpz]

@claude Please review this Windows autopilot host-bash PR again. This is a maintainer-triggered retry after the Claude workflow routing change; treat this comment as the review request. First check whether the branch conflict is still the blocking issue. Then review the Windows host-bash timeout and process-tree cleanup behavior, especially the taskkill /t /f path, and call out any concrete code or test changes needed on this branch.

[claude]

Review: POK-47 Windows autopilot host bash (PR #52)

1. BLOCKING — Branch conflict with main. The PR is mergeable: CONFLICTING with mergeStateStatus: DIRTY. The branch codex/pok-47-windows-autopilot-bash must be rebased or merged with main to resolve the conflict before this can land. The three commits on this branch touch src/main.ts, src/security/sandbox.ts, src/tools/bash.ts, and docs/configuration.md — any of those may have diverged upstream.

2. The aborting flag in runUnsandboxedShellCommand is correctly designed for the async kill path. Before this PR, killChild was synchronous (process.kill / child.kill), so the onAbort handler could call killChild() then immediately finish(reject) in the same tick without the child's close event racing in. Now that killChild is async (it awaits terminateWindowsProcessTree which spawns taskkill and waits for it), the close event can fire during that await. Setting aborting = true synchronously before the await and guarding both the close and error handlers with if (aborting) return prevents the promise from resolving with a normal result when it should reject with an AbortError. This is exactly right.

3. terminateWindowsProcessTree implementation is correct. taskkill /pid <pid> /t /f with windowsHide: true and stdio: "ignore" is the standard Windows process-tree kill. It returns a boolean (exitCode === 0) so the caller can fall back to child.kill("SIGKILL") if taskkill fails (process already gone, permissions, etc.). The settled guard on the internal finish prevents the promise from hanging if taskkill errors after already emitting close. No resource leaks.

4. Test timeout of 5ms may be fragile on slow CI. In tests/security/sandbox.windows-timeout.test.ts, the test relies on timeoutMs: 5 to trigger the abort path before the fake child emits close. The fake bash child never emits close on its own, so the 5ms timer is the only trigger — this works because the timeout fires as a macrotask while the child sits idle. On an extremely slow or overloaded CI runner, 5ms should still be fine since no real I/O is involved, but it is worth watching for flakes. A more robust alternative would be to expose the AbortController so the test can trigger it directly, but that would require refactoring executeUnsandboxedBash's internals and is not worth holding this PR.

5. resolveEffectiveBashExecution correctly gates on source === "autopilot", not "yolo". The test "does not treat YOLO as native Windows bash host-execution opt-in" validates this. The routing logic in bash.ts means that on native Windows without autopilot, the tool returns a clear recoverable error rather than silently falling back to sandboxed execution (which would fail because sandbox-runtime doesn't support Windows). The error messages are consistent across host-platform.ts, bash.ts, doctor.sh, and configuration.md.

Summary: The core logic is sound. The async killChild + aborting flag pattern is the necessary fix for correct abort semantics now that taskkill is async. The only merge-blocking item is the branch conflict. After resolving that, this is ready to merge.

[HYT-koota]

@claude Please retry now that I have collaborator access. This PR is showing as conflicted with the latest upstream main; please resolve or advise on the conflict, then review the Windows autopilot host-bash behavior, especially the timeout/process-tree cleanup path.

[claude]

PR Review: POK-47 Windows autopilot host bash

1. Merge conflict (merge-blocking)

The PR is marked as CONFLICTING with main (mergeStateStatus: DIRTY). The conflict needs to be resolved before this can land. The most recent commit on main (f8236de Consolidate Claude mentions into review workflow) likely touched CI/config files rather than source code, so the conflict is probably minor — a rebase should surface it clearly. Run git fetch origin main && git rebase origin/main from the PR branch to identify the exact conflict.

2. terminateWindowsProcessTree has no internal timeout (medium)

sandbox.ts:780–810: If taskkill.exe hangs (unlikely but possible), killChild awaits terminateWindowsProcessTree indefinitely, and the abort promise never settles. In practice taskkill /f calls TerminateProcess which is a kernel call that shouldn't block, so this is low-risk. Consider adding a defensive 5-second timeout inside terminateWindowsProcessTree that force-kills the taskkill child and resolves false, so the fallback child.kill("SIGKILL") path in killChild gets a chance to run.

3. Race between abort and close events (low, pre-existing)

sandbox.ts:731–733, 763–776: The aborting flag correctly prevents close/error from racing with killChild after aborting is set. However, if the child process exits naturally between when the timeout fires and when onAbort runs, the close handler can resolve the promise before aborting is set to true. The abort error is then silently swallowed and a normal result is returned (instead of a timeout error). This race exists in the current code too, but the async killChild widens the window slightly on Windows. The caller in executeUnsandboxedBash already handles this by only throwing bash_timeout in the catch block, so the net effect is that a just-in-time completion is returned as success — which is arguably the right behavior anyway.

4. Everything else looks correct

• Startup guard: assertNativeWindowsRuntimeSupported throws a clear error for Windows without autopilot. Correctly gated.
• Sandbox skip: shouldInitializeSandboxRuntime correctly returns false on Windows+autopilot since sandbox-runtime doesn't support Windows.
• Bash routing: resolveEffectiveBashExecution properly distinguishes Windows autopilot from YOLO and routes to unsandboxed execution only for autopilot. Explicit sandboxMode: "full_access" still works on Windows without autopilot, which is correct since full_access already means explicit user approval for host execution.
• Process tree cleanup: taskkill /pid X /t /f is the right approach for Windows. The fallback to child.kill("SIGKILL") handles the case where taskkill fails. The aborting flag and async killChild are correctly synchronized.
• platform threading: The platform parameter is threaded from ExecuteUnsandboxedBashInput → executeUnsandboxedBash → runUnsandboxedShellCommand with process.platform fallback. The detached option and kill-path both use input.platform instead of process.platform. Tests that simulate Windows behavior on Linux work correctly because the mock intercepts spawn.
• Tests: Good coverage — Windows autopilot execution, Windows rejection without autopilot, YOLO exclusion, and Windows timeout/process-tree cleanup. Platform mocking is properly set up and torn down.

No other merge-relevant issues found.

[HYT-koota]

@claude Added a defensive timeout around the Windows taskkill /t /f cleanup path so a hung taskkill now falls back to child.kill("SIGKILL"), and covered it with a regression in tests/security/sandbox.windows-timeout.test.ts. Re-ran pnpm test -- tests/security/sandbox.windows-timeout.test.ts and pnpm build; both passed. Please review again, and re-check whether the remaining blocker is only the branch conflict with main.

[claude]

Review: POK-47 Windows autopilot host bash (updated)

1. BLOCKING — Branch conflict with main. The PR is still mergeable: CONFLICTING. This remains the only merge-blocking issue. The branch codex/pok-47-windows-autopilot-bash must be rebased or merged with main before this can land. The conflict is likely minor since the most recent main commits touched CI/config files rather than the source files modified here.

2. terminateWindowsProcessTree timeout correctly addresses prior feedback. The TASKKILL_TIMEOUT_MS = 5_000 defensive timeout with settled guard, clearTimeout, and removeAllListeners cleanup is correctly implemented (sandbox.ts:783–823). When the timeout fires, it kills the hung taskkill child and resolves false, allowing killChild to fall through to child.kill("SIGKILL"). No resource leaks — the settled boolean prevents double-resolution, and listeners are cleaned up in both the timeout and normal paths.

3. New regression test "falls back to killing bash when taskkill hangs" correctly validates the timeout path. The test overrides the spawn mock so taskkill never emits close, then asserts that the taskkill child receives kill("SIGKILL") (from the 5s timeout) and the bash child also receives kill("SIGKILL") (from the fallback path in killChild). The 8-second test timeout provides adequate headroom for the 5-second internal timeout plus mock overhead. The test correctly uses vi.useRealTimers() so the real setTimeout fires.

4. All previous review findings remain valid. The aborting flag synchronization with async killChild, the resolveEffectiveBashExecution autopilot-vs-YOLO gating, the startup guard in assertNativeWindowsRuntimeSupported, the sandbox-runtime skip logic, and the error message consistency across host-platform.ts, bash.ts, doctor.sh, and configuration.md are all unchanged and correct.

Summary: The defensive taskkill timeout is correctly implemented and well-tested. There are no new merge-relevant issues. The only remaining blocker is the branch conflict with main. Once that's resolved, this is ready to merge.