feat: Project View — review, commit, and merge in the main window - #11
Open
ajcarberry wants to merge 23 commits into
Open
feat: Project View — review, commit, and merge in the main window#11ajcarberry wants to merge 23 commits into
ajcarberry wants to merge 23 commits into
Conversation
…c branch Backend half of the review/merge view, extracted from feat/miniplayer-agentic-development at its final state (cherry-picking was infeasible — the feature's fixes are interleaved with Mission Control and workspace work). All surfaces are keyed on plain repositoryPath; the agentic seams were severed: - merge-registry keeps only the in-flight-abort half (the landing-record half existed for workspace banding, which stays behind) - DiffService drops workspaceDirtyStats (only the workspace model used it) - ReviewOpenRequest.workspacePath is renamed to repositoryPath — it was always just the repository path every diff/status/stage IPC call targets - IPC_CHANNELS carries only the diff/merge/review groups lore-repository is refactored into lore-operation/lore-status/ lore-repository-info collaborators, commit now resolves the committed revision (MergeService records it), and file status carries the SDK's conflict flags. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…reload bridge The review window is a per-repository secondary BrowserWindow (frameless, sandboxed, hardened like the main window) opened via review:open with its workflow + compare selection preloaded. Re-opening the same repository re-targets and focuses the existing window; switching workflow or closing the window aborts an in-flight merge so it is never stranded on disk. The renderer pulls its open request over review:requestContext, identified by webContents — no repository id crosses the query string. The preload bridge gains diff/review/merge groups, and commit's bridge typing now reflects the resolved revision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports the full review surface from the agentic branch: the window root routes its preloaded open request to the commit workflow (compare picker, file list with stage/unstage and conflict flags, unified diff pane, commit bar with push) or the merge workflow (per-file accept-mine/accept-theirs conflict resolution, merging-commits and conflicts ledger sidebar, abort confirmation, landed banner). Adds the shared secondary-window bootstrap (ThemeModeLoader extracted from App), SectionLabel, pluralize, the review.html vite entry, and the review test suites. Severed from the agentic source: the IntentionPanel (agent-intention sidebar) and its snapshot plumbing, and Mission Control references in user-visible copy — the merge bar and landed banner no longer point at a window that does not exist on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports the diff/merge integration suites: DiffService.compare against real commits (revision→revision, revision→working tree, binary sentinel, truncation with pre-truncation lineStats), the merge landing workflows (clean land, mine/theirs conflict resolution, already/externally landed, ahead-with-nothing-to-reconcile, abort restore), and the merge failure arcs (refused landings, staged pre-flight, stale on-disk merges, moved targets, imported files). The agentic suite's workspaceDirtyStats test is not ported — that method stayed behind with the workspace model. world.ts gains secondClient.syncAndRead for asserting landings reach another client. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrites the agentic branch's live specs to drive the review window from the card's WorkingSet-header Review/Merge actions instead of Mission Control. The commit spec covers the compare picker (revision→revision and →working tree), one file row per change kind with badges and binary sentinel, staging that survives refetches, and a commit+push a second independent client really sees; the conflict spec reaches a real flagConflictUnresolved through a pending merge after sync and asserts the ⚠ no-staging treatment on both the card and the review file list. The merge spec arranges branch work out-of-band through the real CLI under the app's HOME (an unpushed branch commit, as an agent would leave it), then proves both conflict resolutions land the right content on main via a second client, plus the abort arc restoring the working tree. Also ports the card working set's conflict rows (⚠ replaces the stage checkbox, "conflicts with rN" message), the review-workflow routing spec, the review-window ui.ts helper layer with a card-driven opener, the homeDir fixture, and flips the divergence-and-conflict integration todo into a real assertion — the conflict-visibility gap is fixed by the ported status mapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the review window's live scenarios to the e2e coverage index and the scenario catalog (commit workflow, conflict treatment, merge mine/theirs/ abort, mocked routing), catalogs the diff-compare and merge integration suites, records the conflict-visibility finding as fixed and the pre-existing branch-switcher selection race as a new known finding, and adds one-line CLAUDE.md entries for the diff/merge operations and the review window's channels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The inline merge-target/opener logic pushed SyncView over the complexity limit and read branchGraph.current as a ref in hook deps; extracting it as a pure, tested helper (the file's buildTransportProps pattern) resolves both and drops the useCallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review appears only while the working set has dirty files — a clean checkout has nothing to review. Merge appears only when the target is a distinct branch AND the branch carries revisions the target lacks — the merge service's own ancestry predicate (hasRevisionsToLand), exposed over a new lore:revisionsToLand channel and read by useRevisionsToLand, keyed on branch, target, and the branch's tip revision so the landing's own merge commit withdraws the entry (in-sync, landed here, or landed by another client all read the same). Unknown state — loading, failure — withholds Merge rather than offering one that could land nothing. The live specs assert the gates end-to-end: a fresh clone offers no Review entry until the working set reads dirty, the Merge entry appears only once the predicate holds, and it withdraws after the merge lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eview window The review/merge surface — now named the Project View — is a third morph stage of the single ambient window (pill → card → Project View), mirroring the pill ↔ card morph, instead of a separate BrowserWindow. A new window:setView IPC grows the window to the review footprint (anchored like the card morph, capped and clamped to the work area), drops always-on-top while the view is open, and restores the card's exact bounds on exit. The renderer crossfades the surfaces; the post-fade steps (hiding the card from the accessibility tree, shrinking the window) are sequenced by a renderer timer rather than a CSS transition delay, because transition clocks stall in the transparent window right after the resize (observed live on macOS). Ways in and out: the WorkingSet header's gated Review/Merge actions and a new always-visible footer icon (left of Open in File Explorer) open the view; the workflow headers' Back returns to the card, and the TitleBar collapse control drops straight to the ambient pill. Backing out of a live merge routes through the existing discard confirmation; collapsing past it backs the merge out fire-and-forget (the old window-close semantics). Gone with the separate window: review.html and its vite entry, the review:* channels and preload group, registerReviewWindow and the secondary-window recipe, useReviewContext, bootstrapWindow, and merge-registry (its abort-on-close consumer no longer exists; stale on-disk merges remain covered by the restart arcs). MiniPlayer's morph and repository-mutation logic moved into useProjectViewMorph and useRepositoryMutations to stay inside the size limits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hitecture One conflict, in the e2e coverage index: main reformatted the table while this branch added the Project View rows — resolved by appending the rows to the reformatted table. The hidden-launch mode, parallel projects, and focus-project split compose cleanly with the Project View morph: setView never shows the window, and the morph's post-fade sequencing is timer-driven, which backgroundThrottling: false keeps honest on hidden windows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code comments now state only present-tense constraints: the packet/design citations and probe narratives that traveled with the agentic port are gone (the decision history lives in docs/ and PR bodies), agentic-era "workspace" wording reads as checkout/source branch, and the surface is consistently the Project View. Also removes getSessionCommits — its only consumer was the workspace model, which never existed on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Under the parallel harness a just-closed sibling menu's DOM can linger through its close transition, briefly duplicating revision items across the source and target dropdowns; portals mount in open order, so .last() addresses the menu that was actually opened. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… lanes The constellation's branch-point anchor paired the parent's branchPoint with the child lane's OLDEST node — but assembleBranchGraph gives the child lane its full lineage, so the oldest node is the repo root, not the fork. A fresh branch off main's tip also collapsed the parent lane to a single node, because parentWindow sliced away everything past the branch point and nothing remained when main had not advanced. Together they drew the branch as forking from the wrong main revision. The anchor now targets the child lane's own copy of the branch-point revision (falling back to the oldest node only when the branch point is outside the walked window), the parent trunk renders when the fork is the parent's tip, and the fork connector drops vertically at the shared x. Applied from the sibling branch's 5f8602d, whose test fixtures are also production-shaped (full child lineage) — the disjoint fixtures were what let this pass. A live assertion in the merge arrangement (exactly the fresh-branch repro) pins the vertical connector in the real app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A dedicated live-timeline spec drives the exact regression arc — two commits on main, then a branch created at the tip with one commit of its own — and asserts WHICH nodes align: the parent lane renders both trunk revisions, and the fork connector drops vertically from the parent's branch-point node to the child lane's copy of that same revision, not the repo root. Two pre-fork commits are load-bearing: with one, root == fork and a mis-anchored fork is indistinguishable. Proven discriminating by running it against the pre-fix renderer (parent lane collapses to one node; the alignment assertions guard the anchor half). The connector probe added to the merge arrangement is removed: the layout forces anchored pairs to one x, so x1 === x2 held even under the bug — it asserted nothing. loreInClone moves to support/ui.ts for reuse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…switcher The WorkingSet header now offers a single entry — Review when the working set is dirty, Merge only when it is clean with revisions to land — never both. Inside the view, a Review | Merge segmented switcher in the workflow headers crosses between the two, rebuilding the open request from the card's live context. The Merge segment gates on everything the merge actually requires: a distinct target with revisions to land, and nothing staged — the merge pre-flight refuses staged files, and that error is now unreachable from the UI (staged files imply a dirty working set, whose card entry is Review; the commit view's segment disables live as files are staged and re-enables as they unstage). Leaving a live merge through the switcher routes through the same discard confirmation as Back and Abort — one useMergeLeave lifecycle owns all three — and lands in the commit view rather than the card. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ive re-opens Two data-integrity fixes for the merge arc. Aborting a merge resets the working tree wholesale — probed live: unstaged edits revert and untracked files are DELETED. The pre-flight only refused staged files, so a merge started over unstaged or untracked work made every abort path (explicit, failure back-outs, stale-merge cleanup) destructive. requireMergeableCheckout now refuses ANY uncommitted work by name before anything materializes, and the commit view's Merge segment gates live on the same condition, so the refusal is unreachable from the UI. The card ↔ view morph remembered the card's bounds on EVERY projectView request, so the workflow switcher's re-open overwrote them with the review footprint and exiting "restored" a review-sized window. Only the first card → view transition stores bounds now. The abort e2e asserts the window's real OS bounds return to the exact pre-open footprint after an in-view switch — proven discriminating against the reverted fix (width stayed 1220 instead of 360). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d collapse, leaner contracts From a clean-context adversarial review of the branch: - The merge header and "Merging commits" sidebar counted the branch's entire walked lineage — trunk included — as work the merge lands. Both now show only the commits above the fork (the graph's branch point), falling back to the full list only when no fork resolves. - The TitleBar collapse discarded a live merge fire-and-forget, bypassing the confirmation every other exit gets. Each workflow view now owns its TitleBar, so the merge routes collapse through the same discard confirmation as Back, Abort, and the switcher (one leave lifecycle, three destinations). - window:setView now validates through a Zod schema in validators.ts — the standards allow exactly one non-Zod channel and this was a second. - ReviewOpenRequest carries repositoryName and targetBranch, deleting the view's repository-list refetch, the raciest un-cancelled effect, and an impossible-state 'main' fallback; the footer opener reuses the workflow switch instead of rebuilding the request; start() holds an in-flight guard across its awaits; the morph timer clears on unmount; dead lore-status exports and ported-comment residue (garbled sentences, phantom consumers, packet IDs, fileDump references) are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two path-mapping tests asserted POSIX literals against node:path output, which joins with backslashes on Windows — the service is platform-correct, the expectations were not. First Windows CI run for these ported tests (the source branch had no Windows coverage). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Landings through branchMergeInto advance only the target's remote tip, so walking the parent lane from the local tip hid every landed merge — the graph showed no landing, no merge links, and a stale main until some later sync made them pop in with the mirror-image commit message. Walk from the remote-aware tip (the rule lore-status already applies), degrading to the local tip when the remote tip is unreadable or its revisions were never fetched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh
Every revision now renders on the lane of the branch it was committed on: the shared pre-fork trunk rides the parent lane (which no longer elides its history at the fork), the child lane sprouts at the branch point, and trunk ledger rows stay selectable on the parent lane with selection/current markers routed by hash. Previously the trunk rode the child lane, misattributing the parent's commits to the feature branch. Merge connectors now run between their two real nodes even when the layout cannot give them one x — diagonal, arrowhead rotated to match the slope — instead of degrading to a floating label; labels remain only for sources outside the walked window. Merge anchors win ties with the fork anchor, since the fork connector reads fine diagonal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh
Adversarial review of this branch found layers that guessed where the SDK records the answer. Every guess is replaced by the recorded field, and every agentic-branch port with no consumer here is deleted: - targetRevision streams from the merge's own BRANCH_MERGE_START_BEGIN event; the follow-up branchInfo read (which raced an advancing target and could diff never-merged content) is gone. - The stale-merge guard triggers on revisionMerged — non-zero exactly when a merge is pending (probed live) — instead of inferring from status-row shapes, and refuses by name rather than resetting the tree when the user staged their own files on top of a pending merge. - flagMerged is mapped and drives the unrelated-staged guard; the "everything staged after start is the merge's import" timing subtraction is deleted. The live probe disproved the old comment: target-added imports DO carry flagMerged; user rows do not. - branchHead compare targets resolve the remote-preferring tip (the rule every other tip consumer applies), retiring MergeView's stale-local-tip workaround. - The target-advanced/no-merge patterns match the SDK's own error text before wrapping — a branch named with the refusal's words no longer discards a retryable merge (pinned by test). - Deleted dead weight: lore-repository-info + wrapper, userId notification plumbing, divergence derivation with no reader, branchTip, IPC_CHANNELS const, forkChildX, repositoryId, and two duplicated helpers (ownCommits, distinctStatusPaths consolidate). Net app-source delta: −160 lines. New pins: a live A2-restart-protect arc proving user bytes survive, two live seams (merge→timeline connector, text hunk render), and the false-positive unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh
The start effect was the one async path in the Project View that never settled its busy flag in a `finally`: a rejection from the merge bridge left `starting` true, so the view held its loader forever with no way to reach the merge or clear it. A rejection now lands in the existing start-error state, which already carries the Abort that clears a stranded merge. Loading both sides of a conflict gets its own guard — that failure is notified rather than fatal, since the merge is live on disk by then. The effect moves into a useMergeStart hook beside useMergeLeave; the component body was already at the max-lines ceiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh
The `// --- section ---` dividers appeared only in this branch's files; the rest of the codebase uses none, so they read as a foreign hand. isMergeFile has no caller outside lore-status. The pluralize example cited "workspace", a term this branch retired for "checkout". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
window:setViewgrows the window to the review footprint (anchored, work-area-clamped), drops always-on-top while open, and restores the card's exact bounds on exit.lore:revisionsToLandancestry predicate) plus an always-visible footer icon. An in-view Review|Merge switcher crosses workflows; its Merge segment disables while any work is uncommitted — merge aborts reset the working tree (probed live: unstaged edits revert, untracked files are deleted), so the service also refuses to start over any dirt, by name.flagConflict*mapped into file status; ⚠ rows on the card and review list).Hardening worth calling out
Merge is the destructive surface here, so most of the review effort went into it:
BRANCH_MERGE_START_BEGINevent rather than a follow-up branch-tip query, so a target advancing mid-merge cannot race it. A target that advanced anyway fails with an actionable error and leaves the source-branch merge commit intact.window:setViewis Zod-validated at the IPC boundary; a rejected merge-start surfaces as an error state instead of stranding the view on its loader.Validation
pnpm claude:pre-commit: exit 0 — types, format, lint, Jest 714/714 (coverage thresholds held), Playwright 31 expected / 0 unexpected / 0 flaky (hidden + parallel architecture from test: silent, parallel, worktree-safe Playwright e2e architecture #10)loreserver— a landed merge confirmed from a second independent clone, both conflict resolutions landing the correct side, abort restoring the pre-merge file, and each guard aboveNotes
feat/miniplayer-agentic-developmentand reduced to this branch's scope; expect deliberate divergences there (workspacePath→repositoryPath, no workspace/agentic surfaces).mainhas pre-existing Prettier drift, and the format gate rewrites those files on touch.useBranches.deriveCurrentBranch) — see the scenario catalog's Known findings.🤖 Generated with Claude Code
https://claude.ai/code/session_01S3DHYFcTpNYY7vyudMxrBh