Refactor Git Changes workflow and diff handling#311
Refactor Git Changes workflow and diff handling#311VooDisss wants to merge 31 commits intoNeuralNomadsAI:devfrom
Conversation
Introduce a local Git Changes view model at the right-panel boundary instead of letting the UI depend directly on the raw OpenCode SDK file status payload. This keeps the current flat Git Changes behavior intact while creating a stable seam for the staged-vs-unstaged work that follows. The runtime probe confirmed that file.status() currently only returns path, added, removed, and a single status value. Normalizing that payload in one place reduces coupling in RightPanel and GitChangesTab, lets future commits enrich the model without another cross-cutting rename, and preserves the existing viewer behavior as a safe checkpoint before backend Git-status changes.
Add a CodeNomad-side Git status endpoint for worktree-aware source control state so the Git Changes panel can evolve beyond the flat SDK payload returned by file.status(). The current OpenCode response only exposes path, added, removed, and a single status value, which is not enough to model staged and unstaged sections correctly. This change resolves worktree slugs to real directories on the server, reads staged, unstaged, and untracked paths directly from Git, and threads that richer state into the existing right-panel model without changing the visible tab yet. Keeping the UI flat in this checkpoint gives us a safe place to validate the new data path before the next commit starts splitting selection and rendering by staged versus unstaged entries.
Update the Git Changes panel to select list items by a staged-or-unstaged section identifier instead of raw file path alone. This prepares the panel for a file to appear more than once when both the index and working tree differ, which the upcoming grouped source-control view requires. The visible UI remains largely unchanged in this checkpoint, but the right-panel state and list model now distinguish staged:path from unstaged:path internally. Landing that identity change on its own keeps the staged/unstaged grouping work smaller and reduces the risk of mixing data-model, selection, and rendering changes in one commit.
Rework the Git Changes side panel into a source-control style view with separate Staged Changes and Changes sections, collapse state persistence, and section-aware row identity. This makes the panel behave much closer to VS Code and gives users the missing distinction between what is already in the index and what is still only in the working tree. The follow-up fixes in this checkpoint are what make the grouping correct rather than merely cosmetic. CodeNomad now fills in staged-only files that the proxied SDK status payload omits, normalizes Windows path separators before merging SDK and server Git data, and carries section-specific line counts so staged and unstaged rows no longer share the same totals. Those fixes were necessary to eliminate duplicate rows and zero-count staged entries on Windows.
Make Git Changes row selection load the correct before and after content for staged and unstaged entries instead of routing everything through the generic SDK file.read path. The old approach had no reliable concept of index-versus-working-tree scope, which caused wrong diffs once the panel started showing the same path in separate staged and unstaged sections. This checkpoint adds a dedicated worktree diff endpoint on the CodeNomad server, reads index blobs directly with git cat-file, and handles untracked files as a first-class case when deriving both diff content and line counts. Those changes fix the .opencode/package-lock.json failure, preserve the correct staged-vs-unstaged markdown behavior, and remove the brittle reverse-patch reconstruction path from the Git Changes viewer.
Refine the Git Changes list to use filename-first rows with a secondary parent-path label and much tighter spacing so the panel scans more like a source-control outline instead of a full-path browser. This also threads the active session prompt API through the instance shell so Git-related prompt insertion can be driven from the right panel without introducing a new global store. The prompt insertion affordance is still in an intermediate state: the plumbing now supports inserting HTML comment context from the active Git selection, and the diff viewer reports modified-editor line selections, but the final gutter-based interaction will land in the next checkpoint. Splitting that Monaco-specific UI work from the compactness pass keeps the current commit reviewable while preserving the integration work already needed for the follow-up feature.
Replace the temporary header-driven prompt insertion affordance with a line-anchored control in the Git diff viewer so adding Git change context feels local to the code the user is inspecting. Hovering a line now exposes the action at the modified-editor seam, and selecting a range reuses the same control to insert the file path plus selected line span into the prompt. The final implementation intentionally avoids Monaco’s built-in glyph and content-widget placement for the visible button because those approaches produced unstable alignment, clipping, and hover-loss in this split diff layout. Instead, the control is rendered in a viewer overlay and positioned from the modified editor’s actual DOM metrics and Monaco line geometry, which gives deterministic seam placement and preserves clickability.
Preserve deleted files in the staged and unstaged Git Changes sections and let the server diff contract render their before/after states instead of hiding them or rejecting selection in the panel. This closes the last real correctness gap in the grouped source-control view, where deletion states previously disappeared from the new UI even though they are part of normal git workflows. This checkpoint also finishes the seam-anchored prompt affordance cleanup by removing an unused overlay ref and routing the button label through the existing translation key instead of hardcoded English. The remaining implementation stays intentionally focused on exact git-state visibility and the prompt-context workflow already agreed for this feature.
Tighten the Git Changes panel around the last gatekeeper concerns by making stale selection recovery explicit, failing visibly when the richer worktree git status cannot be loaded, and restoring a clear binary-file guard in the server-backed diff path. The grouped staged/unstaged view depends on exact server data for correctness, so quietly falling back to aggregate SDK status or asking for diffs with invalid selections would leave the UI in misleading or broken states. This checkpoint keeps the grouped source-control model honest under refreshes and stage/unstage transitions, and prevents the new diff endpoint from trying to render binary content as UTF-8 text. Those fixes are intentionally narrow and do not broaden the public behavior beyond what the feature already promises.
This checkpoint closes the last small correctness gaps in the staged/unstaged Git Changes series without widening the feature surface. The header totals now derive from the same section-aware list items that the UI renders, which prevents the summary counts from drifting from the visible grouped rows after enrichment or section-specific transformations. It also hardens the server diff/status layer a bit further by making staged blob reads binary-aware and by using Git-style line counting semantics for untracked files. Those adjustments keep the grouped view and diff experience consistent with Git behavior while preserving the existing UI contract. The intent of this commit is cleanup and correctness, not new product behavior. It keeps the staged/unstaged feature cherry-pickable by finishing the series with narrowly scoped accuracy fixes instead of mixing in unrelated package or documentation changes.
This checkpoint introduces the server-backed mutation surface that the staged/unstaged Git Changes UI needs before any new buttons are added. CodeNomad now exposes worktree-scoped stage, unstage, and commit endpoints alongside the existing git-status and git-diff routes, and the UI API client can call those endpoints without yet changing visible behavior. The new mutation layer is intentionally separate from the existing git-status reader so read and write responsibilities stay narrowly scoped. The server contract stays minimal: stage and unstage return a simple success response, while commit optionally returns the new HEAD SHA. This keeps the endpoints easy to reason about and lets the UI continue to refresh from source-of-truth status data after each mutation instead of trying to maintain optimistic Git state locally. This commit also bakes in the gatekeeper feedback before any UI wiring lands. Mutation bodies now validate and normalize paths more strictly, reject absolute or traversal-like values, reject blank commit messages after trim, and preserve actionable Git failure messages instead of collapsing all SCM failures into a vague generic error. That should make the next UI checkpoint safer and easier to test because the mutation contract is already hardened before row actions and commit controls are introduced.
This checkpoint brings the first visible SCM mutation controls into the Git Changes tab without widening the feature beyond the already committed worktree mutation endpoints. Files in the Changes section now expose a per-row stage action, files in Staged Changes expose an unstage action, and the interaction stays local to each file so the UI remains compact and easy to test before adding commit composition. The implementation also solves the subtle selection problem introduced by the staged and unstaged split. Git row identities are section-qualified by path and section, so a file changes item identity when it moves between sections. RightPanel now remaps selection by path and section after each mutation refresh, preferring the moved entry when the file crosses sections and falling back to the best valid remaining item only when necessary. That preserves diff continuity instead of dropping or reopening the wrong file after stage or unstage. The row action affordance was tuned for readability during review: hover-only visibility keeps the list uncluttered, while the compact dark button with a high-contrast white glyph makes the action legible without introducing a new theme accent. This commit intentionally stops before adding the commit message box and Commit button so staging behavior can be validated in isolation as its own checkpoint.
This checkpoint completes the next logical step in the Git Changes workflow by adding a minimal commit composer to the staged section. The Git Changes tab now includes a worktree-scoped commit draft, a commit action embedded into the staged container, and the expected disabled-state rules so users cannot submit an empty message or commit when nothing is staged. The interaction stays intentionally narrow: it composes and submits a normal git commit, refreshes the grouped status from source of truth, preserves draft text on failure, and clears the draft only after a successful commit. The implementation also respects the UI behavior established earlier in the series. The commit draft is reset when the viewed worktree changes, the selected diff is revalidated after commit the same way it is after stage and unstage, and the compose affordance was shaped through review so it sits inside the staged section with a subtle overlaid button rather than adding more toolbar clutter. That keeps the source-control workflow compact while still leaving room for users to grow the message field when needed. This commit additionally closes the remaining binary-related inconsistencies discovered during testing. Binary Git diffs now use a dedicated binary-viewer message path so line-oriented prompt insertion is suppressed in that state, and untracked binary files no longer receive meaningless text line counts in the changes list. Together, those fixes keep binary files non-line-oriented both in the viewer and in list statistics, which better matches the actual capabilities of the feature.
This checkpoint fills in the non-English locale coverage for the Git Changes workflow that was added across the recent staged and unstaged SCM work. The new strings cover staged and unstaged section labels, stage and unstage actions, the staged commit compose box, and the dedicated binary-viewer message path so the feature no longer falls back to English outside the default locale. The intent here is consistency rather than new behavior. By updating the existing locale-specific instance message files directly, the commit keeps the i18n surface aligned with the current custom message merge system and avoids introducing feature-specific exceptions or English-only branches in the UI. This commit deliberately stays isolated to localization so the SCM workflow feature remains easy to review in layered checkpoints: backend mutation API, per-file stage and unstage actions, commit composition, and finally locale parity.
Refine the stage and unstage affordance in the Git Changes list so the action no longer permanently consumes horizontal space in every row. The control now reveals on row hover with a delayed hide, keeping the right edge visually compact while still making the action immediately available when the user intends to interact with a row. This change moves the interaction from a simple CSS opacity toggle to an explicit hover and linger model in GitChangesTab. The reveal area becomes clickable as soon as the row is hovered, the line-count stats shift only slightly and fade rather than jumping abruptly, and the action stays visible briefly after the pointer leaves so users can move into the control without racing the hover state. The button icon itself now uses deterministic centered bars instead of a text glyph. That avoids baseline-driven misalignment and makes the plus/minus symbol optically centered inside the control regardless of font metrics. The non-hover fallback still keeps the action visible on coarse-pointer layouts so staging and unstaging remain reliable outside desktop hover interactions.
Introduce a dedicated passive refresh path for the Git Changes tab so status can update automatically when the tab becomes active without reusing the heavier manual refresh flow for every passive update. The new path reloads status with a single-flight guard, ignores stale request completions, and only reopens the selected diff when the selected item's fingerprint has materially changed. That preserves the already-correct stage, unstage, and commit behavior while reducing unnecessary diff churn and preparing the tab for interval-based refresh in the next checkpoint. This keeps the design conservative: no new realtime infrastructure, no background watchers, and no refresh activity when the Git Changes tab is not the active visible view.
Add a low-frequency passive refresh loop for the Git Changes tab so status updates appear automatically while the tab is actively visible, without introducing background watchers or realtime infrastructure. The polling path reuses the previously added passive refresh flow rather than the heavier manual refresh logic. That keeps the refresh conservative: it refreshes status every 20 seconds only while the Git Changes tab is the active right-panel view, and it still avoids unnecessary diff reloads when the selected item's fingerprint has not changed. This preserves the cautious design goal for the feature: no hidden refresh work when the user is elsewhere, no push architecture, and no broad operational complexity beyond a small, explicit timer scoped to the active Git view.
Add a narrow workspace.filesChanged invalidation event so the Git Changes tab can refresh immediately after successful file-mutating agent tools complete, instead of waiting for the active-tab polling interval. The server now synthesizes this event from raw completed tool parts in the instance event bridge for write, edit, and apply_patch, and publishes it through the existing workspace event bus. The UI subscribes only while the Git Changes tab is active and responds by calling the existing passiveRefreshGitStatus path, preserving the conservative refresh model and avoiding a second Git refresh implementation. This approach was chosen over file-watchers or broader realtime infrastructure because the raw instance event stream already exposes the required completion signal. The implementation was tested in the running UI and confirmed to update Git Changes promptly for edit, write, and apply_patch without waiting for the 20-second polling fallback.
Close the remaining blocker-level issues in the post-9fb8573 Git Changes work by tightening refresh correctness rather than changing the accepted product design. This checkpoint fixes duplicate stage or unstage submission, stale diff responses overwriting newer selections, passive refresh restoring an older selection after the user has already moved on, and over-broad tool-driven invalidation that could refresh the wrong worktree. The server now resolves and emits a concrete worktreeSlug on workspace.filesChanged events when a completed write, edit, or apply_patch tool mutation is observed. The UI requires a positive worktree slug match before forcing a Git Changes refresh, so tool mutations in one worktree no longer invalidate another worktree tab by accident. For the active worktree, tool-driven invalidation now forces the selected diff to reload even when the coarse list fingerprint is unchanged, which keeps the viewer aligned with the updated file content. On the UI side, openGitFile now uses request versioning to ignore stale diff responses, passive refresh avoids restoring a stale selection when the user changes files mid-flight, and the row action only fires a single mutation per click. These fixes were re-reviewed by the same gatekeeper workers after implementation, and the resulting verdict is non-blocker.
Close the remaining blocker-level issues in the post-9fb8573 Git Changes series by making auto-refresh resilient under overlap and by containing best-effort invalidation failures on the server side. The Git Changes tab now queues one follow-up passive refresh when a completed write, edit, or apply_patch invalidation arrives during an active refresh instead of silently dropping that signal. The status loader also captures a stable worktree context and invalidates in-flight request versions on worktree reset, preventing stale or mixed worktree data from repopulating the tab after the user switches contexts. On the server, workspace.filesChanged publication now catches and logs async lookup failures so the invalidation bridge remains truly best-effort rather than risking an unhandled promise rejection. This checkpoint was validated with server and UI typechecks and is intended as the final hardening pass before one more gatekeeper review.
Close the remaining blocker-level issues in the post-9fb8573 Git Changes work by preserving the strongest pending refresh intent during overlapping auto-refresh activity, by making the tab-activation refresh discrete instead of self-reactive, and by hardening unstage behavior for fresh repositories with no HEAD commit yet. The Git Changes tab now merges queued passive refresh requests so forceReloadSelectedDiff survives when a completed write, edit, or apply_patch invalidation lands during an in-flight refresh. The activation-triggered refresh now runs only on actual git-changes tab or worktree activation transitions rather than being tied to reactive state touched during refresh, which avoids self-sustaining reload loops. On the server, unstageWorktreePaths now falls back to removing paths from the index on unborn branches so newly staged files can still be unstaged before the first commit. This checkpoint was validated with server and UI typechecks and is intended as the final blocker-fix pass before one more five-agent gatekeeper review.
Harden the git-diff endpoint so it enforces the same worktree-relative path boundary checks as the stage, unstage, and commit mutation paths. This closes the remaining server-side blocker where git-diff could previously accept absolute or traversing paths and broaden the read surface beyond the selected worktree. The worktree-relative path validator now lives as a shared helper in git-mutations and is reused by git-status when serving git-diff requests. This keeps the server surface consistent: both mutation and diff operations now reject invalid paths using the same normalization and traversal rules rather than relying on ad hoc slash cleanup in one place and stricter checks in another. This change was typechecked in both the server and UI packages. Rename handling was investigated through the UI and compared against VS Code behavior; because the current split add/delete presentation behaves comparably and no corruption was observed in stage or unstage flows, it is not being treated as a blocker in this checkpoint.
|
I want you to perform the gatekeeper checks for latest PR state check for regressions |
|
Gatekeeper pass on the latest PR state. I reviewed the current branch diff against I do not consider this ready to merge yet. Findings
Verification Notes I validated the rename findings with isolated local repos in this workspace shell:
Testing Gaps I could not run the repo typechecks in this workspace because dependencies/type packages are not installed here. The runs fail immediately on missing packages like Maintainability Note
-- |
This commit closes the remaining real correctness concerns around the Git Changes pipeline by tightening rename/copy handling, making diff requests rename-aware, and strengthening the grouped status loading path. Rename and copy entries are now canonicalized to the destination path while preserving the original path as metadata, which keeps status and numstat data aligned and gives the diff loader enough information to fetch the correct preimage for staged and unstaged rename scenarios. The UI-side loading path was also hardened so the server-backed staged or unstaged detail is fetched in parallel with the SDK status call instead of being blocked behind it, and passive refreshes now refresh selected diffs more reliably instead of relying on a coarse fingerprint that could miss same-sized content edits. Server and UI typechecks were rerun after the change set to verify the widened contracts and request flow.
This commit closes the remaining blocker-level issues in the Git Changes hardening pass by making the grouped staged or unstaged status flow render from authoritative server detail independently of the proxied SDK status call, and by correcting the mixed rename state diff semantics for the staged-rename-then-edit-again case. On the UI side, loadGitStatus now starts SDK and server status requests concurrently but awaits the server-backed detail first, so a stalled SDK status call no longer prevents the grouped Git Changes view from loading. SDK data is now treated as best-effort enrichment rather than a rendezvous point for rendering. On the server side, unstaged rename diffs now choose the correct index preimage path by preferring the current path in the index and falling back to originalPath only when needed. This fixes the case where a rename was already staged and the renamed file was then modified again, which previously risked comparing the working tree against the wrong blob. Server and UI typechecks were rerun after the change.
This commit closes the remaining blocker in the Git diff pipeline by making the unstaged diff path binary-aware on both sides of the comparison. The staged branch already used binary-aware blob reads for both before and after content, but the unstaged branch still treated the index/preimage as plain text and only derived isBinary from the working-tree side. The unstaged branch now reads the resolved index preimage through the same binary-aware helper used for staged diffs and preserves the binary flag even when the working-tree file is missing. This prevents binary deletions and other preimage-only binary cases from being surfaced as line-oriented text diffs. Server and UI typechecks were rerun after the change.
This commit refines the Git Changes row action reveal so the right-edge stage or unstage strip behaves like a true gutter instead of a shrinking inline control attached to the stats cluster. The action zone is now owned by the row itself, anchored against the row edge, and treated as the only interactive surface for stage or unstage within that area. The visible plus or minus symbol remains an animated cue, but it no longer defines the click target or creates dead zones around the symbol. The hover and linger behavior was tightened at the same time. The reveal now remains active while either the row body or the gutter is hovered, which fixes cases where the delayed hide could race against movement between the row and the action strip. When the cue appears, the row content yields a small amount of right-side layout space so the line-count stats move left by layout instead of relying on a fragile transform-only nudge. The implementation intentionally stays narrow and only touches the Git Changes row markup and the right-panel gutter styles. RightPanel state, git status loading, and server-side diff behavior are left unchanged in this checkpoint. This commit is meant to preserve the now-correct hitbox, cue activation, and count-shift behavior before the next feature pass for ctrl-click multi-selection and bulk stage or unstage actions.
This commit adds ctrl-click or cmd-click multi-selection to the Git Changes list so row actions can operate on a deliberate group of files instead of only a single row. The selection state now lives with the existing Git Changes data flow in RightPanel, is passed into the tab renderer, and is pruned whenever the staged or unstaged list changes so stale row ids do not survive refreshes or mutations. Bulk stage or unstage is intentionally section-scoped. Clicking a row action gutter on a selected row now applies the action to all currently selected rows in the same staged or unstaged section as the clicked row, while mixed-section selections are ignored for that action to avoid ambiguous behavior. Plain row clicks still open the diff and clear bulk selection, so the existing single-row inspection flow remains intact when modifier keys are not used. The row styling was also updated so bulk-selected rows are visually distinct and keep their selection highlight while hovered, rather than falling back to the generic list hover treatment. The implementation stays within the Git Changes UI layer and its mutation coordinator without expanding the server surface. Project typechecks were rerun after the change to confirm the new state and prop wiring.
This commit extends the Git Changes row multi-selection flow with a minimal shift-click range selection model. RightPanel now tracks a bulk-selection anchor row and, when shift-click is used, replaces the current bulk selection with the contiguous range between the anchor and the clicked row instead of layering on more complex additive selection rules. The interaction stays intentionally simple. Plain clicks still clear bulk selection and open the clicked diff, while ctrl-click or cmd-click continues to toggle individual rows and update the anchor. Shift-click does not open a diff row; it only computes the replacement range. The anchor is also cleared or pruned when selection state or the underlying row list changes so stale ids do not survive refreshes. This checkpoint only touches the Git Changes selection coordinator in RightPanel. It preserves the existing section-scoped bulk stage or unstage behavior and is intended as the minimal range-selection step before a final polish pass to suppress native text selection during modifier-based row selection gestures.
This commit polishes the Git Changes row multi-selection flow by preventing the browser's native text-selection behavior from activating during modifier-based row clicks. Shift-click and ctrl-click or cmd-click now prevent default on mousedown at the row level, which stops the list from highlighting file names or counts while the custom row-selection logic runs. The selection model was also tightened so shift-click range selection keeps the established anchor stable instead of moving it on every range extension. That makes repeated shift-click interactions behave more predictably because the growing or shrinking range continues to reference the same starting row until a plain click or ctrl-click or cmd-click deliberately establishes a new anchor. The active-row styling was adjusted at the same time so the currently opened diff row is visually lighter than the broader bulk-selected set. This makes it easier to distinguish the primary focused row from the rest of the selected rows at a glance while preserving the existing bulk-selection highlight. The change remains intentionally narrow and leaves plain click behavior otherwise untouched.
This commit slightly softens the active Git Changes row background so the currently opened diff row remains easy to spot without overpowering the broader ctrl-click or cmd-click bulk-selection styling. The base active-row fill now stays closer to the normal surface color while preserving the existing accent outline. The combined bulk-selected plus active state already keeps the selection color family, so this change only adjusts the standalone active-row fill. The goal is visual clarity: the focused row should read as distinct, but it should not visually erase or compete with the multi-selection state.
This commit changes Git Changes shift-click range selection from replacement semantics to additive semantics. A shift-click range now unions the anchor-to-clicked block with the existing bulk-selected set instead of discarding previously selected rows, which makes modifier-based row selection more flexible when building a non-contiguous working set. The existing stable-anchor behavior is preserved. Shift-click still uses the established anchor without moving it, while plain click and ctrl-click or cmd-click continue to establish or replace the anchor intentionally. The implementation remains local to RightPanel's Git Changes selection coordinator and does not change server behavior, row rendering contracts, or the existing section-scoped bulk stage or unstage rules.
|
Any better now? |
1 similar comment
|
Any better now? |
|
Updated gatekeeper pass on the latest PR head. What I understood you were asking: re-check the current PR state after the follow-up fixes and tell you whether the earlier gatekeeper blockers are still real. I reviewed the current branch state against Verdict: yes, materially better now. I would consider this mergeable based on concrete issues only. Previous blockers that now look resolved
Remaining finding
Testing note I again could not do a full repo typecheck/build in this workspace because dependencies are not installed here, so this gatekeeper pass is based on code inspection plus git-behavior validation in isolated local repro repos. Overall I do not see a remaining concrete blocker in the reviewed Git Changes server/UI implementation. If you want the ruthless gatekeeper answer: the earlier hard findings look addressed, and what remains is non-blocking polish rather than a reason to hold the PR. Size note Still worth tracking later, not as a blocker: -- |
|
What are the changes in package/server doing? |
|
What I understand you’re asking: explain the purpose of the Short version: the server changes turn the Git Changes tab from a UI that mostly depended on the proxied OpenCode file APIs into a server-backed worktree-aware Git surface that can answer the questions the UI actually needs: staged vs unstaged state, section-aware diffs, stage/unstage/commit mutations, and bounded invalidation when files change. Here is what each server area is doing. 1.
This is the typed contract between server and UI. 2.
This file is mostly request validation, worktree resolution, and wiring those routes to the underlying git helpers. 3. It does two main jobs:
More concretely, it:
This file is the reason the UI can show staged vs unstaged sections and load the correct diff for each section. 4. It wraps git commands for:
It also does safety/normalization work:
So this file is the server authority for per-file stage, per-file unstage, and commit. 5. It provides:
This is what keeps the new git endpoints worktree-scoped instead of accidentally operating on the wrong checkout. 6. Specifically it watches completed tool events for:
Then it resolves the affected directory back to a worktree slug and publishes a worktree-scoped invalidation event. This is what lets the Git Changes tab refresh itself after agent-driven file mutations without adding a separate bespoke invalidation channel. 7. 8. Why the server changes exist at all The UI needs server help to answer questions like:
That is the real job of the If useful, I can also break this down one level deeper into “read path”, “mutation path”, and “event invalidation path” as an execution flow. -- |
|
When is workspace.fileChanged event sent? |
|
What I understand you’re asking: explain exactly when Short answer first:
1. When
|
Depending on the hard coded tool calls might not be the best approach here. Any new tool call or a custom tool call editing the files will fire the event. Instead we should rely on the earlier event. If I'm not wrong it was session.updated. That brings in the changes.
|
|
From your message above
What happens in case use isn't setup? What happens to the UI / UX
Is the git binary run from the user environment as the node / opencode binary are run in packages/server? |
|
What I understand you’re asking: answer two very specific follow-ups.
1. If Git user identity is not configured, what happens?Server behaviorThe commit path is very thin right now:
It just does:
So if author identity is missing, Git itself is what fails, typically with a message along the lines of:
The server does not intercept that case and turn it into a friendlier domain-specific error. It just returns the Git failure text. Route handling:
That means the HTTP response becomes an error with that message body. UI / UX behaviorThe UI commit flow is here:
The request layer does this on any non-OK response:
Then the Git Changes tab catches that and shows an error toast:
So the actual UX today is:
Practical verdict on UX qualityThis is functional but not polished. Good:
Weak:
So if user identity is not set up, the feature does not corrupt state, but the UX is basically: raw Git error in a toast. 2. Is
|
2 participants
Git Changes PR Review Context
Fixes: #310
Purpose of this document
This document is intended to give a PR reviewer or gatekeeper enough neutral context to review the Git Changes feature series accurately.
BEFORE/AFTER SNAPSHOT:
It distinguishes:
It should not be treated as a request to approve the PR automatically. It exists to reduce false-positive review findings caused by missing context.
High-level scope
The work in this series refactors and extends the existing
Git Changestab in the right panel.The intended feature scope includes:
This work is intentionally implemented inside the existing Git Changes vertical slice rather than as a new SCM subsystem.
Files and areas intentionally changed
Server / API surface
The following server areas were intentionally extended:
packages/server/src/api-types.tspackages/server/src/events/bus.tspackages/server/src/server/http-server.tspackages/server/src/server/routes/workspaces.tspackages/server/src/workspaces/git-status.tspackages/server/src/workspaces/git-mutations.tspackages/server/src/workspaces/worktree-directory.tspackages/server/src/workspaces/instance-events.tsUI surface
The following UI areas were intentionally extended:
packages/ui/src/components/file-viewer/monaco-diff-viewer.tsxpackages/ui/src/components/instance/instance-shell2.tsxpackages/ui/src/components/instance/shell/right-panel/RightPanel.tsxpackages/ui/src/components/instance/shell/right-panel/git-changes-model.tspackages/ui/src/components/instance/shell/right-panel/tabs/GitChangesTab.tsxpackages/ui/src/components/instance/shell/right-panel/types.tspackages/ui/src/components/instance/shell/storage.tspackages/ui/src/components/prompt-input.tsxpackages/ui/src/components/prompt-input/types.tspackages/ui/src/components/session/session-view.tsxpackages/ui/src/lib/api-client.tspackages/ui/src/lib/i18n/messages/*/instance.tspackages/ui/src/styles/panels/right-panel.cssIntentional product and architecture decisions
The following outcomes were deliberate and should not be flagged as issues merely because they exist.
Git status / diff architecture
file.status()payload.Git Changes UI structure
Staged ChangesChangesDiff behavior
Stage / unstage / commit workflow
Staged Changessection.Prompt-context insertion
Prompt insertion is intentionally an HTML comment marker, not a full diff payload.
The expected inserted form is:
<!-- Git change context: <path> lines X-Y -->The trigger UI is intentionally a seam/gutter action in the Monaco diff viewer, not a toolbar button.
Row action reveal behavior
Auto-refresh behavior
The accepted refresh model is intentionally hybrid:
writeeditapply_patchThis hybrid model is intentional. Polling remains as a fallback even after tool-event invalidation.
Behaviors explicitly tested during development
The following behaviors were explicitly exercised during development and used to guide fixes.
Grouped staged / unstaged behavior
Diff behavior
Mutation behavior
ChangesStaged ChangesPrompt-context behavior
Auto-refresh behavior
writeeditapply_patchReview findings that were investigated and are no longer intended blocker topics
The following areas were previously raised by strict reviews and then either fixed or determined to be acceptable within scope.
Fixed in the current series
git-diffpath traversal / absolute path boundary issueInvestigated and considered non-blocking within current intent
Remaining non-blocker follow-up areas
The following are still reasonable follow-up topics, but they were not part of the required blocker-fix scope.
RightPanel.tsxright-panel.cssThese follow-ups should not be interpreted as evidence that the core implementation is incomplete unless a reviewer finds a new concrete failure.
Suggested review focus
If a gatekeeper or reviewer is evaluating this PR, the most useful focus areas are:
Less useful review topics, unless tied to a concrete failure, are:
Summary
This series intentionally evolves the existing Git Changes tab into a more complete source-control workflow for:
The intended review standard is to find concrete correctness, layering, or maintenance problems that remain after this series — not to re-argue the already accepted product choices listed above.