diff --git a/apps/desktop/src/main/bot-acp-session-ledger.ts b/apps/desktop/src/main/bot-acp-session-ledger.ts index fae9caa1..aef25bbe 100644 --- a/apps/desktop/src/main/bot-acp-session-ledger.ts +++ b/apps/desktop/src/main/bot-acp-session-ledger.ts @@ -1,6 +1,7 @@ -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; import { dirname, isAbsolute, resolve } from "node:path"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; const ID = /^[A-Za-z0-9:_-]{1,128}$/u; const MAX_BYTES = 2_000_000; @@ -22,9 +23,7 @@ interface StoredAcpSessions { function parse(path: string): StoredAcpSessions | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw) > MAX_BYTES) - throw new Error("ACP owner ledger is too large."); + const raw = readBoundedRegularFile(path, MAX_BYTES).toString("utf8"); const value = JSON.parse(raw) as Partial; if ( value.version !== 1 || @@ -129,7 +128,7 @@ export class BotAcpSessionLedger { revision: this.stored.revision + 1, sessions: [...this.stored.sessions, record], }; - const serialized = JSON.stringify(next); + const serialized = `${JSON.stringify(next, null, 2)}\n`; if ( next.sessions.length > MAX_RECORDS || Buffer.byteLength(serialized) > MAX_BYTES diff --git a/apps/desktop/src/main/bot-automation-ledger.ts b/apps/desktop/src/main/bot-automation-ledger.ts index 04de517c..00a7b32f 100644 --- a/apps/desktop/src/main/bot-automation-ledger.ts +++ b/apps/desktop/src/main/bot-automation-ledger.ts @@ -1,7 +1,8 @@ -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; +import { isAbsolute, resolve } from "node:path"; import type { BotPermissions } from "@doolittle/contracts/bots"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; export interface AutomationTargetApproval { id: string; @@ -42,9 +43,7 @@ function date(value: unknown): boolean { } function parse(path: string): Stored | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw) > 8_000_000) - throw new Error("Automation dispatch ledger is full."); + const raw = readBoundedRegularFile(path, 8_000_000).toString("utf8"); const value = JSON.parse(raw) as Stored; if ( value?.version !== 1 || @@ -66,7 +65,7 @@ function parse(path: string): Stored | null { !HASH.test(row.definitionDigest) || (row.workspacePath !== undefined && (typeof row.workspacePath !== "string" || - !row.workspacePath.startsWith("/") || + !isAbsolute(row.workspacePath) || resolve(row.workspacePath) !== row.workspacePath)) || !row.permissions || ![ @@ -159,7 +158,7 @@ export class BotAutomationLedger { throw new Error("Automation dispatch ledger requires recovery."); const value = { ...next, revision: this.stored.revision + 1 }; if ( - Buffer.byteLength(JSON.stringify(value)) > 8_000_000 || + Buffer.byteLength(JSON.stringify(value, null, 2)) + 1 > 8_000_000 || value.approvals.length > 2_000 || value.fires.length > 5_000 ) diff --git a/apps/desktop/src/main/bot-catalog.ts b/apps/desktop/src/main/bot-catalog.ts index 3a8ea89a..8146aa97 100644 --- a/apps/desktop/src/main/bot-catalog.ts +++ b/apps/desktop/src/main/bot-catalog.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; import { isAbsolute, resolve } from "node:path"; import { type BotCatalogResponse, @@ -14,6 +14,7 @@ import { import { normalizeBotCharacter } from "@doolittle/contracts/bot-character"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; import type { BackendManager } from "./backend"; +import { readBoundedRegularFile } from "./regular-file"; const BOT_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/u; // AgentRuntime derives this id from the unmodified Doolittle character name. @@ -252,10 +253,10 @@ export class BotCatalog { private load(): StoredBotCatalog { if (!existsSync(this.filePath)) return { version: 1, bots: [] }; - const raw = readFileSync(this.filePath, "utf8"); - if (Buffer.byteLength(raw) > MAX_CATALOG_BYTES) { - throw new Error("The bot catalog exceeds its supported size."); - } + const raw = readBoundedRegularFile( + this.filePath, + MAX_CATALOG_BYTES, + ).toString("utf8"); const value = JSON.parse(raw) as StoredBotCatalog; if ( value.version !== 1 || @@ -281,17 +282,22 @@ export class BotCatalog { }; } - private save(): void { + private save(next: StoredBotCatalog): void { + if ( + Buffer.byteLength(JSON.stringify(next, null, 2)) + 1 > + MAX_CATALOG_BYTES + ) + throw new Error("The bot catalog is full. No change was saved."); mkdirSync(resolve(this.filePath, ".."), { recursive: true, mode: 0o700 }); - writeJsonAtomicSync(this.filePath, this.stored, { trailingNewline: true }); + writeJsonAtomicSync(this.filePath, next, { trailingNewline: true }); chmodSync(this.filePath, 0o600); + this.stored = next; } defaultBot(): BotDefinition { const state = this.defaultBackend.getState(); if (state.agentId && !this.stored.defaultAgentId) { - this.stored.defaultAgentId = state.agentId; - this.save(); + this.save({ ...this.stored, defaultAgentId: state.agentId }); } const agentId = this.stored.defaultAgentId ?? state.agentId ?? LEGACY_AGENT_ID; @@ -389,8 +395,7 @@ export class BotCatalog { createdAt: now, updatedAt: now, }; - this.stored.bots.push(definition); - this.save(); + this.save({ ...this.stored, bots: [...this.stored.bots, definition] }); return definition; } @@ -405,12 +410,10 @@ export class BotCatalog { if (Object.keys(input).some((key) => key !== "name")) { throw new Error("The lead bot only supports display-name changes."); } - this.stored.defaultName = requiredText( - input.name, - "Bot name", - MAX_NAME_LENGTH, - ); - this.save(); + this.save({ + ...this.stored, + defaultName: requiredText(input.name, "Bot name", MAX_NAME_LENGTH), + }); return this.defaultBot(); } const workspacePath = input.workspacePath @@ -445,10 +448,10 @@ export class BotCatalog { updatedAt: new Date().toISOString(), }; beforeCommit?.(updated); - this.stored.bots = this.stored.bots.map((bot) => - bot.id === id ? updated : bot, - ); - this.save(); + this.save({ + ...this.stored, + bots: this.stored.bots.map((bot) => (bot.id === id ? updated : bot)), + }); return updated; } @@ -460,10 +463,10 @@ export class BotCatalog { if (existing.archivedAt) return existing; const now = new Date().toISOString(); const archived = { ...existing, archivedAt: now, updatedAt: now }; - this.stored.bots = this.stored.bots.map((bot) => - bot.id === id ? archived : bot, - ); - this.save(); + this.save({ + ...this.stored, + bots: this.stored.bots.map((bot) => (bot.id === id ? archived : bot)), + }); return archived; } } diff --git a/apps/desktop/src/main/bot-consultation-ledger.ts b/apps/desktop/src/main/bot-consultation-ledger.ts index 6e063115..60042d0e 100644 --- a/apps/desktop/src/main/bot-consultation-ledger.ts +++ b/apps/desktop/src/main/bot-consultation-ledger.ts @@ -1,11 +1,12 @@ -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; +import { isAbsolute, resolve } from "node:path"; import type { BotConsultationResult, BotPermissions, BotRunOwner, } from "@doolittle/contracts/bots"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; const ID = /^[A-Za-z0-9:_-]{1,128}$/u; const MAX_LEDGER_BYTES = 8_000_000; @@ -46,9 +47,7 @@ interface StoredConsultations { function parse(path: string): StoredConsultations | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw) > MAX_LEDGER_BYTES) - throw new Error("The consultation ledger is too large."); + const raw = readBoundedRegularFile(path, MAX_LEDGER_BYTES).toString("utf8"); const value = JSON.parse(raw) as Partial; if ( value.version !== 1 || @@ -66,7 +65,7 @@ function parse(path: string): StoredConsultations | null { !ID.test(record.targetBotId) || (record.workspacePath !== undefined && (typeof record.workspacePath !== "string" || - !record.workspacePath.startsWith("/") || + !isAbsolute(record.workspacePath) || resolve(record.workspacePath) !== record.workspacePath)) || !ID.test(record.origin?.runId) || !Array.isArray(record.ancestry) || @@ -144,7 +143,7 @@ export class BotConsultationLedger { const next = { ...this.stored, revision: this.stored.revision + 1 }; if ( next.records.length > MAX_RECORDS || - Buffer.byteLength(JSON.stringify(next)) > MAX_LEDGER_BYTES + Buffer.byteLength(JSON.stringify(next, null, 2)) + 1 > MAX_LEDGER_BYTES ) { throw new Error("The consultation ledger is full."); } diff --git a/apps/desktop/src/main/bot-conversation-ledger.ts b/apps/desktop/src/main/bot-conversation-ledger.ts index 1e8b658e..39c36fdc 100644 --- a/apps/desktop/src/main/bot-conversation-ledger.ts +++ b/apps/desktop/src/main/bot-conversation-ledger.ts @@ -1,6 +1,7 @@ -import { chmodSync, existsSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync } from "node:fs"; import { isAbsolute, resolve } from "node:path"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; const SESSION_ID = /^[a-zA-Z0-9:_-]{1,128}$/u; const BOT_ID = /^[a-z0-9][a-z0-9_-]{0,63}$/u; @@ -166,9 +167,7 @@ function validConversation(value: unknown): value is BoundConversation { function parseLedger(path: string): StoredLedger | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw, "utf8") > MAX_LEDGER_BYTES) - throw new Error("The conversation owner ledger is too large."); + const raw = readBoundedRegularFile(path, MAX_LEDGER_BYTES).toString("utf8"); const value = JSON.parse(raw) as Partial; if ( value.version !== 1 || @@ -298,7 +297,7 @@ export class BotConversationLedger { ); this.resources.set(resource.resourceId, resource); } - if ((!primary && backup) || (primary && !backup)) this.save(); + if ((!primary && backup) || (primary && !backup)) this.save("repair"); } get(sessionId: string): BoundConversation | null { @@ -504,7 +503,7 @@ export class BotConversationLedger { return created; } - private save(): void { + private save(mode: "mutation" | "repair" = "mutation"): void { const value: StoredLedger = { version: 1, conversations: [...this.conversations.values()], @@ -512,6 +511,17 @@ export class BotConversationLedger { projects: [...this.projects.values()], resources: [...this.resources.values()], }; + if ( + Buffer.byteLength(JSON.stringify(value, null, 2)) + 1 > + MAX_LEDGER_BYTES + ) { + // A valid legacy copy can fit on disk while the normalized, pretty + // representation does not. Keep that sole recovery source untouched. + if (mode === "repair") return; + throw new Error( + "The conversation owner ledger is full. No change was saved.", + ); + } // Two independently atomic copies allow recovery after an interrupted // upgrade without permitting a conflicting owner to be silently adopted. writeJsonAtomicSync(this.backupPath, value, { trailingNewline: true }); diff --git a/apps/desktop/src/main/bot-knowledge-ledger.ts b/apps/desktop/src/main/bot-knowledge-ledger.ts index 9388eb2c..f92c4bdb 100644 --- a/apps/desktop/src/main/bot-knowledge-ledger.ts +++ b/apps/desktop/src/main/bot-knowledge-ledger.ts @@ -1,10 +1,11 @@ -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; import { dirname, resolve } from "node:path"; import type { KnowledgeGrant, SharedKnowledgeRecord, } from "@doolittle/contracts/bots"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; const ID = /^[A-Za-z0-9:_-]{1,128}$/u; const UUID = /^[0-9a-f-]{36}$/iu; @@ -33,9 +34,7 @@ interface StoredKnowledge { function parse(path: string): StoredKnowledge | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw) > MAX_BYTES) - throw new Error("Knowledge ledger is too large."); + const raw = readBoundedRegularFile(path, MAX_BYTES).toString("utf8"); const value = JSON.parse(raw) as Partial; if ( (value.version !== 1 && value.version !== 2) || @@ -155,7 +154,7 @@ export class BotKnowledgeLedger { private save(next: StoredKnowledge): void { if (this.failed) throw new Error("Knowledge ledger requires recovery."); const value = { ...next, revision: this.stored.revision + 1 }; - if (Buffer.byteLength(JSON.stringify(value)) > MAX_BYTES) + if (Buffer.byteLength(JSON.stringify(value, null, 2)) + 1 > MAX_BYTES) throw new Error("Knowledge ledger is full."); try { mkdirSync(dirname(this.path), { recursive: true, mode: 0o700 }); diff --git a/apps/desktop/src/main/bot-team-catalog.ts b/apps/desktop/src/main/bot-team-catalog.ts index 8b5286b2..451e3fd3 100644 --- a/apps/desktop/src/main/bot-team-catalog.ts +++ b/apps/desktop/src/main/bot-team-catalog.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { chmodSync, existsSync, mkdirSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync } from "node:fs"; import { dirname, resolve } from "node:path"; import type { BotDefinition, @@ -9,6 +9,7 @@ import type { UpdateBotTeamInput, } from "@doolittle/contracts/bots"; import { writeJsonAtomicSync } from "@elizaos/agent/utils/atomic-json"; +import { readBoundedRegularFile } from "./regular-file"; const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/iu; const MAX_BYTES = 1_000_000; @@ -17,9 +18,7 @@ function date(value: unknown): boolean { } function parse(path: string): BotTeamCatalogResponse | null { if (!existsSync(path)) return null; - const raw = readFileSync(path, "utf8"); - if (Buffer.byteLength(raw) > MAX_BYTES) - throw new Error("Team catalog is too large."); + const raw = readBoundedRegularFile(path, MAX_BYTES).toString("utf8"); const data = JSON.parse(raw) as BotTeamCatalogResponse; if ( data?.version !== 1 || @@ -156,7 +155,7 @@ export class BotTeamCatalog { revision: this.stored.revision + 1, teams, }; - if (Buffer.byteLength(JSON.stringify(next)) > MAX_BYTES) + if (Buffer.byteLength(JSON.stringify(next, null, 2)) + 1 > MAX_BYTES) throw new Error("Team catalog is full."); try { mkdirSync(dirname(this.path), { recursive: true, mode: 0o700 }); diff --git a/docs/engineering/codebase-review-2026-10-25.md b/docs/engineering/codebase-review-2026-10-25.md new file mode 100644 index 00000000..949ae26b --- /dev/null +++ b/docs/engineering/codebase-review-2026-10-25.md @@ -0,0 +1,112 @@ +# Doolittle codebase review — October 25, 2026 + +**What this repo does:** Doolittle supplies desktop and terminal clients for an ElizaOS agent, including coding tools, conversations, background work, account connections, and wallet/trading tools. Both clients share the engine and application services. This review assumes one local operator, several bots and concurrent sessions, and recurring jobs left running for weeks. + +## Assessment + +This is a substantial, functioning alpha with real monorepo boundaries. It is beyond a prototype, but its current evidence does not establish a dependable replacement for a mature coding assistant across long sessions and every supported platform. The next work should strengthen recovery and long-running operation before adding more surface area. + +The workspace map is coherent. `apps/cli` and `apps/desktop` own client entry points. `packages/agent` owns the shared runtime, gateway, services, and terminal experience; `packages/contracts`, `packages/acp`, and `packages/ui` hold shared interfaces and client building blocks. Provider bridges live under `packages/plugins`; evaluation tooling has its own workspace. The monorepo checker enforces declared workspace dependencies, exports, import boundaries, and cycles. These are executable rules, not just a folder convention. + +```mermaid +flowchart TD + CLI[CLI / terminal chat] --> Agent[Shared application and engine host] + Desktop[Electron desktop] --> Main[Main process: IPC, project grants, bot lifecycle] + Main --> Agent + Agent --> SDK[ElizaOS runtime and plugins] + Agent --> Workers[Isolated bot workers] + SDK --> Providers[Model and provider transports] + SDK --> Tools[Coding, browser, daily assistant, wallet tools] + Agent --> State[Native memories and local projections] + Main --> Ledgers[Desktop ownership and dispatch ledgers] + Contracts[Shared contracts and ACP] -.-> CLI + Contracts -.-> Desktop +``` + +### What is strong + +- **One engine for both clients.** CLI and desktop converge on the shared application and ElizaOS engine host. Doolittle is not maintaining two independent agents. +- **Ownership is taken seriously.** Bot/session attribution, explicit workspace approval, request scopes, worker admission limits, and durable dispatch identities have concrete implementations and tests. Historical conversation data is not automatically treated as filesystem consent. +- **Failure is usually explicit.** Conflicting ledger copies, missing capabilities, unavailable tools, and unsupported execution paths are refused or labeled. Native capability reporting distinguishes a working transport from a verified capability. +- **Release controls are substantial.** CI checks types, boundaries, documentation, dependencies, source workers, native Electron behavior, and clean-HEAD packaged acceptance. Release workflows also handle provenance and platform packaging. +- **The development loop is usable.** The desktop watcher rebuilds and restarts its own main process/runtime; the renderer uses hot reload. Testing changes does not require reinstalling the desktop app. +- **Evaluation is more than a model saying it succeeded.** Coding evaluation inspects mutation, build, runtime, and handoff receipts. Live client checks verify a repaired source fixture externally. These are useful foundations, although they cover much less than the full product promise. + +### What changed in this pass + +Focused commits address real persistence problems: + +1. Seven desktop state readers now reuse the existing bounded regular-file reader. Limits apply before allocating the entire file; symlinked primary files are rejected and supported backup recovery is preserved. +2. Bot catalog changes become visible only after their durable write succeeds. JSON size budgets now measure the actual pretty-printed on-disk representation; conversation writes also enforce their read budget before replacing either copy. +3. A valid legacy conversation copy stays readable when its normalized repair would exceed the write budget. Recovery leaves that source untouched and new writes still fail closed. +4. Automation and consultation ledger parsers accept canonical native Windows workspace paths. They previously accepted such records on write and rejected them on reload. + +The new regression cases were run against the old behavior first. Seven unsafe-read cases, four failed-catalog-publication cases, and two Windows-path cases failed before their fixes. Additional tests cover valid reads, backup recovery, and the serialized-size boundary. The Windows-path tests emulate native path semantics on macOS; they are not a native Windows acceptance run. + +## Must fix + +1. **Recurring desktop jobs eventually exhaust a permanent dispatch history** (`apps/desktop/src/main/bot-automation-ledger.ts:156`, `apps/desktop/src/main/bot-automation-ledger.ts:209`). + - **What this is:** The desktop records each target-bot automation dispatch before submitting it, preserving identity and duplicate prevention across restarts. + - **Problem:** The ledger refuses more than 5,000 fires or 8 MB, and has no retention or archival operation. A five-minute recurring job reaches 5,000 fires in about 17.4 days; the byte limit can arrive sooner, and completed records remain in the same array. + - **Fix:** Add explicit archival of terminal records while preserving the durable duplicate-prevention contract. Keep active records and a compact, bounded replay history; define when an old fire identity can no longer be replayed before deleting it. + - **If we skip it:** A normal long-running assistant eventually stops admitting new target-bot jobs and needs manual state intervention. Refusing the run is safer than losing its identity, but it is still an operational failure. + +## Should fix + +2. **Conversation replacement needs direct recovery tests** (`packages/agent/src/runtime/chat-turn/conversation-persistence.ts:769`). + - **What this is:** Context replacement writes an Eliza summary, snapshots and deletes replaced native memories, then lets the caller update the local conversation projection. Compensation restores the old state when a step fails. + - **Problem:** The maintained test tree does not directly exercise the replacement/replay rollback helpers. A failure after some native deletions, or a failure updating the projection after native success, is a high-cost history-consistency case left without a focused regression journey. + - **Fix:** Add failure-injection tests around a real conversation replacement: summary write failure, partial deletion failure, projection failure, and failed compensation. Assert both authorities after restart and check that an incomplete rollback is reported. + - **If we skip it:** Ordinary chat tests can stay green while a rare compression or replay failure damages a user's recoverable history. This is a coverage gap, not a reproduced data-loss incident. + +3. **Provider CLI output has no memory budget** (`packages/plugins/provider-transport/src/command.ts:136`). + - **What this is:** Claude and Devin inference bridges collect child-process stdout and stderr until exit or timeout. + - **Problem:** Both streams append to strings without a byte limit. A faulty or very noisy provider can emit hundreds of megabytes before its 120-second invocation deadline; the deadline does not bound memory consumption. + - **Fix:** Count combined output bytes, terminate the owned process when a documented response budget is exceeded, and return a bounded diagnostic. Add a subprocess test that exceeds the limit on each stream. + - **If we skip it:** A broken provider can exhaust a worker's memory or force large allocations while reporting an error. This finding comes from the implementation; no out-of-memory stress run was performed. + +4. **Windows provider cancellation only kills the immediate child** (`packages/plugins/provider-transport/src/command.ts:46`). + - **What this is:** The shared provider runner attempts to terminate its process tree on cancellation or deadline. + - **Problem:** POSIX launches a separate process group and kills that group. On Windows it calls only `child.kill()`, which does not terminate descendant processes; a provider launcher that starts a worker can leave that worker running and inherited pipes open after timeout. + - **Fix:** Use an owned Windows process-tree termination mechanism and test a launcher with a child that holds stdout open. Assert that cancellation settles and both owned processes stop. + - **If we skip it:** A Windows inference request may outlive its deadline or leave provider work running after the user cancels. This is a source-level platform finding, not a reproduced Windows failure. + +## Dependency and maintenance position + +The fresh production audit covers 38 locked dependency roots and 1,739 lockfile packages and reports **zero advisories at every severity**. That is a good current result, not a guarantee against unpublished defects or future advisories. + +The goal of a patch-free dependency tree is **not complete**. Seven package patches and three declared SDK compatibility workspaces remain. Their ownership and upstream requirements are documented in [SDK package upgrades](sdk-package-upgrades.md). Core compatibility remains on `2.0.3-beta.7`; independently usable packages have explicit newer versions. Removing these patches without replacement behavior would weaken state safety, tool ownership, or retained wallet functionality. + +The large composition files remain a maintenance cost: desktop `ChatPage.tsx`, `App.tsx`, and the bot process registry carry substantial lifecycle and coordination logic. Their size alone is not a bug. Future extraction should follow ownership boundaries such as run-event recovery or worker lifecycle, with a behavior test before moving code. Another wholesale rewrite would add risk without addressing the concrete findings above. + +The review map counted approximately 295,000 TypeScript/JavaScript source and test lines in 2,246 files at the starting revision, excluding generated outputs and installed dependencies. This is a large product for a local assistant. Keep the architecture checks and narrow commits; do not treat a clean folder layout as evidence that the runtime is simple. + +## Verification in this pass + +- All tooling and workspace type checks: passed. +- Fast boundary suite: **161 tests in 28 files passed**. +- Formatting/lint: passed. +- Monorepo, hygiene, workflow security, plugin boundaries, SDK declarations, documentation truth, local links, unused dependencies: passed. +- Production advisory policy: passed, zero advisories at every severity. +- Source-worker acceptance: **7 tests in 3 files passed**. +- Native desktop workbench: **4 tests passed**, covering first chat, grants across restart, denied cross-bot/root access, and stable catalog polling. +- Live coding: **both CLI and desktop passed** against the real linked Codex provider through ElizaOS, with source repair independently verified (38.5 seconds and 43.3 seconds). These runs preceded the final legacy-copy recovery correction; that correction is covered by the new failure regressions and final branch CI. +- The first Linux CI run failed the fresh-bot composer check at its default five-second assertion timeout; the other three native journeys passed. That journey now waits up to 30 seconds for the independently started bot worker, within its unchanged 60-second overall deadline. It does not retry or skip the assertion. +- Packaged installer/signing acceptance was not rerun locally in this pass. PR CI runs its own packaged acceptance. + +## Readiness + +| Area | Assessment | +| ----------------------------- | --------------------------------------------------------------------------------------------------------------------------- | +| Monorepo organization | Coherent and enforced; preserve the shared engine boundary. | +| CLI and desktop | Real clients with meaningful executable acceptance; full daily-use reliability is not established. | +| Security and state boundaries | Strong deliberate controls; this pass corrected several persistence defects. This was not a comprehensive penetration test. | +| Long-running assistance | Needs dispatch-history retention and stronger recovery evidence. | +| Wallet/trading | Retained; this review did not submit transactions or verify trading profitability. | +| Dependencies | Fresh production advisory audit clean; seven patches and three compatibility workspaces remain. | +| Coding quality | Real coding canaries and evaluation infrastructure exist; no fresh representative long-task benchmark cohort was run here. | +| Release status | Source validation is separate from signed releases, native Windows operation, and installation acceptance. | + +**Verdict:** A credible working alpha. Fix durable dispatch retention first, then test conversation recovery and harden provider process limits before calling it a dependable daily-driver replacement. + +**Not checked:** This was a repo-wide map and risk-based source review, not a line-by-line read of all 2,246 files. Deep inspection concentrated on runtime entry points, worker/server ownership, desktop persistence and project access, conversation persistence, provider process transport, and build/test/release controls. UI routes, service families, ACP, evaluation tooling, and support packages were sampled; native Rust internals were not reviewed; every command, skill document, plugin integration, and UI interaction was not exercised. No full visual/accessibility audit, native Windows/macOS signing assessment, live wallet transaction, multi-day soak, exhaustive malicious-filesystem race test, or representative competitor benchmark was performed. Mutable ancestor-directory replacement is outside the bounded file reader's guarantee. diff --git a/tests/boundary/catalog-publication.test.ts b/tests/boundary/catalog-publication.test.ts new file mode 100644 index 00000000..0ac01d50 --- /dev/null +++ b/tests/boundary/catalog-publication.test.ts @@ -0,0 +1,151 @@ +import { mkdirSync, mkdtempSync, renameSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; +import { afterEach, expect, it } from "vitest"; +import { BotCatalog } from "../../apps/desktop/src/main/bot-catalog"; + +const dirs: string[] = []; +afterEach(() => { + for (const dir of dirs.splice(0)) + rmSync(dir, { recursive: true, force: true }); +}); +function fixture() { + const dir = mkdtempSync(resolve(tmpdir(), "doolittle-catalog-publication-")); + dirs.push(dir); + const backend = { + getState: () => ({ + phase: "ready", + agentId: "00000000-0000-4000-8000-000000000001", + name: "Lead", + }), + getWorkspaceDirectory: () => dir, + } as ConstructorParameters[1]; + const catalog = new BotCatalog(dir, backend, dir); + catalog.defaultBot(); + const saved = catalog.create({ + name: "Saved", + persona: "Help", + model: { provider: "offline", model: "offline" }, + }); + const path = resolve(dir, "bots/catalog.json"); + renameSync(path, `${path}.saved`); + mkdirSync(path); + return { catalog, saved }; +} +it("does not publish a bot whose creation could not be saved", () => { + const { catalog } = fixture(); + expect(() => + catalog.create({ + name: "Phantom", + persona: "Help", + model: { provider: "offline", model: "offline" }, + }), + ).toThrow(); + expect(catalog.list(new Map()).bots.map((bot) => bot.name)).not.toContain( + "Phantom", + ); +}); +it("keeps the previous settings after a failed update", () => { + const { catalog, saved } = fixture(); + expect(() => catalog.update(saved.id, { name: "Unsaved" })).toThrow(); + expect(catalog.get(saved.id)?.name).toBe("Saved"); +}); +it("keeps the bot available after a failed archive", () => { + const { catalog, saved } = fixture(); + expect(() => catalog.archive(saved.id)).toThrow(); + expect(catalog.get(saved.id)?.archivedAt).toBeUndefined(); +}); +it("keeps the lead display name after a failed rename", () => { + const { catalog } = fixture(); + expect(() => catalog.update("default", { name: "Unsaved lead" })).toThrow(); + expect(catalog.defaultBot().name).toBe("Lead"); +}); + +it("refuses a pretty-printed ACP update before replacing the last readable ledger", async () => { + const { BotAcpSessionLedger } = await import( + "../../apps/desktop/src/main/bot-acp-session-ledger" + ); + const { readFileSync, writeFileSync } = await import("node:fs"); + const dir = mkdtempSync(resolve(tmpdir(), "doolittle-ledger-budget-")); + dirs.push(dir); + mkdirSync(resolve(dir, "bots")); + const createdAt = new Date().toISOString(); + const old = { + sessionId: "old", + botId: "bot", + originConversationId: "origin", + workspacePath: "/", + createdAt, + }; + const added = { + sessionId: "new", + botId: "bot", + originConversationId: "origin", + workspacePath: "/project", + createdAt, + }; + const seed = { version: 1, revision: 0, sessions: [old] }; + const next = { version: 1, revision: 1, sessions: [old, added] }; + old.workspacePath += "x".repeat( + 2_000_000 - Buffer.byteLength(JSON.stringify(next)) - 1, + ); + const before = `${JSON.stringify(seed, null, 2)}\n`; + expect(Buffer.byteLength(before)).toBeLessThanOrEqual(2_000_000); + expect(Buffer.byteLength(JSON.stringify(next))).toBeLessThanOrEqual( + 2_000_000, + ); + expect(Buffer.byteLength(JSON.stringify(next, null, 2)) + 1).toBeGreaterThan( + 2_000_000, + ); + const path = resolve(dir, "bots", "acp-session-owners.json"); + writeFileSync(path, before); + const ledger = new BotAcpSessionLedger(dir); + expect(() => + ledger.bind({ + sessionId: "new", + botId: "bot", + originConversationId: "origin", + workspacePath: "/project", + }), + ).toThrow(/full/); + expect(readFileSync(path, "utf8")).toBe(before); + expect(new BotAcpSessionLedger(dir).get("new")).toBeNull(); +}); + +for (const copy of ["primary", "backup"]) { + it(`recovers a full compact legacy conversation ${copy} without an oversized repair`, async () => { + const { BotConversationLedger } = await import( + "../../apps/desktop/src/main/bot-conversation-ledger" + ); + const { existsSync, readFileSync, writeFileSync } = await import("node:fs"); + const dir = mkdtempSync(resolve(tmpdir(), "doolittle-legacy-recovery-")); + dirs.push(dir); + mkdirSync(resolve(dir, "bots")); + const stamp = "2026-10-25T00:00:00.000Z"; + const conversations = Array.from({ length: 25_000 }, (_, index) => ({ + sessionId: `session-${index}`, + botId: "bot", + createdAt: stamp, + updatedAt: stamp, + })); + const legacy = { version: 1, conversations }; + const raw = JSON.stringify(legacy); + expect(Buffer.byteLength(raw)).toBeLessThanOrEqual(4_000_000); + expect(Buffer.byteLength(JSON.stringify(legacy, null, 2))).toBeGreaterThan( + 4_000_000, + ); + const primary = resolve(dir, "bots/conversation-owners.json"); + const backup = resolve(dir, "bots/conversation-owners.backup.json"); + const source = copy === "primary" ? primary : backup; + const missing = copy === "primary" ? backup : primary; + writeFileSync(source, raw); + const ledger = new BotConversationLedger(dir); + expect(ledger.get("session-0")?.botId).toBe("bot"); + expect(ledger.get("session-24999")?.botId).toBe("bot"); + expect(existsSync(missing)).toBe(false); + expect(() => ledger.bind("bot", "new-session")).toThrow(/full/); + expect(ledger.get("new-session")).toBeNull(); + expect(readFileSync(source, "utf8")).toBe(raw); + expect(new BotConversationLedger(dir).get("session-0")?.botId).toBe("bot"); + }); +} diff --git a/tests/boundary/desktop-ledger-windows.test.ts b/tests/boundary/desktop-ledger-windows.test.ts new file mode 100644 index 00000000..c4a041e6 --- /dev/null +++ b/tests/boundary/desktop-ledger-windows.test.ts @@ -0,0 +1,97 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; +import { afterEach, expect, it, vi } from "vitest"; + +// Exercise Windows validation while keeping the fixture files on this host. +vi.mock("node:path", async () => { + const native = await vi.importActual("node:path"); + return { + ...native, + isAbsolute: (path: string) => + /^[A-Za-z]:/u.test(path) + ? native.win32.isAbsolute(path) + : native.isAbsolute(path), + resolve: (...paths: string[]) => + paths.length === 1 && /^[A-Za-z]:/u.test(paths[0] ?? "") + ? native.win32.resolve(...paths) + : native.resolve(...paths), + }; +}); + +import { BotAutomationLedger } from "../../apps/desktop/src/main/bot-automation-ledger"; +import { BotConsultationLedger } from "../../apps/desktop/src/main/bot-consultation-ledger"; + +const dirs: string[] = []; +afterEach(() => { + for (const dir of dirs.splice(0)) + rmSync(dir, { recursive: true, force: true }); +}); +function fixture(file: string, value: unknown): string { + const dir = mkdtempSync(resolve(tmpdir(), "doolittle-windows-ledger-")); + dirs.push(dir); + mkdirSync(resolve(dir, "bots")); + writeFileSync(resolve(dir, "bots", file), JSON.stringify(value)); + return dir; +} +const workspacePath = "C:\\Users\\operator\\workspace"; +const stamp = new Date().toISOString(); +const permissions = { + connectionIds: [], + workspacePaths: [workspacePath], + toolIds: [], + allowMutation: true, + allowDelegation: true, +}; +it("restores an automation approval with a canonical Windows workspace", () => { + const id = "00000000-0000-4000-8000-000000000001"; + const approval = { + id, + jobId: id, + targetBotId: id, + targetRevision: stamp, + definitionDigest: "a".repeat(64), + permissions, + workspacePath, + createdAt: stamp, + }; + const dir = fixture("automation-dispatches.json", { + version: 1, + revision: 0, + approvals: [approval], + fires: [], + }); + expect(new BotAutomationLedger(dir).approval(id)?.workspacePath).toBe( + workspacePath, + ); +}); +it("restores a consultation with a canonical Windows workspace", () => { + const record = { + dispatchId: "dispatch-1", + rootRunId: "root-1", + ancestry: [], + depth: 1, + origin: { + runId: "origin-run", + sessionId: "origin-session", + botId: "bot-a", + }, + targetBotId: "bot-b", + targetSessionId: "target-session", + targetRunId: "target-run", + permissions, + workspacePath, + deadline: stamp, + status: "prepared", + createdAt: stamp, + updatedAt: stamp, + }; + const dir = fixture("consultations.json", { + version: 1, + revision: 0, + records: [record], + }); + expect( + new BotConsultationLedger(dir).get(record.dispatchId)?.workspacePath, + ).toBe(workspacePath); +}); diff --git a/tests/boundary/desktop-state-read.test.ts b/tests/boundary/desktop-state-read.test.ts new file mode 100644 index 00000000..9c7b4bb5 --- /dev/null +++ b/tests/boundary/desktop-state-read.test.ts @@ -0,0 +1,118 @@ +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + truncateSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { BotAcpSessionLedger } from "../../apps/desktop/src/main/bot-acp-session-ledger"; +import { BotAutomationLedger } from "../../apps/desktop/src/main/bot-automation-ledger"; +import { BotCatalog } from "../../apps/desktop/src/main/bot-catalog"; +import { BotConsultationLedger } from "../../apps/desktop/src/main/bot-consultation-ledger"; +import { BotConversationLedger } from "../../apps/desktop/src/main/bot-conversation-ledger"; +import { BotKnowledgeLedger } from "../../apps/desktop/src/main/bot-knowledge-ledger"; +import { BotTeamCatalog } from "../../apps/desktop/src/main/bot-team-catalog"; + +const directories: string[] = []; +afterEach(() => { + for (const dir of directories.splice(0)) + rmSync(dir, { recursive: true, force: true }); +}); +const ledgers = [ + { + name: "conversation-owners", + limit: 4_000_000, + empty: { version: 1, conversations: [] }, + open: (dir: string) => new BotConversationLedger(dir), + }, + { + name: "acp-session-owners", + limit: 2_000_000, + empty: { version: 1, revision: 0, sessions: [] }, + open: (dir: string) => new BotAcpSessionLedger(dir), + }, + { + name: "automation-dispatches", + limit: 8_000_000, + empty: { version: 1, revision: 0, approvals: [], fires: [] }, + open: (dir: string) => new BotAutomationLedger(dir), + }, + { + name: "consultations", + limit: 8_000_000, + empty: { version: 1, revision: 0, records: [] }, + open: (dir: string) => new BotConsultationLedger(dir), + }, + { + name: "knowledge", + limit: 4_000_000, + empty: { version: 2, revision: 0, records: [], grants: [] }, + open: (dir: string) => new BotKnowledgeLedger(dir), + }, + { + name: "teams", + limit: 1_000_000, + empty: { version: 1, revision: 0, teams: [] }, + open: (dir: string) => + new BotTeamCatalog(dir, () => { + throw new Error("unexpected bot lookup"); + }), + }, + { + name: "catalog", + limit: 2_000_000, + empty: { version: 1, bots: [] }, + open: (dir: string) => + new BotCatalog( + dir, + {} as ConstructorParameters[1], + dir, + ), + }, +]; +function fixture(name: string) { + const dir = mkdtempSync(resolve(tmpdir(), "doolittle-state-read-")); + directories.push(dir); + mkdirSync(resolve(dir, "bots")); + return { + dir, + primary: resolve(dir, "bots", `${name}.json`), + backup: resolve(dir, "bots", `${name}.backup.json`), + }; +} +for (const ledger of ledgers) + describe(ledger.name, () => { + it("reads a valid regular file", () => { + const f = fixture(ledger.name); + writeFileSync(f.primary, JSON.stringify(ledger.empty)); + expect(() => ledger.open(f.dir)).not.toThrow(); + }); + it("refuses a linked primary instead of importing another file as authority", () => { + const f = fixture(ledger.name); + const other = resolve(f.dir, "other.json"); + writeFileSync(other, JSON.stringify(ledger.empty)); + symlinkSync(other, f.primary); + expect(() => ledger.open(f.dir)).toThrow(); + expect(JSON.parse(readFileSync(other, "utf8"))).toEqual(ledger.empty); + }); + it("rejects an oversized regular file", () => { + const f = fixture(ledger.name); + writeFileSync(f.primary, "{}"); + truncateSync(f.primary, ledger.limit + 1); + expect(() => ledger.open(f.dir)).toThrow(); + }); + if (ledger.name !== "catalog") + it("recovers from a rejected primary using its valid regular backup", () => { + const f = fixture(ledger.name); + const other = resolve(f.dir, "other.json"); + writeFileSync(other, JSON.stringify(ledger.empty)); + symlinkSync(other, f.primary); + writeFileSync(f.backup, JSON.stringify(ledger.empty)); + expect(() => ledger.open(f.dir)).not.toThrow(); + }); + }); diff --git a/tests/e2e/desktop-core.pw.ts b/tests/e2e/desktop-core.pw.ts index 00e6075d..e061b4d9 100644 --- a/tests/e2e/desktop-core.pw.ts +++ b/tests/e2e/desktop-core.pw.ts @@ -73,7 +73,10 @@ test.describe("fresh native desktop", () => { const composer = page.getByRole("textbox", { name: "Message Fresh Scout", }); - await expect(composer).toBeEnabled(); + // Creating a named bot starts a second cold SDK worker after the lead + // runtime is ready. Wait for its observable readiness, not the default + // five-second assertion budget; the journey still has its 60s deadline. + await expect(composer).toBeEnabled({ timeout: 30_000 }); await composer.fill("A short offline hello"); await composer.press("Enter"); await expect(