Repository navigation
Conversation
Go: the /workspace/<id>/go dashboard is now a JS SPA shell with no server-rendered usage windows, so HTML scraping always fails. Fetch usage from the console JSON API (GET /console/api/go/status with the __Host-console_session browser cookie + x-org-id workspace header) and parse fiveHour/week/month meters into rolling/weekly/monthly windows. Legacy HTML scraping stays as fallback. Zen: opencode v2 removed the parseable text summary table (│Total Cost ...), breaking the table parser. Parse 'opencode stats --days N --models --json' first (model keys rebuilt as providerID/modelID so the Zen prefix filter is unchanged) and fall back to legacy text parsing for older CLIs. Fixes opgginc#155
❌ AI Code Review CancelledNewer commit detected or workflow was manually cancelled. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792019760734, 1792020124319
section DependencyContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818705, 1792019861089
section lane-a-slop
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818710, 1792019865946
section lane-b-dry
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818723, 1792019892396
section lane-c-reviewer
claude-opus-5-5 : 1792019818715, 1792019997438
section lane-d-ponytail
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818693, 1792019865780
section RepoContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818712, 1792019895648
section ResolvedIssues
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792019818720, 1792019836677
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
Converge with opgginc#154 (credit sephalias for the endpoint find, verified live): GET /zen/go/v1/usage with the existing opencode-go API key needs no workspace ID, cookies, or extra setup. Keeps the change minimal — existing window structs and legacy HTML parser/tests stay.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792020262050, 1792020677407
section DependencyContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321657, 1792020393081
section lane-a-slop
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321662, 1792020382226
section lane-b-dry
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321664, 1792020376175
section lane-c-reviewer
claude-opus-5-5 : 1792020321633, 1792020532649
section lane-d-ponytail
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321652, 1792020373321
section RepoContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321620, 1792020398233
section ResolvedIssues
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792020321659, 1792020337366
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
There was a problem hiding this comment.
Summary
ngl this is a solid fix for #155. Both breakages target the actual upstream cause instead of papering over symptoms:
- OpenCode Go: switching from scraping a now-SPA dashboard to the key-based
GET /zen/go/v1/usageis the right call. It also deletes the whole browser-cookie + browser-history + workspace-ID discovery path, which means the app no longer goes poking around in Chrome/Arc/Edge cookie and history DBs just to show a percentage. That's a real privacy and attack-surface win, not just a bug fix. Error classification survives too, because it reusesfetchData(401/403 →authenticationFailed). - OpenCode Zen: rebuilding JSON model keys as
providerID/modelIDsoisOpenCodeZenModeland the commandcode exclusion keep working unchanged is clean. The test that pipesparseStatsJSONthroughadjustStatsForDisplayand asserts the excluded cost end to end is exactly the right test. Routing stderr tonullDeviceso CLI warnings can't corrupt the JSON on stdout is a nice touch. parseGoUsageAPIis a pure static, accepts number-or-string percents, keeps partial windows, and has malformed-shape tests. clean.
Concrete benefit: Go usage windows and Zen cost show up again on current upstream instead of hard-failing.
What still needs work (non-blocking, but please fix)
- Stale docs are the big one. README,
docs/AI_USAGE_API_REFERENCE.md, andscripts/query-opencode-go.shstill tell users to set up workspace IDs and auth cookies that the app now completely ignores (see inline). - Dead legacy code: the HTML scraper stack, workspace-ID extractors,
OpenCodeGoDashboardCredentials, and the twoBrowserCookieServicemethods this PR orphaned are all still here. The commit message says the retention is deliberate, but "keep the diff minimal" doesn't justify keeping a dead feature alive through its tests. - Zen fallback can still surface the exact
Cannot parse total costerror from #155 while hiding the real JSON-path failure. - Copy-paste:
runOpenCodeStatsJSONis a ~50-line twin ofrunOpenCodeStats, anddoubleValue/isoDateduplicateAPIValueParser.
Smaller things outside the inline comments:
- On the JSON path, avg/day is now
total / 7and the per-model Messages row is gone (JSON has no message counts). That's probably fine since the v2 text table is gone anyway, but it's a visible UI change worth a line in the PR description. - The comment block in
fetch()(is now a JS SPA shell,API errors propagate with their classification intact.) is PR-description history. One sentence about what the code does is enough; the backstory belongs in the commit message. - Only the parsers are tested.
init(session:)is already injectable, so aURLProtocolstub test for "usage endpoint 401 →authenticationFailed" would be cheap.
Verification signal
- Smoke tests: skipped (
fork-source-unavailable). This is a fork PR and the isolated smoke job could not fetch the source, so this review has no local test/typecheck signal. The author reports 21xcodebuild testpasses for the two provider test classes. - CI on the head SHA:
Lintpassed.Test,CI, andBuild and Releasewere still in progress when this review's data was captured, so double-check them before merging. - No dependency or manifest changes. No known-critical security exposure found in existing manifests.
Verdict
Approving. This restores two broken providers and shrinks the credential surface, and none of the remaining issues is a regression from the (fully broken) base. That said, please fix the docs before or right after merge. Shipping a README that tells people to paste browser auth cookies into a config file the app no longer reads is not great.
| return ProviderResult(usage: usage, details: details) | ||
| } | ||
|
|
||
| // MARK: - Key-based usage API (current) |
There was a problem hiding this comment.
Legacy dashboard scraper left dead: tests keep a removed feature alive
The (current) label gives it away: the legacy path is still here, it's just dead. With fetchDashboardUsage / fetchFirstDashboardUsage / dashboardCredentialCandidates deleted, nothing in production calls any of these anymore:
parseDashboardUsageHTMLplusparseWindow,captureObjectBody,captureNumber, andnormalizedDashboardHTML(~90 lines of regex scraping for markup the PR says no longer exists)extractWorkspaceIDs/extractWorkspaceIDprivate struct OpenCodeGoDashboardCredentials(zero references)BrowserCookieService.getCookies(hostSuffix:names:)andgetHistoryEntries(hostSuffix:pathPrefix:limit:)(plusBrowserHistoryEntry). This PR removed their only caller.
The only things keeping the HTML parser and workspace extractor alive are their tests, and tests passing on dead code don't make it live. The commit message says the retention is intentional to keep the diff minimal, but a smaller diff that leaves a whole dead feature behind isn't actually less code. Every future reader now has to work out which Go parser is real.
Fix: delete the functions above and their now-obsolete tests (testWorkspaceIDExtraction... and the HTML-markup parser tests), and keep only parseGoUsageAPI. If you really want a rollback path, that's what git history is for.
| let (dashboardUsage, credentialSource) = try await fetchFirstDashboardUsage(from: credentialCandidates) | ||
| // The /workspace/<id>/go dashboard is now a JS SPA shell with no | ||
| // server-rendered usage data, so usage comes from the key-based | ||
| // usage API instead. No workspace ID, cookies, or extra setup needed. |
There was a problem hiding this comment.
Go setup docs still require cookies: users configure ignored settings
This PR drops OPENCODE_GO_WORKSPACE_ID, OPENCODE_GO_AUTH_COOKIE, OPENCODE_GO_CONFIG_FILE, the opencode-go.json config paths, and browser-cookie discovery. The user-facing docs still say that setup is required:
README.md:233andREADME.md:402("live usage also needsOPENCODE_GO_WORKSPACE_IDandOPENCODE_GO_AUTH_COOKIE...")docs/AI_USAGE_API_REFERENCE.md:291-309: the whole dashboard/cookie section plus the env overrides listscripts/query-opencode-go.sh: still scrapes/workspace/<id>/gowith cookies, and its:605message tells users to set the workspace ID and cookie
So users will keep pasting browser auth cookies into a config file the app never reads again, and the per-provider reference script (AGENTS.md says scripts/ is the source for quota queries) now disagrees with the app. Company policy also says stale README isn't acceptable.
Fix: rewrite the Go sections in README and the API reference to describe GET https://opencode.ai/zen/go/v1/usage with the opencode-go key from auth.json and its usage.{rolling,weekly,monthly}.{percent,resetsAt} shape. Then point query-opencode-go.sh at the same endpoint and delete its cookie/workspace flags.
| private static func doubleValue(_ value: Any?) -> Double? { | ||
| if let number = value as? NSNumber { return number.doubleValue } | ||
| if let string = value as? String { return Double(string) } | ||
| return nil | ||
| } | ||
|
|
||
| private static func isoDate(_ value: String?) -> Date? { | ||
| guard let value else { return nil } | ||
| let withFraction = ISO8601DateFormatter() | ||
| withFraction.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
| if let date = withFraction.date(from: value) { return date } | ||
| return ISO8601DateFormatter().date(from: value) | ||
| } |
There was a problem hiding this comment.
Parse helpers duplicate APIValueParser: date/number rules drift
Both helpers already exist in the shared APIValueParser (Models/ProviderResult.swift:1443):
isoDateis the same fractional → plainISO8601DateFormatterfallback asAPIValueParser.parseDate(from:), which also trims input and handles date-only strings. Grok/CommandCode/Kiro already use the shared one. It also builds up to two formatters per window on every refresh.doubleValueis copied byte-for-byte intoOpenCodeZenProvider.swift:364. That's two private copies of the NSNumber/String coercion in one PR.
Fix: call APIValueParser.parseDate(from:) here and delete isoDate. For doubleValue, either add a single-value Double? variant next to APIValueParser.parseDouble and use it from both providers, or at least keep a single shared copy instead of two private ones.
| /// Runs `opencode stats --days N --models --json` and returns raw JSON data. | ||
| /// OpenCode v2 removed the parseable text summary table, so this is the | ||
| /// primary stats source; text parsing remains as fallback for older CLIs. | ||
| private func runOpenCodeStatsJSON(days: Int) async throws -> Data { |
There was a problem hiding this comment.
Stats runner duplicated: process-lifecycle fixes must land twice
ctrl+c ctrl+v said hi. runOpenCodeStatsJSON is a near-verbatim copy of runOpenCodeStats (line 218): same opencodePath guard, same killStaleOpenCodeStatsProcesses(), same Process / Pipe / readabilityHandler / terminationHandler / continuation body. The only real differences are the extra --json argument, stderr going to nullDevice, and returning Data instead of a UTF-8 String.
That zombie-reaping and process-lifecycle code is exactly the part that has already needed bug fixes (see the stale-process reaper comment). Now the next hang or leak fix has to land in two places, and it'll drift.
Fix: give one runner the parameters, e.g. runOpenCodeStats(days:json:) -> Data with json controlling --json and stderr routing. Have the text caller decode UTF-8 itself, then delete the twin.
| } catch { | ||
| logger.warning("OpenCode Zen: JSON stats failed (\(error.localizedDescription, privacy: .public)), falling back to text parse") | ||
| debugLog("JSON stats failed, falling back to text parse: \(error.localizedDescription)") | ||
| let output = try await runOpenCodeStats(days: 7) | ||
| stats = try parseStats(output) |
There was a problem hiding this comment.
Fallback masks JSON error: users see the old #155 message again
This catch treats every JSON-path failure the same way: non-zero CLI exit, launch failure, stray stdout before the JSON, or a missing cost key all fall through to the legacy text parser. On a v2 CLI the text parser always throws Cannot parse total cost, which is the exact opaque #155 error this PR is fixing. The real cause only goes to the log, so the user sees the old misleading message again and the bug report will point at the wrong thing.
On legacy CLIs, every refresh now starts two opencode stats processes back to back, and both count against the same fetch timeout.
Fix: if the fallback also fails, rethrow the original JSON error (or include it in the thrown message), e.g.
} catch let jsonError {
logger.warning(...)
do {
stats = try parseStats(try await runOpenCodeStats(days: 7))
} catch {
throw jsonError
}
}Even better, only fall back when the output isn't JSON at all, so a v2 CLI that exits non-zero doesn't pay for a second process.
| let percent = doubleValue(window["percent"]) else { | ||
| return nil | ||
| } | ||
| let resetDate = isoDate(window["resetsAt"] as? String) ?? now |
There was a problem hiding this comment.
Missing resetsAt becomes now: menu shows a fake reset time
If resetsAt is missing or unparseable, this sets the reset date to now. The menu then shows "Resets: " and computes pace against a window with zero time left. That's made-up data shown to the user as fact.
createUsageWindowRow already accepts resetDate: Date? = nil and just skips the reset row, and DetailedUsage.fiveHourReset / openCodeGoMonthlyReset are optional too. So the cleaner fix is to make OpenCodeGoUsageWindow.resetDate optional (and resetInSeconds optional or 0) and pass nil through. Log the omission so it shows up if the API shape changes.
| private let session: URLSession | ||
| private let modelsURL = URL(string: "https://opencode.ai/zen/go/v1/models")! | ||
| private let usageURL = URL(string: "https://opencode.ai/zen/go/v1/usage")! | ||
| private let usageSourceLabel = "OpenCode Go API (zen/go/v1/usage)" |
There was a problem hiding this comment.
Source label shows endpoint path: odd UI text and a dead parameter
This becomes authUsageSummary, which the menu renders as Using in: OpenCode Go API (zen/go/v1/usage) (ProviderMenuBuilder.swift:1037). Other providers use plain names like "Cursor" and "Grok CLI", so this is the only one leaking an endpoint path into the UI. The literal also restates usageURL one line above, so the two will drift if the endpoint moves.
Also, makeResult(credentialSource:) now always receives this one constant. The parameter only existed when credentials varied per candidate, which they no longer do.
Fix: use a plain label like "OpenCode Go API", inline it in makeResult, and drop the credentialSource parameter.
- Delete the dead dashboard HTML scraper, workspace-ID extractors, orphaned credentials struct, and their obsolete tests; remove the now-callerless BrowserCookieService history reader + entry type. - Rewrite README/API reference/query-opencode-go.sh for the key-based zen/go/v1/usage endpoint (no workspace/cookie setup). - Reuse APIValueParser (new optional parseDouble overload) instead of local number/date helpers in both providers. - Unify the opencode stats runner (days:json:) so lifecycle fixes land once; fallback rethrows the original JSON error when text also fails. - Missing resetsAt now yields a nil reset date instead of now. - Plain 'OpenCode Go API' source label; drop credentialSource param.
|
All 7 review findings addressed in 364fe80 (tests green locally, CI running):
|
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Initial model: Model usage by lane
gantt
dateFormat x
axisFormat %H:%M
todayMarker off
section Main
claude-opus-5-5 : 1792023627815, 1792024115703
section ContextAudit
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681700, 1792023896431
section ContextAudit.lane-e-ponytail-audit
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023775509, 1792023892480
section ContextAudit.ResolvedIssues
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023687472, 1792023764432
section DependencyContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681708, 1792023740197
section lane-a-slop
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681730, 1792023778990
section lane-b-dry
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681716, 1792023786309
section lane-c-reviewer
claude-opus-5-5 : 1792023681719, 1792023782990
section lane-d-ponytail
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681734, 1792023781650
section RepoContext
opengateway/deepseek/deepseek-v4.1-flash-ultrafast : 1792023681711, 1792023793492
Input excludes cached tokens. Output includes reasoning tokens. Speed reflects the requested API tier; provider acceptance is not recorded. |
There was a problem hiding this comment.
Summary
Round 2, and honestly this follow-up commit is clean. All 7 findings from last round are actually fixed in the code, not just answered in a reply:
- The dead dashboard HTML scraper, workspace-ID extractors, credentials struct, browser-history reader, and their tests are all gone. A repo-wide grep finds zero leftovers.
- README,
docs/AI_USAGE_API_REFERENCE.md, andscripts/query-opencode-go.shnow describe the key-basedzen/go/v1/usageflow. No more cookie/workspace setup instructions. - A missing
resetsAtnow staysnilinstead of turning into a fake "resets now", and there's a test for it. - There's one
runOpenCodeStats(days:json:)runner instead of two copies, and the JSON-path error is rethrown when the text fallback also fails, so users won't see the misleading #155 message again. - The source label is a plain
OpenCode Go API, and the private parse helpers were folded intoAPIValueParser.
Net diff is +294 / −945. Most of the removed code is the code that read Chrome/Arc/Edge cookie and history databases just to show a percentage, so you get less code, less privacy exposure, and an approach that works with the current upstream. Solid work.
What's left (non-blocking)
The inline comments are cleanup. Most of it is code from the old dashboard approach that the API switch didn't fully remove:
- Go provider:
makeResulthas only one caller and re-checks a guard the parser already enforces.missingWindowNamesstill reports the old HTML field names (rollingUsage/…). APIValueParser: it now has two separate numeric coercion paths, and both new parsers copy the same 9-line JSON-root preamble.query-opencode-go.sh: a 45-line python heredoc parses JSON even thoughjqis already required. The non-2xx error block is copied in two places. When parsing fails, the reason gets swallowed, andusage_errorwas removed from--jsonoutput without a replacement.- Zen: the JSON→text fallback and
parseStatsJSON's summing/malformed-input branches have no tests. The new test's "commandcode excluded" comment doesn't match whatexcludedCostactually covers.
Not on a diff line, so it goes here: BrowserCookie.profileKey and BrowserCookie.displaySource (Services/BrowserCookieService.swift:602-608) have no callers since the browser-dashboard candidate code was removed. Delete them in this PR so the cleanup is complete.
Validation
- Smoke tests: skipped (
fork-source-unavailable). The fork source couldn't be fetched for the isolated smoke job, so I have no local test/typecheck signal for this round. - CI on
364fe80: Lint ✅. Test, CI, and Build and Release were still pending when this review ran. Since smoke was skipped, thexcodebuild testrun is the only check on the new parsers, so make sure it's green before merging. - Benefit check: restores OpenCode Go usage (the SPA dashboard broke scraping) and OpenCode Zen cost (opencode v2 removed the text table). That's a real user-facing fix for #155.
Approving. Nothing left blocks the merge, and the open items are cleanup that can land here or in a follow-up.
Additional findings
4 findings — expand
- delete: OpenCodeZenProvider keeps a complete second parser for the superseded `opencode stats` text table (parseModelUsageStats and its helpers) reachable only through the JSON-failure fallback, so every stats change needs two parsers and two test surfaces for one CLI endpoint. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeZenProvider.swift, CopilotMonitor/CopilotMonitorTests/OpenCodeZenProviderTests.swift]
- shrink: BrowserCookieService still holds two near-identical cookie readers (readCookies and readBrowserCookies) repeating the temp-copy, read-only SQLite open, decrypt and plaintext-fallback sequence, while the generic host/name-filtered reader already covers the GitHub case the specific one exists for. [CopilotMonitor/CopilotMonitor/Services/BrowserCookieService.swift]
- delete: OpenCodeZenProvider's debugLog writes to a /tmp file in every build because it omits the #if DEBUG guard the repository's other debug loggers use, shipping development-only file logging into release fetch cycles that structured os.log already covers. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeZenProvider.swift]
- delete: OpenCodeGoUsageWindow carries resetInSeconds, recomputed from resetDate at parse time yet read by nothing outside tests, leaving two stored representations of one API value. [CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift, CopilotMonitor/CopilotMonitorTests/OpenCodeGoProviderTests.swift]
Unresolved last round (not re-raised this round)
CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift— Missing resetsAt becomes now: menu shows a fake reset time (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeZenProvider.swift— Fallback masks JSON error: users see the old #155 message again (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift— Go setup docs still require cookies: users configure ignored settings (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift— Source label shows endpoint path: odd UI text and a dead parameter (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift— Legacy dashboard scraper left dead: tests keep a removed feature alive (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeZenProvider.swift— Stats runner duplicated: process-lifecycle fixes must land twice (round 1)CopilotMonitor/CopilotMonitor/Providers/OpenCodeGoProvider.swift— Parse helpers duplicate APIValueParser: date/number rules drift (round 1)
| let jsonData = try await runOpenCodeStats(days: 7, json: true) | ||
| stats = try Self.parseStatsJSON(jsonData, days: 7) | ||
| debugLog("Parsed stats via --json") | ||
| } catch let jsonError { |
There was a problem hiding this comment.
JSON→text fallback untested: error-precedence fix can silently regress
The interesting new behavior here is the fallback logic: JSON fails → text runs → if text also fails, rethrow the JSON error. That's the actual fix for the misleading #155 message. It has zero coverage, because it's welded to Process, so a refactor that swaps throw jsonError for throw error would pass every test.
The smallest seam: pull the decision into a static helper that takes the two attempts as closures/results, e.g. static func resolveStats(json: () async throws -> OpenCodeStats, text: () async throws -> OpenCodeStats), and add two tests: JSON fails + text succeeds → text stats; both fail → the JSON error comes back.
Also worth deciding: this keeps the whole ~160-line legacy table parser alive just for pre-v2 CLIs, and every v2 failure now spawns a second opencode stats process. If you know a CLI version floor, a one-time version check is less surface than two parsers.
Tiny nit at line 317: days > 0 ? … : 0 guards a parameter that's always 7 at both call sites.
| /// Optional single-value coercion (Double/Int/NSNumber/numeric String). | ||
| /// Nil when the value is missing or not numeric. | ||
| static func parseDouble(_ value: Any?) -> Double? { |
There was a problem hiding this comment.
Two coercion ladders in APIValueParser: parsing rules drift
Good call reusing APIValueParser instead of private helpers. But the new parseDouble(_:) now sits next to parseDouble(from:keys:), which has its own separate Double/Int/NSNumber/String ladder. Two coercion rules in the same type will drift: a fix to one (trimming whitespace, say) won't reach the other.
Make the dict variant delegate:
static func parseDouble(from dict: [String: Any], keys: [String]) -> Double {
for key in keys {
if let value = parseDouble(dict[key]) { return value }
}
return 0.0
}Related: OpenCodeZenProvider.parseStatsJSON (lines 292-300) and OpenCodeGoProvider.parseGoUsageAPI (lines 132-141) copy the same 9-line JSONSerialization → decodingError → as? [String: Any] preamble, and only the label differs. A small APIValueParser.jsonObject(from:label:) throws -> [String: Any] would put that rule in one place instead of two (and stop a third copy appearing with the next parser).
| // A missing/unparseable resetsAt stays nil so the menu shows no | ||
| // reset row rather than a made-up reset time. | ||
| let resetDate = APIValueParser.parseDate(from: window["resetsAt"] as? String) | ||
| let resetInSeconds = resetDate.map { max(0, Int($0.timeIntervalSince(now).rounded())) } ?? 0 |
There was a problem hiding this comment.
resetInSeconds duplicates resetDate: test-only state
resetInSeconds is now derived from resetDate, and nothing in the app reads it. Menu consumers only use resetDate (fiveHourReset / sevenDayReset / openCodeGoMonthlyReset). The only readers are the two test assertions. On top of that, the ?? 0 fallback means "unknown reset" and "resets right now" look identical. That's exactly the ambiguity the resetDate: Date? change was meant to remove.
Drop the resetInSeconds field from OpenCodeGoUsageWindow, and have the tests assert on resetDate (or compute resetDate.timeIntervalSince(now) inline).
| fi | ||
|
|
||
| python3 - "$html_file" <<'PY' | ||
| python3 - "$body_file" <<'PY' |
There was a problem hiding this comment.
python3 heredoc next to required jq: extra runtime and lost error reason
The script already requires jq (line 318) and uses it for error extraction and output formatting. Yet fetch_usage still runs a 45-line python3 heredoc to read three numbers and compute a duration, so there's a second hard dependency (require_command python3). The python was only there because the old HTML scraper needed regex; with a JSON API, jq covers it (.usage.rolling.percent, .resetsAt | sub("\\.[0-9]+Z$"; "Z") | fromdateiso8601, a small def dur for Xd Yh).
Two more things in the same function:
- The non-2xx block (
jq -r '.error.message // .message // .error // empty'→rm→HTTP $status from …fallback) is copy-pasted betweenvalidate_models_api(lines 158-164) and here (lines 181-186). Fold it intoapi_getor a smallerror_message_fromhelper. - When parsing fails (
sys.exit(2)with{"error": "No OpenCode Go usage windows…"}on stdout),mainthrows that away. Text mode then printsusage API request failed, which is wrong because the request succeeded. And--jsonoutput no longer hasusage_errorat all, so consumers getusage: nullwith no reason. Either write the parse error to stderr like the HTTP path does, or putusage_errorback in the JSON output.
Nit: "OpenCode Go API (zen/go/v1/usage)" is hardcoded twice (lines 277, 310) even though USAGE_URL already owns that path.
| tokenManager: tokenManager | ||
| ) | ||
| } | ||
| private static func makeResult( |
There was a problem hiding this comment.
Single-caller makeResult with an unreachable guard: dashboard-era leftovers
makeResult has exactly one caller (fetch() right above). Its first guard (guard !dashboardUsage.usagePercents.isEmpty, line 71) can never fire: parseGoUsageAPI already throws on that exact condition (line 149), and it's the only path that feeds this function. So there are two copies of the same check with two different error messages, and one of them is dead.
While you're here, the window vocabulary is also stuck in the HTML era. missingWindowNames still appends "rollingUsage" / "weeklyUsage" / "monthlyUsage", the field names of the deleted dashboard markup. The API calls them rolling / weekly / monthly (lines 144-146), and the new test at OpenCodeGoProviderTests.swift:46 locks in the stale names. Same story for the type name OpenCodeGoDashboardUsage.
Fix:
- inline the
makeResultbody back intofetch(), or keep it but delete the duplicate guard and letparseGoUsageAPIstay the only place that enforces it - rename the missing-window labels to
rolling/weekly/monthly(ideally derived from the same key list the parser reads) and update the test to match
| modelCosts: stats.modelCosts, | ||
| modelMessages: stats.modelMessages | ||
| ) | ||
| // commandcode cost must be excluded from Zen totals |
There was a problem hiding this comment.
Misleading exclusion comment: summing branch has no test
The comment says commandcode cost is what gets excluded, but the excludedCost assertion two lines down checks $7.33. Only $1.25 of that is commandcode; the other ~$6.08 is top-level cost that no model row accounts for (16.7118 vs. a model sum of 9.3849). Someone reading this later will assume excludedCost == commandcode and "fix" either the fixture or the code. The test also never checks that commandcode/deepseek/deepseek-v4.1-flash was parsed and then dropped from adjusted.modelCosts, so the KeepsZenFilter part of the name is only proven indirectly.
Then there's the branch that matters most: parseStatsJSON sums repeated providerID/modelID keys (OpenCodeZenProvider.swift:313). A regression to overwrite-instead-of-sum would quietly under-count Zen cost and nothing here would catch it.
Fix:
- assert
adjusted.modelCosts["commandcode/…"] == nil, and either make the fixture's models sum tocostor update the comment to explain the unattributed remainder - add a small table test covering duplicate-key summing, string-encoded costs, skipped empty IDs, invalid JSON, and a non-object root
Fixes #155. Supersedes / converges with #154 on the Go side (see note below).
Two independent upstream changes broke these providers on v2.12.0:
OpenCode Go —
No usage windows were foundThe
/workspace/<id>/godashboard is now a JS SPA shell with no server-rendered usage data, so HTML scraping can never succeed. This switches usage fetching to the key-based usage API (GET https://opencode.ai/zen/go/v1/usagewith the existingopencode-goAPI key from auth.json — no workspace ID, cookies, or extra setup), which returns rolling (5h) / weekly / monthly windows with percent used + resetsAt. Verified live. API errors propagate with their classification intact. The dead scraper stack, workspace extractors, orphaned cookie/history plumbing, and their tests are deleted; README, API reference, andquery-opencode-go.shnow describe the key-based endpoint.Note: #154 (sephalias) independently discovered the same endpoint — I verified it live and converged this PR onto it (credit to sephalias for the find). This PR differs from #154 by keeping the existing window structs and additionally fixing Zen below, so either PR's Go approach merges cleanly.
OpenCode Zen —
Cannot parse total costopencode v2 removed the parseable text summary table, so the table parser fails on current CLIs. This parses
opencode stats --days N --models --jsonfirst (model keys rebuilt asproviderID/modelIDso the existing Zen prefix filter and commandcode exclusion are unchanged) and falls back to legacy text parsing for older CLIs; if both fail, the original JSON error is rethrown. Note: on the JSON path avg/day is total/7 and per-model message counts are unavailable (JSON exposes no message counts), so the Messages row is gone.Verification
xcodebuild test: OpenCodeZenProviderTests + OpenCodeGoProviderTests — all pass.opencode statstotals; Go windows + 37-model validation return live data; ChatGPT/Copilot/DeepSeek rows unchanged.