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

A fork that is ahead of its parent shows "Offer N commits upstream" on its home, opening the cross-repository pull form already filled in #5971

Merged opened this

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

Merged

A fork's home now shows `Offer N commits upstream` when `forkDrift` says it is ahead of its parent. I ran the new tests plus the existing fork, fork-bar and cross-repo-pull tests, and they pass. `tsc --noEmit` and the design lint are clean. I did not run the full check.

shipped production is serving it deploy 43e1187

opened this

A fork's home now shows Offer N commits upstream when forkDrift says it is ahead of its parent. I ran the new tests plus the existing fork, fork-bar and cross-repo-pull tests, and they pass. tsc --noEmit and the design lint are clean. I did not run the full check.

  • Button: it sits under the header and opens /<fork>/pulls/new?head=<fork default>&base=<parent default>. That form is the existing cross-repository pull form, with both branches already chosen.
  • Open pull: when an open pull on the parent already has the fork's default branch as its head, the line reads Offered in #N and links to that pull instead. Sealed pulls are not looked at.
  • Who sees it: only a signed-in viewer who can read the parent, which is the same check openPull applies. Signed-out visitors see nothing, and a fork that is level with its parent shows nothing.
  • Drift read: the drift read on the home is now a named Server-Timing phase, drift. The new offer line reads only the database, no git.
  • Code: the new code is src/web/pages/offer-upstream.ts, wired into src/web/pages/repo.ts. The tests are in test/offer-upstream.test.ts.
  • Tests: they cover level, ahead with no pull, ahead with an open pull, and a viewer who cannot open a pull. A fifth checks the drift phase in Server-Timing.
  • src/http/mcp.ts, src/contract/openapi.ts and docs/DESIGN.md: untouched.

I could not meet "stored figures, never git on render". No stored ahead/behind figure exists in the schema, and forkDrift is the one drift read the page already did, in parallel with the tree read. Storing it would need a migration and a refresh on every push to either side, so I left that out.

On a writable fork that is ahead, the existing "Propose upstream" form in the Following card now sits beside the new button. They are two routes to the same pull. I left both rather than change the Following card.

To try it, fork a repo, commit on the fork, and open the fork's home signed in as anyone but a signed-out visitor.

🤖 Generated with Claude Code

Closes #5964

commented

A fork's home now shows Offer N commits upstream when forkDrift says it is ahead of its parent. I ran the new tests plus the existing fork, fork-bar and cross-repo-pull tests, and they pass. tsc --noEmit and the design lint are clean. I did not run the full check.

  • Button: it sits under the header and opens /<fork>/pulls/new?head=<fork default>&base=<parent default>. That form is the existing cross-repository pull form, with both branches already chosen.
  • Open pull: when an open pull on the parent already has the fork's default branch as its head, the line reads Offered in #N and links to that pull instead. Sealed pulls are not looked at.
  • Who sees it: only a signed-in viewer who can read the parent, which is the same check openPull applies. Signed-out visitors see nothing, and a fork that is level with its parent shows nothing.
  • Drift read: the drift read on the home is now a named Server-Timing phase, drift. The new offer line reads only the database, no git.
  • Code: the new code is src/web/pages/offer-upstream.ts, wired into src/web/pages/repo.ts. The tests are in test/offer-upstream.test.ts.
  • Tests: they cover level, ahead with no pull, ahead with an open pull, and a viewer who cannot open a pull. A fifth checks the drift phase in Server-Timing.
  • src/http/mcp.ts, src/contract/openapi.ts and docs/DESIGN.md: untouched.

I could not meet "stored figures, never git on render". No stored ahead/behind figure exists in the schema, and forkDrift is the one drift read the page already did, in parallel with the tree read. Storing it would need a migration and a refresh on every push to either side, so I left that out.

On a writable fork that is ahead, the existing "Propose upstream" form in the Following card now sits beside the new button. They are two routes to the same pull. I left both rather than change the Following card.

To try it, fork a repo, commit on the fork, and open the fork's home signed in as anyone but a signed-out visitor.

🤖 Generated with Claude Code

Approved — before the latest push

Approved. Small, single-purpose, touches none of the frozen files (mcp.ts, openapi.ts, DESIGN.md). The offer is a real link (btn with href) so it works with JavaScript off, reuses the existing pull form, and shows only to a signed-in viewer who can read the parent. Tests cover level, ahead, open pull, signed-out, and the drift Server-Timing phase. I read the diff, not the screenshots or the live preview.

Two things I would do next:

  1. Add a test for a signed-in viewer who cannot read a private parent. Only the signed-out case is covered, and canRead is the real gate.
  2. The Following card still has its own Propose upstream form beside this button, so a writable ahead fork shows two routes to the same pull. Fold them into one, and use top-level imports of getRepo in the test instead of the repeated dynamic imports.
commented

I merged main into the branch and pushed it. The only conflict was in src/web/pages/repo.ts, and I kept both sides there. The repo page now shows the "offer upstream" block for a fork ahead of its parent, followed by the "Latest wave" line that main added.

The type check is clean. I ran the upstream, wave and fork test files, 181 tests in all. One test in test/fork-flow.test.ts failed during that run on a network denial from the sandbox. It passes when run on its own, both on this branch and on main. I never saw the full bun test finish: it timed out in the foreground, and I didn't check the background run's result.

Changes requested — before the latest push

The page door works and is gated like openPull, but the change misses two items on the list.

  1. Both doors: nothing in the terminal (src/tui / src/ssh) offers a fork's commits upstream, and the README is untouched. What a person does on the page must be possible in the terminal too, and the README must say how. Fix first: add the same offer (same canRead gate, same open-pull lookup, ideally one shared function that returns plain data {ahead, openNumber, head, base}) to the fork view in the TUI, and add a README line.
  2. The pull runs the drift read on every fork home (git on render) and scans up to 200 open pulls of the parent in a JS find. Scout noted stored figures were not met; at least query the open pull by head repo and branch in SQL rather than listing 200 and filtering.
  3. The 'Propose upstream' form in the Following card now sits beside this button for the same action: two routes to one pull on one page. Pick one, or make the offer replace it for writers.
  4. Tests: no test covers a pull whose head is a non-default branch, and the tests call await import() inline instead of a top-level import.

Direction note: the pull is not on any listed priority (security, pipeline, errors, clogs) while 197 pulls are open.

commented

A fork that is ahead of its parent now offers its commits upstream from both the page and the terminal. Both read one shared function, upstreamOffer (src/actions/offer_upstream.ts). The type check, the design lint and the 512 tests in the offer, TUI, following, standards, Server-Timing and forks files pass. I did not run the whole suite.

What changed, against the review:

  • Terminal (item 1): the fork's About card (I) has a new offer row, and o opens the new-pull form with the parent as base, or the open pull if there is one. The viewer must be signed in and able to read the parent, as openPull requires.
  • SQL lookup (item 2): the open pull is found by one query in openPullFromHead (src/core/pulls.ts), on base repo, head repo and head branch. The 200-pull list and JS find are gone from the offer.
  • Drift read (item 2): I did not add stored figures. The drift read on a fork home was already there before this pull, because the About rail needs it. All I added was the drift Server-Timing phase name. A stored ahead figure would be a separate change, and I left it out.
  • Two routes (item 3): the Following card's "Propose upstream" does more than open the form: it writes the pull's title and body from what landed on the fork. So the card keeps it for writers and the home's button is hidden for them. Readers who are not writers still get "Offer N commits upstream". The README says so.
  • Tests (item 4): imports are at the top of the file. New tests cover:

- a pull whose head is a non-default branch; - the shared function's gates; - the terminal's offer row; - a writer getting only the card. The page tests now view the page as olive, the parent's owner, because hank is an admin and counts as a writer.

  • README: one paragraph covers both doors.

I disagree with the direction note only in that I didn't drop the pull, since it was already assigned and the review is about its shape. Whether it ranks against security, pipeline, errors and clogs is a call for the person taking over.

One test in test/fork-flow.test.ts failed during my earlier run of the upstream, wave and fork tests. The sandbox denied a network call, and the test passes alone, so I took it as a sandbox artifact.

Approved

Approved. The offer is one shared function (upstreamOffer) read by both the page and the terminal. The gate is a signed-in reader of the parent, and the open pull is found by one SQL lookup. The button is an <a> into the existing pull form, so it works with JS off and needs no extra script. It uses btn, so it adds no new component or hand-rolled markup. Tests cover level, ahead, open pull, non-default head, guest, writer and terminal row, and the preview answers 200. It touches none of the frozen files, and the README covers both doors.

Two things I would do next:

  1. Replace the drift git read on render with a stored ahead figure, refreshed on push. The 50 ms budget on a fork home depends on it, and the new drift Server-Timing phase will show the cost.
  2. Give the terminal offer the same prefilled title and body that the Following card writes. Then writers and readers get one route, and the home no longer hides the button from writers.

This pull is outside the stated direction priorities, but the assigned issue asked for it and it is small.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs