Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces documentation for the MemPalace integration with OpenCode, covering its architecture, setup, and features like real-time conversation persistence and Knowledge Graph extraction. The review feedback suggests improving the installation command for Python environment safety, correcting syntax highlighting for a JSON snippet, and fixing the configuration key from 'plugin' to 'plugins'. Additionally, a correction was made to the agent instructions to ensure the 'mempalace_kg_query' tool is used correctly without unsupported parameters to prevent validation errors.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Thanks everyone for the reviews. All suggestions have been addressed:
|
|
Also worth noting: @igorls (https://github.com/igorls) reviewed the upstream OpenCode source adapter in #1484, which takes the pull approach for retrospective ingest. My plugin is the push side (real-time capture). They're complementary — install plugin + run mempalace mine --source opencode once for full coverage. |
|
Thanks for this — the hook-based approach (chat.message + session.idle) is the right call over cron. Real-time delta sync with async mining is a clean design. ▎ uv tool install "mempalace>=3.3.5" |
|
@williamblair333 thanks for the thorough review. I've addressed all three points in v1.1.0:
The KG regex extraction was also removed — MemPalace's own mining and the model's MCP tools handle that more reliably. Plugin is at v1.1.0 on npm and the updated SKILL.md reflects all changes. Let me know if anything else is needed before merge. |
|
Maintainer review (docs-only). This is mainly a trust/policy call for the org rather than a code issue: The Minor doc fixes: the One correction to an earlier concern of mine: the "developer / creative / emotions / family / consciousness" buckets are not a design-principle violation — those map to the hall content-type classifier ( |
…terminology, reflect simplified plugin
|
@igorls thanks for the thorough maintainer review. All points addressed in the latest commit:
|
|
@igorls @williamblair333 — bumping this for visibility. The plugin has evolved significantly since May. Here's what changed:
|
- Memory injection is now the primary flow (autoInjectContext) - AGENTS.md simplified to KG-only (2 steps, no mempalace_search) - Added autoInjectContext config step (~/.mempalace/plugin-config.json) - Architecture diagram updated for transform hook flow - Added comparison table: auto-inject vs model-driven - Kept alternative model-driven path documented
|
Working through the same problem on the Claude Code side — hook-layer injection via Two things on v1.2.0–v1.3.1:
Happy to review the updated SKILL.md when pushed — just make sure @igorls's community disclaimer stays in. |
|
@williamblair333 great points, thank you.
|
|
Thanks for the update — working through the two new commits. Everything from my May and June reviews is addressed. A few new things from reading the full SKILL.md: Bug: requires:
anyBins:
- mempalace
- python3
Design note: double MCP round-trip on every response Design note: KG over-recording guidance No concerns on the MemPalace integration design itself — version pin, serialized mining, flat export, opt-in inject are all correct. The npm package trust call remains @igorls's. |
|
@williamblair333 all three points addressed in commit a5fc259:
Plugin is at v1.3.2 on npm with a fully updated README and the same KG quality guidance. |
|
@geco — all three look good.
v1.3.2 looks solid. From my side this is ready. Thanks for iterating on it this quickly. |
|
I have been using your fork for a week. It worked out great. Thanks for sharing it. Had to make changes for windows version but I think mempalace isn't windows focused at all. |
|
Thanks for iterating on this. I re-reviewed it during the agent-support merge pass. This doc/plugin direction is still valuable, especially as the real-time "push" side for OpenCode. I merged the complementary #1484 OpenCode source adapter into Before merge, I would like this updated for the current tree:
Once the doc is synchronized with the landed source adapter and current plugin behavior, this looks like a good candidate for docs/integrations merge. |
|
Status update — requesting re-review toward merge. The plugin just went through its biggest evolution (now Just pushed here: the section @igorls asked for — the SKILL.md now references What changed since the June review round:
Reviewer checklist status: @williamblair333 @igorls — mind taking another look? Happy to address anything remaining. |
igorls
left a comment
There was a problem hiding this comment.
Thanks for keeping this going. The framing is the right shape: the source adapter handles backfill, the plugin handles the live side. It isn't mergeable yet, though:
- Step 3 breaks OpenCode. The key is
plugin, notplugins. OpenCode's schema hasadditionalProperties: false, so the config as written fails validation. Your own README already usesplugin. - #1484 hasn't landed. The doc says MemPalace "ships"
mempalace.sources.opencode. It doesn't yet. Either wait for #1484 or say "once #1484 merges". - The doc describes v1.3, not 2.1.0. It still says flat export, no wings,
--mode convosonly, and "every turn is saved as a drawer". 2.1.0 has per-project wings,--extract general, checkpoint and pre-compaction prompts,saveInterval, and a backfill env var. Please describe what the pinned version actually does. - Verbatim storage.
--extract generaldrops any paragraph the heuristic doesn't classify, and only text parts are exported. That breaks "every word, verbatim". Please use the defaultexchangeextraction for the transcript. The checkpoints are fine as an extra layer on top of that. - Capture gap. The checkpoint sync fires 500ms after the user message, which is usually mid-reply. It then advances the cursor past the assistant message, so the rest of that reply is never captured. Please sync on
session.idleonly, or track completion per message. - Exit path. The synchronous exit save can block for 30s per query plus 60s per wing. It overrides the default SIGINT/SIGTERM exit, and it ignores
miningLock, so it can start a second mine while one is running. Exit needs to stay fast and never race the lock. - Privacy. Transcripts are written world-readable to a fixed
/tmp/oc-sessions, and a fixed/tmp/oc-plugin-query.pygets executed. Please use a per-user 0700 directory under~/.mempalace, ormkdtemp. - Windows. The frontmatter lists
win32, but the binary is hardcoded to~/.local/bin/mempalaceand the venv lookup tobin/python3. Either resolve the binary from PATH (or an env override), or dropwin32and add the Windows note I asked for. - Docs hygiene.
- Pin the npm version in the doc.
- Use one tool-name convention (
mempalace_mempalace_*) throughout. - Link
integrations/shared/recall-protocol.mdinstead of a "CRITICAL / MUST every response" AGENTS.md. - Don't tell users to create
~/.config/opencode/AGENTS.md; that overwrites the shared-brain rules. - Drop or re-measure the "<500ms" claim. A cold
mempalace searchtook about 20s here. - Set
version:to the current release.
Open question on our side: does a community npm package belong behind an install block in an official integrations/ skill, or as a website/guide/opencode.md page that links to it clearly as third-party? I lean toward the guide page.
|
Thanks for the thorough review, @igorls — this is exactly the kind of pass this needed. Addressed point by point (doc updated on this branch, plugin fixes landed on
On the open question (skill vs guide page): happy either way — a Re-review welcome whenever suits. |
|
Small update, no action needed — just keeping the thread accurate while you have bigger fish to fry. The plugin is now at 2.5.1 on npm (all dogfooded daily). Since my last note: TUI toasts for mines, checkpoints and MCP calls (opt-out), a read-only The SKILL.md on this branch describes the 2.2.0 behavior; I'll sync it to latest whenever you signal the direction is still good — didn't want to churn the branch on every release. @igorls @williamblair333 — whenever you have a moment, no rush at all. |
This PR adds an integration guide for using MemPalace with OpenCode via the
opencode-mempalace-persistenceplugin.What
integrations/opencode/SKILL.md— step-by-step setup for:Why
OpenCode is a popular open-source AI coding tool. This integration makes MemPalace available to its users with zero configuration — install the plugin, set up MCP, and every conversation is automatically saved to the palace.
Features documented
The integration covers:
Links