feat(windows): add native ACP process lifecycle support

[smallgun01]

Summary

This draft adds the Windows-native ACP process lifecycle implementation to OpenAB.

It is intended to make the ACP host reliable on Windows when an agent session is cancelled, times out, exits unexpectedly, or the parent process shuts down.

Review Contract

Goal

Add the Windows-native ACP lifecycle path to OpenAB with explicit process-tree ownership, credential isolation, shutdown coordination, and CI coverage, so upstream maintainers can review the implementation before any merge or release decision.

Non-goals

• This PR does not publish a Windows installer or declare Native GA.
• This PR does not add provider/model functionality or validate a real provider turn.
• This PR does not change upstream main unless an upstream maintainer later merges it.
• This PR does not claim that gateway cancellation stops downstream model computation or billing.

Accepted Residual Risks

• The process-tree termination timeout remains a fixed value and may be made configurable later.
• The Windows implementation currently targets Windows PowerShell 5.1 and depends on the pinned process-wrap Windows support.
• Canary output and operational logging can be made more structured in a follow-up.
• Release packaging, installer UX, and the final Windows support matrix remain outside this spike.

Acceptance Criteria

• Windows ACP child and parent binaries build successfully.
• Windows process-tree cancellation and shutdown clean up descendants through the controller-owned guard path.
• Windows credential refresh uses current-user ACL protection, cross-process locking, fail-closed timeout behavior, and atomic replacement handling.
• Windows shell execution uses an explicit environment baseline and encoded PowerShell commands.
• The bash tool working_dir now rejects absolute paths outside the sandbox on all platforms (Unix included), not just Windows.
• Shutdown rejects new sessions once shutdown has started.
• Windows CI and the dual-layer canary validate the implementation without provider credentials.
• Fork internal review has no remaining blocking Must Fix items.

Follow-ups

• Consider making the process-tree termination timeout configurable.
• Re-evaluate the exact process-wrap version pin and its long-term Windows support.
• Consider structured canary output and additional operational logging.
• Re-run post-merge Windows canary and release-artifact validation after upstream acceptance.

Discussion

Official group discussion thread:

https://discord.com/channels/1491295327620169908/1538116818462314606

Validation

• Fork head: smallgun01:spike/windows-acp-native-agent
• Head commit: 2d805b9
• Windows CI and dual-layer canary: passed on the fork branch.
• Rust, agent, review-contract, schema, and conformance checks: passed on the fork branch.
• Fork internal review: Approve; no remaining blocking Must Fix items.
• Official OpenAB bot source review: no new blocker identified.

Scope and status

This is intentionally a Draft PR for upstream maintainer review. It does not request merge or release approval yet.

The fork-only governance exception on PR #1 is separate from this upstream PR: the fork does not hold the upstream GitHub App credentials, so that fork-only check is not treated as a code failure.

This PR does not by itself publish a Windows installer or declare Native GA. After upstream review and any requested changes, a merge would still require post-merge Windows canary and release-artifact validation.

[chaodu-obk]

Important

CHANGES REQUESTED ⚠️ -- All six round-1 blockers (F1-F6) are verifiably fixed with regression tests; three round-1 important items (F7-F9) remain open, and the new bounded pipe-join can silently discard captured output (F10).

What This PR Does

Adds a Windows-native ACP process lifecycle to OpenAB: Job Object process-tree ownership, ACL-protected atomic auth.json replacement with cross-process locking, PowerShell -EncodedCommand shell execution with supervisor-owned cancellation, plus Windows CI and a no-provider dual-layer canary. This round (round-2) reviews head 39cb67c8, which adds commits da32db3e and 39cb67c8 responding to round-1.

How It Works (delta since round-1)

• process_tree.rs: the terminate controller now loops -- the first request kills, later concurrent callers receive Ok(()) instead of "controller exited before replying" (closes F4)
• auth.rs: the temp file is created atomically with an explicit current-user DACL via CreateFileW + SECURITY_ATTRIBUTES (closes F5); the trustee is the process-token SID from GetTokenInformation(TokenUser) + BuildTrusteeWithSidW, not USERNAME/USERDOMAIN (closes F3); restrict_auth_dacl() re-applies the restricted DACL after both the ReplaceFileW and MoveFileExW commit paths (closes F2)
• tools.rs: post-kill and post-exit pipe joins are bounded (3s / 6s) with reader-task aborts, and the timeout branch wraps cancel_and_wait in a 5s deadline (closes F1), with two new setsid-escape regression tests
• Workflow: adds main push and pull_request triggers with path filters; MANIFEST.json provenance now derives from github.* context (closes F6)

Findings

#	Severity	Finding	Location
F7	🟡	(carried from round-1) Cross-platform behavior change still unflagged: the bash tool working_dir now rejects absolute paths outside the sandbox on ALL platforms, but neither the PR body nor a changelog entry calls this out	openab-agent/src/tools.rs:540
F8	🟡	(carried from round-1) Windows env baseline allow-lists remain duplicated and diverging between the ACP spawn path and the shell tool (e.g. USERPROFILE, USERNAME, LANG appear only in the shell list)	crates/openab-core/src/acp/connection.rs:420, openab-agent/src/tools.rs:77
F9	🟡	(carried from round-1) Canary prompt assertion still accepts result-or-error, so the "no provider call" guarantee rests entirely on proxy env vars	scripts/windows-acp-native-canary.ps1:335
F10	🟡	New in the F1 fix: when the bounded pipe join expires, join_shell_pipes aborts the readers and returns empty stdout/stderr -- a successful command whose background descendant holds the pipes loses ALL captured output silently	openab-agent/src/tools.rs:360-383
F11	🟢	All six round-1 blockers fixed, each with a matching regression test	--

Finding Details

🟡 F7: Unflagged cross-platform working_dir hardening

tool_bash now routes working_dir through validate_path, rejecting absolute paths outside the sandbox on every platform. This is justifiable hardening, but it changes observable behavior for existing Unix deployments inside a Windows-titled PR. Requested change: one sentence in the PR body (and changelog if applicable) stating the new restriction.

🟡 F8: Duplicated, diverging Windows env baselines

The ACP spawn path (connection.rs) and the shell tool (tools.rs) each hardcode a Windows env allow-list, and they already differ (USERPROFILE, USERNAME, HOME, LANG are shell-only; the spawn path passes SystemRoot/SystemDrive separately). Requested change: extract one shared constant so future additions cannot silently diverge.

🟡 F9: Canary prompt assertion is result-or-error

$promptSettled passes when the prompt returns either a result or a JSON-RPC error, so a canary environment that accidentally reaches a real provider still passes. Requested change: assert the specific failure shape expected without credentials (or assert the proxy-blocked error), so the no-provider guarantee is enforced by the assertion, not only by env vars.

🟡 F10: Bounded pipe join silently discards output

join_shell_pipes returns Ok((Vec::new(), Vec::new())) when the deadline expires. For the Completed path this means a command that exits 0 after printing meaningful output -- but leaves a daemonized descendant holding the pipe -- reports success with empty output and no truncation marker. Requested change: preserve the partially read buffers (e.g. stream into a shared buffer the supervisor can take) or at minimum append an explicit [output truncated: pipe held open past deadline] marker so callers can tell loss from silence.

🟢 F11: Round-1 blocker resolution verified

Round-1 finding	Status
F1 unbounded hang on timeout/cancel	Fixed: bounded joins (POST_KILL_PIPE_JOIN, POST_EXIT_PIPE_JOIN) + POST_TIMEOUT_CLEANUP around cancel_and_wait; two setsid-escape regression tests prove both paths return
F2 ReplaceFileW discards restricted DACL	Fixed: restrict_auth_dacl() after both ReplaceFileW and MoveFileExW paths
F3 trustee from USERNAME/USERDOMAIN	Fixed: process-token SID (GetTokenInformation(TokenUser), TRUSTEE_IS_SID via BuildTrusteeWithSidW); regression test spoofs the env vars and still succeeds
F4 single-shot terminate controller	Fixed: controller loops, later callers get Ok(()); second-terminate regression assertion added
F5 ACL-after-create window	Fixed: CreateFileW with explicit SECURITY_ATTRIBUTES, CREATE_NEW, FILE_SHARE_NONE -- no unprotected window
F6 no upstream CI gate	Fixed: main push + pull_request triggers with path filters; MANIFEST derives repository/branch from GITHUB_REPOSITORY / GITHUB_HEAD_REF

Baseline Check

• PR opened: 2026-08-19; head 39cb67c8f2b8bb56764234ef4283cff5e7e14150; base main (merge-base 280db4db = base HEAD, clean)
• Diff: 16 files, +2258 / -230; delta since round-1 head 2d805b9: 6 files, +408 / -82
• Round-1 (2d805b9) already reviewed the full change; this round verifies the F1-F6 fixes and re-checks the open items
• CI on head SHA: all green, including windows-dual-layer-canary, check, and the full smoke-test matrix

What's Good (🟢)

• Every blocker fix ships with a regression test that encodes the exact failure mode (setsid pipe escape, spoofed env trustee, double terminate) -- this is how review findings should be closed
• The token-SID DACL rework is the correct Win32 pattern: create-with-descriptor removes the race entirely instead of narrowing it
• CI trigger fix includes path filters and a per-PR concurrency group, not just a blanket trigger

Three Reasons We Might Not Need This PR

1. Deployment reality is Linux containers -- every upstream smoke gate is Docker/Linux; carrying security-critical Win32 code that maintainers cannot routinely exercise is standing risk, though the new upstream CI triggers (F6 fix) now mitigate this.
2. WSL2 already covers Windows developers via the mature Unix path; native support mainly benefits installer-based end users, and installer/GA remains explicitly out of scope.
3. Maintenance surface: ~1,900 lines of platform-specific lifecycle code with subtle Win32 semantics (Job Objects, DACL inheritance, ReplaceFileW) raises the bar for every future contributor touching these files.

All three are materially weaker than in round-1: the fixes were fast, test-backed, and correct, and the CI gate now runs upstream. The remaining items are small; with F7-F10 addressed this is a mergeable foundation.

[github-actions]

Caution

This PR has been waiting on the author for more than 2 days (labeled pending-contributor since 2026-08-20).
It will be automatically closed in 24 hours if there is no update.

@smallgun01 — You must add a new comment on this PR to remove the closing-soon label and keep it open. Pushing commits alone is not sufficient. Feel free to reopen a new PR later if it gets closed and you want to pick it back up.

[smallgun01]

Still actively working this PR.

Current head is 5f2b353 (merged latest main on 2026-08-21). After review round 2 I pushed the F7–F10 follow-ups (398bf4f, 5bd0ebb) and that merge. Official bot review did not start a third pass because this PR is labeled review-limit-reached.

On this head:

• Windows dual-layer canary and Hermes smoke are green
• Remaining CI red light is clippy::useless_format in crates/openab-core/src/setup/wizard.rs:163, which is not in this PR’s 17-file diff and came in via the main merge

Please keep the PR open. I will continue from here.