GH#17829: Fix headless dispatch when opencode serve is running + triage failure visibility#17866
Conversation
Bug 1: Add @bufbuild/protobuf to plugin dependencies and run npm install during plugin deployment when the module is missing from symlinked node_modules. Bug 2: Detect running opencode server via OPENCODE_SERVER_PASSWORD + port check, and add --attach/--password to both _build_run_cmd and _run_canary_test. Does NOT rely on OPENCODE_PID (intentionally excluded from worker envs per GH#6668). Workflow: Add triage-failed label when triage review output is suppressed by the safety filter, so failed triage is visible on the issue timeline instead of only appearing in pulse logs.
Completion Summary
|
|
Caution Review failedPull request was closed or merged during review WalkthroughThese changes resolve critical bugs blocking headless worker dispatch when an Changes
Sequence DiagramsequenceDiagram
participant Helper as headless-runtime-helper
participant Detect as _detect_opencode_server()
participant Curl as HTTP Endpoint
participant Lsof as Port Listener Check
participant Server as Running<br/>opencode serve
participant Build as Command Builder
participant Run as opencode run
Helper->>Detect: Check OPENCODE_PID & OPENCODE_SERVER_PASSWORD
Detect->>Curl: GET /api/session/list<br/>on localhost:4096
alt Server responds (200 or 401)
Curl-->>Detect: Success
else Curl fails
Detect->>Lsof: Check if port 4096<br/>is listening
Lsof-->>Detect: Process found
end
Detect-->>Helper: Server detected + URL
Helper->>Build: Append attach args<br/>--attach --password
Build-->>Run: Execute with server connection
Run->>Server: Connect & reuse session
Server-->>Run: Session ready
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report SonarCloud: 0 bugs, 0 vulnerabilities, 649 code smells Wed Apr 8 14:50:07 UTC 2026: Code review monitoring started 📈 Current Quality Metrics
Generated on: Wed Apr 8 14:50:10 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Completion Summary
Merged via PR #17866 to main.
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces server detection to prevent session conflicts by allowing commands to attach to existing OpenCode servers, adds a new protobuf dependency, and implements a labeling system to surface triage failures on GitHub issues. Feedback focuses on optimizing the server detection logic to handle HTTP status codes more efficiently and improving the robustness of the plugin dependency installation process by avoiding error suppression and handling symlinks correctly.
| if curl -sf --max-time 2 -o /dev/null "${url}/api/session/list" 2>/dev/null || | ||
| curl -sf --max-time 2 -o /dev/null -w '%{http_code}' "${url}/api/session/list" 2>/dev/null | grep -q '401'; then | ||
| printf '%s\n%s\n' "$url" "$password" | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
The logic for detecting the OpenCode server is inefficient and contains a bug. The second curl command uses the -f flag, which causes curl to exit with an error and produce no output when a 401 Unauthorized response is received. Additionally, performing two separate network calls is redundant. Please remove the blanket 2>/dev/null suppression to ensure authentication or system issues remain visible for debugging.
| if curl -sf --max-time 2 -o /dev/null "${url}/api/session/list" 2>/dev/null || | |
| curl -sf --max-time 2 -o /dev/null -w '%{http_code}' "${url}/api/session/list" 2>/dev/null | grep -q '401'; then | |
| printf '%s\n%s\n' "$url" "$password" | |
| return 0 | |
| fi | |
| local http_code | |
| http_code=$(curl -s --max-time 2 -o /dev/null -w '%{http_code}' "${url}/api/session/list") | |
| if [[ "$http_code" == "200" || "$http_code" == "401" ]]; then | |
| printf '%s\n%s\n' "$url" "$password" | |
| return 0 | |
| fi |
References
- Avoid using 2>/dev/null for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
| if [[ -d "$oc_node_modules" ]]; then | ||
| ln -sf "$oc_node_modules" "$plugin_dir/node_modules" 2>/dev/null || true | ||
| fi | ||
| # Verify critical dependency is available; npm install if not | ||
| if [[ ! -d "$plugin_dir/node_modules/@bufbuild/protobuf" ]]; then | ||
| if command -v npm &>/dev/null; then | ||
| npm install --omit=dev --prefix "$plugin_dir" >/dev/null 2>&1 || | ||
| print_warning "Failed to install plugin dependencies (non-blocking)" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The dependency installation logic has potential issues. Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified or if the operation is critical. Masking these errors can hide permission issues. Please remove the redundant redirection.
| if [[ -d "$oc_node_modules" ]]; then | |
| ln -sf "$oc_node_modules" "$plugin_dir/node_modules" 2>/dev/null || true | |
| fi | |
| # Verify critical dependency is available; npm install if not | |
| if [[ ! -d "$plugin_dir/node_modules/@bufbuild/protobuf" ]]; then | |
| if command -v npm &>/dev/null; then | |
| npm install --omit=dev --prefix "$plugin_dir" >/dev/null 2>&1 || | |
| print_warning "Failed to install plugin dependencies (non-blocking)" | |
| fi | |
| fi | |
| if [[ -d "$oc_node_modules" ]]; then | |
| rm -rf "$plugin_dir/node_modules" | |
| ln -sf "$oc_node_modules" "$plugin_dir/node_modules" | |
| fi | |
| # Verify critical dependency is available; npm install if not | |
| if [[ ! -d "$plugin_dir/node_modules/@bufbuild/protobuf" ]]; then | |
| if command -v npm >/dev/null; then | |
| # Remove symlink to perform a clean local install if shared one is insufficient | |
| [[ -L "$plugin_dir/node_modules" ]] && rm "$plugin_dir/node_modules" | |
| npm install --omit=dev --prefix "$plugin_dir" >/dev/null 2>&1 || | |
| print_warning "Failed to install plugin dependencies (non-blocking)" | |
| fi | |
| fi |
References
- Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified by a preceding check. This practice is redundant and can mask other important issues like permissions problems.
marcusquinn
added
the
review-feedback-scanned
1 participant
Summary
Resolves #17829
Three fixes for headless worker dispatch and triage visibility:
Bug 1 — Missing plugin dependency: Added
@bufbuild/protobuftoopencode-aidevopsplugindependenciesinpackage.json. Updatedagent-deploy.shto verify the dependency exists after symlinking OpenCode'snode_modules, and falls back tonpm install --omit=devwhen it's missing.Bug 2 — Headless dispatch conflict with running server: Added
_detect_opencode_server()helper that checksOPENCODE_SERVER_PASSWORD+ port liveness (via curl health check then lsof fallback). Both_build_run_cmdand_run_canary_testnow append--attach <url> --password <password>when a server is detected. Does NOT rely onOPENCODE_PID(intentionally excluded from worker envs per GH#6668).Workflow improvement — Triage failure visibility: When the triage review output is suppressed by the safety filter (infra markers, missing
## Review:header, or empty output), atriage-failedlabel is now added to the issue. This makes failed triage visible on the issue timeline instead of only appearing in pulse logs. The label is automatically removed when a successful triage review is posted.Files Changed
EDIT: .agents/plugins/opencode-aidevops/package.json— addeddependencies.@bufbuild/protobufEDIT: setup-modules/agent-deploy.sh— verify critical dep after symlink, npm install fallbackEDIT: .agents/scripts/headless-runtime-helper.sh—_detect_opencode_server(), patched_build_run_cmdand_run_canary_testEDIT: .agents/scripts/pulse-wrapper.sh—triage-failedlabel on suppressed triage outputRuntime Testing
self-assessed— no opencode serve environment available locally to runtime-test. ShellCheck clean. JSON valid. Logic verified against issue reproduction steps.Key Decisions
OPENCODE_PORTenv (default 4096), verify via curl health check on/api/session/list(returns 401 without auth but proves server is up), fallback tolsofport check. More robust than hardcoding port 4096.OPENCODE_SERVER_PASSWORD+ port liveness, notOPENCODE_PID. This works for both the pulse's own context (where PID is set) and isolated worker envs (where PID is stripped per GH#6668).aidevops.sh v3.6.174 plugin for OpenCode v1.4.0 with claude-opus-4-6 spent 34m and 19,836 tokens on this with the user in an interactive session.
Summary by CodeRabbit
New Features
Improvements