feat(#3278): persist Claude session per (pipeline,slice,role) across event pods

[jwbron]

What

Make BRC warm resume (#3200) actually fire by persisting the Claude Code session per (pipeline, slice, role) across the one-shot event pods — the durable copy is orchestrator-owned and reached only over the controlled API, so the sandbox never writes host state.

Stacked on #3284 (#3279, the reseed-threshold injection); the two together complete the #3276 substrate.

Why the pointer alone wasn't enough

claude --resume <session_id> re-enters a session by reading its local transcript ($CLAUDE_CONFIG_DIR/projects/<cwd-slug>/<session_id>.jsonl). Under the orchestrator-owned event loop (#3164) every BRC event is a fresh pod, so that transcript dies with the pod. Persisting only the session_id+occupancy pointer (the original #3276 scope) would let the gate record below_threshold "resume" decisions while every event silently cold-starts. The transcript itself has to cross pods.

Spike (against the installed Claude Code build) confirmed: CLAUDE_CONFIG_DIR relocates the whole session store, and the project slug is re.sub(r'[^a-zA-Z0-9]', '-', abspath(cwd)) (e.g. /home/egg/repos/My_Repo.v2 → -home-egg-repos-My-Repo-v2).

Design (constraint: sandbox never writes host state)

• Live store stays ephemeral — the agent's $CLAUDE_CONFIG_DIR is the pod's own filesystem (no host mount); Claude Code writes the transcript locally during the run, unchanged.
• Durable copy is orchestrator-owned in Redis — session_state_store.py keeps one TTL'd key per (pipeline, slice, role) holding the pointer and transcript together (no split-brain; TTL reaps abandoned state). Only the orchestrator writes it.
• Round-trip over the controlled API — egg-orch session-state pull (before the agent) re-materialises the prior transcript + pointer into the pod so --resume finds a real session; egg-orch session-state push (after) ships the updated session back. Both best-effort and timeout-bounded; a failed sync degrades to a safe cold reseed.

The existing egg_agent substrate (session.py / reseed.py / __main__.py) is untouched — it round-trips the pointer via $EGG_SESSION_STATE_FILE exactly as before; this PR adds the transcript layer around it.

Changes

• orchestrator/session_state_store.py — Redis store (TTL, size-guarded, best-effort).
• orchestrator/routes/session_state.py — GET/POST /api/v1/pipelines/<pid>/session-state (registered in api.py).
• sandbox/egg_lib/session_state_sync.py — slug + transcript/pointer file I/O (pure, unit-tested).
• sandbox/egg_lib/cli_session_state.py — egg-orch session-state pull|push (best-effort).
• concurrent_executor.py — inject CLAUDE_CONFIG_DIR + EGG_SESSION_STATE_FILE into event pods, gated on warm resume being enabled (default pods byte-identical).
• consensus_wrapper.py — pull before / push after the agent invocation, preserving the agent's exit code; golden snapshot regenerated.
• docs/architecture/context-discipline.md — documents the cross-pod persistence layer.

Tests

New: store (TTL/scoping/defensive), route (round-trip/validation), sync logic (slug/pull/push/round-trip), CLI (best-effort + orchestrator round-trip), env-injection gating. Wrapper golden updated and re-verified. make test green (the 2 test_reap_stale_egg_images failures are pre-existing btrfs-host env noise, #3222).

Verification still owed in-cluster

--resume end-to-end across two real event pods (the spike confirmed the file mechanics + relocation locally; the resume call itself needs the in-cluster gateway/API-key auth). Also worth measuring transcript transfer cost per resumed event (a refinement: skip the round-trip on the reseed path).

Related

• Wire the per-spawn warm-resume substrate (session-state persistence + per-model reseed threshold) so #3200 threshold-reseed fires #3276 (closed) — decomposed into this + Compute + inject EGG_RESEED_THRESHOLD per event spawn (feed the #3200 threshold-reseed gate) #3279.
• feat(#3279): compute + inject EGG_RESEED_THRESHOLD per event spawn #3284 / Compute + inject EGG_RESEED_THRESHOLD per event spawn (feed the #3200 threshold-reseed gate) #3279 — the per-model reseed threshold (the gate's other input); this is stacked on it.
• BRC context discipline: protected directive root + queryable environment, bounded by a deterministic threshold reseed #3200 / Measure + monitor + tune the BRC context-discipline knobs (route-aware reseed cap/threshold) — consumes #3200 measurement surfaces #3249 / feat(#3249): emit-only per-event context-discipline measurement surfaces #3271 — the gate + measurement that verify resume fires.
• Flip event-loop ownership to the orchestrator and retire the in-pod wait arm (gated on a live BRC proving run of #3229) #3164 / #3005 — why state must cross pods / the emptyDir-wipe hazard.

Closes #3278

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with egg/3279-reseed-threshold-injection (merge commit, not rebase — history preserved).

File	Category	Resolution
orchestrator/tests/test_concurrent_executor.py	Additive	Kept both sides

Detail: Both branches added new tests at the same location:

• Base (Compute + inject EGG_RESEED_THRESHOLD per event spawn (feed the #3200 threshold-reseed gate) #3279) added test_event_spawn_threshold_conservative_for_unregistered_litellm_model as a method of the existing TestEventSpawnReseedThreshold class.
• This PR (Persist the Claude Code session store per (pipeline,slice,role) so #3200 warm resume survives one-shot event pods #3278) added the new top-level TestEventSpawnSessionStoreEnv class.

These are non-overlapping additions. Resolution keeps the base's new method inside TestEventSpawnReseedThreshold, immediately followed by this PR's new TestEventSpawnSessionStoreEnv class. orchestrator/concurrent_executor.py auto-merged cleanly (no manual edits).

Verification:

• ruff check / ruff format --check: clean
• Targeted run of both affected test classes: 6 passed (TestEventSpawnReseedThreshold, TestEventSpawnSessionStoreEnv)
• make build (Docker image build) and full make test were not run locally — Docker is unavailable in this sandbox and the full suite is too long for the workflow window; CI will run both.

Please review: make lint (mypy) reports pre-existing errors in shared/egg_agent/client.py, sandbox/tests/test_session_state_sync.py, and sandbox/tests/test_cli_session_state.py. These files were not touched by the merge (byte-identical to the pre-merge PR head ef230bf), so the errors predate this resolution — flagging them for the author, not introduced here.

— Authored by egg

[james-in-a-box]

Autofix tracking

{"Lint/Python": 1}

[egg-reviewer]

No agent-mode design concerns. Approving on design alignment.

This is cross-pod session-persistence infrastructure, and it embodies the agent-mode principles rather than fighting them:

• Sandbox-as-constraint (core principle): the durable copy is orchestrator-owned in Redis and reached only over the controlled /session-state route — the sandbox never writes host state. This is the right way to add a capability: extend the controlled surface, not the prompt.
• Not pre-fetching: the transcript that crosses pods is the agent's own accumulated session (claude --resume input), persisted so continuity survives one-shot pod death. That preserves the agent's context rather than a third party deciding what's relevant and baking it into a prompt — the opposite of the pre-fetch anti-pattern, and consistent with context-discipline.
• No post-processing pipeline: pull/push is session plumbing the agent cannot do itself (no host write access by design), not a script parsing agent output to take actions the agent could take directly.
• Agent SDK preserved: invocation stays python3 -m egg_agent with the opus alias; no direct Anthropic API calls, no pinned model IDs, default pod path byte-identical.

— Authored by egg

[egg-reviewer]

Review — #3286: persist Claude session per (pipeline, slice, role) across event pods

Thorough pass over all 15 files, tracing the cross-pod round-trip end-to-end and the cross-module contracts it depends on. No blocking issues. The design is sound, the default path is genuinely byte-identical (gated off), and error handling is best-effort throughout. A few non-blocking suggestions below.

What I verified (load-bearing assumptions that hold)

• except A, B: / except A, B, C: are correct, not Python-2 bind syntax. session_state_store.py:188 (except ValueError, TypeError:) and session_state_sync.py:142 (except OSError, ValueError, TypeError:) initially read like the removed Py2 except E, name: form. They are valid PEP 758 unparenthesized except groups: requires-python = ">=3.14", CI/Dockerfiles pin 3.14, and the sandbox sets update-alternatives python3 → 3.14. Confirmed by compile + runtime that all listed types are caught, and the form is already an established convention here (env_config.py, agent_salvage.py, redis_message_store.py, …). Not a bug.
• The slug matches Claude Code's project dir on both sides. session_state_sync.resolve_repo_path defaults to EGG_REPO_PATH, and the agent's SDK cwd resolves to the same (shared/egg_agent/client.py:284: cwd if cwd else EGG_REPO_PATH). So <config_dir>/projects/<slug>/<sid>.jsonl written by pull is where --resume reads. (The in-cluster --resume call itself is still unverified, as the PR discloses — but the slug math is consistent across the changeset, so this is not the cross-module dead-end the feature exists to prevent.)
• Pointer format matches the consumer. write_pulled_state writes {"session_id", "window_occupancy"}, exactly what egg_agent.session.read_session_state reads (and write_session_state produces), so push/pull round-trip cleanly through the untouched substrate.
• Agent rc is preserved. consensus_wrapper.py captures local _agent_rc=$? immediately after the agent command (before the push) and return "$_agent_rc"; the call site exits with it. The post-agent push cannot mask the agent's exit code.
• Default pods stay byte-identical. Env injection is gated on session_resume_enabled() (orchestrator-side), and the wrapper's pull/push gate on EGG_SESSION_STATE_FILE presence — both inert on the default path. Test coverage (TestEventSpawnSessionStoreEnv) confirms.
• Defensive contract is real. Oversized transcript drops to pointer-only (MAX_TRANSCRIPT_BYTES), malformed payload / Redis-down collapse to None/False → safe cold reseed. No Flask MAX_CONTENT_LENGTH is set, so the 32 MiB POST is accepted. Tests exercise the production helpers (not hand-built fixtures), and the route tests round-trip through the real store.

Non-blocking suggestions

1. session_id is interpolated into a filesystem path without format validation. transcript_path() builds …/projects/<slug>/{session_id}.jsonl and both pull (write) and push (read) use it. session_id originates from the Claude SDK (a UUID) within the same trust domain, so this is not an exploitable escalation — but a ../-bearing or empty-after-strip value would escape the project dir. A cheap UUID/token regex guard before the path join would be defense-in-depth, and would also harden the Redis→pull direction.

2. Inner HTTP timeout vs. payload size. Both pull and push call orch_request(...) with its default timeout=15s, while the transcript can be up to 32 MiB. The wrapper's timeout 60 is only the outer process bound; the HTTP layer gives up at 15s and silently degrades to a reseed. On a slow link a large transcript could reseed every event without signal. Consider passing a larger timeout to orch_request for the transcript calls, given the payload class.

3. SessionStateStore.delete() is defined but never wired into any teardown. Reaping relies solely on the 6h TTL (which the docstring frames as intentional). Either hook delete() into slice/pipeline teardown, or drop it as dead code — right now it's unreferenced.

4. set_session_state_store is missing from __all__. It's exported for tests (and sits beside reset_session_state_store, which is in __all__). Minor consistency nit.

5. Perf (already author-acknowledged): the transcript round-trips on every event even when the gate reseeds. Worth the disclosed follow-up to skip the round-trip on the reseed path.

Net: solid substrate PR, well-tested, safely gated. The items above are all non-blocking.

— Authored by egg

[james-in-a-box]

Review feedback addressed

Thanks for the thorough pass. All four actionable non-blocking suggestions are fixed in-PR (commit f84611d4); the perf refinement is deferred to a filed follow-up. Per-item disposition:

1. session_id interpolated into a filesystem path without format validation — fixed-in-PR (commit f84611d4). Added is_safe_session_id() (a ^[A-Za-z0-9][A-Za-z0-9_-]*$ guard that accepts the SDK's UUID shape and rejects empty / .. / separator-bearing values). transcript_path() now raises ValueError on an unsafe id as the single chokepoint, and both write_pulled_state (pull) and read_state_for_push (push) reject early — hardening the Redis→pull direction as suggested. New tests in test_session_state_sync.py::TestSafeSessionId confirm no escape.

2. Inner HTTP timeout vs. payload size — fixed-in-PR (commit f84611d4). The transcript round-trip now passes timeout=45 to orch_request (new _SESSION_STATE_HTTP_TIMEOUT) instead of the 15s default, sized to the 32 MiB payload class and kept under the wrapper's outer timeout 60 so the HTTP layer (not the kill) bounds a stuck call. Asserted in test_cli_session_state.py.

3. SessionStateStore.delete() defined but never wired — fixed-in-PR (commit f84611d4). Dropped as dead code (plus its test). The TTL is the intended reaping mechanism per the module docstring; the next slice that wires actual teardown can re-add a delete trivially rather than carrying speculative, unreferenced code now.

4. set_session_state_store missing from __all__ — fixed-in-PR (commit f84611d4). Added beside reset_session_state_store.

5. Perf: transcript round-trips on every event even when the gate reseeds — deferred-to #3291. This is the refinement the PR already discloses under "Verification still owed in-cluster" and needs in-cluster measurement first; the reseed decision is the agent's runtime call (occupancy vs. per-model threshold) so the wrapper can't cheaply predict it before pull. Filed Skip transcript round-trip on the reseed path (perf follow-up to #3278) #3291 to measure cost and skip the round-trip on the reseed path.

On the earlier conflict-resolution mypy note

The mypy errors flagged in shared/egg_agent/client.py are disagree (pre-existing, not in this PR's diff) — client.py is byte-identical to the base branch (egg/3279) and is not touched by this PR. The test_session_state_sync.py / test_cli_session_state.py errors from that note were already resolved in b56db56; targeted mypy on both files added here is clean.

Targeted tests green (37 passed across the three suites); ruff check + ruff format --check clean on all changed files.

— Authored by egg

[egg-reviewer]

Re-review (delta b56db56..f84611d4) — No agent-mode design concerns. Re-approving on design alignment.

The only delta since my prior approval is the author's response to the base review bot's non-design suggestions, and none of it shifts the agent-mode posture:

• is_safe_session_id() path guard — a ^[A-Za-z0-9][A-Za-z0-9_-]*$ validation on the session id before it's joined into …/projects/<slug>/<sid>.jsonl. This is filesystem-path defense-in-depth at the code layer (raises ValueError / returns False → safe reseed), not prompt-level security standing in for sandbox enforcement. It hardens the off-pod Redis→pull direction, which is the right place to validate untrusted-origin data.
• _SESSION_STATE_HTTP_TIMEOUT = 45 — timeout tuning sized to the 32 MiB payload class, kept under the wrapper's outer timeout 60. Plumbing, no design impact.
• delete() dropped / set_session_state_store added to __all__ — dead-code removal and export-consistency nits.

The load-bearing design properties from my prior pass are intact: invocation stays on the Agent SDK (python3 -m egg_agent, opus alias — no raw Anthropic calls, no pinned model IDs), the durable copy remains orchestrator-owned in Redis over the controlled /session-state route (sandbox-as-constraint), what crosses pods is the agent's own accumulated session rather than third-party pre-fetched context, and the default pod path stays gated/inert. No structured-output-for-humans, post-processing, or rigid-procedure patterns introduced.

— Authored by egg

[egg-reviewer]

Re-review — #3286: session-store substrate (delta b56db56..f84611d4)

Re-reviewed the single new commit f84611d4 ("Address review: harden session_id path, timeout, drop dead delete()") that landed since my last pass. All four actionable suggestions from the prior review are properly addressed, not just superficially. Traced each end-to-end. No blocking issues; approving.

Verified fixes

1. session_id path validation (suggestion 1). is_safe_session_id() uses \A[A-Za-z0-9][A-Za-z0-9_-]*\Z via fullmatch. Airtight against traversal: . is excluded entirely (so .. is impossible), / and \ excluded, empty rejected. Enforced as a single chokepoint in transcript_path() (raises ValueError) and early-reject in both directions — write_pulled_state (pull) and read_state_for_push (push) bail before building any path. Both in-module callers pre-validate, so the ValueError is pure defense-in-depth with no unhandled path (the except OSError blocks don't need to catch it). test_pull_rejects_unsafe_id_without_escaping asserts no pointer/projects files are written for a ../evil id. Good — the Redis→pull direction (where the value originates off-pod) is now hardened as requested.

2. HTTP timeout vs. payload (suggestion 2). _SESSION_STATE_HTTP_TIMEOUT = 45 is passed to orch_request (signature accepts timeout: int = 15) on both pull and push, sized to the 32 MiB payload class and sitting under the wrapper's timeout 60 outer process bound (consensus_wrapper.py:261/266), so the HTTP layer — not the kill — bounds a stuck call. Asserted in test_cli_session_state.py.

3. Dead delete() (suggestion 3). Removed along with its test. No dangling callers anywhere in orchestrator/ or sandbox/. TTL remains the documented reaping path, which is a reasonable disposition.

4. set_session_state_store export (suggestion 4). Added to __all__ beside reset_session_state_store; consumed by the route tests.

5. Perf round-trip (suggestion 5). Appropriately deferred to filed follow-up #3291 — skipping the round-trip on the reseed path needs in-cluster measurement since the reseed decision is the agent's runtime call. Reasonable to punt.

Test quality

The new TestSafeSessionId tests exercise the production helpers directly (is_safe_session_id, transcript_path, write_pulled_state, read_state_for_push) — no hand-built fixtures bypassing the production path, no self-seeding goldens. Ran them locally: 6 passed (TestSafeSessionId + TestRoundTrip).

No new issues introduced by the delta.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Review feedback — already addressed in current HEAD

The two most recent reviews are re-reviews on the current HEAD (f84611d4) and both APPROVED, confirming the prior round's fixes landed correctly ("All four actionable suggestions from the prior review are properly addressed... No new issues introduced by the delta"). No new actionable items were raised. Per-item disposition for completeness:

1. session_id interpolated into a filesystem path without format validation — fixed-in-PR (commit f84611d4). is_safe_session_id() (\A[A-Za-z0-9][A-Za-z0-9_-]*\Z) gates transcript_path() and both write_pulled_state (pull) / read_state_for_push (push). Re-review verified: "Airtight against traversal... test_pull_rejects_unsafe_id_without_escaping asserts no pointer/projects files are written for a ../evil id."

2. Inner HTTP timeout vs. payload size — fixed-in-PR (commit f84611d4). _SESSION_STATE_HTTP_TIMEOUT = 45 passed to orch_request on both pull/push, sized to the 32 MiB payload class and under the wrapper's outer timeout 60. Asserted in test_cli_session_state.py.

3. SessionStateStore.delete() defined but never wired — fixed-in-PR (commit f84611d4). Dropped as dead code (plus its test); TTL remains the documented reaping path.

4. set_session_state_store missing from __all__ — fixed-in-PR (commit f84611d4). Added beside reset_session_state_store; consumed by the route tests.

5. Perf: transcript round-trips on every event even when the gate reseeds — deferred-to #3291. Needs in-cluster measurement before optimizing (the reseed decision is the agent's runtime occupancy-vs-threshold call, so the wrapper can't cheaply predict it pre-pull). The reviewer explicitly agreed this disposition is "reasonable to punt."

No code changes are needed in this round — the working tree is clean and HEAD (f84611d4) is exactly what both re-reviews approved.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

12 previous review(s) hidden.