Skip to content

test(session): fix shell-cancel race when trap hasn't installed yet#27408

Merged
kitlangton merged 1 commit into
devfrom
fix/shell-cancel-race-marker
May 14, 2026
Merged

test(session): fix shell-cancel race when trap hasn't installed yet#27408
kitlangton merged 1 commit into
devfrom
fix/shell-cancel-race-marker

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Summary

The `cancel persists aborted shell result when shell ignores TERM` test at `test/session/prompt.test.ts:1509` flakes on Linux CI because of a timing race:

  1. Test forks a shell running `trap '' TERM; sleep 30`.
  2. Test waits 50ms.
  3. Test calls `prompt.cancel(chat.id)` which fires `SIGTERM`.

On a slow runner, the SIGTERM can arrive before bash has parsed and installed the `trap` line, so the kill-escalation path the test is named after never gets exercised — and the resulting failure mode varies run-to-run.

Fix

Touch a marker file after trap installs, and poll for its existence before cancelling. The marker is a hard happens-before that the 50ms sleep was only approximating:

```
trap '' TERM; touch "${ready}"; sleep 30
```

POSIX guarantees the three commands run sequentially within bash, so once the marker exists the trap is installed.

Why polling beats a longer sleep

A bigger `Effect.sleep` would still be theoretically racy under CI load and would always burn its full duration. The poll typically resolves in 10–30ms (first or second tick) and is provably race-free.

Test plan

  • Single run passes locally
  • 5 consecutive local runs pass
  • Linux CI green on this PR

The "cancel persists aborted shell result when shell ignores TERM" test
relied on a 50ms sleep between forking the shell and calling cancel.
On a slow Linux CI runner, SIGTERM can arrive before bash has installed
the `trap '' TERM` line — exercising the wrong code path (or the right
one, intermittently) and flaking the test.

Touch a marker file after `trap` and poll for its existence before
cancelling. The poll is the hard happens-before that the 50ms sleep was
hoping for.
@kitlangton kitlangton enabled auto-merge (squash) May 14, 2026 01:03
@kitlangton kitlangton merged commit 3fc7486 into dev May 14, 2026
12 checks passed
@kitlangton kitlangton deleted the fix/shell-cancel-race-marker branch May 14, 2026 01:10
AIALRA-0 pushed a commit to AIALRA-0/opencode-turn-engine that referenced this pull request Jun 10, 2026
AIALRA-0 pushed a commit to AIALRA-0/opencode-turn-engine that referenced this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant