Skip to content

Commit 9fd02b2

Browse files
refactor(update): hide install spawn mechanics
1 parent 4ea0fa4 commit 9fd02b2

9 files changed

Lines changed: 166 additions & 104 deletions

File tree

docs/intentional-architecture-rewrite-2026-06-27/decision-log.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ Wiki services own registry product verbs such as listing and dropping wikis. The
105105

106106
The CLI edge owns discovering the current Node command and entrypoint. Lifecycle services pass that worker-program value through their workflows, and the job runtime validates that an entrypoint exists before queueing a detached worker. `src/platform/jobs/worker-process.ts` owns only the mechanics of spawning the detached process. The shared shape lives in `src/shared/worker-program.ts` so lifecycle code does not import platform worker-process mechanics and platform code does not import service workflow types.
107107

108+
### Update services consume install outcomes
109+
110+
The update service owns update workflow policy: check latest, honor dismissals, acquire the update lock, decide whether installation should run, refresh state after success, and return a user-facing result. The platform update install module owns npm mechanics: command, arguments, child-process spawn options, missing-npm handling, and install-failure hints. Tests for update workflow inject `UpdateInstallFn`, while tests for `src/platform/update/install.ts` inject spawn mechanics.
111+
108112
### Guard boundaries with tests
109113

110114
Architecture-boundary tests are part of the rewrite, not decoration. When a smell is removed, add or update a test that makes the old leak harder to reintroduce.

docs/intentional-architecture-rewrite-2026-06-27/status.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ Branch: `codex/intentional-architecture-rewrite`
55

66
## Current State
77

8-
The branch has more than 230 committed rewrite commits past `dev`. The worklog records 184 production slices so far.
8+
The branch has more than 230 committed rewrite commits past `dev`. The worklog records 185 production slices so far.
99

10-
The diff is broad: 474 files changed, with 23,845 insertions and 12,888 deletions.
10+
The diff is broad: 475 files changed, with 23,969 insertions and 12,950 deletions.
1111

1212
This is no longer a small cleanup branch. It is a real ownership rewrite.
1313

@@ -37,6 +37,7 @@ This is no longer a small cleanup branch. It is a real ownership rewrite.
3737
- Moved lifecycle operation construction and Absorb input/source handling into `src/services/lifecycle/` and removed the old top-level `src/operations/` and `src/absorb/` source buckets.
3838
- Normalized lifecycle operation failures into lifecycle-owned result contracts before command rendering sees them.
3939
- Moved worker-program shape into `src/shared/worker-program.ts` so lifecycle services no longer import platform worker-process mechanics.
40+
- Reshaped update install injection so update services accept typed install results while platform update modules own npm child-process mechanics.
4041
- Moved GitHub source resolution mechanics into `src/platform/github/`.
4142
- Moved provider execution runtime into `src/agent/runtime/`, especially Claude and Codex app-server mechanics, and made provider runtime environment flow through explicit job/registry contracts.
4243
- Moved setup, diagnostics, update, automation, jobs, sync, lifecycle, config, and agents workflows behind service-owned contracts.
@@ -60,9 +61,9 @@ This is no longer a small cleanup branch. It is a real ownership rewrite.
6061

6162
## Latest Checkpoint
6263

63-
The latest slice moved the background job worker-program contract into `src/shared/worker-program.ts`. CLI edges create the current worker program, lifecycle workflows pass the shared contract, job runtime validates it, and `src/platform/jobs/worker-process.ts` owns only detached process spawning.
64+
The latest slice replaced the update workflow's child-process-shaped `spawnFn` option with a typed `installFn` contract. `src/services/update/` now sees installer outcomes as `{ output, errorOutput, code }`, while `src/platform/update/install.ts` owns the npm command, spawn options, ENOENT handling, and install-failure hints.
6465

65-
Verification passed: focused jobs/lifecycle/sync/operation-command/boundary tests, `npm run lint`, full `npm test`, `npm run build`, `node dist/codealmanac.js --version`, `node dist/codealmanac.js absorb --help`, and `node dist/codealmanac.js sync --help`.
66+
Verification passed: focused update/update-install/boundary tests, `npm run lint`, full `npm test`, `npm run build`, `node dist/codealmanac.js --version`, `node dist/codealmanac.js update --help`, and `node dist/codealmanac.js config --help`.
6667

6768
## Immediate Next Work
6869

docs/intentional-architecture-rewrite-2026-06-27/worklog.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,3 +1257,11 @@ One-hundred-eighty-fourth production slice:
12571257
- Updated lifecycle workflows and operation types to import the shared contract instead of `src/platform/jobs/worker-process.ts`.
12581258
- Kept detached worker spawning in `src/platform/jobs/worker-process.ts` and queued-job startup in `src/services/jobs/runtime/background-start.ts`.
12591259
- Strengthened boundary coverage so lifecycle services cannot reintroduce the platform worker-process import.
1260+
1261+
One-hundred-eighty-fifth production slice:
1262+
1263+
- Replaced the update workflow's child-process-shaped `spawnFn` option with an `UpdateInstallFn` that returns a typed install result.
1264+
- Removed `node:child_process`, `SpawnOptions`, and `UpdateInstallSpawnFn` from `src/services/update/types.ts`.
1265+
- Kept npm command, spawn options, ENOENT handling, and nonzero-exit hints in `src/platform/update/install.ts`.
1266+
- Moved npm spawn argument coverage into `test/update-install.test.ts`, while update workflow tests now inject installer outcomes.
1267+
- Strengthened boundary coverage so update service types cannot reintroduce platform spawn mechanics.

src/services/update/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
export { runUpdateWorkflow } from "./update.js";
22
export type {
33
UpdateCheckFn,
4+
UpdateInstallFn,
45
UpdateInstallResult,
5-
UpdateInstallSpawnFn,
66
UpdateOptions,
77
UpdateWorkflowResult,
88
} from "./types.js";

src/services/update/types.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import type { SpawnOptions } from "node:child_process";
2-
31
export interface UpdateCheckRequest {
42
installedVersion?: string;
53
cacheSeconds?: number;
@@ -28,23 +26,14 @@ export type UpdateCheckFn = (
2826
request?: UpdateCheckRequest,
2927
) => Promise<UpdateCheckResult>;
3028

31-
export interface UpdateInstallChildProcess {
32-
on(event: "error", listener: (error: NodeJS.ErrnoException) => void): this;
33-
on(event: "exit", listener: (code: number | null) => void): this;
34-
}
35-
36-
export type UpdateInstallSpawnFn = (
37-
command: string,
38-
args: readonly string[],
39-
options?: SpawnOptions,
40-
) => UpdateInstallChildProcess;
41-
4229
export interface UpdateInstallResult {
4330
output: string;
4431
errorOutput: string;
4532
code: number;
4633
}
4734

35+
export type UpdateInstallFn = () => Promise<UpdateInstallResult>;
36+
4837
export interface UpdateOptions {
4938
dismiss?: boolean;
5039
check?: boolean;
@@ -55,7 +44,7 @@ export interface UpdateOptions {
5544
configPath?: string;
5645
installedVersion?: string;
5746
checkFn?: UpdateCheckFn;
58-
spawnFn?: UpdateInstallSpawnFn;
47+
installFn?: UpdateInstallFn;
5948
now?: () => number;
6049
lockPath?: string;
6150
lockStaleSeconds?: number;

src/services/update/update.ts

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ import {
88
installLatestPackage,
99
type InstallLatestPackageResult,
1010
} from "../../platform/update/install.js";
11-
import type { spawn as nodeSpawn } from "node:child_process";
1211
import { acquireUpdateLock } from "../../platform/update/lock.js";
1312
import { readState, writeState } from "../../platform/update/state.js";
1413
import { readInstalledVersion } from "../../platform/update/version.js";
1514
import { isNewerVersion } from "../../shared/version.js";
1615
import type {
1716
UpdateInstallResult,
18-
UpdateInstallSpawnFn,
1917
UpdateOptions,
2018
UpdateWorkflowResult,
2119
} from "./types.js";
@@ -141,9 +139,8 @@ async function installIfNeeded(
141139
}
142140

143141
try {
144-
const install = updateInstallResultFromPlatform(
145-
await installLatestPackageForUpdate(opts.spawnFn),
146-
);
142+
const installFn = opts.installFn ?? installLatestPackageForUpdate;
143+
const install = await installFn();
147144
if (install.code !== 0) {
148145
return { status: "install-result", result: install };
149146
}
@@ -154,15 +151,8 @@ async function installIfNeeded(
154151
}
155152
}
156153

157-
async function installLatestPackageForUpdate(
158-
spawnFn?: UpdateInstallSpawnFn,
159-
): Promise<InstallLatestPackageResult> {
160-
return await installLatestPackage({
161-
spawnFn: spawnFn === undefined
162-
? undefined
163-
: ((command, args, options) =>
164-
spawnFn(command, args ?? [], options)) as typeof nodeSpawn,
165-
});
154+
async function installLatestPackageForUpdate(): Promise<UpdateInstallResult> {
155+
return updateInstallResultFromPlatform(await installLatestPackage());
166156
}
167157

168158
function updateInstallResultFromPlatform(

test/architecture-boundaries.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,7 @@ describe("architecture boundaries", () => {
10041004
const updateServiceIndex = await readSource("src/services/update/index.ts");
10051005
const updateService = await readSource("src/services/update/update.ts");
10061006
const updateTypes = await readSource("src/services/update/types.ts");
1007+
const updateInstall = await readSource("src/platform/update/install.ts");
10071008

10081009
expect(existsSync(join(ROOT, "src/cli/commands/update-render.ts"))).toBe(
10091010
true,
@@ -1034,13 +1035,21 @@ describe("architecture boundaries", () => {
10341035
expect(updateService).not.toContain("stdout:");
10351036
expect(updateService).not.toContain("stderr:");
10361037
expect(updateService).not.toContain("exitCode:");
1038+
expect(updateService).not.toContain("node:child_process");
1039+
expect(updateService).toContain("installFn");
10371040
expect(updateService).toContain("updateInstallResultFromPlatform");
10381041
expect(updateTypes).not.toContain(
10391042
"UpdateInstallResult = PlatformInstallLatestPackageResult",
10401043
);
10411044
expect(updateTypes).not.toContain("typeof platformCheckForUpdate");
10421045
expect(updateTypes).not.toContain("typeof nodeSpawn");
1046+
expect(updateTypes).not.toContain("UpdateInstallSpawnFn");
1047+
expect(updateTypes).not.toContain("SpawnOptions");
1048+
expect(updateTypes).not.toContain("node:child_process");
1049+
expect(updateTypes).toContain("UpdateInstallFn");
10431050
expect(updateTypes).not.toContain("platform/update/check");
1051+
expect(updateInstall).toContain("node:child_process");
1052+
expect(updateInstall).toContain("spawnFn");
10441053
});
10451054

10461055
it("keeps config command adapters out of config persistence mechanics", async () => {

test/update-install.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { EventEmitter } from "node:events";
2+
import type { spawn as nodeSpawn } from "node:child_process";
3+
4+
import { describe, expect, it } from "vitest";
5+
6+
import { installLatestPackage } from "../src/platform/update/install.js";
7+
8+
type PlatformSpawnFn = typeof nodeSpawn;
9+
10+
function fakeSpawn(exitCode: number | null): {
11+
spawnFn: PlatformSpawnFn;
12+
calls: Array<{
13+
command: string;
14+
args: readonly string[];
15+
options: unknown;
16+
}>;
17+
} {
18+
const calls: Array<{
19+
command: string;
20+
args: readonly string[];
21+
options: unknown;
22+
}> = [];
23+
const spawnFn = ((command, args, options) => {
24+
calls.push({
25+
command,
26+
args: args ?? [],
27+
options,
28+
});
29+
const emitter = new EventEmitter();
30+
queueMicrotask(() => {
31+
emitter.emit("exit", exitCode, null);
32+
});
33+
return emitter as ReturnType<PlatformSpawnFn>;
34+
}) as PlatformSpawnFn;
35+
return { spawnFn, calls };
36+
}
37+
38+
function fakeSpawnError(code: string): PlatformSpawnFn {
39+
return (() => {
40+
const emitter = new EventEmitter();
41+
queueMicrotask(() => {
42+
const err = new Error(`spawn npm ${code}`) as NodeJS.ErrnoException;
43+
err.code = code;
44+
emitter.emit("error", err);
45+
});
46+
return emitter as ReturnType<PlatformSpawnFn>;
47+
}) as PlatformSpawnFn;
48+
}
49+
50+
describe("installLatestPackage", () => {
51+
it("spawns npm with the global codealmanac latest install command", async () => {
52+
const { spawnFn, calls } = fakeSpawn(0);
53+
54+
const result = await installLatestPackage({ spawnFn });
55+
56+
expect(result).toEqual({
57+
stdout: "almanac: updated.\n",
58+
stderr: "",
59+
exitCode: 0,
60+
});
61+
expect(calls).toEqual([
62+
{
63+
command: "npm",
64+
args: ["i", "-g", "codealmanac@latest"],
65+
options: { stdio: "inherit" },
66+
},
67+
]);
68+
});
69+
70+
it("returns a clear message when npm is not on PATH", async () => {
71+
const result = await installLatestPackage({
72+
spawnFn: fakeSpawnError("ENOENT"),
73+
});
74+
75+
expect(result.exitCode).toBe(1);
76+
expect(result.stderr).toContain("`npm` not found on PATH");
77+
});
78+
79+
it("returns the npm exit code and permission hint on install failure", async () => {
80+
const { spawnFn } = fakeSpawn(243);
81+
82+
const result = await installLatestPackage({ spawnFn });
83+
84+
expect(result.exitCode).toBe(243);
85+
expect(result.stderr).toContain("EACCES");
86+
expect(result.stderr).toContain("sudo npm i -g codealmanac@latest");
87+
});
88+
});

0 commit comments

Comments
 (0)