Rectify: Ingredient Key Normalization and Issue-URL Extraction Immunity

src/autoskillit/fleet/AGENTS.md

@@ -17,6 +17,7 @@ IL-2 fleet campaign layer — parallel issue dispatch, semaphore, sidecar, liven
 | `sidecar.py` | Per-issue JSONL sidecar — `IssueSidecarEntry`, append/read/`compute_remaining` helpers |
 | `_dispatch_reaper.py` | Stale dispatch process reaping — `reap_stale_dispatches()`, `reap_stale_dispatches_async()` |
 | `_label_cleanup.py` | Infrastructure-level label cleanup — `cleanup_orphaned_labels`, `sweep_stale_dispatch_labels`, `discover_campaign_state_files` |
+| `_issue_url_helpers.py` | Canonical issue-URL extraction — `extract_issue_urls()` dual-key accessor (plural `issue_urls` wins, singular `issue_url` fallback) |
 | `_liveness.py` | `is_dispatch_session_alive()` — boot_id + starttime_ticks liveness gate |
 | `_semaphore.py` | `FleetSemaphore` — configurable `asyncio.BoundedSemaphore` implementing `FleetLock` |
 | `_sidecar_rpc.py` | `run_python`-callable entry points: `write_sidecar_entry`, `get_remaining_issues` |

src/autoskillit/fleet/_api.py

@@ -24,6 +24,7 @@
 )
 from autoskillit.fleet._capture import _extract_captures, _normalize_capture_spec
 from autoskillit.fleet._expressions import _CAMPAIGN_REF_RE, _interpolate_campaign_refs
+from autoskillit.fleet._issue_url_helpers import extract_issue_urls
 from autoskillit.fleet._outcome import _checkpoint_to_dict, classify_dispatch_outcome
 from autoskillit.fleet.result_parser import parse_l3_result_block
 from autoskillit.fleet.state import DispatchStatus
@@ -653,7 +654,7 @@ def _reject_with_state(error_code: FleetErrorCode, message: str) -> DispatchResu
         [f"%%L3_DONE::{pid[:8]}%%" for pid in prior_ids] if prior_ids else None
     )
 
-    _issue_urls_raw = effective_ingredients.get("issue_urls", "") if effective_ingredients else ""
+    _issue_urls_raw = extract_issue_urls(effective_ingredients)
 
     def _on_spawn(pid: int, ticks: int) -> None:
         from autoskillit.core import read_boot_id

src/autoskillit/fleet/_issue_url_helpers.py

@@ -0,0 +1,29 @@
+"""Canonical issue-URL extraction from recipe ingredients.
+
+Single accessor for the dual-key ``issue_urls`` (plural, CSV) and ``issue_url``
+(singular) ingredient forms. Plural wins when both are present — it is the
+canonical recipe-side key for batch recipes (``bem-wrapper``, ``implement-findings``);
+singular is the fallback for single-issue recipes (``implementation``,
+``remediation``, ``research-*``).
+
+Centralizing the lookup at a single function prevents the singular/plural
+mismatch that orphaned labels for the 7th time (issue #4112).
+"""
+
+from __future__ import annotations
+
+
+def extract_issue_urls(ingredients: dict[str, str] | None) -> str:
+    """Return the issue-URL CSV from a recipe ingredients dict.
+
+    Tries ``issue_urls`` first (plural, batch recipes), then falls back to
+    ``issue_url`` (singular, single-issue recipes). Returns ``""`` if neither
+    key is present, both values are empty strings, or the ingredients dict is
+    ``None``/empty.
+    """
+    if not ingredients:
+        return ""
+    raw = ingredients.get("issue_urls", "")
+    if not raw:
+        raw = ingredients.get("issue_url", "")
+    return raw

src/autoskillit/skills_extended/resolve-review/SKILL.md

@@ -506,6 +506,7 @@ classified `REJECT` with `category: "arch_violation"`.
 | MCP env forward coverage | `test_mcp_env_forward_coverage.py` | `mcp_env_forward_vars` values missing from `CmdSpec.env` in any cmd-builder (skill, food-truck, headless, resume, interactive) |
 | Rule severity consistency | `test_rule_severity_consistency.py` | Direct `RuleFinding()` construction in `@semantic_rule`/`@block_rule` bodies — must use `make_finding()`/`make_block_finding()`; `_KNOWN_NON_CONFORMING_RULES` entries without `# tracking: #NNNN` comments |
 | No direct swap_labels in fleet | `test_swap_labels_guard.py` | Direct `swap_labels` calls in `fleet/` outside `_label_cleanup.py` — must route through `cleanup_orphaned_labels` |
+| Issue URL extraction guard | `test_issue_url_extraction_guard.py` | Raw `.get("issue_url")` / `.get("issue_urls")` in `fleet/` outside `_issue_url_helpers.py` and `state_types.py` — must use `extract_issue_urls()`; `fleet_claim_guard.py` must retain both key variants |
 | Pipeline ordering | `test_pipeline_ordering.py` | Moving `run_semantic_rules` before `_prune_skipped_steps` in `load_and_validate` — semantic rules must run on post-prune recipe |
 
 When a reviewer suggestion would cause a change matching any row above, classify

tests/_test_filter.py

@@ -769,7 +769,7 @@ class ImportContext(enum.StrEnum):
             # Execution file-level entries:
             "execution/test_headless_path_validation.py",
             "execution/test_zero_write_detection.py",
-            # Fleet file-level entries (8 of N import autoskillit.recipe):
+            # Fleet file-level entries (9 of N import autoskillit.recipe):
             "fleet/test_fleet_e2e.py",
             "fleet/test_fleet_e2e_codex.py",
             "fleet/test_campaign_capture.py",
@@ -780,6 +780,7 @@ class ImportContext(enum.StrEnum):
             "fleet/test_dispatch_state_handle.py",
             "fleet/test_research_campaign_dispatch.py",
             "fleet/test_dispatch_failure_semantics.py",
+            "fleet/test_dispatch_labels_cleaned.py",
             "fleet/test_gate_state_persistence.py",
             # Other file-level entries:
             "infra/test_pretty_output_recipe.py",

tests/arch/AGENTS.md

@@ -110,6 +110,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests.
 | `test_cmd_spec_resume_no_string_match.py` | AST guard: no `"--resume" in <expr>` patterns in production code — use CmdSpec.is_resume instead |
 | `test_subagent_filter_guard.py` | AST guard: all assistant-record NDJSON processing sites must use _is_parent_assistant_record or _is_parent_assistant predicate |
 | `test_swap_labels_guard.py` | AST guard: direct swap_labels calls in fleet/ must go through cleanup_orphaned_labels |
+| `test_issue_url_extraction_guard.py` | AST guard: raw `.get('issue_url')` / `.get('issue_urls')` banned in fleet/; dual-key asserted in fleet_claim_guard.py |
 | `test_enqueue_ready_type_enforcement.py` | AST guard: mutation methods (_enqueue_direct, _enable_auto_merge_direct) must accept EnqueueReady, not str |
 | `test_origin_isolation_contract.py` | AST + shell lint guard: no hardcoded "origin" in git remote operations outside allowlist; shell scripts must try upstream before origin |
 | `test_backend_annotation_context_awareness.py` | Context-aware pattern detection: distinguishes self-referential autoskillit: documentation from genuine cross-skill dependencies |

tests/arch/test_issue_url_extraction_guard.py

@@ -0,0 +1,103 @@
+"""AST guard: raw ``.get('issue_url')`` / ``.get('issue_urls')`` calls banned in ``fleet/``.
+
+Issue #4112 defense-in-depth: the canonical extraction function lives in
+``_issue_url_helpers.py``, and any raw ``.get()`` call elsewhere in ``fleet/``
+(except ``_issue_url_helpers.py`` itself and ``state_types.py``, which
+deserializes ``DispatchRecord`` from its own JSON dict) would re-introduce the
+singular/plural key mismatch that orphaned labels for the 7th time.
+
+Also enforces that ``fleet_claim_guard.py`` retains BOTH key variants in its
+inline dual-key lookup — the guard is a stdlib-only hook script and cannot
+import the canonical helper, so it keeps its inline pattern and this test
+guards that both ``issue_urls`` and ``issue_url`` keys remain present.
+"""
+
+from __future__ import annotations
+
+import ast
+from pathlib import Path
+
+import pytest
+
+pytestmark = [pytest.mark.layer("arch"), pytest.mark.small]
+
+
+SRC_ROOT = Path(__file__).resolve().parent.parent.parent / "src" / "autoskillit" / "fleet"
+HOOK_GUARD_PATH = (
+    Path(__file__).resolve().parent.parent.parent
+    / "src"
+    / "autoskillit"
+    / "hooks"
+    / "guards"
+    / "fleet_claim_guard.py"
+)
+
+# Files exempt from the raw-``get`` ban:
+# - ``_issue_url_helpers.py``: defines the canonical accessor.
+# - ``state_types.py``: deserializes ``DispatchRecord`` from its own JSON dict.
+EXEMPT_FILES: frozenset[str] = frozenset({"_issue_url_helpers.py", "state_types.py"})
+
+BANNED_KEYS: frozenset[str] = frozenset({"issue_url", "issue_urls"})
+
+
+def _find_issue_url_get_calls(path: Path) -> list[tuple[int, str]]:
+    """Return ``(line, key)`` tuples for raw ``.get`` calls on issue-URL keys."""
+    try:
+        tree = ast.parse(path.read_text())
+    except SyntaxError:
+        return []
+    results: list[tuple[int, str]] = []
+    for node in ast.walk(tree):
+        if not isinstance(node, ast.Call):
+            continue
+        func = node.func
+        if not (isinstance(func, ast.Attribute) and func.attr == "get"):
+            continue
+        if not node.args:
+            continue
+        first_arg = node.args[0]
+        if not isinstance(first_arg, ast.Constant):
+            continue
+        if first_arg.value in BANNED_KEYS:
+            results.append((node.lineno, first_arg.value))
+    return results
+
+
+def test_no_raw_issue_url_get_in_fleet() -> None:
+    """Raw ``.get('issue_url')`` / ``.get('issue_urls')`` are banned in fleet/.
+
+    All fleet code must route through ``extract_issue_urls()`` to guarantee the
+    dual-key lookup is applied uniformly. Any direct ``.get()`` call would
+    re-introduce the singular/plural mismatch.
+    """
+    violations: list[str] = []
+    for py_file in sorted(SRC_ROOT.glob("*.py")):
+        if py_file.name in EXEMPT_FILES:
+            continue
+        for lineno, key in _find_issue_url_get_calls(py_file):
+            violations.append(f"{py_file.name}:{lineno} (.get({key!r}))")
+
+    assert not violations, (
+        "Raw .get('issue_url') / .get('issue_urls') calls in fleet/ must go "
+        f"through extract_issue_urls() in _issue_url_helpers.py. Found in: {violations}"
+    )
+
+
+def test_fleet_claim_guard_has_dual_key_lookup() -> None:
+    """``fleet_claim_guard.py`` must look up BOTH ``issue_urls`` AND ``issue_url``.
+
+    The hook is stdlib-only and cannot import ``extract_issue_urls()``, so it
+    keeps its inline dual-key pattern. This test enforces that both key
+    variants remain present — if a contributor removes either the plural or
+    singular lookup, this test fails.
+    """
+    assert HOOK_GUARD_PATH.exists(), f"Hook guard not found at {HOOK_GUARD_PATH}"
+    found_keys = {key for _, key in _find_issue_url_get_calls(HOOK_GUARD_PATH)}
+
+    assert "issue_urls" in found_keys, (
+        "fleet_claim_guard.py must look up the plural 'issue_urls' key."
+    )
+    assert "issue_url" in found_keys, (
+        "fleet_claim_guard.py must look up the singular 'issue_url' key "
+        "(dual-key ingredient access is required)."
+    )

tests/arch/test_subpackage_isolation.py

@@ -878,7 +878,7 @@ def test_no_subpackage_exceeds_10_files() -> None:
         "cli": 21,
         "hooks": 14,  # +recipe_confirmed_post_hook.py
         "pipeline": 12,
-        "fleet": 22,  # REQ-CNST-003-E9: _dispatch_reaper.py; +_sidecar_synthesis.py; +_reset.py
+        "fleet": 23,  # +_issue_url_helpers.py  # noqa: E501
         "recipe/rules": 51,  # +commit_guard_regression_route +rules_model +rules_gitignored_deliverable  # noqa: E501
         "server/tools": 27,  # +_preflight.py (dispatch-feasibility preflight)
         "hooks/guards": 32,  # +fleet_claim_guard, +reset_resume_gate, +recipe_read_guard

tests/fleet/AGENTS.md

@@ -23,6 +23,7 @@ Fleet campaign dispatch, state persistence, and sidecar tests.
 | `test_dispatch_recipe_kind_gate.py` | Recipe kind dispatch gate — FOOD_TRUCK accepted, CAMPAIGN rejected |
 | `test_dispatch_crash_diagnostics.py` | Crash path diagnostic persistence and structured logging |
 | `test_dispatch_labels_cleaned.py` | Labels_cleaned field persistence on failure/success outcomes |
+| `test_issue_url_extraction.py` | Unit tests for `extract_issue_urls()` dual-key canonical accessor (singular/plural ingredient key resolution) |
 | `test_dispatch_identity_continuity.py` | Tests for dispatch_id identity continuity on resume — prior_dispatch_id threading through API layer |
 | `test_dispatch_state_handle.py` | Tests for DispatchStateHandle factory and dispatch state invariants — resume path state file creation and capture persistence (Group J) |
 | `test_dispatch_lifespan.py` | Group G (fleet part): lifespan_started surface + envelope propagation |

tests/fleet/test_dispatch_labels_cleaned.py

@@ -175,3 +175,98 @@ async def test_labels_cleaned_true_with_no_client_no_sidecar(
         record = _read_dispatch_record(tool_ctx)
         assert record["status"] == "failure"
         assert record["labels_cleaned"] is True
+
+    @pytest.mark.anyio
+    async def test_singular_key_ingredient_triggers_fallback_cleanup(
+        self, tool_ctx, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Singular ``issue_url`` ingredient triggers cleanup via the ``issue_url`` fallback.
+
+        Without sidecar entries, ``cleanup_orphaned_labels`` falls back to the
+        ``issue_url`` kwarg. This test exercises that path with the singular
+        ``issue_url`` recipe ingredient (used by implementation/remediation).
+        Regression guard for issue #4112.
+        """
+        import dataclasses
+
+        from autoskillit.recipe.schema import RecipeIngredient
+        from tests.fakes import _DEFAULT_SKILL_RESULT
+
+        issue_url = "https://github.com/owner/repo/issues/42"
+        _setup_dispatch(
+            tool_ctx,
+            monkeypatch,
+            ingredients={"issue_url": RecipeIngredient(description="Issue URL")},
+        )
+        swap_labels_mock = AsyncMock(return_value={"success": True})
+        github_client = AsyncMock()
+        github_client.swap_labels = swap_labels_mock
+        tool_ctx.github_client = github_client
+
+        failure_result = dataclasses.replace(
+            _DEFAULT_SKILL_RESULT,
+            success=False,
+            result='{"success": false, "reason": "context_exhaustion"}',
+            subtype="success",
+            is_error=False,
+            exit_code=0,
+        )
+        # No sidecar is written — exercises the issue_url fallback path.
+        tool_ctx.executor = InMemoryHeadlessExecutor(default_result=failure_result)
+        monkeypatch.setattr(
+            "autoskillit.fleet._api.parse_l3_result_block",
+            lambda **_: _make_no_sentinel(),
+        )
+
+        await _run(tool_ctx, ingredients={"issue_url": issue_url})
+
+        # swap_labels should have been called via the issue_url fallback path.
+        swap_labels_mock.assert_awaited_once()
+        call_list = swap_labels_mock.call_args_list
+        assert len(call_list) == 1
+        owner_arg, repo_arg, number_arg = call_list[0].args
+        assert owner_arg == "owner"
+        assert repo_arg == "repo"
+        assert number_arg == 42
+
+    @pytest.mark.anyio
+    async def test_singular_key_ingredient_populates_dispatch_record_issue_url(
+        self, tool_ctx, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """DispatchRecord.issue_url is populated from the singular ``issue_url`` ingredient.
+
+        Regression guard for issue #4112: the field was previously stored as
+        ``""`` for single-issue recipes because ``_api.py`` only read the
+        plural ``issue_urls`` key.
+        """
+        import dataclasses
+
+        from autoskillit.recipe.schema import RecipeIngredient
+        from tests.fakes import _DEFAULT_SKILL_RESULT
+
+        issue_url = "https://github.com/owner/repo/issues/42"
+        _setup_dispatch(
+            tool_ctx,
+            monkeypatch,
+            ingredients={"issue_url": RecipeIngredient(description="Issue URL")},
+        )
+        tool_ctx.github_client = None  # skip label cleanup
+        tool_ctx.executor = InMemoryHeadlessExecutor(
+            default_result=dataclasses.replace(
+                _DEFAULT_SKILL_RESULT,
+                success=True,
+                result='{"success": true}',
+                subtype="success",
+                is_error=False,
+                exit_code=0,
+            )
+        )
+        monkeypatch.setattr(
+            "autoskillit.fleet._api.parse_l3_result_block",
+            lambda **_: _make_completed_clean(True),
+        )
+
+        await _run(tool_ctx, ingredients={"issue_url": issue_url})
+
+        record = _read_dispatch_record(tool_ctx)
+        assert record["issue_url"] == issue_url

tests/fleet/test_issue_url_extraction.py

@@ -0,0 +1,53 @@
+"""Unit tests for ``extract_issue_urls()`` dual-key canonical accessor.
+
+Validates the single canonical function that resolves the singular/plural
+ingredient-key mismatch that orphaned labels for the 7th time (issue #4112).
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from autoskillit.fleet._issue_url_helpers import extract_issue_urls
+
+pytestmark = [pytest.mark.layer("fleet"), pytest.mark.small, pytest.mark.feature("fleet")]
+
+
+def test_plural_key_only() -> None:
+    """Batch recipes use the plural ``issue_urls`` key."""
+    assert extract_issue_urls({"issue_urls": "url1,url2"}) == "url1,url2"
+
+
+def test_singular_key_only() -> None:
+    """Single-issue recipes use the singular ``issue_url`` key."""
+    assert extract_issue_urls({"issue_url": "url1"}) == "url1"
+
+
+def test_plural_wins_when_both_present() -> None:
+    """Plural takes precedence when both keys are populated."""
+    assert extract_issue_urls({"issue_urls": "url1", "issue_url": "url2"}) == "url1"
+
+
+def test_empty_dict() -> None:
+    """Empty ingredients dict returns empty string."""
+    assert extract_issue_urls({}) == ""
+
+
+def test_none_ingredients() -> None:
+    """``None`` ingredients returns empty string (defensive guard)."""
+    assert extract_issue_urls(None) == ""
+
+
+def test_empty_plural_falls_through_to_singular() -> None:
+    """Empty plural value falls through to singular lookup."""
+    assert extract_issue_urls({"issue_urls": "", "issue_url": "url1"}) == "url1"
+
+
+def test_unrelated_keys_ignored() -> None:
+    """Other ingredient keys do not affect extraction."""
+    assert extract_issue_urls({"label": "in-progress", "issue_url": "url1"}) == "url1"
+
+
+def test_both_empty_returns_empty() -> None:
+    """When both keys are empty, return empty string."""
+    assert extract_issue_urls({"issue_urls": "", "issue_url": ""}) == ""