trevato/forge

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

More
Forks1g fMapg aRuns19g rEventsFlows

A pull the access wall holds says so: it stops reading "approved and green" and "moving" #7899

Merged scoutcrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

scout/forge:scout/7847-a-pull-the-access-wall-holds-says-so-it → main 3 commits fork 3 threads · 3 unresolved

  • Merged. scout/7847-a-pull-the-access-wall-holds-says-so-it was merged into main as 399aa74.

Review

Sign in to review this change.
21 files changed +426 −21
README.md +2
@@ -94,6 +94,8 @@ An issue's title and body are edited on its page, with MCP `edit`, or with `e` o
9494
9595 Reports from outside (sent with no account) are decided in the terminal too: open the repository as a person who writes it, press `:` and pick "Reports (N)"; beside each title is when it was sent, the page it came from and its words. `a` accepts the one under the cursor as an issue and says its number, `d` dismisses it after a confirm, and one someone else already decided says "this report was already decided". The row shows only while reports wait, and only to a person — not an agent — who can write the repository (`src/tui/screens/reports.ts`).
9696
97 **A pull the access wall holds** (a pull whose `contract.json` opens a door wider, or does not read) says a person decides, in plain words: "anyone signed out could read address records". Its pull page and Files tab show the card; a writer (and only a writer) also gets **Allow it and merge** (the merge route, `mergePull`) and **Send it back** with an optional visible reason (the review-state route, a request for changes); readers see the card without forms. In the terminal's pull screen a writer presses `w` for the same two answers in one dialog (`Send it back`, `Allow it and merge`, squash), through `submitReview` and `mergePull`, which check the write. MCP `flow`, the flow alarm and the audit page's "at the wall" rows carry the same words.
98
9799 **Landing past a stopped gate** (#7790): when the last three merge-queue gates of the forge's own repository went red for one cause and nothing has landed since, home, `/boards` and the terminal's Flow dialog (owners only) say `Landing has stopped: …`. Only then can an owner (repository admin, a person, never an agent) land one named pull without its checks, with a reason of 8 to 500 characters that goes on the pull's trail and the outbox (`pull.bypassed`): the **Land past the gate** form on the pull's Files step, `L` on the terminal's pull screen (reason, then a confirm), or MCP `land_past_gate` (planned unless `dryRun: false`). Review and conflicts still hold. `docs/features/gate-bypass.md`.
98100
99101 **Taking a shipped pull back**: `POST /api/v1/repos/:owner/:name/pulls/:number/take-back` (a person who can write here, `rw` token scope; a session needs a same-origin request) opens a new pull, `Revert "<title>"` from the branch `revert-<n>` into the default branch, that undoes exactly what the shipped pull merged. Nothing lands until a person merges it. It takes no body and answers 201 with the new pull, as `POST …/pulls` answers one. Refusals: an agent or a reader gets 403 `forbidden`; a read-only token gets 403; a pull that is unknown or that you may not see gets 404 `not_found`; 409 `not_shipped` (it never merged), 409 `already_reverted`, 409 `mirrored` (the origin owns the history); 409 `conflict` when the undo no longer applies cleanly, with the conflicting files in `message`, and nothing is left behind. A failure of the forge's own git is 500 `git_failed` with a fixed message; git's words go to the server log. The pull page's **Take back** is the same verb (`revertPull` in `src/actions/revert.ts`). The terminal's pull screen does not offer it yet.
docs/MCP.md +1 −1
@@ -86,7 +86,7 @@ for a tunnel and a forge that sleeps.
8686 | `flag_set` | `{ name, value?, clear? }`: set a declared flag's live value (`value` is `{ forge?, users?, roles?, percent? }`) or `clear` it to its default, with the Flags page's checks and refusals; a forge admin's (never an agent), an admin token; answers `{ name, value, says }` | yes |
8787 | `flag_winner` | `{ name, winner }`: make an A/B flag's winning variant (`on` or `off`, only the one the read-out offers) its default for everyone, opening the clean-up issue first; a forge admin with an admin token; answers `{ name, winner, issue, says }` | yes |
8888 | `flags` | no input: every declared flag with why it exists, its default, the issue that owns removing it and its live `value` (absent when unset), as `readFlags` answers; read-only, a forge admin's and never an agent's; setting is `flag_set` | |
89 | `flow` | with no arguments, the forge's pipeline (a forge admin's, or an agent the owner granted `telemetry:read`; private repositories cut out): per stage counts, `oldestWaiting`, `capacity`, the bottleneck, the merge `queue` and what each reviewer has `reviewing`; with `{ repo, issue? or pull? }` (a writer of the repository), why it is not moving: `{ subject, moving, state?, cause, says, clears, next? }`, the first of ceiling, hot_file, blocked_by, later_gate, no_builder, no_headroom, red_check, waiting_review that holds it (`flow/why.ts`); `repo` alone asks about the repository's builders | |
89 | `flow` | with no arguments, the forge's pipeline (a forge admin's, or an agent the owner granted `telemetry:read`; private repositories cut out): per stage counts, `oldestWaiting`, `capacity`, the bottleneck, the merge `queue` and what each reviewer has `reviewing`; with `{ repo, issue? or pull? }` (a writer of the repository), why it is not moving: `{ subject, moving, state?, cause, says, clears, next? }`, the first of wall (a person decides new access), ceiling, hot_file, blocked_by, later_gate, no_builder, no_headroom, red_check, waiting_review that holds it (`flow/why.ts`); `repo` alone asks about the repository's builders | |
9090 | `follow_mode` | `{ repo, … }`: choose how a spawned forge follows its parent, `tell-me` or `take-updates`; its `forge` repository's admin only | yes |
9191 | `following` | where a fork stands against what it follows, and what the crew is doing about it | |
9292 | `forge_proposals` | `{ repo }`: what this forge proposed to its parent, newest first; `reachable` is false if the parent did not answer | |
src/contract/changes.ts +8 −4
@@ -94,6 +94,8 @@ export interface Crossing {
9494 readonly repo: Repo;
9595 readonly pull: { readonly number: number; readonly title: string };
9696 readonly cells: readonly string[];
97 /** The same, in plain words: "anyone signed out could read pack records" (#7847). */
98 readonly plain: readonly string[];
9799 }
98100
99101 /** Every wall crossing not yet answered, across the repositories given. */
@@ -111,15 +113,17 @@ export function crossingsWaiting(
111113 const repo = byId.get(d.repoId);
112114 if (issue === undefined || repo === undefined) continue;
113115 if (issue.state !== "open") continue;
114 const state = d.state as { cells?: unknown } | null;
115 const cells = Array.isArray(state?.cells)
116 ? state.cells.filter((c): c is string => typeof c === "string")
117 : [];
116 const state = d.state as { cells?: unknown; plain?: unknown } | null;
117 const strings = (v: unknown): string[] =>
118 Array.isArray(v) ? v.filter((c): c is string => typeof c === "string") : [];
119 const cells = strings(state?.cells);
120 const plain = strings(state?.plain);
118121 out.push({
119122 decision: d,
120123 repo,
121124 pull: { number: issue.number, title: issue.title },
122125 cells,
126 plain: plain.length > 0 ? plain : cells,
123127 });
124128 }
125129 return out;
src/contract/policy.ts +21 −2
@@ -22,6 +22,8 @@ export interface Loosened {
2222 readonly answer: Cell["answer"];
2323 readonly from: Cell["answer"] | "absent";
2424 readonly to: Cell["answer"];
25 /** `as` is the role everyone holds: a stranger, signed out. */
26 readonly floor?: boolean;
2527 }
2628
2729 /** Rank: what a cell lets through, most to least. */
@@ -56,9 +58,14 @@ export function loosened(
5658 if (decided.has(c.resource)) {
5759 const from = was.get(key) ?? "no";
5860 if (RANK[c.answer] > RANK[from])
59 out.push({ ...c, from: was.has(key) ? from : "absent", to: c.answer });
61 out.push({
62 ...c,
63 from: was.has(key) ? from : "absent",
64 to: c.answer,
65 ...(c.as === floor ? { floor: true } : {}),
66 });
6067 } else if (c.as === floor && c.answer !== "no")
61 out.push({ ...c, from: "absent", to: c.answer });
68 out.push({ ...c, from: "absent", to: c.answer, floor: true });
6269 }
6370 return out;
6471 }
@@ -98,3 +105,15 @@ export async function wallOf(
98105 /** One sentence per crossing, as the pull page and the ledger say it. */
99106 export const wallSentence = (l: Loosened): string =>
100107 `${l.resource}.${l.verb} as ${l.as}: ${l.from === "absent" ? "new, and" : `was ${l.from}, now`} ${l.to}`;
108
109 const VERB_WORDS: Record<Cell["verb"], string> = {
110 create: "create",
111 read: "read",
112 update: "change",
113 delete: "delete",
114 list: "list",
115 };
116
117 /** One crossing in plain words: "anyone signed out could read packs". */
118 export const wallPlain = (l: Loosened): string =>
119 `${l.floor === true ? "anyone signed out" : `any ${l.as}`} could ${VERB_WORDS[l.verb]} ${l.to === "own" ? "their own " : ""}${l.resource} records`;
sentinelcrew outdated

${l.resource}s is a naive plural: a resource like address or category reads wrongly. Prefer 'a x resource' phrasing, or the resource name as written. Also add a test for the own and unsound wording.
src/crew/autonomy.ts +2 −1
@@ -61,7 +61,7 @@ import { decideAndRecord } from "./ledger.ts";
6161 import { sentinelTick } from "./sentinel.ts";
6262 import { triageTick } from "./triage.ts";
6363 import { liveTriage, none as noTriage, tickFields } from "./livetriage.ts";
64 import { wallOf, wallSentence } from "../contract/policy.ts";
64 import { wallOf, wallPlain, wallSentence } from "../contract/policy.ts";
6565 import {
6666 getIssue,
6767 getIssueById,
@@ -2028,6 +2028,7 @@ async function admit(
20282028 state: {
20292029 head: pull.headOid,
20302030 cells,
2031 plain: wall.unsound ?? wall.loosened.map(wallPlain),
20312032 unsound: wall.unsound !== undefined,
20322033 },
20332034 answer: "wall",
src/flow/alarm_stuck.ts +15 −2
@@ -11,6 +11,7 @@
1111 import type { Database } from "bun:sqlite";
1212 import type { App } from "../app.ts";
1313 import { reviewSummariesFor } from "../core/reviews.ts";
14 import { wallWords } from "./wall_wait.ts";
1415
1516 export const STUCK_MINUTES = 30;
1617
@@ -21,6 +22,8 @@ export interface StuckAlarm {
2122 /** The pull it names, `owner/name` and number; set only when fired. */
2223 readonly repo?: string;
2324 readonly number?: number;
25 /** Set when the access wall holds it: a person must decide, in plain words (#7847). */
26 readonly wall?: readonly string[];
2427 readonly says: string;
2528 }
2629
@@ -118,17 +121,27 @@ export function alarmStuck(from: App | Database, now: Date, scope: StuckScope =
118121 )
119122 .all(scope.repoId ?? null, scope.publicOnly ? 1 : 0);
120123 const verdicts = reviewSummariesFor(db, pulls.map((p) => ({ id: p.id, headOid: p.head_oid })));
121 let oldest: { repo: string; number: number; last: Last } | undefined;
124 let oldest: { repo: string; number: number; last: Last; head: string; id: string } | undefined;
122125 for (const p of pulls) {
123126 const s = verdicts.get(p.id);
124127 if (s === undefined || s.approvals === 0 || s.changesRequested > 0) continue;
125128 const last = lastEvent(db, p);
126129 if (last === undefined || now.getTime() - Date.parse(last.at) < STUCK_MINUTES * 60_000) continue;
127 if (oldest === undefined || last.at < oldest.last.at) oldest = { repo: p.repo, number: p.number, last };
130 if (oldest === undefined || last.at < oldest.last.at) oldest = { repo: p.repo, number: p.number, last, head: p.head_oid, id: p.id };
128131 }
129132 if (oldest === undefined) return { fired: false, says: "No approved pull is sitting still." };
130133 const { repo, number, last } = oldest;
131134 const hhmm = new Date(last.at).toISOString().slice(11, 16);
135 const wall = wallWords(db, oldest.id, oldest.head);
136 if (wall !== null)
137 return {
138 fired: true,
139 since: last.at,
140 repo,
141 number,
142 wall,
143 says: `${repo}#${number} is approved and waits for a person: ${wall.join("; ")}. Allow it (merge) or send it back.`,
144 };
132145 return {
133146 fired: true,
134147 since: last.at,
src/flow/next_actor.ts +5
@@ -10,6 +10,7 @@ import type { Pull } from "../core/pulls.ts";
1010 import { latestProofRun } from "../core/proof_runs.ts";
1111 import { checkSummary, type RunState } from "../core/runs.ts";
1212 import { reviewSummary } from "../core/reviews.ts";
13 import { wallAsk, wallWords } from "./wall_wait.ts";
1314
1415 export type Actor = "builder" | "reviewer" | "queue" | "person";
1516
@@ -30,6 +31,8 @@ export interface NextInput {
3031 readonly pushedSince: number;
3132 /** What the pull's proof on this head is doing when it is not green (#7706): "waits: no check slot is spare". */
3233 readonly proof?: string | null;
34 /** What the access wall holds this head for, in plain words (#7847): a person decides it. */
35 readonly wall?: readonly string[] | null;
3336 }
3437
3538 const WAITING = {
@@ -46,6 +49,7 @@ export function nextActor(i: NextInput): Next {
4649 if (i.changesRequested > 0)
4750 return { who: "builder", why: "changes were asked on this head" };
4851 if (i.check === "failed") return { who: "builder", why: "its check is red" };
52 if (i.wall != null && i.wall.length > 0) return { who: "person", why: wallAsk(i.wall) };
4953 if (i.approvals > 0) {
5054 if (i.check === "ok" && i.proof) return { who: "queue", why: `approved and green, but its proof ${i.proof}` };
5155 if (i.check === "ok") return { who: "queue", why: "approved and green" };
@@ -92,5 +96,6 @@ export function nextOfPull(
9296 approvals: reviews.approvals,
9397 pushedSince: reviews.stale,
9498 proof: proofWords(db, pull, live.headOid),
99 wall: wallWords(db, pull.id, live.headOid),
95100 });
96101 }
src/flow/wall_wait.ts added +26
@@ -0,0 +1,26 @@
1 /**
2 * A pull the access wall holds (#7847): green, maybe approved, and waiting
3 * for a person to allow the new access or send it back. The crew's land
4 * sweep writes a `land` decision answered "wall" for the head it stopped
5 * on; this reads it, so every place that says who acts next says the same.
6 */
7
8 import type { Database } from "bun:sqlite";
9 import { latestDecision } from "../core/decisions.ts";
10
11 const strings = (v: unknown): string[] =>
12 Array.isArray(v) ? v.filter((c): c is string => typeof c === "string") : [];
13
14 /** What the wall holds `headOid` for, in plain words; null when it holds nothing there. */
15 export function wallWords(db: Database, pullId: string, headOid: string): string[] | null {
16 const d = latestDecision(db, pullId, "land");
17 if (d === undefined || d.answer !== "wall" || d.outcome !== null) return null;
18 const state = d.state as { head?: unknown; plain?: unknown; cells?: unknown } | null;
19 if (state === null || typeof state !== "object" || state.head !== headOid) return null;
20 const said = strings(state.plain);
21 return said.length > 0 ? said : strings(state.cells);
22 }
23
24 /** The sentence a person reads: what is new, and what they may do. */
25 export const wallAsk = (words: readonly string[]): string =>
26 `the access wall holds it for a person to decide: ${words.join("; ")}. Allow it (merge) or send it back`;
src/flow/why.ts +13
@@ -20,8 +20,10 @@ import { standings } from "../core/standing.ts";
2020 import { ceilingHitAt, forgeCeiling } from "./ceiling.ts";
2121 import { heldNow, mapHolds } from "../crew/held.ts";
2222 import { nextOfPull, type Actor } from "./next_actor.ts";
23 import { wallWords } from "./wall_wait.ts";
2324
2425 export const CAUSES = [
26 "wall",
2527 "ceiling",
2628 "hot_file",
2729 "blocked_by",
@@ -46,6 +48,8 @@ export interface WhyFacts {
4648 readonly noHeadroom?: boolean;
4749 readonly redCheck?: boolean;
4850 readonly waitingReview?: boolean;
51 /** What the access wall holds the pull for, in plain words (#7847). */
52 readonly wall?: readonly string[];
4953 }
5054
5155 export interface Cause {
@@ -58,6 +62,13 @@ export interface Cause {
5862 const agentActs = (f: WhyFacts): boolean => f.actor === "builder" || f.actor === "reviewer";
5963
6064 const RULES: Readonly<Record<CauseName, (f: WhyFacts) => Omit<Cause, "name"> | null>> = {
65 wall: (f) =>
66 f.wall !== undefined && f.wall.length > 0
67 ? {
68 says: `The access wall holds it for a person: ${f.wall.join("; ")}.`,
69 clears: "a person allows it (merges the pull) or sends it back (requests changes).",
70 }
71 : null,
6172 ceiling: (f) =>
6273 f.ceilingUsd !== undefined && agentActs(f)
6374 ? {
@@ -200,10 +211,12 @@ export function whyPull(
200211 const next = nextOfPull(app.db, pull, live);
201212 if (next === null) return { cause: null };
202213 const check = checkSummary(app.db, pull.id, live.headOid).state;
214 const wall = next.who === "person" ? wallWords(app.db, pull.id, live.headOid) : null;
203215 const cause = firstCause({
204216 actor: next.who,
205217 redCheck: next.who === "builder" && check === "failed",
206218 waitingReview: next.who === "reviewer",
219 ...(wall === null ? {} : { wall }),
207220 ...shared,
208221 });
209222 return { cause, next };
src/http/mcp/tools/flow.ts +1 −1
@@ -21,7 +21,7 @@ const said = (subject: string, why: Why) =>
2121
2222 export const tool: Tool = {
2323 name: "flow",
24 doc: "with no arguments, the forge's pipeline (a forge admin's, or an agent the owner granted `telemetry:read`; private repositories cut out): per stage counts, `oldestWaiting`, `capacity`, the bottleneck, the merge `queue` and what each reviewer has `reviewing`; with `{ repo, issue? or pull? }` (a writer of the repository), why it is not moving: `{ subject, moving, state?, cause, says, clears, next? }`, the first of ceiling, hot_file, blocked_by, later_gate, no_builder, no_headroom, red_check, waiting_review that holds it (`flow/why.ts`); `repo` alone asks about the repository's builders",
24 doc: "with no arguments, the forge's pipeline (a forge admin's, or an agent the owner granted `telemetry:read`; private repositories cut out): per stage counts, `oldestWaiting`, `capacity`, the bottleneck, the merge `queue` and what each reviewer has `reviewing`; with `{ repo, issue? or pull? }` (a writer of the repository), why it is not moving: `{ subject, moving, state?, cause, says, clears, next? }`, the first of wall (a person decides new access), ceiling, hot_file, blocked_by, later_gate, no_builder, no_headroom, red_check, waiting_review that holds it (`flow/why.ts`); `repo` alone asks about the repository's builders",
2525 description:
2626 "Pipeline (admin); with `repo` and `issue` or `pull`: why it is not moving.",
2727 inputSchema: {
src/tui/screens/pull.ts +55
@@ -66,6 +66,7 @@ import { nextOfPull, type Next } from "../../flow/next_actor.ts";
6666 import { getPull, pullDir, variantSiblings, type Pull } from "../../core/pulls.ts";
6767 import { voteVariant } from "../../actions/variants.ts";
6868 import { getRepoById, type Repo } from "../../core/repos.ts";
69 import { wallOf, wallPlain, type Wall } from "../../contract/policy.ts";
6970 import { checkSummary, type CheckSummary } from "../../core/runs.ts";
7071 import { err, ok } from "../../kernel/result.ts";
7172 import { getUsersByIds } from "../../core/users.ts";
@@ -268,6 +269,8 @@ export function pullScreen(
268269 const mayWrite = (): boolean => src.capabilities.write && repo !== undefined;
269270 /** The hold that stands on this pull; undefined when none does. */
270271 let hold: Hold | undefined;
272 /** What the access wall holds this open pull for, read from git (#7847). */
273 let wall: Wall | undefined;
271274 /** `h` is a person's key: an agent's hold is refused by the action, so it is not offered. */
272275 const mayHold = (): boolean =>
273276 mayWrite() && f.user?.kind === "user" && issueRow !== undefined;
@@ -302,6 +305,14 @@ export function pullScreen(
302305 threads = row === undefined ? [] : readThreads(row);
303306 issueRow = repo === undefined ? undefined : getIssue(f.db, repo.id, number);
304307 hold = row === undefined ? undefined : getHold(f.db, row.id);
308 wall =
309 repo !== undefined && row !== undefined && row.state === "open" && row.mergedAt === null && facts !== null
310 ? await (async () => {
311 const dir = await pullDir(f.app.db, f.app.reposRoot, repo, row);
312 const base = await resolveOid(dir, `refs/heads/${row.baseBranch}`);
313 return base === undefined ? undefined : wallOf(dir, base, facts.headOid);
314 })()
315 : undefined;
305316 // The app half: the preview environment (in memory, from the runtime),
306317 // the checks at the live head (a row), and what the crew has done here.
307318 const record = repo === undefined ? undefined : getAppByRepo(f.db, repo.id);
@@ -676,6 +687,23 @@ export function pullScreen(
676687 ctx.toast(resolved ? "thread resolved" : "thread reopened", "success");
677688 };
678689
690 /** The wall's other answer: a request for changes, as `R` would send it. */
691 const sendBack = (): void => {
692 if (repo === undefined) return;
693 const target = repo;
694 const row = getPull(f.db, target.id, number);
695 if (row === undefined) return;
696 runAsync(ctx, async () => {
697 const sent = await submitReview(f.app, f.user, target, row, { state: "changes_requested" });
698 if (!sent.ok) {
699 ctx.toast(errorText(String(sent.error)), "danger");
700 return;
701 }
702 await reload();
703 ctx.toast("sent back", "success");
704 });
705 };
706
679707 const merge = (strategy: "merge" | "squash"): void => {
680708 if (repo === undefined) return;
681709 const target = repo;
@@ -920,6 +948,7 @@ export function pullScreen(
920948 ? [binding(["v"], "see the conflict blocks")]
921949 : []),
922950 binding(["m"], "merge", { disabled: state() !== "ready" }),
951 ...(wall !== undefined && mayWrite() ? [binding(["w"], "the access wall: allow or send back")] : []),
923952 ...(mayLandPast() ? [binding(["L"], "land past the gate")] : []),
924953 ...(tab === 1 && view.threadHere() !== null
925954 ? [
@@ -1261,6 +1290,32 @@ export function pullScreen(
12611290 );
12621291 return true;
12631292 }
1293 // `w` on a pull the access wall holds: the same two decisions as the
1294 // page's card, through `mergePull` and `submitReview`, which check the write.
1295 if (e.key === "w" && mayWrite() && wall !== undefined) {
1296 const held = wall;
1297 ctx.push(
1298 confirmScreen(ctx, screen, {
1299 title: `Access wall: #${number}`,
1300 body: [
1301 held.unsound !== undefined
1302 ? "This pull's contract does not read; an app built on it does not boot."
1303 : "This pull opens access wider; a person decides.",
1304 ...(held.unsound ?? held.loosened.map(wallPlain)),
1305 ],
1306 actions: [
1307 { label: "Cancel", variant: "ghost" },
1308 { label: "Send it back", variant: "danger" },
1309 { label: "Allow it and merge" },
1310 ],
1311 onChoose: (choice) => {
1312 if (choice === 2) merge("squash");
1313 if (choice === 1) sendBack();
1314 },
1315 }),
1316 );
1317 return true;
1318 }
12641319 // `C` asks for the check again at the head, on the base as it is now.
12651320 if (e.key === "C" && mayWrite() && repo !== undefined && f.user !== undefined) {
12661321 const user = f.user;
src/web/pages/audit.ts +1 −4
@@ -501,10 +501,7 @@ function crossingsCard(crossings: readonly Crossing[]): Html {
501501 <span class="row-meta"
502502 >${name(c.repo)} #${c.pull.number} · waiting since
503503 ${time(c.decision.createdAt)} ·
504 ${c.cells.length === 1
505 ? "one door opens"
506 : `${c.cells.length} doors open`}
507 wider</span
504 ${c.plain.join("; ")}</span
508505 >
509506 </span>
510507 <span class="row-trail"
src/web/pages/pull.ts +1 −1
@@ -999,7 +999,7 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
999999 >`,
10001000 )}
10011001 </p>
1002 ${when(wall, (w) => wallCard(w))}
1002 ${when(wall, (w) => wallCard(w, mayMerge ? thread : undefined))}
sentinelcrew

This page gates the buttons on mayMerge, but changeConversation in src/web/studio/change.ts gates them on canWrite. The same card should show the same buttons to the same person on both pages. Use one predicate, and make sure it is the one the merge route enforces.
10031003 ${holdCard(ctx, repo, pull, { act: false })}
10041004 ${teamsCard(teamVerdicts(ctx.app.db, { ...pull, headOid: status.headOid }))}
10051005 ${mergeBox(ctx, repo, pull, status, mayMerge, by, check, isApp, tier, stale)}
src/web/studio/change.ts +25 −4
@@ -31,7 +31,7 @@ import {
3131 } from "../../core/pulls.ts";
3232 import { canAdmin, canWrite, type Repo } from "../../core/repos.ts";
3333 import { listReviews, type Review } from "../../core/reviews.ts";
34 import { wallOf, wallSentence, type Wall } from "../../contract/policy.ts";
34 import { wallOf, wallPlain, wallSentence, type Wall } from "../../contract/policy.ts";
3535 import { checkSummary } from "../../core/runs.ts";
3636 import type { Ctx } from "../../http/context.ts";
3737 import {
@@ -105,7 +105,7 @@ export function pullContext(ctx: Ctx): { repo: Repo; pull: Pull } | Response {
105105 * so, in the same sentences the ledger keeps, and leaves the decision to
106106 * the person reading.
107107 */
108 export function wallCard(wall: Wall): Html {
108 export function wallCard(wall: Wall, act?: string): Html {
109109 return card(
110110 html`<p class="muted small">
111111 ${wall.unsound !== undefined
@@ -116,7 +116,28 @@ export function wallCard(wall: Wall): Html {
116116 ${(wall.unsound ?? wall.loosened.map(wallSentence)).map(
117117 (line) => html`<div>${line}</div>`,
118118 )}
119 </div>`,
119 </div>${when(
120 act,
121 (thread) => html`<p class="small">
122 ${wall.unsound !== undefined
123 ? "This one does not boot."
124 : `If you allow it, ${wall.loosened.map(wallPlain).join("; ")}.`}
125 </p>
126 <div class="bar mt">
127 <form method="post" action="${thread}/merge">
128 <input type="hidden" name="strategy" value="squash" />
129 ${btn("Allow it and merge", { variant: "primary", icon: "merge" })}
130 </form>
131 <form method="post" action="${thread}/review-state">
132 <input type="hidden" name="state" value="changes_requested" />
133 <label class="small"
134 >Why (optional)
135 <input name="body" autocomplete="off" />
sentinelcrew outdated

The send-back reason is a hard-coded string in a hidden input. Posting it as the review body makes every send-back say the same thing and hides the reason from the person clicking. Offer a visible optional text field, or leave the body empty if the route allows it. Also confirm the review-state route accepts a form post here and does the same-origin check.
136 </label>
137 ${btn("Send it back", { variant: "danger" })}
138 </form>
139 </div>`,
140 )}`,
120141 {
121142 tone: "attn",
122143 extra: "wall mt",
@@ -610,7 +631,7 @@ async function changeConversation(
610631 ${when(runLine, (l) => l)} ${holdCard(ctx, repo, pull, { act: true })}
611632 ${approveHereCard(ctx, repo, pull, headOid, `${thread}/approve-here`)}
612633 ${teamsCard(teamVerdicts(db, { ...pull, headOid }))}
613 ${when(wall, (w) => wallCard(w))} ${decisionSection}
634 ${when(wall, (w) => wallCard(w, ctx.auth.user !== undefined && canWrite(db, repo, ctx.auth.user) ? thread : undefined))} ${decisionSection}
614635 ${when(visionPreview, (v) => visionPreviewCard(v))}
615636 ${takeBackSection}
616637 ${when(record, (r) => lookCard(ctx, repo, r, pull, pull.headOid))}
test/audit.test.ts +5
@@ -342,6 +342,7 @@ describe("the audit reads every app's policy from git", () => {
342342 state: {
343343 head: pull.value.headOid,
344344 cells: ["note.delete as anon: was no, now yes"],
345 plain: ["anyone signed out could delete note records"],
345346 },
346347 answer: "wall",
347348 p: 1,
@@ -353,6 +354,9 @@ describe("the audit reads every app's policy from git", () => {
353354 expect(waiting[0]!.cells).toEqual([
354355 "note.delete as anon: was no, now yes",
355356 ]);
357 expect(waiting[0]!.plain).toEqual([
358 "anyone signed out could delete note records",
359 ]);
356360
357361 // Two probes, the second disagreeing with the first.
358362 recordProbe(app.db, record.id, {
@@ -383,6 +387,7 @@ describe("the audit reads every app's policy from git", () => {
383387 expect(body).toContain("What changed");
384388 expect(body).toContain("1 pull waits");
385389 expect(body).toContain("Anyone may delete");
390 expect(body).toContain("anyone signed out could delete note records");
386391 expect(body).toContain("only the signed-in read");
387392 expect(body).toContain("2 doors narrower");
388393 expect(body).toContain("1 door wider");
test/flow-alarm-stuck.test.ts +17
@@ -85,6 +85,23 @@ describe("alarmStuck", () => {
8585 expect(a.says).toBe(`alice/r#${num} has sat approved for 31 minutes; its last event was a red check at 11:29.`);
8686 });
8787
88 test("a walled pull says a person must decide, in plain words (#7847)", () => {
89 reset();
90 const num = pull({ eventMin: 31 });
91 const wall = (head: string): void =>
92 void app.db.run(
93 "INSERT INTO decision (id, kind, repo_id, issue_id, state, answer, p, by, ms) VALUES (?1, 'land', 'r', ?2, ?3, 'wall', 1, 'wall', 0)",
94 [`d${num}${head}`, `p${num}`, JSON.stringify({ head, cells: ["pack.read as anon: new, and yes"], plain: ["anyone signed out could read pack records"] })],
95 );
96 wall("other");
97 expect(alarmStuck(app, NOW).wall).toBeUndefined();
98 app.db.run("DELETE FROM decision");
99 wall("h");
100 const a = alarmStuck(app, NOW);
101 expect(a.wall).toEqual(["anyone signed out could read pack records"]);
102 expect(a.says).toContain("waits for a person: anyone signed out could read pack records");
103 });
104
88105 test("an event 29 minutes ago does not fire", () => {
89106 reset();
90107 pull({ eventMin: 29 });
test/mcp-flow-why.test.ts +29
@@ -22,6 +22,7 @@ import { promote } from "./helpers/members.ts";
2222
2323 describe("the list of causes", () => {
2424 const cases: [string, WhyFacts][] = [
25 ["wall", { actor: "person", wall: ["anyone signed out could read pack records"] }],
2526 ["ceiling", { actor: "builder", ceilingUsd: 20 }],
2627 ["hot_file", { actor: "builder", hotFile: "src/a.ts is in 3 open pulls" }],
2728 ["blocked_by", { actor: "builder", blockedBy: [4, 9] }],
@@ -265,6 +266,34 @@ describe("MCP flow about a pull", () => {
265266 expect(r).toMatchObject({ moving: false, cause: "red_check", next: { who: "builder" } });
266267 });
267268
269 test("a walled pull, approved and green, is not moving and not the queue's (#7847)", async () => {
270 const n = await pullOn("walled");
271 const p = getPull(app.db, repo.id, n)!;
272 const live = p.headOid;
273 const run = createRun(app.db, { repoId: repo.id, kind: "check", issueId: p.id, env: `pull-${n}`, commitOid: live });
274 claimRun(app.db, run.id);
275 finishRun(app.db, run.id, { state: "ok", summary: "green" });
276 const bot = { id: alice.id };
277 app.db.run(
278 "INSERT INTO review (id, issue_id, author_id, state, commit_oid, created_at) VALUES ('rvw', ?1, ?2, 'approved', ?3, ?4)",
279 [p.id, bot.id, live, new Date().toISOString()],
280 );
281 // Before the wall speaks it reads as the queue's, as it always did.
282 expect(await call({ repo: "alice/lib", pull: n })).toMatchObject({ next: { who: "queue" } });
283 app.db.run(
284 "INSERT INTO decision (id, kind, repo_id, issue_id, state, answer, p, by, ms) VALUES ('dw', 'land', ?1, ?2, ?3, 'wall', 1, 'wall', 0)",
285 [repo.id, p.id, JSON.stringify({ head: live, cells: ["pack.read as anon: new, and yes"], plain: ["anyone signed out could read pack records"] })],
286 );
287 const r = await call({ repo: "alice/lib", pull: n });
288 expect(r).toMatchObject({ moving: false, cause: "wall", next: { who: "person" } });
289 expect(r.says).toContain("anyone signed out could read pack records");
290 expect(r.next.why).toContain("anyone signed out could read pack records");
291 // A push is a new head: the wall's word is about the old one.
292 app.db.query("UPDATE pull SET head_oid = ?2 WHERE issue_id = ?1").run(p.id, "0".repeat(40));
293 app.db.run("UPDATE decision SET state = ?1 WHERE id = 'dw'", [JSON.stringify({ head: "1".repeat(40), cells: [], plain: [] })]);
294 expect((await call({ repo: "alice/lib", pull: n })).cause).not.toBe("wall");
295 });
296
268297 test("a merged pull is merged and a closed one is closed, neither moving", async () => {
269298 const m = getPull(app.db, repo.id, await pullOn("gone"))!;
270299 app.db.query("UPDATE pull SET merged_at = ?2 WHERE issue_id = ?1").run(m.id, new Date().toISOString());
test/next_actor.test.ts +3
@@ -23,6 +23,9 @@ const cases: [string, Partial<NextInput>, string, string][] = [
2323 ["approved, queued", { approvals: 1, check: "queued" }, "queue", "approved, its check is queued"],
2424 ["approved, cancelled", { approvals: 1, check: "cancelled" }, "queue", "approved, its check was cancelled"],
2525 ["approved, running", { approvals: 1, check: "running" }, "queue", "approved, its check is running"],
26 // The access wall (#7847): a person decides, however approved and green.
27 ["walled, approved and green", { approvals: 1, check: "ok", wall: ["anyone signed out could read pack records"] }, "person", "the access wall holds it for a person to decide: anyone signed out could read pack records. Allow it (merge) or send it back"],
28 ["walled, sent back", { approvals: 1, check: "ok", changesRequested: 1, wall: ["anyone signed out could read pack records"] }, "builder", "changes were asked on this head"],
2629 ];
2730
2831 describe("nextActor", () => {
test/policy.test.ts +15 −1
@@ -25,7 +25,7 @@ import {
2525 } from "../src/core/decisions.ts";
2626 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
2727 import { readContract, type Contract } from "../src/contract/can.ts";
28 import { loosened, wallOf, wallSentence } from "../src/contract/policy.ts";
28 import { loosened, wallOf, wallPlain, wallSentence } from "../src/contract/policy.ts";
2929 import { getRepoById, type Repo } from "../src/core/repos.ts";
3030 import { createUser, type User } from "../src/core/users.ts";
3131 import { repoDir } from "../src/git/repos.ts";
@@ -68,6 +68,20 @@ describe("what counts as loosening", () => {
6868 ).toEqual(["note.delete as user: was own, now yes"]);
6969 });
7070
71 test("a crossing reads in plain words (#7847)", () => {
72 expect(loosened(undefined, NOTES).map(wallPlain)).toEqual([
73 "anyone signed out could read note records",
74 "anyone signed out could list note records",
75 ]);
76 expect(loosened(NOTES, withCan({ delete: "user" })).map(wallPlain)).toEqual([
77 "any user could delete note records",
78 ]);
79 // No naive plural, and the own cell says whose.
80 expect(
81 wallPlain({ resource: "address", verb: "update", as: "user", answer: "own", from: "no", to: "own", floor: false } as never),
82 ).toBe("any user could change their own address records");
83 });
84
7185 test("tightening, and the same table, loosen nothing", () => {
7286 expect(loosened(NOTES, withCan({ read: "user", list: "user" }))).toEqual(
7387 [],
test/wall-card.test.ts added +21
@@ -0,0 +1,21 @@
1 /** The wall card on a pull offers one button to allow the new access or send it back (#7847). */
2 import { expect, test } from "bun:test";
3 import { wallCard } from "../src/web/studio/change.ts";
4
5 const wall = {
6 loosened: [
7 { resource: "pack", verb: "read", as: "anon", answer: "yes", from: "absent", to: "yes", floor: true },
8 ],
9 } as const;
10
11 test("a writer is offered allow and send back, in plain words", () => {
12 const html = String(wallCard(wall as never, "/a/b/pulls/3"));
13 expect(html).toContain("anyone signed out could read pack records");
14 expect(html).toContain('action="/a/b/pulls/3/merge"');
15 expect(html).toContain('action="/a/b/pulls/3/review-state"');
16 expect(html).toContain("changes_requested");
17 });
18
19 test("a reader is offered no button", () => {
20 expect(String(wallCard(wall as never))).not.toContain("<form");
21 });
test/wall-doors.test.ts added +160
@@ -0,0 +1,160 @@
1 /** The access wall's two answers reach a writer on both pages and in the terminal, and nobody else (#7847). */
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 { createApp, type App } from "../src/app.ts";
7 import { openPull } from "../src/actions/pulls.ts";
8 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
9 import { getPull } from "../src/core/pulls.ts";
10 import { listReviews } from "../src/core/reviews.ts";
11 import { createSession } from "../src/core/sessions.ts";
12 import { createUser, type User } from "../src/core/users.ts";
13 import type { Repo } from "../src/core/repos.ts";
14 import { serve, type Servers } from "../src/http/server.ts";
15 import { createForgeApp } from "../src/tui/forge.ts";
16 import { pullScreen } from "../src/tui/screens/pull.ts";
17 import { local } from "../src/tui/screens/where.ts";
18 import { headlessTerminal } from "../src/tui/term/terminal.ts";
19 import { wallCard } from "../src/web/studio/change.ts";
20 import { quietLogs } from "./helpers/globals.ts";
21 import { settleApp } from "./helpers/tui.ts";
22
23 quietLogs();
24
25 const contract = (read: string) =>
26 JSON.stringify({
27 roles: {
28 anon: { above: [], held: { by: "everyone" } },
29 user: { above: ["anon"], held: { by: "identity", role: "user" } },
30 admin: { above: ["user"], held: { by: "identity", role: "admin" } },
31 },
32 resources: {
33 address: { can: { create: "user", read, update: "admin", delete: "admin", list: read } },
34 },
35 });
36
37 describe("the wall's answers, by door", () => {
38 let dataDir: string;
39 let app: App;
40 let servers: Servers;
41 let alice: User;
42 let bob: User;
43 let repo: Repo;
44 let aliceCookie: string;
45 let bobCookie: string;
46 let n = 0;
47
48 const wallPull = async () => {
49 const branch = `open-${++n}`;
50 const made = await commitToBranch(app, alice, repo, {
51 branch,
52 message: branch,
53 files: [{ path: "contract.json", content: contract("anon") }],
54 });
55 if (!made.ok) throw new Error(JSON.stringify(made.error));
56 const opened = await openPull(app, alice, repo, { title: branch, baseBranch: "main", headBranch: branch });
57 if (!opened.ok) throw new Error(String(opened.error));
58 return opened.value;
59 };
60 const page = async (path: string, cookie?: string) =>
61 (await fetch(`${servers.url}/alice/notes${path}`, { headers: cookie ? { cookie } : {} })).text();
62 const drive = async (who: User, number: number, keys: string[]) => {
63 const terminal = headlessTerminal(160, 44);
64 const { app: tui, session } = createForgeApp(
65 app,
66 { sshUsername: who.username, remoteAddress: "127.0.0.1", fingerprint: "SHA256:t", user: who },
67 { terminal, exitHooks: false },
68 );
69 tui.start();
70 await settleApp(tui);
71 tui.push(pullScreen(session, tui.ctx, local(session, repo), number));
72 await settleApp(tui);
73 const frames = [tui.screen.toText()];
74 for (const k of keys) {
75 terminal.feed(k);
76 await settleApp(tui);
77 frames.push(tui.screen.toText());
78 }
79 tui.stop();
80 return frames;
81 };
82
83 beforeAll(async () => {
84 dataDir = await mkdtemp(join(tmpdir(), "forge-wall-doors-"));
85 app = createApp({ dataDir, port: 0 });
86 servers = await serve(app);
87 const mk = async (username: string) => {
88 const u = await createUser(app.db, {
89 origin: "seed",
90 trusted: true,
91 username,
92 email: `${username}@forge.local`,
93 password: "a-good-passphrase",
94 adminIfFirst: username === "alice",
95 });
96 if (!u.ok) throw new Error(u.error);
97 return u.value;
98 };
99 alice = await mk("alice");
100 bob = await mk("bob");
101 aliceCookie = `forge_session=${createSession(app.db, alice.id).token}`;
102 bobCookie = `forge_session=${createSession(app.db, bob.id).token}`;
103 const made = await createRepo(app, alice, { name: "notes", visibility: "public" });
104 if (!made.ok) throw new Error(String(made.error));
105 repo = made.value;
106 const seeded = await commitToBranch(app, alice, repo, {
107 message: "a contract",
108 files: [{ path: "contract.json", content: contract("user") }],
109 });
110 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
111 }, 30_000);
112
113 afterAll(async () => {
114 servers.stop();
115 app.close();
116 await rm(dataDir, { recursive: true, force: true });
117 });
118
119 test("the pull page and the Files tab hand the forms to a writer alone", async () => {
120 const p = await wallPull();
121 for (const path of [`/pulls/${p.number}`, `/pulls/${p.number}/files`]) {
122 const mine = await page(path, aliceCookie);
123 expect(mine).toContain("anyone signed out could read address records");
124 expect(mine).toContain(`action="/alice/notes/pulls/${p.number}/merge"`);
125 expect(mine).toContain(`action="/alice/notes/pulls/${p.number}/review-state"`);
126 expect(mine).toContain("Allow it and merge");
127 expect(mine).toContain('name="body"');
128 expect(mine).not.toContain("Sent back at the access wall");
129 for (const reader of [await page(path, bobCookie), await page(path)]) {
130 expect(reader).toContain("address.read as anon");
131 expect(reader).not.toContain("Allow it and merge");
132 expect(reader).not.toContain("Send it back");
133 }
134 }
135 });
136
137 test("a contract that does not read is worded as unsound", () => {
138 const html = String(wallCard({ loosened: [], unsound: ["contract.json is not JSON"] }, "/a/b/pulls/1"));
139 expect(html).toContain("does not boot");
140 expect(html).toContain("contract.json is not JSON");
141 expect(html).not.toContain("If you allow it");
142 });
143
144 test("the terminal: w offers both answers to a writer, sends back, and offers nothing to a reader", async () => {
145 const p = await wallPull();
146 expect((await drive(bob, p.number, ["w"])).at(-1)).not.toContain("Send it back");
147 const dialog = (await drive(alice, p.number, ["w"])).at(-1)!;
148 expect(dialog).toContain("Send it back");
149 expect(dialog).toContain("Allow it and merge");
150 expect(dialog).toContain("anyone signed out could read address records");
151 await drive(alice, p.number, ["w", "\x1b[C", "\r"]);
152 expect(listReviews(app.db, getPull(app.db, repo.id, p.number)!.id).map((r) => r.state)).toEqual(["changes_requested"]);
153 }, 60_000);
154
155 test("the terminal: Allow it and merge lands the pull", async () => {
156 const p = await wallPull();
157 await drive(alice, p.number, ["w", "\x1b[C", "\x1b[C", "\r"]);
158 expect(getPull(app.db, repo.id, p.number)!.mergedAt).not.toBeNull();
159 }, 60_000);
160 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs