feat(ai): AI subsystem overhaul, phases 0-10 (vault, v4 config, provider plugins, catalog, core, tasks, local runtimes, sessions, surface adoption, config CLI) - #298
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🐉 eve review — 🔴 REQUEST_CHANGES · 8 findings
Previous runs (10)
|
Contributor
|
Important Review skippedToo many files! This PR contains 324 files, which is 174 over the limit of 150. To get a review, narrow the scope: Upgrade to Pro+ to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (329)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![eve-bot-lovinka[bot]](https://avatars.githubusercontent.com/in/4092731?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
b8e1c6d· 12 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 4 |
| 🟡 Medium | 5 |
| 🔵 Low | 3 |
|
Review completed and posted.
|
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🟡 Review comments
21f93aa· 4 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟡 Medium | 2 |
| 🔵 Low | 2 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
f702a96· 6 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 5 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
2dd1d2f· 9 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 6 |
| 🔵 Low | 1 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
6df7725· 9 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 3 |
| 🔵 Low | 4 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
07625af· 8 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 4 |
| 🔵 Low | 2 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
9473a60· 7 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 3 |
| 🟡 Medium | 2 |
| 🔵 Low | 2 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
e617a84· 9 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 5 |
| 🔵 Low | 2 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
7ecef5c· 6 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 2 |
| 🔵 Low | 3 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
ff0ccc0· 10 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 5 |
| 🔵 Low | 3 |
|
Delta review completed and posted.
|
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
8916e50· 9 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 4 |
| 🟡 Medium | 2 |
| 🔵 Low | 3 |
|
Delta review completed and posted.
|
genesiscz
force-pushed
the
feat/fixes-2026-07-27
branch
from
July 29, 2026 04:38
1a71acf to
6b6cb1e
Compare
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
0a80f2e· 10 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 4 |
| 🔵 Low | 5 |
…tag pinning - 45edd73 fix(security): rotation key escrow, generate-path rung fallback, GCM tag pinning
…ELS; real long-context rates - e5b408e feat(ask): catalog-backed model listing, drop stale KNOWN_MODELS - 1c0f9bd feat(ai-proxy): catalog-backed static fallbacks - 897b70b refactor(indexer): rename clashing MODEL_REGISTRY export - 83f3c75 fix(ai-catalog): price Sonnet 4.5 long context at its real >200k rate
…gle callLLM - 4ec1c83 feat(ai-core): ModelRef grammar - 0b8d1a8 feat(ai-catalog): record tool support so pickers can filter on it again - 9fa3c83 feat(ai-core): resolveModel ladder - 80c775e chore(ai-core): drop unused ModelRequest.explicit flag - 83d2cbe feat(ai-catalog): current-generation openai, google and groq entries - 8c8c9dd feat(ai-core): composable auth fetch - 1f416b0 refactor(ai-core): single callLLM implementation; ChatEngine delegates - 3823874 style(ai-core): biome line wrap in streamLLM signature
…b tags, banded pricing rules - aea9592 fix(security): rebind vault store when home moves, block real keychain under tests - 9997279 feat(ai-core): per-request job tags ride the gateway binding - aff43af style: biome reflow residue from staged-only hook passes - 0b5fc3f feat(security): triple-layer lockdown keeping tests off the real OS keychain - 6b47187 feat(ai-catalog): time-windowed and context-banded pricing rules - 349c7e9 feat(security): dual-signal test detection and non-interactive keychain write barrier
- 70b36e4 feat(ai-config): doctor + account ops library - 6fb2760 feat(ai): tools ai config account/default/link commands - 3e95539 fix(security): refuse master-key rotation while the env rung supplies the key - cb07d40 feat(ai): tools ai config secret commands - ed959ec style(ai-config): biome import order and wrapping - a8dab42 feat(ai): config doctor + interactive TUI
…adapters - 5d34225 refactor(ai-local): model descriptors extracted from ModelRegistry blob - 8722d21 feat(ai-local): unified artifact store over HF hub + url sources - 00df39a refactor(ai-local): runtime folders (transformers-js/coreml/sherpa) - 34686ac feat(ai-local): plugin adapters over descriptor/artifact/runtime tree
…e, ask and proxy adopt - 5a10e93 feat(ai-session): session store + json backend (ask format) - 244fb64 feat(ai-session): sqlite backend - c933d33 feat(ai-session): mini-agent tool loop with interject - 670afd3 refactor(youtube): ask sessions ride shared session store - 3253070 refactor(ai-session): ask + proxy sessions on shared store
…ngle TTS path, one summarize - 368b916 feat(ai-tasks): unified ai.* facade (chat/embed/translate/summarize/image) - 494b02b feat(ai-tasks): transcription through provider bindings - 8c6b623 feat(ai-providers): huggingface plugin so hf-cloud accounts have a home - 5e517d5 feat(ai-tasks): single TTS path (say/youtube/xai unified) - 300290e refactor(ai-tasks): one summarize path; realtime stub - 4708aa1 fix(ai-tasks): --provider without --model resolves a real model ref
…legacy cloud task dropped - 4d07fc0 fix(ai-tasks): facade owns the binding lifetime, and TranscriptionManager stops being a second ASR path - 007ff32 refactor(ai-tasks): Summarizer and Translator run the facade's single path - 690a0ab fix(ai-config): v4 migration drops the legacy "cloud" task provider
…gistration, usage events - f6fca7c feat(ai-proxy): bill AI accounts by @account ref, with credential hot-reload - 1a05e34 feat(ai-proxy): client keys as vault references instead of plaintext - ced78b7 feat(ai-proxy): register the proxy as an AI-config account so @Proxy refs resolve - 9c9e233 feat(ai-proxy): emit usage events from the ledger booking site - d2925d9 fix(ai-proxy-client): resolve the proxy config under GENESIS_TOOLS_HOME - 9a43dbf fix(ai-config): AiConfigStore.mutate awaits its callback
…n over the unified stack - 6689146 refactor(ai): shared usage-token helpers and one call-cost implementation - 98f09da refactor(ask): provider detection over unified config/plugins/catalog - bd2684a feat(ai-core): chooseProviderModel; youtube stops importing ask internals - a4a9328 test(ai): pin the scanned-all guard and grandfathered env resolution; drop dead ASR picker
…on; cache-rate fixes - 9513118 feat(ai-usage): unified usage events (record/query) - 0542892 feat(claude,dev-dashboard): usage through unified layer - 7109076 fix(ai-usage): sandbox the claude db path, wire the proxy sink, record cost provenance - d96cdbf fix(ai-usage): price cached input at cache rates, not twice at full rate - 5287801 docs(ai-usage): costSource is a top-level field, not a meta key
…ricing, AI docs rewrite - 2d51c03 refactor(eve): env facade for the agent, one process.env reference - e31d73b chore(ai): delete the ai/device re-export shim - 77e7872 refactor(claude): summarize prices via catalog, not ask's pricing shim - f39141c docs(ai): rewrite the AI section for the new layers, add the two recipes - 97adf9f fix(ai-core): callLLM releases a binding it resolved itself
… fixes - fb79069 fix(ai,security): PR #298 review round — logout that logs out, and five ladder fixes - 3ccec3d style(claude): import order in the summarize engine - 92cc506 refactor(ai): reuse slugify and the one TASK_CAPABILITY map - 8bdcf37 perf(ai,ask,ai-proxy): fix ask usage attribution and three hot-path costs - dc10257 fix(ai-proxy,ai,security): address PR #298 review on cache identity, vault bytes, HF cache listing - 71fdca2 fix(ai): address PR #298 review on config validation, load races and backup coverage - 5a23600 fix(ai): address PR #298 review on the migration chain and local plugin bindings - 8c28d00 fix(ai): address PR #298 review on bind transport, repair hints and the credentials guard - 9215204 fix(ai): address PR #298 review on refresh-once, cache paths, pricing rules and send concurrency - 4c60124 fix(ai,ci): address PR #298 review on catalog layering, derived lists, the preload and the pre-commit hook - 2c5ace0 fix(claude): address PR #298 review by enforcing the token journal's byte cap - 458265e fix(ask,ai-proxy): address PR #298 review on model switching and fingerprint coverage - 233c070 fix(ai,security): address PR #298 review on grok-sub binding, escrow recovery and the fingerprint stat - a0f49c8 refactor(ai): address PR #298 review by naming the converter after what it returns - 10491f1 test(ask): address PR #298 review by pinning switchModel's attribution fields - 4bf1e3e docs(plugin): record the reviewer bot logins and the thread-close protocol - ffa07c0 fix(claude,ai-proxy): address PR #298 review on the teammate binary fallback and the client-secret mint - 4f3aea3 docs(plugin): remove the contradiction between the resolve rule and its exception - 5b6b976 fix(claude,plugin): address PR #298 review on test env restore, the fallback seam and the missing step - e725f04 fix(plugin,docs): per-line biome ignores now that the plugin-scripts override is gone
…xcludes move into the runner
genesiscz
force-pushed
the
feat/ai-overhaul
branch
from
July 30, 2026 01:06
f12e168 to
5758fe8
Compare
There was a problem hiding this comment.
🐉 eve review — 🔴 Changes requested
5758fe8· 8 actionable findings · view run ↗
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 3 |
| 🔵 Low | 4 |
Not line-anchorable (review without the full diff context)
- 🟠 High
src/ask/providers/ProviderManager.ts:65· confidence 97/100 — Provider cache bypasses config and credential refreshes: After one full detection pass, this singleton returns cached bound providers forever without even loadingAiConfigStore. Consequently account deletion/disablement, default changes, endpoint edits, and vault credential rotation are invisible to long-running ask/youtube consumers—the exact stale-credential problemAiConfigStorewas introduced to solve. The new test even pins this behavior by changing the config root and expecting the cached answer. Cache catalog data if needed, but invalidate/rebind providers from a config/vault generation stamp (as ai-proxy's binding fingerprint does). - 🟡 Medium
src/ask/providers/ProviderManager.ts:64· confidence 99/100 — Implementation is unrelated to the supplied migration plan: The supplied.claude/plans/2026-01-31-clack-prompts-migration.mdspecifies a gradual@inquirer/promptsto@clack/promptsmigration, shared prompt/color utilities, and tool-by-tool CLI adoption. This hunk instead replaces provider discovery, account selection, credential binding, and model catalog behavior as part of a repository-wide AI overhaul. That matches the PR title but is substantive scope beyond the supplied plan; the plan/spec association should be corrected or replaced so reviewers and future implementers do not treat this AI architecture as execution of the Clack migration plan.
|
|
||
| const store = await AiConfigStore.load(); | ||
| const endpoint = gatewayEndpoint(config); | ||
| const existing = store.account(GATEWAY_ACCOUNT_ID); |
There was a problem hiding this comment.
🧹 Quality | 🟡 Medium · confidence 99/100
Gateway linking can overwrite a concurrent account edit
existing is captured before acquiring the config lock, then its editable fields are copied into the entry written inside withLock. Although withLock rereads fresh config, this callback ignores the fresh account in data; a concurrent edit to name, label, enabled, tags, apps, billing, or credentials between lines 95 and 103 is overwritten with the stale snapshot. Resolve the existing gateway account from data.accounts inside the locked callback and derive the replacement from that value. Add a lock-interleaving test to preserve concurrent user edits.
🧩 Analysis
Grep evidence: const existing = store\.account\(GATEWAY_ACCOUNT_ID\)
| /** Re-read when another process has written since we loaded. */ | ||
| private async refreshIfStale(): Promise<void> { | ||
| const current = AiConfigStore.stampOf(this.storage); | ||
| if (current.mtimeMs === this.stamp.mtimeMs && current.size === this.stamp.size) { |
There was a problem hiding this comment.
🧹 Quality | 🟡 Medium · confidence 99/100
Equal-size rewrites can still leave the config stale
The refresh stamp compares only mtimeMs and file size. A same-length replacement whose timestamp is preserved (the surrounding comment explicitly names rsync -t, restores, and atomic replacements) passes this check even when its contents changed, so a long-running process can retain stale account settings or credential references indefinitely. Size only fixes differing-size writes; it does not make the stated preserved-timestamp case safe. Include a content digest or another replacement-sensitive identity in the stamp, and add a test that rewrites different same-length JSON while preserving mtime.
🧩 Analysis
Grep evidence: current\.mtimeMs === this\.stamp\.mtimeMs && current\.size === this\.stamp\.size
| const blob = await exportVault(passphrase); | ||
| const target = resolve(flags.out); | ||
|
|
||
| writeFileSync(target, blob, { mode: 0o600 }); |
There was a problem hiding this comment.
🧹 Quality | 🔵 Low · confidence 93/100
Vault export bypasses the repository write primitive
The new command uses synchronous Node writeFileSync/chmodSync for an output file. Project review memory requires Bun file APIs for writes and explicit output-mode behavior; this also blocks the CLI event loop during export and performs a second non-atomic permission operation. Use the repository/Bun owner-only write path so permissions are correct at creation and errors retain operational context.
🧩 Analysis
Grep evidence: writeFileSync\(target, blob, \{ mode: 0o600 \}\)
| @@ -0,0 +1,73 @@ | |||
| /** | |||
There was a problem hiding this comment.
🧪 Tests | 🔵 Low · confidence 65/100
No test changes accompany 73 added lines in apps/eve/agent/lib/env.ts
This PR adds 73 lines to apps/eve/agent/lib/env.ts with no touching test change (no changed test names env and none under apps/eve/agent/lib/). If the change alters behavior, add or extend a test that pins it (deterministic static check — ignore if the change is genuinely untestable or covered elsewhere).
🧩 Analysis
Grep evidence: env
| @@ -0,0 +1,32 @@ | |||
| import { loadConfigFresh } from "@app/ai-proxy/lib/config"; | |||
There was a problem hiding this comment.
🧪 Tests | 🔵 Low · confidence 65/100
No test changes accompany 32 added lines in src/ai-proxy/commands/link.ts
This PR adds 32 lines to src/ai-proxy/commands/link.ts with no touching test change (no changed test names link and none under src/ai-proxy/commands/). If the change alters behavior, add or extend a test that pins it (deterministic static check — ignore if the change is genuinely untestable or covered elsewhere).
🧩 Analysis
Grep evidence: link
| @@ -1,7 +1,8 @@ | |||
| import * as p from "@clack/prompts"; | |||
| import { AIConfig } from "@genesiscz/utils/ai/AIConfig"; | |||
| import { type ClearableCredential, clearCredentials } from "@genesiscz/utils/ai/config/account-ops"; | |||
There was a problem hiding this comment.
🧪 Tests | 🔵 Low · confidence 65/100
No test changes accompany 24 added lines in src/claude/commands/logout.ts
This PR adds 24 lines to src/claude/commands/logout.ts with no touching test change (no changed test names logout and none under src/claude/commands/). If the change alters behavior, add or extend a test that pins it (deterministic static check — ignore if the change is genuinely untestable or covered elsewhere).
🧩 Analysis
Grep evidence: logout
|
Delta review completed and posted.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The AI-subsystem overhaul, phases 0-10. 119 commits, 327 files, +29811/-5025 against
master.What this is
One modular, config-driven AI layer under
src/utils/ai/andsrc/utils/security/, replacing the parallel stacks that had accumulated: 4 LLM call paths, 4 TTS entry points, 2 account stores, 3 pricing tables, a stale hand-kept model catalog, and SDK singletons reading API keys straight from the environment.security/: encrypted secret vault (AES-256-GCM per entry, HKDF-derived per-entry keys, entry path bound as GCM AAD so ciphertexts cannot be swapped between entries), master key resolved via a ladder (env, OS keychain via@napi-rs/keyring, opt-in key file), passphrase-wrapped export/import, rotation that aborts before writing if any entry fails to decrypt.ai/config/: v4 schema (zod) with immutable accountidvs renameablename, first-class@account/<id>refs with a reverse index (referrersOf, external scanners pluggable), derived selectors instead of stored booleans, per-accountuseEnvApiKey.ai/providers/: ~20 provider plugins behind one interface with a singleresolveCredentialchokepoint; argless SDK factories and bare singletons are gone, enforced by a CI guard.ai/catalog/: all-provider model registry plus live discovery overlays (LiteLLM, OpenRouter, per-plugin probes); unified async pricing.ai/core/:ModelRefgrammar (provider/model,@account/<id>:<model>, aliases), one resolution ladder (explicit, app default, task default, global default), a singlecallLLM/streamLLM.ai/tasks/:ai.chat/summarize/translate/embed/transcribe/speak/...facades; TTS entry points collapse from 4 to 1.ai/local/: artifacts/runtimes/adapters restructure of the ONNX, CoreML and sherpa stack, exposed as regular provider plugins.ai/session/:SessionBackend(sqlite and JSONL),SessionStore.turn()with atomic question+answer append,MiniAgentwith interject support.ai/usage/: centralrecordUsage/queryUsage; the ai-proxy client ledger is untouched by design.tools ai config account add|list|show|edit|rm|test,default set,link ls,secret set|rotate|export|import,doctor;rmrefuses whilereferrersOfreports links._schemaVersion: 3sentinel keeps old binaries' migration check quiet; a defaults snapshot old code never touches.Behavior changes
API keys resolve from configured accounts. Ambient env pickup works only through an account's explicit
useEnvApiKey(pickups alive before the migration are grandfathered into accounts, so nothing silently loses capability); a configured account now outranks an env var instead of the reverse. Secrets move from plaintext config fields to the vault as{ type: "secure", path: ... }refs.Verification
origin/masterare fixed in this branch (test preload list, awalkFilesfallback that silently dropped subtrees on transient errors, serialization of load-sensitive test files, and related runner fixes).tsgo --noEmitandbiome check .clean repo-wide.Not in this PR
assertSafeToWriteRealConfig()plus a migration guard that checks both cwd and the code's own location).AIConfigfacade (it projects v4 into the v3 shape and syncs edits back; deleting it and its importers is a follow-up PR).