fix(coderd/x/chatd): make OpenAI computer use stateless#25624
fix(coderd/x/chatd): make OpenAI computer use stateless#25624ibetitsmike wants to merge 2 commits into
Conversation
|
/coder-agents-review |
|
Review posted | Chat Review historydeep-review v0.5.0 | Round 3 | Last posted: Round 3, 13 findings (1 P2, 7 P3, 5 Nit), APPROVE. Review Finding inventoryFindings
Contested and acknowledged(none) Round logRound 1Panel (13 reviewers). 0 P0-P1, 1 P2, 5 P3, 3 Nit, 0 dropped. Netero clean. Reviewed against 0d9718e..a914786. Round 2Panel (13 reviewers). 0 P0-P2, 1 P3, 2 Nit new. 1 dropped. All 10 R1 findings fixed. Netero clean. Reviewed against 0d9718e..b92e676. Round 3Panel (10 reviewers). No new findings. All 13 R1-R2 findings fixed. Netero clean. Reviewed against 0d9718e..df1da85. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
There was a problem hiding this comment.
Clean Netero pass. The design is well-reasoned: disabling storage before chain-mode selection is the right causal fix, and the reasoning preservation is the necessary companion. Test ratio is strong at 1:3.2. Six reviewers independently flagged the same silent no-op in WithStorageDisabled (CRF-1), which is the strongest signal from this round.
Severity count: 1 P2, 5 P3, 3 Nit. No blockers.
"A function named WithStorageDisabled that doesn't disable storage in all code paths is a dishonest contract." (Knov)
🤖 This review was automatically generated with Coder Agents.
OpenAI computer-use turns must not combine store=false with previous_response_id. Force OpenAI computer-use provider options to disable store and clear previous_response_id before chain-mode selection. Preserve OpenAI reasoning replay metadata even when the visible reasoning text is empty so stateless manual replay keeps the required reasoning item before provider computer calls.
a914786 to
b92e676
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
All 10 R1 findings fixed. Every fix verified by the panel (13 reviewers, unanimous confirmation). The CRF-1 fix is particularly well done: removing the !ok guard so unknown types fall through to model-based option creation, rather than just logging a warning.
R2 found 1 P3 and 2 Nits (plus 1 dropped). All minor polish.
"Boring, correct code. Silence earned." (Hisoka)
🤖 This review was automatically generated with Coder Agents.
Tighten OpenAI-only store option creation, rename tests to use Store terminology, and simplify the computer-use provider options comment.
|
/coder-agents-review |
There was a problem hiding this comment.
All 13 prior findings (10 from R1, 3 from R2) fixed and verified. R3 panel (10 reviewers) found no new issues. Netero clean across all 3 rounds.
14 findings total across 3 rounds: 1 P2, 6 P3, 4 Nit, 2 dropped, 1 non-postable. All addressed. The code is clean, well-tested (1:4.2 test-to-code ratio), and correctly structured.
"I tried to build a case against this change and couldn't. The problem is correctly understood, the solution is proportional, and the fix is at the right causal level." (Pariston, R3)
🤖 This review was automatically generated with Coder Agents.
OpenAI computer-use chats now run stateless against OpenAI by forcing
store=falseand clearingprevious_response_idbefore chain-mode selection. This avoids invalid OpenAI Responses requests that combinestore=falsewithprevious_response_id.This also preserves empty OpenAI reasoning items that carry response item IDs, so stateless replay keeps the reasoning item required before provider computer calls.