test(windows): await ACL probe teardown after timeout

[mohanagy]

Summary

• keep the existing 5-second hanging-probe deadline and kill behavior
• defer timeout rejection until the killed PowerShell child emits close, preventing the next ACL integration test from overlapping teardown
• add a platform-independent contract for deferred, single-use timeout settlement
• leave production ACL code and all timeout constants unchanged

Failure evidence

The exact development run and the first PR head both failed Windows Node 22 in the functional descriptor probe immediately after the intentionally hanging probe. Windows Node 20/24 passed, and every later functional ACL probe on Node 22 completed in roughly 1–1.6 seconds.

• development failure: https://github.com/mohanagy/miftah/actions/runs/31436336048
• falsifying PR run for the marker-placement hypothesis: https://github.com/mohanagy/miftah/actions/runs/31437321673

The timeout callback previously called child.kill() and rejected immediately. The revised harness still kills at 5 seconds but does not advance the test until close confirms teardown.

Local validation

• npx vitest run tests/windows-config-migration-acl.test.ts
• npm run lint
• npm run typecheck
• npm test -- --no-file-parallelism — 145 files passed; 1,837 tests passed; 34 skipped
• npm run build

Windows Node 20/22/24 validation remains pending on the revised head.

Closes #352

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of Windows permission checks that exceed their time limit.
  • Preserved timeout diagnostics while ensuring results are settled reliably after the underlying process completes.
  • Added safeguards against duplicate timeout handling and retained the first timeout error.
  • Ensured errors that occur when starting a permission check are reported immediately when no timeout is pending.
  • Added a fallback to settle timed-out checks when the underlying process does not close.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 14665d7f-f69f-418b-8cac-9f971bd95c76

📥 Commits

Reviewing files that changed from the base of the PR and between 3a80767 and 2384619.

📒 Files selected for processing (1)

• tests/windows-config-migration-acl.test.ts

---

📝 Walkthrough

Walkthrough

The Windows ACL probe now records timeout errors and rejects after child closure or a 2-second fallback. It preserves the first error, suppresses later errors, and settles once. Vitest fake-timer tests cover these behaviors.

Changes

Windows ACL timeout handling

Layer / File(s)	Summary
Record and defer probe timeouts tests/windows-config-migration-acl.test.ts	The probe records the first stage-specific timeout before killing the child process. It defers rejection until child closure or fallback expiry.
Settle deferred probe errors tests/windows-config-migration-acl.test.ts	Child-start and close handling use the deferred settlement state. Tests verify first-error preservation, idempotence, fallback settlement, and immediate rejection without a pending timeout.

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

Possibly related PRs

• mohanagy/miftah#229: Introduces the Windows ACL probe flow covered by this timeout handling.
• mohanagy/miftah#353: Modifies the same Windows ACL test timeout behavior.
• mohanagy/miftah#358: Refines timeout handling in the same Windows ACL test.

Poem

A timeout waits while children close,
The first error stays composed.
A fallback ends the waiting spree,
One clean rejection sets it free.
Fake timers check the flow. 🐇

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (2 errors)

Check name	Status	Explanation	Resolution
Linked Issues check	❌ Error	The changes preserve the bounded hanging probe and production boundaries, but required Windows 20/22/24 and protected-matrix validation remains pending.	Run the focused Windows ACL tests on Node 20, 22, and 24 and the full protected matrix on the revised commit before merge.
Docstring Coverage	❌ Error	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the Windows ACL test change and deferred teardown behavior.
Description check	✅ Passed	The description provides a focused summary, failure evidence, and validation commands, but omits the required Security impact section.
Out of Scope Changes check	✅ Passed	The changes are limited to the Windows ACL test harness and directly support the linked issue without modifying production ACL behavior.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/issue-352-acl-bootstrap-boundary

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/windows-config-migration-acl.test.ts`:
- Around line 610-621: Update the test “defers a timed-out ACL probe rejection
until child closure is observed” to call markTimedOut with a second Error before
settleAfterClose, then assert the rejection is the original timeoutError by
identity using toBe rather than only collection equality. Preserve the existing
deferred settlement and idempotent second settleAfterClose assertions.
- Around line 104-122: Update the ChildProcess error-handling flow to preserve a
timeout recorded by createDeferredPowerShellProbeTimeout.markTimedOut when
child.kill() emits error, defer rejection until close via settleAfterClose(),
and retain immediate startup-error rejection when no timeout is pending. Add a
regression test covering timeout → error → close and asserting rejection with
the timeout error.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1adede2c-10c6-40dd-aff2-fe71c73cb85d

📥 Commits

Reviewing files that changed from the base of the PR and between 047c24f and de855f3.

📒 Files selected for processing (1)

• tests/windows-config-migration-acl.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/windows-config-migration-acl.test.ts`:
- Around line 104-129: Update the Windows ACL probe timeout flow around
createDeferredPowerShellProbeTimeout and the child kill/onTimeout handling:
record the timeout before invoking child.kill(), so synchronous kill errors
cannot reject first. Add a one-shot fallback to settle the deferred timeout when
the child never emits close, and adjust the regression to have kill() emit the
error synchronously.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 65a45252-3314-45a7-8c28-356e4d4c1955

📥 Commits

Reviewing files that changed from the base of the PR and between de855f3 and 3a80767.

📒 Files selected for processing (1)

• tests/windows-config-migration-acl.test.ts