From b75dff0dd11d58f2974865d59095dc61e4c35564 Mon Sep 17 00:00:00 2001 From: Anil Murty Date: Mon, 11 May 2026 22:24:08 -0700 Subject: [PATCH] Purge legacy [mcp_servers.ocw] / ocw-managed [otel] from Codex config on onboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the rebrand, `ocw onboard --codex` wrote: - `[mcp_servers.ocw]` with `command = "ocw"` and `args = ["mcp"]` - `[otel]` block with `# Managed by ocw — do not edit this block manually` After the rebrand, `tj onboard --codex` correctly appends `[mcp_servers.tj]`, but it left the legacy `ocw`-managed sections in place. The result was a Codex config carrying both blocks — and the orphan `[mcp_servers.ocw]` points at a binary that no longer exists, so Codex would try to spawn a non-existent MCP server on every launch. Fix: `_codex_purge_legacy_ocw()` strips both legacy section trees unconditionally on every `tj onboard --codex`. If the purge actually changed anything, the cleaned file is persisted immediately — so the "already configured" early-return path no longer leaves stale `ocw` sections sitting in the file forever. The `[otel]` block is only stripped when it carries the `# Managed by ocw` comment, so tj-managed (or user-authored) `[otel]` blocks are preserved. Found while running tests/manual-pre-release-testing.md (Codex CLI block). Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/unit/test_onboard_codex.py | 118 ++++++++++++++++++++++++++++++- tokenjam/cli/cmd_onboard.py | 60 ++++++++++++++++ 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_onboard_codex.py b/tests/unit/test_onboard_codex.py index f0426c3..6a864ee 100644 --- a/tests/unit/test_onboard_codex.py +++ b/tests/unit/test_onboard_codex.py @@ -1,7 +1,11 @@ """Unit tests for Codex config writing helpers in cmd_onboard.""" from __future__ import annotations -from tokenjam.cli.cmd_onboard import _codex_apply_block, _codex_mcp_toml_block +from tokenjam.cli.cmd_onboard import ( + _codex_apply_block, + _codex_mcp_toml_block, + _codex_purge_legacy_ocw, +) class TestCodexApplyBlock: @@ -32,3 +36,115 @@ def test_otel_block_unchanged_when_mcp_appended(self): block = _codex_mcp_toml_block() result = _codex_apply_block(otel_section, r"\[mcp_servers\.tj\]", False, block, False) assert result.startswith(otel_section.rstrip()) + + +class TestCodexPurgeLegacyOcw: + def test_empty_content_unchanged(self): + assert _codex_purge_legacy_ocw("") == "" + + def test_no_legacy_sections_unchanged(self): + content = ( + '[otel]\n' + '# Managed by tj — do not edit this block manually\n' + 'log_user_prompt = false\n' + '\n' + '[mcp_servers.tj]\n' + 'command = "tj"\n' + 'args = ["mcp"]\n' + ) + # Trailing newline normalization is fine, but the substantive content + # should be identical. + result = _codex_purge_legacy_ocw(content) + assert "[mcp_servers.ocw]" not in result + assert "Managed by ocw" not in result + assert "[mcp_servers.tj]" in result + assert "[otel]" in result + assert "Managed by tj" in result + + def test_drops_legacy_mcp_servers_ocw_block(self): + content = ( + '[mcp_servers.ocw]\n' + '# Managed by ocw — gives Codex access to OCW observability tools\n' + 'command = "ocw"\n' + 'args = ["mcp"]\n' + '\n' + '[mcp_servers.tj]\n' + 'command = "tj"\n' + 'args = ["mcp"]\n' + ) + result = _codex_purge_legacy_ocw(content) + assert "[mcp_servers.ocw]" not in result + assert 'command = "ocw"' not in result + assert "[mcp_servers.tj]" in result + assert 'command = "tj"' in result + + def test_drops_legacy_otel_block_marked_managed_by_ocw(self): + content = ( + '[otel]\n' + '# Managed by ocw — do not edit this block manually\n' + 'log_user_prompt = false\n' + '\n' + '[otel.exporter."otlp-http"]\n' + 'endpoint = "http://127.0.0.1:7391/v1/logs"\n' + 'protocol = "json"\n' + '\n' + '[otel.exporter."otlp-http".headers]\n' + 'Authorization = "Bearer abc123"\n' + ) + result = _codex_purge_legacy_ocw(content) + assert "[otel]" not in result + assert "[otel.exporter" not in result + assert "Managed by ocw" not in result + assert "Authorization" not in result + + def test_preserves_otel_block_not_marked_ocw(self): + """A [otel] block without the 'Managed by ocw' comment is left alone — + only the legacy ocw-marked sections are stripped.""" + content = ( + '[otel]\n' + 'log_user_prompt = false\n' + '\n' + '[otel.exporter."otlp-http"]\n' + 'endpoint = "http://127.0.0.1:7391/v1/logs"\n' + ) + result = _codex_purge_legacy_ocw(content) + assert "[otel]" in result + assert "[otel.exporter" in result + + def test_real_world_dual_blocks_from_user_report(self): + """End-to-end repro of the actual file from issue testing: + both [mcp_servers.ocw] and [mcp_servers.tj] present, plus an [otel] + block carrying the legacy 'Managed by ocw' comment.""" + content = ( + '[otel]\n' + '# Managed by ocw — do not edit this block manually\n' + 'log_user_prompt = false\n' + '\n' + '[otel.exporter."otlp-http"]\n' + 'endpoint = "http://127.0.0.1:7391/v1/logs"\n' + 'protocol = "json"\n' + '\n' + '[otel.exporter."otlp-http".headers]\n' + 'Authorization = "Bearer d473add7b0a19c18ca8a8014f7df760e29017578fcdc93339424172192c787ea"\n' + '\n' + '[mcp_servers.ocw]\n' + '# Managed by ocw — gives Codex access to OCW observability tools\n' + 'command = "ocw"\n' + 'args = ["mcp"]\n' + '\n' + '[mcp_servers.tj]\n' + '# Managed by tj — gives Codex access to TokenJam observability tools\n' + 'command = "tj"\n' + 'args = ["mcp"]\n' + ) + result = _codex_purge_legacy_ocw(content) + # All ocw-managed sections gone. + assert "[mcp_servers.ocw]" not in result + assert "Managed by ocw" not in result + assert 'command = "ocw"' not in result + # The [otel] block was ocw-managed, so it was stripped — the caller + # is responsible for writing a fresh tj-managed [otel] block. + assert "[otel]" not in result + # The tj MCP block survived. + assert "[mcp_servers.tj]" in result + assert 'command = "tj"' in result diff --git a/tokenjam/cli/cmd_onboard.py b/tokenjam/cli/cmd_onboard.py index 8561ab9..0774c43 100644 --- a/tokenjam/cli/cmd_onboard.py +++ b/tokenjam/cli/cmd_onboard.py @@ -391,6 +391,15 @@ def _onboard_codex( existing_content = codex_config_path.read_text() if codex_config_path.exists() else "" + # Purge any legacy `ocw`-managed sections left over from pre-rebrand + # onboards. If anything was stripped, persist the cleaned file now so the + # "already configured" early-return path below doesn't leave the legacy + # sections sitting in the file forever. + cleaned = _codex_purge_legacy_ocw(existing_content) + if cleaned != existing_content: + codex_config_path.write_text(cleaned) + existing_content = cleaned + # Check whether an [otel] section already exists existing_codex: dict = {} if existing_content: @@ -549,6 +558,57 @@ def _codex_apply_block( return (content.rstrip() + "\n\n" + block) if content.strip() else block +def _codex_purge_legacy_ocw(content: str) -> str: + """Remove ocw-managed sections from ~/.codex/config.toml left behind by + pre-rebrand onboards. + + Before the project was renamed, the legacy `ocw` CLI wrote a + `[mcp_servers.ocw]` block (pointing `command = "ocw"`) and an `[otel]` + block with a `# Managed by ocw` comment. After the rebrand, the new + onboard appends the `[mcp_servers.tj]` block but does not touch the + legacy `ocw` sections — so Codex ends up with both registered, and + tries to spawn a non-existent `ocw` MCP server on every launch. + + This unconditionally strips the legacy sections so the normal onboard + flow can write fresh tj-managed blocks in their place. + """ + import re as _re + + # Drop the entire [mcp_servers.ocw] section (and any nested tables). + content = _re.sub( + r"\[mcp_servers\.ocw\].*?(?=\n\[|\Z)", + "", + content, + flags=_re.DOTALL, + ) + + # If the existing [otel] block is marked "Managed by ocw", strip the + # whole [otel] tree so the new tj-managed block is written cleanly. + # We use a non-anchored search across the whole content because the + # comment may sit a few lines below the section header. + has_legacy_otel = bool( + _re.search( + r"\[otel\][^\[]*?#\s*Managed by ocw", + content, + flags=_re.IGNORECASE, + ) + ) + if has_legacy_otel: + for pat in ( + r"\[otel\.exporter\.\"otlp-http\"\.headers\].*?(?=\n\[|\Z)", + r"\[otel\.exporter\.\"otlp-http\"\].*?(?=\n\[|\Z)", + r"\[otel\.exporter\.\"otlp-grpc\"\.headers\].*?(?=\n\[|\Z)", + r"\[otel\.exporter\.\"otlp-grpc\"\].*?(?=\n\[|\Z)", + r"\[otel\.resource\].*?(?=\n\[|\Z)", + r"\[otel\].*?(?=\n\[|\Z)", + ): + content = _re.sub(pat, "", content, flags=_re.DOTALL) + + # Collapse runs of 3+ blank lines left behind by removals. + content = _re.sub(r"\n{3,}", "\n\n", content) + return content.strip() + ("\n" if content.strip() else "") + + def _codex_strip_otel_sections(content: str) -> str: """Remove all [otel] sections (including nested exporter/resource tables).""" import re as _re