feat: OpenCode plugin with 22 auto-capture hooks (closes #236, #244)#237
Conversation
- 22 hook handlers across session lifecycle, messages, tool lifecycle, parts, files, permissions, tasks, commands, and config - Two-layer enrichment pipeline: /context + /enrich via system.transform - Two slash commands: /recall and /remember - Full Claude Code hook parity documented with gap analysis
|
@xuli500177 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
📝 WalkthroughWalkthroughAdds an OpenCode plugin (manifest, implementation, docs, and commands) that captures OpenCode events, forwards structured observations to agentmemory endpoints, injects enriched context into system prompts, and updates README/ROADMAP; also adds ChangesOpenCode Agentmemory Plugin Implementation
Sequence Diagram(s)sequenceDiagram
participant OpenCode
participant Plugin
participant Agentmemory
OpenCode->>Plugin: experimental.chat.system.transform invocation
Plugin->>Agentmemory: GET /agentmemory/context (once per session)
Plugin->>Agentmemory: POST /agentmemory/enrich (up to 10 stashed files)
Agentmemory-->>Plugin: context + enriched fragments
Plugin->>OpenCode: append enriched context into output.system
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 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)
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 |
|
you have a lot of pending prs |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
plugin/opencode/agentmemory-capture.ts (1)
8-31: 💤 Low valueSilent error swallowing makes plugin failures invisible.
Both
postandpostJsoncatch every error (network, timeout, malformed JSON) without any logging. This is a deliberate "don't take down the user's session" choice and is reasonable, but combined with the rest of the file (no debug toggle), an entire session of failed observations will appear as nothing — which makes diagnosing "why is no memory being captured?" very hard for end users following the README.A single env-gated debug log is enough to make this self-diagnosable without spamming logs in normal operation.
Optional: gated debug log
-async function post(path: string, body: Record<string, unknown>): Promise<void> { - try { +const DEBUG = process.env.AGENTMEMORY_DEBUG === "1"; +async function post(path: string, body: Record<string, unknown>): Promise<void> { + try { await fetch(`${API}/agentmemory${path}`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), signal: AbortSignal.timeout(5000), }); - } catch {} + } catch (err) { + if (DEBUG) console.error(`[agentmemory] POST ${path} failed:`, err); + } }🤖 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/opencode/agentmemory-capture.ts` around lines 8 - 31, Add guarded error logging to the network helpers so failures aren’t silently swallowed: in both post and postJson wrap the catch blocks to log the caught error (include error.message and the request path and body summary) when a debug env flag (e.g., process.env.OPENCODE_AGENTMEMORY_DEBUG === "1") is set, but keep returning/ignoring the error as today to avoid crashing; update the catch in postJson to likewise log before returning null. Reference the functions post and postJson and ensure logs are concise and gated behind the env var.
🤖 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 `@plugin/opencode/agentmemory-capture.ts`:
- Around line 370-376: The stashedFiles set can grow beyond MAX_STASHED_FILES
because the file.edited handler and the chat.message file-path handling call
stashedFiles.add(...) without trimming; extract a small helper (e.g.,
addStashedFileOrTrim or enforceStashCap) that adds a path to stashedFiles and
immediately enforces the MAX_STASHED_FILES cap (removing oldest/any
deterministic entries) and call that helper from the file.edited branch, the
chat.message file-path handling, and keep using it in tool.execute.before to
ensure the cap is always enforced consistently across stashedFiles updates.
- Around line 50-53: seenSubtaskIds and seenToolCallIds currently grow unbounded
for long-lived sessions; replace each with a bounded FIFO dedup structure (e.g.,
a small helper BoundedSet or OrderedMap-based set) that caps entries (choose a
sensible cap like 10k or make it configurable) and evicts oldest entries on
insert when capacity is exceeded; update all usages that currently call
seenSubtaskIds.add/has and seenToolCallIds.add/has to use the new BoundedSet API
and ensure the same clearing on session.created and session.deleted remains;
also apply the same replacement at the other dedup sites referenced (the block
around the 256-307 area) so all dedup sets use FIFO eviction.
- Around line 232-235: The duration_ms calculation can yield a large negative
number when info.time.completed is unset; update the duration_ms assignment in
agentmemory-capture.ts to compute only when info.time.completed (or an
equivalent end timestamp) exists and is >= info.time.created, otherwise set null
— i.e., check info.time.completed (or casted endTime) and info.time.created
(startTime) first and use (completed - created) || null similar to the
tool-state branch to avoid negative durations for the duration_ms field.
- Around line 109-130: The handler is posting /summarize in both the
"session.idle" branch and again when handling "session.status" with status.type
=== "idle", causing duplicate summarize requests; remove the redundant summarize
call from the "session.idle" branch (the block checking if (type ===
"session.idle") should no longer call post("/summarize") using props.sessionID
or activeSessionId) and keep the summarize behavior only in the "session.status"
branch (which already checks status.type === "idle"); leave the observe(sid,
"session_status", ...) logic unchanged.
- Around line 487-529: Add defensive checks and only clear stashedFiles after a
successful enrich: in the "experimental.chat.system.transform" handler, verify
output?.system is an array (e.g., if (!Array.isArray(output?.system)) return;)
before attempting output.system.push(...); in the
"experimental.session.compacting" handler, verify output?.context is an array
(e.g., if (!Array.isArray(output?.context)) return;) before
output.context.push(...); for stashed file handling in
"experimental.chat.system.transform", do not call stashedFiles.clear()
unconditionally—create the files array to send, call postJson("/enrich", {
sessionId: sid, files, toolName: "enrich_inject" }), and only remove the
specific files sent from stashedFiles when enrichResult is valid (i.e., contains
context); keep stashed files if the POST fails so they can be retried.
- Around line 531-544: The config handler currently drops config events when
activeSessionId is falsy, causing startup config_loaded observations to be lost;
change the logic in the config async handler to not gate on activeSessionId
alone — introduce a separate boolean (e.g., seenStartupConfig) or buffer the
payload and either create/use a synthetic session id until session.created
occurs, then call observe(sessionId, "config_loaded", payload) so the event is
recorded once per process; also defensively build agents/mcp_servers/providers
by checking typeof input.agent/mcp/provider === "object" before Object.keys and
treat string values as a single-name entry (or wrap them into arrays) so numeric
string indices are not used when calling observe.
- Around line 81-83: The code sets projectPath from ctx.project?.id which is an
opaque identifier; change AgentmemoryCapturePlugin so projectPath uses the
filesystem path (ctx.worktree or a filesystem path on ctx.project, not
ctx.project?.id) and keep ctx.project?.id as a separate identifier field if
needed; update any places that previously sent projectPath as cwd in
observations to send the filesystem path (projectPath) as cwd and, if the
backend needs project id, include ctx.project?.id in a distinct property.
- Around line 274-305: The handler for completed/error tool states dereferences
st.input directly before JSON.stringify which can return undefined and make
.slice crash; update both occurrences (inside the completed and error branches
where observe(...) is called) to defensively coalesce input so JSON.stringify
always receives a defined value (e.g., use st.input ?? null) and then coalesce
the stringify result to an empty string before slicing, ensuring observe(...)
calls (in the block using seenToolCallIds, st, startTime/endTime and calling
observe) never throw on missing input.
In `@plugin/opencode/README.md`:
- Line 12: The badge in plugin/opencode/README.md currently reads "MCP-44_tools"
and conflicts with the top-level README's "51 tools"; update the badge text to
match the canonical count (e.g., change "MCP-44_tools" to "MCP-51_tools") so
both files agree, by editing the <img> tag in plugin/opencode/README.md (and if
the canonical source is the plugin instead, update the top-level README
occurrences of "51 tools" instead) ensuring the numeric value is consistent
across README.md and plugin/opencode/README.md.
---
Nitpick comments:
In `@plugin/opencode/agentmemory-capture.ts`:
- Around line 8-31: Add guarded error logging to the network helpers so failures
aren’t silently swallowed: in both post and postJson wrap the catch blocks to
log the caught error (include error.message and the request path and body
summary) when a debug env flag (e.g., process.env.OPENCODE_AGENTMEMORY_DEBUG ===
"1") is set, but keep returning/ignoring the error as today to avoid crashing;
update the catch in postJson to likewise log before returning null. Reference
the functions post and postJson and ensure logs are concise and gated behind the
env var.
🪄 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: b93fb723-d7d2-435c-9cfa-c539f2219e8c
📒 Files selected for processing (7)
README.mdROADMAP.mdplugin/opencode/README.mdplugin/opencode/agentmemory-capture.tsplugin/opencode/commands/recall.mdplugin/opencode/commands/remember.mdplugin/opencode/plugin.json
| export const AgentmemoryCapturePlugin: Plugin = async (ctx) => { | ||
| projectPath = ctx.project?.id || ctx.worktree || null; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
opencode plugin SDK ctx.project ctx.worktree fields v1.14
💡 Result:
In OpenCode's plugin SDK (compatible with v1.14 as used in plugins like opencode-plugin-otel [1]), plugins receive a context object (PluginInput or ctx) with fields including ctx.project and ctx.worktree. ctx.worktree is a top-level string field representing the git worktree path [2][3][4]. ctx.project is an object of type Project with the following fields [5][6]: - id: string (project identifier, e.g., git hash or "global") [4] - worktree: string (git worktree root directory; ctx.worktree is an alias) [4][5] - vcs?: "git" (version control system) [4][5][6] - name?: string [5] - icon?: { url?: string, override?: string, color?: string } [5] - commands?: { start?: string } (startup script for new workspaces) [5] - time: { created: number, updated: number, initialized?: number } [5][6] - sandboxes: Array<string> [5] Official docs confirm the structure without listing all subfields explicitly [3][7]. No breaking changes to these fields noted in v1.14 [1]. Use TypeScript types from "@opencode-ai/plugin" or "@opencode-ai/sdk" for full definitions [2].
Citations:
- 1: chore: upgrade opencode SDK to 1.14.20 DEVtheOPS/opencode-plugin-otel#34
- 2: https://github.com/sst/opencode/blob/c7b35342/packages/plugin/src/index.ts
- 3: https://opencode.ai/docs/plugins/
- 4: https://gist.github.com/rstacruz/946d02757525c9a0f49b25e316fbe715
- 5: https://github.com/sst/opencode/blob/9ad6588f/packages/sdk/js/src/v2/gen/types.gen.ts
- 6: https://www.mintlify.com/anomalyco/opencode/sdk/types
- 7: https://dev.opencode.ai/docs/plugins
🏁 Script executed:
cat -n plugin/opencode/agentmemory-capture.tsRepository: rohitg00/agentmemory
Length of output: 22956
🏁 Script executed:
# Check if there are any TypeScript type definitions for the Plugin type
rg -A 20 "type Plugin" plugin/opencode/agentmemory-capture.ts | head -30Repository: rohitg00/agentmemory
Length of output: 46
🏁 Script executed:
# Look for how ctx is typed in the SDK or other plugin files
fd -name "*.ts" -path "*plugin*" | head -10Repository: rohitg00/agentmemory
Length of output: 295
🏁 Script executed:
# Find plugin files
fd --type f "\.ts$" plugin/Repository: rohitg00/agentmemory
Length of output: 103
🏁 Script executed:
# Check the import to understand the Plugin type
head -5 plugin/opencode/agentmemory-capture.tsRepository: rohitg00/agentmemory
Length of output: 327
Use the filesystem path for projectPath, not the project identifier.
ctx.project?.id is an opaque identifier (hash/slug), while ctx.worktree is the filesystem path. The current code incorrectly sends the project ID as cwd to the backend in every observation (lines 41–42, 104–105, 495, 524), which will not work since the backend expects cwd to be a valid working directory.
Suggested fix
- projectPath = ctx.project?.id || ctx.worktree || null;
+ projectPath = ctx.worktree || ctx.project?.worktree || null;Keep ctx.project?.id as a separate identifier if the backend needs to correlate sessions by project; send it as a distinct field rather than as cwd.
📝 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.
| export const AgentmemoryCapturePlugin: Plugin = async (ctx) => { | |
| projectPath = ctx.project?.id || ctx.worktree || null; | |
| export const AgentmemoryCapturePlugin: Plugin = async (ctx) => { | |
| projectPath = ctx.worktree || ctx.project?.worktree || null; |
🤖 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/opencode/agentmemory-capture.ts` around lines 81 - 83, The code sets
projectPath from ctx.project?.id which is an opaque identifier; change
AgentmemoryCapturePlugin so projectPath uses the filesystem path (ctx.worktree
or a filesystem path on ctx.project, not ctx.project?.id) and keep
ctx.project?.id as a separate identifier field if needed; update any places that
previously sent projectPath as cwd in observations to send the filesystem path
(projectPath) as cwd and, if the backend needs project id, include
ctx.project?.id in a distinct property.
mem::observe checks hookType === "prompt_submit" to extract raw.userPrompt and set session.firstPrompt. The plugin was using "user_prompt_submit" which didn't match, so sessions were never named.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
plugin/opencode/agentmemory-capture.ts (8)
82-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
ctx.project?.idis an opaque slug, not a filesystem path —cwdwill be wrong in every observation.This issue was previously flagged and remains unresolved.
ctx.project?.idis a hash/slug identifier;ctx.worktreeis the filesystem working directory that the backend expects ascwd.Proposed fix
- projectPath = ctx.project?.id || ctx.worktree || null; + projectPath = ctx.worktree || ctx.project?.worktree || null;🤖 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/opencode/agentmemory-capture.ts` at line 82, The current assignment uses ctx.project?.id (an opaque slug) for projectPath which yields an incorrect cwd; change the assignment in agentmemory-capture.ts so projectPath is derived from ctx.worktree first (e.g., projectPath = ctx.worktree || ctx.project?.id || null) and ensure any downstream use that sets observation.cwd uses projectPath (or directly ctx.worktree) so the filesystem working directory is correct.
487-527:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
Array.isArrayguards onoutput.systemandoutput.contextbefore.push(), andstashedFiles.clear()precedes the enrich request.Previously flagged, still unresolved:
output.system.push(...)at lines 498 and 513 will throw ifoutput.systemis not an array (possible in experimental hooks).output.context.push(...)at line 527 has the same risk.stashedFiles.clear()at line 504 discards all stashed files before confirming the/enrichPOST succeeded; a transient failure silently drops them with no retry.🤖 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/opencode/agentmemory-capture.ts` around lines 487 - 527, In the experimental.chat.system.transform and experimental.session.compacting handlers, guard calls to output.system.push and output.context.push with Array.isArray checks (e.g., only push when Array.isArray(output.system) or Array.isArray(output.context)), and move the stashedFiles.clear() so it only runs after a successful postJson("/enrich") response (clear the items only if enrichResult exists and contains the expected context); locate these changes around the experimental.chat.system.transform function (where output.system is mutated and postJson("/enrich") is called) and the experimental.session.compacting handler (where output.context is mutated) and ensure you check the enrichResult before clearing stashedFiles to avoid dropping files on transient failures.
232-234:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
duration_msproduces a large negative number whentime.completedis unset.
((info.time as any).completed || 0) - ((info.time as any).created || 0)falls back to0 - created_timestamp, yielding a large negative value for still-streaming or error-terminated messages. Previously flagged, still unresolved.🤖 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/opencode/agentmemory-capture.ts` around lines 232 - 234, The duration_ms calculation can become a large negative value because it falls back to 0 when info.time.completed is unset; update the duration_ms expression (the usage around duration_ms and info.time) to only compute ((info.time as any).completed - (info.time as any).created) when info.time?.completed is a valid number (or not null/undefined) and info.time?.created is present; otherwise set duration_ms to null; also ensure you don't coerce missing completed to 0 (use explicit presence checks or Number.isFinite) so streaming or error-terminated messages yield null instead of negative durations.
370-376:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
stashedFilescap not enforced onfile.edited, andprops.fileis added without a null/empty guard.Two issues here:
- (Duplicate) The
MAX_STASHED_FILEScap is trimmed intool.execute.before(lines 479–483) but not in thisfile.editedbranch, allowing the set to grow past the bound — previously flagged.- (New)
props.file as stringis a compile-time assertion only. Ifprops.fileisundefinedat runtime,stashedFiles.add(undefined)storesundefined, which serializes tonullin the/enrichJSON body, potentially causing backend errors.Proposed fix
if (type === "file.edited") { - const sid = activeSessionId; - if (sid) { - stashedFiles.add(props.file as string); - } + if (activeSessionId && props.file && typeof props.file === "string") { + stashedFiles.add(props.file); + if (stashedFiles.size > MAX_STASHED_FILES) { + const keep = [...stashedFiles].slice(-MAX_STASHED_FILES); + stashedFiles.clear(); + for (const f of keep) stashedFiles.add(f); + } + } }🤖 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/opencode/agentmemory-capture.ts` around lines 370 - 376, The file.edited branch currently adds props.file to stashedFiles without validating it and without enforcing the MAX_STASHED_FILES cap; update the file.edited handler (the branch where type === "file.edited") to first guard that props.file is a non-empty string (e.g., truthy and typeof === "string") before calling stashedFiles.add(props.file), and after adding enforce the same trimming logic used in tool.execute.before (ensure stashedFiles.size is capped to MAX_STASHED_FILES by removing oldest entries) so the set cannot grow past the configured bound; reference stashedFiles, MAX_STASHED_FILES, tool.execute.before, props.file, and the file.edited branch when making the change.
50-53:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff
seenSubtaskIdsandseenToolCallIdsgrow unbounded for long-lived sessions.This was previously flagged and is still unresolved. Both sets are cleared only on
session.created/session.deleted; in agent loops with thousands of tool calls, this is a slow process-level memory leak.🤖 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/opencode/agentmemory-capture.ts` around lines 50 - 53, seenSubtaskIds and seenToolCallIds currently live as global Sets and grow unbounded; change them to be per-session and bounded: replace the global Set<string> seenSubtaskIds and seenToolCallIds with a Map<string, Set<string>> (keyed by sessionId) so each session gets its own set (e.g., seenSubtaskIdsBySession, seenToolCallIdsBySession), initialize the session entry on session.created and remove it on session.deleted, and additionally enforce a hard cap or LRU-style eviction for each session set (e.g., if set.size > MAX_PER_SESSION remove oldest entries) to prevent long-running sessions from leaking memory during thousands of tool calls; update any code that referenced seenSubtaskIds/seenToolCallIds to look up by current sessionId.
531-544:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
config_loadedevents fired before the firstsession.createdare silently dropped;input.agentis not guarded against string values beforeObject.keys.Previously flagged, still unresolved. The
confighook typically fires at OpenCode startup before any session exists, soactiveSessionIdwill benulland the observation is lost for the first session. Additionally,Object.keys(input.agent as Record<string, unknown>)can return numeric indices ifinput.agentis a string in a single-agent shorthand config.🤖 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/opencode/agentmemory-capture.ts` around lines 531 - 544, The config hook currently drops early config events when activeSessionId is null and assumes input.agent/mcp/provider are objects; modify the config handler (config) to 1) if activeSessionId is null, stash the prepared payload in a temporary pendingConfig variable and return, and 2) add a small listener for the first session.created event to consume pendingConfig by calling observe(sessionId, "config_loaded", payload); also guard agents/mcp_servers/providers by normalizing input.agent/input.mcp/input.provider: if typeof === "string" treat as single-element array, if Array.isArray use it directly, otherwise if object use Object.keys, and always coerce to [] when undefined, and use observe(...) when activeSessionId is present.
109-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate
/summarizecall on session idle transition.Both
session.idle(lines 110–115) andsession.statuswithstatus.type === "idle"(lines 122–124) independently callPOST /summarize. Per the OpenCode event contract, these fire together for the same idle transition, producing two identical calls per event. Previously flagged, still unresolved.🤖 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/opencode/agentmemory-capture.ts` around lines 109 - 115, The code currently posts to "/summarize" in both the session.idle branch (when type === "session.idle" using sid = props.sessionID || activeSessionId and calling post("/summarize", { sessionId: sid })) and again in the session.status branch when status.type === "idle", causing duplicate calls; fix by deduplicating: remove the post("/summarize", ...) from the session.idle handler and centralize the summarize call in the session.status handler (or alternatively add a short-lived per-session dedupe guard keyed by sessionId) so only one POST /summarize is made per idle transition.
283-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
JSON.stringify(st.input).slice(0, 4000)crashes withTypeErrorwhenst.inputisundefined.
JSON.stringify(undefined)returnsundefined(not a string), so.slice()throws. Previously flagged, still unresolved. The adjacentst.output/st.errorfields already use the defensive|| ""pattern.Proposed fix (both lines)
- tool_input: JSON.stringify(st.input).slice(0, 4000), + tool_input: (JSON.stringify(st.input ?? null) ?? "").slice(0, 4000),Also applies to: 301-301
🤖 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/opencode/agentmemory-capture.ts` at line 283, The current assignment for tool_input uses JSON.stringify(st.input).slice(...) which throws when JSON.stringify returns undefined; change the tool_input assignment so you first coerce or default the result of JSON.stringify(st.input) to an empty string before calling slice (i.e. ensure slice is called on a string), and make the identical defensive change to the other occurrence where st.output/st.error are sliced (refer to the tool_input assignment and the corresponding tool_output/tool_error-like assignments that use JSON.stringify(st.output)/st.error).
🤖 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 `@plugin/opencode/agentmemory-capture.ts`:
- Around line 491-499: The code currently calls contextInjectedSessions.add(sid)
before awaiting postJson("/context"), which marks the session as injected even
if the fetch fails; change the flow so you only add sid to
contextInjectedSessions after a successful context fetch. Specifically, call
postJson("/context", { sessionId: sid, project: projectPath }) first, check that
the returned result is an object and contains a context ((result as
any).context), push that context into output.system, and only then call
contextInjectedSessions.add(sid); do not add the sid on null/failed responses
(consider keeping the add inside the successful branch and optionally wrap the
call in try/catch to avoid swallowing errors).
---
Duplicate comments:
In `@plugin/opencode/agentmemory-capture.ts`:
- Line 82: The current assignment uses ctx.project?.id (an opaque slug) for
projectPath which yields an incorrect cwd; change the assignment in
agentmemory-capture.ts so projectPath is derived from ctx.worktree first (e.g.,
projectPath = ctx.worktree || ctx.project?.id || null) and ensure any downstream
use that sets observation.cwd uses projectPath (or directly ctx.worktree) so the
filesystem working directory is correct.
- Around line 487-527: In the experimental.chat.system.transform and
experimental.session.compacting handlers, guard calls to output.system.push and
output.context.push with Array.isArray checks (e.g., only push when
Array.isArray(output.system) or Array.isArray(output.context)), and move the
stashedFiles.clear() so it only runs after a successful postJson("/enrich")
response (clear the items only if enrichResult exists and contains the expected
context); locate these changes around the experimental.chat.system.transform
function (where output.system is mutated and postJson("/enrich") is called) and
the experimental.session.compacting handler (where output.context is mutated)
and ensure you check the enrichResult before clearing stashedFiles to avoid
dropping files on transient failures.
- Around line 232-234: The duration_ms calculation can become a large negative
value because it falls back to 0 when info.time.completed is unset; update the
duration_ms expression (the usage around duration_ms and info.time) to only
compute ((info.time as any).completed - (info.time as any).created) when
info.time?.completed is a valid number (or not null/undefined) and
info.time?.created is present; otherwise set duration_ms to null; also ensure
you don't coerce missing completed to 0 (use explicit presence checks or
Number.isFinite) so streaming or error-terminated messages yield null instead of
negative durations.
- Around line 370-376: The file.edited branch currently adds props.file to
stashedFiles without validating it and without enforcing the MAX_STASHED_FILES
cap; update the file.edited handler (the branch where type === "file.edited") to
first guard that props.file is a non-empty string (e.g., truthy and typeof ===
"string") before calling stashedFiles.add(props.file), and after adding enforce
the same trimming logic used in tool.execute.before (ensure stashedFiles.size is
capped to MAX_STASHED_FILES by removing oldest entries) so the set cannot grow
past the configured bound; reference stashedFiles, MAX_STASHED_FILES,
tool.execute.before, props.file, and the file.edited branch when making the
change.
- Around line 50-53: seenSubtaskIds and seenToolCallIds currently live as global
Sets and grow unbounded; change them to be per-session and bounded: replace the
global Set<string> seenSubtaskIds and seenToolCallIds with a Map<string,
Set<string>> (keyed by sessionId) so each session gets its own set (e.g.,
seenSubtaskIdsBySession, seenToolCallIdsBySession), initialize the session entry
on session.created and remove it on session.deleted, and additionally enforce a
hard cap or LRU-style eviction for each session set (e.g., if set.size >
MAX_PER_SESSION remove oldest entries) to prevent long-running sessions from
leaking memory during thousands of tool calls; update any code that referenced
seenSubtaskIds/seenToolCallIds to look up by current sessionId.
- Around line 531-544: The config hook currently drops early config events when
activeSessionId is null and assumes input.agent/mcp/provider are objects; modify
the config handler (config) to 1) if activeSessionId is null, stash the prepared
payload in a temporary pendingConfig variable and return, and 2) add a small
listener for the first session.created event to consume pendingConfig by calling
observe(sessionId, "config_loaded", payload); also guard
agents/mcp_servers/providers by normalizing
input.agent/input.mcp/input.provider: if typeof === "string" treat as
single-element array, if Array.isArray use it directly, otherwise if object use
Object.keys, and always coerce to [] when undefined, and use observe(...) when
activeSessionId is present.
- Around line 109-115: The code currently posts to "/summarize" in both the
session.idle branch (when type === "session.idle" using sid = props.sessionID ||
activeSessionId and calling post("/summarize", { sessionId: sid })) and again in
the session.status branch when status.type === "idle", causing duplicate calls;
fix by deduplicating: remove the post("/summarize", ...) from the session.idle
handler and centralize the summarize call in the session.status handler (or
alternatively add a short-lived per-session dedupe guard keyed by sessionId) so
only one POST /summarize is made per idle transition.
- Line 283: The current assignment for tool_input uses
JSON.stringify(st.input).slice(...) which throws when JSON.stringify returns
undefined; change the tool_input assignment so you first coerce or default the
result of JSON.stringify(st.input) to an empty string before calling slice (i.e.
ensure slice is called on a string), and make the identical defensive change to
the other occurrence where st.output/st.error are sliced (refer to the
tool_input assignment and the corresponding tool_output/tool_error-like
assignments that use JSON.stringify(st.output)/st.error).
🪄 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: f60c3ecc-6be0-48ad-ab0f-f0d790de8744
📒 Files selected for processing (1)
plugin/opencode/agentmemory-capture.ts
|
@cl0ckt0wer Can you please fix pending coderabbit issues? |
|
Working on it 🤟. Good to see you're active still |
- Use ctx.worktree for projectPath instead of opaque project.id - Add Array.isArray guards before output.system/.context.push() - Only delete stashed files after successful enrich POST - Defensive JSON.stringify for undefined tool inputs - Per-session Map-based dedup sets to prevent unbounded growth - Fix negative duration_ms when time.completed is unset - Validate props.file and enforce MAX_STASHED_FILES in file.edited - Deduplicate /summarize call on session idle - Buffer early config events until session.created flushes them - Move contextInjectedSessions.add after successful context fetch - Add DEBUG-gated error logging to network helpers - Guard config input.agent/mcp/provider against non-object types - Fix MCP badge count 44→51 in plugin README
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugin/opencode/README.md`:
- Around line 153-157: Add a language identifier to the fenced code block that
starts with "System prompt = [OpenCode instructions] + [memory context] + [file
enrichment] + [user message]": change the opening ``` to ```text (or
```plaintext) so the fence reads e.g. ```text and satisfies markdownlint MD040
and improves rendering consistency.
🪄 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: 9af29773-c581-4d82-86f2-45246d6d2ab3
📒 Files selected for processing (2)
plugin/opencode/README.mdplugin/opencode/agentmemory-capture.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/opencode/agentmemory-capture.ts
|
Hi @cl0ckt0wer — really nice work on this. OpenCode parity has been on the Q2 roadmap (Planned: "OpenCode hook bus") and an early arrival is welcome. The 22-hook coverage and the gap-analysis README in the plugin folder are well done, and the two-layer enrichment story ( One coordination question before I review in depth: you also have #239 open ("session instruction injection and consolidation pipeline"), which touches the exact same files as this PR ( What I think is happening: this is the v1 plugin, and #239 adds the three gaps you found in the clco→oco port sweep on top. If that's right, can we either: Option A (preferred) — fold #239 into this PR. Squash everything into one OpenCode PR that lands the plugin + the three gap fixes together. Cleaner CHANGELOG entry, single review cycle. Option B — land this first, then rebase #239 onto the result so it's a clean diff of just the three gap fixes. Either works, just want to avoid two parallel branches against the same files. Let me know which you prefer and I'll review the consolidated PR straight through. One scope note while you're here: the plugin currently lists "44 MCP tools" in the badge in the latest version (in #239) vs "51" in this branch. agentmemory ships 51 today (per |
|
I agree, option A. I'm on it. There was a lot of scope creep sorry about that. |
…line (closes rohitg00#233) Three gaps from the Claude Code plugin port sweep: - Inject agentmemory usage instructions (memory_save, memory_recall, etc.) into the system prompt on first turn via experimental.chat.system.transform, replacing the skills mechanism that OpenCode lacks - Call /crystals/auto and /consolidate-pipeline on session.deleted, mirroring Claude's CONSOLIDATION_ENABLED behavior - Document MEMORY.md vs AGENTS.md architecture comparison (two-hop file bridge vs one-hop direct injection) Gap A (SubagentStop) is unfixable — OpenCode's SubtaskPart type has no completion/result fields. Gap C (Claude MEMORY.md bridge) is intentionally skipped — OpenCode uses direct injection.
|
Folded #239 into this PR (Option A). Here's what changed: From #239 (cherry-picked):
Preserved from #237 (existing):
Tool count badge: kept at "51 tools" (matches current Ready for consolidated review. |
|
don't merge yet, still doing code review |
- Guard instructions push with Array.isArray check + fix TOCTOU race by moving contextInjectedSessions.add() before the await - Session-scope stashedFiles via stashFor() helper (was process-global, could cross-contaminate concurrent sessions) - Fix tool prefix in instructions (agentmemory_memory_ not agentmemory_) - Check typeof string before pushing .context into system arrays - Change olderThanDays: 0 → 7 in consolidation fire-and-forget - Increase fire-and-forget timeout from 5s to 30s (consolidation takes minutes, 5s was guaranteed to abort)
- Fix session cross-contamination: file.edited and tool.execute.before now use props.sessionID/input.sessionID fallback instead of only activeSessionId, preventing subagent hijack of parent stash - Fix contextInjectedSessions regression from round 1: move add(sid) to after instructions push (synchronous) but before context fetch (async), so failed /context calls don't permanently skip injection - Fix Map memory leak: prune session entries (stashedFiles, seenSubtaskIds, seenToolCallIds) when session.status goes idle, preventing unbounded growth for crash-killed sessions
- Enforce MAX_STASHED_FILES cap on chat.message (was missing, could grow unbounded from 50+ file-ref messages) - Remove activeSessionId fallback from session.deleted; log warning when both info.id and sessionID are missing instead of guessing - Add subtaskSetFor/toolCallSetFor lazy-init wrappers matching stashFor pattern; fixes dedup failures for subagents spawned without a preceding session.created event - Guard chat.params against missing input.model (TypeError crash)
- Add safeSlice() helper replacing all unsafe (v as string || "").slice() calls; handles objects/BigInt/circular refs via try/catch JSON.stringify - Restore activeSessionId fallback on session.deleted (if both info.id and sessionID missing, fall back like every other handler) - Fix message.updated to check info.id before info.sessionID (matches session.created/session.updated pattern) - Add props.sessionID to message.part.updated fallback chain - Guard undefined callID and subtask ID (prevent dedup silently dropping all subsequent undefined-ID events) - Cap todo.updated at 100 entries (match session.diff's slice pattern) - Fix duration_ms || → ?? (0ms genuine duration no longer falsifies) - Fix extractErrorMessage || → ?? (0/false error values preserved) - Replace extractErrorMessage.slice with safeSlice in retry handler
CRITICAL: - Revert message.updated session ID resolution: info.id is the message UUID, not session ID. In message.updated, info is the message object (has role/tokens/modelID), not session info like in session.created. Use props.sessionID || info.sessionID || activeSessionId instead. - Fix post_tool_use_failure -> post_tool_failure: server types.ts defines "post_tool_failure" (no "use_"); compress-synthetic.ts classifies by that exact string. All error observations were misclassified as "other" instead of "error". - Fix contextInjected ordering: move add(sid) after the context fetch completes, not before. If /context times out, session is no longer permanently marked injected, allowing retry on next transform. HIGH: - Add process.cwd() fallback for projectPath (was null, causing silent 400s on every REST call when no workspace) - Guard session.start against null activeSessionId with early return (was sending sessionId: null to API) - Fix duration_ms: use typeof number checks instead of || 0 defaults; missing timing data now correctly reports null instead of 0ms
|
ok we should be good now |
|
Hello guys. Is there any hope to see plugins for opencode? Thanx =) |
|
Can't wait for this PR to be merged, nice work ! |
|
Amazing! Doing final checks and we'll be good to merge. Good work @cl0ckt0wer |
|
Thanks buddy |
Brings 10 days of main into the OpenCode plugin branch so the PR no longer conflicts on README + carries the new surfaces that shipped between v0.9.2 (when the branch opened) and v0.9.20: - v0.9.19 commit linking (rohitg00#498): KV.commits + Session.commitShas + memory_commit_lookup/memory_commits MCP tools (53 total now, plugin badge bumped from 51) - v0.9.19 Azure OpenAI v1 URL pattern (rohitg00#462) + Dijkstra graph retrieval (rohitg00#463) - v0.9.19 env passthrough on MCP server entries (rohitg00#460): ${VAR} expansion for AGENTMEMORY_URL / AGENTMEMORY_SECRET so one wired entry covers local + remote - v0.9.20 Codex Stop revert (rohitg00#501) README conflict resolution kept main's richer "Other agents" table shape (env-passthrough block + per-host config-file column + programmatic-access section) and re-added the OpenCode entry as two rows: "OpenCode (MCP only)" for the bare MCP wiring + "OpenCode (full plugin)" pointing at this plugin's 22-hook capture surface. src/triggers/api.ts auto-merged: PR's 3-line title->summary/firstPrompt addition (lines 535, 543, 544) survived alongside main's other api.ts churn since. plugin/opencode/plugin.json bumped 0.9.4 -> 0.9.20 to match the canonical version everything else ships on. plugin/opencode/README.md MCP-tool badge bumped 51 -> 53.
Quality + integration wave. Bundles 11 PRs since v0.9.20: Contributor feature: - #237 OpenCode plugin with 22 auto-capture hooks (@cl0ckt0wer) Bug fixes (9): - #516 memory_recall endpoint + format/token_budget (@serhiizghama, closes #507/#440) - #461 env-file AGENTMEMORY_DROP_STALE_INDEX flag honored (@honor2030, closes #456) - #487 Windows hook path quoting (@honor2030, closes #477) - #517 viewer IME composition guard (@jonathanzhan1975) - #472 chunk large sessions for LLM context window (@efenex) - #473 surface lessons in smart-search + diagnose tally (@efenex) - #486 declare all Hermes plugin hooks (@honor2030) - #500 rebuildIndex non-blocking on boot (@efenex) - #504 batched embed in rebuildIndex (25h -> 3h) (@efenex) - #491 cli skip onboarding without tty (@honor2030) Upstream-installer revert: - #546 drop --next workaround now that iii-hq/iii#1660 shipped 1067/1067 tests pass across 95 files.
Summary
Adds a complete OpenCode plugin (
plugin/opencode/) that gives OpenCode agents full persistent memory via agentmemory, matching Claude Code hook parity./contexton first turn, file enrichment via/enrichon subsequent turns with stashed files/recall(memory search) and/remember(save insight)Recent fix (#244)
api::session::startnow stores OpenCode'stitlefromsession.createdas bothsummaryandfirstPrompton the session, so sessions display human-readable names instead of raw IDs.What's included
Plugin (
plugin/opencode/agentmemory-capture.ts) — 546 linessession.created,session.idle,session.status,session.compacted,session.updated,session.diff,session.deleted,session.errorchat.message,message.updated(user+assistant),message.removedmessage.part.updated(ToolPart: completed/error) +tool.execute.beforemessage.part.updated(subtask, step-finish, reasoning, file, patch, compaction, agent, retry)tool.execute.before+file.edited+message.part.updated(file) →experimental.chat.system.transformpermission.updated,permission.repliedtodo.updatedcommand.executedchat.params,configexperimental.chat.system.transform,experimental.session.compacting(WIP)Docs (
plugin/opencode/README.md)Key design decisions
message.part.updatedToolPart states instead oftool.execute.after— richer data with timing and error statesexperimental.chat.system.transforminjects context directly into system prompt instead of syncing to MEMORY.mdTesting
opencode runCLI + agentmemory API observation inspectionsession.compactedconfirmed working (event bus) —experimental.session.compactingWIP pending Go binary wiringexperimental.session.compacting(SDK typed but not wired in OpenCode v1.14.41)Related
Summary by CodeRabbit