refactor: memory improvements

.compozy/tasks/mem-improvs/reviews-001/_meta.md

@@ -0,0 +1,11 @@
+---
+provider: coderabbit
+pr: "35"
+round: 1
+created_at: 2026-04-18T00:23:26.966343Z
+---
+
+## Summary
+- Total: 13
+- Resolved: 0
+- Unresolved: 13

.compozy/tasks/mem-improvs/reviews-001/issue_001.md

@@ -0,0 +1,24 @@
+---
+status: resolved
+file: go.mod
+line: 17
+severity: nitpick
+author: coderabbitai[bot]
+provider_ref: review:4132976935,nitpick_hash:c032c612cf2e
+review_hash: c032c612cf2e
+source_review_id: "4132976935"
+source_review_submitted_at: "2026-04-18T00:19:15Z"
+---
+
+# Issue 001: Confirm whether HTTPS proxy functionality in the unreleased gorilla/websocket commit is required.
+## Review Comment
+
+The pinned pseudo-version v1.5.4-0.20250319132907-e064f32e3674 points to an unreleased commit (23 commits ahead of v1.5.3, the latest stable release). This commit implements HTTPS proxy functionality. If this feature is not required by this PR, prefer the latest tagged release v1.5.3 to maintain better provenance tracking and easier vulnerability management.
+
+## Triage
+
+- Decision: `invalid`
+- Notes:
+  - The `gorilla/websocket` pseudo-version was already present in the repo before this memory batch; this PR only surfaced it as a direct requirement in `go.mod`.
+  - The current scoped changes do not modify Daytona websocket behavior, and the codebase gives no evidence that `v1.5.3` is a safe drop-in replacement for the existing sidecar transport.
+  - Changing this dependency here would be unrelated dependency churn with unclear behavioral impact, so no code change is warranted in this batch.

.compozy/tasks/mem-improvs/reviews-001/issue_002.md

@@ -0,0 +1,73 @@
+---
+status: resolved
+file: internal/cli/memory.go
+line: 177
+author: coderabbitai[bot]
+provider_ref: thread:PRRT_kwDOR5y4QM5745as,comment:PRRC_kwDOR5y4QM65BAP5
+---
+
+# Issue 002: _⚠️ Potential issue_ | _🟠 Major_
+## Review Comment
+
+_⚠️ Potential issue_ | _🟠 Major_
+
+**Accept multi-word queries here.**
+
+`cobra.ExactArgs(1)` plus `query := args[0]` makes the documented examples fail: `agh memory search auth rewrite` is parsed as 2 args and never reaches the search call. This blocks the common unquoted CLI case.
+
+
+<details>
+<summary>Suggested fix</summary>
+
+```diff
+-		Use:   "search <query>",
++		Use:   "search <terms...>",
+ 		Short: "Search durable memory",
+@@
+-		Args: cobra.ExactArgs(1),
++		Args: cobra.MinimumNArgs(1),
+ 		RunE: func(cmd *cobra.Command, args []string) error {
+@@
+-			query := strings.TrimSpace(args[0])
++			query := strings.TrimSpace(strings.Join(args, " "))
+ 			if query == "" {
+ 				return errors.New("memory query is required")
+ 			}
+```
+</details>
+
+
+Also applies to: 184-185
+
+<details>
+<summary>🤖 Prompt for AI Agents</summary>
+
+```
+Verify each finding against the current code and only fix it if needed.
+
+In `@internal/cli/memory.go` around lines 170 - 177, The CLI currently enforces
+ExactArgs(1) and uses query := args[0], which breaks multi-word queries like
+"agh memory search auth rewrite"; change the command's Args from
+cobra.ExactArgs(1) to cobra.MinimumNArgs(1), replace any direct use of args[0]
+with query := strings.Join(args, " "), and add an import for "strings" if
+missing; apply the same change to the other memory command mentioned (the block
+around the lines referenced 184-185) so both accept and join multi-word queries.
+```
+
+</details>
+
+<!-- fingerprinting:phantom:medusa:grasshopper -->
+
+<!-- This is an auto-generated comment by CodeRabbit -->
+
+## Triage
+
+- Decision: `valid`
+- Notes:
+  - `newMemorySearchCommand` currently uses `cobra.ExactArgs(1)` and `args[0]`, so unquoted multi-word searches like `agh memory search auth rewrite` fail argument validation before reaching the client.
+  - The fix is straightforward and localized: accept `cobra.MinimumNArgs(1)`, join `args` into one query string, and add CLI coverage for the common unquoted form.
+
+## Resolution
+
+- Updated `agh memory search` to accept `search <terms...>` with `cobra.MinimumNArgs(1)` and `strings.Join(args, " ")`.
+- Added CLI regression coverage for the unquoted multi-argument invocation.

.compozy/tasks/mem-improvs/reviews-001/issue_003.md

@@ -0,0 +1,31 @@
+---
+status: resolved
+file: internal/daemon/daemon_memory_e2e_integration_test.go
+line: 21
+severity: nitpick
+author: coderabbitai[bot]
+provider_ref: review:4132976935,nitpick_hash:a216b942a23f
+review_hash: a216b942a23f
+source_review_id: "4132976935"
+source_review_submitted_at: "2026-04-18T00:19:15Z"
+---
+
+# Issue 003: Please wrap these E2E scenarios in t.Run("Should...") subtests.
+## Review Comment
+
+Both tests pack several independent contract checks into one large flow, which makes failures harder to localize and drifts from the repository’s test shape. Splitting the main assertions into `t.Run("Should ...")` blocks would make regressions much easier to pinpoint.
+
+As per coding guidelines, `**/*_test.go`: MUST use t.Run("Should...") pattern for ALL test cases`.
+
+Also applies to: 225-225
+
+## Triage
+
+- Decision: `valid`
+- Notes:
+  - Both E2E tests currently bundle several independent contract checks into single large flows, which makes failures harder to localize and does not match the repository’s `t.Run("Should ...")` style.
+  - I will keep the scenario setup intact and split the assertion groups into explicit `Should ...` subtests so regressions point at the failing contract directly.
+
+## Resolution
+
+- Split both daemon memory E2E scenarios into focused `t.Run("Should ...")` subtests around search parity, reindex/health, stored message integrity, prompt recall injection, and stale index preservation.

.compozy/tasks/mem-improvs/reviews-001/issue_004.md

@@ -0,0 +1,27 @@
+---
+status: resolved
+file: internal/daemon/daemon_test.go
+line: 3146
+severity: nitpick
+author: coderabbitai[bot]
+provider_ref: review:4132976935,nitpick_hash:661e281738f2
+review_hash: 661e281738f2
+source_review_id: "4132976935"
+source_review_submitted_at: "2026-04-18T00:19:15Z"
+---
+
+# Issue 004: Extract the memory fixture serializer instead of duplicating raw frontmatter.
+## Review Comment
+
+This helper now embeds the same document shape already serialized in `internal/daemon/daemon_memory_e2e_integration_test.go:398-408`. Reusing that formatter, or extracting a shared helper, will keep both test suites aligned if frontmatter fields or newline handling change.
+
+## Triage
+
+- Decision: `valid`
+- Notes:
+  - `writeDaemonMemoryIndex` duplicates the same memory document serialization shape already provided by `memoryDocument(...)` in the E2E test file.
+  - Reusing the shared formatter inside package `daemon` removes format drift risk for frontmatter/newline handling without changing test intent.
+
+## Resolution
+
+- Extracted the shared `memoryDocument(...)` serializer into the non-tagged daemon test file set and reused it from `writeDaemonMemoryIndex`, so both unit and integration test builds share one formatter.

.compozy/tasks/mem-improvs/reviews-001/issue_005.md

@@ -0,0 +1,94 @@
+---
+status: resolved
+file: internal/memory/catalog.go
+line: 535
+author: coderabbitai[bot]
+provider_ref: thread:PRRT_kwDOR5y4QM5745ax,comment:PRRC_kwDOR5y4QM65BAP-
+---
+
+# Issue 005: _⚠️ Potential issue_ | _🟠 Major_
+## Review Comment
+
+_⚠️ Potential issue_ | _🟠 Major_
+
+<details>
+<summary>🧩 Analysis chain</summary>
+
+🌐 Web query:
+
+`In SQLite FTS5 MATCH syntax, are AND/OR/NOT/NEAR reserved operators, and what is the recommended way to search for those words literally?`
+
+💡 Result:
+
+Yes, AND, OR, and NOT are reserved boolean operators in SQLite FTS5 MATCH syntax (case-sensitive). NEAR is a reserved keyword for proximity groups. To search for these words literally as terms (e.g., rows containing the token "AND"), enclose them in double quotes: "AND", "OR", "NOT", "NEAR". Within quoted strings, escape embedded double quotes by doubling them (SQL-style). Barewords (unquoted tokens) that exactly match "AND", "OR", or "NOT" are not treated as strings but as operators, so quoting is required for literal search. NEAR as a bareword initiates a NEAR group, so quote it for literal matching. Examples: - MATCH '"AND"' -- matches rows containing token "AND" - MATCH '"one two"' -- phrase search - MATCH 'NEAR("AND" "OR")' -- NEAR group with literals "AND" and "OR"
+
+Citations:
+
+- 1: https://www.sqlite.org/fts5.html
+- 2: https://sqlite.org/fts5.html
+
+---
+
+
+
+</details>
+
+**Quote MATCH terms before concatenating them to prevent reserved operator interpretation.**
+
+`tokenizeSearchQuery()` emits operator keywords like `and`, `or`, `not`, and `near`. Joining them unquoted (e.g., `not found` or `auth or sessions`) causes FTS5 to interpret these as operators rather than literal search terms, producing incorrect results or syntax errors. Double-quote each token and join with explicit `AND`.
+
+<details>
+<summary>Suggested fix</summary>
+
+```diff
+ func buildCatalogMatchQuery(query string) (string, error) {
+ 	terms := tokenizeSearchQuery(query)
+ 	if len(terms) == 0 {
+ 		return "", wrapValidationError("search query", query, errors.New("query is required"))
+ 	}
+-	return strings.Join(terms, " "), nil
++	quoted := make([]string, 0, len(terms))
++	for _, term := range terms {
++		quoted = append(quoted, fmt.Sprintf("%q", term))
++	}
++	return strings.Join(quoted, " AND "), nil
+ }
+```
+</details>
+
+Also applies to: 537-554
+
+<details>
+<summary>🤖 Prompt for AI Agents</summary>
+
+```
+Verify each finding against the current code and only fix it if needed.
+
+In `@internal/memory/catalog.go` around lines 529 - 535, The current
+buildCatalogMatchQuery uses tokenizeSearchQuery terms unquoted, letting FTS5
+treat words like "and/or/not/near" as operators; update buildCatalogMatchQuery
+(and the similar logic at the other block around lines 537-554) to wrap each
+token in double quotes and join them with explicit AND (e.g., "\"token\" AND
+\"token\"") so every token is treated as a literal phrase; reference the
+tokenizeSearchQuery call to get tokens, map each token to a quoted form, and
+return strings.Join(quotedTokens, " AND "), preserving existing error handling.
+```
+
+</details>
+
+<!-- fingerprinting:phantom:medusa:grasshopper -->
+
+<!-- This is an auto-generated comment by CodeRabbit -->
+
+## Triage
+
+- Decision: `valid`
+- Notes:
+  - `buildCatalogMatchQuery` currently concatenates raw search tokens into an FTS5 `MATCH` expression.
+  - Tokens such as `and`, `or`, `not`, and `near` are parsed as operators instead of literal terms, which can change results or produce syntax errors for otherwise normal user queries.
+  - I will quote each token, join them with explicit `AND`, and add regression coverage through the store search path.
+
+## Resolution
+
+- Quoted every catalog search token and joined terms with explicit `AND` so FTS5 reserved keywords are treated as literals.
+- Added a store search regression test proving queries like `not` work through the catalog path instead of failing as operator syntax.

.compozy/tasks/mem-improvs/reviews-001/issue_006.md

@@ -0,0 +1,62 @@
+---
+status: resolved
+file: internal/memory/recall.go
+line: 46
+author: coderabbitai[bot]
+provider_ref: thread:PRRT_kwDOR5y4QM5745az,comment:PRRC_kwDOR5y4QM65BAQB
+---
+
+# Issue 006: _⚠️ Potential issue_ | _🟠 Major_
+## Review Comment
+
+_⚠️ Potential issue_ | _🟠 Major_
+
+**Keep recall augmentation best-effort.**
+
+Returning the search error here aborts prompt delivery for an auxiliary feature. Any transient catalog issue—or a query-parse failure from the search layer—will fail the whole turn instead of just skipping recall.
+
+
+<details>
+<summary>Suggested fix</summary>
+
+```diff
+ 		results, err := target.Search(ctx, query, SearchOptions{
+ 			Workspace: workspaceRoot,
+ 			Limit:     maxRecallResults,
+ 		})
+ 		if err != nil {
+-			return message, err
++			return message, nil
+ 		}
+```
+</details>
+
+<details>
+<summary>🤖 Prompt for AI Agents</summary>
+
+```
+Verify each finding against the current code and only fix it if needed.
+
+In `@internal/memory/recall.go` around lines 41 - 46, The recall augmentation
+should be best-effort: when calling target.Search(ctx, query,
+SearchOptions{Workspace: workspaceRoot, Limit: maxRecallResults}) and receiving
+a non-nil err from the results, do not return the error from this function;
+instead log the error (including context like query and workspaceRoot) and
+continue execution with an empty/zero results set so the function returns the
+original message; update the error-handling around the results, err assignment
+to handle failures by logging and skipping recall rather than returning err.
+```
+
+</details>
+
+<!-- fingerprinting:phantom:medusa:grasshopper -->
+
+<!-- This is an auto-generated comment by CodeRabbit -->
+
+## Triage
+
+- Decision: `invalid`
+- Notes:
+  - The current prompt path already keeps augmentation best-effort for ordinary failures. `Manager.Prompt` logs augmenter errors and still dispatches the original message, so a recall search error does not abort prompt delivery today.
+  - Swallowing all errors inside `NewRecallAugmenter` would also hide cancellation/deadline signals once cancellation handling is corrected in `manager_prompt.go`.
+  - The real bug is in manager-level cancellation propagation, which is addressed under issue 011. No change is needed in `internal/memory/recall.go`.