Clarify modern model catalog counts and variant presets#100
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR makes the installer three-mode (full, modern, legacy) with default "full" that merges modern base-model entries and legacy explicit presets, updates shipped model/template counts and context/output mappings, refactors CLI parsing and atomic config writes, and adds tests and testable exports for the installer. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as install-opencode-codex-auth.js
participant Parser as Flag\ Parser
participant Loader as Template\ Loader
participant Merger as Model\ Merger
participant Writer as Config\ Writer
participant FS as FileSystem
User->>CLI: Run installer (with --modern / --legacy / none)
CLI->>Parser: Parse CLI flags
Parser-->>CLI: Return configMode (modern / legacy / full)
alt Full Mode (default)
CLI->>Loader: Load modern + legacy templates
Loader->>Merger: Provide model entries
Merger->>Merger: Merge provider.openai.models (check collisions)
Merger-->>CLI: Return merged template
else Modern Mode
CLI->>Loader: Load modern template
Loader-->>CLI: Return modern template
else Legacy Mode
CLI->>Loader: Load legacy template
Loader-->>CLI: Return legacy template
end
CLI->>Writer: Write effective opencode.json (atomic)
Writer->>FS: write temp file -> rename (with Windows retry)
Writer-->>CLI: Confirm write and backup created
CLI->>User: Output result and exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/development/CONFIG_FLOW.md (1)
122-127:⚠️ Potential issue | 🟡 MinorUpdate legacy model entry count in CONFIG_FLOW.md from 26 to 34.
Lines 122–127 state the legacy template ships "26 explicit model entries," but the actual config file (
config/opencode-legacy.json) contains 34 model entries. This contradicts other documentation files that correctly reference 34 shipped presets. Update this section to reflect the accurate count.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/CONFIG_FLOW.md` around lines 122 - 127, Update the legacy model entry count text in CONFIG_FLOW.md: replace the phrase "26 explicit model entries" with "34 explicit model entries" so the docs match the actual config/opencode-legacy.json shipped presets; locate the exact string "26 explicit model entries" in the CONFIG_FLOW.md content and change the number to 34.
🧹 Nitpick comments (2)
scripts/install-opencode-codex-auth.js (1)
174-180: Consider validating only the required template for the selected mode.Currently both templates are validated even when only one is needed (e.g.,
--modernonly needsmodernTemplatePath). This is fine for robustness but could be relaxed if you want faster failure for single-mode installs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install-opencode-codex-auth.js` around lines 174 - 180, The main function currently checks both modernTemplatePath and legacyTemplatePath regardless of the selected install mode; change the validation to only check the template required by the chosen mode (e.g., if the install is modern only validate modernTemplatePath; if legacy only validate legacyTemplatePath; if neither or combined validate both). Locate the mode-determining variable or flag used by this script (for example an isModern flag or process.argv/commander option) and branch the existsSync checks accordingly, keeping the same Error messages and behavior for missing templates in each branch.docs/configuration.md (1)
49-49: Minor: Sentence starts with lowercase.The paragraph at line 49 starts with lowercase "the shipped config templates...". Consider capitalizing for consistency with typical prose style, though this is a stylistic nit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/configuration.md` at line 49, Minor stylistic issue: capitalize the first word of the sentence that currently begins "the shipped config templates include 9 base model families..." in docs/configuration.md so it reads "The shipped config templates include 9 base model families..." and leave the rest of the sentence unchanged (keep references to model names like gpt-5.4-pro and gpt-5.3-codex-spark intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/development/CONFIG_FLOW.md`:
- Around line 69-73: Add the missing bullet for the modern template section
documenting gpt-5.1's context size: insert a line alongside the other model
bullets (e.g., near the lines listing gpt-5.4 and gpt-5.4-mini variants) stating
that `gpt-5.1` uses `context=272,000`, keeping the same format as the
surrounding bullets so the list remains consistent with `config/README.md` and
`docs/configuration.md`.
---
Outside diff comments:
In `@docs/development/CONFIG_FLOW.md`:
- Around line 122-127: Update the legacy model entry count text in
CONFIG_FLOW.md: replace the phrase "26 explicit model entries" with "34 explicit
model entries" so the docs match the actual config/opencode-legacy.json shipped
presets; locate the exact string "26 explicit model entries" in the
CONFIG_FLOW.md content and change the number to 34.
---
Nitpick comments:
In `@docs/configuration.md`:
- Line 49: Minor stylistic issue: capitalize the first word of the sentence that
currently begins "the shipped config templates include 9 base model families..."
in docs/configuration.md so it reads "The shipped config templates include 9
base model families..." and leave the rest of the sentence unchanged (keep
references to model names like gpt-5.4-pro and gpt-5.3-codex-spark intact).
In `@scripts/install-opencode-codex-auth.js`:
- Around line 174-180: The main function currently checks both
modernTemplatePath and legacyTemplatePath regardless of the selected install
mode; change the validation to only check the template required by the chosen
mode (e.g., if the install is modern only validate modernTemplatePath; if legacy
only validate legacyTemplatePath; if neither or combined validate both). Locate
the mode-determining variable or flag used by this script (for example an
isModern flag or process.argv/commander option) and branch the existsSync checks
accordingly, keeping the same Error messages and behavior for missing templates
in each branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e291ebba-9ee0-4762-9c42-6ff5efbdf34b
📒 Files selected for processing (6)
README.mdconfig/README.mddocs/configuration.mddocs/development/CONFIG_FLOW.mddocs/getting-started.mdscripts/install-opencode-codex-auth.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install-opencode-codex-auth.js`:
- Around line 15-22: Move the help-check to run before the mutual-exclusion
guard so help is shown regardless of conflicting flags: check args.has("--help")
|| args.has("-h") and display the help/usage (and exit) before evaluating
requestedModern/requestedLegacy or computing configMode; specifically adjust the
order around the symbols requestedModern, requestedLegacy, configMode and the
args.has(...) check so --help short-circuits and returns help output
immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74cea36b-f12d-4b22-9ea9-5784a5c48d1f
📒 Files selected for processing (3)
docs/configuration.mddocs/development/CONFIG_FLOW.mdscripts/install-opencode-codex-auth.js
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/configuration.md
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install-opencode-codex-auth.js`:
- Around line 233-247: clearCache currently returns early when skipCacheClear
(--no-cache-clear) is true, preventing removePluginFromCachePackage from running
and leaving a pinned oc-chatgpt-multi-auth entry; move the call to
removePluginFromCachePackage(paths, dryRun) so it always executes regardless of
skipCacheClear (i.e., run it before the early return or after the skip branch)
to ensure the unpin step in removePluginFromCachePackage always runs; update
clearCache to still respect dryRun for removePluginFromCachePackage by passing
the dryRun flag through.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6476e4a1-d40e-4744-9336-b55118b21eb5
📒 Files selected for processing (2)
scripts/install-opencode-codex-auth.jstest/install-opencode-codex-auth.test.ts
ndycode
added a commit
that referenced
this pull request
Apr 6, 2026
Clarify modern model catalog counts and variant presets
ndycode
added a commit
that referenced
this pull request
Apr 6, 2026
Clarify modern model catalog counts and variant presets
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.
Summary
--modernfor the compact variant-based config and--legacyfor the explicit legacy-only layoutEPERM/EBUSYretry handling, and redacted error logging that never echoes config bodies--helpbefore conflicting mode validationTesting
node --check scripts/install-opencode-codex-auth.jsnode scripts/install-opencode-codex-auth.js --modern --legacy --helpnpx vitest run test/install-opencode-codex-auth.test.tsgit diff --checkCloses #98
Summary by CodeRabbit
New Features
Documentation
UX
Tests
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this pr resolves the two previously-flagged blockers (unsafe spreads in
mergeFullTemplate, missing concurrency/token-safety hardening) and ships a genuine new capability: a merged "full" catalog default so users get both--variantbase-model entries and the full explicit preset catalog without hand-editingopencode.json.key changes:
runInstallernow defaults toconfigMode="full", reading both templates concurrently and merging them throughmergeFullTemplatewith collision detection —--modernand--legacybecome explicit opt-outswriteFileAtomicwrites to a temp file then callsrenameWithWindowsRetry, preventing truncatedopencode.jsonon windows AV/indexer lock events;mode: 0o600limits token-carrying config exposure at the filesystem levelEPERM/EBUSYfor bothrenameandcopyFile; non-lock errors are re-thrown immediatelyformatErrorForLogextracts onlyerror.message, ensuring provider configs or credentials in the full error object are never echoed to stdout--helpshort-circuit:parseCliArgsnow returnswantsHelpbefore any flag-conflict validation, so--modern --legacy --helpshows help rather than throwing--no-cache-clearunpin behavior, collision detection, and windows retry for bothcopyFileandrenamerunInstallerintegration tests only exercisefullmode;--modernand--legacysingle-template paths have no end-to-end vitest coverage (P2, not a blocker)Confidence Score: 4/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant CLI as CLI / npx participant I as runInstaller participant P as parseCliArgs participant L as loadTemplate participant M as mergeFullTemplate participant W as writeFileAtomic participant R as renameWithWindowsRetry CLI->>I: runInstaller(argv) I->>P: parseCliArgs(argv) alt "--help / -h" P-->>I: wantsHelp=true I-->>CLI: printHelp, exitCode=0 else "--modern + --legacy" P-->>I: throw error else "valid flags" P-->>I: configMode, dryRun, skipCacheClear end I->>L: loadTemplate(configMode, paths) alt "mode=modern" L-->>I: readJson(modern) else "mode=legacy" L-->>I: readJson(legacy) else "mode=full (default)" L->>M: mergeFullTemplate(modern, legacy) M-->>L: merged or collision error L-->>I: merged template end I->>I: backupConfig via copyFileWithWindowsRetry loop "up to 5x on EPERM/EBUSY" I->>I: retry copyFile with exp backoff end I->>I: readJson existing config, preserve custom keys I->>W: writeFileAtomic(configPath, content) W->>W: writeFile tempPath mode=0o600 W->>R: renameWithWindowsRetry(tempPath, configPath) loop "up to 5x on EPERM/EBUSY" R->>R: rename with exp backoff end W-->>I: done or cleanup tempPath I->>I: clearCache or removePluginFromCachePackage I-->>CLI: exitCode=0, action=install, configMode, configPathPrompt To Fix All With AI
Reviews (8): Last reviewed commit: "Guard full template provider spreads" | Re-trigger Greptile
Context used: