fix(proxy): abandon unavailable owner on thread-scoped goal restart - #1764
fix(proxy): abandon unavailable owner on thread-scoped goal restart#1764mastertyko wants to merge 5 commits into
Conversation
Current Codex sends thread-id with the process session, so affinity classified the restart as thread_header and never retired the raw legacy owner. Grant the one-shot abandonment flag when a process session is present, allow retirement CAS from thread_header requests, and consult the raw row as session_header interpretation so later thread turns stay on the replacement.
|
@codex review |
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThread-header goal restarts can abandon unavailable raw process-session owners. Retirement and lookup use ChangesSticky restart routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change enables recovery for the affected goal-restart requests while preserving fail-closed handling for other request types. Focused tests and validation pass, and no actionable merge-blocking risk remains. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GoalRestartRequest
participant Affinity
participant AccountSelection
participant StickySelection
participant ReplacementAccount
GoalRestartRequest->>Affinity: Send process-session and thread-header identity
Affinity->>AccountSelection: Forward legacy continuity source
AccountSelection->>StickySelection: Interpret raw key as session_header
StickySelection->>StickySelection: Retire unavailable owner
AccountSelection->>ReplacementAccount: Select eligible replacement
ReplacementAccount->>GoalRestartRequest: Continue thread on replacement
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ty rejects subscripting account_ids_by_key because the stub field is optional. Compare the whole mapping like the existing session-header test.
|
@codex review |
![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: 076f0c77a5
ℹ️ 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".
Codex review: a hardcoded session_header lookup hid thread-only raw rows after a process-session tombstone, and the restart path lacked /backend-api/codex/responses coverage for session-id plus thread-id. Look up the raw key with the source that actually wrote it, and add the route-level restart plus follow-up continuity test.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7082250a9
ℹ️ 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".
Security-authorized replacement still looked up the raw process-session row with thread_header, so a session_header tombstone resurrected the retired owner as a continuity conflict. Lookup the raw key with legacy_continuity_source instead.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Current Codex sends both a shared process
session-idand a distinctthread-idon a self-contained goal restart. Affinity classified that request asthread_header, so the one-shotabandon_unavailable_legacy_ownerflag never set and retirement CAS never ran. The restart stayed fail-closed on the unavailable legacy owner.Type of change
fix:— bug fix (no behavior change beyond the bug)Linked issue: None — leftover composition hole after #1680 and #1703; no matching open issue.
OpenSpec
Change directory:
openspec/changes/goal-restart-thread-header-abandonment/Changes
thread_headerrequests. The write stays scoped tosession_headerinterpretation.session_headerinterpretation so a scoped tombstone cannot revive the retired owner on later thread-id turns.turn_state, incremental, file-pinned, conversation-bound, and unresolved tool-state requests fail-closed.Test plan
Intentionally not run locally: full
pre-commit/local-ci,ty checkof the whole tree, and dashboard/browser suites. Those stay on required GitHub CI.Related work
session_idwithoutthread-id.thread_headerwin for current Codex.session_header-only gate and does not fix this hole.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests