Repository navigation
fix(pr): confirm push and pull request outcomes from the remote before recording delivery - #294
Merged
Merged
Conversation
…e recording delivery Opening a pull request for a Task branch now decides what happened by reading it back, not by what `git push` and `gh pr create` reported. - A push whose gate passed is confirmed against the branch on origin's push URL. A push that reported failure but left the remote at the commit is delivered; one that reported success without it is not. A push the gate or its preparation refused is never reconciled against the remote, so a commit an earlier attempt pushed cannot stand in for it. The Human Review, live approval, cancellation and pinned-commit checks run exactly as before, immediately before git is spawned. - Looking up the branch's existing pull request is strict: a `gh` that cannot be asked, or a pull request that does not name the branch as its head, stops the delivery instead of reading as "no pull request". - The request to open the pull request is recorded on the Task (`pr_create_unconfirmed`) before it is sent. A failed or unreadable answer is reconciled with GitHub; an unknown outcome keeps the record and the retry finds the pull request instead of opening another. The record is cleared in the write that links the pull request. Older Task files load unchanged. - A created pull request is accepted only if the answer is a pull request URL and the remote branch is still at the confirmed commit. Tests inject a push or response lost after landing, a lookup that fails, a branch that moves, a board that stops accepting writes, an approval withdrawn or an operation cancelled at the gate with the commit already on the remote, and a reload from the board file.
…le its request is recorded The cancellation check before opening a pull request ran before the request was recorded on the Task, a disk write, so a cancellation that landed during that write did not stop `gh pr create`. The check now runs again once the record is written and before gh is started. The unconfirmed record is left in place on that refusal: it only says a pull request may exist, and a retry looks the branch's pull request up strictly before it opens one.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Persistent State Integrity |  |
confirm_none can return success after a persistence failure. In the changed src-tauri/src/commands/pr/unconfirmed.rs:292-315, lifecycle::record_typed returns `RecordWriteError::PublishedNotDurab… |
Handle RecordWriteError::PublishedNotDurable as an error, not as Ok(Discarded). Report that the visible board change is not confirmed durable and that it may return after a restart. Preserve the lifecycle write's adopted in-memory state… |
-
Check name Status Explanation Title check The title clearly describes the primary correctness change: confirming remote push and pull-request outcomes before recording delivery. Description check The description directly explains the remote verification, unanswered pull-request recovery, user actions, safeguards, and testing for this changeset. Recoverable Resource Lifecycle No explicit lifecycle failure is introduced. The PR link is persisted before terminal cleanup, so a cleanup refusal retains Task::pr_urland the task remains recoverable. Unknowngh pr createoutc…Platform Boundary Safety No changed code introduces an ungated platform-specific API or an evident cross-platform compile failure. The new runtime paths use platform-neutral std::fs,std::process, andtokio::processAPI…Full details: Recoverable Resource Lifecycle
-
Explanation
No explicit lifecycle failure is introduced. The PR link is persisted before terminal cleanup, so a cleanup refusal retains
Task::pr_urland the task remains recoverable. Unknowngh pr createoutcomes retainpr_create_unconfirmedand block another request. Record-removal failures leave the record or report that durability is unconfirmed. Recovery also provides explicit link and retry paths, and it performs no remote cleanup.
Full details: Platform Boundary Safety
-
Explanation
No changed code introduces an ungated platform-specific API or an evident cross-platform compile failure. The new runtime paths use platform-neutral
std::fs,std::process, andtokio::processAPIs. Unix-only shell-script tests andPermissionsExthelpers are gated with#[cfg(unix)]. The new acceptance journey is inside the existing Linux-only integration test crate (#![cfg(target_os = "linux")]). The pre-existing Unix symlink inFakeGhis unchanged.
-
Full details: Persistent State Integrity
-
Explanation
confirm_nonecan return success after a persistence failure. In the changedsrc-tauri/src/commands/pr/unconfirmed.rs:292-315,lifecycle::record_typedreturnsRecordWriteError::PublishedNotDurableafter the discard is not confirmed durable. The code maps that error todurable = falseand returnsOk(ResolveOutcome::Discarded { durable: false }). The lifecycle contract states that this outcome must not be reported as success. This changed recovery path therefore reports a successful persisted decision when persistence failed.Resolution
Handle
RecordWriteError::PublishedNotDurableas an error, not asOk(Discarded). Report that the visible board change is not confirmed durable and that it may return after a restart. Preserve the lifecycle write's adopted in-memory state and prevent a retry from treating the operation as durably completed until a later save confirms it.
Warning
Review coverage is incomplete: 1 file could not be fully reviewed. Findings from completed review steps are included; see review info for details.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
…wered A recorded request to open a pull request now decides what a retry does. While it is unanswered, a lookup that lists no pull request for the branch no longer admits another gh pr create: a listing can lag behind a pull request that was just opened. The refusal comes before anything is pushed, and the admission is checked in the same write that records a new request, so one request is never replaced by another. The record is removed only by linking the pull request, or when it is known that no request was made: GitHub refused the request with a client error, gh could not be started, or the operation was stopped before the request was sent. A server error, a timeout or an unrecognized failure keeps it.
…pen the pull request A successful push is not an approval of the step that follows it. The approval rules the push gate applies, including that the task's approval still covers exactly the commit that was pushed, are now read again immediately before gh pr create, after the request has been recorded. A refusal there sends nothing and takes the unsent request's record back; the verified push stands.
…nt repositories A pull request is named by its branch alone: gh pr create --head <branch> and gh pr list --head <branch> both read it as a branch of the repository gh works in. With a fork as origin the branch is pushed to the fork while gh looks in the upstream, so the pull request SlashIt would open or link is not verifiably the one from the branch it pushed. The delivery now stops before anything is looked up, pushed or opened when origin's push URL and gh's repository are both known and differ. The listing keeps passing over pull requests from forks: the one SlashIt opens is never from a fork, so they are not the task's own.
…very was admitted against A restack replays the approved commits and publishes a rewritten tip, while the approval keeps naming the commit it was given for. The re-check made before gh pr create compared the approval with the pushed commit, so an approved delivery that restacked was refused after its branch was already on the remote. It now compares the approval with the approved commit the delivery was admitted against. A withdrawn approval, or one that now covers another commit, is still refused.
…oof it opened nothing A GraphQL error was read as a refusal unless it said "something went wrong", but GitHub also answers a request that may have been applied with errors such as a timeout or "We couldn't respond to your request in time". A failure is now a refusal only when gh reports an HTTP 4xx other than 408, or a GraphQL error carrying one of GitHub's validation, permission or not-found messages for the request. Anything else keeps the request recorded as unanswered. A refusal also settles the record only when the listing of the branch's pull requests was read and shows none; when it cannot be read, the record stays. gh is now started by a small wrapper that tells a process that could not be spawned from one that was started and failed, so that only the former is treated as a request that was never sent.
… confirmed durable A write of the record that a pull request is about to be requested can reach the board file and still be reported as an error because the final synchronization failed. The board then holds the record while the message said nothing was recorded, and every later attempt was refused as if a request had been sent. The error is now told apart from a write that published nothing: when the record is this attempt's, it is taken back, since nothing was sent, and the message says what happened. When the record cannot be taken back, the message says that and that a retry may refuse.
The check that origin and gh use the same repository goes on when it cannot judge: a push URL that is not an owner/repository, or a git or gh that does not answer. It now logs the reason in each case, so a delivery that went on unchecked can be told from one that was checked. Hosts are still not compared, as an ssh host alias names the same repository under another host.
… new request The test stopped at the first lookup, which fails before the record is read, so it passed without the rule it is named for. It now lets the listing answer again with the pull request still hidden, removes the remote branch so a push would show, and requires the retry to be refused before pushing or opening anything.
A remote URL can carry a token or a password. The repository check logged origin's push URL whole when it could not judge it, and the messages about an unanswered pull request request quoted what gh and git printed, which names the URL they failed on. Userinfo is now dropped from a remote URL (both scheme://user:secret@host and scp-like user@host:path) and from the URLs inside that output before any of it is logged or shown.
…s removed When the record of a request that was never sent and its removal were both published without being confirmed durable, the message said SlashIt could not remove the record and that a retry may refuse, while the board and the file hold no record. The retract now tells a record that remains (the write published nothing) from one that was removed with the removal unconfirmed, and only the first predicts a refusal.
… request The fixture failed pr create with plain text, which is not an answer that shows GitHub opened nothing, so a retry was now refused as an unconfirmed request. It fails with an HTTP 422 refusal instead, which is how a request that opened nothing is reported.
…est request A new timeline entry names the decision of a person who confirmed that no pull request exists for a branch and discarded the record of a request to open one whose outcome SlashIt never learned. It records the discarded attempt (branch, commit, start time) and says that the decision is the person's and was not confirmed by GitHub.
…s never learned A task that holds an unanswered request to open its pull request can now be settled by a person through resolve_unconfirmed_pr_create, bound to the exact record they were shown (branch, commit, start time). Linking names the pull request they found. It is linked only after gh and git agree on the repository, GitHub answers for exactly that pull request, its head branch is the task's branch in this repository and not a fork's, and its head commit is the attempted commit or contains it; the existing link then records it and settles the request in the same write. Confirming that no pull request exists makes one read-only lookup, refuses and offers the link when GitHub lists the pull request, and otherwise discards the record on the person's confirmation alone, which is recorded on the timeline as their decision and not as something GitHub proved. Neither pushes a branch, runs gh pr create or changes anything on GitHub. A stale confirmation, a running task, another pull request operation holding the task, or an operation a lifecycle transition ended is refused, and a write that published nothing is an error. link_pr no longer links a task that holds an unanswered request by URL alone.
…uest The task drawer shows a task's unanswered request to open its pull request with its repository, branch, commit and start time, and the two things a person can do: link the pull request they found, which the backend checks before linking, or confirm after checking GitHub themselves that none exists, behind an explicit acknowledgement of the duplicate risk and bound to the attempt that was shown. The panel tells apart an unknown outcome, a pull request linked and verified, and a request the person discarded, which stays worded as their decision and not as something GitHub confirmed.
…he drawer One journey leaves a request to open a pull request unanswered the way a lost connection does, checks that it survives a restart and blocks a second request, shows the repository, branch and commit in the drawer, has the person confirm behind an explicit acknowledgement that no pull request exists, checks that the confirmation ran no gh pr create and pushed nothing, and then retries under the approval that still stands to open exactly one pull request. The fake gh gains a failure that says nothing about whether it opened anything, and names its repository for repo view --json nameWithOwner. Document the recovery, and what an older build does with a board that holds it, in the product model.
After a failed answer the confirmation stayed ticked, so a second click sent the same acknowledgement without the person making it again. Any answer, a refusal and an error included, now spends it.
…ompare what it links The recovery looked at the task's status before it reserved the task, and the reservation ends whatever owns the task, so a run started in between would have been cancelled. The check is now made under the lifecycle lease, before the reservation can end anything, and covers a run, an AI review or fix, a pull request helper and another pull request operation: the recovery is refused and the owner is left running. A test now replaces the record between the lookup and the write and requires the discard to be refused as stale, which only the compare-and-set in the write catches. The link depends on the reservation, which every writer of the record holds; the code says so. What is linked is GitHub's own spelling of the URL, and the pull request must be on the host of the repository gh works in (origin's host is not compared, as an ssh alias names another host). The docs describe what linking reports when a write is published but not confirmed durable.
shingonoide
marked this pull request as ready for review
November 2, 2026 01:06
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
gh pr createblock a second request until it is resolved. This ports the earlier unpublished work onto currentmainwithout weakening the gated push added since.gh pr listis an error rather than "no PR", and a listed PR must name the task's branch as its head. Cross-repository entries are skipped on purpose:gh pr create --head <branch>without an owner always opens a same-repository pull request.push_branch_pinned_gatedwith the Human Review rule, live approval check, cancellation check and pinned-commit constraint. A push the gate refused is never overruled by a commit an earlier attempt already pushed. Only a push whose gate passed is confirmed against the remote, read fromorigin's push URL.pr_create_unconfirmedrecord is written durably, in one admission step, beforegh pr create. While it is set, no furthergh pr createis admitted, whatever the lookup returns, because a listing can lag a pull request that was just opened. A positive lookup links the existing pull request and removes the record. A negative or failing lookup keeps it. No timers are involved.ghcould not be started, or the operation was stopped before the request was sent. A 5xx, a timeout, a dropped connection or any unrecognised answer stays unconfirmed.gh pr create, with no await before the process starts. The check is bound to the commit the delivery was admitted against, so an approved delivery that restacks still goes through, and a withdrawn or replaced approval stops the request. The verified push stays and the unsent record is taken back.origin(the push URL andgh repo viewname different repositories) is refused before anything is pushed or opened, because the pull request could not be tied to the pushed branch. When the repositories cannot be compared (a local path,ghunusable) the delivery proceeds and the reason is logged.gh pr createor makes any other remote change, and the automatic path stays fail-closed:ghworks in (and origin's push URL names the same repository),gh pr viewanswers for exactly that URL, its head branch is the task's branch, it is not cross-repository, and its head commit equals or contains the attempted commit. Anything that cannot be established is refused with the reason. The existing link then records it and clears the record in one write. GitHub's canonical URL is what is stored.link_prrefuses a task that holds an unanswered request and points at this recovery. It is unchanged for every other task.pr_create_unconfirmedis a separate optional Task field (serde default, omitted when empty). Boards without an outstanding record are unchanged.Testing
Run locally with
GIT_CONFIG_GLOBAL=/dev/null:cargo test -p slashit-ui --lib commands::: 955 passed;--test pr_review_integration: 27 passed;-p slashit-frontend: 311 + 3 passed;-p slashit-activity: 35 passedcargo testof the CI crate set, including the ACP fixture (built through the dev-dependency): passedcargo clippyin the CI form (-D warnings) andcargo check -p slashit-frontend --target wasm32-unknown-unknowngh pr createresponse lost; lookup negative then positive; lookup fails; board reloaded from disk (restart); remote head differs; approval withdrawn or replaced after the push and after a restack; cancellation before create; record write fails or is published but not confirmed durable; fork asorigin; credentials in URLs.gh pr createduring either recovery; a later request still needs its normal approval and opens exactly one pull request; a running, queued or owned task is refused and left running; write failures leave the record; repeating the recovery sends nothing.cargo fmt --check: not run;pr.rsalready differs from rustfmt onmaintrunk buildalone: not run; the acceptance build runs itNotes
ghcalls have no timeout (as before): a hungghholds the task's reservation.Taskand the timeline reject unknown fields. An older build reading a board that holds an unanswered request, or the person's discard entry, reports that project's board as unreadable, leaves the file untouched and refuses to save over it. Boards without either are readable by both. Opening the board with this build, or removing thepr_create_unconfirmedtable (and any discard entry) by hand after copying the file, recovers it. Removing the table drops the record that a pull request may exist.docs/architecture/product-model.mddescribes this.publish_approved_to_existing_pr(a pull request that already exists) does not read the remote back after its push; that path is unchanged frommain.