FEA-1504: Gate embedded agent dashboard - #264
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 573c54d50a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
c3d3370 to
c6d86c5
Compare
thadeusb
left a comment
thadeusb
left a comment
There was a problem hiding this comment.
Two things make me want to talk before this lands instead of just patching. The renderer only builds on your machine: the vendored design-system points every export at a dist/ that .gitignore excludes, and globals.css pulls in two tailwind modules that aren't in package.json, so a clean clone or CI can't produce a bundle. And the description says strangler-fig with a legacy iframe fallback, but the code dropped legacy.html and hid the sidecar dashboard behind 'coming soon' stubs, so it's a hard cutover, not parallel operation. Lots of fixable bugs underneath too, but I want the build and the migration strategy settled first.
2a3bb7d to
d7c538b
Compare
…React renderer - Replace agent-dashboard sidecar (external process) with in-process node:sqlite database - Create functional repository layer matching symphony-alpha patterns (sessions, agents, events, dashboard stores) - Add Vite + React 19 renderer with DesignSystemProvider shell and DS components - Add hook ingestion endpoint (POST /api/hooks/event) writing directly to local SQLite - Extend preload bridge with typed db IPC channels (db.getSessions, db.getAgents, db.getEvents, db.getDashboardSummary) - Update window.ts to load Vite-built renderer (dev server in dev, file in prod) - Vendor @closedloop-ai/design-system as file: dep with subpath component exports - Preserve legacy HTML renderer as legacy.html for strangler-fig migration via iframe - Add DashboardPage using DS Card/Badge/Button components with real-time summary queries Testing: pnpm build:renderer (Vite) and npx tsc (main process) both compile clean; all DB-layer errors resolved; pre-existing tsc errors (node-pty, ws, react) unchanged. Risks: Sidecar not yet disabled — this adds parallel in-process DB alongside existing agent-monitor; legacy HTML still works via iframe fallback.
d7c538b to
a6427ce
Compare
- Fix build ordering: renderer builds after clean:dist, not before - Fix vendor DS: gitignore negation for dist/, add tw-animate-css dep - Fix runtimeStatus.healthy → gatewayHealthy in App.tsx - Wire agentDatabase.close() into shutdown sequence - Gate all db:* IPC handlers behind agentMonitorEnabled - Add zod validation to hooks-ingestion payload - Fix agent updateStmt to include metadata; fix session update guard - Add PRAGMA user_version migration system for schema versioning - Remove unpopulated Token Usage card from dashboard - Add 3s polling to DashboardPage and ApprovalsPanel - Add error handling to approval actions - Replace window.prompt with inline input in SettingsPanel - Restore Labs settings tab with feature flag toggles - Force light mode via DesignSystemProvider - Add lucide-react icons to MetricCard and WorkflowStatTile - Rebuild AnalyticsView with DS components (ActivityHeatmap, DonutChart, RankedBar, SegmentedBar, LineChart) - Rebuild WorkflowsView with DS composites (OrchestrationDag, SankeyGraph, AgentCollaborationNetwork, WorkflowStatTile, effectiveness table) - Enhance DashboardPage with tool usage RankedBar + agent types table - Enhance SessionsView with search, status filter, polling - Enhance ActivityFeedView with search, type filter, expandable detail - Add analytics + workflow database queries (tool usage, daily events, status distributions, orchestration DAG, tool flow transitions) - Code-split pages with React.lazy() — initial load 49KB vs 1.6MB - Add useQueryCache hook with TTL for shared cross-component caching - Rename Dashboard → "My Tasks", Workflows → "Agents" in nav Testing: typecheck passes, build produces chunked output Risks: vendor/design-system/dist/ adds ~1074 files to git Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Hoist all analytics + workflow SQL into prepared statements at init time instead of re-preparing on every poll (major perf fix) - Nav order: My Tasks, Sessions, Activity Feed, Analytics, Agents - Rename: Dashboard → Sessions, Kanban Board → My Tasks, Workflows → Agents - Remove duplicate Sessions nav item (Dashboard now serves that role) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add schema migration v2: indexes on events.tool_name and events(session_id, rowid) for tool queries - Rewrite tool transitions from correlated subquery to LEAD() window function, scoped to last 7 days (was 5.2s full scan → sub-second) - Scope tool usage query to last 30 days, daily events to last 365 days - Increase cache TTL for analytics/workflows to 10s/15s poll (heavy queries don't need 3s freshness) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix schema migration v2: rowid is a pseudo-column that can't be indexed — use (session_id, created_at) instead - Add rollupOptions.output.manualChunks to split vendor bundle: vendor-charts (recharts/d3), vendor-icons (lucide-react), vendor-radix, vendor-ds - Bump chunkSizeWarningLimit to 1000 (vendor chunks cache once) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Schema migration v3: add indexes on events(event_type), events(created_at, tool_name), agents(status), agents(type), agents(parent_agent_id) - events-by-type: 1.1s → 0.15s with event_type index - Dashboard: split getSummary (49ms, 5s poll) from getAnalytics (4s+, 60s poll) so initial load is instant - Fix tool usage query column order for index coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…y Feed into Sessions - New AgentsPage with Analytics/Orchestration tabs replaces separate Analytics and Workflows nav items - Sessions page (dashboard) now has Sessions/Activity Feed tabs - Nav: My Tasks, Sessions, Agents, then Gateway section - Removes activity-feed, analytics, workflows as separate NavIds Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 0 of the in-process agent monitor replacement (PR #264): make the integration branch current and compilable before any feature work. - Merge origin/main (175 commits) into feat/in-process-agent-database, bringing in FEA-1431 genai-prices pricing, FEA-1434 billing_mode, and the FEA-1435/1436 cost-reconciliation subsystem. - Resolve conflicts: - app.ts: keep both 264's in-process SQLite wiring (openAgentDatabase, desktop:db:* IPC handlers) and main's reconciliation subsystem imports/wiring; drop the broken HarnessImportService import, field, constructor, start(), and stop() — harness-parsers/ never existed on the branch and was the tsc build break. - preload.ts: union both sides — 264's db.* bridge plus main's flags / ingest-progress / managed-key-hint methods. - package.json: bump version to 0.15.105 (above main's 0.15.104); keep both @playwright/test (main) and @tailwindcss/* (264). - index.html: keep 264's clean Vite entry (drop the monolithic shell). - pnpm-lock.yaml: regenerated via pnpm install against the merged package.json (re-links vendored @closedloop-ai/design-system). - model_pricing is now absent from the entire desktop src (main's FEA-1431 removed it); the new-DB read path (src/main/database) never referenced it. The Phase 1 relay repoint stays out of scope. Testing: - just desktop-typecheck passes (tsc --noEmit, exit 0, zero errors) - git merge-base --is-ancestor origin/main HEAD holds after this commit - grep confirms zero model_pricing references in apps/desktop/src Risks: - Sidecar + iframe runtime still present (cutover is Phase 1); two SQLite DBs coexist by design until then. No external contract changed. - Scope limited to Phase 0; no write path, relay repoint, or feature work in this commit.
…on (Phase 1 S5) Cut the agent-monitor runtime from the vendor sidecar to the in-process listener and repoint every consumer of the old vendor dashboard.db — all in one change so :4820 is never double-bound. - Atomic cutover (app.ts): stop instantiating/starting AgentMonitorSidecar; construct the hook lifecycle (createLifecycle over the shared connection, billing_mode stamping, webContents 'desktop:db:changed' live-update emit) and AgentHookListener instead. startAgentCapture() (boot + enable path) runs the boot migration, starts the listener, self-heals hooks, and starts the relay. Shutdown stops the listener before closing the shared DB. The listener's onBindError preserves the degraded-tray UX on EADDRINUSE. Sandbox changes need no restart (the listener reads the sandbox live). - Relay repoint (agent-session-sync-service.ts): new getConnection option — the relay reads the shared in-process connection (no per-cycle open/close, no existsSync no-op) and the token query drops the +baseline_* arithmetic (the v4 plain columns are already effective). Tests keep the path-based fallback. - Cost-reconciliation repoint (plan-challenge blocker): reconciliation-worker loadMeteredUsageRows drops +baseline_* and reads the shared connection via app.ts loadUsageRows; removed the now-dead loadMeteredUsageRowsFromDisk. This was the second live vendor-DB consumer the cutover would have silently broken. - Boot migration (agent-monitor-db-migration.ts): one-time idempotent import of the vendor dashboard.db (sessions/agents/events, baseline->effective tokens, billing_mode) into the in-process DB, then rename dashboard.db -> dashboard.db.migrated (downgrade-safe, not deleted). - IPC: get-agent-monitor-url -> listener; ingest-progress -> null; reprocess -> honest unavailable (no destructive clear; filesystem import returns in Phase 2). - Migrated the relay + reconciliation test fixtures to the v4 schema (effective in plain columns) and retargeted the 4 agent-monitor-wiring-static assertions to the listener-based invariants. Updated apps/desktop/CLAUDE.md to describe the in-process runtime (sidecar source dormant until Phase 3E). Testing: just desktop-lint, just desktop-typecheck, full suite (2131 pass / 0 fail / 14 pre-existing skips) incl. new agent-monitor-db-migration tests (import+rename, idempotency, absent, and cost-recon reads the real v4 schema with no 'no such column' baseline error). Risks: relay/recon now depend on the shared connection — covered by the repointed fixtures + the real-v4-schema reconciliation test. The vendor sidecar source remains in the tree (dormant) until the Phase 3E teardown.
…ning) From the adversarial code + database review of the Phase 1 diff: - HIGH: the boot migration aborted entirely on a single orphaned vendor agent row. INSERT OR IGNORE does NOT swallow a FK-constraint failure (it throws), so one agent whose session was pruned rolled back the whole import and re-failed on every boot (dashboard.db never renamed). copyAgents now explicitly skips agents whose session_id is absent from the imported sessions, matching its comment. Added a regression test seeding an orphaned vendor agent. - MEDIUM: moved BEGIN IMMEDIATE inside processEvent's try so a stale open transaction (from a prior ROLLBACK that itself failed) is rolled back and the next event recovers, instead of escaping processEvent's "never throws" contract. - LOW: runMigrations wraps each migration DDL + user_version bump in one transaction so a crash mid-migration rolls back both (clean re-run); the v4 CREATE TABLE token_usage is now IF NOT EXISTS as belt-and-suspenders. - LOW: shutdown stops the cost-reconciliation timers before closing the shared DB connection (close the connection last); fixed a stale dashboard.db comment. Testing: just desktop-lint, just desktop-typecheck; full suite 2132 pass / 0 fail / 14 pre-existing skips (incl. the new orphaned-agent migration regression).
…n anchors Per repo CLAUDE.md D6, the in-code legacy-migration anchors now cite the ClosedLoop cleanup tickets that track their removal.
…echeck) 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.
- replace vendored desktop design-system with the published GitHub Packages release\n- update renderer session drill-down to follow the published SessionTable contract\n- refresh desktop lockfile and packaging flow for registry-based installs\n\nTesting:\n- pnpm -C apps/desktop typecheck\n- pnpm -C apps/desktop build:renderer\n- pnpm -C apps/desktop dev\n\nRisks:\n- desktop still pins an explicit design-system prerelease until a stable package version exists\n- session detail deep links now depend on hash-based navigation staying aligned with the dashboard tab
- replace the renderer bootstrap import of DesignSystemProvider with next-themes ThemeProvider\n- avoid the published design-system root bundle that still references a missing global React binding\n\nTesting:\n- pnpm -C apps/desktop typecheck\n- pnpm -C apps/desktop build:renderer\n- just desktop-dev\n\nRisks:\n- desktop no longer mounts the design-system root toaster wrapper until the published package root entry is fixed upstream
- force desktop renderer imports onto the design-system ESM files\n- inject missing React imports into broken published design-system modules during bundling\n\nTesting:\n- pnpm -C apps/desktop typecheck\n- pnpm -C apps/desktop build:renderer\n- just desktop-dev\n\nRisks:\n- this is a consumer-side compatibility patch for a malformed published package and should be removed once the design-system publish is corrected upstream
- Add a first-party in-process collection layer (src/main/collectors/):
boot-time bulk import + live file watchers for all five agent CLIs
(Claude, Codex, Cursor, Copilot, OpenCode), writing through a new
idempotent importSession into the in-process node:sqlite DB. Ports the
per-tool parsers/home modules from CommonJS to first-party TypeScript.
- Add a first-party Claude file collector (historical import + watch over
~/.claude/projects). Its live watcher is gated off when hooks are
installed (hooks own live Claude capture; avoids double-counting turns).
- Own the hook handlers: ship resources/hooks/{hook-handler,codex-hook-
handler}.js via electron-builder extraResources; agent-monitor-path now
resolves resources/hooks. No build-agent-monitor, no generated tree.
- Gate all collection by agentMonitorEnabled and the FEA-1407 sandbox
(fail-closed, per session); wire CollectorManager into app.ts boot,
the enable/disable toggle, and shutdown (before agentDatabase.close()).
Restart the collectors on a hooks toggle so the Claude-watch gating is
re-evaluated rather than frozen at boot.
- Remove the third-party Claude-Code-Agent-Monitor entirely (absorbs
FEA-1501): delete all scripts/agent-monitor-*, build-agent-monitor.mjs,
the dormant sidecar, the iframe embed, the dashboard.db migration
adapter, the agent-monitor e2e/audit suites, the vendor deps + lockfile
entries, and the package/stage/electron-builder wiring. Rewrite the
CLAUDE.md "Agent Monitor" section and THIRD_PARTY_NOTICES.
- Bump desktop version 0.15.109 -> 0.15.110.
Testing:
- just desktop-lint, desktop-typecheck (main + renderer), desktop-test:
all green (1935 tests, 0 fail, 0 skipped).
- Real `pnpm build` (tsc emit + Vite renderer) succeeds.
- New tests (collectors-{parsers,import,hook-handler}): idempotent
re-import adds no duplicate rows, fail-closed sandbox gating, both hook
handlers' POST envelope, and ported-parser fixtures.
- Manually verified locally: the dashboard populates from existing
sessions on a fresh DB.
Risks:
- The full electron-builder DMG (`pnpm package`) was not run locally
(needs macOS + signing; not part of PR CI, only release.yml). The
packaging config was updated and review-verified, but a clean-machine
DMG smoke test remains the real proof for the asar-external node:sqlite
path.
- Dropping the dashboard.db migration adapter (FEA-1501) means existing
internal vendor-DB users re-derive history from on-disk transcripts on
next launch; sessions whose transcripts were deleted are lost
(acceptable pre-launch).
- The :4820 HTTP hook transport is intentionally preserved (FEA-1500) so
hook commands already baked into ~/.claude/settings.json keep working.
…into feat/in-process-agent-database
…pp token The Tests (and Release) workflows install deps with this repo's GITHUB_TOKEN, which lacks read access to @closedloop-ai/design-system — it's published to GitHub Packages from another org repo, so pnpm install fails with ERR_PNPM_FETCH_403. Mint an org-scoped GitHub App token (the CLOSEDLOOP_APP_* app already used by compatibility-smoke for cross-repo access) with packages:read and use it for pnpm install only; the release publish step keeps the repo GITHUB_TOKEN for contents:write. This is a pre-existing branch CI failure (red on the prior commit too), not specific to this PR's code.
The @closedloop-ai/design-system + loops-api GitHub Packages now grant closedloop-electron read access (package Manage Actions access), so the stock GITHUB_TOKEN can fetch them. Revert the org-scoped app-token install step in test.yml + release.yml back to GITHUB_TOKEN.
UI Regression ReviewCritical Issues1. Deep subpath imports from Action: Verify 2. Fragile Action: Either upstream High Issues3. CSS custom properties may render invisible UI Action: Verify the DS theme includes all referenced CSS vars, or add fallback definitions in 4. DS version range is too broad Action: Pin to Medium Issues5. 6. Low / Cosmetic
Verdict
|
UI Regression Review — All Items Addressed#1: DS subpath exports ✅ VerifiedInstalled #2: MetricCard slot selectors 🔒 Mitigated by version pin
#3: CSS custom properties ✅ VerifiedAll referenced vars ( #4: DS version pinned ✅
#5: Topbar labels ✅ AddedMissing #6: Dead postcss config ✅ Removed
#7: @tailwindcss/typography 🔒 Required by DS globals.cssKept as devDependency — the DS packages #8: PR description ✅ UpdatedRemoved stale |
mikeangstadt
left a comment
mikeangstadt
left a comment
There was a problem hiding this comment.
Addressing prior review feedback
@shafty023: "remove clone of design system" ✅ Done
The vendored vendor/design-system/ was removed in commit a876313 (FEA-1503). The branch now consumes @closedloop-ai/design-system as a published npm dependency from GitHub Package Registry. The stale .gitignore negation (!apps/desktop/vendor/design-system/dist/) has also been cleaned up.
@thadeusb: Build concerns ✅ Resolved
- Vendored DS dist/ gitignored: Vendored DS is gone; the published npm package includes its dist.
- globals.css pulls in tailwind modules not in package.json: The DS globals.css
@plugin "@tailwindcss/typography"is now resolvable —@tailwindcss/typographyis in devDependencies. The@tailwindcss/viteplugin handles the Tailwind pipeline; the dead PostCSS config and its deps (@tailwindcss/postcss,postcss) have been removed. - Strangler-fig / legacy.html: PR description updated to reflect that this is a direct replacement, not parallel operation.
Additional fixes in this batch
- DS version pinned to
0.1.1-dev.26892521643.1(was">=0.1.1-0"which could pull breaking prereleases) - Added missing Topbar labels for
activity,analytics,workflows - Build verified:
pnpm buildpasses (2542 modules, 2.57s)
… type decoupling, tests) - Add API key set/clear controls and an auto update status/apply banner to the React Settings UI, consuming existing but previously unused IPC (getApiKeyStatus/setApiKey/clearApiKey, desktop:update-status/-available, applyUpdate). Onboarding wizard, command-key management, and sandbox/origins editors remain deferred to a later phase. - Decouple renderer-facing DB response types into src/shared/agent-db-contract.ts so the renderer no longer imports from src/main/database/types; raw rows map to the shared DTOs at the existing repository toRow() boundary. - Add React renderer tests (UpdateBanner reducers plus active source guards) replacing the stale skipped renderer-logs guard; keep one accurately labeled skip for the still deferred diagnostics and command-signing UI. - Pin @closedloop-ai/design-system to the locked prerelease and bump desktop version to 0.15.111. Testing: - just desktop-lint: clean - just desktop-typecheck: clean (main and renderer) - just desktop-test: 1949 pass / 0 fail - pnpm build:renderer: success (2545 modules) Risks: - Settings parity ports only the quick win controls; heavier setup/recovery controls stay deferred. - Type move is internal only (IPC ships as one unit with the app); no persisted-contract migration needed.
…into feat/in-process-agent-database
Compatibility Smoke Test Results
|
|
Thanks for the thorough pass, this is genuinely helpful. One important update before the item by item notes: since this review was written, the vendored design-system clone was removed (commit B1 (deep subpath imports): not an issue. The published package B2 (data-slot overrides): not an issue. B3 (CSS custom properties): not an issue. All referenced variables ( B7 (@tailwindcss/typography unused): keeping it. There are no B8 (legacy.html iframe fallback): no code issue. There is no B5 (missing Topbar labels): fixed in B6 (dead postcss config): fixed in B4 (version range too broad): fixed in Separately, the three remaining open review threads (Settings setup/recovery controls, renderer DB type coupling, and the skipped renderer guards) are addressed in commit |
Keep the legacy monitor as the default boot path, put the design-system dashboard runtime and storage behind the explicit Labs flag, and package the embedded agent dashboard assets and tests. Validation: pnpm -r prebuild; pnpm -C apps/desktop lint; pnpm typecheck; pnpm -C apps/desktop assert:design-system-boundary; pnpm -C apps/desktop test:boot:design-system-off; pnpm -C apps/desktop measure:agent-dashboard-storage; pnpm test; pnpm -C apps/desktop build.
shafty023
changed the title
3 participants
Summary
Finalizes the embedded agent dashboard work for PR #264 by keeping the legacy monitor as the default boot path and gating the new design-system dashboard runtime, storage, and IPC behind the explicit Labs flag.
Changes
Testing
pnpm -r prebuildpnpm -C apps/desktop lintpnpm typecheckpnpm -C apps/desktop assert:design-system-boundarypnpm -C apps/desktop test:boot:design-system-offpnpm -C apps/desktop measure:agent-dashboard-storagepnpm testpnpm -C apps/desktop buildReview State
Collaborated & Orchestrated by ClosedLoop.AI | GitHub