docs: document session-state warm-resume (#3278) [doc-updater] - #3334
Conversation
Update docs for the cross-pod BRC warm-resume session store added in #3332: - STRUCTURE.md: add orchestrator/session_state_store.py, routes/session_state.py, sandbox/egg_lib/cli_session_state.py, and session_state_sync.py - orchestrator-cli.md: add egg-orch session-state pull|push to the Quick Reference table and EGG_SESSION_STATE_FILE to the Environment Variables table Triggered by: #3332
This comment has been minimized.
This comment has been minimized.
Autofix tracking{"Test/Unit Tests": 1} |
This comment has been minimized.
This comment has been minimized.
The de-ledger rewrite on main removed the #2548 cross-reference from docs/reference/orchestrator-cli.md and the slice-1/context-branch tie from docs/guides/concurrent-execution.md, but the guard tests in tests/docs/test_context_pr_doc_terminology.py still require them. This branch carried pre-de-ledger copies that the merge with main dropped, so the merge result failed the tests. Sync both files to current main and re-add the required references the same way PR #3334 did: frame #2548 as the origin reshaped by #2777, and note slice-1 stacks directly on egg/<id>/work (not a context branch).
There was a problem hiding this comment.
ACK — documentation is accurate. This is a doc-only PR reflecting the code landed by #3332 (#3278 warm-resume substrate). I verified every claim against the actual sources rather than the PR description.
Verified against code
STRUCTURE.md — four new files, all present and correctly described:
orchestrator/session_state_store.py:54→SESSION_STATE_TTL_SECONDS = 6 * 60 * 60✓ "6-hour TTL"orchestrator/session_state_store.py:60→MAX_TRANSCRIPT_BYTES = 32 * 1024 * 1024✓ "32 MiB transcript cap"- "degrades to pointer-only on overflow" ✓ —
put()lines 136–148 settranscript = Noneand still store the pointer record. - Key shape
(pipeline, slice, role)✓ —_key()line 113–115 (slice_id or 'none'). orchestrator/routes/session_state.pypush/pull per(pipeline,slice,role)✓ — POST/GET handlers at lines 56/117.sandbox/egg_lib/cli_session_state.py(session-state pull|push, env-resolved identity, calls/session-stateroute) ✓ — and it's wired in viaorch_cli.py:4984 register_session_state_subcommand.sandbox/egg_lib/session_state_sync.py("slug math, transcript path resolution,write_pulled_state(),read_state_for_push()") ✓ —claude_project_slug,transcript_path,write_pulled_state,read_state_for_pushall present.
orchestrator-cli.md — CLI rows + env var:
- "Event-pump wrapper only" / "Gated on
EGG_SESSION_STATE_FILE" / "Best-effort: … exits 0" ✓ —consensus_wrapper.py:260-267only invokes pull/push under[ -n "${EGG_SESSION_STATE_FILE:-}" ]with|| true; the CLI itself returns 0 on every failure path (cli_session_state.py:75-80, 137-142). EGG_SESSION_STATE_FILEdefault/tmp/egg-session-state.json✓ —concurrent_executor.py:90 _POD_SESSION_STATE_FILE.- "Set by the concurrent executor only when warm resume is enabled (
EGG_SESSION_RESUME/EGG_CONTEXT_DISCIPLINE)" ✓ —concurrent_executor.py:194guards the assignment onsession_resume_enabled(), which returns true for eitherEGG_SESSION_RESUMEorcontext_discipline_enabled()(shared/egg_agent/session.py:79-95).
concurrent-execution.md / orchestrator-cli.md #2548 attribution:
- The added clause "slice-1 (and every later slice) stacks directly on
egg/<id>/work" is consistent with the existing doc body (concurrent-execution.md:1063already states "Slice-1 stacks onegg/<id>/work"). No contradiction introduced.
Notes (non-blocking)
- STRUCTURE.md insertion order is fine — the
sandbox/egg_lib/list is not alphabetized (e.g.orchestration.py,orch_cli.py,orch_client.py), so placing the two new files afterorch_cli.pyfollows the existing loose grouping. - Scope is correctly limited to the new files + new CLI/env surface; #3278 also touched several existing modules (
session.py,reseed.py,consensus_wrapper.py, …) but those already have STRUCTURE.md entries and need no edit.
No blocking issues. Documentation matches the implementation exactly.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Review feedback dispositionsThe formal review from
No code changes are needed. All CI checks are green on the current HEAD (the earlier Unit Tests failure was already resolved by the autofixer). — Authored by egg |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…-140121-3139/work # Conflicts: # docs/guides/concurrent-execution.md # docs/reference/orchestrator-cli.md
Conflict Resolution SummaryResolved merge conflicts with
Analysis: The merge base had neither cross-ref; both branches added one independently to resolve the same "restore #2548 cross-refs" lint check. The two phrasings are semantically equivalent, so I kept main's now-canonical wording to avoid doc drift rather than reintroducing a competing phrasing. The PR's actual contribution — the session-state warm-resume docs (new files in Verification: Please review: Confirm main's #2548 cross-ref phrasing is acceptable over the PR's near-identical wording. No semantic/code conflicts were involved. — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review — delta since 0084c826
The only change since my prior review is the merge commit cd29f27 resolving conflicts with main in docs/reference/orchestrator-cli.md and docs/guides/concurrent-execution.md. Both conflicts were over an independently-added #2548 cross-ref (the PR added one; main added a near-identical one via its own "Fix checks: #2548" commits). I re-reviewed the merged result.
Conflict resolution is correct:
orchestrator-cli.md— main's canonical #2548 phrasing was kept. The "Context PR Surfaces" section has two distinct, coherent #2548 mentions (lines 653, 669); no duplication, no broken table.concurrent-execution.md— main's version was kept, so this file now has zero net diff vs main (git diff origin/main...HEAD -- docs/guides/concurrent-execution.mdis empty). Existing #2548 references are intact.
The PR's actual contribution survived the merge fully intact — git diff origin/main...HEAD shows all session-state warm-resume docs present:
STRUCTURE.md:session_state_store.py,routes/session_state.py,cli_session_state.py,session_state_sync.pyorchestrator-cli.md:session-state pull|pushrows +EGG_SESSION_STATE_FILEenv var
Re-verified documented facts against current branch code (all match):
session_state_store.py:54,60→SESSION_STATE_TTL_SECONDS = 6h,MAX_TRANSCRIPT_BYTES = 32 MiB✓concurrent_executor.py:91→ default/tmp/egg-session-state.json✓concurrent_executor.py:202-204→EGG_SESSION_STATE_FILEset only insideif session_resume_enabled():✓consensus_wrapper.py:260-266→ pull/push gated on[ -n "${EGG_SESSION_STATE_FILE:-}" ], best-effort|| true✓- All four documented modules exist on the branch ✓
No blocking issues. The conflict resolution adopted main's phrasing to avoid doc drift and left the PR's documentation contribution untouched and accurate.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
Review feedback dispositionsBoth formal reviews from Latest re-review (commit Prior review (commit
No code changes are needed; the working tree at HEAD ( — Authored by egg |
|
egg feedback addressed. View run logs 10 previous review(s) hidden. |
Update documentation to reflect changes from #3332 (feat(#3278): persist Claude session per (pipeline,slice,role) across event pods):
orchestrator/session_state_store.py(Redis-backed cross-pod session store),orchestrator/routes/session_state.py(push/pull API endpoints),sandbox/egg_lib/cli_session_state.py(egg-orch session-state pull|pushCLI), andsandbox/egg_lib/session_state_sync.py(filesystem helpers for transcript round-trip)egg-orch session-state pull|pushto the Quick Reference table andEGG_SESSION_STATE_FILEto the Environment Variables tableTriggered by: #3332
Test Plan
sandbox/egg_lib/cli_session_state.pyandorchestrator/concurrent_executor.pyAuthored-by: egg