fix(anthropic): surface upstream error bodies through sidecar bridges - #1020
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a bug-stack campaign with issue and PR triage, repair plans, dispositions, and Anthropic 400-error research. It also implements Anthropic error-body formatting, secret redaction, adapter registration, and bridge and persistence regression tests. ChangesBug stack campaign
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WebSearchBridge
participant AnthropicAdapter
participant ClientResponse
participant RequestLog
WebSearchBridge->>AnthropicAdapter: pass Anthropic error response body
AnthropicAdapter->>AnthropicAdapter: parse, redact, and truncate JSON error message
AnthropicAdapter->>ClientResponse: return formatted provider error with HTTP status
AnthropicAdapter->>RequestLog: persist formatted upstreamError
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/000_plan.md`:
- Around line 5-17: Mark August 5 activity as planned rather than completed:
update devlog/_plan/260805_bug_stack_campaign/000_plan.md at lines 5-19 and 52,
001_issue_triage.md at line 1, 002_pr_triage.md at line 1, and
003_anthropic_400_research.md at lines 5-8 and 21 to use planned language or
actual collection dates, while preserving the historical August 1 and August 4
dates in 003_anthropic_400_research.md.
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md:1 requires no change.
In `@devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md`:
- Around line 40-41: Convert the issue-ID lines at
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md lines 40-41 into list
items by prefixing each continuation with “-” or using inline code. Also convert
the feature-PR line at devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md
line 37 and stale-PR line at
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md line 42 into list
items, without changing their identifiers or surrounding content.
In `@devlog/_plan/260805_bug_stack_campaign/020_phase2_anthropic_400_fix.md`:
- Around line 38-54: Renumber the ordered list under the Tests section so it
starts at 1 and continues sequentially through 4, including the items beginning
with “Unit,” “Bridge integration,” “Persistence integration,” and “Pin the
read-failure branch.”
In
`@devlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.md`:
- Around line 44-50: The synthesis policy must be type-specific and
terminal-state-aware: define the reasoning closure sequence using
response.reasoning_summary_text.done, response.reasoning_summary_part.done, and
response.output_item.done with the completed summary; add distinct cleanup
behavior for response.completed, response.failed, response.incomplete, EOF, and
cancellation. Ensure completed terminals close open items before
response.completed, while failed and incomplete terminals preserve partial items
without marking them successful, and release repair state plus retained budget
on every terminal path.
- Around line 50-52: Update the snapshot fallback handling described in the
repair plan so malformed or contradictory upstream snapshots that produce
output: [] are not reported as response.completed. Ensure the Codex lifecycle
consumer emits response.incomplete or response.failed, skips continuation
persistence, or preserves a valid response, and add coverage for both the
"hello" commitment and continuation replay paths.
In `@devlog/_plan/260805_bug_stack_campaign/070_issue907_jawcode_prices.md`:
- Around line 39-44: Expand the table-driven tests around resolveMatchedPrice in
tests/usage-cost.test.ts to cover Terra and Luna model pricing for every
provider that exposes those models, not just the openai row. Assert each
provider’s expected standard/Fast four-tuples along with source and status
provenance, preserving the existing cursor and kiro coverage.
- Around line 12-18: Update the metadata-generation plan around
JAWCODE_MODELS_JSON and the generator reference to pin the external Jawcode
source using its commit SHA or packages/ai/src/models.json hash. Require the
scoped checkout to be clean before generation, and record the pinned revision
alongside the regeneration inputs so src/generated/jawcode-model-metadata.ts is
reproducible.
In `@devlog/_plan/260805_bug_stack_campaign/110_issue993_kiro_profile_arn.md`:
- Around line 23-32: In oauthCredentialFromImported(), bind identity.profileArn
from readKiroCliIdentity() to the imported credential’s account before
persisting it: compare the ARN account component with the imported accountId and
omit the ARN when they differ, while preserving matching-account behavior. Add a
regression test covering mismatched whoami and SQLite accounts.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 324d1d83-7749-4fe0-af6d-6885889a10a4
📒 Files selected for processing (19)
devlog/_plan/260805_bug_stack_campaign/000_plan.mddevlog/_plan/260805_bug_stack_campaign/001_issue_triage.mddevlog/_plan/260805_bug_stack_campaign/002_pr_triage.mddevlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.mddevlog/_plan/260805_bug_stack_campaign/010_phase1_pr988_review_merge.mddevlog/_plan/260805_bug_stack_campaign/020_phase2_anthropic_400_fix.mddevlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.mddevlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.mddevlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.mddevlog/_plan/260805_bug_stack_campaign/060_issue938_uuid_item_ids.mddevlog/_plan/260805_bug_stack_campaign/070_issue907_jawcode_prices.mddevlog/_plan/260805_bug_stack_campaign/080_issue1007_login_url_flush.mddevlog/_plan/260805_bug_stack_campaign/090_issue1001_forced_answer_validation.mddevlog/_plan/260805_bug_stack_campaign/100_issue992_routed_context_window.mddevlog/_plan/260805_bug_stack_campaign/110_issue993_kiro_profile_arn.mddevlog/_plan/260805_bug_stack_campaign/120_issue959_provider_headers.mddevlog/_plan/260805_bug_stack_campaign/130_dispositions.mdsrc/adapters/anthropic.tstests/anthropic-error-body.test.ts
| As of 2026-08-05 (KST), bring the open bug surface of lidge-jun/opencodex to | ||
| the state where every item is either landed, closed with evidence, or carried | ||
| by a stacked PR grounded in code somebody actually read. Three fronts: | ||
|
|
||
| 1. PR #988 — review against the GUI design system, small fixes if needed, | ||
| merge to `dev` (user authorized this merge/push). | ||
| 2. Full triage of open bug issues + bug/code-improvement PRs against | ||
| `origin/dev` HEAD `e44d234f08e03dd4dbf0c4aa13af43046d86b0a6`. Items | ||
| already fixed on `dev` get closed with evidence; the rest enter the stack. | ||
| 3. New regression: 400 `invalid_request_error` when a new Codex session/thread | ||
| spawns against `anthropic/claude-opus-5` (2026-08-05 00:22 KST, request | ||
| `ocx-mset3rk6-1i6`, conversation `2c0f87c664e130b3927c4884fafd8283`). | ||
| Root-cause and fix with a regression test. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
printf '%s\n' '--- campaign files ---'
git ls-files 'devlog/_plan/260805_bug_stack_campaign/*'
printf '%s\n' '--- referenced files and line ranges ---'
for f in \
devlog/_plan/260805_bug_stack_campaign/000_plan.md \
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md \
devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md \
devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md \
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md
do
echo "### $f"
nl -ba "$f" | sed -n '1,60p'
done
printf '%s\n' '--- all explicit 2026 dates in campaign ---'
rg -n -o '2026-[0-9]{2}-[0-9]{2}([^0-9]|$)|[0-9]{2}/[0-9]{2}/2026' \
devlog/_plan/260805_bug_stack_campaignRepository: lidge-jun/opencodex
Length of output: 2690
🏁 Script executed:
for f in \
devlog/_plan/260805_bug_stack_campaign/000_plan.md \
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md \
devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md \
devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md \
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md
do
echo "### $f"
awk 'NR <= 110 { printf "%6d\t%s\n", NR, $0 }' "$f"
done
printf '%s\n' '--- date-bearing and planning language ---'
rg -n -i 'planned|plan|future|as of|collected|evidence|observed|completed|closed|disposition|triage|2026-08-0[1-5]' \
devlog/_plan/260805_bug_stack_campaignRepository: lidge-jun/opencodex
Length of output: 35240
Mark August 5 activity as planned until that date. Because the review date is 2026-08-04, update 000_plan.md:5-19,52, 001_issue_triage.md:1, 002_pr_triage.md:1, and 003_anthropic_400_research.md:5-8,21 to use planned language or actual collection dates. Keep the historical August 1 and August 4 dates in 003_anthropic_400_research.md unchanged. 130_dispositions.md:1 has no date and needs no change.
📍 Affects 5 files
devlog/_plan/260805_bug_stack_campaign/000_plan.md#L5-L17(this comment)devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md#L1-L1devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md#L1-L1devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md#L5-L15devlog/_plan/260805_bug_stack_campaign/130_dispositions.md#L1-L1
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/000_plan.md` around lines 5 - 17, Mark
August 5 activity as planned rather than completed: update
devlog/_plan/260805_bug_stack_campaign/000_plan.md at lines 5-19 and 52,
001_issue_triage.md at line 1, 002_pr_triage.md at line 1, and
003_anthropic_400_research.md at lines 5-8 and 21 to use planned language or
actual collection dates, while preserving the historical August 1 and August 4
dates in 003_anthropic_400_research.md.
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md:1 requires no change.
| #657, #572, #561, #540, #241, #201, #178, #177, #95, #92, #417, #415, #414, | ||
| #386. (Improvement #820 and #809 are code-level and stay in the campaign's |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Render issue and PR identifiers as list content, not headings.
Lines that start with #974, #811, or #933 trigger MD018 and can render inconsistently. Prefix each continuation with - or wrap the identifiers in code.
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md#L40-L41: convert the two issue-ID lines to list items.devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md#L37-L37: convert the feature-PR line to a list item.devlog/_plan/260805_bug_stack_campaign/130_dispositions.md#L42-L42: convert the stale-PR line to a list item.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 40-40: No space after hash on atx style heading
(MD018, no-missing-space-atx)
[warning] 41-41: No space after hash on atx style heading
(MD018, no-missing-space-atx)
📍 Affects 3 files
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md#L40-L41(this comment)devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md#L37-L37devlog/_plan/260805_bug_stack_campaign/130_dispositions.md#L42-L42
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md` around lines 40 -
41, Convert the issue-ID lines at
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md lines 40-41 into list
items by prefixing each continuation with “-” or using inline code. Also convert
the feature-PR line at devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md
line 37 and stale-PR line at
devlog/_plan/260805_bug_stack_campaign/130_dispositions.md line 42 into list
items, without changing their identifiers or surrounding content.
Source: Linters/SAST tools
| 3. Unit: `formatAnthropicErrorBody` on a real Anthropic 400 envelope | ||
| (`{"type":"error","error":{"type":"invalid_request_error","message":"…"}}`) | ||
| → `"invalid_request_error: …"`; on HTML → `""`; on JSON without error | ||
| fields → `""`; secret-shaped content inside `message` is redacted. | ||
| 4. Bridge integration: fake anthropic upstream returning a JSON 400 through | ||
| the web-search loop → the client-facing JSON error response carries | ||
| `Provider error 400: invalid_request_error: …` (not the bare status; the | ||
| thrown `LoopError` is converted by `runWithWebSearch`, | ||
| `src/web-search/loop.ts:657-662`). Follow the existing web-search loop | ||
| test harness (find the suite covering `loop.ts` error paths and extend | ||
| it). | ||
| 5. Persistence integration: same fake 400 through `/v1/responses` with | ||
| web_search enabled against a routed anthropic model → the usage entry's | ||
| `upstreamError` contains the upstream message (proves the | ||
| `relay.ts:429` → `request-log.ts:648` capture chain end to end; no | ||
| production change expected in those files). | ||
| 6. Pin the read-failure branch: upstream body that fails to read still |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restart the ordered list at Line 38.
Lines 38, 42, 49, and 54 start a new list under ### Tests, but they continue numbering from the previous list. Markdownlint MD029 requires this list to start at 1. Renumber these items from 1 through 4 so the document is lint-clean.
Proposed fix
-3. Unit: `formatAnthropicErrorBody` on a real Anthropic 400 envelope
+1. Unit: `formatAnthropicErrorBody` on a real Anthropic 400 envelope
...
-4. Bridge integration: fake anthropic upstream returning a JSON 400 through
+2. Bridge integration: fake anthropic upstream returning a JSON 400 through
...
-5. Persistence integration: same fake 400 through `/v1/responses` with
+3. Persistence integration: same fake 400 through `/v1/responses` with
...
-6. Pin the read-failure branch: upstream body that fails to read still
+4. Pin the read-failure branch: upstream body that fails to read still📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 3. Unit: `formatAnthropicErrorBody` on a real Anthropic 400 envelope | |
| (`{"type":"error","error":{"type":"invalid_request_error","message":"…"}}`) | |
| → `"invalid_request_error: …"`; on HTML → `""`; on JSON without error | |
| fields → `""`; secret-shaped content inside `message` is redacted. | |
| 4. Bridge integration: fake anthropic upstream returning a JSON 400 through | |
| the web-search loop → the client-facing JSON error response carries | |
| `Provider error 400: invalid_request_error: …` (not the bare status; the | |
| thrown `LoopError` is converted by `runWithWebSearch`, | |
| `src/web-search/loop.ts:657-662`). Follow the existing web-search loop | |
| test harness (find the suite covering `loop.ts` error paths and extend | |
| it). | |
| 5. Persistence integration: same fake 400 through `/v1/responses` with | |
| web_search enabled against a routed anthropic model → the usage entry's | |
| `upstreamError` contains the upstream message (proves the | |
| `relay.ts:429` → `request-log.ts:648` capture chain end to end; no | |
| production change expected in those files). | |
| 6. Pin the read-failure branch: upstream body that fails to read still | |
| 1. Unit: `formatAnthropicErrorBody` on a real Anthropic 400 envelope | |
| (`{"type":"error","error":{"type":"invalid_request_error","message":"…"}}`) | |
| → `"invalid_request_error: …"`; on HTML → `""`; on JSON without error | |
| fields → `""`; secret-shaped content inside `message` is redacted. | |
| 2. Bridge integration: fake anthropic upstream returning a JSON 400 through | |
| the web-search loop → the client-facing JSON error response carries | |
| `Provider error 400: invalid_request_error: …` (not the bare status; the | |
| thrown `LoopError` is converted by `runWithWebSearch`, | |
| `src/web-search/loop.ts:657-662`). Follow the existing web-search loop | |
| test harness (find the suite covering `loop.ts` error paths and extend | |
| it). | |
| 3. Persistence integration: same fake 400 through `/v1/responses` with | |
| web_search enabled against a routed anthropic model → the usage entry's | |
| `upstreamError` contains the upstream message (proves the | |
| `relay.ts:429` → `request-log.ts:648` capture chain end to end; no | |
| production change expected in those files). | |
| 4. Pin the read-failure branch: upstream body that fails to read still |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 38-38: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 42-42: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 49-49: Ordered list item prefix
Expected: 3; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
[warning] 54-54: Ordered list item prefix
Expected: 4; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/020_phase2_anthropic_400_fix.md`
around lines 38 - 54, Renumber the ordered list under the Tests section so it
starts at 1 and continues sequentially through 4, including the items beginning
with “Unit,” “Bridge integration,” “Persistence integration,” and “Pin the
read-failure branch.”
Source: Linters/SAST tools
| - Generator: `scripts/generate-jawcode-metadata.ts:22` — | ||
| `JAWCODE_MODELS_JSON` or `../jawcode/packages/ai/src/models.json`; copies | ||
| cost fields untransformed (`:82`). | ||
| - Local jawcode source `/Users/jun/Developer/new/700_projects/jawcode/packages/ai/src/models.json` | ||
| is stale in four bundles: `github-copilot` (:10667,:10723), `openai` | ||
| (:59069,:59121), `openai-codex` (:59822,:59876), `opencode-zen` | ||
| (:61734,:61784). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial
Pin the external Jawcode revision before regenerating metadata.
JAWCODE_MODELS_JSON points to a developer-specific path, and the plan says the external checkout is dirty. A later regeneration can use different source data without a reviewable source change. Record the Jawcode commit SHA or a hash of packages/ai/src/models.json, and require a clean scoped checkout for generation. This makes src/generated/jawcode-model-metadata.ts reproducible.
Also applies to: 32-34
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/070_issue907_jawcode_prices.md` around
lines 12 - 18, Update the metadata-generation plan around JAWCODE_MODELS_JSON
and the generator reference to pin the external Jawcode source using its commit
SHA or packages/ai/src/models.json hash. Require the scoped checkout to be clean
before generation, and record the pinned revision alongside the regeneration
inputs so src/generated/jawcode-model-metadata.ts is reproducible.
| MODIFY `src/oauth/kiro.ts`: | ||
| - Extend `readKiroCliIdentity()` (`:175-185`) to return | ||
| `{email?, profileArn?}`; parse only documented `whoami --format json` | ||
| shapes (`profileArn`, `profile_arn`, `profile.arn`); validate length + | ||
| `arn:<partition>:codewhisperer:<region>:<account>:profile/<id>` structure. | ||
| - `oauthCredentialFromImported()` (`:249-264`): | ||
| `resolvedProfileArn = imported.profileArn ?? identity.profileArn` → | ||
| `credential.kiro.profileArn` + `accountId`. Account-safe: `whoami` runs | ||
| against the same active CLI session just imported. Fail closed when | ||
| absent. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- planned change ---'
sed -n '1,45p' devlog/_plan/260805_bug_stack_campaign/110_issue993_kiro_profile_arn.md
printf '%s\n' '--- relevant implementation symbols ---'
rg -n -A45 -B12 'readKiroCliIdentity|oauthCredentialFromImported|resolveKiroProfileArn' src/oauth/kiro.ts
printf '%s\n' '--- related account/session handling ---'
rg -n -A8 -B8 'readKiroCliCredential|imported|whoami|profileArn|accountId' src/oauth/kiro.tsRepository: lidge-jun/opencodex
Length of output: 25844
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Kiro runner and import definitions ---'
sed -n '1,175p' src/oauth/kiro.ts
printf '%s\n' '--- SQLite credential shape and source selection ---'
sed -n '240,375p' src/oauth/kiro-credentials.ts
printf '%s\n' '--- tests covering import identity binding ---'
rg -n -A12 -B8 'whoami|profileArn|oauthCredentialFromImported|accountId|sqlite' tests/kiro-oauth.test.ts tests 2>/dev/null | head -n 260Repository: lidge-jun/opencodex
Length of output: 30397
Other (CWE-345)
Reachability: Internal
Bind whoami output to the imported account before persisting profileArn.
readKiroCliIdentity() queries the active CLI session, while readKiroCliSqliteCredential() can read a configured or fallback database. These sources are not bound to the same session. If they represent different accounts, oauthCredentialFromImported() can persist account B’s ARN for account A, and resolveKiroProfileArn() will use it as authoritative. Verify the accounts match, or omit the ARN on mismatch. Add a regression test for this case.
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/110_issue993_kiro_profile_arn.md`
around lines 23 - 32, In oauthCredentialFromImported(), bind identity.profileArn
from readKiroCliIdentity() to the imported credential’s account before
persisting it: compare the ARN account component with the imported accountId and
omit the ARN when they differ, while preserving matching-account behavior. Add a
regression test covering mismatched whoami and SQLite accounts.
…obe evidence, corrected citations, dependency-honest roadmap
…delity diagnosis, formatErrorBody plan, PR988 head plan
…, host-ledger retention bound)
… via formatErrorBody
2724c76 to
6c8a683
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md`:
- Line 1: Correct future-dated audit evidence by replacing the date in
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md lines 1-1 and
devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md lines 1-1 with the
actual dates. In
devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md, lines 5-8,
correct the incident date, and on lines 19-21 correct the probe date;
alternatively mark each entry as planned until it occurs.
In
`@devlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.md`:
- Around line 22-31: Update fetchWithTransientRetry and its error-classification
flow to distinguish ECONNRESET during socket establishment from resets on an
established or pooled socket using explicit phase metadata; classify only
established-socket resets as connect_error, while treating pre-connection and
unknown-phase resets as neutral. Preserve existing cause traversal limits and
add separate coverage for pre-connection, established-socket, and ambiguous
reset cases.
In `@devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md`:
- Around line 31-35: Update the extracted event-sequence helper from
sendResponsesJsonAsEvents() to emit the complete message lifecycle for
response.output messages: response.content_part.added,
response.output_text.delta, response.output_text.done, and
response.content_part.done, while preserving response.created, per-item
completion, and the status-preserving terminal event. Ensure both HTTP SSE
serialization and WebSocket conversion consume this lifecycle, and expand the
relevant tests to verify committed text and continuation behavior.
- Around line 40-49: Define and implement explicit bounded-body failure
semantics in the bounded JSON branch around the response parsing flow:
distinguish malformed complete JSON, size-limit failures, total timeouts, and
inactivity timeouts, specifying each case’s status, JSON body, headers, logging,
and persistence behavior. Persist only complete safe responses; never persist
partial or oversized data, and on any failure omit SSE headers and the [DONE]
marker while reusing sanitizePassthroughHeaders() for header cleanup. Add
assertions covering every failure case without changing successful response
behavior.
In `@tests/anthropic-error-body.test.ts`:
- Around line 44-52: Add a regression test alongside the existing
formatAnthropicErrorBody tests using a message longer than 400 characters, then
assert that the formatted result returned by formatAnthropicErrorBody has
exactly 400 characters. Reuse the existing headers and request setup while
targeting the truncation behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6faa52aa-eab3-41a6-aa24-9b59cb84170e
📒 Files selected for processing (19)
devlog/_plan/260805_bug_stack_campaign/000_plan.mddevlog/_plan/260805_bug_stack_campaign/001_issue_triage.mddevlog/_plan/260805_bug_stack_campaign/002_pr_triage.mddevlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.mddevlog/_plan/260805_bug_stack_campaign/010_phase1_pr988_review_merge.mddevlog/_plan/260805_bug_stack_campaign/020_phase2_anthropic_400_fix.mddevlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.mddevlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.mddevlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.mddevlog/_plan/260805_bug_stack_campaign/060_issue938_uuid_item_ids.mddevlog/_plan/260805_bug_stack_campaign/070_issue907_jawcode_prices.mddevlog/_plan/260805_bug_stack_campaign/080_issue1007_login_url_flush.mddevlog/_plan/260805_bug_stack_campaign/090_issue1001_forced_answer_validation.mddevlog/_plan/260805_bug_stack_campaign/100_issue992_routed_context_window.mddevlog/_plan/260805_bug_stack_campaign/110_issue993_kiro_profile_arn.mddevlog/_plan/260805_bug_stack_campaign/120_issue959_provider_headers.mddevlog/_plan/260805_bug_stack_campaign/130_dispositions.mdsrc/adapters/anthropic.tstests/anthropic-error-body.test.ts
| @@ -0,0 +1,43 @@ | |||
| # 001 — Open issue triage (2026-08-05, vs origin/dev e44d234f0) | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct future-dated audit evidence.
August 5, 2026 is tomorrow relative to August 4, 2026. These records present future triage, incidents, and probe results as completed evidence. Use the actual observation date, or mark the work as planned until it occurs.
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md#L1-L1: replace2026-08-05with the actual triage date.devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md#L1-L1: replace2026-08-05with the actual PR-triage date.devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md#L5-L8: correct the incident date before retaining these entries as observed failures.devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md#L19-L21: correct the probe date before retaining these entries as completed live-probe evidence.
📍 Affects 3 files
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md#L1-L1(this comment)devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md#L1-L1devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md#L5-L8devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md#L19-L21
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md` at line 1,
Correct future-dated audit evidence by replacing the date in
devlog/_plan/260805_bug_stack_campaign/001_issue_triage.md lines 1-1 and
devlog/_plan/260805_bug_stack_campaign/002_pr_triage.md lines 1-1 with the
actual dates. In
devlog/_plan/260805_bug_stack_campaign/003_anthropic_400_research.md, lines 5-8,
correct the incident date, and on lines 19-21 correct the probe date;
alternatively mark each entry as planned until it occurs.
| | `ENOTFOUND`, `EAI_AGAIN`, `ENETUNREACH`, `ENETDOWN`, `EHOSTUNREACH`, `ECONNREFUSED`; Bun `ConnectionRefused`/`FailedToOpenSocket` | neutral — record provider-origin host ledger only | | ||
| | `ECONNRESET`, `EPIPE`, established/read-then-close | keep `connect_error` | | ||
| | TLS/cert/handshake or unknown rejection | keep `connect_error` | | ||
| | `TimeoutError` | keep `timeout` | | ||
| | client abort | 499 path, no health evidence | | ||
| | any real HTTP response (incl. manual 3xx) | clears the host streak; existing status policy applies | | ||
| | 5xx → then reachability rejection | account-attributed (credential path was reached) | | ||
|
|
||
| Never classify from message text; walk at most three `Error.cause` links, | ||
| stop on cycles/non-Errors. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'ECONNRESET|isPreConnectReachabilityError|classifyTransportFailureKind|ConnectionRefused|Error\.cause' \
src testsRepository: lidge-jun/opencodex
Length of output: 24245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plan file ---'
cat -n devlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.md
printf '%s\n' '--- attribution and transport symbols ---'
rg -n -C 6 \
'isPreConnectReachabilityError|classifyTransportFailureKind|connect_error|timeout|provider-origin|host streak|account-attributed|credential|redirect|manual' \
src tests devlog/_plan/260805_bug_stack_campaignRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plan lines 1-90 ---'
sed -n '1,90p' devlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.md
printf '%s\n' '--- exact symbols in source and tests ---'
rg -n -C 10 \
'isPreConnectReachabilityError|classifyTransportFailureKind|provider-origin|host streak|connect_error|ECONNRESET|ConnectionRefused|FailedToOpenSocket' \
src tests \
-g '*.ts' \
-g '*.tsx' \
| head -n 1200
printf '%s\n' '--- likely send-site files ---'
rg -l \
'connect_error|isPreConnectReachabilityError|classifyTransportFailureKind|provider-origin|host streak' \
src tests \
-g '*.ts' \
-g '*.tsx'Repository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- retry implementation ---'
sed -n '1,45p;260,355p' src/lib/upstream-retry.ts
printf '%s\n' '--- response transport catches and send sites ---'
rg -n -C 18 \
'transportFailureResponse|fetchWithTransientRetry|retryCodexPoolOnAlternateAccount|sendCompactAttempt|fetch\(' \
src/server/responses/core.ts src/server/responses/compact.ts src/lib/upstream-retry.ts
printf '%s\n' '--- all current phase/commit evidence identifiers ---'
rg -n -C 5 \
'response.*received|response.*header|headers.*received|request.*committed|committed|established|pre.?connect|before.*response|after.*response|body.*read|read.*close' \
src/lib src/server/responses src/codex \
-g '*.ts' | head -n 1000Repository: lidge-jun/opencodex
Length of output: 50376
🌐 Web query:
Bun fetch ECONNRESET connection establishment response headers error phase stale keep-alive socket
💡 Result:
In Bun, an ECONNRESET error during the connection establishment or response header phase, particularly when using fetch, is often related to the handling of stale keep-alive (persistent) sockets [1][2][3][4]. Because Bun's fetch implementation automatically reuses connections by default, several scenarios can lead to this error when the server-side state disagrees with the client-side pool: 1. Stale Keep-Alive Socket Reuse: Bun may attempt to reuse a pooled connection that the server has already closed due to idle timeouts [3][4]. If the server sends a FIN/RST packet, Bun should ideally detect this and retry on a new socket [1][3]. However, bugs in how Bun parses connection headers (e.g., failing to recognize Connection: close within a comma-separated list [2] or ignoring it on non-2xx status codes [5][6]) can result in "dead" sockets being kept in the pool and subsequently reused, triggering an ECONNRESET when the request is sent [2][5]. 2. Silent Retries and Stream Issues: In older or specific versions, Bun would silently retry requests on a new connection if it encountered an ECONNRESET on a reused socket [3]. This behavior, especially with non-idempotent methods like POST, can cause unexpected errors or stream corruption [3][4]. Recent updates have aimed to restrict these retries to idempotent methods (GET, HEAD, etc.) to prevent duplicate actions and ensure correct error propagation [4]. 3. ProxyTunnel and Close-Delimited Responses: When using an HTTP proxy, some configurations involving close-delimited responses (missing Content-Length or Transfer-Encoding) were incorrectly interpreted by Bun as a connection reset (ECONNRESET) rather than the natural end of the message [7][8]. This was addressed by aligning Bun's ProxyTunnel logic to treat such EOF signals as a completed request [7]. Recommended Workarounds and Troubleshooting: - Disable Keep-Alive: If you encounter intermittent ECONNRESET errors, verify if disabling keep-alive resolves the issue by setting { keepalive: false } in your fetch options [2][8]: await fetch(url, {...options, keepalive: false }); - Upgrade Bun: Ensure you are using the latest version of Bun, as many of these issues—specifically those related to connection header parsing (e.g., Connection: close) and retry logic—have been addressed in recent updates [5][6]. - Verbose Logging: To gain more insight into the failure, pass verbose: true in the second argument to fetch [2][3]. This can provide additional context on why the socket connection was closed unexpectedly. If the issue persists, check if the server is forcibly closing connections or if there is a load balancer/proxy in the path that might be aggressively reaping idle sockets [1][3].
Citations:
- 1: fetch(): reused keep-alive socket isn't liveness-checked — a half-open pooled connection hangs the request instead of reconnecting oven-sh/bun#31894
- 2: Bun canary Rust fetch reuses sockets after
Connection: closeappears in a token list oven-sh/bun#31463 - 3: fetch silently retries POST on keep-alive disconnect (ECONNRESET) and merges response streams oven-sh/bun#28706
- 4: Don't retry non-idempotent HTTP methods on keep-alive disconnect oven-sh/bun#28708
- 5: fetch: honour Connection: close on non-2xx responses and HTTP/1.0 defaults in the keep-alive pool oven-sh/bun#35545
- 6: http: honor Connection: close on non-2xx responses oven-sh/bun#36370
- 7: ProxyTunnel: close-delimited responses via proxy cause ECONNRESET oven-sh/bun#23719
- 8: HTTPS over HTTP Proxy: close-delimited response triggers ECONNRESET oven-sh/bun#23717
Define a safe ECONNRESET phase rule.
Lines 22–25 classify ECONNRESET as connect_error, but lines 30–31 provide only code and cause traversal. The current fetchWithTransientRetry() API exposes no evidence that distinguishes socket establishment from a reset on an already-established pooled socket. A pre-connection reset can therefore mutate account streaks, soft-avoid state, affinity, or the active account. Add phase metadata, or classify ambiguous resets as neutral. Test pre-connection, established-socket, and ambiguous resets separately.
🤖 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
`@devlog/_plan/260805_bug_stack_campaign/030_issue914_dns_transport_attribution.md`
around lines 22 - 31, Update fetchWithTransientRetry and its
error-classification flow to distinguish ECONNRESET during socket establishment
from resets on an established or pooled socket using explicit phase metadata;
classify only established-socket resets as connect_error, while treating
pre-connection and unknown-phase resets as neutral. Preserve existing cause
traversal limits and add separate coverage for pre-connection,
established-socket, and ambiguous reset cases.
| ADD `src/server/responses-json-events.ts` — extract the pure event sequence | ||
| from `sendResponsesJsonAsEvents()` (`response.created` → one | ||
| `response.output_item.done` per output item → status-preserving terminal) + | ||
| an SSE serializer appending exactly one `data: [DONE]\n\n`; optional | ||
| client-facing payload rewrite hook (060 composes ID normalization here). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'sendResponsesJsonAsEvents|responses-json-events|response\.output_text|response\.content_part|response\.output_item\.done' \
src testsRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate server files ---'
fd -t f . src/server src tests | rg '(ws-bridge|responses|outbound|issue893|260805_bug_stack_campaign)'
printf '%s\n' '--- relevant symbols ---'
rg -n -C 5 \
'function sendResponsesJsonAsEvents|sendResponsesJsonAsEvents|readBounded|JSON\.parse|response\.output_text\.done|response\.content_part\.added|response\.content_part\.done|output_text\.done|Codex' \
src/server src tests devlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.md \
--glob '!tests/chat-completions-endpoint.test.ts' \
--glob '!tests/ws-endpoint.test.ts'Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- plan files ---'
for f in \
devlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.md \
devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md \
devlog/_plan/260805_bug_stack_campaign/060_issue*.md
do
for match in $f; do
if [ -f "$match" ]; then
echo "### $match"
sed -n '1,140p' "$match"
fi
done
doneRepository: lidge-jun/opencodex
Length of output: 12800
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- precise existing JSON conversion tests ---'
sed -n '205,275p' tests/ws-endpoint.test.ts
sed -n '330,390p' tests/ws-endpoint.test.ts
printf '%s\n' '--- response conversion and consumer lifecycle references ---'
rg -n -C 6 \
'convertResponses|response\.output_item\.done|response\.content_part|response\.output_text\.done|output_text\.delta|message' \
src/claude src/codex src/server tests \
--glob '*.ts' \
--glob '!tests/chat-completions-endpoint.test.ts' \
--glob '!tests/ws-endpoint.test.ts'Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'sendResponsesJsonAsEvents|responses-json-events|response\.output_text|response\.content_part|response\.output_item\.done' src testsRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- source locations ---'
rg -n \
'function sendResponsesJsonAsEvents|export function sendResponsesJsonAsEvents|sendResponsesJsonAsEvents|response\.output_item\.done|content_part\.added|response\.output_text\.done|content_part\.done|response\.completed|response\.incomplete|response\.failed' \
src/server/ws-bridge.ts src/bridge.ts src/server/responses/core.ts
printf '%s\n' '--- ws-bridge implementation ---'
sed -n '250,345p' src/server/ws-bridge.ts
printf '%s\n' '--- Codex lifecycle consumer ---'
sed -n '470,535p' src/bridge.ts
sed -n '810,860p' src/bridge.ts
sed -n '1110,1145p' src/bridge.ts
printf '%s\n' '--- bounded JSON and HTTP branch ---'
sed -n '820,870p' src/server/responses/core.ts
sed -n '1845,1990p' src/server/responses/core.ts
sed -n '2090,2175p' src/server/responses/core.tsRepository: lidge-jun/opencodex
Length of output: 28935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused plan and test references ---'
rg -n -C 5 \
'message|lifecycle|parity|output_item\.done|content_part|output_text\.done|Codex|bounded JSON|malformed|oversized|timeout|failed|incomplete' \
devlog/_plan/260805_bug_stack_campaign/040_issue893_sparse_snapshot_repair.md \
devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md \
tests/ws-endpoint.test.ts \
tests/deepseek-inbound-wire.test.ts \
--glob '!tests/chat-completions-endpoint.test.ts'Repository: lidge-jun/opencodex
Length of output: 37442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
targets = [
Path("src/server/ws-bridge.ts"),
Path("src/server/responses/core.ts"),
Path("src/bridge.ts"),
]
terms = (
"sendResponsesJsonAsEvents",
"readBounded",
"response.content_part",
"response.output_text.done",
"response.output_item.done",
"response.completed",
"response.incomplete",
"response.failed",
)
for path in targets:
print(f"### {path}")
lines = path.read_text().splitlines()
hits = [i for i, line in enumerate(lines) if any(term in line for term in terms)]
for i in hits:
lo, hi = max(0, i - 4), min(len(lines), i + 7)
print(f"-- lines {lo+1}-{hi}")
for n in range(lo, hi):
print(f"{n+1}:{lines[n]}")
PYRepository: lidge-jun/opencodex
Length of output: 29032
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'sendResponsesJsonAsEvents|responses-json-events|response\.output_text|response\.content_part|response\.output_item\.done' src testsRepository: lidge-jun/opencodex
Length of output: 50376
Define the buffered HTTP message lifecycle.
At devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md:31-35, the shared helper emits only response.created, response.output_item.done, and a terminal event. The Codex message contract also requires response.content_part.added, response.output_text.delta, response.output_text.done, and response.content_part.done (src/bridge.ts:517-531, src/bridge.ts:838-850). Without these events, Codex can render the text as truncated and fail to commit continuation state. The existing JSON tests assert only the sparse sequence (tests/ws-endpoint.test.ts:220-233).
When response.output contains a message, emit the complete message lifecycle for both HTTP SSE and WebSocket conversion. Feed the generated HTTP stream into the Codex-facing lifecycle consumer and assert committed text and continuation behavior.
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md`
around lines 31 - 35, Update the extracted event-sequence helper from
sendResponsesJsonAsEvents() to emit the complete message lifecycle for
response.output messages: response.content_part.added,
response.output_text.delta, response.output_text.done, and
response.content_part.done, while preserving response.created, per-item
completion, and the status-preserving terminal event. Ensure both HTTP SSE
serialization and WebSocket conversion consume this lifecycle, and expand the
relevant tests to verify committed text and continuation behavior.
| MODIFY `src/server/responses/core.ts` — capture | ||
| `const clientRequestedStream = parsed.stream` immediately before | ||
| `applyFinalRouteRequestNormalization()` (`core.ts:1479`); after final adapter | ||
| resolution apply the transport-neutral policy (`stream:false` upstream); in | ||
| the bounded JSON branch (`core.ts:2124`): parse once after existing | ||
| byte/inactivity limits; record the raw response first; apply client-facing | ||
| ID normalization; return JSON unchanged for non-streaming clients and WS; | ||
| serialize terminal SSE + `[DONE]` for HTTP clients that requested streaming; | ||
| strip stale `content-length`/`content-encoding`, set SSE content type + | ||
| no-store. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate plan file ---'
sed -n '1,120p' devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md
printf '%s\n' '--- core.ts symbols and relevant ranges ---'
rg -n "applyFinalRouteRequestNormalization|bounded|content-length|content-encoding|\\[DONE\\]|clientRequestedStream|parsed\\.stream|JSON" src/server/responses/core.ts
sed -n '1430,1515p' src/server/responses/core.ts
sed -n '2060,2195p' src/server/responses/core.ts
printf '%s\n' '--- related tests and error handling references ---'
rg -n "inactivity|total timeout|timeout|malformed|truncated|bounded|raw response|applyFinalRouteRequestNormalization|\\[DONE\\]|content-length|content-encoding" src test tests devlog 2>/dev/null | head -n 300Repository: lidge-jun/opencodex
Length of output: 49960
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- route normalization and adapter selection ---'
sed -n '800,875p' src/server/responses/core.ts
sed -n '1840,1995p' src/server/responses/core.ts
printf '%s\n' '--- complete bounded JSON branch and following paths ---'
sed -n '2110,2225p' src/server/responses/core.ts
printf '%s\n' '--- bounded-body implementation ---'
fd -i 'bounded-body' .
for f in $(fd -i 'bounded-body' .); do
wc -l "$f"
cat -n "$f"
done
printf '%s\n' '--- response error helpers and logging/recording definitions ---'
rg -n "function formatErrorResponse|export function formatErrorResponse|function inspectResponseLogJson|function rememberPassthroughResponse|rememberPassthroughResponse|inspectResponseLogJson" src/server src/bridge.ts
sed -n '480,545p' src/server/responses/core.ts
sed -n '580,690p' src/server/responses/core.ts
printf '%s\n' '--- relevant tests ---'
sed -n '100,165p' tests/deepseek-inbound-wire.test.ts
sed -n '210,330p' tests/ws-endpoint.test.ts
rg -n "readBoundedResponseBody|upstream JSON response exceeded|upstream JSON response stalled|response.*malformed|malformed.*response" tests srcRepository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- exact error envelope ---'
sed -n '1745,1795p' src/bridge.ts
printf '%s\n' '--- passthrough header sanitization ---'
rg -n -A35 -B8 "function sanitizePassthroughHeaders|export function sanitizePassthroughHeaders" src
printf '%s\n' '--- recording callback and bounded branch callers ---'
sed -n '1695,1735p' src/server/responses/core.ts
sed -n '2010,2165p' src/server/responses/core.ts
printf '%s\n' '--- DeepSeek test body ---'
sed -n '120,180p' tests/deepseek-inbound-wire.test.ts
printf '%s\n' '--- bounded-body call sites and tests for response contracts ---'
rg -n -A12 -B8 "readBoundedResponseBody\\(" src tests
printf '%s\n' '--- bounded-branch structural facts ---'
python3 - <<'PY'
from pathlib import Path
p = Path("src/server/responses/core.ts").read_text()
start = p.index('if (headers.get("content-type")?.toLowerCase().includes("application/json"))')
end = p.index('\n const body = relayWithAbort', start)
branch = p[start:end]
facts = {
"bounded_reader": "readBoundedResponseBody(upstreamResponse" in branch,
"explicit_oversized_502": 'return formatErrorResponse(502, "upstream_error", "upstream JSON response exceeded the safe body limit")' in branch,
"explicit_truncated_502": 'return formatErrorResponse(502, "upstream_error", "upstream JSON response stalled before completing")' in branch,
"json_parse_is_throwing_control_flow": "JSON.parse(text)" in branch and "catch { /* non-JSON despite content-type; recording is best-effort */ }" not in branch,
"raw_status_on_success": "status: upstreamResponse.status" in branch,
"raw_status_on_failures": branch.count("status: upstreamResponse.status"),
"done_marker": "[DONE]" in branch,
"failure_response_constructor": "formatErrorResponse" in branch,
}
for k, v in facts.items():
print(f"{k}={v}")
PYRepository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
core = Path("src/server/responses/core.ts").read_text().splitlines()
plan = Path("devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md").read_text()
def line_number(fragment, start=0):
for i in range(start, len(core)):
if fragment in core[i]:
return i + 1
return None
branch_start = line_number('if (headers.get("content-type")?.toLowerCase().includes("application/json"))')
branch_end = line_number('const body = relayWithAbort', branch_start or 0)
branch = "\n".join(core[(branch_start or 1)-1:(branch_end or len(core))-1])
print(f"bounded_branch={branch_start}-{(branch_end or 1)-1}")
print(f"non_2xx_guard_before_branch={line_number('if (!upstreamResponse.ok)') < branch_start}")
print(f"oversized_returns_502={'formatErrorResponse(502' in branch and 'exceeded the safe body limit' in branch}")
print(f"truncated_returns_502={'formatErrorResponse(502' in branch and 'stalled before completing' in branch}")
print(f"parse_only_inside_best_effort_callback='try {' in branch and 'JSON.parse(text)' in branch and 'best-effort' in branch}")
print(f"parse_failure_returns_response='catch { /* non-JSON despite content-type; recording is best-effort */ }' in branch")
print(f"bounded_failure_calls_inspect_log='inspectResponseLogJson(logCtx, text)' in branch and 'formatErrorResponse' in branch.split('const text =', 1)[0])
print(f"bounded_failure_records_passthrough='rememberPassthroughResponse' in branch.split('const text =', 1)[0])
print(f"done_marker_in_current_branch='[DONE]' in branch")
print(f"plan_lists_malformed_failure='malformed' in plan and 'Matrix:' in plan")
print(f"plan_lists_size_failure='oversized' in plan and 'Matrix:' in plan")
print(f"plan_lists_total_timeout='total-timeout' in plan and 'Matrix:' in plan")
print(f"plan_lists_inactivity_timeout='inactivity-timeout' in plan and 'Matrix:' in plan")
PYRepository: lidge-jun/opencodex
Length of output: 492
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
core = Path("src/server/responses/core.ts").read_text().splitlines()
plan = Path("devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md").read_text()
def line_number(fragment, start=0):
for i in range(start, len(core)):
if fragment in core[i]:
return i + 1
return None
branch_start = line_number('if (headers.get("content-type")?.toLowerCase().includes("application/json"))')
branch_end = line_number('const body = relayWithAbort', branch_start or 0)
branch = "\n".join(core[(branch_start or 1)-1:(branch_end or len(core))-1])
before_text = branch.split("const text =", 1)[0]
checks = {
"bounded_branch": f"{branch_start}-{(branch_end or 1)-1}",
"non_2xx_guard_before_branch": line_number("if (!upstreamResponse.ok)") < branch_start,
"oversized_returns_502": "exceeded the safe body limit" in branch and 'formatErrorResponse(502' in branch,
"truncated_returns_502": "stalled before completing" in branch and 'formatErrorResponse(502' in branch,
"parse_in_branch": "JSON.parse(text)" in branch,
"parse_failure_is_caught": "best-effort" in branch,
"bounded_failure_logs_raw_text": "inspectResponseLogJson(logCtx, text)" in branch and "formatErrorResponse" in before_text,
"bounded_failure_records_passthrough": "rememberPassthroughResponse" in before_text,
"done_marker_in_current_branch": "[DONE]" in branch,
"plan_lists_malformed_failure": "malformed" in plan and "Matrix:" in plan,
"plan_lists_size_failure": "oversized" in plan and "Matrix:" in plan,
"plan_lists_total_timeout": "total-timeout" in plan and "Matrix:" in plan,
"plan_lists_inactivity_timeout": "inactivity-timeout" in plan and "Matrix:" in plan,
}
for key, value in checks.items():
print(f"{key}={value}")
PYRepository: lidge-jun/opencodex
Length of output: 560
Define bounded-body failure semantics before changing the HTTP wire format.
At src/server/responses/core.ts:2132-2162, oversized and stalled bodies return synthetic 502 JSON errors, while malformed complete JSON currently keeps the upstream status and body. Define the status, body, headers, logging, and persistence rules for malformed JSON, size limits, total timeouts, and inactivity timeouts. Record only complete safe data, never persist partial or oversized data, and emit neither SSE headers nor [DONE] on failure. Add assertions for each case. sanitizePassthroughHeaders() already removes stale content-length and content-encoding headers.
🤖 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 `@devlog/_plan/260805_bug_stack_campaign/050_issue875_deepseek_flash_stall.md`
around lines 40 - 49, Define and implement explicit bounded-body failure
semantics in the bounded JSON branch around the response parsing flow:
distinguish malformed complete JSON, size-limit failures, total timeouts, and
inactivity timeouts, specifying each case’s status, JSON body, headers, logging,
and persistence behavior. Persist only complete safe responses; never persist
partial or oversized data, and on any failure omit SSE headers and the [DONE]
marker while reusing sanitizePassthroughHeaders() for header cleanup. Add
assertions covering every failure case without changing successful response
behavior.
| test("secret-shaped content inside the message is redacted", () => { | ||
| const body = JSON.stringify({ | ||
| type: "error", | ||
| error: { type: "invalid_request_error", message: "bad key sk-redact-me-please" }, | ||
| }); | ||
| const out = formatAnthropicErrorBody(400, headers, body); | ||
| expect(out).not.toContain("sk-redact-me-please"); | ||
| expect(out).toContain("[REDACTED]"); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a regression test for the 400-character limit.
formatAnthropicErrorBody limits output at src/adapters/anthropic.ts Line 261. The current tests do not verify this contract. Add a message longer than 400 characters and assert that the formatted result has exactly 400 characters.
Proposed test
test("secret-shaped content inside the message is redacted", () => {
// ...
});
+
+ test("limits formatted output to 400 characters", () => {
+ const message = "x".repeat(500);
+ const out = formatAnthropicErrorBody(400, headers, JSON.stringify({
+ error: { type: "invalid_request_error", message },
+ }));
+ expect(out.length).toBe(400);
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("secret-shaped content inside the message is redacted", () => { | |
| const body = JSON.stringify({ | |
| type: "error", | |
| error: { type: "invalid_request_error", message: "bad key sk-redact-me-please" }, | |
| }); | |
| const out = formatAnthropicErrorBody(400, headers, body); | |
| expect(out).not.toContain("sk-redact-me-please"); | |
| expect(out).toContain("[REDACTED]"); | |
| }); | |
| test("secret-shaped content inside the message is redacted", () => { | |
| const body = JSON.stringify({ | |
| type: "error", | |
| error: { type: "invalid_request_error", message: "bad key sk-redact-me-please" }, | |
| }); | |
| const out = formatAnthropicErrorBody(400, headers, body); | |
| expect(out).not.toContain("sk-redact-me-please"); | |
| expect(out).toContain("[REDACTED]"); | |
| }); | |
| test("limits formatted output to 400 characters", () => { | |
| const message = "x".repeat(500); | |
| const out = formatAnthropicErrorBody(400, headers, JSON.stringify({ | |
| error: { type: "invalid_request_error", message }, | |
| })); | |
| expect(out.length).toBe(400); | |
| }); | |
| }); |
🤖 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/anthropic-error-body.test.ts` around lines 44 - 52, Add a regression
test alongside the existing formatAnthropicErrorBody tests using a message
longer than 400 characters, then assert that the formatted result returned by
formatAnthropicErrorBody has exactly 400 characters. Reuse the existing headers
and request setup while targeting the truncation behavior.
Source: Path instructions
#1020 review blocker 1. An upstream 4xx body quotes the offending header back at the caller ("x-api-key: <value>"), and the existing rules only covered `key=value`, JSON string fields, and known token shapes. A custom credential in that form survived redactSecretString and reached client-visible error text through the new error-body bridge. Adds a colon-labelled rule for the same credential vocabulary, excluding the `Bearer <token>` form so the dedicated Bearer rule keeps the scheme readable. Regression covers a non-sk-shaped custom credential and asserts ordinary diagnostics ("model: gpt-5.5") stay intact.
|
Rebased onto current Blocker: the new error-body bridge could echo a credential back to the client.
|
Summary
Stack 01 of the bug-stack campaign (
devlog/_plan/260805_bug_stack_campaign/).On 2026-08-05 00:22 KST, thread-spawn requests against
anthropic/claude-opus-5failed with 400s that the ledger recorded only as bareProvider error 400— the upstream reason was unrecoverable. Investigation (003 research doc + two live probes against the real Anthropic API) showed:src/web-search/loop.ts:460-484), which formats the upstream body only when the adapter implementsformatErrorBody(src/adapters/base.ts:24). openai-chat, google, and kiro implement it; anthropic did not, so the bridge discarded even well-formed Anthropic JSON error envelopes.This PR adds
formatAnthropicErrorBodyto the anthropic adapter (JSON-envelope extraction →type: message,redactSecretString, 400-char bound — same contract asformatOpenAIChatErrorBody), so the next occurrence lands in the client message AND the persistedupstreamErrorwith its real reason.No retry policy, no request-log/relay production change — the existing capture chain (
relay.ts→request-log.ts) already carries the formatted message; a test pins that end to end.Tests
tests/anthropic-error-body.test.ts(8): envelope/string/HTML/non-JSON shapes, secret redaction, real-adapter bridge integration throughrunWithWebSearchexercising the exact incident branch, persistence throughresponseWithDeferredRequestLog, body-read-failure bare-status pin.bun run typecheck— 0 errors; focused suites green;bun run privacy:scan— pass.bun run teston Linux (ssh lidge): 8232 pass / 0 fail (baseline 8222).Limitations
Summary by CodeRabbit
Bug Fixes
Documentation
Tests