Implementation Plan: Orchestrator Must Claim All Issues Upfront in Multi-Issue Runs

[Trecek]

Summary

When process-issues orchestrates a multi-issue run, claim_issue is currently deferred to each recipe's internal step graph. This leaves every not-yet-started issue unclaimed and vulnerable to parallel pickup by another session. The fix requires three coordinated changes:

1. claim_issue MCP tool gains an allow_reentry: bool = False parameter — when True and the in-progress label is already present, returns claimed: true (reentry) instead of claimed: false (blocked).
2. process-issues skill is refactored to claim all manifest issues upfront before dispatching any recipe, track which were successfully claimed, pass upfront_claimed: "true" as a recipe ingredient, and release uncompleted issues on fatal failure.
3. Three recipes (implementation.yaml, remediation.yaml, implementation-groups.yaml) each gain an upfront_claimed ingredient (default "false") and pass it as allow_reentry to their claim_issue step so that a pre-claim by the orchestrator is recognized as "proceed" rather than "abort".

Requirements

BATCH

• REQ-BATCH-001: The process-issues skill must call claim_issue for every issue in the triage manifest before dispatching any recipe execution.
• REQ-BATCH-002: The system must iterate through all issues in the manifest and call claim_issue individually for each, collecting results before proceeding.
• REQ-BATCH-003: Issues where claim_issue returns claimed: false (already claimed by another session) must be excluded from the dispatch list and logged as skipped.

COMPAT

• REQ-COMPAT-001: Per-recipe claim_issue steps must not abort when the in-progress label was already applied by the same orchestration session's upfront claim.
• REQ-COMPAT-002: The claim_issue tool's on_result routing in recipes must treat a pre-existing label applied by the current session as a proceed condition, not an escalate-stop condition.
• REQ-COMPAT-003: Single-issue recipe flows (no orchestrator) must continue to function identically to current behavior.

RELEASE

• REQ-RELEASE-001: The process-issues skill must release all upfront-claimed but unprocessed issues when the orchestrator encounters a fatal failure.
• REQ-RELEASE-002: The system must track which issues were claimed upfront and which have been handed off to recipe execution, so that only uncompleted issues are released on failure.
• REQ-RELEASE-003: Issues that completed recipe execution (success or recipe-level failure with its own release) must not be double-released by the orchestrator cleanup.

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 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;

    %% TERMINALS %%
    START([● process-issues invoked])
    DONE([DONE])
    ERROR([FATAL ERROR])

    subgraph Phase0 ["Phase 0 — Parse & Discover"]
        direction TB
        Parse["Parse args<br/>━━━━━━━━━━<br/>--batch N, --dry-run,<br/>--comment, --merge-batch"]
        Discover["Discover manifest<br/>━━━━━━━━━━<br/>triage_manifest_*.json"]
        DryRun{"--dry-run?"}
        Confirm{"User confirms Y/n?"}
    end

    subgraph Phase1 ["● Phase 1 — Upfront Claiming (MODIFIED)"]
        direction TB
        Flatten["● Flatten all issues<br/>━━━━━━━━━━<br/>Collect issues from<br/>all selected batches"]
        ClaimCall["● claim_issue()<br/>━━━━━━━━━━<br/>allow_reentry=False<br/>for each issue"]
        ClaimedDecision{"● claimed == true?"}
        TrackClaimed["● pre_claimed_urls<br/>━━━━━━━━━━<br/>append issue_url"]
        TrackSkipped["● Log skipped<br/>━━━━━━━━━━<br/>foreign session owns it"]
    end

    subgraph Phase2 ["● Phase 2 — Batch Dispatch (MODIFIED)"]
        direction TB
        BatchIssueLoop{"For each issue<br/>in batch (asc order)"}
        InPreClaimed{"● issue_url in<br/>pre_claimed_urls?"}
        SkipIssue["Skip issue<br/>━━━━━━━━━━<br/>not pre-claimed"]
        LoadRecipe["load_recipe()<br/>━━━━━━━━━━<br/>● with upfront_claimed:<br/>'true' ingredient"]
        MarkDone["● completed_urls<br/>━━━━━━━━━━<br/>append after recipe<br/>returns"]
    end

    subgraph RecipeInternal ["● Recipe Internal — claim_issue step (MODIFIED)"]
        direction TB
        RecipeClaim["● recipe claim_issue<br/>━━━━━━━━━━<br/>allow_reentry:<br/>inputs.upfront_claimed"]
        ClaimTool["● claim_issue tool<br/>━━━━━━━━━━<br/>allow_reentry=True:<br/>label present→claimed=true<br/>allow_reentry=False:<br/>label present→claimed=false"]
        ClaimResult{"result.claimed<br/>== true?"}
        ProceedRecipe["compute_branch<br/>━━━━━━━━━━<br/>continue recipe..."]
        EscalateStop["escalate_stop<br/>━━━━━━━━━━<br/>foreign claim detected"]
    end

    subgraph Phase3 ["● Phase 3 — Fatal Cleanup (NEW)"]
        direction TB
        Diff["● Compute uncompleted<br/>━━━━━━━━━━<br/>pre_claimed_urls −<br/>completed_urls"]
        ReleaseLoop["● release_issue()<br/>━━━━━━━━━━<br/>for each uncompleted url"]
    end

    subgraph Phase4 ["Phase 4 — Summary"]
        direction TB
        WriteReport["Write process_report<br/>━━━━━━━━━━<br/>successes/failures/<br/>skipped counts"]
    end

    %% FLOW %%
    START --> Parse --> Discover --> DryRun
    DryRun -->|"yes"| DONE
    DryRun -->|"no"| Confirm
    Confirm -->|"n"| DONE
    Confirm -->|"Y"| Flatten

    Flatten --> ClaimCall
    ClaimCall --> ClaimedDecision
    ClaimedDecision -->|"true"| TrackClaimed --> ClaimCall
    ClaimedDecision -->|"false"| TrackSkipped --> ClaimCall
    ClaimCall -->|"all done"| BatchIssueLoop

    BatchIssueLoop --> InPreClaimed
    InPreClaimed -->|"no"| SkipIssue --> BatchIssueLoop
    InPreClaimed -->|"yes"| LoadRecipe
    LoadRecipe --> MarkDone --> BatchIssueLoop
    BatchIssueLoop -->|"all done"| WriteReport --> DONE

    LoadRecipe -->|"fatal error"| Diff
    Diff --> ReleaseLoop --> ERROR

    LoadRecipe -.->|"dispatches"| RecipeClaim
    RecipeClaim --> ClaimTool --> ClaimResult
    ClaimResult -->|"true"| ProceedRecipe
    ClaimResult -->|"false"| EscalateStop

    %% CLASS ASSIGNMENTS %%
    class START,DONE,ERROR terminal;
    class Parse,Discover handler;
    class DryRun,Confirm stateNode;
    class Flatten,ClaimCall,TrackClaimed,TrackSkipped newComponent;
    class ClaimedDecision stateNode;
    class BatchIssueLoop phase;
    class InPreClaimed stateNode;
    class SkipIssue detector;
    class LoadRecipe,MarkDone handler;
    class RecipeClaim,ClaimTool newComponent;
    class ClaimResult stateNode;
    class ProceedRecipe handler;
    class EscalateStop detector;
    class Diff,ReleaseLoop newComponent;
    class WriteReport output;

Loading

Color Legend:

Color	Category	Description
Dark Blue	Terminal	Start, done, and error states
Teal	State	Decision and routing nodes
Purple	Phase	Control flow and loop nodes
Orange	Handler	Processing and execution nodes
Green	Modified/New	● Components changed by this PR
Red	Detector	Validation gates and failure handling
Dark Teal	Output	Generated artifacts and results

Closes #445

Implementation Plan

Plan file: temp/make-plan/orchestrator_upfront_claim_plan_2026-03-20_171414.md

Token Usage Summary

Token Usage Summary

Step	input	output	cached	count	time
plan	9.7k	265.2k	8.3M	16	1h 47m
verify	294	235.8k	11.5M	15	1h 20m
implement	6.2k	313.3k	40.0M	15	2h 42m
fix	60	13.0k	1.1M	3	11m 58s
audit_impl	132	73.4k	2.5M	9	25m 3s
open_pr	370	207.5k	10.8M	14	1h 28m
review_pr	203	259.5k	5.7M	8	1h 3m
resolve_review	3.7k	183.4k	13.7M	8	1h 21m
resolve_conflicts	75	30.6k	2.6M	3	10m 44s
diagnose_ci	22	7.3k	466.1k	1	2m 33s
resolve_ci	13	3.1k	239.8k	1	2m 4s
Total	20.7k	1.6M	97.1M		10h 35m

🤖 Generated with Claude Code via AutoSkillit

[Trecek]

AutoSkillit PR Review — Verdict: changes_requested. Found 4 blocking issues (weak assertions in tests) and 1 info finding. See inline comments.

[Trecek]

[warning] tests: test_claim_issue_allow_reentry_true_returns_claimed_true_when_already_labeled calls claim_issue directly without confirming that tool_ctx.github_client is wired to the same context object that _require_enabled()/_ctx() inside claim_issue uses. If the implementation pulls from a different singleton the mock will silently not intercept calls and the test may succeed for the wrong reason. Verify the fixture injection mechanism matches the one used by passing tests in this class.

[Trecek]

Investigated — this is intentional. The tool_ctx fixture at tests/conftest.py:206 does monkeypatch.setattr(_state, "_ctx", ctx). Inside claim_issue, _get_ctx() reads _state._ctx — the exact same object. Setting tool_ctx.github_client = mock_client mutates the object already in _state._ctx, so the mock intercepts correctly. This is the established pattern used by all other passing tests in this class (e.g. test_claim_issue_returns_gate_error). No change needed.

[Trecek]

[info] tests: assert result.get("reentry", False) is False masks the distinction between the key being absent and being explicitly set to False. If the contract is that reentry should not appear in the normal-claim path, assert "reentry" not in result is more precise.