feat: seamless adapter switching with permission defaults#1935
feat: seamless adapter switching with permission defaults#1935apsisvictor-sys wants to merge 4 commits intopaperclipai:masterfrom
Conversation
Prevents accidentally committing agent databases, instruction files, and company configuration to the repository. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
When switching agents between claude_local, codex_local, and opencode_local adapters: - claude_local: auto-applies dangerouslySkipPermissions: true - opencode_local: injects OPENCODE_PERMISSION env var (all-allow) - Expands CROSS_ADAPTER_CONFIG_KEYS to preserve promptTemplate, bootstrapPromptTemplate, extraArgs, timeoutSec, graceSec across adapter switches - UI edit mode applies permission bypass defaults on adapter change Enables one-click adapter switching from the dashboard without manual permission or config reconfiguration. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- claude-local: session cwd validation warning before resume - codex-local: idle heartbeat session optimization - heartbeat: log finalizeAgentStatus errors instead of swallowing; break queue recursion with setImmediate - shared/constants: add gemini_local to AGENT_ADAPTER_TYPES Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Greptile SummaryThis PR makes adapter switching a one-click operation by auto-applying permission defaults per adapter type, preserving shared config fields across switches, and keeping the UI form state in sync with server behaviour. It also bundles several orthogonal reliability fixes (heartbeat queue recursion, silent error swallowing, orphan temp dirs, idle session growth). Key changes:
Confidence Score: 4/5Safe to merge after addressing the API semantics change documentation and the missing thinking path; all remaining findings are P2 or lower. All 574 tests pass and TypeScript compiles cleanly. The core adapter-switch logic is well-tested. The main concerns are P2: a silent replace→merge semantics change to the PATCH adapterConfig field (breaking for direct API consumers, correct for a PATCH endpoint), a gap where adapter-only switches without adapterConfig don't strip old fields, an undocumented openclaw_gateway enablement, and a missing PR thinking path per CONTRIBUTING.md. server/src/routes/agents.ts — review the replace→merge semantics change and the adapter-type-only patch gap; ui/src/components/AgentConfigForm.tsx — confirm the openclaw_gateway addition is intentional. Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1758-1766
Comment:
**`adapterConfig` PATCH semantics silently changed from replace to merge**
Previously, sending `PATCH /agents/:id { "adapterConfig": { "model": "x" } }` would **replace** the entire stored `adapterConfig` with the provided object. This PR changes that to a **merge** — existing fields that aren't in the patch body are now preserved automatically.
This is arguably more correct for a PATCH endpoint (RFC 7386), but it is a **breaking change** for any existing API consumer or script that relied on the old replace-semantics to clear stale fields. For example, a caller that used to send a minimal `adapterConfig` to explicitly reset the config to a known state will now silently accumulate all previous fields.
Consider documenting this behaviour change in `docs/api/agents.md` and the PR description so downstream consumers are aware.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1745-1766
Comment:
**Adapter-specific fields not stripped when only `adapterType` is patched**
When a client sends `PATCH /agents/:id { "adapterType": "claude_local" }` **without** an `adapterConfig` key, the `if (hasOwnProperty(patchData, "adapterConfig"))` block is skipped entirely. `preserveCrossAdapterConfig` never runs, and `applyCreateDefaultsByAdapterType` receives the **full existing adapter config** — including all adapter-specific fields from the old adapter (e.g. `chrome`, `command`, `url`).
This leaves stale, adapter-incompatible fields in the stored config whenever a caller switches adapter type via the API without supplying a new `adapterConfig`.
Consider adding a guard for the adapter-type-change-only path, e.g.:
```ts
if (adapterTypeChanged && !Object.prototype.hasOwnProperty.call(patchData, "adapterConfig")) {
const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {};
patchData.adapterConfig = preserveCrossAdapterConfig(existingAdapterConfig);
}
```
The UI always sends `adapterConfig`, so this doesn't affect the current frontend, but it makes the API contract fragile for direct callers.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/components/AgentConfigForm.tsx
Line: 599-606
Comment:
**`OPENCODE_PERMISSION` env var not injected in the UI overlay on adapter switch**
The server's `applyCreateDefaultsByAdapterType` injects `OPENCODE_PERMISSION` into the agent's `env` map when switching to `opencode_local`. The UI overlay for the edit-mode adapter switch (lines 574–607) correctly sets `dangerouslySkipPermissions` for `claude_local` and `dangerouslyBypassApprovalsAndSandbox` for `codex_local`, but has no equivalent injection for `opencode_local`.
As a result, the env section of the form won't show `OPENCODE_PERMISSION` immediately after the user switches to `opencode_local` — it only appears after the form is saved and re-fetched from the server. This is a visual inconsistency rather than a correctness bug (the server applies the default on save), but it does mean the form shows an incomplete state while the user is still configuring the agent.
Consider adding the same injection in the UI overlay:
```ts
...(t === "claude_local"
? { dangerouslySkipPermissions: true }
: t === "codex_local"
? { dangerouslyBypassApprovalsAndSandbox: DEFAULT_CODEX_LOCAL_BYPASS_APPROVALS_AND_SANDBOX }
: t === "opencode_local"
? { env: { ...preserved.env as object, OPENCODE_PERMISSION: OPENCODE_PERMISSION_ALLOW_ALL } }
: {}),
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/components/AgentConfigForm.tsx
Line: 992
Comment:
**`openclaw_gateway` added to `ENABLED_ADAPTER_TYPES` without mention in PR description**
`openclaw_gateway` was quietly added to the set of enabled adapter types in the UI. This means users will now see it as a selectable option in the adapter dropdown. Is this intentional as part of this PR? The PR description doesn't mention it, and it may warrant its own entry in the "additional fixes" section along with a brief note on what it enables.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: adapter session validation, heartbe..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| patchData.adapterConfig = adapterTypeChanged | ||
| ? { | ||
| ...preserveCrossAdapterConfig(existingAdapterConfig), | ||
| ...adapterConfig, | ||
| } | ||
| : { | ||
| ...existingAdapterConfig, | ||
| ...adapterConfig, | ||
| }; |
There was a problem hiding this comment.
adapterConfig PATCH semantics silently changed from replace to merge
Previously, sending PATCH /agents/:id { "adapterConfig": { "model": "x" } } would replace the entire stored adapterConfig with the provided object. This PR changes that to a merge — existing fields that aren't in the patch body are now preserved automatically.
This is arguably more correct for a PATCH endpoint (RFC 7386), but it is a breaking change for any existing API consumer or script that relied on the old replace-semantics to clear stale fields. For example, a caller that used to send a minimal adapterConfig to explicitly reset the config to a known state will now silently accumulate all previous fields.
Consider documenting this behaviour change in docs/api/agents.md and the PR description so downstream consumers are aware.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1758-1766
Comment:
**`adapterConfig` PATCH semantics silently changed from replace to merge**
Previously, sending `PATCH /agents/:id { "adapterConfig": { "model": "x" } }` would **replace** the entire stored `adapterConfig` with the provided object. This PR changes that to a **merge** — existing fields that aren't in the patch body are now preserved automatically.
This is arguably more correct for a PATCH endpoint (RFC 7386), but it is a **breaking change** for any existing API consumer or script that relied on the old replace-semantics to clear stale fields. For example, a caller that used to send a minimal `adapterConfig` to explicitly reset the config to a known state will now silently accumulate all previous fields.
Consider documenting this behaviour change in `docs/api/agents.md` and the PR description so downstream consumers are aware.
How can I resolve this? If you propose a fix, please make it concise.| @@ -1701,11 +1754,30 @@ export function agentRoutes(db: Db) { | |||
| if (changingInstructionsPath) { | |||
| await assertCanManageInstructionsPath(req, existing); | |||
| } | |||
| patchData.adapterConfig = adapterConfig; | |||
| const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {}; | |||
| patchData.adapterConfig = adapterTypeChanged | |||
| ? { | |||
| ...preserveCrossAdapterConfig(existingAdapterConfig), | |||
| ...adapterConfig, | |||
| } | |||
| : { | |||
| ...existingAdapterConfig, | |||
| ...adapterConfig, | |||
| }; | |||
There was a problem hiding this comment.
adapterType is patched
When a client sends PATCH /agents/:id { "adapterType": "claude_local" } without an adapterConfig key, the if (hasOwnProperty(patchData, "adapterConfig")) block is skipped entirely. preserveCrossAdapterConfig never runs, and applyCreateDefaultsByAdapterType receives the full existing adapter config — including all adapter-specific fields from the old adapter (e.g. chrome, command, url).
This leaves stale, adapter-incompatible fields in the stored config whenever a caller switches adapter type via the API without supplying a new adapterConfig.
Consider adding a guard for the adapter-type-change-only path, e.g.:
if (adapterTypeChanged && !Object.prototype.hasOwnProperty.call(patchData, "adapterConfig")) {
const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {};
patchData.adapterConfig = preserveCrossAdapterConfig(existingAdapterConfig);
}The UI always sends adapterConfig, so this doesn't affect the current frontend, but it makes the API contract fragile for direct callers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1745-1766
Comment:
**Adapter-specific fields not stripped when only `adapterType` is patched**
When a client sends `PATCH /agents/:id { "adapterType": "claude_local" }` **without** an `adapterConfig` key, the `if (hasOwnProperty(patchData, "adapterConfig"))` block is skipped entirely. `preserveCrossAdapterConfig` never runs, and `applyCreateDefaultsByAdapterType` receives the **full existing adapter config** — including all adapter-specific fields from the old adapter (e.g. `chrome`, `command`, `url`).
This leaves stale, adapter-incompatible fields in the stored config whenever a caller switches adapter type via the API without supplying a new `adapterConfig`.
Consider adding a guard for the adapter-type-change-only path, e.g.:
```ts
if (adapterTypeChanged && !Object.prototype.hasOwnProperty.call(patchData, "adapterConfig")) {
const existingAdapterConfig = asRecord(existing.adapterConfig) ?? {};
patchData.adapterConfig = preserveCrossAdapterConfig(existingAdapterConfig);
}
```
The UI always sends `adapterConfig`, so this doesn't affect the current frontend, but it makes the API contract fragile for direct callers.
How can I resolve this? If you propose a fix, please make it concise.| ...(t === "claude_local" | ||
| ? { dangerouslySkipPermissions: true } | ||
| : t === "codex_local" | ||
| ? { | ||
| dangerouslyBypassApprovalsAndSandbox: | ||
| DEFAULT_CODEX_LOCAL_BYPASS_APPROVALS_AND_SANDBOX, | ||
| } | ||
| : {}), |
There was a problem hiding this comment.
OPENCODE_PERMISSION env var not injected in the UI overlay on adapter switch
The server's applyCreateDefaultsByAdapterType injects OPENCODE_PERMISSION into the agent's env map when switching to opencode_local. The UI overlay for the edit-mode adapter switch (lines 574–607) correctly sets dangerouslySkipPermissions for claude_local and dangerouslyBypassApprovalsAndSandbox for codex_local, but has no equivalent injection for opencode_local.
As a result, the env section of the form won't show OPENCODE_PERMISSION immediately after the user switches to opencode_local — it only appears after the form is saved and re-fetched from the server. This is a visual inconsistency rather than a correctness bug (the server applies the default on save), but it does mean the form shows an incomplete state while the user is still configuring the agent.
Consider adding the same injection in the UI overlay:
...(t === "claude_local"
? { dangerouslySkipPermissions: true }
: t === "codex_local"
? { dangerouslyBypassApprovalsAndSandbox: DEFAULT_CODEX_LOCAL_BYPASS_APPROVALS_AND_SANDBOX }
: t === "opencode_local"
? { env: { ...preserved.env as object, OPENCODE_PERMISSION: OPENCODE_PERMISSION_ALLOW_ALL } }
: {}),Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/components/AgentConfigForm.tsx
Line: 599-606
Comment:
**`OPENCODE_PERMISSION` env var not injected in the UI overlay on adapter switch**
The server's `applyCreateDefaultsByAdapterType` injects `OPENCODE_PERMISSION` into the agent's `env` map when switching to `opencode_local`. The UI overlay for the edit-mode adapter switch (lines 574–607) correctly sets `dangerouslySkipPermissions` for `claude_local` and `dangerouslyBypassApprovalsAndSandbox` for `codex_local`, but has no equivalent injection for `opencode_local`.
As a result, the env section of the form won't show `OPENCODE_PERMISSION` immediately after the user switches to `opencode_local` — it only appears after the form is saved and re-fetched from the server. This is a visual inconsistency rather than a correctness bug (the server applies the default on save), but it does mean the form shows an incomplete state while the user is still configuring the agent.
Consider adding the same injection in the UI overlay:
```ts
...(t === "claude_local"
? { dangerouslySkipPermissions: true }
: t === "codex_local"
? { dangerouslyBypassApprovalsAndSandbox: DEFAULT_CODEX_LOCAL_BYPASS_APPROVALS_AND_SANDBOX }
: t === "opencode_local"
? { env: { ...preserved.env as object, OPENCODE_PERMISSION: OPENCODE_PERMISSION_ALLOW_ALL } }
: {}),
```
How can I resolve this? If you propose a fix, please make it concise.| </div> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
openclaw_gateway added to ENABLED_ADAPTER_TYPES without mention in PR description
openclaw_gateway was quietly added to the set of enabled adapter types in the UI. This means users will now see it as a selectable option in the adapter dropdown. Is this intentional as part of this PR? The PR description doesn't mention it, and it may warrant its own entry in the "additional fixes" section along with a brief note on what it enables.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/components/AgentConfigForm.tsx
Line: 992
Comment:
**`openclaw_gateway` added to `ENABLED_ADAPTER_TYPES` without mention in PR description**
`openclaw_gateway` was quietly added to the set of enabled adapter types in the UI. This means users will now see it as a selectable option in the adapter dropdown. Is this intentional as part of this PR? The PR description doesn't mention it, and it may warrant its own entry in the "additional fixes" section along with a brief note on what it enables.
How can I resolve this? If you propose a fix, please make it concise.…, revert openclaw_gateway - Handle edge case where PATCH sends only adapterType without adapterConfig: now strips adapter-specific fields via preserveCrossAdapterConfig - Remove openclaw_gateway from ENABLED_ADAPTER_TYPES (unintentional inclusion) - Add test for adapter-type-only switch path Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 participant
Summary
Switching agents between
claude_local,codex_local, andopencode_localadapters currently requires manually reconfiguring permission bypass flags, model settings, and other adapter-specific fields every time. This PR makes adapter switching a one-click operation from the dashboard by:Auto-applying permission bypass defaults per adapter type — When an agent is switched to a new adapter, the server automatically applies the correct autonomous-mode configuration:
claude_local→dangerouslySkipPermissions: truecodex_local→dangerouslyBypassApprovalsAndSandbox: true(already existed)opencode_local→ injectsOPENCODE_PERMISSIONenv var with all permissions set to"allow"(OpenCode has no CLI flag equivalent)Preserving cross-adapter config on switch — Expands
CROSS_ADAPTER_CONFIG_KEYSto also preservepromptTemplate,bootstrapPromptTemplate,extraArgs,timeoutSec, andgraceSecacross adapter switches. Previously these were silently dropped.UI-side defaults on adapter change — The
AgentConfigFormnow applies permission bypass defaults when switching adapters in edit mode, matching what the server does so the form state is immediately correct.Additional fixes in this PR
startNextQueuedRunForAgentis now called viasetImmediateto break the call stack when many runs are queued for the same agent, preventing potential stack overflow.finalizeAgentStatuserrors in the outer catch are now logged instead of silently swallowed, making it easier to diagnose agents stuck in "running" state.fs.rmfor the ephemeral skills directory is nowawaited to prevent orphan temp directories.AGENT_ADAPTER_TYPESenum (the adapter package existed but the type was missing from the constant)..paperclip-home/to prevent accidental commit of instance databases, agent configs, and company data.How it works
When
PATCH /agents/:iddetects an adapter type change:The
OPENCODE_PERMISSIONenv var injection respects existing values — if a user has already set a custom permission policy, it won't be overwritten.Test plan
promptTemplate,extraArgs,timeoutSec,graceSecclaude_localauto-appliesdangerouslySkipPermissions: trueopencode_localinjectsOPENCODE_PERMISSIONenv varOPENCODE_PERMISSIONenv var is not overriddentsc --noEmit🤖 Generated with Claude Code