Repository navigation
feat(providers): dashboard control for the advertised CLI client version - #14817
adivekar-utexas wants to merge 12 commits into
Conversation
|
Follow-up (d76de26) after another pass for thoroughness. Closed a real robustness gap. 10 more tests (32 -> 42): One of those tests initially failed and the bug was in the test fixture, not the card: the mock's GET already reported the override as set, so the draft matched and Enter was correctly ignored as a no-op. Worth recording, because it means the unchanged-draft guard is doing real work rather than being decorative. |
|
Heads-up on the e2e spec added in 00a9b6a (tests/e2e/providers-cli-version.spec.ts). It renders the real /dashboard/providers/{claude,codex,openai} pages in Chromium and asserts the card appears with the right caveat per kind, is absent on an unrelated provider, and round-trips an override through PUT. It follows the same harness as providers-bailian-coding-plan.spec.ts. It is NOT exercised on this PR, because tests-e2e lives in ci.yml which only triggers on PRs to main. It will run at the release-to-main merge and on the nightly. The component behaviour itself is covered by 42 unit tests on this branch. I could not run it locally. The isolated production build expands to fill whatever heap it is given and then OOMs: measured live-at-OOM 3963 MB under a 4 GB ceiling and 5837 MB under a 6 GB ceiling, i.e. 97% and 95% of the cap each time. check:build-scope reports no leak (9627 files, under the 12000 cap), Node 22.22.2 is inside the engines range, and .build is only 235M so it is not self-feeding output. The repo's own comment in build-next-isolated.mjs notes that heap size does not fix a poisoned scope. This looks like a local environment limit rather than anything in this diff. |
|
Nice work — the leaf-module design (no DB read on the header hot path), reject-don't-coerce validation and the |
|
Both asks are done (1cbcd20). 1. OpenAPI. 2. Also fixed: Remaining CI red, all pre-existing on
I have left those alone rather than folding unrelated drift into this PR. Happy to send a separate small PR for the migration counts and the five Thanks for the note on the |
|
Re-homed to |
Anthropic and OpenAI gate new models on the CLI client version they see (The gpt-6-astra model requires a newer version of Codex; Anthropic does the same for new Opus/Sonnet tiers), and the captured pins rot as operators real CLIs move on. The only knob was an env var plus a restart. Adds a dashboard override on top of the env var and the pin: - src/shared/constants/cliVersions.ts: dependency-free leaf store. A module-level map, not a settings read, because these getters sit on the per-request header path. Malformed values are dropped, never coerced. - claudeCodeClient.ts and open-sse/config/codexClient.ts resolve through it and report which layer won (settings, then env, then pin). Behaviour is byte-identical when no override is set; for Codex that means a caller-forwarded version still beats CODEX_CLIENT_VERSION, deliberately. - GET/PUT /api/settings/cli-versions rejects a bad version loudly instead of silently falling back to the pin the way the env vars do. - Hydration on cold boot AND on every settings write, so import-json and config restores take effect and a restart does not drop the override (the diegosouzapw#5312 RC-A failure mode). - ProviderCliVersionSection on the claude/codex provider pages, surfacing the effective version, its source, and the billing-version caveat. 32 tests: normalization, per-layer precedence, Codex caller forwarding, hydration idempotence and corrupt-settings resilience, route auth/validation/ clear-on-null, and the component render and save flows.
…dances
applyRuntimeSettings: an ABSENT cliVersionOverrides field must not clear a
live override. Both production callers pass getSettings() output where the key
is always present (default {}), but several existing tests, and any future
caller, pass a PARTIAL settings object. A silent revert to the pin is
invisible in a way the visible toggles above it are not, so absence now means
"not carried" rather than "the operator cleared it". An explicit empty map
still clears.
Adds 10 tests: ProviderExtraPanels registration for claude/codex only, the
disabled Save/Reset states, Enter-to-submit, failed-save draft retention, the
loading skeleton, unrelated settings writes preserving the override, partial
settings safety, explicit-clear semantics, the getSettings default, and
cross-kind isolation.
Renders the actual /dashboard/providers/{claude,codex,openai} pages in
Chromium and asserts the card appears with the right caveat per kind, is
absent on an unrelated provider, and round-trips an override through PUT.
The unit tests already covered the component in isolation; this is the
hydration-level check that the card really shows up in the dashboard.
Addresses both review asks. - docs/openapi.yaml documents GET/PUT /api/settings/cli-versions and adds a CliVersionStatus component schema. check:openapi-coverage goes 701/719 to 702/719 and check:openapi-routes stays green. - src/i18n/messages/en.json gains the 18 cliVersion* keys the card uses, with values matching the providerText() fallbacks exactly. Only en.json, per the note that other locales are synced separately. i18nUiCoverage 99.7% and the quality ratchet is OK (57 metrics, 3 improved). Also fixes the unused CLI_VERSION_PATTERN import in the store test that was failing the no-new-ESLint-warnings gate, and uses it to assert the exported pattern agrees with normalizeCliVersion at the 32/33 character boundary.
2ca5c78 to
09198e9
Compare
… timing CI on release/v3.8.52 failed three checks that this PR caused: - i18n-vi-completeness and i18n-pt-br require key parity with en.json, so the 18 new providers.cliVersion* keys are added to vi.json and pt-BR.json, following the same pattern as other en.json additions. - balance-e2e-shards requires every e2e spec to have an entry in config/quality/e2e-timings.json. providers-cli-version.spec.ts is seeded with its line count (136), which is the convention the file uses for its weights.
|
Follow-up on the CI rerun: the timings-seed fix and vi/pt-BR cliVersion translations now pass all eight full unit shards and all four fast-path shards. API Route Typecheck and fast-path Vitest also pass. One PR-specific blocker remains: i18n UI Coverage runs check-new-key-coverage.mjs and rejects the 18 new providers.cliVersion* keys because they are not translated into the other locales. This conflicts with the earlier request to add en.json only and sync other locales separately. I have not changed that gate, added placeholder translations, or expanded this PR into a full locale sync. Could the release captain run the normal translation sync, or advise the intended handling for this PR? Other failures in this run are outside the feature: merge-integrity reports generated omni-providers drift (not omni-settings); Fast Quality Gates reports complexity regressions in deepseekWebTools/opencodeHeaders/sidebarSearch plus existing cycles; Docs Sync reports repository-wide version/date findings; Lint fails npm audit; full Vitest reports post-teardown window-is-not-defined errors in providerCardAudioBadge/compressionCockpit through FlowCanvas. Build stopped because the runner received a shutdown signal, not a reported compilation failure. Keeping unrelated changes out of this PR. No local production build or e2e run was attempted. |
|
Rechecked every discussion comment and inline review comment: no additional maintainer requests or inline review threads exist. Both requested changes are present: GET/PUT OpenAPI documentation and all 18 English cliVersion keys. OpenAPI routes and coverage pass (703/722 documented). Pushed 68ec6a3 after finding and reproducing an async lifecycle bug: a load failure arriving after the card unmounted still showed an error notification. The new regression failed before the change and passes with effect cleanup, which also ignores obsolete provider loads. Fresh local validation: 31 feature node tests plus 5 shard-balancing tests pass, 12 component tests pass, 8 vi/pt-BR locale tests pass, changed component/test ESLint and diff checks pass. Build scope is clean (9923 files). Every local Node process stayed within 1 GB. No production build, dashboard typecheck, or e2e run was attempted under the operator machine limits; the Playwright spec remains unexecuted locally. The outstanding all-locale gate is still not resolved: the translation backend is unconfigured locally. I have not inserted English copies/placeholders or weakened the gate. Please advise whether the release translation sync can supply the remaining locales, consistent with your earlier instruction to sync them separately. The feature is not claimed fully green; fresh CI is pending. |
Reconcile with the automatic version discovery that landed on the base: dashboard override > env > discovered cache > pin for both CLI kinds, rename the caller-aware Codex resolver to resolveCodexAdvertisedVersion (the base already exports resolveCodexClientVersion(fetchImpl)), add the `discovered` source to the API/UI/openapi, and leave the generated skill docs at the base content. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Hi @adivekar-utexas — closing this pull request to clear the open queue. The branch |
|
Hi @adivekar-utexas — reopening this pull request. The close was a mistake, and it is back in the merge queue. The branch |
What
A dashboard control for the CLI client version OmniRoute advertises for the Claude Code and Codex identity presets, so an operator can get past upstream model gates without an env var and a restart.
Both upstreams gate new models on the client version they see. Anthropic gates new Opus/Sonnet tiers; OpenAI returns "The gpt-6-astra model requires a newer version of Codex". The captured pins rot as operators' real CLIs move on, and until now the only knob was
CLAUDE_CODE_CLIENT_VERSION/CODEX_CLIENT_VERSIONplus a process restart.Renders on the
claudeandcodexprovider pages only.Precedence
settings->CLAUDE_CODE_CLIENT_VERSIONenv -> pin2.1.258settings-> caller-forwarded ->CODEX_CLIENT_VERSIONenv -> pin0.155.0settings-> env -> pinWhen
settingsis unset the behaviour is byte-identical to today. For Codex that means the caller's own forwarded version still beats the env var. That is surprising, but it is deliberate existing behaviour (getCodexClientVersionFromHeadersdocuments why: a pinned default silently rots every time the user upgrades their CLI), so it is preserved and pinned by a test. The dashboard override outranks the caller because it is explicit operator intent and this was asked for as a global setting.The API returns
source(settings/env/default) per kind on purpose: the usual support question is "why isn't my override taking effect", and the answer is almost always the precedence order.Two footguns worth flagging
${version}.${CLAUDE_CODE_CLIENT_BUILD_REVISION}, and the build revision is a separate pinned constant (1e2). Overriding to2.1.260reports2.1.260.1e2, which is not what a real 2.1.260 binary sends.CLAUDE_CODE_SDK_PACKAGE_VERSIONandCLAUDE_CODE_RUNTIME_VERSIONdo not move either. The card says this inline and a test pins it.DEFAULT_CODEX_CLIENT_VERSION = "0.155.0"looks ahead of every real Codex release I could find (latest surfaced0.153.4). Advertising a version higher than any real release may itself be a fingerprint. GPT-6 Astra needs>= 0.153.0. Worth a maintainer opinion; not changed here.Implementation notes
src/shared/constants/cliVersions.tsis a dependency-free leaf with a module-level map, not a settings read: these getters sit on the per-request header path (claudeCodeClient.tsis explicitly a leaf shared by executors and compatibility bridges, so it cannot import a service).PUT /api/settings/cli-versionsrejects loudly instead, which is the fix for that documented footgun.instrumentation-node.ts(cold boot) andapplyRuntimeSettings(soimport-jsonand other settings writes take effect). ThethinkingBudget#5312 RC-Abug is the cautionary tale: an in-memory config with no boot read resets on every restart.cliVersionOverridesfield is treated as "not carried", not "cleared". Both production callers passgetSettings()output, where the key is always present (default{}), but several existing tests callapplyRuntimeSettingswith a partial settings object. Clearing silently would drop a value that goes onto the wire as a client fingerprint, which is invisible in a way the visible toggles are not. An explicit{}still clears.Tests
42 new tests, all passing:
tests/unit/cli-version-overrides.test.ts(18),tests/unit/settings/cli-version-overrides-route.test.ts(13),ProviderCliVersionSection.test.tsx(11). Covers normalization accept/reject, per-layer precedence in isolation, Codex caller forwarding with headers present/absent/invalid, hydration idempotence and corrupt-settings resilience, route auth/validation/clear-on-null/persistence, and the component's render-only-for-claude/codex and save/reset flows. The UI cases also pin the panel registration, the disabled Save/Reset states, Enter-to-submit, draft retention on a failed save, and the loading skeleton.Gate evidence (local)
Clean:
typecheck:core,check:open-sse-typecheck,check:dashboard-typecheck(197 errors, all inside the frozen baseline), ESLint on every touched file,check:file-size,check:env-doc-sync(no env vars added),check:test-discovery,check:mutation-test-coverage --strict,check:deps,check:dead-code(369 vs 377 baseline),check:complexity-ratchets(2925 vs 3218; 1324 vs 1437),check:forgotten-sibling-tests,check:ai-attribution.Zero complexity regressions per file; both new files are at 0 violations.
Pre-existing on
release/v3.8.51and not caused by this PR (verified by re-running on a pristine base worktree):check:docs-counts(3-4 STRICT drifts),i18n:check-keys(bslocale missing 16combos.*keys),check:mutation-test-coveragedrift listing 5 upstream tests (flagship-0day-discovery,metered-budget-must-not-disable-flat-rate,expiry-first-account-rotation,claude-passthrough-empty-response).check:known-symbolscannot run locally (bun not installed in this checkout).No i18n keys were added to
en.jsonon purpose:check-new-key-coveragerequires a real translation in all 66 locales and the translation backend is not configured here. The card usesproviderText()with literals, which is the supported fallback path and renders correct English everywhere.