Status and acceptance criteria¶
What is built, what is next, and the checklist the finished agent has to satisfy. The criteria come from the feature issue that opened the project; they are reproduced here so they survive the issue being closed.
๐ Where things stand¶
| Component | State |
|---|---|
| Classifier / allowlist / mention parsing | implemented, unit tested |
Config loader (config.yaml) |
implemented, unit tested |
| Poller (async, ETag conditional requests) | implemented, unit tested |
| Payload mapping (REST dicts โ trigger models) | implemented, unit tested |
| SQLite store (watermarks, ETags, migrations) | implemented, unit tested |
| Queue and per-PR lease | implemented, unit tested |
| Bootstrap checks for a new host | implemented, unit tested |
Daemon loop calling poll_once() on a schedule |
implemented, unit tested |
| Budget governor (windows, ladder, reserve-then-settle) | implemented, unit tested |
| Workspace (fetch, checkout, merge-base diff, teardown) | implemented, unit tested |
| Budget layer 2 (path exclusions, pre-flight estimate) | implemented, unit tested |
Engine seam (ReviewEngine, Capabilities, FakeEngine) |
implemented, unit tested |
| Review worker (claim โ run โ settle), and its supervisor | implemented, unit tested |
Engine adapter (CliEngine + ClaudeCliEngine) |
implemented, unit tested, wired |
| Publisher (๐, head re-check, one comment per PR) | implemented, unit tested |
| Circuit breaker and calibration decay | implemented, unit tested |
| Per-pull-request pacer (review interval, daily cap) and the trigger fold | implemented, unit tested |
pr-review-agent <noun> <verb> command tree, config generate |
implemented, unit tested |
systemd user unit and service install |
implemented, unit tested |
Logging: level, auto/text/json shape, journald priorities |
implemented, unit tested |
Multi-repo: repo-scoped queue and watermarks, shared budget authority, service install --instance |
implemented, unit tested |
Review skill (skill install), and the prompt read from it |
implemented, unit tested |
| Incremental review since the last round, by content | implemented, unit tested |
| Coverage footer naming what the exclusions withheld | implemented, unit tested |
| Mandatory effort, risk and merge assessment on every review | implemented, unit tested |
@claude describe and the description skill |
implemented, unit tested |
| Code-smell baseline and the format and test sweep items in the review contract | implemented, unit tested |
| Retention sweep | not started |
The ordering is deliberate: the budget governor lands before the review worker, so the spending rails exist before anything can spend.
๐งญ Next¶
- The last budget piece that needs a running engine: the ladder's 60 % rung. The circuit breaker has landed, so an over-estimated limit is no longer invisible: a usage-limit failure refuses every claim for five hours and decays the effective limits toward the real one. What it has not had is a sighting of the real error โ see the known gaps. Layer 3's wall clock has landed too, validated below the queue lease; its token ceiling did not, because every way to build one assumes an engine that reports tokens, and it belonged with the breaker. See BUDGET.md.
- Retention sweep. Purge content on merge; keep the ledger. It is
specified in terms of the
runstable the publisher added, andRunStore.purge_contentis already there waiting for a caller.
The queue now drains, and the agent can spend. The
review worker claims through the governor, checks
the pull request out, runs the configured adapter and settles the ledger.
"Nothing can spend" has stopped being structural and is now what the rails
enforce: the windows, the ladder, the pre-flight estimate, max_run_tokens
and worker.count. CLAUDE.md ยง5's rule โ nothing reaches an engine outside
the governor โ is doing work rather than describing a property the code had
for free, and a test pins it.
The publisher now makes the result visible: a ๐ at claim
time, a live head_sha re-check, and one ordinary comment per review. It can make no other kind of write, which is how
DESIGN.md's third mitigation stops
being a promise about code and becomes a property of it.
What is missing is anywhere for a finished pull request's content to go:
review bodies accumulate in runs until the retention sweep lands.
A second engine plus a shared conformance suite is deliberately last: the
seam is worth defining early and filling late. It will be another CLI โ
codex or opencode โ because every adapter is a
subprocess and no vendor SDK is
linked.
โ Acceptance criteria¶
- [x] Feature is accessible: the agent posts a review comment on a freshly
opened pull request from an allowlisted author, and on an allowlisted
maintainer's
@claudecomment, with a ๐ acknowledgement within 15 s of the trigger being seen. The original criterion said "a line-anchored review ... within 15 s of the trigger"; both halves were reworded when the publisher landed. Findings are posted in one comment rather than inline, because inline comments require the reviews endpoint and with it aneventfield โ see PUBLISHER.md. And the poll interval is 10โ600 s, so no acknowledgement can be within 15 s of a comment being written; the ๐ is what makes that latency imperceptible, which is the reason it was specified. - [x] A clean review posts a single "no issues found" comment. Since 1.3.0 each round posts its own comment rather than editing the last.
- [ ] Triggers are correctly scoped: no review on pushes to an existing
pull request, no auto-review of an unlisted contributor's pull request,
no trigger from a non-allowlisted account, no self-reply loop, no
duplicate review for an unchanged
head_sha, and no trigger from@claudeinside a code fence or a blockquote. - [ ] Budget limits are enforced: a synthetic concurrent load cannot breach
any configured window (done); daily pacing prevents the weekly
allowance being consumed in one day (done); the degradation ladder is
observed at 85 / 100 % (done) and at 60 % (with the engine);
per_contributor_pctbounds one contributor's share of the week (done); the pacer bounds how often and how many times one pull request is reviewed, deferring rather than dropping what it refuses (done); path exclusions keep a vendored-only change from being refused on size, and a pre-flight estimate overmax_run_tokensrefuses a run and releases its reservation (done); a review wall clock above the queue lease fails startup (done); per-run ceilings terminate an over-budget review (with the breaker). - [ ] Plan lockout is prevented: with
reviewer_share_pctconfigured, agent usage never exceeds its share of the session or weekly window (done), and a usage-limit error trips the breaker and decays the calibrated estimate (done โ against a detector whose markers are still unconfirmed; see the known gaps). - [x]
budget.enabled: falsetakes effect without a restart, and so doespublish.dry_run: true. - [x] Every posted comment is traceable to a ledger row recording engine,
model, mode, token usage and
usage_confidence:runs.dedupe_keyjoins the queue row and the ledger rows that paid for it, and theledger_openindex keeps at most one of those rows open at a time, so a settle can never be ambiguous about which reservation it closes. - [ ] Operates entirely outbound: no inbound port opened on the host, verified end to end from the target server.
- [ ] Retention behaves as specified: review content is purged after a pull
request merges, ledger metrics survive the purge, and the
max_age_daysbackstop fires for long-open pull requests. - [ ] Test coverage: unit tests for mention parsing (fenced code, inline
code, blockquotes), allowlist id-vs-login matching, dedupe key generation
per trigger kind, rolling-window arithmetic, and the reserve-then-settle
governor under concurrency (done); integration tests against recorded
GitHub API fixtures (done โ
tests/test_integration.pyreplays a recorded/pullslisting, comments page and/pulls/{n}throughDaemon.run_once, from fixturesscripts/record_fixtures.pyre-records); an engine conformance suite run against every adapter using a fixture pull request with seeded defects (the suite exists intests/test_engine.py; it runs againstFakeEnginealone until there is an adapter to add, andtests/test_cli_engine_live.pycovers the one real adapter when it is run by hand). - [ ] Documentation: configuration reference, deployment and firewall prerequisites, billing-mode guidance, and an operator runbook covering the kill switch and the retention policy.
The first three lines of the test-coverage item, and the trigger-scoping
criterion, are met today by tests/test_mention.py,
tests/test_allowlist.py and tests/test_classifier.py.
๐ญ Known gaps¶
Small things that are known and not yet done, so they are not rediscovered as bugs:
- The primary GitHub rate limit carries
x-ratelimit-resetbut noRetry-After, so the client raises rather than sleeping to the reset. See POLLER.md. - Review content accumulates in
runsand nothing purges it yet.RunStore.purge_contentexists and has no caller; the retention sweep is the next component. - Nothing validates the configured token limits. They are the operator's guess at a quota the plan does not publish. The circuit breaker now catches an over-estimate and decays the effective limits toward the real one, but it only learns by hitting the wall โ every trip is a lockout the operator could have avoided by guessing lower to begin with. Set them conservatively low. See BUDGET.md.
- Worktrees stranded by the relative-
cache_dirbug are not cleaned up by code. Before the fix, git wrote each run directory inside the mirror, andgit worktree prunewill not remove a directory that still exists. An operator who ran an affected version deletes.cache/repos/*.git/.cache/by hand, once. It cannot recur now that the path is absolute, which is why there is no code for it. claim()'sadmithook is optional, so "nothing spends outside the governor" is held by a test rather than by the type system:tests/test_worker_claim.py::test_a_claim_is_never_taken_without_an_admit_predicateasserts the hook the worker passes. The worker is the only caller that claims.worker.countneeds a restart to take effect.SIGHUPswaps only the budget section, and the workers are built once at startup.- A reopened pull request does not recover the trigger that was abandoned
while it was closed. The worker now refuses a merged or closed pull
request before the checkout,
which is what stopped the agent paying for reviews nobody would read. What
it cannot do is bring the row back afterwards: every trigger is gated on a
timestamp that has already passed, and the abandoned row's dedupe key
blocks a second enqueue. A fresh
@claudecomment after the reopen works, and re-triggering on reopen was considered and declined โ it would mean a fourth watched endpoint or a per-pull-request poll, which is the cost POLLER.md exists to refuse.