Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/long-bars-slide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"agentweaver": patch
---

Fixed run detail state staying stale after confirming an outcome plan, and stopped a retry error loop on transient not-ready responses.
15 changes: 13 additions & 2 deletions apps/Agentweaver.Api/Endpoints/CoordinatorEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public static void MapCoordinatorEndpoints(this WebApplication app)
// -----------------------------------------------------------------------

// GET /api/runs/{coordinatorRunId}/work-plan — the persisted work plan (subtasks + dependencies)
// for a coordinator run. 404 when the run is not a coordinator run / has no work plan yet.
// for a coordinator run. An active coordinator without a plan returns the typed
// work_plan_not_ready 404 while asynchronous decomposition is still in progress.
app.MapGet("/api/runs/{coordinatorRunId}/work-plan", GetCoordinatorWorkPlanAsync)
.WithName("GetCoordinatorWorkPlan")
.WithTags("Coordinator")
Expand Down Expand Up @@ -502,7 +503,17 @@ public static async Task<IResult> GetCoordinatorWorkPlanAsync(
return ForbiddenError();

var plan = await ReadWorkPlanWithBriefWaitAsync(coordinator, coordinatorRunId, ct);
if (plan is null) return NotFoundError("work_plan_not_found", "The coordinator work plan was not found.");
if (plan is null)
{
var outcomeSpec = await coordinator.GetOutcomeSpecAsync(coordinatorRunId, ct);
var awaitingMaterialization = run.ParentRunId is null
&& string.Equals(run.AgentName, "Coordinator", StringComparison.Ordinal)
&& !EndpointHelpers.IsTerminal(run.Status)
&& string.Equals(outcomeSpec?.Status, "confirmed", StringComparison.Ordinal);
return awaitingMaterialization
? NotFoundError("work_plan_not_ready", "The coordinator work plan is still being created.")
: NotFoundError("work_plan_not_found", "The coordinator work plan was not found.");
}

return Results.Json(MapWorkPlan(plan));
}
Expand Down
12 changes: 10 additions & 2 deletions apps/Agentweaver.Api/Endpoints/RunEndpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,11 @@ await revisionStore.InsertRevisionAsync(
if (run.Status is RunStatus.Pending)
return Results.Json(Array.Empty<WorkspaceNode>());

// An active run can become visible before asynchronous worktree provisioning
// writes its path. This is an empty workspace, not a missing artifact source.
if (run.Status is RunStatus.InProgress && string.IsNullOrEmpty(run.WorktreePath))
return Results.Json(Array.Empty<WorkspaceNode>());

// Merged runs: enumerate the commit tree from git (worktree has been deleted).
if (run.Status is RunStatus.Merged)
{
Expand Down Expand Up @@ -2292,8 +2297,11 @@ await EnsureProvisionalAgentHostScopeClosedAsync(

if (!hasWorktreePath || !hasWorktreeBranch)
{
logger.LogError("Run {RunId} has incomplete worktree metadata while retrieving file entries", runId);
return Results.Problem("Run worktree metadata is incomplete.", statusCode: StatusCodes.Status500InternalServerError);
// Worktree fields are persisted independently while a child run starts.
// Treat a partial snapshot as no artifacts yet, not as a server failure that
// causes the live artifact browser to retry aggressively.
logger.LogDebug("Run {RunId} worktree metadata is not ready while retrieving file entries", runId);
return Results.Json(Array.Empty<WorkspaceFileEntry>());
}

if (!Directory.Exists(run.WorktreePath!))
Expand Down
54 changes: 54 additions & 0 deletions apps/web/src/__tests__/ArtifactBrowser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,60 @@ describe('ArtifactBrowser', () => {
expect(getRunFilesMock()).toHaveBeenCalledTimes(2);
});

it('stops polling a removed artifact source instead of retrying its 404 response', async () => {
vi.useFakeTimers();
getRunFilesMock().mockRejectedValue(new ApiError(404, 'run not found'));

render(
<Wrapper>
<ArtifactBrowser runId="gone-run" runStatus="in_progress" />
</Wrapper>,
);

await vi.waitFor(() => expect(getRunFilesMock()).toHaveBeenCalledTimes(1));
await act(async () => { await vi.advanceTimersByTimeAsync(60_000); });
expect(getRunFilesMock()).toHaveBeenCalledTimes(1);
});

it('stops artifact polling after bounded retries for a persistent server error', async () => {
vi.useFakeTimers();
getRunFilesMock().mockRejectedValue(new ApiError(500, 'persistent server failure'));

render(
<Wrapper>
<ArtifactBrowser runId="failing-run" runStatus="in_progress" />
</Wrapper>,
);

await vi.waitFor(() => expect(getRunFilesMock()).toHaveBeenCalledTimes(1));
await act(async () => { await vi.advanceTimersByTimeAsync(60_000); });
expect(getRunFilesMock()).toHaveBeenCalledTimes(3);
expect(document.body.textContent).toContain('persistent server failure');
});

it('stops Files-tab polling for a genuinely missing workspace source', async () => {
vi.useFakeTimers();
getRunFilesMock().mockResolvedValue([]);
getRunWorkspaceMock().mockRejectedValue(new ApiError(404, 'run not found'));

renderHook(() =>
useArtifactBrowser(
'gone-workspace-run',
'in_progress',
undefined,
undefined,
undefined,
undefined,
undefined,
'files',
),
);

await vi.waitFor(() => expect(getRunWorkspaceMock()).toHaveBeenCalledTimes(1));
await act(async () => { await vi.advanceTimersByTimeAsync(60_000); });
expect(getRunWorkspaceMock()).toHaveBeenCalledTimes(1);
});

// AB-01b: at a review gate, an empty file list means the run reached review with zero
// committed changes — surface a clear explanation, not a bare "No changes" label.
it('explains "no changes produced" when the file list is empty at a review gate', async () => {
Expand Down
3 changes: 3 additions & 0 deletions apps/web/src/__tests__/OutcomePlanPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ describe('OutcomePlanPanel confirm retry', () => {
it('keeps the REST confirmed status when stale SSE still says awaiting confirmation after confirm', async () => {
vi.mocked(apiClient.confirmOutcomeSpec).mockResolvedValue(confirmedSpec);
const onReconnect = vi.fn();
const onConfirmed = vi.fn();

render(
<Wrapper>
Expand All @@ -144,6 +145,7 @@ describe('OutcomePlanPanel confirm retry', () => {
events={[staleAwaitingEvent]}
streamStatus="streaming"
onReconnect={onReconnect}
onConfirmed={onConfirmed}
/>
</Wrapper>,
);
Expand All @@ -154,6 +156,7 @@ describe('OutcomePlanPanel confirm retry', () => {
expect(screen.getByText(/Outcome plan confirmed by Ahmed/i)).toBeTruthy();
expect(screen.queryByRole('button', { name: /confirm plan/i })).toBeNull();
expect(onReconnect).toHaveBeenCalledTimes(1);
expect(onConfirmed).toHaveBeenCalledTimes(1);
});

it('shows the updated interrupted message for run_not_active failures', async () => {
Expand Down
7 changes: 6 additions & 1 deletion apps/web/src/components/OutcomePlanPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ interface OutcomePlanPanelProps {
runStatus?: string;
onCollapse?: () => void;
onReconnect?: () => void;
/** Reconcile the parent run snapshot after confirmation changes coordinator state. */
onConfirmed?: () => void;
onClarifyPlan?: () => void;
clarificationSent?: boolean;
/**
Expand All @@ -236,7 +238,7 @@ interface OutcomePlanPanelProps {
onFooterChange?: (node: ReactNode) => void;
}

export function OutcomePlanPanel({ runId, events, streamStatus, runStatus, onCollapse, onReconnect, onClarifyPlan, clarificationSent = false, onFooterChange }: OutcomePlanPanelProps) {
export function OutcomePlanPanel({ runId, events, streamStatus, runStatus, onCollapse, onReconnect, onConfirmed, onClarifyPlan, clarificationSent = false, onFooterChange }: OutcomePlanPanelProps) {
const styles = useStyles();

const [specFromApi, setSpecFromApi] = useState<OutcomeSpec | null>(null);
Expand Down Expand Up @@ -381,6 +383,9 @@ export function OutcomePlanPanel({ runId, events, streamStatus, runStatus, onCol
const updated = await apiClient.confirmOutcomeSpec(runId, allowTaskPromotion);
if (updated) setSpecFromApi(updated);
else await fetchSpec();
// The parent derives its header/tree from separate REST snapshots, rather
// than this panel's spec state. Refresh those snapshots immediately.
onConfirmed?.();
// Reconnect the SSE stream so post-confirmation events (outcome_spec.confirmed,
// coordinator work plan, subtask events) arrive without a manual page refresh.
onReconnect?.();
Expand Down
35 changes: 30 additions & 5 deletions apps/web/src/hooks/useArtifactBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
} from '../api/types';
const POLL_INTERVAL_MS = 3000;
const RETRY_POLL_INTERVAL_MS = 15000;
const MAX_SERVER_ERROR_RETRIES = 2;

export const FILTERS = [
{ label: 'All', value: 'all' },
Expand Down Expand Up @@ -166,6 +167,7 @@ export function useArtifactBrowser(
let active = true;
let intervalId: ReturnType<typeof setInterval> | undefined;
let requestInFlight = false;
let serverErrorRetries = 0;

const startPolling = (intervalMs: number) => {
if (intervalId !== undefined) clearInterval(intervalId);
Expand All @@ -178,25 +180,36 @@ export function useArtifactBrowser(
(adapter?.getFiles ?? apiClient.getRunFiles.bind(apiClient))(runId, activeFilter)
.then((data) => {
if (active) {
serverErrorRetries = 0;
setFiles(data);
setFilesError(null);
setFilesLoading(false);
}
})
.catch((err: unknown) => {
if (active) {
if (err instanceof ApiError && err.status === 409) {
setFilesError('Workspace files unavailable for this run state.');
if (err instanceof ApiError && (err.status === 409 || err.status === 404)) {
setFilesError(err.status === 409
? 'Workspace files unavailable for this run state.'
: extractErrorMessage(err));
setFilesLoading(false);
// 409 is permanent — stop polling
// 404/409 identify a gone or permanently unavailable artifact source.
// Transient provisioning is represented by the API as an empty list.
clearInterval(intervalId);
active = false;
} else {
setFilesError(extractErrorMessage(err));
setFilesLoading(false);
// Keep the error visible, but do not hammer a failing server every three seconds.
// A new live event/filter change restarts this effect and retries immediately.
if (err instanceof ApiError && err.status >= 500) startPolling(RETRY_POLL_INTERVAL_MS);
if (err instanceof ApiError && err.status >= 500) {
if (serverErrorRetries++ >= MAX_SERVER_ERROR_RETRIES) {
clearInterval(intervalId);
active = false;
} else {
startPolling(RETRY_POLL_INTERVAL_MS);
}
}
}
}
})
Expand Down Expand Up @@ -232,6 +245,7 @@ export function useArtifactBrowser(
let active = true;
let workspaceIntervalId: ReturnType<typeof setInterval> | undefined;
let requestInFlight = false;
let serverErrorRetries = 0;

const startPolling = (intervalMs: number) => {
if (workspaceIntervalId !== undefined) clearInterval(workspaceIntervalId);
Expand All @@ -244,6 +258,7 @@ export function useArtifactBrowser(
(adapter?.getWorkspace ?? apiClient.getRunWorkspace.bind(apiClient))(runId)
.then((data) => {
if (active) {
serverErrorRetries = 0;
setWorkspaceFiles(data);
setWorkspaceError(null);
setWorkspaceLoading(false);
Expand All @@ -253,7 +268,17 @@ export function useArtifactBrowser(
if (active) {
setWorkspaceError(extractErrorMessage(err));
setWorkspaceLoading(false);
if (err instanceof ApiError && err.status >= 500) startPolling(RETRY_POLL_INTERVAL_MS);
if (err instanceof ApiError && (err.status === 404 || err.status === 409)) {
clearInterval(workspaceIntervalId);
active = false;
} else if (err instanceof ApiError && err.status >= 500) {
if (serverErrorRetries++ >= MAX_SERVER_ERROR_RETRIES) {
clearInterval(workspaceIntervalId);
active = false;
} else {
startPolling(RETRY_POLL_INTERVAL_MS);
}
}
}
})
.finally(() => { requestInFlight = false; });
Expand Down
22 changes: 21 additions & 1 deletion apps/web/src/pages/CoordinatorRunPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,12 @@ function readStr(p: Record<string, unknown>, keys: string[]): string | undefined
return undefined;
}

function apiErrorCode(err: unknown): string | undefined {
if (!(err instanceof ApiError) || typeof err.payload !== 'object' || err.payload === null) return undefined;
const error = (err.payload as Record<string, unknown>).error;
return typeof error === 'string' ? error : undefined;
}

function readEventTimestamp(p: Record<string, unknown>): string | undefined {
return readStr(p, ['timestamp_utc', 'timestampUtc', 'updated_at', 'updatedAt', 'timestamp']);
}
Expand Down Expand Up @@ -2433,6 +2439,7 @@ export function CoordinatorRunPage() {
if (!runId) return;
let cancelled = false;
let timer: ReturnType<typeof setTimeout> | undefined;
let consecutiveWorkPlanNotReady = 0;
const TERMINAL = new Set<OrchPhase>(['complete', 'failed', 'blocked', 'declined']);
queueMicrotask(() => {
setRunLoadError(null);
Expand Down Expand Up @@ -2468,6 +2475,7 @@ export function CoordinatorRunPage() {
setIsChildRun(childRun);
let wp: WorkPlanResponse | null = null;
let workPlanFailed = false;
let workPlanNotReady = false;
if (!childRun) {
try {
wp = await apiClient.getWorkPlan(runId);
Expand All @@ -2482,6 +2490,9 @@ export function CoordinatorRunPage() {
if (err instanceof ApiError && err.status === 404) {
setNoWorkPlan(true);
setWorkPlanError(null);
// A confirmed coordinator persists its work plan asynchronously. This
// typed 404 is expected briefly; exponentially back off until it exists.
workPlanNotReady = apiErrorCode(err) === 'work_plan_not_ready';
} else {
workPlanFailed = true;
setWorkPlanError(formatApiError(err, 'The work plan could not be loaded.'));
Expand All @@ -2501,6 +2512,7 @@ export function CoordinatorRunPage() {
setCoordinatorSteerable(typeof detail?.coordinator_steerable === 'boolean' ? detail.coordinator_steerable : undefined);
setWorkPlanStatus(wpStatus);
setRunLevelStatus(detail?.status ?? undefined);
if (wp) consecutiveWorkPlanNotReady = 0;
// Seed the option toggles once from the run detail; subsequent user toggles own the state.
if (!seededToggles.current && detail) {
setAutopilot(Boolean(detail.autopilot));
Expand All @@ -2513,7 +2525,14 @@ export function CoordinatorRunPage() {
const phase = normalizePhase(statusField) !== 'unknown'
? normalizePhase(statusField)
: normalizePhase(wpStatus);
if (!TERMINAL.has(phase)) timer = setTimeout(() => { void tick(); }, workPlanFailed ? 8000 : 4000);
if (!TERMINAL.has(phase)) {
const delay = workPlanNotReady
? Math.min(30_000, 8_000 * (2 ** consecutiveWorkPlanNotReady++))
: workPlanFailed
? 8_000
: 4_000;
timer = setTimeout(() => { void tick(); }, delay);
}
};

void tick();
Expand Down Expand Up @@ -4838,6 +4857,7 @@ export function CoordinatorRunPage() {
runStatus={runLevelStatus}
onCollapse={() => setPlanPanelOpen(false)}
onReconnect={reconnectStream}
onConfirmed={() => setRetryRefreshNonce((value) => value + 1)}
onClarifyPlan={() => { setPlanPanelOpen(false); focusOutcomePlanComposer(); }}
clarificationSent={outcomePlanClarifying}
onFooterChange={setPlanFooter}
Expand Down
24 changes: 20 additions & 4 deletions tests/Agentweaver.Tests/ArtifactFilesEndpointTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,21 @@ public async Task InProgressRun_WithoutWorktreeMetadata_Returns200WithEmptyArray
}

[Fact]
public async Task InProgressRun_WithIncompleteWorktreeMetadata_ReturnsServerError()
public async Task InProgressRun_WithoutWorktreePath_ReturnsEmptyWorkspaceWhileProvisioning()
{
var runId = await InsertOwnerRunAsync(RunStatus.InProgress);

var response = await _ownerClient.GetAsync($"/api/runs/{runId}/workspace");

response.StatusCode.Should().Be(HttpStatusCode.OK,
"an active run may be visible before asynchronous worktree provisioning completes");
var workspace = await response.Content.ReadFromJsonAsync<JsonElement>();
workspace.ValueKind.Should().Be(JsonValueKind.Array);
workspace.GetArrayLength().Should().Be(0);
}

[Fact]
public async Task InProgressRun_WithIncompleteWorktreeMetadata_ReturnsEmptyArrayWhileProvisioning()
{
const string persistedPath = @"C:\worktrees\incomplete-run";
var runId = await InsertOwnerRunAsync(
Expand All @@ -160,9 +174,11 @@ public async Task InProgressRun_WithIncompleteWorktreeMetadata_ReturnsServerErro

var response = await _ownerClient.GetAsync($"/api/runs/{runId}/files?filter=all");

response.StatusCode.Should().Be(HttpStatusCode.InternalServerError,
"a partially persisted worktree is corrupt metadata, not an unprovisioned run");
(await response.Content.ReadAsStringAsync()).Should().NotContain(persistedPath);
response.StatusCode.Should().Be(HttpStatusCode.OK,
"worktree metadata is persisted asynchronously and must not surface as a retryable server error");
var files = await response.Content.ReadFromJsonAsync<JsonElement>();
files.ValueKind.Should().Be(JsonValueKind.Array);
files.GetArrayLength().Should().Be(0);
}

[Fact]
Expand Down
Loading
Loading