t1455: add non-code routine execution guidance and helper#4251
t1455: add non-code routine execution guidance and helper#4251marcusquinn 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 enhances the system's capability to manage and automate non-code operational tasks by providing clear guidelines for distinguishing between code-change workflows and direct operational execution. It introduces a structured approach for designing and deploying recurring routines and offers a helper script to streamline the scheduling process. These changes aim to improve efficiency and reduce the overhead associated with automating routine tasks that do not require code modifications or pull request traceability. 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 (4)
WalkthroughThis pull request introduces a structured framework for designing and scheduling non-code operational routines, with updated dispatch documentation to distinguish code-change work from operational task execution, alongside a new Bash utility script that plans and installs scheduled routines via cron or launchd. Changes
Sequence DiagramsequenceDiagram
participant User
participant Routine Helper as routine-helper.sh
participant Opencode as opencode run
participant System as cron/launchd
User->>Routine Helper: plan --name --schedule --prompt
Routine Helper->>Routine Helper: validate inputs & cron syntax
Routine Helper->>Routine Helper: build opencode invocation
Routine Helper-->>User: show plan & feasibility
User->>Routine Helper: install-cron/install-launchd
Routine Helper->>Routine Helper: parse cron to launchd XML (if launchd)
Routine Helper->>System: register job (cron entry or plist)
System-->>Routine Helper: confirmation
System->>System: at scheduled time
System->>Opencode: invoke with targets_cmd, run_template
Opencode->>Opencode: execute SOP against targets
Opencode-->>System: report outcomes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
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)
📝 Coding Plan
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 �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Mar 12 21:52:50 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 new concept of 'operational routines' for non-code tasks, distinguishing them from code-change tasks that use /full-loop. It adds extensive documentation for this new workflow and a new helper script, routine-helper.sh, to facilitate scheduling these routines with cron or launchd. The changes are well-documented and the new script is robust. I've found a minor documentation issue and a high-severity XML injection vulnerability in the new helper script that could lead to malformed launchd configuration files.
.agents/scripts/routine-helper.sh
Outdated
| <array> | ||
| <string>/bin/bash</string> | ||
| <string>-lc</string> | ||
| <string>${command}</string> |
There was a problem hiding this comment.
This is a potential XML injection vulnerability. The ${command} variable is inserted directly into the XML heredoc without proper escaping. If the command string contains special XML characters like <, >, or & (e.g., in a prompt with redirection or shell logic), it will generate a malformed .plist file, causing launchctl load to fail. The command string should be XML-escaped before being embedded in the plist.
As per the rule for preventing injection, input must be sanitized and context-specific escaping applied before interpolation. You can achieve this by defining an XML-escaping function or using sed to replace the special characters (e.g., & with &, < with <) before the cat <<EOF block.
References
- To prevent injection vulnerabilities in shell scripts, sanitize input using a strict allowlist and also perform context-specific escaping (e.g., doubling single quotes for SQL strings) before interpolating the value into a query. This principle applies to XML content as well, requiring XML-specific escaping.
| 2. **It dispatches `opencode run` for each item** — one worker per task | ||
| 3. **Code workers** handle branch -> implementation -> PR -> CI -> merge | ||
| 4. **Ops workers** execute the requested SOP/command and report outcomes | ||
| 4. **No databases, no state machines, no complex bash pipelines** |
There was a problem hiding this comment.
There's a minor typo in this numbered list. This item is numbered as 4., but the previous item is also 4.. This should be 5. to maintain the correct sequence.
| 4. **No databases, no state machines, no complex bash pipelines** | |
| 5. **No databases, no state machines, no complex bash pipelines** |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
.agents/AGENTS.md (1)
45-66: Keep the root guide at the decision rule and link out for the workflow.This five-step playbook duplicates
.agents/scripts/commands/routine.mdand adds fairly task-specific guidance to the top-level instructions. Trimming this to the routing rule plus a pointer to/routinewould reduce drift and keep.agents/AGENTS.mdfocused on broadly applicable rules. Based on learnings: "Applies to **/AGENTS.md : Every instruction in AGENTS.md must be relevant to >80% of tasks" and "Applies to **/AGENTS.md : Use progressive disclosure in AGENTS.md by providing pointers to subagents rather than inline content."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/AGENTS.md around lines 45 - 66, The top-level AGENTS.md should not inline the five-step playbook that duplicates .agents/scripts/commands/routine.md; replace the detailed step list (Design the operator..., Prove it ad-hoc..., Harden guardrails..., Pilot safely..., Schedule the command...) with the concise decision rule already present and a single pointer to the routine doc (referencing the existing /routine or .agents/scripts/commands/routine.md) so AGENTS.md stays focused on broadly applicable rules and avoids task-specific guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/commands/routine.md:
- Around line 94-107: Clarify that the example Routine Spec template is not
currently executable with the helper: update the doc text near the "Routine Spec
Template" example to state that .agents/scripts/routine-helper.sh currently only
accepts a literal --prompt and does not wire targets_cmd or run_template into
the plan, install-cron, or install-launchd flows; alternatively, if you prefer
to make it runnable now, implement support in routine-helper.sh to parse
targets_cmd and run_template and pass them through to the
plan/install-cron/install-launchd code paths (modify routine-helper.sh and the
plan/install handlers to accept and expand those fields). Reference the fields
targets_cmd and run_template and the script routine-helper.sh and the commands
plan, install-cron, install-launchd so maintainers can locate the code to
change.
In @.agents/scripts/commands/runners.md:
- Around line 240-242: Update the contradictory bullet that hard-codes
"/full-loop": replace the line that reads '**Always** dispatches workers via
`opencode run "/full-loop ..."` with wording that says workers are dispatched
via `opencode run` and the command is chosen by task type (e.g., code tasks may
use `"/full-loop ..."` while ops routines use a different command), so the doc
reflects dispatch via `opencode run` with command chosen by task type rather
than always using `/full-loop`.
- Around line 34-36: The docs disagree: runners.md implies flexible
execution-mode routing but pulse.md hard-codes dispatches to "/full-loop" (and
counts workers via grep '/full-loop'); fix by either (A) updating runners.md to
explicitly state that the pulse implementation currently always uses the
"/full-loop" path for worker dispatch and counting, or (B) make pulse.md follow
the general routing model by replacing hard-coded "/full-loop" dispatch targets
and the worker-count grep with the generic routing variable/interface used by
runners.md (so functions/commands that call opencode run use the common
execution-mode variable instead of "/full-loop"). Locate occurrences of
"/full-loop", the worker-counting grep, and the dispatch command invocations in
pulse.md and update them accordingly, and update runners.md text to reflect
which behavior is implementation-specific.
In @.agents/scripts/routine-helper.sh:
- Around line 81-85: The validate_routine_prompt function currently only warns
when a prompt contains "/full-loop"; change it to fail fast by returning a
non-zero exit (e.g., call exit 1) after printing an error message to stderr so
callers (plan and install-* flows) stop immediately—update
validate_routine_prompt to detect "/full-loop", print a clear error (not a
warning) to >&2, and then exit non-zero instead of continuing.
- Around line 224-229: ROUTINE_NAME is used verbatim in paths and launchd labels
so add a strict validation step to reject or sanitize unsafe characters before
any interpolation: implement and call a new function (e.g.,
validate_routine_name) that enforces a safe charset (letters, digits,
underscore, hyphen; no spaces, slashes, or other path separators) and returns
non-zero on violation, and invoke it alongside the existing
validate_required/validate_cron_expression/validate_routine_prompt checks;
update references to ROUTINE_NAME usages (cron log path, plist filename, launchd
label) to only proceed when validate_routine_name succeeds (or produce an
explicit error message) to prevent broken artifacts or path traversal.
- Around line 314-338: The plist heredoc injects unescaped dynamic values
(command, HOME, PATH, ROUTINE_NAME, launchd_schedule_xml-derived strings) into
<string> nodes causing XML parsing failures; add a small xml_escape function
(e.g., xml_escape() that replaces & < > " ' with entities) in
.agents/scripts/routine-helper.sh and use it for every interpolated value in the
heredoc: the Label value (sh.aidevops.routine-${ROUTINE_NAME}), the
ProgramArguments command entry (${command}), both EnvironmentVariables (${HOME}
and ${PATH}), and the StandardOutPath/StandardErrorPath outputs; also ensure any
place you insert launchd_schedule_xml is either pre-escaped or generated using
the escape helper so the final plist contains only XML-safe text.
---
Nitpick comments:
In @.agents/AGENTS.md:
- Around line 45-66: The top-level AGENTS.md should not inline the five-step
playbook that duplicates .agents/scripts/commands/routine.md; replace the
detailed step list (Design the operator..., Prove it ad-hoc..., Harden
guardrails..., Pilot safely..., Schedule the command...) with the concise
decision rule already present and a single pointer to the routine doc
(referencing the existing /routine or .agents/scripts/commands/routine.md) so
AGENTS.md stays focused on broadly applicable rules and avoids task-specific
guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4521dc73-352f-4393-a86f-9da6d3ab515d
📒 Files selected for processing (4)
.agents/AGENTS.md.agents/scripts/commands/routine.md.agents/scripts/commands/runners.md.agents/scripts/routine-helper.sh
| 5. Dispatches workers via `opencode run`, routing to the right agent and execution mode | ||
|
|
||
| See `scripts/commands/pulse.md` for the full spec. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only consistency check: verify pulse docs for hard-coded /full-loop assumptions.
set -euo pipefail
fd pulse.md .agents/scripts/commands -t f | while read -r f; do
echo "== $f =="
rg -n --context 2 '/full-loop|WORKER_COUNT|opencode run' "$f"
doneRepository: marcusquinn/aidevops
Length of output: 7206
Clarify execution mode routing: runners.md describes flexible routing, but pulse.md hard-codes /full-loop throughout.
The runners.md description ("routing to the right agent and execution mode") implies a flexible routing model, but pulse.md consistently hard-codes /full-loop for all worker dispatches (lines 195, 578, 659, 711, 996, 1007, 1020, 1054) and worker counting (line 44: grep '/full-loop'). Either update runners.md to specify that pulse uses /full-loop dispatch, or document the general routing model more clearly to distinguish it from this specific implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/commands/runners.md around lines 34 - 36, The docs disagree:
runners.md implies flexible execution-mode routing but pulse.md hard-codes
dispatches to "/full-loop" (and counts workers via grep '/full-loop'); fix by
either (A) updating runners.md to explicitly state that the pulse implementation
currently always uses the "/full-loop" path for worker dispatch and counting, or
(B) make pulse.md follow the general routing model by replacing hard-coded
"/full-loop" dispatch targets and the worker-count grep with the generic routing
variable/interface used by runners.md (so functions/commands that call opencode
run use the common execution-mode variable instead of "/full-loop"). Locate
occurrences of "/full-loop", the worker-counting grep, and the dispatch command
invocations in pulse.md and update them accordingly, and update runners.md text
to reflect which behavior is implementation-specific.
🔍 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: Thu Mar 12 22:07:12 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
…stion The validate_routine_name charset was ^[A-Za-z0-9_-]+$ but the coderabbit review (issue #4273) suggested ^[A-Za-z0-9._-]+$ to allow names like 'seo.weekly'. Update the regex and error message to match. All three HIGH findings from PR #4251 review are now fully resolved: - validate_routine_name enforces safe charset (this commit adds dot support) - xml_escape() applied to all plist <string> values (c2bb03f) - ROUTINE_NAME validated before path/label interpolation (c2bb03f) shellcheck: zero issues
…stion (#4292) The validate_routine_name charset was ^[A-Za-z0-9_-]+$ but the coderabbit review (issue #4273) suggested ^[A-Za-z0-9._-]+$ to allow names like 'seo.weekly'. Update the regex and error message to match. All three HIGH findings from PR #4251 review are now fully resolved: - validate_routine_name enforces safe charset (this commit adds dot support) - xml_escape() applied to all plist <string> values (c2bb03f) - ROUTINE_NAME validated before path/label interpolation (c2bb03f) shellcheck: zero issues
1 participant
Summary
/full-loopis for code-change tasks while recurring operational tasks should run direct agent commands/routinecommand guidance covering SOP/targets/schedule design, pilot rollout, and safety gatesroutine-helper.shto plan and install scheduledopencode runroutines for cron/launchd usageVerification
shellcheck .agents/scripts/routine-helper.shmarkdown-formatter checkfor updated/new command docs and.agents/AGENTS.md.agents/scripts/routine-helper.sh plan --name seo-weekly --schedule "0 9 * * 1" --dir "/tmp" --agent "SEO" --title "Weekly rankings" --prompt "/seo-export --account client-a --format summary"Closes #4250
Summary by CodeRabbit
Documentation
New Features