Fix Orchestrator Anti-Patterns Identified by run_cmd Pattern Mining

[Trecek]

Summary

Pattern mining of 736 run_cmd invocations across 47 orchestrator sessions identified five
behavioral anti-patterns where the orchestrator bypasses the recipe routing discipline. This
plan fixes all five:

1. AP1 — CI Investigation Spiral (22 occurrences): Create a diagnose-ci skill so the
   orchestrator has a proper run_skill channel for CI log investigation. Update recipes to
   insert a diagnose_ci step before resolve_ci on CI failure paths.

2. AP2 — Push-Per-Merge (8 wasted pushes): Switch push_integration_branch in
   pr-merge-pipeline.yaml from run_cmd to the push_to_remote MCP tool (the same tool
   all other push steps use), and add an explicit kitchen rule forbidding intermediate pushes
   inside the merge loop.

3. AP3 — Branch Checkout Retries (12 redundant attempts): Add base_branch_name
   parameter to the create_unique_branch MCP tool. Replace the create_branch run_cmd
   bash script in all recipes with two cleaner steps: compute_branch (name-only run_cmd)

   • create_branch (create_unique_branch MCP tool). The atomic MCP call eliminates the
     gap where the orchestrator inserts extra git fetch + checkout -b commands.

4. AP4 — Multi-Issue Boilerplate (80-command sessions): Add a batch-implementation.yaml
   recipe skeleton that runs shared setup (clone, base_sha) once and loops per-issue for the
   branch+implement+push steps. Full loop implementation is deferred pending write_telemetry_files
   from issue feat: new MCP tools for high-frequency run_cmd patterns (pattern mining 2026-03-08) #237; this plan delivers the recipe structure and the shared-setup savings.

5. AP5 — Unnamed Step Commands (88.3%): Add step_name to every run_cmd step in all
   bundled recipes that currently lacks one. Target: ≤20% unnamed commands.

Architecture Impact

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 45, 'rankSpacing': 55, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    %% TERMINALS %%
    START([START])
    DONE([DONE])
    ESCALATE([ESCALATE])

    subgraph AP3 ["AP3 Fix: Branch Creation (implementation, audit-and-fix, remediation)"]
        direction TB
        COMPUTE["● compute_branch<br/>━━━━━━━━━━<br/>run_cmd: name only<br/>slug/issue → proposed_branch<br/>no git operations"]
        CUB["★ create_branch<br/>━━━━━━━━━━<br/>create_unique_branch tool<br/>base_branch_name=proposed_branch<br/>atomic: ls-remote + checkout -b"]
        PUSH_MT["push_merge_target<br/>━━━━━━━━━━<br/>push_to_remote MCP"]
    end

    subgraph IMPL ["Core Loop (implement → test → merge)"]
        direction LR
        PLAN["plan / implement<br/>━━━━━━━━━━<br/>run_skill sessions"]
        TEST{"test passes?"}
        FIX["fix<br/>━━━━━━━━━━<br/>resolve-failures"]
        MERGE["merge<br/>━━━━━━━━━━<br/>merge_worktree MCP"]
    end

    subgraph AP1 ["AP1 Fix: CI Failure Path (implementation, audit-and-fix, remediation)"]
        direction TB
        PUSH_PR["push / open_pr / review_pr<br/>━━━━━━━━━━<br/>upstream push + GitHub PR"]
        CI_WATCH{"● ci_watch<br/>━━━━━━━━━━<br/>wait_for_ci"}
        DIAG_CI["★ diagnose_ci<br/>━━━━━━━━━━<br/>run_skill: /autoskillit:diagnose-ci<br/>gh logs → failure_type, is_fixable<br/>writes temp/diagnose-ci/diagnosis_*.md"]
        RES_CI["● resolve_ci<br/>━━━━━━━━━━<br/>resolve-failures + diagnosis_path<br/>retries: 2"]
        RE_PUSH["re_push<br/>━━━━━━━━━━<br/>push_to_remote MCP"]
    end

    subgraph AP2 ["AP2 Fix: PR Merge Pipeline Push (pr-merge-pipeline)"]
        direction TB
        MERGE_LOOP["merge_pr loop<br/>━━━━━━━━━━<br/>sequential PR merges<br/>no intermediate pushes"]
        PUSH_INT["● push_integration_branch<br/>━━━━━━━━━━<br/>push_to_remote MCP tool<br/>(was: run_cmd git push)"]
        CI_WATCH_PR{"● ci_watch_pr<br/>━━━━━━━━━━<br/>wait_for_ci: integration branch"}
        DIAG_CI_PR["★ diagnose_ci_pr<br/>━━━━━━━━━━<br/>run_skill: /autoskillit:diagnose-ci<br/>observability only — no auto-retry"]
    end

    %% FLOW: Branch Creation %%
    START --> COMPUTE
    COMPUTE -->|"proposed_branch captured"| CUB
    CUB -->|"merge_target captured"| PUSH_MT
    PUSH_MT --> PLAN

    %% FLOW: Core Loop %%
    PLAN --> TEST
    TEST -->|"pass"| MERGE
    TEST -->|"fail"| FIX
    FIX --> TEST
    MERGE -->|"success"| PUSH_PR

    %% FLOW: CI failure path %%
    PUSH_PR --> CI_WATCH
    CI_WATCH -->|"success"| DONE
    CI_WATCH -->|"failure"| DIAG_CI
    DIAG_CI -->|"diagnosis_path → resolve_ci<br/>(success OR failure)"| RES_CI
    RES_CI -->|"success"| RE_PUSH
    RES_CI -->|"exhausted / failure"| ESCALATE
    RE_PUSH -->|"loop back"| CI_WATCH

    %% FLOW: PR merge pipeline (parallel flow) %%
    MERGE_LOOP -->|"all PRs merged"| PUSH_INT
    PUSH_INT --> CI_WATCH_PR
    CI_WATCH_PR -->|"success"| DONE
    CI_WATCH_PR -->|"failure"| DIAG_CI_PR
    DIAG_CI_PR -->|"always → cleanup_failure"| ESCALATE

    %% CLASS ASSIGNMENTS %%
    class START,DONE,ESCALATE terminal;
    class COMPUTE,MERGE,PUSH_MT,RE_PUSH,PUSH_PR,PLAN,MERGE_LOOP handler;
    class CUB,DIAG_CI,DIAG_CI_PR,PUSH_INT newComponent;
    class RES_CI phase;
    class CI_WATCH,CI_WATCH_PR stateNode;
    class TEST detector;

Loading

Error/Resilience Diagram

%%{init: {'flowchart': {'nodeSpacing': 45, 'rankSpacing': 55, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    T_SUCCESS([CI PASSED])
    T_ESCALATE([ESCALATED])
    T_CLEANUP([CLEANUP FAILURE])

    subgraph AP3_RES ["AP3: Branch Collision Guard (create_unique_branch)"]
        direction LR
        LS_REMOTE["● ls-remote check<br/>━━━━━━━━━━<br/>refs/heads/{proposed_branch}"]
        SUFFIX{"collision?"}
        CHECKOUT["git checkout -b<br/>━━━━━━━━━━<br/>atomic branch creation"]
        SUFFIX_N["append suffix<br/>━━━━━━━━━━<br/>-2, -3, ... -100"]
    end

    subgraph AP1_IMPL ["AP1: CI Failure Recovery (implementation / audit-and-fix / remediation)"]
        direction TB
        CI_WATCH{"● ci_watch<br/>━━━━━━━━━━<br/>wait_for_ci<br/>success | failure"}
        DIAG_CI["★ diagnose_ci<br/>━━━━━━━━━━<br/>/autoskillit:diagnose-ci<br/>classifies: test|lint|build<br/>writes diagnosis_*.md<br/>captures: diagnosis_path"]
        RES_CI["● resolve_ci<br/>━━━━━━━━━━<br/>resolve-failures + diagnosis_path<br/>retries: 2"]
        RE_PUSH["re_push<br/>━━━━━━━━━━<br/>push_to_remote MCP<br/>re-triggers CI watch loop"]
    end

    subgraph AP1_PR ["AP1: Integration CI Observability (pr-merge-pipeline)"]
        direction TB
        CI_WATCH_PR{"● ci_watch_pr<br/>━━━━━━━━━━<br/>wait_for_ci: integration branch"}
        DIAG_CI_PR["★ diagnose_ci_pr<br/>━━━━━━━━━━<br/>/autoskillit:diagnose-ci<br/>observability only<br/>NO automated remediation"]
    end

    subgraph TEST_LOOP ["Test-Fix Loop (all recipes)"]
        direction LR
        TEST_GATE{"test passes?<br/>━━━━━━━━━━<br/>task test-check"}
        FIX["fix<br/>━━━━━━━━━━<br/>resolve-failures<br/>no hard retry limit"]
    end

    LS_REMOTE --> SUFFIX
    SUFFIX -->|"no collision"| CHECKOUT
    SUFFIX -->|"collision"| SUFFIX_N
    SUFFIX_N -->|"retry ls-remote"| SUFFIX
    CHECKOUT --> TEST_GATE

    TEST_GATE -->|"pass"| CI_WATCH
    TEST_GATE -->|"fail"| FIX
    FIX -->|"retry"| TEST_GATE

    CI_WATCH -->|"success"| T_SUCCESS
    CI_WATCH -->|"failure"| DIAG_CI
    DIAG_CI -->|"success OR failure"| RES_CI
    RES_CI -->|"resolved"| RE_PUSH
    RES_CI -->|"exhausted (retry 2)"| T_ESCALATE
    RE_PUSH -->|"re-triggers CI loop"| CI_WATCH

    CI_WATCH_PR -->|"success"| T_SUCCESS
    CI_WATCH_PR -->|"failure"| DIAG_CI_PR
    DIAG_CI_PR -->|"always (no auto-retry)"| T_CLEANUP

    class T_SUCCESS,T_ESCALATE,T_CLEANUP terminal;
    class LS_REMOTE,RE_PUSH,FIX handler;
    class CHECKOUT output;
    class SUFFIX_N phase;
    class DIAG_CI,DIAG_CI_PR newComponent;
    class RES_CI phase;
    class SUFFIX,CI_WATCH,CI_WATCH_PR,TEST_GATE detector;

Loading

Closes #238

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-20260309-163144-711104/temp/make-plan/orchestrator_anti_patterns_fix_plan_2026-03-09_120000.md

Token Usage Summary

Token Usage Summary

Step	input	output	cache_create	cache_read
plan	5,385	206,073	565,097	7,786,890
verify	229	117,284	463,772	7,676,097
implement	12,168	439,202	1,182,612	73,253,502
fix	320	158,164	467,166	23,403,907
audit_impl	528	99,514	389,958	2,613,670
open_pr	104	52,902	189,905	4,079,028
review_pr	10,284	216,697	565,519	7,468,841
resolve_review	387	136,019	448,381	20,173,933
Total	60,783	1,738,007	5,316,054	161,843,046

Step Timing Summary

Step Timing Summary

Step	Duration	Invocations
clone	1s	1
capture_base_sha	0s	1
set_merge_target	0s	1
create_branch	1s	1
push_merge_target	1s	1
plan	5m 17s	1
verify	4m 37s	1
implement	8m 54s	1
fix	5m 47s	1
test	1m 34s	2
merge	1m 30s	1
audit_impl	3m 2s	1
push	1s	1
Total	31m 5s

🤖 Generated with Claude Code via AutoSkillit

[Trecek]

AutoSkillit Review Findings

Verdict: changes_requested

9 actionable findings (clear fixes available) + 1 decision finding (requires human judgment).

src/autoskillit/recipes/batch-implementation.yaml

Line	Severity	Dimension	Message
383	warning	bugs	create_branch runs inside the batch loop against the same shared clone. After the first issue's feature branch is created, subsequent iterations branch from the current HEAD (the prior feature branch) rather than base_branch. There is no git checkout base_branch between iterations, so branches stack on each other.
402	warning	bugs	The 'implement' step invokes /autoskillit:make-plan (planning only), but on_success routes to open_pr_step which opens a PR — implying working code is present. A TODO comment acknowledges this, but opening a PR after make-plan with no actual implementation step will produce empty PRs. Should this step use implement-worktree-no-merge instead? ⚠️ requires_decision
454	warning	bugs	next_or_done_issue uses ${{ result.next }} == more_issues but action: route calls no tool and no prior step produces a next field. The routing condition is permanently unsatisfiable — the pipeline always routes to confirm_cleanup and never loops back to get_issue_title.

src/autoskillit/recipes/pr-merge-pipeline.yaml

Line	Severity	Dimension	Message
880	warning	cohesion	Step is named 'diagnose_ci_pr' here but 'diagnose_ci' in all other recipes (implementation.yaml, implementation-groups.yaml, audit-and-fix.yaml, remediation.yaml). The '_pr' suffix breaks cross-recipe naming symmetry without adding structural distinction.
888	warning	cohesion	diagnose_ci_pr uses 'on_result' with a single unconditional route to 'cleanup_failure', while all sibling diagnose_ci steps use 'on_success'/'on_failure' routing. The asymmetric routing pattern diverges from the established convention for this step type across every other recipe.

src/autoskillit/server/tools_git.py

Line	Severity	Dimension	Message
1099	warning	defense	create_unique_branch has no validation guard: if base_branch_name is None and slug is also empty (both are defaults), base_name becomes '' or '-{issue_number}' — both invalid git branch names. Add a defensive check that raises an error when neither base_branch_name nor slug is provided.
1103	warning	defense	base_branch_name is typed str

tests/recipe/test_anti_pattern_guards.py

Line	Severity	Dimension	Message
1381	warning	tests	test_implementation_groups_ci_failure_routes_to_diagnose_ci asserts only ci_watch.on_failure == 'diagnose_ci' and step existence, but its sister test for implementation.yaml also asserts diag.tool == 'run_skill' and 'diagnosis_path' in diag.capture. The weaker assertions here mean a misconfigured diagnose_ci step (wrong tool or missing capture) would pass undetected for implementation-groups.
1409	warning	tests	test_pr_merge_pipeline_has_no_loop_push_kitchen_rule uses fragile prose substring matching ('NEVER push inside the' or 'no intermediate push') on a joined rule string. Any rewording (e.g. 'NEVER push within the') silently breaks the guard. Prefer asserting a stable structural property instead.

tests/server/test_tools_git.py

Line	Severity	Dimension	Message
1649	warning	tests	The generator expression passed to next() has no default, so if no subprocess call contains 'ls-remote' in its args, next() raises a bare StopIteration with no diagnostic message. Add next(..., None) and assert the result for a readable failure.

[Trecek]

Collapsed into integration PR #288 (#288)