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 under app/Actions/ that a .stx file 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/DashboardFeedAction was 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.ts opens 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.
    • reviewerLoadFor had 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 with await 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.

    ShowPullRequestAction and ReviewQueueAction close 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, currentUser deliberately 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 a ros_ token to its owner and stops there, and in authorizeRepository, 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 thread and stack, matching the table in AGENTS.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-requests because 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_page beside productId is 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 same authorizeRepository, which accepts either.

  • The OpenAPI document is generated from the actions (./buddy generate:openapi) and published, so it cannot drift from what the server accepts

    GET /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 ETag from 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:openapi in the same step as the document, and checked against a fresh render by tests/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.storeActions/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.
    • ruleToSchema duck-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 wanted 12.

    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.

  • 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 grepped routes/. 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/:id and 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.routes is a single table shared by the whole process, and route.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, naming GET /, GET /features and GET /{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 what routes/ 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. operations carries 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 through ShowWorkflowRunAction, ListWorkflowRunsAction and ShowJobLogAction; and stopping one is CancelWorkflowRunAction, behind workflow: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_at alone 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 a COUNT, which on a large table costs more than the page did.

  • ETag and If-None-Match on 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's updated_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-Match is 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-After that is true

    app/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. And Retry-After is 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.

    remaining counts 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 - a not_found answered with a 200 only shows up when somebody's retry loop never terminates.

    fix is in words and is not a restatement of the rule: "must match ^[a-z]" says what failed, not what to do. Retry-After goes 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 same parseDiff. 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. The path filter and file-level paging keep the size sane.

    It is also the first reader behind an ETag, and building it made conditional async - the expensive readers are precisely the ones worth a 304, and a synchronous signature pushed every one of them into wrapping a promise in a Response body 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.

    SubmitReviewAction takes comments: [...] 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 CommentOnCodeAction writes. 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. And pr: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. notifyProgramsOnly is a separate function rather than a flag on notify, because the difference is not a setting.

    tests/unit/webhook-coverage.test.ts writes 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 events column 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 it

    An operations table, app/Api/operations.ts for the shape, app/Api/progress.ts for the worker half, and GET /api/operations/{id} plus POST /api/operations/{id}/cancel.

    The problem it removes: an endpoint that starts work and answers 202 Accepted has 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.

    queued and running are separate states because conflating them hides the failure worth seeing: work accepted and never picked up. cancelled is separate from failed because 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 running with cancelling: true until it does. Reporting cancelled immediately 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: tokenIdFor read request._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 an access_tokens id. 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 users row 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.ts has 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 bot pill 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 by approvalsSatisfied.

    A machine's objection is not affected. changes_requested from 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.ts already documents for the merge settings, one column later: findRepositoryByPath selects an explicit list, so a new column reads as undefined and 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_tokens and a token_usage_windows counter, spent by app/Api/token-limits.ts before 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-After in the header and the body and goes through apiError, 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 integer and 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.
  • 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 one recordAudit every caller uses, read off the request by tokenIdFor so 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 at POST /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_output is 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:3000 is 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_diff calls /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 isError with 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. isError puts 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 with bun build --compile, over the shared implementations in app/Cli/commands.ts. The same commands are registered on buddy for working inside this repository - two front ends, one implementation, because buddy refuses 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 checkout fetches into pr/<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.

    review reads its body from a file or standard input. A review is prose; --body is 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

    security on macOS, secret-tool on Linux, cmdkey on Windows - each the utility that platform already ships, none a dependency this project adds. A token in ~/.reviewos/token is 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_TOKEN is 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. login checks a token against /api/user before 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, an ETag, and fields.
    • 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 same buildStack the interface does.
    • GET /api/user - the smallest useful thing an API can offer, and the one login needs to check a token before storing it.