This repository was archived by the owner on Jul 30, 2026. It is now read-only.
feat(cli): establish deterministic command semantics - #145
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough本次变更将 CLI 重构为显式子命令与执行计划模型，统一本地报告 JSON envelope、错误分类、缓存作用域和 TUI 退出行为，并移除 Cursor/Trae 相关客户端、适配器及账户管理能力；同时更新 Codex 用量读取、Droid 归因、文档和测试。 ChangesCLI 与报告执行
核心边界与数据处理
文档与验证
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Remove Tokscale-owned Cursor and Codex account management, implicit Cursor API refreshes, and OAuth token mutation. Read only the current provider-owned Codex authentication for subscription usage and document the boundary in ADR 0023.
Replace the conflicting --agents and --clients booleans with one typed --ranking option. Keep automatic selection explicit, preserve requested agent views with an empty state, and reject source or presentation combinations that cannot affect the execution plan.
makoMakoGo
commented
Jul 15, 2026
makoMakoGo
left a comment
makoMakoGo
left a comment
Owner
Author
There was a problem hiding this comment.
复核了最新 HEAD。e8094473 已通过移除 Tokscale 自有的 Cursor/Codex 凭证与多账号命令，从结构上关闭原第 1 条风险；375005be 也已用 typed --ranking 消除 Wrapped 布尔优先级。以下 5 条在最新提交上仍可复现，建议在转为 Ready 前处理。
Remove both clients from discovery, parsing, CLI, TUI, Wrapped, credentials, dependencies, tests, assets, and user documentation. Retain only retired parser discriminants so unrelated source-cache shards remain decodable.
Distinguish normal TUI quit from user interruption, restore the terminal before mapping Ctrl-C to exit 130, and cover the behavior through a real PTY regression test.\n\nBroaden ADR 0024 into the Subscription Usage subsystem boundary and redesign record, with Cursor and Trae removal documented as its first applied decision.
Preserve typed settings and environment failures through resolve and execute so malformed configuration returns exit 2 while operational I/O failures remain exit 1. Resolve the headless timeout into its execution plan before starting the child process and cover both classifications with black-box regressions.
Keep the user-provided no-spinner value in report execution plans and derive the effective JSON progress policy only at execution. Cover distinct plans and unchanged quiet JSON behavior with unit and black-box regressions.
Keep invalid request, malformed environment, and operational failures typed across the core report boundary. Parse settings from bytes so invalid UTF-8 is classified as malformed content while real read failures remain operational.
makoMakoGo
marked this pull request as ready for review
July 15, 2026 13:13
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 75 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/tokscale-cli/src/commands/models.rs (1)
70-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win在发生错误时正确清理 Spinner（避免破坏终端输出）
在当前的实现中，如果底层的
get_model_report（或get_hourly_report）异步执行失败，?操作符会导致函数在此处直接提前返回。这会直接跳过下方的spinner.stop()调用，导致终端残留未清理的 Spinner 文本，并可能出现内容交错输出的 UI 问题。
请先接收Result，确保在停止 Spinner 之后再对错误进行解包或返回。
crates/tokscale-cli/src/commands/models.rs#L70-L88: 提取report_result，在调用spinner.stop()之后再执行map_err(...)?。crates/tokscale-cli/src/commands/hourly.rs#L50-L68: 执行相同的重构，确保遇到错误时 Spinner 也能被正常清理。🛠️ 建议的修复方式（以 models.rs 为例）
- let report = rt - .block_on(async { - get_model_report(ReportOptions { - home_dir: home_dir.clone(), - use_env_roots, - clients: clients.clone(), - since: since.clone(), - until: until.clone(), - year: year.clone(), - group_by: group_by.clone(), - scanner_settings, - }) - .await - }) - .map_err(anyhow::Error::new)?; + let report_result = rt + .block_on(async { + get_model_report(ReportOptions { + home_dir: home_dir.clone(), + use_env_roots, + clients: clients.clone(), + since: since.clone(), + until: until.clone(), + year: year.clone(), + group_by: group_by.clone(), + scanner_settings, + }) + .await + }); if let Some(spinner) = spinner { spinner.stop(); } + + let report = report_result.map_err(anyhow::Error::new)?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-cli/src/commands/models.rs` around lines 70 - 88, Update the report execution flow in crates/tokscale-cli/src/commands/models.rs lines 70-88 to store the async call result before applying map_err and returning, then stop the spinner before unwrapping the result. Apply the same refactor to crates/tokscale-cli/src/commands/hourly.rs lines 50-68 so both get_model_report and get_hourly_report clean up their spinner on errors.
🧹 Nitpick comments (2)
crates/tokscale-cli/src/commands/time_metrics.rs (1)
86-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win为
time-metrics --benchmark补一条stderr回归测试。
time-metrics目前只有表格输出用例，还缺少--benchmark下"Processing time"迁移到stderr的断言；补一条与models/monthly对齐的测试，锁住这条 CLI 契约。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-cli/src/commands/time_metrics.rs` around lines 86 - 88, 为 time-metrics 命令的测试补充 benchmark 场景，使用与 models/monthly 测试一致的方式执行 --benchmark，并断言包含 “Processing time” 的输出写入 stderr；同时验证该内容不出现在 stdout，以固定现有 CLI 输出契约。crates/tokscale-cli/src/tui/data/mod.rs (1)
120-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value考虑将重复的
home_dir解析逻辑提取为辅助函数。在这个文件中，解析
home_dir并在缺失时回退到dirs::home_dir()以及设置use_env_roots的逻辑被重复了三次（在此处、第 197 行和第 279 行）。将其提取为一个独立的方法有助于减少重复代码并提高可维护性。♻️ 建议的重构
你可以将这部分提取为一个私有方法：
impl DataLoader { fn resolve_home(&self) -> Result<(String, bool)> { match &self.home_dir { Some(home) => Ok((home.to_string_lossy().into_owned(), false)), None => Ok(( dirs::home_dir() .ok_or_else(|| anyhow::anyhow!("Could not find home directory"))? .to_string_lossy() .into_owned(), true, )), } } // ... }然后在这里直接调用
let (home, use_env_roots) = self.resolve_home()?;即可。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tokscale-cli/src/tui/data/mod.rs` around lines 120 - 129, Extract the duplicated home-directory resolution logic into a private DataLoader method, such as resolve_home, returning the resolved String and use_env_roots flag while preserving the existing error behavior. Replace the three duplicated match blocks, including this location and the corresponding logic near the other call sites, with calls to the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tokscale-cli/src/commands/clients.rs`:
- Line 119: 限制 discover_opencode_dbs 仅在 selected_clients 包含 ClientId::OpenCode
时执行，避免筛选其他客户端时访问 OpenCode 路径或污染 health 结果；同时补充客户端过滤回归测试，覆盖仅选择 Claude 时不触发
OpenCode 发现。
In `@crates/tokscale-cli/src/commands/headless.rs`:
- Around line 107-119: Update the validation in the headless command around
source, child command, and format parsing so invalid arguments are represented
as CliFailure::InvalidConfiguration rather than anyhow errors classified as
Operational. Preserve the existing validation messages and ensure unsupported
sources, empty child commands, and invalid formats return exit code 2.
In `@crates/tokscale-core/src/sessions/droid.rs`:
- Around line 297-358: Update droid_agent_dependency_path so subagents that can
inherit a Validator role from their parent include the parent-chain dependency
used by resolve_droid_agent in cache invalidation, including the mission
features file when an ancestor is a Mission worker, while preserving the
existing transcript dependency. Ensure changes to relevant parent settings or
mission features invalidate the cached role assignment.
---
Outside diff comments:
In `@crates/tokscale-cli/src/commands/models.rs`:
- Around line 70-88: Update the report execution flow in
crates/tokscale-cli/src/commands/models.rs lines 70-88 to store the async call
result before applying map_err and returning, then stop the spinner before
unwrapping the result. Apply the same refactor to
crates/tokscale-cli/src/commands/hourly.rs lines 50-68 so both get_model_report
and get_hourly_report clean up their spinner on errors.
---
Nitpick comments:
In `@crates/tokscale-cli/src/commands/time_metrics.rs`:
- Around line 86-88: 为 time-metrics 命令的测试补充 benchmark 场景，使用与 models/monthly
测试一致的方式执行 --benchmark，并断言包含 “Processing time” 的输出写入 stderr；同时验证该内容不出现在
stdout，以固定现有 CLI 输出契约。
In `@crates/tokscale-cli/src/tui/data/mod.rs`:
- Around line 120-129: Extract the duplicated home-directory resolution logic
into a private DataLoader method, such as resolve_home, returning the resolved
String and use_env_roots flag while preserving the existing error behavior.
Replace the three duplicated match blocks, including this location and the
corresponding logic near the other call sites, with calls to the helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d22fd2b5-7f76-4fbd-b172-737066c6f0b3
⛔ Files ignored due to path filters (3)
.github/assets/client-cursor.jpgis excluded by!**/*.jpg.github/assets/client-trae.pngis excluded by!**/*.pngCargo.lockis excluded by!**/*.lock
📒 Files selected for processing (73)
AGENTS.mdCargo.tomlREADME.mdREADME.zh-cn.mdcrates/tokscale-cli/Cargo.tomlcrates/tokscale-cli/src/cli.rscrates/tokscale-cli/src/commands/cache.rscrates/tokscale-cli/src/commands/clients.rscrates/tokscale-cli/src/commands/graph.rscrates/tokscale-cli/src/commands/headless.rscrates/tokscale-cli/src/commands/hourly.rscrates/tokscale-cli/src/commands/integrations.rscrates/tokscale-cli/src/commands/models.rscrates/tokscale-cli/src/commands/monthly.rscrates/tokscale-cli/src/commands/pricing.rscrates/tokscale-cli/src/commands/shared.rscrates/tokscale-cli/src/commands/time_metrics.rscrates/tokscale-cli/src/commands/usage/codex.rscrates/tokscale-cli/src/commands/usage/mod.rscrates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-cli/src/cursor.rscrates/tokscale-cli/src/failure.rscrates/tokscale-cli/src/main.rscrates/tokscale-cli/src/main_tests.rscrates/tokscale-cli/src/trae.rscrates/tokscale-cli/src/tui/app.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/colors.rscrates/tokscale-cli/src/tui/data/mod.rscrates/tokscale-cli/src/tui/mod.rscrates/tokscale-cli/src/tui/settings.rscrates/tokscale-cli/src/tui/ui/agents.rscrates/tokscale-cli/src/tui/ui/daily.rscrates/tokscale-cli/src/tui/ui/dialog/source_picker.rscrates/tokscale-cli/src/tui/ui/footer.rscrates/tokscale-cli/src/tui/ui/header.rscrates/tokscale-cli/src/tui/ui/hourly.rscrates/tokscale-cli/src/tui/ui/usage.rscrates/tokscale-cli/src/tui/ui/widgets.rscrates/tokscale-cli/tests/cli_tests.rscrates/tokscale-cli/tests/copilot_memory.rscrates/tokscale-cli/tests/tui_exit_tests.rscrates/tokscale-core/client-catalog.jsoncrates/tokscale-core/src/adapters/file.rscrates/tokscale-core/src/adapters/mod.rscrates/tokscale-core/src/adapters/trae.rscrates/tokscale-core/src/aggregate/tui.rscrates/tokscale-core/src/clients.rscrates/tokscale-core/src/lib.rscrates/tokscale-core/src/lib_tests.rscrates/tokscale-core/src/local_clients.rscrates/tokscale-core/src/local_report_error.rscrates/tokscale-core/src/message_cache.rscrates/tokscale-core/src/scanner.rscrates/tokscale-core/src/sessionize.rscrates/tokscale-core/src/sessions/cursor.rscrates/tokscale-core/src/sessions/droid.rscrates/tokscale-core/src/sessions/mod.rscrates/tokscale-core/src/sessions/trae.rsdocs/adr/0007-client-identity-catalog.mddocs/adr/0014-explicit-subscription-usage-boundary.mddocs/adr/0015-local-only-product-surface.mddocs/adr/0018-bounded-source-fold-pipeline.mddocs/adr/0022-deterministic-cli-command-semantics.mddocs/adr/0023-provider-owned-credentials.mddocs/adr/0024-subscription-usage-redesign.mddocs/cli.mddocs/clients.mddocs/configuration.mddocs/development.mddocs/pricing.mdpackages/cli/package.jsonpackages/tokscale/package.json
💤 Files with no reviewable changes (13)
- packages/cli/package.json
- crates/tokscale-core/src/sessions/cursor.rs
- packages/tokscale/package.json
- crates/tokscale-core/src/sessions/trae.rs
- crates/tokscale-core/src/adapters/trae.rs
- crates/tokscale-core/src/sessions/mod.rs
- Cargo.toml
- crates/tokscale-cli/src/tui/ui/widgets.rs
- crates/tokscale-core/client-catalog.json
- crates/tokscale-cli/src/tui/colors.rs
- crates/tokscale-cli/src/trae.rs
- crates/tokscale-cli/src/cursor.rs
- crates/tokscale-core/src/local_clients.rs
6 tasks
Owner
Author
|
Follow-up on the non-inline comments from the latest CodeRabbit review:
Validation after the changes: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--home, optional-tab validation, refresh overrides, and aggregate-cache scope authoritativeContract highlights
tokscaleis the default TUI; configured launches usetokscale tui ...models,monthly,hourly, andtime-metricsalways produce reports regardless of TTY state--lightare rejected; known v4 forms receive a migration hint but no compatibility alias{ data, health, metadata }, while warnings and benchmark output stay on stderrtokscale cache warmis the explicit maintenance commandpricing lookup/pricing overrides, graph path output, and theheadless ... -- <command>boundary are explicit--ranking <agents|clients>selection; automatic mode is explicit and incompatible presentation options fail during resolutionqexits the TUI successfully, while Ctrl-C is a typed interruption that restores terminal state before returning 130--no-spinner; JSON-derived progress suppression is applied only during executioncursorandtraeare invalid client IDs and have no parser, scanner, TUI entry, cache sync, or command namespaceDesign records
Validation
cargo fmt --all -- --checkcargo clippy --locked --workspace --all-features -- -D warningscargo test --locked --workspace --all-features(2290 passed, 5 ignored)qreturns 0, Ctrl-C returns 130, and terminal state is restored--no-spinnerintentbun run build:clitraenamespace return exit code 2Release note
This is the v5 breaking CLI contract, but the package version is intentionally not changed here. The version-only release commit remains separate so merging this implementation cannot trigger npm publication before release validation.
Closes #143
Summary by CodeRabbit
home_dir与--no-refresh，退出与终端状态恢复更可靠。pricing lookup与pricing overrides。