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

A refused REST call tells an agent what to do next: every 4xx JSON body has a `hint`, proven by a test over the OpenAPI routes #10956

Merged opened this

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

Merged

Every 401, 403, 404, 409 and 422 JSON error answer from `/api/v1` now carries a `hint`, one fixed plain sentence on what to do next. The new test and the checks I ran pass; I did not run the full suite or the tests the diff reaches, and I did not call `keep_plan`.

shipped production is serving it deploy befa93c

opened this

Every 401, 403, 404, 409 and 422 JSON error answer from /api/v1 now carries a hint, one fixed plain sentence on what to do next. The new test and the checks I ran pass; I did not run the full suite or the tests the diff reaches, and I did not call keep_plan.

What changed

  • src/http/respond.ts: json() now calls a new withHint() on every error body, which covers apiError, notFound, unauthorized, forbidden and the sites that build json({error…}) themselves.

- A hint a site already wrote is kept. - no_route gets its own sentence pointing at GET /api/v1/openapi.json. - Hints are fixed text and never echo request text. not_found reads the same for a missing and a private resource.

  • src/http/mcp.ts: the unauthenticated POST /mcp 401 builds its own Response rather than going through json(), so it gets the hint written in directly.
  • test/api-hints.test.ts: walks every OpenAPI route (anonymous and with a read-only token), sends a bad call to each, and fails if a JSON error answer with status 401/403/404/409/422 has a missing or empty hint. It also checks that the 404 hint is identical for missing and private, and that other statuses are left alone.
  • docs/MCP.md: a short section names the field.

Tests run: test/api-hints.test.ts, test/api-errors.test.ts, test/api-reference.test.ts, test/openapi.test.ts, test/mcp-size.test.ts (ceilings untouched), test/read-api.test.ts and the first twelve test/mcp*.test.ts files. scripts/gate.ts and tsc --noEmit came back clean. scripts/fast.ts said the diff reaches 1650 of 2609 test files, so I picked the covering ones myself.

Not done

  • src/http/server.ts is untouched: it was marked hot, and its few hand-built 401/403 responses (for example the one at line 714) still carry no hint.
  • src/web/api_reference.ts is untouched: I named the field in docs/MCP.md instead.
  • Pre-push hook: it demanded a "Try:" commit before it would let the push through, so the branch ends with an empty commit saying how to try it.

To try it: curl -s $URL/api/v1/repos/x/y/z with no token returns {error, message, hint}.

🤖 Generated with Claude Code

Closes #10950

commented

Every 401, 403, 404, 409 and 422 JSON error answer from /api/v1 now carries a hint, one fixed plain sentence on what to do next. The new test and the checks I ran pass; I did not run the full suite or the tests the diff reaches, and I did not call keep_plan.

What changed

  • src/http/respond.ts: json() now calls a new withHint() on every error body, which covers apiError, notFound, unauthorized, forbidden and the sites that build json({error…}) themselves.

- A hint a site already wrote is kept. - no_route gets its own sentence pointing at GET /api/v1/openapi.json. - Hints are fixed text and never echo request text. not_found reads the same for a missing and a private resource.

  • src/http/mcp.ts: the unauthenticated POST /mcp 401 builds its own Response rather than going through json(), so it gets the hint written in directly.
  • test/api-hints.test.ts: walks every OpenAPI route (anonymous and with a read-only token), sends a bad call to each, and fails if a JSON error answer with status 401/403/404/409/422 has a missing or empty hint. It also checks that the 404 hint is identical for missing and private, and that other statuses are left alone.
  • docs/MCP.md: a short section names the field.

Tests run: test/api-hints.test.ts, test/api-errors.test.ts, test/api-reference.test.ts, test/openapi.test.ts, test/mcp-size.test.ts (ceilings untouched), test/read-api.test.ts and the first twelve test/mcp*.test.ts files. scripts/gate.ts and tsc --noEmit came back clean. scripts/fast.ts said the diff reaches 1650 of 2609 test files, so I picked the covering ones myself.

Not done

  • src/http/server.ts is untouched: it was marked hot, and its few hand-built 401/403 responses (for example the one at line 714) still carry no hint.
  • src/web/api_reference.ts is untouched: I named the field in docs/MCP.md instead.
  • Pre-push hook: it demanded a "Try:" commit before it would let the push through, so the branch ends with an empty commit saying how to try it.

To try it: curl -s $URL/api/v1/repos/x/y/z with no token returns {error, message, hint}.

🤖 Generated with Claude Code

commented

I merged main into the branch and pushed it. The only conflict was in docs/MCP.md, and both sides' text is kept.

Resolution: I took main's version of the file, with its drift paragraphs on the flow tool ({ kind: "drift" } and action: "update"). I then appended this pull's new "hint on a refused REST call (#10950)" section at the end. Both sides had added text at the same spot, so the section order is my choice, not something the merge settled.

What I ran:

  • bun x tsc --noEmit and bun scripts/gate.ts both ran clean.
  • bun test test/api-hints.test.ts passed all 3 tests. That is the test that walks every OpenAPI route and fails if a 4xx JSON body lacks a hint.
  • I did not run the rest of the suite. The forge runs the full check on this push.

What this pull does (unchanged by the merge): every 401, 403, 404, 409 and 422 JSON answer from /api/v1 carries a hint, one fixed plain sentence saying what to do next. It never echoes request text. A 404 reads the same whether the resource is missing or private.

To try it: run curl -i $FORGE_URL/api/v1/repos/x/y/z with no token and read the hint in the 401 body.

🤖 Generated with Claude Code

commented

The check failures are fixed and pushed. I ran the 20 test files I edited locally and all 147 tests in them pass. tsc and the gate are clean. I did not run the full suite here, so the forge's check is the proof for anything outside those files.

The 32 failures had three causes:

  • Exact-body assertions: 28 of the 32 were tests that asserted a refused body with toEqual({ error, message }), and every refused body now also carries hint. I switched them to toMatchObject in 17 files, so they still pin error and message but allow the new key. The tests were wrong only in being stricter than the contract, and the hint itself is still checked by test/api-hints.test.ts.
  • mcp-write-scope: flow gained a drift-update write on main, and this test called it with a dummy action: "x", which flow treats as a read. It now calls flow with its real update arguments. This failed on a clean origin/main checkout in my sandbox too, so it comes from main, not this pull.
  • mcp-system-stalls: the test faked a 5 s stall, so any /mcp request that had just finished in that process was blamed instead of the named job. It now fakes a 300 ms stall (the limit is 250 ms) after waiting 350 ms. This also failed on a clean main checkout in my sandbox.

If you'd rather have the main fixes in their own pull, they are the mcp-write-scope and mcp-system-stalls test changes. No source behaviour changed in this push.

🤖 Generated with Claude Code

Changes requested — before the latest push

Mechanism is sound (central withHint in json(), fixed text, 404 identical), but two things must change. First: the hint is not on every refusal. Hand-built 401/403 responses in src/http/server.ts (e.g. ~line 714) bypass json() and carry no hint, and the walk test does not catch them. Route them through json()/withHint (or add the hint), and make the test fail on them: also call with a token lacking scope, a bad body on write routes (422) and a wrong-state call (409) so each status in the issue is actually exercised, and assert seen covers 401,403,404,409,422 not just 401 and 404. Second: unrelated test fixes (mcp-write-scope, mcp-system-stalls) are scope creep for a single-purpose pull; move them to their own pull. Also: hints are generic per status, so a 409 like held/review_required gets "resource changed or already exists", which misleads; and 403/422 hints do not name the scope or field the issue asked for. Give the common codes (held, review_required, checks_required, invalid, not_admin) their own sentences via HINTS_BY_CODE. Remove the empty Try: commit by squashing if possible. I did not verify mcp-size.

commented

Every 4xx JSON refusal from the REST API carries a hint, and the walk test now has to see all five statuses. Two of sentinel's points I did not do; the reasons are below. tsc and scripts/gate.ts are clean and test/api-hints.test.ts passes. I did not re-run the other suites, but I pushed on top of the earlier commit that already fixed them.

What changed

  • Hints per code: HINTS_BY_CODE in src/http/respond.ts now gives held, review_required, checks_required, invalid, not_admin and forbidden their own sentences, so a 409 like held no longer says "resource changed or already exists".

- The forbidden sentence names the token scope: a read-only (ro) token cannot write, so mint an rw token. - The invalid sentence names the offending field.

  • Test: test/api-hints.test.ts makes explicit calls that provoke each status, and the test fails unless 401, 403, 404, 409 and 422 were all seen. The calls are:

- 401: no token. - 403: a read-only token writing, and a non-admin on an admin route. - 404: an unknown path. - 409: maintenance/resume when the forge isn't in maintenance. - 422: ?limit=abc.

  • Scope creep: the mcp-write-scope and mcp-system-stalls test edits are reverted to main, so they go to their own pull. If those two tests fail on this branch without the edits, that is the pre-existing flake, not hints.

What I chose not to do

  • Hand-built Responses in server.ts: I left gitChallenge at line 714 and the git push refusals as they are. They are plain-text git wire responses (auth required, forbidden), not JSON bodies. A hint would corrupt what a git client reads. Every JSON refusal goes through json(), so the issue's scope, every JSON 4xx, is covered.
  • Squashing the empty Try: commit: that needs a force push, which is refused here. A person can squash on merge.
  • mcp-size: I did not touch it.

Try it Run curl -i -X POST $URL/api/v1/repos -d '{}' and read the hint in the 401 body.

🤖 Generated with Claude Code

Approved

Earlier asks are met. Hints are now per code (held, review_required, checks_required, invalid, not_admin, forbidden, no_route) instead of one generic sentence per status. The walk test fails unless 401, 403, 404, 409 and 422 were each actually seen. The unrelated mcp test edits are reverted. Plain-text git wire responses in server.ts are rightly left alone. Hints are fixed text, so they never echo request text, and 404 reads the same for a missing and a private resource. docs/MCP.md names the field, and mcp-size is untouched. Non-blocking: the forbidden hint talks about read-only tokens even when the refusal is a role or origin refusal; the message carries the specifics, but a more neutral sentence would mislead less.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs