Skip to content

Needs Attention surfaces builders whose PR is already merged (stale prReady) #901

@waleedkadous

Description

@waleedkadous

Symptom

After a PR is merged, the corresponding builder still appears in the Needs Attention list on the dashboard with a "PR review" badge — even though there is nothing left for the human to do. Observed on shannon today: #1842 port-shannon-landing-page-next (PR #1864 merged 18:35 UTC) was still surfacing under Needs Attention with a 1-day elapsed counter.

Repro: merge a SPIR/AIR/PIR builder's PR via gh pr merge --merge --admin. Builder advances to phase: verified. Refresh the dashboard. Row persists.

Root cause

Two interacting problems that combine to produce the stale row:

1. Data hazard from the v3.1.4 → v3.1.5 transition (already-shipped state)

In-flight builders that crossed the v3.1.4 → v3.1.5 boundary have stale pr_ready_for_human: true in their status.yaml. The v3.1.4 index.ts:advanceProtocolPhase had a line-453 setter that re-set pr_ready_for_human = true on the post-merge porch done (the terminal prverified advance). Issue #887 / PR #888 removed that setter, but for builders that already wrote the state under v3.1.4, the stale true remains.

Concrete example: /Users/mwk/Development/cluesmith/shannon/.builders/spir-1842/codev/projects/1842-port-shannon-landing-page-next/status.yaml shows phase: verified, gates.pr.status: approved, pr_ready_for_human: true.

This issue is self-correcting for new builders going forward (#888's removal of line-453 stops the re-set). But the consumer-side filter still has bug #2 below, which we should fix regardless.

2. NeedsAttentionList builder-fallback loop has no merged-PR awareness

The iter-2 fallback in packages/dashboard/src/components/NeedsAttentionList.tsx:buildItems (added per Gemini's catch on PR #874) emits a "PR review" row for any builder with b.prReady === true whose PR didn't appear in the prs array. The intent was to defend against the case "PR missing from cache due to GitHub API pagination or transient failure." But the same code path fires when the PR has been merged — in which case the PR is correctly absent from prs (which lists only OPEN PRs), and surfacing the row is wrong.

The fallback doesn't distinguish "PR cache miss (defensive surface)" from "PR is merged and gone (do not surface)."

Fix

Cross-reference the builder against the workspace's recently-merged PR list (already available in the overview data) before emitting the defensive fallback row. The data we need is already plumbed through: overview.ts:fetchMergedPRsCached populates mergedPRs on the overview response (PROverview[] shape, includes linkedIssue).

Acceptance criteria

  • OverviewData exposes mergedPRs (or recentlyClosed) on the type returned to the dashboard — verify the shape already does this; if not, plumb it through packages/types/src/api.ts.
  • NeedsAttentionList's buildItems accepts the merged-PR list (or equivalent issue-ID set) as input.
  • In the builder-loop fallback: before emitting a prReady-driven row, check if b.issueId matches any merged PR's linkedIssue in the recent window. If yes, skip — the builder is post-merge and the row is stale.
  • PR-loop unchanged (it iterates open PRs only, so already correct).
  • Existing tests stay green. Add a regression test: builder with prReady: true, builder's issueId matches a merged-PR linkedIssue → row NOT emitted.
  • Existing defensive test (still surfaces a prReady builder when its PR is missing from prs) updated to clarify the scenario: the missing PR must NOT be in the merged list (only "missing-due-to-cache" cases get the defensive emit).

Out of scope

Files to touch

  • packages/dashboard/src/components/NeedsAttentionList.tsx — main change to buildItems signature and loop logic
  • packages/dashboard/__tests__/NeedsAttentionList.test.tsx — add regression test, update the defensive test
  • packages/codev/src/agent-farm/servers/overview.ts — confirm mergedPRs is exposed in the response (may already be — verify)
  • packages/types/src/api.ts — only if the merged-PR shape needs to be added to the overview type

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions