From 60661441b1c231115d090607dd18fedb87d9f8ac Mon Sep 17 00:00:00 2001 From: Glucksberg <80581902+Glucksberg@users.noreply.github.com> Date: Sat, 24 Jan 2026 19:30:32 -0400 Subject: [PATCH] feat(gateway-tool): add config.patch action for safe partial config updates (#1624) * fix(ui): enable save button only when config has changes The save button in the Control UI config editor was not properly gating on whether actual changes were made. This adds: - `configRawOriginal` state to track the original raw config for comparison - Change detection for both form mode (via computeDiff) and raw mode - `hasChanges` check in canSave/canApply logic - Set `configFormDirty` when raw mode edits occur - Handle raw mode UI correctly (badge shows "Unsaved changes", no diff panel) Fixes #1609 Co-Authored-By: Claude Opus 4.5 * feat(gateway-tool): add config.patch action for safe partial config updates Exposes the existing config.patch server method to agents, allowing safe partial config updates that merge with existing config instead of replacing it. - Add config.patch to GATEWAY_ACTIONS in gateway tool - Add restart + sentinel logic to config.patch server method - Extend ConfigPatchParamsSchema with sessionKey, note, restartDelayMs - Add unit test for config.patch gateway tool action Closes #1617 --------- Co-authored-by: Claude Opus 4.5 --- src/agents/clawdbot-gateway-tool.test.ts | 26 +++++++++++++ src/agents/tools/gateway-tool.ts | 43 +++++++++++++++++++-- src/gateway/protocol/schema/config.ts | 3 ++ src/gateway/server-methods/config.ts | 42 +++++++++++++++++++++ ui/src/ui/app-render.ts | 6 ++- ui/src/ui/app.ts | 1 + ui/src/ui/controllers/config.test.ts | 33 ++++++++++++++++ ui/src/ui/controllers/config.ts | 2 + ui/src/ui/views/config.browser.test.ts | 1 + ui/src/ui/views/config.ts | 48 +++++++++++++----------- 10 files changed, 180 insertions(+), 25 deletions(-) diff --git a/src/agents/clawdbot-gateway-tool.test.ts b/src/agents/clawdbot-gateway-tool.test.ts index 0a283198c..b377a53ac 100644 --- a/src/agents/clawdbot-gateway-tool.test.ts +++ b/src/agents/clawdbot-gateway-tool.test.ts @@ -89,6 +89,32 @@ describe("gateway tool", () => { ); }); + it("passes config.patch through gateway call", async () => { + const { callGatewayTool } = await import("./tools/gateway.js"); + const tool = createClawdbotTools({ + agentSessionKey: "agent:main:whatsapp:dm:+15555550123", + }).find((candidate) => candidate.name === "gateway"); + expect(tool).toBeDefined(); + if (!tool) throw new Error("missing gateway tool"); + + const raw = '{\n channels: { telegram: { groups: { "*": { requireMention: false } } } }\n}\n'; + await tool.execute("call4", { + action: "config.patch", + raw, + }); + + expect(callGatewayTool).toHaveBeenCalledWith("config.get", expect.any(Object), {}); + expect(callGatewayTool).toHaveBeenCalledWith( + "config.patch", + expect.any(Object), + expect.objectContaining({ + raw: raw.trim(), + baseHash: "hash-1", + sessionKey: "agent:main:whatsapp:dm:+15555550123", + }), + ); + }); + it("passes update.run through gateway call", async () => { const { callGatewayTool } = await import("./tools/gateway.js"); const tool = createClawdbotTools({ diff --git a/src/agents/tools/gateway-tool.ts b/src/agents/tools/gateway-tool.ts index 602fe4ec1..1ede53282 100644 --- a/src/agents/tools/gateway-tool.ts +++ b/src/agents/tools/gateway-tool.ts @@ -20,6 +20,7 @@ const GATEWAY_ACTIONS = [ "config.get", "config.schema", "config.apply", + "config.patch", "update.run", ] as const; @@ -35,10 +36,10 @@ const GatewayToolSchema = Type.Object({ gatewayUrl: Type.Optional(Type.String()), gatewayToken: Type.Optional(Type.String()), timeoutMs: Type.Optional(Type.Number()), - // config.apply + // config.apply, config.patch raw: Type.Optional(Type.String()), baseHash: Type.Optional(Type.String()), - // config.apply, update.run + // config.apply, config.patch, update.run sessionKey: Type.Optional(Type.String()), note: Type.Optional(Type.String()), restartDelayMs: Type.Optional(Type.Number()), @@ -56,7 +57,7 @@ export function createGatewayTool(opts?: { label: "Gateway", name: "gateway", description: - "Restart, apply config, or update the gateway in-place (SIGUSR1). Use config.apply/update.run to write config or run updates with validation and restart.", + "Restart, apply config, or update the gateway in-place (SIGUSR1). Use config.patch for safe partial config updates (merges with existing). Use config.apply only when replacing entire config. Both trigger restart after writing.", parameters: GatewayToolSchema, execute: async (_toolCallId, args) => { const params = args as Record; @@ -195,6 +196,42 @@ export function createGatewayTool(opts?: { }); return jsonResult({ ok: true, result }); } + if (action === "config.patch") { + const raw = readStringParam(params, "raw", { required: true }); + let baseHash = readStringParam(params, "baseHash"); + if (!baseHash) { + const snapshot = await callGatewayTool("config.get", gatewayOpts, {}); + if (snapshot && typeof snapshot === "object") { + const hash = (snapshot as { hash?: unknown }).hash; + if (typeof hash === "string" && hash.trim()) { + baseHash = hash.trim(); + } else { + const rawSnapshot = (snapshot as { raw?: unknown }).raw; + if (typeof rawSnapshot === "string") { + baseHash = crypto.createHash("sha256").update(rawSnapshot).digest("hex"); + } + } + } + } + const sessionKey = + typeof params.sessionKey === "string" && params.sessionKey.trim() + ? params.sessionKey.trim() + : opts?.agentSessionKey?.trim() || undefined; + const note = + typeof params.note === "string" && params.note.trim() ? params.note.trim() : undefined; + const restartDelayMs = + typeof params.restartDelayMs === "number" && Number.isFinite(params.restartDelayMs) + ? Math.floor(params.restartDelayMs) + : undefined; + const result = await callGatewayTool("config.patch", gatewayOpts, { + raw, + baseHash, + sessionKey, + note, + restartDelayMs, + }); + return jsonResult({ ok: true, result }); + } if (action === "update.run") { const sessionKey = typeof params.sessionKey === "string" && params.sessionKey.trim() diff --git a/src/gateway/protocol/schema/config.ts b/src/gateway/protocol/schema/config.ts index 10d0a7647..beeaac5d5 100644 --- a/src/gateway/protocol/schema/config.ts +++ b/src/gateway/protocol/schema/config.ts @@ -27,6 +27,9 @@ export const ConfigPatchParamsSchema = Type.Object( { raw: NonEmptyString, baseHash: Type.Optional(NonEmptyString), + sessionKey: Type.Optional(Type.String()), + note: Type.Optional(Type.String()), + restartDelayMs: Type.Optional(Type.Integer({ minimum: 0 })), }, { additionalProperties: false }, ); diff --git a/src/gateway/server-methods/config.ts b/src/gateway/server-methods/config.ts index ae746a48c..d248228ef 100644 --- a/src/gateway/server-methods/config.ts +++ b/src/gateway/server-methods/config.ts @@ -260,12 +260,54 @@ export const configHandlers: GatewayRequestHandlers = { return; } await writeConfigFile(validated.config); + + const sessionKey = + typeof (params as { sessionKey?: unknown }).sessionKey === "string" + ? (params as { sessionKey?: string }).sessionKey?.trim() || undefined + : undefined; + const note = + typeof (params as { note?: unknown }).note === "string" + ? (params as { note?: string }).note?.trim() || undefined + : undefined; + const restartDelayMsRaw = (params as { restartDelayMs?: unknown }).restartDelayMs; + const restartDelayMs = + typeof restartDelayMsRaw === "number" && Number.isFinite(restartDelayMsRaw) + ? Math.max(0, Math.floor(restartDelayMsRaw)) + : undefined; + + const payload: RestartSentinelPayload = { + kind: "config-apply", + status: "ok", + ts: Date.now(), + sessionKey, + message: note ?? null, + doctorHint: formatDoctorNonInteractiveHint(), + stats: { + mode: "config.patch", + root: CONFIG_PATH_CLAWDBOT, + }, + }; + let sentinelPath: string | null = null; + try { + sentinelPath = await writeRestartSentinel(payload); + } catch { + sentinelPath = null; + } + const restart = scheduleGatewaySigusr1Restart({ + delayMs: restartDelayMs, + reason: "config.patch", + }); respond( true, { ok: true, path: CONFIG_PATH_CLAWDBOT, config: validated.config, + restart, + sentinel: { + path: sentinelPath, + payload, + }, }, undefined, ); diff --git a/ui/src/ui/app-render.ts b/ui/src/ui/app-render.ts index 4fa30722f..d9834d0ec 100644 --- a/ui/src/ui/app-render.ts +++ b/ui/src/ui/app-render.ts @@ -493,6 +493,7 @@ export function renderApp(state: AppViewState) { ${state.tab === "config" ? renderConfig({ raw: state.configRaw, + originalRaw: state.configRawOriginal, valid: state.configValid, issues: state.configIssues, loading: state.configLoading, @@ -509,7 +510,10 @@ export function renderApp(state: AppViewState) { searchQuery: state.configSearchQuery, activeSection: state.configActiveSection, activeSubsection: state.configActiveSubsection, - onRawChange: (next) => (state.configRaw = next), + onRawChange: (next) => { + state.configRaw = next; + state.configFormDirty = true; + }, onFormModeChange: (mode) => (state.configFormMode = mode), onFormPatch: (path, value) => updateConfigFormValue(state, path, value), onSearchChange: (query) => (state.configSearchQuery = query), diff --git a/ui/src/ui/app.ts b/ui/src/ui/app.ts index cd3537f6d..0e21d283a 100644 --- a/ui/src/ui/app.ts +++ b/ui/src/ui/app.ts @@ -154,6 +154,7 @@ export class ClawdbotApp extends LitElement { @state() configLoading = false; @state() configRaw = "{\n}\n"; + @state() configRawOriginal = ""; @state() configValid: boolean | null = null; @state() configIssues: unknown[] = []; @state() configSaving = false; diff --git a/ui/src/ui/controllers/config.test.ts b/ui/src/ui/controllers/config.test.ts index 408a7d63b..e1bbb1c92 100644 --- a/ui/src/ui/controllers/config.test.ts +++ b/ui/src/ui/controllers/config.test.ts @@ -15,6 +15,7 @@ function createState(): ConfigState { applySessionKey: "main", configLoading: false, configRaw: "", + configRawOriginal: "", configValid: null, configIssues: [], configSaving: false, @@ -26,6 +27,7 @@ function createState(): ConfigState { configSchemaLoading: false, configUiHints: {}, configForm: null, + configFormOriginal: null, configFormDirty: false, configFormMode: "form", lastError: null, @@ -63,6 +65,37 @@ describe("applyConfigSnapshot", () => { expect(state.configForm).toEqual({ gateway: { mode: "local" } }); }); + + it("sets configRawOriginal when clean for change detection", () => { + const state = createState(); + applyConfigSnapshot(state, { + config: { gateway: { mode: "local" } }, + valid: true, + issues: [], + raw: '{ "gateway": { "mode": "local" } }', + }); + + expect(state.configRawOriginal).toBe('{ "gateway": { "mode": "local" } }'); + expect(state.configFormOriginal).toEqual({ gateway: { mode: "local" } }); + }); + + it("preserves configRawOriginal when dirty", () => { + const state = createState(); + state.configFormDirty = true; + state.configRawOriginal = '{ "original": true }'; + state.configFormOriginal = { original: true }; + + applyConfigSnapshot(state, { + config: { gateway: { mode: "local" } }, + valid: true, + issues: [], + raw: '{ "gateway": { "mode": "local" } }', + }); + + // Original values should be preserved when dirty + expect(state.configRawOriginal).toBe('{ "original": true }'); + expect(state.configFormOriginal).toEqual({ original: true }); + }); }); describe("updateConfigFormValue", () => { diff --git a/ui/src/ui/controllers/config.ts b/ui/src/ui/controllers/config.ts index 71dccaedd..c66876eba 100644 --- a/ui/src/ui/controllers/config.ts +++ b/ui/src/ui/controllers/config.ts @@ -17,6 +17,7 @@ export type ConfigState = { applySessionKey: string; configLoading: boolean; configRaw: string; + configRawOriginal: string; configValid: boolean | null; configIssues: unknown[]; configSaving: boolean; @@ -98,6 +99,7 @@ export function applyConfigSnapshot(state: ConfigState, snapshot: ConfigSnapshot if (!state.configFormDirty) { state.configForm = cloneConfigObject(snapshot.config ?? {}); state.configFormOriginal = cloneConfigObject(snapshot.config ?? {}); + state.configRawOriginal = rawFromSnapshot; } } diff --git a/ui/src/ui/views/config.browser.test.ts b/ui/src/ui/views/config.browser.test.ts index 663784791..6f19312e7 100644 --- a/ui/src/ui/views/config.browser.test.ts +++ b/ui/src/ui/views/config.browser.test.ts @@ -6,6 +6,7 @@ import { renderConfig } from "./config"; describe("config view", () => { const baseProps = () => ({ raw: "{\n}\n", + originalRaw: "{\n}\n", valid: true, issues: [], loading: false, diff --git a/ui/src/ui/views/config.ts b/ui/src/ui/views/config.ts index 53b550efe..dc4453fb8 100644 --- a/ui/src/ui/views/config.ts +++ b/ui/src/ui/views/config.ts @@ -10,6 +10,7 @@ import { export type ConfigProps = { raw: string; + originalRaw: string; valid: boolean | null; issues: unknown[]; loading: boolean; @@ -187,29 +188,17 @@ export function renderConfig(props: ConfigProps) { const formUnsafe = analysis.schema ? analysis.unsupportedPaths.length > 0 : false; - const canSaveForm = - Boolean(props.formValue) && !props.loading && !formUnsafe; - const canSave = - props.connected && - !props.saving && - (props.formMode === "raw" ? true : canSaveForm); - const canApply = - props.connected && - !props.applying && - !props.updating && - (props.formMode === "raw" ? true : canSaveForm); - const canUpdate = props.connected && !props.applying && !props.updating; // Get available sections from schema const schemaProps = analysis.schema?.properties ?? {}; const availableSections = SECTIONS.filter(s => s.key in schemaProps); - + // Add any sections in schema but not in our list const knownKeys = new Set(SECTIONS.map(s => s.key)); const extraSections = Object.keys(schemaProps) .filter(k => !knownKeys.has(k)) .map(k => ({ key: k, label: k.charAt(0).toUpperCase() + k.slice(1) })); - + const allSections = [...availableSections, ...extraSections]; const activeSectionSchema = @@ -236,12 +225,29 @@ export function renderConfig(props: ConfigProps) { : isAllSubsection ? null : props.activeSubsection ?? (subsections[0]?.key ?? null); - - // Compute diff for showing changes - const diff = props.formMode === "form" + + // Compute diff for showing changes (works for both form and raw modes) + const diff = props.formMode === "form" ? computeDiff(props.originalValue, props.formValue) : []; - const hasChanges = diff.length > 0; + const hasRawChanges = props.formMode === "raw" && props.raw !== props.originalRaw; + const hasChanges = props.formMode === "form" ? diff.length > 0 : hasRawChanges; + + // Save/apply buttons require actual changes to be enabled + const canSaveForm = + Boolean(props.formValue) && !props.loading && !formUnsafe; + const canSave = + props.connected && + !props.saving && + hasChanges && + (props.formMode === "raw" ? true : canSaveForm); + const canApply = + props.connected && + !props.applying && + !props.updating && + hasChanges && + (props.formMode === "raw" ? true : canSaveForm); + const canUpdate = props.connected && !props.applying && !props.updating; return html`
@@ -319,7 +325,7 @@ export function renderConfig(props: ConfigProps) {
${hasChanges ? html` - ${diff.length} unsaved change${diff.length !== 1 ? "s" : ""} + ${props.formMode === "raw" ? "Unsaved changes" : `${diff.length} unsaved change${diff.length !== 1 ? "s" : ""}`} ` : html` No changes `} @@ -352,8 +358,8 @@ export function renderConfig(props: ConfigProps) {
- - ${hasChanges ? html` + + ${hasChanges && props.formMode === "form" ? html`
View ${diff.length} pending change${diff.length !== 1 ? "s" : ""}