diff --git a/.changeset/long-bars-slide.md b/.changeset/long-bars-slide.md new file mode 100644 index 000000000..c370198f4 --- /dev/null +++ b/.changeset/long-bars-slide.md @@ -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. diff --git a/apps/Agentweaver.Api/Endpoints/CoordinatorEndpoints.cs b/apps/Agentweaver.Api/Endpoints/CoordinatorEndpoints.cs index 485d251f0..c292b69e6 100644 --- a/apps/Agentweaver.Api/Endpoints/CoordinatorEndpoints.cs +++ b/apps/Agentweaver.Api/Endpoints/CoordinatorEndpoints.cs @@ -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") @@ -502,7 +503,17 @@ public static async Task 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)); } diff --git a/apps/Agentweaver.Api/Endpoints/RunEndpoints.cs b/apps/Agentweaver.Api/Endpoints/RunEndpoints.cs index a78495ca3..7020457f6 100644 --- a/apps/Agentweaver.Api/Endpoints/RunEndpoints.cs +++ b/apps/Agentweaver.Api/Endpoints/RunEndpoints.cs @@ -1488,6 +1488,11 @@ await revisionStore.InsertRevisionAsync( if (run.Status is RunStatus.Pending) return Results.Json(Array.Empty()); + // 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()); + // Merged runs: enumerate the commit tree from git (worktree has been deleted). if (run.Status is RunStatus.Merged) { @@ -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()); } if (!Directory.Exists(run.WorktreePath!)) diff --git a/apps/web/src/__tests__/ArtifactBrowser.test.tsx b/apps/web/src/__tests__/ArtifactBrowser.test.tsx index 1d7f73749..e7ff696f1 100644 --- a/apps/web/src/__tests__/ArtifactBrowser.test.tsx +++ b/apps/web/src/__tests__/ArtifactBrowser.test.tsx @@ -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( + + + , + ); + + 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( + + + , + ); + + 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 () => { diff --git a/apps/web/src/__tests__/OutcomePlanPanel.test.tsx b/apps/web/src/__tests__/OutcomePlanPanel.test.tsx index 2eb3c141c..04072a5b7 100644 --- a/apps/web/src/__tests__/OutcomePlanPanel.test.tsx +++ b/apps/web/src/__tests__/OutcomePlanPanel.test.tsx @@ -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( @@ -144,6 +145,7 @@ describe('OutcomePlanPanel confirm retry', () => { events={[staleAwaitingEvent]} streamStatus="streaming" onReconnect={onReconnect} + onConfirmed={onConfirmed} /> , ); @@ -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 () => { diff --git a/apps/web/src/components/OutcomePlanPanel.tsx b/apps/web/src/components/OutcomePlanPanel.tsx index 9c8f2129d..a0becd4b0 100644 --- a/apps/web/src/components/OutcomePlanPanel.tsx +++ b/apps/web/src/components/OutcomePlanPanel.tsx @@ -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; /** @@ -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(null); @@ -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?.(); diff --git a/apps/web/src/hooks/useArtifactBrowser.ts b/apps/web/src/hooks/useArtifactBrowser.ts index c1e95705c..32cec6fff 100644 --- a/apps/web/src/hooks/useArtifactBrowser.ts +++ b/apps/web/src/hooks/useArtifactBrowser.ts @@ -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' }, @@ -166,6 +167,7 @@ export function useArtifactBrowser( let active = true; let intervalId: ReturnType | undefined; let requestInFlight = false; + let serverErrorRetries = 0; const startPolling = (intervalMs: number) => { if (intervalId !== undefined) clearInterval(intervalId); @@ -178,6 +180,7 @@ export function useArtifactBrowser( (adapter?.getFiles ?? apiClient.getRunFiles.bind(apiClient))(runId, activeFilter) .then((data) => { if (active) { + serverErrorRetries = 0; setFiles(data); setFilesError(null); setFilesLoading(false); @@ -185,10 +188,13 @@ export function useArtifactBrowser( }) .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 { @@ -196,7 +202,14 @@ export function useArtifactBrowser( 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); + } + } } } }) @@ -232,6 +245,7 @@ export function useArtifactBrowser( let active = true; let workspaceIntervalId: ReturnType | undefined; let requestInFlight = false; + let serverErrorRetries = 0; const startPolling = (intervalMs: number) => { if (workspaceIntervalId !== undefined) clearInterval(workspaceIntervalId); @@ -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); @@ -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; }); diff --git a/apps/web/src/pages/CoordinatorRunPage.tsx b/apps/web/src/pages/CoordinatorRunPage.tsx index 7ba51d37b..61f859a91 100644 --- a/apps/web/src/pages/CoordinatorRunPage.tsx +++ b/apps/web/src/pages/CoordinatorRunPage.tsx @@ -350,6 +350,12 @@ function readStr(p: Record, 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).error; + return typeof error === 'string' ? error : undefined; +} + function readEventTimestamp(p: Record): string | undefined { return readStr(p, ['timestamp_utc', 'timestampUtc', 'updated_at', 'updatedAt', 'timestamp']); } @@ -2433,6 +2439,7 @@ export function CoordinatorRunPage() { if (!runId) return; let cancelled = false; let timer: ReturnType | undefined; + let consecutiveWorkPlanNotReady = 0; const TERMINAL = new Set(['complete', 'failed', 'blocked', 'declined']); queueMicrotask(() => { setRunLoadError(null); @@ -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); @@ -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.')); @@ -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)); @@ -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(); @@ -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} diff --git a/tests/Agentweaver.Tests/ArtifactFilesEndpointTests.cs b/tests/Agentweaver.Tests/ArtifactFilesEndpointTests.cs index be3c20bb4..bd6510d5c 100644 --- a/tests/Agentweaver.Tests/ArtifactFilesEndpointTests.cs +++ b/tests/Agentweaver.Tests/ArtifactFilesEndpointTests.cs @@ -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(); + 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( @@ -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(); + files.ValueKind.Should().Be(JsonValueKind.Array); + files.GetArrayLength().Should().Be(0); } [Fact] diff --git a/tests/Agentweaver.Tests/Coordinator/CoordinatorPhase2EndpointsTests.cs b/tests/Agentweaver.Tests/Coordinator/CoordinatorPhase2EndpointsTests.cs index 193ff538a..ab6d7851b 100644 --- a/tests/Agentweaver.Tests/Coordinator/CoordinatorPhase2EndpointsTests.cs +++ b/tests/Agentweaver.Tests/Coordinator/CoordinatorPhase2EndpointsTests.cs @@ -98,6 +98,20 @@ public async Task WorkPlan_RunWithoutPlan_Returns404() "the pre-decomposition state is typed so clients can distinguish it from a missing run"); } + [Fact] + public async Task WorkPlan_ActiveCoordinatorWithoutPlan_ReturnsTypedNotReady404() + { + var runId = await InsertInactiveCoordinatorRunAsync(CoordinatorWebApplicationFactory.OwnerUser); + await SeedConfirmedOutcomeSpecAsync(runId); + + var resp = await _owner.GetAsync($"/api/runs/{runId}/work-plan"); + + resp.StatusCode.Should().Be(HttpStatusCode.NotFound); + var body = await resp.Content.ReadFromJsonAsync(); + body.GetProperty("error").GetString().Should().Be("work_plan_not_ready", + "an active coordinator can be asynchronously creating its plan after confirmation"); + } + [Fact] public async Task WorkPlan_UnknownRun_Returns404() { @@ -993,4 +1007,23 @@ private async Task InsertInactiveCoordinatorRunAsync( await runStore.InsertAsync(run, CancellationToken.None); return runId.ToString(); } + + private async Task SeedConfirmedOutcomeSpecAsync(string coordinatorRunId) + { + using var scope = _factory.Services.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + db.OutcomeSpecs.Add(new Agentweaver.Api.Memory.OutcomeSpec + { + ProjectId = "proj-x", + CoordinatorRunId = coordinatorRunId, + Goal = "g", + DesiredOutcome = "o", + Scope = "s", + Assumptions = "a", + Status = "confirmed", + CreatedAt = DateTimeOffset.UtcNow, + UpdatedAt = DateTimeOffset.UtcNow, + }); + await db.SaveChangesAsync(); + } }