test: diagnose Windows descendant readiness boundary

[mohanagy]

Summary

• Diagnose the Windows secret-provider cold-start boundary without weakening timeouts or Job Object containment.
• Add a provider-entry sentinel and a Windows-only settlement diagnostic.
• Share the readiness-wait helper requested in review (test: share Windows provider-entry readiness wait helper #125).

Validation

• npm exec -- vitest run tests/secret-providers.test.ts
• npm exec -- eslint tests/secret-providers.test.ts
• npm run typecheck

Refs #115
Refs #125

Summary by CodeRabbit

• Tests
  • Strengthened Windows-specific secret command lifecycle coverage with provider-entry synchronization.
  • Added a sentinel-based readiness check to ensure the fake provider signals “entered” before assertions proceed.
  • Updated Windows tests to coordinate provider startup, descendant PID capture, and provider/child shutdown behavior.
  • Added timing safeguards to fail fast if commands don’t settle within the expected window.

[coderabbitai]

📝 Walkthrough

Walkthrough

The fake secret provider emits a startup sentinel, and Windows tests wait for it before validating command settlement or orphaned-descendant behavior.

Changes

Windows provider lifecycle

Layer / File(s)	Summary
Provider readiness sentinel tests/fixtures/fake-secret-provider.mjs	The fixture reads MIFTAH_FAKE_PROVIDER_READY_PATH and writes provider-entered when the provider starts.
Windows lifecycle timing tests tests/secret-providers.test.ts	Adds sentinel polling, validates command settlement after provider entry, and synchronizes descendant-process assertions with provider startup.

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

Possibly related issues

• mohanagy/miftah#125: Proposes extracting the duplicated provider-entry readiness waits added to these tests.

Possibly related PRs

• mohanagy/miftah#103: Adds related provider and orphaned-descendant lifecycle coverage.

Poem

A rabbit marked the provider’s wake,
So timing tests know what path to take.
Windows waits, then checks with care,
While orphaned helpers vanish in air.
Hop, hop—clean results everywhere!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers summary and validation, but it omits the required Security impact section and the checklist-style validation details.	Add a Security impact section and complete the template’s validation checklist with the exact commands run and relevant results.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately highlights the Windows descendant-readiness boundary fix.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/115-windows-descendant-readiness

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/secret-providers.test.ts (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the duplicated "wait for provider-entered marker" logic into a shared helper.

Both sites implement the identical pattern: waitForCondition wrapping a readFile(providerReadyPath, "utf8") === "provider-entered" check with ENOENT tolerance, differing only in the description string. Extracting a helper (e.g. waitForProviderEntered(providerReadyPath, description)) removes the duplication and gives future marker-boundary tests a single implementation to update.

• tests/secret-providers.test.ts#L412-422: replace this inline block with a call to the shared helper.
• tests/secret-providers.test.ts#L1251-1261: replace this inline block with a call to the shared helper.

♻️ Proposed helper

+async function waitForProviderEntered(providerReadyPath: string, description: string): Promise<void> {
+  await waitForCondition(async () => {
+    try {
+      return (await readFile(providerReadyPath, "utf8")) === "provider-entered";
+    } catch (error) {
+      if (errorCode(error) === "ENOENT") return false;
+      throw error;
+    }
+  }, description);
+}

Then at each call site:

-          await waitForCondition(
-            async () => {
-              try {
-                return (await readFile(providerReadyPath, "utf8")) === "provider-entered";
-              } catch (error) {
-                if (errorCode(error) === "ENOENT") return false;
-                throw error;
-              }
-            },
-            "the cold fake provider to enter through the Windows helper"
-          );
+          await waitForProviderEntered(providerReadyPath, "the cold fake provider to enter through the Windows helper");

🤖 Prompt for 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.

In `@tests/secret-providers.test.ts` at line 1, In tests/secret-providers.test.ts,
extract the duplicated waitForCondition/readFile logic that waits for the
"provider-entered" marker into a shared
waitForProviderEntered(providerReadyPath, description) helper. Preserve ENOENT
tolerance and the existing marker check, then replace both inline blocks near
the referenced call sites with helper calls using their respective descriptions.

🤖 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.

Outside diff comments:
In `@tests/secret-providers.test.ts`:
- Line 1: In tests/secret-providers.test.ts, extract the duplicated
waitForCondition/readFile logic that waits for the "provider-entered" marker
into a shared waitForProviderEntered(providerReadyPath, description) helper.
Preserve ENOENT tolerance and the existing marker check, then replace both
inline blocks near the referenced call sites with helper calls using their
respective descriptions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e5ddbf25-60af-445a-a5d6-ecb6a209641c

📥 Commits

Reviewing files that changed from the base of the PR and between 8139328 and 651039d.

📒 Files selected for processing (2)

• tests/fixtures/fake-secret-provider.mjs
• tests/secret-providers.test.ts