Rectify: check_review_posted Step Wiring (#4205)

## Summary

Parts A and B of issue #4204 introduced the `check_review_posted` step
and `review-effect-verification-waypoint` rule across all 4 bundled
recipes, but the combined merged tree has 74 failing tests from 5
mechanical defects introduced by an incomplete round-3 remediation
(`fe8931e8f`):

- **RC1**: `optional: true` without `skip_when_false` left on
`check_review_posted` in 3 recipes (implementation,
implementation-groups, remediation) → `optional-without-skip-when` ERROR
cascades to 70+ tests
- **RC2**: No catch-all arm in `check_review_posted.on_result` in all 4
recipes → `unrouted-callable-verdict` ERROR (sentinel output
`no_reviews_posted` is unrouted)
- **RC3**: `check_review_posted.output_dir` in merge-prs uses a static
relative path (`{{AUTOSKILLIT_TEMP}}/review-pr`, no `${{` template) with
no `work_dir` → `run-python-requires-work-dir` ERROR
- **RC4**: `_review_effect_verification_waypoint` step-selection uses
`.startswith("/review-pr")` — never matches `/autoskillit:review-pr`
forms → rule permanently silent → 3 "fires" tests fail
- **RC4a**: `re_push_review_integration.on_success:
derive_batch_ci_event` in merge-prs bypasses `check_review_posted` on
the rebase-then-repush path → will cause the rule to fire on merge-prs
once RC4 is fixed

Part A eliminates all 5 defects. Part B (separate task, explicit
authorization required) adds architectural immunity to prevent
recurrence.

**Note on merged tree state:** After merging both impl branches,
`_analysis_bfs.py` already supports `barrier: str | frozenset[str]`
(updated by Part B), so the `frozenset` barrier call in
`_review_effect_verification_waypoint` is correct and requires no
change. The `check_review_posted` callable, its `skill_contracts.yaml`
entry, `smoke_utils` exports, and `tests/test_smoke_utils.py` are all
present and passing. Only the 5 defects above need repair.

Closes #4204

## Implementation Plan

Plan file:
`/home/talon/projects/autoskillit-runs/remediation-20260707-160731-479581/.autoskillit/temp/rectify/rectify_check-review-posted-wiring_2026-07-07_163000_part_a.md`

🤖 Generated with [Claude Code](https://claude.com/claude-code) via
AutoSkillit
<!-- autoskillit:pipeline-signature
steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr
-->

## Token Usage Summary

| Step | Model | count | uncached | output | cache_read | peak_ctx |
turns | cache_write | time |

|------|-------|-------|----------|--------|------------|----------|-------|-------------|------|
| investigate* | fable | 1 | 17.0k | 59.5k | 2.7M | 181.9k | 71 | 236.8k
| 21m 39s |
| rectify* | sonnet | 1 | 225 | 59.8k | 2.5M | 148.3k | 77 | 130.0k |
30m 29s |
| dry_walkthrough* | sonnet | 2 | 1.2k | 37.6k | 993.9k | 81.8k | 57 |
113.6k | 15m 1s |
| implement* | sonnet | 2 | 472 | 30.7k | 3.6M | 86.7k | 166 | 131.5k |
10m 57s |
| audit_impl* | sonnet | 2 | 188 | 31.7k | 947.3k | 67.4k | 62 | 112.2k
| 11m 59s |
| prepare_pr* | MiniMax-M3 | 1 | 44.2k | 3.9k | 199.8k | 47.3k | 14 | 0
| 52s |
| compose_pr* | MiniMax-M3 | 1 | 40.0k | 3.4k | 192.1k | 41.9k | 13 | 0
| 48s |
| **Total** | | | 103.3k | 226.4k | 11.2M | 181.9k | | 724.2k | 1h 31m |

\* *Step used a non-Anthropic provider; caching behavior may differ.*

## Token Efficiency

| Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC |
|------|-------------|----------------|-----------------|------------|
| investigate | 0 | — | — | — |
| rectify | 0 | — | — | — |
| dry_walkthrough | 0 | — | — | — |
| implement | 1567 | 2325.4 | 83.9 | 19.6 |
| audit_impl | 0 | — | — | — |
| prepare_pr | 0 | — | — | — |
| compose_pr | 0 | — | — | — |
| **Total** | **1567** | 7147.9 | 462.1 | 144.5 |

## Model Usage Breakdown

| Model | steps | uncached | output | cache_read | cache_write | time |
|-------|-------|----------|--------|------------|-------------|------|
| fable | 1 | 17.0k | 59.5k | 2.7M | 236.8k | 21m 39s |
| sonnet | 4 | 2.1k | 159.7k | 8.1M | 487.3k | 1h 8m |
| MiniMax-M3 | 2 | 84.2k | 7.2k | 391.9k | 0 | 1m 40s |

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: Trecek

src/autoskillit/core/types/_type_backend.py

@@ -142,6 +142,8 @@ class BackendCapabilities:
     has_unguarded_filesystem_access: bool = field(default=False)
     # True when backend's git metadata directories (.git/worktrees/) are writable
     git_metadata_writable: bool = field(default=True)
+    # True when the backend can make outbound GitHub API write calls without sandbox restriction
+    github_api_callable: bool = field(default=False)
     # Native skill invocation prefix character used by this backend's model/CLI.
     # Claude Code uses "/" (slash-commands via the Skill tool).
     # Codex uses "$" (dollar-mention via extract_tool_mentions).
@@ -262,6 +264,7 @@ def model_class(model: str) -> str:
     supports_context_window_suffix=True,
     has_unguarded_filesystem_access=False,
     git_metadata_writable=True,
+    github_api_callable=True,
     skill_sigil="/",
     session_dir_persistent=False,
     supports_model_invocation_gating=True,
@@ -313,6 +316,7 @@ class SkillSessionConfig:
     resume_message: str | None = None
     sandbox_mode: str = "workspace-write"
     backend_override: str | None = None
+    network_access: bool = False
 
 
 @dataclass(frozen=True, slots=True)

src/autoskillit/core/types/_type_constants_registries.py

@@ -302,10 +302,14 @@ class SkillCapabilityDef:
 
     description: str
     codex_status: Literal["works-as-is", "degraded", "fix-required", "not-applicable"]
+    required_sandbox_overrides: frozenset[str] = frozenset()
+    worker_routable: bool = False
 
     @property
     def required_backends(self) -> frozenset[str]:
-        if self.codex_status == "not-applicable":
+        # worker_routable=True → reroute (REROUTE), not reject: required_backends must be empty
+        # so the compat gate doesn't fire for routable capabilities.
+        if self.codex_status == "not-applicable" and not self.worker_routable:
             return frozenset({AGENT_BACKEND_CLAUDE_CODE})
         return frozenset()
 
@@ -353,6 +357,16 @@ def required_backends(self) -> frozenset[str]:
             "git worktree add, git checkout -b)"
         ),
         codex_status="not-applicable",
+        worker_routable=True,
+    ),
+    "github_api_write": SkillCapabilityDef(
+        description=(
+            "Skill makes outbound GitHub API write calls (gh pr review, gh api --method POST, "
+            "gh pr create, gh pr merge, gh issue create/edit/close, etc.) that require network "
+            "access. On Codex workers, enables network_access=true in the workspace-write sandbox."
+        ),
+        codex_status="fix-required",
+        required_sandbox_overrides=frozenset({"sandbox_workspace_write.network_access=true"}),
     ),
 }
 

src/autoskillit/core/types/_type_protocols_execution.py

@@ -73,6 +73,7 @@ async def run(
         caller_session_id: str | None = None,
         inspector_eligible: bool = False,
         inspector_model: str = "",
+        network_access: bool = False,
     ) -> SkillResult: ...
 
     async def dispatch_food_truck(

src/autoskillit/execution/backends/_backend_cmd_builder_base.py

@@ -163,6 +163,7 @@ def _apply_config(self, config: SkillSessionConfig) -> dict[str, Any]:
             "resume_checkpoint": config.resume_checkpoint,
             "resume_message": config.resume_message,
             "sandbox_mode": config.sandbox_mode,
+            "network_access": config.network_access,
         }
 
 

src/autoskillit/execution/backends/codex.py

@@ -592,6 +592,7 @@ def capabilities(self) -> BackendCapabilities:
             # field; rules_backend_compat.py emits ERROR for
             # git_metadata_write skills on backends where this is False.
             git_metadata_writable=False,  # sandbox excludes .git metadata path
+            github_api_callable=False,
             skill_sigil="$",
             session_dir_persistent=True,
             supports_model_invocation_gating=False,
@@ -692,6 +693,7 @@ def build_skill_session_cmd(
         resume_checkpoint: SessionCheckpoint | None = None,
         resume_message: str | None = None,
         sandbox_mode: str = "workspace-write",
+        network_access: bool = False,
     ) -> CmdSpec:
         if config is not None:
             cfg = self._apply_config(config)
@@ -712,6 +714,7 @@ def build_skill_session_cmd(
             resume_checkpoint = cfg["resume_checkpoint"]
             resume_message = cfg["resume_message"]
             sandbox_mode = cfg["sandbox_mode"]
+            network_access = cfg.get("network_access", False)
         if plugin_source is not None:
             logger.warning("codex_plugin_source_discarded", plugin_source=str(plugin_source))
         if output_format != OutputFormat.JSON:
@@ -790,9 +793,13 @@ def build_skill_session_cmd(
             required=SKILL_SESSION_REQUIRED_ENV | {MCP_CLIENT_BACKEND_ENV_VAR},
         )
 
+        _net_overrides: list[str] = []
+        if network_access:
+            _net_overrides.append("sandbox_workspace_write.network_access=true")
         cmd = _codex_exec_base(
             sandbox=sandbox_mode,
             bypass_hook_trust=self.capabilities.mcp_config_capable,
+            extra_overrides=_net_overrides,
         )
         if model:
             cmd += [CodexFlags.MODEL, self.translate_model(model)]

src/autoskillit/execution/headless/__init__.py

@@ -145,6 +145,7 @@ async def run_headless_core(
     caller_session_id: str | None = None,
     inspector_eligible: bool = False,
     inspector_model: str = "",
+    network_access: bool = False,
 ) -> SkillResult:
     """Shared headless runner used by run_skill.
 
@@ -193,6 +194,7 @@ async def run_headless_core(
             sandbox_mode="read-only"
             if readonly_skill
             else ctx.backend.capabilities.default_skill_sandbox_mode,
+            network_access=network_access,
         )
         step_backend: CodingAgentBackend | None = None
         if backend_override is not None:
@@ -303,6 +305,7 @@ async def run(
         caller_session_id: str | None = None,
         inspector_eligible: bool = False,
         inspector_model: str = "",
+        network_access: bool = False,
     ) -> SkillResult:
         cfg = self._ctx.config.run_skill
         effective_timeout = timeout if timeout is not None else cfg.timeout
@@ -344,6 +347,7 @@ async def run(
             caller_session_id=caller_session_id,
             inspector_eligible=inspector_eligible,
             inspector_model=inspector_model,
+            network_access=network_access,
         )
 
     async def dispatch_food_truck(

src/autoskillit/recipe/_analysis_bfs.py

@@ -99,12 +99,16 @@ def _build_success_step_graph(recipe: Recipe) -> dict[str, set[str]]:
 def bfs_reachable_without_barrier(
     recipe: Recipe,
     start: str,
-    barrier: str,
+    barrier: str | frozenset[str],
 ) -> set[str]:
     """BFS from ``start`` through success-path routing edges, stopping at ``barrier``.
 
     Returns all step names reachable from ``start`` without crossing ``barrier``.
-    The barrier step itself is not included in the returned set.
+    The barrier step itself is included in the returned set — it is visited but
+    not expanded beyond.
+
+    ``barrier`` may be a single step name (str) or a frozenset of step names.
+    When a frozenset is provided, any of the named steps acts as a barrier.
 
     Only follows on_result conditions and on_success edges — error paths
     (on_failure, on_context_limit) are excluded because they are not
@@ -113,8 +117,9 @@ def bfs_reachable_without_barrier(
     This is the canonical implementation of the BFS-barrier pattern previously
     duplicated inline in ``push-before-audit`` and ``merge-base-unpublished``.
     """
+    barriers: set[str] = {barrier} if isinstance(barrier, str) else set(barrier)
     graph = _build_success_step_graph(recipe)
-    return _bfs_capped(graph, {start}, {barrier})
+    return _bfs_capped(graph, {start}, barriers)
 
 
 def _bfs_capped(

src/autoskillit/recipe/rules/graph/rules_graph_review.py

@@ -163,6 +163,61 @@ def _check_run_skill_missing_context_limit(ctx: ValidationContext) -> list[RuleF
     return findings
 
 
+_REVIEW_EFFECT_ADVANCE_GATES = frozenset({"check_review_loop", "derive_batch_ci_event"})
+
+
+def _get_review_pr_steps(ctx: ValidationContext) -> list[str]:
+    """Return step IDs that dispatch the review-pr skill (either prefix form).
+
+    Exposed as a module-level function (not inlined in the rule body) so that
+    silence tests can call it directly and assert the rule is triggered on the
+    recipe under test before asserting zero findings.
+    """
+    return [
+        step_id
+        for step_id, step in ctx.recipe.steps.items()
+        if step.tool == "run_skill" and step.skill_name == "review-pr"
+    ]
+
+
+@semantic_rule(
+    name="review-effect-verification-waypoint",
+    severity=Severity.ERROR,
+    description=(
+        "No advance gate (check_review_loop or derive_batch_ci_event) must be reachable "
+        "from any review-pr skill step on the success path without first crossing "
+        "check_review_posted. Applies to all steps dispatching the review-pr skill "
+        "regardless of step id (covers review_pr_integration in merge-prs.yaml)."
+    ),
+)
+def _review_effect_verification_waypoint(ctx: ValidationContext) -> list[RuleFinding]:
+    effect_steps = _get_review_pr_steps(ctx)
+    if not effect_steps:
+        return []
+    findings = []
+    for start in effect_steps:
+        reachable = bfs_reachable_without_barrier(
+            ctx.recipe,
+            start=start,
+            barrier=frozenset({"check_review_posted"}),
+        )
+        for advance_gate in _REVIEW_EFFECT_ADVANCE_GATES:
+            if advance_gate in reachable:
+                findings.append(
+                    make_finding(
+                        rule_name="review-effect-verification-waypoint",
+                        step_name=start,
+                        message=(
+                            f"Step '{start}' (review-pr skill) can reach '{advance_gate}' "
+                            f"without crossing check_review_posted. Insert a check_review_posted "
+                            f"run_python step on every success path from '{start}' to "
+                            f"'{advance_gate}'."
+                        ),
+                    )
+                )
+    return findings
+
+
 @semantic_rule(
     name="review-mode-reentry-waypoint-guard",
     description=(

src/autoskillit/recipe/schema.py

@@ -17,6 +17,7 @@
     CaptureEntrySpec,
     DispatchGateType,
     RecipeSource,
+    resolve_skill_name,
 )
 
 AUTOSKILLIT_VERSION_KEY: Final = "autoskillit_version"
@@ -155,6 +156,16 @@ def __post_init__(self) -> None:
                 f"the list, producing duplicates. Set retries: 0."
             )
 
+    @property
+    def skill_name(self) -> str | None:
+        """Canonical skill name, resolving /name and /autoskillit:name forms.
+
+        Returns None if with_args has no skill_command or if resolve_skill_name
+        cannot extract a name (template-only, unrecognized format, etc.).
+        Use this in rule predicates instead of raw string operations on with_args.
+        """
+        return resolve_skill_name(str((self.with_args or {}).get("skill_command", "")))
+
 
 @dataclass(frozen=True, slots=True)
 class RecipeBlock:

src/autoskillit/recipe/skill_contracts.yaml

@@ -2574,6 +2574,35 @@ callable_contracts:
       type: str
     - name: had_blocking
       type: str
+  autoskillit.smoke_utils.check_review_posted:
+    description: >
+      Verifies that batch_review_response_{pr_number}.json exists in output_dir
+      (github mode) or always passes (local mode). No subprocess calls.
+      output_dir must be an absolute path.
+    inputs:
+    - name: pr_number
+      type: int
+      required: true
+    - name: output_dir
+      type: str
+      required: true
+    - name: mode
+      type: str
+      required: true
+      allowed_values:
+      - local
+      - github
+    outputs:
+    - name: reviews_posted
+      type: str
+      allowed_values:
+      - "true"
+      - "false"
+    - name: sentinel
+      type: str
+      allowed_values:
+      - no_reviews_posted
+      - ""
   autoskillit.smoke_utils.check_bug_report_non_empty:
     inputs:
     - name: workspace