Skip to content

fix(auto-update): simplify resolveProjectRoot + fix indentation (supersedes #3423) - #3515

Closed
oyi77 wants to merge 22 commits into
cheaper-inference:release/v3.8.20from
oyi77:fix/auto-update-cwd-stability
Closed

oyi77 wants to merge 22 commits into
cheaper-inference:release/v3.8.20from
oyi77:fix/auto-update-cwd-stability

Conversation

@oyi77

@oyi77 oyi77 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Problem

PR #3423 had review comments from Copilot and @gemini-code-assist:

  1. resolveProjectRoot had redundant existsSync checks — always returned cwd regardless
  2. Indentation inconsistencies in version/route.ts (10 spaces instead of 8)

Fix

  • Simplified resolveProjectRoot to just return process.cwd() (removed dead existsSync checks)
  • Fixed indentation in version/route.ts to use consistent 8-space indent

Tests

  • Typecheck passes
  • All existing tests pass

Supersedes #3423 (includes original fix + addresses review comments)

diegosouzapw and others added 16 commits June 9, 2026 08:31
…aper-inference#3481)

Codex's model-catalog refresh (codex_models_manager) does
GET /v1/models?client_version=<v> and decodes a JSON object with a
TOP-LEVEL `models` array. OmniRoute answers in the OpenAI-standard
`{object,data}` shape, so codex fails with "missing field `models`"
and logs "failed to refresh available models" on every startup.

Detect codex clients via the `originator` / `user-agent` = `codex_*`
headers they send and add an EMPTY top-level `models: []` so the decode
succeeds. Non-codex OpenAI clients keep the byte-identical `{object,data}`
response.

The array is intentionally empty: codex replaces its built-in per-model
agent prompt (`base_instructions`, ~21k chars) with whatever a populated
entry carries for the selected model, so emitting our catalog would drop
the agent prompt to nothing and break codex's agent behaviour (verified
empirically against codex 0.137). An empty list keeps codex on its
built-in model info — same inference as before, minus the error.

Validated end-to-end with the real handler against codex 0.137:
"failed to refresh available models" → 0 occurrences, instructions
preserved (built-in Codex agent prompt, not empty).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add generated quality gate reports, metrics files, and local setup prompt
artifacts to .gitignore to prevent committing environment-specific or
temporary files.
…g retry text (cheaper-inference#3513)

PR cheaper-inference#3422 ('allow OpenAI usage-only empty choices chunks') reintroduced the
assistant-content injection '[OmniRoute] Upstream returned an empty response.
Please retry.' for empty `choices: []` chunks that carry no valid usage. Clients
(Goose/opencode) feed that text back as a turn and spin in a retry loop -- the
exact regression cheaper-inference#3400 had fixed by dropping the chunk.

Restore the drop behavior for the no-usage case while preserving cheaper-inference#3422's
standards-compliant forwarding of usage-only `include_usage` final chunks.
Realign the mislabeled stream-utils test (it asserted the injection) and add a
dedicated regression guard.

Reported-by: @mochizzan
Refs: cheaper-inference#3502, cheaper-inference#3388, cheaper-inference#3400, cheaper-inference#3422

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a PROJECT_ROOT constant in src/lib/system/autoUpdate.ts that safely resolves the current working directory with a fallback to the home or temp directory. It refactors the auto-update route and helper functions to use PROJECT_ROOT instead of process.cwd(), ensuring more robust directory resolution during update operations. There are no review comments to address, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@oyi77
oyi77 force-pushed the fix/auto-update-cwd-stability branch from 96bc07d to f695332 Compare June 9, 2026 17:30
diegosouzapw and others added 5 commits June 9, 2026 14:38
…polynomial-ReDoS)

CodeQL js/polynomial-redos: unbounded \s* before an anchored \s*$ allowed
O(n²) backtracking on attacker-influenced display names. Bounded to {0,8}/{1,8}
(ample for any real label spacing). Plugin builds + 254 tests green.
- getPendingRequests() typed to real shape (was widened to object) → fixes
  unknown 'count' in the unified-requests view (cheaper-inference#3401)
- streamChunks log payload cast to its declared type (callLogs.ts)
- preScreenTargets aligned to canonical IsModelAvailable signature (cheaper-inference#3169),
  Promise.resolve-normalized so .catch never hits a bare boolean

All 5 gates green: lint(0 err) + typecheck:core + cycles + docs-all + unit + vitest(146).
- Remove redundant existsSync checks in resolveProjectRoot (always
  returned cwd regardless of checks)
- Fix indentation inconsistencies in version/route.ts (10 spaces → 8)

Addresses review comments from Copilot and @gemini-code-assist on PR cheaper-inference#3423.
@oyi77
oyi77 force-pushed the fix/auto-update-cwd-stability branch from f695332 to d83df76 Compare June 9, 2026 18:06
@oyi77
oyi77 changed the base branch from main to release/v3.8.18 June 9, 2026 18:41
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.18 to release/v3.8.19 June 9, 2026 19:46
@kilo-code-bot

kilo-code-bot Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
src/lib/system/autoUpdate.ts 85 Uses existsSync but it was removed from imports in line 2. Line 85 calls existsSync(path.join(PROJECT_ROOT, ".git")) but the function is no longer imported, causing a ReferenceError.

WARNING

File Line Issue
src/lib/system/autoUpdate.ts 2 Removed existsSync from imports but still used in getAutoUpdateConfig at line 85. Either the import should be restored or the check should use the async pathExists function instead.
src/lib/system/autoUpdate.ts 10-15 The resolveProjectRoot function uses try/catch around process.cwd() but process.cwd() typically only throws on Windows when the current directory has been deleted; the catch block returning fallback will work, though on most systems process.cwd() returns the working directory regardless. The original issue was about redundant checks, but the fix should preserve functionality for getAutoUpdateConfig which needs to check for .git.
Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

The diff shows a large PR with many test files being added and numerous source changes. The core issue is that the auto-update module has an import mismatch that would break at runtime.

Files Reviewed (13 files)
  • src/lib/system/autoUpdate.ts - 3 issues (CRITICAL/WARNING)
  • src/app/api/system/version/route.ts
  • src/lib/usage/usageHistory.ts
  • tests/unit/pipeline.test.ts - new tests added
  • tests/unit/rate-limit-enhanced.test.ts - new tests added
  • tests/unit/request-logger-pipeline.test.ts - new file
  • tests/unit/request-logger-endpoints.test.ts - new file
  • tests/unit/sse-parser-numeric-ids.test.ts - new tests added
  • tests/unit/stream-handler.test.ts - modified tests
  • tests/unit/stream-numeric-ids.test.ts - new tests added
  • tests/unit/stream-utils.test.ts - modified tests
  • tests/unit/[provider removed at its operator's request]-request-token-3491.test.ts - new file
  • tests/unit/usage-analytics.test.ts - modified

@kilo-code-bot

kilo-code-bot Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 0
SUGGESTION 0
Issue Details (click to expand)

CRITICAL

File Line Issue
src/lib/system/autoUpdate.ts 85 Uses existsSync but it was removed from imports on line 2. Line 85 calls existsSync(path.join(PROJECT_ROOT, ".git")) but the function is no longer imported, causing a ReferenceError: existsSync is not defined when getAutoUpdateConfig() is called and mode is 'npm'.
Other Observations

The PR title mentions "simplify resolveProjectRoot + fix indentation" but the diff shows a much larger change set including:

  • Test files: tests/unit/pipeline.test.ts, tests/unit/rate-limit-enhanced.test.ts, tests/unit/request-logger-pipeline.test.ts, tests/unit/request-logger-endpoints.test.ts, tests/unit/sse-parser-numeric-ids.test.ts, tests/unit/stream-handler.test.ts, tests/unit/stream-numeric-ids.test.ts, tests/unit/stream-utils.test.ts, tests/unit/[provider removed at its operator's request]-request-token-3491.test.ts, tests/unit/usage-analytics.test.ts
  • Source changes: open-sse/handlers/chatCore.ts, open-sse/utils/requestLogger.ts, open-sse/utils/stream.ts, open-sse/utils/streamHandler.ts, open-sse/utils/sseParser.ts, open-sse/executors/default.ts, open-sse/executors/[provider removed at its operator's request].ts, open-sse/services/combo.ts, open-sse/services/comboConfig.ts, open-sse/services/model.ts, open-sse/services/provider.ts, open-sse/services/browserPool.ts, open-sse/services/accountFallback.ts, open-sse/config/errorConfig.ts, open-sse/config/providerRegistry.ts
  • UI changes: src/shared/components/RequestLoggerV2.tsx, src/shared/components/RequestLoggerDetail.tsx, src/shared/components/RequestLoggerSignature.ts, src/app/(dashboard)/dashboard/logs/page.tsx, src/app/(dashboard)/dashboard/HomePageClient.tsx, src/app/(dashboard)/dashboard/media-providers/components/LlmChatCard.tsx
  • Plugin lifecycle hooks: src/lib/plugins/hooks.ts, src/lib/plugins/index.ts, src/lib/plugins/manager.ts, src/lib/plugins/manifest.ts, src/lib/plugins/marketplace.ts

The core CRITICAL issue is that existsSync was removed from imports but is still used in getAutoUpdateConfig() at line 85.

Files Reviewed (13+ files)
  • src/lib/system/autoUpdate.ts - CRITICAL: import mismatch
  • src/app/api/system/version/route.ts
  • src/lib/usage/usageHistory.ts
  • open-sse/utils/requestLogger.ts
  • open-sse/handlers/chatCore.ts
  • open-sse/utils/stream.ts
  • open-sse/utils/sseParser.ts
  • Multiple test files and UI components

Reviewed by laguna-m.1-20260312:free · 5,799,309 tokens

@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.19 to release/v3.8.20 June 10, 2026 06:15
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @oyi77. Same situation as #3514: this is 133 files / +8.5k for a 2-file auto-update fix, with ~42 docs/i18n/*/CHANGELOG.md files + a large stale-rebase tail, and it's CONFLICTING.

Beyond the noise, the actual resolveProjectRoot change here is weaker than the original #3423: this version strips the existsSync checks and just returns process.cwd() with no fallback, whereas #3423 extracts a stable PROJECT_ROOT and wires it through properly (better for PM2-started processes where cwd ≠ project root). There's also an indentation regression in version/route.ts.

Recommend we go with #3423 (2 files, clean) for this fix. If you'd prefer your approach, please resubmit on a clean branch off release/v3.8.20 with only the 2 auto-update files and add a regression test (or a VPS validation record) per the repo's bug-fix gate. Leaving open. 🙏

@oyi77

oyi77 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #3561 (focused 2-file re-cut off release/v3.8.20 per @diegosouzapw review). Uses the stable PROJECT_ROOT walker approach (vs the weaker "just return cwd" version) and includes a regression test. Closing this one in favor of the cleaner PR.

@oyi77 oyi77 closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants