[luvs01]

A frozen install that exits 124 or 137 on its own currently looks like a timeout and can be retried with --force. This change retries only when the helper's own deadline expires, and waits for its original process group to finish before reporting success or starting another attempt.

The installer exit status is carried as data through a fixed reader; GNU timeout's status belongs to that reader, not the installer. A group-local keeper owns TERM/grace/KILL cleanup. Bootstrap FIFO names are removed before installation starts, and installer processes do not inherit the control or result descriptors. Caller cancellation takes precedence over an already available result.

The 45-second attempt default, shared 150-second root/GUI budget, three-attempt maximum, and timeout-only --force retry remain. Cleanup time is reserved within the shared budget. The script retains Bash 3.2-compatible syntax. This contract covers the original process group, not descendants that deliberately detach or hostile same-user process inspection.

Validation:

• Windows/MSYS: 10 cases executed against the final integrated script, including raw 124/137 exits, cancellation boundaries, and direct-pipe completion. The configured 2-second grace measured 2,330 ms through completion; direct-pipe cases observed both output EOFs, child close, and the original group absent.
• bun check --no-script --noEmit: 2,243 files, zero errors.
• Windows CI-workflow test: 1 passed, 21 POSIX cases skipped; this is not a POSIX test pass.
• Test layout, standard ratchet, structure/privacy checks, Bash syntax, TypeScript parsing, and diff checks passed.

Linux Cross-platform CI passed on a9a073dc: all four test shards and gates succeeded. The test 4/4 log confirms all 21 POSIX cases and the workflow assertion executed and passed, including independent exit 124/137/SIGKILL, deadline grace, parent cancellation, process-group draining, and the real Bun postinstall retry.

Follow-up 78fca144 probes timeout --foreground, matching the keeper's actual invocation, before any installer starts. New regression cases cover rejection of unsupported candidates and fallback to compatible gtimeout. Isolated Windows/MSYS checks observed exit 69 with no install call for unsupported candidates and one successful install through the fallback. The direct pinned typecheck passed; latest-head Linux CI is pending. The Linux results above are for a9a073dc.

Native macOS/Bash 3.2 execution remains unverified. No workflow runner, permissions, or job timeout definitions change.

Summary by CodeRabbit

• Chores
  • CI dependency installs now retry only when an attempt reaches its deadline. Setup and other failures stop without retrying, while cancellation returns the status associated with the cancellation signal.
  • Each attempt is bounded by its deadline and the remaining overall time budget.
• Tests
  • Expanded coverage for retry limits, cancellation, failure handling, and cleanup of spawned processes.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: dev
• File: .coderabbit.yaml
• Commit: 3ea8965
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 74360528-a67e-41f4-a370-3347d51b5e19

Commits

Reviewing files that changed from the base of the PR and between a9a073d and 78fca14.

Files selected for processing: 2

• scripts/ci/bun-install-frozen.sh
• tests/ci-workflows/ci-bun-install-frozen.test.ts

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

Walkthrough

The installer now uses a keeper and FIFO protocol to bound Bun install attempts, report outcomes, and clean up process groups. It retries only explicit deadline outcomes and applies a shared total budget.

Changes: Bun install attempts

Layer / File(s)	Summary
Keeper protocol and timeout selection scripts/ci/bun-install-frozen.sh	The script validates deadline settings and selects timeout or gtimeout after a GNU-options probe. A keeper reports install outcomes and cleans up its process group.
Attempt coordination and process cleanup scripts/ci/bun-install-frozen.sh, tests/ci-workflows/ci-bun-install-frozen.test.ts	run_attempt coordinates FIFO handshakes, per-attempt deadlines, cancellation statuses, and process-group draining. Tests cover interruption, cleanup, descriptor isolation, and keeper failure.
Total budget and retry decisions scripts/ci/bun-install-frozen.sh, tests/ci-workflows/ci-bun-install-frozen.test.ts	The retry loop reserves time for cleanup and retries only explicit deadline outcomes. Tests cover failure classification, deadline retries, attempt exhaustion, and the shared root/GUI budget.

Priority: Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CI_script
  participant run_attempt
  participant keeper
  participant GNU_timeout
  participant Bun_install
  CI_script->>run_attempt: Start bounded attempt
  run_attempt->>keeper: Establish FIFO handshake and authorize deadline
  keeper->>GNU_timeout: Bound install status reporting
  GNU_timeout->>Bun_install: Run install command
  GNU_timeout-->>keeper: Return install status or deadline
  keeper->>Bun_install: Terminate process group after attempt
  run_attempt-->>CI_script: Return attempt verdict

Loading

Possibly related PRs

• lidge-jun/opencodex#7870: Introduced the installer script and original retry-policy tests, which this PR revises.
• lidge-jun/opencodex#7900: Changed retry command construction in the same script for Bash 3.2 with set -u.
• lidge-jun/opencodex#7898: Added --force to timeout retries in the same script, which this PR retains for later attempts.

Merge Risk: Minimal · up to 78fca

No merge-blocking issue was identified. Native macOS and Bash 3.2 execution remain unverified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "fix(ci): distinguish install failures from owned deadlines" names the main change. The PR retries an installer attempt only when the helper's own deadline expires, not when the installer ex…
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.

Full details: Title check

Explanation

The title "fix(ci): distinguish install failures from owned deadlines" names the main change. The PR retries an installer attempt only when the helper's own deadline expires, not when the installer exits with status 124 or 137. The raw summary and objectives both describe this retry-policy change in scripts/ci/bun-install-frozen.sh and its test file. The title is specific, short, and free of file lists or vague terms.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• 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.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

Additional comments: 1

scripts/ci/bun-install-frozen.sh-82-82: 1

82-82: Minor · Functional Correctness · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Make the timeout probe check the flag that the keeper actually uses.

The probe on Line 82 checks --signal=TERM --kill-after=1s. The script no longer passes either option. The keeper on Line 49 runs "$timer" --foreground "${deadline}s" ..., and the probe never tests --foreground.

Failure mode:

• A timeout can accept --signal and --kill-after but reject --foreground. Some non-GNU or older builds do this.
• In that case the probe passes and selects that binary.
• The timeout command on Line 49 then exits immediately with a usage error, for example 125.
• The < <(report_install "$@") process substitution still starts, so bun install runs.
• The keeper sees a non-zero rc that is not 124. It writes fail 70 and runs kill -KILL 0, which kills the install while it runs.
• install_dir prints failed in ... (exit 70, fail); not retrying. The Line 86 message (GNU timeout is required) never appears, so the failure gives no useful hint.

Fix: probe the exact options and duration form used on Line 49. The scripts instructions require "actionable failures", and this mismatch hides the real cause.

🛠️ Proposed fix

-  if command -v "$candidate" >/dev/null 2>&1 && "$candidate" --signal=TERM --kill-after=1s 1s true >/dev/null 2>&1; then
+  if command -v "$candidate" >/dev/null 2>&1 && "$candidate" --foreground 1s true >/dev/null 2>&1; then

Source: Path instructions

---

Review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: dev
• File: .coderabbit.yaml
• Commit: 3ea8965
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 2a8935fc-5ea1-4e8c-929f-bece7b831bf5

Commits

Reviewing files that changed from the base of the PR and between 3ea8965 and a9a073d.

Files selected for processing: 2

• scripts/ci/bun-install-frozen.sh
• tests/ci-workflows/ci-bun-install-frozen.test.ts

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

[lidge-jun]

Maintainer integration (lane L1, cross-platform CI). Squash-merging under the maintainer-integration exception in MAINTAINERS.md.

Exact head: 78fca1448351ed698e9ba4a922e393d93b9806e6
Exact-head CI: CI run 41013964408 (pull_request, headSha 78fca14…, conclusion success): gates, test 1–4/4, storage policy, api usage, ci all success. The test 4/4 log executed tests/ci-workflows/ci-bun-install-frozen.test.ts with its POSIX cases (32 result lines, no skips on Linux). enforce-target, hygiene, CodeRabbit: pass.

Independent review + security review (dependency installation boundary): a fresh reviewer separate from the author checked git diff origin/dev...78fca144.

• Correctness: no defect found. The installer exit status is carried as data, so a real bun install exit of 124/137 is no longer read as a deadline. --force is added only after the helper's own deadline expires, and the original process group is drained before the next attempt.
• Security: --frozen-lockfile is kept on every attempt, arguments are passed as quoted array elements, there is no new interpolation or command injection, nothing new reaches the logs, and workflow triggers, permissions and job timeouts are unchanged. No vulnerability found.
• Residual (coverage, not security): native macOS/Bash 3.2 execution remains unverified, and the Windows-local suite skips the POSIX cases. The batched lane=all dispatch on dev after this lands will exercise macOS.
• Local checks: bash -n scripts/ci/bun-install-frozen.sh exit 0; Windows focused run 1 pass / 23 skip / 0 fail.

Union check against current dev: two files only, no test-layout, ratchet or count changes, and git merge-tree is clean.