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

[crisacm]

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.

[github-actions]

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:

1. 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.)

2. 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!

[DeusData]

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:

1. Merged current main into the branch. It merged cleanly; this just lets CI run on today's tree.
2. 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.
3. 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.
4. 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!

[DeusData]

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.

1. 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.