From 5c22b8df5182e8c05578d1dc9fa69ba2a502d3a3 Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 2 Sep 2026 22:58:28 +0000 Subject: [PATCH 1/8] fix: keep nested subtask delegation active --- src/__tests__/provider-delegation.spec.ts | 5 +-- src/core/webview/ClineProvider.ts | 35 ++++++++++++++----- .../ClineProvider.apiHandlerRebuild.spec.ts | 35 +++++++++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/__tests__/provider-delegation.spec.ts b/src/__tests__/provider-delegation.spec.ts index 0b7aef8775..c934170438 100644 --- a/src/__tests__/provider-delegation.spec.ts +++ b/src/__tests__/provider-delegation.spec.ts @@ -249,8 +249,9 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { // Provider-level event expect(providerEmit).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-1") - // Mode switch - expect(handleModeSwitch).toHaveBeenCalledWith("code") + // The parent has already been removed, so the mode switch must not publish a + // transient empty-task state before the child is created. + expect(handleModeSwitch).toHaveBeenCalledWith("code", undefined, { preparePendingTask: true }) }) it("posts taskHistoryItemUpdated to the webview when isViewLaunched is true", async () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 0a251aba5f..05cc220d64 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1696,15 +1696,20 @@ export class ClineProvider * @param targetTask The task whose in-memory mode should be updated. Defaults to the * current task. Pass null to apply only global mode/profile effects for a pending child. */ - public async handleModeSwitch(newMode: Mode, targetTask: Task | null | undefined = this.getCurrentTask()) { + public async handleModeSwitch( + newMode: Mode, + targetTask: Task | null | undefined = this.getCurrentTask(), + options: { preparePendingTask?: boolean } = {}, + ) { return this.enqueueProviderProfileMutation((signal) => - this.handleModeSwitchUnlocked(newMode, targetTask, signal), + this.handleModeSwitchUnlocked(newMode, targetTask, options, signal), ) } private async handleModeSwitchUnlocked( newMode: Mode, targetTask: Task | null | undefined, + options: { preparePendingTask?: boolean }, signal?: AbortSignal, ): Promise { const task = targetTask @@ -1744,7 +1749,7 @@ export class ClineProvider // If workspace lock is on, keep the current API config — don't load mode-specific config const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false) if (lockApiConfigAcrossModes) { - if (targetTask !== null) { + if (targetTask !== null && !options.preparePendingTask) { await this.postStateToWebview() } return @@ -1778,7 +1783,15 @@ export class ClineProvider if (hasActualSettings) { await this.activateProviderProfileUnlocked( { name: profile.name }, - targetTask === null ? { skipCurrentTaskRebuild: true } : undefined, + targetTask === null + ? { skipCurrentTaskRebuild: true } + : options.preparePendingTask + ? { + skipCurrentTaskRebuild: true, + applyProviderSettingsToContext: true, + suppressStatePost: true, + } + : undefined, signal, ) } else { @@ -1800,7 +1813,7 @@ export class ClineProvider } } - if (targetTask !== null) { + if (targetTask !== null && !options.preparePendingTask) { await this.postStateToWebview() } } @@ -1978,6 +1991,8 @@ export class ClineProvider persistModeConfig?: boolean persistTaskHistory?: boolean skipCurrentTaskRebuild?: boolean + applyProviderSettingsToContext?: boolean + suppressStatePost?: boolean }, ) { return this.enqueueProviderProfileMutation((signal) => @@ -1991,6 +2006,8 @@ export class ClineProvider persistModeConfig?: boolean persistTaskHistory?: boolean skipCurrentTaskRebuild?: boolean + applyProviderSettingsToContext?: boolean + suppressStatePost?: boolean }, signal?: AbortSignal, ): Promise { @@ -2001,8 +2018,10 @@ export class ClineProvider const persistModeConfig = options?.persistModeConfig ?? true const persistTaskHistory = options?.persistTaskHistory ?? true const skipCurrentTaskRebuild = options?.skipCurrentTaskRebuild ?? false + const applyProviderSettingsToContext = options?.applyProviderSettingsToContext ?? !skipCurrentTaskRebuild + const suppressStatePost = options?.suppressStatePost ?? false - if (!skipCurrentTaskRebuild) { + if (applyProviderSettingsToContext) { // See `upsertProviderProfile` for a description of what this is doing. await Promise.all([ this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), @@ -2026,7 +2045,7 @@ export class ClineProvider await this.persistStickyProviderProfileToCurrentTask(name, { skipCurrentTaskRebuild }) } - if (!skipCurrentTaskRebuild) { + if (!skipCurrentTaskRebuild && !suppressStatePost) { await this.postStateToWebview() } @@ -3885,7 +3904,7 @@ export class ClineProvider // The mode switch must happen before createTask() because the Task constructor // initializes its mode from provider.getState() during initializeTaskMode(). try { - await this.handleModeSwitch(mode as any) + await this.handleModeSwitch(mode as any, undefined, { preparePendingTask: true }) } catch (e) { this.log( `[delegateParentAndOpenChild] handleModeSwitch failed for mode '${mode}': ${ diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 99d254cb9a..8ec5d8b149 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -612,6 +612,41 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { expect(provider["providerSettingsManager"].activateProfile).toHaveBeenCalledWith({ name: "ask-profile" }) }) + test("pending child preparation applies its profile without posting an empty task state", async () => { + const unrelatedTask = new Task(defaultTaskOptions) + await provider.addClineToStack(unrelatedTask) + provider["providerSettingsManager"].getModeConfigId = vi.fn().mockResolvedValue("ask-id") + provider["providerSettingsManager"].listConfig = vi + .fn() + .mockResolvedValue([{ name: "ask-profile", id: "ask-id", apiProvider: providerIdentifiers.openrouter }]) + provider["providerSettingsManager"].getProfile = vi.fn().mockResolvedValue({ + name: "ask-profile", + id: "ask-id", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4.1-mini", + }) + provider["providerSettingsManager"].activateProfile = vi.fn().mockResolvedValue({ + name: "ask-profile", + id: "ask-id", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4.1-mini", + }) + const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + const setValueSpy = vi.spyOn(provider.contextProxy, "setValue") + const setProviderSettingsSpy = vi.spyOn(provider.contextProxy, "setProviderSettings") + postStateSpy.mockClear() + + await provider["handleModeSwitchUnlocked"]("ask" as Mode, undefined, { preparePendingTask: true }) + + expect(setValueSpy).toHaveBeenCalledWith("currentApiConfigName", "ask-profile") + expect(setProviderSettingsSpy).toHaveBeenCalledWith( + expect.objectContaining({ openRouterModelId: "openai/gpt-4.1-mini" }), + ) + expect(unrelatedTask.updateApiConfiguration).not.toHaveBeenCalled() + expect(unrelatedTask.setTaskApiConfigName).not.toHaveBeenCalled() + expect(postStateSpy).not.toHaveBeenCalled() + }) + test("calls updateApiConfiguration when provider/model unchanged but settings differ (explicit profile switch)", async () => { const mockTask = new Task({ ...defaultTaskOptions, From 74e331a38bc249b4320ec4b31ebf97eb4b616f5f Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 00:42:50 +0000 Subject: [PATCH 2/8] fix: isolate nested child mode preparation --- src/__tests__/provider-delegation.spec.ts | 2 +- src/core/webview/ClineProvider.ts | 23 +++++----- .../ClineProvider.apiHandlerRebuild.spec.ts | 45 ++++++++++++++++++- src/eslint-suppressions.json | 2 +- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/src/__tests__/provider-delegation.spec.ts b/src/__tests__/provider-delegation.spec.ts index c934170438..e3e32a2365 100644 --- a/src/__tests__/provider-delegation.spec.ts +++ b/src/__tests__/provider-delegation.spec.ts @@ -251,7 +251,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { // The parent has already been removed, so the mode switch must not publish a // transient empty-task state before the child is created. - expect(handleModeSwitch).toHaveBeenCalledWith("code", undefined, { preparePendingTask: true }) + expect(handleModeSwitch).toHaveBeenCalledWith("code", null, { preparePendingTask: true }) }) it("posts taskHistoryItemUpdated to the webview when isViewLaunched is true", async () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 05cc220d64..f36513dfe1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1781,19 +1781,16 @@ export class ClineProvider const hasActualSettings = !!fullProfile.apiProvider if (hasActualSettings) { - await this.activateProviderProfileUnlocked( - { name: profile.name }, - targetTask === null + const activationOptions = options.preparePendingTask + ? { + skipCurrentTaskRebuild: true, + applyProviderSettingsToContext: true, + suppressStatePost: true, + } + : targetTask === null ? { skipCurrentTaskRebuild: true } - : options.preparePendingTask - ? { - skipCurrentTaskRebuild: true, - applyProviderSettingsToContext: true, - suppressStatePost: true, - } - : undefined, - signal, - ) + : undefined + await this.activateProviderProfileUnlocked({ name: profile.name }, activationOptions, signal) } else { // The task will continue with the current/default configuration. } @@ -3904,7 +3901,7 @@ export class ClineProvider // The mode switch must happen before createTask() because the Task constructor // initializes its mode from provider.getState() during initializeTaskMode(). try { - await this.handleModeSwitch(mode as any, undefined, { preparePendingTask: true }) + await this.handleModeSwitch(mode, null, { preparePendingTask: true }) } catch (e) { this.log( `[delegateParentAndOpenChild] handleModeSwitch failed for mode '${mode}': ${ diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 8ec5d8b149..bd0793f9b9 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -614,6 +614,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { test("pending child preparation applies its profile without posting an empty task state", async () => { const unrelatedTask = new Task(defaultTaskOptions) + unrelatedTask["_taskMode"] = "code" as Mode await provider.addClineToStack(unrelatedTask) provider["providerSettingsManager"].getModeConfigId = vi.fn().mockResolvedValue("ask-id") provider["providerSettingsManager"].listConfig = vi @@ -632,11 +633,12 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { openRouterModelId: "openai/gpt-4.1-mini", }) const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + const updateTaskHistorySpy = vi.spyOn(provider, "updateTaskHistory") const setValueSpy = vi.spyOn(provider.contextProxy, "setValue") const setProviderSettingsSpy = vi.spyOn(provider.contextProxy, "setProviderSettings") postStateSpy.mockClear() - await provider["handleModeSwitchUnlocked"]("ask" as Mode, undefined, { preparePendingTask: true }) + await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) expect(setValueSpy).toHaveBeenCalledWith("currentApiConfigName", "ask-profile") expect(setProviderSettingsSpy).toHaveBeenCalledWith( @@ -644,6 +646,47 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { ) expect(unrelatedTask.updateApiConfiguration).not.toHaveBeenCalled() expect(unrelatedTask.setTaskApiConfigName).not.toHaveBeenCalled() + expect(unrelatedTask["_taskMode"]).toBe("code") + expect(updateTaskHistorySpy).not.toHaveBeenCalled() + expect(postStateSpy).not.toHaveBeenCalled() + }) + + test("pending child preparation keeps the current profile when the mode has no saved profile", async () => { + const unrelatedTask = new Task(defaultTaskOptions) + unrelatedTask["_taskMode"] = "code" as Mode + await provider.addClineToStack(unrelatedTask) + const activateProfileSpy = vi.spyOn(provider["providerSettingsManager"], "activateProfile") + const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + postStateSpy.mockClear() + + await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) + + expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "ask") + expect(activateProfileSpy).not.toHaveBeenCalled() + expect(unrelatedTask.updateApiConfiguration).not.toHaveBeenCalled() + expect(unrelatedTask.setTaskApiConfigName).not.toHaveBeenCalled() + expect(unrelatedTask["_taskMode"]).toBe("code") + expect(postStateSpy).not.toHaveBeenCalled() + }) + + test("pending child preparation preserves the locked profile without posting state", async () => { + const unrelatedTask = new Task(defaultTaskOptions) + unrelatedTask["_taskMode"] = "code" as Mode + await provider.addClineToStack(unrelatedTask) + vi.mocked(mockContext.workspaceState.get).mockReturnValue(true) + const getModeConfigIdSpy = vi.spyOn(provider["providerSettingsManager"], "getModeConfigId") + const activateProfileSpy = vi.spyOn(provider["providerSettingsManager"], "activateProfile") + const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + postStateSpy.mockClear() + + await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) + + expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "ask") + expect(getModeConfigIdSpy).not.toHaveBeenCalled() + expect(activateProfileSpy).not.toHaveBeenCalled() + expect(unrelatedTask.updateApiConfiguration).not.toHaveBeenCalled() + expect(unrelatedTask.setTaskApiConfigName).not.toHaveBeenCalled() + expect(unrelatedTask["_taskMode"]).toBe("code") expect(postStateSpy).not.toHaveBeenCalled() }) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 0706dbe6fb..781840a0f0 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -1026,7 +1026,7 @@ }, "core/webview/ClineProvider.ts": { "@typescript-eslint/no-explicit-any": { - "count": 12 + "count": 11 } }, "core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts": { From 825ebdecdb9d3cab37f569058b8e3bb5197e49e7 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 01:25:47 +0000 Subject: [PATCH 3/8] test: model provider delegation handoffs --- docs/architecture/task-lifecycle-model.md | 10 + package.json | 2 +- scripts/check-provider-handoff.ts | 358 ++++++++++++++++++ src/__tests__/provider-delegation.spec.ts | 6 +- src/core/task-persistence/index.ts | 9 + src/core/task-persistence/providerHandoff.ts | 53 +++ src/core/webview/ClineProvider.ts | 61 ++- .../ClineProvider.apiHandlerRebuild.spec.ts | 15 +- 8 files changed, 495 insertions(+), 19 deletions(-) create mode 100644 scripts/check-provider-handoff.ts create mode 100644 src/core/task-persistence/providerHandoff.ts diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 588ffd5204..54d2e97f53 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -37,6 +37,16 @@ The model has three fixed task slots, enough to cover competing siblings and a n Production completion also accepts a recovery-compatible `active` parent that still awaits the returning child, then clears the stale pointers. Normal model transitions never create that intermediate state, so it is covered by a focused reducer test rather than admitted as a generally valid reachable state. +## Provider handoff refinement model + +The same command runs `scripts/check-provider-handoff.ts`, a separate bounded model for the concrete provider steps that refine the atomic `delegate(parent, child)` lifecycle operation. It imports the production handoff policy and profile decision functions from `src/core/task-persistence/providerHandoff.ts`; its single persistence step calls `delegateTaskToChild` rather than duplicating the persisted transition. + +The model covers both a sole live parent and a nested parent whose removal exposes an unrelated root task. For each topology it checks saved, unsaved, and workspace-locked profile paths through these observable phases: remove parent, prepare child profile, create the paused child, persist delegation, start the child, and publish the child state. It enforces that pending preparation publishes no intermediate state, cannot mutate the exposed root task, creates the child with the requested mode and selected profile, and starts the child only after exactly one atomic delegation commit. + +An injected legacy policy retains the pre-fix implicit-current-task targeting and intermediate publication behavior without modifying repository history. The checker requires shortest counterexamples for both an empty publication after removing a sole parent and mutation of an exposed root during nested delegation. These witnesses are regression ratchets for the provider handoff policy, not generally allowed lifecycle states. + +This model deliberately keeps profile identities as opaque names/IDs and does not model API secrets, provider construction, VS Code transport latency, filesystem durability, scheduler fairness, or rollback cleanup. Focused provider tests remain responsible for proving that `ClineProvider` interprets the shared production policy correctly. + ## Shared-store concurrency model The same `pnpm lifecycle:model-check` command also runs a second bounded explorer over two `TaskHistoryStore` hosts. It imports the production `computeHistoryDelta` and `mergeHistoryDelta` functions, so its semantics match the store rather than assuming coherent caches or transactional pair writes: diff --git a/package.json b/package.json index d27f53bf21..6fa3ee47d5 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "lint": "turbo lint --log-order grouped --output-logs new-only", "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", - "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts", + "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-provider-handoff.ts && tsx scripts/check-task-store-concurrency.ts", "test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only", "format": "turbo format --log-order grouped --output-logs new-only", "build": "turbo build --log-order grouped --output-logs new-only", diff --git a/scripts/check-provider-handoff.ts b/scripts/check-provider-handoff.ts new file mode 100644 index 0000000000..209f3491bb --- /dev/null +++ b/scripts/check-provider-handoff.ts @@ -0,0 +1,358 @@ +import assert from "node:assert/strict" + +import type { HistoryItem } from "../packages/types/src/history" +import { + createProviderHandoffPlan, + decideProviderHandoffProfile, + type ProviderProfileRef, +} from "../src/core/task-persistence/providerHandoff" +import { delegateTaskToChild } from "../src/core/task-persistence/taskLifecycle" + +type TaskId = "root" | "parent" | "child" +type Topology = "sole-parent" | "exposed-root" +type ProfileScenario = "saved" | "unsaved" | "locked" +type Phase = + | "parent-open" + | "parent-removed" + | "profile-prepared" + | "child-created" + | "delegation-committed" + | "child-running" + | "settled" + +interface RuntimeTask { + mode: string + profile: string +} + +interface ModelState { + topology: Topology + scenario: ProfileScenario + phase: Phase + currentTaskId?: TaskId + rootTask: RuntimeTask + rootHistory: HistoryItem + parentHistory: HistoryItem + childTask?: RuntimeTask + childStarted: boolean + globalMode: string + globalProfile: string + modeProfileId?: string + publications: Array + refinementCommits: number +} + +interface ModelPolicy { + target: "none" | "implicit-current" + mutateExposedTask: boolean + publishWhilePending: boolean + applyProviderSettingsToContext: boolean +} + +interface TraceStep { + action: string + state: ModelState +} + +interface ModelResult { + states: number + traces: number + actions: Set +} + +interface Counterexample { + violation: string + trace: TraceStep[] +} + +const requestedMode = "child-mode" +const currentProfile: ProviderProfileRef = { name: "root-profile", id: "root-profile-id" } +const savedProfile: ProviderProfileRef = { name: "child-profile", id: "child-profile-id" } +const MAX_STATES = 100 +const actionOrder = [ + "remove-parent", + "prepare-profile", + "create-child", + "persist-delegation", + "start-child", + "publish-child", +] as const + +const legacyPolicy: ModelPolicy = { + target: "implicit-current", + mutateExposedTask: true, + publishWhilePending: true, + applyProviderSettingsToContext: true, +} + +function history(id: TaskId, parentTaskId?: TaskId): HistoryItem { + return { + id, + number: id === "root" ? 0 : id === "parent" ? 1 : 2, + ts: id === "root" ? 0 : id === "parent" ? 1 : 2, + task: id, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + status: "active", + mode: "root-mode", + parentTaskId, + rootTaskId: parentTaskId ? "root" : undefined, + childIds: [], + } +} + +function initialState(topology: Topology, scenario: ProfileScenario): ModelState { + const parentHistory = history("parent", topology === "exposed-root" ? "root" : undefined) + const rootHistory = topology === "exposed-root" ? delegateTaskToChild(history("root"), "parent") : history("root") + return { + topology, + scenario, + phase: "parent-open", + currentTaskId: "parent", + rootTask: { mode: "root-mode", profile: currentProfile.name }, + rootHistory, + parentHistory, + childStarted: false, + globalMode: "root-mode", + globalProfile: currentProfile.name, + publications: [], + refinementCommits: 0, + } +} + +function profileDecision(state: ModelState) { + return decideProviderHandoffProfile({ + locked: state.scenario === "locked", + currentProfile, + savedProfile: state.scenario === "saved" ? savedProfile : undefined, + }) +} + +function productionPolicy(): ModelPolicy { + const { policy } = createProviderHandoffPlan(requestedMode) + return { + target: policy.targetTask === null ? "none" : "implicit-current", + mutateExposedTask: policy.mutateExposedTask, + publishWhilePending: policy.publishWhilePending, + applyProviderSettingsToContext: policy.applyProviderSettingsToContext, + } +} + +function nextAction(phase: Phase): (typeof actionOrder)[number] | undefined { + switch (phase) { + case "parent-open": + return "remove-parent" + case "parent-removed": + return "prepare-profile" + case "profile-prepared": + return "create-child" + case "child-created": + return "persist-delegation" + case "delegation-committed": + return "start-child" + case "child-running": + return "publish-child" + case "settled": + return undefined + } +} + +function transition(state: ModelState, action: (typeof actionOrder)[number], policy: ModelPolicy): ModelState { + const next = structuredClone(state) + const decision = profileDecision(state) + + switch (action) { + case "remove-parent": + next.phase = "parent-removed" + next.currentTaskId = state.topology === "exposed-root" ? "root" : undefined + return next + case "prepare-profile": { + next.phase = "profile-prepared" + next.globalMode = requestedMode + if (policy.applyProviderSettingsToContext && decision.profile) { + next.globalProfile = decision.profile.name + } + if (decision.source === "unsaved-current") { + next.modeProfileId = decision.persistModeProfileId + } + if (policy.target === "implicit-current" && policy.mutateExposedTask && next.currentTaskId === "root") { + next.rootTask = { mode: requestedMode, profile: next.globalProfile } + next.rootHistory = { ...next.rootHistory, mode: requestedMode } + } + if (policy.publishWhilePending) next.publications.push(next.currentTaskId) + return next + } + case "create-child": + next.phase = "child-created" + next.currentTaskId = "child" + next.childTask = { mode: next.globalMode, profile: next.globalProfile } + return next + case "persist-delegation": + next.phase = "delegation-committed" + next.parentHistory = delegateTaskToChild(next.parentHistory, "child") + next.refinementCommits++ + return next + case "start-child": + next.phase = "child-running" + next.childStarted = true + return next + case "publish-child": + next.phase = "settled" + next.publications.push(next.currentTaskId) + return next + } +} + +function phaseAtLeast(state: ModelState, phase: Phase): boolean { + const phases: Phase[] = [ + "parent-open", + "parent-removed", + "profile-prepared", + "child-created", + "delegation-committed", + "child-running", + "settled", + ] + return phases.indexOf(state.phase) >= phases.indexOf(phase) +} + +function violations(state: ModelState): string[] { + const result: string[] = [] + const initialRoot = initialState(state.topology, state.scenario) + const decision = profileDecision(state) + const expectedProfile = decision.profile?.name ?? currentProfile.name + + if (state.publications.some((taskId) => taskId === undefined)) { + result.push("published an empty task while child handoff was pending") + } + if (state.phase !== "settled" && state.publications.length > 0) { + result.push("published state before child handoff settled") + } + if ( + JSON.stringify(state.rootTask) !== JSON.stringify(initialRoot.rootTask) || + JSON.stringify(state.rootHistory) !== JSON.stringify(initialRoot.rootHistory) + ) { + result.push("mutated the unrelated exposed root task") + } + if (phaseAtLeast(state, "profile-prepared") && state.globalProfile !== expectedProfile) { + result.push("prepared the wrong child profile") + } + if (state.scenario === "unsaved" && phaseAtLeast(state, "profile-prepared")) { + if (state.modeProfileId !== currentProfile.id) result.push("did not persist the inherited unsaved profile") + } + if (state.scenario !== "unsaved" && state.modeProfileId !== undefined) { + result.push("persisted an unexpected mode profile") + } + if (phaseAtLeast(state, "child-created")) { + if (state.childTask?.mode !== requestedMode || state.childTask.profile !== expectedProfile) { + result.push("created the child with the wrong mode or profile") + } + } + if (state.childStarted && state.refinementCommits !== 1) { + result.push("started the child before the atomic delegation commit") + } + if (phaseAtLeast(state, "delegation-committed")) { + const expectedParent = delegateTaskToChild(initialRoot.parentHistory, "child") + if (JSON.stringify(state.parentHistory) !== JSON.stringify(expectedParent)) { + result.push("delegation commit did not refine delegateTaskToChild") + } + if (state.refinementCommits !== 1) result.push("atomic delegation commit count was not exactly one") + } + if (state.phase === "settled" && state.publications.at(-1) !== "child") { + result.push("final publication did not identify the child") + } + return result +} + +function canonical(state: ModelState): string { + return JSON.stringify(state) +} + +function runModel(policy: ModelPolicy): ModelResult { + const queue: Array<{ state: ModelState; trace: TraceStep[] }> = [] + for (const topology of ["sole-parent", "exposed-root"] as const) { + for (const scenario of ["saved", "unsaved", "locked"] as const) { + const state = initialState(topology, scenario) + queue.push({ state, trace: [{ action: "initial", state }] }) + } + } + + const visited = new Set(queue.map(({ state }) => canonical(state))) + const actions = new Set() + let settledTraces = 0 + for (let index = 0; index < queue.length; index++) { + const node = queue[index]! + const found = violations(node.state) + if (found.length) throw new Error(`${found.join("; ")}\n${formatTrace(node.trace)}`) + const action = nextAction(node.state.phase) + if (!action) { + settledTraces++ + continue + } + actions.add(action) + const next = transition(node.state, action, policy) + const key = canonical(next) + if (!visited.has(key)) { + visited.add(key) + if (visited.size > MAX_STATES) { + throw new Error(`Provider handoff exploration exceeded its ${MAX_STATES}-state budget`) + } + queue.push({ state: next, trace: [...node.trace, { action, state: next }] }) + } + } + return { states: visited.size, traces: settledTraces, actions } +} + +function findCounterexample( + policy: ModelPolicy, + topology: Topology, + violationText: string, +): Counterexample | undefined { + let state = initialState(topology, "saved") + const trace: TraceStep[] = [{ action: "initial", state }] + while (true) { + const found = violations(state).find((violation) => violation === violationText) + if (found) return { violation: found, trace } + const action = nextAction(state.phase) + if (!action) return undefined + state = transition(state, action, policy) + trace.push({ action, state }) + } +} + +function formatTrace(trace: TraceStep[]): string { + return trace + .map( + ({ action, state }) => + `${action}: phase=${state.phase}, current=${state.currentTaskId ?? "none"}, rootMode=${state.rootTask.mode}, globalProfile=${state.globalProfile}, publications=${JSON.stringify(state.publications)}`, + ) + .join(" -> ") +} + +const result = runModel(productionPolicy()) +assert.deepEqual([...result.actions], actionOrder) +assert.equal(result.traces, 6) + +const emptyPublication = findCounterexample( + legacyPolicy, + "sole-parent", + "published an empty task while child handoff was pending", +) +const rootMutation = findCounterexample(legacyPolicy, "exposed-root", "mutated the unrelated exposed root task") +assert(emptyPublication) +assert(rootMutation) +assert.deepEqual( + emptyPublication.trace.map(({ action }) => action), + ["initial", "remove-parent", "prepare-profile"], +) +assert.deepEqual( + rootMutation.trace.map(({ action }) => action), + ["initial", "remove-parent", "prepare-profile"], +) + +console.log( + `Provider handoff model check passed: ${result.states} reachable states, ${result.traces} scenario traces, ${result.actions.size}/${actionOrder.length} actions reachable, 3/3 profile paths, 2/2 legacy counterexamples reproduced`, +) +console.log(`Legacy empty-publication counterexample: ${formatTrace(emptyPublication.trace)}`) +console.log(`Legacy exposed-root mutation counterexample: ${formatTrace(rootMutation.trace)}`) diff --git a/src/__tests__/provider-delegation.spec.ts b/src/__tests__/provider-delegation.spec.ts index e3e32a2365..ea624dc4a4 100644 --- a/src/__tests__/provider-delegation.spec.ts +++ b/src/__tests__/provider-delegation.spec.ts @@ -5,6 +5,7 @@ import type { HistoryItem } from "@roo-code/types" import { RooCodeEventName } from "@roo-code/types" import { ClineProvider } from "../core/webview/ClineProvider" import { TaskScheduler } from "../core/task/TaskScheduler" +import { createProviderHandoffPlan } from "../core/task-persistence/providerHandoff" const parentHistoryItem: HistoryItem = { id: "parent-1", @@ -251,7 +252,10 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { // The parent has already been removed, so the mode switch must not publish a // transient empty-task state before the child is created. - expect(handleModeSwitch).toHaveBeenCalledWith("code", null, { preparePendingTask: true }) + const handoff = createProviderHandoffPlan("code") + expect(handleModeSwitch).toHaveBeenCalledWith(handoff.requestedMode, handoff.policy.targetTask, { + pendingHandoff: handoff.policy, + }) }) it("posts taskHistoryItemUpdated to the webview when isViewLaunched is true", async () => { diff --git a/src/core/task-persistence/index.ts b/src/core/task-persistence/index.ts index 463df8a0bb..3b084a47db 100644 --- a/src/core/task-persistence/index.ts +++ b/src/core/task-persistence/index.ts @@ -2,6 +2,15 @@ export { type ApiMessage, readApiMessages, saveApiMessages } from "./apiMessages export { readTaskMessages, saveTaskMessages } from "./taskMessages" export { taskMetadata } from "./taskMetadata" export { TaskHistoryStore } from "./TaskHistoryStore" +export { + createProviderHandoffPlan, + decideProviderHandoffProfile, + getProviderHandoffActivationOptions, + PRODUCTION_PROVIDER_HANDOFF_POLICY, + type ProviderHandoffPolicy, + type ProviderHandoffProfileDecision, + type ProviderProfileRef, +} from "./providerHandoff" export { abandonDelegatedChild, assertValidTransition, diff --git a/src/core/task-persistence/providerHandoff.ts b/src/core/task-persistence/providerHandoff.ts new file mode 100644 index 0000000000..03cc3d1008 --- /dev/null +++ b/src/core/task-persistence/providerHandoff.ts @@ -0,0 +1,53 @@ +export interface ProviderProfileRef { + name: string + id?: string +} + +export interface ProviderHandoffPolicy { + targetTask: null + mutateExposedTask: boolean + publishWhilePending: boolean + applyProviderSettingsToContext: boolean +} + +export const PRODUCTION_PROVIDER_HANDOFF_POLICY = { + targetTask: null, + mutateExposedTask: false, + publishWhilePending: false, + applyProviderSettingsToContext: true, +} as const satisfies ProviderHandoffPolicy + +export function createProviderHandoffPlan(requestedMode: string) { + return { + requestedMode, + policy: PRODUCTION_PROVIDER_HANDOFF_POLICY, + } as const +} + +export type ProviderHandoffProfileDecision = + | { source: "locked-current"; profile?: ProviderProfileRef } + | { source: "saved"; profile: ProviderProfileRef } + | { source: "unsaved-current"; profile?: ProviderProfileRef; persistModeProfileId?: string } + +export function decideProviderHandoffProfile(params: { + locked: boolean + currentProfile?: ProviderProfileRef + savedProfile?: ProviderProfileRef +}): ProviderHandoffProfileDecision { + const { locked, currentProfile, savedProfile } = params + if (locked) return { source: "locked-current", profile: currentProfile } + if (savedProfile) return { source: "saved", profile: savedProfile } + return { + source: "unsaved-current", + profile: currentProfile, + persistModeProfileId: currentProfile?.id, + } +} + +export function getProviderHandoffActivationOptions(policy: ProviderHandoffPolicy) { + return { + skipCurrentTaskRebuild: !policy.mutateExposedTask, + applyProviderSettingsToContext: policy.applyProviderSettingsToContext, + suppressStatePost: !policy.publishWhilePending, + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index f36513dfe1..f21b6385e1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -116,8 +116,12 @@ import { TaskHistoryStore, abandonDelegatedChild, completeDelegatedChild, + createProviderHandoffPlan, + decideProviderHandoffProfile, delegateTaskToChild, + getProviderHandoffActivationOptions, interruptDelegatedChild, + type ProviderHandoffPolicy, } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -1699,7 +1703,7 @@ export class ClineProvider public async handleModeSwitch( newMode: Mode, targetTask: Task | null | undefined = this.getCurrentTask(), - options: { preparePendingTask?: boolean } = {}, + options: { pendingHandoff?: ProviderHandoffPolicy } = {}, ) { return this.enqueueProviderProfileMutation((signal) => this.handleModeSwitchUnlocked(newMode, targetTask, options, signal), @@ -1709,7 +1713,7 @@ export class ClineProvider private async handleModeSwitchUnlocked( newMode: Mode, targetTask: Task | null | undefined, - options: { preparePendingTask?: boolean }, + options: { pendingHandoff?: ProviderHandoffPolicy }, signal?: AbortSignal, ): Promise { const task = targetTask @@ -1749,7 +1753,17 @@ export class ClineProvider // If workspace lock is on, keep the current API config — don't load mode-specific config const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false) if (lockApiConfigAcrossModes) { - if (targetTask !== null && !options.preparePendingTask) { + if (options.pendingHandoff) { + const currentProfileName = this.getGlobalState("currentApiConfigName") + const decision = decideProviderHandoffProfile({ + locked: true, + currentProfile: currentProfileName ? { name: currentProfileName } : undefined, + }) + if (decision.source !== "locked-current") { + throw new Error("Expected locked child profile decision") + } + } + if (targetTask !== null && (options.pendingHandoff?.publishWhilePending ?? true)) { await this.postStateToWebview() } return @@ -1781,16 +1795,21 @@ export class ClineProvider const hasActualSettings = !!fullProfile.apiProvider if (hasActualSettings) { - const activationOptions = options.preparePendingTask - ? { - skipCurrentTaskRebuild: true, - applyProviderSettingsToContext: true, - suppressStatePost: true, - } + let profileName = profile.name + if (options.pendingHandoff) { + const decision = decideProviderHandoffProfile({ + locked: false, + savedProfile: { name: profile.name, id: profile.id }, + }) + if (decision.source !== "saved") throw new Error("Expected saved child profile decision") + profileName = decision.profile.name + } + const activationOptions = options.pendingHandoff + ? getProviderHandoffActivationOptions(options.pendingHandoff) : targetTask === null ? { skipCurrentTaskRebuild: true } : undefined - await this.activateProviderProfileUnlocked({ name: profile.name }, activationOptions, signal) + await this.activateProviderProfileUnlocked({ name: profileName }, activationOptions, signal) } else { // The task will continue with the current/default configuration. } @@ -1803,14 +1822,25 @@ export class ClineProvider if (currentApiConfigNameAfter) { const config = listApiConfig.find((c) => c.name === currentApiConfigNameAfter) + let configId = config?.id + if (options.pendingHandoff) { + const decision = decideProviderHandoffProfile({ + locked: false, + currentProfile: { name: currentApiConfigNameAfter, id: config?.id }, + }) + if (decision.source !== "unsaved-current") { + throw new Error("Expected unsaved child profile decision") + } + configId = decision.persistModeProfileId + } - if (config?.id) { - await this.providerSettingsManager.setModeConfig(newMode, config.id) + if (configId) { + await this.providerSettingsManager.setModeConfig(newMode, configId) } } } - if (targetTask !== null && !options.preparePendingTask) { + if (targetTask !== null && (options.pendingHandoff?.publishWhilePending ?? true)) { await this.postStateToWebview() } } @@ -3901,7 +3931,10 @@ export class ClineProvider // The mode switch must happen before createTask() because the Task constructor // initializes its mode from provider.getState() during initializeTaskMode(). try { - await this.handleModeSwitch(mode, null, { preparePendingTask: true }) + const handoff = createProviderHandoffPlan(mode) + await this.handleModeSwitch(handoff.requestedMode, handoff.policy.targetTask, { + pendingHandoff: handoff.policy, + }) } catch (e) { this.log( `[delegateParentAndOpenChild] handleModeSwitch failed for mode '${mode}': ${ diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index bd0793f9b9..dfe31e1a44 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -8,6 +8,7 @@ import { getModelId, RooCodeEventName } from "@roo-code/types" import { ContextProxy } from "../../config/ContextProxy" import type { Mode } from "../../../shared/modes" import { Task, TaskOptions } from "../../task/Task" +import { PRODUCTION_PROVIDER_HANDOFF_POLICY } from "../../task-persistence/providerHandoff" import { ClineProvider } from "../ClineProvider" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" @@ -638,7 +639,9 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { const setProviderSettingsSpy = vi.spyOn(provider.contextProxy, "setProviderSettings") postStateSpy.mockClear() - await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) + await provider.handleModeSwitch("ask" as Mode, null, { + pendingHandoff: PRODUCTION_PROVIDER_HANDOFF_POLICY, + }) expect(setValueSpy).toHaveBeenCalledWith("currentApiConfigName", "ask-profile") expect(setProviderSettingsSpy).toHaveBeenCalledWith( @@ -655,13 +658,17 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { const unrelatedTask = new Task(defaultTaskOptions) unrelatedTask["_taskMode"] = "code" as Mode await provider.addClineToStack(unrelatedTask) + await provider.contextProxy.setValue("currentApiConfigName", "test-config") const activateProfileSpy = vi.spyOn(provider["providerSettingsManager"], "activateProfile") const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) postStateSpy.mockClear() - await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) + await provider.handleModeSwitch("ask" as Mode, null, { + pendingHandoff: PRODUCTION_PROVIDER_HANDOFF_POLICY, + }) expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "ask") + expect(provider["providerSettingsManager"].setModeConfig).toHaveBeenCalledWith("ask", "test-id") expect(activateProfileSpy).not.toHaveBeenCalled() expect(unrelatedTask.updateApiConfiguration).not.toHaveBeenCalled() expect(unrelatedTask.setTaskApiConfigName).not.toHaveBeenCalled() @@ -679,7 +686,9 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) postStateSpy.mockClear() - await provider.handleModeSwitch("ask" as Mode, null, { preparePendingTask: true }) + await provider.handleModeSwitch("ask" as Mode, null, { + pendingHandoff: PRODUCTION_PROVIDER_HANDOFF_POLICY, + }) expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "ask") expect(getModeConfigIdSpy).not.toHaveBeenCalled() From 6874eac91a0e76b776e5fe9d1a73a9b944864920 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 03:44:37 +0000 Subject: [PATCH 4/8] fix(ci): test mutation gate against merge result --- .github/workflows/mutation-testing.yml | 5 ++--- scripts/stryker-diff.test.mjs | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index b4f9f47b55..66c5461307 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -25,12 +25,11 @@ jobs: echo "## Changed-code mutation testing" >> "$GITHUB_STEP_SUMMARY" echo "Mutation testing was enforced on each pull request before it entered the merge queue." >> "$GITHUB_STEP_SUMMARY" - - name: Checkout pull request head + - name: Checkout pull request merge result if: github.event_name == 'pull_request' uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.event.pull_request.head.sha }} + ref: refs/pull/${{ github.event.pull_request.number }}/merge fetch-depth: 0 persist-credentials: false diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index af1687ec68..3c664ce49b 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -4,6 +4,7 @@ import fs from "node:fs" import os from "node:os" import path from "node:path" import { describe, it } from "node:test" +import { fileURLToPath } from "node:url" import { MAX_CHANGED_LINES, @@ -22,6 +23,19 @@ import { validateDisableDirectives, } from "./stryker-diff.mjs" +const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") + +describe("mutation testing workflow", () => { + it("checks out the pull request merge result from the base repository", () => { + const workflow = fs.readFileSync(path.join(repositoryRoot, ".github/workflows/mutation-testing.yml"), "utf8") + + assert.ok(workflow.includes("- name: Checkout pull request merge result")) + assert.ok(workflow.includes("ref: refs/pull/${{ github.event.pull_request.number }}/merge")) + assert.ok(!workflow.includes("repository: ${{ github.event.pull_request.head.repo.full_name }}")) + assert.ok(!workflow.includes("ref: ${{ github.event.pull_request.head.sha }}")) + }) +}) + describe("parseNameStatus", () => { it("parses added, modified, and renamed paths", () => { assert.deepEqual( From dea463c2122134cec9c5fb62ac4d54325406a4c7 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 03:58:00 +0000 Subject: [PATCH 5/8] fix(ci): resolve package-local vitest binaries --- scripts/stryker-diff.mjs | 21 +++++++++++-- scripts/stryker-diff.test.mjs | 57 +++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index ede7d9defe..862f8f3cb1 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -304,14 +304,23 @@ export function preferDirectTestFiles(testFiles, sourceFiles) { return direct.length > 0 ? direct : testFiles } -function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) { +export function resolveVitestBinary(repoRoot, packageEntry) { + const packageRoot = path.join(repoRoot, packageEntry.root) + const runRoot = path.join(repoRoot, packageEntry.runRoot ?? packageEntry.root) + const candidates = [...new Set([runRoot, packageRoot, repoRoot])].map((root) => + path.join(root, "node_modules/.bin/vitest"), + ) + return candidates.find((candidate) => fs.existsSync(candidate)) ?? candidates.at(-1) +} + +export function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) { const packageRoot = path.join(repoRoot, packageEntry.root) const runRoot = path.join(repoRoot, packageEntry.runRoot ?? packageEntry.root) const outputFile = path.join(reportDirectory, "vitest-related.json") const configFile = path.relative(runRoot, path.join(packageRoot, packageEntry.vitestConfig)).replaceAll("\\", "/") const sourceFiles = [...new Set(packageEntry.selectors.map(selectorFile))] const result = spawnSync( - path.join(repoRoot, "node_modules/.bin/vitest"), + resolveVitestBinary(repoRoot, packageEntry), ["related", ...sourceFiles, "--run", "--config", configFile, "--reporter=json", `--outputFile=${outputFile}`], { cwd: runRoot, @@ -325,6 +334,9 @@ function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) { if (result.error?.code === "ETIMEDOUT") { throw new Error(`${packageEntry.id} related-test discovery exceeded 5 minutes`) } + if (result.error) { + throw new Error(`${packageEntry.id} related-test discovery could not start: ${result.error.message}`) + } if (result.status !== 0) { throw new Error( `${packageEntry.id} related-test discovery failed:\n${stripAnsi(`${result.stdout ?? ""}${result.stderr ?? ""}`).trim()}`, @@ -378,6 +390,11 @@ function runStryker(repoRoot, packageEntry, reportRoot, dryRunOnly) { `${packageEntry.id} mutation run exceeded 12 minutes. Split the PR or obtain a maintainer-reviewed narrow exclusion.`, ) } + if (result.error) { + throw new Error( + `${packageEntry.id} Stryker ${dryRunOnly ? "preflight" : "run"} could not start: ${result.error.message}`, + ) + } if (result.status !== 0) { throw new Error( `${packageEntry.id} Stryker ${dryRunOnly ? "preflight" : "run"} failed:\n${stripAnsi(output).trim()}`, diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index 3c664ce49b..e5cb3fb66c 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -9,7 +9,9 @@ import { fileURLToPath } from "node:url" import { MAX_CHANGED_LINES, MAX_MUTANTS, + PACKAGE_CONFIGS, buildManifest, + discoverRelatedTestFiles, evaluateReport, executableChangedLines, formatAnnotations, @@ -18,6 +20,7 @@ import { parseNameStatus, parseVitestTestFiles, preferDirectTestFiles, + resolveVitestBinary, packageForPath, selectFromGit, validateDisableDirectives, @@ -29,8 +32,13 @@ describe("mutation testing workflow", () => { it("checks out the pull request merge result from the base repository", () => { const workflow = fs.readFileSync(path.join(repositoryRoot, ".github/workflows/mutation-testing.yml"), "utf8") + assert.ok(workflow.includes(" pull_request:")) + assert.ok(!workflow.includes("pull_request_target:")) + assert.ok(workflow.includes(" contents: read")) assert.ok(workflow.includes("- name: Checkout pull request merge result")) assert.ok(workflow.includes("ref: refs/pull/${{ github.event.pull_request.number }}/merge")) + assert.ok(workflow.includes("fetch-depth: 0")) + assert.ok(workflow.includes("persist-credentials: false")) assert.ok(!workflow.includes("repository: ${{ github.event.pull_request.head.repo.full_name }}")) assert.ok(!workflow.includes("ref: ${{ github.event.pull_request.head.sha }}")) }) @@ -192,6 +200,55 @@ describe("preferDirectTestFiles", () => { }) }) +describe("related-test discovery", () => { + it("resolves Vitest from each package before falling back to the repository", () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "stryker-vitest-")) + const extension = PACKAGE_CONFIGS.find(({ id }) => id === "extension") + const webview = PACKAGE_CONFIGS.find(({ id }) => id === "webview") + const extensionBinary = path.join(repo, "src/node_modules/.bin/vitest") + const webviewBinary = path.join(repo, "webview-ui/node_modules/.bin/vitest") + const rootBinary = path.join(repo, "node_modules/.bin/vitest") + + try { + fs.mkdirSync(path.dirname(extensionBinary), { recursive: true }) + fs.mkdirSync(path.dirname(webviewBinary), { recursive: true }) + fs.writeFileSync(extensionBinary, "") + fs.writeFileSync(webviewBinary, "") + + assert.equal(resolveVitestBinary(repo, extension), extensionBinary) + assert.equal(resolveVitestBinary(repo, webview), webviewBinary) + + fs.rmSync(extensionBinary) + fs.mkdirSync(path.dirname(rootBinary), { recursive: true }) + fs.writeFileSync(rootBinary, "") + assert.equal(resolveVitestBinary(repo, extension), rootBinary) + } finally { + fs.rmSync(repo, { recursive: true, force: true }) + } + }) + + it("reports a Vitest launch error when no binary exists", () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "stryker-vitest-")) + const reportDirectory = path.join(repo, "reports") + const packageEntry = { + id: "extension", + root: "src", + vitestConfig: "vitest.config.ts", + selectors: ["utils/value.ts:1-1"], + } + + try { + fs.mkdirSync(path.join(repo, "src"), { recursive: true }) + assert.throws( + () => discoverRelatedTestFiles(repo, packageEntry, reportDirectory), + /extension related-test discovery could not start:.*ENOENT/, + ) + } finally { + fs.rmSync(repo, { recursive: true, force: true }) + } + }) +}) + describe("selectFromGit", () => { it("derives changed executable ranges from the base/head merge base", () => { const repo = fs.mkdtempSync(path.join(os.tmpdir(), "stryker-diff-")) From f04839aa559b1ef775012fccc2f371461cdffdb7 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 04:52:22 +0000 Subject: [PATCH 6/8] improve(ci): explain mutation gate failures --- .github/workflows/mutation-testing.yml | 10 ++ scripts/stryker-diff.mjs | 178 ++++++++++++++++++++++--- scripts/stryker-diff.test.mjs | 154 +++++++++++++++++++++ 3 files changed, 326 insertions(+), 16 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 66c5461307..6b7bec2303 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -58,6 +58,7 @@ jobs: run: node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" - name: Upload mutation reports + id: mutation_report if: always() && github.event_name == 'pull_request' uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: @@ -65,3 +66,12 @@ jobs: path: reports/mutation/ if-no-files-found: ignore retention-days: 7 + + - name: Link mutation report artifact + if: always() && github.event_name == 'pull_request' && steps.mutation_report.outputs.artifact-url != '' + env: + ARTIFACT_URL: ${{ steps.mutation_report.outputs.artifact-url }} + run: | + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "### Download mutation reports" >> "$GITHUB_STEP_SUMMARY" + echo "[Open the changed-code-mutation-report artifact]($ARTIFACT_URL), then open the package's mutation.html file." >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index 862f8f3cb1..c8350b1340 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -431,8 +431,7 @@ function escapeWorkflowValue(value) { .replaceAll(",", "%2C") } -export function formatAnnotations(blockingMutants, packageRoot) { - const perFile = new Map() +export function formatAnnotations(blockingMutants, packageRoot, state = { total: 0, perFile: new Map() }) { const annotations = [] for (const mutant of blockingMutants.sort((left, right) => { @@ -441,9 +440,8 @@ export function formatAnnotations(blockingMutants, packageRoot) { })) { const repositoryPath = path.posix.join(packageRoot, mutant.filePath.replaceAll("\\", "/")) const key = `${repositoryPath}:${mutant.location.start.line}` - const fileCount = perFile.get(repositoryPath) ?? 0 - if (annotations.some((annotation) => annotation.key === key) || fileCount >= 7 || annotations.length >= 20) - continue + const fileCount = state.perFile.get(repositoryPath) ?? 0 + if (annotations.some((annotation) => annotation.key === key) || fileCount >= 7 || state.total >= 20) continue const replacement = String(mutant.replacement ?? "") .replace(/\s+/g, " ") @@ -455,17 +453,59 @@ export function formatAnnotations(blockingMutants, packageRoot) { line: mutant.location.start.line, message: `${mutant.status} ${mutant.mutatorName} mutant${replacement ? ` (replacement: ${replacement})` : ""}. ` + - "Add or strengthen a focused test that fails under this mutation, or add a maintainer-approved targeted exclusion with a reason.", + "See the job summary for the complete list and resolution guidance.", }) - perFile.set(repositoryPath, fileCount + 1) + state.perFile.set(repositoryPath, fileCount + 1) + state.total++ } return annotations } -function appendSummary(rows, failures) { - if (!process.env.GITHUB_STEP_SUMMARY) return +function markdownCell(value) { + return String(value ?? "—") + .replace(/\s+/g, " ") + .trim() + .replaceAll("|", "\\|") + .slice(0, 120) +} + +export function testsFromMutationReport(report, fallback = []) { + const testFiles = Object.keys(report.testFiles ?? {}) + return testFiles.length > 0 ? testFiles : fallback +} + +export function formatBlockingMutants(blockingMutants, packageRoot) { + const grouped = new Map() + for (const mutant of [...blockingMutants].sort((left, right) => { + const pathOrder = left.filePath.localeCompare(right.filePath) + return pathOrder || left.location.start.line - right.location.start.line + })) { + const repositoryPath = path.posix.join(packageRoot, mutant.filePath.replaceAll("\\", "/")) + const group = grouped.get(repositoryPath) ?? [] + group.push(mutant) + grouped.set(repositoryPath, group) + } + const lines = [] + for (const [filePath, mutants] of grouped) { + lines.push( + `#### \`${filePath}\``, + "", + "| Line | Status | Mutator | Replacement |", + "| ---: | --- | --- | --- |", + ) + for (const mutant of mutants) { + lines.push( + `| ${mutant.location.start.line} | ${markdownCell(mutant.status)} | ${markdownCell(mutant.mutatorName)} | ${markdownCell(mutant.replacement)} |`, + ) + } + lines.push("") + } + return lines +} + +export function formatSummary(rows, failures, manifest = {}) { const lines = [ "## Changed-code mutation testing", "", @@ -478,8 +518,89 @@ function appendSummary(rows, failures) { ) } if (rows.length === 0) lines.push("| — | 0 | 0 | 0 | 0 | 0 | 0 | Not applicable |") - if (failures.length > 0) lines.push("", ...failures.map((failure) => `- ${failure}`)) - fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${lines.join("\n")}\n`) + + if (rows.length > 0) { + lines.push("", "### Focused tests") + for (const row of rows) { + const cwd = row.runRoot ?? row.root + if (row.testFiles?.length > 0) { + lines.push(`- **${row.id}** (cwd \`${cwd}\`): ${row.testFiles.map((file) => `\`${file}\``).join(", ")}`) + } else { + lines.push( + `- **${row.id}** (cwd \`${cwd}\`): the mutation run did not complete far enough to report its selected tests; use the exact reproduction command below.`, + ) + } + } + } + + const blockingRows = rows.filter((row) => row.blocking?.length > 0) + if (blockingRows.length > 0) { + lines.push( + "", + "### All surviving and uncovered mutants", + "", + "Annotations highlight up to 20 unique locations (maximum 7 per file). This summary lists every blocking mutant.", + "", + ) + for (const row of blockingRows) { + lines.push(`### ${row.id}`, "", ...formatBlockingMutants(row.blocking, row.runRoot ?? row.root)) + } + lines.push( + "### Resolve a mutation gap", + "", + "Add or strengthen a focused test that fails under the mutation. If the mutant is equivalent, request maintainer approval for the narrowest mutator-specific exclusion and explain why it cannot change behavior:", + "", + "```ts", + "// Stryker disable next-line ConditionalExpression: normalized input cannot reach the alternate branch", + "const result = condition ? value : fallback", + "```", + "", + "Broad `all` exclusions and exclusions without a concrete reason are rejected by the gate.", + ) + } + + if (manifest.baseSha && manifest.headSha) { + lines.push( + "", + "### Reproduce locally", + "", + "From a full checkout containing both commits:", + "", + "```bash", + "pnpm install --frozen-lockfile", + `node scripts/stryker-diff.mjs ci --base ${manifest.baseSha} --head ${manifest.headSha}`, + "```", + ) + } + + if (rows.length > 0) { + lines.push("", "### Mutation reports", "") + for (const row of rows) lines.push(`- **${row.id}:** \`${row.reportPath}\``) + lines.push( + "", + "The workflow uploads generated reports in the `changed-code-mutation-report` artifact. A direct artifact link appears below after upload.", + ) + } + + if (failures.length > 0) { + lines.push( + "", + "### Failures", + "", + ...failures.map((failure) => { + const detail = + failure.length > 4_000 ? `${failure.slice(0, 4_000)}\n[truncated; see the step log]` : failure + return `- ${detail.replaceAll("\n", "\n ")}` + }), + ) + } + + return `${lines.join("\n")}\n` +} + +function appendSummary(rows, failures, manifest) { + if (!process.env.GITHUB_STEP_SUMMARY) return + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, formatSummary(rows, failures, manifest)) } export function evaluateReport(report, packageEntry) { @@ -508,9 +629,13 @@ export function evaluateReport(report, packageEntry) { export function runManifest(repoRoot, manifest, reportRoot) { const rows = [] const failures = [] + const annotationState = { total: 0, perFile: new Map() } for (const packageEntry of manifest.packages) { let counts + const reportPath = path + .relative(repoRoot, path.join(reportRoot, packageEntry.id, "mutation.html")) + .replaceAll("\\", "/") try { const reportDirectory = path.join(reportRoot, packageEntry.id) fs.mkdirSync(reportDirectory, { recursive: true }) @@ -531,6 +656,11 @@ export function runManifest(repoRoot, manifest, reportRoot) { if (generatedMutants === 0) { rows.push({ id: packageEntry.id, + root: packageEntry.root, + runRoot: packageEntry.runRoot, + selectors: packageEntry.selectors, + testFiles: packageEntry.testFiles ?? [], + reportPath, changedLines: packageEntry.changedExecutableLines, valid: 0, killed: 0, @@ -543,10 +673,15 @@ export function runManifest(repoRoot, manifest, reportRoot) { } runStryker(repoRoot, packageEntry, reportRoot, false) - const reportPath = path.join(reportRoot, packageEntry.id, "mutation.json") - const report = JSON.parse(fs.readFileSync(reportPath, "utf8")) + const jsonReportPath = path.join(reportRoot, packageEntry.id, "mutation.json") + const report = JSON.parse(fs.readFileSync(jsonReportPath, "utf8")) + packageEntry.testFiles = testsFromMutationReport(report, packageEntry.testFiles) counts = mutantCounts(report) - for (const annotation of formatAnnotations(counts.blocking, packageEntry.runRoot ?? packageEntry.root)) { + for (const annotation of formatAnnotations( + counts.blocking, + packageEntry.runRoot ?? packageEntry.root, + annotationState, + )) { console.log( `::error file=${escapeWorkflowValue(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowValue(annotation.message)}`, ) @@ -554,6 +689,11 @@ export function runManifest(repoRoot, manifest, reportRoot) { evaluateReport(report, packageEntry) rows.push({ id: packageEntry.id, + root: packageEntry.root, + runRoot: packageEntry.runRoot, + selectors: packageEntry.selectors, + testFiles: packageEntry.testFiles ?? [], + reportPath, changedLines: packageEntry.changedExecutableLines, ...counts, result: "Passed", @@ -562,18 +702,24 @@ export function runManifest(repoRoot, manifest, reportRoot) { failures.push(error.message) rows.push({ id: packageEntry.id, + root: packageEntry.root, + runRoot: packageEntry.runRoot, + selectors: packageEntry.selectors, + testFiles: packageEntry.testFiles ?? [], + reportPath, changedLines: packageEntry.changedExecutableLines, valid: counts?.valid ?? 0, killed: counts?.killed ?? 0, timeout: counts?.timeout ?? 0, survived: counts?.survived ?? 0, noCoverage: counts?.noCoverage ?? 0, + blocking: counts?.blocking ?? [], result: "Failed", }) } } - appendSummary(rows, failures) + appendSummary(rows, failures, manifest) if (failures.length > 0) throw new Error(failures.join("\n")) return rows } @@ -596,7 +742,7 @@ function main() { const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation") const manifest = selectFromGit(repoRoot, baseSha, headSha) if (manifest.packages.length === 0) { - appendSummary([], []) + appendSummary([], [], manifest) console.log("No changed executable lines in mutation-tested packages; mutation testing is not applicable.") return } diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index e5cb3fb66c..e7ebbb9eaa 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -15,6 +15,8 @@ import { evaluateReport, executableChangedLines, formatAnnotations, + formatBlockingMutants, + formatSummary, mutantCounts, parseChangedLines, parseNameStatus, @@ -23,6 +25,7 @@ import { resolveVitestBinary, packageForPath, selectFromGit, + testsFromMutationReport, validateDisableDirectives, } from "./stryker-diff.mjs" @@ -41,6 +44,8 @@ describe("mutation testing workflow", () => { assert.ok(workflow.includes("persist-credentials: false")) assert.ok(!workflow.includes("repository: ${{ github.event.pull_request.head.repo.full_name }}")) assert.ok(!workflow.includes("ref: ${{ github.event.pull_request.head.sha }}")) + assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) + assert.ok(workflow.includes("open the package's mutation.html file")) }) }) @@ -320,6 +325,155 @@ describe("mutation exclusions", () => { }) }) +describe("failure output", () => { + const blocking = [ + { + filePath: "core/value.ts", + status: "Survived", + mutatorName: "ConditionalExpression", + replacement: "true", + location: { start: { line: 4 } }, + }, + { + filePath: "core/value.ts", + status: "NoCoverage", + mutatorName: "StringLiteral", + replacement: '"left | right"', + location: { start: { line: 4 } }, + }, + { + filePath: "utils/other.ts", + status: "Survived", + mutatorName: "BooleanLiteral", + replacement: "false", + location: { start: { line: 9 } }, + }, + ] + + it("lists every blocking mutant with tests, reproduction, exclusion, and report guidance", () => { + const baseSha = "a".repeat(40) + const headSha = "b".repeat(40) + const summary = formatSummary( + [ + { + id: "extension", + root: "src", + selectors: ["core/value.ts:4-4"], + testFiles: ["core/__tests__/value.test.ts"], + reportPath: "reports/mutation/extension/mutation.html", + changedLines: 1, + valid: 3, + killed: 0, + timeout: 0, + survived: 2, + noCoverage: 1, + blocking, + result: "Failed", + }, + ], + ["extension has blocking mutants"], + { baseSha, headSha }, + ) + + assert.ok(summary.includes("`core/__tests__/value.test.ts`")) + assert.ok(summary.includes("#### `src/core/value.ts`")) + assert.ok(summary.includes("#### `src/utils/other.ts`")) + for (const mutant of blocking) assert.ok(summary.includes(mutant.mutatorName)) + assert.ok(summary.includes('"left \\| right"')) + assert.ok(summary.includes(`node scripts/stryker-diff.mjs ci --base ${baseSha} --head ${headSha}`)) + assert.ok(summary.includes("Stryker disable next-line ConditionalExpression:")) + assert.ok(summary.includes("`reports/mutation/extension/mutation.html`")) + assert.ok(summary.includes("`changed-code-mutation-report` artifact")) + }) + + it("caps annotations without truncating the grouped summary", () => { + const manyMutants = Array.from({ length: 30 }, (_, index) => ({ + filePath: `file-${Math.floor(index / 10)}.ts`, + status: "Survived", + mutatorName: `Mutator${index}`, + replacement: `replacement-${index}`, + location: { start: { line: (index % 10) + 1 } }, + })) + const annotations = formatAnnotations(manyMutants, "src") + const grouped = formatBlockingMutants(manyMutants, "src").join("\n") + + assert.equal(annotations.length, 20) + for (const file of new Set(annotations.map(({ file }) => file))) { + assert.ok(annotations.filter((annotation) => annotation.file === file).length <= 7) + } + for (const mutant of manyMutants) assert.ok(grouped.includes(mutant.mutatorName)) + }) + + it("shares annotation limits across packages", () => { + const state = { total: 0, perFile: new Map() } + const first = formatAnnotations( + Array.from({ length: 15 }, (_, index) => ({ + filePath: `first-${index}.ts`, + status: "Survived", + mutatorName: "BooleanLiteral", + location: { start: { line: 1 } }, + })), + "packages/core", + state, + ) + const second = formatAnnotations( + Array.from({ length: 15 }, (_, index) => ({ + filePath: `second-${index}.ts`, + status: "NoCoverage", + mutatorName: "StringLiteral", + location: { start: { line: 1 } }, + })), + "packages/cloud", + state, + ) + + assert.equal(first.length, 15) + assert.equal(second.length, 5) + assert.equal(state.total, 20) + }) + + it("uses the actual tests recorded by Stryker", () => { + assert.deepEqual( + testsFromMutationReport({ testFiles: { "src/value.test.ts": {}, "src/other.spec.ts": {} } }, [ + "fallback.test.ts", + ]), + ["src/value.test.ts", "src/other.spec.ts"], + ) + assert.deepEqual(testsFromMutationReport({}, ["fallback.test.ts"]), ["fallback.test.ts"]) + }) + + it("keeps the maximum blocking-mutant inventory within GitHub's summary limit", () => { + const rows = Array.from({ length: 6 }, (_, packageIndex) => ({ + id: `package-${packageIndex}`, + root: `packages/package-${packageIndex}`, + selectors: ["src/value.ts:1-500"], + testFiles: ["src/value.test.ts"], + reportPath: `reports/mutation/package-${packageIndex}/mutation.html`, + changedLines: 500, + valid: MAX_MUTANTS, + killed: 0, + timeout: 0, + survived: MAX_MUTANTS, + noCoverage: 0, + blocking: Array.from({ length: MAX_MUTANTS }, (_, mutantIndex) => ({ + filePath: `src/file-${mutantIndex}.ts`, + status: "Survived", + mutatorName: `Package${packageIndex}Mutator${mutantIndex}`, + replacement: "x".repeat(1_000), + location: { start: { line: 1 } }, + })), + result: "Failed", + })) + const summary = formatSummary(rows, ["mutation failure"], { + baseSha: "a".repeat(40), + headSha: "b".repeat(40), + }) + + assert.equal(new Set(summary.match(/Package\dMutator\d+/g)).size, 6 * MAX_MUTANTS) + assert.ok(Buffer.byteLength(summary) < 1024 * 1024) + }) +}) + describe("report evaluation", () => { const packageEntry = { id: "core", root: "packages/core" } From 291964ea841a8a5edc05a3038bd61477be616be0 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 05:05:32 +0000 Subject: [PATCH 7/8] fix(ci): preserve mutation annotation punctuation --- scripts/stryker-diff.mjs | 21 +++++++++++---------- scripts/stryker-diff.test.mjs | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/scripts/stryker-diff.mjs b/scripts/stryker-diff.mjs index c8350b1340..c0e8a6cd1a 100644 --- a/scripts/stryker-diff.mjs +++ b/scripts/stryker-diff.mjs @@ -422,13 +422,16 @@ export function mutantCounts(report) { return counts } -function escapeWorkflowValue(value) { - return String(value) - .replaceAll("%", "%25") - .replaceAll("\r", "%0D") - .replaceAll("\n", "%0A") - .replaceAll(":", "%3A") - .replaceAll(",", "%2C") +function escapeWorkflowData(value) { + return String(value).replaceAll("%", "%25").replaceAll("\r", "%0D").replaceAll("\n", "%0A") +} + +function escapeWorkflowProperty(value) { + return escapeWorkflowData(value).replaceAll(":", "%3A").replaceAll(",", "%2C") +} + +export function formatAnnotationCommand(annotation) { + return `::error file=${escapeWorkflowProperty(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowData(annotation.message)}` } export function formatAnnotations(blockingMutants, packageRoot, state = { total: 0, perFile: new Map() }) { @@ -682,9 +685,7 @@ export function runManifest(repoRoot, manifest, reportRoot) { packageEntry.runRoot ?? packageEntry.root, annotationState, )) { - console.log( - `::error file=${escapeWorkflowValue(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowValue(annotation.message)}`, - ) + console.log(formatAnnotationCommand(annotation)) } evaluateReport(report, packageEntry) rows.push({ diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index e7ebbb9eaa..a51751b1bf 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -15,6 +15,7 @@ import { evaluateReport, executableChangedLines, formatAnnotations, + formatAnnotationCommand, formatBlockingMutants, formatSummary, mutantCounts, @@ -432,6 +433,19 @@ describe("failure output", () => { assert.equal(state.total, 20) }) + it("preserves punctuation in annotation messages while escaping properties", () => { + const command = formatAnnotationCommand({ + file: "src/value:one,two.ts", + line: 4, + message: "Survived mutant (replacement: left, right). 100% reproducible.", + }) + + assert.equal( + command, + "::error file=src/value%3Aone%2Ctwo.ts,line=4,title=Mutation test gap::Survived mutant (replacement: left, right). 100%25 reproducible.", + ) + }) + it("uses the actual tests recorded by Stryker", () => { assert.deepEqual( testsFromMutationReport({ testFiles: { "src/value.test.ts": {}, "src/other.spec.ts": {} } }, [ From 30d682586889883c816524398660efe38c300bad Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 3 Sep 2026 05:11:36 +0000 Subject: [PATCH 8/8] fix(ci): align mutation selectors with merge result --- .github/workflows/mutation-testing.yml | 12 +++-- scripts/stryker-diff.test.mjs | 73 +++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 6b7bec2303..29b9d30f8b 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -29,7 +29,7 @@ jobs: if: github.event_name == 'pull_request' uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: - ref: refs/pull/${{ github.event.pull_request.number }}/merge + ref: ${{ github.sha }} fetch-depth: 0 persist-credentials: false @@ -54,7 +54,7 @@ jobs: if: github.event_name == 'pull_request' env: BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} + HEAD_SHA: ${{ github.sha }} run: node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA" - name: Upload mutation reports @@ -72,6 +72,8 @@ jobs: env: ARTIFACT_URL: ${{ steps.mutation_report.outputs.artifact-url }} run: | - echo "" >> "$GITHUB_STEP_SUMMARY" - echo "### Download mutation reports" >> "$GITHUB_STEP_SUMMARY" - echo "[Open the changed-code-mutation-report artifact]($ARTIFACT_URL), then open the package's mutation.html file." >> "$GITHUB_STEP_SUMMARY" + { + echo "" + echo "### Download mutation reports" + echo "[Open the changed-code-mutation-report artifact]($ARTIFACT_URL), then open the package's mutation.html file." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/stryker-diff.test.mjs b/scripts/stryker-diff.test.mjs index a51751b1bf..0f39dc507f 100644 --- a/scripts/stryker-diff.test.mjs +++ b/scripts/stryker-diff.test.mjs @@ -25,6 +25,7 @@ import { preferDirectTestFiles, resolveVitestBinary, packageForPath, + runManifest, selectFromGit, testsFromMutationReport, validateDisableDirectives, @@ -40,11 +41,14 @@ describe("mutation testing workflow", () => { assert.ok(!workflow.includes("pull_request_target:")) assert.ok(workflow.includes(" contents: read")) assert.ok(workflow.includes("- name: Checkout pull request merge result")) - assert.ok(workflow.includes("ref: refs/pull/${{ github.event.pull_request.number }}/merge")) + assert.ok(workflow.includes("ref: ${{ github.sha }}")) + assert.ok(!workflow.includes("ref: refs/pull/${{ github.event.pull_request.number }}/merge")) assert.ok(workflow.includes("fetch-depth: 0")) assert.ok(workflow.includes("persist-credentials: false")) assert.ok(!workflow.includes("repository: ${{ github.event.pull_request.head.repo.full_name }}")) assert.ok(!workflow.includes("ref: ${{ github.event.pull_request.head.sha }}")) + assert.ok(workflow.includes("HEAD_SHA: ${{ github.sha }}")) + assert.ok(!workflow.includes("HEAD_SHA: ${{ github.event.pull_request.head.sha }}")) assert.ok(workflow.includes("steps.mutation_report.outputs.artifact-url")) assert.ok(workflow.includes("open the package's mutation.html file")) }) @@ -291,6 +295,43 @@ describe("selectFromGit", () => { fs.rmSync(repo, { recursive: true, force: true }) } }) + + it("uses merge-result line coordinates when the base shifts a pull request edit", () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "stryker-merge-diff-")) + const runGit = (...args) => execFileSync("git", args, { cwd: repo, encoding: "utf8" }).trim() + + try { + runGit("init", "--initial-branch=main") + runGit("config", "user.name", "Mutation Test") + runGit("config", "user.email", "mutation@example.com") + fs.mkdirSync(path.join(repo, "packages/core/src"), { recursive: true }) + fs.writeFileSync(path.join(repo, "packages/core/src/value.ts"), "const first = 1\nconst changed = true\n") + runGit("add", ".") + runGit("commit", "-m", "initial") + + runGit("checkout", "-b", "feature") + fs.writeFileSync(path.join(repo, "packages/core/src/value.ts"), "const first = 1\nconst changed = false\n") + runGit("commit", "-am", "change value") + + runGit("checkout", "main") + fs.writeFileSync( + path.join(repo, "packages/core/src/value.ts"), + "const inserted = 0\nconst first = 1\nconst changed = true\n", + ) + runGit("commit", "-am", "shift source lines") + const baseSha = runGit("rev-parse", "HEAD") + runGit("merge", "--no-ff", "feature", "-m", "merge feature") + const mergeSha = runGit("rev-parse", "HEAD") + + const manifest = selectFromGit(repo, baseSha, mergeSha) + assert.deepEqual( + manifest.packages.map(({ id, selectors }) => ({ id, selectors })), + [{ id: "core", selectors: ["src/value.ts:3-3"] }], + ) + } finally { + fs.rmSync(repo, { recursive: true, force: true }) + } + }) }) describe("mutation exclusions", () => { @@ -446,6 +487,36 @@ describe("failure output", () => { ) }) + it("reports a Stryker preflight launch error when the binary is missing", () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), "stryker-launch-")) + const reportRoot = path.join(repo, "reports") + + try { + fs.mkdirSync(path.join(repo, "packages/core"), { recursive: true }) + assert.throws( + () => + runManifest( + repo, + { + packages: [ + { + id: "core", + root: "packages/core", + vitestConfig: "vitest.unit.config.ts", + selectors: ["src/value.ts:1-1"], + changedExecutableLines: 1, + }, + ], + }, + reportRoot, + ), + /core Stryker preflight could not start:.*ENOENT/, + ) + } finally { + fs.rmSync(repo, { recursive: true, force: true }) + } + }) + it("uses the actual tests recorded by Stryker", () => { assert.deepEqual( testsFromMutationReport({ testFiles: { "src/value.test.ts": {}, "src/other.spec.ts": {} } }, [