feat: pi-mem — persistent memory extension for pi-mono agents#1786
feat: pi-mem — persistent memory extension for pi-mono agents#1786ArtemisAI wants to merge 4 commits intothedotmack:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new top-level Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as pi-agent Runtime
participant Ext as pi-mem Extension
participant Claude as claude-mem Worker
participant LLM as LLM
Agent->>Ext: session_start (user prompt)
Ext->>Claude: POST /api/sessions/init
Claude-->>Ext: session initialized (contentSessionId)
Agent->>Ext: context (before LLM call)
Ext->>Claude: GET /api/context/inject?project=...
Claude-->>Ext: injected context
Ext->>LLM: inject <pi-mem-context>
Agent->>LLM: LLM call
LLM-->>Agent: tool calls
Agent->>Ext: tool_result
Ext->>Claude: POST /api/sessions/observations (async, skip memory_recall)
Agent->>Ext: agent_end
Ext->>Claude: POST /api/sessions/summarize
Claude-->>Ext: ack
Ext->>Claude: POST /api/sessions/complete (delayed)
Claude-->>Ext: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Adds a new pi-agent/ integration that adapts pi-mono agent lifecycle events into claude-mem worker HTTP calls, enabling cross-session memory capture, context injection, and explicit memory search for pi-based agents. It also introduces a new platform source label (pi-agent) for tagging and UI/source sorting.
Changes:
- Add
pi-agent/src/pi-mem.tsextension implementing session init, context injection, observation capture, summarization, amemory_recalltool, and/memory-statuscommand. - Add pi-agent docs/skill guidance (
pi-agent/README.md,pi-agent/skills/mem-search/SKILL.md). - Register
pi-agentas a platform source and includepi-agent/in the npm packagefileslist.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/platform-source.ts | Adds normalization/sorting support for the new pi-agent platform source. |
| pi-agent/src/pi-mem.ts | Implements the pi-mono extension adapter + memory_recall tool and /memory-status command. |
| pi-agent/skills/mem-search/SKILL.md | Adds user-facing guidance on using the memory search tool. |
| pi-agent/README.md | Documents architecture, installation, configuration, and cross-engine behavior. |
| package.json | Publishes the pi-agent/ directory in the npm package. |
| // Await summarize so the worker receives it before complete | ||
| await workerPost("/api/sessions/summarize", { | ||
| contentSessionId, | ||
| last_assistant_message: lastAssistantMessage, | ||
| }); | ||
|
|
||
| // Delay completion to let in-flight observations arrive | ||
| const sid = contentSessionId; | ||
| setTimeout(() => { | ||
| workerPostFireAndForget("/api/sessions/complete", { | ||
| contentSessionId: sid, | ||
| }); | ||
| }, SESSION_COMPLETE_DELAY_MS); |
There was a problem hiding this comment.
The worker expects platformSource (camelCase) on POST /api/sessions/summarize and POST /api/sessions/complete to correctly associate the request with the session’s platform source. Consider including it in both calls to avoid defaulting to claude when the session row was created without the right platform source.
pi-agent/README.md
Outdated
| | Cursor | `cursor` | `my-app` | | ||
| | Codex | `codex` | `my-app` | | ||
|
|
||
| Context injection queries return observations from **all engines** for the same project by default. Pass `platform_source` to filter. |
There was a problem hiding this comment.
This README says to pass platform_source to filter context injection, but the worker HTTP API expects platformSource (camelCase) on /api/context/inject query params. Update the docs to use the correct parameter name so filtering works.
| Context injection queries return observations from **all engines** for the same project by default. Pass `platform_source` to filter. | |
| Context injection queries return observations from **all engines** for the same project by default. Pass `platformSource` to filter. |
src/shared/platform-source.ts
Outdated
| if (source.includes('codex')) return 'codex'; | ||
| if (source.includes('cursor')) return 'cursor'; | ||
| if (source.includes('claude')) return 'claude'; | ||
| if (source.includes('pi')) return 'pi-agent'; |
There was a problem hiding this comment.
normalizePlatformSource() maps any source containing "pi" to "pi-agent". This will incorrectly remap unrelated sources like "api" (contains "pi") to "pi-agent" and can misclassify future platform strings. Narrow the match (e.g., exact match for "pi-agent", prefix match like "pi-", or includes("pi-agent") / includes("pi-mono")).
| if (source.includes('pi')) return 'pi-agent'; | |
| if ( | |
| source === 'pi-agent' || | |
| source.startsWith('pi-') || | |
| source.includes('pi-agent') || | |
| source.includes('pi-mono') | |
| ) { | |
| return 'pi-agent'; | |
| } |
pi-agent/extensions/pi-mem.ts
Outdated
| contentSessionId, | ||
| project: projectName, | ||
| prompt: event.prompt || "pi-agent session", | ||
| platform_source: PLATFORM_SOURCE, |
There was a problem hiding this comment.
The worker HTTP API expects platformSource (camelCase) in POST bodies (e.g., POST /api/sessions/init normalizes req.body.platformSource). Sending platform_source here will be ignored and sessions will be tagged with the default platform source instead of pi-agent. Rename this field to platformSource for init (and keep it consistent across other calls).
| platform_source: PLATFORM_SOURCE, | |
| platformSource: PLATFORM_SOURCE, |
| workerPostFireAndForget("/api/sessions/observations", { | ||
| contentSessionId, | ||
| tool_name: toolName, | ||
| tool_input: event.input || {}, | ||
| tool_response: toolResponseText, | ||
| cwd: sessionCwd, | ||
| }); |
There was a problem hiding this comment.
workerPostFireAndForget("/api/sessions/observations", ...) does not include platformSource. The worker uses req.body.platformSource to correctly tag/lookup the session; without it, observations may be stored under the default platform source (especially if init also omitted/misnamed it). Include platformSource: PLATFORM_SOURCE to keep tagging consistent.
pi-agent/extensions/pi-mem.ts
Outdated
| // Configuration | ||
| // ============================================================================= | ||
|
|
||
| const WORKER_PORT = parseInt(process.env.CLAUDE_MEM_PORT || "37777", 10); |
There was a problem hiding this comment.
WORKER_PORT uses parseInt(...) without validating the result. If CLAUDE_MEM_PORT is set to a non-numeric value, WORKER_PORT becomes NaN and all requests will fail with a confusing URL. Fall back to the default when Number.isFinite(WORKER_PORT) is false (or validate and throw a clearer error).
| const WORKER_PORT = parseInt(process.env.CLAUDE_MEM_PORT || "37777", 10); | |
| const DEFAULT_WORKER_PORT = 37777; | |
| const rawWorkerPort = process.env.CLAUDE_MEM_PORT; | |
| const parsedWorkerPort = rawWorkerPort ? parseInt(rawWorkerPort, 10) : DEFAULT_WORKER_PORT; | |
| const WORKER_PORT = Number.isFinite(parsedWorkerPort) ? parsedWorkerPort : DEFAULT_WORKER_PORT; |
pi-agent/src/pi-mem.ts
Outdated
| * is a proven integration pattern for pi-mono-based runtimes. | ||
| * | ||
| * Install: | ||
| * cp pi-mem.ts ~/.pi/agent/extensions/pi-mem.ts |
There was a problem hiding this comment.
The header comment’s install command (cp pi-mem.ts ...) doesn’t match the actual location of this file in the repo (pi-agent/src/pi-mem.ts). Update the instruction so a reader copying from the repository uses the correct source path.
| * cp pi-mem.ts ~/.pi/agent/extensions/pi-mem.ts | |
| * cp pi-agent/src/pi-mem.ts ~/.pi/agent/extensions/pi-mem.ts |
pi-agent/src/pi-mem.ts
Outdated
|
|
||
| const projects = encodeURIComponent(projectName); | ||
| const contextText = await workerGetText( | ||
| `/api/context/inject?projects=${projects}&platform_source=${PLATFORM_SOURCE}`, |
There was a problem hiding this comment.
The context inject endpoint reads the query param platformSource (camelCase), not platform_source. As written, this filter will be ignored by the worker. Update the query string parameter name (and consider whether you want to filter at all, since filtering prevents cross-engine recall).
| `/api/context/inject?projects=${projects}&platform_source=${PLATFORM_SOURCE}`, | |
| `/api/context/inject?projects=${projects}&platformSource=${PLATFORM_SOURCE}`, |
pi-agent/extensions/pi-mem.ts
Outdated
| // Inject as a system message in the conversation | ||
| return { | ||
| messages: [ | ||
| ...event.messages, | ||
| { | ||
| role: "user" as const, | ||
| content: [ | ||
| { | ||
| type: "text" as const, | ||
| text: `<pi-mem-context>\n${contextText}\n</pi-mem-context>`, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Comment says this is injected as a system message, but the code sets role: "user". If pi-mono supports system, using user can change model behavior (treating memory as user intent rather than background context). Align the role with the intended semantics (or update the comment if system is not supported).
pi-agent/src/pi-mem.ts
Outdated
| const project = encodeURIComponent(projectName); | ||
|
|
||
| const result = await workerGetText( | ||
| `/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, |
There was a problem hiding this comment.
The worker’s unified search endpoint expects query=... (not q=...). With q, the worker treats this as a filter-only request and will ignore the user’s search term, returning recent items instead of matching memories. Use the correct query parameter name.
| `/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, | |
| `/api/search?query=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pi-agent/README.md`:
- Around line 24-25: The README's init flow overstates what happens at
session_start; update the diagram/table so session_start reflects the local-only
behavior implemented in pi-agent/src/pi-mem.ts (it only sets local state and
appends the session marker) and show that the POST /api/sessions/init call
actually occurs during before_agent_start (when a prompt is provided). Adjust
the entries referenced (lines showing session_start and before_agent_start, and
the similar lines at 47-48) to accurately describe these handlers and their
effects.
- Around line 20-39: The fenced architecture diagram block in README.md is
missing a language tag which triggers markdownlint MD040; update the
triple-backtick fence surrounding the "Pi-Agent (pi-coding-agent / OpenClaw /
custom) ..." diagram to include a language tag (e.g., change ``` to ```text) so
the block is explicitly marked as text and the MD040 warning is resolved.
In `@pi-agent/skills/mem-search/SKILL.md`:
- Around line 23-25: The fenced code block in SKILL.md containing
memory_recall(...) lacks a language tag and triggers markdownlint MD040; update
the triple-backtick fence to include a language label such as text (e.g.,
```text) so the example is properly tagged and the linter stops flagging it;
locate the fenced block with the memory_recall(query="authentication middleware
refactor", limit=10) snippet and add the language tag to the opening backticks.
In `@pi-agent/src/pi-mem.ts`:
- Around line 327-334: Clamp params.limit to a safe range before building the
search URL: in the execute method, replace the current raw assignment of limit
with a clamped integer (e.g., Math.max(1, Math.min(params.limit||5, 20))) so any
caller-provided limit is bounded to a sane range; then use that clamped value
when constructing the `/api/search` query passed to workerGetText (referencing
execute, params.limit, workerGetText, and the `/api/search` call).
- Around line 41-58: The worker HTTP calls (e.g., workerPost) currently await
fetch with no timeout and can hang indefinitely; add an AbortController-based
timeout to each fetch site: create an AbortController, start a timer (use a
configurable constant like WORKER_FETCH_TIMEOUT_MS or env var) that calls
controller.abort() after the timeout, pass controller.signal to fetch, clear the
timer when fetch completes (in finally), and handle AbortError in the catch to
log a timeout message and return null; apply the same pattern to the other
identical fetch calls in this file.
- Around line 171-174: The code is forcing platform-specific filtering by
appending &platform_source=${PLATFORM_SOURCE} to the workerGetText query
strings; remove the platform_source query parameter from both the context
injection call that sets contextText (the workerGetText call using
`/api/context/inject?projects=${projects}`) and the search call that assigns
result (the workerGetText call using
`/api/search?q=${query}&limit=${limit}&project=${project}`) so that
ObservationCompiler can return observations across all platform sources; keep
existing URL encoding (encodeURIComponent(projectName)) and other query params
intact.
In `@src/shared/platform-source.ts`:
- Line 17: normalizePlatformSource currently uses a broad substring check
(source.includes('pi')) which misclassifies strings like "pipeline" or
"api-client"; update normalizePlatformSource to match only explicit known
pi-agent identifiers (e.g., compare against a whitelist/set such as ['pi',
'pi-agent', 'pi_agent', 'piagent'] or use a regex anchored to whole-string or
known suffixes/prefixes) and replace the includes check with an exact membership
test (or anchored regex) so only those exact identifiers map to 'pi-agent'.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28400e94-b89b-4f98-b73f-9c57f6309b4a
📒 Files selected for processing (5)
package.jsonpi-agent/README.mdpi-agent/skills/mem-search/SKILL.mdpi-agent/src/pi-mem.tssrc/shared/platform-source.ts
pi-agent/src/pi-mem.ts
Outdated
| const projects = encodeURIComponent(projectName); | ||
| const contextText = await workerGetText( | ||
| `/api/context/inject?projects=${projects}&platform_source=${PLATFORM_SOURCE}`, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- context injection route ---'
rg -n -C4 'context/inject|platform_source' --type=ts --type=js src tests openclaw pi-agent
echo
echo '--- search route ---'
rg -n -C4 '/api/search|platform_source' --type=ts --type=js src tests openclaw pi-agentRepository: thedotmack/claude-mem
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the handleContextInject implementation
echo "=== handleContextInject method ==="
sed -n '200,260p' src/services/worker/http/routes/SearchRoutes.ts
echo
echo "=== Check if getObservations is called with platformSource ==="
rg -n -A5 'handleContextInject|getObservations' src/services/worker/http/routes/SearchRoutes.ts | head -30Repository: thedotmack/claude-mem
Length of output: 3248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the context-generator to see how platform_source is used
echo "=== context-generator usage of platform_source ==="
fd -name "context-generator*" -type f | head -5
echo
echo "=== Search for platform_source in context-generator ==="
rg -n "platform_source" src/ --type=ts --type=js | grep -i "context-generator\|generateContext" | head -20Repository: thedotmack/claude-mem
Length of output: 363
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find context-generator files
echo "=== context-generator files ==="
find . -name "*context-generator*" -type f 2>/dev/null | grep -v node_modules | head -10
echo
echo "=== Check if context-generator.cjs or .ts exists ==="
ls -la src/ | grep -i context
ls -la src/**/*context* 2>/dev/null | head -20Repository: thedotmack/claude-mem
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the context-generator.ts implementation
echo "=== context-generator.ts (first 100 lines) ==="
head -100 src/services/context-generator.ts
echo
echo "=== Search for platform_source usage in context-generator.ts ==="
rg -n "platform_source" src/services/context-generator.ts -A3 -B3Repository: thedotmack/claude-mem
Length of output: 1014
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the actual context implementation
echo "=== context/index.ts ==="
head -50 src/services/context/index.ts
echo
echo "=== Look for ContextBuilder and platform_source usage ==="
rg -n "platform_source" src/services/context/ -A2 -B2 | head -60Repository: thedotmack/claude-mem
Length of output: 4393
🏁 Script executed:
#!/bin/bash
set -euo progfail
# Check ContextBuilder to see if platform_source is always applied or conditional
echo "=== ContextBuilder.ts (first 80 lines) ==="
head -80 src/services/context/ContextBuilder.ts
echo
echo "=== Check ContextBuilder for platform_source handling ==="
rg -n "platform_source" src/services/context/ContextBuilder.ts -A2 -B2 | head -40Repository: thedotmack/claude-mem
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check ContextBuilder to see if platform_source is always applied or conditional
echo "=== ContextBuilder.ts (first 80 lines) ==="
head -80 src/services/context/ContextBuilder.ts
echo
echo "=== Check ContextBuilder for platform_source handling ==="
rg -n "platform_source" src/services/context/ContextBuilder.ts -A2 -B2 | head -40Repository: thedotmack/claude-mem
Length of output: 2929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how platformSource is used in ContextBuilder
echo "=== ContextBuilder.ts - platformSource usage ==="
rg -n "platformSource\|platform_source" src/services/context/ContextBuilder.ts -A3 -B3
echo
echo "=== Check queryObservations call signature ==="
rg -n "queryObservations|querySummaries" src/services/context/ContextBuilder.ts | head -20Repository: thedotmack/claude-mem
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look at the full ContextBuilder to see how queries are called
echo "=== ContextBuilder.ts lines 130-200 ==="
sed -n '130,200p' src/services/context/ContextBuilder.ts
echo
echo "=== ContextBuilder.ts lines 200-270 ==="
sed -n '200,270p' src/services/context/ContextBuilder.tsRepository: thedotmack/claude-mem
Length of output: 1569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the queryObservations signature to see the full picture
echo "=== ObservationCompiler.ts - queryObservations signature and implementation ==="
sed -n '1,100p' src/services/context/ObservationCompiler.ts | tail -80Repository: thedotmack/claude-mem
Length of output: 2354
Remove platform_source=pi-agent from query strings to enable cross-engine memory sharing.
Both context injection (line 173) and search (line 333) hard-code platform_source=pi-agent. The worker's ObservationCompiler treats this as a SQL WHERE filter (COALESCE(s.platform_source, 'claude') = ?), which excludes observations from OpenClaw and Claude Code—contradicting the documented default behavior of same-project sharing across all engines.
Affected lines
171-174:
const projects = encodeURIComponent(projectName);
const contextText = await workerGetText(
`/api/context/inject?projects=${projects}&platform_source=${PLATFORM_SOURCE}`,
);
332-334:
const result = await workerGetText(
`/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`,
);
Remove &platform_source=${PLATFORM_SOURCE} from both calls to allow the worker to return observations from all platform sources within the same project.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pi-agent/src/pi-mem.ts` around lines 171 - 174, The code is forcing
platform-specific filtering by appending &platform_source=${PLATFORM_SOURCE} to
the workerGetText query strings; remove the platform_source query parameter from
both the context injection call that sets contextText (the workerGetText call
using `/api/context/inject?projects=${projects}`) and the search call that
assigns result (the workerGetText call using
`/api/search?q=${query}&limit=${limit}&project=${project}`) so that
ObservationCompiler can return observations across all platform sources; keep
existing URL encoding (encodeURIComponent(projectName)) and other query params
intact.
pi-agent/src/pi-mem.ts
Outdated
| async execute(_toolCallId, params) { | ||
| const query = encodeURIComponent(String(params.query)); | ||
| const limit = typeof params.limit === "number" ? params.limit : 5; | ||
| const project = encodeURIComponent(projectName); | ||
|
|
||
| const result = await workerGetText( | ||
| `/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, | ||
| ); |
There was a problem hiding this comment.
Clamp limit before calling /api/search.
This tool is LLM-invoked, so an accidental limit=1000 turns one recall into a large search/result payload with no guardrail. Bound it to a sane integer range before building the URL.
⚙️ Proposed fix
async execute(_toolCallId, params) {
const query = encodeURIComponent(String(params.query));
- const limit = typeof params.limit === "number" ? params.limit : 5;
+ const rawLimit = typeof params.limit === "number" ? params.limit : 5;
+ const limit = Math.max(1, Math.min(10, Math.floor(rawLimit)));
const project = encodeURIComponent(projectName);
const result = await workerGetText(📝 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.
| async execute(_toolCallId, params) { | |
| const query = encodeURIComponent(String(params.query)); | |
| const limit = typeof params.limit === "number" ? params.limit : 5; | |
| const project = encodeURIComponent(projectName); | |
| const result = await workerGetText( | |
| `/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, | |
| ); | |
| async execute(_toolCallId, params) { | |
| const query = encodeURIComponent(String(params.query)); | |
| const rawLimit = typeof params.limit === "number" ? params.limit : 5; | |
| const limit = Math.max(1, Math.min(10, Math.floor(rawLimit))); | |
| const project = encodeURIComponent(projectName); | |
| const result = await workerGetText( | |
| `/api/search?q=${query}&limit=${limit}&project=${project}&platform_source=${PLATFORM_SOURCE}`, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pi-agent/src/pi-mem.ts` around lines 327 - 334, Clamp params.limit to a safe
range before building the search URL: in the execute method, replace the current
raw assignment of limit with a clamped integer (e.g., Math.max(1,
Math.min(params.limit||5, 20))) so any caller-provided limit is bounded to a
sane range; then use that clamped value when constructing the `/api/search`
query passed to workerGetText (referencing execute, params.limit, workerGetText,
and the `/api/search` call).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pi-agent/extensions/pi-mem.ts (1)
250-253: Consider adding a truncation indicator.When truncating
toolResponseText, the stored observation won't indicate that content was cut off. Adding a marker would help when reviewing stored observations.💡 Proposed enhancement
// Truncate to prevent oversized payloads if (toolResponseText.length > MAX_TOOL_RESPONSE_LENGTH) { - toolResponseText = toolResponseText.slice(0, MAX_TOOL_RESPONSE_LENGTH); + toolResponseText = toolResponseText.slice(0, MAX_TOOL_RESPONSE_LENGTH - 12) + " [truncated]"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pi-agent/extensions/pi-mem.ts` around lines 250 - 253, The truncation currently cuts toolResponseText silently; update the truncation logic so when toolResponseText.length > MAX_TOOL_RESPONSE_LENGTH you reserve space for and append a truncation indicator (e.g. "…", " [TRUNCATED]" or similar) rather than cutting raw content — compute sliceLength = MAX_TOOL_RESPONSE_LENGTH - indicator.length, set toolResponseText = toolResponseText.slice(0, sliceLength) + indicator, and ensure MAX_TOOL_RESPONSE_LENGTH and toolResponseText are used so stored observations clearly signal truncated content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pi-agent/extensions/pi-mem.ts`:
- Around line 206-218: The inline comment "Inject as a system message in the
conversation" mismatches the actual injected message which uses role: "user" in
the block that appends to event.messages (see the object containing role: "user"
and content built from contextText); either change the comment to say "Inject as
a user message" (if role: "user" is intentional because pi-mono API doesn't
support mid-conversation system messages) or change the role value to "system"
to match the comment; update the comment next to the code in pi-mem.ts and
ensure the object with role: "user" (or role: "system") and the contextText
content remain consistent.
---
Nitpick comments:
In `@pi-agent/extensions/pi-mem.ts`:
- Around line 250-253: The truncation currently cuts toolResponseText silently;
update the truncation logic so when toolResponseText.length >
MAX_TOOL_RESPONSE_LENGTH you reserve space for and append a truncation indicator
(e.g. "…", " [TRUNCATED]" or similar) rather than cutting raw content — compute
sliceLength = MAX_TOOL_RESPONSE_LENGTH - indicator.length, set toolResponseText
= toolResponseText.slice(0, sliceLength) + indicator, and ensure
MAX_TOOL_RESPONSE_LENGTH and toolResponseText are used so stored observations
clearly signal truncated content.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8dd3ab98-03a3-4975-be67-b7326d53cb13
📒 Files selected for processing (6)
pi-agent/LICENSEpi-agent/README.mdpi-agent/extensions/pi-mem.tspi-agent/package.jsonpi-agent/skills/mem-search/SKILL.mdsrc/shared/platform-source.ts
✅ Files skipped from review due to trivial changes (4)
- pi-agent/LICENSE
- pi-agent/skills/mem-search/SKILL.md
- pi-agent/README.md
- pi-agent/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/platform-source.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pi-agent/extensions/pi-mem.ts (1)
287-291: Consider adding defensive check for consistency.The type guard at line 289 differs from the one at line 245. Line 245 includes
&& "text" in blockto guard against malformed content blocks, while this one doesn't. For consistency and robustness:♻️ Align with existing pattern
lastAssistantMessage = msg.content - .filter((block): block is { type: "text"; text: string } => block.type === "text") + .filter((block): block is { type: "text"; text: string } => block.type === "text" && "text" in block) .map((block) => block.text) .join("\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pi-agent/extensions/pi-mem.ts` around lines 287 - 291, The filter predicate used when building lastAssistantMessage from Array.isArray(msg.content) is missing the defensive check present earlier; update the predicate used in the .filter for msg.content (the block type guard inside the code that sets lastAssistantMessage) to match the earlier pattern by also checking && "text" in block so it only maps blocks that are type === "text" and contain the text property, ensuring consistency with the filter at line 245.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pi-agent/extensions/pi-mem.ts`:
- Around line 287-291: The filter predicate used when building
lastAssistantMessage from Array.isArray(msg.content) is missing the defensive
check present earlier; update the predicate used in the .filter for msg.content
(the block type guard inside the code that sets lastAssistantMessage) to match
the earlier pattern by also checking && "text" in block so it only maps blocks
that are type === "text" and contain the text property, ensuring consistency
with the filter at line 245.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37e6d20b-905b-43b4-b864-3bf3324e645b
📒 Files selected for processing (3)
pi-agent/README.mdpi-agent/extensions/pi-mem.tspi-agent/package.json
✅ Files skipped from review due to trivial changes (2)
- pi-agent/package.json
- pi-agent/README.md
Addresses all review feedback from PR thedotmack#1786: - Fix platformSource camelCase in all worker POST bodies - Fix search query parameter (q → query) - Add NaN guard on port parsing - Add platformSource to summarize/complete/observations calls
Add pi-agent-memory, a native pi-mono extension that connects pi-agents to claude-mem's worker service for cross-session, cross-engine memory. - Extension registers session lifecycle hooks (init, context injection, observation capture, summarize, complete) - Registers memory_recall tool for explicit memory search - Registers /memory-status command for health checks - All HTTP calls use AbortController timeouts - Fire-and-forget for tool_result to avoid blocking the agent loop - Cross-engine: pi-agent observations share the same database with Claude Code, Cursor, Codex, and OpenClaw Also updates platform-source normalization to recognize pi-agent identifiers via exact Set matching.
d5c63d6 to
197db6b
Compare
Summary
Adds
pi-agent/— a pi-mono extension adapter that gives pi-agents (pi-coding-agent, OpenClaw, custom pi-mono runtimes) persistent cross-session memory via the claude-mem worker HTTP API.pi-agent/src/pi-mem.ts(~250 lines) — Standard pi-mono extension subscribing tosession_start,before_agent_start,context,tool_result,agent_end,session_compact,session_shutdown. Derived from the proven OpenClaw plugin pattern (openclaw/src/index.ts).memory_recalltool — Registered LLM tool for explicit history search via hybrid FTS5 + Chroma/memory-statuscommand — Health check for worker connectivitypi-agent— Added tonormalizePlatformSource()andsortPlatformSources()insrc/shared/platform-source.tspi-agent/README.md— Architecture, installation, configuration, cross-engine memory sharing docspi-agent/skills/mem-search/SKILL.md— Pi-agent skill for memory search guidanceThis follows the same pattern as
openclaw/— a self-contained integration directory with minimal changes to core code (2 lines inplatform-source.ts+ 1 line inpackage.jsonfiles array).How it works
Pi-mono events map to claude-mem worker HTTP calls:
session_startPOST /api/sessions/initbefore_agent_startPOST /api/sessions/initcontextGET /api/context/injecttool_resultPOST /api/sessions/observationsagent_endPOST /api/sessions/summarize+completesession_compactsession_shutdownE2E Test Results
Tested with real pi-coding-agent sessions against a live claude-mem worker (v12.1.0):
session_start→ worker initbefore_agent_start→ prompt capturetool_result→ observation capturecontext→ injection into LLMmemory_recalltool invocationpi-agentin DB)Test plan
cp pi-agent/src/pi-mem.ts ~/.pi/agent/extensions/pi-mem.tsnpm run worker:startmemory_recalltool work/memory-statuscommand reports connectedplatform_source: pi-agentin the DB