Skip to content

fix(discover+ccusage): Windows drive colon not sanitized in slug + accept ccusage 20.x 'period' field - #2370

Closed
mickel-am wants to merge 1 commit into
rtk-ai:developfrom
mickel-am:fix/windows-discover-slug-and-ccusage-period
Closed

mickel-am wants to merge 1 commit into
rtk-ai:developfrom
mickel-am:fix/windows-discover-slug-and-ccusage-period

Conversation

@mickel-am

Copy link
Copy Markdown

Two independent bugs found running rtk on Windows (Win10) with ccusage 20.9.

Bug 1 — rtk discover scans 0 sessions on Windows

Affected: all Windows paths with a drive letter (e.g. f:\AMPLIA_APP 1)

Root cause: encode_project_path in src/discover/provider.rs did not include : in SANITIZED_CHARS. Claude Code replaces both backslash and colon with - when generating project slugs:

f:\AMPLIA_APP 1  →  f--AMPLIA-APP-1   (Claude Code / actual directory)
f:\AMPLIA_APP 1  →  f:-AMPLIA-APP-1   (RTK before this fix)

The substring match in discover_sessions_in_projects_dir always missed every project directory on Windows.

Fix: Add : to SANITIZED_CHARS.

  • Corrects the test_encode_project_path_windows assertion (was asserting the wrong slug)
  • Adds test_encode_project_path_windows_with_space as a regression test

Bug 2 — rtk cc-economics shows $0 with ccusage ≥ 20.x

Root cause: ccusage 20.x changed the monthly JSON field from "month" to "period" in the all-agents response. Serde deserialization silently returned Ok(None), zeroing out the economics report.

Fix: Add #[serde(alias = "period")] to MonthlyEntry.month in src/analytics/ccusage.rs — backwards-compatible, accepts both old and new field names.

  • Adds test_parse_monthly_period_field as a regression test

…x 'period' field

**Bug 1 — discover finds 0 sessions on Windows**
Claude Code converts `:` in Windows drive paths to `-` when generating
project slugs (e.g. `f:\AMPLIA_APP 1` → `f--AMPLIA-APP-1`), but
`encode_project_path` was not sanitizing `:`, producing `f:-AMPLIA-APP-1`
and failing the substring match. Fix: add `:` to SANITIZED_CHARS.
Updates the existing Windows test and adds a regression test for paths
with spaces.

**Bug 2 — cc-economics shows $0 with ccusage 20.x**
ccusage 20.x changed the monthly JSON field from `"month"` to `"period"`
in the all-agents response. `MonthlyEntry` deserialization was silently
returning Ok(None), zeroing out the economics report. Fix: add
`#[serde(alias = "period")]` so both old and new field names are accepted.
Adds a regression test covering the new field name.

Reported-by: mickel.baptista@andrademaia.com
@CLAassistant

CLAassistant commented Jun 10, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@rtk-release-bot

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale due to 90 days of inactivity. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. Your contribution is very much appreciated — we're sorry we haven't been able to review it yet.

@KuSh

KuSh commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Thanks for chasing both of these down on a real Windows box. The drive-letter colon in the project slug was fixed by #2952, merged on 2026-08-13, which added ':' to SANITIZED_CHARS in src/discover/provider.rs and corrected the Windows test to expect C--Users-foo-bar. The ccusage field rename was fixed by #2732, merged on 2026-07-07, which added the "period" serde alias to the daily, weekly and monthly entries with a test for each, so the monthly alias here is already present. The only thing left is the combined f:\AMPLIA_APP 1 test input, and both halves of what it asserts are covered by the existing colon and space tests. Closing as covered, but please comment or reopen if you think a case was missed.

@KuSh KuSh closed this Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants