fix(review): meter visual vision BYOK calls#4363
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
# Conflicts: # src/queue/processors.ts # test/unit/visual-vision-wiring.test.ts
…path rewrite #4335/#4353 rewrote runVisualVisionForAdvisory into a BYOK-vs-self-host dual path 42 minutes before this PR opened, so GitHub couldn't auto-merge the original diff. Reapplies the same BYOK daily-cap check + recordVisualVisionUsage accounting, scoped to only the BYOK branch (self-host consumes the operator's own resources and was never part of this spend surface). Moves the cap check back before the shot-fetching loop, matching this PR's own test name/intent ("...before fetching screenshots") -- my first pass had it after the loop. Also updates two tests for unrelated main drift since this PR opened: the "declines when no route crossed..." test now needs stubMinerCheckOnly() for #4513's install-wide reputation check, and both new tests need `mode: "live"` for the #token-bleed-spend-gate paused-mode field.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | cad1ab7 | Commit Preview URL Branch Preview URL |
Jul 10 2026, 08:26 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4363 +/- ##
=======================================
Coverage 94.11% 94.11%
=======================================
Files 432 432
Lines 38348 38368 +20
Branches 13979 13988 +9
=======================================
+ Hits 36091 36111 +20
Misses 1600 1600
Partials 657 657
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-10 08:54:45 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…al-vision's usage status recordVisualVisionUsage hardcoded status: "ok" even when visionResponse.failure was set (a genuine timeout/http_error/exception) -- the detail string already distinguished this from a completed-but-empty response, but that distinction was discarded at the status level. Now uses "error" for a genuine failure, matching runAgentSummary's existing convention (services/ai-summaries.ts) for the same shape of problem. Since countByokAiEventsForRepoSince/sumByokAiUsageForRepoSince gate on status = "ok", introducing "error" required deciding whether it should still count toward the daily BYOK cap. It must: excluding failed attempts would turn a flaky or misconfigured provider into a way to bypass the cap entirely via forced failures, defeating the whole point of #4363's own fix. Switched both functions from an exact "ok" match to an explicit BYOK_SPEND_ATTEMPT_STATUSES allowlist (["ok", "error"]). Caught during review: an exclusion-based filter (!= "quota_exceeded") is NOT safe here, because ai_usage_events is also reused for BYOK key-lifecycle audit rows (recordAiKeyChange's "set"/"replace"/"delete") whose model is also byok:<provider>-prefixed -- an exclusion would have silently started counting those as spend. Caught by a new regression test before it shipped.
Motivation
runVisualVisionForAdvisory) previously decrypted a repo BYOK key and called the external provider without checking the shared per-repo daily BYOK cap, recording usage, or caching, allowing repeated contributor actions to cause maintainer-paid provider calls.Description
countByokAiEventsForRepoSinceand usingAI_BYOK_DAILY_REPO_LIMIT/DEFAULT_BYOK_DAILY_REPO_LIMITwithclampNumberinrunVisualVisionForAdvisory(src/queue/processors.ts).recordVisualVisionUsagethat callsrecordAiUsageEventwithfeature: "visual_vision"andmodel:byok:; provider usage/token/cost fields are forwarded when available so calls count toward the shared BYOK counter (src/queue/processors.ts`).callAiProviderusage is now recorded where applicable).AI_BYOK_DAILY_REPO_LIMIT(ensures no screenshot/provider calls) and verifying successful visual BYOK calls increment the repo/day counter (test/unit/visual-vision-wiring.test.ts).Testing
npx vitest run test/unit/visual-vision-wiring.test.ts, and the new/modified tests passed.npm run typecheckand it passed with no errors.npm run test:coverage, but the run exposed unrelated long-running/hang behavior in the existingtest/unit/queue.test.tsshard and was interrupted (investigation of the unrelated suite noise is out of scope for this patch).npm audit --audit-level=moderatecould not complete in this environment due to the registry returning403 Forbidden.Codex Task