feat: durable candidates MVP + scoped traceable recall + Codex archive watcher#1055
feat: durable candidates MVP + scoped traceable recall + Codex archive watcher#1055MarcoLissitzky wants to merge 18 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>
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>
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>
Signed-off-by: Jenq Shiin Haw <1343528669@qq.com>
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. |
📝 WalkthroughWalkthroughThis change adds durable-candidate archive workflows, scoped traceable recall, recall configuration, archive watching, deterministic replay handling, new API/MCP/hook integrations, runtime build metadata, viewer trace inspection, documentation, and extensive tests. ChangesDurable candidates and archive ingestion
Scoped recall and observability
Replay, build, and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/functions/observe.ts (1)
183-194: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winImage reference count leaks in the error rollback path.
incrementImageRef(kv, filePath)is called at line 158, but the catch block at lines 182-192 callsdeleteImagewithout a correspondingdecrementImageRef. This leaves a stale entry inKV.imageRefswith count > 0 for a file that no longer exists.🛡️ Proposed fix: add
decrementImageRefto the rollback path} catch (error) { if (raw.imageData) { + const { decrementImageRef } = await import("./image-refs.js"); + await decrementImageRef(kv, raw.imageData); const { deleteImage } = await import("../utils/image-store.js"); const { deletedBytes } = await deleteImage(raw.imageData); if (deletedBytes > 0) { sdk.trigger({ function_id: "mem::disk-size-delta", payload: { deltaBytes: -deletedBytes }, action: TriggerAction.Void(), }); } } throw error; }🤖 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/observe.ts` around lines 183 - 194, Update the error rollback path in the observe handler to call decrementImageRef for the same file path after incrementImageRef has been performed, before or alongside deleteImage. Preserve the existing deletedBytes disk-size trigger and rethrow behavior.INSTALL_FOR_AGENTS.md (1)
103-113: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix duplicate step 7 numbering.
Inserting the new step 6 shifted "Install native skills" to step 7 (line 103), but the existing "Verify a save and recall round-trip" section (line 113) was not renumbered to step 8. Both sections are now numbered "7".
📝 Proposed fix
-## 7. Verify a save and recall round-trip +## 8. Verify a save and recall round-trip🤖 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 `@INSTALL_FOR_AGENTS.md` around lines 103 - 113, Renumber the “Verify a save and recall round-trip” section heading following “Install native skills” from step 7 to step 8, leaving the installation section and its content unchanged.plugin/scripts/pre-tool-use.mjs (1)
62-74: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThread
cwdinto enrich identity resolutioncwdis posted here, butapi::enrichdrops it and only forwardssessionId,files,terms,toolName, andprojecttomem::enrich. That means enrich calls never get repo/checkout scoping from the worktree path; resolverepoId/checkoutIdfromcwdhere or remove the field.🤖 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 `@plugin/scripts/pre-tool-use.mjs` around lines 62 - 74, The enrich request currently sends cwd, but the downstream api::enrich flow does not use it for identity resolution. Update the api::enrich-to-mem::enrich forwarding path to resolve and pass repoId and checkoutId from cwd, preserving project and the existing enrich fields; alternatively remove cwd from this request if identity scoping cannot be supported.
🧹 Nitpick comments (12)
src/state/hybrid-search.ts (1)
128-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGraph mode reporting can be inconsistent on partial failures.
If
searchByEntitiesfails butexpandFromChunkssucceeds (or vice versa),graphModeis overwritten todisabledeven thoughgraphResultscontains entries from the successful call. This misleads trace/debug output. Consider only marking graph as disabled when no graph results were obtained.♻️ Suggested fix: only disable graph mode when no results exist
} catch (err) { - graphMode = { - status: "disabled", - attempted: true, - reason: err instanceof Error ? err.message : "graph retrieval failed", - fallback: "BM25/vector", - }; + if (graphResults.length === 0) { + graphMode = { + status: "disabled", + attempted: true, + reason: err instanceof Error ? err.message : "graph retrieval failed", + fallback: "BM25/vector", + }; + } } } const topVectorObs = vectorResults.slice(0, 5).map((r) => r.obsId); if (topVectorObs.length > 0) { try { const expansionResults = await this.graphRetrieval.expandFromChunks(topVectorObs, 1, 5); graphResults = [...graphResults, ...expansionResults]; } catch (err) { - graphMode = { - status: "disabled", - attempted: true, - reason: err instanceof Error ? err.message : "graph expansion failed", - fallback: "BM25/vector", - }; + if (graphResults.length === 0) { + graphMode = { + status: "disabled", + attempted: true, + reason: err instanceof Error ? err.message : "graph expansion failed", + fallback: "BM25/vector", + }; + } } }🤖 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/hybrid-search.ts` around lines 128 - 161, Update the error handling in the graph retrieval calls within the hybrid search flow so graphMode is set to disabled only when graphResults remains empty after the attempted operations. Preserve successful results from either searchByEntities or expandFromChunks, and ensure partial failures do not overwrite a healthy/usable graph status when entries were obtained.src/triggers/api.ts (1)
1285-1289: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
checkoutIdin themem::rememberpayload spread.
checkoutIdis set twice: line 1286 (body value, else identity fallback) and again line 1289 (body value if defined). Becausereq.body.checkoutIdis already validated as a string at Line 1267, the two branches produce the same value when present, making line 1289 dead/redundant and obscuring the identity-fallback intent. Drop the second spread.Proposed cleanup
...(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 }),🤖 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 1285 - 1289, Remove the redundant checkoutId spread from the mem::remember payload construction. Keep the earlier checkoutId selection that uses the validated request-body string with identity.checkoutId as the fallback, and leave the other payload fields unchanged.src/functions/remember.ts (1)
151-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo
recordAudit()for this state-changing write.
mem::rememberpersists a new/superseding memory (and mutates the superseded row), yet unlikemem::forgetbelow it records no audit entry. Per the repo guideline to userecordAudit()for state-changing operations, consider recording an audit event on save. Please confirm whether auditing here is intentionally omitted or handled downstream (e.g., viamem::cascade-update).🤖 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 151 - 226, Add a recordAudit() call to the mem::remember save flow after persisting the new memory and updating any superseded memory, covering both the creation and supersession state changes. Use the existing audit conventions and memory identifiers, and do not rely on mem::cascade-update to audit this write.Source: Coding guidelines
test/durable-candidates.test.ts (1)
130-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: prefer
KVconstants over hardcoded scope strings.The suite mixes
KV.summaries/KV.memories/KV.durableRecommendationswith hardcoded equivalents like"mem:summaries","mem:memories", and"mem:obs:<id>". Using theKVhelpers consistently keeps the tests resilient if a scope prefix changes.Also applies to: 173-174, 206-207
🤖 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/durable-candidates.test.ts` around lines 130 - 141, Replace hardcoded memory scope strings in the durable-candidate tests, including the cases around the recommendation setup and the referenced sections, with the corresponding KV constants or helpers such as KV.summaries, KV.memories, KV.durableRecommendations, and the observation-scope helper. Keep test behavior unchanged while ensuring all key construction uses the centralized KV definitions.src/types.ts (1)
112-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm
wouldPromote: falseliteral is intentional.
DurableCandidateRecommendationfixeswouldPromoteto the literalfalseeven forrecommendation: "auto_promote_eligible". This aligns with the dry-run posture described in the PR objectives (real promotion deferred pending archive-baseline reconciliation), but the always-false literal means consumers can never branch to a promote action from this type. If a future eligible-and-would-promote state is expected, widen toboolean; otherwise this is fine as an intentional dry-run marker.🤖 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/types.ts` around lines 112 - 118, Confirm that DurableCandidateRecommendation is intentionally dry-run only: retain wouldPromote as the literal false because promotion remains deferred, and document or otherwise preserve this contract for consumers. If the type must represent future eligible actions, widen wouldPromote to boolean and update related recommendation handling accordingly.src/recall/tokens.ts (1)
3-25: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCJK range misses Hangul and astral ideographs, underestimating tokens.
CJK_CHARonly spans BMP ranges up to\u9fff/\ufaff. Korean Hangul (U+AC00–U+D7A3) and CJK Unified Ideographs Extension B+ (U+20000+, iterated aschar.length === 2viafor..of) fall into the ASCII branch and are counted at roughly1/4token each. For an estimator namedconservative-unicodethat feeds recall budget enforcement, this under-counts and can let context exceed the intended budget. Consider widening the range to include Hangul and using code-point checks for astral CJK.♻️ Suggested range extension
-const CJK_CHAR = /[\u3040-\u30ff\u3400-\u9fff\uf900-\ufaff]/u; +const CJK_CHAR = /[\u3040-\u30ff\u3400-\u9fff\uac00-\ud7a3\uf900-\ufaff\u{20000}-\u{2fa1f}]/u;🤖 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 - 25, Update CJK_CHAR and the character classification in countTokens so Korean Hangul (U+AC00–U+D7A3) and astral CJK ideographs are treated as individual CJK tokens rather than entering the asciiRun path. Use code-point-aware checks compatible with for...of iteration, while preserving the existing ASCII-run batching behavior for non-CJK text.test/viewer-recall-xss.test.ts (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEscaper extraction is tightly coupled to viewer HTML indentation.
The regex
/function esc\(s\) \{[\s\S]*?\r?\n \}/requires the closing brace to be preceded by exactly four spaces. Any reformatting ofsrc/viewer/index.html(indent change, prettier, tabs) will makeescSourceundefined and fail the test at Line 22 rather than at a meaningful assertion — a brittle coupling to layout rather than behavior. Consider anchoring on a less layout-dependent boundary or extracting the escaper into a shared module the test can import directly.The
new Function(...)OpenGrep hint is a false positive here: the source is read from a trusted in-repo file, not attacker input.🤖 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/viewer-recall-xss.test.ts` around lines 21 - 23, Make the escaper extraction in the viewer recall XSS test independent of the exact closing-brace indentation by anchoring the regex to a stable, layout-insensitive boundary, or reuse the escaper from a shared importable module. Keep the existing behavioral assertions and trusted-source handling unchanged.src/recall/ledger.ts (1)
78-95: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueInjection-ledger entries are never pruned.
recordInjectionwrites one entry per(sessionId, itemId)and nothing reaps stale keys, soKV.injectionLedgergrows monotonically as sessions and items accumulate. Consider a retention/TTL sweep (similar to the recall-trace retention config) to bound storage.🤖 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/ledger.ts` around lines 78 - 95, The recordInjection flow writes injection-ledger entries without any retention, allowing KV.injectionLedger to grow indefinitely. Add a retention or TTL cleanup mechanism for stale InjectionLedgerEntry records, reusing the existing recall-trace retention configuration and cleanup patterns, while preserving current writes for active entries.src/recall/core.ts (2)
103-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
fingerprintId()for content-addressable dedup.
normalizeDuplicateplus aSet<string>hand-rolls content-addressable deduplication. The repo convention is to usefingerprintId()for this, which keeps dedup semantics consistent with the rest of the codebase.As per coding guidelines: "Use
fingerprintId()for content-addressable deduplication andgenerateId()for unique IDs."🤖 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 103 - 105, Replace the normalizeDuplicate-based deduplication with the repository-standard fingerprintId() in the recall deduplication flow, including updating the Set key type/usage as needed. Preserve the existing duplicate-detection behavior while using fingerprintId() for content-addressable identity.Source: Coding guidelines
375-396: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize per-hit KV lookups to avoid serial N+1 latency.
Each hit triggers a sequential
await this.kv.get<Memory>(...)(and a session lookup), so recall latency scales linearly with the candidate count. Since the per-hit resolutions are independent, resolve them withPromise.all(the session cache still dedupes repeated session IDs).As per coding guidelines: "Use
Promise.allfor independent parallel operations where possible."🤖 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 375 - 396, Update the hit-processing flow around the loop in the recall method to resolve each hit’s memory and fallback session concurrently with Promise.all, while preserving the existing candidate mapping and sessionFor cache behavior. Avoid awaiting KV and session lookups sequentially inside the per-hit iteration; first resolve independent per-hit data in parallel, then build candidates with the same fields and scoring.Source: Coding guidelines
src/eval/schemas.ts (1)
106-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
DurableCandidateTypeEnuminstead of re-inlining the type list.This literal list duplicates
DurableCandidateTypeEnum(Line 35-42), whichSummaryOutputSchemaalready reuses. Keeping two copies risks drift.♻️ 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 the candidate type field in the relevant schema to reuse DurableCandidateTypeEnum instead of defining an inline z.enum list, matching SummaryOutputSchema and keeping the existing optional behavior.src/recall/trace-store.ts (1)
98-110: 🚀 Performance & Scalability | 🔵 TrivialTrace pruning scans the full trace set on every write.
persistRecallTracecallskv.listover all retained traces and re-sorts/prunes on each write. As recall volume grows this becomes an O(n) read + delete on the hot recall path. Consider pruning periodically (e.g., time-bucketed or every N writes) or maintaining a bounded index rather than enumerating all traces per persist.🤖 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 98 - 110, Update persistRecallTrace so it no longer lists, sorts, and deletes the full trace set on every write. Introduce periodic or threshold-based pruning (or a bounded trace index) while preserving retentionDays and maxTraces enforcement when pruning runs; keep the normal persist path limited to storing the new trace.
🤖 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 @.env.example:
- Line 107: Update the AGENTMEMORY_RECALL_ALLOWED_ROOTS example comment to
describe the platform-specific path delimiter used by src/recall/identity.ts:
colon on Unix-like systems and semicolon on Windows, rather than documenting
only the Windows separator.
In `@src/cli/archive-watcher.ts`:
- Around line 381-387: Update classifyResponse() so non-retryable HTTP client
errors, including 401, 403, and other applicable 4xx statuses, return the
terminal archive error class instead of "transient_http". Preserve transient
classification for 5xx, 408, and 429 responses, while retaining the existing
reason-based missing and invalid classifications.
In `@src/functions/durable-candidates.ts`:
- Around line 274-296: Capture a single evaluation timestamp with new
Date().toISOString() before the for-of loop in the durable candidate evaluation
flow, then reuse that value for every recommendation’s evaluatedAt field. Keep
the per-candidate eligibility and persistence logic unchanged.
- Around line 683-775: Align the missing-session-id fallback in parseJsonlText
used by durable-candidates with mem::replay::import-jsonl, replacing
fingerprintId("sess", text) with the same deterministic generateId("sess")
behavior. Ensure both paths derive identical session IDs before the KV.sessions
lookup, while preserving embedded session IDs unchanged.
In `@src/hooks/pre-compact.ts`:
- Around line 51-67: In the pre-compact hook, isolate the context-epoch POST
before the `/agentmemory/context` request by wrapping it in its own best-effort
try/catch. Ensure failures from that epoch advance are handled without escaping
to the outer catch, so the context fetch still executes; keep the existing
context injection behavior and timeout unchanged.
In `@src/triggers/api.ts`:
- Around line 1186-1187: Update the api::remember request body type used by the
surrounding handler to declare the accessed top-level cwd and repoId fields,
alongside the existing fields. Keep the resolveRecallIdentity call using the
top-level cwd and ensure repoId is typed consistently with its usage, rather
than relying only on scope.repoId.
In `@src/types.ts`:
- Around line 386-394: Update the AuditEntry.operation union in src/types.ts to
include the lease_renew operation emitted by the lease renewal flow in
leases.ts, while preserving all existing operation values.
In `@src/viewer/index.html`:
- Line 2434: Update the trace row builder around finalContextTokenCount to
coerce or pass the value through the existing esc() helper before concatenating
it into innerHTML, while preserving the 0 fallback for missing values. Keep the
surrounding token-count rendering unchanged.
---
Outside diff comments:
In `@INSTALL_FOR_AGENTS.md`:
- Around line 103-113: Renumber the “Verify a save and recall round-trip”
section heading following “Install native skills” from step 7 to step 8, leaving
the installation section and its content unchanged.
In `@plugin/scripts/pre-tool-use.mjs`:
- Around line 62-74: The enrich request currently sends cwd, but the downstream
api::enrich flow does not use it for identity resolution. Update the
api::enrich-to-mem::enrich forwarding path to resolve and pass repoId and
checkoutId from cwd, preserving project and the existing enrich fields;
alternatively remove cwd from this request if identity scoping cannot be
supported.
In `@src/functions/observe.ts`:
- Around line 183-194: Update the error rollback path in the observe handler to
call decrementImageRef for the same file path after incrementImageRef has been
performed, before or alongside deleteImage. Preserve the existing deletedBytes
disk-size trigger and rethrow behavior.
---
Nitpick comments:
In `@src/eval/schemas.ts`:
- Around line 106-108: Update the candidate type field in the relevant schema to
reuse DurableCandidateTypeEnum instead of defining an inline z.enum list,
matching SummaryOutputSchema and keeping the existing optional behavior.
In `@src/functions/remember.ts`:
- Around line 151-226: Add a recordAudit() call to the mem::remember save flow
after persisting the new memory and updating any superseded memory, covering
both the creation and supersession state changes. Use the existing audit
conventions and memory identifiers, and do not rely on mem::cascade-update to
audit this write.
In `@src/recall/core.ts`:
- Around line 103-105: Replace the normalizeDuplicate-based deduplication with
the repository-standard fingerprintId() in the recall deduplication flow,
including updating the Set key type/usage as needed. Preserve the existing
duplicate-detection behavior while using fingerprintId() for content-addressable
identity.
- Around line 375-396: Update the hit-processing flow around the loop in the
recall method to resolve each hit’s memory and fallback session concurrently
with Promise.all, while preserving the existing candidate mapping and sessionFor
cache behavior. Avoid awaiting KV and session lookups sequentially inside the
per-hit iteration; first resolve independent per-hit data in parallel, then
build candidates with the same fields and scoring.
In `@src/recall/ledger.ts`:
- Around line 78-95: The recordInjection flow writes injection-ledger entries
without any retention, allowing KV.injectionLedger to grow indefinitely. Add a
retention or TTL cleanup mechanism for stale InjectionLedgerEntry records,
reusing the existing recall-trace retention configuration and cleanup patterns,
while preserving current writes for active entries.
In `@src/recall/tokens.ts`:
- Around line 3-25: Update CJK_CHAR and the character classification in
countTokens so Korean Hangul (U+AC00–U+D7A3) and astral CJK ideographs are
treated as individual CJK tokens rather than entering the asciiRun path. Use
code-point-aware checks compatible with for...of iteration, while preserving the
existing ASCII-run batching behavior for non-CJK text.
In `@src/recall/trace-store.ts`:
- Around line 98-110: Update persistRecallTrace so it no longer lists, sorts,
and deletes the full trace set on every write. Introduce periodic or
threshold-based pruning (or a bounded trace index) while preserving
retentionDays and maxTraces enforcement when pruning runs; keep the normal
persist path limited to storing the new trace.
In `@src/state/hybrid-search.ts`:
- Around line 128-161: Update the error handling in the graph retrieval calls
within the hybrid search flow so graphMode is set to disabled only when
graphResults remains empty after the attempted operations. Preserve successful
results from either searchByEntities or expandFromChunks, and ensure partial
failures do not overwrite a healthy/usable graph status when entries were
obtained.
In `@src/triggers/api.ts`:
- Around line 1285-1289: Remove the redundant checkoutId spread from the
mem::remember payload construction. Keep the earlier checkoutId selection that
uses the validated request-body string with identity.checkoutId as the fallback,
and leave the other payload fields unchanged.
In `@src/types.ts`:
- Around line 112-118: Confirm that DurableCandidateRecommendation is
intentionally dry-run only: retain wouldPromote as the literal false because
promotion remains deferred, and document or otherwise preserve this contract for
consumers. If the type must represent future eligible actions, widen
wouldPromote to boolean and update related recommendation handling accordingly.
In `@test/durable-candidates.test.ts`:
- Around line 130-141: Replace hardcoded memory scope strings in the
durable-candidate tests, including the cases around the recommendation setup and
the referenced sections, with the corresponding KV constants or helpers such as
KV.summaries, KV.memories, KV.durableRecommendations, and the observation-scope
helper. Keep test behavior unchanged while ensuring all key construction uses
the centralized KV definitions.
In `@test/viewer-recall-xss.test.ts`:
- Around line 21-23: Make the escaper extraction in the viewer recall XSS test
independent of the exact closing-brace indentation by anchoring the regex to a
stable, layout-insensitive boundary, or reuse the escaper from a shared
importable module. Keep the existing behavioral assertions and trusted-source
handling unchanged.
🪄 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: 612ab0ed-abc8-48a6-adb5-60ac071ae702
📒 Files selected for processing (92)
.env.example.gitignoreAGENTS.mdCODERABBIT_REVIEW_TRIAGE.mdINSTALL_FOR_AGENTS.mdP2_FINAL_ACCEPTANCE.mdREADME.mddocs/continuity-schema-note.mddocs/p1-durable-candidates-handoff-2026-07-11.mdeval/recall/README.mdeval/recall/fixtures/sanitized.jsoneval/recall/runner.tseval/recall/score.tspackage.jsonplugin/plugin.jsonplugin/scripts/archive-watcher-mutex.ps1plugin/scripts/install-archive-watcher.ps1plugin/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/scripts/uninstall-archive-watcher.ps1plugin/scripts/watch-archived-sessions.ps1plugin/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/build-info.tssrc/cli.tssrc/cli/archive-watcher.tssrc/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/kv.tssrc/state/memory-utils.tssrc/state/schema.tssrc/triggers/api.tssrc/types.tssrc/viewer/index.htmltest/archive-watcher.test.tstest/build-info.test.tstest/durable-candidate-utils.test.tstest/durable-candidates.test.tstest/recall-benchmark-score.test.tstest/recall-config.test.tstest/recall-context-limit.test.tstest/recall-core.test.tstest/recall-identity.test.tstest/recall-trace-store.test.tstest/remember-project-scope.test.tstest/replay-import-key.test.tstest/replay.test.tstest/summarize.test.tstest/vector-retrieval-health.test.tstest/viewer-recall-xss.test.ts
| # AGENTMEMORY_RECALL_MAX_SEMANTIC_TOKENS=600 | ||
| # AGENTMEMORY_RECALL_UNKNOWN_AUTO_INJECTION=false | ||
| # AGENTMEMORY_RECALL_UNKNOWN_EXPLICIT_SEARCH=true | ||
| # AGENTMEMORY_RECALL_ALLOWED_ROOTS=~/work;~/src # Semicolon-separated trusted roots for cwd-based repo identity on Windows; defaults to the worker cwd |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find how AGENTMEMORY_RECALL_ALLOWED_ROOTS is parsed in config.ts
rg -n -C5 'AGENTMEMORY_RECALL_ALLOWED_ROOTS|allowedRoots|ALLOWED_ROOTS' --type=ts src/Repository: rohitg00/agentmemory
Length of output: 3480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## .env.example around line 107\n'
sed -n '100,112p' .env.example
printf '\n## Search for docs mentioning AGENTMEMORY_RECALL_ALLOWED_ROOTS or delimiter wording\n'
rg -n -C2 'AGENTMEMORY_RECALL_ALLOWED_ROOTS|semicolon|delimiter|path.delimiter|trusted roots' --glob '!**/node_modules/**' .Repository: rohitg00/agentmemory
Length of output: 4363
Use the platform path separator here. src/recall/identity.ts splits AGENTMEMORY_RECALL_ALLOWED_ROOTS with path.delimiter, so this example should say : on Unix and ; on Windows (or just refer to the platform delimiter) instead of only mentioning Windows.
🤖 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 @.env.example at line 107, Update the AGENTMEMORY_RECALL_ALLOWED_ROOTS
example comment to describe the platform-specific path delimiter used by
src/recall/identity.ts: colon on Unix-like systems and semicolon on Windows,
rather than documenting only the Windows separator.
| function classifyResponse(status: number, reason: string): ArchiveErrorClass { | ||
| const lower = reason.toLowerCase(); | ||
| if (lower.includes("missing")) return "missing_archive"; | ||
| if (lower.includes("invalid") || lower.includes("outside") || lower.includes("symlink")) return "invalid_archive"; | ||
| if (status >= 500 || status === 408 || status === 429 || status === 401 || status === 403) return "transient_http"; | ||
| return "transient_http"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and the types/consumers for archive error handling.
sed -n '340,460p' src/cli/archive-watcher.ts
printf '\n--- usages ---\n'
rg -n "classifyResponse|ArchiveErrorClass|invalid_archive|transient_http|markPending|pending" src/cli/archive-watcher.ts src -g '!**/node_modules/**'Repository: rohitg00/agentmemory
Length of output: 17390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first, then inspect the surrounding implementation.
ast-grep outline src/cli/archive-watcher.ts --view expanded
printf '\n--- relevant slices ---\n'
sed -n '1,220p' src/cli/archive-watcher.ts
printf '\n--- middle ---\n'
sed -n '220,520p' src/cli/archive-watcher.ts
printf '\n--- tail ---\n'
sed -n '520,760p' src/cli/archive-watcher.tsRepository: rohitg00/agentmemory
Length of output: 32305
Handle non-retryable HTTP failures as terminal
classifyResponse() always returns "transient_http" for the status branch, so the check is effectively redundant. More importantly, 401/403 (and other 4xx client errors) are retried via markPending() instead of becoming terminal, so a bad auth/configuration can spin forever on backoff. Treat non-retryable client errors as terminal here.
🤖 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/cli/archive-watcher.ts` around lines 381 - 387, Update classifyResponse()
so non-retryable HTTP client errors, including 401, 403, and other applicable
4xx statuses, return the terminal archive error class instead of
"transient_http". Preserve transient classification for 5xx, 408, and 429
responses, while retaining the existing reason-based missing and invalid
classifications.
| for (const candidate of all) { | ||
| const reasons: string[] = []; | ||
| if (candidate.confidence >= 0.9) reasons.push("confidence >= 0.90"); | ||
| if (candidate.project) reasons.push("project scope explicit"); | ||
| if (candidate.sourceObservationIds.length >= 3) reasons.push("3 evidence observations"); | ||
| const hasConflict = memories.some((memory) => | ||
| memory.isLatest !== false && | ||
| memory.project === candidate.project && | ||
| jaccardSimilarity(memory.content.toLowerCase(), candidate.content.toLowerCase()) > 0.7, | ||
| ); | ||
| if (!hasConflict) reasons.push("no conflicting memory"); | ||
| if (!isPlanOrTodo(candidate)) reasons.push("not a plan or todo"); | ||
| const eligible = reasons.length === 5; | ||
| const recommendation: DurableCandidateRecommendation = { | ||
| candidateId: candidate.id, | ||
| recommendation: eligible ? "auto_promote_eligible" : "not_eligible", | ||
| reasons, | ||
| wouldPromote: false, | ||
| evaluatedAt: new Date().toISOString(), | ||
| }; | ||
| await kv.set(KV.durableRecommendations, candidate.id, recommendation); | ||
| recommendations.push(recommendation); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Capture the evaluation timestamp once. evaluatedAt: new Date().toISOString() is recomputed on every loop iteration, producing slightly different timestamps for a single evaluation run. Capture it once before the loop and reuse it.
♻️ Proposed change
const memories = await kv.list<Memory>(KV.memories);
const recommendations: DurableCandidateRecommendation[] = [];
+ const evaluatedAt = new Date().toISOString();
for (const candidate of all) {- evaluatedAt: new Date().toISOString(),
+ evaluatedAt,As per coding guidelines: "Capture timestamps once with new Date().toISOString() and reuse the captured value."
📝 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.
| for (const candidate of all) { | |
| const reasons: string[] = []; | |
| if (candidate.confidence >= 0.9) reasons.push("confidence >= 0.90"); | |
| if (candidate.project) reasons.push("project scope explicit"); | |
| if (candidate.sourceObservationIds.length >= 3) reasons.push("3 evidence observations"); | |
| const hasConflict = memories.some((memory) => | |
| memory.isLatest !== false && | |
| memory.project === candidate.project && | |
| jaccardSimilarity(memory.content.toLowerCase(), candidate.content.toLowerCase()) > 0.7, | |
| ); | |
| if (!hasConflict) reasons.push("no conflicting memory"); | |
| if (!isPlanOrTodo(candidate)) reasons.push("not a plan or todo"); | |
| const eligible = reasons.length === 5; | |
| const recommendation: DurableCandidateRecommendation = { | |
| candidateId: candidate.id, | |
| recommendation: eligible ? "auto_promote_eligible" : "not_eligible", | |
| reasons, | |
| wouldPromote: false, | |
| evaluatedAt: new Date().toISOString(), | |
| }; | |
| await kv.set(KV.durableRecommendations, candidate.id, recommendation); | |
| recommendations.push(recommendation); | |
| } | |
| const memories = await kv.list<Memory>(KV.memories); | |
| const recommendations: DurableCandidateRecommendation[] = []; | |
| const evaluatedAt = new Date().toISOString(); | |
| for (const candidate of all) { | |
| const reasons: string[] = []; | |
| if (candidate.confidence >= 0.9) reasons.push("confidence >= 0.90"); | |
| if (candidate.project) reasons.push("project scope explicit"); | |
| if (candidate.sourceObservationIds.length >= 3) reasons.push("3 evidence observations"); | |
| const hasConflict = memories.some((memory) => | |
| memory.isLatest !== false && | |
| memory.project === candidate.project && | |
| jaccardSimilarity(memory.content.toLowerCase(), candidate.content.toLowerCase()) > 0.7, | |
| ); | |
| if (!hasConflict) reasons.push("no conflicting memory"); | |
| if (!isPlanOrTodo(candidate)) reasons.push("not a plan or todo"); | |
| const eligible = reasons.length === 5; | |
| const recommendation: DurableCandidateRecommendation = { | |
| candidateId: candidate.id, | |
| recommendation: eligible ? "auto_promote_eligible" : "not_eligible", | |
| reasons, | |
| wouldPromote: false, | |
| evaluatedAt, | |
| }; | |
| await kv.set(KV.durableRecommendations, candidate.id, recommendation); | |
| recommendations.push(recommendation); | |
| } |
🤖 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 274 - 296, Capture a single
evaluation timestamp with new Date().toISOString() before the for-of loop in the
durable candidate evaluation flow, then reuse that value for every
recommendation’s evaluatedAt field. Keep the per-candidate eligibility and
persistence logic unchanged.
Source: Coding guidelines
| 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); | ||
| if (!session) { | ||
| record = { | ||
| ...record, | ||
| status: "failed", | ||
| failureStage: "observations", | ||
| lastError: "session_not_found_after_import", | ||
| updatedAt: new Date().toISOString(), | ||
| }; | ||
| await kv.set(KV.archiveImports, idempotencyKey, record); | ||
| skipped.push({ | ||
| archivePath: file, | ||
| sessionId: parsed.sessionId, | ||
| reason: record.lastError || "session_not_found_after_import", | ||
| }); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the two call sites use different fallback strategies.
rg -nP 'parseJsonlText\(' src/functions/durable-candidates.ts src/functions/replay.ts
# Confirm parseJsonlText's default fallback is deterministic
ast-grep run --pattern 'const effectiveSessionId = $$$' --lang typescript src/replay/jsonl-parser.tsRepository: rohitg00/agentmemory
Length of output: 459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant import path and parser fallback details with context.
sed -n '360,430p' src/functions/replay.ts
printf '\n---\n'
sed -n '120,155p' src/replay/jsonl-parser.ts
printf '\n---\n'
sed -n '683,775p' src/functions/durable-candidates.tsRepository: rohitg00/agentmemory
Length of output: 7685
Session-id fallback should match mem::replay::import-jsonl.
durable-candidates falls back to fingerprintId("sess", text), but import-jsonl re-parses the same file with generateId("sess"). For files without an embedded session id, the import writes under a different session key, so kv.get<Session>(KV.sessions, parsed.sessionId) misses and the archive is marked session_not_found_after_import. Use the same fallback on both paths, preferably the deterministic one.
🤖 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 - 775, Align the
missing-session-id fallback in parseJsonlText used by durable-candidates with
mem::replay::import-jsonl, replacing fingerprintId("sess", text) with the same
deterministic generateId("sess") behavior. Ensure both paths derive identical
session IDs before the KV.sessions lookup, while preserving embedded session IDs
unchanged.
| await fetch(`${REST_URL}/agentmemory/recall/context-epoch`, { | ||
| method: "POST", | ||
| headers: authHeaders(), | ||
| body: JSON.stringify({ sessionId }), | ||
| signal: AbortSignal.timeout(1000), | ||
| }); | ||
| const res = await fetch(`${REST_URL}/agentmemory/context`, { | ||
| method: "POST", | ||
| headers: authHeaders(), | ||
| body: JSON.stringify({ sessionId, project, budget: 1500 }), | ||
| body: JSON.stringify({ | ||
| sessionId, | ||
| project, | ||
| cwd: (data.cwd as string | undefined) || process.cwd(), | ||
| outputMode: "bootstrap", | ||
| }), | ||
| signal: AbortSignal.timeout(5000), | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Isolate the context-epoch advance so its failure doesn't skip context injection.
Both requests live in the same try. If the 1s context-epoch POST throws (timeout/transient error), control jumps straight to the outer catch and the /agentmemory/context fetch is never issued, so compaction loses its bootstrap context. Wrap the epoch advance in its own best-effort try so context injection still proceeds.
🛡️ Proposed fix
try {
- await fetch(`${REST_URL}/agentmemory/recall/context-epoch`, {
- method: "POST",
- headers: authHeaders(),
- body: JSON.stringify({ sessionId }),
- signal: AbortSignal.timeout(1000),
- });
+ try {
+ await fetch(`${REST_URL}/agentmemory/recall/context-epoch`, {
+ method: "POST",
+ headers: authHeaders(),
+ body: JSON.stringify({ sessionId }),
+ signal: AbortSignal.timeout(1000),
+ });
+ } catch {
+ // best-effort epoch advance; still fetch context below
+ }
const res = await fetch(`${REST_URL}/agentmemory/context`, {📝 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.
| await fetch(`${REST_URL}/agentmemory/recall/context-epoch`, { | |
| method: "POST", | |
| headers: authHeaders(), | |
| body: JSON.stringify({ sessionId }), | |
| signal: AbortSignal.timeout(1000), | |
| }); | |
| const res = await fetch(`${REST_URL}/agentmemory/context`, { | |
| method: "POST", | |
| headers: authHeaders(), | |
| body: JSON.stringify({ sessionId, project, budget: 1500 }), | |
| body: JSON.stringify({ | |
| sessionId, | |
| project, | |
| cwd: (data.cwd as string | undefined) || process.cwd(), | |
| outputMode: "bootstrap", | |
| }), | |
| signal: AbortSignal.timeout(5000), | |
| }); | |
| try { | |
| await fetch(`${REST_URL}/agentmemory/recall/context-epoch`, { | |
| method: "POST", | |
| headers: authHeaders(), | |
| body: JSON.stringify({ sessionId }), | |
| signal: AbortSignal.timeout(1000), | |
| }); | |
| } catch { | |
| // best-effort epoch advance; still fetch context below | |
| } | |
| const res = await fetch(`${REST_URL}/agentmemory/context`, { | |
| method: "POST", | |
| headers: authHeaders(), | |
| body: JSON.stringify({ | |
| sessionId, | |
| project, | |
| cwd: (data.cwd as string | undefined) || process.cwd(), | |
| outputMode: "bootstrap", | |
| }), | |
| signal: AbortSignal.timeout(5000), | |
| }); |
🤖 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/hooks/pre-compact.ts` around lines 51 - 67, In the pre-compact hook,
isolate the context-epoch POST before the `/agentmemory/context` request by
wrapping it in its own best-effort try/catch. Ensure failures from that epoch
advance are handled without escaping to the outer catch, so the context fetch
still executes; keep the existing context injection behavior and timeout
unchanged.
| const cwd = typeof req.body.cwd === "string" && req.body.cwd.trim() ? req.body.cwd.trim() : undefined; | ||
| const identity = cwd ? await resolveRecallIdentity(cwd, req.body.project || "") : undefined; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --pattern 'interface ApiRequest$_ { $$$ }' --lang typescript src/triggers/api.ts
rg -nP 'type ApiRequest|interface ApiRequest' src -A6Repository: rohitg00/agentmemory
Length of output: 744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the request handler and the ApiRequest type used by src/triggers/api.ts.
sed -n '1140,1305p' src/triggers/api.ts
echo '---'
rg -n "export (interface|type) ApiRequest|interface ApiRequest|type ApiRequest" . -g '!dist' -g '!build' -g '!node_modules' -A12 -B4Repository: rohitg00/agentmemory
Length of output: 8273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the SDK type declarations if they're vendored or referenced locally.
fd -a "iii-sdk" .
fd -a "api.d.ts" .
fd -a "index.d.ts" .Repository: rohitg00/agentmemory
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the declared iii-sdk version and whether any local typings pin ApiRequest's body shape.
sed -n '1,220p' package.json
echo '---'
rg -n '"iii-sdk"|ApiRequest' package.json pnpm-lock.yaml package-lock.json yarn.lock src -A4 -B4Repository: rohitg00/agentmemory
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe whether the dependency types are available in the lockfile or workspace sources.
fd -a "iii-sdk" . -t f
fd -a "*.d.ts" . -t f | head -n 50Repository: rohitg00/agentmemory
Length of output: 444
🌐 Web query:
iii-sdk 0.11.2 ApiRequest type definition body index signature cwd repoId
💡 Result:
In the iii-sdk (version 0.11.x), the ApiRequest type is a generic TypeScript object used for HTTP trigger handlers [1]. The type definition typically includes specific fields for request metadata and data, but it does not contain index signatures for 'cwd' or 'repoId' [2][1]. Based on authoritative documentation and source code for the iii-sdk, the ApiRequest structure is defined as follows [1]: type ApiRequest<TBody = unknown> = { path_params: Record<string, string>; // Route parameters query_params: Record<string, string | string[]>; // Query string body: TBody; // Parsed JSON body headers: Record<string, string | string[]>; // Request headers method: string; // HTTP method request_body: any; // Raw body stream (sometimes referenced as ChannelReader) } The fields 'cwd' (current working directory) and 'repoId' (repository identifier) are not part of the standard ApiRequest type definition in this SDK [2][1]. These concepts are generally not exposed via the incoming HTTP request object in the iii-sdk architecture, which focuses on function registration, trigger binding, and invocation over WebSockets [3][4][1]. If you are attempting to access repository or environment-specific data, such information is typically managed by the iii Engine or external configuration, not the runtime ApiRequest object [4][5].
Citations:
- 1: https://www.mintlify.com/iii-hq/sdk/concepts/triggers
- 2: https://docs.rs/iii-sdk/latest/src/iii_sdk/types.rs.html
- 3: https://www.npmjs.com/package/iii-sdk
- 4: https://www.mintlify.com/iii-hq/sdk/introduction
- 5: https://iii.dev/docs/0-10-0/api-reference/sdk-node
🌐 Web query:
iii-sdk ApiRequest TypeScript definition generic body index signature
💡 Result:
In the iii-sdk for Node.js/TypeScript, the ApiRequest interface is used to represent incoming HTTP requests [1]. While the SDK documentation provides standard field definitions (such as path, method, headers, and query parameters) [1], the body field within ApiRequest is typically defined as a generic or an any-type to accommodate varying JSON payloads [1]. Regarding the use of an index signature to handle dynamic body content, developers often define their specific request body types when working with functions in iii-sdk [2][3]. Because the Node.js SDK does not automatically infer types from TypeScript [3], developers are encouraged to use explicit schemas (e.g., via Zod and JSON Schema) to validate and structure the request data [3]. If you need to define a body with dynamic keys, you can implement an interface with an index signature in your handler function's input type: interface CustomBody { [key: string]: any; // or define specific known keys alongside dynamic ones } // Example usage async function myHandler(req: ApiRequest): Promise { const body = req.body as CustomBody; //... } For formal definitions, the iii-sdk exports the ApiRequest type directly [1][4], and you can inspect the package's type definitions (located in the installed node_modules/iii-sdk/dist/index.d.ts or similar paths) to see the exact structure currently provided by your installed version [5]. Changes to type definitions are managed within the sdk/packages/node/iii/src/iii-types.ts file in the official repository [6].
Citations:
- 1: https://iii.dev/docs/0-10-0/modules/module-http
- 2: https://iii.dev/docs/creating-workers/functions
- 3: https://iii.dev/docs/how-to/define-request-response-formats
- 4: https://iii.dev/docs/0-10-0/api-reference/sdk-node
- 5: https://www.npmjs.com/package/iii-sdk
- 6: feat: update tsdown version, add zod dependency, and enhance type def… iii-hq/iii#1343
Add cwd and top-level repoId to the api::remember request body type req.body.cwd and req.body.repoId are accessed here, but the ApiRequest<...> generic only declares scope.repoId. Either extend the body type or read the nested field consistently.
🤖 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 1186 - 1187, Update the api::remember
request body type used by the surrounding handler to declare the accessed
top-level cwd and repoId fields, alongside the existing fields. Keep the
resolveRecallIdentity call using the top-level cwd and ensure repoId is typed
consistently with its usage, rather than relying only on scope.repoId.
| export interface RecallItemStats { | ||
| itemId: string; | ||
| recallCount: number; | ||
| lastRecalledAt?: string; | ||
| recentQuery?: string; | ||
| averageScore: number; | ||
| scopeMismatchCount: number; | ||
| /** Scaled integer accumulator used to derive averageScore atomically. */ | ||
| scoreTotalMicros?: number; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e ts schema.ts src/state --exec cat -n {}
rg -nP 'recallStats|recallTrace|injectionLedger|archiveImport' src/state/schema.tsRepository: rohitg00/agentmemory
Length of output: 5424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the AuditEntry type and nearby unions in src/types.ts.
fd -e ts types.ts src --exec sh -c 'echo "--- {}"; cat -n "$1" | sed -n "1,260p"' sh {} \;
# Search for audit operation literals and recordAudit calls.
rg -n "recordAudit\\(|operation:\\s*\"|operation\\s*\\|" srcRepository: rohitg00/agentmemory
Length of output: 20645
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the AuditEntry type in src/types.ts and print the surrounding lines.
rg -n "interface AuditEntry|type AuditEntry|operation:" src/types.ts
cat -n src/types.ts | sed -n '260,420p'Repository: rohitg00/agentmemory
Length of output: 4832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/types.ts | sed -n '790,860p'Repository: rohitg00/agentmemory
Length of output: 2055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re, pathlib
types = pathlib.Path("src/types.ts").read_text()
m = re.search(r"export interface AuditEntry\s*{.*?operation:\s*(.*?)\n\s*userId\?:", types, re.S)
if not m:
raise SystemExit("AuditEntry.operation union not found")
union_block = m.group(1)
union_ops = re.findall(r'"([^"]+)"', union_block)
ops = set()
for path in pathlib.Path("src").rglob("*.ts"):
txt = path.read_text()
for mm in re.finditer(r'recordAudit\(\s*[^,]+,\s*"([^"]+)"', txt):
ops.add(mm.group(1))
missing = sorted(ops - set(union_ops))
extra = sorted(set(union_ops) - ops)
print("UNION_COUNT", len(union_ops))
print("CALLSITE_COUNT", len(ops))
print("MISSING_FROM_UNION", missing)
print("UNUSED_IN_CALLS", extra)
PYRepository: rohitg00/agentmemory
Length of output: 391
Add lease_renew to AuditEntry.operation The new KV scopes are already registered; the remaining gap is that src/functions/leases.ts emits lease_renew, but src/types.ts does not include it in the audit operation union.
🤖 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/types.ts` around lines 386 - 394, Update the AuditEntry.operation union
in src/types.ts to include the lease_renew operation emitted by the lease
renewal flow in leases.ts, while preserving all existing operation values.
Source: Path instructions
| '<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>' + |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Escape finalContextTokenCount for consistency. Every other trace field is run through esc(), but this one is concatenated raw with only a || 0 fallback. If a trace ever carries a non-numeric string here it would be injected into innerHTML unescaped. Coerce/escape it to keep the row builder uniformly safe.
🛡️ Proposed change
- '<td>' + (trace.finalContextTokenCount || 0) + ' tokens</td>' +
+ '<td>' + esc(String(trace.finalContextTokenCount || 0)) + ' tokens</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.
| '<td>' + (trace.finalContextTokenCount || 0) + ' tokens</td>' + | |
| '<td>' + esc(String(trace.finalContextTokenCount || 0)) + ' tokens</td>' + |
🤖 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` at line 2434, Update the trace row builder around
finalContextTokenCount to coerce or pass the value through the existing esc()
helper before concatenating it into innerHTML, while preserving the 0 fallback
for missing values. Keep the surrounding token-count rendering unchanged.
1 participant
What
Three layered capabilities, same infrastructure:
P1: Durable Candidates
POST /agentmemory/archive/process)POST /agentmemory/durable-candidates/promote)P2: Scoped Traceable Recall
src/recall/core.ts) — all search paths route through itRecallTracewith selected/dropped items and structured reasonsrecall-debug,why-memoryCodex Archive Watcher
~/.codex/archived_sessions/*.jsonlevery 30sPOST /agentmemory/session/end~/.agentmemory/archive-watcher-state.jsonfor crash recoveryCodeRabbit review fixes (round-trip from PR #1050)
src/recall/identity.ts: validate cwd against allowed root)state::update+ increment instead of get/recompute/set)Verification
Known limitations (P1)
Archive corpus alignment is not yet resolved — see
docs/p1-durable-candidates-handoff-2026-07-11.md.Closes #1050
Summary by CodeRabbit
New Features
Documentation
Tests