fix(ui): auto-create agent config entry so agents-page edits mark form dirty by dunamismax · Pull Request #39326 · openclaw/openclaw

dunamismax · 2026-03-08T01:10:15Z

Summary

Problem: On the gateway GUI agents page, toggling any setting (model, tool profile, tool override, skill) leaves the Save button permanently grayed out. Changes are silently lost.
Why it matters: This blocks all GUI-based agent configuration for users who don't edit YAML — a core workflow. Reported on current release 2026.3.2.
What changed: Added findAgentConfigEntryIndex() and ensureAgentConfigEntry() helpers in controllers/config.ts. All seven agents-page change callbacks in app-render.ts now use these helpers instead of inline list-search + early-return guards. When the selected agent has no explicit agents.list entry (e.g. the default "main" agent inheriting global/defaults), ensureAgentConfigEntry seeds a minimal { id } entry via updateConfigFormValue, which correctly marks the form dirty and triggers a Lit re-render.
What did NOT change (scope boundary): No server-side changes. No config schema changes. No migration. The config-form dirty-state mechanism itself (updateConfigFormValue, removeConfigFormValue, @state() configFormDirty) was already correct — the bug was that callbacks never reached those functions for inherited agents.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: Cannot save agent changes #39268

User-visible / Behavior Changes

Changing any agent setting on the GUI agents page (model, tool profile, tool overrides, skills) now correctly enables the Save button.
For inherited agents without an explicit agents.list entry, the first edit auto-creates a minimal { id: "<agentId>" } entry in the config form. This entry is written to disk only when the user clicks Save.

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: macOS (platform-independent — Lit templates)
Runtime/container: Node v24, pnpm 10.23
Model/provider: N/A (UI-only)
Integration/channel: Gateway web dashboard
Relevant config: Default config with no explicit agents.list entries

Steps

Open gateway GUI, navigate to Agents page
Select the default "main" agent
Change any setting: model dropdown, tool profile preset, individual tool toggle, or skill toggle
Observe: Save button now enables (was previously stuck disabled)
Click Save, config persists, reload page, setting retained

Expected

Save button enables after any change; config persists on save.

Actual (before fix)

Save button stays permanently grayed out; changes silently lost.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

New tests added in controllers/config.test.ts:

finds explicit agent entries — verifies findAgentConfigEntryIndex locates existing entries
creates an agent override entry when editing an inherited agent — verifies ensureAgentConfigEntry seeds { id }, sets configFormDirty = true
reuses the existing agent entry instead of duplicating it — verifies no double-create when entry already exists

All checks pass:

pnpm check (format + tsgo + lint + all lint gates) ✅
pnpm build ✅
pnpm ui:build ✅
pnpm --dir ui test (targeted: config.test.ts + agents-panels-tools-skills.browser.test.ts — 16 tests) ✅

Human Verification (required)

Verified scenarios: Code-path analysis confirming that all seven change callbacks (onModelChange, onModelFallbacksChange, onToolsProfileChange, onToolsOverridesChange, onAgentSkillToggle, onAgentSkillsClear, onAgentSkillsDisableAll) now reach updateConfigFormValue/removeConfigFormValue for both inherited and explicit agents.
Edge cases checked: Agent with no agents.list at all (empty config); agent already in list (no duplicate entry created); clearing model on inherited agent (no-op when entry doesn't exist, clean removal when it does).
What I did NOT verify: Manual browser testing against a live gateway instance with real agent configs. The reporter was on Windows 11 — not tested on Windows specifically.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert the single commit.
Files/config to restore: ui/src/ui/app-render.ts, ui/src/ui/controllers/config.ts, ui/src/ui/controllers/config.test.ts
Known bad symptoms reviewers should watch for: If the auto-created { id } entry causes unexpected config serialization, the config file would gain a new agents.list array entry after saving — this is correct behavior (it is the explicit override the user intended), but reviewers should confirm it does not produce invalid config shapes.

Risks and Mitigations

Risk: Auto-creating the agent entry could produce an agents.list with a minimal { id } entry that the gateway config validator rejects.
- Mitigation: updateConfigFormValue (the existing mechanism used for all other config edits) handles form serialization and coercion. The { id } stub is the minimal valid shape. The validator is tested separately and accepts entries with only id set.
Risk: On configs that already have an explicit agents.list entry, the findAgentConfigEntryIndex lookup could mismatch.
- Mitigation: The find logic is identical to the previous inline code — same entry.id === agentId comparison. New unit test confirms correct index resolution.

…m dirty When the selected agent has no explicit entry in agents.list (e.g. the default 'main' agent inheriting global/defaults settings), every change callback in app-render.ts bailed out at the 'find agent index' step because there was no matching list entry to update. The config form was never mutated, configFormDirty was never set, and the Save button stayed permanently disabled. Add findAgentConfigEntryIndex() and ensureAgentConfigEntry() helpers in controllers/config.ts. ensureAgentConfigEntry seeds a minimal { id } entry via updateConfigFormValue when the agent is not yet in the list, which correctly marks the form dirty and triggers a Lit re-render. All seven agents-page change callbacks (onModelChange, onModelFallbacksChange, onToolsProfileChange, onToolsOverridesChange, onAgentSkillToggle, onAgentSkillsClear, onAgentSkillsDisableAll) now use these helpers instead of inline list-search + early-return guards. Fixes #39268

greptile-apps · 2026-03-08T01:15:18Z

Greptile Summary

This PR fixes a real and impactful bug: agent-page settings changes (model, tool profile, overrides, skills) were silently discarded for inherited agents because the change callbacks bailed out early when no explicit agents.list entry existed for the agent. The fix introduces two clean helpers (findAgentConfigEntryIndex, ensureAgentConfigEntry) and applies them consistently across all seven callbacks, replacing duplicated inline list-search boilerplate.

Key changes and their correctness:

findAgentConfigEntryIndex: straightforward extraction of repeated findIndex logic — correct.
ensureAgentConfigEntry: seeds a minimal { id } stub via updateConfigFormValue, which marks the form dirty and triggers a Lit re-render — correct.
Per-callback decision (ensure vs find): setting a value → ensure; clearing/removing a value → find (no-op for inherited agents). This is semantically correct across all seven callbacks.
onAgentSkillsClear vs onAgentSkillsDisableAll: intentionally asymmetric — clearing reverts to inherited (no stub created); disabling all writes an explicit [] override (stub created). This is the right design.

Two issues worth addressing:

onModelFallbacksChange can produce model: { fallbacks: [...] } without a primary key if the user sets fallbacks on an inherited agent before setting an explicit primary model in the current session. Whether this is a valid config shape depends on the gateway's Zod schema.
The optional config parameter of ensureAgentConfigEntry is functionally redundant when called with configValue from app-render.ts (they resolve to the same object), and omitting it would make the internal nextIndex calculation always authoritative against state.configForm.

Tests cover the three critical paths (find, create-stub, no-duplicate) and are well-written.

Confidence Score: 4/5

Safe to merge; core fix is correct and well-tested, with one minor edge case in onModelFallbacksChange that's unlikely in normal UI flow.
The bug fix is sound: ensureAgentConfigEntry correctly seeds a minimal { id } stub and marks the form dirty for all seven callbacks. The refactoring eliminates significant duplication cleanly. Two minor concerns prevent a 5: (1) onModelFallbacksChange could emit model: { fallbacks: [...] } without primary in the edge case where fallbacks are set before a primary model on an inherited agent, and (2) the config parameter on ensureAgentConfigEntry is redundant and could theoretically allow stale index calculations if the function were called from a context where state.configForm has already been mutated in the same render cycle. Neither issue is triggered by normal UI interactions.
ui/src/ui/app-render.ts lines 785–824 (onModelFallbacksChange) and ui/src/ui/controllers/config.ts lines 242–261 (ensureAgentConfigEntry signature)

Comments Outside Diff (1)

ui/src/ui/app-render.ts, line 785-824 (link)

model object without primary when setting fallbacks on a newly-created entry

When ensureAgentIndex(agentId) creates a new entry (inherited agent, no existing agents.list entry), configValue is a snapshot from the last render and does not yet contain the new entry. So entry is undefined, existing is undefined, and resolvePrimary() returns null.

This leads to:

const next = primary          // null
  ? { primary, fallbacks: normalized }
  : { fallbacks: normalized };  // ← { fallbacks: [...] } with no `primary`
updateConfigFormValue(state, basePath, next);

If the config validator or gateway requires model (when an object) to have a primary field, this would produce an invalid config shape. This could happen if the user opens the fallbacks editor for an inherited agent before ever setting a primary model.

The onModelChange handler correctly stores the model as a plain string (modelId), so a subsequent onModelFallbacksChange call (after a Lit re-render) would find existing = "modelId" and resolve primary correctly. But if onModelFallbacksChange fires first (e.g. a corner-case where fallbacks are set without a primary), the stub { fallbacks: [...] } could be written to disk.

Consider guarding this case:

if (normalized.length > 0 && !primary) {
  // No primary model set yet; skip writing fallbacks-only model object
  return;
}

or documenting the constraint so it's intentional.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/src/ui/app-render.ts
Line: 785-824

Comment:
**`model` object without `primary` when setting fallbacks on a newly-created entry**

When `ensureAgentIndex(agentId)` creates a new entry (inherited agent, no existing `agents.list` entry), `configValue` is a snapshot from the last render and does not yet contain the new entry. So `entry` is `undefined`, `existing` is `undefined`, and `resolvePrimary()` returns `null`.

This leads to:
```ts
const next = primary          // null
  ? { primary, fallbacks: normalized }
  : { fallbacks: normalized };  // ← { fallbacks: [...] } with no `primary`
updateConfigFormValue(state, basePath, next);
```

If the config validator or gateway requires `model` (when an object) to have a `primary` field, this would produce an invalid config shape. This could happen if the user opens the fallbacks editor for an inherited agent before ever setting a primary model.

The `onModelChange` handler correctly stores the model as a plain string (`modelId`), so a subsequent `onModelFallbacksChange` call (after a Lit re-render) would find `existing = "modelId"` and resolve `primary` correctly. But if `onModelFallbacksChange` fires first (e.g. a corner-case where fallbacks are set without a primary), the stub `{ fallbacks: [...] }` could be written to disk.

Consider guarding this case:
```ts
if (normalized.length > 0 && !primary) {
  // No primary model set yet; skip writing fallbacks-only model object
  return;
}
```
or documenting the constraint so it's intentional.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 0895102}

greptile-apps · 2026-03-08T01:15:25Z

ui/src/ui/controllers/config.ts

+export function ensureAgentConfigEntry(
+  state: ConfigState,
+  agentId: string,
+  config?: Record<string, unknown> | null,
+): number {
+  const normalizedAgentId = agentId.trim();
+  if (!normalizedAgentId) {
+    return -1;
+  }
+  const source =
+    config ?? state.configForm ?? (state.configSnapshot?.config as Record<string, unknown> | null);
+  const existingIndex = findAgentConfigEntryIndex(source, normalizedAgentId);
+  if (existingIndex >= 0) {
+    return existingIndex;
+  }
+  const list = (source as { agents?: { list?: unknown[] } } | null)?.agents?.list;
+  const nextIndex = Array.isArray(list) ? list.length : 0;
+  updateConfigFormValue(state, ["agents", "list", nextIndex, "id"], normalizedAgentId);
+  return nextIndex;
+}


config parameter vs. state.configForm precedence

source is computed as config ?? state.configForm ?? .... In app-render.ts, this is called as:

const ensureAgentIndex = (agentId) => ensureAgentConfigEntry(state, agentId, configValue);

where configValue = state.configForm ?? state.configSnapshot?.config ?? null.

Because configValue is captured at render time, it is equivalent to state.configForm (or the snapshot when the form is clean). This means the config parameter is functionally redundant — passing it doesn't change source relative to what the function would compute on its own.

The concern is that passing a render-time snapshot as config bypasses the "check state.configForm first" protection. If, hypothetically, two edits occurred synchronously without a Lit re-render between them, the second call's configValue would not reflect the entry created by the first call, and ensureAgentConfigEntry would calculate nextIndex from the stale list length (potentially duplicating the entry at index 0).

In practice this does not occur today because each user interaction fires a single callback before the next Lit render. However, to make the function more robust, consider dropping the config parameter and always resolving source from state.configForm ?? state.configSnapshot?.config internally, so the guard against duplicate creation is always authoritative:

export function ensureAgentConfigEntry( state: ConfigState, agentId: string, ): number { ... const source = state.configForm ?? (state.configSnapshot?.config as Record<string, unknown> | null); ... }

This would also align the signature with how the function is tested (called without a third argument).

Prompt To Fix With AI

This is a comment left during a code review. Path: ui/src/ui/controllers/config.ts Line: 242-261 Comment: **`config` parameter vs. `state.configForm` precedence** `source` is computed as `config ?? state.configForm ?? ...`. In `app-render.ts`, this is called as: ```ts const ensureAgentIndex = (agentId) => ensureAgentConfigEntry(state, agentId, configValue); ``` where `configValue = state.configForm ?? state.configSnapshot?.config ?? null`. Because `configValue` is captured at render time, it is equivalent to `state.configForm` (or the snapshot when the form is clean). This means the `config` parameter is functionally redundant — passing it doesn't change `source` relative to what the function would compute on its own. The concern is that passing a render-time snapshot as `config` bypasses the "check `state.configForm` first" protection. If, hypothetically, two edits occurred synchronously without a Lit re-render between them, the second call's `configValue` would not reflect the entry created by the first call, and `ensureAgentConfigEntry` would calculate `nextIndex` from the stale list length (potentially duplicating the entry at index 0). In practice this does not occur today because each user interaction fires a single callback before the next Lit render. However, to make the function more robust, consider dropping the `config` parameter and always resolving `source` from `state.configForm ?? state.configSnapshot?.config` internally, so the guard against duplicate creation is always authoritative: ```ts export function ensureAgentConfigEntry( state: ConfigState, agentId: string, ): number { ... const source = state.configForm ?? (state.configSnapshot?.config as Record<string, unknown> | null); ... } ``` This would also align the signature with how the function is tested (called without a third argument). How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08951026d7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-08T01:17:19Z

ui/src/ui/controllers/config.ts

+      entry &&
+      typeof entry === "object" &&
+      "id" in entry &&
+      (entry as { id?: string }).id === normalizedAgentId,


Match agent IDs using canonical normalization

findAgentConfigEntryIndex now treats IDs as an exact string match against agentId.trim(), but the UI receives normalized IDs from agents.list while existing config entries can still be non-canonical (for example mixed-case IDs from manual/legacy config). In that case this lookup returns -1, ensureAgentConfigEntry appends a new {id} row, and subsequent edits can be written to the duplicate entry instead of the original one. Because runtime resolution uses normalized first-match lookup (src/agents/agent-scope.ts:114), those GUI edits may be ignored after save for affected configs.

Useful? React with 👍 / 👎.

@dunamismax

Landed from contributor PR #39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

steipete · 2026-03-08T02:14:37Z

Rebased onto latest main, deep-reviewed, and landed with a follow-up fix for live form-state reads around inherited agent overrides.

What changed on top of the original PR:

tightened the Control UI callbacks to read/write the current pending config form instead of a stale snapshot
added a regression test for reusing an already-created pending agent entry
preserved clear-fallback behavior by writing a primary-only override when inherited fallbacks are cleared
updated CHANGELOG.md
ran pnpm lint, pnpm build, and pnpm test

Landed commit: 49261b0
Source commit: 0895102

Thanks @dunamismax.

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

@dunamismax

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]> (cherry picked from commit 49261b0)

@dunamismax

) * Secrets: refresh baseline for tts line drift (cherry picked from commit 0018f47) * fix: auto-create inherited agent override entries Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]> (cherry picked from commit 49261b0) * fix: preserve agents-page selection after config save Landed from contributor PR openclaw#39301 by @MumuTW. Co-authored-by: MumuTW <[email protected]> (cherry picked from commit c0a7c30) * Agents UI: compose save state from config state (cherry picked from commit 96f4f50) * Agents UI: complete config state test fixture (cherry picked from commit 0125bd9) * Secrets: refresh baseline for model provider docs (cherry picked from commit 14916fb) * docs: dedupe changelog contributor attribution (cherry picked from commit 75a44de) * build: reduce build log noise (cherry picked from commit dd8fd98) * build: prepare 2026.3.7-beta.1 release (cherry picked from commit 3596a46) * fix(ui): align control-ui device auth token signing (cherry picked from commit e0f80cf) * build: prepare 2026.3.7 release (cherry picked from commit 42a1394) --------- Co-authored-by: Vincent Koc <[email protected]> Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: dunamismax <[email protected]> Co-authored-by: MumuTW <[email protected]>

openclaw-barnacle bot added app: web-ui App: web-ui size: M labels Mar 8, 2026

greptile-apps bot reviewed Mar 8, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Mar 8, 2026

View reviewed changes

steipete mentioned this pull request Mar 8, 2026

[Bug]: Cannot save agent changes #39268

Open

steipete added a commit that referenced this pull request Mar 8, 2026

fix: auto-create inherited agent override entries

49261b0

Landed from contributor PR #39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

steipete closed this Mar 8, 2026

openperf pushed a commit to openperf/moltbot that referenced this pull request Mar 8, 2026

fix: auto-create inherited agent override entries

2a371f9

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

mcaxtr pushed a commit to mcaxtr/openclaw that referenced this pull request Mar 8, 2026

fix: auto-create inherited agent override entries

83347e1

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

Saitop pushed a commit to NomiciAI/openclaw that referenced this pull request Mar 8, 2026

fix: auto-create inherited agent override entries

8ca267f

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

GordonSH-oss pushed a commit to GordonSH-oss/openclaw that referenced this pull request Mar 9, 2026

fix: auto-create inherited agent override entries

819848d

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

jenawant pushed a commit to jenawant/openclaw that referenced this pull request Mar 10, 2026

fix: auto-create inherited agent override entries

beb3016

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

dhoman pushed a commit to dhoman/chrono-claw that referenced this pull request Mar 11, 2026

fix: auto-create inherited agent override entries

00e1c12

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

senw-developers pushed a commit to senw-developers/va-openclaw that referenced this pull request Mar 17, 2026

fix: auto-create inherited agent override entries

188d42e

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

V-Gutierrez pushed a commit to V-Gutierrez/openclaw-vendor that referenced this pull request Mar 17, 2026

fix: auto-create inherited agent override entries

81b583d

Landed from contributor PR openclaw#39326 by @dunamismax. Co-authored-by: dunamismax <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ui): auto-create agent config entry so agents-page edits mark form dirty#39326

fix(ui): auto-create agent config entry so agents-page edits mark form dirty#39326
dunamismax wants to merge 1 commit intoopenclaw:mainfrom
dunamismax:fix/agents-save-dirty-state

dunamismax commented Mar 8, 2026

Uh oh!

greptile-apps bot commented Mar 8, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps bot Mar 8, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 8, 2026

Uh oh!

steipete commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dunamismax commented Mar 8, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual (before fix)

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

Uh oh!

greptile-apps bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps bot commented Mar 8, 2026 •

edited

Loading