fix(cli): leave a user-owned OpenCode module untouched instead of appending the block by crisacm · Pull Request #2641 · DeusData/codebase-memory-mcp

crisacm · 2026-10-14T16:20:13Z

Summary

The generated OpenCode adapter (~/.config/opencode/plugins/cbm-augment.ts) is an auto-loaded module, so it is installed as a managed block rather than a whole-file write. That is deliberate: the directory is auto-loaded, so a user may legitimately keep their own module there.

But when the file on disk carried content and none of our markers, the install appended our block to it anyway — even though the block's own header tells readers to "remove the markers to take ownership". That contradicts the promise and produces a file the client cannot parse.

Reproduction

install -y against 0.11.0, in an isolated HOME, starting from the hand-port shape described in #2204:

// the user's own module: content, no markers
import { spawn } from 'node:child_process';
const BIN = '/Users/x/.local/bin/codebase-memory-mcp';
function augment(payload) { /* … */ }
export default { id: 'codebase-memory-augment', setup() {} };

After the run the managed block sits after that content, so module-level names are declared twice:

SyntaxError: Identifier 'spawn' has already been declared

OpenCode's loader then drops the whole extension and the only trace is a load warning. Once the block itself carries export default (#2089), the same append turns into a duplicate default export instead. So a hand-port is not a durable end state — which is why #2385 matters — but the installer should not manufacture the broken file in the meantime.

Change

cbm_text_managed_block_status() — a read-only ownership probe: 0 writable (absent, whitespace-only, or already ours), 1 the file carries content but none of our markers, -1 unsafe state or I/O failure.

install_generated_client_extension() probes before writing:

writable — unchanged behaviour: install, or refresh the block in place.
foreign — leave the file exactly as it is and report it, in the real run and in --dry-run. The probe runs before generation so the preview names the refusal instead of promising a write the run will not make (the same honesty rule install on 0.9.1-rc.1 removes existing Claude Code hooks instead of installing them #1387 established for hook scripts).
probe error — falls through to the existing write, which reports it through the usual error path.

cbm_text_upsert_managed_block() and its other callers (instruction documents, where appending to a user's file is the intent) are untouched — the new semantics are scoped to the auto-loaded module case.

If you would rather the run fail loudly than skip, that is a small change to the same call site; I picked "leave alone and say so" because it matches the ownership wording in the generated header.

Verification

End-to-end in isolated HOMEs on the built binary:

on-disk file	before	after
foreign module, no markers	appended → `SyntaxError`	left untouched, reported
no file at all	created	created (92 lines, markers present)
stale block, markers present	replaced	refreshed in place

Tests:

scripts/test.sh --suites config_text_edit — 23 passed, including the new config_text_managed_block_status_reports_foreign_module (absent / empty / whitespace / ours / foreign / unbalanced markers / malformed markers).
scripts/test.sh --suites cli — 361 passed, 1 failed, and the new cli_extension_leaves_user_owned_module_untouched_issue2204 is the +1 over the baseline: it seeds a user-owned module, runs the dry run, and asserts both the warning and that the file is byte-identical afterwards (modelled on the existing cli_dry_run_predicts_refused_hook_script_issue1387).
The single failure is pre-existing and unrelated. Same machine, same suite, same test:

tree result

main at 0ca6e7d (no changes) 360 passed, 1 failed

this branch 361 passed, 1 failed

In both trees the failing test is cli_registry_installs_gitlab_and_devin_lifecycle_context with the identical diagnostic plan=1 clean_plan=1 installed=0 cleaned=1 gitlab=1 devin=1 uninstall=0 — it touches neither the OpenCode adapter nor the managed-block writer. Flagging it here so a red test-unix job is not read as fallout from this change.
clang-format 20.1.8 (CI's pinned version) reports no changes to any touched file.

Refs #2204. The template side — registering the 2.x hooks in setup — is #2385 and is deliberately not touched here. Happy to fold this into #2385 instead if you would rather keep the OpenCode changes on one branch.

…ending the block The generated OpenCode adapter is an auto-loaded module, so it is installed as a managed block rather than a whole-file write. When the file on disk carried content but none of our markers, the upsert appended the block anyway, even though the block's own header tells readers to "remove the markers to take ownership". The appended prelude redeclares module-level names the user's own module also declares -- `import { spawn }`, `const BIN`, `function augment` -- and both sides declare a default export once the block itself carries one, so the result cannot be parsed at all: SyntaxError: Identifier 'spawn' has already been declared The client then drops the whole extension with only a load warning to show for it. A hand-port around DeusData#2204 is therefore not a durable end state. Probe block ownership before writing. A marker-less file that carries content is left alone and reported in both the real run and the dry run, instead of being turned into an unparseable hybrid. An absent, empty, whitespace-only, or already-ours file still installs or refreshes exactly as before. Refs DeusData#2204 Signed-off-by: Cristian Castillo <[email protected]>

github-actions · 2026-10-14T16:20:49Z

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

It will not be closed for inactivity. No stale bot touches pull requests here.
It may still sit a while before a human reads it. That is on us, not on you.
Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
Get CI green, or say which failures you believe are pre-existing.
Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

DeusData

Thank you, @crisacm, for this fix and for the unusually thorough write-up. The before/after table and the pre-existing-failure comparison made it easy to follow. The behavior is right: the generated header says "remove the markers to take ownership", so a marker-less file at that path is the user's, and leaving it alone and saying so is what the installer should do. Several people hit this on #2204, so this will help.

CI stopped at lint, so the tests never ran. Two changes, then CI can run the suites:

Memory-core ratchet. lint-memory-core fails because src/cli/config_text_edit.c gains two raw free() calls (72 -> 74). We route allocations through src/foundation/mem_core.h, and the linter only lets each file's count of raw allocator calls go down. text_read_file() still hands back memory from plain malloc, so swapping in cbm_free is not a drop-in fix here. The simplest way through is to let the probe reuse the read-validate-scan step that text_upsert_managed_block() already performs (for example a small shared static helper, or a probe-only mode), so no new free() site is added. You can check it locally with make -f Makefile.cbm lint-memory-core. (The cypher.c line in the same log asks us to tighten our own baseline; that one is not yours.)
Install-plan receipt. When g_install_plan is set, install_generated_client_extension() still calls plan_record(label, "extension", path) before the ownership check, so the plan receipt lists a write that the real run will not make. The receipt is built from the same code path precisely so it cannot drift from the run. Could you skip plan_record() when the file is foreign? The message you print there already explains the skip. A check for that in your new cli test would be welcome if it is easy.

Your branch still merges cleanly with current main, so no rebase is needed. Thanks again for the careful work!

Signed-off-by: Martin Vogel <[email protected]>

…robe cbm_text_managed_block_status() repeated the read, validate and scan step of text_upsert_managed_block() with two raw free() calls of its own, which put src/cli/config_text_edit.c over its memory-core baseline (72 -> 74). Both now go through text_read_managed_document(), so the probe and the writer judge a file by the same rules, and the file is back at 72. An absent file still reads as an empty document, so the probe returns the same results as before. Signed-off-by: Martin Vogel <[email protected]>

With --plan, install_generated_client_extension() recorded the extension before the ownership check and printed the skip notice on stdout. The receipt therefore listed a write the run would not make, and `install --plan` no longer printed valid JSON when a user-owned module was present. For a user-owned module, plan mode now records and prints nothing. The dry run prints the "would be skipped" notice instead, alongside the hook-script notice from DeusData#1387, and the real run keeps its "left untouched" line. The cli test now also checks the receipt and plan-mode stdout, with a control that the receipt does list the module once it is no longer user-owned. Refs DeusData#2204 Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-19T22:45:00Z

Thank you, @crisacm, for this fix and for the care that went into it. The write-up, the before/after table and the side-by-side run showing that the one red cli test fails on main too made this easy to review, and the behavior is exactly right: a module at that path without our markers is the user's, and the installer should leave it alone and say so. Several people ran into this on #2204, so this will help them directly.

To save you the round trip, we made the changes from our review ourselves and pushed them on top of your branch. Your commit is unchanged; here is what we added after it:

Merged current main into the branch. It merged cleanly; this just lets CI run on today's tree.
Memory-core ratchet ("refactor(cli): share the managed-block read step with the ownership probe"). The probe and text_upsert_managed_block() now share one small helper, text_read_managed_document(), that reads the file, validates it and locates our block. The probe no longer carries its own copy of that step and its two extra free() calls, so src/cli/config_text_edit.c is back at its baseline of 72 and lint-memory-core passes. Behavior is unchanged: a missing file still reads as an empty document, and all 23 config_text_edit tests pass, including your seven-case probe test.
Install-plan receipt ("fix(cli): keep a user-owned OpenCode module out of the install plan"). For a user-owned module, plan mode now neither records the extension nor prints anything. The receipt should list only writes the run will make, and while checking this we found that the "would be skipped" line printed in plan mode also landed on stdout ahead of the JSON, so install --plan stopped printing valid JSON whenever such a module was present. That message now prints in --dry-run instead, next to the similar hook-script notice from install on 0.9.1-rc.1 removes existing Claude Code hooks instead of installing them #1387, and the real run keeps your "left untouched" line.
Test. Your cli test now also checks that the plan receipt does not list the module and that plan mode prints nothing about it, plus a control showing the receipt does list the path once the user's file is gone, so the new check cannot pass just because OpenCode was not detected.

With these, the full cli suite (363 tests) passes locally. Once CI is green we will merge. Thanks again for a really careful contribution!

text_upsert_managed_block read doc.bom_len into a function-scope local, but only the "no managed region yet" branch uses it. cppcheck 2.20 (the CI lint image) flags that as variableScope. Declare it inside the else branch instead; doc stays in scope and unmodified, so behaviour is unchanged. Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-20T00:59:15Z

Hi @crisacm, thank you again for this fix and for your patience with us here!

This is a small follow-up to our own earlier fix-up on your branch, not to your change. Our refactor commit ("share the managed-block read step with the ownership probe") left a variable in a wider scope than it needs, and the cppcheck version our CI lint job uses flagged it.

In text_upsert_managed_block, the bom_len local is now declared inside the branch that inserts a new managed block, the only place that reads it. It is a one-line move with no behaviour change; the config_text_edit and cli suites pass unchanged.

No action is needed from you. Once CI is green we will take it from here.

The byte check in cli_extension_leaves_user_owned_module_untouched_issue2204 ran only after a dry run and the plan receipt, and neither of them writes, so it could not catch the DeusData#2204 append. The test now also runs a real install over the user-owned module, then checks that the file is byte-identical and that the run reports it as left untouched. Each phase's stdout checks are bounded to that phase's slice of the capture, so no phase can pass on another's line. With the early return that ends the user-owned branch of install_generated_client_extension() dropped, the test now fails; before this change it still passed. Refs DeusData#2204 Signed-off-by: Martin Vogel <[email protected]>

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-26T01:25:22Z

Hi @crisacm, thank you again for this fix, and for your patience while it waited on our side. Your change is the right one: a module at that path without our markers belongs to the user, and the installer now leaves it alone and says so.

We added two small things on top of your branch so it can go through CI on today's tree:

Test: your cli test now also runs a real install (not only the dry run) over the user-owned module, and checks that the file is byte-identical afterwards and that the run reports it as left untouched. Each output check now looks only at the output of its own run. A dry run never writes, so this is the check that would catch the original append if it ever came back; we confirmed that the test fails if the early return for a user-owned module is removed.
We merged current main into the branch. It merged cleanly; this just lets CI run against the current tree.

No action is needed from you. If you push to this branch again, please pull first, since it now has our commits on top. Once CI is green we will merge. Thanks again for such a careful contribution!

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-27T18:18:18Z

Thanks again, @crisacm. Several other pull requests landed on main this evening, including changes in files this one also touches, so we merged current main into your branch once more (one signed-off merge commit, 4876266; your changes are untouched). That keeps the required checks honest against today's tree; we will merge once they are green. Please pull before pushing again.

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-28T17:16:01Z

Thank you for your patience, @crisacm. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, 2a195bd; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-29T00:59:23Z

Thank you for your patience, @crisacm. A pull request that merged into main a few minutes ago (#2290) changes src/cli/cli.c, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 079e59d; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-30T21:31:11Z

Thank you for your patience, @crisacm. Main has moved since the last CI run on this branch, and some of the new commits change src/cli/cli.c, which this pull request also changes, so that run can no longer vouch for today's main. We merged current main into your branch (one merge commit, 8f6974e; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-31T05:12:37Z

Thank you for your patience, @crisacm. The macOS legs of the last CI run here were cancelled by GitHub itself: it is retiring its macOS 14 runners on November 2 and ran scheduled brownouts beforehand, so those jobs never got a runner. Nothing in your change caused it. Main now runs CI on macOS 15 (#2849), so we merged current main into your branch (one merge commit, 502d86a; it merged cleanly and your own change is untouched) to get a run on the new runners. Please pull before pushing again.

Signed-off-by: Martin Vogel <[email protected]>

DeusData · 2026-10-31T23:07:53Z

Thank you for your patience, @crisacm. Your change was green, but several pull requests merged yesterday touch src/cli/cli.c and tests/test_cli.c, which this pull request also changes, so that run could no longer vouch for today's main. We merged current main into your branch (no conflicts, your own change is unchanged) to get a fresh CI run on the combined tree. Please pull before pushing again. We will merge as soon as the required checks are green.

DeusData · 2026-11-01T02:02:41Z

Merged as 96c2fef - thank you, @crisacm! A plugin file at that path without our markers now belongs to the user: install leaves it untouched and says so, instead of appending our block. Thank you for the fix and for your patience while it waited on our side.

crisacm requested a review from DeusData as a code owner October 14, 2026 16:20

DeusData reviewed Oct 19, 2026

View reviewed changes

Merge branch 'main' into fix/2204-opencode-extension-untouched

ccedf27

Signed-off-by: Martin Vogel <[email protected]>

DeusData added bug Something isn't working editor/integration Editor compatibility and CLI integration labels Oct 19, 2026

DeusData added 2 commits October 20, 2026 00:42

Merge branch 'main' into fix/2204-opencode-extension-untouched

4bead60

DeusData mentioned this pull request Oct 19, 2026

fix(cli): register OpenCode 2 hooks via the setup context #2385

Merged

DeusData added 2 commits October 26, 2026 02:22

Merge origin/main into fix/2204-opencode-extension-untouched

8fe97d7

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

4876266

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

2a195bd

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

079e59d

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

8f6974e

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

502d86a

Signed-off-by: Martin Vogel <[email protected]>

DeusData added 3 commits October 31, 2026 16:38

Merge origin/main into fix/2204-opencode-extension-untouched

0d1d44b

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

9cb2a47

Signed-off-by: Martin Vogel <[email protected]>

Merge origin/main into fix/2204-opencode-extension-untouched

8d9a8e3

Signed-off-by: Martin Vogel <[email protected]>

DeusData merged commit 96c2fef into DeusData:main Nov 1, 2026
64 of 66 checks passed

DeusData mentioned this pull request Nov 1, 2026

OpenCode v2 plugin #2204

Closed

Navigation Menu

Repository navigation

fix(cli): leave a user-owned OpenCode module untouched instead of appending the block - #2641

fix(cli): leave a user-owned OpenCode module untouched instead of appending the block#2641
DeusData merged 16 commits into
DeusData:mainfrom
crisacm:fix/2204-opencode-extension-untouched

crisacm commented Oct 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

DeusData left a comment

Uh oh!

DeusData commented Oct 19, 2026

Uh oh!

DeusData commented Oct 20, 2026

Uh oh!

DeusData commented Oct 26, 2026

Uh oh!

DeusData commented Oct 27, 2026

Uh oh!

DeusData commented Oct 28, 2026

Uh oh!

DeusData commented Oct 29, 2026

Uh oh!

DeusData commented Oct 30, 2026

Uh oh!

DeusData commented Oct 31, 2026

Uh oh!

DeusData commented Oct 31, 2026

Uh oh!

Uh oh!

DeusData commented Nov 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tree	result
`main` at `0ca6e7d` (no changes)	360 passed, 1 failed
this branch	361 passed, 1 failed

Conversation

crisacm commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reproduction

Change

Verification

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

DeusData left a comment

Choose a reason for hiding this comment

Uh oh!

DeusData commented Oct 19, 2026

Uh oh!

DeusData commented Oct 20, 2026

Uh oh!

DeusData commented Oct 26, 2026

Uh oh!

DeusData commented Oct 27, 2026

Uh oh!

DeusData commented Oct 28, 2026

Uh oh!

DeusData commented Oct 29, 2026

Uh oh!

DeusData commented Oct 30, 2026

Uh oh!

DeusData commented Oct 31, 2026

Uh oh!

DeusData commented Oct 31, 2026

Uh oh!

Uh oh!

DeusData commented Nov 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

crisacm commented Oct 14, 2026 •

edited

Loading