test(workflow): gate windows cli timing on model-call hook by echoVic · Pull Request #15 · echoVic/orca-agent

echoVic · 2026-07-31T05:12:22Z

Problem

The native Windows CI (windows-ci.yml) has been red on main because four tests/workflow_cli_contract.rs contracts fail deterministically on both Windows x64 and ARM64:

workflow_run_returns_before_slow_workflow_completes
workflow_stop_requests_real_background_stop
workflow_pause_resume_and_clone_control_persisted_run
workflow_source_command_prints_saved_workflow_source

Root causes:

Timing race (3 tests). These held a workflow "in flight" with a pre_model_call hook that ran sleep/Start-Sleep, then raced a fixed thread::sleep in the test against it. The workflow runs in a separate background worker process; on loaded Windows runners the worker startup + first model call outran the short (150–250 ms) test waits, so workflow pause/stop observed an already-terminal task and exited 1 (Some(1) vs Some(0) in CI).
Path canonicalization (1 test). workflow source reports the path resolved from the working directory (not canonicalized), so on Windows the test's script.canonicalize() expectation added the \\?\ verbatim prefix and expanded 8.3 short names, e.g. C:\Users\RUNNER~1\... vs \\?\C:\Users\runneradmin\....

Fix (test-only)

Replace the wall-clock sleep with a deterministic gate: the hook writes a started marker and blocks until the test writes a release file (condition-based waiting, matching the proven shell_session_contract.rs pattern). The workflow is provably mid-run when the test observes it, independent of CI scheduling. The hook self-releases after ~25 s to stay within the 30 s hook timeout.
Compare the reported source path after canonicalizing both sides so it is platform-neutral.

The product behavior is unchanged and correct; only the test expectations were platform-fragile.

Verification

cargo fmt --all -- --check
cargo test -p blade-deepseek --test workflow_cli_contract — 10/10, deterministic across 4 runs on macOS
Full cargo test --workspace --all-targets --locked -- --test-threads=1 green (aside from a pre-existing intermittent tui_pty_contract render flake that passes in isolation)
Windows-shaped hook TOML validated through the repo's real toml + HookConfig parser (backslash paths preserved, quotes balanced)
node scripts/validate-windows-platform-boundaries.mjs

Windows x64 + ARM64 CI on this PR is the authoritative check.

Summary by CodeRabbit

Tests
- Improved workflow command validation across platforms by standardizing source path comparisons.
- Replaced fixed delays with reliable state-based synchronization for slow, stop, and pause/resume scenarios.
- Added clearer diagnostics when workflow tests fail.
- Expanded coverage for execution markers, release controls, configuration escaping, and shell path handling.
- Reduced test flakiness and increased confidence in consistent workflow behavior across supported environments.

The workflow CLI pause/stop/slow-run contracts raced a wall-clock sleep hook against background-worker startup on loaded Windows CI runners, so pause and stop observed an already-terminal task and exited non-zero. The source contract also compared the reported path against a `canonicalize`d path, which diverges on Windows by adding the `\\?\` verbatim prefix and expanding short names. Replace the sleep with a deterministic pre_model_call gate: the hook records a start marker and blocks until the test writes a release file, so the workflow is provably mid-run when the test observes it. Compare the source path after canonicalizing both sides. Co-authored-by: TRAE CLI <noreply@bytedance.com>

coderabbitai · 2026-07-31T05:12:42Z

Warning

Review limit reached

@echoVic, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 56ee2435-9b11-4ed3-ad68-4f623d0cffd7

📥 Commits

Reviewing files that changed from the base of the PR and between 035787b and 1b05430.

📒 Files selected for processing (1)

scripts/test-validate-windows-platform-boundaries.mjs

📝 Walkthrough

Walkthrough

Workflow CLI contract tests now canonicalize source paths and replace fixed sleeps with deterministic mock-provider delays. Tests poll workflow activity before completion, stop, or pause operations and include diagnostic state output on failure.

Changes

Workflow CLI test synchronization

Layer / File(s)	Summary
Deterministic test control `tests/workflow_cli_contract.rs`	Source-path assertions canonicalize both paths. Slow, stop, and pause tests use delayed mock-provider calls and bounded workflow-state polling. Stop and pause failures include stderr and persisted workflow details.

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

Possibly related PRs

echoVic/orca-agent#13: Both changes modify workflow test timing and synchronization across platforms.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies the Windows workflow CLI timing tests and their deterministic model-call timing gate, which matches the primary changes.
Docstring Coverage	✅ Passed	Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%.
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.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Fix failing CI checks

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/windows-workflow-cli-gate

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

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

The Windows CI still failed the three timing gates after the first fix, so mirror the shell_session_contract.rs marker-wait pattern exactly (Set-Content marker + Test-Path poll, no custom deadline expression) and surface the workflow show state plus a hook activity log on failure to pin down why the run leaves its active state before release. Co-authored-by: TRAE CLI <noreply@bytedance.com>

CI diagnostics showed the release-file gate could not work on Windows: the pre_model_call hook has a hard 30s timeout, and the worker's hook process file writes were not observable to the test until the hook was reaped, so the gate always hit the timeout and the run failed. Replace the gate with the proven approach: a fixed 8s Start-Sleep/sleep hold (well under the 30s hook timeout, matching session_server_contract) plus test-side polling for the queued/running state. This removes the background-worker startup race without depending on cross-process file visibility or the hook lifecycle. Co-authored-by: TRAE CLI <noreply@bytedance.com>

CI diagnostics proved the shell sleep hook could not reliably hold the workflow worker on Windows: the pre_model_call hook either hit its 30s timeout (blocking gate) or failed to delay the run at all, so the CLI control commands raced an already-terminal task. Drive the delay from inside the mock provider instead. A `mock_stream_delay_ms <n>` agent prompt performs a real, cancellable, platform-neutral delay in the streaming provider path, with no shell, hook, or cross-process file dependency. Tests poll for the active state before issuing run/stop/pause, removing the background-worker startup race that the fixed sleeps were guarding against. Co-authored-by: TRAE CLI <noreply@bytedance.com>

The workflow CLI contracts no longer need platform-specific shell sleep hooks. They now use the mock provider's cancellable cross-platform delay and poll persisted active state, which avoids both worker-startup races and the 30-second hook timeout. Keep the dialect-aware hook assertion for the remaining hook-based fixtures and require the stronger workflow pattern explicitly. Co-authored-by: TRAE CLI <noreply@bytedance.com>

echoVic and others added 4 commits July 31, 2026 13:55

echoVic merged commit 1b05430 into main Jul 31, 2026
1 of 3 checks passed

coderabbitai Bot mentioned this pull request Jul 31, 2026

test(workflow): classify active state by exclusion, not allowlist #16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(workflow): gate windows cli timing on model-call hook - #15

test(workflow): gate windows cli timing on model-call hook#15
echoVic merged 5 commits into
mainfrom
fix/windows-workflow-cli-gate

echoVic commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

echoVic commented Jul 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix (test-only)

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

echoVic commented Jul 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 31, 2026 •

edited

Loading