Green gate: distinguish infra-induced check failures from genuine reds

[jwbron]

Closes #3417.

What

The green gate's fail-open guarantee previously stopped at check execution: an infra fault inside a check (gateway hiccup on a git call, mid-run session-token expiry, OOM-killed worker, disk pressure) exits the check non-zero and surfaced as a definitive red that on mode blocks on. This PR classifies a narrow set of infra signatures so those reds fail open instead, enabled by default.

How

Runner-side tagging (the _RUNNER_PROGRAM executed in the check-runner pod): each red check gets an infra field in the verdict, set when either

• the check's full combined output contains one of the exact strings in _INFRA_OUTPUT_SIGNATURES, or
• the check process died by SIGKILL (rc -9/137), the OOM-killer shape; no test runner signals failure via SIGKILL.

Tagging runs runner-side deliberately: only the runner sees the full output. The verdict carries a truncated 4000-char tail, and an early gateway error (e.g. the test selector's first git call) can scroll out of it; there's a test pinning that case. The signature list is passed via env from the orchestrator, so it stays a single orchestrator-owned constant.

Orchestrator-side decision (run_slice_green_gate): a red verdict where every failed check is infra-tagged fails open with a loud warning. When genuine and infra-tagged reds mix, the gate blocks on the genuine reds only, and the infra-tagged check's name/output stay out of the cascade-routed failure message so nobody chases an infra ghost. A verdict whose checks carry no infra key at all (an in-flight pre-rollout runner) is treated as genuinely red; absence of the tag never fails open.

The signature allowlist

Per the issue's security caveat, the list is a handful of exact strings emitted only by egg's own plumbing, never fuzzy patterns:

Signature	Source
ERROR: GATEWAY_URL environment variable is not set.	sandbox/scripts/git: env not wired into the runner pod
GATEWAY SIDECAR NOT AVAILABLE	sandbox/scripts/git: wrapper's gateway health probe failed
ERROR: EGG_SESSION_TOKEN not set. Session required for gateway access	sandbox/scripts/git: session token missing
Authentication failed - check session token	sandbox/scripts/git: HTTP 401, mid-run token expiry/revocation
No space left on device	kernel ENOSPC strerror: disk pressure is infrastructure either way

Known residual risk (called out in the module docstring and the constant's comment): the signatures are substring-matched against untrusted check output, so a check that prints one while genuinely failing fails itself open. That is inherent to any output-based classification; the mitigation is keeping the allowlist to exact egg-plumbing strings and never adding fuzzy patterns like Killed or connection refused.

Config

Env var	Default	Meaning
EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN	on	off/0/false/no restores strict every-red-blocks behavior; any other value resolves to on

Default-on per discussion: the whole point of #3417 is that on mode should not block slices for infrastructure that is not their fault, and the classification only ever narrows what blocks; operators who want the strict gate opt out explicitly.

Testing

21 new tests in orchestrator/tests/test_slice_green_gate.py (83 total, all green):

• Runner program executed for real in a subprocess: signature red tagged, genuine red untagged, green check never tagged even with signature-shaped output, SIGKILL (kill -9 $$) tagged, and signature-scrolled-out-of-tail still detected.
• Gate wiring: all-infra reds fail open (with full job/session/worktree cleanup), mixed reds block on genuine reds only (infra name/output kept out of the failure message), kill switch off blocks on infra reds, tag-less pre-rollout verdicts still block.
• Switch resolution (default on, exact disabled values, typo degrades to on) and manifest env carrying the signature list.

Docs updated: docs/architecture/slice-dag.md env table + gate description, docs/development/STRUCTURE.md.

[egg-reviewer]

No agent-mode design concerns.

This PR is orchestrator-internal infrastructure — the green gate's check-runner classifies infra-induced red checks (SIGKILL/OOM shape, exact egg-plumbing output signatures) so they fail open instead of blocking a slice. There's no agent-mode surface here: no LLM invocation or prompt construction, no model identifiers, no Anthropic API calls, and no agent-output post-processing. The infra verdict field is machine-consumed by run_slice_green_gate, a genuine in-code downstream parser, not human-facing output. The infra-signature allowlist is enforced in code rather than via prompt instructions, which is the right place for that boundary.

— Authored by egg

[egg-reviewer]

Green gate infra-red fail-open (#3417) — review

Thorough, well-documented change. The core state machine is correct: runner-side classify_infra tags only red checks (green never tagged, verified by test_green_check_never_tagged_even_with_signature_output), classification runs over full output not the truncated tail (test_signature_scrolled_out_of_tail_is_still_detected), the orchestrator fails open only when every red is infra-tagged, blocks on genuine reds in mixed verdicts while keeping infra names/output out of the cascade message, and tag-absence (pre-rollout runner) is treated as genuinely red. I verified all five entries in _INFRA_OUTPUT_SIGNATURES are exact substrings of what sandbox/scripts/git emits (GATEWAY_URL … is not set., GATEWAY SIDECAR NOT AVAILABLE, EGG_SESSION_TOKEN not set …, Authentication failed - check session token), all written to stderr which the runner captures via stderr=subprocess.STDOUT. Cleanup on the fail-open return is covered. No hard logic bug found.

One concern rises to blocking, plus two advisory notes.

Blocking — genuine reds are concretely maskable for egg's own green-gate slices

The docstring acknowledges the generic residual risk ("a check that prints a signature while genuinely failing fails itself open"). What the PR does not account for is that this PR puts the exact signature strings into egg's own test corpus, making the failure reachable in egg's normal path rather than hypothetical:

$ grep -rln "GATEWAY SIDECAR NOT AVAILABLE\|No space left on device\|Authentication failed - check session token" --include="*.py" orchestrator/
orchestrator/slice_green_gate.py
orchestrator/tests/test_slice_green_gate.py

The green gate runs egg's configured make test (changeset-narrowed) at the slice tip. A slice touching green-gate code selects test_slice_green_gate.py. If any test there genuinely regresses, pytest's assertion introspection prints the compared values — e.g. assert None == 'GATEWAY SIDECAR NOT AVAILABLE', or the fixture literal "output_tail": "GATEWAY SIDECAR NOT AVAILABLE" in the shown frame. That substring lands in the test check's combined output → classify_infra tags it infra → if test is the only red check (lint green, etc.), not genuine_failed is true and the gate fails open, masking the real regression — including a regression in the infra-tagging logic itself (self-masking: the break that produces a wrong infra value is hidden by the same mechanism). This is precisely the failure the green gate exists to catch (a red slipping through as green), and it fires on the code path this very PR modifies.

This fits the "masking a genuine failure signal" bar rather than a mere edge case. It is partly inherent to output-based classification, but it is not fully inherent — the four git-wrapper signatures are the fragile ones (a substring a test can legitimately print), while the SIGKILL and ENOSPC arms are robust. Please either:

• narrow the git-wrapper signatures so a bare test literal can't trigger them (e.g. require the signature to be paired with the git wrapper's early non-zero exit / require it near end-of-output, or anchor on a fuller multi-line fragment less likely to appear verbatim in a test), or
• if you judge the risk acceptable, state explicitly in the PR/docstring why default-on is safe given egg's own test suite now emits these strings, so the merger is making an informed call rather than inheriting a hidden hole.

Non-blocking

1. SIGKILL (-9/137) tagging also fails open on a genuine OOM caused by the slice's own code. A memory-explosion bug introduced by the slice will OOM-kill the test process → rc 137 → tagged infra → fail open. That is a real code regression the gate should block, not infrastructure. The _INFRA_OUTPUT_SIGNATURES string list is scoped tightly; the SIGKILL arm is the broad one. Worth a sentence acknowledging that OOM-from-code-under-test is indistinguishable here and is accepted.

2. Comment accuracy. classify_infra's note says rc 137 covers "OOM kill or pod deadline." The k8s activeDeadlineSeconds kills the runner pod (PID 1 python), not an individual check subprocess, so no verdict is printed and that case fails open via the existing parse_verdict is None path — it never surfaces as a per-check 137. Minor; the comment slightly overstates the pod-deadline coverage of this arm.

— Authored by egg

[james-in-a-box]

Thanks for the thorough review. All three items are addressed in commit 76dd454.

Blocking — genuine reds concretely maskable for egg's own green-gate slices — fixed-in-PR (commit 76dd454)

Agreed — this is exactly the self-masking hole worth closing, and I took the narrowing option rather than the "document the risk" one. Split _INFRA_OUTPUT_SIGNATURES into two match modes:

• _INFRA_LINE_SIGNATURES — the four git-wrapper strings, now matched whole-line (the stripped output line must equal the signature, not merely contain it). The wrapper emits each as a bare echo line, so the real fault still tags; but every way a regressed test_slice_green_gate.py reprints a literal — E assert None == 'GATEWAY SIDECAR NOT AVAILABLE', a source-repr fixture literal, a unified-diff - line — embeds it mid-line behind an E/assert/- /quote prefix, so whole-line matching rejects it. The gate can no longer tag its own red regression (including a break in the tagging logic itself) as infra and fail open.
• _INFRA_SUBSTRING_SIGNATURES — the kernel ENOSPC strerror stays substring-matched, since it surfaces embedded in a larger message ([Errno 28] No space left on device). This matches your own assessment that the four git-wrapper signatures are the fragile ones and ENOSPC is robust.

This keeps the deliberate full-output (not truncated-tail) matching intact — the "scrolled out of tail" test still passes. New test test_signature_printed_midline_is_not_tagged pins the guard; test_indented_signature_line_is_tagged proves the wrapper's leading-whitespace banner line still tags after stripping.

Non-blocking 1 — SIGKILL fails open on a genuine OOM from the slice's own code — fixed-in-PR (commit 76dd454)

Agreed. Documented explicitly: the classify_infra SIGKILL comment and the module docstring now state that an OOM caused by the slice's own memory-explosion bug is indistinguishable here from an infra OOM and is accepted as fail-open — the SIGKILL arm is the broad one, called out as the sole residual hole.

Non-blocking 2 — comment overstates rc 137 pod-deadline coverage — fixed-in-PR (commit 76dd454)

Agreed. Corrected the comment: a pod-deadline (activeDeadlineSeconds) kill takes down the runner (PID 1 python), not a per-check subprocess, so no verdict is printed and that case fails open via the orchestrator's missing-verdict path — it never surfaces as a per-check 137. The SIGKILL arm's note now reads "OOM killer" rather than "OOM kill or pod deadline."

86 green-gate tests pass (was 83; +3); make lint clean on both changed files (the pre-existing mypy errors in shared/egg_agent/client.py are unrelated to this PR).

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-reviewed the delta since fa75f78 (single commit 76dd454, touching only orchestrator/slice_green_gate.py and its tests). The change splits the infra-signature allowlist into whole-line (_INFRA_LINE_SIGNATURES) and substring (_INFRA_SUBSTRING_SIGNATURES) match modes to close the self-masking hole the general review flagged, plus comment/docstring corrections. This is orchestrator-internal check classification with no agent-mode surface: no LLM invocation or prompt construction, no model identifiers, no Anthropic API calls, and no agent-output post-processing. The infra verdict field stays machine-consumed by run_slice_green_gate, and the signature allowlist remains enforced in code rather than via prompt instructions — the correct boundary. Unchanged from my prior approval.

— Authored by egg

[james-in-a-box]

Autofix tracking

{"Test/Unit Tests": 1}

[egg-reviewer]

No agent-mode design concerns.

Re-reviewed the delta since my last approval (76dd454). It's a single commit (3b6c26f) touching only shared/egg_contracts/tests/test_artifact_spec.py: after orchestrator/routes/pipelines.py was decomposed into the routes/pipelines/ package (#3312), TestConsistencyC_PromptDerivesFromSpec was reading the removed monolith path and erroring with FileNotFoundError. The fix repoints the scan at the concatenated _prompt_*.py submodules — the successor to the pre-split prompt-construction code.

This is a mechanical test-path update with no agent-mode surface: no LLM invocation, prompt construction, model identifiers, Anthropic API calls, or agent-output post-processing. If anything it reinforces agent-mode alignment — the invariant it restores forbids raw .egg-state/agent-outputs/… path literals in the prompt builders, keeping agent-facing paths derived-from-spec rather than hardcoded. Unchanged from my prior approval.

— Authored by egg

[egg-reviewer]

Re-review — #3417 green gate infra-red fail-open

Re-reviewed the delta since my prior blocking review (fa75f78): two commits — 76dd454 (the substantive fix) and 3b6c26f (a green-keeping test fix). My previous blocking concern is properly resolved, not superficially patched. Approving with two minor non-blocking notes.

Blocking concern from last review — resolved ✔

The self-masking hole (this PR seeds the exact signature literals into egg's own test_slice_green_gate.py, so a genuine regression there prints one via pytest introspection → tagged infra → the gate fails its own red open) is closed the right way — narrowing, not documenting-and-shipping.

I verified the fix end-to-end:

• _INFRA_LINE_SIGNATURES (the four git-wrapper strings) now match whole-line ({ln.strip() for ln in out.splitlines()} membership). I checked each against sandbox/scripts/git: ERROR: GATEWAY_URL … is not set. (echo … >&2, line 23), GATEWAY SIDECAR NOT AVAILABLE (banner heredoc line 39, leading whitespace that .strip() removes), ERROR: EGG_SESSION_TOKEN not set … (line 130), Authentication failed - check session token (line 191) — all emitted as standalone lines, so the real fault still tags. Every way a regressed test reprints a literal (E assert None == 'GATEWAY SIDECAR NOT AVAILABLE', source-repr, unified-diff -) embeds it mid-line behind a prefix, so whole-line matching rejects it. This is strictly the conservative direction: whole-line is tighter, so the only residual failure mode is a real infra fault being under-tagged (gate blocks) — never a genuine red being masked.
• _INFRA_SUBSTRING_SIGNATURES keeps ENOSPC substring-matched ([Errno 28] No space left on device surfaces mid-message) — matches my prior assessment that this arm is the robust one.
• Tests exercise the production path. _run_runner executes the real _RUNNER_PROGRAM (python3 -c sgg._RUNNER_PROGRAM) with the env built from the real sgg._INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES — no hand-built reimplementation, no self-seeded golden. The three new tests are meaningful: test_signature_printed_midline_is_not_tagged pins the guard, test_indented_signature_line_is_tagged proves the stripped banner line still tags, test_enospc_is_tagged_as_substring_midline pins the substring arm. Existing test_signature_scrolled_out_of_tail_is_still_detected (own-line signature, full-output scan) is unaffected.
• Env-format change is internally consistent — the list→{"line", "substring"} dict is written by _build_runner_job_manifest and read by the embedded _RUNNER_PROGRAM, both from the same slice_green_gate.py, delivered together in one manifest. Missing-env default ("{}") degrades to "tag nothing by signature" = treat as genuine red = safe.

Both non-blocking items also landed: the SIGKILL/OOM-from-own-code caveat is documented as the accepted broad arm, and the rc-137 comment is corrected (pod-deadline kills PID 1, fails open via the missing-verdict path, never a per-check 137).

3b6c26f (artifact-spec test repoint) — correct

pipelines.py was decomposed into routes/pipelines/ (#3312) after my last review, breaking TestConsistencyC_PromptDerivesFromSpec with FileNotFoundError. Repointing it at the package's _prompt_*.py submodules is right. I confirmed all three invariants still hold statically: the 3 agent-outputs specs (architect-output, architect-slices, risk-analyst-output) each have a _resolve_artifact_path("<name>", …) call in _prompt_agent.py (the needle substring-matches through the _-prefixed alias); no banned {_identifier} templated literal exists anywhere in the package; the glob is non-empty.

Non-blocking

1. Stale pipelines.py references in test_artifact_spec.py. The assertion/docstring messages still say "pipelines.py reintroduced raw agent-output path literals" / "call in pipelines.py" (lines ~458, ~483) though the scanned code now lives in _prompt_*.py. Cosmetic — worth a sweep while you're here.
2. Narrowed ratchet scope. The old monolith test scanned the whole file; the new one scans only _prompt_*.py. That aligns with the test's stated intent (agent-facing prompt construction), and today no banned literal lives elsewhere. But if a future agent-facing prompt f-string gets authored outside _prompt_*.py, a raw path literal there wouldn't be caught. Consider a comment on _prompt_builder_files asserting the invariant "all agent-facing path-naming prompt construction lives in _prompt_*.py" so a future author knows not to break it.

— Authored by egg

[james-in-a-box]

Conflict Resolution Summary

Merged latest main into this branch. One file conflicted:

File	Category	Resolution
shared/egg_contracts/tests/test_artifact_spec.py	Semantic (convergent)	Adopted main's version of TestConsistencyC_PromptDerivesFromSpec

Background: Both branches independently updated this test class for the same underlying change — orchestrator/routes/pipelines.py being decomposed into the orchestrator/routes/pipelines/ package (#3312). They diverged on how the invariant scans the decomposed package:

• This PR (3b6c26f) renamed the anchor to PROMPT_BUILDER_DIR, added a _prompt_builder_files() helper, and scanned only the _prompt_*.py submodules.
• main (fe9ea79) kept PIPELINES_PATH and scanned every *.py in the package.

Resolution: Took main's version wholesale and dropped this PR's now-redundant _prompt_builder_files() helper (which would otherwise dangle, referencing the removed PROMPT_BUILDER_DIR). main's *.py glob is the strict superset — it's robust to a resolve_artifact_path("<name>") call moving from _prompt_agent.py into _populate.py/_drafts.py, whereas the narrow _prompt_*.py glob would silently miss it.

Verified functionally equivalent today: every literal resolve_artifact_path("<name>") call the reverse-direction invariant matches currently lives in _prompt_agent.py, so both globs pass. Confirmed against the live package: no banned {_identifier} literals, and all five spec names (analysis-draft-human, architect-output, architect-slices, plan-draft-human, risk-analyst-output) resolve under the *.py glob.

This PR's change to this test was an incidental "fix checks" commit, not part of the green-gate feature, so nothing in the core change (orchestrator/slice_green_gate.py and its tests) was affected by the conflict.

Please review: the choice to drop _prompt_builder_files() in favor of the package-wide *.py scan. Both are correct today; main's is the more durable invariant.

Note (pre-existing, not from this merge): make lint reports 3 mypy errors in shared/egg_agent/client.py. That file is byte-identical to main (untouched by this PR or the merge) — the errors are inherited from the base branch. Ruff, ruff-format, and mypy on the resolved test file all pass.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-reviewed the delta since my last approval (3b6c26f). It's a single merge commit (5127ec2) bringing origin/main into the branch, whose only PR-authored content is:

• The conflict resolution in shared/egg_contracts/tests/test_artifact_spec.py, adopting main's broader PROMPT_BUILDER/package-wide *.py invariant scan over this PR's narrower _prompt_*.py glob and dropping the redundant helper. Relative to main the only remaining change here is a docstring repoint (orchestrator/routes/pipelines.py → the orchestrator/routes/pipelines/ package and its _prompt_*.py submodules) — which also clears the stale-reference cosmetic note from the prior general review.
• Doc/comment touch-ups in docs/architecture/slice-dag.md and docs/development/STRUCTURE.md documenting the new EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN knob and the #3417 infra-signature fail-open.

No agent-mode surface in any of it: no LLM invocation or prompt construction, no model identifiers, no Anthropic API calls, no agent-output post-processing. The green-gate core (slice_green_gate.py) — including the whole-line/substring signature split (_INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES) that closed the self-masking hole — is unchanged from my prior approval, and the infra verdict field remains machine-consumed by run_slice_green_gate with the signature allowlist enforced in code rather than via prompt instructions (the correct boundary). If anything, the test_artifact_spec.py invariant it preserves reinforces agent-mode alignment by keeping agent-facing paths derived-from-spec rather than hardcoded. Unchanged from my prior approval.

— Authored by egg

[egg-reviewer]

Re-review — #3417 green gate infra-red fail-open

Re-reviewed the delta since my prior approval (3b6c26f). It is a single merge commit (5127ec2) bringing origin/main into the branch. No new feature work — this is a merge-and-conflict-resolution delta.

What actually changed since my last review

I isolated the PR-authored delta with git log 3b6c26f..HEAD --not origin/main -p and verified every non-conflict file against main directly:

• scripts/select_tests/_cli.py, _constants.py, _graph.py, tests/tools/test_select_tests_fallbacks.py, tests/tools/test_select_tests_monorepo.py, shared/egg_tool_output/__init__.py — all byte-identical to origin/main (git diff main..HEAD is empty for each). These are #3516's changes arriving via the merge, not this PR's work.
• shared/egg_contracts/tests/test_artifact_spec.py — the only PR-authored change: the conflict resolution.

Green-gate core — unchanged ✔

orchestrator/slice_green_gate.py and test_slice_green_gate.py are untouched by this merge (git diff 3b6c26f..HEAD -- '*green_gate*' is empty). The whole-line/substring signature split (_INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES) that closed the self-masking hole in my prior review is exactly as approved. My prior end-to-end verification of that fix stands.

Conflict resolution in test_artifact_spec.py — correct ✔

Both branches independently repointed TestConsistencyC_PromptDerivesFromSpec after pipelines.py was decomposed into the routes/pipelines/ package (#3312). The merge adopted main's resolution (scan every *.py in the package via PIPELINES_PATH) over this PR's narrower _prompt_*.py glob, and dropped the now-redundant _prompt_builder_files() helper. I verified:

• No dangling references to the removed helpers (grep for _prompt_builder_files / PROMPT_BUILDER_DIR → none).
• The invariant still functions: the *.py glob is a strict superset of _prompt_*.py, and the three agent-output specs (architect-output, architect-slices, risk-analyst-output) each have a literal _resolve_artifact_path("<name>", …) call in _prompt_agent.py that the needle substring-matches through the _-prefixed alias. No banned {_identifier} templated literal exists anywhere in the package.
• The merge result for this file equals main's version except the module docstring, which correctly describes the agent-facing prompt f-strings as living in the _prompt_*.py submodules — accurate, since the literal-name calls are in _prompt_agent.py.

This also resolves my prior non-blocking concern #2 (narrowed ratchet scope): the package-wide *.py glob is the more durable invariant and catches a resolve_artifact_path("<name>") call moving out of _prompt_agent.py.

Non-blocking (unchanged from prior review)

1. Stale pipelines.py references. The assertion/docstring messages in test_artifact_spec.py (lines ~389, ~404, ~455, ~483) still say "pipelines.py reintroduced raw agent-output path literals" / "call in pipelines.py" though the scanned code now lives in the package. This came in verbatim from main and is purely cosmetic — the strings still point a reader at the right conceptual location. Worth a sweep whenever this file is next touched, but not merge-blocking.

Verdict: approve. The delta is a clean, correct merge; the feature is unchanged from my prior approval; the conflict resolution is sound and improves the invariant.

— Authored by egg

[james-in-a-box]

Thanks for the re-reviews. The blocking concern (self-masking hole) and both original non-blocking notes were already resolved in 76dd454, and the ratchet-scope note was resolved by the merge. The one remaining open item — the stale pipelines.py references — is addressed in 9b419b1.

Stale pipelines.py references in test_artifact_spec.py — fixed-in-PR (commit 9b419b1)

Agreed. After pipelines.py was decomposed into the routes/pipelines/ package (#3312), TestConsistencyC_PromptDerivesFromSpec scans the whole package but its prose still said "pipelines.py." Swept every reader-facing reference to point at the package:

• Class docstring: "the agent-facing prompts in the pipelines/ package…" and "…must appear as a resolve_artifact_path(\"<name>\", …) call somewhere in the pipelines/ package."
• Ratchet comment: "…from creeping back into the pipelines/ package…"
• Both assertion messages: "the pipelines/ package reintroduced raw agent-output path literals…" and "…call in the pipelines/ package — drift between spec and prompt rendering…"
• Renamed test_pipelines_py_is_readable → test_pipelines_package_is_readable to match what it now checks (a directory + non-empty *.py glob). The only live reference was the definition itself; the other hits are immutable BRC-history logs.

The one intentional pipelines.py mention that remains is the historical decomposition comment ("pipelines.py was decomposed into the pipelines/ package") — accurate as written.

ruff check / ruff format --check clean; the 3 TestConsistencyC_PromptDerivesFromSpec tests pass.

Narrowed ratchet scope (prior non-blocking #2) — fixed-in-PR (commit 5127ec2)

Already resolved by the merge, as you noted: it adopted main's package-wide *.py glob over this PR's narrower _prompt_*.py glob, which is the more durable invariant and catches a resolve_artifact_path("<name>") call moving out of _prompt_agent.py.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-reviewed the delta since my last approval. The branch was rebased and now carries a single PR-authored commit ahead of main — 9b419b1 "Sweep stale pipelines.py references in test_artifact_spec.py" — touching only shared/egg_contracts/tests/test_artifact_spec.py (15 insertions, 11 deletions). It repoints TestConsistencyC_PromptDerivesFromSpec's docstrings and assertion messages from the removed pipelines.py monolith to the routes/pipelines/ package and renames test_pipelines_py_is_readable → test_pipelines_package_is_readable. This is exactly the cosmetic stale-reference cleanup the prior general review flagged as non-blocking.

No agent-mode surface in the change: no LLM invocation or prompt construction, no model identifiers, no Anthropic API calls, and no agent-output post-processing. If anything it reinforces agent-mode alignment — the invariant it maintains forbids raw .egg-state/agent-outputs/{_identifier}-… path literals in the prompt builders, keeping agent-facing paths derived-from-spec rather than hardcoded.

The green-gate core (slice_green_gate.py) — including the whole-line/substring signature split (_INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES) that closed the self-masking hole, with the infra verdict field machine-consumed by run_slice_green_gate and the signature allowlist enforced in code rather than via prompt instructions — is unchanged from my prior approval. Unchanged verdict.

— Authored by egg

[egg-reviewer]

Re-reviewed the delta since my last review at 5127ec2. The only PR-authored commit ahead is 9b419b1 "Sweep stale pipelines.py references in test_artifact_spec.py", touching only shared/egg_contracts/tests/test_artifact_spec.py (+15 / -11).

This is a pure documentation/naming cleanup with no production-code change and no behavioral change to the test logic. It resolves the one remaining open item from the prior review (stale pipelines.py references after the #3312 decomposition into the routes/pipelines/ package).

Verified:

• PIPELINES_PATH already pointed at the orchestrator/routes/pipelines/ package directory (not in the diff) — the assertions were already scanning the package; only the surrounding prose was stale. The rename test_pipelines_py_is_readable → test_pipelines_package_is_readable correctly aligns the name with what it checks (.is_dir() + non-empty *.py glob).
• The package exists and the old pipelines.py monolith is gone.
• The invariant tests are not vacuous: resolve_artifact_path(...) calls are present across _drafts.py, _populate.py, and _prompt_agent.py, so test_every_agent_output_spec_has_resolve_call and test_no_raw_agent_output_literals_remain exercise real package content.
• The one remaining pipelines.py mention (line 414) is an accurate historical decomposition comment, correct as written.

The earlier blocking concern (self-masking infra-signature hole) was resolved in 76dd454, which predates this review point, and the green-gate core (slice_green_gate.py, the _INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES split, and the machine-consumed infra verdict) is untouched by this delta.

No blocking issues. Approving.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns.

Re-reviewed the delta since my last approval at 9b419b1. The only change is the merge commit 871b562 bringing origin/main into the branch — there are no new PR-authored feature commits. git log 9b419b1..HEAD --not origin/main yields just that merge, and the merge's own conflict resolutions (combined diff) touch only two documentation files (docs/architecture/slice-dag.md, docs/development/STRUCTURE.md) — descriptive prose, no agent-mode surface.

The green-gate core is unchanged from my prior approval (git diff 9b419b1..HEAD -- orchestrator/slice_green_gate.py is empty): the whole-line/substring signature split (_INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES) is still enforced in code rather than via prompt instructions, and the infra verdict remains machine-consumed by run_slice_green_gate. No LLM invocation, prompt construction, model identifiers, Anthropic API calls, or agent-output post-processing in this delta.

Unchanged verdict — approving.

— Authored by egg

[egg-reviewer]

Re-reviewed the delta since my last approval at 9b419b1. The only change is the merge commit 871b562 bringing origin/main into the branch — there are no new PR-authored feature commits. git log 9b419b1..HEAD --not origin/main yields just that merge.

Verified unchanged / correct:

• Green-gate core is untouched — git diff 9b419b1..HEAD -- orchestrator/slice_green_gate.py is empty. The whole-line/substring signature split (_INFRA_LINE_SIGNATURES / _INFRA_SUBSTRING_SIGNATURES) that closed the self-masking hole is still enforced in code, and the infra verdict remains machine-consumed by run_slice_green_gate. Both test files (test_slice_green_gate.py, test_artifact_spec.py) are also byte-identical to my last review.

• docs/architecture/slice-dag.md conflict resolution is correct. The merge had to reconcile this PR's #3417 clause with main's #3541 change to the same sentence. The resolution preserved both: the fail-open clause now reads "fail-open on infra errors, including infra-signature-tagged reds inside check execution, #3417" (PR intent), and the base-resolution clause reads "root → latest completed chain tip, else the pipeline branch (#3541); child → parent's integration branch" (main intent). Neither side's meaning was lost.

• docs/development/STRUCTURE.md — the #3417 line-113 edit was already present and reviewed at 9b419b1; the remaining line differences against my last review point are pure offset shifts from main's additions elsewhere.

The earlier blocking concern (self-masking infra-signature hole) was resolved in 76dd454, which predates this review point. No new agent-facing surface, no code change, no behavioral change in this delta.

No blocking issues. Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

20 previous review(s) hidden.