-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix:(opencode plugin) Model changes overwritten on first message in new session #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughModified superpowers.js to introduce dynamic path discovery for dependencies and directories, adding runtime state helpers for UI model integration, and enhancing bootstrap/skill loading flows to inject model payloads conditionally based on discovered model.json data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.opencode/plugin/superpowers.js:
- Around line 71-79: The getTUIModelAndAgent function reads model.json and
returns data.recent?.[0] without validating required fields; update
getTUIModelAndAgent to check that the selected model (the model variable
returned) exists and that model.providerID and model.modelID are non-empty
before returning it, otherwise return null/undefined or a safe fallback;
reference the model variable and the modelFile/read path logic so you validate
fields after JSON.parse and before injecting/returning the object to prevent
undefined providerID/modelID from being propagated.
- Around line 14-52: The dynamic import of skillsCorePath (await
import(skillsCorePath)) will fail on Windows because Node ESM requires file
URLs; change the import to convert the filesystem path to a file URL using
pathToFileURL from the 'url' module (e.g., call
pathToFileURL(skillsCorePath).href) before calling await import, and ensure
pathToFileURL is imported at the top so the import of skillsCore (const
skillsCore = await import(...)) uses a valid file:// URL.
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
|
|
||
| // Find the superpowers lib directory by looking in common locations | ||
| const findSkillsCorePath = () => { | ||
| const possiblePaths = [ | ||
| path.join(__dirname, '../../lib/skills-core.js'), | ||
| path.join(process.cwd(), '.opencode/lib/skills-core.js'), | ||
| path.join(path.dirname(__dirname), 'superpowers/lib/skills-core.js'), | ||
| path.join(path.dirname(__dirname), 'lib/skills-core.js'), | ||
| ]; | ||
|
|
||
| for (const p of possiblePaths) { | ||
| if (fs.existsSync(p)) { | ||
| return p; | ||
| } | ||
| } | ||
| throw new Error(`skills-core.js not found. Looked in: ${possiblePaths.join(', ')}`); | ||
| }; | ||
|
|
||
| const findSuperpowersSkillsDir = () => { | ||
| const possiblePaths = [ | ||
| path.join(__dirname, '../../skills'), | ||
| path.join(process.cwd(), '.opencode/skills'), | ||
| path.join(path.dirname(__dirname), 'superpowers/skills'), | ||
| path.join(path.dirname(__dirname), 'skills'), | ||
| ]; | ||
|
|
||
| for (const p of possiblePaths) { | ||
| if (fs.existsSync(p)) { | ||
| return p; | ||
| } | ||
| } | ||
| throw new Error(`Superpowers skills directory not found. Looked in: ${possiblePaths.join(', ')}`); | ||
| }; | ||
|
|
||
| const skillsCorePath = findSkillsCorePath(); | ||
| const skillsCore = await import(skillsCorePath); | ||
| const superpowersSkillsDir = findSuperpowersSkillsDir(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Node.js ESM import() Windows absolute file paths pathToFileURL
💡 Result:
Short answer: convert the Windows filesystem path to a file: URL (use url.pathToFileURL or new URL) and pass that URL (or its .href) to import(). Node ESM resolves modules by URL, so plain Windows paths won't work.
Example (recommended):
- import { pathToFileURL } from 'url';
- const fileUrl = pathToFileURL('C:\project\mod.js').href; // => 'file:///C:/project/mod.js'
- const mod = await import(fileUrl);
Or using URL:
- const fileUrl = new URL('file:///C:/project/mod.js').href;
- const mod = await import(fileUrl);
Notes:
- Ensure the file extension is present (e.g. .js). Node ESM uses URL semantics (percent-encoding for special chars) so pathToFileURL is recommended to get correct encoding and drive-letter form. [1][2]
Sources:
- Node.js ESM docs (URL / file: URLs, recommendation to use url.pathToFileURL). [1][2]
🏁 Script executed:
cd .opencode/plugin && head -60 superpowers.js | tail -50Repository: obra/superpowers
Length of output: 1752
Convert the dynamic import path to a file URL for Windows compatibility.
await import(skillsCorePath) at line 46 will throw ERR_INVALID_MODULE_SPECIFIER on Windows when skillsCorePath is an absolute path like C:\project\.... Node.js ESM requires file URLs, not filesystem paths. Use pathToFileURL() to convert the path.
Proposed fix
-import { fileURLToPath } from 'url';
+import { fileURLToPath, pathToFileURL } from 'url';
...
-const skillsCore = await import(skillsCorePath);
+const skillsCore = await import(pathToFileURL(skillsCorePath).href);🤖 Prompt for AI Agents
In @.opencode/plugin/superpowers.js around lines 14 - 52, The dynamic import of
skillsCorePath (await import(skillsCorePath)) will fail on Windows because Node
ESM requires file URLs; change the import to convert the filesystem path to a
file URL using pathToFileURL from the 'url' module (e.g., call
pathToFileURL(skillsCorePath).href) before calling await import, and ensure
pathToFileURL is imported at the top so the import of skillsCore (const
skillsCore = await import(...)) uses a valid file:// URL.
| const getTUIModelAndAgent = () => { | ||
| try { | ||
| const modelFile = path.join(getStateDir(), 'model.json'); | ||
| const data = JSON.parse(fs.readFileSync(modelFile, 'utf8')); | ||
|
|
||
| const model = data.recent?.[0]; | ||
| return { | ||
| model | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against incomplete model.json entries before injecting.
If data.recent[0] exists but lacks providerID or modelID, you’ll inject undefined values and risk overwriting the intended model. Validate both fields before returning.
💡 Suggested hardening
- const model = data.recent?.[0];
- return {
- model
- };
+ const model = data.recent?.[0];
+ if (!model?.providerID || !model?.modelID) return null;
+ return {
+ model: {
+ providerID: model.providerID,
+ modelID: model.modelID
+ }
+ };🤖 Prompt for AI Agents
In @.opencode/plugin/superpowers.js around lines 71 - 79, The
getTUIModelAndAgent function reads model.json and returns data.recent?.[0]
without validating required fields; update getTUIModelAndAgent to check that the
selected model (the model variable returned) exists and that model.providerID
and model.modelID are non-empty before returning it, otherwise return
null/undefined or a safe fallback; reference the model variable and the
modelFile/read path logic so you validate fields after JSON.parse and before
injecting/returning the object to prevent undefined providerID/modelID from
being propagated.
Problem
When changing model in OpenCode TUI before sending any message, the plugin's bootstrap injection causes the model to revert to the previous session's model.
Scenario that triggers bug:
modelfield)createUserMessage()useslastModel()to find modellastModel()finds Model A from previous sessionSimple explanation:
You change model, but plugin's system message doesn't specify which model to use.
Server looks for the last user message to guess the model.
No messages sent yet in this session, so it grabs the model from your last session.
Your change gets overwritten by that old model.
Root Cause
When no messages exist in current session,
lastModel()falls back to previous session's last user message. Plugin doesn't specify current model in synthetic messages.Solution
Plugin reads current model from TUI's cache file (
~/.local/state/opencode/model.json) which persists your model selection. Includes this model in all synthetic messages.Changes
getStateDir()helper to find opencode state directorygetTUIModelAndAgent()to read model frommodel.jsoncachemodel: {providerID, modelID}in all synthetic messagesskills-core.js(supports symlink and copy installs)Testing
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.