Skip to content

Pass over a staged patch that cannot be accepted - #15

Merged
SimonCropp merged 2 commits into
mainfrom
simon/stale-staged-patches
Sep 24, 2026
Merged

SimonCropp merged 2 commits into
mainfrom
simon/stale-staged-patches

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

A staged patch names the source file its snapshot belongs to, which is a separate question from where it was staged. A queued snapshot is asked both: the owner holds only what a run sent it, and the scan drops what names a source outside the root. A staged one was asked neither, so a scan offered every patch under the root's obj, however old.

Two kinds of those can never be accepted. One naming a source outside the root, which reviewing a directory must not reach into. And one naming a source that has gone, since the literal is located by searching that file, so there is nothing for an accept to rewrite. Both are now passed over, which leaves the staged half asked what the queued half already was.

The integration harness is where this repo's own obj filled with the second kind: it copied a scenario's staged files under the harness directory rather than moving them, leaving the originals to name a temp directory that goes when the scenario ends. Maps had it worse, since most scenarios never publish them and so only ever left them behind. Publishing now moves, and the harness takes whatever else names its directory with it, so a run leaves obj as it found it.

Verify and DiffEngine references go to 33.1.3 and 20.5.0.

Tests

Two finder tests: a staged snapshot outside the root, and one whose source is gone. Each fails when its half of the check is removed.

Some manager tests built their staged list by running the finder over a real temp source, which the root rule now declines, so the staging helper hands the record back directly instead.

Locally this cleared 210 staged files, from 81 patches going back a month, and 808 maps, all naming temp directories that no longer exist. Two full integration runs afterwards left obj empty, which is what confirms the harness half.

A staged patch names the source file its snapshot belongs to, which is a
separate question from where it was staged. A queued snapshot is asked both:
the owner holds only what a run sent it, and the scan drops what names a source
outside the root. A staged one was asked neither, so a scan offered every patch
under the root's obj, however old.

Two kinds of those can never be accepted. One naming a source outside the root,
which reviewing a directory must not reach into. And one naming a source that
has gone, since the literal is located by searching that file, so there is
nothing for an accept to rewrite. Both are now passed over, which leaves the
staged half asked what the queued half already was.

The integration harness is where this repo's own obj filled with the second
kind: it copied a scenario's staged files under the harness directory rather
than moving them, leaving the originals to name a temp directory that goes when
the scenario ends. Maps had it worse, since most scenarios never publish them
and so only ever left them behind. Publishing now moves, and the harness takes
whatever else names its directory with it, so a run leaves obj as it found it.
@SimonCropp SimonCropp added this to the 0.9.1 milestone Sep 24, 2026
@SimonCropp
SimonCropp merged commit 4c305dc into main Sep 24, 2026
2 checks passed
@SimonCropp
SimonCropp deleted the simon/stale-staged-patches branch September 24, 2026 01:16
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