fix(opcore): decide raw-vs-parsed before the request, not after #291

Merged

coilysiren merged 1 commit from aos/claude/bk79-raw-response into main

2026-08-15 16:23:28 +00:00

coilyco-ops commented

2026-08-15 15:51:45 +00:00

Member

Part one of #289, and it fixes coilyco-flight-deck/agentic-os#1044.

The correction to #289

#289 reads the code as "specverb honours RawResponse, opcore ignores it." The first half is not true. specverb/request.go had the branch, but it was unreachable:

_, respBody, status, err := rt.FireCapture(...)
if err != nil { return err }        // <- always returns first
if desc.RawResponse { ... }          // <- dead for every non-JSON body

FireCapture JSON-decodes every success body and returns a coded error on failure, so a text/plain log or an application/zip archive aborted the call before either path consulted the flag. Both call sites were broken, not one. That is why setting the field "changes nothing" on the opcore path and equally on the CLI path.

The fix

send - the transport, extracted out of FireCapture. Never inspects the payload.
FireCaptureRaw - send without the decode.
specverb.fire and opcore.Operation.Execute both branch on Desc.RawResponse before firing.

Deciding after the call cannot work, because the decode error is indistinguishable from a real one.

Security floor unchanged

Only the decode is skipped, never a gate. Restrict, auth, the redirect floor, and the non-2xx failure path all behave as before, and an op that declares no media type stays parsed - the fail-safe direction. A raw op has no decoded value, so an inline grant's fail-when has nothing to evaluate and does not run.

Tests

http/opcore/rawresponse_test.go pins six cases. The first two fail on main with the exact errors from the downstream report:

--- FAIL: TestExecuteReturnsDeclaredPlaintextIntact
    a declared-plaintext body failed the call: invalid character '-' after top-level value
--- FAIL: TestExecuteReturnsDeclaredBinaryIntact
    a declared-ZIP body failed the call: invalid character 'P' looking for beginning of value

The other four hold the line: undeclared non-JSON still rejected, upstream 404 still upstream_failed, empty raw body still a success, restrict still enforced on a raw leaf.

Verification

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

Two things I did not get to a clean state, both pre-existing and neither caused by this change:

pkg/broker tests fail locally on macOS - the sandbox $TMPDIR path exceeds the unix socket name limit (bind: invalid argument). Identical failure on a clean tree.
pre-commit run --all-files reports 15 code-comment violations, an 84-line README.md, and drift in repo-cli-guard/SKILL.md. All identical on a clean tree; this branch adds none. CI is green on main at the same base commit, so this looks like local catalog-hook drift rather than a red repo.

I also noticed pre-commit autofixes trailing whitespace in three assets/mark/umbra-favicon-*.svg files from #290. I deliberately left those out of this commit rather than sweep an unrelated fix into a bugfix PR.

Still open on #289

Part two. The inline grammar has no node that sets RawResponse, so a hand-written .mcp.kdl grant still cannot request it - only the spec-driven path infers it from the declared media type. This PR does not close #289.

Written up in docs/specverb-raw-responses.md.

Part one of #289, and it fixes `coilyco-flight-deck/agentic-os#1044`. ## The correction to #289 #289 reads the code as "specverb honours `RawResponse`, opcore ignores it." The first half is not true. `specverb/request.go` had the branch, but it was **unreachable**: ```go _, respBody, status, err := rt.FireCapture(...) if err != nil { return err } // <- always returns first if desc.RawResponse { ... } // <- dead for every non-JSON body ``` `FireCapture` JSON-decodes every success body and returns a coded error on failure, so a `text/plain` log or an `application/zip` archive aborted the call before either path consulted the flag. Both call sites were broken, not one. That is why setting the field "changes nothing" on the opcore path and equally on the CLI path. ## The fix * `send` - the transport, extracted out of `FireCapture`. Never inspects the payload. * `FireCaptureRaw` - `send` without the decode. * `specverb.fire` and `opcore.Operation.Execute` both branch on `Desc.RawResponse` **before** firing. Deciding after the call cannot work, because the decode error is indistinguishable from a real one. ## Security floor unchanged Only the decode is skipped, never a gate. Restrict, auth, the redirect floor, and the non-2xx failure path all behave as before, and an op that declares no media type stays parsed - the fail-safe direction. A raw op has no decoded value, so an inline grant's `fail-when` has nothing to evaluate and does not run. ## Tests `http/opcore/rawresponse_test.go` pins six cases. The first two fail on `main` with the exact errors from the downstream report: ``` --- FAIL: TestExecuteReturnsDeclaredPlaintextIntact a declared-plaintext body failed the call: invalid character '-' after top-level value --- FAIL: TestExecuteReturnsDeclaredBinaryIntact a declared-ZIP body failed the call: invalid character 'P' looking for beginning of value ``` The other four hold the line: undeclared non-JSON still rejected, upstream 404 still `upstream_failed`, empty raw body still a success, restrict still enforced on a raw leaf. ## Verification `make build`, `make vet`, `make lint` (0 issues), and `go test ./http/...` are green. `godoc-current.txt` regenerated. Two things I did **not** get to a clean state, both pre-existing and neither caused by this change: * `pkg/broker` tests fail locally on macOS - the sandbox `$TMPDIR` path exceeds the unix socket name limit (`bind: invalid argument`). Identical failure on a clean tree. * `pre-commit run --all-files` reports 15 code-comment violations, an 84-line `README.md`, and drift in `repo-cli-guard/SKILL.md`. All identical on a clean tree; this branch adds none. CI is green on `main` at the same base commit, so this looks like local catalog-hook drift rather than a red repo. I also noticed `pre-commit` autofixes trailing whitespace in three `assets/mark/umbra-favicon-*.svg` files from #290. I deliberately left those out of this commit rather than sweep an unrelated fix into a bugfix PR. ## Still open on #289 Part two. The inline grammar has no node that sets `RawResponse`, so a hand-written `.mcp.kdl` grant still cannot request it - only the spec-driven path infers it from the declared media type. This PR does not close #289. Written up in `docs/specverb-raw-responses.md`.

coilyco-ops added 1 commit

2026-08-15 15:51:46 +00:00

fix(opcore): decide raw-vs-parsed before the request, not after

ci / test (pull_request) Successful in 43s

Details

ci / secrets (pull_request) Successful in 12m10s

Details

ci / lint (pull_request) Successful in 12m32s

Details

60f2f68fbb

Descriptor.RawResponse was set correctly from the spec's declared media
type and honoured by a branch neither path could reach. FireCapture
JSON-decodes every success body and returns a coded error on failure, so
a text/plain log or an application/zip archive aborted the call before
specverb's fire and opcore's Execute ever consulted the flag. Setting the
field changed nothing, which is what made this look like a missing feature
rather than an unreachable branch.

Split the transport out of FireCapture as send, add FireCaptureRaw over
it, and choose the path before firing on both call sites. Only the decode
is skipped: restrict, auth, the redirect floor, and the non-2xx failure
path are untouched, and an op that declares no media type stays parsed.

This is part one of #289. The inline grammar still has no node that sets
RawResponse, so a hand-written .mcp.kdl grant cannot request it yet.

Fixes the aosguard forgejo action-job/action-run logs leaves reported in
coilyco-flight-deck/agentic-os#1044.

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

coilyco-ops requested review from coilysiren

2026-08-15 15:51:46 +00:00

coilyco-ops referenced this pull request

2026-08-15 15:56:24 +00:00

Descriptor.RawResponse is honoured by specverb but ignored by opcore.Operation.Execute #289