mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
feat/opencode2-native-commands
13 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
8f84852f97 |
fix(review): surface partial GitLab comment submissions (#1164)
Surface partial GitLab submission outcomes and preserve narrowed, duplicate-safe retries across dialog reopen and same-tab refresh. Follow-up for an explicit blocked-recovery escape: #1166. |
||
|
Michael Ramos
|
fa294b12e4 |
feat(review): add PR and MR artifact gallery (#1055)
Adds a first-class artifact gallery and annotation workflow for GitHub pull requests and GitLab merge requests, including images, GIFs, video timestamps, rendered HTML, Markdown, provenance-aware feedback, authenticated provider resources, and reliable CDN/media handling. |
||
|
Michael Ramos
|
e3de938914 |
feat(review): PR Overview panel + description/comment annotations + media (#981)
Combine the PR Summary/Comments/Checks tabs into one PR Overview panel, then make the description and comments annotatable and render their media. - PR Overview panel (one sidebar entry) + comment UI (avatars, filters, hide bots, live context, responsive stacking). - Annotate the PR description (select → comment) and PR comments (Annotate button), with Ask AI; notes show in the Annotations sidebar and ship to the agent. - Split/Unified diff toggle relocated into the dock tab strip. - Render images + video in descriptions and comments (raw HTML + markdown), capped to the card so nothing bleeds. - Review-flow fixes: copy-all feedback, prose-only feedback preamble, no image control on prose notes, GitHub review-body seeding; stronger review trailer. - Add Claude Sonnet 5 as the default Ask AI model. No server, endpoint, or Pi-runtime changes. |
||
|
Michael Ramos
|
6ec1a66c9b |
feat(review): large-PR pipeline, instant-open checkout, scroll perf, and worker-pool highlighting (#893)
* feat(review): large GitHub PR fallback + non-blocking PR checkout
Two PR-mode improvements:
1. Large GitHub PRs no longer fail to load. When `gh pr diff` is refused
(HTTP 406 for oversized diffs), fetchGhPR pages through the pulls files
API and stitches the per-file patches into a unified diff — mirroring
the existing GitLab raw_diffs fallback. Path quoting matches git's
exact rules (bare spaces unquoted) so downstream parsers round-trip;
truncation at the API's 3000-file cap is surfaced, never silent.
2. The --local worktree/clone no longer blocks startup. The review server
opens as soon as the platform diff arrives; the checkout warms in the
background as a seeded not-ready pool entry. Consumers that need real
files (agent jobs, full-stack diff, code-nav, semantic diff, AI
sessions) await pool.ensure(), with creations serialized so concurrent
fetches can't clobber the shared FETCH_HEAD. Cross-repo clone steps
converted from spawnSync to async spawns; warmup children are killed
on exit (plus `git worktree prune`) so aborted sessions can't leak
stale registrations; failed checkouts degrade honestly (no agent runs
in the wrong directory claiming local access) with a 30s retry
cooldown.
* fix(review): survive long PR checkout warmups + classify reconstructed renames
Stress-testing against oven-sh/bun#30412 (2,188 files) surfaced three bugs:
- Bun.serve's default 10s idleTimeout killed /api/semantic-diff while it
parked on the background checkout warmup (a clone that can take minutes).
Disable the idle timeout on all servers — AI SSE streams can also stall
>10s between bytes while a permission prompt waits.
- The file-badge hook memoized that failed fetch in a module-level cache
keyed by patch, pinning every badge to empty until a hard refresh. Never
cache failures; retry with backoff (5s/15s/30s).
- reconstructGhPatch/reconstructPatch omitted the `similarity index` line,
which Pierre's parser keys rename classification off — pure renames
rendered as blank plain changes with no old path. Emit 100% for
patch-less renames/copies (exactly accurate) and a synthetic 99% for
patched ones (consumers only branch on 100% vs not).
* feat(review): local full-diff upgrade for PRs whose API diff is truncated
On oversized PRs the platform APIs withhold per-file patch content entirely
(bun#30412: 1,066 of 2,188 files came back with status added/modified, zeroed
counts, and no patch). Those files rendered as empty stubs with no diff.
- fetchGhPR/fetchGlMR flag the result `patchIncomplete` when patch-less
non-rename entries exist or the 3000-file cap truncates the listing.
- New runPRLayerLocalDiff (pr-stack.ts) recomputes the exact layer diff in
the local checkout: platform merge-base + head SHA two-dot diff (three-dot
vs baseSha fallback), fetch-by-SHA for objects missing from shallow clones,
-l0 so rename detection doesn't silently degrade on huge PRs.
- The review UI shows a "Partial diff · Load full diff" notice in layer
scope; clicking re-requests the layer scope and the server swaps in the
recomputed full diff (waiting out the background clone if needed).
- PR scope/switch state writes are epoch-guarded: a request parked on the
checkout warmup can no longer overwrite a newer scope select or pr-switch.
- draftKey follows the upgraded patch so annotation drafts survive pr-switch
round-trips; recompute failures surface in the response error field.
- Pi server mirrors all of it, including an agentCwd fallback so the upgrade
works for PRs switched-to under a cross-repo clone pool.
* fix(review): use GitLab's too_large/collapsed flags for withheld-diff detection
External review caught a false negative: a too-large ADDED file comes back
new_file:true with an empty diff — indistinguishable from a legitimately
empty new file under the old heuristic, so the partial-diff upgrade was
never offered for exactly the files that matter most on big MRs.
The REST /diffs endpoint marks withheld content explicitly per entry
(verified against gitlab.com): too_large/collapsed are now authoritative in
both directions — withheld adds/deletes are flagged, binaries and empty
files are never misflagged. Older GitLab without the fields keeps the
empty-diff-on-modification heuristic.
* feat(prompts): unify review-denied suffix — triage first, no coding off raw feedback
The per-runtime defaults map (#627) gave OpenCode and Pi a different
review-denied suffix than every other runtime; updating one meant the
others silently kept "you must address all of them" — an instruction to
start coding immediately. Claude Code, Amp, Droid, Codex, Copilot, Gemini,
and Kiro were all still on it.
One default for every runtime now: triage the feedback, verify it against
the code, discuss before changing anything. Per-runtime customization
remains available via config (prompts.review.runtimes.<rt>.denied), which
resolves above the built-in default as before.
* fix(prompts): generalize review-denied suffix — 'from review', not 'external AI reviewers'
Review feedback isn't always from AI reviewers or agent jobs; often it's
the human reviewer's own annotations. Neutral wording covers both.
* fix(review): non-blocking 'Load full diff' + flag-handling hardenings
Self-review findings:
- The partial-diff upgrade reused the scope-switch handler, so clicking
"Load full diff" raised the full-screen PRSwitchOverlay — blocking the
entire UI, potentially for minutes behind a cold clone, with no text and
no cancel. The upgrade now has its own loading state: the notice shows a
spinner ("Loading full diff…") and the reviewer keeps working with the
partial diff while the request parks. Server-side epoch guards already
handle scope/PR changes made during the wait.
- GitLab too_large/collapsed: treat explicit null like absent (flags
inconclusive → legacy heuristic decides) instead of silently exonerating.
- Rename-limit lift uses -l100000 instead of -l0 ("0 = unlimited" only
holds on git >= 2.29; on older git it could disable detection outright).
* fix(review): stop scroll-driven sem stampede when semantic diff is failing
The badge retry change (
|
||
|
Michael Ramos
|
0be4295a2e |
Plan-look chooser + 0.20.0 release dialog (with GitLab / sem / install fixes) (#879)
* feat(plan): default to the grid look + look-and-feel image chooser - Flip gridEnabled default back to true (classic grid / floating-card look), reverting #863's flat-by-default so existing users without an explicit choice return to the grid view they're used to. - Rewrite the look-and-feel announcement into a two-image chooser: Grid (classic, now the default) vs Clean (the new flat look), each a clickable screenshot that hover-zooms to preview — mirroring the code-review DiffTypeSetupDialog pattern. - Bump the announcement version (1 -> 2) so existing users are re-offered the choice once. - Add look-grid.png / look-flat.png preview assets (resized app screenshots). * fix(review): restore GitLab /diffs fallback for MR review #871's pr-gitlab refactor moved to the raw_diffs endpoint and deleted the paginated /diffs path, hard-breaking MR review on self-hosted GitLab too old to expose raw_diffs (opaque "Failed to fetch MR diff") and silently rendering zero files when raw_diffs returns empty for oversized MRs. Keep raw_diffs as the primary path (it preserves binary markers + collapsed content), but fall back to the restored paginated /diffs + reconstructPatch when raw_diffs fails or is empty, and throw a clear "diff is empty / too large" error if both come back empty. Restores parsePaginatedArray / reconstructPatch and adds tests for the fallback paths. * fix(install): bound the optional sem sidecar download The semantic-diff 'sem' download (#871) ran curl / Invoke-WebRequest with no timeout. The sidecar installs after plannotator itself, so a slow or hung fetch could wedge an otherwise-complete install. Add --connect-timeout 10 --max-time 120/60 (sh, cmd) and -TimeoutSec 120/60 (ps1) so it times out and skips gracefully, and document the PLANNOTATOR_SKIP_SEM_INSTALL opt-out in the installer help. * chore(install): remove the Glimpse install option from the installers The installers offered to `npm install -g glimpseui` (a wizard prompt + --glimpse/--no-glimpse flags + saved pref). Drop all of it across install.sh, install.ps1, and install.cmd: flags, usage text, the wizard question, the glimpse_present detection, the saved-pref read/write, the persistence clauses, and the install block. The guided wizard is now two questions (extras, model-invocable). Runtime Glimpse support is unchanged: the app still opens in the native window if glimpseui is on PATH (packages/server/browser.ts), still gated by PLANNOTATOR_GLIMPSE. Updated install.test.ts (asserts the installers are glimpse-free) and the installation docs. * feat(plan): 0.20.0 release announcement dialog Turn the first-run look chooser into a two-page 0.20.0 announcement. - Page 1: header with a "Full release notes" link, a four-up feature grid (fresh look, semantic review, multi-repo review, leaner install), and the grid/clean plan-look chooser. - Page 2: "Workspaces are coming" teaser with the waitlist image + link. - Footer actions grouped bottom-right on both pages (shimmering "Workspaces are coming" teaser via TextShimmer + the primary action) so focus stays in one corner across the page turn. - Reshoot the grid/clean chooser screenshots on a clean prose plan and add the workspaces teaser image. * feat(plan): add full-page HTML feature card, lead with leaner install Add a fifth what's-new card for --render-html (annotate HTML reports and explainers rendered full-screen) and move Leaner install to the front of the grid. |
||
|
Michael Ramos
|
c3ba55e889 |
feat(review): add semantic diff overview (#871)
* feat(review): add semantic diff overview * fix(review): address semantic diff review findings * fix(review): harden semantic diff fallback * fix(review): avoid repo-local sem execution * fix(review): normalize semantic diff cwd * test(review): cover semantic diff local cwd * fix(gitlab): fetch raw MR diffs * fix(review): harden sem path resolution * docs(review): add semantic diff handoff * feat(review): semantic diff cards, header sem badges, jump-to-line - Restyle the semantic panel as real bordered cards (shadcn surface) instead of ASCII box-drawing; centered column, grid-aligned entity rows. - Hide orphan (module-level) changes from the rows, matching sem's default; the count still surfaces in the summary line. - Add a 'sem · N' hover popover to each diff file header showing that file's semantic changes, reusing the same row component as the panel. - Clicking an entity (panel or popover) scrolls the diff to the lines via pierre's [data-selected-line]; centers only when off-screen so manual drag-selection isn't disturbed. - Extract shared rows/helpers into semanticDiffShared; share one cached /api/semantic-diff fetch across header badges. * feat(review): land on All files by default Semantic diff stays available via the file-tree nav entry and header badges, but it's no longer the initial landing view. * chore(review): drop dead change-symbol entries, log badge fetch failures - Remove unreachable 'moved'/'renamed' entries from the changeSymbols table (getChangeSymbol early-returns for those before the lookup). - Log a console.error once per patch when the file-header badge's semantic diff fetch fails or returns a non-ok status, so a systemic failure leaves a trace instead of every badge silently showing nothing. * feat(review): flatten semantic diff panel into grouped list - Dissolve the per-file bordered cards into flat sections: the file path is now a quiet underlined header (single hairline) with entity rows flush beneath, hierarchy from whitespace + hover tint instead of boxes. - Split the path into muted directory + emphasized filename for faster scanning. - Nudge the add/remove glyphs (⊕/⊖) ~15% larger, line-height pinned so rows don't grow; modified/rename/reorder glyphs unchanged. |
||
|
Hrand Liu
|
e0aee7451b |
feat: add PLANNOTATOR_DATA_DIR env var to customize data directory (#795)
* feat: add PLANNOTATOR_DATA_DIR env var to customize data directory * fix: update missed hardcoded paths to use PLANNOTATOR_DATA_DIR OpenCode plugin and VS Code extension still used hardcoded ~/.plannotator paths, causing the IPC registry and plan backing file to diverge from the server when PLANNOTATOR_DATA_DIR is set. Also exports data-dir from @plannotator/shared and documents the new env var in AGENTS.md. Co-authored-by: Chris Werner Rau <14326070+cwrau@users.noreply.github.com> Co-authored-by: João O. Santos <34689526+Joao-O-Santos@users.noreply.github.com> * fix: vendor data-dir.ts into Pi extension and rewrite imports The Pi extension copies shared/server modules into generated/ at build time. Without vendoring data-dir.ts and rewriting the parent-relative imports, typecheck fails on all generated files that import getPlannotatorDataDir. * refactor: eliminate duplicated data-dir logic and clean up call sites - VS Code extension: replace inlined getPlannotatorDataDir() copy with import from the canonical packages/shared/data-dir.ts (esbuild bundles it, so no runtime dependency needed) - storage.ts: hoist repeated getPlannotatorDataDir() calls to a module-level DATA_DIR constant, matching the pattern config.ts uses - data-dir.ts: remove inaccurate docstring claim about relative path resolution (the code does not call resolve()) - improvement-hooks.ts: hoist to DATA_DIR constant, clarify comments on the two-level hook lookup (hooks/ subdir vs root fallback) * fix: resolve relative PLANNOTATOR_DATA_DIR to absolute path A relative value like ./data would break readArchivedPlan's path traversal guard, which compares a resolve()'d absolute path against the still-relative planDir prefix. Always return an absolute path so all callers get consistent path shapes. * fix: use @plannotator/shared/data-dir imports in server package Switch from relative ../shared/data-dir imports to the package export, matching the convention every other server file follows. Update Pi vendor script sed rules to match the new import style. * fix: use package imports in server and respect data dir in compound skill Server modules: switch from relative ../shared/data-dir imports to @plannotator/shared/data-dir, matching the convention every other server file follows. Update Pi vendor script sed rules to match. Compound skill: update hardcoded ~/.plannotator paths to check PLANNOTATOR_DATA_DIR first, so the skill reads plans and writes the improvement hook to the correct location when users set a custom data directory. Co-authored-by: Chris Werner Rau <14326070+cwrau@users.noreply.github.com> Co-authored-by: João O. Santos <34689526+Joao-O-Santos@users.noreply.github.com> * fix: remove remaining hardcoded ~/.plannotator assumptions - Settings UI: replace hardcoded path in label and placeholder with generic text that doesn't assume a specific data directory - quickLabels: update agent tip to reference PLANNOTATOR_DATA_DIR so the agent checks the correct plans directory - codex-review: hoist getPlannotatorDataDir() to module-level DATA_DIR constant, eliminating redundant per-call resolution in debugLog() - Tests: make submit-plan and storage tests resilient to PLANNOTATOR_DATA_DIR being set in the environment - Install scripts (sh, ps1, cmd): check PLANNOTATOR_DATA_DIR before falling back to ~/.plannotator for config.json attestation lookup * fix: expand tilde in install script and update test assertions install.sh: PLANNOTATOR_DATA_DIR set to ~/... stays literal inside double quotes, so the config file check silently failed. Add case statement to expand ~ the same way the runtime data-dir.ts does. install.test.ts: update three assertions that checked for hardcoded ~/.plannotator paths — now verify PLANNOTATOR_DATA_DIR awareness instead. * docs: add PLANNOTATOR_DATA_DIR to env var reference with VS Code note Document the new env var on the marketing site's environment variables reference page. Include a footnote about ensuring VS Code inherits the variable when launched from the Dock. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> Co-authored-by: Chris Werner Rau <14326070+cwrau@users.noreply.github.com> Co-authored-by: João O. Santos <34689526+Joao-O-Santos@users.noreply.github.com> |
||
|
Michael Ramos
|
927ca6d27e |
fix(gitlab): handle concatenated JSON pages from glab --paginate (#717)
glab api --paginate joins page responses as adjacent JSON arrays (`[...][...]`), which JSON.parse rejects. This caused MR reviews to fail with "JSON Parse error" for any MR with more than 100 changed files. Replace the bare JSON.parse with a walker that splits the output into top-level arrays (string- and depth-aware so `][` inside diff content is not mistaken for a page boundary) and merges them. Fixes #714 |
||
|
Michael Ramos
|
3fb0b9cf03 |
fix(gitlab): persist unposted inline comments + split pr-provider from browser-safe pr-types (#719)
Closes #680. Two changes that landed together because the persistence fix exposed a hidden architectural constraint. 1. GitLab inline comments: when one or more discussion POSTs failed (e.g. transient `i/o timeout`), the failed comment bodies were lost. Now `submitGlMRReview` writes them to `~/.plannotator/failed-comments/{host}-{project}-mr{iid}-{ts}.json` in both the all-fail and partial-fail branches. The throw-vs-warn split is preserved deliberately: all-fail throws so the UI retries from a clean state, partial-fail warns so the UI doesn't resubmit already-posted content. 2. Split `packages/shared/pr-provider.ts` into `pr-types.ts` (browser-safe types + pure label/URL helpers) and `pr-provider.ts` (server-only dispatch that imports pr-github / pr-gitlab). The review-editor browser bundle previously dragged pr-gitlab.ts in as dead code via static imports, which silently constrained the file to never use Node built-ins. Adding `fs`/`os`/`path` for (1) broke the review build until we routed browser imports to pr-types and left server callers on the now server-only pr-provider facade. Server-only `pr-provider.ts` re-exports `pr-types` so existing server-side imports keep working unchanged. |
||
|
Michael Ramos
|
bb404f8d14 |
feat: stacked PR review — PR switching, scope toggling, multi-PR posting (#620)
* feat(shared): add isSameProject, PR stack types, and PR list provider Extends PRRef/PRMetadata with defaultBranch, PRStackInfo, PRStackTree, PRStackNode, PRDiffScope, and PRListItem types. Adds isSameProject() for owner/repo validation on PR switching. Adds fetchPRStack() and fetchPRList() dispatch functions (GitHub-only for now, GitLab stubs). Includes 9 new tests for isSameProject covering GitHub, GitLab, and cross-platform scenarios. For provenance purposes, this commit was AI assisted. * feat(shared): add GitHub PR stack tree walking and PR list fetching Implements fetchGhPRStack() which walks up/down the PR stack via GraphQL, resolving numbers and titles for each node in the chain. Collapses queryPRsByHead/queryPRsByBase into a single queryPRsByRef helper. Adds fetchGhPRList() using gh pr list. Fixes GHE support by removing hostnameArgs from fetchGhPRList (--repo already handles GHE). Filters jq "null" string from defaultBranch detection. For provenance purposes, this commit was AI assisted. * feat(shared): fetch defaultBranch for GitLab MRs Queries the project's default_branch via glab API so getPRStackInfo can detect stacked MRs on GitLab. Best-effort — caught errors fall back to undefined. For provenance purposes, this commit was AI assisted. * feat(shared): add PR stack detection and full-stack diff module New pr-stack module with: - getPRStackInfo(): detects stacked PRs from baseBranch vs defaultBranch - getPRDiffScopeOptions(): generates layer/full-stack scope options - runPRFullStackDiff(): computes diff from default branch to HEAD - resolvePRFullStackBaseRef(): resolves origin/main or local main - checkoutPRHead(): fetches and checks out a PR head in a worktree - buildMinimalStackTree(): builds UI tree from stack info Includes 13 tests covering ref resolution, branch fallbacks, and GitLab ref formats. For provenance purposes, this commit was AI assisted. * feat(shared): add worktree pool for per-PR agent isolation Creates a session-scoped pool of git worktrees — each PR visited during a stacked review gets its own isolated checkout. Agents run in their PR's worktree undisturbed by PR switches. Handles deduplication of concurrent ensure() calls for the same PR. Includes 11 tests covering caching, cross-repo restrictions, GitLab ref formats, and cleanup. For provenance purposes, this commit was AI assisted. * feat(shared): add diffScope/prUrl to agent jobs, branch diff type Adds prUrl and diffScope optional fields to AgentJobInfo so agent findings carry the PR and scope context they were launched under. Exports new pr-stack and worktree-pool modules from package.json. Adds 'branch' to DefaultDiffType union for branch diff as default. For provenance purposes, this commit was AI assisted. * feat(ui): add PR annotation fields, Popover, and SearchableSelect Extends CodeAnnotation with prUrl, prNumber, prTitle, prRepo, and diffScope fields for stacked PR attribution. Adds shared Popover wrapper around radix-ui. Adds SearchableSelect for filterable dropdown lists (used by PR selector). For provenance purposes, this commit was AI assisted. * feat(ui): add branch diff as default option, new Git settings tab Adds 'Branch' as a fourth default diff type option in both the first-run dialog and settings panel. Moves the default diff type setting from the Display tab to a new Git tab in review mode. Updates config store validators to accept 'branch'. For provenance purposes, this commit was AI assisted. * feat(server): add prUrl/diffScope plumbing to agent jobs and prompts Threads prUrl and diffScope through the agent job lifecycle so findings carry the PR and scope they were generated under. Adds full-stack prompt branch to codex-review and tour-review — when in full-stack mode, the diff is inlined in the prompt instead of telling the agent to run git diff. Re-exports isSameProject and new PR provider functions from server/pr.ts. For provenance purposes, this commit was AI assisted. * feat(server): add stacked PR support to Bun review server Adds PR switching, layer/full-stack scope toggling, PR list caching, worktree pool integration, and multi-PR platform posting to the Bun review server. Key additions: - /api/pr-diff-scope: switch between layer and full-stack diffs - /api/pr-list: cached PR list for the current repo - /api/pr-switch: in-place navigation between PRs in a stack - /api/pr-action: targetPrUrl support for multi-PR posting - /api/file-content: full-stack branch for hunk expansion - prSwitchCache/prStackTreeCache for session-scoped caching - diffScope tagging on agent job completion - Scope guard: returns 400 on full-stack diff failure instead of overwriting the working diff with empty content For provenance purposes, this commit was AI assisted. * feat(ai): pass cwd to Claude agent SDK for worktree support Forwards the working directory to the Claude agent provider so agents run in the correct worktree when reviewing stacked PRs. For provenance purposes, this commit was AI assisted. * feat(pi): add stacked PR support to Pi server (Bun parity) Mirrors all stacked PR features from the Bun server: - PR switching, scope toggling, PR list, multi-PR posting - prSwitchCache/prStackTreeCache with initial PR seeding - diffScope/prUrl plumbing in agent jobs - Worktree pool creation and lifecycle - Full-stack file-content resolution matching Bun's guard structure - targetPrUrl support on /api/pr-action Hoists worktreePool declaration to outer scope in plannotator-browser to fix TS18004 scoping error. Updates vendor.sh for new shared modules. For provenance purposes, this commit was AI assisted. * feat(hook): create worktree pool for PR review sessions Creates a worktree pool when opening a PR review with --local, seeding it with the initial PR's checkout. Integrates pool cleanup into server shutdown. Passes the pool to startReviewServer for agent isolation during PR switching. For provenance purposes, this commit was AI assisted. * feat(review-editor): add hooks for PR stack, context, and annotations - useAnnotationFactory: stamps prUrl/prNumber/prTitle/prRepo/diffScope onto annotations, only when viewing a stacked PR - usePRStack: handles scope selection and PR switching with loading state - usePRContext: adds URL-change detection to prevent stale-fetch race when switching PRs (discards in-flight responses for previous PR) For provenance purposes, this commit was AI assisted. * feat(review-editor): add stacked PR UI components - PRSelector: searchable dropdown for switching between PRs in a repo - PRSwitchOverlay: loading animation during PR switch - StackedPRLabel: stack tree popover with scope selector and PR navigation - ReviewSubmissionDialog: multi-PR submission dialog with per-target status, orphaned findings section with copy-as-markdown, and partial failure retry For provenance purposes, this commit was AI assisted. * feat(review-editor): multi-PR export with heading hierarchy Updates exportReviewFeedback for multi-PR sessions: - Groups annotations by prUrl, then by file within each PR - Uses proper heading hierarchy (## for files, ### for annotations in multi-PR mode) - Detects single-PR mismatch (annotations from a different PR than the current view) and uses annotation-level PR context - Adds diffScope labels per PR group when present Includes 5 new tests: multi-PR headings, single-PR mismatch, diffScope labels, and non-stacked annotation handling. For provenance purposes, this commit was AI assisted. * feat(review-editor): integrate stacked PR into sidebar, diff panel, and agents - ReviewSidebar: groups annotations by PR in multi-PR sessions, shows PR headers with annotation counts - ReviewDiffPanel: filters annotations by prUrl and diffScope so only matching annotations appear in the diff gutter - ReviewStateContext: adds prDiffScope to shared review state - ReviewAgentJobDetailPanel: shows diffScope in job detail - PRSummaryTab: shows stack info in PR summary - index.css: PR switch shimmer and overlay animations For provenance purposes, this commit was AI assisted. * feat(review-editor): wire stacked PR into main review app Integrates all stacked PR features into the review editor: - PR stack state management (prStackInfo, prStackTree, prDiffScope) - applyPRResponse: shared handler for PR switch and scope toggle, preserves active file index on scope changes - Multi-PR platform posting via Promise.allSettled with parallel requests, partial failure retry, and per-target status tracking - ReviewSubmissionDialog replaces inline dialog JSX - useAnnotationFactory stamps PR context onto annotations - keepalive on /api/feedback to survive tab closure - Proper try/catch/finally on handlePlatformAction For provenance purposes, this commit was AI assisted. * docs: add stacked PR review documentation Updates AGENTS.md, code-review command docs, and AI code review guide with stacked PR review capabilities. For provenance purposes, this commit was AI assisted. * fix(server): stamp prNumber/prTitle/prRepo on agent findings Agent annotations only had prUrl and diffScope, missing prNumber, prTitle, and prRepo. When agent findings were the only annotations for a PR target in the submission dialog, the target rendered as #0 with no title. Now resolves full PR context from prSwitchCache at job completion and stamps all five fields. Both Bun and Pi. For provenance purposes, this commit was AI assisted. * feat(ui): rename diff options — "Committed" replaces "Branch" / "Current PR Diff" Consolidates two confusing committed-diff options into one: - Settings/first-run: "Committed" — "Everything you've committed on this branch" - Mid-session switcher: "Committed changes" (replaces both "vs main" and "Current PR Diff") Uses merge-base under the hood (matches GitHub PR behavior). Removes the two-dot branch diff from the UI — it stays in the runtime DiffType union for backwards compat. Old "branch" values in config/cookies are silently upgraded to merge-base. Git settings tab now uses radio cards with descriptions instead of a cramped segmented control. First-run dialog descriptions rewritten in plain language — no git commands. For provenance purposes, this commit was AI assisted. * fix(review-editor): rename client-side "PR Diff" labels to "Committed changes" DiffTypePicker.tsx had a hardcoded "PR Diff" override for merge-base when the base picker is present. exportFeedback.ts also used "PR Diff" in export labels. Both now say "Committed changes" to match the server-side label and settings UI. For provenance purposes, this commit was AI assisted. * fix(server): discover stack UI for root PRs targeting the default branch Root PRs (baseBranch === defaultBranch) were excluded from stack detection because getPRStackInfo returned null. Now the server always fetches the stack tree in PR mode. If the tree reveals descendant PRs, prStackInfo is retroactively set with source "tree-discovered", enabling the stack UI, scope selector, and PR navigation from the root of a stack. Both Bun and Pi servers updated. Adds "tree-discovered" to the PRStackInfo source union. For provenance purposes, this commit was AI assisted. * fix(review-editor): derive diff scope from annotations, not UI state The export function now reads diffScope from annotations instead of the prReviewScope parameter. Fixes two issues: 1. Agent job "Copy All" showed the wrong scope when the user switched between layer/full-stack after launching the agent 2. Mixed-scope annotations produced a confusing "layer, full-stack" comma-joined label instead of grouping by scope Extracts renderScopedGroups helper for scope-aware grouping — used by both single-PR and multi-PR export paths. When annotations share one scope, it appears in the header. When mixed, annotations are grouped under ## Layer / ## Full-stack headings. Includes 4 new tests: uniform scope derivation, mixed scope grouping, single scope header, and prReviewScope override prevention. For provenance purposes, this commit was AI assisted. * fix(server): add tree-discovered stack fallback to pr-switch handler The initial-load path upgrades prStackInfo for root PRs when the stack tree reveals descendants, but the pr-switch handler was missing this logic. The server now sends correct prStackInfo after switching to a root-of-stack PR. Both Bun and Pi. Also removes stale prReviewScope dependency from agent job panel's copyAllText useMemo. For provenance purposes, this commit was AI assisted. * fix: extract resolveStackInfo helper, fix stack UI on non-stacked PRs Extracts the tree-discovered stack fallback into resolveStackInfo() in pr-stack.ts — eliminates 4 copies of the same logic across Bun startup, Bun pr-switch, Pi startup, and Pi pr-switch. Fixes StackedPRLabel showing on every PR: the check now counts non-default-branch nodes (> 1) instead of all nodes (> 1). Without this, every PR showed a "Stack (1 PR)" popover because the tree always has at least [defaultBranch, currentPR]. For provenance purposes, this commit was AI assisted. * fix(review-editor): don't re-open already-succeeded PR tabs on retry On partial failure retry, openUrls was pre-seeded with URLs from previously succeeded targets, causing those PR pages to re-open in the browser alongside newly succeeded ones. Now starts empty — only URLs from the current attempt are opened. For provenance purposes, this commit was AI assisted. * refactor(review-editor): extract PR session state into usePRSession hook Consolidates 5 independent useState calls (prMetadata, prStackInfo, prStackTree, prDiffScope, prDiffScopeOptions) into a single usePRSession hook with atomic updatePRSession callback. Replaces two identical 5-line setter blocks (initial load and applyPRResponse) with single updatePRSession calls. All ~60 consumer sites unchanged — same variable names via destructuring. For provenance purposes, this commit was AI assisted. |
||
|
Michael Ramos
|
b375a804b2 |
feat(review): AI review agents, local worktree, and UI polish (#491)
* feat(review): add Codex AI review agent with live logs, --local worktree, and panel UI
Hook up Codex as the first AI review agent in the code review system:
- Spawn `codex exec` with Codex's native review prompt and output schema
- Parse structured findings (ReviewOutputEvent) and push as external annotations
- Annotations appear inline in the diff viewer pinned to specific lines
- Review verdict (correct/incorrect + confidence + explanation) displayed in panel
Agent job infrastructure enhancements:
- Server-side command building via `buildCommand` callback (providers don't need frontend commands)
- Result ingestion via `onJobComplete` callback (reads output file, transforms findings)
- `addAnnotations` method on external annotation handler (bypasses HTTP for server-internal producers)
- Live stderr streaming via `job:log` SSE events with 200ms buffer-and-flush
- `cwd` and `summary` fields on AgentJobInfo
PR review with --local worktree:
- `plannotator review <PR_URL> --local` creates a temp git worktree with the PR branch
- Agent gets full local file access without touching the user's working tree
- Hybrid server mode: both prMetadata (platform features) and gitContext (local access)
- Automatic worktree cleanup on session end
- Runtime-agnostic worktree primitives in packages/shared/worktree.ts
Panel UI redesign:
- Findings | Logs tab system with underline-style tabs
- LiveLogViewer component with auto-scroll, truncation, and copy
- Review verdict card (correct/incorrect with confidence and explanation)
- Pending state with labeled "Review Verdict — Pending..." (not skeleton bars)
- Job card click opens detail panel directly (removed separate icon button)
- Dismissed annotation tracking (deleted annotations persist as "dismissed" in panel)
- Copy all annotations as formatted markdown
- Worktree badge in header with info dialog showing path
- CopyButton extended with inline variant for reuse
- ConfirmDialog extended with wide option
For provenance purposes, this commit was AI assisted.
* style(review): UX polish pass — visual quality improvements across code review UI
- VerdictCard: remove AI-template left-border, use background-only tint
- Inline annotations: 6px radius, subtle shadow, hover elevation, action button scale
- File tree: tighter indentation (4 + depth*10), reduced container padding
- Select dropdowns: normalized to 4px border-radius matching pierre diffs
- Dockview tabs: close button pushed to far right with margin-left auto, visible at 0.25 opacity
- PR icons moved from sidebar to header (next to PR link)
- AnnotationRow: translate-x hover feedback
- Border-radius normalized to `rounded` (4px) across all components
- Type scale consolidated: text-[8px]/[9px] → text-[10px], text-[11px] → text-xs
- Sidebar tab hit targets increased (px-2.5 py-1.5, w-4 icons)
- Sticky file group headers in annotation sidebar
- FileTree controls collapsed (worktree + diff selectors share one row)
- Tab micro-animations (transition-all duration-150)
- ScrollFade component for gradient indicators on scrollable containers
- Prose containment: max-w-2xl + px-6 padding on PR Summary/Comments/Checks panels
- MarkdownBody: leading-relaxed for comfortable reading
- PR Comments: surface lift (bg-muted/10), hover feedback, author font-semibold
- PR Checks: link affordance (text-primary, underline on hover, external link icon)
For provenance purposes, this commit was AI assisted.
* feat(review): PR comments panel — search, filter, collapse, navigation, and polish
Comments panel enhancements:
- Search: real-time text filter across author and body, match count display
- Keyboard navigation: j/k to move between comments, scroll-to-selected
- Sort: toggle between oldest/newest first
- Collapsible comments: click header to collapse, collapse/expand all controls
- Author exclusion filter: click authors to hide their comments (not inclusion)
- Comment actions: hover-reveal "View on GitHub" link + copy button (bottom-right)
- Review URLs: PRReview type now includes optional url field, populated from GitHub API
PR Summary fixes:
- Label contrast: use theme foreground color for label text instead of raw GitHub hex
- Linked issues: replaced broken hardcoded SVG with proper GitHub Octicons issue-opened icon
Data plumbing:
- platformUser exposed through ReviewStateContext for "Mine" filtering
- Panel wrapper changed to overflow-hidden for sticky toolbar support
- "Commented" review badge hidden (noise — only show Approved/Changes Requested/Dismissed)
For provenance purposes, this commit was AI assisted.
* feat(review): inline review threads with outdated/resolved state and diff hunk previews
PR Review Threads:
- Fetch inline code review comments via GitHub GraphQL (reviewThreads query)
- PRReviewThread and PRThreadComment types with isResolved, isOutdated, path, line, diffSide
- ThreadCard component: file/line context, Outdated/Resolved badges, nested replies
- Resolved/outdated threads: gradient fade on body with "Show full comment" expand
- GitLab: reviewThreads placeholder (TODO: parse DiffNote positions from notes)
DiffHunkPreview:
- Renders diff hunks using @pierre/diffs FileDiff component (read-only, compact)
- Full theme integration: reads computed CSS vars, injects via unsafeCSS (same as main DiffViewer)
- Respects user font settings from ReviewState context
- Handles bare GitHub diffHunk format (prepends synthetic file headers for pierre parsing)
Comments Panel Polish:
- Comment cards: bg-card + subtle shadow for depth and isolation from panel background
- Hover: shadow elevation (0_2px_6px) for interactive feedback
- Thread cards: dimmed shadow for resolved/outdated, full shadow for active
- Prose padding: px-8 (32px) across all dockview panels (Summary, Comments, Checks, Findings)
For provenance purposes, this commit was AI assisted.
* feat(review): Claude Code agent, cross-repo --local, render fixes
Claude Code review agent:
- claude-review.ts: prompt (adapted from code-review plugin), command builder
(dontAsk + granular allowedTools/disallowedTools), JSONL stream output parser
- Prompt sent via stdin (not argv) to avoid quoting/variadic flag conflicts
- stream-json --verbose for live JSONL streaming + final structured_output
- Same schema as Codex — transformReviewFindings is now provider-agnostic
Agent jobs infrastructure:
- stdout capture (captureStdout option) for providers that return results on stdout
- stdin prompt writing (stdinPrompt option) for providers that read prompt from stdin
- cwd override in buildCommand return for providers without -C flag
- await stdoutDone before onJobComplete to prevent drain race condition
- job.prompt field for transparent prompt display in detail panel
Cross-repo --local:
- Detect same-repo vs cross-repo via parseRemoteUrl comparison
- Cross-repo: shallow clone via gh/glab repo clone (--depth 1 --no-checkout + targeted fetch)
- Cross-repo uses platform diff (gh pr diff) for display, clone for agent file access
- Same-repo: existing worktree path unchanged
- Cleanup: rmSync for clones, worktree remove for same-repo
Performance: jobLogs context split
- Separate JobLogsContext to prevent high-frequency log SSE from re-rendering all panels
- Only ReviewAgentJobDetailPanel subscribes to JobLogsProvider
- Standard React pattern: split contexts by update frequency
Image error handling:
- SafeHtmlBlock component wraps dangerouslySetInnerHTML with img onerror handlers
- Broken images (expired GitHub JWTs) hide on first 404 instead of flickering
- Prevents console 404 flood from re-render retry loops
For provenance purposes, this commit was AI assisted.
* fix(review): decontaminate --local from diff pipeline, fix worktree setup, default local for PRs
The --local flag was setting gitContext from the worktree, which contaminated
the diff rendering pipeline — causing pierre "trailing context mismatch" errors
because /api/file-content read worktree files instead of using the GitHub API.
Root cause: gitContext serves two purposes — diff pipeline (file contents, diff
switching, staging) and agent sandbox (cwd for agent processes). These are now
properly separated via a new agentCwd option on ReviewServerOptions.
Changes:
- Add agentCwd to ReviewServerOptions, independent of gitContext
- Agent handler (getCwd, buildCommand, onJobComplete) prefers agentCwd
- Stop setting gitContext in --local PR path — diff pipeline untouched
- Revert band-aid !isPRMode guards (no longer needed)
- Fix same-repo worktree: fetch origin/<baseBranch> so agents see correct diff
- Fix cross-repo clone: create local branch at baseSha for git diff accuracy
- Fix FETCH_HEAD ordering: fetch base branch before PR head (createWorktree needs PR tip)
- Fix macOS path mismatch: realpathSync(tmpdir()) so agent paths strip correctly
- Change buildCodexReviewUserMessage signature from GitContext to focused options
- Make --local the default for PR/MR reviews (--no-local to opt out)
- Pass agentCwd to client for worktree badge display
For provenance purposes, this commit was AI assisted.
* style(review): unify panel headers, responsive buttons, dockview polish
- Unify all panel headers at 33px via --panel-header-h CSS variable
- FileHeader now uses shared variable instead of hardcoded 30px
- Dockview tab bar height, font size, and padding aligned with sidebars
- FileTree header uses fixed height instead of padding-based sizing
- Consistent border opacity (border-border/50) across all panels
- Consistent font weight (font-semibold) on all header labels
- Remove dockview tab focus outline (::after pseudo-element)
- Dockview tab close button pushed to right edge
- Tab bar void area uses muted background
- Top app header compacted from h-12 to py-1
- Sidebar footer: copy button and diff stats side by side
- FeedbackButton responsive labels (Send/Post at md, full labels at lg)
- Move ReviewAgentsIcon to packages/ui for shared use
- Agent empty state uses shared ReviewAgentsIcon instead of hardcoded SVG
- Sidebar header label truncates when narrow (tabs never clip)
- Detail panel prompt disclosures get proper spacing
- React.memo on PR tab components (PRSummaryTab, PRCommentsTab)
- Inline onerror on img tags for broken GitHub image handling
For provenance purposes, this commit was AI assisted.
* fix: type assertion for Bun stdin FileSink
Bun's proc.stdin is typed as `number | FileSink` but we need to call
.write() and .end() on it. Cast to FileSink to satisfy tsc --noEmit.
For provenance purposes, this commit was AI assisted.
* fix(review): XSS in sanitizeHtml, git flag injection, cross-repo ref mismatch
Security:
- Remove `onerror` from DOMPurify ALLOWED_ATTR — was allowing arbitrary JS
execution via PR descriptions containing `<img onerror="...">`. Replace
with SafeHtml component that attaches error handlers via useEffect + ref.
- Add `--` end-of-options separator to git fetch and git branch calls to
prevent flag injection via crafted branch names from API responses.
Bug fix:
- Cross-repo clones now create both local branch AND remote-tracking ref
(`refs/remotes/origin/<baseBranch>`) at baseSha, so agents can use either
`git diff main...HEAD` or `git diff origin/main...HEAD`.
Polish:
- Add copy button to verdict card in agent detail panel
- Remove hover:translate-x animation from finding rows
- Reduce file tree indent per level, remove extra file offset
- Add pr-action endpoint logging for debugging submit failures
For provenance purposes, this commit was AI assisted.
* chore: upgrade @pierre/diffs from 1.1.0-beta.19 to ^1.1.12
The beta pin was needed for processFile() API (expandable diff context),
which shipped in 1.1.0 stable on March 14. We were 13 releases behind.
Notable fixes in the upgrade path:
- 1.1.5: Fix diffAcceptRejectHunk with partial FileDiffMetadata
- 1.1.6: Patch parsing fix for renames and dotfiles
- 1.1.8: Fix maxLineDiffLength regression
May resolve intermittent "trailing context mismatch" errors in diff rendering.
For provenance purposes, this commit was AI assisted.
* fix(review): remove shell provider, flag injection, Claude log formatting, copy UX
Security:
- Remove shell provider from agent capabilities (unauthenticated RCE vector)
- Move `--` before prRepo in `gh repo clone` to prevent flag injection
Features:
- Wire up formatClaudeLogEvent — Claude live logs now show readable text
instead of raw JSONL
- Sidebar annotations: copy + delete buttons appear on hover (no overlap)
- Agent finding rows: copy button on hover (progressive disclosure)
- Verdict card: copy button pushed to the right
- File tree: reduced indent per level, files aligned with folders
Infra:
- PR action endpoint logging for debugging submit failures
For provenance purposes, this commit was AI assisted.
* fix: validate repo identifier to prevent flag injection in gh repo clone
The `--` separator in `gh repo clone` separates gh args from git args,
not positional args from flags. Using `--` before prRepo would break
the git flags. Instead, validate that the repo identifier doesn't start
with `-` to prevent flag injection via crafted PR URLs.
For provenance purposes, this commit was AI assisted.
* feat(review): Claude-specific review model with severity, reasoning, and multi-agent prompt
Claude review agent now has its own schema, prompt, and transform — separate
from Codex's P0-P3 priority model. Each provider uses its natural review style.
Schema changes:
- Claude findings use severity (important/nit/pre_existing) instead of priority (0-3)
- Flat structure: file, line, end_line instead of nested code_location
- description (single field) instead of title + body
- reasoning field captures the validation chain per finding
- summary with counts instead of overall_correctness/confidence
Prompt: Converges the open-source Claude Code review prompt with the remote
review service model. 4 parallel agents (Bug+Regression at Opus, Security at
Opus, Code Quality at Sonnet, Guideline Compliance at Haiku), validation step,
deduplication, severity classification. CLAUDE.md and REVIEW.md awareness.
UI: Severity markers (colored dots) on finding rows. Collapsible reasoning
section via <details>. New optional severity/reasoning fields on CodeAnnotation
and the external annotation store — backward compatible, only set by Claude.
Transform: transformClaudeFindings normalizes Claude output into the shared
annotation format. Codex path (transformReviewFindings) is completely untouched.
For provenance purposes, this commit was AI assisted.
* fix: use bg-amber-500 for nit severity dot (bg-warning may not be defined)
For provenance purposes, this commit was AI assisted.
* fix: security hardening, debug cleanup, deduplication, Pi mirroring, findings UX
Security:
- Add -- separator to ensureObjectAvailable git fetch (worktree.ts)
- Validate baseBranch against path traversal (..) before git ref operations
- Use process.once('exit') instead of process.on for worktree cleanup
Debug:
- Gate debugLog behind PLANNOTATOR_DEBUG env var (no more unconditional writes)
- Remove PARSE_OUTPUT_RAW dump that logged full JSON output to disk
Deduplication:
- Extract toRelativePath to packages/server/path-utils.ts (was duplicated
in codex-review.ts and claude-review.ts)
Pi extension:
- Remove shell provider from capabilities
- Add buildCommand callback to AgentJobHandlerOptions
- POST handler calls buildCommand for server-side command synthesis
UI:
- Findings sorted by severity (important → nit → pre_existing)
- Severity legend under findings header (colored dots)
- Reasoning always visible (not collapsible) — fixes click navigation bug
where <details> captured click events and broke annotation linkage
- Full finding text shown (removed line-clamp-2 truncation)
- Sidebar annotation hover actions aligned to the right
- DiffHunkPreview: cancel requestAnimationFrame on unmount
For provenance purposes, this commit was AI assisted.
* fix: move toRelativePath import to top of claude-review.ts
For provenance purposes, this commit was AI assisted.
* fix: same-repo detection compares host, platform-aware comment links, remove design docs
- Same-repo detection now compares both owner/repo AND hostname from the
git remote URL against prMetadata.host. Prevents false positives on
GitHub Enterprise where different instances share org/repo names.
- "View on GitHub" label in PR comments tab now shows "View on GitLab"
for GitLab MR comments based on the comment URL.
- Remove internal design docs (PR_LOCAL_WORKTREE.md, AGENT_LIVE_LOGS.md)
that were development artifacts, not user-facing documentation.
For provenance purposes, this commit was AI assisted.
* fix(review): show severity markers and reasoning in inline diff annotations
The severity and reasoning fields from Claude findings were only visible in
the agent detail panel, not in the inline diff annotations. Now:
- DiffAnnotationMetadata carries severity and reasoning fields
- DiffViewer passes them through when mapping annotations
- InlineAnnotation renders colored severity dot and reasoning text
For provenance purposes, this commit was AI assisted.
* fix: prefix Claude findings text with [severity] tag
Findings now show as "[important] description", "[nit] description",
"[pre_existing] description" — consistent with Codex's [P0]/[P1] tags.
For provenance purposes, this commit was AI assisted.
* feat(pi): full agent review mirroring — stdin, stdout, live logs, result ingestion
Pi extension agent-jobs handler now mirrors the Bun server's full capabilities:
- stdin piping for Claude prompt delivery
- stdout capture for Claude JSONL stream parsing
- Live stderr streaming with 200ms buffer-and-flush for job:log events
- Claude JSONL formatting via vendored formatClaudeLogEvent
- onJobComplete callback for result parsing and annotation push
- Full buildCommand integration in POST handler
- jobOutputPaths tracking with cleanup on kill
Pi serverReview.ts now wires buildCommand and onJobComplete with the same
logic as the Bun review server — Codex and Claude commands are built
server-side, results are parsed and transformed into external annotations.
Runtime compatibility: replaced Bun.file/Bun.write in codex-review.ts with
node:fs/promises equivalents (writeFile, readFile, existsSync) that work on
both Bun and Node. Verified Bun build passes.
Vendoring: vendor.sh now copies codex-review.ts, claude-review.ts, and
path-utils.ts from packages/server/ with import path rewriting for the
generated/ layout.
Also: Review Prompt label, px-8 padding on agent detail header/tabs/logs.
For provenance purposes, this commit was AI assisted.
* fix: remove duplicate isPRMode declaration in Pi serverReview.ts
For provenance purposes, this commit was AI assisted.
* fix: include reasoning in all copy and feedback export paths
- exportReviewFeedback: appends **Reasoning:** after finding text
- Per-finding copy button: appends reasoning to copy text
- Sidebar annotation copy: appends reasoning to copy text
This ensures reasoning flows through Copy All, Send Feedback, and
individual copy actions — not just the visual rendering.
For provenance purposes, this commit was AI assisted.
* fix: six verified findings — navigation, dedup, cleanup, copy, diff match, Windows paths
1. openDiffFile: clicking a finding now navigates to the correct file
before selecting the annotation (was silently selecting in wrong file)
2. SEVERITY_STYLES: extracted to packages/ui/types.ts as shared constant,
imported in both ReviewAgentJobDetailPanel and InlineAnnotation
(was duplicated with per-render rebuild in InlineAnnotation)
3. killJob: added jobOutputPaths.delete calls to match Pi's version
(was leaking two strings per killed job)
4. CommentActions: replaced hand-rolled copy with CopyButton inline
variant (was reimplementing useState/clipboard/setTimeout pattern)
5. Branch mode prompt: changed from three-dot to two-dot to match
the UI's actual diff computation (agent was reviewing different diff)
6. toRelativePath: uses path.relative + forward-slash normalization
for Windows compatibility (was string prefix matching with / only)
For provenance purposes, this commit was AI assisted.
* perf: wrap ReviewSidebar in React.memo to prevent re-renders during log streaming
Every job:log SSE event triggers setJobLogs in useAgentJobs, which re-renders
App.tsx. Without memo, the sidebar re-renders on every event (~5/sec) even
though its props (agentJobs.jobs, capabilities, callbacks) haven't changed.
This caused visible flickering when a review tab was open during agent runs.
React.memo shallow-compares props — all sidebar props are stable references
(jobs array only changes on status events, callbacks are useCallback-wrapped),
so the sidebar correctly skips re-renders during log streaming.
For provenance purposes, this commit was AI assisted.
* fix(security): remove find/ls/cat from Claude allowed tools, add glab CLI
Security: Bash(find:*) allowed find -exec to spawn arbitrary subprocesses
that bypassed --disallowedTools. Removed find, ls, and cat — Claude has
Glob, Read, and Grep built-in which cover file access without shell exec.
Feature: Added glab mr view/diff/list and glab api to allowed tools so
Claude can inspect GitLab MR context in remote-mode reviews.
For provenance purposes, this commit was AI assisted.
* fix: Pi addAnnotations, Pi stdout drain, cross-repo exit codes
Pi extension:
- Add addAnnotations() to external-annotations.ts return object —
serverReview.ts calls it when agent jobs complete but the method
was missing (build/runtime error)
- Change proc.on('exit') to proc.on('close') in agent-jobs.ts —
Node's 'exit' fires before stdio streams drain, so stdoutBuf could
be incomplete when onJobComplete parses Claude's JSONL result
Cross-repo --local:
- Check git checkout FETCH_HEAD exit code — throw if it fails so the
outer catch falls back to remote-only with a clear warning
- Log warning if baseSha fetch fails (non-fatal, agents just can't
diff locally)
For provenance purposes, this commit was AI assisted.
* fix: FETCH_HEAD ordering, Pi worktree-aware cwd, SEVERITY_ORDER hoisted
Critical:
- Move ensureObjectAvailable before PR head fetch — it can overwrite
FETCH_HEAD if baseSha needs fetching, causing createWorktree to
check out the base commit instead of the PR head
- Pi serverReview.ts: extract resolveAgentCwd() helper used by getCwd,
buildCommand, and onJobComplete — was bypassing worktree-aware path
resolution, causing agents to run in wrong directory
Cleanup:
- Hoist SEVERITY_ORDER to module scope in ReviewAgentJobDetailPanel
(was recreated inside component body on every render)
For provenance purposes, this commit was AI assisted.
* fix: Bun/Pi parity — provider default, annotation error logging
- Bun agent-jobs: change provider default from "shell" to "" (shell was
removed from capabilities, default should match Pi)
- Pi serverReview: log errors from addAnnotations in onJobComplete
(Bun logs them, Pi was silently ignoring)
For provenance purposes, this commit was AI assisted.
* docs: add AI Code Review Agents guide with full prompt transparency
New docs page covering:
- Overview of Codex and Claude review agents
- How findings work (severity/priority, reasoning, navigation)
- Local worktree behavior (same-repo vs cross-repo)
- Full transparency section with:
- Claude multi-agent pipeline prompt (all 6 steps)
- Claude command and allowed/blocked tools
- Codex review prompt and command
- Both output schemas (Claude severity + Codex priority)
- Security notes (read-only, no network, local execution, no commenting)
- Customization via CLAUDE.md and REVIEW.md
For provenance purposes, this commit was AI assisted.
* docs: add provenance links for Claude and Codex review integrations
Credit Anthropic's Claude Code Review service, the open-source
code-review plugin, and OpenAI's Codex CLI as the foundations
for our review agent integrations.
For provenance purposes, this commit was AI assisted.
* fix: temp clone leak, j/k key conflict, thread header null line
- Cross-repo: clean up localPath in catch block when fetch/checkout
fails after clone succeeds (directory was leaking in /tmp)
- Remove j/k/arrow keyboard navigation from PR comments panel —
these shortcuts belong to the file tree only, both registering
global handlers caused double-navigation
- Thread header null guard: check thread.line before building range
label to prevent "L12–null" for outdated GitHub threads
For provenance purposes, this commit was AI assisted.
* fix: hoist localPath for catch-block scope, validate baseSha format
The previous rmSync(localPath) in the catch block was dead code —
const declarations inside try are not in scope in catch (separate
lexical environments per ECMAScript spec). The ReferenceError was
silently swallowed by the inner try/catch, so temp directories
still leaked on failed fetch/checkout.
Fix: hoist `let localPath` before the try block so it's accessible
in catch. Guard with `if (localPath)` since the error could occur
before assignment.
Also: validate baseSha is a hex SHA (40-64 chars) to prevent git
flag injection via crafted API responses. Validate baseBranch
rejects both '..' and '-' prefixes.
For provenance purposes, this commit was AI assisted.
* docs: rewrite AI Code Review guide for clarity and readability
Restructured for a technical audience: shorter paragraphs, cleaner
tables, removed em-dashes and filler prose, tightened the pipeline
diagram, streamlined security section into scannable single-line items.
For provenance purposes, this commit was AI assisted.
* docs: rewrite transparency section with exact prompts and commands
Replaced summarized/paraphrased transparency section with the actual
prompts, commands, schemas, and tool allowlists as they exist in the
code. One short security note at the top, then raw content.
For provenance purposes, this commit was AI assisted.
* docs: add mini TOC to transparency section
For provenance purposes, this commit was AI assisted.
* fix: stdout drain hang, Codex verdict override, Claude parse logging, memo removal
Critical:
- Race stdoutDone against 2s timeout after proc.exited — prevents
permanent job hang when Bun's ReadableStream doesn't close after
process exit. The process is dead; 2s is a cleanup deadline.
Bug fix:
- Codex verdict: override to "Issues Found" when P0/P1 findings exist,
regardless of the freeform overall_correctness string. Prevents green
"Correct" badge when Codex says "mostly correct but has issues."
P2/P3-only findings still trust Codex's verdict.
Observability:
- Log Claude parse failures with buffer size and last 200 bytes so we
can diagnose empty-findings cases.
Performance:
- Remove React.memo from ReviewSidebar — was blocking legitimate
re-renders (job status, findings) to prevent cosmetic log flickering.
The tradeoff was wrong.
Docs:
- Remove "shell" from CLAUDE.md capabilities table (provider was removed).
For provenance purposes, this commit was AI assisted.
* fix: type assertions for Bun ReadableStream async iteration
Bun's proc.stdout/stderr support for-await at runtime but TypeScript's
ReadableStream type doesn't declare [Symbol.asyncIterator]. Cast through
unknown to AsyncIterable<Uint8Array> — standard Bun workaround, same
pattern as the FileSink cast for stdin.
For provenance purposes, this commit was AI assisted.
|
||
|
Michael Ramos
|
588b6a4de7 |
feat(review): support GitHub Enterprise and fix self-hosted GitLab MR diffs (#460)
* feat(review): support GitHub Enterprise and fix self-hosted GitLab MR diffs Add `host` field to GitHub PR types and update URL parsing to accept any host with `/pull/` pattern, enabling GitHub Enterprise Server URLs like `ghe.company.com/owner/repo/pull/123`. All 8 `gh` CLI functions now pass `--hostname` (for `gh api`/`gh auth`) and `HOST/OWNER/REPO` (for `gh pr`) when targeting non-github.com hosts. Replace broken `glab mr diff --hostname` invocation (glab mr subcommands don't support --hostname) with `glab api .../diffs` which does. Diffs are now reconstructed from structured API response instead of heuristic parsing of bare diff output. Closes #348 Closes #457 For provenance purposes, this commit was AI assisted. * fix(review): remove destructive regex from GitLab paginated JSON parser glab api --paginate already returns a single merged JSON array, so the regex that replaced ][ with , was unnecessary and could corrupt diff content containing array indexing patterns like arr[i][j]. For provenance purposes, this commit was AI assisted. |
||
|
Michael Ramos
|
40002e69dc |
feat: GitLab merge request review support (#364)
* feat: GitLab merge request review support Add full GitLab MR review parity with existing GitHub PR review: - Auto-detect platform from URL (github.com vs any GitLab host) - Extract GitHub logic into pr-github.ts, new pr-gitlab.ts implementation - Widen PRRef/PRMetadata to discriminated unions for type safety - Dispatch functions route to correct platform implementation - Platform-aware UI labels (PR/MR, #/!, GitHub/GitLab icons) - Self-hosted GitLab support via --hostname flag - Normalize glab diff output to standard git format - Handle glab CLI differences (no --jq, Content-Type header for --input) - Defensive JSON parsing for GitLab context API responses Tested against gitlab.com with inline comments, multi-line ranges, approval, and PR context tabs (summary, comments, checks). For provenance purposes, this commit was AI assisted. * fix: correct GitLab enum mappings and add shared file path encoding - Map GitLab job statuses to UI-expected enums (failed→FAILURE, canceled→NEUTRAL) - Map GitLab detailed_merge_status to CLEAN/BLOCKED/BEHIND/DIRTY/UNKNOWN - Fix false approval state on repos without required approvers - Add shared encodeApiFilePath helper used by both GitHub and GitLab For provenance purposes, this commit was AI assisted. * fix: align panel headers and refine file tree selection style - Use shared --panel-header-h CSS variable for consistent header heights across file tree search, file header, and annotations panel - Update GitLab icon to use official tanuki SVG paths with currentColor - Replace solid primary fill on active file tree items with 30% tinted background for better readability and semantic color preservation For provenance purposes, this commit was AI assisted. |