Repository navigation
fix: load knowledge memories without enabling automation - #590
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (13)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe daemon now keeps the memory catalog available when memory automation is disabled. Runtime dependencies and shutdown use the catalog store. The web client reports server failures with a user-facing message and disables automatic retries. ChangesWorkspace memory catalog availability
Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Knowledge memories can load without automatic memory automation, with empty states and user-friendly server errors while avoiding automatic retries. The change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
|
| Filename | Overview |
|---|---|
| internal/daemon/boot_memory_catalog.go | Retains an opened catalog store for operator access in every memory-automation configuration. |
| internal/daemon/runtime_dependencies.go | Supplies the retained catalog to shared HTTP and UDS handlers without enabling automation-specific services. |
| internal/daemon/daemon_shutdown_resources.go | Consolidates catalog closure under the published memory-catalog lifecycle handle. |
| internal/daemon/boot_checkpoint.go | Reuses the retained catalog for checkpoint coverage when the automatic memory store is absent. |
| internal/daemon/daemon_memory_e2e_integration_test.go | Covers populated, empty, and inaccessible catalogs over HTTP and UDS with automation disabled. |
| web/src/systems/knowledge/lib/query-options.ts | Disables automatic retries specifically for the paginated Knowledge list query. |
| web/src/systems/knowledge/adapters/knowledge-api.ts | Preserves typed list failures while presenting a readable generic message for server errors. |
| docs/qa/scenarios/MS-001.md | Documents disabled-automation listing, error recovery, source preservation, and scope-isolation regression checks. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
C[Knowledge UI / API / CLI] --> H[HTTP or UDS memory handlers]
H --> K[Retained memoryCatalogStore]
K --> D[(Catalog database)]
K --> F[Profile and workspace memory files]
E{memory.enabled} -->|true| A[Automatic memory runtime]
E -->|false| X[No prompt injection, extraction, or dreaming]
A --> K
S{session.compaction.enabled} -->|true| P[Checkpoint summary runtime]
P --> K
B[Daemon boot] --> K
K --> Q[Published runtime generation]
Q --> Z[Single shutdown ownership]
Reviews (1): Last reviewed commit: "fix: load knowledge memories without ena..." | Re-trigger Greptile
Problem and root cause
Closes #586.
With the shipped
memory.enabled = falsedefault, opening Command Palette → Knowledge requestsGET /api/memory?profile=default&scope=workspace&workspace_id=...and receives HTTP 500, including for an empty workspace.Tracing the palette's
useMemories→memoriesListOptions→listMemories→BaseHandlers.ListMemory→ scopedStore.CatalogHeadersexposed a boot dependency error: the daemon createdstate.memoryStoreonly when automatic memory was enabled. Disabled-memory boot migrated the catalog and either retained it privately for compaction or closed it. Public HTTP/UDS handlers therefore received a nil store and returnedmemory.internal. The existing boot feature-flag suite reproduced the failure in both memory-off combinations before the fix; memory-on combinations passed. The issue does not include the reporter's configuration, so this identifies a reproduced shipped-default cause rather than claiming inspection of their machine.The list query also inherited TanStack Query's automatic retry behavior, multiplying the failing request, while the adapter displayed a technical status message.
Resulting behavior and implementation
memoryCatalogStorefor operator API/CLI access regardless of automatic-memory enablement. Reuse it for compaction, publish it with the daemon generation, and close it once through the existing shutdown/failed-boot ownership paths.memoryStoreand its existing enablement checks intact. Listing Knowledge does not opt the user into prompt injection, extraction, background dreaming, or native agent memory writes.KnowledgeApiErrorwith their original status and a readable message; an explicit refetch can recover normally.Validation
Before-fix reproduction:
TestBootInjectsComposedAssemblerForFeatureFlagCombinationsfailed in the two memory-off cases because the public memory store was absent. After the fix, all four combinations pass with-race, preserving the existing prompt inclusion/exclusion assertions.CGO_ENABLED=1 go test -race ./internal/api/core -run '^TestMemoryHandlersAndHelpers$' -count=1— passed; includes existing workspace/profile isolation coverage.CGO_ENABLED=1 go test -race -tags integration ./internal/daemon -run '^TestDaemonE2EKnowledgeWithoutMemoryAutomation$' -count=1— passed. Real CLI-started daemons, real SQLite, unique temporary homes/workspaces/ports/sockets, HTTP and UDS requests, populated/empty/invalid-storage cases, source-byte preservation, and harness-owned process teardown. Both memory automation and compaction are disabled in these cases.bunx turbo run test --filter=./web -- src/systems/knowledge/hooks/__tests__/use-knowledge.test.tsx src/systems/knowledge/adapters/__tests__/knowledge-api.test.ts— 37 tests passed. The retry regression uses an actual QueryClient with three retries configured globally and proves one failed list attempt followed by successful explicit recovery.bunx turbo run build --filter=./web— passed, including the dependent web typecheck and codegen check.Final diff reviewed;
git diff --checkpassed. The integration test passes the test-convention helper; that helper reports existing violations in untouched portions of the olderdaemon_test.gofile.env -u COMPOZY_INTERNAL_RESTART_OPERATION_ID make gate— passed: Go lint (0 issues), race-enabled daemon/skills suites, web lint (0 warnings/errors), typecheck, and all 7,118 web tests across 771 files. Gate evidence is recorded under.cache/gate/.Published-head CI: CI run 34497282148 passed for
9526f110aaa57d5059a8990b6197ce30b147d55c, including Go race shards, static/build/platform checks, frontend lint/typecheck/tests/build, runtime E2E, desktop E2E, and all four web E2E shards. React Doctor and Vercel also passed; CodeRabbit approved and Greptile reported no actionable findings.Local daemon tests remove the inherited
COMPOZY_INTERNAL_RESTART_OPERATION_IDonly from their own process environment. No active host daemon was restarted or modified.Cross-surface and compatibility impact
Audit owner: this PR, following
docs/_memory/change-impact.md.ws_...reference accepted on ingress.Limits
Local execution was on macOS. No local Linux or Electron run, live ACP-provider validation, or manual browser walkthrough is claimed. Required PR CI is the delivery gate for the published head. The successful web build emitted existing warnings for
::highlightrules in unchanged session-search CSS, large chunks, and plugin timings; this change does not suppress them. The full web test suite also emits Reactact(...)and timer warnings from unchanged tests; scoped Knowledge tests pass without those warnings.