Revert Claude Code substrate - #2742
Conversation
Abandons the Claude Code substrate initiative. Reverts the three commits that landed substrate code/docs on main: - 802f77d (#2715) walking-skeleton spike - 173f483 (#2718) substrate docs in index/structure docs - 82c4ba4 (#2731) substrate-swap rollout integration (slices 1-2) Removes orchestrator/substrate/, plugins/egg-sdlc/, the claude-code-substrate ADR, associated tests, and #2717/#2623 .egg-state pipeline artifacts.
There was a problem hiding this comment.
No agent-mode design concerns. This is a pure revert (65K lines removed, ~2 modified in non-deleted files) of the Claude Code substrate initiative; it does not introduce any new prompts, agent-dispatch patterns, pre-fetching, post-processing pipelines, structured-output-for-humans surfaces, hardcoded model IDs, or direct API calls. Whether to abandon the substrate strategy is an operator decision, not an agent-mode design question.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: clean wholesale revert, approve
Verified this is a faithful three-commit revert (802f77d, 173f483, 82c4ba4) with no collateral changes and no dangling references.
What I checked
Revert completeness — searched the whole tree (excluding .egg-state/) for the removed surfaces (EGG_SUBSTRATE, SubstrateBundle, select_substrate, ClaudeCodeSpawner, K3sSpawnerAdapter, run_pipeline_in_process, InProcessMessageBus, PreToolUseHookPolicy, LocalWorktreeManager, orchestrator.substrate, plugins/egg-sdlc, claude-code-substrate, the literal word substrate). Zero hits in .py, .md, .toml, .yaml, .yml, .json, Makefile, .github/, or scripts/.
Collateral check on the three reverted commits:
802f77d— only substrate code/tests/docs +.egg-state/artifacts. The conftest.py touch was the substrate fixture plus a small "fix-along" on twoexceptclauses (see note below).173f483— onlydocs/development/STRUCTURE.md+docs/index.md, both substrate-only.82c4ba4— only substrate code/tests/docs +.egg-state/artifacts.
No unrelated work is being lost.
Surgical modified files (the ones that aren't pure deletions):
orchestrator/concurrent_executor.py—_spawn_agentis restored to its pre-substrate form (legacyself.spawn_fn(...)path only);_spawn_agent_via_substrate/_teardown_role_worktreeare removed cleanly; no orphan imports — all ofAgentExecution,AgentExecutionStatus,Path,UTC,datetimeare still used elsewhere in the file.integration_tests/regression/conftest.py—substratefixture +_in_sandbox_agent_contexthelper removed;__all__updated;osstill used elsewhere in the file (no orphan import).- The three docs files: only the substrate bullet/section is removed; rest of the file is untouched.
Orphan tests: none. None of the remaining tests reference the substrate fixture, from orchestrator.substrate, or any of the removed test helper names.
Pre-existing egg-sdlc references in docs and pyproject.toml are to the separate bin/egg-sdlc CLI script and to the plugins/refine-plan/ plugin, not to the removed plugins/egg-sdlc/ plugin. Not orphans.
Non-blocking note
integration_tests/regression/conftest.py:283,293 — the revert reintroduces the un-parenthesized form:
except OSError, subprocess.TimeoutExpired:
return None
...
except ValueError, UnicodeDecodeError:
return NoneThis is valid Python 3.14 syntax per PEP 758, which the project's requires-python = ">=3.14" floor allows (verified with py_compile on 3.14.5). It is not a syntax error and the file imports cleanly.
That said, this form will read as a Python 2 syntax error to almost every reviewer who sees it (the original substrate PR added # fmt: skip parens specifically to avoid that confusion). Since the only purpose of the revert here is to match pre-substrate state, I'd suggest leaving those two paren-wraps in place — it's a 2-line readability win that has nothing to do with the substrate work and would otherwise be the kind of nit a future reviewer flags. Strictly optional.
Verdict
LGTM. The revert is mechanically clean, eliminates all live references to the removed surfaces, and preserves the pre-substrate k3s path verbatim in concurrent_executor.py. The make test result the PR description cites (17887 passed / 41 skipped) is consistent with what I'd expect given that nothing on main outside the reverted commits referenced these modules.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Restore the (OSError, subprocess.TimeoutExpired) and (ValueError, UnicodeDecodeError) paren-wraps the substrate PR had added. PEP 758 makes the bare form valid on Python 3.14, but it reads as a Python 2 SyntaxError to most reviewers, which is the kind of nit that would otherwise be flagged later.
Review responseBoth bot reviews approved; only one actionable item was raised. Disposition
— Authored by egg |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Autofix tracking{"Lint/Python": 1} |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review at 1f2dea7: no agent-mode design concerns.
Delta since the previous review (a38bfbd) is two commits (3c14f60, 1f2dea7) that only touch integration_tests/regression/conftest.py and net to zero tree changes — the auto-formatter commit reverted the paren-wrap re-add. Nothing in the delta touches agent prompts, dispatch, pre-fetching, structured-output surfaces, model IDs, or direct API call paths.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review at 1f2dea7: approve, no new issues
Delta since previous review
Two commits added since a38bfbd:
-
3c14f60— "Parenthesize except tuples in lifecycle_secret" (egg-reviewer[bot])- Re-added the paren-wraps in
integration_tests/regression/conftest.py:283,293per the non-blocking note from my previous review.
- Re-added the paren-wraps in
-
1f2dea7— "Fix checks: apply automated formatting fixes" (egg)- The exact same two lines, reverted: ruff (configured with
target-version = "py314"+UPrules) prefers the PEP 758 bare form on 3.14+ and unwrapped the parens.
- The exact same two lines, reverted: ruff (configured with
git diff a38bfbd..1f2dea7 is empty — the two commits cancel out. The tree is byte-identical to what I approved before.
Verification
git diff --name-only prev-review pr-2742→ no files changed.- Verified the bare
except OSError, subprocess.TimeoutExpired:form is valid syntax on Python 3.14.5 (PEP 758), and the project pinsrequires-python = ">=3.14", so this runs.
On the unresolved non-blocking suggestion
My prior "leave the parens for readability" note can't actually be addressed without lint-config changes — ruff at py314 will keep removing them. If the team wants to enforce parens for human readability, the fix lives in pyproject.toml ([tool.ruff.lint.per-file-ignores] for whichever UP rule unwraps them, or a project-wide ignore), not in the source files. That's well outside the scope of a revert PR.
I am not asking for any change here. Strictly non-blocking, and tracking it separately from this revert is the right call.
Verdict
LGTM. Same approval as before. Net-zero delta means no regression risk introduced by the re-push.
— Authored by egg
|
egg review completed. View run logs 7 previous review(s) hidden. |
…k_planner.md Conflict: plugins/egg-sdlc/skills/egg-sdlc/agents/task_planner.md was deleted on origin/main (Claude Code substrate revert, #2742) but had a one-line doc edit on this branch removing 'babysit' from a PipelineMode enum mention. Accepted the deletion — the file no longer exists on main and the PR's edit is moot.
Summary
Abandons the Claude Code substrate initiative. Reverts the three commits that landed substrate code/docs on
main:802f77d(Spike: walking-skeleton Claude Code substrate for egg SDLC (#2623) #2715) — walking-skeleton spike173f483(docs: add claude-code substrate to index and structure docs [doc-updater] #2718) — substrate docs in index/structure docs82c4ba4([#2717] Substrate-swap follow-up: integration branch #2731) — substrate-swap rollout integration (slices 1–2, [slice-1] Roll out Claude Code substrate to remaining roles + plan/... #2724/[slice-2] Roll out Claude Code substrate to remaining roles + plan/... #2726)Removes
orchestrator/substrate/,plugins/egg-sdlc/, theclaude-code-substrateADR, associated unit/integration tests, and the#2717/#2623.egg-statepipeline artifacts. 73 files, −65,257 lines.The four substrate PRs (#2715, #2724, #2726, #2731) were already merged, so they cannot be closed — this revert is how the work is removed. Related tracking issues (#2717, #2729, #2733, #2734, #2735, #2736) are being closed alongside this PR.
Test plan
make test— 17887 passed, 41 skipped (changeset-aware suite green; nomaincode depends on the removed substrate modules)orchestrator.substrate/plugins/egg-sdlc/claude-code-substratein code or docs