Skip to content

Send headers passed to bk api --headers - #1019

Merged
malclocke merged 2 commits into
mainfrom
malc/fix-api-headers
Oct 8, 2026
Merged

malclocke merged 2 commits into
mainfrom
malc/fix-api-headers

Conversation

@malclocke

Copy link
Copy Markdown
Contributor

Description

bk api accepts --headers/-H, but the values were parsed into a map that was never used, so REST requests went out without them:

bk api -H "X-Custom: value" /pipelines   # X-Custom was not sent

This passes the headers through to the HTTP client so they are sent with the request.

Changes

  • Add a WithHeaders option to the internal REST client. Custom headers are applied after the defaults, so they can override Accept or Content-Type. Authorization and User-Agent are still set by the auth transport.
  • Repeated headers (-H "X: a" -H "X: b") are all sent.
  • A header without a colon now returns an error instead of being silently dropped.
  • --file (GraphQL) requests still ignore --headers, as before.

Testing

  • Tests have run locally (with go test ./...) — cmd/preflight and internal/preflight timing tests flaked in the full run and pass in isolation
  • Code is formatted (with go fmt ./...)

Disclosures / Credits

Amp found the bug, wrote the fix and tests, and opened this PR.

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]>
@malclocke
malclocke marked this pull request as ready for review October 8, 2026 03:54
@malclocke
malclocke requested review from a team as code owners October 8, 2026 03:54

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cmd/api/api.go
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]>

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@buildsworth-bk-app
buildsworth-bk-app Bot dismissed their stale review October 8, 2026 04:06

Superseded by a newer Buildsworth review.

@malclocke
malclocke merged commit 6347939 into main Oct 8, 2026
2 checks passed
@malclocke
malclocke deleted the malc/fix-api-headers branch October 8, 2026 04:14
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.

2 participants