Fix review findings: perplexity transport, URL corruption, version injection, docs

[vyhuholl]

Closes #19. Addresses 10 of the 14 findings in #8, plus several bugs found in the follow-up review attached to that issue.

Everything here is confined to main. Nothing in this branch touches PR #10 or internal/mcp.

Why this is worth reviewing first

Two of these are not cleanups — they are "the tool does not do what it says":

• perplexity-search has never worked. It sent GET https://api.perplexity.ai/ and printed a blank answer with exit code 0.
• ddg-search silently corrupted result URLs whose target contained percent-encoding.

Both are reproduced by tests added in this branch.

---

Real bugs

perplexity-search never reached the API

Search built a resty request with no method and no path and dispatched it with Send(), which resolves to Execute(r.Method, r.URL) — i.e. Execute("", ""). resty joined the empty path onto the base URL, and Go's http.NewRequest defaults an empty method to GET, so every search fired:

GET https://api.perplexity.ai/     ← actual
POST https://api.perplexity.ai/chat/completions   ← intended

checkAPIError has no 404 case, so the wrong response parsed as a valid one: the tool printed an empty answer and exited 0.

Do now takes an explicit method and path. TestSearchSendsPostToChatCompletions asserts the method, path and body a real request carries.

The answer was read from a field the API does not return

Independently of the above, APIResponse.Answer was mapped from a top-level json:"answer" field. Perplexity's OpenAI-compatible response puts the text at choices[0].message.content, so Answer was always empty even against a correct endpoint. Fixed, with ErrNoChoices guarding an empty choices array instead of printing nothing.

Sources now come from search_results (which carries titles), falling back to the deprecated top-level citations array when it is absent.

--max-results was a no-op

The flag was declared, parsed, passed to Search and discarded into _ int. It now caps the source list. The flag is kept rather than removed because skills/perplexity-search/skill.yaml passes it.

The default model no longer exists

sonar-medium-online — along with the rest of the sonar-*-online family — was retired by Perplexity and is rejected by the API. Even with the transport fixed, the default would have failed with a 400. Default is now sonar; the model lists in README and SKILL.md were updated to the current family.

extractURL decoded DuckDuckGo targets twice

u.Query().Get("uddg") already returns the percent-decoded value; the following url.QueryUnescape decoded it a second time. Any target URL whose own form still contained %xx or + came back corrupted:

.../a%2520b?q=c%2Bd    →  https://example.com/a b?q=c d      (literal spaces)
.../wiki/C%252B%252B   →  https://en.wikipedia.org/wiki/C++  (different page)

The existing test used a target with no inner encoding, so it passed. Regression tests for both cases added.

--version could never be set

-ldflags -X patches variables, not constants, so const version = "dev" made the documented build-time injection impossible — all three binaries always reported dev. Changed to var in all three, and verified:

$ go build -ldflags="-X main.version=v9.9.9-test" ./cmd/ddg-search
$ ./ddg-search --version
ddg-search version v9.9.9-test

The CI build step also passed no -ldflags, and never built or smoke-tested perplexity-search at all. Both fixed.

---

Cleanups

• ddg-search assembled its query from args.First() plus a strings.Builder loop named querySb82; replaced with strings.Join(args.Slice(), " ").
• Removed the dead Parser.IsEmptyResults / Parser.IsRateLimitPage and the orphaned testdata/rate_limit.html. FindRateLimitIndicator stays a deliberate no-op — the false-positive problem it works around is real.
• Removed the unused Client.apiKey field, the unused SearchOptions type and a duplicate SetContext call in internal/perplexity.
• Used the existing http5xxThreshold constant where a bare 500 had been left.

Tests

TestIsRateLimited never called isRateLimited — it reimplemented the predicate inline as status == 429 || status >= 500, so it covered neither DuckDuckGo's 202 signal nor the transport-error branch, and could not have caught a change to the real function. It now calls the real one, with a companion httptest driving Client.Do through a 202 retry.

Also added the missing coverage for buildSearchParams, Searcher.calculateDelay (a separate, jitter-free implementation from the Client's) and Search's blank-query early return.

Package	Before	After
internal/perplexity	75.8%	91.9%
internal/search	70.4%	77.6%
total	57.8%	66.7%

---

Three things that were not in the original findings

1. CI on main has been failing since 2026-08-24, and not because of the code. golangci-lint v2.9.0 is built with Go 1.25 and panics when type-checking against the Go 1.27 standard library:

panic: file requires newer Go version go1.27 (application built with go1.26)

mise lint fails the same way locally, which made the repo's own mandatory check impossible to run. Bumped the pin to v2.13.2 in both .mise.toml and ci.yml. v2.13 renamed two linters this config already disabled under their old names, so exhaustruct_v5 and gomodguard_v2 started firing — disabled them the way wsl/wsl_v5 already is. If you would rather pin a different version, that is the one line to change; everything else in this branch is independent of it.

2. perplexity-search searched only its first argument. perplexity-search golang tutorial without quotes searched for golang. This is the same defect as finding 1.2 in ddg-search, which the review caught only on the DuckDuckGo side. Fixed, since the README documents exactly that unquoted invocation.

3. The .env support in the docs does not exist. Neither the binary nor any dependency reads .env — the key comes from the environment only. README, SKILL.md, the error message and the spec all promised loading that never happened. They now say to export the variable, with a note on how to source a .env yourself. Adding a dotenv dependency seemed out of scope for a fix branch; happy to do it instead if you would rather keep the documented behaviour.

Docs and specs

README.md also had the go install path from #19 and a make build step for a Makefile that does not exist.

Because the default model and the stdout format changed, skill.yaml and SKILL.md are updated in the same change, per the repo's own rule. The openspec/specs requirements for perplexity-search and rate-limit-handler are updated to match actual behaviour.

CLAUDE.md is added here as well — it documents the retry nesting, the deliberate FindRateLimitIndicator stub, and the two contracts this branch fixes, so they are harder to reintroduce.

Deliberately not included

Finding	Why
3.2 — permanent vs. transient error classification	The follow-up review withdrew it: for a short-lived CLI, under-retrying real transient failures is worse than a few seconds of wasted backoff.
11.3 — raise the coverage gate	Coverage is now 66.7% against a 50% gate. Raising it further needs cmd/* tests (all three are at 0%); better as its own change.
2.1 — document the (N+1)² retry compounding	Documented in CLAUDE.md; a user-facing note can follow separately.
3.3 — align stdout write-error handling	Cosmetic; behaviour on a broken pipe is equally harmless either way.
Everything in PR #10 / internal/mcp	Out of scope for this branch.

Verification

mise lint     # 0 issues
mise test     # all packages pass, -race
go build ./...