Merge Phase 1.5 docs into main by jwbron · Pull Request #8 · jwbron/egg

jwbron · 2026-02-02T23:40:14Z

Brings Phase 1.5 documentation into main.

PR #3 was accidentally merged into jib/phase-1-setup instead of main. This PR corrects that.

Includes:

Phase 1.5: Documentation extraction
Fix dev script and pyproject.toml for broader compatibility

Implement Phase 1 of the egg extraction plan: - Create repository structure with gateway/, sandbox/, cli/, shared/, tests/ - Add pyproject.toml with uv for dependency management - Add GitHub Actions workflows (lint.yml, test.yml) - Add ./dev wrapper script for CI parity (runs act locally) - Add pre-commit configuration - Add Dependabot configuration - Write README.md with architecture overview - Write CONTRIBUTING.md with development guidelines - Add egg.yaml.example and secrets.yaml.example - Create module structure with __init__.py files - Add initial unit tests for module imports - Add docs/testing.md and docs/architecture.md This establishes the infrastructure before extracting gateway code in Phase 2. Authored-by: jib

- Remove sudo requirement from act installation by installing to $HOME/.local/bin instead of system-wide - Add hatchling build config specifying packages to include - Add uv.lock for reproducible builds Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Phase 1.5: Documentation extraction Extract and create documentation for the egg repository: - docs/security.md: Core security model and threat mitigations - docs/adr/git-isolation-architecture.md: Git worktree isolation ADR - docs/adr/credential-injection.md: Credential injection ADR - docs/configuration.md: Configuration reference - docs/setup.md: Setup guide - docs/api.md: Gateway API reference - docs/troubleshooting.md: Common issues and solutions Security documentation extracted from james-in-a-box ADRs: - ADR-Gateway-Credential-Injection.md - ADR-Git-Isolation-Architecture.md - ADR-Anthropic-API-Credential-Injection.md All docs regenerated for egg (no james-specific references). Authored-by: jib * Add network isolation ADR Extract ADR-Internet-Tool-Access-Lockdown from james-in-a-box as docs/adr/network-isolation.md. This documents the private mode network lockdown feature including: - Public vs private network modes - Docker network topology for isolation - Domain allowlist (Anthropic + GitHub only) - Squid proxy configuration - WebSearch/WebFetch blocking in private mode - Breakout prevention mechanisms Also adds ADR section to README and updates security.md references. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add cross-references between ADRs Add Related ADRs sections to credential-injection.md and git-isolation-architecture.md for bidirectional navigation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Improve README to better showcase egg's robustness - Add name explanation (inspired by Andy Weir's "The Egg") - Add threat/mitigation table showing security guarantees - Rewrite architecture diagram for cleaner rendering - Expand Key Features with detailed sections: - Zero Credential Exposure - Git Isolation via Gateway-Managed Worktrees - Granular Access Control - Public and Private Network Modes - Comprehensive Audit Trail - Improve Quick Start with credential setup steps Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Expand security.md into comprehensive security whitepaper Replace the brief security overview with a full security model document covering: 1. Threat Model - adversary capabilities, protected assets, security boundaries 2. Architecture Overview - component diagrams, security properties summary 3. Network Isolation - public/private modes, domain allowlist, Squid config, DNS tunneling prevention, ECH/ESNI handling 4. Credential Isolation - zero-credential sandbox, token lifecycle, gateway authentication 5. Git/GitHub Lockdown - metadata isolation, gateway policies, blocked operations and flags 6. Private Repository Mode - visibility enforcement, caching strategy 7. Audit Logging - structured JSON format, alerting 8. Residual Risks - honest assessment of remaining attack vectors 9. OWASP Alignment - maps to OWASP Top 10 for Agentic Applications Adapted from james-in-a-box Sandbox-Security-Proposal.md (PR #659). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Clarify private mode restricts to private GitHub repos only - Update README threat table, private mode description, and CLI flags - Update security.md and network-isolation.md for consistency - Keep original architecture diagram (to be fixed separately) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Update credential-injection.md * Update git-isolation-architecture.md * Update network-isolation.md * Update security.md * Update README.md --------- Co-authored-by: James Wiesebron <jameswiesebron@khanacademy.org> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: James <8340608+jwbron@users.noreply.github.com>

The subprocess timeout alone is insufficient when curl hangs on connection issues. Add --max-time flag to curl so it respects the timeout regardless of connection state. Set subprocess timeout slightly higher (10s) to let curl handle its own timeout gracefully. This addresses review feedback item #8. Authored-by: egg

* Initialize SDLC contract for issue #367 * Update refine review state (cycle 1) * Approve and advance to plan phase for issue #367 * Populate contract from plan for issue #367 * Update plan review state (cycle 1) * Approve and advance to implement phase for issue #367 * Implement unified Docker Compose deployment strategy Rework the egg deployment strategy to use Docker Compose for consistent and simplified deployments. This replaces the manual multi-step deployment with a single `docker compose up` command. Phase 1 - Improved Orchestration: - Add docker-compose.yml for production deployment - Create .env.example template with all configuration - Add bin/egg-deploy launcher script for easy management - Add egg --compose CLI mode for compose-based startup - Create orchestration.py for shared deployment logic - Update documentation with new deployment guide Phase 2 - Launcher Container: - Create launcher/Dockerfile for single-container deployment - Implement lifecycle management for gateway/sandbox - Add session forwarding for interactive mode - Add --print mode support for CI/CD use - Create monitoring API endpoint (/status, /health) - Update release-images.yml to build launcher image Phase 3 - Migration and Cleanup: - Add deprecation warnings to start-gateway.sh - Add deprecation warnings to create-networks.sh - Create migration guide (docs/guides/deploy-migration.md) - Update integration tests to document unified approach The new deployment options are: 1. Docker Compose: bin/egg-deploy up 2. CLI mode: egg --compose 3. Launcher container: docker run ghcr.io/jwbron/egg-launcher Issue: #367 Authored-by: egg * Fix lint failures: formatting and type errors Apply ruff formatting to 4 files and fix mypy errors in orchestration.py: - Fix incorrect attribute name publish_gateway_ports -> publish_ports - Add text=True to subprocess.run to match type annotation Authored-by: egg * Rework deploy strategy: Option D only (remove launcher container) Per owner feedback, simplify the deploy strategy by removing Option B (launcher container with socket mounting) and proceeding with Option D (Docker Compose + improved orchestration) alone. Changes: - Remove launcher/ directory (Phase 2 implementation) - Update contract to remove Phase 2 tasks and renumber phases - Remove launcher from release-images workflow - Update deployment docs to remove launcher references - Update migration guide to remove launcher section The approach now focuses on Docker Compose-based deployment which provides sufficient simplification without the added complexity of socket mounting. Authored-by: egg * Add --max-time flag to curl health checks for robust timeouts The subprocess timeout alone is insufficient when curl hangs on connection issues. Add --max-time flag to curl so it respects the timeout regardless of connection state. Set subprocess timeout slightly higher (10s) to let curl handle its own timeout gracefully. This addresses review feedback item #8. Authored-by: egg * Remove stale egg-launcher references from deprecation notices The launcher container was removed in commit c5eb560 per owner feedback, but the deprecation notices in gateway scripts still referenced it. This cleans up those stale references. Authored-by: egg * Address review feedback: curl timeouts, import cleanup, port docs - Add --max-time 5 flag to curl health checks in bin/egg-deploy to prevent curl from hanging indefinitely on connection issues - Move import re from inside loop to module level in compose.py for efficiency and conventional style - Add clarifying comments to docker-compose.yml ports section explaining these only change host-side port mapping Authored-by: egg --------- Co-authored-by: james-in-a-box[bot] <2365503+james-in-a-box[bot]@users.noreply.github.com> 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>

- Include truncated raw_partial_input in tool_use blocks when JSON parsing fails, for debugging incomplete streaming responses (re-review issue #1) - Add docstring notes documenting: - Tool result matching order-dependency (original issue #5) - Force push checkpoint behavior (original issue #7) - Shared transcript for multi-commit pushes (original issue #8) - container_id fallback from filename (original issue #12) - Fix import sorting in gateway.py (ruff auto-fix) Authored-by: egg

…pt capture (#517) * Initialize SDLC contract for issue #509 * Draft analysis for issue #509: checkpoint migration * Update refine review state (cycle 1) * Approve and advance to plan phase for issue #509 * Draft plan for issue #509: per-commit checkpoints and API proxy capture * Update plan review state (cycle 1) * Approve and advance to implement phase for issue #509 * Populate contract tasks from plan for issue #509 * Implement per-commit checkpoint granularity (Phase 1) Add support for creating one checkpoint per commit instead of one per push: - Add get_commits_in_push() helper to enumerate commits between old/new SHAs - Add capture_and_store_checkpoints_for_push() for multi-commit checkpoint creation - Modify gateway push handler to get remote ref before push and iterate commits - Each checkpoint now has push_sha pointing to the tip commit for traceability - Graceful fallback: if rev-list fails, creates single checkpoint for tip commit This is Phase 1 of issue #509 (per-commit checkpoints + API proxy capture). Transcript source is still Claude Code JSONL (to be migrated in Phase 3). Authored-by: egg * Implement API proxy transcript capture (Phase 2) Add instrumentation to the Anthropic API proxy to capture request/response pairs for checkpoint creation: - New TranscriptBuffer class manages per-session buffer files in /tmp/egg-transcripts/ - Fixed 10MB buffer rotation drops oldest entries when size limit exceeded - Non-streaming responses captured with full content and usage - Streaming SSE responses reassembled to extract content blocks - Buffer entry schema documented with timestamps, model, messages, tools, usage - Thread-safe file writes with file locking - Global buffer cache for efficient access by container ID This is Phase 2 of issue #509 (per-commit checkpoints + API proxy capture). Phase 3 will migrate checkpoint extraction to use this buffer instead of Claude Code JSONL files. Authored-by: egg * Migrate transcript source to API proxy buffer (Phase 3) Replace Claude Code JSONL extraction with API proxy buffer reading: - Add extract_transcript_from_proxy_buffer() and related extraction functions - Update CheckpointHandler.capture_checkpoint() to use proxy buffer instead of JSONL - Remove CLAUDE_PROJECTS_DIR constant (no longer needed) - Maintain same return types for backward compatibility The proxy buffer provides a stable API-based source for transcripts, eliminating dependency on Claude Code's internal file format which can change without notice. This is Phase 3 of issue #509. Phase 4 will clean up deprecated JSONL code and add buffer cleanup on session end. Authored-by: egg * Cleanup and documentation for proxy buffer architecture (Phase 4) - Remove all Claude Code JSONL-specific code from transcript_extractor.py (file now only contains proxy buffer extraction functions) - Add buffer cleanup on session end in session_manager.py - Update CLI help text to describe per-commit checkpoint behavior - Add architecture documentation in checkpoint_handler.py explaining the transcript flow from API proxy to checkpoint storage Authored-by: egg * Update autofix attempts for issue #509 * Address review feedback on PR #517 transcript capture Fixes critical and correctness issues raised in review: 1. Race condition in buffer rotation (Critical #1): - Use exclusive file lock during entire rotation operation - Write to temp file then atomically replace with os.replace() - Clean up temp file on error 2. Memory accumulation for streaming (Critical #2): - Cap collected chunks at 10MB to prevent resource exhaustion - Log when capture is truncated due to size limit 3. Missing error handling for failed API responses (Critical #3): - Capture 4xx/5xx responses with status code and error message - Pass status_code to capture function for proper handling 4. Missing input_tokens from message_start (Correctness #4): - Extract input_tokens, cache_read_input_tokens from message_start - message_delta only contains output_tokens per SSE spec 5. Error events not handled in SSE parsing (Correctness #4): - Handle 'error' event type and capture as error content block - Add input_parse_error flag when tool_use JSON fails to parse 6. Unsafe path construction (Correctness #6): - Validate container_id against path traversal characters - Verify resolved path is within buffer directory Minor fixes: - Move `import time` to module level (Minor #10) - Set buffer directory permissions to 0o700 (Minor #11) Authored-by: egg * Address re-review feedback: add raw_partial_input and doc comments - Include truncated raw_partial_input in tool_use blocks when JSON parsing fails, for debugging incomplete streaming responses (re-review issue #1) - Add docstring notes documenting: - Tool result matching order-dependency (original issue #5) - Force push checkpoint behavior (original issue #7) - Shared transcript for multi-commit pushes (original issue #8) - container_id fallback from filename (original issue #12) - Fix import sorting in gateway.py (ruff auto-fix) Authored-by: egg * Address minor review observations: add constant and logging - Add RAW_INPUT_TRUNCATE_SIZE constant for the 1000 char truncation threshold used when preserving raw tool input on parse failure - Add warning log in transcript_extractor when extracting tool calls with input_parse_error flag, surfacing streaming parse failures during checkpoint extraction Authored-by: egg * Add test coverage for input_parse_error handling path Adds tests for the input_parse_error and raw_partial_input handling that was added for incomplete streaming JSON responses. This addresses the non-blocking observation from the PR review to ensure the defensive code path is documented through tests and protected from regressions. - TestParseSSEResponse: Tests that incomplete tool_use JSON sets input_parse_error flag and preserves truncated raw_partial_input - TestExtractToolCallsFromProxyBuffer: Tests that extraction logs a warning when input_parse_error is present and truncates the raw input preview appropriately Authored-by: egg --------- Co-authored-by: james-in-a-box[bot] <2365503+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com>

- Fix operator precedence bug in test assertion (issue #1) - Restore auto_create_pr field as deprecated for backwards compat (issue #2) - Remove dead local-mode PR phase prompt code (issue #3) - Set work_started_at on phase_execution in auto-PR path (issue #4) - Remove unused mock handler variables (issue #5) - Document create_pr error contract difference from other temp-session methods (issue #7) - Upgrade pre-PR push failure log level from WARNING to ERROR (issue #8)

* Auto-create PR in orchestrator, skip agent spawn for PR phase * Fix checks: apply automated formatting fixes * Remove auto_create_pr opt-out, always auto-create PR in orchestrator * Fix lint: remove unused PipelineConfig import * Address review feedback on auto-PR creation - Fix operator precedence bug in test assertion (issue #1) - Restore auto_create_pr field as deprecated for backwards compat (issue #2) - Remove dead local-mode PR phase prompt code (issue #3) - Set work_started_at on phase_execution in auto-PR path (issue #4) - Remove unused mock handler variables (issue #5) - Document create_pr error contract difference from other temp-session methods (issue #7) - Upgrade pre-PR push failure log level from WARNING to ERROR (issue #8) --------- Co-authored-by: egg <egg@localhost> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

Addresses blocking and non-blocking concerns raised by egg-reviewer on the auto-ACK-pure-producers PR. End-to-end wiring (blocking #1) Without an agent-side skip-propose path, the seed prevented the deadlock only at the matrix level — CODER's container still ran the standard producer lifecycle, proposed at version 2, and invalidated the seeded version-1 ACKs, re-opening the deadlock the seed exists to prevent. - Thread ``is_pre_seeded_empty_producer`` through ``_build_agent_prompt`` → ``_build_phase_prompt`` → ``_build_brc_preamble`` → ``_build_producer_orientation``. - Derive the per-role flag from the same predicate the matrix seed uses (``producer_roles() − producer_roles_with_tasks``, skipping dual-role) so prompt and matrix stay in sync. - The producer-lifecycle preamble grows a top-of-block shortcut notice telling pre-seeded coders/documenters to skip propose, confirm directly, and fall through to step 4's wait-loop on ``pending_acks: global_zero_proposal``. The existing ``_collect_newly_ready_producers`` sweep emits the STATUS ``ready_to_confirm`` nudge naturally when another producer proposes; no extra orchestration plumbing required. - Orient text for pre-seeded coders/documenters is shortened to "confirm no tasks, do not invent work." - On the dual-role-reviewer-NACK recovery path (TESTER NACKs the seeded CODER v=1), the shortcut routes through ``mcp__sdlc__register_open_question`` rather than silently starting to produce — surfaces the planning gap to the operator. Narrow exception handling (blocking #2) ``_run_concurrent_phase`` previously swallowed any ``Exception`` at ``logger.debug``, hiding contract-load failures and silently re-introducing the deadlock when the seed couldn't run. Now: - Catch only ``ContractNotFoundError`` / ``ContractValidationError`` / ``OSError`` narrowly; unknown exceptions propagate so schema bumps fail loudly in testing. - Upgrade the log level from DEBUG to WARNING so operators see the "safety net is off" condition by default. - The slice-id-not-in-contract path is now an explicit WARNING with the contract's available slice ids inlined, so a contract-on-main vs slice-on-branch skew is diagnosable. Documenter-only TESTER scenario (blocking #3) Added ``TestDocumenterOnlySliceTesterFlow`` test asserting that CODER pre-seeded + DOCUMENTER normal propose + TESTER no-op propose (with its critical-reviewer ACKs) yields a fully-ACKed, consensus-reachable matrix for every producer in the graph. Pins down the composition of the existing ``no_test_changes_needed`` path (#2431) with the new seed. Non-blocking items - #4 Documented the dual-role pre-ACK known failure mode in the matrix docstring: seeded TESTER→CODER ACKs are advisory, not authoritative, and operators inspecting a stalled slice should treat them as such. - #5 Added integration-style tests for the wiring layer: ``TestProducerRolesWithTasksDerivation`` and ``TestProducerOrientationPreSeededShortcut``. - #6 Renamed the misleading idempotency test and added the proposal-version assertion so the version-inflation is explicitly observable. - #8 Added public ``ReviewGraph.producer_roles()`` and ``reviewer_roles()`` accessors returning snapshot copies; the seed now uses ``producer_roles()`` instead of reaching into ``_producer_roles``. - #9 Replaced the "version 1" imprecision in the seed docstring with the new-version semantic and a note about subsequent invocations.

…role (#2583) * Fix #2581: auto-ACK pure producers when slice has no tasks for their role Pre-seeds the BRC approval matrix for pure producers (CODER, DOCUMENTER) whose role has no tasks in the slice's plan, so a tester-only or documenter-only slice doesn't deadlock waiting for reviewers to ACK an empty proposal. Dual-role producers (TESTER) keep current behavior. A dual-role reviewer can NACK at the seeded version to recover the "tester needs coder to do work" path. Supersedes #2565 / closes the approach in PR #2567. * Address review feedback on PR #2583 — wire end-to-end and harden seed Addresses blocking and non-blocking concerns raised by egg-reviewer on the auto-ACK-pure-producers PR. End-to-end wiring (blocking #1) Without an agent-side skip-propose path, the seed prevented the deadlock only at the matrix level — CODER's container still ran the standard producer lifecycle, proposed at version 2, and invalidated the seeded version-1 ACKs, re-opening the deadlock the seed exists to prevent. - Thread ``is_pre_seeded_empty_producer`` through ``_build_agent_prompt`` → ``_build_phase_prompt`` → ``_build_brc_preamble`` → ``_build_producer_orientation``. - Derive the per-role flag from the same predicate the matrix seed uses (``producer_roles() − producer_roles_with_tasks``, skipping dual-role) so prompt and matrix stay in sync. - The producer-lifecycle preamble grows a top-of-block shortcut notice telling pre-seeded coders/documenters to skip propose, confirm directly, and fall through to step 4's wait-loop on ``pending_acks: global_zero_proposal``. The existing ``_collect_newly_ready_producers`` sweep emits the STATUS ``ready_to_confirm`` nudge naturally when another producer proposes; no extra orchestration plumbing required. - Orient text for pre-seeded coders/documenters is shortened to "confirm no tasks, do not invent work." - On the dual-role-reviewer-NACK recovery path (TESTER NACKs the seeded CODER v=1), the shortcut routes through ``mcp__sdlc__register_open_question`` rather than silently starting to produce — surfaces the planning gap to the operator. Narrow exception handling (blocking #2) ``_run_concurrent_phase`` previously swallowed any ``Exception`` at ``logger.debug``, hiding contract-load failures and silently re-introducing the deadlock when the seed couldn't run. Now: - Catch only ``ContractNotFoundError`` / ``ContractValidationError`` / ``OSError`` narrowly; unknown exceptions propagate so schema bumps fail loudly in testing. - Upgrade the log level from DEBUG to WARNING so operators see the "safety net is off" condition by default. - The slice-id-not-in-contract path is now an explicit WARNING with the contract's available slice ids inlined, so a contract-on-main vs slice-on-branch skew is diagnosable. Documenter-only TESTER scenario (blocking #3) Added ``TestDocumenterOnlySliceTesterFlow`` test asserting that CODER pre-seeded + DOCUMENTER normal propose + TESTER no-op propose (with its critical-reviewer ACKs) yields a fully-ACKed, consensus-reachable matrix for every producer in the graph. Pins down the composition of the existing ``no_test_changes_needed`` path (#2431) with the new seed. Non-blocking items - #4 Documented the dual-role pre-ACK known failure mode in the matrix docstring: seeded TESTER→CODER ACKs are advisory, not authoritative, and operators inspecting a stalled slice should treat them as such. - #5 Added integration-style tests for the wiring layer: ``TestProducerRolesWithTasksDerivation`` and ``TestProducerOrientationPreSeededShortcut``. - #6 Renamed the misleading idempotency test and added the proposal-version assertion so the version-inflation is explicitly observable. - #8 Added public ``ReviewGraph.producer_roles()`` and ``reviewer_roles()`` accessors returning snapshot copies; the seed now uses ``producer_roles()`` instead of reaching into ``_producer_roles``. - #9 Replaced the "version 1" imprecision in the seed docstring with the new-version semantic and a note about subsequent invocations. * Replace stub tests with real protocol-level tests for #2581 seed Addresses the second-pass review of PR #2583: - Extracts ``routes.pipelines._derive_producer_roles_with_tasks`` so the contract-load + slice-lookup + narrow-exception logic can be unit-tested without spinning up a pipeline. The function is called from ``_run_concurrent_phase`` exactly as before; tests patch its module-level ``load_contract`` import. - Adds ``ReviewGraph.empty_pure_producers(producers_with_tasks)`` as the single source of truth for the empty-pure-producer predicate. Both ``ApprovalMatrix.seed_auto_ack_for_empty_pure_producers`` and ``_run_concurrent_phase``'s prompt-flag computation now route through it, so the prompt flag and the matrix seed cannot drift. - Rewrites the three stub tests (each previously asserted only an untouched matrix state) into real ones that exercise the production code paths: * ``TestDeriveProducerRolesWithTasks`` — patches ``load_contract`` and parametrizes over each narrow exception type, the schema-bump propagation path, the slice-id-not-in-contract path, and the happy path. Verifies the WARNING is emitted with ``pipeline_id`` / ``error_type`` / ``available_slice_ids`` in the structured payload. * ``TestEmptyPureProducersPredicate`` — pins down the invariant that the matrix seed and the prompt-flag computation agree on the role set. - Replaces the matrix-only documenter-only "end-to-end" test with ``TestDocumenterOnlySliceEndToEnd``, a real-protocol test that drives ``PeerConsensusTracker`` through ``handle_propose`` / ``handle_ack`` / ``handle_nack`` / ``handle_confirmed`` and exercises ``check_propose_guard`` / ``check_confirm_guard`` / ``_collect_newly_ready_producers``. Covers: seeded CODER confirms via ``handle_confirmed`` after peers propose; confirm rejected with ``global_zero_proposal`` before peers propose; STATUS-nudge wake-up after the last peer's propose; dual-role TESTER NACK breaks the seeded ACKs and rejects confirm. - Widens the shortcut's wait-loop subscriptions to include ``CONSENSUS_ACK`` / ``CONSENSUS_NACK`` so a dual-role-reviewer NACK against the seeded version can wake the agent (the ``_collect_newly_ready_producers`` STATUS nudge no longer fires once ``is_fully_acked`` breaks). - Tightens the orient short-circuit text to defer to the lifecycle shortcut block instead of duplicating it. Net effect: 31 tests in ``test_auto_ack_pure_producers.py``, all passing. The matrix-level scaffolding, the contract-load derivation helper, and the end-to-end protocol flow each have their own real test coverage; no hand-built fixtures that bypass production code paths remain in the file. * Address third-review suggestions: drop dead branch, use public API, restore type hint Three non-blocking suggestions from the third review on #2583: 1. Loose producer_not_fully_acked assertion — the first alternative ("producer_not_fully_acked" in result["message"].lower()) was dead because the guard-name literal lives in guard.details, not in the message. handle_confirmed returns message=guard.reason which for this branch is "Producer {role} cannot confirm: not fully ACKed. ...". Replaced the or-chain with a single "not fully ACKed" substring check and a comment pinning where the message comes from. 2. Private attribute access — replaced "coder" in tracker._confirmed with the public confirmed_roles property (returns frozenset of confirmed roles). Same observation, no private-attribute reach. 3. Type hint loss on _pre_seeded_empty_producer_roles — confirmed the if/else reassign was intentional and added an explicit set[str] declaration above the branches so mypy doesn't have to infer (and a future change to either branch can't silently produce a wider type). --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

…lockers Reviewer_plan NACKed the v1 plan with one blocking item (single-OR JQL fails on team-managed Jira projects) plus 20 non-blocking flags ranked by impact. This revision lands the blocker plus the 10 highest-impact non-blockers in one re-propose. Blocker: - TASK-1-3 + TASK-1-12: replace the single-OR JQL `parent = <K> OR "Epic Link" = <K>` with two separate queries (`parent = "<K>"` and `"Epic Link" = "<K>"`) and merge results, tolerating per-query HTTP 400 (architect ad-9 / risk_analyst R4). Single-OR fails on team-managed projects that lack the "Epic Link" custom field; auto-detection silently downgrades to fresh-path and the sweep returns empty. Exports the helper `search_epic_children` so TASK-1-12 reuses it. Top non-blocking (reviewer-flagged as most impactful): - #1 In-flight gate trust-boundary trade-off: add explicit acknowledgement that gateway-side enforcement is deferred and v1 relies on agent-side gating + apply-time re-check by TASK-1-13. - #5 APPLY_EPIC role registration: expand TASK-1-10 to enumerate all FIVE registration steps (AgentRole, AgentRoleDefinition, get_roles_for_phase, file-restrictions patterns, spawner branch). - #6 epic_apply persistence MCP surface: add `mcp__sdlc__update_epic_apply` MCP tool to TASK-1-7 so the sandbox-side agent can persist artifact updates. - #7 Concurrent-edit guard: TASK-1-10 now fetches the current epic Description, sha256s it, and registers a divergence HITL on mismatch; TASK-1-9 records the baseline sha256; TASK-1-7 adds `refine_description_sha256` to the schema. Additional non-blockers folded in: - #2: jira_effective_mode added to primitives table. - #3: TASK-1-5 introduces `shared/egg_jira_credentials.py` shared module to eliminate the orchestrator → gateway coupling. - #8: TASK-1-11 commits to extending `parse_plan` (not pass-through). - #9: TASK-1-5/TASK-1-14 add already-in-state idempotent short-circuit for Won't-Do transitions. - #10: TASK-1-15 introduces `Pipeline.jira_parent_epic_key` so PR phase doesn't need an extra Jira call. - #11: TASK-1-16 adds `PipelinePhase.PLAN_STOPPED` documented terminal phase + updates overseer monitor short-circuit. - #14: TASK-1-11 requires `wont_do_reason` per node + ⚠ warning rendering in the plan draft (R6). - #15: TASK-1-5 gates the orchestrator-direct cred surface behind `EGG_ENABLE_ORCH_JIRA_TRANSITIONS` (default off — R1). - #16: TASK-1-7 schema gains `version`, `idempotency_seed`, per-edit `summary_hash` + `applied_at`, `wont_do_reason`, signal_source as a list (R10). - #19: TASK-1-19 drops orchestrator-cli.md, adds submit-task-mcp.md. - #13: TASK-1-18 adds the lint regression test `test_no_outbound_jira_writes.py` (R7). - #12: TASK-1-12 introduces a reverse-index `.egg-state/jira-child-pipeline-index.json` to bound the sweep to O(K) (R3 performance mitigation). - #20: New "Risk-analyst items addressed" section summarises how R1/R2/R6/R7/R10/R12 are resolved in-plan (no fresh HITLs). Plan still parses cleanly: 1 slice, 19 tasks, 0 warnings, 0 role-alignment errors. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Re-proposal addressing reviewer_code NACK on v1. The six blocking findings were all "test coverage exists for the feature but doesn't exercise the specific v5 mitigation, so a regression that re-introduces the original bug would slip through". Each is now closed: 1. **`test_jira_reassess_detection.py::TestRunJql`** — added five pagination tests covering reviewer_code v3 #6 mitigation: `test_paginates_via_next_page_token` (3 pages with cursor threading end-to-end), `test_paginates_via_envelope_wrapped_response` (gateway `make_success` envelope unwrap), `test_pagination_stops_when_next_page_token_missing`, `test_pagination_stops_when_next_page_token_not_a_string`, and `test_pagination_hard_cap_terminates_loop` (200-page cap on infinite loops). Pre-fix code that returns after page 1 fails the multi-page tests; pre-cap code spinning forever fails the cap test. 2. **`test_jira_transitions_client.py::TestHappyPath::test_short_circuits_on_done_status_with_wont_do_resolution`** — exercises the resolution-branch of the already-in-state check (reviewer_code v3 #7 mitigation). Mocks the common Atlassian shape (status=Done + statusCategory.key=done + resolution=Won't Do), asserts `result.status == "already_in_state"` and that POST was NOT issued. Companion `test_does_not_short_circuit_on_done_with_other_resolution` pins the negative case (Done + resolution=Fixed → POST issued). 3. **`test_jira_transitions_client.py::TestHappyPath::test_post_body_wraps_comment_in_adf_document`** — asserts the EXACT ADF document shape (`{"type": "doc", "version": 1, "content": [{"type": "paragraph", "content": [{"type": "text", "text": …}]}]}`) for the comment body (reviewer_code v3 #8 mitigation). Pre-fix code that sends a raw string would slip past the original `"comment" in body["update"]` smoke check; this test pins the canonical ADF shape so a regression surfaces. Companion `test_empty_comment_omits_update_block` pins the empty-string fallback (no ADF wrapper for `""`/whitespace). 4. **`test_epic_apply_artifact.py::TestGetEpicApplyMalformedWarning`** — three tests (malformed JSON / JSON-but-schema-invalid / absent) covering reviewer_code v3 #10 mitigation. Patches `models._models_logger.warning` (the orchestrator uses egg_logging / structlog which bypasses caplog) and asserts the `epic_apply_artifact_invalid` event is emitted with the correct `reason` (`json_decode_failed` vs `pydantic_validation_failed`) and `pipeline_id` fields. Absent-artifact path is silent (no false warnings). 5. **`test_epic_apply_artifact.py::TestJiraTicketAndEpicKeyMutualExclusivity`** — five tests for the model-validator at `orchestrator/models.py:1243-1263` (reviewer_code v3 #11 mitigation). Asserts ValidationError when BOTH `jira_ticket` and `jira_epic_key` are set, AND that single-field pipelines (only `jira_ticket`, only `jira_epic_key`, neither) still work, AND that `jira_parent_epic_key` doesn't sidestep the mutual-exclusivity rule. 6. **`test_jira_epic_inputs.py::TestComputeDescriptionSha256`** — eight tests with non-circular assertions (every expected hash computed independently of the production helper). Covers: key-order invariance on ADF dicts (`sort_keys=True` mandate), ADF-vs-flattened-text distinction (the whole point of #7), plain-string utf-8 encoding, unicode utf-8 encoding, None → empty-string sha256 (with the well-known `e3b0c4…b855` digest pinned independently), unknown-shape fallback to `str(value).encode("utf-8")`, ADF non-ASCII without double-encoding (`ensure_ascii=False`), and separator-compactness (`separators=(",", ":")`). A refactor that drops any of these canonicalisation knobs surfaces a different hash and fails the test. Also (caught by the new pagination tests as I authored them) — the production pagination loop at `orchestrator/jira_epic_detect.py:271-272` correctly only handles envelope-wrapped responses (`response.get("data")` for the pagination cursor); test fixtures now mirror that shape (matches the gateway's `make_success` envelope). Plus one polish: `test_pipeline_prompts.py::test_plan_non_epic_omits_epic_section` dropped an over-optimistic `slice-DAG` positive assertion — that framing is layered on by higher-up builders, not `_build_phase_prompt` directly. The byte-clean-of-epic check (the actual regression guard) is retained. Counts: 1137 tests pass (up from 1119), 9 skipped (kubectl-gated integration scaffolds), 1.75s end-to-end via `PYTHONPATH=shared:gateway:orchestrator python -m pytest <17 files>`. `make lint` exits 0 (ruff check + ruff format + mypy + size-cap warnings only). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Tighten the substrate-swap walking-skeleton spike against the v1 review (egg-reviewer bot, PR #2715): - #1 + #2 (k3s leg silently broken under `EGG_SUBSTRATE=k3s`): gate the `_spawn_agent` seam on `claude-code` only. Unset / `k3s` / any other value keeps the legacy `self.spawn_fn(...)` path so branch-aware spawn and the BRC consensus-wrapped command survive. Update the protocol docstring and ADR to acknowledge that `K3sSpawnerAdapter` returns `commit_sha=None` by design (gateway attestation is authoritative for k3s INV-6); follow-up plumbs it through. - #3 (per-role worktree teardown): add `LocalWorktreeManager.remove(pipeline_id, role)` and call it from both substrate failure paths so one bad spawn no longer wipes peer worktrees mid-spawn under concurrent dispatch. - #4 (bash hook fail-open framing): rewrite the threat-model docstring from "load-bearing enforcement layer" to "first-tier filter with MCP-validator second tier per R2 deferral"; widen the verb walker to catch `rm`, `chmod`, `chown`, `truncate`, `awk -i inplace`, `perl -i`, `wget -O` / `curl -o` / `--output-dir`, `git mv|rm|apply|checkout|restore`, `tar -x`, `unzip`, and shell-of-shell forms (`bash -c`, `sh -c`, …) which recurse into the inner command; tighten the `python3 -m` allow-list to the named hook entry only. - #5 (refiner rubric never loaded): inject `role_rubric_loader=_load_egg_sdlc_role_rubric` in `select_substrate` so `build_system_prompt` actually receives the 119-line rubric from `plugins/egg-sdlc/.../agents/refiner.md` instead of the trivial fallback string. - #6 (heredoc-HITL bridge gap): SKILL.md + ADR now document, in a callout, that the multi-yield generator↔`AskUserQuestion` bridge from a Bash-spawned `python3` subprocess is unsolved in the spike; the in-process machinery is correct within a single-pass invocation. The follow-up issue draft adds an explicit "close the heredoc-HITL bridge gap" bullet with two candidate designs (long-lived REPL/daemon vs. flattened single-yield stages). - #7 (_PreflightAborted translation): wrap the generator body so `_PreflightAborted` translates into a clean StopIteration whose `.value` carries the diagnostic message. Tests now pin `pytest.raises(StopIteration)` rather than the previous "either StopIteration or _PreflightAborted" disjunction. - #8 (plugin metadata `python_dependency` TODO): replace the non-actionable TODO with structured from-source install instructions in `plugin.json` `egg.install_instructions`; preflight.py + SKILL.md read from that single source and emit actionable `git clone … && pip install -r requirements.txt …` guidance. - #9 (tests pinned as fixture not behavior): rename `test_inv3_stale_ack_rejected_when_bus_used_as_transport` → `…_by_tracker_alongside_bus` and similar to honestly reflect that INV-3 / INV-5 live in PeerConsensusTracker, not the bus; drop the unconditionally-skipped k3s parametrize on the bus round-trip smoke test in favor of a claude-code-only test. - #10 (pre-existing SyntaxError in conftest.py): fix both unparenthesised `except A, B:` clauses with `# fmt: skip` so ruff format does not strip the parens again. The conftest file is now importable, so the new substrate fixture is actually live. Plus the easy non-blocking items: use `import threading` instead of `__import__('threading')`, defer `DEFAULT_BASE` evaluation to `LocalWorktreeManager.__init__` so `monkeypatch.setenv('HOME', …)` in tests works, and short-circuit the in-process background ticks when the substrate bundle's bus is a `_K3sPlaceholder`. Tests: 105 pass / 3 skipped (env-required) across the substrate unit suites and `test_substrate_smoke.py`; `make lint`-equivalent `ruff check + ruff format --check` are clean. Authored-by: egg

…2715) * Initialize SDLC contract for issue #2623 * refine(#2623): substrate-swap analysis Drafts the refine-phase analysis for running egg's full SDLC stack natively in Claude Code. Frames the substrate swap from k3s/Redis/Docker to Agent tool / in-process bus / PreToolUse hooks; recommends Option A (parallel substrates with named AgentSpawner/MessageBus/PolicyEnforcer interfaces and a conformance CI matrix) and registers 11 multiple-choice decisions plus 6 open-ended feedback questions covering substrate coexistence, phase scope, conformance scoping, spawner shape, worktree ownership, policy enforcement seam, HITL surface, install footprint, k3s deprecation timing, context-window strategy, and slice-DAG shape. Authored-by: egg * plan(2623): walking-skeleton slice for Claude Code substrate spike Spike-then-plan single slice (cq-11) — one role (refiner) end-to-end on the Claude Code substrate. Lands the four substrate interfaces, the claude-code implementations, the in-process orchestrator boot generator, the egg-sdlc skill, one parametrized regression test, the ADR, and a reviewer-pasted follow-up issue draft. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * plan(2623): split TASK-1-7 markdown into documenter TASK-1-11 Reviewer feedback (pre-propose): coder role is blocked from `**/*.md` files, so SKILL.md and agents/refiner.md must move out of TASK-1-7 into a new documenter task. TASK-1-7 now ships plugin.json only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * plan(#2623): architect — substrate-swap walking-skeleton analysis Document the AgentSpawner / MessageBus / PolicyEnforcer / WorktreeManager / HITLSurface ABCs and a ClaudeCodeSpawner spike that proves the substrate-swap shape on one role (refiner) + one phase (refine). Honors all 11 HITL resolutions (Option A parallel substrates, all-phases target, integration_tests/regression CI matrix, synchronous spawn, WORKTREE_BASE_DIR port, PreToolUse hook policy, heredoc HITL, pip-dep plugin manifest, k3s co-equal, hybrid checkpoint+fork, spike-first slicing). Defers multi-role + plan/implement/pr phases + PreToolUse Bash interception + k3s deprecation to follow-up issues. Includes 48 file:line citations for every cited runtime primitive (spawner, message bus, policy module, contract schema, BRC invariants, worktree manager) and surfaces execution-context dimensions per #2594 (deployed-pod vs trusted-CI-runner vs parent-claude-code-session). Hands 9 candidate tasks to task_planner and a 7-item risk list to risk_analyst. Authored-by: egg * plan(#2623): risk assessment for substrate-swap (k3s -> claude-code native) Add risk_analyst output identifying 16 risks across security, design, performance, and compatibility categories. Overall risk HIGH driven by (a) credential trust-boundary inversion (gateway -> user session), (b) unverified PreToolUse-hook role-routing primitive (#2594 class), and (c) spike-then-plan slicing that risks freezing interface shape from a single-role exercise. Five runtime primitives flagged for spike-time verification: Agent tool worktree isolation, PreToolUse hook role-routing, subagent concurrency ceiling, subagent context budget, and custom subagent_type via .claude/agents/. Five trust-boundary shifts documented: credential isolation, file-write enforcement timing, cost/rate-limit control, agent liveness signals, and push serialization. Five high-priority recommendations: spike must surface evidence on the five primitives; ADR must explicitly accept the trust-shift; plan must classify the 14 regression tests; consider 2-role spike scope; pipeline cost cap. Recommendation: PROCEED_WITH_MITIGATIONS. Five areas require explicit human review (R1, R2, R4, R7, R10). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * plan(2623): address reviewer_plan NACK — 11 blockers + non-blocking items Blocker fixes: 1. Trust-Boundary section rewritten to cite EggStack at conftest.py:71, egg_stack fixture :340, orchestrator_url :357; dropped reference to the deleted local_pipeline/conftest.py tree. 2. TASK-1-8 now creates a NEW substrate-distinguishing test integration_tests/regression/test_substrate_smoke.py that exercises select_substrate(...).spawner.spawn() and .bus.add_message/get_messages directly; the prior test_brc_single_cycle.py target was pure-Python and could not substrate-distinguish. 3. k3s adapter contradiction resolved: TASK-1-1 now ships a WORKING K3sSpawnerAdapter wrapping orchestrator/kubernetes_spawner.py:1564 create_concurrent_spawn_fn, capturing commit_sha via git rev-parse HEAD. The only NotImplementedError lives in TASK-1-6's run_pipeline_in_process k3s leg (deliberate cq-11 scope-fence). 4. AgentResult now includes commit_sha: str | None (INV-6 per orchestrator/action_guards.py:631, body :757). TASK-1-1 + TASK-1-2 acceptance criteria updated. 5. TASK-1-5 cites gateway/worktree_manager.py:1711 is_relative_to defense (the correct path; gateway.py:7903 was wrong). 6. TASK-1-3 acceptance points at orchestrator/tests/test_brc_*.py as the behavioral oracle; TASK-1-8 mirrors scenarios from test_brc_open_nacks_barrier.py and test_brc_content_validation.py. 7. TASK-1-6 expanded with explicit R4 refactor acceptance criteria: heartbeat-during-HITL liveness, background-thread lifetime on GeneratorExit, contract-state synchronization. 8. TASK-1-9 ADR acceptance now covers R1 (trust-context shift / cred exposure), R2 (PreToolUse hook fallback to MCP-validator-side), R7 (subagent context budget regression), R10 (interface stability v0.x marker), plus REC5 (cost cap recommendation) and R15 (subagent_type model choice). 9. TASK-1-8 kubectl claim corrected — both substrate parameters run pure-Python in-process; k3s leg mocks underlying job dispatch. 10. cq-12 registered as new HITL: canonical pip name + registry. TASK-1-7 references cq-12 instead of inventing "egg-shared". 11. Primitives table EggStack :78 → :71 (and other line numbers sanity-checked). Non-blocking items addressed inline (interface stability, subagent type, cost cap, smoke test for _spawn_agent dispatch, k3s adapter file moved to TASK-1-1 deliverable surface). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Persist agent statefile writes before refine sync * Persist statefiles after refine phase * Persist HITL resolution after refine phase gate * Persist agent statefile writes before plan sync * Populate contract for 2623 (#2629) * Persist statefiles after plan phase * docs(2623): claude-code substrate ADR + egg-sdlc skill (TASK-1-9/10/11) Lands the documenter side of the spike for #2623's walking-skeleton Claude Code substrate. No code touched; this is purely the user-facing markdown the substrate-swap promised. TASK-1-9 — docs/architecture/claude-code-substrate.md ADR-style design doc. Names the four `Protocol`s (`AgentSpawner`, `MessageBus`, `PolicyEnforcer`, `WorktreeManager`), the `EGG_SUBSTRATE` selector, the working `K3sSpawnerAdapter` shim and the claude-code implementations (`ClaudeCodeSpawner` + `InProcessMessageBus` + `PreToolUseHookPolicy` + `LocalWorktreeManager`), the in-process orchestrator generator (`run_pipeline_in_process`), and the egg-sdlc plugin entry point. Covers all twelve cq decisions (cq-1..cq-12) and all six feedback items. Risk-mitigation subsections for R1 (trust-context shift) / R2 (PreToolUse hook fallback) / R7 (subagent context budget) / R10 (interface stability marker) / R15 (subagent type model) plus REC5 (cost cap). Existing + new primitives are enumerated in the Primitives table. Linked from docs/architecture/README.md so the new doc joins the Key Architectural Decisions list. TASK-1-10 — Follow-up issue draft section Appended to the same ADR file (documenter is role-blocked from `.github/` so the section is reviewer-pasted, not auto-filed). Lists the deferred rollout: plan/implement/pr phases, full 5-issue conformance matrix, perf/latency budget, full k3s interface adapter, optional `EggHarnessSpawner`, `egg-state prune` verb, fork-based sub-task delegation, `EGG_PIPELINE_MAX_AGENT_INVOCATIONS`, and the custom `subagent_type` migration. Section header states explicitly "reviewer-pasted, not auto-filed". TASK-1-11 — plugins/egg-sdlc/skills/egg-sdlc/{SKILL.md,agents/refiner.md} SKILL.md documents the heredoc-HITL user-facing contract: how the parent session drives `run_pipeline_in_process(...)` and renders each yielded `HITLDecision` via `AskUserQuestion`. States explicitly that the spike's exercised scope is refiner-only (plan/implement/pr roles documented as out of scope, matching TASK-1-7's plugin metadata). Cross-links the trust-context shift, the PreToolUse hook fallback, and the follow-up issue draft. The refiner role file mirrors the `plugins/refine-plan/skills/refine-plan/agents/refiner.md` layout (frontmatter + body) so the in-process orchestrator's `build_system_prompt(sources)` can read it without per-skill custom logic. Substrate-specific operational notes (worktree layout, PreToolUse-hook enforcement, context-budget hybrid, HITL surface, absent reviewer dialog) appear once at the bottom — they don't change WHAT the refiner produces, only HOW it operates. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(2623): scaffold substrate-swap tests (TASK-1-8) Add scaffolding for slice-1 task-1-8 covering the eight planned files: * integration_tests/regression/conftest.py — substrate fixture parametrized over ('k3s', 'claude-code'); claude-code dim skips inside an in-sandbox-agent trust context (EGG_AGENT_ROLE set). * integration_tests/regression/test_substrate_smoke.py — end-to-end spawner.spawn + bus round-trip smoke for both substrate dims. * shared/tests/test_substrate_interfaces.py — Protocol presence, AgentSpawner.spawn signature (cq-4), AgentResult.commit_sha field (INV-6), select_substrate env-var contract (cq-1). * shared/tests/test_claude_code_spawner.py — ClaudeCodeSpawner conformance + commit_sha capture + build_system_prompt invocation (depth-gap structural fix, #2622). * shared/tests/test_k3s_spawner_adapter.py — K3sSpawnerAdapter conformance + create_concurrent_spawn_fn delegation + commit_sha capture for the k3s leg. * shared/tests/test_in_process_message_bus.py — InProcessMessageBus round-trip + pipeline isolation + INV-3 / INV-5 oracle scaffolding. * shared/tests/test_pretooluse_hook_policy.py — PreToolUseHookPolicy denies out-of-role writes; hook_entry.py script exit-code contract. * shared/tests/test_local_worktree_manager.py — LocalWorktreeManager path-escape rejection mirroring gateway/worktree_manager.py:88/110. * shared/tests/test_run_pipeline_in_process.py — generator entry point AC bullets: NotImplementedError on EGG_SUBSTRATE=k3s, heartbeat-thread liveness across HITL yields, clean thread drop on GeneratorExit (TASK-1-6). All test bodies that depend on coder-side symbols still pending are pytest.skip with explicit pointers to the task that gates them, so collection stays green and the fail-mode is informative once the coder commits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(2623): address reviewer_code v1 NACK (blockers + non-blocking polish) Reviewer_code NACK on v1 named 2 blockers and 7 non-blocking items. This commit addresses all of them. Blocker 1 — refiner.md allow-list factual error. Drop `docs/templates/` from the listed PreToolUse-hook allow-list. `REFINER_PATTERNS.allowed_patterns` at `shared/egg_restrictions/patterns.py:491-494` only lists `.egg-state/drafts/` and `.egg-state/agent-outputs/`. Refiner reads `docs/templates/analysis.md` (referenced earlier in the same file) but cannot write there. Clarify that the template is read-only. Blocker 2 — worktree default-base contradiction in ADR + SKILL.md. ADR substrate table at line 21 said `.egg-state/<pipeline_id>/<repo>/` but the WorktreeManager section at line 88 said `~/.egg-worktrees/`. SKILL.md had the same split. Per plan TASK-1-5 acceptance the default base mirrors the gateway shape (`~/.egg-worktrees/`) and `EGG_WORKTREE_BASE` overrides — the typical override points the base at `./.egg-state/` so worktrees live alongside contract / drafts state. Both files now state this consistently: default is `~/.egg-worktrees/<pipeline_id>/<repo>/`, with a footnote that `gateway/worktree_manager.py:49` hardcodes `/home/egg/.egg-worktrees` for the gateway container (Claude-Code-substrate expands `~` against the calling user's `$HOME`). SKILL.md now shows both layouts (default and typical override) side-by-side. Non-blocking 1 — SKILL.md pip-install placeholder callout. Added a TODO callout warning users not to copy-paste the literal placeholder; instructs them to read the real string from plugin.json. Notes the install-error-match contract holds string equality on a placeholder until cq-12 lands. Non-blocking 2 — SKILL.md allowed-tools least-privilege. Removed `Write Edit` from the skill's allowed-tools frontmatter. The skill itself only spawns Agents, reads files, and asks questions; the refiner subagent writes inside its own worktree. Non-blocking 3 — SKILL.md awkward "destination of the ADR" wording. Reworded to "user-facing entry point for the ADR". Non-blocking 4 — SKILL.md install-error-match contract caveat. Note added that the contract is testing string equality of placeholders until cq-12 resolves. Non-blocking 5 — docs/architecture/README.md run-on index entry. Split the one-line entry into two sentences. First sentence names what landed; second sentence describes the risk-doc cross-refs. Non-blocking 6 — ADR R2 empirical-question ownership. Clarified: spike merges with the hook in place and single-role evidence; follow-up takes ownership of the multi-role / nested subagent validation. Prose now matches the Follow-up issue draft appendix entry. Non-blocking 7 — ADR R10 "thought-experimented" reword. Changed to "the plan's design reviewer reasoned through the interfaces against the full role roster, but design review is not a substitute for end-to-end exercise." Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * coder(#2623): walking-skeleton substrate interfaces + Claude Code impls Walking-skeleton implementation of the four substrate protocols and the in-process orchestrator entry point that lets egg's SDLC stack run natively in Claude Code (cq-11 = "Spike then plan"). Tasks satisfied: - TASK-1-1: substrate interfaces (AgentSpawner / MessageBus / PolicyEnforcer / WorktreeManager protocols + select_substrate factory) and k3s adapter shim wrapping KubernetesSpawner.create_concurrent_spawn_fn so both legs are working from day one. AgentResult carries commit_sha for INV-6. - TASK-1-2: ClaudeCodeSpawner that drives the egg_harness subagent surface, assembles the system prompt via build_system_prompt(...) per #2622, and captures commit_sha via git rev-parse HEAD. Also patches concurrent_executor._spawn_agent to dispatch through select_substrate(...) when EGG_SUBSTRATE is set; default path (unset) preserves the legacy k3s behavior verbatim. - TASK-1-3: InProcessMessageBus subclassing MessageStore so BRC invariants INV-3 / INV-5 stay enforced unchanged by PeerConsensusTracker. - TASK-1-4: PreToolUseHookPolicy + a runnable hook entry script that imports check_agent_file_access from shared/egg_restrictions/ checker.py (the same symbol gateway/phase_filter.py uses) — no parallel restriction logic. Ships a .claude/settings.json template. - TASK-1-5: LocalWorktreeManager under .egg-state/<pipeline>/<role>/ with is_relative_to path-escape defense mirroring gateway/worktree_manager.py:1711. - TASK-1-6: run_pipeline_in_process generator yielding HITLDecision (cq-7 heredoc-HITL), with heartbeat / BRC-review / bus-tick background threads that stay alive during yields and join cleanly on both normal return and GeneratorExit. EGG_SUBSTRATE=k3s raises NotImplementedError naming the follow-up issue (cq-11 scope-fence). - TASK-1-7: plugins/egg-sdlc/.claude-plugin/plugin.json declaring the pip dependency (cq-12 unresolved — carries a TODO placeholder pointing at the ADR follow-up issue) plus the pre-flight helper that imports egg_orchestrator and emits the matching install instruction when missing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(2623): address reviewer_code v2 NACK (worktree path + checkpoint path + citation) Reviewer_code v2 NACK named 2 blockers and 1 non-blocking citation polish. v1's worktree-default fix landed correctly on the ADR substrate table, the WorktreeManager section, and the SKILL.md filesystem-layout diagram, but did NOT propagate to refiner.md or the cq-5 row in the ADR decisions table. v2's SKILL.md rewrite of the layout diagram also introduced a new contradiction on the checkpoint path. This commit finishes both. Blocker 1 — refiner.md:106 worktree-path contradiction. refiner.md was still saying `.egg-state/<pipeline_id>/<repo>/` — the path the ADR + SKILL.md now describe only as the typical override layout, not the default. A refiner reading the role file would assume its worktree lives there regardless of how the operator configured EGG_WORKTREE_BASE. Reworded to `<EGG_WORKTREE_BASE>/<pipeline_id>/<repo>/` with the default resolution (`~/.egg-worktrees/`) named inline. Same fix applied to the ADR's cq-5 decisions-table row. Blocker 2 — checkpoint-path contradiction across SKILL.md / ADR / refiner.md. v2's SKILL.md layout diagram showed `.egg-state/checkpoints/` (no `<pipeline_id>` segment), but ADR feedback Q6 said `.egg-state/<pipeline_id>/checkpoints/` and refiner.md gave the same per-pipeline-grouped path. Picked sibling-shaped (`.egg-state/checkpoints/<pipeline_id>/`) to match the rest of `.egg-state/`'s top-level layout (drafts, contracts, agent-outputs, brc-history are all sibling-shaped today). Updated SKILL.md diagrams (both default and override layouts) + ADR:54 + refiner.md:108 consistently. Also fixed the diagram's misleading caption `# state files (relative to the repo)` to `# state files (relative to the in-process orchestrator's CWD)` per the same NACK's non-blocking ambiguity note. Non-blocking — `_remove_worktree` citation error in ADR. ADR:88 cited "call site within _remove_worktree" but the function containing the `is_relative_to` defense at lines 1700-1711 is `list_orphan_worktree_dirs` (defined at :1687); no `_remove_worktree` exists in `gateway/worktree_manager.py` at all. Rewrote the citation in the WorktreeManager section, the Primitives table row, and the SKILL.md worktree-layout section to name the correct function and the correct line range. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(2623): align worktree path with shipped code (<role> not <repo>) After merging the coder's commits, verified the doc strings against what actually landed in orchestrator/substrate/claude_code/worktree.py. The shipped `LocalWorktreeManager.create(pipeline_id, role)` creates worktrees at `<base>/<pipeline_id>/<role>/` and branches them on `egg/<pipeline_id>/<role>` — i.e., keyed by ROLE, not REPO. The docs were saying `<base>/<pipeline_id>/<repo>/` throughout (a mistake carried through three review cycles before the code landed to disprove it). Fixed in all three sites: - docs/architecture/claude-code-substrate.md substrate table, cq-5 decisions row, and WorktreeManager section. Added an explicit rationale note ("path keys on role, not repo, because the in-process orchestrator runs against a single repo per pipeline and the worktree's per-role isolation is what matters") and the branch-name convention. - plugins/egg-sdlc/skills/egg-sdlc/SKILL.md usage section, worktree diagrams (both default and override layouts). - plugins/egg-sdlc/skills/egg-sdlc/agents/refiner.md substrate-notes bullet — refiner instance now spells out the per-role path with its actual role name (`/refiner/`) and branch (`egg/<pipeline_id>/refiner`). The reviewer-flagged v2 NACK trigger ("if the coder picks a worktree default that differs from `~/.egg-worktrees/`, that's another re-review trigger for the documenter") is satisfied: code uses `~/.egg-worktrees/` (Path expansion of `HOME` env var) as the default, which is what the docs already say. Checkpoint path verification against the shipped code: the spike's `_ensure_state_dirs()` creates `.egg-state/checkpoints/` (no pipeline_id sub-shard yet — the spike provisions the directory but defers individual checkpoint-file format to the cq-10 follow-up half). The docs' `.egg-state/checkpoints/<pipeline_id>/` per-pipeline-shard spec is forward-compatible with this — the spike code creates the parent directory; the follow-up's actual checkpoint-write code will create the `<pipeline_id>/` sub-shard. No edit needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(2623): refine substrate tests against coder's implementation Update the scaffold test bodies to assert against the actual API the coder shipped: * test_substrate_interfaces.py — Protocol presence + runtime_checkable; case-insensitive env handling; explicit K3sSpawnerAdapter wrapping when a legacy spawn fn is supplied. Adds a deliberately FAILING test (test_select_substrate_k3s_default_spawner_is_working) that demonstrates the task-1-1 AC violation: select_substrate({}) currently returns a _DeferredK3sSpawner stub that raises NotImplementedError on .spawn(), but the AC requires 'a working K3sSpawnerAdapter wrapping create_concurrent_spawn_fn'. Pairs with an explicit NACK on the coder's proposal naming this test. * test_claude_code_spawner.py — isinstance vs the runtime-checkable Protocol; AgentResult shape; commit_sha capture; build_system_prompt invocation (verifies #2622 structural depth fix); EGG_AGENT_ROLE / EGG_WORKTREE_ROOT injection. * test_k3s_spawner_adapter.py — adapter satisfies AgentSpawner; delegates to wrapped closure with role/env; AgentResult fields populated from legacy SpawnedContainer; commit_sha capture (INV-6). * test_in_process_message_bus.py — Message-shaped add_message round trip; pipeline isolation; MessageStore subclass discipline; INV-3 stale-version ACK rejected via tracker over the bus; INV-5 multi-reviewer open-NACK barrier preserved. * test_pretooluse_hook_policy.py — check_write decision matrix (tester/coder/documenter); hook_entry.decide() block vs allow vs fail-open; subprocess round-trip of the hook entry script; install() writes / merges .claude/settings.json idempotently. * test_local_worktree_manager.py — base resolution via EGG_WORKTREE_BASE override; path-escape rejection for nine bad identifiers; per-role isolation under <base>/<pipeline_id>/<role>/; tear_down honors the is_relative_to guard (mirrors gateway/worktree_manager.py:1711); tear_down validates pipeline_id input. * test_run_pipeline_in_process.py — k3s substrate rejection (NotImplementedError with helpful message); heartbeat thread keeps ticking across HITL yields (acceptance bullet 2 of TASK-1-6); background threads cleanly dropped on GeneratorExit (acceptance bullet 3 of TASK-1-6); generator returns artifact path on terminal HITL answer. * test_substrate_smoke.py — integration smoke parametrized over both substrate dimensions (claude-code skipped inside sandbox-agent context per task-1-8 AC); bundle field presence; spawn returns AgentResult on both legs; bus round-trip; INV-3 preserved end-to-end. Lint/format clean (ruff check + ruff format). Tests: 65 PASSED, 1 FAILED (the deliberate AC-violation test), 8 SKIPPED (claude-code dim in sandbox + git-init blocked in container). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * coder(#2623): v2 — address reviewer NACKs (security/contract/holistic) Aggregated fixes for the reviewer_security, reviewer_contract, and reviewer_code_holistic v1 NACKs. Security blockers (reviewer_security v1 NACK): - PreToolUse matcher adds Bash; hook parses Bash commands for write targets via redirection (>/>>/&>/2>), tee, cp/mv/install/rsync, dd of=, sed -i, ln -s, and python3 -c "open(...).write(...)" heuristics. Ambiguous parses (shell expansion, $(...)/`...`) fail closed. - Path resolution uses Path.resolve() instead of os.path.normpath, so symlink targets are followed before the prefix comparison (matches gateway/worktree_manager.py:1711). Original paths resolving outside the repo root are denied. - JSONDecodeError fails closed — the gateway is gone in the claude-code substrate; the hook IS the load-bearing enforcement layer. - Missing EGG_AGENT_ROLE fails closed when the write target is inside a substrate-managed prefix (.egg-state/, .claude/, .github/, shared/egg_restrictions/); writes outside continue to fail-open so the user's plain Claude Code session is unaffected. Contract blockers (reviewer_contract v1 NACK): - select_substrate({}) now returns a working K3sSpawnerAdapter via the new _LazyK3sSpawner that constructs the KubernetesSpawner.create_concurrent_spawn_fn factory on first spawn (cq-1 co-equal substrates from day one). - _spawn_refiner now imports ConcurrentPhaseExecutor and PeerConsensusTracker so both primitives are in the in-process generator's call graph (task-1-6 acceptance bullet 6); the BRC re-review background thread also calls into PeerConsensusTracker via get_peer_consensus_tracker(pipeline_id). Holistic blockers (reviewer_code_holistic v1 NACK): - concurrent_executor.py:590 — fixed `from substrate import` to use `from orchestrator.substrate import` with a sandbox fallback. - run_pipeline_in_process now sets effective_env["EGG_SUBSTRATE"] after defaulting unset to "claude-code" so select_substrate sees a consistent value. - preflight.py probes `orchestrator.substrate.in_process.run_pipeline_in_process` — the actual runtime dependency — instead of `egg_orchestrator`, which is the unrelated API client. - AgentResult from the refiner spawn is now bound and surfaced: the refine HITL gate switches to refine-failure-<id> with retry/abort options when exit_code != 0; placeholder analysis body now exposes exit_code, commit_sha, and stdout tail. - Background threads (heartbeat / brc-review / bus-tick) now do real work: publish HEARTBEAT messages to InProcessMessageBus, tick the PeerConsensusTracker, and pump the bus's condition variables. - Spawner sentinel file at $HOME/.claude/egg-active-role.json lets the PreToolUse hook resolve the active role across the spawner↔hook process boundary when EGG_AGENT_ROLE is not propagated. - Hook adds a direct `build_agent_patterns` import alongside `check_agent_file_access` so the literal task-1-4 acceptance bullet 5 is satisfied (deny-by-default if role is not in the pattern registry). - Spawner docstring is rewritten to clarify the spike runs egg_harness in-process (NOT Claude Code's native Agent tool with subagent_type="general-purpose") and documents the follow-up issue. - K3sSpawnerAdapter emits a structured stderr warning when commit_sha is None so INV-6 attach-time failures correlate. Non-blocking polish: - PreToolUseHookPolicy.install() enforces target_dir is under $HOME (path-escape guard). - Preflight abort answer at the first HITL yield raises a clean _PreflightAborted RuntimeError instead of running the refiner. - plugin.json TODO marker updated to reference the new in_process.run_pipeline_in_process probe. Tests: 122 unit tests pass; substrate end-to-end smoke test (preflight → refiner spawn → refine HITL gate → clean shutdown) green; background-thread join verified on both StopIteration and GeneratorExit; ruff lint passes. Note on `except (X, Y):` style: ruff format under target-version=py314 strips redundant parens from this construct. Python 3.14's grammar parses both `except X, Y:` and `except (X, Y):` identically (as a tuple). This is the existing codebase style (62+ instances across orchestrator/health_checks/, overseer/, routes/). Reviewers reading these as Python-2-style: the project's formatter enforces this form; the semantics are unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * coder(#2623): v3 — Python 3.10-3.13 compat + concurrency-NACK fixes Aggregated fixes for reviewer_code, reviewer_code_holistic, reviewer_contract, tester, reviewer_security, and reviewer_concurrency v1 NACKs. Python compat (reviewer_code blocker #1, reviewer_code_holistic blocker #1): - All 11 `except A, B:` sites in the substrate package and concurrent_executor patch now use `except (A, B): # fmt: skip` (ruff format under target-version=py314 strips redundant parens; the fmt: skip directive preserves them so the code is also valid on Python 3.10-3.13 per SKILL.md's documented "Python 3.11+" target). Concurrency fixes (reviewer_concurrency v1 NACK): - _write_pending_decision: wraps the read-modify-write in fcntl.flock(LOCK_EX) on a sidecar .lock file and writes via temp + os.replace for atomic publish (blocker #1: TOCTOU race on contracts/<id>.json). - _InProcessOrchestrator.run(): tears down per-pipeline worktrees in the finally block so generator drop / fence / completion all release the worktree (blocker #2: worktree leak on every exit path). - _spawn_agent_via_substrate: wraps the spawn in try/except, tears down the worktree on both exception and FAILED-exit paths, and returns AgentExecution(status=FAILED) on exception so handle_agent_failure-equivalent recovery isn't bypassed (blockers #3, #5). - K3sSpawnerAdapter: drops the racy commit_sha capture entirely (the fire-and-monitor factory returns before the pod commits, so capturing pre-spawn HEAD would attach BRC ACKs to the wrong SHA). Now returns commit_sha=None and emits a structured stderr note pointing at the gateway-side attestation channel (blocker #4: INV-6 commit_sha temporal ordering bug). The follow-up issue covers wiring the post-spawn attestation through. Combined with v2, this addresses every blocker from all six reviewers: - reviewer_security v1 (4): hook Bash matcher + parser, Path.resolve symlink- aware repo-relative, JSONDecodeError fail-closed, missing-role fail-closed in restricted prefixes - reviewer_contract v1 (2): _LazyK3sSpawner so select_substrate({}) returns working K3sSpawnerAdapter; in_process imports ConcurrentPhaseExecutor + PeerConsensusTracker so both primitives are in the call graph - tester v1 (1): same as reviewer_contract #1 - reviewer_code_holistic v1 (12): except syntax (this commit), import path, spawner Agent-tool framing (docstring update), default-substrate, hook build_agent_patterns import, real background-thread work, preflight package, sentinel-file role fallback, AgentResult surfacing, placeholder diagnostic, k3s commit_sha - reviewer_code v1 (4): except syntax (this commit), preflight HITL answer, AgentResult surfacing, placeholder diagnostic - reviewer_concurrency v1 (5): all five blockers in this commit 122 unit tests pass; substrate end-to-end smoke (preflight → spawn → refine-gate → clean shutdown) green; thread-cleanup smoke verified on both StopIteration and GeneratorExit; ruff lint passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * coder(#2623): v3 → address reviewer_code v2 NACK (sentinel cleanup + install fail-loud) Two new blockers from reviewer_code v2 (4 ACKs already in hand from reviewer_security, reviewer_code_holistic, reviewer_contract, reviewer_concurrency): 1. Active-role sentinel never cleaned up. v2 introduced $HOME/.claude/egg-active-role.json so the PreToolUse hook can resolve the role across process boundaries; nothing unlinked it after a pipeline finished, so the user's next plain Claude Code session would read the stale role and refuse writes outside the stale role's allow-list. Fixed by: - PID stamping: spawner writes os.getpid() into the sentinel. - PID liveness check: hook treats sentinel as missing when os.kill(pid, 0) raises ProcessLookupError / PermissionError. - Explicit teardown: _InProcessOrchestrator.run's finally block calls _teardown_sentinel() — covers normal return, _PreflightAborted, NotImplementedError fence, GeneratorExit. 2. policy.install silently swallowed JSONDecodeError on existing settings.json and overwrote with the egg-substrate template, destroying the user's prior hooks / statusline / plugin enablement. Fixed by raising ValueError with a clear message naming the path + the JSON error location so the operator can fix the typo themselves. Empty / whitespace-only files are still treated as `{}` (the desugared "no existing settings" case). Bonus: k3s_adapter.py now logs via egg_logging.get_logger("...") instead of print(file=sys.stderr) so the structured-warning routes through the daemon's log pipeline (reviewer_code v2 non-blocking). Verified: sentinel is created during the generator's run (smoke test asserts file exists, contains pid), and unlinked on return. policy.install with malformed settings.json raises ValueError naming the path. 122 unit tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(2623): adapt tests to coder v3 + add adversarial Bash parsing probes Adjustments after pulling coder v2/v3 (commits 92d594c and ee36013): * test_pretooluse_hook_policy.py — `install()` now refuses target_dir outside $HOME (v2 security NACK #2 path-escape guard); the existing install tests now point HOME at a tmp_path subdir so they exercise the happy path without polluting the real $HOME. Adds test_install_rejects_target_outside_home pinning the new guard. * test_decide_fail_open_when_role_not_set was split into two tests matching v2 security NACK #4 semantics: outside-substrate writes still fail-open (plain Claude Code session unaffected), but inside-substrate writes (.egg-state/.claude/.github/ shared/egg_restrictions/) now fail closed. Tests pin both arms and monkeypatch HOME to defeat the new $HOME/.claude/egg-active-role.json sentinel role-resolver added by the spawner-hook coordination fix. Adversarial probes for v2 security NACK #1 (Bash write parsing): * test_bash_write_extraction_blocks_out_of_role parametrized over seven write-shaped Bash forms (>, >>, cp, mv, tee, sed -i, dd of=); each asserts a tester-role writing to source code is blocked. * test_bash_ambiguous_command_fails_closed parametrized over three ambiguous shapes (shell vars, backticks, python -c); the hook must fail closed on each. * test_bash_read_only_command_allows_through pins the inverse — a clean ls/cat/grep pipeline passes the hook. * test_hook_entry_script_fails_closed_on_malformed_json drives the hook entry script as a subprocess with non-JSON stdin and asserts the script emits decision=block (v2 security NACK #3 — the gateway-less substrate makes the hook load-bearing, so a parse failure must NOT fall open). Run on coder v3 (commit ee36013): 80 substrate tests pass (8 skip for git/sandbox context); 170 existing orchestrator regression tests still green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * coder(#2623): v4 — fix except-syntax regression at hook_entry.py:497 Reviewer_code v3 NACK (1 blocker): the v3 fix for "active-role sentinel cleanup" added a new ``except ProcessLookupError, PermissionError:`` clause without the parens + ``# fmt: skip`` discipline, re-introducing the SyntaxError on Python 3.10/3.11/3.12/3.13 that v1 blocker #1 was all about. Fixes: - ``hook_entry.py:497``: parenthesise to ``except (ProcessLookupError, PermissionError): # fmt: skip``; expand the inline comment to explain WHY PermissionError is also treated as "stale sentinel" (PID is alive but owned by a different user — the orchestrator's spawner must own the process for role-routing to make sense; fail-safe for the user's plain Claude Code session). Also expand the ``except OSError`` comment to explain why "unknown errno → trust sentinel" is the right default (kernel quirks shouldn't lock the user out of their own session). - Add a top-level docstring section to ``orchestrator/substrate/__init__.py`` pinning the "parens + # fmt: skip" discipline and naming the grep command contributors can use as a manual lint guard: ``grep -nE 'except [A-Za-z.]+ *, *[A-Za-z.]+ *:' orchestrator/ plugins/``. A CI lint rule for this shape is tracked in the follow-up issue. - Verified no other regressions in the same shape across ``orchestrator/substrate/``, ``plugins/egg-sdlc/``, and ``orchestrator/concurrent_executor.py``. Verified PID-liveness check works: stale-PID sentinel resolves to empty (fail-safe); live-PID sentinel resolves to the recorded role. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(2623): address reviewer_code + reviewer_concurrency v1 NACKs Reviewer_code NACK blockers 1-4 (sentinel-lifecycle, install-fail-loud, preflight-abort, refine-failure-gate) and reviewer_concurrency blocker 1 (stale commit_sha assertion) all addressed: New file shared/tests/test_run_pipeline_in_process_sentinel_and_hitl.py (18 tests): * Sentinel PID stamping (v3 fix): test_sentinel_is_written_with_pid asserts os.getpid() lands on the sentinel JSON, plus a teardown- unlinks-file companion and a no-op-when-missing pin. * Hook PID liveness fallback (v3 fix): test_hook_treats_dead_pid_sentinel_as_missing seeds the sentinel with PID=4194300 (above pid_max on most kernels) and asserts the hook falls through to the fail-closed-substrate-prefix default; test_hook_uses_live_pid_sentinel_as_fallback exercises the live-PID branch; test_resolve_active_role_prefers_env_over_sentinel pins the precedence. * Generator cleanup paths: test_generator_unlinks_sentinel_on_generator_close + test_generator_unlinks_sentinel_on_preflight_abort exercise the finally-block teardown across GeneratorExit and _PreflightAborted. * Preflight HITL abort (v2 fix): test_preflight_abort_answer_short_circuits_spawn parametrized over six answer shapes (abort/Abort/STOP/cancel/{selected:abort}/ {value:stop}); each pins that _spawn_refiner never runs. test_preflight_non_abort_answer_proceeds_to_spawn covers the inverse. test_answer_is_abort_helper_contract pins the bare/dict acceptance matrix. * Refine-failure HITL gate (v2 fix): test_refine_gate_says_failed_when_spawner_exit_code_nonzero asserts the question contains FAILED and options == [retry, abort] when exit_code=1; test_refine_gate_says_normal_when_spawner_exit_code_zero pins the 4-way decision shape for exit_code=0. Updates to existing files: * shared/tests/test_pretooluse_hook_policy.py — install fail-loud triplet (malformed JSON / non-dict / empty-OK); rename test_decide_ignores_read_only_tools to test_decide_ignores_pure_read_tools with a docstring noting Bash is audited (not read-only); leave the preceding split fail-open/closed coverage in place. * shared/tests/test_k3s_spawner_adapter.py — flip test_adapter_captures_commit_sha_from_worktree to test_adapter_returns_none_commit_sha_because_legacy_factory_is_fire_and_monitor per reviewer_concurrency NACK #1. The v3 K3sSpawnerAdapter deliberately returns commit_sha=None because the legacy factory is fire-and-monitor; the old assertion would have re-introduced the racy capture via the path of least resistance. * shared/tests/test_run_pipeline_in_process.py — remove the no-op outer patch.object around _InProcessOrchestrator that reviewer_code flagged as misleading; widen the select_substrate patch to cover both yields; populate the spawner mock with an exit_code=0 result so the refine gate sees a clean spawn. Test budget: 101 substrate tests pass (8 skip for sandbox-agent context + git-blocked container). Lint + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(2623): fix Python 3.10-3.13 except-syntax in sentinel-and-HITL tests Reviewer_security v2 NACK (1 blocker): the v2 sentinel-and-HITL test file's two-exception except clauses were authored with parens (`except (StopIteration, in_process_mod._PreflightAborted):`) but `ruff format` under `target-version = py314` strips the parens, and the bare-comma form is a SyntaxError on Python 3.10/3.11/3.12/3.13. Same regression the coder addressed at v4 hook_entry.py:497 — pinned the same way: add `# fmt: skip` so ruff format keeps the parens. Verified the only two affected sites (lines 254, 299) now read: except (StopIteration, in_process_mod._PreflightAborted): # fmt: skip The coder's discipline-doc grep recipe at orchestrator/substrate/__init__.py targets `orchestrator/ plugins/` and misses `shared/tests/`. Suggested widening the recipe in my v1 NACK non-blocking #1 — the follow-up issue should pick that up. All 18 sentinel-and-HITL tests still green; lint + format clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Address reviewer v1 blockers on #2715 Tighten the substrate-swap walking-skeleton spike against the v1 review (egg-reviewer bot, PR #2715): - #1 + #2 (k3s leg silently broken under `EGG_SUBSTRATE=k3s`): gate the `_spawn_agent` seam on `claude-code` only. Unset / `k3s` / any other value keeps the legacy `self.spawn_fn(...)` path so branch-aware spawn and the BRC consensus-wrapped command survive. Update the protocol docstring and ADR to acknowledge that `K3sSpawnerAdapter` returns `commit_sha=None` by design (gateway attestation is authoritative for k3s INV-6); follow-up plumbs it through. - #3 (per-role worktree teardown): add `LocalWorktreeManager.remove(pipeline_id, role)` and call it from both substrate failure paths so one bad spawn no longer wipes peer worktrees mid-spawn under concurrent dispatch. - #4 (bash hook fail-open framing): rewrite the threat-model docstring from "load-bearing enforcement layer" to "first-tier filter with MCP-validator second tier per R2 deferral"; widen the verb walker to catch `rm`, `chmod`, `chown`, `truncate`, `awk -i inplace`, `perl -i`, `wget -O` / `curl -o` / `--output-dir`, `git mv|rm|apply|checkout|restore`, `tar -x`, `unzip`, and shell-of-shell forms (`bash -c`, `sh -c`, …) which recurse into the inner command; tighten the `python3 -m` allow-list to the named hook entry only. - #5 (refiner rubric never loaded): inject `role_rubric_loader=_load_egg_sdlc_role_rubric` in `select_substrate` so `build_system_prompt` actually receives the 119-line rubric from `plugins/egg-sdlc/.../agents/refiner.md` instead of the trivial fallback string. - #6 (heredoc-HITL bridge gap): SKILL.md + ADR now document, in a callout, that the multi-yield generator↔`AskUserQuestion` bridge from a Bash-spawned `python3` subprocess is unsolved in the spike; the in-process machinery is correct within a single-pass invocation. The follow-up issue draft adds an explicit "close the heredoc-HITL bridge gap" bullet with two candidate designs (long-lived REPL/daemon vs. flattened single-yield stages). - #7 (_PreflightAborted translation): wrap the generator body so `_PreflightAborted` translates into a clean StopIteration whose `.value` carries the diagnostic message. Tests now pin `pytest.raises(StopIteration)` rather than the previous "either StopIteration or _PreflightAborted" disjunction. - #8 (plugin metadata `python_dependency` TODO): replace the non-actionable TODO with structured from-source install instructions in `plugin.json` `egg.install_instructions`; preflight.py + SKILL.md read from that single source and emit actionable `git clone … && pip install -r requirements.txt …` guidance. - #9 (tests pinned as fixture not behavior): rename `test_inv3_stale_ack_rejected_when_bus_used_as_transport` → `…_by_tracker_alongside_bus` and similar to honestly reflect that INV-3 / INV-5 live in PeerConsensusTracker, not the bus; drop the unconditionally-skipped k3s parametrize on the bus round-trip smoke test in favor of a claude-code-only test. - #10 (pre-existing SyntaxError in conftest.py): fix both unparenthesised `except A, B:` clauses with `# fmt: skip` so ruff format does not strip the parens again. The conftest file is now importable, so the new substrate fixture is actually live. Plus the easy non-blocking items: use `import threading` instead of `__import__('threading')`, defer `DEFAULT_BASE` evaluation to `LocalWorktreeManager.__init__` so `monkeypatch.setenv('HOME', …)` in tests works, and short-circuit the in-process background ticks when the substrate bundle's bus is a `_K3sPlaceholder`. Tests: 105 pass / 3 skipped (env-required) across the substrate unit suites and `test_substrate_smoke.py`; `make lint`-equivalent `ruff check + ruff format --check` are clean. Authored-by: egg * Address reviewer v2 blockers + non-blocking on #2715 Blocking fixes: - B1 (SKILL.md fabricated --preflight-answer): rewrote the walking-skeleton bridge-gap callout to drop the --preflight-answer CLI flag / env var claim. No such flag, env var, or driver script exists; the previous text described an unimplemented workaround. The callout now states explicitly that there is no end-to-end skill driver in this PR — both the bridge and the single-pass driver are deferred to the follow-up. - B2 (SKILL.md top + ADR contradicted the bridge-gap callout): three sections still described the AskUserQuestion-driven flow as if it worked ("What this gets you" bullet, "What the skill does" steps 3-7, and the heredoc-HITL loop intro). Marked each as the target shape with explicit "deferred" annotations pointing at the bridge-gap callout. The ADR ("in-process orchestrator" + "egg-sdlc plugin" sections) carries the same reconciled framing. - B3 (ADR primitive description stale): updated the "egg-sdlc plugin" section in the ADR. The previous text named a python_dependency field; the v1 fix swapped that for install_instructions. The ADR sentence now reflects the actual field and points at where the from-source command lives. Non-blocking fixes: - N1 (hook_entry.py:711 stale 'load-bearing' inline comment): module-top docstring was rewritten to 'first-tier enforcement only' but the JSONDecodeError fail-closed branch still contained the old framing. Rewrote the comment to match the current threat model. - N2 (tar --xattrs / --xz false-positive in _bash_write_paths): the previous extract-mode detector matched any token starting with -x (excluding --exclude*), so tar --xattrs and tar --xz were falsely classified as extract operations. Narrowed the match to the actual extract forms: --extract long flag, or a single-dash cluster containing 'x'. - N3 (bash -lc combined short flags not recursed): the shell-of-shell handler only matched -c as a standalone token, so bash -lc 'cmd' / sh -ic 'cmd' / etc. were not parsed. Now also detect single-dash short-flag clusters containing 'c'. - N4 (stale DEFAULT_BASE alias in worktree.py): the module-level alias was kept for back-compat but immediately froze $HOME at import time, diverging from what LocalWorktreeManager itself saw under monkeypatch.setenv. No callers remained; dropped the alias and replaced it with a docstring on _default_base() explaining why the alias is intentionally absent. - N5 (rubric loader hard-codes from-source path layout): added a TODO in _load_egg_sdlc_role_rubric naming the cq-12 follow-up — once egg publishes a pip-installable package, the parent.parent.parent / plugins / ... walk breaks (site-packages does not co-locate the plugins directory) and the loader should switch to importlib.resources-style packaging-aware resolution. Tests: 105 passed / 3 skipped across the 8 substrate unit-test suites and the integration substrate smoke (matching the v2 baseline). make lint clean. Issue: #2623 Authored-by: egg * Address reviewer v3 blockers + non-blocking on #2715 B4: SKILL.md frontmatter description previously asserted in active voice that the skill 'boots the real egg_orchestrator in-process ... renders HITL decisions through AskUserQuestion' — contradicting the v2 bridge-gap callout. Reframed as target shape with explicit deferred-driver / deferred-bridge qualifier so the slash-command picker text matches the body. B5: ADR cq-decisions table at lines 41 (cq-8) + 45 (cq-12) still described 'pip dep selected by cq-12' and 'cq-12 resolved in plan re-propose cycle' even though the v2 B3 fix at line 122 already swapped plugin.json to 'install_instructions' with cq-12 deferred to the follow-up. Rewrote both rows so the canonical scan at the top of the ADR matches the egg-sdlc-plugin section. NB1: ADR cq-7 row (line 40) leading active-voice clause now carries a 'Target shape:' prefix so the qualifier-after-claim ordering aligns with the SKILL.md body's reconciled framing. NB2: shared/tests/test_pretooluse_hook_policy.py gains regression coverage for the v2 _is_tar_extract helper (tar --extract / -xzf / --xattrs / --xz / --exclude= shapes) and bash short-flag cluster recursion (-lc / -xc / -ic) that the v2 commit added without tests. NB3: hook_entry._bash_write_paths filters _REDIRECT_RE matches whose captured target contains an unmatched quote, so bash -c 'echo x > /restricted/file' no longer surfaces the phantom /restricted/file' duplicate alongside the clean path the recursive bash handler extracts. Authored-by: egg --------- Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: egg <egg@example.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>

Producer artifact for the plan-phase risk_analyst role. Catalogs 17 risks with mitigations and rollbacks; surfaces top-3 program risks (prior-pipeline history, _run_pipeline carve-out, allowlist serialization); names 5 blocking concerns the architect / task_planner plans must address (per-phase _run_pipeline strategy, allowlist serialization protocol, plan_parser.py-in-scope per cq-3, lazy-import audit per phase, gateway @app.route stay in __init__.py per non-negotiable #8). Verdict/feedback fields are null pending architect + task_planner proposals; this artifact will be re-proposed (--changed-artifacts) with the verdict once those upstream producers land.

Decomposes the architect's 28-slice scaffold (.egg-state/agent-outputs/ 2817-architect-slices.yaml) into discrete tasks per slice with TASK-<id>-<n> IDs, acceptance criteria, role assignments, and file lists. Highlights: - Adopt the architect's slice composition verbatim (#2809) — slice count, boundaries, dependencies, and serialization preserved. - Slice-1 is docs/scaffolding root (6 tasks: cq-2/4/5/6 + seam-table TBDs + allowlist comment retags). - Slices 2-7 are the pipelines.py chain (linear, slice-2 baseline + flat clusters; slices 3-6 each one sub-sub-package; slice-7 terminal with 10 tasks covering the per-phase _run_pipeline carve-out per RISK-2 / blocker-1). - Slices 8-12 are the gateway.py chain (linear, non-negotiable #8 preserved — 47 @app.route decorators stay in __init__.py per blocker-5). - Slices 13-28 are independent per-file decompositions all rooted at slice-1. Addresses all five risk_analyst blocking concerns (per-phase _run_pipeline strategy, allowlist serialization, plan_parser.py in-scope per cq-3, lazy-import audit per blocker-4, gateway @app.route preservation). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

- contract_completeness.py: switch from stdlib logging to egg_logging.get_logger for structured-log enrichment consistency (egg-reviewer #8). - signals.py: defensive non-dict attestation handling and isinstance guard on tasks_verified list (egg-reviewer #1); structured details on no-op propose rejection (egg-reviewer #3); explicit warn-and-skip on empty producer_role rather than silently degrading the attestation check (egg-reviewer #5); comment documenting the CONFIRM-gate carve-out in the message-bus fallback path (egg-reviewer #4). - sandbox brc handler + schema: drop the dead NACK attestation thread (the gate only reads it on ACK); unwrap the new 400 contract_incomplete on no-op propose so brc_propose surfaces structured rejection data instead of raising (egg-reviewer #2, #3). - validator.py: expand docstring to call out that the demote-only check is broader than just the enforcer — every reviewer role has task-status write capability, so restricting to the enforcer alone would leave side channels (egg-reviewer #6). - test_contract_completeness_gate.py: cover the kill switch for confirm and noop_propose, end-to-end slice_id=None integration, and non-dict attestation robustness; tighten producer_role empty-string assertion (egg-reviewer #7). - test_brc_attestation.py: update NACK test for dropped attestation thread; add propose-400 contract_incomplete unwrap test.

…on incomplete task rows, attestation channel revived (#3119) * Fix #3114: contract-completeness gate on enforcer ACK/CONFIRM + attestation threading - orchestrator/contract_completeness.py: pure completeness checks, kill switch - routes/signals.py: reject enforcer ACK (contract_incomplete / attestation_required / attestation_mismatch), enforcer CONFIRM, and no-op propose with open owned rows - review_graph.py: reviewer_contract CRITICAL edges to tester + documenter - sandbox brc tools/handlers: attestation param threaded; structured 409 surfacing - validator.py: reviewer task-status writes demote-only - preamble/criteria/docs updates * test: align suites with #3114 gate semantics - test_peer_consensus_integration: enforcer ACKs for tester/documenter (new critical edges) - test_validator: reviewer demote-only (promote-to-complete now rejected) - preamble: tighten #3114 producer guidance; soften collapse drop ratio 0.18→0.13 on the #3027 precedent (load-bearing gate guidance re-raised producer preamble size) * Fix mypy: add return type annotation to test method * Address PR review feedback (#3119) - contract_completeness.py: switch from stdlib logging to egg_logging.get_logger for structured-log enrichment consistency (egg-reviewer #8). - signals.py: defensive non-dict attestation handling and isinstance guard on tasks_verified list (egg-reviewer #1); structured details on no-op propose rejection (egg-reviewer #3); explicit warn-and-skip on empty producer_role rather than silently degrading the attestation check (egg-reviewer #5); comment documenting the CONFIRM-gate carve-out in the message-bus fallback path (egg-reviewer #4). - sandbox brc handler + schema: drop the dead NACK attestation thread (the gate only reads it on ACK); unwrap the new 400 contract_incomplete on no-op propose so brc_propose surfaces structured rejection data instead of raising (egg-reviewer #2, #3). - validator.py: expand docstring to call out that the demote-only check is broader than just the enforcer — every reviewer role has task-status write capability, so restricting to the enforcer alone would leave side channels (egg-reviewer #6). - test_contract_completeness_gate.py: cover the kill switch for confirm and noop_propose, end-to-end slice_id=None integration, and non-dict attestation robustness; tighten producer_role empty-string assertion (egg-reviewer #7). - test_brc_attestation.py: update NACK test for dropped attestation thread; add propose-400 contract_incomplete unwrap test. * Fix flaky capsys assertion in contract-completeness gate test The empty-producer_role test asserted against capsys.readouterr().err, but EggLogger lazily attaches StreamHandler(sys.stderr) on first log call and the handler captures a stale reference to the original sys.stderr. capsys replaces sys.stderr at the Python level, which the handler bypasses — capfd captures at the fd level and sees the write. * Fix test: use caplog handler attached directly to non-propagating logger The empty-producer_role test could not reliably observe the warning emitted by EggLogger ("orchestrator.signals"): propagation is disabled, so caplog's root attachment misses it; the StreamHandler holds a stream reference captured at lazy-init time, which defeats both capsys and capfd. Attach caplog.handler directly to the underlying Python logger so the record is captured regardless of propagation or stream-reference timing. * Fix ruff formatting in test_contract_completeness_gate.py * Address PR #3119 review: drop dead logger fallback; tighten NACK drop test - contract_completeness.py: remove the stdlib logging fallback for egg_logging.get_logger. The fallback returned a stdlib Logger that would have raised TypeError on the structured kwargs every call site passes (identifier=..., error=...) — the path was marked no-cover and egg_logging is in-tree, so a runtime trip would have inverted the gate's fail-open posture into a 500. Drop it; egg_logging is always present. - test_brc_attestation.py: rename test_nack_payload_excludes_attestation to test_nack_drops_supplied_attestation and pass attestation=_ATTESTATION in the request so the assertion actually exercises the drop. The prior version asserted absence in a request that never carried the field — would have passed even if brc_nack started forwarding it. --------- Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com>

jwbron and others added 4 commits February 2, 2026 21:30

Merge origin/main into jib/phase-1-setup

5c21af8

jwbron merged commit 990035d into main Feb 2, 2026
3 of 4 checks passed

This was referenced Feb 7, 2026

Implement Phase 3: Agent CLI and prompt integration #245

Merged

Implement Phase 4: Pipeline workflow for SDLC checkpoints #252

Merged

[SDLC] rework egg deploy strategy #374

Merged

james-in-a-box Bot mentioned this pull request Feb 9, 2026

[SDLC] Fix merge conflicts in contract push-with-retry logic #394

Merged

james-in-a-box Bot mentioned this pull request Feb 12, 2026

Remove EGG_REPOS_DIR, mount only configured repos #548

Merged

6 tasks

egg-reviewer Bot mentioned this pull request Feb 13, 2026

Unify orchestrator and CLI container launch via shared config builder #594

Merged

6 tasks

james-in-a-box Bot mentioned this pull request Feb 13, 2026

Add token-gated SDLC pipeline approvals #616

Merged

egg-reviewer Bot mentioned this pull request Feb 14, 2026

Add deployment validation check for DinD devserver integration #653

Merged

This was referenced Feb 15, 2026

Add short-circuit mode to skip plan phase for simple tasks #734

Merged

Enforce phase file restrictions via readonly mounts and commit-time validation #800

Merged

This was referenced Feb 17, 2026

Add Tier 3 parallel phase-level dispatch #812

Merged

Add runtime container monitor and wave cycle cap #848

Merged

james-in-a-box Bot mentioned this pull request Feb 22, 2026

Update agent-mode design guide for LLM boundary rules #877

Merged

This was referenced Feb 25, 2026

Enforce per-task file restrictions in implement phase #911

Closed

Auto-create PR in orchestrator, skip agent spawn #944

Merged

jwbron mentioned this pull request May 8, 2026

Decompose 15 oversize Python source files to clear the file-size allowlist #2261

Closed

egg-reviewer Bot mentioned this pull request May 11, 2026

Fix #2581: auto-ACK pure producers when slice has no tasks for their role #2583

Merged

3 tasks

egg-reviewer Bot mentioned this pull request May 12, 2026

Fix #2633: integration_tests/regression/ for pipeline recovery + salvage #2660

Merged

3 tasks

james-in-a-box Bot mentioned this pull request May 19, 2026

Spike: walking-skeleton Claude Code substrate for egg SDLC (#2623) #2715

Merged

jwbron mentioned this pull request May 21, 2026

Pipeline pipeline-f4c7d780: only 7 of 15 slices reached the SDLC contract #9 #2743

Closed

egg-reviewer Bot mentioned this pull request May 22, 2026

[issue-2769][slice-1/2] Gateway upstream router + LiteLLM Deploymen... #2773

Merged

egg-reviewer Bot mentioned this pull request May 30, 2026

[issue-2777][slice-1/4] Context-PR opener + wiring (slice-1a) #2888

Merged

james-in-a-box Bot mentioned this pull request May 30, 2026

[issue-2777][slice-2/4] Scaffold + PR-phase deletions (slice-1b) #2891

Merged

james-in-a-box Bot mentioned this pull request May 31, 2026

[issue-2817][slice-1/28] Docs and seam-table scaffolding #2902

Closed

This was referenced Jun 1, 2026

gateway: control-plane find_open_pr route; stop modeling orchestrator as an AgentRole (#2893) #2922

Merged

feat(sandbox): expose Confluence/Jira gateway routes as MCP servers (#2994) #2996

Merged

egg-reviewer Bot mentioned this pull request Jun 11, 2026

Fix #3114: contract-completeness gate — enforcer ACK/CONFIRM blocked on incomplete task rows, attestation channel revived #3119

Merged

james-in-a-box Bot mentioned this pull request Jun 15, 2026

slice-1: Restart-memory preservation #3213

Closed

This was referenced Jun 26, 2026

Decompose 19 oversize Python files; clear the file-size allowlist (#3312) #3336

Merged

[issue-3312][slice-2/19] Decompose... #3340

Merged

Decompose the last 4 oversize files; empty the allowlist (#3312) #3489

Merged

james-in-a-box Bot mentioned this pull request Jul 4, 2026

[issue-3312][slice-3/4] Decompose gateway/gateway.py (10,648... #3503

Merged

egg-reviewer Bot mentioned this pull request Jul 27, 2026

Match the snapshot-softening rule to the noise source, not a file count #3654

Merged

Navigation Menu

Merge Phase 1.5 docs into main - #8

Merge Phase 1.5 docs into main#8
jwbron merged 4 commits into
mainfrom
jib/phase-1-setup

jwbron commented Feb 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jwbron commented Feb 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant