Skip to content

Implementation Plan: T5-P6-A9-WP1 Close the fail-open path where init_session() returns a reclaimed /dev/shm path#4129

Merged
Trecek merged 6 commits into
developfrom
t5-p6-a9-wp1-close-the-fail-open-path-where-init-session-ret/4058
Jun 27, 2026
Merged

Implementation Plan: T5-P6-A9-WP1 Close the fail-open path where init_session() returns a reclaimed /dev/shm path#4129
Trecek merged 6 commits into
developfrom
t5-p6-a9-wp1-close-the-fail-open-path-where-init-session-ret/4058

Conversation

@Trecek

@Trecek Trecek commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Close the fail-open path in run_skill where init_session() returns a ValidatedAddDir pointing to a /dev/shm path that no longer exists (reclaimed by the OS or cleaned up by cleanup_stale()). Add an is_dir() guard after init_session() and before skill_add_dirs.append(session_root) that crash-closes immediately — logging a structured warning and returning SkillResult.crashed().to_json() — so the dead path never reaches the executor.

Additionally: add a parallel guard on the replay-snapshot branch (defense-in-depth), add validate_session_exists() to DefaultSessionSkillManager and its Protocol, and add structured logging to cleanup_stale().

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-20260627-120424-389616/.autoskillit/temp/make-plan/T5-P6-A9-WP1_plan_2026-06-27_121500.md

Closes #4058

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step Model count uncached output cache_read peak_ctx turns cache_write time
plan* opus[1m] 1 756 15.7k 1.1M 91.7k 45 96.5k 9m 27s
verify* sonnet 1 1.3k 10.7k 685.4k 55.8k 41 56.0k 7m 17s
implement* MiniMax-M3 1 86.7k 16.2k 4.0M 0 141 0 5m 34s
fix* sonnet 1 182 9.9k 1.3M 81.3k 59 62.8k 6m 36s
audit_impl* sonnet 1 44 10.4k 165.5k 44.1k 14 36.7k 5m 33s
prepare_pr* MiniMax-M3 1 42.8k 2.4k 187.4k 0 13 0 32s
compose_pr* MiniMax-M3 1 36.3k 1.7k 177.7k 0 12 0 35s
review_pr* sonnet 1 158 41.5k 1.2M 94.8k 46 77.2k 10m 41s
resolve_review* sonnet 1 207 15.4k 1.4M 81.0k 66 63.0k 9m 50s
Total 168.4k 123.9k 10.4M 94.8k 392.2k 56m 9s

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

Token Efficiency

Step LoC Changed cache_read/LoC cache_write/LoC output/LoC
plan 0
verify 0
implement 181 22071.2 0.0 89.6
fix 21 63991.2 2989.3 470.1
audit_impl 0
prepare_pr 0
compose_pr 0
review_pr 0
resolve_review 3 481018.0 21014.7 5130.0
Total 205 50576.4 1913.0 604.4

Model Usage Breakdown

Model steps uncached output cache_read cache_write time
opus[1m] 1 756 15.7k 1.1M 96.5k 9m 27s
sonnet 5 1.9k 87.9k 4.9M 295.7k 40m 0s
MiniMax-M3 3 165.7k 20.3k 4.4M 0 6m 42s

Trecek and others added 6 commits June 27, 2026 12:26
…_stale logging

Add validate_session_exists() to SessionSkillManager Protocol and
DefaultSessionSkillManager as a reusable check for live session dirs.

Also emit a structured 'cleanup_stale_removed' log event in cleanup_stale()
before shutil.rmtree so stale-dir removals are observable.
… /dev/shm path

Insert is_dir() guards on both injection points in run_skill where a
ValidatedAddDir over an ephemeral /dev/shm root is consumed:

- After init_session() (line ~922) — covers the fresh-session branch
- After restore_skill_snapshot() (line ~886) — covers the replay branch

When the directory does not exist (reclaimed by OS or removed by
cleanup_stale), return SkillResult.crashed() with subtype=crashed instead
of flowing the dead path into the executor.
Add four tests for DefaultSessionSkillManager.validate_session_exists()
(true for live, false after cleanup, false for unknown) and the new
cleanup_stale_removed log event.

Add two tests for run_skill: a stale-path (nonexistent /dev/shm dir)
must crash-close with subtype=crashed and never reach the executor;
a valid on-disk path must proceed to the executor.
- Use tmp_path (real directory) in test_run_skill_calls_session_skill_manager_init_session
  and test_run_skill_activates_deps_for_tier3_target so the new is_dir() guard passes
  instead of crash-closing before the executor
- Bump tools_execution.py line limit from 1150→1200 in _LINE_LIMIT_EXEMPTIONS with
  rationale for the two stale-path guard blocks (+26 net lines)
The run_skill guard that fires when init_session() returns a path
not found on disk was using Path(session_root.path).is_dir() directly
instead of the SessionSkillManager.validate_session_exists() method
introduced in ec82d7f for exactly this purpose. Align the guard with
the Protocol contract.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n_exists

The guard now calls validate_session_exists() instead of Path.is_dir(),
so the MagicMock must explicitly return False for the stale-path scenario
instead of relying on the filesystem check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Trecek Trecek added this pull request to the merge queue Jun 27, 2026
Merged via the queue into develop with commit c088079 Jun 27, 2026
3 checks passed
@Trecek Trecek deleted the t5-p6-a9-wp1-close-the-fail-open-path-where-init-session-ret/4058 branch June 27, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant