feat(memory): add createSupermemoryClient factory#83
Conversation
Extracted from sage's `src/integrations/supermemory-client.ts` (originally
sage PR #198) so every AA consumer that talks to Supermemory shares the
same endpoint, timeout, retry, and Worker-fetch wiring instead of
re-rolling their own copy. Sage previously had four call sites with
subtly different defaults; centralising removes that drift class for all
future consumers.
Wraps the official `supermemory` SDK (v4.21.1) with:
- Default `timeout: 8s` (matches the per-call cap that fixed the
2026-05-04 SEARCH_ERROR outage in prod — sage PR #198)
- Default `maxRetries: 2` (SDK default, surfaced explicitly so it's
obvious from the call site that 5xx retries happen transparently)
- Per-call `globalThis.fetch` closure for the SDK's `fetch` option
(keeps `vi.stubGlobal('fetch', ...)` working — SDK captures the
reference at construction time otherwise)
- Returns `null` when the API key env var is unset so callers no-op
cleanly during local dev without an explicit env-presence check
Worker-safe (matches AA PR #82's same-day fix):
- Public surface uses `EnvSource = Readonly<Record<string, string |
undefined>>` instead of `NodeJS.ProcessEnv` so Worker-only TS
projects can consume the .d.ts
- No-arg / default env path falls back to `globalThis.process?.env ??
{}` via a private helper so Cloudflare Workers without nodejs_compat
don't ReferenceError at the no-arg call site
Configurable env var names (`apiKeyEnvVar`, `endpointEnvVar`) for
surfaces that namespace their secrets. Defaults match the de-facto
Supermemory conventions (SUPERMEMORY_API_KEY, SUPERMEMORY_ENDPOINT).
7 new tests: missing/blank API key, present API key constructs the
client, custom apiKeyEnvVar, fetch override threading, default endpoint,
SUPERMEMORY_ENDPOINT override, no-globalThis.process Worker simulation.
84/84 memory tests pass (77 existing + 7 new); build clean.
Sage adoption (PR #4 in the proactive-batch-runner sequence) replaces
its local `src/integrations/supermemory-client.ts` with this factory.
The local file may stay as a thin re-export alias for one minor cycle
to avoid breaking direct importers, then be deleted.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b01fb196a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "dependencies": { | ||
| "@agent-relay/memory": "^6.0.9" | ||
| "@agent-relay/memory": "^6.0.9", | ||
| "supermemory": "^4.21.1" |
There was a problem hiding this comment.
Regenerate lockfile after adding supermemory dependency
Adding "supermemory": "^4.21.1" to packages/memory/package.json without the corresponding package-lock.json update breaks reproducible installs and can fail CI on a clean checkout. I checked the repo workflows and both .github/workflows/ci.yml and .github/workflows/publish.yml run npm ci, which requires the lockfile to match package manifests; this commit introduces a new runtime dependency but leaves the lockfile out of sync.
Useful? React with 👍 / 👎.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR introduces a new Supermemory SDK integration to the ChangesSupermemory SDK Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
`npm ci` failed on PR #83 because adding `supermemory@^4.21.1` to packages/memory/package.json without refreshing package-lock.json puts the lockfile out of sync. Unlike PR #81 (no new deps) and PR #82 (internal workspace dep), this PR is the first in the series to add a real npm-registry dependency, so the lock must update. Diff is in two parts, both unavoidable: 1. New `node_modules/supermemory@4.21.1` entry — what this PR needs. 2. Workspace package versions sync from 0.4.23 → 0.4.25 — the recent release commits (2740310, 941a840) bumped the workspace package.json files but never refreshed the lockfile. `npm ci` refuses any out-of-sync state, so the sync has to land here even though it's incidental to this feature. Verified locally with the same install path CI runs: - rm -rf node_modules - npm install (regenerates lock) - cd packages/memory && npm test (84/84 pass) - npm run build (clean) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
createSupermemoryClient(options)factory in@agent-assistant/memorysupermemorySDK (v4.21.1) with the timeout / retry / fetch wiring sage shipped in PR #198EnvSourceshape (noNodeJS.ProcessEnvin public surface, noprocessaccess at the no-arg path)Why
Sage shipped this factory locally at
src/integrations/supermemory-client.tsin PR #198 to fix the prod SEARCH_ERROR outage. It centralises 4 call sites that previously had subtly different defaults for endpoint, timeout, retries, and fetch wiring. Moving it upstream so future AA consumers don't reimplement the same wrapper.Surface
Design notes
timeout: 8sis the per-call cap that fixed the 2026-05-04 SEARCH_ERROR outage;maxRetries: 2matches the SDK default but is surfaced explicitly so callers know 5xx retries are happening.globalThis.fetchclosure. The SDK capturesfetchat construction; without the closure,vi.stubGlobal('fetch', ...)set after construction is invisible. The SDK already callsfetch.call(undefined, url, init)internally to dodge the Cloudflare Workers "Illegal invocation" foot-gun (sdk-ts/src/client.ts:594), so passing the closure is purely for test ergonomics.readFailOpenFromEnvfix:EnvSourceinstead ofNodeJS.ProcessEnv, no-arg path readsglobalThis.process?.env ?? {}so Workers without nodejs_compat getnull(no API key) instead of aprocess is not definedReferenceError.apiKeyEnvVar/endpointEnvVarlets surfaces namespace their secrets (e.g.MY_APP_SUPERMEMORY_API_KEY). Defaults match the de-facto Supermemory conventions.nullon missing key. Callers no-op cleanly during local dev without a separate env-presence check. Sage's existing call sites already handle this branch.Test plan
cd packages/memory && npm test— 84/84 pass (77 existing + 7 new)apiKeyEnvVarhonoredfetchoverride threaded to SDKSUPERMEMORY_ENDPOINToverridedelete globalThis.processWorker simulation does not thrownpm run buildcleanNodeJS.ProcessEnv/node:processindist/supermemory-client.d.tspublic signatures (only docstring mentions explaining why)Out of scope (follow-up)
chore(release)commit per the existing conventionsrc/integrations/supermemory-client.tsbecomes a thin re-export alias for one minor compat cycle, then deletes; staged at sageworkflows/proactive-batch-runner/03-aa-memory.ts+04-sage-adopt.tsSequence status
boundedParallel)@agent-assistant/coordination@0.4.25runFollowUpBatch+ policy + counter)🤖 Generated with Claude Code