Skip to content

Commit a71761a

Browse files
easonLiangWorldedtecheasonLiangWorldedtech
committed
Merge upstream/main (a0d03a0) into fws/u6-apply-patch-wiring
The only conflicted path was src/core/tools/ApplyDiffTool.ts and it conflicted in its import block: this branch added versionTokenOfStat for the read/observe authorization, upstream 1b9cb7d added isPathOutsideWorkspace for the outside-workspace auto-approve guard. Both imports are kept, so both semantics survive the merge - upstream's isOutsideWorkspace flag on the appliedDiff payload (:168/:174 and :373) and this branch's stat-around-read authorization (:82-:101). Task.ts, applyDiffTool.failureReporting.spec.ts and src/eslint-suppressions.json auto-merged; nothing on the review side was answered or resolved by this merge. Two test files needed integration work, in setup only, no assertion removed: - applyDiffTool.outsideWorkspace.spec.ts arrived from upstream with a Task double that predates this branch's tool, so it now also carries the stat result and an observationRegistry, because the merged tool stats around its own read and consults the registry before saving. - guardedWrite.spec.ts wrote task.abort = true at two rows. Task.ts:3587 (main) exposes abort as an accessor over the run state, so writing it is a TS2540 on a Task-typed double. The double now carries the accessor plus the state holder it reads, and the two rows flip that holder instead of writing the accessor. Same mid-flight abort, same assertions. After both: tsc --noEmit -p tsconfig.json is exit 0 on the merged tree, all 68 test files under core/tools/__tests__ and core/task/__tests__ pass (1424 passed, 5 skipped), and eslint --max-warnings=0 and prettier --check are exit 0 on every file both sides changed. Controls against the merged tree, both reverted: forcing the payload flag to false takes outsideWorkspace.spec.ts to 6 failed, and skipping the stat-around-read block takes failureReporting/guardedWrite to 1 failed, so neither semantic is carried by accident. The suppression records did not rise: 265 file records with a summed count of 2866 on this merge against 2868 on main and on the branch head. The narrow --prune-suppressions runs were checks only and were reverted.
2 parents 42a1bbb + a0d03a0 commit a71761a

34 files changed

Lines changed: 900 additions & 193 deletions

‎.github/workflows/code-qa.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,11 @@ jobs:
247247
- name: Verify coverage cache inputs
248248
if: matrix.collect-coverage
249249
run: pnpm --dir src run verify:coverage-cache-inputs
250+
# Validate build/static cache inputs (issue #2290 contract) so a
251+
# stale webview or extension bundle is never restored as a hit.
252+
- name: Verify build cache inputs
253+
if: matrix.collect-coverage
254+
run: pnpm --dir src run verify:build-cache-inputs
250255
- name: Save Turbo cache
251256
if: steps.turbo-cache.outputs.cache-hit != 'true'
252257
uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0

‎apps/vscode-e2e/AGENTS.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,13 @@ The `model` field can be added to either match when a test targets a specific mo
140140
Use `streamingProfile: { ttft: <ms> }` on a fixture, not a flat `latency: <ms>`, when a test needs
141141
to simulate a slow or hung provider (e.g. to cancel an in-flight request mid-stream). `ttft` delays
142142
only the first SSE chunk, so the pending window is exactly the configured value. Flat `latency`
143-
delays _every_ chunk, and aimock never observes client disconnects — after a test cancels the
144-
request, a flat-latency stream keeps flushing chunks server-side for `chunks × latency` before
145-
reaching the dead socket, which can interleave with the next test's request against the same mock
146-
server. See `SUBTASK_API_HANG_RESPONSE_LATENCY_MS` in `fixtures/subtasks.ts` for an example,
147-
including the bounded post-test drain the calling suite uses to wait out that window.
143+
delays _every_ chunk. Before aimock 1.47.0, the mock did not observe client disconnects — after a
144+
test cancelled the request, a flat-latency stream kept flushing chunks server-side for
145+
`chunks × latency` before reaching the dead socket, which could interleave with the next test's
146+
request against the same mock server. Version 1.47.0 stops the reply on disconnect, but the
147+
suite keeps its drain wait until an e2e run shows it is not needed. See
148+
`SUBTASK_API_HANG_RESPONSE_LATENCY_MS` in `fixtures/subtasks.ts` for an example, including the
149+
bounded post-test drain the calling suite uses to wait out that window.
148150

149151
## 404 errors in logs are expected
150152

‎apps/vscode-e2e/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"@roo-code/config-eslint": "workspace:^",
2626
"@roo-code/config-typescript": "workspace:^",
2727
"@roo-code/types": "workspace:^",
28-
"@copilotkit/aimock": "1.35.0",
28+
"@copilotkit/aimock": "1.47.0",
2929
"@types/mocha": "10.0.10",
3030
"@types/node": "catalog:",
3131
"@types/vscode": "1.100.0",

‎apps/vscode-e2e/src/suite/subtasks.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,11 @@ const waitForAimockRequestContaining = async (
9393
// suite becomes flaky again on slow CI runners, widen this value first.
9494
const SUBTASK_API_HANG_DRAIN_GRACE_MS = 500
9595

96-
// aimock does not observe client disconnects: after the API-hang child request is cancelled,
97-
// the mock keeps the delayed stream pending server-side until the fixture's ttft has fully
98-
// elapsed, then flushes the remaining chunks to the dead socket. A streamed request opened by
99-
// the next test can interleave with that late flush, so wait out the remainder of the delayed
96+
// aimock 1.47.0 stops and journals a reply when the client disconnects. Before that, the mock
97+
// kept the delayed stream pending server-side until the fixture's ttft had fully elapsed, then
98+
// flushed the remaining chunks to the dead socket. A streamed request opened by the next test
99+
// could interleave with that late flush. This wait is a conservative guard kept until an e2e
100+
// run shows it is not needed: wait out the remainder of the delayed
100101
// window before the next test runs. The deadline is anchored to the journal timestamp of the
101102
// request this test created (never earlier traffic), and bounded by one latency window plus
102103
// grace, so it cannot hide a genuine hang.

‎pnpm-lock.yaml‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/core/task/Task.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages"
145145
import { prepareApiConversationMessage } from "./apiConversationHistory"
146146
import { shouldAddUserMessageToHistory } from "./messageCounting"
147147
import { type TaskExecutionContext } from "./providerHandoff"
148-
import { initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState"
148+
import { abort, abortReason, initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState"
149149
import { applyRunEvent, type RunRejection } from "./run-state/runStateShadow"
150150
import {
151151
divergenceKey,
@@ -397,7 +397,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
397397

398398
providerRef: WeakRef<ClineProvider>
399399
private readonly globalStoragePath: string
400-
abort: boolean = false
401400
currentRequestAbortController?: AbortController
402401
/**
403402
* Controller for the waiter on an in-flight `ensureModelFetched()` call (see
@@ -415,7 +414,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
415414

416415
didFinishAbortingStream = false
417416
abandoned = false
418-
abortReason?: ClineApiReqCancelReason
419417
isInitialized = false
420418

421419
runState: RunState = initialRunState
@@ -3586,16 +3584,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
35863584
this.driveRunEvent({ tag: "completionAccepted", askTs })
35873585
}
35883586

3587+
get abort(): boolean {
3588+
return abort(this.runState)
3589+
}
3590+
3591+
get abortReason(): ClineApiReqCancelReason | undefined {
3592+
return abortReason(this.runState)
3593+
}
3594+
35893595
setAbortReason(reason: ClineApiReqCancelReason): void {
3590-
this.abortReason ??= reason
35913596
this.driveRunEvent({ tag: "reasonSet", reason })
35923597
}
35933598

35943599
requestAbort(isAbandoned: boolean): void {
35953600
if (isAbandoned) {
35963601
this.abandoned = true
35973602
}
3598-
this.abort = true
35993603
this.driveRunEvent({ tag: "abortRequested", abandoned: isAbandoned })
36003604
}
36013605

@@ -3605,7 +3609,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
36053609
}
36063610

36073611
requestDispose(): void {
3608-
this.abort = true
36093612
this.driveRunEvent({ tag: "disposeRequested" })
36103613
}
36113614

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { ContextProxy } from "../../config/ContextProxy"
2121
import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
2222
import { attemptCompletionTool, type AttemptCompletionCallbacks } from "../../tools/AttemptCompletionTool"
2323
import type { ToolUse } from "../../../shared/tools"
24+
import { setAbortLatch } from "../../../test-utils/run-state"
2425

2526
type TaskPersistenceAccess = {
2627
addToApiConversationHistory: (message: Anthropic.MessageParam) => Promise<void>
@@ -909,7 +910,7 @@ describe("Task persistence", () => {
909910
task: "test task",
910911
startTask: false,
911912
})
912-
task.abort = true
913+
setAbortLatch(task, true)
913914
task.userMessageContent = [{ type: "tool_result", tool_use_id: "tool-1", content: "done" }]
914915
const waitForPersistence = vi.spyOn(task, "waitForCurrentAssistantMessagePersistence")
915916

‎src/core/task/__tests__/Task.run-shadow.spec.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,14 @@ describe("Task run-state write helpers", () => {
101101
task.requestAbort(false)
102102

103103
expect([task.abort, task.abandoned]).toEqual([true, false])
104-
expect(task.runState.latches).toMatchObject({ abort: true, abandoned: false })
104+
expect(task.runState.latches.abandoned).toBe(false)
105105
})
106106

107107
it("requestAbort(true) sets abort and abandoned", () => {
108108
task.requestAbort(true)
109109

110110
expect([task.abort, task.abandoned]).toEqual([true, true])
111-
expect(task.runState.latches).toMatchObject({ abort: true, abandoned: true })
111+
expect(task.runState.latches.abandoned).toBe(true)
112112
})
113113

114114
it("requestAbandon sets abandoned after an abort", () => {
@@ -124,7 +124,7 @@ describe("Task run-state write helpers", () => {
124124
task.requestDispose()
125125

126126
expect(task.abort).toBe(true)
127-
expect(task.runState.latches).toMatchObject({ abort: true, disposed: true })
127+
expect(task.runState.latches.disposed).toBe(true)
128128
})
129129

130130
it("drives the ask events", () => {
@@ -149,12 +149,15 @@ describe("Task run-state write helpers", () => {
149149
expect(task.runRejections).toEqual([{ event: { tag: "abandonRequested" }, state: initialRunState }])
150150
})
151151

152-
it("keeps the first abort reason in the field and in the kernel", () => {
152+
it("has no abort reason before one is set", () => {
153+
expect(task.abortReason).toBeUndefined()
154+
})
155+
156+
it("keeps the first abort reason", () => {
153157
task.setAbortReason("user_cancelled")
154158
task.setAbortReason("streaming_failed")
155159

156160
expect(task.abortReason).toBe("user_cancelled")
157-
expect(task.runState.latches.reason).toBe("user_cancelled")
158161
expect(task.runRejections).toEqual([])
159162
})
160163
})

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ vi.mock("delay", () => ({
9999

100100
import delay from "delay"
101101
import pWaitFor from "p-wait-for"
102+
import { setAbortLatch } from "../../../test-utils/run-state"
102103

103104
vi.mock("uuid", async (importOriginal) => {
104105
const actual = await importOriginal<typeof import("uuid")>()
@@ -1071,7 +1072,7 @@ describe("Cline", () => {
10711072
const saySpy = vi.spyOn(task, "say").mockImplementation(async (...args) => {
10721073
const [type, _text, _images, partial] = args
10731074
if (type === "text" && partial === false) {
1074-
task.abort = true
1075+
setAbortLatch(task, true)
10751076
throw new Error("presenter text failed while aborting")
10761077
}
10771078
return say(...args)
@@ -4922,7 +4923,7 @@ describe("Cline", () => {
49224923
const abortTaskSpy = vi.spyOn(task, "abortTask").mockResolvedValue(undefined)
49234924

49244925
// Test Case 1: Stream failure should NOT abort task
4925-
task.abort = false
4926+
setAbortLatch(task, false)
49264927
task.abandoned = false
49274928

49284929
// Simulate the catch block behavior for stream failure
@@ -4943,7 +4944,7 @@ describe("Cline", () => {
49434944
expect(abortTaskSpy).not.toHaveBeenCalled()
49444945

49454946
// Test Case 2: User cancellation SHOULD abort task
4946-
task.abort = true
4947+
setAbortLatch(task, true)
49474948

49484949
// For user cancellation, abortTask SHOULD be called
49494950
if (task.abort) {
@@ -7344,7 +7345,7 @@ describe("Cline", () => {
73447345
await flushMicrotasks()
73457346
consoleErrorSpy.mockClear()
73467347

7347-
task.abort = true
7348+
setAbortLatch(task, true)
73487349
getTaskTestAccess(task).presentAssistantMessageSafe()
73497350
await flushMicrotasks()
73507351

@@ -7398,7 +7399,7 @@ describe("Cline", () => {
73987399
consoleErrorSpy.mockClear()
73997400

74007401
// Simulate the TOCTOU race: abort flips between throw and catch.
7401-
task.abort = true
7402+
setAbortLatch(task, true)
74027403
getTaskTestAccess(task).presentAssistantMessageSafe()
74037404
await flushMicrotasks()
74047405

‎src/core/task/__tests__/ask-abort-race.spec.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { ClineMessage } from "@roo-code/types"
33
import { checkAutoApproval } from "../../auto-approval"
44
import { MessageQueueService } from "../../message-queue/MessageQueueService"
55
import { Task } from "../Task"
6+
import { setAbortLatch } from "../../../test-utils/run-state"
67

78
vi.mock("../../auto-approval", async (importOriginal) => {
89
const actual = await importOriginal<typeof import("../../auto-approval")>()
@@ -11,7 +12,7 @@ vi.mock("../../auto-approval", async (importOriginal) => {
1112

1213
function buildTask(getState: () => Promise<Record<string, unknown>>) {
1314
const task = Object.create(Task.prototype) as Task
14-
task["abort"] = false
15+
setAbortLatch(task, false)
1516
task["clineMessages"] = []
1617
task["lastMessageTs"] = undefined
1718
task["addToClineMessages"] = vi.fn(async () => {})
@@ -44,7 +45,7 @@ describe("Task.ask abort during the auto-approval await", () => {
4445
})
4546

4647
const askPromise = task.ask("tool", JSON.stringify({ tool: "readFile" }), partial)
47-
task["abort"] = true
48+
setAbortLatch(task, true)
4849
gate.release()
4950

5051
await expect(askPromise).rejects.toThrow(/aborted/)
@@ -62,7 +63,7 @@ describe("Task.ask abort during the auto-approval await", () => {
6263

6364
const askPromise = task.ask("tool", JSON.stringify({ tool: "readFile" }), false)
6465
await vi.waitFor(() => expect(checkAutoApproval).toHaveBeenCalled())
65-
task["abort"] = true
66+
setAbortLatch(task, true)
6667
gate.release()
6768

6869
await expect(askPromise).rejects.toThrow(/aborted/)
@@ -83,7 +84,7 @@ describe("Task.ask abort during the auto-approval await", () => {
8384
task["clineMessages"] = [{ ...original }]
8485

8586
const askPromise = task.ask("followup", "new", partial)
86-
task["abort"] = true
87+
setAbortLatch(task, true)
8788
gate.release()
8889

8990
await expect(askPromise).rejects.toThrow(/aborted/)
@@ -102,7 +103,7 @@ describe("Task.ask abort during the auto-approval await", () => {
102103
task.messageQueueService.addMessage("queued feedback")
103104

104105
const askPromise = task.ask("followup", "Q?", false)
105-
task["abort"] = true
106+
setAbortLatch(task, true)
106107
gate.release()
107108

108109
await expect(askPromise).rejects.toThrow(/aborted/)

0 commit comments

Comments
 (0)