Repository navigation
fix(opencode): include cache read and write usage totals - #21886
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughOpenCode usage queries now include Priority: ➖ Normal Severity of issue fixed: Medium 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The pull request also changes relay plugin installation and overlay behavior, OpenCode hook configuration, shared config-directory resolution, related tests, and the SSH localhost E2E test. The summary identifies these changes in
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 1f81ddcc-d65e-4455-8adb-05b9811b94d5
📒 Files selected for processing (11)
src/main/opencode-usage/opencode-usage-row-queries.tssrc/main/opencode-usage/scanner.test.tssrc/main/opencode/hook-service.test.tssrc/main/opencode/hook-service.tssrc/relay/omp-status-extension.tssrc/relay/opencode-canonical-config.tssrc/relay/plugin-overlay.test.tssrc/relay/plugin-overlay.tssrc/relay/relay-agent-hook-runtime.tssrc/shared/opencode-config-directory.tstests/e2e/ssh-localhost.spec.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
|
|
||
| function getSessionTokenTotalExpression(db: Database.Database): string { | ||
| const cacheWrite = columnExists(db, 'session', 'tokens_cache_write') | ||
| ? ' + tokens_cache_write' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
-e 'tokens_cache_write' \
-e 'CREATE TABLE session' \
-e 'ALTER TABLE session' \
--glob '*.ts' --glob '*.sql' .Repository: stablyai/orca
Length of output: 35206
Normalize nullable cache-write tokens before calculating totals.
If session.tokens_cache_write is NULL, SQLite makes the total expression NULL. The WHERE ... > 0 predicate then excludes sessions with other valid token usage. Use COALESCE(tokens_cache_write, 0) in the total expression and COALESCE(s.tokens_cache_write, 0) in the selected value.
| env.ORCA_OPENCODE_SOURCE_CONFIG_DIR = sourceDir | ||
| } | ||
| } else { | ||
| this.pluginOverlay.installOpenCodePlugin(opencodeAgent, context.env) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the source-directory resolver and the PTY environment merge contract.
resolver_file="$(rg -l --type ts '\bresolveOpenCodeSourceConfigDir\b' src | head -n 1)"
if [ -n "$resolver_file" ]; then
ast-grep outline "$resolver_file" --items all --type function
rg -n -C 12 --type ts '\bresolveOpenCodeSourceConfigDir\b|OPENCODE_CONFIG_DIR' "$resolver_file"
fi
rg -n -C 10 --type ts '\baddEnvAugmenter\s*\(|PtyEnvAugmenter|OPENCODE_CONFIG_DIR' src
rg -n -C 8 --type ts 'isRelayOverlayPath|\.orca-relay|installOpenCodePlugin' src/relay testsRepository: stablyai/orca
Length of output: 45541
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- resolver declarations/usages ---'
rg -n -C 18 --type ts 'function resolveOpenCodeSourceConfigDir|const resolveOpenCodeSourceConfigDir|resolveOpenCodeSourceConfigDir\s*=|resolveOpenCodeSourceConfigDir\(' src
printf '%s\n' '--- canonical installer ---'
sed -n '1,180p' src/relay/opencode-canonical-config.ts
printf '%s\n' '--- runtime caller and context/env construction ---'
sed -n '1,150p' src/relay/relay-agent-hook-runtime.ts
rg -n -C 16 --type ts 'RelayAgentHookRuntime|new RelayAgentHookRuntime|addEnvAugmenter|envAugmenter|PtyEnvAugmenter' src/relay src/mainRepository: stablyai/orca
Length of output: 42664
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- canonical config implementation ---'
sed -n '1,150p' src/relay/opencode-canonical-config.ts
printf '%s\n' '--- config-directory resolver ---'
rg -n -C 20 --type ts 'resolveOpenCodeConfigDirectory' src/shared src/relay
printf '%s\n' '--- relay overlay predicate and materialization ---'
rg -n -C 18 --type ts 'isRelayOpenCodeOverlayPath|materializeOpenCode\(' src/relay/opencode-canonical-config.ts src/relay/plugin-overlay.tsRepository: stablyai/orca
Length of output: 15248
Ensure relay-overlay launches use the selected plugin.
When sourceDir is an inherited OPENCODE_CONFIG_DIR under .orca-relay, this branch installs the selected plugin in the canonical config directory but returns no OPENCODE_CONFIG_DIR override. The child can therefore continue using the inherited relay overlay and load a stale plugin or the wrong OpenCode variant. Update the effective overlay or replace its config override after installation.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The cache-token fix itself is correct, but the branch also carries fix(opencode): preserve global config discovery — a behavioral rewrite of OpenCode plugin installation that the PR description does not cover (it even states there are no SSH/platform changes). Two issues in that commit: a v1 opencode process now loads the OpenCode 2 plugin too, and the WSL guest path still overrides OPENCODE_CONFIG_DIR.
Reviewed changes
- OpenCode cache-read/write in session totals —
selectSessionUsageRowsnow readstokens_cache_write, includes both cache counters in the serializedtotal, and the row-count query counts write-only sessions; DBs without the write column fall back to0.cache-read-reproduction.test.tsalready covers the missing-column path. - Status plugin installed into OpenCode's real config root — local
OpenCodeHookService.buildPtyEnvand the SSH relay writeorca-opencode-status.js/orca-opencode2-status.jsinto$XDG_CONFIG_HOME/opencode/pluginsand stop settingOPENCODE_CONFIG_DIR; the overlay path is kept only when the user set a config dir. - Supporting extraction — new
resolveOpenCodeConfigDirectory,installOpenCodePluginInCanonicalConfig/isRelayOpenCodeOverlayPath, andwriteRelayOmpStatusExtension; the SSH e2e probe now checks the canonical root.
⚠️ WSL guest path still overrides OPENCODE_CONFIG_DIR with a plugin-only overlay
The config-discovery fix covers the local and SSH relay paths but leaves the WSL guest untouched. The host still points OPENCODE_CONFIG_DIR at the guest overlay (src/main/ipc/pty/host-env/assembly.ts:144-149), and createInstallPluginsHandler deliberately declines to mirror the default root (src/relay/wsl-install-plugins-handler.ts:29-33). Upstream resolves Global.config = Flag.OPENCODE_CONFIG_DIR ?? Path.config, so that env var replaces the XDG root rather than appending — a WSL user who has not set OPENCODE_CONFIG_DIR therefore gets a config root containing only Orca's plugin, and their ~/.config/opencode auth, models, keybinds, and plugins disappear. That is the exact failure the other two paths are being fixed for. The handler comment and wsl-install-plugins-handler.test.ts:176-194 encode the opposite ("APPENDS") premise.
Technical details
# WSL still replaces the guest's global OpenCode config root
## Affected sites
- `src/main/ipc/pty/host-env/assembly.ts:144-149` — sets `OPENCODE_CONFIG_DIR` to `getOpenCodeOverlayDir(...)` whenever the relay returned an overlay dir.
- `src/relay/wsl-install-plugins-handler.ts:29-33, 72-91` — materializes a plugin-only overlay when the guest has no explicit `OPENCODE_CONFIG_DIR`; the comment asserts OpenCode appends.
- `src/relay/wsl-install-plugins-handler.test.ts:176-194` — pins the "does not mirror the XDG default" behavior.
## Evidence
- `https://raw.githubusercontent.com/anomalyco/opencode/dev/packages/core/src/global.ts` — `config: Flag.OPENCODE_CONFIG_DIR ?? Path.config`.
- `https://raw.githubusercontent.com/anomalyco/opencode/dev/packages/core/src/flag/flag.ts` — `OPENCODE_CONFIG_DIR` is `process.env["OPENCODE_CONFIG_DIR"]`.
- `https://raw.githubusercontent.com/sindresorhus/xdg-basedir/main/index.js` — `xdgConfig` defaults to `~/.config` on every platform.
## Required outcome
- A WSL `opencode`/`opencode2` process launched without an explicit config dir must still read `~/.config/opencode`, with Orca's plugin added alongside it — the same contract the local/SSH paths now have.
## Open questions for the human
- Is WSL intentionally out of scope for this commit, or was it missed? If intentional, the "APPENDS" comment and the corresponding test should be corrected, since they document the opposite of upstream behavior.ℹ️ The PR description describes only half the diff
The title, "What Changed", and "Notes" describe a change "local to Orca's SQLite usage reader" with "No remote protocol, SSH, mobile, or platform-specific process behavior changes." In fact the branch contains a second commit that changes SSH relay behavior, plugin installation on three host paths, and platform config discovery. A reviewer merging this cannot assess the risk from the description; either split the config-discovery commit into its own PR or document it here.
ℹ️ Nitpicks
src/main/opencode-usage/opencode-usage-row-queries.ts:103—tokenTotalExpression.replaceAll('tokens_', 's.tokens_')builds aliased SQL by string substitution; passing an alias/prefix intogetSessionTokenTotalExpressionwould make the coupling explicit instead of relying on every column starting withtokens_.src/relay/omp-status-extension.ts:5,8,10— hardcodes.orca-relay,omp-managed-status-extension,orca-agent-status.ts, and@orca-managed-pi-extension, duplicating the constants defined inplugin-overlay.ts; the two can drift.src/main/opencode/hook-service.ts:124— the local install resolves the config root from the Orca process env, while the relay passes the PTY env (context.env) intoresolveOpenCodeConfigDirectory. If a shell rc exports a differentXDG_CONFIG_HOMEthan the Orca main process sees, the write root and OpenCode's read root diverge.src/relay/opencode-canonical-config.ts:21-26unlinks the target before writing (symlink guard) while the localwritePluginToConfigDir(src/main/opencode/hook-service.ts:259-263) does not; worth making the two consistent.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — one redundant lint suppression inline.
Reviewed changes (incremental)
This run covers the one commit added since the prior review at 19ae781:
fix(opencode): satisfy aggregate query safety checks(1d4ca87) — adds fouroxlint-disable-next-line typescript/consistent-type-assertionsdirectives, each with aSAFETY:rationale, around the SQLite.get() as { count?: number }/.all() as OpenCodeSessionUsageRow[]casts insrc/main/opencode-usage/opencode-usage-row-queries.ts. No behavior change. The prior review's two findings (a v1opencodeprocess now loading the OpenCode 2 plugin, and the WSL guest path still overridingOPENCODE_CONFIG_DIR) are unaffected by this commit and remain open.
ℹ️ Nitpicks
src/main/opencode-usage/opencode-usage-row-queries.ts:50,81— theSAFETY:rationale is identical boilerplate at each count cast ("validated by the typed count field below"), butrow?.count ?? 0validates nothing — the cast is unchecked. AGENTS.md asks for a line-specific explanation; one accurate rationale shared across the sites would carry more signal than the repeated claim.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| WHERE tokens_input + tokens_output + tokens_reasoning + tokens_cache_read > 0` | ||
| WHERE ${getSessionTokenTotalExpression(db)} > 0` | ||
| ) | ||
| // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: SQLite aggregate rows are validated by the typed count field below. |
There was a problem hiding this comment.
oxlint reports this directive as unused: the rule reports the assertion at the statement start (const row = db), so the directive on line 81 already covers the cast on line 89. Removing this one keeps the casting scan clean — it is dead suppression.
1d4ca87 to
ec8929c
Compare
There was a problem hiding this comment.
ℹ️ No new issues in the new commits — the branch was rebased and narrowed.
Reviewed changes
This run covers the delta since the prior pullfrog review at 1d4ca87 (which spanned 11 changed files):
- Dropped the config-discovery commit — the plugin-installation rewrite that added
src/relay/opencode-canonical-config.tsnow lives in #21854, so this PR is back to the usage fix plus its tests. - Regenerated the runtime-required English catalog (
ec8929c) — picks up the OpenCode Go session-cookie strings that#21462added toen.json;node config/scripts/generate-runtime-required-english-catalog.mjsnow reports the catalog coversen.json. - Usage fix unchanged —
selectSessionUsageRowsstill readstokens_cache_write, includes cache read and write in the serializedtotal, and the row-count query counts write-only sessions;scanner.test.tsassertstotalTokens: 1925and the no-write-column path stays covered bycache-read-reproduction.test.ts.
The prior review's two substantive findings were about the dropped config-discovery commit, and its inline thread has been resolved as no longer part of this PR. The one remaining open thread is the earlier nitpick about the redundant oxlint-disable at opencode-usage-row-queries.ts:88, which is unchanged by these commits.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ELI5
OpenCode reports cache tokens separately. Orca was dropping those values in its session summary, so usage totals could be far too low.
What Changed
Session-level OpenCode usage now includes both cache-read and cache-write tokens in the serialized cache fields and total token count. Databases from older OpenCode versions without the write column continue to work, and write-only sessions are included.
Why
OpenCode's own usage model treats input, output, reasoning, cache-read, and cache-write as separate counters. Preserving all five keeps Orca's totals aligned with the source data and avoids losing usage during aggregate reads.
Linked Issue
Fixes #20186
Visual Proof
N/A — usage aggregation is a background data path with no UI layout or interaction change.
Testing
Validated with the OpenCode usage scanner suites (20 tests) and
pnpm tc:node.AI Disclosure
Review
Agent skill upstream boundary
Notes
The change is local to Orca's SQLite usage reader and remains compatible with older databases. No remote protocol, SSH, mobile, or platform-specific process behavior changes.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (CI will cover remaining checks)