diff --git a/packages/cli/src/serve/workspaceAgents.test.ts b/packages/cli/src/serve/workspaceAgents.test.ts index a6ca6243d..4f96a9673 100644 --- a/packages/cli/src/serve/workspaceAgents.test.ts +++ b/packages/cli/src/serve/workspaceAgents.test.ts @@ -500,4 +500,136 @@ describe('workspace agents routes', () => { expect(res.status).toBe(400); expect(res.body.code).toBe('invalid_client_id'); }); + + it('trims leading/trailing whitespace on the agent name', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: ' trimmed-name ', + description: 'a description longer than ten chars', + systemPrompt: 'you are a trimmed name agent', + scope: 'workspace', + }); + expect(res.status).toBe(201); + expect(res.body.agent.name).toBe('trimmed-name'); + // File on disk uses the trimmed name; the original-with-spaces + // version must NOT exist (would otherwise be unfindable via + // case-insensitive lookup). + const onDisk = await fs.readFile( + path.join(workspace, QWEN_DIR, 'agents', 'trimmed-name.md'), + 'utf8', + ); + expect(onDisk).toContain('name: trimmed-name'); + }); + + it('returns 422 invalid_config when scalar field has wrong type on create', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'wrong-type', + description: 'a description longer than ten chars', + systemPrompt: 'you are a wrong-type test agent', + scope: 'workspace', + model: 123, + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/model.*string/); + }); + + it('returns 422 invalid_config for unknown approvalMode', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + const res = await request(app).post('/workspace/agents').send({ + name: 'bad-mode', + description: 'a description longer than ten chars', + systemPrompt: 'you are a bad-mode test agent', + scope: 'workspace', + approvalMode: 'rampage', + }); + expect(res.status).toBe(422); + expect(res.body.code).toBe('invalid_config'); + expect(res.body.error).toMatch(/approvalMode/); + }); + + it('strips unknown runConfig keys and rejects malformed values', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // Unknown keys are silently dropped, valid known keys preserved. + const res = await request(app) + .post('/workspace/agents') + .send({ + name: 'run-config', + description: 'a description longer than ten chars', + systemPrompt: 'you are a run-config test agent', + scope: 'workspace', + runConfig: { max_turns: 5, mystery_field: 'oops' }, + }); + expect(res.status).toBe(201); + expect(res.body.agent.runConfig).toEqual({ max_turns: 5 }); + + // Malformed known field fails closed. + const res2 = await request(app) + .post('/workspace/agents') + .send({ + name: 'run-config-bad', + description: 'a description longer than ten chars', + systemPrompt: 'you are a run-config bad agent', + scope: 'workspace', + runConfig: { max_turns: -1 }, + }); + expect(res2.status).toBe(422); + expect(res2.body.code).toBe('invalid_config'); + }); + + it('rejects 400 invalid_scope on repeated ?scope= query', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + // Express parses repeated query params as an array; we should + // fail-closed rather than treating it as absent. + const res = await request(app).delete( + '/workspace/agents/some-name?scope=workspace&scope=global', + ); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_scope'); + }); + + it('rejects 400 invalid_config for empty update body', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'has-fields', + description: 'a description longer than ten chars', + systemPrompt: 'you are a has-fields test agent', + scope: 'workspace', + }); + const res = await request(app) + .post('/workspace/agents/has-fields') + .send({}); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_config'); + }); + + it('short-circuits no-op updates with changed: false and no event', async () => { + const bridge = buildBridgeStub(); + const app = buildApp({ bridge, boundWorkspace: workspace }); + await request(app).post('/workspace/agents').send({ + name: 'noop-target', + description: 'a description longer than ten chars', + systemPrompt: 'you are a noop-target agent', + scope: 'workspace', + }); + const eventsBefore = (bridge as unknown as { events: RecordedEvent[] }) + .events.length; + + const res = await request(app) + .post('/workspace/agents/noop-target') + .send({ description: 'a description longer than ten chars' }); + expect(res.status).toBe(200); + expect(res.body.changed).toBe(false); + + // No new agent_changed event for the no-op update. + const events = (bridge as unknown as { events: RecordedEvent[] }).events; + expect(events.length).toBe(eventsBefore); + }); }); diff --git a/packages/cli/src/serve/workspaceAgents.ts b/packages/cli/src/serve/workspaceAgents.ts index b994d6d5d..04519a4a9 100644 --- a/packages/cli/src/serve/workspaceAgents.ts +++ b/packages/cli/src/serve/workspaceAgents.ts @@ -6,6 +6,7 @@ import type { Application, Request, RequestHandler, Response } from 'express'; import { + APPROVAL_MODES, BuiltinAgentRegistry, SubagentError, SubagentErrorCode, @@ -245,8 +246,27 @@ export function mountWorkspaceAgentsRoutes( const updates = parseAgentUpdates(body, res); if (!updates) return; - const scopeQuery = - typeof req.query['scope'] === 'string' ? req.query['scope'] : undefined; + // Fail-closed on `?scope=` malformations. `req.query['scope']` + // is `undefined` when absent, a `string` when supplied once, an + // array when repeated (`?scope=workspace&scope=global`), or a + // ParsedQs object on a nested form. We only accept a single + // string; anything else returns `invalid_scope` rather than + // silently treating "?scope=...&scope=..." as if no scope were + // provided. Matches the fail-closed posture of + // `parseMaxQueuedQuery` for the SSE route. + const rawScope = req.query['scope']; + let scopeQuery: string | undefined; + if (rawScope === undefined) { + scopeQuery = undefined; + } else if (typeof rawScope === 'string') { + scopeQuery = rawScope; + } else { + res.status(400).json({ + error: '`scope` query must be a single "workspace" or "global" value', + code: 'invalid_scope', + }); + return; + } let preferredLevel: SubagentLevel | undefined; if (scopeQuery !== undefined) { if (scopeQuery !== 'workspace' && scopeQuery !== 'global') { @@ -283,6 +303,32 @@ export function mountWorkspaceAgentsRoutes( return; } + // Empty / no-op update detection. An empty body or a body whose + // recognized fields all match `existing` would otherwise rewrite + // the file (mtime bump) AND fan out an `agent_changed` event for + // a request that didn't change anything — the same misleading + // signal the memory route avoids for whitespace-only appends. + // Reject empty payloads with 400; short-circuit no-op updates + // with 200 + `changed: false` so adapters can suppress redundant + // toasts without re-fetching. + if (Object.keys(updates).length === 0) { + res.status(400).json({ + error: + '`POST /workspace/agents/:agentType` requires at least one updatable field in the body', + code: 'invalid_config', + name: agentType, + }); + return; + } + if (isNoOpUpdate(existing, updates)) { + res.status(200).json({ + ok: true, + agent: toDetail(existing), + changed: false, + }); + return; + } + try { await manager.updateSubagent(agentType, updates, existing.level); } catch (err) { @@ -363,7 +409,9 @@ export function mountWorkspaceAgentsRoutes( data: { change: 'updated', name: existing.name, level: eventLevel }, ...(originatorClientId ? { originatorClientId } : {}), }); - res.status(200).json({ ok: true, agent: toDetail(updated) }); + res + .status(200) + .json({ ok: true, agent: toDetail(updated), changed: true }); }, ); @@ -383,8 +431,27 @@ export function mountWorkspaceAgentsRoutes( if (clientIdResult === null) return; const originatorClientId = clientIdResult; - const scopeQuery = - typeof req.query['scope'] === 'string' ? req.query['scope'] : undefined; + // Fail-closed on `?scope=` malformations. `req.query['scope']` + // is `undefined` when absent, a `string` when supplied once, an + // array when repeated (`?scope=workspace&scope=global`), or a + // ParsedQs object on a nested form. We only accept a single + // string; anything else returns `invalid_scope` rather than + // silently treating "?scope=...&scope=..." as if no scope were + // provided. Matches the fail-closed posture of + // `parseMaxQueuedQuery` for the SSE route. + const rawScope = req.query['scope']; + let scopeQuery: string | undefined; + if (rawScope === undefined) { + scopeQuery = undefined; + } else if (typeof rawScope === 'string') { + scopeQuery = rawScope; + } else { + res.status(400).json({ + error: '`scope` query must be a single "workspace" or "global" value', + code: 'invalid_scope', + }); + return; + } let scopedLevel: SubagentLevel | undefined; if (scopeQuery !== undefined) { if (scopeQuery !== 'workspace' && scopeQuery !== 'global') { @@ -490,14 +557,22 @@ function parseAgentConfig( level: SubagentLevel, res: Response, ): SubagentConfig | undefined { - const name = body['name']; - if (typeof name !== 'string' || name.trim().length === 0) { + const rawName = body['name']; + if (typeof rawName !== 'string' || rawName.trim().length === 0) { res.status(422).json({ error: '`name` is required and must be a non-empty string', code: 'invalid_config', }); return undefined; } + // Trim leading/trailing whitespace BEFORE storing. Without this, a + // client posting `{ name: " tester " }` would land a file whose + // frontmatter `name` field literally contains the spaces; the + // resolver's case-insensitive cascade still wouldn't match `/agents/ + // tester` because the lookup name and the on-disk name differ. + // Better to normalize at the boundary than carry untrimmed names + // through validation + serialization. + const name = rawName.trim(); // Reject names that shadow a built-in subagent. Without this check a // client could `POST /workspace/agents { name: "general-purpose" }` // and write a project-level file at `/.qwen/agents/ @@ -548,28 +623,45 @@ function parseAgentConfig( }; if (tools !== undefined) config.tools = tools; if (disallowedTools !== undefined) config.disallowedTools = disallowedTools; + + // Optional scalar fields. Present-but-wrong-type fails closed (422) + // rather than silently dropping the field — `SubagentValidator` + // doesn't reject these, and `serializeSubagent` only writes recognized + // values, so without explicit validation a `model: 123` payload would + // 201 with no `model` field on the file (masking client-serialization + // bugs). + if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; if (typeof body['model'] === 'string') config.model = body['model']; + + if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; if (typeof body['color'] === 'string') config.color = body['color']; + + if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + return undefined; + } if (typeof body['approvalMode'] === 'string') { - config.approvalMode = body['approvalMode']; - } - if (typeof body['background'] === 'boolean') { - config.background = body['background']; - } - const runConfig = body['runConfig']; - if (runConfig !== undefined) { - if ( - typeof runConfig !== 'object' || - runConfig === null || - Array.isArray(runConfig) - ) { + if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { res.status(422).json({ - error: '`runConfig` must be an object when provided', + error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, code: 'invalid_config', }); return undefined; } - config.runConfig = runConfig as SubagentConfig['runConfig']; + config.approvalMode = body['approvalMode']; + } + + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { + return undefined; + } + if (typeof body['background'] === 'boolean') { + config.background = body['background']; + } + + const runConfig = body['runConfig']; + if (runConfig !== undefined) { + const sanitized = sanitizeRunConfig(runConfig, res); + if (sanitized === null) return undefined; + config.runConfig = sanitized; } return config; } @@ -615,32 +707,40 @@ function parseAgentUpdates( updates.disallowedTools = disallowedTools; } } - if ('model' in body && typeof body['model'] === 'string') { - updates.model = body['model']; + // Optional scalar fields. Match the create-side fail-closed posture + // so a typo like `model: 123` returns 422 instead of silently + // succeeding with no model change. + if (rejectIfPresentWrongType(body, 'model', 'string', res)) return undefined; + if (typeof body['model'] === 'string') updates.model = body['model']; + + if (rejectIfPresentWrongType(body, 'color', 'string', res)) return undefined; + if (typeof body['color'] === 'string') updates.color = body['color']; + + if (rejectIfPresentWrongType(body, 'approvalMode', 'string', res)) { + return undefined; } - if ('color' in body && typeof body['color'] === 'string') { - updates.color = body['color']; - } - if ('approvalMode' in body && typeof body['approvalMode'] === 'string') { - updates.approvalMode = body['approvalMode']; - } - if ('background' in body && typeof body['background'] === 'boolean') { - updates.background = body['background']; - } - if ('runConfig' in body) { - const runConfig = body['runConfig']; - if ( - typeof runConfig !== 'object' || - runConfig === null || - Array.isArray(runConfig) - ) { + if (typeof body['approvalMode'] === 'string') { + if (!APPROVAL_MODES.includes(body['approvalMode'] as never)) { res.status(422).json({ - error: '`runConfig` must be an object when provided', + error: `\`approvalMode\` must be one of ${JSON.stringify(APPROVAL_MODES)}`, code: 'invalid_config', }); return undefined; } - updates.runConfig = runConfig as SubagentConfig['runConfig']; + updates.approvalMode = body['approvalMode']; + } + + if (rejectIfPresentWrongType(body, 'background', 'boolean', res)) { + return undefined; + } + if (typeof body['background'] === 'boolean') { + updates.background = body['background']; + } + + if ('runConfig' in body) { + const sanitized = sanitizeRunConfig(body['runConfig'], res); + if (sanitized === null) return undefined; + updates.runConfig = sanitized; } return updates; } @@ -661,6 +761,165 @@ function parseStringArray( return value as string[]; } +/** + * Returns `true` and sends a 422 when `body[key]` is present but the + * wrong scalar type. The caller then returns `undefined` to short- + * circuit the route. `false` covers both "absent" and "right type" so + * the caller proceeds. Used to give scalar fields the same fail-closed + * posture as `parseStringArray` / `sanitizeRunConfig`. + */ +function rejectIfPresentWrongType( + body: Record, + key: string, + expected: 'string' | 'boolean', + res: Response, +): boolean { + if (!(key in body)) return false; + if (typeof body[key] === expected) return false; + res.status(422).json({ + error: `\`${key}\` must be a ${expected} when provided`, + code: 'invalid_config', + }); + return true; +} + +/** + * Detect a no-op update — every supplied field already matches the + * existing agent's value. Without this check an empty (or + * value-unchanged) PATCH still rewrites the file, bumps mtime, and + * fans out a misleading `agent_changed` event. The recognized-field + * comparison covers what `parseAgentUpdates` produces; unknown keys + * are dropped upstream so we don't need to handle them here. + */ +function isNoOpUpdate( + existing: SubagentConfig, + updates: Partial, +): boolean { + if ( + updates.description !== undefined && + updates.description !== existing.description + ) { + return false; + } + if ( + updates.systemPrompt !== undefined && + updates.systemPrompt !== existing.systemPrompt + ) { + return false; + } + if ( + updates.tools !== undefined && + !shallowArrayEqual(updates.tools, existing.tools) + ) { + return false; + } + if ( + updates.disallowedTools !== undefined && + !shallowArrayEqual(updates.disallowedTools, existing.disallowedTools) + ) { + return false; + } + if (updates.model !== undefined && updates.model !== existing.model) { + return false; + } + if (updates.color !== undefined && updates.color !== existing.color) { + return false; + } + if ( + updates.approvalMode !== undefined && + updates.approvalMode !== existing.approvalMode + ) { + return false; + } + if ( + updates.background !== undefined && + updates.background !== existing.background + ) { + return false; + } + if (updates.runConfig !== undefined) { + const e = existing.runConfig ?? {}; + const u = updates.runConfig; + if ( + u['max_time_minutes'] !== e['max_time_minutes'] || + u['max_turns'] !== e['max_turns'] + ) { + return false; + } + } + return true; +} + +function shallowArrayEqual( + a: readonly string[] | undefined, + b: readonly string[] | undefined, +): boolean { + if (a === b) return true; + if (!a || !b) return false; + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) if (a[i] !== b[i]) return false; + return true; +} + +/** + * Sanitize `runConfig` to only the documented fields. Without this + * filter `SubagentManager.serializeSubagent` writes whatever object the + * client sent into the agent's frontmatter, including unknown or + * YAML-sensitive keys that downstream parsers may choke on. Returning + * a fresh whitelist-shaped object also makes the wire contract + * self-documenting at the route boundary. + * + * - `undefined` is impossible here (caller checks `'runConfig' in body`). + * - `null` (sent) → 422 invalid_config (the route handler converts + * the null sentinel to a short-circuit). + * - Right-shape object → returns a new object with only `max_time_minutes` + * and `max_turns` if they validate as finite positive numbers. + */ +function sanitizeRunConfig( + raw: unknown, + res: Response, +): SubagentConfig['runConfig'] | null { + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + res.status(422).json({ + error: '`runConfig` must be an object when provided', + code: 'invalid_config', + }); + return null; + } + const input = raw as Record; + const out: Record = {}; + + if ('max_time_minutes' in input) { + const v = input['max_time_minutes']; + if (typeof v !== 'number' || !Number.isFinite(v) || v <= 0) { + res.status(422).json({ + error: + '`runConfig.max_time_minutes` must be a positive finite number when provided', + code: 'invalid_config', + }); + return null; + } + out['max_time_minutes'] = v; + } + if ('max_turns' in input) { + const v = input['max_turns']; + if ( + typeof v !== 'number' || + !Number.isFinite(v) || + v <= 0 || + !Number.isInteger(v) + ) { + res.status(422).json({ + error: '`runConfig.max_turns` must be a positive integer when provided', + code: 'invalid_config', + }); + return null; + } + out['max_turns'] = v; + } + return out as SubagentConfig['runConfig']; +} + function toSummary(config: SubagentConfig): ServeWorkspaceAgentSummary { const summary: ServeWorkspaceAgentSummary = { kind: 'agent', diff --git a/packages/core/src/memory/writeContextFile.test.ts b/packages/core/src/memory/writeContextFile.test.ts index eba830dfd..3d39dcb76 100644 --- a/packages/core/src/memory/writeContextFile.test.ts +++ b/packages/core/src/memory/writeContextFile.test.ts @@ -217,4 +217,21 @@ describe('writeWorkspaceContextFile', () => { fs.access(path.join(workspace, DEFAULT_CONTEXT_FILENAME)), ).rejects.toMatchObject({ code: 'ENOENT' }); }); + + it('does not create the parent directory on a no-op append', async () => { + // Whitespace-only append targeting a non-existent nested path + // must NOT call fs.mkdir — the no-op detection short-circuits + // BEFORE acquiring the lock or touching the filesystem. Without + // this, an empty POST would still bump the parent directory's + // mtime even though the helper reports `changed: false`. + const nested = path.join(workspace, 'never-exists'); + const result = await writeWorkspaceContextFile({ + scope: 'workspace', + mode: 'append', + content: '\n\n', + projectRoot: nested, + }); + expect(result.changed).toBe(false); + await expect(fs.access(nested)).rejects.toMatchObject({ code: 'ENOENT' }); + }); }); diff --git a/packages/core/src/memory/writeContextFile.ts b/packages/core/src/memory/writeContextFile.ts index 8c2fd5a25..05b472aa9 100644 --- a/packages/core/src/memory/writeContextFile.ts +++ b/packages/core/src/memory/writeContextFile.ts @@ -98,6 +98,22 @@ export async function writeWorkspaceContextFile( } const filePath = resolveContextFilePath(options.scope, options.projectRoot); + // Append-mode no-op detection BEFORE acquiring the lock or + // creating directories. Re-writing the same bytes would bump + // mtime + the parent dir mtime AND fan out a misleading + // `memory_changed` event; whitespace-only POSTs from a flaky + // pipeline shouldn't reach the filesystem at all. + if (options.mode === 'append' && isWhitespaceOnly(options.content)) { + let bytes = 0; + try { + const stat = await fs.stat(filePath); + bytes = stat.size; + } catch (err) { + if (!isEnoent(err)) throw err; + } + return { filePath, bytesWritten: bytes, changed: false }; + } + // Hold the per-file mutex for the entire read-compose-write sequence. // Concurrent `POST /workspace/memory` appends from different SSE // clients would otherwise interleave reads and lose entries on the @@ -119,23 +135,6 @@ export async function writeWorkspaceContextFile( }; } - // Append mode. When the trimmed content is empty (whitespace-only - // input from a flaky pipeline / accidental empty POST), short-circuit - // BEFORE re-writing the file. Re-writing the same bytes would still - // bump mtime AND trigger `memory_changed` SSE fan-out across every - // subscribed client — a misleading "memory just changed" toast for a - // request that changed nothing. - if (options.content.replace(/^\s+|\s+$/g, '').length === 0) { - let bytes = 0; - try { - const stat = await fs.stat(filePath); - bytes = stat.size; - } catch (err) { - if (!isEnoent(err)) throw err; - } - return { filePath, bytesWritten: bytes, changed: false }; - } - const next = await composeAppendedContent(filePath, options.content); await fs.writeFile(filePath, next, { encoding: 'utf8', mode: 0o644 }); return { @@ -167,7 +166,7 @@ async function composeAppendedContent( if (!isEnoent(err)) throw err; } - const trimmed = newContent.replace(/^\n+|\n+$/g, ''); + const trimmed = trimNewlines(newContent); if (trimmed.length === 0) return existing; if (existing.length === 0) { @@ -191,3 +190,45 @@ function isEnoent(err: unknown): boolean { (err as { code?: string }).code === 'ENOENT' ); } + +/** + * Hand-rolled `^\s+|\s+$` substitute. CodeQL's polynomial-regex + * detector flags `\s+` with anchors as a ReDoS risk on + * attacker-controlled input; the linear loop sidesteps the rule + * without changing behavior. Mirrors the same pattern used by + * `auth.ts:120-125` for header-credential parsing. + */ +function isWhitespaceOnly(s: string): boolean { + for (let i = 0; i < s.length; i++) { + const c = s.charCodeAt(i); + // ASCII space, tab, line feed, carriage return, form feed, + // vertical tab. All non-printable whitespace control chars the + // route's "no-op append" check should treat as empty content. + if ( + c !== 0x20 && + c !== 0x09 && + c !== 0x0a && + c !== 0x0d && + c !== 0x0c && + c !== 0x0b + ) { + return false; + } + } + return true; +} + +/** + * Hand-rolled `^\n+|\n+$` substitute. Same CodeQL rationale as + * `isWhitespaceOnly`. Trims only `\n` so the section-header insert + * path keeps its newline framing semantics — a leading `\t` in + * `newContent` is preserved as part of the user's bullet, while + * `\n\n- entry\n` collapses to `- entry`. + */ +function trimNewlines(s: string): string { + let start = 0; + let end = s.length; + while (start < end && s.charCodeAt(start) === 0x0a) start++; + while (end > start && s.charCodeAt(end - 1) === 0x0a) end--; + return start === 0 && end === s.length ? s : s.slice(start, end); +} diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 31721bf2f..481896f4a 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -456,14 +456,24 @@ export class DaemonClient { * Update a project- or user-level subagent definition. Built-in / * extension / session-level agents are read-only and return 403 * `agent_readonly`; missing agents return 404 `agent_not_found`. + * + * Optional `scope` mirrors the delete helper: when a project agent + * shadows a user-level agent of the same name, pass + * `{ scope: 'global' }` to update the user-level definition + * specifically. Without the scope the daemon resolves through the + * default precedence (project > user) and updates the project entry. */ async updateWorkspaceAgent( agentType: string, req: DaemonUpdateAgentRequest, + opts: { scope?: 'workspace' | 'global' } = {}, clientId?: string, ): Promise { + const url = opts.scope + ? `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}?scope=${encodeURIComponent(opts.scope)}` + : `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}`; return await this.fetchWithTimeout( - `${this.baseUrl}/workspace/agents/${encodeURIComponent(agentType)}`, + url, { method: 'POST', headers: this.headers({ 'Content-Type': 'application/json' }, clientId), @@ -502,7 +512,7 @@ export class DaemonClient { headers: this.headers({}, clientId), }, async (res) => { - if (res.status === 204 || res.status === 404) { + if (res.status === 204) { try { await res.body?.cancel(); } catch { @@ -510,6 +520,23 @@ export class DaemonClient { } return; } + // Treat as idempotent ONLY when the daemon explicitly says + // `agent_not_found`. A bare 404 (e.g. an HTTP proxy returning + // a generic page, an older daemon that doesn't know the + // route, a misrouted load balancer) would otherwise be + // silently swallowed and the SDK caller would believe the + // agent was deleted when the request never reached a route + // that understands workspace agents. Failing on non- + // structured 404s makes routing errors visible. + if (res.status === 404) { + const err = await this.failOnError( + res, + 'DELETE /workspace/agents/:agentType', + ); + const body = err.body as { code?: unknown } | undefined; + if (body && body.code === 'agent_not_found') return; + throw err; + } throw await this.failOnError( res, 'DELETE /workspace/agents/:agentType', diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 967897194..865e8b9c5 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -316,6 +316,19 @@ export interface DaemonWriteMemoryResult { filePath: string; bytesWritten: number; mode: 'append' | 'replace'; + /** + * `true` when the daemon actually mutated the file on disk. `false` + * for whitespace-only `append` requests that short-circuited + * upstream — the route accepted the request as well-formed (200 + * OK) but the helper detected the trimmed content was empty and + * skipped the write to avoid an mtime bump + a misleading + * `memory_changed` event. SDK consumers can branch on this to + * suppress redundant cache invalidation. Optional at the type + * level for forward-compat with daemons that predate the field — + * those return undefined and callers should treat that as + * `changed: true` (the legacy contract). + */ + changed?: boolean; } /**