feat: durable candidates MVP + scoped traceable recall - #1050
feat: durable candidates MVP + scoped traceable recall#1050MarcoLissitzky wants to merge 6 commits into
Conversation
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
Update all documentation to reflect the 6-commit feature branch: - AGENTS.md: stats bump (184 src files, ~42,400 LOC, 1,500+ tests, 296 functions, 58 KV scopes), new recall code pattern section - README.md: stat line update, skills catalog (10 invocable + 7 reference), scoped traceable recall section, recall TOML config, durable candidates env vars, stale skill counts (15→17, 8→10 invocable) - plugin.json: refresh description with recall capabilities - Skill reference docs regenerated via npm run skills:gen All 6 tool-count-consistency tests pass. Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
|
@MarcoLissitzky is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds centralized scoped recall with persisted traces, configurable token budgets, identity-aware integrations, durable candidate extraction and promotion, archive replay improvements, retrieval health tracking, and recall debugging surfaces across APIs, MCP, hooks, and the viewer. ChangesRecall and durable candidate flow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/functions/remember.ts (1)
95-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLegacy unscoped memories are not treated as wildcards here.
sameScope()requires the scope level to match, so a legacyunknownmemory will not be superseded by a new project-scoped write (and vice versa). If that crossover should still dedupe existing rows, relax this guard; otherwise update the comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/functions/remember.ts` around lines 95 - 101, Update the supersession guard in the remember flow around sameScope and normalizeScope so legacy unscoped memories are treated as wildcards when deduplicating against project-scoped writes, while preserving the protection for explicitly different projects; alternatively, if crossover deduplication is not intended, revise the nearby comment to accurately describe sameScope behavior.
🧹 Nitpick comments (6)
test/vector-retrieval-health.test.ts (1)
5-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
afterEachto guarantee fake-timer cleanup.
vi.useRealTimers()on line 18 won't execute if any assertion between lines 8–17 throws, leaking fake timers into the second test. Move cleanup to anafterEachhook.♻️ Proposed refactor
+afterEach(() => vi.useRealTimers()); + describe("VectorRetrievalHealth", () => { it("opens after transient failures and allows only one half-open probe", () => { vi.useFakeTimers(); const health = new VectorRetrievalHealth(2, 1000); expect(health.begin(true, true)).toMatchObject({ status: "healthy", attempted: true }); health.failure(new Error("429 quota exceeded")); expect(health.begin(true, true)).toMatchObject({ status: "healthy", attempted: true }); health.failure(new Error("429 quota exceeded")); expect(health.begin(true, true)).toMatchObject({ status: "degraded", attempted: false }); vi.advanceTimersByTime(1000); expect(health.begin(true, true)).toMatchObject({ status: "healthy", attempted: true }); expect(health.begin(true, true)).toMatchObject({ status: "degraded", attempted: false }); health.success(); expect(health.begin(true, true)).toMatchObject({ status: "healthy", attempted: true }); - vi.useRealTimers(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/vector-retrieval-health.test.ts` around lines 5 - 19, Move fake-timer cleanup from the test body into an afterEach hook for the test suite containing “opens after transient failures and allows only one half-open probe.” Keep vi.useFakeTimers() in the test setup, and ensure the afterEach hook always calls vi.useRealTimers(), including when an assertion fails.test/remember-project-scope.test.ts (1)
206-256: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUpdate the stale isolation comment in
src/functions/remember.ts.sameScope()already treats unknown-vs-scoped memories as different scopes, so the “wildcard” note no longer matches the code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/remember-project-scope.test.ts` around lines 206 - 256, Update the stale isolation comment in sameScope() within remember.ts to accurately describe that unknown and scoped memories are treated as different scopes; remove the outdated “wildcard” characterization without changing the existing scope-comparison logic or tests.src/eval/schemas.ts (1)
106-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
DurableCandidateTypeEnuminstead of duplicating the literal list.
RememberInputSchema.typere-declares the same six candidate types already defined inDurableCandidateTypeEnum(Line 35). These can silently drift apart when the candidate taxonomy changes. Reference the shared enum.♻️ Proposed change
- type: z - .enum(["pattern", "preference", "architecture", "bug", "workflow", "fact"]) - .optional(), + type: DurableCandidateTypeEnum.optional(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eval/schemas.ts` around lines 106 - 108, Update RememberInputSchema.type to reuse DurableCandidateTypeEnum instead of declaring a separate z.enum literal list, while preserving its optional behavior.src/recall/tokens.ts (1)
3-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCJK-only special-casing under-covers other multi-byte scripts.
CJK_CHARcovers Hiragana/Katakana/CJK ideographs but omits Hangul (\uac00-\ud7a3), CJK punctuation (\u3000-\u303f), and fullwidth forms (\uff00-\uffef). Text in those ranges falls into the ASCII-run branch and gets divided by 4, understating token cost for a "conservative-unicode" estimator that should err toward overestimating.♻️ Broaden multi-byte detection
-const CJK_CHAR = /[\u3040-\u30ff\u3400-\u9fff\uf900-\ufaff]/u; +const WIDE_CHAR = /[\u3000-\u303f\u3040-\u30ff\u3400-\u9fff\uac00-\ud7a3\uf900-\ufaff\uff00-\uffef]/u;Then replace
CJK_CHAR.test(char)withWIDE_CHAR.test(char)insidecountTokens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/recall/tokens.ts` around lines 3 - 26, Broaden the character-range constant used by countTokens from CJK-only detection to WIDE_CHAR, including Hangul, CJK punctuation, and fullwidth forms; then replace the CJK_CHAR.test(char) check with WIDE_CHAR.test(char) so these characters are counted individually rather than as part of a four-character run.src/recall/trace-store.ts (1)
48-60: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffRetention trimming runs a full scan on every recall.
persistRecallTracelists the entirerecallTracesscope and issues per-entry deletes on every single recall. As traffic grows this turns each recall into an O(n) list plus a burst ofstate::deletecalls on the request path. Consider trimming opportunistically (e.g., only when the store exceedsmaxTracesby a margin, or on a periodic job) rather than every write.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/recall/trace-store.ts` around lines 48 - 60, Update persistRecallTrace around the KV.recallTraces retention logic to avoid listing and deleting entries on every recall. Move trimming to an opportunistic threshold or periodic cleanup path, while preserving retentionDays and maxTraces enforcement when cleanup runs; keep the trace write on the normal request path.src/recall/core.ts (1)
357-378: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSemantic hydration issues KV reads serially per hit.
hybridRecallis fetched with a limit of up tomax(limit*5, 50), then each hit triggers a serialkv.get(memories)(and asessionForlookup). On a warm corpus this is dozens of sequentialstate::getround-trips per recall. Consider batching the memory lookups withPromise.allbefore building candidates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/recall/core.ts` around lines 357 - 378, Update hybridRecall’s hit hydration flow to batch per-hit memory and session lookups with Promise.all instead of awaiting kv.get and sessionFor serially inside the loop. Preserve the existing memory-versus-observation candidate construction and field values while pairing each hit with its resolved memory/session data before pushing candidates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 1266: Reconcile the test-count statistic in the README by updating the
development section’s outdated “1,423+ tests” value to match the current “1,500+
tests” project statistic. Keep the surrounding development guidance unchanged.
- Line 1590: Update the endpoint count in the README REST API description from
136 to 125, leaving the surrounding port, binding, and authorization details
unchanged.
In `@src/config.ts`:
- Around line 183-187: Update the error-label argument in the
maxSessionSummaries configuration within the positiveConfigInt call to use the
TOML key format, recall_budget.max_session_summaries, matching envOrToml and the
actual config.toml key.
In `@src/functions/context.ts`:
- Around line 40-64: Add an optional limit?: number field to the mem::context
input type and forward data.limit in the recallCore.recall request. Update only
the registration callback’s input definition and recall payload, preserving
existing defaults and behavior when limit is omitted.
In `@src/functions/durable-candidates.ts`:
- Around line 683-759: Update the archive import flow in archive processing and
the mem::replay::import-jsonl trigger to use one session ID: pass
parsed.sessionId in the trigger payload and make the importer use that supplied
ID instead of generating a fallback, so the subsequent kv.get(KV.sessions,
parsed.sessionId) resolves the imported session.
In `@src/functions/observe.ts`:
- Around line 182-192: The rollback branch in observe should stop directly
deleting raw.imageData and instead call decrementImageRef with kv, sdk, and
raw.imageData. Preserve the existing conditional and ensure the decrement
operation handles reference counts, embeddings, and disk-size accounting
consistently.
In `@src/hooks/pre-compact.ts`:
- Around line 51-65: Isolate the `context-epoch` fetch from the primary context
retrieval error handling in the surrounding pre-compact hook. Ensure failures
from the epoch call, including timeout or connection errors, are caught or
otherwise ignored so execution always proceeds to the `context` fetch, while
preserving the existing context-fetch behavior.
In `@src/mcp/tools-registry.ts`:
- Around line 35-41: Constrain the outputMode property in the memory_recall tool
schema to accept only the supported structured and context values, matching the
handling in the server. Reject unsupported values at schema validation before
they reach the server while preserving the existing descriptions and behavior
for valid modes.
In `@src/recall/identity.ts`:
- Around line 15-23: Update normalizeRemote to remove the URL scheme, including
git://, ssh://, and https:// forms, before producing the normalized value.
Ensure scheme-based URLs and SCP-style SSH remotes for the same repository yield
the identical host/path representation while preserving credential, .git,
trailing-slash, and lowercase normalization.
- Around line 25-50: Update resolveRecallIdentity and its gitValue usage so
request-supplied cwd is validated against an allowed workspace root before any
git probe, rejecting or falling back safely for paths outside that root.
Preserve identity resolution for valid workspaces, and ensure invalid input
cannot trigger synchronous execFileSync calls on arbitrary host paths.
In `@src/recall/trace-store.ts`:
- Around line 62-80: The recall-stat updates in the selected and scope-mismatch
processing still use non-atomic get/recompute/set logic. Replace both update
paths in the trace handling flow with state::update and atomic increment
operations for recallCount and scopeMismatchCount, while preserving the
averageScore and metadata calculations through the update mechanism so
concurrent recalls cannot overwrite each other.
In `@src/state/schema.ts`:
- Line 8: Update the shared type definitions in types.ts by moving or adding
InjectionLedgerEntry from recall/ledger.ts and defining the
durableRecommendations KV record shape there, alongside ArchiveImportRecord,
RecallTrace, and RecallItemStats. Then update consumers such as the ledger and
KV schema usage to import these shared types, preserving the existing fields and
behavior.
In `@src/triggers/api.ts`:
- Around line 1250-1254: Remove the redundant checkoutId spread from the
remember payload construction near the existing repoId and checkoutId selection
logic. Keep the earlier checkoutId expression that uses req.body.checkoutId or
identity.checkoutId, and leave the other payload fields unchanged.
In `@src/viewer/index.html`:
- Around line 2424-2437: Update the droppedCountsByDecision rendering inside the
traces.map callback to coerce each dropped[k] value to a string and pass it
through esc before concatenating it into the innerHTML string, while preserving
the existing escaped-key formatting.
---
Outside diff comments:
In `@src/functions/remember.ts`:
- Around line 95-101: Update the supersession guard in the remember flow around
sameScope and normalizeScope so legacy unscoped memories are treated as
wildcards when deduplicating against project-scoped writes, while preserving the
protection for explicitly different projects; alternatively, if crossover
deduplication is not intended, revise the nearby comment to accurately describe
sameScope behavior.
---
Nitpick comments:
In `@src/eval/schemas.ts`:
- Around line 106-108: Update RememberInputSchema.type to reuse
DurableCandidateTypeEnum instead of declaring a separate z.enum literal list,
while preserving its optional behavior.
In `@src/recall/core.ts`:
- Around line 357-378: Update hybridRecall’s hit hydration flow to batch per-hit
memory and session lookups with Promise.all instead of awaiting kv.get and
sessionFor serially inside the loop. Preserve the existing
memory-versus-observation candidate construction and field values while pairing
each hit with its resolved memory/session data before pushing candidates.
In `@src/recall/tokens.ts`:
- Around line 3-26: Broaden the character-range constant used by countTokens
from CJK-only detection to WIDE_CHAR, including Hangul, CJK punctuation, and
fullwidth forms; then replace the CJK_CHAR.test(char) check with
WIDE_CHAR.test(char) so these characters are counted individually rather than as
part of a four-character run.
In `@src/recall/trace-store.ts`:
- Around line 48-60: Update persistRecallTrace around the KV.recallTraces
retention logic to avoid listing and deleting entries on every recall. Move
trimming to an opportunistic threshold or periodic cleanup path, while
preserving retentionDays and maxTraces enforcement when cleanup runs; keep the
trace write on the normal request path.
In `@test/remember-project-scope.test.ts`:
- Around line 206-256: Update the stale isolation comment in sameScope() within
remember.ts to accurately describe that unknown and scoped memories are treated
as different scopes; remove the outdated “wildcard” characterization without
changing the existing scope-comparison logic or tests.
In `@test/vector-retrieval-health.test.ts`:
- Around line 5-19: Move fake-timer cleanup from the test body into an afterEach
hook for the test suite containing “opens after transient failures and allows
only one half-open probe.” Keep vi.useFakeTimers() in the test setup, and ensure
the afterEach hook always calls vi.useRealTimers(), including when an assertion
fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88c4ec68-facb-48f7-bb0c-a18f7e81573e
📒 Files selected for processing (73)
.env.example.gitignoreAGENTS.mdREADME.mddocs/continuity-schema-note.mddocs/p1-durable-candidates-handoff-2026-07-11.mdeval/recall/README.mdeval/recall/fixtures/sanitized.jsoneval/recall/score.tspackage.jsonplugin/plugin.jsonplugin/scripts/notification.mjsplugin/scripts/post-commit.mjsplugin/scripts/post-tool-failure.mjsplugin/scripts/post-tool-use.mjsplugin/scripts/pre-compact.mjsplugin/scripts/pre-tool-use.mjsplugin/scripts/prompt-submit.mjsplugin/scripts/session-end.mjsplugin/scripts/session-start.mjsplugin/scripts/stop.mjsplugin/scripts/subagent-start.mjsplugin/scripts/subagent-stop.mjsplugin/scripts/task-completed.mjsplugin/skills/agentmemory-config/REFERENCE.mdplugin/skills/agentmemory-mcp-tools/REFERENCE.mdplugin/skills/agentmemory-rest-api/REFERENCE.mdplugin/skills/recall-debug/SKILL.mdplugin/skills/why-memory/SKILL.mdscripts/build-runtime-assets.mjssrc/cli/connect/index.tssrc/config.tssrc/eval/schemas.tssrc/functions/context.tssrc/functions/durable-candidate-utils.tssrc/functions/durable-candidates.tssrc/functions/enrich.tssrc/functions/image-quota-cleanup.tssrc/functions/observe.tssrc/functions/remember.tssrc/functions/replay.tssrc/functions/slots.tssrc/functions/summarize.tssrc/hooks/pre-compact.tssrc/hooks/pre-tool-use.tssrc/hooks/prompt-submit.tssrc/index.tssrc/mcp/server.tssrc/mcp/tools-registry.tssrc/prompts/summary.tssrc/recall/core.tssrc/recall/identity.tssrc/recall/ledger.tssrc/recall/scope.tssrc/recall/tokens.tssrc/recall/trace-store.tssrc/recall/vector-health.tssrc/replay/jsonl-parser.tssrc/state/hybrid-search.tssrc/state/memory-utils.tssrc/state/schema.tssrc/triggers/api.tssrc/types.tssrc/viewer/index.htmltest/durable-candidate-utils.test.tstest/durable-candidates.test.tstest/recall-benchmark-score.test.tstest/recall-core.test.tstest/remember-project-scope.test.tstest/replay-import-key.test.tstest/replay.test.tstest/summarize.test.tstest/vector-retrieval-health.test.ts
| | Custom plugin systems | `iii worker add <name>` | | ||
|
|
||
| **174 source files · ~37,800 LOC · 1,423+ tests · 258 functions · 44 KV scopes** — all on three primitives. No `agentmemory plugin install`. The plugin system is iii itself. | ||
| **184 source files · ~42,400 LOC · 1,500+ tests · 296 functions · 58 KV scopes** — all on three primitives. No `agentmemory plugin install`. The plugin system is iii itself. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test count inconsistency between project stats and development section.
Line 1266 states "1,500+ tests" but line 1624 (development section) still says "1,423+ tests". These should be reconciled to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 1266, Reconcile the test-count statistic in the README by
updating the development section’s outdated “1,423+ tests” value to match the
current “1,500+ tests” project statistic. Keep the surrounding development
guidance unchanged.
| <h2 id="api"><picture><source media="(prefers-color-scheme: dark)" srcset="assets/tags/light/section-api.svg"><img src="assets/tags/section-api.svg" alt="API" height="32" /></picture></h2> | ||
|
|
||
| 128 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers. | ||
| 136 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Endpoint count mismatch: README says 136 but auto-generated reference says 125.
The auto-generated plugin/skills/agentmemory-rest-api/REFERENCE.md (line 8) lists exactly 125 registered endpoints, generated from src/triggers/api.ts. However, this README line claims 136 endpoints. The coding guideline for src/triggers/api.ts requires endpoint counts in README.md to be updated when adding REST endpoints. The auto-generated reference is the source of truth — update this line to match the actual registered count.
🔧 Proposed fix
-136 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers.
+125 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 136 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers. | |
| 125 endpoints on port `3111`. The REST API binds to `127.0.0.1` by default. Protected endpoints require `Authorization: Bearer <secret>` when `AGENTMEMORY_SECRET` is set, and mesh sync endpoints require `AGENTMEMORY_SECRET` on both peers. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 1590, Update the endpoint count in the README REST API
description from 136 to 125, leaving the surrounding port, binding, and
authorization details unchanged.
Source: Coding guidelines
| maxSessionSummaries: positiveConfigInt( | ||
| envOrToml(env, "AGENTMEMORY_RECALL_MAX_SESSION_SUMMARIES", toml, "recall_budget", "max_session_summaries"), | ||
| budget.maxSessionSummaries, | ||
| "recall_budget.maxSessionSummaries", | ||
| ), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Error label uses camelCase and won't match the TOML key.
The validated key is max_session_summaries, but the error label is recall_budget.maxSessionSummaries. A validation failure would point users at a key that doesn't exist in their config.toml.
✏️ Fix label
maxSessionSummaries: positiveConfigInt(
envOrToml(env, "AGENTMEMORY_RECALL_MAX_SESSION_SUMMARIES", toml, "recall_budget", "max_session_summaries"),
budget.maxSessionSummaries,
- "recall_budget.maxSessionSummaries",
+ "recall_budget.max_session_summaries",
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| maxSessionSummaries: positiveConfigInt( | |
| envOrToml(env, "AGENTMEMORY_RECALL_MAX_SESSION_SUMMARIES", toml, "recall_budget", "max_session_summaries"), | |
| budget.maxSessionSummaries, | |
| "recall_budget.maxSessionSummaries", | |
| ), | |
| maxSessionSummaries: positiveConfigInt( | |
| envOrToml(env, "AGENTMEMORY_RECALL_MAX_SESSION_SUMMARIES", toml, "recall_budget", "max_session_summaries"), | |
| budget.maxSessionSummaries, | |
| "recall_budget.max_session_summaries", | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config.ts` around lines 183 - 187, Update the error-label argument in the
maxSessionSummaries configuration within the positiveConfigInt call to use the
TOML key format, recall_budget.max_session_summaries, matching envOrToml and the
actual config.toml key.
| sdk.registerFunction("mem::context", | ||
| async (data: { sessionId: string; project: string; budget?: number }) => { | ||
| async (data: { | ||
| sessionId: string; | ||
| project: string; | ||
| budget?: number; | ||
| query?: string; | ||
| projectId?: string; | ||
| repoId?: string; | ||
| checkoutId?: string; | ||
| outputMode?: "bootstrap" | "prompt_injection" | "rendered_context" | "ranked_results"; | ||
| entryPoint?: "context" | "search" | "smart_search" | "memory_recall" | "prompt" | "session_start" | "enrich"; | ||
| debug?: boolean; | ||
| }) => { | ||
| if (recallCore) { | ||
| const result = await recallCore.recall({ | ||
| entryPoint: data.entryPoint || "context", | ||
| outputMode: data.outputMode || "rendered_context", | ||
| query: data.query, | ||
| sessionId: data.sessionId, | ||
| projectId: data.projectId || data.project, | ||
| repoId: data.repoId, | ||
| checkoutId: data.checkoutId, | ||
| ...(data.budget ? { budget: { maxContextTokens: data.budget } } : {}), | ||
| debug: data.debug, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== context.ts ==\n'
sed -n '1,140p' src/functions/context.ts
printf '\n== server.ts matches ==\n'
rg -n "mem::context|limit" src/mcp/server.ts src/functions/context.ts src -g '!**/dist/**' -g '!**/build/**'
printf '\n== recall core matches ==\n'
rg -n "rankedLimit|request\.limit|limit" src -g '!**/dist/**' -g '!**/build/**'Repository: rohitg00/agentmemory
Length of output: 46650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/mcp/server.ts around smart_search ==\n'
sed -n '280,320p' src/mcp/server.ts
printf '\n== src/recall/core.ts around ranked limit ==\n'
sed -n '190,360p' src/recall/core.ts
printf '\n== other mem::context callers ==\n'
rg -n "function_id: \"mem::context\"|function_id: 'mem::context'|registerFunction\\(\"mem::context\"" src -g '!**/dist/**' -g '!**/build/**'Repository: rohitg00/agentmemory
Length of output: 10420
Forward limit into recallCore.recall
limit is dropped before the recall call, so ranked_results falls back to 20 even when memory_smart_search passes a higher value. Add limit?: number to the input and forward it into recallCore.recall.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/context.ts` around lines 40 - 64, Add an optional limit?:
number field to the mem::context input type and forward data.limit in the
recallCore.recall request. Update only the registration callback’s input
definition and recall payload, preserving existing defaults and behavior when
limit is omitted.
| const parsed = parseJsonlText(text); | ||
|
|
||
| const fileHash = hashText(text); | ||
| const idempotencyKey = fingerprintId( | ||
| "arch", | ||
| `${parsed.sessionId}\n${fileHash}`, | ||
| ); | ||
| const existingImport = await kv.get<ArchiveImportRecord>( | ||
| KV.archiveImports, | ||
| idempotencyKey, | ||
| ); | ||
| if ( | ||
| existingImport && | ||
| (existingImport.status === "completed" || existingImport.summaryCreated === true) | ||
| ) { | ||
| skipped.push({ | ||
| archivePath: file, | ||
| sessionId: parsed.sessionId, | ||
| reason: "already_completed", | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| const startedAt = new Date().toISOString(); | ||
| let record: ArchiveImportRecord = { | ||
| ...existingImport, | ||
| id: idempotencyKey, | ||
| archivePath: file, | ||
| fileHash, | ||
| sessionId: parsed.sessionId, | ||
| status: existingImport?.status || "discovered", | ||
| createdAt: existingImport?.createdAt || startedAt, | ||
| updatedAt: startedAt, | ||
| attempts: (existingImport?.attempts || 0) + 1, | ||
| parsedObservationCount: parsed.observations.length, | ||
| importedObservationCount: | ||
| existingImport?.importedObservationCount || 0, | ||
| source: "archive-process", | ||
| }; | ||
| await kv.set(KV.archiveImports, idempotencyKey, record); | ||
|
|
||
| const observationsAlreadyImported = | ||
| record.status === "observations_imported" || | ||
| record.status === "summarizing" || | ||
| (record.status === "failed" && record.failureStage === "summary"); | ||
| if (!observationsAlreadyImported) { | ||
| record = { | ||
| ...record, | ||
| status: "importing_observations", | ||
| failureStage: undefined, | ||
| lastError: undefined, | ||
| updatedAt: new Date().toISOString(), | ||
| }; | ||
| await kv.set(KV.archiveImports, idempotencyKey, record); | ||
|
|
||
| const importResult = (await sdk.trigger({ | ||
| function_id: "mem::replay::import-jsonl", | ||
| payload: { path: file, maxFiles: 1 }, | ||
| })) as { success?: boolean; error?: string }; | ||
| if (!importResult?.success) { | ||
| record = { | ||
| ...record, | ||
| status: "failed", | ||
| failureStage: "observations", | ||
| lastError: importResult?.error || "import_failed", | ||
| updatedAt: new Date().toISOString(), | ||
| }; | ||
| await kv.set(KV.archiveImports, idempotencyKey, record); | ||
| skipped.push({ | ||
| archivePath: file, | ||
| sessionId: parsed.sessionId, | ||
| reason: record.lastError || "import_failed", | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| const session = await kv.get<Session>(KV.sessions, parsed.sessionId); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the fallback used by import-jsonl and whether it accepts an explicit sessionId.
rg -nP 'parseJsonlText\s*\(' src/functions/replay.ts src/functions/durable-candidates.ts
ast-grep run --pattern 'parseJsonlText($$$)' --lang typescript srcRepository: rohitg00/agentmemory
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant functions and call sites.
ast-grep outline src/functions/replay.ts --view expanded
ast-grep outline src/functions/durable-candidates.ts --view expanded
# Read the parser implementation and the import trigger path around the key lines.
grep -n "function parseJsonlText\|const parseJsonlText\|export .*parseJsonlText" -n src/functions/*.ts
sed -n '1,220p' src/functions/replay.ts
sed -n '650,780p' src/functions/durable-candidates.tsRepository: rohitg00/agentmemory
Length of output: 1499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the specific sections that handle JSONL parsing and session creation.
sed -n '330,430p' src/functions/replay.ts
sed -n '680,770p' src/functions/durable-candidates.ts
# Also inspect the parser helper if it lives in a shared file.
rg -n "parseJsonlText|generateId\\(\"sess\"\\)|fingerprintId\\(\"sess\"" src -SRepository: rohitg00/agentmemory
Length of output: 7892
Share one session ID across the archive import flow
archive::process fingerprints parsed.sessionId, but mem::replay::import-jsonl re-parses the same file with a generated fallback, so archives without embedded session metadata can be stored under a different session key and the later kv.get(KV.sessions, parsed.sessionId) misses. Pass the same session id through the trigger payload or reuse the imported session id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/functions/durable-candidates.ts` around lines 683 - 759, Update the
archive import flow in archive processing and the mem::replay::import-jsonl
trigger to use one session ID: pass parsed.sessionId in the trigger payload and
make the importer use that supplied ID instead of generating a fallback, so the
subsequent kv.get(KV.sessions, parsed.sessionId) resolves the imported session.
| function gitValue(cwd: string, args: string[]): string | undefined { | ||
| try { | ||
| const value = execFileSync("git", args, { | ||
| cwd, | ||
| stdio: ["ignore", "pipe", "ignore"], | ||
| timeout: 500, | ||
| }).toString().trim(); | ||
| return value || undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| export function resolveRecallIdentity( | ||
| cwd: string, | ||
| projectId: string, | ||
| ): RecallIdentity { | ||
| const checkoutRoot = gitValue(cwd, ["rev-parse", "--show-toplevel"]) || cwd; | ||
| const normalizedRoot = resolve(checkoutRoot).replace(/\\/g, "/").toLowerCase(); | ||
| const remote = gitValue(checkoutRoot, ["remote", "get-url", "origin"]); | ||
| return { | ||
| projectId, | ||
| repoId: remote ? fingerprint(normalizeRemote(remote)) : fingerprint(normalizedRoot), | ||
| checkoutId: fingerprint(normalizedRoot), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'function isSensitive' -A 15 src/functions/replay.ts
rg -n 'resolveRecallIdentity\(' -B3 -A3 src/triggers/api.tsRepository: rohitg00/agentmemory
Length of output: 1779
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/recall/identity.ts ---'
cat -n src/recall/identity.ts | sed -n '1,220p'
echo '--- relevant api trigger context ---'
sed -n '360,390p' src/triggers/api.ts
echo
sed -n '730,755p' src/triggers/api.ts
echo
sed -n '1140,1165p' src/triggers/api.ts
echo '--- search for path guards / cwd validation in recall paths ---'
rg -n 'isSensitive\(|sensitive path|cwd|resolveRecallIdentity\(|gitValue\(' src -g '!**/*.map'Repository: rohitg00/agentmemory
Length of output: 17700
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/recall/identity.ts outline ---'
ast-grep outline src/recall/identity.ts --view expanded
echo '--- related recall path helpers ---'
rg -n 'hybridSearch|kv\.list|persistRecallTrace|RecallCore|resolveRecallIdentity' src/recall src -g '!**/*.map'Repository: rohitg00/agentmemory
Length of output: 31324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/recall/identity.ts')
print(p.exists(), p.stat().st_size if p.exists() else None)
PYRepository: rohitg00/agentmemory
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- top of src/triggers/api.ts ---'
sed -n '1,140p' src/triggers/api.ts
echo '--- resolveRecallIdentity implementation ---'
sed -n '1,120p' src/recall/identity.ts
echo '--- any auth / request provenance in api trigger ---'
rg -n 'auth|token|apikey|api key|request body|express|fastify|router|endpoint' src/triggers/api.ts src/triggers -g '!**/*.map'Repository: rohitg00/agentmemory
Length of output: 38716
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/triggers/api.tsRepository: rohitg00/agentmemory
Length of output: 7977
Avoid sync git probes on request-supplied cwd.
resolveRecallIdentity() is called from API handlers with cwd straight from the request body, and each call does up to two synchronous execFileSync("git", ...) calls with 500ms timeouts. That can block the worker on the hot request path and lets callers point git at arbitrary host paths. Restrict cwd to an allowed workspace root, or move the git lookup off the request path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/recall/identity.ts` around lines 25 - 50, Update resolveRecallIdentity
and its gitValue usage so request-supplied cwd is validated against an allowed
workspace root before any git probe, rejecting or falling back safely for paths
outside that root. Preserve identity resolution for valid workspaces, and ensure
invalid input cannot trigger synchronous execFileSync calls on arbitrary host
paths.
| await Promise.all(trace.selected.map(async (item) => { | ||
| const current = (await kv.get<RecallItemStats>(KV.recallStats, item.id)) || emptyStats(item.id); | ||
| const count = current.recallCount + 1; | ||
| await kv.set(KV.recallStats, item.id, { | ||
| ...current, | ||
| recallCount: count, | ||
| averageScore: ((current.averageScore * current.recallCount) + item.score) / count, | ||
| lastRecalledAt: trace.timestamp, | ||
| recentQuery: trace.query, | ||
| }); | ||
| })); | ||
| const mismatches = trace.dropped.filter((item) => item.decision === "scope_mismatch"); | ||
| await Promise.all(mismatches.map(async (item) => { | ||
| const current = (await kv.get<RecallItemStats>(KV.recallStats, item.id)) || emptyStats(item.id); | ||
| await kv.set(KV.recallStats, item.id, { | ||
| ...current, | ||
| scopeMismatchCount: current.scopeMismatchCount + 1, | ||
| }); | ||
| })); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== trace-store outline ==\n'
ast-grep outline src/recall/trace-store.ts --view expanded || true
printf '\n== kv API references ==\n'
rg -n "class .*kv|interface .*kv|type .*kv|kv\.update|kv\.get<|kv\.set<|update\(" src --glob '!**/dist/**' --glob '!**/build/**' || true
printf '\n== recall trace-store full file (limited) ==\n'
wc -l src/recall/trace-store.ts
sed -n '1,220p' src/recall/trace-store.ts
printf '\n== kv-related files ==\n'
fd -a ".*(kv|store).*\.ts$" src . || trueRepository: rohitg00/agentmemory
Length of output: 21656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for KV implementation and atomic ops ==\n'
rg -n "export (class|function)|kv\.|update\(|atomic|mutex|lock|transaction|compareAndSwap|cas" src -g '*.ts' || true
printf '\n== search for RecallItemStats and emptyStats ==\n'
rg -n "RecallItemStats|emptyStats|recallCount|scopeMismatchCount|averageScore|lastRecalledAt|recentQuery" src -g '*.ts' || trueRepository: rohitg00/agentmemory
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== state/kv.ts ==\n'
wc -l src/state/kv.ts
sed -n '1,240p' src/state/kv.ts
printf '\n== in-memory kv ==\n'
wc -l src/mcp/in-memory-kv.ts
sed -n '1,260p' src/mcp/in-memory-kv.ts
printf '\n== trace-store callers / locks ==\n'
rg -n "persistRecallTrace\(" src -g '*.ts'
rg -n "withKeyedLock\(|recallTraces|recallStats" src -g '*.ts'Repository: rohitg00/agentmemory
Length of output: 9874
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== keyed mutex ==\n'
cat -n src/state/keyed-mutex.ts
printf '\n== relevant recall API caller paths ==\n'
sed -n '1,220p' src/functions/smart-search.ts
sed -n '1,220p' src/recall/core.tsRepository: rohitg00/agentmemory
Length of output: 18240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== state::update implementation ==\n'
rg -n "function_id: \"state::update\"|state::update|state::get|state::set|state::list|state::delete" src -g '*.ts'
printf '\n== trace-store surrounding caller in RecallCore ==\n'
sed -n '240,320p' src/recall/core.ts
printf '\n== API endpoints around recall stats/traces ==\n'
sed -n '2340,2385p' src/triggers/api.tsRepository: rohitg00/agentmemory
Length of output: 6243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for state trigger registrations ==\n'
rg -n "register.*state|state::" src -g '*.ts'
printf '\n== state schema and any comments about atomicity ==\n'
sed -n '1,180p' src/state/schema.tsRepository: rohitg00/agentmemory
Length of output: 5568
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for state:: handlers in repo ==\n'
rg -n "\"state::(get|set|update|delete|list)\"|state::(get|set|update|delete|list)" src -g '*.ts'
printf '\n== keyed lock used with shared KV stats? ==\n'
rg -n "recallStats|recallTraces|persistRecallTrace|withKeyedLock" src -g '*.ts'Repository: rohitg00/agentmemory
Length of output: 9147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '260,310p' src/recall/core.ts
sed -n '1,120p' src/recall/ledger.tsRepository: rohitg00/agentmemory
Length of output: 5079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== kv.update usage patterns ==\n'
rg -n "kv\.update\(" src -g '*.ts' -A4 -B2
printf '\n== any docs/comments for update op types ==\n'
rg -n "path:|type: \"(inc|increment|set|unset|push|splice)\"|ops" src -g '*.ts'Repository: rohitg00/agentmemory
Length of output: 24225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "timestamp:" src/recall/core.ts src/types.ts src -g '*.ts' | sed -n '1,120p'
sed -n '300,420p' src/recall/core.tsRepository: rohitg00/agentmemory
Length of output: 9024
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '360,470p' src/recall/core.tsRepository: rohitg00/agentmemory
Length of output: 2823
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== trace construction in RecallCore ==\n'
rg -n "trace =|timestamp:|persistRecallTrace\(" src/recall/core.ts -n -A40 -B20
printf '\n== state::update handler / implementation ==\n'
rg -n "state::update|function_id: 'state::update'|function_id: \"state::update\"" src -g '*.ts' -A30 -B10
printf '\n== any README/docs for state update semantics ==\n'
rg -n "state::update|atomic update|compare-and-swap|increment" README.md docs src -g '*.md' -g '*.ts' -A4 -B2Repository: rohitg00/agentmemory
Length of output: 17565
🌐 Web query:
iii-sdk state::update atomic increment semantics StateKV get set update documentation
💡 Result:
The iii-sdk provides atomic state management through the state::update function [1][2]. This function is designed to prevent race conditions inherent in read-modify-write patterns by allowing multiple operations to be applied to a stored JSON value atomically [1][3][4]. Atomic Increment Semantics The increment operation is part of a set of atomic operations (alongside set, merge, append, decrement, and remove) executed in order on a scoped key-value pair [5][3][6]. When using the increment operation: - It targets a numeric field within a stored JSON object [5]. - You specify the path to the field and the value (by) to add [5]. - If the target field does not exist or is not a number, the operation will fail with specific errors (e.g., increment.not_number or increment.target_not_object) [5]. It is recommended to initialize the field (e.g., using set to 0) if it does not yet exist [5]. - Multiple operations can be sent in a single state::update call, and they are processed sequentially as a single atomic unit [1][4]. Usage The state::update function accepts a payload containing the scope, key, and an array of operations [1][3][4]. Example (TypeScript): await iii.trigger({ function_id: 'state::update', payload: { scope: 'users', key: 'user_123', ops: [ { type: 'increment', path: 'loginCount', by: 1 } ], }, }); Recommendations - Use state::update for partial mutations to ensure atomicity [3][6]. - Avoid read-then-set patterns (get followed by set) for modifications, as these are not atomic and may result in lost updates during concurrent execution [3][6][7]. - The update operation returns the state of the record after the operations have been applied [3][4]. Documentation Note: While early versions (e.g., 0.10.0) suggested using read-modify-write for some state interactions [7], modern iii-sdk versions (0.11.0+) explicitly prioritize the state::update function for atomic modifications [5][3][6].
Citations:
- 1: https://iii.dev/docs/0-10-0/how-to/manage-state
- 2: https://iii.dev/docs/how-to/use-functions-and-triggers
- 3: https://github.com/iii-hq/iii/blob/d51a06dabf41d8121d99e622dc351f1e2ebb7810/skills/references/state-management.py
- 4: https://iii-hq-iii.mintlify.app/modules/state
- 5: https://iii.dev/docs/0-11-0/workers/iii-state
- 6: https://motiadev.mintlify.app/0-11-0/examples/state-management
- 7: https://iii.dev/docs/0-10-0/examples/state-management
Use atomic state updates for recall stats
These counters still do get → recompute → set, so concurrent recalls can drop recallCount, averageScore, or scopeMismatchCount updates. Switch this to state::update with atomic increment ops.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/recall/trace-store.ts` around lines 62 - 80, The recall-stat updates in
the selected and scope-mismatch processing still use non-atomic
get/recompute/set logic. Replace both update paths in the trace handling flow
with state::update and atomic increment operations for recallCount and
scopeMismatchCount, while preserving the averageScore and metadata calculations
through the update mechanism so concurrent recalls cannot overwrite each other.
| observations: (sessionId: string) => `mem:obs:${sessionId}`, | ||
| memories: "mem:memories", | ||
| summaries: "mem:summaries", | ||
| archiveImports: "mem:archive-imports", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'interface RecallItemStats|interface ArchiveImport|interface DurableRecommendation|interface InjectionLedgerEntry' src/types.ts src/recall/ledger.tsRepository: rohitg00/agentmemory
Length of output: 330
🏁 Script executed:
#!/bin/bash
sed -n '1,120p' src/state/schema.ts
printf '\n---\n'
sed -n '150,220p' src/types.ts
printf '\n---\n'
sed -n '360,410p' src/types.ts
printf '\n---\n'
sed -n '1,80p' src/recall/ledger.tsRepository: rohitg00/agentmemory
Length of output: 9482
🏁 Script executed:
#!/bin/bash
rg -n 'DurableRecommendation|durableRecommendations|ArchiveImportRecord|RecallTrace|RecallItemStats|InjectionLedgerEntry' src/types.ts src/recall src/stateRepository: rohitg00/agentmemory
Length of output: 1976
Add the missing shared KV types
ArchiveImportRecord, RecallTrace, and RecallItemStats already exist in src/types.ts, but InjectionLedgerEntry is only defined in src/recall/ledger.ts, and durableRecommendations still has no shared type there. Move those shapes into src/types.ts so the new KV scopes follow the schema/types convention.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/state/schema.ts` at line 8, Update the shared type definitions in
types.ts by moving or adding InjectionLedgerEntry from recall/ledger.ts and
defining the durableRecommendations KV record shape there, alongside
ArchiveImportRecord, RecallTrace, and RecallItemStats. Then update consumers
such as the ledger and KV schema usage to import these shared types, preserving
the existing fields and behavior.
Source: Coding guidelines
| ...(typeof req.body.repoId === "string" ? { repoId: req.body.repoId } : identity?.repoId ? { repoId: identity.repoId } : {}), | ||
| ...(typeof req.body.checkoutId === "string" ? { checkoutId: req.body.checkoutId } : identity ? { checkoutId: identity.checkoutId } : {}), | ||
| ...(scope !== undefined && { scope }), | ||
| ...(req.body.origin !== undefined && { origin: req.body.origin }), | ||
| ...(req.body.checkoutId !== undefined && { checkoutId: req.body.checkoutId }), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Duplicate checkoutId key in the remember payload.
checkoutId is set twice in this object spread: Line 1251 (body value or identity.checkoutId fallback) and again Line 1254 (req.body.checkoutId). The later spread wins, so when a caller omits checkoutId but supplies cwd, the identity-derived fallback on Line 1251 is still preserved (Line 1254 spreads nothing) — but the redundant key is confusing and looks like an edit artifact. Drop Line 1254.
Proposed cleanup
...(scope !== undefined && { scope }),
...(req.body.origin !== undefined && { origin: req.body.origin }),
- ...(req.body.checkoutId !== undefined && { checkoutId: req.body.checkoutId }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...(typeof req.body.repoId === "string" ? { repoId: req.body.repoId } : identity?.repoId ? { repoId: identity.repoId } : {}), | |
| ...(typeof req.body.checkoutId === "string" ? { checkoutId: req.body.checkoutId } : identity ? { checkoutId: identity.checkoutId } : {}), | |
| ...(scope !== undefined && { scope }), | |
| ...(req.body.origin !== undefined && { origin: req.body.origin }), | |
| ...(req.body.checkoutId !== undefined && { checkoutId: req.body.checkoutId }), | |
| ...(typeof req.body.repoId === "string" ? { repoId: req.body.repoId } : identity?.repoId ? { repoId: identity.repoId } : {}), | |
| ...(typeof req.body.checkoutId === "string" ? { checkoutId: req.body.checkoutId } : identity ? { checkoutId: identity.checkoutId } : {}), | |
| ...(scope !== undefined && { scope }), | |
| ...(req.body.origin !== undefined && { origin: req.body.origin }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/triggers/api.ts` around lines 1250 - 1254, Remove the redundant
checkoutId spread from the remember payload construction near the existing
repoId and checkoutId selection logic. Keep the earlier checkoutId expression
that uses req.body.checkoutId or identity.checkoutId, and leave the other
payload fields unchanged.
| var rows = traces.map(function(trace) { | ||
| var mode = trace.retrievalMode || {}; | ||
| var vector = mode.vector || {}; | ||
| var selected = (trace.selected || []).length; | ||
| var dropped = trace.droppedCountsByDecision || {}; | ||
| return '<tr>' + | ||
| '<td>' + esc(trace.timestamp || '') + '</td>' + | ||
| '<td>' + esc(trace.entryPoint || '') + ' / ' + esc(trace.outputMode || '') + '</td>' + | ||
| '<td>' + esc(trace.query || '[no query]') + '</td>' + | ||
| '<td>' + selected + ' selected, ' + Object.keys(dropped).map(function(k) { return esc(k) + ': ' + dropped[k]; }).join(', ') + '</td>' + | ||
| '<td>' + (trace.finalContextTokenCount || 0) + ' tokens</td>' + | ||
| '<td>' + esc(vector.status || 'disabled') + (vector.reason ? ': ' + esc(vector.reason) : '') + '</td>' + | ||
| '</tr>'; | ||
| }).join(''); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Unescaped dropped[k] values in innerHTML.
The droppedCountsByDecision object keys are escaped via esc(k), but the values dropped[k] are concatenated directly into the HTML string without escaping. If the API ever returns string values (or an attacker can influence the response), this is an XSS vector. Coerce and escape the value.
🛡️ Proposed fix
- '<td>' + selected + ' selected, ' + Object.keys(dropped).map(function(k) { return esc(k) + ': ' + dropped[k]; }).join(', ') + '</td>' +
+ '<td>' + selected + ' selected, ' + Object.keys(dropped).map(function(k) { return esc(k) + ': ' + esc(String(dropped[k])); }).join(', ') + '</td>' +📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var rows = traces.map(function(trace) { | |
| var mode = trace.retrievalMode || {}; | |
| var vector = mode.vector || {}; | |
| var selected = (trace.selected || []).length; | |
| var dropped = trace.droppedCountsByDecision || {}; | |
| return '<tr>' + | |
| '<td>' + esc(trace.timestamp || '') + '</td>' + | |
| '<td>' + esc(trace.entryPoint || '') + ' / ' + esc(trace.outputMode || '') + '</td>' + | |
| '<td>' + esc(trace.query || '[no query]') + '</td>' + | |
| '<td>' + selected + ' selected, ' + Object.keys(dropped).map(function(k) { return esc(k) + ': ' + dropped[k]; }).join(', ') + '</td>' + | |
| '<td>' + (trace.finalContextTokenCount || 0) + ' tokens</td>' + | |
| '<td>' + esc(vector.status || 'disabled') + (vector.reason ? ': ' + esc(vector.reason) : '') + '</td>' + | |
| '</tr>'; | |
| }).join(''); | |
| var rows = traces.map(function(trace) { | |
| var mode = trace.retrievalMode || {}; | |
| var vector = mode.vector || {}; | |
| var selected = (trace.selected || []).length; | |
| var dropped = trace.droppedCountsByDecision || {}; | |
| return '<tr>' + | |
| '<td>' + esc(trace.timestamp || '') + '</td>' + | |
| '<td>' + esc(trace.entryPoint || '') + ' / ' + esc(trace.outputMode || '') + '</td>' + | |
| '<td>' + esc(trace.query || '[no query]') + '</td>' + | |
| '<td>' + selected + ' selected, ' + Object.keys(dropped).map(function(k) { return esc(k) + ': ' + esc(String(dropped[k])); }).join(', ') + '</td>' + | |
| '<td>' + (trace.finalContextTokenCount || 0) + ' tokens</td>' + | |
| '<td>' + esc(vector.status || 'disabled') + (vector.reason ? ': ' + esc(vector.reason) : '') + '</td>' + | |
| '</tr>'; | |
| }).join(''); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 2437-2437: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: el.innerHTML = '
Recall Traces
' + rows + '| Time | Entry | Query | Decision | Budget | Vector |
|---|
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/viewer/index.html` around lines 2424 - 2437, Update the
droppedCountsByDecision rendering inside the traces.map callback to coerce each
dropped[k] value to a string and pass it through esc before concatenating it
into the innerHTML string, while preserving the existing escaped-key formatting.
1 participant
What
Two related memory capabilities built on the same infrastructure:
P1: Durable Candidates
POST /agentmemory/archive/process)POST /agentmemory/durable-candidates/promote)POST /agentmemory/durable-candidates/backfill)P2: Scoped Traceable Recall
src/recall/core.ts) — all search paths route through it instead of calling hybridSearch/kv.list directlyRecallTracewith selected/dropped items and structured reasonsGET /agentmemory/recall/debugand/recall/debug/{traceId})recall-debugandwhy-memoryDocs sync
npm run skills:genVerification
Known limitations (P1)
The archive corpus alignment is not yet resolved — see
docs/p1-durable-candidates-handoff-2026-07-11.md. The29 sessions / 965 observationsbaseline has not been reproduced against the current archive snapshot. Real backfill/promote should not be run until the gap is explained.Files changed
src/recall/(7 files),src/functions/durable-candidates.ts,src/functions/durable-candidate-utils.tssrc/config.ts,src/types.ts,src/triggers/api.ts,src/mcp/server.ts,src/mcp/tools-registry.ts,src/index.tsrecall-core.test.ts,durable-candidates.test.ts,durable-candidate-utils.test.ts,vector-retrieval-health.test.ts,recall-benchmark-score.test.ts,replay-import-key.test.tsSummary by CodeRabbit
New Features
Documentation
Bug Fixes