[vadymhimself]

Summary

• When a combo target fails and the combo hops to the next target, executeComboResponses logged only [combo] <id>: <target> failed with <status> after <n>ms. A revoked ChatGPT session, an invalid key, or a plan downgrade therefore produced long runs of identical failed with 401 lines with nothing saying why, which read like a provider outage.
• The warning now appends the upstream error code and message the failure already parsed (upstreamCode, upstreamMessage, falling back to the normalized classificationText). No extra body read. The reason is collapsed to one line, redacted, stripped of control characters and capped at 200 characters by the existing sanitizeLogMetadataString helper, so a failure stays one log line and credentials echoed by an upstream body are not printed.
• Example: [combo] fan: primary/model-primary failed with 401 after 2ms: invalid_api_key: Incorrect API key provided: <redacted>. Rotate it.
• Log-only change in src/server/responses/core-combo.ts; routing, cooldown and the client response are unchanged.

Verification

• New test the hop warning names the upstream refusal on one redacted line in tests/responses/responses-combo-exhausted-error.test.ts, run before the src change: bun test tests/responses/responses-combo-exhausted-error.test.ts -> 16 pass, 1 fail (Received: "[combo] fan: primary/model-primary failed with 401 after 2ms"). After the change: 17 pass, 0 fail. The test also asserts that the API key from the upstream body is not in the line.
• bun x tsc --noEmit -> exit 0.
• bun run test:changed (Bun 1.4.3 via OCX_TEST_RUNNER_BUN) -> 15894 pass, 34 skip, 1 fail across 636 files. The single failure, tests/cli/cli-connect-readiness.test.ts "a rejected preferred runtime falls back without rewriting the saved selection", fails identically on an unmodified upstream/dev 8640c2f checkout on this machine (local-environment dependent) and is unrelated to combo logging.
• Full bun run test was not run locally: test:changed already covered 636 files through the changed module's import graph; the remainder is left to CI.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed. (No docs change needed: log text only.)
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (Upstream error text is redacted through sanitizeLogMetadataString before it reaches the log; covered by the test.)

Source: carried from the vadymhimself fork build (commits c234b70 and fde8786, core-combo.ts hunks), re-authored to reuse sanitizeLogMetadataString.

🤖 Generated with Claude Code

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• Required local validation passed; commands, results, and any full-suite exception are documented.

• I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Improvements
  • Failure warnings now include a concise, sanitized reason when one is available, alongside the target, status, and elapsed time.
  • Multiline error messages are condensed into a single line, and sensitive API keys are redacted.
  • Unparsed upstream response bodies are not included in warning logs, helping keep diagnostic details useful without exposing raw response content.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

⏳ DRAFT

• review readiness checklist open (0/4 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

• ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
• ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: d23f4b15-7c89-401f-ba8b-991635fe04e4

📥 Commits

Reviewing files that changed from the base of the PR and between 142afaf and e5eb961.

📒 Files selected for processing: 2

• src/server/responses/core-combo.ts
• tests/responses/responses-combo-exhausted-error.test.ts

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

📝 Walkthrough

Combo failure warnings now include a sanitized upstream error reason when available. Tests check that parsed error details are normalized and redacted, and that an unparsed response body is not logged.

Changes: Combo failure logging

Layer / File(s)	Summary
Add and verify sanitized failure reasons src/server/responses/core-combo.ts, tests/responses/responses-combo-exhausted-error.test.ts	The warning appends a sanitized, whitespace-collapsed upstream code and message, limited to 200 characters, when the reason is non-empty. Tests check key redaction for a parsed 401 error and confirm that an unparsed 502 response body is not logged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e5eb9

The warning change is described as logging only sanitized, bounded parsed details, with tests for redaction and unparsed-body omission. No PR-attributable failure is established; the reported suite failure is said to reproduce on upstream/dev.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: logging the upstream refusal reason when a combo target fails.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Ingwannu]

Intake/scoped review at 142afaf876e11309aed5ec570a5e8e87edff690b.

The warning adds upstream refusal metadata through the existing sanitizer and one-line bound. Because classificationText can originate in an upstream envelope, the log/credential review must establish the allowed fields rather than treating string truncation as a privacy proof. This intake does not certify that new logging boundary or initiate a scan.

Current-head functional workflows are awaiting fork approval; that is pending execution, not a passing test result. No local suite was run for this branch in this intake. No workflow approval, new scan, live configuration change or merge was performed. Required review/readiness gates remain in force.

[vadymhimself]

Thanks for the intake. On the logging boundary: the warning now carries only two parsed fields, upstreamCode and upstreamMessage, through sanitizeLogMetadataString with a 200-char bound. The classificationText fallback is removed in the latest push to this branch, so an unparsed upstream body is never logged. A new test sends a plain-text 502 body and asserts the line ends at after <n>ms with nothing appended; it fails with the fallback restored and passes without it. tsc --noEmit is clean.

🤖 Generated with Claude Code

[lidge-jun]

Thank you @vadymhimself! Both commits are absorbed into #7355 with your authorship intact. One follow-up commit there redacts the parsed fields before collapsing whitespace, so multiline credential framing in an upstream code cannot slip past redaction. The maintainer will close this PR when #7355 lands.

[lidge-jun]

Thank you for this fix! It was absorbed into #7355 (merged into dev as 718692b) with your original commits/Co-authored-by credit. Closing in favor of that PR.