Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions apps/daemon/src/app-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Comment thread
cbeaulieu-gt marked this conversation as resolved.
Comment thread
cbeaulieu-gt marked this conversation as resolved.
}

// Cap on how many recent working directories we remember. Keeps the picker's
Expand All @@ -146,6 +150,7 @@ const ALLOWED_KEYS: ReadonlySet<keyof AppConfigPrefs> = new Set([
'projectLocations',
'defaultProjectLocationId',
'recentLinkedDirs',
'updateInstallMode',
] as const);

function configFile(dataDir: string): string {
Expand Down Expand Up @@ -587,6 +592,14 @@ function applyConfigValue(
}
return;
}
if (key === 'updateInstallMode') {
if (value === 'automatic' || value === 'manual') {
target[key] = value;
} else {
delete target[key];
Comment thread
cbeaulieu-gt marked this conversation as resolved.
}
return;
}
}

function filterAllowedKeys(obj: Record<string, unknown>): AppConfigPrefs {
Expand Down
71 changes: 71 additions & 0 deletions apps/daemon/tests/app-config-update-install-mode.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// 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.

import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';

import { readAppConfig, writeAppConfig } from '../src/app-config.js';

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();
});
});
32 changes: 31 additions & 1 deletion apps/desktop/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/main/updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ export type DesktopUpdaterDeps = {
openPath?: (path: string) => Promise<string>;
processExecPath?: string;
processPid?: number;
/** Lazy-read the user's install-mode preference at check time. Absent → treated as 'automatic'. */
readUpdateInstallMode?: () => Promise<'automatic' | 'manual' | undefined>;
spawnDetached?: SpawnInstallerHelper;
};

Expand Down Expand Up @@ -2269,6 +2271,7 @@ export function createDesktopUpdater(
const openPath = deps.openPath ?? (async () => "openPath is not available");
const processPid = deps.processPid ?? process.pid;
const extractLauncherPayloadArchive = deps.extractLauncherPayloadArchive ?? defaultExtractLauncherPayloadArchive;
const readUpdateInstallMode = deps.readUpdateInstallMode;
const spawnDetached: SpawnInstallerHelper = deps.spawnDetached ?? ((command, args, options) => spawn(command, args, options));
const launchInstallerAfterQuit = deps.launchInstallerAfterQuit ?? ((input) => (
config.platform === "win32"
Expand Down Expand Up @@ -2523,7 +2526,9 @@ export function createDesktopUpdater(
lastCheckedAt,
}));
if (root != null) scheduleBackCleanup(root.realRoot, logger);
const selected = selectUpdateCandidateWithFallback(body, config, await hasValidLauncherPayloadContext(config));
const pref = readUpdateInstallMode ? await readUpdateInstallMode() : undefined;
const preferPayload = pref === 'manual' ? false : await hasValidLauncherPayloadContext(config);
Comment thread
cbeaulieu-gt marked this conversation as resolved.
Outdated
const selected = selectUpdateCandidateWithFallback(body, config, preferPayload);
if (!selected.ok) return setState(selected.state, selected.error);
if (compareVersions(selected.candidate.version, config.currentVersion) <= 0) {
logUpdateEvent("check-not-available", { candidateVersion: selected.candidate.version });
Expand Down
92 changes: 92 additions & 0 deletions apps/desktop/tests/main/index-update-install-mode.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Tests for the exported readUpdateInstallMode helper in apps/desktop/src/main/index.ts — #4467 (PR2).
//
// Spec:
// readUpdateInstallMode(baseUrl, fetchImpl?) fetches GET /api/app-config from the daemon
// and returns the updateInstallMode pref, or undefined on any failure.
//
// Signature: (baseUrl: string, fetchImpl?: typeof globalThis.fetch) => Promise<'automatic' | 'manual' | undefined>
//
// These tests are RED until the implementation lands.

import { describe, expect, it } from "vitest";

import { readUpdateInstallMode } from "../../src/main/index.js";

// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------

type FetchImpl = typeof globalThis.fetch;

/** Builds a fetch stub that resolves to a JSON response body. */
function fakeFetchOk(body: unknown): FetchImpl {
return async (_input: RequestInfo | URL, _init?: RequestInit) => {
const text = JSON.stringify(body);
return new Response(text, {
status: 200,
headers: { "content-type": "application/json" },
});
};
}

/** Builds a fetch stub that resolves to a non-OK HTTP status. */
function fakeFetchStatus(status: number): FetchImpl {
return async () =>
new Response("error", { status });
}

/** Builds a fetch stub that rejects with an error. */
function fakeFetchReject(message: string): FetchImpl {
return async () => {
throw new Error(message);
};
}

// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------

describe("readUpdateInstallMode", () => {
it("returns 'manual' when the daemon config contains updateInstallMode='manual'", async () => {
const fetch = fakeFetchOk({ config: { updateInstallMode: "manual" } });
const result = await readUpdateInstallMode("http://127.0.0.1:9999", fetch);
expect(result).toBe("manual");
});

it("returns 'automatic' when the daemon config contains updateInstallMode='automatic'", async () => {
const fetch = fakeFetchOk({ config: { updateInstallMode: "automatic" } });
const result = await readUpdateInstallMode("http://127.0.0.1:9999", fetch);
expect(result).toBe("automatic");
});

it("returns undefined when fetch throws (network error / daemon unreachable)", async () => {
const fetch = fakeFetchReject("ECONNREFUSED");
const result = await readUpdateInstallMode("http://127.0.0.1:9999", fetch);
expect(result).toBeUndefined();
});

it("returns undefined when the daemon config does not contain the field", async () => {
const fetch = fakeFetchOk({ config: { agentId: "claude" } });
const result = await readUpdateInstallMode("http://127.0.0.1:9999", fetch);
expect(result).toBeUndefined();
});

it("returns undefined when the daemon returns a non-OK HTTP status", async () => {
const fetch = fakeFetchStatus(503);
const result = await readUpdateInstallMode("http://127.0.0.1:9999", fetch);
expect(result).toBeUndefined();
});

it("calls GET /api/app-config relative to the given baseUrl", async () => {
const calls: string[] = [];
const fetch: FetchImpl = async (input) => {
calls.push(String(input));
return new Response(JSON.stringify({ config: { updateInstallMode: "manual" } }), {
status: 200,
headers: { "content-type": "application/json" },
});
};
await readUpdateInstallMode("http://127.0.0.1:8080", fetch);
expect(calls).toEqual(["http://127.0.0.1:8080/api/app-config"]);
});
});
Loading
Loading