ee50cdf · 691943d · 946157c
@@ -30,3 +30,5 @@
The terminal has the same door: the palette's "Notes" (`src/tui/screens/thoughtn
A note sent from an app's forge button (or the terminal's "Tell the builders") files a from-app issue, and the sender is told its number at once: "Filed as #N". Anyone the repository would show the issue to (`canRead`, so a signed-in reader of a private repository too, not only a public app on a public repository) also gets the issue's link and the repository's; everyone else gets the number and no link. The list shows `#N` beside each note. The browser button links only to the forge's own origin. In the terminal (`src/tui/screens/notes.ts`) the sender is the SSH user: the confirmation reads "Sent as #N, owner/repo/issues/N" and the list and moderation screens carry `#N`; the terminal has no clickable link.
The crew's answers come back to the sender: under each note the button's list (and the terminal's "Notes from the app", where picking a note shows them) carries the comments on its issue by the repository's crew and writers, with author and time, oldest first. Anyone else's comments stay on the forge. The button shows the latest five and says how many are earlier.
When a pull that closes a note's issue merges, the forge leaves one plain-words comment on it (`crew/note_closed.ts`): what was wrong and what changed, from the pull's description, with no hashes or links, saying "the next release" until one is cut, then rewritten to name it ("release 57 (10 Nov 2026)"). Both ways a merge closes an issue (the merge itself, and a reviewer's box pass on an already-landed pull) go through `noteShipped`. The forge marks its comment in `note_shipped`, so a person's comment with the same opening never stands in for it. The button shows it as a reply and the note reads shipped.
@@ -14,6 +14,7 @@
import type { App } from "../app.ts";
import { PASS_LEAD, tickBoxes, unticked } from "../core/done_on_main.ts";
import { addComment, getIssue, setIssueState as setIssueStateCore, updateIssueText } from "../core/issues.ts";
import { noteShipped } from "../crew/note_closed.ts";
import { isStructural, pillarsOf } from "../core/pillars.ts";
import { closesFromBody, type Pull } from "../core/pulls.ts";
import type { Repo } from "../core/repos.ts";
@@ -141,12 +142,14 @@
export function applyBoxPass(app: App, repo: Repo, pull: Pull, reviewer: User, p
        });
    }
    // A pull that landed before its boxes were read closes the issue now, as the landing would have.
    if (pull.mergedAt !== null && unticked(body).length === 0 && setIssueStateCore(app.db, issue.id, "closed"))
    if (pull.mergedAt !== null && unticked(body).length === 0 && setIssueStateCore(app.db, issue.id, "closed")) {
      app.bus.append({
        kind: "issue.closed",
        repoId: repo.id,
        actorId: reviewer.id,
        payload: { number: n, by: "merge", pull: pull.number },
      });
      noteShipped(app, reviewer.id, repo, issue, pull);
    }
  }
}
@@ -10,6 +10,7 @@
import { takeGuestPull } from "./guests.ts";
import { requireAbility } from "./roles.ts";
import { announceTeamWaits } from "./team_waits.ts";
import { addComment as addIssueComment } from "../core/issues.ts";
import { noteShipped } from "../crew/note_closed.ts";
import { landedNote, leftNote, unticked } from "../core/done_on_main.ts";
import { resolveOpen } from "../core/decisions.ts";
import type { App } from "../app.ts";
@@ -649,6 +650,8 @@
export async function mergePull(
      actorId: writer.value.id,
      payload: { number: issue.number, by: "merge", pull: pull.number },
    });
    // A note from an app's button hears, in plain words, that it shipped (#10564).
      
        junipercrew
        1d ago
        
      
      box_pass.ts:149 closes issues too and is not covered. Share one helper for both paths.
    
    noteShipped(app, writer.value.id, repo, issue, pull);
  }
  return merged;
}
@@ -21,6 +21,7 @@
import {
} from "../release/checkout.ts";
import { beginSwap, endSwap, handover, startRelease } from "../release/swap.ts";
import { running } from "../kernel/version.ts";
import { noteReleased } from "../crew/note_closed.ts";
import { recordRelease, type Release } from "../release/record.ts";
import { scanHold } from "../release/scan_hold.ts";
import type { Pull } from "../core/pulls.ts";
@@ -192,6 +193,8 @@
async function cut(
    app.log.warn("release held by code scanning", { sha: sha.slice(0, 12), pull: pull.number, findings: hold.count });
    addComment(app, by, repo, getIssueById(app.db, pull.id) ?? pull, `The landed tree ${sha.slice(0, 12)} is not released: ${hold.line}.`);
  }
  // The notes this pull closed learn which release carries their fix (#10564).
  if (verdict.ok && !hold.held) noteReleased(app.db, release);
  if (verdict.ok && out.ok && !hold.held) await swapIn(app, by, repo, pull, sha, out.value);
  return release;
}
@@ -0,0 +1,1 @@
a1c7edf33c001c572a64ae81284a9832e7d9cb74853ea3b7a37c549d48cc5596
@@ -0,0 +1,11 @@
// The forge's own "this note shipped" comment is marked, so a second landing
// finds it by what the forge stored, never by what the comment says (#10564).
// One row per issue; the pull is kept so the release can be named later. Additive.
export default `
  CREATE TABLE note_shipped (
    issue_id    TEXT PRIMARY KEY REFERENCES issue(id) ON DELETE CASCADE,
    comment_id  TEXT NOT NULL REFERENCES comment(id) ON DELETE CASCADE,
    pull_number INTEGER NOT NULL
  );
  CREATE INDEX note_shipped_pull ON note_shipped (pull_number);
  `;
@@ -0,0 +1,113 @@
/**
 * A note's fix has landed (#10564): the pull that closes a `from-app` issue
 * leaves one comment on it, in plain words, written from the pull's own
 * description. The forge button shows it with the note, which reads shipped
 * once the issue is closed (`actions/app_notes.ts`). Pure text plus one
 * idempotent write; both ways a merge closes an issue (`mergePull`, the box
 * pass) call `noteShipped`. The comment is found again by the `note_shipped`
 * row the forge stores with it, never by its words, and when the release is
 * cut `noteReleased` rewrites it to name that release.
 */
import type { Database } from "bun:sqlite";
import { addComment, type Comment, type Issue } from "../core/issues.ts";
import { fromAppIssues } from "../core/from_app.ts";
import type { Pull } from "../core/pulls.ts";
import { getPull } from "../core/pulls.ts";
import { releaseLabel, releaseOfPull, type Release } from "../release/record.ts";
import type { App } from "../app.ts";
import type { Repo } from "../core/repos.ts";
const MAX = 600;
/** Starts every such comment; only for reading, never for finding one again. */
const LEAD = "Good news: this is fixed";
/** Lines of a description that say nothing to a sender: closings, code, headings' marks, links, attribution. */
function plain(text: string): string {
  return text
      
        junipercrew
        1d ago
        outdated
      
      This hex strip removes real words and long numbers. Anchor it to a context such as 'commit <sha>' or require at least one digit.
    
    .replace(/```[\s\S]*?```/g, " ")
    .replace(/^\s*(close[sd]?|fix(e[sd])?|resolve[sd]?)\b[^\n]*#\d[^\n]*$/gim, " ")
    .replace(/^[^\n\w]*generated (with|by)\b[^\n]*$/gim, " ")
    .replace(/^\s*co-authored-by:[^\n]*$/gim, " ")
    .replace(/!?\[([^\]]*)\]\([^)]*\)/g, "$1")
    .replace(/https?:\/\/\S+/g, "")
    .replace(/`([^`]*)`/g, "$1")
    .replace(/^\s{0,3}(#{1,6}|[-*>]|\d+\.)\s+/gm, "")
    .replace(/[*_~]{1,3}/g, "")
    .replace(/#\d+/g, "")
    // A hash is named as one ("commit abc1234") or has digits and letters both; a word or a number is not.
    .replace(/\b(commit|sha|revision)\s+[0-9a-f]{7,40}\b/gi, "")
    .replace(/\b(?=[0-9a-f]*\d)(?=[0-9a-f]*[a-f])[0-9a-f]{7,40}\b/gi, "")
    .replace(/\(\s*[,;]?\s*\)/g, "")
    .replace(/\s+([.,;)])/g, "$1")
    .replace(/[ \t]+/g, " ");
}
/** What was wrong and what changed, from a pull's title and description: no hashes, no markup, short. */
export function plainSummary(title: string, body: string): string {
  const paras = plain(body)
    .split(/\n\s*\n/)
    .map((p) => p.replace(/\s*\n\s*/g, " ").trim())
    .filter((p) => p !== "");
  // The description's opening carries what changed; the rest is process.
  let text = paras.slice(0, 2).join("\n\n");
  if (text === "") text = plain(title).replace(/\s+/g, " ").trim();
  if (text.length > MAX) text = `${text.slice(0, MAX - 3).replace(/\s+\S*$/, "")}...`;
  return text;
      
        junipercrew
        1d ago
        outdated
      
      Dedup on body text is fragile. A person's comment with this opening suppresses the real one. Use a marker the forge writes, such as the comment kind.
    
}
/** The comment's words. */
export function shippedWords(pull: Pull, release: string | undefined): string {
  const where = release === undefined ? "It ships in the next release" : `It shipped in ${release}`;
  return `${LEAD}. ${where}.\n\n${plainSummary(pull.title, pull.body)}`;
}
/** The comment the forge wrote for this issue, if it has. */
      
        junipercrew
        1d ago
        outdated
      
      releaseName gives a 12-char commit hash, and at merge time release is nearly always undefined. The issue wants the release named in plain words. Update the comment when the release is cut, with a readable label.
    
function told(db: Database, issueId: string): { commentId: string; pullNumber: number } | undefined {
  const row = db
    .query<{ comment_id: string; pull_number: number }, [string]>("SELECT comment_id, pull_number FROM note_shipped WHERE issue_id = ?1")
    .get(issueId);
  return row === null ? undefined : { commentId: row.comment_id, pullNumber: row.pull_number };
}
const wordsFor = (db: Database, pull: Pull, release: Release | undefined): string =>
  shippedWords(pull, release === undefined ? undefined : releaseLabel(release));
/**
 * Say a landed fix on `issue` once. Nothing when the issue is not a note from
 * an app, or the forge already wrote the comment. Returns the new comment.
 */
export function tellNoteShipped(db: Database, by: string, issue: Issue, pull: Pull): Comment | undefined {
  if (!fromAppIssues(db, [issue.id]).has(issue.id)) return undefined;
  if (told(db, issue.id) !== undefined) return undefined;
  const made = addComment(db, { issueId: issue.id, authorId: by, body: wordsFor(db, pull, releaseOfPull(db, pull.repoId, pull.number)) });
  if (!made.ok) return undefined;
  db.query("INSERT OR IGNORE INTO note_shipped (issue_id, comment_id, pull_number) VALUES (?1, ?2, ?3)").run(issue.id, made.value.id, pull.number);
  return made.value;
}
/** The one way a merge tells a note's sender: write the comment and announce it. */
export function noteShipped(app: App, by: string, repo: Repo, issue: Issue, pull: Pull): void {
  const said = tellNoteShipped(app.db, by, issue, getPull(app.db, repo.id, pull.number) ?? pull);
  if (said === undefined) return;
  app.bus.append({
    kind: "issue.commented",
    repoId: repo.id,
    actorId: by,
    payload: { number: issue.number, body: said.body, commentId: said.id },
  });
}
/** A release carrying `pull` was cut: its notes' comments now name it. Returns how many were rewritten. */
export function noteReleased(db: Database, release: Release): number {
  if (!release.ok || release.pullNumber === null) return 0;
  const pull = getPull(db, release.repoId, release.pullNumber);
  if (pull === undefined) return 0;
  const rows = db
    .query<{ comment_id: string }, [number, string]>(
      `SELECT s.comment_id FROM note_shipped s JOIN issue i ON i.id = s.issue_id WHERE s.pull_number = ?1 AND i.repo_id = ?2`,
    )
    .all(release.pullNumber, release.repoId);
  const body = wordsFor(db, pull, release);
  for (const r of rows) db.query("UPDATE comment SET body = ?1 WHERE id = ?2").run(body, r.comment_id);
  return rows.length;
}
@@ -82,3 +82,10 @@
export function releaseOfPull(db: Database, repoId: string, pullNumber: number):
/** What a release is called where people read it: the landed commit, short. */
export const releaseName = (r: Release): string => r.sha.slice(0, 12);
/** What a release is called in plain words: its number and the day it was cut. */
export const releaseLabel = (r: Release): string => {
  const day = new Date(r.createdAt);
  const when = Number.isNaN(day.getTime()) ? r.createdAt.slice(0, 10) : day.toLocaleDateString("en-GB", { day: "numeric", month: "short", year: "numeric", timeZone: "UTC" });
  return `release ${r.id} (${when})`;
};
@@ -0,0 +1,174 @@
/** A merged pull that closes a note tells its sender, once, in plain words (#10564). */
import { afterAll, beforeAll, describe, expect, test } from "bun:test";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { newApp } from "../src/actions/apps.ts";
import { notesFor, sendNote, type Sender } from "../src/actions/app_notes.ts";
import { mergePull, openPull } from "../src/actions/pulls.ts";
import { commitToBranch } from "../src/actions/repos.ts";
import { createApp, type App } from "../src/app.ts";
import { getAppBySlug } from "../src/core/apps.ts";
import { getIssue, listComments, addComment, setIssueState } from "../src/core/issues.ts";
import { getPull } from "../src/core/pulls.ts";
import { getRepo } from "../src/core/repos.ts";
import { createUser, type User } from "../src/core/users.ts";
import { applyBoxPass } from "../src/actions/box_pass.ts";
import { recordRelease } from "../src/release/record.ts";
import { noteReleased, plainSummary, shippedWords, tellNoteShipped } from "../src/crew/note_closed.ts";
import { quietLogs } from "./helpers/globals.ts";
quietLogs();
describe("plainSummary", () => {
  test("drops hashes, links, code, closings and markup", () => {
    const out = plainSummary(
      "t",
      "## What\n\nThe **button** was hidden on phones (see https://x.test/a, `src/a.ts`, 0123abcd45ef).\n\nCloses #12\n\n```\ncode\n```\n\n- tests",
    );
    expect(out).not.toMatch(/https?:|`|\*|0123abcd|#12|code\n/);
    expect(out).toContain("The button was hidden on phones");
  });
  test("keeps the attribution line out of a short description", () => {
    const out = plainSummary("t", "Fixed the tiny button.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)\n\nCo-Authored-By: Claude <[email protected]>");
    expect(out).toBe("Fixed the tiny button.");
  });
  test("strips hashes but not words or numbers", () => {
    const out = plainSummary("t", "The defaced banner showed 1234567 visitors, was in commit 0123abcd45ef (also 9f8e7d6c5b4a).");
    expect(out).toContain("defaced");
    expect(out).toContain("1234567");
    expect(out).not.toMatch(/0123abcd|9f8e7d6c|\(\)|commit/);
  });
  test("falls back to the title and stays short", () => {
    expect(plainSummary("Make it blue", "")).toBe("Make it blue");
    expect(plainSummary("t", "word ".repeat(400)).length).toBeLessThanOrEqual(600);
  });
});
describe("a closing pull tells the note's sender", () => {
  let dataDir: string;
  let app: App;
  let alice: User;
  beforeAll(async () => {
    dataDir = await mkdtemp(join(tmpdir(), "forge-note-closed-"));
    app = createApp({ dataDir, port: 0, host: "forge.test" });
    const made = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", adminIfFirst: true });
    if (!made.ok) throw new Error(String(made.error));
    alice = made.value;
    const a = await newApp(app, alice, { name: "sandbox-game", description: "Words", visibility: "private" });
    if (!a.ok) throw new Error(String(a.error));
  });
  afterAll(async () => {
    app.close();
    await rm(dataDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 });
  });
  test("merging posts one comment, a second delivery posts none, and the status reads shipped", async () => {
    const record = getAppBySlug(app.db, "sandbox-game")!;
    const repo = getRepo(app.db, "alice", "sandbox-game")!;
    const who: Sender = { key: "v:closer1", name: "visitor", person: false };
    const sent = await sendNote(app, record, repo, "production", undefined, who, { text: "the button is tiny", page: "", viewport: "390x844" }, "10.2.0.1");
    if (!sent.ok) throw new Error(sent.error.message);
    const n = sent.value.number;
    expect(sent.value.status.state).not.toBe("shipped");
    const body = `The button was too small to tap on phones. It is now 48px tall (commit 0123abcd45ef).\n\nCloses #${n}`;
    const seeded = await commitToBranch(app, alice, repo, { message: "seed", files: [{ path: "README.md", content: "hi\n" }] });
    expect(seeded.ok).toBe(true);
    const branched = await commitToBranch(app, alice, repo, { branch: "fix", base: "main", message: "fix", files: [{ path: "a.txt", content: "x\n" }] });
    expect(branched.ok).toBe(true);
    const opened = await openPull(app, alice, repo, { title: "Bigger button", body, baseBranch: "main", headBranch: "fix" });
    if (!opened.ok) throw new Error(JSON.stringify(opened.error));
    const merged = await mergePull(app, alice, repo, opened.value, "squash");
    expect(merged.ok).toBe(true);
    const issue = getIssue(app.db, repo.id, n)!;
    expect(issue.state).toBe("closed");
    const said = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
    expect(said()).toHaveLength(1);
    expect(said()[0]!.body).toContain("too small to tap on phones");
    expect(said()[0]!.body).not.toMatch(/0123abcd|#\d/);
    expect(said()[0]!.body).toContain("next release");
    // Re-delivery: the same landing again says nothing more.
    expect(tellNoteShipped(app.db, alice.id, issue, opened.value)).toBeUndefined();
    expect(said()).toHaveLength(1);
    const note = notesFor(app, record, repo, undefined, who)[0]!;
    expect(note.status.state).toBe("shipped");
    expect(note.replies.map((r) => r.text)).toEqual([said()[0]!.body]);
  });
  test("an issue that is not a note hears nothing", async () => {
    const repo = getRepo(app.db, "alice", "sandbox-game")!;
    const { openIssueThrough } = await import("../src/actions/issues.ts");
    const plainIssue = await openIssueThrough(app, alice, repo, { title: "Plain", body: "x" });
    if (!plainIssue.ok) throw new Error(String(plainIssue.error));
    const pull = { number: 1, repoId: repo.id, title: "t", body: "" } as never;
    expect(tellNoteShipped(app.db, alice.id, plainIssue.value, pull)).toBeUndefined();
    expect(shippedWords(pull, "abc")).toContain("shipped in abc");
  });
  async function land(text: string, title: string, bodyOf: (n: number) => string, branch: string, merge = true) {
    const record = getAppBySlug(app.db, "sandbox-game")!;
    const repo = getRepo(app.db, "alice", "sandbox-game")!;
    const who: Sender = { key: `v:${branch}`, name: "visitor", person: false };
    const sent = await sendNote(app, record, repo, "production", undefined, who, { text, page: "", viewport: "390x844" }, `10.2.0.${branch.length}`);
    if (!sent.ok) throw new Error(sent.error.message);
    const n = sent.value.number;
    const b = await commitToBranch(app, alice, repo, { branch, base: "main", message: branch, files: [{ path: `${branch}.txt`, content: "x\n" }] });
    expect(b.ok).toBe(true);
    const opened = await openPull(app, alice, repo, { title, body: bodyOf(n), baseBranch: "main", headBranch: branch });
    if (!opened.ok) throw new Error(JSON.stringify(opened.error));
    if (merge) expect((await mergePull(app, alice, repo, opened.value, "squash")).ok).toBe(true);
    return { n, repo, pull: opened.value, record, who };
  }
  const marked = (issueId: string) => app.db.query("SELECT comment_id FROM note_shipped WHERE issue_id = ?1").all(issueId);
  test("a person's comment that opens the same way does not stand in for the forge's", async () => {
    const { n, repo } = await land("the logo is blurry", "Sharper logo", (k) => `The logo was blurry. It is sharp now.\n\nCloses #${k}`, "logo", false);
    const issue = getIssue(app.db, repo.id, n)!;
    addComment(app.db, { issueId: issue.id, authorId: alice.id, body: "Good news: this is fixed, says a person" });
    const pull = getPullFor(repo.id, "logo");
    expect(tellNoteShipped(app.db, alice.id, issue, pull)).toBeDefined();
    expect(marked(issue.id)).toHaveLength(1);
  });
  test("the comment names the release once one is cut, in words, not a hash", async () => {
    const { n, repo, pull } = await land("the footer overlaps", "Fix footer", (k) => `The footer overlapped the text. It no longer does.\n\nCloses #${k}`, "footer");
    const issue = getIssue(app.db, repo.id, n)!;
    const text = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
    expect(text()[0]!.body).toContain("next release");
    expect(noteReleased(app.db, recordRelease(app.db, { sha: "f00dfeedcafe1234", repoId: repo.id, pullNumber: pull.number, ok: false, line: "bad" }))).toBe(0);
    const rel = recordRelease(app.db, { sha: "f00dfeedcafe1234", repoId: repo.id, pullNumber: pull.number, ok: true, line: "ok" });
    expect(noteReleased(app.db, rel)).toBe(1);
    expect(text()).toHaveLength(1);
    expect(text()[0]!.body).toContain(`release ${rel.id} (`);
    expect(text()[0]!.body).not.toMatch(/next release|f00dfeed/);
    expect(text()[0]!.body).toContain("footer overlapped");
  });
  test("the box pass's closing path tells the sender too, once", async () => {
    const { n, repo, pull } = await land("the menu is cut off", "Menu fits", (k) => `The menu was cut off. It fits now.\n\nCloses #${k}`, "menu");
    const issue = getIssue(app.db, repo.id, n)!;
    // As if the pull had landed before its boxes were read: the issue is open and unannounced.
    app.db.query("DELETE FROM comment WHERE id = (SELECT comment_id FROM note_shipped WHERE issue_id = ?1)").run(issue.id);
    expect(marked(issue.id)).toHaveLength(0);
    setIssueState(app.db, issue.id, "open");
    const merged = getPullFor(repo.id, "menu");
    const pass = { ticks: [{ issue: n, box: 1, text: "x" }] };
    applyBoxPass(app, repo, merged, alice, pass);
    expect(getIssue(app.db, repo.id, n)!.state).toBe("closed");
    const said = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
    expect(said()).toHaveLength(1);
    expect(said()[0]!.body).toContain("menu was cut off");
    expect(pull.number).toBe(merged.number);
  });
  function getPullFor(repoId: string, head: string) {
    const row = app.db.query<{ number: number }, [string, string]>("SELECT issue.number FROM pull JOIN issue ON issue.id = pull.issue_id WHERE issue.repo_id = ?1 AND pull.head_branch = ?2").get(...([repoId, head] as [string, string]));
    if (row === null) throw new Error("no pull");
    return getPull(app.db, repoId, row.number)!;
  }
});
		@@ -30,3 +30,5 @@ The terminal has the same door: the palette's "Notes" (`src/tui/screens/thoughtn
30	30	A note sent from an app's forge button (or the terminal's "Tell the builders") files a from-app issue, and the sender is told its number at once: "Filed as #N". Anyone the repository would show the issue to (`canRead`, so a signed-in reader of a private repository too, not only a public app on a public repository) also gets the issue's link and the repository's; everyone else gets the number and no link. The list shows `#N` beside each note. The browser button links only to the forge's own origin. In the terminal (`src/tui/screens/notes.ts`) the sender is the SSH user: the confirmation reads "Sent as #N, owner/repo/issues/N" and the list and moderation screens carry `#N`; the terminal has no clickable link.
31	31
32	32	The crew's answers come back to the sender: under each note the button's list (and the terminal's "Notes from the app", where picking a note shows them) carries the comments on its issue by the repository's crew and writers, with author and time, oldest first. Anyone else's comments stay on the forge. The button shows the latest five and says how many are earlier.
	33
	34	When a pull that closes a note's issue merges, the forge leaves one plain-words comment on it (`crew/note_closed.ts`): what was wrong and what changed, from the pull's description, with no hashes or links, saying "the next release" until one is cut, then rewritten to name it ("release 57 (10 Nov 2026)"). Both ways a merge closes an issue (the merge itself, and a reviewer's box pass on an already-landed pull) go through `noteShipped`. The forge marks its comment in `note_shipped`, so a person's comment with the same opening never stands in for it. The button shows it as a reply and the note reads shipped.
		@@ -14,6 +14,7 @@
14	14	import type { App } from "../app.ts";
15	15	import { PASS_LEAD, tickBoxes, unticked } from "../core/done_on_main.ts";
16	16	import { addComment, getIssue, setIssueState as setIssueStateCore, updateIssueText } from "../core/issues.ts";
	17	import { noteShipped } from "../crew/note_closed.ts";
17	18	import { isStructural, pillarsOf } from "../core/pillars.ts";
18	19	import { closesFromBody, type Pull } from "../core/pulls.ts";
19	20	import type { Repo } from "../core/repos.ts";
		@@ -141,12 +142,14 @@ export function applyBoxPass(app: App, repo: Repo, pull: Pull, reviewer: User, p
141	142	});
142	143	}
143	144	// A pull that landed before its boxes were read closes the issue now, as the landing would have.
144		if (pull.mergedAt !== null && unticked(body).length === 0 && setIssueStateCore(app.db, issue.id, "closed"))
	145	if (pull.mergedAt !== null && unticked(body).length === 0 && setIssueStateCore(app.db, issue.id, "closed")) {
145	146	app.bus.append({
146	147	kind: "issue.closed",
147	148	repoId: repo.id,
148	149	actorId: reviewer.id,
149	150	payload: { number: n, by: "merge", pull: pull.number },
150	151	});
	152	noteShipped(app, reviewer.id, repo, issue, pull);
	153	}
151	154	}
152	155	}
		@@ -10,6 +10,7 @@ import { takeGuestPull } from "./guests.ts";
10	10	import { requireAbility } from "./roles.ts";
11	11	import { announceTeamWaits } from "./team_waits.ts";
12	12	import { addComment as addIssueComment } from "../core/issues.ts";
	13	import { noteShipped } from "../crew/note_closed.ts";
13	14	import { landedNote, leftNote, unticked } from "../core/done_on_main.ts";
14	15	import { resolveOpen } from "../core/decisions.ts";
15	16	import type { App } from "../app.ts";
		@@ -649,6 +650,8 @@ export async function mergePull(
649	650	actorId: writer.value.id,
650	651	payload: { number: issue.number, by: "merge", pull: pull.number },
651	652	});
	653	// A note from an app's button hears, in plain words, that it shipped (#10564).
		junipercrew 1d ago `box_pass.ts:149` closes issues too and is not covered. Share one helper for both paths.
	654	noteShipped(app, writer.value.id, repo, issue, pull);
652	655	}
653	656	return merged;
654	657	}
		@@ -21,6 +21,7 @@ import {
21	21	} from "../release/checkout.ts";
22	22	import { beginSwap, endSwap, handover, startRelease } from "../release/swap.ts";
23	23	import { running } from "../kernel/version.ts";
	24	import { noteReleased } from "../crew/note_closed.ts";
24	25	import { recordRelease, type Release } from "../release/record.ts";
25	26	import { scanHold } from "../release/scan_hold.ts";
26	27	import type { Pull } from "../core/pulls.ts";
		@@ -192,6 +193,8 @@ async function cut(
192	193	app.log.warn("release held by code scanning", { sha: sha.slice(0, 12), pull: pull.number, findings: hold.count });
193	194	addComment(app, by, repo, getIssueById(app.db, pull.id) ?? pull, `The landed tree ${sha.slice(0, 12)} is not released: ${hold.line}.`);
194	195	}
	196	// The notes this pull closed learn which release carries their fix (#10564).
	197	if (verdict.ok && !hold.held) noteReleased(app.db, release);
195	198	if (verdict.ok && out.ok && !hold.held) await swapIn(app, by, repo, pull, sha, out.value);
196	199	return release;
197	200	}
		@@ -0,0 +1,1 @@
	1	a1c7edf33c001c572a64ae81284a9832e7d9cb74853ea3b7a37c549d48cc5596
		@@ -0,0 +1,11 @@
	1	// The forge's own "this note shipped" comment is marked, so a second landing
	2	// finds it by what the forge stored, never by what the comment says (#10564).
	3	// One row per issue; the pull is kept so the release can be named later. Additive.
	4	export default `
	5	CREATE TABLE note_shipped (
	6	issue_id TEXT PRIMARY KEY REFERENCES issue(id) ON DELETE CASCADE,
	7	comment_id TEXT NOT NULL REFERENCES comment(id) ON DELETE CASCADE,
	8	pull_number INTEGER NOT NULL
	9	);
	10	CREATE INDEX note_shipped_pull ON note_shipped (pull_number);
	11	`;
		@@ -0,0 +1,113 @@
	1	/**
	2	* A note's fix has landed (#10564): the pull that closes a `from-app` issue
	3	* leaves one comment on it, in plain words, written from the pull's own
	4	* description. The forge button shows it with the note, which reads shipped
	5	* once the issue is closed (`actions/app_notes.ts`). Pure text plus one
	6	* idempotent write; both ways a merge closes an issue (`mergePull`, the box
	7	* pass) call `noteShipped`. The comment is found again by the `note_shipped`
	8	* row the forge stores with it, never by its words, and when the release is
	9	* cut `noteReleased` rewrites it to name that release.
	10	*/
	11	import type { Database } from "bun:sqlite";
	12	import { addComment, type Comment, type Issue } from "../core/issues.ts";
	13	import { fromAppIssues } from "../core/from_app.ts";
	14	import type { Pull } from "../core/pulls.ts";
	15	import { getPull } from "../core/pulls.ts";
	16	import { releaseLabel, releaseOfPull, type Release } from "../release/record.ts";
	17	import type { App } from "../app.ts";
	18	import type { Repo } from "../core/repos.ts";
	19
	20	const MAX = 600;
	21	/** Starts every such comment; only for reading, never for finding one again. */
	22	const LEAD = "Good news: this is fixed";
	23
	24	/** Lines of a description that say nothing to a sender: closings, code, headings' marks, links, attribution. */
	25	function plain(text: string): string {
	26	return text
		junipercrew 1d ago outdated This hex strip removes real words and long numbers. Anchor it to a context such as 'commit <sha>' or require at least one digit.
	27	.replace(/```[\s\S]*?```/g, " ")
	28	.replace(/^\s(close[sd]?\|fix(e[sd])?\|resolve[sd]?)\b[^\n]#\d[^\n]*$/gim, " ")
	29	.replace(/^[^\n\w]generated (with\|by)\b[^\n]$/gim, " ")
	30	.replace(/^\sco-authored-by:[^\n]$/gim, " ")
	31	.replace(/!?\[([^\]])\]\([^)]\)/g, "$1")
	32	.replace(/https?:\/\/\S+/g, "")
	33	.replace(/`([^`]*)`/g, "$1")
	34	.replace(/^\s{0,3}(#{1,6}\|[-*>]\|\d+\.)\s+/gm, "")
	35	.replace(/[*_~]{1,3}/g, "")
	36	.replace(/#\d+/g, "")
	37	// A hash is named as one ("commit abc1234") or has digits and letters both; a word or a number is not.
	38	.replace(/\b(commit\|sha\|revision)\s+[0-9a-f]{7,40}\b/gi, "")
	39	.replace(/\b(?=[0-9a-f]\d)(?=[0-9a-f][a-f])[0-9a-f]{7,40}\b/gi, "")
	40	.replace(/\(\s[,;]?\s\)/g, "")
	41	.replace(/\s+([.,;)])/g, "$1")
	42	.replace(/[ \t]+/g, " ");
	43	}
	44
	45	/** What was wrong and what changed, from a pull's title and description: no hashes, no markup, short. */
	46	export function plainSummary(title: string, body: string): string {
	47	const paras = plain(body)
	48	.split(/\n\s*\n/)
	49	.map((p) => p.replace(/\s\n\s/g, " ").trim())
	50	.filter((p) => p !== "");
	51	// The description's opening carries what changed; the rest is process.
	52	let text = paras.slice(0, 2).join("\n\n");
	53	if (text === "") text = plain(title).replace(/\s+/g, " ").trim();
	54	if (text.length > MAX) text = `${text.slice(0, MAX - 3).replace(/\s+\S*$/, "")}...`;
	55	return text;
		junipercrew 1d ago outdated Dedup on body text is fragile. A person's comment with this opening suppresses the real one. Use a marker the forge writes, such as the comment kind.
	56	}
	57
	58	/** The comment's words. */
	59	export function shippedWords(pull: Pull, release: string \| undefined): string {
	60	const where = release === undefined ? "It ships in the next release" : `It shipped in ${release}`;
	61	return `${LEAD}. ${where}.\n\n${plainSummary(pull.title, pull.body)}`;
	62	}
	63
	64	/** The comment the forge wrote for this issue, if it has. */
		junipercrew 1d ago outdated `releaseName` gives a 12-char commit hash, and at merge time `release` is nearly always undefined. The issue wants the release named in plain words. Update the comment when the release is cut, with a readable label.
	65	function told(db: Database, issueId: string): { commentId: string; pullNumber: number } \| undefined {
	66	const row = db
	67	.query<{ comment_id: string; pull_number: number }, [string]>("SELECT comment_id, pull_number FROM note_shipped WHERE issue_id = ?1")
	68	.get(issueId);
	69	return row === null ? undefined : { commentId: row.comment_id, pullNumber: row.pull_number };
	70	}
	71
	72	const wordsFor = (db: Database, pull: Pull, release: Release \| undefined): string =>
	73	shippedWords(pull, release === undefined ? undefined : releaseLabel(release));
	74
	75	/**
	76	* Say a landed fix on `issue` once. Nothing when the issue is not a note from
	77	* an app, or the forge already wrote the comment. Returns the new comment.
	78	*/
	79	export function tellNoteShipped(db: Database, by: string, issue: Issue, pull: Pull): Comment \| undefined {
	80	if (!fromAppIssues(db, [issue.id]).has(issue.id)) return undefined;
	81	if (told(db, issue.id) !== undefined) return undefined;
	82	const made = addComment(db, { issueId: issue.id, authorId: by, body: wordsFor(db, pull, releaseOfPull(db, pull.repoId, pull.number)) });
	83	if (!made.ok) return undefined;
	84	db.query("INSERT OR IGNORE INTO note_shipped (issue_id, comment_id, pull_number) VALUES (?1, ?2, ?3)").run(issue.id, made.value.id, pull.number);
	85	return made.value;
	86	}
	87
	88	/** The one way a merge tells a note's sender: write the comment and announce it. */
	89	export function noteShipped(app: App, by: string, repo: Repo, issue: Issue, pull: Pull): void {
	90	const said = tellNoteShipped(app.db, by, issue, getPull(app.db, repo.id, pull.number) ?? pull);
	91	if (said === undefined) return;
	92	app.bus.append({
	93	kind: "issue.commented",
	94	repoId: repo.id,
	95	actorId: by,
	96	payload: { number: issue.number, body: said.body, commentId: said.id },
	97	});
	98	}
	99
	100	/** A release carrying `pull` was cut: its notes' comments now name it. Returns how many were rewritten. */
	101	export function noteReleased(db: Database, release: Release): number {
	102	if (!release.ok \|\| release.pullNumber === null) return 0;
	103	const pull = getPull(db, release.repoId, release.pullNumber);
	104	if (pull === undefined) return 0;
	105	const rows = db
	106	.query<{ comment_id: string }, [number, string]>(
	107	`SELECT s.comment_id FROM note_shipped s JOIN issue i ON i.id = s.issue_id WHERE s.pull_number = ?1 AND i.repo_id = ?2`,
	108	)
	109	.all(release.pullNumber, release.repoId);
	110	const body = wordsFor(db, pull, release);
	111	for (const r of rows) db.query("UPDATE comment SET body = ?1 WHERE id = ?2").run(body, r.comment_id);
	112	return rows.length;
	113	}
		@@ -82,3 +82,10 @@ export function releaseOfPull(db: Database, repoId: string, pullNumber: number):
82	82
83	83	/** What a release is called where people read it: the landed commit, short. */
84	84	export const releaseName = (r: Release): string => r.sha.slice(0, 12);
	85
	86	/** What a release is called in plain words: its number and the day it was cut. */
	87	export const releaseLabel = (r: Release): string => {
	88	const day = new Date(r.createdAt);
	89	const when = Number.isNaN(day.getTime()) ? r.createdAt.slice(0, 10) : day.toLocaleDateString("en-GB", { day: "numeric", month: "short", year: "numeric", timeZone: "UTC" });
	90	return `release ${r.id} (${when})`;
	91	};
		@@ -0,0 +1,174 @@
	1	/** A merged pull that closes a note tells its sender, once, in plain words (#10564). */
	2	import { afterAll, beforeAll, describe, expect, test } from "bun:test";
	3	import { mkdtemp, rm } from "node:fs/promises";
	4	import { tmpdir } from "node:os";
	5	import { join } from "node:path";
	6	import { newApp } from "../src/actions/apps.ts";
	7	import { notesFor, sendNote, type Sender } from "../src/actions/app_notes.ts";
	8	import { mergePull, openPull } from "../src/actions/pulls.ts";
	9	import { commitToBranch } from "../src/actions/repos.ts";
	10	import { createApp, type App } from "../src/app.ts";
	11	import { getAppBySlug } from "../src/core/apps.ts";
	12	import { getIssue, listComments, addComment, setIssueState } from "../src/core/issues.ts";
	13	import { getPull } from "../src/core/pulls.ts";
	14	import { getRepo } from "../src/core/repos.ts";
	15	import { createUser, type User } from "../src/core/users.ts";
	16	import { applyBoxPass } from "../src/actions/box_pass.ts";
	17	import { recordRelease } from "../src/release/record.ts";
	18	import { noteReleased, plainSummary, shippedWords, tellNoteShipped } from "../src/crew/note_closed.ts";
	19	import { quietLogs } from "./helpers/globals.ts";
	20
	21	quietLogs();
	22
	23	describe("plainSummary", () => {
	24	test("drops hashes, links, code, closings and markup", () => {
	25	const out = plainSummary(
	26	"t",
	27	"## What\n\nThe button was hidden on phones (see https://x.test/a, `src/a.ts`, 0123abcd45ef).\n\nCloses #12\n\n```\ncode\n```\n\n- tests",
	28	);
	29	expect(out).not.toMatch(/https?:\|`\|\*\|0123abcd\|#12\|code\n/);
	30	expect(out).toContain("The button was hidden on phones");
	31	});
	32	test("keeps the attribution line out of a short description", () => {
	33	const out = plainSummary("t", "Fixed the tiny button.\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)\n\nCo-Authored-By: Claude <[email protected]>");
	34	expect(out).toBe("Fixed the tiny button.");
	35	});
	36	test("strips hashes but not words or numbers", () => {
	37	const out = plainSummary("t", "The defaced banner showed 1234567 visitors, was in commit 0123abcd45ef (also 9f8e7d6c5b4a).");
	38	expect(out).toContain("defaced");
	39	expect(out).toContain("1234567");
	40	expect(out).not.toMatch(/0123abcd\|9f8e7d6c\|\(\)\|commit/);
	41	});
	42	test("falls back to the title and stays short", () => {
	43	expect(plainSummary("Make it blue", "")).toBe("Make it blue");
	44	expect(plainSummary("t", "word ".repeat(400)).length).toBeLessThanOrEqual(600);
	45	});
	46	});
	47
	48	describe("a closing pull tells the note's sender", () => {
	49	let dataDir: string;
	50	let app: App;
	51	let alice: User;
	52
	53	beforeAll(async () => {
	54	dataDir = await mkdtemp(join(tmpdir(), "forge-note-closed-"));
	55	app = createApp({ dataDir, port: 0, host: "forge.test" });
	56	const made = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", adminIfFirst: true });
	57	if (!made.ok) throw new Error(String(made.error));
	58	alice = made.value;
	59	const a = await newApp(app, alice, { name: "sandbox-game", description: "Words", visibility: "private" });
	60	if (!a.ok) throw new Error(String(a.error));
	61	});
	62	afterAll(async () => {
	63	app.close();
	64	await rm(dataDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 });
	65	});
	66
	67	test("merging posts one comment, a second delivery posts none, and the status reads shipped", async () => {
	68	const record = getAppBySlug(app.db, "sandbox-game")!;
	69	const repo = getRepo(app.db, "alice", "sandbox-game")!;
	70	const who: Sender = { key: "v:closer1", name: "visitor", person: false };
	71	const sent = await sendNote(app, record, repo, "production", undefined, who, { text: "the button is tiny", page: "", viewport: "390x844" }, "10.2.0.1");
	72	if (!sent.ok) throw new Error(sent.error.message);
	73	const n = sent.value.number;
	74	expect(sent.value.status.state).not.toBe("shipped");
	75
	76	const body = `The button was too small to tap on phones. It is now 48px tall (commit 0123abcd45ef).\n\nCloses #${n}`;
	77	const seeded = await commitToBranch(app, alice, repo, { message: "seed", files: [{ path: "README.md", content: "hi\n" }] });
	78	expect(seeded.ok).toBe(true);
	79	const branched = await commitToBranch(app, alice, repo, { branch: "fix", base: "main", message: "fix", files: [{ path: "a.txt", content: "x\n" }] });
	80	expect(branched.ok).toBe(true);
	81	const opened = await openPull(app, alice, repo, { title: "Bigger button", body, baseBranch: "main", headBranch: "fix" });
	82	if (!opened.ok) throw new Error(JSON.stringify(opened.error));
	83	const merged = await mergePull(app, alice, repo, opened.value, "squash");
	84	expect(merged.ok).toBe(true);
	85
	86	const issue = getIssue(app.db, repo.id, n)!;
	87	expect(issue.state).toBe("closed");
	88	const said = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
	89	expect(said()).toHaveLength(1);
	90	expect(said()[0]!.body).toContain("too small to tap on phones");
	91	expect(said()[0]!.body).not.toMatch(/0123abcd\|#\d/);
	92	expect(said()[0]!.body).toContain("next release");
	93
	94	// Re-delivery: the same landing again says nothing more.
	95	expect(tellNoteShipped(app.db, alice.id, issue, opened.value)).toBeUndefined();
	96	expect(said()).toHaveLength(1);
	97
	98	const note = notesFor(app, record, repo, undefined, who)[0]!;
	99	expect(note.status.state).toBe("shipped");
	100	expect(note.replies.map((r) => r.text)).toEqual([said()[0]!.body]);
	101	});
	102
	103	test("an issue that is not a note hears nothing", async () => {
	104	const repo = getRepo(app.db, "alice", "sandbox-game")!;
	105	const { openIssueThrough } = await import("../src/actions/issues.ts");
	106	const plainIssue = await openIssueThrough(app, alice, repo, { title: "Plain", body: "x" });
	107	if (!plainIssue.ok) throw new Error(String(plainIssue.error));
	108	const pull = { number: 1, repoId: repo.id, title: "t", body: "" } as never;
	109	expect(tellNoteShipped(app.db, alice.id, plainIssue.value, pull)).toBeUndefined();
	110	expect(shippedWords(pull, "abc")).toContain("shipped in abc");
	111	});
	112
	113	async function land(text: string, title: string, bodyOf: (n: number) => string, branch: string, merge = true) {
	114	const record = getAppBySlug(app.db, "sandbox-game")!;
	115	const repo = getRepo(app.db, "alice", "sandbox-game")!;
	116	const who: Sender = { key: `v:${branch}`, name: "visitor", person: false };
	117	const sent = await sendNote(app, record, repo, "production", undefined, who, { text, page: "", viewport: "390x844" }, `10.2.0.${branch.length}`);
	118	if (!sent.ok) throw new Error(sent.error.message);
	119	const n = sent.value.number;
	120	const b = await commitToBranch(app, alice, repo, { branch, base: "main", message: branch, files: [{ path: `${branch}.txt`, content: "x\n" }] });
	121	expect(b.ok).toBe(true);
	122	const opened = await openPull(app, alice, repo, { title, body: bodyOf(n), baseBranch: "main", headBranch: branch });
	123	if (!opened.ok) throw new Error(JSON.stringify(opened.error));
	124	if (merge) expect((await mergePull(app, alice, repo, opened.value, "squash")).ok).toBe(true);
	125	return { n, repo, pull: opened.value, record, who };
	126	}
	127	const marked = (issueId: string) => app.db.query("SELECT comment_id FROM note_shipped WHERE issue_id = ?1").all(issueId);
	128
	129	test("a person's comment that opens the same way does not stand in for the forge's", async () => {
	130	const { n, repo } = await land("the logo is blurry", "Sharper logo", (k) => `The logo was blurry. It is sharp now.\n\nCloses #${k}`, "logo", false);
	131	const issue = getIssue(app.db, repo.id, n)!;
	132	addComment(app.db, { issueId: issue.id, authorId: alice.id, body: "Good news: this is fixed, says a person" });
	133	const pull = getPullFor(repo.id, "logo");
	134	expect(tellNoteShipped(app.db, alice.id, issue, pull)).toBeDefined();
	135	expect(marked(issue.id)).toHaveLength(1);
	136	});
	137
	138	test("the comment names the release once one is cut, in words, not a hash", async () => {
	139	const { n, repo, pull } = await land("the footer overlaps", "Fix footer", (k) => `The footer overlapped the text. It no longer does.\n\nCloses #${k}`, "footer");
	140	const issue = getIssue(app.db, repo.id, n)!;
	141	const text = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
	142	expect(text()[0]!.body).toContain("next release");
	143	expect(noteReleased(app.db, recordRelease(app.db, { sha: "f00dfeedcafe1234", repoId: repo.id, pullNumber: pull.number, ok: false, line: "bad" }))).toBe(0);
	144	const rel = recordRelease(app.db, { sha: "f00dfeedcafe1234", repoId: repo.id, pullNumber: pull.number, ok: true, line: "ok" });
	145	expect(noteReleased(app.db, rel)).toBe(1);
	146	expect(text()).toHaveLength(1);
	147	expect(text()[0]!.body).toContain(`release ${rel.id} (`);
	148	expect(text()[0]!.body).not.toMatch(/next release\|f00dfeed/);
	149	expect(text()[0]!.body).toContain("footer overlapped");
	150	});
	151
	152	test("the box pass's closing path tells the sender too, once", async () => {
	153	const { n, repo, pull } = await land("the menu is cut off", "Menu fits", (k) => `The menu was cut off. It fits now.\n\nCloses #${k}`, "menu");
	154	const issue = getIssue(app.db, repo.id, n)!;
	155	// As if the pull had landed before its boxes were read: the issue is open and unannounced.
	156	app.db.query("DELETE FROM comment WHERE id = (SELECT comment_id FROM note_shipped WHERE issue_id = ?1)").run(issue.id);
	157	expect(marked(issue.id)).toHaveLength(0);
	158	setIssueState(app.db, issue.id, "open");
	159	const merged = getPullFor(repo.id, "menu");
	160	const pass = { ticks: [{ issue: n, box: 1, text: "x" }] };
	161	applyBoxPass(app, repo, merged, alice, pass);
	162	expect(getIssue(app.db, repo.id, n)!.state).toBe("closed");
	163	const said = () => listComments(app.db, issue.id, { kind: "comment" }).filter((c) => c.body.startsWith("Good news"));
	164	expect(said()).toHaveLength(1);
	165	expect(said()[0]!.body).toContain("menu was cut off");
	166	expect(pull.number).toBe(merged.number);
	167	});
	168
	169	function getPullFor(repoId: string, head: string) {
	170	const row = app.db.query<{ number: number }, [string, string]>("SELECT issue.number FROM pull JOIN issue ON issue.id = pull.issue_id WHERE issue.repo_id = ?1 AND pull.head_branch = ?2").get(...([repoId, head] as [string, string]));
	171	if (row === null) throw new Error("no pull");
	172	return getPull(app.db, repoId, row.number)!;
	173	}
	174	});