Skip to content

feat(gpui): diff edit mode and stacked pull requests#110

Closed
hewigovens wants to merge 1 commit into
mainfrom
gpui-diffedit-stacked-pr
Closed

feat(gpui): diff edit mode and stacked pull requests#110
hewigovens wants to merge 1 commit into
mainfrom
gpui-diffedit-stacked-pr

Conversation

@hewigovens

@hewigovens hewigovens commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Closes the two largest shell-parity gaps (both rows now Yes in the matrix).

Diff edit mode — enter via the Edit Diff button or gutter menu; checkbox selection of lines, change groups, files, or select-all (SwiftUI's exact full-diff-index model); apply to all four core destinations (Discard, Move to Working Copy, New Child, New Parallel) with the shared staleness/conflict guards, plus a new core-level ensure_commit_mutable check on every diffedit flow. Unified-view-only editing with view-mode restore, post-apply file re-selection. 11 component tests incl. per-destination end-to-end and stale rejection.

Stacked pull requests — panel from the change context menu over the shared core engine: async stack detection, inline bookmark editing with validity/duplicate checks, AI naming for auto-assigned layers via the provider chain (user edits always win), submit with per-layer Created/Updated/Failed outcomes and PR links. 8 component tests.

Shell-only work reusing existing core APIs (one small core addition: the immutability guard). Parity matrix + guide updated.

@hewigovens hewigovens marked this pull request as ready for review July 12, 2026 07:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2ede344efa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/gpui/src/repo/window/diff_edit.rs Outdated
Comment thread shell/gpui/src/repo/window/diff_edit.rs Outdated
@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from 2ede344 to dc3a38e Compare July 12, 2026 07:21

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc3a38ea3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/gpui/src/repo/window/diff_edit_apply.rs Outdated
Comment thread shell/gpui/src/repo/window/diff_edit_apply.rs Outdated
@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from dc3a38e to 58c331c Compare July 12, 2026 09:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 58c331c7e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/gpui/src/repo/window/stacked_pr.rs
Comment thread shell/gpui/src/repo/window/diff_edit_state.rs Outdated
@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from 58c331c to 291b0c8 Compare July 12, 2026 10:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 291b0c8063

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/gpui/src/repo/window/diff_edit.rs Outdated
@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from 291b0c8 to 0e27fbb Compare July 12, 2026 11:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e27fbb7de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


pub(crate) fn warning(&self, index: usize) -> Option<&'static str> {
let name = self.inputs.get(index)?.text();
if name.is_empty() || !is_valid_bookmark_name(name) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject base and existing bookmark names

When a GPUI user edits a stack layer to main/trunk or another bookmark that already points outside the detected stack, this validator still enables Submit because it only checks syntax and in-stack duplicates. The submit path creates-or-moves those bookmarks before pushing, so a valid-looking name can fast-forward the repo's base branch to the feature stack; compare edited names against the current bookmark list/base bookmark and reject occupied or protected names unless the bookmark already belongs to that same layer.

Useful? React with 👍 / 👎.

@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from 0e27fbb to 4e3eb39 Compare July 12, 2026 11:40

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e3eb39564

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +122 to +127
if self.diff_edit_selection_summary().0 == 0 {
self.show_toast(EMPTY_SELECTION_MESSAGE, cx);
return;
}
let message = self.diff_edit.message.text().to_owned();
self.apply_diff_edit(destination, message, cx);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Block apply while Select All is still loading

If a user clicks Select All on a large multi-file change and then immediately clicks an apply destination while the header still says Selecting..., diff_edit_selection_summary() can already be nonzero from the files that were loaded first, so this path proceeds and build_diff_edit_request omits any paths still in select_all_pending. Fresh evidence in this revision is that Select All now tracks pending preloads, but the apply entry point does not check selecting_all/select_all_pending; please disable or reject apply until the bulk selection has finished.

Useful? React with 👍 / 👎.

@hewigovens hewigovens force-pushed the gpui-diffedit-stacked-pr branch from 4e3eb39 to 57d58e7 Compare July 12, 2026 21:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57d58e7a91

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/justfile
Comment on lines +294 to +295
"{{project}}/Resources/DockIcon/jayjay-dock-light.png" \
"{{project}}/Resources/DockIcon/jayjay-dock-dark.png"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Create the DockIcon resource directory before rendering

On a clean checkout shell/mac/Resources/DockIcon is not present (I checked the resource tree with fdfind), but release-dry-run and release both depend on sync-icon, which now writes the rendered PNGs directly into that directory. The Swift renderer will fail with “Could not write Dock icon…” before the app build starts unless a developer has created the directory locally; add a mkdir -p "{{project}}/Resources/DockIcon" before these output paths or commit a tracked resource directory/file.

Useful? React with 👍 / 👎.

Comment on lines +51 to +52
dock_plugin="$app_path/Contents/PlugIns/JayJayDockTilePlugin.plugin"
if [[ ! -x "$dock_plugin/Contents/MacOS/JayJayDockTilePlugin" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the Dock plugin check until the app bundles one

This verifier now rejects every release archive unless Contents/PlugIns/JayJayDockTilePlugin.plugin exists, but the repo does not define or copy any JayJayDockTilePlugin target/source (repo-wide rg only finds these new verifier lines). As a result the normal just shell::release-dry-run/just shell::release archives built from shell/mac/project.yml fail verification even when the signed app is otherwise valid; either add the plugin to the app bundle or do not make this check mandatory yet.

Useful? React with 👍 / 👎.

Comment on lines +133 to +136
let Some(cached) = vm
.diff_cache
.values()
.find(|loaded| loaded.diff.path == path && loaded.projection.is_none())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Key diff-edit loads by the current revision

When an async preload from a previously selected change finishes after select_change has cleared the cache, it can still insert a LoadedDiff keyed by the old rev; this lookup then accepts it by path only while Diff Edit is active on a different change with the same file path. In that race, the Diff Edit card is built from stale old/new content and selections either fail as stale or are shown against the wrong file version, so this should look up the exact cache key for the active rev/hunk/ignore-whitespace (or generation-guard preloads) instead of scanning diff_cache.values().

Useful? React with 👍 / 👎.

@hewigovens hewigovens closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant