t1433: add provider-aware headless runtime for pulse and workers#4116
t1433: add provider-aware headless runtime for pulse and workers#4116alex-solovyev merged 2 commits intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and efficiency of headless OpenCode operations by introducing a sophisticated runtime helper. This helper intelligently manages model selection, persists sessions across runs, and implements a dynamic backoff mechanism for API providers, ensuring that automated tasks can gracefully handle transient failures and rate limits without manual intervention. The changes streamline the execution of pulse and worker dispatches, making the system more resilient and self-correcting. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe changes introduce a provider-aware headless runtime wrapper ( Changes
Sequence Diagram(s)sequenceDiagram
participant Dispatch as Pulse Dispatch<br/>(pulse.md)
participant Helper as headless-runtime-<br/>helper.sh
participant StateDB as SQLite<br/>state.db
participant Provider as Provider<br/>(Anthropic/OpenAI)
participant OpenCode as OpenCode<br/>Runtime
Dispatch->>Helper: run --role task --session-key task-123<br/>--prompt "execute subtask"
Helper->>Helper: init_state_db if needed
Helper->>StateDB: check provider_backoff for active windows
StateDB-->>Helper: backoff status
alt Backoff Active
Helper-->>Dispatch: skip (backoff window active)
else Backoff Expired or None
Helper->>StateDB: get last_provider, rotation state
StateDB-->>Helper: provider history
Helper->>Helper: choose_model (rotate or override)
Helper->>Helper: get_auth_signature for selected provider
Helper->>OpenCode: opencode run --model selected_model<br/>--session-id persisted_session<br/>--prompt from file
OpenCode-->>Helper: output + exit_code
alt Exit Success
Helper->>Helper: extract_session_id_from_output
Helper->>StateDB: store_session_id for provider
Helper-->>Dispatch: return (0)
else Exit Failure
Helper->>Helper: classify_failure_reason (rate_limit/auth_error/provider_error)
Helper->>Helper: parse_retry_after_seconds
Helper->>StateDB: record_provider_backoff with window
Helper-->>Dispatch: return (1)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
|
Dispatching worker to rebase onto main and resolve merge conflicts.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.agents/scripts/pulse-wrapper.sh (1)
104-105: Design is sound — model selection properly delegated to headless helper.The empty
PULSE_MODELdefault is intentional and safe:headless-runtime-helper.shprovidesDEFAULT_HEADLESS_MODELS="anthropic/claude-sonnet-4-6,openai/gpt-5.3-codex"as fallback (context snippet 3, line 18). This enables provider rotation without hard-coding a single model here.Consider adding an existence check for the helper script to surface misconfiguration early rather than failing silently in
run_pulse():🛡️ Optional defensive check
PULSE_MODEL="${PULSE_MODEL:-}" HEADLESS_RUNTIME_HELPER="${HEADLESS_RUNTIME_HELPER:-${SCRIPT_DIR}/headless-runtime-helper.sh}" + +if [[ ! -x "$HEADLESS_RUNTIME_HELPER" ]]; then + echo "[pulse-wrapper] ERROR: headless-runtime-helper.sh not found or not executable at ${HEADLESS_RUNTIME_HELPER}" >>"$LOGFILE" + # Fall back to direct opencode if helper is missing (graceful degradation) +fiAs per coding guidelines: "Automation scripts - focus on: Reliability and robustness, Clear logging and feedback".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/pulse-wrapper.sh around lines 104 - 105, Add a defensive check that verifies the HEADLESS_RUNTIME_HELPER file exists and is executable before run_pulse() is invoked: if HEADLESS_RUNTIME_HELPER (the variable set from SCRIPT_DIR/headless-runtime-helper.sh) is missing or not executable, log a clear error message referencing HEADLESS_RUNTIME_HELPER and SCRIPT_DIR and exit with a non‑zero status so misconfiguration is surfaced early; keep PULSE_MODEL default behavior unchanged and ensure run_pulse() will not proceed when the helper is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/pulse-wrapper.sh:
- Around line 104-105: Add a defensive check that verifies the
HEADLESS_RUNTIME_HELPER file exists and is executable before run_pulse() is
invoked: if HEADLESS_RUNTIME_HELPER (the variable set from
SCRIPT_DIR/headless-runtime-helper.sh) is missing or not executable, log a clear
error message referencing HEADLESS_RUNTIME_HELPER and SCRIPT_DIR and exit with a
non‑zero status so misconfiguration is surfaced early; keep PULSE_MODEL default
behavior unchanged and ensure run_pulse() will not proceed when the helper is
absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5062987e-c9bd-44b0-9d76-4cb8b82ce168
📒 Files selected for processing (5)
.agents/scripts/commands/pulse.md.agents/scripts/headless-runtime-helper.sh.agents/scripts/pulse-wrapper.shsetup.shtests/test-headless-runtime-helper.sh
marcusquinn
force-pushed
the
feature/provider-aware-headless-runtime
branch
from
eeef6f7 to
fdc852a
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Mar 11 03:07:33 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
Route pulse and worker headless runs through a shared helper so provider backoff, session reuse, and no-Zen fallback are enforced consistently. This lets headless automation pause cleanly during subscription rate limits and resume automatically when auth changes or retry windows expire.
alex-solovyev
force-pushed
the
feature/provider-aware-headless-runtime
branch
from
fdc852a to
8340831
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Mar 11 03:09:09 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![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 a robust headless-runtime-helper.sh script to manage headless opencode runs for background workers and the supervisor pulse, handling provider rotation, session persistence, and a sophisticated backoff mechanism. A security audit, however, identified two high-severity command injection vulnerabilities: one in headless-runtime-helper.sh where the backoff set command fails to validate the retry_seconds argument, and another in setup.sh where the _cron_escape function's failure to handle newlines allows crontab injection. Additionally, a critical portability bug and a couple of medium-severity issues were found in the new helper script that could improve robustness and correctness.
I am having trouble creating individual review comments. Click here to see my feedback.
.agents/scripts/headless-runtime-helper.sh (349)
The date command used to parse the stored_retry_after timestamp is not portable and will fail on macOS. The date -j -u -f ... combination is invalid because the -u flag is not compatible with -j on BSD-style date (like on macOS).
This will cause the first part of the || chain to fail, and since GNU date (with -d) is usually not present on macOS, the command will fall back to printf '%s' "0". This sets retry_epoch to 0, causing the backoff to be cleared immediately on every check, rendering the backoff mechanism ineffective on macOS.
To fix this, remove the invalid -u flag from the macOS portion of the command.
retry_epoch=$(date -j -f '%Y-%m-%dT%H:%M:%SZ' "$stored_retry_after" '+%s' 2>/dev/null || date -u -d "$stored_retry_after" '+%s' 2>/dev/null || printf '%s' "0")
.agents/scripts/headless-runtime-helper.sh (550-554)
The backoff set command takes a retry_seconds argument and passes it directly to the date command without validation. An attacker who can control the arguments to this script can achieve arbitrary command execution via command injection.
Remediation: Validate that retry_seconds is an integer before using it in the date command.
if [[ "$retry_seconds" != "300" ]]; then
if [[ ! "$retry_seconds" =~ ^[0-9]+$ ]]; then
print_error "retry_seconds must be an integer"
return 1
fi
local retry_after
retry_after=$(date -u -v+"${retry_seconds}"S '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null || date -u -d "+${retry_seconds} seconds" '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null || printf '%s' "")
db_query "UPDATE provider_backoff SET retry_after = '$(sql_escape "$retry_after")' WHERE provider = '$(sql_escape "$provider")';" >/dev/null
fi
References
- Employ a defense-in-depth strategy for handling user-provided input. Sanitize input at the entry point using a strict allowlist, and also apply context-specific escaping or safe handling mechanisms (e.g., parameterized queries for SQL,
--argforjq) at each point of use. - When parsing delimited data from an untrusted source in a shell script, validate numeric fields before using them in calculations. This provides defense-in-depth against injection attacks that could result from delimiter shifting caused by malicious data.
setup.sh (995)
The setup.sh script is vulnerable to crontab injection via the AIDEVOPS_HEADLESS_MODELS and AIDEVOPS_HEADLESS_PROVIDER_ALLOWLIST environment variables. The _cron_escape function (line 105) only escapes single quotes and does not handle newlines. When these variables are interpolated into the crontab entry string at line 995, an attacker can inject new lines to create arbitrary crontab entries, potentially leading to unauthorized command execution.
Remediation: Modify the _cron_escape function to remove or escape newlines in the input string.
References
- To prevent command injection from shell metacharacters in crontab entries, wrap variables in single quotes. This is a POSIX-compliant method to prevent all shell expansion, including
$(...), backticks, and variable expansion. - Employ a defense-in-depth strategy for handling user-provided input. Sanitize input at the entry point using a strict allowlist, and also apply context-specific escaping or safe handling mechanisms (e.g., parameterized queries for SQL,
--argforjq) at each point of use.
.agents/scripts/headless-runtime-helper.sh (95-107)
The fallback behavior in sha256_text when no hashing command is found is to return the original value. This is problematic because it's not a hash and could lead to unexpected behavior in get_auth_signature, especially around detecting auth changes. It's safer to fail fast if a required tool is missing.
I suggest modifying the function to exit with an error if neither shasum nor sha256sum is available.
sha256_text() {
local value="$1"
if command -v shasum >/dev/null 2>&1; then
printf '%s' "$value" | shasum -a 256 | awk '{print $1}'
elif command -v sha256sum >/dev/null 2>&1; then
printf '%s' "$value" | sha256sum | awk '{print $1}'
else
print_error "FATAL: sha256_text requires 'shasum' or 'sha256sum' but neither was found." >&2
exit 1
fi
}
References
- In shell scripts, functions that wrap a command should propagate its exit code to the caller. Avoid masking potential errors with a hardcoded
return 0. Instead, usereturn $?or capture the exit code and return it (e.g.,local rc=$?; return $rc). - All functions in shell scripts must have explicit
returnstatements to ensure predictable behavior and exit code propagation.
.agents/scripts/headless-runtime-helper.sh (426)
There is a duplicate "step-finish" string in the set of event types. While this doesn't cause a functional issue in Python sets, it indicates a small oversight. Removing the duplicate will make the code cleaner.
if event_type in {"text", "tool", "tool-invocation", "tool-result", "step-start", "step_finish", "reasoning"}:
Surface pulse misconfiguration immediately instead of letting the wrapper reach run_pulse with a missing helper path. This makes provider-aware headless dispatch failures obvious during setup and runtime restarts.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Mar 11 03:10:44 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Commit 3987b05 re-introduced inline PATH="/usr/local/bin:/usr/bin:/bin" into the supervisor-pulse cron entry, overriding the global PATH managed by _ensure_cron_path(). This broke opencode discovery on nvm/fnm/volta systems — node was unreachable, causing 1634+ failed pulse starts. Also removes OPENCODE_BIN (resolved from PATH at runtime via command -v). Reverts the PATH portion of 3987b05, preserving headless runtime. Closes #4240
…#4241) Commit 3987b05 re-introduced inline PATH="/usr/local/bin:/usr/bin:/bin" into the supervisor-pulse cron entry, overriding the global PATH managed by _ensure_cron_path(). This broke opencode discovery on nvm/fnm/volta systems — node was unreachable, causing 1634+ failed pulse starts. Also removes OPENCODE_BIN (resolved from PATH at runtime via command -v). Reverts the PATH portion of 3987b05, preserving headless runtime. Closes #4240
2 participants
Summary
opencode rundispatchesCloses #4115
Summary by CodeRabbit
New Features
Tests
Documentation