Skip to content

Log request model and credential presence on response recovery - #16273

Open
meehol wants to merge 2 commits into
warpdotdev:masterfrom
meehol:meehol/log-request-context-on-recovery
Open

meehol wants to merge 2 commits into
warpdotdev:masterfrom
meehol:meehol/log-request-context-on-recovery

Conversation

@meehol

@meehol meehol commented Oct 3, 2026 •

Copy link
Copy Markdown

Description

When a MultiAgent response stream fails and Warp retries, resumes, or gives up, the log line identifies the failed request only as original or resume. It does not say which model the request targeted or whether it carried user-provided API keys or custom model providers.

That makes transport failures like the TLS UnexpectedEof reports in #7247 hard to attribute from a user's log alone. In particular, it is not possible to tell whether a failing request was a Warp-hosted model or one using the user's own provider credentials, so a report like "hosted models work but my own key fails" cannot be confirmed or ruled out.

This change appends model=, user_api_keys=, and custom_model_providers= to both the "recovering" and "not recovering" log lines. Only the presence of credentials is logged (booleans), never their values.

Example of the new log suffix:
failed_request=original model=<model-id> user_api_keys=true custom_model_providers=false

This is diagnostic only and does not change retry, resume, or error-surfacing behavior, so it does not by itself fix #7247.

Additional evidence from the same machine on Warp stable v0.2026.09.30.08.29.stable_01 (2026-10-03, UTC), all with is_online=true and the same UnexpectedEof ("peer closed connection without sending TLS close_notify"):

  • 03:27:37 original request, attempt 1/3, then resumes at 2/3 and 3/3 failed the same way, ending in recovery=none reason=budget_exhausted
  • 04:44:27 original request, attempt 1/3
  • 04:47:14 resume, attempt 2/3

The log records model selection changes, but not which model a given failed request used. In this log, 5 of the last 6 failure chains happened with auto-genius as the last selected model, so the failures do not appear to be specific to requests using a user-provided key (another pane could have been using a different model). All of these are recovery=resume failures after client actions were received, and each resumed request fails about 60 to 65 seconds after the previous failure, which suggests a cut of roughly 60s somewhere on the connection (details and timestamps on #7247). Other requests with a different model selected returned no visible reply while the log shows no transport error, so more than one failure mode may be involved.

Because the failing request's model and credential routing cannot be read from the existing log lines, those cases cannot be separated after the fact. Recording them on the failure line is what this PR adds.

Linked Issue

Refs #7247

  • The linked issue is labeled ready-to-spec or ready-to-implement.
  • Where appropriate, screenshots or a short video of the implementation are included below (not applicable: log-only change with no UI).

Testing

Automated:

  • Added request_context_label_reports_a_hosted_request_without_user_credentials and request_context_label_reports_attached_credentials_without_leaking_them in response_stream_tests.rs. The second asserts that a credential value attached to the request does not appear in the label.
  • cargo test -p warp --lib ai::blocklist::controller::response_stream: 18 passed, 0 failed (includes the existing recovery tests).
  • cargo clippy -p warp --all-targets --tests -- -D warnings: clean.
  • ./script/format: no further changes.

Manual:

  • I have manually tested my changes locally with ./script/run

I have not exercised this end to end in the running app, because reproducing the transport failure requires a network fault mid-stream. The format of the string is covered by the unit tests above. Happy to add a manual run if a maintainer wants one and can suggest a reliable way to trigger a mid-stream disconnect.

CHANGELOG-NONE

meehol added 2 commits October 2, 2026 23:56
When a MultiAgent response stream fails and Warp retries, resumes, or gives
up, the log line identified the failed request only as original or resume.
It did not say which model the request targeted or whether it carried
user-provided API keys or custom model providers, so transport failures such
as the TLS UnexpectedEof reports in warpdotdev#7247 could not be attributed to hosted
versus bring-your-own-key requests from a user's log alone.

Append model, user_api_keys, and custom_model_providers to both the recovering
and not-recovering log lines. Only the presence of credentials is logged,
never their values, and a test asserts a credential value does not appear in
the label.

Refs warpdotdev#7247
@cla-bot

cla-bot Bot commented Oct 3, 2026

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @meehol on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Oct 3, 2026
@warp-for-oss

warp-for-oss Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

@meehol

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @coolcom200.

Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@warp-for-oss warp-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This PR adds model and credential-presence context to response-stream recovery warning logs, while keeping credential values out of the logged label. It also adds focused unit coverage for the hosted-request case and for avoiding leakage of an attached credential value.

Concerns

  • No blocking concerns found. The added helper doc comment explains the diagnostic rationale rather than restating implementation, and the new tests cover the new deterministic label behavior.
  • No approved spec context was provided for this PR.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@warp-for-oss
warp-for-oss Bot requested a review from coolcom200 October 3, 2026 04:59
@meehol

meehol commented Oct 3, 2026

Copy link
Copy Markdown
Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label Oct 3, 2026
@cla-bot

cla-bot Bot commented Oct 3, 2026

Copy link
Copy Markdown

The cla-bot has been summoned, and re-checked this pull request!

@meehol meehol mentioned this pull request Oct 3, 2026
3 of 4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request failed with error: Transport

1 participant