Skip to content

test: prove Windows helper waits for provider readiness - #147

Merged
mohanagy merged 2 commits into
developmentfrom
fix/144-windows-readiness-barrier
Jul 21, 2026
Merged

test: prove Windows helper waits for provider readiness#147
mohanagy merged 2 commits into
developmentfrom
fix/144-windows-readiness-barrier

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Fixes #144

Replaces the post-settlement marker check with a controllable local TCP readiness barrier. The regression races fixture-barrier entry against command settlement, requires the command to remain pending, then releases the provider and verifies stdout plus argv.

Validation:

  • npm test -- tests/secret-providers.test.ts
  • npm run lint
  • npm run typecheck

Summary by CodeRabbit

  • Tests
    • Improved synchronization for secret-provider integration tests using a TCP readiness “barrier.”
    • Updated the Windows-specific secret command runner test to keep the provider pending until the barrier is released, then verify expected output and captured command arguments.
    • Added safer port handling and more reliable startup/wait behavior to reduce timing-related flakiness.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 50a64d10-661c-4a59-81e7-510aceb7b1c7

📥 Commits

Reviewing files that changed from the base of the PR and between 5b790af and 29c452a.

📒 Files selected for processing (1)
  • tests/secret-providers.test.ts
📝 Walkthrough

Walkthrough

Adds a TCP readiness barrier to the fake secret provider and updates the Windows secret-command-runner regression test to verify runner settlement remains pending until the provider is explicitly released.

Changes

Provider readiness synchronization

Layer / File(s) Summary
Fixture TCP barrier
tests/fixtures/fake-secret-provider.mjs
The fake provider validates an optional TCP barrier port, waits for a data or end signal, and handles socket errors or premature closure.
Runner readiness regression
tests/secret-providers.test.ts
A local TCP barrier controls provider release, while the Windows test verifies pending settlement, stdout, recorded arguments, and cleanup.

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

Sequence Diagram(s)

sequenceDiagram
  participant SecretCommandRunner
  participant FakeSecretProvider
  participant ReadinessBarrier
  SecretCommandRunner->>FakeSecretProvider: start provider command
  FakeSecretProvider->>ReadinessBarrier: connect and wait
  ReadinessBarrier-->>SecretCommandRunner: signal provider reached barrier
  ReadinessBarrier->>FakeSecretProvider: release with payload
  FakeSecretProvider-->>SecretCommandRunner: continue and return stdout
Loading

Possibly related PRs

  • mohanagy/miftah#121: Modifies the same fixture and test to establish a provider readiness boundary.
  • mohanagy/miftah#62: Expands the secret-provider command execution tests touched by this readiness regression.

Poem

A bunny guards the TCP gate,
The provider waits, then meets its fate.
“Release!” hops across the wire,
Commands resume with stdout fire.
Tests now know the timing’s right—
Ears up high, the build takes flight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes a summary and validation commands, but it omits the required Security impact section and most checklist items. Add a Security impact section and complete the repository checklist, including test outcomes for the required commands and any applicable docs/update notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the main change: the Windows helper now waits for provider readiness.
Linked Issues check ✅ Passed The changes add a controllable TCP readiness barrier and a focused regression that proves the helper stays pending until release.
Out of Scope Changes check ✅ Passed The diff stays focused on the readiness-barrier fixture and its regression test without unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/144-windows-readiness-barrier

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

@mohanagy

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mohanagy

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@mohanagy
mohanagy force-pushed the fix/144-windows-readiness-barrier branch from f7f9648 to 5b790af Compare July 21, 2026 09:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/secret-providers.test.ts`:
- Around line 143-149: Update the server setup promise around server.listen so
the server retains a permanent no-op error listener after listening succeeds,
matching the accepted socket handling. Remove only the temporary reject listener
and attach the persistent handler before resolving, while preserving rejection
behavior for listen-time errors.
🪄 Autofix (Beta)

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: 1a28b71b-bf49-4ba1-9174-b87049e8ff28

📥 Commits

Reviewing files that changed from the base of the PR and between f7f9648 and 5b790af.

📒 Files selected for processing (2)
  • tests/fixtures/fake-secret-provider.mjs
  • tests/secret-providers.test.ts

Comment thread tests/secret-providers.test.ts
@mohanagy

Copy link
Copy Markdown
Owner Author

Addressed CodeRabbit finding under #151 in 29c452a. The readiness barrier retains a no-op server error listener after listen() succeeds, while the temporary listen-time reject listener is still removed. A focused regression emitted a post-listen server error and failed before the change; it now passes. The full tests/secret-providers.test.ts file also passes locally (80 passed, 12 Windows-only skipped).

@mohanagy
mohanagy merged commit a39e3f3 into development Jul 21, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: prove Windows helper settlement follows provider readiness barrier

1 participant