Skip to content

test(ssh): wait for a real zombie before the unreaped-server launch test - #597

Merged
yaacovcorcos merged 1 commit into
mainfrom
claude/ssh-tunnel-flake-20261019
Oct 19, 2026
Merged

yaacovcorcos merged 1 commit into
mainfrom
claude/ssh-tunnel-flake-20261019

Conversation

@yaacovcorcos

Copy link
Copy Markdown
Contributor

What Changed

packages/ssh/src/tunnel.test.ts, "replaces a managed server that exited but was not reaped": instead of reading the background child's process state once, the test now waits (polling every 0.1 s, at most ~10 s) until

  1. the parent shell has become sleep, and only then kills the child, and
  2. ps reports the child as a zombie (Z).

Test-only, inside a SCIENT-FORK region (the test comes from upstream T3 #19432; upstream main still has the same code). No timeouts raised, product code unchanged.

Why

The test is a recurring flake in the required "Test Workspaces" job: expected '' to include 'Z' (main runs 39170121279 and 39155611850, 18 October). A flake there throws PRs out of the merge queue.

The parent is sh -c "sleep 600 & echo $!; exec sleep 600". On Ubuntu sh is dash, which reaps exited children after every builtin, including that echo (dash 0.5.12 trap.c:90-91 installs the SIGCHLD handler; eval.c:922 reaps after builtins). The PID reaches the test while dash is still finishing the echo; a kill in that window is reaped before the exec, so no zombie exists and ps prints nothing, which is exactly the '' in the failing runs (a merely late kill would show a live state such as S).

Verification

  • vp fmt --check, vp lint, tsc-rs --noEmit for packages/ssh, git diff --check: clean.
  • On Windows the file runs 26 passed / 12 skipped; this test skips itself there, so this PR's Linux CI is its first real run. The polling loop's success and give-up exit codes were checked in Git Bash.

Documentation Impact

None — test-only change.

🤖 Generated with Claude Code

The "replaces a managed server that exited but was not reaped" test
spawned `sh -c "sleep 600 & echo $!; exec sleep 600"`, killed the
background child as soon as its PID arrived, and read `ps -o stat=` once.
The PID arrives while dash is still finishing the `echo` builtin, and
dash reaps exited children after every builtin. When the SIGKILL landed
in that window, the shell reaped the child before exec'ing `sleep`, so
`ps` found no process and printed nothing ("expected '' to include 'Z'").

The test now waits until the parent has exec'd `sleep`, which never
reaps, before killing the child, then polls with a bounded deadline
until `ps` reports the zombie, since SIGKILL also takes effect
asynchronously. The test came from upstream T3 (#19432), which still
has the race; the change sits in a SCIENT-FORK region.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Oct 19, 2026
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 26.2 KiB 26.2 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 26.6 KiB 26.6 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 2cce7fe · PR result: 94581a0 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Full compact HTTP snapshot with live cursor.

  • Codex decoded thread snapshot: 133.1 KiB
  • Claude decoded thread snapshot: 133.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@yaacovcorcos
yaacovcorcos added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 8070134 Oct 19, 2026
51 of 59 checks passed
@yaacovcorcos
yaacovcorcos deleted the claude/ssh-tunnel-flake-20261019 branch October 19, 2026 12:43
yaacovcorcos pushed a commit that referenced this pull request Oct 19, 2026
Catches up owned main with Scient PRs #597 (SSH tunnel test flake) and #598
(APA citation test flake). Clean merge; no conflicts or dependency changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
yaacovcorcos pushed a commit to nattikahana/scient-desktop that referenced this pull request Oct 20, 2026
A queue entry ran the full matrix even when its PR's CI had just passed on
the same content. Now a merge_group run reuses that PR run when it tested
the same tree against the same base and the PR head already holds that
tree: the PR contained the latest main when its CI ran, nothing landed
since, and nothing is queued ahead. Then the merge changes nothing, so the
PR's changed-file list equals the queue's diff for every change gate, and
Compute recipe sealing's path-limited history read follows the PR head,
never the merge commit, whose timestamp differs between the two merges.

- Same-repository PR runs record a qualification receipt (schema 2: event,
  tested merge commit, base, tree). Fork PRs never do.
- The queue probe checks the queue commit merges exactly that PR head onto
  the group base, and that the PR run's tested merge has the same tree and
  parents, then republishes its seven reports.
- A matrix job skipped as a whole reports one unexpanded name, so reusing
  queue runs add small jobs that report the required Test Web N and Test
  Server N names.
- A main push that lands several queue entries now reuses when each entry
  qualified from its first parent back to the previous main, up to the
  queue's four-entry merge limit. Before, ScientFactory#597 and ScientFactory#598 landing together
  made ScientFactory#598's push run full CI ("Qualification receipt differs").

Any missing, expired or mismatched evidence still runs normal CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant