docs: document gateway session idle timeout config [doc-updater] - #1898
Merged
Conversation
![egg-reviewer[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Documentation is accurate — the env var names, defaults, and behavior description all match the implementation in gateway/gateway.py:5714-5715 and gateway/session_manager.py.
One non-blocking issue:
Missing minimum for EGG_SESSION_IDLE_TIMEOUT_MINUTES — The YAML comment documents minimum: 1 for EGG_SESSION_CLEANUP_INTERVAL_MINUTES, but the idle timeout has an undocumented floor of 5 minutes (max(5, ...) at gateway/gateway.py:5715). An operator setting this to, say, 2 would silently get 5. Add # minimum: 5 to the idle timeout comment for consistency:
- name: EGG_SESSION_IDLE_TIMEOUT_MINUTES
value: "120" # default: 60, minimum: 5— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
Agreed. Added — Authored by egg |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
egg review completed. View run logs 3 previous review(s) hidden. |
jwbron
approved these changes
Apr 22, 2026
james-in-a-box Bot
added a commit
that referenced
this pull request
Apr 23, 2026
* docs: document gateway session idle timeout config * docs: add minimum value comment for EGG_SESSION_IDLE_TIMEOUT_MINUTES --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
james-in-a-box Bot
added a commit
that referenced
this pull request
Apr 23, 2026
* docs: document gateway session idle timeout config * docs: add minimum value comment for EGG_SESSION_IDLE_TIMEOUT_MINUTES --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
jwbron
added a commit
that referenced
this pull request
Apr 23, 2026
* Initialize SDLC contract for issue #1897
* refine(#1897): draft analysis for agent wait heuristics
Analyze the sleep/poll-loop patterns observed in pipeline issue-1762-membump
and surface seven HITL decisions plus an open-feedback comment covering
scope, blocking-primitive shape, --wait cap, in-memory store behavior, the
QUESTION message type, anti-pattern enforcement, and the consensus_wrapper
stay-alive loop.
Recommends Option B: tighten BRC preamble + add typed blocking primitive
(egg-orch message wait --for TYPE) + new docs/reference/agent-wait-patterns.md.
Item #3 (idempotent consensus confirmed) is already covered by PR #1896.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refine(#1897): address reviewer_refine NACK — restore decision-6, trace call stack, concrete cost data
Blocker fixes:
- Add decision-6 HTML comment block annotated as SUPERSEDED (contract
has no deletion mechanism; the first add-decision call had its option
text mangled by shell command substitution on inline backticks).
Decision-7 is reworded to be meaningfully different from decision-6
(focuses explicitly on the scope of enforcement).
- Trace the full CLI → HTTP → signal handler → _existing_confirmed_for_role
call stack for `egg-orch consensus confirmed` so item #3 idempotency
coverage is verified, not asserted.
- Add verbatim timestamps from the issue body (21:10:19, 22:16:08, etc)
so the oversight transcript can be replayed.
Non-blocking improvements:
- Document the inter-decision coupling between decision-1 (blocking
primitive) and decision-4 (in-memory store).
- Cite concrete server-load numbers (3-7 agents × O(10) pipelines ≈
30-70 sockets; HTTP_PROXY idle timeout is the binding cap).
- Add container-lifecycle constraint for decision-8 (SIGTERM handling
during graceful shutdown with long XREAD BLOCK).
- Replace line-number references to test_pipeline_prompts.py with
test function names to avoid drift.
- Explicitly name the fallback trigger from Option B to Option C/D
based on decision-1 outcome.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after refine phase
* Persist HITL resolution after refine phase gate
* plan(#1897): architect output — Option C 6-track decomposition
Decomposes issue #1897's HITL-approved Option C (full scope) into 6
independently mergeable tracks:
1. Prompt audit + single-idiom docs (agent-wait-patterns.md)
2. Configurable --wait cap via EGG_MESSAGE_POLL_MAX_WAIT env var
3. New typed blocking primitive: egg-orch message wait --for TYPE
4. In-memory store true blocking via threading.Condition
5. HEARTBEAT message type with structured state + remove QUESTION
6. consensus_wrapper shell sleep loop → event-driven message wait
Captures 8 findings (F1-F8), 7 technical decisions (TD-1..TD-7),
7 risks (R1-R7), test strategy across 6 new test classes and 5
updated fixtures, and explicit hand-off questions for task_planner
and risk_analyst. Merge order: track-2, 4, 5, 3, 1, 6.
Applies HITL resolutions: decisions 1-8 (full scope, new CLI,
env-configurable cap, in-memory blocking, remove QUESTION, Don'ts
in preamble, wrapper event-driven replacement) and Q1-Q5 answers
from the refine phase gate.
Refs: #1897
* risk_analyst: assess technical risks for #1897 agent wait heuristics (plan)
Produce risk assessment for full-scope Option C: typed `message wait`
CLI + env-configurable wait cap + in-memory condition-variable blocking
+ remove QUESTION + add HEARTBEAT type + explicit prompt Don'ts +
consensus_wrapper SSE/XREAD replacement + docs.
Overall risk MEDIUM-to-HIGH. Five HIGH-severity risks:
- RISK-1: QUESTION is actively advertised in reviewer prompt preamble
(pipelines.py:6062-6074) and BRC_HISTORY_TYPES, NOT 'test fixtures
only' as refine analysis claimed. Removing it regresses reviewer UX
unless staged carefully.
- RISK-2: new HEARTBEAT type collides with existing PROGRESS-heartbeat
timer in health_monitor.py - Tier 1 alarms fire on agents reporting
state correctly via new channel unless explicitly wired.
- RISK-3: WSGI worker starvation - 30-70 concurrent long-polling
sockets will saturate default Gunicorn sync worker pool.
- RISK-4: Gateway Squid idle timeout coupling has no code-level gate;
raising EGG_MESSAGE_POLL_MAX_WAIT without Squid bump produces 504s.
- RISK-5: in-memory MessageStore condition variable + clear() at phase
transitions can block threads forever without notify_all semantics.
All risks have named mitigations, affected components cited by file:line,
rollback plans, and 10 concrete regression tests. Five questions flagged
for task_planner: QUESTION scope, SSE-vs-XREAD choice, exit-code
contract, WSGI config task, startup-log warning. External research
skipped - internal protocol refactor with no third-party deps.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: implementation plan for #1897 (event-driven BRC wait + heartbeats)
Single-PR plan organising full Option C scope into eight phases:
1. Backend message-store primitives (typed XREAD BLOCK + cond-var blocking
on the in-memory store + EGG_MESSAGE_POLL_MAX_WAIT env cap).
2. New GET /messages/wait HTTP route + egg-orch message wait CLI.
3. HEARTBEAT message type with structured state field and Tier-1
liveness consumer.
4. consensus_wrapper rewrite from sleep-loop to event-driven wait.
5. Producer/reviewer prompt audit: canonical idiom + explicit Don'ts.
6. QUESTION message-type removal (zero production callers today).
7. Concurrent-integration tests + #1896 dedup regression test.
8. Documentation: agent-wait-patterns.md + concurrent-execution.md.
* plan: align #1897 plan with existing CONSENSUS_CONFIRMED enum
Use the existing MessageType.CONSENSUS_CONFIRMED (final flavour, body
field) for the consensus_wrapper wait and the producer/reviewer STAY
ALIVE preamble — there is no separate CONSENSUS_REACHED enum member
today. Per architect TD-5 and the existing dedup logic at
routes/signals.py:1456 (final flavour) the wait should target the
existing type, not invent a new one. Also clarifies acceptance for
TASK-4-1 to assert the wrapper does not unblock on the pending_acks
flavour.
* plan(#1897): incorporate risk_analyst HIGH/MEDIUM mitigations
Major plan revision addressing the 7 HIGH-severity risks the
risk_analyst flagged in their proposal:
- RISK-1, RISK-10 (QUESTION removal blast radius): expand Phase 7
to enumerate every QUESTION reference (prompt at
pipelines.py:6062-6074, BRC_HISTORY_TYPES at :4775,
test_brc_history.py at 816/871/893/974-985/1261-1281,
test_concurrent_integration.py:165-174, plus 3 other test
files) and stage commits in dependency order so the test suite
is green at every commit boundary.
- RISK-2 (HEARTBEAT vs PROGRESS-heartbeat collision): add
TASK-3-3 wiring HealthMonitor MESSAGE_SENT subscription to
reset last_heartbeat on HEARTBEAT messages, with explicit
test coverage that HEARTBEAT-only emission does not trip the
heartbeat_timeout alarm. Legacy PROGRESS-heartbeat path
retained behind a TODO.
- RISK-3 (WSGI worker starvation): add Phase 4 (WSGI worker
pool sizing): switch Gunicorn to gevent workers, raise
--timeout to 2 × cap, dedicated /healthz endpoint for
readiness probe, egg_inflight_long_polls metric.
- RISK-4 (gateway Squid timeout coupling): TASK-2-3 emits a
startup WARNING log when EGG_MESSAGE_POLL_MAX_WAIT > 90;
TASK-8-3 adds a deliberately-misconfigured-cap test that
asserts the resulting 504 is named.
- RISK-5 (cv blocking + clear() race): TASK-1-1 specifies
per-pipeline threading.Condition with notify_all() in
add_message AND clear(), wake-up re-checks pipeline_id;
TASK-1-3 removes the silent non-blocking fallback at
routes/messages.py:181-184.
- RISK-6, RISK-7 (consensus_wrapper SIGTERM): switch from
XREAD BLOCK to SSE via curl --no-buffer against the
existing orchestrator/sse.py endpoint. Curl honours SIGTERM
via socket close. Falls back to current shell sleep loop
if SSE unavailable (zero-Redis local-dev path).
- RISK-9 (exit-code contract): TASK-2-2 codifies 0/1/2/3
semantics; TASK-2-4 adds egg-orch message wait-loop
convenience command that encapsulates the case-statement
so the prompt can ship a one-liner.
Also answers the 5 open questions risk_analyst flagged for
task_planner: keep QUESTION removal in scope (Q1), use SSE
(Q2), exit-code contract specified (Q3), gevent async workers
(Q4), startup warning at >90s threshold (Q5).
Phase count is now 9 (was 8) — added Phase 4 (WSGI sizing) as
a separate logical commit.
* plan(#1897): address reviewer_plan NACK — schema, SSE event, idiom regex
Fixes for the 7 blocking + 7 non-blocking items reviewer_plan raised
against my prior proposals:
Blocking fixes:
- BLOCKING-3 / BLOCKING-7 (CONSENSUS_REACHED vs CONSENSUS_CONFIRMED):
TASK-5-1 now correctly states the SSE event-name is
EventType.CONSENSUS_REACHED.value = 'consensus.reached' (an
EventType, not a MessageType). The bus message type remains
CONSENSUS_CONFIRMED. The SSE event-name distinguishes final from
pending_acks naturally — no metadata filter needed for the wrapper.
All other "CONSENSUS_REACHED" references in the plan refer to the
bus MessageType.CONSENSUS_CONFIRMED used by agents, not by the
wrapper.
- BLOCKING-4 (consensus_wrapper is shell): already addressed in
prior revision — TASK-5-1 uses curl SSE from shell, not Python.
- BLOCKING-5 (idiom × exit-code mismatch): already addressed via
TASK-2-4's `egg-orch message wait-loop` convenience CLI. Prompt
in TASK-6-1 calls wait-loop, not raw wait.
- BLOCKING-6 (cv signal on clear): already addressed in TASK-1-1
(per-pipeline cv, notify_all in both add_message AND clear,
blocking loop re-checks pipeline_id after wake).
Non-blocking fixes in this commit:
- TASK-3-1: HEARTBEAT state goes in `metadata` (dict), NOT `body`
(str), matching the existing convention used by
routes/signals.py:1448 for pending_acks. Server-side schema
validation moves from body to metadata.
- TASK-6-1 acceptance: regex assertion that `for i in [0-9]` and
`sleep [0-9]+` only appear inside the documented anti-pattern
paragraph (catches a regression that smuggles `sleep 30 &&`
back into the preamble).
- TASK-6-2: widen grep to ALL rules / agent-prompt directories
(sandbox/agent-config/rules/ AND shared/agent-prompts/ if
present), with explicit `for i in [0-9]+` / `sleep [0-9]+`
patterns.
- TASK-8-1 / TASK-8-2: deliberate-regression sanity checks moved
out of the test acceptance text and into manual_steps as a
pre-merge action the author performs locally and documents in
the PR description.
- manual_steps: added explicit phase-ordering requirement (1→9 in
order; Phase 4 before Phase 6; Phase 6 before Phase 7) and
added the deliberate-regression sanity checks as pre-merge
manual steps.
- Strategy block: added explicit "phases land 1→9 in numerical
order" sentence with the same Phase-4→6→7 dependency note.
* plan(#1897): revise architect output addressing reviewer_plan NACK
Blocker fixes (all 3):
1. TD-5/TD-7 replaced fabricated CONSENSUS_CONFIRMED_FINAL with new
additive MessageType.CONSENSUS_REACHED (TD-8). signals.py:1469
emits the new type alongside the existing CONSENSUS_CONFIRMED for
final flavour only — existing consumers unaffected, wait primitive
filters cleanly. Tracks 5 and 6 updated accordingly.
2. F7 and Track 5 now enumerate all 11 QUESTION call sites (not just
the ~6 senders): BRC_HISTORY_TYPES at pipelines.py:4775, reviewer
preamble at pipelines.py:6062-6074, 4 substantive test functions
in test_brc_history.py, 3 assertions in test_concurrent_integration.py,
test_mcp_tools.py + test_pipeline_prompts.py + test_brc_cli_args.py
+ test_checkpoint_inter_agent.py + concurrent-execution.md. Reviewer
UX implication documented (clarifications via NACK).
3. TD-3 / F5 / Track 5 now specify HealthMonitor._on_message_sent
wiring at health_monitor.py:330-360 to reset last_heartbeat on
HEARTBEAT messages. Without this, agents that migrate to
HEARTBEAT trip false heartbeat_timeout alerts (RISK-2).
Non-blocking fixes:
- New Track 7: WSGI worker model + operator guide (RISK-3 mitigation)
- TD-3: HEARTBEAT rate limit is HARD (429 at send_message), not just
overseer log
- Track 6 dependencies now include track-5 (new MessageType)
- TD-9: message wait --since for transport-retry resumability
- TD-7 example uses CONSENSUS_REACHED (matches TD-8)
- Findings F1-F8 cross-reference risk_analyst's RISK-1..RISK-12;
Q-risk-1/2/3 removed (already answered)
Merge order updated: track-2, 4, 7, 5, 3, 1, 6.
Refs: #1897
* plan(#1897): revision 3 — reconcile architect output with CONFIRMED plan
Addresses reviewer_plan NACK blocking-1-rev2: my revision 2 chose a
message-wait-plus-new-MessageType design for consensus_wrapper while
the task_planner's already-CONFIRMED plan (Phase 5 / TASK-5-1) uses
SSE via curl --no-buffer on the EXISTING EventType.CONSENSUS_REACHED.
Adopted reviewer's recommended option (b): defer to the plan.
Key changes:
- TD-5 rewritten to endorse SSE via curl --no-buffer against
orchestrator/sse.py's EventType.CONSENSUS_REACHED (events.py:67).
- TD-8 (new MessageType.CONSENSUS_REACHED) DELETED — the existing
SSE event-name 'consensus.reached' already distinguishes final
from pending without any MessageType schema change, and SSE
falls back to shell sleep loop on Redis-down (RISK-6 load-
bearing for local dev).
- Track 6 rewritten to mirror plan Phase 5 / TASK-5-1 verbatim
(curl --no-buffer, trap for SIGTERM, 503 fallback).
- Track 5 no longer adds a CONSENSUS_REACHED MessageType — only
HEARTBEAT (new) and QUESTION (removed).
- TD-7 preamble example updated to plan TASK-6-1 canonical idiom:
'egg-orch message wait-loop --for CONSENSUS_CONFIRMED --for
CONSENSUS_RE_REVIEW --for OVERSEER_ALERT' (wait-loop is the
plan TASK-2-4 convenience CLI that encapsulates retry + exit-
code handling).
- All 7 architect tracks explicitly mapped to plan phases (Track 1
→ 6+8, Track 2 → 2, Track 3 → 1+2, Track 4 → 1, Track 5 → 3+7,
Track 6 → 5, Track 7 → 4). Plan is the implementation source of
truth; architect tracks document rationale.
- A8/A9 alternatives record the revision-2 designs and why they
were superseded.
- No open questions for task_planner remain (previously Q-planner-1
is resolved since the plan shipped the SSE answer).
Refs: #1897
* risk_analyst(#1897): revision 2 — reconcile with architect rev 3 + CONFIRMED plan
Updates summary and adds revision_notes documenting that each HIGH/MEDIUM risk
is addressed by a locked-in mitigation in the CONFIRMED task_planner plan:
- RISK-6/RISK-7 → SSE via curl --no-buffer (architect TD-5, plan Phase 5)
- RISK-2 → MESSAGE_SENT subscription in HealthMonitor (plan TASK-3-3)
- RISK-3 → gevent workers + /healthz + in-flight gauge (plan Phase 4)
- RISK-9 → 0/1/2/3 exit-code contract + wait-loop CLI (plan TASK-2-2/2-4)
- RISK-5 → per-pipeline Condition + clear() notify_all (plan TASK-1-1)
No new risks surfaced. Post-mitigation residual risk is MEDIUM (prompt
regression + deploy-config drift). External research skipped: internal
refactor, no new third-party supply-chain deps.
* docs: document server-side contract decision bridge (#1899)
Update docs to reflect the new _queue_and_await_contract_decisions()
function added in ae9535b99. Contract HITL decisions registered via
egg-contract add-decision/add-feedback are now bridged into the
orchestrator decision queue after phase gate approval, ensuring they
are surfaced to humans in all modes (not just prompt-driven CLI).
Authored-by: egg
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* docs: document gateway session idle timeout config [doc-updater] (#1898)
* docs: document gateway session idle timeout config
* docs: add minimum value comment for EGG_SESSION_IDLE_TIMEOUT_MINUTES
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1895: right-size gateway, orchestrator, and sandbox pod resources (#1904)
* Fix #1895: right-size gateway, orchestrator, and sandbox pod resources
Tuned requests/limits against a 5-minute trace captured 2026-04-22 against
3 concurrent pipelines (14 sandbox agents), post-#1887.
- Gateway (k8s/base/gateway-deployment.yaml): CPU limit 1 -> 2 cores
(observed spikes to 878m / 88% of 1-core cap during proxy bursts);
mem request 2Gi -> 1Gi, mem limit 4Gi -> 2Gi (post-#1887 steady state
1.37-1.56Gi; #1886's 4Gi band-aid is no longer needed).
- Orchestrator (k8s/base/orchestrator-deployment.yaml): mem request
256Mi -> 512Mi, mem limit 512Mi -> 1Gi (pod sits consistently at
283-301Mi, actively burstable-using above old request).
- Sandbox default (orchestrator/kubernetes_client.py): req 500m/512Mi
-> 250m/384Mi, lim 2c/2Gi -> 1c/1Gi. Observed per-agent max
468m CPU / 407Mi mem leaves 2x+ headroom under new limits. Frees
~3.5 cores and ~1.8Gi of reservation at 14-agent fleet size.
- All three remain Burstable QoS (idle:spike ratio too wide for
Guaranteed on a single-node cluster).
- New docs/deploy/resource-sizing.md records observed-usage table,
QoS rationale, and re-capture recipe.
Related: #1888 (parent right-sizing), #1886 (4Gi interim bump reverted
here), #1887 (SSE refactor that enabled the memory reduction).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Relax gateway and sandbox limits after observing test-phase load
Follow-up to the initial right-sizing on this branch. A second snapshot
~25 minutes into the same pipeline session showed gateway memory climbing
to 2.2Gi (past the proposed 2Gi cap) even as the sandbox fleet shrank
from 14 to 10 agents. The driver is the implement-phase tester running
`make test` — test output routes through the gateway SSE stream and
grows the working set. This is load-driven, not a leak.
Revised net change vs main:
- Gateway: CPU limit 1 -> 2 only. Memory request 2Gi and limit 4Gi
stay put (the #1886 interim bump turns out to be the right steady
state for a single-node cluster hosting test-running sandboxes, not
a band-aid to revert). TODO(#1885) comment dropped since we've now
concluded the review.
- Orchestrator: unchanged from previous commit (mem req 256Mi -> 512Mi,
mem limit 512Mi -> 1Gi).
- Sandbox default: CPU request 500m -> 250m only. CPU limit (2c),
mem request (512Mi), and mem limit (2Gi) all revert to main's values.
The tester at 566Mi memory and occasional 468m CPU spikes sit
comfortably under the 2c/2Gi limits; shrinking them risks OOM/throttling
under `make test`. The 500m -> 250m CPU request drop still frees
3.5 cores of node reservation at current fleet size.
Updated docs/deploy/resource-sizing.md with both snapshots and the
reasoning for keeping gateway memory at 4Gi.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix GATEWAY_MEM_TRACE env var name in resource-sizing doc
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1905: /sdlc auto-resolves phase_gate follow-ups from context (#1908)
* Initialize SDLC contract for issue #1905
* Fix #1905: /sdlc auto-resolves phase_gate follow-ups from context
Adds a session-scoped `resolved_questions_map` to the /sdlc skill's
Phase 4 (HITL) so draft-embedded answers collected during a `phase_gate`
are reused when the orchestrator subsequently registers the same
questions as standalone `choice`/`feedback` decisions — instead of
re-prompting the user for each one.
Changes (skill-only; orchestrator protocol untouched):
- New `### Resolved Questions Map` subsection defining the map and
lowercase+strip normalization rule.
- Step 5 of the phase_gate handler now populates the map alongside
the existing Resolved Questions display block.
- `choice` handler: Before prompting, looks up the normalized question,
matches stored answers against `decision.options`, and on a compatible
match auto-submits `{"action":"select","selected":...}` with a
user-visible one-line note. Falls through to the prompt on no match
or incompatible option.
- `feedback` handler: Before prompting, prefills answers for matched
questions, prompts only for the unmatched, merges into a single
`{"action":"submit_feedback","answers":{...}}` submission, and prints
a one-line auto-resolution note.
Closes tasks 1-1, 1-2, 1-3 from contract issue-1905.
* docs: document /sdlc skill's resolved_questions_map auto-resolution
Adds documentation for the session-scoped `resolved_questions_map` added
to the `/sdlc` Claude Code skill in #1905 (Phase 4 HITL handler) so
draft-embedded answers collected during a `phase_gate` are reused when
the orchestrator subsequently registers the same questions as standalone
`choice`/`feedback` decisions — avoiding the user being prompted twice.
- `docs/hitl-decisions.md`: new "/sdlc Skill: Auto-Resolving Repeated
Questions" section covering the map definition, the choice and
feedback auto-resolution flows (including the user-visible one-line
note format and the option-compatibility fall-through), the
transparency requirement, and the scope / non-goals (skill-only,
exact-match only, session-scoped, unaffected in egg-sdlc terminal
mode). Also adds `skills/sdlc/SKILL.md` to the Related Files list.
- `docs/guides/sdlc-pipeline.md`: cross-reference paragraph in the HITL
section linking to the new documentation, so readers who land on the
SDLC guide learn that the skill now avoids re-prompting for
questions answered earlier in the same session.
* test: add structural tests for SKILL.md resolved_questions_map changes
Adds tests/test_sdlc_skill_resolved_questions_map.py covering the three
contract tasks for issue #1905:
- task-1-1: new `### Resolved Questions Map` subsection exists above
the phase_gate handler with normalization rule (strip+lowercase) and
session-scoped description; Step 5 of the phase_gate handler populates
the map alongside the Resolved Questions display block.
- task-1-2: `### For choice type decisions:` section begins with a
`Before prompting` paragraph documenting the lookup, option-compatibility
check, `{"action": "select"}` provide_input payload, `Auto-resolved ...
from captured context.` note, and fall-through on no-match.
- task-1-3: `### For feedback type decisions:` section begins with a
`Before prompting` paragraph documenting per-question lookup,
prefilled-vs-unmatched split, all-matched fast path, single merged
`{"action": "submit_feedback"}` provide_input call, and the
Auto-resolved note.
SKILL.md is a markdown behavior spec (interpreted by Claude at runtime),
so there is no executable code to exercise. These tests lock in the
structural shape of the spec — if the refiner or a future edit drops any
of the required elements, the tests fail with a precise pointer.
26 tests, all passing.
* Persist statefiles after implement phase
* Address review feedback: clarify q-index preservation, document change_approach behavior
---------
Co-authored-by: egg-orchestrator <egg@localhost>
Co-authored-by: egg <egg@example.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Replace interactive mode with run_agent_task MCP primitive (#1900)
* Initialize SDLC contract for issue #1762
* refine: draft analysis for #1762 (custom-phase MCP primitive)
Outline replacement of interactive mode with a generic
run_custom_phase MCP primitive. Analysis covers current interactive
mode callgraph (bin/egg → cli.main → run_claude → ensure_compose_services),
roster-subset plumbing that ConcurrentPhaseExecutor already supports,
and the BRC short-circuit path for degenerate rosters via
ApprovalMatrix.is_fully_acked().
Registered 9 multiple-choice decisions and 9 open-ended feedback
questions via egg-contract covering naming, BABYSIT interaction,
removal scope, roster storage location, degenerate-roster policy,
artifact retrieval, role eligibility, rollout sequencing, and
doc/test coverage.
Complexity: high (cross-cutting across orchestrator, sandbox,
bin/, docs/, tests/, and the GHA action).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refine: apply reviewer_refine non-blocking cleanups
Address non-blocking feedback from reviewer_refine ACK:
- Remove refiner's absolute worktree path leak from bin/egg
description (repo-relative is sufficient).
- Name BOTH compose call sites — runtime.py:686 (run_claude) AND
runtime.py:1060 (exec_in_new_container) — so decision-4 is easier
to answer.
- Correct concurrent_executor.py line refs (:108 for the kwarg
position; class at :102).
- Give run_claude a precise end-line estimate (634-~1050) rather
than "920+".
- Strip template boilerplate from Open Questions section so it
reads as a clean pointer to the registered contract items.
- Reframe decision-9 as confirm-Option-A rather than re-asking the
Options Analysis axis.
- Note Q5 overlap with decision-2 so the human isn't asked the same
thing twice.
No content change to the research, options, recommendation, or
complexity — these are drafting-quality fixes for the plan phase.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after refine phase
* Persist HITL resolution after refine phase gate
* plan(risk): 14-risk assessment for #1762 custom-phase primitive + interactive removal
Overall MEDIUM-HIGH, GO-with-mitigations. High-severity risks: gha_exec
import path break after relocating out of egg_lib/cli.py (R1), Pipeline
persistence for the new active_roles field and CUSTOM enum variant (R2),
reviewer-graph deadlock when a producer is selected without its critical
reviewer (R3), integration_tests compose fixture migration (R4). Surfaces
6 HITL-review questions covering decision-6 gap (producer w/o reviewer),
PipelineMode.BABYSIT audit scope, repo allowlist, draft-file keying, and
HITL gate integration scope.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: architect output for #1762 (custom-phase MCP primitive)
Document the end-to-end architecture for replacing interactive mode
with a generic run_agent_task MCP tool + PipelineMode.CUSTOM +
Pipeline.active_roles:
- MCP tool surface (run_agent_task, per decision-1 HITL resolution)
- Pipeline/PipelineMode model changes (active_roles field, CUSTOM value)
- Route validation (phase-scoped role subset, producer-required,
auto-generated branch, CUSTOM+PR reusing BABYSIT pre-flight)
- Concurrent-executor roster plumbing (honor pipeline.active_roles)
- BRC short-circuit unchanged (approval_matrix.is_fully_acked
already handles empty reviewer lists)
- Full interactive-mode removal (bin/egg, cli.main, run_claude,
run_interactive, compose.py, all ensure_compose_services call
sites; gha_exec relocates to sandbox/egg_lib/gha_exec.py)
- Docs rewrite scope and new agent-task guide
- Test coverage plan (degenerate rosters, CUSTOM+PR, persistence)
- Risks called out for risk_analyst (GHA regression, compose
callers, active_roles read-path completeness, phase-completion
semantics for single-phase pipelines)
- Acceptance-criteria hints for task_planner (16 ACs)
- Open questions flagged for reviewer_plan (has_contract for
CUSTOM+PR, start_phase reuse vs. dedicated custom_phase field,
deprecation stubs vs. deletion for bin/egg-deploy)
Architecture aligns with all 9 HITL-resolved decisions from the
refine phase gate.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: draft implementation plan for #1762 (run_agent_task + interactive-mode removal)
Decompose HITL-resolved refine analysis into 7 phases / 30 tasks covering:
Phase 1 — model + role-validation plumbing (PipelineMode.CUSTOM,
Pipeline.active_roles, validate_roles_for_custom_phase helper)
Phase 2 — route handler + roster threading (create_pipeline mode=custom
branch, _run_concurrent_phase roster override, BABYSIT preflight reuse
for CUSTOM+pr_number per decision-2)
Phase 3 — run_agent_task MCP tool definition + handler
Phase 4 — BABYSIT subsumption (route builds CUSTOM-like internal state
while user-facing babysit_pr tool stays)
Phase 5 — removal of bin/egg, egg_lib/cli.py (gha_exec relocated to
egg_lib/gha_exec.py), compose.py, run_claude, run_interactive, compose
paths in bin/egg-deploy
Phase 6 — integration-test compose fixture migration
Phase 7 — tests + docs rewrite (README, local-quickstart, deployment,
declarative-setup, kubernetes-migration, deploy-migration, sdlc-pipeline,
mcp-deployment-tools, agent-roles, CLAUDE.md, new custom-phase.md)
All 9 HITL decisions from refine gate adopted verbatim. Plan-phase
resolutions documented for 9 feedback items F1-F9. Test strategy covers
degenerate-roster short-circuit, reviewer-only rejection, cross-phase
rejection, BABYSIT parity, and GHA import relocation per risk_analyst R1.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: revise #1762 plan to address reviewer_plan NACK (6 blocking + 7 non-blocking)
Blocking resolutions:
1. TASK-5-7 extended to remove pyproject.toml [project.scripts] egg entry;
TASK-5-9 added to scrub egg --setup references from Makefile error strings
(risk_analyst R6).
2. TASK-2-7 added to audit and broaden all PipelineMode.BABYSIT gates via
new _uses_per_role_staging() helper so CUSTOM+pr_number inherits BABYSIT's
staging-branch derivation (concurrent_executor.py:174), has_contract
semantics (routes/pipelines.py:957), and PR-diff orient prompts (6192, 6357)
(risk_analyst R5).
3. TASK-2-8 added to key CUSTOM drafts by pipeline_id even when issue_number
is set, preventing draft-file collision with concurrent ISSUE-mode pipelines
(risk_analyst R11).
4. TASK-2-1 extended with explicit repo-allowlist acceptance criterion
("repo_not_allowed" HTTP 400); TASK-7-2 adds test_run_agent_task_security.py
(risk_analyst R9).
5. TASK-6-2 added to migrate top-level integration_tests/conftest.py off
compose (the egg_stack session fixture), in addition to the existing
TASK-6-1 for local_pipeline/conftest.py (risk_analyst R4).
6. TASK-2-9 added to guard phase-advance sites (pipelines.py:10591-10594,
:10957-10958) so CUSTOM pipelines terminate as COMPLETE after one phase
instead of auto-advancing into plan/implement.
Non-blocking resolutions:
- Added "Dependency Ordering" section with phase graph.
- Added "Risk Mitigation Map" table mapping each risk_analyst risk to
mitigating tasks.
- F1 revised from "out-of-scope for v1" to "parity with ISSUE mode"
(architect q1_hitl_scope, risk_analyst R14); TASK-2-1 acceptance
confirms config.hitl_gates passthrough.
- TASK-3-1/3-2 add "qualifier" schema field and use it in pipeline_id
composition (submit_task-compatible) to avoid collisions for repeat
CUSTOM runs on same issue/PR.
- TASK-2-4 acceptance broadened to exercise active_roles=["coder"] alone
(R3 producer-without-reviewer case) and assert CONSENSUS_REACHED on
first propose via ApprovalMatrix.is_fully_acked() empty-reviewer
short-circuit.
- TASK-5-8 ambiguity resolved: keep init, stub compose subs with exit 2.
- TASK-5-4 reasoning clarified: runtime.py:686 disappears transitively
via run_claude deletion; :1060 is a surgical edit to surviving
exec_in_new_container.
- TASK-5-3 grep acceptance now includes --include='*.py'.
Plan now has 7 phases and 38 tasks (up from 33); yaml-tasks appendix
validated — no pr_plan key, pr.description/test_plan/manual_steps
populated.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after plan phase
* Persist HITL resolution after plan phase gate
* feat(models,roles): add PipelineMode.CUSTOM + active_roles + validator
Phase 1 of #1762 — the data-layer plumbing for run_agent_task:
- orchestrator/models.py: add PipelineMode.CUSTOM enum value and a new
optional Pipeline.active_roles: list[str] | None field with a
field_validator that rejects empty lists, unknown AgentRole values,
and reviewer-only rosters (those deadlock BRC).
- orchestrator/state_store.py: thread an optional active_roles kwarg
through create_pipeline() so callers can persist the resolved roster.
- shared/egg_contracts/agent_roles.py: add validate_roles_for_custom_phase
helper that validates a user-supplied role subset against a phase's
producers + reviewers (after repo / has_contract filtering). Returns
(resolved_roles, None) on success or (None, error_reason) on failure,
with reasons aligned to the route-level 400 responses planned for
Phase 2.
Backward-compatible: active_roles defaults to None, so existing
pipeline JSON deserialises unchanged. All existing tests should pass.
Refs: TASK-1-1, TASK-1-2, TASK-1-3, TASK-1-4
* docs: add run_agent_task (custom-phase) guide
Phase 7 docs landing for #1762 — new tutorial for the run_agent_task
MCP primitive that replaces interactive mode:
- docs/guides/custom-phase.md: new guide covering input schema, role
selection rules, BRC short-circuit for degenerate rosters,
common invocation patterns (research-only refiner, single-coder
drive-by, coder+reviewer, PR-targeted via BABYSIT subsumption,
pre-populated analysis/plan), error responses, artifact retrieval
via git show, and the relationship to ISSUE and BABYSIT modes.
- docs/index.md: add the guide to the Guides table and to the
Task-Specific Guides lookup table so callers looking for
"one-off single-phase work" land here.
Mirrors the Phase 1 data-layer plumbing (PipelineMode.CUSTOM,
Pipeline.active_roles, validate_roles_for_custom_phase) that landed in
b18c645b1. Follow-up commits will remove interactive-mode references
from the other F9-listed docs as the coder's subtractive phases
(Phase 5 onward) land.
Refs: TASK-7-8, F9
* test: add Phase 1 tests for #1762 (PipelineMode.CUSTOM + active_roles + validator)
Cover the data-layer plumbing landed in coder commit b18c645b1:
- shared/tests/test_validate_roles_for_custom_phase.py (41 tests) —
exhaustive coverage of the new validate_roles_for_custom_phase()
helper: default roster fallback (None / []), invalid_phase,
cross_phase_role (overseer/autofixer/conflict_resolver/inspector),
reviewer_only_roster (BRC deadlock guard), invalid_roles (unknown
value, cross-phase reviewer/producer, egg-only reviewer on non-egg
repo), reviewer_contract_without_artifact, canonical ordering,
deduplication, case sensitivity, whitespace handling.
- orchestrator/tests/test_pipeline_custom_mode.py (21 tests) —
PipelineMode.CUSTOM enum value, str-enum round-trip; Pipeline
.active_roles field default=None, validator rejecting empty list /
unknown roles / reviewer-only rosters; legacy pipeline JSON
deserialises with default None (backward compat guarantee);
schema compatibility (field not required, accepts null).
- orchestrator/tests/test_state_store_active_roles.py (8 tests) —
StateStore.create_pipeline(active_roles=...) kwarg optional
(backward compat), kwarg is on returned pipeline, persists and
round-trips through save/load, ValidationError surfaces correctly
for invalid rosters.
All 70 new tests pass. Refs: TASK-1-1, TASK-1-2, TASK-1-3, TASK-1-4.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(routes): add run_agent_task CUSTOM branch to create_pipeline
Phase 2 of #1762 — the route handler and roster threading for the new
run_agent_task MCP primitive:
- orchestrator/routes/pipelines.py: extend create_pipeline() to accept
mode=custom with a required `phase` and optional `roles` list.
Validate phase membership, call validate_roles_for_custom_phase, and
return structured HTTP 400 with a details.reason ("missing_phase",
"invalid_phase", "invalid_roles", "reviewer_only_roster",
"cross_phase_role", "reviewer_contract_without_artifact",
"repo_not_allowed"). Auto-generate branch 'egg/custom-<pipeline_id>'
when no branch is passed AND no PR is targeted; otherwise inherit
the PR head branch. Reuse the BABYSIT PR preflight unchanged for
CUSTOM+pr_number.
- Introduce _uses_per_role_staging() helper so CUSTOM+PR inherits
BABYSIT's per-role staging-branch derivation, has_contract=False,
and PR-diff-aware orient prompts.
- Thread pipeline.mode into _pipeline_identifier / _get_draft_path so
CUSTOM pipelines always key drafts by pipeline_id (avoids collision
with a concurrent ISSUE-mode pipeline on the same issue_number).
- _run_concurrent_phase now reads pipeline.active_roles when set and
builds the roster from it instead of get_roles_for_phase; the
existing review-graph filter at lines 7263-7270 already prunes edges
to the active set.
- Repo allowlist check via config.repo_config.is_writable/readable_repo
(risk_analyst R9). Rejects shell-metacharacter repos with a 400
and reason "repo_not_allowed".
- has_contract logic extended: CUSTOM without PR sets has_contract=True
when analysis/plan is passed inline OR an ISSUE contract file exists
for the same issue_number.
- Phase-advance guard: CUSTOM-mode pipelines mark COMPLETE after their
single phase reaches CONSENSUS_REACHED (no auto-advance).
- orchestrator/concurrent_executor.py: ConcurrentPhaseExecutor now
documents that `roles=` is driven by Pipeline.active_roles for CUSTOM
mode. get_worktree_branch extended to treat CUSTOM+pr_number the
same as BABYSIT (per-role staging-branch egg/babysit-pr/<pr>/<sha>/<role>).
Refs: TASK-2-1..TASK-2-9, TASK-4-1
* feat(mcp): add run_agent_task MCP tool + handler
Phase 3 of #1762 — the user-facing MCP primitive that lets hosts spawn
a CUSTOM-mode pipeline for one phase with a chosen role subset.
- orchestrator/mcp_tools.py: add run_agent_task to PIPELINE_TOOLS with
inputSchema for phase (refine|plan|implement), roles, repo,
description, branch, base_branch, pr_number, issue_number, analysis,
plan, qualifier, config. Only phase/repo/description are required.
- _handle_run_agent_task() forwards to POST /api/v1/pipelines with
mode=custom. Pipeline-ID derivation matches the plan:
issue + qualifier → issue-<N>-<qualifier>
issue only → issue-<N>-custom
pr + qualifier → pr-<N>-<qualifier>
pr only → pr-<N> (BABYSIT-compatible)
neither → custom-<hex>
- Register 'run_agent_task' in handle_tool_call's handlers dict.
- Docstring for _handle_babysit_pr notes that BABYSIT is now a façade
over the CUSTOM code path.
Refs: TASK-3-1, TASK-3-2, TASK-3-3, TASK-4-2
* test: add Phase 2+3 tests for #1762 (run_agent_task route + MCP handler)
Cover the new CUSTOM-mode route handler and MCP tool plumbing landed
in coder commits 3a873073e (route) and bfc7c4d4c (MCP tool):
- orchestrator/tests/test_run_agent_task_handler.py (29 tests) —
PipelineToolHandler._handle_run_agent_task client-side validation
(missing/invalid phase, missing repo, shell-metachar repo rejection,
missing description, roles non-list, qualifier regex, issue_number
and pr_number positive-int checks). Pipeline-ID derivation rules:
issue+qualifier, issue only, pr only (BABYSIT-compatible), pr+qualifier,
synthetic fallback. Request-body construction (mode=custom, phase,
roles omitted when null, analysis/plan forwarded, pr_number forwarded,
config JSON string parsed, invalid config returns error). Server
error handling (400 with reason surfaced, 409 with existing_pipeline_id
surfaced). Success shape (task_id + status, created_not_started when
start fails).
- orchestrator/tests/test_pipelines_routes_custom_mode.py (19 tests) —
POST /api/v1/pipelines with mode=custom: missing_phase, invalid_phase,
valid phase acceptance; reviewer_only_roster, cross_phase_role,
invalid_roles, reviewer_contract_without_artifact (decision-6/8 gates);
auto-generated egg/custom-<pipeline_id> branch fallback (decision-7);
caller branch preserved; repo allowlist 400 (risk_analyst R9),
shell-metachar repo rejected; CUSTOM+PR inherits BABYSIT pre-flight
(merged/fork/empty-diff); pr_number type checks.
All 48 new tests pass. Refs: TASK-2-1..TASK-2-9, TASK-3-1..TASK-3-3.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(sandbox,deploy): remove interactive mode (coder-scoped slice)
Phase 5 of #1762 — coder-owned file changes only. The tester /
reviewer and script-owner slices (bin/egg, bin/egg-deploy, sandbox/egg,
and the associated test-file deletions) are out of coder's file
boundaries and ship in a separate commit from the appropriate role.
Removed:
- sandbox/egg_lib/cli.py: the interactive-mode main() and the
now-relocated gha_exec(). main() is gone; gha_exec() moved.
- sandbox/egg_lib/compose.py: the 932-line compose lifecycle module.
No replacement — deployment is k8s-only.
- sandbox/egg_lib/runtime.py::run_claude(): the interactive entry
point (~310 lines). exec_in_new_container() survives for GHA.
- sandbox/egg_lib/runtime.py::ensure_compose_services() call sites:
the one inside run_claude goes transitively; the one inside
exec_in_new_container is removed in place.
- sandbox/entrypoint.py::run_interactive() + dispatcher branch.
No-command + pipeline-mode still errors cleanly via the
pre-existing branch; no-command + non-pipeline mode now exits 2
with a clear "use run_agent_task" message.
- Makefile: egg --setup error-message strings replaced with
bin/egg-deploy init.
- pyproject.toml: the egg = egg_lib.cli:main script entry is gone
(without this uv pip install -e . would ImportError).
Added:
- sandbox/egg_lib/gha_exec.py: new module housing gha_exec()
relocated from cli.py. Signature + return semantics byte-identical.
- action/entrypoint.sh: import updated from egg_lib.cli import
gha_exec to egg_lib.gha_exec import gha_exec.
- sandbox/egg_lib/__init__.py: re-exports gha_exec from the new
module path; drops the main and run_claude re-exports.
Refs: TASK-5-1, TASK-5-2, TASK-5-3, TASK-5-4, TASK-5-5, TASK-5-6,
TASK-5-9 (partial; Makefile only)
* docs: address reviewer_code NACK on custom-phase.md
Reviewer_code flagged 3 blocking items + 3 non-blocking nits against
the initial docs/guides/custom-phase.md at 22:16:08. Fixes:
Blocking:
- Error-response table: the previous table used fabricated reason
strings. Rewrite to use the exact strings returned by
validate_roles_for_custom_phase in agent_roles.py b18c645b1:
reviewer_only_roster, cross_phase_role, invalid_roles,
reviewer_contract_without_artifact, invalid_phase. Response shape
switched from {error, detail} to {details: {reason}} per plan
TASK-2-1. Added a pointer to the source lines and commit.
- Sample response body: status "running" -> "started" (matches the
BABYSIT handler pattern and TASK-3-2 acceptance).
- Invalid CLI command: "egg-orch pipeline show" -> "egg-orch pipeline
get" (and mention "status" subcommand); the subcommand "show" does
not exist.
Non-blocking:
- Self-referential PID example (issue-1762-membump) swapped for a
neutral custom-ab12cd34 placeholder, with a comment noting callers
should substitute their actual pipeline id.
- "egg-sdlc submit-task" prefixed with bin/ to reflect that bin/egg is
removed in this PR but bin/egg-sdlc is not (TASK-5-7 only removes
the top-level egg binary).
- reviewer_contract auto-handling: added a cross-reference to
TASK-2-2 and pipelines.py:957 where the route computes has_contract,
with the concrete signals (analysis / plan / existing contract file).
Refs: TASK-7-8
* docs: expand #1762 doc sweep — qualifier, README, CLAUDE, quickstart
Adds the qualifier field + pipeline-id derivation table to
docs/guides/custom-phase.md (was missing from the initial draft;
present in orchestrator/mcp_tools.py:213 inputSchema and used by
_handle_run_agent_task to build issue-<N>-<qualifier> /
pr-<N>-<qualifier> / custom-<hex> pipeline ids). Clarifies that
CUSTOM contracts are keyed by pipeline_id not issue-<N>.json
(avoiding collision with ISSUE-mode).
Scrubs interactive-mode + compose references now that coder commit
f93764c31 deleted sandbox/egg_lib/cli.py, compose.py, run_claude(),
run_interactive(), and the bin/egg entry:
- README.md Quick Start: replace `egg`/`egg --setup`/`egg --private`
walkthrough with bin/egg-deploy init/up + the three MCP tools
(submit_task / babysit_pr / run_agent_task). Points at the new
custom-phase guide.
- CLAUDE.md Key Entry Points: replace "Interactive use goes through
the claude CLI" line with a pointer to the three MCP tools and a
note that bin/egg was removed in #1762.
- docs/guides/local-quickstart.md: swap `egg --setup` (step 1) for
`bin/egg-deploy init` and drop the `egg --public`/`--private`/
`--exec` examples. Redirect readers to the MCP tool calls. Replace
the `egg --reset` troubleshooting tip with the
`make build && make k3s-import && make deploy` equivalent.
Refs: F9, TASK-7-5, TASK-7-8 (sweeps)
* test: replace test_cli_main.py with test_gha_exec.py for #1762 Phase 5
The coder removed sandbox/egg_lib/cli.py (interactive mode entry point)
in commit f93764c31 and relocated gha_exec() to
sandbox/egg_lib/gha_exec.py. Per TASK-7-3 of the #1762 plan:
- Delete tests/sandbox/test_cli_main.py (the module it tests no longer
exists).
- Add tests/sandbox/test_gha_exec.py (20 tests) covering:
* Import-path relocation (risk_analyst R1): gha_exec importable from
egg_lib.gha_exec; re-exported from the egg_lib package; legacy
egg_lib.cli module no longer findable (ImportError sentinel).
* action/entrypoint.sh references the new import path — scripts in
lockstep with Python so GHA does not break at merge time.
* Happy-path orchestration (exit 0 on success, 1 on container failure).
* Failure paths: network creation, gateway start, empty prompt.
* Mode detection: visibility private/internal → private, visibility
public → public, explicit INPUT_MODE overrides auto-detection.
* Claude command construction includes prompt + model.
* Extra-env passthrough: EGG_BOT_NAME, EGG_ISSUE_NUMBER,
EGG_COMMIT_SHA, EGG_AGENT_ROLE, EGG_PR_NUMBER, EGG_PIPELINE_ID are
forwarded to exec_in_new_container; no spurious keys when unset.
All 20 new tests pass. Refs: TASK-7-3, risk_analyst R1.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(routes,exec,models): address reviewer_code NACK on #1762 run_agent_task
Blocking fix (TASK-2-8 regression in ~16 call sites of
_pipeline_identifier / _get_draft_path):
- orchestrator/routes/pipelines.py: _pipeline_identifier now auto-
detects CUSTOM-style pipeline_ids from their naming convention —
`custom-<hex>` and `pr-*` always key on pipeline_id; any pipeline_id
that matches `issue-<N>-<qualifier>` (as opposed to bare `issue-<N>`)
also keys on pipeline_id. This is a belt-and-braces check so TASK-2-8
works across every existing call site that was not explicitly
threaded with mode=; callers that DO pass mode=CUSTOM continue to
win unconditionally. ISSUE-mode bare `issue-<N>` keys unchanged.
Non-blocking polish items from the same review:
- orchestrator/routes/pipelines.py: remove the double `custom-` prefix
in auto-generated branch names (`egg/custom-custom-<hex>` →
`egg/<pipeline_id>` when pipeline_id already starts with `custom-`).
- orchestrator/routes/pipelines.py: drop the redundant
`from egg_contracts.models import PipelinePhase as _PipelinePhase`
local import — PipelinePhase is already imported at module scope.
- orchestrator/concurrent_executor.py: extract `_uses_per_role_staging`
to a module-level helper (DRY with routes/pipelines.py; avoids
re-computing the BABYSIT / CUSTOM+PR check inline with five nested
ifs).
- orchestrator/models.py: Pipeline.active_roles producer check now
uses an explicit cross-phase set difference so `overseer` /
`autofixer` / `conflict_resolver` / `inspector` cannot spuriously
satisfy "at least one producer" if a Pipeline is constructed
directly (outside the validate_roles_for_custom_phase path).
All 549 regression + new-CUSTOM tests pass (720 including the wider
BRC / orient-prompt / concurrent-executor suites). Tester's existing
`invalid_roles` reason for cross-phase roles is preserved — the
`role_not_in_phase` distinction reviewer_code suggested is
non-blocking and deferred to a follow-up.
* docs: further F9 sweep — deployment, deploy-migration, declarative-setup
Continues the #1762 doc sweep to cover interactive-mode + compose
removal across the remaining F9-listed docs:
- docs/guides/deployment.md:
* Deployment Methods table: drop the "egg CLI" row (removed in
#1762); `bin/egg-deploy` against k3s is now the only local-dev
path. Add a removal-note callout pointing at the custom-phase
guide.
* Remove the "egg CLI (Recommended)" section wholesale.
* `bin/egg-deploy init` note: the `lifecycle-secret` is no longer
auto-generated by `egg --setup` (that wizard is gone); spell out
the openssl fallback as the primary path.
* Claude-binary-not-found troubleshooting: `egg --reset` replaced
with the equivalent `make build && make k3s-import && make
deploy` sequence.
- docs/guides/deploy-migration.md:
* Header note updated to record the #1762 compose removal in
addition to the #1553 k3s migration, and to state explicitly that
none of the `docker compose` commands below still work. Still
retained for historical reference.
- docs/architecture/declarative-setup.md:
* CLI Interface section: `egg --setup` / `egg --setup --full`
flagged as removed in #1762; replacement pointer to
`bin/egg-deploy init` + manual `~/.config/egg/` setup.
* Implementation Status: `egg_lib/setup_flow.py` was deleted with
bin/egg / egg_lib/cli.py / egg_lib/compose.py in #1762 —
paragraph reframed as historical.
* Related Documentation: add custom-phase guide cross-reference.
Refs: F9, TASK-7-5, TASK-7-7
* test: register run_agent_task, delete obsolete sandbox tests, fix lint (#1762)
After coder commit a23be9b91 (Phase 1-5 coder-owned slice), three tests
referenced modules that no longer exist, one test asserted a closed set
of tool names that did not include run_agent_task, and ruff flagged
unused imports in my Phase 1+5 tests.
Changes:
- orchestrator/tests/test_mcp_tools.py::TestToolRouting
::test_all_tools_registered — add "run_agent_task" to the expected
set so the assertion matches the new PIPELINE_TOOLS surface.
- tests/sandbox/test_egg.py DELETED — the test loads
sandbox/egg (the top-level launcher binary) via SourceFileLoader,
which imports egg_lib.cli. egg_lib.cli was removed in the coder's
Phase 5 slice. The binary (sandbox/egg + bin/egg symlink) is on the
script-owner's removal slice per the coder's handoff note.
- sandbox/tests/test_entrypoint_pipeline_guard.py DELETED — tested
run_interactive() which was removed. Replaced by
sandbox/tests/test_entrypoint_no_interactive.py:
* TestRunInteractiveRemoved asserts the attribute is gone (regression
sentinel so a stale re-export can't silently restore it).
* TestNoArgsInPipelineMode: pipeline mode + no args → exit 1 with
orchestrator completion signal (matches entrypoint.py:2048-2072).
* TestNoArgsInHostMode: host mode + no args → exit 2 with
"use run_agent_task" message, no orchestrator signal (nothing
to notify).
- Ruff auto-fix applied to the new Phase 1/2/3/5 tests
(test_validate_roles_for_custom_phase, test_pipeline_custom_mode,
test_run_agent_task_handler, test_gha_exec) — remove unused imports
(subprocess, importlib, MagicMock, json) and reformat lines.
All 141 new tests + 540 existing orchestrator/shared tests pass. Refs:
TASK-7-3.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after implement phase
* Remove ephemeral agent-output handoff artifacts (#1731)
* Persist statefiles after pr phase
* Fix checks: apply automated formatting fixes
* chore: delete bin/egg and sandbox/egg entrypoints (#1762)
These are the final file-removal tasks from plan #1762 (TASK-5-7 / TASK-5-8).
The pipeline's coder agent was blocked from making this change by the gateway
file-boundary policy (see #1901) because bin/egg (symlink) and sandbox/egg
(extensionless script) don't match any extension-based entry in
CODER_PATTERNS.allowed_patterns. The rest of #1762 is in PR #1900; this commit
completes the removal manually from the host.
bin/egg was a symlink to ../sandbox/egg that executed the interactive sandbox
CLI, and sandbox/egg was the target script. Both are dead code now that
egg_lib.cli.main (the interactive mode entry point) has been removed in PR
#1900.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix failing unit tests from interactive mode removal
- Remove TestRunInteractiveSubprocess tests referencing deleted
run_interactive function (removed in #1762)
- Patch repo allowlist in CUSTOM-mode route tests so role validation
errors surface instead of repo_not_allowed short-circuit
- Fix bare prefix computation in _read_source_branch_artifacts to use
issue_number directly instead of _pipeline_identifier which returns
pipeline_id for CUSTOM-mode pipelines, breaking the fallback chain
* Address review feedback: atomic CUSTOM phase init, compose stubs, BRC parity
- Move CUSTOM phase initialization into state_store.create_pipeline
(alongside BABYSIT handling) so the phase is set atomically during
creation. Removes the post-creation try/except Exception: pass block
that could silently leave pipelines on the wrong phase. (Blocking
review item 1, suggestion 6.)
- Stub bin/egg-deploy compose commands (up/down/logs/build) with
deprecation exit 2 pointing at Kubernetes docs. Removes all
docker compose / COMPOSE_FILE references. (TASK-5-8.)
- Extend _brc_history_identifier to handle CUSTOM+PR pipelines with
SHA-based keys, matching BABYSIT behavior for transcript
preservation across re-runs. (Suggestion 2.)
- Add heuristic invariant documentation to _pipeline_identifier so
future ID patterns are flagged. (Suggestion 4.)
- Replace except Exception: pass with logger.warning in the repo
allowlist block so broken config is observable. (Suggestion 5.)
- Add TestCustomPhaseThreading tests verifying custom_phase is
threaded from route to create_pipeline for all three phases.
* Address re-review feedback: fix init_config next steps, add CUSTOM+PR BRC history tests
- Fix init_config 'Next steps' pointing to deprecated egg-deploy up;
now directs to Kubernetes deployment guide.
- Remove compose_project_name from generated config.yaml template
(no consumer after #1762 compose removal).
- Add CUSTOM+PR test coverage for _brc_history_identifier: format
tests, SHA-based namespacing, BABYSIT parity, and fallback cases.
---------
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: James Wiesebron <jameswiesebron@khanacademy.org>
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com>
* plan(#1897): architect revision 4 — address reviewer_plan NACK on rev 3
Three blocking items resolved:
1. Line numbers in current_architecture.brc_preamble_assembly (and
downstream F1, F7, file_inventory, risk_analyst propagation):
- producer STAY ALIVE 5959 → 6231
- reviewer STAY ALIVE 6020 → 6292
- QUESTION reviewer example 6062-6074 → 6338-6346
- BRC_HISTORY_TYPES 4775 → 5037-5052
- messages.py try/except 181-184 → 179-184
- health_monitor._on_message_sent 330-360 → 330-363
Re-verified via fresh grep. Added grep anchors and symbolic
references so future drift is harmless.
2. Track 7 rewritten — the orchestrator runs Waitress via
waitress.serve() at orchestrator/cli.py:284-290, NOT Gunicorn.
New scope: EGG_WAITRESS_THREADS env var, /healthz on
orchestrator/routes/health.py, egg_inflight_long_polls gauge,
MAX_WAIT × thread-count coupling documented. Plan TASK-4-*
flagged for same correction.
3. Track 6 SSE URL corrected — `/api/v1/pipelines/<id>/stream`
(decorator at orchestrator/routes/pipelines.py:11772), NOT
`/events`. Plan TASK-5-1 flagged for same correction.
Seven non-blocking items also addressed: MAX_READY_POLL_CYCLES vs
MAX_READY_POLLS citation, off-by-2 on messages.py, health_monitor
line range, merge_order Phase 9 added, 'Plan is CONFIRMED' softened
to 'Plan is at revision 3, CONSENSUS_PROPOSE', three-subcase SSE
fallback semantics (503 / connection-refused / Redis-down), three
separate timeouts distinguished (gateway session idle / Squid proxy
idle / Waitress connection).
Refs: #1897
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan(#1897): revision 4 — address reviewer_plan NACK (blockers 1-6 + 10 non-blocking)
Rewrites the plan to address the reviewer_plan NACK of 2026-04-23T05:49:10:
Blocker fixes (6):
1. Phase 4 rebuilt on Waitress (the actual production server per
orchestrator/cli.py:284-290 waitress.serve(threads=16)), NOT
Gunicorn. New EGG_ORCH_WAITRESS_THREADS env var, refuse-to-boot
below 4. Gunicorn migration filed as follow-up.
2. TASK-4-2 deleted — /api/v1/health at routes/health.py:34-77
already does NOT touch the message store (HealthTracker
in-memory only), k8s probes already point at it. Regression
test added in TASK-4-3 to lock that in.
3. TASK-2-3 file list fixed — uses new orchestrator/env_config.py
(created), orchestrator/api.py (route reg), orchestrator/cli.py
(startup log). Dropped orchestrator/config.py and app.py which
do not exist.
4. TASK-5-1 SSE URL corrected to /api/v1/pipelines/<id>/stream
(verified at routes/pipelines.py:11772 and README.md:136-137),
NOT /events. New acceptance test locks SSE event-name literal
'consensus.reached' so future refactors cannot silently break.
5. New TASK-7-5 drops QUESTION from cmd_message_send argparse
choices at orch_cli.py:1862 and help text. Ordered AFTER
TASK-7-1/7-2/7-3, BEFORE TASK-7-4.
6. TASK-2-4 wait-loop semantics pinned: loops FOREVER, exits only
on terminal match (exit-0 + match) or permanent (exit-3 → exit
1); exit-1 timeout continues silently. TASK-6-1 prompt rewritten
to drop EGG_MESSAGE_POLL_MAX_WAIT reference, add literal "run
this exact command and do nothing else" framing.
Non-blocking fixes (10):
- Line numbers updated to verified values (STAY ALIVE 6231/6292,
QUESTION example 6342-6346, BRC_HISTORY_TYPES 5037-5052).
- Test file paths corrected to actual names (test_messages.py not
_route, test_signals.py not _route, test_health_routes.py plural,
test_app_startup.py explicitly marked as new file).
- shared/prompts/ (not shared/agent-prompts/ which doesn't exist).
- TASK-3-2 metadata-not-body wording tightened.
- TASK-2-2 author musing deleted (argparse misuse → exit 3 per
contract, no ambiguity).
- TASK-5-1 MAX_READY_POLLS (bash) vs MAX_READY_POLL_CYCLES (Python
at consensus_wrapper.py:38) clarified with file references.
- RISK-4 mitigation rewritten: name Squid read_timeout/request_
timeout directives in the gateway image (rebuild-required), NOT
a k8s ConfigMap key which does not exist.
- Phase independence table added (only 1→2, 2→6, 4→6, 6→7 are
hard-ordered).
- TASK-8-3 harness clarified (subprocess + proxy simulator, not
ambiguous "boot the orchestrator").
- New TASK-3-4 adds HEARTBEAT rate limit (EGG_HEARTBEAT_RATE_LIMIT
default 20/min, 429 on exceed) per architect TD-3.
Plan grew from 1187 lines (rev 3) to 1565 lines (rev 4); 24 tasks →
26 tasks (TASK-7-5 and TASK-3-4 are new).
Refs: #1897
* risk_analyst(#1897): revision 3 — address reviewer_plan NACK
Fixes two blocking factual errors and five non-blocking tightenings
from reviewer_plan NACK 3994da6c on rev 2.
Blocking fixes (verified against code):
1. RISK-3 + DEP-4 — orchestrator uses Waitress, not Gunicorn. Verified
at orchestrator/cli.py:288-290: `waitress.serve(app, host=host,
port=port, threads=16)`. No gunicorn, gevent, or worker_class
anywhere in orchestrator/ or k8s/. Rewrote mitigation from
"switch to gevent/eventlet" to "raise EGG_ORCH_WAITRESS_THREADS
(default max(16, EGG_MAX_CONCURRENT_LONG_POLLS + 4))"; dropped
Gunicorn --timeout point (Waitress channel_timeout is idle-channel
only, not per-request). DEP-4 status upgraded from "PRESENT but
not audited" to "PRESENT — Waitress 16 threads, audited,
undersized for new workload".
2. RISK-4 + DEP-3 — Squid timeouts are baked into gateway image at
gateway/squid.conf:135-137 (connect_timeout 30, read_timeout 60,
request_timeout 60); no ConfigMap key exists. Rewrote mitigation
to offer Path A (new ConfigMap + entrypoint template) vs Path B
(hardcoded cap in orchestrator + refusal-to-boot when
EGG_MESSAGE_POLL_MAX_WAIT > 60). Updated DEP-3 status to
"ENVIRONMENTAL — baked into image; NO ConfigMap affordance".
Non-blocking tightenings:
- DEP-2 — flag unmitigated connection-pool sizing gap; recommend
plan add TASK-1-4 for redis.ConnectionPool(max_connections=...).
- RISK-2 — note architect TD-3 HEARTBEAT rate-limit (429 above
20/min) dropped from plan TASK-3-1; classify as deferred residual.
- RISK-7 — drop "If SSE path chosen" conditional (SSE is locked by
plan Phase 5); point at sandbox SIGTERM acceptance test.
- open_questions_for_task_planner — mark Q3 (exit-code contract) and
Q5 (warn on raised cap) RESOLVED with task pointers.
- testing_recommendations — align load test with plan TASK-4-1
smoketest (10 concurrent waits); rescope 50-socket peak as
follow-up issue.
- security_posture_summary — add side-channel completeness note
(wait --for HEARTBEAT observable but no worse than short-poll).
All twelve risks remain valid; no new risks surfaced from the
reviewer NACK reconciliation.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs(#1897): agent-wait-patterns reference + concurrent-execution "How to wait" + mission rule
Creates the canonical `docs/reference/agent-wait-patterns.md` that the
new `egg-orch message wait-loop` idiom and its supporting primitives
(exit-code contract, HEARTBEAT schema, rate-limit, Waitress-threads
coupling, Squid-directive coupling) all link to. Anchors the STAY
ALIVE wait behaviour in one authoritative reference so future prompt
tweaks cannot regress back to sleep/poll loops.
- New reference file — all eight sections from TASK-9-1 (canonical
idiom for producer+reviewer, four anti-patterns quoted from #1897,
`egg-orch message wait` exit-code contract 0/1/2/3, HEARTBEAT
metadata schema + when to emit, `EGG_HEARTBEAT_RATE_LIMIT` 429
shape, `EGG_MESSAGE_POLL_MAX_WAIT` ↔ gateway-Squid coupling with
the image-rebuild caveat called out, `EGG_ORCH_WAITRESS_THREADS`
refuse-below-4 rule, cross-ref to Concurrent Execution guide).
- `docs/guides/concurrent-execution.md` — TASK-9-2: adds a "How to
wait" subsection under Message Bus pointing at the new reference,
drops QUESTION from the Message Types table and the JSON example,
replaces the "in-memory doesn't block" note with the new
both-backends-block semantics, mentions the clear-on-transition
wake-up, and calls out QUESTION's removal with a pointer to the
structured alternatives.
- `docs/index.md` — adds the new reference to the Reference table and
a task-type lookup row for "Agent STAY ALIVE / bus waits".
- `sandbox/agent-config/rules/mission.md` — TASK-6-2: replaces the
`egg-orch message poll --wait 30` rule with the new wait-loop
rule and a forward pointer to the reference.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Phase 1-2 (#1897): event-driven message wait primitive
Adds the backend plumbing that lets agents block on a typed BRC event
instead of simulating waits with sleep-and-poll loops:
- MessageStore (in-memory): per-pipeline threading.Condition so
get_messages(wait=N, wait_for_types=[...]) blocks until a matching
message is appended OR clear() fires notify_all() (RISK-5).
- RedisMessageStore: XREAD BLOCK loop with a server-side message_type
filter and a 100-iteration inner-loop cap to bound flood-of-unwanted-
types cases.
- Removed the silent TypeError -> non-blocking fallback in
routes/messages.py — both backends now support wait natively and a
regression must propagate (not false-green CI).
- New HTTP endpoint GET /api/v1/pipelines/{id}/messages/wait accepting
?for=TYPE (repeatable, required), ?from=ROLE, ?timeout=N (clamped by
EGG_MESSAGE_POLL_MAX_WAIT, default 60).
- HEARTBEAT enum member + server-side schema validation in send_message
(metadata.state in {WORKING, WAITING_ON_ROLE, PROPOSED, IDLE};
WAITING_ON_ROLE requires metadata.waiting_on).
- EGG_MESSAGE_POLL_MAX_WAIT env knob with a startup WARNING…
jwbron
added a commit
that referenced
this pull request
Apr 23, 2026
* Initialize SDLC contract for issue #1765
* docs: fix mcp-deployment-tools and STRUCTURE for #1759 [doc-updater] (#1835)
* docs: fix mcp-deployment-tools and STRUCTURE for #1759
Update docs to match the as-merged implementation from #1759:
- mcp-deployment-tools.md: remove the `repo` parameter from
prune_stale_worktrees (dropped during review — gateway sweeps all
repos; keeping the field would silently mislead callers)
- mcp-deployment-tools.md: fix "Both fields are required" in
validate_network_isolation (only pipeline_id is required; role
defaults to "coder")
- STRUCTURE.md: add orchestrator/redaction.py,
orchestrator/routes/deployment.py, and
integration_tests/local_pipeline/test_k8s_deployment_tools.py
* docs: fix alphabetical order of test_k8s_deployment_tools in STRUCTURE.md
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1838: inline per-repo worktree errors into GatewayError message (#1843)
* Fix #1838: inline per-repo worktree errors into GatewayError message
When every worktree fails, the gateway returns a 500 with the per-repo
reasons in details.errors. Downstream callers (kubernetes_spawner,
concurrent_executor) only stringify the exception, so the specific
cause was dropped and spawn failures surfaced as the generic
"Failed to create any worktrees" in pipeline.error.
create_worktrees now catches GatewayError, inlines details.errors into
the message, and re-raises. details stays populated for callers that
want structured access.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Retrigger CI: runner shutdown caused spurious failure
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix #1840: reconcile stale RUNNING records on non-RUNNING pipelines (#1846)
* Fix #1840: reconcile stale RUNNING records on non-RUNNING pipelines
The RUNNING-only guard in `_reconcile_pod_state` (and the mirror
check in `_reconciliation_sweep`) dropped pod-exit events on any
pipeline that had gone FAILED or AWAITING_HUMAN. When upstream
cleanup (e.g. #1837) left agent/container records stuck at RUNNING,
those records survived forever — `get_status` reported live agents
that no longer had backing pods, diverging from `list_containers`.
Loosen both guards to reconcile stale RUNNING records regardless
of the pipeline's top-level status, with a cheap short-circuit when
a terminal pipeline has nothing to clean up (common case). The
pipeline's own status is only escalated to FAILED when it was still
RUNNING — terminal states and AWAITING_HUMAN are preserved so this
path never undoes a human gate or re-terminates an already-failed
pipeline.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix checks: update container_monitor tests for #1840 behavior change
Tests test_ignores_non_running_pipelines and test_skips_non_running_pipelines
asserted the old behavior where non-RUNNING pipelines were skipped entirely.
With #1840, non-RUNNING pipelines with stale RUNNING records are now
reconciled (records updated, pipeline status preserved). Updated both tests
to match the new intended behavior.
* Address review feedback on #1840 reconciliation fix
- Update _reconcile_pod_state docstring to reflect new behavior
- Scope sweep short-circuit to current phase only (matches sweep body)
- Set explicit error in FAILED pipeline test for meaningful assertion
- Replace Any type hint with PipelineStatus | None
- Use explicit None check instead of truthiness in _make_pipeline
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1841: propagate full ContainerInfo through concurrent-spawn bookkeeping (#1845)
* Fix #1841: propagate full ContainerInfo through concurrent-spawn bookkeeping
The concurrent-spawn path in _run_concurrent_phase rebuilt a minimal
ContainerInfo from scratch, discarding the K8s-specific fields (pod_name,
namespace, job_name) the spawner had already populated. Carry the full
ContainerInfo through AgentExecution.container_info so downstream state
records preserve K8s metadata for debugging and tooling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review: fix _spawn_and_wait K8s metadata, add status assertion
* Fix _spawn_and_wait None dereference, add sequential path test
Remove unreachable else-branch in _spawn_and_wait that would crash with
AttributeError if reached (spawned.container_info dereferenced after
None check). Use model_copy unconditionally — consistent with the rest
of the function which already assumes container_info is not None.
Add test_spawn_and_wait_k8s.py covering K8s metadata preservation
through the sequential spawn path.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1839: retry transient worktree-creation failures during agent spawn (#1847)
* Fix #1839: retry transient worktree-creation failures during agent spawn
A single transient `gateway.create_worktrees` error used to kill the entire
pipeline — one fetch timeout, connection reset, or lock contention against
the per-repo git lock cascaded to every agent in a concurrent phase and
forced a full cancel+resubmit.
Add a bounded retry (default 2 retries, 2s initial backoff, 2.5x scale)
around the call in `kubernetes_spawner._spawn_agent_job`, with a coarse
classifier that fails fast on permanent errors (4xx validation, 404
"Repository not found") and retries on transient ones (408/429/5xx and
connection-level failures). Emit a structured `event_type=spawn_attempt`
log per attempt with outcome, error category, and duration so spawn
failures can be analyzed without correlating orchestrator and gateway
pod logs.
Retry budget is configurable via new `PipelineConfig` fields
(`spawn_max_retries`, `spawn_retry_initial_backoff_seconds`) and plumbed
through `create_concurrent_spawn_fn`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: fix classify/retry inconsistency, forward retry config
- Reorder _classify_spawn_error to check message fragments before status
codes, matching _is_transient_spawn_failure priority. Fixes the case
where GatewayError('Repository not found', status_code=500) was logged
as 'transient_500' but not retried.
- Forward spawn_max_retries and spawn_retry_initial_backoff_seconds
through _spawn_and_wait (single-phase path) and restart_agent_job so
PipelineConfig knobs apply consistently across all spawn paths.
- Replace getattr(pipeline.config, ...) with direct attribute access for
Pydantic fields that always have defaults.
- Add test asserting _classify_spawn_error and _is_transient_spawn_failure
agree on permanent-message-with-transient-status edge case.
* Forward spawn retry config in restart_agent route
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* docs: update orchestrator reconciliation docs for #1840 (#1848)
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* docs: document spawn_max_retries config fields (#1849)
Add spawn_max_retries and spawn_retry_initial_backoff_seconds to the
PipelineConfig reference table in sdlc-pipeline.md. These fields were
introduced in #1847 to retry transient gateway worktree-creation failures
during agent spawn.
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* Fix #1837: finalize spawn-failure cleanup state and surface accurate error (#1844)
* Fix #1837: finalize spawn-failure cleanup state and surface accurate error
`_run_concurrent_phase` left two footguns on partial spawn failures:
survivor agents/containers stayed `RUNNING` in the pipeline store because
the reconciler skips non-RUNNING pipelines, and the caller formatted
`pipeline.error` as "Container exited with code 1" even though no
container ever exited.
The cleanup block now writes the aborted survivors back as FAILED before
returning and raises a dedicated `SpawnFailureError` (subclass of
`KubernetesSpawnError`) whose message lists the roles and reasons. The
outer handler already catches `KubernetesSpawnError`, so `pipeline.error`
now reads "Spawn failed for <roles> — …" instead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: rename variable, strengthen test assertions
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1855: expose readiness history in check_health (#1858)
* Fix #1855: expose readiness history in check_health
Extend the orchestrator and gateway /api/v1/health endpoints (and the
MCP check_health tool that composes them) with healthy_since,
last_unhealthy_at, process_start_time, and a bounded recent_transitions
ring buffer. A shared HealthTracker utility records every health
observation and derives transition state, so operators can tell
"stable for hours" from "just came up after recent flapping" without
cross-referencing logs.
* Fix checks: add egg_health to gateway Dockerfile, fix still-healthy branch in HealthTracker
* Add missing egg_health COPY to orchestrator Dockerfile
The orchestrator Dockerfile did not copy the shared egg_health package,
causing a ModuleNotFoundError in the container. Also adds a TODO for
wiring actual health status when degraded-state evaluation is added.
Addresses review feedback on PR #1858.
---------
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1850: stop silent-docker-defaulting when runtime detection fails (#1860)
* Fix #1850: stop silent-docker-defaulting when runtime detection fails
get_deployment_context silently reported runtime=docker when the
orchestrator ran in k3s but EGG_RUNTIME was unset, so rebuild_and_rollout
and other gated tools refused with a misleading "not_available_on_runtime:
docker" instead of "apiserver unreachable."
- Auto-detect runtime from KUBERNETES_SERVICE_HOST when EGG_RUNTIME is
unset; expose provenance via a new detection_source field on the
deployment context.
- Demote runtime to "unknown" with detection_error when both apiserver
probes fail, so rebuild_and_rollout can refuse with
runtime_detection_failed rather than masquerade as a docker deployment.
- Flag images_unavailable when cluster introspection succeeds but the
deployment listing came back empty, so an empty images map no longer
silently contradicts the tool's own docstring.
- Set EGG_RUNTIME=kubernetes explicitly in the orchestrator k8s
manifest, closing the specific misconfig that triggered the report.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback on runtime detection PR
- MCP handler: explicitly short-circuit runtime_detection_failed
instead of relying on the accidental fallthrough via missing
progress_stream_id (suggestion #1)
- _resolve_runtime: warn on unrecognized EGG_RUNTIME values so
typos like "k8s" don't silently disable all k8s routes (#5)
- _handle_get_deployment_context: fix stale docstring that
incorrectly described the Docker response shape (#6)
- cluster_info: add nodes_unavailable flag when node-list probe
fails, matching the images_unavailable pattern (#4)
* Add nodes_unavailable to cluster_unreachable path, test, and docs
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1857: dedupe per-agent worktree creation in session_create (#1863)
* Fix #1857: stop session_create from racing create_worktrees on .git/config.lock
The orchestrator's per-agent spawn made two worktree-touching gateway calls
under different container_ids: create_worktrees with "{pipeline_id}-{role}",
then register_session with the k8s job_name. Each ran its own
``git worktree add`` against the same bare repo, so 3 concurrent agents x 2
calls = 6 processes contending for ``.git/config.lock``. One lost on every
phase, taking that role's spawn down with "Failed to create any worktrees".
Adds a worktree_container_id arg through the gateway-client/session_create
boundary. When set, the gateway's session_create looks up the existing
worktree via WorktreeManager.lookup_worktree instead of creating a second
one. The k8s spawner threads agent_worktree_id through, so the second call
becomes a no-op on the bare repo.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: validate worktree_container_id at API boundary
- Add regex check for path traversal and unsafe characters on
worktree_container_id in session_create, returning a clear 400
instead of letting it fall through to a 500 from validate_identifier.
- Add docstring note to lookup_worktree explaining the deliberate
omission of _chown_recursive and _configure_push_upstream.
- Add test coverage for empty-string, >256-char, and unsafe-character
validation branches of worktree_container_id.
* Fix checks: apply automated formatting fixes
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: egg <egg@localhost>
* docs: document egg_health package in STRUCTURE.md and shared/README.md (#1864)
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* Fix #1853: add get_service_logs MCP tool for gateway/orchestrator pods (#1861)
* Fix #1853: add get_service_logs MCP tool for gateway/orchestrator pods
`get_container_logs` only covers agent-sandbox containers, so
gateway-side spawn failures (Connection refused, Remote end closed
connection, push_worktree_branch returned False) left operators with no
in-MCP way to cross-reference the gateway pod's logs — they had to shell
into the cluster with kubectl.
Adds `get_service_logs(service, lines=100, since_seconds=None)` behind
the same `@require_lifecycle_secret` auth as the other deployment tools.
`service` is allowlisted to `gateway`/`orchestrator` so this stays a
diagnostic endpoint and doesn't drift into a generic kubectl-logs proxy
— agent-pod logs already have their own container-scoped tool.
Resolves the service to its Deployment in `egg-system`, reads the
selector, and returns logs keyed by pod so replicas and mid-rollout
reads are legible. A pod that vanishes between list and read is skipped
rather than failing the whole call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix hardcoded port 9848: use GATEWAY_PORT constant
* Address review feedback: per-pod error handling and schema-allowlist sync test
Catch JobOperationError per-pod in get_service_logs so a transient
failure on one replica returns partial results instead of failing the
entire request. Add cross-reference test asserting MCP schema enum and
route _SERVICE_LOG_ALLOWLIST stay in sync.
* Document optional error field in get_service_logs return shape
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1851: gate pipeline submission on gateway readiness (#1859)
* Fix #1851: gate pipeline submission on gateway readiness
Block POST /api/v1/pipelines on a bounded wait_for_healthy() probe so
fresh deploys / pod restarts surface a single 503 "gateway not ready"
error instead of a downstream cascade of per-agent ConnectionRefused
spawn failures (which leaves a half-created branch on remote and forces
a manual cancel + qualifier-bumped resubmit).
Wait window is configurable via EGG_GATEWAY_READY_TIMEOUT_SECONDS
(default 60s, set to 0 to disable the gate for harnesses that wire the
gateway up out of band).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix checks: mock gateway client in tests broken by readiness gate
The gateway readiness gate added in aeccd69 calls wait_for_healthy()
before pipeline creation. Five existing tests in test_pipelines_api.py
did not mock get_gateway_client, so the real health check timed out
and returned 503 instead of the expected status codes.
Add @patch('routes.pipelines.get_gateway_client') to the four
decorator-style tests and EGG_GATEWAY_READY_TIMEOUT_SECONDS=0 to the
env-dict-style test, matching the pattern used by all other passing
pipeline-creation tests in the file.
* Fix tests: configure gateway mock ls_remote_branch to return False
The gateway mock's ls_remote_branch defaulted to a truthy MagicMock,
causing the branch-existence check to trigger an extra get_state_store
call (test 1) and return 409 before reaching the OSError path (test 2).
* Address review: add Retry-After header on 503, clamp negative timeout
- Add Retry-After header to the 503 gateway-not-ready response per
RFC 7231 §6.6.4, set to the configured timeout value so automated
callers can back off appropriately.
- Clamp negative EGG_GATEWAY_READY_TIMEOUT_SECONDS to 0 via max()
so only 0 is the documented disable mechanism.
- Add test for Retry-After header assertion on the 503 path.
- Add test for negative timeout clamping behaviour.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
* Fix #1852: propagate push failure detail to callers (#1862)
* Fix #1852: propagate push failure detail to callers
push_worktree_branch previously returned bool, forcing callers to
surface the opaque string "push_worktree_branch returned False" when a
push failed — operators had to read gateway source to tell a
non-fast-forward from an auth failure from a network error.
The function now returns a PushResult dataclass (truthy-compatible so
existing if-push_ok callers work unchanged) carrying a category and
the raw git stderr. The contract-init push path surfaces that as
"Failed to push contract init to remote: non_fast_forward: <stderr>"
so the failure class is visible without a side-channel investigation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback on PushResult PR
- Remove getattr fallback in pipelines.py — call push_result.describe()
directly since push_worktree_branch always returns PushResult
- Update test_pipeline_fails_when_contract_push_fails to mock with
PushResult instead of bare False, exercising the real code path
- Update test_state_store.py sync mocks to use PushResult for type
accuracy
- Make bare "403" classifier match more specific (" 403") to reduce
false-positive risk
- Use "/" instead of ":" as nesting separator for
reconcile_retry_failed categories to avoid collision with the
category:detail output format in describe()
- Update module docstring in test_reconcile_and_push_pr_branch.py to
reference PushResult instead of bool
* Update test_sync_worktree.py mocks to use PushResult
* Fix flaky retry test: scope asyncio.sleep mock to module level
The test_max_retries_exhausted_raises test failed in CI with
asyncio.sleep being called 180 times instead of the expected 3.
The root cause: @patch('...asyncio.sleep') patches the sleep function
on the global asyncio module, so any unrelated async code running
during the test (leaked tasks, event loop infrastructure) inflates
the mock's call count.
Fix: introduce a module-level _sleep = asyncio.sleep reference in
retry.py and patch that instead. This scopes the mock to only the
retry module's own sleep calls.
* Update remaining bare-bool push mock to PushResult in test_pipeline_failure_path
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
* docs: document gateway readiness gate [doc-updater] (#1867)
* docs: document gateway readiness gate and EGG_GATEWAY_READY_TIMEOUT_SECONDS
* docs: note gateway_error field nullability in 503 example
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1865: stop cleanup_pipeline from deleting sibling pipelines' worktrees (#1868)
* Fix #1865: stop cleanup_pipeline from deleting sibling pipelines' worktrees
The filesystem scan in `cleanup_pipeline` used a naive prefix match
(`entry.name.startswith(f"{pipeline_id}-")`), which collides whenever
one pipeline ID is a prefix of another (e.g. cleanup of `issue-1758`
would match active worktrees belonging to `issue-1758-worktree-fix`,
wiping them mid-phase). The gateway's `list_worktrees_for_pipeline`
had the same bug with a (now-false) comment claiming pipeline IDs were
`issue-{number}`.
Tightened both scans to only match `{pipeline_id}` exactly or
`{pipeline_id}-{role}` where `{role}` has no hyphens — matching the
shape of every `AgentRole` value. The orchestrator checks against
the actual `AgentRole` set; the gateway (which doesn't depend on the
shared enum) uses `re.fullmatch(r"{pipeline_id}-[a-z_]+")`.
Added regression tests in both components that verify cleanup of a
short pipeline leaves a longer sibling pipeline's worktrees
untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: tighten prefix matching and clean up regex
* Fix checks: apply automated formatting fixes
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: egg <egg@localhost>
* docs: update tool counts for get_service_logs [doc-updater] (#1866)
* docs: update tool counts for get_service_logs addition
Update stale "five" counts to "six" in deployment diagnostics guide and
MCP deployment tools reference, following the addition of get_service_logs
in #1853. The reference doc already had the full get_service_logs section;
these were the only remaining stale counts.
* docs: fix missed tool counts and k8s-only vs k8s-specific wording
* docs: fix k8s-specific → deployment tools wording on line 34
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1870: grant orchestrator RBAC for get_service_logs on egg-system (#1871)
* Fix #1870: grant orchestrator RBAC to read its own namespace's pod logs
`get_service_logs` was returning HTTP 500 on every call because the
`egg-orchestrator` ServiceAccount only had RBAC in the `egg-agents`
namespace. Reading the gateway/orchestrator Deployment and its pod
logs from `egg-system` hit a 403 from the apiserver, the Python
kubernetes client raised ApiException, and the route mapped it to a
500 — making #1853's fix effectively a no-op.
Adds a namespace-scoped Role `egg-service-log-reader` in `egg-system`
granting `apps/deployments:get`, `pods:get,list`, and `pods/log:get`,
bound to the orchestrator's ServiceAccount. Kept deliberately narrow
so this RBAC matches the allowlist of services the MCP tool exposes.
Also unwraps `urllib.error.HTTPError` bodies in
`_handle_get_service_logs` so the caller sees the orchestrator's
structured `message` instead of `HTTP Error 500: INTERNAL SERVER
ERROR` — the bug report flagged the empty-looking error as an
obstacle to diagnosing this exact RBAC issue.
Drive-by: enable `--allow-multiple-documents` on the check-yaml
pre-commit hook. It was already failing against the pre-existing
rbac.yaml; adding this file made the miss visible.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Tighten fallback test assertion to distinguish from structured path
* Fix flaky timeout test: make monotonic mock robust to pre-loop calls
The test_timeout_with_unresolved_nacks_returns_failure test was timing
out because its time.monotonic() mock returned 0.0 only for the very
first call. If any code between mock activation and the start_time
capture (e.g. module imports, logging, leaked threads) called
time.monotonic(), the 0.0 was consumed, start_time got 1801.0, and
elapsed was always 0.0 — causing an infinite loop since time.sleep
was also mocked to be instant.
Fix: advance by 2000s on every call so the elapsed delta always
exceeds the 1800s consensus_timeout regardless of how many pre-loop
calls occur.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
* Fix #1869: fail fast when per-agent worktree is missing at spawn time (#1872)
* Fix #1869: fail fast when per-agent worktree is missing at spawn time
Producers in pipeline ``issue-1758-vn`` came up with no worktree and
spent ~6 minutes burning tokens against gateway ``Worktree not found``
errors before a human cancelled — ``spawn_agent_job`` silently
completed whenever ``create_worktrees`` was skipped (empty ``repos``)
or the worktree vanished between creation and Job start, leaving the
container to discover the problem at runtime.
Add two spawn-time guards in ``KubernetesSpawner.spawn_agent_job``:
1. ``_find_missing_worktrees`` — after the ``create_worktrees`` loop
completes, verify each expected per-agent worktree exists on disk.
If any are missing (e.g. a concurrent ``cleanup_pipeline`` raced
in), raise ``KubernetesSpawnError`` with the full path list.
2. Role-aware empty-repos guard — a producer role (any non-reviewer /
non-operator role, defined via ``_ROLES_WITHOUT_WORKTREE``) spawned
with ``repos=[]`` cannot do git, so refuse instead of pretending to
succeed.
Both paths propagate through ``_spawn_agent`` in
``concurrent_executor.py`` and mark the agent as FAILED with the
diagnostic in ``pipeline.error`` — operators see the cause in the
first status poll.
Tests add the two regressions (``test_spawn_producer_without_repos_raises``,
``test_spawn_missing_worktree_on_disk_raises``) plus a positive
reviewer case, and an autouse ``conftest`` fixture stubs both checks
for pre-existing tests that exercised unrelated spawn mechanics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: strengthen worktree guard tests
- Re-enable real _role_needs_worktree in reviewer-without-repos test so
the guard is actually exercised instead of being stubbed out by conftest
- Add test_roles_without_worktree_are_valid to catch typos or stale
entries if AgentRole ever renames values
- Fix _FakeWorktreeResult default path to use repo-only name (repo not
owner/repo) matching actual gateway on-disk structure
* Fix checks: apply automated formatting fixes
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: egg <egg@localhost>
* Fix #1873: retry agents that die to transient API errors at startup (#1875)
* Fix #1873: retry agents that die to transient API errors at startup
Agents whose very-first-turn Anthropic API call fails (socket close,
5xx, network blip) surface as success=False + exit 1 from the Agent
SDK. The consensus wrapper's is_transient_crash() only handled
signal-based exits, so a 5-second first-turn failure stalled the
whole BRC phase and required human intervention.
Add is_startup_failure(): exit code 1 within STARTUP_FAILURE_WINDOW_SECONDS
(default 30s) routes into the existing backoff + MAX_RESTARTS machinery.
Post-work exit 1 still fails fast; non-1 exit codes are unaffected.
Both the initial-run and restart-loop handlers gain the check.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: docs, test dedup, PR body
- Add STARTUP_FAILURE_WINDOW_SECONDS to concurrent-execution.md config table
- Extract _make_mock_orch_no_consensus helper to reduce ~90 lines of
duplicated mock setup across 6 tests
- Remove 'Generated with Claude Code' from PR description
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1874: keep per-agent worktrees out of gateway cleanup sweeps (#1876)
* Fix #1874: keep per-agent worktrees out of gateway cleanup sweeps
The gateway's orphan-worktree cleanup (startup sweep and /worktrees/prune
route) derives its active-container set from session_manager.list_sessions(),
whose container_ids are k8s Job names (e.g. "egg-agent-issue-1758-again-coder").
Per-agent worktree directories on disk are named after the orchestrator's
`agent_worktree_id` ({pipeline_id}-{role}, e.g. "issue-1758-again-coder"),
so the two sets never overlap — every live pipeline's per-agent worktrees
were eligible for deletion as orphans.
This surfaced as #1874 after a fresh gateway deploy: worktrees that
`create_worktrees` had just built were wiped by the background startup
cleanup thread, landing producers in a permanent "Worktree not found"
loop even though #1872's `_find_missing_worktrees` check had passed.
Fix:
- Surface `pipeline_id` and `agent_role` on `SessionManager.list_sessions()`.
- Add `_derive_worktree_anchor_ids(sessions)` in gateway.py and fold its
output into both the startup-cleanup active set and
`_collect_active_container_ids` so `{pipeline_id}` and
`{pipeline_id}-{role}` directories are treated as active whenever the
session for that agent is live.
- Defense in depth: `cleanup_orphaned_worktrees` now also skips any
container_id tracked in `_active_worktrees`, shielding worktrees whose
`register_session` hasn't landed yet from an in-flight sweep.
Tests cover the new list_sessions fields, the derivation helper and its
integration into `_collect_active_container_ids`, and the in-memory
guard in `cleanup_orphaned_worktrees`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: deduplicate session-to-anchor logic, align dry-run with cleanup
- Extract _container_ids_from_sessions() helper so _collect_active_container_ids()
and main() share the same container-id + anchor collection logic.
- Add _active_worktrees guard to list_orphan_worktree_dirs() so the dry-run prune
route output matches what cleanup_orphaned_worktrees would actually skip.
- Fix existing symlink-escape test to initialize _lock and _active_worktrees.
- Add tests for both changes.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* docs+fix: Update agent-recovery docs; fix flaky TestGetStatusWait (#1877)
* docs: document startup failure retry in consensus wrapper
* docs: address review feedback on startup failure docs
- Add catch-all statement for non-signal, non-1 exit codes
- Update 'Restart with Backoff' opening to include startup failures
- Document startup_failure_window_seconds configurability
* Fix flaky test_wait_zero_no_sleep by avoiding global asyncio.sleep patch
The TestGetStatusWait tests patched mcp_server.asyncio.sleep, which
replaced asyncio.sleep globally (since mcp_server.asyncio IS the asyncio
module). During full CI runs, anyio's wait_all_tasks_blocked() polls with
await sleep(0.1), and these background calls were captured by the mock,
causing assert_not_called() to fail with 250 spurious calls.
Fix: introduce a module-level _async_sleep reference in mcp_server.py and
patch that instead. This scopes the mock to _apply_get_status_wait without
affecting the global asyncio.sleep used by anyio internals.
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com>
* Fix #1878 + #1879: honor skip_cleanup for worktrees, retry transient spawn failures at phase level (#1880)
* Fix #1878: honor skip_cleanup for per-agent worktrees on pipeline failure
When a pipeline fails, the outer cleanup block sets skip_cleanup=True and
logs "preserving worktrees for retry", but the safety-net call to
spawner.cleanup_pipeline() runs unconditionally and wipes both the
pipeline-level worktree and every per-agent "{pipeline_id}-{role}"
directory — including any in-progress merge. Give cleanup_pipeline a
preserve_agent_worktrees flag and pass skip_cleanup through to it so the
log line matches reality.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Rename preserve_agent_worktrees to preserve_worktrees
The flag preserves all worktrees (pipeline-level and per-agent), not just
agent worktrees. The shorter name is more accurate per review feedback.
* Fix #1879: retry transient spawn failures at the phase level
Per-role retries (3 attempts, ~7s budget, #1842) aren't long enough to
bridge a gateway cold start (~30s). Once they exhaust, the phase
coordinator applies an all-or-nothing rule: one failed role stops every
survivor and raises SpawnFailureError, throwing away real in-progress
work (observed in issue-1758-more-more on 2026-04-22).
Add a bounded phase-level retry inside _run_concurrent_phase: if any
spawn failure matches a known transient pattern ("connection refused",
"remote end closed", "timed out", "service unavailable", etc.),
respawn just the failed roles via executor.spawn_specific_roles()
with 30s/90s backoff (default 2 attempts). Survivors are left running
during the retry window — BRC can't start without the full cohort
anyway, so there's no correctness risk. Before each retry, clear any
half-created gateway worktree state for the failed roles so
create_worktrees sees a clean slate. If all failures are permanent,
or the retry budget exhausts, fall through to the existing abort path
unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* docs: add phase-level spawn retry config fields [doc-updater] (#1881)
* docs: add phase_spawn_max_retries config fields
Document the two new PipelineConfig fields introduced in #1879 for
phase-level spawn retries alongside the existing per-role retry fields.
Authored-by: egg
* docs: add total attempts clarification for phase_spawn_max_retries
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Raise gateway memory limit to 4Gi as interim fix for OOM crash-loop (#1885) (#1886)
* Raise gateway memory limit to 4Gi (from 1Gi)
Gateway was OOM-killed 4 times in 29 minutes with 3 concurrent pipelines
running. Per-request streaming-capture allocates ~30MB of transient memory
(10MB chunk list + joined string + decoded string + parsed events) in
proxy_anthropic_messages; with 32 Flask threads and ~15 concurrent
Anthropic streams in flight, this easily exceeds the 1Gi cap even without
a leak. See #1885 for follow-up investigation of leak vs. load-pressure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: tighten request ratio, surface GATEWAY_THREADS, add TODO
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1885: parse SSE streams incrementally; add opt-in mem trace sampler (#1887)
* Fix #1885: parse SSE streams incrementally; opt-in tracemalloc sampler
The streaming proxy held up to 10 MB of raw chunks per concurrent Anthropic
request and then allocated another 2–3× of that at completion via
b"".join(chunks).decode("utf-8").split("\n"). Under ~15 concurrent streams
(3 pipelines × 5 agents) that peak easily cleared 1 GiB — the OOM
high-water mark documented in the issue.
- Add _SSEAccumulator that parses chunks as they arrive. It owns only a
small partial-line buffer and the usual parsed state dicts; raw bytes
are never retained. _parse_sse_response is now a thin wrapper over it
for test coverage. proxy_anthropic_messages feeds the accumulator
directly so the high-water mark drops from O(response × 3) to
O(parsed content). The 10 MB cap is kept as a defensive stop-feeding
trigger.
- Add opt-in mem_trace sampler (GATEWAY_MEM_TRACE=1). Logs RSS + top-N
tracemalloc allocation sites every 30s to stdout so the trail
survives pod OOM via `kubectl logs --previous`. We intentionally log
rather than write files: /home/egg/.egg-state is an emptyDir in
k8s/base/gateway-deployment.yaml and would be wiped on pod restart,
defeating the diagnostic purpose.
Tests cover chunk-boundary correctness (including a UTF-8 codepoint split
across chunks, which is the main reason for an IncrementalDecoder) and
the mem_trace opt-in / sample-shape paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: guard duplicate threads, clamp interval, restore try/except
- Add _started module guard to start_if_enabled() so repeated calls don't
spawn duplicate daemon threads (review item 1).
- Clamp GATEWAY_MEM_TRACE_INTERVAL_SECONDS to min 1.0s to prevent tight
sleep(0) loops from zero/negative values (review item 2).
- Wrap accumulator.result() + _capture_streaming_response in try/except in
the generator finally block, restoring fail-safe semantics from the
pre-refactor code (review item 3).
- Tighten test assertion: assert _line_buf == "" directly instead of
sys.getsizeof check (review item 4).
- Add tests for the _started guard and interval clamping.
* Address re-review suggestions: clamp top_n, reset _started in test
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* docs: add mem_trace.py to gateway structure listing (#1891)
Authored-by: egg
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* Fix #1884: auto-prune stale gateway sessions via idle timeout (#1892)
* Fix #1884: auto-prune stale gateway sessions via idle timeout
Adds a background pruner thread in SessionManager that periodically calls
both prune_expired_sessions() and a new prune_idle_sessions() so entries
for dead or cancelled containers no longer accumulate across gateway
restarts. Idle pruning complements TTL pruning: every successful
validate_session() refreshes last_seen, so a stale last_seen reliably
indicates a container that has stopped making requests even when its
24h explicit TTL is still far in the future.
The pruner is started from gateway.main() after startup cleanup, with
interval and idle threshold configurable via
EGG_SESSION_CLEANUP_INTERVAL_MINUTES / EGG_SESSION_IDLE_TIMEOUT_MINUTES
(defaults 15 min / 60 min). The first prune runs after the interval,
giving sessions restored from disk time to re-validate before eviction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: validate env param floors, widen type hints to float, update docstring
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1889 + #1890: bridge contract decisions at phase_gate; harden plan HITL gate (#1896)
* Fix #1889 + #1890: bridge contract decisions at phase_gate; harden plan HITL gate
#1889: When an agent registered questions via egg-contract add-decision /
add-feedback during refine/plan, those entries lived only in the contract
JSON. The orchestrator's decision queue was blind to them, so approving
the phase_gate via the HTTP API/MCP silently discarded them and the
next phase's agents had to guess. Added _queue_and_await_contract_decisions
which, after phase_gate approval, promotes unresolved contract-scoped
decisions/feedback into orchestrator choice/feedback decisions, waits
for each, and syncs resolutions back to the contract.
#1890: Wrapped the call sites for _populate_contract_from_plan and
_sync_pipeline_decisions_to_contract in try/except — an uncaught
escape was enough to skip the HITL gate below, stalling the pipeline
until the overseer intervened. Also made handle_consensus_confirmed_signal
idempotent with respect to the message store: a role that already
emitted a final (or pending_acks) CONFIRMED in the current phase no
longer writes another, so the agent's retry-loop "for i in 1..10; do
egg-orch consensus confirmed; done" stops polluting the bus.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Address review feedback: filter contract decisions by type=hitl, harden feedback persistence
- Add type==hitl filter to pending_decisions so AUTO decisions are never
promoted as human choice decisions (blocking review feedback).
- Mark feedback as submitted even when the resolution JSON doesn't match
the expected {answers: {...}} structure — the human responded and
shouldn't be asked again.
- Add clarifying comments on the 10k message limit tradeoff and
phase-null matching behavior in _existing_confirmed_for_role.
- Add two new tests: test_bridge_skips_auto_decisions and
test_bridge_marks_feedback_submitted_on_unparseable_resolution.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* docs: document server-side contract decision bridge (#1899)
Update docs to reflect the new _queue_and_await_contract_decisions()
function added in ae9535b99. Contract HITL decisions registered via
egg-contract add-decision/add-feedback are now bridged into the
orchestrator decision queue after phase gate approval, ensuring they
are surfaced to humans in all modes (not just prompt-driven CLI).
Authored-by: egg
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
* docs: document gateway session idle timeout config [doc-updater] (#1898)
* docs: document gateway session idle timeout config
* docs: add minimum value comment for EGG_SESSION_IDLE_TIMEOUT_MINUTES
---------
Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1895: right-size gateway, orchestrator, and sandbox pod resources (#1904)
* Fix #1895: right-size gateway, orchestrator, and sandbox pod resources
Tuned requests/limits against a 5-minute trace captured 2026-04-22 against
3 concurrent pipelines (14 sandbox agents), post-#1887.
- Gateway (k8s/base/gateway-deployment.yaml): CPU limit 1 -> 2 cores
(observed spikes to 878m / 88% of 1-core cap during proxy bursts);
mem request 2Gi -> 1Gi, mem limit 4Gi -> 2Gi (post-#1887 steady state
1.37-1.56Gi; #1886's 4Gi band-aid is no longer needed).
- Orchestrator (k8s/base/orchestrator-deployment.yaml): mem request
256Mi -> 512Mi, mem limit 512Mi -> 1Gi (pod sits consistently at
283-301Mi, actively burstable-using above old request).
- Sandbox default (orchestrator/kubernetes_client.py): req 500m/512Mi
-> 250m/384Mi, lim 2c/2Gi -> 1c/1Gi. Observed per-agent max
468m CPU / 407Mi mem leaves 2x+ headroom under new limits. Frees
~3.5 cores and ~1.8Gi of reservation at 14-agent fleet size.
- All three remain Burstable QoS (idle:spike ratio too wide for
Guaranteed on a single-node cluster).
- New docs/deploy/resource-sizing.md records observed-usage table,
QoS rationale, and re-capture recipe.
Related: #1888 (parent right-sizing), #1886 (4Gi interim bump reverted
here), #1887 (SSE refactor that enabled the memory reduction).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Relax gateway and sandbox limits after observing test-phase load
Follow-up to the initial right-sizing on this branch. A second snapshot
~25 minutes into the same pipeline session showed gateway memory climbing
to 2.2Gi (past the proposed 2Gi cap) even as the sandbox fleet shrank
from 14 to 10 agents. The driver is the implement-phase tester running
`make test` — test output routes through the gateway SSE stream and
grows the working set. This is load-driven, not a leak.
Revised net change vs main:
- Gateway: CPU limit 1 -> 2 only. Memory request 2Gi and limit 4Gi
stay put (the #1886 interim bump turns out to be the right steady
state for a single-node cluster hosting test-running sandboxes, not
a band-aid to revert). TODO(#1885) comment dropped since we've now
concluded the review.
- Orchestrator: unchanged from previous commit (mem req 256Mi -> 512Mi,
mem limit 512Mi -> 1Gi).
- Sandbox default: CPU request 500m -> 250m only. CPU limit (2c),
mem request (512Mi), and mem limit (2Gi) all revert to main's values.
The tester at 566Mi memory and occasional 468m CPU spikes sit
comfortably under the 2c/2Gi limits; shrinking them risks OOM/throttling
under `make test`. The 500m -> 250m CPU request drop still frees
3.5 cores of node reservation at current fleet size.
Updated docs/deploy/resource-sizing.md with both snapshots and the
reasoning for keeping gateway memory at 4Gi.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix GATEWAY_MEM_TRACE env var name in resource-sizing doc
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Fix #1905: /sdlc auto-resolves phase_gate follow-ups from context (#1908)
* Initialize SDLC contract for issue #1905
* Fix #1905: /sdlc auto-resolves phase_gate follow-ups from context
Adds a session-scoped `resolved_questions_map` to the /sdlc skill's
Phase 4 (HITL) so draft-embedded answers collected during a `phase_gate`
are reused when the orchestrator subsequently registers the same
questions as standalone `choice`/`feedback` decisions — instead of
re-prompting the user for each one.
Changes (skill-only; orchestrator protocol untouched):
- New `### Resolved Questions Map` subsection defining the map and
lowercase+strip normalization rule.
- Step 5 of the phase_gate handler now populates the map alongside
the existing Resolved Questions display block.
- `choice` handler: Before prompting, looks up the normalized question,
matches stored answers against `decision.options`, and on a compatible
match auto-submits `{"action":"select","selected":...}` with a
user-visible one-line note. Falls through to the prompt on no match
or incompatible option.
- `feedback` handler: Before prompting, prefills answers for matched
questions, prompts only for the unmatched, merges into a single
`{"action":"submit_feedback","answers":{...}}` submission, and prints
a one-line auto-resolution note.
Closes tasks 1-1, 1-2, 1-3 from contract issue-1905.
* docs: document /sdlc skill's resolved_questions_map auto-resolution
Adds documentation for the session-scoped `resolved_questions_map` added
to the `/sdlc` Claude Code skill in #1905 (Phase 4 HITL handler) so
draft-embedded answers collected during a `phase_gate` are reused when
the orchestrator subsequently registers the same questions as standalone
`choice`/`feedback` decisions — avoiding the user being prompted twice.
- `docs/hitl-decisions.md`: new "/sdlc Skill: Auto-Resolving Repeated
Questions" section covering the map definition, the choice and
feedback auto-resolution flows (including the user-visible one-line
note format and the option-compatibility fall-through), the
transparency requirement, and the scope / non-goals (skill-only,
exact-match only, session-scoped, unaffected in egg-sdlc terminal
mode). Also adds `skills/sdlc/SKILL.md` to the Related Files list.
- `docs/guides/sdlc-pipeline.md`: cross-reference paragraph in the HITL
section linking to the new documentation, so readers who land on the
SDLC guide learn that the skill now avoids re-prompting for
questions answered earlier in the same session.
* test: add structural tests for SKILL.md resolved_questions_map changes
Adds tests/test_sdlc_skill_resolved_questions_map.py covering the three
contract tasks for issue #1905:
- task-1-1: new `### Resolved Questions Map` subsection exists above
the phase_gate handler with normalization rule (strip+lowercase) and
session-scoped description; Step 5 of the phase_gate handler populates
the map alongside the Resolved Questions display block.
- task-1-2: `### For choice type decisions:` section begins with a
`Before prompting` paragraph documenting the lookup, option-compatibility
check, `{"action": "select"}` provide_input payload, `Auto-resolved ...
from captured context.` note, and fall-through on no-match.
- task-1-3: `### For feedback type decisions:` section begins with a
`Before prompting` paragraph documenting per-question lookup,
prefilled-vs-unmatched split, all-matched fast path, single merged
`{"action": "submit_feedback"}` provide_input call, and the
Auto-resolved note.
SKILL.md is a markdown behavior spec (interpreted by Claude at runtime),
so there is no executable code to exercise. These tests lock in the
structural shape of the spec — if the refiner or a future edit drops any
of the required elements, the tests fail with a precise pointer.
26 tests, all passing.
* Persist statefiles after implement phase
* Address review feedback: clarify q-index preservation, document change_approach behavior
---------
Co-authored-by: egg-orchestrator <egg@localhost>
Co-authored-by: egg <egg@example.com>
Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
* Replace interactive mode with run_agent_task MCP primitive (#1900)
* Initialize SDLC contract for issue #1762
* refine: draft analysis for #1762 (custom-phase MCP primitive)
Outline replacement of interactive mode with a generic
run_custom_phase MCP primitive. Analysis covers current interactive
mode callgraph (bin/egg → cli.main → run_claude → ensure_compose_services),
roster-subset plumbing that ConcurrentPhaseExecutor already supports,
and the BRC short-circuit path for degenerate rosters via
ApprovalMatrix.is_fully_acked().
Registered 9 multiple-choice decisions and 9 open-ended feedback
questions via egg-contract covering naming, BABYSIT interaction,
removal scope, roster storage location, degenerate-roster policy,
artifact retrieval, role eligibility, rollout sequencing, and
doc/test coverage.
Complexity: high (cross-cutting across orchestrator, sandbox,
bin/, docs/, tests/, and the GHA action).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* refine: apply reviewer_refine non-blocking cleanups
Address non-blocking feedback from reviewer_refine ACK:
- Remove refiner's absolute worktree path leak from bin/egg
description (repo-relative is sufficient).
- Name BOTH compose call sites — runtime.py:686 (run_claude) AND
runtime.py:1060 (exec_in_new_container) — so decision-4 is easier
to answer.
- Correct concurrent_executor.py line refs (:108 for the kwarg
position; class at :102).
- Give run_claude a precise end-line estimate (634-~1050) rather
than "920+".
- Strip template boilerplate from Open Questions section so it
reads as a clean pointer to the registered contract items.
- Reframe decision-9 as confirm-Option-A rather than re-asking the
Options Analysis axis.
- Note Q5 overlap with decision-2 so the human isn't asked the same
thing twice.
No content change to the research, options, recommendation, or
complexity — these are drafting-quality fixes for the plan phase.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after refine phase
* Persist HITL resolution after refine phase gate
* plan(risk): 14-risk assessment for #1762 custom-phase primitive + interactive removal
Overall MEDIUM-HIGH, GO-with-mitigations. High-severity risks: gha_exec
import path break after relocating out of egg_lib/cli.py (R1), Pipeline
persistence for the new active_roles field and CUSTOM enum variant (R2),
reviewer-graph deadlock when a producer is selected without its critical
reviewer (R3), integration_tests compose fixture migration (R4). Surfaces
6 HITL-review questions covering decision-6 gap (producer w/o reviewer),
PipelineMode.BABYSIT audit scope, repo allowlist, draft-file keying, and
HITL gate integration scope.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: architect output for #1762 (custom-phase MCP primitive)
Document the end-to-end architecture for replacing interactive mode
with a generic run_agent_task MCP tool + PipelineMode.CUSTOM +
Pipeline.active_roles:
- MCP tool surface (run_agent_task, per decision-1 HITL resolution)
- Pipeline/PipelineMode model changes (active_roles field, CUSTOM value)
- Route validation (phase-scoped role subset, producer-required,
auto-generated branch, CUSTOM+PR reusing BABYSIT pre-flight)
- Concurrent-executor roster plumbing (honor pipeline.active_roles)
- BRC short-circuit unchanged (approval_matrix.is_fully_acked
already handles empty reviewer lists)
- Full interactive-mode removal (bin/egg, cli.main, run_claude,
run_interactive, compose.py, all ensure_compose_services call
sites; gha_exec relocates to sandbox/egg_lib/gha_exec.py)
- Docs rewrite scope and new agent-task guide
- Test coverage plan (degenerate rosters, CUSTOM+PR, persistence)
- Risks called out for risk_analyst (GHA regression, compose
callers, active_roles read-path completeness, phase-completion
semantics for single-phase pipelines)
- Acceptance-criteria hints for task_planner (16 ACs)
- Open questions flagged for reviewer_plan (has_contract for
CUSTOM+PR, start_phase reuse vs. dedicated custom_phase field,
deprecation stubs vs. deletion for bin/egg-deploy)
Architecture aligns with all 9 HITL-resolved decisions from the
refine phase gate.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: draft implementation plan for #1762 (run_agent_task + interactive-mode removal)
Decompose HITL-resolved refine analysis into 7 phases / 30 tasks covering:
Phase 1 — model + role-validation plumbing (PipelineMode.CUSTOM,
Pipeline.active_roles, validate_roles_for_custom_phase helper)
Phase 2 — route handler + roster threading (create_pipeline mode=custom
branch, _run_concurrent_phase roster override, BABYSIT preflight reuse
for CUSTOM+pr_number per decision-2)
Phase 3 — run_agent_task MCP tool definition + handler
Phase 4 — BABYSIT subsumption (route builds CUSTOM-like internal state
while user-facing babysit_pr tool stays)
Phase 5 — removal of bin/egg, egg_lib/cli.py (gha_exec relocated to
egg_lib/gha_exec.py), compose.py, run_claude, run_interactive, compose
paths in bin/egg-deploy
Phase 6 — integration-test compose fixture migration
Phase 7 — tests + docs rewrite (README, local-quickstart, deployment,
declarative-setup, kubernetes-migration, deploy-migration, sdlc-pipeline,
mcp-deployment-tools, agent-roles, CLAUDE.md, new custom-phase.md)
All 9 HITL decisions from refine gate adopted verbatim. Plan-phase
resolutions documented for 9 feedback items F1-F9. Test strategy covers
degenerate-roster short-circuit, reviewer-only rejection, cross-phase
rejection, BABYSIT parity, and GHA import relocation per risk_analyst R1.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* plan: revise #1762 plan to address reviewer_plan NACK (6 blocking + 7 non-blocking)
Blocking resolutions:
1. TASK-5-7 extended to remove pyproject.toml [project.scripts] egg entry;
TASK-5-9 added to scrub egg --setup references from Makefile error strings
(risk_analyst R6).
2. TASK-2-7 added to audit and broaden all PipelineMode.BABYSIT gates via
new _uses_per_role_staging() helper so CUSTOM+pr_number inherits BABYSIT's
staging-branch derivation (concurrent_executor.py:174), has_contract
semantics (routes/pipelines.py:957), and PR-diff orient prompts (6192, 6357)
(risk_analyst R5).
3. TASK-2-8 added to key CUSTOM drafts by pipeline_id even when issue_number
is set, preventing draft-file collision with concurrent ISSUE-mode pipelines
(risk_analyst R11).
4. TASK-2-1 extended with explicit repo-allowlist acceptance criterion
("repo_not_allowed" HTTP 400); TASK-7-2 adds test_run_agent_task_security.py
(risk_analyst R9).
5. TASK-6-2 added to migrate top-level integration_tests/conftest.py off
compose (the egg_stack session fixture), in addition to the existing
TASK-6-1 for local_pipeline/conftest.py (risk_analyst R4).
6. TASK-2-9 added to guard phase-advance sites (pipelines.py:10591-10594,
:10957-10958) so CUSTOM pipelines terminate as COMPLETE after one phase
instead of auto-advancing into plan/implement.
Non-blocking resolutions:
- Added "Dependency Ordering" section with phase graph.
- Added "Risk Mitigation Map" table mapping each risk_analyst risk to
mitigating tasks.
- F1 revised from "out-of-scope for v1" to "parity with ISSUE mode"
(architect q1_hitl_scope, risk_analyst R14); TASK-2-1 acceptance
confirms config.hitl_gates passthrough.
- TASK-3-1/3-2 add "qualifier" schema field and use it in pipeline_id
composition (submit_task-compatible) to avoid collisions for repeat
CUSTOM runs on same issue/PR.
- TASK-2-4 acceptance broadened to exercise active_roles=["coder"] alone
(R3 producer-without-reviewer case) and assert CONSENSUS_REACHED on
first propose via ApprovalMatrix.is_fully_acked() empty-reviewer
short-circuit.
- TASK-5-8 ambiguity resolved: keep init, stub compose subs with exit 2.
- TASK-5-4 reasoning clarified: runtime.py:686 disappears transitively
via run_claude deletion; :1060 is a surgical edit to surviving
exec_in_new_container.
- TASK-5-3 grep acceptance now includes --include='*.py'.
Plan now has 7 phases and 38 tasks (up from 33); yaml-tasks appendix
validated — no pr_plan key, pr.description/test_plan/manual_steps
populated.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* Persist statefiles after plan phase
* Persist HITL resolution after plan phase gate
* feat(models,roles): add PipelineMode.CUSTOM + active_roles + validator
Phase 1 of #1762 — the data-layer plumbing for run_agent_task:
- orchestrator/models.py: add PipelineMode.CUSTOM enum value and a new
optional Pipeline.active_roles: list[str] | None field with a
field_validator that rejects empty lists, unknown AgentRole values,
and reviewer-only rosters (those deadlock BRC).
- orchestrator/state_store.py: thread an optional active_roles kwarg
through create_pipeline() so callers can persist the resolved roster.
- shared/egg_contracts/agent_roles.py: add validate_roles_for_custom_phase
helper that validates a user-supplied role subset against a phase's
producers + reviewers (after repo / has_contract filtering). Returns
(resolved_roles, None) on success or (None, error_reason) on failure,
with reasons aligned to the route-level 400 responses planned for
Phase 2.
Backward-compatible: active_roles defaults to None, so existing
pipeline JSON deserialises unchanged. All existing tests should pass.
Refs: TASK-1-1, TASK-1-2, TASK-1-3, TASK-1-4
* docs: add run_agent_task (custom-phase) guide
Phase 7 docs landing for #1762 — new tutorial for the run_agent_task
MCP primitive that replaces interactive mode:
- docs/guides/custom-phase.md: new guide covering input schema, role
selection rules, BRC short-circuit for degenerate rosters,
common invocation patterns (research-only refiner, single-coder
drive-by, coder+reviewer, PR-targeted via BABYSIT subsumption,
pre-populated analysis/plan), error responses, artifact retrieval
via git show, and the relationship to ISSUE and BABYSIT modes.
- docs/index.md: add the guide to the Guides table and to the
Task-Specific Guides lookup table so callers looking for
"one-off single-phase work" land here.
Mirrors the Phase 1 data-layer plumbing (PipelineMode.CUSTOM,
Pipeline.active_roles, validate_roles_for_custom_phase) that landed in
b18c645b1. Follow-up commits will remove interactive-mode references
from the other F9-listed docs as the coder's subtractive phases
(Phase 5 onward) land.
Refs: TASK-7-8, F9
* test: add Phase 1 tests for #1762 (PipelineMode.CUSTOM + active_roles + validator)
Cover the data-layer plumbing landed in coder commit b18c645b1:
- shared/tests/test_validate_roles_for_custom_phase.py (41 tests) —
exhaustive coverage of the new validate_roles_for_custom_phase()
helper: default roster fallback (None / []), invalid_phase,
cross_phase_role (overseer/autofixer/conflict_resolver/inspector),
reviewer_only_roster (BRC deadlock guard), invalid_roles (unknown
value, cross-phase reviewer/producer, egg-only reviewer on non-egg
repo), reviewer_contract_without_artifact, canonical ordering,
deduplication, case sensitivity, whitespace handling.
- orchestrator/tests/test_pipeline_custom_mode.py (21 tests) —
PipelineMode.CUSTOM enum value, str-enum round-trip; Pipeline
.active_roles field default=None, validator rejecting empty list /
unknown roles / reviewer-only rosters; legacy pipeline JSON
deserialises with default None (backward compat guarantee);
schema compatibility (field not required, accepts null).
- orchestrator/tests/test_state_store_active_roles.py (8 tests) —
StateStore.create_pipeline(active_roles=...) kwarg optional
(backward compat), kwarg is on returned pipeline, persists and
round-trips through save/load, ValidationError surfaces correctly
for invalid rosters.
All 70 new tests pass. Refs: TASK-1-1, TASK-1-2, TASK-1-3, TASK-1-4.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* feat(routes): add run_agent_task CUSTOM branch to create_pipeline
Phase 2 of #1762 — the route handler and roster threading for the new
run_agent_task MCP primitive:
- orchestrator/routes/pipelines.py: extend create_pipeline() to accept
mode=custom with a required `phase` and optional `roles` list.
Validate phase membership, call validate_roles_for_custom_phase, and
return structured HTTP 400 with a details.reason ("missing_phase",
"invalid_phase", "invalid_roles", "reviewer_only_roster",
"cross_phase_role", "reviewer_contract_without_artifact",
"repo_not_allowed"). Auto-generate branch 'egg/custom-<pipeline_id>'
when no branch is passed AND no PR is targeted; otherwise inherit
the PR head branch. Reuse the BABYSIT PR preflight unchanged for
CUSTOM+pr_number.
- Introduce _uses_per_role_staging() helper so CUSTOM+PR inherits
BABYSIT's per-role staging-branch derivation, has_contract=False,
and PR-diff-aware orient prompts.
- Thread pipeline.mode into _pipeline_identifier / _get_draft_path so
CUSTOM pipelines always key drafts by pipeline_id (avoids collision
with a concurrent ISSUE-mode pipeline on the same issue_number).
- _run_concurrent_phase now reads pipeline.active_roles when set and
builds the roster from it instead of get_roles_for_phase; the
existing review-graph filter at lines 7263-7270 already prunes edges
to the active set.
- Repo allowlist check via config.repo_config.is_writable/readable_repo
(risk_analyst R9). Rejects shell-metacharacter repos with a 400
and reason "repo_not_allowed".
- has_contract logic extended: CUSTOM without PR sets has_contract=True
when analysis/plan is passed inline OR an ISSUE contract file exists
for the same issue_number.
- Phase-advance guard: CUSTOM-mode pipelines mark COMPLETE after their
single phase reaches CONSENSUS_REACHED (no auto-advance).
- orchestrator/concurrent_executor.py: ConcurrentPhaseExecutor now
documents that `roles=` is driven by Pipeline.active_roles for CUSTOM
mode. get_worktree_branch extended to treat CUSTOM+pr_number the
same as BABYSIT (per-role staging-branch egg/babysit-pr/<pr>/<sha>/<role>).
Refs: TASK-2-1..TASK-2-9, TASK-4-1
* feat(mcp): add run_agent_task MCP tool + handler
Phase 3 of #1762 — the user-facing MCP primitive that lets hosts spawn
a CUSTOM-mode pipeline for one phase with a chosen role subset.
- orchestrator/mcp_tools.py: add run_agent_task to PIPELINE_TOOLS with
inputSchema for phase (refine|plan|implement), roles, repo,
description, branch, base_branch, pr_number, issue_number, analysis,
plan, qualifier, config. Only phase/repo/description are required.
- _handle_run_agent_task() forwards to POST /api/v1/pipelines with
mode=custom. Pipeline-ID derivation matches the plan:
issue + qualifier → issue-<N>-<qualifier>
issue only → issue-<N>-custom
pr + qualifier → pr-<N>-<qualifier…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs: Update deployment guide for gateway session idle pruning
Update documentation to reflect changes from 4548f1d:
EGG_SESSION_IDLE_TIMEOUT_MINUTESandEGG_SESSION_CLEANUP_INTERVAL_MINUTES— so operators can adjust the thresholds if legitimate long-running agents need longer sessions.Triggered by: #1892
Test Plan
Authored-by: egg