tas/Codewalkers
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Fix review task completion bug + add initiative-level Request Changes

Browse Source 
Critical: review/merge tasks hit an early return in handleTaskCompleted()
that skipped the phase completion check, leaving phases stuck in
in_progress forever. Changed to an if-block wrapping only the merge step.

Also adds requestChangesOnInitiative() which creates/reuses a
"Finalization" phase for initiative-level review feedback, with dedup
guards for both phase and initiative request-changes flows.
This commit is contained in:
Lukas May
2026-03-06 09:41:28 +01:00
parent 1b8e496d39
commit 65bcbf1a35
6 changed files with 152 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
apps/server/events/index.ts
View File

@@ -52,6 +52,7 @@ export type {
AccountCredentialsValidatedEvent,
InitiativePendingReviewEvent,
InitiativeReviewApprovedEvent,
InitiativeChangesRequestedEvent,
DomainEventMap,
DomainEventType,
} from './types.js';

12
apps/server/events/types.ts
View File

@@ -591,6 +591,15 @@ export interface InitiativeReviewApprovedEvent extends DomainEvent {
};
}
export interface InitiativeChangesRequestedEvent extends DomainEvent {
type: 'initiative:changes_requested';
payload: {
initiativeId: string;
phaseId: string;
taskId: string;
};
}
/**
* Chat Session Events
*/
@@ -668,7 +677,8 @@ export type DomainEventMap =
| ChatMessageCreatedEvent
| ChatSessionClosedEvent
| InitiativePendingReviewEvent
| InitiativeReviewApprovedEvent;
| InitiativeReviewApprovedEvent
| InitiativeChangesRequestedEvent;
/**
* Event type literal union for type checking

92
apps/server/execution/orchestrator.ts
View File

@@ -11,7 +11,7 @@
* - Review per-phase: pause after each phase for diff review
*/
import type { EventBus, TaskCompletedEvent, PhasePendingReviewEvent, PhaseChangesRequestedEvent, PhaseMergedEvent, TaskMergedEvent, PhaseQueuedEvent, AgentStoppedEvent, InitiativePendingReviewEvent, InitiativeReviewApprovedEvent } from '../events/index.js';
import type { EventBus, TaskCompletedEvent, PhasePendingReviewEvent, PhaseChangesRequestedEvent, PhaseMergedEvent, TaskMergedEvent, PhaseQueuedEvent, AgentStoppedEvent, InitiativePendingReviewEvent, InitiativeReviewApprovedEvent, InitiativeChangesRequestedEvent } from '../events/index.js';
import type { BranchManager } from '../git/branch-manager.js';
import type { PhaseRepository } from '../db/repositories/phase-repository.js';
import type { TaskRepository } from '../db/repositories/task-repository.js';
@@ -150,9 +150,8 @@ export class ExecutionOrchestrator {
const phase = await this.phaseRepository.findById(task.phaseId);
if (!phase) return;
// Skip merge/review tasks — they already work on the phase branch directly
if (task.category === 'merge' || task.category === 'review') return;
// Skip merge for review/merge tasks — they already work on the phase branch directly
if (task.category !== 'merge' && task.category !== 'review') {
const initBranch = initiative.branch;
const phBranch = phaseBranchName(initBranch, phase.name);
const tBranch = taskBranchName(initBranch, task.id);
@@ -164,6 +163,7 @@ export class ExecutionOrchestrator {
});
this.phaseMergeLocks.set(task.phaseId, mergeOp.catch(() => {}));
await mergeOp;
}
// Check if all phase tasks are done
const phaseTasks = await this.taskRepository.findByPhaseId(task.phaseId);
@@ -356,6 +356,15 @@ export class ExecutionOrchestrator {
const initiative = await this.initiativeRepository.findById(phase.initiativeId);
if (!initiative) throw new Error(`Initiative not found: ${phase.initiativeId}`);
// Guard: don't create duplicate review tasks
const existingTasks = await this.taskRepository.findByPhaseId(phaseId);
const activeReview = existingTasks.find(
(t) => t.category === 'review' && (t.status === 'pending' || t.status === 'in_progress'),
);
if (activeReview) {
return { taskId: activeReview.id };
}
// Build revision task description from comments + summary
const lines: string[] = [];
if (summary) {
@@ -418,6 +427,81 @@ export class ExecutionOrchestrator {
return { taskId: task.id };
}
/**
* Request changes on an initiative that's pending review.
* Creates/reuses a "Finalization" phase and adds a review task to it.
*/
async requestChangesOnInitiative(
initiativeId: string,
summary: string,
): Promise<{ taskId: string }> {
const initiative = await this.initiativeRepository.findById(initiativeId);
if (!initiative) throw new Error(`Initiative not found: ${initiativeId}`);
if (initiative.status !== 'pending_review') {
throw new Error(`Initiative ${initiativeId} is not pending review (status: ${initiative.status})`);
}
// Find or create a "Finalization" phase
const phases = await this.phaseRepository.findByInitiativeId(initiativeId);
let finalizationPhase = phases.find((p) => p.name === 'Finalization');
if (!finalizationPhase) {
finalizationPhase = await this.phaseRepository.create({
initiativeId,
name: 'Finalization',
status: 'in_progress',
});
} else if (finalizationPhase.status === 'completed' || finalizationPhase.status === 'pending_review') {
await this.phaseRepository.update(finalizationPhase.id, { status: 'in_progress' as any });
}
// Guard: don't create duplicate review tasks
const existingTasks = await this.taskRepository.findByPhaseId(finalizationPhase.id);
const activeReview = existingTasks.find(
(t) => t.category === 'review' && (t.status === 'pending' || t.status === 'in_progress'),
);
if (activeReview) {
// Still reset initiative to active
await this.initiativeRepository.update(initiativeId, { status: 'active' as any });
this.scheduleDispatch();
return { taskId: activeReview.id };
}
// Create review task
const task = await this.taskRepository.create({
phaseId: finalizationPhase.id,
initiativeId,
name: `Address initiative review feedback`,
description: `## Summary\n\n${summary}`,
category: 'review',
priority: 'high',
});
// Reset initiative status to active
await this.initiativeRepository.update(initiativeId, { status: 'active' as any });
// Queue task for dispatch
await this.dispatchManager.queue(task.id);
// Emit event
const event: InitiativeChangesRequestedEvent = {
type: 'initiative:changes_requested',
timestamp: new Date(),
payload: {
initiativeId,
phaseId: finalizationPhase.id,
taskId: task.id,
},
};
this.eventBus.emit(event);
log.info({ initiativeId, phaseId: finalizationPhase.id, taskId: task.id }, 'changes requested on initiative');
this.scheduleDispatch();
return { taskId: task.id };
}
/**
* Re-queue approved phases for an initiative into the in-memory dispatch queue.
* Self-healing: ensures phases aren't lost if the server restarted since the

14
apps/server/trpc/routers/initiative.ts
View File

@@ -335,5 +335,19 @@ export function initiativeProcedures(publicProcedure: ProcedureBuilder) {
await orchestrator.approveInitiative(input.initiativeId, input.strategy);
return { success: true };
}),
requestInitiativeChanges: publicProcedure
.input(z.object({
initiativeId: z.string().min(1),
summary: z.string().trim().min(1),
}))
.mutation(async ({ ctx, input }) => {
const orchestrator = requireExecutionOrchestrator(ctx);
const result = await orchestrator.requestChangesOnInitiative(
input.initiativeId,
input.summary,
);
return { success: true, taskId: result.taskId };
}),
};
}

2
apps/server/trpc/routers/phase.ts
View File

@@ -371,7 +371,7 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) {
requestPhaseChanges: publicProcedure
.input(z.object({
phaseId: z.string().min(1),
summary: z.string().optional(),
summary: z.string().trim().min(1).optional(),
}))
.mutation(async ({ ctx, input }) => {
const orchestrator = requireExecutionOrchestrator(ctx);

28
apps/web/src/components/review/InitiativeReview.tsx
View File

@@ -1,6 +1,6 @@
import { useCallback, useMemo, useRef, useState } from "react";
import { toast } from "sonner";
import { Loader2, GitBranch, ArrowRight, FileCode, Plus, Minus, Upload, GitMerge } from "lucide-react";
import { Loader2, GitBranch, ArrowRight, FileCode, Plus, Minus, Upload, GitMerge, RotateCcw } from "lucide-react";
import { Button } from "@/components/ui/button";
import { trpc } from "@/lib/trpc";
import { parseUnifiedDiff } from "./parse-diff";
@@ -82,6 +82,14 @@ export function InitiativeReview({ initiativeId, onCompleted }: InitiativeReview
onError: (err) => toast.error(`Failed to stop: ${err.message}`),
});
const requestChangesMutation = trpc.requestInitiativeChanges.useMutation({
onSuccess: () => {
toast.success("Changes requested — review task created");
onCompleted();
},
onError: (err) => toast.error(err.message),
});
const approveMutation = trpc.approveInitiativeReview.useMutation({
onSuccess: (_data, variables) => {
const msg = variables.strategy === "merge_and_push"
@@ -189,6 +197,24 @@ export function InitiativeReview({ initiativeId, onCompleted }: InitiativeReview
{/* Right: preview + action buttons */}
<div className="flex items-center gap-2 shrink-0">
{previewState && <PreviewControls preview={previewState} />}
<Button
variant="outline"
size="sm"
onClick={() => {
const summary = window.prompt("Describe the changes needed:");
if (!summary?.trim()) return;
requestChangesMutation.mutate({ initiativeId, summary: summary.trim() });
}}
disabled={requestChangesMutation.isPending}
className="h-8 text-xs px-3 text-status-error-fg border-status-error-border hover:bg-status-error-bg"
>
{requestChangesMutation.isPending ? (
<Loader2 className="h-3 w-3 animate-spin" />
) : (
<RotateCcw className="h-3 w-3" />
)}
Request Changes
</Button>
<Button
variant="outline"
size="sm"