diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e19c11..9c9a9b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # aicodeman +## 1.9.4 + +### Patch Changes + +- Fix a latent bug where a partial settings PUT silently reset live service state, and trim the `xterm-zerolag-input` README callout. + - **`PUT /api/settings` no longer resets watchers on a partial body.** The three `toggleService` calls (subagent watcher, workflow-run watcher, image watcher) read the raw request body with `??` defaults, so every key a caller omitted was treated as "apply the default". A body of just `{statusLineTelemetry:true}` would START the subagent watcher and STOP the workflow and image watchers, undoing the persisted config. They now resolve from `merged` (persisted settings + incoming), the same convention the `tmuxHistoryLimit` branch in that handler already used, so any PUT reconciles services to the effective stored state. Nothing triggered this in practice because every shipped client sends a full settings payload rebuilt from the DOM, but it was a trap for the next partial-update caller. + - **Regression test**: `test/routes/system-routes-settings-partial-put.test.ts` (4 cases) pins both directions, omitted keys preserve state and explicit keys still take effect. Verified to fail against the pre-fix handler. + - **CLAUDE.md** records the rule under "Adding Features → App setting": anything acting on a setting in that handler must resolve from `merged`, never the request body. + - **`xterm-zerolag-input` README**: removed the links line (getcodeman.com / install one-liner / star link) from the Codeman callout above the demo GIF. The callout keeps its links in the heading and body. + ## 1.9.3 ### Patch Changes diff --git a/CLAUDE.md b/CLAUDE.md index 6e094048..f5edebec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,7 +74,7 @@ When user says "COM": CI runs `npm run check:lockfile` on every push/PR, so lockfile drift fails the build even if the `version-packages` script is bypassed. -**Version**: 1.9.3 (must match `package.json`) +**Version**: 1.9.4 (must match `package.json`) ## Project Overview @@ -292,7 +292,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L - **API endpoint**: Types in `src/types/` domain file, route in `src/web/routes/*-routes.ts`. Return the `ApiResponse` envelope (`{ success: true, data }`; errors via `createErrorResponse()` with proper status code). Validate with Zod schemas in `schemas.ts`. - **SSE event**: Add to `src/web/sse-events.ts` + `SSE_EVENTS` in `constants.js`, emit via `broadcast()`, handle in `app.js` (`addListener(`) - **Session setting**: Add to `SessionState`, include in `session.toState()`, call `persistSessionState()` -- **App setting**: decide per-device vs synced first. Per-device keys go in the `displayKeys` set in settings-ui.js and must NOT be added to `SettingsUpdateSchema` (it is `.strict()`). +- **App setting**: decide per-device vs synced first. Per-device keys go in the `displayKeys` set in settings-ui.js and must NOT be added to `SettingsUpdateSchema` (it is `.strict()`). ⚠️ Anything in `PUT /api/settings` that acts on a setting (the `toggleService` watcher calls) must resolve from **`merged`** (persisted + incoming), never from the raw request body: a partial PUT omits keys it doesn't intend to change, and `body.x ?? default` turns every omission into "apply the default" and silently resets live services. Pinned by `test/routes/system-routes-settings-partial-put.test.ts`. - **Hook event**: Add to `HookEventType`, add hook in `hooks-config.ts:generateHooksConfig()`, update `HookEventSchema` - **Mobile feature**: Add to relevant singleton, guard with `MobileDetection.isMobile()`. New header buttons must stay off phones (`test/mobile-header-buttons-policy.test.ts`). - **New test**: Pick unique port (search `const PORT =`). Route tests use `app.inject()` (no port needed) — see `test/routes/_route-test-utils.ts`. diff --git a/package-lock.json b/package-lock.json index dc203f7c..ebaf4b56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "aicodeman", - "version": "1.9.3", + "version": "1.9.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "aicodeman", - "version": "1.9.3", + "version": "1.9.4", "hasInstallScript": true, "license": "MIT", "workspaces": [ @@ -12333,7 +12333,7 @@ } }, "packages/xterm-zerolag-input": { - "version": "0.1.6", + "version": "0.1.7", "license": "MIT", "devDependencies": { "jsdom": "^24.1.3", diff --git a/package.json b/package.json index 1ca86cc5..c9c3ddba 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "aicodeman", - "version": "1.9.3", + "version": "1.9.4", "description": "Mission control for AI coding agents - run 20 autonomous agents with real-time monitoring and session persistence", "type": "module", "main": "dist/index.js", diff --git a/packages/xterm-zerolag-input/CHANGELOG.md b/packages/xterm-zerolag-input/CHANGELOG.md index b272f832..892a3907 100644 --- a/packages/xterm-zerolag-input/CHANGELOG.md +++ b/packages/xterm-zerolag-input/CHANGELOG.md @@ -1,5 +1,15 @@ # xterm-zerolag-input +## 0.1.7 + +### Patch Changes + +- Fix a latent bug where a partial settings PUT silently reset live service state, and trim the `xterm-zerolag-input` README callout. + - **`PUT /api/settings` no longer resets watchers on a partial body.** The three `toggleService` calls (subagent watcher, workflow-run watcher, image watcher) read the raw request body with `??` defaults, so every key a caller omitted was treated as "apply the default". A body of just `{statusLineTelemetry:true}` would START the subagent watcher and STOP the workflow and image watchers, undoing the persisted config. They now resolve from `merged` (persisted settings + incoming), the same convention the `tmuxHistoryLimit` branch in that handler already used, so any PUT reconciles services to the effective stored state. Nothing triggered this in practice because every shipped client sends a full settings payload rebuilt from the DOM, but it was a trap for the next partial-update caller. + - **Regression test**: `test/routes/system-routes-settings-partial-put.test.ts` (4 cases) pins both directions, omitted keys preserve state and explicit keys still take effect. Verified to fail against the pre-fix handler. + - **CLAUDE.md** records the rule under "Adding Features → App setting": anything acting on a setting in that handler must resolve from `merged`, never the request body. + - **`xterm-zerolag-input` README**: removed the links line (getcodeman.com / install one-liner / star link) from the Codeman callout above the demo GIF. The callout keeps its links in the heading and body. + ## 0.1.6 ### Patch Changes diff --git a/packages/xterm-zerolag-input/README.md b/packages/xterm-zerolag-input/README.md index 80e46c3e..2e8b314a 100644 --- a/packages/xterm-zerolag-input/README.md +++ b/packages/xterm-zerolag-input/README.md @@ -19,8 +19,6 @@ > This overlay is the local echo engine of [**Codeman**](https://github.com/Ark0N/Codeman), mission control for AI coding agents: run and monitor a dozen Claude Code, Codex, OpenCode and Gemini sessions at once, watch their subagents work in live floating windows, let them run autonomously overnight, and drive all of it from your phone. > > That last part is why this library exists. The demo below is a real Codeman session on two phones. -> -> **[getcodeman.com](https://getcodeman.com)** · install with `curl -fsSL https://getcodeman.com/install | bash` · [star it on GitHub](https://github.com/Ark0N/Codeman)

Side-by-side phones typing into the same remote session: with zerolag the text appears at 0ms, without it every keystroke waits 600ms to 2.7s for the server echo diff --git a/packages/xterm-zerolag-input/package.json b/packages/xterm-zerolag-input/package.json index d23a3567..6b86fd10 100644 --- a/packages/xterm-zerolag-input/package.json +++ b/packages/xterm-zerolag-input/package.json @@ -1,6 +1,6 @@ { "name": "xterm-zerolag-input", - "version": "0.1.6", + "version": "0.1.7", "description": "Instant keystroke feedback overlay for xterm.js — eliminates perceived input latency over high-RTT connections", "type": "module", "main": "dist/index.cjs", diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index 021791fe..718f7dc1 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -692,20 +692,27 @@ export function registerSystemRoutes( await ctx.mux.setHistoryLimit(resolveTerminalHistoryConfig(merged).tmuxHistoryLimit); } + // Service toggles resolve from `merged` (existing + incoming), NEVER from the + // raw request body. A PARTIAL PUT omits keys it does not intend to change, and + // reading the body directly turned every omission into "apply the default": + // a body of just `{statusLineTelemetry:true}` would START the subagent watcher + // (`?? true`) and STOP the workflow + image watchers (`?? false`), silently + // undoing the user's persisted config. Reading `merged` makes any PUT reconcile + // services to the effective stored settings instead, which also self-heals + // drift. Same convention as the tmuxHistoryLimit block above. // Handle subagent tracking toggle dynamically - toggleService((settings.subagentTrackingEnabled as boolean) ?? true, subagentWatcher, 'Subagent watcher'); + toggleService((merged.subagentTrackingEnabled as boolean) ?? true, subagentWatcher, 'Subagent watcher'); // Handle ultracode/workflow run watcher toggle dynamically (default OFF). // Either the docked panel OR the floating windows keep the watcher running. toggleService( - ((settings.showUltracodeAgents as boolean) ?? false) || - ((settings.ultracodeFloatingWindows as boolean) ?? false), + ((merged.showUltracodeAgents as boolean) ?? false) || ((merged.ultracodeFloatingWindows as boolean) ?? false), workflowRunWatcher, 'Workflow run watcher' ); // Handle image watcher toggle dynamically - toggleService((settings.imageWatcherEnabled as boolean) ?? false, imageWatcher, 'Image watcher', () => { + toggleService((merged.imageWatcherEnabled as boolean) ?? false, imageWatcher, 'Image watcher', () => { // Re-watch all active sessions that have image watcher enabled for (const session of ctx.sessions.values()) { if (session.imageWatcherEnabled) { diff --git a/test/routes/system-routes-settings-partial-put.test.ts b/test/routes/system-routes-settings-partial-put.test.ts new file mode 100644 index 00000000..56134313 --- /dev/null +++ b/test/routes/system-routes-settings-partial-put.test.ts @@ -0,0 +1,146 @@ +/** + * @fileoverview PUT /api/settings must not reset service state on a PARTIAL body. + * + * The three service toggles (subagent watcher, workflow-run watcher, image + * watcher) used to read the RAW REQUEST BODY with `??` defaults, so any key the + * caller omitted was treated as "apply the default". A body of just + * `{statusLineTelemetry:true}` therefore STARTED the subagent watcher (`?? true`) + * and STOPPED the workflow + image watchers (`?? false`), silently undoing the + * persisted config. Nothing triggered it in practice only because every shipped + * client sends a full settings payload rebuilt from the DOM. + * + * They now resolve from `merged` (existing settings.json + incoming), so a PUT + * reconciles services to the effective stored state. These tests pin that: + * omitted keys preserve state, explicit keys still take effect. + * + * Uses app.inject() — no real HTTP ports needed. Port: N/A. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js'; +import { registerSystemRoutes } from '../../src/web/routes/system-routes.js'; + +// vi.mock factories are hoisted above module-level consts, so the stubs and the +// persisted-settings fixture have to be built inside vi.hoisted(). +const { EXISTING_SETTINGS, subagentWatcher, imageWatcher, workflowRunWatcher } = vi.hoisted(() => { + /** Watcher stub whose isRunning() reflects its persisted state. */ + const makeWatcher = (running: boolean) => { + let isOn = running; + return { + isRunning: vi.fn(() => isOn), + start: vi.fn(() => { + isOn = true; + }), + stop: vi.fn(() => { + isOn = false; + }), + getStats: vi.fn(() => ({})), + watchSession: vi.fn(), + getRecentRunSummaries: vi.fn(() => []), + // The stubs are module singletons (vi.mock needs them hoisted), so a + // start()/stop() in one test would otherwise carry into the next and make + // its "not called" assertion pass vacuously — isRunning() already matches + // the expected end state, so toggleService short-circuits. + __resetRunning: () => { + isOn = running; + }, + }; + }; + return { + // Persisted settings.json for these tests: two watchers ON, subagent tracking OFF. + EXISTING_SETTINGS: { subagentTrackingEnabled: false, imageWatcherEnabled: true, showUltracodeAgents: true }, + subagentWatcher: makeWatcher(false), + imageWatcher: makeWatcher(true), + workflowRunWatcher: makeWatcher(true), + }; +}); + +vi.mock('node:fs/promises', () => ({ + default: { + readFile: vi.fn(async () => JSON.stringify(EXISTING_SETTINGS)), + writeFile: vi.fn(async () => undefined), + }, +})); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, existsSync: vi.fn(() => true), mkdirSync: vi.fn(), readdirSync: vi.fn(() => []) }; +}); + +vi.mock('../../src/subagent-watcher.js', () => ({ subagentWatcher })); +vi.mock('../../src/image-watcher.js', () => ({ imageWatcher })); +vi.mock('../../src/workflow-run-watcher.js', () => ({ workflowRunWatcher })); + +describe('PUT /api/settings — partial body must not reset service toggles', () => { + let harness: RouteTestHarness; + + beforeEach(async () => { + harness = await createRouteTestHarness(registerSystemRoutes); + for (const w of [subagentWatcher, imageWatcher, workflowRunWatcher]) { + w.start.mockClear(); + w.stop.mockClear(); + w.__resetRunning(); // running state, not just call records — see makeWatcher + } + }); + + afterEach(async () => { + await harness.app.close(); + }); + + it('leaves all three watchers alone when the body omits their keys', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + // Action-only body: the exact shape that used to flip all three watchers. + payload: { statusLineTelemetry: true }, + }); + + expect(res.statusCode).toBe(200); + // Persisted OFF and omitted — must NOT be started by the `?? true` default. + expect(subagentWatcher.start).not.toHaveBeenCalled(); + // Persisted ON and omitted — must NOT be stopped by the `?? false` defaults. + expect(imageWatcher.stop).not.toHaveBeenCalled(); + expect(workflowRunWatcher.stop).not.toHaveBeenCalled(); + }); + + it('still starts a watcher when the body explicitly enables it', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + payload: { subagentTrackingEnabled: true }, + }); + + expect(res.statusCode).toBe(200); + expect(subagentWatcher.start).toHaveBeenCalledTimes(1); + // Unrelated watchers stay untouched. + expect(imageWatcher.stop).not.toHaveBeenCalled(); + expect(workflowRunWatcher.stop).not.toHaveBeenCalled(); + }); + + it('still stops a watcher when the body explicitly disables it', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + payload: { imageWatcherEnabled: false }, + }); + + expect(res.statusCode).toBe(200); + expect(imageWatcher.stop).toHaveBeenCalledTimes(1); + expect(subagentWatcher.start).not.toHaveBeenCalled(); + expect(workflowRunWatcher.stop).not.toHaveBeenCalled(); + }); + + it('keeps the workflow watcher running when only one of its two keys is sent', async () => { + // Either showUltracodeAgents OR ultracodeFloatingWindows keeps it alive, and + // the OR must be evaluated over merged state, not over this partial body. + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/settings', + payload: { ultracodeFloatingWindows: false }, + }); + + expect(res.statusCode).toBe(200); + // showUltracodeAgents is still true in settings.json, so it stays up. + expect(workflowRunWatcher.stop).not.toHaveBeenCalled(); + }); +});