Repository navigation
fix(tunnel): keep embedded tunnels alive through heartbeat write failures - #1411
Conversation
swjnxyf
left a comment
There was a problem hiding this comment.
Code review of ef2c504a6535953174b381a2514dc966ebc09f64 for #1410: No blocking issues found (LGTM).
- The change prevents a transient
readiness.jsonpublication failure from terminating the already-owned Tunnel, while preserving the existing startup gate, shutdown behavior, and durable restart-fence handling. - Runtime status exposes
tunnel_health_unavailableduring publication failure and clears it after recovery. The persisted heartbeat continues to expire after seven seconds, so it cannot be mistaken for a fresh readiness signal. - The regression tests cover shared-reader lock contention, subsequent recovery, missing health files, stale observations, and initial-launch failure. Windows core CI, including the new Tunnel tests, passed, along with the relevant Linux/macOS core jobs.
Non-blocking observation: persistent ACL or I/O errors still require operator intervention to restore publication; this is consistent with the focused scope of the fix.
Review outcome: Ready from the code-review perspective. GitHub does not allow the PR author to approve their own PR, so this is submitted as a review comment rather than an APPROVE event.
|
Thanks for the focused fix for #1410. The code review found no blocking issues, and the full CI run, including the Windows-native Tunnel heartbeat contention tests, has passed. This preserves the running Tunnel on transient health-file write failures without relaxing the initial launch gate or restart-fence protections. Merging this fix into |
Port the bounded heartbeat reliability fix from yyjeqhc#1411 (6d50934). Preserve initial publication, stale observations, fatal lifecycle and ownership boundaries. Add deterministic contention/recovery tests and native Windows coverage.
A periodic
readiness.jsonwrite failure currently tears down the embedded OpenAI Tunnel task and leaves the profile terminally failed until the owning Server restarts. Holding a separately opened shared file lock deterministically reproduces the writer failure; the existing supervisor then maps it totunnel_health_unavailable.Keep heartbeat publication recoverable after launch: retain the same owned task and live readiness observations, expose
tunnel_health_unavailablein runtime status, and try publication again at the normal health cadence. A successful write clears the diagnostic and publishes the latest observation. Log failure/recovery transitions only. Saved observations still expire after seven seconds, and the writer retains its existing private file, ACL, validation and nonblocking lock.The initial status write remains a launch prerequisite. Actual task errors, startup deadlines, shutdown and durable restart-fence behavior retain their existing handling; this change does not restart a task or replay work.
Add deterministic tests for shared-reader contention, recovery with updated readiness, missing files, stale saved readiness, terminal diagnostics and the initial launch gate. Run the new tests in Windows core CI as well as the ordinary Linux suite, and document the runtime contract.
Fixes #1410.
Validation on macOS:
cargo test --locked -p webcodex-environment --lib tunnel::health_tests— 2 passed.cargo test --locked -p webcodex --lib server_tunnels— 3 passed.cargo test --locked -p webcodex --lib project_entry::openai_tunnel_service::tests— 16 passed.cargo test --locked -p webcodex --lib server_shutdown— 9 passed.python3 -m unittest scripts.tests.test_ci_contract_ownership scripts.tests.test_ci_path_risk— 45 passed.cargo fmt --all -- --check,python3 scripts/check_markdown_links.py, andgit diff --checkpassed.The Rust test build emitted three warnings in unchanged code (
RunnerPollRequest,wait_for_admitted_action, andManagedWorktreeSource). The lock conflict was reproduced locally; the reporter's Windows machine and a live OpenAI connection were not exercised. Native Windows validation is delegated to the added CI step.