Commit Graph

2 Commits

Author SHA1 Message Date
Rohit Ghumare 5d70ecfb3c feat: migrate agentmemory to iii v0.11 and add upgrade command (#116)
* feat: migrate agentmemory to iii v0.11 and add upgrade command

Migrate the codebase from legacy iii-sdk v0.3 APIs to v0.11 trigger/register patterns, update runtime configs, and align stream/state behavior with newer engine semantics. Add a new `agentmemory upgrade` CLI command so users can refresh dependencies and iii runtime components with one entrypoint.

* chore: remove pnpm lockfile from migration PR

Drop pnpm-lock.yaml from this branch to keep the migration PR focused on source and config changes only.

* fix(ci): sync npm lockfile with iii-sdk v0.11 dependency

Update package-lock.json so npm ci in CI matches package.json after the iii-sdk migration.

* fix(ci): resolve build parse errors after v0.11 migration

Fix malformed braces introduced during API migration in triggers/health/branch-aware files so tsdown build passes in CI.

* fix: harden migration follow-up fixes and audits

Apply the reviewed v0.11 follow-up fixes across runtime, docs, and tests by tightening validation, correcting upgrade/error handling, and adding missing audit coverage on state mutations. This also addresses stream fallback behavior, lock consistency, retention source-bucket cleanup, and test/mock alignment so build and test remain green.

* fix: address 9 unresolved CodeRabbit findings on iii v0.11 migration

CodeRabbit flagged 9 remaining issues on #116 — all real. Each was
verified against the current branch code before applying. Two
findings were deliberately skipped as policy/scope issues and are
documented at the bottom.

### Applied (9 real findings)

- src/triggers/api.ts — api.ts numeric query param validation:
  multiple sites forwarded `parseInt(params.limit)` result to the
  downstream function without a Number.isFinite check. A non-numeric
  query value produced NaN at the iii-sdk trigger boundary. Added
  parseOptionalInt and parseOptionalFloat helpers at the top of the
  file, wired into api::crystal-list, api::lesson-list, and
  api::insight-list (5 sites total).

- src/triggers/api.ts — api::observe, api::context, api::session::start,
  api::session::end were forwarding req.body verbatim to sdk.trigger
  with no validation. Added explicit type checks mirroring the existing
  api::search pattern, construct a sanitized payload object before
  triggering.

- src/cli.ts — p.confirm() can return a cancel Symbol on Ctrl+C, which
  is truthy, so the upgrade path ran even when the user cancelled.
  Added p.isCancel() check and explicit boolean comparison.

- src/cli.ts — removed dead `failed` flag in runUpgrade: every caller
  already calls process.exit(1) immediately, so the final `if (failed)`
  branch was unreachable. Simplified requireSuccess accordingly.

- src/functions/leases.ts — mem::lease-renew recorded audit events as
  "lease_acquire", which conflated renewals with first claims. Renamed
  the audit event to "lease_renew" so downstream audit filters can tell
  the two operations apart.

- src/functions/relations.ts — the pair lock
  `mem:${firstId}:${secondId}` serialized concurrent relate(A,B) calls
  with identical pairs, but did NOT protect concurrent relate(A,B) +
  relate(A,C). Both modify memory A's `relatedIds` array, which is a
  classic last-writer-wins race producing lost relation edges. Fixed
  by replacing the pair lock with nested per-entity locks in canonical
  sort order: withKeyedLock(mem:firstId) wrapping withKeyedLock(mem:secondId).
  Since the ids are sorted deterministically, no deadlock is possible.

- src/functions/sketches.ts + src/functions/summarize.ts + new
  src/functions/audit.ts safeAudit helper — recordAudit was awaited
  after kv.set calls. An audit write failure would reject and the
  caller would see an error even though the target state was already
  persisted. New safeAudit wrapper swallows audit errors and logs them
  via ctx.logger.warn, preserving the mutation's success. Applied to
  all 11 audit sites in sketches.ts and the single site in summarize.ts.

- src/mcp/server.ts — three issues in the MCP handler:
  1. memory_profile's refresh flag used `args.refresh === "true"`,
     ignoring boolean `true` from MCP clients that send proper types.
     Now accepts both.
  2. memory_sketch_create built sketchPayload with
     `asNonEmptyString(args.title)` which could return undefined,
     then forwarded that to the downstream function. Added explicit
     validation + 400 response at the MCP boundary.
  3. memory_recall, memory_team_feed, memory_audit_query used
     `(args.limit as number) || 10` which replaced an explicit 0 with
     10. Changed to typeof check so explicit 0 (rare but legal) is
     preserved.

- src/functions/governance.ts — mem::governance-bulk used Promise.all
  for the delete batch, which fails fast on the first error. The audit
  record still said `deleted: candidates.length` even if only half
  actually succeeded. Switched to Promise.allSettled, split results
  into successfulIds and failures arrays, record audit with both counts
  plus the per-failure details for traceability.

- src/functions/mesh.ts — mem::mesh-register, mem::mesh-sync,
  mem::mesh-receive, mem::mesh-remove all dereferenced `data.*` without
  checking that data was passed. A TypeError would bubble up on null
  payload. Added early null/type guards returning structured errors.

- src/functions/obsidian-export.ts — resolveVaultDir(data.vaultDir)
  was called without validating that vaultDir was a string, and
  `new Set(data.types)` without validating types was an array of
  strings. Added explicit validation returning 400-style error
  responses.

- src/functions/retention.ts — the eviction loop silently swallowed
  kv.delete errors via a bare `continue`. Added ctx.logger.warn on
  catch, included memoryId and sourceBucket in the log, and now
  returns `failed` count alongside `evicted` in the response.

- src/viewer/index.html — two WebSocket bugs:
  1. connectWs assigned to the mutable global state.ws before binding
     handlers, so old sockets could have their callbacks fire and
     mutate retry/direct state after state.ws was overwritten by a
     new socket. Fixed by creating a local `ws` variable, binding
     all handlers to it, only then assigning state.ws = ws. Each
     handler guards `if (state.ws !== ws) return` so stale callbacks
     are dropped.
  2. handleStreamEvent routed EVERY incoming event to routeWsMessage,
     so non-observation events (session.activity, etc.) were being
     treated as timeline observations and causing UI confusion. Added
     a looksLikeObservation helper + event_type gate that only routes
     real observation payloads.

- test/retention.test.ts — added an explicit sourceBucket eviction
  test that seeds a semantic memory at high age, runs retention-score,
  then runs retention-evict at high threshold and asserts BOTH
  KV.semantic and KV.memories are empty. Proves the candidate.sourceBucket
  branch added in this PR actually routes to the right bucket.

- src/types.ts — side fix: the ExportData.version union on line 254
  used comma separators instead of pipes in 3 positions (0.7.9,
  0.8.0, 0.8.1 → needed | between them). tsdown strips types so the
  build passed, but tsc --noEmit would have thrown and any IDE showed
  squiggles. Pre-existing latent bug, fixed while in the file.

### Deliberately skipped

- retention.ts eviction audit (CodeRabbit asked to add recordAudit
  to the eviction loop): policy change, not a bug fix. Verified:
  auto-forget.ts, evict.ts, retention-evict, remember-forget all
  skip audit by convention — only governance audits. Adding audit
  everywhere is a policy decision tracked in issue #125.

- file-index.ts sequential → parallel session lookup: micro-opt,
  the loop is already cache-backed, negligible savings, not worth
  the readability cost.

Tests: 655/655. Build clean.

* fix(api): harden request validation at HTTP boundaries

Validate and sanitize session, observe, and context inputs plus numeric query params before forwarding to memory functions, returning 400 for invalid values instead of propagating malformed payloads. Also remove duplicate CLI command registration introduced during merge resolution.

* fix: address 6 new CodeRabbit findings on iii v0.11 migration (#116 round 2)

CodeRabbit's second review pass on #116 flagged 6 real issues introduced
by the previous round of fixes (commit 3e3ac7e). Each was verified
against the current code before applying.

### Fixed

- src/functions/governance.ts — mem::governance-bulk switched to
  Promise.allSettled in the previous round but fanned out every
  kv.delete concurrently. The governance endpoint can target tens of
  thousands of memories in a single call; firing them all at once
  overwhelms the state worker. Batched with BATCH_SIZE=50 chunks,
  awaited serially while preserving the per-batch allSettled so a
  single failure doesn't abort the rest.
- src/functions/governance.ts — the recordAudit call AFTER the bulk
  delete is already committed was still using `await recordAudit`, so
  an audit write failure would bubble up as a "failed" response even
  though the rows were actually deleted. Switched to safeAudit so the
  deletes are reflected faithfully in the response regardless of
  audit health.
- src/functions/relations.ts — the mem::relate handler was emitting
  three recordAudit calls INTERLEAVED with three kv.set writes:
  relation → audit → source update → audit → target update → audit.
  A thrown audit in the middle would leave KV.relations and
  source/target.relatedIds in a partial state. Restructured to
  complete all three durable writes first, then emit a single
  safeAudit at the end. Also changed the operation from the generic
  "evolve" (which made audit queries indistinguishable from actual
  version evolutions) to a new "relation_create" event, added to the
  AuditEntry.operation union in src/types.ts.
- src/functions/retention.ts — mem::retention-score used
  `{ ...DEFAULT_DECAY, ...data.config }` which is only a shallow
  merge. A caller passing `{ config: { tierThresholds: { cold: 0.2 } } }`
  would drop the hot and warm thresholds, causing every downstream
  `s.score >= config.tierThresholds.hot` comparison to produce NaN
  and misclassify every tier bucket. Deep-merged tierThresholds
  explicitly.
- src/functions/retention.ts — mem::retention-evict previously
  used `candidate.sourceBucket || KV.memories` as the delete bucket.
  That's correct for new retention scores (which now store
  sourceBucket) but wrong for pre-migration scores whose sourceBucket
  is undefined: the fallback would try to delete semantic ids from
  KV.memories and silently no-op, leaving the real semantic memory
  alive. Fixed by splitting the delete path: if sourceBucket is set,
  use it; otherwise attempt both KV.memories and KV.semantic with
  individual .catch(() => {}) wrappers so whichever bucket actually
  contains the id succeeds and the other is a no-op. Legacy rows
  retire naturally on the next scoring run, which writes sourceBucket.
- src/triggers/api.ts — api::obsidian-export was validated at the
  downstream mem::obsidian-export layer in the previous round, but
  the API boundary handler was still forwarding body.vaultDir without
  a type check. Added the same 400 response pattern used by
  api::search / api::observe / api::context for consistency.

### Skipped from CodeRabbit's suggestions

- CodeRabbit's proposed fix for retention.ts:249 was
  `candidate.sourceBucket || KV.retentionScores` which would try to
  delete the memory from the retention scores namespace — that's
  where the scoring entry lives, not where the memory lives. That
  fix is wrong. The two-bucket fallback approach above is the
  correct one.

Tests: 655/655. Build clean.

* fix(agentmemory): harden deletion, relation, and API validation paths

Bound retention/governance side effects and make audit logging best-effort so successful state writes are not masked by telemetry failures. Also tighten boundary validation for MCP/API inputs, resolve relation lock edge cases, and align retention test mocks with v0.11 trigger/registerFunction call shapes.

* fix: address remaining CodeRabbit validation and eviction issues

Remove duplicate CLI help examples, harden eviction/delete bookkeeping, allow file-context without sessionId, sanitize governance failure output, validate retention-evict inputs with bounded limits, and tighten summarize/observations/obsidian-export input validation.

* fix(v0.11): align stream send payloads and migration docs

Use the engine-compatible stream payload key `type` for stream::send events and update docs/plugin examples to reflect v0.11 function registration and trigger request shapes.
2026-04-15 14:02:22 +01:00
Rohit Ghumare 857f71e3c6 feat: v0.6.0 advanced retrieval with real-world benchmarks (#76)
* feat: add 9 orchestration modules for v0.5.0

Add actions, frontier, leases, routines, signals, checkpoints,
flow-compress, mesh, and branch-aware modules with full MCP tools,
REST endpoints, and 170 new tests. Includes SSRF protection,
race-condition-safe keyed mutex locking, SHA-256 fingerprinting,
and fixes for 29 CodeRabbit review findings.

- 9 new source files (src/functions/*)
- 8 new test files (170 tests, total 386)
- 10 new MCP tools, 23 new REST endpoints
- 8 new KV scopes, new types for orchestration
- Version bump to 0.5.0, README and viewer updated

* feat: add sentinels, sketches, crystallize, diagnostics, facets modules

5 new modules inspired by beads patterns but with original naming and
iii-engine real-time streaming (no polling):

- sentinels: event-driven condition watchers (webhook, timer, threshold,
  pattern, approval) that auto-unblock gated actions via SSE
- sketches: ephemeral action graphs with auto-expiry, promote or discard
- crystallize: LLM-powered compaction of completed action chains into
  compact crystal digests with key outcomes and lessons
- diagnostics: self-diagnosis across 8 categories (actions, leases,
  sentinels, sketches, signals, sessions, memories, mesh) with auto-heal
- facets: multi-dimensional tagging (dimension:value) with AND/OR queries

- 5 new source files, 5 new test files (132 tests, total 518)
- 9 new MCP tools (total 37), 21 new REST endpoints (total 93)
- 4 new KV scopes (total 33), new types for all modules
- README stats and function table updated

* fix: address code review findings across v0.5.0 modules

- actions: validate edges before persisting, set blocked status for requires deps
- leases: use mem:action lock key, reject blocked actions, check expiry on release
- checkpoints: validate linkedActionIds exist, check requires edges in unblock
- mesh: add SSRF validation on peer registration
- routines: remove invalid "failed" action status check
- export-import: add v0.5.0 scope export/import (actions, sentinels, sketches, etc)
- mcp/server: validate CSV inputs are strings before splitting
- schema: replace runtime require with static import
- README: fix stale tool/endpoint counts (28→37, 72→93)

* fix: second round code review — mesh locks, routines DAG, MCP input validation, README counts

- mesh.ts: add withKeyedLock on action writes in receive path, add IPv6 private ranges to SSRF check
- routines.ts: validate DAG (duplicate orders, unknown deps), set dep actions to blocked, refresh stepStatus in routine-status
- checkpoints.ts: runtime type enum validation, set linked pending actions to blocked
- leases.ts: validate ttlMs is finite positive number
- export-import.ts: add skip strategy checks for all v0.5.0 import blocks
- mcp/server.ts: typeof guards on tags, config JSON.parse, actionIds, linkedActionIds, categories
- README.md: Tools 18→37, Functions 33→50, stats line updated
- test: update checkpoint test for new blocked-on-create behavior

* fix: third round review — lease renew/release safety, mesh locking, export replace cleanup, MCP input guards

- leases.ts: renew extends from max(now, existing expiry) instead of now; release verifies action ownership before mutation
- mesh.ts: withKeyedLock on memory writes in mesh-receive; applySyncData validates id/updatedAt and locks both memory and action writes
- routines.ts: stepStatus maps "blocked" to "pending" explicitly; progress includes blocked/cancelled counts; routine-freeze wrapped in withKeyedLock
- export-import.ts: remove unused RoutineRun import; replace strategy clears all orchestration namespaces; skip strategy for graphNodes/graphEdges/semantic/procedural
- mcp/server.ts: sentinel_trigger JSON.parse with typeof+try/catch; facet_query typeof guards on matchAll/matchAny; remove redundant requires cast; .filter(Boolean) on concepts/files/tags/requires CSV splits
- README.md: clarify API table is a representative subset

* fix: fourth round review — redirect SSRF, blocked-on-create, missing action handling, boolean normalization

- mesh.ts: add redirect:"error" to both outbound fetch calls to prevent SSRF via redirect
- routines.ts: create actions with status "blocked" directly when hasDeps (eliminates two-pass race); handle missing actions in routine-status as cancelled; progress.total uses run.actionIds.length
- mcp/server.ts: sentinel config accepts object values directly; normalize unreadOnly/dryRun for both JSON booleans and string values
- README.md: consistent bundle size (365KB) across both occurrences

* feat: v0.6.0 advanced retrieval — triple-stream search, stemming, real benchmarks

Search improvements:
- Porter stemmer for word normalization (authentication ↔ authenticating)
- 40+ coding-domain synonym groups (db ↔ database, k8s ↔ kubernetes)
- Binary-search prefix matching replaces O(n) full scan
- Session diversification (max 3 results per session)
- Co-occurrence graph edges between all concept pairs

New retrieval modules:
- Sliding window inference pipeline (context enrichment at ingestion)
- Adaptive query expansion (LLM-generated reformulations)
- Triple-stream search (BM25 + Vector + Graph with RRF fusion)
- Append-only temporal knowledge graph (versioned edges, point-in-time queries)
- Graph-augmented retrieval (entity search + neighborhood expansion)
- Ebbinghaus retention scoring (decay + tiered hot/warm/cold/evictable)

Real-world benchmarks (240 observations, 20 labeled queries):
- Quality eval: 64.1% recall@10 with Xenova embeddings (vs 55.8% grep)
- Scale eval: 92-100% token savings vs built-in memory at 240-50K observations
- Cross-session: 12/12 queries found vs 10/12 for 200-line MEMORY.md cap
- Token measurement uses actual search results (fixed fake constant bug)
- Removed old microbenchmarks (bench.ts, run-bench.ts, COMPARISON.md)
2026-03-18 08:47:35 +00:00