Skip to content

feat(api)!: send application/problem+json on problem bodies - #660

Merged
ThomasK33 merged 1 commit into
mainfrom
issue352-problem-json
Nov 4, 2026
Merged

ThomasK33 merged 1 commit into
mainfrom
issue352-problem-json

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Every error body with the RFC 9457 problem members now has Content-Type: application/problem+json. Before, it was application/json. This is item 5 of #352. Result envelopes and success bodies keep application/json.

Why

I checked the vendored spec 412b90c3 and recorded the answer on #352 before coding (issuecomment-6628318574):

  1. The spec's Errors section says problem details come "under application/problem+json".
  2. In the write 409s, application/problem+json carries the problem body and application/json carries the result envelope (CommitWriteResponse, MergeConflict).
  3. Upstream's live server answers application/problem+json for every Accept value I tried.

The desk rule for item 5: follow what the spec documents.

What changes

  1. writeProblemJSON (internal/api/errors.go) writes every body that newProblem builds as application/problem+json. That covers writeErr, writeProblem, writeInvalid, the ref_guard_failed bodies (merge guard, notes guard) and the git-surface 429.
  2. writeJSON keeps application/json for everything else: success bodies, and the wcFail, wmFailConflict and wrefNoteFail result envelopes.
  3. Docs: docs/reference/http-api.md (Errors and the notes guard) and the RFC 9457 row of docs/explanation/wire-compatibility.md.

SDK effect

SDK Effect
0.1.0 Go, Python, TypeScript None. Each reads the error body as JSON whatever the type.
1.18.0-beta None. It accepts both types.
1.17.1 Python None. It calls response.json() with no type check.
1.17.1 Go and TypeScript They parse error JSON only for application/json. The error message becomes the raw body text instead of error, and Body becomes a string.

Validation

  • go build ./..., golangci-lint run (2.15.0): 0 issues. Docs gate: 0 broken links, link-checker tests OK.
  • Race lanes on head afb8c3aa with CI's flags: ./internal/api/... and ./cmd/... pass. internal/githttp lanes: see the comment below.
  • Tests: requireEnvelope and decodeEnvelope, which back most error assertions in internal/api, now require application/problem+json. So do the writeErr unit test, the merge and notes guard checks and the two 429 tests. The new TestResultEnvelopeContentType pins application/json on the result envelopes.

Dogfood

I used the verify-openstorage harness: one local node built from origin/main (63f81faa) and one from this head, each with 17 PASS checks.

Request Main This PR
401 no token, 404 branch, 404 alias repo, 400 bad branch name, 409 merge guard, 409 notes guard application/json application/problem+json
412 commit-pack stale guard (result envelope), 200 branches application/json application/json
  • SDK 0.1.0 Go (go/v0.1.0) on the same five error calls gave the same type, code, detail and message on both nodes, including RefGuardError with guard and current_sha.
  • SDK 1.17.1 Go gave message="repository not found" on main and the raw problem JSON on this PR. That is the effect in the BREAKING CHANGE note below.
  • The harness minted its tokens with openstorage mint, because SDK 0.1.0's own tokens fail here (Accept the tokens that SDK 0.1.0 signs: SDK sub claims and org-wide tokens without a repo claim #658).

Refs #352

BREAKING CHANGE: error bodies with problem members (code, detail, ...) are now sent as Content-Type: application/problem+json, as upstream documents. SDK 1.17.1 Go and TypeScript clients parse error JSON only for application/json, so their error message becomes the raw body text and the parsed body becomes a string. SDK 0.1.0 and the 1.17.1 Python client are not affected.


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

Every body that newProblem builds (writeErr, writeProblem, writeInvalid,
the ref_guard_failed bodies and the git-surface 429) now has
Content-Type application/problem+json, the type upstream's spec documents
for problem details. Result envelopes and success bodies keep
application/json.

_Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high_

Change-Id: I1031f8f8f36505c402a72d1c1654e37c54636319
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T10:37:00.224989Z afb8c3a Manual request
🔒 Security Review ✅ Completed 2026-11-04T10:37:17.989478Z afb8c3a Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: afb8c3aa26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: afb8c3aa26

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33

Copy link
Copy Markdown
Member Author

Local race lanes on head afb8c3aa (CI flags, git 2.55, under the shared heavy lock) all passed:

Lane Result
go test ./internal/api/... ./cmd/... -tags=openstorage_viewcheck -race -count=1 ok (internal/api 279.5 s, cmd/openstorage 9.8 s)
go test ./internal/githttp -tags=openstorage_viewcheck -race -count=1 (PostgreSQL) ok (898.6 s)
OPENSTORAGE_TEST_BACKEND=local go test ./internal/githttp -race -count=1 ok (882.6 s)

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant