fix(cross-repo): per-repo worktree-branch + PR resolutie (IDEA-062) (#49)
Cross-repo sprints (sprint-product = repo X, maar een taak heeft
task.repo_url naar repo Y) faalden op twee plekken omdat sprint-brede
beslissingen werden toegepast op per-repo git-state.
1. createWorktreeForJob (src/git/worktree.ts)
reuseBranch wordt sprint-breed bepaald in wait-for-job.ts. De eerste
job die repo Y target krijgt reuseBranch=true terwijl de branch daar
nooit is aangemaakt -> `git worktree add <path> <branch>` faalt met
"invalid reference" -> job vast, worker UNHEALTHY. Idem na een
container-recreate (clone is dan vers).
Fix: 3-weg fallback in het reuseBranch-pad:
- lokale branch bestaat -> hergebruik
- alleen op origin -> recreate lokaal vanaf origin/<branch>
- nergens -> fresh vanaf baseRef
Lost ook het container-recreate-verlies op.
2. maybeCreateAutoPr (src/tools/update-job-status.ts)
De sprint/story sibling-lookup voor pr_url-hergebruik filterde niet
op repo. Een repo-Y-job erfde de pr_url van een repo-X-sibling ->
job.pr_url wees naar de verkeerde repo en er werd nooit een PR voor
de repo-Y-branch aangemaakt (branch wel gepusht, maar PR-loos).
Fix: siblings groeperen per repo-bucket ((task.repo_url ?? null));
alleen een sibling uit dezelfde bucket levert een herbruikbare
pr_url. Geldt voor SPRINT- en STORY-mode. createPullRequest zelf was
al repo-correct (gh pr create draait in de worktree).
Tests: 3 nieuwe in worktree.test.ts (reuse-local / recreate-from-origin
/ fresh-fallback), 2 nieuwe in update-job-status-auto-pr.test.ts
(cross-repo story + sprint). update-job-status-mock omgezet naar
findMany. Alle 373 tests groen, build groen.
package-lock.json: version 0.7.0 -> 0.8.0 (was niet mee-gesynced in de
v0.8.0-bump commit 55fa133).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
55fa133150
commit
84c194d4e5
5 changed files with 171 additions and 15 deletions
|
|
@ -113,6 +113,71 @@ describe('createWorktreeForJob', () => {
|
|||
}),
|
||||
).rejects.toThrow('Worktree path already exists')
|
||||
})
|
||||
|
||||
it('reuseBranch: reuses an existing local branch', async () => {
|
||||
const { repoDir, originDir } = await setupRepo()
|
||||
tmpDirs.push(repoDir, originDir)
|
||||
await makeWorktreeParent()
|
||||
|
||||
// Sibling already created the branch locally.
|
||||
await git(['branch', 'feat/sprint-abc', 'origin/main'], repoDir)
|
||||
|
||||
const result = await createWorktreeForJob({
|
||||
repoRoot: repoDir,
|
||||
jobId: 'job-reuse-local',
|
||||
branchName: 'feat/sprint-abc',
|
||||
baseRef: 'origin/main',
|
||||
reuseBranch: true,
|
||||
})
|
||||
|
||||
const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath)
|
||||
expect(stdout.trim()).toBe('feat/sprint-abc')
|
||||
expect(result.branchName).toBe('feat/sprint-abc')
|
||||
})
|
||||
|
||||
it('reuseBranch: recreates a local branch from origin when only the remote has it', async () => {
|
||||
const { repoDir, originDir } = await setupRepo()
|
||||
tmpDirs.push(repoDir, originDir)
|
||||
await makeWorktreeParent()
|
||||
|
||||
// Branch exists on origin (a sibling pushed it, or the container was
|
||||
// recreated and the local clone is fresh) but not as a local branch.
|
||||
await git(['branch', 'feat/sprint-xyz', 'origin/main'], repoDir)
|
||||
await git(['push', 'origin', 'feat/sprint-xyz'], repoDir)
|
||||
await git(['branch', '-D', 'feat/sprint-xyz'], repoDir)
|
||||
|
||||
const result = await createWorktreeForJob({
|
||||
repoRoot: repoDir,
|
||||
jobId: 'job-reuse-origin',
|
||||
branchName: 'feat/sprint-xyz',
|
||||
baseRef: 'origin/main',
|
||||
reuseBranch: true,
|
||||
})
|
||||
|
||||
const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath)
|
||||
expect(stdout.trim()).toBe('feat/sprint-xyz')
|
||||
})
|
||||
|
||||
it('reuseBranch: falls back to a fresh branch when it exists nowhere (cross-repo sprint)', async () => {
|
||||
const { repoDir, originDir } = await setupRepo()
|
||||
tmpDirs.push(repoDir, originDir)
|
||||
await makeWorktreeParent()
|
||||
|
||||
// reuseBranch is decided sprint-wide; for the first job targeting THIS
|
||||
// repo the branch exists neither locally nor on origin. Must not throw
|
||||
// "invalid reference" — should create it fresh from baseRef.
|
||||
const result = await createWorktreeForJob({
|
||||
repoRoot: repoDir,
|
||||
jobId: 'job-reuse-fresh',
|
||||
branchName: 'feat/sprint-newrepo',
|
||||
baseRef: 'origin/main',
|
||||
reuseBranch: true,
|
||||
})
|
||||
|
||||
const { stdout } = await git(['rev-parse', '--abbrev-ref', 'HEAD'], result.worktreePath)
|
||||
expect(stdout.trim()).toBe('feat/sprint-newrepo')
|
||||
expect(result.branchName).toBe('feat/sprint-newrepo')
|
||||
})
|
||||
})
|
||||
|
||||
describe('removeWorktreeForJob', () => {
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ vi.mock('../src/prisma.js', () => ({
|
|||
prisma: {
|
||||
product: { findUnique: vi.fn() },
|
||||
task: { findUnique: vi.fn() },
|
||||
claudeJob: { findFirst: vi.fn(), findUnique: vi.fn() },
|
||||
claudeJob: { findFirst: vi.fn(), findMany: vi.fn(), findUnique: vi.fn() },
|
||||
},
|
||||
}))
|
||||
|
||||
|
|
@ -22,6 +22,7 @@ const mockPrisma = prisma as unknown as {
|
|||
task: { findUnique: ReturnType<typeof vi.fn> }
|
||||
claudeJob: {
|
||||
findFirst: ReturnType<typeof vi.fn>
|
||||
findMany: ReturnType<typeof vi.fn>
|
||||
findUnique: ReturnType<typeof vi.fn>
|
||||
}
|
||||
}
|
||||
|
|
@ -41,9 +42,10 @@ beforeEach(() => {
|
|||
mockPrisma.product.findUnique.mockResolvedValue({ auto_pr: true })
|
||||
mockPrisma.task.findUnique.mockResolvedValue({
|
||||
title: 'Add feature',
|
||||
repo_url: null,
|
||||
story: { id: 'story-1', code: 'SCRUM-42', title: 'Story title' },
|
||||
})
|
||||
mockPrisma.claudeJob.findFirst.mockResolvedValue(null) // no sibling PR by default
|
||||
mockPrisma.claudeJob.findMany.mockResolvedValue([]) // no sibling PRs by default
|
||||
// Default: legacy job zonder sprint_run (STORY-mode pad).
|
||||
mockPrisma.claudeJob.findUnique.mockResolvedValue({ sprint_run_id: null, sprint_run: null })
|
||||
mockCreatePr.mockResolvedValue({ url: 'https://github.com/org/repo/pull/99' })
|
||||
|
|
@ -62,12 +64,27 @@ describe('maybeCreateAutoPr', () => {
|
|||
})
|
||||
|
||||
it('reuses sibling pr_url when another job in same story already opened a PR', async () => {
|
||||
mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/77' })
|
||||
mockPrisma.claudeJob.findMany.mockResolvedValue([
|
||||
{ pr_url: 'https://github.com/org/repo/pull/77', task: { repo_url: null } },
|
||||
])
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
expect(url).toBe('https://github.com/org/repo/pull/77')
|
||||
expect(mockCreatePr).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does NOT reuse a sibling PR from a different repo (cross-repo story)', async () => {
|
||||
// Sibling targeted another repo via task.repo_url — its PR must not leak in.
|
||||
mockPrisma.claudeJob.findMany.mockResolvedValue([
|
||||
{
|
||||
pr_url: 'https://github.com/org/other-repo/pull/12',
|
||||
task: { repo_url: 'https://github.com/org/other-repo' },
|
||||
},
|
||||
])
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
expect(url).toBe('https://github.com/org/repo/pull/99') // fresh PR, not the sibling's
|
||||
expect(mockCreatePr).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('returns null when auto_pr=false', async () => {
|
||||
mockPrisma.product.findUnique.mockResolvedValue({ auto_pr: false })
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
|
|
@ -78,6 +95,7 @@ describe('maybeCreateAutoPr', () => {
|
|||
it('uses story title without code prefix when story has no code', async () => {
|
||||
mockPrisma.task.findUnique.mockResolvedValue({
|
||||
title: 'Add feature',
|
||||
repo_url: null,
|
||||
story: { id: 'story-1', code: null, title: 'Story title' },
|
||||
})
|
||||
await maybeCreateAutoPr(BASE_OPTS)
|
||||
|
|
@ -113,7 +131,9 @@ describe('maybeCreateAutoPr', () => {
|
|||
sprint_run_id: 'run-1',
|
||||
sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } },
|
||||
})
|
||||
mockPrisma.claudeJob.findFirst.mockResolvedValue({ pr_url: 'https://github.com/org/repo/pull/55' })
|
||||
mockPrisma.claudeJob.findMany.mockResolvedValue([
|
||||
{ pr_url: 'https://github.com/org/repo/pull/55', task: { repo_url: null } },
|
||||
])
|
||||
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
|
||||
|
|
@ -121,6 +141,29 @@ describe('maybeCreateAutoPr', () => {
|
|||
expect(mockCreatePr).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('SPRINT-mode: cross-repo — sibling-PR van ander repo wordt niet hergebruikt', async () => {
|
||||
mockPrisma.claudeJob.findUnique.mockResolvedValue({
|
||||
sprint_run_id: 'run-1',
|
||||
sprint_run: { id: 'run-1', pr_strategy: 'SPRINT', sprint: { sprint_goal: 'Goal' } },
|
||||
})
|
||||
// Deze job target een ander repo via task.repo_url.
|
||||
mockPrisma.task.findUnique.mockResolvedValue({
|
||||
title: 'MCP-taak',
|
||||
repo_url: 'https://github.com/org/scrum4me-mcp',
|
||||
story: { id: 'story-1', code: 'SCRUM-9', title: 'Story title' },
|
||||
})
|
||||
// Sibling met pr_url hoort bij het product-repo (repo_url null) → andere bucket.
|
||||
mockPrisma.claudeJob.findMany.mockResolvedValue([
|
||||
{ pr_url: 'https://github.com/org/repo/pull/201', task: { repo_url: null } },
|
||||
])
|
||||
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
|
||||
// Geen hergebruik van de product-repo PR → eigen draft-PR voor het mcp-repo.
|
||||
expect(url).toBe('https://github.com/org/repo/pull/99')
|
||||
expect(mockCreatePr).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('returns null and does not throw when gh fails', async () => {
|
||||
mockCreatePr.mockResolvedValue({ error: 'gh CLI not found' })
|
||||
const url = await maybeCreateAutoPr(BASE_OPTS)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue