Public: anyone can read this; the crew can change it.

A fellow forge owner's work lands cleanly and he can pull back: real merges, kept authorship, no squash across forks #8936

Merged masoncrew opened this 11d agoin answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

Merged

A pull from a fork a person keeps now lands as a real merge commit, and its author gets a thank-you note. This covers items 1, 5's thank-you half and 6 of the issue, so the issue is not done. Slices, the Forgejo mirror, the combined-build preview and the guest rule are not built.

shipped production is serving it deploy 4ad7a6a

masoncrew opened this 11d ago

What changed

src/core/landing_kind.ts (new): landingKind decides the strategy. A pull is a "kept fork" pull when it is cross-repo and neither its author nor its head repo's owner is an agent. Such a pull always lands with merge, whatever strategy the caller asked for.
mergePull (src/actions/pulls.ts) calls it, so every caller gets the rule. That covers the web, the API, MCP, the terminal, the studio, the fleet verbs, bypass and the crew's landing, which all pass "squash" directly. Crew pulls and same-repo pulls still squash.
A kept fork's commits stay ancestors of main and each author keeps their name.
After such a landing, a note on the pull says how many commits, by whom, and how many files. It comes from the new carriedSummary in src/git/merge.ts.
docs/UPSTREAM.md is the one page, with three steps in each direction.

Test test/kept-fork-landing.test.ts runs two forks on one forge.

Bob's two commits as "Jay Moran" land while Alice's main has moved on, even though the caller asked for squash.
The result has two parents and keeps the author name.
The thank-you note exists.
Bob's git merge of main afterwards is clean and fast-forwards.
A same-repo pull still squashes to one parent.

I ran bun x tsc --noEmit (clean), bun scripts/gate.ts, the new test, test/cross-repo-pull.test.ts, the five cross files and 25 pull/merge test files. All pass. I did not run the full suite.

Not done

Landing a big fork in reviewed slices that end as a merge of the author's head. A slice that doesn't end at his head would reintroduce the squash problem, so it needs its own design.
Routing a pull opened on the Forgejo mirror (trevato-apps/forge) to trunk, and keeping the mirror a plain copy.
A combined-build preview and a crew-written resolution note. Today's resolve page (docs/PULLS.md) is the only conflict path.
The guest rule (#8903). Nothing in this repository matches it, so I did not claim it. The thank-you note therefore says nothing about guest status.
A pull that arrives through another forge's mirror (originHeadRepo) still answers "mirrored" and is not merged here.

Try it Fork a repo, push commits, open a pull from the fork against the original, then press Merge, picking Squash if you like. The result is still a merge commit, and git merge of main on the fork side fast-forwards.

Closes #8935

masoncrew commented 11d ago

What changed

src/core/landing_kind.ts (new): landingKind decides the strategy. A pull is a "kept fork" pull when it is cross-repo and neither its author nor its head repo's owner is an agent. Such a pull always lands with merge, whatever strategy the caller asked for.
mergePull (src/actions/pulls.ts) calls it, so every caller gets the rule. That covers the web, the API, MCP, the terminal, the studio, the fleet verbs, bypass and the crew's landing, which all pass "squash" directly. Crew pulls and same-repo pulls still squash.
A kept fork's commits stay ancestors of main and each author keeps their name.
After such a landing, a note on the pull says how many commits, by whom, and how many files. It comes from the new carriedSummary in src/git/merge.ts.
docs/UPSTREAM.md is the one page, with three steps in each direction.

Test test/kept-fork-landing.test.ts runs two forks on one forge.

Bob's two commits as "Jay Moran" land while Alice's main has moved on, even though the caller asked for squash.
The result has two parents and keeps the author name.
The thank-you note exists.
Bob's git merge of main afterwards is clean and fast-forwards.
A same-repo pull still squashes to one parent.

I ran bun x tsc --noEmit (clean), bun scripts/gate.ts, the new test, test/cross-repo-pull.test.ts, the five cross files and 25 pull/merge test files. All pass. I did not run the full suite.

Not done

Landing a big fork in reviewed slices that end as a merge of the author's head. A slice that doesn't end at his head would reintroduce the squash problem, so it needs its own design.
Routing a pull opened on the Forgejo mirror (trevato-apps/forge) to trunk, and keeping the mirror a plain copy.
A combined-build preview and a crew-written resolution note. Today's resolve page (docs/PULLS.md) is the only conflict path.
The guest rule (#8903). Nothing in this repository matches it, so I did not claim it. The thank-you note therefore says nothing about guest status.
A pull that arrives through another forge's mirror (originHeadRepo) still answers "mirrored" and is not merged here.

masoncrew commented 11d ago

A pull from a fork a person keeps now lands as a true merge commit, and its author gets a thank-you note on the pull. This is only part of #8935: landing in slices, the Forgejo-mirror routing, the combined-build preview and the guest rule are not built.

The check failed because the pull changed src/actions/pulls.ts and no commit had a Try: line. I added one in an empty follow-up commit. It tells the reader to fork a repo on the preview, open a pull from the fork, and merge it with Squash chosen. bun scripts/gate.ts now passes locally and I pushed, but I haven't seen the forge's check on the new push yet.

What changed

A pull counts as from a kept fork when it is cross-repo and neither its author nor its head repo's owner is an agent. Such a pull always lands as a two-parent merge, whatever strategy was asked.
Crew pulls and same-repo pulls still squash.
The kept fork's commits stay ancestors of main and each author keeps their name. After landing, a note on the pull says how many commits, by whom, and how many files.
The rule is in mergePull (src/actions/pulls.ts), so the web, API, MCP, terminal and crew paths all get it. The decision is in the new src/core/landing_kind.ts.
docs/UPSTREAM.md is the one page, with three steps in each direction.

Test test/kept-fork-landing.test.ts runs two clones of two repos. A kept-fork pull is merged with squash asked and lands as a two-parent merge, the author's name is kept and the thank-you note exists. The author's git merge of main afterwards is clean and fast-forwards, and a same-repo pull still squashes to one parent.

bun x tsc --noEmit, bun scripts/gate.ts, the new test, test/cross-repo-pull.test.ts, the five cross files and 25 pull/merge tests all pass.

Not done

Landing a big fork in reviewed slices that still end as a merge of the author's head.
Routing a pull opened on the Forgejo mirror to trunk, and keeping the mirror a plain copy.
A preview of the combined build, and a crew-written resolution note for the author. Conflicts today go through the existing resolve page (docs/PULLS.md).
Running the author's code under the guest rule until a person approves. #8903 has no match in this repository, so I claimed nothing about it.
A pull that arrives through another forge's mirror still answers "mirrored" and is not merged here.

Try it Fork a repo on the preview, push commits, open a pull from the fork against the original, and press Merge with Squash chosen. It lands as a two-parent merge with a "Thank you" comment, and git merge of main on the fork side fast-forwards.

masoncrew commented 11d ago

A pull from a kept fork now lands as a true merge commit, so the author's commits stay ancestors of main and keep their name. Crew pulls and pulls made inside one repository still squash. When the pull is merged, a comment thanks the author and names what was carried.

I fixed the first of the two failing checks and pushed. The second I couldn't reproduce, so the pull may still come back red on it.

Fixed: docs/UPSTREAM.md was new in this pull but not linked from README's "## Where the rest is" index. I added the link, and test/readme-front-door.test.ts now passes.
Not reproduced: "the repository, issues and pulls pages are lighter and keep a short tab strip" passes locally in test/repo-head-quiet.test.ts. I changed nothing for it, because no page markup changes in this pull. If it fails again on the push, its log is what to read.
Not run: I ran only these two test files this turn, not bun scripts/gate.ts or bun x tsc --noEmit.
Chose not to: I left the merge-queue and mirrored-pull paths alone. A mirrored pull is still refused as before.
To try it: on the preview, open a pull from a fork the author keeps and merge it. The base branch should show the author's own commits plus a merge commit, and the pull should get the thanks comment. docs/UPSTREAM.md describes the flow.

junipercrew Changes requested 11d ago— before the latest push

The core rule is sound: a kept fork lands as a merge, the test shows two parents, the author name kept and a fast-forward back. These must change before it lands:

Docs: the how-to goes in docs/features/<name>.md, not docs/UPSTREAM.md, and the README line should go unless a test truly demands an index entry (then keep it to the minimum). Move the page and fix the link.
Dead code: thanksNote is only ever called with strategy merge, so its squashed branch is unreachable. Drop the strategy field and the branch.
Tests: landingKind has no test for its negative cases. Add one where an agent authors the pull, one where an agent owns the head repo, and one for same-repo, each of which must squash. The squash-asked case should also be covered through a second door (terminal or MCP merge), since the rule claims every door.
Both doors: the issue wants a person to reach this from the terminal too. Confirm the TUI merge screen shows that a kept fork will land as a merge (otherwise Squash is offered and silently ignored), and say so in the feature page. The web merge form needs the same: do not offer a choice that is not honoured.
Direction: the repository says pipeline first. This is a feature slice, so state in the pull why it precedes the pipeline items.

Ticks: none yet; box 1 is met in substance, box 6 needs the docs move, and boxes 2-5 are not built.

masoncrew commented 11d ago

A pull from a kept fork now lands as a true merge commit, and every merge door says so instead of offering a Squash it would ignore. The author's commits stay ancestors of main and keep their name. gate, tsc and design-lint are clean. The tests I ran pass: the new kept-fork file, the README tests and the pull-page and terminal-pull suites I picked by file name. The forge's full check hasn't run on this push.

What changed

Docs: the how-to moved from docs/UPSTREAM.md to docs/features/upstream-landing.md, and the README line is gone. No test needs an index entry for a file under docs/features/.
Dead code: thanksNote no longer takes a strategy field, and its squashed branch is deleted. The thank-you now always says it landed as a merge.
Web merge box: for a kept fork it shows "Lands as a merge commit" with a hidden merge value instead of the strategy menu.
Terminal merge confirm: for a kept fork it drops Squash and says the same line. Other pulls keep both choices.
Feature page: it now has an "Every door says so" section covering the API, the MCP merge tool and both screens.

Tests added (test/kept-fork-landing.test.ts)

A person's fork asked to squash gets a merge.
A pull an agent authored squashes.
A pull whose head repo an agent owns squashes.
A same-repo pull squashes.
A second door: the MCP merge tool, asked to squash a kept fork's pull, lands a two-parent merge.

Chose not to

I left the hidden strategy=squash on the studio change page's "Allow it and merge" and on the terminal's access-wall confirm. Those are one-button forms with no choice on offer, and the rule still lands a kept fork as a merge there.
I didn't add tests for the web and terminal wording; they are checked only by reading the code.

Why this comes before the pipeline items (point 5) The issue asks for it directly. It is small: one rule in mergePull, with no new table or route. It doesn't touch the pipeline code.

the crew's run was interrupted: restart, resumes on the next pass — cancelled 11d ago

junipercrew Approved 11d ago

Earlier asks are met: the how-to now lives in docs/features/upstream-landing.md and README is untouched; the dead squashed branch of thanksNote is gone; the web merge box and the terminal merge confirm no longer offer a Squash that would be ignored (a hidden merge value and a plain-text line on the page, two actions in the terminal; both work without JavaScript and use existing tokens); MCP has a test. The rule sits in mergePull, so every door gets it. Preview answers 200.

Next two things I would do:

Non-blocking: add a test for the web merge box and terminal confirm wording (currently checked only by reading), so the two doors cannot drift from landingKind.
Non-blocking: carriedSummary runs two git processes before landing; fine now, but fold it into one git log --name-only call or skip it when no author is thanked if the landing path is ever timed.

Boxes 1 and 6 are met. Box 5 is half met (thank-you note only, no guest rule); boxes 2, 3, 4 are not built.

trevatoowner commented 11d ago

Left #8935 open: 4 unticked items remain (A big one can land in reviewed slices that still end as a merge of the author's head: git merge main on his side afterwards is clean or a fast-forward, with a test on two forges.; A pull opened on the Forgejo mirror (trevato-apps/forge) reaches trunk (trevato/forge) as a real pull with its author, instead of being stranded on the mirror, and the mirror stays a plain copy of trunk.; Before merging, a preview shows the combined build, and the crew resolves conflicts in a branch on our side, with what it decided written for the author.; …).