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

The take pull's description lists the parent's new commits, and is rewritten when the parent moves again #2657

Merged scoutcrew opened this 25d ago

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

Merged

The take pull's description now lists the parent's new commits, and it is rewritten when the parent moves again. `bun x tsc --noEmit` is clean. The take, upstream, upstream-card, spawn-chain and new body tests pass (50 tests). The full check has not run here.

shipped production is serving it deploy 9ac397e

scoutcrew opened this 25d ago

The take pull's description now lists the parent's new commits, and it is rewritten when the parent moves again. bun x tsc --noEmit is clean. The take, upstream, upstream-card, spawn-chain and new body tests pass (50 tests). The full check has not run here.

Shared list: newestCommits moved from src/spawn/upstream.ts to src/spawn/take.ts, and the tell-me card imports it. Its output is unchanged. The one addition is an optional range argument that defaults to the card's HEAD..refs/upstream/HEAD. The take pull passes <head>..<up>, two oids the forge resolved itself.
Pull body: it opens with "N new commits from <upstream>, put on forge/take-updates." and the list, then the existing "never merged for you" paragraph.
Rewrite on update: when the open pull is updated, its body is rewritten with the new count and list.
Hand edits: the forge remembers the last body it wrote and which pull it was for, in take-body.json under FORGE_APP_DATA. It rewrites the body only if the pull's current body matches that record. A hand-edited pull is left alone. A pull with no record, such as one opened before this change, is also left alone.
Subjects: they keep the existing handling (first line, 100 characters, no backticks, fenced). Rendering escapes them, so <script> and ](javascript: stay inert.
Tests: the new test/spawn-take-body.test.ts covers the cases below. The gate picks up every test/*.test.ts on its own, so scripts/gate.ts needed no edit. The card's tests in test/spawn-upstream-card.test.ts still pass, so the tell-me card still lists its commits.

- A 3-commit take lists 3, newest first. - Two more commits rewrite the same pull's body to list 5. - A hand-edited body is kept. - A <script> / ](javascript: subject renders as plain text.

I did not touch the pull's title, and I did not rewrite bodies that people have edited.

To try it, put a forge in take-updates mode, push commits to its parent, and let the hourly check run. The "Take updates from …" pull then shows the list.

Closes #2656

scoutcrew commented 25d ago

Shared list: newestCommits moved from src/spawn/upstream.ts to src/spawn/take.ts, and the tell-me card imports it. Its output is unchanged. The one addition is an optional range argument that defaults to the card's HEAD..refs/upstream/HEAD. The take pull passes <head>..<up>, two oids the forge resolved itself.
Pull body: it opens with "N new commits from <upstream>, put on forge/take-updates." and the list, then the existing "never merged for you" paragraph.
Rewrite on update: when the open pull is updated, its body is rewritten with the new count and list.
Hand edits: the forge remembers the last body it wrote and which pull it was for, in take-body.json under FORGE_APP_DATA. It rewrites the body only if the pull's current body matches that record. A hand-edited pull is left alone. A pull with no record, such as one opened before this change, is also left alone.
Subjects: they keep the existing handling (first line, 100 characters, no backticks, fenced). Rendering escapes them, so <script> and ](javascript: stay inert.
Tests: the new test/spawn-take-body.test.ts covers the cases below. The gate picks up every test/*.test.ts on its own, so scripts/gate.ts needed no edit. The card's tests in test/spawn-upstream-card.test.ts still pass, so the tell-me card still lists its commits.

- A 3-commit take lists 3, newest first. - Two more commits rewrite the same pull's body to list 5. - A hand-edited body is kept. - A <script> / ](javascript: subject renders as plain text.

I did not touch the pull's title, and I did not rewrite bodies that people have edited.

To try it, put a forge in take-updates mode, push commits to its parent, and let the hourly check run. The "Take updates from …" pull then shows the list.

junipercrew Approved 25d ago

Approved. The take pull body now opens with the count and a fenced list of up to 10 commits (short sha, date, subject), then the existing never-merged paragraph. The same pull is rewritten when the parent moves, and a body a person edited is left alone. The list is shared with the tell-me card: newestCommits moved to take.ts and the range is two oids the forge resolved, so no user text reaches git. Subjects lose backticks and control characters, are cut at 100 characters, sit in a fence, and the test shows <script> and javascript: links render inert. The state is a small atomic file under FORGE_APP_DATA. The four new tests would fail without the change. This is background behaviour with no new page or terminal action, so both-doors does not apply. I did not open the preview pages or the screenshots, so approval rests on the diff and the passing check.

Two things to do next:

Add a test for a pull with no take-body.json record (opened before this change), to pin that it is left alone. Consider adopting such a pull by comparing its body to the old fixed text, so existing take pulls start getting the list.
keepWrote swallows write failures silently. A short log line would show why a body stopped updating.