Skip to content

Report merges completed during Delivery as merged - #810

Merged
JacobStephens2 merged 7 commits into
mainfrom
issue-806
Oct 19, 2026
Merged

JacobStephens2 merged 7 commits into
mainfrom
issue-806

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

An externally merged PR was rejected as merged, not open when its implementation session ended. Delivery now recognizes a completed merge and reports the actual outcome, including for a Run originally asking for ready for review.

Delivery gate observes MERGED
  expected repository, PR identity and branches
  exact local head + no remaining local work
    -> report merged; stop publication and PR edits
  otherwise
    -> fail and retain local work

Closes #806

Evidence

  • Before: cargo test --test merge_run a_pr_merged_while_the_opening_session_runs_is_reported_as_merged -- --exact failed with exit 1 and PR … is merged, not open, matching the original run log. PR Deepen the Command reader around selection and ownership #800 merged at 11:05:09 UTC while the implementation session continued until 11:06:21 UTC.
    After: the regression passes for both ordinary Runs and Merge runs. A second regression covers a merge during CI. Both confirm the deleted Issue branch stays deleted.
  • Before: the Spec review's exact reproduction, bash /tmp/thirdshift-806-spec-repro.sh, failed: an edit hidden by assume-unchanged was discarded after reporting success.
    After: the same reproduction passes for both assume-unchanged and skip-worktree; the permanent externally_merged_pr_retains_tracked_changes_hidden_by_index_flags regression retains those edits. Staged, unstaged, untracked and newer committed work also remain retained on failure.
  • cargo test --test merge_run: all 36 tests pass.
  • cargo test --test pr_lifecycle: all 4 tests pass. Updated the pending-CI lifecycle regression to accept a completed merge while retaining closed/draft failures and branch-deletion checks.
  • cargo test --test spec_push_race: all 3 tests pass. A merge observed before the push comparison reports completion and still leaves the deleted branch absent.
  • cargo check --all-targets, cargo fmt --check, and cargo clippy --all-targets -- -D warnings pass.
  • Full suite: 1,972 passed, 2 existing ignores, 0 failures, across all 49 Cargo test executables. Built with cargo test --no-run --message-format=json, then ran every reported test executable with --test-threads=4 under a four-worker runner that waits for every child and checks every exit code (python3 /tmp/issue-806-full-suite.py).

Test seams

  • CLI Run and Merge run, using real Git repositories and the existing GitHub/Harness fakes: terminal PR observations during opening and CI, actual reported outcome, branch publication, checkout cleanup and local-work retention.

Unaddressed findings

Standards

None. No Standards findings.

Spec

None. The one P1 finding about hidden tracked edits was reproduced as written, fixed, and retained as a regression test. The Spec reviewer verified the correction.

Security

No introduced Security finding was confirmed. This guidance review is incomplete because the skill's required execution sandbox could not start: bwrap: Creating new namespace failed: Resource temporarily unavailable (exit 1). Repository code and proof-of-concept tests were not executed, and no source fix was made.

Source inspection covered all eight changed files in git diff fbdde945c44a045591d2d59daef01b5cc85f9064...HEAD and supporting PR identity, Delivery, GitHub snapshot, worktree ownership, cleanup and failure-preservation code. Relevant attack-class guidance and the repository's trust and Self-merge ADRs were read; no SECURITY.md or standalone threat model was found.

To complete validation, use a host supporting the skill's required no-network, read-only-source sandbox with an allowlisted environment and resource limits. Run the focused merge, PR lifecycle and push-race regressions with local fakes; run any candidate proof-of-concept on both HEAD and the untouched pinned merge base before classifying or fixing it. This blocker is a validation gap, not a confirmed Security finding.

Security review outcome

  • Security review incomplete: Required sandbox could not create namespaces, preventing local proof-of-concept and regression validation.

Review coverage

Independent Standards and Spec reviews used main as the fixed point. Both reviewed all eight changed files, including the correction in src/worktree/acquisition.rs and the updated lifecycle and push-comparison regressions. Changed files left unread: none on either axis. Generated-output exclusions: none.

Final reports and each axis's combined files-read list are saved in .thirdshift-review-7h4Xy7/standards.md and .thirdshift-review-7h4Xy7/spec.md in the implementation workspace.

Merge Danger

Door: two-way

No persisted format or configuration changes.

Blast Radius: Delivery

Changes terminal merge classification for Runs and Spec PRs. Closed, retargeted or mismatched PRs still fail; local work omitted from the merge remains retained. Self-merge authorization remains with thirdshift.

Built with codex · gpt-6.1-sol · high

@JacobStephens2
JacobStephens2 merged commit 378de17 into main Oct 19, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-806 branch October 19, 2026 18:36
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.

check why run result was failed when the resulting PR was merged

1 participant