fix(compact): sanitize plaintext compaction replays - #1720
Conversation
cabe4ad to
a4d37ff
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cabe4ad369
ℹ️ 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".
a4d37ff to
a06a097
Compare
a06a097 to
cb7c9ae
Compare
cb7c9ae to
75e7997
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75e7997236
ℹ️ 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".
5a2b9d0 to
0ff7778
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ff7778a47
ℹ️ 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".
| request_state.preferred_account_id = None | ||
| request_state.excluded_account_ids.add(session.account.id) |
There was a problem hiding this comment.
Keep recovery-lane reconnects on their current account
When a request is already running in an account-neutral recovery lane and its current upstream remains silent, this branch now clears the preferred owner and explicitly excludes the lane's current account, allowing the same recovered task to move again. That contradicts openspec/changes/classify-required-http-bridge-owner-unavailable/specs/responses-api-compat/spec.md:147-160, which requires reconnects for an established recovery lane to retain its current account and typed ownership provenance; otherwise a second ambiguous resend can lose the recovered turn state or duplicate work on another account.
AGENTS.md reference: AGENTS.md:L105-L110
Useful? React with 👍 / 👎.
381247d to
db7ce90
Compare
Summary
compactionitemsgAAAA...) unchanged on both normal/responsescontinuation and compact requestsThis replaces the earlier too-broad encrypted-state rewrite: valid provider compact state is authoritative continuity and must stay opaque, not become
[compact state] [unverified compact state omitted].Live failure and follow-up
The original invalid-encrypted-content failures were caused by replaying local/plaintext compact fallback state as a provider compaction item. A later live post-compact hang showed the opposite hazard too: replacing valid provider encrypted compact state broke continuation context. This PR now keeps the two cases separate.
Testing
uv run pytest tests/unit/test_openai_requests.py::test_compact_rewrites_plaintext_compaction_replay_without_encrypted_content tests/unit/test_openai_requests.py::test_compact_preserves_provider_encrypted_compaction_replay tests/unit/test_openai_requests.py::test_responses_preserves_provider_encrypted_compaction_replay tests/unit/test_openai_requests.py::test_compact_strips_poisoned_local_compact_fallback_items tests/unit/test_openai_requests.py::test_responses_strips_poisoned_local_compact_fallback_items-> 5 passeduv run pytest tests/unit/test_openai_requests.py-> 172 passedgit diff --name-only origin/main...HEADcontains onlyapp/core/openai/requests.pyandtests/unit/test_openai_requests.py