diff --git a/docs-web/content/docs/settings-quality-assurance.mdx b/docs-web/content/docs/settings-quality-assurance.mdx index fd70eec726..19b62e5231 100644 --- a/docs-web/content/docs/settings-quality-assurance.mdx +++ b/docs-web/content/docs/settings-quality-assurance.mdx @@ -35,6 +35,8 @@ A practical review flow is: Disabling QA removes an important last check before merge automation continues. +Disabling task-completion QA while leaving sprint-completion QA enabled moves task-local defect discovery to the integrated sprint review. That review can split several distinct blockers into separate tracked follow-up tasks, so this configuration can make the sprint task list grow sharply near completion. + Before applying changes, check: - Whether the value affects provider credentials, Docker runtime behavior, Git automation, memory retention, or destructive cleanup. @@ -52,6 +54,7 @@ If the saved setting does not appear to take effect: - If a task exhausts QA and enters `QA_REVIEW_FAILED`, resolve or dismiss its QA handoff after reviewing or correcting the work. Code UX clears that task's QA history and retry guardrail, returns a still-parked task to code-complete review state, and lets the active sprint run one fresh QA cycle. Restarting alone intentionally preserves the QA hold. - A fix continuation created by the review that reaches the configured cap gets one final verification review. A CLI continuation with no patch and no commits ahead is treated as `follow_up_no_progress` and applies the exhaustion policy immediately; repeated continuations cannot extend the budget indefinitely. - Recovered failed, cancelled, or errored QA attempts retry only within the bounded infrastructure grace. All terminal attempts count toward the hard ceiling, so repeated container loss eventually opens the configured handoff. +- Sprint QA review limits count review cycles, not the number of findings in each earlier cycle. The final configured cycle is verification-only: if it does not pass, Code UX opens one sprint-scoped human handoff and does not create another automatic follow-up batch. Completed follow-up work cannot bypass that exhausted-budget handoff merely because it changed the task snapshot. ## Related Documentation diff --git a/docs-web/content/docs/user-sprint-orchestration.mdx b/docs-web/content/docs/user-sprint-orchestration.mdx index ccabe82d2e..65b454282f 100644 --- a/docs-web/content/docs/user-sprint-orchestration.mdx +++ b/docs-web/content/docs/user-sprint-orchestration.mdx @@ -118,7 +118,7 @@ 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, 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. +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. Earlier review cycles may create separate tracked tasks for distinct blockers, but the final configured cycle is verification-only and cannot create another automatic batch. At the sprint-QA cap, any non-passing result raises one sprint-scoped human handoff with the attempt count and latest review summary or provider error, even when completed follow-up work changed the task snapshot. 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. Keeping task-completion QA enabled reduces task-local defects being deferred to this integrated review. 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. diff --git a/docs-web/settings/merge-gates-autofix.md b/docs-web/settings/merge-gates-autofix.md index feb9273309..db1db5faf0 100644 --- a/docs-web/settings/merge-gates-autofix.md +++ b/docs-web/settings/merge-gates-autofix.md @@ -41,6 +41,15 @@ Before applying changes, check: - Whether a project override is masking the system value you expected to change. - Whether a running sprint needs to be paused, restarted, or allowed to finish before the new value can be observed. +## CI Repair Invariants + +- A PR check rollup may contain an older cancelled or failed check beside its rerun. Code UX groups checks by workflow and check name and evaluates only the latest timestamped observation, so a newer pending or successful rerun supersedes historical failure. +- CI-repair evidence is selected from the newest branch-matched workflow run. A successful newer run prevents an older failed run from being sent back to a repair agent. +- A task blocked by CI remains code-complete and cannot be projected back to pending coding work merely because its provider session already completed. +- CI-owned tasks do not open `merge_required` attention. CI repair or its human handoff remains the only active blocker until checks settle. +- Coding-budget and CI-repair guardrail handoffs use distinct deduplication keys. Resolving one handoff resets only its matching guardrail purpose. +- When the task later settles as completed, Code UX resolves any remaining task guardrail handoff so the dashboard does not retain a stale intervention. + ## Troubleshooting If the saved setting does not appear to take effect: @@ -49,6 +58,7 @@ If the saved setting does not appear to take effect: - Check for a project or sprint override that takes precedence over the system value. - Refresh the affected dashboard page if the setting controls a rendered surface. - Restart the local runtime only when the setting explicitly controls startup, listener, or process-level behavior. +- If repair repeats after a rerun passed, inspect the PR rollup timestamps and the newest branch run. Historical failed rows should no longer count once a later observation for the same workflow/check is pending or successful. ## Related Documentation diff --git a/docs-web/settings/quality-assurance.md b/docs-web/settings/quality-assurance.md index fd70eec726..19b62e5231 100644 --- a/docs-web/settings/quality-assurance.md +++ b/docs-web/settings/quality-assurance.md @@ -35,6 +35,8 @@ A practical review flow is: Disabling QA removes an important last check before merge automation continues. +Disabling task-completion QA while leaving sprint-completion QA enabled moves task-local defect discovery to the integrated sprint review. That review can split several distinct blockers into separate tracked follow-up tasks, so this configuration can make the sprint task list grow sharply near completion. + Before applying changes, check: - Whether the value affects provider credentials, Docker runtime behavior, Git automation, memory retention, or destructive cleanup. @@ -52,6 +54,7 @@ If the saved setting does not appear to take effect: - If a task exhausts QA and enters `QA_REVIEW_FAILED`, resolve or dismiss its QA handoff after reviewing or correcting the work. Code UX clears that task's QA history and retry guardrail, returns a still-parked task to code-complete review state, and lets the active sprint run one fresh QA cycle. Restarting alone intentionally preserves the QA hold. - A fix continuation created by the review that reaches the configured cap gets one final verification review. A CLI continuation with no patch and no commits ahead is treated as `follow_up_no_progress` and applies the exhaustion policy immediately; repeated continuations cannot extend the budget indefinitely. - Recovered failed, cancelled, or errored QA attempts retry only within the bounded infrastructure grace. All terminal attempts count toward the hard ceiling, so repeated container loss eventually opens the configured handoff. +- Sprint QA review limits count review cycles, not the number of findings in each earlier cycle. The final configured cycle is verification-only: if it does not pass, Code UX opens one sprint-scoped human handoff and does not create another automatic follow-up batch. Completed follow-up work cannot bypass that exhausted-budget handoff merely because it changed the task snapshot. ## Related Documentation diff --git a/docs-web/user/sprint-orchestration.md b/docs-web/user/sprint-orchestration.md index 18900afa72..b7305e1596 100644 --- a/docs-web/user/sprint-orchestration.md +++ b/docs-web/user/sprint-orchestration.md @@ -118,7 +118,7 @@ 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, 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. +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. Earlier review cycles may create separate tracked tasks for distinct blockers, but the final configured cycle is verification-only and cannot create another automatic batch. At the sprint-QA cap, any non-passing result raises one sprint-scoped human handoff with the attempt count and latest review summary or provider error, even when completed follow-up work changed the task snapshot. 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. Keeping task-completion QA enabled reduces task-local defects being deferred to this integrated review. 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. diff --git a/docs/architecture/quality-assurance-agent.md b/docs/architecture/quality-assurance-agent.md index b082e95294..1a672b9914 100644 --- a/docs/architecture/quality-assurance-agent.md +++ b/docs/architecture/quality-assurance-agent.md @@ -222,6 +222,7 @@ Behavior: - if QA requests follow-up work and Code UX can continue that task session, sprint completion is held open - if sprint-completion QA targets a task that is already merged, Code UX does not reopen that settled session; it records the target for traceability and creates follow-up sprint tasks so repair work goes through a new tracked task branch - if QA creates follow-up tasks, sprint completion is held open until those new tasks finish and sprint QA passes on a later run +- automatic follow-up creation is allowed only before the final configured sprint review cycle. The final cycle is reserved for verification: if it still requests changes, Code UX records the findings, creates one sprint-scoped human handoff, and does not create another unreviewable task batch - sprint QA runs once for the finished sprint, then only runs again after a prior `changes_requested` or failed result and meaningful sprint task state changes have occurred - a passing sprint QA result is final for that sprint state and is not retriggered by another orchestration cycle with no real work changes - sprint task state changes are detected purely by serializing all current subtasks into a `SprintQaSnapshot` (including status, prompt, and merge indicators) and comparing it with the payload of the latest QA run; if a historical QA run lacks a saved snapshot, Code UX falls back to comparing the newest task modification timestamp against the QA run's finish timestamp @@ -230,11 +231,13 @@ Behavior: - later runs are only used to check QA-requested fixes or follow-up work - `maxSprintReviewRuns = 3` is the default sprint QA budget for new or unset settings - `maxSprintReviewRuns = 1` means sprint fixes are not re-checked by QA +- an exhausted sprint QA budget is authoritative even when completed follow-up work changed the task snapshot. Snapshot changes cannot start an over-budget review or suppress the required human handoff - if every reviewer in the latest sprint QA cycle passes, Code UX proceeds to main-merge evaluation and eventual completion - if any reviewer is still running, failed, requested changes, or waiting on follow-up work, the main merge stays blocked - reviewer rows remain visible per agent, while the shared `run_index` spends one sprint QA budget cycle - while a sprint QA review is running, Code UX now refreshes the parent sprint-run heartbeat and lease so long reviews are not mistaken for stalled orchestration and failed by runtime cleanup - stale sprint-level `running` QA rows are also reconciled against execution invocation state before gating; if the backing invocation already ended, Code UX reclassifies the stale row and immediately allows a retry instead of keeping sprint completion blocked forever +- `maxSprintReviewRuns` limits review cycles, not the number of defects that an earlier review can split into tracked work. Keeping task-completion QA enabled catches task-local defects before merge and reduces the amount of remediation deferred to the full integrated-sprint review ## Session Continuation diff --git a/docs/sprint-loop/atomic-loop.md b/docs/sprint-loop/atomic-loop.md index 069b5af965..b7c44daf0e 100644 --- a/docs/sprint-loop/atomic-loop.md +++ b/docs/sprint-loop/atomic-loop.md @@ -214,8 +214,8 @@ For `action=status`: - QA review budgets count review cycles, not reviewer rows. Multiple reviewer rows with the same `run_index` spend one task or sprint QA attempt while still preserving reviewer-specific `agent_preset_id`, `agent_name`, payload details, and task-run events for dashboard history. Latest-cycle summaries prefer blocking rows (`running`, `changes_requested`, or `failed`) over passing rows, so a single passing reviewer cannot hide another reviewer that still blocks the cycle. - 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. -- 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. +- 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. Before the final configured cycle, a changes-requested review may route follow-up task/session repair through the existing sprint QA logic. The final cycle is verification-only: a non-passing verdict records its findings and escalates without creating work that has no remaining review budget. +- 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; a non-passing cycle at that cap opens one sprint-scoped `human_escalation_required` handoff even when completed follow-up work changed the task snapshot. The handoff records the attempt count, cap, reason, and latest review summary or 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. diff --git a/src/domain/qa-review/sprint-qa-snapshot.ts b/src/domain/qa-review/sprint-qa-snapshot.ts index fbfcd0dfae..7556eb1c2f 100644 --- a/src/domain/qa-review/sprint-qa-snapshot.ts +++ b/src/domain/qa-review/sprint-qa-snapshot.ts @@ -51,8 +51,8 @@ export function shouldRunSprintQaReview(args: EvaluateSprintQaReviewNeedArgs): b export type SprintQaReviewDecision = | { action: "run_review"; reason: "no_prior_review" | "needs_review" } - | { action: "block_completion"; reason: "review_running" | "awaiting_follow_up" } - | { action: "skip_review"; reason: "already_passed" | "retry_budget_exhausted" }; + | { action: "block_completion"; reason: "review_running" | "awaiting_follow_up" | "retry_budget_exhausted" } + | { action: "skip_review"; reason: "already_passed" }; export interface EvaluateSprintQaReviewDecisionArgs { latestRun: QaReviewRunRecord | null; @@ -88,7 +88,7 @@ export function evaluateSprintQaReviewDecision( && latestRun.runIndex >= args.maxSprintReviewRuns; if (retryBudgetExhausted) { - return { action: "skip_review", reason: "retry_budget_exhausted" }; + return { action: "block_completion", reason: "retry_budget_exhausted" }; } if ( @@ -138,17 +138,20 @@ export function evaluateSprintQaReviewCycleDecision( return { action: "run_review", reason: "needs_review" }; } - if ( - args.latestRuns.some((run) => run.outcome === "changes_requested" || run.status === "failed") - && !args.shouldRunReview - ) { - return { action: "block_completion", reason: "awaiting_follow_up" }; - } - const retryBudgetExhausted = latestRunIndex >= args.maxSprintReviewRuns && args.latestRuns.every((run) => run.status !== "running"); + // The review budget is a hard guardrail. Once it is spent, a changed task + // snapshot means completed follow-up work is waiting for human disposition; + // it must not bypass the escalation path or start another review cycle. if (retryBudgetExhausted) { + return { action: "block_completion", reason: "retry_budget_exhausted" }; + } + + if ( + args.latestRuns.some((run) => run.outcome === "changes_requested" || run.status === "failed") + && !args.shouldRunReview + ) { return { action: "block_completion", reason: "awaiting_follow_up" }; } diff --git a/src/services/quality-assurance-service.ts b/src/services/quality-assurance-service.ts index 291167df04..3515305e03 100644 --- a/src/services/quality-assurance-service.ts +++ b/src/services/quality-assurance-service.ts @@ -679,7 +679,6 @@ export class QualityAssuranceService { sprintRunId: args.sprintRunId, latestRuns, maxRuns, - shouldRunReview, }); return { reviewed: false, @@ -819,7 +818,11 @@ export class QualityAssuranceService { const targetTaskRun = targetTask ? this.resolveTaskRunForSubtask(targetTask, args.sprintRunId) : null; const fixInstructions = review.fixInstructions; const canContinueTargetTask = Boolean(targetTask && !this.isMergedSubtask(targetTask)); - const continued = targetTask && fixInstructions && canContinueTargetTask + // Reserve the final configured review as a verification/handoff cycle. + // Creating more automatic work at the cap leaves no budget to verify it + // and previously trapped the sprint in an invisible heartbeat loop. + const canApplyAutomaticFollowUp = runIndex < maxRuns; + const continued = canApplyAutomaticFollowUp && targetTask && fixInstructions && canContinueTargetTask ? await this.requestFixesForTask({ task: targetTask, taskRun: targetTaskRun, @@ -829,15 +832,17 @@ export class QualityAssuranceService { prompt: fixInstructions, }) : { applied: false, mode: "none" as const }; - const createdFollowUpTasks = this.createSprintFollowUpTasks({ - projectId: args.projectId, - sprintId: args.sprintId, - targetTask, - fixInstructions, - review, - existingSubtasks: args.subtasks, - sourceRunId: changesRequested.run.id, - }); + const createdFollowUpTasks = canApplyAutomaticFollowUp + ? this.createSprintFollowUpTasks({ + projectId: args.projectId, + sprintId: args.sprintId, + targetTask, + fixInstructions, + review, + existingSubtasks: args.subtasks, + sourceRunId: changesRequested.run.id, + }) + : []; this.deps.qaReviewRepository.updateRun(changesRequested.run.id, { targetTaskKey: targetTask?.id || review.targetTaskKey, @@ -848,9 +853,14 @@ export class QualityAssuranceService { ...review.raw, continued: continued.applied, continuationMode: continued.mode, - continuationSkippedReason: targetTask && fixInstructions && !canContinueTargetTask - ? "target_task_already_merged" - : undefined, + continuationSkippedReason: !canApplyAutomaticFollowUp + ? "sprint_qa_retry_budget_exhausted" + : targetTask && fixInstructions && !canContinueTargetTask + ? "target_task_already_merged" + : undefined, + automaticFollowUpSuppressedReason: canApplyAutomaticFollowUp + ? undefined + : "sprint_qa_retry_budget_exhausted", createdFollowUpTaskKeys: createdFollowUpTasks.map((task) => task.taskKey), taskSnapshot: currentTaskSnapshot, }, @@ -867,6 +877,19 @@ export class QualityAssuranceService { clearMergeProjectionForRerun(targetTask); } + if (!canApplyAutomaticFollowUp) { + const terminalLatestRuns = this.deps.qaReviewRepository + .listLatestSprintCycleRuns(args.sprintId) + .filter((run) => run.sprintRunId === args.sprintRunId); + this.openSprintQaHumanHandoffIfTerminal({ + projectId: args.projectId, + sprintId: args.sprintId, + sprintRunId: args.sprintRunId, + latestRuns: terminalLatestRuns, + maxRuns, + }); + } + return { reviewed: true, blockedCompletion: true, @@ -876,7 +899,7 @@ export class QualityAssuranceService { targetTask?.id || review.targetTaskKey, continued.applied, createdFollowUpTasks.map((task) => task.taskKey), - ), + ) + (!canApplyAutomaticFollowUp ? renderSprintQaBudgetExhaustedReport(maxRuns) : ""), }; } @@ -927,11 +950,10 @@ export class QualityAssuranceService { sprintRunId: string; latestRuns: QaReviewRunRecord[]; maxRuns: number; - shouldRunReview: boolean; }): void { const service = this.deps.projectAttentionService; const latestRun = args.latestRuns[0] ?? null; - if (!service || !latestRun || args.shouldRunReview) { + if (!service || !latestRun) { return; } @@ -947,9 +969,10 @@ export class QualityAssuranceService { const reason = terminalFailure ? "terminal_review_failure" : "retry_budget_exhausted"; - const lastProviderError = terminalFailure?.summaryMarkdown?.trim() + const latestReviewDetail = terminalFailure?.summaryMarkdown?.trim() || latestRun.summaryMarkdown?.trim() || "Sprint QA did not produce a passing verdict."; + const lastProviderError = terminalFailure ? latestReviewDetail : null; const errorCode = terminalFailure?.payload?.error_code; const attempts = latestRun.runIndex; @@ -969,7 +992,7 @@ export class QualityAssuranceService { "Sprint completion remains blocked because QA could not produce a passing verdict.", `Attempts: ${attempts}/${args.maxRuns}.`, `Reason: ${reason}.`, - `Latest provider error: ${lastProviderError}`, + `${terminalFailure ? "Latest provider error" : "Latest QA result"}: ${latestReviewDetail}`, "Resolve this handoff after correcting the provider or reviewing the result to reset sprint QA and allow one fresh review cycle.", ].join("\n\n"), payload: { @@ -981,6 +1004,7 @@ export class QualityAssuranceService { runsUsed: attempts, maxRuns: args.maxRuns, lastProviderError, + latestQaSummary: latestRun.summaryMarkdown?.trim() || null, lastProviderErrorCode: typeof errorCode === "string" ? errorCode : null, latestQaRunId: terminalFailure?.id ?? latestRun.id, sprintRunId: args.sprintRunId, @@ -2232,6 +2256,10 @@ function renderSprintQaPendingReport(run: QaReviewRunRecord): string { return `\nSprint QA must be retried before merge.${summary ? ` ${summary}` : ""}\n`; } +function renderSprintQaBudgetExhaustedReport(maxRuns: number): string { + return `\nSprint QA used all ${maxRuns} configured review cycles. No additional automatic follow-up tasks were created; human review is required.\n`; +} + function renderSprintQaFailedReport(error: unknown): string { const summary = error instanceof Error ? error.message : String(error); return `\nSprint QA failed and blocked merge: ${summary}\n`; 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 6be18b4ef0..6366db63b4 100644 --- a/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts +++ b/tests/backend/domain/qa-review/sprint-qa-snapshot.test.ts @@ -153,11 +153,11 @@ describe("Sprint QA Snapshot", () => { expected: { action: "run_review", reason: "needs_review" }, }, { - name: "skips when a completed changes-requested review exhausted the retry budget", + name: "blocks when a completed changes-requested review exhausted the retry budget", latestRun: makeRun({ status: "completed", outcome: "changes_requested", runIndex: 1 }), maxSprintReviewRuns: 1, shouldRunReview: true, - expected: { action: "skip_review", reason: "retry_budget_exhausted" }, + expected: { action: "block_completion", reason: "retry_budget_exhausted" }, }, { name: "keeps completion blocked while review is running", @@ -218,7 +218,15 @@ describe("Sprint QA Snapshot", () => { latestRuns: [makeRun({ status: "failed", outcome: null, runIndex: 3 })], maxSprintReviewRuns: 3, shouldRunReview: false, - })).toEqual({ action: "block_completion", reason: "awaiting_follow_up" }); + })).toEqual({ action: "block_completion", reason: "retry_budget_exhausted" }); + }); + + it("honors the exhausted budget even when completed follow-up work changed the snapshot", () => { + expect(evaluateSprintQaReviewCycleDecision({ + latestRuns: [makeRun({ status: "completed", outcome: "changes_requested", runIndex: 5 })], + maxSprintReviewRuns: 5, + shouldRunReview: true, + })).toEqual({ action: "block_completion", reason: "retry_budget_exhausted" }); }); }); }); diff --git a/tests/backend/services/quality-assurance-service.test.ts b/tests/backend/services/quality-assurance-service.test.ts index aba2bd10bf..062eebe343 100644 --- a/tests/backend/services/quality-assurance-service.test.ts +++ b/tests/backend/services/quality-assurance-service.test.ts @@ -530,6 +530,73 @@ describe("QualityAssuranceService", () => { continuationSkippedReason: "target_task_already_merged", createdFollowUpTaskKeys: ["T02"], }); + + projectRepository.updateTask(tasks[1]!.id, { + status: "completed", + isMerged: true, + mergeIndicator: "MERGED", + }); + const completedFollowUp = projectRepository.getTask(tasks[1]!.id)!; + const preFinalSubtasks = [ + { + record_id: task.id, + project_id: project.id, + sprint_id: sprint.id, + id: "T1", + title: "Merged task", + prompt: "Implement the merged task.", + depends_on: [], + is_independent: true, + status: "COMPLETED", + is_merged: true, + merge_indicator: "MERGED", + }, + { + record_id: completedFollowUp.id, + project_id: project.id, + sprint_id: sprint.id, + id: completedFollowUp.taskKey, + title: completedFollowUp.title, + prompt: completedFollowUp.promptMarkdown, + depends_on: ["T1"], + is_independent: false, + status: "COMPLETED", + is_merged: true, + merge_indicator: "MERGED", + }, + ] as any; + const secondRun = qaReviewRepository.createRun({ + projectId: project.id, + sprintId: sprint.id, + sprintRunId: sprintRun.id, + triggerType: "sprint_completion", + runIndex: 2, + payload: { taskSnapshot: buildSprintQaSnapshot(preFinalSubtasks) }, + }); + qaReviewRepository.updateRun(secondRun.id, { + status: "completed", + outcome: "changes_requested", + summaryMarkdown: "One final verification is required.", + payload: { taskSnapshot: buildSprintQaSnapshot(preFinalSubtasks) }, + finishedAt: new Date().toISOString(), + }); + preFinalSubtasks[1].prompt = `${preFinalSubtasks[1].prompt}\nVerified implementation evidence.`; + + const finalOutcome = await service.reviewSprintCompletion({ + projectId: project.id, + sprintId: sprint.id, + sprintRunId: sprintRun.id, + repoPath: dir, + subtasks: preFinalSubtasks, + }); + + expect(finalOutcome.reportText).toContain("No additional automatic follow-up tasks were created"); + expect(projectRepository.listTasks(project.id, sprint.id)).toHaveLength(2); + expect(qaReviewRepository.getLatestSprintRun(sprint.id)?.payload).toMatchObject({ + continuationSkippedReason: "sprint_qa_retry_budget_exhausted", + automaticFollowUpSuppressedReason: "sprint_qa_retry_budget_exhausted", + createdFollowUpTaskKeys: [], + }); }); it("does not rerun sprint QA after a passing result with no meaningful sprint changes", async () => { @@ -661,6 +728,7 @@ describe("QualityAssuranceService", () => { const projectRepository = new ProjectManagementRepository(storage); const executionRepository = new ExecutionRepository(storage); const qaReviewRepository = new QaReviewRepository(storage); + const attentionRepository = new ProjectAttentionRepository(storage); const providerRunner = { runProviderForText: vi.fn(), runProvider: vi.fn(), @@ -716,6 +784,15 @@ describe("QualityAssuranceService", () => { }), getGithubToken: () => undefined, sendSessionMessage: async () => ({}), + projectAttentionService: { + listActiveProjectItems: (projectId: string) => attentionRepository.listProjectAttentionItems(projectId, { + statuses: ["open", "claimed"], + limit: 500, + }), + openItem: (input: Parameters[0]) => ( + attentionRepository.openOrRefreshItem(input) + ), + } as any, }); const initialSubtasks = [ @@ -788,6 +865,23 @@ describe("QualityAssuranceService", () => { reportText: expect.stringContaining("Sprint QA is still waiting on follow-up work"), }); expect(providerRunner.runProviderForText).not.toHaveBeenCalled(); + expect(attentionRepository.listProjectAttentionItems(project.id, { + statuses: ["open", "claimed"], + })).toEqual([ + expect.objectContaining({ + sprintId: sprint.id, + taskId: null, + attentionType: "human_escalation_required", + ownerType: "human", + payload: expect.objectContaining({ + sourceAttentionType: "qa_review", + qaScope: "sprint", + qaReason: "retry_budget_exhausted", + attempts: 1, + maxAttempts: 1, + }), + }), + ]); }); it("recovers stale running task QA rows when the backing invocation already finished", async () => {