Watch a repo for commits pushed by anyone but me
The bot only ever saw pull requests, so a collaborator with write access who pushes straight to master was invisible: 18 of the last 50 commits on ten31-brand's master arrived that way, unreviewed and unannounced. Opt-in per repo (watch_pushes + [gitea] my_emails). Each poll cycle it pages commits newer than a stored cursor across every branch except ones with an open PR, keeps the commits whose git email isn't mine, posts a notice naming author, branch, commits and files, records them, and only then runs a claude -p assessment threaded under it. Identity comes from the git email because these commits aren't linked to Gitea accounts. The notice is deterministic Gitea data and the "outside the expected area" flag is computed from globs, so the thing you actually need never depends on a model call. Recording before the assessment is what stops a crash in those minutes from reposting the whole notice. Both prompts now put every attacker-authored field — titles, commit subjects, filenames, the diff — inside a nonce-tagged fence, so a forged terminator can't reach the instruction region. Folds in the ROADMAP item to bound two unbounded awaits and exit if the poll task dies, and adds the repo's first test: the watch policy against a real-shaped Gitea fixture, with 17 mutations confirmed to fail it.
This commit is contained in:
@@ -2,11 +2,12 @@
|
||||
|
||||
gitea-review-bot is a **thin Matrix bot that runs an AI PR-review-and-merge workflow for your Gitea
|
||||
repos**, one review room per repo. It is a *separate* bot from `matrix-bridge` (which turns Matrix
|
||||
messages into interactive Claude Code sessions): this one's single responsibility is **reviewing
|
||||
pull requests** — so every room it's in is a review room and there is no manual "ignore this room"
|
||||
messages into interactive Claude Code sessions): this one's single responsibility is **watching what
|
||||
lands in a repo** — so every room it's in is a review room and there is no manual "ignore this room"
|
||||
wiring. Invite it to a room, it auto-joins and maps the room to a repo by name, then for that repo
|
||||
it polls Gitea PRs, posts a headless `claude -p` review as a Matrix thread, and lets you merge/reject
|
||||
from the thread.
|
||||
from the thread. Opt-in per repo, it also watches **commits pushed by anyone but me** and posts an
|
||||
assessment of what they did (see the push watch in Commands).
|
||||
|
||||
> **Inbox check:** At session start, if `~/Projects/standards/INBOX.md` exists, scan it for items
|
||||
> tagged `(gitea-review-bot)` and surface them before proposing next steps; triage with `/triage`.
|
||||
@@ -64,6 +65,45 @@ flat JSON in a writable `state/` mount.
|
||||
because the bot merges with `force_merge: true` (proven on ten31-site PR #4 against this exact rule
|
||||
shape). Tell a collaborator that "1 approval required" is expected, not stuck. Post-merge publishing
|
||||
is opt-in per repo (`[repo.<name>].deploy_on_merge`); without it a merge merges and redacts, nothing more.
|
||||
- **Push watch (opt-in per repo):** notifies whenever someone who isn't me pushes commits, and posts a
|
||||
`claude -p` assessment of what they did. Turn on with `[repo.<name>].watch_pushes = true` plus
|
||||
`[gitea] my_emails` (my commit emails — anyone *not* on that list notifies, so an unrecognised
|
||||
address surfaces rather than slips through; these repos' commits aren't linked to Gitea accounts,
|
||||
the API returns `author: null`, so the **git email is the only identity that works**). Optional
|
||||
`[repo.<name>].expected_paths` globs mark the collaborator's expected territory; files outside it
|
||||
are flagged in the notice (deterministically, not by the model) and called out to the assessor.
|
||||
Shape: **notify first, assess second** — the notice posts immediately, the commits are recorded in
|
||||
`state/` *before* the assessment starts, and the assessment lands as a threaded reply. Scope is
|
||||
every branch *except* branches with an open PR; the default branch is walked first, so a commit
|
||||
reachable from several branches is reported as being on `master` rather than on some stale branch.
|
||||
On first run the watch arms at "now" (it never replays history into the room); to force a test,
|
||||
backdate `watch.since` in `state/rooms.json`.
|
||||
- **Why the ordering:** recording before the assessment is what stops a crash, a restart or a send
|
||||
error in those minutes from reposting the whole notice and re-running the assessment. The cost is
|
||||
the opposite failure — a crash mid-assessment leaves a bare "_Assessing…_" in the room with no
|
||||
retry. Preferred deliberately: a missing assessment is visible, a duplicate one is noise.
|
||||
- **What it says out loud rather than logging:** more than `WATCH_MAX_PAGES × WATCH_COMMIT_LIMIT`
|
||||
(150) commits arriving at once, and a commit whose file list Gitea didn't return (the path check
|
||||
is then incomplete, and it must never print the all-clear).
|
||||
- **Bounds:** ≤`WATCH_MAX_BATCHES` (2) assessments per repo per cycle, each ≤`[defaults]
|
||||
watch_timeout_seconds` (480s; a real two-commit run with one panel subagent measured 173s), so a
|
||||
busy push can hold the other rooms' polling for ~16 min worst case — the head-of-line cost ROADMAP
|
||||
Phase 3 tracks, now bounded rather than open-ended. Held-over batches keep their commits out of
|
||||
`seen` and pin the `since` cursor, so they go out next cycle; after `WATCH_GIVEUP_HOURS` (48) the
|
||||
bot stops pinning and logs what it gave up on, so an unsendable batch can't freeze the watch.
|
||||
- **Merging a PR banks its commits** (`remember_pr_commits`, including the merge commit Gitea
|
||||
creates), whether it was merged from the thread or in Gitea's web UI — otherwise those commits
|
||||
land on `master` and read as a fresh push of work the review already covered.
|
||||
- **What it is not:** the author check reads the git email, which anyone can set. It is an
|
||||
accountability aid for a cooperative collaborator, not an authentication boundary. And push
|
||||
notices are *not* redactable — `clear` only reaches PR threads, so they accumulate as a log.
|
||||
- **Tests:** `python3 tests/test_watch.py` — no pytest, no deps, stubs matrix-nio. Covers the push
|
||||
watch's *policy*: who gets reported, cross-branch dedupe, the `since` cursor, the path flag, and
|
||||
that every attacker-authored field lands inside the prompt's fence. 17 mutations of `src/bot.py`
|
||||
were each confirmed to flip it to FAIL (2026-08-21). What it does **not** cover is the orchestration
|
||||
inside `watch_repo` — the record-before-assess ordering, the per-cycle cap, held batches — because
|
||||
that lives in closures over the live clients; those are verified by exercising the bot in a real
|
||||
room, which is still how everything else here is verified.
|
||||
- **Prove the poll loop is alive** (silence is normal — the loop logs only on a review or an error, so
|
||||
a healthy bot can go days without a line). Sample the Spark's host socket table fast enough to catch
|
||||
a poll: `end=$((SECONDS+130)); while [ $SECONDS -lt $end ]; do case "$(cat /proc/net/tcp)" in *EE73*)
|
||||
@@ -82,14 +122,24 @@ flat JSON in a writable `state/` mount.
|
||||
merge/reject/deploy
|
||||
in-thread; whole-thread redaction on resolve (server-enumerated, restart-proof) — a merge always
|
||||
redacts (a failed post-merge deploy leaves a top-level warning, not an orphaned thread); `clear`
|
||||
redacts without touching the PR.
|
||||
redacts without touching the PR. Same loop, per repo, runs the opt-in **push watch**:
|
||||
`select_push_batches()` holds the whole policy (whose commits, already-seen, cross-branch dedupe) as
|
||||
a pure function with no I/O — the part worth testing — while `watch_repo()` does the fetching and
|
||||
`post_push_notice()` the notify-then-assess.
|
||||
- `config.example.toml` — homeserver, `[mac]` (ssh alias + the reused matrix-bridge wrapper paths),
|
||||
`[gitea]` (api_base/owner/verify_tls), `[defaults]`, optional `[repo.<name>]` deploy overrides.
|
||||
`[gitea]` (api_base/owner/verify_tls/my_emails), `[defaults]`, optional `[repo.<name>]` deploy +
|
||||
push-watch overrides.
|
||||
- `.env.example` — `MATRIX_*` + `GITEA_TOKEN` (real `.env` gitignored). The live token is scoped
|
||||
`write:repository` only: repo/PR calls work, but `/user` and anything needing `read:user` 403s.
|
||||
- `Dockerfile` · `docker-compose.yml` · `docker-entrypoint.sh` — the Spark container (generic image;
|
||||
secrets/config via read-only mounts; entrypoint writes `~/.ssh/config` for the Mac alias).
|
||||
- `state/` — gitignored runtime JSON (`rooms.json`: room map + agents + heads + thread roots).
|
||||
- `tests/` — `test_watch.py` + a sanitized five-branch Gitea fixture (identities replaced, history
|
||||
overlap preserved because that overlap is what the dedupe is for).
|
||||
- `state/` — gitignored runtime JSON (`rooms.json`: room map + agents + heads + thread roots, plus
|
||||
`watch: {since, seen[]}` per push-watched room — `since` is the Gitea `since` cursor, trailing the
|
||||
newest commit by 24h so a committer's clock skew can't hide a push, and clamped to now so a
|
||||
future-dated commit can't push the cursor into the future and silence the watch; `seen` is the last
|
||||
500 SHAs accounted for).
|
||||
|
||||
## Decisions
|
||||
|
||||
@@ -103,6 +153,17 @@ flat JSON in a writable `state/` mount.
|
||||
output + its own overall recommendation. Beat: bot-orchestrated separate `claude -p` runs (more
|
||||
deterministic but 3× the sessions + more bot code). Reopens if headless subagent spawning is flaky.
|
||||
- **Panel composition is per-room, set in chat** (onboarding message + `agents +/-`), not config.
|
||||
`design-checker` joined the panel with the push watch — it audits against a repo's own
|
||||
`design/DESIGN.md` + tokens, which is exactly the brand question ten31-brand's room asks.
|
||||
- **Push watch: notify first, assess second** (2026-08-21). The notice is a plain Gitea-derived
|
||||
message posted before any `claude -p` runs, and the path flag ("outside the expected area") is
|
||||
computed from globs, not asked of the model — so the thing you actually need (that someone pushed,
|
||||
and where) never depends on a model call that can time out, fail, or be wrong. The assessment is a
|
||||
threaded reply under it. Beat: one composed message after the assessment (prettier, but a 10-minute
|
||||
`claude -p` budget sits between a push and knowing about it).
|
||||
- **Watch scope: every branch except ones with an open PR.** A PR already gets the full review
|
||||
treatment, so re-notifying its commits is noise. Beat: master-only (misses a side-branch push that
|
||||
never opens a PR).
|
||||
- **Reuse matrix-bridge's Mac wrappers + Spark→Mac SSH key** (don't duplicate the seam).
|
||||
- **Auto-map by name:** room `<x>` → Gitea `<owner>/<x>` + `~/Projects/<x>`; mapping persists to
|
||||
`state/` (mirrors matrix-bridge D14). One room per repo.
|
||||
@@ -111,36 +172,54 @@ flat JSON in a writable `state/` mount.
|
||||
change), and serial per-room polling with a 10-min review budget makes any staleness threshold
|
||||
either too loose to be timely or noisy mid-review. Reopens only on an *observed* wedge **plus**
|
||||
confirmation the spark-control tile already renders Docker health. The cheaper half of the same
|
||||
goal is the ROADMAP fold-in: bound the unbounded awaits and die if the poll task dies, so
|
||||
`restart: unless-stopped` handles it.
|
||||
goal was the ROADMAP fold-in — **done 2026-08-21**: both unbounded awaits are bounded and the poll
|
||||
task's death now exits the process, so `restart: unless-stopped` handles it.
|
||||
|
||||
## Sovereignty
|
||||
|
||||
**Untrusted input.** A PR's title/description and a pushed commit's subjects, filenames and diff are
|
||||
written by someone else and go to `claude -p` running on the Mac, in the owner's checkout, under the
|
||||
owner's OAuth token. Both prompts put *every* such field inside one `UNTRUSTED DATA <nonce>` fence
|
||||
with a per-run random tag, so a forged terminator can't close it, and instruct the model to report
|
||||
rather than obey anything inside. That is the cheap half; the tool-policy half (an explicit read-only
|
||||
`--allowedTools` on that run) needs a matrix-bridge change and is the top ROADMAP item.
|
||||
|
||||
|
||||
Reviews send PR diffs to `claude -p` (the subscription), not a frontier API on payload data; that's
|
||||
acceptable for code review of these repos. Before pointing the bot at a repo with sensitive content,
|
||||
revisit this — local inference via Spark Control would be the path.
|
||||
|
||||
## Current state
|
||||
|
||||
**LIVE and in steady use**, now serving **two** review rooms. Verified 2026-08-18: the container has been
|
||||
up since 08-13 with zero restarts on `063e694`, and the poll loop is genuinely alive (two Gitea connections
|
||||
observed exactly 60s apart — see the liveness recipe in Commands). `master` is only docs ahead, so **no
|
||||
redeploy is pending**. No test suite: verification means exercising the bot in a real review room.
|
||||
**LIVE and in steady use**, serving **two** review rooms. Verified 2026-08-18: the container had been
|
||||
up since 08-13 with zero restarts, and the poll loop is genuinely alive (two Gitea connections exactly
|
||||
60s apart — see the liveness recipe in Commands).
|
||||
|
||||
**2026-08-21 — the collaborator push watch shipped.** `master` now carries code the Spark hasn't got
|
||||
yet, so **a redeploy is pending** (and the Spark's `config.toml` needs the new `[gitea] my_emails` +
|
||||
`[repo.ten31-brand]` keys, which are gitignored and live only there). Two independent reviewer passes
|
||||
ran over it; the first found the batch being recorded only *after* its assessment (a crash would have
|
||||
replayed the whole notice) and an all-clear line printable from a missing file list, the second found
|
||||
untrusted PR/commit metadata sitting above the prompt's fence. All fixed, each with a mutation that
|
||||
reproduces it.
|
||||
|
||||
- **ten31-site** — proven end-to-end on PR #4 (2026-06-28: review → `merge` + `yes` → force_merge →
|
||||
auto-publish → redacted). **PR #5 "Update index.html" is still open**, reviewed 2026-06-30, its thread
|
||||
live in the room and waiting on a `merge`/`reject`.
|
||||
live in the room and waiting on a `merge`/`reject`. Push watch is **off** here.
|
||||
- **ten31-brand** — onboarded 2026-08-18 to watch the interactive deck
|
||||
(`design/deck/interactive/index.html`). Panel is empty; no path filter (deliberate — the repo is small
|
||||
enough that every PR is worth seeing). It has had **zero PRs ever**: all work went straight to `master`,
|
||||
and the bot sees only pull requests. Collaborator `jonathan` has write access but is blocked from
|
||||
pushing to `master`, so his changes must arrive as PRs; the owner's own pushes still bypass the bot.
|
||||
(`design/deck/interactive/index.html`). It has had **zero PRs ever**, and collaborator `jonathan`
|
||||
**is not blocked from `master`** — an earlier note here said he was, but the Gitea API says 18 of the
|
||||
last 50 commits on `master` are his (`Jonathan Kirkwood <jonathan@ten31.vc>`, most recently
|
||||
2026-08-21), pushed directly. That gap is what the push watch closes. His three `jonathan/*` branches
|
||||
are all already merged into `master`. Every commit he has ever made touches exactly one file,
|
||||
`design/deck/interactive/index.html`, which is why `expected_paths` is that subtree.
|
||||
|
||||
**Next steps, in priority order:**
|
||||
1. **Fix the orphaned-thread prune** (`src/bot.py` ~L594): the poll loop drops a PR's thread root from
|
||||
1. **Deploy the push watch and watch one real notice land** (see the pending redeploy above).
|
||||
2. **Fix the orphaned-thread prune** (`src/bot.py` ~L594): the poll loop drops a PR's thread root from
|
||||
state when the PR leaves the open list *without redacting*, so a PR merged or closed in the Gitea web
|
||||
UI strands its thread and `clear` can no longer reach it. More likely to bite now with two rooms.
|
||||
2. **`/triage` the 3 untriaged inbox items** tagged for this repo — adjudication never reads the inbox, so
|
||||
3. **`/triage` the 3 untriaged inbox items** tagged for this repo — adjudication never reads the inbox, so
|
||||
their content is still unknown here.
|
||||
3. With a 2nd repo mapped, ROADMAP Phase 3's verify is unblocked: next time both rooms have open PRs at
|
||||
4. With a 2nd repo mapped, ROADMAP Phase 3's verify is unblocked: next time both rooms have open PRs at
|
||||
once, watch a cycle (`docker compose logs -f`) for a long review head-of-line-blocking the other room.
|
||||
|
||||
Reference in New Issue
Block a user