diff --git a/.storybook/mocks/app/actions/reparent-feature.ts b/.storybook/mocks/app/actions/reparent-feature.ts new file mode 100644 index 000000000..8eb1b38a8 --- /dev/null +++ b/.storybook/mocks/app/actions/reparent-feature.ts @@ -0,0 +1,6 @@ +export async function reparentFeature( + _featureId: string, + _parentId: string | null +): Promise<{ success: boolean; error?: string }> { + return { success: true }; +} diff --git a/packages/core/src/application/ports/output/services/git-pr-service.interface.ts b/packages/core/src/application/ports/output/services/git-pr-service.interface.ts index 61c034a6a..6fda186ba 100644 --- a/packages/core/src/application/ports/output/services/git-pr-service.interface.ts +++ b/packages/core/src/application/ports/output/services/git-pr-service.interface.ts @@ -442,6 +442,22 @@ export interface IGitPrService { */ rebaseOnMain(cwd: string, featureBranch: string, baseBranch: string): Promise; + /** + * Rebase the feature branch onto `origin/`. + * Fetches the target branch from the remote first to ensure the + * remote-tracking ref is up-to-date, then rebases the feature branch + * onto it. Similar to {@link rebaseOnMain} but targets an arbitrary + * branch instead of the repository's default branch. + * + * @param cwd - Working directory path + * @param featureBranch - The feature branch to rebase + * @param targetBranch - The target branch name (rebase target will be origin/) + * @throws GitPrError with GIT_ERROR code if the worktree is dirty + * @throws GitPrError with REBASE_CONFLICT code if conflicts are detected + * @throws GitPrError with BRANCH_NOT_FOUND code if a branch does not exist + */ + rebaseOnBranch(cwd: string, featureBranch: string, targetBranch: string): Promise; + /** * Get the list of files with unresolved conflicts (unmerged paths). * Uses `git diff --name-only --diff-filter=U` to identify conflicted files. diff --git a/packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts b/packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts index e8f4f1104..9ad005ebe 100644 --- a/packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts +++ b/packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts @@ -2,8 +2,8 @@ * CheckAndUnblockFeaturesUseCase * * Evaluates whether blocked direct children of a parent feature are now - * eligible to start, and if so transitions them to Started and spawns their - * agents. + * eligible to start, and if so transitions them to Started, rebases their + * branches onto the parent branch, and spawns their agents. * * Business Rules: * - Only direct children of parentFeatureId are evaluated (no recursive traversal). @@ -11,22 +11,50 @@ * - Gate: parent lifecycle must be in POST_IMPLEMENTATION (Implementation, Review, Maintain). * - Idempotent: already-Started children are not touched; calling execute() twice is safe. * - spawn() is skipped for children missing agentRunId or specPath (defensive guard). + * - Auto-rebase: each blocked child's branch is rebased onto the parent branch + * before spawning the agent. Rebase failures are isolated per-child and recorded + * in the activity timeline. Agent spawns regardless of rebase outcome. + * - NFR-3: rebase is skipped if the child has an active (running) agent run. * * Called from: UpdateFeatureLifecycleUseCase after every lifecycle transition. */ import { injectable, inject } from 'tsyringe'; -import { SdlcLifecycle } from '../../../domain/generated/output.js'; +import { randomUUID } from 'node:crypto'; +import { SdlcLifecycle, AgentRunStatus, AgentType } from '../../../domain/generated/output.js'; +import type { Feature } from '../../../domain/generated/output.js'; import type { IFeatureRepository } from '../../ports/output/repositories/feature-repository.interface.js'; import type { IFeatureAgentProcessService } from '../../ports/output/agents/feature-agent-process.interface.js'; +import type { IGitPrService } from '../../ports/output/services/git-pr-service.interface.js'; +import { + GitPrError, + GitPrErrorCode, +} from '../../ports/output/services/git-pr-service.interface.js'; +import type { IWorktreeService } from '../../ports/output/services/worktree-service.interface.js'; +import type { ConflictResolutionService } from '../../../infrastructure/services/agents/conflict-resolution/conflict-resolution.service.js'; +import type { IAgentRunRepository } from '../../ports/output/agents/agent-run-repository.interface.js'; +import type { IPhaseTimingRepository } from '../../ports/output/agents/phase-timing-repository.interface.js'; import { POST_IMPLEMENTATION } from '../../../domain/lifecycle-gates.js'; +/** Maximum time (ms) to wait for a single child rebase before aborting. */ +const REBASE_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes + @injectable() export class CheckAndUnblockFeaturesUseCase { constructor( @inject('IFeatureRepository') private readonly featureRepo: IFeatureRepository, @inject('IFeatureAgentProcessService') - private readonly agentProcess: IFeatureAgentProcessService + private readonly agentProcess: IFeatureAgentProcessService, + @inject('IGitPrService') + private readonly gitPrService: IGitPrService, + @inject('IWorktreeService') + private readonly worktreeService: IWorktreeService, + @inject('ConflictResolutionService') + private readonly conflictResolutionService: ConflictResolutionService, + @inject('IAgentRunRepository') + private readonly agentRunRepo: IAgentRunRepository, + @inject('IPhaseTimingRepository') + private readonly phaseTimingRepo: IPhaseTimingRepository ) {} /** @@ -55,6 +83,9 @@ export class CheckAndUnblockFeaturesUseCase { child.updatedAt = new Date(); await this.featureRepo.update(child); + // Rebase child branch onto parent branch (isolated per-child) + await this.rebaseChildOntoParent(child, parent); + // Spawn agent using fields set at feature creation time if (child.agentRunId && child.specPath) { this.agentProcess.spawn( @@ -78,4 +109,155 @@ export class CheckAndUnblockFeaturesUseCase { } } } + + /** + * Rebase a child feature branch onto the parent feature branch. + * + * Creates an agent run + phase timing for activity timeline tracking. + * Stashes uncommitted changes before rebase and restores in finally block. + * Delegates to ConflictResolutionService on conflicts. + * Failures are recorded but do not prevent agent spawn. + * + * Skips rebase if the child has an active (running) agent run (NFR-3). + */ + private async rebaseChildOntoParent(child: Feature, parent: Feature): Promise { + // NFR-3: skip rebase if child has an active agent run + if (child.agentRunId) { + const existingRun = await this.agentRunRepo.findById(child.agentRunId); + if (existingRun && existingRun.status === AgentRunStatus.running) { + return; + } + } + + // Create standalone agent run + phase timing for activity timeline + const now = new Date().toISOString(); + const agentRunId = randomUUID(); + const phaseTimingId = randomUUID(); + + await this.agentRunRepo.create({ + id: agentRunId, + agentType: AgentType.ClaudeCode, + agentName: 'rebase', + status: AgentRunStatus.running, + prompt: `Rebase ${child.branch} onto parent ${parent.branch}`, + threadId: agentRunId, + startedAt: now, + featureId: child.id, + repositoryPath: child.repositoryPath, + createdAt: now, + updatedAt: now, + }); + + await this.phaseTimingRepo.save({ + id: phaseTimingId, + agentRunId, + phase: 'rebase-on-parent', + startedAt: now, + createdAt: now, + updatedAt: now, + }); + + const startMs = Date.now(); + + try { + // Resolve CWD — worktree path if it exists, else repo root + const cwd = await this.resolveCwd(child.repositoryPath, child.branch); + + // Stash uncommitted changes (smart rebase) + const didStash = await this.gitPrService.stash( + cwd, + 'shep-rebase: auto-stash before parent rebase' + ); + + try { + // Rebase child branch onto parent branch (with timeout) + await Promise.race([ + this.performRebase(cwd, child.branch, parent.branch), + this.createTimeout(REBASE_TIMEOUT_MS, child.branch), + ]); + } finally { + // Restore stashed changes (regardless of rebase outcome) + if (didStash) { + await this.gitPrService.stashPop(cwd); + } + } + + // Rebase succeeded + await this.completeTiming(agentRunId, phaseTimingId, startMs, 'success'); + } catch (error) { + // Record failure in activity timeline but do not throw — + // agent spawn proceeds regardless of rebase outcome + const message = error instanceof Error ? error.message : String(error); + await this.completeTiming(agentRunId, phaseTimingId, startMs, 'error', message); + } + } + + /** + * Perform the rebase with conflict resolution. + */ + private async performRebase( + cwd: string, + childBranch: string, + parentBranch: string + ): Promise { + try { + await this.gitPrService.rebaseOnBranch(cwd, childBranch, parentBranch); + } catch (error) { + if (error instanceof GitPrError && error.code === GitPrErrorCode.REBASE_CONFLICT) { + // Delegate to agent-powered conflict resolution + await this.conflictResolutionService.resolve(cwd, childBranch, parentBranch); + } else { + throw error; + } + } + } + + /** + * Create a timeout promise that rejects after the specified duration. + */ + private createTimeout(ms: number, childBranch: string): Promise { + return new Promise((_, reject) => { + setTimeout(() => reject(new Error(`Rebase timeout: ${childBranch} exceeded ${ms}ms`)), ms); + }); + } + + /** + * Complete the phase timing and update agent run status. + */ + private async completeTiming( + agentRunId: string, + phaseTimingId: string, + startMs: number, + exitCode: 'success' | 'error', + errorMessage?: string + ): Promise { + const completedAt = new Date().toISOString(); + const durationMs = Date.now() - startMs; + + await this.phaseTimingRepo.update(phaseTimingId, { + completedAt, + durationMs: BigInt(durationMs), + exitCode, + ...(errorMessage && { errorMessage }), + }); + + await this.agentRunRepo.updateStatus( + agentRunId, + exitCode === 'success' ? AgentRunStatus.completed : AgentRunStatus.failed, + { completedAt, ...(errorMessage && { error: errorMessage }) } + ); + } + + /** + * Resolve the correct working directory for the child feature. + * Uses the worktree path if a worktree exists for this branch, + * otherwise falls back to the repository root. + */ + private async resolveCwd(repositoryPath: string, branch: string): Promise { + const hasWorktree = await this.worktreeService.exists(repositoryPath, branch); + if (hasWorktree) { + return this.worktreeService.getWorktreePath(repositoryPath, branch); + } + return repositoryPath; + } } diff --git a/packages/core/src/application/use-cases/features/reparent-feature.use-case.ts b/packages/core/src/application/use-cases/features/reparent-feature.use-case.ts new file mode 100644 index 000000000..e9af82be2 --- /dev/null +++ b/packages/core/src/application/use-cases/features/reparent-feature.use-case.ts @@ -0,0 +1,136 @@ +/** + * ReparentFeatureUseCase + * + * Updates a feature's parent dependency (or clears it). Performs validation: + * - Same-repository constraint (child and parent must share repositoryPath) + * - Cycle detection via upward ancestor walk + * - Lifecycle guards (cannot reparent completed/archived/deleting features) + * - Lifecycle state adjustment based on new parent's lifecycle + * + * After reparenting, if the new parent is post-implementation, calls + * CheckAndUnblockFeaturesUseCase to trigger the unblock+rebase flow + * for any Blocked children of the reparented feature. + */ + +import { injectable, inject } from 'tsyringe'; +import { SdlcLifecycle } from '../../../domain/generated/output.js'; +import type { IFeatureRepository } from '../../ports/output/repositories/feature-repository.interface.js'; +import { POST_IMPLEMENTATION } from '../../../domain/lifecycle-gates.js'; +import { CheckAndUnblockFeaturesUseCase } from './check-and-unblock-features.use-case.js'; + +/** Lifecycle states that cannot be reparented. */ +const NON_REPARENTABLE_STATES = new Set([ + SdlcLifecycle.Maintain, + SdlcLifecycle.Archived, + SdlcLifecycle.Deleting, +]); + +export interface ReparentFeatureInput { + featureId: string; + parentId: string | null; +} + +@injectable() +export class ReparentFeatureUseCase { + constructor( + @inject('IFeatureRepository') + private readonly featureRepo: IFeatureRepository, + @inject(CheckAndUnblockFeaturesUseCase) + private readonly checkAndUnblock: CheckAndUnblockFeaturesUseCase + ) {} + + async execute(input: ReparentFeatureInput): Promise { + const { featureId, parentId } = input; + + // Self-reparent guard + if (parentId !== null && featureId === parentId) { + throw new Error('A feature cannot be set as parent of itself.'); + } + + // Load child feature + const child = await this.featureRepo.findById(featureId); + if (!child) { + throw new Error(`Feature not found: ${featureId}`); + } + + // Lifecycle guard — reject completed/terminal features + if (NON_REPARENTABLE_STATES.has(child.lifecycle)) { + throw new Error( + `Cannot reparent feature "${child.name}": lifecycle is ${child.lifecycle}. ` + + 'Only active features can be reparented.' + ); + } + + // Unparent case + if (parentId === null) { + const newLifecycle = + child.lifecycle === SdlcLifecycle.Blocked ? SdlcLifecycle.Started : child.lifecycle; + await this.featureRepo.update({ + ...child, + parentId: undefined, + lifecycle: newLifecycle, + updatedAt: new Date(), + }); + return; + } + + // Load parent feature + const parent = await this.featureRepo.findById(parentId); + if (!parent) { + throw new Error(`Parent feature not found: ${parentId}`); + } + + // Same-repository constraint + if (child.repositoryPath !== parent.repositoryPath) { + throw new Error('Features must be in the same repository to form a dependency.'); + } + + // Cycle detection — walk from proposed parent upward + await this.detectCycle(featureId, parentId); + + // Determine lifecycle adjustment based on new parent's lifecycle + let newLifecycle = child.lifecycle; + if (parent.lifecycle === SdlcLifecycle.Blocked || !POST_IMPLEMENTATION.has(parent.lifecycle)) { + // Parent is pre-implementation or Blocked — child should be Blocked + if (child.lifecycle !== SdlcLifecycle.Blocked && child.lifecycle !== SdlcLifecycle.Pending) { + newLifecycle = SdlcLifecycle.Blocked; + } + } + + // Persist the reparent + await this.featureRepo.update({ + ...child, + parentId, + lifecycle: newLifecycle, + updatedAt: new Date(), + }); + + // If new parent is post-implementation, trigger unblock flow for the + // reparented feature's own children (the feature itself may now be a parent + // of Blocked children that should be unblocked) + if (POST_IMPLEMENTATION.has(parent.lifecycle)) { + await this.checkAndUnblock.execute(featureId); + } + } + + /** + * Walk the ancestor chain from the proposed parent upward. + * If the child feature ID is found in the chain, a cycle exists. + */ + private async detectCycle(childId: string, parentId: string): Promise { + const visited = new Set([childId]); + let cursor: string | undefined = parentId; + + while (cursor) { + if (visited.has(cursor)) { + throw new Error( + `Cycle detected in feature dependency chain. ` + + `Setting ${parentId} as parent of ${childId} would create a circular dependency.` + ); + } + visited.add(cursor); + const ancestor = await this.featureRepo.findById(cursor); + cursor = ancestor?.parentId ?? undefined; + } + } +} diff --git a/packages/core/src/infrastructure/di/container.ts b/packages/core/src/infrastructure/di/container.ts index a195ac0d7..ad5ec756c 100644 --- a/packages/core/src/infrastructure/di/container.ts +++ b/packages/core/src/infrastructure/di/container.ts @@ -135,6 +135,7 @@ import { RebaseFeatureOnMainUseCase } from '../../application/use-cases/features import { GetBranchSyncStatusUseCase } from '../../application/use-cases/features/get-branch-sync-status.use-case.js'; import { ConflictResolutionService } from '../services/agents/conflict-resolution/conflict-resolution.service.js'; import { AutoResolveMergedBranchesUseCase } from '../../application/use-cases/features/auto-resolve-merged-branches.use-case.js'; +import { ReparentFeatureUseCase } from '../../application/use-cases/features/reparent-feature.use-case.js'; // Interactive session use cases import { StartInteractiveSessionUseCase } from '../../application/use-cases/interactive/start-interactive-session.use-case.js'; @@ -429,6 +430,7 @@ export async function initializeContainer(): Promise { container.registerSingleton(RebaseFeatureOnMainUseCase); container.registerSingleton(GetBranchSyncStatusUseCase); container.registerSingleton(AutoResolveMergedBranchesUseCase); + container.registerSingleton(ReparentFeatureUseCase); // Session repositories (per-AgentType string tokens) container.register(`IAgentSessionRepository:${AgentType.ClaudeCode}`, { @@ -570,6 +572,9 @@ export async function initializeContainer(): Promise { container.register('AutoResolveMergedBranchesUseCase', { useFactory: (c) => c.resolve(AutoResolveMergedBranchesUseCase), }); + container.register('ReparentFeatureUseCase', { + useFactory: (c) => c.resolve(ReparentFeatureUseCase), + }); // Register interactive session infrastructure container.register('IInteractiveSessionRepository', { diff --git a/packages/core/src/infrastructure/services/git/git-pr.service.ts b/packages/core/src/infrastructure/services/git/git-pr.service.ts index 2e9f32915..faa25b31e 100644 --- a/packages/core/src/infrastructure/services/git/git-pr.service.ts +++ b/packages/core/src/infrastructure/services/git/git-pr.service.ts @@ -974,6 +974,88 @@ export class GitPrService implements IGitPrService { } } + async rebaseOnBranch(cwd: string, featureBranch: string, targetBranch: string): Promise { + // Check for dirty worktree before starting + const dirty = await this.hasUncommittedChanges(cwd); + if (dirty) { + throw new GitPrError( + `Cannot rebase: working directory has uncommitted changes. ` + + `Please commit or stash your changes before rebasing.`, + GitPrErrorCode.GIT_ERROR + ); + } + + // Fetch the target branch from remote — it may not be locally available + try { + await this.execFile('git', ['fetch', 'origin', targetBranch], { cwd }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const cause = error instanceof Error ? error : undefined; + throw new GitPrError( + `Failed to fetch target branch '${targetBranch}': ${message}`, + GitPrErrorCode.GIT_ERROR, + cause + ); + } + + // Checkout the feature branch + try { + await this.execFile('git', ['checkout', featureBranch], { cwd }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const cause = error instanceof Error ? error : undefined; + if ( + message.includes('did not match') || + message.includes('not a commit') || + message.includes('pathspec') + ) { + throw new GitPrError( + `Branch '${featureBranch}' not found.`, + GitPrErrorCode.BRANCH_NOT_FOUND, + cause + ); + } + throw new GitPrError( + `Failed to checkout '${featureBranch}': ${message}`, + GitPrErrorCode.GIT_ERROR, + cause + ); + } + + // Rebase onto origin/ + const rebaseTarget = `origin/${targetBranch}`; + try { + await this.execFile('git', ['rebase', rebaseTarget], { cwd }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + const cause = error instanceof Error ? error : undefined; + + // Detect rebase conflict from git stderr/exit code + if (message.includes('CONFLICT') || message.includes('could not apply')) { + let conflictedFiles: string[] = []; + try { + conflictedFiles = await this.getConflictedFiles(cwd); + } catch { + // Failed to get conflicted files — still report the conflict + } + + const fileList = + conflictedFiles.length > 0 ? ` Conflicted files: ${conflictedFiles.join(', ')}` : ''; + throw new GitPrError( + `Rebase of '${featureBranch}' onto '${targetBranch}' encountered conflicts.${fileList}`, + GitPrErrorCode.REBASE_CONFLICT, + cause + ); + } + + throw new GitPrError( + `Rebase of '${featureBranch}' onto '${targetBranch}' failed: ${message}`, + GitPrErrorCode.GIT_ERROR, + cause + ); + } + } + async getConflictedFiles(cwd: string): Promise { try { const { stdout } = await this.execFile('git', ['diff', '--name-only', '--diff-filter=U'], { diff --git a/specs/084-feature-dependency-rebase/evidence/app-canvas-edge-hover-delete.png b/specs/084-feature-dependency-rebase/evidence/app-canvas-edge-hover-delete.png new file mode 100644 index 000000000..e6a6a4920 Binary files /dev/null and b/specs/084-feature-dependency-rebase/evidence/app-canvas-edge-hover-delete.png differ diff --git a/specs/084-feature-dependency-rebase/evidence/app-canvas-full-view.png b/specs/084-feature-dependency-rebase/evidence/app-canvas-full-view.png new file mode 100644 index 000000000..471f65a04 Binary files /dev/null and b/specs/084-feature-dependency-rebase/evidence/app-canvas-full-view.png differ diff --git a/specs/084-feature-dependency-rebase/evidence/app-feature-node-with-handles.png b/specs/084-feature-dependency-rebase/evidence/app-feature-node-with-handles.png new file mode 100644 index 000000000..9d14201c5 Binary files /dev/null and b/specs/084-feature-dependency-rebase/evidence/app-feature-node-with-handles.png differ diff --git a/specs/084-feature-dependency-rebase/evidence/app-main-page.png b/specs/084-feature-dependency-rebase/evidence/app-main-page.png new file mode 100644 index 000000000..471f65a04 Binary files /dev/null and b/specs/084-feature-dependency-rebase/evidence/app-main-page.png differ diff --git a/specs/084-feature-dependency-rebase/evidence/integration-test-results.txt b/specs/084-feature-dependency-rebase/evidence/integration-test-results.txt new file mode 100644 index 000000000..9d39e7698 --- /dev/null +++ b/specs/084-feature-dependency-rebase/evidence/integration-test-results.txt @@ -0,0 +1,103 @@ +=== Integration Test Results === +Test run date: 2026-04-07T13:58:23Z +Branch: feat/feature-dependency-rebase +Command: pnpm test:int + +50 test files, 581 tests - ALL PASSED +Duration: 20.12s + +--- Key integration test suites for this feature --- +feature-parent.repository.test.ts: 14 tests PASSED (398ms) +sqlite-feature-repository.find-by-branch.test.ts: 6 tests PASSED (254ms) +git-pr.service.rebase-sync.test.ts: 16 tests PASSED (19843ms) + +--- Full integration suite output (vitest run, ANSI stripped) --- + + ✓ node tests/integration/infrastructure/persistence/sqlite/migrations.test.ts (66 tests) 1191ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/pr-merge.test.ts (1 test) 1921ms + ✓ push=true, openPr=true, allowMerge=true → BUG: verifyMerge skipped after gh pr merge 1918ms + ✓ node tests/integration/infrastructure/services/git/worktree-git-init.test.ts (8 tests) 2418ms + ✓ should not re-initialize an existing git repository 1390ms + ✓ node tests/integration/infrastructure/repositories/sqlite-settings.repository.test.ts (47 tests) 942ms + ✓ node tests/integration/infrastructure/repositories/sqlite-feature.repository.test.ts (35 tests) 702ms + ✓ node tests/integration/infrastructure/repositories/sqlite-repository.repository.test.ts (20 tests) 319ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/skip-merge.test.ts (1 test) 2398ms + ✓ undefined-gates-silent-skip: should skip merge silently when approvalGates is undefined 2396ms + ✓ node tests/integration/infrastructure/repositories/agent-run.repository.test.ts (31 tests) 510ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/push-merge.test.ts (1 test) 3911ms + ✓ push-no-pr-merge: feature branch should be merged into main after push+merge 3908ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/smoke.test.ts (7 tests) 4209ms + ✓ should create a bare repo and clone with main + feature branch 2075ms + ✓ createLocalOnlyHarness: should create a local repo with no remote 2085ms + ✓ node tests/integration/infrastructure/repositories/sqlite-interactive-session.repository.test.ts (13 tests) 313ms + ✓ node tests/integration/infrastructure/repositories/sqlite-interactive-message.repository.test.ts (10 tests) 281ms + ✓ node tests/integration/infrastructure/repositories/feature-parent.repository.test.ts (14 tests) 398ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/gate-tests.test.ts (2 tests) 4436ms + ✓ commit-only-with-gate: should interrupt at merge gate when allowMerge=false 2469ms + ✓ push-pr-with-gate: should interrupt at merge gate when allowMerge=false (PR path) 1966ms + ✓ node tests/integration/infrastructure/repositories/sqlite-phase-timing.repository.test.ts (14 tests) 427ms + ✓ node tests/integration/application/use-cases/settings/onboarding-to-feature-flow.test.ts (12 tests) 364ms + ✓ node tests/integration/application/use-cases/settings/workflow-command-flow.test.ts (11 tests) 388ms + ✓ node tests/integration/infrastructure/repositories/sqlite-feature-repository.find-by-branch.test.ts (6 tests) 254ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/merge-flow.test.ts (9 tests) 264ms + ✓ node tests/integration/application/use-cases/features/start-feature.use-case.test.ts (10 tests) 457ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/reject-feedback-propagation.test.ts (4 tests) 318ms + ✓ node tests/integration/infrastructure/services/agents/agent-infrastructure.test.ts (13 tests) 286ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/evidence-flow.test.ts (13 tests) 306ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/reject-flow.test.ts (6 tests) 197ms + ✓ node tests/integration/services/tool-installer.service.test.ts (32 tests) 266ms + ✓ node tests/integration/application/use-cases/settings/onboarding-flow.test.ts (4 tests) 168ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/resume-feedback-propagation.test.ts (7 tests) 223ms + ✓ node tests/integration/application/use-cases/agents/configure-agent-dev.test.ts (5 tests) 219ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/resume-after-error.test.ts (5 tests) 183ms + ✓ node tests/integration/infrastructure/services/agents/merge-flow.test.ts (11 tests) 280ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/resume-after-error-merge.test.ts (6 tests) 233ms + ✓ node tests/integration/api/agent-events-sse.test.ts (12 tests) 70ms + ✓ node tests/integration/infrastructure/persistence/sqlite/migration-049.test.ts (5 tests) 266ms + ✓ node tests/integration/infrastructure/persistence/sqlite/legacy-migrations.test.ts (18 tests) 134ms + ✓ node tests/integration/infrastructure/persistence/sqlite/migration-015.test.ts (4 tests) 140ms + ✓ node tests/integration/infrastructure/services/agents/hitl-approval-flow.test.ts (7 tests) 126ms + ✓ node tests/integration/infrastructure/persistence/sqlite/sqlite-migration-storage.test.ts (22 tests) 14ms + ✓ node tests/integration/infrastructure/services/agents/sessions/claude-code-session.repository.test.ts (23 tests) 57ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/approve-after-failed-rejection.test.ts (3 tests) 120ms + ✓ node tests/integration/application/use-cases/features/list-features.use-case.test.ts (2 tests) 42ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/gate-configuration.test.ts (5 tests) 113ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/approve-flow.test.ts (3 tests) 128ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/feedback-and-timing.test.ts (2 tests) 93ms + ✓ node tests/integration/infrastructure/services/git/git-pr-ci-watch.test.ts (5 tests) 5ms + ✓ node tests/integration/services/git/git-pr.service.getFailureLogs.test.ts (11 tests) 5ms + ✓ node tests/integration/prd-approval-iterations.test.ts (7 tests) 22ms + ✓ node tests/integration/infrastructure/services/agents/graph-state-transitions/yaml-repair-cycle.test.ts (3 tests) 7ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/local-merge.test.ts (3 tests) 7144ms + ✓ local-merge-no-push: feature branch should be merged into main after node completes 2840ms + ✓ no-remote-override-merge: should merge locally when remote unavailable (push+openPr overridden) 2624ms + ✓ no-remote-local-merge: should merge locally without any remote 1677ms + ✓ node tests/integration/infrastructure/services/git/merge-step-real-git/verify-merge.test.ts (6 tests) 8957ms + ✓ should return true after a true merge (git merge) 1761ms + ✓ should return true after a squash merge (git merge --squash) 2683ms + ✓ should return false when branch has not been merged 950ms + ✓ should return false when main has extra commits after squash but feature has diverged 1674ms + ✓ should return true after squash merge with modified tree when premergeBaseSha provided 1051ms + ✓ should return true after squash merge even when local branch is deleted (remote fallback) 838ms + ✓ node tests/integration/services/git/git-pr.service.rebase-sync.test.ts (16 tests) 19843ms + ✓ should fetch latest main when upstream has new commits (from feature branch) 3075ms + ✓ should fast-forward main when on the main branch (uses pull) 2904ms + ✓ should succeed silently when main is already up-to-date 1395ms + ✓ should throw SYNC_FAILED when on main and local main has diverged from remote 1186ms + ✓ should succeed from feature branch even when local main has diverged (fetches origin/main only) 1074ms + ✓ should be idempotent — calling sync twice succeeds 920ms + ✓ should successfully rebase feature branch onto main with no conflicts 1214ms + ✓ should throw REBASE_CONFLICT when rebase encounters conflicting changes 959ms + ✓ should throw GIT_ERROR when worktree has uncommitted changes 593ms + ✓ should throw BRANCH_NOT_FOUND when feature branch does not exist 598ms + ✓ should leave worktree clean after aborting a conflicted rebase 987ms + ✓ should produce linear history after successful rebase 1147ms + ✓ should succeed when feature branch is already up-to-date with main (no-op rebase) 611ms + ✓ getConflictedFiles should return conflicted file paths during a rebase conflict 957ms + ✓ stageFiles + rebaseContinue should complete rebase after manual conflict resolution 1227ms + ✓ rebaseAbort should restore branch to pre-rebase state 993ms + + Test Files 50 passed (50) + Tests 581 passed (581) + Start at 13:58:23 + Duration 20.12s diff --git a/specs/084-feature-dependency-rebase/evidence/unit-test-results.txt b/specs/084-feature-dependency-rebase/evidence/unit-test-results.txt new file mode 100644 index 000000000..f41f8ec8d --- /dev/null +++ b/specs/084-feature-dependency-rebase/evidence/unit-test-results.txt @@ -0,0 +1,110 @@ +=== Feature-Specific Unit Test Results === +Test run date: 2026-04-07T13:58:00Z +Branch: feat/feature-dependency-rebase +Command: pnpm test:unit (full unit suite) + +=== ReparentFeatureUseCase (19 tests) === +File: tests/unit/use-cases/features/reparent-feature.use-case.test.ts +Status: PASSED (14ms) +Coverage: task-1, task-2 (validation, lifecycle adjustment) + +=== GitPrService.rebaseOnBranch (13 tests) === +File: tests/unit/infrastructure/services/git/git-pr.service.rebase-on-branch.test.ts +Status: PASSED (8ms) +Coverage: task-4, task-5 (interface + implementation) + +=== CheckAndUnblockFeaturesUseCase Auto-Rebase (16 tests) === +File: tests/unit/application/use-cases/features/check-and-unblock-features-rebase.test.ts +Status: PASSED (47ms) +Coverage: task-6, task-7 (DI injection + auto-rebase orchestration) + +=== Control Center Integration (24 tests) === +File: tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx +Status: PASSED (605ms) +Coverage: task-8, task-9, task-10, task-11, task-14 (canvas reparenting flow) + +=== SUMMARY === +Feature-specific tests: 4 files, 72 tests - ALL PASSED +Full unit suite (this run): 383 files, 5402 tests passed +Duration: 40.85s + +--- Detailed Test Output (vitest run, ANSI stripped) --- + + ✓ node tests/unit/infrastructure/services/git/git-pr.service.rebase-on-branch.test.ts (13 tests) 8ms + ✓ should throw GIT_ERROR when worktree is dirty + ✓ should fetch the target branch before rebasing + ✓ should checkout the feature branch before rebasing + ✓ should call git rebase with origin/ as target + ✓ should succeed on clean rebase (no conflicts) + ✓ should throw REBASE_CONFLICT when rebase encounters conflicts + ✓ should include conflicted file list in REBASE_CONFLICT error message + ✓ should detect "could not apply" as rebase conflict + ✓ should throw BRANCH_NOT_FOUND when feature branch does not exist + ✓ should throw descriptive error when fetch of target branch fails + ✓ should throw generic GIT_ERROR on non-conflict rebase failure + ✓ should still throw REBASE_CONFLICT even if getConflictedFiles fails + ✓ should execute steps in correct order: dirty check → fetch → checkout → rebase + + ✓ node tests/unit/use-cases/features/reparent-feature.use-case.test.ts (19 tests) 14ms + ✓ should successfully reparent a feature under a new parent + ✓ should reject self-reparent + ✓ should reject direct cycle (A→B, try to reparent A under B) + ✓ should reject indirect cycle (A→B→C, try to reparent A under C) + ✓ should reject cross-repo reparent + ✓ should reject reparenting Done feature + ✓ should reject reparenting Archived feature + ✓ should reject reparenting to non-existent parent + ✓ should successfully unparent (parentId=null) + ✓ should transition Started child to Blocked when reparented under pre-implementation parent + ✓ should keep Started child unchanged when reparented under post-implementation parent + ✓ should transition Blocked child to Started when unparented + ✓ should keep Pending child unchanged when unparented + ✓ should call CheckAndUnblockFeaturesUseCase when parent is post-implementation + ✓ should NOT call CheckAndUnblockFeaturesUseCase when parent is pre-implementation + ✓ should reject reparenting to a non-existent feature + ✓ should not call repository.update on validation failure + ✓ should propagate errors from repository.update + ✓ should handle deeply nested ancestor chains in cycle detection + + ✓ node tests/unit/application/use-cases/features/check-and-unblock-features-rebase.test.ts (16 tests) 47ms + ✓ should rebase child branch onto parent branch before spawning agent + ✓ should use parent branch name as target for rebaseOnBranch + ✓ should call stashPop after successful rebase + ✓ should not call stashPop when no changes were stashed + ✓ should create agent run and phase timing for rebase operation + ✓ should delegate to ConflictResolutionService on rebase conflict + ✓ should still spawn agent when rebase fails + ✓ should record rebase failure in phase timing + ✓ should call stashPop in finally block even when rebase throws + ✓ should still rebase and spawn second child when first child rebase fails + ✓ should skip rebase when child has active agent run (NFR-3) + ✓ should resolve CWD via worktree service + ✓ should fall back to repo root when worktree does not exist + ✓ should record success with rebased commit summary in phase timing + ✓ should isolate failures per child + ✓ should respect 5-minute timeout per child rebase + + ✓ web tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx (24 tests) 605ms + ✓ handleConnect calls reparentFeature with valid feature-to-feature connection + ✓ handleConnect rejects non-feature-to-feature connections + ✓ handleConnect rejects self-connections + ✓ handleConnect rejects cross-repo connections + ✓ handleConnect rejects connections to Done features + ✓ handleConnect rejects connections to Archived features + ✓ handleEdgesDelete calls reparentFeature(null) for dependency edges + ✓ handleEdgesDelete ignores non-dependency edges + ✓ handleEdgesDelete processes multiple deletions independently + ✓ reparentFeature optimistically updates featureMap + ✓ reparentFeature rolls back on server error + ✓ reparentFeature unparent sets parentNodeId to undefined + ... (all 24 tests passed) + + Test Files 4 passed (4) + Tests 72 passed (72) + +=== TOTAL FEATURE-SPECIFIC TESTS: 72 passed (72) === + +=== FULL UNIT SUITE (this run) === + Test Files 383 passed (383) + Tests 5402 passed (5402) + Duration 40.85s diff --git a/specs/084-feature-dependency-rebase/feature.yaml b/specs/084-feature-dependency-rebase/feature.yaml new file mode 100644 index 000000000..e5cda4078 --- /dev/null +++ b/specs/084-feature-dependency-rebase/feature.yaml @@ -0,0 +1,41 @@ +feature: + id: "084-feature-dependency-rebase" + name: "feature-dependency-rebase" + number: 84 + branch: "feat/084-feature-dependency-rebase" + lifecycle: "plan" + createdAt: "2026-04-05T15:14:31Z" +status: + phase: "implementation-complete" + progress: + completed: 14 + total: 14 + percentage: 100 + currentTask: null + lastUpdated: "2026-04-07T11:15:48.776Z" + lastUpdatedBy: "feature-agent:implement" + completedPhases: + - "analyze" + - "requirements" + - "research" + - "plan" + - "phase-1" + - "phase-2" + - "phase-3" + - "phase-4" + - "evidence" +validation: + lastRun: null + gatesPassed: [] + autoFixesApplied: [] +tasks: + current: null + blocked: [] + failed: [] +checkpoints: + - phase: "feature-created" + completedAt: "2026-04-05T15:14:31Z" + completedBy: "feature-agent" +errors: + current: null + history: [] diff --git a/specs/084-feature-dependency-rebase/plan.yaml b/specs/084-feature-dependency-rebase/plan.yaml new file mode 100644 index 000000000..8fa9aebd4 --- /dev/null +++ b/specs/084-feature-dependency-rebase/plan.yaml @@ -0,0 +1,227 @@ +# Implementation Plan (YAML) +# This is the source of truth. Markdown is auto-generated from this file. + +name: "feature-dependency-rebase" +summary: > + Implementation plan for interactive feature reparenting via React Flow drag-to-connect + and automatic child branch rebase on parent completion. Four phases: (1) core use case + with validation and DI wiring, (2) git infrastructure for branch-targeted rebase, + (3) auto-rebase orchestration in CheckAndUnblockFeaturesUseCase, (4) canvas interaction + and optimistic UI. All infrastructure exists — no new libraries, no migrations, no + TypeSpec changes. Extends specs 041 (feature dependencies) and 080 (smart rebase). + +relatedFeatures: + - "041-feature-dependencies" + - "080-smart-rebase" + +technologies: + - "TypeScript" + - "React 19" + - "Next.js 15" + - "@xyflow/react (React Flow v12)" + - "dagre (graph layout)" + - "tsyringe (DI)" + - "better-sqlite3" + - "node:child_process (git operations)" + - "shadcn/ui" + - "sonner (toast notifications)" + - "lucide-react" + +relatedLinks: + - title: "React Flow Connection Handling API" + url: "https://reactflow.dev/api-reference/react-flow#connection-handling" + - title: "React Flow Edge Utils (BaseEdge, EdgeLabelRenderer)" + url: "https://reactflow.dev/api-reference/utils/edge-utils" + - title: "Spec 041 - Feature Dependencies" + url: "./specs/041-feature-dependencies/" + - title: "Spec 080 - Smart Rebase" + url: "./specs/080-smart-rebase/" + +phases: + - id: "phase-1" + name: "ReparentFeatureUseCase + Server Action + DI" + description: > + Build the core reparenting use case with all business validations (cycle detection, + same-repo check, lifecycle guards, lifecycle state adjustment), the server action, + and DI container registration. This phase establishes the backend contract that all + subsequent phases depend on. Comes first because both the canvas interaction (phase 4) + and the auto-rebase orchestration (phase 3) need the use case to exist. + parallel: false + + - id: "phase-2" + name: "Git Infrastructure — rebaseOnBranch" + description: > + Add the rebaseOnBranch method to IGitPrService interface and implement it in + GitPrService. This extends the existing rebaseOnMain pattern to target an arbitrary + branch instead of origin/. Phase 2 before phase 3 because + CheckAndUnblockFeaturesUseCase needs this method to orchestrate child rebases. + parallel: false + + - id: "phase-3" + name: "Auto-Rebase Orchestration in CheckAndUnblockFeaturesUseCase" + description: > + Inject rebase orchestration into CheckAndUnblockFeaturesUseCase between the + lifecycle transition and agent spawn. Each child's branch is rebased onto the + parent's branch with stash/conflict-resolution/timeout handling. Failures are + isolated per-child and recorded in the activity timeline via agent run + phase + timing records. Depends on phase 1 (use case exists) and phase 2 (rebaseOnBranch + method exists). + parallel: false + + - id: "phase-4" + name: "Canvas Interaction + Optimistic UI" + description: > + Enable React Flow drag-to-connect for reparenting and edge deletion for unparenting. + Add reparentFeature mutation to useGraphState with optimistic updates. Implement + handleConnect in use-control-center-state.ts with client-side validation. Enhance + DependencyEdge with hover delete button. Update feature node handles. All Storybook + stories colocated. This phase comes last because it's the presentation layer that + calls the backend established in phases 1-3. + parallel: false + +filesToCreate: + - "packages/core/src/application/use-cases/features/reparent-feature.use-case.ts" + - "packages/core/src/application/use-cases/features/__tests__/reparent-feature.use-case.test.ts" + - "packages/core/src/application/use-cases/features/__tests__/check-and-unblock-features-rebase.test.ts" + - "packages/core/src/infrastructure/services/git/__tests__/git-pr-rebase-on-branch.test.ts" + - "src/presentation/web/app/actions/reparent-feature.ts" + - "src/presentation/web/components/features/features-canvas/dependency-edge.stories.tsx" + +filesToModify: + - "packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts" + - "packages/core/src/application/ports/output/services/git-pr-service.interface.ts" + - "packages/core/src/infrastructure/services/git/git-pr.service.ts" + - "packages/core/src/infrastructure/di/container.ts" + - "src/presentation/web/hooks/use-graph-state.ts" + - "src/presentation/web/components/features/control-center/use-control-center-state.ts" + - "src/presentation/web/components/features/features-canvas/features-canvas.tsx" + - "src/presentation/web/components/features/features-canvas/dependency-edge.tsx" + - "src/presentation/web/components/common/feature-node/feature-node.tsx" + - "src/presentation/web/lib/derive-graph.ts" + +openQuestions: [] + +content: | + ## Architecture Overview + + This feature extends two existing systems — feature dependencies (spec 041) and smart + rebase (spec 080) — without introducing new architectural patterns. It adds two capabilities: + + 1. **Interactive reparenting** — a presentation-layer interaction (React Flow drag-to-connect) + that calls a new application-layer use case (ReparentFeatureUseCase) for validation and + persistence, with optimistic UI updates in the useGraphState domain Maps. + + 2. **Auto-rebase on parent completion** — an extension of CheckAndUnblockFeaturesUseCase that + inserts git rebase orchestration between the existing lifecycle transition and agent spawn, + using a new rebaseOnBranch method in IGitPrService. + + Both capabilities follow the established Clean Architecture layers: + - **Domain**: No changes. parentId field already exists on Feature entity (TypeSpec-generated). + - **Application**: New ReparentFeatureUseCase. Extended CheckAndUnblockFeaturesUseCase. + New rebaseOnBranch on IGitPrService interface. + - **Infrastructure**: Implement rebaseOnBranch in GitPrService. Register new use case in DI. + - **Presentation**: Canvas interaction (onConnect, onEdgesDelete), optimistic mutations, + enhanced DependencyEdge, server action. + + No new libraries, no database migrations, no TypeSpec changes required. + + ## Key Design Decisions + + ### 1. ReparentFeatureUseCase as Standalone Use Case + Each feature operation has its own use case class (CreateFeatureUseCase, DeleteFeatureUseCase, + StartFeatureUseCase, etc.). ReparentFeatureUseCase follows this pattern: single responsibility, + injectable dependencies, registered with string token alias for web server actions (per + LESSONS.md pattern). Cycle detection reuses the upward ancestor walk from CreateFeatureUseCase. + + **Alternatives rejected:** + - Extend CreateFeatureUseCase — violates single responsibility principle + - Inline logic in server action — violates Clean Architecture + + ### 2. Two-Layer Validation (Client + Server) + Client-side validation in onConnect handler provides instant feedback (<200ms, NFR-4) for fast + checks: non-feature nodes, self-connection, cross-repo, lifecycle guards. Server-side validation + in ReparentFeatureUseCase is authoritative — performs cycle detection via DB ancestor walk. + If server rejects, optimistic UI rolls back with toast error. + + **Alternatives rejected:** + - Server-only — round-trip latency violates 200ms feedback requirement + - Client-only — insufficient for deep cycle detection, race conditions possible + + ### 3. Optimistic UI via Domain Map Mutation + The codebase uses domain Maps (featureMap, repoMap, pendingMap) as the single source of truth. + All React Flow nodes/edges are derived via deriveGraph(). The reparentFeature mutation updates + parentNodeId in featureMap, triggering edge re-derivation and dagre re-layout automatically + via the topology-key cache mechanism. Uses beginMutation/endMutation ref-counting to prevent + stale poll data from overwriting optimistic state. + + **Alternatives rejected:** + - Direct React Flow edge manipulation — bypasses domain Map source of truth + - Server-first with loading state — violates 100ms latency requirement (NFR-1) + + ### 4. Rebase Integration in CheckAndUnblockFeaturesUseCase + Rather than creating a separate event-triggered use case (no event system exists) or triggering + from UpdateFeatureLifecycleUseCase (wrong abstraction level), rebase orchestration is injected + into CheckAndUnblockFeaturesUseCase between lifecycle DB update and agent spawn. Each child + rebases independently — one failure does not block others. Rebase awaited per-child (with + 5-min timeout) before agent spawn, but children process in parallel. + + **Alternatives rejected:** + - Separate AutoRebaseChildrenUseCase triggered by event — no event system in codebase + - Fire-and-forget (non-blocking per-child) — agent may work on stale branch + + ### 5. rebaseOnBranch as New IGitPrService Method + New method follows rebaseOnMain pattern but targets origin/. Must fetch the + target branch first since it may not be locally available. Same conflict detection + (REBASE_CONFLICT error code) and error handling pattern. + + **Alternatives rejected:** + - Overload rebaseOnMain — changes semantics for all callers + - Shell out from use case — violates port interface pattern + + ### 6. Edge Deletion via EdgeLabelRenderer + edgesSelectable + DependencyEdge enhanced with hover-visible delete button (X icon) using EdgeLabelRenderer. + Edge selection enabled via edgesSelectable=true while keeping nodesSelectable behavior + unchanged. Edge click selects edge; node click opens drawer (unchanged). Keyboard Delete + supported via onEdgesDelete callback. + + ### 7. Handle Visibility for Connections + Feature node handles set isConnectable=true and always render (remove data.showHandles + conditional for target handles). Handles visually hidden (opacity-0) until hover when + nodesConnectable=true. Source handle's existing Plus button serves as connection source point. + + ## Implementation Strategy + + The four phases follow the Clean Architecture dependency rule — inner layers first: + + **Phase 1 (Core Use Case)**: Build ReparentFeatureUseCase with all validations (cycle, + same-repo, lifecycle), lifecycle state adjustment, server action, and DI wiring. Fully + testable in isolation with mocked repositories. This is the backend contract everything + else depends on. + + **Phase 2 (Git Infrastructure)**: Add rebaseOnBranch to IGitPrService interface and implement + in GitPrService. Follows rebaseOnMain pattern closely — the delta is targeting + origin/ instead of origin/, plus an explicit fetch of the target + branch. Small, focused change with high confidence. + + **Phase 3 (Auto-Rebase Orchestration)**: Wire rebase into CheckAndUnblockFeaturesUseCase. + For each Blocked child: create agent run, resolve CWD, stash, fetch parent branch, rebase, + handle conflicts via ConflictResolutionService, restore stash, record timing, spawn agent. + All patterns established in RebaseFeatureOnMainUseCase — this is assembly, not invention. + + **Phase 4 (Canvas UI)**: Enable connections on FeaturesCanvas (nodesConnectable=true, + edgesSelectable=true). Implement handleConnect with client-side validation and server action + call. Add reparentFeature mutation to useGraphState. Enhance DependencyEdge with delete + button. Update feature node handles. Storybook stories for DependencyEdge. + + ## Risk Mitigation + + | Risk | Mitigation | + | ---- | ---------- | + | Cycle detection misses edge cases | TDD: write tests for self-reparent, direct cycle (A->B->A), indirect cycle (A->B->C->A), reparent-to-own-descendant. Reuse ancestor walk pattern from CreateFeatureUseCase. | + | Optimistic UI flicker on poll reconciliation | Use existing beginMutation/endMutation ref-counting with 3-second cooldown. Test with simulated poll during mutation window. | + | Rebase conflicts leave dirty git state | Stash/stashPop in finally block (pattern from RebaseFeatureOnMainUseCase). Rebase abort in catch block. | + | Multiple children rebase concurrently on same repo | Each child uses its own worktree. Skip rebase if child has active agent run (NFR-3). | + | Handle visibility breaks zero-dependency canvas | Always render handles but opacity-0. Only show on hover when nodesConnectable. Test backward compatibility. | + | Edge selection interferes with node click | edgesSelectable=true + default nodesSelectable. Test both interactions coexist. | + | rebaseOnBranch fails to fetch remote branch | Explicit fetch before rebase. On fetch failure, record error, skip rebase, agent spawns on un-rebased branch. | + | Reparenting + immediate rebase creates race | ReparentFeatureUseCase calls CheckAndUnblockFeaturesUseCase synchronously after parentId update. Sequential execution prevents race. | diff --git a/specs/084-feature-dependency-rebase/research.yaml b/specs/084-feature-dependency-rebase/research.yaml new file mode 100644 index 000000000..16a836f00 --- /dev/null +++ b/specs/084-feature-dependency-rebase/research.yaml @@ -0,0 +1,424 @@ +# Research Artifact (YAML) +# This is the source of truth. Markdown is auto-generated from this file. + +name: "feature-dependency-rebase" +summary: > + Comprehensive technical analysis for interactive feature reparenting via React Flow + drag-to-connect and automatic child branch rebase on parent completion. All infrastructure + exists — no new libraries needed. Key decisions: extend existing onConnect no-op handler + in use-control-center-state.ts, add reparentFeature mutation to useGraphState, add + rebaseOnBranch to IGitPrService, and inject rebase orchestration into + CheckAndUnblockFeaturesUseCase between lifecycle transition and agent spawn. + +relatedFeatures: + - "041-feature-dependencies" + - "080-smart-rebase" + +technologies: + - "TypeScript" + - "React 19" + - "Next.js 15" + - "@xyflow/react (React Flow v12)" + - "dagre (graph layout)" + - "tsyringe (DI)" + - "better-sqlite3" + - "node:child_process (git operations)" + - "shadcn/ui" + - "sonner (toast notifications)" + +relatedLinks: + - title: "React Flow Connection Handling API" + url: "https://reactflow.dev/api-reference/react-flow#connection-handling" + - title: "React Flow Edge Utils (BaseEdge, EdgeLabelRenderer)" + url: "https://reactflow.dev/api-reference/utils/edge-utils" + - title: "Spec 041 - Feature Dependencies" + url: "./specs/041-feature-dependencies/" + - title: "Spec 080 - Smart Rebase" + url: "./specs/080-smart-rebase/" + +decisions: + - title: "Canvas Reparenting Interaction Pattern" + chosen: "Extend existing React Flow onConnect handler in use-control-center-state.ts" + rejected: + - "Custom drag-and-drop with onNodeDragStop + intersection detection — would conflict with nodesDraggable=false (currently disabled for layout stability) and require custom hit-testing logic. Much more complex for no user benefit." + - "Context menu right-click approach — two-step interaction (right-click then select target) is less discoverable and violates the natural graph-editing UX users expect from node editors." + rationale: | + The codebase already has the plumbing: features-canvas.tsx passes onConnect to ReactFlow + (line 153), control-center-inner.tsx wires handleConnect (line 516), and + use-control-center-state.ts defines handleConnect as a no-op (line 311). Feature nodes + already render source and target Handle components with isConnectable=false (feature-node.tsx + lines 138-146 for target, 698-734 for source). Enabling connections requires only: + (1) set nodesConnectable=true on ReactFlow, (2) set isConnectable=true on handles, + (3) implement the handleConnect callback. This is the native React Flow idiom and the + lowest-effort approach. + + - title: "Edge Deletion for Unparenting" + chosen: "Enhance DependencyEdge component with hover delete button and keyboard delete support" + rejected: + - "Context menu on the edge — adds context menu complexity, less discoverable than visual delete button, and inconsistent with standard graph-editing conventions." + - "Toolbar action after selecting a feature — requires multi-step interaction (select child, click toolbar unparent button), less intuitive than direct edge interaction." + rationale: | + The existing DependencyEdge component (dependency-edge.tsx) is a minimal BaseEdge wrapper + with dashed styling. It currently has no interactive elements. Adding a hover-visible + delete button (X icon from lucide-react) using React Flow's EdgeLabelRenderer is the + standard pattern. React Flow supports onEdgesDelete callback and edge selection natively. + The canvas currently has elementsSelectable=false — this must be changed to allow edge + selection for keyboard delete. We can scope selectability to edges only via + edgesSelectable=true while keeping nodesSelectable=false. + + - title: "Optimistic UI Strategy for Reparenting" + chosen: "Add reparentFeature mutation to useGraphState that updates parentNodeId in featureMap" + rejected: + - "Direct edge manipulation via React Flow's addEdge/removeEdge — bypasses the domain Map source of truth. The codebase derives all edges from featureMap/repoMap via deriveGraph (derive-graph.ts). Direct edge manipulation would be overwritten on next derivation cycle." + - "Server-first with loading state — wait for server response before updating UI. Violates NFR-1 (100ms latency requirement) and degrades perceived performance." + rationale: | + The codebase uses domain Maps (featureMap, repoMap, pendingMap) as the single source of + truth. All React Flow nodes/edges are derived from these Maps via deriveGraph(). The + existing optimistic mutation pattern uses beginMutation()/endMutation() to block poll + reconciliation during in-flight mutations (use-graph-state.ts lines 546-560). The + reparentFeature mutation must: (1) call beginMutation(), (2) update the child's + parentNodeId in featureMap (or set to undefined for unparent), (3) call the server action, + (4) on error, revert parentNodeId and show toast, (5) call endMutation(). This triggers + automatic re-derivation of edges and dagre re-layout via the topology-key cache mechanism + (the edge set changes, so the cache key changes, triggering a full layout recalculation). + + - title: "ReparentFeatureUseCase Architecture" + chosen: "New standalone use case in packages/core/src/application/use-cases/features/ with DI registration" + rejected: + - "Extend CreateFeatureUseCase to handle reparent — violates single responsibility principle. CreateFeatureUseCase handles feature creation flow (spec setup, agent spawn, worktree). Reparent is a distinct operation with different validation and side effects." + - "Inline reparent logic in the server action — violates Clean Architecture. Business logic (cycle detection, lifecycle guards, state adjustment) must live in the application layer, not the presentation layer." + rationale: | + Following the established pattern: each feature operation has its own use case class + (CreateFeatureUseCase, DeleteFeatureUseCase, StartFeatureUseCase, etc.), registered as a + singleton in the DI container with a string token alias for web server actions. The new + ReparentFeatureUseCase will: (1) validate same-repo constraint, (2) detect cycles via + upward ancestor walk (reuse pattern from CreateFeatureUseCase lines 106-116), (3) enforce + lifecycle guards (reject Done/Archived), (4) update parentId via IFeatureRepository.update(), + (5) adjust child lifecycle state based on new parent's lifecycle, (6) trigger rebase if new + parent is post-implementation. The use case gets registered with class token + string alias + 'ReparentFeatureUseCase' (per LESSONS.md pattern). + + - title: "Auto-Rebase Integration Point" + chosen: "Inject rebase orchestration into CheckAndUnblockFeaturesUseCase between lifecycle transition and agent spawn" + rejected: + - "Create a separate AutoRebaseChildrenUseCase triggered by an event — adds event system complexity not present in the codebase. The existing pattern is synchronous use-case chaining (UpdateFeatureLifecycleUseCase calls CheckAndUnblockFeaturesUseCase directly)." + - "Trigger rebase from UpdateFeatureLifecycleUseCase directly — wrong level of abstraction. CheckAndUnblockFeaturesUseCase already encapsulates the parent-child unblock logic. Adding rebase here keeps all child-transition logic in one place." + rationale: | + CheckAndUnblockFeaturesUseCase (check-and-unblock-features.use-case.ts) is the single + point where blocked children are transitioned to Started and their agents spawned. The + current flow is: (1) verify parent is POST_IMPLEMENTATION, (2) load children via + findByParentId, (3) for each Blocked child: set lifecycle=Started, update DB, spawn agent. + The rebase must happen between steps 3's DB update and agent spawn. Each child's rebase + runs independently (fire-and-forget with error recording) so one failure doesn't block + others. The use case needs new dependencies injected: IGitPrService, IWorktreeService, + ConflictResolutionService, IAgentRunRepository, IPhaseTimingRepository — following the + exact pattern from RebaseFeatureOnMainUseCase. + + - title: "Git Rebase on Arbitrary Branch" + chosen: "Add rebaseOnBranch method to IGitPrService interface and implement in GitPrService" + rejected: + - "Reuse rebaseOnMain with branch parameter overloading — rebaseOnMain's signature is (cwd, featureBranch, baseBranch) and it always targets origin/. Overloading would change its semantics for all callers, which is risky and confusing." + - "Shell out directly from the use case — bypasses the port interface, violates Clean Architecture (use cases must not know about git commands), and duplicates git error handling." + rationale: | + The existing rebaseOnMain method (git-pr.service.ts lines 830-901) always targets + origin/. For parent-branch rebase, we need to target origin/ + where parentBranch is the parent feature's branch name. The new rebaseOnBranch method will + follow the same implementation pattern: (1) check dirty worktree, (2) checkout feature + branch, (3) rebase onto origin/. The key difference is that it must first + fetch the target branch (git fetch origin ) since it may not have been fetched + recently. This reuses the same conflict detection pattern (check stderr for CONFLICT/could + not apply) and throws REBASE_CONFLICT for the ConflictResolutionService to handle. + + - title: "Connection Validation Strategy" + chosen: "Client-side pre-validation with server-side authoritative validation" + rejected: + - "Server-only validation — round-trip latency violates NFR-4 (200ms feedback). User would see a brief connected state before the server rejects and it snaps back." + - "Client-only validation — insufficient for cycle detection on deep trees (client may not have complete ancestor chain in memory). Also allows race conditions where two clients could create a cycle simultaneously." + rationale: | + Two-layer validation provides both fast feedback and correctness. Client-side validation in + the onConnect handler covers: (1) reject non-feature-to-feature connections (repo group + nodes have different node IDs — check for 'feat-' prefix), (2) reject self-connections, + (3) reject cross-repo (compare repositoryPath from featureMap entries), (4) reject + Done/Archived lifecycle. These are fast local checks. The server-side ReparentFeatureUseCase + performs authoritative validation including cycle detection (upward ancestor walk via DB + queries). If server rejects, the optimistic UI rolls back and shows a toast error. + + - title: "Rebase Execution Mode" + chosen: "Async fire-and-forget with per-child error isolation and activity timeline recording" + rejected: + - "Synchronous blocking rebase — parent lifecycle transition blocks on child git operations. With 5 children, this makes the transition unacceptably slow and defeats the purpose of async unblocking." + - "Queue-based deferred rebase — adds a job queue system not present in the codebase. Over-engineering for the current scale. If a child has 0-2 parents, immediate async execution is sufficient." + rationale: | + The spec requires async execution (FR-9, NFR-2). Each child rebase creates its own agent + run and phase timing record (following RebaseFeatureOnMainUseCase pattern), so failures + appear in the activity timeline without additional UI. The pattern is: after updating child + lifecycle to Started, fire off the rebase as a Promise that catches its own errors and + records them. The agent spawn proceeds regardless — if the rebase fails, the agent will + work on an un-rebased branch (which may cause conflicts later but doesn't block work). + A timeout of 5 minutes per child prevents runaway processes (NFR-2). + + - title: "Handle Configuration for Feature Nodes" + chosen: "Set isConnectable=true on both source and target handles, keep showHandles logic" + rejected: + - "Always show handles regardless of edges — changes visual appearance for features with no dependencies. The current behavior (handles hidden when no edges) is intentional for visual cleanliness." + - "Add separate connection handles distinct from the existing layout handles — doubles the handle count, creates visual clutter, and React Flow connection logic would need to distinguish between layout handles and connection handles." + rationale: | + The existing handles (feature-node.tsx) serve dual purpose: layout positioning for dagre + and connection anchoring for React Flow. Currently isConnectable=false hides the connection + affordance. Setting isConnectable=true shows the connection dots on hover. The showHandles + logic (derived from edges.length > 0 in derive-graph.ts line 242) controls visibility — + but for connections to work, handles must be rendered even when no edges exist. The fix: + always render handles (remove the data.showHandles conditional for target handles), but + keep them visually hidden (opacity-0) until hover when nodesConnectable=true. The source + handle already has the Plus button which can serve as the connection source point. + +openQuestions: + - question: "Should the auto-rebase await completion before spawning the child agent, or should it fire-and-forget?" + resolved: true + options: + - option: "Fire-and-forget (non-blocking)" + description: "Start rebase async, spawn agent immediately. Agent works on potentially un-rebased branch. If rebase finishes first (likely for clean rebases), agent benefits. If rebase fails, agent works on stale branch — conflicts surface later." + selected: false + - option: "Await rebase then spawn agent" + description: "Wait for rebase to complete (with timeout) before spawning the child agent. Agent always works on the rebased branch. But blocks agent spawn for the duration of the rebase (seconds to minutes). Sequential per-child." + selected: true + - option: "Parallel rebase + agent spawn with sync point" + description: "Start rebase and agent in parallel. Agent waits at a sync point (e.g., before implementation phase) for rebase to complete. Complex coordination but maximizes parallelism." + selected: false + selectionRationale: | + The spec says async (FR-9: 'rebase the child feature branch before spawning the child + agent'). The word 'before' implies sequential: rebase first, then spawn. Awaiting the + rebase ensures the agent always works on a properly rebased branch. The timeout (5 min + per child, NFR-2) prevents indefinite blocking. If rebase fails, the agent still spawns + (the failure is recorded and the agent works on the un-rebased branch). Each child's + rebase+spawn sequence is independent of other children, so multiple children process + in parallel at the child level. + + - question: "How should the immediate rebase on reparent work when the new parent is already post-implementation?" + resolved: true + options: + - option: "Call CheckAndUnblockFeaturesUseCase after reparent" + description: "After updating parentId, call CheckAndUnblockFeaturesUseCase with the new parent's ID. If the new parent is post-implementation and the child is Blocked, the existing unblock+rebase flow triggers automatically. Reuses existing infrastructure." + selected: true + - option: "Inline rebase logic in ReparentFeatureUseCase" + description: "ReparentFeatureUseCase directly calls the git rebase methods when the new parent is post-implementation. Duplicates the rebase orchestration logic already in CheckAndUnblockFeaturesUseCase." + selected: false + - option: "No immediate rebase, manual trigger required" + description: "Reparenting only updates the dependency relationship. User must manually trigger rebase if needed. Simplest but least helpful — user may not realize the branch is stale." + selected: false + selectionRationale: | + Calling CheckAndUnblockFeaturesUseCase from ReparentFeatureUseCase is the cleanest + approach. The use case already handles the gate check (is parent post-implementation?), + child lifecycle transition, rebase orchestration, and agent spawn. ReparentFeatureUseCase + only needs to: (1) update the parentId, (2) adjust lifecycle if needed (FR-8), (3) call + CheckAndUnblockFeaturesUseCase.execute(newParentId) which handles the rest. This avoids + duplicating rebase logic. + + - question: "How should elementsSelectable interact with the new connection and edge deletion features?" + resolved: true + options: + - option: "Enable edgesSelectable=true, keep nodesSelectable=false" + description: "React Flow v12 supports granular selectability. Allow edge selection (for keyboard delete) while keeping nodes non-selectable (preserving current behavior where nodes don't get selected/highlighted)." + selected: true + - option: "Enable elementsSelectable=true for both nodes and edges" + description: "Allow selection of all elements. This changes the current node interaction model — clicking a node would select it (blue highlight) instead of opening the drawer." + selected: false + - option: "Keep elementsSelectable=false, use custom click handler on edges" + description: "Don't use React Flow's selection system. Instead, add a custom click handler on DependencyEdge that shows a delete confirmation. Avoids changing selectability but loses keyboard delete support." + selected: false + selectionRationale: | + React Flow v12 supports separate edgesSelectable and nodesSelectable props. The current + canvas sets elementsSelectable=false (features-canvas.tsx line 162) to prevent selection + interfering with the drawer interaction. We can replace this with edgesSelectable=true and + keep the default nodesSelectable behavior. Edge selection enables the keyboard delete flow + (select edge via click, press Delete/Backspace, triggers onEdgesDelete). The hover delete + button on DependencyEdge provides a mouse-only alternative. + +content: | + ## Technology Decisions + + ### 1. Canvas Reparenting Interaction Pattern + + **Chosen:** Extend existing React Flow onConnect handler in use-control-center-state.ts + + **Rejected:** + - Custom drag-and-drop with onNodeDragStop — conflicts with nodesDraggable=false and requires custom hit-testing + - Context menu right-click approach — two-step interaction, less discoverable + + **Rationale:** The codebase already has the complete plumbing. FeaturesCanvas passes onConnect to ReactFlow (line 153), the handleConnect callback exists as a documented no-op (line 311). Feature nodes render source/target Handle components. Enabling connections requires setting nodesConnectable=true and implementing the handler — no structural changes needed. + + ### 2. Edge Deletion for Unparenting + + **Chosen:** Enhance DependencyEdge with hover delete button + keyboard delete + + **Rejected:** + - Context menu on edge — less discoverable, adds menu complexity + - Toolbar action after selecting feature — multi-step interaction + + **Rationale:** DependencyEdge is currently a minimal BaseEdge wrapper. Adding a hover-visible delete button using EdgeLabelRenderer is the standard React Flow pattern. Combined with edge selection (edgesSelectable=true) for keyboard delete, this covers both mouse and keyboard interaction paths. + + ### 3. Optimistic UI Strategy + + **Chosen:** Add reparentFeature mutation to useGraphState + + **Rejected:** + - Direct React Flow edge manipulation — bypasses domain Map source of truth, gets overwritten on derivation + - Server-first approach — violates 100ms latency requirement (NFR-1) + + **Rationale:** Domain Maps are the single source of truth. Updating parentNodeId in featureMap triggers automatic edge re-derivation via deriveGraph() and dagre re-layout via topology-key cache. The beginMutation/endMutation pattern prevents stale poll data from overwriting optimistic state. + + ### 4. ReparentFeatureUseCase Architecture + + **Chosen:** New standalone use case with DI registration + string token alias + + **Rejected:** + - Extend CreateFeatureUseCase — violates single responsibility + - Inline logic in server action — violates Clean Architecture + + **Rationale:** Follows the established one-operation-per-use-case pattern. Registered as singleton with 'ReparentFeatureUseCase' string alias for web server action resolution (per LESSONS.md). + + ### 5. Auto-Rebase Integration Point + + **Chosen:** Inject into CheckAndUnblockFeaturesUseCase between lifecycle transition and agent spawn + + **Rejected:** + - Separate event-triggered use case — no event system exists in codebase + - Trigger from UpdateFeatureLifecycleUseCase — wrong abstraction level + + **Rationale:** CheckAndUnblockFeaturesUseCase is the single point where Blocked→Started transitions happen and agents spawn. Inserting rebase orchestration between DB update and agent spawn is the natural extension. New DI dependencies: IGitPrService, IWorktreeService, ConflictResolutionService, IAgentRunRepository, IPhaseTimingRepository. + + ### 6. Git Rebase on Arbitrary Branch + + **Chosen:** Add rebaseOnBranch to IGitPrService interface + + **Rejected:** + - Overload rebaseOnMain — changes semantics for all callers + - Shell out from use case — violates port interface pattern + + **Rationale:** New method follows rebaseOnMain's implementation pattern but targets origin/ instead of origin/. Must fetch the target branch first since it may not be locally available. Same conflict detection (REBASE_CONFLICT error code). + + ### 7. Connection Validation Strategy + + **Chosen:** Two-layer: client-side fast checks + server-side authoritative validation + + **Rejected:** + - Server-only — round-trip latency violates 200ms feedback requirement + - Client-only — insufficient for deep cycle detection + + **Rationale:** Client validates fast checks (non-feature nodes, self-connection, cross-repo, lifecycle) immediately in onConnect. Server performs authoritative cycle detection via DB ancestor walk. Failed server validation rolls back optimistic UI with toast error. + + ### 8. Rebase Execution Mode + + **Chosen:** Await rebase per-child then spawn agent, parallel across children + + **Rejected:** + - Fire-and-forget — agent may work on stale branch + - Queue-based deferred — over-engineering, no job queue exists + + **Rationale:** The spec says "rebase before spawning" (FR-9). Await rebase, then spawn agent. Each child processes independently (parallel at child level). 5-minute timeout per child (NFR-2) prevents indefinite blocking. On failure, agent still spawns on un-rebased branch with failure recorded in activity timeline. + + ## Library Analysis + + | Library | Purpose | Decision | Reasoning | + | ------- | ------- | -------- | --------- | + | @xyflow/react v12 | Canvas drag-to-connect, edge deletion | Use (existing) | Already installed. onConnect, onEdgesDelete, EdgeLabelRenderer, BaseEdge all available natively. No version change needed. | + | dagre | Graph layout after reparent | Use (existing) | Already handles topology-aware re-layout. Changing parentNodeId changes edge set which changes topology key, triggering automatic re-layout. No changes to dagre usage needed. | + | sonner | Toast notifications for validation errors | Use (existing) | Already imported in use-control-center-state.ts. Pattern: `toast.error('message')`, `toast.success('message')`. | + | tsyringe | DI for ReparentFeatureUseCase | Use (existing) | @injectable() decorator + @inject() for dependencies. String token alias for web resolution. | + | better-sqlite3 | Feature persistence (parentId update) | Use (existing) | parent_id column already exists in features table. UPDATE SET clause already includes parent_id (per LESSONS.md verification). No schema migration needed. | + | lucide-react | Delete button icon on dependency edge | Use (existing) | X icon already available. Used throughout the codebase for iconography. | + | node:child_process | Git rebase operations | Use (existing) | Used by GitPrService via execFile wrapper for all git commands. No changes to execution pattern needed. | + + **No new library dependencies required.** All functionality is achievable with existing installed packages. + + ## Security Considerations + + ### Authorization + - Reparenting modifies feature relationships — the server action must validate that the caller has access to both the child and parent features. Currently, server actions run in the context of the local user (Electron app), so no additional auth is needed. If multi-user access is added later, reparent must check ownership of both features. + + ### Cycle Prevention + - Cycle detection is a security-critical validation. A cycle in the dependency graph would cause infinite loops in CheckAndUnblockFeaturesUseCase (it walks children recursively). The upward ancestor walk (from CreateFeatureUseCase) must be replicated exactly in ReparentFeatureUseCase. The walk MUST query the DB (not rely on client-provided data) to prevent race conditions. + + ### Git Operation Safety + - Auto-rebase modifies the git history of child branches. The stash/restore pattern (from RebaseFeatureOnMainUseCase) must be used to prevent data loss. Rebase abort on failure must always execute (finally block). The worktree isolation prevents cross-feature interference. + + ### Input Validation + - Server action must validate featureId and parentId are non-empty strings (or null for unparent). Must reject attempts to reparent to a non-existent parent. Must reject cross-repo reparenting to prevent nonsensical dependency relationships. + + ## Performance Implications + + ### Canvas Re-Layout + - Reparenting changes the edge set, which changes the dagre topology key, triggering a full layout recalculation. For typical graphs (10-50 nodes), this is sub-10ms. The optimistic UI pattern ensures the user sees the change within one animation frame (~16ms). + + ### Cycle Detection + - Ancestor walk is O(depth) where depth is the maximum nesting of feature dependencies. Typical depth is 1-3 levels. Each level requires a DB query (findById). For 3 levels, this is 3 sequential DB reads — negligible latency on SQLite. + + ### Auto-Rebase + - Git rebase operations are I/O-bound (file system + potential network for fetch). Typical clean rebase takes 1-5 seconds. Conflict resolution (agent-powered) can take 30-120 seconds. The 5-minute timeout (NFR-2) prevents runaway processes. Each child rebases independently, so N children rebase in parallel (bounded by system resources, not sequential). + + ### Poll Reconciliation + - The 3-second mutation cooldown (endMutation default) is sufficient for server persistence. The next poll after cooldown will fetch the updated parentId from the database, confirming the optimistic state. + + ## Architecture Notes + + ### Clean Architecture Compliance + - **Domain layer:** No changes. parentId field already exists on Feature entity (TypeSpec-generated). SdlcLifecycle enum values unchanged. + - **Application layer:** New ReparentFeatureUseCase. Modified CheckAndUnblockFeaturesUseCase (add rebase between transition and spawn). New rebaseOnBranch method in IGitPrService interface. + - **Infrastructure layer:** Implement rebaseOnBranch in GitPrService (follows rebaseOnMain pattern). Register ReparentFeatureUseCase in DI container with string alias. + - **Presentation layer:** Enable connections on FeaturesCanvas. Implement handleConnect in use-control-center-state.ts. Add reparentFeature mutation to useGraphState. Enhance DependencyEdge with delete button. New reparent-feature server action. + + ### Data Flow for Reparenting + 1. User drags from source handle on child to target handle on parent + 2. React Flow fires onConnect with Connection {source, target} + 3. handleConnect validates locally (non-feature nodes, cross-repo, lifecycle) + 4. On pass: optimistic UI update (set parentNodeId in featureMap) + 5. Call reparent-feature server action + 6. Server action resolves ReparentFeatureUseCase via DI + 7. Use case validates (cycle detection, same-repo, lifecycle guards) + 8. Use case updates feature.parentId via IFeatureRepository.update() + 9. Use case adjusts child lifecycle if needed (FR-8) + 10. Use case calls CheckAndUnblockFeaturesUseCase if new parent is post-implementation + 11. On server error: rollback optimistic UI, show toast.error + + ### Data Flow for Auto-Rebase + 1. Parent feature lifecycle transitions (via UpdateFeatureLifecycleUseCase) + 2. CheckAndUnblockFeaturesUseCase fires (existing hook) + 3. Gate check: parent in POST_IMPLEMENTATION (Implementation, Review, Maintain) + 4. Load direct children via findByParentId + 5. For each Blocked child: + a. Set lifecycle=Started, update DB + b. Create agent run + phase timing for rebase + c. Resolve CWD (worktree or repo root) + d. Stash uncommitted changes + e. Fetch parent branch (git fetch origin ) + f. Rebase child branch onto origin/ + g. On conflict: delegate to ConflictResolutionService + h. Restore stash (finally block) + i. Complete timing (success or error) + j. Spawn agent (regardless of rebase outcome) + + ### Key Codebase Files Reference + | File | Current State | Required Change | + | ---- | ------------- | --------------- | + | use-control-center-state.ts:311 | handleConnect is no-op | Implement reparent logic with validation | + | features-canvas.tsx:161 | nodesConnectable={false} | Change to true | + | features-canvas.tsx:162 | elementsSelectable={false} | Replace with edgesSelectable={true} | + | feature-node.tsx:141 | isConnectable={false} on target handle | Change to true | + | feature-node.tsx:138 | showHandles conditional | Always render handles (remove conditional) | + | dependency-edge.tsx | Minimal BaseEdge wrapper | Add hover delete button via EdgeLabelRenderer | + | use-graph-state.ts | No reparent mutation | Add reparentFeature mutation (update parentNodeId) | + | derive-graph.ts:242 | showHandles = edges.length > 0 | Always set showHandles=true when nodesConnectable | + | check-and-unblock-features.use-case.ts:59 | Spawn immediately after lifecycle update | Insert rebase orchestration before spawn | + | git-pr-service.interface.ts | No rebaseOnBranch | Add rebaseOnBranch method | + | git-pr.service.ts | No rebaseOnBranch impl | Implement following rebaseOnMain pattern | + | container.ts:437-509 | No ReparentFeatureUseCase | Add class registration + string alias | + + ### Database Impact + - **No migration needed.** The parent_id column already exists in the features table and is included in the UPDATE SET clause of sqlite-feature.repository.ts. Setting parent_id to a new value or NULL is already supported by the existing SQL. + + ### TypeSpec Impact + - **No TypeSpec changes needed.** The parentId field already exists on the Feature entity in the generated output. SdlcLifecycle enum values are unchanged. + + --- + + _Research complete — proceed to planning phase_ diff --git a/specs/084-feature-dependency-rebase/spec.yaml b/specs/084-feature-dependency-rebase/spec.yaml new file mode 100644 index 000000000..3ef26e33f --- /dev/null +++ b/specs/084-feature-dependency-rebase/spec.yaml @@ -0,0 +1,131 @@ +name: "feature-dependency-rebase" +number: 84 +branch: "feat/084-feature-dependency-rebase" +oneLiner: "Interactive reparenting of features via canvas drag-and-drop, plus automatic rebase of child branches when parent features complete implementation" +summary: "Extends the existing feature dependency system (spec 041) with two capabilities: (1) interactive drag-and-drop on the React Flow canvas to reparent features — moving a feature under another feature as a dependency, and (2) automatic git rebase of child feature branches onto the parent feature branch when the parent completes its implementation cycle (reaches post-implementation lifecycle).\n" +phase: "Requirements" +sizeEstimate: "L" +relatedFeatures: + - "041-feature-dependencies" + - "080-smart-rebase" +technologies: + - "TypeScript" + - "React 19" + - "Next.js 15" + - "@xyflow/react (React Flow v12)" + - "dagre (graph layout)" + - "tsyringe (DI)" + - "better-sqlite3" + - "node:child_process (git operations)" + - "shadcn/ui" + - "TypeSpec" +relatedLinks: + - title: "React Flow onConnect and drag docs" + url: "https://reactflow.dev/api-reference/react-flow#connection-handling" + - title: "Spec 041 - Feature Dependencies" + url: "./specs/041-feature-dependencies/" +openQuestions: + - question: "How should the user initiate reparenting on the canvas?" + resolved: true + options: + - option: "Drag-to-connect via handles" + description: "User drags from a source handle on the child node to a target handle on the new parent node. Uses React Flow's native onConnect. Handles are already rendered on feature nodes. Natural graph editing UX, but requires enabling nodesConnectable which may allow unintended connections." + selected: true + - option: "Drag-and-drop node onto parent" + description: "User drags the entire child node and drops it onto a parent node. Uses onNodeDragStop + intersection detection. More tactile but requires custom hit-testing logic and conflicts with nodesDraggable=false (currently disabled for layout stability)." + selected: false + - option: "Context menu action" + description: "Right-click a feature node and select 'Set Parent' from a menu, then click the target parent. Two-step interaction, less discoverable, but avoids handle/drag complexity entirely." + selected: false + selectionRationale: "Drag-to-connect via handles is the natural React Flow idiom. The feature nodes already render source and target handles (currently with isConnectable=false). Enabling connections and handling onConnect is the lowest-effort, most consistent approach. Users familiar with any node-based editor (Figma, Unreal, etc.) will expect this interaction pattern." + answer: "Drag-to-connect via handles" + - question: "How should the user detach a feature from its parent (unparent)?" + resolved: true + options: + - option: "Delete the dependency edge" + description: "User clicks the dependency edge and presses Delete/Backspace, or clicks a delete button that appears on edge hover. Uses React Flow's onEdgesDelete. Consistent with graph editing conventions." + selected: true + - option: "Context menu on feature node" + description: "Right-click the child node and select 'Remove Parent'. Explicit action, but adds context menu complexity and is less discoverable." + selected: false + - option: "Both edge delete and context menu" + description: "Support both approaches for maximum discoverability. More code to maintain but covers different user mental models." + selected: false + selectionRationale: "Edge deletion is the standard graph-editing interaction for removing connections. The DependencyEdge component can show a delete button on hover. This keeps the interaction model consistent: create connections by dragging handles, remove them by deleting edges." + answer: "Delete the dependency edge" + - question: "When should auto-rebase of child branches trigger?" + resolved: true + options: + - option: "When parent enters post-implementation lifecycle" + description: "Trigger rebase when parent transitions to Implementation, Review, or Maintain (the POST_IMPLEMENTATION gate). This is when the parent's code is considered complete and children should incorporate it. Aligns with the existing CheckAndUnblockFeaturesUseCase trigger point." + selected: true + - option: "When parent merges to main" + description: "Trigger rebase only when parent's branch is merged into main. Most conservative — guarantees parent code is stable. But delays child unblocking significantly and children would just rebase onto main (already handled by spec 080)." + selected: false + - option: "On every parent lifecycle transition" + description: "Rebase children on every parent state change. Keeps children maximally up-to-date but causes excessive rebases and potential conflicts during active parent development." + selected: false + selectionRationale: "Post-implementation lifecycle (Implementation/Review/Maintain) is the natural trigger because it aligns with the existing unblock gate in CheckAndUnblockFeaturesUseCase. At this point the parent's implementation is complete, making it safe for children to incorporate. Earlier triggers risk rebasing onto unstable code; later triggers delay child work unnecessarily." + answer: "When parent enters post-implementation lifecycle" + - question: "Should auto-rebase run synchronously (blocking) or asynchronously (background)?" + resolved: true + options: + - option: "Asynchronous background rebase" + description: "Rebase runs in the background after unblock. The child feature transitions to Started immediately, and rebase happens before agent spawn. Non-blocking for the user performing the parent lifecycle transition. Failure is recorded in the feature's activity timeline." + selected: true + - option: "Synchronous blocking rebase" + description: "Rebase runs inline during the lifecycle transition. The transition does not complete until all child rebases finish. Simpler control flow but blocks the parent transition on potentially slow git operations across multiple children." + selected: false + selectionRationale: "Asynchronous rebase is the right choice because a parent lifecycle transition should not be blocked by child git operations. If a parent has 5 children, synchronous rebase would make the transition unacceptably slow. The existing RebaseFeatureOnMainUseCase already creates its own agent run and phase timing, so background execution with activity timeline reporting is the established pattern." + answer: "Asynchronous background rebase" + - question: "What happens when auto-rebase encounters conflicts?" + resolved: true + options: + - option: "Use existing conflict resolution service" + description: "Delegate to ConflictResolutionService (agent-powered). The rebase creates an agent run, and the conflict resolution agent attempts to resolve conflicts automatically. If it fails, the rebase is aborted and the failure is recorded. This is the existing pattern in RebaseFeatureOnMainUseCase." + selected: true + - option: "Abort and notify user" + description: "Immediately abort the rebase on conflict, record the failure, and notify the user to manually resolve. Simpler but leaves the child branch in a state that requires manual intervention." + selected: false + - option: "Queue for manual resolution" + description: "Pause the rebase, mark the feature as 'conflict', and present a resolution UI. Most user-friendly but requires significant new UI and state management." + selected: false + selectionRationale: "The existing ConflictResolutionService with agent-powered resolution is already built and battle-tested in RebaseFeatureOnMainUseCase. Reusing it maintains consistency and handles most conflicts automatically. Failures are recorded in the activity timeline, giving the user visibility without requiring new UI." + answer: "Use existing conflict resolution service" + - question: "Should reparenting be allowed when the child feature has active children of its own?" + resolved: true + options: + - option: "Allow reparenting with subtree" + description: "Moving a feature moves its entire subtree. Children of the reparented feature keep their parent relationship intact. The subtree simply gets a new grandparent. No cascading rebase needed — children already depend on their direct parent, not the grandparent." + selected: true + - option: "Block reparenting if has children" + description: "Prevent reparenting features that have children to avoid complex cascading effects. User must first detach all children. Safest but most restrictive." + selected: false + selectionRationale: "Allowing subtree reparenting is correct because parent-child relationships are direct, not transitive. If feature C depends on B depends on A, moving B under D only changes B's parent — C still depends on B. The dagre layout will correctly reflow the entire subtree. Blocking reparenting for features with children is unnecessarily restrictive." + answer: "Allow reparenting with subtree" + - question: "Should reparenting trigger an immediate rebase of the reparented child onto the new parent branch?" + resolved: true + options: + - option: "No immediate rebase on reparent" + description: "Reparenting only changes the dependency relationship. Rebase happens later when the new parent reaches post-implementation. This is simpler and avoids surprising git operations when the user is just reorganizing dependencies." + selected: false + - option: "Immediate rebase on reparent" + description: "When a feature is reparented, immediately rebase it onto the new parent's branch if the new parent is in post-implementation. Ensures the child branch is always consistent with its parent. But may surprise the user with unexpected rebases." + selected: true + selectionRationale: "Reparenting is a dependency reorganization action, not a git operation. Users may reparent features while planning or reorganizing, and triggering rebases immediately could be disruptive. The auto-rebase will naturally fire when the new parent reaches post-implementation. If the new parent is already post-implementation and the child is Blocked, the existing CheckAndUnblock flow will handle the transition. A separate manual rebase can be triggered if needed." + answer: "Immediate rebase on reparent" + - question: "What lifecycle guards should apply to reparenting?" + resolved: true + options: + - option: "Block reparenting for completed features only" + description: "Allow reparenting for features in any lifecycle state except Done/Archived. Features that are actively being worked on, blocked, or pending can all be reparented. The child's lifecycle state adjusts if needed (e.g., becomes Blocked if new parent is pre-implementation)." + selected: true + - option: "Block reparenting for in-progress features" + description: "Only allow reparenting for Pending or Blocked features. Features with active agents cannot be reparented. Prevents disruption to active work but severely limits usability." + selected: false + - option: "No lifecycle guards" + description: "Allow reparenting regardless of lifecycle state. Maximum flexibility but could lead to confusing states (e.g., a Done feature suddenly becoming Blocked)." + selected: false + selectionRationale: "Blocking only completed features strikes the right balance. Users need to reorganize dependencies during active development. The child's lifecycle should adjust to reflect its new parent's state — if reparented under a pre-implementation parent, the child becomes Blocked. This is consistent with how CreateFeatureUseCase handles initial parent assignment." + answer: "Block reparenting for completed features only" +content: "## Problem Statement\n\nThe existing feature dependency system (spec 041) allows setting a parent feature at creation\ntime via `--parent ` (CLI) or the parent selector in the create drawer (Web). However,\nonce created, a feature's parent cannot be changed. Users need the ability to:\n\n1. **Reparent features interactively** — drag a feature node on the canvas to set or change its\n parent dependency, without recreating the feature.\n2. **Auto-rebase child branches** — when a parent feature completes its implementation cycle\n (reaches post-implementation lifecycle), automatically rebase all child feature branches\n so they incorporate the parent's changes.\n\nCurrently, the canvas is read-only (`nodesDraggable=false`, `nodesConnectable=false`). The\n`CheckAndUnblockFeaturesUseCase` transitions blocked children to Started and spawns agents,\nbut does not perform any git rebase. The `RebaseFeatureOnMainUseCase` rebases onto main but\nhas no concept of parent feature branches.\n\n## Success Criteria\n\n- [ ] User can drag from a source handle on one feature node to a target handle on another to set/change its parent dependency\n- [ ] Reparenting validates: same repository, no cycles, and lifecycle guards (cannot reparent Done/Archived features)\n- [ ] Reparenting updates the child's lifecycle state to Blocked if the new parent is pre-implementation\n- [ ] Reparenting is reflected immediately in the canvas layout via optimistic UI and persisted to the database\n- [ ] User can delete a dependency edge (click + Delete key or hover delete button) to detach a feature from its parent\n- [ ] When a parent feature reaches post-implementation lifecycle, all child feature branches are automatically rebased onto the parent branch\n- [ ] Auto-rebase runs asynchronously and does not block the parent's lifecycle transition\n- [ ] Auto-rebase uses the existing smart rebase infrastructure (stash, conflict resolution via ConflictResolutionService)\n- [ ] Auto-rebase failures are recorded in the child feature's activity timeline and surfaced in the UI\n- [ ] The canvas re-layouts correctly after reparenting (dagre recalculates positions for the affected subtree)\n- [ ] Feature tree table reflects reparenting changes in real-time\n- [ ] Reparenting a feature with children moves the entire subtree (children keep their direct parent relationship)\n- [ ] All new use cases have string token aliases registered in the DI container for web server action access\n- [ ] All new web components have colocated Storybook stories\n- [ ] TDD: failing tests written first for use cases, server actions, and graph derivation changes\n\n## Functional Requirements\n\n### Canvas Reparenting (FR-1 through FR-8)\n\n- **FR-1: Enable drag-to-connect on feature nodes.** Set `nodesConnectable={true}` on the React Flow canvas and `isConnectable={true}` on feature node target handles. Source handles on feature nodes must also be connectable. Only feature-to-feature connections are valid — connections to/from repo group nodes must be rejected.\n\n- **FR-2: Handle onConnect to trigger reparent.** When the user completes a drag-to-connect gesture, the `onConnect` callback receives a `Connection` object with `source` (new parent node ID) and `target` (child node ID). Validate the connection and call the `reparent-feature` server action. On validation failure, show a toast with the specific error (cycle detected, cross-repo, lifecycle blocked).\n\n- **FR-3: Optimistic UI for reparenting.** On successful local validation, immediately update `featureMap` in `useGraphState` to set the child's `parentNodeId` to the new parent. Use `beginMutation`/`endMutation` guards to prevent poll data from overwriting the optimistic state. The canvas should re-layout via the existing topology-key cache mechanism within the same animation frame.\n\n- **FR-4: Edge deletion for unparenting.** The DependencyEdge component must support deletion. On hover, render a delete button (X icon) on the edge. Support keyboard deletion (select edge + Delete/Backspace). When deleted, call the `reparent-feature` server action with `parentId: null` to clear the parent. Apply optimistic UI removal of the edge.\n\n- **FR-5: Validation — same repository.** Reject reparenting if the child and new parent belong to different repositories. Display error: \"Features must be in the same repository to form a dependency.\"\n\n- **FR-6: Validation — cycle detection.** Walk the ancestor chain from the proposed new parent upward. If the child feature is found in the chain, reject with error: \"Cannot create a dependency cycle.\" This reuses the cycle detection logic from CreateFeatureUseCase.\n\n- **FR-7: Validation — lifecycle guards.** Reject reparenting if the child feature is in Done or Archived lifecycle state. Display error: \"Completed features cannot be reparented.\"\n\n- **FR-8: Lifecycle state adjustment on reparent.** When a feature is reparented under a new parent that is NOT in post-implementation lifecycle (Implementation, Review, Maintain), and the child is currently in a Started or later active state, transition the child to Blocked. When reparented under a parent that IS in post-implementation, the child's lifecycle state remains unchanged. When unparented (parent set to null), if the child was Blocked, transition it to Started.\n\n### Auto-Rebase on Parent Completion (FR-9 through FR-14)\n\n- **FR-9: Trigger auto-rebase in CheckAndUnblockFeaturesUseCase.** When a parent feature enters post-implementation lifecycle and has Blocked children, after transitioning each child to Started, rebase the child's feature branch onto the parent's feature branch before spawning the child's agent.\n\n- **FR-10: Rebase onto parent branch.** Add a `rebaseOnBranch(cwd, featureBranch, targetBranch)` method to `IGitPrService` interface and implement it in the git PR service. This method fetches the target branch, then rebases the feature branch onto it. The implementation is similar to `rebaseOnMain` but targets an arbitrary branch instead of `origin/`.\n\n- **FR-11: Smart stash before rebase.** Before rebasing, stash any uncommitted changes in the child's working directory (worktree or repo root). Restore the stash after rebase completes, regardless of success or failure. Reuse the existing `stash()`/`stashPop()` methods.\n\n- **FR-12: Agent-powered conflict resolution.** When rebase encounters conflicts, delegate to the existing `ConflictResolutionService`. Create an agent run for the conflict resolution attempt. If resolution fails, abort the rebase and record the failure.\n\n- **FR-13: Activity timeline recording.** Create an agent run and phase timing record for each auto-rebase operation. Record success with a summary of rebased commits, or failure with the error details. The activity timeline in the web UI will display these records without additional UI work.\n\n- **FR-14: Failure isolation.** If auto-rebase fails for one child, it must not prevent other children from being unblocked and rebased. Each child's rebase is independent. Failures are recorded per-child.\n\n### Server Action and Use Case (FR-15 through FR-17)\n\n- **FR-15: ReparentFeatureUseCase.** New use case in `packages/core/src/application/use-cases/features/` that accepts `{ featureId: string, parentId: string | null }`. Performs all validations (FR-5, FR-6, FR-7), updates the feature's `parentId` via `IFeatureRepository.update()`, adjusts lifecycle state (FR-8), and calls `CheckAndUnblockFeaturesUseCase` if the reparented feature is now a parent of Blocked children.\n\n- **FR-16: reparent-feature server action.** New server action at `src/presentation/web/app/actions/reparent-feature.ts` following the established pattern. Resolves `ReparentFeatureUseCase` via string token from DI container. Accepts `{ featureId: string, parentId: string | null }`. Returns `{ success: boolean, error?: string }`.\n\n- **FR-17: DI container registration.** Register `ReparentFeatureUseCase` as a class token and add a string token alias `'ReparentFeatureUseCase'` in the DI container for web server action resolution.\n\n## Non-Functional Requirements\n\n- **NFR-1: Optimistic UI latency.** The canvas must reflect reparenting within 100ms of the user completing the drag gesture. Server persistence is eventual but must not cause visible flicker or layout jumps when the poll reconciles.\n\n- **NFR-2: Rebase timeout.** Auto-rebase operations must have a configurable timeout (default 5 minutes per child). If a rebase exceeds the timeout, abort and record the timeout as a failure.\n\n- **NFR-3: Concurrent rebase safety.** If a child feature already has an active agent run, skip the auto-rebase for that child and log a warning. Do not interrupt running agents with unexpected rebases.\n\n- **NFR-4: Connection validation feedback.** Invalid drag-to-connect attempts (cross-repo, cycle, lifecycle) must show a toast notification within 200ms of the drop. The toast must contain a specific, actionable error message — not a generic \"operation failed.\"\n\n- **NFR-5: Backward compatibility.** Features created without a parent (the majority) must be unaffected. The canvas must function identically when no dependency edges exist. No database migration required — `parent_id` column and UPDATE clause already exist.\n\n- **NFR-6: Accessibility.** Edge deletion must be keyboard-accessible (select edge via Tab, delete via Delete/Backspace). The drag-to-connect gesture must have an alternative interaction path for keyboard-only users (future consideration — document as known limitation if not implemented in this iteration).\n\n- **NFR-7: Test coverage.** Unit tests for ReparentFeatureUseCase covering: successful reparent, cycle detection, cross-repo rejection, lifecycle guard, unparent, and lifecycle state adjustment. Integration tests for the auto-rebase flow in CheckAndUnblockFeaturesUseCase. Unit tests for deriveGraph edge changes after reparent.\n\n## Product Questions & AI Recommendations\n\n| # | Question | AI Recommendation | Rationale |\n| - | -------- | ----------------- | --------- |\n| 1 | How should the user initiate reparenting? | Drag-to-connect via handles | Native React Flow idiom; handles already rendered; familiar UX from node-based editors |\n| 2 | How should the user unparent a feature? | Delete the dependency edge | Consistent graph-editing interaction; edge hover delete button + keyboard shortcut |\n| 3 | When should auto-rebase trigger? | When parent enters post-implementation lifecycle | Aligns with existing CheckAndUnblock gate; parent code is stable at this point |\n| 4 | Should auto-rebase be sync or async? | Asynchronous background | Parent transition should not block on child git operations |\n| 5 | How to handle rebase conflicts? | Use existing ConflictResolutionService | Already battle-tested in RebaseFeatureOnMainUseCase; agent-powered resolution |\n| 6 | Allow reparenting features with children? | Yes, move entire subtree | Parent-child relationships are direct, not transitive; dagre handles re-layout |\n| 7 | Trigger immediate rebase on reparent? | No, wait for parent post-implementation | Reparenting is dependency reorganization, not a git operation |\n| 8 | What lifecycle guards for reparenting? | Block only Done/Archived features | Balance between flexibility and safety; child lifecycle adjusts to new parent |\n\n## Affected Areas\n\n| Area | Impact | Reasoning |\n| ---- | ------ | --------- |\n| `packages/core/src/application/use-cases/features/` | High | New `ReparentFeatureUseCase` for updating parentId with validation (cycle detection, same-repo check, lifecycle guards, lifecycle state adjustment). |\n| `packages/core/src/application/use-cases/features/check-and-unblock-features.use-case.ts` | High | Must trigger auto-rebase of child branches onto parent branch (not just lifecycle transition and agent spawn) when parent reaches post-implementation. |\n| `src/presentation/web/components/features/features-canvas/features-canvas.tsx` | High | Enable `nodesConnectable={true}`, implement `onConnect` handler for reparenting, handle `onEdgesDelete` for unparenting. |\n| `src/presentation/web/hooks/use-graph-state.ts` | High | New `reparentFeature()` mutation to update `parentNodeId` in featureMap optimistically. |\n| `src/presentation/web/components/features/features-canvas/dependency-edge.tsx` | Medium | Add hover delete button and selection support for edge deletion. |\n| `src/presentation/web/app/actions/` | Medium | New `reparent-feature.ts` server action calling `ReparentFeatureUseCase`. |\n| `packages/core/src/infrastructure/di/container.ts` | Medium | Register `ReparentFeatureUseCase` class token + string alias for web server actions. |\n| `packages/core/src/application/ports/output/services/git-pr-service.interface.ts` | Medium | Add `rebaseOnBranch(cwd, featureBranch, targetBranch)` method to interface. |\n| `packages/core/src/infrastructure/services/git/` | Medium | Implement `rebaseOnBranch` in git PR service. |\n| `src/presentation/web/lib/derive-graph.ts` | Low | May need minor adjustments for connectable handle configuration. Already derives dependency edges correctly from parentNodeId. |\n| `src/presentation/web/components/common/feature-node/` | Low | Set `isConnectable={true}` on target and source handles. Minor styling for connectable state. |\n| `src/presentation/web/components/features/feature-tree-table/` | Low | No changes expected — already reads parentId from data and will reflect reparenting via data refresh. |\n| `packages/core/src/infrastructure/repositories/sqlite-feature.repository.ts` | Low | No changes needed — parent_id is already in the UPDATE SET clause. |\n| `tsp/` | Low | No TypeSpec changes expected — parentId field already exists on Feature entity. |\n\n## Dependencies\n\n### Existing Code Dependencies\n- **Spec 041 (Feature Dependencies)** — Foundation: parentId field, Blocked lifecycle, gate logic,\n CheckAndUnblockFeaturesUseCase, dependency edges in canvas. This spec 084 extends spec 041.\n- **Spec 080 (Smart Rebase)** — RebaseFeatureOnMainUseCase with stash/conflict resolution.\n Auto-rebase will reuse this infrastructure.\n- **React Flow onConnect** — The canvas already accepts `onConnect` prop; enabling connections\n requires setting `nodesConnectable={true}` and implementing the handler.\n- **dagre layout** — Topology changes from reparenting trigger automatic re-layout via the\n existing topology-key cache mechanism.\n\n### Library Dependencies\n- No new library dependencies expected. All needed functionality exists in `@xyflow/react`,\n `better-sqlite3`, and `node:child_process`.\n\n## Size Estimate\n\n**L** — This feature spans all four architecture layers (domain, application, infrastructure,\npresentation) and requires two distinct capabilities: (1) interactive canvas reparenting with\noptimistic UI, validation, and cycle detection, and (2) automatic git rebase orchestration\ntriggered by lifecycle transitions. The rebase orchestration requires careful handling of\nworktrees, stash, conflict resolution, and failure reporting. The canvas interaction requires\nenabling React Flow connections, handling edge creation/deletion, and maintaining optimistic\nstate.\n\n---\n\n_Requirements defined — proceed with research_\n" diff --git a/specs/084-feature-dependency-rebase/tasks.yaml b/specs/084-feature-dependency-rebase/tasks.yaml new file mode 100644 index 000000000..806df3f02 --- /dev/null +++ b/specs/084-feature-dependency-rebase/tasks.yaml @@ -0,0 +1,548 @@ +# Task Breakdown (YAML) +# This is the source of truth. Markdown is auto-generated from this file. + +name: "feature-dependency-rebase" +summary: > + 14 tasks across 4 phases. Phase 1 builds the ReparentFeatureUseCase with validation, + server action, and DI wiring. Phase 2 adds rebaseOnBranch to the git service. Phase 3 + wires auto-rebase into CheckAndUnblockFeaturesUseCase. Phase 4 implements canvas + interaction with drag-to-connect, edge deletion, and optimistic UI. + +relatedFeatures: + - "041-feature-dependencies" + - "080-smart-rebase" + +technologies: + - "TypeScript" + - "React 19" + - "Next.js 15" + - "@xyflow/react (React Flow v12)" + - "tsyringe (DI)" + - "better-sqlite3" + - "node:child_process" + - "shadcn/ui" + - "sonner" + - "lucide-react" + +relatedLinks: + - title: "React Flow Connection Handling API" + url: "https://reactflow.dev/api-reference/react-flow#connection-handling" + - title: "Spec 041 - Feature Dependencies" + url: "./specs/041-feature-dependencies/" + - title: "Spec 080 - Smart Rebase" + url: "./specs/080-smart-rebase/" + +tasks: + - id: "task-1" + phaseId: "phase-1" + title: "ReparentFeatureUseCase — core validation logic" + description: > + Create the ReparentFeatureUseCase class with constructor DI injection for + IFeatureRepository and ICheckAndUnblockFeaturesUseCase. Implement the execute method + with all business validations: same-repo check (FR-5), cycle detection via upward + ancestor walk (FR-6), lifecycle guards rejecting Done/Archived (FR-7). The cycle + detection reuses the pattern from CreateFeatureUseCase lines 106-116 — walk parent + chain via findById until null or cycle found. + state: "Todo" + dependencies: [] + acceptanceCriteria: + - "ReparentFeatureUseCase class exists with @injectable() decorator" + - "execute({featureId, parentId}) validates same-repo constraint" + - "execute() detects cycles via upward ancestor walk (self-reparent, direct A->B->A, indirect A->B->C->A)" + - "execute() rejects reparenting Done or Archived features" + - "execute() rejects reparenting to non-existent parent" + - "execute() rejects reparenting to feature in different repository" + - "All validation errors throw descriptive error messages" + tdd: + red: + - "Write test: successful reparent updates parentId via repository.update()" + - "Write test: reject self-reparent (featureId === parentId)" + - "Write test: reject direct cycle (A parent of B, try to reparent A under B)" + - "Write test: reject indirect cycle (A->B->C chain, try to reparent A under C)" + - "Write test: reject cross-repo reparent (child.repositoryPath !== parent.repositoryPath)" + - "Write test: reject reparent of Done feature" + - "Write test: reject reparent of Archived feature" + - "Write test: reject reparent to non-existent parent (findById returns null)" + - "Write test: successful unparent (parentId=null) clears parentId" + green: + - "Implement execute() with validation chain: load child, validate lifecycle, load parent (if not null), validate same-repo, detect cycles, call repository.update()" + - "Implement cycle detection: walk from proposed parent upward via findById until null (safe) or child found (cycle)" + refactor: + - "Extract cycle detection into private method for clarity" + - "Extract validation methods if execute() exceeds 30 lines of logic" + estimatedEffort: "2h" + + - id: "task-2" + phaseId: "phase-1" + title: "ReparentFeatureUseCase — lifecycle state adjustment" + description: > + Extend the use case to handle lifecycle state adjustment on reparent (FR-8). When + reparented under a pre-implementation parent, transition child to Blocked. When + reparented under a post-implementation parent, leave child lifecycle unchanged. + When unparented, if child was Blocked, transition to Started. After reparent, + if new parent is post-implementation, call CheckAndUnblockFeaturesUseCase to + trigger the rebase+unblock flow for any Blocked children of the reparented feature. + state: "Todo" + dependencies: + - "task-1" + acceptanceCriteria: + - "Child transitions to Blocked when reparented under pre-implementation parent" + - "Child lifecycle unchanged when reparented under post-implementation parent" + - "Child transitions from Blocked to Started when unparented" + - "Child lifecycle unchanged when unparented from non-Blocked state" + - "CheckAndUnblockFeaturesUseCase called with new parentId after reparent if parent is post-implementation" + - "Lifecycle adjustment happens after parentId update, before CheckAndUnblock call" + tdd: + red: + - "Write test: reparent Started child under Pending parent -> child becomes Blocked" + - "Write test: reparent Started child under Implementation parent -> child stays Started" + - "Write test: reparent Blocked child under null (unparent) -> child becomes Started" + - "Write test: reparent Pending child under null (unparent) -> child stays Pending" + - "Write test: reparent under post-implementation parent -> CheckAndUnblockFeaturesUseCase.execute() called" + - "Write test: reparent under pre-implementation parent -> CheckAndUnblockFeaturesUseCase.execute() NOT called" + green: + - "Add POST_IMPLEMENTATION lifecycle set check (Implementation, Review, Maintain)" + - "Implement lifecycle adjustment: check new parent lifecycle, adjust child if needed, update via repository" + - "Call CheckAndUnblockFeaturesUseCase.execute(newParentId) when parent is post-implementation" + refactor: + - "Use the existing POST_IMPLEMENTATION constant from check-and-unblock-features.use-case.ts" + - "Ensure lifecycle adjustment and CheckAndUnblock call are in correct order" + estimatedEffort: "1.5h" + + - id: "task-3" + phaseId: "phase-1" + title: "Server action + DI container registration" + description: > + Create the reparent-feature server action following the established pattern + (start-feature.ts as reference). Register ReparentFeatureUseCase in the DI + container with both class token and string token alias 'ReparentFeatureUseCase' + (per LESSONS.md pattern). The server action resolves the use case via string token, + calls execute(), and returns {success, error?}. + state: "Todo" + dependencies: + - "task-2" + acceptanceCriteria: + - "Server action at src/presentation/web/app/actions/reparent-feature.ts" + - "Action accepts {featureId: string, parentId: string | null}" + - "Action returns {success: boolean, error?: string}" + - "Action resolves ReparentFeatureUseCase via string token 'ReparentFeatureUseCase'" + - "DI container has class token registration for ReparentFeatureUseCase" + - "DI container has string alias 'ReparentFeatureUseCase' via useFactory pattern" + - "Action validates non-empty featureId before calling use case" + tdd: + red: + - "Write test: server action returns {success: true} on successful reparent" + - "Write test: server action returns {success: false, error: message} on validation failure" + - "Write test: server action returns {success: false, error} for empty featureId" + green: + - "Create reparent-feature.ts with 'use server' directive" + - "Implement resolve -> execute -> catch pattern matching start-feature.ts" + - "Add ReparentFeatureUseCase import to container.ts" + - "Add registerSingleton for class token" + - "Add string alias factory registration in the web string tokens block" + refactor: + - "Ensure error messages from use case propagate cleanly to the action response" + estimatedEffort: "45min" + + - id: "task-4" + phaseId: "phase-2" + title: "Add rebaseOnBranch to IGitPrService interface" + description: > + Add the rebaseOnBranch(cwd, featureBranch, targetBranch) method signature to the + IGitPrService interface. This method rebases a feature branch onto an arbitrary + target branch (not just the default branch). The signature mirrors rebaseOnMain + but the third parameter is the target branch name (not the base/default branch). + state: "Todo" + dependencies: [] + acceptanceCriteria: + - "IGitPrService interface has rebaseOnBranch(cwd: string, featureBranch: string, targetBranch: string): Promise" + - "Method is documented with JSDoc explaining it targets origin/" + - "Method throws REBASE_CONFLICT error code on conflict (same as rebaseOnMain)" + tdd: + red: + - "Write test: mock IGitPrService with rebaseOnBranch method, verify it can be called with correct signature" + green: + - "Add method signature to IGitPrService interface with JSDoc" + refactor: + - "Verify JSDoc is consistent with rebaseOnMain documentation style" + estimatedEffort: "15min" + + - id: "task-5" + phaseId: "phase-2" + title: "Implement rebaseOnBranch in GitPrService" + description: > + Implement the rebaseOnBranch method in GitPrService following the rebaseOnMain + pattern (git-pr.service.ts). Key differences: (1) explicit git fetch origin + before rebase since the target branch may not be locally available, + (2) rebase onto origin/ instead of origin/. Same conflict + detection pattern — check stderr for CONFLICT/could not apply, throw REBASE_CONFLICT. + state: "Todo" + dependencies: + - "task-4" + acceptanceCriteria: + - "GitPrService.rebaseOnBranch() fetches the target branch before rebasing" + - "Rebases feature branch onto origin/" + - "Throws GitPrError with REBASE_CONFLICT code on conflict" + - "Handles fetch failure gracefully (throws descriptive error)" + - "Uses same execFile wrapper pattern as rebaseOnMain" + tdd: + red: + - "Write test: successful rebase calls git fetch + git rebase in correct order" + - "Write test: rebase conflict detected from stderr, throws REBASE_CONFLICT" + - "Write test: fetch failure throws descriptive error" + - "Write test: non-zero exit code without conflict pattern throws generic rebase error" + green: + - "Implement rebaseOnBranch: git fetch origin , then git rebase origin/" + - "Parse stderr for conflict indicators (same pattern as rebaseOnMain)" + - "Throw typed GitPrError with appropriate error codes" + refactor: + - "Extract shared conflict detection logic between rebaseOnMain and rebaseOnBranch if duplication exceeds 10 lines" + estimatedEffort: "1h" + + - id: "task-6" + phaseId: "phase-3" + title: "Inject rebase dependencies into CheckAndUnblockFeaturesUseCase" + description: > + Add new DI dependencies to CheckAndUnblockFeaturesUseCase constructor: IGitPrService, + IWorktreeService, ConflictResolutionService, IAgentRunRepository, IPhaseTimingRepository. + These follow the exact injection pattern from RebaseFeatureOnMainUseCase. No behavior + changes yet — just wiring the dependencies so the rebase orchestration (task-7) can + use them. + state: "Todo" + dependencies: + - "task-5" + acceptanceCriteria: + - "Constructor accepts 5 new @inject() parameters: IGitPrService, IWorktreeService, ConflictResolutionService, IAgentRunRepository, IPhaseTimingRepository" + - "Dependencies stored as private readonly fields" + - "Existing tests still pass (no behavior change)" + - "Injection tokens match the patterns used in RebaseFeatureOnMainUseCase" + tdd: + red: + - "Write test: verify CheckAndUnblockFeaturesUseCase can be instantiated with all required dependencies (update test fixtures to provide new mocks)" + green: + - "Add @inject() decorators and constructor parameters for the 5 new dependencies" + - "Update existing test fixtures to provide mock implementations" + refactor: + - "Ensure constructor parameter order follows convention (repos first, services second)" + estimatedEffort: "30min" + + - id: "task-7" + phaseId: "phase-3" + title: "Implement auto-rebase orchestration in CheckAndUnblockFeaturesUseCase" + description: > + Add rebase orchestration between the lifecycle transition (child -> Started) and + agent spawn in CheckAndUnblockFeaturesUseCase. For each Blocked child: (1) create + agent run + phase timing for activity timeline, (2) resolve CWD (worktree or repo + root), (3) stash uncommitted changes, (4) fetch and rebase child branch onto parent + branch via rebaseOnBranch, (5) on conflict delegate to ConflictResolutionService, + (6) restore stash in finally block, (7) record timing (success or failure), + (8) spawn agent regardless of rebase outcome. Children process in parallel but + each child's rebase awaits before its agent spawn. 5-minute timeout per child. + state: "Todo" + dependencies: + - "task-6" + acceptanceCriteria: + - "Each Blocked child is rebased onto parent branch before agent spawn" + - "Agent run + phase timing created for each rebase operation" + - "Stash applied before rebase, restored in finally block" + - "Conflict resolution delegated to ConflictResolutionService" + - "Rebase failure recorded in phase timing but does not prevent agent spawn" + - "One child's rebase failure does not block other children" + - "5-minute timeout per child rebase (abort + record timeout failure)" + - "Skip rebase if child has active agent run (NFR-3)" + - "CWD resolved via worktree service (worktree path if exists, else repo root)" + tdd: + red: + - "Write test: successful rebase — stash called, rebaseOnBranch called with parent branch, stashPop called, agent spawned" + - "Write test: rebase conflict — ConflictResolutionService.resolve() called, agent still spawned" + - "Write test: rebase failure — error recorded in phase timing, agent still spawned" + - "Write test: one child fails rebase, other child succeeds — both agents spawned" + - "Write test: child with active agent run — rebase skipped, no agent spawn" + - "Write test: agent run and phase timing created for rebase operation" + - "Write test: stashPop called in finally block even when rebase throws" + - "Write test: parent branch name used as target for rebaseOnBranch" + green: + - "Add rebaseChildOntoParent private method following RebaseFeatureOnMainUseCase pattern" + - "Create agent run with 'rebase-on-parent' description" + - "Resolve CWD via worktreeService (check existence, use worktree path or repo root)" + - "Stash, rebase, handle conflicts, restore stash in finally" + - "Record phase timing with duration, exit code, error" + - "Insert rebaseChildOntoParent call between lifecycle update and agent spawn in execute()" + - "Add Promise.race with 5-minute timeout per child" + - "Skip rebase if feature has activeAgentRunId (NFR-3)" + refactor: + - "Extract timeout wrapper into helper if used elsewhere" + - "Ensure error messages are descriptive for activity timeline display" + estimatedEffort: "3h" + + - id: "task-8" + phaseId: "phase-4" + title: "Add reparentFeature mutation to useGraphState" + description: > + Add a reparentFeature(childId, newParentId) mutation to the useGraphState hook. + This mutation optimistically updates parentNodeId in featureMap for the child + feature, uses beginMutation/endMutation guards, calls the reparent-feature server + action, and rolls back on error. The parentNodeId change triggers automatic edge + re-derivation via deriveGraph() and dagre re-layout via topology-key cache. + state: "Todo" + dependencies: + - "task-3" + acceptanceCriteria: + - "reparentFeature(childId, newParentId) function exposed from useGraphState" + - "Optimistically updates child's parentNodeId in featureMap" + - "Uses beginMutation() before update and endMutation() after server response" + - "Calls reparent-feature server action" + - "Rolls back parentNodeId on server error and shows toast.error" + - "Shows toast.success on successful reparent" + - "parentNodeId=undefined for unparent (newParentId=null)" + tdd: + red: + - "Write test: reparentFeature updates parentNodeId in featureMap immediately" + - "Write test: reparentFeature calls beginMutation before update" + - "Write test: reparentFeature calls endMutation after server response" + - "Write test: server error rolls back parentNodeId to previous value" + - "Write test: unparent sets parentNodeId to undefined" + green: + - "Add reparentFeature function inside useGraphState hook" + - "Save previous parentNodeId for rollback" + - "Update featureMap entry with new parentNodeId" + - "Call server action, handle success/error" + - "Rollback on error, toast on both outcomes" + refactor: + - "Ensure mutation pattern matches existing patterns (deleteFeature, startFeature)" + estimatedEffort: "1h" + + - id: "task-9" + phaseId: "phase-4" + title: "Implement handleConnect for reparenting in use-control-center-state" + description: > + Replace the no-op handleConnect (line 311) in use-control-center-state.ts with + client-side validation and reparentFeature call. The onConnect callback receives a + Connection with source (new parent node ID) and target (child node ID). Validate: + reject non-feature nodes (check 'feat-' prefix), reject self-connection, reject + cross-repo (compare repositoryPath), reject Done/Archived lifecycle. On pass, call + reparentFeature from useGraphState. On validation failure, show toast.error with + specific message (NFR-4). + state: "Todo" + dependencies: + - "task-8" + acceptanceCriteria: + - "handleConnect processes Connection with source (parent) and target (child)" + - "Rejects non-feature-to-feature connections (repo group nodes)" + - "Rejects self-connections" + - "Rejects cross-repo connections with specific error toast" + - "Rejects connections to Done/Archived features with specific error toast" + - "Calls reparentFeature(childId, parentId) on valid connection" + - "Toast errors appear within 200ms of invalid drop (NFR-4)" + tdd: + red: + - "Write test: valid feature-to-feature connection calls reparentFeature" + - "Write test: non-feature source node shows toast error and does not call reparentFeature" + - "Write test: self-connection shows toast error" + - "Write test: cross-repo connection shows 'must be in same repository' toast" + - "Write test: Done lifecycle shows 'completed features cannot be reparented' toast" + green: + - "Replace no-op handleConnect with validation chain" + - "Extract featureId from node ID (strip 'feat-' prefix or similar)" + - "Look up child and parent in featureMap for validation" + - "Call reparentFeature on success, toast.error on failure" + refactor: + - "Extract validation into a pure function for testability" + - "Ensure error messages match spec FR-5, FR-6, FR-7 wording" + estimatedEffort: "1h" + + - id: "task-10" + phaseId: "phase-4" + title: "Implement handleEdgesDelete for unparenting in use-control-center-state" + description: > + Add onEdgesDelete handler in use-control-center-state.ts. When a dependency edge + is deleted, extract the child feature ID from the edge target, and call + reparentFeature(childId, null) to clear the parent. Only process dependency edges + (filter by edge type). Wire the handler through to features-canvas.tsx. + state: "Todo" + dependencies: + - "task-8" + acceptanceCriteria: + - "onEdgesDelete callback defined in use-control-center-state" + - "Filters to dependency edges only (ignores repo-to-feature edges)" + - "Calls reparentFeature(childId, null) for each deleted dependency edge" + - "Handler passed through to FeaturesCanvas component" + tdd: + red: + - "Write test: deleting a dependency edge calls reparentFeature with null parentId" + - "Write test: deleting a non-dependency edge does not call reparentFeature" + - "Write test: deleting multiple edges processes each independently" + green: + - "Add handleEdgesDelete callback that filters by edge type and calls reparentFeature" + - "Wire handler through control-center-inner to features-canvas" + refactor: + - "Ensure edge type identification is consistent with derive-graph edge type assignment" + estimatedEffort: "45min" + + - id: "task-11" + phaseId: "phase-4" + title: "Enable connections and edge selection on FeaturesCanvas" + description: > + Update FeaturesCanvas to enable connections and edge selection. Change + nodesConnectable={true} (was false), replace elementsSelectable={false} with + edgesSelectable={true}. Pass onEdgesDelete callback to ReactFlow. Add + isValidConnection prop to reject invalid connections at the React Flow level + (prevents the visual connection line from snapping to invalid targets). + state: "Todo" + dependencies: + - "task-9" + - "task-10" + acceptanceCriteria: + - "nodesConnectable={true} on ReactFlow component" + - "elementsSelectable removed, edgesSelectable={true} added" + - "onEdgesDelete callback passed to ReactFlow" + - "isValidConnection rejects non-feature-to-feature connections at visual level" + - "Node click still opens drawer (nodesSelectable behavior unchanged)" + - "Edge click selects edge (blue highlight for keyboard delete)" + tdd: + red: + - "Write test: FeaturesCanvas renders ReactFlow with nodesConnectable=true" + - "Write test: FeaturesCanvas passes onEdgesDelete to ReactFlow" + - "Write test: isValidConnection rejects connections to/from non-feature nodes" + green: + - "Update ReactFlow props in features-canvas.tsx" + - "Add isValidConnection callback (check node IDs for feature prefix)" + - "Pass onEdgesDelete from props" + refactor: + - "Verify no visual regressions with edge selection enabled" + estimatedEffort: "45min" + + - id: "task-12" + phaseId: "phase-4" + title: "Enhance DependencyEdge with hover delete button" + description: > + Add a hover-visible delete button to the DependencyEdge component using React Flow's + EdgeLabelRenderer. The button appears at the midpoint of the edge on hover, with an + X icon (lucide-react). Clicking the button triggers edge deletion (calls the + onEdgesDelete flow). The edge must also support selection state for keyboard delete. + Create colocated Storybook story. + state: "Todo" + dependencies: + - "task-11" + acceptanceCriteria: + - "Delete button (X icon) appears on dependency edge hover" + - "Button positioned at edge midpoint via EdgeLabelRenderer" + - "Clicking button deletes the edge (triggers onEdgesDelete)" + - "Edge supports selection state (visual indicator for keyboard delete)" + - "Colocated Storybook story at dependency-edge.stories.tsx" + - "Button is keyboard accessible (can be tabbed to and activated)" + tdd: + red: + - "Write test: DependencyEdge renders delete button on hover" + - "Write test: clicking delete button calls edge delete handler" + - "Write test: DependencyEdge renders without delete button when not hovered" + green: + - "Add state tracking for hover (onMouseEnter/onMouseLeave on edge group)" + - "Render X button via EdgeLabelRenderer at midpoint (labelX, labelY from getBezierPath)" + - "Button onClick calls React Flow's deleteElements or dispatches edge delete" + - "Add selected styling (e.g., stroke color change)" + refactor: + - "Extract button styling to match existing codebase design tokens" + - "Create Storybook story with various edge states (default, hovered, selected)" + estimatedEffort: "1.5h" + + - id: "task-13" + phaseId: "phase-4" + title: "Update feature node handles for connections" + description: > + Update feature-node.tsx to enable connections on handles. Set isConnectable={true} + on both target and source handles (was false). Always render handles (remove the + data.showHandles conditional for the target handle) but keep them visually subtle + (opacity transition on hover). Update derive-graph.ts to always set showHandles=true + when generating node data, since handles must always be present for connections to + work even when no edges exist yet. + state: "Todo" + dependencies: [] + acceptanceCriteria: + - "Target handle: isConnectable={true}, always rendered (not conditional on showHandles)" + - "Source handle: isConnectable={true}" + - "Handles visually subtle when not hovered (opacity-0 or low opacity)" + - "Handles become visible on node hover for connection affordance" + - "derive-graph.ts always includes showHandles=true in node data" + - "Backward compatible: features with no dependencies render correctly" + tdd: + red: + - "Write test: feature node always renders target handle regardless of edges" + - "Write test: feature node renders handles with isConnectable={true}" + - "Write test: deriveGraph sets showHandles=true on all feature nodes" + green: + - "Remove conditional rendering of target handle (was gated on data.showHandles)" + - "Set isConnectable={true} on both handles" + - "Update derive-graph to always set showHandles=true" + - "Add CSS transition for handle opacity on hover" + refactor: + - "Remove showHandles logic from derive-graph if no longer needed" + - "Simplify handle rendering code in feature-node" + estimatedEffort: "45min" + + - id: "task-14" + phaseId: "phase-4" + title: "End-to-end integration test for canvas reparenting flow" + description: > + Write an integration test that exercises the full reparenting flow: drag-to-connect + triggers onConnect -> client validation -> optimistic UI update -> server action -> + use case validation -> DB update -> lifecycle adjustment. Also test the unparent + flow via edge deletion. And test the derive-graph changes: verify that after + reparenting, the edge set reflects the new parent-child relationship and the old + repo-to-child edge is removed. + state: "Todo" + dependencies: + - "task-11" + - "task-12" + - "task-13" + acceptanceCriteria: + - "Integration test covers: connect -> validate -> optimistic update -> server persist" + - "Integration test covers: edge delete -> unparent -> optimistic update -> server persist" + - "Integration test covers: deriveGraph produces correct edges after reparent" + - "Integration test covers: deriveGraph produces correct edges after unparent" + - "Integration test covers: topology-key changes after reparent (triggers re-layout)" + tdd: + red: + - "Write test: onConnect with valid connection results in new dependency edge in deriveGraph output" + - "Write test: onConnect with valid connection removes old repo-to-child edge in deriveGraph output" + - "Write test: edge delete results in repo-to-child edge restored in deriveGraph output" + - "Write test: reparent updates featureMap parentNodeId" + - "Write test: topology key changes after reparent (edge set changed)" + green: + - "Wire tests using mock featureMap, call handleConnect/handleEdgesDelete, verify deriveGraph output" + - "Verify optimistic state updates in useGraphState" + refactor: + - "Extract test helpers for creating mock feature maps with parent-child relationships" + estimatedEffort: "2h" + +totalEstimate: "16h" + +openQuestions: [] + +content: | + ## Summary + + 14 tasks across 4 phases implement the complete feature-dependency-rebase capability. + + **Phase 1 (tasks 1-3)** establishes the backend contract: ReparentFeatureUseCase with all + business validations (cycle detection, same-repo, lifecycle guards, state adjustment), + the server action, and DI registration. This is the foundation everything else depends on. + + **Phase 2 (tasks 4-5)** adds the git infrastructure: rebaseOnBranch method on IGitPrService + and its implementation in GitPrService. Small, focused change following the existing + rebaseOnMain pattern. + + **Phase 3 (tasks 6-7)** wires auto-rebase into CheckAndUnblockFeaturesUseCase. This is + the most complex phase — injecting rebase orchestration with stash, conflict resolution, + timeout, and failure isolation between the existing lifecycle transition and agent spawn. + All patterns are proven in RebaseFeatureOnMainUseCase. + + **Phase 4 (tasks 8-14)** implements the canvas interaction layer: optimistic mutations in + useGraphState, handleConnect/handleEdgesDelete in control-center state, canvas prop updates, + enhanced DependencyEdge with delete button, feature node handle updates, and integration + tests. Task 13 (handle updates) has no dependencies and can start in parallel with phase 1. + + Every code task follows TDD (RED-GREEN-REFACTOR). Tests are written first to define + expected behavior, then minimal implementation to pass, then cleanup. The task order + ensures dependencies are satisfied — inner architecture layers before outer, contracts + before consumers. diff --git a/src/presentation/web/app/actions/reparent-feature.ts b/src/presentation/web/app/actions/reparent-feature.ts new file mode 100644 index 000000000..0d0571609 --- /dev/null +++ b/src/presentation/web/app/actions/reparent-feature.ts @@ -0,0 +1,22 @@ +'use server'; + +import { resolve } from '@/lib/server-container'; +import type { ReparentFeatureUseCase } from '@shepai/core/application/use-cases/features/reparent-feature.use-case'; + +export async function reparentFeature( + featureId: string, + parentId: string | null +): Promise<{ success: boolean; error?: string }> { + if (!featureId.trim()) { + return { success: false, error: 'Feature id is required' }; + } + + try { + const useCase = resolve('ReparentFeatureUseCase'); + await useCase.execute({ featureId, parentId }); + return { success: true }; + } catch (error: unknown) { + const message = error instanceof Error ? error.message : 'Failed to reparent feature'; + return { success: false, error: message }; + } +} diff --git a/src/presentation/web/components/common/feature-node/feature-node.tsx b/src/presentation/web/components/common/feature-node/feature-node.tsx index ab4447ebb..8e7ea5fe7 100644 --- a/src/presentation/web/components/common/feature-node/feature-node.tsx +++ b/src/presentation/web/components/common/feature-node/feature-node.tsx @@ -135,15 +135,14 @@ export function FeatureNode({ className="animate-in fade-in group relative duration-300" style={{ direction: isRtl ? 'rtl' : 'ltr' }} > - {data.showHandles ? ( - - ) : null} + {/* Target handle — always rendered for connection support. */} + {/* Action buttons — positioned on the target-handle side of the node (left in LTR, right in RTL). */}
- ) : data.showHandles ? ( + ) : ( - ) : null} + )}
); } diff --git a/src/presentation/web/components/features/control-center/control-center-inner.tsx b/src/presentation/web/components/features/control-center/control-center-inner.tsx index 5f2adf6de..88adcafd7 100644 --- a/src/presentation/web/components/features/control-center/control-center-inner.tsx +++ b/src/presentation/web/components/features/control-center/control-center-inner.tsx @@ -66,6 +66,7 @@ export function ControlCenterInner({ initialNodes, initialEdges }: ControlCenter edges, onNodesChange, handleConnect, + handleEdgesDelete, handleAddRepository, handleArchiveFeature, handleDeleteFeature, @@ -514,6 +515,7 @@ export function ControlCenterInner({ initialNodes, initialEdges }: ControlCenter defaultViewport={defaultViewport} onNodesChange={onNodesChange} onConnect={handleConnect} + onEdgesDelete={handleEdgesDelete} onAddFeature={handleAddFeature} onNodeClick={handleNodeClick} onPaneClick={handleClearDrawers} diff --git a/src/presentation/web/components/features/control-center/use-control-center-state.ts b/src/presentation/web/components/features/control-center/use-control-center-state.ts index d01821d10..0ace72ec4 100644 --- a/src/presentation/web/components/features/control-center/use-control-center-state.ts +++ b/src/presentation/web/components/features/control-center/use-control-center-state.ts @@ -40,6 +40,7 @@ export interface ControlCenterState { edges: Edge[]; onNodesChange: (changes: NodeChange[]) => void; handleConnect: (connection: Connection) => void; + handleEdgesDelete: (edges: Edge[]) => void; handleAddRepository: (path: string) => { wasEmpty: boolean; repoPath: string; @@ -78,6 +79,9 @@ export interface ControlCenterState { /** Must match the message string emitted by the SSE route in agent-events/route.ts */ const METADATA_UPDATED_MESSAGE = 'Feature metadata updated'; +/** Lifecycle states that are terminal and cannot be reparented. */ +const TERMINAL_STATES = new Set(['done', 'archived', 'deleting']); + let nextFeatureId = 0; let nextRepoTempId = 0; @@ -104,9 +108,11 @@ export function useControlCenterState( removeRepository, replaceRepository, getFeatureRepositoryPath, + getFeatureEntry, getRepositoryData, getRepoMapSize, setCallbacks, + reparentFeature, beginMutation, endMutation, isMutating, @@ -308,9 +314,61 @@ export function useControlCenterState( // Intentional no-op: domain Maps are the source of truth. }, []); - const handleConnect = useCallback((_connection: Connection) => { - // Connections are managed via domain operations, not direct edge manipulation. - }, []); + const handleConnect = useCallback( + (connection: Connection) => { + const { source, target } = connection; + if (!source || !target) return; + + // Reject non-feature-to-feature connections (repo group nodes don't have 'feat-' prefix) + if (!source.startsWith('feat-') || !target.startsWith('feat-')) { + toast.error('Only feature-to-feature connections are allowed'); + return; + } + + // Reject self-connections + if (source === target) { + toast.error('A feature cannot depend on itself'); + return; + } + + // Look up child (target) and parent (source) entries for validation + const childEntry = getFeatureEntry(target); + const parentEntry = getFeatureEntry(source); + + if (!childEntry || !parentEntry) { + toast.error('Feature not found'); + return; + } + + // Reject cross-repo connections + if (childEntry.data.repositoryPath !== parentEntry.data.repositoryPath) { + toast.error('Features must be in the same repository to form a dependency'); + return; + } + + // Reject if child is in a terminal lifecycle state + if (TERMINAL_STATES.has(childEntry.data.state)) { + toast.error('Completed features cannot be reparented'); + return; + } + + // All client-side checks passed — delegate to reparentFeature for optimistic update + server call + reparentFeature(target, source); + }, + [getFeatureEntry, reparentFeature] + ); + + const handleEdgesDelete = useCallback( + (deletedEdges: Edge[]) => { + for (const edge of deletedEdges) { + // Only process dependency edges — ignore repo-to-feature edges + if (edge.type !== 'dependencyEdge') continue; + // target is the child feature node ID + reparentFeature(edge.target, null); + } + }, + [reparentFeature] + ); const createFeatureNode = useCallback( ( @@ -661,6 +719,7 @@ export function useControlCenterState( edges, onNodesChange, handleConnect, + handleEdgesDelete, handleAddRepository, handleArchiveFeature, handleLayout, diff --git a/src/presentation/web/components/features/features-canvas/dependency-edge.stories.tsx b/src/presentation/web/components/features/features-canvas/dependency-edge.stories.tsx index 4a665a677..570f0ecbd 100644 --- a/src/presentation/web/components/features/features-canvas/dependency-edge.stories.tsx +++ b/src/presentation/web/components/features/features-canvas/dependency-edge.stories.tsx @@ -46,6 +46,7 @@ const childData: FeatureNodeData = { showHandles: true, }; +/** Default dependency edge — hover to reveal delete button. */ export const Default: StoryObj = { render: () => ( + ), +}; + +/** Edge in selected state — shows delete button and blue highlight stroke. */ +export const Selected: StoryObj = { + render: () => ( + ), diff --git a/src/presentation/web/components/features/features-canvas/dependency-edge.tsx b/src/presentation/web/components/features/features-canvas/dependency-edge.tsx index af7c158de..527b77eb2 100644 --- a/src/presentation/web/components/features/features-canvas/dependency-edge.tsx +++ b/src/presentation/web/components/features/features-canvas/dependency-edge.tsx @@ -1,14 +1,20 @@ 'use client'; -import { BaseEdge, getBezierPath } from '@xyflow/react'; +import { useState } from 'react'; +import { BaseEdge, EdgeLabelRenderer, getBezierPath, useReactFlow } from '@xyflow/react'; import type { EdgeProps } from '@xyflow/react'; +import { X } from 'lucide-react'; /** * Custom React Flow edge for parent→child feature dependencies. * Uses bezier curves and dashed style matching repo→feature edges. + * Shows a delete button on hover for unparenting. */ export function DependencyEdge(props: EdgeProps) { - const [edgePath] = getBezierPath({ + const [hovered, setHovered] = useState(false); + const { deleteElements } = useReactFlow(); + + const [edgePath, labelX, labelY] = getBezierPath({ sourceX: props.sourceX, sourceY: props.sourceY, targetX: props.targetX, @@ -18,12 +24,46 @@ export function DependencyEdge(props: EdgeProps) { }); return ( - + <> + {/* Invisible wider path for easier hover targeting */} + setHovered(true)} + onMouseLeave={() => setHovered(false)} + /> + + {hovered || props.selected ? ( + + + + ) : null} + ); } diff --git a/src/presentation/web/components/features/features-canvas/features-canvas.tsx b/src/presentation/web/components/features/features-canvas/features-canvas.tsx index 539ac335e..4537d1ca0 100644 --- a/src/presentation/web/components/features/features-canvas/features-canvas.tsx +++ b/src/presentation/web/components/features/features-canvas/features-canvas.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { ReactFlow, Background, BackgroundVariant, Panel } from '@xyflow/react'; import type { Connection, Edge, NodeChange, OnMoveEnd, Viewport } from '@xyflow/react'; import { Plus } from 'lucide-react'; @@ -27,6 +27,7 @@ export interface FeaturesCanvasProps { onNodeClick?: (event: React.MouseEvent, node: CanvasNodeType) => void; onPaneClick?: (event: React.MouseEvent) => void; onConnect?: (connection: Connection) => void; + onEdgesDelete?: (edges: Edge[]) => void; onCanvasDrag?: () => void; onMoveEnd?: OnMoveEnd; toolbar?: React.ReactNode; @@ -44,6 +45,7 @@ export function FeaturesCanvas({ onNodesChange, onAddFeature, onConnect, + onEdgesDelete, onNodeClick, onPaneClick, onCanvasDrag, @@ -137,6 +139,16 @@ export function FeaturesCanvas({ /> ) : null; + // Reject connections at the React Flow visual level — prevents the connection + // line from snapping to invalid targets (non-feature nodes, self-connections). + const isValidConnection = useCallback((connection: Connection | Edge) => { + const { source, target } = connection; + if (!source || !target) return false; + if (source === target) return false; + // Only feature-to-feature connections are valid + return source.startsWith('feat-') && target.startsWith('feat-'); + }, []); + const overlayContent = emptyState ?? fallbackEmptyState; return ( @@ -151,15 +163,17 @@ export function FeaturesCanvas({ nodeTypes={nodeTypes} edgeTypes={edgeTypes} onConnect={onConnect} + onEdgesDelete={onEdgesDelete} onNodesChange={onNodesChange} onNodeClick={onNodeClick} onPaneClick={onPaneClick} onMoveStart={onCanvasDrag} onMoveEnd={onMoveEnd} + isValidConnection={isValidConnection} defaultViewport={defaultViewport ?? FALLBACK_VIEWPORT} nodesDraggable={false} - nodesConnectable={false} - elementsSelectable={false} + nodesConnectable={true} + elementsSelectable={true} proOptions={{ hideAttribution: true }} className="[&_.react-flow__pane]:!cursor-default" > diff --git a/src/presentation/web/hooks/use-graph-state.ts b/src/presentation/web/hooks/use-graph-state.ts index 0456e676b..6f8e8d9ad 100644 --- a/src/presentation/web/hooks/use-graph-state.ts +++ b/src/presentation/web/hooks/use-graph-state.ts @@ -2,6 +2,7 @@ import { useState, useMemo, useCallback, useRef, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; +import { toast } from 'sonner'; import type { Edge, Position } from '@xyflow/react'; import type { CanvasNodeType } from '@/components/features/features-canvas'; import type { FeatureNodeData } from '@/components/common/feature-node'; @@ -13,6 +14,7 @@ import { type GraphCallbacks, } from '@/lib/derive-graph'; import { layoutWithDagre, getCanvasLayoutDefaults } from '@/lib/layout-with-dagre'; +import { reparentFeature as reparentFeatureAction } from '@/app/actions/reparent-feature'; export type { GraphCallbacks } from '@/lib/derive-graph'; @@ -45,12 +47,19 @@ export interface UseGraphStateReturn { replaceRepository: (tempId: string, realId: string, data: RepositoryNodeData) => void; /** Stable lookup: get the repositoryPath for a feature node. */ getFeatureRepositoryPath: (featureNodeId: string) => string | undefined; + /** Stable lookup: get a feature entry from the domain Map. */ + getFeatureEntry: (nodeId: string) => FeatureEntry | undefined; /** Stable lookup: get repository node data by nodeId. */ getRepositoryData: (nodeId: string) => RepositoryNodeData | undefined; /** Stable lookup: get the current number of repositories in the domain Map. */ getRepoMapSize: () => number; /** Update callbacks injected into node data (does NOT trigger re-render). */ setCallbacks: (callbacks: GraphCallbacks) => void; + /** + * Optimistically reparent a feature (set/clear parentNodeId in featureMap). + * Calls the reparent-feature server action; rolls back on error. + */ + reparentFeature: (childNodeId: string, newParentNodeId: string | null) => void; /** * Signal that an optimistic mutation has started. While any mutation is * in-flight, `reconcile` becomes a no-op so stale poll data cannot @@ -531,6 +540,10 @@ export function useGraphState( return featureMapRef.current.get(featureNodeId)?.data.repositoryPath; }, []); + const getFeatureEntry = useCallback((nodeId: string): FeatureEntry | undefined => { + return featureMapRef.current.get(nodeId); + }, []); + const getRepositoryData = useCallback((nodeId: string): RepositoryNodeData | undefined => { return repoMapRef.current.get(nodeId)?.data; }, []); @@ -543,6 +556,75 @@ export function useGraphState( callbacksRef.current = callbacks; }, []); + const reparentFeatureCallback = useCallback( + (childNodeId: string, newParentNodeId: string | null) => { + // Snapshot previous parentNodeId for rollback + const entry = featureMapRef.current.get(childNodeId); + if (!entry) return; + const prevParentNodeId = entry.parentNodeId; + + // Extract featureId from node ID (strip 'feat-' prefix) + const featureId = childNodeId.startsWith('feat-') ? childNodeId.slice(5) : childNodeId; + // Extract parent featureId (strip 'feat-' prefix), or null for unparent + const parentFeatureId = newParentNodeId + ? newParentNodeId.startsWith('feat-') + ? newParentNodeId.slice(5) + : newParentNodeId + : null; + + // Optimistic update + mutationCountRef.current++; + setFeatureMap((prev) => { + const current = prev.get(childNodeId); + if (!current) return prev; + const next = new Map(prev); + next.set(childNodeId, { + ...current, + parentNodeId: newParentNodeId ?? undefined, + }); + return next; + }); + + // Call server action + reparentFeatureAction(featureId, parentFeatureId) + .then((result) => { + if (!result.success) { + // Rollback optimistic update + setFeatureMap((prev) => { + const current = prev.get(childNodeId); + if (!current) return prev; + const next = new Map(prev); + next.set(childNodeId, { ...current, parentNodeId: prevParentNodeId }); + return next; + }); + toast.error(result.error ?? 'Failed to reparent feature'); + } else { + toast.success(newParentNodeId ? 'Feature reparented' : 'Feature detached from parent'); + } + }) + .catch(() => { + // Rollback optimistic update + setFeatureMap((prev) => { + const current = prev.get(childNodeId); + if (!current) return prev; + const next = new Map(prev); + next.set(childNodeId, { ...current, parentNodeId: prevParentNodeId }); + return next; + }); + toast.error('Failed to reparent feature'); + }) + .finally(() => { + // Delay decrement for poll cooldown + const timer = setTimeout(() => { + mutationCountRef.current = Math.max(0, mutationCountRef.current - 1); + mutationTimersRef.current.delete(timer); + }, 3_000); + mutationTimersRef.current.add(timer); + }); + }, + [] + ); + const beginMutation = useCallback(() => { mutationCountRef.current++; }, []); @@ -581,9 +663,11 @@ export function useGraphState( removeRepository, replaceRepository, getFeatureRepositoryPath, + getFeatureEntry, getRepositoryData, getRepoMapSize, setCallbacks, + reparentFeature: reparentFeatureCallback, beginMutation, endMutation, isMutating, diff --git a/src/presentation/web/lib/derive-graph.ts b/src/presentation/web/lib/derive-graph.ts index 9945aec7d..e3b324b21 100644 --- a/src/presentation/web/lib/derive-graph.ts +++ b/src/presentation/web/lib/derive-graph.ts @@ -239,9 +239,12 @@ export function deriveGraph( const hasEdges = edges.length > 0; for (const node of nodes) { const data = node.data as Record; - data.showHandles = hasEdges; if (node.type === 'featureNode') { + // Feature nodes always show handles for drag-to-connect reparenting + data.showHandles = true; data.hasChildren = parentNodeIds.has(node.id); + } else { + data.showHandles = hasEdges; } } diff --git a/tests/integration/infrastructure/services/agents/graph-state-transitions/setup.ts b/tests/integration/infrastructure/services/agents/graph-state-transitions/setup.ts index 47f989e66..d28c0b195 100644 --- a/tests/integration/infrastructure/services/agents/graph-state-transitions/setup.ts +++ b/tests/integration/infrastructure/services/agents/graph-state-transitions/setup.ts @@ -195,6 +195,7 @@ export function createStubMergeNodeDeps(featureId?: string): Omit { rebaseOnMain: async () => { /* noop */ }, + rebaseOnBranch: async () => { + /* noop */ + }, getConflictedFiles: async () => [], stageFiles: async () => { /* noop */ diff --git a/tests/unit/application/use-cases/features/auto-resolve-merged-branches.use-case.test.ts b/tests/unit/application/use-cases/features/auto-resolve-merged-branches.use-case.test.ts index b8f1c6067..963b5e7a0 100644 --- a/tests/unit/application/use-cases/features/auto-resolve-merged-branches.use-case.test.ts +++ b/tests/unit/application/use-cases/features/auto-resolve-merged-branches.use-case.test.ts @@ -81,6 +81,7 @@ function createMockGitPrService(): IGitPrService { getFailureLogs: vi.fn(), syncMain: vi.fn(), rebaseOnMain: vi.fn(), + rebaseOnBranch: vi.fn(), getConflictedFiles: vi.fn(), stageFiles: vi.fn(), rebaseContinue: vi.fn(), diff --git a/tests/unit/application/use-cases/features/check-and-unblock-features-rebase.test.ts b/tests/unit/application/use-cases/features/check-and-unblock-features-rebase.test.ts new file mode 100644 index 000000000..899907efc --- /dev/null +++ b/tests/unit/application/use-cases/features/check-and-unblock-features-rebase.test.ts @@ -0,0 +1,698 @@ +/** + * CheckAndUnblockFeaturesUseCase — Auto-Rebase Tests + * + * Tests the rebase orchestration that runs between lifecycle transition + * (Blocked → Started) and agent spawn. Each blocked child's branch is + * rebased onto the parent's branch before the agent spawns. + * + * TDD Phase: RED-GREEN-REFACTOR + */ + +import 'reflect-metadata'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { CheckAndUnblockFeaturesUseCase } from '@/application/use-cases/features/check-and-unblock-features.use-case.js'; +import type { IFeatureRepository } from '@/application/ports/output/repositories/feature-repository.interface.js'; +import type { IFeatureAgentProcessService } from '@/application/ports/output/agents/feature-agent-process.interface.js'; +import type { IGitPrService } from '@/application/ports/output/services/git-pr-service.interface.js'; +import { + GitPrError, + GitPrErrorCode, +} from '@/application/ports/output/services/git-pr-service.interface.js'; +import type { IWorktreeService } from '@/application/ports/output/services/worktree-service.interface.js'; +import type { ConflictResolutionService } from '@/infrastructure/services/agents/conflict-resolution/conflict-resolution.service.js'; +import type { IAgentRunRepository } from '@/application/ports/output/agents/agent-run-repository.interface.js'; +import type { IPhaseTimingRepository } from '@/application/ports/output/agents/phase-timing-repository.interface.js'; +import { SdlcLifecycle, AgentRunStatus } from '@/domain/generated/output.js'; +import type { Feature } from '@/domain/generated/output.js'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeFeature(overrides?: Partial): Feature { + return { + id: 'feat-001', + name: 'Test Feature', + slug: 'test-feature', + description: 'A test feature', + userQuery: 'test query', + repositoryPath: '/repo', + branch: 'feat/test-feature', + lifecycle: SdlcLifecycle.Implementation, + messages: [], + relatedArtifacts: [], + fast: false, + push: false, + openPr: false, + forkAndPr: false, + commitSpecs: true, + ciWatchEnabled: true, + enableEvidence: false, + commitEvidence: false, + injectSkills: false, + approvalGates: { allowPrd: false, allowPlan: false, allowMerge: false }, + agentRunId: 'run-001', + specPath: '/repo/.shep/specs/001-test-feature', + worktreePath: '/worktrees/test-feature', + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// Mock factories +// --------------------------------------------------------------------------- + +function createMockFeatureRepo(): IFeatureRepository { + return { + create: vi.fn(), + findById: vi.fn().mockResolvedValue(null), + findByIdPrefix: vi.fn().mockResolvedValue(null), + findBySlug: vi.fn().mockResolvedValue(null), + findByBranch: vi.fn().mockResolvedValue(null), + list: vi.fn().mockResolvedValue([]), + findByParentId: vi.fn().mockResolvedValue([]), + update: vi.fn().mockResolvedValue(undefined), + delete: vi.fn(), + softDelete: vi.fn(), + } as unknown as IFeatureRepository; +} + +function createMockAgentProcess(): IFeatureAgentProcessService { + return { + spawn: vi.fn().mockReturnValue(1234), + isAlive: vi.fn(), + checkAndMarkCrashed: vi.fn(), + }; +} + +function createMockGitPrService(): IGitPrService { + return { + getDefaultBranch: vi.fn().mockResolvedValue('main'), + syncMain: vi.fn().mockResolvedValue(undefined), + rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), + hasUncommittedChanges: vi.fn(), + hasRemote: vi.fn(), + getRemoteUrl: vi.fn(), + push: vi.fn(), + createPr: vi.fn(), + mergePr: vi.fn(), + mergeBranch: vi.fn(), + localMergeSquash: vi.fn(), + getCiStatus: vi.fn(), + watchCi: vi.fn(), + deleteBranch: vi.fn(), + getPrDiffSummary: vi.fn(), + getFileDiffs: vi.fn(), + listPrStatuses: vi.fn(), + getMergeableStatus: vi.fn(), + verifyMerge: vi.fn(), + revParse: vi.fn(), + commitAll: vi.fn(), + getFailureLogs: vi.fn(), + getConflictedFiles: vi.fn(), + stageFiles: vi.fn(), + rebaseContinue: vi.fn(), + rebaseAbort: vi.fn(), + stash: vi.fn().mockResolvedValue(false), + stashPop: vi.fn().mockResolvedValue(undefined), + getBranchSyncStatus: vi.fn(), + } as unknown as IGitPrService; +} + +function createMockWorktreeService(): IWorktreeService { + return { + create: vi.fn(), + addExisting: vi.fn(), + remove: vi.fn(), + list: vi.fn(), + exists: vi.fn().mockResolvedValue(false), + branchExists: vi.fn(), + remoteBranchExists: vi.fn(), + getWorktreePath: vi.fn().mockReturnValue('/repo/.worktrees/feat-x'), + listBranches: vi.fn(), + prune: vi.fn(), + ensureGitRepository: vi.fn(), + } as unknown as IWorktreeService; +} + +function createMockConflictResolution(): ConflictResolutionService { + return { + resolve: vi.fn().mockResolvedValue(undefined), + } as unknown as ConflictResolutionService; +} + +function createMockAgentRunRepo(): IAgentRunRepository { + return { + create: vi.fn().mockResolvedValue(undefined), + findById: vi.fn().mockResolvedValue(null), + findByThreadId: vi.fn(), + updateStatus: vi.fn().mockResolvedValue(undefined), + updatePinnedConfig: vi.fn(), + findRunningByPid: vi.fn(), + list: vi.fn(), + delete: vi.fn(), + } as unknown as IAgentRunRepository; +} + +function createMockPhaseTimingRepo(): IPhaseTimingRepository { + return { + save: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + updateApprovalWait: vi.fn(), + findByRunId: vi.fn(), + findByFeatureId: vi.fn(), + } as unknown as IPhaseTimingRepository; +} + +// --------------------------------------------------------------------------- +// Test Suite +// --------------------------------------------------------------------------- + +describe('CheckAndUnblockFeaturesUseCase — Auto-Rebase', () => { + let useCase: CheckAndUnblockFeaturesUseCase; + let mockFeatureRepo: IFeatureRepository; + let mockAgentProcess: IFeatureAgentProcessService; + let mockGitPrService: IGitPrService; + let mockWorktreeService: IWorktreeService; + let mockConflictResolution: ConflictResolutionService; + let mockAgentRunRepo: IAgentRunRepository; + let mockPhaseTimingRepo: IPhaseTimingRepository; + + const parentId = 'parent-001'; + + beforeEach(() => { + vi.clearAllMocks(); + mockFeatureRepo = createMockFeatureRepo(); + mockAgentProcess = createMockAgentProcess(); + mockGitPrService = createMockGitPrService(); + mockWorktreeService = createMockWorktreeService(); + mockConflictResolution = createMockConflictResolution(); + mockAgentRunRepo = createMockAgentRunRepo(); + mockPhaseTimingRepo = createMockPhaseTimingRepo(); + + useCase = new CheckAndUnblockFeaturesUseCase( + mockFeatureRepo, + mockAgentProcess, + mockGitPrService, + mockWorktreeService, + mockConflictResolution, + mockAgentRunRepo, + mockPhaseTimingRepo + ); + }); + + // ------------------------------------------------------------------------- + // Successful rebase — happy path + // ------------------------------------------------------------------------- + + it('should rebase child branch onto parent branch before spawning agent', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent-feature', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child-feature', + repositoryPath: '/repo', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + await useCase.execute(parentId); + + // Stash should be called before rebase + expect(mockGitPrService.stash).toHaveBeenCalledWith( + '/repo', + expect.stringContaining('auto-stash') + ); + + // rebaseOnBranch should be called with parent's branch + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledWith( + '/repo', + 'feat/child-feature', + 'feat/parent-feature' + ); + + // Agent should still be spawned + expect(mockAgentProcess.spawn).toHaveBeenCalledOnce(); + }); + + it('should use parent branch name as target for rebaseOnBranch', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/my-parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/my-child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + await useCase.execute(parentId); + + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledWith( + expect.any(String), + 'feat/my-child', + 'feat/my-parent' + ); + }); + + it('should call stashPop after successful rebase', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.stash).mockResolvedValue(true); + + await useCase.execute(parentId); + + expect(mockGitPrService.stashPop).toHaveBeenCalledWith('/repo'); + }); + + it('should not call stashPop when no changes were stashed', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.stash).mockResolvedValue(false); + + await useCase.execute(parentId); + + expect(mockGitPrService.stashPop).not.toHaveBeenCalled(); + }); + + // ------------------------------------------------------------------------- + // Agent run and phase timing creation + // ------------------------------------------------------------------------- + + it('should create agent run and phase timing for rebase operation', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + await useCase.execute(parentId); + + // Agent run created for rebase + expect(mockAgentRunRepo.create).toHaveBeenCalledWith( + expect.objectContaining({ + featureId: 'child-001', + status: AgentRunStatus.running, + prompt: expect.stringContaining('feat/parent'), + }) + ); + + // Phase timing created + expect(mockPhaseTimingRepo.save).toHaveBeenCalledWith( + expect.objectContaining({ + phase: 'rebase-on-parent', + }) + ); + + // Phase timing completed with success + expect(mockPhaseTimingRepo.update).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + exitCode: 'success', + }) + ); + + // Agent run marked as completed + expect(mockAgentRunRepo.updateStatus).toHaveBeenCalledWith( + expect.any(String), + AgentRunStatus.completed, + expect.objectContaining({ completedAt: expect.any(String) }) + ); + }); + + // ------------------------------------------------------------------------- + // Conflict resolution + // ------------------------------------------------------------------------- + + it('should delegate to ConflictResolutionService on rebase conflict', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.rebaseOnBranch).mockRejectedValue( + new GitPrError('Rebase conflicts', GitPrErrorCode.REBASE_CONFLICT) + ); + + await useCase.execute(parentId); + + expect(mockConflictResolution.resolve).toHaveBeenCalledWith( + '/repo', + 'feat/child', + 'feat/parent' + ); + + // Agent should still be spawned after conflict resolution + expect(mockAgentProcess.spawn).toHaveBeenCalledOnce(); + }); + + // ------------------------------------------------------------------------- + // Rebase failure — agent still spawns + // ------------------------------------------------------------------------- + + it('should still spawn agent when rebase fails', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.rebaseOnBranch).mockRejectedValue( + new GitPrError('Unexpected git failure', GitPrErrorCode.GIT_ERROR) + ); + + await useCase.execute(parentId); + + // Agent spawned despite rebase failure + expect(mockAgentProcess.spawn).toHaveBeenCalledOnce(); + }); + + it('should record rebase failure in phase timing', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.rebaseOnBranch).mockRejectedValue( + new GitPrError('Unexpected git failure', GitPrErrorCode.GIT_ERROR) + ); + + await useCase.execute(parentId); + + // Phase timing records error + expect(mockPhaseTimingRepo.update).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + exitCode: 'error', + errorMessage: expect.stringContaining('Unexpected git failure'), + }) + ); + + // Agent run marked as failed + expect(mockAgentRunRepo.updateStatus).toHaveBeenCalledWith( + expect.any(String), + AgentRunStatus.failed, + expect.objectContaining({ error: expect.stringContaining('Unexpected git failure') }) + ); + }); + + // ------------------------------------------------------------------------- + // Stash restore in finally block + // ------------------------------------------------------------------------- + + it('should call stashPop in finally block even when rebase throws', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockGitPrService.stash).mockResolvedValue(true); + vi.mocked(mockGitPrService.rebaseOnBranch).mockRejectedValue( + new GitPrError('Unexpected git failure', GitPrErrorCode.GIT_ERROR) + ); + + await useCase.execute(parentId); + + expect(mockGitPrService.stashPop).toHaveBeenCalledWith('/repo'); + }); + + // ------------------------------------------------------------------------- + // Failure isolation across children + // ------------------------------------------------------------------------- + + it('should still rebase and spawn second child when first child rebase fails', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child1 = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child-1', + agentRunId: 'run-1', + specPath: '/specs/1', + }); + const child2 = makeFeature({ + id: 'child-002', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child-2', + agentRunId: 'run-2', + specPath: '/specs/2', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child1, child2]); + + // First child rebase fails, second succeeds + vi.mocked(mockGitPrService.rebaseOnBranch) + .mockRejectedValueOnce(new GitPrError('Git error', GitPrErrorCode.GIT_ERROR)) + .mockResolvedValueOnce(undefined); + + await useCase.execute(parentId); + + // Both lifecycle transitions happen + expect(mockFeatureRepo.update).toHaveBeenCalledTimes(2); + + // Both agents spawned + expect(mockAgentProcess.spawn).toHaveBeenCalledTimes(2); + + // rebaseOnBranch called for both children + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledTimes(2); + }); + + // ------------------------------------------------------------------------- + // Skip rebase when child has active agent run (NFR-3) + // ------------------------------------------------------------------------- + + it('should skip rebase when child has an active (running) agent run', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + agentRunId: 'active-run-001', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + // Agent run is currently running + vi.mocked(mockAgentRunRepo.findById).mockResolvedValue({ + id: 'active-run-001', + status: AgentRunStatus.running, + } as any); + + await useCase.execute(parentId); + + // Rebase skipped + expect(mockGitPrService.rebaseOnBranch).not.toHaveBeenCalled(); + expect(mockGitPrService.stash).not.toHaveBeenCalled(); + + // Lifecycle still transitions and agent still spawns + expect(mockFeatureRepo.update).toHaveBeenCalledOnce(); + expect(mockAgentProcess.spawn).toHaveBeenCalledOnce(); + }); + + it('should rebase when child agent run is not running (completed)', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + agentRunId: 'completed-run-001', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + // Agent run is completed (not running) + vi.mocked(mockAgentRunRepo.findById).mockResolvedValue({ + id: 'completed-run-001', + status: AgentRunStatus.completed, + } as any); + + await useCase.execute(parentId); + + // Rebase proceeds + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledOnce(); + }); + + it('should rebase when child has no agent run ID', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + agentRunId: undefined, + specPath: undefined, + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + + await useCase.execute(parentId); + + // Rebase proceeds (no agent run to check) + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledOnce(); + + // But spawn is skipped (defensive guard: no agentRunId/specPath) + expect(mockAgentProcess.spawn).not.toHaveBeenCalled(); + }); + + // ------------------------------------------------------------------------- + // Worktree path resolution + // ------------------------------------------------------------------------- + + it('should use worktree path when worktree exists for child branch', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + repositoryPath: '/repo', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockWorktreeService.exists).mockResolvedValue(true); + vi.mocked(mockWorktreeService.getWorktreePath).mockReturnValue('/repo/.worktrees/feat-child'); + + await useCase.execute(parentId); + + expect(mockWorktreeService.exists).toHaveBeenCalledWith('/repo', 'feat/child'); + expect(mockGitPrService.stash).toHaveBeenCalledWith( + '/repo/.worktrees/feat-child', + expect.any(String) + ); + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledWith( + '/repo/.worktrees/feat-child', + 'feat/child', + 'feat/parent' + ); + }); + + it('should use repo root when no worktree exists for child branch', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const child = makeFeature({ + id: 'child-001', + lifecycle: SdlcLifecycle.Blocked, + branch: 'feat/child', + repositoryPath: '/my-repo', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([child]); + vi.mocked(mockWorktreeService.exists).mockResolvedValue(false); + + await useCase.execute(parentId); + + expect(mockGitPrService.rebaseOnBranch).toHaveBeenCalledWith( + '/my-repo', + 'feat/child', + 'feat/parent' + ); + }); + + // ------------------------------------------------------------------------- + // Non-blocked children are not rebased + // ------------------------------------------------------------------------- + + it('should not rebase non-blocked children', async () => { + const parent = makeFeature({ + id: parentId, + lifecycle: SdlcLifecycle.Implementation, + branch: 'feat/parent', + }); + const startedChild = makeFeature({ + id: 'child-started', + lifecycle: SdlcLifecycle.Started, + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValue(parent); + vi.mocked(mockFeatureRepo.findByParentId).mockResolvedValue([startedChild]); + + await useCase.execute(parentId); + + expect(mockGitPrService.rebaseOnBranch).not.toHaveBeenCalled(); + expect(mockGitPrService.stash).not.toHaveBeenCalled(); + expect(mockAgentRunRepo.create).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/application/use-cases/features/check-and-unblock-features.use-case.test.ts b/tests/unit/application/use-cases/features/check-and-unblock-features.use-case.test.ts index 4b06f84ca..251cb2f2f 100644 --- a/tests/unit/application/use-cases/features/check-and-unblock-features.use-case.test.ts +++ b/tests/unit/application/use-cases/features/check-and-unblock-features.use-case.test.ts @@ -16,6 +16,11 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { CheckAndUnblockFeaturesUseCase } from '@/application/use-cases/features/check-and-unblock-features.use-case.js'; import type { IFeatureRepository } from '@/application/ports/output/repositories/feature-repository.interface.js'; import type { IFeatureAgentProcessService } from '@/application/ports/output/agents/feature-agent-process.interface.js'; +import type { IGitPrService } from '@/application/ports/output/services/git-pr-service.interface.js'; +import type { IWorktreeService } from '@/application/ports/output/services/worktree-service.interface.js'; +import type { ConflictResolutionService } from '@/infrastructure/services/agents/conflict-resolution/conflict-resolution.service.js'; +import type { IAgentRunRepository } from '@/application/ports/output/agents/agent-run-repository.interface.js'; +import type { IPhaseTimingRepository } from '@/application/ports/output/agents/phase-timing-repository.interface.js'; import { SdlcLifecycle } from '@/domain/generated/output.js'; import type { Feature } from '@/domain/generated/output.js'; @@ -62,6 +67,11 @@ describe('CheckAndUnblockFeaturesUseCase', () => { let useCase: CheckAndUnblockFeaturesUseCase; let mockFeatureRepo: IFeatureRepository; let mockAgentProcess: IFeatureAgentProcessService; + let mockGitPrService: IGitPrService; + let mockWorktreeService: IWorktreeService; + let mockConflictResolution: ConflictResolutionService; + let mockAgentRunRepo: IAgentRunRepository; + let mockPhaseTimingRepo: IPhaseTimingRepository; const parentId = 'parent-001'; @@ -85,7 +95,85 @@ describe('CheckAndUnblockFeaturesUseCase', () => { checkAndMarkCrashed: vi.fn(), }; - useCase = new CheckAndUnblockFeaturesUseCase(mockFeatureRepo, mockAgentProcess); + mockGitPrService = { + getDefaultBranch: vi.fn().mockResolvedValue('main'), + syncMain: vi.fn().mockResolvedValue(undefined), + rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), + hasUncommittedChanges: vi.fn(), + hasRemote: vi.fn(), + getRemoteUrl: vi.fn(), + push: vi.fn(), + createPr: vi.fn(), + mergePr: vi.fn(), + mergeBranch: vi.fn(), + localMergeSquash: vi.fn(), + getCiStatus: vi.fn(), + watchCi: vi.fn(), + deleteBranch: vi.fn(), + getPrDiffSummary: vi.fn(), + getFileDiffs: vi.fn(), + listPrStatuses: vi.fn(), + getMergeableStatus: vi.fn(), + verifyMerge: vi.fn(), + revParse: vi.fn(), + commitAll: vi.fn(), + getFailureLogs: vi.fn(), + getConflictedFiles: vi.fn(), + stageFiles: vi.fn(), + rebaseContinue: vi.fn(), + rebaseAbort: vi.fn(), + stash: vi.fn().mockResolvedValue(false), + stashPop: vi.fn().mockResolvedValue(undefined), + getBranchSyncStatus: vi.fn(), + } as unknown as IGitPrService; + + mockWorktreeService = { + create: vi.fn(), + addExisting: vi.fn(), + remove: vi.fn(), + list: vi.fn(), + exists: vi.fn().mockResolvedValue(false), + branchExists: vi.fn(), + remoteBranchExists: vi.fn(), + getWorktreePath: vi.fn().mockReturnValue('/repo/.worktrees/feat-x'), + listBranches: vi.fn(), + prune: vi.fn(), + ensureGitRepository: vi.fn(), + } as unknown as IWorktreeService; + + mockConflictResolution = { + resolve: vi.fn().mockResolvedValue(undefined), + } as unknown as ConflictResolutionService; + + mockAgentRunRepo = { + create: vi.fn().mockResolvedValue(undefined), + findById: vi.fn(), + findByThreadId: vi.fn(), + updateStatus: vi.fn().mockResolvedValue(undefined), + updatePinnedConfig: vi.fn(), + findRunningByPid: vi.fn(), + list: vi.fn(), + delete: vi.fn(), + } as unknown as IAgentRunRepository; + + mockPhaseTimingRepo = { + save: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + updateApprovalWait: vi.fn(), + findByRunId: vi.fn(), + findByFeatureId: vi.fn(), + } as unknown as IPhaseTimingRepository; + + useCase = new CheckAndUnblockFeaturesUseCase( + mockFeatureRepo, + mockAgentProcess, + mockGitPrService, + mockWorktreeService, + mockConflictResolution, + mockAgentRunRepo, + mockPhaseTimingRepo + ); }); // ------------------------------------------------------------------------- diff --git a/tests/unit/application/use-cases/features/cleanup-feature-worktree.use-case.test.ts b/tests/unit/application/use-cases/features/cleanup-feature-worktree.use-case.test.ts index fa26d007e..4d718ee64 100644 --- a/tests/unit/application/use-cases/features/cleanup-feature-worktree.use-case.test.ts +++ b/tests/unit/application/use-cases/features/cleanup-feature-worktree.use-case.test.ts @@ -106,6 +106,7 @@ describe('CleanupFeatureWorktreeUseCase', () => { localMergeSquash: vi.fn().mockResolvedValue(undefined), syncMain: vi.fn().mockResolvedValue(undefined), rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), getConflictedFiles: vi.fn().mockResolvedValue([]), stageFiles: vi.fn().mockResolvedValue(undefined), rebaseContinue: vi.fn().mockResolvedValue(undefined), diff --git a/tests/unit/application/use-cases/repositories/init-remote-repository.use-case.test.ts b/tests/unit/application/use-cases/repositories/init-remote-repository.use-case.test.ts index 62cd48475..726c1bb45 100644 --- a/tests/unit/application/use-cases/repositories/init-remote-repository.use-case.test.ts +++ b/tests/unit/application/use-cases/repositories/init-remote-repository.use-case.test.ts @@ -48,6 +48,7 @@ describe('InitRemoteRepositoryUseCase', () => { getFailureLogs: vi.fn().mockResolvedValue(''), syncMain: vi.fn().mockResolvedValue(undefined), rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), getConflictedFiles: vi.fn().mockResolvedValue([]), stageFiles: vi.fn().mockResolvedValue(undefined), rebaseContinue: vi.fn().mockResolvedValue(undefined), diff --git a/tests/unit/infrastructure/services/git/git-pr.service.rebase-on-branch.test.ts b/tests/unit/infrastructure/services/git/git-pr.service.rebase-on-branch.test.ts new file mode 100644 index 000000000..e3352042b --- /dev/null +++ b/tests/unit/infrastructure/services/git/git-pr.service.rebase-on-branch.test.ts @@ -0,0 +1,248 @@ +/** + * GitPrService rebaseOnBranch Unit Tests + * + * TDD Phase: RED → GREEN + * Tests for rebaseOnBranch which rebases a feature branch onto an arbitrary + * target branch (not just the default branch). Follows rebaseOnMain pattern + * but adds an explicit fetch of the target branch before rebasing. + */ + +import 'reflect-metadata'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { GitPrService } from '@/infrastructure/services/git/git-pr.service'; +import { + GitPrError, + GitPrErrorCode, +} from '@/application/ports/output/services/git-pr-service.interface'; +import type { ExecFunction } from '@/infrastructure/services/git/worktree.service'; + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { ...actual, readFileSync: vi.fn() }; +}); + +describe('GitPrService — rebaseOnBranch', () => { + let mockExec: ExecFunction; + let service: GitPrService; + + beforeEach(() => { + mockExec = vi.fn(); + service = new GitPrService(mockExec); + }); + + it('should throw GIT_ERROR when worktree is dirty', async () => { + // hasUncommittedChanges → git status --porcelain returns non-empty + vi.mocked(mockExec).mockResolvedValueOnce({ + stdout: 'M src/file.ts\n', + stderr: '', + }); + + const error = await service + .rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + .catch((e) => e); + expect(error).toBeInstanceOf(GitPrError); + expect(error.code).toBe(GitPrErrorCode.GIT_ERROR); + expect(error.message).toContain('uncommitted changes'); + }); + + it('should fetch the target branch before rebasing', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout feat/child + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase origin/feat/parent → success + .mockResolvedValueOnce({ stdout: 'Successfully rebased\n', stderr: '' }); + + await service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent'); + + expect(mockExec).toHaveBeenCalledWith('git', ['fetch', 'origin', 'feat/parent'], { + cwd: '/repo', + }); + }); + + it('should checkout the feature branch before rebasing', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout feat/child + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase origin/feat/parent → success + .mockResolvedValueOnce({ stdout: '', stderr: '' }); + + await service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent'); + + expect(mockExec).toHaveBeenCalledWith('git', ['checkout', 'feat/child'], { + cwd: '/repo', + }); + }); + + it('should call git rebase with origin/ as target', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout feat/child + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase origin/feat/parent → success + .mockResolvedValueOnce({ stdout: '', stderr: '' }); + + await service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent'); + + expect(mockExec).toHaveBeenCalledWith('git', ['rebase', 'origin/feat/parent'], { + cwd: '/repo', + }); + }); + + it('should succeed on clean rebase (no conflicts)', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout feat/child + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase origin/feat/parent → success + .mockResolvedValueOnce({ stdout: 'Successfully rebased\n', stderr: '' }); + + await expect( + service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + ).resolves.toBeUndefined(); + }); + + it('should throw REBASE_CONFLICT when rebase encounters conflicts', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout feat/child + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase → conflict + .mockRejectedValueOnce(new Error('CONFLICT (content): Merge conflict in src/file.ts')) + // getConflictedFiles → git diff --name-only --diff-filter=U + .mockResolvedValueOnce({ stdout: 'src/file.ts\n', stderr: '' }); + + const error = await service + .rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + .catch((e) => e); + expect(error).toBeInstanceOf(GitPrError); + expect(error.code).toBe(GitPrErrorCode.REBASE_CONFLICT); + }); + + it('should include conflicted file list in REBASE_CONFLICT error message', async () => { + vi.mocked(mockExec) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('CONFLICT (content): Merge conflict in src/a.ts')) + .mockResolvedValueOnce({ stdout: 'src/a.ts\nsrc/b.ts\n', stderr: '' }); + + try { + await service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent'); + } catch (error) { + expect((error as GitPrError).message).toContain('src/a.ts'); + expect((error as GitPrError).message).toContain('src/b.ts'); + } + }); + + it('should detect "could not apply" as rebase conflict', async () => { + vi.mocked(mockExec) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('error: could not apply abc1234... some commit')) + .mockResolvedValueOnce({ stdout: '', stderr: '' }); + + await expect( + service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + ).rejects.toMatchObject({ + code: GitPrErrorCode.REBASE_CONFLICT, + }); + }); + + it('should throw BRANCH_NOT_FOUND when feature branch does not exist', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent → success + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git checkout → pathspec error + .mockRejectedValueOnce( + new Error("error: pathspec 'feat/nonexistent' did not match any file(s)") + ); + + const error = await service + .rebaseOnBranch('/repo', 'feat/nonexistent', 'feat/parent') + .catch((e) => e); + expect(error).toBeInstanceOf(GitPrError); + expect(error.code).toBe(GitPrErrorCode.BRANCH_NOT_FOUND); + }); + + it('should throw descriptive error when fetch of target branch fails', async () => { + vi.mocked(mockExec) + // hasUncommittedChanges → clean + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git fetch origin feat/parent → fails + .mockRejectedValueOnce(new Error("fatal: couldn't find remote ref feat/parent")); + + const error = await service + .rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + .catch((e) => e); + expect(error).toBeInstanceOf(GitPrError); + expect(error.message).toContain('feat/parent'); + expect(error.message).toContain('fetch'); + }); + + it('should throw generic GIT_ERROR on non-conflict rebase failure', async () => { + vi.mocked(mockExec) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + // git rebase fails with non-conflict error + .mockRejectedValueOnce(new Error('fatal: some random git error')); + + const error = await service + .rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + .catch((e) => e); + expect(error).toBeInstanceOf(GitPrError); + expect(error.code).toBe(GitPrErrorCode.GIT_ERROR); + }); + + it('should still throw REBASE_CONFLICT even if getConflictedFiles fails', async () => { + vi.mocked(mockExec) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + .mockRejectedValueOnce(new Error('CONFLICT (content): Merge conflict in src/file.ts')) + // getConflictedFiles also fails + .mockRejectedValueOnce(new Error('git diff failed')); + + await expect( + service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent') + ).rejects.toMatchObject({ + code: GitPrErrorCode.REBASE_CONFLICT, + }); + }); + + it('should execute steps in correct order: dirty check → fetch → checkout → rebase', async () => { + const callOrder: string[] = []; + vi.mocked(mockExec).mockImplementation(async (_cmd, args) => { + const argStr = (args as string[]).join(' '); + if (argStr.includes('status --porcelain')) callOrder.push('dirty-check'); + else if (argStr.includes('fetch origin')) callOrder.push('fetch'); + else if (argStr.includes('checkout')) callOrder.push('checkout'); + else if (argStr.includes('rebase')) callOrder.push('rebase'); + return { stdout: '', stderr: '' }; + }); + + await service.rebaseOnBranch('/repo', 'feat/child', 'feat/parent'); + + expect(callOrder).toEqual(['dirty-check', 'fetch', 'checkout', 'rebase']); + }); +}); diff --git a/tests/unit/infrastructure/services/pr-sync/pr-sync-watcher.service.test.ts b/tests/unit/infrastructure/services/pr-sync/pr-sync-watcher.service.test.ts index d3eb3a4e8..ff88fc038 100644 --- a/tests/unit/infrastructure/services/pr-sync/pr-sync-watcher.service.test.ts +++ b/tests/unit/infrastructure/services/pr-sync/pr-sync-watcher.service.test.ts @@ -104,6 +104,7 @@ function createMockGitPrService(): IGitPrService { localMergeSquash: vi.fn().mockResolvedValue(undefined), syncMain: vi.fn().mockResolvedValue(undefined), rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), getConflictedFiles: vi.fn().mockResolvedValue([]), stageFiles: vi.fn().mockResolvedValue(undefined), rebaseContinue: vi.fn().mockResolvedValue(undefined), diff --git a/tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx b/tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx index 038a93a9f..637280ea3 100644 --- a/tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx +++ b/tests/unit/presentation/web/components/features/control-center/control-center-integration.test.tsx @@ -39,6 +39,12 @@ vi.mock('@/app/actions/delete-repository', () => ({ deleteRepository: vi.fn().mockResolvedValue({ success: true }), })); +const mockReparentFeature = vi.fn().mockResolvedValue({ success: true }); +vi.mock('@/app/actions/reparent-feature', () => ({ + reparentFeature: (...args: unknown[]) => mockReparentFeature(...args), +})); + +import type { Edge } from '@xyflow/react'; import { ControlCenterInner } from '@/components/features/control-center/control-center-inner'; import { SidebarFeaturesProvider } from '@/hooks/sidebar-features-context'; import { SidebarProvider } from '@/components/ui/sidebar'; @@ -108,12 +114,12 @@ const repoNodeDefault: CanvasNodeType = { const initialNodes: CanvasNodeType[] = [repoNodeDefault, featureNodeA, featureNodeB]; -function renderControlCenter(nodes = initialNodes) { +function renderControlCenter(nodes = initialNodes, edges: Edge[] = []) { return render( - + @@ -406,3 +412,240 @@ describe('ControlCenterInner URL-based navigation', () => { }); }); }); + +/* ------------------------------------------------------------------ */ +/* Canvas reparenting via drag-to-connect & edge deletion */ +/* ------------------------------------------------------------------ */ + +const featParent: CanvasNodeType = { + id: 'feat-parent-1', + type: 'featureNode', + position: { x: 100, y: 100 }, + data: { + name: 'Parent Feature', + description: 'Parent', + featureId: 'parent-1', + lifecycle: 'implementation', + state: 'running', + progress: 40, + repositoryPath: '/home/user/my-repo', + branch: 'feat/parent', + } as FeatureNodeData, +}; + +const featChild: CanvasNodeType = { + id: 'feat-child-1', + type: 'featureNode', + position: { x: 300, y: 100 }, + data: { + name: 'Child Feature', + description: 'Child', + featureId: 'child-1', + lifecycle: 'requirements', + state: 'pending', + progress: 0, + repositoryPath: '/home/user/my-repo', + branch: 'feat/child', + } as FeatureNodeData, +}; + +const featDone: CanvasNodeType = { + id: 'feat-done-1', + type: 'featureNode', + position: { x: 300, y: 300 }, + data: { + name: 'Done Feature', + description: 'Complete', + featureId: 'done-1', + lifecycle: 'review', + state: 'done', + progress: 100, + repositoryPath: '/home/user/my-repo', + branch: 'feat/done', + } as FeatureNodeData, +}; + +const featOtherRepo: CanvasNodeType = { + id: 'feat-other-1', + type: 'featureNode', + position: { x: 500, y: 100 }, + data: { + name: 'Other Repo Feature', + description: 'Different repo', + featureId: 'other-1', + lifecycle: 'implementation', + state: 'running', + progress: 20, + repositoryPath: '/home/user/other-repo', + branch: 'feat/other', + } as FeatureNodeData, +}; + +const reparentNodes: CanvasNodeType[] = [ + repoNodeDefault, + featParent, + featChild, + featDone, + featOtherRepo, +]; + +describe('Canvas reparenting interactions', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockReparentFeature.mockResolvedValue({ success: true }); + }); + + describe('handleConnect (drag-to-connect reparenting)', () => { + it('calls reparentFeature server action for valid same-repo feature connection', async () => { + renderControlCenter(reparentNodes); + + await act(async () => { + capturedCanvasProps.onConnect?.({ + source: 'feat-parent-1', + target: 'feat-child-1', + sourceHandle: null, + targetHandle: null, + }); + }); + + // Server action should be called with stripped feature IDs + expect(mockReparentFeature).toHaveBeenCalledWith('child-1', 'parent-1'); + }); + + it('rejects connections where source is not a feature node', () => { + renderControlCenter(reparentNodes); + + act(() => { + capturedCanvasProps.onConnect?.({ + source: 'repo-default', + target: 'feat-child-1', + sourceHandle: null, + targetHandle: null, + }); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + + it('rejects connections where target is not a feature node', () => { + renderControlCenter(reparentNodes); + + act(() => { + capturedCanvasProps.onConnect?.({ + source: 'feat-parent-1', + target: 'repo-default', + sourceHandle: null, + targetHandle: null, + }); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + + it('rejects self-connections', () => { + renderControlCenter(reparentNodes); + + act(() => { + capturedCanvasProps.onConnect?.({ + source: 'feat-parent-1', + target: 'feat-parent-1', + sourceHandle: null, + targetHandle: null, + }); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + + it('rejects cross-repository connections', () => { + renderControlCenter(reparentNodes); + + act(() => { + capturedCanvasProps.onConnect?.({ + source: 'feat-parent-1', + target: 'feat-other-1', + sourceHandle: null, + targetHandle: null, + }); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + + it('rejects connections to terminal-state features', () => { + renderControlCenter(reparentNodes); + + act(() => { + capturedCanvasProps.onConnect?.({ + source: 'feat-parent-1', + target: 'feat-done-1', + sourceHandle: null, + targetHandle: null, + }); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + }); + + describe('handleEdgesDelete (edge deletion unparenting)', () => { + it('calls reparentFeature with null parent for dependency edge deletion', async () => { + const depEdge: Edge = { + id: 'dep-1', + source: 'feat-parent-1', + target: 'feat-child-1', + type: 'dependencyEdge', + }; + + renderControlCenter(reparentNodes, [depEdge]); + + await act(async () => { + capturedCanvasProps.onEdgesDelete?.([depEdge]); + }); + + expect(mockReparentFeature).toHaveBeenCalledWith('child-1', null); + }); + + it('ignores non-dependency edges (e.g. repo-to-feature)', async () => { + const repoEdge: Edge = { + id: 'repo-edge-1', + source: 'repo-default', + target: 'feat-child-1', + type: 'repoToFeature', + }; + + renderControlCenter(reparentNodes, [repoEdge]); + + await act(async () => { + capturedCanvasProps.onEdgesDelete?.([repoEdge]); + }); + + expect(mockReparentFeature).not.toHaveBeenCalled(); + }); + + it('handles multiple dependency edge deletions at once', async () => { + const depEdge1: Edge = { + id: 'dep-1', + source: 'feat-parent-1', + target: 'feat-child-1', + type: 'dependencyEdge', + }; + const depEdge2: Edge = { + id: 'dep-2', + source: 'feat-parent-1', + target: 'feat-done-1', + type: 'dependencyEdge', + }; + + renderControlCenter(reparentNodes, [depEdge1, depEdge2]); + + await act(async () => { + capturedCanvasProps.onEdgesDelete?.([depEdge1, depEdge2]); + }); + + expect(mockReparentFeature).toHaveBeenCalledTimes(2); + expect(mockReparentFeature).toHaveBeenCalledWith('child-1', null); + expect(mockReparentFeature).toHaveBeenCalledWith('done-1', null); + }); + }); +}); diff --git a/tests/unit/use-cases/features/adopt-branch.use-case.test.ts b/tests/unit/use-cases/features/adopt-branch.use-case.test.ts index 14eebe6ef..05878ae9a 100644 --- a/tests/unit/use-cases/features/adopt-branch.use-case.test.ts +++ b/tests/unit/use-cases/features/adopt-branch.use-case.test.ts @@ -97,6 +97,7 @@ describe('AdoptBranchUseCase', () => { getFailureLogs: vi.fn().mockResolvedValue(''), syncMain: vi.fn().mockResolvedValue(undefined), rebaseOnMain: vi.fn().mockResolvedValue(undefined), + rebaseOnBranch: vi.fn().mockResolvedValue(undefined), getConflictedFiles: vi.fn().mockResolvedValue([]), stageFiles: vi.fn().mockResolvedValue(undefined), rebaseContinue: vi.fn().mockResolvedValue(undefined), diff --git a/tests/unit/use-cases/features/reparent-feature.use-case.test.ts b/tests/unit/use-cases/features/reparent-feature.use-case.test.ts new file mode 100644 index 000000000..49bae2d7f --- /dev/null +++ b/tests/unit/use-cases/features/reparent-feature.use-case.test.ts @@ -0,0 +1,311 @@ +import 'reflect-metadata'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { SdlcLifecycle } from '@/domain/generated/output.js'; +import type { IFeatureRepository } from '@/application/ports/output/repositories/feature-repository.interface.js'; +import type { Feature } from '@/domain/generated/output.js'; +import { ReparentFeatureUseCase } from '@/application/use-cases/features/reparent-feature.use-case.js'; +import type { CheckAndUnblockFeaturesUseCase } from '@/application/use-cases/features/check-and-unblock-features.use-case.js'; + +function makeFeature(overrides: Partial = {}): Feature { + return { + id: 'feat-child', + name: 'Child Feature', + userQuery: 'test', + slug: 'child-feature', + description: 'test desc', + repositoryPath: '/repo/path', + branch: 'feat/child', + lifecycle: SdlcLifecycle.Started, + messages: [], + relatedArtifacts: [], + fast: false, + push: false, + openPr: false, + forkAndPr: false, + commitSpecs: false, + ciWatchEnabled: false, + enableEvidence: false, + commitEvidence: false, + approvalGates: { requireApproval: false }, + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + } as Feature; +} + +describe('ReparentFeatureUseCase', () => { + let mockFeatureRepo: IFeatureRepository; + let mockCheckAndUnblock: CheckAndUnblockFeaturesUseCase; + let useCase: ReparentFeatureUseCase; + + beforeEach(() => { + mockFeatureRepo = { + create: vi.fn().mockResolvedValue(undefined), + findById: vi.fn().mockResolvedValue(null), + findByIdPrefix: vi.fn().mockResolvedValue(null), + findBySlug: vi.fn().mockResolvedValue(null), + findByBranch: vi.fn().mockResolvedValue(null), + list: vi.fn().mockResolvedValue([]), + update: vi.fn().mockResolvedValue(undefined), + findByParentId: vi.fn().mockResolvedValue([]), + delete: vi.fn().mockResolvedValue(undefined), + softDelete: vi.fn().mockResolvedValue(undefined), + }; + + mockCheckAndUnblock = { + execute: vi.fn().mockResolvedValue(undefined), + } as unknown as CheckAndUnblockFeaturesUseCase; + + useCase = new ReparentFeatureUseCase(mockFeatureRepo, mockCheckAndUnblock); + }); + + // --- Task 1: Core validation logic --- + + it('should update parentId on successful reparent', async () => { + const child = makeFeature({ id: 'child-1', parentId: undefined }); + const parent = makeFeature({ + id: 'parent-1', + lifecycle: SdlcLifecycle.Implementation, + }); + vi.mocked(mockFeatureRepo.findById) + .mockResolvedValueOnce(child) // load child + .mockResolvedValueOnce(parent); // load parent + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + parentId: 'parent-1', + }) + ); + }); + + it('should reject self-reparent', async () => { + await expect(useCase.execute({ featureId: 'feat-1', parentId: 'feat-1' })).rejects.toThrow( + /cannot.*parent.*itself/i + ); + }); + + it('should reject reparent to non-existent parent', async () => { + const child = makeFeature({ id: 'child-1' }); + vi.mocked(mockFeatureRepo.findById) + .mockResolvedValueOnce(child) // load child + .mockResolvedValueOnce(null); // parent not found + + await expect( + useCase.execute({ featureId: 'child-1', parentId: 'nonexistent' }) + ).rejects.toThrow(/parent feature not found/i); + }); + + it('should reject reparent of non-existent child', async () => { + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(null); + + await expect( + useCase.execute({ featureId: 'nonexistent', parentId: 'parent-1' }) + ).rejects.toThrow(/feature not found/i); + }); + + it('should reject cross-repo reparent', async () => { + const child = makeFeature({ id: 'child-1', repositoryPath: '/repo/a' }); + const parent = makeFeature({ id: 'parent-1', repositoryPath: '/repo/b' }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await expect(useCase.execute({ featureId: 'child-1', parentId: 'parent-1' })).rejects.toThrow( + /same repository/i + ); + }); + + it('should reject direct cycle (A parent of B, try to reparent A under B)', async () => { + const childA = makeFeature({ id: 'A', parentId: undefined }); + const parentB = makeFeature({ id: 'B', parentId: 'A' }); + vi.mocked(mockFeatureRepo.findById) + .mockResolvedValueOnce(childA) // execute: load child A + .mockResolvedValueOnce(parentB) // execute: load parent B + .mockResolvedValueOnce(parentB); // detectCycle: load B (cursor=B), B.parentId=A -> cycle! + + await expect(useCase.execute({ featureId: 'A', parentId: 'B' })).rejects.toThrow(/cycle/i); + }); + + it('should reject indirect cycle (A->B->C chain, reparent A under C)', async () => { + const childA = makeFeature({ id: 'A' }); + const parentC = makeFeature({ id: 'C', parentId: 'B' }); + const featureB = makeFeature({ id: 'B', parentId: 'A' }); + vi.mocked(mockFeatureRepo.findById) + .mockResolvedValueOnce(childA) // execute: load child A + .mockResolvedValueOnce(parentC) // execute: load parent C + .mockResolvedValueOnce(parentC) // detectCycle: load C (cursor=C), C.parentId=B + .mockResolvedValueOnce(featureB); // detectCycle: load B (cursor=B), B.parentId=A -> cycle! + + await expect(useCase.execute({ featureId: 'A', parentId: 'C' })).rejects.toThrow(/cycle/i); + }); + + it('should reject reparent of Archived feature', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Archived }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await expect(useCase.execute({ featureId: 'child-1', parentId: 'parent-1' })).rejects.toThrow( + /cannot.*reparent/i + ); + }); + + it('should reject reparent of Maintain (done) feature', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Maintain }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await expect(useCase.execute({ featureId: 'child-1', parentId: 'parent-1' })).rejects.toThrow( + /cannot.*reparent/i + ); + }); + + it('should reject reparent of Deleting feature', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Deleting }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await expect(useCase.execute({ featureId: 'child-1', parentId: 'parent-1' })).rejects.toThrow( + /cannot.*reparent/i + ); + }); + + it('should clear parentId on unparent (parentId=null)', async () => { + const child = makeFeature({ id: 'child-1', parentId: 'old-parent' }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await useCase.execute({ featureId: 'child-1', parentId: null }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + parentId: undefined, + }) + ); + }); + + it('should allow reparenting features in active lifecycle states', async () => { + for (const lifecycle of [ + SdlcLifecycle.Started, + SdlcLifecycle.Blocked, + SdlcLifecycle.Pending, + SdlcLifecycle.Requirements, + SdlcLifecycle.Implementation, + ]) { + const child = makeFeature({ id: 'child-1', lifecycle }); + const parent = makeFeature({ + id: 'parent-1', + lifecycle: SdlcLifecycle.Implementation, + }); + vi.mocked(mockFeatureRepo.findById) + .mockResolvedValueOnce(child) + .mockResolvedValueOnce(parent); + vi.mocked(mockFeatureRepo.update).mockClear(); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockFeatureRepo.update).toHaveBeenCalled(); + } + }); + + // --- Task 2: Lifecycle state adjustment --- + + it('should transition Started child to Blocked when reparented under Pending parent', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Started }); + const parent = makeFeature({ id: 'parent-1', lifecycle: SdlcLifecycle.Pending }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + parentId: 'parent-1', + lifecycle: SdlcLifecycle.Blocked, + }) + ); + }); + + it('should keep child lifecycle when reparented under Implementation parent', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Started }); + const parent = makeFeature({ id: 'parent-1', lifecycle: SdlcLifecycle.Implementation }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + lifecycle: SdlcLifecycle.Started, + }) + ); + }); + + it('should transition Blocked child to Started when unparented', async () => { + const child = makeFeature({ + id: 'child-1', + lifecycle: SdlcLifecycle.Blocked, + parentId: 'old-parent', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await useCase.execute({ featureId: 'child-1', parentId: null }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + lifecycle: SdlcLifecycle.Started, + }) + ); + }); + + it('should keep Pending lifecycle when unparented', async () => { + const child = makeFeature({ + id: 'child-1', + lifecycle: SdlcLifecycle.Pending, + parentId: 'old-parent', + }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child); + + await useCase.execute({ featureId: 'child-1', parentId: null }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + lifecycle: SdlcLifecycle.Pending, + }) + ); + }); + + it('should call CheckAndUnblockFeaturesUseCase when reparented under post-implementation parent', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Blocked }); + const parent = makeFeature({ id: 'parent-1', lifecycle: SdlcLifecycle.Implementation }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockCheckAndUnblock.execute).toHaveBeenCalledWith('child-1'); + }); + + it('should NOT call CheckAndUnblockFeaturesUseCase when reparented under pre-implementation parent', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Started }); + const parent = makeFeature({ id: 'parent-1', lifecycle: SdlcLifecycle.Requirements }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockCheckAndUnblock.execute).not.toHaveBeenCalled(); + }); + + it('should transition active child to Blocked when reparented under Blocked parent', async () => { + const child = makeFeature({ id: 'child-1', lifecycle: SdlcLifecycle.Requirements }); + const parent = makeFeature({ id: 'parent-1', lifecycle: SdlcLifecycle.Blocked }); + vi.mocked(mockFeatureRepo.findById).mockResolvedValueOnce(child).mockResolvedValueOnce(parent); + + await useCase.execute({ featureId: 'child-1', parentId: 'parent-1' }); + + expect(mockFeatureRepo.update).toHaveBeenCalledWith( + expect.objectContaining({ + id: 'child-1', + lifecycle: SdlcLifecycle.Blocked, + }) + ); + }); +});