Implementation Plan: Auto-Merge Direct Merge Fallback (Issue #401)#446
Conversation
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested (22 findings: 4 critical, 17 warning)
| Re-submits the PR after conflict resolution. Routes back to wait_for_queue. | ||
| If re-entry fails, release_issue_failure preserves the clone for debugging. | ||
|
|
||
| direct_merge: |
There was a problem hiding this comment.
[warning] defense: direct_merge step has no retries; a transient network error on 'gh pr merge' immediately routes to confirm_cleanup with no retry attempt. Add retries: 1 with on_exhausted: confirm_cleanup.
There was a problem hiding this comment.
Investigated — this is intentional. The direct_merge step intentionally has no retries to prevent double-merge (gh pr merge --squash --auto on an already-merging PR). Transient failure degrades gracefully to confirm_cleanup per design. See step note: 'On failure (PR already merged, not mergeable), degrades gracefully to confirm_cleanup.'
| direct_merge: | ||
| tool: run_cmd | ||
| with: | ||
| cmd: "gh pr merge '${{ context.pr_number }}' --squash --auto" |
There was a problem hiding this comment.
[warning] arch: re_push_direct_fix (push_to_remote) is missing an on_exhausted handler; retry exhaustion has no defined route, leaving the pipeline in an undefined state.
There was a problem hiding this comment.
Investigated — this is intentional. on_exhausted is only applicable when retries: is defined. re_push_direct_fix has no retries: field, so on_exhausted would be semantically undefined. Single-attempt push failure is handled by on_failure: release_issue_failure. Adding on_exhausted without retries would be schema-invalid.
| remote_url: "${{ context.remote_url }}" | ||
| branch: "${{ context.merge_target }}" | ||
| on_success: redirect_merge | ||
| on_failure: release_issue_failure |
There was a problem hiding this comment.
[warning] arch: re_push_direct_fix (push_to_remote) is missing an on_exhausted handler; retry exhaustion has no defined route.
There was a problem hiding this comment.
Investigated — this is intentional. re_push_direct_fix in remediation.yaml has no retries: field. on_exhausted is only applicable with a retry count defined. on_failure: release_issue_failure handles single-attempt failure.
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review found 22 blocking issues. See inline comments. (Note: REQUEST_CHANGES submitted as COMMENT because reviewer is the PR author — verdict is still changes_requested.)
) When auto_merge=true and no merge queue is available, route_queue_mode now falls through to a direct_merge step chain instead of incorrectly routing to release_issue_success. Added five steps (direct_merge, wait_for_direct_merge, direct_merge_conflict_fix, re_push_direct_fix, redirect_merge) to all three recipes: implementation.yaml, remediation.yaml, implementation-groups.yaml. Also updated auto_merge ingredient description to reflect the dual behavior. Added 13 new parametric tests covering all three recipes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… validity test Addresses reviewer comments 2968534882/884/885 (critical): bare next() raises StopIteration instead of AssertionError when condition is missing, giving confusing test failures. Add sentinel default and explicit assertion for clear diagnostics. Addresses reviewer comment 2968534886 (warning): add test_impl_groups_recipe_still_valid to match the validation pattern established for impl, remed, and merge-prs recipes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eout in remediation.yaml Addresses critical reviewer comment 2968534873: the timeout default route in wait_for_direct_merge was confirm_cleanup, which silently ends without releasing the issue claim or recording a timeout event. impl-groups and implementation.yaml both correctly route to release_issue_timeout. release_issue_timeout already exists in remediation.yaml (line 808). Align for consistent claim lifecycle handling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ect_merge loop Addresses reviewer comments across all three recipes: Dead captures (slop): direct_merge_state and conflict_escalation_required captures are never referenced downstream — on_result conditions reference result.* directly. Removed from implementation-groups.yaml, implementation.yaml, and remediation.yaml. && chaining (defense): STATE=$(gh pr view ...) && if [...] silently swallows transient gh CLI failures — a MERGED PR could go undetected until timeout. Replaced && with ; so the if-block always evaluates $STATE after assignment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… implementation-groups.yaml Addresses reviewer comment 2968534852: implementation-groups.yaml used '== true' (single-quoted string) while implementation.yaml and remediation.yaml both use == true (unquoted). The queue_available capture is always the string "true" or "false" from jq output, and the evaluator treats unquoted true consistently. Normalizing removes the inconsistency risk. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The wait_for_direct_merge timeout was routed to release_issue_timeout (added in previous commit) but the step definition was missing from remediation.yaml. Implementation-groups.yaml and implementation.yaml both define this step. Add the equivalent step to remediation.yaml to complete the fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…y implicit-handoff rule The implicit-handoff semantic rule requires capture blocks on skill steps that declare outputs. direct_merge_conflict_fix calls resolve-merge-conflicts which declares 5 outputs, so a capture: block is required. The capture variable conflict_escalation_required is referenced by on_result conditions via result.* directly, but the capture block is still needed to satisfy the semantic contract. Restores the blocks removed in the previous dead-capture cleanup commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8840b17 to
e9c839d
Compare
Summary
When
auto_merge == "true"andqueue_available == "false", the pipeline previously droppedthe PR unmerged by routing the
route_queue_modedefault case torelease_issue_success.This is incorrect — GitHub's
gh pr merge --squash --autoworks without a merge queue bymerging directly once required checks pass. The fix adds a
direct_mergestep chain asthe default route, mirroring the queue path's structure (enable → poll → conflict-fix →
re-push → re-enable). The same change is applied in three recipes:
implementation.yaml,remediation.yaml, andimplementation-groups.yaml.Requirements
ROUTE — Recipe Routing Updates
auto_mergeis"true"andqueue_availableis"false", bothimplementation.yamlandremediation.yamlmust route to a direct-merge step instead of skipping to cleanup/release.gh pr merge --squash --autoto enable GitHub-native auto-merge regardless of merge queue presence.mergedcompletion rather than callingwait_for_merge_queue.FAIL — Failure Handling
DESC — Ingredient Description
auto_mergeingredient description in bothimplementation.yamlandremediation.yamlmust be updated to reflect that it controls automatic merging after checks pass, not specifically merge queue enrollment.Architecture Impact
Process Flow Diagram
%%{init: {'flowchart': {'nodeSpacing': 45, 'rankSpacing': 55, 'curve': 'basis'}}}%% flowchart TB 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 detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; START([CI checks pass]) subgraph Detection ["Queue Detection"] CMQ["check_merge_queue<br/>━━━━━━━━━━<br/>GraphQL: mergeQueue id?<br/>captures queue_available"] RQM{"● route_queue_mode<br/>━━━━━━━━━━<br/>auto_merge? queue?"} end subgraph QueuePath ["Queue Path"] EAM["enable_auto_merge<br/>━━━━━━━━━━<br/>gh pr merge --squash --auto<br/>→ enroll in queue"] WFQ["wait_for_queue<br/>━━━━━━━━━━<br/>wait_for_merge_queue<br/>900 s timeout"] REENROLL["reenroll_stalled_pr<br/>━━━━━━━━━━<br/>toggle_auto_merge"] QEF["queue_ejected_fix<br/>━━━━━━━━━━<br/>resolve-merge-conflicts"] RPQF["re_push_queue_fix<br/>━━━━━━━━━━<br/>push_to_remote"] RMQ["reenter_merge_queue<br/>━━━━━━━━━━<br/>gh pr merge --squash --auto"] end subgraph DirectPath ["★ Direct Merge Path (new)"] DM["★ direct_merge<br/>━━━━━━━━━━<br/>gh pr merge --squash --auto<br/>direct (no queue)"] WFDM["★ wait_for_direct_merge<br/>━━━━━━━━━━<br/>poll gh pr view<br/>10 s × 90 = 15 min"] DMCF["★ direct_merge_conflict_fix<br/>━━━━━━━━━━<br/>resolve-merge-conflicts"] RPDF["★ re_push_direct_fix<br/>━━━━━━━━━━<br/>push_to_remote"] RDM["★ redirect_merge<br/>━━━━━━━━━━<br/>gh pr merge --squash --auto<br/>re-enable after fix"] end SUCCESS["release_issue_success<br/>━━━━━━━━━━<br/>label: staged"] TIMEOUT["release_issue_timeout<br/>━━━━━━━━━━<br/>no staged label"] FAILURE["release_issue_failure<br/>━━━━━━━━━━<br/>→ cleanup_failure"] CONFIRM["confirm_cleanup<br/>━━━━━━━━━━<br/>delete clone? yes/no"] DONE([done]) START --> CMQ CMQ --> RQM RQM -->|"auto_merge != 'true'"| CONFIRM RQM -->|"queue_available == true"| EAM RQM -->|"● default (was release_issue_success)"| DM EAM --> WFQ EAM -->|"on_failure"| CONFIRM WFQ -->|"merged"| SUCCESS WFQ -->|"ejected"| QEF WFQ -->|"stalled"| REENROLL WFQ -->|"timeout"| TIMEOUT REENROLL --> WFQ QEF -->|"resolved"| RPQF QEF -->|"escalation_required"| FAILURE RPQF --> RMQ RMQ --> WFQ DM --> WFDM DM -->|"on_failure"| CONFIRM WFDM -->|"merged"| SUCCESS WFDM -->|"closed"| DMCF WFDM -->|"timeout"| TIMEOUT DMCF -->|"resolved"| RPDF DMCF -->|"escalation_required"| FAILURE RPDF --> RDM RDM --> WFDM SUCCESS --> CONFIRM TIMEOUT --> CONFIRM CONFIRM -->|"yes"| DONE CONFIRM -->|"no"| DONE class START,DONE terminal; class CMQ phase; class RQM stateNode; class EAM,WFQ,REENROLL,QEF,RPQF,RMQ handler; class DM,WFDM,DMCF,RPDF,RDM newComponent; class CONFIRM detector; class SUCCESS,TIMEOUT,FAILURE output;Color Legend:
● route_queue_mode— modified routing decision★New direct-merge path steps added by this PRconfirm_cleanupgateState Lifecycle Diagram
%%{init: {'flowchart': {'nodeSpacing': 48, 'rankSpacing': 56, 'curve': 'basis'}}}%% flowchart TB 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 gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000; classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; subgraph Inputs ["INIT_ONLY — Ingredient Fields (set at recipe start)"] direction LR OPR["inputs.open_pr<br/>━━━━━━━━━━<br/>default: true<br/>SKIP_GATE for all PR steps"] AM["● inputs.auto_merge<br/>━━━━━━━━━━<br/>default: true<br/>● desc: direct merge fallback"] BB["inputs.base_branch<br/>━━━━━━━━━━<br/>merge target"] IU["inputs.issue_url<br/>━━━━━━━━━━<br/>optional: release gate"] end subgraph Prerequisites ["SET_ONCE — Pipeline Context (captured before merge path)"] direction LR QA["context.queue_available<br/>━━━━━━━━━━<br/>SET: check_merge_queue<br/>READ: route_queue_mode"] PRN["context.pr_number<br/>━━━━━━━━━━<br/>SET: extract_pr_number<br/>READ: direct_merge, wait_for_direct_merge"] WD["context.work_dir<br/>━━━━━━━━━━<br/>SET: clone<br/>READ: all direct-merge steps"] MT["context.merge_target<br/>━━━━━━━━━━<br/>SET: create_branch<br/>READ: re_push_direct_fix"] end subgraph Gate ["VALIDATION GATE — skip_when_false"] GATE["inputs.open_pr == true<br/>━━━━━━━━━━<br/>Guards all 5 new steps:<br/>direct_merge, wait_for_direct_merge,<br/>direct_merge_conflict_fix,<br/>re_push_direct_fix, redirect_merge"] end subgraph RoutingDecision ["● route_queue_mode — State-Based Router"] RQM["● route_queue_mode<br/>━━━━━━━━━━<br/>reads: inputs.auto_merge<br/>reads: context.queue_available<br/>● default → direct_merge (was: release_issue_success)"] end subgraph NewCaptures ["★ CAPTURE_RESULT — New Context Fields (direct merge path)"] direction TB DMS["★ direct_merge_state<br/>━━━━━━━━━━<br/>SET: wait_for_direct_merge<br/>values: merged | closed | timeout<br/>READ: on_result conditions"] CER["conflict_escalation_required<br/>━━━━━━━━━━<br/>SET: direct_merge_conflict_fix<br/>values: true | false<br/>READ: on_result conditions"] end subgraph RouteContracts ["ON_RESULT CONTRACTS — Typed routing guards"] direction TB WFDMc["wait_for_direct_merge<br/>━━━━━━━━━━<br/>merged → release_issue_success<br/>closed → direct_merge_conflict_fix<br/>default → release_issue_timeout"] DMCFc["direct_merge_conflict_fix<br/>━━━━━━━━━━<br/>escalation_required == true → release_issue_failure<br/>default → re_push_direct_fix"] end OPR -->|"evaluated each step"| GATE AM -->|"read by"| RQM QA -->|"read by"| RQM BB -->|"passed to"| DMCFc IU -->|"gates"| RouteContracts PRN -->|"passed to"| Gate WD -->|"passed to"| Gate MT -->|"passed to"| Gate GATE -->|"pass → execute"| RQM RQM -->|"default route changed"| NewCaptures DMS -->|"consumed by"| WFDMc CER -->|"consumed by"| DMCFc class OPR,AM,BB,IU detector; class QA,PRN,WD,MT stateNode; class GATE gap; class RQM phase; class DMS newComponent; class CER handler; class WFDMc,DMCFc output;Color Legend:
inputs.open_prvalidation gate protecting all new steps● route_queue_mode— routing logic changed by this PR★ direct_merge_state— new field introduced by this PRconflict_escalation_required— existing field, also written by new stepon_resulttyped routing guards (consume captured fields)Closes #401
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260320-165150-373150/temp/make-plan/direct_merge_fallback_plan_2026-03-20_000001.mdToken Usage Summary
Token Summary\n\n(Token data accumulated server-side)
🤖 Generated with Claude Code via AutoSkillit