Skip to content

Commit 2ae4bc8

Browse files
Trecekclaude
andauthored
Integration: collapsed PRs #275, #276, #274 into main (#279)
* feat: add deletion regression guards for PR merge pipeline Adds two targeted guards that detect when a stale PR reintroduces code deliberately deleted on the base branch: 1. merge-pr/SKILL.md — Step 1.5: Deletion Regression Scan - Runs for ALL PRs before any merge attempt - Uses git merge-base + --diff-filter=D/M to find base-branch deletions - Cross-references PR additions against deleted files and symbols - Forces conflict report path with ## Deletion Regressions section - Emits deletion_regression=true/false output token in Step 5 2. review-pr/SKILL.md — Step 2.5 + 7th audit subagent - Pre-computes deletion context (merge_base, deleted_files, deleted_symbols) - Adds deletion_regression as 7th parallel audit dimension in Step 3 - Subagent flags regressions as severity=critical, requires_decision=false - Routes to verdict=changes_requested for automated remediation 3. tests/skills/test_deletion_regression_guards.py — structural guards - 13 tests asserting both skills have required sections and instructions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(report_bug): include session diagnostics in auto-filed GitHub issues Add _read_session_diagnostics and _format_diagnostics_section helpers to tools_integrations.py. Wire them into _run_report_session (new log_dir param) and _file_or_update_github_issue (new diag param) so that proc traces, anomaly records, and peak metrics from flush_session_log are appended to new issue bodies (full) and duplicate comments (condensed). Diagnostics are gracefully skipped for empty/fallback session IDs or missing directories. Add 17 tests: 7 for _read_session_diagnostics, 6 for _format_diagnostics_section, and 4 integration tests validating the full report_bug flow. * fix: correct test boundary detection edge cases in skill SKILL.md structure - merge-pr Step 1.5: swap 'if regressions found' before 'if no regressions' so 'conflict report' text precedes the first 'Step 2' occurrence that the test uses as a section boundary - review-pr Step 2.5: remove 'Step 3' and deletion_regression (underscore) from the intro so the section boundary includes the full bash code block (--diff-filter=D and MERGE_BASE are now within the detected section) - review-pr Step 3: move deletion_regression out of JSON schema union so its first occurrence is in item 7 (immediately followed by severity/ requires_decision fields within the 500/600-char test window) - All 13 tests in test_deletion_regression_guards.py now pass Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove literal 'Step 3.5' and 'Step 4' refs from Step 1.5 body The test_merge_pr_skill_fetches_all_pr_files test uses find('Step 3.5') as the start and find('Step 4') as the end to scope the section under examination. Mentioning these step numbers in the Step 1.5 prose caused the section boundary to be detected mid-paragraph (within Step 1.5 itself rather than at the actual ### Step 3.5 heading), resulting in a false negative (no 'git diff --name-only' command found in the empty section). Replaced with prose references: 'PR Changes Inventory section' and 'the conflict report' to avoid polluting the section boundary detection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(arch): route resolve_log_dir through server.helpers to satisfy REQ-IMP-003 tools_integrations.py was importing autoskillit.execution.session_log directly, violating the arch rule that tools_*.py may only import from core, pipeline, and server sub-packages. Re-export resolve_log_dir via server.helpers (an intra-server module, arch-compliant as L3→L1) and use the module-level import. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add deletion_regression to review-pr finding schema dimension union The Step 3 JSON schema example in review-pr/SKILL.md had a stale dimension union string missing deletion_regression. Also adds a targeted test that checks the schema union string directly, preventing this gap from recurring (the existing broad test passes because deletion_regression appears in the 7th subagent description but not the schema block). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: Phase 2 ephemeral skill gating and open-kitchen human entry point Implements all 6 deliverables from issue #269 Phase 2: 1. open-kitchen/close-kitchen SKILL.md files with disable-model-invocation: true (human-only slash commands, not agent-invocable) 2. Cook session visibility gap fix: mcp.enable(tags={"kitchen"}) in server/__init__.py when AUTOSKILLIT_KITCHEN_OPEN=1, so cook sessions start with kitchen pre-revealed 3. Platform-aware ephemeral directory resolver: /dev/shm → /tmp → $TMPDIR fallback chain (resolve_ephemeral_root in workspace/session_skills.py) 4. SkillsDirectoryProvider: tier-aware skill content provider with frontmatter injection for Tier 2 skills (open-kitchen, close-kitchen get disable-model-invocation: true in plain sessions, removed in cook sessions) 5. SessionSkillManager: manages per-session ephemeral skill directories, cleans stale dirs at startup, activate_tier2() removes gate flag on demand 6. Hook enforcement: open_kitchen_guard.py PreToolUse hook blocks open_kitchen from headless sessions (AUTOSKILLIT_HEADLESS=1 injected by headless.py) Also adds SkillSessionManager Protocol to core/types.py, wires SessionSkillManager into ToolContext and _factory.py, registers new hook in hook_registry.py, and adds 16 Phase 2 tests (T1-T16) covering all new components. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: expand requires_decision window in test to cover schema union offset * chore: commit auto-generated files * fix(review): address 8 PR review findings from #274 - helpers.py: declare resolve_log_dir in __all__ instead of noqa: F401 backward-compat hack - tools_integrations.py: guard config.linux_tracing.log_dir against AttributeError - tools_integrations.py: add exc_info=True to diagnostics read failure warning - tools_integrations.py: validate session_id against safe char set before path construction - tools_integrations.py: use .get() for anomalies/proc_trace_tail in _format_diagnostics_section - tests: replace fragile call_args[0][3] positional access with kwargs.get("body", args[3]) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): quote PR_FILES via --pathspecs-from-file and increase grep context to -B20 - Use `echo "$PR_FILES" | git diff --pathspecs-from-file=-` so space-containing paths are not word-split by bash - Increase grep context from -B5 to -B20 when extracting commit SHA from git log -p output, since diff headers appear more than 5 lines above the matched symbol line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): fix test boundary detection and anchor deletion_regression search - Replace find("Step 2") with find("### Step 2") in three tests to prevent premature truncation of step_15_section when "Step 2" appears in inline prose ("Step 2 / Step 3 as normal") before the actual section header - Anchor deletion_regression severity test to "deletion_regression — Deliberate" paragraph instead of first occurrence (which falls in the JSON schema union block), ensuring the test validates the 7th subagent instruction rather than a template value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): correct deletion_regression anchor to match markdown bold syntax The SKILL.md uses **deletion_regression** (markdown bold), so the search string must be '7. **deletion_regression**' not 'deletion_regression — Deliberate' (which doesn't appear verbatim in the raw file). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): replace import os as _os alias with plain import os Unnecessary alias — os is not imported elsewhere in the module and there is no name collision with other names. * fix(review): rename SkillSessionManager Protocol to SessionSkillManager Unifies naming with the concrete implementation class in workspace/session_skills.py, which was already named SessionSkillManager. * fix(review): validate session_id and skill_name against path traversal init_session and activate_tier2 now reject any session_id or skill_name containing path separators or dot-dot sequences before constructing paths under the ephemeral root. * fix(review): replace importlib.reload + vacuous assertion with behavioral test Uses FastMCP Client to assert GATED_TOOLS are visible after mcp.enable(tags={'kitchen'}). Eliminates module reload that mutated global state and could bleed between xdist workers. * fix(review): replace inspect.getsource() with behavioral test for AUTOSKILLIT_HEADLESS Calls run_headless_core with a MockSubprocessRunner and asserts AUTOSKILLIT_HEADLESS=1 appears in the spawned command, replacing a source-text grep that passed even if the constant was in dead code. * fix(review): strengthen human-session hook assertion and fix pre-existing E501 test_open_kitchen_guard_allows_human_session now asserts returncode==0 and empty stdout instead of accepting any non-deny response including silent crashes. Fix pre-existing E501 line-length violations in test_tools_execution.py that were blocking pre-commit. * fix(review): rename SessionSkillManager to DefaultSessionSkillManager Eliminates import ambiguity with the identically-named Protocol in core/types.py. Updates workspace/__init__.py export, _factory.py construction, and test imports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): add path-traversal test for _SAFE_SESSION_ID_RE and split fallback tests Adds a test asserting None is returned for path-traversal session_id inputs. Splits the combined no_session_*/crashed_* test into two focused tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): harden session_skills.py frontmatter handling and session_id validation - Fix _FM_PATTERN to match empty frontmatter (---\n---) by making the newline before closing --- optional - Fix _remove_disable_model_invocation to return bare body when disable-model-invocation was the only key, avoiding degenerate ---\n\n--- - Add null byte guard to session_id validation in init_session and activate_tier2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): remove resolve_log_dir re-export from helpers.py resolve_log_dir is an L1 execution symbol; re-exporting it from L3 server/helpers.py was an arch violation. tools_integrations.py now imports it directly from autoskillit.execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): strengthen open_kitchen_guard, fix __init__.py comment, remove redundant getattr - open_kitchen_guard.py: include OSError in exception catch so broken pipe on stdin.read() fails open instead of propagating uncaught - server/__init__.py: move 'Must appear after all tool module imports' comment to just above mcp.disable() where it is factually correct - tools_integrations.py: access config.linux_tracing.log_dir directly after the is not None guard; getattr fallback was redundant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): remove __all__ re-export of resolve_log_dir from helpers.py Retains the import in helpers.py (where tools_integrations.py fetches it within the allowed autoskillit.server namespace per REQ-IMP-003) but drops the explicit __all__ declaration that advertised it as a public re-export. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5355a91 commit 2ae4bc8

28 files changed

Lines changed: 1698 additions & 42 deletions

CLAUDE.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,10 @@ src/autoskillit/
134134
│ ├── session.py # ClaudeSessionResult, SkillResult, extract_token_usage
135135
│ └── testing.py # Pytest output parsing and pass/fail adjudication
136136
├── workspace/ # L1 workspace sub-package
137-
│ ├── __init__.py # Re-exports CleanupResult, SkillResolver, clone_repo, remove_clone, push_to_remote
137+
│ ├── __init__.py # Re-exports CleanupResult, SkillResolver, SessionSkillManager, clone_repo, remove_clone, push_to_remote
138138
│ ├── cleanup.py # Directory teardown utilities (CleanupResult, preserve list)
139139
│ ├── clone.py # Clone-based run isolation: clone_repo, remove_clone, push_to_remote
140+
│ ├── session_skills.py # Per-session ephemeral skill dirs (SkillsDirectoryProvider, SessionSkillManager, TIER2_SKILLS, resolve_ephemeral_root)
140141
│ └── skills.py # Bundled skill listing (SkillResolver)
141142
├── recipe/ # L2 recipe sub-package
142143
│ ├── __init__.py # Re-exports Recipe, RecipeStep, validate_recipe, load_recipe, etc.
@@ -196,7 +197,8 @@ src/autoskillit/
196197
│ ├── quota_check.py # Quota guard hook — blocks run_skill when threshold exceeded
197198
│ ├── remove_clone_guard.py # Remove-clone guard — denies remove_clone calls with keep != "true"
198199
│ ├── skill_cmd_check.py # PreToolUse hook — validates skill_command path argument format
199-
│ └── skill_command_guard.py # PreToolUse hook — blocks run_skill with non-slash skill_command
200+
│ ├── skill_command_guard.py # PreToolUse hook — blocks run_skill with non-slash skill_command
201+
│ └── open_kitchen_guard.py # PreToolUse hook — blocks open_kitchen from headless sessions
200202
├── migrations/ # Data: versioned migration YAML notes
201203
│ └── __init__.py
202204
├── recipes/ # Bundled recipe YAML definitions

src/autoskillit/core/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
RestartScope,
5959
RetryReason,
6060
SessionOutcome,
61+
SessionSkillManager,
6162
Severity,
6263
SkillResult,
6364
SkillSource,
@@ -124,6 +125,7 @@
124125
"SessionOutcome",
125126
"Severity",
126127
"SkillResult",
128+
"SessionSkillManager",
127129
"SkillSource",
128130
"SubprocessResult",
129131
"SubprocessRunner",

src/autoskillit/core/types.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,3 +786,14 @@ async def status(
786786
run_id: int | None = None,
787787
cwd: str = "",
788788
) -> dict[str, Any]: ...
789+
790+
791+
@runtime_checkable
792+
class SessionSkillManager(Protocol):
793+
"""Protocol for managing per-session ephemeral skill directories."""
794+
795+
def init_session(self, session_id: str, *, cook_session: bool = False) -> Path: ...
796+
797+
def activate_tier2(self, session_id: str, skill_name: str) -> bool: ...
798+
799+
def cleanup_stale(self, max_age_seconds: int = 86400) -> int: ...

src/autoskillit/execution/headless.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,9 +403,11 @@ async def run_headless_core(
403403
if add_dir:
404404
cmd.extend([ClaudeFlags.ADD_DIR, add_dir])
405405

406+
env_vars = ["AUTOSKILLIT_HEADLESS=1"]
406407
delay_ms = cfg.exit_after_stop_delay_ms
407408
if delay_ms > 0:
408-
cmd = ["env", f"CLAUDE_CODE_EXIT_AFTER_STOP_DELAY={delay_ms}"] + cmd
409+
env_vars.append(f"CLAUDE_CODE_EXIT_AFTER_STOP_DELAY={delay_ms}")
410+
cmd = ["env"] + env_vars + cmd
409411

410412
effective_timeout = timeout if timeout is not None else cfg.timeout
411413
effective_stale = stale_threshold if stale_threshold is not None else cfg.stale_threshold

src/autoskillit/hook_registry.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ class HookDef:
2828
matcher="mcp__.*autoskillit.*__remove_clone",
2929
scripts=["remove_clone_guard.py"],
3030
),
31+
HookDef(
32+
matcher=r"mcp__.*autoskillit.*__open_kitchen.*",
33+
scripts=["open_kitchen_guard.py"],
34+
),
3135
]
3236

3337

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#!/usr/bin/env python3
2+
"""PreToolUse hook — blocks open_kitchen from headless sessions.
3+
4+
Headless sessions launched by run_skill have AUTOSKILLIT_HEADLESS=1 in their
5+
environment. This hook denies open_kitchen calls from those sessions, enforcing
6+
that only humans (via /autoskillit:open-kitchen) can open the kitchen.
7+
"""
8+
9+
import json
10+
import os
11+
import sys
12+
13+
14+
def main() -> None:
15+
try:
16+
json.loads(sys.stdin.read())
17+
except (json.JSONDecodeError, ValueError, OSError):
18+
sys.exit(0) # fail-open on malformed input or broken pipe
19+
20+
if os.environ.get("AUTOSKILLIT_HEADLESS") == "1":
21+
payload = json.dumps(
22+
{
23+
"hookSpecificOutput": {
24+
"hookEventName": "PreToolUse",
25+
"permissionDecision": "deny",
26+
"permissionDecisionReason": (
27+
"open_kitchen cannot be called from headless sessions. "
28+
"Open the kitchen in your human session using /autoskillit:open-kitchen."
29+
),
30+
}
31+
}
32+
)
33+
sys.stdout.write(payload + "\n")
34+
sys.exit(0)
35+
36+
37+
if __name__ == "__main__":
38+
main()

src/autoskillit/pipeline/context.py

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
MigrationService,
2222
OutputPatternResolver,
2323
RecipeRepository,
24+
SessionSkillManager,
2425
SubprocessRunner,
2526
TestRunner,
2627
TimingStore,
@@ -39,23 +40,24 @@ class ToolContext:
3940
4041
Fields
4142
------
42-
config: AutomationConfig loaded from .autoskillit/config.yaml
43-
audit: AuditStore — records pipeline failures
44-
token_log: TokenStore — per-step token tracking
45-
timing_log: TimingStore — per-step wall-clock duration tracking
46-
gate: GatePolicy — enables/disables gated tools
47-
plugin_dir: Absolute path string to the autoskillit package directory
48-
runner: SubprocessRunner implementation (DefaultSubprocessRunner in production,
49-
MockSubprocessRunner in tests)
50-
executor: HeadlessExecutor — runs headless Claude Code sessions
51-
tester: TestRunner — runs the project test suite
52-
recipes: RecipeRepository — loads and lists pipeline recipes
53-
migrations: MigrationService — applies versioned migration notes to recipes
54-
db_reader: DatabaseReader — executes read-only SQLite queries
55-
workspace_mgr: WorkspaceManager — manages workspace directory teardown
56-
clone_mgr: CloneManager — clone-based pipeline run isolation
57-
github_client: GitHubFetcher — fetches GitHub issue content
58-
ci_watcher: CIWatcher — watches GitHub Actions CI runs
43+
config: AutomationConfig loaded from .autoskillit/config.yaml
44+
audit: AuditStore — records pipeline failures
45+
token_log: TokenStore — per-step token tracking
46+
timing_log: TimingStore — per-step wall-clock duration tracking
47+
gate: GatePolicy — enables/disables gated tools
48+
plugin_dir: Absolute path string to the autoskillit package directory
49+
runner: SubprocessRunner implementation (DefaultSubprocessRunner in production,
50+
MockSubprocessRunner in tests)
51+
executor: HeadlessExecutor — runs headless Claude Code sessions
52+
tester: TestRunner — runs the project test suite
53+
recipes: RecipeRepository — loads and lists pipeline recipes
54+
migrations: MigrationService — applies versioned migration notes to recipes
55+
db_reader: DatabaseReader — executes read-only SQLite queries
56+
workspace_mgr: WorkspaceManager — manages workspace directory teardown
57+
clone_mgr: CloneManager — clone-based pipeline run isolation
58+
github_client: GitHubFetcher — fetches GitHub issue content
59+
ci_watcher: CIWatcher — watches GitHub Actions CI runs
60+
session_skill_manager: SessionSkillManager — manages per-session ephemeral skill dirs
5961
"""
6062

6163
config: AutomationConfig
@@ -75,3 +77,4 @@ class ToolContext:
7577
github_client: GitHubFetcher | None = field(default=None)
7678
ci_watcher: CIWatcher | None = field(default=None)
7779
output_pattern_resolver: OutputPatternResolver | None = field(default=None)
80+
session_skill_manager: SessionSkillManager | None = field(default=None)

src/autoskillit/server/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
# Import all tool sub-modules to trigger @mcp.tool() registration.
4747
# These imports must come AFTER mcp, _get_ctx, _get_config are defined
4848
# because tool modules import `mcp` from this package at import time.
49+
import os # noqa: E402
50+
4951
from autoskillit.core import PIPELINE_FORBIDDEN_TOOLS # noqa: E402, F401
5052
from autoskillit.server import ( # noqa: E402, F401
5153
helpers,
@@ -64,3 +66,8 @@
6466
# Apply global visibility transform: all sessions start with kitchen tools hidden.
6567
# Must appear after all tool module imports so the registered tools are in place.
6668
mcp.disable(tags={"kitchen"})
69+
70+
# Cook sessions (AUTOSKILLIT_KITCHEN_OPEN=1) pre-reveal kitchen tools at module load
71+
# so the session starts with all 36 tools visible without calling open_kitchen.
72+
if os.environ.get("AUTOSKILLIT_KITCHEN_OPEN") == "1":
73+
mcp.enable(tags={"kitchen"})

src/autoskillit/server/_factory.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@
3535
load_bundled_manifest,
3636
resolve_skill_name,
3737
)
38-
from autoskillit.workspace import DefaultCloneManager, DefaultWorkspaceManager
38+
from autoskillit.workspace import (
39+
DefaultCloneManager,
40+
DefaultSessionSkillManager,
41+
DefaultWorkspaceManager,
42+
SkillsDirectoryProvider,
43+
resolve_ephemeral_root,
44+
)
3945

4046
# Sentinel: distinguish "caller passed runner=None explicitly" from "not provided"
4147
_UNSET: Any = object()
@@ -87,6 +93,11 @@ def make_context(
8793
gate = DefaultGateState(enabled=False)
8894
if os.environ.get("AUTOSKILLIT_KITCHEN_OPEN") == "1":
8995
gate.enable()
96+
97+
provider = SkillsDirectoryProvider()
98+
ephemeral_root = resolve_ephemeral_root()
99+
session_mgr = DefaultSessionSkillManager(provider, ephemeral_root)
100+
90101
ctx = ToolContext(
91102
config=config,
92103
audit=DefaultAuditLog(),
@@ -102,6 +113,7 @@ def make_context(
102113
clone_mgr=DefaultCloneManager(),
103114
github_client=DefaultGitHubFetcher(token=github_token),
104115
ci_watcher=DefaultCIWatcher(token=github_token),
116+
session_skill_manager=session_mgr,
105117
)
106118

107119
def _resolve_output_patterns(skill_command: str) -> list[str]:

src/autoskillit/server/_state.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ def _initialize(ctx: ToolContext) -> None:
7070
except Exception:
7171
logger.warning("telemetry_recovery_at_startup_failed", exc_info=True)
7272

73+
# Session skill cleanup: remove ephemeral skill dirs from previous server runs.
74+
if ctx.session_skill_manager is not None:
75+
try:
76+
removed = ctx.session_skill_manager.cleanup_stale()
77+
if removed:
78+
logger.info("session_skill_cleanup", extra={"removed": removed})
79+
except Exception:
80+
logger.warning("session_skill_cleanup_failed", exc_info=True)
81+
7382

7483
def _get_ctx() -> ToolContext:
7584
"""Return the active ToolContext. Raises if _initialize() has not been called."""

0 commit comments

Comments
 (0)