-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: preserve GPT-5 conversation continuity after subtask completion #8403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Modified completeSubtask to check if the model is GPT-5 before setting skipPrevResponseIdOnce - GPT-5 models now maintain previous_response_id chain after subtask completion - Non-GPT-5 models continue to skip previous_response_id as before - Added comprehensive tests for both GPT-5 and non-GPT-5 behavior Fixes #8402
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review initialized: auditing my own code with the emotional range of a diff algorithm.
| @@ -0,0 +1 @@ | |||
| Subproject commit e46929b8d8add0cd3c412d69f8ac882c405a4ba9 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Unrelated git submodule/gitsubtree artifact committed. No matching .gitmodules update and not referenced by the code changes. Please drop this file from the PR.
| @@ -0,0 +1 @@ | |||
| Subproject commit 88a473b017af37091c85ce3056e444e856f80d6e | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Unrelated submodule/gitreference introduced. This appears accidental and unrelated to GPT‑5 continuity fix. Please remove from the PR.
| // GPT-5 only receives the latest message without the subtask result context. | ||
| // For other models, we skip to ensure the full conversation is sent. | ||
| const modelId = this.api.getModel().id | ||
| const isGpt5Model = modelId && modelId.startsWith("gpt-5") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] GPT‑5 detection robustness. Model ids can be prefixed or vary in case across providers. Suggest normalizing and using includes:\n\nsuggestion\n\t\t\tconst modelId = this.api.getModel().id\n\t\t\tconst isGpt5Model = typeof modelId === \"string\" && modelId.toLowerCase().includes(\"gpt-5\")\n\n\nAlso consider centralizing this in a small helper to keep parity with other call sites.
| }) | ||
|
|
||
| describe("GPT-5 Subtask Completion", () => { | ||
| it("should NOT skip previous_response_id for GPT-5 models after subtask completion", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Completeness. Add a test that, after completeSubtask() with GPT‑5, the subsequent attemptApiRequest() includes metadata.previousResponseId (and omits suppressPreviousResponseId). This validates the continuity chain beyond just the flag.
Description
This PR fixes an issue where GPT-5 models (specifically gpt-5-codex) were unable to read subtask results after completion. The root cause was that the parent task was unconditionally setting
skipPrevResponseIdOnce = trueafter a subtask completed, which broke the conversation continuity chain that GPT-5 models rely on.Problem
When using gpt-5-codex:
skipPrevResponseIdOncecauses the next API call to omit theprevious_response_id, breaking continuitySolution
Modified the
completeSubtask()method to conditionally handle theskipPrevResponseIdOnceflag:Changes
src/core/task/Task.tsto check if the model is GPT-5 before setting the skip flagsrc/core/task/__tests__/Task.spec.tscovering:Testing
Fixes #8402
Important
Fixes GPT-5 conversation continuity issue after subtask completion by conditionally setting a flag in
Task.ts.completeSubtask()inTask.ts.skipPrevResponseIdOnce.skipPrevResponseIdOnceto ensure full conversation is sent.Task.spec.tsto verify behavior for GPT-5 and non-GPT-5 models.This description was created by
for f9a8ccd. You can customize this summary. It will automatically update as commits are pushed.