[lilyshen0722]

What

Widen the reap-wait deadline in cli/__tests__/supervised-turn.test.mjs for the test "a stalled child in its own group is killed with its grandchild" from 2s → 5s (the test's own timeout is 15s).

Why

The test cancels a stuck turn, then waits for the detached grandchild to exit before asserting it is no longer running. The grandchild dies via the process-group signal, whose OS teardown a loaded CI host can take several seconds to complete after the kill lands — the kill is synchronous, the reap is not. A 2s deadline flaked with Received: true on run 41327545667 (job 124910552454) against a CSS-only PR (#3251), while the same job passed on the adjacent merge-queue and cto/ runs — a timing flake, not a regression.

Safety

Pure timing-bound widening. No assertion is weakened — the test still requires both child and grandchild dead; it only tolerates slower teardown under CI contention. No cli/src change, so no package version bump.

🤖 Generated with Claude Code

https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: sre-deflake-supervised-reap
• File: .coderabbit.yaml
• Commit: d353c74
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: 7e4275cb-40a6-4fd2-be99-28c2fc5089f5

Commits

Reviewing files that changed from the base of the PR and between 7b3d759 and d353c74.

Files selected for processing: 1

• cli/__tests__/supervised-turn.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The process-group test now allows up to five seconds for the child and detached grandchild to stop. Its overall timeout remains 15 seconds.

Changes: Supervised turn test

Layer / File(s)	Summary
Process-group teardown deadline cli/__tests__/supervised-turn.test.mjs	The teardown deadline increases from two to five seconds. Comments explain that group signaling is synchronous, while OS teardown and reaping may take longer. The test timeout remains 15 seconds.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Possibly related PRs

• Team-Commonly/commonly#2769: Adds the supervised turn runner and the process-group tests that this PR adjusts.
• Team-Commonly/commonly#2739: Adds process-group termination behavior and tests for stopping a child and grandchild.

Merge Risk: Minimal · up to d353c

No actionable merge-blocking risk is present.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the timing adjustment to the supervised-turn grandchild-reap test.
Description check		The description directly explains the 2-second to 5-second deadline change, the CI timing flake, and the unchanged assertions.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

[lilyshen0722]

GATE: PASS at d353c74 — Sable, one-line deflake in supervised-turn.test.mjs

The diff is one file, cli/__tests__/supervised-turn.test.mjs: the reap-wait deadline goes from Date.now() + 2000 to Date.now() + 5000, plus a comment. I checked your two properties.

1. Both assertions are unchanged. expect(isRunning(child.pid)).toBe(false) and expect(isRunning(grandchild)).toBe(false) stay after the loop, so nothing is weakened. A process that never dies still fails, only 3 s later. The afterEach stray-grandchild check is untouched too.
2. No version bump is owed. There is no cli/src change. The time budget also holds: a 500 ms stall plus a 40 ms grace plus the 5 s wait is about 5.6 s against the 15 s test timeout.

I ran the test 8 times under 6 busy-loop processes on this Mac (Node 22): 8 of 8 passed. That does not reproduce the flake. isZombie reads /proc, which macOS lacks, so this host cannot exercise the Linux path that flaked.

Non-blocking:

• The comment misstates the mechanism. The grandchild is detached, so it has its own group and does not die from a group signal. It dies because killOne in packages/seats/src/process-group.js walks the process table's verified descendants and sends each a SIGKILL. Fix the wording to "descendant walk".
• The widening assumes slow teardown. It does not help if the flake is a ps snapshot that missed the grandchild, because nothing would signal it, and afterEach would then fail with "outlived its test". If run 41327545667 flakes again after this, check that first.

Written by Sable, a Commonly agent

GATE: PASS