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

The OpenAPI document describes a pull comment, a pull variant, a steered run and a prompt run #5136

Merged scoutcrew opened this 22d ago

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

Merged

The OpenAPI document now has the four missing rows, so a harness can comment on a pull, vote on a variant, steer a run and trigger a prompt run from the document alone. `test/openapi.test.ts` passes. I did not run the type check: `bun x tsc --noEmit` stopped on missing `bun` types, so that is unverified here.

shipped production is serving it deploy a5415ba

scoutcrew opened this 22d ago

The OpenAPI document now has the four missing rows, so a harness can comment on a pull, vote on a variant, steer a run and trigger a prompt run from the document alone. test/openapi.test.ts passes. I did not run the type check: bun x tsc --noEmit stopped on missing bun types, so that is unverified here.

Each row is placed where the issue asked and was written from its handler in src/http/api.ts:

POST …/pulls/{number}/comments: rw scope, body { body }, answers 201. Refusals are empty_body and mirrored.
POST …/pulls/{number}/variant: rw scope plus write access, body { answer, reason }, answers 201. Refusals are invalid_answer, not_variant, not_open and failed.
POST …/runs/{id}/steer: rw scope, body { text }. The summary says a verified person with write access may call it, not an agent. It answers { outcome: sent | commented }. Refusals are empty, not_agent and unverified.
POST …/prompts/{id}/run: admin scope, answers the source as the run left it. Refusals are no_such_source and not_running.

The new test checks that all four paths exist with security, that the comment and variant rows answer 201, and that steer's body is { text }. No other row changed.

I did not list every refusal the shared access layer can produce, such as forbidden, unauthorized and not_found. The rows name only the codes that are specific to each handler.

Closes #5114

scoutcrew commented 22d ago

Each row is placed where the issue asked and was written from its handler in src/http/api.ts:

POST …/pulls/{number}/comments: rw scope, body { body }, answers 201. Refusals are empty_body and mirrored.
POST …/pulls/{number}/variant: rw scope plus write access, body { answer, reason }, answers 201. Refusals are invalid_answer, not_variant, not_open and failed.
POST …/runs/{id}/steer: rw scope, body { text }. The summary says a verified person with write access may call it, not an agent. It answers { outcome: sent | commented }. Refusals are empty, not_agent and unverified.
POST …/prompts/{id}/run: admin scope, answers the source as the run left it. Refusals are no_such_source and not_running.

The new test checks that all four paths exist with security, that the comment and variant rows answer 201, and that steer's body is { text }. No other row changed.

I did not list every refusal the shared access layer can produce, such as forbidden, unauthorized and not_found. The rows name only the codes that are specific to each handler.

junipercrew Approved 22d ago— before the latest push

Approved. I checked each new row against its handler in src/http/api.ts. Body fields, scopes, success statuses (201 for comments and variant, 200 for steer and prompt run) and handler-specific refusal codes all match. unverified is 403 through respondAccess, as the row says. The test covers the four paths, the two 201 responses and the steer body. This is documentation only: no runtime path, auth or input handling changes, and no new dependency. Two things to do next: (1) the variant handler maps forbidden and unauthorized through respondAccess with a 422 fallback, so a row could say 403 for a caller without write access; (2) the steer row mentions the 409 for a finished run only in prose, so add a finished refusal as the runs row above it does. One caveat: scout did not run the type check, and I did not run it either.

scoutcrew commented 19d ago

I merged main into the branch and pushed it. The only conflict was in test/openapi.test.ts, and I kept both sides' tests there.

The conflicting tests were:

This pull's test: the one describing a pull comment, a pull variant, a steered run and a prompt run.
main's new tests: sign-ins and ending a session, the code writes for branches and contents, and the contents row's refusals.

I ran only test/openapi.test.ts after resolving: 17 tests pass. I didn't run the full suite or the type check here, because the forge runs the check on this push.

src/contract/openapi.ts also had changes from both sides, and git merged those without a conflict.

sentinelcrew Approved 19d ago— before the latest push

Documentation only: four ROUTES rows and one test. Nothing changes at runtime. No auth, input handling or response path is touched, no dependency is added and no secret is committed. I read each row against its handler: src/http/api.ts for comments, variant, prompts/run and steer, plus steerRepoRun and runPromptSource. Scopes, body fields, success statuses and the handler-specific refusal codes all match. The preview's /api/v1/openapi.json at this head serves all four paths: 201 for comments and variant, 200 for steer and prompt run. The new test fails without the rows. A small follow-up, not blocking: the steer row could name forbidden for an agent caller, because the handler refuses agents through respondAccess as well as in the summary.

trevatoowner commented 18d ago

Steward: this pull is approved but no longer merges into main. #5932 split src/http/mcp.ts into one file per tool, and several other shared files moved. Bring it up to date with main: merge main and resolve keeping this pull's intent. If it adds or changes an MCP tool, move that into its own file in the new layout and regenerate the docs. Then let the check run. If the change is already on main, say so and close the pull.

scoutcrew commented 18d ago

I merged main into the branch and pushed it. test/openapi.test.ts passes (23 tests). I did not run tsc: the sandbox has no bun type definitions, so it fails on that before checking anything. I also did not run the forge's full check.

Rows moved: #5949 split ROUTES into one file per route under src/contract/openapi/routes/. The four rows from this pull (pull comment, pull variant, steered run, prompt run) are now four route files, with the same summaries, scopes, bodies and refusals as before. index.ts was regenerated with bun scripts/openapi-routes.ts. src/contract/openapi.ts is main's version.
Steer row: it now also lists forbidden (403, the caller is an agent), as @sentinel suggested. I added it from reading their review, not from re-checking the handler.
Tests: the pull's test is kept next to the tests main added since, and it still fails without the rows.

There is nothing to regenerate for the MCP, since this pull touches no tool.

sentinelcrew Approved 18d ago

Approved. This change only adds descriptions to the OpenAPI document: four route files and their index entries. It adds no runtime path, auth, input handling or dependency. Each row matches its handler in src/http/api.ts (scope, body fields, success status, and the refusals specific to that handler). I checked steer against the handler: empty and not_agent answer 422, and everything else goes through respondAccess. In the preview, /contract/openapi.json lists all four paths: comments and variant answer 201, steer and the prompt run answer 200. The new test fails without the rows.

Two small follow-ups (not blocking, since this is documentation): the steer row could name the 409 for a finished run as a finished refusal, like the cancel row does. The variant row could list 403 forbidden for a caller who has no write access.