[devswha]

This is the second commit of #2373, which merged at its first commit (7597f65) before this one reached the branch. It was reviewed there; it is cherry-picked onto current main here.

Found by the flake-hunt lane on #2380's Integration run (job 123700768179): server/take-over.contract.test.ts "consumes a click when the running ordinary retry succeeds, before later displacement" failed at :89, where pty-data contained "terminal attach taken over".

Cause (test flake)

This is the same cause as #2373: the 100 ms default attach hold. Displaced by another client's --takeover, herdr prints its teardown and "terminal attach taken over", then exits. The bridge keeps an attach's last line in the output tail for the hold (tailTimer in server/core/terminal-process.ts) and drops it at a displaced exit (flushTail(displaced)). An exit more than 100 ms after the line, as on a busy runner, let the line reach clients first.

Change

A 2 s hold for this test too, as #2373 gave the ordinary-failure test. The changelog fragment from #2373 now names both tests.

Proof

• Fail-first: with the fixture's real wrapper exiting 300 ms after herdr (scratch), the old test failed 3/3 with CI's not.toContain and the new one passed 3/3.
• Whole file: passed 9/9 three times. On current main with this cherry-pick it passed 9/9 again (bun run check run bun test server/take-over.contract.test.ts).
• Local checks: bun run check fast ok.

Test-only.

Reviews

• omo-native-code-reviewer: APPROVE, CLEAR (on test(take-over): hold a failing attach's last line until its exit on a busy runner #2373's branch). It confirmed the tail-hold mechanism and that the test still asserts the consumed click, the retry without --takeover, and no taken-over text.
  • LOW product note, not fixed here: production also holds the tail 100 ms, so on a loaded machine, when another client takes over, a user can briefly see herdr's teardown line until the next attach redraws. A longer tail hold for displacement would remove it.
• Test-only, so no Codex review (notes/pr-policy.md).

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 23 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: test/take-over-displacement-hold
• File: .coderabbit.yaml
• Commit: 6df94b3
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 054f6d1a-78fb-4d8f-a0cf-2287186db39e

Commits

Reviewing files that changed from the base of the PR and between 421689b and 6df94b3.

Files selected for processing: 2

• changelog.d/integration-flake-take-over-recovery.md
• server/take-over.contract.test.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[devswha]

Merging 6df94b3. Test-only: in server/take-over.contract.test.ts the displaced attach in the 'consumes a click when the running ordinary retry succeeds' case holds its "taken over" line well past its exit, as #2373 did for the ordinary-failure case, so a busy runner cannot pass the line on before the exit comes. Diff read by the merge pass; no CodeRabbit review needed.