PR #403 follow-ups: fast-forwarded branches bypass the merge-parent map nf-1yi4 ← Beads

closed priority 2 task unassigned activesession:a

Twinned from https://github.com/cfsh/ninefold/issues/411 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #403. None blocked that PR — all six #373 items are implemented and verified in the tree at ec48038. These are the residuals.

1. manifest.sh: a fast-forwarded branch never reaches the new second-parent map, so #373 item 1's under-report window is still open for it.

rebuild_f5.sh runs git merge --no-edit "origin/$br" with no --no-ff, so the first branch in the loop that is a pure descendant of origin/main fast-forwards — no merge commit, no 'origin/<br>' subject, no mergemap entry. It falls through to the git rev-parse "origin/$br" + --is-ancestor fallback, which is exactly the stale-vs-build path item 1 set out to remove.

Live right now on origin/f5/integration: 7 open PR branches, 5 merge commits.

``` 1763e11 [c4c968d 1483413] Merge 'origin/fix/userpath-guard-372' c4c968d [f0bf3f9 ec48038] Merge 'origin/fix/playtest-plumbing-373' f0bf3f9 [09f825d 77e00d1] Merge 'origin/fix/issue-383-tall-pennant' 09f825d [9c52d7e dbb110f] Merge 'origin/feat/probe-gate-389' 9c52d7e [d5bb2b1 470f8f9] Merge 'origin/feat/issue-366-row-label-probe' d5bb2b1 [3c10c10] /triage: group issues into named chunks <- feat/397-triage-command, FF'd 3c10c10 [ebe7f27] poll cadence: gate on change <- chore/396-session-lifecycle, contained ```

feat/397-triage-command (#402, head d5bb2b1) fast-forwarded in as the first merge, and chore/396-session-lifecycle (#401, head 3c10c10) is stacked underneath it. Neither is named by any merge subject, so both take the fallback: push to either after the f5 rebuild, run ./f5.sh before the next one, and the row is dropped from the manifest — 2 of 7 branches in today's build.

Cheapest fix is in the producer, not the reader: git merge --no-edit --no-ff "origin/$br" in tools/pr/rebuild_f5.sh, so every integrated branch leaves a merge commit and mergemap is always populated. The fallback then only ever handles genuine no-op merges (#334 under #335), which is what it was kept for.

2. CarryManifestForward compares the whole path, not the sha segment.

PlaytestJournal.CarryManifestForward copies whenever from != to. The routine trigger is a date-only rename (F5 the next morning), where the carried manifest still describes the running build — correct. But the rename also fires when the sha differs, and there the copied manifest.json names a build that is not the one running, and /playtest treats the manifest as ground truth for routing dictation to PRs. Before this PR that dir simply had no manifest, which was wrong-but-safe.

Narrow: it needs the assembly sha to move without ./f5.sh rewriting the flag file, i.e. a manual checkout in the f5 clone. A one-line guard — carry forward only when the segment after the date matches — closes it.

3. CurrentNameUnder's slash == 0 branch has no test.

PlaytestLog.CurrentNameUnder changed behaviour for a leading-slash path too (slash <= 0 returned dir; slash < 0 now rebuilds it as "/" + want), and the code comment asserts that outcome — but the six new cases cover bare-name, empty, trailing-separator, ancestor-preservation and both no-op paths, not this one. Item 6's whole argument was making the branch reachable from the test project; this is the one branch still asserted only in prose.

4. .claude/commands/playtest.md never mentions manifest.json.

Item 4 offered two resolutions — carry the file forward, or write the sibling-resolution rule into the skill contract — and #403 took the copy. Fair. But the skill doc still has no reference to manifest.json at all, so the file the code calls "ground truth for /playtest" has no documented consumer. Out of #403's scope; tracking it here so it is not lost.

Dependencies

None.

Comments

No comments.

Add a comment