fix: ignore non-object memory metadata

[compoodment]

Summary

• Treat non-object metadataJson values in the memory runtime bridge as missing metadata.
• Add a regression test for JSON null metadata so matching memory search results still return instead of throwing.

Quality: Q5 proven

Evidence:

• Current upstream throws Cannot read properties of null (reading 'collection') when metadataJson decodes to JSON null.
• The failure is deterministic in test/unit/memory-runtime.test.ts before the fix.
• The fix is limited to parseMetadataJson() and preserves malformed metadata as best-effort {}.

Scope:

• Trigger: daemon/search result returns syntactically valid but non-object metadataJson.
• Impact: the memory runtime bridge can fail a public memory search path instead of returning the matching hit.
• Security: not a security claim; this is robustness/availability for recall.

Verification

corepack pnpm@9 exec tsc -p tsconfig.tests.json && node --test .ts-build/test/unit/memory-runtime.test.js
corepack pnpm@9 run test:ts

Regression proof before fix:

TypeError: Cannot read properties of null (reading 'collection')

After fix:

memory runtime bridge treats non-object metadata JSON as missing
18/18 focused memory-runtime tests passed
218/218 unit tests passed

– Vale

[coderabbitai]

Warning

Review limit reached

@compoodment, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 20 minutes and 35 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c1622b30-74f0-47b4-a7f5-aaaa05288e45

📥 Commits

Reviewing files that changed from the base of the PR and between 96b7694 and 30b7340.

📒 Files selected for processing (2)

• src/memory-runtime.ts
• test/unit/memory-runtime.test.ts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[compoodment]

Vale review — PR #346

Quality: Q4/5 sharp

Finding: The fix is correct and narrow. parseMetadataJson now validates that the parsed value is a non-null, non-array object before returning it. On null, 42, "string", true, or array metadata, it falls through to {}, which all callers handle safely — typeof meta.collection === "string" defaults to "memory", no throw path.

Evidence:

• Before fix: JSON.parse("null") → null → null.collection → TypeError. Deterministic, public search path.
• After fix: null → falls through typeof parsed === "object" check → returns {} → downstream safe.
• Test covers JSON null explicitly; 33 additions, 1 deletion, 2 files changed.
• CodeRabbit CI: pass.

Proof gaps / minor notes:

1. typeof null === "object" — the fix guards against this with parsed &&, which is correct. But the !Array.isArray(parsed) guard is also important: JSON arrays are typeof "object" and would pass without it. Good that it's there.
2. No linked issue. This bug has no tracking issue. If there are other callers or downstream consumers that assume metadataJson always decodes to a record (e.g., the daemon side), this PR only fixes the runtime bridge. Worth noting in the PR body or opening an issue for daemon-side validation.
3. Test coverage covers null but not the other non-object cases (42, true, "string", []). These all hit the same code path and fall through to {}, which is correct, but explicit test rows for number/boolean/string/array metadata would make the guard visibly exercised. Low priority since the path is the same.

Verdict: Request changes are not warranted — the fix is correct, narrow, and tested for the reported failure. The minor gaps (missing linked issue, incomplete non-object test coverage) are suggestions, not blockers.

Action: Approve-equivalent. The two minor suggestions above can be addressed or deferred at maintainer discretion.

– Vale

[compoodment]

Vale Review

Quality: Q5/5 — proven
Name: Fix: ignore non-object memory metadata

Evidence:

• Current upstream parseMetadataJson() returns the raw JSON.parse() result. When metadataJson decodes to null, undefined, a number, or a string, the caller accesses .collection on it and throws Cannot read properties of null (reading 'collection').
• The fix adds a type guard: if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) — non-objects fall through to the existing catch/missing path and return {}.
• Regression test covers null metadata and confirms matching results still return.

Diff review:

• src/memory-runtime.ts: parseMetadataJson() now validates the parsed value is a non-null, non-array object before returning. Malformed metadata gets best-effort {}. Clean, narrow, no side effects.
• test/unit/memory-runtime.test.ts: New test "memory runtime bridge treats non-object metadata JSON as missing" with metadataJson: new TextEncoder().encode("null"). Confirms result count and snippet survive. Good.

Overlaps with PR #342: PR #342 also handles parseGrepMetadata() returning null/non-object in the grep path. PR #346 fixes the broader parseMetadataJson() in the search-result path. These are complementary — #342 covers grep metadata, #346 covers search-result metadata. Both should merge.

Verdict: Approve. Narrow, proven, tested, no blast radius.

– Vale