feat: add Amp (Sourcegraph) integration with auto-capture plugin#1049
feat: add Amp (Sourcegraph) integration with auto-capture plugin#1049zuwasi wants to merge 1 commit into
Conversation
Add native Amp support via two layers: 1. MCP connectivity — �gentmemory connect amp writes the canonical MCP block into �mp.mcpServers in ~/.config/amp/settings.json (or %APPDATA%/amp/settings.json on Windows). Amp uses the �mp.mcpServers wrapper key, not the standard mcpServers. 2. Native plugin — �gentmemory connect amp --with-hooks also copies integrations/amp/agentmemory.ts to ~/.config/amp/plugins/. The plugin hooks into Amp's session.start, agent.start, tool.call, tool.result, and agent.end events to automatically capture observations (prompts, tool calls, tool results, failures) and POST them to the agentmemory REST API. It also registers four memory tools (memory_recall, memory_save, memory_smart_search, memory_sessions) and three command palette commands (agentmemory-recall, agentmemory-remember, agentmemory-session-history). The adapter is Windows-safe and added to the Windows allowlist alongside copilot-cli. Amp's plugin API runs via Bun, so the plugin file uses fetch() (available globally in Bun) and Bun.spawnSync for git toplevel resolution — no Node-specific dependencies. Tests: 8 new tests in test/connect-amp.test.ts covering detect, install, already-wired, force, preserve-existing, dry-run, and adapter registration. Updated test/cli-connect.test.ts to include amp in the supported agent list (19 adapters).
|
@zuwasi 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 Amp support through a lifecycle plugin, MCP configuration adapter, CLI registration, tests, package publication, and installation documentation. ChangesAmp integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Amp
participant AgentmemoryPlugin
participant MemoryServer
Amp->>AgentmemoryPlugin: Send lifecycle and tool events
AgentmemoryPlugin->>MemoryServer: Post observations and session data
MemoryServer-->>AgentmemoryPlugin: Return context and memory results
AgentmemoryPlugin-->>Amp: Inject context and expose tool responses
Possibly related PRs
🚥 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: 5
🧹 Nitpick comments (2)
integrations/amp/agentmemory.ts (1)
16-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate
postintopostJsonto remove duplication.
postandpostJsonduplicate the same fetch/timeout/error-handling logic;postonly differs by discarding the result.♻️ Proposed consolidation
-async function post( - path: string, - body: Record<string, unknown>, - timeoutMs = 5000, -): Promise<void> { - try { - await fetch(`${API_URL}/agentmemory${path}`, { - method: 'POST', - headers: authHeaders(), - body: JSON.stringify(body), - signal: AbortSignal.timeout(timeoutMs), - }) - } catch (e) { - if (DEBUG) logger?.log(`POST ${path} failed: ${(e as Error).message}`) - } -} - async function postJson( path: string, body: Record<string, unknown>, timeoutMs = 5000, ): Promise<Record<string, unknown> | null> { try { const res = await fetch(`${API_URL}/agentmemory${path}`, { method: 'POST', headers: authHeaders(), body: JSON.stringify(body), signal: AbortSignal.timeout(timeoutMs), }) return res.ok ? ((await res.json()) as Record<string, unknown>) : null } catch (e) { if (DEBUG) logger?.log(`POST ${path} failed: ${(e as Error).message}`) return null } } + +async function post( + path: string, + body: Record<string, unknown>, + timeoutMs = 5000, +): Promise<void> { + await postJson(path, body, timeoutMs) +}🤖 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 `@integrations/amp/agentmemory.ts` around lines 16 - 50, Remove the duplicate fetch implementation from post and consolidate its behavior into postJson, reusing postJson for requests where the response body is discarded. Preserve the existing POST options, timeout handling, and DEBUG error logging, while keeping post’s Promise<void> contract for callers.src/cli/connect/amp.ts (1)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueA few comments restate what the code already says.
E.g.
// Amp uses "amp.mcpServers" as the wrapper key (not "mcpServers").right aboveWRAPPER_KEY, and// Install the plugin file when --with-hooks is passedaboveif (opts.withHooks). As per coding guidelines,src/**/*.tsshould prefer clear naming over comments explaining what code does.Also applies to: 40-41, 115-115, 135-135
🤖 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/connect/amp.ts` around lines 16 - 24, Remove comments in the Amp CLI module that merely restate clear code behavior, including the wrapper-key note above WRAPPER_KEY and the installation note above the opts.withHooks condition. Preserve comments that provide necessary external context, such as Amp configuration paths or non-obvious compatibility details.Source: Coding guidelines
🤖 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 `@integrations/amp/agentmemory.ts`:
- Around line 52-67: Cache the result of resolveProject() so git resolution is
performed once per process (or once per working directory) rather than on every
observation. Update resolveProject and its surrounding state to return the
cached project name while preserving AGENTMEMORY_PROJECT_NAME precedence and the
existing cwd fallback behavior; ensure observe’s agent.start, tool.result, and
tool.call paths reuse that cached value.
- Around line 205-215: Update the agent.end handler to await both post calls for
/summarize and /session/end before deleting the session from contextCache and
returning. Preserve their existing payloads and timeout values.
In `@src/cli/connect/amp.ts`:
- Around line 57-69: The findPluginSource function derives its starting path
incorrectly from import.meta.url, causing plugin lookup to fail on macOS/Linux.
Replace the manual URL pathname conversion with fileURLToPath(import.meta.url),
and add a withHooks: true test case covering successful plugin installation.
- Around line 107-113: Move the backupFile and logBackup calls in the connect
flow behind the condition that determines whether writeJsonAtomic will modify
the configuration. Preserve directory creation for missing CONFIG_PATH, and
ensure already-wired --with-hooks runs skip both the backup and write
operations.
In `@test/connect-amp.test.ts`:
- Around line 127-134: The AMP adapter tests lack coverage for the withHooks
installation path. Add a test near the existing dry-run case that invokes
adapter.install with withHooks: true and verifies the plugin file is copied to
the PLUGIN_FILE destination, exercising findPluginSource and copyFileSync.
---
Nitpick comments:
In `@integrations/amp/agentmemory.ts`:
- Around line 16-50: Remove the duplicate fetch implementation from post and
consolidate its behavior into postJson, reusing postJson for requests where the
response body is discarded. Preserve the existing POST options, timeout
handling, and DEBUG error logging, while keeping post’s Promise<void> contract
for callers.
In `@src/cli/connect/amp.ts`:
- Around line 16-24: Remove comments in the Amp CLI module that merely restate
clear code behavior, including the wrapper-key note above WRAPPER_KEY and the
installation note above the opts.withHooks condition. Preserve comments that
provide necessary external context, such as Amp configuration paths or
non-obvious compatibility details.
🪄 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: 2ae3d533-061f-4baa-bf3d-2f09f341fe06
📒 Files selected for processing (8)
README.mdintegrations/amp/README.mdintegrations/amp/agentmemory.tspackage.jsonsrc/cli/connect/amp.tssrc/cli/connect/index.tstest/cli-connect.test.tstest/connect-amp.test.ts
| // --- Project resolution ------------------------------------------- | ||
| function resolveProject(): string { | ||
| const explicit = process.env.AGENTMEMORY_PROJECT_NAME | ||
| if (explicit && explicit.trim()) return explicit.trim() | ||
| const cwd = process.cwd() | ||
| try { | ||
| const top = Bun.spawnSync(['git', 'rev-parse', '--show-toplevel'], { | ||
| cwd, | ||
| stdout: 'pipe', | ||
| stderr: 'ignore', | ||
| timeout: 500, | ||
| }).stdout?.toString().trim() | ||
| if (top) return top.split(/[/\\]/).pop()! | ||
| } catch {} | ||
| return cwd.split(/[/\\]/).pop()! | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Cache resolveProject() instead of spawning git synchronously on every event.
resolveProject() runs a blocking Bun.spawnSync on every call, and observe() calls it on every agent.start and every single tool.result/tool.call observation — i.e. potentially dozens of times per session. Amp's own docs note that plugins are long-lived processes that may run for multiple threads concurrently, so a synchronous subprocess spawn in one thread's handler blocks the whole plugin's event loop for every other concurrently-running thread too.
The resolved project name is stable for the process lifetime (or at minimum per cwd); cache it instead of re-spawning git each time.
⚡ Proposed caching fix
+const projectCache = new Map<string, string>()
+
function resolveProject(): string {
const explicit = process.env.AGENTMEMORY_PROJECT_NAME
if (explicit && explicit.trim()) return explicit.trim()
const cwd = process.cwd()
+ const cached = projectCache.get(cwd)
+ if (cached) return cached
try {
const top = Bun.spawnSync(['git', 'rev-parse', '--show-toplevel'], {
cwd,
stdout: 'pipe',
stderr: 'ignore',
timeout: 500,
}).stdout?.toString().trim()
- if (top) return top.split(/[/\\]/).pop()!
+ if (top) {
+ const name = top.split(/[/\\]/).pop()!
+ projectCache.set(cwd, name)
+ return name
+ }
} catch {}
- return cwd.split(/[/\\]/).pop()!
+ const fallback = cwd.split(/[/\\]/).pop()!
+ projectCache.set(cwd, fallback)
+ return fallback
}Also applies to: 70-83
🤖 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 `@integrations/amp/agentmemory.ts` around lines 52 - 67, Cache the result of
resolveProject() so git resolution is performed once per process (or once per
working directory) rather than on every observation. Update resolveProject and
its surrounding state to return the cached project name while preserving
AGENTMEMORY_PROJECT_NAME precedence and the existing cwd fallback behavior;
ensure observe’s agent.start, tool.result, and tool.call paths reuse that cached
value.
| amp.on('agent.end', async (event) => { | ||
| const sessionId = event.thread.id | ||
| const toolCalls = amp.helpers.toolCallsInMessages(event.messages) | ||
| await observe(sessionId, 'agent_end', { | ||
| status: event.status, | ||
| tool_call_count: toolCalls.length, | ||
| }) | ||
| post('/summarize', { sessionId }, 120000) | ||
| post('/session/end', { sessionId }, 5000) | ||
| contextCache.delete(sessionId) | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the Amp (Sourcegraph) plugin API guarantee pending promises/fetches complete after an agent.end hook handler returns?
💡 Result:
No, the Amp (Sourcegraph) plugin API does not guarantee that pending promises or asynchronous fetches complete after an agent.end hook handler returns [1]. The agent.end hook is treated as a fire-and-forget event [1]. In the context of Amp's plugin architecture, lifecycle handlers—including agent.end—are typically invoked without an orchestration mechanism that waits for the handler's internal asynchronous operations to resolve before proceeding [1]. Developers implementing these hooks should assume that any asynchronous tasks, such as network requests (fetches) or other promise-based logic, will continue to run in the background after the handler returns, or may be terminated if the runtime environment ceases execution of the plugin process [1]. For scenarios requiring reliable execution of asynchronous tasks, such as reporting telemetry or state synchronization, developers should handle the lifecycle of those operations independently, as the plugin API does not provide a mechanism to await the completion of tasks triggered within the agent.end hook [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and locate the post helper / agent.end hook.
git ls-files 'integrations/amp/*' 'integrations/**' | sed -n '1,200p'
printf '\n--- outline agentmemory.ts ---\n'
ast-grep outline integrations/amp/agentmemory.ts --view expanded || true
printf '\n--- search for post definition/usages ---\n'
rg -n "function post|const post|post\\(" integrations/amp -g '!**/dist/**' -g '!**/build/**'
printf '\n--- relevant snippet around agent.end ---\n'
sed -n '180,235p' integrations/amp/agentmemory.tsRepository: rohitg00/agentmemory
Length of output: 3826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,70p' integrations/amp/agentmemory.tsRepository: rohitg00/agentmemory
Length of output: 2366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('integrations/amp/agentmemory.ts')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 70:
print(f"{i:4d}: {line}")
PYRepository: rohitg00/agentmemory
Length of output: 2786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('integrations/amp/agentmemory.ts').read_text().splitlines()
for i in range(1, 71):
print(f"{i:4d}: {text[i-1]}")
PYRepository: rohitg00/agentmemory
Length of output: 2786
Await the final session writes. agent.end should await /summarize and /session/end; otherwise the hook can return before those last writes complete, and they may be dropped during teardown.
🤖 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 `@integrations/amp/agentmemory.ts` around lines 205 - 215, Update the agent.end
handler to await both post calls for /summarize and /session/end before deleting
the session from contextCache and returning. Preserve their existing payloads
and timeout values.
| function findPluginSource(): string { | ||
| let dir = dirname(new URL(import.meta.url).pathname.replace(/^\//, "")); | ||
| for (let i = 0; i < 15; i++) { | ||
| const candidate = join(dir, "integrations", "amp", "agentmemory.ts"); | ||
| if (existsSync(candidate)) return candidate; | ||
| const parent = dirname(dir); | ||
| if (parent === dir) break; | ||
| dir = parent; | ||
| } | ||
| throw new Error( | ||
| "Could not locate integrations/amp/agentmemory.ts — is the package installed correctly?", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/cli/connect/amp.ts ==\n'
nl -ba src/cli/connect/amp.ts | sed -n '1,220p'
printf '\n== test/connect-amp.test.ts ==\n'
nl -ba test/connect-amp.test.ts | sed -n '1,260p'
printf '\n== search for withHooks / findPluginSource ==\n'
rg -n "withHooks|findPluginSource|agentmemory\.ts|install\(" src testRepository: rohitg00/agentmemory
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/cli/connect/amp.ts ==\n'
cat -n src/cli/connect/amp.ts | sed -n '1,220p'
printf '\n== test/connect-amp.test.ts ==\n'
cat -n test/connect-amp.test.ts | sed -n '1,260p'
printf '\n== search for withHooks / findPluginSource ==\n'
rg -n "withHooks|findPluginSource|agentmemory\.ts|install\(" src testRepository: rohitg00/agentmemory
Length of output: 22491
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const { dirname, join } = require('node:path');
const samples = [
'file:///home/user/pkg/dist/cli/connect/amp.js',
'file:///C:/Users/user/pkg/dist/cli/connect/amp.js',
];
for (const url of samples) {
const pathname = new URL(url).pathname;
const stripped = pathname.replace(/^\//, '');
console.log('\nURL:', url);
console.log('pathname:', pathname);
console.log('stripped:', stripped);
console.log('dirname(stripped):', dirname(stripped));
console.log('candidate:', join(dirname(stripped), 'integrations', 'amp', 'agentmemory.ts'));
}
JSRepository: rohitg00/agentmemory
Length of output: 724
Use fileURLToPath(import.meta.url) for plugin lookup new URL(import.meta.url).pathname.replace(/^\//, "") turns POSIX absolute paths into relative ones, so findPluginSource() walks from process.cwd() and --with-hooks silently skips plugin installation on macOS/Linux. Add a withHooks: true test case here too.
🤖 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/connect/amp.ts` around lines 57 - 69, The findPluginSource function
derives its starting path incorrectly from import.meta.url, causing plugin
lookup to fail on macOS/Linux. Replace the manual URL pathname conversion with
fileURLToPath(import.meta.url), and add a withHooks: true test case covering
successful plugin installation.
| let backupPath: string | undefined; | ||
| if (existsSync(CONFIG_PATH)) { | ||
| backupPath = backupFile(CONFIG_PATH, this.name); | ||
| logBackup(backupPath); | ||
| } else { | ||
| mkdirSync(dirname(CONFIG_PATH), { recursive: true }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect backupFile() to see if it creates a new timestamped file each call
rg -n -A 15 'export function backupFile' src/cli/connect/util.tsRepository: rohitg00/agentmemory
Length of output: 648
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding logic in src/cli/connect/amp.ts
sed -n '70,150p' src/cli/connect/amp.ts
# Inspect backup helper context
sed -n '1,120p' src/cli/connect/util.tsRepository: rohitg00/agentmemory
Length of output: 6362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '70,150p' src/cli/connect/amp.ts
printf '\n---\n'
sed -n '1,120p' src/cli/connect/util.tsRepository: rohitg00/agentmemory
Length of output: 6367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the control flow around the backup/write decision in src/cli/connect/amp.ts
nl -ba src/cli/connect/amp.ts | sed -n '80,145p'Repository: rohitg00/agentmemory
Length of output: 198
Skip the backup on no-op runs. When --with-hooks is re-run against an already-wired config, backupFile() still creates a new timestamped copy even though writeJsonAtomic() is skipped. Move the backup behind the write condition so unchanged installs don’t accumulate backups.
🤖 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/connect/amp.ts` around lines 107 - 113, Move the backupFile and
logBackup calls in the connect flow behind the condition that determines whether
writeJsonAtomic will modify the configuration. Preserve directory creation for
missing CONFIG_PATH, and ensure already-wired --with-hooks runs skip both the
backup and write operations.
| it("dry-run does not write any file", async () => { | ||
| const configDir = ampConfigDir(home); | ||
| mkdirSync(configDir, { recursive: true }); | ||
| const { adapter } = await import("../src/cli/connect/amp.js"); | ||
| const result = await adapter.install({ dryRun: true, force: false }); | ||
| expect(result.kind).toBe("installed"); | ||
| expect(existsSync(join(configDir, "settings.json"))).toBe(false); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add coverage for withHooks: true (plugin file install path).
No test exercises adapter.install({ ..., withHooks: true }), so the plugin-copy path in findPluginSource()/copyFileSync is completely untested — this is how the POSIX path-resolution bug flagged in src/cli/connect/amp.ts (lines 57-69) went unnoticed.
Want me to draft a test that runs install({ withHooks: true }) and asserts the plugin file gets copied to PLUGIN_FILE?
🤖 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/connect-amp.test.ts` around lines 127 - 134, The AMP adapter tests lack
coverage for the withHooks installation path. Add a test near the existing
dry-run case that invokes adapter.install with withHooks: true and verifies the
plugin file is copied to the PLUGIN_FILE destination, exercising
findPluginSource and copyFileSync.
1 participant
Summary
Adds native Amp (Sourcegraph's coding agent) support to agentmemory, following the same two-layer pattern as the existing Claude Code and Codex integrations.
What's included
1. MCP connectivity (\�gentmemory connect amp)
Writes the canonical MCP block (
px -y @agentmemory/mcp\ + env passthrough) into \�mp.mcpServers\ in the Amp settings file:
Amp uses the \�mp.mcpServers\ wrapper key (not the standard \mcpServers), so a dedicated adapter is needed rather than the shared \json-mcp-adapter.
2. Native plugin (\�gentmemory connect amp --with-hooks)
Also copies \integrations/amp/agentmemory.ts\ to ~/.config/amp/plugins/agentmemory.ts. The plugin uses Amp's Plugin API to hook into agent lifecycle events:
The plugin also registers:
Files changed
Design notes
eadJsonSafe, \writeJsonAtomic\ utilities from \util.ts.
Test results
\
test/connect-amp.test.ts: 8 passed
test/cli-connect.test.ts: 25 passed (all existing + updated agent list)
\\
All new tests pass. No existing tests broken.
Usage
\\�ash
1. Start the memory server
npx @agentmemory/agentmemory
2. Wire MCP + install the auto-capture plugin
agentmemory connect amp --with-hooks
3. Restart Amp (or run plugins: reload from the command palette)
\\
Summary by CodeRabbit
New Features
connect ampsetup for configuring Amp’s MCP connection and optional plugin installation.Documentation