From c2d90229ca195ad3c28eecc44752452970be823a Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 11 Jun 2026 02:40:33 -0300 Subject: [PATCH] fix(review): resolve findings from /review-reviews battery (v3.8.21 hardening) (#3618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-release hardening from the /review-reviews battery — 15 findings resolved (L1-L13,L15) + L14 live-verified WONTFIX, convergence re-review clean. lint/typecheck:core/test:vitest(146)/build green; zero new test:unit failures vs baseline 797de433f. --- .npmignore | 21 ++++ CHANGELOG.md | 3 + open-sse/config/antigravityModelAliases.ts | 37 ++++++ open-sse/executors/antigravity.ts | 6 +- open-sse/handlers/chatCore.ts | 12 +- open-sse/handlers/chatCore/idempotency.ts | 45 ++++--- open-sse/handlers/chatCore/semanticCache.ts | 2 +- open-sse/services/usage.ts | 25 +--- .../translator/response/gemini-to-openai.ts | 11 +- package.json | 9 +- .../v1/completions/textCompletionTransform.ts | 7 +- src/lib/usage/providerLimits.ts | 32 ++++- src/lib/usage/usageHistory.ts | 1 - ...igravity-local-usage-fallback-3821.test.ts | 119 ++++++++++++++++++ tests/unit/antigravity-model-aliases.test.ts | 30 ++++- .../chatcore-extracted-modules-3821.test.ts | 110 ++++++++++++++++ .../unit/completions-text-format-3571.test.ts | 50 ++++++++ tests/unit/executor-agy.test.ts | 35 ++++++ .../mcp-published-files-closure-3578.test.ts | 53 ++++++++ ...rovider-limits-sanitize-scope-3821.test.ts | 107 ++++++++++++++++ tests/unit/response-sanitizer.test.ts | 22 ++++ .../translator-resp-gemini-to-openai.test.ts | 109 ++++++++++++++++ .../unit/upstream-ca-test-route-3488.test.ts | 36 ++++-- 23 files changed, 815 insertions(+), 67 deletions(-) create mode 100644 tests/unit/antigravity-local-usage-fallback-3821.test.ts create mode 100644 tests/unit/chatcore-extracted-modules-3821.test.ts create mode 100644 tests/unit/provider-limits-sanitize-scope-3821.test.ts diff --git a/.npmignore b/.npmignore index 3cd4ba971a..8e4fd8d8e0 100644 --- a/.npmignore +++ b/.npmignore @@ -9,6 +9,16 @@ app/vscode-extension/ **/db.json # Source code (pre-built app/ is published instead) +# +# NOTE (#3578 / #3821-review): package.json "files" is the source of truth for what +# ships. It now allowlists the backend source closure the MCP server needs at runtime +# (open-sse/, src/lib, src/server, ...) and OVERRIDES the broad src/ + open-sse/ excludes +# below — npm honors files[] over .npmignore for inclusion. These lines are kept only as +# intent/back-stop: if files[] is ever trimmed back to specific paths, they must NOT be +# allowed to re-hide the MCP closure (that would silently reintroduce the --mcp +# ERR_MODULE_NOT_FOUND #3578 fixed). The closure gate in +# tests/unit/mcp-published-files-closure-3578.test.ts asserts the real `npm pack` output +# in both directions (closure present + zero test files), catching such a regression. src/ open-sse/ docs/ @@ -18,6 +28,17 @@ images/ logs/ scripts/ +# Co-located tests must never ship even when their parent dir is allowlisted by files[]. +# (Primary guard is the "!**/*.test.*" negations in package.json files[]; this is defense +# in depth for any nested dir the allowlist pulls in.) +**/__tests__/ +**/*.test.ts +**/*.test.tsx +**/*.test.js +**/*.test.mjs +**/*.spec.ts +**/*.spec.tsx + # Config/dev files *.md !README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index a1d7cc206d..30fba81c29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - **refactor(chatCore):** extract the chatCore request phases — idempotency check, semantic cache check, common request sanitization, and memory/skills injection — into dedicated `open-sse/handlers/chatCore/` modules (`idempotency.ts`, `semanticCache.ts`, `sanitization.ts`, `memorySkillsInjection.ts`), slimming the monolithic handler with no behavior change. (Maintainer follow-up: re-derive `idempotencyKey` at the Phase 9.2 save site after the check moved into the module, fixing a `ReferenceError` on successful non-cached responses.) ([#3598](https://github.com/diegosouzapw/OmniRoute/pull/3598) — thanks @oyi77) - **docs(opencode-provider):** soft-deprecate `@omniroute/opencode-provider` in favour of `@omniroute/opencode-plugin`. The provider package writes a **static** model list to `opencode.json` that drifts behind the live OmniRoute catalog, whereas the plugin fetches `/v1/models` at OpenCode startup. The package keeps working (no code/behavior change), but its npm description and README now carry a deprecation banner with the one-line migration, and a guard test pins the notice. ([#3419](https://github.com/diegosouzapw/OmniRoute/issues/3419) — thanks @herjarsa) +- **chore(review):** pre-release hardening from a multi-reviewer `/review-reviews` battery over the v3.8.21 diff (7 Opus reviewers; zero blocker/high). Resolved findings: npm tarball no longer ships co-located test files (`files[]` negations + reconciled `.npmignore`; the #3578 closure gate now asserts the real `npm pack` output in both directions); `getSanitizedCachedProviderLimitsMap` scopes its connection scan to antigravity/agy instead of decrypting every active connection on each dashboard poll; the Antigravity quota-tier remap (`toClientAntigravityQuotaModelId`) is centralized in `antigravityModelAliases.ts` (was an inline if-ladder in `usage.ts`); the chatCore idempotency check returns its resolved key so the save site reuses a single derivation; and new tests pin the chatCore extracted modules, the Antigravity `usage_history` fallback contract, the reasoning-wrapper prefix-preservation heuristic, the Antigravity SSE `markdown` branch, and the upstream-ca/test no-persist guarantee. (Live-verified that agy consumer tokens are accepted by the non-daily `cloudcode-pa` host used by `retrieveUserQuota`, so #3604 is not agy-host-limited.) ### 🔧 Bug Fixes @@ -28,6 +29,8 @@ - **fix(antigravity):** the Antigravity/agy Gemini 3.5 Flash catalog now exposes clean public tier IDs (`gemini-3.5-flash-low`/`-medium`/`-high`, matching Antigravity 2.0.4's Low/Medium/High selector) and maps them to the live upstream IDs at the executor boundary, instead of the old confusing `-preview`/`-agent` names. Antigravity model-id normalization moved out of the global model resolver into the executor so client-visible IDs are no longer rewritten before account/credential routing and logging. (Maintainer follow-up: kept `gemini-3.5-flash-preview` as a hidden backward-compat alias routing to the High tier so saved combos/configs keep working; live-validated the tier set via the `agy` CLI catalog.) ([#3603](https://github.com/diegosouzapw/OmniRoute/pull/3603) — thanks @dhaern) - **fix(usage):** Antigravity/agy Provider Limits now report accurate consumption — `retrieveUserQuota` (live usage) is preferred over the `fetchAvailableModels` catalog view (which keeps reporting full buckets after real usage), with a local `usage_history` fallback for buckets that are only catalog-visible; cached entries are sanitized so retired upstream IDs are not re-exposed, and a deduplicated post-usage refresh keeps the dashboard fresh after each request. (Maintainer follow-up: the post-usage refresh is decoupled through a lightweight `usageEvents` bus so `usageHistory` no longer imports `providerLimits`/the executors graph, keeping the `typecheck:core` surface stable.) ([#3604](https://github.com/diegosouzapw/OmniRoute/pull/3604) — thanks @dhaern) - **fix(gemini):** textual reasoning wrappers emitted as assistant prose (``/``/``/``, including malformed/open tags like `