Optimize the workspace metadata monitor#428
Conversation
Greptile SummaryThis PR optimizes the workspace metadata monitor by caching the resolved
Confidence Score: 3/5Not safe to merge as-is: the P1 catch path in loadTaskWorkspaceMetadata can permanently lock a task worktree out of git metadata if its repoRoot becomes invalid. One confirmed P1 (stale repoRoot infinite failure loop in loadTaskWorkspaceMetadata catch path) brings the ceiling to 4; a second gap (stateToken-match not updating repoRoot) means the optimization never recovers after an error, warranting a score of 3. src/server/workspace-metadata-monitor.ts — specifically the catch block and stateToken-match early return in loadTaskWorkspaceMetadata.
|
| Filename | Overview |
|---|---|
| src/server/workspace-metadata-monitor.ts | Adds repoRoot caching to both CachedHomeGitMetadata and CachedTaskWorkspaceMetadata; loadHomeGitMetadata is correctly fixed, but loadTaskWorkspaceMetadata has two gaps: the catch path returns stale repoRoot (P1 infinite-failure loop) and the stateToken-match path doesn't propagate the fresh repoRoot from the probe (P2, optimization doesn't recover after errors). |
| src/workspace/git-sync.ts | probeGitWorkspaceState accepts an optional repoRoot to skip resolveRepoRoot; headCommit is now parsed from the git status porcelain output (# branch.oid) instead of a separate rev-parse call; getTrackedDiffTotals skips git diff --numstat when there are no tracked changes; countUntrackedAdditions early-exits on empty input; getGitSyncSummary runs both in parallel. All changes look correct. |
| web-ui/src/components/card-detail-view.tsx | Detail diff polling is now gated on sessionSummary?.state === "running", avoiding unnecessary polling when no task is active. Comment updated to reflect the narrower condition. Change is correct and intentional. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Poll timer fires] --> B[loadHomeGitMetadata]
A --> C[loadTaskWorkspaceMetadata × N tasks]
B --> B1{"cached repoRoot?"}
B1 -->|yes| B2[probeGitWorkspaceState\nwith repoRoot — skips resolveRepoRoot]
B1 -->|no| B3[probeGitWorkspaceState\ncalls resolveRepoRoot]
B2 --> B4{stateToken match?}
B3 --> B4
B4 -->|yes| B5["return {...entry.homeGit, repoRoot: probe.repoRoot} ✅"]
B4 -->|no| B6[getGitSyncSummary\n→ getTrackedDiffTotals\n→ countUntrackedAdditions]
B6 --> B7[cache summary + stateToken + repoRoot]
B2 -->|throws| B8["return {...entry.homeGit, repoRoot: null} ✅"]
C --> C1{"cached repoRoot?"}
C1 -->|yes| C2[probeGitWorkspaceState\nwith repoRoot]
C1 -->|no| C3[probeGitWorkspaceState\ncalls resolveRepoRoot]
C2 --> C4{stateToken match?}
C3 --> C4
C4 -->|yes| C5["return current ❌ repoRoot not updated"]
C4 -->|no| C6[getGitSyncSummary]
C6 --> C7[cache data + stateToken + repoRoot ✅]
C2 -->|throws, current exists| C8["return current ❌ stale repoRoot"]
Comments Outside Diff (1)
-
src/server/workspace-metadata-monitor.ts, line 262-265 (link)Stale
repoRootnot cleared on error — infinite failure loopWhen
probeGitWorkspaceStatethrows using a cached (now-invalid)repoRoot(e.g. a worktree was moved), the catch block returnscurrentunchanged. That preserves the stalerepoRoot, so the next poll passes the same invalid root in again, fails again, and the function never self-heals.loadHomeGitMetadatawas fixed with this exact pattern in this PR; the same fix is needed here.Prompt To Fix With AI
This is a comment left during a code review. Path: src/server/workspace-metadata-monitor.ts Line: 262-265 Comment: **Stale `repoRoot` not cleared on error — infinite failure loop** When `probeGitWorkspaceState` throws using a cached (now-invalid) `repoRoot` (e.g. a worktree was moved), the catch block returns `current` unchanged. That preserves the stale `repoRoot`, so the next poll passes the same invalid root in again, fails again, and the function never self-heals. `loadHomeGitMetadata` was fixed with this exact pattern in this PR; the same fix is needed here. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/server/workspace-metadata-monitor.ts
Line: 262-265
Comment:
**Stale `repoRoot` not cleared on error — infinite failure loop**
When `probeGitWorkspaceState` throws using a cached (now-invalid) `repoRoot` (e.g. a worktree was moved), the catch block returns `current` unchanged. That preserves the stale `repoRoot`, so the next poll passes the same invalid root in again, fails again, and the function never self-heals. `loadHomeGitMetadata` was fixed with this exact pattern in this PR; the same fix is needed here.
```suggestion
} catch {
if (current) {
return { ...current, repoRoot: null };
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/server/workspace-metadata-monitor.ts
Line: 236-243
Comment:
**`repoRoot` not refreshed on stateToken match — optimization never re-engages after recovery**
When the stateToken matches, `current` is returned unchanged. If `current.repoRoot` is `null` (cleared by a prior error), it stays `null` even though `probe.repoRoot` is freshly valid — so every subsequent poll must re-run `resolveRepoRoot` indefinitely, negating the core optimization of this PR. Compare with `loadHomeGitMetadata`, which does `return { ...entry.homeGit, repoRoot: probe.repoRoot }` on a match.
```suggestion
if (
current &&
current.stateToken === probe.stateToken &&
current.data.path === pathInfo.path &&
current.data.baseRef === pathInfo.baseRef
) {
return { ...current, repoRoot: probe.repoRoot };
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "Address comments" | Re-trigger Greptile
1 similar comment
|
One freshness concern with the new polling behavior: when a task finishes, users may be looking at the kanban board and see the card move from in progress to review, but the file/line change counts on the card can still reflect the previous metadata snapshot until the 10s poll catches up. That could be confusing because the task appears ready for review before the visible git status has caught up. Could we trigger an immediate workspace metadata refresh when a task session transitions out of running, or specifically when it enters review? That should keep the idle optimization while preserving immediate card status freshness at the point users are most likely to look at it. It should also bump taskWorkspaceStateVersion, which helps the open detail diff refetch without waiting for the slower poll. |
This PR uses the data we obtain from the probe to prevent getting the diff totals. This reduces the function execution time from
11.534msto0.001ms, which is mostly the case when polling with no changes yet.