Repository navigation
feat(plugins): ship Agent Session Recap (DAN-77) - #140
Conversation
Capture the Tier 1-4 marketplace backlog against today's declarative plugin model, then add a Strand-maintained Recap builtin that summarizes files, risky paths, and leftover TODOs in the active worktree. Co-authored-by: Daniels <danielss-dev@users.noreply.github.com>
danielss-dev
left a comment
danielss-dev
left a comment
There was a problem hiding this comment.
Bugbot pass on Agent Session Recap (DAN-77). Found concrete failures in how Recap merges/loads patches for the TODO scan — not style nits. No approve/merge.
| linkedWorktree: meta?.is_linked_worktree ?? false, | ||
| baselineShort: baseline?.short ?? null, | ||
| status: canReadRepo ? status : [], | ||
| diffs: canReadRepo ? uniqueDiffs(unstagedDiffs, stagedDiffs, review) : [], |
There was a problem hiding this comment.
bug Review pool overwrites loaded local patches — TODO scan stays empty
Failure mode: uniqueDiffs(unstagedDiffs, stagedDiffs, review) keeps the last pool per path. This effect loads patches into unstagedDiffs / stagedDiffs, but the model then prefers baselineDiffs / reviewUnstagedDiffs from refreshReviewDiffs(), which only writes summaries (patchLoaded: false, empty patch via mergeDiffSummaries). buildAgentSessionRecap skips those rows (patchLoaded === false || !diff.patch), so patchesScanned stays 0 and the UI permanently shows “Patches are not loaded yet…” even after the load Promise succeeds.
Same last-write-wins also drops the unstaged side when a path is partially staged (uniqueDiffs(unstaged, staged) keeps staged only), so TODOs only in the unstaged hunk are never scanned.
Evidence: refreshReviewDiffs → repoDiffSummary only; Recap retains + refreshes review, then discards the loaded local FileDiffs here.
Fix sketch: When merging pools, prefer an entry with patchLoaded !== false, or scan patches from every pool independently (no path Map overwrite). For partial stages, union both sides’ patches for the TODO extractor.
There was a problem hiding this comment.
Fixed in 8188117: uniqueDiffs prefers loaded patches; Recap uses unionDiffs so unstaged+staged patches for the same path are both scanned.
| .map((diff) => diff.path); | ||
| return Promise.all([ | ||
| unstaged.length ? current.loadDiffFiles('unstaged', unstaged) : Promise.resolve(), | ||
| staged.length ? current.loadDiffFiles('staged', staged) : Promise.resolve(), |
There was a problem hiding this comment.
bug Review patches are never loaded — baseline-only sessions never scan TODOs
Failure mode: The loader only calls loadDiffFiles('unstaged' | 'staged', …). Paths that exist only in baselineDiffs / reviewUnstagedDiffs (pinned Review baseline + clean worktree, or review-only rows) never receive patches. The empty-state copy tells users to “Pin a Review baseline”, then the ready UI sticks on plugins.recap.todosUnavailable (“not loaded yet”) forever — not a transient loading state.
Evidence: Missing list is built from uniqueDiffs(unstagedDiffs, stagedDiffs) only; no loadDiffFiles('review', …) despite retainDiffs(path, 'review') + refreshReviewDiffs().
Fix sketch: After refresh, also take up to RECAP_PATCH_SCAN_LIMIT review paths with patchLoaded === false and loadDiffFiles('review', paths) (keep the bound so Recap does not become a whole-tree loader).
There was a problem hiding this comment.
Fixed in 8188117: recapMissingPatchPaths also loadDiffFiles('review', …) up to RECAP_PATCH_SCAN_LIMIT.
| releaseReview(); | ||
| releaseLocal(); | ||
| }; | ||
| }, [visible, path, baseline?.oid, canReadRepo]); |
There was a problem hiding this comment.
bug Patch load effect does not re-run when the session keeps changing
Failure mode: Deps are only [visible, path, baseline?.oid, canReadRepo]. While Recap stays mounted/visible, further agent edits add paths or mergeDiffSummaries resets patchLoaded: false when the content revision changes. The file list updates via useMemo, but those patches are never loaded again, so leftover TODOs in the live session are missed until remount or baseline change.
Evidence: Effect cleanup/retainers keep diffs refreshing via needsDiffs, but this effect itself does not re-enter the load pipeline; mergeDiffSummaries intentionally clears patches on revision mismatch.
Fix sketch: Also depend on something that moves when unloaded paths appear (e.g. diffsTick, or a stable fingerprint of paths with patchLoaded === false), still capped by RECAP_PATCH_SCAN_LIMIT.
There was a problem hiding this comment.
Fixed in 8188117: load effect depends on recapUnloadedPatchKey so new/reset unloaded paths re-trigger load without looping on refresh.
Prefer loaded patches over review summaries, union unstaged and staged sides, load review-only paths, and re-fetch when unloaded paths appear. Co-authored-by: Daniels <danielss-dev@users.noreply.github.com>
2 participants
Implements DAN-77: capture Claude plugin research against today's declarative model, then ship Agent Session Recap as the first Tier-1 dogfood plugin.
What landed
docs/plugin-marketplace-backlog.md— Tier 1–4 ideas tagged withmarkdown/status,repository.read/ai.invoke, vs new view types,network.fetch, and isolation. Marketplace order recorded; remote catalog explicitly deferred.daniels.session-recap(Strand-maintained, same reservation as Heroi/Quick Notes). Static declarative views cannot follow worktree/review context, so Recap is not a fake static markdown snapshot.repository.readonly. Nolist/badgetypes, Risk Radar, secret sniffer, CI panels, or remote marketplace.Proof
validatePluginManifest; module reserved fordaniels.session-recap.pnpm --filter ./ui exec tsc --noEmitandpnpm --filter ./ui testpass (includingrecap.test.ts).Linear: DAN-77