test(sandbox): wait for forward fixture readiness

[apurvvkumaria]

Summary

The recovery test fixture could report a forward as healthy before its detached listener had actually bound the socket. Under runner contention that race exhausted the production recovery polling window and created an unrelated CI failure on #7596. This change makes fixture readiness event-driven while leaving production recovery behavior unchanged.

Related Issue

Part of #7140. Follow-up to the unrelated CI failure observed on #7596.

Changes

• Write a private ready marker from the fake listener only after the server listen callback fires.
• Wait for that marker before initial fixture setup completes and before fake forward start publishes pending or running metadata.
• Preserve listener PID tracking and cleanup, with a bounded five-second failure budget for a listener that never becomes ready.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: the diff is confined to a fake test listener and changes no command, output, flag, default, or documented recovery behavior.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: an independent Codex Desktop review checked listener lifecycle, PID cleanup, platform support, and production sandbox semantics; it reported no actionable findings.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: the exact diff only stabilizes test/recover-port-forward.test.ts; existing recover and forward documentation remains accurate.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit: not applicable
• Station profile/scenario: not applicable
• Result: not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
• Supporting evidence:

Verification

• PR description includes a Signed-off-by line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 3 focused files, 42 tests passed after rebasing onto current main.
• Applicable broad gate passed — not applicable; this is a one-file fixture-only change. The changed-file hook set, Biome, and CLI typecheck passed.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new doc pages only)

Additional stability evidence: ten sequential fixture repetitions passed, and a separate run passed while three CPU-saturation processes were active.

---

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

• Tests
  • Improved test fixture reliability by waiting for mock forward listeners to be ready before running checks.
  • Added timeout handling when listener readiness cannot be confirmed.
  • Reduced potential race conditions during port-forwarding recovery tests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 49a6e3c6-4f9d-4eff-850c-da7ef512bcc5

📥 Commits

Reviewing files that changed from the base of the PR and between 968385b and 0aa3ebe.

📒 Files selected for processing (1)

• test/recover-port-forward.test.ts

---

📝 Walkthrough

Walkthrough

The recovery port-forward fixture now uses ready-file synchronization to confirm mock listeners have bound their ports before proceeding. Forward startup removes stale markers, waits with a timeout, and exits unsuccessfully when readiness is not observed.

Changes

Listener readiness synchronization

Layer / File(s)	Summary
Listener readiness helpers test/recover-port-forward.test.ts	Adds a bounded readiness timeout, writes a marker after listener binding, and waits for that marker during reachable-forward startup.
Forward startup gating test/recover-port-forward.test.ts	Updates fake openshell forward start and reachable-port setup to clear, pass, and await the listener ready marker, failing on timeout.

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

Suggested labels: area: sandbox, bug-fix

Suggested reviewers: cjagwani, cv, ericksoa

🚥 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 clearly matches the main change: waiting for the forward fixture to become ready before continuing.
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.

✨ 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 codex/recover-forward-listener-handshake

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 0aa3ebe in the codex/recover-forwar... branch remains at 96%, unchanged from commit 968385b in the main branch.

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

1 optional E2E recommendation

• tunnel-lifecycle

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.