bde5265 · 08aad75
@@ -19,6 +19,8 @@
export interface Flash {
  message: string;
  /** A one-time value rendered in a copyable monospace block. */
  code?: string;
  /** One same-origin link after the message ("see it live"). */
  link?: { href: string; label: string };
}
// Cookies cap out around 4KB; a flash is a sentence, not a payload.
@@ -63,5 +65,23 @@
export function readFlash(request: Request): Flash | undefined {
    ...(typeof value.code === "string" && value.code !== ""
      ? { code: value.code.slice(0, 512) }
      : {}),
    ...linkOf(value.link),
  };
}
/** A flash link read back: a same-origin path and a short label, or nothing. */
function linkOf(value: unknown): { link?: NonNullable<Flash["link"]> } {
  if (typeof value !== "object" || value === null) return {};
  const { href, label } = value as Record<string, unknown>;
  if (typeof href !== "string" || typeof label !== "string") return {};
  if (!isLocalPath(href) || label === "" || label.length > 80) return {};
  return { link: { href, label } };
}
/** A path on this origin: one leading slash, no scheme, no control bytes. */
export function isLocalPath(path: string): boolean {
  return (
    path.length <= 512 &&
    /^\/(?![/\\])[^\u0000-\u001f\u007f\\]*$/.test(path)
  );
}
@@ -10,6 +10,7 @@
import { BODY_MAX, readBounded, STRANGER_MAX } from "../http/limits.ts";
import { scopeAllows, type TokenScope } from "../core/tokens.ts";
import { isLocalPath } from "./flash.ts";
import { crossOrigin, type Ctx } from "../http/context.ts";
export type Form = Awaited<ReturnType<Request["formData"]>>;
@@ -58,6 +59,16 @@
export function field(form: Form, name: string): string {
  return typeof value === "string" ? value : "";
}
/**
 * Where a form asks to land back: its `back` field when that is a path on
 * this origin, otherwise `fallback`. Never a URL — a caller writes this
 * field, so anything with a scheme or host is dropped.
 */
export function backTo(form: Form, fallback: string): string {
  const back = field(form, "back");
  return isLocalPath(back) ? back : fallback;
}
/** Whether the client asked for JSON — a script keeping the page in place —
 * rather than the redirect a plain form gets. */
export function prefersJson(ctx: Ctx): boolean {
@@ -394,7 +394,10 @@
function flashToast(flash: Flash): Html {
  return html`<div class="toast" data-tone="${tone}" role="status">
    ${icon(glyph)}
    <div class="toast-body">
      ${flash.message}
      ${flash.message}${when(
        flash.link,
        (link) => html` · <a href="${link.href}">${link.label}</a>`,
      )}
      ${when(
        flash.code,
        (code) =>
@@ -49,7 +49,7 @@
import {
} from "../../core/users.ts";
import type { Ctx } from "../../http/context.ts";
import { commentCard } from "../comments.ts";
import { field, readForm } from "../form.ts";
import { backTo, field, readForm } from "../form.ts";
import { html, raw, when, type Html } from "../html.ts";
import { flashOrJson, flashTo } from "../layout.ts";
import {
@@ -724,7 +724,10 @@
export async function stateSubmit(
  if (form instanceof Response) return form;
  const wanted = field(form, "state") === "closed" ? "closed" : "open";
  const dest = `${repoBase(found.repo)}/${kind === "pull" ? "pulls" : "issues"}/${found.issue.number}`;
  const dest = backTo(
    form,
    `${repoBase(found.repo)}/${kind === "pull" ? "pulls" : "issues"}/${found.issue.number}`,
  );
  const why = field(form, "reason");
  const intoText = field(form, "into").trim();
  const into = intoText
@@ -21,7 +21,7 @@
import { canRead, getRepo, type Repo } from "../../core/repos.ts";
import { bringUpToDate } from "../../crew/autonomy.ts";
import type { Ctx } from "../../http/context.ts";
import { loadRepo, notFoundPage, repoBase, signInFirst } from "../chrome.ts";
import { field, readForm, type Form } from "../form.ts";
import { backTo, field, readForm, type Form } from "../form.ts";
import { flashOrJson, flashTo } from "../layout.ts";
import { FORGE_TARGET } from "./feed.ts";
import { changeHref } from "./nouns.ts";
@@ -133,6 +133,8 @@
export async function shipSubmit(ctx: Ctx): Promise<Response> {
    : undefined;
  if (!pull || !mayView(ctx.app.db, repo, pull, ctx.auth.user))
    return notFoundPage(ctx);
  // The inbox and the feed say where they were; the pull page does not.
  const back = backTo(form, changeHref(base, pull.number));
  const merged = await mergePull(ctx.app, ctx.auth.user, repo, pull, "squash");
  // A conflict is never the person's to fix: the pull's agent brings the
  // branch up to date, and the pull comes back green, or lands on its own.
@@ -142,7 +144,7 @@
export async function shipSubmit(ctx: Ctx): Promise<Response> {
    merged.error.kind === "conflict"
  ) {
    const who = bringUpToDate(ctx.app, repo, pull, merged.error.files);
    return flashOrJson(ctx, base, {
    return flashOrJson(ctx, back, {
      tone: "danger" as const,
      message:
        who === undefined
@@ -150,9 +152,12 @@
export async function shipSubmit(ctx: Ctx): Promise<Response> {
          : `It conflicts with main (${merged.error.files.join(", ")}); ${who} is bringing it up to date${repo.autonomy === "autonomous" ? ", then it lands on its own" : ", then it comes back to you"}.`,
    });
  }
  return flashOrJson(ctx, base, {
  return flashOrJson(ctx, back, {
    ...(merged.ok
      ? { message: `Shipped "${pull.title}". Production is redeploying.` }
      ? {
          message: `Shipped #${pull.number}`,
          link: { href: `${base}/app/production/open`, label: "see it live" },
        }
      : {
          tone: "danger" as const,
          message: `That could not ship: ${mergeWords(merged.error)}.`,
@@ -900,7 +900,7 @@
const askFormId = (d: Decision): string => `ask-${d.id}`;
 * It sits after the panel's form, because a form inside a form is not
 * HTML; the button reaches it by id.
 */
export function decisionForm(d: Decision): Html | null {
export function decisionForm(d: Decision, back?: string): Html | null {
  const pull = d.pull;
  const own = html`id="${askFormId(d)}" method="post" hidden`;
  if (d.kind === "parked")
@@ -909,13 +909,14 @@
export function decisionForm(d: Decision): Html | null {
    </form>`;
  if (pull === undefined) return null;
  const base = repoBase(d.repo);
  const where = back === undefined ? null : html`<input type="hidden" name="back" value="${back}" />`;
  if (d.kind === "stopped")
    return html`<form ${own} action="${base}/pulls/${pull.number}/state">
      <input type="hidden" name="state" value="closed" />
      <input type="hidden" name="state" value="closed" />${where}
    </form>`;
  if (d.kind === "look" || d.kind === "review")
    return html`<form ${own} action="${base}/pulls/${pull.number}/ship">
      <input type="hidden" name="pull" value="${pull.number}" />
      <input type="hidden" name="pull" value="${pull.number}" />${where}
    </form>`;
  return null;
}
@@ -420,7 +420,7 @@
export function needsYouSection(
          ${reasonInput(top?.reason)} ${btn("Preview", { size: "sm" })}
          <span class="muted small">on the selected asks</span>
        </form>`}
    ${decisions.map((d) => decisionForm(d))}
    ${decisions.map((d) => decisionForm(d, ctx.url.pathname))}
      
        junipercrew
        29d ago
        
      
      Only the pathname is sent, so a filtered inbox comes back unfiltered after Ship or Close. Send pathname plus search, still validated by isLocalPath.
    
  </section>`;
  return { body, scriptJs: SELECT_JS };
}
@@ -400,6 +400,87 @@
describe("the studio", () => {
    expect(getUserByName(app.db, "scout")).toBeDefined();
  });
  test("ship from the home returns to the home with a flash and one link; a foreign back lands on the pull page", async () => {
    const green = async (name: string) => {
      const branch = await commitToBranch(app, alice, repo, {
        branch: `feature/${name}`,
        base: "main",
        message: name,
        files: [{ path: `${name.toUpperCase()}.md`, content: `${name}\n` }],
      });
      if (!branch.ok) throw new Error(JSON.stringify(branch.error));
      const opened = await openPull(app, alice, repo, {
        title: name,
        body: "",
        baseBranch: "main",
        headBranch: `feature/${name}`,
      });
      if (!opened.ok) throw new Error(String(opened.error));
      const check = createRun(app.db, {
        repoId: repo.id,
        kind: "check",
        issueId: opened.value.id,
        env: `pull-${opened.value.number}`,
        commitOid: opened.value.headOid,
        reason: "pull",
      });
      claimRun(app.db, check.id);
      finishRun(app.db, check.id, { state: "ok", summary: "check passed in 1s" });
      return opened.value;
    };
    const home = await green("fromhome");
    const shipped = await post(
      `/alice/notes/pulls/${home.number}/ship`,
      { back: "/" },
      "; forge_mode=studio",
    );
    expect(shipped.status).toBe(303);
    expect(shipped.headers.get("location")).toBe("/");
    const flash = decodeURIComponent(
      (shipped.headers.get("set-cookie") ?? "").split("forge_flash=")[1] ?? "",
    );
    expect(flash).toContain(`Shipped #${home.number}`);
    expect(flash).toContain("see it live");
    expect(getPull(app.db, repo.id, home.number)?.mergedAt).not.toBeNull();
    const evil = await green("fromevil");
    const lost = await post(
      `/alice/notes/pulls/${evil.number}/ship`,
      { back: "https://evil.example/" },
      "; forge_mode=studio",
    );
    expect(lost.status).toBe(303);
    expect(lost.headers.get("location")).toBe(
      `/alice/notes/changes/${evil.number}`,
    );
  });
  test("close from the home returns to the home", async () => {
    const branch = await commitToBranch(app, alice, repo, {
      branch: "feature/closeme",
      base: "main",
      message: "Close me",
      files: [{ path: "CLOSEME.md", content: "x\n" }],
    });
    if (!branch.ok) throw new Error(JSON.stringify(branch.error));
    const opened = await openPull(app, alice, repo, {
      title: "Close me",
      body: "",
      baseBranch: "main",
      headBranch: "feature/closeme",
    });
    if (!opened.ok) throw new Error(String(opened.error));
    const n = opened.value.number;
    const closed = await post(
      `/alice/notes/pulls/${n}/state`,
      { state: "closed", back: "/" },
      "; forge_mode=studio",
    );
    expect(closed.status).toBe(303);
    expect(closed.headers.get("location")).toBe("/");
    expect(getPull(app.db, repo.id, n)?.state).toBe("closed");
  });
  test("a tell shipped long ago still reads shipped once it is buried under 250 later events", async () => {
    const told = openIssue(app, alice, repo, {
      title: "Buried tell",
		@@ -19,6 +19,8 @@ export interface Flash {
19	19	message: string;
20	20	/** A one-time value rendered in a copyable monospace block. */
21	21	code?: string;
	22	/** One same-origin link after the message ("see it live"). */
	23	link?: { href: string; label: string };
22	24	}
23	25
24	26	// Cookies cap out around 4KB; a flash is a sentence, not a payload.
		@@ -63,5 +65,23 @@ export function readFlash(request: Request): Flash \| undefined {
63	65	...(typeof value.code === "string" && value.code !== ""
64	66	? { code: value.code.slice(0, 512) }
65	67	: {}),
	68	...linkOf(value.link),
66	69	};
67	70	}
	71
	72	/** A flash link read back: a same-origin path and a short label, or nothing. */
	73	function linkOf(value: unknown): { link?: NonNullable<Flash["link"]> } {
	74	if (typeof value !== "object" \|\| value === null) return {};
	75	const { href, label } = value as Record<string, unknown>;
	76	if (typeof href !== "string" \|\| typeof label !== "string") return {};
	77	if (!isLocalPath(href) \|\| label === "" \|\| label.length > 80) return {};
	78	return { link: { href, label } };
	79	}
	80
	81	/** A path on this origin: one leading slash, no scheme, no control bytes. */
	82	export function isLocalPath(path: string): boolean {
	83	return (
	84	path.length <= 512 &&
	85	/^\/(?![/\\])[^\u0000-\u001f\u007f\\]*$/.test(path)
	86	);
	87	}
		@@ -10,6 +10,7 @@
10	10
11	11	import { BODY_MAX, readBounded, STRANGER_MAX } from "../http/limits.ts";
12	12	import { scopeAllows, type TokenScope } from "../core/tokens.ts";
	13	import { isLocalPath } from "./flash.ts";
13	14	import { crossOrigin, type Ctx } from "../http/context.ts";
14	15
15	16	export type Form = Awaited<ReturnType<Request["formData"]>>;
		@@ -58,6 +59,16 @@ export function field(form: Form, name: string): string {
58	59	return typeof value === "string" ? value : "";
59	60	}
60	61
	62	/**
	63	* Where a form asks to land back: its `back` field when that is a path on
	64	* this origin, otherwise `fallback`. Never a URL — a caller writes this
	65	* field, so anything with a scheme or host is dropped.
	66	*/
	67	export function backTo(form: Form, fallback: string): string {
	68	const back = field(form, "back");
	69	return isLocalPath(back) ? back : fallback;
	70	}
	71
61	72	/** Whether the client asked for JSON — a script keeping the page in place —
62	73	* rather than the redirect a plain form gets. */
63	74	export function prefersJson(ctx: Ctx): boolean {
		@@ -394,7 +394,10 @@ function flashToast(flash: Flash): Html {
394	394	return html`<div class="toast" data-tone="${tone}" role="status">
395	395	${icon(glyph)}
396	396	<div class="toast-body">
397		${flash.message}
	397	${flash.message}${when(
	398	flash.link,
	399	(link) => html` · <a href="${link.href}">${link.label}</a>`,
	400	)}
398	401	${when(
399	402	flash.code,
400	403	(code) =>
		@@ -49,7 +49,7 @@ import {
49	49	} from "../../core/users.ts";
50	50	import type { Ctx } from "../../http/context.ts";
51	51	import { commentCard } from "../comments.ts";
52		import { field, readForm } from "../form.ts";
	52	import { backTo, field, readForm } from "../form.ts";
53	53	import { html, raw, when, type Html } from "../html.ts";
54	54	import { flashOrJson, flashTo } from "../layout.ts";
55	55	import {
		@@ -724,7 +724,10 @@ export async function stateSubmit(
724	724	if (form instanceof Response) return form;
725	725
726	726	const wanted = field(form, "state") === "closed" ? "closed" : "open";
727		const dest = `${repoBase(found.repo)}/${kind === "pull" ? "pulls" : "issues"}/${found.issue.number}`;
	727	const dest = backTo(
	728	form,
	729	`${repoBase(found.repo)}/${kind === "pull" ? "pulls" : "issues"}/${found.issue.number}`,
	730	);
728	731	const why = field(form, "reason");
729	732	const intoText = field(form, "into").trim();
730	733	const into = intoText
		@@ -21,7 +21,7 @@ import { canRead, getRepo, type Repo } from "../../core/repos.ts";
21	21	import { bringUpToDate } from "../../crew/autonomy.ts";
22	22	import type { Ctx } from "../../http/context.ts";
23	23	import { loadRepo, notFoundPage, repoBase, signInFirst } from "../chrome.ts";
24		import { field, readForm, type Form } from "../form.ts";
	24	import { backTo, field, readForm, type Form } from "../form.ts";
25	25	import { flashOrJson, flashTo } from "../layout.ts";
26	26	import { FORGE_TARGET } from "./feed.ts";
27	27	import { changeHref } from "./nouns.ts";
		@@ -133,6 +133,8 @@ export async function shipSubmit(ctx: Ctx): Promise<Response> {
133	133	: undefined;
134	134	if (!pull \|\| !mayView(ctx.app.db, repo, pull, ctx.auth.user))
135	135	return notFoundPage(ctx);
	136	// The inbox and the feed say where they were; the pull page does not.
	137	const back = backTo(form, changeHref(base, pull.number));
136	138	const merged = await mergePull(ctx.app, ctx.auth.user, repo, pull, "squash");
137	139	// A conflict is never the person's to fix: the pull's agent brings the
138	140	// branch up to date, and the pull comes back green, or lands on its own.
		@@ -142,7 +144,7 @@ export async function shipSubmit(ctx: Ctx): Promise<Response> {
142	144	merged.error.kind === "conflict"
143	145	) {
144	146	const who = bringUpToDate(ctx.app, repo, pull, merged.error.files);
145		return flashOrJson(ctx, base, {
	147	return flashOrJson(ctx, back, {
146	148	tone: "danger" as const,
147	149	message:
148	150	who === undefined
		@@ -150,9 +152,12 @@ export async function shipSubmit(ctx: Ctx): Promise<Response> {
150	152	: `It conflicts with main (${merged.error.files.join(", ")}); ${who} is bringing it up to date${repo.autonomy === "autonomous" ? ", then it lands on its own" : ", then it comes back to you"}.`,
151	153	});
152	154	}
153		return flashOrJson(ctx, base, {
	155	return flashOrJson(ctx, back, {
154	156	...(merged.ok
155		? { message: `Shipped "${pull.title}". Production is redeploying.` }
	157	? {
	158	message: `Shipped #${pull.number}`,
	159	link: { href: `${base}/app/production/open`, label: "see it live" },
	160	}
156	161	: {
157	162	tone: "danger" as const,
158	163	message: `That could not ship: ${mergeWords(merged.error)}.`,
		@@ -900,7 +900,7 @@ const askFormId = (d: Decision): string => `ask-${d.id}`;
900	900	* It sits after the panel's form, because a form inside a form is not
901	901	* HTML; the button reaches it by id.
902	902	*/
903		export function decisionForm(d: Decision): Html \| null {
	903	export function decisionForm(d: Decision, back?: string): Html \| null {
904	904	const pull = d.pull;
905	905	const own = html`id="${askFormId(d)}" method="post" hidden`;
906	906	if (d.kind === "parked")
		@@ -909,13 +909,14 @@ export function decisionForm(d: Decision): Html \| null {
909	909	</form>`;
910	910	if (pull === undefined) return null;
911	911	const base = repoBase(d.repo);
	912	const where = back === undefined ? null : html`<input type="hidden" name="back" value="${back}" />`;
912	913	if (d.kind === "stopped")
913	914	return html`<form ${own} action="${base}/pulls/${pull.number}/state">
914		<input type="hidden" name="state" value="closed" />
	915	<input type="hidden" name="state" value="closed" />${where}
915	916	</form>`;
916	917	if (d.kind === "look" \|\| d.kind === "review")
917	918	return html`<form ${own} action="${base}/pulls/${pull.number}/ship">
918		<input type="hidden" name="pull" value="${pull.number}" />
	919	<input type="hidden" name="pull" value="${pull.number}" />${where}
919	920	</form>`;
920	921	return null;
921	922	}
		@@ -420,7 +420,7 @@ export function needsYouSection(
420	420	${reasonInput(top?.reason)} ${btn("Preview", { size: "sm" })}
421	421	<span class="muted small">on the selected asks</span>
422	422	</form>`}
423		${decisions.map((d) => decisionForm(d))}
	423	${decisions.map((d) => decisionForm(d, ctx.url.pathname))}
		junipercrew 29d ago Only the pathname is sent, so a filtered inbox comes back unfiltered after Ship or Close. Send pathname plus search, still validated by isLocalPath.
424	424	</section>`;
425	425	return { body, scriptJs: SELECT_JS };
426	426	}
		@@ -400,6 +400,87 @@ describe("the studio", () => {
400	400	expect(getUserByName(app.db, "scout")).toBeDefined();
401	401	});
402	402
	403	test("ship from the home returns to the home with a flash and one link; a foreign back lands on the pull page", async () => {
	404	const green = async (name: string) => {
	405	const branch = await commitToBranch(app, alice, repo, {
	406	branch: `feature/${name}`,
	407	base: "main",
	408	message: name,
	409	files: [{ path: `${name.toUpperCase()}.md`, content: `${name}\n` }],
	410	});
	411	if (!branch.ok) throw new Error(JSON.stringify(branch.error));
	412	const opened = await openPull(app, alice, repo, {
	413	title: name,
	414	body: "",
	415	baseBranch: "main",
	416	headBranch: `feature/${name}`,
	417	});
	418	if (!opened.ok) throw new Error(String(opened.error));
	419	const check = createRun(app.db, {
	420	repoId: repo.id,
	421	kind: "check",
	422	issueId: opened.value.id,
	423	env: `pull-${opened.value.number}`,
	424	commitOid: opened.value.headOid,
	425	reason: "pull",
	426	});
	427	claimRun(app.db, check.id);
	428	finishRun(app.db, check.id, { state: "ok", summary: "check passed in 1s" });
	429	return opened.value;
	430	};
	431	const home = await green("fromhome");
	432	const shipped = await post(
	433	`/alice/notes/pulls/${home.number}/ship`,
	434	{ back: "/" },
	435	"; forge_mode=studio",
	436	);
	437	expect(shipped.status).toBe(303);
	438	expect(shipped.headers.get("location")).toBe("/");
	439	const flash = decodeURIComponent(
	440	(shipped.headers.get("set-cookie") ?? "").split("forge_flash=")[1] ?? "",
	441	);
	442	expect(flash).toContain(`Shipped #${home.number}`);
	443	expect(flash).toContain("see it live");
	444	expect(getPull(app.db, repo.id, home.number)?.mergedAt).not.toBeNull();
	445
	446	const evil = await green("fromevil");
	447	const lost = await post(
	448	`/alice/notes/pulls/${evil.number}/ship`,
	449	{ back: "https://evil.example/" },
	450	"; forge_mode=studio",
	451	);
	452	expect(lost.status).toBe(303);
	453	expect(lost.headers.get("location")).toBe(
	454	`/alice/notes/changes/${evil.number}`,
	455	);
	456	});
	457
	458	test("close from the home returns to the home", async () => {
	459	const branch = await commitToBranch(app, alice, repo, {
	460	branch: "feature/closeme",
	461	base: "main",
	462	message: "Close me",
	463	files: [{ path: "CLOSEME.md", content: "x\n" }],
	464	});
	465	if (!branch.ok) throw new Error(JSON.stringify(branch.error));
	466	const opened = await openPull(app, alice, repo, {
	467	title: "Close me",
	468	body: "",
	469	baseBranch: "main",
	470	headBranch: "feature/closeme",
	471	});
	472	if (!opened.ok) throw new Error(String(opened.error));
	473	const n = opened.value.number;
	474	const closed = await post(
	475	`/alice/notes/pulls/${n}/state`,
	476	{ state: "closed", back: "/" },
	477	"; forge_mode=studio",
	478	);
	479	expect(closed.status).toBe(303);
	480	expect(closed.headers.get("location")).toBe("/");
	481	expect(getPull(app.db, repo.id, n)?.state).toBe("closed");
	482	});
	483
403	484	test("a tell shipped long ago still reads shipped once it is buried under 250 later events", async () => {
404	485	const told = openIssue(app, alice, repo, {
405	486	title: "Buried tell",