fix: resolve stale project path caching in persistent desktop editor plugin#824
Open
Chewji9875 wants to merge 1 commit into
Open
fix: resolve stale project path caching in persistent desktop editor plugin#824Chewji9875 wants to merge 1 commit into
Chewji9875 wants to merge 1 commit into
Conversation
|
@Chewji9875 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR refactors project path resolution in the agentmemory capture plugin to derive ChangesDynamic Project Resolution in AgentMemory Plugin
Sequence Diagram(s)No sequence diagram generated; this is a localized refactoring within a single plugin file that consolidates project resolution logic without introducing new component interactions or multi-actor flows. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugin/opencode/agentmemory-capture.ts (1)
613-619:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix cache-miss project resolution in
experimental.chat.system.transform(shadowedctx).
plugin/opencode/agentmemory-capture.tslines 613-619: the cache-miss branch usesctx.worktree/ctx.project?.id, butctxthere is the cached value fromstartContextCache.get(sid)(string/undefined), so on a miss it can throw before/contextis fetched. Rename the cached variable (e.g.,cachedContext) and computeprojectfrom the outer pluginctx(or store project data alongside the cached context) with a safe fallback toprocess.cwd().🤖 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 613 - 619, The cache-miss branch in experimental.chat.system.transform incorrectly reuses the cached variable name `ctx` (from startContextCache.get(sid)) causing access to `ctx.worktree`/`ctx.project?.id` on a string/undefined and throwing before postJson("/context") runs; rename the cached value (e.g., `cachedContext`) and when making the /context request compute the `project` from the outer plugin `ctx` (the real context) or include project metadata in the cached entry, falling back to process.cwd() via resolveProject(), so the call in startContextCache handling uses the correct source for worktree/project instead of the cached string.src/functions/compress.ts (1)
74-79:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd runtime payload validation before accessing
data.rawand identifiers.The handler dereferences
data.raw,data.observationId, anddata.sessionIdimmediately; malformed trigger payloads can crash this function before any fallback behavior executes.Suggested patch
sdk.registerFunction("mem::compress", async (data: { observationId: string; sessionId: string; raw: RawObservation; }) => { + if ( + !data || + typeof data.observationId !== "string" || + typeof data.sessionId !== "string" || + !data.raw + ) { + logger.warn("Invalid mem::compress payload"); + return { success: false, error: "invalid_payload" }; + } + const startMs = Date.now();As per coding guidelines:
src/**/*.tsrequiressdk.registerFunctionhandlers with validation of inputs.Also applies to: 82-90
🤖 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/compress.ts` around lines 74 - 79, The handler registered as sdk.registerFunction("mem::compress") is dereferencing data.raw, data.observationId, and data.sessionId immediately; add runtime payload validation at the start of that async handler to guard against malformed triggers by: verify that data is an object, that observationId and sessionId are non-empty strings, and that raw matches the expected shape (or at least is an object/has required keys) before any access; if validation fails, return or throw a controlled error (or call the existing fallback path) instead of proceeding, and use the handler name (mem::compress) in log/error messages to aid debugging.test/auto-compress.test.ts (1)
82-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe “unset default” test is no longer testing unset behavior.
Line 82 sets
AGENTMEMORY_AUTO_COMPRESSto"false"inbeforeEach, so the test at Line 88 does not exercise the unset/default path despite its name and intent.Suggested patch
it("default (AGENTMEMORY_AUTO_COMPRESS unset): does NOT fire mem::compress", async () => { + delete process.env["AGENTMEMORY_AUTO_COMPRESS"]; const { registerObserveFunction } = await import( "../src/functions/observe.js" );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/auto-compress.test.ts` around lines 82 - 89, The test named "default (AGENTMEMORY_AUTO_COMPRESS unset): does NOT fire mem::compress" is broken because beforeEach/afterEach set AGENTMEMORY_AUTO_COMPRESS to "false" so the unset path isn't exercised; change the setup to actually unset the env var (remove or delete process.env["AGENTMEMORY_AUTO_COMPRESS"] in beforeEach/afterEach) or explicitly clear it at the start of that specific test so the import/registerObserveFunction flow executes with AGENTMEMORY_AUTO_COMPRESS undefined; look for the beforeEach/afterEach that manipulate process.env and the test containing the registerObserveFunction import to apply the fix.
🧹 Nitpick comments (1)
src/functions/compress.ts (1)
93-155: ⚡ Quick winExtract shared persist/index/stream publish flow to prevent branch drift.
The noop and non-noop paths duplicate the same write/index/publish logic. A shared helper will reduce divergence risk and keep future changes consistent.
Also applies to: 256-317
🤖 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/compress.ts` around lines 93 - 155, Extract the repeated persist/index/publish sequence (kv.set with KV.observations, getSearchIndex().add, vectorIndexAddGuarded, the two sdk.trigger calls to "stream::set" and "stream::send", and the streamResults rejection logging) into a single helper (e.g., persistAndPublishCompressedObservation) that accepts the synthetic observation and original data/session identifiers; replace both the noop and non-noop branches with a call to that helper so all writes, BM25 indexing, vector indexing, and stream publishing (including the existing try/catch and logger.warn behavior) are centralized and reused to avoid branch drift.
🤖 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 `@src/functions/compress.ts`:
- Around line 82-164: The noop fallback branch (the block handling provider.name
=== "noop" that calls buildSyntheticCompression, kv.set, getSearchIndex().add,
vectorIndexAddGuarded, and sdk.trigger) must be wrapped in a try/catch so any
rejected promise (e.g., kv.set) doesn't throw out of the mem::compress handler;
surround the entire noop branch logic in a single try block and in the catch log
the error via logger.warn/error (include err.message or String(err)) and return
a structured failure response like { success: false, error: String(err) } so the
handler remains stable and future awaits in this branch are handled
consistently.
In `@src/providers/embedding/openrouter.ts`:
- Around line 20-25: The current parsing of OPENROUTER_EMBEDDING_DIMENSIONS
using parseInt on dimStr is too permissive (e.g. "1.5" or "2048abc")—replace the
loose parse with a strict positive-integer string check: validate dimStr (from
getEnvVar) against a regex like /^\s*[1-9]\d*\s*$/ (or trim and use /^\d+$/ then
ensure >0), throw the same Error if it fails, and only then safely parse with
Number.parseInt(dimStr, 10) (or Number) to set the dimensions value; update the
logic around the parsed/parsed variable used later so only strictly valid
positive integers are accepted.
---
Outside diff comments:
In `@plugin/opencode/agentmemory-capture.ts`:
- Around line 613-619: The cache-miss branch in
experimental.chat.system.transform incorrectly reuses the cached variable name
`ctx` (from startContextCache.get(sid)) causing access to
`ctx.worktree`/`ctx.project?.id` on a string/undefined and throwing before
postJson("/context") runs; rename the cached value (e.g., `cachedContext`) and
when making the /context request compute the `project` from the outer plugin
`ctx` (the real context) or include project metadata in the cached entry,
falling back to process.cwd() via resolveProject(), so the call in
startContextCache handling uses the correct source for worktree/project instead
of the cached string.
In `@src/functions/compress.ts`:
- Around line 74-79: The handler registered as
sdk.registerFunction("mem::compress") is dereferencing data.raw,
data.observationId, and data.sessionId immediately; add runtime payload
validation at the start of that async handler to guard against malformed
triggers by: verify that data is an object, that observationId and sessionId are
non-empty strings, and that raw matches the expected shape (or at least is an
object/has required keys) before any access; if validation fails, return or
throw a controlled error (or call the existing fallback path) instead of
proceeding, and use the handler name (mem::compress) in log/error messages to
aid debugging.
In `@test/auto-compress.test.ts`:
- Around line 82-89: The test named "default (AGENTMEMORY_AUTO_COMPRESS unset):
does NOT fire mem::compress" is broken because beforeEach/afterEach set
AGENTMEMORY_AUTO_COMPRESS to "false" so the unset path isn't exercised; change
the setup to actually unset the env var (remove or delete
process.env["AGENTMEMORY_AUTO_COMPRESS"] in beforeEach/afterEach) or explicitly
clear it at the start of that specific test so the
import/registerObserveFunction flow executes with AGENTMEMORY_AUTO_COMPRESS
undefined; look for the beforeEach/afterEach that manipulate process.env and the
test containing the registerObserveFunction import to apply the fix.
---
Nitpick comments:
In `@src/functions/compress.ts`:
- Around line 93-155: Extract the repeated persist/index/publish sequence
(kv.set with KV.observations, getSearchIndex().add, vectorIndexAddGuarded, the
two sdk.trigger calls to "stream::set" and "stream::send", and the streamResults
rejection logging) into a single helper (e.g.,
persistAndPublishCompressedObservation) that accepts the synthetic observation
and original data/session identifiers; replace both the noop and non-noop
branches with a call to that helper so all writes, BM25 indexing, vector
indexing, and stream publishing (including the existing try/catch and
logger.warn behavior) are centralized and reused to avoid branch drift.
🪄 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: d35a2129-721a-46ec-8437-e2a4f0352eca
📒 Files selected for processing (5)
plugin/opencode/agentmemory-capture.tssrc/functions/compress.tssrc/providers/embedding/openrouter.tstest/auto-compress.test.tstest/embedding-provider.test.ts
Comment on lines
+82
to
+164
| if (provider.name === "noop") { | ||
| logger.info("Compression skipped (noop provider) — generating synthetic compression", { | ||
| obsId: data.observationId, | ||
| }); | ||
| const synthetic = buildSyntheticCompression(data.raw); | ||
| synthetic.id = data.observationId; | ||
| synthetic.sessionId = data.sessionId; | ||
| if (data.raw.timestamp) { | ||
| synthetic.timestamp = data.raw.timestamp; | ||
| } | ||
|
|
||
| await kv.set( | ||
| KV.observations(data.sessionId), | ||
| data.observationId, | ||
| synthetic, | ||
| ); | ||
|
|
||
| try { | ||
| getSearchIndex().add(synthetic); | ||
| } catch (err) { | ||
| logger.warn("Failed to index compressed observation into BM25", { | ||
| obsId: synthetic.id, | ||
| sessionId: synthetic.sessionId, | ||
| title: synthetic.title, | ||
| error: err instanceof Error ? err.message : String(err), | ||
| }); | ||
| } | ||
|
|
||
| await vectorIndexAddGuarded( | ||
| synthetic.id, | ||
| synthetic.sessionId, | ||
| synthetic.title + " " + (synthetic.narrative || ""), | ||
| { kind: "observation", logId: synthetic.id }, | ||
| ); | ||
|
|
||
| const streamResults = await Promise.allSettled([ | ||
| sdk.trigger({ | ||
| function_id: "stream::set", | ||
| payload: { | ||
| stream_name: STREAM.name, | ||
| group_id: STREAM.group(data.sessionId), | ||
| item_id: data.observationId, | ||
| data: { type: "compressed", observation: synthetic }, | ||
| }, | ||
| }), | ||
| sdk.trigger({ | ||
| function_id: "stream::send", | ||
| payload: { | ||
| stream_name: STREAM.name, | ||
| group_id: STREAM.viewerGroup, | ||
| id: `compressed-${data.observationId}`, | ||
| type: "compressed_observation", | ||
| data: { | ||
| type: "compressed", | ||
| observation: synthetic, | ||
| sessionId: data.sessionId, | ||
| }, | ||
| }, | ||
| action: TriggerAction.Void(), | ||
| }), | ||
| ]); | ||
|
|
||
| for (const res of streamResults) { | ||
| if (res.status === "rejected") { | ||
| logger.warn("Non-fatal stream publish failure after compress", { | ||
| sessionId: data.sessionId, | ||
| observationId: data.observationId, | ||
| error: | ||
| res.reason instanceof Error | ||
| ? res.reason.message | ||
| : String(res.reason), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| logger.info("Observation compressed (synthetic noop fallback)", { | ||
| obsId: data.observationId, | ||
| type: synthetic.type, | ||
| importance: synthetic.importance, | ||
| }); | ||
|
|
||
| return { success: true, compressed: synthetic, qualityScore: synthetic.confidence * 100 }; | ||
| } |
Contributor
There was a problem hiding this comment.
Wrap the noop fallback path in error handling to preserve handler stability.
kv.set at Line 93 (and any future awaited call in this branch) can reject, but this branch is outside the try/catch that starts at Line 207. That makes mem::compress fail by throwing instead of returning a structured { success: false, error: ... } response.
Suggested patch
if (provider.name === "noop") {
- logger.info("Compression skipped (noop provider) — generating synthetic compression", {
- obsId: data.observationId,
- });
- const synthetic = buildSyntheticCompression(data.raw);
- synthetic.id = data.observationId;
- synthetic.sessionId = data.sessionId;
- if (data.raw.timestamp) {
- synthetic.timestamp = data.raw.timestamp;
- }
-
- await kv.set(
- KV.observations(data.sessionId),
- data.observationId,
- synthetic,
- );
+ try {
+ logger.info("Compression skipped (noop provider) — generating synthetic compression", {
+ obsId: data.observationId,
+ });
+ const synthetic = buildSyntheticCompression(data.raw);
+ synthetic.id = data.observationId;
+ synthetic.sessionId = data.sessionId;
+ if (data.raw.timestamp) {
+ synthetic.timestamp = data.raw.timestamp;
+ }
+
+ await kv.set(
+ KV.observations(data.sessionId),
+ data.observationId,
+ synthetic,
+ );
- try {
- getSearchIndex().add(synthetic);
- } catch (err) {
- logger.warn("Failed to index compressed observation into BM25", {
- obsId: synthetic.id,
- sessionId: synthetic.sessionId,
- title: synthetic.title,
- error: err instanceof Error ? err.message : String(err),
- });
- }
+ try {
+ getSearchIndex().add(synthetic);
+ } catch (err) {
+ logger.warn("Failed to index compressed observation into BM25", {
+ obsId: synthetic.id,
+ sessionId: synthetic.sessionId,
+ title: synthetic.title,
+ error: err instanceof Error ? err.message : String(err),
+ });
+ }
- await vectorIndexAddGuarded(
- synthetic.id,
- synthetic.sessionId,
- synthetic.title + " " + (synthetic.narrative || ""),
- { kind: "observation", logId: synthetic.id },
- );
+ await vectorIndexAddGuarded(
+ synthetic.id,
+ synthetic.sessionId,
+ synthetic.title + " " + (synthetic.narrative || ""),
+ { kind: "observation", logId: synthetic.id },
+ );
- const streamResults = await Promise.allSettled([
- sdk.trigger({
- function_id: "stream::set",
- payload: {
- stream_name: STREAM.name,
- group_id: STREAM.group(data.sessionId),
- item_id: data.observationId,
- data: { type: "compressed", observation: synthetic },
- },
- }),
- sdk.trigger({
- function_id: "stream::send",
- payload: {
- stream_name: STREAM.name,
- group_id: STREAM.viewerGroup,
- id: `compressed-${data.observationId}`,
- type: "compressed_observation",
- data: {
- type: "compressed",
- observation: synthetic,
- sessionId: data.sessionId,
- },
- },
- action: TriggerAction.Void(),
- }),
- ]);
+ const streamResults = await Promise.allSettled([
+ sdk.trigger({
+ function_id: "stream::set",
+ payload: {
+ stream_name: STREAM.name,
+ group_id: STREAM.group(data.sessionId),
+ item_id: data.observationId,
+ data: { type: "compressed", observation: synthetic },
+ },
+ }),
+ sdk.trigger({
+ function_id: "stream::send",
+ payload: {
+ stream_name: STREAM.name,
+ group_id: STREAM.viewerGroup,
+ id: `compressed-${data.observationId}`,
+ type: "compressed_observation",
+ data: {
+ type: "compressed",
+ observation: synthetic,
+ sessionId: data.sessionId,
+ },
+ },
+ action: TriggerAction.Void(),
+ }),
+ ]);
- for (const res of streamResults) {
- if (res.status === "rejected") {
- logger.warn("Non-fatal stream publish failure after compress", {
- sessionId: data.sessionId,
- observationId: data.observationId,
- error:
- res.reason instanceof Error
- ? res.reason.message
- : String(res.reason),
- });
+ for (const res of streamResults) {
+ if (res.status === "rejected") {
+ logger.warn("Non-fatal stream publish failure after compress", {
+ sessionId: data.sessionId,
+ observationId: data.observationId,
+ error:
+ res.reason instanceof Error
+ ? res.reason.message
+ : String(res.reason),
+ });
+ }
}
- }
- logger.info("Observation compressed (synthetic noop fallback)", {
- obsId: data.observationId,
- type: synthetic.type,
- importance: synthetic.importance,
- });
+ logger.info("Observation compressed (synthetic noop fallback)", {
+ obsId: data.observationId,
+ type: synthetic.type,
+ importance: synthetic.importance,
+ });
- return { success: true, compressed: synthetic, qualityScore: synthetic.confidence * 100 };
+ return { success: true, compressed: synthetic, qualityScore: synthetic.confidence * 100 };
+ } catch (err) {
+ const msg = err instanceof Error ? err.message : String(err);
+ logger.error("Synthetic noop compression failed", {
+ obsId: data.observationId,
+ error: msg,
+ });
+ return { success: false, error: "compression_failed" };
+ }
}📝 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.
Suggested change
| if (provider.name === "noop") { | |
| logger.info("Compression skipped (noop provider) — generating synthetic compression", { | |
| obsId: data.observationId, | |
| }); | |
| const synthetic = buildSyntheticCompression(data.raw); | |
| synthetic.id = data.observationId; | |
| synthetic.sessionId = data.sessionId; | |
| if (data.raw.timestamp) { | |
| synthetic.timestamp = data.raw.timestamp; | |
| } | |
| await kv.set( | |
| KV.observations(data.sessionId), | |
| data.observationId, | |
| synthetic, | |
| ); | |
| try { | |
| getSearchIndex().add(synthetic); | |
| } catch (err) { | |
| logger.warn("Failed to index compressed observation into BM25", { | |
| obsId: synthetic.id, | |
| sessionId: synthetic.sessionId, | |
| title: synthetic.title, | |
| error: err instanceof Error ? err.message : String(err), | |
| }); | |
| } | |
| await vectorIndexAddGuarded( | |
| synthetic.id, | |
| synthetic.sessionId, | |
| synthetic.title + " " + (synthetic.narrative || ""), | |
| { kind: "observation", logId: synthetic.id }, | |
| ); | |
| const streamResults = await Promise.allSettled([ | |
| sdk.trigger({ | |
| function_id: "stream::set", | |
| payload: { | |
| stream_name: STREAM.name, | |
| group_id: STREAM.group(data.sessionId), | |
| item_id: data.observationId, | |
| data: { type: "compressed", observation: synthetic }, | |
| }, | |
| }), | |
| sdk.trigger({ | |
| function_id: "stream::send", | |
| payload: { | |
| stream_name: STREAM.name, | |
| group_id: STREAM.viewerGroup, | |
| id: `compressed-${data.observationId}`, | |
| type: "compressed_observation", | |
| data: { | |
| type: "compressed", | |
| observation: synthetic, | |
| sessionId: data.sessionId, | |
| }, | |
| }, | |
| action: TriggerAction.Void(), | |
| }), | |
| ]); | |
| for (const res of streamResults) { | |
| if (res.status === "rejected") { | |
| logger.warn("Non-fatal stream publish failure after compress", { | |
| sessionId: data.sessionId, | |
| observationId: data.observationId, | |
| error: | |
| res.reason instanceof Error | |
| ? res.reason.message | |
| : String(res.reason), | |
| }); | |
| } | |
| } | |
| logger.info("Observation compressed (synthetic noop fallback)", { | |
| obsId: data.observationId, | |
| type: synthetic.type, | |
| importance: synthetic.importance, | |
| }); | |
| return { success: true, compressed: synthetic, qualityScore: synthetic.confidence * 100 }; | |
| } | |
| if (provider.name === "noop") { | |
| try { | |
| logger.info("Compression skipped (noop provider) — generating synthetic compression", { | |
| obsId: data.observationId, | |
| }); | |
| const synthetic = buildSyntheticCompression(data.raw); | |
| synthetic.id = data.observationId; | |
| synthetic.sessionId = data.sessionId; | |
| if (data.raw.timestamp) { | |
| synthetic.timestamp = data.raw.timestamp; | |
| } | |
| await kv.set( | |
| KV.observations(data.sessionId), | |
| data.observationId, | |
| synthetic, | |
| ); | |
| try { | |
| getSearchIndex().add(synthetic); | |
| } catch (err) { | |
| logger.warn("Failed to index compressed observation into BM25", { | |
| obsId: synthetic.id, | |
| sessionId: synthetic.sessionId, | |
| title: synthetic.title, | |
| error: err instanceof Error ? err.message : String(err), | |
| }); | |
| } | |
| await vectorIndexAddGuarded( | |
| synthetic.id, | |
| synthetic.sessionId, | |
| synthetic.title + " " + (synthetic.narrative || ""), | |
| { kind: "observation", logId: synthetic.id }, | |
| ); | |
| const streamResults = await Promise.allSettled([ | |
| sdk.trigger({ | |
| function_id: "stream::set", | |
| payload: { | |
| stream_name: STREAM.name, | |
| group_id: STREAM.group(data.sessionId), | |
| item_id: data.observationId, | |
| data: { type: "compressed", observation: synthetic }, | |
| }, | |
| }), | |
| sdk.trigger({ | |
| function_id: "stream::send", | |
| payload: { | |
| stream_name: STREAM.name, | |
| group_id: STREAM.viewerGroup, | |
| id: `compressed-${data.observationId}`, | |
| type: "compressed_observation", | |
| data: { | |
| type: "compressed", | |
| observation: synthetic, | |
| sessionId: data.sessionId, | |
| }, | |
| }, | |
| action: TriggerAction.Void(), | |
| }), | |
| ]); | |
| for (const res of streamResults) { | |
| if (res.status === "rejected") { | |
| logger.warn("Non-fatal stream publish failure after compress", { | |
| sessionId: data.sessionId, | |
| observationId: data.observationId, | |
| error: | |
| res.reason instanceof Error | |
| ? res.reason.message | |
| : String(res.reason), | |
| }); | |
| } | |
| } | |
| logger.info("Observation compressed (synthetic noop fallback)", { | |
| obsId: data.observationId, | |
| type: synthetic.type, | |
| importance: synthetic.importance, | |
| }); | |
| return { success: true, compressed: synthetic, qualityScore: synthetic.confidence * 100 }; | |
| } catch (err) { | |
| const msg = err instanceof Error ? err.message : String(err); | |
| logger.error("Synthetic noop compression failed", { | |
| obsId: data.observationId, | |
| error: msg, | |
| }); | |
| return { success: false, error: "compression_failed" }; | |
| } | |
| } |
🤖 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/compress.ts` around lines 82 - 164, The noop fallback branch
(the block handling provider.name === "noop" that calls
buildSyntheticCompression, kv.set, getSearchIndex().add, vectorIndexAddGuarded,
and sdk.trigger) must be wrapped in a try/catch so any rejected promise (e.g.,
kv.set) doesn't throw out of the mem::compress handler; surround the entire noop
branch logic in a single try block and in the catch log the error via
logger.warn/error (include err.message or String(err)) and return a structured
failure response like { success: false, error: String(err) } so the handler
remains stable and future awaits in this branch are handled consistently.
Comment on lines
+20
to
+25
| const dimStr = getEnvVar("OPENROUTER_EMBEDDING_DIMENSIONS"); | ||
| if (dimStr !== undefined && dimStr.trim().length > 0) { | ||
| const parsed = parseInt(dimStr, 10); | ||
| if (!Number.isFinite(parsed) || parsed <= 0) { | ||
| throw new Error( | ||
| `OPENROUTER_EMBEDDING_DIMENSIONS must be a positive integer, got: ${dimStr}`, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript, what do parseInt("1.5", 10) and parseInt("2048abc", 10) return, and why can this be unsafe for strict integer env validation?
💡 Result:
parseInt("1.5", 10) returns 1.[1] parseInt("2048abc", 10) returns 2048.[1] Why this can be unsafe for strict integer validation: parseInt parses as much of the leading string as it can form a valid integer in the given radix, and when it encounters a character that is not a valid numeral for that radix, it ignores that character and everything after it (rather than failing with NaN). MDN explicitly notes that “If parseInt encounters a character in the input string that is not a valid numeral in the specified radix, it ignores it and all succeeding characters and returns the integer value parsed up to that point,” and gives examples including that “because. is not a numeral either, the return value will always be an integer.”[1] Thus, “1.5” is truncated at the decimal point to “1”, and “2048abc” is truncated at the first non-numeric character (“a”) to “2048”, even though the original strings are not strict integers.
Citations:
src/providers/embedding/openrouter.ts: parse OPENROUTER_EMBEDDING_DIMENSIONS with strict positive-integer validation
parseInt(dimStr, 10) is too permissive for “positive integer” checks: e.g. parseInt("1.5", 10) === 1 and parseInt("2048abc", 10) === 2048, so invalid env values can silently pass.
Proposed fix
- const dimStr = getEnvVar("OPENROUTER_EMBEDDING_DIMENSIONS");
- if (dimStr !== undefined && dimStr.trim().length > 0) {
- const parsed = parseInt(dimStr, 10);
- if (!Number.isFinite(parsed) || parsed <= 0) {
+ const dimStr = getEnvVar("OPENROUTER_EMBEDDING_DIMENSIONS");
+ if (dimStr !== undefined && dimStr.trim().length > 0) {
+ const parsed = Number(dimStr);
+ if (!Number.isInteger(parsed) || parsed <= 0) {
throw new Error(
`OPENROUTER_EMBEDDING_DIMENSIONS must be a positive integer, got: ${dimStr}`,
);
}
this.dimensions = parsed;📝 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.
Suggested change
| const dimStr = getEnvVar("OPENROUTER_EMBEDDING_DIMENSIONS"); | |
| if (dimStr !== undefined && dimStr.trim().length > 0) { | |
| const parsed = parseInt(dimStr, 10); | |
| if (!Number.isFinite(parsed) || parsed <= 0) { | |
| throw new Error( | |
| `OPENROUTER_EMBEDDING_DIMENSIONS must be a positive integer, got: ${dimStr}`, | |
| const dimStr = getEnvVar("OPENROUTER_EMBEDDING_DIMENSIONS"); | |
| if (dimStr !== undefined && dimStr.trim().length > 0) { | |
| const parsed = Number(dimStr); | |
| if (!Number.isInteger(parsed) || parsed <= 0) { | |
| throw new Error( | |
| `OPENROUTER_EMBEDDING_DIMENSIONS must be a positive integer, got: ${dimStr}`, | |
| ); | |
| } | |
| this.dimensions = parsed; |
🤖 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/providers/embedding/openrouter.ts` around lines 20 - 25, The current
parsing of OPENROUTER_EMBEDDING_DIMENSIONS using parseInt on dimStr is too
permissive (e.g. "1.5" or "2048abc")—replace the loose parse with a strict
positive-integer string check: validate dimStr (from getEnvVar) against a regex
like /^\s*[1-9]\d*\s*$/ (or trim and use /^\d+$/ then ensure >0), throw the same
Error if it fails, and only then safely parse with Number.parseInt(dimStr, 10)
(or Number) to set the dimensions value; update the logic around the
parsed/parsed variable used later so only strictly valid positive integers are
accepted.
Chewji9875
changed the title
Chewji9875
changed the title
Problem: OpenCode Desktop sessions recorded with stale projectPath, attributing memories to openstellar-mcp-adapter instead of current project (e.g., oh-my-openagent). User-visible: sessions in viewer appear under wrong project.
Root Cause: plugin/opencode/agentmemory-capture.ts globally cached projectPath at module scope. Persistent Node.js plugin process meant projectPath wasn't updated on project switches.
Solution:
1. Removed global projectPath variable and its initial assignment.
2. Imported resolveProject from src/hooks/_project.ts.
3. Dynamically resolved projectPath in session.created handler and observe() function using resolveProject().
Verification:
- Manual testing:
1. Start OpenCode Desktop.
2. Open project-A, perform actions.
3. Switch to project-B, perform actions.
4. Verify in agentmemory viewer that sessions for project-A and project-B are correctly attributed.
- Code review confirmed removal of global state and dynamic resolution.
Chewji9875
force-pushed
the
fix/opencode-project-path
branch
from
b2f4949 to
d8d7734
Compare
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.
Problem
When utilizing persistent desktop editor integrations (running as long-lived Node.js plugin processes), sessions are sometimes recorded with a stale
projectPath(e.g., cached from a previous workspace or adaptation context). This causes memories to be erroneously logged under incorrect project directories, even after switching workspaces in the active editor.Root Cause
The capture connector cached
projectPathglobally at the module scope on initialization. In multi-project, long-lived workspace sessions, this value was never updated to reflect the active directory of the current session.Solution
projectPathstate variables.resolveProjectfromsrc/hooks/_project.ts.Verification
Verifications performed to guarantee robustness:
Summary by CodeRabbit
Bug Fixes
Refactor