Implementation Plan: Queue Ejection Loop Fix

[Trecek]

Summary

Fixes a 3-iteration ejection loop in the merge queue pipeline by introducing ejection-cause enrichment (ejected_ci_failure state and ejection_cause field in wait_for_merge_queue), a CI gate after every force-push (ci_watch_post_queue_fix step), and two post-rebase manifest validation gates (language-aware validity check and duplicate key scan) in resolve-merge-conflicts. Closes all six gaps identified in #627: blind CI ejection routing, missing CI gate after re-push, absent manifest/semantic validation, and missing head_sha in CI results.

Individual Group Plans

Group 1: Implementation Plan: Queue Ejection Loop Fix — PART A ONLY

This part addresses the Python code layer for the queue ejection loop fix (Gaps 2 and 5 from issue #627).

Gap 2 — execution/merge_queue.py currently returns pr_state="ejected" for every ejection regardless of cause. When GitHub's CI fails on a merge-group commit, the recipe cannot distinguish a CI failure ejection from a conflict ejection, so it retries conflict resolution indefinitely (no-op rebase loop). The fix: when the ejection is confirmed and checks_state == "FAILURE", return pr_state="ejected_ci_failure" plus an ejection_cause="ci_failure" field, allowing recipe on_result routing to send CI failures directly to diagnose_ci instead of queue_ejected_fix.

Gap 5 — server/tools_ci.py infers head_sha from git rev-parse HEAD but never includes it in the JSON response. Recipe orchestrators cannot verify that CI results correspond to the current HEAD after a force-push. The fix: include head_sha in the wait_for_ci return dict when it was resolved.

Group 2: Implementation Plan: Queue Ejection Loop Fix — PART B ONLY

This part addresses the recipe and skill layer of the queue ejection loop fix (Gaps 1, 3, 4, 6 from issue #627). Part A (code layer) must be implemented first — this part routes on pr_state="ejected_ci_failure" which Part A introduces.

Gap 1 — re_push_queue_fix routes directly to reenter_merge_queue after force-push, bypassing CI. Fix: insert a new ci_watch_post_queue_fix step between re_push_queue_fix and reenter_merge_queue, mirroring the existing ci_watch step.

Gap 6 — wait_for_queue routes all ejected states to queue_ejected_fix (conflict resolution), even when the ejection was caused by a CI failure that conflict resolution cannot fix. Fix: add an ejected_ci_failure route before ejected in wait_for_queue.on_result, routing to diagnose_ci instead.

Gap 3 — resolve-merge-conflicts SKILL.md runs only pre-commit run --all-files post-rebase. Fix: add Step 5a — language-detected manifest validation using fast non-compiling checks.

Gap 4 — Even a clean rebase can produce duplicate keys when both branches independently added the same dependency. Fix: add Step 5b — targeted duplicate key scan in TOML/JSON manifest files.

Applied to: recipes/implementation.yaml, recipes/remediation.yaml, recipes/implementation-groups.yaml, skills_extended/resolve-merge-conflicts/SKILL.md.

Architecture Impact

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, '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 detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;

    %% TERMINALS %%
    START([wait_for_queue\nrecipe step])
    END_OK([release_issue_success])
    END_FAIL([release_issue_failure])
    END_TIMEOUT([release_issue_timeout])
    END_DIAG([diagnose_ci])

    subgraph MQPoll ["● Merge Queue Watcher (merge_queue.py)"]
        direction TB
        POLL["poll GitHub GraphQL\n━━━━━━━━━━\nPR state + queue state\n+ checks_state"]
        MERGED{"merged?"}
        CI_FAIL{"● checks_state\n== 'FAILURE'?"}
        CONFIRM["confirmation window\n━━━━━━━━━━\nnot_in_queue_cycles++"]
        CONFIRMED{"cycles ≥ threshold?"}
        STALL{"stall retries\nexhausted?"}
        TIMEOUT{"deadline\nexceeded?"}
    end

    subgraph EjectRoute ["● Recipe Ejection Routing (implementation.yaml)"]
        direction TB
        ROUTE{"● pr_state?"}
        REENROLL["reenroll_stalled_pr\n━━━━━━━━━━\ntoggle_auto_merge tool"]
    end

    subgraph ConflictFix ["● Conflict Fix Sub-Flow (implementation.yaml)"]
        direction TB
        QFIX["queue_ejected_fix\n━━━━━━━━━━\nresolve-merge-conflicts skill"]
        ESC{"escalation_required?"}
        REPUSH["re_push_queue_fix\n━━━━━━━━━━\npush_to_remote force=true"]
        CI_WATCH["● ci_watch_post_queue_fix\n━━━━━━━━━━\nwait_for_ci tool\ntimeout=300s"]
        CI_PASS{"CI pass?"}
        DETECT["detect_ci_conflict\n━━━━━━━━━━\ndiagnose-ci skill"]
        REENTER["reenter_merge_queue\n━━━━━━━━━━\ngh pr merge --squash --auto"]
    end

    subgraph WFCITool ["● wait_for_ci tool handler (tools_ci.py)"]
        direction LR
        INFER["infer head_sha\n━━━━━━━━━━\ngit rev-parse HEAD"]
        CIWAIT["ci_watcher.wait(scope)"]
        ENRICH["● result includes head_sha\n━━━━━━━━━━\nverifies SHA matches HEAD\nafter force-push"]
    end

    %% MAIN FLOW %%
    START --> POLL
    POLL --> MERGED
    MERGED -->|"yes"| END_OK
    MERGED -->|"no"| CONFIRM
    CONFIRM --> CONFIRMED
    CONFIRMED -->|"no"| STALL
    CONFIRMED -->|"yes (not in queue)"| CI_FAIL
    STALL -->|"yes"| END_TIMEOUT
    STALL -->|"no"| TIMEOUT
    TIMEOUT -->|"yes"| END_TIMEOUT
    TIMEOUT -->|"no"| POLL

    CI_FAIL -->|"yes"| ROUTE
    CI_FAIL -->|"no"| ROUTE

    ROUTE -->|"ejected_ci_failure\n(● new route)"| END_DIAG
    ROUTE -->|"ejected"| QFIX
    ROUTE -->|"stalled"| REENROLL
    ROUTE -->|"timeout"| END_TIMEOUT
    REENROLL -->|"success"| START
    REENROLL -->|"failure"| END_FAIL

    QFIX --> ESC
    ESC -->|"true"| END_FAIL
    ESC -->|"false"| REPUSH
    REPUSH -->|"failure"| END_FAIL
    REPUSH -->|"success"| CI_WATCH

    CI_WATCH --> INFER --> CIWAIT --> ENRICH
    ENRICH --> CI_PASS
    CI_PASS -->|"failure"| DETECT
    CI_PASS -->|"success"| REENTER
    DETECT --> END_FAIL
    REENTER -->|"success"| START
    REENTER -->|"failure"| END_FAIL

    %% CLASS ASSIGNMENTS %%
    class START terminal;
    class END_OK,END_FAIL,END_TIMEOUT,END_DIAG terminal;
    class POLL,CONFIRM handler;
    class MERGED,CONFIRMED,STALL,TIMEOUT stateNode;
    class CI_FAIL,ROUTE,ESC,CI_PASS detector;
    class QFIX,REPUSH,REENTER handler;
    class REENROLL,DETECT handler;
    class CI_WATCH,INFER,CIWAIT,ENRICH newComponent;

Loading

State Lifecycle Diagram

%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, '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 detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    subgraph MQResult ["wait_for_merge_queue Return Dict (merge_queue.py)"]
        direction TB
        PS["● pr_state : str\n━━━━━━━━━━\nmerged | ejected\nejected_ci_failure | stalled\ntimeout | error\n(bare literals, no StrEnum)"]
        SUC["success : bool\n━━━━━━━━━━\ntrue only for 'merged'"]
        REASON["reason : str\n━━━━━━━━━━\nhuman-readable\nalways present"]
        STALL["stall_retries_attempted : int\n━━━━━━━━━━\nalways present\nexcept 'error' path"]
        EC["● ejection_cause : str\n━━━━━━━━━━\n'ci_failure' only\nwhen pr_state==ejected_ci_failure\nCONDITIONAL FIELD"]
    end

    subgraph InternalPoll ["PRFetchState — Internal Polling State (not returned)"]
        direction LR
        CHECKS["checks_state : str|None\n━━━━━━━━━━\nGitHub StatusCheckRollup\nNone = no checks configured"]
        INQUEUE["in_queue : bool\n━━━━━━━━━━\nPR in mergeQueue.entries"]
        QSTATE["queue_state : str|None\n━━━━━━━━━━\nUNMERGEABLE | AWAITING_CHECKS\n| LOCKED | null"]
    end

    subgraph Gate1 ["● Ejection Decision Gate (merge_queue.py)"]
        direction TB
        CFAIL{"checks_state\n== 'FAILURE'?"}
        SET_ECI["● set pr_state='ejected_ci_failure'\n━━━━━━━━━━\nejection_cause='ci_failure'\nINJECTED into result"]
        SET_EJ["set pr_state='ejected'\n━━━━━━━━━━\nno ejection_cause field\n(absent, not null)"]
    end

    subgraph CIScope ["CIRunScope — Frozen Input Scope (core/types)"]
        direction LR
        WF["workflow : str|None\n━━━━━━━━━━\ne.g. 'tests.yml'"]
        HS["● head_sha : str|None\n━━━━━━━━━━\ngit rev-parse HEAD\nor caller-supplied"]
    end

    subgraph CIResult ["● wait_for_ci Return Dict (tools_ci.py)"]
        direction TB
        RUNID["run_id : int|None\n━━━━━━━━━━\nGitHub Actions run ID"]
        CONC["conclusion : str\n━━━━━━━━━━\nsuccess|failure|cancelled\naction_required|timed_out\nno_runs|error|unknown"]
        FJOBS["failed_jobs : list\n━━━━━━━━━━\nalways present\nempty on billing errors"]
        HSHA["● head_sha : str\n━━━━━━━━━━\nCONDITIONAL: present only\nwhen scope.head_sha truthy\ninjected by tool layer"]
    end

    subgraph ConsumerGate ["Recipe Routing Gate (on_result)"]
        direction TB
        ROUTE{"pr_state value?"}
        R1["ejected_ci_failure\n→ diagnose_ci"]
        R2["ejected\n→ queue_ejected_fix"]
        R3["merged|stalled|timeout\n→ other routes"]
    end

    %% FLOW %%
    CHECKS --> CFAIL
    INQUEUE --> CFAIL
    QSTATE --> CFAIL
    CFAIL -->|"FAILURE"| SET_ECI
    CFAIL -->|"other"| SET_EJ
    SET_ECI --> PS
    SET_ECI --> EC
    SET_EJ --> PS
    PS --> SUC
    PS --> REASON
    PS --> STALL

    HS --> CIResult
    WF --> CIResult
    RUNID --> CONC
    CONC --> FJOBS
    FJOBS --> HSHA

    PS --> ROUTE
    EC --> ROUTE
    ROUTE --> R1
    ROUTE --> R2
    ROUTE --> R3

    HSHA -.->|"verifies HEAD\nafter force-push"| R2

    %% CLASS ASSIGNMENTS %%
    class PS,EC,HSHA,SET_ECI,HS,CFAIL gap;
    class SUC,REASON,STALL,RUNID,CONC,FJOBS output;
    class CHECKS,INQUEUE,QSTATE,WF stateNode;
    class SET_EJ handler;
    class ROUTE,R1,R2,R3 detector;
    class InternalPoll phase;

Loading

Error/Resilience Diagram

%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, '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 detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    END_OK([release_issue_success])
    END_FAIL([release_issue_failure\n━━━━━━━━━━\nhuman escalation\nclone preserved])
    END_DIAG([diagnose_ci])

    subgraph MQLoop ["● Merge Queue Poll Loop (merge_queue.py)"]
        direction TB
        POLL["GraphQL fetch\n━━━━━━━━━━\nPR + queue state"]
        POLL_ERR{"Exception\ncaught?"}
        TIMEOUT_CHK{"deadline\nexceeded?"}
        STALL_CHK{"stall retries\n≥ max (3)?"}
    end

    subgraph EjectGate ["● Ejection Classification Gate (merge_queue.py)"]
        direction TB
        EJECT_DECISION{"● checks_state\n== 'FAILURE'?"}
        CI_EJ["● ejected_ci_failure\n━━━━━━━━━━\nejection_cause=ci_failure\nskips conflict resolution"]
        CONF_EJ["ejected\n━━━━━━━━━━\nno cause field\nconflict resolution"]
    end

    subgraph StallBreaker ["Stall Circuit Breaker (merge_queue.py)"]
        direction LR
        TOGGLE["_toggle_auto_merge\n━━━━━━━━━━\ndisable → 2s → re-enable\nbackoff: 30/60/120s"]
        TOGGLE_ERR{"Exception\ncaught?"}
    end

    subgraph ConflictPath ["Conflict Resolution Path (implementation.yaml)"]
        direction TB
        QFIX["queue_ejected_fix\n━━━━━━━━━━\nresolve-merge-conflicts skill"]
        ESC_CHK{"escalation\nrequired?"}
        REPUSH["re_push_queue_fix\n━━━━━━━━━━\nforce-push"]
        REPUSH_FAIL{"push\nfailed?"}
    end

    subgraph CIGate ["● CI Gate After Re-Push (implementation.yaml + tools_ci.py)"]
        direction TB
        CI_WATCH["● ci_watch_post_queue_fix\n━━━━━━━━━━\nwait_for_ci, timeout=300s\nincludes head_sha"]
        CI_CONC{"conclusion\n== success?"}
        DETECT["detect_ci_conflict\n━━━━━━━━━━\ngit merge-base check\n(stale base?)"]
        DETECT_CHK{"stale\nbase?"}
        CI_CF["ci_conflict_fix\n━━━━━━━━━━\nresolve-merge-conflicts"]
    end

    subgraph ManifestGates ["● Post-Rebase Manifest Validation (SKILL.md)"]
        direction TB
        STEP5A["● Step 5a: manifest validity\n━━━━━━━━━━\ncargo metadata / node JSON.parse\nuv lock --check / tomllib"]
        STEP5A_CHK{"manifest\nvalid?"}
        STEP5B["● Step 5b: duplicate key scan\n━━━━━━━━━━\nTOML dep sections\nJSON object_pairs_hook"]
        STEP5B_CHK{"duplicates\nfound?"}
        REBASE_ABORT["git rebase --abort\n━━━━━━━━━━\nescalation_required=true"]
    end

    %% POLL LOOP FLOW %%
    POLL --> POLL_ERR
    POLL_ERR -->|"yes: log + retry"| POLL
    POLL_ERR -->|"no"| TIMEOUT_CHK
    TIMEOUT_CHK -->|"yes"| END_FAIL
    TIMEOUT_CHK -->|"no"| STALL_CHK
    STALL_CHK -->|"yes: stalled"| END_FAIL
    STALL_CHK -->|"no: stall attempt"| TOGGLE
    TOGGLE --> TOGGLE_ERR
    TOGGLE_ERR -->|"yes: log + increment"| STALL_CHK
    TOGGLE_ERR -->|"no: success"| POLL

    %% EJECTION GATE %%
    STALL_CHK -->|"ejection confirmed"| EJECT_DECISION
    EJECT_DECISION -->|"FAILURE"| CI_EJ
    EJECT_DECISION -->|"other"| CONF_EJ
    CI_EJ --> END_DIAG
    CONF_EJ --> QFIX

    %% CONFLICT PATH %%
    QFIX --> STEP5A
    STEP5A --> STEP5A_CHK
    STEP5A_CHK -->|"invalid"| REBASE_ABORT
    STEP5A_CHK -->|"valid"| STEP5B
    STEP5B --> STEP5B_CHK
    STEP5B_CHK -->|"duplicates"| REBASE_ABORT
    STEP5B_CHK -->|"clean"| ESC_CHK
    REBASE_ABORT --> ESC_CHK
    ESC_CHK -->|"true"| END_FAIL
    ESC_CHK -->|"false"| REPUSH
    REPUSH --> REPUSH_FAIL
    REPUSH_FAIL -->|"yes"| END_FAIL
    REPUSH_FAIL -->|"no"| CI_WATCH

    %% CI GATE %%
    CI_WATCH --> CI_CONC
    CI_CONC -->|"yes"| END_OK
    CI_CONC -->|"no"| DETECT
    DETECT --> DETECT_CHK
    DETECT_CHK -->|"yes: stale base"| CI_CF
    DETECT_CHK -->|"no: code failure"| END_DIAG
    CI_CF --> ESC_CHK

    %% CLASS ASSIGNMENTS %%
    class END_OK,END_FAIL,END_DIAG terminal;
    class POLL,TOGGLE handler;
    class POLL_ERR,TOGGLE_ERR,TIMEOUT_CHK,STALL_CHK gap;
    class EJECT_DECISION,CI_CONC,DETECT_CHK,STEP5A_CHK,STEP5B_CHK,ESC_CHK,REPUSH_FAIL detector;
    class CI_EJ,CONF_EJ,REBASE_ABORT output;
    class QFIX,REPUSH,CI_WATCH,DETECT,CI_CF handler;
    class STEP5A,STEP5B phase;

Loading

Closes #627

Implementation Plan

Plan files:

• /home/talon/projects/autoskillit-runs/impl-20260405-122055-152199/.autoskillit/temp/make-plan/queue_ejection_loop_plan_2026-04-05_122055_part_a.md
• /home/talon/projects/autoskillit-runs/impl-20260405-122055-152199/.autoskillit/temp/make-plan/queue_ejection_loop_plan_2026-04-05_122055_part_b.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step	uncached	output	cache_read	cache_write	count	time
plan	37	31.7k	1.9M	113.2k	1	11m 19s
review	3.4k	5.6k	147.3k	41.5k	1	5m 45s
verify	44	35.4k	1.9M	144.8k	2	11m 15s
implement	100	33.5k	4.6M	123.5k	2	12m 17s
audit_impl	15	14.0k	279.5k	44.2k	1	3m 46s
open_pr	33	30.5k	1.2M	68.1k	1	10m 58s
Total	3.6k	150.8k	9.9M	535.3k		55m 23s

[Trecek]

AutoSkillit PR Review — Verdict: changes_requested

8 actionable findings (warning severity) require fixes before merge.

[Trecek]

[info] slop: Two-line comment restates what the code does. The second sentence duplicates the first. A single concise comment is sufficient.

[Trecek]

[info] defense: scope.head_sha type contract is not visible here. If the attribute is Optional[str], the truthiness guard is correct. If it can be an empty string sentinel, document whether absence means 'not applicable' vs 'could not determine'.

[Trecek]

[info] tests: No test covers checks_state='ERROR' at ejection. The production code only checks == 'FAILURE'; any other non-None value falls through to plain 'ejected'. A test for ERROR would prevent regressions if the condition is accidentally widened.

[Trecek]

[warning] tests: The ejected_ci_failure condition is matched with 'in' (substring) while the plain ejected condition uses exact string equality. This asymmetry is fragile: minor whitespace variation in the recipe YAML would cause ejected_idx to be None, producing a misleading 'ejected route must still exist' failure rather than an ordering failure.

[Trecek]

Investigated — this is intentional. The actual YAML condition string is '${{ result.pr_state }} == ejected_ci_failure' (a full template expression). Substring match ('ejected_ci_failure' in w) is the only correct approach to match this full expression. The ejected condition uses exact equality because its full expression '${{ result.pr_state }} == ejected' can be matched exactly and is unambiguous. The asymmetry is deliberate: substring is needed for ejected_ci_failure because it appears inside a longer expression string, not as a standalone value.

[Trecek]

[info] tests: The assertion only verifies head_sha is present and correct. It does not assert that the CI watcher result fields (run_id, conclusion, failed_jobs) are also present in the merged output. A bug where the merge returns only {head_sha: ...} would not be caught.

[Trecek]

[info] tests: The test checks that head_sha and exit_code are absent on git failure, but does not verify the CI watcher result fields are still present. If the implementation accidentally returns an empty dict on git failure, this test would still pass.

[Trecek]

[info] cohesion: ci_watch_post_queue_fix in remediation.yaml is byte-for-byte identical to implementation.yaml, including skip_when_false: 'inputs.open_pr'. The remediation flow operates on already-open PRs — confirm whether this skip_when_false guard is semantically correct for the remediation recipe context.

[Trecek]

AutoSkillit review found 8 blocking issues (verdict: changes_requested). See inline comments for details.