diff --git a/.gitignore b/.gitignore index 74c30f1..e41de35 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ # Node / Vite node_modules/ +/.pnpm-store/ ui/dist/ ui/.vite/ **/*.tsbuildinfo diff --git a/README.md b/README.md index 3e57fc3..a873790 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,9 @@ keyboard-first, but never keyboard-only: almost every action works from the keyboard alone, and the mouse stays first-class. Repository and Work tabs close from their close control, with Delete/Backspace while focused, or by middle-click. Light, dark, and system appearance apply consistently to the shell, code diffs, -and the live Diff settings preview. +and the live Diff settings preview. Diff syntax colors offer five paired Pierre +palettes—including red–green and blue–yellow accessible variants—that follow +the resolved app appearance automatically. ## Features @@ -87,11 +89,15 @@ and the live Diff settings preview. - **Review view (⌘5)** — read an agent's changes as whole files with the edits inline, not isolated hunks. A file-tree queue tracks what you've - reviewed, a pinnable baseline captures everything since a commit — + reviewed; staged files remain visible, and the branch-start baseline + captures every commit since the detected fork point — including work the agent already staged or committed — and a change map beside the scrollbar shows where every edit sits in the file (click to jump). Inline feedback notes persist with their baseline/branch comparison, - so switching review targets never mixes two agents' feedback. + so switching review targets never mixes two agents' feedback. The selected + Codex or Claude Code subscription can inspect that exact review set for + possible defects. Findings stay pending until you explicitly add selected + ones as severity-labelled notes; AI review never edits repository files. - **Hosted pull requests** — browse the latest 100 GitHub or Azure DevOps PRs for the active repository, with the active PR for your checked-out branch opening and being followed automatically even before the PR view is opened. @@ -113,8 +119,8 @@ and the live Diff settings preview. and selected-file addition/deletion totals, track exact-head viewed/changed progress, filter unviewed files or unresolved threads, switch stacked/split layout in place, jump from timeline comments to their - file/thread, read GitHub review threads with replies directly beneath their code, - reply to them, resolve or reopen them, and add stale-head-guarded GitHub or + file/thread, read GitHub or Azure review threads with replies directly beneath + their code, reply to them, resolve or reopen them, and add stale-head-guarded GitHub or Azure comments to selected line ranges. Selected ranges can instead be queued in one preserved, exact-head review draft with Markdown preview and submitted as Comment, Approve, or Request changes; GitHub batches the pending comments atomically, @@ -179,9 +185,10 @@ and the live Diff settings preview. annotated-tag, all-tag, and guarded force-with-lease pushes; explicit push of any local branch to a chosen remote destination; upstream set/change/unset; selected-remote-branch fetch/pull; richer ref menus copy names, refs, and SHAs; - local branches merged into the primary branch are marked in the sidebar and - commit graph and can be cleared in bulk with independent local/remote - selection; branches, tags, stashes, remotes, ordered multi-commit + local branches merged into the primary branch by ancestry or by an exact-tip + completed GitHub/Azure PR are marked in the sidebar and commit graph and can + be cleared in bulk with independent, guarded local/remote selection; branches, + tags, stashes, remotes, ordered multi-commit cherry-pick, merge-mainline cherry-pick/revert, branch/tag/commit comparison, merge, and a fully keyboard-operable interactive rebase (reorder, reword, edit/pause-to-amend, squash, fixup, drop, and merge preservation) with @@ -226,11 +233,11 @@ and the live Diff settings preview. terminal / diff / git / hosting / integrations / AI, consistent keyboard-native dropdowns, and update checks for both direct and Microsoft Store installations. -- **AI commit messages** — suggest subject + body from staged changes (or all +- **AI writing and code review** — suggest subject + body from staged changes (or all unstaged changes when nothing is staged) via your ChatGPT subscription (Codex CLI, `gpt-5.6-luna`) or Claude Code CLI (`claude-sonnet-5` by default); Settings → AI for focused provider sign-in, - per-provider model selection used by commit and PR generation, and CLI health + per-provider model selection used by commit, PR, and Review generation, and CLI health checks with a remembered connected indicator. Packaged builds resolve these tools and their runtimes through the user's recovered Unix or Windows `PATH`; custom paths remain available. @@ -240,6 +247,9 @@ and the live Diff settings preview. default. Provider failures are reduced to concise, actionable hints; raw CLI session, prompt, and patch transcripts are never displayed. Repository-family writing profiles keep terminology and style consistent across worktrees. + Review findings are structured, path/line-validated, stale-diff guarded, and + require explicit acceptance before they become notes; repository files are + never changed by an AI review. - **Fast by design** — reads go through [gix](https://github.com/GitoxideLabs/gitoxide), writes through git2 and your system `git`. Performance targets live in [`PRD.md`](./PRD.md) §8 and are measured in @@ -258,7 +268,8 @@ the accepted package, and production Store signing is complete. Store installs check Microsoft's native package-update API on launch, notify when an update is available, and hand installation back to the Store. DPI-tailored unplated icon assets keep the Store taskbar and Start icon as sharp and background-free as -the direct MSI. The standalone GitHub MSI remains unsigned; the +the direct MSI, while explicit native Windows icon handles keep the taskbar +identity intact across in-place updates. The standalone GitHub MSI remains unsigned; the certificate-backed offline-WebView2 MSI workflow is only a fallback. Listing copy, privacy and user-content policies, in-product inappropriate- content reporting, and release credentials are configured. The first automated diff --git a/ROADMAP.md b/ROADMAP.md index f8c5466..5abcd55 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -76,7 +76,8 @@ system ported verbatim. No real feature surface yet. create + track a remote branch, or `Create branch…` via prompt - ☑ Clear merged branches from the command palette with per-branch local and matching remote selection (`BranchCleanupDialog`; worktree-held branches - stay protected) + stay protected; DAN-41 also recognizes exact tips from completed + GitHub/Azure squash or rebase PRs without weakening remote deletion) - ☑ **File tree** - ☑ Working-tree view, status badges, click to file detail (`Repo::work_tree` lists index entries overlaid with status; Sidebar Files tab renders a @@ -85,7 +86,8 @@ system ported verbatim. No real feature surface yet. - ☑ **macOS packaging** — *release CI builds, signs, and notarizes the universal DMG (v0.5.0, 2026-06-12). See `docs/packaging.md` for the runbook.* - ☑ Real app icon (canonical `strand.svg` mark, rounded white-tile - `strand.png`, and generated desktop and platform bundles) + `strand.png`, generated desktop/platform bundles, and DAN-40's updater-safe + native Windows taskbar handle assignment) - ☑ Apple Developer ID signing + notarization (release CI signs + notarizes `Strand_0.5.0_universal.dmg`) - ☑ First DMG ships — superseded by the public v0.5.0 GitHub Release @@ -1070,11 +1072,11 @@ passes on its measured platform. Doc-only change: PRD §8, `docs/perf-baseline.m rendered Markdown, color-coded checks, discussions with top-level comment creation, current-branch auto-open, and full-width lazy selected-file Pierre diffs shipped 2026-07-13 through authenticated provider CLIs. Inline/review - comments, exact-head batched GitHub/Azure inline reviews, GitHub thread + comments, exact-head batched GitHub/Azure inline reviews, GitHub/Azure thread replies/resolution, permission-gated Ready for review, merge controls, close/reopen, PR worktrees, and GitHub branch update now ship. The accepted - 1.0 review bar is closed; Azure existing-thread lifecycle and pagination - beyond the bounded GitHub detail query are 1.1 work. Azure DevOps Server + 1.0 review bar is closed; pagination beyond the bounded GitHub detail query + is 1.1 work. Azure DevOps Server 2020+ now uses the optional release-pinned `strand-azdo` REST helper with PAT/private-CA profiles and Windows integrated authentication, while Azure DevOps Services keeps the official `az` path. GitLab and Bitbucket are @@ -2174,8 +2176,8 @@ and Store certification remain external gates. - ☑ Isolated, cancellable provider execution with sensitive-file confirmation, deterministic bounded context, per-repository writing profiles, PR-draft coverage/undo, and explicit alternate-provider retry -- ◐ Hosted-review expansion — GitLab/Bitbucket adapters, Azure existing-thread - lifecycle, deeper pagination, merge queue/auto-complete, and review-evolution +- ◐ Hosted-review expansion — GitLab/Bitbucket adapters, deeper pagination, + merge queue/auto-complete, and review-evolution comparisons build on the completed GitHub/Azure 1.0 workspace. **AI commit messages (2026-07-01):** Subscription-first suggestions prefer the @@ -2493,6 +2495,59 @@ buttons. The controls track the same history as Mod+Z, Mod+Shift+Z, and Ctrl+Y, and reset when Strand intentionally rebuilds the editor after a clean external refresh or Discard. +**DAN-41 provider-aware merged branches shipped (2026-08-07):** Sidebar and +commit-graph checks plus **Clear merged branches…** now recognize a local branch +whose exact current tip is the recorded source of a completed GitHub or Azure +PR into the primary branch, covering squash and rebase merges that Git ancestry +cannot prove. Provider lookup is delayed and session-cached off the repo-open +hot path; explicit cleanup refreshes and freezes the snapshot, keeps remote +deletion ancestry-only, and rechecks the local branch SHA at deletion time. + +**DAN-40 updater-safe Windows taskbar icon shipped (2026-08-07):** Strand now +loads the embedded `icon.ico` resource into explicit big and small native HWND +icon handles after Tauri reaches `RunEvent::Ready`. Windows no longer depends +on its executable-path icon cache after an in-place updater replacement. A +native before/after probe moved the visible window from zero icon handles to +the correct Strand artwork, and the release policy check locks the contract. + +**DAN-39 Azure review-thread lifecycle shipped (2026-08-07):** Azure DevOps +Services and Server inline discussions now retain provider range/side/status +metadata in Code, group replies beneath the anchored diff, and expose the same +Reply, Resolve, and Reopen controls as GitHub on open pull requests. Azure +Services writes through the signed-in `az` CLI; Server uses helper protocol v6. + +**DAN-30 Windows resource-usage audit closed (2026-08-07):** The reported +326.5MB is Task Manager working set for WebView2's normal manager, GPU, +renderer, network, storage, and crashpad process group—not Strand-private +allocation. It is below the measured 408MB empty-shell working-set baseline; +the production acceptance measure remains 248→280MB private bytes and 7MB JS +heap with one medium repository, within the Windows PRD budget. + +**DAN-29 Pierre syntax palettes shipped (2026-08-07):** Settings → Diff now +offers Standard, Soft, Vibrant, red–green-accessible, and blue–yellow-accessible +syntax-color families with a live preview. Strand automatically selects each +family's matching light or dark Pierre theme, persists the choice, lazy-loads +non-default palettes, and updates the shared highlight worker cache once per +family change across local, review, hosted-PR, and merge-conflict surfaces. + +**DAN-18 AI code review shipped (2026-08-07):** Review can now ask the selected +Codex or Claude Code subscription to inspect the exact uncommitted inbox or pinned +baseline session for actionable defects. Findings return as severity-labelled, +pending suggestions that require explicit acceptance before becoming removable +review notes; the backend rejects invented paths, downgrades invalid +line anchors to file notes, bounds context/output, and reuses sensitive-file, +cancellation, authentication, and provider-fallback contracts. A content hash +prevents an answer from attaching after the reviewed diff changes, and AI +review never edits repository files or persists findings automatically. + +**Review continuity and branch-start baselines shipped (2026-08-07):** Inbox +mode now compares HEAD to the index-overlaid working tree, so staged and +partially staged files stay visible instead of disappearing after acceptance. +The initial baseline action detects the current branch's parent and pins the +fork-point merge base, making the complete branch history the default session +scope. Combined staged patches remain read-only at hunk level so index writes +cannot apply the wrong patch. + --- ## Cross-cutting tracks (run in parallel with all milestones) diff --git a/TASKS.md b/TASKS.md index 763d7e9..30b4859 100644 --- a/TASKS.md +++ b/TASKS.md @@ -731,13 +731,15 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- (confirm). HEAD shows a disabled "Current branch". - ☑ Merged-branch indicators (DAN-19 — `refs::Branch.merged` uses commit ancestry against the repository's primary branch; sidebar icons and - commit-graph ref chips mark contained non-current branches that are safe to - delete, without mislabeling the primary branch while a feature is checked out). + commit-graph ref chips also use `providerMergedBranchNames` to mark an exact + local source tip from a completed GitHub/Azure PR into that primary branch, + without mislabeling the primary branch while a feature is checked out). - ☑ Clear merged branches in bulk (`BranchCleanupDialog` + `mergedBranchCleanupPlan`: palette action with per-branch local selection, opt-in matching upstream/origin deletion, checked-out-worktree exclusion, remote-tip containment via `RemoteBranch.merged`, and a deletion-time - `Repo::delete_branch(force=false)` containment/worktree guard). + ancestry guard or provider-tip `Repo::delete_branch_at` guard; DAN-41 adds + squash/rebase PR cleanup while keeping provider-unproven remotes protected). - ☑ Ref clipboard/context expansion (2026-07-16): local branches copy name / full ref / SHA and the current branch exposes Pull + Push strategy submenus; remote branches copy short name / remote ref / SHA; tags copy name / SHA and @@ -1289,11 +1291,14 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- initializes) - ☑ **Diff**: default layout (`defaultDiffLayout`, seeds repos without a per-repo `diff-mode:` row — `loadRepoDiffMode` falls back to it), diff font - (`--diffs-font-family`, pierces Pierre's shadow DOM), change indicators + (`--diffs-font-family`, pierces Pierre's shadow DOM), five paired Pierre + syntax-palette families (`diffSyntaxTheme`, DAN-29), change indicators (`classic`/`bars`/`none`), line numbers, word-level highlight; live Pierre - preview. Options flow through `diffAppearanceOptions()` (`components/Diff.tsx`) - into both `Diff` and LocalChanges' `fileDiffOptions` memo. MergeResolver - deliberately stays pinned (gutter measurement). + preview. Syntax palettes flow through `pierreThemePair()` into the shared + worker pool and every diff/merge surface; the remaining options flow through + `diffAppearanceOptions()` (`components/Diff.tsx`) into both `Diff` and + LocalChanges' `fileDiffOptions` memo. MergeResolver keeps its gutter-affecting + settings pinned for reliable measurement. - ☑ **Git**: global `user.name`/`user.email` read/write (`gitconfig::global_identity` / `set_global_identity`, IPC `git_global_identity` / `git_set_global_identity`) + default clone/open @@ -1424,16 +1429,17 @@ tree: watch the agent work, review fast, accept or reject safely. - ☑ **Dedicated Review view** (`views/Review.tsx`, sidebar "Review" row with a pending-count badge, ⌘4, palette "Show: Review") — review lives in its own surface; Local Changes stays a pure staging workspace. Two modes: - **inbox** (no baseline → the unstaged set; diffs keep per-hunk + **inbox** (no baseline → staged + unstaged changes combined against HEAD; + staging never removes a file, and unstaged-only diffs keep safe per-hunk Stage/Discard via the shared `HunkAnnotatedDiff`) and **session** (baseline pinned → everything since that commit incl. the agent's commits, rendered read-only with file-level actions). Queue on the left (Pierre tree), one file at a time on the right, progress bar + verdict actions in the toolbar, keyboard-hint footer. - ☑ Review diffs carry **whole-file context** — the agent's edits read - inside the entire file, not isolated hunks (`diff_unstaged_full` / - `diff_since_full` in `strand-core/src/diff.rs`, - `repo_diff_unstaged_full` / `repo_diff_since_full` IPC; inbox pool lives + inside the entire file, not isolated hunks (`diff_since_full` in + `strand-core/src/diff.rs`, `repo_diff_since_full` IPC; inbox compares + against `HEAD` so the pool survives staging and lives in `reviewUnstagedDiffs`, refreshed only while the Review view is live so Local Changes' hot path doesn't pay for it). - ☑ Review queue is a Pierre tree (`PierreTree` with the new @@ -1466,7 +1472,9 @@ tree: watch the agent work, review fast, accept or reject safely. (`diff_tree_to_workdir_with_index` against the baseline tree, so committed + staged + unstaged agent work shows in one diff), `repo_diff_since` IPC, `RepoMeta.head_oid` to pin it, persisted per-repo (`reviewSession` in - `lib/db.ts`); pin/move/clear from the Review toolbar or the palette. + `lib/db.ts`); the initial toolbar/palette action detects the current branch's + parent with `repoDetectBaseBranch` and pins its merge base, while move/clear + and explicit commit-graph baselines remain available. - ☑ Review-state tracking: reviewed map (`path → FNV hash of the diff`, `hashPatch` in `lib/patch.ts`) — a file the agent touches after review flips back to unreviewed (row shows "changed"); persisted per-repo in @@ -1563,8 +1571,8 @@ tree: watch the agent work, review fast, accept or reject safely. signed-in `gh` / `az` accounts plus helper/profile authentication readiness (`hosting_connection_status`, `HostingSection`). Azure DevOps Services continues to use the official `az` CLI. Both Azure - adapters support iteration-tracked inline comments and review submission; - replies/resolution on existing Azure threads remain out of scope. + adapters support iteration-tracked inline comments, review submission, and + reply/resolve/reopen writes on existing inline threads (protocol v6). - ☑ Hide provider write controls for terminal pull requests: merged/completed PRs expose read-only Summary, Timeline, Code, and thread cards; closed/ abandoned PRs keep only their Reopen lifecycle action @@ -1620,16 +1628,17 @@ tree: watch the agent work, review fast, accept or reject safely. character count, provider avatars with initials fallback, and comment permalinks. File-backed timeline comments expose a keyboard-operable **View in Code** action that selects the file and focuses its fetched - GitHub thread when coordinates exist. GitHub Code uses Pierre's native hover-gutter `+`, + provider thread when coordinates exist. Code uses Pierre's native hover-gutter `+`, line-range selection, persistent fetched thread cards with replies and resolved/outdated state, and an annotation-row composer through `repo_pull_request_inline_comment`, with - exact-head validation before publishing. GitHub thread cards now publish + exact-head validation before publishing. GitHub and Azure thread cards publish immediate replies and Resolve/Reopen writes through provider-capability- gated GraphQL mutations, patching Code + Timeline locally without a detail/patch reload (`repo_pull_request_thread_reply`, - `repo_pull_request_thread_resolve`). Azure replies/resolution on existing - threads, direct binary attachment uploads, and suggestions are 1.1 scope. + `repo_pull_request_thread_resolve`; Azure Services uses `az devops invoke` + and Azure Server uses helper protocol v6). Direct binary attachment uploads + and suggestions remain 1.1 scope. - ☑ Submit reviews: comment, approve, and request changes through one exact-head review draft (`repo_pull_request_submit_review`, GitHub atomic review payload, Azure Services/Server iteration-tracked inline writes plus @@ -1750,7 +1759,9 @@ tree: watch the agent work, review fast, accept or reject safely. - ☑ Replace placeholder icon with a real source (canonical `strand.svg` mark, rounded white-tile `strand.png` app icon, and generated desktop, Store, - Android, and iOS bundles) + Android, and iOS bundles; DAN-40 reasserts the embedded `icon.ico` as native + big/small HWND icons on Windows `RunEvent::Ready`, guarded by the release + policy check, so updater replacement cannot leave a generic taskbar icon) - ☑ Apple Developer ID + notarization pipeline. Local signing: `pnpm tauri build --target aarch64-apple-darwin` + `APPLE_SIGNING_IDENTITY` yields a Developer-ID-signed DMG. Release CI signs **and notarizes** the @@ -1794,6 +1805,9 @@ tree: watch the agent work, review fast, accept or reject safely. — lanes/merges/invariants; `lib/conflictParse.test.ts` — parse + resolution assembly; `lib/fuzzy.test.ts` — palette scoring, extracted to `lib/fuzzy.ts`. `pnpm --filter ./ui test`.) +- ☑ Stale Vite optimizer recovery after dependency upgrades + (`ui/scripts/clean-stale-js.mjs` validates cached optimizer source paths and + removes only `ui/node_modules/.vite` when pnpm has removed one; 2026-08-07). - ☑ Stable auto-update channel is signed and fail-closed; GitHub's stable `releases/latest` endpoint excludes prereleases. A user-selectable beta channel is explicitly 1.1 scope so 1.0 cannot silently change trust channels @@ -1857,7 +1871,10 @@ a running-app pass. targets: macOS **< 250MB** (unchanged; confirm on the Mac box), Windows **< 300MB private** plus app-attributable **< 50MB over the empty shell** (measured ~32MB — the number app code actually controls), Linux TBD at the - GNOME+KDE platform pass. Windows passes both restated figures. + GNOME+KDE platform pass. Windows passes both restated figures. DAN-30's + 326.5MB Task Manager WebView2-group screenshot is working set, not private + bytes, and remains below the measured 408MB empty-shell working-set baseline + (`docs/perf-baseline.md`). - ☑ **Virtualize the Local Changes stacked diff pane** (perf follow-up from the webview pass) — done 2026-07-06. `DiffPane` (`views/LocalChanges.tsx`) now wraps the stacked file list in Pierre's `` @@ -2218,6 +2235,10 @@ extraction above as prerequisite. **Do not start before 1.0 ships** back to all unstaged changes when no staged diff exists) - ☑ Pull-request title/description suggestions from committed merge-base diffs (`repo_suggest_pull_request`, Create PR **Fill with Codex/Claude Code**) +- ☑ AI review of the exact Review pool (DAN-18: `repo_review_changes`, + structured/validated `CodeReviewFinding`s, Review toolbar + palette action, + stale-diff guard, and a transient approval list; findings change nothing + until the user chooses Add note/Add all, and AI review never edits files) - ☑ Windows CLI spawning hardened (DAN-11: `ai/bin.rs` resolves `.exe`/`.cmd`/ `.bat` only — never npm's extensionless POSIX shims — and runs batch shims via `cmd /C`; prompts travel over stdin; null stdin + 30s/120s timeouts so diff --git a/crates/strand-azdo-protocol/src/lib.rs b/crates/strand-azdo-protocol/src/lib.rs index bb50402..7959196 100644 --- a/crates/strand-azdo-protocol/src/lib.rs +++ b/crates/strand-azdo-protocol/src/lib.rs @@ -7,7 +7,7 @@ use serde_json::Value; use url::Url; use uuid::Uuid; -pub const PROTOCOL_VERSION: u32 = 5; +pub const PROTOCOL_VERSION: u32 = 6; pub const MAX_REQUEST_BYTES: usize = 128 * 1024; pub const MAX_RESPONSE_BYTES: usize = 16 * 1024 * 1024; @@ -101,6 +101,21 @@ pub enum Operation { repository: String, id: u64, }, + ReplyToThread { + project: String, + repository: String, + id: u64, + thread_id: u64, + parent_comment_id: u64, + body: String, + }, + SetThreadStatus { + project: String, + repository: String, + id: u64, + thread_id: u64, + status: ThreadStatus, + }, PullRequestIterations { project: String, repository: String, @@ -176,6 +191,13 @@ pub enum PullRequestStatus { Abandoned, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ThreadStatus { + Active, + Fixed, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ReviewVote { diff --git a/crates/strand-azdo/src/client.rs b/crates/strand-azdo/src/client.rs index 33dc6d0..d5c57a0 100644 --- a/crates/strand-azdo/src/client.rs +++ b/crates/strand-azdo/src/client.rs @@ -213,6 +213,50 @@ fn request_spec(operation: Operation) -> RequestSpec { body: None, unwrap_value: false, }, + Operation::ReplyToThread { + project, + repository, + id, + thread_id, + parent_comment_id, + body, + } => RequestSpec { + method: Method::POST, + path: git_path( + &project, + &repository, + &format!("pullrequests/{id}/threads/{thread_id}/comments"), + ), + query: api(), + body: Some(json!({ + "parentCommentId": parent_comment_id, + "content": body, + "commentType": 1 + })), + unwrap_value: false, + }, + Operation::SetThreadStatus { + project, + repository, + id, + thread_id, + status, + } => RequestSpec { + method: Method::PATCH, + path: git_path( + &project, + &repository, + &format!("pullrequests/{id}/threads/{thread_id}"), + ), + query: api(), + body: Some(json!({ + "status": match status { + strand_azdo_protocol::ThreadStatus::Active => 1, + strand_azdo_protocol::ThreadStatus::Fixed => 2, + } + })), + unwrap_value: false, + }, Operation::PullRequestIterations { project, repository, @@ -709,6 +753,42 @@ mod tests { 27 ); + let reply = request_spec(Operation::ReplyToThread { + project: "Project".into(), + repository: "Repo".into(), + id: 12, + thread_id: 34, + parent_comment_id: 1, + body: "Fixed in the latest revision.".into(), + }); + assert_eq!(reply.method, Method::POST); + assert!(reply + .path + .ends_with("pullrequests/12/threads/34/comments")); + let reply_body = reply.body.unwrap(); + assert_eq!(reply_body["content"], "Fixed in the latest revision."); + assert_eq!(reply_body["parentCommentId"], 1); + + let resolve = request_spec(Operation::SetThreadStatus { + project: "Project".into(), + repository: "Repo".into(), + id: 12, + thread_id: 34, + status: strand_azdo_protocol::ThreadStatus::Fixed, + }); + assert_eq!(resolve.method, Method::PATCH); + assert!(resolve.path.ends_with("pullrequests/12/threads/34")); + assert_eq!(resolve.body.unwrap()["status"], 2); + + let reopen = request_spec(Operation::SetThreadStatus { + project: "Project".into(), + repository: "Repo".into(), + id: 12, + thread_id: 34, + status: strand_azdo_protocol::ThreadStatus::Active, + }); + assert_eq!(reopen.body.unwrap()["status"], 1); + let policy = request_spec(Operation::Policies { project: "Project".into(), project_id: "project-id".into(), diff --git a/crates/strand-core/src/branch.rs b/crates/strand-core/src/branch.rs index b4079f6..4c4fe7e 100644 --- a/crates/strand-core/src/branch.rs +++ b/crates/strand-core/src/branch.rs @@ -212,6 +212,41 @@ impl Repo { Ok(()) } + /// Delete a provider-confirmed merged branch only while it still points at + /// the exact source commit the provider reported. This is the squash/rebase + /// merge counterpart to [`delete_branch`]'s ancestry guard. + pub fn delete_branch_at(&self, name: &str, expected_target: &str) -> Result<()> { + let expected = git2::Oid::from_str(expected_target)?; + let repo = self.git2()?; + let mut branch = repo.find_branch(name, git2::BranchType::Local)?; + if branch.is_head() { + return Err(crate::Error::Other(format!( + "cannot delete branch {name}: it is the current branch" + ))); + } + if let Some(worktree) = self + .worktrees() + .unwrap_or_default() + .into_iter() + .find(|worktree| !worktree.is_current && worktree.branch.as_deref() == Some(name)) + { + return Err(crate::Error::Other(format!( + "cannot delete branch {name}: it is checked out in worktree {}", + worktree.path + ))); + } + let actual = branch.get().target().ok_or_else(|| { + crate::Error::Other(format!("cannot delete branch {name}: it has no commit target")) + })?; + if actual != expected { + return Err(crate::Error::Other(format!( + "cannot delete branch {name}: it moved after its pull request was checked; refresh and try again" + ))); + } + branch.delete()?; + Ok(()) + } + /// Rename a local branch (`git branch -m `). git2 moves the /// branch's config section (upstream) along, and HEAD follows when the /// renamed branch is checked out. No force — errors if `new` exists. @@ -414,4 +449,35 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + + #[test] + fn provider_merged_delete_requires_the_exact_unchanged_tip() { + let (repo, dir) = scratch_repo(); + std::fs::write(dir.join("a.txt"), "a\n").unwrap(); + git(&dir, &["add", "a.txt"]); + git(&dir, &["commit", "-q", "-m", "init"]); + git(&dir, &["branch", "squashed"]); + let target = git(&dir, &["rev-parse", "squashed"]); + + let err = repo + .delete_branch_at("squashed", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + .unwrap_err(); + assert!(err.to_string().contains("it moved")); + assert!(repo + .refs() + .unwrap() + .branches + .iter() + .any(|branch| branch.name == "squashed")); + + repo.delete_branch_at("squashed", &target).unwrap(); + assert!(!repo + .refs() + .unwrap() + .branches + .iter() + .any(|branch| branch.name == "squashed")); + + let _ = std::fs::remove_dir_all(dir); + } } diff --git a/crates/strand-core/src/diff.rs b/crates/strand-core/src/diff.rs index 56cf5b4..c111aeb 100644 --- a/crates/strand-core/src/diff.rs +++ b/crates/strand-core/src/diff.rs @@ -408,6 +408,17 @@ mod tests { paths.sort(); assert_eq!(paths, vec!["a.txt", "b.txt", "c.txt"]); + // Review inbox uses HEAD as its baseline: staged work must remain + // visible alongside loose work after the user stages it. + let mut uncommitted: Vec = repo + .diff_since_full("HEAD") + .unwrap() + .into_iter() + .map(|d| d.path) + .collect(); + uncommitted.sort(); + assert_eq!(uncommitted, vec!["b.txt", "c.txt"]); + // diff_unstaged would only see c.txt — that's the gap diff_since fills. let unstaged: Vec = repo.diff_unstaged().unwrap().into_iter().map(|d| d.path).collect(); diff --git a/crates/strand-tauri/Cargo.toml b/crates/strand-tauri/Cargo.toml index 630771f..370cacb 100644 --- a/crates/strand-tauri/Cargo.toml +++ b/crates/strand-tauri/Cargo.toml @@ -57,6 +57,8 @@ windows = { version = "0.61", features = [ "Foundation", "Services_Store", "System", + "Win32_System_LibraryLoader", + "Win32_UI_WindowsAndMessaging", ] } windows-sys = { version = "0.61", features = [ "Win32_Foundation", diff --git a/crates/strand-tauri/src/ai/input.rs b/crates/strand-tauri/src/ai/input.rs index 758d5e4..9d85fde 100644 --- a/crates/strand-tauri/src/ai/input.rs +++ b/crates/strand-tauri/src/ai/input.rs @@ -28,6 +28,7 @@ pub enum AiInputScope { Staged, Unstaged, Committed, + Review, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/strand-tauri/src/ai/mod.rs b/crates/strand-tauri/src/ai/mod.rs index b519943..b370bdb 100644 --- a/crates/strand-tauri/src/ai/mod.rs +++ b/crates/strand-tauri/src/ai/mod.rs @@ -182,6 +182,44 @@ pub struct PullRequestSuggestion { pub description: String, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "lowercase")] +pub enum CodeReviewSeverity { + Critical, + High, + #[default] + Medium, + Low, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(rename_all = "lowercase")] +pub enum CodeReviewSide { + #[default] + New, + Old, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CodeReviewFinding { + pub path: String, + #[serde(default)] + pub line: Option, + #[serde(default)] + pub side: CodeReviewSide, + #[serde(default)] + pub severity: CodeReviewSeverity, + pub title: String, + #[serde(default)] + pub body: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CodeReviewSuggestion { + #[serde(default)] + pub findings: Vec, +} + pub fn provider_status(provider: AiProvider, cli_override: Option<&str>) -> AiProviderStatus { adapter(provider).status(cli_override) } @@ -313,6 +351,134 @@ pub fn suggest_pull_request_with_request( }) } +#[allow(clippy::too_many_arguments)] +pub fn review_changes_with_request( + provider: AiProvider, + repo_path: &std::path::Path, + diffs: &[FileDiff], + model: Option<&str>, + cli_override: Option<&str>, + cancel: Option<&bin::AiCancelHandle>, + decision: &AiSensitiveDecision, +) -> Result, String> { + if diffs.is_empty() { + return Err("Nothing changed — make a change before starting AI review.".into()); + } + let prepared = match input::prepare_input(diffs, AiInputScope::Review, decision)? { + input::InputPreparation::NeedsConfirmation { + fingerprint, + mut coverage, + sensitive_files, + } => { + let built = prompt::build_code_review_prompt(diffs); + apply_prompt_coverage(&mut coverage, &built); + return Ok(AiGenerationOutcome::NeedsConfirmation { + fingerprint, + coverage, + sensitive_files, + }); + } + input::InputPreparation::Ready(prepared) => prepared, + }; + let mut coverage = prepared.coverage; + let built = prompt::build_code_review_prompt(&prepared.diffs); + apply_prompt_coverage(&mut coverage, &built); + let raw = adapter(provider).suggest(repo_path, &built.text, model, cli_override, cancel)?; + let mut suggestion = parse::parse_code_review_suggestion(&raw)?; + normalize_review_findings(&mut suggestion, &prepared.diffs); + Ok(AiGenerationOutcome::Generated { + suggestion, + coverage, + provider, + }) +} + +fn normalize_review_findings(suggestion: &mut CodeReviewSuggestion, diffs: &[FileDiff]) { + suggestion.findings.truncate(50); + suggestion.findings.retain_mut(|finding| { + let candidate = normalize_path(&finding.path); + let exact = |diff: &&FileDiff, candidate: &str| { + normalize_path(&diff.path) == candidate + || diff + .old_path + .as_deref() + .is_some_and(|path| normalize_path(path) == candidate) + }; + let diff = diffs.iter().find(|diff| exact(diff, &candidate)).or_else(|| { + candidate + .strip_prefix("a/") + .or_else(|| candidate.strip_prefix("b/")) + .and_then(|stripped| diffs.iter().find(|diff| exact(diff, stripped))) + }); + let Some(diff) = diff else { + return false; + }; + finding.path = diff.path.clone(); + finding.title = truncate_utf8(finding.title.trim(), 160).to_string(); + finding.body = truncate_utf8(finding.body.trim(), 2_000).to_string(); + if finding.title.is_empty() { + return false; + } + finding.line = finding + .line + .filter(|line| *line > 0 && patch_contains_line(&diff.patch, *line, finding.side)); + true + }); +} + +fn normalize_path(path: &str) -> String { + let path = path.trim().replace('\\', "/"); + path.strip_prefix("./") + .unwrap_or(&path) + .to_string() +} + +fn patch_contains_line(patch: &str, wanted: u32, side: CodeReviewSide) -> bool { + let mut old_line = None; + let mut new_line = None; + for row in patch.lines() { + if row.starts_with("@@ ") { + let mut parts = row.split_whitespace(); + let _marker = parts.next(); + old_line = parts.next().and_then(|part| patch_range_start(part, '-')); + new_line = parts.next().and_then(|part| patch_range_start(part, '+')); + continue; + } + let (Some(old), Some(new)) = (old_line, new_line) else { + continue; + }; + match row.as_bytes().first().copied() { + Some(b' ') => { + if (side == CodeReviewSide::Old && old == wanted) + || (side == CodeReviewSide::New && new == wanted) + { + return true; + } + old_line = Some(old + 1); + new_line = Some(new + 1); + } + Some(b'-') => { + if side == CodeReviewSide::Old && old == wanted { + return true; + } + old_line = Some(old + 1); + } + Some(b'+') => { + if side == CodeReviewSide::New && new == wanted { + return true; + } + new_line = Some(new + 1); + } + _ => {} + } + } + false +} + +fn patch_range_start(value: &str, prefix: char) -> Option { + value.strip_prefix(prefix)?.split(',').next()?.parse().ok() +} + pub(crate) fn truncate_utf8(value: &str, max_bytes: usize) -> &str { let mut end = value.len().min(max_bytes); while !value.is_char_boundary(end) { @@ -456,4 +622,56 @@ mod tests { assert_eq!(truncated.len(), 120); assert!(truncated.is_char_boundary(truncated.len())); } + + #[test] + fn review_findings_are_bounded_and_anchored_to_supplied_diffs() { + let diffs = vec![FileDiff { + path: "src/lib.rs".into(), + old_path: None, + status: strand_core::diff::DiffStatus::Modified, + adds: 1, + dels: 1, + binary: false, + patch: + "diff --git a/src/lib.rs b/src/lib.rs\n@@ -10,2 +10,2 @@\n-old\n+new\n context\n" + .into(), + }]; + let mut suggestion = CodeReviewSuggestion { + findings: vec![ + CodeReviewFinding { + path: "b/src/lib.rs".into(), + line: Some(10), + side: CodeReviewSide::New, + severity: CodeReviewSeverity::High, + title: " Race ".into(), + body: " Unsynchronized write. ".into(), + }, + CodeReviewFinding { + path: "src/lib.rs".into(), + line: Some(99), + side: CodeReviewSide::New, + severity: CodeReviewSeverity::Low, + title: "Bad line".into(), + body: String::new(), + }, + CodeReviewFinding { + path: "invented.rs".into(), + line: Some(1), + side: CodeReviewSide::New, + severity: CodeReviewSeverity::Critical, + title: "Hallucinated".into(), + body: String::new(), + }, + ], + }; + + normalize_review_findings(&mut suggestion, &diffs); + + assert_eq!(suggestion.findings.len(), 2); + assert_eq!(suggestion.findings[0].path, "src/lib.rs"); + assert_eq!(suggestion.findings[0].line, Some(10)); + assert_eq!(suggestion.findings[0].title, "Race"); + assert_eq!(suggestion.findings[0].body, "Unsynchronized write."); + assert_eq!(suggestion.findings[1].line, None); + } } diff --git a/crates/strand-tauri/src/ai/parse.rs b/crates/strand-tauri/src/ai/parse.rs index 5d05dfe..1a8d6d8 100644 --- a/crates/strand-tauri/src/ai/parse.rs +++ b/crates/strand-tauri/src/ai/parse.rs @@ -1,4 +1,4 @@ -use super::{CommitMessageSuggestion, PullRequestSuggestion}; +use super::{CodeReviewSuggestion, CommitMessageSuggestion, PullRequestSuggestion}; const MAX_COMMIT_BODY_BYTES: usize = 65_536; const MAX_DIAGNOSTIC_BYTES: usize = 2_048; @@ -59,6 +59,28 @@ pub fn parse_pull_request_suggestion(raw: &str) -> Result Result { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Err("AI returned an empty response.".into()); + } + serde_json::from_str::(trimmed) + .ok() + .or_else(|| { + extract_json_object(trimmed) + .and_then(|json| serde_json::from_str::(&json).ok()) + }) + .or_else(|| { + extract_fenced_json(trimmed) + .and_then(|json| serde_json::from_str::(&json).ok()) + }) + .ok_or_else(|| { + "Could not parse AI review. Expected JSON with a findings array.".to_string() + }) +} + fn normalize(mut s: CommitMessageSuggestion) -> Result { s.subject = s.subject.trim().to_string(); if s.subject.is_empty() { @@ -236,4 +258,18 @@ mod tests { assert!(suggestion.title.len() <= 512); assert!(suggestion.title.is_char_boundary(suggestion.title.len())); } + + #[test] + fn parses_code_review_json_in_prose() { + let raw = "Review:\n{\"findings\":[{\"path\":\"src/lib.rs\",\"line\":7,\"side\":\"new\",\"severity\":\"high\",\"title\":\"Race\",\"body\":\"The write is unsynchronized.\"}]}"; + let review = parse_code_review_suggestion(raw).unwrap(); + assert_eq!(review.findings.len(), 1); + assert_eq!(review.findings[0].path, "src/lib.rs"); + } + + #[test] + fn code_review_parse_error_does_not_echo_source() { + let err = parse_code_review_suggestion("secret source copied by model").unwrap_err(); + assert!(!err.contains("secret source")); + } } diff --git a/crates/strand-tauri/src/ai/prompt.rs b/crates/strand-tauri/src/ai/prompt.rs index 9d28b68..41170c7 100644 --- a/crates/strand-tauri/src/ai/prompt.rs +++ b/crates/strand-tauri/src/ai/prompt.rs @@ -20,6 +20,12 @@ Reply with JSON only, no markdown fences: {\"title\":\"...\",\"description\":\". Keep the title concise. Write a useful Markdown description that explains what changed and why.\n\ Mention testing only when the changes provide clear evidence; do not invent results or implementation details."; +const CODE_REVIEW_INSTRUCTION: &str = "Review the changed code for actionable defects. Trace how each change affects callers and related behavior visible in the supplied patches.\n\ +Prioritize correctness, data loss, security, concurrency, performance, and compatibility problems. Do not report style preferences or praise.\n\ +Every finding must identify a changed file. Use a line present on the selected diff side when possible; otherwise use null.\n\ +Reply with JSON only, no markdown fences: {\"findings\":[{\"path\":\"src/file.rs\",\"line\":42,\"side\":\"new\",\"severity\":\"high\",\"title\":\"Short issue\",\"body\":\"Why this can fail and when\"}]}.\n\ +Severity is critical, high, medium, or low. Return {\"findings\":[]} when there are no actionable defects."; + pub struct PromptBuild { pub text: String, pub manifest_files: usize, @@ -52,6 +58,10 @@ pub fn build_pull_request_prompt( ) } +pub fn build_code_review_prompt(diffs: &[FileDiff]) -> PromptBuild { + build(CODE_REVIEW_INSTRUCTION, None, diffs, &[], None) +} + fn build( instruction: &str, branches: Option<(&str, &str)>, @@ -296,6 +306,15 @@ mod tests { assert_eq!(built.omitted_patch_files, 4); } + #[test] + fn review_prompt_requires_structured_actionable_findings() { + let built = build_code_review_prompt(&[sample_diff("src/lib.rs", "+unsafe change")]); + assert!(built.text.contains("actionable defects")); + assert!(built.text.contains("\"findings\"")); + assert!(built.text.contains("critical, high, medium, or low")); + assert!(built.text.contains(" CmdResult< Ok(()) } +#[tauri::command(async)] +pub fn repo_branch_delete_at( + path: String, + name: String, + expected_target: String, +) -> CmdResult<()> { + Repo::discover(&path)?.delete_branch_at(&name, &expected_target)?; + Ok(()) +} + #[tauri::command(async)] pub fn repo_branch_rename(path: String, old_name: String, new_name: String) -> CmdResult<()> { Repo::discover(&path)?.rename_branch(&old_name, &new_name)?; @@ -1973,6 +1983,46 @@ pub async fn repo_suggest_pull_request( result } +#[tauri::command(async)] +#[allow(clippy::too_many_arguments)] +pub async fn repo_review_changes( + path: String, + baseline: Option, + provider: ai::AiProvider, + model: Option, + openai_cli: Option, + anthropic_cli: Option, + request: ai::AiGenerationRequest, + state: State<'_, AppState>, +) -> CmdResult> { + let cancel = ai::bin::AiCancelHandle::new(); + let op_id = Some(request.op_id.clone()); + register_op(&state, &op_id, OperationCancelHandle::Ai(cancel.clone())); + let result = run_blocking("ai review changes", move || { + let repo = Repo::discover(&path)?; + let diffs = match baseline.as_deref().map(str::trim).filter(|value| !value.is_empty()) { + Some(baseline) => repo.diff_since(baseline)?, + // Review inbox is HEAD → index + worktree, so staged files stay + // in scope and the provider sees exactly what the UI shows. + None => repo.diff_since("HEAD")?, + }; + let override_path = ai_cli_override(provider, openai_cli, anthropic_cli); + ai::review_changes_with_request( + provider, + repo.path(), + &diffs, + model.as_deref(), + override_path.as_deref(), + Some(&cancel), + &request.sensitive_decision, + ) + .map_err(CmdError::from_msg) + }) + .await; + deregister_op(&state, &op_id); + result +} + impl CmdError { fn from_msg(message: String) -> Self { Self { message } diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 25b28de..4071843 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -12,6 +12,79 @@ mod terminal; use tauri::Manager; +#[cfg(target_os = "windows")] +fn apply_windows_taskbar_icon() -> windows::core::Result { + use windows::{ + core::{BOOL, PCWSTR}, + Win32::{ + Foundation::{HINSTANCE, HWND, LPARAM, WPARAM}, + System::LibraryLoader::GetModuleHandleW, + UI::WindowsAndMessaging::{ + EnumWindows, GetSystemMetrics, GetWindowThreadProcessId, IsWindowVisible, + LoadImageW, SendMessageW, ICON_BIG, ICON_SMALL, IMAGE_ICON, LR_SHARED, SM_CXICON, + SM_CXSMICON, SM_CYICON, SM_CYSMICON, WM_SETICON, + }, + }, + }; + + unsafe extern "system" fn find_visible_process_window(hwnd: HWND, state: LPARAM) -> BOOL { + let mut process_id = 0; + unsafe { + GetWindowThreadProcessId(hwnd, Some(&mut process_id)); + if process_id == std::process::id() && IsWindowVisible(hwnd).as_bool() { + *(state.0 as *mut HWND) = hwnd; + } + } + true.into() + } + + // tauri-winres embeds icon.ico as resource 32512. Load shared handles from + // the running module so Windows owns their lifetime for the whole process. + const APP_ICON_RESOURCE_ID: usize = 32512; + unsafe { + let mut hwnd = HWND::default(); + EnumWindows( + Some(find_visible_process_window), + LPARAM((&mut hwnd as *mut HWND) as isize), + )?; + if hwnd.0.is_null() { + return Ok(false); + } + let module = GetModuleHandleW(None)?; + let instance = HINSTANCE(module.0); + let resource = PCWSTR(APP_ICON_RESOURCE_ID as *const u16); + let big = LoadImageW( + Some(instance), + resource, + IMAGE_ICON, + GetSystemMetrics(SM_CXICON), + GetSystemMetrics(SM_CYICON), + LR_SHARED, + )?; + let small = LoadImageW( + Some(instance), + resource, + IMAGE_ICON, + GetSystemMetrics(SM_CXSMICON), + GetSystemMetrics(SM_CYSMICON), + LR_SHARED, + )?; + SendMessageW( + hwnd, + WM_SETICON, + Some(WPARAM(ICON_BIG as usize)), + Some(LPARAM(big.0 as isize)), + ); + SendMessageW( + hwnd, + WM_SETICON, + Some(WPARAM(ICON_SMALL as usize)), + Some(LPARAM(small.0 as isize)), + ); + } + Ok(true) +} + /// Append Rust panics to a local crash log so alpha bug reports come with /// evidence. Local-only — nothing leaves the machine (PRD §10); opt-in /// remote crash reporting is separate future work. The previous hook still @@ -189,6 +262,7 @@ fn main() { commands::repo_worktree_copy_include, commands::repo_branch_create, commands::repo_branch_delete, + commands::repo_branch_delete_at, commands::repo_branch_rename, commands::repo_branch_set_upstream, commands::repo_branch_delete_remote, @@ -233,6 +307,7 @@ fn main() { commands::ai_provider_logout, commands::repo_suggest_commit_message, commands::repo_suggest_pull_request, + commands::repo_review_changes, commands::crash_report_check, ]) .setup(|app| { @@ -273,7 +348,21 @@ fn main() { .build(tauri::generate_context!()) .expect("error while building strand") .run(|app, event| { - if matches!(event, tauri::RunEvent::Exit | tauri::RunEvent::ExitRequested { .. }) { + #[cfg(target_os = "windows")] + if matches!(&event, tauri::RunEvent::Ready) { + // Assign both HWND icon handles after Tauri finishes restoring + // the window. Otherwise Windows falls back to its executable- + // path cache, which can go generic after an in-place update. + match apply_windows_taskbar_icon() { + Ok(true) => {} + Ok(false) => tracing::warn!("no visible Windows taskbar handle was found"), + Err(error) => tracing::warn!("failed to apply Windows taskbar icon: {error}"), + } + } + if matches!( + event, + tauri::RunEvent::Exit | tauri::RunEvent::ExitRequested { .. } + ) { app.state::().terminals.close_all(None); } }); diff --git a/crates/strand-tauri/src/pull_requests.rs b/crates/strand-tauri/src/pull_requests.rs index ac866bd..9f08edc 100644 --- a/crates/strand-tauri/src/pull_requests.rs +++ b/crates/strand-tauri/src/pull_requests.rs @@ -18,6 +18,7 @@ use serde_json::Value; use strand_azdo_protocol::{ DiffSide as AzdoDiffSide, MergeStrategy as AzdoMergeStrategy, Operation as AzdoOperation, PullRequestStatus as AzdoPullRequestStatus, ReviewVote as AzdoReviewVote, + ThreadStatus as AzdoThreadStatus, }; use strand_core::Repo; use uuid::Uuid; @@ -37,7 +38,7 @@ const MAX_AZURE_ITERATION_CHANGE_PAGES: u32 = 32; const GITHUB_BRANCH_STATE: &str = "open"; const AZURE_BRANCH_STATUS: &str = "active"; const GITHUB_LIST_FIELDS: &str = concat!( - "number,title,state,isDraft,author,headRefName,baseRefName,createdAt,updatedAt,", + "number,title,state,isDraft,author,headRefName,headRefOid,baseRefName,createdAt,updatedAt,", "closedAt,mergedAt,url,reviewDecision,additions,deletions,changedFiles" ); const GITHUB_DETAIL_FIELDS: &str = concat!( @@ -260,6 +261,12 @@ pub struct PullRequestReviewThreadUpdate { pub can_unresolve: bool, } +#[derive(Debug)] +struct AzureDiscussion { + comments: Vec, + review_threads: Vec, +} + #[derive(Debug, Clone, Serialize)] pub struct PullRequest { pub id: u64, @@ -629,9 +636,23 @@ pub fn reply_to_thread(path: &str, thread_id: &str, body: &str) -> Result reply_to_thread_github(path, thread_id, body), - HostRepo::Azure { .. } | HostRepo::AzureServer { .. } => Err( - "Azure DevOps review-thread replies are not available yet. Open this pull request on Azure DevOps to reply." - .to_string(), + HostRepo::Azure { + organization, + project, + repo, + } => reply_to_thread_azure(path, &organization, &project, &repo, thread_id, body), + HostRepo::AzureServer { + profile_id, + collection_url, + project, + repo, + } => reply_to_thread_azure_server( + profile_id, + &collection_url, + &project, + &repo, + thread_id, + body, ), } } @@ -645,9 +666,29 @@ pub fn set_thread_resolved( let (_, host) = host_for_path(path)?; match host { HostRepo::GitHub { .. } => set_thread_resolved_github(path, thread_id, resolved), - HostRepo::Azure { .. } | HostRepo::AzureServer { .. } => Err( - "Azure DevOps review-thread resolution is not available yet. Open this pull request on Azure DevOps to update the thread." - .to_string(), + HostRepo::Azure { + organization, + project, + repo, + } => set_thread_resolved_azure( + path, + &organization, + &project, + &repo, + thread_id, + resolved, + ), + HostRepo::AzureServer { + profile_id, + project, + repo, + .. + } => set_thread_resolved_azure_server( + profile_id, + &project, + &repo, + thread_id, + resolved, ), } } @@ -1692,8 +1733,10 @@ fn detail_azure_server( let checks = scope.spawn(|| azure_server_policies(profile_id, &project, &project_id, id)); (comments.join(), commits.join(), checks.join()) }); - pull_request.comments = + let discussion = comments.map_err(|_| "Azure DevOps Server discussion worker failed".to_string())??; + pull_request.comments = discussion.comments; + pull_request.review_threads = discussion.review_threads; pull_request.comment_count = pull_request.comments.len(); pull_request.commits = commits.map_err(|_| "Azure DevOps Server commit worker failed".to_string())??; @@ -1723,6 +1766,7 @@ fn activity_azure_server( let pull_request = parse_azure_pr(&value, &collection_url, &project, &repo, None) .ok_or_else(|| format!("Azure DevOps Server returned no data for PR #{id}"))?; let comments = azure_server_comments(profile_id, &collection_url, &project, &repo, id)? + .comments .into_iter() .map(|comment| PullRequestActivityComment { id: comment.id, @@ -1781,7 +1825,7 @@ fn azure_server_comments( project: &str, repo: &str, id: u64, -) -> Result> { +) -> Result { let value = server_execute( profile_id, AzdoOperation::Threads { @@ -1790,9 +1834,72 @@ fn azure_server_comments( id, }, )?; - Ok(parse_azure_comments( + Ok(parse_azure_discussion( &value, &azure_server_pr_url(collection_url, project, repo, id), + id, + )) +} + +fn reply_to_thread_azure_server( + profile_id: Uuid, + collection_url: &str, + project: &str, + repo: &str, + thread_id: &str, + body: &str, +) -> Result { + let (pull_request_id, azure_thread_id, parent_comment_id) = + parse_azure_thread_id(thread_id)?; + let value = server_execute( + profile_id, + AzdoOperation::ReplyToThread { + project: project.into(), + repository: repo.into(), + id: pull_request_id, + thread_id: azure_thread_id, + parent_comment_id, + body: body.into(), + }, + )?; + parse_azure_comment( + &value, + azure_thread_id, + &azure_server_pr_url(collection_url, project, repo, pull_request_id), + None, + ) + .ok_or_else(|| "Azure DevOps Server returned no usable review-thread reply".into()) +} + +fn set_thread_resolved_azure_server( + profile_id: Uuid, + project: &str, + repo: &str, + thread_id: &str, + resolved: bool, +) -> Result { + let (pull_request_id, azure_thread_id, parent_comment_id) = + parse_azure_thread_id(thread_id)?; + let value = server_execute( + profile_id, + AzdoOperation::SetThreadStatus { + project: project.into(), + repository: repo.into(), + id: pull_request_id, + thread_id: azure_thread_id, + status: if resolved { + AzdoThreadStatus::Fixed + } else { + AzdoThreadStatus::Active + }, + }, + )?; + Ok(parse_azure_thread_update( + &value, + pull_request_id, + azure_thread_id, + parent_comment_id, + resolved, )) } @@ -2300,8 +2407,9 @@ fn detail_azure( let checks = scope.spawn(|| azure_policies(cwd, &organization, id)); (comments.join(), commits.join(), checks.join()) }); - pull_request.comments = - comments.map_err(|_| "Azure discussion query worker failed".to_string())??; + let discussion = comments.map_err(|_| "Azure discussion query worker failed".to_string())??; + pull_request.comments = discussion.comments; + pull_request.review_threads = discussion.review_threads; pull_request.comment_count = pull_request.comments.len(); pull_request.commits = commits.map_err(|_| "Azure commit query worker failed".to_string())??; @@ -2331,6 +2439,7 @@ fn activity_azure( let pull_request = parse_azure_pr(&value, &organization, &project, &repo, None) .ok_or_else(|| format!("Azure CLI returned no data for PR #{id}"))?; let comments = azure_comments(cwd, &organization, &project, &repo, id)? + .comments .into_iter() .map(|comment| PullRequestActivityComment { id: comment.id, @@ -2400,7 +2509,7 @@ fn azure_comments( project: &str, repo: &str, id: u64, -) -> Result> { +) -> Result { let organization_url = format!("https://dev.azure.com/{organization}/"); let id_text = id.to_string(); let project_arg = format!("project={project}"); @@ -2432,11 +2541,12 @@ fn azure_comments( )?; let value: Value = serde_json::from_slice(&output) .map_err(|error| format!("Azure CLI returned invalid discussion JSON: {error}"))?; - Ok(parse_azure_comments( + Ok(parse_azure_discussion( &value, &format!( "https://dev.azure.com/{organization}/{project}/_git/{repo}/pullrequest/{id_text}" ), + id, )) } @@ -2550,6 +2660,134 @@ fn azure_invoke_json( .map_err(|error| format!("Azure CLI returned invalid {resource} JSON: {error}")) } +fn azure_invoke_write_json( + cwd: &str, + organization: &str, + resource: &str, + route_parameters: &[String], + method: &str, + payload: &Value, +) -> Result { + let mut request = tempfile::NamedTempFile::new() + .map_err(|error| format!("Could not prepare Azure {resource} request: {error}"))?; + serde_json::to_writer(&mut request, payload) + .map_err(|error| format!("Could not encode Azure {resource} request: {error}"))?; + request + .flush() + .map_err(|error| format!("Could not prepare Azure {resource} request: {error}"))?; + let request_path = request + .path() + .to_str() + .ok_or_else(|| format!("Azure {resource} request path is not valid UTF-8"))?; + let organization_url = format!("https://dev.azure.com/{organization}/"); + let mut args = vec![ + "devops".to_string(), + "invoke".to_string(), + "--area".to_string(), + "git".to_string(), + "--resource".to_string(), + resource.to_string(), + "--route-parameters".to_string(), + ]; + args.extend_from_slice(route_parameters); + args.extend([ + "--organization".into(), + organization_url, + "--api-version".into(), + "7.1".into(), + "--http-method".into(), + method.into(), + "--in-file".into(), + request_path.into(), + "--media-type".into(), + "application/json".into(), + "--output".into(), + "json".into(), + "--only-show-errors".into(), + ]); + let arg_refs = args.iter().map(String::as_str).collect::>(); + let output = run_command( + cwd, + "az", + &arg_refs, + &[("AZURE_EXTENSION_USE_DYNAMIC_INSTALL", "no")], + )?; + serde_json::from_slice(&output) + .map_err(|error| format!("Azure CLI returned invalid {resource} JSON: {error}")) +} + +fn reply_to_thread_azure( + cwd: &str, + organization: &str, + project: &str, + repo: &str, + thread_id: &str, + body: &str, +) -> Result { + let (pull_request_id, azure_thread_id, parent_comment_id) = + parse_azure_thread_id(thread_id)?; + let routes = vec![ + format!("project={project}"), + format!("repositoryId={repo}"), + format!("pullRequestId={pull_request_id}"), + format!("threadId={azure_thread_id}"), + ]; + let value = azure_invoke_write_json( + cwd, + organization, + "pullRequestThreadComments", + &routes, + "POST", + &serde_json::json!({ + "parentCommentId": parent_comment_id, + "content": body, + "commentType": 1 + }), + )?; + parse_azure_comment( + &value, + azure_thread_id, + &format!( + "https://dev.azure.com/{organization}/{project}/_git/{repo}/pullrequest/{pull_request_id}" + ), + None, + ) + .ok_or_else(|| "Azure DevOps returned no usable review-thread reply".into()) +} + +fn set_thread_resolved_azure( + cwd: &str, + organization: &str, + project: &str, + repo: &str, + thread_id: &str, + resolved: bool, +) -> Result { + let (pull_request_id, azure_thread_id, parent_comment_id) = + parse_azure_thread_id(thread_id)?; + let routes = vec![ + format!("project={project}"), + format!("repositoryId={repo}"), + format!("pullRequestId={pull_request_id}"), + format!("threadId={azure_thread_id}"), + ]; + let value = azure_invoke_write_json( + cwd, + organization, + "pullRequestThreads", + &routes, + "PATCH", + &serde_json::json!({ "status": if resolved { 2 } else { 1 } }), + )?; + Ok(parse_azure_thread_update( + &value, + pull_request_id, + azure_thread_id, + parent_comment_id, + resolved, + )) +} + fn azure_review_coordinates( cwd: &str, organization: &str, @@ -3349,6 +3587,41 @@ fn validate_thread_id(thread_id: &str) -> Result<()> { Ok(()) } +fn azure_thread_id(pull_request_id: u64, thread_id: u64, parent_comment_id: u64) -> String { + format!("azure:{pull_request_id}:{thread_id}:{parent_comment_id}") +} + +fn parse_azure_thread_id(thread_id: &str) -> Result<(u64, u64, u64)> { + let mut parts = thread_id.split(':'); + if parts.next() != Some("azure") { + return Err("Azure review thread is invalid; refresh the pull request and try again".into()); + } + let pull_request_id = parts + .next() + .and_then(|value| value.parse::().ok()) + .filter(|value| *value > 0); + let azure_thread_id = parts + .next() + .and_then(|value| value.parse::().ok()) + .filter(|value| *value > 0); + let parent_comment_id = parts + .next() + .and_then(|value| value.parse::().ok()) + .filter(|value| *value > 0); + if parts.next().is_some() + || pull_request_id.is_none() + || azure_thread_id.is_none() + || parent_comment_id.is_none() + { + return Err("Azure review thread is invalid; refresh the pull request and try again".into()); + } + Ok(( + pull_request_id.unwrap(), + azure_thread_id.unwrap(), + parent_comment_id.unwrap(), + )) +} + fn validate_review_id(review_id: &str) -> Result<()> { if review_id.trim().is_empty() || review_id.len() > MAX_THREAD_ID_BYTES @@ -4171,14 +4444,50 @@ fn parse_azure_commits(value: &Value) -> Vec { .collect() } -fn parse_azure_comments(value: &Value, pr_url: &str) -> Vec { - let threads = value +fn azure_threads(value: &Value) -> &[Value] { + value .get("value") .and_then(Value::as_array) .or_else(|| value.as_array()) .map(Vec::as_slice) - .unwrap_or(&[]); - threads + .unwrap_or(&[]) +} + +fn parse_azure_comment( + value: &Value, + thread_id: u64, + pr_url: &str, + path: Option, +) -> Option { + let comment = value.get("value").filter(|value| value.is_object()).unwrap_or(value); + if comment + .get("isDeleted") + .and_then(Value::as_bool) + .unwrap_or(false) + { + return None; + } + let body = text(comment.get("content")).unwrap_or_default(); + if body.trim().is_empty() { + return None; + } + let comment_id = comment.get("id").and_then(Value::as_u64)?; + Some(PullRequestComment { + id: format!("{thread_id}:{comment_id}"), + author: text(comment.pointer("/author/displayName")).unwrap_or_else(|| "unknown".into()), + avatar_url: text(comment.pointer("/author/imageUrl")) + .or_else(|| text(comment.pointer("/author/_links/avatar/href"))), + body, + created_at: text(comment.get("publishedDate")).unwrap_or_default(), + url: pr_url.to_string(), + is_system: text(comment.get("commentType")) + .is_some_and(|kind| kind.eq_ignore_ascii_case("system")), + path, + }) +} + +fn parse_azure_comments(value: &Value, pr_url: &str) -> Vec { + azure_threads(value) .iter() .filter(|thread| { !thread @@ -4189,37 +4498,116 @@ fn parse_azure_comments(value: &Value, pr_url: &str) -> Vec .flat_map(|thread| { let thread_id = thread.get("id").and_then(Value::as_u64).unwrap_or(0); let path = text(thread.pointer("/threadContext/filePath")); - array(thread, "comments").iter().filter_map(move |comment| { - if comment - .get("isDeleted") - .and_then(Value::as_bool) - .unwrap_or(false) - { - return None; - } - let body = text(comment.get("content")).unwrap_or_default(); - if body.trim().is_empty() { - return None; - } - let comment_id = comment.get("id").and_then(Value::as_u64).unwrap_or(0); - Some(PullRequestComment { - id: format!("{thread_id}:{comment_id}"), - author: text(comment.pointer("/author/displayName")) - .unwrap_or_else(|| "unknown".into()), - avatar_url: text(comment.pointer("/author/imageUrl")) - .or_else(|| text(comment.pointer("/author/_links/avatar/href"))), - body, - created_at: text(comment.get("publishedDate")).unwrap_or_default(), - url: pr_url.to_string(), - is_system: text(comment.get("commentType")) - .is_some_and(|kind| kind.eq_ignore_ascii_case("system")), - path: path.clone(), + array(thread, "comments") + .iter() + .filter_map(move |comment| parse_azure_comment(comment, thread_id, pr_url, path.clone())) + }) + .collect() +} + +fn azure_thread_resolved(value: &Value, fallback: bool) -> bool { + match value.get("status") { + Some(Value::Number(status)) => match status.as_u64() { + Some(1 | 6) => false, + Some(2..=5) => true, + _ => fallback, + }, + Some(Value::String(status)) => match status.to_ascii_lowercase().as_str() { + "active" | "pending" => false, + "fixed" | "wontfix" | "closed" | "bydesign" => true, + _ => fallback, + }, + _ => fallback, + } +} + +fn parse_azure_review_threads( + value: &Value, + pr_url: &str, + pull_request_id: u64, +) -> Vec { + azure_threads(value) + .iter() + .filter(|thread| { + !thread + .get("isDeleted") + .and_then(Value::as_bool) + .unwrap_or(false) + }) + .filter_map(|thread| { + let thread_id = thread.get("id").and_then(Value::as_u64)?; + let path = text(thread.pointer("/threadContext/filePath"))? + .trim_start_matches('/') + .to_string(); + if path.is_empty() { + return None; + } + let right_start = thread.pointer("/threadContext/rightFileStart/line").and_then(Value::as_u64); + let right_end = thread.pointer("/threadContext/rightFileEnd/line").and_then(Value::as_u64); + let left_start = thread.pointer("/threadContext/leftFileStart/line").and_then(Value::as_u64); + let left_end = thread.pointer("/threadContext/leftFileEnd/line").and_then(Value::as_u64); + let (start_line, end_line, side) = if right_start.is_some() || right_end.is_some() { + let end = right_end.or(right_start)?; + (right_start.unwrap_or(end), end, PullRequestDiffSide::Additions) + } else { + let end = left_end.or(left_start)?; + (left_start.unwrap_or(end), end, PullRequestDiffSide::Deletions) + }; + let comments = array(thread, "comments") + .iter() + .filter_map(|comment| { + parse_azure_comment(comment, thread_id, pr_url, Some(path.clone())) }) + .collect::>(); + if comments.is_empty() { + return None; + } + let parent_comment_id = array(thread, "comments") + .iter() + .find_map(|comment| comment.get("id").and_then(Value::as_u64))?; + let is_resolved = azure_thread_resolved(thread, false); + Some(PullRequestReviewThread { + id: azure_thread_id(pull_request_id, thread_id, parent_comment_id), + path, + start_line: u32::try_from(start_line).ok()?, + end_line: u32::try_from(end_line).ok()?, + side, + is_resolved, + is_outdated: false, + can_reply: true, + can_resolve: !is_resolved, + can_unresolve: is_resolved, + comments, }) }) .collect() } +fn parse_azure_thread_update( + value: &Value, + pull_request_id: u64, + thread_id: u64, + parent_comment_id: u64, + resolved: bool, +) -> PullRequestReviewThreadUpdate { + let is_resolved = azure_thread_resolved(value, resolved); + PullRequestReviewThreadUpdate { + id: azure_thread_id(pull_request_id, thread_id, parent_comment_id), + is_resolved, + is_outdated: false, + can_reply: true, + can_resolve: !is_resolved, + can_unresolve: is_resolved, + } +} + +fn parse_azure_discussion(value: &Value, pr_url: &str, pull_request_id: u64) -> AzureDiscussion { + AzureDiscussion { + comments: parse_azure_comments(value, pr_url), + review_threads: parse_azure_review_threads(value, pr_url, pull_request_id), + } +} + fn array<'a>(value: &'a Value, key: &str) -> &'a [Value] { value .get(key) @@ -4622,6 +5010,7 @@ mod tests { "changedFiles", "closedAt", "mergedAt", + "headRefOid", ] { assert!(GITHUB_LIST_FIELDS.contains(field)); } @@ -4837,6 +5226,64 @@ mod tests { assert!(comments[1].is_system); } + #[test] + fn normalizes_azure_review_threads_with_replies_ranges_and_statuses() { + let value = serde_json::json!({ "value": [ + { + "id": 9, + "status": "active", + "threadContext": { + "filePath": "/src/lib.rs", + "rightFileStart": { "line": 27, "offset": 1 }, + "rightFileEnd": { "line": 29, "offset": 1 } + }, + "comments": [ + { "id": 1, "content": "Please validate this.", "commentType": "text", + "publishedDate": "2026-07-13T12:00:00Z", "author": { "displayName": "Octo" } }, + { "id": 2, "content": "Fixed.", "commentType": "text", + "publishedDate": "2026-07-13T12:05:00Z", "author": { "displayName": "Ada" } } + ] + }, + { + "id": 10, + "status": 2, + "threadContext": { + "filePath": "/src/old.rs", + "leftFileStart": { "line": 4, "offset": 1 }, + "leftFileEnd": { "line": 4, "offset": 1 } + }, + "comments": [ + { "id": 1, "content": "Why remove this?", "commentType": "text", + "author": { "displayName": "Grace" } } + ] + }, + { "id": 11, "status": "active", "comments": [ + { "id": 1, "content": "General comment", "commentType": "text", + "author": { "displayName": "Linus" } } + ] } + ] }); + + let discussion = parse_azure_discussion(&value, "https://dev.azure.com/acme/pr/7", 7); + assert_eq!(discussion.comments.len(), 4); + assert_eq!(discussion.review_threads.len(), 2); + let added = &discussion.review_threads[0]; + assert_eq!(added.id, "azure:7:9:1"); + assert_eq!(added.path, "src/lib.rs"); + assert_eq!((added.start_line, added.end_line), (27, 29)); + assert_eq!(added.side, PullRequestDiffSide::Additions); + assert!(!added.is_resolved); + assert!(added.can_reply); + assert!(added.can_resolve); + assert!(!added.can_unresolve); + assert_eq!(added.comments[1].author, "Ada"); + assert_eq!(added.comments[0].path.as_deref(), Some("src/lib.rs")); + let deleted = &discussion.review_threads[1]; + assert_eq!(deleted.side, PullRequestDiffSide::Deletions); + assert!(deleted.is_resolved); + assert!(!deleted.can_resolve); + assert!(deleted.can_unresolve); + } + #[test] fn normalizes_github_review_threads_with_replies_and_ranges() { let value = serde_json::json!({ @@ -4969,37 +5416,37 @@ mod tests { } #[test] - fn validates_thread_ids_and_rejects_azure_thread_writes() { + fn validates_provider_thread_ids() { assert!(validate_thread_id("PRRT_kwDOExample").is_ok()); assert!(validate_thread_id(" ").is_err()); assert!(validate_thread_id("bad\nid").is_err()); assert!(validate_thread_id(&"x".repeat(MAX_THREAD_ID_BYTES + 1)).is_err()); + assert_eq!(azure_thread_id(7, 9, 1), "azure:7:9:1"); + assert_eq!( + parse_azure_thread_id("azure:7:9:1").unwrap(), + (7, 9, 1) + ); + for invalid in [ + "thread-1", + "azure:7:9", + "azure:0:9:1", + "azure:7:0:1", + "azure:7:9:0", + "azure:7:9:1:10", + ] { + assert!(parse_azure_thread_id(invalid).is_err()); + } - let dir = std::env::temp_dir().join(format!( - "strand-pr-thread-provider-{}-{:?}", - std::process::id(), - std::thread::current().id() - )); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).unwrap(); - git(&dir, &["init", "-q"]); - git( - &dir, - &[ - "remote", - "add", - "origin", - "https://dev.azure.com/acme/project/_git/repo", - ], + let resolved = parse_azure_thread_update( + &serde_json::json!({ "id": 9, "status": "fixed" }), + 7, + 9, + 1, + true, ); - let path = dir.to_str().unwrap(); - assert!(reply_to_thread(path, "thread-1", "Reply") - .unwrap_err() - .contains("not available")); - assert!(set_thread_resolved(path, "thread-1", true) - .unwrap_err() - .contains("not available")); - let _ = std::fs::remove_dir_all(dir); + assert_eq!(resolved.id, "azure:7:9:1"); + assert!(resolved.is_resolved); + assert!(resolved.can_unresolve); } #[test] diff --git a/docs/learnings.md b/docs/learnings.md index ab114e0..d2c0262 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -1577,8 +1577,9 @@ also produce false negatives for older merged branches. **How to apply.** Compute the flag while collecting refs with libgit2 commit ancestry (`graph_descendant_of`, including equal tips), and refresh it through -the existing snapshot path after history or checkout operations. Squash merges -do not preserve ancestry and therefore do not receive this mark. +the existing snapshot path after history or checkout operations. Squash and +rebase merges do not preserve ancestry and therefore do not change this core +field; provider-aware UI may add the exact-tip overlay described below. **Bulk cleanup extension (2026-07-17).** `RemoteBranch.merged` applies the same containment test to remote-tracking tips, but does not suppress the primary ref @@ -1591,6 +1592,18 @@ after its local branch was merged. Local cleanup calls worktree occupancy at deletion time; never rely on a frozen dialog snapshot as the final destructive-operation guard. +**Provider exact-tip extension (2026-08-07, DAN-41).** The sidebar, commit +graph, and bulk cleanup may additionally mark a non-current local branch when a +completed GitHub/Azure PR targets the primary branch, the PR source name matches +the local branch, and its recorded source SHA exactly equals the current local +tip. This is the safe signal for squash/rebase merges: never infer them from +tree equality, subjects, PR numbers, or ahead/behind. Keep discovery async, +delayed, deduplicated, and session-cached off repo-open/ref-snapshot hot paths; +explicit cleanup refreshes once and freezes its display plan. Remote deletion +remains ancestry-only. Local provider-confirmed deletion must call +`Repo::delete_branch_at`, which rejects a moved tip and any current/worktree- +held branch at execution time. + --- ## Windows CLI launch uses persisted PATH and cmd-compatible canonical paths @@ -1765,6 +1778,11 @@ history. When adding a new Azure DevOps Server write operation, bump the helper protocol so an already-installed older helper is upgraded rather than selected by a matching but incomplete capability contract. +Azure thread IDs are scoped to a pull request rather than globally unique, and +a reply also needs the thread's root comment ID. Normalize all three as one +provider-prefixed ID in shared UI state, and decode it only at the provider +boundary; the visible thread ID alone cannot build either Azure write route. + **Hosted viewed marks fingerprint the rendered file, not the provider patch (2026-07-18).** A provider/Pierre cache key can identify the whole PR patch, so using it for one file makes every unrelated push invalidate all review @@ -1832,6 +1850,16 @@ Windows decorations/shadow and show it. Verify persistence by changing state, closing the process through the UI, and launching the exact workspace binary. A same-process hide/show or app-identity launch does not prove disk restore. +**A Windows executable icon is not a retained taskbar icon (2026-08-07, +DAN-40).** Extracting a valid `icon.ico` from `strand.exe` proves the resource, +not the visible HWND state. After an in-place update, Windows' executable-path +cache can fall back to a generic document icon when `WM_GETICON` returns no big +or small handle. At `RunEvent::Ready`, enumerate the current process's visible +top-level window, load shared big/small handles from winres resource `32512`, +and send both `WM_SETICON` messages. Verify the real visible HWND in a fresh +process: `ICON_BIG` and `ICON_SMALL` must be non-zero and extract to the Strand +artwork. Keep this contract in `scripts/check-release-security.mjs`. + **Animated notifications need one stable accessibility channel (2026-07-18).** Keep visible success/error/network pills `aria-hidden` and mirror the active message through the always-mounted assertive live region in `ToastViewport`; @@ -2140,3 +2168,35 @@ manifest logo, producing blur and an accent-color backplate. Generate exact `lightunplated` variants directly from canonical `strand.png`, copy every variant into the MSIX, and keep the cross-platform policy check aligned with that matrix. + +**Pierre worker themes are global light/dark pairs (2026-08-07).** Use the +official palette names already registered by `@pierre/diffs`, initialize the +pool with both the light and dark member, and change families through +`pool.setRenderOptions`. A mounted diff's `themeType` should only select which +already-highlighted palette to display, so ordinary Strand light/dark toggles +do not discard and recompute highlighted ASTs. Do not register those official +names again: duplicate registration logs errors and makes Vite emit a second +copy of every palette chunk. + +**AI review results must stay bound to the submitted diff (2026-08-07).** Key +each request by the review baseline plus per-file patch hashes and cancel or +ignore it when that key changes. Normalize model paths against the submitted +`FileDiff`s on the Rust side, drop unknown files, and retain a line anchor only +when the patch proves that line exists on the reported old/new side; a valid +finding with a bad line becomes a file note. Keep provider findings transient: +AI review must not edit files or persist notes until the reviewer explicitly +accepts an individual finding or the pending set. Append accepted findings in +one write without replacing human or previously accepted AI feedback. This +keeps hallucinated or stale coordinates out of the diff and avoids N IndexedDB +writes for N findings. + +## Vite optimizer cache must follow pnpm dependency upgrades (2026-08-07) + +Vite's generated `ui/node_modules/.vite/deps/_metadata.json` can survive a +`pnpm install` while still pointing at a package-version directory pnpm has +removed. The failure is an ENOENT during dependency optimization, before Vite +can reliably invalidate its own cache. The pre-dev/build +`ui/scripts/clean-stale-js.mjs` check therefore validates every recorded +optimizer `src` path and removes only `ui/node_modules/.vite` when one is +missing. Keep this check generic; do not special-case a package or delete the +whole dependency installation. diff --git a/docs/packaging.md b/docs/packaging.md index e1df085..59a703a 100644 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -53,6 +53,14 @@ locally instead: `crates/strand-tauri/icons/`, matching the paths already listed in `tauri.conf.json`. +On Windows, the executable resource is also the runtime taskbar source. +`main.rs` loads `icon.ico` resource `32512` into explicit big and small HWND +icons at `RunEvent::Ready`; this avoids relying on the executable-path icon +cache, which can retain a generic icon after the updater replaces the binary in +place. `scripts/check-release-security.mjs` fails if that runtime contract is +removed. Verify a candidate with `WM_GETICON` for both `ICON_BIG` and +`ICON_SMALL` after launch, not only by extracting the executable resource. + ## 2. Apple Developer ID signing 1. Enroll in the Apple Developer Program; create a **Developer ID @@ -193,7 +201,8 @@ manifest to a draft versioned helper release, publishes it as a prerelease, and then promotes the identical artifacts to `strand-azdo-protocol-N`. Strand constructs that channel from its compiled protocol version, so publishing protocol N+1 cannot break reinstall -for an older Strand release. Protocol 5 is additionally promoted to the legacy +for an older Strand release. The current thread-lifecycle contract is protocol +6. Protocol 5 is additionally promoted to the legacy `strand-azdo-latest` channel used by already-published Strand 1.2 clients. A post-promotion Linux smoke job downloads through the protocol channel, executes the published helper, and rechecks its version, protocol, archive/binary hashes, diff --git a/docs/perf-baseline.md b/docs/perf-baseline.md index 944df0e..5da5c18 100644 --- a/docs/perf-baseline.md +++ b/docs/perf-baseline.md @@ -253,6 +253,13 @@ helper processes (browser / GPU / renderer / network / utility + the diff highlight workers). JS heap is tiny (7 MB), so the overage is structural to WebView2's multi-process model, not app allocation. +**DAN-30 follow-up (2026-08-07):** A Task Manager screenshot reported 326.5MB +for the WebView2 process group: 203.6MB GPU, 99.2MB renderer, and the remainder +across manager/network/storage/crashpad processes. Task Manager's Memory column +is working set, so it is not comparable to the PRD's private-byte guardrail; +it is also below this baseline's 408MB empty-shell working set. No Strand-owned +growth or supported process-count optimization was identified. + ## Verdict vs PRD §8 (webview/app targets) | target | result | status | diff --git a/scripts/azdo-helper-manifest.test.mjs b/scripts/azdo-helper-manifest.test.mjs index 2bc2800..61da576 100644 --- a/scripts/azdo-helper-manifest.test.mjs +++ b/scripts/azdo-helper-manifest.test.mjs @@ -18,7 +18,7 @@ async function fixture(overrides = {}) { path.join(directory, `${target}.metadata.json`), JSON.stringify({ helper_version: '1.2.1', - protocol_version: 5, + protocol_version: 6, target, name: `strand-azdo-1.2.1-${target}.${target.includes('linux') ? 'tar.gz' : 'zip'}`, archive_sha256: `${index}`.repeat(64), @@ -37,7 +37,7 @@ test('manifest derives one helper and protocol version from build metadata', asy await buildManifest(directory, output); const manifest = JSON.parse(await readFile(output, 'utf8')); assert.equal(manifest.strand_version, '1.2.1'); - assert.equal(manifest.protocol_version, 5); + assert.equal(manifest.protocol_version, 6); assert.equal(manifest.assets.length, 3); assert.ok(manifest.assets.every((asset) => !('helper_version' in asset))); assert.ok(manifest.assets.every((asset) => !('protocol_version' in asset))); diff --git a/scripts/check-release-security.mjs b/scripts/check-release-security.mjs index 523f19a..92ece41 100644 --- a/scripts/check-release-security.mjs +++ b/scripts/check-release-security.mjs @@ -5,6 +5,7 @@ const capability = JSON.parse( readFileSync('crates/strand-tauri/capabilities/default.json', 'utf8'), ); const releaseWorkflow = readFileSync('.github/workflows/release.yml', 'utf8'); +const tauriMain = readFileSync('crates/strand-tauri/src/main.rs', 'utf8'); function fail(message) { throw new Error(`release security check failed: ${message}`); @@ -78,6 +79,17 @@ try { if (!publicKey.includes('minisign public key: 84FCBFD2A981CE5D')) { fail('updater public key is missing or unexpected'); } +for (const fragment of [ + 'const APP_ICON_RESOURCE_ID: usize = 32512', + 'RunEvent::Ready', + 'WM_SETICON', + 'ICON_BIG', + 'ICON_SMALL', +]) { + if (!tauriMain.includes(fragment)) { + fail(`Windows updater-safe taskbar icon contract is missing: ${fragment}`); + } +} const tagCheckout = 'ref: ${{ github.event.inputs.tag || github.ref }}'; if (releaseWorkflow.split(tagCheckout).length - 1 < 3) { @@ -109,4 +121,4 @@ for (const fragment of [ } } -console.log('Release CSP, capabilities, signed updater/helper channels, tag checkout, and Linux Sigstore policies are valid.'); +console.log('Release CSP, capabilities, updater-safe Windows icon, signed updater/helper channels, tag checkout, and Linux Sigstore policies are valid.'); diff --git a/ui/scripts/clean-stale-js.mjs b/ui/scripts/clean-stale-js.mjs index f3b092c..70fbdab 100644 --- a/ui/scripts/clean-stale-js.mjs +++ b/ui/scripts/clean-stale-js.mjs @@ -1,5 +1,6 @@ -// Remove stale compiled `.js` / `.js.map` files that sit next to their -// `.ts` / `.tsx` source under `ui/src`. +// Remove stale generated frontend artifacts before Vite starts: +// - compiled `.js` / `.js.map` files beside `.ts` / `.tsx` source; +// - optimizer metadata that points at a dependency pnpm has removed. // // Why this exists: Vite resolves `.js` before `.tsx`, so a leftover compiled // `.js` (emitted by an older `tsc` run, before tsconfig set `noEmit`) shadows @@ -7,14 +8,19 @@ // has `noEmit: true` so these should not regenerate — this is a safety net that // runs before `dev`/`build`, and can be invoked directly via `pnpm clean:js`. // -// Only files with a matching `.ts`/`.tsx` sibling are removed, so any -// genuinely hand-authored `.js` is left untouched. +// Source cleanup only removes files with a matching `.ts`/`.tsx` sibling, so +// genuinely hand-authored `.js` is untouched. Optimizer recovery removes only +// Vite's generated `ui/node_modules/.vite` cache. -import { readdirSync, statSync, existsSync, rmSync } from 'node:fs'; +import { readdirSync, statSync, existsSync, readFileSync, rmSync } from 'node:fs'; import { join, resolve, dirname, relative } from 'node:path'; import { fileURLToPath } from 'node:url'; -const srcDir = resolve(dirname(fileURLToPath(import.meta.url)), '..', 'src'); +const uiDir = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const srcDir = join(uiDir, 'src'); +const viteCacheDir = join(uiDir, 'node_modules', '.vite'); +const viteDepsDir = join(viteCacheDir, 'deps'); +const viteMetadata = join(viteDepsDir, '_metadata.json'); /** Does a `.ts`/`.tsx` source exist for this emitted `.js`/`.js.map` artifact? */ function hasSource(fullPath) { @@ -42,8 +48,36 @@ function walk(dir) { } if (existsSync(srcDir)) walk(srcDir); + +// A dependency upgrade can leave Vite's optimizer metadata pointing at a +// removed pnpm package directory. Vite then fails before it can invalidate the +// cache itself (ENOENT while reading the old dependency entry). Remove only +// the generated optimizer cache when one of its recorded source files is gone. +let removedViteCache = false; +if (existsSync(viteMetadata)) { + try { + const metadata = JSON.parse(readFileSync(viteMetadata, 'utf8')); + const dependencies = [ + ...Object.values(metadata.optimized ?? {}), + ...Object.values(metadata.discovered ?? {}), + ]; + const stale = dependencies.some((dependency) => + typeof dependency?.src === 'string' + && !existsSync(resolve(viteDepsDir, dependency.src))); + if (stale) { + rmSync(viteCacheDir, { recursive: true, force: true }); + removedViteCache = true; + } + } catch (error) { + console.warn(`clean-stale-js: could not inspect Vite cache: ${error.message}`); + } +} + console.log( removed > 0 ? `clean-stale-js: removed ${removed} stale compiled file(s) from ui/src` : 'clean-stale-js: nothing to clean', ); +if (removedViteCache) { + console.log('clean-stale-js: removed stale Vite dependency cache'); +} diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 1dab91d..00db94e 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -232,6 +232,7 @@ export function App() { const pruneWorktrees = useRepo((s) => s.pruneWorktrees); const baseline = useRepo((s) => s.baseline); const setBaseline = useRepo((s) => s.setBaseline); + const setBranchBaseline = useRepo((s) => s.setBranchBaseline); const clearBaseline = useRepo((s) => s.clearBaseline); const stageReviewed = useRepo((s) => s.stageReviewed); const clearReviewNotes = useRepo((s) => s.clearReviewNotes); @@ -1487,8 +1488,14 @@ export function App() { requestDiffSearch(); } }, { id: 'suggest-commit', label: 'Suggest commit message', group: 'Actions', shortcut: keyHint('suggest-commit'), keywords: 'ai generate commit message chatgpt codex claude suggest', run: () => { requestSuggestCommitMessage(); } }, - { id: 'review-baseline', label: baseline ? `Review: move baseline to HEAD (now at ${baseline.short})` : 'Review: pin baseline at HEAD', group: 'Actions', keywords: 'ai agent session since diff review baseline', run: () => { - void setBaseline().then(() => { setView('review'); selectFile(null); }) + { id: 'review-with-ai', label: 'Review changes with AI…', group: 'Actions', keywords: 'ai codex claude inspect code bugs issues findings', run: () => { + setView('review'); + selectFile(null); + window.setTimeout(() => window.dispatchEvent(new CustomEvent('strand:review-with-ai')), 50); + } }, + { id: 'review-baseline', label: baseline ? `Review: move baseline to HEAD (now at ${baseline.short})` : 'Review: start at branch fork point', group: 'Actions', keywords: 'ai agent session since diff review baseline branch creation fork point', run: () => { + const action = baseline ? setBaseline() : setBranchBaseline(); + void action.then(() => { setView('review'); selectFile(null); }) .catch((e) => showToast(`Set baseline failed: ${errMessage(e)}`, 'error')); } }, ...(baseline ? [{ id: 'review-clear', label: 'Review: clear baseline', group: 'Actions', keywords: 'ai agent session review baseline', run: () => { void clearBaseline(); } } satisfies PaletteAction] : []), @@ -1502,7 +1509,7 @@ export function App() { const st = useRepo.getState(); const pool = st.baseline ? st.baselineDiffs : st.reviewUnstagedDiffs; // Union: pool files with notes + noted paths outside the pool - // (staged away, or Review never populated the pool this session) — + // (committed/reverted away, or Review never populated the pool) — // a stored note always exports, just without an excerpt. const files = collectFeedbackFiles(pool, st.reviewNotes); if (!st.activePath || files.length === 0) { @@ -1702,7 +1709,7 @@ export function App() { showToast, meta, abortOperation, requestCommitSearch, requestDiffSearch, requestSuggestCommitMessage, requestSelectSinceBaseline, openInEditor, openInTerminal, openSettingsAt, repoActions, setRebaseDialog, setRemoteDialog, setRenameBranchDialog, - baseline, setBaseline, clearBaseline, stageReviewed, commits, resetTo, + baseline, setBaseline, setBranchBaseline, clearBaseline, stageReviewed, commits, resetTo, unstagedCount, stagedCount, baselineDiffCount, copyDiffs, reviewNoteCount, clearReviewNotes, keyHint, platform, cycleTab, view, workspaces, activeWorkspaceId, importCodeWorkspaceFlow, pruneWorktrees, diff --git a/ui/src/components/Diff.tsx b/ui/src/components/Diff.tsx index 15c5f80..9dff43d 100644 --- a/ui/src/components/Diff.tsx +++ b/ui/src/components/Diff.tsx @@ -104,6 +104,7 @@ export function Diff({ // `disableBackground` keeps the surface on our tokens; the theme drives the // syntax colors, which need to flip with light/dark. const resolvedTheme = useSettings((s) => s.resolvedTheme); + const diffSyntaxTheme = useSettings((s) => s.diffSyntaxTheme); const diffIndicators = useSettings((s) => s.diffIndicators); const diffLineNumbers = useSettings((s) => s.diffLineNumbers); const diffWordHighlight = useSettings((s) => s.diffWordHighlight); @@ -119,7 +120,7 @@ export function Diff({ }, [patch]); const options = { diffStyle: layout, - ...pierreThemeOptions(resolvedTheme), + ...pierreThemeOptions(resolvedTheme, diffSyntaxTheme), disableBackground: true, disableFileHeader: hideFileHeader, ...diffAppearanceOptions({ diffIndicators, diffLineNumbers, diffWordHighlight }), @@ -147,12 +148,13 @@ export function ParsedDiff({ style, }: ParsedDiffProps) { const resolvedTheme = useSettings((s) => s.resolvedTheme); + const diffSyntaxTheme = useSettings((s) => s.diffSyntaxTheme); const diffIndicators = useSettings((s) => s.diffIndicators); const diffLineNumbers = useSettings((s) => s.diffLineNumbers); const diffWordHighlight = useSettings((s) => s.diffWordHighlight); const options = { diffStyle: layout, - ...pierreThemeOptions(resolvedTheme), + ...pierreThemeOptions(resolvedTheme, diffSyntaxTheme), disableBackground: true, disableFileHeader: hideFileHeader, enableLineSelection: Boolean(onLineSelected), diff --git a/ui/src/components/DiffWorkerPool.tsx b/ui/src/components/DiffWorkerPool.tsx index 8a3fea2..b325364 100644 --- a/ui/src/components/DiffWorkerPool.tsx +++ b/ui/src/components/DiffWorkerPool.tsx @@ -3,6 +3,7 @@ import { WorkerPoolContextProvider, useWorkerPool } from '@pierre/diffs/react'; // Vite bundles the package's worker entry as a real Worker module. import DiffsWorker from '@pierre/diffs/worker/worker.js?worker'; +import { pierreThemePair } from '../lib/pierreTheme'; import { useSettings } from '../stores/settings'; /** @@ -26,7 +27,7 @@ export function DiffWorkerPool({ children }: { children: ReactNode }) { poolSize: 2, }} highlighterOptions={{ - theme: { dark: 'pierre-dark', light: 'pierre-light' }, + theme: pierreThemePair(useSettings.getState().diffSyntaxTheme), lineDiffType: initialLineDiffType(), }} > @@ -47,11 +48,15 @@ function initialLineDiffType(): 'word-alt' | 'none' { */ function RenderOptionsSync() { const pool = useWorkerPool(); + const diffSyntaxTheme = useSettings((s) => s.diffSyntaxTheme); const wordHighlight = useSettings((s) => s.diffWordHighlight); useEffect(() => { void pool - ?.setRenderOptions({ lineDiffType: wordHighlight ? 'word-alt' : 'none' }) + ?.setRenderOptions({ + theme: pierreThemePair(diffSyntaxTheme), + lineDiffType: wordHighlight ? 'word-alt' : 'none', + }) .catch((e) => console.warn('diff worker setRenderOptions failed', e)); - }, [pool, wordHighlight]); + }, [pool, diffSyntaxTheme, wordHighlight]); return null; } diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index 32777a2..51125e6 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -12,10 +12,12 @@ import { ignorePatterns } from '../lib/ignore'; import { applyEmptyDirectoryMutation } from '../lib/emptyDirectories'; import { applyLocalTreeMutation, retainLoadedIgnoredChildren } from '../lib/localTreeMutation'; import { t } from '../lib/i18n'; -import { worktreeName } from '../lib/repoIdentity'; +import { pathKey, worktreeName } from '../lib/repoIdentity'; +import { providerMergedBranchNames } from '../lib/branchIntegration'; import { workTreeGitStatus } from '../lib/workTreeGitStatus'; import { errMessage, tauri } from '../lib/tauri'; import { defaultRemote, useRepo } from '../stores/repo'; +import { useBranchIntegration } from '../stores/branchIntegration'; import { useWork } from '../stores/work'; import type { Branch, @@ -188,6 +190,11 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const recents = useRepo((s) => s.recents); const forgetRecent = useRepo((s) => s.forgetRecent); const refs = useRepo((s) => s.refs); + const activePath = useRepo((s) => s.activePath); + const integration = useBranchIntegration((s) => ( + activePath ? s.records[pathKey(activePath)] : undefined + )); + const refreshBranchIntegration = useBranchIntegration((s) => s.refresh); const pullMode = useRepo((s) => s.pullMode); const setBranchUpstream = useRepo((s) => s.setBranchUpstream); const checkout = useRepo((s) => s.checkout); @@ -259,6 +266,20 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, ); const [tab, setTab] = useState('git'); + const providerMergedBranches = useMemo( + () => integration?.data ? providerMergedBranchNames(refs, integration.data) : new Set(), + [integration?.data, refs], + ); + useEffect(() => { + if (!activePath || integration || refs.branches.length < 2) return; + // Provider discovery is intentionally off the repository-open hot path. + // A short idle delay lets the local snapshot paint first; the session cache + // prevents repeated CLI/network work when switching views or repositories. + const timer = window.setTimeout(() => { + void refreshBranchIntegration(activePath); + }, 800); + return () => window.clearTimeout(timer); + }, [activePath, integration, refreshBranchIntegration, refs.branches.length]); useEffect(() => { if (view === 'work') setTab('files'); }, [view]); @@ -1289,6 +1310,7 @@ export function Sidebar({ onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, const renderBranchLeaf = (b: Branch, depth: number) => { const wt = !b.is_head ? worktreeByBranch.get(b.name) : undefined; + const merged = b.merged || providerMergedBranches.has(b.name); return ( { expect(plan.checkedOut).toEqual(['in-use']); }); + it('includes an exact provider-confirmed squash merge without offering an unproven remote', () => { + const local = branch('squashed', { merged: false }); + const plan = mergedBranchCleanupPlan( + refs([local], [remote('origin', 'squashed', false)]), + [], + new Set(['squashed']), + ); + + expect(plan.candidates).toHaveLength(1); + expect(plan.candidates[0]).toMatchObject({ providerMerged: true, remote: null }); + }); + it('uses the configured upstream even when the remote branch has another name', () => { const upstream = remote('fork', 'published-name'); const local = branch('local-name', { diff --git a/ui/src/lib/branchCleanup.ts b/ui/src/lib/branchCleanup.ts index 929a901..cdff2c1 100644 --- a/ui/src/lib/branchCleanup.ts +++ b/ui/src/lib/branchCleanup.ts @@ -4,6 +4,8 @@ export interface BranchCleanupCandidate { local: Branch; /** Matching remote-tracking ref, when it is still known locally. */ remote: RemoteBranch | null; + /** Provider confirmed this exact tip was merged even though ancestry did not. */ + providerMerged: boolean; } export interface BranchCleanupPlan { @@ -35,12 +37,18 @@ function matchingRemote(branch: Branch, refs: Refs): RemoteBranch | null { * Build the safe, display-ready cleanup plan from already-loaded ref/worktree * state. No ancestry walk or network request belongs on the dialog-open path. */ -export function mergedBranchCleanupPlan(refs: Refs, worktrees: Worktree[]): BranchCleanupPlan { +export function mergedBranchCleanupPlan( + refs: Refs, + worktrees: Worktree[], + providerMergedBranches: ReadonlySet = new Set(), +): BranchCleanupPlan { const checkedOutBranches = new Set( worktrees.map((worktree) => worktree.branch).filter((branch): branch is string => !!branch), ); const merged = refs.branches.filter( - (branch) => branch.merged && !branch.is_head && branch.name !== refs.primary_branch, + (branch) => (branch.merged || providerMergedBranches.has(branch.name)) + && !branch.is_head + && branch.name !== refs.primary_branch, ); const checkedOut = merged @@ -48,7 +56,11 @@ export function mergedBranchCleanupPlan(refs: Refs, worktrees: Worktree[]): Bran .map((branch) => branch.name); const candidates = merged .filter((branch) => !checkedOutBranches.has(branch.name)) - .map((local) => ({ local, remote: matchingRemote(local, refs) })); + .map((local) => ({ + local, + remote: matchingRemote(local, refs), + providerMerged: !local.merged && providerMergedBranches.has(local.name), + })); return { candidates, checkedOut }; } diff --git a/ui/src/lib/branchIntegration.test.ts b/ui/src/lib/branchIntegration.test.ts new file mode 100644 index 0000000..e99fddd --- /dev/null +++ b/ui/src/lib/branchIntegration.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest'; + +import { providerMergedBranchNames } from './branchIntegration'; +import type { Branch, PullRequest, PullRequestList, Refs } from './types'; + +function branch(name: string, target: string, isHead = false): Branch { + return { + name, + full_name: `refs/heads/${name}`, + target, + is_head: isHead, + merged: false, + upstream: null, + ahead: 0, + behind: 0, + }; +} + +function pullRequest(overrides: Partial): PullRequest { + return { + id: 1, + title: 'Merged change', + state: 'completed', + is_draft: false, + can_mark_ready: false, + author: 'Daniel', + source_branch: 'feature', + source_commit: 'a'.repeat(40), + target_branch: 'main', + created_at: '', + updated_at: '', + completed_at: '', + url: '', + description: '', + merge_status: '', + review_status: '', + comment_count: 0, + commit_count: 0, + additions: null, + deletions: null, + changed_files: null, + labels: [], + reviewers: [], + reviews: [], + checks: [], + checks_complete: false, + comments: [], + review_threads: [], + authored_by_viewer: false, + commits: [], + ...overrides, + }; +} + +function data(pullRequests: PullRequest[]): PullRequestList { + return { + repository: { provider: 'azure_dev_ops', remote: 'origin', label: 'repo', viewer: null }, + pull_requests: pullRequests, + }; +} + +const refs: Refs = { + primary_branch: 'main', + branches: [branch('main', 'f'.repeat(40)), branch('feature', 'a'.repeat(40))], + remotes: [], + remote_branches: [], + tags: [], +}; + +describe('providerMergedBranchNames', () => { + it('marks the exact current source tip of a completed PR into the primary branch', () => { + expect([...providerMergedBranchNames(refs, data([pullRequest({})]))]).toEqual(['feature']); + expect([...providerMergedBranchNames(refs, data([pullRequest({ state: 'merged' })]))]).toEqual(['feature']); + }); + + it('rejects closed PRs, other targets, moved branches, and the checked-out branch', () => { + expect(providerMergedBranchNames(refs, data([pullRequest({ state: 'closed' })])).size).toBe(0); + expect(providerMergedBranchNames(refs, data([pullRequest({ target_branch: 'release' })])).size).toBe(0); + expect(providerMergedBranchNames(refs, data([pullRequest({ source_commit: 'b'.repeat(40) })])).size).toBe(0); + const checkedOut = { ...refs, branches: [refs.branches[0], branch('feature', 'a'.repeat(40), true)] }; + expect(providerMergedBranchNames(checkedOut, data([pullRequest({})])).size).toBe(0); + }); +}); diff --git a/ui/src/lib/branchIntegration.ts b/ui/src/lib/branchIntegration.ts new file mode 100644 index 0000000..4a629b4 --- /dev/null +++ b/ui/src/lib/branchIntegration.ts @@ -0,0 +1,29 @@ +import type { PullRequestList, Refs } from './types'; + +/** Exact local branches whose current tip is the source tip of a completed PR + * into this repository's primary branch. */ +export function providerMergedBranchNames( + refs: Refs, + data: PullRequestList, +): Set { + const primary = refs.primary_branch; + if (!primary) return new Set(); + const localTargets = new Map( + refs.branches + .filter((branch) => !branch.is_head && branch.name !== primary) + .map((branch) => [branch.name, branch.target.toLowerCase()]), + ); + const names = new Set(); + for (const pr of data.pull_requests) { + const state = pr.state.toLowerCase(); + if ( + (state === 'merged' || state === 'completed') + && pr.target_branch === primary + && !!pr.source_commit + && localTargets.get(pr.source_branch) === pr.source_commit.toLowerCase() + ) { + names.add(pr.source_branch); + } + } + return names; +} diff --git a/ui/src/lib/pierreTheme.ts b/ui/src/lib/pierreTheme.ts index 7e90415..84e1045 100644 --- a/ui/src/lib/pierreTheme.ts +++ b/ui/src/lib/pierreTheme.ts @@ -1,13 +1,32 @@ -import type { Theme } from '../stores/settings'; +import type { DiffsThemeNames, ThemesType } from '@pierre/diffs'; + +import type { DiffSyntaxTheme, Theme } from '../stores/settings'; + +export const DIFF_SYNTAX_THEME_OPTIONS: readonly { id: DiffSyntaxTheme; label: string }[] = [ + { id: 'standard', label: 'Standard' }, + { id: 'soft', label: 'Soft' }, + { id: 'vibrant', label: 'Vibrant' }, + { id: 'protanopia-deuteranopia', label: 'Red–green accessible' }, + { id: 'tritanopia', label: 'Blue–yellow accessible' }, +]; + +export function pierreThemePair(family: DiffSyntaxTheme): ThemesType { + const suffix = family === 'standard' ? '' : `-${family}`; + return { + light: `pierre-light${suffix}` as DiffsThemeNames, + dark: `pierre-dark${suffix}` as DiffsThemeNames, + }; +} /** * Pierre's worker pool renders both palettes into one cached AST. `themeType` * selects which palette the mounted instance displays; `theme` remains the * single-theme fallback when the worker pool is unavailable. */ -export function pierreThemeOptions(theme: Theme) { +export function pierreThemeOptions(theme: Theme, family: DiffSyntaxTheme = 'standard') { + const pair = pierreThemePair(family); return { - theme: theme === 'light' ? 'pierre-light' : 'pierre-dark', + theme: pair[theme], themeType: theme, } as const; } diff --git a/ui/src/lib/tauri.ts b/ui/src/lib/tauri.ts index 5cc509b..a4d7a1b 100644 --- a/ui/src/lib/tauri.ts +++ b/ui/src/lib/tauri.ts @@ -15,6 +15,7 @@ import type { Commit, CommitSignature, CommitMessageSuggestion, + CodeReviewSuggestion, CommitSearchMode, CommitOutcome, CrashCheck, @@ -202,6 +203,23 @@ export const tauri = { anthropicCli: anthropicCli ?? null, request, }), + repoReviewChanges: ( + path: string, + baseline: string | null, + provider: AiProvider, + model: string, + request: AiGenerationRequest, + openaiCli?: string | null, + anthropicCli?: string | null, + ) => invoke>('repo_review_changes', { + path, + baseline, + provider, + model, + openaiCli: openaiCli ?? null, + anthropicCli: anthropicCli ?? null, + request, + }), repoPullRequestActivity: (path: string, id: number) => invoke('repo_pull_request_activity', { path, id }), repoPullRequest: (path: string, id: number) => invoke('repo_pull_request', { path, id }), @@ -495,6 +513,8 @@ export const tauri = { ) => invoke('repo_branch_create', { path, name, startPoint, checkout }), repoBranchDelete: (path: string, name: string, force: boolean) => invoke('repo_branch_delete', { path, name, force }), + repoBranchDeleteAt: (path: string, name: string, expectedTarget: string) => + invoke('repo_branch_delete_at', { path, name, expectedTarget }), repoBranchRename: (path: string, oldName: string, newName: string) => invoke('repo_branch_rename', { path, oldName, newName }), repoBranchSetUpstream: (path: string, branch: string, upstream: string | null) => diff --git a/ui/src/lib/theme.test.ts b/ui/src/lib/theme.test.ts index 6575e1a..1e8af8f 100644 --- a/ui/src/lib/theme.test.ts +++ b/ui/src/lib/theme.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { pierreThemeOptions } from './pierreTheme'; +import { DIFF_SYNTAX_THEME_OPTIONS, pierreThemeOptions, pierreThemePair } from './pierreTheme'; describe('pierreThemeOptions', () => { it('selects the light palette independently of the OS theme', () => { @@ -16,4 +16,28 @@ describe('pierreThemeOptions', () => { themeType: 'dark', }); }); + + it('selects the matching family member for each app appearance', () => { + expect(pierreThemeOptions('light', 'soft')).toEqual({ + theme: 'pierre-light-soft', + themeType: 'light', + }); + expect(pierreThemeOptions('dark', 'vibrant')).toEqual({ + theme: 'pierre-dark-vibrant', + themeType: 'dark', + }); + }); + + it('exposes every official Pierre palette as a light/dark pair', () => { + expect(DIFF_SYNTAX_THEME_OPTIONS.map(({ id }) => pierreThemePair(id))).toEqual([ + { light: 'pierre-light', dark: 'pierre-dark' }, + { light: 'pierre-light-soft', dark: 'pierre-dark-soft' }, + { light: 'pierre-light-vibrant', dark: 'pierre-dark-vibrant' }, + { + light: 'pierre-light-protanopia-deuteranopia', + dark: 'pierre-dark-protanopia-deuteranopia', + }, + { light: 'pierre-light-tritanopia', dark: 'pierre-dark-tritanopia' }, + ]); + }); }); diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index 0368354..8f3a56c 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -153,6 +153,9 @@ export interface ReviewNote { * persisted before this field existed are all new-side. */ side?: 'new' | 'old'; + /** Accepted AI findings stay distinguishable from human-authored notes. */ + source?: 'ai'; + severity?: CodeReviewSeverity; createdAt: number; } @@ -812,6 +815,21 @@ export interface CommitMessageSuggestion { body: string | null; } +export type CodeReviewSeverity = 'critical' | 'high' | 'medium' | 'low'; + +export interface CodeReviewFinding { + path: string; + line: number | null; + side: 'new' | 'old'; + severity: CodeReviewSeverity; + title: string; + body: string; +} + +export interface CodeReviewSuggestion { + findings: CodeReviewFinding[]; +} + export type AiSensitiveDecision = | { mode: 'scan' } | { mode: 'exclude'; fingerprint: string } @@ -823,7 +841,7 @@ export interface AiGenerationRequest { styleInstruction: string | null; } -export type AiInputScope = 'staged' | 'unstaged' | 'committed'; +export type AiInputScope = 'staged' | 'unstaged' | 'committed' | 'review'; export interface AiInputCoverage { scope: AiInputScope; diff --git a/ui/src/stores/branchIntegration.ts b/ui/src/stores/branchIntegration.ts new file mode 100644 index 0000000..963f0d5 --- /dev/null +++ b/ui/src/stores/branchIntegration.ts @@ -0,0 +1,52 @@ +import { create } from 'zustand'; + +import { pathKey } from '../lib/repoIdentity'; +import { errMessage, tauri } from '../lib/tauri'; +import type { PullRequestList } from '../lib/types'; + +export interface BranchIntegrationRecord { + status: 'loading' | 'loaded' | 'error'; + data: PullRequestList | null; + error: string | null; +} + +interface BranchIntegrationState { + records: Record; + refresh(path: string, force?: boolean): Promise; +} + +const requests = new Map>(); + +export const useBranchIntegration = create((set, get) => ({ + records: {}, + + async refresh(path, force = false) { + const key = pathKey(path); + const current = get().records[key]; + if (!force && current?.status === 'loaded') return; + const pending = requests.get(key); + if (pending) return pending; + + set((state) => ({ + records: { + ...state.records, + [key]: { status: 'loading', data: current?.data ?? null, error: null }, + }, + })); + const request = tauri.repoPullRequests(path).then( + (data) => set((state) => ({ + records: { ...state.records, [key]: { status: 'loaded', data, error: null } }, + })), + (error) => set((state) => ({ + records: { + ...state.records, + [key]: { status: 'error', data: current?.data ?? null, error: errMessage(error) }, + }, + })), + ).finally(() => { + requests.delete(key); + }); + requests.set(key, request); + return request; + }, +})); diff --git a/ui/src/stores/repo.test.ts b/ui/src/stores/repo.test.ts index 1914ad1..5d203f7 100644 --- a/ui/src/stores/repo.test.ts +++ b/ui/src/stores/repo.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { RepoMeta } from '../lib/types'; const values = new Map(); const storage: Storage = { @@ -15,7 +16,7 @@ vi.stubGlobal('navigator', { userAgent: '' }); vi.stubGlobal('document', { documentElement: { dataset: {} } }); const { tauri } = await import('../lib/tauri'); -const { useRepo } = await import('./repo'); +const { addAiReviewNoteSet, useRepo } = await import('./repo'); const original = useRepo.getState(); @@ -70,4 +71,76 @@ describe('repository navigation state', () => { expect(refreshLog).toHaveBeenCalledOnce(); expect(refreshWorktrees).toHaveBeenCalledOnce(); }); + + it('keeps staged files in the uncommitted Review pool', async () => { + const diffs = [{ + path: 'staged.ts', old_path: null, status: 'modified', adds: 1, dels: 0, + patch: '@@ -1 +1 @@\n-old\n+new\n', binary: false, + }] as const; + const diffSince = vi.spyOn(tauri, 'repoDiffSinceFull').mockResolvedValue([...diffs]); + useRepo.setState({ activePath: '/repo', baseline: null, reviewUnstagedDiffs: [] }); + + await useRepo.getState().refreshReviewDiffs(); + + expect(diffSince).toHaveBeenCalledWith('/repo', 'HEAD'); + expect(useRepo.getState().reviewUnstagedDiffs).toEqual(diffs); + }); + + it('pins the initial baseline at the detected branch fork point', async () => { + const setBaseline = vi.fn(async () => {}); + const detect = vi.spyOn(tauri, 'repoDetectBaseBranch').mockResolvedValue({ + name: 'main', + merge_base: '1234567890', + }); + useRepo.setState({ + activePath: '/repo', + meta: { branch: 'feature', detached: false } as RepoMeta, + setBaseline, + }); + + await expect(useRepo.getState().setBranchBaseline()).resolves.toEqual({ + name: 'main', + merge_base: '1234567890', + }); + expect(detect).toHaveBeenCalledWith('/repo', 'feature'); + expect(setBaseline).toHaveBeenCalledWith('1234567890'); + }); +}); + +describe('AI review notes', () => { + it('adds accepted AI findings without touching existing feedback', () => { + const next = addAiReviewNoteSet( + { + 'src/old.ts': [ + { id: 'human', text: 'Keep this', line: null, createdAt: 1 }, + { id: 'old-ai', text: 'Existing finding', line: 2, source: 'ai', severity: 'low', createdAt: 2 }, + ], + 'src/ai-only.ts': [ + { id: 'old-only', text: 'Keep this too', line: null, source: 'ai', severity: 'medium', createdAt: 3 }, + ], + }, + [{ + path: 'src/new.ts', + line: 7, + side: 'new', + severity: 'high', + title: 'Possible race', + body: 'The shared value is not synchronized.', + }], + ); + + expect(next['src/old.ts']).toEqual([ + { id: 'human', text: 'Keep this', line: null, createdAt: 1 }, + { id: 'old-ai', text: 'Existing finding', line: 2, source: 'ai', severity: 'low', createdAt: 2 }, + ]); + expect(next['src/ai-only.ts']).toEqual([ + { id: 'old-only', text: 'Keep this too', line: null, source: 'ai', severity: 'medium', createdAt: 3 }, + ]); + expect(next['src/new.ts']).toMatchObject([{ + text: 'Possible race — The shared value is not synchronized.', + line: 7, + source: 'ai', + severity: 'high', + }]); + }); }); diff --git a/ui/src/stores/repo.ts b/ui/src/stores/repo.ts index 4212353..b34fd72 100644 --- a/ui/src/stores/repo.ts +++ b/ui/src/stores/repo.ts @@ -19,6 +19,8 @@ import { jsonEqual, stable } from '../lib/stable'; import { errMessage, tauri } from '../lib/tauri'; import { useSettings, type DiffMode } from './settings'; import type { + BaseBranch, + CodeReviewFinding, Commit, BranchPushRequest, CommitSearchMode, @@ -115,7 +117,8 @@ export interface RepoState { * Review-session baseline for the active repo: "show me everything since * this commit". Pinned by the user (or restored from SQLite); drives the * Review view's session mode and {@link RepoState.baselineDiffs}. Without - * a baseline the Review view falls back to the unstaged set (inbox mode). + * a baseline the Review view falls back to all uncommitted changes (staged + * + unstaged, combined against HEAD). */ baseline: StoredBaseline | null; /** @@ -125,9 +128,9 @@ export interface RepoState { */ baselineDiffs: FileDiff[]; /** - * Inbox-mode counterpart: the unstaged set with whole-file context - * (`diff_unstaged_full`). Only refreshed while the Review view is open (or - * on entry), so the regular `unstagedDiffs` hot path doesn't pay for it. + * Inbox-mode counterpart: all uncommitted changes with whole-file context + * (`diff_since_full("HEAD")`). Only refreshed while the Review view is open + * (or on entry), so the regular local-diff hot path doesn't pay for it. */ reviewUnstagedDiffs: FileDiff[]; @@ -338,12 +341,14 @@ export interface RepoState { /** Pin the review baseline at `oid` (default: the current HEAD). */ setBaseline(oid?: string): Promise; + /** Detect the active branch's parent and pin the baseline at its fork point. */ + setBranchBaseline(): Promise; /** Clear the review baseline (and its persisted record). */ clearBaseline(): Promise; /** * Refresh the Review view's diff pool (whole-file context): with a baseline * → `diff_since_full` into {@link RepoState.baselineDiffs}; without → - * `diff_unstaged_full` into {@link RepoState.reviewUnstagedDiffs}. + * `diff_since_full("HEAD")` into {@link RepoState.reviewUnstagedDiffs}. */ refreshReviewDiffs(): Promise; /** Load the persisted baseline + reviewed map when a repo becomes active. */ @@ -359,6 +364,8 @@ export interface RepoState { addReviewNote(file: string, text: string, line: number | null, side?: 'new' | 'old'): void; /** Remove one note from `file` by id. */ removeReviewNote(file: string, id: string): void; + /** Add user-approved AI findings as notes in one persisted write. */ + addAiReviewFindings(findings: CodeReviewFinding[]): void; /** Drop every note for the active repo (after a feedback export, usually). */ clearReviewNotes(): void; /** Stage every unstaged file whose reviewed mark matches its current diff. */ @@ -792,6 +799,31 @@ export function makeReviewNote( }; } +/** Append only findings the reviewer explicitly accepted; avoid duplicates. */ +export function addAiReviewNoteSet( + current: Record, + findings: CodeReviewFinding[], +): Record { + const next: Record = { ...current }; + for (const finding of findings) { + const text = finding.body ? `${finding.title} — ${finding.body}` : finding.title; + const duplicate = (next[finding.path] ?? []).some((note) => + note.source === 'ai' + && note.text === text + && note.line === finding.line + && (note.side ?? 'new') === finding.side + && note.severity === finding.severity); + if (duplicate) continue; + const note = makeReviewNote(text, finding.line, finding.side); + if (!note) continue; + next[finding.path] = [ + ...(next[finding.path] ?? []), + { ...note, source: 'ai', severity: finding.severity }, + ]; + } + return next; +} + const EMPTY_ACTIVE = { activePath: null as string | null, meta: null as RepoMeta | null, @@ -1190,6 +1222,18 @@ export const useRepo = create((set, get) => ({ ]); }, + async setBranchBaseline() { + const path = get().activePath; + const meta = get().meta; + if (!path || !meta) throw new Error('No repository is open'); + if (meta.detached) throw new Error('A branch baseline is unavailable while HEAD is detached'); + const hit = await tauri.repoDetectBaseBranch(path, meta.branch); + if (get().activePath !== path) throw new Error('The active repository changed'); + if (!hit) throw new Error(`Could not detect where ${meta.branch} was created`); + await get().setBaseline(hit.merge_base); + return hit; + }, + async clearBaseline() { const path = get().activePath; const scope = activeReviewNoteScope(get(), null); @@ -1232,11 +1276,13 @@ export const useRepo = create((set, get) => ({ return; } try { - const diffs = await tauri.repoDiffUnstagedFull(path); + // HEAD → index + worktree keeps a file visible after staging and + // combines partially staged files into the exact on-disk result. + const diffs = await tauri.repoDiffSinceFull(path, 'HEAD'); if (get().activePath !== path || get().baseline != null) return; set({ reviewUnstagedDiffs: diffs, diffsTick: get().diffsTick + 1 }); } catch (e) { - console.warn('repoDiffUnstagedFull failed', e); + console.warn('repoDiffSinceFull(HEAD) failed', e); } }, @@ -1306,6 +1352,16 @@ export const useRepo = create((set, get) => ({ console.warn('review notes persist failed', e)); }, + addAiReviewFindings(findings) { + const path = get().activePath; + if (!path || findings.length === 0) return; + const next = addAiReviewNoteSet(get().reviewNotes, findings); + const scope = activeReviewNoteScope(get()); + set({ reviewNotes: next }); + void reviewSession.setNotes(path, next, scope).catch((e) => + console.warn('review notes persist failed', e)); + }, + clearReviewNotes() { const path = get().activePath; if (!path) return; diff --git a/ui/src/stores/settings.test.ts b/ui/src/stores/settings.test.ts index 4278925..d8586a2 100644 --- a/ui/src/stores/settings.test.ts +++ b/ui/src/stores/settings.test.ts @@ -21,9 +21,11 @@ test('persists startup settings and seeds the configured initial space', async ( anthropic: null, }); useSettings.getState().set('startupSpace', 'review'); + useSettings.getState().set('diffSyntaxTheme', 'protanopia-deuteranopia'); const persisted = JSON.parse(storage.getItem('strand.settings') ?? '{}'); expect(persisted.state.startupSpace).toBe('review'); + expect(persisted.state.diffSyntaxTheme).toBe('protanopia-deuteranopia'); expect(persisted.state.aiConnectionStatus).toEqual({ openai: { installed: true, loggedIn: true, checkedAt: 123 }, anthropic: null, diff --git a/ui/src/stores/settings.ts b/ui/src/stores/settings.ts index 8b42a3e..51958ec 100644 --- a/ui/src/stores/settings.ts +++ b/ui/src/stores/settings.ts @@ -15,6 +15,13 @@ export type AccentId = 'amber' | 'rose' | 'magenta' | 'violet' | 'blue' | 'cyan' export type Platform = 'mac' | 'win11' | 'linux'; export type Density = 'compact' | 'default' | 'relaxed'; export type DiffMode = 'stacked' | 'split'; +/** Pierre syntax palette family; Strand selects the matching light/dark member. */ +export type DiffSyntaxTheme = + | 'standard' + | 'soft' + | 'vibrant' + | 'protanopia-deuteranopia' + | 'tritanopia'; export type GraphStyle = 'classic' | 'bold' | 'mono'; /** Repository space shown after Strand launches. */ export type StartupSpace = 'work' | 'local' | 'review' | 'pull-requests' | 'commits'; @@ -113,6 +120,7 @@ export interface SettingsState { defaultDiffLayout: DiffMode; /** Font for diff/code panes; `inherit` follows `monoFont`. */ diffFont: MonoFont | 'inherit'; + diffSyntaxTheme: DiffSyntaxTheme; diffLineNumbers: boolean; diffIndicators: DiffIndicators; /** Intra-line (word-level) change emphasis in diffs. */ @@ -230,6 +238,14 @@ export const STARTUP_SPACE_OPTIONS: { id: StartupSpace; label: string }[] = [ { id: 'commits', label: 'All Commits' }, ]; +export const DIFF_SYNTAX_THEMES: readonly DiffSyntaxTheme[] = [ + 'standard', + 'soft', + 'vibrant', + 'protanopia-deuteranopia', + 'tritanopia', +]; + export const useSettings = create()( persist( (set) => ({ @@ -249,6 +265,7 @@ export const useSettings = create()( monoFont: 'jetbrains', defaultDiffLayout: 'stacked', diffFont: 'inherit', + diffSyntaxTheme: 'standard', diffLineNumbers: true, diffIndicators: 'bars', diffWordHighlight: true, @@ -295,11 +312,15 @@ export const useSettings = create()( const terminalFont = TERMINAL_FONT_OPTIONS.some((option) => option.id === next.terminalFont) ? next.terminalFont : current.terminalFont; + const diffSyntaxTheme = DIFF_SYNTAX_THEMES.includes(next.diffSyntaxTheme) + ? next.diffSyntaxTheme + : current.diffSyntaxTheme; const storedSize = Number(next.terminalFontSize); return { ...next, platform: detectPlatform(), terminalFont, + diffSyntaxTheme, terminalFontSize: Number.isFinite(storedSize) ? Math.min(32, Math.max(10, Math.round(storedSize))) : current.terminalFontSize, diff --git a/ui/src/styles/features.css b/ui/src/styles/features.css index b111c85..1660453 100644 --- a/ui/src/styles/features.css +++ b/ui/src/styles/features.css @@ -5509,6 +5509,93 @@ textarea.clone-input { .rv-toolbar .h-link.danger { color: var(--del); } +.rv-toolbar .h-link:disabled { + color: var(--text-dim); + cursor: default; +} +.rv-ai-action { + display: inline-flex; + align-items: center; + gap: 4px; +} +.rv-ai-review { + flex: none; + background: color-mix(in srgb, var(--accent) 3%, var(--bg-elev)); + box-shadow: 0 0.5px 0 var(--border); +} +.rv-ai-panel { + display: flex; + align-items: center; + gap: 10px; + min-height: 30px; + padding: 5px 12px; + font-size: 11px; + color: var(--text-2); + background: color-mix(in srgb, var(--accent) 5%, var(--bg-elev)); +} +.rv-ai-panel > span:first-child { margin-right: auto; } +.rv-ai-panel .h-link { flex: none; color: var(--accent); } +.rv-ai-findings { + max-height: 240px; + overflow-y: auto; + padding: 0 12px 8px; +} +.rv-ai-finding { + display: grid; + grid-template-columns: auto minmax(0, 1fr) auto; + align-items: start; + gap: 10px; + padding: 8px 0; + border-top: 0.5px solid var(--border); + font-size: 11px; +} +.rv-ai-severity { + min-width: 50px; + padding: 2px 5px; + border-radius: 4px; + color: var(--text-2); + background: var(--bg-base); + font-size: 9px; + font-weight: 700; + letter-spacing: 0.04em; + line-height: 1.35; + text-align: center; + text-transform: uppercase; +} +.rv-ai-severity.critical, +.rv-ai-severity.high { color: var(--del); } +.rv-ai-severity.medium { color: var(--warn); } +.rv-ai-severity.low { color: var(--text-dim); } +.rv-ai-finding-copy { min-width: 0; } +.rv-ai-finding-title { + color: var(--text); + font-weight: 600; +} +.rv-ai-finding-body { + margin-top: 2px; + color: var(--text-2); + line-height: 1.4; +} +.rv-ai-location { + display: block; + max-width: 100%; + margin-top: 4px; + overflow: hidden; + color: var(--accent); + font-family: var(--font-mono); + font-size: 10px; + text-overflow: ellipsis; + white-space: nowrap; + cursor: pointer; +} +.rv-ai-location:hover { color: var(--accent-2); } +.rv-ai-finding-actions { + display: flex; + align-items: center; + gap: 10px; + white-space: nowrap; +} +.rv-ai-finding-actions .h-link { color: var(--accent); } /* File queue (left panel) — a Pierre tree; this is just its flex host. */ .rv-tree { @@ -5659,6 +5746,18 @@ textarea.clone-input { color: var(--accent); background: color-mix(in srgb, var(--accent) 12%, transparent); } +.rv-note-source { + flex: none; + font-size: 9px; + font-weight: 650; + text-transform: uppercase; + letter-spacing: 0.03em; + color: var(--text-dim); +} +.rv-note-source.critical, +.rv-note-source.high { color: var(--del); } +.rv-note-source.medium { color: var(--warn); } +.rv-note-source.low { color: var(--accent); } .rv-note-text { flex: 1; min-width: 0; diff --git a/ui/src/views/BranchCleanupDialog.tsx b/ui/src/views/BranchCleanupDialog.tsx index f65c487..e552815 100644 --- a/ui/src/views/BranchCleanupDialog.tsx +++ b/ui/src/views/BranchCleanupDialog.tsx @@ -2,7 +2,10 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { Icon } from '../components/Icon'; import { mergedBranchCleanupPlan } from '../lib/branchCleanup'; +import { providerMergedBranchNames } from '../lib/branchIntegration'; +import { pathKey } from '../lib/repoIdentity'; import { errMessage, tauri } from '../lib/tauri'; +import { useBranchIntegration } from '../stores/branchIntegration'; import { useRepo } from '../stores/repo'; function ScopeCheckbox({ @@ -49,13 +52,31 @@ export function BranchCleanupDialog({ const worktrees = useRepo((state) => state.worktrees); const refreshRefs = useRepo((state) => state.refreshRefs); const refreshLog = useRepo((state) => state.refreshLog); + const integration = useBranchIntegration((state) => ( + activePath ? state.records[pathKey(activePath)] : undefined + )); + const refreshBranchIntegration = useBranchIntegration((state) => state.refresh); + const [refreshingProvider, setRefreshingProvider] = useState(!!activePath); + const providerMergedBranches = useMemo( + () => integration?.status === 'loaded' && integration.data + ? providerMergedBranchNames(refs, integration.data) + : new Set(), + [integration?.data, integration?.status, refs], + ); + const checkingProvider = refreshingProvider || (!!activePath && (!integration || ( + integration.status === 'loading' && !integration.data + ))); - // Freeze the plan while the dialog is open. A remote push updates the local - // tracking ref, and watcher refreshes must not make rows disappear mid-run. - const [plan] = useState(() => mergedBranchCleanupPlan(refs, worktrees)); - const [localSelection, setLocalSelection] = useState( - () => new Set(plan.candidates.map((candidate) => candidate.local.name)), + const computedPlan = useMemo( + () => mergedBranchCleanupPlan(refs, worktrees, providerMergedBranches), + [providerMergedBranches, refs, worktrees], ); + // Freeze the plan after the provider refresh. A push updates tracking refs, + // and watcher refreshes must not make rows disappear during a cleanup run. + const [frozenPlan, setFrozenPlan] = useState(null); + const plan = frozenPlan ?? computedPlan; + const [localSelection, setLocalSelection] = useState>(() => new Set()); + const selectionInitialized = useRef(false); // Remote deletion is deliberately opt-in: it changes the shared repository. const [remoteSelection, setRemoteSelection] = useState>(() => new Set()); const [busy, setBusy] = useState(false); @@ -67,6 +88,26 @@ export function BranchCleanupDialog({ openerRef.current = document.activeElement as HTMLElement | null; } + useEffect(() => { + let cancelled = false; + if (!activePath) { + setRefreshingProvider(false); + return; + } + setRefreshingProvider(true); + void refreshBranchIntegration(activePath, true).finally(() => { + if (!cancelled) setRefreshingProvider(false); + }); + return () => { cancelled = true; }; + }, [activePath, refreshBranchIntegration]); + + useEffect(() => { + if (checkingProvider || selectionInitialized.current) return; + selectionInitialized.current = true; + setFrozenPlan(computedPlan); + setLocalSelection(new Set(computedPlan.candidates.map((candidate) => candidate.local.name))); + }, [checkingProvider, computedPlan]); + const remoteByRef = useMemo(() => { const remotes = new Map>(); for (const candidate of plan.candidates) { @@ -147,7 +188,7 @@ export function BranchCleanupDialog({ } async function clearSelected() { - if (!activePath || busy || total === 0) return; + if (!activePath || busy || checkingProvider || total === 0) return; setBusy(true); setCompleted(0); @@ -186,7 +227,15 @@ export function BranchCleanupDialog({ continue; } try { - await tauri.repoBranchDelete(activePath, candidate.local.name, false); + if (candidate.providerMerged) { + await tauri.repoBranchDeleteAt( + activePath, + candidate.local.name, + candidate.local.target, + ); + } else { + await tauri.repoBranchDelete(activePath, candidate.local.name, false); + } localCleared += 1; } catch (error) { errors.push(`${candidate.local.name}: ${errMessage(error)}`); @@ -249,18 +298,24 @@ export function BranchCleanupDialog({

These branch tips are contained by{' '} - {refs.primary_branch ?? 'the primary branch'}. Choose the local refs to - remove and any matching remote refs that still exist. + {refs.primary_branch ?? 'the primary branch'}, or their exact current tip + was merged there by the hosted provider. Choose the local refs to remove and any + matching remote refs that still exist.

- {plan.candidates.length > 0 ? ( + {checkingProvider ? ( +
+ + Checking hosted pull requests for squash and rebase merges… +
+ ) : plan.candidates.length > 0 ? (
Branch 0 && !allLocals} - disabled={busy} + disabled={busy || checkingProvider} label={`Local (${plan.candidates.length})`} onChange={(checked) => setLocalSelection( checked @@ -271,7 +326,7 @@ export function BranchCleanupDialog({ 0 && !allRemotes} - disabled={busy || remoteKeys.length === 0} + disabled={busy || checkingProvider || remoteKeys.length === 0} label={`Remote (${remoteKeys.length})`} onChange={(checked) => setRemoteSelection(checked ? new Set(remoteKeys) : new Set())} /> @@ -287,7 +342,7 @@ export function BranchCleanupDialog({ toggleLocal(candidate.local.name, event.target.checked)} /> @@ -298,7 +353,7 @@ export function BranchCleanupDialog({ toggleRemote( candidate.remote!.full_name, @@ -331,6 +386,13 @@ export function BranchCleanupDialog({
) : null} + {integration?.status === 'error' ? ( +
+ + Hosted merge status is unavailable; showing ancestry-only results. {integration.error} +
+ ) : null} + {remoteCount > 0 ? (
Remote deletion runs git push --delete. Strand cannot restore a remote @@ -344,9 +406,9 @@ export function BranchCleanupDialog({ {busy ? `Clearing ${completed} of ${total}…` : summary} - {plan.candidates.length > 0 ? ( + {!checkingProvider && plan.candidates.length > 0 ? ( diff --git a/ui/src/views/Commits.tsx b/ui/src/views/Commits.tsx index 846bd2c..bc04cd3 100644 --- a/ui/src/views/Commits.tsx +++ b/ui/src/views/Commits.tsx @@ -6,9 +6,12 @@ import { computeGraph } from '../lib/graph'; import { pickCommitPatchDestination } from '../lib/dialog'; import { EDITABLE_SELECTOR, eventInside } from '../lib/keys'; import { selectedCommitsOldestFirst } from '../lib/historySelection'; +import { providerMergedBranchNames } from '../lib/branchIntegration'; +import { pathKey } from '../lib/repoIdentity'; import { errMessage, tauri } from '../lib/tauri'; import type { Commit, Refs, Stash } from '../lib/types'; import { useRepo } from '../stores/repo'; +import { useBranchIntegration } from '../stores/branchIntegration'; import { useSettings } from '../stores/settings'; import { useWork } from '../stores/work'; import { ContextMenu, type MenuItem } from '../components/ContextMenu'; @@ -64,6 +67,10 @@ export function Commits({ onCreateTag, onInteractiveRebase, onResetTo, onCreateW const meta = useRepo((s) => s.meta); const stashes = useRepo((s) => s.stashes); const refs = useRepo((s) => s.refs); + const activePath = useRepo((s) => s.activePath); + const integration = useBranchIntegration((s) => ( + activePath ? s.records[pathKey(activePath)] : undefined + )); const selectedCommit = useRepo((s) => s.selectedCommit); const selectCommit = useRepo((s) => s.selectCommit); const stashApply = useRepo((s) => s.stashApply); @@ -477,7 +484,14 @@ export function Commits({ onCreateTag, onInteractiveRebase, onResetTo, onCreateW }, [rowH], ); - const refsByOid = useMemo(() => indexRefs(refs), [refs]); + const providerMergedBranches = useMemo( + () => integration?.data ? providerMergedBranchNames(refs, integration.data) : new Set(), + [integration?.data, refs], + ); + const refsByOid = useMemo( + () => indexRefs(refs, providerMergedBranches), + [providerMergedBranches, refs], + ); const currentCommit = useMemo(() => currentCommitHash(refs, commits), [commits, refs]); const colWidth = graphColWidth(graph.laneCount); @@ -1446,7 +1460,7 @@ interface RefChip { title?: string; } -function indexRefs(refs: Refs): Map { +function indexRefs(refs: Refs, providerMergedBranches: ReadonlySet): Map { const m = new Map(); const push = (oid: string, chip: RefChip) => { const arr = m.get(oid); @@ -1454,12 +1468,13 @@ function indexRefs(refs: Refs): Map { else m.set(oid, [chip]); }; for (const b of refs.branches) { + const merged = b.merged || providerMergedBranches.has(b.name); push(b.target, { key: `b:${b.full_name}`, label: b.name, kind: b.is_head ? 'head' : 'local', - merged: b.merged, - title: b.merged + merged, + title: merged ? `Merged into ${refs.primary_branch ?? 'the primary branch'}; safe to delete` : undefined, }); diff --git a/ui/src/views/LocalChanges.tsx b/ui/src/views/LocalChanges.tsx index 2a05085..6fc9b33 100644 --- a/ui/src/views/LocalChanges.tsx +++ b/ui/src/views/LocalChanges.tsx @@ -943,6 +943,7 @@ export function HunkAnnotatedDiff({ const applyPatch = useRepo((s) => s.applyPatch); const discardPatch = useRepo((s) => s.discardPatch); const resolvedTheme = useSettings((s) => s.resolvedTheme); + const diffSyntaxTheme = useSettings((s) => s.diffSyntaxTheme); const [pending, setPending] = useState(null); const [lineSelection, setLineSelection] = useState(null); const [linePicker, setLinePicker] = useState(null); @@ -1189,7 +1190,7 @@ export function HunkAnnotatedDiff({ const fileDiffOptions = useMemo( () => ({ diffStyle: layout, - ...pierreThemeOptions(resolvedTheme), + ...pierreThemeOptions(resolvedTheme, diffSyntaxTheme), disableBackground: true, disableFileHeader: true, ...diffAppearanceOptions({ diffIndicators, diffLineNumbers, diffWordHighlight }), @@ -1197,7 +1198,7 @@ export function HunkAnnotatedDiff({ enableLineSelection: true, onLineSelected, }), - [layout, resolvedTheme, diffIndicators, diffLineNumbers, diffWordHighlight, onLineEnter, onLineSelected], + [layout, resolvedTheme, diffSyntaxTheme, diffIndicators, diffLineNumbers, diffWordHighlight, onLineEnter, onLineSelected], ); async function run(meta: BlockMeta, direction: SliceDirection, target: ApplyTarget) { diff --git a/ui/src/views/MergeResolver.tsx b/ui/src/views/MergeResolver.tsx index 8d069da..f88b951 100644 --- a/ui/src/views/MergeResolver.tsx +++ b/ui/src/views/MergeResolver.tsx @@ -3,6 +3,7 @@ import { File as PierreFile } from '@pierre/diffs/react'; import { Icon } from '../components/Icon'; import { EDITABLE_SELECTOR, eventInside } from '../lib/keys'; +import { pierreThemeOptions } from '../lib/pierreTheme'; import { errMessage, tauri } from '../lib/tauri'; import { useRepo } from '../stores/repo'; import { useSettings } from '../stores/settings'; @@ -26,7 +27,9 @@ export function MergeResolver({ path, onClose }: { path: string; onClose: () => const activePath = useRepo((s) => s.activePath); const oursBranch = useRepo((s) => s.meta?.branch ?? 'HEAD'); const resolveConflict = useRepo((s) => s.resolveConflict); - const pierreTheme = useSettings((s) => s.resolvedTheme) === 'light' ? 'pierre-light' : 'pierre-dark'; + const resolvedTheme = useSettings((s) => s.resolvedTheme); + const diffSyntaxTheme = useSettings((s) => s.diffSyntaxTheme); + const pierreTheme = pierreThemeOptions(resolvedTheme, diffSyntaxTheme); const [raw, setRaw] = useState(null); const [loadError, setLoadError] = useState(null); @@ -161,7 +164,7 @@ export function MergeResolver({ path, onClose }: { path: string; onClose: () => side: 'theirs' | 'ours' | 'result', onPick?: (index: number) => void, ) => ({ - theme: pierreTheme, + ...pierreTheme, disableBackground: true, disableFileHeader: true, onLineClick: (p: { lineNumber: number }) => { diff --git a/ui/src/views/Review.tsx b/ui/src/views/Review.tsx index 401a47e..088ca11 100644 --- a/ui/src/views/Review.tsx +++ b/ui/src/views/Review.tsx @@ -19,11 +19,19 @@ import { import { EDITABLE_SELECTOR, eventInside } from '../lib/keys'; import { hashFileDiff as hashOf } from '../lib/patch'; import { matchTarget, scrollToDiffLine, type DiffLineTarget } from '../lib/diffJump'; +import { aiCoverageLabel, aiRequestMatches, otherAiProvider } from '../lib/aiGeneration'; import { useSettled } from '../lib/useSettled'; import { concatPatches, patchesToMarkdown } from '../lib/patchExport'; import { buildReviewFeedback, collectFeedbackFiles } from '../lib/reviewExport'; -import { gitErrorHint } from '../lib/tauri'; -import type { FileDiff } from '../lib/types'; +import { AI_AUTH_REQUIRED, gitErrorHint, isCancelled, tauri } from '../lib/tauri'; +import type { + AiInputCoverage, + AiProvider, + AiSensitiveDecision, + AiSensitiveFile, + CodeReviewFinding, + FileDiff, +} from '../lib/types'; import { useRepo } from '../stores/repo'; import { useSettings } from '../stores/settings'; import { treeFileOrder } from '../lib/treeOrder'; @@ -38,9 +46,9 @@ import { HunkAnnotatedDiff, scrollDiff, stepChangeBlock } from './LocalChanges'; * * Two modes, decided by whether a baseline is pinned: * - * - **Inbox** (no baseline): the review set is the *unstaged* changes. - * Accepting (staging) a file removes it from the inbox; diffs keep their - * per-hunk Stage / Discard actions. + * - **Inbox** (no baseline): the review set is every uncommitted change, + * staged + unstaged. Staging never removes work from review; safe unstaged- + * only diffs keep their per-hunk Stage / Discard actions. * - **Session** (baseline pinned at a commit): the review set is everything * since that commit — committed + staged + unstaged — so an agent that * commits as it goes can't slip work past the review. Diffs render @@ -62,9 +70,11 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri const reviewNotes = useRepo((s) => s.reviewNotes); const addReviewNote = useRepo((s) => s.addReviewNote); const removeReviewNote = useRepo((s) => s.removeReviewNote); + const addAiReviewFindings = useRepo((s) => s.addAiReviewFindings); const activePath = useRepo((s) => s.activePath); const meta = useRepo((s) => s.meta); const setBaseline = useRepo((s) => s.setBaseline); + const setBranchBaseline = useRepo((s) => s.setBranchBaseline); const clearBaseline = useRepo((s) => s.clearBaseline); const refreshReviewDiffs = useRepo((s) => s.refreshReviewDiffs); const stageReviewed = useRepo((s) => s.stageReviewed); @@ -76,6 +86,11 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri const setView = useRepo((s) => s.setView); const stagedDiffs = useRepo((s) => s.stagedDiffs); const diffMode = useSettings((s) => s.diffMode); + const aiProvider = useSettings((s) => s.aiProvider); + const openaiModel = useSettings((s) => s.openaiModel); + const anthropicModel = useSettings((s) => s.anthropicModel); + const openaiCli = useSettings((s) => s.openaiCli); + const anthropicCli = useSettings((s) => s.anthropicCli); const layout = diffMode === 'split' ? 'split' : 'unified'; // The pool only auto-refreshes while this view is open (or a baseline is @@ -86,6 +101,7 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri const sessionMode = baseline != null; const pool: FileDiff[] = sessionMode ? baselineDiffs : reviewUnstagedDiffs; + const aiReviewKey = useMemo(() => reviewPoolKey(baseline?.oid ?? null, pool), [baseline, pool]); // Review state per file, derived once per pool/marks change. type Verdict = 'pending' | 'reviewed' | 'stale'; @@ -249,9 +265,191 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri return () => clearTimeout(t); }, [notice]); + const pinBaseline = useCallback(() => { + const action = sessionMode ? setBaseline() : setBranchBaseline(); + void action + .then((hit) => { + if (hit) setNotice(`Reviewing from the fork point with ${hit.name}.`); + }) + .catch(fail(sessionMode ? 'Move baseline' : 'Find branch start')); + }, [sessionMode, setBaseline, setBranchBaseline, fail]); + + // ── AI code review ─────────────────────────────────────────────────── + const [reviewingWithAi, setReviewingWithAi] = useState(false); + const [aiError, setAiError] = useState(null); + const [aiCoverage, setAiCoverage] = useState<{ + coverage: AiInputCoverage; + provider: AiProvider; + } | null>(null); + // Provider output stays transient until the reviewer explicitly accepts a + // finding. Running AI review never edits files or persists review notes. + const [pendingAiFindings, setPendingAiFindings] = useState([]); + const [aiSensitivePrompt, setAiSensitivePrompt] = useState<{ + fingerprint: string; + files: AiSensitiveFile[]; + } | null>(null); + const [aiRetryProvider, setAiRetryProvider] = useState(null); + const aiRequestRef = useRef<{ + opId: string; + path: string; + provider: AiProvider; + target: string; + model: string; + } | null>(null); + const reviewingWithAiRef = useRef(false); + + const cancelAiReview = useCallback(() => { + const request = aiRequestRef.current; + aiRequestRef.current = null; + reviewingWithAiRef.current = false; + setReviewingWithAi(false); + if (request) void tauri.repoCancelOp(request.opId); + }, []); + + useEffect( + () => cancelAiReview, + [ + activePath, + aiProvider, + openaiCli, + anthropicCli, + openaiModel, + anthropicModel, + aiReviewKey, + cancelAiReview, + ], + ); + + useEffect(() => { + setPendingAiFindings([]); + setAiCoverage(null); + }, [aiReviewKey]); + + const runAiReview = useCallback( + async ( + sensitiveDecision: AiSensitiveDecision = { mode: 'scan' }, + provider: AiProvider = aiProvider, + ) => { + const repoState = useRepo.getState(); + const requestPool = repoState.baseline + ? repoState.baselineDiffs + : repoState.reviewUnstagedDiffs; + if (!repoState.activePath || reviewingWithAiRef.current) return; + if (requestPool.length === 0) { + setNotice('Nothing changed — there is no code to review.'); + return; + } + const opId = `ai-review-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const model = provider === 'openai' ? openaiModel : anthropicModel; + const request = { + opId, + path: repoState.activePath, + provider, + target: currentReviewPoolKey(repoState), + model, + }; + aiRequestRef.current = request; + reviewingWithAiRef.current = true; + setReviewingWithAi(true); + setAiError(null); + setAiCoverage(null); + setPendingAiFindings([]); + setAiSensitivePrompt(null); + setAiRetryProvider(null); + try { + const outcome = await tauri.repoReviewChanges( + request.path, + repoState.baseline?.oid ?? null, + provider, + model, + { opId, sensitiveDecision, styleInstruction: null }, + openaiCli, + anthropicCli, + ); + const current = useRepo.getState(); + if ( + aiRequestRef.current !== request + || !aiRequestMatches(request, { + path: current.activePath ?? '', + provider, + target: currentReviewPoolKey(current), + }) + ) return; + if (outcome.status === 'needs_confirmation') { + setAiSensitivePrompt({ + fingerprint: outcome.fingerprint, + files: outcome.sensitiveFiles, + }); + return; + } + if (outcome.provider !== provider) return; + setPendingAiFindings(outcome.suggestion.findings); + setAiCoverage({ coverage: outcome.coverage, provider: outcome.provider }); + const count = outcome.suggestion.findings.length; + const providerLabel = provider === 'openai' ? 'Codex' : 'Claude Code'; + setNotice( + count === 0 + ? `${providerLabel} found no actionable issues in the included changes.` + : `${providerLabel} found ${count} possible issue${count === 1 ? '' : 's'} for you to review.`, + ); + } catch (error) { + if (aiRequestRef.current !== request || isCancelled(error)) return; + const message = gitErrorHint(error); + if (message.startsWith(AI_AUTH_REQUIRED)) { + try { + await tauri.aiProviderLogin(provider, openaiCli, anthropicCli); + setAiError('Sign-in started — complete it in the browser or CLI window, then review again.'); + } catch (loginError) { + setAiError(`Sign-in failed: ${gitErrorHint(loginError)}`); + } + } else { + setAiError(`AI review failed: ${message}`); + setAiRetryProvider(otherAiProvider(provider)); + } + } finally { + if (aiRequestRef.current === request) { + aiRequestRef.current = null; + reviewingWithAiRef.current = false; + setReviewingWithAi(false); + } + } + }, + [ + aiProvider, + openaiModel, + anthropicModel, + openaiCli, + anthropicCli, + ], + ); + + const acceptAiFindings = useCallback((findings: CodeReviewFinding[]) => { + if (findings.length === 0) return; + addAiReviewFindings(findings); + const accepted = new Set(findings); + setPendingAiFindings((current) => current.filter((finding) => !accepted.has(finding))); + setNotice( + `Added ${findings.length} AI finding${findings.length === 1 ? '' : 's'} as review ` + + `note${findings.length === 1 ? '' : 's'} — no files were changed.`, + ); + }, [addAiReviewFindings]); + + const dismissAiFindings = useCallback((findings: CodeReviewFinding[]) => { + const dismissed = new Set(findings); + setPendingAiFindings((current) => current.filter((finding) => !dismissed.has(finding))); + }, []); + + useEffect(() => { + const onRequest = () => { + void refreshReviewDiffs().then(() => runAiReview()); + }; + window.addEventListener('strand:review-with-ai', onRequest); + return () => window.removeEventListener('strand:review-with-ai', onRequest); + }, [refreshReviewDiffs, runAiReview]); + const displayedNotes = displayed ? (reviewNotes[displayed.path] ?? []) : []; // The export is the UNION of pool files with notes and noted paths that - // left the pool (staged away in inbox mode, …) — a stored note must never + // left the pool (committed/reverted in inbox mode, …) — a stored note must never // silently drop from the feedback. Counts follow the same union. const feedbackFiles = useMemo( () => collectFeedbackFiles(pool, reviewNotes), @@ -529,14 +727,14 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri when={when} reviewedCount={0} total={0} - onPin={() => void setBaseline()} + onPin={pinBaseline} onClear={() => void clearBaseline()} />
{sessionMode ? 'Session is clean' : 'Nothing to review'} {sessionMode ? `No changes since ${baseline!.short}. Let the agent work — this view follows along live.` - : 'No unstaged changes. Pin a baseline before an agent session to also track what it commits.'} + : 'No uncommitted changes. Start a branch baseline to review its committed work too.'}
); @@ -550,10 +748,32 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri when={when} reviewedCount={pool.length - pendingCount} total={pool.length} - onPin={() => void setBaseline()} + onPin={pinBaseline} onClear={() => void clearBaseline()} extra={ <> + + {reviewingWithAi && ( + + )} {noteCount > 0 && ( + + + + ) : aiError ? ( + <> + {aiError} + {aiRetryProvider && ( + + )} + + + ) : pendingAiFindings.length > 0 ? ( + <> + + {pendingAiFindings.length} suggested issue{pendingAiFindings.length === 1 ? '' : 's'} + {' · '}review before adding as notes; no files are changed + {aiCoverage ? ` · ${aiCoverageLabel(aiCoverage.coverage, aiCoverage.provider)}` : ''} + + + + + ) : aiCoverage ? ( + {aiCoverageLabel(aiCoverage.coverage, aiCoverage.provider)} · no files were changed + ) : null} +
+ {pendingAiFindings.length > 0 && !aiSensitivePrompt && !aiError && ( +
+ {pendingAiFindings.map((finding, index) => ( +
+ {finding.severity} +
+
{finding.title}
+
{finding.body}
+ +
+
+ + +
+
+ ))} +
+ )} +
+ )} +
@@ -651,7 +982,7 @@ export function Review({ onOpenFileInEditor }: { onOpenFileInEditor: (file: stri )} - {sessionMode && !unstagedSet.has(displayed.path) && stagedSet.has(displayed.path) && ( + {stagedSet.has(displayed.path) && ( {sessionMode && (
- Unstaged changes + Uncommitted changes 4/8 reviewed - +
diff --git a/website/script.js b/website/script.js index 5ba8b30..590a0b7 100644 --- a/website/script.js +++ b/website/script.js @@ -236,9 +236,9 @@ }); $('#baseline-link').addEventListener('click', () => { sessionMode = !sessionMode; - $('.rv-chip', mock).lastChild.textContent = sessionMode ? 'Session since 9c4e7a1 · now' : 'Unstaged changes'; - $('#baseline-link').textContent = sessionMode ? 'Clear baseline' : 'Pin baseline at HEAD'; - showDemoToast(sessionMode ? 'Baseline pinned at HEAD' : 'Baseline cleared'); + $('.rv-chip', mock).lastChild.textContent = sessionMode ? 'Session since 9c4e7a1 · now' : 'Uncommitted changes'; + $('#baseline-link').textContent = sessionMode ? 'Clear baseline' : 'Review from branch start'; + showDemoToast(sessionMode ? 'Baseline pinned at branch fork point' : 'Baseline cleared'); }); discardLink.addEventListener('click', () => { if (!discardArmed) { @@ -384,8 +384,8 @@ b.setAttribute('aria-pressed', String(on)); }); crumbLeaf.textContent = scope === 'workspace' ? 'Workspace Review' : 'Review'; - crumbNote.textContent = scope === 'workspace' ? '· 2 repos + 1 worktree · 8 files to review' : '· 8 unstaged files'; - $('.rv-chip', mock).lastChild.textContent = scope === 'workspace' ? 'Workspace changes' : (sessionMode ? 'Session since 9c4e7a1 · now' : 'Unstaged changes'); + crumbNote.textContent = scope === 'workspace' ? '· 2 repos + 1 worktree · 8 files to review' : '· 8 uncommitted files'; + $('.rv-chip', mock).lastChild.textContent = scope === 'workspace' ? 'Workspace changes' : (sessionMode ? 'Session since 9c4e7a1 · now' : 'Uncommitted changes'); }); });