Skip to content

refactor(api): remove dead WithHTTPClient option (POS-109) - #32

Merged
nsollazzo merged 1 commit into
mainfrom
pos-109-remove-dead-withhttpclient
Jul 8, 2026
Merged

refactor(api): remove dead WithHTTPClient option (POS-109)#32
nsollazzo merged 1 commit into
mainfrom
pos-109-remove-dead-withhttpclient

Conversation

@nsollazzo

Copy link
Copy Markdown
Contributor

What

Remove the dead WithHTTPClient Option from internal/api/client.go.

Why this is today's pick (POS-109 daily code-health)

Scanned all three repos (positronick, cli, ui) for the single highest impact-to-risk simplification I could prove safe and verify today:

  • cli is pristine — go vet, staticcheck, and golangci-lint all report 0 issues; full suite green.
  • ui is a component library, so "unused exports" are its public product surface — dead-export pruning there is a trap, not a safe win.
  • positronick (the app) surfaced no dead code from static scanning and needs a heavier setup (pnpm + DB) to verify safely.

WithHTTPClient is the one unambiguously dead symbol: an exported Option with zero callers anywhere in the module — no production code, no tests. golang.org/x/tools/cmd/deadcode flags it and a full-repo grep including _test.go confirms it. (WithSleep, the sibling option, is a live test seam and is kept; AdminSoul/AdminListing are tested API-contract methods with plausible planned use, so deliberately left alone.)

Before / after

-// WithHTTPClient replaces the underlying *http.Client (e.g. for a custom
-// transport). The caller owns the timeout configuration.
-func WithHTTPClient(h *http.Client) Option { return func(c *Client) { c.http = h } }

Net: 4 lines removed, 0 added. No behavior change — the http field and its default construction are untouched.

Gates (all green)

  • go build ./...
  • go vet ./...
  • gofmt (no diff)
  • golangci-lint (v2.12.2, CI version) -> 0 issues
  • go test ./... -race -> all packages pass

Scope

Surgical, single focused change (Rule 5). Dead-code removal only; no refactors of adjacent code.

🤖 Generated with Claude Code

WithHTTPClient was an exported Client Option with zero callers anywhere
in the module — no production code and no tests. golang.org/x/tools
deadcode flags it and a full-repo grep (including _test.go) confirms it.
Removing it shrinks the api package's public surface with no behavior
change; the http field and its default construction stay untouched, and
the sibling WithSleep option (a live test seam) is unaffected.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@nsollazzo

Copy link
Copy Markdown
Contributor Author

✅ R. Cutie (Quality) — approve · independent review + e2e green

Last-eyes review of the POS-109 daily code-health change. Reviewed the whole file (not just the hunk) and traced the removed symbol's blast radius.

Note on the "Approve" button: the GitHub API rejects a formal approval here because the bot token identity is the same account as the PR author (Can not approve your own pull request). This comment is my binding sign-off; the merge gate (human/CEO) has my green light. Formal Approve requires a distinct reviewer identity.

Change

Removes the exported WithHTTPClient Option from internal/api/client.go. 4 lines removed, 0 added, single hunk.

Independently verified (fresh clone of Positronick/cli, PR head 83db590)

  • Genuinely deadgrep -rn WithHTTPClient over *.go on the PR head: 0 matches. On origin/main the only references were the definition itself (client.go:77,79) — no callers in production or tests, module-wide. The dead-code claim holds.
  • No behavior change — the http field is still defaulted at construction (client.go:99 http: &http.Client{Timeout: defaultTimeout}) and used at client.go:152 (c.http.Do(req)). WithHTTPClient was the only external setter; removing it cannot leave http nil because the constructor always sets it. Sibling WithSleep (a live test seam) correctly kept.

Four gates — re-run locally, all green

Gate Result
go build ./... OK
go vet ./... OK
gofmt -l . clean (no diff)
golangci-lint run (v2.12.2, CI version) 0 issues
go test ./... -race all packages pass

e2e — make e2e (go test ./cmd/positronick/ -v)

All pass: TestE2ESmoke (7 subtests incl. the real-http network_failure path), TestE2ESoulInstall, and the installer/goreleaser/skill contract tests. ok cmd/positronick 2.974s.

Verdict: approve-with-no-nits. Surgical, provably safe, fully verified. Not merging — merge is a human/CEO gate per POS-110.

@nsollazzo

Copy link
Copy Markdown
Contributor Author

R. Cutie — Review: APPROVE ✅ (do not merge — human/CEO gate)

Independent review + e2e for the POS-109 daily code-health change. I re-verified every claim from scratch on a fresh checkout of pos-109-remove-dead-withhttpclient (HEAD 83db590) — not trusting the author's narrative.

What I verified independently

Check Result
WithHTTPClient callers, module-wide incl. _test.go grep -rn0 matches — genuinely dead
go build ./... ✅ exit 0
go vet ./... ✅ exit 0
gofmt -l . ✅ no diff
golangci-lint run ./... (v2.12.2 — the CI version) 0 issues
go test ./... -race ✅ all packages pass
e2e smoke go build ./cmd/positronick + binary runs

Two things I checked that the diff could have broken but didn't

  • The http field is not orphaned by removing its Option. It's still declared (client.go:69) and used on the request path (client.go:152 c.http.Do(req)), and default construction in New is untouched. So this is a true zero-behavior-change removal — no dead field left behind, no new lint.
  • The kept sibling WithSleep is genuinely live, not another dead symbol slipping through: 2 real callers in client_test.go (L67, L567). Author's judgment to keep it while removing WithHTTPClient is correct.

Gate that's N/A here (stated, not skipped)

"New test must fail without the change" does not apply — this is dead-code removal with no new test and no behavior change. Nothing to red/green. Correctly handled.

Verdict

Approve. No Blockers, no Should-fix, no Nits. Textbook surgical (Rule 5) dead-code removal, fully evidenced.

Merge remains a human/CEO gate — I review and propose, I do not merge.

— R. Cutie

@nsollazzo
nsollazzo merged commit 9d6378f into main Jul 8, 2026
9 checks passed
@nsollazzo
nsollazzo deleted the pos-109-remove-dead-withhttpclient branch July 8, 2026 13:37
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.

1 participant