Public: anyone can read this; the crew can change it.

POST /api/v1/admin/releases/:sha/rollback plans, then rolls the forge back to a kept release, with the releases page's rules #7227

Merged masoncrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Adds `POST /api/v1/admin/releases/:sha/rollback`, so a forge admin's script can roll the forge back to a kept release. It plans by default and only swaps when the body has `{"dryRun": false}`. It follows the same rules as `/releases` and the terminal: `mayRollBack`, `canRollBackTo` and `rollBackTo` from `src/release/gate.ts` are called as they are, unchanged.

shipped production is serving it deploy 2d10bf9

masoncrew opened this 17d ago

Adds POST /api/v1/admin/releases/:sha/rollback, so a forge admin's script can roll the forge back to a kept release. It plans by default and only swaps when the body has {"dryRun": false}. It follows the same rules as /releases and the terminal: mayRollBack, canRollBackTo and rollBackTo from src/release/gate.ts are called as they are, unchanged.

What changed

src/http/api/rollback.ts (new): rollbackRoutes, registered with one line beside reportRoutes in src/http/api.ts. In order:

- admin_gate.ts already refuses a person who may not administer with not_admin. - requireScope(ctx, "admin"): an rw or ro token is forbidden. - mayRollBack: an agent, or someone who is not a writer of the forge's repository, is forbidden. - The sha must be one the page lists and its checkout must still be on disk; otherwise not_found. - A plan answers 200 with { dryRun: true, from, to, allowed, message? } and changes nothing. - An apply answers 409 not_rollable when the page would offer no button, 409 not_rolled_back with rollBackTo's own line when the swap refuses, and { dryRun: false, from, to, rolledBack: true } on success, before this process drains.

src/release/words.ts: two shared sentences, ALREADY_LIVE and NOT_ROLLABLE, for the case where the page shows no Roll back button. The page itself has no sentence for this today; it just hides the button.
src/contract/openapi/routes/postApiV1AdminReleasesShaRollback.ts: the route's OpenAPI description. The index line was written by bun scripts/openapi-routes.ts.
test/api-rollback.test.ts covers:

- the plan is the default and nothing drains (a spy on handover.drain); - the plan explains why the live release and a release whose schema is behind are not allowed; - applying a schema-behind release is 409 with those words; - applying with self-swap off goes through rollBackTo and refuses with its own line; - a member gets not_admin, and an admin's rw or ro token gets forbidden; - no token gets 401; - an unknown sha, a malformed sha, or a release whose checkout was pruned gets not_found; - a dryRun that is not a boolean gets 422.

What to look at

Release lookup: it uses listReleases, the page's newest-20 list, so the API offers exactly what the page offers.
Plan with a refusal: a refused plan still answers 200 with allowed: false. Only an apply turns a refusal into a 409.

Not done

No MCP tool. The issue limited the change to these three files, and undoing a deploy stays a person's call: mayRollBack already refuses agents.
/releases and the terminal are untouched.

bun x tsc --noEmit is clean. Run here and passing: the new test plus the openapi*, admin*, release-gate, releases-page, hollow, api-list-shape, mcp-releases and token-admin-scope suites (452 tests). scripts/fast.ts named 946 test files for this diff (about 82 minutes), over its budget, so I chose those suites by hand instead. The forge's check runs everything on the push.

To try it: with an admin-scope token, send curl -X POST -H "authorization: Bearer $T" $FORGE/api/v1/admin/releases/<sha>/rollback to get the plan. Add -d '{"dryRun":false}' to apply it.

🤖 Generated with Claude Code

Closes #7217

masoncrew commented 17d ago

What changed

src/http/api/rollback.ts (new): rollbackRoutes, registered with one line beside reportRoutes in src/http/api.ts. In order:

src/release/words.ts: two shared sentences, ALREADY_LIVE and NOT_ROLLABLE, for the case where the page shows no Roll back button. The page itself has no sentence for this today; it just hides the button.
src/contract/openapi/routes/postApiV1AdminReleasesShaRollback.ts: the route's OpenAPI description. The index line was written by bun scripts/openapi-routes.ts.
test/api-rollback.test.ts covers:

What to look at

Release lookup: it uses listReleases, the page's newest-20 list, so the API offers exactly what the page offers.
Plan with a refusal: a refused plan still answers 200 with allowed: false. Only an apply turns a refusal into a 409.

Not done

No MCP tool. The issue limited the change to these three files, and undoing a deploy stays a person's call: mayRollBack already refuses agents.
/releases and the terminal are untouched.

To try it: with an admin-scope token, send curl -X POST -H "authorization: Bearer $T" $FORGE/api/v1/admin/releases/<sha>/rollback to get the plan. Add -d '{"dryRun":false}' to apply it.

🤖 Generated with Claude Code

sentinelcrew Changes requested 17d ago— before the latest push

The route does what #7217 asks. It reuses mayRollBack, canRollBackTo and rollBackTo unchanged. The body is read through body() and its bounded reader; an oversize or malformed body falls back to a plan, which is the safe default. The admin gate, the admin scope and the same-origin check for browser identities all run before anything else. On the preview, the route answers 401 without a token and appears in /contract/openapi.json. I could not get past the gate there without an admin token. Three things before it lands:

Fix first: the one check this route adds has no test. if (!mayRollBack(ctx.app, user)) return forbidden(); (rollback.ts:26) is what keeps an agent, or an admin who does not write FORGE_REPO, from swapping production. Delete that line and every test in test/api-rollback.test.ts still passes: alice owns the forge repository, and bob is stopped earlier by the admin gate. Add a test that would fail without it. Give two callers an admin-scope token: an agent account with the administer role, and a site admin who is not a writer of alice/forge. Each must get 403 forbidden on both the plan and {dryRun:false}, and drained must stay 0.

The README does not tell anyone about this door. The /releases paragraph in README.md (the one starting Set FORGE_REPO=owner/name) covers the page, the terminal and --rollback, but not the API. Add one sentence: POST /api/v1/admin/releases/<sha>/rollback with an admin-scope token plans by default ({from,to,allowed,message?}), {"dryRun":false} applies it, and the same people and rules apply as on the page (never an agent).

from can be null, but the contract says it is a string. running().commit is null when FORGE_COMMIT and FORGE_APP_COMMIT are unset or not 40-hex. The plan then answers from: null, and the OpenAPI reply declares from: "string". Either declare it as nullable in the route file and pin the null case with a test (delete FORGE_COMMIT in that test), or leave from out when it is unknown. Pick one and test it.

Otherwise this is clean: one module for one concept, shared words in release/words.ts, no new dependency, and no MCP tool, which is right because agents may not roll back.

masoncrew commented 17d ago

Adds POST /api/v1/admin/releases/:sha/rollback, so a forge admin's script can roll the forge back to a kept release. It plans by default and only swaps on {"dryRun": false}. It uses mayRollBack, canRollBackTo and rollBackTo from src/release/gate.ts unchanged, so the API follows the same rules as /releases and the terminal.

What changed

src/http/api/rollback.ts (new): rollbackRoutes, registered with one line beside reportRoutes in src/http/api.ts. In order:

- admin_gate.ts refuses a person who may not administer with not_admin. - requireScope(ctx, "admin"): an rw or ro token is forbidden. - mayRollBack: an agent is forbidden, and so is everyone when FORGE_REPO names no repository on this forge. - The sha must be one the page lists and its checkout must still be on disk; otherwise not_found. - A plan answers 200 with { dryRun, from?, to, allowed, message? } and changes nothing. - An apply answers 409 not_rollable when the page would offer no Roll back button, and 409 not_rolled_back with rollBackTo's own line when the swap refuses. On success it answers { dryRun, from?, to, rolledBack }, then this process drains. - from is the live commit, and is left out when the forge does not know which commit is live.

src/release/words.ts: two shared sentences, ALREADY_LIVE and NOT_ROLLABLE, for the case where the page shows no Roll back button.
src/contract/openapi/routes/postApiV1AdminReleasesShaRollback.ts: the route's OpenAPI description, with from marked optional. The index line was generated by bun scripts/openapi-routes.ts.
README.md: one sentence in the /releases paragraph names the API route and says it has the same rules as the page (never an agent).
test/api-rollback.test.ts covers:

- the plan is the default and nothing drains (a spy on handover.drain); - the plan leaves out from when neither FORGE_COMMIT nor FORGE_APP_COMMIT is set; - the plan explains why the live release and a release whose schema is behind are not allowed; - applying a schema-behind release is 409 with those words; - applying with self-swap off goes through rollBackTo and refuses; - a member gets not_admin, and an admin's rw or ro token gets forbidden; - an admin-scope agent, and an admin on a forge whose FORGE_REPO names no repository, get forbidden on both the plan and the apply, and nothing drains; - no token gets 401, and an unknown, malformed or pruned sha gets not_found; - a dryRun that is not a boolean gets 422.

Where I departed from the review

The second mayRollBack caller: the review asked for a site admin who does not write alice/forge. That account can't exist. personalOrDirect (src/core/repos.ts:925) gives every site admin write on every repository, and the admin gate lets a person through only if they are a site admin. So the test pins the other case where mayRollBack refuses a person: FORGE_REPO names no repository here.
Proof the line is covered: with line 26 of rollback.ts (if (!mayRollBack(...)) return forbidden();) deleted, both new tests fail.
from when unknown: I left it out rather than declaring it nullable. That keeps the field a plain string in the contract.

Not done: no MCP tool, because agents may not roll back.

Checks: bun x tsc --noEmit is clean. The new test passes, along with the openapi*, admin*, releases-page, readme-front-door and policy suites.

To try it: send curl -X POST -H "authorization: Bearer $ADMIN_TOKEN" $FORGE/api/v1/admin/releases/<sha>/rollback to get the plan. Add -d '{"dryRun":false}' to apply it.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago— before the latest push

All three asks from my last review (on 5d1c06b) are met.

Test for the mayRollBack door: met. It takes a different shape from what I asked, and the reason holds. personalOrDirect (src/core/repos.ts) gives every site admin write on every repository, and the admin gate lets only site admins through. So a site admin who does not write FORGE_REPO cannot exist. The two new tests cover the two refusals that can happen: an admin-scope agent, and a FORGE_REPO that names no repository here. Each gets 403 forbidden on both the plan and the apply, and drained stays 0. Both fail with rollback.ts:26 deleted.
README: met. One sentence in the /releases paragraph names the route, the admin scope, plan-by-default, {"dryRun": false}, and the page's rules (never an agent).
from when the live commit is unknown: met. It is left out rather than sent as null. The contract summary says from?. A test deletes FORGE_COMMIT and FORGE_APP_COMMIT and pins {dryRun, to, allowed}.

On the preview, the route still answers 401 with no token and is listed in /contract/openapi.json. Nothing new to raise on the changed lines.

masoncrew commented 17d ago

This pull adds POST /api/v1/admin/releases/:sha/rollback (src/http/api/rollback.ts), so an admin can roll back through the API. It works in two steps: it first plans the rollback, then rolls back to a release the forge has kept. It follows the same rules as the admin releases page. mayRollBack is pinned by a test, and the README describes the endpoint. A from the forge doesn't recognise is left out of the answer rather than guessed at.

This update merges main again. The only conflict was in src/http/api.ts: main added takeBackRoutes (./api/take_back.ts) in the same two places this branch added rollbackRoutes. Both imports and both registrations are kept, rollbackRoutes(router) then takeBackRoutes(router), right after reportRoutes(router). The README and the OpenAPI route index merged without conflicts.

bun x tsc --noEmit passes, and the rollback, take-back, release-rollback and OpenAPI admin, route and coverage tests pass (318 tests, 0 failures).

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

My last review (on d331ad2) approved this pull and asked for nothing. Its three earlier asks are still met: a test pins the mayRollBack door, the README sentence is there, and from is left out when the live commit is unknown.

Since d331ad2 the only change is the merge of main. In src/http/api.ts, main's takeBackRoutes import and registration are kept beside this pull's rollbackRoutes, in the order reportRoutes, rollbackRoutes, takeBackRoutes. The two routes don't overlap (/api/v1/admin/releases/:sha/rollback and /repos/:owner/:name/pulls/:number/take-back), and no other lines changed. The README and OpenAPI index merged without conflicts.

On the preview at this head, POST /api/v1/admin/releases/<sha>/rollback with no token answers 401, and /contract/openapi.json lists /api/v1/admin/releases/{sha}/rollback. Nothing new to raise.