feat: omo config profiles integration + audit fixes (skills/plugins/marker/i18n) - #56
feat: omo config profiles integration + audit fixes (skills/plugins/marker/i18n)#56zswll2 wants to merge 27 commits into
Conversation
… provider Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…rofiles blocks Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…locks Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…stem sanitize Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…kills Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…y UX Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
|
@zswll2 is attempting to deploy a commit to the Projects Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function createProfile(name) { | ||
| safeName(name); | ||
| const dir = path.join(PROFILES_DIR, name); | ||
| if (fs.existsSync(dir)) throw new Error('Profile already exists'); | ||
| fs.mkdirSync(dir, { recursive: true }); | ||
| const safe = safeName(name); | ||
| // setOmoProfile writes profiles.<name>.[opencode] surgically and creates the | ||
| // omo.jsonc skeleton first when the file does not exist yet (Todo 3). | ||
| configProviders.setOmoProfile(OMO_CONFIG_PATH, safe, {}); | ||
| return { success: true }; | ||
| } |
There was a problem hiding this comment.
When /api/profiles receives an existing profile name, createProfile unconditionally rewrites its [opencode] block with {}, silently erasing the saved configuration instead of rejecting the duplicate.
| function createProfile(name) { | |
| safeName(name); | |
| const dir = path.join(PROFILES_DIR, name); | |
| if (fs.existsSync(dir)) throw new Error('Profile already exists'); | |
| fs.mkdirSync(dir, { recursive: true }); | |
| const safe = safeName(name); | |
| // setOmoProfile writes profiles.<name>.[opencode] surgically and creates the | |
| // omo.jsonc skeleton first when the file does not exist yet (Todo 3). | |
| configProviders.setOmoProfile(OMO_CONFIG_PATH, safe, {}); | |
| return { success: true }; | |
| } | |
| function createProfile(name) { | |
| const safe = safeName(name); | |
| if (configProviders.getOmoProfile(OMO_CONFIG_PATH, safe)) { | |
| throw new Error('Profile already exists'); | |
| } | |
| // setOmoProfile writes profiles.<name>.[opencode] surgically and creates the | |
| // omo.jsonc skeleton first when the file does not exist yet (Todo 3). | |
| configProviders.setOmoProfile(OMO_CONFIG_PATH, safe, {}); | |
| return { success: true }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/profile-manager.js
Line: 133-139
Comment:
**Duplicate creation erases profiles**
When `/api/profiles` receives an existing profile name, `createProfile` unconditionally rewrites its `[opencode]` block with `{}`, silently erasing the saved configuration instead of rejecting the duplicate.
```suggestion
function createProfile(name) {
const safe = safeName(name);
if (configProviders.getOmoProfile(OMO_CONFIG_PATH, safe)) {
throw new Error('Profile already exists');
}
// setOmoProfile writes profiles.<name>.[opencode] surgically and creates the
// omo.jsonc skeleton first when the file does not exist yet (Todo 3).
configProviders.setOmoProfile(OMO_CONFIG_PATH, safe, {});
return { success: true };
}
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| const result = profileManager.activateProfile(req.params.name); | ||
| try { | ||
| writeStudioMarker(req.params.name); | ||
| } catch (markerErr) { | ||
| return res.status(500).json({ error: `Failed to persist active profile marker: ${markerErr.message}` }); | ||
| } | ||
| res.json(result); |
There was a problem hiding this comment.
If studio.json is unwritable while ~/.omo/omo.jsonc remains writable, activation bakes the configuration and deletes the source profile before writeStudioMarker fails, causing a 500 response while the activation remains committed and the persisted active state stays stale.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/index.js
Line: 5141-5147
Comment:
**Activation marker commits separately**
If `studio.json` is unwritable while `~/.omo/omo.jsonc` remains writable, activation bakes the configuration and deletes the source profile before `writeStudioMarker` fails, causing a 500 response while the activation remains committed and the persisted active state stays stale.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const port = await findAvailablePort(DEFAULT_PORT); | ||
| app.listen(port, '127.0.0.1', () => { | ||
| console.log(`Server running at http://127.0.0.1:${port}`); | ||
| app.listen(port, HOST, () => { |
There was a problem hiding this comment.
If HOST is set to 0.0.0.0 or another network-reachable address, the server exposes unauthenticated management routes such as /api/shutdown, /api/config, /api/restore, and /api/profiles, allowing any client that reaches the port to terminate the process or read and alter the host user's OpenCode state.
How this was verified: The configured host is passed to app.listen, while originless requests are accepted and the privileged routes have no authentication checks.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/index.js
Line: 5878
Comment:
**Network bind bypasses local boundary**
If `HOST` is set to `0.0.0.0` or another network-reachable address, the server exposes unauthenticated management routes such as `/api/shutdown`, `/api/config`, `/api/restore`, and `/api/profiles`, allowing any client that reaches the port to terminate the process or read and alter the host user's OpenCode state.
**How this was verified:** The configured host is passed to `app.listen`, while originless requests are accepted and the privileged routes have no authentication checks.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
This PR replaces legacy symlink-based profile switching with OMO profiles stored in
WalkthroughThe server now discovers OMO configuration files and edits only the Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant ProfileManager
participant ConfigProviders
participant OmoJsonc
Client->>Server: Select profile name
Server->>ProfileManager: Activate profile
ProfileManager->>ConfigProviders: Read or import profile block
ConfigProviders->>OmoJsonc: Merge [opencode] configuration
ProfileManager->>ConfigProviders: Remove activated profile block
Server-->>Client: Return activation result and profile list
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)server/lib/config-providers.test.jsFile contains syntax errors that prevent linting: Line 1: Illegal use of an import declaration outside of a module; Line 2: Illegal use of an import declaration outside of a module; Line 3: Illegal use of an import declaration outside of a module; Line 4: Illegal use of an import declaration outside of a module; Line 5: Illegal use of an import declaration outside of a module; Line 7: Illegal use of an import declaration outside of a module 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client-next/dev-with-port.js`:
- Line 22: Update the dev server spawn configuration in dev so it binds to
127.0.0.1 by default instead of 0.0.0.0, and only use an externally accessible
hostname when an explicit environment option requests it.
In `@client-next/messages/en.json`:
- Around line 708-715: Remove the earlier duplicate usage range labels from
client-next/messages/en.json lines 661-667 and client-next/messages/zh-CN.json
lines 661-667, while retaining the later blocks at client-next/messages/en.json
lines 708-715 and client-next/messages/zh-CN.json lines 709-715. Ensure each
usage object defines range24h, range7d, range30d, range3m, range6m, range1y, and
rangeCustom exactly once.
In `@client-next/src/components/provider-detail.tsx`:
- Line 513: Replace the hardcoded Chinese OMO profile description with one
shared next-intl message key across provider-detail.tsx lines 513-513,
provider-card.tsx lines 23-23, and config/page.tsx lines 96-96; use the same
localized message in all three components so each UI displays consistently in
the active language.
In `@server/http-smoke.test.js`:
- Around line 96-106: Update the child environment in the smoke test’s spawn
call to explicitly set HOST to 127.0.0.1 alongside HOME, ensuring
server/index.js logs the loopback startup URL regardless of the parent process
environment.
- Around line 108-112: The health check around the fetch call in the smoke test
can remain pending indefinitely. Add an AbortController-based short timeout to
the /api/health request, ensure the timer is cleared after completion, and let
an abort propagate as a smoke-test failure while preserving the existing status
and body assertions.
In `@server/index.js`:
- Line 121: Add a startup warning near the HOST initialization or server startup
path when HOST resolves to a non-loopback address, explicitly stating that the
unauthenticated write APIs are network-exposed and require an authenticating
proxy. Preserve the existing 127.0.0.1 default and avoid warning for
loopback-only hosts.
- Line 2219: Update the searchRoots filtering around getSearchRoots() so each
root is normalized with path resolution before comparison, and compare it
against the resolved cwd using the same normalization. Preserve exclusion of the
cwd root for relative paths, trailing separators, and equivalent resolved paths.
- Around line 2739-2762: Update the provider activation handler around
profileManager.activateProfile to call the existing writeStudioMarker(name)
after successful activation, matching the persistence behavior of
/api/profiles/:name/activate. Keep marker writing out of the failure path and
preserve the existing response and diagnostics handling.
In `@server/lib/config-providers.js`:
- Around line 519-544: Canonicalize profile-name handling across both sites: in
server/lib/config-providers.js, update listOmoProfiles to return only keys where
sanitizeOmoKey(key) equals the original key; in server/profile-manager.js,
update safeName to throw when sanitizeOmoKey(name) differs from name, while
preserving its existing separator and dot-navigation validation.
- Around line 519-544: Update listOmoProfiles to return only profile keys that
are fixed points of sanitizeOmoKey, or otherwise ensure getOmoProfile and
deleteOmoProfile can resolve every listed key consistently. Preserve the
existing behavior for valid names while preventing entries such as profiles
ending in .json or .jsonc from being listed when subsequent API operations
cannot access them.
- Around line 373-402: Apply the existing symlink rejection logic to every root
and config candidate produced by resolveOmoCandidateRoots, getOmoSearchRoots,
and buildOmoRootCandidates. Do not directly accept caller roots named .omo or
the homedir-based ~/.omo unless the directory is not symlinked, and ensure
generated omo.jsonc candidates are rejected when either the .omo directory or
file is symlinked before findExistingPaths can select them.
In `@server/lib/config-providers.test.js`:
- Around line 367-384: Update the test around the existing provider-root
precedence case to exercise the default search scope instead of passing an
ordered non-empty roots array. Set up a temporary home directory containing
~/.omo and another omo root, call providers.getOmoSearchRoots with the relevant
cwd and homeDir options, and assert that the ~/.omo path is returned first,
directly verifying getOmoSearchRoots precedence.
In `@server/profile-manager.test.js`:
- Around line 348-359: Update the importLegacyProfile test fixture so
backup-2026 contains a valid omo.jsonc before asserting its behavior, ensuring
the assertion exercises artifact-name handling rather than the missing-config
branch. Then update importLegacyProfile to reject artifact names via
isLegacyArtifactName, preserving the existing no-import error behavior and
confirming listProfiles continues to hide the artifact directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 37f7e650-f848-4cc1-9a50-f780a28d1e3b
⛔ Files ignored due to path filters (3)
client-next/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
.gitignoreREADME_CN.mdclient-next/dev-with-port.jsclient-next/messages/en.jsonclient-next/messages/ko.jsonclient-next/messages/zh-CN.jsonclient-next/src/app/config/page.tsxclient-next/src/app/profiles/page.tsxclient-next/src/components/provider-card.tsxclient-next/src/components/provider-detail.tsxclient-next/src/lib/api.tsclient-next/src/types/index.tsserver/http-smoke.test.jsserver/index.jsserver/lib/config-providers.jsserver/lib/config-providers.test.jsserver/profile-manager.jsserver/profile-manager.test.js
| console.log(`Starting Next.js on port ${port}`); | ||
|
|
||
| const dev = spawn('npx', ['next', 'dev'], { | ||
| const dev = spawn('npx', ['next', 'dev', '--hostname', '0.0.0.0'], { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not expose the development server by default.
--hostname 0.0.0.0 accepts connections on every routable interface. This exposes the Next.js development server to other hosts on the local network.
Keep 127.0.0.1 as the default. Require an explicit environment option for external binding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client-next/dev-with-port.js` at line 22, Update the dev server spawn
configuration in dev so it binds to 127.0.0.1 by default instead of 0.0.0.0, and
only use an externally accessible hostname when an explicit environment option
requests it.
| "cost": "Cost", | ||
| "range24h": "Last 24 hours", | ||
| "range7d": "Last 7 days", | ||
| "range30d": "Last 30 days", | ||
| "range3m": "Last 3 months", | ||
| "range6m": "Last 6 months", | ||
| "range1y": "Last year", | ||
| "rangeCustom": "Custom range" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Duplicate usage range keys in two locale files. A new block of range labels was appended to the usage object without removing the existing block, so each of range24h, range7d, range30d, range3m, range6m, range1y, and rangeCustom is defined twice in the same object. JSON.parse keeps the last occurrence; duplicate-key linters and some i18n extraction tools report an error.
client-next/messages/en.json#L708-L715: delete the earlier definitions at Lines 661-667 and keep this block.client-next/messages/zh-CN.json#L709-L715: delete the earlier definitions at Lines 661-667 and keep this block.
📍 Affects 2 files
client-next/messages/en.json#L708-L715(this comment)client-next/messages/zh-CN.json#L709-L715
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client-next/messages/en.json` around lines 708 - 715, Remove the earlier
duplicate usage range labels from client-next/messages/en.json lines 661-667 and
client-next/messages/zh-CN.json lines 661-667, while retaining the later blocks
at client-next/messages/en.json lines 708-715 and
client-next/messages/zh-CN.json lines 709-715. Ensure each usage object defines
range24h, range7d, range30d, range3m, range6m, range1y, and rangeCustom exactly
once.
| <p className="text-xs text-muted-foreground"> | ||
| Save named OpenAgent config files and switch the active plugin config by copying one into place. | ||
| {profileDir && <code className="ml-1 rounded bg-muted px-1 py-0.5 font-mono">{profileDir}</code>} | ||
| OMO 配置块（profiles.<name>），切换会把选中 profile 合并进当前配置并移除该 profile 块。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The same Chinese OMO description is hardcoded in three English components. One sentence describing profiles.<name> blocks was written in Chinese and copied into three places, bypassing the next-intl catalogs that the rest of the app uses. English and Korean users see Chinese text inside otherwise English UI.
client-next/src/components/provider-detail.tsx#L513-L513: replace the Chinese paragraph with anext-intlmessage key, or with English matching the surrounding panel.client-next/src/components/provider-card.tsx#L23-L23: replace theoh-my-openagentdescription value with the same shared message key.client-next/src/app/config/page.tsx#L96-L96: replace the Chinese sentence in the Alert with the same shared message key so the paragraph reads in one language.
📍 Affects 3 files
client-next/src/components/provider-detail.tsx#L513-L513(this comment)client-next/src/components/provider-card.tsx#L23-L23client-next/src/app/config/page.tsx#L96-L96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client-next/src/components/provider-detail.tsx` at line 513, Replace the
hardcoded Chinese OMO profile description with one shared next-intl message key
across provider-detail.tsx lines 513-513, provider-card.tsx lines 23-23, and
config/page.tsx lines 96-96; use the same localized message in all three
components so each UI displays consistently in the active language.
| const child = spawn(process.execPath, ['index.js'], { | ||
| cwd: __dirname, // server/ — `node index.js` must resolve from here | ||
| env: { ...process.env, HOME: sandboxHome }, | ||
| stdio: ['ignore', 'pipe', 'pipe'], | ||
| }); | ||
|
|
||
| let baseUrl; | ||
| try { | ||
| // Fail LOUDLY if the port probe never matches — no false green. | ||
| baseUrl = await waitForStartupLine(child); | ||
| assert.match(baseUrl, /^http:\/\/127\.0\.0\.1:\d+$/, `unexpected startup URL: ${baseUrl}`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Isolate the smoke test from inherited HOST.
server/index.js logs the configured bind host. If the parent environment sets HOST=0.0.0.0, Line 106 rejects the valid startup URL before the test calls /api/health.
Set HOST: '127.0.0.1' in the spawned child environment. This keeps the smoke test deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/http-smoke.test.js` around lines 96 - 106, Update the child
environment in the smoke test’s spawn call to explicitly set HOST to 127.0.0.1
alongside HOME, ensuring server/index.js logs the loopback startup URL
regardless of the parent process environment.
| const res = await fetch(`${baseUrl}/api/health`); | ||
| assert.strictEqual(res.status, 200, `expected HTTP 200, got ${res.status}`); | ||
| // Misleading-success guard: assert the JSON body, not just the 200. | ||
| const body = await res.json(); | ||
| assert.strictEqual(body.status, 'ok', `expected body.status === 'ok', got ${JSON.stringify(body)}`); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the health request duration.
If the child accepts the connection but does not complete /api/health, await fetch() can remain pending. The finally block then cannot terminate the child until the request fails.
Abort the request after a short timeout and report the timeout as a smoke-test failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/http-smoke.test.js` around lines 108 - 112, The health check around
the fetch call in the smoke test can remain pending indefinitely. Add an
AbortController-based short timeout to the /api/health request, ensure the timer
is cleared after completion, and let an abort propagate as a smoke-test failure
while preserving the existing status and body assertions.
| let result; | ||
| try { | ||
| // M2/MINOR-a: bake-only activation fully delegated to profileManager; | ||
| // M-C foreign-key rejection fires exactly once, there — not here. | ||
| result = profileManager.activateProfile(name); | ||
| } catch (error) { | ||
| return res.status(400).json({ | ||
| success: false, | ||
| diagnostics: [{ | ||
| severity: 'error', | ||
| code: 'OPENAGENT_PROFILE_ACTIVATE_FAILED', | ||
| message: error.message | ||
| }] | ||
| }); | ||
| } | ||
|
|
||
| configProviders.writeConfigTextAtomicSync(activePath, raw, 'utf8'); | ||
| const updatedProvider = getProviderByIdOrNull(provider.id) || provider; | ||
| const updatedDetails = loadProviderDetails(updatedProvider); | ||
| res.json({ | ||
| success: true, | ||
| path: activePath, | ||
| selectedPath: selected.path, | ||
| ...result, | ||
| diagnostics: updatedDetails.diagnostics || [], | ||
| revision: updatedDetails.revision, | ||
| profiles: listOpenAgentProfiles(updatedProvider) | ||
| profiles: buildOmoProfilesResponse(updatedProvider).profiles |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist the active marker here too, or activation through the provider UI is lost on restart.
/api/profiles/:name/activate (Line 5141) calls profileManager.activateProfile and then writeStudioMarker(name). This endpoint calls the same profileManager.activateProfile but never writes the marker.
The result: the profiles page reports the activated profile after a restart, but activation performed through the provider detail panel does not survive a restart, and /api/profiles no longer filters that name out of legacy. Two entry points to one operation must produce the same persisted state.
Proposed fix
} catch (error) {
return res.status(400).json({
success: false,
diagnostics: [{
severity: 'error',
code: 'OPENAGENT_PROFILE_ACTIVATE_FAILED',
message: error.message
}]
});
}
+ try {
+ writeStudioMarker(name);
+ } catch (markerErr) {
+ return res.status(500).json({
+ success: false,
+ diagnostics: [{
+ severity: 'error',
+ code: 'OPENAGENT_PROFILE_MARKER_FAILED',
+ message: markerErr.message
+ }]
+ });
+ }
+
const updatedProvider = getProviderByIdOrNull(provider.id) || provider;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let result; | |
| try { | |
| // M2/MINOR-a: bake-only activation fully delegated to profileManager; | |
| // M-C foreign-key rejection fires exactly once, there — not here. | |
| result = profileManager.activateProfile(name); | |
| } catch (error) { | |
| return res.status(400).json({ | |
| success: false, | |
| diagnostics: [{ | |
| severity: 'error', | |
| code: 'OPENAGENT_PROFILE_ACTIVATE_FAILED', | |
| message: error.message | |
| }] | |
| }); | |
| } | |
| configProviders.writeConfigTextAtomicSync(activePath, raw, 'utf8'); | |
| const updatedProvider = getProviderByIdOrNull(provider.id) || provider; | |
| const updatedDetails = loadProviderDetails(updatedProvider); | |
| res.json({ | |
| success: true, | |
| path: activePath, | |
| selectedPath: selected.path, | |
| ...result, | |
| diagnostics: updatedDetails.diagnostics || [], | |
| revision: updatedDetails.revision, | |
| profiles: listOpenAgentProfiles(updatedProvider) | |
| profiles: buildOmoProfilesResponse(updatedProvider).profiles | |
| let result; | |
| try { | |
| // M2/MINOR-a: bake-only activation fully delegated to profileManager; | |
| // M-C foreign-key rejection fires exactly once, there — not here. | |
| result = profileManager.activateProfile(name); | |
| } catch (error) { | |
| return res.status(400).json({ | |
| success: false, | |
| diagnostics: [{ | |
| severity: 'error', | |
| code: 'OPENAGENT_PROFILE_ACTIVATE_FAILED', | |
| message: error.message | |
| }] | |
| }); | |
| } | |
| try { | |
| writeStudioMarker(name); | |
| } catch (markerErr) { | |
| return res.status(500).json({ | |
| success: false, | |
| diagnostics: [{ | |
| severity: 'error', | |
| code: 'OPENAGENT_PROFILE_MARKER_FAILED', | |
| message: markerErr.message | |
| }] | |
| }); | |
| } | |
| const updatedProvider = getProviderByIdOrNull(provider.id) || provider; | |
| const updatedDetails = loadProviderDetails(updatedProvider); | |
| res.json({ | |
| success: true, | |
| ...result, | |
| diagnostics: updatedDetails.diagnostics || [], | |
| revision: updatedDetails.revision, | |
| profiles: buildOmoProfilesResponse(updatedProvider).profiles |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/index.js` around lines 2739 - 2762, Update the provider activation
handler around profileManager.activateProfile to call the existing
writeStudioMarker(name) after successful activation, matching the persistence
behavior of /api/profiles/:name/activate. Keep marker writing out of the failure
path and preserve the existing response and diagnostics handling.
| const buildOmoRootCandidates = (omoRoots) => { | ||
| const candidates = []; | ||
| for (const root of omoRoots) { | ||
| for (const basename of OMO_BASENAMES) { | ||
| candidates.push(toAbsolutePath(root, basename)); | ||
| } | ||
| } | ||
| return uniqNormalizedPaths(candidates); | ||
| }; | ||
|
|
||
| // Derive omo candidate roots from the caller-provided search scope when one is | ||
| // given (caller roots that are `.omo` dirs are used directly; other caller roots | ||
| // contribute their `root/.omo` subdir only when it holds a loadable omo config). | ||
| // Fall back to the homedir-based getOmoSearchRoots() only when the caller scoped | ||
| // nothing, so caller-scoped detection never leaks the real ~/.omo into results. | ||
| const resolveOmoCandidateRoots = ({ roots = [], customPaths = [] } = {}) => { | ||
| const callerRoots = uniqNormalizedPaths([...(roots || []), ...(customPaths || [])]); | ||
| if (callerRoots.length === 0) return getOmoSearchRoots(); | ||
| const omoRoots = []; | ||
| for (const root of callerRoots) { | ||
| if (getPathBasenameAnySeparator(root) === '.omo') { | ||
| omoRoots.push(root); | ||
| continue; | ||
| } | ||
| if (findLoadableOmoConfigPathInDir(root)) { | ||
| omoRoots.push(normalizePath(path.join(root, '.omo'))); | ||
| } | ||
| } | ||
| return uniqNormalizedPaths(omoRoots); | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
The symlink guard does not cover .omo roots.
findLoadableOmoConfigPathInDir rejects a symlinked .omo directory and a symlinked omo.jsonc. That guard is skipped for two paths:
resolveOmoCandidateRootspushes a caller root named.omodirectly (Line 393-395).getOmoSearchRootsalways adds~/.omowithout a check (Line 368).
buildOmoRootCandidates then emits <root>/omo.jsonc and findExistingPaths accepts it even when the directory or the file is a symlink. So a symlinked ~/.omo/omo.jsonc is still selected as activePath, and writeOmoBlock later replaces it through atomicWriteTextSync. Apply the same symlink check to every candidate.
🛡️ Proposed fix
const buildOmoRootCandidates = (omoRoots) => {
const candidates = [];
for (const root of omoRoots) {
+ if (isSymlinkedPathSync(root)) continue;
for (const basename of OMO_BASENAMES) {
- candidates.push(toAbsolutePath(root, basename));
+ const candidate = toAbsolutePath(root, basename);
+ if (!isSymlinkedPathSync(candidate)) candidates.push(candidate);
}
}
return uniqNormalizedPaths(candidates);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const buildOmoRootCandidates = (omoRoots) => { | |
| const candidates = []; | |
| for (const root of omoRoots) { | |
| for (const basename of OMO_BASENAMES) { | |
| candidates.push(toAbsolutePath(root, basename)); | |
| } | |
| } | |
| return uniqNormalizedPaths(candidates); | |
| }; | |
| // Derive omo candidate roots from the caller-provided search scope when one is | |
| // given (caller roots that are `.omo` dirs are used directly; other caller roots | |
| // contribute their `root/.omo` subdir only when it holds a loadable omo config). | |
| // Fall back to the homedir-based getOmoSearchRoots() only when the caller scoped | |
| // nothing, so caller-scoped detection never leaks the real ~/.omo into results. | |
| const resolveOmoCandidateRoots = ({ roots = [], customPaths = [] } = {}) => { | |
| const callerRoots = uniqNormalizedPaths([...(roots || []), ...(customPaths || [])]); | |
| if (callerRoots.length === 0) return getOmoSearchRoots(); | |
| const omoRoots = []; | |
| for (const root of callerRoots) { | |
| if (getPathBasenameAnySeparator(root) === '.omo') { | |
| omoRoots.push(root); | |
| continue; | |
| } | |
| if (findLoadableOmoConfigPathInDir(root)) { | |
| omoRoots.push(normalizePath(path.join(root, '.omo'))); | |
| } | |
| } | |
| return uniqNormalizedPaths(omoRoots); | |
| }; | |
| const buildOmoRootCandidates = (omoRoots) => { | |
| const candidates = []; | |
| for (const root of omoRoots) { | |
| if (isSymlinkedPathSync(root)) continue; | |
| for (const basename of OMO_BASENAMES) { | |
| const candidate = toAbsolutePath(root, basename); | |
| if (!isSymlinkedPathSync(candidate)) candidates.push(candidate); | |
| } | |
| } | |
| return uniqNormalizedPaths(candidates); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/config-providers.js` around lines 373 - 402, Apply the existing
symlink rejection logic to every root and config candidate produced by
resolveOmoCandidateRoots, getOmoSearchRoots, and buildOmoRootCandidates. Do not
directly accept caller roots named .omo or the homedir-based ~/.omo unless the
directory is not symlinked, and ensure generated omo.jsonc candidates are
rejected when either the .omo directory or file is symlinked before
findExistingPaths can select them.
| const listOmoProfiles = (filePath) => { | ||
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return []; | ||
| try { | ||
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | ||
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return []; | ||
| return Object.keys(parsed.profiles); | ||
| } catch { | ||
| return []; | ||
| } | ||
| }; | ||
|
|
||
| // Whole profiles.<name> object, or null (missing/malformed file, invalid name, | ||
| // absent profile). Read-side never throws, matching getOmoConfigBlock. | ||
| const getOmoProfile = (filePath, name) => { | ||
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return null; | ||
| const safeName = sanitizeOmoKey(name); | ||
| if (!safeName) return null; | ||
| try { | ||
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | ||
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return null; | ||
| const profile = parsed.profiles[safeName]; | ||
| return isPlainObject(profile) ? profile : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Profile keys have no single canonical form. Three different name rules are in use: listOmoProfiles emits raw JSON keys, sanitizeOmoKey trims and strips .json/.jsonc, and safeName only blocks separators and dot-navigation. Names that differ between these rules become unreachable or collide with a different profile.
server/lib/config-providers.js#L519-L544: filterlistOmoProfilesto keys wheresanitizeOmoKey(key) === key, so every listed name is readable bygetOmoProfileand removable bydeleteOmoProfile.server/profile-manager.js#L49-L57: makesafeNamethrow whenconfigProviders.sanitizeOmoKey(name) !== name, socreateProfile('work.json')cannot overwriteprofiles.workwith an empty block.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 521-521: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 536-536: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
📍 Affects 2 files
server/lib/config-providers.js#L519-L544(this comment)server/profile-manager.js#L49-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/config-providers.js` around lines 519 - 544, Canonicalize
profile-name handling across both sites: in server/lib/config-providers.js,
update listOmoProfiles to return only keys where sanitizeOmoKey(key) equals the
original key; in server/profile-manager.js, update safeName to throw when
sanitizeOmoKey(name) differs from name, while preserving its existing separator
and dot-navigation validation.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
listOmoProfiles and getOmoProfile do not agree on key identity.
listOmoProfiles returns raw keys of profiles. getOmoProfile and deleteOmoProfile first pass the name through sanitizeOmoKey, which trims whitespace and strips a trailing .json/.jsonc.
A key that is not a fixed point of sanitizeOmoKey becomes unreachable. Example: profiles contains "work.json" (written by hand or by the OMO plugin). listOmoProfiles reports work.json. getOmoProfile(path, 'work.json') looks up work and returns null. server/profile-manager.js deleteProfile then short-circuits at Line 151 and returns { success: true, removed: false }, so the entry can never be removed through the API, and activateProfile reports Profile not found for a name the UI lists.
Filter the listing to keys that survive sanitization, or look up the raw key when the sanitized key is absent.
🐛 Proposed fix
const listOmoProfiles = (filePath) => {
if (typeof filePath !== 'string' || !isFileSync(filePath)) return [];
try {
const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8'));
if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return [];
- return Object.keys(parsed.profiles);
+ // Only keys that round-trip through sanitizeOmoKey are readable/deletable.
+ return Object.keys(parsed.profiles).filter((key) => sanitizeOmoKey(key) === key);
} catch {
return [];
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const listOmoProfiles = (filePath) => { | |
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return []; | |
| try { | |
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | |
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return []; | |
| return Object.keys(parsed.profiles); | |
| } catch { | |
| return []; | |
| } | |
| }; | |
| // Whole profiles.<name> object, or null (missing/malformed file, invalid name, | |
| // absent profile). Read-side never throws, matching getOmoConfigBlock. | |
| const getOmoProfile = (filePath, name) => { | |
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return null; | |
| const safeName = sanitizeOmoKey(name); | |
| if (!safeName) return null; | |
| try { | |
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | |
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return null; | |
| const profile = parsed.profiles[safeName]; | |
| return isPlainObject(profile) ? profile : null; | |
| } catch { | |
| return null; | |
| } | |
| }; | |
| const listOmoProfiles = (filePath) => { | |
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return []; | |
| try { | |
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | |
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return []; | |
| // Only keys that round-trip through sanitizeOmoKey are readable/deletable. | |
| return Object.keys(parsed.profiles).filter((key) => sanitizeOmoKey(key) === key); | |
| } catch { | |
| return []; | |
| } | |
| }; | |
| // Whole profiles.<name> object, or null (missing/malformed file, invalid name, | |
| // absent profile). Read-side never throws, matching getOmoConfigBlock. | |
| const getOmoProfile = (filePath, name) => { | |
| if (typeof filePath !== 'string' || !isFileSync(filePath)) return null; | |
| const safeName = sanitizeOmoKey(name); | |
| if (!safeName) return null; | |
| try { | |
| const parsed = parseJsonText(fs.readFileSync(filePath, 'utf8')); | |
| if (!isPlainObject(parsed) || !isPlainObject(parsed.profiles)) return null; | |
| const profile = parsed.profiles[safeName]; | |
| return isPlainObject(profile) ? profile : null; | |
| } catch { | |
| return null; | |
| } | |
| }; |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 521-521: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 536-536: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/config-providers.js` around lines 519 - 544, Update
listOmoProfiles to return only profile keys that are fixed points of
sanitizeOmoKey, or otherwise ensure getOmoProfile and deleteOmoProfile can
resolve every listed key consistently. Preserve the existing behavior for valid
names while preventing entries such as profiles ending in .json or .jsonc from
being listed when subsequent API operations cannot access them.
| it('prefers the ~/.omo user root over another omo root when both hold omo.jsonc', () => { | ||
| const homeDir = makeTempDir(); | ||
| const otherRoot = makeTempDir(); | ||
| const userOmoDir = path.join(homeDir, '.omo'); | ||
| const otherOmoDir = path.join(otherRoot, '.omo'); | ||
| fs.mkdirSync(userOmoDir, { recursive: true }); | ||
| fs.mkdirSync(otherOmoDir, { recursive: true }); | ||
| const userOmoPath = path.join(userOmoDir, 'omo.jsonc'); | ||
| const otherOmoPath = path.join(otherOmoDir, 'omo.jsonc'); | ||
| fs.writeFileSync(userOmoPath, '{ "[opencode]": { "theme": "user" } }'); | ||
| fs.writeFileSync(otherOmoPath, '{ "[opencode]": { "theme": "other" } }'); | ||
|
|
||
| const openAgent = providers.detectProviders({ roots: [userOmoDir, otherOmoDir] }) | ||
| .find((provider) => provider.id === providers.PROVIDER_IDS.OH_MY_OPENAGENT); | ||
|
|
||
| expect(openAgent.activePath).toBe(userOmoPath); | ||
| expect(providers.getOmoConfigBlock(openAgent.activePath)).toEqual({ theme: 'user' }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
This test does not verify user-root precedence.
The test passes roots: [userOmoDir, otherOmoDir]. With a non-empty caller scope, resolveOmoCandidateRoots returns the caller roots in the given order and never calls getOmoSearchRoots. So the assertion only confirms that the first caller root wins. Swapping the array order would flip the result, and the ~/.omo-first rule in getOmoSearchRoots stays untested.
Add a case that exercises the default scope, for example by setting HOME to a temp dir and calling providers.getOmoSearchRoots({ cwd, homeDir }) directly, and assert that the ~/.omo root is first.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 375-375: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(userOmoPath, '{ "[opencode]": { "theme": "user" } }')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 376-376: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(otherOmoPath, '{ "[opencode]": { "theme": "other" } }')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/config-providers.test.js` around lines 367 - 384, Update the test
around the existing provider-root precedence case to exercise the default search
scope instead of passing an ordered non-empty roots array. Set up a temporary
home directory containing ~/.omo and another omo root, call
providers.getOmoSearchRoots with the relevant cwd and homeDir options, and
assert that the ~/.omo path is returned first, directly verifying
getOmoSearchRoots precedence.
| test('importLegacyProfile throws on missing/invalid legacy dirs and artifact names', (t) => { | ||
| const home = makeTempHome(t); | ||
| writeOmo(home, { '[opencode]': {} }); | ||
| const legacyDir = makeLegacyDirs(home, ['backup-2026', 'empty']); | ||
|
|
||
| const pm = loadProfileManager(home); | ||
| assert.throws(() => pm.importLegacyProfile('backup-2026'), /没有可导入的 omo 配置/, 'artifact dir rejected'); | ||
| writeLegacyConfig(legacyDir, 'empty'); | ||
| assert.throws(() => pm.importLegacyProfile('empty'), /没有可导入的 omo 配置/, 'dir with only opencode.json rejected'); | ||
| assert.throws(() => pm.importLegacyProfile('missing-dir'), /没有可导入的 omo 配置/, 'no dir at all rejected'); | ||
| assert.deepStrictEqual(pm.listProfiles().legacy, ['empty'], 'nothing imported (backup-* is an artifact name)'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The artifact dir rejected assertion does not test artifact rejection.
makeLegacyDirs creates backup-2026 with no config file. importLegacyProfile never calls isLegacyArtifactName; it reaches the 没有可导入的 omo 配置 throw because both omo.jsonc and oh-my-openagent.json are absent. The three assertions on Line 354, Line 356 and Line 357 all exercise the same missing-config branch.
The untested case is the one that matters: an artifact-named directory that does contain a valid omo.jsonc. listProfiles hides it, but importLegacyProfile('backup-2026') would import it. Add that fixture and assert the expected behaviour, then add the artifact check to importLegacyProfile if rejection is intended.
💚 Suggested test addition
const pm = loadProfileManager(home);
assert.throws(() => pm.importLegacyProfile('backup-2026'), /没有可导入的 omo 配置/, 'artifact dir rejected');
+ // An artifact-named dir holding a real omo.jsonc must also be refused,
+ // otherwise listProfiles hides a name that import still accepts.
+ writeLegacyOmoJsonc(legacyDir, 'backup-2026', { model: 'x' });
+ assert.throws(() => pm.importLegacyProfile('backup-2026'), /legacy/, 'artifact name refused even with a config');
writeLegacyConfig(legacyDir, 'empty');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/profile-manager.test.js` around lines 348 - 359, Update the
importLegacyProfile test fixture so backup-2026 contains a valid omo.jsonc
before asserting its behavior, ensuring the assertion exercises artifact-name
handling rather than the missing-config branch. Then update importLegacyProfile
to reject artifact names via isLegacyArtifactName, preserving the existing
no-import error behavior and confirming listProfiles continues to hide the
artifact directory.
1 participant
Summary
OMO (oh-my-openagent) config integration for opencode-studio: migrate profile management from legacy symlink-directory switching to the omo plugin 4.19.4
~/.omo/omo.jsoncmodel, plus a full audit-fix pass (skills/plugins/marker/i18n) and test infrastructure.Part 1 — OMO config integration (baseline)
~/.omo/omo.jsoncand its[opencode]block; jsonc-preserving block writes (writeOmoBlock) andprofiles.<name>CRUD; scope omo detection to caller roots; prioritize~/.omo/omo.jsoncin path resolutionactivateProfilebakes[opencode]then deletes the source blockPart 2 — Audit fixes (2026-08-02 audit, reviewed 4 rounds)
sanitizeOmoKeypreserves Chinese/space profile names (智谱AI,v1.0 测试round-trip); invalid names (!!!,#$%,.,..) still rejected; filesystem sanitize kept for paths/api/skills; symlink skills are read-only — POST/DELETE return 403 (package-entry lstat guard prevents rmSync symlink traversal into legacy dirs)plugin(singular) key, merged+deduped withplugins; npm plugins are display-only — POST/DELETE return 400detectConfigProvidersexcludes the server process cwd from candidates (only thegetSearchRoots()spread is filtered; omo-first ordering preserved)activeOmoProfilekey in~/.config/opencode-studio/studio.jsonpersists the active profile across restarts; legacy list excludes the consumed name; marker write failure returns explicit 500; UI shows a read-only "Already active" cardt()keys in en/zh-CN/ko (3 new keys)/api/commands{}is correct, no code changePart 3 — Tooling
getSkillDirs,loadAggregatedConfig,aggregatePlugins,getSearchRoots,detectConfigProviders)HOSTenv (default127.0.0.1); dev server exposed on0.0.0.0.omo/workspace + dev docs gitignoredVerification
node --test server/profile-manager.test.js→ 24/24node --test server/http-smoke.test.js→ 1/1npx --yes vitest@2 run lib/config-providers.test.js→ 33/33cd client-next && npx tsc --noEmit→ exit 0/api/skillsincludesocr-vision,/api/pluginslists the 3 npm plugins,/api/config-providershas no server-cwd candidate,/api/profileshasactivefield,/api/commands={}Notes
~/.config/opencode-profiles/*/) are never modified or deleted — read-only, activation imports their omo config~/.config/opencode/omo.jsonc/oh-my-openagent.jsonuntouched (read-only legacy sources)getSearchRootsnot globally changed;activeProfilesauth key untouchedGreptile Summary
The PR migrates profile management to OMO JSONC blocks and updates provider detection, skills/plugins handling, markers, localization, networking, and tests.
Confidence Score: 4/5
The PR is not safe to merge until duplicate profile creation, partial activation persistence, and unauthenticated non-loopback server exposure are addressed.
Existing profile contents can be overwritten by a duplicate create request, activation can commit despite returning an error when marker persistence fails, and a configured network bind exposes privileged routes without authentication.
Files Needing Attention: server/profile-manager.js, server/index.js
Security Review
Configuring the backend with a non-loopback
HOSTexposes unauthenticated management routes capable of reading or modifying local configuration and terminating the server. How this was verified: The configured host is passed toapp.listen, while shared middleware accepts originless requests and privileged routes have no authentication checks.Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as Profiles UI participant API as Express API participant PM as Profile Manager participant OMO as ~/.omo/omo.jsonc participant Studio as studio.json UI->>API: Activate profile API->>PM: activateProfile(name) PM->>OMO: Bake [opencode] PM->>OMO: Delete source profile PM-->>API: success API->>Studio: Persist activeOmoProfile alt marker write fails API-->>UI: HTTP 500 Note over OMO,Studio: OMO changed but marker remains stale else marker write succeeds API-->>UI: success endPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore: add npm lockfiles" | Re-trigger Greptile