Commit graph

3 commits

Author SHA1 Message Date
Willy Tarreau
bff33b4e32 DEV: patchbot: update: support replacing a whole note blob (setnotes)
Notes are append-only on the wire, which makes concurrent edits
conflict-free but leaves no way to revise or clean up a note from the
page: fixing a note requires a hand-edit of the storage file. This adds
the replacement directive that the design had reserved:

    <cid> setnotes <hash> <replacement text>

Unlike the other directives, a replacement must carry a token of the
base it was computed from, or it could silently destroy a concurrent
update (one reviewer's append landing between another's read and
replace). The token is the SDBM hash (8 hex chars) of the note blob
the client based its edit on: the server only applies the replacement
if it still matches the stored blob (empty-string hash for a commit
without notes). On mismatch the directive is dropped, the line is left
exactly as found, and a "conflict <cid>" line is emitted in the
response before the resulting lines so that the client can point the
user at what needs manual reconciliation; other directives from the
same POST are still applied, and nothing is written nor committed when
everything conflicted. SDBM is trivially computed on both sides (a
concurrency token, not a security feature, and JS crypto is unavailable
on plain http anyway), and its small multiplier keeps the whole hash
computation exact in awk's double-precision arithmetic, which is
precisely why it was chosen over wider-multiplier hashes.

An empty replacement deletes the notes (and the line if no state is
left), finally allowing obsolete notes to be removed without editing
the file by hand. Replacements are capped to 4000 chars instead of the
500-char append cap, since a coalesced blob may legitimately have grown
beyond a single addition.
2026-07-07 14:45:32 +02:00
Willy Tarreau
13d465a61a DEV: patchbot: update: return the stored overlay as JSON on GET
This adds the read side of the review persistence CGI: GET
update.cgi?branch=X.Y now returns the current overlay for that branch
as a JSON array of {"cid", "state", "notes"} objects with absent fields
omitted; a missing or empty file yields "[]". The raw storage format
never travels: the notes are unescaped by the parser and JSON-escaped
on output, so the client can JSON.parse() the response and insert the
notes via textContent directly. Unparseable lines or fields are
silently skipped as everywhere else.

Reads are lockless: the atomic rename on the write side guarantees
that the file is always a complete valid version. The response carries
Cache-Control: no-store so that a browser never reuses a stale overlay
on refresh.
2026-07-07 14:45:32 +02:00
Willy Tarreau
b232b554f2 DEV: patchbot: update: add an awk backend to persist review edits
The backport review page keeps the human edits (verdict overrides and
notes) only in the loaded DOM: they are lost on reload and never shared
between reviewers. This adds the server side of the shared persistence
design: update.awk, a GNU awk CGI script which stores these edits into
one file per major branch (e.g. "3.5") inside a dedicated git
repository, one line per touched commit:

    <commit_id> [state <n|u|w|y>] [notes "<quoted notes>"]

The overlay only ever stores human edits keyed on the commit id; the AI
verdict and explanation stay in the generated HTML. Commit ids are
length-agnostic and matched by symmetric prefix (first match wins), so
the current 8-char pipeline and a future 12-char one both work without
any migration.

POST applies line-oriented directives ("<cid> state <n|u|w|y|revert>",
"<cid> notes <text to append>"), none of which carries a base value:
states are last-write-wins and notes are append-only (capped to 500
chars per push and sanitised so that no newline may ever enter a stored
line), which keeps concurrent edits conflict-free. Broken directives,
fields or lines are silently ignored, never fatal, and lines not being
modified are preserved byte-for-byte so that admin hand-edits survive.
Writers are serialised by a mkdir lock at an obvious place (<repo>/lock)
with PID-gated crash takeover via atomic rename, the file itself is
replaced by an atomic rename from a temp file inside the lock dir, and
every resulting state is committed to git, which acts as the event log
(git blame/log -L provide the full history). A crash at any point leaves
at worst a stale lock (reclaimed on the next write) or a valid but
uncommitted tree (folded into the next commit), never a broken file.

The takeover races are covered: the staleness decision and the takeover
rename are not one atomic operation, so the thief verifies after the
rename, discarding the stolen dir only if it still carries the pid that
was judged dead and renaming it back in place untouched otherwise; the
victim redoes the whole locked cycle from a fresh read when its final
rename fails, since nothing was applied to the branch file yet; a writer
finding its own pid in the lock adopts it as stale; and the release only
removes the lock after checking that it still contains our own pid.

A few awk specifics are worth noting: external commands (git, mkdir,
mv) go through /bin/sh, so everything interpolated into a command line
is shell-quoted (single-quote is escaped and the argument placed inside
single quotes); the -b (bytes) flag keeps all string operations byte-
based regardless of the locale; and a manual argv parsing due to gawk
silently consumes a leading "-r" argument, ignoring ours.

Also note that gawk uses a file cache for getline() and co, which opens
lots of traps so we need to be extremely careful about properly closing
files if we want to check for changes (e.g. lock's pid file).

Finally, writing through a redirection whose target cannot be opened is
a fatal awk error terminating the script without even a response, so the
writes into the (stealable) lock dir are arranged to resist a takeover:
the pid is written through the shell, where a vanished dir is a plain
command failure, and the temp file is opened the very instant the lock
is acquired, its descriptor surviving a later theft. The update.cgi
wrapper considers any >0 return code as a failure and returns a generic
error as it will indicate that the awk script itself couldn't produce
a valid response.

For now, only the POST ("save changes") action is implemented.
2026-07-07 14:45:32 +02:00