Repository navigation
Conversation
Signed-off-by: Matt Toohey <contact@matttoohey.com>
After rebasing the revert-commit action onto main, its session row no longer matched what the backend queue controller (#993) expects of a `running` row: - Busy checks (`has_running_session_for_branch`, `get_*_sessions_for_branch`) now filter on `sessions.branch_id`, and the revert row was created without one, so a running revert left its branch looking idle to the queue. - The stranded-session sweep requires every `running` row to carry a registry entry from before its insert until the runner registers it, and a failed start must be recorded on the row and kick the queue. Create the revert row with `with_branch`, take the liveness hold before the insert, and hand it to the shared `launch_running_pipeline_session` instead of starting the runner inline. `RunningPipelineSession` grows a `pre_head_sha` (None for rebase/squash/push, unchanged) and an owned prompt so the revert can carry its captured HEAD and SHA-bearing label through the launcher. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Address the review of the revert-commit action: - Busy check under the launch lock. `revert_commit_for_branch` checked for in-flight work without the branch launch lock, then awaited a HEAD lookup before inserting its row, so a rebase, squash or chat session could start in that window and run alongside the revert. The check and the session/pending-commit insert now share one locked section (`insert_running_revert_pipeline_session_if_idle`). The same row insert is shared with rebase/squash through `insert_running_pending_commit_session`. - HEAD is read by the runner, not the command. `PipelineConfig` gains `capture_pre_head`, which makes `run_pipeline` read pre-HEAD before the first step for a kind-less revert, the way it already does for a rebase. The read happens after the row makes the branch busy and goes through `pipeline_git_runner`, so it respects `remote_working_dir` on a multi-repo workspace and matches the post-HEAD read it is compared against. The bare `ws_exec` helper is gone. Keeping the read off the command also means a Stop during it hits a registered runner, and the command answers without waiting on a workspace round trip. - `revertCommit` used an undefined `invoke`; it now uses `invokeCommand` like `rebaseBranch`/`squashCommits`, which also routes web mode to the `revert_commit` dispatch arm. - BranchCard adds a "Reverting…" pending-session stub keyed on the returned session id, matching rebase/squash. The busy gate stays, since the backend refuses rather than queues a revert (queueing would need a persisted target SHA, which is a data-model change). It now reuses `immediateGitActionDisabledReason`, so a detached HEAD or wrong checkout also blocks a revert. - The timeline context menu renders the disabled reason inline under "Revert". A disabled item takes no pointer events, so a title tooltip would never show. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
… reads Address the follow-up review of the revert-commit action: - Refuse a revert during a reset-to-origin or discard. Neither has a session row, so the busy check could not see them, and a revert started under one raced its `git reset --hard`. The revert gate now reads `branch_worktree_operation_in_flight` under the launch lock, as the forced start does, and refuses with its own message. - Stop orphaned queued rows from keeping a branch busy. `branch_has_work_in_flight` now reuses `session_commands::has_queued_branch_session`, which skips `Unlinked` rows the drain steps over. Before, one such row blocked Revert on the branch for good, and a rebase, squash, push or pull queued behind it alone was never drained. - Keep the revert commit linked when the runner's up-front pre-HEAD read fails. `capture_pre_head` now also promises that the pipeline hands off only from a failed step (HEAD unchanged) and that a completed run moved HEAD, which holds for `git revert`. So the handoff reads HEAD as the pre-pipeline HEAD, and the no-AI completion claims HEAD for the pending row instead of deleting it. Deleting it left a "Reverting…" stub that was never pruned. - Run orphan reassociation on the no-AI path only for a rebase again, matching the AI path. A revert adds a commit on top and orphans nothing, so it no longer pays the history walk or the remote round trip. - `immediateGitActionDisabledReason` now covers Revert as well as reset to origin, and blocks while a reset or discard is in flight. Reset to origin still shows its own "Resetting..." first, so it never reads as blocked by itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Address the follow-up review of the revert-commit action: - Delete commit is a third HEAD-mover with no session row, and it takes neither the worktree marker nor the launch lock. If a revert's commit landed after a delete had read HEAD, the delete's `git reset --hard` to the parent wiped the revert along with the deleted commit, and the no-AI completion claimed the wrong commit for the revert row. `immediateGitActionDisabledReason` now refuses with "Deleting commit" while `deletingCommitKeys` is non-empty, alongside reset and discard. The reset-to-origin row reads the same reason, which is correct: a reset racing a delete hits the same hazard. A pending-commit delete also counts, which over-blocks only briefly. - Correct the `insert_running_revert_pipeline_session_if_idle` doc. A busy row only keeps queue-routed work from moving HEAD before the runner reads it. Reset-to-origin, discard and delete-commit bypass the queue. The marker only stops a revert starting under a reset or discard, only frontend gates keep those two from starting beside a running revert, and nothing gates a delete-commit then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…rt guards Apply the review suggestions on the revert-commit action: - `resolve_pipeline_artifacts_without_ai` no longer loads the linked commit up front. A completed run with a pre-pipeline HEAD (or one that asked for it) finalizes; anything else deletes its pending row. The lookup was redundant: `delete_pending_commit_for_session` already matches only `sha IS NULL`, and finalize reloads the row and handles a missing one. Finalize's no-HEAD guard became unreachable from its only caller, so it is now a `debug_assert!`. No behaviour change; a new test pins that an unfinished run drops a pending row, keeps a landed one and is a no-op without a row. - Correct the rebase arm comment in `pre_head_for_pipeline_handoff`. A rebase only reaches it when its up-front HEAD read failed, and it must not read HEAD then, since it may be detached mid-rebase. - Add `revert_commit_for_branch(` to the web guard's `WRITE_MARKERS`, so `web_arms_that_write_running_rows_are_detached` covers the revert arm if it ever gains an await. - Cover `revertCommit` in the invoke-shape test (`revert_commit` with `branchId`, `commitSha`, `provider: null`), since the wrapper once shipped calling an undefined `invoke`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e70866d13
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let commit_sha = validate_commit_sha(commit_sha)?; | ||
| Ok(vec
When a repository has a local Git identity different from the user's global identity, a conflict-free revert is authored, committed, and signed off using that local identity. The deterministic runner does not apply git_identity_env_from_global_config(); that override is only applied during AI handoff. Consequently, the same Revert action uses different identities depending on whether it encounters conflicts, and the successful path bypasses the global-identity requirement explicitly included in revert_handoff_prompt. As confirmed by git revert -h, --signoff only means “add a Signed-off-by trailer”; it does not select the identity. Apply the global author/committer environment to the deterministic command as well.
Useful? React with 👍 / 👎.
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
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
Adds a Revert action to the branch timeline's commit context menu. Right-clicking a commit on a branch runs
git revertthrough the no-AI pipeline machinery, producing a new revert commit on top of the branch and surfacing a "Reverting…" pending-session stub in the branch card while it runs.Backend (
src-tauri/src/prs)revert.rswithrevert_commit_for_branchand arevert_commitTauri command, plus a web-mode dispatch arm inweb_server.rs.with_branch, the liveness hold is taken before the insert, and the row goes through the sharedlaunch_running_pipeline_session.RunningPipelineSessiongains an optionalpre_head_shaand owned prompt.insert_running_revert_pipeline_session_if_idle). A revert is refused while a reset-to-origin or discard is in flight, and the pre-pipeline HEAD is read by the runner (viapipeline_git_runner, respectingremote_working_dir) rather than the command.PipelineConfiggainscapture_pre_headfor this.branch_has_work_in_flightnow reuseshas_queued_branch_session, so orphanedUnlinkedqueued rows no longer keep a branch busy forever.resolve_pipeline_artifacts_without_aiis simplified: a completed run with a pre-pipeline HEAD finalizes, anything else drops its pending row. Orphan reassociation on the no-AI path runs only for rebases again.prs/tests/revert.rscover the launch, refusals, pending-row handling and the web guard's write markers.Frontend
revertCommitincommands.tsusesinvokeCommand(covered by the invoke-shape test).TimelineContextMenushows a Revert item with the disabled reason rendered inline, since disabled items take no pointer events.BranchCardwires the action, adds the "Reverting…" stub keyed on the returned session id, and extendsimmediateGitActionDisabledReasonto cover Revert and to block while a reset, discard, or commit delete is in flight (a delete is a HEAD-mover with no session row and could otherwise wipe a freshly landed revert).Test plan
cargo testfor theprsrevert tests and the web guard write-marker testcommands.test.tsinvoke-shape coverage forrevertCommit🤖 Generated with Claude Code