Describe revoked Ollama API keys accurately#1943
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 5:27 PM ET / 21:27 UTC. Summary Reproducibility: yes. from source inspection: current main exposes Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow copy/docs/test update after contributor-supplied real behavior proof, while keeping endpoint-validation semantics with #1940. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main exposes Is this the best way to solve the issue? Yes for the code path: updating the single public error string with focused test and docs coverage is the narrow maintainable fix. It still needs real behavior proof before merge because the current proof is only a deterministic test run. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 63c69fdcc47f. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
c46ca4a to
109fce5
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
1 similar comment
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
@clawsweeper ready for maintainer review |
|
@clawsweeper I forgot to add the ready for maintainer review |
|
@clawsweeper address review |
|
@clawsweeper ready for review |
|
@clawsweeper ready |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
The committed proof image was a 1x1 placeholder, not real evidence. Executable test output in the PR description replaces it.
718c6c3 to
3af3289
Compare
|
Maintainer proof for exact head
Ready to merge this exact SHA. |
Summary
Basis
Ollama's current official authentication documentation states that API keys do not currently expire and can be revoked at any time: https://docs.ollama.com/api/authentication
The production fetcher already maps 401 and 403 to
OllamaUsageError.apiUnauthorized; this change corrects that error's public description. The strengthened regression uses injected HTTP transport to exercise the bearer request, 401/403 mapping, and final localized message together without network access or credentials.Validation
swift test --filter OllamaUsageFetcherTests— 18 tests passedswift test --filter OllamaUsageFetcherRetryMappingTests— 7 tests / 8 cases passedmake check— passed with 0 format or lint violationsmake test— all 49 current groups passedLive-proof boundary
No external provider or Keychain probe was run: repository policy requires explicit authorization for live provider credentials. The safe evidence is the official lifecycle documentation plus the exact 401/403 runtime mapping under injected transport.
Risk
Very low: one error string, focused tests, docs, and changelog only. Dependencies and credential handling are unchanged.