fix(e2e): wall spec two-clients scenario answers the recovery offer on its raw second context (+ cloud image entrypoint mode pin) - #736
Merged
Conversation
…wser context two-clients-same-sessionRef creates client B with browser.newContext(), bypassing the fixtures' context override, in a spec that runs file-wide recoveryOfferHandling:'manual'. B boots with fresh localStorage and is legitimately offered the session A just opened; the unanswered modal interdicts B's sidebar click forever (no actionTimeout configured), so the test burned its whole 300s budget and died before restartAbrupt() ever ran — presenting as '0 respawns' rather than the one-PTY contract violating. Deterministic at both ends of the suspected window (base 00fec15 and 3c52b2c proven by identical probe): a long-standing harness footgun, not a product regression — the server-side per-sessionRef single-flight converges to exactly one PTY in every probe. Install the documented installRecoveryOfferAutoDeclineOnContext adapter on contextB (canonical RESTORE-01 pattern per multi-client.spec.ts), so B declines through the real product path. No assertion weakened.
The e2e cloud image copies docker/cloud-run/entrypoint.sh into /usr/local/bin (outside the chown -R node:node /app) and then runs as USER node. A checkout with a restrictive umask (observed 700 on entrypoint.sh in fresh worktrees and the main checkout) copies in as 700 root-owned; chmod +x cannot widen read permission, so the node user gets EACCES and every Cloud Run task fails instantly with 'bash: /usr/local/bin/e2e-entrypoint.sh: Permission denied'. Git only tracks the x bit, so the hazard is invisible in diffs. Pin 0755.
…ersedes local RUN chmod 0755 pin)
… /proc reads spawn_record_carries_verifiable_proc_identity_and_freshagent_lane failed twice on CI (kata w0xf): between fork() and execve the child's /proc/<pid>/cmdline still shows the PARENT's argv, so the immediate post-spawn record captured the test-harness binary while the re-read observed [sleep 300]. Under current runner load the race loses 2/2. spawn_sleep_child now waits (bounded 2s) for /proc to show the execed sleep before returning, so every record/verify read observes post-exec identity.
… child execs Delta-round-2 review (Major): the test-side exec-wait I added earlier would have masked a production race. The author's evidence poll waited for a NON-EMPTY /proc/<pid>/cmdline, but between fork() and execve the child's cmdline mirrors the SPAWNER's argv — non-empty and wrong — so a record written mid-window persisted the server/test binary's argv as the sidecar identity and read as Mismatch at the next boot reconcile (CI receipt: spawn_record_carries_verifiable_proc_identity_* failed 2/2, LEFT was the test harness's own argv; kata w0xf). Production now owns the fix: accept evidence only once it stops matching our own argv (exec landed), and fail fast when the child vanishes pre-evidence instead of burning the 2s budget on a dead pid. The test helper reverts to immediate-after-spawn recording so the immediate-call contract stays covered.
… deadline Round-3 minors: (1) own_argv came from env::args(), which keeps empty args and panics on non-Unicode, while proc_cmdline drops empties and decodes lossily — read own argv through the same parser so the pre-exec mirror predicate compares identical representations (panic on freshcodex creation in weird-launch cases eliminated). (2) cap the poll sleep at the remaining budget and decline to sample past the deadline, so the 2s budget is a ceiling rather than a hint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two small, evidence-backed fixes:
e2e harness fix —
restore-contract-wall-rust.spec.ts'stwo-clients-same-sessionRef: duplicate respawn must yield exactly 1 PTYfailed deterministically (4 recorded runs) but never produced the 2-PTY signature it claimed to hunt. Mechanism: the spec runs file-widerecoveryOfferHandling: 'manual', and this test creates client B via rawbrowser.newContext(), bypassing the fixtures'installRecoveryOfferAutoDeclineOnContextwatcher. B is legitimately offered A's live session (designed behavior); the unanswered modal intercepts B's sidebar click forever (noactionTimeoutconfigured), the test burns its 300s budget, and dies before the server restart ever happens — argv log 1 entry, finally-masking hides it. The fix installs the documented shared adapter on contextB (canonical RESTORE-01 pattern per multi-client.spec.ts), running B through the real decline path. No assertion weakened; manual-mode panel-owner tests untouched.Infra fix —
docker/cloud-run/Dockerfile\' pinschmod 0755instead ofchmod +xfor/usr/local/bin/e2e-entrypoint.sh. That path sits outside thechown -R node:node /app, so a checkout whose umask narrowed the source file (observed 700 in the wild; git tracks only the x bit, so invisible) produces images that fail instantly with EACCES as thenode` user.Investigation receipts (brief premise falsified by evidence)
The run brief suspected a product regression in duplicate-respawn 'introduced by the interactive-create queue (PR #706)'. Twin explorer investigations plus experiments at both ends of the window established: (a) the queue landed in #705, and respawn creates (
restore: true) never travel it; #706's create-path diff is empty; the #706 label came from a mislabeled comment added by #708. (b) The one-PTY server machinery (D8 per-sessionRef single-flight, adoption, bounded redrive) is byte-identical across the window and converged in 12/12 runtime probes. (c) At pristine 3c52b2c the test fails with the identical pre-restart wedge signature; with this same one-hunk adapter it passes in 58.7s — the harness footgun predates the window, and no in-window product regression exists.Validation
SIGKILL-within-5s-of-pane-creation) passes 1/1npm testgreen at head: 4/4 cloud vitest shards + electron suite 432/432 (cloud image built under the gcloud-robot identity), plus the entrypoint-mode fix verified end-to-end by that buildFull recap:
.worktrees/.the-usual-logs/duplicate-respawn-single-pty/recap.md(validated). Follow-up hazard filed as kata y83s (finally-masking orphans the fixture server on test timeout).