Classify provider quota errors as environment faults on opencode-http

[jackmcintyre]

Addresses #323. Doesn't close it: the four unseeded profiles and the zero-token timeout heuristic are still open, and so is the claude gap described below.

A provider usage-limit error wasn't recognised as an environment fault. The loop recorded env_fault: false, spent max_dev_attempts on the story, deferred it, and moved to the next story, which hit the same wall. One 5-hour quota window turned into three deferred stories and seven dead sessions, four of which consumed zero tokens. The specs were fine. The first story of the run completed and merged on the same adapter and model, it just happened to be the one that exhausted the quota.

The structural half

run() has always called _classify_env_fault for every adapter. The implementation lived on GenericAdapter, and OpencodeHttpAdapter is its sibling rather than its subclass, so the base identity no-op applied and the feature was absent. The base docstring justified that with "adapters with no post-mortem signal (HTTP/mock)", which stopped being true once opencode_http started teeing the serve process's stdout to logs/<task_id>.log. That's the exact file the classifier reads.

The classifier now lives in adapters/env_fault.EnvFaultMixin and both adapters mix it in. The signal is the log, not the transport, so any adapter that writes logs/<task_id>.log inherits it instead of re-omitting it. Patterns compile lazily off self.profile, so mixing the class in is the whole wiring step. There's no __init__ line to forget, which is how this got missed the first time.

One profile seeds patterns, on purpose

opencode.toml is anchored on the structured field the serve process emits, error.error="AI_APICallError: ...", and verified by replaying the shipped profile through the mixin over a real outage's logs. Four sessions classify, the healthy merged story's three don't, and the first hit lands on story 2's second attempt. That's where the run now pauses instead of marching into stories 3, 4 and 5.

codex, gemini, copilot and antigravity stay inert. I drafted patterns for all four and withdrew them. They could only be written from error strings scraped off public issue trackers, for CLIs I haven't run, and an unverified pattern isn't a neutral bet. The tmux adapters match against a pane capture that contains the model's own output, so a pattern that fires on a story which merely implements rate limiting pauses a healthy run. That's worse than the miss, because the miss only reproduces current behaviour. An adversarial pass demonstrated it on realistic fixture and assertion text before any of them shipped.

I also drafted a quota pattern for claude.toml and withdrew it for the same reason, so that profile is untouched. An Anthropic plan's usage limit is still unclassified on that adapter. Its existing connection pattern already matches ordinary prose about provider errors, including this repo's own CHANGELOG and docs, so a bmad-loop session working on bmad-loop is exposed. That's pre-existing #194 debt. It's now pinned by an xfail(strict) test rather than left undocumented, and fixing it needs a captured Claude Code quota line plus a stronger anchor than "API Error".

Which file gets scanned is part of the contract

Rebasing onto current main turned up a fourth defect, and it's the one worth reading.

The readable-logs work repurposed opencode's logs/<task_id>.log. It's now the curated [bmad] conversation transcript written by the SSE reader, and the server's own stdout moved to <task_id>.server.out. The classifier was still scanning .log, which breaks two things at once. The provider's AI_APICallError lines aren't there any more, so nothing would ever classify. And the patterns are only sound against a log the model can't write to, so pointing them at a transcript of the model talking would have made the verbatim-citation false positives live.

EnvFaultMixin now has ENV_FAULT_LOG_SUFFIX, and the opencode adapter overrides it to .server.out. The safety property is documented at the override, because it's a property of the file, not of the patterns.

None of the unit tests caught this. They write their fixture to whatever path the code resolves, so they agree with any answer. The only thing that caught it was the end-to-end test through run(), which exists because a reviewer flagged its absence as the lowest-severity finding in the whole review.

Also fixed in the classifier

Three more defects, all found by adversarial review rather than by me.

The 64 KiB tail seek lands on an arbitrary byte, so the line straddling the window edge arrived as a fragment whose head could be a U+FFFD from the errors="replace" decode. That fragment is discarded now, unless discarding it would leave nothing to scan.

_excerpt marked a dropped prefix but silently dropped a suffix, so a truncated excerpt read as a complete line that ended there. Both ends are marked, and the markers are spent from the evidence budget rather than added on top of it.

The evidence window is centred on the match and slides left when the match sits near the end of the line. opencode's logfmt puts around 250 characters of metadata before error.error=, so before this the operator saw every field except the failure. The quota evidence now reads through to the reset timestamp.

Tests

Corpora are split three ways: lines that must classify (captured from the outage), lines that must not (realistic healthy-session output), and lines that pin the limit of the approach. A story quoting a provider error verbatim produces a line byte-identical to the emitted one, so it matches, and no content-based pattern can separate them. That's tolerable for opencode only because the file it scans is <task_id>.server.out, which the model can't write to. It's asserted positively so it can't change quietly, and that assertion is the one that will fire if the scanned file is ever repointed at something carrying model output again.

Reachability of the bait is asserted too. That corpus was briefly vacuous, guarding nothing while looking thorough, after the profile it had been written for was withdrawn.

Correlation ids in the corpora are fakes. The run they came from is a private project.

Verification

3323 passed, 42 skipped, 5 xfailed. The xfails are all the claude debt test. ruff, black, isort and pyright clean.

The replay over the original outage logs was re-run after the rebase, with the log suffix overridden, because that run predates the .log / .server.out split and its captured bytes were written when .log was still the server sink. Same four sessions classify, the healthy story's three still don't.

Summary by CodeRabbit

• New Features
  • Improved detection of provider quota, rate-limit, and connection/transport environment faults.
  • OpenCode HTTP sessions now analyze captured server logs to determine whether a timeout is fault-related.
  • Fault results now include concise evidence excerpts for easier diagnosis.
• Bug Fixes
  • Avoids misclassifying healthy timeouts and completed sessions as environment faults.
  • Prevents reuse of stale per-task log output when re-running with the same task id.
  • More robust evidence handling for truncated or missing logs.
• Tests
  • Added and expanded pattern and evidence extraction coverage, including OpenCode end-to-end regressions.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 45 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 Plus

Run ID: 8fecea21-1676-4b5c-ac08-ca87a5ec9de2

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6a3d6 and 64efb7e.

📒 Files selected for processing (10)

• src/bmad_loop/adapters/base.py
• src/bmad_loop/adapters/env_fault.py
• src/bmad_loop/adapters/generic.py
• src/bmad_loop/adapters/opencode_http.py
• src/bmad_loop/adapters/profile.py
• src/bmad_loop/data/profiles/opencode.toml
• tests/test_env_fault_patterns.py
• tests/test_generic_tmux.py
• tests/test_opencode_http.py
• tests/test_profile.py

Walkthrough

A shared EnvFaultMixin classifies eligible failed sessions from bounded adapter logs. Generic and opencode HTTP adapters use it, with opencode scanning resettable server output logs. Profile patterns, evidence extraction, and runtime behavior receive expanded test coverage.

Changes

Environment fault classification

Layer / File(s)	Summary
Shared fault classifier src/bmad_loop/adapters/env_fault.py	Adds bounded log-tail scanning, ANSI stripping, timed regex matching, evidence excerpts, lifecycle breadcrumbs, and SessionResult updates.
Adapter integration src/bmad_loop/adapters/generic.py, src/bmad_loop/adapters/opencode_http.py	Moves classification into EnvFaultMixin, preserves compatibility exports, and configures opencode to scan and reset .server.out logs.
Profile configuration and contract documentation src/bmad_loop/adapters/base.py, src/bmad_loop/adapters/profile.py, src/bmad_loop/data/profiles/opencode.toml, tests/test_profile.py	Documents matching semantics and adds anchored quota/connectivity patterns for opencode profiles.
Pattern validation tests/test_env_fault_patterns.py	Adds corpus tests for valid outages, healthy output, anchors, bare causes, known false positives, and unseeded profiles.
Runtime and evidence validation tests/test_generic_tmux.py, tests/test_opencode_http.py	Tests truncation edge cases, inert classification, server-log evidence, lifecycle events, stale-log cleanup, and end-to-end behavior.

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

Sequence Diagram(s)

sequenceDiagram
  participant Session as Adapter session
  participant Opencode as OpencodeHttpAdapter
  participant ServerLog as task.server.out
  participant Classifier as EnvFaultMixin
  participant Result as SessionResult
  Session->>Opencode: start or re-arm task
  Opencode->>ServerLog: unlink stale server log
  Opencode->>ServerLog: capture server output
  Session->>Classifier: classify non-completed result
  Classifier->>ServerLog: read and scan log tail
  ServerLog-->>Classifier: matched fault evidence
  Classifier->>Result: set env_fault and evidence

Loading

Possibly related PRs

• bmad-code-org/bmad-loop#164: Related GenericAdapter mixin and inheritance changes.
• bmad-code-org/bmad-loop#165: Earlier OpencodeHttpAdapter session lifecycle work used by the server-log integration.
• bmad-code-org/bmad-loop#272: Related environment-fault classification through adapter profiles and evidence extraction.

Suggested reviewers: pbean

Poem

A rabbit found an error in the log,
And bounded it neatly—no endless slog.
Quotas and sockets leave a trace,
While healthy runs hop into place.
“Env fault!” sings the bunny with glee.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.05% 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 accurately reflects the main user-visible change: opencode-http now classifies provider quota errors as environment faults.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/test_generic_tmux.py (1)

4345-4363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This test passes with the straddler drop removed.

The 72 KB of padding pushes the matching line entirely outside the 64 KiB window, so nothing is scanned that could match — the assertion holds whether or not lines[1:] runs. Sizing the padding just under ENV_FAULT_TAIL_BYTES so the seek cuts through the matching line would actually pin the drop (and the U+FFFD-head concern the docstring cites).

♻️ Sketch: put the straddler at the window edge

-    straddler = b"API Error: Connection refused STRADDLER\n"
-    pad = b"filler line\n" * 6000  # comfortably over ENV_FAULT_TAIL_BYTES
-    _write_task_log(adapter, straddler + pad)
+    straddler = b"API Error: Connection refused STRADDLER\n"
+    # Land the seek INSIDE the straddler: everything after it is exactly
+    # ENV_FAULT_TAIL_BYTES minus half the line, so the window opens mid-match.
+    pad = b"filler line\n" * ((generic.ENV_FAULT_TAIL_BYTES - len(straddler) // 2) // 12)
+    _write_task_log(adapter, b"lead\n" + straddler + pad)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_generic_tmux.py` around lines 4345 - 4363, Update
test_classify_env_fault_drops_the_partial_line_at_the_tail_seek so the matching
straddler crosses the ENV_FAULT_TAIL_BYTES read boundary rather than being
entirely before it. Size or position the padding to place the seek in the middle
of straddler, then retain assertions that the partial matching line is discarded
and no env-fault evidence is produced.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/bmad_loop/adapters/opencode_http.py`:
- Around line 370-378: Update OpencodeHttpAdapter.start_session to unlink the
existing <task_id>.server.out before invoking _spawn_server, while preserving
the append behavior across port-collision retries within one spawn. Keep the
existing result.json cleanup and ensure the reset occurs at the start of each
new session cycle.

In `@tests/test_opencode_http.py`:
- Around line 2507-2510: Update the test docstring describing the fake server’s
output file to reference the asserted <task_id>.server.out filename instead of
logs/<task_id>.log. Keep the rest of the end-to-end session behavior description
unchanged.

---

Nitpick comments:
In `@tests/test_generic_tmux.py`:
- Around line 4345-4363: Update
test_classify_env_fault_drops_the_partial_line_at_the_tail_seek so the matching
straddler crosses the ENV_FAULT_TAIL_BYTES read boundary rather than being
entirely before it. Size or position the padding to place the seek in the middle
of straddler, then retain assertions that the partial matching line is discarded
and no env-fault evidence is produced.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 68dafd8f-defd-47e3-9285-173499d50ee5

📥 Commits

Reviewing files that changed from the base of the PR and between fb9aafa and 9d0458b.

📒 Files selected for processing (10)

• src/bmad_loop/adapters/base.py
• src/bmad_loop/adapters/env_fault.py
• src/bmad_loop/adapters/generic.py
• src/bmad_loop/adapters/opencode_http.py
• src/bmad_loop/adapters/profile.py
• src/bmad_loop/data/profiles/opencode.toml
• tests/test_env_fault_patterns.py
• tests/test_generic_tmux.py
• tests/test_opencode_http.py
• tests/test_profile.py

[greptile-apps]

Greptile Summary

This PR fixes a missing environment-fault classification on the opencode-http adapter that caused a 5-hour provider quota outage to burn three stories' retry budgets instead of pausing the run. The classifier was implemented on GenericAdapter rather than shared, so its HTTP sibling silently omitted it despite writing the exact log file the classifier reads.

• Extracts _classify_env_fault and _env_fault_evidence into a new EnvFaultMixin (along with bug fixes: truncated-head fragment drop, match-centred windowing, and both-end ellipsis markers), and mixes it into both adapters with a per-adapter ENV_FAULT_LOG_SUFFIX so each transport scans the right file.
• Seeds two env_fault_patterns in opencode.toml anchored on the server's structured error.error="AI_APICallError: …" logfmt field, verified against captured outage logs; adds a start_session unlink so re-armed runs can't classify off a stale prior-cycle error and loop forever.
• Adds comprehensive corpus-driven tests covering false-positives (BAIT), false-negatives (REAL_BY_PROFILE), anchor-reaching bait, verbatim-citation characterisation, and an end-to-end run() regression that pins the scan to .server.out rather than .log.

Confidence Score: 5/5

Safe to merge — the change is additive (a new mixin + a new log unlink) with no modifications to the core run() control flow or result handling, and is guarded by a thorough test suite including an end-to-end regression through run().

The mixin's guard conditions (status membership, result_json check) are unchanged from the pre-existing GenericAdapter implementation and protect the rest of the result path. The stale-log unlink parallels the existing pattern in GenericAdapter.start_session and cannot corrupt a live session. The env_fault_patterns are narrowly anchored on a server-owned structured field and have been verified against a corpus of realistic bait lines. No regressions were found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/bmad_loop/adapters/env_fault.py	New EnvFaultMixin extracted from GenericAdapter; adds truncated-head fragment drop, both-end ellipsis markers, match-centred windowing, and ENV_FAULT_LOG_SUFFIX class variable so each transport scans the right file.
src/bmad_loop/adapters/opencode_http.py	Mixes in EnvFaultMixin, overrides ENV_FAULT_LOG_SUFFIX to ".server.out", and unlinks the server log at session start to prevent stale-log false positives on re-armed runs.
src/bmad_loop/adapters/generic.py	Removes the inline classify/evidence implementation (now in EnvFaultMixin) and re-exports the moved constants for backward compat; GenericAdapter now inherits from EnvFaultMixin.
src/bmad_loop/data/profiles/opencode.toml	Seeds two env_fault_patterns anchored on error.error="AI_APICallError: …" — one for quota/rate-limit refusals, one for socket-drop connection failures — verified against a captured 5-hour outage.
tests/test_env_fault_patterns.py	New corpus-driven test file covering false-positive (BAIT), anchor-reaching bait, verbatim-citation characterisation, false-negative (REAL_BY_PROFILE), and unseeded-profile inertness; also pins claude's known false-positive as xfail(strict).
tests/test_opencode_http.py	Adds eight new env-fault tests including an end-to-end run() regression that pins the scan to .server.out rather than .log, and a stale-log unlink test that drives a real session.
tests/test_generic_tmux.py	Adds three new tests: dropped-suffix ellipsis, partial-line fragment drop on tail seek, and no-newline degenerate window; also hardens the inert-without-patterns test to build an empty profile rather than borrow a shipped profile.

_{Reviews (3): Last reviewed commit: "fix(adapters,profiles): classify provide..." | Re-trigger Greptile}

[jackmcintyre]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_generic_tmux.py`:
- Around line 4351-4355: Adjust the test data setup around straddler and pad so
the 64 KiB tail-read boundary falls within the straddler line itself, rather
than after it in the padding. Preserve the scenario where no other matching line
exists, ensuring the test fails if partial lines at the tail boundary are
scanned.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: beb32dbc-4822-4ed3-aa68-9292eed2cbd3

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0458b and 9c6a3d6.

📒 Files selected for processing (10)

• src/bmad_loop/adapters/base.py
• src/bmad_loop/adapters/env_fault.py
• src/bmad_loop/adapters/generic.py
• src/bmad_loop/adapters/opencode_http.py
• src/bmad_loop/adapters/profile.py
• src/bmad_loop/data/profiles/opencode.toml
• tests/test_env_fault_patterns.py
• tests/test_generic_tmux.py
• tests/test_opencode_http.py
• tests/test_profile.py

🚧 Files skipped from review as they are similar to previous changes (6)

• src/bmad_loop/adapters/profile.py
• src/bmad_loop/data/profiles/opencode.toml
• tests/test_profile.py
• tests/test_opencode_http.py
• src/bmad_loop/adapters/base.py
• src/bmad_loop/adapters/generic.py