Signed-in users comment (on a review or a line of the combined diff) and approve from review pages, through the same forge shell-cmd code as ssh.
revisions2r2: behavior change in 1 of 4 commits
| r2 | cat | f00bfd0 | what changed since r1 behavior change in 1 of 4 commits | |
| r1 | cat | 48d37c3 |
No comments yet: forge comment 11 adds one.
1/4 Take comments and approvals over HTTP, as ssh makes them
The private server answers POST /<owner>/<repo>/reviews/<id>/comment and .../approve by running the forge's own command, forge shell-cmd comment|approve, as the user, with the message on stdin: what ssh's git-shell-commands run. So the comment file, the approve line, the rules (no self-approval, comments on an older revision, the 32 KiB cap), the event and the rebuild are the same, byte for byte. With FORGE_VIA=web, which the server sets, the two refusals that tell you to check out or push (a review that moved on since the page was built, a tip it never had) say to reload the page instead; over ssh they read as before. A private repo you can't read is the missing repo's 404, as for its pages. Otherwise a post takes a user: a session, from the forge's own Origin and carrying the session's CSRF token, or a Bearer token (one for a single repo works there only). Basic auth doesn't post, since browsers resend it by themselves. Fields come as a form or JSON (body, at, line, csrf); answers are JSON in the forge's own words. Each user gets 20 posts a minute and four run at once. /login/whoami tells a page's script who is signed in, with the token, and is never cached. Caddy sends these posts to the private server before it looks in www, so a public repo's review, which is static files, takes them too. The scenarios' HTTP client reads -d @file as curl does, for a body longer than Linux lets one argument be.
| M | bin/ | 26 | +++++- |
| M | lib/ | 6 | +- |
| M | lib/ | 133 | ++++++++++++++++++++++++++++++- |
| M | test/ | 9 | ++- |
| M | test/ | 16 | ++++ |
| M | test/ | 20 | ++++- |
| A | test/ | 36 | ++++++++ |
| A | test/ | 48 | +++++++++++ |
| A | test/ | 38 | +++++++++ |
| A | test/ | 48 | +++++++++++ |
| A | test/ | 42 | +++++++++ |
| A | test/ | 47 | +++++++++++ |
| A | test/ | 35 | ++++++++ |
@@ -1158,8 +1158,18 @@ EOF
11581158 # their release downloads (sent as attachments by that server, and left11591159 # uncompressed by @compress), and the 404 every other missing page gets11601160 # (lib/private-server.ts). So Caddy needs no list of private repos.1161+ #1162+ # Comments and approvals posted from review pages go to the private1163+ # server too, public repo or not, whatever www has at the path.11611164 hashed='^/assets/([^/]+\.)?[0-9A-Za-z_-]{10}\.[a-z]+$'11621165 cat <<EOF1166+ @discuss {1167+ method POST1168+ path_regexp ^/[^/]+/[^/]+/reviews/[0-9]+/(comment|approve)$1169+ }1170+ handle @discuss {1171+ reverse_proxy 127.0.0.1:${FORGE_PRIVATE_PORT:-8093}1172+ }11631173 @public file {11641174 root $FORGE_WWW11651175 try_files {path} {path}/@@ -1614,7 +1624,9 @@ review_changed() {
16141624 #16151625 # Stored as <repo>.git/forge/comments/<review>/<n>: header lines, a blank16161626 # line, the message. A line comment records the line's text too, so it still1617-# reads right once the review moves on.1627+# reads right once the review moves on. With FORGE_VIA=web (a review page,1628+# through lib/private-server.ts), comment and approve word a refusal for a1629+# page: reload it, rather than push or check out.16181630 sc_comment() {16191631 [ $# -ge 2 ] || die "usage: comment <owner>/<repo> <id|branch> [--at <sha>] [--line <path>:<line>] <message"16201632 open_review "$1" "$2"@@ -1632,7 +1644,10 @@ sc_comment() {
16321644 # You commented on the revision you have checked out; if the review has16331645 # moved on since, the comment belongs to that older revision.16341646 if [ -n "$at" ] && [ "$at" != "$RTIP" ]; then1635- rev_of "$at" >/dev/null || die "$(printf %.7s "$at") isn't a pushed revision of #$RID; push it, or name the review by number"1647+ if ! rev_of "$at" >/dev/null; then1648+ [ "${FORGE_VIA:-}" != web ] || die "$(printf %.7s "$at") isn't a revision of #$RID; reload the page and comment again"1649+ die "$(printf %.7s "$at") isn't a pushed revision of #$RID; push it, or name the review by number"1650+ fi16361651 sha=$at16371652 fi16381653 path= line= text=@@ -1686,8 +1701,11 @@ sc_approve() {
16861701 [ "$RLIVE" = 1 ] || die "#$RID is closed"16871702 [ "$user" != "$RAUTHOR" ] || die "you opened #$RID; an approval has to come from someone else"16881703 rev=$(rev_of "$RTIP" || true)1689- [ -z "$at" ] || [ "$at" = "$RTIP" ] ||1690- die "#$RID has moved on to ${rev:+r$rev }$(printf %.7s "$RTIP") since $(printf %.7s "$at"); run forge checkout $RID and look again"1704+ if [ -n "$at" ] && [ "$at" != "$RTIP" ]; then1705+ moved="#$RID has moved on to ${rev:+r$rev }$(printf %.7s "$RTIP") since $(printf %.7s "$at")"1706+ [ "${FORGE_VIA:-}" != web ] || die "$moved; reload the page to see what changed, and approve that"1707+ die "$moved; run forge checkout $RID and look again"1708+ fi16911709 tree=$(git --git-dir="$RDIR" rev-parse "$RTIP^{tree}")16921710 if awk -v u="$user" -v t="$tree" '$1 == "approve" && $3 == u && $5 == t { f = 1 } END { exit !f }' "$RF"; then16931711 echo "you already approved this tree of #$RID"@@ -159,14 +159,14 @@ export class Login {
159159 }160160 }161161 162-// A request body, as JSON or a form, of at most 64K.163-export function readBody(req: http.IncomingMessage): Promise<string> {162+// A request body, as JSON or a form, of at most 64K (or max bytes).163+export function readBody(req: http.IncomingMessage, max = 65536): Promise<string> {164164 return new Promise((ok, no) => {165165 const parts: Buffer[] = [];166166 let n = 0;167167 req.on('data', (d: Buffer) => {168168 n += d.length;169- if (n > 65536) {169+ if (n > max) {170170 no(new LoginError('request too large'));171171 req.destroy();172172 } else parts.push(d);@@ -17,16 +17,21 @@
1717 // does any missing page of www, since Caddy sends those here too.1818 //1919 // /login/ sign in, add a passkey from an ssh link, sign out (lib/login.ts)20+// /login/whoami who is signed in, and their CSRF token, for review pages' script2021 // /private/ the private repos you can read2122 // /assets/<file> the forge's assets, for pages here before a build has shipped them2223 // /<owner>/<repo>/ a private repo's pages2324 // /auth/git Caddy's forward_auth for HTTPS clones of repos without2425 // git-daemon-export-ok: may this token read X-Forge-Repo?26+// POST /<owner>/<repo>/reviews/<id>/comment, .../approve27+// a comment or approval from a review page, public or28+// private, made the way ssh makes it (discuss, below)2529 //2630 // node lib/private-server.ts (FORGE_ROOT, FORGE_BASE_URL, FORGE_PRIVATE_PORT: 8093)2731 import http from 'node:http';2832 import fs from 'node:fs';2933 import path from 'node:path';34+import { spawn } from 'node:child_process';3035 import { fileURLToPath } from 'node:url';3136 import { Identity, SESSION_TTL, TOKEN_PREFIX, cookieValue, csrfToken, sameSecret, sessionCookie, type Session, type Token } from './identity.ts';3237 import { canRead, isPublic, readable, repoDir, users } from './access.ts';@@ -135,6 +140,43 @@ export class Gate {
135140 }136141 }137142 143+// Only Authorization: Bearer, for posts: a browser resends basic auth it144+// has cached by itself, which would make it as ambient as a cookie.145+export function bearer(req: http.IncomingMessage): string | null {146+ return /^Bearer +(\S+)$/i.exec(req.headers.authorization ?? '')?.[1] ?? null;147+}148+149+// Comments and approvals from review pages (site/assets/discuss.ts) are150+// made by the forge's own command, the one ssh's git-shell-commands run:151+// forge shell-cmd comment|approve, as the user, with the message on stdin.152+// So the comment file, the approve line in the review log, the rules (who153+// may approve, outdated revisions, the 32 KiB cap), the event and the154+// rebuild are exactly what forge comment and forge approve get.155+const FORGE_BIN = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'bin', 'forge');156+export const POST_PATH = /^\/([^/]+)\/([^/]+)\/reviews\/([1-9][0-9]{0,8})\/(comment|approve)$/;157+// Each user posts at most this many times a minute, and this many run at once.158+const POSTS_PER_MINUTE = 20;159+const RUNNING_MAX = 4;160+// A comment is at most 32 KiB (sc_comment); escaped as JSON, a few times that.161+const POST_MAX = 160 * 1024;162+163+export function shellCmd(cfg: PrivateConfig, user: string, args: string[], input: string): Promise<{ code: number | null; out: string; err: string }> {164+ return new Promise((ok, no) => {165+ // FORGE_VIA: refusals worded for a page (reload it), not for a checkout.166+ const env: NodeJS.ProcessEnv = { ...process.env, FORGE_ROOT: cfg.root, FORGE_USER: user, FORGE_VIA: 'web' };167+ if (cfg.baseUrl) env.FORGE_BASE_URL = cfg.baseUrl;168+ const p = spawn(FORGE_BIN, ['shell-cmd', ...args], { cwd: cfg.root, env, stdio: ['pipe', 'pipe', 'pipe'], timeout: 5 * 60 * 1000 });169+ const out: Buffer[] = [];170+ const err: Buffer[] = [];171+ p.stdout.on('data', (d: Buffer) => out.push(d));172+ p.stderr.on('data', (d: Buffer) => err.push(d));173+ p.on('error', no);174+ p.on('close', (code) => ok({ code, out: Buffer.concat(out).toString('utf8'), err: Buffer.concat(err).toString('utf8') }));175+ p.stdin.on('error', () => {});176+ p.stdin.end(input);177+ });178+}179+138180 // The sign-in and list pages: no framing, scripts and styles of our own only.139181 const PAGE_HEADERS = {140182 'Content-Type': 'text/html; charset=utf-8', 'Cache-Control': 'private, no-store', 'X-Frame-Options': 'DENY', 'Referrer-Policy': 'same-origin',@@ -255,6 +297,92 @@ export function privateHandler(cfg: PrivateConfig): http.RequestListener {
255297 return { 'Set-Cookie': sessionCookie(secret, SESSION_TTL) };256298 }257299 300+ // Who is signed in, for the script of static review pages, which shows301+ // the comment form and Approve button to someone signed in, with the CSRF302+ // token their posts carry. Never kept by any cache.303+ function whoami(req: http.IncomingMessage, res: http.ServerResponse): void {304+ const who = gate.bySession(cookieValue(req.headers.cookie));305+ const body = who ? { user: who.user, csrf: csrfToken(who.secret!) } : { user: null };306+ json(res, 200, body, { 'Cache-Control': 'private, no-store', Vary: 'Cookie', ...cookie(who) });307+ }308+309+ // A comment or approval. A private repo you can't read, signed in or310+ // not, is the 404 a missing repo gets, as for its pages. Then it takes a311+ // user, signed in or with a Bearer token (which may be for this repo312+ // only), and for a session, a post from the forge's own pages (Origin)313+ // that carries the session's CSRF token. The fields come as a form or314+ // JSON: body (markdown), at (the tip the page showed, so a comment315+ // belongs to the revision its writer saw and an approval of a review that316+ // has moved on is refused), line (path:n, on the new side), csrf. The317+ // answer is JSON, { ok, message, comment } or { error }, in the forge's318+ // own words.319+ const recent = new Map<string, number[]>();320+ let running = 0;321+ async function discuss(req: http.IncomingMessage, res: http.ServerResponse, m: RegExpExecArray): Promise<void> {322+ const who = gate.byToken(bearer(req)) ?? gate.bySession(cookieValue(req.headers.cookie));323+ let slug: string;324+ try {325+ slug = `${decodeURIComponent(m[1])}/${decodeURIComponent(m[2])}`;326+ } catch {327+ return notFound(res);328+ }329+ const dir = repoDir(cfg.root, slug);330+ if (!dir || !(isPublic(dir) || gate.reads(who, slug, dir))) return notFound(res);331+ const what = m[4] === 'comment' ? 'comment' : 'approve';332+ if (!who) return json(res, 401, { error: `Sign in to ${what}` });333+ if (who.token?.repo && who.token.repo !== slug) return json(res, 403, { error: `This token is for ${who.token.repo} only` });334+ if (who.session && (!login || req.headers.origin !== login.rp.origin)) return json(res, 403, { error: 'not from this forge' });335+ let raw: string;336+ try {337+ if (Number(req.headers['content-length'] ?? 0) > POST_MAX) throw new Error('too large');338+ raw = await readBody(req, POST_MAX);339+ } catch {340+ return json(res, 413, { error: `${what}: longer than 32 KiB` });341+ }342+ let form: Record<string, unknown>;343+ if (/^application\/json\b/i.test(req.headers['content-type'] ?? '')) {344+ try {345+ form = JSON.parse(raw || '{}');346+ } catch {347+ return json(res, 400, { error: 'not JSON' });348+ }349+ if (!form || typeof form !== 'object' || Array.isArray(form)) return json(res, 400, { error: 'not a JSON object' });350+ } else form = Object.fromEntries(new URLSearchParams(raw));351+ const field = (k: string): string => (form[k] == null ? '' : String(form[k]));352+ if (who.session && !sameSecret(field('csrf'), csrfToken(who.secret!))) return json(res, 403, { error: 'This page is out of date: reload it and try again' });353+ const at = field('at');354+ if (at && !/^([0-9a-f]{40}|[0-9a-f]{64})$/.test(at)) return json(res, 400, { error: `${what}: at is not a commit id` });355+ const t = Date.now();356+ const mine = (recent.get(who.user) ?? []).filter((x) => t - x < 60_000);357+ if (mine.length >= POSTS_PER_MINUTE) return json(res, 429, { error: 'That is a lot at once: wait a minute' }, { 'Retry-After': '60' });358+ if (running >= RUNNING_MAX) return json(res, 503, { error: 'The forge is busy: try again in a moment' }, { 'Retry-After': '5' });359+ mine.push(t);360+ recent.set(who.user, mine);361+ const args = [what, slug, m[3]];362+ if (at) args.push('--at', at);363+ let input = '';364+ if (what === 'comment') {365+ if (field('line')) args.push('--line', field('line'));366+ // Lines of text, as forge comment -m sends it.367+ input = field('body');368+ if (input && !input.endsWith('\n')) input += '\n';369+ }370+ running++;371+ let r;372+ try {373+ r = await shellCmd(cfg, who.user, args, input);374+ } finally {375+ running--;376+ }377+ if (r.code !== 0) {378+ const said = r.err.split('\n').filter((l) => l.startsWith('forge: ')).map((l) => l.slice(7)).join('\n') || `${what} failed`;379+ return json(res, /^no (review|repo) /.test(said) ? 404 : 400, { error: said }, cookie(who));380+ }381+ const message = r.out.trim();382+ const n = /^comment (\d+) /.exec(message)?.[1];383+ json(res, 200, { ok: true, message, ...(n ? { comment: Number(n) } : {}) }, cookie(who));384+ }385+258386 async function post(req: http.IncomingMessage, res: http.ServerResponse, url: URL): Promise<void> {259387 // Only the forge's own pages may post here (and sign anyone in or out).260388 if (!login || req.headers.origin !== login.rp.origin) return json(res, 403, { error: 'not from this forge' });@@ -300,7 +428,10 @@ export function privateHandler(cfg: PrivateConfig): http.RequestListener {
300428 try {301429 const url = new URL(req.url ?? '/', 'http://x');302430 const get = req.method === 'GET' || req.method === 'HEAD';303- if (url.pathname.startsWith('/login/') && req.method === 'POST') post(req, res, url).catch(fail);431+ const write = req.method === 'POST' ? POST_PATH.exec(url.pathname) : null;432+ if (write) discuss(req, res, write).catch(fail);433+ else if (url.pathname === '/login/whoami' && get) whoami(req, res);434+ else if (url.pathname.startsWith('/login/') && req.method === 'POST') post(req, res, url).catch(fail);304435 else if (url.pathname === '/login/' && get) loginPage(req, res, url);305436 else if (url.pathname === '/private/' && get) listPage(req, res);306437 else if (url.pathname === '/login' || url.pathname === '/private') res.writeHead(308, { Location: url.pathname + '/' + url.search }).end();@@ -10,7 +10,7 @@
1010 // -w <format> then format, with %{http_code} and %{redirect_url}1111 // -H 'Name: value' a request header (more than one: repeat it)1212 // -X <method> the request's method13-// -d <data> a POST body, form-encoded unless -H says otherwise13+// -d <data> a POST body, form-encoded unless -H says otherwise; @file reads it from file1414 // -u user:password basic auth1515 // --max-time <s> give up after that long (exit 28, as curl)1616 // -s, -N accepted, and what it does anyway: no progress, no buffering@@ -43,7 +43,12 @@ for (let i = 0; i < args.length; i++) {
4343 else if (a === '-I') head = true;4444 else if (a === '-w') format = next();4545 else if (a === '-X') method = next();46- else if (a === '-d') data = next();46+ else if (a === '-d') {47+ // @file: the file's contents less carriage returns and newlines, as curl48+ // reads it, for bodies longer than an argument may be.49+ const d = next();50+ data = d.startsWith('@') ? fs.readFileSync(d.slice(1), 'utf8').replace(/[\r\n]/g, '') : d;51+ }4752 else if (a === '--max-time') maxTime = Number(next());4853 else if (a === '-u') headers.push(['Authorization', `Basic ${Buffer.from(next()).toString('base64')}`]);4954 else if (a === '-H') {@@ -21,6 +21,8 @@
2121 # sync_builds hooks and ssh commands build pages before returning2222 # lq "<sql>" forge lake query: the rows as CSV, no header2323 # private_server lib/private-server.ts for this forge, at $PRIV24+# signin ana ana adds a passkey and signs in at $PRIV: the session cookie25+# csrf_of <cookie> the CSRF token /login/whoami gives that session's pages2426 # http [curl options] url an HTTP request: curl's options (test/http.ts)2527 # response url [http options] a whole HTTP response less its Date, to compare2628 #@@ -305,3 +307,17 @@ response() {
305307 shift306308 http -s -i "$@" "$u" | grep -av '^Date: '307309 }310+311+# signin user: a passkey for user from the link ssh passkey prints, added312+# and signed in with at $PRIV by the software authenticator, which keeps it313+# in $T/<user>.key. Prints the session cookie's value.314+signin() {315+ code=$(ssh_as "$1" passkey | sed -n 's/^ .*#enroll=//p')316+ node "$T_HOME/test/authenticator.ts" enroll "$PRIV" "$code" "$T/$1.key"317+}318+319+# csrf_of cookie: the CSRF token the session's pages post with, as their320+# script gets it from /login/whoami.321+csrf_of() {322+ http -s -H "Cookie: __Host-forge=$1" "$PRIV/login/whoami" | node -e 'let s = ""; process.stdin.on("data", (d) => (s += d)).on("end", () => console.log(JSON.parse(s).csrf ?? ""))'323+}@@ -2,7 +2,9 @@
22 # to the private server, so it needs no list of private repos. With caddy33 # installed, the real thing: public pages come from www with Caddy's cache44 # headers, a private page goes to its reader with a token, and anyone else5-# gets the same response as for a page that isn't there.5+# gets the same response as for a page that isn't there. A comment posted6+# to a public repo's review, whose pages are static files, reaches the7+# private server as well.68 . "$(dirname "$0")/../lib.sh"79 user dami810 user cat@@ -13,6 +15,9 @@ for r in secret open; do
1315 at "$T/w/$r" commit "first"1416 ok push_as dami "$T/w/$r" origin main1517 done18+at "$T/w/open" git switch -q -c dami/feature19+at "$T/w/open" commit "feature"20+ok push_as dami "$T/w/open" origin dami/feature1621 build1722 1823 ok forge server caddyfile git.test@@ -22,7 +27,10 @@ has "$T/Caddyfile" '^ try_files \{path\} \{path\}/$'
2227 has "$T/Caddyfile" '^ reverse_proxy 127\.0\.0\.1:8093$'2328 lacks "$T/Caddyfile" 'private/www'2429 # The private server is the last handler: only what nothing else took.25-eq "$(grep -n 'reverse_proxy 127.0.0.1:8093' "$T/Caddyfile" | cut -d: -f1)" "$(($(grep -n '^}$' "$T/Caddyfile" | head -n 1 | cut -d: -f1) - 2))"30+eq "$(grep -n 'reverse_proxy 127.0.0.1:8093' "$T/Caddyfile" | tail -n 1 | cut -d: -f1)" "$(($(grep -n '^}$' "$T/Caddyfile" | head -n 1 | cut -d: -f1) - 2))"31+# And comments and approvals, before www's files.32+has "$T/Caddyfile" '^ path_regexp \^/\[\^/\]\+/\[\^/\]\+/reviews/\[0-9\]\+/\(comment\|approve\)\$$'33+[ "$(grep -n '^ handle @discuss {$' "$T/Caddyfile" | cut -d: -f1)" -lt "$(grep -n '^ handle @public {$' "$T/Caddyfile" | cut -d: -f1)" ] || fail "@discuss comes after @public"2634 command -v caddy >/dev/null 2>&1 || { note "caddy is not installed: skipped running it"; exit 0; }2735 ok caddy adapt --config "$T/Caddyfile" --adapter caddyfile2836 @@ -66,3 +74,11 @@ same "$c/dami/secret/"
6674 same "$c/dami/secret/" -H "Authorization: Bearer $cat"6775 same "$c/dami/open/nope/"6876 same "$c/nobody/x/y/"77+78+# A comment on a public review, through Caddy, with a token.79+exists "$FORGE_WWW/dami/open/reviews/1/index.html"80+eq "$(http -s -o "$T/res" -w '%{http_code}' -H "Authorization: Bearer $cat" -d 'body=Through Caddy' "$c/dami/open/reviews/1/comment")" 20081+has "$T/res" '"ok":true,"message":"comment 1 on #1'82+has "$FORGE_ROOT/dami/open.git/forge/comments/1/1" '^Through Caddy$'83+eq "$(http -s -o "$T/res" -w '%{http_code}' -d 'body=Signed out' "$c/dami/open/reviews/1/comment")" 40184+same "$c/dami/secret/reviews/1/comment" -X POST -H "Authorization: Bearer $cat" -d 'body=hi'@@ -0,0 +1,36 @@
1+# Approve on a review page, signed in, writes the approve line forge2+# approve writes, so forge.requireapproval lets main move to that tree. The3+# review's author can't approve it from the web either, and approving the4+# same tree twice adds nothing.5+. "$(dirname "$0")/../lib.sh"6+user dami7+user ana8+repo dami/greet9+clone dami dami/greet "$T/w/dami"10+at "$T/w/dami" commit "first"11+ok push_as dami "$T/w/dami" origin main12+git --git-dir="$FORGE_ROOT/dami/greet.git" config forge.requireapproval true13+at "$T/w/dami" git switch -q -c dami/feature14+at "$T/w/dami" commit "feature"15+ok push_as dami "$T/w/dami" origin dami/feature16+tip=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)17+export FORGE_BASE_URL=https://forge.test18+private_server19+ana=$(signin ana) || fail "ana can't sign in"20+dami=$(signin dami) || fail "dami can't sign in"21+approve() { http -s -w ' %{http_code}' -H "Cookie: __Host-forge=$1" -H 'Origin: https://forge.test' -d "at=$tip&csrf=$(csrf_of "$1")" "$PRIV/dami/greet/reviews/1/approve"; }22+log=$FORGE_ROOT/dami/greet.git/forge/reviews/123+24+approve "$dami" >"$T/res"25+has "$T/res" '"error":"you opened #1; an approval has to come from someone else"\} 400$'26+lacks "$log" '^approve '27+fails_with "is not approved" push_as dami "$T/w/dami" origin dami/feature:main28+29+approve "$ana" >"$T/res"30+has "$T/res" '"ok":true,"message":"approved #1 r1 \('"$(printf %.7s "$tip")"'\): https://forge.test/dami/greet/reviews/1/"\} 200$'31+has "$log" "^approve $T_NOW ana $tip $(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse "$tip^{tree}")\$"32+approve "$ana" >"$T/res"33+has "$T/res" '"message":"you already approved this tree of #1"'34+eq "$(grep -c '^approve ' "$log")" 135+has "$FORGE_ROOT/.forge/events.jsonl" '"type":"approve","user":"ana","review":1'36+ok push_as dami "$T/w/dami" origin dami/feature:main@@ -0,0 +1,48 @@
1+# A comment or approval that rides on the session cookie must come from2+# the forge's own pages: the Origin is the forge's and the post carries the3+# session's CSRF token, which another session's token isn't. Signed out,4+# nothing is posted. None of these write anything.5+. "$(dirname "$0")/../lib.sh"6+user dami7+user ana8+user cat9+repo dami/greet10+clone dami dami/greet "$T/w/dami"11+at "$T/w/dami" commit "first"12+ok push_as dami "$T/w/dami" origin main13+at "$T/w/dami" git switch -q -c dami/feature14+at "$T/w/dami" commit "feature"15+ok push_as dami "$T/w/dami" origin dami/feature16+export FORGE_BASE_URL=https://forge.test17+private_server18+ana=$(signin ana) || fail "ana can't sign in"19+cat=$(signin cat) || fail "cat can't sign in"20+csrf=$(csrf_of "$ana")21+[ -n "$csrf" ] || fail "no csrf token for ana"22+[ "$csrf" != "$(csrf_of "$cat")" ] || fail "two sessions have one csrf token"23+code() { http -s -o "$T/res" -w '%{http_code}' -X POST "$@"; }24+for a in comment approve; do25+ u=$PRIV/dami/greet/reviews/1/$a26+ eq "$(code -H "Cookie: __Host-forge=$ana" -H 'Origin: https://forge.test' -d 'body=hi' "$u")" 40327+ has "$T/res" '"error":"This page is out of date: reload it and try again"'28+ eq "$(code -H "Cookie: __Host-forge=$ana" -H 'Origin: https://forge.test' -d "body=hi&csrf=$(csrf_of "$cat")" "$u")" 40329+ eq "$(code -H "Cookie: __Host-forge=$ana" -H 'Origin: https://evil.test' -d "body=hi&csrf=$csrf" "$u")" 40330+ has "$T/res" '"error":"not from this forge"'31+ eq "$(code -H "Cookie: __Host-forge=$ana" -d "body=hi&csrf=$csrf" "$u")" 40332+ eq "$(code -H 'Origin: https://forge.test' -d "body=hi&csrf=$csrf" "$u")" 40133+ has "$T/res" "\"error\":\"Sign in to $a\""34+ eq "$(code -H 'Cookie: __Host-forge=nope' -H 'Origin: https://forge.test' -d "body=hi&csrf=$csrf" "$u")" 40135+done36+missing "$FORGE_ROOT/dami/greet.git/forge/comments/1"37+lacks "$FORGE_ROOT/dami/greet.git/forge/reviews/1" '^approve '38+# The real thing, to show the rest were refused for what they lacked.39+eq "$(code -H "Cookie: __Host-forge=$ana" -H 'Origin: https://forge.test' -d "body=hi&csrf=$csrf" "$PRIV/dami/greet/reviews/1/comment")" 20040+exists "$FORGE_ROOT/dami/greet.git/forge/comments/1/1"41+42+# who is asking, for the page's script: never kept by a cache.43+http -s -D "$T/h" -o "$T/who" -H "Cookie: __Host-forge=$ana" "$PRIV/login/whoami"44+has "$T/h" '^Cache-Control: private, no-store'45+has "$T/h" '^Vary: Cookie'46+has "$T/who" "^\\{\"user\":\"ana\",\"csrf\":\"$csrf\"\\}\$"47+http -s -o "$T/who" "$PRIV/login/whoami"48+has "$T/who" '^\{"user":null\}$'@@ -0,0 +1,38 @@
1+# Posts from the web are capped: 20 a minute for each user (then 429, with2+# Retry-After), and a body no larger than a comment may be, escaped. The3+# comment itself is held to forge comment's 32 KiB.4+. "$(dirname "$0")/../lib.sh"5+user dami6+user ana7+user cat8+repo dami/greet9+clone dami dami/greet "$T/w/dami"10+at "$T/w/dami" commit "first"11+ok push_as dami "$T/w/dami" origin main12+at "$T/w/dami" git switch -q -c dami/feature13+at "$T/w/dami" commit "feature"14+ok push_as dami "$T/w/dami" origin dami/feature15+private_server16+ana=$(ssh_as ana token 2>/dev/null)17+cat=$(ssh_as cat token 2>/dev/null)18+approve() { http -s -o "$T/res" -D "$T/h" -w '%{http_code}' -H "Authorization: Bearer $1" -X POST "$PRIV/dami/greet/reviews/1/approve"; }19+20+n=021+while [ "$n" -lt 20 ]; do22+ eq "$(approve "$ana")" 20023+ n=$((n + 1))24+done25+eq "$(approve "$ana")" 42926+has "$T/h" '^Retry-After: 60'27+has "$T/res" '"error":"That is a lot at once: wait a minute"'28+eq "$(approve "$cat")" 20029+30+# The body from a file: Linux takes no argument longer than 128 KiB.31+code() {32+ node -e 'process.stdout.write("body=" + "a".repeat(Number(process.argv[1])))' "$1" >"$T/body"33+ http -s -o "$T/res" -w '%{http_code}' -H "Authorization: Bearer $cat" -d "@$T/body" "$PRIV/dami/greet/reviews/1/comment"34+}35+eq "$(code 32768)" 40036+has "$T/res" '"error":"comment: longer than 32 KiB"'37+eq "$(code 200000)" 41338+eq "$(code 32767)" 200@@ -0,0 +1,48 @@
1+# A review page posts the tip it shows. When the review has moved on since2+# the page was built, a comment from it belongs to the revision it showed,3+# as one from a checkout of that revision does, and an approval from it is4+# refused. A tip the review never had is refused too. The forge words5+# these refusals for a page (reload it); over ssh they still say to check6+# out or push.7+. "$(dirname "$0")/../lib.sh"8+user dami9+user ana10+repo dami/greet11+clone dami dami/greet "$T/w/dami"12+at "$T/w/dami" commit "first"13+ok push_as dami "$T/w/dami" origin main14+at "$T/w/dami" git switch -q -c dami/feature15+at "$T/w/dami" commit "feature" greet.sh16+ok push_as dami "$T/w/dami" origin dami/feature17+r1=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)18+at "$T/w/dami" commit "more" greet.sh19+ok push_as dami "$T/w/dami" origin dami/feature20+r2=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)21+export FORGE_BASE_URL=https://forge.test22+private_server23+ana=$(signin ana) || fail "ana can't sign in"24+csrf=$(csrf_of "$ana")25+post() { http -s -w ' %{http_code}' -H "Cookie: __Host-forge=$ana" -H 'Origin: https://forge.test' -d "$2&csrf=$csrf" "$PRIV/dami/greet/reviews/1/$1"; }26+d=$FORGE_ROOT/dami/greet.git/forge/comments/127+28+post comment "body=On the first&at=$r1&line=greet.sh:1" >"$T/res"29+has "$T/res" '"message":"comment 1 on #1 r1: https://forge.test/dami/greet/reviews/1/#comment-1\\n\(#1 has a newer revision, so this shows as outdated\)"'30+has "$d/1" "^sha $r1\$"31+has "$d/1" '^rev 1$'32+post comment "body=On the second&at=$r2" >"$T/res"33+has "$d/2" "^sha $r2\$"34+has "$d/2" '^rev 2$'35+36+post approve "at=$r1" >"$T/res"37+has "$T/res" "\"error\":\"#1 has moved on to r2 $(printf %.7s "$r2") since $(printf %.7s "$r1"); reload the page to see what changed, and approve that\"\\} 400\$"38+fails_with "^forge: #1 has moved on to r2 $(printf %.7s "$r2") since $(printf %.7s "$r1"); run forge checkout 1 and look again\$" ssh_as ana approve dami/greet 1 --at "$r1"39+lacks "$FORGE_ROOT/dami/greet.git/forge/reviews/1" '^approve '40+main=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse main)41+post comment "body=Nowhere&at=$main" >"$T/res"42+has "$T/res" "\"error\":\"$(printf %.7s "$main") isn't a revision of #1; reload the page and comment again\"\\} 400\$"43+echo Nowhere | fails_with "^forge: $(printf %.7s "$main") isn't a pushed revision of #1; push it, or name the review by number\$" ssh_as ana comment dami/greet 1 --at "$main"44+post comment "body=Nowhere&at=nope" >"$T/res"45+has "$T/res" '"error":"comment: at is not a commit id"\} 400$'46+post approve "at=$r2" >"$T/res"47+has "$T/res" '"ok":true'48+missing "$d/3"@@ -0,0 +1,42 @@
1+# On a private repo, a reader comments and approves from its pages like2+# anyone on a public one. Anyone else, signed in or not, with a token or3+# not, gets the response a repo that isn't there gets, byte for byte, and4+# nothing is written.5+. "$(dirname "$0")/../lib.sh"6+user dami7+user ana8+user cat9+repo --private dami/secret10+clone dami dami/secret "$T/w/dami"11+at "$T/w/dami" commit "first"12+ok push_as dami "$T/w/dami" origin main13+at "$T/w/dami" git switch -q -c dami/feature14+at "$T/w/dami" commit "feature"15+ok push_as dami "$T/w/dami" origin dami/feature16+ok ssh_as dami readers dami/secret add ana17+export FORGE_BASE_URL=https://forge.test18+private_server19+ana=$(signin ana) || fail "ana can't sign in"20+cat=$(signin cat) || fail "cat can't sign in"21+cat_token=$(ssh_as cat token 2>/dev/null)22+web() { http -s -o "$T/res" -w '%{http_code}' -H "Cookie: __Host-forge=$1" -H 'Origin: https://forge.test' -d "body=hi&csrf=$(csrf_of "$1")" "$PRIV/dami/secret/reviews/1/$2"; }23+24+eq "$(web "$ana" comment)" 20025+eq "$(web "$ana" approve)" 20026+has "$FORGE_ROOT/dami/secret.git/forge/comments/1/1" '^user ana$'27+has "$FORGE_ROOT/dami/secret.git/forge/reviews/1" '^approve [0-9]+ ana '28+has "$FORGE_ROOT/.forge/events.jsonl" '"type":"comment","user":"ana","review":1,"private":true'29+30+for a in comment approve; do31+ response "$PRIV/dami/nope/reviews/1/$a" -X POST -H 'Origin: https://forge.test' -d 'body=hi' >"$T/missing"32+ has "$T/missing" '^HTTP/1.1 404 Not Found'33+ same() {34+ response "$PRIV/dami/secret/reviews/1/$a" -X POST -H 'Origin: https://forge.test' "$@" >"$T/got"35+ cmp -s "$T/missing" "$T/got" || { diff "$T/missing" "$T/got" >&2 || true; fail "$a on dami/secret with $* is not the missing repo's 404"; }36+ }37+ same -d 'body=hi'38+ same -H "Cookie: __Host-forge=$cat" -d "body=hi&csrf=$(csrf_of "$cat")"39+ same -H "Authorization: Bearer $cat_token" -d 'body=hi'40+done41+eq "$(ls "$FORGE_ROOT/dami/secret.git/forge/comments/1")" 142+eq "$(grep -c '^approve ' "$FORGE_ROOT/dami/secret.git/forge/reviews/1")" 1@@ -0,0 +1,47 @@
1+# A comment posted from a review page, signed in with a passkey, is the2+# comment forge comment makes over ssh: the same file, byte for byte, for3+# the same text at the same time, on the whole review and on a line, and4+# the same event for open pages. The answer says where it went.5+. "$(dirname "$0")/../lib.sh"6+user dami7+user ana8+repo dami/greet9+clone dami dami/greet "$T/w/dami"10+at "$T/w/dami" commit "first" greet.sh11+ok push_as dami "$T/w/dami" origin main12+at "$T/w/dami" git switch -q -c dami/feature13+at "$T/w/dami" commit "feature" greet.sh14+ok push_as dami "$T/w/dami" origin dami/feature15+tip=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)16+export FORGE_BASE_URL=https://forge.test17+private_server18+ana=$(signin ana) || fail "ana can't sign in"19+csrf=$(csrf_of "$ana")20+post() { http -s -H "Cookie: __Host-forge=$ana" -H 'Origin: https://forge.test' -H 'Content-Type: application/json' -d "$1" "$PRIV/dami/greet/reviews/1/comment"; }21+d=$FORGE_ROOT/dami/greet.git/forge/comments/122+23+printf 'Looks *good*\n\nOne thing.\n' | ssh_as ana comment dami/greet 1 >/dev/null24+post '{"body":"Looks *good*\n\nOne thing.","at":"'"$tip"'","csrf":"'"$csrf"'"}' >"$T/res"25+has "$T/res" '"ok":true'26+has "$T/res" '"comment":2'27+has "$T/res" '"message":"comment 2 on #1 r1: https://forge.test/dami/greet/reviews/1/#comment-2"'28+cmp "$d/1" "$d/2" || fail "the web's comment differs from ssh's"29+has "$d/2" '^user ana$'30+31+printf 'Why 2?\n' | ssh_as ana comment dami/greet 1 --line greet.sh:2 >/dev/null32+post '{"body":"Why 2?","line":"greet.sh:2","at":"'"$tip"'","csrf":"'"$csrf"'"}' >"$T/res"33+has "$T/res" '"comment":4'34+cmp "$d/3" "$d/4" || fail "the web's line comment differs from ssh's"35+has "$d/4" '^path greet.sh$'36+has "$d/4" '^line 2$'37+38+# Open pages hear of each the same way.39+eq "$(grep -c '"type":"comment","user":"ana","review":1' "$FORGE_ROOT/.forge/events.jsonl")" 440+eq "$(grep '"type":"comment"' "$FORGE_ROOT/.forge/events.jsonl" | sort -u | wc -l | tr -d ' ')" 141+42+# What the forge refuses, it refuses in its own words.43+post '{"body":"x","line":"nope.sh:1","at":"'"$tip"'","csrf":"'"$csrf"'"}' >"$T/res"44+has "$T/res" '"error":"comment: no file nope.sh in #1 at '"$(printf %.7s "$tip")"'"'45+post '{"body":" ","csrf":"'"$csrf"'"}' >"$T/res"46+has "$T/res" '"error":"comment: empty message"'47+missing "$d/5"@@ -0,0 +1,35 @@
1+# A token (Authorization: Bearer) comments and approves without a cookie,2+# so without a CSRF token: curl and scripts can. A token for one repo works3+# on that repo only, and a token in basic auth, which browsers resend by4+# themselves, doesn't post.5+. "$(dirname "$0")/../lib.sh"6+user dami7+user ana8+repo dami/greet9+repo dami/other10+for r in greet other; do11+ clone dami "dami/$r" "$T/w/$r"12+ at "$T/w/$r" commit "first"13+ ok push_as dami "$T/w/$r" origin main14+ at "$T/w/$r" git switch -q -c dami/feature15+ at "$T/w/$r" commit "feature"16+ ok push_as dami "$T/w/$r" origin dami/feature17+done18+export FORGE_BASE_URL=https://forge.test19+private_server20+all=$(ssh_as ana token 2>/dev/null)21+one=$(ssh_as ana token --repo dami/other 2>/dev/null)22+code() { http -s -o "$T/res" -w '%{http_code}' -X POST "$@"; }23+24+eq "$(code -H "Authorization: Bearer $all" -d 'body=From curl' "$PRIV/dami/greet/reviews/1/comment")" 20025+has "$FORGE_ROOT/dami/greet.git/forge/comments/1/1" '^user ana$'26+has "$FORGE_ROOT/dami/greet.git/forge/comments/1/1" '^From curl$'27+eq "$(code -H "Authorization: Bearer $all" -H 'Content-Type: application/json' -d '{}' "$PRIV/dami/greet/reviews/1/approve")" 20028+has "$FORGE_ROOT/dami/greet.git/forge/reviews/1" '^approve [0-9]+ ana '29+30+eq "$(code -H "Authorization: Bearer $one" -d 'body=Wrong repo' "$PRIV/dami/greet/reviews/1/comment")" 40331+has "$T/res" '"error":"This token is for dami/other only"'32+eq "$(code -H "Authorization: Bearer $one" -d 'body=Right repo' "$PRIV/dami/other/reviews/1/comment")" 20033+eq "$(code -u "ana:$all" -d 'body=Basic' "$PRIV/dami/greet/reviews/1/comment")" 40134+eq "$(code -H "Authorization: Bearer forge_nope" -d 'body=Revoked' "$PRIV/dami/greet/reviews/1/comment")" 40135+eq "$(ls "$FORGE_ROOT/dami/greet.git/forge/comments/1")" 12/4 Comment and approve from a review page, signed in
The review page's discussion ends in a comment form and an Approve button. Pages are static files, the same for everyone, so both start hidden and do nothing without a script: discuss.ts asks /login/whoami who is signed in and shows them to a user, without Approve for the review's author or a review that isn't live. It posts the tip the page shows, so a comment from a page built before a push belongs to the revision its writer read, and an approval from it is refused, in the forge's words. A sent comment shows as pending, its text as typed (the build renders and sanitizes the markdown, not the browser), until the rebuilt page comes in: live.ts swaps it in when the build's event arrives, and discuss.ts asks for it again now and then should none arrive, through a forge:refresh event live.ts now takes. Drafts are kept in sessionStorage, so a swap or a reload doesn't lose them.
| M | site/ | 24 | +++- |
| A | site/ | 196 | ++++++++++++++++++++++++++++++ |
| M | site/ | 6 | +- |
| M | site/ | 1 | + |
| A | test/ | 34 | +++++ |
@@ -1,6 +1,8 @@
11 {# A review's discussion: approvals, then comments oldest first. A line22 comment quotes its line as it was when written; comments from an older3- revision are outdated. Written with forge comment and forge approve. #}3+ revision are outdated. Written with forge comment and forge approve, or4+ from this page signed in: the form at the end stays hidden until5+ discuss.ts finds who is signed in, and posts the tip the page shows. #}46 {% set u = p.repo.urls %}57 {% css %}68 ul.approvals { list-style: none; margin: 0 0 12px; padding: 0; display: flex; flex-wrap: wrap; gap: 4px 18px; font-size: 0.92em; }@@ -21,13 +23,25 @@ ol.comments { list-style: none; margin: 0; padding: 0; }
2123 .comment.outdated .quote pre { color: var(--muted); }2224 .comment .markdown { padding: 8px 12px; font-size: 0.95rem; line-height: 1.55; }2325 .comment .markdown > :last-child { margin-bottom: 0; }26+/* Sent from this page, until the rebuilt page shows it. */27+.comment.pending { border-style: dashed; }28+.comment.pending .text { padding: 8px 12px; white-space: pre-wrap; overflow-wrap: anywhere; color: var(--muted); }29+.reply { margin: 12px 0 0; }30+.reply textarea { display: block; width: 100%; min-height: 6em; resize: vertical; font: 14px/1.5 var(--sans); color: var(--fg); background: var(--bg); border: 1px solid var(--border); border-radius: var(--radius); padding: 8px 10px; }31+.reply textarea:focus { outline: none; border-color: var(--accent); box-shadow: 0 0 0 1px var(--accent); }32+.reply-bar { display: flex; flex-wrap: wrap; align-items: center; gap: 6px 8px; margin-top: 8px; font-size: 0.92em; }33+.reply-bar .grow { flex: 1; }34+.reply-bar button.go { background: var(--accent); border-color: var(--accent); color: #fff; }35+.reply-bar button.go:hover { filter: brightness(1.1); }36+.reply-bar button:disabled { opacity: 0.6; cursor: default; filter: none; }37+.reply-status.bad { color: var(--fail); white-space: pre-line; }2438 {% endcss %}2539 <section class="discussion" id="discussion">2640 <h2>Discussion <span class="desc">{{ rv.comments.length | plural('comment') }}{% if rv.approvals.length %}, {{ rv.approvals.length | plural('approval') }}{% endif %}</span></h2>2741 {% if rv.approvals.length %}2842 <ul class="approvals">2943 {% for a in rv.approvals %}30-<li class="{{ 'current' if a.current else 'stale' }}"><span class="tick">✔</span> <b>{{ a.user }}</b> approved{% if a.rev %} r{{ a.rev }}{% endif %} <span class="date">({{ a.ts | ago }})</span>{% if not a.current %}, changed since{% endif %}</li>44+<li class="{{ 'current' if a.current else 'stale' }}" data-user="{{ a.user }}"><span class="tick">✔</span> <b>{{ a.user }}</b> approved{% if a.rev %} r{{ a.rev }}{% endif %} <span class="date">({{ a.ts | ago }})</span>{% if not a.current %}, changed since{% endif %}</li>3145 {% endfor %}3246 </ul>3347 {% endif %}@@ -45,6 +59,10 @@ ol.comments { list-style: none; margin: 0; padding: 0; }
4559 {% endfor %}4660 </ol>4761 {% else %}48-<p class="desc">No comments yet: <code>forge comment {{ rv.id }}</code> adds one.</p>62+<p class="desc" id="no-comments">No comments yet: <code>forge comment {{ rv.id }}</code> adds one.</p>4963 {% endif %}64+<form class="reply" id="reply" data-post="{{ u.review(rv.id).href }}" data-at="{{ rv.tip }}" data-author="{{ rv.author or '' }}" hidden>65+<textarea name="body" maxlength="32768" aria-label="Comment on #{{ rv.id }}" placeholder="Comment on #{{ rv.id }}. Markdown is fine."></textarea>66+<div class="reply-bar"><span class="desc who"></span><span class="grow"></span><span class="reply-status" role="status"></span>{% if rv.live %}<button type="button" data-approve title="Approve this tree of #{{ rv.id }}, as forge approve {{ rv.id }} does">Approve</button>{% endif %}<button type="submit" class="go">Comment</button></div>67+</form>5068 </section>@@ -0,0 +1,196 @@
1+// Commenting and approving from a review page, signed in. Pages are static2+// and the same for everyone, so the comment form and the Approve button3+// (discussion.njk) start hidden; this asks the private server who is4+// signed in (/login/whoami, never cached) and shows them to a user. Posts5+// go to /<owner>/<repo>/reviews/<id>/comment and .../approve with the6+// session's CSRF token and the tip the page shows, and lib/private-server.ts7+// runs forge comment's and forge approve's own code for them. Until the8+// rebuilt page comes in (live.ts swaps it in once the build is done), a9+// sent comment shows as pending, as typed: its markdown is rendered by the10+// build, not here. Drafts outlast that swap, and reloads (sessionStorage).11+(() => {12+ interface Me {13+ user: string;14+ csrf: string;15+ }16+ interface Sent {17+ kind: 'comment' | 'approve';18+ n?: number;19+ text?: string;20+ }21+ let me: Me | null = null;22+ // What was sent from this page and isn't on it yet.23+ let sent: Sent[] = [];24+25+ const draftKey = (k: string): string => `forge.draft ${location.pathname} ${k}`;26+ const draft = (k: string, text?: string): string => {27+ try {28+ if (text === undefined) return sessionStorage.getItem(draftKey(k)) ?? '';29+ if (text) sessionStorage.setItem(draftKey(k), text);30+ else sessionStorage.removeItem(draftKey(k));31+ } catch {}32+ return text ?? '';33+ };34+35+ class Refused extends Error {}36+ async function send(url: string, fields: Record<string, string>): Promise<{ message: string; comment?: number }> {37+ const r = await fetch(url, {38+ method: 'POST', credentials: 'same-origin', headers: { 'Content-Type': 'application/json' },39+ body: JSON.stringify({ ...fields, csrf: me?.csrf ?? '' }),40+ });41+ const data = (await r.json().catch(() => ({}))) as { error?: string; message?: string; comment?: number };42+ if (!r.ok) throw new Refused(data.error ?? (r.status === 404 ? 'This review is not there any more.' : `The forge said HTTP ${r.status}.`));43+ return { message: data.message ?? '', comment: data.comment };44+ }45+46+ const say = (form: Element | null, text: string, bad = false): void => {47+ const s = form?.querySelector<HTMLElement>('.reply-status');48+ if (!s) return;49+ s.textContent = text;50+ s.classList.toggle('bad', bad);51+ };52+ const busy = (form: Element, on: boolean): void => {53+ for (const b of form.querySelectorAll<HTMLButtonElement>('button')) b.disabled = on;54+ };55+56+ // Is it on the page now? Then it is no longer pending.57+ const shown = (s: Sent): boolean =>58+ s.kind === 'comment' ? !!document.getElementById(`comment-${s.n}`) : !!me && [...document.querySelectorAll<HTMLElement>('ul.approvals li.current')].some((li) => li.dataset.user === me!.user);59+60+ // Pending comments at the end of the discussion, and the approval in the61+ // form's status line, drawn again after each swap of the content.62+ function drawSent(): void {63+ sent = sent.filter((s) => !shown(s));64+ for (const el of document.querySelectorAll('[data-sent]')) el.remove();65+ const form = document.getElementById('reply');66+ if (!form || !me) return;67+ for (const s of sent) {68+ if (s.kind === 'approve') {69+ say(form, 'Approved; it shows here once the page is rebuilt.');70+ continue;71+ }72+ let list = document.querySelector('ol.comments');73+ if (!list) {74+ list = document.createElement('ol');75+ list.className = 'comments';76+ list.setAttribute('data-sent', '');77+ form.before(list);78+ }79+ const li = document.createElement('li');80+ li.className = 'comment pending';81+ li.setAttribute('data-sent', '');82+ li.innerHTML = '<div class="comment-head"><b></b> <span class="date">sent, shown here once the page is rebuilt</span></div><div class="text"></div>';83+ li.querySelector('b')!.textContent = me.user;84+ li.querySelector('.text')!.textContent = s.text ?? '';85+ list.append(li);86+ }87+ if (document.querySelector('ol.comments [data-sent]')) document.getElementById('no-comments')?.setAttribute('hidden', '');88+ }89+90+ // live.ts brings the rebuilt page when the build's event arrives. Should91+ // none arrive (the stream is down, say), ask for it now and then.92+ let waiting: ReturnType<typeof setTimeout> | undefined;93+ function watch(wait = 4000): void {94+ clearTimeout(waiting);95+ if (!sent.length || wait > 120000) return;96+ waiting = setTimeout(() => {97+ document.dispatchEvent(new Event('forge:refresh'));98+ watch(wait * 2);99+ }, wait);100+ }101+102+ function setup(): void {103+ const form = document.getElementById('reply') as HTMLFormElement | null;104+ if (form && me) {105+ form.hidden = false;106+ form.querySelector('.who')!.textContent = `as ${me.user}`;107+ const text = form.querySelector('textarea')!;108+ if (!text.value) text.value = draft('review');109+ const approve = form.querySelector<HTMLButtonElement>('[data-approve]');110+ if (approve && me.user === form.dataset.author) approve.hidden = true;111+ else if (approve && shown({ kind: 'approve' })) {112+ approve.disabled = true;113+ approve.textContent = 'Approved';114+ approve.title = 'You approved this tree';115+ }116+ }117+ drawSent();118+ }119+120+ document.addEventListener('input', (e) => {121+ const t = e.target as HTMLTextAreaElement;122+ if (t.closest('form#reply')) draft('review', t.value);123+ });124+125+ // Ctrl or Cmd and Enter sends, as in most comment boxes.126+ document.addEventListener('keydown', (e) => {127+ const t = e.target as HTMLElement;128+ if (e.key === 'Enter' && (e.metaKey || e.ctrlKey) && t.matches('form#reply textarea')) {129+ e.preventDefault();130+ t.closest('form')!.requestSubmit();131+ }132+ });133+134+ document.addEventListener('submit', async (e) => {135+ const form = (e.target as Element).closest<HTMLFormElement>('form#reply');136+ if (!form || !me) return;137+ e.preventDefault();138+ const box = form.querySelector('textarea')!;139+ const text = box.value;140+ if (!text.trim()) return say(form, 'Write a comment first.', true);141+ busy(form, true);142+ say(form, 'Sending…');143+ try {144+ const r = await send(form.dataset.post + 'comment', { body: text, at: form.dataset.at ?? '' });145+ box.value = '';146+ draft('review', '');147+ sent.push({ kind: 'comment', n: r.comment, text });148+ // forge comment's own note, when the review moved on since this page was built.149+ say(form, r.message.split('\n').slice(1).join(' '));150+ drawSent();151+ watch();152+ } catch (err) {153+ say(form, err instanceof Refused ? err.message : "Couldn't reach the forge; your text is still here.", true);154+ } finally {155+ busy(form, false);156+ }157+ });158+159+ document.addEventListener('click', async (e) => {160+ const b = (e.target as Element).closest<HTMLButtonElement>('form#reply [data-approve]');161+ const form = b?.closest<HTMLFormElement>('form');162+ if (!b || !form || !me) return;163+ busy(form, true);164+ say(form, 'Approving…');165+ try {166+ const r = await send(form.dataset.post + 'approve', { at: form.dataset.at ?? '' });167+ // Approved before: nothing changes, so nothing to wait for.168+ if (/^you already approved/.test(r.message)) say(form, 'You approved this tree already.');169+ else {170+ sent.push({ kind: 'approve' });171+ drawSent();172+ watch();173+ }174+ b.textContent = 'Approved';175+ busy(form, false);176+ b.disabled = true;177+ } catch (err) {178+ say(form, err instanceof Refused ? err.message : "Couldn't reach the forge.", true);179+ busy(form, false);180+ }181+ });182+183+ // Signed out, or a server without the private server (npm run dev): the184+ // controls stay hidden.185+ if (!document.getElementById('reply')) return;186+ fetch('/login/whoami', { cache: 'no-store', credentials: 'same-origin' })187+ .then((r) => (r.ok ? r.json() : null))188+ .then((w: { user?: string | null; csrf?: string } | null) => {189+ if (!w?.user || !w.csrf) return;190+ me = { user: w.user, csrf: w.csrf };191+ setup();192+ // live.ts swaps in rebuilt content.193+ document.addEventListener('forge:content', setup);194+ })195+ .catch(() => {});196+})();@@ -2,7 +2,8 @@
22 // stays empty while all is well: it shows a push being applied ("cat pushed33 // main, updating") or a dropped connection. When this44 // repo's pages are rebuilt, the new #content is swapped in without a reload,5-// keeping the scroll position, and the status clears shortly after.5+// keeping the scroll position, and the status clears shortly after. A6+// forge:refresh event on the document asks for the page again the same way.67 // Reconnects if the stream drops. A server without the stream (a static file78 // server, npm run dev) answers 404; then it stays quiet.89 (() => {@@ -23,6 +24,9 @@
2324 document.dispatchEvent(new Event('forge:content'));2425 };2526 27+ // discuss.ts asks again after a post, should no build event reach the page.28+ document.addEventListener('forge:refresh', () => void refresh());29+2630 let quiet: ReturnType<typeof setTimeout> | undefined;2731 document.addEventListener('fx:sse:built', (e) => {2832 let repo: string | undefined;@@ -8,6 +8,7 @@ pagination:
88 {% set u = p.repo.urls %}99 {% block head %}{{ m.reviewMeta(rv, p.repo, p.title, u.review(rv.id).href) }}{% endblock %}1010 {% block scripts %}<script src="{{ assets.diffs }}" defer></script>11+<script src="{{ assets.discuss }}" defer></script>1112 {% endblock %}1213 {% block content %}1314 {% css %}@@ -0,0 +1,34 @@
1+# A review page carries a comment form and an Approve button for whoever2+# is signed in. The page is a static file, the same for everyone, so they3+# start hidden and discuss.js shows them once /login/whoami names a user.4+# The form posts the tip the page shows and knows the review's author,5+# whose Approve button it hides. A review that isn't live has no Approve6+# button. A private repo's review page has the same form.7+. "$(dirname "$0")/../lib.sh"8+user dami9+repo dami/greet10+repo --private dami/secret11+for r in greet secret; do12+ clone dami "dami/$r" "$T/w/$r"13+ at "$T/w/$r" commit "first"14+ ok push_as dami "$T/w/$r" origin main15+ at "$T/w/$r" git switch -q -c dami/feature16+ at "$T/w/$r" commit "feature"17+ ok push_as dami "$T/w/$r" origin dami/feature18+done19+at "$T/w/greet" git switch -q -c dami/gone20+at "$T/w/greet" commit "gone"21+ok push_as dami "$T/w/greet" origin dami/gone22+ok push_as dami "$T/w/greet" origin :dami/gone23+build24+tip=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)25+26+p=$FORGE_WWW/dami/greet/reviews/1/index.html27+has "$p" "<form class=\"reply\" id=\"reply\" data-post=\"/dami/greet/reviews/1/\" data-at=\"$tip\" data-author=\"dami\" hidden>"28+has "$p" '<textarea name="body" maxlength="32768"'29+has "$p" '<button type="button" data-approve '30+has "$p" '<script src="/assets/discuss\.[A-Za-z0-9_-]{10}\.js" defer>'31+exists "$FORGE_WWW$(grep -o '/assets/discuss\.[A-Za-z0-9_-]*\.js' "$p")"32+has "$FORGE_WWW/dami/greet/reviews/2/index.html" 'id="reply" data-post="/dami/greet/reviews/2/"'33+lacks "$FORGE_WWW/dami/greet/reviews/2/index.html" 'data-approve'34+has "$FORGE_ROOT/.forge/private/www/dami/secret/reviews/1/index.html" 'id="reply" data-post="/dami/secret/reviews/1/"'3/4 Comment on a line from the combined diff
Signed in, the combined diff says who you are commenting as, and a + sits beside the line under the pointer (or the line last tapped, on a phone). It opens a form under that line, for its new side; the post names the line as forge comment's <path>:<line> and carries the tip the diff shows. The sent comment waits under its line, as typed, until the rebuilt diff shows it there, and open forms and their drafts come back after the content is swapped. The diff names each file's path for this (data-path), on the combined diff only, and not for a deleted file, which has no new side.
| M | lib/ | 5 | +- |
| M | lib/ | 7 | +- |
| M | site/ | 259 | ++++++++++++++++++++++-------- |
| M | site/ | 17 | ++ |
| A | test/ | 26 | +++ |
@@ -470,14 +470,15 @@ export const markupFilters = new Set(['ago', 'date']);
470470 // Lazy renderers, registered as Nunjucks filters (setup() in render-page.ts).471471 export const filters = {472472 // `branch` links each changed file to its page on that branch, if present.473- // With `review`, its current line comments show under their lines. With473+ // With `review`, its current line comments show under their lines, and474+ // files carry their paths for new ones (site/assets/discuss.ts). With474475 // `mechanical` (review pages), files Jev judged mechanical are marked.475476 diffHtml(files: DiffFile[] | undefined, repo: RepoHead, prefix = '', branch: string | null = null, review: Review | null = null, mechanical = false): string {476477 const paths = branch ? new Set(registry.get(repo.slug)?.trees.get(branch)?.blobs.map((e) => e.path)) : null;477478 const blobHref = paths && branch ? (p: string) => (paths.has(p) ? repo.urls.blob(branch, p).href : null) : null;478479 const annotate = review ? inline(review, (c) => commentHtml(c, repo, review)) : null;479480 const dir = registry.get(repo.slug)?.dir;480- return html.diff(files ?? [], { prefix, blobHref, annotate, mechanical: mechanical && dir ? isMechanical(dir) : null });481+ return html.diff(files ?? [], { prefix, blobHref, annotate, mechanical: mechanical && dir ? isMechanical(dir) : null, paths: !!review });481482 },482483 // Review descriptions (one line in the log, "\n" for line breaks) and483484 // comments are markdown, sanitized like READMEs; links resolve on the branch.@@ -73,7 +73,9 @@ type Annotate = ((path: string, newLine: number) => string | null) | null;
7373 7474 // `mechanical(file)` says Jev judged the change mechanical (a rename,7575 // lockfile, generated file...): it is labelled, and diffs.ts folds it.76-export function diff(files: DiffFile[], { prefix = '', blobHref, annotate, mechanical }: { prefix?: string; blobHref?: ((path: string) => string | null) | null; annotate?: Annotate; mechanical?: ((f: DiffFile) => boolean) | null } = {}): string {76+// With `paths`, each file that still exists carries its path77+// (data-path), for line comments from the page (discuss.ts).78+export function diff(files: DiffFile[], { prefix = '', blobHref, annotate, mechanical, paths = false }: { prefix?: string; blobHref?: ((path: string) => string | null) | null; annotate?: Annotate; mechanical?: ((f: DiffFile) => boolean) | null; paths?: boolean } = {}): string {7779 let budget = MAX_PAGE_LINES;7880 return files.map((f, i) => {7981 const id = `${prefix}f${i}`;@@ -84,7 +86,8 @@ export function diff(files: DiffFile[], { prefix = '', blobHref, annotate, mecha
8486 ` <span class="counts"><span class="i">+${f.add}</span> <span class="d">-${f.del}</span></span>` +8587 (mech ? ' <span class="jev-mech" title="Likely mechanical (Jev): a rename, reformatting, generated or vendored code, a lockfile, a version bump or a search-and-replace">mechanical</span>' : '') +8688 (fileHref ? ` <a class="view" href="${fileHref}">view file</a>` : '') + '</header>';87- const wrap = (body: string): string => `<section class="file jump" id="${id}"${mech ? ' data-mechanical' : ''}>${head}${body}</section>`;89+ const at = paths && f.status !== 'D' ? ` data-path="${esc(f.path)}"` : '';90+ const wrap = (body: string): string => `<section class="file jump" id="${id}"${at}${mech ? ' data-mechanical' : ''}>${head}${body}</section>`;8891 if (f.omitted) return wrap('<p class="note">Diff not shown: this commit changes too many lines.</p>');8992 if (f.binary) return wrap('<p class="note">Binary file.</p>');9093 if (!f.hunks.length) return wrap(`<p class="note">${f.mode ? 'File mode ' + esc(f.mode) : 'No content changes.'}</p>`);@@ -1,12 +1,14 @@
11 // Commenting and approving from a review page, signed in. Pages are static22 // and the same for everyone, so the comment form and the Approve button33 // (discussion.njk) start hidden; this asks the private server who is4-// signed in (/login/whoami, never cached) and shows them to a user. Posts5-// go to /<owner>/<repo>/reviews/<id>/comment and .../approve with the6-// session's CSRF token and the tip the page shows, and lib/private-server.ts7-// runs forge comment's and forge approve's own code for them. Until the8-// rebuilt page comes in (live.ts swaps it in once the build is done), a9-// sent comment shows as pending, as typed: its markdown is rendered by the4+// signed in (/login/whoami, never cached) and shows them to a user. On the5+// combined diff, a + beside the line under the pointer (or the line last6+// clicked) opens a form for a comment on that line's new side. Posts go to7+// /<owner>/<repo>/reviews/<id>/comment and .../approve with the session's8+// CSRF token and the tip the page shows, and lib/private-server.ts runs9+// forge comment's and forge approve's own code for them. Until the rebuilt10+// page comes in (live.ts swaps it in once the build is done), a sent11+// comment shows as pending, as typed: its markdown is rendered by the1012 // build, not here. Drafts outlast that swap, and reloads (sessionStorage).1113 (() => {1214 interface Me {@@ -17,10 +19,14 @@
1719 kind: 'comment' | 'approve';1820 n?: number;1921 text?: string;22+ // path:n, for a line comment.23+ line?: string;2024 }2125 let me: Me | null = null;2226 // What was sent from this page and isn't on it yet.2327 let sent: Sent[] = [];28+ // Line comment forms open on this page, by path:n.29+ const opened = new Set<string>();2430 2531 const draftKey = (k: string): string => `forge.draft ${location.pathname} ${k}`;2632 const draft = (k: string, text?: string): string => {@@ -42,6 +48,7 @@
4248 if (!r.ok) throw new Refused(data.error ?? (r.status === 404 ? 'This review is not there any more.' : `The forge said HTTP ${r.status}.`));4349 return { message: data.message ?? '', comment: data.comment };4450 }51+ const failed = (err: unknown): string => (err instanceof Refused ? err.message : "Couldn't reach the forge; your text is still here.");4552 4653 const say = (form: Element | null, text: string, bad = false): void => {4754 const s = form?.querySelector<HTMLElement>('.reply-status');@@ -53,38 +60,120 @@
5360 for (const b of form.querySelectorAll<HTMLButtonElement>('button')) b.disabled = on;5461 };5562 63+ // ------------------------------------------------ lines of the combined diff64+65+ const lines = (): HTMLElement | null => document.getElementById('line-comments');66+ const newLine = (row: Element): string => row.querySelectorAll('.ln')[1]?.textContent ?? '';67+ // path:n of a row on the new side, or null.68+ const whereOf = (row: Element): string | null => {69+ const path = row.closest<HTMLElement>('section.file[data-path]')?.dataset.path;70+ const n = newLine(row);71+ return path && n ? `${path}:${n}` : null;72+ };73+ function rowOf(where: string): HTMLElement | null {74+ const i = where.lastIndexOf(':');75+ const file = [...document.querySelectorAll<HTMLElement>('section.file[data-path]')].find((f) => f.dataset.path === where.slice(0, i));76+ return [...(file?.querySelectorAll<HTMLElement>('.code a.l') ?? [])].find((l) => newLine(l) === where.slice(i + 1)) ?? null;77+ }78+ // Under a row, after what is under it already.79+ function below(row: HTMLElement, el: HTMLElement): void {80+ let at: Element = row;81+ while (at.nextElementSibling?.classList.contains('cmts')) at = at.nextElementSibling;82+ at.after(el);83+ }84+85+ // The + of the line under the pointer, or tapped last.86+ const plus = document.createElement('button');87+ plus.type = 'button';88+ plus.className = 'addline';89+ plus.textContent = '+';90+ function place(row: HTMLElement): void {91+ const where = me && lines() ? whereOf(row) : null;92+ const rows = row.parentElement;93+ const code = row.closest('.code');94+ if (!where || !rows || !code) return;95+ plus.dataset.where = where;96+ plus.title = `Comment on ${where}`;97+ plus.setAttribute('aria-label', plus.title);98+ if (plus.parentElement !== rows) rows.append(plus);99+ plus.style.top = `${row.offsetTop + 1}px`;100+ plus.style.left = `${code.scrollLeft + 2}px`;101+ }102+103+ function openForm(where: string, focus: boolean): void {104+ const row = rowOf(where);105+ if (!row || !me) return;106+ const had = [...document.querySelectorAll<HTMLElement>('[data-line-form]')].find((x) => x.dataset.lineForm === where);107+ if (had) {108+ if (focus) had.querySelector('textarea')?.focus();109+ return;110+ }111+ opened.add(where);112+ const box = document.createElement('div');113+ box.className = 'cmts';114+ box.dataset.lineForm = where;115+ box.innerHTML = '<form class="linecmt"><div class="where"></div><textarea name="body" maxlength="32768" placeholder="Comment on this line. Markdown is fine."></textarea>' +116+ '<div class="reply-bar"><span class="reply-status" role="status"></span><span class="grow"></span><button type="button" data-cancel>Cancel</button><button type="submit" class="go">Comment</button></div></form>';117+ box.querySelector('.where')!.textContent = `${where}, as ${me.user}`;118+ const text = box.querySelector('textarea')!;119+ text.setAttribute('aria-label', `Comment on ${where}`);120+ text.value = draft(`line ${where}`);121+ below(row, box);122+ if (focus) text.focus();123+ }124+ const closeForm = (box: HTMLElement): void => {125+ opened.delete(box.dataset.lineForm ?? '');126+ box.remove();127+ };128+129+ // ------------------------------------------------ sent, not shown yet130+56131 // Is it on the page now? Then it is no longer pending.57132 const shown = (s: Sent): boolean =>58133 s.kind === 'comment' ? !!document.getElementById(`comment-${s.n}`) : !!me && [...document.querySelectorAll<HTMLElement>('ul.approvals li.current')].some((li) => li.dataset.user === me!.user);59134 60- // Pending comments at the end of the discussion, and the approval in the61- // form's status line, drawn again after each swap of the content.135+ const pendingHtml = '<div class="comment-head"><b></b> <span class="date">sent, shown here once the page is rebuilt</span></div><div class="text"></div>';136+ function pendingBox(tag: string, cls: string, s: Sent): HTMLElement {137+ const el = document.createElement(tag);138+ el.className = cls;139+ el.setAttribute('data-sent', '');140+ el.innerHTML = pendingHtml;141+ el.querySelector('b')!.textContent = me?.user ?? '';142+ el.querySelector('.text')!.textContent = s.text ?? '';143+ return el;144+ }145+146+ // Pending comments at the end of the discussion or under their line, and147+ // the approval in the form's status line, drawn again after each swap.62148 function drawSent(): void {63149 sent = sent.filter((s) => !shown(s));64150 for (const el of document.querySelectorAll('[data-sent]')) el.remove();65151 const form = document.getElementById('reply');66- if (!form || !me) return;67152 for (const s of sent) {68- if (s.kind === 'approve') {69- say(form, 'Approved; it shows here once the page is rebuilt.');70- continue;71- }72- let list = document.querySelector('ol.comments');73- if (!list) {74- list = document.createElement('ol');75- list.className = 'comments';76- list.setAttribute('data-sent', '');77- form.before(list);153+ if (s.kind === 'approve') say(form, 'Approved; it shows here once the page is rebuilt.');154+ else if (s.line) {155+ const row = rowOf(s.line);156+ if (!row) continue;157+ const box = document.createElement('div');158+ box.className = 'cmts';159+ box.setAttribute('data-sent', '');160+ const c = pendingBox('div', 'cmt pending', s);161+ c.removeAttribute('data-sent');162+ c.querySelector('.comment-head')!.className = 'cmt-head';163+ box.append(c);164+ below(row, box);165+ } else if (form) {166+ let list = document.querySelector('ol.comments');167+ if (!list) {168+ list = document.createElement('ol');169+ list.className = 'comments';170+ list.setAttribute('data-sent', '');171+ form.before(list);172+ }173+ list.append(pendingBox('li', 'comment pending', s));174+ document.getElementById('no-comments')?.setAttribute('hidden', '');78175 }79- const li = document.createElement('li');80- li.className = 'comment pending';81- li.setAttribute('data-sent', '');82- li.innerHTML = '<div class="comment-head"><b></b> <span class="date">sent, shown here once the page is rebuilt</span></div><div class="text"></div>';83- li.querySelector('b')!.textContent = me.user;84- li.querySelector('.text')!.textContent = s.text ?? '';85- list.append(li);86176 }87- if (document.querySelector('ol.comments [data-sent]')) document.getElementById('no-comments')?.setAttribute('hidden', '');88177 }89178 90179 // live.ts brings the rebuilt page when the build's event arrives. Should@@ -99,9 +188,12 @@
99188 }, wait);100189 }101190 191+ // ------------------------------------------------ the page, signed in192+102193 function setup(): void {194+ if (!me) return;103195 const form = document.getElementById('reply') as HTMLFormElement | null;104- if (form && me) {196+ if (form) {105197 form.hidden = false;106198 form.querySelector('.who')!.textContent = `as ${me.user}`;107199 const text = form.querySelector('textarea')!;@@ -114,75 +206,112 @@
114206 approve.title = 'You approved this tree';115207 }116208 }209+ const hint = lines();210+ if (hint) {211+ hint.hidden = false;212+ hint.textContent = `Commenting as ${me.user}: the + beside a line comments on it.`;213+ document.body.classList.add('commenting');214+ for (const where of opened) openForm(where, false);215+ }117216 drawSent();118217 }119218 219+ document.addEventListener('mouseover', (e) => {220+ const row = (e.target as Element).closest<HTMLElement>('section.file[data-path] .code a.l');221+ if (row) place(row);222+ });223+224+ document.addEventListener('click', async (e) => {225+ const t = e.target as Element;226+ const row = t.closest<HTMLElement>('section.file[data-path] .code a.l');227+ // A tap on a line (no pointer to hover with) puts the + there.228+ if (row) return place(row);229+ if (t.closest('.addline')) {230+ e.preventDefault();231+ return openForm(plus.dataset.where ?? '', true);232+ }233+ const cancel = t.closest('[data-line-form] [data-cancel]')?.closest<HTMLElement>('[data-line-form]');234+ if (cancel) {235+ draft(`line ${cancel.dataset.lineForm}`, '');236+ return closeForm(cancel);237+ }238+ const b = t.closest<HTMLButtonElement>('form#reply [data-approve]');239+ const form = b?.closest<HTMLFormElement>('form');240+ if (!b || !form || !me) return;241+ busy(form, true);242+ say(form, 'Approving…');243+ try {244+ const r = await send(form.dataset.post + 'approve', { at: form.dataset.at ?? '' });245+ // Approved before: nothing changes, so nothing to wait for.246+ if (/^you already approved/.test(r.message)) say(form, 'You approved this tree already.');247+ else {248+ sent.push({ kind: 'approve' });249+ drawSent();250+ watch();251+ }252+ busy(form, false);253+ b.textContent = 'Approved';254+ b.disabled = true;255+ } catch (err) {256+ say(form, failed(err), true);257+ busy(form, false);258+ }259+ });260+261+ // A line's + follows the code when it scrolls sideways.262+ document.addEventListener('scroll', (e) => {263+ const code = e.target as Element;264+ if (code instanceof Element && code.matches('.code') && code.contains(plus)) plus.style.left = `${code.scrollLeft + 2}px`;265+ }, true);266+120267 document.addEventListener('input', (e) => {121268 const t = e.target as HTMLTextAreaElement;122- if (t.closest('form#reply')) draft('review', t.value);269+ const box = t.closest<HTMLElement>('[data-line-form]');270+ if (box) draft(`line ${box.dataset.lineForm}`, t.value);271+ else if (t.closest('form#reply')) draft('review', t.value);123272 });124273 125274 // Ctrl or Cmd and Enter sends, as in most comment boxes.126275 document.addEventListener('keydown', (e) => {127276 const t = e.target as HTMLElement;128- if (e.key === 'Enter' && (e.metaKey || e.ctrlKey) && t.matches('form#reply textarea')) {277+ if (e.key === 'Enter' && (e.metaKey || e.ctrlKey) && t.matches('form#reply textarea, form.linecmt textarea')) {129278 e.preventDefault();130279 t.closest('form')!.requestSubmit();131280 }132281 });133282 134283 document.addEventListener('submit', async (e) => {135- const form = (e.target as Element).closest<HTMLFormElement>('form#reply');136- if (!form || !me) return;284+ const form = (e.target as Element).closest<HTMLFormElement>('form#reply, form.linecmt');285+ const box = form?.closest<HTMLElement>('[data-line-form]');286+ const post = box ? lines()?.dataset : form?.dataset;287+ if (!form || !post || !me) return;137288 e.preventDefault();138- const box = form.querySelector('textarea')!;139- const text = box.value;289+ const field = form.querySelector('textarea')!;290+ const text = field.value;140291 if (!text.trim()) return say(form, 'Write a comment first.', true);292+ const where = box?.dataset.lineForm;141293 busy(form, true);142294 say(form, 'Sending…');143295 try {144- const r = await send(form.dataset.post + 'comment', { body: text, at: form.dataset.at ?? '' });145- box.value = '';146- draft('review', '');147- sent.push({ kind: 'comment', n: r.comment, text });296+ const r = await send(post.post + 'comment', { body: text, at: post.at ?? '', ...(where ? { line: where } : {}) });297+ field.value = '';298+ draft(where ? `line ${where}` : 'review', '');299+ if (box) closeForm(box);300+ sent.push({ kind: 'comment', n: r.comment, text, line: where });148301 // forge comment's own note, when the review moved on since this page was built.149- say(form, r.message.split('\n').slice(1).join(' '));302+ say(document.getElementById('reply'), r.message.split('\n').slice(1).join(' '));150303 drawSent();151304 watch();152305 } catch (err) {153- say(form, err instanceof Refused ? err.message : "Couldn't reach the forge; your text is still here.", true);306+ say(form, failed(err), true);154307 } finally {155308 busy(form, false);156309 }157310 });158311 159- document.addEventListener('click', async (e) => {160- const b = (e.target as Element).closest<HTMLButtonElement>('form#reply [data-approve]');161- const form = b?.closest<HTMLFormElement>('form');162- if (!b || !form || !me) return;163- busy(form, true);164- say(form, 'Approving…');165- try {166- const r = await send(form.dataset.post + 'approve', { at: form.dataset.at ?? '' });167- // Approved before: nothing changes, so nothing to wait for.168- if (/^you already approved/.test(r.message)) say(form, 'You approved this tree already.');169- else {170- sent.push({ kind: 'approve' });171- drawSent();172- watch();173- }174- b.textContent = 'Approved';175- busy(form, false);176- b.disabled = true;177- } catch (err) {178- say(form, err instanceof Refused ? err.message : "Couldn't reach the forge.", true);179- busy(form, false);180- }181- });182-183312 // Signed out, or a server without the private server (npm run dev): the184313 // controls stay hidden.185- if (!document.getElementById('reply')) return;314+ if (!document.getElementById('reply') && !lines()) return;186315 fetch('/login/whoami', { cache: 'no-store', credentials: 'same-origin' })187316 .then((r) => (r.ok ? r.json() : null))188317 .then((w: { user?: string | null; csrf?: string } | null) => {@@ -7,6 +7,7 @@ pagination:
77 {% set rv = p.review %}88 {% block head %}{{ m.reviewMeta(rv, p.repo, p.title, p.repo.urls.reviewDiff(rv.id).href) }}{% endblock %}99 {% block scripts %}<script src="{{ assets.diffs }}" defer></script>10+<script src="{{ assets.discuss }}" defer></script>1011 {% endblock %}1112 {% block content %}1213 {% css %}@@ -21,11 +22,27 @@ pagination:
2122 .code:has(.cmts) { container-type: inline-size; }2223 .code .cmts { position: sticky; left: 0; width: 100cqw; padding: 8px 10px; background: var(--subtle); border-block: 1px solid var(--border); font: 15px/1.5 var(--sans); white-space: normal; }2324 .cmt + .cmt { margin-top: 8px; }25+/* A new line comment, signed in (discuss.ts): the + on the line under the26+ pointer (or the line last clicked) opens a form under it. */27+.commenting .code.diff .rows { position: relative; }28+.addline { position: absolute; z-index: 2; width: 20px; height: 17px; padding: 0; font: 600 13px/15px var(--sans); text-align: center; color: #fff; background: var(--accent); border-color: var(--accent); border-radius: 4px; }29+.addline:hover { background: var(--accent); filter: brightness(1.1); }30+.cmts .where { font: 12px/1.6 var(--mono); color: var(--muted); margin-bottom: 4px; overflow-wrap: anywhere; }31+.cmts textarea { display: block; width: 100%; min-height: 5em; resize: vertical; font: 14px/1.5 var(--sans); color: var(--fg); background: var(--bg); border: 1px solid var(--border); border-radius: var(--radius); padding: 8px 10px; }32+.cmts textarea:focus { outline: none; border-color: var(--accent); box-shadow: 0 0 0 1px var(--accent); }33+.cmts .reply-bar { display: flex; flex-wrap: wrap; align-items: center; gap: 6px 8px; margin-top: 8px; font-size: 0.92em; }34+.cmts .reply-bar .grow { flex: 1; }35+.cmts .reply-bar button.go { background: var(--accent); border-color: var(--accent); color: #fff; }36+.cmts .reply-bar button:disabled { opacity: 0.6; cursor: default; }37+.cmts .reply-status.bad { color: var(--fail); white-space: pre-line; }38+.cmt.pending { border-style: dashed; }39+.cmt.pending .text { padding: 8px 12px; white-space: pre-wrap; overflow-wrap: anywhere; color: var(--muted); }2440 {% endcss %}2541 <div class="meta-head">2642 <h1><a href="{{ p.repo.urls.review(rv.id).href }}" data-key="u">{{ rv.title }}</a> <span class="desc">#{{ rv.id }}</span></h1>2743 <div class="byline">Combined diff of {{ rv.layer.length | plural('commit') }} on <span class="mono">{{ rv.branch }}</span></div>2844 </div>45+<p class="desc" id="line-comments" data-post="{{ p.repo.urls.review(rv.id).href }}" data-at="{{ rv.tip }}" hidden></p>2946 <div class="changes"><span class="tools"><span class="count" data-viewed="{{ p.repo.slug }}" hidden></span><button type="button" data-wrap>Wrap lines</button></span>3047 {{ rv.combined | diffstatHtml | safe }}3148 </div>@@ -0,0 +1,26 @@
1+# The combined diff names each file's path (data-path) for line comments2+# from the page, which discuss.js offers to a user signed in: a hidden hint3+# that knows where to post and the tip the diff shows. A deleted file has4+# no new side to comment on, so no path. Diffs elsewhere carry none.5+. "$(dirname "$0")/../lib.sh"6+user dami7+repo dami/greet8+clone dami dami/greet "$T/w/dami"9+at "$T/w/dami" commit "first" greet.sh10+at "$T/w/dami" commit "old" old.txt11+ok push_as dami "$T/w/dami" origin main12+at "$T/w/dami" git switch -q -c dami/feature13+at "$T/w/dami" commit "feature" greet.sh14+at "$T/w/dami" git rm -q old.txt15+at "$T/w/dami" git commit -q -m "drop old"16+ok push_as dami "$T/w/dami" origin dami/feature17+build18+tip=$(git --git-dir="$FORGE_ROOT/dami/greet.git" rev-parse dami/feature)19+20+d=$FORGE_WWW/dami/greet/reviews/1/diff/index.html21+has "$d" "<p class=\"desc\" id=\"line-comments\" data-post=\"/dami/greet/reviews/1/\" data-at=\"$tip\" hidden></p>"22+has "$d" '<section class="file jump" id="f0" data-path="greet.sh">'23+has "$d" '<section class="file jump" id="f1"><header'24+has "$d" '<script src="/assets/discuss\.[A-Za-z0-9_-]{10}\.js" defer>'25+lacks "$FORGE_WWW/dami/greet/reviews/1/index.html" 'data-path='26+lacks "$FORGE_WWW/dami/greet/commit/$tip/index.html" 'data-path='4/4 Document commenting and approving from the web
Comments and approvals gets a part of its own on the web: what a page shows to someone signed in and why it starts hidden, which revision a post from a page belongs to, the endpoints and their fields, how a post is authenticated (a session with its CSRF token and Origin, or a Bearer token), the 404 for a private repo you can't read, the rate limit, and that the server runs the same command as ssh. Reading pages says how to use the controls, the overview and What runs mention them, and the item leaves Not yet.
| M | README.md | 66 | +++++++++++++++++++++++++----- |
@@ -22,10 +22,11 @@ It adds what stagit doesn't have, borrowing from [args.io](https://args.io):
2222 - **Revisions are recorded per push.** Force-pushing a review records a new2323 revision. Its page shows what changed: the diff between the two tips, or2424 after a rebase, each commit paired with its earlier version (`git range-diff`).25-- **Discussion from the terminal.** `forge comment` and `forge approve` add26- comments (on a line or the whole review) and approvals, which the review27- page shows. Approvals are keyed by tree like CI, and `main` can require one28- from someone other than the author. `forge land` checks both and25+- **Discussion from the terminal, or the page.** `forge comment` and26+ `forge approve` add comments (on a line or the whole review) and27+ approvals, which the review page shows; signed in, the page adds them28+ too. Approvals are keyed by tree like CI, and `main` can require one from29+ someone other than the author. `forge land` checks both and2930 fast-forwards.3031 - **CI runs on your machine.** `forge attest` runs `.forge/ci` in a clean3132 worktree. It signs the result with the SSH key you push with and stores it@@ -88,6 +89,7 @@ forge release publish -> refs/releases/<tag>
8889 11ty build -> /srv/git/.forge/www (static)8990 events.jsonl -> live page updates9091 forge comment, approve ─── ssh ──────────────> comment, approve: next to the review log92+review page, signed in ─── HTTPS ────────────> the same, through the private server9193 ```9294 9395 ### What runs@@ -98,7 +100,7 @@ forge comment, approve ─── ssh ──────────────>
98100 | Caddy | Serves `.forge/www` as plain files, read-only HTTPS clones (`git http-backend` through fcgiwrap; nothing else inside a bare repo is served; a private repo takes a token), and `go get` meta tags; proxies `/events/`, every path `www` has no file for (to the private server) and, with a pages domain, sites. |99101 | `forge build` | Runs 11ty after each push, in the background and one repo at a time, then swaps the new pages into `www/`. Unchanged pages are hard links to the last build's. First it adds the push's commits to the repo's commit-graph (see [Cleaning up](#cleaning-up)). |100102 | `lib/events-server.ts` | Tails `.forge/events.jsonl`, which hooks and builds append to, and streams it as server-sent events at `/events/<owner>/<repo>/`. Open pages say who pushed and swap in fresh content when the build finishes. Without it pages still work; their status line says offline. A private repo streams only to its readers, signed in. |101-| `lib/private-server.ts` | Serves private repos' pages to their readers, signed in with a passkey or holding a token, the sign-in pages (`/login/`, `/private/`), and the 404 of every page that isn't there; answers Caddy whether a token may clone a private repo over HTTPS (see [Private pages](#private-pages)). |103+| `lib/private-server.ts` | Serves private repos' pages to their readers, signed in with a passkey or holding a token, the sign-in pages (`/login/`, `/private/`), and the 404 of every page that isn't there; takes comments and approvals posted from review pages (see [Comments and approvals](#comments-and-approvals)); answers Caddy whether a token may clone a private repo over HTTPS (see [Private pages](#private-pages)). |102104 | `lib/pages-server.ts` | Only with a pages domain. Serves each site on its own hostname the way Cloudflare Pages does (`_headers`, `_redirects`, pretty URLs, the nearest `404.html` or a single-page-app fallback) and redirects old `/sites/...` paths there. Runs as the caddy user, since deploys are untrusted content. |103105 | `forge backup` | Daily, from a loop in `start.sh`. Snapshots the forge with restic into a bucket, then checks the snapshots and test-restores the new one (see [Backups and moving hosts](#backups-and-moving-hosts)). |104106 | `forge mirror` | After builds, in the background: syncs the forge's own pages to a bucket with rclone, for the edge to serve while the forge is down. |@@ -291,8 +293,10 @@ from `FORGE_BASE_URL` (just the path if that isn't set).
291293 292294 ### Comments and approvals293295 294-Both are written from the terminal over the forge's ssh account, and the295-pages rebuild in the background, so an open review page updates live.296+Both are written from the terminal over the forge's ssh account, or from297+the review page once you are signed in (see [On the web](#on-the-web)298+below), and the pages rebuild in the background, so an open review page299+updates live.296300 297301 ```sh298302 forge reviews # open and draft reviews: CI, approvals@@ -312,7 +316,7 @@ ssh git@git.example.com land dami/greet 7 # the forge lands it itself; no clon
312316 older comments show as outdated and still quote the line they were about.313317 Line comments on the current revision also appear under their line in the314318 combined diff. A comment made from a checkout belongs to the revision315- checked out.319+ checked out, and one from a page to the revision the page shows.316320 - **Approvals** are lines in the review log, `approve <ts> <user> <sha>317321 <tree>`. Like CI results they are keyed by tree, so rewording or318322 reordering commits keeps them; if the tree changes, the page shows@@ -346,6 +350,39 @@ land a review pushed a moment ago. The ssh commands live in
346350 `git-shell-commands/`; on an existing server, run `forge server init` again to347351 add them (the container does that on every boot).348352 353+#### On the web354+355+Signed in with a passkey (see [Private pages](#private-pages)), a review356+page ends in a comment box and an Approve button, and the combined diff357+puts a + beside the line under the pointer, which opens a box for a358+comment on that line. Pages are static files, the same for everyone, so359+these start hidden and do nothing without JavaScript: a script asks360+`/login/whoami` (never cached) who is signed in and shows them to a user,361+without Approve for the review's author. A page posts the tip it shows,362+like `forge comment` from a checkout: if the review has moved on since it363+was built, a comment goes to the revision you read, and an approval is364+refused. Until the rebuilt page arrives, a sent comment shows as pending,365+as typed; the build renders and sanitizes its markdown, as for any other.366+367+The posts go to the private server, `POST368+/<owner>/<repo>/reviews/<id>/comment` (fields `body`, `at`, `line` as369+`<path>:<n>`) and `.../approve` (`at`), as a form or JSON, and it runs370+`forge shell-cmd comment|approve` as you with the message on stdin, the371+command ssh runs. So a comment file, an approve line, the rules and the372+messages are the same either way, byte for byte. A session's post must373+come from the forge's own origin and carry its CSRF token (the page gets it374+from `/login/whoami`); a token works as `Authorization: Bearer` without375+one, for curl, and a token for one repo only there. Basic auth doesn't376+post, since browsers send it by themselves. A private repo you can't read377+answers 404, the same as a missing one. Each user may post 20 times a378+minute. Caddy sends these posts to the private server before it looks in379+`www`, so public reviews, whose pages are plain files, take them too.380+381+```sh382+curl -H "Authorization: Bearer $token" -d 'body=Looks good' -d 'line=greet.sh:12' \383+ https://git.example.com/dami/greet/reviews/7/comment384+```385+349386 ### CI attestations, and what they prove350387 351388 `forge attest [--stack]` checks out each commit into a temporary worktree and@@ -1220,6 +1257,15 @@ viewed". Viewed marks are kept in the browser's localStorage, keyed by repo,
12201257 path and a hash of that file's diff, so they survive a force-push that leaves12211258 the file alone and clear when the file changes.12221259 1260+Above the diffs, the discussion: approvals, then comments oldest first. Signed1261+in, it ends in a box to comment in, with Approve beside it (Ctrl or Cmd and1262+Enter sends), and on the combined diff a + follows the pointer down the lines:1263+click it for a box under that line (on a phone, tap the line, then the +). A1264+comment you send waits on the page, as you typed it, until the rebuilt page1265+swaps in a moment later with it rendered; what you are typing outlasts that1266+swap, and a reload. Signed out, or without JavaScript, the page shows none of1267+this.1268+12231269 "What changed since rN" compares a revision with the one before. If both were12241270 pushed on the same base, it is the diff between the two tips. If the branch12251271 was rebased, that diff would include everything that landed meanwhile, so@@ -1321,12 +1367,10 @@ may be missing needs an explicit check or `or ''`.
13211367 | `/<owner>/<repo>/releases/download/<tag>/<file>` | a release file; `latest/` for the newest release's |13221368 | `/<owner>/<repo>/insights/` | charts from the lake; `insights.json` and Parquet files next to it |13231369 | `/insights/` | the same for the whole forge, and a table by repo |1324-| `/login/` | sign in with a passkey, add one (`#enroll=<code>`), sign out |1370+| `/login/` | sign in with a passkey, add one (`#enroll=<code>`), sign out; `whoami` says who is signed in, for review pages |13251371 | `/private/` | the private repos you can read |13261372 | `/sites/<owner>/<repo>/<name>/` | a deployed site |13271373 13281374 ## Not yet13291375 1330-- Commenting and approving from the web pages; for now both come from the1331- terminal.13321376 - Bugs or issues in the args.io style.