Skip to content

Implementation Plan: Fix Codex persistent_root scope mismatch (#4391) - #4463

Merged
Trecek merged 8 commits into
developfrom
impl-issue-4391-codex-persistent-root-20260804-182539
Aug 5, 2026
Merged

Implementation Plan: Fix Codex persistent_root scope mismatch (#4391)#4463
Trecek merged 8 commits into
developfrom
impl-issue-4391-codex-persistent-root-20260804-182539

Conversation

@Trecek

@Trecek Trecek commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Issue #4391: any recipe step routed to a persistent-session backend (Codex) via
agent_backend.recipe_overrides/step_overrides crashes before launch with
RuntimeError: A persistent_root is required for persistent generated-home sessions
whenever the global backend (config.agent_backend.backend, default
claude-code) is non-persistent. Root cause is a scope mismatch: make_context()
resolves persistent_root exactly once against the global backend
(server/_factory.py:341) and bakes it into the kitchen-lifetime
DefaultSessionSkillManager, while _initialize_bound_records
(workspace/session_skills.py:736-742) computes persistent from the step
backend (projection_context.backend). The persistent root is a pure function of
(temp_dir, backend)resolve_persistent_session_root derives
temp_dir / backend.conventions.persistent_session_root_subdir — so a single
baked Path | None is 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

Trecek and others added 8 commits August 4, 2026 18:39
)

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
Trecek added this pull request to the merge queue Aug 5, 2026
@Trecek
Trecek removed this pull request from the merge queue due to a manual request Aug 5, 2026
@Trecek
Trecek enabled auto-merge August 5, 2026 06:05
@Trecek
Trecek added this pull request to the merge queue Aug 5, 2026
Merged via the queue into develop with commit d3913f4 Aug 5, 2026
3 checks passed
@Trecek
Trecek deleted the impl-issue-4391-codex-persistent-root-20260804-182539 branch August 5, 2026 06:19
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