feat(api): ana api raw-JSON passthrough verb (#25)#26
Merged
Conversation
Thin authenticated HTTP passthrough. Short-form Connect-RPC
(`textql.rpc.public.<Service>/<Method>`) and documented REST
(`/v1/...`) distinguished by leading slash. Default method POST with
`{}` body so RPC shorthand works out of the box; `--data`/`--data-stdin`
supply a custom body, `--raw` skips pretty-print. Non-2xx bodies go to
stderr so stdout stays `| jq`-clean.
Transport gains `DoRaw` and moves bearer/UA injection to a
`bearerTransport` RoundTripper so Unary/Stream/DoRaw share one auth
path with no per-call-site header plumbing. Coverage stays at 100%
on `./internal/...`.
`--data ""` at the `ana api` layer previously sent Content-Type:
application/json with a zero-byte request body. That's semantically
odd (empty-but-JSON-tagged) and different from both "no --data"
(defaults to `{}`) and `--data '{}'` (literal empty object). Guard on
`len(body) > 0` in both the body-reader and Content-Type branches so
empty-slice and nil behave identically — no body, no Content-Type.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an Changes
Sequence DiagramsequenceDiagram
participant User as CLI User
participant Cmd as ana api
participant Client as transport.Client
participant Transport as bearerTransport
participant HTTP as net/http Transport
participant Server as API Server
User->>Cmd: ana api <path> [--method/--data/--raw]
Cmd->>Client: request via DoRaw (method,path,body)
Client->>Transport: RoundTrip(enriched http.Request)
Transport->>Transport: tokenFn(ctx) -> token\nset Authorization & User-Agent (if missing)
Transport->>HTTP: RoundTrip(request)
HTTP->>Server: HTTP request
Server-->>HTTP: HTTP response (status + body)
HTTP->>Transport: Response
Transport->>Client: Response
Client->>Cmd: (status, body) or error
alt status 2xx
Cmd->>User: pretty-print JSON or write raw bytes to stdout
else non-2xx
Cmd->>User: write body to stderr and return error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/call_test.go`:
- Around line 97-125: Add a new test that asserts an explicit empty --data
string produces an empty body (not the default {}), e.g. mirror
TestDataFlagUsedAsBody but call cmd.Run with []string{"--data", "", "foo/Bar"}
using the same fakeDeps, New(...) and stdio setup and then assert that
f.lastBody equals the empty string; place the test alongside
TestDataFlagUsedAsBody/TestDataStdinUsedAsBody so every body-resolution branch
(omitted, non-empty, empty, and stdin) is covered.
In `@internal/api/call.go`:
- Around line 63-66: The current check only ensures the first positional is
present but ignores any extra arguments (fs.NArg() and fs.Arg(0) usage), so
commands like `ana api /v1/things stray` silently drop extras; change the
validation to reject when there are extra positionals by returning a UsageErrf
if fs.NArg() != 1 (or if any additional fs.Arg(i) is non-blank), and only then
set path := fs.Arg(0); update the validation logic around fs.NArg() and
fs.Arg(0) to enforce exactly one non-blank positional argument.
In `@internal/transport/client.go`:
- Around line 84-97: The RoundTrip implementation mutates the incoming
*http.Request (violating net/http contract); fix by cloning the request before
injecting headers: in bearerTransport.RoundTrip, call cloned :=
req.Clone(req.Context()) (ensure cloned.Header is non-nil) and modify
cloned.Header (set User-Agent and Authorization using b.c.userAgent and
b.c.tokenFn) and then call b.next.RoundTrip(cloned) instead of modifying req in
place; keep the existing token error handling and header-presence checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed0b9c97-7cab-4308-8bd6-b7faa7725a88
⛔ Files ignored due to path filters (5)
README.mdis excluded by!**/*.mddocs/features.mdis excluded by!**/docs/**,!**/*.mdinternal/CLAUDE.mdis excluded by!**/*.mdinternal/api/CLAUDE.mdis excluded by!**/*.mdinternal/transport/CLAUDE.mdis excluded by!**/*.md
📒 Files selected for processing (8)
cmd/ana/main.gocmd/ana/main_test.gointernal/api/api.gointernal/api/api_test.gointernal/api/call.gointernal/api/call_test.gointernal/transport/client.gointernal/transport/client_test.go
- transport: bearerTransport.RoundTrip now clones the incoming request
before mutating headers, per the net/http RoundTripper contract (must
not modify the request except for consuming/closing the body).
- api: reject extra positional args — `ana api /v1/x stray` was silently
dropping `stray`, masking typos where the user probably meant a flag.
- api: pin the `--data ""` contract with a test so the empty-string
explicit body can't drift back to the POST default {}.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/api/call.go (1)
63-69:⚠️ Potential issue | 🟡 MinorDispatch the trimmed path, not the raw positional.
Line 63 validates
strings.TrimSpace(fs.Arg(0)), but Line 69 still forwards the untrimmed token.ana api " /v1/things "will therefore pass validation and then hit a different URL than the user intended.✂️ Proposed fix
if fs.NArg() == 0 || strings.TrimSpace(fs.Arg(0)) == "" { return cli.UsageErrf("api: <path> positional argument required") } if fs.NArg() > 1 { return cli.UsageErrf("api: unexpected positional arguments: %v", fs.Args()[1:]) } - path := fs.Arg(0) + path := strings.TrimSpace(fs.Arg(0))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/call.go` around lines 63 - 69, The code validates the positional arg with strings.TrimSpace(fs.Arg(0)) but then assigns the raw token to path; change the assignment so path = strings.TrimSpace(fs.Arg(0)) (or assign a trimmed variable before use) and ensure all subsequent uses (the variable path in call dispatch code) use that trimmed value so an input like " /v1/things " is dispatched to the trimmed URL; update any references to fs.Arg(0) in the surrounding function (e.g., where path is used) to use the new trimmed variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/api/call.go`:
- Around line 63-69: The code validates the positional arg with
strings.TrimSpace(fs.Arg(0)) but then assigns the raw token to path; change the
assignment so path = strings.TrimSpace(fs.Arg(0)) (or assign a trimmed variable
before use) and ensure all subsequent uses (the variable path in call dispatch
code) use that trimmed value so an input like " /v1/things " is dispatched to
the trimmed URL; update any references to fs.Arg(0) in the surrounding function
(e.g., where path is used) to use the new trimmed variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7b455af-7f2c-4f52-8b5d-faeeca1a1f93
📒 Files selected for processing (4)
internal/api/call.gointernal/api/call_test.gointernal/transport/client.gointernal/transport/client_test.go
A whitespace-padded positional like `ana api " /v1/things "` previously passed the blank-check (which ran on the trimmed view) but got forwarded verbatim to the transport, where joinURL would stitch it into a malformed URL. Trim once, reuse, and re-check against empty so the single source of truth for `path` is the trimmed string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #25.
Summary
ana api <path>— authenticated raw-JSON passthrough. Leading-slash path = verbatim (covers/v1/...REST and pre-resolved/rpc/public/...); no leading slash = Connect-RPC short form (textql.rpc.public.<Service>/<Method>, prefixed with/rpc/public/).--method(default POST),--data/--data-stdin(mutually exclusive),--raw(skip pretty-print). GET/HEAD omit the default body; other methods default to{}so short-form RPC calls Just Work. Non-2xx bodies land on stderr so stdout stays| jq-clean.DoRaw, and bearer + User-Agent moved to abearerTransportRoundTripper so Unary/Stream/DoRaw share one auth path.--data ""is treated like no body (no Content-Type, no zero-byte request body).Coverage stays at 100% on
./internal/....Summary by CodeRabbit
New Features
apiCLI command to make authenticated HTTP requests with a required .--method,--data,--data-stdin(mutually exclusive), and--rawpassthrough.Improvements
Tests