12 - The API and agents
On this page 5
A coding agent is a contributor now. It opens pull requests, it reads diffs, it answers review comments, and it does all of that through a token rather than a browser. Every forge in this space was designed for a person with a mouse and grew an API afterwards, which is why the API is always the second-class surface: it lags the interface, it exposes a different vocabulary, and the credential it accepts is either too weak to do the job or so broad that handing it to a script is a decision nobody wants to sign off on.
That last one is the concrete complaint that started this phase. On GitHub, packages:read cannot be
granted by a fine-grained token at all, so an agent that needs to read one package is issued a classic
token carrying every scope on the account. The rule that fixes it lives in
phase 1; this phase is the rest of the surface that has to be
right for the rule to be worth anything.
The bar: anything a person can do in the interface, a token can do through the API, in the same vocabulary, and it is discoverable without reading the source.
Parity
-
Every interface action has an API equivalent, because both call the same action in
app/Actions/. Where the interface reaches somewhere the API cannot, that is a bug filed against this list, not a design decision.Checked mechanically now, in
tests/unit/api-parity.test.ts: every module underapp/Actions/that a.stxfile imports must also be reachable from a registered route. That is the honest shadow of the rule - it cannot tell whether an endpoint's answer is as complete as a page's, but it catches the shape every violation so far has had, which is a page reaching into server code no route reaches.It found three on its first run, none of which anybody had noticed:
Feed/DashboardFeedActionwas registered nowhere. Written, keyset paginated, with a docstring explaining that it exists to serve the second page of the feed - and unreachable. The page rendered its own first page from the same module and nothing served the rest.- The webhook delivery log had no endpoint.
deliveries.tsopens by saying it is "pure over plain rows, so the page and the endpoint cannot disagree", and there was no endpoint. It answers one question - "you never called my endpoint" - so an agent debugging its own webhook had to read HTML for it. reviewerLoadForhad one caller and it was a view. The same shape as the review queue, which is the violation that prompted this box. It travels with the suggestions now rather than getting a route of its own: who has worked on these files and how much each is carrying are two halves of one decision, and an agent making two calls would sometimes act on a stale half.
The checker itself had a bug worth recording, because it was the same class: it matched only
from '...'and this codebase reaches for dependencies withawait import(...)as often - so it reported two endpoints as missing that had just been written. A checker that cries wolf is a checker somebody deletes.Two of those bugs are fixed. Building the MCP tools meant checking that each one pointed at a real route, and two of five did not: reading one pull request, and the review queue.
reviewQueue()existed with exactly one caller,resources/views/reviews.stx- the interface reached the most useful question a reviewing agent has, what should I look at, and the API could not, so an agent had to scrape the page or reconstruct the ordering itself.ShowPullRequestActionandReviewQueueActionclose them, over the same functions the pages call. Writing a client against the surface is the only reliable way to find gaps like these: they are invisible from inside, because the page works.The third was the whole credential. This project's own fine-grained tokens - the ones phase 1 exists to provide - authenticated git over HTTP and the browse endpoints, and every JSON endpoint answered them 401. They are a different credential in a different table from the framework's,
currentUserdeliberately refuses to resolve one (answering "who" would drop the reach and the grants), and nothing else picked them up. So the credential this project is built around could not call this project's API, and the entire e2e suite missed it because it issues framework tokens, which work.Found by building per-token rate limits on top of it and watching them meter nothing.
Fixed in
app/Middleware/Auth.ts, which now resolves aros_token to its owner and stops there, and inauthorizeRepository, which applies the two halves that need the repository in hand: the reach (a repository outside it reads as missing, exactly as it does to a stranger, because "your token cannot see that one" confirms it exists) and the grants (a missing scope is named, because here the repository is visible and the holder may do it, so the scope is the one thing that turns a 403 into a fix). A token narrows and never widens, and the user's own permission is checked first, so removing somebody from a repository revokes their token's reach into it with nothing to remember.A token of ours that fails to authenticate is now a 401 rather than a fall-through to anonymous. An expired token reading a private repository used to be answered 404, which sends somebody looking for a repository that was there yesterday.
-
One vocabulary. The API says
repository,pull request,review threadandstack, matching the table inAGENTS.md, rather than a second set of names for the same things.tests/unit/api-vocabulary.test.ts, reading the generated document rather than the source, because the document is what a client sees and a name that never reaches it is an internal one this rule does not govern.A drift guard rather than proof of correctness: the vocabulary is right today, and what this makes expensive is the day somebody adds
/api/repos/merge-requestsbecause that is what they called it at their last job. Each banned word is paired with the term that wins - a bare blocklist reads as arbitrary, and the next person deletes the line rather than renaming their endpoint.It also pins parameter names as snake case.
per_pagebesideproductIdis two conventions in one document, and a client written against one fails on the other in a way that reads as a typo rather than as a rule.The one deliberate pair is pinned so nobody "fixes" it in either direction:
/{owner}/{repository}in a browsable path, because a URL is the most visible copy there is and the domain table says never "repo" there, and?repo=as the compact parameter every forge's clients already type. Both resolve through the sameauthorizeRepository, which accepts either. -
The OpenAPI document is generated from the actions (
./buddy generate:openapi) and published, so it cannot drift from what the server acceptsGET /api/openapi.json. Generating it is not publishing it: a document only somebody with the source can find is a document for the group that did not need it, and the bar is discoverability without reading the source.Public and unauthenticated, deliberately. It describes which endpoints exist, not what is in them, and every one of them still answers a stranger exactly as it did. An API that hides its own shape is an API whose clients are written by guessing.
Served from the file rather than regenerated per request - generation imports every action, which is a second of work for a document that changes on deploy - with an
ETagfrom the file's size and mtime rather than its contents, because hashing three quarters of a megabyte to answer "has it changed since you deployed" costs more than sending it.A test asserts the document describes the endpoint serving it. That circle closing is the cheapest possible proof it was generated from the route table that is actually running.
-
A generated TypeScript client, built from that document rather than maintained by hand
storage/framework/api/client.ts, generated by./buddy generate:openapiin the same step as the document, and checked against a fresh render bytests/unit/openapi-coverage.test.ts. A separate command is how the two drift; a client nothing verifies is a hand-maintained client with extra steps.The output has no imports and no dependencies - one file to copy into a browser app, a Bun script, or another repository. A generated client that needs a runtime package pins its consumer to this framework's release cadence, which is the coupling a client exists to remove. Nothing in it throws for a 4xx either: an API where every refusal is an exception forces a try/catch around normal control flow, and that catch swallows the real failures too.
Built upstream, in
@stacksjs/api, because none of it is specific to this project. Doing so uncovered two defects that made the document describe almost nothing:- The generator found an action by title-casing the route name into a guessed path
(
posts.store→Actions/PostsStoreAction). Every route not following that convention was documented as accepting no input at all. The registry has known the real handler string all along; it simply was not reported. ruleToSchemaduck-typed_type,_min,_email,_required- none of which the validation library has. So every documented field came out as{ type: 'string' }, unconstrained and never required. A document that types everything as a string is worse than one that says nothing: code generated from it compiles and then sends"12"where the server wanted12.
Both shipped in Stacks 0.70.325 and 0.70.327, with a GET's fields now becoming query parameters rather than a request body - a GET with a body is a request most intermediaries drop silently.
- The generator found an action by title-casing the route name into a guessed path
(
-
Tests that walk the route table and fail on a route with no OpenAPI operation
tests/unit/openapi-coverage.test.ts. A route that exists and is undocumented fails the discoverability bar quietly: it works, nothing errors, and the only people who know about it are the ones who greppedroutes/. The generated client will not have it either.It found one on its first run -
GET /api/search, added without re-running the generator - which is precisely the failure mode it exists for. It also found a trap in itself: the route table says/queries/:idand the document says/queries/{id}, so comparing them verbatim reports a missing route for all 173 parameterised paths. The tempting fix is excluding them, which would silently stop testing a quarter of the API; the right one is normalising the two spellings.Verified by deleting an operation from the document and watching it fail by name, rather than trusting that a green test was testing anything.
It answered differently depending on what ran before it, which is the one thing a parity check cannot do.
route.routesis a single table shared by the whole process, androute.serve()adds the 65 file-based views to it - so run alone the test passed, and run after any e2e file that boots a server it failed, namingGET /,GET /featuresandGET /{owner}/{repository}as undocumented API. They are pages. They answer HTML, no generated client wants them, and the document is right not to carry them.Resetting the table does not work:
importRoutes()is guarded against running twice, so clearing it and re-importing leaves it empty and the test passes by describing nothing. It reads the table from a child process now (tests/helpers/declaredRoutes.ts), which is the honest definition anyway - the API surface is whatroutes/declares, not what happens to be registered by the time this file runs. -
Long-running resources, including CI workflow runs, expose their state machine and control operations through the same public actions used by the interface and CLI. No UI-only pause, retry, cancellation, log, or approval path.
The operations half held from the start.
operationscarries a real state machine -queued,running,succeeded,failed,cancelled- and both reading one and cancelling it are public actions (ShowOperationAction,CancelOperationAction), with no separate path for the interface. Failed jobs gained the other control operation with the administration surface: a retry that moves the row rather than copying it.The CI clause waited for CI, and closed with it. A workflow run's states are declared in one place (
app/Actions/Workflow/states.ts) and the transitions are checked there rather than at each caller; the run, its jobs, its steps and its log are read throughShowWorkflowRunAction,ListWorkflowRunsActionandShowJobLogAction; and stopping one isCancelWorkflowRunAction, behindworkflow:cancel.The run screen's cancel button posts an ordinary form to that endpoint rather than to a route of its own, and the action answers a browser with a redirect back to the run and a program with the row. That is the whole point of the clause: a control the interface has and the API does not is how a product grows a second, undocumented way to change its own state. The button is shown on
workflow:cancel- the ability the action enforces, not the level it currently sits at - so what is offered and what is allowed cannot drift apart.The parity check above is what keeps a UI-only path from appearing later, since one would show up as a view reaching a module no route reaches.
Built for a program, not a person with curl
-
Cursor pagination with a stable total ordering. Offset pagination over a table people are actively writing to silently skips and repeats rows, and a paginating agent will hit it.
app/Api/cursor.ts. The part worth stating is that the ordering has to be total:created_atalone is not, so two rows written in the same millisecond straddle a page boundary and one is never returned - the same failure offset has, which would make the rewrite pointless. Every ordering ends in the primary key. The caller asks for one row more than the page and it is never returned; it answers "is there more" without aCOUNT, which on a large table costs more than the page did. -
ETagandIf-None-Matchon read endpoints. An agent that polls should be cheap to serve, so the honest answer to polling is to make it free rather than to forbid it.app/Api/etag.ts. Tags come from cheap facts - a row'supdated_at, a count, a head sha - never from the rendered body, because a tag hashed from the response saves the transfer and nothing else, and serving a pull request means reading its comments, reviews, checks and diff stat. Weak (W/) because that is the true claim: the bytes are not identical a second later, the resource is.If-None-Matchis a list and is compared entry by entry, which is the bug that makes conditional requests silently never match. -
Idempotency keys on anything that creates. Agents retry on timeout, and the current behavior of every forge is to create the comment twice.
app/Api/idempotency.ts. Four outcomes, and the two unhappy ones matter as much as the replay: a recycled key carrying a different body is refused rather than replayed, because returning the first response for a second request means the second was never created and the client is told it succeeded - silent, and worse than the duplicate; and a second request arriving while the first is still running is held, because two retries racing is exactly what a timeout produces.Keys are scoped to the token and the endpoint. A key is chosen by the client and two clients will eventually choose the same one - a bad UUID seed, or the literal
1- and unscoped, one agent's retry returns another agent's response, which is a disclosure rather than a duplicate. -
Rate limits that are documented, returned in headers on every response rather than only on the rejection, per token rather than per account, and paired with a
Retry-Afterthat is trueapp/Api/rate-limit.ts. All three properties are the complaint about how this is usually done. A client that only learns its budget when it runs out cannot pace itself, only recover. A shared per-account bucket means the first bad retry loop takes everything down with it - and the first bad loop is never malice, it is a retry with no backoff. AndRetry-Afteris the window's real remaining time rather than a fixed guess, because a client that trusts a guess and retries into another rejection learns to ignore the header.remainingcounts what is left after this request. Reporting the count before it means a client told it has 1 sends one more and is refused, which makes the number useless for the only thing it is for.Reads are generous and writes are tight: the design asks clients to poll and then makes polling free with
ETag, so punishing it would be incoherent - while a thousand reads are invisible and a thousand comments are somebody's afternoon. -
Errors that name the field and the fix, with a stable machine-readable code. "Validation failed" costs a retry loop that will never succeed.
app/Api/errors.ts. The code is a closed set so a client can exhaustively handle it, and adding one is a deliberate act that shows in review rather than a string typed at a call site. The status comes from the code rather than from the caller, so the two cannot disagree - anot_foundanswered with a 200 only shows up when somebody's retry loop never terminates.fixis in words and is not a restatement of the rule: "must match ^[a-z]" says what failed, not what to do.Retry-Aftergoes in the header and the body, because a client on a generic HTTP layer reads one and a client written against this API reads the other, and sending only one means half of them busy-loop. -
Partial responses: ask for the fields you need. A pull request list that always carries every body is expensive on both ends.
app/Api/fields.ts. Four hundred open pull requests with a paragraph each is a megabyte of prose to build a list of titles, and the caller throws every body away.wants()is the part that matters: a serializer narrowing after reading every column has saved the transfer and none of the work, and the expensive fields are exactly the ones a list does not need - body, labels, review state, check summary, each a query. Callers ask before running them.Every failure mode degrades rather than refuses, because this is an optional optimisation: no parameter means everything, an unknown field name is ignored rather than 422'd, and an identifier is added back when a caller drops it by accident.
-
A structured diff endpoint: hunks, ranges and line origins as JSON, from the same parser the review screen uses. Scraping the rendered diff should never be the only way to get one.
/api/repos/pulls/diff/structured, over the sameparseDiff. Not a second parser and not a second definition of what a hunk is: if the two ever disagreed, the diff a reviewer approved would not be the diff an agent read.Every line carries both line numbers even though one is always null. An agent commenting on line 40 of the new file needs the new-side number, and deriving it from a patch means counting hunk offsets - which is exactly the arithmetic that puts a comment two lines off.
Collected rather than streamed, which is the one place it differs from
/diff/rows. That streams because a browser can paint the first file while git writes the last; a JSON document has no such shape, so the choice was between collecting it and inventing a second streaming format nobody asked for. Thepathfilter and file-level paging keep the size sane.It is also the first reader behind an
ETag, and building it madeconditionalasync - the expensive readers are precisely the ones worth a 304, and a synchronous signature pushed every one of them into wrapping a promise in aResponsebody by hand, which loses the status and the headers. A test now pins that the builder is never called on a hit, because a builder that ran anyway would save the transfer and nothing else. -
Submit a whole review in one request: many comments plus a verdict, atomically. A review assembled by twelve round trips is twelve chances to leave half a review behind.
SubmitReviewActiontakescomments: [...]alongside the verdict, rather than a second endpoint. It is the browser flow turned inside out and exists for callers with no drafts to publish: the interface writes pending threads as the reviewer types, and an agent assembles its comments in memory with nowhere to put them until it submits.Every comment is validated before any is written, and all the problems come back at once. A caller sending twelve with two mistakes should learn about both - fixing the first otherwise only earns them the second error on the next attempt, and an agent doing that is an agent in a loop. A connection that drops mid-review leaves comments attached to a real review rather than orphans attached to nothing, because the review row is written first and the notification fires only after every comment lands.
Each comment becomes a thread of the same shape
CommentOnCodeActionwrites. A comment left by an agent and one left by a person have to be the same kind of object, or every reader of them grows a special case. -
Webhooks are the supported way to stay current (phase 5), and every event that changes something an agent cares about has one
Auditing the nine phase 5 shipped against what a program needs found the important one missing: the head moved. An agent that reviewed a change and hears nothing when the author pushes a fix has two options and both are bad - poll every open pull request forever, or never look again. A person has neither problem: they are told when somebody re-requests review, and they were going to open the page anyway.
So
pr:synchronized, from the push job, only when the sha genuinely changed and only after the row is updated - a receiver that immediately reads the pull request back must not see the old head. Andpr:ready_for_review, because nothing else about a draft changes when it becomes ready, so there is no push, comment or review request to observe.Both are webhooks and neither is a notification. Telling every reviewer about every push is how an inbox becomes something people filter, and the inbox is the channel that has to work when everything else does not.
notifyProgramsOnlyis a separate function rather than a flag onnotify, because the difference is not a setting.tests/unit/webhook-coverage.test.tswrites the claim down: which events a program needs and why, that each is dispatched rather than merely advertised (an advertised event that never fires is worse than an absent one - the client cannot tell), and that a wildcard picks up events added later.It immediately found a second defect. The
eventscolumn declared a JSON array and defaulted to["*"];subscribes()splits on commas. A webhook created with the column default subscribed to nothing and was silent forever, and its owner's only clue would have been that nothing ever arrived - indistinguishable from the endpoint being wrong. Only rows written through the endpoint, which stores the comma form, ever worked, which is exactly why it survived. The test now asserts against the model's own default so the two cannot drift apart again.Checks are still absent, deliberately: they are phase 9 and there is nothing to emit yet. The same reasoning as the MCP tool that is not there.
-
A consistent operation pattern for asynchronous work: create with an idempotency key, receive a resource and status URL, poll cheaply with
ETag, follow a cursor-based event or log stream, and cancel with the same token authority that created itAn
operationstable,app/Api/operations.tsfor the shape,app/Api/progress.tsfor the worker half, andGET /api/operations/{id}plusPOST /api/operations/{id}/cancel.The problem it removes: an endpoint that starts work and answers
202 Acceptedhas told the caller nothing they can act on. Did it start? Is it still going? Did it fail an hour ago? The usual answer is "poll the resource and infer", so every client writes a different inference and each is wrong differently - a mirror that has not moved is indistinguishable from a sync that never ran.queuedandrunningare separate states because conflating them hides the failure worth seeing: work accepted and never picked up.cancelledis separate fromfailedbecause a caller who asked to stop should not be told their work broke.Cancel needs the same token authority, not merely the same person. Two agents under one account must not be able to stop each other, which is a distinction that only started mattering when agents began holding tokens. A person cancelling from a session what their own token started is fine.
Cancelling is a request, not an act: the work is running elsewhere and notices at its next checkpoint, so the status keeps saying
runningwithcancelling: trueuntil it does. Reportingcancelledimmediately would be the one lie a status endpoint must not tell. Queued work stops at once, because there is no checkpoint to wait for.The idempotency key is scoped to the token, the kind and the subject. A key is chosen by the client and two clients will eventually choose the same one, so an unscoped lookup would join one caller's retry to another's work - a disclosure rather than a duplicate.
Mirror sync is the first user, and answering
{ queued: true }is what it used to do. The event or log stream is the one piece not built: nothing here produces a stream yet, and inventing one before phase 9's workflow logs would be guessing at its shape.Building it found a defect in what was already shipped:
tokenIdForreadrequest._currentAccessToken, which is the framework's token row from a different table with its own id space. Every audit entry written for a framework-authenticated request recorded somebody else's primary key as anaccess_tokensid. Silent, until a column referenced the real table and the foreign key refused it.
Agents as a first-class kind of contributor
Not a special case bolted on. A machine account is an account, subject to the same permission resolution, and the differences are the ones that genuinely matter.
-
Machine accounts, defined in phase 1, can be authors, reviewers and assignees like anyone else
True by construction - a machine account is a
usersrow and nothing in the permission path knows the difference - which is exactly why it needed a test rather than a reading. "It should work" and "it does" are different statements, and the gap between them is where an exclusion added somewhere for some other reason -WHERE machine_for_organization_id IS NULL- would live unnoticed.tests/e2e/machine-contributor.test.tshas one review a pull request, take a review request, take an assignment and comment, all under its own token rather than a person acting for it. That is the part that could not have worked a commit ago: it cannot sign in, so a token is the only way it acts at all, and this project's tokens could not call this API. -
Attribution is visible: a pull request opened by a machine account, or a review submitted by one, says so plainly in the interface. Not to shame it, but because a reader's standard for "somebody looked at this" depends on who looked.
A
botpill beside the author in the pull request header and beside each verdict in the review panel. Deliberately quiet - small, lowercase, the same weight as the text around it - because an agent is a contributor here and a red badge would read as a warning about the change rather than as a fact about who wrote it. -
An approval from a machine account counts toward required approvals only if the repository opts in. Default off, because the failure mode is a branch protected by a robot approving its own class of change.
repositories.count_machine_approvals, off by default, read byapprovalsSatisfied.A machine's objection is not affected.
changes_requestedfrom a machine account blocks exactly as anyone else's does, because the two directions are not symmetric: declining to count a robot's approval is cautious, and ignoring a robot's objection is the opposite. A repository that opted out has said it does not want a robot's yes to be the reason something merged, not that it wants a robot's no thrown away.An uncounted approval is reported rather than dropped, and the refusal says why. "1 more approval is required" on a pull request that visibly has one reads as a bug in the counting, and the reader's next move is to ask a colleague rather than to find the setting.
Wiring it found the trap
access.tsalready documents for the merge settings, one column later:findRepositoryByPathselects an explicit list, so a new column reads asundefinedand the setting is configurable and inert. The comment warning about it was three lines above the list. -
Per-token limits an owner can set: how many pull requests, comments or reviews an hour. The first bad agent loop is not malice, it is a retry with no backoff, and the repository should survive it.
Three columns on
access_tokensand atoken_usage_windowscounter, spent byapp/Api/token-limits.tsbefore anything is created. Three budgets rather than one, because the three cost different amounts of somebody's attention: forty comments an hour from a linting agent is a working configuration and forty pull requests an hour is not, and one number cannot express both.Null means the instance default, not unlimited. A column defaulting to no limit would leave every token issued before this existed unlimited forever, and those are exactly the ones running unattended. Zero is honoured as zero.
A table rather than the cache, because a budget that resets on deploy is not a budget - and a loop with no backoff outlives a deploy. Only reads are free; only creation is metered.
A refusal costs no write, so a client already being refused cannot extend its own lockout by retrying. The refusal carries
Retry-Afterin the header and the body and goes throughapiError, not a second error shape.Two bugs found by testing it end to end, both of which passed every unit test:
- The window column was an
integerand epoch milliseconds have not fitted in one since 1970. Every insert threw, the caller treats a counter failure as "allow", and the limit metered nothing while appearing to work. It is an ISO string now, and the catch logs: a failure that allows must at least say so. - Far worse, and the reason the feature could not work at all - see the parity note below.
- The window column was an
-
Everything a token did is in the audit log, attributable to the token and not only to the account behind it
audit_events.access_token_id, written through the onerecordAuditevery caller uses, read off the request bytokenIdForso no call site has to know where the router keeps it.Attribution to the account alone stops being enough once agents are contributors. One account can hold a personal token, a CI token and an agent's token at once, and "chris deleted the repository" is a different sentence from "the deploy token chris issued in March deleted the repository": the first sends somebody to ask Chris, the second sends them to revoke a credential.
Null for anything done in a browser, which is most of the log, and that absence is itself the signal - a session did it, not a token.
-
Repository rules can require that an agent-authored change carries a human approval before it merges, expressed as a rule rather than as a convention people remember
protected_branches.require_human_approval_for_agents, off by default, checked at merge alongside the approval count. "We always look at the bot's pull requests" is true for about three weeks, and the week it stops being true is the week nobody notices - because what changed is nobody's attention rather than any file.Distinct from
count_machine_approvals, which is about whose approval counts. This is about whose change needs one, and a repository can reasonably want both: an agent's review is worth counting, and an agent's own change still gets a person.One human approval, not all of them. The requirement is that somebody looked, and a rule demanding every approval be human would make an agent reviewer useless on exactly the branches most likely to have one.
MCP
The Model Context Protocol is how an agent gets tools, and Stacks already ships MCP support
(stacks-ai), so this is wiring rather than invention.
-
A ReviewOS MCP server exposing the review surface as tools: list what is waiting on me, read a pull request, read its diff by file, comment on a line, submit a review, read a check's output
app/Mcp/, reached atPOST /api/mcp. JSON-RPC 2.0 over HTTP rather than stdio, because this one is hosted: it runs alongside the instance and is reached over the network by whichever agent holds a token. A stdio server would have to run on the agent's machine with a credential sitting in its environment, which is the deployment this design exists to avoid.Five tools, not six.
read_check_outputis deliberately absent: checks are phase 9 and there is no endpoint behind it. Shipping the tool anyway would advertise a capability that answers 404, and a model handed a tool that always fails does not conclude the tool is broken, it concludes the task is impossible and gives up on the whole line of work. An absent tool is a capability an agent works around; a broken one is a capability it stops trusting. The sixth tool arrives with the endpoint.The tool list is curated rather than complete. A menu with two hundred entries costs the model context on every single call while making the six that matter harder to find, and everything absent is still reachable through the HTTP API.
-
Tools are scoped by the token that authenticates the connection, with no ambient authority. The agent gets exactly the token's permissions and nothing that leaks from the server process.
Every tool is an HTTP call to this instance's own public API carrying the token the connection authenticated with. The server process holds no credential, so "no ambient authority" is a property of the construction rather than a rule somebody has to keep obeying: there is no path through the dispatcher that reaches the API without the caller's token, because there is nothing else to reach it with.
The cost is a round trip to ourselves per call. Worth it. The alternative is calling the actions directly, which means re-deriving which token may do what, and a second implementation of that is the one bug in this codebase that would matter most.
The origin comes from the incoming request rather than from configuration, so an instance behind a proxy or on a custom port reaches itself without being told where it lives.
http://localhost:3000is right until the first deployment that is not that, and then it is wrong only in production. -
Read tools return the structured diff, not HTML
read_pull_request_diffcalls/api/repos/pulls/diff/structured, the endpoint above. An agent handed HTML has to parse it back into hunks, which is a parser it should never have had to write. -
Self-hostable alongside the instance, and documented in the self-hosting guide
Self-hostable by construction: it is part of the instance, so running one is running the forge, with nothing extra to deploy and nothing extra to hand a credential to.
Documented in
docs/self-hosting.md, which now exists. The section is about the token rather than the server, because the token is the whole of the configuration - the server holds no credential of its own, so there is no second permission check to set up and none to get wrong. What it tells an operator is which scopes a reviewing agent actually needs, that reach makes unlisted repositories read as missing, and the two repository settings worth knowing before pointing an agent at a protected branch. -
Tests: a tool call against a repository the token cannot read fails the same way the API does
tests/e2e/mcp.test.ts, and the assertion is not against a hard-coded 404. It asks the API directly with the same token, then asks through the tool, and requires the second to carry the first's status - so the two cannot drift apart. If the API ever started answering 403, the test would notice that the tool had not.Not "fails similarly": it is the API failing, relayed. There is no second permission check here to disagree with the first one, because there is no first one here at all.
A refusal comes back as
isErrorwith the API's own words rather than as a JSON-RPC error. The distinction decides who hears it: a JSON-RPC error is a protocol failure and most clients surface it to the operator, so a model that asked about a repository it cannot read would be told nothing and would try again.isErrorputs the refusal in front of the model, which is the only way it learns to stop asking.
Command line
-
A CLI for the operations that belong in a terminal: open a pull request from the current branch, check out someone else's, see the stack, submit a review from a file
cli/reviewos.ts, built withbun build --compile, over the shared implementations inapp/Cli/commands.ts. The same commands are registered onbuddyfor working inside this repository - two front ends, one implementation, becausebuddyrefuses to run outside a Stacks checkout and the person this is for has never seen this codebase.Exactly those four, and deliberately not more. A CLI that mirrors the whole product is a second product to maintain, and the browser is better at most of it. These are the four where a terminal wins because the answer is already in the working copy or the input is already in a file.
The instance and the repository are read from the git remote rather than asked for, in all three spellings the same remote has -
https://host/o/r.git,git@host:o/r,ssh://git@host/o/r- so somebody who cloned over ssh and somebody who cloned over https are working on the same repository as far as the CLI is concerned.pr checkoutfetches intopr/<n>rather than checking out the contributor's branch by name. Their branch may not exist locally, may exist and be something else, and is theirs: a reviewer who commits on it by accident has written on somebody else's branch.reviewreads its body from a file or standard input. A review is prose;--bodyis fine for one sentence and hostile for the paragraph a real review is, which is most of why the command exists. -
It authenticates with the same fine-grained tokens, and stores them in the OS keychain rather than a dotfile
securityon macOS,secret-toolon Linux,cmdkeyon Windows - each the utility that platform already ships, none a dependency this project adds. A token in~/.reviewos/tokenis readable by every process the user runs, ends up in backups, and survives in a synced home directory long after somebody thought they had removed it.REVIEWOS_TOKENis read first and stored nowhere, which is how CI supplies one: scoped to the job, injected by the runner's secret store, never written to a disk.There is deliberately no file fallback. A CLI that quietly writes a dotfile when the keychain is unavailable teaches its users that the keychain is optional, and the machines where it is unavailable are the shared ones.
loginchecks a token against/api/userbefore storing it, because finding out on the next command is a worse first experience than one round trip. -
It is a client of the public API only. If the CLI needs an endpoint that does not exist, the endpoint gets built, which keeps parity honest.
Held absolutely: nothing under
app/Cli/imports a model or touches the database. The value is entirely in it being absolute - the moment one command reaches into the database because it is quicker, the CLI stops being a check on whether the API is complete and becomes a second implementation of the product.It immediately paid for itself. Three endpoints did not exist and now do:
GET /api/repos/pulls- listing was reachable only by rendering a page. Built on the phase's own primitives rather than three new ones: a cursor, anETag, andfields.GET /api/repos/pulls/stack- a stack was visible only as a navigation strip, so a client would have had to fetch every pull request and rebuild the chain, which is a second answer to what lands first. It calls the samebuildStackthe interface does.GET /api/user- the smallest useful thing an API can offer, and the oneloginneeds to check a token before storing it.