diff --git a/src/main/runtime/folder-workspace-git-routing.test.ts b/src/main/runtime/folder-workspace-git-routing.test.ts index 7549a4d7ed7..69d96e6b835 100644 --- a/src/main/runtime/folder-workspace-git-routing.test.ts +++ b/src/main/runtime/folder-workspace-git-routing.test.ts @@ -545,6 +545,52 @@ describe('#6357 monorepo folder workspace', () => { expect(await runtime.getRuntimeGitConflictOperation(`path:${CHILD_API}`)).toBe('merge') }) + /** + * Hold a folder-workspace generation open so cancellation has a real in-flight + * lane to act on, and record every child selector cancellation is routed to. + */ + async function withInFlightGeneration( + runtime: OrcaRuntimeService, + selector: string + ): Promise<{ + generateSelectors: () => string[] + cancelSelectors: () => string[] + settle: () => Promise + }> { + let release = (): void => {} + const generate = vi + .spyOn( + runtime['gitCommands'] as { generateRuntimeCommitMessage: (...args: never[]) => unknown }, + 'generateRuntimeCommitMessage' + ) + .mockImplementation( + () => + new Promise((resolve) => { + release = () => resolve({ success: true, message: 'drafted' }) + }) as never + ) + const cancel = vi + .spyOn( + runtime['gitCommands'] as { + cancelRuntimeGenerateCommitMessage: (...args: never[]) => unknown + }, + 'cancelRuntimeGenerateCommitMessage' + ) + .mockResolvedValue({ ok: true } as never) + const inFlight = runtime.generateRuntimeCommitMessage(selector) + await vi.waitFor(() => expect(generate).toHaveBeenCalled()) + return { + generateSelectors: () => generate.mock.calls.map((call) => String(call[0])), + cancelSelectors: () => cancel.mock.calls.map((call) => String(call[0])), + settle: async () => { + release() + await inFlight + generate.mockRestore() + cancel.mockRestore() + } + } + } + it('M3: cancelling generation reaches the child repo instead of dying on the folder selector', async () => { buildFixture() const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never) @@ -554,7 +600,53 @@ describe('#6357 monorepo folder workspace', () => { // to the child path. Passing the folder selector straight through threw // `selector_not_found`, and every caller is fire-and-forget — so Cancel silently // did nothing and the draft kept running. - const inner = vi + const run = await withInFlightGeneration(runtime, selector) + await expect(runtime.cancelRuntimeGenerateCommitMessage(selector)).resolves.toEqual({ + ok: true + }) + const routed = run.cancelSelectors() + expect(routed).not.toContain(selector) + // Why: exactly the lane the generation started under — see M4 for why not more. + expect(routed).toEqual(run.generateSelectors()) + const staged = await runtime.getRuntimeGitStatus(routed[0] as string) + expect(staged.entries.length).toBeGreaterThan(0) + await run.settle() + }) + + it('M4: cancelling one child repo leaves a sibling repo untouched', async () => { + buildFixture() + const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never) + const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}` + await runtime.stageRuntimeGitPath(selector, 'fint_api/src/app.ts') + // Why: cancellation is keyed by cwd alone, with no notion of who asked. Signalling + // every child — which is what routing Cancel by a fresh staged-repo *re-selection* + // amounts to — kills a draft the user started from the sibling repo's own view. + // The renderer keeps generation per-worktree precisely so navigating away does not + // cancel it (SourceControl.tsx), so that concurrent draft is reachable, not theoretical. + // Markers unique to each child, so a routed selector can be named by repo. + writeFileSync(join(CHILD_API, 'src', 'only-in-api.ts'), 'marker\n') + writeFileSync(join(CONTAINER, 'fint-portal', 'src', 'only-in-portal.ts'), 'marker\n') + const run = await withInFlightGeneration(runtime, selector) + await runtime.cancelRuntimeGenerateCommitMessage(selector) + const routed = run.cancelSelectors() + const signalled = await Promise.all( + routed.map(async (target) => + (await runtime.getRuntimeGitStatus(target)).entries.map((entry) => entry.path) + ) + ) + // Before this, Cancel fanned out to every child, so the sibling was signalled too. + expect(signalled.flat()).toContain('src/only-in-api.ts') + expect(signalled.flat()).not.toContain('src/only-in-portal.ts') + expect(routed).toHaveLength(1) + await run.settle() + }) + + it('M5: Cancel with nothing in flight signals no child at all', async () => { + buildFixture() + const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never) + const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}` + await runtime.stageRuntimeGitPath(selector, 'fint_api/src/app.ts') + const cancel = vi .spyOn( runtime['gitCommands'] as { cancelRuntimeGenerateCommitMessage: (...args: never[]) => unknown @@ -562,16 +654,13 @@ describe('#6357 monorepo folder workspace', () => { 'cancelRuntimeGenerateCommitMessage' ) .mockResolvedValue({ ok: true } as never) + // Why: a stale Cancel click (the draft already finished) must not become a + // remote kill switch for whatever is running in these repos now. await expect(runtime.cancelRuntimeGenerateCommitMessage(selector)).resolves.toEqual({ ok: true }) - const routed = inner.mock.calls.map((call) => String(call[0])) - expect(routed).not.toContain(selector) - expect(routed.length).toBeGreaterThan(0) - // Why: the lane the generation actually started under must be among them. - const staged = await runtime.getRuntimeGitStatus(routed[0] as string) - expect(staged.entries.length).toBeGreaterThan(0) - inner.mockRestore() + expect(cancel).not.toHaveBeenCalled() + cancel.mockRestore() }) it('E: the real child git worktree resolves fine when its repo is registered', async () => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index de1b093cc8c..7f7ed340e27 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2934,6 +2934,10 @@ export class OrcaRuntimeService { | null private readonly agentSessionClaimSigner: AgentSessionClaimSigner private readonly agentSessionCreateOperations = new Map() + // Normalized folder-workspace path -> the child selectors its in-flight commit-message + // generations are running under. Cancellation is keyed by cwd, so without this a + // workspace-level Cancel would also kill a draft running in an unrelated child repo. + private readonly folderWorkspaceCommitMessageLanes = new Map>() private sshRelayRecoveryGenerationByTargetId = new Map() private accountServices: RuntimeAccountServices | null = null private commitMessageAgentEnv: CommitMessageAgentEnvironmentResolvers | null = null @@ -24265,23 +24269,28 @@ export class OrcaRuntimeService { } /** - * The one child repo a staged-index operation belongs to. Null when the selector - * is not a folder workspace, so callers can fall through to the single-repo path. + * The one child repo a staged-index operation belongs to, plus the workspace it + * was resolved under. Null when the selector is not a folder workspace, so + * callers can fall through to the single-repo path. */ - private async selectFolderWorkspaceStagedTarget( - selector: string - ): Promise<{ ok: true; target: RuntimeGitTarget } | { ok: false; error: string } | null> { + private async selectFolderWorkspaceStagedTarget(selector: string): Promise<{ + folderPath: string + selection: { ok: true; target: RuntimeGitTarget } | { ok: false; error: string } + } | null> { const scope = await this.resolveFolderWorkspaceLaunchScope(selector) const folderWorkspace = scope?.folderWorkspace if (!folderWorkspace) { return null } - return selectFolderWorkspaceCommitTarget( - await this.inspectFolderWorkspaceChildRepos( - folderWorkspace.folderPath, - probeChildRepoHasStagedChanges + return { + folderPath: folderWorkspace.folderPath, + selection: selectFolderWorkspaceCommitTarget( + await this.inspectFolderWorkspaceChildRepos( + folderWorkspace.folderPath, + probeChildRepoHasStagedChanges + ) ) - ) + } } /** @@ -24305,11 +24314,11 @@ export class OrcaRuntimeService { if (message.trim().length === 0) { throw new Error('Commit message is required') } - if (!selected.ok) { - return { success: false, error: selected.error } + if (!selected.selection.ok) { + return { success: false, error: selected.selection.error } } try { - return await commitRuntimeGitForTarget(selected.target, message) + return await commitRuntimeGitForTarget(selected.selection.target, message) } catch (error) { return { success: false, error: error instanceof Error ? error.message : 'Commit failed' } } @@ -24330,19 +24339,32 @@ export class OrcaRuntimeService { if (!selected) { return null } - if (!selected.ok) { + if (!selected.selection.ok) { return { success: false, error: - selected.error === 'nothing to commit' + selected.selection.error === 'nothing to commit' ? 'No staged changes to summarize.' - : selected.error + : selected.selection.error + } + } + // Why record: the cancel lane is keyed to the child path, and re-selecting at + // cancel time can name a different repo (staging moves between the two calls). + // Remembering the lane this call actually started is the only way Cancel can + // hit that one and nothing else. + const childSelector = `id:${selected.selection.target.worktree.id}` + const key = normalizeRuntimePathForComparison(selected.folderPath) + const lanes = this.folderWorkspaceCommitMessageLanes.get(key) ?? new Set() + lanes.add(childSelector) + this.folderWorkspaceCommitMessageLanes.set(key, lanes) + try { + return await this.gitCommands.generateRuntimeCommitMessage(childSelector, settingsOverride) + } finally { + lanes.delete(childSelector) + if (lanes.size === 0) { + this.folderWorkspaceCommitMessageLanes.delete(key) } } - return this.gitCommands.generateRuntimeCommitMessage( - `id:${selected.target.worktree.id}`, - settingsOverride - ) } /** @@ -24351,11 +24373,10 @@ export class OrcaRuntimeService { * keyed to that child's path — the folder selector resolves to no worktree and * would just throw `selector_not_found`, leaving Cancel silently dead. * - * Why cancel every child rather than re-select the staged one: selection is a - * fresh read, and staging can change between generate and cancel, so it can - * name a different repo than the one actually generating. Both cancel paths are - * cwd-keyed best-effort no-ops when nothing is in flight, so the broadest - * correct move is to signal every lane this selector could have started. + * Why only the recorded lanes: cancellation is keyed by cwd alone, so signalling + * every child would abort a draft another view started in a *different* child + * repo. `folderWorkspaceCommitMessageLanes` holds exactly the children this + * workspace's own generations are running in, so nothing else is touched. * Returns null when the selector is not a folder workspace. */ private async cancelFolderWorkspaceGenerateCommitMessage( @@ -24366,27 +24387,22 @@ export class OrcaRuntimeService { if (!folderWorkspace) { return null } - const childRepos = listFolderWorkspaceChildRepos( - this.requireStore().getRepos(), - folderWorkspace.folderPath + const lanes = this.folderWorkspaceCommitMessageLanes.get( + normalizeRuntimePathForComparison(folderWorkspace.folderPath) ) await Promise.all( - childRepos.map(async (repo) => { - const target = await this.resolveChildRepoGitTarget(repo) - if (!target) { - return - } + [...(lanes ?? [])].map(async (childSelector) => // Why swallow: a child that cannot be reached has nothing in flight to // cancel, and one unreachable repo must not strand the others. - await this.gitCommands - .cancelRuntimeGenerateCommitMessage(`id:${target.worktree.id}`) + this.gitCommands + .cancelRuntimeGenerateCommitMessage(childSelector) .catch((error: unknown) => { console.warn('[runtime] folder-workspace commit-message cancel failed', { - repo: repo.path, + childSelector, error }) }) - }) + ) ) return { ok: true } }