Skip to content

feat(cli)!: remove deprecated per-client boolean flags#465

Draft
junhoyeo wants to merge 8 commits intomainfrom
chore/remove-deprecated-client-flags
Draft

feat(cli)!: remove deprecated per-client boolean flags#465
junhoyeo wants to merge 8 commits intomainfrom
chore/remove-deprecated-client-flags

Conversation

@junhoyeo
Copy link
Copy Markdown
Owner

@junhoyeo junhoyeo commented Apr 25, 2026 •
edited
Loading

Warning

DO NOT MERGE until next major release (v3.0.0).
This PR removes deprecated CLI flags. Merging before the deprecation window closes will break user scripts that still use --opencode, --claude, --codex, etc.

Depends on

This PR cannot be reviewed in isolation until #464 lands. Until then, the diff here includes #464's content. Once #464 merges into main, this branch's diff collapses to just the removal commits.

Summary

Removes the 19 legacy per-client boolean flags that were hidden from --help and marked deprecated by #464:

--opencode, --claude, --codex, --copilot, --gemini, --cursor, --amp,
--droid, --openclaw, --hermes, --pi, --kimi, --qwen, --roocode,
--kilocode, --kilo, --mux, --crush, --synthetic

The deprecation has been visible to users for the entire window between #464 and v3.0.0. Anyone running tokscale interactively on a TTY has been seeing a one-line stderr warning every time they used a legacy flag.

Migration

Before After
tokscale --opencode tokscale --client opencode
tokscale --opencode --claude tokscale --client opencode,claude
tokscale --synthetic tokscale --client synthetic

What changed

crates/tokscale-cli/src/main.rs

  • Drop the 19 hidden bool fields from ClientFlags.
  • Simplify build_client_filter_with_defaults: single canonical-path resolution, no legacy fallback loop.
  • Remove emit_legacy_client_flag_warning (and its TTY-gated stderr emission).
  • Drop 4 legacy-only unit tests. Replace the canonical-and-legacy mix test with a guard test that asserts --claude (and every other removed flag, table-driven) now produces a clap parse error. Plus a positive sweep verifying --client opencode|claude|synthetic still parse.
  • Add id = "client_filter" and value_name = "CLIENTS" to the --client arg. The id rename disambiguates the flattened arg from the outer struct's also-named clients field — clap requires globally unique arg ids and the legacy bool fields previously masked this collision. The explicit value_name preserves the <CLIENTS> placeholder in --help instead of leaking the auto-derived <client_filter>.

crates/tokscale-cli/tests/cli_tests.rs

Migrate 61 --<client> usages to --client <id> form (mechanical sed rename plus two manual .arg → .args fixes for assert_cmd's single-arg API).

READMEs

  • Drop the "Deprecation notice" paragraphs in all four READMEs (en/ko/ja/zh-cn).

Diff size

6 files changed, 117 insertions(+), 276 deletions(-)

Net -159 lines.

Test plan

  • cargo test -p tokscale-cli --bin tokscale — 369 pass, 0 fail (removed 4 legacy-only tests; kept the table-driven rejection guard).
  • cargo test -p tokscale-cli --test cli_tests — 83 pass, 0 fail (every test migrated to --client form).
  • Manual smoke verification:
    • tokscale --opencode → error: unexpected argument '--opencode' found ✓
    • tokscale --client opencode --json → works ✓
    • tokscale --help shows -c, --client <CLIENTS> (no <client_filter> leak) and no legacy flags ✓

Release notes

Suggested entry for the v3.0.0 changelog:

BREAKING: Removed the per-client boolean flags (--opencode, --claude, etc.). Use --client (short -c) instead. The flags were deprecated in v2.x and have been hidden from --help since.

junhoyeo and others added 3 commits April 26, 2026 05:05
@junhoyeo
feat(cli): add --client/-c filter flag, deprecate per-client booleans
753c605 
Replace 19 individual boolean flags (--opencode, --claude, --codex, ...)
with a single repeatable, comma-separated --client/-c flag backed by a
clap ValueEnum. Cleans up --help (19 lines collapse to 1) and stops
flag-list growth as new clients are added.

Legacy boolean flags remain functional but are hidden from --help and
will be removed in the next major release; using them prints a one-line
deprecation warning to stderr only when stderr is a TTY (so JSON
pipelines and tests stay clean).

The Synthetic meta-client is exposed as a first-class --client value
without changing tokscale-core: ClientFilter mirrors ClientId on the CLI
side, leaving the core crate's clap-free layering and synthetic
post-processing semantics intact.

Updates filtering docs in README.md, README.ko.md, README.ja.md, and
README.zh-cn.md.
@github-actions
style: auto-fix lint issues [skip ci]
0aaef42
@junhoyeo
feat(tui,cli): unify client filter around ClientFilter, add defaultCl…
b111a80 
…ients setting

Replace the split TUI state (HashSet<ClientId> + bool include_synthetic) with
a single HashSet<ClientFilter>, so the source picker, in-memory app state,
disk cache key, and CLI flag parser all share one representation. The
Synthetic meta-client now lives as a regular set member instead of a
parallel boolean.

Cache schema unchanged on disk: load_cache and save_cached_data still
serialize (enabled_clients: Vec<String>, include_synthetic: bool) so caches
written by older releases keep loading without a migration step. The
projection happens at the read/write boundary.

Add ClientFilter::to_client_id / from_client_id / from_filter_str helpers
for boundary code that still needs Vec<ClientId> (the loader API) or
parses canonical id strings (settings.json). Reorder ClientFilter
variants to mirror ClientId::ALL declaration order so --help possible
values, the TUI picker rows, and value_variants() iteration all agree on
a single chronological ordering, with Synthetic appended at the end.

Add a defaultClients setting (top-level in settings.json) that applies
when the user passes neither --client/-c nor a legacy boolean flag. CLI
flags always override defaults completely — no merging — so 'tokscale
--client codex' is not surprised by extra entries from settings. Unknown
ids in defaultClients are silently dropped to keep stale config from
breaking tokscale.

Refactor source_picker.rs to drop its local SourceOption enum and walk
ClientFilter::value_variants() directly. Hotkey for Synthetic stays 'x'
to preserve current display; it collides with Mux which also binds 'x'
in client_ui — pre-existing bug noted with a TODO, scoped out of this
change.

Update README.md / README.ko.md / README.ja.md / README.zh-cn.md
Configuration sections to document defaultClients.
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 25, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
tokscale Ignored Ignored Preview Apr 25, 2026 9:44pm

Request Review

junhoyeo and others added 2 commits April 26, 2026 06:09
@junhoyeo
fix(cli): exclude Synthetic from no-filter default + migrate stale RE…
308b471 
…ADME examples

Two related issues caught in code review:

1. The no-filter TUI default and the `submit` warm-cache filter set
   drifted apart. TUI launch with no `--client` flag enabled every
   ClientFilter including Synthetic; `run_warm_tui_cache()` warmed the
   cache with Synthetic excluded. Result: every TUI launch after
   `submit` was a stale-cache hit instead of fresh, defeating the
   warming. Pre-refactor behavior was 'every ClientId, include_synthetic
   = false'; this restores it.

   Fix: introduce `ClientFilter::default_set()` as the single source of
   truth for 'no filter' semantics. Both code paths (App constructor in
   tui/app.rs, the cache lookup in tui/mod.rs, and run_warm_tui_cache in
   main.rs) now consult it. Drop the now-unused `clap::ValueEnum`
   imports in tui/app.rs and tui/mod.rs.

   Synthetic detection has always been opt-in because it post-processes
   other clients' sessions to re-attribute messages — flipping it on
   silently for everyone is a real behavior change.

   Add unit tests:
   - `test_client_filter_default_set_excludes_synthetic` — guards the
     contract directly.
   - `test_app_no_filter_default_matches_default_set` — guards that
     App's constructor stays in lockstep.

2. Replace stale legacy-flag examples that still appeared in README
   sections unrelated to the 'Filtering by Platform' deprecation block:
   `tokscale models --week --claude`, `tokscale submit --opencode
   --claude ...`, `tokscale graph --opencode --claude`. After the major
   bump that removes the deprecated flags these examples become outright
   wrong; even before that they contradict the canonical interface.
   Updated in all four READMEs (en/ko/ja/zh-cn).

Test plan: 369 unit + 83 integration = 452 pass, 0 fail.
@github-actions
style: auto-fix lint issues [skip ci]
a0f5ae2
@junhoyeo junhoyeo force-pushed the chore/remove-deprecated-client-flags branch from 40ebeba to f1caf3d Compare April 25, 2026 21:11
@junhoyeo junhoyeo mentioned this pull request Apr 25, 2026
Merged
@junhoyeo
Copy link
Copy Markdown
Owner Author

junhoyeo commented Apr 25, 2026

Update: Oracle review — addressed LOW finding + rebased on updated #464

LOW: legacy-rejection guard was too narrow

The original `test_client_flags_legacy_long_flags_rejected` only checked `--claude`. Expanded to a table over all 19 removed flags with `--synthetic` called out explicitly — it's the only legacy flag without a matching `ClientId`, so its handling has always been special-cased and is the most likely to regress if anyone resurrects a boolean field. Also added a positive sanity sweep to verify `--client opencode`, `--client claude`, and `--client synthetic` all still parse, so the negative test cannot silently pass via an over-tight parser.

Pushed as `f1caf3d test(cli): expand legacy-flag rejection guard to all 19 removed flags`.

Rebased on updated #464

Branch was rebased after `308b471` landed on `feat/cli-client-filter-flag`. The rebase pulls in:

  • The HIGH-severity `ClientFilter::default_set()` fix (no-filter TUI / warm-cache mismatch)
  • The MEDIUM-severity README example migration (stale `--claude` / `--opencode` examples in non-deprecation sections)

So once #464 lands, this branch's diff collapses cleanly to just the removal commits with no carryover noise.

Numbers (this branch only)

PR remains DRAFT — DO NOT MERGE until v3.0.0.

junhoyeo added 3 commits April 26, 2026 06:41
@junhoyeo
fix(cli): warm-cache honors defaultClients + make build_client_filter…
1ccb7dc 
… tests hermetic

Two issues caught in PR code review:

1. **`run_warm_tui_cache` ignored `defaultClients`** (devin-ai-integration P2)

   `run_warm_tui_cache` always used `ClientFilter::default_set()` (every
   real client, Synthetic excluded), regardless of the user's
   `defaultClients` setting. `tui::run` on a no-flag launch DOES honor
   `defaultClients` via `build_client_filter` upstream. Result: a user
   with `defaultClients = ["opencode", "claude"]` got a warm cache
   storing all 18 clients while the next `tokscale` launch wanted only
   the 2 configured ones — guaranteed cache miss, defeating the warming.

   Fix: extract `resolve_default_tui_filter_set` mirroring the
   no-flag-launch resolution chain (defaultClients → fall back to
   default_set). Both `run_warm_tui_cache` and the documentation comment
   now consult it.

2. **`build_client_filter` tests were non-hermetic** (cubic-dev-ai P2)

   The 5 unit tests for `build_client_filter` were calling the wrapper
   that loads `~/.config/tokscale/settings.json`. They passed for me
   (no `defaultClients` set) but a developer with their own
   `defaultClients` would get unrelated filter ids appended to every
   assertion. Pure variant `build_client_filter_with_defaults` already
   existed; these tests now use it with `&[]` for hermetic input.

Adds:
- `resolve_default_tui_filter_set_with(&[String])` — pure variant of
  the resolver, unit-testable.
- 4 new tests: configured defaults override, empty fallback, unknown id
  drop, all-unknown fallback to `default_set`, synthetic supported via
  `defaultClients` for opt-in power users.

Test plan: 374 unit + 83 integration = 457 pass, 0 fail.
@junhoyeo
feat(cli)!: remove deprecated per-client boolean flags
dd7efd1 
BREAKING CHANGE: The legacy single-client flags
(--opencode, --claude, --codex, --copilot, --gemini, --cursor, --amp,
--droid, --openclaw, --hermes, --pi, --kimi, --qwen, --roocode,
--kilocode, --kilo, --mux, --crush, --synthetic) are removed.

Use --client/-c instead:

  tokscale --client opencode,claude
  tokscale -c opencode -c claude

The flags were marked deprecated and hidden from --help on the same
release that introduced --client. They have always been a strict subset
of --client's functionality, so migration is a mechanical rename.

This commit:
- Drops the 19 hidden bool fields from ClientFlags
- Simplifies build_client_filter_with_defaults: single canonical path
  plus the defaultClients fallback
- Removes emit_legacy_client_flag_warning and the IsTerminal-gated
  stderr emission
- Drops the four legacy-only unit tests; replaces the
  canonical+legacy-mix test with a guard test that asserts --claude
  now produces a clap parse error
- Migrates every --opencode / --claude / etc. usage in cli_tests.rs to
  the --client form (61 sites)
- Drops the 'Deprecation notice' paragraph from README.md / README.ko.md
  / README.ja.md / README.zh-cn.md

Adds id="client_filter" + value_name="CLIENTS" to the --client arg.
The id rename disambiguates the flattened arg from the outer struct's
also-named clients field (clap requires globally unique arg ids and the
legacy bool fields previously diluted this collision). The explicit
value_name preserves the <CLIENTS> placeholder in --help instead of
auto-deriving <client_filter>.

Net change: -159 lines.
@junhoyeo
test(cli): expand legacy-flag rejection guard to all 19 removed flags
d87330b 
Per code review feedback, the original guard only checked --claude. Make
the test table-driven over every removed flag, with --synthetic called
out explicitly because it is the only legacy flag that did not have a
matching ClientId — its handling has always been special-cased and is
the most likely surface to regress if anyone resurrects the boolean
fields.

Also adds a positive sanity sweep to ensure --client opencode / claude /
synthetic still parse, so the test does not silently pass via an
over-tight parser.
@junhoyeo junhoyeo force-pushed the chore/remove-deprecated-client-flags branch from 467c10f to d87330b Compare April 25, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@junhoyeo