diff --git a/.github/CI.md b/.github/CI.md index da6aab67b7..559f1804ce 100644 --- a/.github/CI.md +++ b/.github/CI.md @@ -89,7 +89,8 @@ Both jobs use CircleCI's hosted Windows executor (`circleci/windows@5.0`). 2. The credential-free build validates the canonical remote, full tag SHA, tag-to-SHA identity, protected `main` ancestry, and every project version file. It provisions/asserts Node 24.x via the `OpenJS.NodeJS.LTS` winget - package, pnpm 11.24.0, the Rust MSVC target, Git, and Inno Setup 6. + package, the exact pnpm version pinned by `apps/desktop-tauri/package.json`, + the Rust MSVC target, Git, and Inno Setup 6. 3. It uses a new temporary `WorkRoot`, runs `release-doctor.ps1`, then runs `windows-release-build.ps1` with the immutable SHA and `-SmokeInstall`. It never uploads. Six assets — `CodexBar--Setup.exe` and its diff --git a/AGENTS.md b/AGENTS.md index e8499fa1f9..48e7e0e430 100755 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,7 +108,7 @@ pnpm run tauri:build ## Runtime/Tooling Preferences -- Package manager: **pnpm@11.24.0** (`packageManager` in `apps/desktop-tauri/package.json` + lockfile). Do not introduce npm or yarn lockfiles. +- Package manager: **pnpm**, with the exact version pinned only by `packageManager` in `apps/desktop-tauri/package.json` (and reflected by the lockfile). Do not introduce npm or yarn lockfiles. - Node: CircleCI pins **Node 24.18.0**; no `.nvmrc` in repo. Prefer Node 24.18.0 locally for hosted parity. - Rust: edition **2024**, stable toolchain; CI target `x86_64-pc-windows-msvc`. No committed `rust-toolchain.toml` / `rustfmt.toml` / `clippy.toml` — defaults plus CI flags (`clippy -- -D warnings`). - Tray / DPAPI / browser-cookie behavior: validate on **Windows-native** hosts. WSL/Linux is insufficient for those paths. diff --git a/apps/desktop-tauri/package.json b/apps/desktop-tauri/package.json index 5f2e4a6189..dd9e83435b 100644 --- a/apps/desktop-tauri/package.json +++ b/apps/desktop-tauri/package.json @@ -2,7 +2,7 @@ "name": "desktop-tauri", "private": true, "version": "0.55.0", - "packageManager": "pnpm@11.24.0", + "packageManager": "pnpm@11.25.0", "type": "module", "scripts": { "dev": "vite", diff --git a/docs/adr/0006-dependency-consolidation-candidates.md b/docs/adr/0006-dependency-consolidation-candidates.md new file mode 100644 index 0000000000..df45f9f388 --- /dev/null +++ b/docs/adr/0006-dependency-consolidation-candidates.md @@ -0,0 +1,245 @@ +# ADR 0006: Dependency consolidation candidates + +Date: 2026-09-05 +Status: Proposed + +## Context + +Win-CodexBar has grown across roughly 70 providers, a Rust backend, a Tauri 2 +shell, and a React frontend. The codebase now contains several places where the +same domain information or infrastructure policy is expressed repeatedly. +Adding dependencies is only worthwhile when a crate removes measurable +boilerplate, reduces synchronization points, or materially improves +correctness. This ADR records candidates for review before implementation. It +does not approve adding every crate listed below. + +The September 2026 audit found these concrete pressure points: + +- `rust/src/core/provider.rs` is 1,131 lines. `ProviderId` has 70 variants, with + repeated handwritten tables for iteration, CLI names, display names, cookie + domains, aliases, and brand colors. The largest repeated metadata blocks span + roughly 540 lines. +- `apps/desktop-tauri/src/types/bridge.ts` is 958 lines with 101 exported + types/interfaces that must stay aligned with Rust DTOs. +- `apps/desktop-tauri/src/lib/tauri.ts` is 534 lines with 91 handwritten invoke + wrappers, while the Rust shell exposes 98 `#[tauri::command]` functions. +- `FetchContext` is manually constructed at 26 call sites. +- The Rust code contains about 20 `lock().unwrap*` sites, including eight + explicit poison-recovery patterns using `into_inner()`. +- There are many manual `sort`, `dedup`, `HashSet` uniqueness, grouping, and + collection reshaping paths across provider, session, cost, and dashboard + code. +- URL-like configuration and command inputs are commonly represented as raw + `String`/`&str`, including gateway, proxy, dashboard, status, release, and + provider endpoint URLs. `validate_external_url` currently performs manual + HTTP(S) prefix validation. +- Provider and configuration parsing has many generic Serde error mappings that + report the parser error but not the precise failing field path. +- Several in-memory caches hand-roll `Mutex>`, timestamps, and TTL + checks for provider, chart/local-usage, status, token, and pricing data. +- Cost and spend calculations are extensively represented as `f64`. + +## Decision + +Use a tiered adoption policy. A dependency is added only in the PR that also +uses it to remove or centralize existing code. Do not add dormant dependencies +for hypothetical future use. + +### Tier 1: highest-confidence candidates + +#### `strum` + +Use derives such as `EnumIter`, `EnumString`, `Display`, `VariantArray`, and +variant metadata where they eliminate repeated enum plumbing. + +Primary target: `ProviderId` and other enums that manually implement `all`, +`parse`, `as_str`, `display_name`, or alias tables. + +Acceptance criteria: + +- provider metadata has one obvious source of truth per property; +- aliases and compatibility names remain behaviorally identical; +- exhaustive provider factory behavior remains compile-time checked; +- net handwritten LOC decreases materially. + +#### `ts-rs` + +Generate TypeScript DTO definitions from Rust for bridge types that are true +wire contracts. Keep frontend-only unions/types handwritten when Rust is not +the source of truth. + +Primary target: the mirrored portions of `src/types/bridge.ts`. + +Acceptance criteria: + +- generated files are deterministic and checked in or generated by an explicit + reproducible step; +- Serde rename/tag behavior is covered by tests; +- the generated boundary removes manual Rust/TypeScript synchronization rather + than introducing a second schema layer; +- no runtime dependency is added to the frontend. + +Do not adopt `tauri-specta` in the same step unless its Tauri 2 release line is +independently proven stable and it replaces enough of `src/lib/tauri.ts` to +justify the additional integration surface. + +#### `serde_with` + +Use only where it replaces custom Serde adapters or repetitive conversion +logic, for example display/from-string bridges or optional/default handling. + +Acceptance criteria: each use deletes or simplifies existing serialization +code; avoid cosmetic attribute churn. + +#### `url` + +Promote URL values to `url::Url` at validation/security boundaries where the +value is semantically a URL. The crate is already present transitively through +`reqwest`, so making it direct should not materially enlarge the dependency +graph at the current lockfile state. + +Primary targets: external URL validation and provider/gateway/proxy endpoints. + +Acceptance criteria: + +- scheme allowlists stay explicit; +- file/javascript and other unsafe schemes remain rejected where applicable; +- persisted wire compatibility is preserved when settings currently store + strings. + +#### `parking_lot` + +Use for internal synchronization where standard-library poisoning semantics are +not part of the intended behavior and where it removes repeated poison recovery. + +Acceptance criteria: + +- no lock is held across `.await`; +- migration is focused rather than repo-wide mechanical churn; +- test-only locks do not justify the dependency by themselves. + +#### `itertools` + +Use for collection transforms that become materially clearer, especially +unique/sorted/grouped pipelines that currently require temporary sets or +multi-step mutation. + +Acceptance criteria: the replacement is shorter and easier to read than the +standard-library version; simple iterator chains remain standard library only. + +#### `bon` + +Prototype on `FetchContext`, which has 26 literal construction sites, before +using it elsewhere. + +Acceptance criteria: + +- call sites communicate only intentional overrides; +- defaults remain centralized; +- builder generation does not obscure required fields or make diagnostics + worse; +- measurable net LOC/readability improvement is shown in the prototype diff. + +#### `serde_path_to_error` + +Use at user-facing or provider-facing JSON boundaries where a precise field path +would materially improve diagnostics. + +Acceptance criteria: preserve the existing provider-specific context while +adding the failing data path; do not wrap silent best-effort parsers whose +failure is intentionally ignored. + +### Tier 2: architecture-specific candidates + +These need a focused prototype and should not be added merely because the crate +is useful in general. + +#### `tokio-util` + +Candidate for `CancellationToken` in refresh/probe/background-task ownership. +It is already present transitively through the current HTTP stack. Adopt only +where it replaces bespoke cancellation flags/channels and clarifies task +lifetime. + +#### `secrecy` + +Candidate for API keys, tokens, and other credentials that currently travel as +ordinary `String`s. Adoption must integrate with existing DPAPI, keyring, +redaction, and persistence behavior rather than creating a parallel secret +model. + +#### `moka` + +Candidate for simple in-memory TTL caches currently implemented as +`Mutex>` plus timestamps. Do not replace the JSONL/disk cost cache +or other caches with domain-specific persistence/invalidation semantics unless a +prototype proves equivalence. + +#### `reqwest-middleware` and `reqwest-retry` + +Candidate for genuinely shared idempotent HTTP retry/backoff policy. The repo is +currently on `reqwest 0.12`, so compatible middleware versions must be verified +before adoption. Provider-specific authentication refresh, WAF fallback, +alternate-host logic, and semantic retries must remain provider-owned. + +#### `rust_decimal` + +Candidate for monetary totals if floating-point rounding is demonstrated to be +a product-level correctness problem. Migration would be broad because cost, +pricing, bridge DTOs, formatting, tests, and persisted/exported values currently +use `f64`. + +Do not migrate merely for theoretical precision. First add a failing test or +real rounding example that changes an observable result. + +#### `phf` + +Candidate for immutable lookup tables that are currently runtime-built +`LazyLock>` values. Prefer ordinary `match`, slices, or arrays when +those are already clearer and compile-time cheap. + +## Rejected default: dependency accumulation + +Do not add all candidates in one dependency-only PR. That would increase build, +supply-chain, maintenance, and audit surface while proving none of the claimed +benefits. + +The preferred sequence is small implementation PRs, each with before/after +evidence: + +1. `strum` for provider/enum metadata. +2. `ts-rs` for Rust-to-TypeScript wire types. +3. `url` for URL validation boundaries. +4. `bon` prototype for `FetchContext`. +5. `serde_path_to_error` for selected provider/config diagnostics. +6. `parking_lot` and `itertools` where the diff clearly pays for them. +7. Tier 2 only after a concrete problem demonstrates the need. + +`serde_with` may be folded into one of the above only when the same PR has a +clear serialization target. + +## Review questions + +Reviewers should challenge each candidate with the same questions: + +1. Which current code is deleted or centralized by this crate? +2. How many call sites or synchronization points disappear? +3. Does the dependency become part of a public/wire/persistence contract? +4. What new failure mode, compile-time cost, runtime cost, or supply-chain + surface does it add? +5. Could a small local helper or derive already in the graph solve the same + problem more simply? +6. Is the migration independently reversible? +7. What test proves behavior stayed identical? + +## Consequences + +- Dependency additions become evidence-driven rather than preference-driven. +- The highest-value work focuses on duplicated sources of truth first: + `ProviderId` metadata and the Rust/TypeScript bridge contract. +- Large architectural migrations are split from low-risk boilerplate removal. +- Reviewers can reject individual candidates without blocking the rest of the + strategy. +- This ADR is intentionally `Proposed`; accepting it does not automatically + authorize every Tier 1 or Tier 2 dependency. Each implementation PR still + needs its own measured justification and tests. diff --git a/docs/release/ci-cd.md b/docs/release/ci-cd.md index bdb881f99a..d806bc9ffd 100644 --- a/docs/release/ci-cd.md +++ b/docs/release/ci-cd.md @@ -46,8 +46,9 @@ restricted `GH_TOKEN` context. branches, PRs, non-semver tags, non-canonical remotes, tag/SHA mismatch, and commits whose tag is not reachable from protected `origin/main`. 2. The build job provisions/asserts Node 24.x via the `OpenJS.NodeJS.LTS` - winget package, pnpm 11.24.0, the Windows MSVC Rust target, Git, and Inno - Setup 6. It validates every committed project version against the tag. + winget package, the exact pnpm version pinned by + `apps/desktop-tauri/package.json`, the Windows MSVC Rust target, Git, and + Inno Setup 6. It validates every committed project version against the tag. 3. The build invokes `scripts/release-doctor.ps1 -SkipGitHub`, then `scripts/windows-release-build.ps1 -Ref -SmokeInstall` with a fresh temporary `WorkRoot`. It never receives `GH_TOKEN` and never uploads. diff --git a/scripts/install-release-prerequisites.ps1 b/scripts/install-release-prerequisites.ps1 index 3c9ecf3429..9f3ce01c00 100644 --- a/scripts/install-release-prerequisites.ps1 +++ b/scripts/install-release-prerequisites.ps1 @@ -471,8 +471,8 @@ if (-not (Test-Path -LiteralPath $packageJsonPath -PathType Leaf)) { } $packageJson = Get-Content -Raw -LiteralPath $packageJsonPath | ConvertFrom-Json $expectedPnpm = [string]$packageJson.packageManager -replace '^pnpm@', '' -if ($expectedPnpm -notmatch '^11\.24\.0$') { - throw "Unexpected packageManager '$($packageJson.packageManager)'; release pipeline pins pnpm 11.24.0." +if ($packageJson.packageManager -notmatch '^pnpm@\d+\.\d+\.\d+$') { + throw "Unexpected packageManager '$($packageJson.packageManager)'; expected an exact pnpm semver pin." } Require-PrerequisiteCommand 'git' 'Git.Git' 'git' | Out-Null @@ -586,4 +586,4 @@ if ($innoVersion -notmatch '^6\.') { Write-Host "[ok] Inno Setup $innoVersion ($iscc)" Write-Host '' -Write-Host "Release prerequisites passed (Git, Node $requiredNodeMajor, pnpm 11.24.0, Rust MSVC target, Inno Setup 6)." +Write-Host "Release prerequisites passed (Git, Node $requiredNodeMajor, pnpm $expectedPnpm, Rust MSVC target, Inno Setup 6)." diff --git a/scripts/release-pipeline.tests.ps1 b/scripts/release-pipeline.tests.ps1 index 3088c82470..89e4381775 100644 --- a/scripts/release-pipeline.tests.ps1 +++ b/scripts/release-pipeline.tests.ps1 @@ -34,7 +34,12 @@ Assert-Throws { Assert-NodeMajor 'v23.11.0' 24 } 'non-24 Node major rejected by $prerequisiteText = Get-Content -Raw -LiteralPath (Join-Path $scriptRoot 'install-release-prerequisites.ps1') Assert-True ($prerequisiteText -match '\$requiredNodeMajor\s*=\s*24') 'release prerequisite pins Node major 24' -Assert-True ($prerequisiteText -match '11\\.24\\.0') 'release prerequisite keeps pnpm 11.24.0 pinned' +$packageJson = Get-Content -Raw -LiteralPath (Join-Path $scriptRoot '..\apps\desktop-tauri\package.json') | ConvertFrom-Json +$expectedPnpm = [string]$packageJson.packageManager -replace '^pnpm@', '' +Assert-True ($packageJson.packageManager -match '^pnpm@\d+\.\d+\.\d+$') 'package metadata pins an exact pnpm semver' +Assert-True ($prerequisiteText -match '\$expectedPnpm\s*=') 'release prerequisite derives pnpm from package metadata' +Assert-True ($prerequisiteText -match 'pnpm@\$expectedPnpm') 'release prerequisite activates the derived pnpm version' +Assert-True ($prerequisiteText -notmatch [regex]::Escape("pnpm $expectedPnpm,")) 'release prerequisite does not duplicate the pnpm version in status text' Assert-Equal (Normalize-GitHubRepository 'https://github.com/nesszer/Win-CodexBar.git') 'nesszer/win-codexbar' 'HTTPS canonical URL' Assert-Equal (Normalize-GitHubRepository 'git@github.com:nesszer/Win-CodexBar.git') 'nesszer/win-codexbar' 'SSH canonical URL'