diff --git a/backend/src/routes/change-walkthroughs.ts b/backend/src/routes/change-walkthroughs.ts index 1e02f576a..10b960cd2 100644 --- a/backend/src/routes/change-walkthroughs.ts +++ b/backend/src/routes/change-walkthroughs.ts @@ -41,5 +41,16 @@ export function createChangeWalkthroughRoutes(service: ChangeWalkthroughService) } }) + app.delete('/:sessionId', (c) => { + const sourceParam = c.req.query('source') + const source = sourceParam === undefined ? DEFAULT_WALKTHROUGH_SOURCE : parseWalkthroughSourceKey(sourceParam) + if (!source) { + return c.json({ error: 'Invalid walkthrough source' }, 400) + } + + service.cancelGeneration(c.req.param('sessionId'), source) + return c.body(null, 204) + }) + return app } diff --git a/backend/src/services/change-walkthroughs.ts b/backend/src/services/change-walkthroughs.ts index 0b3745771..ab577009b 100644 --- a/backend/src/services/change-walkthroughs.ts +++ b/backend/src/services/change-walkthroughs.ts @@ -673,6 +673,7 @@ interface InFlightGeneration { modelStarted: Promise result: Promise<{ walkthrough: ChangeWalkthrough; created: boolean }> walkthrough: ChangeWalkthrough | null + controller: AbortController } class WalkthroughCallSkippedError extends Error { @@ -682,6 +683,13 @@ class WalkthroughCallSkippedError extends Error { } } +export class WalkthroughCancelledError extends Error { + constructor() { + super('Walkthrough generation stopped') + this.name = 'WalkthroughCancelledError' + } +} + function toGenerationError(error: unknown): WalkthroughGenerationError { if (error instanceof ServiceError) { return { @@ -812,10 +820,35 @@ export class ChangeWalkthroughService { async startGeneration(sessionId: string, request: GenerateChangeWalkthroughRequest): Promise { const entry = this.begin(sessionId, request) - await Promise.race([entry.modelStarted, entry.result]) + try { + await Promise.race([entry.modelStarted, entry.result]) + } catch (error) { + if (!(error instanceof WalkthroughCancelledError)) { + throw error + } + } return this.getState(sessionId, request.source ?? DEFAULT_WALKTHROUGH_SOURCE) } + /** + * Stops an in-flight generation for a session and source. The in-flight entry is removed + * synchronously so the next read reports `generating: false`, the partial walkthrough is + * persisted, and the abort signal stops the model calls that are still running. + */ + cancelGeneration(sessionId: string, source: WalkthroughSource = DEFAULT_WALKTHROUGH_SOURCE): boolean { + const key = entryKey(sessionId, source) + const entry = this.inFlight.get(key) + if (!entry) { + return false + } + this.inFlight.delete(key) + if (entry.walkthrough) { + this.saveIfSessionLive(entry.walkthrough) + } + entry.controller.abort() + return true + } + /** Removes the stored walkthroughs of a deleted session, including one still being generated. */ handleEvent(event: SSEEvent): void { switch (event.type) { @@ -910,18 +943,20 @@ export class ChangeWalkthroughService { resolveModelStarted() } - const entry = { modelStarted, walkthrough: null } as InFlightGeneration + const entry = { modelStarted, walkthrough: null, controller: new AbortController() } as InFlightGeneration this.inFlight.set(key, entry) - entry.result = this.runGenerate(sessionId, source, request, markModelStarted, entry) + entry.result = this.runGenerate(sessionId, source, request, markModelStarted, entry, entry.controller.signal) .catch((error) => { - if (modelCalled) { + if (modelCalled && !(error instanceof WalkthroughCancelledError)) { this.failures.set(key, toGenerationError(error)) } throw error }) .finally(() => { - this.inFlight.delete(key) + if (this.inFlight.get(key) === entry) { + this.inFlight.delete(key) + } this.deletedDuringGeneration.delete(key) this.invalidateCurrentHash(key) }) @@ -953,18 +988,27 @@ export class ChangeWalkthroughService { } } + private throwIfCancelled(signal: AbortSignal): void { + if (signal.aborted) { + throw new WalkthroughCancelledError() + } + } + private async runGenerate( sessionId: string, source: WalkthroughSource, request: GenerateChangeWalkthroughRequest, onModelStart: () => void, entry: InFlightGeneration, + signal: AbortSignal, ): Promise<{ walkthrough: ChangeWalkthrough; created: boolean }> { const session = await this.readSession(sessionId) + this.throwIfCancelled(signal) const key = entryKey(sessionId, source) if (source.kind === 'pullRequest') { await this.fetchPullRequestChanges(session, source) + this.throwIfCancelled(signal) } let changes: FileDiffInfo[] @@ -983,6 +1027,7 @@ export class ChangeWalkthroughService { code: 'WALKTHROUGH_CHANGES_UNAVAILABLE', }) } + this.throwIfCancelled(signal) if (changes.length === 0) { throw new ChangeWalkthroughError(noChangesMessage(source), 409, { @@ -1001,7 +1046,7 @@ export class ChangeWalkthroughService { return { walkthrough: previous, created: false } } - const model = await this.resolveWalkthroughModel(session) + const model = await this.resolveWalkthroughModel(session, request.model, signal) const storedModel = formatOpenCodeModelRef(model) const resumingStops = previous.stops.map((stop) => stop.status === 'ready' ? stop : { ...stop, status: 'pending' as const }, @@ -1021,6 +1066,7 @@ export class ChangeWalkthroughService { resuming, onModelStart, entry, + signal, ) if (this.deletedDuringGeneration.has(key)) { throw new ChangeWalkthroughError('Session not found', 404) @@ -1028,7 +1074,7 @@ export class ChangeWalkthroughService { return { walkthrough: resumed, created: true } } - const model = await this.resolveWalkthroughModel(session) + const model = await this.resolveWalkthroughModel(session, request.model, signal) const modelKey = computeModelKey(model) const storedModel = formatOpenCodeModelRef(model) @@ -1048,6 +1094,7 @@ export class ChangeWalkthroughService { hunks: WalkthroughHunk[] omittedFiles: WalkthroughOmittedFile[] }): ChangeWalkthrough => { + this.throwIfCancelled(signal) const walkthrough: ChangeWalkthrough = { sessionId, source, @@ -1110,8 +1157,11 @@ export class ChangeWalkthroughService { const prompt = buildWalkthroughPrompt({ title, hunks }) onModelStart() - const parsed = await this.callModelParsed(prompt, model, (text) => - parseWalkthroughResponse(text, hunks), + const parsed = await this.callModelParsed( + prompt, + model, + (text) => parseWalkthroughResponse(text, hunks), + signal, ) const stops = parsed.stops.map((stop) => applySingleCallKeys(stop, modelKey)) @@ -1138,8 +1188,11 @@ export class ChangeWalkthroughService { }) onModelStart() - const planned = await this.callModelParsed(planPrompt, model, (text) => - parseWalkthroughPlan(text, outline.hunks), + const planned = await this.callModelParsed( + planPrompt, + model, + (text) => parseWalkthroughPlan(text, outline.hunks), + signal, ) const stops = planned.stops.map((stop) => applyReuse(stop, reusable, modelKey)) @@ -1162,6 +1215,7 @@ export class ChangeWalkthroughService { walkthrough, onModelStart, entry, + signal, ) } @@ -1181,6 +1235,7 @@ export class ChangeWalkthroughService { walkthrough: ChangeWalkthrough, onModelStart: () => void, entry: InFlightGeneration, + signal: AbortSignal, ): Promise { const modelKey = computeModelKey(model) const key = entryKey(walkthrough.sessionId, walkthrough.source) @@ -1189,6 +1244,9 @@ export class ChangeWalkthroughService { let current = walkthrough await mapWithConcurrency(stops, WALKTHROUGH_EXPLAIN_CONCURRENCY, async (stop) => { + if (signal.aborted) { + return + } const explainPrompt = buildWalkthroughExplainPrompt({ title, summary, stop, hunks }) onModelStart() @@ -1198,6 +1256,7 @@ export class ChangeWalkthroughService { explainPrompt, model, parseWalkthroughExplanation, + signal, () => this.deletedDuringGeneration.has(key), ) explained = { @@ -1207,13 +1266,16 @@ export class ChangeWalkthroughService { explanationKey: computeExplanationKey(modelKey, stop.hunkIds), } } catch (error) { - if (error instanceof WalkthroughCallSkippedError) { + if (error instanceof WalkthroughCallSkippedError || error instanceof WalkthroughCancelledError) { return } logger.warn('Failed to explain a change walkthrough stop', getErrorMessage(error)) explained = { ...stop, status: 'failed' } } + if (signal.aborted) { + return + } current = this.replaceStop(current, explained) entry.walkthrough = current completed += 1 @@ -1229,23 +1291,33 @@ export class ChangeWalkthroughService { prompt: string, model: ModelRef, parse: (text: string) => T | null, + signal: AbortSignal, shouldSkip?: () => boolean, ): Promise { let lastError = new ChangeWalkthroughError('Failed to generate the change walkthrough', 502) for (let attempt = 0; attempt < WALKTHROUGH_CALL_ATTEMPTS; attempt += 1) { + if (signal.aborted) { + throw new WalkthroughCancelledError() + } let text: string try { text = await WALKTHROUGH_MODEL_LIMITER.run(() => { if (shouldSkip?.()) { throw new WalkthroughCallSkippedError() } - return generateTextWithTimeout(this.openCodeClient, { prompt, model }, this.timeoutMs) + if (signal.aborted) { + throw new WalkthroughCancelledError() + } + return generateTextWithTimeout(this.openCodeClient, { prompt, model }, this.timeoutMs, signal) }) } catch (error) { if (error instanceof WalkthroughCallSkippedError) { throw error } + if (error instanceof WalkthroughCancelledError || signal.aborted) { + throw new WalkthroughCancelledError() + } lastError = error instanceof GenerateTextTimeoutError ? new ChangeWalkthroughError('Generating the change walkthrough timed out', 504, { @@ -1298,34 +1370,57 @@ export class ChangeWalkthroughService { } /** - * Resolves the model for one generation, waiting once for OpenCode's lazily-loaded global catalog so an explicit - * walkthrough model is registered before the first call. A failed wait is swallowed because generation proceeds and - * lets OpenCode surface its own error; the wait is per generation, never per stop. + * Resolves the model for one generation, preferring the configured walkthrough model, then the request model, then + * the session model. Waits once for OpenCode's lazily-loaded global catalog so an explicit model is registered before + * the first call. A failed wait is swallowed because generation proceeds and lets OpenCode surface its own error; the + * wait is per generation, never per stop. */ - private async resolveWalkthroughModel(session: SessionInfo): Promise { + private async resolveWalkthroughModel( + session: SessionInfo, + requestModel?: string, + signal?: AbortSignal, + ): Promise { const configured = this.settingsService.getSettings().preferences.walkthroughModel?.trim() - const parsed = configured ? parseOpenCodeModelRef(configured) : undefined - const preferred = parsed ?? session.model + const parsedConfigured = configured ? parseOpenCodeModelRef(configured) : undefined + const parsedRequest = requestModel ? parseOpenCodeModelRef(requestModel) : undefined + const preferred = parsedConfigured ?? parsedRequest ?? session.model if (preferred) { try { await resolveOpenCodeModel(this.openCodeClient, getOpenCodeGlobalConfigPath(), { preferredModel: formatOpenCodeModelRef(preferred), + ...(signal ? { signal } : {}), }) } catch { + if (signal?.aborted) { + throw new WalkthroughCancelledError() + } return preferred } + if (signal?.aborted) { + throw new WalkthroughCancelledError() + } return preferred } let resolved try { - resolved = await resolveOpenCodeModel(this.openCodeClient, getOpenCodeGlobalConfigPath()) + resolved = await resolveOpenCodeModel( + this.openCodeClient, + getOpenCodeGlobalConfigPath(), + signal ? { signal } : undefined, + ) } catch (error) { + if (signal?.aborted) { + throw new WalkthroughCancelledError() + } throw new ChangeWalkthroughError( getErrorMessage(error) || 'Failed to resolve the walkthrough model', 502, { code: 'WALKTHROUGH_MODEL_UNAVAILABLE' }, ) } + if (signal?.aborted) { + throw new WalkthroughCancelledError() + } return resolved.variant ? { providerID: resolved.providerID, id: resolved.id, variant: resolved.variant } diff --git a/backend/src/services/opencode/generate-text.ts b/backend/src/services/opencode/generate-text.ts index 8c2c8fe87..5ebcf2eef 100644 --- a/backend/src/services/opencode/generate-text.ts +++ b/backend/src/services/opencode/generate-text.ts @@ -14,9 +14,18 @@ export async function generateTextWithTimeout( client: OpenCodeClient, input: { prompt: string; model?: ModelRef }, timeoutMs: number, + signal?: AbortSignal, ): Promise { const controller = new AbortController() let timer: ReturnType | undefined + const abortFromSignal = () => controller.abort(signal?.reason) + if (signal) { + if (signal.aborted) { + controller.abort(signal.reason) + } else { + signal.addEventListener('abort', abortFromSignal, { once: true }) + } + } const timeout = new Promise((_resolve, reject) => { timer = setTimeout(() => { controller.abort() @@ -29,6 +38,7 @@ export async function generateTextWithTimeout( return text } finally { if (timer) clearTimeout(timer) + signal?.removeEventListener('abort', abortFromSignal) } } diff --git a/backend/test/routes/change-walkthroughs.test.ts b/backend/test/routes/change-walkthroughs.test.ts index 415ba44c6..d1c427d53 100644 --- a/backend/test/routes/change-walkthroughs.test.ts +++ b/backend/test/routes/change-walkthroughs.test.ts @@ -113,6 +113,14 @@ function replyForPrompt(prompt: string, summary = 'A summary'): string { return JSON.stringify({ summary, stops: [{ title: 'A', explanation: 'Why', hunkIds: ids }] }) } +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + describe('change walkthrough routes', () => { let db: Database let sessions: Record @@ -387,4 +395,57 @@ describe('change walkthrough routes', () => { expect(res.status).toBe(404) }) + + it('DELETE stops an in-flight generation', async () => { + fake.setGenerateImpl(() => new Promise(() => {})) + + const postRes = await app.request(`/change-walkthroughs/${SESSION_ID}`, { method: 'POST' }) + expect(postRes.status).toBe(202) + + const res = await app.request(`/change-walkthroughs/${SESSION_ID}`, { method: 'DELETE' }) + expect(res.status).toBe(204) + + const getRes = await app.request(`/change-walkthroughs/${SESSION_ID}`) + const state = (await getRes.json()) as { generating: boolean; error: unknown } + expect(state.generating).toBe(false) + expect(state.error).toBeNull() + }) + + it('resolves a pending POST with the stopped state after DELETE cancels it', async () => { + const diff = createDeferred() + const diffMock = vi.mocked(fake.client.api.session.diff) + diffMock.mockImplementationOnce(() => diff.promise) + + const postPromise = app.request(`/change-walkthroughs/${SESSION_ID}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({}), + }) + + await vi.waitFor(() => expect(diffMock).toHaveBeenCalledTimes(1)) + + const deleteRes = await app.request(`/change-walkthroughs/${SESSION_ID}`, { method: 'DELETE' }) + expect(deleteRes.status).toBe(204) + + diff.resolve(CHANGES) + + const postRes = await postPromise + expect(postRes.status).toBe(200) + const body = (await postRes.json()) as { generating: boolean; walkthrough: unknown; error: unknown } + expect(body.generating).toBe(false) + expect(body.walkthrough).toBeNull() + expect(body.error).toBeNull() + }) + + it('DELETE is a no-op when nothing is generating', async () => { + const res = await app.request(`/change-walkthroughs/${SESSION_ID}`, { method: 'DELETE' }) + + expect(res.status).toBe(204) + }) + + it('DELETE rejects an invalid source with 400', async () => { + const res = await app.request(`/change-walkthroughs/${SESSION_ID}?source=branch:-x`, { method: 'DELETE' }) + + expect(res.status).toBe(400) + }) }) diff --git a/backend/test/services/change-walkthroughs.test.ts b/backend/test/services/change-walkthroughs.test.ts index 1cc13528b..b7f79ad00 100644 --- a/backend/test/services/change-walkthroughs.test.ts +++ b/backend/test/services/change-walkthroughs.test.ts @@ -25,6 +25,7 @@ import { stubLoadedModelCatalog } from '../helpers/stub-opencode-client' import { createCommittedRepo, createGitAuthService, createOrigin, cloneOrigin, git, uniqueName } from '../helpers/git-fixtures' import { ChangeWalkthroughService, + WalkthroughCancelledError, buildWalkthroughInput, buildWalkthroughOutline, buildWalkthroughPlanPrompt, @@ -42,6 +43,33 @@ import { ChangeWalkthroughError } from '../../src/services/change-walkthrough-er const SESSION_ID = 'ses_walkthrough' +function abortable(promise: Promise, signal?: AbortSignal): Promise { + if (!signal) return promise + if (signal.aborted) return Promise.reject(new DOMException('Aborted', 'AbortError')) + return new Promise((resolve, reject) => { + const onAbort = () => reject(new DOMException('Aborted', 'AbortError')) + signal.addEventListener('abort', onAbort, { once: true }) + promise.then( + (value) => { + signal.removeEventListener('abort', onAbort) + resolve(value) + }, + (error) => { + signal.removeEventListener('abort', onAbort) + reject(error) + }, + ) + }) +} + +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + function userMessage(id: string): SessionMessageInfo { return { id, type: 'user', time: { created: 0 }, text: id } as SessionMessageInfo } @@ -185,10 +213,10 @@ function createFakeClient(sessions: Record) { }), }, generate: { - text: vi.fn(async (input: { prompt: string; model?: ModelRef }) => { + text: vi.fn(async (input: { prompt: string; model?: ModelRef }, options?: { signal?: AbortSignal }) => { generateCalls.push(input.prompt) generateModels.push(input.model) - return { text: await generateImpl(input.prompt) } + return { text: await abortable(generateImpl(input.prompt), options?.signal) } }), }, }, @@ -1238,6 +1266,28 @@ describe('ChangeWalkthroughService', () => { expect(fake.generateModels[0]).toEqual({ providerID: 'anthropic', id: 'claude-sonnet-4' }) }) + it('uses the request model over the session model', async () => { + sessions[SESSION_ID]!.info = { + id: SESSION_ID, + title: 'Title', + model: { providerID: 'anthropic', id: 'claude-sonnet-4' }, + } as SessionInfo + fake.setGenerateImpl(async () => modelReply([{ title: 'A', explanation: 'x', hunkIds: THREE_HUNK_IDS }])) + + await service.generate(SESSION_ID, { model: 'openai/gpt-5-mini' }) + + expect(fake.generateModels[0]).toEqual({ providerID: 'openai', id: 'gpt-5-mini' }) + }) + + it('prefers the walkthrough model preference over the request model', async () => { + new SettingsService(db).updateSettings({ walkthroughModel: 'openai/gpt-5' }) + fake.setGenerateImpl(async () => modelReply([{ title: 'A', explanation: 'x', hunkIds: THREE_HUNK_IDS }])) + + await service.generate(SESSION_ID, { model: 'anthropic/claude-sonnet-4' }) + + expect(fake.generateModels[0]).toEqual({ providerID: 'openai', id: 'gpt-5' }) + }) + it('reuses the resolved default model for every stop', async () => { sessions[SESSION_ID]!.changes = LARGE_CHANGES fake.setGenerateImpl((prompt) => @@ -2280,4 +2330,139 @@ describe('ChangeWalkthroughService', () => { expect(victimExplainStarted).toBe(1) }) }) + + describe('cancelGeneration', () => { + it('stops a pending generation without recording an error', async () => { + fake.setGenerateImpl(() => new Promise(() => {})) + + const pending = service.generate(SESSION_ID, {}) + await vi.waitFor(() => expect(fake.generateCalls).toHaveLength(1)) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + await expect(pending).rejects.toBeInstanceOf(WalkthroughCancelledError) + + const state = await service.getState(SESSION_ID) + expect(state.generating).toBe(false) + expect(state.error).toBeNull() + }) + + it('returns false when nothing is generating', () => { + expect(service.cancelGeneration(SESSION_ID)).toBe(false) + }) + + it('keeps explained stops and persists the partial walkthrough', async () => { + sessions[SESSION_ID]!.changes = LARGE_CHANGES + fake.setGenerateImpl((prompt) => { + if (prompt.includes(PLAN_MARKER)) { + return Promise.resolve(LARGE_PLAN_REPLY) + } + return new Promise(() => {}) + }) + + const pending = service.generate(SESSION_ID, {}) + + await vi.waitFor(async () => { + const state = await service.getState(SESSION_ID) + expect(state.generating).toBe(true) + expect(state.walkthrough?.stops).toHaveLength(3) + expect(state.walkthrough?.stops.every((stop) => stop.status === 'pending')).toBe(true) + }) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + + await vi.waitFor(async () => { + expect((await service.getState(SESSION_ID)).generating).toBe(false) + }) + + const state = await service.getState(SESSION_ID) + expect(state.error).toBeNull() + const stored = getChangeWalkthrough(db, SESSION_ID, 'session') + expect(stored).not.toBeNull() + expect(stored!.stops.every((stop) => stop.status === 'pending')).toBe(true) + + await pending + }) + + it('aborts only the requested source', async () => { + fake.setGenerateImpl(() => new Promise(() => {})) + + const pending = service.generate(SESSION_ID, {}) + await vi.waitFor(() => expect(fake.generateCalls).toHaveLength(1)) + + expect(service.cancelGeneration(SESSION_ID, { kind: 'staged' })).toBe(false) + expect((await service.getState(SESSION_ID)).generating).toBe(true) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + await expect(pending).rejects.toBeInstanceOf(WalkthroughCancelledError) + }) + }) + + describe('cancellation before the model call', () => { + it('does not persist a mechanical-only walkthrough cancelled during the changes read', async () => { + sessions[SESSION_ID]!.info = { + id: SESSION_ID, + title: 'Title', + model: { providerID: 'anthropic', id: 'claude-sonnet-4' }, + } as SessionInfo + fake.setGenerateImpl(async () => modelReply([{ title: 'A', explanation: 'x', hunkIds: THREE_HUNK_IDS }])) + const previous = await service.generate(SESSION_ID, {}) + expect(previous.created).toBe(true) + + const mechanical = [change('pnpm-lock.yaml', hunkPatch(1))] + sessions[SESSION_ID]!.changes = mechanical + const diff = createDeferred() + const diffMock = vi.mocked(fake.client.api.session.diff) + diffMock.mockImplementationOnce(() => diff.promise) + + const pending = service.generate(SESSION_ID, {}) + await vi.waitFor(() => expect(diffMock).toHaveBeenCalledTimes(2)) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + diff.resolve(mechanical) + + await expect(pending).rejects.toBeInstanceOf(WalkthroughCancelledError) + expect(fake.generateCalls).toHaveLength(1) + expect(getChangeWalkthrough(db, SESSION_ID, 'session')).toEqual(previous.walkthrough) + }) + + it('cancels while resolving the preferred model without persisting the mechanical-only changes', async () => { + sessions[SESSION_ID]!.info = { + id: SESSION_ID, + title: 'Title', + model: { providerID: 'anthropic', id: 'claude-sonnet-4' }, + } as SessionInfo + sessions[SESSION_ID]!.changes = [change('pnpm-lock.yaml', hunkPatch(1))] + + const config = createDeferred>>() + const configMock = vi.mocked(fake.client.api.config.get) + configMock.mockImplementationOnce(() => config.promise) + + const pending = service.generate(SESSION_ID, {}) + await vi.waitFor(() => expect(configMock).toHaveBeenCalledTimes(1)) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + config.resolve([]) + + await expect(pending).rejects.toBeInstanceOf(WalkthroughCancelledError) + expect(fake.generateCalls).toHaveLength(0) + expect(getChangeWalkthrough(db, SESSION_ID, 'session')).toBeNull() + expect((await service.getState(SESSION_ID)).error).toBeNull() + }) + + it('cancels while resolving the default model without a 502 failure', async () => { + const config = createDeferred>>() + const configMock = vi.mocked(fake.client.api.config.get) + configMock.mockImplementationOnce(() => config.promise) + + const pending = service.generate(SESSION_ID, {}) + await vi.waitFor(() => expect(configMock).toHaveBeenCalledTimes(1)) + + expect(service.cancelGeneration(SESSION_ID)).toBe(true) + config.resolve([]) + + await expect(pending).rejects.toBeInstanceOf(WalkthroughCancelledError) + expect(fake.generateCalls).toHaveLength(0) + expect((await service.getState(SESSION_ID)).error).toBeNull() + }) + }) }) diff --git a/frontend/src/api/changeWalkthroughs.ts b/frontend/src/api/changeWalkthroughs.ts index cd193742a..082e14dd8 100644 --- a/frontend/src/api/changeWalkthroughs.ts +++ b/frontend/src/api/changeWalkthroughs.ts @@ -1,4 +1,4 @@ -import { fetchWrapper } from './fetchWrapper' +import { fetchWrapper, fetchWrapperVoid } from './fetchWrapper' import { API_BASE_URL } from '@/config' import { walkthroughSourceKey } from '@opencode-manager/shared/schemas' import type { @@ -35,3 +35,13 @@ export async function generateChangeWalkthrough( body: JSON.stringify(request), }) } + +export async function cancelChangeWalkthrough( + sessionId: string, + source: WalkthroughSource, +): Promise { + await fetchWrapperVoid(walkthroughUrl(sessionId), { + method: 'DELETE', + params: { source: walkthroughSourceKey(source) }, + }) +} diff --git a/frontend/src/components/navigation/ToolSidePanel.test.tsx b/frontend/src/components/navigation/ToolSidePanel.test.tsx index 4c2a292bb..78873319f 100644 --- a/frontend/src/components/navigation/ToolSidePanel.test.tsx +++ b/frontend/src/components/navigation/ToolSidePanel.test.tsx @@ -28,6 +28,7 @@ vi.mock('@/components/session/ChangesWalkthroughSheet', () => ({ ChangesWalkthroughNav: () => null, ChangesWalkthroughRegenerate: () => null, ChangesWalkthroughSourcePicker: () => null, + ChangesWalkthroughStop: () => null, ChangesWalkthroughView: () =>
walkthrough-tool
, })) diff --git a/frontend/src/components/navigation/ToolSidePanel.tsx b/frontend/src/components/navigation/ToolSidePanel.tsx index 0debe3135..0a3697a88 100644 --- a/frontend/src/components/navigation/ToolSidePanel.tsx +++ b/frontend/src/components/navigation/ToolSidePanel.tsx @@ -9,6 +9,7 @@ import { ChangesWalkthroughProvider, ChangesWalkthroughRegenerate, ChangesWalkthroughSourcePicker, + ChangesWalkthroughStop, ChangesWalkthroughView, type WalkthroughSourceRequest, } from '@/components/session/ChangesWalkthroughSheet' @@ -18,6 +19,7 @@ import { RepoActionsContent } from '@/components/repo/RepoActionsDialog' import { RepoSkillsContent } from '@/components/repo/RepoSkillsDialog' import { RepoSchedulesContent } from '@/components/schedules/RepoSchedulesContent' import type { SkillFileInfo } from '@opencode-manager/shared' +import type { ModelRef } from '@opencode-manager/shared/opencode' import { buildToolItems, toolKeyOf, type MoreDrawerItem } from '@/components/navigation/moreDrawerItems' import { isPanelTool, type ToolPanelState, type PanelTool } from '@/hooks/useToolPanel' import { useOpenNavItem } from '@/hooks/useOpenNavItem' @@ -38,6 +40,7 @@ interface ToolSidePanelProps { selectedFilePath?: string onSkillLoaded?: (skill: SkillFileInfo) => void walkthroughSourceRequest?: WalkthroughSourceRequest + walkthroughModel?: ModelRef | null } interface RailButtonProps { @@ -92,6 +95,7 @@ export function ToolSidePanel({ selectedFilePath, onSkillLoaded, walkthroughSourceRequest, + walkthroughModel, }: ToolSidePanelProps) { const { activeTool, toggleTool, closePanel } = panel const location = useLocation() @@ -172,6 +176,7 @@ export function ToolSidePanel({ {activeTool === 'walkthrough' ? ( <> + @@ -198,6 +203,7 @@ export function ToolSidePanel({ sessionId={sessionId} active sourceRequest={walkthroughSourceRequest} + model={walkthroughModel} > {panelContent} diff --git a/frontend/src/components/session/ChangesWalkthroughSheet.test.tsx b/frontend/src/components/session/ChangesWalkthroughSheet.test.tsx index e9e991ced..ad6fa5614 100644 --- a/frontend/src/components/session/ChangesWalkthroughSheet.test.tsx +++ b/frontend/src/components/session/ChangesWalkthroughSheet.test.tsx @@ -10,11 +10,13 @@ import type { ChangeWalkthrough, ChangeWalkthroughState } from '@opencode-manage const mocks = vi.hoisted(() => ({ getChangeWalkthrough: vi.fn(), generateChangeWalkthrough: vi.fn(), + cancelChangeWalkthrough: vi.fn(), })) vi.mock('@/api/changeWalkthroughs', () => ({ getChangeWalkthrough: mocks.getChangeWalkthrough, generateChangeWalkthrough: mocks.generateChangeWalkthrough, + cancelChangeWalkthrough: mocks.cancelChangeWalkthrough, })) const walkthrough: ChangeWalkthrough = { @@ -169,6 +171,22 @@ describe('ChangesWalkthroughSheet', () => { expect(screen.getByText('assets/logo.png — binary file')).toBeInTheDocument() }) + it('generates with the selected model', async () => { + const user = userEvent.setup() + mocks.getChangeWalkthrough.mockResolvedValue(state({ walkthrough: null })) + mocks.generateChangeWalkthrough.mockResolvedValue(state()) + renderSheet({ model: { providerID: 'anthropic', id: 'claude', variant: 'high' } }) + + await user.click(await screen.findByRole('button', { name: 'Generate walkthrough' })) + + await waitFor(() => { + expect(mocks.generateChangeWalkthrough).toHaveBeenCalledWith('ses_1', { + source: { kind: 'session' }, + model: 'anthropic/claude#high', + }) + }) + }) + it('opens a stop from the overview list without the summary or list', async () => { const user = userEvent.setup() mocks.getChangeWalkthrough.mockResolvedValue(state()) @@ -265,6 +283,28 @@ describe('ChangesWalkthroughSheet', () => { }) }) + it('stops an in-flight generation from the header', async () => { + const user = userEvent.setup() + mocks.getChangeWalkthrough.mockResolvedValue(state({ generating: true })) + mocks.cancelChangeWalkthrough.mockResolvedValue(undefined) + renderSheet() + + const stop = await screen.findByRole('button', { name: 'Stop generating walkthrough' }) + await user.click(stop) + + await waitFor(() => { + expect(mocks.cancelChangeWalkthrough).toHaveBeenCalledWith('ses_1', { kind: 'session' }) + }) + }) + + it('does not show a stop button when idle', async () => { + mocks.getChangeWalkthrough.mockResolvedValue(state()) + renderSheet() + + await screen.findByText('Overview') + expect(screen.queryByRole('button', { name: 'Stop generating walkthrough' })).not.toBeInTheDocument() + }) + it('switches to staged changes', async () => { const user = userEvent.setup() mocks.getChangeWalkthrough.mockResolvedValue(state({ walkthrough: null })) diff --git a/frontend/src/components/session/ChangesWalkthroughSheet.tsx b/frontend/src/components/session/ChangesWalkthroughSheet.tsx index dbbad664b..e1d091bfb 100644 --- a/frontend/src/components/session/ChangesWalkthroughSheet.tsx +++ b/frontend/src/components/session/ChangesWalkthroughSheet.tsx @@ -1,6 +1,6 @@ import { createContext, memo, useCallback, useContext, useEffect, useRef, useState } from 'react' import type { ReactNode, RefObject } from 'react' -import { ChevronDown, ChevronLeft, ChevronRight, Loader2, RefreshCw } from 'lucide-react' +import { ChevronDown, ChevronLeft, ChevronRight, Loader2, RefreshCw, Square } from 'lucide-react' import { SideDrawer, SideDrawerHeader } from '@/components/ui/side-drawer' import { Button } from '@/components/ui/button' import { @@ -16,7 +16,11 @@ import { Input } from '@/components/ui/input' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { DiffLines } from '@/components/file-browser/DiffLines' import { ScheduleRunMarkdown } from '@/components/schedules/ScheduleRunMarkdown' -import { useChangeWalkthrough, useGenerateChangeWalkthrough } from '@/hooks/useChangeWalkthrough' +import { + useCancelChangeWalkthrough, + useChangeWalkthrough, + useGenerateChangeWalkthrough, +} from '@/hooks/useChangeWalkthrough' import { GIT_STATUS_COLORS, GIT_STATUS_LABELS } from '@/lib/git-status-styles' import { cn } from '@/lib/utils' import { @@ -33,6 +37,7 @@ import type { WalkthroughSource, WalkthroughStop, } from '@opencode-manager/shared/schemas' +import { formatOpenCodeModelRef, type ModelRef } from '@opencode-manager/shared/opencode' interface WalkthroughErrorLike { message?: string @@ -129,6 +134,8 @@ interface ChangesWalkthroughContextValue { contextLimitFiles: WalkthroughOmittedFile[] generate: () => void regenerate: () => void + stop: () => void + stopping: boolean stops: WalkthroughStop[] stopIndex: number | null selectedStop: WalkthroughStop | null @@ -251,6 +258,7 @@ interface ChangesWalkthroughProviderProps { sessionId: string active: boolean sourceRequest?: WalkthroughSourceRequest + model?: ModelRef | null children: ReactNode } @@ -259,6 +267,7 @@ export function ChangesWalkthroughProvider({ sessionId, active, sourceRequest, + model, children, }: ChangesWalkthroughProviderProps) { const [source, setSource] = useState(DEFAULT_WALKTHROUGH_SOURCE) @@ -268,6 +277,8 @@ export function ChangesWalkthroughProvider({ const sourceKey = walkthroughSourceKey(source) const stateQuery = useChangeWalkthrough(sessionId, active, source) const generate = useGenerateChangeWalkthrough(sessionId, source) + const cancel = useCancelChangeWalkthrough(sessionId, source) + const stop = useCallback(() => cancel.mutate(), [cancel]) const resetGenerate = generate.reset const [stopIndex, setStopIndex] = useState(null) const scrollRef = useRef(null) @@ -319,6 +330,8 @@ export function ChangesWalkthroughProvider({ (stop) => stop.status === 'failed' || (stop.status === 'pending' && !generating), ).length + const modelRequest = model ? { model: formatOpenCodeModelRef(model) } : {} + const selectStop = useCallback((index: number | null) => { setStopIndex(index) scrollRef.current?.scrollTo?.({ top: 0 }) @@ -331,8 +344,10 @@ export function ChangesWalkthroughProvider({ generating, error, contextLimitFiles, - generate: () => generate.mutate({}), - regenerate: () => generate.mutate({ regenerate: true }), + generate: () => generate.mutate(modelRequest), + regenerate: () => generate.mutate({ ...modelRequest, regenerate: true }), + stop, + stopping: cancel.isPending, stops, stopIndex: clampedIndex, selectedStop, @@ -406,6 +421,26 @@ export const ChangesWalkthroughNav = memo(function ChangesWalkthroughNav() { ) }) +export const ChangesWalkthroughStop = memo(function ChangesWalkthroughStop() { + const { generating, stopping, stop } = useChangesWalkthrough() + + if (!generating) return null + + return ( + + ) +}) + /** Regenerates the current walkthrough from the chrome; renders nothing until one exists. */ export const ChangesWalkthroughRegenerate = memo(function ChangesWalkthroughRegenerate() { const { walkthrough, generating, regenerate, sourcePending } = useChangesWalkthrough() @@ -657,6 +692,7 @@ interface ChangesWalkthroughSheetProps { open: boolean onOpenChange: (open: boolean) => void sourceRequest?: WalkthroughSourceRequest + model?: ModelRef | null } /** The walkthrough as a right-side sheet: full screen on mobile, a drawer over the page on desktop. */ @@ -665,18 +701,20 @@ export function ChangesWalkthroughSheet({ open, onOpenChange, sourceRequest, + model, }: ChangesWalkthroughSheetProps) { const close = useCallback(() => onOpenChange(false), [onOpenChange]) return ( - + + diff --git a/frontend/src/hooks/useChangeWalkthrough.test.tsx b/frontend/src/hooks/useChangeWalkthrough.test.tsx index dfeb28f70..6b9383de9 100644 --- a/frontend/src/hooks/useChangeWalkthrough.test.tsx +++ b/frontend/src/hooks/useChangeWalkthrough.test.tsx @@ -2,7 +2,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { act, renderHook, waitFor } from '@testing-library/react' import type { ReactNode } from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { changeWalkthroughQueryKey, useChangeWalkthrough, useGenerateChangeWalkthrough } from './useChangeWalkthrough' +import { changeWalkthroughQueryKey, useCancelChangeWalkthrough, useChangeWalkthrough, useGenerateChangeWalkthrough } from './useChangeWalkthrough' import { DEFAULT_WALKTHROUGH_SOURCE, walkthroughHunksIdentity } from '@opencode-manager/shared/schemas' import type { ChangeWalkthrough, @@ -13,11 +13,13 @@ import type { const mocks = vi.hoisted(() => ({ getChangeWalkthrough: vi.fn(), generateChangeWalkthrough: vi.fn(), + cancelChangeWalkthrough: vi.fn(), })) vi.mock('@/api/changeWalkthroughs', () => ({ getChangeWalkthrough: mocks.getChangeWalkthrough, generateChangeWalkthrough: mocks.generateChangeWalkthrough, + cancelChangeWalkthrough: mocks.cancelChangeWalkthrough, })) const walkthroughForA: ChangeWalkthrough = { @@ -187,3 +189,30 @@ describe('useChangeWalkthrough', () => { await waitFor(() => expect(mocks.getChangeWalkthrough).toHaveBeenCalledTimes(4)) }) }) + +describe('useCancelChangeWalkthrough', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('cancels the walkthrough and optimistically clears the generating state', async () => { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const queryKey = changeWalkthroughQueryKey('ses_A', 'session') + queryClient.setQueryData(queryKey, state({ generating: true })) + mocks.cancelChangeWalkthrough.mockResolvedValue(undefined) + mocks.getChangeWalkthrough.mockResolvedValue(state({ generating: false })) + + const { result } = renderHook(() => useCancelChangeWalkthrough('ses_A', DEFAULT_WALKTHROUGH_SOURCE), { + wrapper: createWrapper(queryClient), + }) + + await act(async () => { + result.current.mutate() + }) + + expect(mocks.cancelChangeWalkthrough).toHaveBeenCalledWith('ses_A', { kind: 'session' }) + await waitFor(() => { + expect(queryClient.getQueryData(queryKey)?.generating).toBe(false) + }) + }) +}) diff --git a/frontend/src/hooks/useChangeWalkthrough.ts b/frontend/src/hooks/useChangeWalkthrough.ts index 5d70d9ad6..b3b3487e4 100644 --- a/frontend/src/hooks/useChangeWalkthrough.ts +++ b/frontend/src/hooks/useChangeWalkthrough.ts @@ -1,5 +1,5 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' -import { generateChangeWalkthrough, getChangeWalkthrough } from '@/api/changeWalkthroughs' +import { cancelChangeWalkthrough, generateChangeWalkthrough, getChangeWalkthrough } from '@/api/changeWalkthroughs' import { walkthroughHunksIdentity, walkthroughSourceKey } from '@opencode-manager/shared/schemas' import type { ChangeWalkthrough, @@ -81,3 +81,23 @@ export function useGenerateChangeWalkthrough(sessionId: string | undefined, sour }, }) } + +export function useCancelChangeWalkthrough(sessionId: string | undefined, source: WalkthroughSource) { + const queryClient = useQueryClient() + const sourceKey = walkthroughSourceKey(source) + + return useMutation({ + mutationFn: async () => { + const targetSessionId = sessionId! + await cancelChangeWalkthrough(targetSessionId, source) + return { sessionId: targetSessionId, sourceKey } + }, + onSuccess: ({ sessionId: targetSessionId, sourceKey: targetSourceKey }) => { + const queryKey = changeWalkthroughQueryKey(targetSessionId, targetSourceKey) + queryClient.setQueryData(queryKey, (cached) => + cached ? { ...cached, generating: false, error: null } : cached, + ) + queryClient.invalidateQueries({ queryKey }) + }, + }) +} diff --git a/frontend/src/pages/SessionDetail.tsx b/frontend/src/pages/SessionDetail.tsx index 69c3502b6..18cb7c94a 100644 --- a/frontend/src/pages/SessionDetail.tsx +++ b/frontend/src/pages/SessionDetail.tsx @@ -846,6 +846,7 @@ export function SessionDetail() { repoDirectory={repoDirectory} onSkillLoaded={handleSkillLoaded} walkthroughSourceRequest={walkthroughSourceRequest} + walkthroughModel={modelRef} /> )} @@ -922,6 +923,7 @@ export function SessionDetail() { open={!docked && walkthroughOpen} onOpenChange={setWalkthroughOpen} sourceRequest={walkthroughSourceRequest} + model={modelRef} /> )} diff --git a/shared/src/schemas/change-walkthroughs.ts b/shared/src/schemas/change-walkthroughs.ts index aec74059d..4c40d5cdb 100644 --- a/shared/src/schemas/change-walkthroughs.ts +++ b/shared/src/schemas/change-walkthroughs.ts @@ -187,6 +187,7 @@ export function walkthroughHunksIdentity( export const GenerateChangeWalkthroughRequestSchema = z.object({ regenerate: z.boolean().optional(), source: WalkthroughSourceSchema.optional(), + model: z.string().trim().min(1).max(512).optional(), }); export type GenerateChangeWalkthroughRequest = z.infer;