fix(multi): stability pass for #627 #640 #474 #638 #431 #544 #563#648
Merged
Conversation
Six issues, one PR. Each lands with a targeted regression test; 1119/1119 vitest pass. #627 OpenAI thinking-model fallback src/providers/openai.ts now reads message.reasoning_content alongside message.reasoning. DeepSeek V4 / Qwen3 / GLM / Kimi return the underscored field — previously compress silently failed (0/700 calls) and the circuit breaker tripped. #640 + #474 stop reaps the worker process src/index.ts writes ~/.agentmemory/worker.pid on registerWorker, clears it on graceful shutdown. src/cli.ts runStop now reads the worker pidfile and signals SIGTERM alongside the engine pids. Fixes both: the daemon wrapper surviving stop (#640) and the iii engine retaining stale function registrations because the worker reconnected to the new engine (#474). #638 OpenCode session implicit-create on observe src/functions/observe.ts now creates the session record on the first observation when project + cwd are present and no session exists. OpenCode plugins (and any caller that skips POST /session/start) no longer leak observations into a session memory_sessions never lists, and summarize stops bailing with 'Session not found'. #431 OpenCode auto-context (zero-config injection) plugin/opencode/agentmemory-capture.ts captures the context returned by POST /session/start into a per-session cache. The existing experimental.chat.system.transform hook now reads from the cache first, falls back to /context. Cleanup on session.deleted. #544 paginated /memories + /export src/triggers/api.ts adds three query modes to /memories: ?count=true — totals only, viewer status badge ?limit=N&offset=M — paged slice, default unlimited /export now forwards maxSessions + offset query params to mem::export (which already supported them). Viewer dashboard caps the memories fetch at 500; the memories tab at 2000. Both stop the iii invocation timeout from masking real corpora as 0 memories. #563 viewer graph cool-down on >1000 nodes src/viewer/index.html adds tick-decayed damping (coolBoost), per-node velocity caps tiered by node count, and quiescence-based raf parking. Mousedown wakes the parked loop. Dense graphs settle instead of bouncing forever; CPU returns to idle once the layout is quiet. #637 Windows em-dash ByteString — deferred to follow-up Cannot reproduce on macOS / Linux. The user-suggested defensive encoding fix is unsafe without a Windows repro confirming the actual exception path. Will land separately once a Windows runner or the reporter can validate.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughImplements start-context caching in OpenCode plugin; worker pidfile lifecycle (write/clear) and CLI stop handling for worker reaping; implicit session creation in mem::observe when project+cwd present; /memories and /export pagination and count mode; OpenAI provider fallback to reasoning_content; viewer limits and graph cooldown/wake logic. ChangesCore functionality and infrastructure
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/viewer/index.html (1)
1584-1584: 💤 Low value
quietTicksis not initialized in graphSim state.The
tickCountis explicitly initialized to0in the graphSim object, butquietTicksis not, despite being used similarly. The code handles this with|| 0patterns, but adding it to the initial state would be more consistent.Suggested fix
-var graphSim = { nodes: [], edges: [], running: false, canvas: null, ctx: null, raf: null, panX: 0, panY: 0, zoom: 1, dragNode: null, mouseX: 0, mouseY: 0, tickCount: 0 }; +var graphSim = { nodes: [], edges: [], running: false, canvas: null, ctx: null, raf: null, panX: 0, panY: 0, zoom: 1, dragNode: null, mouseX: 0, mouseY: 0, tickCount: 0, quietTicks: 0 };🤖 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/viewer/index.html` at line 1584, graphSim's initial state is missing quietTicks; add quietTicks: 0 to the graphSim object (which already contains tickCount: 0) so code that reads graphSim.quietTicks doesn't need fallback patterns. Update the graphSim declaration (the object with nodes, edges, running, canvas, ctx, raf, panX, panY, zoom, dragNode, mouseX, mouseY, tickCount) to include quietTicks: 0.src/functions/observe.ts (1)
239-240: ⚡ Quick winReuse one captured timestamp for session create fields.
The new session object calls
new Date().toISOString()multiple times in the same write path (Line 239 and Line 240). Capture once and reuse for consistent metadata.Proposed fix
+ const nowIso = new Date().toISOString(); await kv.set(KV.sessions, payload.sessionId, { id: payload.sessionId, project: payload.project, cwd: payload.cwd, - startedAt: payload.timestamp ?? new Date().toISOString(), - updatedAt: new Date().toISOString(), + startedAt: payload.timestamp ?? nowIso, + updatedAt: nowIso, status: "active", observationCount: 1,As per coding guidelines: “Capture timestamps once with
new Date().toISOString()and reuse rather than calling repeatedly.”🤖 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/functions/observe.ts` around lines 239 - 240, Capture a single timestamp once (e.g., const ts = new Date().toISOString()) and reuse it for both session fields instead of calling new Date().toISOString() twice; set startedAt to payload.timestamp ?? ts and updatedAt to ts so startedAt/updatedAt in the new session object (the properties startedAt and updatedAt in the session creation code) remain consistent.
🤖 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 187-200: The start-context caching uses the mutable
activeSessionId after an await which can change concurrently; capture a stable
session id before calling postJson (e.g., const sessionId = activeSessionId) and
use that local sessionId for the null check and when setting startContextCache
with startCtx (referencing activeSessionId, postJson("/session/start"),
startCtx, and startContextCache) so the cached context is bound to the correct
session even if activeSessionId changes during the await.
In `@src/cli.ts`:
- Around line 2282-2284: The Docker stop paths currently return before clearing
worker.pid, so ensure clearWorkerPidfile() is always invoked in all stop
branches (native and Docker). Modify the docker stop branches in stop logic
(where the code currently returns early) to call clearWorkerPidfile() before any
return, or refactor to call clearWorkerPidfile() alongside clearEnginePidfile()
and clearEngineState() so clearWorkerPidfile() runs unconditionally regardless
of whether the Docker or native stop path is taken.
- Around line 2250-2258: The worker PID file is only read after the early-return
when the engine is down, so orphan worker processes are not reaped; call
readWorkerPidfile() (and populate workerCandidates/workerPid) before the
running/if (!running) check (or additionally call it inside that branch) so
workerCandidates is populated even when running is false, and then proceed to
merge workerCandidates with candidates and perform the reap logic using the
existing variables (readWorkerPidfile, workerPid, workerCandidates, candidates).
In `@src/viewer/index.html`:
- Around line 1763-1767: The wheel handler currently updates graphSim.zoom but
doesn't wake a parked simulation, so add the same wake logic used in the
mousedown handler: set graphSim.quietTicks = 0 and if (graphSim.running &&
!graphSim.raf) assign graphSim.raf = requestAnimationFrame(runSimulation) (or
the equivalent call path) after updating graphSim.zoom in the wheel event
handler to ensure immediate visual feedback when zooming while parked.
---
Nitpick comments:
In `@src/functions/observe.ts`:
- Around line 239-240: Capture a single timestamp once (e.g., const ts = new
Date().toISOString()) and reuse it for both session fields instead of calling
new Date().toISOString() twice; set startedAt to payload.timestamp ?? ts and
updatedAt to ts so startedAt/updatedAt in the new session object (the properties
startedAt and updatedAt in the session creation code) remain consistent.
In `@src/viewer/index.html`:
- Line 1584: graphSim's initial state is missing quietTicks; add quietTicks: 0
to the graphSim object (which already contains tickCount: 0) so code that reads
graphSim.quietTicks doesn't need fallback patterns. Update the graphSim
declaration (the object with nodes, edges, running, canvas, ctx, raf, panX,
panY, zoom, dragNode, mouseX, mouseY, tickCount) to include quietTicks: 0.
🪄 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: 289ab8c2-8ab0-4854-932c-7c980e630186
📒 Files selected for processing (13)
plugin/opencode/agentmemory-capture.tssrc/cli.tssrc/functions/observe.tssrc/index.tssrc/providers/openai.tssrc/triggers/api.tssrc/viewer/index.htmltest/fetch-timeout.test.tstest/memories-pagination.test.tstest/observe-implicit-session.test.tstest/opencode-auto-context.test.tstest/stop-worker-pidfile.test.tstest/viewer-graph-cooldown.test.ts
Addresses inline review on PR #648 — verified each finding against current code and fixed the still-valid ones. opencode plugin: snapshot activeSessionId into a local 'sessionId' before await postJson('/session/start') — a second session.created event during the await could rebind activeSessionId and cache the context against the wrong key. The cache write + observe call now use the snapshotted id. src/cli.ts: clearWorkerPidfile() now runs in every stop branch: - Docker engine-not-running early return - Docker stopDockerEngine path - native engine-not-running 'Nothing to stop' - native happy path (was already there) The worker pid is now read up front so the engine-down branch can also reap an orphaned worker process (previously fell through to 'preserve for manual cleanup'). A new dedicated branch reaps the worker and exits cleanly when only the worker is lingering. src/viewer/index.html: wakeGraphSim() shared helper consolidates the quietTicks reset + raf restart pattern. Wheel handler, zoomGraph(), recenterGraph(), and mousedown all now wake the parked simulation so zoom/pan/click feedback is immediate after the layout has settled. graphSim object initializes quietTicks: 0 alongside tickCount: 0. src/functions/observe.ts: dedupe new Date().toISOString() into a single 'ts' local for the implicit-create path so startedAt and updatedAt stay consistent. test/opencode-auto-context.test.ts: regex updated to assert the snapshot-then-cache pattern instead of the previous direct activeSessionId reference. 1119/1119 vitest pass.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/functions/observe.ts (1)
236-247: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd an audit entry for implicit session creation.
This branch writes a new
KV.sessionsrecord but does not emit an audit record for that state change.As per coding guidelines, "Register functions using
sdk.registerFunction()with validation of inputs, work via kv.get/kv.set/kv.list, and record audit entries viarecordAudit()for state-changing operations".🤖 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/functions/observe.ts` around lines 236 - 247, The new KV.sessions write (kv.set with KV.sessions and payload.sessionId) must be accompanied by an audit entry: after creating the session record in the branch that initializes a session (the code that sets id, project, cwd, startedAt, updatedAt, status:"active", observationCount:1 and optional firstPrompt), call recordAudit(...) to log the implicit session creation; include the session id (payload.sessionId), project, status "active", timestamp (ts or payload.timestamp), and any firstPrompt/trimmedPrompt in the audit metadata, and populate the actor/actorId from the request payload or execution context consistent with other recordAudit calls in this file.
🤖 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.
Outside diff comments:
In `@src/functions/observe.ts`:
- Around line 236-247: The new KV.sessions write (kv.set with KV.sessions and
payload.sessionId) must be accompanied by an audit entry: after creating the
session record in the branch that initializes a session (the code that sets id,
project, cwd, startedAt, updatedAt, status:"active", observationCount:1 and
optional firstPrompt), call recordAudit(...) to log the implicit session
creation; include the session id (payload.sessionId), project, status "active",
timestamp (ts or payload.timestamp), and any firstPrompt/trimmedPrompt in the
audit metadata, and populate the actor/actorId from the request payload or
execution context consistent with other recordAudit calls in this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae94974a-7235-44e8-be9e-bbc9860308e7
📒 Files selected for processing (5)
plugin/opencode/agentmemory-capture.tssrc/cli.tssrc/functions/observe.tssrc/viewer/index.htmltest/opencode-auto-context.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
End-to-end stability pass over six open issues. Each fix lands with a targeted regression test; full suite is 1119/1119 (104 files) on macOS Apple Silicon.
message.reasoning_contentalongsidemessage.reasoningagentmemory stopleft daemon wrapper alive → duplicate workersagentmemory stopleft engine running with stale registrationsmem::observeimplicit-creates the session whenproject + cwdpresent and no record existsAGENTS.mdinstruction for memory lookup/session/startcontext, inject via existingchat.system.transformhook (zero-config)/memories+/exporttimed out on real corpora (8K+ memories) → viewer showed0?count=true,?limit=N&offset=M, viewer caps requests at 500 / 2000>1000nodesPer-fix detail
#627 —
src/providers/openai.tsDeepSeek V4 / Qwen3 / GLM / Kimi return
message.reasoning_content(with underscore). Previous code only checkedmessage.reasoning→ silent compress failure, 0/700 calls succeeded, circuit breaker repeatedly opened.Test:
test/fetch-timeout.test.ts— adds 3 thinking-model fallback cases covering DeepSeek shape, older o-series shape, and content-wins-over-both.#640 + #474 —
src/cli.ts+src/index.tsRoot cause: the agentmemory worker process is spawned by
iii-execinside the engine.agentmemory stoponly signaled engine pids; the worker (spawneddetached, signal not propagated, or kept alive by a wrapper script) survived and reconnected to the next engine. Result: duplicate worker registrations or stale function registrations from a previous code revision.Test:
test/stop-worker-pidfile.test.ts— verifies both source files useworker.pidand thatrunStopwires it into the kill loop.#638 —
src/functions/observe.tsOpenCode plugins (and any client that skips
POST /session/start) sent observations against a session that didn't exist server-side. Observations stacked up butmemory_sessionslisted nothing, thensummarizebailed withSession not found for summarize.Now: when
mem::observeruns and no session record exists, create one inline using theHookPayload'sproject + cwd + timestamp(only when both are present, so older test payloads keep their no-op path).Test:
test/observe-implicit-session.test.ts— 3 cases (implicit-create happy path, no-op when project/cwd missing, existing session not overwritten).#431 —
plugin/opencode/agentmemory-capture.tsPOST /session/startalready returns the project context. Plugin now caches it (startContextCache) and the existingexperimental.chat.system.transformhook reads from the cache first, falling back to a fresh/contextcall if the cache missed. Result: zero-config injection without a second round-trip. Cache cleared onsession.deleted.Test:
test/opencode-auto-context.test.ts— verifies cache write at session.created, read in chat.system.transform with/contextfallback, cleanup on session.deleted.#544 —
src/triggers/api.ts+src/viewer/index.htmlUnbounded
/memoriesand/exportinvocations hit the iii engine's invocation timeout on a real corpus (40 sessions × 34K observations × 8K memories). The viewer's failed fetch silently fell through to "0 memories"./memoriesnow supports three modes:?count=true→ totals only ({total, latestCount})?limit=N&offset=M→ paged slice (cap 5000)/exportforwards?maxSessions=N&offset=Mtomem::exportwhich already supported it.Viewer dashboard caps fetches: 500 on the dashboard, 2000 on the memories tab.
Test:
test/memories-pagination.test.ts— 5 cases covering count mode, limit/offset, cap-at-5000, export passthrough, viewer caps.#563 —
src/viewer/index.htmlDense graphs (>1000 nodes) oscillated forever because 0.9 damping couldn't bleed off the per-tick force pile-up. Three changes:
coolBoost = min(0.4, tickCount / 1500)tightens damping over time so layouts settle.Test:
test/viewer-graph-cooldown.test.ts— 5 cases covering tickCount growth, coolBoost damping, velocity cap, raf park, mousedown wake.Deferred: #637 (Windows em-dash ByteString)
Cannot reproduce on macOS / Linux. The user-suggested defensive encoding fix is unsafe without a Windows repro that confirms the actual exception path. Will file a separate PR once we have a Windows runner or the reporter validates a candidate patch. Not included in this PR to keep blast radius tight.
Test plan
npm test— 1119/1119 pass (104 files), no regressionsnpm run build— 21 files, 2438 KB, no bundle bloat~/.agentmemory/on boot, removed on graceful shutdowntest/Summary by CodeRabbit
New Features
Bug Fixes
Tests