Skip to content

Commit e631493

Browse files
authored
feat: close architecture gaps G1, G2, G3 — error types, state versioning, structured logging (#27)
* feat(g1): add structured error types with toBeamCodeError and errorMessage utilities * refactor(g1): replace local toError with toBeamCodeError in process-supervisor * feat(g3): add structured JSON logger with level filtering and safe serialization * feat(g3): wire structured logger as default in beamcode entry point * feat(g2): add state schema versioning with step-registry migrator - Add state-migrator with step-registry pattern (Map<version, MigrationFn>) - Wire migrateSession into FileStorage load/loadAll/saveSync - Stamp schemaVersion on save, migrate on load - Add schemaVersion field to PersistedSession type - v0→v1 migration: ensure messageHistory, pendingMessages, pendingPermissions - Skip unmigrateable sessions in loadAll instead of crashing * docs: add G1/G2/G3 architecture gaps implementation plan * fix: address CI type check failure and review feedback - Fix TS2352: cast Record<string, unknown> via unknown for PersistedSession - Prevent log field spoofing: skip reserved keys (time, level, msg, component) in ctx - Extract makeV0Session helper to deduplicate test code - Clean up stale section comments in index.ts exports
1 parent b0052fb commit e631493

13 files changed

+1270
-12
lines changed

docs/plans/2026-02-17-arch-gaps-g1-g2-g3.md

Lines changed: 755 additions & 0 deletions
Large diffs are not rendered by default.

src/adapters/file-storage.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,91 @@ describe("FileStorage", () => {
447447
});
448448
});
449449

450+
// -----------------------------------------------------------------------
451+
// Schema migration on load
452+
// -----------------------------------------------------------------------
453+
454+
describe("schema migration on load", () => {
455+
/** Create a v0 session JSON string (no schemaVersion field). */
456+
function makeV0Session(id: string): string {
457+
return JSON.stringify({
458+
id,
459+
state: {
460+
session_id: id,
461+
model: "test",
462+
cwd: "/test",
463+
tools: [],
464+
permissionMode: "default",
465+
claude_code_version: "1.0",
466+
mcp_servers: [],
467+
agents: [],
468+
slash_commands: [],
469+
skills: [],
470+
total_cost_usd: 0,
471+
num_turns: 0,
472+
context_used_percent: 0,
473+
is_compacting: false,
474+
git_branch: "",
475+
is_worktree: false,
476+
repo_root: "",
477+
git_ahead: 0,
478+
git_behind: 0,
479+
total_lines_added: 0,
480+
total_lines_removed: 0,
481+
},
482+
messageHistory: [],
483+
});
484+
}
485+
486+
it("migrates unversioned sessions on load", () => {
487+
writeFileSync(join(dir, `${VALID_UUID}.json`), makeV0Session(VALID_UUID));
488+
489+
const loaded = storage.load(VALID_UUID);
490+
expect(loaded).not.toBeNull();
491+
expect(loaded!.schemaVersion).toBe(1);
492+
expect(loaded!.pendingMessages).toEqual([]);
493+
expect(loaded!.pendingPermissions).toEqual([]);
494+
});
495+
496+
it("stamps schemaVersion on save", () => {
497+
const session = makeSession(VALID_UUID);
498+
storage.saveSync(session);
499+
500+
// Read raw file to check schemaVersion was stamped
501+
const raw = JSON.parse(readFileSync(join(dir, `${VALID_UUID}.json`), "utf-8"));
502+
expect(raw.schemaVersion).toBe(1);
503+
});
504+
505+
it("migrates unversioned sessions in loadAll", () => {
506+
writeFileSync(join(dir, `${VALID_UUID}.json`), makeV0Session(VALID_UUID));
507+
writeFileSync(join(dir, `${VALID_UUID_2}.json`), makeV0Session(VALID_UUID_2));
508+
509+
const all = storage.loadAll();
510+
expect(all).toHaveLength(2);
511+
for (const session of all) {
512+
expect(session.schemaVersion).toBe(1);
513+
}
514+
});
515+
516+
it("skips sessions that fail migration in loadAll", () => {
517+
// Write a valid session
518+
storage.saveSync(makeSession(VALID_UUID));
519+
// Write a session with a future schema version (unmigrateable)
520+
writeFileSync(
521+
join(dir, `${VALID_UUID_2}.json`),
522+
JSON.stringify({
523+
id: VALID_UUID_2,
524+
state: { session_id: VALID_UUID_2 },
525+
schemaVersion: 999,
526+
}),
527+
);
528+
529+
const all = storage.loadAll();
530+
expect(all).toHaveLength(1);
531+
expect(all[0].id).toBe(VALID_UUID);
532+
});
533+
});
534+
450535
// -----------------------------------------------------------------------
451536
// Error handling (atomic write failures)
452537
// -----------------------------------------------------------------------

src/adapters/file-storage.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { join, normalize, resolve } from "node:path";
1313
import type { LauncherStateStorage, SessionStorage } from "../interfaces/storage.js";
1414
import type { PersistedSession } from "../types/session-state.js";
15+
import { CURRENT_SCHEMA_VERSION, migrateSession } from "./state-migrator.js";
1516

1617
const SESSION_ID_PATTERN = /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/;
1718

@@ -129,6 +130,7 @@ export class FileStorage implements SessionStorage, LauncherStateStorage {
129130

130131
saveSync(session: PersistedSession): void {
131132
try {
133+
session.schemaVersion = CURRENT_SCHEMA_VERSION;
132134
this.atomicWrite(this.filePath(session.id), JSON.stringify(session));
133135
} catch (err) {
134136
// Log but don't crash — storage failures shouldn't kill sessions
@@ -139,7 +141,7 @@ export class FileStorage implements SessionStorage, LauncherStateStorage {
139141
load(sessionId: string): PersistedSession | null {
140142
try {
141143
const raw = readFileSync(this.filePath(sessionId), "utf-8");
142-
return JSON.parse(raw) as PersistedSession;
144+
return migrateSession(JSON.parse(raw));
143145
} catch {
144146
return null;
145147
}
@@ -157,7 +159,8 @@ export class FileStorage implements SessionStorage, LauncherStateStorage {
157159
if (!SESSION_ID_PATTERN.test(sessionId)) continue;
158160
try {
159161
const raw = readFileSync(safeJoin(this.dir, file), "utf-8");
160-
sessions.push(JSON.parse(raw));
162+
const migrated = migrateSession(JSON.parse(raw));
163+
if (migrated) sessions.push(migrated);
161164
} catch {
162165
// Skip corrupt files
163166
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { describe, expect, it } from "vitest";
2+
import { CURRENT_SCHEMA_VERSION, migrateSession } from "./state-migrator.js";
3+
4+
describe("state-migrator", () => {
5+
it("returns null for null input", () => {
6+
expect(migrateSession(null)).toBeNull();
7+
});
8+
9+
it("returns null for non-object input", () => {
10+
expect(migrateSession("string")).toBeNull();
11+
expect(migrateSession(42)).toBeNull();
12+
});
13+
14+
it("returns null when id is not a string", () => {
15+
expect(migrateSession({ state: {} })).toBeNull();
16+
expect(migrateSession({ id: 123, state: {} })).toBeNull();
17+
});
18+
19+
it("returns null when state is not an object", () => {
20+
expect(migrateSession({ id: "test", state: "corrupt" })).toBeNull();
21+
expect(migrateSession({ id: "test" })).toBeNull();
22+
});
23+
24+
it("adds schemaVersion to v0 (unversioned) sessions", () => {
25+
const v0 = { id: "test-id", state: { session_id: "test-id" }, messageHistory: [] };
26+
const result = migrateSession(v0);
27+
expect(result).not.toBeNull();
28+
expect(result!.schemaVersion).toBe(CURRENT_SCHEMA_VERSION);
29+
});
30+
31+
it("passes through current-version sessions unchanged", () => {
32+
const current = {
33+
id: "test-id",
34+
state: { session_id: "test-id" },
35+
messageHistory: [],
36+
pendingMessages: [],
37+
pendingPermissions: [],
38+
schemaVersion: CURRENT_SCHEMA_VERSION,
39+
};
40+
const result = migrateSession(current);
41+
expect(result).toEqual(current);
42+
});
43+
44+
it("returns null for future versions it cannot handle", () => {
45+
const future = {
46+
id: "test-id",
47+
state: { session_id: "test-id" },
48+
messageHistory: [],
49+
schemaVersion: 999,
50+
};
51+
expect(migrateSession(future)).toBeNull();
52+
});
53+
54+
it("adds default fields missing in v0 sessions", () => {
55+
const v0 = { id: "test-id", state: { session_id: "test-id" } };
56+
const result = migrateSession(v0);
57+
expect(result!.messageHistory).toEqual([]);
58+
expect(result!.pendingMessages).toEqual([]);
59+
expect(result!.pendingPermissions).toEqual([]);
60+
});
61+
});

src/adapters/state-migrator.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import type { PersistedSession } from "../types/session-state.js";
2+
3+
export const CURRENT_SCHEMA_VERSION = 1;
4+
5+
type MigrationFn = (session: Record<string, unknown>) => Record<string, unknown>;
6+
7+
/** Map from source version to the function that migrates to source+1. */
8+
const migrations: Map<number, MigrationFn> = new Map([[0, migrateV0ToV1]]);
9+
10+
function migrateV0ToV1(session: Record<string, unknown>): Record<string, unknown> {
11+
return {
12+
...session,
13+
messageHistory: Array.isArray(session.messageHistory) ? session.messageHistory : [],
14+
pendingMessages: Array.isArray(session.pendingMessages) ? session.pendingMessages : [],
15+
pendingPermissions: Array.isArray(session.pendingPermissions) ? session.pendingPermissions : [],
16+
schemaVersion: 1,
17+
};
18+
}
19+
20+
/**
21+
* Migrate a persisted session to the current schema version.
22+
* Returns null if the session cannot be migrated (corrupt, missing fields, or future version).
23+
*/
24+
export function migrateSession(raw: unknown): PersistedSession | null {
25+
if (raw == null || typeof raw !== "object") return null;
26+
27+
const session = raw as Record<string, unknown>;
28+
29+
// Required fields
30+
if (typeof session.id !== "string") return null;
31+
if (session.state == null || typeof session.state !== "object") return null;
32+
33+
let version = typeof session.schemaVersion === "number" ? session.schemaVersion : 0;
34+
35+
// Cannot downgrade from future versions
36+
if (version > CURRENT_SCHEMA_VERSION) return null;
37+
38+
// Already current
39+
if (version === CURRENT_SCHEMA_VERSION) return raw as unknown as PersistedSession;
40+
41+
// Run migration chain
42+
let current = session;
43+
while (version < CURRENT_SCHEMA_VERSION) {
44+
const migrate = migrations.get(version);
45+
if (!migrate) return null; // Gap in migration chain
46+
current = migrate(current);
47+
version++;
48+
}
49+
50+
return current as unknown as PersistedSession;
51+
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { describe, expect, it } from "vitest";
2+
import { LogLevel, StructuredLogger } from "./structured-logger.js";
3+
4+
describe("StructuredLogger", () => {
5+
it("outputs JSON lines to the writer", () => {
6+
const lines: string[] = [];
7+
const logger = new StructuredLogger({ writer: (line) => lines.push(line) });
8+
9+
logger.info("server started", { port: 3456 });
10+
11+
const parsed = JSON.parse(lines[0]);
12+
expect(parsed.level).toBe("info");
13+
expect(parsed.msg).toBe("server started");
14+
expect(parsed.port).toBe(3456);
15+
expect(parsed.time).toBeTypeOf("string"); // ISO 8601
16+
});
17+
18+
it("respects log level filtering", () => {
19+
const lines: string[] = [];
20+
const logger = new StructuredLogger({
21+
writer: (line) => lines.push(line),
22+
level: LogLevel.WARN,
23+
});
24+
25+
logger.debug("hidden");
26+
logger.info("hidden");
27+
logger.warn("visible");
28+
logger.error("visible");
29+
30+
expect(lines).toHaveLength(2);
31+
});
32+
33+
it("includes component name when set", () => {
34+
const lines: string[] = [];
35+
const logger = new StructuredLogger({
36+
writer: (line) => lines.push(line),
37+
component: "session-bridge",
38+
});
39+
40+
logger.info("test");
41+
42+
const parsed = JSON.parse(lines[0]);
43+
expect(parsed.component).toBe("session-bridge");
44+
});
45+
46+
it("passes correlation ID through ctx parameter", () => {
47+
const lines: string[] = [];
48+
const logger = new StructuredLogger({ writer: (line) => lines.push(line) });
49+
50+
logger.info("message received", { correlationId: "sess-abc-123" });
51+
52+
const parsed = JSON.parse(lines[0]);
53+
expect(parsed.correlationId).toBe("sess-abc-123");
54+
});
55+
56+
it("serializes error objects with stack", () => {
57+
const lines: string[] = [];
58+
const logger = new StructuredLogger({ writer: (line) => lines.push(line) });
59+
60+
logger.error("failed", { error: new Error("boom") });
61+
62+
const parsed = JSON.parse(lines[0]);
63+
expect(parsed.error).toBe("boom");
64+
expect(parsed.errorStack).toContain("Error: boom");
65+
});
66+
67+
it("does not allow ctx to overwrite reserved fields", () => {
68+
const lines: string[] = [];
69+
const logger = new StructuredLogger({
70+
writer: (line) => lines.push(line),
71+
component: "test",
72+
});
73+
74+
logger.info("spoofed", { level: "debug", time: "fake", msg: "injected", component: "evil" });
75+
76+
const parsed = JSON.parse(lines[0]);
77+
expect(parsed.level).toBe("info");
78+
expect(parsed.msg).toBe("spoofed");
79+
expect(parsed.component).toBe("test");
80+
expect(parsed.time).not.toBe("fake");
81+
});
82+
83+
it("survives circular references in ctx", () => {
84+
const lines: string[] = [];
85+
const logger = new StructuredLogger({ writer: (line) => lines.push(line) });
86+
87+
const circular: Record<string, unknown> = { key: "value" };
88+
circular.self = circular;
89+
90+
// Should not throw
91+
logger.error("circular data", circular);
92+
93+
expect(lines).toHaveLength(1);
94+
const parsed = JSON.parse(lines[0]);
95+
expect(parsed.msg).toBe("circular data");
96+
});
97+
});

0 commit comments

Comments
 (0)