diff --git a/docs-web/content/docs/user-automation-and-ci.mdx b/docs-web/content/docs/user-automation-and-ci.mdx index 67fce9eb60..7efff17ff8 100644 --- a/docs-web/content/docs/user-automation-and-ci.mdx +++ b/docs-web/content/docs/user-automation-and-ci.mdx @@ -114,12 +114,14 @@ The `merge_indicator` field on a subtask documents the merge state: ## QA gate (enabled by default) -If a QA agent preset is wired to `qa_review` in routing, completed tasks pass through a QA review *before* the merge protocol greenlights them. A failed review: +If a QA agent preset is wired to `qa_review` in routing, completed tasks pass through a QA review *before* the merge protocol greenlights them. A non-passing review that exhausts its configured guardrail: - Sets task status to `QA_REVIEW_FAILED`. - Creates an attention item with the QA agent's findings. - Pauses the task until rectified. +Provider or infrastructure errors do not immediately create this handoff. They retry automatically until the applicable QA run cap is reached, and the sprint watch loop stays alive while worker or human attention is active. + ## Attention items: who handles them Anything not auto-handled becomes an attention item. The dashboard's **Live Session** page surfaces them. diff --git a/docs-web/content/docs/user-dashboard-chat.mdx b/docs-web/content/docs/user-dashboard-chat.mdx index 67490da59d..5964439e0e 100644 --- a/docs-web/content/docs/user-dashboard-chat.mdx +++ b/docs-web/content/docs/user-dashboard-chat.mdx @@ -11,6 +11,8 @@ The **Chat** page (`/chat`) is a thread-based conversation surface that lets you - **Invocations** — A historical log of server-created execution invocations, including provider-backed agent runs and MCP `CallTool` activity. - **Main panel** — The active thread (or invocation), rendered as a chat transcript with user, assistant, and tool messages. Markdown is rendered with `marked`, including code blocks. +Markdown links use the dashboard's theme-aware signal colors across thread messages, invocation transcripts, truncated system messages, and 3D Chat. In dark mode they render in high-contrast mint with a brighter jade hover/focus state instead of a fixed dark blue; provider-branded badges retain their own readable semantic colors. + 3D chat and agent avatar surfaces use the standard WebGL avatar with studio lighting and pointer-aware head movement. The removed flashlight beam, target glow, low-battery flicker, and emissive shell boost are no longer part of the chat layout; reduced-motion settings or WebGL fallback mode continue to use the static SVG avatar. ## No-project assistant diff --git a/docs-web/content/docs/user-sprint-orchestration.mdx b/docs-web/content/docs/user-sprint-orchestration.mdx index 0f01eb0932..9ff2018b34 100644 --- a/docs-web/content/docs/user-sprint-orchestration.mdx +++ b/docs-web/content/docs/user-sprint-orchestration.mdx @@ -117,7 +117,9 @@ When a worker resolves a merge conflict, Code UX clears the task's stale `MERGE_ CLI tasks that complete with a worker branch but no PR use a branch-only merge path in both LOCAL and REMOTE git modes; REMOTE mode then pushes the sprint feature branch. If the task snapshot lost the worker branch, Code UX recovers it from the completed task run before checking merge readiness. For CLI-backed runs, branch-only classification and protocol merge-required attention wait for the git-finalize event (`cli_git_pushed` or `cli_git_no_changes`) so provider/session completion cannot race ahead of branch materialization. Task QA reviews run from an isolated snapshot of that selected branch in both Docker and host execution, so a visible default-branch checkout cannot create a false missing-file rejection. That merge runs in a temporary worktree through the containerized Git helper so the visible checkout and `.code-ux/` runtime files do not interfere with task settlement. When several clean LOCAL worker branches are ready in one cycle, they share that worktree while each successful merge is committed and published to the feature branch independently. Code UX normalizes temporary worktree gitdir metadata after creation so later helper-container Git calls resolve the same repository. Once the task is settled as merged, stale task-run worker branch evidence is suppressed from live status so old branches do not keep re-entering merge scans. -Sprint-completion QA stays fail-closed. If its provider terminates without a verdict and the sprint has not changed, or the non-passing review cycle reaches its retry cap, Code UX raises one sprint-scoped human handoff with the attempt count and latest provider error instead of leaving the sprint silently running. After the provider or review result is corrected, resolving that handoff resets only sprint-completion QA and permits one fresh review cycle. +Sprint-completion QA stays fail-closed. If its provider terminates without a verdict, Code UX retries on the next watch cycle even when the sprint has not changed. Only after the configured sprint-QA retry cap is reached does Code UX raise one sprint-scoped human handoff with the attempt count and latest provider error. The sprint remains active while waiting for that handoff, so resolving it resets only sprint-completion QA and permits one fresh review cycle without manually resuming the sprint. + +Provider diagnostics are derived from real provider error events. In particular, Codex tool output or repository fixtures that merely mention authentication, quota, or rate limits do not get reported as provider authentication failures. Worker-owned merge-conflict repair and LOCAL task-branch merges resolve `.code-ux/**` conflicts to the target branch side before deciding whether a provider is needed. A conflict only in Code UX runtime artifacts does not dispatch a provider, and invalid Docker repair workspaces are reseeded before provider execution. Real source conflicts outside `.code-ux/` still fail closed and remain visible as merge-conflict work. @@ -136,7 +138,7 @@ Override via `maxFailures` in settings or `JULES_API_MAX_FAILS` in the environme Two distinct retry surfaces: 1. **Task-level retry** — `retryFailed: true` (default). Failed sessions get a fresh worker session next cycle. The original failure stays attached for diagnosis. -2. **CI autofix retry** — If a PR's CI is failing and `waitForJulesCiAutofix: true`, Code UX dispatches a CI fix worker. Up to `julesCiAutofixMaxRetries` (legacy mirror default `5`, max `20`) attempts before creating a human handoff. +2. **CI autofix retry** — If a PR's CI is failing and automatic repair is enabled, Code UX dispatches a CI fix worker and keeps the sprint active while it runs. Failures requeue the same attention item until the `ci_fix` guardrail is reached; only then is a human handoff created. ## Action-required automation diff --git a/docs-web/user/automation-and-ci.md b/docs-web/user/automation-and-ci.md index 6e2a160082..7bb9ef13e6 100644 --- a/docs-web/user/automation-and-ci.md +++ b/docs-web/user/automation-and-ci.md @@ -114,12 +114,14 @@ The `merge_indicator` field on a subtask documents the merge state: ## QA gate (enabled by default) -If a QA agent preset is wired to `qa_review` in routing, completed tasks pass through a QA review *before* the merge protocol greenlights them. A failed review: +If a QA agent preset is wired to `qa_review` in routing, completed tasks pass through a QA review *before* the merge protocol greenlights them. A non-passing review that exhausts its configured guardrail: - Sets task status to `QA_REVIEW_FAILED`. - Creates an attention item with the QA agent's findings. - Pauses the task until rectified. +Provider or infrastructure errors do not immediately create this handoff. They retry automatically until the applicable QA run cap is reached, and the sprint watch loop stays alive while worker or human attention is active. + ## Attention items: who handles them Anything not auto-handled becomes an attention item. The dashboard's **Live Session** page surfaces them. diff --git a/docs-web/user/sprint-orchestration.md b/docs-web/user/sprint-orchestration.md index 376a55391c..ef2d71dac9 100644 --- a/docs-web/user/sprint-orchestration.md +++ b/docs-web/user/sprint-orchestration.md @@ -117,7 +117,9 @@ When a worker resolves a merge conflict, Code UX clears the task's stale `MERGE_ CLI tasks that complete with a worker branch but no PR use a branch-only merge path in both LOCAL and REMOTE git modes; REMOTE mode then pushes the sprint feature branch. If the task snapshot lost the worker branch, Code UX recovers it from the completed task run before checking merge readiness. For CLI-backed runs, branch-only classification and protocol merge-required attention wait for the git-finalize event (`cli_git_pushed` or `cli_git_no_changes`) so provider/session completion cannot race ahead of branch materialization. Task QA reviews run from an isolated snapshot of that selected branch in both Docker and host execution, so a visible default-branch checkout cannot create a false missing-file rejection. That merge runs in a temporary worktree through the containerized Git helper so the visible checkout and `.code-ux/` runtime files do not interfere with task settlement. When several clean LOCAL worker branches are ready in one cycle, they share that worktree while each successful merge is committed and published to the feature branch independently. Code UX normalizes temporary worktree gitdir metadata after creation so later helper-container Git calls resolve the same repository. Once the task is settled as merged, stale task-run worker branch evidence is suppressed from live status so old branches do not keep re-entering merge scans. -Sprint-completion QA stays fail-closed. If its provider terminates without a verdict and the sprint has not changed, or the non-passing review cycle reaches its retry cap, Code UX raises one sprint-scoped human handoff with the attempt count and latest provider error instead of leaving the sprint silently running. After the provider or review result is corrected, resolving that handoff resets only sprint-completion QA and permits one fresh review cycle. +Sprint-completion QA stays fail-closed. If its provider terminates without a verdict, Code UX retries on the next watch cycle even when the sprint has not changed. Only after the configured sprint-QA retry cap is reached does Code UX raise one sprint-scoped human handoff with the attempt count and latest provider error. The sprint remains active while waiting for that handoff, so resolving it resets only sprint-completion QA and permits one fresh review cycle without manually resuming the sprint. + +Provider diagnostics are derived from real provider error events. In particular, Codex tool output or repository fixtures that merely mention authentication, quota, or rate limits do not get reported as provider authentication failures. Worker-owned merge-conflict repair and LOCAL task-branch merges resolve `.code-ux/**` conflicts to the target branch side before deciding whether a provider is needed. A conflict only in Code UX runtime artifacts does not dispatch a provider, and invalid Docker repair workspaces are reseeded before provider execution. Real source conflicts outside `.code-ux/` still fail closed and remain visible as merge-conflict work. @@ -136,7 +138,7 @@ Override via `maxFailures` in settings or `JULES_API_MAX_FAILS` in the environme Two distinct retry surfaces: 1. **Task-level retry** — `retryFailed: true` (default). Failed sessions get a fresh worker session next cycle. The original failure stays attached for diagnosis. -2. **CI autofix retry** — If a PR's CI is failing and `waitForJulesCiAutofix: true`, Code UX dispatches a CI fix worker. Up to `julesCiAutofixMaxRetries` (legacy mirror default `5`, max `20`) attempts before creating a human handoff. +2. **CI autofix retry** — If a PR's CI is failing and automatic repair is enabled, Code UX dispatches a CI fix worker and keeps the sprint active while it runs. Failures requeue the same attention item until the `ci_fix` guardrail is reached; only then is a human handoff created. ## Action-required automation diff --git a/docs/architecture/quality-assurance-agent.md b/docs/architecture/quality-assurance-agent.md index 69b2a32004..0f4d5d30a1 100644 --- a/docs/architecture/quality-assurance-agent.md +++ b/docs/architecture/quality-assurance-agent.md @@ -158,6 +158,8 @@ Task-level prompt scope: If task QA is still pending, running, or has failed without exhausting `maxTaskReviewRuns`, Code UX marks the task merge state as `QA_PENDING` and keeps the sprint active instead of auto-merging. If QA is exhausted and configured to `ESCALATE_TO_HUMAN`, the task is held in `QA_REVIEW_FAILED` and will not be merged or marked complete until a human resolves it. +Provider/infrastructure failures in sprint-completion QA are also retryable without requiring task changes. Code UX starts the next review cycle until `maxSprintReviewRuns` is reached, opens the sprint-scoped human handoff only at that cap, and leaves the sprint run active so it can observe resolution and continue automatically. + Recovery guarantees: - task QA no longer depends only on catching a single in-cycle transition edge; if a task is already code-complete and still has no successful QA run, Code UX will enqueue the missing review on the next orchestration cycle instead of leaving the task parked in `QA_PENDING` @@ -187,7 +189,7 @@ This separation keeps repository writes, provider calls, task status mutations, - whether a task QA attempt is still within budget or should require human attention - whether a normalized task review means pass, changes requested, retryable failure, or fatal failure -- whether a sprint completion review should run, stay blocked, or be skipped because it already passed or exhausted its retry budget +- whether a sprint completion review should run, retry a provider failure, stay blocked, or be skipped because it already passed or exhausted its retry budget - whether a `running` QA review row is still legitimately active or should be recovered as a failed stale run - the initial completed task review always counts as run `1` diff --git a/docs/sprint-loop/atomic-loop.md b/docs/sprint-loop/atomic-loop.md index 835bf67ed5..921ae008ce 100644 --- a/docs/sprint-loop/atomic-loop.md +++ b/docs/sprint-loop/atomic-loop.md @@ -168,9 +168,9 @@ When `action=orchestrate`, `wait` is true, and `watchLoop` is enabled: - Provider load for start-ready admission combines running provider invocations with active task runs that have not yet produced terminal provider evidence. This prevents wide DAG cycles from creating a large queue of running Docker dispatches that all wait inside the provider-slot gate before their provider invocation rows exist. Provider-slot wait logs are also throttled per provider across concurrent waiters. - Loop exits when: - all tasks reach their terminal state (e.g., `COMPLETED` + merged, or `FAILED`) and the final merge is settled: remote-git mode requires GitHub to report the completion PR as merged, while local-git mode requires the sprint feature branch to merge into the configured local default branch, or - - no runnable tasks remain, or + - no runnable tasks remain and no sprint-scoped worker or human handoff is still active, or - merge-required tasks are detected that need manual intervention. -- The checkpoint window triggers internal reports and lease renewals without stopping the run. The run pauses for human handoff (such as `CREATE_PR` mode for main-branch merges) or cancels if aborted. +- The checkpoint window triggers internal reports and lease renewals without stopping the run. Task/QA guardrail handoffs keep the run alive so it can observe their resolution; explicit dashboard control and final main-merge policies such as `CREATE_PR` may still pause the run. Cancellation remains terminal. - In local-git mode, the final sprint feature-branch merge runs in a temporary Git worktree and force-updates the configured default branch after the merge succeeds. The visible project checkout is not switched between branches, so user-facing local workspaces stay on the branch the operator had checked out. - If that visible checkout has user-created dirty work at finalization time, Code UX first preserves that work on a `dirty-ref-` branch, completes the clean sprint merge, and then copies the preserved dirty commit back into the visible checkout with `cherry-pick --no-commit` followed by an unstage step. Non-conflicting dirty files therefore return as ordinary uncommitted working-tree changes, not as a merge commit. If the restore conflicts or cannot be applied cleanly, Code UX aborts the restore, leaves the dirty branch intact, and opens a dashboard attention item naming the branch and affected paths. Dirty files under the repo-local `.code-ux/` directory are ignored by this preservation check so Code UX runtime artifacts do not block or alter local merges. When the checked-out target branch is the one being updated, the working tree is refreshed to match the merged commit before dirty work is restored. - The watch loop uses the same `task-transition-state.ts` helper as the cycle @@ -214,7 +214,9 @@ For `action=status`: - Starting or resuming orchestration resolves stale sprint-level `manual_attention` escalations from prior runs. The new run recomputes current blockers, while task-specific human attention remains open until explicitly handled. - Before task QA gates are evaluated, the sprint cycle reconciles running task QA invocations with provider runtime state. Missing provider linkage or a missing Docker session container makes the stale QA row retryable instead of blocking the task indefinitely at `QA_PENDING`. - Sprint-completion QA also uses the sprint trigger's `agentPresetIds` list, or one default fallback reviewer when the list is empty. Completion is allowed only after the latest sprint QA cycle has all reviewers passed; any running, failed, or changes-requested reviewer blocks completion and a changes-requested review may route one follow-up task/session repair using the existing sprint QA follow-up logic. -- A terminal sprint-completion QA provider failure with no changed task snapshot, or a non-passing cycle at the sprint QA retry cap, opens one sprint-scoped `human_escalation_required` handoff instead of leaving a healthy watch loop to heartbeat forever. The handoff records the attempt count, cap, reason, and latest provider error, remains deduplicated across runtime restarts, and continues to block completion. Explicitly resolving it clears only the sprint-completion QA history so the current sprint can run one fresh review cycle; task QA history and guardrail ledgers remain intact. +- Sprint-completion QA provider/infrastructure failures retry on the next watch cycle even when the task snapshot is unchanged. Each failed cycle advances the configured `maxSprintReviewRuns` guardrail; only a non-passing cycle at that cap opens one sprint-scoped `human_escalation_required` handoff. The handoff records the attempt count, cap, reason, and latest provider error, remains deduplicated across runtime restarts, and keeps the sprint run alive while blocking completion. Explicitly resolving it clears only the sprint-completion QA history so the current sprint can run one fresh review cycle; task QA history and guardrail ledgers remain intact. +- Codex error classification reads structured `error`/`turn.failed` events plus stderr, not ordinary JSON event payloads. Agent prose, repository fixtures, and tool output that mention authentication, quota, or rate limits therefore cannot be mistaken for the provider's failure reason. +- Main-merge CI-fix attention records carry explicit source and target branches. A worker-owned CI repair opened during finalization is recognized in that same cycle, so the watch loop waits and keeps heartbeating until the worker succeeds or exhausts the `ci_fix` guardrail instead of pausing beside an active invocation. - Sprint orchestration resolves LOCAL vs REMOTE git behavior from the effective project/sprint settings (`settings.git.githubMode`). Local-git projects therefore use the local worker-to-feature merge path consistently during both single-cycle and watch-loop runs. - Sprint orchestration resolves providers from exact provider-config ids in effective settings. A route value such as `mockup-cli` or `gemini-fast` must correspond to a configured provider instance with that exact id; provider-type aliases no longer select arbitrary same-type instances. Sprint/project overrides that explicitly add a provider instance are preserved before route validation so isolated mockup runs cannot fall back to inherited live providers. - Invocation-route provider maps are replace-on-write at project and sprint scope. If a scoped setting declares `task_coding.providers`, `merge_conflict.providers`, or `qa_review.providers`, that route uses the declared provider-config ids only instead of deep-merging inherited route providers. diff --git a/src/domain/qa-review/sprint-qa-snapshot.ts b/src/domain/qa-review/sprint-qa-snapshot.ts index 31b8c878c7..fbfcd0dfae 100644 --- a/src/domain/qa-review/sprint-qa-snapshot.ts +++ b/src/domain/qa-review/sprint-qa-snapshot.ts @@ -77,9 +77,15 @@ export function evaluateSprintQaReviewDecision( return { action: "skip_review", reason: "already_passed" }; } + const terminalProviderFailure = latestRun.status === "failed" + || latestRun.status === "errored" + || latestRun.status === "cancelled"; + if (terminalProviderFailure && latestRun.runIndex < args.maxSprintReviewRuns) { + return { action: "run_review", reason: "needs_review" }; + } + const retryBudgetExhausted = typeof latestRun.runIndex === "number" - && latestRun.runIndex >= args.maxSprintReviewRuns - && latestRun.status === "completed"; + && latestRun.runIndex >= args.maxSprintReviewRuns; if (retryBudgetExhausted) { return { action: "skip_review", reason: "retry_budget_exhausted" }; @@ -117,6 +123,21 @@ export function evaluateSprintQaReviewCycleDecision( return { action: "skip_review", reason: "already_passed" }; } + const latestRunIndex = args.latestRuns.reduce( + (maxRunIndex, run) => Math.max(maxRunIndex, typeof run.runIndex === "number" ? run.runIndex : 0), + 0, + ); + const hasTerminalProviderFailure = args.latestRuns.some((run) => ( + run.status === "failed" || run.status === "errored" || run.status === "cancelled" + )); + + // Provider/infrastructure failures produced no QA verdict. Retry them even + // when the task snapshot is unchanged, and hand off only after the configured + // sprint-QA guardrail is actually exhausted. + if (hasTerminalProviderFailure && latestRunIndex < args.maxSprintReviewRuns) { + return { action: "run_review", reason: "needs_review" }; + } + if ( args.latestRuns.some((run) => run.outcome === "changes_requested" || run.status === "failed") && !args.shouldRunReview @@ -124,9 +145,8 @@ export function evaluateSprintQaReviewCycleDecision( return { action: "block_completion", reason: "awaiting_follow_up" }; } - const retryBudgetExhausted = typeof latestRun.runIndex === "number" - && latestRun.runIndex >= args.maxSprintReviewRuns - && args.latestRuns.every((run) => run.status === "completed" || run.status === "failed"); + const retryBudgetExhausted = latestRunIndex >= args.maxSprintReviewRuns + && args.latestRuns.every((run) => run.status !== "running"); if (retryBudgetExhausted) { return { action: "block_completion", reason: "awaiting_follow_up" }; diff --git a/src/domain/sprint/orchestrator/watch-loop-runner.ts b/src/domain/sprint/orchestrator/watch-loop-runner.ts index 60450d1474..fcad2e27b0 100644 --- a/src/domain/sprint/orchestrator/watch-loop-runner.ts +++ b/src/domain/sprint/orchestrator/watch-loop-runner.ts @@ -650,13 +650,13 @@ export class WatchLoopRunner { featureBranch: defaultFeatureBranch, defaultBranch, mergeStage: "main", - prNumber: mergeFeedback.prNumber, - prUrl: mergeFeedback.prUrl, - mergeStateStatus: mergeFeedback.mergeStateStatus, conflictingBranches: { source: defaultFeatureBranch, target: defaultBranch, }, + prNumber: mergeFeedback.prNumber, + prUrl: mergeFeedback.prUrl, + mergeStateStatus: mergeFeedback.mergeStateStatus, sprintNumber: scopedExecutionContext.sprintNumber, sprintName: scopedExecutionContext.sprint.name, featureBranchTaskContexts: selectMergedTaskContexts(subtasks, { limit: 8 }), @@ -712,6 +712,10 @@ export class WatchLoopRunner { featureBranch: defaultBranch, defaultBranch, mergeStage: "main", + conflictingBranches: { + source: defaultFeatureBranch, + target: defaultBranch, + }, prNumber: mergeFeedback.prNumber, prUrl: mergeFeedback.prUrl, mergeStateStatus: mergeFeedback.mergeStateStatus, @@ -1525,9 +1529,13 @@ function isMainMergeAttentionInScope( : null; const sourceBranch = typeof conflictingBranches?.source === "string" ? conflictingBranches.source - : typeof payload.featureBranch === "string" - ? payload.featureBranch - : null; + : item.attentionType === "ci_fix_required" && typeof payload.workerBranch === "string" + ? payload.workerBranch + : item.attentionType === "ci_fix_required" && typeof payload.branchName === "string" + ? payload.branchName + : typeof payload.featureBranch === "string" + ? payload.featureBranch + : null; const targetBranch = typeof conflictingBranches?.target === "string" ? conflictingBranches.target : typeof payload.defaultBranch === "string" diff --git a/src/domain/sprint/task-transition-state.ts b/src/domain/sprint/task-transition-state.ts index 9f51358e9d..504ab940d5 100644 --- a/src/domain/sprint/task-transition-state.ts +++ b/src/domain/sprint/task-transition-state.ts @@ -52,6 +52,7 @@ export interface SprintTransitionState { runningTasks: Subtask[]; readyTasks: Subtask[]; activeWorkerAttentionItems: ProjectAttentionItemRecord[]; + activeHumanAttentionItems: ProjectAttentionItemRecord[]; activeWorkerMergeConflictAttention: boolean; activeMainMergeAttentionItems: ProjectAttentionItemRecord[]; qaPendingTasks: Subtask[]; @@ -63,6 +64,7 @@ export interface SprintTransitionState { noMoreActionPossible: boolean; needsManualMerge: boolean; waitingOnWorkerAttention: boolean; + waitingOnHumanAttention: boolean; allFinished: boolean; } @@ -265,7 +267,21 @@ export function evaluateSprintTransitionState(params: SprintTransitionStateParam || isLocalCliTaskAwaitingBranchEvidence(task, params) )) .map(({ task }) => task); + const sprintIds = new Set( + subtasks + .map((task) => task.sprint_id?.trim()) + .filter((sprintId): sprintId is string => Boolean(sprintId)), + ); + const activeSprintAttentionItems = activeProjectAttentionItems.filter((item) => ( + item.sprintRunId === sprintRunId + || (typeof item.sprintId === "string" && sprintIds.has(item.sprintId)) + )); const activeWorkerAttentionItems = activeProjectAttentionItems.filter((item) => item.ownerType === "worker"); + const activeHumanAttentionItems = activeSprintAttentionItems.filter((item) => ( + item.ownerType === "human" + || item.attentionType === "human_escalation_required" + || item.attentionType === "dashboard_reply_required" + )); const activeWorkerMergeConflictAttention = activeWorkerAttentionItems.some((item) => item.attentionType === "merge_conflict"); const workerMergeConflictTasksStillActive = workerEscalatedMergeConflictTasks.filter((task) => { const taskId = task.record_id?.trim(); @@ -288,7 +304,12 @@ export function evaluateSprintTransitionState(params: SprintTransitionStateParam const waitingOnWorkerAttention = workerMergeConflictTasksStillActive.length > 0 || activeWorkerMergeConflictAttention || activeWorkerAttentionItems.length > 0; - const allFinished = allTerminal || ((needsManualMerge || noMoreActionPossible) && !waitingOnWorkerAttention); + const waitingOnHumanAttention = activeHumanAttentionItems.length > 0; + const allFinished = allTerminal || ( + (needsManualMerge || noMoreActionPossible) + && !waitingOnWorkerAttention + && !waitingOnHumanAttention + ); return { tasksByStatus, @@ -296,6 +317,7 @@ export function evaluateSprintTransitionState(params: SprintTransitionStateParam runningTasks, readyTasks, activeWorkerAttentionItems, + activeHumanAttentionItems, activeWorkerMergeConflictAttention, activeMainMergeAttentionItems, qaPendingTasks, @@ -307,6 +329,7 @@ export function evaluateSprintTransitionState(params: SprintTransitionStateParam noMoreActionPossible, needsManualMerge, waitingOnWorkerAttention, + waitingOnHumanAttention, allFinished, }; } diff --git a/src/services/provider-execution-service.ts b/src/services/provider-execution-service.ts index 7cabb37e7f..fa6d896021 100644 --- a/src/services/provider-execution-service.ts +++ b/src/services/provider-execution-service.ts @@ -830,7 +830,13 @@ export class ProviderExecutionService { }); } } - return providerResult; + return { + ...providerResult, + // Downstream workflows historically preferred stderr, where Codex emits + // only its benign stdin notice. Preserve raw stdout for audit/telemetry, + // but give callers the classifier's actionable diagnostic. + stderr: classification.userMessage, + }; } } diff --git a/src/services/quality-assurance-service.ts b/src/services/quality-assurance-service.ts index 1fd64a35eb..99c0cad300 100644 --- a/src/services/quality-assurance-service.ts +++ b/src/services/quality-assurance-service.ts @@ -911,7 +911,7 @@ export class QualityAssuranceService { run.status === "failed" || run.status === "errored" || run.status === "cancelled" )) ?? null; const retryBudgetExhausted = latestRun.runIndex >= args.maxRuns; - if (!allTerminal || (!terminalFailure && !retryBudgetExhausted)) { + if (!allTerminal || !retryBudgetExhausted) { return; } diff --git a/src/shared/providers/provider-error-classifier.ts b/src/shared/providers/provider-error-classifier.ts index 4559884a20..1190a96b46 100644 --- a/src/shared/providers/provider-error-classifier.ts +++ b/src/shared/providers/provider-error-classifier.ts @@ -642,7 +642,7 @@ export function classifyProviderError( provider: Exclude, result: CommandResult, ): ProviderErrorClassification { - const combined = `${result.stdout}\n${result.stderr}`; + const combined = buildProviderDiagnosticText(provider, result); const providerPatterns = PROVIDER_PATTERNS[provider] ?? []; if (provider === "codex" && isCodexTransportServerError(combined)) { @@ -699,6 +699,31 @@ export function classifyProviderError( return buildUnknownClassification(provider, combined); } +/** + * Codex `exec --json` stdout is a complete event stream, not an error stream. It + * includes assistant prose plus tool inputs/results, so repository text such as + * "authentication failed" or "rate limit" must not classify the provider failure. + * Only structured Codex error events, non-JSON stdout, and stderr are diagnostic. + */ +function buildProviderDiagnosticText( + provider: Exclude, + result: CommandResult, +): string { + if (provider !== "codex") { + return `${result.stdout}\n${result.stderr}`; + } + + const structuredError = extractCodexStructuredError(result.stdout); + const unstructuredStdout = result.stdout + .split("\n") + .filter((line) => !line.trimStart().startsWith("{")) + .join("\n"); + + return [structuredError, unstructuredStdout, result.stderr] + .filter((part): part is string => typeof part === "string" && part.trim().length > 0) + .join("\n"); +} + export function isTransientCodexTransportError(result: CommandResult): boolean { const text = `${result.stdout}\n${result.stderr}`.toLowerCase(); return text.includes("stream disconnected before completion") || text.includes("error sending request for url") || text.includes("channel closed"); diff --git a/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts b/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts index 2ce983d3b9..6be18b4ef0 100644 --- a/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts +++ b/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts @@ -139,11 +139,11 @@ describe("Sprint QA Snapshot", () => { expected: { action: "skip_review", reason: "already_passed" }, }, { - name: "blocks completion when a failed review has no meaningful follow-up changes", + name: "retries a failed review even when there are no follow-up changes", latestRun: makeRun({ status: "failed", outcome: null, runIndex: 1 }), maxSprintReviewRuns: 3, shouldRunReview: false, - expected: { action: "block_completion", reason: "awaiting_follow_up" }, + expected: { action: "run_review", reason: "needs_review" }, }, { name: "runs again after a failed review when the sprint changed", @@ -206,5 +206,19 @@ describe("Sprint QA Snapshot", () => { shouldRunReview: false, })).toEqual({ action: "block_completion", reason: "awaiting_follow_up" }); }); + + it("retries provider failures until the sprint QA guardrail is reached", () => { + expect(evaluateSprintQaReviewCycleDecision({ + latestRuns: [makeRun({ status: "failed", outcome: null, runIndex: 1 })], + maxSprintReviewRuns: 3, + shouldRunReview: false, + })).toEqual({ action: "run_review", reason: "needs_review" }); + + expect(evaluateSprintQaReviewCycleDecision({ + latestRuns: [makeRun({ status: "failed", outcome: null, runIndex: 3 })], + maxSprintReviewRuns: 3, + shouldRunReview: false, + })).toEqual({ action: "block_completion", reason: "awaiting_follow_up" }); + }); }); }); diff --git a/tests/backend/services/provider-execution-service.test.ts b/tests/backend/services/provider-execution-service.test.ts index 85c4a9d70c..7fb9d0f74a 100644 --- a/tests/backend/services/provider-execution-service.test.ts +++ b/tests/backend/services/provider-execution-service.test.ts @@ -1566,7 +1566,7 @@ describe("ProviderExecutionService", () => { }); }); - it("Unknown failure passthrough: returns result without throwing on UNKNOWN classification", async () => { + it("Unknown failure passthrough: returns an actionable diagnostic without throwing", async () => { const failedResult = { ...mockResult, ok: false }; providerRunner.runProvider.mockResolvedValue(failedResult); @@ -1583,7 +1583,10 @@ describe("ProviderExecutionService", () => { const result = await service.executeProvider(defaultArgs); - expect(result).toBe(failedResult); + expect(result).toEqual({ + ...failedResult, + stderr: "Unknown error", + }); expect(providerRunner.runProvider).toHaveBeenCalledTimes(1); }); diff --git a/tests/backend/services/quality-assurance-service.test.ts b/tests/backend/services/quality-assurance-service.test.ts index 0549e40548..bcb6b5cc9d 100644 --- a/tests/backend/services/quality-assurance-service.test.ts +++ b/tests/backend/services/quality-assurance-service.test.ts @@ -2269,26 +2269,34 @@ describe("QualityAssuranceService", () => { getGithubToken: () => undefined, sendSessionMessage: async () => ({}), }); - vi.spyOn(service as any, "runReview").mockRejectedValue(new Error("Sprint QA provider timed out.")); + const runReview = vi.spyOn(service as any, "runReview").mockRejectedValue(new Error("Sprint QA provider timed out.")); + const subtasks = [{ + record_id: task.id, + project_id: project.id, + sprint_id: sprint.id, + id: "T1", + title: "Initial task", + prompt: "Implement the initial feature.", + depends_on: [], + is_independent: true, + status: "COMPLETED", + is_merged: true, + merge_indicator: "MERGED", + }] as any; const outcome = await service.reviewSprintCompletion({ projectId: project.id, sprintId: sprint.id, sprintRunId: sprintRun.id, repoPath: dir, - subtasks: [{ - record_id: task.id, - project_id: project.id, - sprint_id: sprint.id, - id: "T1", - title: "Initial task", - prompt: "Implement the initial feature.", - depends_on: [], - is_independent: true, - status: "COMPLETED", - is_merged: true, - merge_indicator: "MERGED", - }] as any, + subtasks, + }); + const retryOutcome = await service.reviewSprintCompletion({ + projectId: project.id, + sprintId: sprint.id, + sprintRunId: sprintRun.id, + repoPath: dir, + subtasks, }); expect(outcome).toMatchObject({ @@ -2296,12 +2304,19 @@ describe("QualityAssuranceService", () => { blockedCompletion: true, mergeBlocked: true, }); + expect(retryOutcome).toMatchObject({ + reviewed: false, + blockedCompletion: true, + mergeBlocked: true, + }); + expect(runReview).toHaveBeenCalledTimes(2); expect(outcome.reportText).toContain("Sprint QA failed and blocked merge"); expect(outcome.reportText).toContain("Sprint QA provider timed out."); const latestRun = qaReviewRepository.getLatestSprintRun(sprint.id); expect(latestRun).toMatchObject({ status: "failed", + runIndex: 2, summaryMarkdown: "Sprint QA provider timed out.", }); expect(latestRun?.payload).toMatchObject({ error_code: "UNKNOWN" }); diff --git a/tests/backend/shared/provider-error-classifier.test.ts b/tests/backend/shared/provider-error-classifier.test.ts index ada0403ab3..b3a9fd013b 100644 --- a/tests/backend/shared/provider-error-classifier.test.ts +++ b/tests/backend/shared/provider-error-classifier.test.ts @@ -327,6 +327,22 @@ describe("classifyProviderError", () => { expect(classification.userMessage).not.toContain("stdin"); }); + it("ignores auth and rate-limit phrases inside successful Codex tool output", () => { + const stdout = [ + '{"type":"thread.started","thread_id":"thread-1"}', + '{"type":"item.completed","item":{"type":"command_execution","aggregated_output":"fixture: Codex authentication failed; rate limit 429"}}', + '{"type":"turn.failed","error":{"message":"turn/start failed: direct app-server input is not allowed"}}', + ].join("\n"); + + const classification = classifyProviderError( + "codex", + makeResult(stdout, "Reading additional input from stdin..."), + ); + + expect(classification.category).toBe("UNKNOWN"); + expect(classification.userMessage).toContain("direct app-server input is not allowed"); + }); + it("falls back to the generic unexpected-error text when codex produced no parseable detail", () => { const classification = classifyProviderError("codex", makeResult("", "Reading additional input from stdin...")); expect(classification.category).toBe("UNKNOWN"); diff --git a/tests/backend/sprint/watch-loop-core.test.ts b/tests/backend/sprint/watch-loop-core.test.ts index d5afe7bfa0..38c7e0eaef 100644 --- a/tests/backend/sprint/watch-loop-core.test.ts +++ b/tests/backend/sprint/watch-loop-core.test.ts @@ -2262,10 +2262,11 @@ describe("WatchLoopRunner", () => { nowSpy.mockRestore(); }); - it("pauses instead of completing when main auto-merge is blocked by failed checks", async () => { + it("keeps the sprint alive when main-merge CI opens a worker fix", async () => { const deps = buildDeps(); const cycleRunner = buildCycleRunner(); const nowSpy = vi.spyOn(Date, "now"); + const activeAttentionItems: any[] = []; nowSpy.mockReturnValueOnce(0).mockReturnValue(1000); @@ -2273,6 +2274,20 @@ describe("WatchLoopRunner", () => { if (id === "watchHeader") return "HEADER"; return ""; }); + deps.executionRepository.getSprintRun = vi.fn() + .mockReturnValueOnce({ status: "running" }) + .mockReturnValue({ status: "paused" }); + deps.projectAttentionService.openItems.mockImplementation((inputs: any[]) => { + const opened = inputs.map((input, index) => ({ + ...input, + id: `opened-${index}`, + status: "open", + summaryMarkdown: input.summaryMarkdown, + })); + activeAttentionItems.push(...opened); + return opened; + }); + deps.projectAttentionService.listActiveProjectItems.mockImplementation(() => activeAttentionItems); cycleRunner.run.mockResolvedValue({ subtasks: [buildMockSubtask({ status: "COMPLETED", is_merged: true, worker_branch: "worker/task-1" })], @@ -2346,14 +2361,10 @@ describe("WatchLoopRunner", () => { sprintRunId: "run-1", }); - expect(result).toContain("Sprint Paused"); + expect(result).toContain("worker is resolving the main-branch merge blocker"); expect(result).not.toContain("Sprint Execution Finished"); - expect(deps.executionRepository.appendSprintRunEvent).not.toHaveBeenCalledWith( - "run-1", - "sprint_completed", - expect.anything(), - expect.anything(), - expect.anything(), + expect(deps.sprintRunLifecycleService.transition).not.toHaveBeenCalledWith( + expect.objectContaining({ status: "paused" }), ); expect(deps.projectAttentionService.openItems).toHaveBeenCalledWith([ expect.objectContaining({ @@ -2363,6 +2374,10 @@ describe("WatchLoopRunner", () => { failedJobLabels: ["CI/build"], failedLogSnippets: [expect.stringContaining("FAIL src/example.test.ts\nExpected: true\nReceived: false")], failedRuns: [expect.objectContaining({ id: 9001 })], + conflictingBranches: { + source: "feature/sprint104-implementation", + target: "main", + }, }), }), ]); @@ -3717,4 +3732,28 @@ describe("evaluateSprintRunState", () => { expect(result.waitingOnWorkerAttention).toBe(true); expect(result.allFinished).toBe(false); }); + + it("keeps a guardrail-exhausted QA handoff alive until the human resolves it", () => { + const task = buildMockSubtask({ + status: "QA_REVIEW_FAILED", + sprint_id: "sprint-1", + is_merged: false, + }); + const result = evaluateSprintRunState({ + subtasks: [task], + manualMergeTasks: [], + workerEscalatedMergeConflictTasks: [], + activeProjectAttentionItems: [{ + ownerType: "human", + attentionType: "human_escalation_required", + sprintId: "sprint-1", + sprintRunId: "run-1", + } as any], + sprintRunId: "run-1", + }); + + expect(result.noMoreActionPossible).toBe(true); + expect(result.waitingOnHumanAttention).toBe(true); + expect(result.allFinished).toBe(false); + }); });