feat: add OpenCode OTel support#109
Conversation
Enable OTel trace export for the OpenCode harness: - Set OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, and OTEL_BSP_SCHEDULE_DELAY=0 in build_otel_exec_env() and build_env_script_lines(). BSP_SCHEDULE_DELAY=0 forces immediate span flush to work around OpenCode's process.exit() killing spans before the batch processor drains. Claude Code doesn't need it (manages its own flush lifecycle). OTEL_METRIC_EXPORT_INTERVAL is Claude-specific (different metrics SDK) and not set for OpenCode. - Set supports_otel = True - Add write_sandbox_config() hook to Harness base class; OpenCode implementation writes opencode.json with experimental.openTelemetry - Add sandbox_config_mounts() to return (host, container) path pairs so backends don't hardcode container paths - Podman backend calls write_sandbox_config() and mounts via sandbox_config_mounts() - OpenShell backend uploads config via sandbox_config_mounts() and delegates OTel env to harness instead of hardcoding Claude-specific vars. CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC only set for Claude. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agentic_ci/backends/openshell/__init__.py (1)
64-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExisting sandboxes skip config refresh, leaving OTEL state stale.
Line 66 returns before Line 82 runs. If the sandbox was created without OTEL and later reused with
otel_portset,opencode.jsonis never rewritten withexperimental.openTelemetry: true, so traces can stay disabled.Suggested fix
if sandbox.exists(): log.section("Sandbox already exists") + self._upload_sandbox_config(otel_enabled=otel_port is not None) return @@ - self._upload_sandbox_config(otel_enabled=otel_port is not None) + self._upload_sandbox_config(otel_enabled=otel_port is not None)Also applies to: 82-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 64 - 67, The early return at line 64-67 (when sandbox already exists) prevents the OTEL configuration refresh logic from running at lines 82-83, causing opencode.json to never be updated with OTEL settings if a sandbox is reused with a new otel_port value. Move the OTEL configuration and opencode.json writing logic (currently at lines 82-83) to execute before the sandbox existence check, or restructure it so that configuration refresh happens independently of sandbox creation, ensuring that OTEL settings are always synchronized even for existing sandboxes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 89-92: The sandbox.exec_cmd call in the upload block is vulnerable
to shell command injection because container_path is interpolated into a shell
string executed via bash -c. To fix this, refactor the upload method to avoid
using bash -c with interpolated paths. Instead, split the operations into
separate exec_cmd calls that pass paths as direct arguments rather than shell
string interpolation, or use shell-safe escaping if bash must be used.
Specifically, replace the single mkdir and mv command that combines
container_path and fname into a shell string with individual exec_cmd calls that
pass these paths as list arguments to avoid any shell metacharacter
interpretation.
- Around line 86-93: The temporary directory created by tempfile.mkdtemp() is
not cleaned up if an exception occurs during sandbox.upload() or
sandbox.exec_cmd() calls, since the shutil.rmtree() cleanup on the final line is
skipped when exceptions are raised. Wrap the entire block that uses config_dir
(from tempfile.mkdtemp through the loop with sandbox.upload and
sandbox.exec_cmd) in a try-finally block to ensure shutil.rmtree(config_dir,
ignore_errors=True) always executes regardless of exceptions, or alternatively
refactor to use tempfile.TemporaryDirectory as a context manager for automatic
cleanup.
In `@src/agentic_ci/harness.py`:
- Around line 383-398: The _SANDBOX_CONFIG_DIR constant hardcodes the path as
/sandbox/.config/opencode, but the Podman container contract specifies that
OpenCode config should be mounted to /home/agent-ci/.config/opencode (matching
the OPENCODE_CONFIG_DIR environment variable in Podman). Update the
_SANDBOX_CONFIG_DIR constant value to /home/agent-ci/.config/opencode so that
the sandbox_config_mounts method returns the correct destination path when
mounting the opencode.json file to the container.
---
Outside diff comments:
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 64-67: The early return at line 64-67 (when sandbox already
exists) prevents the OTEL configuration refresh logic from running at lines
82-83, causing opencode.json to never be updated with OTEL settings if a sandbox
is reused with a new otel_port value. Move the OTEL configuration and
opencode.json writing logic (currently at lines 82-83) to execute before the
sandbox existence check, or restructure it so that configuration refresh happens
independently of sandbox creation, ensuring that OTEL settings are always
synchronized even for existing sandboxes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 05146be1-d898-48f2-a5ad-dc233a335b8e
📒 Files selected for processing (5)
docs/otel-configuration.mdsrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/podman.pysrc/agentic_ci/harness.pytests/test_harness.py
| config_dir = tempfile.mkdtemp(prefix="agentic-ci-config-") | ||
| self.harness.write_sandbox_config(config_dir, otel_enabled=otel_enabled) | ||
| for host_path, container_path in self.harness.sandbox_config_mounts(config_dir): | ||
| sandbox.upload(host_path) | ||
| fname = os.path.basename(host_path) | ||
| cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}" | ||
| sandbox.exec_cmd(["bash", "-c", cmd]) | ||
| shutil.rmtree(config_dir, ignore_errors=True) |
There was a problem hiding this comment.
Temporary config directories leak on exceptions.
If sandbox.upload or sandbox.exec_cmd fails, Line 93 is skipped and host temp dirs accumulate.
Suggested fix
def _upload_sandbox_config(self, otel_enabled=False):
"""Write harness-specific config and upload it to the sandbox."""
config_dir = tempfile.mkdtemp(prefix="agentic-ci-config-")
- self.harness.write_sandbox_config(config_dir, otel_enabled=otel_enabled)
- for host_path, container_path in self.harness.sandbox_config_mounts(config_dir):
- sandbox.upload(host_path)
- fname = os.path.basename(host_path)
- cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}"
- sandbox.exec_cmd(["bash", "-c", cmd])
- shutil.rmtree(config_dir, ignore_errors=True)
+ try:
+ self.harness.write_sandbox_config(config_dir, otel_enabled=otel_enabled)
+ for host_path, container_path in self.harness.sandbox_config_mounts(config_dir):
+ sandbox.upload(host_path)
+ fname = os.path.basename(host_path)
+ cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}"
+ sandbox.exec_cmd(["bash", "-c", cmd])
+ finally:
+ shutil.rmtree(config_dir, ignore_errors=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config_dir = tempfile.mkdtemp(prefix="agentic-ci-config-") | |
| self.harness.write_sandbox_config(config_dir, otel_enabled=otel_enabled) | |
| for host_path, container_path in self.harness.sandbox_config_mounts(config_dir): | |
| sandbox.upload(host_path) | |
| fname = os.path.basename(host_path) | |
| cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}" | |
| sandbox.exec_cmd(["bash", "-c", cmd]) | |
| shutil.rmtree(config_dir, ignore_errors=True) | |
| config_dir = tempfile.mkdtemp(prefix="agentic-ci-config-") | |
| try: | |
| self.harness.write_sandbox_config(config_dir, otel_enabled=otel_enabled) | |
| for host_path, container_path in self.harness.sandbox_config_mounts(config_dir): | |
| sandbox.upload(host_path) | |
| fname = os.path.basename(host_path) | |
| cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}" | |
| sandbox.exec_cmd(["bash", "-c", cmd]) | |
| finally: | |
| shutil.rmtree(config_dir, ignore_errors=True) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/__init__.py` around lines 86 - 93, The
temporary directory created by tempfile.mkdtemp() is not cleaned up if an
exception occurs during sandbox.upload() or sandbox.exec_cmd() calls, since the
shutil.rmtree() cleanup on the final line is skipped when exceptions are raised.
Wrap the entire block that uses config_dir (from tempfile.mkdtemp through the
loop with sandbox.upload and sandbox.exec_cmd) in a try-finally block to ensure
shutil.rmtree(config_dir, ignore_errors=True) always executes regardless of
exceptions, or alternatively refactor to use tempfile.TemporaryDirectory as a
context manager for automatic cleanup.
| sandbox.upload(host_path) | ||
| fname = os.path.basename(host_path) | ||
| cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}" | ||
| sandbox.exec_cmd(["bash", "-c", cmd]) |
There was a problem hiding this comment.
CWE-78: shell command is built from file paths and executed via bash -c.
Line 91 interpolates container_path into a shell string. Today paths are in-tree constants, but this hook is extensible; a future harness path with shell metacharacters turns this into command injection in the sandbox execution context.
Exploit scenario: a crafted mount target like /sandbox/x;touch /tmp/pwned results in arbitrary command execution when bash -c runs.
Suggested fix (avoid shell parsing)
for host_path, container_path in self.harness.sandbox_config_mounts(config_dir):
sandbox.upload(host_path)
fname = os.path.basename(host_path)
- cmd = f"mkdir -p $(dirname {container_path}) && mv {fname} {container_path}"
- sandbox.exec_cmd(["bash", "-c", cmd])
+ target_dir = os.path.dirname(container_path)
+ sandbox.exec_cmd(["mkdir", "-p", target_dir])
+ sandbox.exec_cmd(["mv", fname, container_path])As per coding guidelines, "Validate file paths (prevent path traversal)" and "No shell=True in subprocess".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/__init__.py` around lines 89 - 92, The
sandbox.exec_cmd call in the upload block is vulnerable to shell command
injection because container_path is interpolated into a shell string executed
via bash -c. To fix this, refactor the upload method to avoid using bash -c with
interpolated paths. Instead, split the operations into separate exec_cmd calls
that pass paths as direct arguments rather than shell string interpolation, or
use shell-safe escaping if bash must be used. Specifically, replace the single
mkdir and mv command that combines container_path and fname into a shell string
with individual exec_cmd calls that pass these paths as list arguments to avoid
any shell metacharacter interpretation.
Source: Coding guidelines
| _SANDBOX_CONFIG_DIR = "/sandbox/.config/opencode" | ||
|
|
||
| def write_sandbox_config(self, config_dir, otel_enabled=False): | ||
| opencode_dir = os.path.join(config_dir, ".config", "opencode") | ||
| os.makedirs(opencode_dir, exist_ok=True) | ||
| config = {"$schema": "https://opencode.ai/config.json"} | ||
| if otel_enabled: | ||
| config["experimental"] = {"openTelemetry": True} | ||
| with open(os.path.join(opencode_dir, "opencode.json"), "w") as f: | ||
| json.dump(config, f, indent=2) | ||
|
|
||
| def sandbox_config_mounts(self, config_dir): | ||
| host_path = os.path.join(config_dir, ".config", "opencode", "opencode.json") | ||
| if os.path.exists(host_path): | ||
| return [(host_path, f"{self._SANDBOX_CONFIG_DIR}/opencode.json")] | ||
| return [] |
There was a problem hiding this comment.
Podman receives the wrong opencode.json destination path.
Line 383 hardcodes /sandbox/.config/opencode, but Podman OpenCode runs with OPENCODE_CONFIG_DIR=/home/agent-ci/.config/opencode (container contract). With Line 397, Podman mounts opencode.json into the OpenShell path, so OTEL enablement can be ignored in Podman.
Suggested fix (contract-level; backend-aware mount target)
# src/agentic_ci/harness.py
-class Harness(ABC):
+class Harness(ABC):
@@
- def sandbox_config_mounts(self, config_dir):
+ def sandbox_config_mounts(self, config_dir, runtime: str):
...
return []
class OpenCodeHarness(Harness):
- _SANDBOX_CONFIG_DIR = "/sandbox/.config/opencode"
+ _PODMAN_CONFIG_DIR = "/home/agent-ci/.config/opencode"
+ _OPENSHELL_CONFIG_DIR = "/sandbox/.config/opencode"
- def sandbox_config_mounts(self, config_dir):
+ def sandbox_config_mounts(self, config_dir, runtime: str):
host_path = os.path.join(config_dir, ".config", "opencode", "opencode.json")
if os.path.exists(host_path):
- return [(host_path, f"{self._SANDBOX_CONFIG_DIR}/opencode.json")]
+ target_dir = (
+ self._OPENSHELL_CONFIG_DIR if runtime == "openshell" else self._PODMAN_CONFIG_DIR
+ )
+ return [(host_path, f"{target_dir}/opencode.json")]
return []# src/agentic_ci/backends/podman.py
- for host_path, container_path in self.harness.sandbox_config_mounts(self._config_dir):
+ for host_path, container_path in self.harness.sandbox_config_mounts(
+ self._config_dir, runtime="podman"
+ ):
vols.extend(["-v", f"{host_path}:{container_path}:ro,z"])# src/agentic_ci/backends/openshell/__init__.py
- for host_path, container_path in self.harness.sandbox_config_mounts(config_dir):
+ for host_path, container_path in self.harness.sandbox_config_mounts(
+ config_dir, runtime="openshell"
+ ):
...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/harness.py` around lines 383 - 398, The _SANDBOX_CONFIG_DIR
constant hardcodes the path as /sandbox/.config/opencode, but the Podman
container contract specifies that OpenCode config should be mounted to
/home/agent-ci/.config/opencode (matching the OPENCODE_CONFIG_DIR environment
variable in Podman). Update the _SANDBOX_CONFIG_DIR constant value to
/home/agent-ci/.config/opencode so that the sandbox_config_mounts method returns
the correct destination path when mounting the opencode.json file to the
container.
EmilienM
left a comment
EmilienM
left a comment
There was a problem hiding this comment.
Nice work on the OTel support for OpenCode. The implementation looks clean and the docs are a good addition.
One gap: the new write_sandbox_config() and sandbox_config_mounts() methods on OpenCodeHarness don't have any test coverage. These are doing real work (writing JSON config files, returning mount paths) and are the core mechanism for how OpenCode enables OTel, so they should have tests.
Specifically, I'd expect to see:
write_sandbox_configwith otel_enabled=True: verify it writesopencode.jsonwithexperimental.openTelemetry: truewrite_sandbox_configwith otel_enabled=False: verify it writes the config without the experimental blocksandbox_config_mounts: verify it returns the correct(host_path, container_path)tuple when the config file exists, and an empty list when it doesn't
Also worth adding a test_build_env_script_lines_with_otel for OpenCode (Claude Code already has one at line 145) to verify the OTEL env vars show up in the env script when otel_port is set.
|
Because OpenCode can drop spans on exit, I would add a validation check against local session artifacts, not just collector receipt. For one run, compare expected agent turns/tool calls from OpenCode session output against exported OTLP spans/logs: session id, trace id, span count, missing tool spans, usage fields present, flush mode, and process exit timing. That turns the upstream flush caveat into a measurable known-loss report. Generated with ax - https://github.com/Necmttn/ax |
3 participants
Replaces #85 (now pushed from upstream branch).
Summary
Enable OTel trace export for the OpenCode harness, matching the existing Claude Code support.
Changes
build_otel_exec_env()andbuild_env_script_lines()with OpenCode-specific OTel env vars (OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_PROTOCOL,OTEL_BSP_SCHEDULE_DELAY=0). Setsupports_otel = True.write_sandbox_config()hook to theHarnessbase class. OpenCode implementation writesopencode.jsonwithexperimental.openTelemetry: true(required by OpenCode to enable OTel). Addsandbox_config_mounts()to return(host, container)path pairs so backends don't hardcode container paths.write_sandbox_config()during setup, mount config viasandbox_config_mounts()._upload_sandbox_config()usingsandbox_config_mounts(). Delegate OTel env vars to the harness instead of hardcoding Claude-specific vars. GuardCLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFICto only set for Claude Code harness.docs/otel-configuration.mddocumenting all OTel env vars per harness and why each is harness-specific.Note
OpenCode's OTel trace export has an upstream limitation:
process.exit()can kill spans before the batch processor flushes.OTEL_BSP_SCHEDULE_DELAY=0mitigates this but may not catch all spans. Full fix depends on upstream OpenCode changes.Summary by CodeRabbit
Documentation
New Features