Skip to content

Commit f9a8ccd

Browse files
committed
fix: preserve GPT-5 conversation continuity after subtask completion
- 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
1 parent 702b269 commit f9a8ccd

File tree

4 files changed

+139
-3
lines changed

4 files changed

+139
-3
lines changed

.review/pr-8274

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit e46929b8d8add0cd3c412d69f8ac882c405a4ba9

src/core/task/Task.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,9 +1666,20 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
16661666
content: [{ type: "text", text: `[new_task completed] Result: ${lastMessage}` }],
16671667
})
16681668

1669-
// Set skipPrevResponseIdOnce to ensure the next API call sends the full conversation
1670-
// including the subtask result, not just from before the subtask was created
1671-
this.skipPrevResponseIdOnce = true
1669+
// For GPT-5 models, we should NOT skip the previous_response_id after subtask completion
1670+
// because GPT-5 relies on response continuity to maintain context. When we skip it,
1671+
// GPT-5 only receives the latest message without the subtask result context.
1672+
// For other models, we skip to ensure the full conversation is sent.
1673+
const modelId = this.api.getModel().id
1674+
const isGpt5Model = modelId && modelId.startsWith("gpt-5")
1675+
1676+
if (!isGpt5Model) {
1677+
// Only skip previous_response_id for non-GPT-5 models
1678+
// This ensures the next API call sends the full conversation
1679+
// including the subtask result, not just from before the subtask was created
1680+
this.skipPrevResponseIdOnce = true
1681+
}
1682+
// For GPT-5 models, we maintain continuity by keeping the previous_response_id chain intact
16721683
} catch (error) {
16731684
this.providerRef
16741685
.deref()

src/core/task/__tests__/Task.spec.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,4 +1776,127 @@ describe("Cline", () => {
17761776
consoleErrorSpy.mockRestore()
17771777
})
17781778
})
1779+
1780+
describe("GPT-5 Subtask Completion", () => {
1781+
it("should NOT skip previous_response_id for GPT-5 models after subtask completion", async () => {
1782+
// Create a mock GPT-5 API configuration
1783+
const gpt5ApiConfig = {
1784+
apiProvider: "openai-native" as const,
1785+
apiModelId: "gpt-5-codex",
1786+
openAiNativeApiKey: "test-key",
1787+
}
1788+
1789+
// Create parent task with GPT-5 model
1790+
const parentTask = new Task({
1791+
provider: mockProvider,
1792+
apiConfiguration: gpt5ApiConfig,
1793+
task: "parent task",
1794+
startTask: false,
1795+
})
1796+
1797+
// Mock the API model to return GPT-5
1798+
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
1799+
id: "gpt-5-codex",
1800+
info: {
1801+
contextWindow: 128000,
1802+
maxTokens: 4096,
1803+
inputPrice: 0.25,
1804+
outputPrice: 0.75,
1805+
} as any,
1806+
})
1807+
1808+
// Spy on say and addToApiConversationHistory
1809+
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
1810+
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
1811+
1812+
// Call completeSubtask
1813+
await parentTask.completeSubtask("Subtask completed successfully")
1814+
1815+
// Verify the subtask result was added
1816+
expect(saySpy).toHaveBeenCalledWith("subtask_result", "Subtask completed successfully")
1817+
expect(addToApiSpy).toHaveBeenCalledWith({
1818+
role: "user",
1819+
content: [{ type: "text", text: "[new_task completed] Result: Subtask completed successfully" }],
1820+
})
1821+
1822+
// Verify skipPrevResponseIdOnce is NOT set for GPT-5 models
1823+
expect((parentTask as any).skipPrevResponseIdOnce).toBe(false)
1824+
})
1825+
1826+
it("should skip previous_response_id for non-GPT-5 models after subtask completion", async () => {
1827+
// Create a mock non-GPT-5 API configuration
1828+
const claudeApiConfig = {
1829+
apiProvider: "anthropic" as const,
1830+
apiModelId: "claude-3-5-sonnet-20241022",
1831+
apiKey: "test-key",
1832+
}
1833+
1834+
// Create parent task with Claude model
1835+
const parentTask = new Task({
1836+
provider: mockProvider,
1837+
apiConfiguration: claudeApiConfig,
1838+
task: "parent task",
1839+
startTask: false,
1840+
})
1841+
1842+
// Mock the API model to return Claude
1843+
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
1844+
id: "claude-3-5-sonnet-20241022",
1845+
info: {
1846+
contextWindow: 200000,
1847+
maxTokens: 4096,
1848+
inputPrice: 0.25,
1849+
outputPrice: 0.75,
1850+
} as any,
1851+
})
1852+
1853+
// Spy on say and addToApiConversationHistory
1854+
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
1855+
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
1856+
1857+
// Call completeSubtask
1858+
await parentTask.completeSubtask("Subtask completed successfully")
1859+
1860+
// Verify the subtask result was added
1861+
expect(saySpy).toHaveBeenCalledWith("subtask_result", "Subtask completed successfully")
1862+
expect(addToApiSpy).toHaveBeenCalledWith({
1863+
role: "user",
1864+
content: [{ type: "text", text: "[new_task completed] Result: Subtask completed successfully" }],
1865+
})
1866+
1867+
// Verify skipPrevResponseIdOnce IS set for non-GPT-5 models
1868+
expect((parentTask as any).skipPrevResponseIdOnce).toBe(true)
1869+
})
1870+
1871+
it("should handle edge case where model ID is undefined", async () => {
1872+
// Create task with minimal configuration
1873+
const parentTask = new Task({
1874+
provider: mockProvider,
1875+
apiConfiguration: mockApiConfig,
1876+
task: "parent task",
1877+
startTask: false,
1878+
})
1879+
1880+
// Mock the API model to return undefined ID
1881+
vi.spyOn(parentTask.api, "getModel").mockReturnValue({
1882+
id: undefined as any,
1883+
info: {
1884+
contextWindow: 200000,
1885+
maxTokens: 4096,
1886+
inputPrice: 0.25,
1887+
outputPrice: 0.75,
1888+
} as any,
1889+
})
1890+
1891+
// Spy on say and addToApiConversationHistory
1892+
const saySpy = vi.spyOn(parentTask, "say").mockResolvedValue(undefined)
1893+
const addToApiSpy = vi.spyOn(parentTask as any, "addToApiConversationHistory").mockResolvedValue(undefined)
1894+
1895+
// Call completeSubtask
1896+
await parentTask.completeSubtask("Subtask completed")
1897+
1898+
// When model ID is undefined, should default to skipping (non-GPT-5 behavior)
1899+
expect((parentTask as any).skipPrevResponseIdOnce).toBe(true)
1900+
})
1901+
})
17791902
})

tmp/pr-8287-Roo-Code

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit 88a473b017af37091c85ce3056e444e856f80d6e

0 commit comments

Comments
 (0)