mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(runtime): commit and abort across a folder workspace's child repos
git.commit and the merge/rebase aborts carry no path, so the child-repo routing the rest of this PR uses had nothing to resolve against and they still failed with selector_not_found. That left the workspace Source Control able to list and stage files but not commit them — an affordance made reachable without being made to work. Commit fans out to every child repo with a staged index, which is exactly the set the merged status just showed the user as staged; repos with an empty index are skipped rather than reporting 'nothing to commit'. A partial failure names the repos that failed instead of collapsing to one blanket result. Abort targets only the child repos actually in that conflict state, so a no-op abort settles like it does for a single repo instead of erroring. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -57,6 +57,31 @@ export function matchFolderWorkspaceChildRepo(
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Combine per-child-repo results for an op that has no path to route on. Every
|
||||
* child either succeeded or reports why it did not, so a partial outcome is
|
||||
* described rather than reported as one blanket success or failure.
|
||||
*/
|
||||
export function summarizeFolderWorkspaceFanOut(
|
||||
results: readonly { repoName: string; error?: string }[]
|
||||
): { success: boolean; error?: string } {
|
||||
const failures = results.filter((result) => result.error !== undefined)
|
||||
if (failures.length === 0) {
|
||||
return { success: true }
|
||||
}
|
||||
// Why: naming the repo matters most when only some of them failed — the user
|
||||
// has to know which ones still need attention.
|
||||
const detail = failures.map((failure) => `${failure.repoName}: ${failure.error}`).join('; ')
|
||||
if (failures.length === results.length) {
|
||||
return { success: false, error: detail }
|
||||
}
|
||||
const succeeded = results.length - failures.length
|
||||
return {
|
||||
success: false,
|
||||
error: `Committed ${succeeded} of ${results.length} repos. Failed — ${detail}`
|
||||
}
|
||||
}
|
||||
|
||||
/** Prefix a child repo's status entry path so it stays addressable from the workspace root. */
|
||||
export function prefixFolderWorkspaceEntryPath(
|
||||
folderPath: string,
|
||||
|
||||
@@ -276,6 +276,83 @@ describe('#6357 monorepo folder workspace', () => {
|
||||
expect(status.entries.every((entry) => entry.area !== 'staged')).toBe(true)
|
||||
})
|
||||
|
||||
it('K: commit has no path to route on, so it commits every child repo with a staged index', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
await runtime.bulkStageRuntimeGitPaths(selector, [
|
||||
'fint_api/src/app.ts',
|
||||
'fint-portal/src/app.ts'
|
||||
])
|
||||
await expect(runtime.commitRuntimeGit(selector, 'span two repos')).resolves.toEqual({
|
||||
success: true
|
||||
})
|
||||
for (const dir of [CHILD_API, join(CONTAINER, 'fint-portal')]) {
|
||||
expect(git(dir, 'log', '-1', '--pretty=%s').trim()).toBe('span two repos')
|
||||
}
|
||||
})
|
||||
|
||||
it('K2: a child repo with nothing staged is skipped, not committed empty', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
const portal = join(CONTAINER, 'fint-portal')
|
||||
// Why: staging only one repo is the common case — the user edited one project.
|
||||
await runtime.stageRuntimeGitPath(selector, 'fint_api/src/app.ts')
|
||||
await expect(runtime.commitRuntimeGit(selector, 'only api')).resolves.toEqual({ success: true })
|
||||
expect(git(CHILD_API, 'rev-list', '--count', 'HEAD').trim()).toBe('2')
|
||||
expect(git(portal, 'rev-list', '--count', 'HEAD').trim()).toBe('1')
|
||||
})
|
||||
|
||||
it('K3: committing with nothing staged anywhere reports it instead of silently succeeding', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
await expect(runtime.commitRuntimeGit(selector, 'nothing here')).resolves.toEqual({
|
||||
success: false,
|
||||
error: 'nothing to commit'
|
||||
})
|
||||
})
|
||||
|
||||
it('K4: an empty message is rejected for a folder workspace too', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
// Why: routing around the single-repo path must not drop its validation.
|
||||
await expect(runtime.commitRuntimeGit(selector, ' ')).rejects.toThrow(
|
||||
'Commit message is required'
|
||||
)
|
||||
})
|
||||
|
||||
it('L: abort targets only the child repo actually mid-merge', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
// Build a real conflicting merge in one child repo only.
|
||||
git(CHILD_API, 'checkout', '-q', '--', '.')
|
||||
git(CHILD_API, 'checkout', '-q', '-b', 'other')
|
||||
writeFileSync(join(CHILD_API, 'src', 'app.ts'), 'other branch\n')
|
||||
git(CHILD_API, 'commit', '-qam', 'other')
|
||||
git(CHILD_API, 'checkout', '-q', 'master')
|
||||
writeFileSync(join(CHILD_API, 'src', 'app.ts'), 'master branch\n')
|
||||
git(CHILD_API, 'commit', '-qam', 'master')
|
||||
expect(() => git(CHILD_API, 'merge', 'other')).toThrow()
|
||||
expect(await runtime.getRuntimeGitConflictOperation(`path:${CHILD_API}`)).toBe('merge')
|
||||
|
||||
await expect(runtime.abortRuntimeGitMerge(selector)).resolves.toEqual({ ok: true })
|
||||
expect(await runtime.getRuntimeGitConflictOperation(`path:${CHILD_API}`)).toBe('unknown')
|
||||
})
|
||||
|
||||
it('L2: aborting when no child repo is mid-operation settles instead of erroring', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore({ repos: allRepos() }) as never)
|
||||
const selector = `id:${folderWorkspaceKey(FOLDER_WS_ID)}`
|
||||
// Why: `git merge --abort` errors with no merge in progress; fanning that out
|
||||
// would make a no-op abort fail for a workspace but succeed for a single repo.
|
||||
await expect(runtime.abortRuntimeGitMerge(selector)).resolves.toEqual({ ok: true })
|
||||
await expect(runtime.abortRuntimeGitRebase(selector)).resolves.toEqual({ ok: true })
|
||||
})
|
||||
|
||||
it('E: the real child git worktree resolves fine when its repo is registered', async () => {
|
||||
buildFixture()
|
||||
const runtime = new OrcaRuntimeService(makeStore() as never)
|
||||
|
||||
@@ -183,6 +183,46 @@ export async function getRuntimeGitStatusForTarget(
|
||||
: getGitStatus(target.worktree.path, gitOptions)
|
||||
}
|
||||
|
||||
/**
|
||||
* Commit for an already-resolved target. Exported for the same reason as
|
||||
* `getRuntimeGitStatusForTarget` — a folder workspace commits each child repo
|
||||
* it resolved itself.
|
||||
*/
|
||||
export async function commitRuntimeGitForTarget(
|
||||
target: RuntimeGitTarget,
|
||||
message: string
|
||||
): Promise<{ success: boolean; error?: string }> {
|
||||
const provider = target.connectionId ? getSshGitProvider(target.connectionId) : null
|
||||
if (target.connectionId) {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.commit(target.worktree.path, message)
|
||||
}
|
||||
return commitChanges(target.worktree.path, message, localGitOptionsForTarget(target))
|
||||
}
|
||||
|
||||
/** Abort an in-progress merge or rebase for an already-resolved target. */
|
||||
export async function abortRuntimeGitForTarget(
|
||||
target: RuntimeGitTarget,
|
||||
operation: 'merge' | 'rebase'
|
||||
): Promise<void> {
|
||||
const provider = target.connectionId ? getSshGitProvider(target.connectionId) : null
|
||||
if (target.connectionId) {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
await (operation === 'merge'
|
||||
? provider.abortMerge(target.worktree.path)
|
||||
: provider.abortRebase(target.worktree.path))
|
||||
return
|
||||
}
|
||||
const gitOptions = localGitOptionsForTarget(target)
|
||||
await (operation === 'merge'
|
||||
? abortMerge(target.worktree.path, gitOptions)
|
||||
: abortRebase(target.worktree.path, gitOptions))
|
||||
}
|
||||
|
||||
/** One resolved target plus the subset of the requested paths it owns, already rebased. */
|
||||
export type RuntimeGitTargetPathGroup = {
|
||||
target: RuntimeGitTarget
|
||||
|
||||
@@ -90,7 +90,7 @@ import { getGitCloneFailureMessage } from '../../shared/git-clone-failure-messag
|
||||
import { GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS } from '../../shared/git-fetch-auto-maintenance'
|
||||
import { createHash, randomUUID } from 'node:crypto'
|
||||
import { homedir } from 'node:os'
|
||||
import { isAbsolute, join, resolve } from 'node:path'
|
||||
import { basename, isAbsolute, join, resolve } from 'node:path'
|
||||
import { mkdir, readFile, readdir, rm, stat } from 'node:fs/promises'
|
||||
import { resolveWorktreeCreateBase } from '../worktree-create-base'
|
||||
import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref'
|
||||
@@ -303,7 +303,8 @@ import type { GitStatusResult } from '../../shared/types'
|
||||
import {
|
||||
listFolderWorkspaceChildRepos,
|
||||
matchFolderWorkspaceChildRepo,
|
||||
mergeFolderWorkspaceGitStatus
|
||||
mergeFolderWorkspaceGitStatus,
|
||||
summarizeFolderWorkspaceFanOut
|
||||
} from './folder-workspace-child-repos'
|
||||
import { resolveTerminalStartupCwd } from '../../shared/terminal-startup-cwd'
|
||||
import { isWslUncPath, parseWslUncPath } from '../../shared/wsl-paths'
|
||||
@@ -454,6 +455,8 @@ import { RuntimeEmulatorCommands, setEmulatorBridge } from './orca-runtime-emula
|
||||
import type { EmulatorBridge } from '../emulator/emulator-bridge'
|
||||
import { RuntimeFileCommands } from './orca-runtime-files'
|
||||
import {
|
||||
abortRuntimeGitForTarget,
|
||||
commitRuntimeGitForTarget,
|
||||
getRuntimeGitStatusForTarget,
|
||||
RuntimeGitCommands,
|
||||
type RuntimeGitTargetPathGroup
|
||||
@@ -7103,10 +7106,14 @@ export class OrcaRuntimeService {
|
||||
this.gitCommands.getRuntimeGitHistory.bind(this.gitCommands)
|
||||
getRuntimeGitConflictOperation: RuntimeGitCommands['getRuntimeGitConflictOperation'] =
|
||||
this.gitCommands.getRuntimeGitConflictOperation.bind(this.gitCommands)
|
||||
abortRuntimeGitMerge: RuntimeGitCommands['abortRuntimeGitMerge'] =
|
||||
this.gitCommands.abortRuntimeGitMerge.bind(this.gitCommands)
|
||||
abortRuntimeGitRebase: RuntimeGitCommands['abortRuntimeGitRebase'] =
|
||||
this.gitCommands.abortRuntimeGitRebase.bind(this.gitCommands)
|
||||
// Why: like status, these carry no path to route on, so a folder workspace fans
|
||||
// out across the child repos that are actually in the relevant state.
|
||||
abortRuntimeGitMerge: RuntimeGitCommands['abortRuntimeGitMerge'] = async (selector) =>
|
||||
(await this.abortFolderWorkspaceGit(selector, 'merge')) ??
|
||||
(await this.gitCommands.abortRuntimeGitMerge(selector))
|
||||
abortRuntimeGitRebase: RuntimeGitCommands['abortRuntimeGitRebase'] = async (selector) =>
|
||||
(await this.abortFolderWorkspaceGit(selector, 'rebase')) ??
|
||||
(await this.gitCommands.abortRuntimeGitRebase(selector))
|
||||
checkoutRuntimeGitBranch: RuntimeGitCommands['checkoutRuntimeGitBranch'] =
|
||||
this.gitCommands.checkoutRuntimeGitBranch.bind(this.gitCommands)
|
||||
listRuntimeGitLocalBranches: RuntimeGitCommands['listRuntimeGitLocalBranches'] =
|
||||
@@ -7138,9 +7145,9 @@ export class OrcaRuntimeService {
|
||||
this.gitCommands.getRuntimeGitBranchDiff.bind(this.gitCommands)
|
||||
getRuntimeGitCommitDiff: RuntimeGitCommands['getRuntimeGitCommitDiff'] =
|
||||
this.gitCommands.getRuntimeGitCommitDiff.bind(this.gitCommands)
|
||||
commitRuntimeGit: RuntimeGitCommands['commitRuntimeGit'] = this.gitCommands.commitRuntimeGit.bind(
|
||||
this.gitCommands
|
||||
)
|
||||
commitRuntimeGit: RuntimeGitCommands['commitRuntimeGit'] = async (selector, message) =>
|
||||
(await this.commitFolderWorkspaceGit(selector, message)) ??
|
||||
(await this.gitCommands.commitRuntimeGit(selector, message))
|
||||
generateRuntimeCommitMessage: RuntimeGitCommands['generateRuntimeCommitMessage'] =
|
||||
this.gitCommands.generateRuntimeCommitMessage.bind(this.gitCommands)
|
||||
discoverRuntimeCommitMessageModels: RuntimeGitCommands['discoverRuntimeCommitMessageModels'] =
|
||||
@@ -24198,6 +24205,105 @@ export class OrcaRuntimeService {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Commit for a folder-workspace selector. `git.commit` carries no path, so there
|
||||
* is nothing to route on — instead commit every child repo that actually has
|
||||
* something staged, which is the set the merged status just showed the user as
|
||||
* staged. Child repos with an empty index are skipped rather than reporting
|
||||
* "nothing to commit". Returns null when the selector is not a folder workspace.
|
||||
*/
|
||||
private async commitFolderWorkspaceGit(
|
||||
selector: string,
|
||||
message: string
|
||||
): Promise<{ success: boolean; error?: string } | null> {
|
||||
const scope = await this.resolveFolderWorkspaceLaunchScope(selector)
|
||||
const folderWorkspace = scope?.folderWorkspace
|
||||
if (!folderWorkspace) {
|
||||
return null
|
||||
}
|
||||
// Why: the single-repo path validates this before resolving; routing around it
|
||||
// must not make an empty message reachable only for folder workspaces.
|
||||
if (message.trim().length === 0) {
|
||||
throw new Error('Commit message is required')
|
||||
}
|
||||
const childRepos = listFolderWorkspaceChildRepos(
|
||||
this.requireStore().getRepos(),
|
||||
folderWorkspace.folderPath
|
||||
)
|
||||
const staged = await Promise.all(
|
||||
childRepos.map(async (repo) => {
|
||||
const target = await this.resolveChildRepoGitTarget(repo)
|
||||
if (!target) {
|
||||
return null
|
||||
}
|
||||
const status = await getRuntimeGitStatusForTarget(target).catch(() => null)
|
||||
const hasStaged = status?.entries.some((entry) => entry.area === 'staged') === true
|
||||
return hasStaged ? { repo, target } : null
|
||||
})
|
||||
)
|
||||
const targets = staged.filter((entry) => entry !== null)
|
||||
if (targets.length === 0) {
|
||||
// Why: matches what a single-repo commit reports for an empty index, so the
|
||||
// renderer's existing failure copy stays accurate.
|
||||
return { success: false, error: 'nothing to commit' }
|
||||
}
|
||||
const results = await Promise.all(
|
||||
targets.map(async ({ repo, target }) => {
|
||||
const repoName = repo.displayName || basename(repo.path)
|
||||
try {
|
||||
const result = await commitRuntimeGitForTarget(target, message)
|
||||
return { repoName, ...(result.success ? {} : { error: result.error ?? 'Commit failed' }) }
|
||||
} catch (error) {
|
||||
return { repoName, error: error instanceof Error ? error.message : 'Commit failed' }
|
||||
}
|
||||
})
|
||||
)
|
||||
return summarizeFolderWorkspaceFanOut(results)
|
||||
}
|
||||
|
||||
/**
|
||||
* Abort an in-progress merge/rebase for a folder-workspace selector. Like commit
|
||||
* this carries no path, so it targets every child repo that is actually in that
|
||||
* conflict state — aborting a repo that is not mid-operation would error.
|
||||
* Returns null when the selector is not a folder workspace.
|
||||
*/
|
||||
private async abortFolderWorkspaceGit(
|
||||
selector: string,
|
||||
operation: 'merge' | 'rebase'
|
||||
): Promise<{ ok: true } | null> {
|
||||
const scope = await this.resolveFolderWorkspaceLaunchScope(selector)
|
||||
const folderWorkspace = scope?.folderWorkspace
|
||||
if (!folderWorkspace) {
|
||||
return null
|
||||
}
|
||||
const childRepos = listFolderWorkspaceChildRepos(
|
||||
this.requireStore().getRepos(),
|
||||
folderWorkspace.folderPath
|
||||
)
|
||||
const inOperation = await Promise.all(
|
||||
childRepos.map(async (repo) => {
|
||||
const target = await this.resolveChildRepoGitTarget(repo)
|
||||
if (!target) {
|
||||
return null
|
||||
}
|
||||
const detected = await this.gitCommands
|
||||
.getRuntimeGitConflictOperation(`id:${target.worktree.id}`)
|
||||
.catch(() => 'unknown' as const)
|
||||
// Why: belt-and-braces. Git already refuses `merge --abort` on a mid-rebase
|
||||
// repo (and vice versa), so this narrows what we run rather than what we
|
||||
// prevent — it keeps N-1 pointless failing git invocations off the wire.
|
||||
return detected === operation ? target : null
|
||||
})
|
||||
)
|
||||
const targets = inOperation.filter((target) => target !== null)
|
||||
// Why: aborting nothing is not an error — the conflict may have been resolved
|
||||
// in a terminal already, and the renderer only needs the state to settle.
|
||||
await Promise.all(
|
||||
targets.map((target) => abortRuntimeGitForTarget(target, operation).catch(() => undefined))
|
||||
)
|
||||
return { ok: true }
|
||||
}
|
||||
|
||||
private folderWorkspaceToResolvedWorktree(folderWorkspace: FolderWorkspace): ResolvedWorktree {
|
||||
const worktree = folderWorkspaceToWorktree(folderWorkspace)
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user