fix(review): bound Windows owner command duration

[danielgap]

Closes #990

Summary

• apply one shared 30-second timeout to all five Windows owner and ACL process launches
• surface sanitized timeout diagnostics without stderr, paths, encoded PowerShell payloads, or arbitrary error text
• add selective regression coverage for every operation under both ETIMEDOUT and killed-process failures

Changes

File	Change
lib/review-candidate-view-owner.ts	Centralizes the Windows command timeout and converts timeout-shaped failures into a sanitized typed error.
lib/review-candidate-view.ts	Preserves the safe timeout detail through candidate owner preparation diagnostics.
tests/review-candidate-view.test.ts	Covers the five operations, both timeout shapes, owner-query preservation, and sensitive-data exclusion.

Test plan

• Focused timeout matrix: 5 operations × 2 timeout shapes
• node --experimental-strip-types --test tests/review-candidate-view.test.ts — 137 passed, 7 skipped, 0 failed
• pnpm typecheck — 200 recorded diagnostics, no regressions
• pnpm run check:runtime-modules
• pnpm run check:provider-contract
• pnpm run test:harness
• git diff --check
• Independent read-only verifier — PASS

Validation notes

• pnpm test reached 2,353 passed and 0 failed, but exited with 10 cancellations in tests/rdd-status-line.test.ts (Promise resolution is still pending). The isolated file reproduces the same 7 passed / 10 cancelled result on a clean main checkout; this PR does not touch that file.
• Native review could not execute because the reviewer host returned a quota-exhausted 429; zero reviewers were prepared or submitted. The high-risk fallback required writer self-verification plus an independent verifier, both completed.
• Native Windows process termination and ACL effects remain delegated to Windows CI; local regression coverage uses the existing Linux mock boundary.

Contributor checklist

• Linked an approved issue
• Add exactly one type:* label — maintainer action required: type:bug (fork author lacks permission)
• No shell scripts changed (shellcheck not applicable)
• No skills changed (agent skill testing not applicable)
• No user-facing documentation change required
• Conventional commit format
• No Co-Authored-By trailers

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of Windows ownership and access-control validation by applying a consistent 30-second command timeout.
  • Windows command timeouts now produce clear candidate-view errors instead of being treated as generic validation failures.
  • Failure messages now avoid exposing sensitive file paths, command output, error details, or encoded payloads.
  • Timeout-related failures are reported consistently across Windows ownership and permission checks.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7a1c1dbd-e2b4-47fa-9b07-8f2bda02d2b9

📥 Commits

Reviewing files that changed from the base of the PR and between 25ad285 and 2a0d61b.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8ceff95b-2c91-4b53-9094-5b98879cc2ec

📥 Commits

Reviewing files that changed from the base of the PR and between 0812a5a and 25ad285.

📒 Files selected for processing (3)

• lib/review-candidate-view-owner.ts
• lib/review-candidate-view.ts
• tests/review-candidate-view.test.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The change centralizes Windows owner-command execution with a 30-second timeout and typed timeout errors. Candidate-view preparation now reports timeout details through CandidateViewError. Tests cover all five Windows owner operations and message sanitization.

Changes

Windows owner timeout handling

Layer / File(s)	Summary
Centralized Windows owner command execution lib/review-candidate-view-owner.ts	A shared helper resolves Windows executables, applies a 30-second timeout, and raises WindowsOwnerCommandTimeoutError for timed-out or killed processes. Owner and ACL operations use the helper.
Candidate-view timeout error propagation lib/review-candidate-view.ts	Candidate-view owner preparation maps typed command timeouts to detailed CandidateViewError instances and preserves generic handling for other failures.
Timeout and message validation tests/review-candidate-view.test.ts	Tests verify the 30-second timeout for all five Windows owner operations and confirm that failure messages exclude raw paths, stderr, and payload data.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: applying a duration limit to Windows owner command execution.
Linked Issues check	✅ Passed	Issue #990 requires longer Windows owner-command timeouts and the wrapped cause in CandidateViewError diagnostics. The PR applies one 30,000 ms timeout to whoami.exe, powershell.exe, and `icacls…
Out of Scope Changes check	✅ Passed	The changes remain within Issue #990. They modify Windows owner and ACL process execution, timeout error handling, candidate-owner diagnostics, and related regression tests. The summary identifies no …

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[danielgap]

Maintainer action requested: please add the type:bug label. GitHub rejected the fork author’s label update due to repository permissions.

[danielgap]

Gentle nudge on this one. All checks are green (verify, Windows, CodeRabbit) and the patch is a single commit on current main; the only outstanding step is the maintainer-side type:bug label, since GitHub blocks fork authors from setting it. Happy to adjust anything a review turns up.

[danielgap]

Closing as superseded. Main landed this fix class through #1449 (3cebcf2): the Windows owner probes keep their transient retry (#1288) and now run under a configurable bound (GENTLE_PI_CANDIDATE_WINDOWS_PROBE_TIMEOUT_MS, default 15 s, max 120 s) instead of the hardcoded 5 s, which covers the cold-runner failures this branch targeted.

The one idea from here that main still lacks is the typed timeout error for clearer diagnostics; happy to open a small follow-up if that is useful.