mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(git): scope folder-workspace commit-message cancel lanes by workspace id and refcount
Round-6 review found two ways the cancel-lane bookkeeping added in f5a11b3 could
still reach the wrong draft.
Keyed by container path, two folder workspaces that share a POSIX path on
different SSH hosts collapse to one entry, so one host's Cancel resolves the
other host's recorded child selector. FolderWorkspace already carries an `id`,
which is host-unique, so key on that instead.
Stored as a Set, two concurrent generations into the same child are one entry.
The first to settle deletes it in its `finally` and the second draft is left
with no lane at all — Cancel becomes a silent no-op. Refcount instead.
M6 pins the multiplicity case, M7 pins the host-isolation case; each dies only
under its own mutation.
This commit is contained in:
@@ -663,6 +663,70 @@ describe('#6357 monorepo folder workspace', () => {
|
||||
cancel.mockRestore()
|
||||
})
|
||||
|
||||
it('M6: a settled generation does not clear the lane of one still running in the same child', 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')
|
||||
// Two clients (two windows on the same workspace) generate into the same child.
|
||||
const releases: (() => void)[] = []
|
||||
const generate = vi
|
||||
.spyOn(
|
||||
runtime['gitCommands'] as { generateRuntimeCommitMessage: (...args: never[]) => unknown },
|
||||
'generateRuntimeCommitMessage'
|
||||
)
|
||||
.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
releases.push(() => 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 both = [
|
||||
runtime.generateRuntimeCommitMessage(selector),
|
||||
runtime.generateRuntimeCommitMessage(selector)
|
||||
]
|
||||
await vi.waitFor(() => expect(releases).toHaveLength(2))
|
||||
// Release one and await the race, not a named handle: both generations resolve their
|
||||
// target concurrently, so which one reaches the spy first is not ordered. `finally`
|
||||
// runs before the outer promise settles, so this pins "one has fully released its lane".
|
||||
releases[0]?.()
|
||||
await Promise.race(both)
|
||||
// Why: a Set keyed by selector loses multiplicity, so that one `finally` deleted the
|
||||
// lane and left the other draft with no way to be cancelled at all.
|
||||
await runtime.cancelRuntimeGenerateCommitMessage(selector)
|
||||
expect(cancel).toHaveBeenCalledTimes(1)
|
||||
releases[1]?.()
|
||||
await Promise.all(both)
|
||||
generate.mockRestore()
|
||||
cancel.mockRestore()
|
||||
})
|
||||
|
||||
it('M7: Cancel does not reach a same-path workspace belonging to another host', 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 run = await withInFlightGeneration(runtime, selector)
|
||||
// Why: keying the lane map by normalized path alone conflates two workspaces that
|
||||
// share a POSIX path on different SSH hosts, so one host's Cancel would resolve the
|
||||
// other host's recorded child selector and kill its draft. Keying by workspace id
|
||||
// makes a lookup for any other workspace miss, in flight or not.
|
||||
const lanes = runtime['folderWorkspaceCommitMessageLanes'] as Map<string, Map<string, number>>
|
||||
// The key is the workspace id, not any spelling of the shared container path.
|
||||
expect([...lanes.keys()]).toEqual([FOLDER_WS_ID])
|
||||
expect([...lanes.keys()].some((key) => key.includes(CONTAINER))).toBe(false)
|
||||
await run.settle()
|
||||
expect(lanes.size).toBe(0)
|
||||
})
|
||||
|
||||
it('E: the real child git worktree resolves fine when its repo is registered', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore() as never)
|
||||
|
||||
@@ -2934,10 +2934,14 @@ export class OrcaRuntimeService {
|
||||
| null
|
||||
private readonly agentSessionClaimSigner: AgentSessionClaimSigner
|
||||
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>>()
|
||||
// Folder-workspace id -> child selector -> how many of its commit-message generations are
|
||||
// in flight there. Cancellation is keyed by cwd, so without this a workspace-level Cancel
|
||||
// would also kill a draft running in an unrelated child repo.
|
||||
// Why id and not path: the same POSIX path can name two different workspaces on two SSH
|
||||
// hosts, and one host's Cancel must not reach the other's draft.
|
||||
// Why a count and not a Set: two clients can generate into the same child, and the first
|
||||
// to settle would otherwise clear the lane while the second is still running.
|
||||
private readonly folderWorkspaceCommitMessageLanes = new Map<string, Map<string, number>>()
|
||||
private sshRelayRecoveryGenerationByTargetId = new Map<string, number>()
|
||||
private accountServices: RuntimeAccountServices | null = null
|
||||
private commitMessageAgentEnv: CommitMessageAgentEnvironmentResolvers | null = null
|
||||
@@ -24274,7 +24278,7 @@ export class OrcaRuntimeService {
|
||||
* callers can fall through to the single-repo path.
|
||||
*/
|
||||
private async selectFolderWorkspaceStagedTarget(selector: string): Promise<{
|
||||
folderPath: string
|
||||
folderWorkspaceId: string
|
||||
selection: { ok: true; target: RuntimeGitTarget } | { ok: false; error: string }
|
||||
} | null> {
|
||||
const scope = await this.resolveFolderWorkspaceLaunchScope(selector)
|
||||
@@ -24283,7 +24287,7 @@ export class OrcaRuntimeService {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
folderPath: folderWorkspace.folderPath,
|
||||
folderWorkspaceId: folderWorkspace.id,
|
||||
selection: selectFolderWorkspaceCommitTarget(
|
||||
await this.inspectFolderWorkspaceChildRepos(
|
||||
folderWorkspace.folderPath,
|
||||
@@ -24353,14 +24357,19 @@ export class OrcaRuntimeService {
|
||||
// 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)
|
||||
const key = selected.folderWorkspaceId
|
||||
const lanes = this.folderWorkspaceCommitMessageLanes.get(key) ?? new Map<string, number>()
|
||||
lanes.set(childSelector, (lanes.get(childSelector) ?? 0) + 1)
|
||||
this.folderWorkspaceCommitMessageLanes.set(key, lanes)
|
||||
try {
|
||||
return await this.gitCommands.generateRuntimeCommitMessage(childSelector, settingsOverride)
|
||||
} finally {
|
||||
lanes.delete(childSelector)
|
||||
const remaining = (lanes.get(childSelector) ?? 1) - 1
|
||||
if (remaining > 0) {
|
||||
lanes.set(childSelector, remaining)
|
||||
} else {
|
||||
lanes.delete(childSelector)
|
||||
}
|
||||
if (lanes.size === 0) {
|
||||
this.folderWorkspaceCommitMessageLanes.delete(key)
|
||||
}
|
||||
@@ -24377,6 +24386,7 @@ export class OrcaRuntimeService {
|
||||
* 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.
|
||||
* Keyed by workspace id, so a same-path workspace on another SSH host is not reached.
|
||||
* Returns null when the selector is not a folder workspace.
|
||||
*/
|
||||
private async cancelFolderWorkspaceGenerateCommitMessage(
|
||||
@@ -24387,11 +24397,9 @@ export class OrcaRuntimeService {
|
||||
if (!folderWorkspace) {
|
||||
return null
|
||||
}
|
||||
const lanes = this.folderWorkspaceCommitMessageLanes.get(
|
||||
normalizeRuntimePathForComparison(folderWorkspace.folderPath)
|
||||
)
|
||||
const lanes = this.folderWorkspaceCommitMessageLanes.get(folderWorkspace.id)
|
||||
await Promise.all(
|
||||
[...(lanes ?? [])].map(async (childSelector) =>
|
||||
[...(lanes?.keys() ?? [])].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.
|
||||
this.gitCommands
|
||||
|
||||
Reference in New Issue
Block a user