diff --git a/src/autoskillit/core/types/_type_backend.py b/src/autoskillit/core/types/_type_backend.py index 76918f210e..27f44fec18 100644 --- a/src/autoskillit/core/types/_type_backend.py +++ b/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) diff --git a/src/autoskillit/core/types/_type_constants_registries.py b/src/autoskillit/core/types/_type_constants_registries.py index 23ae967a7b..d7b6dc6ce8 100644 --- a/src/autoskillit/core/types/_type_constants_registries.py +++ b/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"}), ), } diff --git a/src/autoskillit/core/types/_type_protocols_execution.py b/src/autoskillit/core/types/_type_protocols_execution.py index 189e73c7db..511c1b9542 100644 --- a/src/autoskillit/core/types/_type_protocols_execution.py +++ b/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( diff --git a/src/autoskillit/execution/backends/_backend_cmd_builder_base.py b/src/autoskillit/execution/backends/_backend_cmd_builder_base.py index ea4f059f67..afcefd391f 100644 --- a/src/autoskillit/execution/backends/_backend_cmd_builder_base.py +++ b/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, } diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index b5bb19dcee..c91961ce40 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/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)] diff --git a/src/autoskillit/execution/headless/__init__.py b/src/autoskillit/execution/headless/__init__.py index c801de6f03..060fd1c095 100644 --- a/src/autoskillit/execution/headless/__init__.py +++ b/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( diff --git a/src/autoskillit/recipe/_analysis_bfs.py b/src/autoskillit/recipe/_analysis_bfs.py index 1fe7e45d92..32af4bc976 100644 --- a/src/autoskillit/recipe/_analysis_bfs.py +++ b/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( diff --git a/src/autoskillit/recipe/rules/graph/rules_graph_review.py b/src/autoskillit/recipe/rules/graph/rules_graph_review.py index 8a14279d3a..101194b9d0 100644 --- a/src/autoskillit/recipe/rules/graph/rules_graph_review.py +++ b/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=( diff --git a/src/autoskillit/recipe/schema.py b/src/autoskillit/recipe/schema.py index 85e43a0421..e5d8256f8c 100644 --- a/src/autoskillit/recipe/schema.py +++ b/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: diff --git a/src/autoskillit/recipe/skill_contracts.yaml b/src/autoskillit/recipe/skill_contracts.yaml index 6c462da4d2..d2a656988c 100644 --- a/src/autoskillit/recipe/skill_contracts.yaml +++ b/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 diff --git a/src/autoskillit/recipes/diagrams/implementation-groups.md b/src/autoskillit/recipes/diagrams/implementation-groups.md index 3a25aca985..03666c0d0e 100644 --- a/src/autoskillit/recipes/diagrams/implementation-groups.md +++ b/src/autoskillit/recipes/diagrams/implementation-groups.md @@ -1,4 +1,4 @@ - + ## implementation-groups Group-based implementation with per-group plan/implement/test cycles and PR gates. diff --git a/src/autoskillit/recipes/diagrams/implementation.md b/src/autoskillit/recipes/diagrams/implementation.md index a3c6ba26c1..6e267763f6 100644 --- a/src/autoskillit/recipes/diagrams/implementation.md +++ b/src/autoskillit/recipes/diagrams/implementation.md @@ -1,4 +1,4 @@ - + # implementation diff --git a/src/autoskillit/recipes/diagrams/merge-prs.md b/src/autoskillit/recipes/diagrams/merge-prs.md index 99ee8de0c2..f06e85a934 100644 --- a/src/autoskillit/recipes/diagrams/merge-prs.md +++ b/src/autoskillit/recipes/diagrams/merge-prs.md @@ -1,4 +1,4 @@ - + ## merge-prs Merge multiple PRs into an integration branch with conflict resolution and CI gates. diff --git a/src/autoskillit/recipes/diagrams/remediation.md b/src/autoskillit/recipes/diagrams/remediation.md index 985f7fd04a..d66682d98e 100644 --- a/src/autoskillit/recipes/diagrams/remediation.md +++ b/src/autoskillit/recipes/diagrams/remediation.md @@ -1,4 +1,4 @@ - + ## remediation Investigate, rectify, implement, and merge a bug fix with CI and PR gates. diff --git a/src/autoskillit/recipes/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index d16db40f04..d0ca330c9a 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -1123,14 +1123,14 @@ }, { "when": "${{ result.verdict }} == needs_human", - "route": "check_review_loop" + "route": "check_review_posted" }, { "when": "${{ result.verdict }} == approved", - "route": "check_review_loop" + "route": "check_review_posted" }, { - "route": "check_review_loop" + "route": "check_review_posted" } ], "on_failure": "check_repo_ci_event", @@ -1141,7 +1141,7 @@ "review_loop_count", "review_mode" ], - "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" + "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" }, "resolve_review": { "tool": "run_skill", @@ -1243,9 +1243,36 @@ "branch": "${{ context.merge_target }}", "force": "true" }, - "on_success": "check_review_loop", + "on_success": "check_review_posted", "on_failure": "release_issue_failure", - "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_loop to determine if another review-resolve cycle is needed (bounded by review_max_retries). On push failure, escalates via release_issue_failure.\n" + "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_posted (effect verification gate) before advancing via check_review_loop. On push failure, escalates via release_issue_failure.\n" + }, + "check_review_posted": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_review_posted", + "pr_number": "${{ context.pr_number }}", + "output_dir": "{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}", + "mode": "${{ context.review_mode }}" + }, + "on_result": [ + { + "when": "${{ result.reviews_posted }} == 'false'", + "route": "release_issue_failure" + }, + { + "when": "${{ result.reviews_posted }} == 'true'", + "route": "check_review_loop" + }, + { + "route": "release_issue_failure" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "review_loop_count", + "review_mode" + ] }, "check_review_loop": { "tool": "run_python", diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index 0d27ae3272..535450a179 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -1007,10 +1007,10 @@ steps: - when: "${{ result.verdict }} == approved_with_comments" route: resolve_review - when: "${{ result.verdict }} == needs_human" - route: check_review_loop + route: check_review_posted - when: "${{ result.verdict }} == approved" - route: check_review_loop - - route: check_review_loop + route: check_review_posted + - route: check_review_posted on_failure: check_repo_ci_event on_context_limit: check_repo_ci_event optional: true @@ -1024,9 +1024,10 @@ steps: the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) - → check_review_loop (mandatory waypoint to increment review_loop_count). - On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event - (no review to resolve). Skipped when open_pr=false. + → check_review_posted (effect verification gate) → check_review_loop (mandatory + waypoint to increment review_loop_count). On tool-level failure (MCP error, + gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped + when open_pr=false. resolve_review: tool: run_skill @@ -1105,14 +1106,30 @@ steps: remote_url: "${{ context.remote_url }}" branch: "${{ context.merge_target }}" force: 'true' - on_success: check_review_loop + on_success: check_review_posted on_failure: release_issue_failure note: > Pushes the review-fixed feature branch back to the remote after resolve_review - succeeds. Routes to check_review_loop to determine if another review-resolve - cycle is needed (bounded by review_max_retries). On push failure, escalates - via release_issue_failure. + succeeds. Routes to check_review_posted (effect verification gate) before advancing + via check_review_loop. On push failure, escalates via release_issue_failure. + check_review_posted: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_review_posted + pr_number: "${{ context.pr_number }}" + output_dir: "{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}" + mode: "${{ context.review_mode }}" + on_result: + - when: "${{ result.reviews_posted }} == 'false'" + route: release_issue_failure + - when: "${{ result.reviews_posted }} == 'true'" + route: check_review_loop + - route: release_issue_failure + on_failure: release_issue_failure + optional_context_refs: + - review_loop_count + - review_mode check_review_loop: tool: run_python with: diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 78a08aae52..fc99d13e9c 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -1179,14 +1179,14 @@ }, { "when": "${{ result.verdict }} == needs_human", - "route": "check_review_loop" + "route": "check_review_posted" }, { "when": "${{ result.verdict }} == approved", - "route": "check_review_loop" + "route": "check_review_posted" }, { - "route": "check_review_loop" + "route": "check_review_posted" } ], "on_failure": "check_repo_ci_event", @@ -1197,7 +1197,7 @@ "review_loop_count", "review_mode" ], - "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → enrich_diff_context → resolve_review; all other verdicts (including approved, needs_human) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" + "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → enrich_diff_context → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" }, "enrich_diff_context": { "tool": "run_python", @@ -1316,9 +1316,36 @@ "branch": "${{ context.merge_target }}", "force": "true" }, - "on_success": "check_review_loop", + "on_success": "check_review_posted", "on_failure": "release_issue_failure", - "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_loop to determine if another review-resolve cycle is needed (bounded by review_max_retries). On push failure, escalates via release_issue_failure.\n" + "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_posted (effect verification gate) before advancing via check_review_loop. On push failure, escalates via release_issue_failure.\n" + }, + "check_review_posted": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_review_posted", + "pr_number": "${{ context.pr_number }}", + "output_dir": "{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}", + "mode": "${{ context.review_mode }}" + }, + "on_result": [ + { + "when": "${{ result.reviews_posted }} == 'false'", + "route": "release_issue_failure" + }, + { + "when": "${{ result.reviews_posted }} == 'true'", + "route": "check_review_loop" + }, + { + "route": "release_issue_failure" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "review_loop_count", + "review_mode" + ] }, "check_review_loop": { "tool": "run_python", diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index fafe6819ab..9f9316dd43 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -1074,10 +1074,10 @@ steps: - when: ${{ result.verdict }} == approved_with_comments route: enrich_diff_context - when: ${{ result.verdict }} == needs_human - route: check_review_loop + route: check_review_posted - when: ${{ result.verdict }} == approved - route: check_review_loop - - route: check_review_loop + route: check_review_posted + - route: check_review_posted on_failure: check_repo_ci_event on_context_limit: check_repo_ci_event optional: true @@ -1090,9 +1090,10 @@ steps: the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → enrich_diff_context → resolve_review; all other verdicts (including approved, - needs_human) → check_review_loop (mandatory waypoint to increment review_loop_count). - On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event - (no review to resolve). Skipped when open_pr=false. + needs_human) → check_review_posted (effect verification gate) → check_review_loop + (mandatory waypoint to increment review_loop_count). On tool-level failure + (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). + Skipped when open_pr=false. ' enrich_diff_context: @@ -1191,14 +1192,30 @@ steps: remote_url: ${{ context.remote_url }} branch: ${{ context.merge_target }} force: 'true' - on_success: check_review_loop + on_success: check_review_posted on_failure: release_issue_failure note: 'Pushes the review-fixed feature branch back to the remote after resolve_review - succeeds. Routes to check_review_loop to determine if another review-resolve - cycle is needed (bounded by review_max_retries). On push failure, escalates - via release_issue_failure. + succeeds. Routes to check_review_posted (effect verification gate) before advancing + via check_review_loop. On push failure, escalates via release_issue_failure. ' + check_review_posted: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_review_posted + pr_number: ${{ context.pr_number }} + output_dir: '{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}' + mode: ${{ context.review_mode }} + on_result: + - when: ${{ result.reviews_posted }} == 'false' + route: release_issue_failure + - when: ${{ result.reviews_posted }} == 'true' + route: check_review_loop + - route: release_issue_failure + on_failure: release_issue_failure + optional_context_refs: + - review_loop_count + - review_mode check_review_loop: tool: run_python with: diff --git a/src/autoskillit/recipes/merge-prs.json b/src/autoskillit/recipes/merge-prs.json index aa86d90d80..2f028bce08 100644 --- a/src/autoskillit/recipes/merge-prs.json +++ b/src/autoskillit/recipes/merge-prs.json @@ -1520,19 +1520,43 @@ }, { "when": "${{ result.verdict }} == needs_human", - "route": "derive_batch_ci_event" + "route": "check_review_posted" }, { "when": "${{ result.verdict }} == approved", - "route": "derive_batch_ci_event" + "route": "check_review_posted" }, { "when": "true", - "route": "derive_batch_ci_event" + "route": "check_review_posted" } ], "on_failure": "resolve_review_integration", - "note": "Automated review of the integration PR (REQ-REVIEW-001). Invokes review-pr against context.batch_branch. All four verdicts have explicit routes: changes_requested → resolve_review_integration, approved_with_comments → resolve_review_integration, needs_human → derive_batch_ci_event (human reviews inline), approved → derive_batch_ci_event. On tool-level failure, routes to resolve_review_integration (matches implementation.yaml pattern). needs_human is explicitly routed to satisfy the unrouted-verdict-value semantic rule.\n" + "note": "Automated review of the integration PR (REQ-REVIEW-001). Invokes review-pr against context.batch_branch. All four verdicts have explicit routes: changes_requested → resolve_review_integration, approved_with_comments → resolve_review_integration, needs_human → check_review_posted (effect verification gate) → derive_batch_ci_event, approved → check_review_posted → derive_batch_ci_event. On tool-level failure, routes to resolve_review_integration. needs_human is explicitly routed to satisfy the unrouted-verdict-value semantic rule.\n" + }, + "check_review_posted": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_review_posted", + "pr_number": "${{ context.review_pr_number }}", + "output_dir": "{{AUTOSKILLIT_TEMP}}/review-pr", + "mode": "github", + "work_dir": "${{ context.work_dir }}" + }, + "on_result": [ + { + "when": "${{ result.reviews_posted }} == 'false'", + "route": "register_clone_failure" + }, + { + "when": "${{ result.reviews_posted }} == 'true'", + "route": "derive_batch_ci_event" + }, + { + "route": "register_clone_failure" + } + ], + "on_failure": "register_clone_failure" }, "resolve_review_integration": { "tool": "run_skill", @@ -1629,9 +1653,9 @@ "branch": "${{ context.batch_branch }}", "force": "true" }, - "on_success": "derive_batch_ci_event", + "on_success": "check_review_posted", "on_failure": "register_clone_failure", - "note": "Pushes the review-fixed integration branch to the remote (REQ-REVIEW-003). Routes to derive_batch_ci_event for branch-specific CI event derivation before final CI validation. On push failure, escalates to register_clone_failure.\n" + "note": "Pushes the review-fixed integration branch to the remote (REQ-REVIEW-003). Routes to check_review_posted (the effect-verification gate), which in turn routes to derive_batch_ci_event when reviews_posted == 'true'. On push failure, escalates to register_clone_failure.\n" }, "derive_batch_ci_event": { "tool": "check_repo_merge_state", diff --git a/src/autoskillit/recipes/merge-prs.yaml b/src/autoskillit/recipes/merge-prs.yaml index ee1f1f2f18..62010251ca 100644 --- a/src/autoskillit/recipes/merge-prs.yaml +++ b/src/autoskillit/recipes/merge-prs.yaml @@ -1488,21 +1488,36 @@ steps: - when: ${{ result.verdict }} == approved_with_comments route: resolve_review_integration - when: ${{ result.verdict }} == needs_human - route: derive_batch_ci_event + route: check_review_posted - when: ${{ result.verdict }} == approved - route: derive_batch_ci_event + route: check_review_posted - when: 'true' - route: derive_batch_ci_event + route: check_review_posted on_failure: resolve_review_integration note: 'Automated review of the integration PR (REQ-REVIEW-001). Invokes review-pr against context.batch_branch. All four verdicts have explicit routes: changes_requested → resolve_review_integration, approved_with_comments → resolve_review_integration, - needs_human → derive_batch_ci_event (human reviews inline), approved → derive_batch_ci_event. - On tool-level failure, routes to resolve_review_integration (matches implementation.yaml - pattern). needs_human is explicitly routed to satisfy the unrouted-verdict-value - semantic rule. + needs_human → check_review_posted (effect verification gate) → derive_batch_ci_event, + approved → check_review_posted → derive_batch_ci_event. On tool-level failure, + routes to resolve_review_integration. needs_human is explicitly routed to satisfy + the unrouted-verdict-value semantic rule. ' + check_review_posted: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_review_posted + pr_number: ${{ context.review_pr_number }} + output_dir: '{{AUTOSKILLIT_TEMP}}/review-pr' + mode: github + work_dir: ${{ context.work_dir }} + on_result: + - when: ${{ result.reviews_posted }} == 'false' + route: register_clone_failure + - when: ${{ result.reviews_posted }} == 'true' + route: derive_batch_ci_event + - route: register_clone_failure + on_failure: register_clone_failure resolve_review_integration: tool: run_skill model: '' @@ -1576,11 +1591,12 @@ steps: remote_url: ${{ context.remote_url }} branch: ${{ context.batch_branch }} force: 'true' - on_success: derive_batch_ci_event + on_success: check_review_posted on_failure: register_clone_failure note: 'Pushes the review-fixed integration branch to the remote (REQ-REVIEW-003). - Routes to derive_batch_ci_event for branch-specific CI event derivation before - final CI validation. On push failure, escalates to register_clone_failure. + Routes to check_review_posted (the effect-verification gate), which in turn + routes to derive_batch_ci_event when reviews_posted == ''true''. On push + failure, escalates to register_clone_failure. ' derive_batch_ci_event: diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 4635940ed3..84ebe92006 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -1337,14 +1337,14 @@ }, { "when": "${{ result.verdict }} == needs_human", - "route": "check_review_loop" + "route": "check_review_posted" }, { "when": "${{ result.verdict }} == approved", - "route": "check_review_loop" + "route": "check_review_posted" }, { - "route": "check_review_loop" + "route": "check_review_posted" } ], "on_failure": "check_repo_ci_event", @@ -1355,7 +1355,7 @@ "review_loop_count", "review_mode" ], - "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" + "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false.\n" }, "resolve_review": { "tool": "run_skill", @@ -1457,9 +1457,36 @@ "branch": "${{ context.merge_target }}", "force": "true" }, - "on_success": "check_review_loop", + "on_success": "check_review_posted", "on_failure": "release_issue_failure", - "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_loop to determine if another review-resolve cycle is needed (bounded by review_max_retries). On push failure, escalates via release_issue_failure.\n" + "note": "Pushes the review-fixed feature branch back to the remote after resolve_review succeeds. Routes to check_review_posted (effect verification gate) before advancing via check_review_loop. On push failure, escalates via release_issue_failure.\n" + }, + "check_review_posted": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_review_posted", + "pr_number": "${{ context.pr_number }}", + "output_dir": "{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}", + "mode": "${{ context.review_mode }}" + }, + "on_result": [ + { + "when": "${{ result.reviews_posted }} == 'false'", + "route": "release_issue_failure" + }, + { + "when": "${{ result.reviews_posted }} == 'true'", + "route": "check_review_loop" + }, + { + "route": "release_issue_failure" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "review_loop_count", + "review_mode" + ] }, "check_review_loop": { "tool": "run_python", diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 843e8e0a0e..f587a9238b 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -1202,10 +1202,10 @@ steps: - when: ${{ result.verdict }} == approved_with_comments route: resolve_review - when: ${{ result.verdict }} == needs_human - route: check_review_loop + route: check_review_posted - when: ${{ result.verdict }} == approved - route: check_review_loop - - route: check_review_loop + route: check_review_posted + - route: check_review_posted on_failure: check_repo_ci_event on_context_limit: check_repo_ci_event optional: true @@ -1217,9 +1217,10 @@ steps: headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments - → resolve_review; all other verdicts (including approved, needs_human) → check_review_loop - (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP - error, gh unavailable), routes to check_repo_ci_event (no review to resolve). + → resolve_review; all other verdicts (including approved, needs_human) → + check_review_posted (effect verification gate) → check_review_loop (mandatory + waypoint to increment review_loop_count). On tool-level failure (MCP error, + gh unavailable), routes to check_repo_ci_event (no review to resolve). Skipped when open_pr=false. ' @@ -1301,14 +1302,30 @@ steps: remote_url: ${{ context.remote_url }} branch: ${{ context.merge_target }} force: 'true' - on_success: check_review_loop + on_success: check_review_posted on_failure: release_issue_failure note: 'Pushes the review-fixed feature branch back to the remote after resolve_review - succeeds. Routes to check_review_loop to determine if another review-resolve - cycle is needed (bounded by review_max_retries). On push failure, escalates - via release_issue_failure. + succeeds. Routes to check_review_posted (effect verification gate) before advancing + via check_review_loop. On push failure, escalates via release_issue_failure. ' + check_review_posted: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_review_posted + pr_number: ${{ context.pr_number }} + output_dir: '{{AUTOSKILLIT_TEMP}}/review-pr/iter_${{ context.review_loop_count }}' + mode: ${{ context.review_mode }} + on_result: + - when: ${{ result.reviews_posted }} == 'false' + route: release_issue_failure + - when: ${{ result.reviews_posted }} == 'true' + route: check_review_loop + - route: release_issue_failure + on_failure: release_issue_failure + optional_context_refs: + - review_loop_count + - review_mode check_review_loop: tool: run_python with: diff --git a/src/autoskillit/server/tools/_auto_overrides.py b/src/autoskillit/server/tools/_auto_overrides.py index 42d0b5168c..ecead8050a 100644 --- a/src/autoskillit/server/tools/_auto_overrides.py +++ b/src/autoskillit/server/tools/_auto_overrides.py @@ -15,6 +15,7 @@ from autoskillit.core import ( AGENT_BACKEND_CLAUDE_CODE, CAPABILITY_INGREDIENT_TO_SKIP_GUARD, + SKILL_CAPABILITY_REGISTRY, SKILL_TOOLS, CapabilityResolutionDetail, extract_skill_name, @@ -100,8 +101,10 @@ def _provider_aware_capability_overrides( if not skill_name: continue cap_resolved = skill_resolver.resolve(skill_name) - if cap_resolved and "git_metadata_write" in getattr( - cap_resolved, "uses_capabilities", frozenset() + if cap_resolved and any( + SKILL_CAPABILITY_REGISTRY.get(cap) is not None + and SKILL_CAPABILITY_REGISTRY[cap].worker_routable + for cap in getattr(cap_resolved, "uses_capabilities", frozenset()) ): has_capability_requirement = True break @@ -202,20 +205,21 @@ def _compute_effective_backend_map( For each ``run_skill`` step, returns the backend that ``run_skill`` would dispatch to: ``AGENT_BACKEND_CLAUDE_CODE`` if the step has a provider override with - ``ANTHROPIC_BASE_URL`` OR the step's skill requires ``git_metadata_write``; - otherwise the orchestrator's ``backend_name``. Returns ``None`` when there is - no orchestrator backend — callers treat ``None`` as "no per-step awareness; - fall back to ``ctx.backend_name``". + ``ANTHROPIC_BASE_URL`` OR the step's skill declares a ``worker_routable`` + capability (e.g. ``git_metadata_write``); otherwise the orchestrator's + ``backend_name``. Returns ``None`` when there is no orchestrator backend — + callers treat ``None`` as "no per-step awareness; fall back to + ``ctx.backend_name``". Mirrors the ``ANTHROPIC_BASE_URL`` backend-override block and the - ``_skill_requires_claude`` capability disjunct in ``run_skill()`` - (``tools_execution.py``) so admission and dispatch evaluate backend compatibility - using identical per-step logic. + ``_skill_requires_claude`` / ``_has_routing_capability`` predicate in + ``run_skill()`` (``tools_execution.py``) so admission and dispatch evaluate + backend compatibility using identical per-step logic. The ``skill_resolver`` parameter enables capability-driven routing: when - supplied, steps whose skills have ``git_metadata_write`` in - ``uses_capabilities`` map to ``AGENT_BACKEND_CLAUDE_CODE`` regardless of - provider config (REQ-ADMIT-002). + supplied, steps whose skills declare any ``worker_routable=True`` capability + map to ``AGENT_BACKEND_CLAUDE_CODE`` regardless of provider config + (REQ-ADMIT-002). """ if backend_name is None or recipe_steps is None: return None @@ -251,15 +255,10 @@ def _compute_effective_backend_map( skill_name = extract_skill_name(skill_cmd) if skill_cmd else None if skill_name: resolved = skill_resolver.resolve(skill_name) - _resolved_reqs: frozenset[str] = ( - getattr(resolved, "backend_requirements", frozenset()) - if resolved - else frozenset() - ) - if ( - resolved - and "git_metadata_write" in getattr(resolved, "uses_capabilities", frozenset()) - and _resolved_reqs + if resolved and any( + SKILL_CAPABILITY_REGISTRY.get(cap) is not None + and SKILL_CAPABILITY_REGISTRY[cap].worker_routable + for cap in getattr(resolved, "uses_capabilities", frozenset()) ): result[step_name] = AGENT_BACKEND_CLAUDE_CODE continue diff --git a/src/autoskillit/server/tools/tools_execution.py b/src/autoskillit/server/tools/tools_execution.py index 0afb7ff8c4..d3e5853da6 100644 --- a/src/autoskillit/server/tools/tools_execution.py +++ b/src/autoskillit/server/tools/tools_execution.py @@ -19,6 +19,7 @@ AGENT_BACKEND_CLAUDE_CODE, CODEX_SESSIONS_SUBDIR, DISPATCH_ID_ENV_VAR, + SKILL_CAPABILITY_REGISTRY, SKILL_COMMAND_DISPLAY_MAX, WORKTREE_SKILLS, ClaudeDirectoryConventions, @@ -504,6 +505,35 @@ def _compute_write_prefixes( return base_prefixes[0] if base_prefixes else "", tuple(all_prefixes) +def _aggregate_sandbox_overrides(skill_caps: frozenset[str]) -> frozenset[str]: + """Aggregate required_sandbox_overrides from all declared capabilities.""" + return frozenset().union( + *( + SKILL_CAPABILITY_REGISTRY[cap].required_sandbox_overrides + for cap in skill_caps + if cap in SKILL_CAPABILITY_REGISTRY + ) + ) + + +def _has_routing_capability(skill_caps: frozenset[str]) -> bool: + """Return True if any declared capability is worker_routable (triggers backend reroute).""" + return any( + SKILL_CAPABILITY_REGISTRY.get(cap) is not None + and SKILL_CAPABILITY_REGISTRY[cap].worker_routable + for cap in skill_caps + ) + + +def _get_routing_caps(skill_caps: frozenset[str]) -> list[str]: + """Return sorted list of worker_routable capabilities that trigger backend reroute.""" + return sorted( + cap + for cap in skill_caps + if SKILL_CAPABILITY_REGISTRY.get(cap) and SKILL_CAPABILITY_REGISTRY[cap].worker_routable + ) + + @mcp.tool(tags={"autoskillit", "kitchen", "kitchen-core"}, annotations={"readOnlyHint": True}) @_cancellation_shield() @track_response_size("run_skill") @@ -766,29 +796,27 @@ async def run_skill( and not tool_ctx.backend.capabilities.anthropic_provider_capable ) - _skill_reqs: frozenset[str] = ( - getattr(_skill_info, "backend_requirements", frozenset()) - if _skill_info - else frozenset() - ) _skill_caps: frozenset[str] = ( getattr(_skill_info, "uses_capabilities", frozenset()) if _skill_info else frozenset() ) + _sandbox_overrides = _aggregate_sandbox_overrides(_skill_caps) + _network_access = "sandbox_workspace_write.network_access=true" in _sandbox_overrides + _has_routing_cap = _has_routing_capability(_skill_caps) _skill_requires_claude = bool( - _skill_reqs - and "git_metadata_write" in _skill_caps + _has_routing_cap and tool_ctx.backend is not None and not tool_ctx.backend.capabilities.anthropic_provider_capable ) if _skill_requires_claude: if shutil.which("claude") is None: + _routing_caps = _get_routing_caps(_skill_caps) return SkillResult.crashed( exception=RuntimeError( f"Skill {target_name!r} requires claude-code backend " - f"(git_metadata_write capability) but 'claude' binary " + f"({', '.join(_routing_caps)} capability) but 'claude' binary " f"is not found on PATH. Install Claude Code CLI to " f"enable capability-driven routing." ), @@ -1124,6 +1152,7 @@ async def run_skill( caller_session_id=_orchestrator_sid, inspector_eligible=_in_fleet_dispatch and bool(_inspector_model), inspector_model=_inspector_model, + network_access=_network_access, ) except TimeoutError as exc: logger.error( diff --git a/src/autoskillit/skills/sous-chef/SKILL.md b/src/autoskillit/skills/sous-chef/SKILL.md index b50bfb1b99..520c75aaff 100644 --- a/src/autoskillit/skills/sous-chef/SKILL.md +++ b/src/autoskillit/skills/sous-chef/SKILL.md @@ -1,6 +1,6 @@ --- name: sous-chef -uses_capabilities: [cross_skill_ref, open_kitchen, run_skill, test_check] +uses_capabilities: [cross_skill_ref, github_api_write, open_kitchen, run_skill, test_check] description: Internal bootstrap document injected by open_kitchen into every orchestrator session. ---