feat(limits): 新增火山方舟 Ark Coding Plan 额度监控 - #450
Conversation
Add Ark Coding Plan (火山方舟) quota monitoring to the Limits panel.
The subscription refreshes on three windows (5-hour session / weekly /
monthly); percentages are read from the user's own arkcli binary
('arkcli usage plan'), feature-detected so machines without it simply
skip the provider. No token-consumption source is added — consumption
for the compatible CLIs is already counted from their local files, so
this only surfaces the quota (no double counting).
- src/lib/ark-coding-plan-limits.js: self-contained fetch + parse +
disk-cache fallback (mirrors qoder-limits.js; no circular dep)
- usage-limits.js: wire codingPlan into the aggregate fetch
- dashboard: provider registration, 3-window spec, Ark CLI setup hint
(mirrors OpenCode Go), Volcano engine brand mark, zh/zh-TW copy
- tests: 9 backend cases + panel rendering / setup-hint coverage
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Ark Coding Plan as a usage-limit provider. The change retrieves quotas through ChangesArk Coding Plan provider
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to This change adds Ark CLI-based quota monitoring and cache-backed reporting, but it is not yet merge-ready because unbounded CLI output could cause memory pressure and timeout handling may hide otherwise valid quota data. Sequence Diagram(s)sequenceDiagram
participant Dashboard as LimitsPage
participant Usage as usage-limits.js
participant Fetcher as fetchArkCodingPlanLimits
participant ArkCLI as arkcli
participant Panel as UsageLimitsPanel
Dashboard->>Usage: request usage limits
Usage->>Fetcher: fetch Coding Plan quotas
Fetcher->>ArkCLI: query plan and usage
ArkCLI-->>Fetcher: return quota windows and plan tier
Fetcher-->>Usage: return normalized or cached limits
Usage-->>Dashboard: return codingPlan usage data
Dashboard->>Panel: render codingPlan
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/lib/usage-limits.js (1)
3229-3236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePass
nowMsfor consistency and testability.
fetchArkCodingPlanLimitsacceptsnowMsand defaults it toDate.now(). The neighbouring providers pass the shared clock value. Pass it here as well, so the cache timestamps use one clock per fetch cycle and tests can control time.♻️ Proposed change
withProviderTimeout( fetchArkCodingPlanLimits({ commandRunner, home, + nowMs, }),🤖 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 `@src/lib/usage-limits.js` around lines 3229 - 3236, Update the fetchArkCodingPlanLimits call within the provider timeout flow to pass the shared nowMs value alongside commandRunner and home. Preserve the existing timeout and error handling while ensuring cache timestamps use the same clock as neighbouring providers and remain controllable in tests.test/ark-coding-plan-limits.test.js (1)
139-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for cache expiry.
No test passes
nowMs, so the age-bound branches ofreadArkCodingPlanLimitsCacheare never exercised. Add a test that writes the cache, then callsfetchArkCodingPlanLimitswith a failing runner and annowMsfar past everyreset_at. The test must assert that the stale snapshot is not served. This pairs with the cache-bound issue raised onsrc/lib/ark-coding-plan-limits.jsLines 124-148.🤖 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 `@test/ark-coding-plan-limits.test.js` around lines 139 - 163, Add a cache-expiry test alongside the existing fetchArkCodingPlanLimits tests: write a valid cache with a successful run, then use a failing commandRunner and pass fetchArkCodingPlanLimits an nowMs well beyond every reset_at. Assert the expired stale snapshot is not returned, while preserving the existing error behavior when no usable data remains.src/lib/ark-coding-plan-limits.js (1)
262-282: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not let the optional
plans getcall starve the quota call.The three CLI calls run in sequence:
which(2 s cap),plans get(10 s cap), thenusage plan(10 s cap). The worst case is about 22 s.usage-limits.jswraps the whole call inwithProviderTimeout(providerTimeoutMs). IfproviderTimeoutMsis lower than the serial worst case, a slowplans getconsumes the budget and the provider reports an error, even thoughusage planalone would have succeeded.
plans getonly supplies a fallback plan label. Run both commands concurrently so the optional call cannot delay the essential one.♻️ Proposed refactor to run both commands concurrently
- let tier = null; - const plansResult = await runCommand( - commandRunner, - "arkcli", - ["plans", "get", "--format", "json"], - { timeout: ARK_USAGE_PLAN_TIMEOUT_MS }, - ); + const [plansResult, result] = await Promise.all([ + runCommand( + commandRunner, + "arkcli", + ["plans", "get", "--format", "json"], + { timeout: ARK_USAGE_PLAN_TIMEOUT_MS }, + ), + runCommand( + commandRunner, + "arkcli", + ["usage", "plan", "--format", "json"], + { timeout: ARK_USAGE_PLAN_TIMEOUT_MS }, + ), + ]); + + let tier = null; if (!plansResult?.error && plansResult?.status === 0) { try { tier = normalizeArkPlansResponse(JSON.parse(String(plansResult.stdout || ""))); } catch (_error) {} } - - const result = await runCommand( - commandRunner, - "arkcli", - ["usage", "plan", "--format", "json"], - { timeout: ARK_USAGE_PLAN_TIMEOUT_MS }, - );🤖 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 `@src/lib/ark-coding-plan-limits.js` around lines 262 - 282, Update the quota-fetching flow around the plansResult and usage plan runCommand calls so the optional “plans get” and essential “usage plan” commands execute concurrently rather than sequentially. Preserve the existing non-fatal tier parsing and ensure the usage-plan result remains available for quota processing without waiting for the optional plan-label lookup beyond concurrent completion.
🤖 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 `@dashboard/src/lib/pet-quips.js`:
- Line 310: Localize the new Coding Plan labels in
dashboard/src/lib/pet-quips.js at lines 310 and 397: resolve the provider name
and all three period labels through the dashboard copy system, adding the
required entries to dashboard/src/content/copy.csv instead of hardcoding
user-facing text.
In `@src/lib/ark-coding-plan-limits.js`:
- Around line 124-148: Update readArkCodingPlanLimitsCache in
src/lib/ark-coding-plan-limits.js (lines 124-148) to retain only windows whose
reset_at is after nowMs, returning null when none survive; preserve the existing
cache validation and response shape for surviving windows. Add coverage in
test/ark-coding-plan-limits.test.js (lines 139-163) that writes a cache, invokes
fetchArkCodingPlanLimits with a failing runner and nowMs after every reset_at,
and asserts the stale snapshot is not returned.
- Around line 220-229: Update whichBinary and isBinaryAvailable to accept and
propagate a platform option through the arkcli lookup chain, including the
caller in fetchArkCodingPlanLimits. Select “where” when platform is “win32” and
“which” otherwise, and normalize command output so CRLF-separated Windows
results are parsed correctly.
In `@test/ark-coding-plan-limits.test.js`:
- Around line 15-20: Replace the real-looking user_id and profile values in the
USAGE_JSON viewer fixture with clearly synthetic placeholders. Keep the viewer
structure and auth_method unchanged, since normalizeArkCodingPlanResponse does
not depend on these fields.
---
Nitpick comments:
In `@src/lib/ark-coding-plan-limits.js`:
- Around line 262-282: Update the quota-fetching flow around the plansResult and
usage plan runCommand calls so the optional “plans get” and essential “usage
plan” commands execute concurrently rather than sequentially. Preserve the
existing non-fatal tier parsing and ensure the usage-plan result remains
available for quota processing without waiting for the optional plan-label
lookup beyond concurrent completion.
In `@src/lib/usage-limits.js`:
- Around line 3229-3236: Update the fetchArkCodingPlanLimits call within the
provider timeout flow to pass the shared nowMs value alongside commandRunner and
home. Preserve the existing timeout and error handling while ensuring cache
timestamps use the same clock as neighbouring providers and remain controllable
in tests.
In `@test/ark-coding-plan-limits.test.js`:
- Around line 139-163: Add a cache-expiry test alongside the existing
fetchArkCodingPlanLimits tests: write a valid cache with a successful run, then
use a failing commandRunner and pass fetchArkCodingPlanLimits an nowMs well
beyond every reset_at. Assert the expired stale snapshot is not returned, while
preserving the existing error behavior when no usable data remains.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a0b42198-3bfb-41bd-88c9-9ab7116cb3ef
⛔ Files ignored due to path filters (2)
dashboard/public/brand-logos/volcano-ark.svgis excluded by!**/*.svgdashboard/src/content/copy.csvis excluded by!**/*.csv
📒 Files selected for processing (15)
dashboard/src/content/i18n/zh-TW/core.jsondashboard/src/content/i18n/zh/core.jsondashboard/src/hooks/use-limits-display-prefs.test.jsdashboard/src/hooks/use-usage-limits.test.tsxdashboard/src/hooks/use-usage-limits.tsdashboard/src/lib/limits-providers.jsdashboard/src/lib/pet-quips.jsdashboard/src/pages/LimitsPage.jsxdashboard/src/ui/dashboard/components/ProviderIcon.jsxdashboard/src/ui/dashboard/components/UsageLimitsPanel.jsxdashboard/src/ui/dashboard/components/UsageLimitsPanel.test.jsxdashboard/src/ui/dashboard/components/usage-limits-provider-specs.jssrc/lib/ark-coding-plan-limits.jssrc/lib/usage-limits.jstest/ark-coding-plan-limits.test.js
| zcode: "ZCode", | ||
| opencodeGo: "OpenCode Go", | ||
| qoder: "Qoder", | ||
| codingPlan: "Ark Coding Plan", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the new pet-summary labels.
The new strings bypass the dashboard copy system. This causes untranslated provider and period names in pet summaries.
dashboard/src/lib/pet-quips.js#L310-L310: Resolve the Coding Plan provider name from localized copy.dashboard/src/lib/pet-quips.js#L397-L397: Resolve the three Coding Plan period labels from localized copy.
As per coding guidelines: “Never hardcode user-facing text; add it to dashboard/src/content/copy.csv.” As per path instructions: “User-facing strings must come from dashboard/src/content/copy.csv.”
📍 Affects 1 file
dashboard/src/lib/pet-quips.js#L310-L310(this comment)dashboard/src/lib/pet-quips.js#L397-L397
🤖 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 `@dashboard/src/lib/pet-quips.js` at line 310, Localize the new Coding Plan
labels in dashboard/src/lib/pet-quips.js at lines 310 and 397: resolve the
provider name and all three period labels through the dashboard copy system,
adding the required entries to dashboard/src/content/copy.csv instead of
hardcoding user-facing text.
Sources: Coding guidelines, Path instructions
…providers The macOS menu-bar app keeps its own canonical provider-id list; the new codingPlan provider was added to the dashboard list but not to the native store, breaking the parity guardrail. Add it to allProviders (and a display name) and refresh the ui-hardcode baseline for the new panel test assertions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/ops/ui-hardcode-baseline.json`:
- Around line 416-421: Update the baseline generator that produces rawTextTokens
so it records only token counts, never source text fragments or
prompt/message/conversation content. Preserve count fields such as rawText,
remove raw token values from the generated structure, and regenerate
ui-hardcode-baseline.json using the corrected generator.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9727d16c-d12d-444a-9034-8c25a19b8af1
📒 Files selected for processing (2)
TokenTrackerBar/TokenTrackerBar/Models/LimitsSettingsStore.swiftscripts/ops/ui-hardcode-baseline.json
| "rawText": 4, | ||
| "rawTextTokens": [ | ||
| ", ); // Brand + tier, without repeating \"Coding Plan\" (title: \"Ark Coding Plan Lite\"). expect(screen.getByText(\"Ark Coding Plan Lite\")).toBeInTheDocument(); expect(screen.getByText(\"5h\")).toBeInTheDocument(); expect(screen.getByText(\"Weekly\")).toBeInTheDocument(); expect(screen.getByText(\"Monthly\")).toBeInTheDocument(); expect(screen.getByText(\"33%\")).toBeInTheDocument(); expect(screen.getByText(\"16%\")).toBeInTheDocument(); expect(screen.getByText(\"9%\")).toBeInTheDocument(); // Not-configured fallback shows the Ark CLI setup guide. rerender(", | ||
| ", ); // Brand name only — no plan_label suffix to avoid \"OpenCode Go Go\". expect(screen.getByText(\"OpenCode Go\")).toBeInTheDocument(); expect(screen.getByText(\"5h\")).toBeInTheDocument(); expect(screen.getByText(\"Weekly\")).toBeInTheDocument(); expect(screen.getByText(\"Monthly\")).toBeInTheDocument(); expect(screen.getByText(\"12%\")).toBeInTheDocument(); expect(screen.getByText(\"30%\")).toBeInTheDocument(); expect(screen.getByText(\"60%\")).toBeInTheDocument(); // Not-configured fallback. rerender(", | ||
| ", ); expect(screen.getByText(\"Codex\")).toBeInTheDocument(); const row = screen.getByText(\"Credits\").closest(\"div\"); expect(within(row).getByText(\"", | ||
| ", ); expect(screen.getByText(\"Cursor\")).toBeInTheDocument(); rerender(" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Persist counts, not raw text tokens.
Lines 416-421 store source text fragments in rawTextTokens. Update the baseline generator and regenerate this file so entries retain counts such as rawText without retaining raw token values.
As per coding guidelines, **/*: Store token counts only; never collect prompts, messages, or conversation bodies.
🤖 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 `@scripts/ops/ui-hardcode-baseline.json` around lines 416 - 421, Update the
baseline generator that produces rawTextTokens so it records only token counts,
never source text fragments or prompt/message/conversation content. Preserve
count fields such as rawText, remove raw token values from the generated
structure, and regenerate ui-hardcode-baseline.json using the corrected
generator.
Source: Coding guidelines
xiufengsun
left a comment
There was a problem hiding this comment.
NO-MERGE — reviewed exact head c968632621538d76892209595ef11d0d5eff7e36 against current origin/main.
There is a cross-platform functional blocker in src/lib/ark-coding-plan-limits.js: whichBinary() unconditionally spawns the Unix which executable. Native Windows uses where.exe; when which is absent, the spawn returns ENOENT, isBinaryAvailable() returns false, and TokenTracker reports { configured: false } even when arkcli is correctly installed and logged in. This makes Ark Coding Plan unavailable in the supported Windows desktop app.
Please make binary discovery platform-aware (or use an existing cross-platform resolver), and add a regression test that exercises native Windows discovery rather than mocking which as universally available. The current-main integration itself is clean, and the targeted Ark/usage-limits tests pass once this blocker is fixed.
Address review feedback on xiufengsun#450: - Binary discovery used the Unix 'which' unconditionally; native Windows ships 'where.exe' instead, so spawn('which') returned ENOENT and the provider always reported configured:false even with arkcli installed and signed in. Probe with 'where' on win32 (regression test added). - The disk cache was served while any window had a future reset_at, even if a different window had already rolled over; drop windows whose reset_at has passed and refuse the snapshot when none survive (regression test added). - Pass nowMs through to fetchArkCodingPlanLimits for a single clock per fetch cycle (consistency with neighbouring providers).
|
已修复,新增 commit 1. 跨平台二进制发现(你提的 blocker)✅
新增回归测试 2. 顺带修的(CodeRabbit 意见)
3. 未改的两条 CodeRabbit 意见(说明理由)
验证
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/usage-limits.js (1)
3225-3237: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPropagate cancellation to the Ark CLI command.
The 15-second provider timeout can expire while the sequential Ark commands are still running. Each command has its own 10-second timeout, which only sends
SIGTERM; it does not kill the process group or use aSIGKILLfallback. Propagate cancellation throughfetchArkCodingPlanLimits, or enforce a single provider deadline with guaranteed process termination.🤖 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 `@src/lib/usage-limits.js` around lines 3225 - 3237, Update fetchArkCodingPlanLimits and its commandRunner calls to accept and propagate cancellation from withProviderTimeout, ensuring commands stop when the 15-second provider deadline expires. Strengthen command termination so cancellation kills the entire process group and uses a SIGKILL fallback when SIGTERM does not exit promptly, while preserving the existing quota result and error handling.Source: Linters/SAST tools
🧹 Nitpick comments (1)
test/ark-coding-plan-limits.test.js (1)
217-241: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a mixed-expiry cache test.
This test expires all three windows. It covers only the no-valid-window path. Add a case with one expired window and one future
reset_at, then assert that only valid windows are returned. The PR objective requires expired windows to be discarded individually.🤖 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 `@test/ark-coding-plan-limits.test.js` around lines 217 - 241, Add a mixed-expiry test alongside fetchArkCodingPlanLimits, with at least one window reset before nowMs and another reset in the future. Mock an unsuccessful refresh, then assert the result preserves only the future-reset window and discards the expired window individually, including any expected stale/source metadata.
🤖 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 `@test/ark-coding-plan-limits.test.js`:
- Around line 81-85: Update mockRunner to record the command arguments as well
as command names, then assert that the Windows where probe receives the target
arkcli. Apply the same argument capture and assertion to the additional
mockRunner usage identified in the discovery tests, while preserving the
existing success and failure behavior.
---
Outside diff comments:
In `@src/lib/usage-limits.js`:
- Around line 3225-3237: Update fetchArkCodingPlanLimits and its commandRunner
calls to accept and propagate cancellation from withProviderTimeout, ensuring
commands stop when the 15-second provider deadline expires. Strengthen command
termination so cancellation kills the entire process group and uses a SIGKILL
fallback when SIGTERM does not exit promptly, while preserving the existing
quota result and error handling.
---
Nitpick comments:
In `@test/ark-coding-plan-limits.test.js`:
- Around line 217-241: Add a mixed-expiry test alongside
fetchArkCodingPlanLimits, with at least one window reset before nowMs and
another reset in the future. Mock an unsuccessful refresh, then assert the
result preserves only the future-reset window and discards the expired window
individually, including any expected stale/source metadata.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 37111f58-3836-44ac-ad8b-8127ee375f63
📒 Files selected for processing (3)
src/lib/ark-coding-plan-limits.jssrc/lib/usage-limits.jstest/ark-coding-plan-limits.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/ark-coding-plan-limits.js
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/ark-coding-plan-limits.js (1)
183-185: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnforce the output limit for spawned commands.
child_process.spawn()does not enforcemaxBufferfor these piped streams. Lines 267-268 append all output without a byte limit, which can exhaust memory ifarkcliis verbose. Track combined stdout and stderr bytes, terminate the child when the limit is exceeded, and return an error so the cache fallback runs. Add a regression test with output larger thanmaxBuffer.🤖 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 `@src/lib/ark-coding-plan-limits.js` around lines 183 - 185, Update the spawned-command handling around the stdout/stderr accumulation to enforce maxBuffer manually, tracking combined UTF-8 output bytes from both streams. Terminate the child and propagate an error when the limit is exceeded so the cache fallback executes, and add a regression test covering output larger than maxBuffer.src/lib/usage-limits.js (1)
3247-3249: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep the disk-cache fallback on provider timeout.
When
withProviderTimeoutreaches its deadline, it rejects beforefetchArkCodingPlanLimitscan return its internalfailWithCacheresult. Thiscatchreturns only{ configured: true, error }, so a slow or aborted Ark CLI request hides a valid cached snapshot. Read the Ark cache in this outer timeout path before returning the error.🤖 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 `@src/lib/usage-limits.js` around lines 3247 - 3249, Update the outer timeout catch around fetchArkCodingPlanLimits to read and return the Ark disk-cache snapshot before falling back to the timeout error object. Preserve the existing configured/error response when no cached snapshot is available, and reuse the existing Ark cache access and result shape.
🤖 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 `@src/lib/usage-limits.js`:
- Around line 3241-3246: Update the fetchArkCodingPlanLimits call within
fetchUsageLimitsUncached to pass through the existing platform value alongside
commandRunner, home, nowMs, and signal, so Ark uses the requested platform
rather than process.platform.
---
Outside diff comments:
In `@src/lib/ark-coding-plan-limits.js`:
- Around line 183-185: Update the spawned-command handling around the
stdout/stderr accumulation to enforce maxBuffer manually, tracking combined
UTF-8 output bytes from both streams. Terminate the child and propagate an error
when the limit is exceeded so the cache fallback executes, and add a regression
test covering output larger than maxBuffer.
In `@src/lib/usage-limits.js`:
- Around line 3247-3249: Update the outer timeout catch around
fetchArkCodingPlanLimits to read and return the Ark disk-cache snapshot before
falling back to the timeout error object. Preserve the existing configured/error
response when no cached snapshot is available, and reuse the existing Ark cache
access and result shape.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32e2063d-0cf6-4f30-ae21-677bf7155789
📒 Files selected for processing (3)
src/lib/ark-coding-plan-limits.jssrc/lib/usage-limits.jstest/ark-coding-plan-limits.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- test/ark-coding-plan-limits.test.js
|
先不合并,正在修复新发现的问题 |
|
已修复 |
xiufengsun
left a comment
There was a problem hiding this comment.
整体设计是这批 provider PR 里最扎实的:错误隔离与 qoderCn 写法一致、abort 是把 signal 传进 spawn 生命周期的真取消、maxBuffer 补了 cp.spawn 的真实缺口、缓存的时钟回拨守卫和跨 profile 拒绝都很细,macOS 侧 decoding/view/reset detector/settings store 覆盖也比之前的 provider 完整。「只做额度监控不重复计数」经核实成立。四个问题修完即可合:
-
ServerManager.swift 的 PATH 改动越界且未达目的。 这是全 PR 唯一改变所有 macOS 用户内嵌 server 行为的代码:standardPaths 被前置到继承 PATH 之前,从终端启动 .app 的用户,asdf/nvm shim 会被 Homebrew 路径抢先,内嵌 server 按名解析的 git(项目归因)等命令解析结果被静默改变。而它想解决的问题也没解决——arkcli 是 npm install -g 安装,落点在 npm global prefix,nvm/fnm/volta 用户照样发现不了。改为追加 + 探测 npm global prefix,或者把 PATH 发现从本 PR 拆出去单独做完整。
-
~140 行 runCommand/whichBinary/isBinaryAvailable 从 usage-limits.js 复制到了 ark-coding-plan-limits.js。 副本里的三处改进(abort signal、win32 用 where、maxBuffer 上限)原版都没有,等于修复只活在副本里,两份实现从合并第一天就分叉。避免循环依赖的理由成立,但正解是抽成 src/lib/command-runner.js 两边共用。
-
Windows 上 shell:true + 裸名 arkcli 有 CWD 劫持面。 cmd.exe 先搜当前目录再搜 PATH,server 进程 cwd 下的 arkcli.bat 会被优先执行。whichBinary() 已经拿到了绝对路径却只留了个布尔值——把绝对路径传下去用于后续 spawn 即可。
-
没装 arkcli 的用户每轮轮询白付一次 spawn。 任一 provider 临近 reset 时整轮 fetch 会退到 5 秒间隔(CACHE_MIN_TTL_MS),此时无条件 spawn which 的成本被放大。参考 Gemini 的写法:先做便宜的文件检查(arkcli 配置/凭据目录是否存在),文件不在就不 spawn。plans get 和 profile show 也建议按需化——前者只在 usage 响应缺 tier 时才需要,后者只服务缓存守卫,适合长缓存。
两个需要你实机确认的外部事实(代码无法自证):(a) period.percent 是「已用」还是「剩余」,请贴一下与控制台数值的比对;(b) arkcli profile show --format json 是否真实存在——若不存在,跨账号缓存守卫会因 profileIdentity 恒为 null 永远静默失效。
另外:test/ark-coding-plan-limits.test.js 里 usageJsonFor() 硬编码的 user_id "2126262990" 看起来是真实火山账号,换成合成值(同文件 PROFILE_JSON 用的 test-user-001 就很好);macOS 三处(popover/设置列表/重置通知)目前无 Ark 图标,dashboard 已有 volcano-ark.svg,请补 asset catalog 条目。
修完 ping 我,这个功能我是想要的。
…n-limits # Conflicts: # TokenTrackerBar/TokenTrackerBar/Models/UsageLimits.swift # TokenTrackerBar/TokenTrackerBar/Views/UsageLimitsView.swift
Review follow-up for the Ark Coding Plan provider: - Extract src/lib/command-runner.js so ark-coding-plan-limits.js and usage-limits.js no longer fork ~140 lines of runCommand/whichBinary. The three hardening fixes (abort signal wired into the spawn lifecycle, where.exe discovery on native Windows, byte-capped maxBuffer for piped output) now live once and apply to both. - Spawn the resolved absolute arkcli path, never the bare name, so cmd.exe's current-directory-first search cannot hijack the spawn. - Skip every spawn when ~/.arkcli is absent (spawn-free install evidence, mirroring Gemini's credential check); run `plans get` only when the usage payload lacks a tier, and `profile show` only on the cache-guard path. - Revert ServerManager.swift to the fully inherited environment — the provider now covers minimal-PATH launches itself through a spawn-free statSync probe of the common global bin directories. - Tests: exercise native Windows where.exe discovery, keep the runner isolated from the real filesystem via globalBinDirs, and replace the real account id in fixtures/comments with a synthetic one.
All three macOS surfaces (limits popover, settings provider list, reset toast catalog) now render the codingPlan provider from the dashboard's bundled volcano-ark.svg — the same EmbeddedServer brand-logos path already used by Kimi/Kiro/Zcode/Qoder, so no asset catalog entry is needed.
|
四个问题已全部处理完毕( 1. ServerManager.swift — 已完整回退为纯继承环境。Finder 冷启动发现不到全局 bin 的问题改由 provider 自己兜底: 2. 命令执行副本 — 已抽取 3. CWD 劫持 — 4. 免 spawn 预检 + 按需化 — 无 外部事实 (a) percent = 已用:2026-08-15 实机比对,CLI 外部事实 (b) {
"name": "coding-plan_cn-beijing_personal",
"display_name": "Coding Plan Lite",
"owner_trn": "trn:iam::***:root",
"identity_key": "volc-***",
"expires_at": "2026-09-09T23:59:59+08:00"
}
小项:test fixture 与注释中的真实账号 ID 已换为合成值( 相关测试全绿(ark 24/24、usage-limits 109/109),validate 全套 + dashboard build 通过;本机无 xcodegen,macOS xcodebuild 交 CI。修完了,麻烦再看下。 |
|
@xiufengsun 四个评审问题与两个外部事实验证都已处理完毕,详见上一条评论(含实机比对证据与打码后的 |
xiufengsun
left a comment
There was a problem hiding this comment.
复审完成。四项都是真修,而且质量高于要求:command-runner 的抽取不是挪代码,是真正的超集合并(保留了 Kiro 在用的 completeWhen 钩子,win32 where 修复也回流给了 Gemini 探测);resolveBinaryPath 的免 spawn 目录探测比原 PATH 方案作用域更干净还覆盖了 volta/npm-global;CWD 劫持消除有专门测试;~/.arkcli 前置门的零 spawn 断言正好锁在成本点上。两个外部事实的证据也都成立——session 窗口刚滚动时报 0 是「percent=已用」的证伪性观测,方向无法两解;profile show 缺 user_id 字段你主动补的 owner_trn/identity_key 回退避免了缓存守卫全拒,这个坑发现得好。
但抽取过程引入了一个新的阻塞回归:
command-runner.js 的 shell: platform === "win32" 无条件套用到了 usage-limits 的所有既有调用方,其中 detectAntigravityProcess 在 Windows 上会直接失效。 main 上的原实现从来没有 shell 选项;现在 detectAntigravityProcess 传给 powershell.exe 的 -Command 脚本里含字面量 |,Node 在 shell:true 下对 args 不转义只拼接(DEP0190),cmd.exe 会把命令从管道符处劈开,Get-CimInstance 之后的部分变成不存在的 cmd 命令。CI 全绿是因为所有测试都注入 commandRunner stub,真 spawn 分支从未被执行;生产的 local-api 调 getUsageLimits 不传 commandRunner,Windows 托盘上必中。修法:加一个 useShell 选项默认 false,只有 ark 的调用点开启(它需要 shell 是因为 npm 全局装的是 .cmd shim)——ark 的参数全是常量无元字符,开着安全;whichBinary 的 where 探测也不需要 shell。
两个非阻塞但建议顺手处理:
- arkcli 挂死时磁盘缓存读不到了。 profile show 挪进 failWithCache 后是串行执行:which 2s + usage plan 10s + 1s 宽限 + profile show 10s + 1s ≈ 24s,超过外层 15s 的 provider race——挂死场景下 .catch 直接报错,缓存来不及端出去,而「挂了还能看上次数据」正是缓存存在的意义。给 failWithCache 里的 profile show 一个 2-3s 的短超时即可。
- ~/.arkcli 是硬前置门,如果 arkcli 在 Linux/Windows 用别的配置目录(~/.config/arkcli、%APPDATA%\arkcli),那些平台装了也永远 configured:false 且无任何可排查的提示。建议多接受几个候选路径,或让 resolveBinaryPath 命中也算 evidence。
fixture 和图标已确认无需再动。就差 shell 这一处收敛,改完即合。
Review follow-up: the shared command runner had applied shell-on-Windows unconditionally, which broke detectAntigravityProcess — its powershell -Command script contains a literal `|` that cmd.exe would split at under shell execution. - runCommand now takes an explicit `useShell` flag, default false. Only the ark call sites opt in (npm's Windows .cmd shims need a shell and every ark argument is a metacharacter-free constant); `which`/`where` probes and every pre-existing usage-limits spawn stay direct. A regression test pins useShell=false on the powershell enumeration call. - `profile show` on the cache-guard path now runs with a 2.5s timeout instead of the full 10s: it executes after `usage plan` already failed, and a hung arkcli there would push the total past the outer 15s provider race, starving the disk-cache read the fallback exists to serve. - Install evidence now also accepts ~/.config/arkcli and %APPDATA%\arkcli, and a spawn-free statSync hit in a global bin directory counts as evidence on its own — machines without the CLI still pay zero spawns per poll (asserted).
|
@xiufengsun 三处都已处理( shell 收敛(阻塞项): profile show 短超时:cache-guard 路径上的 evidence 放宽:候选目录扩为 相关测试全绿(ark 27/27、usage-limits 109/109),validate 全套 + architecture-guardrails 通过。 |
…nm discovery Post-review hardening round: - whichBinary: `where` on Windows emits CRLF and may list several matches; the first line previously kept its trailing `\r`, which produced a polluted path that fails at spawn. Split on /\r?\n/ and trim each line, with a multi-match CRLF test. - Unsubscribe now drops the disk cache. The authoritative live response saying `subscribed: false` used to leave the cache from the subscribed era in place, so a later transient CLI failure would resurrect the retired plan's numbers through failWithCache. The not-subscribed branch unlinks the cache; a test walks the full sequence (subscribed cache -> unsubscribe -> CLI hang -> no resurrection). - commonGlobalBinDirectories now expands nvm (~/.nvm/versions/node/ <ver>/bin) and fnm (node-versions/<ver>/installation/bin, incl. the macOS Application Support root), newest version first, so an npm-global arkcli stays discoverable under a Finder-launched minimal PATH. - Documented the deliberate fail-open of the cross-profile cache guard (guard applies only when profile show could establish identity).
|
@xiufengsun 又补了一轮加固(
退订清缓存:权威 live 响应说 nvm/fnm 发现: 另外把跨账号缓存守卫的 fail-open 语义用注释显式化了: 相关测试全绿(ark 30/30、usage-limits 109/109),validate 全套 + architecture-guardrails 通过。 |
|
复审完两轮提交( 合并前还有三条要修: 1. 「锁死回归」的测试锁不住回归。 2. 退订 unlink 触发条件过宽。 3. 三条 advisory,不阻塞:
三条 should-fix 改完即可合并。 |
…conditional quoting
Third review round:
- The previous "regression guard" only asserted options handed to a
mock commandRunner, which never reaches the real cp.spawn branch —
reintroducing `shell: useShell || platform === "win32"` kept every
test green. New test/command-runner.test.js stubs cp.spawn itself
and asserts the spawn options: direct spawn (shell === false) by
default even on win32, and unconditional quoting under useShell.
Verified the guard catches the exact regression when reapplied.
- Cache deletion on the not-subscribed path is now driven only by an
explicit coding-plan entry with `subscribed: false`. Payloads with
no coding-plan entry at all (`{}`, `{"items":[]}`, renamed product
key) are ambiguous transients (signed out, backend degraded) and no
longer destroy the disk cache; a test walks all three shapes.
- useShell quoting is no longer gated on whitespace: cmd.exe
metacharacters include `&` `^` `()` and Windows account names (and
therefore npm global paths) may contain them without any space.
- Advisory: searchDirs computation is now lazy (evidence branch never
builds the list), and versionedBinDirs filters non-directories via
readdirSync({ withFileTypes: true }).
|
@xiufengsun 三条 should-fix 已处理( spawn 级回归测试:新建 unlink 收窄:cache 删除现在只由显式的 coding-plan 条目 无条件引号: Advisory 两条顺手带了: 相关测试全绿(command-runner 3/3、ark 31/31、usage-limits 109/109),validate 全套 + architecture-guardrails 通过。 |
The serial arkcli chain (discovery -> usage plan -> plans get / profile show -> cache read) could outrun the outer 15s provider race: worst case a hung CLI burned the full budget before the disk-cache fallback could run, so users saw an error instead of last-known data. Mirrors codexResetCreditListTimeoutMs: each CLI call's timeout is clamped to what is left of providerTimeoutMs minus a kill guard, and calls that no longer fit are skipped. `plans get` skipping only drops the tier label; `profile show` skipping falls back to the existing fail-open cache read; an exhausted budget before `usage plan` reports a timeout error without serving the unverified cache (matching the outer race semantics asserted by the existing timeout-fallback test). With the default 15s budget every timeout is unchanged — the clamp only bites once the chain actually runs long. Also runs command-runner tests in the Windows CI job and adds a .cmd-with-metacharacters spawn test (path contains `&`, no spaces): under useShell the whole path stays quoted, and the default path keeps shell === false with args passed verbatim.
|
@xiufengsun 上轮 advisory 的预算收敛已处理( 预算收敛:按 Windows spawn 覆盖:windows-build job 补跑 pet-quips 的 相关测试全绿(command-runner 4/4、ark 33/33、usage-limits 109/109),validate 全套 + architecture-guardrails 通过,Windows CI 已实跑新测试。 |
需求
为 火山方舟 Ark Coding Plan(https://console.volcengine.com/ark/region:cn-beijing/subscription/coding-plan)增加额度监控。
Coding Plan 是订阅制套餐(Lite/Pro),额度按请求次数在三个周期自动刷新:5小时(session)/ 周 / 月。目前这些数据只能在火山控制台网页查看。
设计要点
只做额度监控,不做 token 消耗统计:Coding Plan 兼容的 Claude Code / Codex / OpenCode 等工具,其 token 消耗已由 TokenTracker 从本地文件统计(与请求走哪个网关无关)。新增 source 只展示套餐配额百分比,不会重复计数。
数据源 = 用户本机的 Ark CLI:通过
arkcli usage plan --format json读取三个周期的已用百分比与重置时间(arkcli plans get补充套餐档位)。arkcli是火山方舟官方 CLI(npm install -g @volcengine/ark-cli),本机安装并登录即可用。Feature-detect + 降级:未安装 / 未登录
arkcli时该 provider 静默跳过(configured: false),不影响其他 provider;命令失败时回退磁盘缓存,无缓存才报错。连接引导:未配置时 Limits 面板显示安装/登录引导(仿 OpenCode Go 模式),含官方文档链接与一键复制命令。
改动
src/lib/ark-coding-plan-limits.js:自包含的 fetch/解析/缓存(仿 qoder-limits.js,避免与 usage-limits 循环依赖)src/lib/usage-limits.js:接入聚合抓取(Promise.all+withProviderTimeout+ 降级)验证
node --test:9/9 新增 + usage-limits 套件 106/106validate:copy/validate:locale(zh、zh-TW 100% 覆盖)通过Summary by CodeRabbit
New Features
Localization
Reliability