Skip to content

test: diagnose Windows cold provider entry timing - #136

Merged
mohanagy merged 4 commits into
developmentfrom
fix/135-windows-cold-provider
Jul 21, 2026
Merged

test: diagnose Windows cold provider entry timing#136
mohanagy merged 4 commits into
developmentfrom
fix/135-windows-cold-provider

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Resolves #135.

Summary

Replaces the Windows-only cold-provider latency probe with a causal contract: the same un-aborted request must return the fixture output and preserve the provider-entry marker after the contained helper settles.

The prior probe failed at its test-owned 1.5-second observation boundary even when the same request completed successfully after settlement. It therefore tested unsupported cold-start timing rather than secret-provider behavior.

Security impact

No production runtime behavior changes. Windows helper delivery, explicit argument-array execution, shell: false, redaction, bounded cancellation, and kill-on-close Job Object containment are unchanged. No timeout was increased, no platform behavior was skipped, and coverage thresholds were not changed.

Validation

  • local npm test: one non-reproducible doctor-test diagnostic was observed, then passed in the immediate coverage rerun and exact-head CI; tracked and closed as test: diagnose intermittent strict doctor capacity assertion under full suite #139 with evidence
  • local npm run test:core: 393 passed, 23 platform skips
  • local npm run test:coverage: 1066 passed, 23 platform skips; 95.49% statements
  • local lint, typecheck, build, CLI smoke, package check, and package test: passed
  • exact-head CI: Linux quality, Verify, and Linux/macOS/Windows Node 20/22/24: passed
  • CodeRabbit: approved with no actionable findings

The diagnostic commits are intentionally preserved.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Windows secret command runner test now directly awaits command completion, verifies fixture output, and confirms the provider-entry marker was written, replacing timeout-based settlement observation and conditional abort logic.

Changes

Windows provider test

Layer / File(s) Summary
Provider readiness assertion
tests/secret-providers.test.ts
The test removes pending-promise and timeout race handling, then validates command output and the synchronous provider-entered marker after runSecretCommand returns.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

  • mohanagy/miftah#62: Adds and refines Windows secret-command runner and external provider execution tests.
  • mohanagy/miftah#121: Introduces provider-readiness synchronization using the same sentinel mechanism.

Poem

A bunny watched the provider start,
No timeout race, no frantic dart.
The command returns, the marker’s bright,
provider-entered proves it right.
Windows hops along in calm delight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change aligns with #135 by replacing the nondeterministic Windows probe with a minimal deterministic diagnostic while preserving the existing safeguards.
Out of Scope Changes check ✅ Passed The diff is confined to the Windows secret-provider test and does not introduce unrelated production or behavior changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title is concise and matches the Windows test change around cold-provider timing and entry-marker diagnostics.
Description check ✅ Passed The description includes Summary, Security impact, and Validation with concrete results, so it largely matches the template.
✨ 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/135-windows-cold-provider

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

@mohanagy

Copy link
Copy Markdown
Owner Author

CodeRabbit reported a temporary Fair Usage review limit (38 minutes) for this one-file diagnostic PR; see its automated status comment above. That is a documented external review-service limitation, not an unresolved review. No CodeRabbit finding or thread exists. I will rely on the exact-head full CI matrix and human maintainer review for this diagnostic, and will not treat the rate-limit message as approval.

@mohanagy

Copy link
Copy Markdown
Owner Author

Maintainer self-review for exact head e1c89b5 is complete. The net diff against current development is one Windows-only test change: it removes an unsupported cold-start latency assertion and retains the causal contract that the contained Node provider completes with its marker and expected output. No production source, timeout, coverage threshold, shell setting, process-tree containment, or Windows skip changed. Exact-head CI is green across Linux quality, Verify, and Linux/macOS/Windows Node 20/22/24. No unresolved human review comments exist; CodeRabbit remains the final required gate.

@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
mohanagy merged commit 9533373 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.

ci: diagnose intermittent Windows core-contract failures on v0.3.2 promotion

1 participant