chore: sync upstream junhoyeo/tokscale (66 commits) + server-side TZ normalization (R2) - #3
chore: sync upstream junhoyeo/tokscale (66 commits) + server-side TZ normalization (R2)#3donghyun-mp wants to merge 77 commits into
Conversation
โฆrage (junhoyeo#514) chore: address audit nits across junhoyeo#509, junhoyeo#511, junhoyeo#512 Consolidated follow-up for audit findings on three recently-merged PRs. None are correctness blockers; this is hardening + coverage. # PR junhoyeo#509 โ feat(zed): support hosted Zed agent threads scanner.rs: cfg-gate the macOS Zed `Library/Application Support` fallback path with `#[cfg(target_os = "macos")]`. Without it the block ran on Linux too โ harmless because the path can't exist there, but inconsistent with the Windows branch which is already cfg-gated. Pure hygiene. lib.rs: document a future-risk in `pricing_multiplier`. Today the +10% Zed-hosted markup is keyed on `message.client == "zed" AND message.provider_id == "zed.dev"`, which is correct because tokscale bundles upstream-provider LiteLLM rows (anthropic/openai/google) for the underlying models. If LiteLLM ever ships rows under provider `zed.dev` that already include the markup, this would double-bill โ a comment now warns the next maintainer to thread matched-price provenance through `apply_pricing_if_available` before relying on that case. lib.rs: two negative regression tests for the markup gate, locking in the existing positive test: - non-zed client + provider_id="zed.dev" โ no markup (e.g., a claudecode message that mentions the provider should pay base rate, not Zed's hosted rate). - zed client + non-zed provider (BYOK path with provider_id pointing directly at "anthropic", etc.) โ no markup. Important forward-compat: if Zed adds BYOK support and the parser switches to upstream provider IDs, the markup must NOT fire. # PR junhoyeo#511 โ fix(codex): deduplicate forked token count history lib.rs: add a negative test that locks in two turns whose `last_token_usage` deltas are byte-identical but emitted at distinct timestamps both survive dedup. The fork-dedup key includes timestamp, so this is correct today; without the test, a future selectivity tightening (e.g. dropping timestamp from the key) could silently erase legitimate usage. # PR junhoyeo#512 โ feat(auth): support non-interactive API token auth SettingsClient.tsx: clear `createdToken` from React state after the user copies it. The raw token is shown once and only once; once copied it should leave the component tree so it no longer lives in DevTools / extension snapshots of React state. Users who haven't copied yet still have the value in the reveal panel until they click copy or navigate away. authToken.test.ts: add an expired-token test asserting `GET /api/auth/token` returns 401 with `{ error: "API token has expired" }` when `authenticatePersonalToken` resolves with `status: "expired"`. The branch existed in route.ts but was uncovered. README.md: document the auth-token precedence (env `TOKSCALE_API_TOKEN` > saved credentials file) and the revocation flow (Settings > API Tokens > Revoke; takes effect immediately, returns 401 thereafter). # Out of scope (intentionally not addressed) - Import-path consistency in `route.ts` (audit nit N1): `@/lib/auth /bearerToken` does not resolve under vitest because the frontend has no vitest config to mirror tsconfig path aliases. Switching the import to the alias breaks `npx vitest run`. The fix is to add a `vitest.config.ts` with `resolve.alias`, which is a wider surface change than this nit-cleanup PR warrants. The relative import stays as it was on main. - Legacy plaintext OR-clause in personalTokens.ts (audit M2): a pre-existing migration affordance, not introduced by junhoyeo#512; removal should be paired with a one-shot data migration that rehashes any remaining plaintext rows. - Dead `session_id_from_meta`/`session_forked_from_id` fields on `CodexParseState` (audit nit on junhoyeo#511): looks like deliberate groundwork by the original author. Removing it without their consent is overreach. # Validation - `cargo test -p tokscale-core` โ 662 passed (3 new tests) - `cargo clippy -p tokscale-core --all-features -- -D warnings` โ clean - `cargo fmt --all -- --check` โ clean - `npx vitest run __tests__/api/authToken.test.ts __tests__/lib/bearerToken.test.ts` โ 8/8 pass Constraint: All fixes must lock in current behavior (negative tests, cfg gates, doc comments) without changing observed behavior of any shipped feature Constraint: No vitest config changes โ the import-style nit yields to that constraint Confidence: high Scope-risk: narrow โ 5 files, +188/-0, no behavior change
โฆunhoyeo#515) Closes the import-style nit deferred from PR junhoyeo#514. The frontend has no `vitest.config.ts`, so vitest cannot resolve tsconfig path aliases. Production code can mix `@/lib/...` and relative imports freely (Next.js bundles both), but the moment a test file pulls in production code that imports `@/lib/...`, vitest fails to resolve the alias unless the symbol is mocked first via `vi.mock(...)`. The result was that `route.ts` imported `authenticatePersonalToken` via `@/...` (test mocks it) but had to import `getBearerToken` via a four-level relative path (test runs the real implementation, so the alias would fail). PR junhoyeo#514's audit flagged this as nit N1 and reverted the alias-cleanup attempt because the cleanup broke `npx vitest run`. Add a minimal `packages/frontend/vitest.config.ts` that mirrors the `@/*` -> `./src/*` mapping in tsconfig.json. Then convert the two imports that were paying the relative-path tax: - `src/app/api/auth/token/route.ts`: switch `getBearerToken` import from `../../../../lib/auth/bearerToken` to `@/lib/auth/bearerToken` so `route.ts` is internally consistent (the matching `personalTokens` import was already using the alias). - `__tests__/lib/bearerToken.test.ts`: switch `getBearerToken` import from `../../src/lib/auth/bearerToken` to `@/lib/auth/bearerToken` for parity with how production code now reads. Other test files that still use `../../src/lib/...` paths (submit.test.ts, renderProfileBadgeSvg.test.ts, renderIsometric3DSvg.test.ts, renderProfileEmbedSvg.test.ts) are left as-is to keep this PR scoped to the original nit. They can be converted in a separate cleanup once the convention here is established. Validation: `npx vitest run` -> 21 test files, **177 tests pass** (0 regressions). Constraint: vitest must resolve `@/...` without breaking existing tests, and without pulling in `vite-tsconfig-paths` as a new dep Rejected: Use `vite-tsconfig-paths` plugin | adds a dependency for what is a 5-line manual config; not worth the supply-chain cost Rejected: Convert all relative test imports to aliases | scope creep; original nit was about route.ts only Confidence: high Scope-risk: narrow โ config-only + 2 import lines Directive: When adding new tests that import production code via `@/...` aliases, no further config is needed; the alias just works.
โฆnhoyeo#516) Closes the M2 security finding deferred from PR junhoyeo#514's audit. # Background Pre-junhoyeo#512 the `api_tokens.token` column stored personal API tokens in plaintext (`tt_<48 hex chars>`). PR junhoyeo#512 introduced SHA-256 at rest for all NEW tokens and a transitional OR-clause in `authenticatePersonalToken` that matched both the hashed value and the raw value, then auto-rehashed the matched row in place. That upgrade fired only when the token's owner re-authenticated, leaving plaintext rows in the DB indefinitely for users who don't. While that bridge was correct as a migration affordance, it widens the breach surface: any read access to the DB during the migration window leaks directly-usable tokens. The audit on PR junhoyeo#512 (forwarded to PR junhoyeo#514's body) flagged this as Medium and asked for the fix to be paired with a one-shot data migration. # Change Two coordinated edits: 1. `migrations/0006_rehash_plaintext_personal_tokens.sql` (new): - `CREATE EXTENSION IF NOT EXISTS pgcrypto` (idempotent; defensive โ pgcrypto is widely available on managed Postgres but not always pre-loaded) - Single-statement `UPDATE api_tokens SET token = encode(digest(token, 'sha256'), 'hex') WHERE LEFT(token, 3) = 'tt_'` rehashes every remaining plaintext row in place. - Identification rationale: SHA-256 hex output is `[0-9a-f]{64}` and can never produce the substring `tt_`. Any row whose stored token still starts with `tt_` is definitionally plaintext. Already-hashed rows are skipped. - Idempotent: re-running after migration matches zero rows. 2. `src/lib/auth/personalTokens.ts`: - Drop the `or(eq(token, hashed), eq(token, raw))` clause; the query now matches only `eq(token, hashed)`. - Drop the `isLegacyPlaintext` detection and the rehash-on-use UPDATE branch โ the migration finished that work. - Drop the `tokenValue: apiTokens.token` SELECT column โ it was only there to detect plaintext. - Drop the `or` import (no longer used). - Add a comment pointing future readers at migration 0006 so the historical context isn't lost. 3. `migrations/meta/_journal.json`: - Append entry for migration 0006. # Validation - `npx vitest run` from `packages/frontend` -> **21 test files / 177 tests pass** (0 regressions). The existing `personalTokens.test.ts` fixtures already use pre-hashed `tokenValue` strings, so they exercise the new (hashed-only) path unchanged. # Deployment notes - The migration must run BEFORE the application code change reaches production, OR they must deploy together. Running the app-side change first against a DB that still contains plaintext rows would lock those users out (their stored token wouldn't match a SHA-256 hash of itself). The included Drizzle migration ordering takes care of this when deployed via the standard `drizzle-kit migrate` -> deploy app sequence. - pgcrypto is required. Verify on the target deployment with `SELECT * FROM pg_extension WHERE extname = 'pgcrypto';` before rolling out. The `CREATE EXTENSION IF NOT EXISTS` in the migration will create it if the role has CREATE privilege; if not, the migration fails loudly rather than silently leaving plaintext behind. # Out of scope - Token rotation policy, max-age, automatic expiry, or per-user revocation rate limits. This PR only addresses at-rest hashing of the EXISTING plaintext rows. Constraint: All existing tokens must continue to authenticate after this lands; no user-visible behavior change beyond the fact that DB reads no longer leak directly-usable values Constraint: The migration must be idempotent and self-contained in a single transaction (Drizzle wraps each migration in BEGIN /COMMIT) Rejected: Two-phase migration with a temporary dual-column schema | needlessly complex; Postgres UPDATE is atomic per row and the table is small (one row per (user, named token)) Rejected: Background trickle migration | leaves plaintext rows visible for the duration; the security ask is "now, not eventually" Rejected: Drop the column and re-issue all tokens | breaks every active token; user-hostile Confidence: high Scope-risk: moderate โ touches the central auth-path query and introduces a destructive (in-place rewrite) data migration; covered by the existing 11 personalTokens tests + the broader auth integration suite Directive: Do not re-introduce a plaintext fallback in `authenticatePersonalToken`. The migration is one-way; future legacy paths should be paired with their own one-shot migration. Not-tested: A real production rollout against a DB that still contains plaintext rows; the fix is verified only against the test suite which mocks the DB layer.
Codex forked child JSONL files can preserve parent records before the child turn_context. Detect forked child metadata, suppress inherited JSON records until the first child turn_context, retain the inherited token baseline for the repeated post-turn_context snapshot, and keep child metadata intact for provider, agent, and workspace attribution. Bump the source-message cache schema because Codex incremental state now stores the forked-child suppression baseline. Validation: - cargo test -p tokscale-core sessions::codex::tests::test_forked_child: PASS, 2 passed - cargo test -p tokscale-core codex: PASS, 61 passed - cargo test -p tokscale-core: PASS, 661 passed, 1 ignored; codebuff 10 passed; hermes 3 passed; doc-tests 0 passed - cargo test --workspace --all-features: PASS - cargo clippy -p tokscale-core --all-features -- -D warnings: PASS - cargo fmt --all --check: PASS - git diff --check: PASS - git diff --cached --check: PASS - ASCII-only scan over touched Rust files: PASS, no matches Rollback: - Revert this commit to restore the previous Codex forked-session parsing behavior and source message cache schema.
* feat(kiro): add Kiro client support for session scanning and submission * feat(kiro): add SQLite-based session parser for kiro-cli data Parse conversations_v2 table from kiro-cli/data.sqlite3, extracting token usage from request_metadata history entries. Discovers the DB at XDG_DATA_HOME or macOS Application Support. Improves file-based parser to use context_window_tokens and context_usage_percentage for more accurate input token estimation. Fixes test assertions for new client count (22 โ 23). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(kiro): handle nullable message_ids in turn metadata Kiro session files can contain null entries in message_ids arrays. Changed type to Vec<Option<String>> and use iter().flatten() to skip nulls. * fix(frontend): add Kiro client icon and display name - Add local kiro.ico asset from favicon - Add Kiro to SOURCE_DISPLAY_NAMES, SOURCE_LOGOS, SOURCE_COLORS - Update SourceLogo.tsx to use local asset - Fixes missing icon in leaderboard filter and capitalization * fix(kiro): use filter_map instead of map_while for JSONL parsing map_while(Result::ok) aborts on the first read error, silently dropping all subsequent valid entries. filter_map(Result::ok) skips individual bad lines and continues processing the rest of the file. --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* fix(kiro): skip malformed JSONL lines without truncating
Replace the lines_filter_map_ok-flagged iterator with an explicit
match line { Err(_) => continue } pattern matching peer parsers
(claudecode.rs, kimi.rs, qwen.rs, pi.rs). The previous map_while
variant would stop on the first I/O error, dropping subsequent
valid rows.
Add test_parse_kiro_skips_malformed_jsonl_lines covering the
mixed-validity case.
Constraint: tokscale parsers must continue past per-line errors so partial usage is still reported
Rejected: map_while(Result::ok) | truncates file on first I/O error, silently under-reports
Confidence: high
Scope-risk: narrow
* style: auto-fix lint issues
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
docs(readme): add Zed row to localized supported-clients tables English README.md already lists Zed; bring localized variants in sync as a follow-up to junhoyeo#534. Confidence: high Scope-risk: narrow
* fix(kiro): skip malformed JSONL lines without truncating
Replace the lines_filter_map_ok-flagged iterator with an explicit
match line { Err(_) => continue } pattern matching peer parsers
(claudecode.rs, kimi.rs, qwen.rs, pi.rs). The previous map_while
variant would stop on the first I/O error, dropping subsequent
valid rows.
Add test_parse_kiro_skips_malformed_jsonl_lines covering the
mixed-validity case.
Constraint: tokscale parsers must continue past per-line errors so partial usage is still reported
Rejected: map_while(Result::ok) | truncates file on first I/O error, silently under-reports
Confidence: high
Scope-risk: narrow
* style(kiro): format generated parser changes
* fix(gemini): include tool tokens in reported totals * fix(gemini): invalidate cache for tool token accounting Invalidate persisted source-message cache entries so existing Gemini files are reparsed with the updated parser behavior instead of reusing stale token/cache-policy results. Constraint: Source fingerprints only track file contents, not parser-accounting semantics. Confidence: high Scope-risk: moderate Tested: rustfmt --edition 2021 --check crates/tokscale-core/src/message_cache.rs crates/tokscale-core/src/sessions/gemini.rs Tested: cargo test -p tokscale-core gemini --quiet Tested: cargo test -p tokscale-core message_cache --quiet Tested: cargo check -p tokscale-core --quiet * style: auto-fix lint issues --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix(gemini): avoid caching partial JSONL parses * fix(gemini): invalidate cache for JSONL cache policy Invalidate persisted source-message cache entries so existing Gemini files are reparsed with the updated parser behavior instead of reusing stale token/cache-policy results. Constraint: Source fingerprints only track file contents, not parser-accounting semantics. Confidence: high Scope-risk: moderate Tested: rustfmt --edition 2021 --check crates/tokscale-core/src/message_cache.rs crates/tokscale-core/src/sessions/gemini.rs Tested: cargo test -p tokscale-core gemini --quiet Tested: cargo test -p tokscale-core message_cache --quiet Tested: cargo check -p tokscale-core --quiet * style: auto-fix lint issues * chore(gemini): drop kiro/scanner rustfmt bleed-over Reset kiro.rs and scanner.rs to main. Those hunks belong in PR junhoyeo#537 (the kiro rustfmt-only PR) and were dragged in unintentionally by the GitHub Actions auto-fix bot (commit 49200c8). Confidence: high Scope-risk: narrow * style: auto-fix lint issues --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix(cursor): require fresh cache for each saved account * fix(cursor): cap secondary-account freshness retries with attempt marker Codex review (PR junhoyeo#533) flagged that .all() over saved-account caches forces implicit sync every run when a secondary account is permanently stale (expired token, removed account, persistently failing API). Touch usage.last-sync-attempt at the end of sync_cursor_cache and short-circuit the secondary-cache freshness check when the marker is recent. The active cache is still required to be fresh โ the original bug fix (fresh secondary masking stale active) is preserved. Constraint: implicit sync must be idempotent under repeat invocations Rejected: per-account marker files | unnecessary fragmentation; one global marker is enough Rejected: write empty CSV on failed account | destroys previously-good cached data Confidence: high Scope-risk: narrow
โฆlints (junhoyeo#539) The Zed scanner macOS-fallback path is gated #[cfg(target_os = "macos")] but its test and helper were not, so ubuntu CI compiled the test, ran it, and failed because the macOS-only branch is absent on Linux. While here, two test-code clippy lints were tripping cargo clippy --all-targets: - cmp_owned on PathBuf::from(\".tokscale\") in paths.rs tests - too_many_arguments on a test helper that builds opencode SQLite payloads Constraint: tokscale-core lib has #![deny(clippy::all)], so test-target lints break CI Confidence: high Scope-risk: narrow (test-only changes)
โฆse) to validation schema (junhoyeo#547) fix(frontend): add missing client sources to validation and type definitions Add antigravity, codebuff, and goose to SUPPORTED_SOURCES in submission validation and ClientType union type. These clients are defined in Rust ClientId enum and frontend constants.ts but were missing from the Zod validation schema, causing submissions containing them to be rejected entirely.
fix(hermes): include agent metadata in parsed usage Validation * Validation tier: Tier 2 โ Narrow runtime change, Hermes parser metadata only with targeted parser and TUI aggregation coverage. * git diff --check: PASS * git diff --cached --check: PASS * cargo test -p tokscale-core --test hermes: PASS (3 passed) * cargo test -p tokscale-cli tui::data::tests::test_aggregate_messages_builds_agent_usage: PASS (1 passed) * cargo fmt --check: PASS * Ledger: not applicable โ not required for selected validation tier/change family. * Version: not applicable โ not required for selected validation tier/change family. * Not run: full workspace tests โ not required for selected validation tier; remote CI can provide broad proof. Rollback * git revert HEAD
Recover Gemini CLI stream-json stats that expose per-model token counts directly under stats.models rather than nested stats.models.*.tokens. Treat stream-json input as already net when only input is present while preserving cache-inclusive normalization for legacy tokens wrappers. Validation: - cargo fmt --check: PASS - cargo test -p tokscale-core gemini: PASS, 32 passed; codebuff 0 passed, 10 filtered out; hermes 0 passed, 3 filtered out - cargo test -p tokscale-core: PASS, 662 passed, 1 ignored; codebuff 10 passed; hermes 3 passed; doc-tests 0 passed - cargo clippy -p tokscale-core --lib -- -D warnings: PASS - git diff --check: PASS - git diff --cached --check: PASS - rg -n "[ะ-ะฏะฐ-ัะั]" crates/tokscale-core/src/sessions/gemini.rs: PASS, no matches Rollback: - git revert HEAD
* fix(antigravity): decode chunked identity probe responses * fix(antigravity): prefer Transfer-Encoding chunked over Content-Length When a response advertises both Transfer-Encoding: chunked and Content-Length, RFC 7230 ยง3.3.3 requires the receiver to ignore Content-Length and decode the body as chunked. Both identity_probe_request and rpc_request previously checked Content-Length first, so a server that sent both headers would be decoded as a fixed-length read on top of chunk framing and the body would silently desync. Reorder both code paths to check chunked first, then fall back to Content-Length, then to close-delimited reads. Add a regression test that serves chunked Antigravity JSON alongside a bogus Content-Length: 1 header and asserts the marker is still detected. --------- Co-authored-by: Junho Yeo <i@junho.io>
Validation * Validation tier: Tier 2 โ narrow runtime change, localized TUI state/footer behavior. * git diff --check: PASS * git diff --cached --check: PASS * cargo test -p tokscale-cli test_initial_hourly_tab_uses_hourly_sort_default: PASS * cargo test -p tokscale-cli test_switch_tab_preserves_daily_sort_after_hourly_roundtrip: PASS * cargo test -p tokscale-cli test_current_count_label_matches_active_tab: PASS * cargo test -p tokscale-cli --bin tokscale: PASS, 482 passed, 1 ignored * cargo fmt --all --check: PASS * rustup run stable cargo clippy -p tokscale-cli --bin tokscale --all-features -- -D warnings: PASS * Ledger: not applicable โ not required for selected validation tier/change family. * Version: not applicable โ not required for selected validation tier/change family. * Not run: no frontend tests โ not required for CLI TUI-only change. * Additional note: cargo test -p tokscale-cli was attempted and failed only in pricing integration tests due external LiteLLM/OpenRouter fetch failures, outside this TUI diff. Rollback * git revert HEAD
* feat(tui): open daily detail rows for selected date * fix(tui): re-anchor daily detail close + sanitize on data refresh Two daily-detail state-machine fixes: - close_daily_detail() re-anchors the Daily summary selection by date rather than by the cached list index. The user can change the Daily sort while in detail mode, which makes daily_list_selected_index stale; finding the date in the freshly-sorted list keeps the same day highlighted on close. The viewport is preserved when the restored row is still in it, otherwise centered around the row. - update_data() exits detail mode when the refreshed UsageData no longer contains the date the user was viewing. Without this guard selected_daily_detail_date stays Some(...) while get_sorted_daily_detail_rows() returns empty, leaving the UI on a ghost detail view that cannot be navigated out of with arrow keys. Tests: - close after sort change: open detail on a target date, switch sort from Date to Cost, close, assert the restored selection still points at the original date. - update_data drops detail when the selected date disappears, with daily_detail_date returning None and the detail rows empty. - update_data preserves detail mode when the selected date still exists after the refresh. --------- Co-authored-by: Junho Yeo <i@junho.io>
) * feat(tui): add Minutely tab for per-minute token breakdown * feat(settings): gate Minutely tab behind opt-in flag, default off Add `minutelyTabEnabled` to settings.json (camelCase, default false) and plumb it through three surfaces so users who do not need per-minute aggregation do not pay for it: - DataLoader skips the minute-bucket map in aggregate_messages when the flag is off. The flag is set via `with_minutely_enabled(bool)` builder on construction in App::new_with_cached_data. - The header tab strip and click-area registration filter Tab::Minutely out when the flag is off, so the tab is invisible and not focusable via mouse. - Tab / BackTab / Left / Right navigation skip Tab::Minutely when the flag is off via new App::next_visible_tab / prev_visible_tab helpers that walk Tab::next/prev until they land on a visible variant. - App::new_with_cached_data clamps initial_tab to Tab::Overview when the requested tab is invisible under the current flag, so a stale TuiConfig pointing at Minutely cannot strand the user on a hidden view. Tests: - settings round-trip and default-false coverage in tui::settings. - DataLoader::aggregate_messages aggregation gate: skip when flag off, run when flag on, group same-minute messages, split adjacent-minute messages, clamp negative tokens and cost to zero. - App tab navigation: default cycle excludes Minutely; with the flag enabled the full 7-tab cycle is restored. Stale Tab::Minutely initial request clamps to Tab::Overview when the flag is off. - Footer current_count_label keeps an explicit "(0 minutes)" arm, verified by a dedicated test that enables the flag. --------- Co-authored-by: Patrick Kรผhn <patrick.kuehn@von-poll.com> Co-authored-by: Junho Yeo <i@junho.io>
โฆab (junhoyeo#568) Add the new `minutelyTabEnabled` setting to the configuration tables in README.md, README.ja.md, README.ko.md, and README.zh-cn.md, plus a short "Enabling the Minutely tab" subsection (localized in each language) explaining when to flip it on, the cost/benefit, the settings.json snippet, and where the tab appears in the strip. The English README also gets a small parenthetical in the features list and the TUI features section noting that Minutely is opt-in. The language READMEs already lag behind the English view list (some still say "4 views"); a full re-sync of those is out of scope for this change \u2014 this PR only adds the new flag's documentation. Cross-references the existing `autoRefreshEnabled` setting for the near-real-time monitoring use case.
โฆhoyeo#569) The `1-6` tab-switch hint appears in every README but the TUI key handler has no number-key branches \u2014 tab switching is only `\u2190/\u2192/Tab/BackTab`. The same handler also exposes nine keys that the docs never listed: `Enter`/`Esc`/`Backspace` for the daily-detail and graph-cell flow added by junhoyeo#564, `Home`/`End` for list navigation, `j` to jump to today, `y` to copy the selected row, `h` to toggle the Overview chart granularity, `v` to toggle the Hourly Table/Profile view, `Shift+R` to toggle auto-refresh, and `+`/`-` to adjust the refresh interval. `Ctrl+C` was also missing. Replace the keyboard navigation block in README.md, README.ja.md, README.ko.md, and README.zh-cn.md with an accurate list covering these keys. Each language version preserves the surrounding bullet ordering so the diff is local. The three language READMEs also still claimed "4 interactive views" in the Features list at line 140 \u2014 missing Hourly and Agents (added long before this round) and the new opt-in Minutely. Their TUI Features subsection already said 6 views, so the inconsistency was only at the top. Align the line 140 wording with the English version (six views + opt-in Minutely) in all three translations. No code changes; touches only README*.md.
When the platform-specific optional package (e.g. @tokscale/cli-darwin-arm64) fails to install (common with bun's optionalDependencies handling), the launcher falls back to which/where to locate a tokscale binary on PATH. That lookup resolves to the npm bin shim, which symlinks back to this same bin.js wrapper, so spawnSync re-enters the launcher and forks itself recursively. Track the wrapper's own realpath (and process.argv[1], and packages/cli/bin.js) and skip any resolved candidate that points back to it. Apply the same check to the search-path matches so a stray symlink in node_modules cannot trigger the same loop.
โฆnhoyeo#600) chore(ui): audit pass on the new ViewSelector + GroupsBrowser Top-5 improvements from the design-guidelines pass against the ViewSelector + GroupsBrowser components added in the previous refactor: 1. Add :focus-visible outline to ViewSelector items, GroupsBrowser TabButtons, and group Cards. Keyboard users had no visible focus indicator on these interactive surfaces. 2. Make ViewSelector responsive. Bar now flex-wraps and Title drops from 30px -> 24px below 480px so the segmented control doesn't get pushed off-screen on small phones. 3. Replace role="tablist" with aria-current="page" in ViewSelector. These are URL navigations, not in-page tab panels; the tab semantics were dishonest and would have broken expected ArrowLeft/Right focus behavior. 4. Match border-radius (10 -> 8) so the ViewSelector Group matches the existing Tabs/SortOptions visual language. 5. Replace the "Loading groups..." string with a proper shimmer skeleton grid (6 cards). Honors prefers-reduced-motion. Added aria-busy + aria-live + aria-label="Loading groups" so SRs get the right announcement. Also added aria-disabled visual styling to the My-groups TabButton when signed out, and role="alert" to the error message. Constraint: keep visual language consistent with the existing SortOptions inside LeaderboardClient Rejected: drop the color transition on hover entirely | the /ui skill's Tailwind guideline against transition-* on hover is opinionated; the styled-components version reads fine and matches the rest of the codebase Confidence: high Scope-risk: very narrow (only the two new components; no schema, no API, no test changes) Directive: when adding link-based segmented controls, prefer aria-current over role=tablist โ links navigate, tabs swap panels Not-tested: arrow-key focus traversal across the Users/Groups items (browsers handle this natively for links via Tab; no custom keyboard handler needed)
The Rust CLI and core changes need to compile cleanly while keeping Trae account sync idempotent and daily active-time aggregation aligned to local calendar days. Constraint: Trae reports account-level sessions from a synced cache rather than root-command live reads. Rejected: Keep append-only Trae manifests | repeated syncs would duplicate the same sessions and inflate reports. Confidence: high Scope-risk: moderate Tested: cargo fmt --check; targeted Trae manifest, Trae dedupe, and local active-time tests Not-tested: Full native-target release matrix
Group invite creation should treat malformed non-empty JSON as a client error while preserving the empty-body default invite flow. Constraint: Empty request bodies are a supported shorthand for default invite settings. Rejected: Treat every JSON parse failure as an empty object | malformed payloads would be silently accepted. Confidence: high Scope-risk: narrow Tested: bunx vitest run __tests__/api/groupInviteCreateRoute.test.ts Not-tested: Browser-level invite form submission
Leaderboard URLs should survive server reloads with validated custom date/search parameters, and the groups browser should load additional pages without replacing the current tab state. Constraint: Invalid custom date ranges must fall back safely instead of leaking raw URL values into client state. Rejected: Client-only date validation | shared URLs would still hydrate from inconsistent server data. Confidence: high Scope-risk: moderate Tested: bunx vitest run __tests__/api/leaderboard.test.ts __tests__/lib/leaderboardDateRange.test.ts Not-tested: Manual browser click-through for group load-more pagination
The READMEs should advertise the currently supported Trae sync flags and include Zed and Kiro wherever client filters and support tables enumerate available clients. Constraint: Trae variant selection is scoped to credential login/logout, not sync. Rejected: Leave translated READMEs stale | users would copy unsupported `trae sync` flags. Confidence: high Scope-risk: narrow Tested: grep for stale Trae sync flags and updated client-filter lists Not-tested: Rendered README table layout in GitHub UI
The leaderboard API route should let Next.js handle request URL dynamic detection instead of catching the framework signal as an application error during production builds. Constraint: The route still needs normal application errors to return the existing JSON 500 response. Rejected: Force the route fully dynamic | moving request URL parsing outside the catch avoids the build-time false error without changing cache policy. Confidence: high Scope-risk: narrow Tested: bun run build Not-tested: Manual API request in a deployed Next runtime
The Trae implementation now uses IDE and Solo only as credential sources while storing synced account-level usage under the single `trae` client, so the module comments should describe that boundary directly. Constraint: Trae IDE and Solo share the international account-level usage API. Rejected: Reintroduce per-variant report clients | that would duplicate the same account usage when both desktop apps are installed. Confidence: high Scope-risk: narrow Tested: cargo fmt --check Not-tested: Live Trae account sync
The users/groups segmented links should switch only the leaderboard view while carrying the active date, sort, and search filters forward; page is intentionally dropped because pagination state is scoped to each view. Constraint: The view selector is link-based SSR navigation, so the href must be computed from server search params. Rejected: Keep hardcoded links | toggling views would silently reset shared leaderboard URLs. Confidence: high Scope-risk: narrow Tested: bunx vitest run __tests__/lib/leaderboardViewSelector.test.ts Not-tested: Manual browser click-through
Malformed JSON was rejected, but valid JSON primitives could still reach invite field extraction; null could throw and arrays or strings would silently behave like default payloads.
Constraint: Empty bodies must keep creating a default member invite.
Rejected: Coerce non-object JSON to `{}` | that would hide bad client payloads and weaken the malformed-body guard.
Confidence: high
Scope-risk: narrow
Tested: bunx vitest run __tests__/api/groupInviteCreateRoute.test.ts
Not-tested: Browser form submission with a corrupted request body
The custom date range parser already validates both endpoints before comparing the range, so the second identical validation check after the comparison was unreachable defensive noise. Confidence: high Scope-risk: narrow Tested: bunx vitest run __tests__/lib/leaderboardDateRange.test.ts Not-tested: Manual custom date filter entry
Repeated overlapping Trae syncs can fetch older copies of sessions that do not win the manifest merge. Delaying the batch write until after the merge avoids creating an artifact that the same GC pass immediately removes, and millisecond filenames reduce same-second overwrite risk for artifacts that do win. Constraint: Trae sync stores one JSON artifact per fetched batch and uses the manifest as the referential-integrity index. Rejected: Keep writing every fetched batch | losing batches remain unreferenced and are intentionally removed by manifest GC. Confidence: high Scope-risk: narrow Tested: cargo fmt --check Tested: cargo test -p tokscale-cli test_manifest_reference
The active-time splitter intentionally uses local calendar days, matching the date filter semantics. Document the v2.2.0-visible alignment and leave an inline note for the DST gap path where a local midnight cannot be represented. Constraint: Existing date filtering docs already state local-time semantics, but active-time cache/readers may have assumed UTC day keys. Rejected: Revert to UTC buckets | that would reintroduce mismatch with local report dates. Confidence: high Scope-risk: narrow Tested: cargo fmt --check Tested: cargo test -p tokscale-core test_compute_daily_active_time_matches_local_day_boundaries_for_fixed_offset
Brings in upstream changes since the last sync (May 2026). Notable themes: - New clients: Kiro, Trae (CLI enum + UI + server validation) - Groups feature (scoped leaderboards, invites, member roles) - Embed templates (orbit, terminal, vitals, blueprint, receipt, etc.) - Codex/Hermes parser tightening + session time metrics - Schema additions: daily_breakdown.active_time_ms, submission active-time Conflict resolutions worth flagging: - crates/tokscale-cli/src/main.rs / clients.rs / tui/data/mod.rs: Union of fork (AnthropicApi) + upstream (Kiro, Trae) clients. ClientId count goes from 22 โ 25. Cap function `cap_graph_result_to_utc_today` stays removed per #2 โ three upstream tests that exercised it are dropped, while the `test_submit_payload_includes_*` helper and `parse_variant_arg` tests come over intact. - packages/frontend/src/lib/leaderboard/getLeaderboard.ts: Fork's `LEADERBOARD_TIMEZONE` / `toDateStringInTz` is the base; upstream's new "custom" and "last-month" periods are folded in TZ-aware (instead of upstream's UTC-only calculation) so KST-anchored boundaries hold for every period. - packages/frontend/src/app/api/submit/route.ts: Union of fork's modelBreakdown + deviceContributions and upstream's active_time_ms in both INSERT VALUES and UPDATE SET clauses. - packages/frontend/src/lib/leaderboard/types.ts: Period union now ["all","month","last-month","week","day","custom"]; SortBy is ["tokens","cost","time"]. fork's "day" + upstream's "last-month"/"custom"/"time" preserved. - packages/frontend/src/app/api/leaderboard/route.ts: Kept fork's NextRequest signature (4ea3289), absorbed upstream's expanded period/sortBy validation lists. - packages/frontend/src/app/(main)/leaderboard/page.tsx: Kept fork's auth-required redirect, absorbed upstream's groups ViewSelector + searchParams plumbing. - packages/frontend/src/app/(main)/leaderboard/LeaderboardClient.tsx: Took fork (HEAD) version verbatim. Upstream's URL-state sync + custom-period UI is a substantial divergence (refresh-interval/ self-hosted-URL on the fork side vs. URL sync + customFrom/customTo on upstream), so weaving them together in this PR carries too much regression risk. Tracked as follow-up. - packages/frontend/src/app/(main)/page.tsx: Kept fork's simplified landing (a9b4276) โ upstream's leaderboard preview is intentionally not surfaced on the company-internal landing. - packages/frontend/src/components/profile/ProfileEmbedDialog.tsx: Stays deleted (fork bf6b9c8). Upstream modifications discarded. - packages/frontend/src/lib/validation/submission.ts and SourceLogo.tsx: Source/logo lists unioned (codebuff/antigravity/zed/anthropic-api + kiro). Fork's local /assets/logos paths kept over upstream's raw GitHub URLs to keep the self-hosted deploy self-contained. Past sync incident (c3c7844, "scope client-slot wipe to each day's incoming clients") flagged the risk of tighter parsers regressing device-aware merges; QA should pay extra attention to multi-device submissions and daily aggregation around midnight in the Asia/Seoul timezone before promoting this to production. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
โฆ(R2) PR #2 removed the CLI-side UTC cap that hid KST 00:00โ09:00 submissions from the daily leaderboard. The CLI now hands every contribution through to the server, but the server still trusted `contribution.date` โ chrono::Local on the client โ which means a submitter on a non-Asia/Seoul machine still lands their work on the wrong day for the KST-anchored leaderboard. PR #2 was a single-machine fix; this closes the loop. Approach (R2 from the design discussion): - Add `normalizeContributionDate` that recomputes `daily_breakdown.date` from the earliest-message `timestampMs` (already in the payload since upstream junhoyeo#187, "add timestamp to server-side contribution data") and the server's configured timezone. - Call it from `/api/submit` per incoming day and use the normalized string for both `existingDaysMap` lookup and the INSERT. - Activation is keyed on `LEADERBOARD_TIMEZONE` being set: an unset env preserves upstream global-SaaS behavior (each user's local day stays). Legacy CLI submissions (schemaVersion = 0, no `timestampMs`) also fall back to the CLI string since there is no signal to rebucket from. Why not the CLI-side env approach (R1): - It needs the CLI to be configured correctly on every contributor's machine. A single misconfigured laptop reintroduces the original bug, while the server-side path makes the bucket independent of submitter configuration. - The server already had to consume `timestampMs` for other purposes, so R2 reuses an existing channel instead of adding a new one. Regression coverage: - packages/frontend/__tests__/lib/normalizeContributionDate.test.ts โ five cases pinning the policy: unset env passthrough, legacy passthrough (null timestampMs), KST 00:30 โ KST today (the original bug case), cross-TZ override (Pacific 17:30 โ KST tomorrow), and the YYYY-MM-DD output contract used as a primary key in `daily_breakdown`. - crates/tokscale-cli/src/main.rs โ adds `test_to_ts_token_contribution_data_preserves_dates_beyond_utc_today`. This is the CLI-side counterpart that locks in PR #2: a far-future contribution must reach the server unchanged, so the server (R2) is free to decide the bucket. Picking a date like "2099-12-31" keeps the assertion stable as the calendar advances. Manual verification (per the test plan): 1. With `LEADERBOARD_TIMEZONE=Asia/Seoul` set on the server, submit at KST 00:30 from a KST machine. Daily leaderboard should show the new row on the current KST day immediately. 2. Submit at the same instant from a UTC-7 (Pacific) machine. The row must land on the same KST day, not the Pacific local day. 3. Legacy CLI (no `timestampMs`): row date follows the CLI string โ verifies the backward-compat fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces several major features and integrations, including support for the Trae (ByteDance AI) and Kiro IDEs, a new subscription usage and quota command/TUI tab, a high-granularity Minutely view, and local custom pricing overrides. It also adds session time metrics and refines parsing logic across multiple existing clients. The review feedback highlights key areas for improvement: ensuring cross-platform path resolution for Trae's global storage, making the Amp total-quota parser robust against missing trailing whitespace, replacing a custom base64 decoder in Copilot with the existing base64 crate dependency, and preventing dummy zero-token messages in Gemini by returning None when all token fields are missing.
| { | ||
| Ok(new) => { | ||
| creds.token = new.token; |
There was a problem hiding this comment.
The path to the Trae global storage directory is hardcoded using the macOS-specific Library/Application Support path. This will cause automatic credential discovery to fail on Windows and Linux platforms. Using the dirs crate's cross-platform directories ensures correct path resolution on all supported operating systems.
let app_dir = if cfg!(target_os = "macos") {
home.join("Library/Application Support").join(variant.app_dir_name())
} else if cfg!(target_os = "windows") {
dirs::data_dir()
.context("could not determine AppData directory")?
.join(variant.app_dir_name())
} else {
dirs::config_dir()
.context("could not determine config directory")?
.join(variant.app_dir_name())
};| if let Some(space_pos) = after.find(|c: char| c.is_ascii_whitespace()) { | ||
| if let Ok(total) = after[..space_pos].replace(',', "").parse::<f64>() { | ||
| if total > 0.0 && total.is_finite() && remaining.is_finite() { | ||
| let used = (total - remaining).max(0.0); | ||
| let used_pct = if used.is_finite() { | ||
| (used / total * 100.0).clamp(0.0, 100.0) | ||
| } else { | ||
| 0.0 | ||
| }; | ||
| let remaining_pct = (100.0 - used_pct).clamp(0.0, 100.0); | ||
| let mut resets_at = None; | ||
|
|
||
| // Estimate reset time from hourly replenish rate | ||
| if let Some(rate) = parse_dollar_after(text, "+$") { | ||
| if rate > 0.0 && used > 0.0 && rate.is_finite() { | ||
| let secs = (used / rate * 3600.0) as i64; | ||
| let resets = | ||
| chrono::Utc::now() + chrono::Duration::seconds(secs); | ||
| resets_at = Some(resets.to_rfc3339()); | ||
| } | ||
| } | ||
|
|
||
| metrics.push(UsageMetric { | ||
| label: "Free".into(), | ||
| used_percent: used_pct, | ||
| remaining_percent: remaining_pct, | ||
| remaining_label: Some(format!("${remaining:.2}/${total:.2}")), | ||
| resets_at, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If there is no whitespace after the total amount (e.g., "$4.50/$20.00"), after.find will return None, causing the parsing of the total amount to be skipped entirely. Using unwrap_or(after.len()) ensures that the entire remaining slice is parsed as the total amount when no trailing whitespace is present.
let space_pos = after.find(|c: char| c.is_ascii_whitespace()).unwrap_or(after.len());
if let Ok(total) = after[..space_pos].replace(',', "").parse::<f64>() {
if total > 0.0 && total.is_finite() && remaining.is_finite() {
let used = (total - remaining).max(0.0);
let used_pct = if used.is_finite() {
(used / total * 100.0).clamp(0.0, 100.0)
} else {
0.0
};
let remaining_pct = (100.0 - used_pct).clamp(0.0, 100.0);
let mut resets_at = None;
// Estimate reset time from hourly replenish rate
if let Some(rate) = parse_dollar_after(text, "+$") {
if rate > 0.0 && used > 0.0 && rate.is_finite() {
let secs = (used / rate * 3600.0) as i64;
let resets =
chrono::Utc::now() + chrono::Duration::seconds(secs);
resets_at = Some(resets.to_rfc3339());
}
}
metrics.push(UsageMetric {
label: "Free".into(),
used_percent: used_pct,
remaining_percent: remaining_pct,
remaining_label: Some(format!("${remaining:.2}/${total:.2}")),
resets_at,
});
}
}| fn base64_decode(input: &str) -> Result<String> { | ||
| // Minimal base64 decode without adding a dependency | ||
| const TABLE: &[Option<u8>; 128] = &{ | ||
| let mut table = [None; 128]; | ||
| let mut i = 0u8; | ||
| while i < 26 { | ||
| table[(b'A' + i) as usize] = Some(i); | ||
| i += 1; | ||
| } | ||
| let mut i = 0u8; | ||
| while i < 26 { | ||
| table[(b'a' + i) as usize] = Some(26 + i); | ||
| i += 1; | ||
| } | ||
| let mut i = 0u8; | ||
| while i < 10 { | ||
| table[(b'0' + i) as usize] = Some(52 + i); | ||
| i += 1; | ||
| } | ||
| table[b'+' as usize] = Some(62); | ||
| table[b'/' as usize] = Some(63); | ||
| table | ||
| }; | ||
|
|
||
| let bytes = input.as_bytes(); | ||
| let mut result = Vec::with_capacity(bytes.len() * 3 / 4); | ||
| let mut buf = 0u32; | ||
| let mut bits = 0u32; | ||
| for &b in bytes { | ||
| if b == b'=' { | ||
| break; | ||
| } | ||
| if (b as usize) >= TABLE.len() { | ||
| continue; | ||
| } | ||
| if let Some(v) = TABLE[b as usize] { | ||
| buf = (buf << 6) | v as u32; | ||
| bits += 6; | ||
| if bits >= 8 { | ||
| bits -= 8; | ||
| result.push((buf >> bits) as u8); | ||
| } | ||
| } | ||
| } | ||
| Ok(String::from_utf8(result)?) | ||
| } | ||
|
|
There was a problem hiding this comment.
The base64 crate is already a dependency of tokscale-cli. Re-implementing a custom base64 decoder is redundant and prone to edge-case bugs (such as handling padding or alternative alphabets). It is highly recommended to reuse the standard base64 crate.
fn base64_decode(input: &str) -> Result<String> {
use base64::prelude::BASE64_STANDARD;
use base64::Engine;
let bytes = BASE64_STANDARD.decode(input.trim())?;
Ok(String::from_utf8(bytes)?)
}| fn deserialize_tokens(value: &Value) -> Option<GeminiTokens> { | ||
| Some(GeminiTokens { | ||
| input: first_i64( | ||
| value, | ||
| &[ | ||
| "input", | ||
| "prompt", | ||
| "input_tokens", | ||
| "prompt_tokens", | ||
| "promptTokenCount", | ||
| ], | ||
| ), | ||
| output: first_i64( | ||
| value, | ||
| &[ | ||
| "output", | ||
| "candidates", | ||
| "output_tokens", | ||
| "completion_tokens", | ||
| "candidatesTokenCount", | ||
| ], | ||
| ), | ||
| cached: first_i64( | ||
| value, | ||
| &["cached", "cached_tokens", "cachedContentTokenCount"], | ||
| ), | ||
| thoughts: first_i64(value, &["thoughts", "reasoning", "thoughts_tokens"]), | ||
| tool: first_i64(value, &["tool", "tool_tokens"]), | ||
| total: first_i64(value, &["total", "totalTokenCount", "total_tokens"]), | ||
| }) | ||
| } |
There was a problem hiding this comment.
The deserialize_tokens function currently always returns Some(GeminiTokens { ... }) even if all token fields are None. This can lead to dummy zero-token messages being parsed and recorded. Returning None when no valid token fields are present prevents this.
fn deserialize_tokens(value: &Value) -> Option<GeminiTokens> {
let input = first_i64(value, &["input", "prompt", "input_tokens", "prompt_tokens", "promptTokenCount"]);
let output = first_i64(value, &["output", "candidates", "output_tokens", "completion_tokens", "candidatesTokenCount"]);
let cached = first_i64(value, &["cached", "cached_tokens", "cachedContentTokenCount"]);
let thoughts = first_i64(value, &["thoughts", "reasoning", "thoughts_tokens"]);
let tool = first_i64(value, &["tool", "tool_tokens"]);
let total = first_i64(value, &["total", "totalTokenCount", "total_tokens"]);
if input.is_none() && output.is_none() && cached.is_none() && thoughts.is_none() && tool.is_none() && total.is_none() {
return None;
}
Some(GeminiTokens {
input,
output,
cached,
thoughts,
tool,
total,
})
}Local main carries five fixes from the prior audit cycle: - 013552e fix(leaderboard): preserve filters across view toggles - 6d0fe74 fix(groups): reject non-object invite payloads - 1a1efd8 refactor(leaderboard): remove duplicate date validation - 8e5778d fix(cli): write Trae sync artifacts only when referenced - 1ac617e docs(cli): document local active-time day buckets Incoming from origin: - 9228a8a ci: update coverage badge [skip ci] (49% -> 47%) No content overlap: origin only touches .github/badges/coverage.svg, which none of the local commits modify. Three-way merge resolves automatically by taking origin's badge update verbatim. Verified pre-merge with `git merge-tree` against the divergence base c9e051a; no conflict markers emitted. Constraint: keep the audited fix chain intact while picking up the bot-pushed badge Rejected: rebase onto origin/main | user asked for a merge commit, and rebasing the badge bot's commit onto local would orphan its [skip ci] semantics Confidence: high Scope-risk: very narrow (one file changes, mechanical merge)
The 2026-05 sync left a handful of type drifts that local `cargo`/CLI tests didn't catch โ Next.js's production `tsc` pass on Vercel did. Five symptoms, one PR each in isolation but easier to land together: - packages/frontend/src/lib/db/helpers.ts: Reinstate `buildModelBreakdown`. The fork-only helper was dropped during the merge because upstream's submit/route lost the call site. The function is now exported and reused by submit/route again. Tests in __tests__/api/submitAuth.test.ts already mock this name, so this was a real regression, not new surface area. - packages/frontend/src/app/api/submit/route.ts: Re-add the two `const modelBreakdown = buildModelBreakdown(โฆ)` lines in both the existing-day update branch and the new-day insert branch. Without them the shorthand object property had no value in scope and the row inserts/updates were dropping model_breakdown silently. - packages/frontend/src/app/(main)/leaderboard/LeaderboardClient.tsx: The fork's `LeaderboardClient` shipped with `'tokens' | 'cost'` and its own narrow `Period` union. The sync added `'time'` to SortBy and `'last-month'/'custom'` to Period, so handing a lib `LeaderboardData` to this component blew up assignment from `prevSortByRef`/fetchData signatures all the way down to page.tsx. Replace the narrow unions with `LeaderboardSortBy` (from leaderboard/constants) and the lib `Period` (from leaderboard/types). The fork's URL-state-sync follow-up inherits the right types from the start instead of fighting them. - packages/frontend/__tests__/api/leaderboard.test.ts: Four call sites still constructed `new Request(...)` even though fork commit 4ea3289 pinned the route handler to `NextRequest`. Updated them to `new NextRequest(...)`; the import was already in place. - packages/frontend/src/components/BlackholeHero.tsx (deleted): Dead since the fork's landing simplification (a9b4276). The `@/../public/assets/hero-bg.png` static import errored because Next 16's type pass doesn't resolve the cross-alias path, and no caller imports `BlackholeHero` anyway. Removing it is cleaner than papering over an unused asset path. Local verification: - `bunx tsc --noEmit` clean in packages/frontend (was 13 errors before) - `cargo test -p tokscale-cli` still 543 passed (no Rust touches) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces integration with Trae (ByteDance's AI IDE) and Kiro, adding credential decryption, token refresh, and usage synchronization. It also adds a new "Subscription Usage" feature to track real-time quotas across AI providers, a "Minutely" tab in the TUI, and session time metrics. Additionally, it implements custom pricing overrides, improves model grouping strategies (adding session-level grouping), and refactors the TUI theme handling for better terminal compatibility. The reviewer provided several high-quality suggestions to improve robustness, including handling OAuth refresh token non-rotation in Claude and Kimi providers, resolving platform-specific paths for Trae's global storage using dirs::data_dir(), making whitespace parsing more robust in the Amp provider, refactoring the daily active time loop in sessionize.rs, and restoring the which/where binary fallback in the Node.js wrapper while avoiding self-referential recursion.
| if let Some(new_rt) = refreshed.refresh_token.as_deref() { | ||
| save_credentials( | ||
| &new, | ||
| new_rt, | ||
| oauth.subscription_type.as_deref(), | ||
| oauth.rate_limit_tier.as_deref(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
If the OAuth server does not rotate the refresh token (which is common and valid in OAuth2), refreshed.refresh_token will be None. In this case, the refreshed access_token is never saved, leading to repeated refreshes or authentication failures. Fall back to the existing refresh_token (rt) if a new one is not returned.
let new_rt = refreshed.refresh_token.as_deref().unwrap_or(rt.as_str());
save_credentials(
&new,
new_rt,
oauth.subscription_type.as_deref(),
oauth.rate_limit_tier.as_deref(),
);| if let (Some(new_rt), Some(expires_in)) = | ||
| (&refreshed.refresh_token, refreshed.expires_in) | ||
| { | ||
| stored_refresh_token = Some(new_rt.clone()); | ||
| save_credentials(&access_token, new_rt, expires_in); | ||
| } |
There was a problem hiding this comment.
If the OAuth server does not rotate the refresh token, refreshed.refresh_token will be None. In this case, the refreshed access_token is never saved. Fall back to the existing refresh_token (rt_str) if a new one is not returned.
if let Some(expires_in) = refreshed.expires_in {
let new_rt = refreshed.refresh_token.as_deref().unwrap_or(rt_str.as_str());
stored_refresh_token = Some(new_rt.to_string());
save_credentials(&access_token, new_rt, expires_in);
}| if let (Some(new_rt), Some(expires_in)) = | ||
| (&refreshed.refresh_token, refreshed.expires_in) | ||
| { | ||
| save_credentials(&new, new_rt, expires_in); | ||
| } |
There was a problem hiding this comment.
If the OAuth server does not rotate the refresh token, refreshed.refresh_token will be None. In this case, the refreshed access_token is never saved. Fall back to the existing refresh_token (rt_str) if a new one is not returned.
if let Some(expires_in) = refreshed.expires_in {
let new_rt = refreshed.refresh_token.as_deref().unwrap_or(rt_str.as_str());
save_credentials(&new, new_rt, expires_in);
}| let home = dirs::home_dir().context("could not determine home directory")?; | ||
| let app_dir = home | ||
| .join("Library/Application Support") | ||
| .join(variant.app_dir_name()); |
There was a problem hiding this comment.
The path Library/Application Support is macOS-specific. On Windows, Trae's global storage is located in %APPDATA%. Using dirs::data_dir() provides a cross-platform way to resolve this directory on both macOS and Windows.
let app_dir = dirs::data_dir()
.context("could not determine data directory")?
.join(variant.app_dir_name());| if let Some(space_pos) = after.find(|c: char| c.is_ascii_whitespace()) { | ||
| if let Ok(total) = after[..space_pos].replace(',', "").parse::<f64>() { |
There was a problem hiding this comment.
Using after.find(is_ascii_whitespace) will fail to parse the total if there is no trailing whitespace after the number (e.g., if the string ends with the number). Using split_whitespace().next() is more robust and handles both cases gracefully.
| if let Some(space_pos) = after.find(|c: char| c.is_ascii_whitespace()) { | |
| if let Ok(total) = after[..space_pos].replace(',', "").parse::<f64>() { | |
| if let Some(total_str) = after.split_whitespace().next() { | |
| if let Ok(total) = total_str.replace(',', "").parse::<f64>() { |
| loop { | ||
| let day_key = day.format("%Y-%m-%d").to_string(); | ||
| let Some(day_start) = local_day_start(day, timezone) else { | ||
| break; | ||
| }; | ||
|
|
||
| let Some(next_day) = day.succ_opt() else { | ||
| break; | ||
| }; | ||
| let Some(next_day_start) = local_day_start(next_day, timezone) else { | ||
| break; | ||
| }; | ||
|
|
||
| let overlap_start = interval.start_ts.max(day_start); | ||
| let overlap_end = interval.end_ts.min(next_day_start); | ||
| let overlap = (overlap_end - overlap_start).max(0); | ||
| let proportion = overlap as f64 / wall as f64; | ||
| let active_for_day = (interval.active_duration_ms as f64 * proportion) as i64; | ||
|
|
||
| if active_for_day > 0 { | ||
| *daily.entry(day_key).or_default() += active_for_day; | ||
| } | ||
|
|
||
| if day == end_date { | ||
| break; | ||
| } | ||
|
|
||
| if let Some(next) = day.succ_opt() { | ||
| day = next; | ||
| } else { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Using a while day <= end_date loop is safer and more idiomatic than a loop with manual breaks. It also completely avoids the redundant day.succ_opt() calls by reusing next_day.
while day <= end_date {
let day_key = day.format("%Y-%m-%d").to_string();
let Some(day_start) = local_day_start(day, timezone) else {
break;
};
let Some(next_day) = day.succ_opt() else {
break;
};
let Some(next_day_start) = local_day_start(next_day, timezone) else {
break;
};
let overlap_start = interval.start_ts.max(day_start);
let overlap_end = interval.end_ts.min(next_day_start);
let overlap = (overlap_end - overlap_start).max(0);
let proportion = overlap as f64 / wall as f64;
let active_for_day = (interval.active_duration_ms as f64 * proportion) as i64;
if active_for_day > 0 {
*daily.entry(day_key).or_default() += active_for_day;
}
day = next_day;
}|
|
||
| let binary = searchPaths.find((p) => existsSync(p) && !isSelfReference(p)); | ||
|
|
||
| if (!binary) { |
There was a problem hiding this comment.
Removing the which/where fallback entirely might break environments where the native binary is installed globally but the wrapper is run from elsewhere. We can safely restore the fallback by checking !isSelfReference(found) to prevent infinite recursion.
if (!binary) {
try {
const whichCmd = process.platform === "win32" ? "where" : "which";
const found = execSync(`${whichCmd} ${binaryName}`, {
encoding: "utf-8",
stdio: ["pipe", "pipe", "pipe"],
})
.trim()
.split("\n")[0];
if (found && existsSync(found) && !isSelfReference(found)) {
binary = found;
}
} catch {}
}
if (!binary) {
donghyun-mp
left a comment
There was a problem hiding this comment.
Thanks for the second pass. Walked through all 7 new inline comments and the merits/scope are:
All seven flag code that arrived in this PR from the upstream sync (junhoyeo/tokscale), not changes authored by this fork. This PR's purpose is to absorb upstream's 66 commits + add the fork-only R2 server-side TZ normalization. Fixing upstream-owned code inside the sync diff would:
- diverge the fork from upstream on every subsequent sync (more conflicts each round),
- hide real bugs from every other downstream consumer of
junhoyeo/tokscale, and - blur the review boundary between "what this fork is doing" and "what upstream is doing."
So the right home for these is upstream PRs/issues, not this sync. Concrete dispositions:
Genuine bugs worth reporting upstream
- OAuth refresh_token rotation (
claude.rs:203,kimi.rs:194,kimi.rs:215): if the provider doesn't rotate the refresh token (spec-allowed and common),refreshed.refresh_tokenisNoneand the new access_token is dropped on the floor. Therefreshed.refresh_token.as_deref().unwrap_or(rt)pattern is correct. Real bug, should be filed atjunhoyeo/tokscale. - Trae macOS-only path (
trae.rs:329, also:248):Library/Application Supportis hardcoded โ Win/Linux discovery silently breaks.dirs::data_dir()/dirs::config_dir()fixes it. Should be filed upstream as a follow-up to PR junhoyeo#555. Low impact for us since the company runs macOS; high impact for any upstream user on Win/Linux. - Amp parser edge case (
amp.rs:64, also:94):after.find(is_ascii_whitespace)returnsNonewhen the number ends the string;split_whitespace().next()is more robust. Real but narrow. - Gemini dummy zero tokens (
gemini.rs:81, prior round):deserialize_tokensshould returnNonewhen every field isNoneinstead of an all-NoneSome. Avoids recording dummy zero-token rows.
Refinements (preference, not bugs)
- Hand-rolled base64 (
copilot.rs:155, prior round): thebase64crate is already a dep; reusing it is cleaner. Upstream cleanup. whileloop overloop+break(sessionize.rs:298): idiomatic; not a behavior change.- Restore
which/wherefallback with!isSelfReference(packages/cli/src/index.ts:171): upstream PR junhoyeo#585 deliberately removed the fallback to escape the bun-optionalDependencies fork-bomb. Gemini's!isSelfReference(found)guard is a fair refinement that recovers the original UX without re-opening the loop โ but the decision is still upstream's, since junhoyeo#585's author already weighed safety vs. fallback coverage.
This PR
Stays focused on (a) the upstream sync as-is and (b) the fork-only R2 normalization + the type-check fixes in ab4d8ed5. Internal QA energy in this PR is best spent on R2 (KST same-day buckets) and the multi-device merge interaction with the new upstream parsers โ that's where the fork's risk surface actually is.
Picks up the upstream commits landed since b05d378: - 64e278c Merge origin/main (coverage badge) - 1ac617e docs(cli): document local active-time day buckets - 8e5778d fix(cli): write Trae sync artifacts only when referenced - 1a1efd8 refactor(leaderboard): remove duplicate date validation - 6d0fe74 fix(groups): reject non-object invite payloads - 013552e fix(leaderboard): preserve filters across view toggles Conflict-free auto-merge per pre-flight dry-run (only leaderboard/page.tsx auto-merged a hunk). Keeping the catch-up in this PR so fork main lands even with upstream after merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audit pass after the upstream sync turned up two regressions in our migration set, both rooted in 0006_enable_rls (02ebf31) being scoped to the public tables that existed in April 2026: 1. **Security regression.** 0006 enables RLS and revokes anon/authenticated GRANTs on {users, sessions, api_tokens, device_codes, submissions, daily_breakdown}. The 2026-05 sync introduced four new public tables (submitted_devices in 0007, groups/group_members/group_invites in 0009) โ none of which inherit RLS. On Supabase deployments PostgREST then exposes them to anon/authenticated keys, which is exactly the class of leak 0006 was written to prevent. A self-hosted KST team's group memberships and invite tokens leaking via `GET /rest/v1/groups` is the concrete failure mode. Add 0012_extend_rls_to_synced_tables.sql, mirroring 0006's pattern exactly: ALTER TABLE โฆ ENABLE ROW LEVEL SECURITY, no policies, and a guarded REVOKE of anon/authenticated grants. Backend (postgres role, BYPASSRLS) is unaffected. 2. **Migration journal idx collision.** The sync left two pairs of duplicate idx values in meta/_journal.json โ idx 6 on both 0006_enable_rls and 0006_rehash_plaintext_personal_tokens, idx 7 on both 0007_add_case_insensitive_username_index and 0007_add_submitted_devices. drizzle-kit's generate path uses `lastIdx + 1` to number new migrations, so the collision could produce a third "idx 7" the next time someone runs `db:generate`. Reassign idx values sequentially in the existing entry order. Tags stay untouched, so __drizzle_migrations rows already applied in production are not re-executed. Verified post-fix: - jq: no duplicate idx, no duplicate tag in _journal.json - tsc --noEmit clean Tags pre-existing in prod (0006_enable_rls, 0007_add_case_insensitive_username_index) are unchanged โ they will not re-run on the next migrate. Tables newly introduced by the sync get RLS for the first time via 0012. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@woongeun-mp ๊ทธ๋ฆฌ๊ณ Upstream์ CLI๋ฅผ ๊ทธ๋๋ก ์ฐ๋ฉด ์ด ๋ฌธ์ ๊ฐ ํด์๊ฐ ์๋๊ณ ์ง์๋๋ค๋ค์... ๐ญ ์ด์ชฝ์์ ๋ฆด๋ฆฌ์ฆ๋ฅผ ํ๋๋ก ํด์ ํ์๋ค์๊ฒ ๊ณต์ ํ๊ฑฐ๋, ๊ทธ๋ฅ ๋จ์ ์ ์ธ์งํ๊ณ ์ฐ๊ฑฐ๋ ํด์ผํ ๊ฒ ๊ฐ์ต๋๋ค. |
Summary
Two commits for clarity:
Why this PR exists (beyond a routine sync)
PR #2 fixed the CLI side: it no longer drops contributions whose date is past UTC today. But the server still trusted the CLI's date string (`chrono::Local` on the submitter's machine). So a teammate submitting at KST 00:30 from a UTC-7 laptop would still write a row with `date = 2026-05-25`, and KST 5/26 "Today" would not see it.
Upstream PR junhoyeo#187 had already added `timestampMs` to every contribution payload โ they store the earliest-message timestamp end-to-end, but explicitly keep `daily_breakdown.date` in the CLI's local-day format so each user sees their own local day on the global SaaS. That's the wrong default for a self-hosted, single-TZ deployment.
This PR introduces a single env-gated server-side rebucket. When `LEADERBOARD_TIMEZONE` is set, the server recomputes the row's day from `timestampMs`; when it is unset, the server keeps upstream behavior. So this change is also a clean candidate for upstream contribution โ it does not break their default.
Sync conflict notes (worth a careful look during review)
Spelled out in the merge commit body. Highlights:
`c3c7844` ("scope client-slot wipe to each day's incoming clients") flagged the risk of tighter upstream parsers regressing device-aware merges. QA should pay extra attention to multi-device submits and daily aggregation around midnight in Asia/Seoul before promoting.
R2 design (a8d9fd9)
```ts
// packages/frontend/src/lib/leaderboard/normalizeContributionDate.ts
export function normalizeContributionDate(
rawDate: string,
timestampMs: number | null,
timezoneOverride: string | null,
): string {
if (!timezoneOverride) return rawDate; // upstream default
if (timestampMs == null) return rawDate; // legacy CLI (schemaVersion = 0)
return new Date(timestampMs).toLocaleDateString("en-CA", {
timeZone: timezoneOverride,
});
}
```
Used in `/api/submit` per incoming day; the normalized string drives both `existingDaysMap` lookup and the INSERT `date`.
Test plan
Follow-ups (not in this PR)
๐ค Generated with Claude Code