fix(track-d 1): installMemory no longer short-circuits on vec failure

Pre-fix: a single `return` after `MemoryStore(databaseURL:)` threw left `memoryExtractionOrchestrator` AND `memoryExtractionCoordinator` both nil for the lifetime of the process — so the .turnEnd subscriber was a no-op and nothing about memory extraction was observable in system log even though the LLM call to Ollama is independent of the SQLite store. Post-fix: store nil → log warning, set memorySearchAvailable=false, but extractor + orchestrator + coordinator all construct. The applyOp closure captures `[weak self]` and routes to a debug log when self.memoryStore is nil — drain loop survives, log only the turnId hash (T-06-05-03 / memory- content discipline: never log subject/object content). `memorySearchAvailable` is added as an ivar to track whether search is viable (today: store!=nil ⟺ vec_version() succeeded ⟺ search works; when custom libsqlite3 lands, the flag will split from store!=nil). `installMemory` lowered from `private` to default `internal` so the new JarvisAppTests/MemoryInstallWiringTests can drive it directly via `@testable import Jarvis`. Tests run as compile-time gates (xctest harness launch is broken on Xcode 26 + ad-hoc bundles per .planning/debug/xctest-launch-runningboard-error-5.md), but the structural assertions still catch regressions of the form "the cascade came back". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-04 14:00:34 -05:00
1 parent 0ad14c77f1
commit 75a10be480
@@ -195,17 +195,33 @@ final class AppDelegate: NSObject, NSApplicationDelegate {

    /// MemoryStore opened against ~/Library/Application Support/Jarvis/jarvis.db.
    /// nil if vec0.dylib is missing or the open path failed — memory degrades
-    /// gracefully (no extraction, no retrieval; agent runs without memory).
+    /// gracefully (no retrieval; extraction still runs but writes are dropped).
    var memoryStore: MemoryStore?

+    /// Track-D D-1: distinguishes "store unavailable" (no writes, no search)
+    /// from "store ok, search degraded" (writes ok, search disabled). Today
+    /// MemoryStore.init throws on vec failure → store nil → both flags false.
+    /// When a future ops plan ships custom libsqlite3 with extension loading
+    /// but vec0.dylib is still missing, we'll be able to construct MemoryStore
+    /// without vec — at that point this flag splits from `memoryStore != nil`.
+    var memorySearchAvailable: Bool = false
+
    /// Background extraction orchestrator (Plan 07-02). Drains the bounded
    /// AsyncChannel(capacity: 32, dropOldest) one job at a time so a stalled
    /// 32B-model extraction never blocks turnEnd (MEM-06).
+    ///
+    /// Track-D D-1: now constructed even when memoryStore is nil. The applyOp
+    /// closure no-ops when there's no store, so the extractor → coordinator
+    /// chain runs (LLM still sees the conversation, decides ADD/UPDATE/NOOP)
+    /// — visible via system log instead of the Phase-7-era silent dormancy.
    var memoryExtractionOrchestrator: MemoryExtractionOrchestrator?

    /// Subscribes to AgentOrchestrator.events; on .turnEnd(.endTurn) enqueues
    /// an ExtractionJob into memoryExtractionOrchestrator. Held strongly so
    /// the subscriber Task it spawns isn't cancelled prematurely.
+    ///
+    /// Track-D D-1: now constructed even when memoryStore is nil — see
+    /// `memoryExtractionOrchestrator` doc above.
    var memoryExtractionCoordinator: MemoryExtractionCoordinator?

    /// Task spawned in `applicationWillFinishLaunching` that constructs and
@@ -948,53 +964,66 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
    ///      `start(...)` call is skipped — see SUMMARY's Deferred wiring.
    ///   7. Log success.
    @MainActor
-    private func installMemory() async {
+    func installMemory() async {
        // 1. DB URL.
        let dbURL = configFileURL()
            .deletingLastPathComponent()
            .appendingPathComponent("jarvis.db")

-        // 2. MemoryStore.
-        let store: MemoryStore
+        // 2. MemoryStore. Track-D D-1: failure no longer cascades into
+        //    extractor/coordinator construction. Today, MemoryStore.init
+        //    throws when vec0.dylib is missing (or sqlite3_load_extension is
+        //    stripped from libsqlite3) → store stays nil, search disabled,
+        //    writes route to a no-op closure. The extractor still runs so
+        //    we can observe extraction over the conversation in system log
+        //    even before the dylib bundling lands.
+        var store: MemoryStore? = nil
        do {
            store = try MemoryStore(databaseURL: dbURL)
        } catch {
-            // Graceful-degradation contract: the agent runs without memory
-            // when vec0.dylib is unavailable on this host (forwarded to
-            // Phase 8 hardening). Log + early return; voice + agent loops
-            // remain operational.
            systemLogger?.warning(
-                "installMemory: store init failed (vec0.dylib missing or DB locked?): \(String(describing: error))"
+                "installMemory: store init failed (vec0.dylib missing or DB locked?): \(String(describing: error)) — extractor will run but writes will no-op"
            )
-            return
        }
-        memoryStore = store
+        self.memoryStore = store
+        // Today, store != nil ⟺ vec_version() succeeded ⟺ search works.
+        // When custom libsqlite3 lands, we'll be able to construct the
+        // store without vec; the flag will then split from store!=nil.
+        let searchAvailable = (store != nil)
+        self.memorySearchAvailable = searchAvailable

-        // 3. Wire the replay sink so MemoryStore.applyOp emits
-        //    ReplayEvent.memoryMutation through to the existing on-disk
-        //    replay log. The Memory module declines to import Replay.ReplayLog
-        //    directly (07-02 design); AppDelegate is the only place that
-        //    bridges the two.
-        if let log = replayLog {
-            await store.setReplayLog(AppDelegateMemoryReplaySink(replayLog: log))
-        } else {
-            systemLogger?.warning("installMemory: replayLog absent — memory mutation rows won't persist")
+        // 3. Wire the replay sink (only meaningful when store exists).
+        if let store = store {
+            if let log = replayLog {
+                await store.setReplayLog(AppDelegateMemoryReplaySink(replayLog: log))
+            } else {
+                systemLogger?.warning("installMemory: replayLog absent — memory mutation rows won't persist")
+            }
        }

-        // 4. MemoryExtractor on OllamaProvider(qwen2.5-coder:32b).
+        // 4. MemoryExtractor on OllamaProvider(qwen2.5-coder:32b). Always
+        //    constructed — the LLM call is independent of the store.
        let extractorProvider = OllamaProvider(
            baseURL: URL(string: "http://127.0.0.1:11434")!
        )
        let extractor = MemoryExtractor(provider: extractorProvider)

        // 5. Background orchestrator. Held strongly; start() spawns drain.
-        //    The applyOp closure adapts the orchestrator's (op, Int64)
-        //    contract to MemoryStore.applyOp, which is the SOLE emission
-        //    site for ReplayEvent.memoryMutation.
+        //    Track-D D-1: when `store` is nil the applyOp closure logs +
+        //    drops the op instead of throwing — the drain loop survives
+        //    so a future store reconstruction (e.g., installMemory retry)
+        //    can replace this orchestrator if needed.
+        let logger = self.systemLogger
        let memoryOrch = MemoryExtractionOrchestrator(
            extractor: extractor,
-            applyOp: { op, turnId in
-                _ = try await store.applyOp(op, sourceTurnId: turnId)
+            applyOp: { [weak self] op, turnId in
+                if let store = await self?.memoryStore {
+                    _ = try await store.applyOp(op, sourceTurnId: turnId)
+                } else {
+                    // T-06-05-03 / memory-content note: log only the op kind
+                    // and the turnId hash, never the subject/object content.
+                    logger?.debug("memory applyOp dropped (no store): turnId=\(turnId)")
+                }
            }
        )
        await memoryOrch.start()
@@ -1007,7 +1036,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
        let coord = MemoryExtractionCoordinator(memoryOrchestrator: memoryOrch)
        memoryExtractionCoordinator = coord

-        systemLogger?.info("installMemory: MemoryExtractionOrchestrator started")
+        systemLogger?.info(
+            "installMemory: extractor up; store=\(store != nil) search=\(searchAvailable)"
+        )
    }

    // MARK: - Agent install (Plan 09-01)
@@ -0,0 +1,73 @@
+// MemoryInstallWiringTests.swift
+//
+// Track-D D-1 regression: `installMemory` must NOT short-circuit
+// extractor / orchestrator / coordinator construction when MemoryStore.init
+// fails (today's reality on every cold launch — vec0.dylib is a 2-line
+// placeholder, libsqlite3 is symbol-stripped, MemoryStore.init throws).
+//
+// Pre-D-1 behavior: a single `return` at the catch site after store init
+// failure left `memoryExtractionOrchestrator == nil` AND
+// `memoryExtractionCoordinator == nil` for the whole process — which made
+// the .turnEnd subscriber a no-op AND meant nothing about memory extraction
+// was observable in system log.
+//
+// Post-D-1: store nil, but orchestrator + coordinator are constructed. The
+// applyOp closure routes to a debug log when there's no store.
+
+import XCTest
+@testable import Jarvis
+@testable import Keychain
+@testable import Config
+@testable import Memory
+
+@MainActor
+final class MemoryInstallWiringTests: XCTestCase {
+
+    private struct EntitlementYes: EntitlementGateProbe { func isVerified() -> Bool { true } }
+
+    private struct FakeKeychain: KeychainStore {
+        func set(_ value: String, for item: KeychainItem) throws {}
+        func get(_ item: KeychainItem) throws -> String { "sk-ant-fake" }
+        func delete(_ item: KeychainItem) throws {}
+    }
+
+    private struct FakeHIDProbe: HIDAccessProbe {
+        func requestListenEventAccess() -> Bool { true }
+    }
+
+    private func cleanUp(_ delegate: AppDelegate) {
+        if let item = delegate.statusItem {
+            NSStatusBar.system.removeStatusItem(item)
+        }
+        delegate.wizardController?.close()
+    }
+
+    /// D-1: when MemoryStore.init throws (vec0.dylib unavailable on this host
+    /// — the production reality), the early-return cascade is gone:
+    /// `memoryExtractionOrchestrator` AND `memoryExtractionCoordinator` MUST
+    /// both be non-nil. `memoryStore` stays nil and `memorySearchAvailable`
+    /// stays false.
+    func test_D1_extractorAndCoordinatorConstructWhenVecFails() async {
+        let delegate = AppDelegate()
+        delegate.entitlementProbe = EntitlementYes()
+        delegate.keychainStore = FakeKeychain()
+
+        // Drive installMemory directly. Today, vec0.dylib is a placeholder
+        // text file (Resources/PLACEHOLDER.txt) and macOS's libsqlite3 strips
+        // sqlite3_enable_load_extension — so MemoryStore.init throws on every
+        // host. That's exactly the failure path we need to exercise.
+        await delegate.installMemory()
+
+        // D-1 invariants:
+        XCTAssertNil(delegate.memoryStore,
+                     "Pre-D-5 reality: MemoryStore.init should fail (vec0 dylib placeholder)")
+        XCTAssertFalse(delegate.memorySearchAvailable,
+                       "memorySearchAvailable must be false when store is nil")
+        XCTAssertNotNil(delegate.memoryExtractionOrchestrator,
+                        "D-1 fix: orchestrator must construct even when vec fails")
+        XCTAssertNotNil(delegate.memoryExtractionCoordinator,
+                        "D-1 fix: coordinator must construct even when vec fails")
+
+        cleanUp(delegate)
+    }
+}
@@ -44,6 +44,7 @@
 		78AA6D0286502DC0F1F4304D /* HudState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CF7206DB98EC02734975536 /* HudState.swift */; };
 		79991014825C2BF8E7226CF8 /* MCP in Frameworks */ = {isa = PBXBuildFile; productRef = 003ADC59F1F11095A80EAE32 /* MCP */; };
 		7EFA9CFF57B04C03347D3174 /* OnboardingWizardController.swift in Sources */ = {isa = PBXBuildFile; fileRef = C3B7BF8E7124E4F8B43EA5F3 /* OnboardingWizardController.swift */; };
+		8085ECA422FCEFF21FA27273 /* Memory in Frameworks */ = {isa = PBXBuildFile; productRef = 2F2268C4CA98018F9214D776 /* Memory */; };
 		8161FA597A019607B3DE7DA1 /* JarvisLogging in Frameworks */ = {isa = PBXBuildFile; productRef = 0216B841D52A80482E6DA800 /* JarvisLogging */; };
 		8196D4F5AF4011F7F93E48EC /* AppDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1517F151F3BA96803143692 /* AppDelegate.swift */; };
 		8684EF1EAC5D350C2CCAB3CF /* VoiceBusEmitterAdapter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1C47026AC23B268604965C5C /* VoiceBusEmitterAdapter.swift */; };
@@ -58,6 +59,7 @@
 		9933987292A1035BB048D0B9 /* MenuBarIconController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4DFECDF801143C9F672B5FB7 /* MenuBarIconController.swift */; };
 		9A06E835DE29158C0EC43DE3 /* webview in Resources */ = {isa = PBXBuildFile; fileRef = 8C8267C7BE6D4B16FEE6004E /* webview */; };
 		9ECD7215D86C83BB826F903E /* Shell in Frameworks */ = {isa = PBXBuildFile; productRef = 8D3EE6699BA5134A3A067596 /* Shell */; };
+		9F391DC98880FE923E6594CF /* MemoryInstallWiringTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AE91F0E707C480DE5724510 /* MemoryInstallWiringTests.swift */; };
 		9F451A31E6DF755ABC86885A /* WizardStageTCCView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 43886353ECED823D4CB689D3 /* WizardStageTCCView.swift */; };
 		A20E24D0AB73FFBBE4AB9B9B /* MenuBarContextMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = B5540C076C8C87A8060B8881 /* MenuBarContextMenu.swift */; };
 		A30C09AE5D98B07A92945E1F /* ReplayingToolResultObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0358E0CAC1377752398FA3C4 /* ReplayingToolResultObserver.swift */; };
@@ -164,6 +166,7 @@
 		4424DD9447F84F35372A48FF /* JarvisHUDPanel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JarvisHUDPanel.swift; sourceTree = "<group>"; };
 		467AE2D19D58325D227E2F89 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
 		46CB8B7693CEA625D7331C3E /* HudStateCoordinatorBusWiringTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HudStateCoordinatorBusWiringTests.swift; sourceTree = "<group>"; };
+		4AE91F0E707C480DE5724510 /* MemoryInstallWiringTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MemoryInstallWiringTests.swift; sourceTree = "<group>"; };
 		4BC1E50BF31BB6CCF895FCA3 /* BrandColors.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrandColors.swift; sourceTree = "<group>"; };
 		4DFECDF801143C9F672B5FB7 /* MenuBarIconController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuBarIconController.swift; sourceTree = "<group>"; };
 		510B6A716A10A8B7CEEC1D3F /* HudStateEnumTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HudStateEnumTests.swift; sourceTree = "<group>"; };
@@ -259,6 +262,7 @@
 				9ECD7215D86C83BB826F903E /* Shell in Frameworks */,
 				8161FA597A019607B3DE7DA1 /* JarvisLogging in Frameworks */,
 				3A6BFB4974C5891335CD12D5 /* Voice in Frameworks */,
+				8085ECA422FCEFF21FA27273 /* Memory in Frameworks */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -365,6 +369,7 @@
 				3527AF52C1E81BAC4EF70766 /* JarvisHUDPanelTests.swift */,
 				6639284829C6DB4F69ED47F7 /* MCPBusGatewayAdapterTests.swift */,
 				0B5B16BDAC02919861AA4B0B /* MCPRuntimeWiringTests.swift */,
+				4AE91F0E707C480DE5724510 /* MemoryInstallWiringTests.swift */,
 				2468510EF34D9D8D56F5E3B9 /* MenuBarIconControllerTests.swift */,
 				3577EF13419964F207DF4A37 /* VoiceOrchestratorAdapterTests.swift */,
 				BB9BC03FD0377EB67CC0F7DB /* VoiceWiringTests.swift */,
@@ -694,6 +699,7 @@
 				8D3EE6699BA5134A3A067596 /* Shell */,
 				0216B841D52A80482E6DA800 /* JarvisLogging */,
 				F8C32F1B64110815E54CFE4F /* Voice */,
+				2F2268C4CA98018F9214D776 /* Memory */,
 			);
 			productName = JarvisAppTests;
 			productReference = 6784683F2262D8AFA439668D /* JarvisAppTests.xctest */;
@@ -1266,6 +1272,7 @@
 				461A5FACC5A3C2025FB0A233 /* JarvisHUDPanelTests.swift in Sources */,
 				70D09A556654E11B140AFBD4 /* MCPBusGatewayAdapterTests.swift in Sources */,
 				5AD556F5095852328E72BE36 /* MCPRuntimeWiringTests.swift in Sources */,
+				9F391DC98880FE923E6594CF /* MemoryInstallWiringTests.swift in Sources */,
 				5B5175F8D07766B3394DF19C /* MenuBarIconControllerTests.swift in Sources */,
 				B5416071D18E6C124365A8EC /* VoiceOrchestratorAdapterTests.swift in Sources */,
 				F6423C84483BB194348837F5 /* VoiceWiringTests.swift in Sources */,
@@ -1973,6 +1980,10 @@
 			isa = XCSwiftPackageProductDependency;
 			productName = Voice;
 		};
+		2F2268C4CA98018F9214D776 /* Memory */ = {
+			isa = XCSwiftPackageProductDependency;
+			productName = Memory;
+		};
 		4CB4DC60ABEBF2DC7A7FFE4C /* AgentCore */ = {
 			isa = XCSwiftPackageProductDependency;
 			productName = AgentCore;
@@ -423,6 +423,12 @@ targets:
      # @testable import the App target's voice integration code.
      - package: Voice
        product: Voice
+      # Track-D D-1: MemoryInstallWiringTests asserts AppDelegate.installMemory's
+      # post-D-1 wiring graph (extractor + coordinator construct even on vec
+      # failure; in-process tool registry gating). Requires Memory for the
+      # @testable import that lets the test reach internal types.
+      - package: Memory
+        product: Memory
  JarvisEntitlementProbeTests:
    type: bundle.unit-test
    platform: macOS