Repository navigation
Send headers passed to bk api --headers - #1019
Conversation
The --headers/-H values were parsed but never passed to the HTTP client, so REST requests went out without them. Pass them through a new WithHeaders client option, allow repeated headers, and reject malformed values instead of silently dropping them. Amp-Thread-ID: https://ampcode.com/threads/T-01a1199e-eb6f-7472-b74f-4ca559fc2108 Co-authored-by: Amp <[email protected]>
There was a problem hiding this comment.
One header-parsing issue prevents valid requests; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 38052, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
Model: gpt-6.1-sol with xhigh thinking.
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. Unless repository policy already allows L2 approval, grant it by mentioning @buildsworth-bk (see approval ceiling and L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only when repository policy or a verified grant allows it; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical pipeline paths). Human approval required unless repository policy explicitly allows L3 approval; per-PR grants cannot allow it.
Kong splits []string flags on commas by default, so values like 'Accept: a, b' or HTTP dates were split and rejected as invalid. Amp-Thread-ID: https://ampcode.com/threads/T-01a1199e-eb6f-7472-b74f-4ca559fc2108 Co-authored-by: Amp <[email protected]>
There was a problem hiding this comment.
The previous blocker is resolved; no new findings.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 38060, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Superseded by a newer Buildsworth review.
Description
bk apiaccepts--headers/-H, but the values were parsed into a map that was never used, so REST requests went out without them:This passes the headers through to the HTTP client so they are sent with the request.
Changes
WithHeadersoption to the internal REST client. Custom headers are applied after the defaults, so they can overrideAcceptorContent-Type.AuthorizationandUser-Agentare still set by the auth transport.-H "X: a" -H "X: b") are all sent.--file(GraphQL) requests still ignore--headers, as before.Testing
go test ./...) —cmd/preflightandinternal/preflighttiming tests flaked in the full run and pass in isolationgo fmt ./...)Disclosures / Credits
Amp found the bug, wrote the fix and tests, and opened this PR.