Implementation Plan: Fix Codex persistent_root scope mismatch (#4391) - #4463
Merged
Trecek merged 8 commits intoAug 5, 2026
Merged
Conversation
) Root cause: DefaultSessionSkillManager resolved persistent_root once against the global backend at make_context() time, but _initialize_bound_records computed persistence from the step's backend — so any recipe/step-level backend override to a persistent backend (Codex) while the global backend was non-persistent crashed with 'A persistent_root is required for persistent generated-home sessions'. - Add resolve_persistent_session_roots(base_root, backends, required_backend_names=...) resolving a dict[str, Path] over all registered backends. A backend's malformed root convention is skipped unless it's in required_backend_names (the load-bearing global backend), in which case the RuntimeError still propagates at construction time. - DefaultSessionSkillManager now holds persistent_roots: dict[str, Path] keyed by backend name, looked up by the invocation's backend at materialization. The invariant RuntimeError guard is kept verbatim. - Add all_backends() to execution/backends, re-exported from autoskillit.execution, to enumerate every registered backend for the map-building call sites. - Wire the new helper into server/_factory.py (make_context), cli/session/_session_cook.py (cook), and cli/fleet/__init__.py (fleet cleanup) — the fleet path no longer hardcodes CODEX_SESSIONS_SUBDIR. - Refactor tests/conftest.py's tool_ctx fixture into a make_tool_ctx factory fixture so tests can build a ToolContext from a specific AutomationConfig (needed by the upcoming #4391 contract test). - Migrate every persistent_root= call site (tests + fixtures) to persistent_roots={"codex": ...}; tests/workspace/conftest.py gains an explicit _UNSET sentinel so an explicit codex_root=None reaches the constructor as {} instead of silently defaulting to a valid path. - Fix test_persistent_backend_declares_its_own_inert_paths, whose mock backend used an arbitrary name under the old backend-agnostic single root — now load-bearing since root lookup is keyed by backend.name. Part of #4391. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ough regression guard (#4391) Config-time validation (acceptance criterion 1 reject-branch): - _check_dispatch_feasibility gains a keyword-only temp_dir parameter, passed at all three production call sites (open_kitchen x2, dispatch_food_truck). For an explicitly-pinned step whose backend has capabilities.session_dir_persistent=True: fail-closed (persistent_root_unverifiable_for_pinned_step) when temp_dir is absent; fail-closed (persistent_root_unresolvable_for_pinned_step, naming the dotted config key as origin/remedy) when the root cannot be derived. Fires identically for all four pin tiers. - Doctor Check 37 (_check_standing_backend_pins_feasibility) gains the same axis, running before the is_recipe_pin early-continue so global step_overrides pins are covered too — ERROR naming the dotted key. - Fixed the two existing tests these new unconditional checks touch: test_explicit_override_to_missing_binary_excluded now passes temp_dir (T6b); the UnsupportedBackend doctor stub now has a real BackendCapabilities so it stays inert to the new branch (T7b). Routing regression guard (acceptance criterion 2): - New public bfs_reachable_without_barrier_in_graph(graph, start, barrier) in _analysis_bfs.py, delegating to the existing _bfs_capped but letting the caller supply the adjacency (ctx.step_graph, which — unlike the existing success-only helper — includes failure/context-limit/ rate-limit/exhausted edges). - New ERROR rule review-failure-fallthrough-guard: from any review-family step's (review-pr/resolve-review, matched by skill name) failure-shaped edges, no CI-advance gate (check_review_loop, check_repo_ci_event, derive_batch_ci_event) may be reachable without crossing check_review_posted or re-entering a review-family step. Closes the #1684 fall-through pattern (shipped for three months, reversed independently by #4448) against a third re-flip. Tests: T1-T2 direct unit coverage for resolve_persistent_session_root(s) (previously zero); T4 factory-wiring assertion on session_skill_manager._persistent_roots; T5 the #4391 contract test — drives the real, unpatched manager built by make_context() through run_skill() with a recipe-level codex pin while the global backend is claude-code (on pre-fix code this crashes with the persistent_root RuntimeError); T6 preflight persistent-root axis (unresolvable / unverifiable / passes); T7 doctor persistent-root axis (malformed / well-formed / global step_overrides pin); T8 the new rule's registration, silence on all five bundled recipes, and four positive/negative routing shapes. Part of #4391. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Trecek
enabled auto-merge
August 5, 2026 06:05
Trecek
deleted the
impl-issue-4391-codex-persistent-root-20260804-182539
branch
August 5, 2026 06:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #4391: any recipe step routed to a persistent-session backend (Codex) via
agent_backend.recipe_overrides/step_overridescrashes before launch withRuntimeError: A persistent_root is required for persistent generated-home sessionswhenever the global backend (
config.agent_backend.backend, defaultclaude-code) is non-persistent. Root cause is a scope mismatch:make_context()resolves
persistent_rootexactly once against the global backend(
server/_factory.py:341) and bakes it into the kitchen-lifetimeDefaultSessionSkillManager, while_initialize_bound_records(
workspace/session_skills.py:736-742) computespersistentfrom the stepbackend (
projection_context.backend). The persistent root is a pure function of(temp_dir, backend)—resolve_persistent_session_rootderivestemp_dir / backend.conventions.persistent_session_root_subdir— so a singlebaked
Path | Noneis the wrong shape for a manager that serves per-step backends.Closes #4391
Implementation Plan
Plan file:
/home/talon/projects/generic_automation_mcp/.autoskillit/temp/make-plan/issue_4391_codex_persistent_root_scope_fix_plan_2026-08-04_172051.md🤖 Generated with Claude Code via AutoSkillit