fix(session): stamp provider identity and refuse mismatched resume - #566
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e741f9f9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub base_url: String, | ||
| /// Auth mode the conversation used. | ||
| pub auth_mode: ApiAuthMode, |
There was a problem hiding this comment.
When two runs use the same endpoint and auth_mode but different API keys—or different users' OAuth credentials—these two fields are identical, so matches permits the resume and the stored transcript is sent to the second account despite the refusal message claiming to prevent this. Include a stable non-secret account or credential identifier in the fingerprint (without persisting the API key itself).
AGENTS.md reference: AGENTS.md:L108-L112
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn normalize_base_url(url: &str) -> String { | ||
| url.trim().trim_end_matches('/').to_ascii_lowercase() |
There was a problem hiding this comment.
For custom OpenAI-compatible endpoints whose base path is case-sensitive, such as /tenant/A versus /tenant/a, lowercasing the entire URL makes distinct services compare equal. The provider clients preserve the path's case when constructing request URLs, so this can bypass the new mismatch refusal; normalize only case-insensitive URL components such as the scheme and host.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e741f9f9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub struct ProviderIdentity { | ||
| /// Resolved API base URL (trailing slashes normalized on compare). | ||
| pub base_url: String, | ||
| /// Auth mode the conversation used. | ||
| pub auth_mode: ApiAuthMode, |
There was a problem hiding this comment.
When a user restarts with a different API key or signs into another OAuth account on the same base URL, both base_url and auth_mode remain equal, so the resume check reports a match and sends the saved conversation using the new account despite the refusal message explicitly claiming to prevent this. Include a stable, non-secret account/credential identifier in the fingerprint without persisting the raw credential.
AGENTS.md reference: AGENTS.md:L108-L112
Useful? React with 👍 / 👎.
| plan_mode, | ||
| label, | ||
| tags, | ||
| provider: provider.or(prior_provider), |
There was a problem hiding this comment.
When a custom base_url contains basic-auth credentials or a token-like query parameter, serialize_masked rewrites that newly stored provider URL before it reaches disk; the next resume compares the redacted URL with the live unredacted URL and permanently rejects the session. Store a stable non-secret fingerprint or compare consistently redacted identities rather than persisting the comparison value in a form the masker mutates.
AGENTS.md reference: AGENTS.md:L108-L112
Useful? React with 👍 / 👎.
| fn normalize_base_url(url: &str) -> String { | ||
| url.trim().trim_end_matches('/').to_ascii_lowercase() |
There was a problem hiding this comment.
When two custom endpoints differ only by case in their path, query, username, or password, lowercasing the entire URL makes them compare equal even though HTTP URL paths and the remaining components can be case-sensitive. The resume guard can therefore accept a session for a distinct service binding; normalize only case-insensitive components such as the scheme and host.
Useful? React with 👍 / 👎.
Resuming only restored the model name, so a conversation saved against one resolved endpoint/auth mode could be sent to whatever the process was currently bound to. Stamp base_url + auth_mode from the live engine on every full save, and refuse resume when that fingerprint does not match the running process (resolved-vs-resolved — not destination file vs running config, which is unsound). Legacy sessions without a stamp warn and continue.
0e741f9 to
a964e96
Compare
1 participant
Summary
SessionDatanow carries an optionalproviderfingerprint (base_url+auth_mode) stamped from the live resolved engine config at save time (/fork, scheduled runs, etc.)./resume) compares that fingerprint to the running resolved config and refuses on mismatch with a message naming both sides.Fixes #537 (refusal half; full mid-session provider rebuild remains future work).
Test plan