Conversation
# Conflicts: # interface/src/components/PromptInspectModal.tsx
- Redesign sidebar to match Spacedrive style: 220px width, labeled nav items,
section headers, consistent row heights, hover states, agent sub-nav
- Make projects global (not per-agent): migration drops agent_id from projects
table, all store/API/tool methods updated, projects route moved to /projects
- Add automatic project logo detection: scans .github/, public/, src-tauri/
for logo/favicon files on create/scan, serves via GET /projects/{id}/logo
- Replace local Toggle with SpaceUI Switch across all files
- Replace SearchInput with SearchBar in AgentChannels
- Use SelectPill + Popover for memory sort dropdown
- Use CircleButtonGroup in CortexChatPanel header
- Add instance selector (SelectPill) in top bar
- New nav structure: Dashboard, Org Chart, Workbench, Tasks + settings footer
- Agent sub-items: Chat, Channels, Memory, Skills, Schedule (accordion style)
- Settings buttons: size md, variant outline
- Input default size changed to md in SpaceUI, focus ring set to accent
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sk notifications - Replace CronReplyBuffer with CronOutcome + set_outcome() tool for clean cron delivery - Cron channels now behave as normal channels with visible LLM output; set_outcome() stores final delivery - Default cron timeout increased to 1500s (25 min) - Add token usage tracking (migration, API endpoint, LLM usage module) - Dashboard: TokenUsageCard with real data, RecentActivityCard improvements, ActionItemsCard updates - WorkersPanel: replace modal with inline slide-over detail view (iOS-style push navigation) - Fix task_create tool not emitting notifications — wire ApiState through AgentDeps - Cortex, branch, worker improvements; LLM model/pricing updates - Design docs updates (autonomy, slash-commands) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add comprehensive CONTRIBUTING.md with dev setup, conventions, and SpaceUI docs - Add SpaceUI note to README contributing section - Switch @spacedrive/* deps from link: to ^0.2.0 (bun link overrides locally, CI pulls from npm) - Add just spaceui-link/spaceui-unlink commands Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR #555 Review — SpaceUI Migration (
|
| # | File | Issue |
|---|---|---|
| 1 | client.ts:1 |
(window as any).__SPACEBOT_BASE_PATH — only any in the codebase. Use global augmentation: declare global { interface Window { __SPACEBOT_BASE_PATH?: string } } |
P1 — React Patterns (4)
| # | File:Line | Issue | Fix |
|---|---|---|---|
| 2 | Sidebar.tsx:239 |
Render-phase setState for expandedAgent — can double-fire in concurrent mode |
Move into useEffect |
| 3 | Sidebar.tsx:248 |
Unsafe (location.search as Record<string, unknown>)?.id cast |
Use TanStack Router's useSearch() hook |
| 4 | PortalPanel.tsx:53 |
Settings useEffect overwrites user's in-progress edits on every conversationsData refetch |
Guard against overwriting dirty state |
| 5 | RecentActivityCard.tsx:44 |
agentIds array recreated every render as query key — triggers unnecessary refetches |
Wrap in useMemo |
P1 — TypeScript (6)
| # | File:Line | Issue | Fix |
|---|---|---|---|
| 6 | ConfigSectionEditor.tsx:29 |
Record<string, string | number | boolean | string[]> erases field-level types, forcing ~45 as assertions |
Use discriminated union or per-section generics |
| 7 | client.ts:356,381 |
history: unknown[] and history: unknown — never narrowed downstream |
Define proper prompt history types |
| 8 | client.ts:1814,1829 |
togglePlatform/disconnectPlatform use Record<string, unknown> body instead of existing schema types |
Use TogglePlatformRequest/DisconnectPlatformRequest |
| 9 | client.ts:1987-2032 |
createLink, updateLink, createGroup, etc. return untyped Promise<any> from response.json() |
Add type assertions like other endpoints |
| 10 | client.ts:2142-2154 |
Portal conversation methods return raw Promise<Response> — no response body typing |
Add response types |
| 11 | client.ts:2290,2298 |
lockSecrets/rotateKey return untyped response.json() — inconsistent with other typed secrets endpoints |
Add type assertions |
P1 — Jamiepine Style (3)
| # | File:Line | Issue | Fix |
|---|---|---|---|
| 12 | OrgGraphInner.tsx:150 |
prevDataRef.current = data — render-body ref mutation is a side effect during render |
Move into useEffect |
| 13 | OrgGraphInner.tsx:544 |
Drag-drop group mutations use raw .then() fire-and-forget — UI silently desyncs on API failure |
Use useMutation with error handling |
| 14 | primitives.tsx:8 |
Direct ../../node_modules/@spacedrive/primitives/dist/index.js import couples to package internals |
Import from @spacedrive/primitives directly |
P2 — React (10)
| # | File | Issue |
|---|---|---|
| 1 | PortalPanel.tsx:50 |
Missing conversations in useEffect dep array (reads it but doesn't list it) |
| 2 | PortalPanel.tsx:78 |
Unusual useEffect pattern for default project selection — useMemo or initial-value-once would be clearer |
| 3 | ApprovalModal.tsx:27 + ActionItemsCard.tsx:53 |
timeAgo() duplicated identically across 2 files — consolidate to @/lib/format |
| 4 | PortalHistoryPopover.tsx:13 + RecentActivityCard.tsx:175 |
formatTimestamp/formatTimeAgo also duplicated — same consolidation |
| 5 | PortalTimeline.tsx:182 |
Auto-scroll doesn't trigger on initial mount if items exist and user isn't at bottom |
| 6 | WorkersPanel.tsx:27 |
Component remounts + re-fetches all data when popover reopens — lift fetching or use keepMounted |
| 7 | RecentActivityCard.tsx:69 |
Promise.all(agentIds.map(...)) fetches per-agent — consider useQueries for better caching |
| 8 | PortalHeader.tsx:25 |
20+ props — extract settings/history/workers into context providers or compose with render props |
| 9 | PortalPanel.tsx:18 |
~15 useState calls — extract conversation, settings, and file upload into custom hooks |
| 10 | PortalComposer.tsx:166 |
Drag-and-drop dragCounter could desync if child elements intercept dragenter — correctly implemented but fragile |
P2 — TypeScript (9)
| # | File | Issue |
|---|---|---|
| 11 | ConfigSectionEditor.tsx:62-149 |
Triple-repeated switch for config value extraction — violates DRY, extract to getConfigSection() helper |
| 12 | ConfigSectionEditor.tsx:56 |
default case returns {} but doesn't handle all SectionId union values — missing exhaustive handling |
| 13 | buildGraph.ts:37 |
Inline type { gradient_start?: string | null | undefined } has redundant | undefined — use named interface |
| 14 | buildGraph.ts:259 |
node.type === "agent" || node.type === "human" string comparison — no type narrowing from @xyflow/react Node type |
| 15 | types.ts:125-143 |
ConversationSettings manually defined instead of derived from schema.d.ts |
| 16 | types.ts:163-182 |
CreatePortalConversationRequest/UpdatePortalConversationRequest duplicate fields from schema types |
| 17 | types.ts |
Memory types duplicated between types.ts and client.ts — dual-source confusion |
| 18 | client.ts:510 |
CortexChatSSEEvent union works but lacks comment explaining SSE protocol |
| 19 | workbench/types.ts:3 |
OrchestrationWorker.tool_calls shadowed by live_tool_calls? — semantically ambiguous |
P2 — Style (8)
| # | File | Issue |
|---|---|---|
| 20 | ConfigSectionEditor.tsx:101 |
useEffect dep array missing sandbox — currently safe via config dep but fragile |
| 21 | OrgGraphInner.tsx:238,376 |
Two catch { } blocks silently swallow localStorage errors — inconsistent with rest of codebase |
| 22 | SecretsSection.tsx:98 |
Generic Failed: ${error.message} on 5 mutation error handlers — should be specific about WHAT failed |
| 23 | UpdatesSection.tsx:98 |
document.execCommand("copy") deprecated — use Clipboard API as primary, remove fallback |
| 24 | Settings.tsx:280,404,448,489 |
catch (error: any) in 4 places — use unknown with type guard |
| 25 | ConfigSectionEditor.tsx:152 |
saveHandlerRef.current.save = handleSave imperative ref pattern bypasses React data flow |
| 26 | buildGraph.ts:228 |
allNodes.find(n => n.id === node.parentId) is O(n) per child — use Map<string, Node> for O(1) |
| 27 | OrgGraphInner.tsx:238,376 |
Silent catch {} — should at least console.warn for debuggability |
P2 — Security (11)
| # | File | Issue |
|---|---|---|
| 28 | client.ts:1 |
Global BASE_PATH from window without validation — standard for embedded apps, low practical risk |
| 29 | client.ts:10 |
setServerUrl() no protocol allowlist — could set unexpected URL (internal callers only, mitigated) |
| 30 | client.ts:311 |
URL construction via string concat — safe given current hardcoded path literals, but centralized builder would be more defensive |
| 31 | client-typed.ts:18 |
Auth token in localStorage — accessible to XSS, acceptable for self-hosted |
| 32 | PortalComposer.tsx:82 |
File upload <input> has no accept attribute or client-side size/type validation |
| 33 | PortalTimeline.tsx:37 |
Image attachments loaded from server URLs — server-side content-type enforcement assumed |
| 34 | SecretsSection.tsx:222 |
Master key held in React state + document.execCommand("copy") fallback — cleared on dialog close (good) |
| 35 | ApiKeysSection.tsx:92 |
Server returns actual API key value to frontend — should return masked/exists-only |
| 36 | ChannelEditModal.tsx:189 |
Platform credentials submitted plaintext via JSON — expected, type="password" used correctly |
| 37 | useEventSource.ts:46 |
SSE connection without auth headers — EventSource API limitation, acceptable for self-hosted |
| 38 | PromptInspectModal.tsx:248 |
message: any type — React's <pre> escaping prevents XSS, but type should be tightened |
Positive Highlights
- Zero
dangerouslySetInnerHTML— XSS fully mitigated by React escaping throughout - Clean decomposition: Settings (2900→967 lines), AgentConfig (1452→252), TopologyGraph (2074→14 files) with barrel exports
@spacedrive/primitivescompat layer maps legacy variants pragmatically (ghost→subtle,secondary→gray)- Discriminated unions used correctly for SSE events, tool states, timeline items
import typeused consistently across all type files- Vertical slice completeness — backend/API/UI/docs all aligned, no orphaned surfaces
useMutation+ React Query pattern used consistently (except OrgGraphInner drag-drop)enabledflags on conditional queries (ApprovalModal, PortalHeader)encodeURIComponentused consistently in API client URL construction- External links use
rel="noopener noreferrer"
Recommended Fix Order
- Sidebar.tsx — render-time setState +
useSearch()(items 2-3) - OrgGraphInner.tsx — render-body ref mutations +
useMutationfor drag-drop (items 12-13) - PortalPanel.tsx — guard settings hydration (item 4)
- client.ts — add response typing to untyped endpoints (items 7-11)
- primitives.tsx — fix import path (item 14)
- ConfigSectionEditor.tsx — address loose typing (item 6, follow-up)
- P2s — batch in follow-up PRs
vsumner
left a comment
There was a problem hiding this comment.
Inline comments for P1 findings. See the main review comment for full context.
vsumner
left a comment
There was a problem hiding this comment.
Rust Backend Review — Consolidated Findings
5 parallel reviewers analyzed 116 backend files (11,147 additions).
Verdict: REQUEST_CHANGES
P0 — Must Fix (3)
1. src/api/usage.rs:232,301,349 — Silent DB error swallowing
if let Ok(row) = q.fetch_one(*pool).await silently drops DB errors, returning partial/empty data. In production, failures appear as zero usage rather than error responses.
// Fix: propagate errors with tracing context
let row = q.fetch_one(*pool).await.map_err(|error| {
tracing::error!(%error, "failed to query token usage");
StatusCode::INTERNAL_SERVER_ERROR
})?;2. src/api/providers.rs:1033 — Naked unwrap without SAFETY comment
doc["llm"]["provider"]["azure"].as_table_mut().unwrap() will panic if value is not a table.
// Fix: make invariant explicit
// SAFETY: We just initialized this table above if it was missing
let azure_table = doc["llm"]["provider"]["azure"].as_table_mut()
.expect("azure table must exist after initialization above");3. src/api/attachments.rs:191-193,284-295 — Silent unwrap_or_default() on DB columns
Missing filenames become empty strings with no indication anything went wrong. Silent data loss if columns are null/misconfigured.
// Fix: at minimum, log warnings
let original_filename: String = row.try_get("original_filename")
.map_err(|e| {
tracing::warn!(%e, "missing original_filename on attachment");
StatusCode::INTERNAL_SERVER_ERROR
})?;P1 — Should Fix (4)
4. migrations/20260407000001_token_usage.sql:2 — Missing IF NOT EXISTS
All other new migrations use CREATE TABLE IF NOT EXISTS. This one doesn't — will fail if migration is run twice. Fix: add IF NOT EXISTS.
5. src/api/wiki.rs:282-289,348-354 — String-based error status matching
Error status codes determined by msg.contains("not found"). Fragile — any error message change silently breaks the status code logic.
// Fix: use structured error enum
.map_err(|e| match e {
WikiError::NotFound => StatusCode::NOT_FOUND,
WikiError::EditFailed(_) => StatusCode::BAD_REQUEST,
_ => StatusCode::INTERNAL_SERVER_ERROR,
})?;6. Multiple files — Generic 500 without context (12+ occurrences)
map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) discards the actual error in wiki.rs (7), settings.rs (5), projects.rs (3).
// Fix: always include error context
.map_err(|error| {
tracing::error!(%error, "failed to update wiki page");
StatusCode::INTERNAL_SERVER_ERROR
})?7. src/wiki/store.rs:603 — FTS query syntax injection
format!("{query}*") passes raw user input as FTS5 query. Not SQL injection (parameterized), but FTS syntax characters (AND, OR, ", *, +) cause malformed queries or unexpected results. Fix: sanitize FTS special characters from user input.
P2 — Recommended (5)
- Slug generation race (
wiki/store.rs:780-806) — Non-transactional uniqueness check. Add unique constraint + retry on conflict. - Silent JSON parse failure (
wiki/store.rs:811) —unwrap_or_default()silently drops decode errors. Log warnings. - No input length limits (
tools/wiki_create.rs:92-95) — Add bounds: title ≤ 200 chars, content ≤ 100K. - Double
as_ref()pattern (projects.rs:168, 12+ occurrences) —store_guard.as_ref().as_ref().ok_or(...). Add helper method onApiState. - Index naming inconsistency —
token_usage_agentvsidx_notifications_inbox. Addidx_prefix for consistency.
Strengths
- Agent cancellation race fix (
channel.rs:77-87) — Correctly changed from write-lock-remove-then-abort to read-lock-abort-retain pattern - Branch spawn lock ordering (
channel_dispatch.rs:335-366) — Write lock acquired BEFORE spawn, preventing fast-completion misses - History write-back — Correctly handles all three completion paths (Ok, MaxTurnsError, PromptCancelled)
- Notification dedup — Partial unique index +
INSERT OR IGNOREis the right pattern - Wiki FTS5 integration — Triggers + content sync + rank-based search well done
🤖 Generated with Claude Code
Additional inline findings (lines not resolvable in diff)P0:
// SAFETY: We just initialized this table above if it was missing
let azure_table = doc["llm"]["provider"]["azure"].as_table_mut()
.expect("azure table must exist after initialization above");P1: Multiple files — Generic 500 without error context (12+ occurrences)
// Fix: always include error context
.map_err(|error| {
tracing::error!(%error, "failed to update wiki page");
StatusCode::INTERNAL_SERVER_ERROR
})?P2:
🤖 Generated with Claude Code |
- Remove non-existent Button props (loading handled upstream, ghost→bare, destructive→accent, secondary→gray) - Remove non-existent NumberStepper props (variant, type, showProgress) - Remove dot prop from Banner - Remove unused imports and dead code - Fix cargo fmt (collapsed import) and unused mpsc import
Co-Authored-By: Claude <noreply@anthropic.com>
- tasks: kanban → linear-style list, GitHub metadata badges, SSE updates - skills: add built-in skills section and three-tier precedence - architecture: global database tables, new API groups, Portal refs, new events - roadmap: move 17 shipped features to completed - meta.json: add new pages, remove phantom mcp and hosted entries - index: add wiki and portal quick links Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Previously resumed and detached workers had wiki tools disabled. Now all workers get wiki access when a wiki store is configured, and the wiki_write setting defaults to true.
Drop the "first compile to launch" framing and giant completed checklist. Replace with a concise summary of where we are, what's in progress, and what's next. Co-Authored-By: Claude <noreply@anthropic.com>
- surface DB errors in usage/attachments/providers instead of swallowing them - replace naked unwraps + string-matched wiki errors with typed WikiError - sanitize FTS5 queries to block operator injection - add tracing context to map_err closures across API handlers - make token_usage migration idempotent with IF NOT EXISTS - remove window as any in client.ts and useServer - type prompt history, platform toggle, portal conversations, link/group/human and secrets endpoints in api/client.ts - fix Sidebar render-phase setState and unsafe location.search cast - fix OrgGraphInner render-body ref mutation and fire-and-forget mutations - fix PortalPanel settings reset on background refetch - stabilise RecentActivityCard query key with useMemo - replace ConfigSectionEditor catch-all type with typed section union - bump @spacedrive/* to 0.2.2 and regenerate openapi schema
|
@vsumner addressed the P0/P1 items from your review in c046be7. quick summary: P0
P1
Also cleaned up as blockers for gate-pr
gate-pr green locally: fmt, check, clippy P2s left for follow-up PRs as you suggested. one small thing to flag: the migration index rename ( |
- read upload body in chunks, abort early when exceeding 50MB limit instead of buffering the entire body into memory first - serve_attachment re-derives path from saved_filename + workspace saved/ dir instead of trusting the disk_path column (prevents arbitrary file read if the DB row is corrupted) - portal attachment lookup now filters by channel_id to prevent cross-conversation attachment references - pre-saved attachment path in save_channel_attachments does a fresh DB lookup with channel_id verification instead of trusting Attachment struct fields, removing the disk_path field from Attachment entirely - size_bytes i64→u64 casts use try_from to avoid wrapping on negative values
Summary
Migrates the entire frontend to SpaceUI, Spacedrive's component library. The local UI primitives (~25 components, ~3000 lines) are gone — replaced by
@spacedrive/primitives,@spacedrive/ai,@spacedrive/forms, and@spacedrive/explorer. Tailwind v3 is out, Tailwind v4 via@tailwindcss/viteis in, with SpaceUI's design token system and theme imports.But this isn't just a component swap. The interface has been restructured from the ground up.
Layout
The old flat nav is replaced with a persistent sidebar (220px, Spacedrive-style) with labeled sections, accordion agent sub-nav, and a global workers popover in the footer. The org chart has its own dedicated tab instead of being crammed into the main view.
A new Dashboard serves as the landing page with real data wired to action items (notifications), token usage, and recent activity cards.
UI rewrites
Every major view was decomposed from monolithic files into modular components:
Tasks got the biggest conceptual change — the kanban board is gone, replaced with a Linear-style task list with detail views, GitHub metadata badges, and SSE-driven updates.
New systems
Wiki — full implementation from scratch. SQLite-backed, tolerant multi-pass edit matching, 6 tools, REST API, frontend route with page browser and wiki-link navigation.
Notifications — SQLite store, API endpoints, SSE real-time broadcasting,
useNotificationshook with optimistic dismiss, wired into the dashboard's action items card.Portal — webchat renamed to portal throughout. Modular PortalPanel/Timeline/Composer/Header. File attachments with multipart upload, drag-and-drop, and timeline rendering. Conversation persistence with full CRUD.
Streaming —
prompt_once_streamingon SpacebotHook with token-by-tokenWorkerTextdeltas. OpenAI Responses API SSE streaming supporting function call deltas, text deltas, reasoning summaries.Built-in skills — compiled into the binary via
include_str!, starting with a wiki-writing skill.Backend
ConversationSettingsstruct with memory mode, delegation, worker context, model selection. Per-channel persistence with resolution chain.ModelOverridesthreaded through all process types. Settings hot-reload viaProcessEvent::SettingsUpdatedResponseModeenum (Active/Observe/MentionOnly) replacing the oldlisten_only_modesystem. Per-channel persistence, TOML support, slash commands that persist to DBtoken_usagetable withUsageAccumulatorflushed per-processDesign docs
Added: conversation-settings, wiki, token-usage-tracking, cron-outcome-delivery, skill-authoring, worker-briefing, attachment-portal-and-defaults, slash-commands, autonomy, goals
Note
Highlights information that users should take into account, even when skimming.
Latest update (commit c046be7): addressed P0/P1 review feedback — DB error surfacing in usage/attachments/providers, typed WikiError replacing string matches, FTS5 query sanitization against operator injection, tracing context in API handlers, idempotent token_usage migration, removed type casts (window as any, unsafe location.search), typed prompt history/platform toggle/portal conversations/link/group/human secrets endpoints, fixed Sidebar setState render-phase issue, fixed OrgGraphInner ref mutation and unsafe mutations, fixed PortalPanel settings reset on refetch, stabilized RecentActivityCard query key, replaced ConfigSectionEditor catch-all type with union, bumped @spacedrive/* to 0.2.2.
Written by Tembo. This will update automatically on new commits.