fix(tracker): live AI status takes precedence over stale status.json

[agent-era-ai]

Summary

• A worktree's tracker/items/<slug>/status.json lags behind reality whenever the agent forgets to update it on resume. The kanban OR'd aiStatus === 'waiting' with the file's waiting_for_input / waiting_for_approval flags, so an actively running agent kept showing yellow "waiting for you" or green "Ready — …" until the file caught up.
• Extracted computeCardStatusFlags as the single source of truth: when live tmux says working / active, both file-based waiting states are suppressed. Real consent gates (aiStatus === 'waiting') and no-session fallbacks keep their existing colours.
• Routed the title-bar ! N waiting / ⟳ N running count and the project-switcher counts through the same helper so all three displays agree, with getItemStatus results cached once per render.

Test plan

• npx tsc -p tsconfig.test.json clean
• npx jest tests/unit/TrackerBoardScreen.test.ts — 24/24 (9 new precedence cases)
• npx jest tests/unit/ — 562/563 (one unrelated flake in dialog-navigation-bug.test.ts, passes on its own)
• npm run build clean
• CI: typecheck + unit + e2e + terminal tests pass

🤖 Generated with Claude Code

[claude]

Code Review — PR #233: live AI status takes precedence over stale status.json

Overview

This PR extracts a computeCardStatusFlags helper to eliminate the stale-status bug where an actively running agent kept showing "waiting for you" or "Ready — …" because status.json lagged behind the live tmux signal. It also caches getItemStatus results once per render and routes all three display sites (kanban cards, title-bar tally, project-switcher counts) through the same helper.

This is a well-motivated, focused fix. The logic is correct and the test coverage is good.

---

Code Quality

computeCardStatusFlags (TrackerBoardScreen.tsx)

• Clean interface: named input object → named output object, easy to test.
• The precedence rule is correct: live working/active suppresses both file-based waiting states; live waiting (consent gate) is always honoured.
• hasSession correctly treats not_running as "no session" which preserves existing semantics.

itemStatusBySlug cache (TrackerBoardScreen.tsx)

• Good call. Avoids paying disk-read cost twice per render (once for the tally loop, once for the card loop).
• Minor concern: the map is built in the component body (not inside a useMemo), so it is re-created on every render including pure-cosmetic re-renders. Given the comment about re-rendering on every keystroke and that getItemStatus reads disk, wrapping this in useMemo (with board as dependency) would be a small but meaningful win. Not a blocker, just worth noting.

useMemo deps in TrackerProjectPickerDialog.tsx

• pullRequests is correctly added to the dependency array. ✓

---

Architecture Concern

TrackerProjectPickerDialog.tsx imports computeCardStatusFlags and isItemPRMerged directly from ../../screens/TrackerBoardScreen.js. Per the project's architecture (AGENTS.md), dialogs live under components/dialogs/ and screens live under screens/ — importing upward from a dialog into a screen is a mild layering violation.

Suggestion: move these two pure utility functions to a shared module (e.g. src/shared/utils/trackerStatus.ts or alongside TrackerService) so both the screen and the dialog can import without a cross-layer dependency.

---

Potential Bug

The old ralphWaiting check had an explicit !!itemStatus && guard:

const ralphWaiting = !!itemStatus && !readyToAdvance && service.isItemWaiting(itemStatus);

The new version drops it:

const ralphWaiting = !isWorking && !readyToAdvance && freshWaiting;

This is fine as long as service.isItemWaiting(null) always returns false. The callers pass freshWaiting: service.isItemWaiting(itemStatus) where itemStatus may be null (from itemStatusBySlug.get(item.slug) ?? null). If isItemWaiting(null) correctly returns false, the behaviour is identical. Worth a quick confirmation / defensive test to lock that in.

---

Test Coverage

9 new cases in computeCardStatusFlags — all important precedence branches are covered. The test names are clear and the toEqual snapshots make the expected state unambiguous. ✓

One missing case: aiStatus === 'waiting' with freshReady: true (consent gate while file also reports ready-to-advance). Currently readyToAdvance would be false (because !isWorking && freshReady — wait, isWorking is false when aiStatus === 'waiting', so readyToAdvance would be !prMerged && !false && freshReady = true). That means a consent-gate card would simultaneously show isWaiting: true and readyToAdvance: true. Is that intentional? If not, the condition should suppress readyToAdvance when the AI is waiting.

---

Summary

Solid fix with good tests. Two items to address before merge:

1. Confirm service.isItemWaiting(null) === false (or add a test for it).
2. Consider extracting computeCardStatusFlags / isItemPRMerged to a shared utility to avoid the dialog→screen import.
3. (Optional) useMemo on itemStatusBySlug to avoid unnecessary disk reads on keystroke re-renders.

[claude]

Code Review — PR #233: live AI status takes precedence over stale status.json

Overview

The fix is conceptually correct and well-executed. The core problem (stale status.json overriding a live tmux signal) is real and the extraction of computeCardStatusFlags as the single source of truth is the right approach. The itemStatusBySlug memoization is a nice bonus that prevents per-keystroke disk reads.

---

Issues

1. Potential display bug — aiWaiting + freshReady combination (medium)

getTrackerCardDisplayState checks readyToAdvance before isWaiting. That means when a live consent gate (aiStatus === 'waiting') coincides with a stale waiting_for_approval in status.json (freshReady=true), the card shows green "Ready ✓" instead of yellow "! waiting":

// In computeCardStatusFlags:
const readyToAdvance = !prMerged && !isWorking && freshReady;
//                                  ^^^^^^^^^^
//                   isWorking is suppressed, but aiWaiting is not

const isWaiting = aiWaiting || ralphWaiting;
// Both readyToAdvance and isWaiting can be true simultaneously

The same suppression that protects isWorking should protect aiWaiting:

const readyToAdvance = !prMerged && !isWorking && !aiWaiting && freshReady;

This scenario is uncommon but reachable (e.g. user starts a new Claude session in a worktree where the previous run left waiting_for_approval in status.json). The existing test suite doesn't cover it — a case like {aiStatus: 'waiting', freshReady: true, freshWaiting: true} should assert readyToAdvance: false, isWaiting: true.

---

2. Architecture — Screen importing from a Dialog is backwards (important)

TrackerProjectPickerDialog.tsx now imports from TrackerBoardScreen.tsx:

import {computeCardStatusFlags, isItemPRMerged} from '../../screens/TrackerBoardScreen.js';

The project's layering (AGENTS.md) puts screens and dialogs at the same component layer; neither should import from the other. This creates a lateral coupling that will break if TrackerBoardScreen is ever restructured and makes TrackerProjectPickerDialog harder to test in isolation.

computeCardStatusFlags and isItemPRMerged are pure functions with no React dependency — they belong in a shared utility module (e.g. src/utils/trackerStatus.ts) or in TrackerService (closer to the rest of the status-reading logic). Both callers import from there instead of from a screen.

---

3. Missing tests for the edge case above

The 9 new computeCardStatusFlags test cases cover the main paths well. The one gap is the aiWaiting + freshReady combination noted above. Worth adding regardless of whether the logic fix lands.

---

What's working well

• computeCardStatusFlags as a pure, exported function — easy to unit-test, single responsibility, clearly named parameters. The shape of the change is right.
• itemStatusBySlug memoized on [board, worktrees] — correctly amortises disk reads across a render cycle so keystrokes don't replay I/O. Good call.
• Test quality — the 9 cases are meaningful and document intent clearly. The aiStatus: 'active' case and the prMerged suppression test are particularly good.
• Title-bar and project-switcher counts now go through the same helper — this was the root of the original inconsistency and is properly fixed.
• getSessionForItem vs getWorktreeForItem swap — using getWorktreeForItem and letting computeCardStatusFlags handle not_running internally is cleaner and correct.

---

Summary

The core logic is sound. Two things to address before merging:

1. Move computeCardStatusFlags / isItemPRMerged out of TrackerBoardScreen.tsx into a shared utility to avoid the screen→dialog import inversion.
2. Guard readyToAdvance against aiWaiting (and add the corresponding test case).

[claude]

Code Review — PR #233: live AI status takes precedence over stale status.json

Overview

Extracts status-precedence logic into a shared pure utility (computeCardStatusFlags) and moves isItemPRMerged alongside it. Ensures the title-bar tally, project-switcher counts, and per-card rendering all apply the same precedence rules. Also memoizes per-item disk reads to avoid redundant I/O during renders.

This is a well-scoped fix for a real UX bug.

---

Code Quality ✅

src/shared/utils/trackerCardStatus.ts — clean extraction. The function is pure, has no side-effects, and is easy to test in isolation. Moving it out of the screen component is the right call given it's now consumed by three different callers.

Precedence chain is correct. Tracing through edge cases:

• working + freshWaiting=true → isWorking=true, isWaiting=false ✓ (stale file overridden)
• waiting + freshReady=true → isWaiting=true, readyToAdvance=false ✓ (consent gate wins)
• undefined + freshWaiting=true + freshReady=true → readyToAdvance=true ✓ (no session, file-based)
• prMerged=true + freshReady=true → readyToAdvance=false ✓

Performance improvement is real. itemStatusBySlug memoized on [board, worktrees] means disk reads happen once per refresh tick instead of once per item per render pass (previously called twice per item in the board render loop).

---

Suggestions / Minor Issues

1. ralphWaiting has an implicit dependency on readyToAdvance that could be made explicit:

const ralphWaiting = !isWorking && !readyToAdvance && freshWaiting;

When aiWaiting=true, readyToAdvance is forced false, so ralphWaiting can be true if freshWaiting=true. That makes isWaiting = aiWaiting || ralphWaiting = true || true = true, which is correct, but the double-true path is slightly surprising. Consider:

const ralphWaiting = !isWorking && !aiWaiting && !readyToAdvance && freshWaiting;

This would be logically equivalent (since readyToAdvance already guards against aiWaiting) and arguably clearer that file-based waiting only applies when there's no live tmux signal at all.

2. The worktrees dep in itemStatusBySlug memo is load-bearing but invisible:

}, [board, worktrees, service, projectPath]);

worktrees isn't actually read inside the memo body — it's there to re-trigger on the same refresh tick that delivers new session state. The existing comment explains this, but a brief inline note (// included to re-run on each status refresh tick) would keep the next reader from pruning it as an unnecessary dep.

3. Test completeness — one case worth adding: the freshWaiting=false, freshReady=true path (a "ready to advance" file state where isItemWaiting returns false but isItemReadyToAdvance returns true). This tests that readyToAdvance fires without freshWaiting set, which is the "waiting_for_approval only" path.

---

Test Coverage ✅

9 new unit tests covering the key state transitions. The naming is precise ("live working overrides file-based waiting_for_input (yellow → cyan)") which makes failures self-documenting.

---

Security / Risk

No external inputs; all flags are derived from in-memory state. No concerns.

---

Verdict: Approve with minor suggestions. The core logic is correct and well-tested. Suggestions above are non-blocking style/clarity improvements.

[claude]

Code Review — PR #233: fix(tracker): live AI status takes precedence over stale status.json

Overview

This PR fixes a real UX bug: a stale status.json file could make an actively-running agent appear yellow ("waiting for you") or green ("Ready — …") until the file caught up. The fix extracts computeCardStatusFlags as the single source of truth for card status rendering, and routes all three display locations (title-bar tally, project-switcher counts, per-card render) through it. This is a clean, well-scoped refactor.

---

✅ Strengths

• Correct precedence rules. Live tmux beats stale file; real consent gates (aiStatus === 'waiting') are preserved; PR-merged state correctly suppresses readyToAdvance. The logic in computeCardStatusFlags is sound.
• Good extraction. Moving isItemPRMerged alongside computeCardStatusFlags into trackerCardStatus.ts is the right call — they belong together and both callers now import from one place.
• Memoized disk reads. itemStatusBySlug avoids calling service.getItemStatus on every keystroke re-render. The memo deps ([board, worktrees, service, projectPath]) correctly invalidate on the next refresh tick.
• Consistent counts. All three display locations now agree by construction. Previously the title-bar count and the per-card render used slightly different code paths.
• Test coverage. 9 new targeted tests cover the key precedence cases (working > waiting, working > ready-to-advance, consent gate preservation, prMerged suppression). That's the right surface to unit-test.

---

Issues & Suggestions

Medium — worktrees as a sentinel dependency is fragile

// `worktrees` isn't read inside the memo body — it's the refresh tick we
// re-run on, since session state changes alongside it.
}, [board, worktrees, service, projectPath]);

React's exhaustive-deps rule would flag worktrees as unused here (and rightly so). Using an unread variable as a cache-busting sentinel is non-obvious and will surprise the next person to touch this. Consider instead keying on a stable refresh counter/timestamp that explicitly tracks "last worktree update," or simply accept that board (which presumably changes on refresh) is the real invalidation signal and drop worktrees from the deps. If board doesn't change reference when worktrees update but items don't, that's worth documenting explicitly.

Minor — service.isItemWaiting(null) / isItemReadyToAdvance(null) called unconditionally in the project picker

const itemStatus = w.feature && hasTracker
  ? service.getItemStatus(p.path, w.feature)
  : null;
const flags = computeCardStatusFlags({
  ...
  freshWaiting: service.isItemWaiting(itemStatus),   // null when !hasTracker
  freshReady:   service.isItemReadyToAdvance(itemStatus),
});

This is safe if isItemWaiting(null) and isItemReadyToAdvance(null) both return false (which they likely do). But it's not visible from this diff — a brief comment noting the null-safe contract, or moving the guard into computeCardStatusFlags's callsite, would make the intent clear.

Minor — Removed sessWt guard is now implicit

The old code had:

const sessWt = (wt?.session?.ai_status && wt.session.ai_status !== 'not_running') ? wt : null;

This filtered out not_running sessions before computing status. The new code passes raw wt and handles it inside computeCardStatusFlags via hasSession = !!aiStatus && aiStatus !== 'not_running'. The behavior is preserved, but the removal of the intermediate variable makes it slightly less obvious to readers that not_running is handled. The test 'not_running session reports hasSession=false' covers it, which is good.

Minor — Missing test: aiStatus === 'waiting' + freshReady === true precedence in the project-picker path

The computeCardStatusFlags tests cover this case at the utility level (the "yellow consent gate, not green Ready" test), but there's no test verifying that TrackerProjectPickerDialog's waiting / working counts are correct when the same scenario occurs. This would be a good follow-up for the project picker path.

Nit — Overly long block comment before computeCardStatusFlags

The 7-line comment above the function restates what the parameter names already express. The key insight worth keeping is just the precedence rule. Suggest trimming to the one-liner that matters most:

// Live tmux status takes precedence over stale status.json signals.
export function computeCardStatusFlags(...

---

Test Plan Completeness

The PR reports 562/563 passing (one unrelated flake). The 9 new computeCardStatusFlags tests cover the critical paths. The existing getTrackerCardDisplayState and isItemPRMerged tests are preserved. This looks good — no gaps in the new logic's coverage.

---

Summary

Approve with minor suggestions. The core fix is correct, well-tested, and properly extracted. The main thing worth addressing before merge is the worktrees-as-sentinel memo dependency — either document it more clearly or restructure so the dependency is explicit. The other items are nits or follow-ups.