fix(bin): resolve the fork trunk as a landing target - #47
Merged
sbracewell64 merged 1 commit intoAug 6, 2026
Merged
Conversation
The "has this landed?" check measured against a trunk this fleet does not land on, so proven-landed work read as unlanded and held its worktree slot. With `origin` fetching kunchenguid/firstmate and pushing sbracewell64/ firstmate, both refs the check considered were wrong at once: origin/main tracked upstream, and refs/heads/main had not been fast-forwarded since the fork trunk advanced five times that day. Measured on the held slot: PR 44 squash-merged, recorded head 2582c15, fork trunk f90ed1d git diff --stat 2582c15 f90ed1d -> empty (content landed) git merge-base --is-ancestor 2582c15 f90ed1d -> false (squash) fm_landed_tree_contains vs refs/heads/main -> 1 (not contained) fm_landed_tree_contains vs origin/main -> 2 (inconclusive) fm_landed_tree_contains vs the fork trunk -> 0 (contained) The default fetch refspec cannot reach the fork, because it points at the fetch url. fm_landed_push_url resolves the push url when it differs from the fetch url - the same fetch-versus-push comparison bin/fm-task-base-lib.sh already made at the spawn site, now owned in one place and read by both - and fm_landed_refresh_push_target fetches that trunk into refs/fm-landing/origin/<name> for fm_landed_candidate_refs to offer. The check is not made more permissive. The added ref is a landing target only because this fleet demonstrably pushes there, never inferred from a remote's name. Only a caller that already refreshes remotes fetches it, so the guard stays local. A push url that exists but cannot be read leaves the landing target unread, and teardown refuses rather than falling back to the upstream answer - a case that previously passed on that fallback. A repository whose fetch and push urls agree resolves no landing ref at all and takes an identical path. Every negative control was watched red first: pre-fix, the fork-landed slot is refused at both the teardown and guard sites, and the unreadable-push-url case wrongly passes. The unlanded, upstream-only, and single-remote controls pass before and after, so they are not vacuous. Verified against the live held slot: the resolved push url fetches the real fork trunk, which reports the merged work contained while upstream stays inconclusive, and the same slot plus one unlanded commit still reports not contained. Full changed-scope run: 75 scripts, 6 failing identically on the unmodified fork trunk (tmux readiness, node ESM resolution, harness env), 0 new.
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.
The defect
The "has this landed?" check measured against a trunk this fleet does not land on, so proven-landed work read as unlanded and held its worktree slot indefinitely.
originfetcheskunchenguid/firstmatebut pushessbracewell64/firstmate. Both refsfm_landed_candidate_refsconsidered were wrong at once:refs/remotes/origin/maintracked upstream, andrefs/heads/mainhad not been fast-forwarded since the fork trunk advanced five times that day. Nothing tracked the trunk this fleet actually lands on.Measured on the held slot (PR 44, squash-merged, recorded head
2582c15, fork trunkf90ed1d):git diff --stat 2582c15 f90ed1dgit merge-base --is-ancestor 2582c15 f90ed1drefs/heads/mainrefs/remotes/origin/mainThe squash case is exactly what
fm_landed_tree_containsexists to handle, and it works. It simply never saw the fork trunk.The fix
The default fetch refspec cannot reach the fork, because it points at the fetch url. So:
fm_landed_push_urlresolves the push url when it differs from the fetch url. This is the same fetch-versus-push comparisonbin/fm-task-base-lib.shalready made at the spawn site in fix(bin): separate task read and contribution bases (land of upstream #1613) #43; it now has one owner and both libraries read it - that one names the upstream side, this one the landing side.fm_landed_refresh_push_targetfetches that trunk intorefs/fm-landing/origin/<name>.fm_landed_candidate_refsoffers it like any other candidate.Why this is not a more permissive check
bin/fm-worktree-guard.shstays local and grows no network dependency.Verification
Every negative control was watched red first.
Pre-fix, with the new tests in place:
The unlanded, upstream-only and single-remote controls pass both before and after, so they are not vacuous. All green post-fix.
Against real state, using the real remote urls, the real commits and a real network fetch:
The same slot plus one genuinely unlanded commit still reports
1(not contained), and with the push url made unreadable the refresh returns1, which the caller must treat as a refusal.New coverage:
tests/fm-teardown.test.shcases (u) through (y) andtests/fm-worktree-guard.test.shcases (o6) through (o8).Test run
Full changed-scope selection: 75 scripts, 69 pass, 6 fail. Those 6 fail identically on the unmodified fork trunk - tmux readiness, node ESM module resolution, and harness env vars - so they are pre-existing and environmental. Zero new failures.
bin/fm-lint.shandbin/fm-doc-audience-check.shboth clean.Known limitation, disclosed
Teardown now performs one additional network fetch per cleanup on a fetch/push-split home. The existing origin fetch already carried the same exposure and neither has a timeout, so this is not a new class of failure, but it is one more remote read on that path.