Skip to content

fix: replace opencode wall-clock timeout with idle timeout#830

Open
agent-p1p wants to merge 2 commits into
masterfrom
pip/mdk-829
Open

fix: replace opencode wall-clock timeout with idle timeout#830
agent-p1p wants to merge 2 commits into
masterfrom
pip/mdk-829

Conversation

@agent-p1p

@agent-p1p agent-p1p commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #829

Summary

  • Replace the fixed five-minute stdout timeout with a 120-second idle timeout reset by every OpenCode stdout line.
  • Retain a one-hour total safety cap across streaming, child exit, and stderr capture.
  • Preserve newly observed OpenCode session IDs on idle, total-timeout, and stream failures without overwriting established sessions.
  • Update environment defaults, installer output, documentation, and deterministic process fixtures.

Verification

  • cargo test -p wn-opencode --locked (40 passed, 1 ignored)
  • 10 consecutive full wn-opencode test-suite runs passed
  • cargo clippy -p wn-opencode --all-targets --locked -- -D warnings
  • just fast-ci
  • integrations/opencode/marmot/tests/test_installer.sh
  • shell syntax checks for installer and mock fixture
  • independent pre-commit review: passed

Sensitive paths

None.


Open in Stage

Summary by CodeRabbit

  • New Features

    • Added configurable idle detection for OpenCode runs, stopping executions that produce no output for the specified period.
    • Extended the overall execution safety limit to support longer-running tasks while ongoing output continues.
    • Improved session preservation when runs fail or time out.
  • Bug Fixes

    • Ensured processes are properly cleaned up after timeout or stream failures.
    • Preserved sessions when output backpressure causes a run to fail.

Preserve observed sessions on timeout and keep a one-hour total safety cap.

Fixes #829
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@agent-p1p, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a366628d-6cec-49d7-b03b-0577c7ce538d

📥 Commits

Reviewing files that changed from the base of the PR and between a276706 and d733ea8.

📒 Files selected for processing (4)
  • integrations/opencode/marmot/Cargo.toml
  • integrations/opencode/marmot/src/bridge.rs
  • integrations/opencode/marmot/src/opencode.rs
  • integrations/opencode/marmot/tests/fixtures/mock-opencode.sh

Walkthrough

OpenCode now supports separate idle and total timeouts, absolute lifecycle deadlines, explicit failed-process cleanup, and structured failures carrying observed sessions. Configuration and installer outputs propagate the new settings, while the bridge persists sessions on failed invocations.

Changes

OpenCode timeout and session recovery

Layer / File(s) Summary
Timeout configuration and propagation
integrations/opencode/marmot/src/config.rs, integrations/opencode/marmot/README.md, scripts/install-opencode-marmot.sh, integrations/opencode/marmot/tests/test_installer.sh
Adds a 120-second idle timeout, raises the total timeout default to 3600 seconds, parses the new environment variable, documents both settings, and propagates them through generated service configurations and installer tests.
Runner timeout lifecycle
integrations/opencode/marmot/src/opencode.rs, integrations/opencode/marmot/src/error.rs, integrations/opencode/marmot/tests/fixtures/mock-opencode.sh
Enforces idle and absolute total deadlines, cleans up failed child processes, returns observed sessions with failures, adds OpencodeIdle, and tests streaming, timeout, child-wait, and backpressure scenarios.
Bridge failure and session persistence
integrations/opencode/marmot/src/bridge.rs
Passes idle timeout configuration into invocations, handles structured runner failures, and persists observed sessions only when no session is already set, including on failure paths.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change from wall-clock to idle timeout handling.
Linked Issues check ✅ Passed The PR implements the requested idle timeout, total cap, session persistence, error variant, docs, and tests for #829.
Out of Scope Changes check ✅ Passed The README, installer, fixture, and test updates all support the timeout refactor and stay within #829's scope.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pip/mdk-829

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

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

@stage-review

stage-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 5 individual chapters for you:

Title
1 Define idle timeout error and configuration
2 Implement idle timeout and session preservation
3 Update bridge to handle idle failures
4 Add tests for idle timeout logic
5 Update installer and documentation
Open in Stage

Chapters generated by Stage for commit d733ea8 on Jul 12, 2026 6:04pm UTC.

@agent-p1p

Copy link
Copy Markdown
Contributor Author

Adversarial review

Verdict: changes required — 1 blocking finding, 2 suggestions.

Blocking

  1. integrations/opencode/marmot/src/opencode.rs:134-138,185-195 — stdout EOF bypasses the idle timeout and can wedge a group for the full one-hour cap.

    Ok(Ok(None)) breaks out of the idle-timed read loop. The subsequent child.wait() and stderr join remain protected only by timeout_at(total_deadline, ...). If OpenCode closes stdout but remains alive, it is permanently unable to produce another stdout line, yet the default behavior waits up to 3600 seconds instead of declaring the invocation idle after 120 seconds. handle_message holds the per-group serial lock for this whole period.

    The new run_total_cap_includes_child_wait_after_stdout_closes test currently codifies this bug by making the total deadline shorter than the idle deadline and expecting OpencodeTimedOut. Please keep idle detection active through the post-EOF child wait (while retaining the total cap), and add the converse regression: idle deadline shorter than total, stdout closes while the child stays alive, expect OpencodeIdle promptly.

Suggestions

  1. opencode.rs:440-451 — make the central streaming regression fail under the old wall-clock implementation. The fixture runs for about 750ms while mock_invocation gives it a 10s total timeout, so the test would also pass before this PR. Prefer paused Tokio time and exercise output beyond the old 300s wall clock but below the new 3600s cap, with every line less than 120s apart.

  2. bridge.rs:406-454,870-907 — cover first-prompt idle persistence through the bridge error path. The current bridge test obtains a backpressure-induced total timeout and then invokes persist_observed_session_if_unset directly. It does not prove that the handle_message idle-error arm persists the observed session or emits the distinct idle message. A focused helper-level test is sufficient; no broad harness abstraction needed.

Out-of-scope follow-ups

Verification

  • cargo test -p wn-opencode --locked: 40 passed, 1 ignored.
  • cargo clippy -p wn-opencode --all-targets --locked -- -D warnings: passed.
  • cargo fmt --all --check: passed.
  • Installer test and shell syntax checks: passed.
  • Commit signature and agent-p1p attribution verified.
  • Sensitive paths touched: none.

Preserve the current idle budget through EOF, child exit, and stderr capture, and cover the bridge failure path and delayed-EOF lifecycle.
@agent-p1p

Copy link
Copy Markdown
Contributor Author

Addressed the blocking EOF lifecycle finding in d733ea8.

  • The idle deadline now remains active after stdout EOF through both child exit and stderr capture; EOF does not reset the remaining budget.
  • Each actual stdout line still starts a fresh idle interval.
  • Added regressions for a live child after EOF, delayed EOF near the idle deadline, long-running output beyond the old wall-clock cap, and run-failure session persistence/reply selection.

Verification:

  • cargo test -p wn-opencode --locked — 42 passed, 1 ignored
  • cargo clippy -p wn-opencode --all-targets --locked -- -D warnings
  • cargo fmt --all --check
  • fixture bash -n
  • independent pre-commit review — passed

A repository-wide just fast-ci attempt reached the workspace OTLP clippy step, then the host ran out of disk; focused crate checks above are green and GitHub CI is running on the pushed commit.

@agent-p1p
agent-p1p marked this pull request as ready for review July 12, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wn-opencode: replace wall-clock timeout with idle timeout

1 participant