orchestrator implementation + persistence layer by Kitenite · Pull Request #98 · ParadevAI/Paradev

Kitenite · 2025-11-18T07:32:14Z

add change types
add persistence and implementation layer

Summary by CodeRabbit

New Features
- Local CLI persistence for environments, workspaces, processes, changes, file diffs and agent summaries with cascade deletion
- Process lifecycle management (create, stop, stop-all) and workspace/environment CRUD
- Configurable storage location via environment variable
Tests
- Extensive unit tests covering full CRUD lifecycles, cascade behavior, date serialization, and concurrent writes
Documentation
- Usage guides, examples, architecture overview, and migration notes for the persistence layer

coderabbitai · 2025-11-18T07:32:27Z

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a JSON-backed persistence layer (StorageAdapter + LowdbAdapter), storage types/config, orchestrators for environments/workspaces/processes/changes with cascade-delete behavior, comprehensive tests and docs, plus dependency and minor import/tsconfig adjustments.

Changes

Cohort / File(s)	Change Summary
Dependencies & TS config `apps/cli/package.json`, `apps/cli/tsconfig.json`, `packages/scripts/tsconfig.json`, `packages/typescript-config/base.json`	Added runtime dependency `lowdb@^7.0.1` and devDependency `bun-types@^1.3.2`; adjusted TypeScript `types` entries across configs (added `bun-types` in base.json; removed in packages/scripts tsconfig; removed explicit empty `types` in apps/cli tsconfig).
Storage API & Config `apps/cli/src/lib/storage/adapter.ts`, `apps/cli/src/lib/storage/config.ts`, `apps/cli/src/lib/storage/index.ts`	Introduced `StorageAdapter` interface, storage path utilities (`getStorageDir`, `getDbPath`, `ensureStorageDir`), and barrel exports for storage types/adapters.
Lowdb Adapter & Types `apps/cli/src/lib/storage/lowdb-adapter.ts`, `apps/cli/src/lib/storage/types.ts`	Implemented `LowdbAdapter` with file-backed JSON persistence, date serialization/deserialization, lazy init, collection/item CRUD, clear; added `Database`/`SerializedDatabase` schema, `Serialized<T>` utility, and `createEmptyDatabase`.
Storage Tests `apps/cli/src/lib/storage/__tests__/lowdb-adapter.test.ts`	Added tests covering init, CRUD, collection ops, date serialization, concurrent writes, and per-test temp DB isolation.
Orchestrators Implementation `apps/cli/src/lib/orchestrators/*.ts`, `apps/cli/src/lib/orchestrators/index.ts`	Added Change, Environment, Process, and Workspace orchestrators implementing CRUD, ID/timestamp generation, agent-specific handling, and cascade-delete helpers; added barrel export.
Orchestrator Tests `apps/cli/src/lib/orchestrators/__tests__/*`	Added comprehensive test suites for Change, Environment, Process, and Workspace orchestrators validating create/get/list/update/delete, cascade deletions, and storage interactions using temporary Lowdb stores.
Type Exports `apps/cli/src/types/index.ts`	Exported `./change` and reordered `workspace` export after `process`.
Persistence Documentation `apps/cli/src/lib/persistence_docs/*`	Added PLAN.md, SUMMARY.md, EXAMPLE.md documenting architecture, data model, cascade rules, usage examples, testing guidance, and migration notes.
Minor Import Reordering & misc `apps/cli/server.ts`, `apps/cli/src/cli.tsx`, `apps/cli/test.js`	Reordered some imports (no behavioral changes).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Orch as Orchestrator
    participant Store as StorageAdapter
    participant Lowdb as LowdbAdapter
    participant FS as Filesystem

    Client->>Orch: create(entity)
    Orch->>Orch: generate id, createdAt
    Orch->>Store: set(collection, id, entity)
    Store->>Lowdb: set(...)
    Lowdb->>Lowdb: init() / deserializeDates()
    Lowdb->>FS: read db.json
    Lowdb->>Lowdb: serializeDates()
    Lowdb->>FS: write db.json
    Lowdb-->>Store: ok
    Store-->>Orch: ok
    Orch-->>Client: entity(with id)

sequenceDiagram
    participant Client
    participant WS as WorkspaceOrchestrator
    participant Store as StorageAdapter
    participant Lowdb as LowdbAdapter
    participant FS as Filesystem

    Client->>WS: delete(workspaceId)

    rect rgb(220,240,255)
    Note over WS,Store: Cascade: for each process -> delete agentSummaries then process
    WS->>Store: getCollection("processes")
    WS->>Store: delete("agentSummaries", ...) / delete("processes", ...)
    end

    rect rgb(240,220,255)
    Note over WS,Store: Cascade: for each change -> delete fileDiffs then change
    WS->>Store: getCollection("changes")
    WS->>Store: delete("fileDiffs", ...) / delete("changes", ...)
    end

    rect rgb(255,235,220)
    Note over WS,Store: Finally delete workspace
    WS->>Store: delete("workspaces", workspaceId)
    end

    Store->>Lowdb: persist
    Lowdb->>FS: write db.json
    Lowdb-->>Store: ok
    Store-->>WS: ok
    WS-->>Client: ok

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas needing extra attention:
- LowdbAdapter: date (de)serialization, init/lazy behavior, concurrent writes and file IO error handling.
- Cascade-delete logic in Environment/Workspace/Change/Process orchestrators: deletion order, agentSummary cleanup, and isolation (no cross-workspace leakage).
- Storage types mapping (Serialized<T> and Serialized* aliases) vs. adapter serialization.
- Test suites: ensure temp DB isolation and timestamp/assertion correctness.

Possibly related PRs

Add more types #97 — Adds Environment/Process/Workspace type definitions and reorganizes type exports; strongly related to the new orchestrator implementations and type usage.

Poem

🐰 I hopped through JSON, nose to the key,
I planted IDs where timestamps could be,
Orchestrators hum, Lowdb keeps the score,
Cascades tidy up every door,
Hooray — data safe, and gardens are free! 🌿

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is minimal and lacks required template sections. It only lists two bullet points without providing context, related issues, type of change, testing details, or additional notes as specified in the template.	Expand the description to include all template sections: provide clear context for the changes, link related issues, specify the type of change, document testing performed, and add any additional context needed for reviewers.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'orchestrator implementation + persistence layer' accurately reflects the main changes in the PR, which include four new orchestrator classes and a complete persistence layer with storage adapters.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62fb568 and d117ab2.

📒 Files selected for processing (3)

apps/cli/tsconfig.json (1 hunks)
packages/scripts/tsconfig.json (1 hunks)
packages/typescript-config/base.json (1 hunks)

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/cli/package.json (1)

1-59: Fix three critical test failures blocking the PR.

The tests are failing because of incompatibilities in test file setup:

apps/cli/test.js (line 2): Imports "./source/app.js" which doesn't exist. The app is at ./src/app.tsx. Change to import App from "./src/app.tsx";

All 5 test files in apps/cli/src/tests/: Import from "bun:test" framework, but package.json test script runs AVA. Convert these to use AVA imports (import test from "ava") and AVA syntax (test() instead of describe() and test()), or switch the test runner to Bun in package.json.

The test command runs AVA but encounters bun:test syntax, causing it to fail with exit code 1.

🧹 Nitpick comments (9)

apps/cli/src/lib/orchestrators/__tests__/environment-orchestrator.test.ts (1)

1-10: Fix import-order lint warning via formatter

CI reported lint/source/organizeImports for this file. The current imports are already reasonably grouped, so this is likely just a formatting rule (e.g., sort alphabetically or group by module type). Rather than hand-tuning, I’d suggest running the project’s lint/format command locally (or bun lint --fix if available) to auto-fix the ordering here.

apps/cli/src/lib/orchestrators/change-orchestrator.ts (1)

45-58: Consider sharing file-diff cascade logic with workspace/environment orchestrators

The delete implementation performs a fileDiff cascade that is structurally similar to what WorkspaceOrchestrator.delete and EnvironmentOrchestrator.deleteWorkspaceCascade already do when removing changes. To reduce duplication and keep cascade rules in sync, you may want to centralize this fileDiff deletion logic (e.g., a shared helper or a small internal utility) and call it from all three locations.
apps/cli/PERSISTENCE_PLAN.md (1)
12-31: Align markdown fences and relationship diagram with implementation

Two small doc nits:
MD040 fenced-code warnings
The tree and relationship diagrams use bare triple backticks without a language. To satisfy markdownlint and improve readability, consider annotating them, e.g.:
- +text
apps/cli/src/
...
...
-``` 
+```text
Environment (id)
...
AgentSummary parent in the diagram
The “Type Relationships” section currently shows AgentSummary (agentId) as a child of Change, but the code cascades agentSummaries from Process (see ProcessOrchestrator.delete and the workspace/environment cascade helpers that filter by summary.agentId === processId). To match the implementation, you may want something like:
Environment (id)
  └── Workspace (environmentId)
      ├── Process (workspaceId)
      │   ├── Agent/Terminal
      │   └── AgentSummary (agentId)
      └── Change (workspaceId)
          └── FileDiff (changeId)
Also applies to: 80-88
apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1)
65-111: Reduce duplication and repeated collection reads in cascade delete

The delete method correctly enforces cascade behavior, but there are two maintainability/perf concerns:

The cascade logic (processes → agentSummaries, changes → fileDiffs) is nearly identical to EnvironmentOrchestrator.deleteWorkspaceCascade, so any future change must be replicated in two places.

Within the loops, getCollection("agentSummaries") and getCollection("fileDiffs") are called for each process/change ID, which repeatedly reads the full collection.

Consider:

Extracting a shared internal helper (e.g., in a small cascade.ts module or a private utility on a common base) that both orchestrators can reuse.

Hoisting the getCollection calls out of the loops, then filtering in-memory for each ID:
const agentSummaries = await this.storage.getCollection("agentSummaries");
// compute summaryIds per processId from this one snapshot
This keeps behavior the same while reducing duplication and I/O.
apps/cli/src/lib/orchestrators/environment-orchestrator.ts (1)

37-51: Consolidate workspace cascade logic and avoid repeated full-collection reads

The environment orchestrator’s delete path is functionally sound and respects the cascade rules, but:

deleteWorkspaceCascade is almost a copy of WorkspaceOrchestrator.delete, which risks divergence if cascade behavior changes in one place but not the other.

Inside the process/change loops, getCollection("agentSummaries") and getCollection("fileDiffs") are called for each ID, repeatedly loading whole collections.

To improve maintainability and efficiency, consider:

Extracting shared cascade helpers (e.g., deleteWorkspaceWithChildren(storage, workspaceId)) and calling those from both EnvironmentOrchestrator and WorkspaceOrchestrator.

Hoisting getCollection("agentSummaries") / getCollection("fileDiffs") outside of the per-ID loops and filtering from a single snapshot.

The external behavior stays identical but will be easier to evolve.

Also applies to: 53-67, 69-116
apps/cli/src/lib/orchestrators/process-orchestrator.ts (2)
79-115: Prefer using type to detect agents instead of "status" in updated

Using "status" in updated works today because only agents have a status field, but it’s a bit brittle if other process types ever gain a status-like property.

You can make the intent clearer and safer by checking type:
-		// Update status for agents
-		if ("status" in updated) {
-			(updated as Agent).status = "stopped";
-		}
+		// Update status for agents
+		if (existing.type === ProcessType.AGENT) {
+			(updated as Agent).status = "stopped";
+		}
and similarly in stopAll, use process.type === ProcessType.AGENT to decide whether to set status.

117-130: Batch agent summary deletions to reduce redundant reads/writes

delete calls storage.delete once per related agentSummary. With the current LowdbAdapter, each delete incurs a full read + write of the JSON file, which can be expensive if a process has many summaries.

A more efficient pattern is:

Load agentSummaries once.

Remove matching entries in-memory.

Call updateCollection("agentSummaries", updatedSummaries) once.

This keeps behavior the same while reducing I/O.
apps/cli/src/lib/storage/types.ts (1)

37-44: Consider future‑proofing Serialized<T> for Date | null fields

Right now the conditional covers Date and Date | undefined, which matches current models. If you later introduce Date | null fields, they won't be serialized as strings by this helper.

If that becomes relevant, you could extend the condition to include Date | null as well.

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

93-167: Potential I/O optimization and concurrency refinement (non‑blocking)

Each mutating method (set, delete, per‑entity operations in orchestrators) does read → mutate → write. With higher write volumes, you may want to:

Batch logical operations at the orchestrator level (e.g., deleting many summaries with a single updateCollection), and/or

Introduce a simple initPromise to ensure only one JSONFilePreset instance is constructed even under concurrent calls.

Not required for correctness, but worth considering if CLI usage grows.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d59e2a and cd0cdf4.

⛔ Files ignored due to path filters (1)

bun.lock is excluded by !**/*.lock

📒 Files selected for processing (18)

apps/cli/PERSISTENCE_PLAN.md (1 hunks)
apps/cli/package.json (1 hunks)
apps/cli/src/lib/orchestrators/__tests__/change-orchestrator.test.ts (1 hunks)
apps/cli/src/lib/orchestrators/__tests__/environment-orchestrator.test.ts (1 hunks)
apps/cli/src/lib/orchestrators/__tests__/process-orchestrator.test.ts (1 hunks)
apps/cli/src/lib/orchestrators/__tests__/workspace-orchestrator.test.ts (1 hunks)
apps/cli/src/lib/orchestrators/change-orchestrator.ts (1 hunks)
apps/cli/src/lib/orchestrators/environment-orchestrator.ts (1 hunks)
apps/cli/src/lib/orchestrators/index.ts (1 hunks)
apps/cli/src/lib/orchestrators/process-orchestrator.ts (1 hunks)
apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1 hunks)
apps/cli/src/lib/storage/__tests__/lowdb-adapter.test.ts (1 hunks)
apps/cli/src/lib/storage/adapter.ts (1 hunks)
apps/cli/src/lib/storage/config.ts (1 hunks)
apps/cli/src/lib/storage/index.ts (1 hunks)
apps/cli/src/lib/storage/lowdb-adapter.ts (1 hunks)
apps/cli/src/lib/storage/types.ts (1 hunks)
apps/cli/src/types/index.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (12)

apps/cli/src/lib/storage/__tests__/lowdb-adapter.test.ts (4)

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

LowdbAdapter (14-168)

apps/cli/src/types/environment.ts (1)

Environment (1-3)

apps/cli/src/types/process.ts (1)

Process (8-18)

apps/cli/src/types/change.ts (1)

Change (1-6)

apps/cli/src/lib/orchestrators/environment-orchestrator.ts (2)

apps/cli/src/lib/storage/adapter.ts (1)

StorageAdapter (7-70)

apps/cli/src/types/environment.ts (1)

Environment (1-3)

apps/cli/src/lib/storage/adapter.ts (1)

apps/cli/src/lib/storage/types.ts (1)

Database (15-22)

apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (2)

apps/cli/src/lib/storage/adapter.ts (1)

StorageAdapter (7-70)

apps/cli/src/types/workspace.ts (1)

LocalWorkspace (12-15)

apps/cli/src/lib/storage/types.ts (3)

apps/cli/src/types/environment.ts (1)

Environment (1-3)

apps/cli/src/types/process.ts (2)

Process (8-18)

Agent (29-32)

apps/cli/src/types/change.ts (2)

Change (1-6)

AgentSummary (25-30)

apps/cli/src/lib/orchestrators/__tests__/workspace-orchestrator.test.ts (4)

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

LowdbAdapter (14-168)

apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1)

WorkspaceOrchestrator (14-112)

apps/cli/src/lib/orchestrators/change-orchestrator.ts (1)

ChangeOrchestrator (12-59)

apps/cli/src/types/workspace.ts (1)

LocalWorkspace (12-15)

apps/cli/src/lib/orchestrators/__tests__/environment-orchestrator.test.ts (4)

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

LowdbAdapter (14-168)

apps/cli/src/lib/orchestrators/environment-orchestrator.ts (1)

EnvironmentOrchestrator (12-117)

apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1)

WorkspaceOrchestrator (14-112)

apps/cli/src/lib/orchestrators/process-orchestrator.ts (1)

ProcessOrchestrator (17-131)

apps/cli/src/lib/storage/lowdb-adapter.ts (3)

apps/cli/src/lib/storage/adapter.ts (1)

StorageAdapter (7-70)

apps/cli/src/lib/storage/types.ts (3)

SerializedDatabase (28-35)

createEmptyDatabase (57-64)

Database (15-22)

apps/cli/src/lib/storage/config.ts (2)

getDbPath (23-25)

ensureStorageDir (31-37)

apps/cli/src/lib/orchestrators/__tests__/process-orchestrator.test.ts (5)

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

LowdbAdapter (14-168)

apps/cli/src/lib/orchestrators/process-orchestrator.ts (1)

ProcessOrchestrator (17-131)

apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1)

WorkspaceOrchestrator (14-112)

apps/cli/src/lib/orchestrators/environment-orchestrator.ts (1)

EnvironmentOrchestrator (12-117)

apps/cli/src/types/process.ts (1)

Agent (29-32)

apps/cli/src/lib/orchestrators/process-orchestrator.ts (2)

apps/cli/src/lib/storage/adapter.ts (1)

StorageAdapter (7-70)

apps/cli/src/types/process.ts (3)

Process (8-18)

Agent (29-32)

Terminal (20-22)

apps/cli/src/lib/orchestrators/__tests__/change-orchestrator.test.ts (4)

apps/cli/src/lib/storage/lowdb-adapter.ts (1)

LowdbAdapter (14-168)

apps/cli/src/lib/orchestrators/change-orchestrator.ts (1)

ChangeOrchestrator (12-59)

apps/cli/src/lib/orchestrators/workspace-orchestrator.ts (1)

WorkspaceOrchestrator (14-112)

apps/cli/src/lib/orchestrators/environment-orchestrator.ts (1)

EnvironmentOrchestrator (12-117)

apps/cli/src/lib/orchestrators/change-orchestrator.ts (2)

apps/cli/src/lib/storage/adapter.ts (1)

StorageAdapter (7-70)

apps/cli/src/types/change.ts (1)

Change (1-6)

🪛 GitHub Actions: CI

apps/cli/PERSISTENCE_PLAN.md