docs: add claude-code substrate to index and structure docs [doc-updater] by james-in-a-box[bot] · Pull Request #2718 · jwbron/egg

james-in-a-box · 2026-05-19T04:46:01Z

Update documentation to reflect changes from 802f77d (Spike: walking-skeleton Claude Code substrate for egg SDLC #2623):

docs/index.md: Added [Claude Code Substrate](architecture/claude-code-substrate.md) to the Architecture table. The ADR file existed and was referenced in docs/architecture/README.md but was missing from the main navigation index.
docs/development/STRUCTURE.md: Added the new orchestrator/substrate/ package to the orchestrator directory tree, including the four Protocol interfaces (spawner.py, message_bus.py, policy.py, worktree.py), the K3sSpawnerAdapter shim, the run_pipeline_in_process in-process entry point, and the claude_code/ subdirectory with its four native implementations.

Triggered by: #2715

Authored-by: egg

egg-reviewer

Summary

Docs-only PR adding the substrate/ package to docs/development/STRUCTURE.md and the Claude Code substrate ADR to docs/index.md. The index entry is accurate. The new substrate/ block in STRUCTURE.md contains one factual error and one structural omission that should be fixed before merge — the PR's entire value proposition is "make the structure docs accurately describe the new code," and one of the descriptions actively misleads the reader.

Blocking

1. `ClaudeCodeSpawner` description is factually wrong (`docs/development/STRUCTURE.md:158`)

│       ├── spawner.py      # `ClaudeCodeSpawner` — dispatches via `shared/egg_harness` + Agent tool

The "+ Agent tool" claim contradicts the implementation, which is explicit and emphatic on this point. From orchestrator/substrate/claude_code/spawner.py:11–18:

The walking-skeleton spike runs egg's existing egg_harness loop in-process to the user's Claude Code session — it does NOT, at the spike level, dispatch via Claude Code's native Agent tool with subagent_type="general-purpose". The harness drives the AnthropicProvider directly and exposes its own tool registry.

And the code itself confirms it (spawner.py:136): the runner resolves to egg_harness.client.run_agent, never to anything that emits an Agent-tool envelope. The Agent-tool spawner is enumerated in the ADR's "Open work" as a ClaudeCodeAgentToolSpawner follow-up.

This is exactly the mental-model error this PR is meant to prevent. A reader scanning STRUCTURE.md will conclude the substrate already dispatches through Claude Code's native subagent surface — which is the architecture the team chose not to ship in the spike, and the security delta documented in R1 of the ADR depends on that distinction.

Fix: drop "+ Agent tool". Suggested replacement:

│       ├── spawner.py      # `ClaudeCodeSpawner` — runs `egg_harness.run_agent` in-process (Agent-tool dispatch is a follow-up; see ADR "Open work")

Non-blocking

2. `claude_code/hook_entry.py` is missing from the listing

orchestrator/substrate/claude_code/ actually contains seven entries (excluding __init__.py): hook_entry.py, message_bus.py, policy.py, settings.template.json, spawner.py, worktree.py. The PR lists only four. The most consequential omission is hook_entry.py (~31 KB — the largest file in the package, and over 4× the size of policy.py).

hook_entry.py is the standalone script Claude Code invokes via python3 -m orchestrator.substrate.claude_code.hook_entry (per settings.template.json). It implements the Bash/Write/Edit parser and the fail-closed denial logic. The PR rolls all of this into the policy.py line, but policy.py (~8 KB) is the PolicyEnforcer adapter (check_write + install), not the hook itself. A reader who finds the description in policy.py would not know hook_entry.py exists or where the actual parser/decision code lives — which is precisely the kind of orientation the structure doc is supposed to provide.

Suggested addition (after the policy.py line):

│       ├── hook_entry.py   # Standalone `python3 -m` PreToolUse hook script invoked by Claude Code (Bash/Write/Edit parser; fail-closed on ambiguous shapes)

3. Optional: `settings.template.json` could be listed

It's a real file in the directory and only mentioned in narrative form (in the policy.py description). The existing STRUCTURE.md convention is to list .py files; whether to extend that to a config template is a judgment call. Worth listing for navigability, but I won't block on it.

— Authored by egg

- Drop incorrect '+ Agent tool' from spawner.py description; the spike runs egg_harness.run_agent in-process and does NOT dispatch via the Agent tool (Agent-tool spawner is an ADR follow-up). - Add hook_entry.py to the listing — it is the standalone PreToolUse hook script and the largest file in the package (~31 KB). - Clarify policy.py is the PolicyEnforcer adapter wrapping hook_entry.py. - List settings.template.json for navigability. Addresses egg-reviewer CHANGES_REQUESTED on PR #2718.

james-in-a-box · 2026-05-19T05:03:29Z

Thanks for the careful review. Addressed all three items in 627b944.

Per-item disposition:

ClaudeCodeSpawner description factually wrong — fixed-in-PR (commit 627b944). Dropped + Agent tool and adopted the suggested replacement (runs egg_harness.run_agent in-process (Agent-tool dispatch is a follow-up; see ADR "Open work")). The docstring at orchestrator/substrate/claude_code/spawner.py:11–28 is the authoritative framing and STRUCTURE.md now matches it.
hook_entry.py missing from listing — fixed-in-PR (commit 627b944). Added the suggested hook_entry.py line. Also tightened the policy.py description to clarify it is the PolicyEnforcer adapter (check_write + install) wrapping hook_entry.py, so readers don't conflate the two.
settings.template.json listing (optional) — fixed-in-PR (commit 627b944). Listed it too — it's a real file in the directory, the navigability benefit outweighs the cost of extending the convention beyond .py files for a single config template that the hook depends on.

— Authored by egg

egg-reviewer

Summary

All three items from my prior review are addressed in 627b944. The delta is exactly the changes requested — no scope creep, no collateral edits. I re-read the four claude_code files referenced in the changed listing (spawner.py, hook_entry.py, policy.py, settings.template.json) and the descriptions now match the source.

Verifications

spawner.py line (was blocking) — docs/development/STRUCTURE.md:151 now reads runs \egg_harness.run_agent` in-process (Agent-tool dispatch is a follow-up; see ADR "Open work"). Matches the authoritative framing in orchestrator/substrate/claude_code/spawner.py:11-28and:136. The misleading + Agent tool` claim is gone.
hook_entry.py line (was non-blocking) — docs/development/STRUCTURE.md:152 adds the file with the suggested description. The file exists at orchestrator/substrate/claude_code/hook_entry.py (~31 KB; the largest in the package) and the description "Bash/Write/Edit parser; fail-closed on ambiguous shapes" matches the docstring at hook_entry.py:1-40 and the actual matcher in settings.template.json (Write|Edit|MultiEdit|NotebookEdit|Bash).
settings.template.json line (was non-blocking) — added at docs/development/STRUCTURE.md:155. The description "registering hook_entry.py as the PreToolUse hook" matches the file's actual contents (one PreToolUse hook entry calling python3 -m orchestrator.substrate.claude_code.hook_entry).
policy.py line was also tightened — docs/development/STRUCTURE.md:154 now reads \PolicyEnforcer` adapter (`check_write` + `install`) wrapping the `hook_entry.py` script. Both methods exist (policy.py:48 check_write, :70 install) and the class re-exports hook_entry.decideviadecide = staticmethod(hook_entry.decide)atpolicy.py:188, so "wrapping the hook_entry.py script" is accurate. The replacement drops the explicit class name PreToolUseHookPolicy`, which is a minor navigability regression vs. the prior line — non-blocking; the rest of STRUCTURE.md is inconsistent about whether class names appear in descriptions.

Non-blocking

None worth flagging. The substrate/ block continues the rest-of-STRUCTURE.md convention of grouping by purpose rather than alphabetizing, which is consistent with the surrounding orchestrator/ tree (lines 100-141 are also non-alphabetical).

— Authored by egg

james-in-a-box · 2026-05-19T05:17:42Z

egg review completed. View run logs

3 previous review(s) hidden.

#2724) * docs: add claude-code substrate to index and structure docs [doc-updater] (#2718) * docs: add claude-code substrate to index and structure docs * docs: fix substrate/claude_code listing per reviewer feedback - Drop incorrect '+ Agent tool' from spawner.py description; the spike runs egg_harness.run_agent in-process and does NOT dispatch via the Agent tool (Agent-tool spawner is an ADR follow-up). - Add hook_entry.py to the listing — it is the standalone PreToolUse hook script and the largest file in the package (~31 KB). - Clarify policy.py is the PolicyEnforcer adapter wrapping hook_entry.py. - List settings.template.json for navigability. Addresses egg-reviewer CHANGES_REQUESTED on PR #2718. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: update deployment guide for Cilium portmap CNI changes [doc-updater] (#2716) * docs: document portmap CNI and iptables reboot requirement (#2713) * docs: mention netfilter-persistent as alternative to re-running after reboot Addresses non-blocking review feedback on #2716. The reviewer noted that install-cilium.sh's own comment block calls out netfilter-persistent / iptables-restore as a system-level persistence alternative to re-running the script after every reboot. Mirror that in the operator-facing doc so long-running k3s host operators know they have an option beyond manual re-runs. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: add reconcile_autostash_pop_conflict to push diagnostic list (#2720) Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> * slice-1 coder: bridge driver + R2 nested-dispatch fake + loader expansion Implements three #2717 slice-1 coder tasks toward the substrate-swap follow-up rollout per cq-1=Option C hybrid bridge (refine/plan flattened, implement daemon), cq-2 phase-sequential slicing, cq-5 early-spike R2 validation, and cq-3 harness re-host alignment. TASK-1-1 (bin/run_pipeline.py): flattened single-yield stage driver that advances `run_pipeline_in_process` to its next yield, serialises the yielded HITLDecision to .egg-state/contracts/<id>.json under a stable `pending_hitl` envelope schema (version, decision, answer, answer_log, status, result, error), and exits. Cross-process generator state is recovered by replaying `answer_log` on each invocation — viable for refine/plan because the generator is deterministic; slice-3's daemon variant (TASK-3-2) consumes the same envelope schema so the two bridges share a state-serialization contract (risk_analyst R17 mitigation). End-to-end round-trip verified: first invocation yields preflight decision; operator answer round-trips through the contract; second invocation replays and advances to the refine-gate decision. TASK-1-9 (integration_tests/regression/_agent_tool_fake.py): test- only nested-Agent-tool dispatch fake. Simulates Claude Code's Agent tool by spawning a child subprocess with controlled EGG_AGENT_ROLE; the child invokes orchestrator/substrate/claude_code/hook_entry.py `decide(...)` directly. Validates the hook-logic half of R2 — given accurate EGG_AGENT_ROLE propagation, does the hook deny a write that violates the *child's* role pattern even when the parent's role would allow it? Hard import guard prevents production use; the file is coder-owned (underscored helper name, mirroring _helpers.py) rather than tester-owned. Production dispatch stays on ClaudeCodeSpawner (harness re-host) per cq-3. TASK-1-6 (orchestrator/substrate/__init__.py): extends `_load_egg_sdlc_role_rubric` so reviewer_refine and reviewer_agent_design are recognised as supported (alongside the existing refiner). Introduces a `_ROLE_RUBRIC_SLICES` mapping that names which #2717 rollout slice ships each role's rubric (slice-1: refine team; slice-2: plan team; slice-3: implement team) so future slice loaders can extend the set declaratively, and a `_RUBRIC_LANDED_ROLES` set documenting which rubric .md files exist on disk today. Roles outside the landed set raise a structured ValueError citing the correct rollout slice ("deferred to follow-up slice-2 of issue #2717's rollout"). The acceptance criterion's "follow-up slice 2" hint for architect is now produced. Lint clean (ruff check + format); mypy clean on the new files; the pre-existing 6 mypy errors in substrate/__init__.py:180-198 are unrelated to this change. 52 existing claude-code-substrate tests still pass. Refs #2717 (slice-1 coder). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): refine-team rubrics + flattened-bridge docs + ADR rollout deltas Slice 1 of the #2717 substrate-swap rollout adds two refine-team reviewer rubrics, closes the heredoc-HITL bridge gap for refine-phase via a flattened bin/run_pipeline.py stage driver, and adds the cq-5 R2 spike for nested PreToolUse-hook role-routing. This commit lands the documenter-owned half: TASK-1-4: New reviewer rubric files at plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_refine.md plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_agent_design.md mirroring the layout of plugins/refine-plan/skills/refine-plan/agents/ so the in-process orchestrator's build_system_prompt(sources) loader picks them up without per-skill custom logic. Both files carry frontmatter (name + description) and the body documents the substrate-specific operational deltas (worktree path, PreToolUse-enforced restrictions, AskUserQuestion surfacing, verdict path) on top of the same rubric the k3s reviewers use. TASK-1-2: SKILL.md is updated to replace the "Walking-skeleton bridge gap" callout with a "How the flattened bridge works" section that names pending_hitl as the single-yield carrier and documents the skill→driver loop. The "What the skill is designed to do" step list moves from aspirational to actually-shipping. The R2 PreToolUse-hook section points at the new test infrastructure (test_pretooluse_hook_nested.py + _agent_tool_fake.py) and the r2-verdict.json file. Frontmatter description re-flows to reflect the slice-1 scope. TASK-1-8: docs/architecture/claude-code-substrate.md is updated for the ADR-level audit trail: - Title and status banner reframe from "spike" to "spike → rollout". - cq-2 / cq-7 / cq-11 table rows reflect what slice 1 lands. - The in-process orchestrator section gets a "The flattened bridge" subsection naming the cq-1 hybrid (Option C) and the slice-3 daemon variant that consumes the same pending_hitl envelope shape (R17 mitigation). - The egg-sdlc plugin section enumerates the three refine-team rubrics and the new bin/run_pipeline.py driver. - The R2 risk-mitigation subsection points at the slice-1 worked example and the slice-5 contingent fallback (cq-6 option 2 + R15 model (b)). - The R15 subsection makes the model (a) → (b) migration contingent on the slice-1 R2 verdict. - The "Open work" + "Follow-up issue draft" sections are replaced with a unified "Rollout deltas" section split into Completed-in-this-rollout (3 slice-1 items, marked with [x] + strikethrough on the obsolete text) and Pending-in-this-rollout (9 items mapped to slices 2-5). The acceptance bar is unchanged. - The primitives table picks up the four new slice-1 modules (bin/run_pipeline.py, _agent_tool_fake.py, test_pretooluse_hook_nested.py, the two new reviewer rubrics). - The conformance-proof section names the slice-1 regression-test additions (test_bridge_flattened_round_trip.py, test_rubric_loader.py). - Stale anchor links to the removed "Follow-up issue draft" section are redirected to the new "Rollout deltas" anchor. These doc changes satisfy TASK-1-2, TASK-1-4, and TASK-1-8 from slice 1 of the #2717 plan; no source or test files are touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester: rubric loader + bridge round-trip + R2 nested-dispatch tests Adds three test files covering #2717 slice-1's tester contract tasks: TASK-1-7 (shared/tests/test_rubric_loader.py): unit tests for `_load_egg_sdlc_role_rubric`. Pins the four AC cases: refiner regression, reviewer_refine load, reviewer_agent_design load, architect raises ValueError with the updated "follow-up slice 2" diagnostic. Adversarial probing layered on: AgentRole enum vs str input equivalence, defense against path-traversal role values, structured fence for unshipped plan-phase roles (reviewer_plan, reviewer_code, task_planner). Eight of 10 tests pass today; two (reviewer_refine, reviewer_agent_design loads) are documenter-dependency failures that flip green once task-1-4 lands the rubric .md files. TASK-1-3 (integration_tests/regression/test_bridge_flattened_round_trip.py): end-to-end round-trip test for the flattened bridge driver. Runs the real `bin/run_pipeline.py` in a fresh subprocess twice against a deterministic pipeline id: stage A captures the preflight HITLDecision into `pending_hitl.decision`, the test writes `answer="approve" + status="answered"`, stage B re-enters the process and replays the answer to advance to the refine-gate decision. Validates that the generator state survives via the contract-state round-trip across process exit. Substrate isolation via a `-c` shim that monkey-patches `orchestrator.substrate.select_substrate` to a MagicMock bundle — no real Claude Code / Anthropic API call. Also pins driver-side idempotency (a re-invocation without a new answer must not silently advance the generator). TASK-1-5 (integration_tests/regression/test_pretooluse_hook_nested.py): cq-5 early-spike R2 verdict test. Uses task-1-9's `_agent_tool_fake` to drive a deterministic nested-dispatch scenario: parent_role=architect + child_role=tester + write_target=orchestrator/foo.py — asserts the hook returns `{"decision": "block", "reason": ...}` and that the deny reason names the child (tester) role rather than the parent. Writes `.egg-state/<pipeline_id>/r2-verdict.json` with the pass verdict per AC. Adds in-role allow control + cross-role probe (parent=coder, child=tester writing orchestrator/* — must deny by the child's role) + dataclass shape pin + EGG_AGENT_ROLE leak guard. Docstring documents the empirical-vs-test-fake limitation cq-3 explicitly accepts (production stays on the harness re-host until slice-5 R15 flips dispatch). All 15 of 17 tests pass today. The 2 failing rubric tests are contracted documenter-dependency failures (task-1-4 not landed yet) and are expected to flip green once the documenter ships. Lint clean (ruff check + format). Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): address reviewer_code v1 NACK on SKILL.md envelope + CLI Reviewer_code NACKed v1 with three blockers against SKILL.md (the two rubric files and the ADR were ACKed as-is). This commit addresses all three blockers plus four non-blocking polish items. Blocking fixes in SKILL.md: 1. CLI invocation example was wrong (used --pipeline-id / --issue, but the driver at plugins/egg-sdlc/skills/egg-sdlc/bin/run_pipeline.py:355-402 takes pipeline_id as a positional arg and --issue-number as the flag). Rewrote the bash loop example to match the actual argparse signature so a copy-paste invocation actually runs. 2. The documented pending_hitl envelope was 5 fields (version, pipeline_id, timestamp, decision, answer) but the driver writes 9 (adds status, result, error, answer_log). Replaced the truncated schema with the full envelope and added per-field semantics (especially the status field, which is the skill's loop predicate: pending / answered / completed / aborted / error). The slice-3 daemon variant inherits all 9 fields. 3. No documented mechanism for the skill body to write pending_hitl.answer (the frontmatter allowed-tools does not include the Write tool). Documented option (a) from the reviewer's NACK: an inline python3 -c "..." invocation, which is covered by the existing Bash(python3 *:*) allowed-tool. The "skill loop" code block now demonstrates the round-trip with a case statement keyed on pending_hitl.status. Non-blocking polish in SKILL.md: - Loop semantics now name "replay" explicitly (the driver spawns a fresh generator and replays answer_log on every invocation; previous text suggested cheap single-step resumption). Added a dedicated "Generator state across invocations (replay semantics)" subsection naming the practical consequence — side effects re-run every call — and pointing at slice 3 as the daemon-variant escape hatch. - Failure-mode bullet for "pending_hitl.decision == null" replaced with the more general "pending_hitl.status ∈ {completed, aborted, error}" bullet so each terminal state has documented diagnostic guidance. Non-blocking polish in docs/architecture/claude-code-substrate.md: - The "Flattened bridge" bullet in the in-process orchestrator section now names the replay path explicitly (promotes answer → answer_log, replays the full log every call, deterministic same-yield-boundary property) and the 5→9 envelope field list mirrors SKILL.md. - Daemon-variant bullet enumerates the same 9-field shape so reviewers comparing slice-1 and slice-3 against the ADR see the full contract. - Schema source-of-truth pointer added to bin/run_pipeline.py:20-46 so future drift triggers fail in one place. Rubric files (reviewer_refine.md, reviewer_agent_design.md) are unchanged — reviewer_code ACKed them in v1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test_bridge_flattened_round_trip: fix subprocess PYTHONPATH The subprocess shim's PYTHONPATH pointed at `<repo>/orchestrator/` and `<repo>/shared/` directly, which lets the subprocess `import substrate` but NOT `import orchestrator.substrate` — the latter requires the *parent* of `orchestrator/` (the repo root) on the path because ``orchestrator/__init__.py`` makes it a real package. Set PYTHONPATH to ``<repo>/shared`` + ``<repo>`` + ``<repo>/gateway`` so: - ``<repo>/shared`` lets ``egg_contracts`` (transitive import from ``orchestrator.substrate.k3s_adapter``) resolve. - ``<repo>`` lets ``import orchestrator`` resolve. Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester v2: fix subprocess PYTHONPATH + non-blocking improvements Addresses reviewer_code v1 blocking #1 + non-blocking items: BLOCKING FIX — subprocess PYTHONPATH: Add `<repo>/orchestrator` to the subprocess shim's PYTHONPATH so bare- name top-level imports inside the `orchestrator/` tree resolve cleanly. Without this, `orchestrator/models.py:16` (`from slice_id_validation import SLICE_ID_PATTERN`) and `in_process.py:531-534`'s bare `from models import HITLDecision` fallback both fail, crashing the driver subprocess with `ModuleNotFoundError` before it yields the first HITL decision. Mirrors the Makefile's `PYTHONPATH := shared:gateway: orchestrator` (test target). Verified: tests now pass with `PYTHONPATH=.:shared:orchestrator pytest <files>` (reviewer_code's exact reproduction env). NON-BLOCKING (reviewer_code v1): - test_bridge_flattened_round_trip.py: drop stale "whichever the coder picks" docstring phrasing — driver locked in positional argv[1]. - test_bridge_flattened_round_trip.py: mirror the driver's ISO-8601 UTC timestamp format in _write_answer instead of `str(time.time())`. - test_rubric_loader.py: extend `test_loader_accepts_enum_and_string_role` parametrization to cover the two NEW roles (reviewer_refine, reviewer_agent_design) — not just the regression role. - test_rubric_loader.py: strengthen path-traversal assertion to verify the allowlist's slice-fence branch fires (not the file-missing-on- disk branch) — pinning the structural defence. - test_pretooluse_hook_nested.py: derive the r2-verdict.json content from the dispatch outcome and write it BEFORE the structured assertions so slice-5 sees an accurate empirical record even when a regression fails one of the assertions. Adds {"r2_verdict": "fail", "reason": ...} payload format for the failure path. Tests verified: 21/21 pass under both the canonical Makefile PYTHONPATH shape and reviewer_code's `.:shared:orchestrator` reproduction shape. Refs #2717 (slice-1 tester, v2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-1 (#2548) * Address slice-1 review: fix install path, bridge answer-write, silent fallbacks Reviewer findings from PR #2724 (egg-reviewer slice-1 review): Blockers (1-4): * The documented `pip install -r requirements.txt` failed — no top-level requirements.txt exists. Switch SKILL.md, plugin.json's `egg.install_instructions`, and `bin/preflight.py` to `pip install .` against pyproject.toml. * SKILL.md claimed Python 3.11+ but pyproject.toml requires >=3.14. Update SKILL.md and preflight to reflect the real floor; correct the matching docstring in orchestrator/substrate/__init__.py. * The inline `python3 -c "..."` answer-write snippet in SKILL.md was broken (shell-interpolated `${ANSWER}` → NameError, deprecated `datetime.utcnow`, non-atomic write). Replace with a dedicated `bin/write_answer.py` helper that reads the JSON-encoded answer from stdin, uses `datetime.now(UTC)`, and writes atomically via tmp + `os.replace`. SKILL.md's loop now invokes the helper. * `_serialise_decision` silently swallowed `model_dump` failures, and `_read_contract` silently overwrote unparseable contracts (dropping `answer_log`). Log to stderr on serialisation fallbacks; refuse to overwrite a corrupted contract and exit 1 instead. Non-blocking (5-11): * `orchestrator/substrate/__init__.py` docstring rewritten to reflect reality: Python 3.14+ introduced PEP 758 (`except A, B:` without parens); ruff under py314 target strips redundant parens, hence `# fmt: skip` on multi-except lines. * Stderr warning in `_advance_generator`'s `finally` so a teardown failure inside `generator.close()` is at least observable. * `allowed-tools` tightened from `Bash(python3 *:*)` to `Bash(python3 plugins/egg-sdlc/skills/egg-sdlc/bin/*:*)` — the two helper scripts are the entire Python surface the skill can invoke. * Re-spawn cost note expanded in SKILL.md: each driver invocation re-runs every prior subagent spawn (real Anthropic API spend); slice-2 compounds to 8 spawns at the final stage. * `_RUBRIC_LANDED_ROLES` registry collapsed into `_CURRENT_LOADER_SLICE` + filesystem probe; same diagnostics, no parallel registry that can drift from disk state. * Abort vocabulary exported as `ABORT_ANSWERS` from `orchestrator.substrate.in_process`; the driver imports it (with a literal fallback when the orchestrator package is unimportable) so the driver, orchestrator, and slice-3 daemon share a single source of truth. * R2 caveat surfaced as an explicit open question for slice-5 sequencing in SKILL.md. Tests: * New `shared/tests/test_write_answer.py` (6 tests, all passing) pins the JSON-encoding round-trip, the timestamp format match against the driver, the atomic-write contract, and the corrupted-contract refusal. * Existing `test_rubric_loader`, `test_run_pipeline_in_process*`, `test_substrate_interfaces`, `test_bridge_flattened_round_trip`, and `test_pretooluse_hook_nested` all still pass. * The pre-existing `test_empty_diff_subprocess_skips_pytest` failure in `tests/tools/` reproduces against HEAD without these changes (detached-HEAD worktree edge case in the test selector). Authored-by: egg * Move skill-loop python3 -c calls into bin/ helpers Address review feedback on PR #2724: - Add bin/read_status.py and extend write_answer.py with --answer-string so every subcommand in SKILL.md's loop body is a single `python3 plugins/.../bin/<helper>.py` invocation. Honors the tightened allowed-tools pattern `Bash(python3 plugins/.../bin/*:*)` per Claude Code's compound-command permission rules — no separate `Bash(python3 -c *)` or `Bash(printf *)` rule needed, no prompt-injection door left open. - Update SKILL.md step 4 to name bin/write_answer.py directly (matches the new loop body). - Replace `slice_hint != _CURRENT_LOADER_SLICE` rubric-loader fence with `slice_hint not in _LANDED_SLICES` (frozenset) so future slices extend rather than replace the landed set — slice-2 won't fence off slice-1's already-shipped refine roles. - Wire test_bridge_flattened_round_trip's _write_answer through subprocess(write_answer.py) so the integration test exercises the production write path end-to-end. - Add test_read_status.py (7 tests) and --answer-string coverage in test_write_answer.py (2 tests). * Address non-blocking review notes on PR #2724 - write_answer.py: clarify --answer-string docstring — the JSON encoding happens at contract serialisation time (json.dumps(contract)), not as a separate json.dumps(answer) step. Reference the special-characters test as the proof of the round-trip. - SKILL.md / read_status.py: document the case statement's intentional fall-through on empty STATUS. read_status.py prints empty + exit 0 when no pending_hitl envelope exists; the case has no *) arm, so the empty value falls through, the case exits 0, and the outer iteration re-invokes run_pipeline.py — which is the recover path. - test_rubric_loader.py: add test_landed_slices_contains_slice1 to mechanically pin the 'extend, don't replace' invariant on _LANDED_SLICES so a future slice cannot silently regress slice-1 by writing frozenset({'slice-2'}) instead of frozenset({'slice-1', 'slice-2'}). --------- Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-orchestrator <egg@localhost>

#2726) * docs: add claude-code substrate to index and structure docs [doc-updater] (#2718) * docs: add claude-code substrate to index and structure docs * docs: fix substrate/claude_code listing per reviewer feedback - Drop incorrect '+ Agent tool' from spawner.py description; the spike runs egg_harness.run_agent in-process and does NOT dispatch via the Agent tool (Agent-tool spawner is an ADR follow-up). - Add hook_entry.py to the listing — it is the standalone PreToolUse hook script and the largest file in the package (~31 KB). - Clarify policy.py is the PolicyEnforcer adapter wrapping hook_entry.py. - List settings.template.json for navigability. Addresses egg-reviewer CHANGES_REQUESTED on PR #2718. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: update deployment guide for Cilium portmap CNI changes [doc-updater] (#2716) * docs: document portmap CNI and iptables reboot requirement (#2713) * docs: mention netfilter-persistent as alternative to re-running after reboot Addresses non-blocking review feedback on #2716. The reviewer noted that install-cilium.sh's own comment block calls out netfilter-persistent / iptables-restore as a system-level persistence alternative to re-running the script after every reboot. Mirror that in the operator-facing doc so long-running k3s host operators know they have an option beyond manual re-runs. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: add reconcile_autostash_pop_conflict to push diagnostic list (#2720) Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> * slice-1 coder: bridge driver + R2 nested-dispatch fake + loader expansion Implements three #2717 slice-1 coder tasks toward the substrate-swap follow-up rollout per cq-1=Option C hybrid bridge (refine/plan flattened, implement daemon), cq-2 phase-sequential slicing, cq-5 early-spike R2 validation, and cq-3 harness re-host alignment. TASK-1-1 (bin/run_pipeline.py): flattened single-yield stage driver that advances `run_pipeline_in_process` to its next yield, serialises the yielded HITLDecision to .egg-state/contracts/<id>.json under a stable `pending_hitl` envelope schema (version, decision, answer, answer_log, status, result, error), and exits. Cross-process generator state is recovered by replaying `answer_log` on each invocation — viable for refine/plan because the generator is deterministic; slice-3's daemon variant (TASK-3-2) consumes the same envelope schema so the two bridges share a state-serialization contract (risk_analyst R17 mitigation). End-to-end round-trip verified: first invocation yields preflight decision; operator answer round-trips through the contract; second invocation replays and advances to the refine-gate decision. TASK-1-9 (integration_tests/regression/_agent_tool_fake.py): test- only nested-Agent-tool dispatch fake. Simulates Claude Code's Agent tool by spawning a child subprocess with controlled EGG_AGENT_ROLE; the child invokes orchestrator/substrate/claude_code/hook_entry.py `decide(...)` directly. Validates the hook-logic half of R2 — given accurate EGG_AGENT_ROLE propagation, does the hook deny a write that violates the *child's* role pattern even when the parent's role would allow it? Hard import guard prevents production use; the file is coder-owned (underscored helper name, mirroring _helpers.py) rather than tester-owned. Production dispatch stays on ClaudeCodeSpawner (harness re-host) per cq-3. TASK-1-6 (orchestrator/substrate/__init__.py): extends `_load_egg_sdlc_role_rubric` so reviewer_refine and reviewer_agent_design are recognised as supported (alongside the existing refiner). Introduces a `_ROLE_RUBRIC_SLICES` mapping that names which #2717 rollout slice ships each role's rubric (slice-1: refine team; slice-2: plan team; slice-3: implement team) so future slice loaders can extend the set declaratively, and a `_RUBRIC_LANDED_ROLES` set documenting which rubric .md files exist on disk today. Roles outside the landed set raise a structured ValueError citing the correct rollout slice ("deferred to follow-up slice-2 of issue #2717's rollout"). The acceptance criterion's "follow-up slice 2" hint for architect is now produced. Lint clean (ruff check + format); mypy clean on the new files; the pre-existing 6 mypy errors in substrate/__init__.py:180-198 are unrelated to this change. 52 existing claude-code-substrate tests still pass. Refs #2717 (slice-1 coder). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): refine-team rubrics + flattened-bridge docs + ADR rollout deltas Slice 1 of the #2717 substrate-swap rollout adds two refine-team reviewer rubrics, closes the heredoc-HITL bridge gap for refine-phase via a flattened bin/run_pipeline.py stage driver, and adds the cq-5 R2 spike for nested PreToolUse-hook role-routing. This commit lands the documenter-owned half: TASK-1-4: New reviewer rubric files at plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_refine.md plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_agent_design.md mirroring the layout of plugins/refine-plan/skills/refine-plan/agents/ so the in-process orchestrator's build_system_prompt(sources) loader picks them up without per-skill custom logic. Both files carry frontmatter (name + description) and the body documents the substrate-specific operational deltas (worktree path, PreToolUse-enforced restrictions, AskUserQuestion surfacing, verdict path) on top of the same rubric the k3s reviewers use. TASK-1-2: SKILL.md is updated to replace the "Walking-skeleton bridge gap" callout with a "How the flattened bridge works" section that names pending_hitl as the single-yield carrier and documents the skill→driver loop. The "What the skill is designed to do" step list moves from aspirational to actually-shipping. The R2 PreToolUse-hook section points at the new test infrastructure (test_pretooluse_hook_nested.py + _agent_tool_fake.py) and the r2-verdict.json file. Frontmatter description re-flows to reflect the slice-1 scope. TASK-1-8: docs/architecture/claude-code-substrate.md is updated for the ADR-level audit trail: - Title and status banner reframe from "spike" to "spike → rollout". - cq-2 / cq-7 / cq-11 table rows reflect what slice 1 lands. - The in-process orchestrator section gets a "The flattened bridge" subsection naming the cq-1 hybrid (Option C) and the slice-3 daemon variant that consumes the same pending_hitl envelope shape (R17 mitigation). - The egg-sdlc plugin section enumerates the three refine-team rubrics and the new bin/run_pipeline.py driver. - The R2 risk-mitigation subsection points at the slice-1 worked example and the slice-5 contingent fallback (cq-6 option 2 + R15 model (b)). - The R15 subsection makes the model (a) → (b) migration contingent on the slice-1 R2 verdict. - The "Open work" + "Follow-up issue draft" sections are replaced with a unified "Rollout deltas" section split into Completed-in-this-rollout (3 slice-1 items, marked with [x] + strikethrough on the obsolete text) and Pending-in-this-rollout (9 items mapped to slices 2-5). The acceptance bar is unchanged. - The primitives table picks up the four new slice-1 modules (bin/run_pipeline.py, _agent_tool_fake.py, test_pretooluse_hook_nested.py, the two new reviewer rubrics). - The conformance-proof section names the slice-1 regression-test additions (test_bridge_flattened_round_trip.py, test_rubric_loader.py). - Stale anchor links to the removed "Follow-up issue draft" section are redirected to the new "Rollout deltas" anchor. These doc changes satisfy TASK-1-2, TASK-1-4, and TASK-1-8 from slice 1 of the #2717 plan; no source or test files are touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester: rubric loader + bridge round-trip + R2 nested-dispatch tests Adds three test files covering #2717 slice-1's tester contract tasks: TASK-1-7 (shared/tests/test_rubric_loader.py): unit tests for `_load_egg_sdlc_role_rubric`. Pins the four AC cases: refiner regression, reviewer_refine load, reviewer_agent_design load, architect raises ValueError with the updated "follow-up slice 2" diagnostic. Adversarial probing layered on: AgentRole enum vs str input equivalence, defense against path-traversal role values, structured fence for unshipped plan-phase roles (reviewer_plan, reviewer_code, task_planner). Eight of 10 tests pass today; two (reviewer_refine, reviewer_agent_design loads) are documenter-dependency failures that flip green once task-1-4 lands the rubric .md files. TASK-1-3 (integration_tests/regression/test_bridge_flattened_round_trip.py): end-to-end round-trip test for the flattened bridge driver. Runs the real `bin/run_pipeline.py` in a fresh subprocess twice against a deterministic pipeline id: stage A captures the preflight HITLDecision into `pending_hitl.decision`, the test writes `answer="approve" + status="answered"`, stage B re-enters the process and replays the answer to advance to the refine-gate decision. Validates that the generator state survives via the contract-state round-trip across process exit. Substrate isolation via a `-c` shim that monkey-patches `orchestrator.substrate.select_substrate` to a MagicMock bundle — no real Claude Code / Anthropic API call. Also pins driver-side idempotency (a re-invocation without a new answer must not silently advance the generator). TASK-1-5 (integration_tests/regression/test_pretooluse_hook_nested.py): cq-5 early-spike R2 verdict test. Uses task-1-9's `_agent_tool_fake` to drive a deterministic nested-dispatch scenario: parent_role=architect + child_role=tester + write_target=orchestrator/foo.py — asserts the hook returns `{"decision": "block", "reason": ...}` and that the deny reason names the child (tester) role rather than the parent. Writes `.egg-state/<pipeline_id>/r2-verdict.json` with the pass verdict per AC. Adds in-role allow control + cross-role probe (parent=coder, child=tester writing orchestrator/* — must deny by the child's role) + dataclass shape pin + EGG_AGENT_ROLE leak guard. Docstring documents the empirical-vs-test-fake limitation cq-3 explicitly accepts (production stays on the harness re-host until slice-5 R15 flips dispatch). All 15 of 17 tests pass today. The 2 failing rubric tests are contracted documenter-dependency failures (task-1-4 not landed yet) and are expected to flip green once the documenter ships. Lint clean (ruff check + format). Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): address reviewer_code v1 NACK on SKILL.md envelope + CLI Reviewer_code NACKed v1 with three blockers against SKILL.md (the two rubric files and the ADR were ACKed as-is). This commit addresses all three blockers plus four non-blocking polish items. Blocking fixes in SKILL.md: 1. CLI invocation example was wrong (used --pipeline-id / --issue, but the driver at plugins/egg-sdlc/skills/egg-sdlc/bin/run_pipeline.py:355-402 takes pipeline_id as a positional arg and --issue-number as the flag). Rewrote the bash loop example to match the actual argparse signature so a copy-paste invocation actually runs. 2. The documented pending_hitl envelope was 5 fields (version, pipeline_id, timestamp, decision, answer) but the driver writes 9 (adds status, result, error, answer_log). Replaced the truncated schema with the full envelope and added per-field semantics (especially the status field, which is the skill's loop predicate: pending / answered / completed / aborted / error). The slice-3 daemon variant inherits all 9 fields. 3. No documented mechanism for the skill body to write pending_hitl.answer (the frontmatter allowed-tools does not include the Write tool). Documented option (a) from the reviewer's NACK: an inline python3 -c "..." invocation, which is covered by the existing Bash(python3 *:*) allowed-tool. The "skill loop" code block now demonstrates the round-trip with a case statement keyed on pending_hitl.status. Non-blocking polish in SKILL.md: - Loop semantics now name "replay" explicitly (the driver spawns a fresh generator and replays answer_log on every invocation; previous text suggested cheap single-step resumption). Added a dedicated "Generator state across invocations (replay semantics)" subsection naming the practical consequence — side effects re-run every call — and pointing at slice 3 as the daemon-variant escape hatch. - Failure-mode bullet for "pending_hitl.decision == null" replaced with the more general "pending_hitl.status ∈ {completed, aborted, error}" bullet so each terminal state has documented diagnostic guidance. Non-blocking polish in docs/architecture/claude-code-substrate.md: - The "Flattened bridge" bullet in the in-process orchestrator section now names the replay path explicitly (promotes answer → answer_log, replays the full log every call, deterministic same-yield-boundary property) and the 5→9 envelope field list mirrors SKILL.md. - Daemon-variant bullet enumerates the same 9-field shape so reviewers comparing slice-1 and slice-3 against the ADR see the full contract. - Schema source-of-truth pointer added to bin/run_pipeline.py:20-46 so future drift triggers fail in one place. Rubric files (reviewer_refine.md, reviewer_agent_design.md) are unchanged — reviewer_code ACKed them in v1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test_bridge_flattened_round_trip: fix subprocess PYTHONPATH The subprocess shim's PYTHONPATH pointed at `<repo>/orchestrator/` and `<repo>/shared/` directly, which lets the subprocess `import substrate` but NOT `import orchestrator.substrate` — the latter requires the *parent* of `orchestrator/` (the repo root) on the path because ``orchestrator/__init__.py`` makes it a real package. Set PYTHONPATH to ``<repo>/shared`` + ``<repo>`` + ``<repo>/gateway`` so: - ``<repo>/shared`` lets ``egg_contracts`` (transitive import from ``orchestrator.substrate.k3s_adapter``) resolve. - ``<repo>`` lets ``import orchestrator`` resolve. Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester v2: fix subprocess PYTHONPATH + non-blocking improvements Addresses reviewer_code v1 blocking #1 + non-blocking items: BLOCKING FIX — subprocess PYTHONPATH: Add `<repo>/orchestrator` to the subprocess shim's PYTHONPATH so bare- name top-level imports inside the `orchestrator/` tree resolve cleanly. Without this, `orchestrator/models.py:16` (`from slice_id_validation import SLICE_ID_PATTERN`) and `in_process.py:531-534`'s bare `from models import HITLDecision` fallback both fail, crashing the driver subprocess with `ModuleNotFoundError` before it yields the first HITL decision. Mirrors the Makefile's `PYTHONPATH := shared:gateway: orchestrator` (test target). Verified: tests now pass with `PYTHONPATH=.:shared:orchestrator pytest <files>` (reviewer_code's exact reproduction env). NON-BLOCKING (reviewer_code v1): - test_bridge_flattened_round_trip.py: drop stale "whichever the coder picks" docstring phrasing — driver locked in positional argv[1]. - test_bridge_flattened_round_trip.py: mirror the driver's ISO-8601 UTC timestamp format in _write_answer instead of `str(time.time())`. - test_rubric_loader.py: extend `test_loader_accepts_enum_and_string_role` parametrization to cover the two NEW roles (reviewer_refine, reviewer_agent_design) — not just the regression role. - test_rubric_loader.py: strengthen path-traversal assertion to verify the allowlist's slice-fence branch fires (not the file-missing-on- disk branch) — pinning the structural defence. - test_pretooluse_hook_nested.py: derive the r2-verdict.json content from the dispatch outcome and write it BEFORE the structured assertions so slice-5 sees an accurate empirical record even when a regression fails one of the assertions. Adds {"r2_verdict": "fail", "reason": ...} payload format for the failure path. Tests verified: 21/21 pass under both the canonical Makefile PYTHONPATH shape and reviewer_code's `.:shared:orchestrator` reproduction shape. Refs #2717 (slice-1 tester, v2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-1 (#2548) * docs(#2717 slice-2): plan-team rubrics + SKILL.md plan-phase section Land the four plan-team agent rubric files under plugins/egg-sdlc/skills/egg-sdlc/agents/ for the claude-code substrate of the egg SDLC pipeline (task-2-3): architect, task_planner, risk_analyst, reviewer_plan. Each rubric mirrors its k3s-substrate counterpart in plugins/refine-plan/skills/refine-plan/agents/ for body content (the substrate swap is structurally invisible to the role) and follows the reviewer_refine.md / reviewer_agent_design.md shape from slice-1 for the substrate-specific notes (worktree layout, PreToolUse hook enforcement, HITL-via-AskUserQuestion, concurrent peers in this slice, output path stability). Update plugins/egg-sdlc/skills/egg-sdlc/SKILL.md (task-2-7): - Bump the rollout-status callout from "slice 1 landed" to "slices 1 + 2 landed"; enumerate both the refine and plan rosters. - Replace the "What's NOT in this skill > Plan / implement / pr" bullet's plan deferral with a dedicated **Plan phase** subsection naming the four roles, their spawn order (architect solo, then task_planner + risk_analyst concurrently, with reviewer_plan ACK/NACK on each producer edge), output paths, and the four standard plan-HITL gate options (approve / request_changes / change_approach / stop). - Bump step 8 (phase fence) into a 10-step flow that walks the plan stage spawn order and the plan-HITL gate. The fence now triggers on "approve and continue to implement" with a pointer to slice 3. - Refresh stale "refine-only" / "refine-team subagents" / artifact-path and failure-mode strings to cover both phases. * slice-2 coder: plan-phase BRC stage + rubric loader expansion (#2717) Implements TASK-2-1 + TASK-2-2 for slice-2 of the #2717 rollout. TASK-2-5 closes as no-op per slice-1's R2 = pass verdict (the PreToolUse hook resolves the child's role correctly under nested dispatch; structural enforcement stays hook-side, no MCP-validator-side parallel layer needed). TASK-2-1 — `_run_plan_phase` on `_InProcessOrchestrator` ======================================================== After the refine HITL gate's `approve_continue` answer, the in-process generator now dispatches the plan phase: a `ThreadPoolExecutor` spawns architect / task_planner / risk_analyst concurrently through the same `ClaudeCodeSpawner` the refiner uses, then reviewer_plan is dispatched once with the producer artifacts as its input. `PeerConsensusTracker` drives the BRC mechanics (`handle_propose` / `handle_ack` / `handle_confirmed`); after consensus the stage yields a plan-HITL gate (`HITLDecision` with `phase="plan"` and the canonical 4-way options). The walking-skeleton fence still fires on `approve_continue` past the plan gate — its diagnostic now points at slice-3 / slice-4 of the #2717 rollout instead of #2623. Why the orchestrator records BRC transitions on the subagents' behalf: the in-process substrate's spawner is synchronous (returns AFTER the agent finishes). In the production HTTP daemon the subagents would emit `egg-orch consensus propose/ack/confirmed` themselves and the daemon's gateway listener would advance the tracker. In-process the spawn-completion IS the signal that the subagent proposed or reviewed, so the orchestrator drives the BRC transitions deterministically — the test (harness-faked subagents that never emit BRC messages) and production (real harness agents whose emissions would be no-op duplicates in this path) both reach CONSENSUS_CONFIRMED on the same code path. TASK-2-2 — `_load_egg_sdlc_role_rubric` extension ================================================== `_RUBRIC_LANDED_ROLES` now includes architect / task_planner / risk_analyst / reviewer_plan alongside the slice-1 refine roster (refiner + reviewer_refine + reviewer_agent_design). The structured- error contract for unshipped roles is preserved: implement-team roles (coder / tester / documenter + 5 reviewers) still raise `ValueError` with a slice-3 pointer. The "missing on disk" fallback diagnostic mentions both TASK-1-4 (slice-1 refine) and TASK-2-3 (slice-2 plan) so a reviewer hitting the error in a re-run knows which documenter task needs to land first. TASK-2-5 — agent-side restriction enforcement (no-op) ====================================================== Slice-1's `test_pretooluse_hook_denies_nested_child_write` confirmed the PreToolUse hook denies a child write outside the child's role under nested dispatch (R2 = pass, recorded in `.egg-state/<pipeline_id>/r2-verdict.json` when the test runs). Per the contingent task description, no `sandbox/egg_agent_tools/handlers/restrictions.py` change is needed; structural enforcement stays hook-side. Tester's TASK-2-6 becomes a regression guard asserting the validator helper is a no-op for in-allow-list writes — handled in tester's slice-2 commit. Smoke (manual, in-process, fake subagents) ========================================== * preflight → refine gate → plan gate sequence yields the expected decisions; spawner is called 5 times (1 refiner + 3 plan producers + 1 plan reviewer); tracker.evaluate() reports is_complete=True with all 4 plan-team agents in CONFIRMED state. * Terminal answer at refine gate (e.g. "stop") still returns the refine artifact path — plan phase is NOT entered. * `approve_continue` at the plan gate still raises `NotImplementedError` with the slice-3 / slice-4 pointer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-2 tester: plan-phase BRC E2E + R2-pass restrictions no-op (#2717) TASK-2-4 — integration_tests/regression/test_inprocess_plan_brc.py ================================================================== Plan-phase in-process BRC end-to-end test covering the four AC bullets: * boots `run_pipeline_in_process` against a deterministic pipeline id with harness-faked subagents (no real Anthropic / Claude Code spawn); * advances past the refine HITL gate via `approve` → `approve_continue`; * asserts the plan stage spawns 3 producers (architect, task_planner, risk_analyst) + 1 reviewer (reviewer_plan) — observed via the fake spawner's `.call_args_list`; * asserts the BRC mechanics reach CONSENSUS_CONFIRMED on every producer edge (architect → reviewer_plan, task_planner → reviewer_plan, risk_analyst → reviewer_plan) by reading `_plan_tracker.evaluate()` — the in-process analogue of bus-side CONSENSUS_CONFIRMED messages (the coder's TASK-2-1 implementation drives `PeerConsensusTracker.handle_propose/handle_ack/ handle_confirmed` deterministically since the substrate's spawner is synchronous); * asserts the plan-HITL decision is yielded with `phase="plan"`, `decision_type="phase_gate"`, non-empty `id` / `question` / `options`. Adversarial probing layered on top: * plan stage MUST NOT run when the operator answers `stop` at the refine gate — a regression that fanned into plan on any non-continue answer would burn three unauthorised subagent spawns; * plan stage MUST NOT spawn implement-phase roles — pins the negative invariant against a misrouted `_PHASE_ROLES` lookup; * refiner is spawned exactly once — pins the single-refiner-spawn invariant against an off-by-one role iteration; * every plan-phase spawn carries `EGG_PHASE=plan` in its env — pins the env-propagation contract so spawned subagents see the right phase. The test skips gracefully when the coder's `_run_plan_phase` is absent (scaffold-first per the role's guidance); 7/7 pass against the coder's slice-2 commit 3a46689. TASK-2-6 — tests/sandbox/egg_agent_tools/test_restrictions_validator.py ======================================================================= Contingent test per slice-1's R2 verdict = `pass`. Per the contract task-2-5 description, "If R2 = pass, this task is a no-op (close with note). Tests for this code path land in TASK-2-6 (tester-owned)." Tests for this code path land here as a **no-op regression guard**: * in-allow-list response shape stable (coder/orchestrator, tester/ tests, documenter/docs) — pins the documented gateway-shape fields `{ok, role, path, can_write, reason, alternative_role}` exactly; * cross-role denial shape stable — pins `can_write=False`, denial `reason` references `shared/egg_restrictions/patterns.py`, `alternative_role` names the single producer that can write; * no new validator symbol — asserts `validate_write_target` (and peers) are NOT present on the restrictions handler module, since R2 = pass meant the cq-6 option-2 enforcement work should NOT have landed; * defensive probes — missing `path` raises HandlerError, unknown role raises HandlerError, list-shaped path returns per-path results with documented shapes. 9/9 pass against the unchanged restrictions handler (no slice-2 source edits in `sandbox/egg_agent_tools/handlers/restrictions.py`). Configured-check results: * ruff check . — PASS (all checks passed) * ruff format check . — FAILS on `orchestrator/substrate/in_process.py` (coder's TASK-2-1 file, 5 long-call sites need re-formatting). My test files pass format check cleanly. This is being NACKed to the coder; my proposal will follow once they push the format fix. * mypy on tester-authored files — PASS (251 source files OK). * Custom checks (scripts/check-*.py) — all 13 pass. * `make lint` / `make test` / `make security` cannot complete in this sandbox: the venv sync fails when uv tries to download pinned wheels (flask, oauthlib) — the wheels.pythonhosted.org TLS chain is "UnknownIssuer" inside the sandbox image (same env constraint the slice-1 tester hit). Tests + lint + custom checks were exercised directly via system pytest / ruff / mypy with the Makefile's canonical `PYTHONPATH := shared:gateway:orchestrator`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-2 coder v2: address NACK blockers (#2717) Addresses 3 NACK reviewers from v1 (commit 3a46689): reviewer_concurrency NACKs: - C1: removed `_write_active_role_sentinel` from `_spawn_plan_producer`'s concurrent path. Each producer carries `EGG_AGENT_ROLE` in its own spawn env (the load-bearing role-resolution channel under concurrent dispatch); the single-valued sentinel cannot disambiguate three concurrent role-holders. The synchronous `_spawn_plan_reviewer` retains the sentinel write because it never overlaps another spawn. - C2: added `self._current_phase` state on `_InProcessOrchestrator` (default "refine"; flipped to "plan" at the top of `_run_plan_phase`). `_publish_heartbeat` reads from it so HEARTBEAT messages carry the right phase across the refine→plan transition. Without this, stuck-phase-transition watchdogs filtering by `phase` would see "refine" while the plan stage is actively running. reviewer_code_holistic NACKs: - H1: architect-first then fanout. `_run_plan_phase_inner` now spawns architect synchronously first, records its CONSENSUS_PROPOSE on the tracker, then fans out task_planner + risk_analyst concurrently through a ThreadPoolExecutor with max_workers=2. The architect's per-role output path is passed into each downstream producer's spawn env (`EGG_ARCHITECT_OUTPUT_PATH`) and prompt_text so they can read its `key_design_decisions` rather than re-deriving them. This matches the role-dependency declarations at `shared/egg_contracts/agent_roles.py:398/422` (TASK_PLANNER_ROLE / RISK_ANALYST_ROLE both list ARCHITECT as their sole dependency) and the architect / task_planner / risk_analyst rubric bodies the documenter shipped. - H2: reviewer_plan verdict-JSON parsing. New helpers `read_plan_reviewer_verdicts` (parses `.egg-state/agent-outputs/<issue>-reviewer_plan-output.json`) and `_apply_reviewer_verdicts` drive per-edge ACK / NACK on the tracker based on the reviewer's actual verdict rather than the exit-code-only heuristic v1 used. Fail-closed when the verdict file is missing AND the reviewer's spawn failed (NACK every edge); optimistic ACK only when the verdict file is missing AND the reviewer's spawn returned exit 0 (harness-faked test path), with the "verdict-not-parsed" status surfaced in the placeholder body so the operator sees the discrepancy at the HITL gate. tester NACK: - T1: ran `ruff format` on the affected files. `_spawn_plan_reviewer` also dropped the dead `EGG_PRODUCER_ARTIFACT_PATHS` env var (reviewer_code_holistic v1 non-blocking #3) in favor of per-role `EGG_<ROLE>_OUTPUT_PATH` env vars that the reviewer_plan rubric actually consumes. Non-blocker polish landed alongside the blockers: - `_synthetic_commit_for(role)` derives a per-role hex SHA so the three concurrent ProposalPayload entries remain commit-distinguishable in the tracker (reviewer_concurrency v1 NB #2). - Tracker-guard rejections (`handle_propose` / `handle_ack` / `handle_nack` / `handle_confirmed`) now log via `logging.getLogger("orchestrator.substrate.in_process").warning` instead of silent `except Exception: pass` (reviewer_code_holistic v1 NB). - `_format_plan_placeholder` now also renders reviewer_plan diagnostics + verdict-parsing status (reviewer_code_holistic v1 NB). File decomposition: - ruff format expanded the v1 diff to 1879 lines, breaching the 1500-line hard cap in `scripts/file-size-allowlist.yaml`. Extracted the plan-phase body (~700 lines) into `orchestrator/substrate/_plan_phase.py` as module-level functions that take the `_InProcessOrchestrator` instance as their first argument. The class's `_run_plan_phase` / `_spawn_plan_producer` / `_spawn_plan_reviewer` / `_plan_producer_output_path` / `_read_plan_reviewer_verdicts` methods stay on the class as thin delegates so the existing test surface (and tester's 16 passing tests against v1) keeps the same method names. `in_process.py` now lands at 1093 lines (under both caps); `_plan_phase.py` at 680 lines. Manual in-process smoke (harness fakes, MagicMock subagents): - Happy path: preflight → refine gate → plan gate; spawner called 5 times in order [refiner, architect, task_planner|risk_analyst, task_planner|risk_analyst, reviewer_plan]; tracker reaches `is_complete=True`. - Refine stop: returns refine artifact path; spawner called 1 time (no plan dispatch). - Mixed verdict: with a per_producer verdict JSON {architect:ACK, task_planner:NACK, risk_analyst:ACK}, the tracker records the NACK on task_planner → reviewer_plan; `is_complete=False`; blocking_agents includes reviewer_plan (unresolved critical NACK) and task_planner (not fully ACKed). - Fail-closed: with reviewer spawn exit_code=1 and no verdict file, the tracker NACKs every critical edge; risk_analyst (advisory edge) still confirms; reviewer_plan blocks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Address slice-1 review: fix install path, bridge answer-write, silent fallbacks Reviewer findings from PR #2724 (egg-reviewer slice-1 review): Blockers (1-4): * The documented `pip install -r requirements.txt` failed — no top-level requirements.txt exists. Switch SKILL.md, plugin.json's `egg.install_instructions`, and `bin/preflight.py` to `pip install .` against pyproject.toml. * SKILL.md claimed Python 3.11+ but pyproject.toml requires >=3.14. Update SKILL.md and preflight to reflect the real floor; correct the matching docstring in orchestrator/substrate/__init__.py. * The inline `python3 -c "..."` answer-write snippet in SKILL.md was broken (shell-interpolated `${ANSWER}` → NameError, deprecated `datetime.utcnow`, non-atomic write). Replace with a dedicated `bin/write_answer.py` helper that reads the JSON-encoded answer from stdin, uses `datetime.now(UTC)`, and writes atomically via tmp + `os.replace`. SKILL.md's loop now invokes the helper. * `_serialise_decision` silently swallowed `model_dump` failures, and `_read_contract` silently overwrote unparseable contracts (dropping `answer_log`). Log to stderr on serialisation fallbacks; refuse to overwrite a corrupted contract and exit 1 instead. Non-blocking (5-11): * `orchestrator/substrate/__init__.py` docstring rewritten to reflect reality: Python 3.14+ introduced PEP 758 (`except A, B:` without parens); ruff under py314 target strips redundant parens, hence `# fmt: skip` on multi-except lines. * Stderr warning in `_advance_generator`'s `finally` so a teardown failure inside `generator.close()` is at least observable. * `allowed-tools` tightened from `Bash(python3 *:*)` to `Bash(python3 plugins/egg-sdlc/skills/egg-sdlc/bin/*:*)` — the two helper scripts are the entire Python surface the skill can invoke. * Re-spawn cost note expanded in SKILL.md: each driver invocation re-runs every prior subagent spawn (real Anthropic API spend); slice-2 compounds to 8 spawns at the final stage. * `_RUBRIC_LANDED_ROLES` registry collapsed into `_CURRENT_LOADER_SLICE` + filesystem probe; same diagnostics, no parallel registry that can drift from disk state. * Abort vocabulary exported as `ABORT_ANSWERS` from `orchestrator.substrate.in_process`; the driver imports it (with a literal fallback when the orchestrator package is unimportable) so the driver, orchestrator, and slice-3 daemon share a single source of truth. * R2 caveat surfaced as an explicit open question for slice-5 sequencing in SKILL.md. Tests: * New `shared/tests/test_write_answer.py` (6 tests, all passing) pins the JSON-encoding round-trip, the timestamp format match against the driver, the atomic-write contract, and the corrupted-contract refusal. * Existing `test_rubric_loader`, `test_run_pipeline_in_process*`, `test_substrate_interfaces`, `test_bridge_flattened_round_trip`, and `test_pretooluse_hook_nested` all still pass. * The pre-existing `test_empty_diff_subprocess_skips_pytest` failure in `tests/tools/` reproduces against HEAD without these changes (detached-HEAD worktree edge case in the test selector). Authored-by: egg * slice-2 coder v4: support rubric-default single-verdict JSON schema (#2717) Addresses reviewer_code_holistic v3 NACK blocker H3 — the rubric the documenter shipped (plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_plan.md "Verdict JSON shape", lines 57-80) documents a single top-level verdict object (verdict ∈ {ACK, NACK}, analysis carrying the eight criteria, feedback blob, artifact_references), not the per_producer wrapper v2/v3's parser expected. A rubric-following reviewer's NACK would silently fall into the "verdict file present but no parseable per_producer entries" branch and the orchestrator's optimistic-ACK fallback would mask the NACK from the operator at the plan-HITL gate. v4 makes `read_plan_reviewer_verdicts` accept BOTH schemas: 1. Rubric-default single-verdict (broadcast). When the JSON's top-level `verdict` is "ACK" or "NACK", the verdict is broadcast to every plan producer edge — ACK acks all three, NACK nacks all three with `feedback` propagated as the per-edge `reason` (a synthetic placeholder fires if `feedback` is empty so the tracker's NACK guard doesn't reject the payload). This is "Option (c)" from the v3 NACK; per-edge granularity is lost but the rubric's "ACK only if every criterion passes" semantic IS preserved. 2. Per-producer extension (per-edge). The existing per_producer wrapper still takes precedence when present and well-formed. Reviewers that want explicit edge granularity (ACK architect + NACK task_planner) write the wrapper; the rubric's default shape stays broadcast-compatible. The function now takes an optional `plan_producers` kwarg so the caller (the in-process orchestrator) can broadcast the single verdict to the right role set. The `_read_plan_reviewer_verdicts` class method delegate also propagates the kwarg so tester-side tests that call the method retain their access pattern. Smoke (manual, in-process, MagicMock subagents): - Rubric-default single-verdict NACK: tracker NACKs architect + task_planner (critical edges), risk_analyst still confirms (advisory), reviewer_plan blocks consensus. is_complete=False; blocking_agents=['architect', 'task_planner', 'reviewer_plan']. - Rubric-default single-verdict ACK: every edge confirmed; is_complete=True. - per_producer wrapper still works: mixed ACK/NACK applied per edge. - Harness-fake path (no verdict file, reviewer exit 0): optimistic ACK preserved so tester's existing 16 passing tests keep their access pattern. - Fail-closed path (no verdict file, reviewer exit non-zero): critical edges NACK'd (unchanged from v2/v3). ruff format + ruff check + file-size lint all pass. `_plan_phase.py` is 747 lines; `in_process.py` 1095 lines. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-2 (#2548) * Move skill-loop python3 -c calls into bin/ helpers Address review feedback on PR #2724: - Add bin/read_status.py and extend write_answer.py with --answer-string so every subcommand in SKILL.md's loop body is a single `python3 plugins/.../bin/<helper>.py` invocation. Honors the tightened allowed-tools pattern `Bash(python3 plugins/.../bin/*:*)` per Claude Code's compound-command permission rules — no separate `Bash(python3 -c *)` or `Bash(printf *)` rule needed, no prompt-injection door left open. - Update SKILL.md step 4 to name bin/write_answer.py directly (matches the new loop body). - Replace `slice_hint != _CURRENT_LOADER_SLICE` rubric-loader fence with `slice_hint not in _LANDED_SLICES` (frozenset) so future slices extend rather than replace the landed set — slice-2 won't fence off slice-1's already-shipped refine roles. - Wire test_bridge_flattened_round_trip's _write_answer through subprocess(write_answer.py) so the integration test exercises the production write path end-to-end. - Add test_read_status.py (7 tests) and --answer-string coverage in test_write_answer.py (2 tests). * Update slice-1 rubric loader tests to match slice-2's loader expansion Slice-1's recent tester commits (831239d / 601df90) added tests pinning 'architect raises ValueError' and 'reviewer_plan/task_planner deferred'. Slice-2's loader extension to the plan team (task-2-2 + task-2-3) makes those roles loadable, so the slice-1 tests fail after the merge. This commit aligns the tests with slice-2's loader reality: - Replace test_load_architect_raises_value_error_with_slice2_hint with test_load_architect_rubric, mirroring the slice-1 success-path tests. - Remove REVIEWER_PLAN and TASK_PLANNER from test_loader_still_rejects_unshipped_roles parameters; keep REVIEWER_CODE (slice-3, still deferred). - Refresh module docstring to reflect architect-loads (vs architect-raises). * Address non-blocking review notes on PR #2724 - write_answer.py: clarify --answer-string docstring — the JSON encoding happens at contract serialisation time (json.dumps(contract)), not as a separate json.dumps(answer) step. Reference the special-characters test as the proof of the round-trip. - SKILL.md / read_status.py: document the case statement's intentional fall-through on empty STATUS. read_status.py prints empty + exit 0 when no pending_hitl envelope exists; the case has no *) arm, so the empty value falls through, the case exits 0, and the outer iteration re-invokes run_pipeline.py — which is the recover path. - test_rubric_loader.py: add test_landed_slices_contains_slice1 to mechanically pin the 'extend, don't replace' invariant on _LANDED_SLICES so a future slice cannot silently regress slice-1 by writing frozenset({'slice-2'}) instead of frozenset({'slice-1', 'slice-2'}). * Address slice-2 review: phase plumbing, doc drift, defensive checks Addresses reviewer_code feedback on PR #2726 (#2717 slice-2): B1 (blocking): thread `phase` through `_write_pending_decision` and `current_phase` so plan-gate decisions persist with `phase: "plan"` instead of the hardcoded `"refine"` left over from the spike. Regression test pins the persisted-vs-yielded phase invariant. B2 (blocking) + N1 + N2 + N3 (SKILL.md doc drift): - Replace plain `approve` with the canonical `approve_continue` so operators following the docs trip the fence instead of silently completing. - Trim overclaim that slice-2 implements `request_changes` / `change_approach` re-spawn loops (it doesn't — they're surfaced but treated as stop). - Document the failure-path `retry` / `abort` option set. - Update the NotImplementedError quote to match the actual raise. N4: delete dead `_SYNTHETIC_PLAN_COMMIT` (no callers — real producers route through `synthetic_commit_for(role)`); fold the "never escape this constant" caveat into `synthetic_commit_for`'s docstring. N5: document the `per_producer` extension shape in `reviewer_plan.md` so reviewers who need per-edge granularity have the documented opt-in instead of guessing. N6: drop unused `pre_merge_condition` plumbing from the plan-phase verdict reader — pre-merge conditions are a PR-merge concept with no consumer in plan-phase. N7 + N8: unlink `<contract>.lock` after the critical section and bound `fcntl.flock` with `LOCK_EX | LOCK_NB` + a 30 s retry deadline so crashed lock-holders surface as `BlockingIOError` instead of hanging the orchestrator forever. N9: defensive `architect_output_path.is_file()` check before the downstream fan-out; surface the broken handoff as a NACK on the architect edge so the operator sees the partial state at the plan-HITL gate instead of debugging chained downstream errors. N10: clear the active-role sentinel at the start of the plan phase so the PreToolUse hook's fallback path doesn't resolve concurrent plan-producers to the stale `refiner` role. N11: drop the misleading `patch.object(restrictions, "get_agent_role", ...)` in `test_unknown_role_raises_handler_error` — `check_file_restriction` short-circuits on the truthy `req["role"]` so the patch never fired; the test still pins the real invariant without the misleading scaffolding. * Address slice-2 v2 review: NB1-NB4 (N9 fail-fast, flock pattern) --------- Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: James Wiesebron <jameswiesebron@khanacademy.org>

* Initialize SDLC contract for issue #2717 * refine: analysis for #2717 substrate-swap follow-up rollout Surface 7 multi-choice decisions (cq-1..cq-7) and 6 open-ended feedback questions covering the bridge-gap design, slice decomposition, Agent- tool dispatcher migration, R15 model-(b) timing, R2 hook validation timing, cost cap default, and k3s adapter scope. * Persist agent statefile writes before refine sync * Persist statefiles after refine phase * Persist HITL resolution after refine phase gate * plan: architect analysis for #2717 substrate-swap follow-up rollout Maps refine-phase HITL decisions (cq-1 through cq-7 + feedback Q1-Q6) onto concrete component changes across 5 slices: 1. Bridge gap closure (Option B stage-script MVP) + R2 hook role-resolution spike (2-subagent worked example). 2. Plan-phase substrate (architect/task_planner/risk_analyst + reviewer_plan). 3. Implement-phase substrate (coder/tester/documenter + 5 reviewers; cq-3 empirical metrics collected here). 4. PR-phase substrate + 5x2 conformance matrix + scope-fence removal. 5. Parallel hardening: EggHarnessSpawner + local-run CLI, cost cap (EGG_PIPELINE_MAX_AGENT_INVOCATIONS=200), drop v0.x markers, ADR refresh. Includes runtime-primitive surfacing per #2594: every cited primitive has file:line evidence and is tagged with purpose (deployed-pod vs test-only) and execution context (in-sandbox-agent vs trusted-CI-runner). * plan: risk assessment for #2717 substrate-swap follow-up rollout Adds risk_analyst output (.egg-state/agent-outputs/2717-risk_analyst-output.json) covering 18 risks (R17–R34) specific to the post-spike rollout that wires the remaining 15 roles + plan/implement/pr phases onto the Claude Code substrate. Key risks called out: - R17: HITL bridge dual-architecture (cq-1 Option C-hybrid) - R18: 15-rubric authorship + structural depth-gap closure - R19/R29: 8-way harness re-host stress on parent session (cq-3 deferred) - R20: existing reviewer rubrics need substrate-aware extension (Q5 declined) - R21: 5-issue conformance reproducibility (Q1 fixed set) - R22: #2261 slice-15 coordination - R23: cost-cap at 200 (cq-6) needs visibility - R26: EggHarnessSpawner as 3rd protocol implementation (Q3 Option A) - R27: MCP-validator fallback structural enforcement gap - R31: 15-subagent trust-context scaling (Q4 declined extras) - 11 implementation recommendations with priorities + open questions for implement-phase HITL. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * plan: 5-slice DAG for #2717 substrate-swap follow-up rollout Decompose the rollout into the phase-sequential chain settled by the refine HITL (cq-2 = Option 3): slice-1 (bridge gap + R2 hook validation + refine reviewers) -> slice-2 (plan-phase substrate) -> slice-3 (implement-phase substrate + daemon HITL bridge) -> slice-4 (pr-phase + 5-issue conformance matrix + scope-fence removal) -> slice-5 (hardening: cost cap + EggHarnessSpawner + R15 contingent + fork primitive + ADR + v0.x marker drop) Each slice has exactly one DAG parent (forest constraint per #2137 satisfied). 42 tasks across the five slices; primitives audit per #2594 cites every named symbol with file:line or marks (NEW — TASK-X-Y). Trust-boundary scope is named: conformance tests live under integration_tests/regression/ (substrate-portable), not integration_tests/local_pipeline/ (kubectl-gated). * plan v2: address reviewer_plan v1 NACK (3 blockers + non-blockers) Blocking fixes: - TASK-1-5 (R2 spike): the harness re-host model bypasses the PreToolUse hook entirely (shared/egg_harness/client.py uses its own ToolRegistry.set_permission_callback, no hook_entry import). Add TASK-1-9 introducing a test-only nested-Agent-tool dispatch fake at integration_tests/regression/_agent_tool_fake.py (underscored helper => coder-owned per MCP file-restriction check). Reframe TASK-1-5 to use the fake; document the empirical- vs-test-fake limitation in the test docstring. Production stays on ClaudeCodeSpawner harness re-host per cq-3. - TASK-4-4 (conformance matrix): switch from "recorded transcripts that no task produces" to MagicMock-style stubs mirroring test_substrate_smoke.py:56. Document the trade-off in the test docstring and note that #2714's closed state is irrelevant per feedback Q1. - TASK-4-2 (fence removal): cite both :212 (call site) and :807-826 (method def) so the coder removes both, not just the call. Non-blocking fixes: - TASK-2-5: agent-side enforcement target moved from orchestrator/mcp_tools.py (wrong surface) to sandbox/egg_agent_tools/handlers/restrictions.py (the in-sandbox tool handler that exposes check_file_restriction at :70 today). - TASK-2-6 / TASK-2-7: renumbered to match file order. - TASK-1-6: explicit dependency note on TASK-1-4. - TASK-3-2: daemon must detach via start_new_session=True so it survives the calling Bash exit. - TASK-1-1: pending_hitl envelope marked as the shared state- serialization contract between Option B (flattened) and Option A (daemon), closing risk_analyst R17 dual-bridge concern. - TASK-5-5 fork primitive: stays on harness re-host (subprocess + egg_harness.run_agent) instead of Agent-tool dispatch, aligning with cq-3's "decide empirically post-implement" deferral. - Primitives table: LocalWorktreeManager line corrected to :59; _maybe_fence dual-location citation added. * Populate contract for 2717 (#2629) * Persist statefiles after plan phase * [slice-1] Roll out Claude Code substrate to remaining roles + plan/... (#2724) * docs: add claude-code substrate to index and structure docs [doc-updater] (#2718) * docs: add claude-code substrate to index and structure docs * docs: fix substrate/claude_code listing per reviewer feedback - Drop incorrect '+ Agent tool' from spawner.py description; the spike runs egg_harness.run_agent in-process and does NOT dispatch via the Agent tool (Agent-tool spawner is an ADR follow-up). - Add hook_entry.py to the listing — it is the standalone PreToolUse hook script and the largest file in the package (~31 KB). - Clarify policy.py is the PolicyEnforcer adapter wrapping hook_entry.py. - List settings.template.json for navigability. Addresses egg-reviewer CHANGES_REQUESTED on PR #2718. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: update deployment guide for Cilium portmap CNI changes [doc-updater] (#2716) * docs: document portmap CNI and iptables reboot requirement (#2713) * docs: mention netfilter-persistent as alternative to re-running after reboot Addresses non-blocking review feedback on #2716. The reviewer noted that install-cilium.sh's own comment block calls out netfilter-persistent / iptables-restore as a system-level persistence alternative to re-running the script after every reboot. Mirror that in the operator-facing doc so long-running k3s host operators know they have an option beyond manual re-runs. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: add reconcile_autostash_pop_conflict to push diagnostic list (#2720) Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> * slice-1 coder: bridge driver + R2 nested-dispatch fake + loader expansion Implements three #2717 slice-1 coder tasks toward the substrate-swap follow-up rollout per cq-1=Option C hybrid bridge (refine/plan flattened, implement daemon), cq-2 phase-sequential slicing, cq-5 early-spike R2 validation, and cq-3 harness re-host alignment. TASK-1-1 (bin/run_pipeline.py): flattened single-yield stage driver that advances `run_pipeline_in_process` to its next yield, serialises the yielded HITLDecision to .egg-state/contracts/<id>.json under a stable `pending_hitl` envelope schema (version, decision, answer, answer_log, status, result, error), and exits. Cross-process generator state is recovered by replaying `answer_log` on each invocation — viable for refine/plan because the generator is deterministic; slice-3's daemon variant (TASK-3-2) consumes the same envelope schema so the two bridges share a state-serialization contract (risk_analyst R17 mitigation). End-to-end round-trip verified: first invocation yields preflight decision; operator answer round-trips through the contract; second invocation replays and advances to the refine-gate decision. TASK-1-9 (integration_tests/regression/_agent_tool_fake.py): test- only nested-Agent-tool dispatch fake. Simulates Claude Code's Agent tool by spawning a child subprocess with controlled EGG_AGENT_ROLE; the child invokes orchestrator/substrate/claude_code/hook_entry.py `decide(...)` directly. Validates the hook-logic half of R2 — given accurate EGG_AGENT_ROLE propagation, does the hook deny a write that violates the *child's* role pattern even when the parent's role would allow it? Hard import guard prevents production use; the file is coder-owned (underscored helper name, mirroring _helpers.py) rather than tester-owned. Production dispatch stays on ClaudeCodeSpawner (harness re-host) per cq-3. TASK-1-6 (orchestrator/substrate/__init__.py): extends `_load_egg_sdlc_role_rubric` so reviewer_refine and reviewer_agent_design are recognised as supported (alongside the existing refiner). Introduces a `_ROLE_RUBRIC_SLICES` mapping that names which #2717 rollout slice ships each role's rubric (slice-1: refine team; slice-2: plan team; slice-3: implement team) so future slice loaders can extend the set declaratively, and a `_RUBRIC_LANDED_ROLES` set documenting which rubric .md files exist on disk today. Roles outside the landed set raise a structured ValueError citing the correct rollout slice ("deferred to follow-up slice-2 of issue #2717's rollout"). The acceptance criterion's "follow-up slice 2" hint for architect is now produced. Lint clean (ruff check + format); mypy clean on the new files; the pre-existing 6 mypy errors in substrate/__init__.py:180-198 are unrelated to this change. 52 existing claude-code-substrate tests still pass. Refs #2717 (slice-1 coder). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): refine-team rubrics + flattened-bridge docs + ADR rollout deltas Slice 1 of the #2717 substrate-swap rollout adds two refine-team reviewer rubrics, closes the heredoc-HITL bridge gap for refine-phase via a flattened bin/run_pipeline.py stage driver, and adds the cq-5 R2 spike for nested PreToolUse-hook role-routing. This commit lands the documenter-owned half: TASK-1-4: New reviewer rubric files at plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_refine.md plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_agent_design.md mirroring the layout of plugins/refine-plan/skills/refine-plan/agents/ so the in-process orchestrator's build_system_prompt(sources) loader picks them up without per-skill custom logic. Both files carry frontmatter (name + description) and the body documents the substrate-specific operational deltas (worktree path, PreToolUse-enforced restrictions, AskUserQuestion surfacing, verdict path) on top of the same rubric the k3s reviewers use. TASK-1-2: SKILL.md is updated to replace the "Walking-skeleton bridge gap" callout with a "How the flattened bridge works" section that names pending_hitl as the single-yield carrier and documents the skill→driver loop. The "What the skill is designed to do" step list moves from aspirational to actually-shipping. The R2 PreToolUse-hook section points at the new test infrastructure (test_pretooluse_hook_nested.py + _agent_tool_fake.py) and the r2-verdict.json file. Frontmatter description re-flows to reflect the slice-1 scope. TASK-1-8: docs/architecture/claude-code-substrate.md is updated for the ADR-level audit trail: - Title and status banner reframe from "spike" to "spike → rollout". - cq-2 / cq-7 / cq-11 table rows reflect what slice 1 lands. - The in-process orchestrator section gets a "The flattened bridge" subsection naming the cq-1 hybrid (Option C) and the slice-3 daemon variant that consumes the same pending_hitl envelope shape (R17 mitigation). - The egg-sdlc plugin section enumerates the three refine-team rubrics and the new bin/run_pipeline.py driver. - The R2 risk-mitigation subsection points at the slice-1 worked example and the slice-5 contingent fallback (cq-6 option 2 + R15 model (b)). - The R15 subsection makes the model (a) → (b) migration contingent on the slice-1 R2 verdict. - The "Open work" + "Follow-up issue draft" sections are replaced with a unified "Rollout deltas" section split into Completed-in-this-rollout (3 slice-1 items, marked with [x] + strikethrough on the obsolete text) and Pending-in-this-rollout (9 items mapped to slices 2-5). The acceptance bar is unchanged. - The primitives table picks up the four new slice-1 modules (bin/run_pipeline.py, _agent_tool_fake.py, test_pretooluse_hook_nested.py, the two new reviewer rubrics). - The conformance-proof section names the slice-1 regression-test additions (test_bridge_flattened_round_trip.py, test_rubric_loader.py). - Stale anchor links to the removed "Follow-up issue draft" section are redirected to the new "Rollout deltas" anchor. These doc changes satisfy TASK-1-2, TASK-1-4, and TASK-1-8 from slice 1 of the #2717 plan; no source or test files are touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester: rubric loader + bridge round-trip + R2 nested-dispatch tests Adds three test files covering #2717 slice-1's tester contract tasks: TASK-1-7 (shared/tests/test_rubric_loader.py): unit tests for `_load_egg_sdlc_role_rubric`. Pins the four AC cases: refiner regression, reviewer_refine load, reviewer_agent_design load, architect raises ValueError with the updated "follow-up slice 2" diagnostic. Adversarial probing layered on: AgentRole enum vs str input equivalence, defense against path-traversal role values, structured fence for unshipped plan-phase roles (reviewer_plan, reviewer_code, task_planner). Eight of 10 tests pass today; two (reviewer_refine, reviewer_agent_design loads) are documenter-dependency failures that flip green once task-1-4 lands the rubric .md files. TASK-1-3 (integration_tests/regression/test_bridge_flattened_round_trip.py): end-to-end round-trip test for the flattened bridge driver. Runs the real `bin/run_pipeline.py` in a fresh subprocess twice against a deterministic pipeline id: stage A captures the preflight HITLDecision into `pending_hitl.decision`, the test writes `answer="approve" + status="answered"`, stage B re-enters the process and replays the answer to advance to the refine-gate decision. Validates that the generator state survives via the contract-state round-trip across process exit. Substrate isolation via a `-c` shim that monkey-patches `orchestrator.substrate.select_substrate` to a MagicMock bundle — no real Claude Code / Anthropic API call. Also pins driver-side idempotency (a re-invocation without a new answer must not silently advance the generator). TASK-1-5 (integration_tests/regression/test_pretooluse_hook_nested.py): cq-5 early-spike R2 verdict test. Uses task-1-9's `_agent_tool_fake` to drive a deterministic nested-dispatch scenario: parent_role=architect + child_role=tester + write_target=orchestrator/foo.py — asserts the hook returns `{"decision": "block", "reason": ...}` and that the deny reason names the child (tester) role rather than the parent. Writes `.egg-state/<pipeline_id>/r2-verdict.json` with the pass verdict per AC. Adds in-role allow control + cross-role probe (parent=coder, child=tester writing orchestrator/* — must deny by the child's role) + dataclass shape pin + EGG_AGENT_ROLE leak guard. Docstring documents the empirical-vs-test-fake limitation cq-3 explicitly accepts (production stays on the harness re-host until slice-5 R15 flips dispatch). All 15 of 17 tests pass today. The 2 failing rubric tests are contracted documenter-dependency failures (task-1-4 not landed yet) and are expected to flip green once the documenter ships. Lint clean (ruff check + format). Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): address reviewer_code v1 NACK on SKILL.md envelope + CLI Reviewer_code NACKed v1 with three blockers against SKILL.md (the two rubric files and the ADR were ACKed as-is). This commit addresses all three blockers plus four non-blocking polish items. Blocking fixes in SKILL.md: 1. CLI invocation example was wrong (used --pipeline-id / --issue, but the driver at plugins/egg-sdlc/skills/egg-sdlc/bin/run_pipeline.py:355-402 takes pipeline_id as a positional arg and --issue-number as the flag). Rewrote the bash loop example to match the actual argparse signature so a copy-paste invocation actually runs. 2. The documented pending_hitl envelope was 5 fields (version, pipeline_id, timestamp, decision, answer) but the driver writes 9 (adds status, result, error, answer_log). Replaced the truncated schema with the full envelope and added per-field semantics (especially the status field, which is the skill's loop predicate: pending / answered / completed / aborted / error). The slice-3 daemon variant inherits all 9 fields. 3. No documented mechanism for the skill body to write pending_hitl.answer (the frontmatter allowed-tools does not include the Write tool). Documented option (a) from the reviewer's NACK: an inline python3 -c "..." invocation, which is covered by the existing Bash(python3 *:*) allowed-tool. The "skill loop" code block now demonstrates the round-trip with a case statement keyed on pending_hitl.status. Non-blocking polish in SKILL.md: - Loop semantics now name "replay" explicitly (the driver spawns a fresh generator and replays answer_log on every invocation; previous text suggested cheap single-step resumption). Added a dedicated "Generator state across invocations (replay semantics)" subsection naming the practical consequence — side effects re-run every call — and pointing at slice 3 as the daemon-variant escape hatch. - Failure-mode bullet for "pending_hitl.decision == null" replaced with the more general "pending_hitl.status ∈ {completed, aborted, error}" bullet so each terminal state has documented diagnostic guidance. Non-blocking polish in docs/architecture/claude-code-substrate.md: - The "Flattened bridge" bullet in the in-process orchestrator section now names the replay path explicitly (promotes answer → answer_log, replays the full log every call, deterministic same-yield-boundary property) and the 5→9 envelope field list mirrors SKILL.md. - Daemon-variant bullet enumerates the same 9-field shape so reviewers comparing slice-1 and slice-3 against the ADR see the full contract. - Schema source-of-truth pointer added to bin/run_pipeline.py:20-46 so future drift triggers fail in one place. Rubric files (reviewer_refine.md, reviewer_agent_design.md) are unchanged — reviewer_code ACKed them in v1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test_bridge_flattened_round_trip: fix subprocess PYTHONPATH The subprocess shim's PYTHONPATH pointed at `<repo>/orchestrator/` and `<repo>/shared/` directly, which lets the subprocess `import substrate` but NOT `import orchestrator.substrate` — the latter requires the *parent* of `orchestrator/` (the repo root) on the path because ``orchestrator/__init__.py`` makes it a real package. Set PYTHONPATH to ``<repo>/shared`` + ``<repo>`` + ``<repo>/gateway`` so: - ``<repo>/shared`` lets ``egg_contracts`` (transitive import from ``orchestrator.substrate.k3s_adapter``) resolve. - ``<repo>`` lets ``import orchestrator`` resolve. Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester v2: fix subprocess PYTHONPATH + non-blocking improvements Addresses reviewer_code v1 blocking #1 + non-blocking items: BLOCKING FIX — subprocess PYTHONPATH: Add `<repo>/orchestrator` to the subprocess shim's PYTHONPATH so bare- name top-level imports inside the `orchestrator/` tree resolve cleanly. Without this, `orchestrator/models.py:16` (`from slice_id_validation import SLICE_ID_PATTERN`) and `in_process.py:531-534`'s bare `from models import HITLDecision` fallback both fail, crashing the driver subprocess with `ModuleNotFoundError` before it yields the first HITL decision. Mirrors the Makefile's `PYTHONPATH := shared:gateway: orchestrator` (test target). Verified: tests now pass with `PYTHONPATH=.:shared:orchestrator pytest <files>` (reviewer_code's exact reproduction env). NON-BLOCKING (reviewer_code v1): - test_bridge_flattened_round_trip.py: drop stale "whichever the coder picks" docstring phrasing — driver locked in positional argv[1]. - test_bridge_flattened_round_trip.py: mirror the driver's ISO-8601 UTC timestamp format in _write_answer instead of `str(time.time())`. - test_rubric_loader.py: extend `test_loader_accepts_enum_and_string_role` parametrization to cover the two NEW roles (reviewer_refine, reviewer_agent_design) — not just the regression role. - test_rubric_loader.py: strengthen path-traversal assertion to verify the allowlist's slice-fence branch fires (not the file-missing-on- disk branch) — pinning the structural defence. - test_pretooluse_hook_nested.py: derive the r2-verdict.json content from the dispatch outcome and write it BEFORE the structured assertions so slice-5 sees an accurate empirical record even when a regression fails one of the assertions. Adds {"r2_verdict": "fail", "reason": ...} payload format for the failure path. Tests verified: 21/21 pass under both the canonical Makefile PYTHONPATH shape and reviewer_code's `.:shared:orchestrator` reproduction shape. Refs #2717 (slice-1 tester, v2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-1 (#2548) * Address slice-1 review: fix install path, bridge answer-write, silent fallbacks Reviewer findings from PR #2724 (egg-reviewer slice-1 review): Blockers (1-4): * The documented `pip install -r requirements.txt` failed — no top-level requirements.txt exists. Switch SKILL.md, plugin.json's `egg.install_instructions`, and `bin/preflight.py` to `pip install .` against pyproject.toml. * SKILL.md claimed Python 3.11+ but pyproject.toml requires >=3.14. Update SKILL.md and preflight to reflect the real floor; correct the matching docstring in orchestrator/substrate/__init__.py. * The inline `python3 -c "..."` answer-write snippet in SKILL.md was broken (shell-interpolated `${ANSWER}` → NameError, deprecated `datetime.utcnow`, non-atomic write). Replace with a dedicated `bin/write_answer.py` helper that reads the JSON-encoded answer from stdin, uses `datetime.now(UTC)`, and writes atomically via tmp + `os.replace`. SKILL.md's loop now invokes the helper. * `_serialise_decision` silently swallowed `model_dump` failures, and `_read_contract` silently overwrote unparseable contracts (dropping `answer_log`). Log to stderr on serialisation fallbacks; refuse to overwrite a corrupted contract and exit 1 instead. Non-blocking (5-11): * `orchestrator/substrate/__init__.py` docstring rewritten to reflect reality: Python 3.14+ introduced PEP 758 (`except A, B:` without parens); ruff under py314 target strips redundant parens, hence `# fmt: skip` on multi-except lines. * Stderr warning in `_advance_generator`'s `finally` so a teardown failure inside `generator.close()` is at least observable. * `allowed-tools` tightened from `Bash(python3 *:*)` to `Bash(python3 plugins/egg-sdlc/skills/egg-sdlc/bin/*:*)` — the two helper scripts are the entire Python surface the skill can invoke. * Re-spawn cost note expanded in SKILL.md: each driver invocation re-runs every prior subagent spawn (real Anthropic API spend); slice-2 compounds to 8 spawns at the final stage. * `_RUBRIC_LANDED_ROLES` registry collapsed into `_CURRENT_LOADER_SLICE` + filesystem probe; same diagnostics, no parallel registry that can drift from disk state. * Abort vocabulary exported as `ABORT_ANSWERS` from `orchestrator.substrate.in_process`; the driver imports it (with a literal fallback when the orchestrator package is unimportable) so the driver, orchestrator, and slice-3 daemon share a single source of truth. * R2 caveat surfaced as an explicit open question for slice-5 sequencing in SKILL.md. Tests: * New `shared/tests/test_write_answer.py` (6 tests, all passing) pins the JSON-encoding round-trip, the timestamp format match against the driver, the atomic-write contract, and the corrupted-contract refusal. * Existing `test_rubric_loader`, `test_run_pipeline_in_process*`, `test_substrate_interfaces`, `test_bridge_flattened_round_trip`, and `test_pretooluse_hook_nested` all still pass. * The pre-existing `test_empty_diff_subprocess_skips_pytest` failure in `tests/tools/` reproduces against HEAD without these changes (detached-HEAD worktree edge case in the test selector). Authored-by: egg * Move skill-loop python3 -c calls into bin/ helpers Address review feedback on PR #2724: - Add bin/read_status.py and extend write_answer.py with --answer-string so every subcommand in SKILL.md's loop body is a single `python3 plugins/.../bin/<helper>.py` invocation. Honors the tightened allowed-tools pattern `Bash(python3 plugins/.../bin/*:*)` per Claude Code's compound-command permission rules — no separate `Bash(python3 -c *)` or `Bash(printf *)` rule needed, no prompt-injection door left open. - Update SKILL.md step 4 to name bin/write_answer.py directly (matches the new loop body). - Replace `slice_hint != _CURRENT_LOADER_SLICE` rubric-loader fence with `slice_hint not in _LANDED_SLICES` (frozenset) so future slices extend rather than replace the landed set — slice-2 won't fence off slice-1's already-shipped refine roles. - Wire test_bridge_flattened_round_trip's _write_answer through subprocess(write_answer.py) so the integration test exercises the production write path end-to-end. - Add test_read_status.py (7 tests) and --answer-string coverage in test_write_answer.py (2 tests). * Address non-blocking review notes on PR #2724 - write_answer.py: clarify --answer-string docstring — the JSON encoding happens at contract serialisation time (json.dumps(contract)), not as a separate json.dumps(answer) step. Reference the special-characters test as the proof of the round-trip. - SKILL.md / read_status.py: document the case statement's intentional fall-through on empty STATUS. read_status.py prints empty + exit 0 when no pending_hitl envelope exists; the case has no *) arm, so the empty value falls through, the case exits 0, and the outer iteration re-invokes run_pipeline.py — which is the recover path. - test_rubric_loader.py: add test_landed_slices_contains_slice1 to mechanically pin the 'extend, don't replace' invariant on _LANDED_SLICES so a future slice cannot silently regress slice-1 by writing frozenset({'slice-2'}) instead of frozenset({'slice-1', 'slice-2'}). --------- Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-orchestrator <egg@localhost> * [slice-2] Roll out Claude Code substrate to remaining roles + plan/... (#2726) * docs: add claude-code substrate to index and structure docs [doc-updater] (#2718) * docs: add claude-code substrate to index and structure docs * docs: fix substrate/claude_code listing per reviewer feedback - Drop incorrect '+ Agent tool' from spawner.py description; the spike runs egg_harness.run_agent in-process and does NOT dispatch via the Agent tool (Agent-tool spawner is an ADR follow-up). - Add hook_entry.py to the listing — it is the standalone PreToolUse hook script and the largest file in the package (~31 KB). - Clarify policy.py is the PolicyEnforcer adapter wrapping hook_entry.py. - List settings.template.json for navigability. Addresses egg-reviewer CHANGES_REQUESTED on PR #2718. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: update deployment guide for Cilium portmap CNI changes [doc-updater] (#2716) * docs: document portmap CNI and iptables reboot requirement (#2713) * docs: mention netfilter-persistent as alternative to re-running after reboot Addresses non-blocking review feedback on #2716. The reviewer noted that install-cilium.sh's own comment block calls out netfilter-persistent / iptables-restore as a system-level persistence alternative to re-running the script after every reboot. Mirror that in the operator-facing doc so long-running k3s host operators know they have an option beyond manual re-runs. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> * docs: add reconcile_autostash_pop_conflict to push diagnostic list (#2720) Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> * slice-1 coder: bridge driver + R2 nested-dispatch fake + loader expansion Implements three #2717 slice-1 coder tasks toward the substrate-swap follow-up rollout per cq-1=Option C hybrid bridge (refine/plan flattened, implement daemon), cq-2 phase-sequential slicing, cq-5 early-spike R2 validation, and cq-3 harness re-host alignment. TASK-1-1 (bin/run_pipeline.py): flattened single-yield stage driver that advances `run_pipeline_in_process` to its next yield, serialises the yielded HITLDecision to .egg-state/contracts/<id>.json under a stable `pending_hitl` envelope schema (version, decision, answer, answer_log, status, result, error), and exits. Cross-process generator state is recovered by replaying `answer_log` on each invocation — viable for refine/plan because the generator is deterministic; slice-3's daemon variant (TASK-3-2) consumes the same envelope schema so the two bridges share a state-serialization contract (risk_analyst R17 mitigation). End-to-end round-trip verified: first invocation yields preflight decision; operator answer round-trips through the contract; second invocation replays and advances to the refine-gate decision. TASK-1-9 (integration_tests/regression/_agent_tool_fake.py): test- only nested-Agent-tool dispatch fake. Simulates Claude Code's Agent tool by spawning a child subprocess with controlled EGG_AGENT_ROLE; the child invokes orchestrator/substrate/claude_code/hook_entry.py `decide(...)` directly. Validates the hook-logic half of R2 — given accurate EGG_AGENT_ROLE propagation, does the hook deny a write that violates the *child's* role pattern even when the parent's role would allow it? Hard import guard prevents production use; the file is coder-owned (underscored helper name, mirroring _helpers.py) rather than tester-owned. Production dispatch stays on ClaudeCodeSpawner (harness re-host) per cq-3. TASK-1-6 (orchestrator/substrate/__init__.py): extends `_load_egg_sdlc_role_rubric` so reviewer_refine and reviewer_agent_design are recognised as supported (alongside the existing refiner). Introduces a `_ROLE_RUBRIC_SLICES` mapping that names which #2717 rollout slice ships each role's rubric (slice-1: refine team; slice-2: plan team; slice-3: implement team) so future slice loaders can extend the set declaratively, and a `_RUBRIC_LANDED_ROLES` set documenting which rubric .md files exist on disk today. Roles outside the landed set raise a structured ValueError citing the correct rollout slice ("deferred to follow-up slice-2 of issue #2717's rollout"). The acceptance criterion's "follow-up slice 2" hint for architect is now produced. Lint clean (ruff check + format); mypy clean on the new files; the pre-existing 6 mypy errors in substrate/__init__.py:180-198 are unrelated to this change. 52 existing claude-code-substrate tests still pass. Refs #2717 (slice-1 coder). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): refine-team rubrics + flattened-bridge docs + ADR rollout deltas Slice 1 of the #2717 substrate-swap rollout adds two refine-team reviewer rubrics, closes the heredoc-HITL bridge gap for refine-phase via a flattened bin/run_pipeline.py stage driver, and adds the cq-5 R2 spike for nested PreToolUse-hook role-routing. This commit lands the documenter-owned half: TASK-1-4: New reviewer rubric files at plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_refine.md plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_agent_design.md mirroring the layout of plugins/refine-plan/skills/refine-plan/agents/ so the in-process orchestrator's build_system_prompt(sources) loader picks them up without per-skill custom logic. Both files carry frontmatter (name + description) and the body documents the substrate-specific operational deltas (worktree path, PreToolUse-enforced restrictions, AskUserQuestion surfacing, verdict path) on top of the same rubric the k3s reviewers use. TASK-1-2: SKILL.md is updated to replace the "Walking-skeleton bridge gap" callout with a "How the flattened bridge works" section that names pending_hitl as the single-yield carrier and documents the skill→driver loop. The "What the skill is designed to do" step list moves from aspirational to actually-shipping. The R2 PreToolUse-hook section points at the new test infrastructure (test_pretooluse_hook_nested.py + _agent_tool_fake.py) and the r2-verdict.json file. Frontmatter description re-flows to reflect the slice-1 scope. TASK-1-8: docs/architecture/claude-code-substrate.md is updated for the ADR-level audit trail: - Title and status banner reframe from "spike" to "spike → rollout". - cq-2 / cq-7 / cq-11 table rows reflect what slice 1 lands. - The in-process orchestrator section gets a "The flattened bridge" subsection naming the cq-1 hybrid (Option C) and the slice-3 daemon variant that consumes the same pending_hitl envelope shape (R17 mitigation). - The egg-sdlc plugin section enumerates the three refine-team rubrics and the new bin/run_pipeline.py driver. - The R2 risk-mitigation subsection points at the slice-1 worked example and the slice-5 contingent fallback (cq-6 option 2 + R15 model (b)). - The R15 subsection makes the model (a) → (b) migration contingent on the slice-1 R2 verdict. - The "Open work" + "Follow-up issue draft" sections are replaced with a unified "Rollout deltas" section split into Completed-in-this-rollout (3 slice-1 items, marked with [x] + strikethrough on the obsolete text) and Pending-in-this-rollout (9 items mapped to slices 2-5). The acceptance bar is unchanged. - The primitives table picks up the four new slice-1 modules (bin/run_pipeline.py, _agent_tool_fake.py, test_pretooluse_hook_nested.py, the two new reviewer rubrics). - The conformance-proof section names the slice-1 regression-test additions (test_bridge_flattened_round_trip.py, test_rubric_loader.py). - Stale anchor links to the removed "Follow-up issue draft" section are redirected to the new "Rollout deltas" anchor. These doc changes satisfy TASK-1-2, TASK-1-4, and TASK-1-8 from slice 1 of the #2717 plan; no source or test files are touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester: rubric loader + bridge round-trip + R2 nested-dispatch tests Adds three test files covering #2717 slice-1's tester contract tasks: TASK-1-7 (shared/tests/test_rubric_loader.py): unit tests for `_load_egg_sdlc_role_rubric`. Pins the four AC cases: refiner regression, reviewer_refine load, reviewer_agent_design load, architect raises ValueError with the updated "follow-up slice 2" diagnostic. Adversarial probing layered on: AgentRole enum vs str input equivalence, defense against path-traversal role values, structured fence for unshipped plan-phase roles (reviewer_plan, reviewer_code, task_planner). Eight of 10 tests pass today; two (reviewer_refine, reviewer_agent_design loads) are documenter-dependency failures that flip green once task-1-4 lands the rubric .md files. TASK-1-3 (integration_tests/regression/test_bridge_flattened_round_trip.py): end-to-end round-trip test for the flattened bridge driver. Runs the real `bin/run_pipeline.py` in a fresh subprocess twice against a deterministic pipeline id: stage A captures the preflight HITLDecision into `pending_hitl.decision`, the test writes `answer="approve" + status="answered"`, stage B re-enters the process and replays the answer to advance to the refine-gate decision. Validates that the generator state survives via the contract-state round-trip across process exit. Substrate isolation via a `-c` shim that monkey-patches `orchestrator.substrate.select_substrate` to a MagicMock bundle — no real Claude Code / Anthropic API call. Also pins driver-side idempotency (a re-invocation without a new answer must not silently advance the generator). TASK-1-5 (integration_tests/regression/test_pretooluse_hook_nested.py): cq-5 early-spike R2 verdict test. Uses task-1-9's `_agent_tool_fake` to drive a deterministic nested-dispatch scenario: parent_role=architect + child_role=tester + write_target=orchestrator/foo.py — asserts the hook returns `{"decision": "block", "reason": ...}` and that the deny reason names the child (tester) role rather than the parent. Writes `.egg-state/<pipeline_id>/r2-verdict.json` with the pass verdict per AC. Adds in-role allow control + cross-role probe (parent=coder, child=tester writing orchestrator/* — must deny by the child's role) + dataclass shape pin + EGG_AGENT_ROLE leak guard. Docstring documents the empirical-vs-test-fake limitation cq-3 explicitly accepts (production stays on the harness re-host until slice-5 R15 flips dispatch). All 15 of 17 tests pass today. The 2 failing rubric tests are contracted documenter-dependency failures (task-1-4 not landed yet) and are expected to flip green once the documenter ships. Lint clean (ruff check + format). Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(#2717 slice-1): address reviewer_code v1 NACK on SKILL.md envelope + CLI Reviewer_code NACKed v1 with three blockers against SKILL.md (the two rubric files and the ADR were ACKed as-is). This commit addresses all three blockers plus four non-blocking polish items. Blocking fixes in SKILL.md: 1. CLI invocation example was wrong (used --pipeline-id / --issue, but the driver at plugins/egg-sdlc/skills/egg-sdlc/bin/run_pipeline.py:355-402 takes pipeline_id as a positional arg and --issue-number as the flag). Rewrote the bash loop example to match the actual argparse signature so a copy-paste invocation actually runs. 2. The documented pending_hitl envelope was 5 fields (version, pipeline_id, timestamp, decision, answer) but the driver writes 9 (adds status, result, error, answer_log). Replaced the truncated schema with the full envelope and added per-field semantics (especially the status field, which is the skill's loop predicate: pending / answered / completed / aborted / error). The slice-3 daemon variant inherits all 9 fields. 3. No documented mechanism for the skill body to write pending_hitl.answer (the frontmatter allowed-tools does not include the Write tool). Documented option (a) from the reviewer's NACK: an inline python3 -c "..." invocation, which is covered by the existing Bash(python3 *:*) allowed-tool. The "skill loop" code block now demonstrates the round-trip with a case statement keyed on pending_hitl.status. Non-blocking polish in SKILL.md: - Loop semantics now name "replay" explicitly (the driver spawns a fresh generator and replays answer_log on every invocation; previous text suggested cheap single-step resumption). Added a dedicated "Generator state across invocations (replay semantics)" subsection naming the practical consequence — side effects re-run every call — and pointing at slice 3 as the daemon-variant escape hatch. - Failure-mode bullet for "pending_hitl.decision == null" replaced with the more general "pending_hitl.status ∈ {completed, aborted, error}" bullet so each terminal state has documented diagnostic guidance. Non-blocking polish in docs/architecture/claude-code-substrate.md: - The "Flattened bridge" bullet in the in-process orchestrator section now names the replay path explicitly (promotes answer → answer_log, replays the full log every call, deterministic same-yield-boundary property) and the 5→9 envelope field list mirrors SKILL.md. - Daemon-variant bullet enumerates the same 9-field shape so reviewers comparing slice-1 and slice-3 against the ADR see the full contract. - Schema source-of-truth pointer added to bin/run_pipeline.py:20-46 so future drift triggers fail in one place. Rubric files (reviewer_refine.md, reviewer_agent_design.md) are unchanged — reviewer_code ACKed them in v1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test_bridge_flattened_round_trip: fix subprocess PYTHONPATH The subprocess shim's PYTHONPATH pointed at `<repo>/orchestrator/` and `<repo>/shared/` directly, which lets the subprocess `import substrate` but NOT `import orchestrator.substrate` — the latter requires the *parent* of `orchestrator/` (the repo root) on the path because ``orchestrator/__init__.py`` makes it a real package. Set PYTHONPATH to ``<repo>/shared`` + ``<repo>`` + ``<repo>/gateway`` so: - ``<repo>/shared`` lets ``egg_contracts`` (transitive import from ``orchestrator.substrate.k3s_adapter``) resolve. - ``<repo>`` lets ``import orchestrator`` resolve. Refs #2717 (slice-1 tester). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-1 tester v2: fix subprocess PYTHONPATH + non-blocking improvements Addresses reviewer_code v1 blocking #1 + non-blocking items: BLOCKING FIX — subprocess PYTHONPATH: Add `<repo>/orchestrator` to the subprocess shim's PYTHONPATH so bare- name top-level imports inside the `orchestrator/` tree resolve cleanly. Without this, `orchestrator/models.py:16` (`from slice_id_validation import SLICE_ID_PATTERN`) and `in_process.py:531-534`'s bare `from models import HITLDecision` fallback both fail, crashing the driver subprocess with `ModuleNotFoundError` before it yields the first HITL decision. Mirrors the Makefile's `PYTHONPATH := shared:gateway: orchestrator` (test target). Verified: tests now pass with `PYTHONPATH=.:shared:orchestrator pytest <files>` (reviewer_code's exact reproduction env). NON-BLOCKING (reviewer_code v1): - test_bridge_flattened_round_trip.py: drop stale "whichever the coder picks" docstring phrasing — driver locked in positional argv[1]. - test_bridge_flattened_round_trip.py: mirror the driver's ISO-8601 UTC timestamp format in _write_answer instead of `str(time.time())`. - test_rubric_loader.py: extend `test_loader_accepts_enum_and_string_role` parametrization to cover the two NEW roles (reviewer_refine, reviewer_agent_design) — not just the regression role. - test_rubric_loader.py: strengthen path-traversal assertion to verify the allowlist's slice-fence branch fires (not the file-missing-on- disk branch) — pinning the structural defence. - test_pretooluse_hook_nested.py: derive the r2-verdict.json content from the dispatch outcome and write it BEFORE the structured assertions so slice-5 sees an accurate empirical record even when a regression fails one of the assertions. Adds {"r2_verdict": "fail", "reason": ...} payload format for the failure path. Tests verified: 21/21 pass under both the canonical Makefile PYTHONPATH shape and reviewer_code's `.:shared:orchestrator` reproduction shape. Refs #2717 (slice-1 tester, v2). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-1 (#2548) * docs(#2717 slice-2): plan-team rubrics + SKILL.md plan-phase section Land the four plan-team agent rubric files under plugins/egg-sdlc/skills/egg-sdlc/agents/ for the claude-code substrate of the egg SDLC pipeline (task-2-3): architect, task_planner, risk_analyst, reviewer_plan. Each rubric mirrors its k3s-substrate counterpart in plugins/refine-plan/skills/refine-plan/agents/ for body content (the substrate swap is structurally invisible to the role) and follows the reviewer_refine.md / reviewer_agent_design.md shape from slice-1 for the substrate-specific notes (worktree layout, PreToolUse hook enforcement, HITL-via-AskUserQuestion, concurrent peers in this slice, output path stability). Update plugins/egg-sdlc/skills/egg-sdlc/SKILL.md (task-2-7): - Bump the rollout-status callout from "slice 1 landed" to "slices 1 + 2 landed"; enumerate both the refine and plan rosters. - Replace the "What's NOT in this skill > Plan / implement / pr" bullet's plan deferral with a dedicated **Plan phase** subsection naming the four roles, their spawn order (architect solo, then task_planner + risk_analyst concurrently, with reviewer_plan ACK/NACK on each producer edge), output paths, and the four standard plan-HITL gate options (approve / request_changes / change_approach / stop). - Bump step 8 (phase fence) into a 10-step flow that walks the plan stage spawn order and the plan-HITL gate. The fence now triggers on "approve and continue to implement" with a pointer to slice 3. - Refresh stale "refine-only" / "refine-team subagents" / artifact-path and failure-mode strings to cover both phases. * slice-2 coder: plan-phase BRC stage + rubric loader expansion (#2717) Implements TASK-2-1 + TASK-2-2 for slice-2 of the #2717 rollout. TASK-2-5 closes as no-op per slice-1's R2 = pass verdict (the PreToolUse hook resolves the child's role correctly under nested dispatch; structural enforcement stays hook-side, no MCP-validator-side parallel layer needed). TASK-2-1 — `_run_plan_phase` on `_InProcessOrchestrator` ======================================================== After the refine HITL gate's `approve_continue` answer, the in-process generator now dispatches the plan phase: a `ThreadPoolExecutor` spawns architect / task_planner / risk_analyst concurrently through the same `ClaudeCodeSpawner` the refiner uses, then reviewer_plan is dispatched once with the producer artifacts as its input. `PeerConsensusTracker` drives the BRC mechanics (`handle_propose` / `handle_ack` / `handle_confirmed`); after consensus the stage yields a plan-HITL gate (`HITLDecision` with `phase="plan"` and the canonical 4-way options). The walking-skeleton fence still fires on `approve_continue` past the plan gate — its diagnostic now points at slice-3 / slice-4 of the #2717 rollout instead of #2623. Why the orchestrator records BRC transitions on the subagents' behalf: the in-process substrate's spawner is synchronous (returns AFTER the agent finishes). In the production HTTP daemon the subagents would emit `egg-orch consensus propose/ack/confirmed` themselves and the daemon's gateway listener would advance the tracker. In-process the spawn-completion IS the signal that the subagent proposed or reviewed, so the orchestrator drives the BRC transitions deterministically — the test (harness-faked subagents that never emit BRC messages) and production (real harness agents whose emissions would be no-op duplicates in this path) both reach CONSENSUS_CONFIRMED on the same code path. TASK-2-2 — `_load_egg_sdlc_role_rubric` extension ================================================== `_RUBRIC_LANDED_ROLES` now includes architect / task_planner / risk_analyst / reviewer_plan alongside the slice-1 refine roster (refiner + reviewer_refine + reviewer_agent_design). The structured- error contract for unshipped roles is preserved: implement-team roles (coder / tester / documenter + 5 reviewers) still raise `ValueError` with a slice-3 pointer. The "missing on disk" fallback diagnostic mentions both TASK-1-4 (slice-1 refine) and TASK-2-3 (slice-2 plan) so a reviewer hitting the error in a re-run knows which documenter task needs to land first. TASK-2-5 — agent-side restriction enforcement (no-op) ====================================================== Slice-1's `test_pretooluse_hook_denies_nested_child_write` confirmed the PreToolUse hook denies a child write outside the child's role under nested dispatch (R2 = pass, recorded in `.egg-state/<pipeline_id>/r2-verdict.json` when the test runs). Per the contingent task description, no `sandbox/egg_agent_tools/handlers/restrictions.py` change is needed; structural enforcement stays hook-side. Tester's TASK-2-6 becomes a regression guard asserting the validator helper is a no-op for in-allow-list writes — handled in tester's slice-2 commit. Smoke (manual, in-process, fake subagents) ========================================== * preflight → refine gate → plan gate sequence yields the expected decisions; spawner is called 5 times (1 refiner + 3 plan producers + 1 plan reviewer); tracker.evaluate() reports is_complete=True with all 4 plan-team agents in CONFIRMED state. * Terminal answer at refine gate (e.g. "stop") still returns the refine artifact path — plan phase is NOT entered. * `approve_continue` at the plan gate still raises `NotImplementedError` with the slice-3 / slice-4 pointer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-2 tester: plan-phase BRC E2E + R2-pass restrictions no-op (#2717) TASK-2-4 — integration_tests/regression/test_inprocess_plan_brc.py ================================================================== Plan-phase in-process BRC end-to-end test covering the four AC bullets: * boots `run_pipeline_in_process` against a deterministic pipeline id with harness-faked subagents (no real Anthropic / Claude Code spawn); * advances past the refine HITL gate via `approve` → `approve_continue`; * asserts the plan stage spawns 3 producers (architect, task_planner, risk_analyst) + 1 reviewer (reviewer_plan) — observed via the fake spawner's `.call_args_list`; * asserts the BRC mechanics reach CONSENSUS_CONFIRMED on every producer edge (architect → reviewer_plan, task_planner → reviewer_plan, risk_analyst → reviewer_plan) by reading `_plan_tracker.evaluate()` — the in-process analogue of bus-side CONSENSUS_CONFIRMED messages (the coder's TASK-2-1 implementation drives `PeerConsensusTracker.handle_propose/handle_ack/ handle_confirmed` deterministically since the substrate's spawner is synchronous); * asserts the plan-HITL decision is yielded with `phase="plan"`, `decision_type="phase_gate"`, non-empty `id` / `question` / `options`. Adversarial probing layered on top: * plan stage MUST NOT run when the operator answers `stop` at the refine gate — a regression that fanned into plan on any non-continue answer would burn three unauthorised subagent spawns; * plan stage MUST NOT spawn implement-phase roles — pins the negative invariant against a misrouted `_PHASE_ROLES` lookup; * refiner is spawned exactly once — pins the single-refiner-spawn invariant against an off-by-one role iteration; * every plan-phase spawn carries `EGG_PHASE=plan` in its env — pins the env-propagation contract so spawned subagents see the right phase. The test skips gracefully when the coder's `_run_plan_phase` is absent (scaffold-first per the role's guidance); 7/7 pass against the coder's slice-2 commit 3a466891e. TASK-2-6 — tests/sandbox/egg_agent_tools/test_restrictions_validator.py ======================================================================= Contingent test per slice-1's R2 verdict = `pass`. Per the contract task-2-5 description, "If R2 = pass, this task is a no-op (close with note). Tests for this code path land in TASK-2-6 (tester-owned)." Tests for this code path land here as a **no-op regression guard**: * in-allow-list response shape stable (coder/orchestrator, tester/ tests, documenter/docs) — pins the documented gateway-shape fields `{ok, role, path, can_write, reason, alternative_role}` exactly; * cross-role denial shape stable — pins `can_write=False`, denial `reason` references `shared/egg_restrictions/patterns.py`, `alternative_role` names the single producer that can write; * no new validator symbol — asserts `validate_write_target` (and peers) are NOT present on the restrictions handler module, since R2 = pass meant the cq-6 option-2 enforcement work should NOT have landed; * defensive probes — missing `path` raises HandlerError, unknown role raises HandlerError, list-shaped path returns per-path results with documented shapes. 9/9 pass against the unchanged restrictions handler (no slice-2 source edits in `sandbox/egg_agent_tools/handlers/restrictions.py`). Configured-check results: * ruff check . — PASS (all checks passed) * ruff format check . — FAILS on `orchestrator/substrate/in_process.py` (coder's TASK-2-1 file, 5 long-call sites need re-formatting). My test files pass format check cleanly. This is being NACKed to the coder; my proposal will follow once they push the format fix. * mypy on tester-authored files — PASS (251 source files OK). * Custom checks (scripts/check-*.py) — all 13 pass. * `make lint` / `make test` / `make security` cannot complete in this sandbox: the venv sync fails when uv tries to download pinned wheels (flask, oauthlib) — the wheels.pythonhosted.org TLS chain is "UnknownIssuer" inside the sandbox image (same env constraint the slice-1 tester hit). Tests + lint + custom checks were exercised directly via system pytest / ruff / mypy with the Makefile's canonical `PYTHONPATH := shared:gateway:orchestrator`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * slice-2 coder v2: address NACK blockers (#2717) Addresses 3 NACK reviewers from v1 (commit 3a466891e): reviewer_concurrency NACKs: - C1: removed `_write_active_role_sentinel` from `_spawn_plan_producer`'s concurrent path. Each producer carries `EGG_AGENT_ROLE` in its own spawn env (the load-bearing role-resolution channel under concurrent dispatch); the single-valued sentinel cannot disambiguate three concurrent role-holders. The synchronous `_spawn_plan_reviewer` retains the sentinel write because it never overlaps another spawn. - C2: added `self._current_phase` state on `_InProcessOrchestrator` (default "refine"; flipped to "plan" at the top of `_run_plan_phase`). `_publish_heartbeat` reads from it so HEARTBEAT messages carry the right phase across the refine→plan transition. Without this, stuck-phase-transition watchdogs filtering by `phase` would see "refine" while the plan stage is actively running. reviewer_code_holistic NACKs: - H1: architect-first then fanout. `_run_plan_phase_inner` now spawns architect synchronously first, records its CONSENSUS_PROPOSE on the tracker, then fans out task_planner + risk_analyst concurrently through a ThreadPoolExecutor with max_workers=2. The architect's per-role output path is passed into each downstream producer's spawn env (`EGG_ARCHITECT_OUTPUT_PATH`) and prompt_text so they can read its `key_design_decisions` rather than re-deriving them. This matches the role-dependency declarations at `shared/egg_contracts/agent_roles.py:398/422` (TASK_PLANNER_ROLE / RISK_ANALYST_ROLE both list ARCHITECT as their sole dependency) and the architect / task_planner / risk_analyst rubric bodies the documenter shipped. - H2: reviewer_plan verdict-JSON parsing. New helpers `read_plan_reviewer_verdicts` (parses `.egg-state/agent-outputs/<issue>-reviewer_plan-output.json`) and `_apply_reviewer_verdicts` drive per-edge ACK / NACK on the tracker based on the reviewer's actual verdict rather than the exit-code-only heuristic v1 used. Fail-closed when the verdict file is missing AND the reviewer's spawn failed (NACK every edge); optimistic ACK only when the verdict file is missing AND the reviewer's spawn returned exit 0 (harness-faked test path), with the "verdict-not-parsed" status surfaced in the placeholder body so the operator sees the discrepancy at the HITL gate. tester NACK: - T1: ran `ruff format` on the affected files. `_spawn_plan_reviewer` also dropped the dead `EGG_PRODUCER_ARTIFACT_PATHS` env var (reviewer_code_holistic v1 non-blocking #3) in favor of per-role `EGG_<ROLE>_OUTPUT_PATH` env vars that the reviewer_plan rubric actually consumes. Non-blocker polish landed alongside the blockers: - `_synthetic_commit_for(role)` derives a per-role hex SHA so the three concurrent ProposalPayload entries remain commit-distinguishable in the tracker (reviewer_concurrency v1 NB #2). - Tracker-guard rejections (`handle_propose` / `handle_ack` / `handle_nack` / `handle_confirmed`) now log via `logging.getLogger("orchestrator.substrate.in_process").warning` instead of silent `except Exception: pass` (reviewer_code_holistic v1 NB). - `_format_plan_placeholder` now also renders reviewer_plan diagnostics + verdict-parsing status (reviewer_code_holistic v1 NB). File decomposition: - ruff format expanded the v1 diff to 1879 lines, breaching the 1500-line hard cap in `scripts/file-size-allowlist.yaml`. Extracted the plan-phase body (~700 lines) into `orchestrator/substrate/_plan_phase.py` as module-level functions that take the `_InProcessOrchestrator` instance as their first argument. The class's `_run_plan_phase` / `_spawn_plan_producer` / `_spawn_plan_reviewer` / `_plan_producer_output_path` / `_read_plan_reviewer_verdicts` methods stay on the class as thin delegates so the existing test surface (and tester's 16 passing tests against v1) keeps the same method names. `in_process.py` now lands at 1093 lines (under both caps); `_plan_phase.py` at 680 lines. Manual in-process smoke (harness fakes, MagicMock subagents): - Happy path: preflight → refine gate → plan gate; spawner called 5 times in order [refiner, architect, task_planner|risk_analyst, task_planner|risk_analyst, reviewer_plan]; tracker reaches `is_complete=True`. - Refine stop: returns refine artifact path; spawner called 1 time (no plan dispatch). - Mixed verdict: with a per_producer verdict JSON {architect:ACK, task_planner:NACK, risk_analyst:ACK}, the tracker records the NACK on task_planner → reviewer_plan; `is_complete=False`; blocking_agents includes reviewer_plan (unresolved critical NACK) and task_planner (not fully ACKed). - Fail-closed: with reviewer spawn exit_code=1 and no verdict file, the tracker NACKs every critical edge; risk_analyst (advisory edge) still confirms; reviewer_plan blocks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Address slice-1 review: fix install path, bridge answer-write, silent fallbacks Reviewer findings from PR #2724 (egg-reviewer slice-1 review): Blockers (1-4): * The documented `pip install -r requirements.txt` failed — no top-level requirements.txt exists. Switch SKILL.md, plugin.json's `egg.install_instructions`, and `bin/preflight.py` to `pip install .` against pyproject.toml. * SKILL.md claimed Python 3.11+ but pyproject.toml requires >=3.14. Update SKILL.md and preflight to reflect the real floor; correct the matching docstring in orchestrator/substrate/__init__.py. * The inline `python3 -c "..."` answer-write snippet in SKILL.md was broken (shell-interpolated `${ANSWER}` → NameError, deprecated `datetime.utcnow`, non-atomic write). Replace with a dedicated `bin/write_answer.py` helper that reads the JSON-encoded answer from stdin, uses `datetime.now(UTC)`, and writes atomically via tmp + `os.replace`. SKILL.md's loop now invokes the helper. * `_serialise_decision` silently swallowed `model_dump` failures, and `_read_contract` silently overwrote unparseable contracts (dropping `answer_log`). Log to stderr on serialisation fallbacks; refuse to overwrite a corrupted contract and exit 1 instead. Non-blocking (5-11): * `orchestrator/substrate/__init__.py` docstring rewritten to reflect reality: Python 3.14+ introduced PEP 758 (`except A, B:` without parens); ruff under py314 target strips redundant parens, hence `# fmt: skip` on multi-except lines. * Stderr warning in `_advance_generator`'s `finally` so a teardown failure inside `generator.close()` is at least observable. * `allowed-tools` tightened from `Bash(python3 *:*)` to `Bash(python3 plugins/egg-sdlc/skills/egg-sdlc/bin/*:*)` — the two helper scripts are the entire Python surface the skill can invoke. * Re-spawn cost note expanded in SKILL.md: each driver invocation re-runs every prior subagent spawn (real Anthropic API spend); slice-2 compounds to 8 spawns at the final stage. * `_RUBRIC_LANDED_ROLES` registry collapsed into `_CURRENT_LOADER_SLICE` + filesystem probe; same diagnostics, no parallel registry that can drift from disk state. * Abort vocabulary exported as `ABORT_ANSWERS` from `orchestrator.substrate.in_process`; the driver imports it (with a literal fallback when the orchestrator package is unimportable) so the driver, orchestrator, and slice-3 daemon share a single source of truth. * R2 caveat surfaced as an explicit open question for slice-5 sequencing in SKILL.md. Tests: * New `shared/tests/test_write_answer.py` (6 tests, all passing) pins the JSON-encoding round-trip, the timestamp format match against the driver, the atomic-write contract, and the corrupted-contract refusal. * Existing `test_rubric_loader`, `test_run_pipeline_in_process*`, `test_substrate_interfaces`, `test_bridge_flattened_round_trip`, and `test_pretooluse_hook_nested` all still pass. * The pre-existing `test_empty_diff_subprocess_skips_pytest` failure in `tests/tools/` reproduces against HEAD without these changes (detached-HEAD worktree edge case in the test selector). Authored-by: egg * slice-2 coder v4: support rubric-default single-verdict JSON schema (#2717) Addresses reviewer_code_holistic v3 NACK blocker H3 — the rubric the documenter shipped (plugins/egg-sdlc/skills/egg-sdlc/agents/reviewer_plan.md "Verdict JSON shape", lines 57-80) documents a single top-level verdict object (verdict ∈ {ACK, NACK}, analysis carrying the eight criteria, feedback blob, artifact_references), not the per_producer wrapper v2/v3's parser expected. A rubric-following reviewer's NACK would silently fall into the "verdict file present but no parseable per_producer entries" branch and the orchestrator's optimistic-ACK fallback would mask the NACK from the operator at the plan-HITL gate. v4 makes `read_plan_reviewer_verdicts` accept BOTH schemas: 1. Rubric-default single-verdict (broadcast). When the JSON's top-level `verdict` is "ACK" or "NACK", the verdict is broadcast to every plan producer edge — ACK acks all three, NACK nacks all three with `feedback` propagated as the per-edge `reason` (a synthetic placeholder fires if `feedback` is empty so the tracker's NACK guard doesn't reject the payload). This is "Option (c)" from the v3 NACK; per-edge granularity is lost but the rubric's "ACK only if every criterion passes" semantic IS preserved. 2. Per-producer extension (per-edge). The existing per_producer wrapper still takes precedence when present and well-formed. Reviewers that want explicit edge granularity (ACK architect + NACK task_planner) write the wrapper; the rubric's default shape stays broadcast-compatible. The function now takes an optional `plan_producers` kwarg so the caller (the in-process orchestrator) can broadcast the single verdict to the right role set. The `_read_plan_reviewer_verdicts` class method delegate also propagates the kwarg so tester-side tests that call the method retain their access pattern. Smoke (manual, in-process, MagicMock subagents): - Rubric-default single-verdict NACK: tracker NACKs architect + task_planner (critical edges), risk_analyst still confirms (advisory), reviewer_plan blocks consensus. is_complete=False; blocking_agents=['architect', 'task_planner', 'reviewer_plan']. - Rubric-default single-verdict ACK: every edge confirmed; is_complete=True. - per_producer wrapper still works: mixed ACK/NACK applied per edge. - Harness-fake path (no verdict file, reviewer exit 0): optimistic ACK preserved so tester's existing 16 passing tests keep their access pattern. - Fail-closed path (no verdict file, reviewer exit non-zero): critical edges NACK'd (unchanged from v2/v3). ruff format + ruff check + file-size lint all pass. `_plan_phase.py` is 747 lines; `in_process.py` 1095 lines. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist BRC history for slice-2 (#2548) * Move skill-loop python3 -c calls into bin/ helpers Address review feedback on PR #2724: - Add bin/read_status.py and extend write_answer.py with --answer-…

* Revert Claude Code substrate 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. * Parenthesize except tuples in lifecycle_secret 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. * Fix checks: apply automated formatting fixes --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: egg <egg@localhost>

docs: add claude-code substrate to index and structure docs

5dea422

egg-reviewer Bot suggested changes May 19, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

egg-reviewer Bot approved these changes May 19, 2026

View reviewed changes

jwbron approved these changes May 19, 2026

View reviewed changes

jwbron merged commit 173f483 into main May 19, 2026
24 checks passed

This was referenced May 19, 2026

[slice-1] Roll out Claude Code substrate to remaining roles + plan/... #2724

Merged

[slice-2] Roll out Claude Code substrate to remaining roles + plan/... #2726

Merged

jwbron mentioned this pull request May 20, 2026

Revert Claude Code substrate #2742

Merged

3 tasks

Navigation Menu

docs: add claude-code substrate to index and structure docs [doc-updater] - #2718

docs: add claude-code substrate to index and structure docs [doc-updater]#2718
jwbron merged 2 commits into
mainfrom
egg/doc-update-claude-code-substrate

james-in-a-box Bot commented May 19, 2026

Uh oh!

egg-reviewer Bot left a comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

james-in-a-box Bot commented May 19, 2026

Uh oh!

This comment has been minimized.

egg-reviewer Bot left a comment

Uh oh!

james-in-a-box Bot commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

james-in-a-box Bot commented May 19, 2026

Uh oh!

egg-reviewer Bot left a comment

Choose a reason for hiding this comment

Summary

Blocking

1. ClaudeCodeSpawner description is factually wrong (docs/development/STRUCTURE.md:158)

Non-blocking

2. claude_code/hook_entry.py is missing from the listing

3. Optional: settings.template.json could be listed

Uh oh!

This comment has been minimized.

This comment has been minimized.

james-in-a-box Bot commented May 19, 2026

Uh oh!

This comment has been minimized.

egg-reviewer Bot left a comment

Choose a reason for hiding this comment

Summary

Verifications

Non-blocking

Uh oh!

james-in-a-box Bot commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

1. `ClaudeCodeSpawner` description is factually wrong (`docs/development/STRUCTURE.md:158`)

2. `claude_code/hook_entry.py` is missing from the listing

3. Optional: `settings.template.json` could be listed