Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit cd8e6d5

Browse files
committed
FEA-1497: Address PR #264 review findings (validation, packaging, typecheck)
Targeted fixes for review comments that remained valid against the current FEA-1497 code (most other comments were already resolved by the in-process rewrite). Each fix is scoped and behavior-preserving. - lifecycle: guard hook session_id at runtime (typeof string, non-empty) before it becomes a SQLite primary-key binding. It arrives as `unknown` through the zod envelope, so a non-string would be stringified to "[object Object]" and stored as a junk PK. (thread: thadeusb) - db IPC: add shared coerceDbId() guard and apply it to every desktop:db:* handler that takes a renderer-supplied id/sessionId/ agentId. IPC is a runtime boundary; TS annotations are erased, so a malformed value could otherwise reach node:sqlite bindings. Invalid args now return the same empty result as the disabled path. Adds db-ipc-validation unit tests. (thread: shafty023) - renderer typecheck: add tsconfig.renderer.json and a typecheck:renderer script chained into `typecheck` so the .tsx renderer is type-checked in CI (previously only src/**/*.ts was, src/renderer excluded). Surfaced and fixed a real latent bug: 7 component files imported ../../main/database/types (resolves to the nonexistent renderer/main) instead of ../../../main/database/types — masked because they are erased `import type`s. Declared @types/react / @types/react-dom (used transitively but never as a direct dep). Renderer now type-checks clean with full strict. (thread: shafty023) - packaging: re-chain build:agent-monitor into the package/release scripts. `build` no longer generates .generated/agent-monitor but stage-packaging hard-requires it and the runtime still loads the generated hook-handler scripts, so `pnpm package` failed on a clean checkout (not covered by CI). (thread: shafty023) - packaging: exclude the file:vendor/design-system dep from the staged prod dependencies. Staging runs `pnpm install --prod` before copying app files, so the relative vendor path can't resolve; the renderer is already bundled into dist/renderer by Vite, so it isn't a runtime module. (thread: shafty023) - vendor DS: fix the root export/main/module fields to point at files that exist — import/module/default -> dist/index.mjs (ESM), require/main -> dist/index.js (CJS). They previously pointed import at the CJS index.js and require/main at a nonexistent index.cjs. (thread: shafty023) - window: gate the dev-server load (http://localhost:5173) behind an explicit CL_RENDERER_DEV_SERVER=1 opt-in, defaulting every build to the app:// renderer. The privileged preload would otherwise attach to whatever local process binds :5173 first. No current recipe starts a Vite dev server, so this also removes a pointless failed loadURL on the common dev path. (thread: shafty023) - bump desktop version 0.15.108 -> 0.15.109 Testing: pnpm lint, pnpm typecheck (main + new renderer project), pnpm build:renderer, and pnpm test (2032 pass / 0 fail) all green locally. Staging script syntax-checked. Risks: Packaging changes (build:agent-monitor re-chain, staged-dep filter) are not exercised by CI; validated by code inspection and script syntax check, full electron-builder package not run locally. Renderer typecheck is now CI-gating — verified clean before wiring in.
1 parent 9f357c4 commit cd8e6d5

17 files changed

Lines changed: 424 additions & 246 deletions

File tree

apps/desktop/package.json

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "desktop",
3-
"version": "0.15.108",
3+
"version": "0.15.109",
44
"description": "ClosedLoop Desktop",
55
"author": "ClosedLoop AI <support@closedloop.ai>",
66
"private": true,
@@ -18,7 +18,8 @@
1818
"dashboard:reset": "node scripts/reset-dashboard-db.mjs",
1919
"dashboard:reset-packs": "node scripts/reset-dashboard-db.mjs --packs-only",
2020
"stage:package": "node scripts/stage-packaging-app.mjs",
21-
"typecheck": "tsc -p tsconfig.json --noEmit",
21+
"typecheck": "tsc -p tsconfig.json --noEmit && pnpm typecheck:renderer",
22+
"typecheck:renderer": "tsc -p tsconfig.renderer.json --noEmit",
2223
"lint": "eslint src/",
2324
"test": "tsx --test test/*.test.ts && node --test \"scripts/agent-monitor-packs/__tests__/*.test.js\" \"scripts/agent-monitor-pull-requests/__tests__/*.test.js\"",
2425
"pretest:contract": "pnpm build:agent-monitor",
@@ -34,8 +35,8 @@
3435
"test:audit:ui": "playwright test --config test-e2e/agent-monitor/playwright.audit.config.ts",
3536
"preaudit:report": "pnpm build:agent-monitor",
3637
"audit:report": "node test-e2e/agent-monitor/inventory/run-report.mjs",
37-
"package": "pnpm clean:package && pnpm build && pnpm stage:package && node scripts/run-electron-builder.mjs",
38-
"release": "pnpm clean:package && pnpm build && pnpm stage:package && node scripts/run-electron-builder.mjs --publish always"
38+
"package": "pnpm clean:package && pnpm build && pnpm build:agent-monitor && pnpm stage:package && node scripts/run-electron-builder.mjs",
39+
"release": "pnpm clean:package && pnpm build && pnpm build:agent-monitor && pnpm stage:package && node scripts/run-electron-builder.mjs --publish always"
3940
},
4041
"dependencies": {
4142
"@closedloop-ai/design-system": "file:vendor/design-system",
@@ -64,6 +65,8 @@
6465
"@tailwindcss/vite": "^4.3.0",
6566
"@types/busboy": "^1.5.4",
6667
"@types/node": "^22.13.8",
68+
"@types/react": "^19.2.0",
69+
"@types/react-dom": "^19.2.0",
6770
"@typescript-eslint/eslint-plugin": "^8.57.1",
6871
"@typescript-eslint/parser": "^8.57.1",
6972
"@vitejs/plugin-react": "4.3.4",

apps/desktop/scripts/stage-packaging-app.mjs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ const rendererEntryFile = path.join(appDir, "src/renderer/index.html");
2525
const stageRendererDir = path.join(stageAppDir, "src/renderer");
2626
const stageNpmrcFile = path.join(stageAppDir, ".npmrc");
2727

28+
// Dependencies that Vite bundles directly into `dist/renderer` and that are
29+
// therefore NOT needed as runtime node modules in the packaged app. The
30+
// vendored design-system is declared as `file:vendor/design-system`, but the
31+
// staging install (`pnpm install --prod` in the stage dir) runs before any app
32+
// files — including `vendor/` — are copied, so that relative spec cannot
33+
// resolve and the install fails. Excluding it here keeps staging install-able;
34+
// the renderer already ships its design-system code inside the Vite bundle.
35+
const BUNDLED_ONLY_DEPENDENCIES = new Set(["@closedloop-ai/design-system"]);
36+
2837
function resolveStageDependencySpec(packageJson, dependencyName, dependency) {
2938
if (typeof dependency.resolved === "string" && dependency.resolved.length > 0) {
3039
return dependency.resolved;
@@ -106,10 +115,12 @@ const stagePackageJson = {
106115
type: packageJson.type,
107116
main: packageJson.main,
108117
dependencies: Object.fromEntries(
109-
Object.entries(installedDependencies).map(([dependencyName, dependency]) => [
110-
dependencyName,
111-
resolveStageDependencySpec(packageJson, dependencyName, dependency),
112-
]),
118+
Object.entries(installedDependencies)
119+
.filter(([dependencyName]) => !BUNDLED_ONLY_DEPENDENCIES.has(dependencyName))
120+
.map(([dependencyName, dependency]) => [
121+
dependencyName,
122+
resolveStageDependencySpec(packageJson, dependencyName, dependency),
123+
]),
113124
),
114125
};
115126
const stageRootPackageJson = {

apps/desktop/src/main/app.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import { DesktopTray } from "./tray.js";
7676
import { DesktopWindow } from "./window.js";
7777
import { AgentHookListener } from "./agent-monitor-listener.js";
7878
import { openAgentDatabase } from "./database/index.js";
79+
import { coerceDbId } from "./database/ipc-validation.js";
7980
import { createLifecycle } from "./database/lifecycle.js";
8081
import { detectBillingMode } from "./billing-mode-detector.js";
8182
import { migrateVendorDashboardDb } from "./agent-monitor-db-migration.js";
@@ -3629,27 +3630,32 @@ export class DesktopApplication {
36293630
return this.agentDatabase.sessions.getAll();
36303631
});
36313632

3632-
ipcMain.handle("desktop:db:get-session", (_event, id: string) => {
3633+
ipcMain.handle("desktop:db:get-session", (_event, id: unknown) => {
36333634
if (!this.isAgentMonitorEnabled()) return undefined;
3634-
return this.agentDatabase.sessions.getById(id);
3635+
const sessionId = coerceDbId(id);
3636+
if (sessionId === null) return undefined;
3637+
return this.agentDatabase.sessions.getById(sessionId);
36353638
});
36363639

3637-
ipcMain.handle("desktop:db:get-agents", (_event, sessionId: string) => {
3640+
ipcMain.handle("desktop:db:get-agents", (_event, sessionId: unknown) => {
36383641
if (!this.isAgentMonitorEnabled()) return [];
3639-
return this.agentDatabase.agents.getBySession(sessionId);
3642+
const id = coerceDbId(sessionId);
3643+
if (id === null) return [];
3644+
return this.agentDatabase.agents.getBySession(id);
36403645
});
36413646

36423647
ipcMain.handle(
36433648
"desktop:db:get-events",
3644-
(_event, sessionId: string, agentId?: string) => {
3649+
(_event, sessionId: unknown, agentId?: unknown) => {
36453650
if (!this.isAgentMonitorEnabled()) return [];
3646-
if (agentId) {
3647-
return this.agentDatabase.events.getBySessionAndAgent(
3648-
sessionId,
3649-
agentId,
3650-
);
3651+
const sid = coerceDbId(sessionId);
3652+
if (sid === null) return [];
3653+
// A missing or malformed agentId falls back to the session-wide query.
3654+
const aid = coerceDbId(agentId);
3655+
if (aid !== null) {
3656+
return this.agentDatabase.events.getBySessionAndAgent(sid, aid);
36513657
}
3652-
return this.agentDatabase.events.getBySession(sessionId);
3658+
return this.agentDatabase.events.getBySession(sid);
36533659
},
36543660
);
36553661

@@ -3670,9 +3676,11 @@ export class DesktopApplication {
36703676
return this.agentDatabase.events.getAll();
36713677
});
36723678

3673-
ipcMain.handle("desktop:db:get-events-with-session", (_event, sessionId: string) => {
3679+
ipcMain.handle("desktop:db:get-events-with-session", (_event, sessionId: unknown) => {
36743680
if (!this.isAgentMonitorEnabled()) return [];
3675-
return this.agentDatabase.events.getWithSession(sessionId);
3681+
const id = coerceDbId(sessionId);
3682+
if (id === null) return [];
3683+
return this.agentDatabase.events.getWithSession(id);
36763684
});
36773685

36783686
ipcMain.handle("desktop:db:get-event-count-by-type", () => {
@@ -3687,9 +3695,11 @@ export class DesktopApplication {
36873695
return this.agentDatabase.dashboard.getTokenAnalytics();
36883696
});
36893697

3690-
ipcMain.handle("desktop:db:get-agent-hierarchy", (_event, sessionId: string) => {
3698+
ipcMain.handle("desktop:db:get-agent-hierarchy", (_event, sessionId: unknown) => {
36913699
if (!this.isAgentMonitorEnabled()) return [];
3692-
return this.agentDatabase.agents.getBySessionWithChildren(sessionId, this.agentDatabase.events);
3700+
const id = coerceDbId(sessionId);
3701+
if (id === null) return [];
3702+
return this.agentDatabase.agents.getBySessionWithChildren(id, this.agentDatabase.events);
36933703
});
36943704

36953705
ipcMain.handle("desktop:db:get-analytics", () => {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Runtime validation for renderer-supplied identifiers that reach the
2+
// `desktop:db:*` IPC handlers. IPC is a runtime trust boundary: the TypeScript
3+
// annotations on the handler arguments are erased at runtime, so a malformed
4+
// `id` / `sessionId` / `agentId` (an object, number, null, or oversized string)
5+
// from a compromised or buggy renderer could otherwise reach `node:sqlite`
6+
// bindings and throw inside the query or alter its behavior. The DB IPC handlers
7+
// must coerce every identifier through these guards before it touches the store
8+
// layer. (CLAUDE.md: runtime-validate gateway, IPC, and persisted payloads.)
9+
10+
/**
11+
* Upper bound on identifier length. Session/agent ids are short UUID-ish
12+
* strings; anything dramatically longer is malformed input, not a real id.
13+
*/
14+
export const MAX_DB_ID_LENGTH = 512;
15+
16+
/**
17+
* Validate a renderer-supplied database identifier.
18+
*
19+
* Returns the value unchanged when it is a usable id (a non-empty string within
20+
* the length bound), or `null` when it is not. Callers treat `null` as "no such
21+
* record" — they return the same empty/undefined result the handlers already
22+
* return when the monitor is disabled, so a bad argument can never reach a
23+
* SQLite binding.
24+
*/
25+
export function coerceDbId(value: unknown): string | null {
26+
if (typeof value !== "string") {
27+
return null;
28+
}
29+
if (value.length === 0 || value.length > MAX_DB_ID_LENGTH) {
30+
return null;
31+
}
32+
return value;
33+
}

apps/desktop/src/main/database/lifecycle.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,8 +439,14 @@ export function createLifecycle(db: DatabaseSync, deps: LifecycleDeps) {
439439
* the fail-silent hook handler.
440440
*/
441441
processEvent(hookType: string, data: HookData, harness: string): boolean {
442+
// `session_id` arrives as `unknown` through the listener's zod envelope
443+
// (`data` is validated only as a string-keyed record), so its declared
444+
// `string` type is not enforced at runtime. Guard the actual type before
445+
// it becomes a SQLite primary-key binding: a non-string (e.g. an object)
446+
// would otherwise be stringified to "[object Object]" and stored as a
447+
// junk primary key. (CLAUDE.md: runtime-validate persisted payloads.)
442448
const sessionId = data.session_id;
443-
if (!sessionId) {
449+
if (typeof sessionId !== "string" || sessionId.length === 0) {
444450
return false;
445451
}
446452
// Read the transcript (file IO) BEFORE opening the write transaction.

apps/desktop/src/main/window.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,29 @@ export class DesktopWindow {
8080
private async loadContent(): Promise<void> {
8181
registerAppProtocol();
8282

83-
if (app.isPackaged) {
84-
await this.browserWindow!.loadURL(`app://renderer/index.html`);
85-
return;
83+
// The privileged renderer window carries the full `desktopApi` preload
84+
// bridge (approvals, settings, logs, DB reads, binary-path mutations). The
85+
// Vite dev server (http://localhost:5173) is only ever loaded into it when a
86+
// developer explicitly opts in via CL_RENDERER_DEV_SERVER=1 — otherwise any
87+
// local process that binds :5173 first could serve renderer JS into the
88+
// privileged window. The default path (including every packaged build) is
89+
// the custom `app://` protocol, which also skips a pointless failed loadURL
90+
// on the common dev workflow where no Vite server is running.
91+
const devServerUrl =
92+
!app.isPackaged && process.env.CL_RENDERER_DEV_SERVER === "1"
93+
? process.env.CL_RENDERER_DEV_SERVER_URL || "http://localhost:5173"
94+
: null;
95+
96+
if (devServerUrl) {
97+
try {
98+
await this.browserWindow!.loadURL(devServerUrl);
99+
return;
100+
} catch {
101+
// Dev server unreachable — fall through to the packaged renderer.
102+
}
86103
}
87104

88-
try {
89-
await this.browserWindow!.loadURL("http://localhost:5173");
90-
} catch {
91-
await this.browserWindow!.loadURL(`app://renderer/index.html`);
92-
}
105+
await this.browserWindow!.loadURL(`app://renderer/index.html`);
93106
}
94107

95108
getWindow(): BrowserWindow | null {

apps/desktop/src/renderer/components/analytics/AnalyticsView.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { ActivityHeatmap } from "@closedloop-ai/design-system/components/ui/prim
77
import { SegmentedBar } from "@closedloop-ai/design-system/components/ui/primitives/segmented-bar";
88
import { Coins, ArrowDownToLine, ArrowUpFromLine, DatabaseZap } from "lucide-react";
99
import { useQueryCache } from "../../hooks/useQueryCache";
10-
import type { AnalyticsData } from "../../main/database/types";
10+
import type { AnalyticsData } from "../../../main/database/types";
1111

1212
const PALETTE = [
1313
"#3b82f6", "#10b981", "#f59e0b", "#ef4444", "#8b5cf6",

apps/desktop/src/renderer/components/dashboard/DashboardPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { MetricCard } from "@closedloop-ai/design-system/components/ui/primitive
22
import { MonitorDot, Bot, Zap, Layers } from "lucide-react";
33
import { useQueryCache } from "../../hooks/useQueryCache";
44
import { SessionsView } from "../sessions/SessionsView";
5-
import type { DashboardSummary, AnalyticsData } from "../../main/database/types";
5+
import type { DashboardSummary, AnalyticsData } from "../../../main/database/types";
66

77
export function DashboardPage() {
88
const { data: summary, error } = useQueryCache<DashboardSummary>(

apps/desktop/src/renderer/components/feed/ActivityFeedView.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Button } from "@closedloop-ai/design-system/components/ui/button";
33
import { Badge } from "@closedloop-ai/design-system/components/ui/badge";
44
import { MetricCard } from "@closedloop-ai/design-system/components/ui/primitives/metric-card";
55
import { Zap, Wrench, AlertCircle, Layers } from "lucide-react";
6-
import type { EventRow } from "../../main/database/types";
6+
import type { EventRow } from "../../../main/database/types";
77

88
const EVENT_TYPE_TONES: Record<string, string> = {
99
tool_use: "bg-blue-500/10 text-blue-600",

apps/desktop/src/renderer/components/kanban/KanbanView.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useState } from "react";
22
import { KanbanBoardLayout, KanbanColumn, KanbanCardFrame } from "@closedloop-ai/design-system/components/ui/layout/kanban-board";
33
import { Badge } from "@closedloop-ai/design-system/components/ui/badge";
44
import { useQueryCache } from "../../hooks/useQueryCache";
5-
import type { SessionWithAgents } from "../../main/database/types";
5+
import type { SessionWithAgents } from "../../../main/database/types";
66

77
function PlayIcon() { return <span className="text-blue-400 text-xs">&#9654;</span>; }
88
function ClockIcon() { return <span className="text-yellow-400 text-xs">&#9201;</span>; }

0 commit comments

Comments
 (0)