feat(updater): add update install mode preference (#4467)

apps/daemon/src/app-config.ts

@@ -122,6 +122,10 @@ export interface AppConfigPrefs {
   // `metadata.linkedDirs` (read-only `--add-dir` awareness, no Design Files
   // import). Stored most-recent-first; capped at RECENT_LINKED_DIRS_MAX.
   recentLinkedDirs?: string[];
+  // Controls whether the updater picks the in-app payload path or the
+  // traditional installer/DMG path. Absent / unset ⇒ treated as 'automatic'
+  // (preserves the #4471 default; backward-safe, no migration needed).
+  updateInstallMode?: 'automatic' | 'manual';
 }
 
 // Cap on how many recent working directories we remember. Keeps the picker's
@@ -146,6 +150,7 @@ const ALLOWED_KEYS: ReadonlySet<keyof AppConfigPrefs> = new Set([
   'projectLocations',
   'defaultProjectLocationId',
   'recentLinkedDirs',
+  'updateInstallMode',
 ] as const);
 
 function configFile(dataDir: string): string {
@@ -587,6 +592,14 @@ function applyConfigValue(
     }
     return;
   }
+  if (key === 'updateInstallMode') {
+    if (value === 'automatic' || value === 'manual') {
+      target[key] = value;
+    } else {
+      delete target[key];
+    }
+    return;
+  }
 }
 
 function filterAllowedKeys(obj: Record<string, unknown>): AppConfigPrefs {

apps/daemon/src/cli.ts

@@ -6930,8 +6930,9 @@ Common options:
         process.exit(2);
       }
       const cfg = await fetchConfig();
-      const next = { ...cfg };
-      delete next[key];
+      // Send explicit null so the daemon's merge-patch handler clears the key;
+      // omitting the key (delete) leaves the old value intact on the server.
+      const next = { ...cfg, [key]: null };
       const written = await writeConfig(next);
       if (flags.json) {
         process.stdout.write(JSON.stringify(written, null, 2) + '\n');

apps/daemon/tests/app-config-update-install-mode.test.ts

@@ -0,0 +1,277 @@
+// Tests for updateInstallMode pref — part of #4467 (PR1).
+//
+// Spec: apps/daemon/src/app-config.ts gains:
+//   - `updateInstallMode?: 'automatic' | 'manual'` on AppConfigPrefs
+//   - key added to ALLOWED_KEYS
+//   - applyConfigValue validates the enum (accept 'automatic'/'manual'; reject anything else)
+//   - absent/unset treated as 'automatic' (no migration needed)
+//
+// These tests are RED until the implementation lands.
+//
+// Additional tests added for #4467 (PR2) — CLI unset regression:
+//   - Daemon-level null-clear guard (non-regression; likely green already)
+//   - CLI `od config unset updateInstallMode` must send { updateInstallMode: null }
+//     in the PUT body so the merge-style writeAppConfig actually clears the value.
+//     The current code (delete next[key]) omits the key, leaving 'manual' stored.
+
+import http from 'node:http';
+import { spawn } from 'node:child_process';
+import { mkdtemp, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import path, { dirname, resolve as pathResolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
+
+import { readAppConfig, writeAppConfig } from '../src/app-config.js';
+
+// ---------------------------------------------------------------------------
+// Helpers shared by the CLI stub tests
+// ---------------------------------------------------------------------------
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirnameLocal = dirname(__filename);
+const REPO_ROOT = pathResolve(__dirnameLocal, '../../..');
+const CLI_SRC = pathResolve(__dirnameLocal, '../src/cli.ts');
+const TSX_CLI = pathResolve(REPO_ROOT, 'node_modules/tsx/dist/cli.mjs');
+
+interface CapturedRequest {
+  method: string;
+  url: string;
+  body: string;
+}
+
+interface StubServer {
+  baseUrl: string;
+  requests: CapturedRequest[];
+  /** Replace the response for the next matching call. */
+  setNextGetResponse: (body: unknown) => void;
+  close: () => Promise<void>;
+}
+
+/** Minimal HTTP stub that records every request body. The GET /api/app-config
+ *  endpoint returns the most-recently set mock response (default: empty config).
+ *  All other methods return 200 with an empty config. */
+async function startConfigStubServer(): Promise<StubServer> {
+  const requests: CapturedRequest[] = [];
+  let nextGetBody: unknown = { config: {} };
+
+  const server = http.createServer((req, res) => {
+    let raw = '';
+    req.on('data', (chunk: Buffer) => {
+      raw += chunk.toString();
+    });
+    req.on('end', () => {
+      requests.push({ method: req.method ?? '', url: req.url ?? '', body: raw });
+      res.statusCode = 200;
+      res.setHeader('content-type', 'application/json');
+      if (req.method === 'GET') {
+        res.end(JSON.stringify(nextGetBody));
+      } else {
+        // PUT — echo back the written config so the CLI can log it.
+        let written: unknown = {};
+        try { written = JSON.parse(raw); } catch { /* ignore */ }
+        res.end(JSON.stringify({ config: written }));
+      }
+    });
+  });
+
+  await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
+  const addr = server.address();
+  if (!addr || typeof addr === 'string') throw new Error('stub server has no address');
+
+  return {
+    baseUrl: `http://127.0.0.1:${(addr as { port: number }).port}`,
+    requests,
+    setNextGetResponse: (body) => { nextGetBody = body; },
+    close: () =>
+      new Promise<void>((resolve, reject) => {
+        server.close((err) => (err ? reject(err) : resolve()));
+      }),
+  };
+}
+
+function runCli(
+  args: string[],
+  daemonUrl: string,
+): Promise<{ stdout: string; stderr: string; code: number | null }> {
+  return new Promise((resolve) => {
+    const env: NodeJS.ProcessEnv = { ...process.env, OD_DAEMON_URL: daemonUrl };
+    delete env.NODE_OPTIONS;
+    const child = spawn(process.execPath, [TSX_CLI, CLI_SRC, ...args], {
+      cwd: pathResolve(__dirnameLocal, '..'),
+      env,
+      stdio: ['pipe', 'pipe', 'pipe'],
+      timeout: 20_000,
+    });
+    let stdout = '';
+    let stderr = '';
+    child.stdout.on('data', (c: Buffer) => { stdout += c.toString(); });
+    child.stderr.on('data', (c: Buffer) => { stderr += c.toString(); });
+    child.on('close', (code) => resolve({ stdout, stderr, code }));
+    child.stdin.end();
+  });
+}
+
+describe('app-config updateInstallMode pref', () => {
+  let dataDir: string;
+
+  beforeEach(async () => {
+    dataDir = await mkdtemp(path.join(tmpdir(), 'od-update-install-mode-'));
+  });
+
+  afterEach(async () => {
+    await rm(dataDir, { recursive: true, force: true });
+  });
+
+  it("persists 'manual' and reads it back", async () => {
+    await writeAppConfig(dataDir, { updateInstallMode: 'manual' });
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBe('manual');
+  });
+
+  it("persists 'automatic' and reads it back", async () => {
+    await writeAppConfig(dataDir, { updateInstallMode: 'automatic' });
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBe('automatic');
+  });
+
+  it('rejects invalid enum values and drops them', async () => {
+    // 'bogus' is not in the allowed enum; writeAppConfig must drop it.
+    await writeAppConfig(dataDir, { updateInstallMode: 'bogus' as any });
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBeUndefined();
+  });
+
+  it('treats absent field as automatic (no stored value, no crash)', async () => {
+    // Fresh config with no updateInstallMode — should be absent (undefined),
+    // which callers treat as 'automatic'.
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBeUndefined();
+  });
+
+  it("updateInstallMode is included in ALLOWED_KEYS (round-trip without unknown-key filter dropping it)", async () => {
+    // ALLOWED_KEYS gate: only keys in the set survive writeAppConfig.
+    // If updateInstallMode is missing from ALLOWED_KEYS it is silently
+    // dropped — this test catches that regression.
+    await writeAppConfig(dataDir, { updateInstallMode: 'manual', agentId: 'claude' });
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBe('manual');
+    expect(cfg.agentId).toBe('claude');
+  });
+
+  it("clears updateInstallMode when null is sent", async () => {
+    await writeAppConfig(dataDir, { updateInstallMode: 'manual' });
+    await writeAppConfig(dataDir, { updateInstallMode: null as any });
+    const cfg = await readAppConfig(dataDir);
+    expect(cfg.updateInstallMode).toBeUndefined();
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Daemon-level null-clear contract (non-regression guard for the fix)
+//
+// writeAppConfig(dir, { updateInstallMode: null }) must clear a previously-
+// stored 'manual' value.  applyConfigValue already handles null → delete, so
+// this should be GREEN on both the current code and after the fix.  It exists
+// to document and protect the daemon-side contract the CLI fix relies on.
+// ---------------------------------------------------------------------------
+describe('app-config updateInstallMode null-clear (daemon-level guard)', () => {
+  let dataDir: string;
+
+  beforeEach(async () => {
+    dataDir = await mkdtemp(path.join(tmpdir(), 'od-uim-null-guard-'));
+  });
+
+  afterEach(async () => {
+    await rm(dataDir, { recursive: true, force: true });
+  });
+
+  it('clears a previously stored manual value when null is written via writeAppConfig', async () => {
+    // Set 'manual' first.
+    await writeAppConfig(dataDir, { updateInstallMode: 'manual' });
+    const before = await readAppConfig(dataDir);
+    expect(before.updateInstallMode).toBe('manual');
+
+    // Send null — applyConfigValue must delete the key.
+    await writeAppConfig(dataDir, { updateInstallMode: null as any });
+    const after = await readAppConfig(dataDir);
+    expect(after.updateInstallMode).toBeUndefined();
+  });
+});
+
+// ---------------------------------------------------------------------------
+// CLI `od config unset updateInstallMode` regression test (RED)
+//
+// The bug: the current unset path does `delete next[key]` then sends the
+// object WITHOUT the key in the PUT body.  writeAppConfig's doWrite only
+// iterates keys that are present in the partial — an absent key is a no-op,
+// so 'manual' survives the unset.
+//
+// The fix contract: unset must send { updateInstallMode: null } so doWrite
+// calls applyConfigValue(…, null) which deletes the stored value.
+//
+// We intercept at the HTTP layer (stub server) because:
+//   1. No live daemon required — fast, no port conflicts.
+//   2. The bug lives entirely in what body the CLI sends; the stub captures it.
+//   3. This is the same approach used by cli-files-write.test.ts.
+// ---------------------------------------------------------------------------
+describe('od config unset updateInstallMode — CLI sends null in PUT body (regression)', () => {
+  let stub: StubServer;
+
+  beforeAll(async () => {
+    stub = await startConfigStubServer();
+  });
+
+  afterAll(async () => {
+    await stub.close();
+  });
+
+  beforeEach(() => {
+    stub.requests.length = 0;
+  });
+
+  it('PUT body includes { updateInstallMode: null } so the daemon can clear it', async () => {
+    // Seed the stub so the CLI's GET /api/app-config returns { updateInstallMode: 'manual' }.
+    stub.setNextGetResponse({ config: { updateInstallMode: 'manual' } });
+
+    const result = await runCli(
+      ['config', 'unset', 'updateInstallMode', '--daemon-url', stub.baseUrl],
+      stub.baseUrl,
+    );
+
+    // The CLI must exit cleanly.
+    expect(result.code).toBe(0);
+
+    // Find the PUT request.
+    const putRequest = stub.requests.find((r) => r.method === 'PUT');
+    expect(putRequest).toBeDefined();
+
+    const putBody = JSON.parse(putRequest!.body) as Record<string, unknown>;
+
+    // The key MUST be present in the PUT body with value null.
+    // With the current buggy code this assertion fails because the key is
+    // absent entirely (delete next[key] removes it before JSON.stringify).
+    expect(Object.prototype.hasOwnProperty.call(putBody, 'updateInstallMode')).toBe(true);
+    expect(putBody['updateInstallMode']).toBeNull();
+  });
+
+  it('PUT body does not contain a non-null updateInstallMode after unset', async () => {
+    // Belt-and-braces: even if the key is present it must not be 'manual'.
+    stub.setNextGetResponse({ config: { updateInstallMode: 'manual' } });
+
+    const result = await runCli(
+      ['config', 'unset', 'updateInstallMode', '--daemon-url', stub.baseUrl],
+      stub.baseUrl,
+    );
+
+    expect(result.code).toBe(0);
+
+    const putRequest = stub.requests.find((r) => r.method === 'PUT');
+    expect(putRequest).toBeDefined();
+    const putBody = JSON.parse(putRequest!.body) as Record<string, unknown>;
+
+    // The value must not be 'manual' — either null (correct) or absent (buggy).
+    // Combined with the previous test, both must pass for a complete fix.
+    expect(putBody['updateInstallMode']).not.toBe('manual');
+  });
+});

apps/desktop/src/main/index.ts

@@ -303,6 +303,30 @@ async function readAppConfigFromDaemon(baseUrl: string): Promise<DesktopAppConfi
   return payload.config;
 }
 
+/**
+ * Reads the `updateInstallMode` preference from the daemon's app-config
+ * endpoint. Returns `'automatic'` or `'manual'` if the value is present and
+ * valid; resolves to `undefined` on any failure (network error, non-OK status,
+ * missing or unrecognised field) so the updater always has a safe fallback.
+ *
+ * The optional `fetchImpl` parameter is provided for test injection.
+ */
+export async function readUpdateInstallMode(
+  baseUrl: string,
+  fetchImpl: typeof globalThis.fetch = globalThis.fetch,
+): Promise<'automatic' | 'manual' | undefined> {
+  try {
+    const response = await fetchImpl(appConfigUrl(baseUrl));
+    if (!response.ok) return undefined;
+    const payload = await response.json() as { config?: { updateInstallMode?: unknown } };
+    const mode = payload?.config?.updateInstallMode;
+    if (mode === 'automatic' || mode === 'manual') return mode;
+    return undefined;
+  } catch {
+    return undefined;
+  }
+}
+
 async function writeAppConfigToDaemon(
   baseUrl: string,
   config: DesktopAppConfigPrefs,
@@ -621,7 +645,13 @@ export async function runDesktopMain(
       runtimeBase: runtime.base,
       source: runtime.source,
     },
-    { openPath: (path) => shell.openPath(path) },
+    {
+      openPath: (path) => shell.openPath(path),
+      readUpdateInstallMode: async () => {
+        const baseUrl = await resolveDaemonBaseUrl(runtime, options)();
+        return readUpdateInstallMode(baseUrl);
+      },
+    },
   );
   // Resolve the namespace root the same way the daemon diagnostics export does
   // (apps/daemon/src/diagnostics-export.ts buildSidecarLogSources). In packaged