fix(specverb): a response offering JSON is parsed, not raw

coilyco-ops commented

2026-08-16 04:01:54 +00:00

Member

Closes #293. My regression from #291, fixed and owned.

The rule

rawResponseOp read "any non-JSON media type present" as raw. It now reads "no JSON offered at all". A response listing JSON beside something else is negotiating content rather than declaring bytes.

That is the direction docs/specverb-raw-responses.md already argued for - a spec that says nothing stays parsed - and the one Darren proposed under "worth deciding separately". Taking it also fixes the reported symptom without touching the vendored spec, which the report was right to rule out.

The mechanism was not what the report inferred

The report says the root produces is read instead of the operation's. Close, but the operation-level list is honoured - I checked. My first regression test declared produces inline on both operations and passed without the fix, which is what caught it.

The real path is a shared $ref response. repoGet's 200 is {"$ref": "#/responses/Repository"}, and a shared response definition has no operation context, so openapi2conv fills its content from the document root - which lists text/html. The regression test now uses that shape and fails without the fix.

Verified against the real snapshot

Not a synthetic. Parsed the vendored forgejo.swagger.lock.json.gz and read the classification back:

repoGet                  raw=false content=[application/json text/html]
repoGetActionJobLogs     raw=true  content=[text/plain]
repoGetActionRunLogs     raw=true  content=[application/zip]

Both acceptance criteria hold: the object read parses again, and a genuinely raw leaf still refuses --query.

Verification

make build, make vet, make lint (0 issues), go test ./http/... green. godoc-current.txt regenerated.

The lint run needed golangci-lint cache clean first - the cache held paths from a temp clone deleted earlier and reported five phantom violations in files that no longer exist. Worth knowing if anyone else sees gosec hits in ../umbra-*/.

Closes #293. My regression from #291, fixed and owned. ## The rule `rawResponseOp` read "any non-JSON media type present" as raw. It now reads "**no JSON offered at all**". A response listing JSON beside something else is negotiating content rather than declaring bytes. That is the direction `docs/specverb-raw-responses.md` already argued for - a spec that says nothing stays parsed - and the one Darren proposed under "worth deciding separately". Taking it also fixes the reported symptom without touching the vendored spec, which the report was right to rule out. ## The mechanism was not what the report inferred The report says the root `produces` is read instead of the operation's. Close, but the operation-level list **is** honoured - I checked. My first regression test declared `produces` inline on both operations and **passed without the fix**, which is what caught it. The real path is a **shared `$ref` response**. `repoGet`'s 200 is `{"$ref": "#/responses/Repository"}`, and a shared response definition has no operation context, so `openapi2conv` fills its content from the document root - which lists `text/html`. The regression test now uses that shape and fails without the fix. ## Verified against the real snapshot Not a synthetic. Parsed the vendored `forgejo.swagger.lock.json.gz` and read the classification back: ``` repoGet raw=false content=[application/json text/html] repoGetActionJobLogs raw=true content=[text/plain] repoGetActionRunLogs raw=true content=[application/zip] ``` Both acceptance criteria hold: the object read parses again, and a genuinely raw leaf still refuses `--query`. ## Verification `make build`, `make vet`, `make lint` (0 issues), `go test ./http/...` green. `godoc-current.txt` regenerated. The lint run needed `golangci-lint cache clean` first - the cache held paths from a temp clone deleted earlier and reported five phantom violations in files that no longer exist. Worth knowing if anyone else sees gosec hits in `../umbra-*/`.

coilyco-ops added 1 commit

2026-08-16 04:01:54 +00:00

fix(specverb): a response offering JSON is parsed, not raw

ci / secrets (pull_request) Successful in 8s

Details

ci / test (pull_request) Successful in 42s

Details

ci / lint (pull_request) Successful in 29s

Details

92afd48af7

#291 read "any non-JSON media type" as raw. A Swagger 2.0 shared $ref
response inherits the document's root produces, and Forgejo's root lists
text/html beside application/json, so every object read in the fleet was
classified raw and --query was refused on all of them.

Raw now means offering no JSON at all. A response listing JSON beside
something else is negotiating content rather than declaring bytes, which
is the fail-safe direction docs/specverb-raw-responses.md already argued
for and the one Darren proposed on the report.

Checked against the vendored Forgejo snapshot rather than a synthetic:
repoGet carries [application/json text/html] and is parsed again, while
repoGetActionJobLogs and repoGetActionRunLogs stay raw. Both halves of
the issue's acceptance hold.

The regression test uses a shared $ref response, because my first attempt
declared the media types inline and passed without the fix. Inline
produces is honoured; the $ref path is where the root leaks in, and a
test that does not reproduce is worse than none.

Closes #293

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-16 04:01:54 +00:00

coilyco-ops referenced this pull request

2026-08-16 04:09:31 +00:00

chore(comments): move the over-long blocks into docs, and fix the skill drift #296

coilyco-ops commented

2026-08-16 04:28:29 +00:00

Author

Member

Superseded, not abandoned. 92afd48 is on main already. Angie (ENG, claude seat).

I stacked #296 on this branch because both touch http/specverb/openapi3.go. Merging #296 therefore carried this commit with it, and main now contains the fix at 99744cc:

$ git merge-base --is-ancestor 92afd48 origin/main
YES

So this pull request has nothing left to merge. Forgejo reports changed_files: 0, its merge base equals its own head, and a merge attempt returns 405 Please try again later - which is the honest answer to a request to merge an empty diff.

Closing this as delivered. The fix is verifiable on main rather than by trusting this note:

$ git show origin/main:http/specverb/openapi3.go | grep offersJSON
if offersJSON(ref.Value.Content) {
// offersJSON reports whether a response lists any JSON media type at all.
func offersJSON(content map[string]*openapi3.MediaType) bool {

Closes #293 was on this PR rather than on #296, so the automatic close did not fire when #296 merged. Closing #293 by hand and recording why there.

The lesson worth keeping: stacking a chore PR on a fix PR made the fix un-mergeable through its own record. Next time the fix goes first and alone, and the chore rebases after it lands.

**Superseded, not abandoned. `92afd48` is on `main` already.** Angie (ENG, `claude` seat). I stacked #296 on this branch because both touch `http/specverb/openapi3.go`. Merging #296 therefore carried this commit with it, and `main` now contains the fix at `99744cc`: ``` $ git merge-base --is-ancestor 92afd48 origin/main YES ``` So this pull request has nothing left to merge. Forgejo reports `changed_files: 0`, its merge base equals its own head, and a merge attempt returns `405 Please try again later` - which is the honest answer to a request to merge an empty diff. Closing this as delivered. The fix is verifiable on `main` rather than by trusting this note: ``` $ git show origin/main:http/specverb/openapi3.go | grep offersJSON if offersJSON(ref.Value.Content) { // offersJSON reports whether a response lists any JSON media type at all. func offersJSON(content map[string]*openapi3.MediaType) bool { ``` `Closes #293` was on this PR rather than on #296, so the automatic close did not fire when #296 merged. Closing #293 by hand and recording why there. **The lesson worth keeping:** stacking a chore PR on a fix PR made the fix un-mergeable through its own record. Next time the fix goes first and alone, and the chore rebases after it lands.

coilyco-ops closed this pull request

2026-08-16 04:28:56 +00:00

coilyco-ops referenced this pull request

2026-08-16 04:29:18 +00:00

--query is refused on every Forgejo verb, because RawResponse reads the root produces instead of the operation's #293