Skip to content

t274: Fix decomposition worker dispatch to use standard CLI and process patterns#1066

Merged
marcusquinn merged 1 commit intomainfrom
feature/t274
Feb 11, 2026
Merged

t274: Fix decomposition worker dispatch to use standard CLI and process patterns#1066
marcusquinn merged 1 commit intomainfrom
feature/t274

Conversation

@marcusquinn
Copy link
Owner

@marcusquinn marcusquinn commented Feb 11, 2026

Summary

Fixes the decomposition worker dispatch (t274) to use the standard CLI resolution and process management patterns, addressing issues found in the initial implementation merged via PR #1060.

Changes

dispatch_decomposition_worker() improvements:

  • CLI resolution: Replace hardcoded Claude (capital C) with resolve_ai_cli() — supports opencode (primary) and claude (deprecated fallback)
  • PID-based throttle: Check for already-running decomposition worker before dispatching another, preventing duplicate workers across pulse cycles
  • Standard dispatch pattern: Use dispatch + wrapper scripts with nohup/setsid matching cmd_dispatch() pattern for proper background process management
  • Metadata fix: Use CASE WHEN SQL to handle empty metadata (avoids leading comma from metadata || ',')
  • Worker prompt: Add explicit PLANS.md edit restriction and #### Phases section hint

Strategy 3 subtask detection:

  • Regex fix: Include cancelled [-] checkbox state in subtask detection (was only matching [ ], [x], [X])

Testing

  • Bash syntax validation (bash -n): PASS
  • ShellCheck validation (extracted function with stubs): PASS (zero violations)
  • Verified CLI invocation patterns match existing build_dispatch_cmd() output format

Related

Summary by CodeRabbit

  • Bug Fixes

    • Prevents concurrent decomposition workers for the same task
    • Enhanced checkbox state detection in auto-pickup logic
  • New Features

    • Improved decomposition dispatch with better process isolation
    • Optimized plan-driven decomposition triggering with explicit AI CLI selection

…tion worker (t274)

- Replace hardcoded "Claude" CLI with resolve_ai_cli() for opencode/claude support
- Add PID-based throttle to prevent duplicate decomposition workers per pulse
- Use nohup/setsid dispatch + wrapper scripts matching cmd_dispatch() pattern
- Fix metadata concatenation to handle empty metadata (avoids leading comma)
- Improve subtask detection regex to include cancelled [-] checkbox state
- Add explicit PLANS.md edit restriction to decomposition worker prompt
@gemini-code-assist
Copy link

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

Walkthrough

The change introduces a robust, throttled decomposition worker dispatch system in the supervisor helper script. It adds PID-based throttling to prevent concurrent workers, dynamic AI CLI resolution with fallback logic, script-based dispatch pattern replacement, and enhanced process lifecycle tracking with improved logging and monitoring across decomposition workflows.

Changes

Cohort / File(s) Summary
Decomposition Worker Dispatch & PID Management
.agents/scripts/supervisor-helper.sh
Implements PID throttling for decomposition workers to prevent concurrent instances per task. Introduces dynamic AI CLI resolution (opencode preferred, claude fallback), replaces Claude-based dispatch with script-based approach using dedicated dispatch and wrapper scripts. Adds nohup/setsid process launching with disown and PID file tracking. Enhances logging for selected AI CLI, dispatcher paths, and worker metadata. Expands checkbox state detection in plan-driven decomposition to include [ ], [x], [X], [-]. Adds plan anchor derivation and decomposition trigger logic with improved process isolation and directory creation for logs/pids.

Sequence Diagram

sequenceDiagram
    participant Supervisor as Supervisor<br/>Helper
    participant PIDMgr as PID File<br/>Manager
    participant CLIRes as AI CLI<br/>Resolver
    participant Dispatcher as Dispatch<br/>Script Gen
    participant Wrapper as Wrapper<br/>Script Gen
    participant Launcher as Process<br/>Launcher
    participant DB as Task<br/>Database
    
    Supervisor->>PIDMgr: Check for existing {task_id}-decompose.pid
    alt PID exists & valid
        PIDMgr-->>Supervisor: PID is alive, log already running
        Supervisor->>Supervisor: Skip decomposition (throttled)
    else PID stale/missing
        PIDMgr-->>Supervisor: Ready to dispatch
        Supervisor->>CLIRes: resolve_ai_cli() — pick opencode or claude
        CLIRes-->>Supervisor: Return selected AI CLI
        Supervisor->>Dispatcher: Create dispatch script<br/>with selected CLI + formatted prompt
        Dispatcher-->>Supervisor: Dispatch script created at pids/{task_id}-decompose-dispatch.sh
        Supervisor->>Wrapper: Create wrapper script<br/>to manage child, log exit codes, cleanup
        Wrapper-->>Supervisor: Wrapper script created at pids/{task_id}-decompose-wrapper.sh
        Supervisor->>Launcher: Launch wrapper via nohup + setsid<br/>with disown + background tracking
        Launcher-->>Supervisor: Worker PID obtained
        Supervisor->>PIDMgr: Store worker PID in {task_id}-decompose.pid
        PIDMgr-->>Supervisor: PID file updated
        Supervisor->>DB: Update task metadata with decomposition worker PID
        DB-->>Supervisor: Metadata committed
        Supervisor->>Supervisor: Log dispatch details & monitor
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #377: Modifies supervisor-helper.sh with AI-CLI resolution and background worker dispatch/PID management for decomposition workflows — directly aligned with the script-based dispatch and worker lifecycle changes.
  • PR #431: Hardens background worker dispatch in the same file using nohup/disown and improved PID handling with exit-code capture — closely related to process launching and lifecycle control patterns.
  • PR #384: Touches supervisor-helper.sh dispatch and worker lifecycle logic; introduces worker cleanup, descendant-kill, and metadata plumbing — complements this PR's PID throttling with cleanup/termination mechanics.

Poem

🔄 With PID files standing guard and scripts that dance,
Decomposition workers throttle at a glance,
No more concurrent chaos—each task gets its turn,
While wrapper scripts log every exit and burn,
Zero Technical Debt marches on! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 't274: Fix decomposition worker dispatch to use standard CLI and process patterns' directly and accurately summarizes the main change: refactoring the decomposition worker dispatch mechanism to align with standard patterns.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/t274

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 46 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Wed Feb 11 03:02:50 UTC 2026: Code review monitoring started
Wed Feb 11 03:02:51 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 46

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 46
  • VULNERABILITIES: 0

Generated on: Wed Feb 11 03:02:54 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

@marcusquinn marcusquinn merged commit f0b42fa into main Feb 11, 2026
9 of 10 checks passed
@marcusquinn marcusquinn deleted the feature/t274 branch February 11, 2026 03:04
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @.agents/scripts/supervisor-helper.sh:
- Around line 12713-12716: The script is passing a --metadata flag to cmd_add
(and updating tasks.metadata) which fails because cmd_add rejects unknown flags
and the metadata column may not exist; remove the --metadata usage on cmd_add
and instead persist the plan_anchor/decomposition_worker_pid via a separate DB
update after cmd_add returns (use the existing db function and sql_escape with
the task id and worker PID), or if you prefer to keep metadata semantics add a
DB migration to create the metadata column and add --metadata handling in
cmd_add; update the code paths referencing decomposition_worker_pid (the db
"UPDATE tasks SET metadata = ..." block and any cmd_add invocations) to follow
the chosen approach.
- Around line 12546-12551: The code reads the PID file using cat which triggers
ShellCheck SC2002; instead, read the PID into existing_pid without using cat by
using shell builtins (e.g., read or filename expansion) where the variable
pid_file and the existing_pid assignment occur; update the block that creates
pid_file and checks kill -0 "$existing_pid" to populate existing_pid via read or
$(<"$pid_file") so ShellCheck no longer flags a useless cat.
🧹 Nitpick comments (1)
.agents/scripts/supervisor-helper.sh (1)

12677-12696: Consider recursive cleanup to avoid orphaned grandchildren.

The wrapper only terminates direct children; opencode/CLI stacks can spawn nested processes. Mirroring the recursive cleanup used in cmd_dispatch would tighten reliability and keep the process tree clean.

Comment on lines +12546 to +12551
# Check for already-running decomposition worker (throttle)
local pid_file="$SUPERVISOR_DIR/pids/${task_id}-decompose.pid"
if [[ -f "$pid_file" ]]; then
local existing_pid
existing_pid=$(cat "$pid_file" 2>/dev/null || true)
if [[ -n "$existing_pid" ]] && kill -0 "$existing_pid" 2>/dev/null; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid SC2002 by reading the PID file without cat.

This line will trip ShellCheck’s “useless cat” warning. Keep the A‑grade by using read or $(<file) instead.
As per coding guidelines: Run ShellCheck with zero violations on all scripts in .agents/scripts/.

🧹 ShellCheck‑clean PID read
-        existing_pid=$(cat "$pid_file" 2>/dev/null || true)
+        if ! read -r existing_pid < "$pid_file"; then
+            existing_pid=""
+        fi
📝 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.

Suggested change
# Check for already-running decomposition worker (throttle)
local pid_file="$SUPERVISOR_DIR/pids/${task_id}-decompose.pid"
if [[ -f "$pid_file" ]]; then
local existing_pid
existing_pid=$(cat "$pid_file" 2>/dev/null || true)
if [[ -n "$existing_pid" ]] && kill -0 "$existing_pid" 2>/dev/null; then
# Check for already-running decomposition worker (throttle)
local pid_file="$SUPERVISOR_DIR/pids/${task_id}-decompose.pid"
if [[ -f "$pid_file" ]]; then
local existing_pid
if ! read -r existing_pid < "$pid_file"; then
existing_pid=""
fi
if [[ -n "$existing_pid" ]] && kill -0 "$existing_pid" 2>/dev/null; then
🤖 Prompt for AI Agents
In @.agents/scripts/supervisor-helper.sh around lines 12546 - 12551, The code
reads the PID file using cat which triggers ShellCheck SC2002; instead, read the
PID into existing_pid without using cat by using shell builtins (e.g., read or
filename expansion) where the variable pid_file and the existing_pid assignment
occur; update the block that creates pid_file and checks kill -0 "$existing_pid"
to populate existing_pid via read or $(<"$pid_file") so ShellCheck no longer
flags a useless cat.

Comment on lines +12713 to +12716
# Update task metadata with worker PID
local escaped_id
escaped_id=$(sql_escape "$task_id")
db "$SUPERVISOR_DB" "UPDATE tasks SET metadata = CASE WHEN metadata IS NULL OR metadata = '' THEN 'decomposition_worker_pid=$worker_pid' ELSE metadata || ',decomposition_worker_pid=$worker_pid' END WHERE id = '$escaped_id';" 2>/dev/null || true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Auto‑pickup will fail: cmd_add doesn’t accept --metadata (and tasks.metadata isn’t in schema).

cmd_add’s option parser rejects unknown flags, so the #plan auto‑pickup path exits early and never dispatches the decomposition worker. Also, UPDATE tasks SET metadata=... will fail silently if the column doesn’t exist.
Fix by either:

  1. Adding --metadata support + a metadata column/migration, or
  2. Removing the flag and persisting plan_anchor via an existing field or a dedicated update after cmd_add.

Also applies to: 12895-12903

🤖 Prompt for AI Agents
In @.agents/scripts/supervisor-helper.sh around lines 12713 - 12716, The script
is passing a --metadata flag to cmd_add (and updating tasks.metadata) which
fails because cmd_add rejects unknown flags and the metadata column may not
exist; remove the --metadata usage on cmd_add and instead persist the
plan_anchor/decomposition_worker_pid via a separate DB update after cmd_add
returns (use the existing db function and sql_escape with the task id and worker
PID), or if you prefer to keep metadata semantics add a DB migration to create
the metadata column and add --metadata handling in cmd_add; update the code
paths referencing decomposition_worker_pid (the db "UPDATE tasks SET metadata =
..." block and any cmd_add invocations) to follow the chosen approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant