mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(runtime): cancel only the child repo a folder-workspace draft is running in
Cancel fanned out to every child repo in the workspace. Cancellation is keyed by cwd alone, with no notion of who asked, so a concurrent draft the user started from a sibling repo's own view was killed too — and the renderer keeps generation per-worktree precisely so navigating away does not cancel it, which makes that concurrent draft reachable rather than theoretical. Record the child selector each folder-workspace generation actually routed to and cancel only those. A stale Cancel with nothing in flight now signals no child at all, instead of acting as a kill switch for whatever runs there next.
This commit is contained in:
@@ -545,6 +545,52 @@ describe('#6357 monorepo folder workspace', () => {
|
|||||||
expect(await runtime.getRuntimeGitConflictOperation(`path:${CHILD_API}`)).toBe('merge')
|
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<void>
|
||||||
|
}> {
|
||||||
|
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 () => {
|
it('M3: cancelling generation reaches the child repo instead of dying on the folder selector', async () => {
|
||||||
buildFixture()
|
buildFixture()
|
||||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
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
|
// to the child path. Passing the folder selector straight through threw
|
||||||
// `selector_not_found`, and every caller is fire-and-forget — so Cancel silently
|
// `selector_not_found`, and every caller is fire-and-forget — so Cancel silently
|
||||||
// did nothing and the draft kept running.
|
// 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(
|
.spyOn(
|
||||||
runtime['gitCommands'] as {
|
runtime['gitCommands'] as {
|
||||||
cancelRuntimeGenerateCommitMessage: (...args: never[]) => unknown
|
cancelRuntimeGenerateCommitMessage: (...args: never[]) => unknown
|
||||||
@@ -562,16 +654,13 @@ describe('#6357 monorepo folder workspace', () => {
|
|||||||
'cancelRuntimeGenerateCommitMessage'
|
'cancelRuntimeGenerateCommitMessage'
|
||||||
)
|
)
|
||||||
.mockResolvedValue({ ok: true } as never)
|
.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({
|
await expect(runtime.cancelRuntimeGenerateCommitMessage(selector)).resolves.toEqual({
|
||||||
ok: true
|
ok: true
|
||||||
})
|
})
|
||||||
const routed = inner.mock.calls.map((call) => String(call[0]))
|
expect(cancel).not.toHaveBeenCalled()
|
||||||
expect(routed).not.toContain(selector)
|
cancel.mockRestore()
|
||||||
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()
|
|
||||||
})
|
})
|
||||||
|
|
||||||
it('E: the real child git worktree resolves fine when its repo is registered', async () => {
|
it('E: the real child git worktree resolves fine when its repo is registered', async () => {
|
||||||
|
|||||||
@@ -2934,6 +2934,10 @@ export class OrcaRuntimeService {
|
|||||||
| null
|
| null
|
||||||
private readonly agentSessionClaimSigner: AgentSessionClaimSigner
|
private readonly agentSessionClaimSigner: AgentSessionClaimSigner
|
||||||
private readonly agentSessionCreateOperations = new Map<string, AgentSessionCreateOperation>()
|
private readonly agentSessionCreateOperations = new Map<string, AgentSessionCreateOperation>()
|
||||||
|
// 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<string, Set<string>>()
|
||||||
private sshRelayRecoveryGenerationByTargetId = new Map<string, number>()
|
private sshRelayRecoveryGenerationByTargetId = new Map<string, number>()
|
||||||
private accountServices: RuntimeAccountServices | null = null
|
private accountServices: RuntimeAccountServices | null = null
|
||||||
private commitMessageAgentEnv: CommitMessageAgentEnvironmentResolvers | 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
|
* The one child repo a staged-index operation belongs to, plus the workspace it
|
||||||
* is not a folder workspace, so callers can fall through to the single-repo path.
|
* 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(
|
private async selectFolderWorkspaceStagedTarget(selector: string): Promise<{
|
||||||
selector: string
|
folderPath: string
|
||||||
): Promise<{ ok: true; target: RuntimeGitTarget } | { ok: false; error: string } | null> {
|
selection: { ok: true; target: RuntimeGitTarget } | { ok: false; error: string }
|
||||||
|
} | null> {
|
||||||
const scope = await this.resolveFolderWorkspaceLaunchScope(selector)
|
const scope = await this.resolveFolderWorkspaceLaunchScope(selector)
|
||||||
const folderWorkspace = scope?.folderWorkspace
|
const folderWorkspace = scope?.folderWorkspace
|
||||||
if (!folderWorkspace) {
|
if (!folderWorkspace) {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
return selectFolderWorkspaceCommitTarget(
|
return {
|
||||||
await this.inspectFolderWorkspaceChildRepos(
|
folderPath: folderWorkspace.folderPath,
|
||||||
folderWorkspace.folderPath,
|
selection: selectFolderWorkspaceCommitTarget(
|
||||||
probeChildRepoHasStagedChanges
|
await this.inspectFolderWorkspaceChildRepos(
|
||||||
|
folderWorkspace.folderPath,
|
||||||
|
probeChildRepoHasStagedChanges
|
||||||
|
)
|
||||||
)
|
)
|
||||||
)
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -24305,11 +24314,11 @@ export class OrcaRuntimeService {
|
|||||||
if (message.trim().length === 0) {
|
if (message.trim().length === 0) {
|
||||||
throw new Error('Commit message is required')
|
throw new Error('Commit message is required')
|
||||||
}
|
}
|
||||||
if (!selected.ok) {
|
if (!selected.selection.ok) {
|
||||||
return { success: false, error: selected.error }
|
return { success: false, error: selected.selection.error }
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
return await commitRuntimeGitForTarget(selected.target, message)
|
return await commitRuntimeGitForTarget(selected.selection.target, message)
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return { success: false, error: error instanceof Error ? error.message : 'Commit failed' }
|
return { success: false, error: error instanceof Error ? error.message : 'Commit failed' }
|
||||||
}
|
}
|
||||||
@@ -24330,19 +24339,32 @@ export class OrcaRuntimeService {
|
|||||||
if (!selected) {
|
if (!selected) {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
if (!selected.ok) {
|
if (!selected.selection.ok) {
|
||||||
return {
|
return {
|
||||||
success: false,
|
success: false,
|
||||||
error:
|
error:
|
||||||
selected.error === 'nothing to commit'
|
selected.selection.error === 'nothing to commit'
|
||||||
? 'No staged changes to summarize.'
|
? '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<string>()
|
||||||
|
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
|
* keyed to that child's path — the folder selector resolves to no worktree and
|
||||||
* would just throw `selector_not_found`, leaving Cancel silently dead.
|
* would just throw `selector_not_found`, leaving Cancel silently dead.
|
||||||
*
|
*
|
||||||
* Why cancel every child rather than re-select the staged one: selection is a
|
* Why only the recorded lanes: cancellation is keyed by cwd alone, so signalling
|
||||||
* fresh read, and staging can change between generate and cancel, so it can
|
* every child would abort a draft another view started in a *different* child
|
||||||
* name a different repo than the one actually generating. Both cancel paths are
|
* repo. `folderWorkspaceCommitMessageLanes` holds exactly the children this
|
||||||
* cwd-keyed best-effort no-ops when nothing is in flight, so the broadest
|
* workspace's own generations are running in, so nothing else is touched.
|
||||||
* correct move is to signal every lane this selector could have started.
|
|
||||||
* Returns null when the selector is not a folder workspace.
|
* Returns null when the selector is not a folder workspace.
|
||||||
*/
|
*/
|
||||||
private async cancelFolderWorkspaceGenerateCommitMessage(
|
private async cancelFolderWorkspaceGenerateCommitMessage(
|
||||||
@@ -24366,27 +24387,22 @@ export class OrcaRuntimeService {
|
|||||||
if (!folderWorkspace) {
|
if (!folderWorkspace) {
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
const childRepos = listFolderWorkspaceChildRepos(
|
const lanes = this.folderWorkspaceCommitMessageLanes.get(
|
||||||
this.requireStore().getRepos(),
|
normalizeRuntimePathForComparison(folderWorkspace.folderPath)
|
||||||
folderWorkspace.folderPath
|
|
||||||
)
|
)
|
||||||
await Promise.all(
|
await Promise.all(
|
||||||
childRepos.map(async (repo) => {
|
[...(lanes ?? [])].map(async (childSelector) =>
|
||||||
const target = await this.resolveChildRepoGitTarget(repo)
|
|
||||||
if (!target) {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
// Why swallow: a child that cannot be reached has nothing in flight to
|
// Why swallow: a child that cannot be reached has nothing in flight to
|
||||||
// cancel, and one unreachable repo must not strand the others.
|
// cancel, and one unreachable repo must not strand the others.
|
||||||
await this.gitCommands
|
this.gitCommands
|
||||||
.cancelRuntimeGenerateCommitMessage(`id:${target.worktree.id}`)
|
.cancelRuntimeGenerateCommitMessage(childSelector)
|
||||||
.catch((error: unknown) => {
|
.catch((error: unknown) => {
|
||||||
console.warn('[runtime] folder-workspace commit-message cancel failed', {
|
console.warn('[runtime] folder-workspace commit-message cancel failed', {
|
||||||
repo: repo.path,
|
childSelector,
|
||||||
error
|
error
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
)
|
||||||
)
|
)
|
||||||
return { ok: true }
|
return { ok: true }
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user