fix(tracker): wire kanban merged-state lookup to GitHubContext

[agent-era-ai]

Summary

• The kanban's merged-state branch (PR Show subdued Merged label on completed tracker cards #224) and the earlier "stays green after merge" guard (PR merged item stays green #221) both read wt.pr.is_merged, but WorktreeInfo.pr was declared and never assigned in production. PR data actually lives on GitHubContext.pullRequests, keyed by worktree path. Switching the lookup unblocks both fixes.
• Extracts a small isItemPRMerged(worktree, pullRequests) helper with unit tests, including one that proves the helper does not consult a stray wt.pr — pinning the lookup path so this can't silently regress a third time.
• Deletes the dead pr?: PRStatus field on WorktreeInfo plus the two getters that depended on it (needs_attention, action_priority) — both never called anywhere. Test fixtures stop mirroring worktree.pr and seed memoryStore.prStatus.set(path, pr) directly where assertions need PR data (matching the production data path).
• Adds a setVisibleWorktrees effect on the kanban so its PR auto-refresh hits this project's currently-visible worktrees, not whatever set the worktree list left in place.

Test plan

• npm run typecheck passes
• npm test — 750/750 pass
• npm run test:terminal — 281/281 pass (1 pre-existing skip)
• Manual: open the tracker board on devteam main; merged-PR items in the cleanup column (terminal-ui-state-detection, requirements-in-worktree) render ◆ Merged in gray instead of their pre-merge state.

🤖 Generated with Claude Code

[claude]

Code Review — PR #229: fix(tracker): wire kanban merged-state lookup to GitHubContext

Overview

This PR correctly diagnoses and fixes a silent wiring bug: TrackerBoardScreen read wt?.pr?.is_merged, but WorktreeInfo.pr was declared and never assigned in production — PR data lives in GitHubContext.pullRequests keyed by worktree path. The fix is surgical and includes a dead-field removal and scoped PR-refresh wiring. Overall this is well-executed; a few things worth calling out.

---

Strengths

• Root cause correctly identified and fixed. Switching to pullRequests[wt.path]?.is_merged via useGitHubContext() is the right lookup path, consistent with how WorktreeListScreen already does it.
• isItemPRMerged extraction is a good call. The inline fix is one line, but the third repeat of the same silent failure justifies naming it and pinning it with tests — including the regression-prevention test that proves the helper ignores a stray wt.pr.
• Dead-code removal is clean. Deleting the pr?, needs_attention, and action_priority fields eliminates the trap for future contributors. The test infrastructure updates are consistent throughout.
• setVisibleWorktrees scoping is correct. Deriving kanbanWorktreePaths from sessionMap via useMemo (and sorting for stable key comparison) means the effect only re-fires when the set of visible worktrees actually changes.

---

Issues

1. Missing integration test — acceptance criterion #5 not met

The requirements list criterion #5 as:

A new test covers the data path: when a fake worktree's PR (in memoryStore.prStatus) has state MERGED, the rendered kanban frame shows the merged indicator. This is the gap that hid the bug.

The new tests in tests/unit/TrackerBoardScreen.test.ts only cover isItemPRMerged in isolation. There is no test that:

1. Seeds memoryStore.prStatus.set(path, new PRStatus({state: 'MERGED'})),
2. Renders the kanban screen, and
3. Asserts ◆ Merged appears in the output frame.

getTrackerCardDisplayState tests pass prMerged: true directly — exactly the "gap that hid the bug" the notes describe. Without this integration test, the same class of wiring bug (e.g., forgetting to pass pullRequests into the render branch, or a future refactor breaking the hook call) could ship undetected again.

2. Comment style

AGENTS.md says "one short line max" for code comments. The block on isItemPRMerged:

// PRs are keyed by worktree path in GitHubContext, never on WorktreeInfo.
// Exported + unit-tested because the obvious-looking `wt.pr.is_merged` spelling
// silently returns undefined and has bitten two prior fixes.

The why is genuinely non-obvious and worth documenting, but this is three lines. Could be trimmed to one, e.g.:

// Reads from GitHubContext (keyed by path), not wt.pr — which is never assigned in prod.

The detailed history lives in the tracker notes and commit messages where it belongs.

3. Manual verification step still unchecked

The test plan notes manual verification against devteam/main as unchecked. Not blocking if that's by design for this environment, but worth confirming before merge.

---

Minor Observations

• The type change in utils.ts (WorktreeInfo['pr'] → PRStatus | undefined | null) is correct and cleaner — no concern.
• Removing the memoryStore.worktrees.set(worktree.path, worktree) call in setupWorktreeWithPR is safe because setupTestWorktree already registers the worktree in the store. Correct, just worth a quick read to confirm for reviewers unfamiliar with the test infrastructure.

---

Summary

The core fix and dead-code removal are solid. The main ask before merging: add at least one E2E or rendered test that goes through the full data path (memoryStore → GitHubContext → kanban render → ◆ Merged visible), which is what the requirements called out as the gap that allowed this class of bug to ship twice.

[claude]

Code Review: fix(tracker): wire kanban merged-state lookup to GitHubContext

Overall: Approve with minor notes. This is a well-reasoned, well-scoped fix that correctly diagnoses a structural mismatch and resolves it without over-engineering.

---

What the PR Does

The kanban was computing prMerged from wt?.pr?.is_merged, but WorktreeInfo.pr was never assigned in production — PR data lives in GitHubContext.pullRequests keyed by worktree path. This meant the merged label (PR #224) and the "stays green after merge" guard (PR #221) were both silently dead since they shipped. This PR wires the correct data source, removes the dead field that was a repeating trap, scopes kanban PR refreshes to the kanban's own visible worktrees, and pins the data path with unit + terminal E2E tests.

---

Code Quality

Strengths:

• isItemPRMerged extraction is the right call. The lookup is one line, but the fact that two prior PRs fell into the same trap justifies extracting, naming, and independently testing it. The test that explicitly proves the helper ignores a stray wt.pr (even if someone reintroduces the field) is a good regression pin.
• Dead code removal is clean and complete. Deleting pr?: PRStatus, needs_attention, and action_priority from WorktreeInfo eliminates the trap entirely — a future contributor can no longer mistake wt.pr for the canonical source. The two removed getters were provably uncalled.
• Test fixture alignment. Stripping worktree.pr = pr from helpers and having them seed memoryStore.prStatus directly mirrors the real data path through FakeGitHubService → GitHubContext.pullRequests. This is the right fix for the fixtures.
• The terminal E2E test covers the gap. A test through memoryStore.prStatus → FakeGitHubService → GitHubContext.pullRequests → kanban render → assert label is exactly the missing coverage that hid the bug both times.
• kanbanWorktreePaths memo is correct. Array.from(sessionMap.values(), w => w.path).sort() properly derives the kanban-scoped worktree set, and the useEffect fires only when the set of paths changes.

Minor notes:

• Comment on isItemPRMerged (line ~90). The comment // Reads from GitHubContext (keyed by path), not wt.pr — which is never assigned in prod. documents past failure more than present behavior. With wt.pr now deleted, "not wt.pr" is vacuously true — future readers won't know there was a wt.pr to avoid. Consider dropping this or condensing to just document why the function exists as a separate helper rather than an inline lookup (i.e., the regression-pinning rationale).

• (wtWithStrayPr as any).pr = new PRStatus(...) in the test. The any cast is acceptable here since it's deliberately simulating a future regression, but it would silently fail to test anything if someone later adds a readonly pr property. The test comment explains the intent well; just worth being aware of the brittleness.

• setVisibleWorktrees effect deps. [kanbanWorktreePaths, setVisibleWorktrees] — if setVisibleWorktrees is a new function reference on every render, this effect fires on every render regardless of the path set change. Assuming setVisibleWorktrees is stable (defined outside the provider's render path or memoized), this is fine. Worth verifying in GitHubContext.tsx if it hasn't already been confirmed.

---

Test Coverage

Test added	What it covers
5 × isItemPRMerged unit tests	Happy path, open PR, no entry, null worktree, stray-field regression
Terminal E2E tracker-board-merged-label.test.mjs	Full path: memoryStore.prStatus → FakeGitHubService → GitHubContext → kanban render → ◆ Merged label
Updated archived/data-flow/unarchive fixtures	Use correct store path, not worktree.pr

This is exactly the coverage that was missing before. Good addition.

---

Performance

No regressions. The kanbanWorktreePaths memo is cheap (one pass over sessionMap values), and setVisibleWorktrees scoping is strictly better than the prior state where the kanban didn't register any visible worktrees at all.

---

Security

No concerns.

---

Summary

The diagnosis is correct, the fix is minimal and precisely targeted, the dead field is gone, and the data path is now pinned end-to-end. The one thing worth double-checking before merge is that setVisibleWorktrees from useGitHubContext() is a stable function reference so the effect doesn't fire on every render. Otherwise this is ready to merge.