feat(opencode): HEADROOM_OPENCODE_BIN for forks + config/rtk fixes - #2021
feat(opencode): HEADROOM_OPENCODE_BIN for forks + config/rtk fixes#2021lennney wants to merge 18 commits into
Conversation
Add HEADROOM_OPENCODE_BIN env var to customize the opencode binary name that headroom resolves via shutil.which. Deduplicate the _opencode_home_dir() logic spread across 4 files into a single _shared.py. Closes headroomlabs-ai#1927 Design: HEADROOM_OPENCODE_BIN controls binary discovery only — config path stays at ~/.config/opencode/ (OpenCode hardcoded location). Use OPENCODE_HOME or OPENCODE_CONFIG to point at a fork custom config. Also: make MCP registrar config writes atomic (temp-file + rename) and add fcntl.flock advisory locking on .lock file to prevent data corruption from concurrent headroom processes. Remove redundant unregister_server call in register_server(force=True) that doubled the race window. - New _shared.py: source of truth for all config path resolution - wrap.py, config.py, mcp_registry/opencode.py: import from _shared - mcp_registry/opencode.py: atomic writes + flock + single RMW - config.py: remove dead imports - Tests: +42 new tests (E2E 30 + concurrency 11)
- paths.py: remove stale HEADROOM_OPENCODE_BIN basename reference from opencode_config_path() docstring (config dir is always ~/.config/opencode, not derived from binary name) - install.py: strip prior Headroom-managed blocks before re-injecting in apply_provider_scope(), mirroring inject_opencode_provider_config(). Fixes revert failure when transitioning from wrapped to persistent install state.
fff8a03 to
0ccf48e
Compare
- _shared.py: _opencode_config_path() checks for opencode.jsonc first, falls back to opencode.json. OPENCODE_CONFIG env var still takes precedence over both. - config.py: opencode_config_paths() uses .with_name() instead of .with_suffix() for backup files, so .jsonc backup is correctly named opencode.jsonc.headroom-backup (not opencode.headroom-backup). - install.py: same backup naming fix in apply_provider_scope() and revert_provider_scope(). - Tests: added test_opencode_config_paths_prefers_jsonc. Closes headroomlabs-ai#1588
Align with Codex wrapper behaviour — only inject rtk instructions into the global OpenCode AGENTS.md (~/.config/opencode/AGENTS.md), not the project-level AGENTS.md. The project file is a tracked team artefact and should not be modified by a wrapper tool. Other agents (Claude, Codex) already follow this pattern. Only OpenCode was double-injecting into both global and project. Closes headroomlabs-ai#1980
CodeQL flagged os.open with 0o644 mask sets the lock file to world-readable. Change to 0o600 (owner-only read/write) since lock files don't need to be readable by other users.
358d7bb stopped injecting rtk into the project-level AGENTS.md for OpenCode wrap (intentional — aligns with Codex behaviour). Update the E2E test to no longer assert that the project file exists or contains the RTK marker.
PR governanceThis PR follows the template and is marked ready for human review. |
- Split semicolons and colon-one-liners into separate statements - Add noqa: E402 for intentional post-sys.path imports
mypy 1.20.2 (CI) requires return type on @contextmanager generators; local mypy 2.2.0 was lenient but CI failed with [no-untyped-def].
import fcntl at module level would crash on Windows where the module doesn't exist. Wrap in try/except and make _locked_config a true no-op when fcntl is unavailable.
apply_provider_scope called strip_opencode_headroom_blocks with default remove_mcp=True, which nuked MCP server entries on reinstall after wrap. Use remove_mcp=False to only strip the provider block.
MCP entries with "command": [] (empty list) would raise IndexError on command_value[0] in the isinstance(list) branch. Add truthiness check to fall through to the safe else branch instead.
…P data
_read_json used strict json.load which fails on // comments,
returning {} and causing _write_json to nuke the entire config
(model, provider, user comments). Add comment-stripping fallback
mirroring _parse_json_loose from config.py.
Regex r'^\s*//[^\n]*\n' only matched comment-only lines ending with \n, missing a trailing comment on the last line of a file (EOF with no newline). Drop the \n requirement to also strip terminating comments. Applied to both _read_json and _parse_json_loose.
JerrettDavis
left a comment
There was a problem hiding this comment.
The implementation is close, and CI is green on the hosted matrix, but I hit a Windows portability failure in the focused OpenCode test suite:
uv run pytest tests/test_providers_opencode_config.py -q
FAILED tests/test_providers_opencode_config.py::test_build_launch_env_with_project
assert 'C:\\...\\entry.opencode.js' in '{"plugin":["C:\\\\...\\\\entry.opencode.js"]}'
The actual launch env is JSON-encoding the Windows path, so backslashes are escaped in OPENCODE_CONFIG_CONTENT. The assertion should parse the JSON and compare config["plugin"] (or compare against json.dumps(str(plugin))), rather than searching for the raw platform path inside the serialized JSON string.
This is small, but worth fixing before merge because this PR is explicitly touching OpenCode config/wrap behavior and the repository supports Windows workflows. The MCP registry tests passed locally (31 passed), and CI is otherwise green.
Replace raw string check on OPENCODE_CONFIG_CONTENT with json.loads() + structured comparison. On Windows the JSON-serialized path backslashes are escaped (C:\\...\\entry.opencode.js), so the raw str(plugin) never matches the JSON string. The parsed comparison works identically on all platforms.
…-bin # Conflicts: # headroom/mcp_registry/opencode.py
f8ffe38 to
bab4f1c
Compare
|
Rebased on upstream/main. Changes:
|
JerrettDavis
left a comment
There was a problem hiding this comment.
Thanks for the rebase and follow-up. The Windows-specific blocker from my last review is fixed: test_build_launch_env_with_project now parses OPENCODE_CONFIG_CONTENT as JSON and compares the plugin array instead of searching for the raw platform path.
Verified locally on Windows:
python -m pytest tests/test_providers_opencode_config.py -q
40 passed
I also rechecked the PR state: current head bab4f1ca is clean with no failed or pending GitHub checks. The broader tests/test_cli/test_wrap_opencode.py file stalled in my freshly cleaned local worktree before pytest emitted the session header, so I am not treating that as a PR failure; the hosted checks are green and the previous concrete blocker is resolved.
Description
Allow
HEADROOM_OPENCODE_BINenv var to support OpenCode forks with custom binary names (e.g. Rolandcode). Also fixes 10+ long-standing and newly-discovered bugs in the OpenCode integration discovered through adversarial testing: config file preference, backup naming, duplicate rtk injection, MCP block stripping, lock file permissions, Windows fcntl compatibility, JSONC parsing edge cases, and more.Closes #1927, closes #1588, closes #1980
Type of Change
Changes Made
Feature
HEADROOM_OPENCODE_BINenv var to customise binary name resolution_opencode_home_dir()logic spread across 4 files into single_shared.pyfcntl.flock)_locked_config()context manager with advisory locking to prevent concurrent write corruptionBug fixes (design review)
opencode.jsoncoveropencode.jsonwhen both exist ([BUG]wrap opencodewrites toopencode.jsoneven ifopencode.jsoncexists #1588).with_name()instead of.with_suffix()so.jsoncbackup is.jsonc.headroom-backupAGENTS.md— only global~/.config/opencode/AGENTS.md([FEATURE] Option to opt out ofrtkinstruction injection into repo-levelAGENTS.mdwhen runningheadroom wrap#1980)apply_provider_scope(), mirroringinject_opencode_provider_config()— and preserve MCP blocks during reapply (only strip provider scope)0o644→0o600(CodeQL)HEADROOM_OPENCODE_BINbasename reference fromopencode_config_path()docstringBug fixes (adversarial testing — 8 workers × concurrent stress + malicious inputs + crash recovery)
apply_provider_scopecalledstrip_opencode_headroom_blockswith defaultremove_mcp=True, nuking MCP entries during install→wrap→reinstall cycleimport fcntlcrashes: Module-levelimport fcntlcausedModuleNotFoundErroron Windows (docstring claimed no-op but code didn't protect it). Now guarded with try/except._locked_configmypy 1.20.2 type error: CI uses mypy 1.x which requires explicit-> Generator[None, None, None]on@contextmanagerfunctions (mypy 2.x relaxed this)_read_jsonused strictjson.loadwhich fails on//comments, returning{}and causing_write_jsonto overwrite entire config (model, provider, user data all lost). Now strips comments as fallback.\nnot stripped: Regex^\s*//[^\n]*\nmissed EOF-terminating comments. Fixed to handle EOF boundary.command: []causes IndexError:_entry_to_specindexingcommand_value[0]without emptiness guard. Now falls through to safe branch.Test updates
test_opencode_config_paths_prefers_jsoncheadroom wrapE2E with custom binaryTesting
headroom wrap)Real Behavior Proof
uv sync), Ubuntu 24.04HEADROOM_OPENCODE_BIN=my-opencode headroom wrap opencode --port 9999 -- --help→ custom binary invoked; (2)headroom wrap opencode --port 9999 -- --help→ defaultopencodebinary; (3)headroom unwrap opencode --port 9999→ provider removed, config restored; (4)uv run pytest ...→ 209 passed; (5) adversarial tests above → 0 errors across all categories; (6)uv run ruff check . && uv run ruff format --check . && uv run mypy headroom --ignore-missing-imports→ 0 errorsHEADROOM_OPENCODE_BINworks. Config writes are atomic (crash-kill mid-write leaves original intact). Lock file0o600. Backup naming correct for.jsonc(.jsonc.headroom-backup). Concurrent 8-thread registration/unregistration produces 0 errors, 0 leaks, 0 missing entries. JSONC files with comments are NOT destroyed by the registrar (non-MCP data preserved). MCP blocks survive install→wrap→reinstall cycle. Windowsimport fcntlguarded by try/except. All lint/format/mypy clean.Review Readiness