Skip to content

Green gate Stage A: config-driven format autofix before slice PR open - #3517

Merged
jwbron merged 10 commits into
mainfrom
issue-3409-green-gate-autofix
Jul 25, 2026
Merged

Green gate Stage A: config-driven format autofix before slice PR open#3517
jwbron merged 10 commits into
mainfrom
issue-3409-green-gate-autofix

Conversation

@jwbron

@jwbron jwbron commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Closes #3409. Follow-up to #3398 (Stage B landed in orchestrator/slice_green_gate.py).

What this does

When the green gate finds a configured check red at the slice tip and that check carries an optional fix command in repositories.yaml (e.g. a lint check with fix: make lint-fix):

  1. The check-runner executes the fix inside its worktree, re-runs the check, and reports a fix sub-object in the verdict (fix exit code, re-run result, changed tracked files). The check's ok stays false: the tip as pushed is still red.
  2. If every failed check re-ran green, the orchestrator stages the tracked modifications (git add -u, so untracked check droppings like caches never enter the commit), commits as egg-green-gate (--no-verify, mirroring the egg-salvage system-identity precedent and orchestrator: flip git-route call sites in pipelines.py from agent_role="coder" to "orchestrator" #2919 orchestrator attribution), and pushes to the slice integration branch via the launcher-authed gateway push route (push_worktree_branch). The runner never pushes.
  3. On push success the gate passes and the slice closes. Any autofix failure (nothing to commit, commit error, push rejection) blocks the slice exactly like an unfixed red, with the error appended to the failure message.
  4. Checks without fix, or whose re-run stays red, or a verdict where only some failed checks are fixable, route to the slice team unchanged (Stage B behavior).

Design notes (deviations from the issue sketch, both simplifications)

  • No patch transport. The issue sketch had the runner return a patch that the orchestrator applies. The runner pod mounts the gateway worktree via hostPath, so the fix's modifications are already on disk when the pod exits; the orchestrator stages and commits from that same worktree. The verdict carries the changed-file list (capped at 100 entries) for observability only.
  • No second runner pass. The re-run happens inside the runner against the exact tree that gets committed, with the repo's pinned toolchain, so the committed tip is already validated green.
  • Gateway authorization: the orchestrator push uses the launcher-auth path (/api/v1/git/push with use_launcher_auth=True), which the gateway treats as orchestrator-trusted; the _SLICE_INTEGRATION_BRANCH_RE + synthetic-session exemption the issue mentions is the session-token path and is not needed here.
  • Fork ordering: the push lands before any close side effect, and dependent slices resolve their parent tip via git ls-remote on origin when they start (create_slice_integration_branch), so they fork after the format commit and cannot inherit unformatted code.
  • Rollout: the autofix commits/pushes only in on mode. In log mode the runner still executes the fix (soak signal: the log line carries autofix_ready), but nothing is committed or pushed.

Changes

  • shared/egg_config/validators.py::validate_checks (+ the two import-fallback copies in config/repo_config.py and orchestrator/routes/pipelines/__init__.py): accept and preserve an optional fix key; empty/None fix is dropped; unknown keys still filtered.
  • orchestrator/slice_green_gate.py: runner fix execution + verdict extension; _commit_and_push_autofix; gate wiring + docstrings.
  • config/repositories.yaml.example: document the fix knob. The live repositories.yaml is deployment-side; egg's lint check needs fix: make lint-fix added there for this to take effect.
  • Tests: orchestrator/tests/test_slice_green_gate.py (runner fix flow executed for real in a subprocess with a real git repo; _commit_and_push_autofix against a real repo with a mocked gateway; gate wiring for fixed/unfixed/partial/log-mode paths), tests/egg_config/test_validators.py (new TestValidateChecks).

Testing

  • orchestrator/tests/test_slice_green_gate.py + tests/egg_config/test_validators.py: 134 passed.
  • Pre-commit (ruff, ruff format, whitespace hooks) clean.

When the per-slice green gate (#3398) finds a configured check red at
the slice tip and the check carries an optional fix command in
repositories.yaml (e.g. lint: make lint-fix), the runner executes the
fix in its worktree and re-runs the check. If every failed check re-ran
green, the orchestrator stages the fix from the shared hostPath
worktree, commits it as egg-green-gate, and pushes it to the slice
integration branch through the launcher-authed gateway push route
before any close side effect, then lets the slice close. Dependent
slices fork from the integration branch remote tip when they start, so
they fork after the format commit.

- validate_checks (shared + both fallbacks) accepts an optional fix key
- runner reports a fix sub-object (re-run verdict, changed files) in
  the check verdict; the check ok stays false at the pushed tip
- orchestrator autofix applies only in on mode; log mode logs that a
  fix was available (soak signal) and never commits or pushes
- checks without fix, or whose re-run stays red, block exactly like
  Stage B

Closes #3409

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Green gate Stage A — config-driven format autofix (#3409)

I traced the full path: validate_checks (3 copies) → _gate_checksEGG_GREEN_GATE_CHECKS_RUNNER_PROGRAM fix/re-run → _commit_and_push_autofixpush_worktree_branch, plus the fork-ordering contract in create_slice_integration_branch. The config plumbing, the mode/autofix_ready gating, the partial-fixability bail-out, and the push-before-close ordering are all correct, and the tests exercise the real runner subprocess and real git for the commit path (no self-seeding goldens, no hand-built-fixture bypass). Two issues need addressing before merge.

Blocking

1. Orchestrator commits on an agent-controlled worktree without disabling hooks — diverges from the codebase invariant it claims to follow.

_commit_and_push_autofix._git (orchestrator/slice_green_gate.py:706-722) runs git -C <worktree> add -u and git commit --no-verify ... directly on the host. The worktree tree is agent-produced integration-branch code. Every other place the orchestrator runs git on such a worktree neutralizes hooks with -c core.hooksPath=/dev/null:

  • state_store/_git.py:99["git", "-c", "core.hooksPath=/dev/null", "-C", ...] (this is the "state-store precedent" the docstring at line 688 cites — it disables hooks on every invocation, not via --no-verify)
  • agent_salvage.py:220 — same, with the explicit comment "the orchestrator runs git on agent-controlled worktrees and must never execute their hooks"
  • gateway_client/_push.py:254 — same

--no-verify only suppresses pre-commit and commit-msg; post-commit (and git add's hooks) still fire. If the worktree's git config resolves core.hooksPath into the tree (the exact scenario the established pattern defends against), this is arbitrary code execution on the orchestrator host. The docstring's claim that this "suppresses hooks the same way" as the state-store precedent is inaccurate — the precedent uses core.hooksPath=/dev/null, which this path omits.

Fix: add -c core.hooksPath=/dev/null to the _git helper's base argv, matching state_store._run_git / agent_salvage._run_git.

2. The committed-and-pushed tree is not the tree the runner validated green — the gate's core guarantee ("committed tip is already validated green") can hold a false pass.

Two independent divergences between the tree the runner re-ran green and the tree _commit_and_push_autofix actually pushes:

  • Sequential re-runs validate intermediate trees. In _RUNNER_PROGRAM (lines 288-315) each check is fixed and re-run in sequence: run_cmd(fix_cmd) then run_cmd(check["command"]) immediately. With N fixable checks, check i's re-run happens before check i+1's fix runs. The committed tree has all fixes applied, so every check except the last was validated against a tree that lacks later fixes. A later fix that breaks an earlier check (or breaks an originally-green check that was never re-run at all — greens are validated only pre-fix) yields a pushed tip that is actually red, but autofix_ready reports green.
  • git add -u drops untracked files that were present during the green re-run. If a fix creates a tracked-worthy new file (codegen, a formatter that splits a module), the re-run passed with that file on disk, but git add -u (line 726) stages only modifications to already-tracked files, so the pushed tip omits it and the check is red as pushed.

For egg's own lint-only config this is safe (single check, make lint-fix only rewrites tracked files). But this is generic infrastructure keyed on arbitrary fix: commands across arbitrary repos, and the docstrings/PR description assert the stronger general guarantee. Recommend either: (a) after applying all fixes, do one final full re-run of every check against the exact staged tree in the runner and gate autofix_ready on that, or (b) explicitly constrain and document that fix commands must only modify already-tracked files and that only single-fixable-check verdicts self-heal — and enforce it (e.g. refuse autofix when more than one check needs a fix, or when untracked files changed).

Non-blocking

  • test_no_git_repo_degrades_changed_files_to_none (test_slice_green_gate.py) depends on tmp_path not being nested inside any git repository for git diff to fail. It's robust under pytest's /tmp-based tmp_path, but a comment noting the assumption would prevent a confusing failure if the tmp root ever changes.
  • Minor: the and repo_host_dir guard at line 1020 is dead defense — wt_result.worktrees is already asserted non-empty at 858, so repo_host_dir is always set past that point. Harmless.

The design (no patch transport, hostPath-shared worktree, push before close, launcher-auth push route) is sound and the fork-ordering argument checks out against create_slice_integration_branch's ls-remote parent resolution. Address the two items above and this is good to land.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

james-in-a-box Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor
Autofix tracking
{"Test/Unit Tests": 1, "Lint/Python": 1}

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

james-in-a-box Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review feedback addressed (commit 419322e)

Thanks for the thorough trace. Both blocking items and both non-blocking items are addressed.

⚠️ Push note: the gateway blocks egg from pushing to this PR's branch (issue-3409-green-gate-autofix) because the PR is human-authored, not egg-owned. The fix commit 419322e is pushed to egg/issue-3409-green-gate-autofix-review-fixes on origin. Please merge/cherry-pick that single commit onto the PR branch so CI runs it here.

Blocking

1. Orchestrator commits on an agent worktree without disabling hooksfixed-in-PR (commit 419322e). The _git helper in _commit_and_push_autofix now prepends -c core.hooksPath=/dev/null to every invocation, matching the state-store precedent (StateStore._run_git / agent_salvage._run_git). You're right that --no-verify only suppresses pre-commit/commit-msg and left post-commit (and git add's hooks) able to execute a worktree-resolved core.hooksPath. The docstring is corrected to describe core.hooksPath=/dev/null as the hook-neutralizing mechanism, with --no-verify as an additional layer rather than the claim that it "suppresses hooks the same way."

2. Committed/pushed tree is not the tree the runner validated greenfixed-in-PR (commit 419322e). Both divergences are now closed generically, not just for egg's single-check config:

  • Sequential/greens divergence: after any fix runs, the runner does one final full re-run of every configured check against the tree with all fixes applied (final_verification in the verdict). _autofix_ready gates self-healing on final_verification.all_ok, not on the per-check intermediate re-runs — so a later fix that breaks an earlier check, or an originally-green check the fix regresses, blocks the slice.
  • git add -u dropping untracked files: the runner captures non-ignored untracked paths (git ls-files --others --exclude-standard) before/after the fixes and reports any the fixes created. _autofix_ready refuses when that count is non-zero (a new source file git add -u would drop) — or unknown (a best-effort git failure), which fails safe to a blocked slice.

Net: the gate self-heals only when it can prove the exact git add -u tip is green.

Non-blocking

3. test_no_git_repo_degrades_changed_files_to_none tmp_path assumptionfixed-in-PR (commit 419322e). Added a comment spelling out that pytest's tmp_path must not be nested inside a git repo for the runner's git diff/git ls-files to genuinely fail, and what would flip if that ever changed.

4. Dead and repo_host_dir guarddisagree (harmless defense-in-depth, kept intentionally). You correctly note wt_result.worktrees is asserted non-empty upstream so repo_host_dir is always set, and flagged it as harmless. I've left the guard: it costs nothing and defends the if autofix_ready and mode == "on" block against an empty-string worktree value, keeping the autofix path from ever running against a falsy path. Happy to drop it if you'd prefer.

New tests cover the final-re-run-red, fix-created-untracked, unknown-untracked, and missing-final_verification block paths (TestAutofixReady, TestGreenGateAutofixWiring) plus the runner's final_verification output (TestRunnerFixFlow). make lint is clean; the logic tests pass (the git-dependent runner tests need host git — git init is blocked in this sandbox, matching the noted baseline).

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: Green gate Stage A autofix (#3409) — both blocking issues remain on the PR branch

I re-reviewed the delta since my previous review at 679f0e8. The two blocking issues from that review are still fully present in this PR branch. The fixes exist only on a separate branch that was never merged here.

What actually changed since 679f0e8

The PR branch advanced by exactly two commits, and git diff 679f0e8..HEAD --name-only shows they touch only shared/egg_contracts/tests/test_artifact_spec.py:

  • d019091 — re-point the artifact-spec ratchet at the orchestrator/routes/pipelines/ package after the #3312 split. Correct: PIPELINES_PATH now targets the package dir and pipelines_text reads sorted(glob("*.py")). The package is flat (all _*.py submodules are top-level), so the glob covers every source file. Good.
  • 8031878 — formatting fixup on the same test file. Fine.

orchestrator/slice_green_gate.py is byte-for-byte identical to what I reviewed at 679f0e8. Grepping this PR's full diff confirms none of the promised fixes are present:

  • core.hooksPath0 occurrences
  • final_verification0 occurrences
  • ls-files --others --exclude-standard (untracked detection) → 0 occurrences
  • _git helper still builds cmd = ["git", "-C", worktree_path] with no hook-neutralizing flag.

Why this is blocking

The "Review feedback addressed (commit 419322e)" comment claims both blockers are fixed. That commit does contain the fixes — I fetched it and confirmed core.hooksPath and final_verification are both present on branch egg/issue-3409-green-gate-autofix-review-fixes. But that commit was never cherry-picked onto the PR branch issue-3409-green-gate-autofix. The author's own push note said so explicitly:

⚠️ the gateway blocks egg from pushing to this PR's branch … The fix commit 419322e is pushed to egg/issue-3409-green-gate-autofix-review-fixes. Please merge/cherry-pick that single commit onto the PR branch so CI runs it here.

That cherry-pick has not happened. CI runs against the PR branch HEAD and a merge merges the PR branch — so as it stands, merging #3517 ships both original blocking bugs:

1. Arbitrary code execution on the orchestrator host via worktree-resolved hooks. _commit_and_push_autofix._git (orchestrator/slice_green_gate.py:706-722 region) runs git add -u and git commit --no-verify on an agent-controlled integration-branch worktree with no -c core.hooksPath=/dev/null. --no-verify suppresses only pre-commit/commit-msg; post-commit and git add hooks still fire. If the worktree resolves core.hooksPath into the tree, that is orchestrator-host code execution — the exact scenario state_store/_git.py:99, agent_salvage.py:220, and gateway_client/_push.py:254 all defend against with core.hooksPath=/dev/null.

2. The committed/pushed tip is not proven green. Sequential per-check re-runs validate intermediate trees (check i's re-run precedes check i+1's fix), and git add -u drops any fix-created untracked file that the green re-run relied on. The gate's stated "committed tip is already validated green" guarantee can hold a false pass. (419322e closes this with a final full re-run + untracked-file detection gated by _autofix_ready — but again, not on this branch.)

Required to land

Cherry-pick / merge 419322e (branch egg/issue-3409-green-gate-autofix-review-fixes) onto issue-3409-green-gate-autofix and let CI run it here. Once the PR HEAD actually contains the hook-disabling _git change and the final_verification + untracked-file gating, I'll re-review that delta. The two new CI-fix commits on their own are fine — the blocker is that the substantive review fixes never reached this branch.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

james-in-a-box Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Conflict Resolution Summary

Merged latest main into this branch. One file conflicted:

File Category Resolution
shared/egg_contracts/tests/test_artifact_spec.py Formatting / wording Both sides described the same #3312 decomposition of pipelines.py into the pipelines/ package. The executable logic was identical (glob("*.py") concatenation + the same asserts); only comment prose and one loop variable name (p vs path) differed. Kept a single coherent version per hunk — no behavioral change.

Details of the three hunks (all inside TestConsistencyC_PromptDerivesFromSpec):

  • Class-level comment explaining the ratchet now reads the whole package — kept the PR's more detailed wording.
  • pipelines_text fixture — identical return; kept main's version (adds an explanatory comment, uses path as the loop var).
  • test_pipelines_py_is_readable assert message — kept "no Python sources under …" wording.

Verification:

  • make lint — passes (exit 0).
  • PYTHONPATH=shared:gateway:orchestrator pytest shared/egg_contracts/tests/test_artifact_spec.py — 50 passed, including the 3 tests in the resolved class.
  • Did not run make build (Docker image build, unrelated to this Python test-file change) or the full make test suite per the workflow guidance; CI will run the full check suite.

Please review: The conflict was purely cosmetic (comments + a variable rename over identical logic), so no runtime behavior is affected. No concerns.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: Green gate Stage A autofix (#3409) — both blocking issues STILL on the PR branch after the main-merge

I re-reviewed the delta since 803187831. Since that review the branch advanced by exactly one commit — the merge 08b8f01 ("Merge origin/main into issue-3409-green-gate-autofix: resolve conflicts in test_artifact_spec.py"). That merge pulls in unrelated main work (the #3312 decomposition, the LiteLLM architecture files, .dockerignore) and a cosmetic conflict resolution in one test file. It does not touch orchestrator/slice_green_gate.py, so both blocking issues from my prior review are still fully present on the PR HEAD.

Verification that the fixes are absent

The fix commit 419322e exists only on egg/issue-3409-green-gate-autofix-review-fixes — I confirmed it is not on origin/main and was never cherry-picked onto this branch:

  • git show origin/main:orchestrator/slice_green_gate.py | grep 'core.hooksPath\|final_verification\|exclude-standard'empty (fix not on main)
  • Same grep against the PR HEAD tree → empty (fix not on the branch)
  • git ls-remote origin egg/issue-3409-green-gate-autofix-review-fixes → still 419322e…, unmerged.

So merging main could not and did not deliver the fixes. Merging #3517 as-is ships both original bugs.

Blocking issue 1 — arbitrary code execution on the orchestrator host via worktree-resolved hooks (unchanged)

_commit_and_push_autofix._git (orchestrator/slice_green_gate.py:706-722) still builds cmd = ["git", "-C", worktree_path] with no -c core.hooksPath=/dev/null, then runs git add -u (:726) and git commit --no-verify (:753) on an agent-controlled integration-branch worktree. --no-verify suppresses only pre-commit/commit-msg; post-commit and git add hooks still fire. If the worktree tree resolves core.hooksPath, that is orchestrator-host code execution — the exact scenario state_store/_git.py, agent_salvage.py, and gateway_client/_push.py all defend against with core.hooksPath=/dev/null.

The docstring at :688-689 still asserts --no-verify "suppresses hooks the same way" as the state-store precedent — which is false and was flagged last time. The state-store precedent uses core.hooksPath=/dev/null, not --no-verify.

Fix: prepend -c core.hooksPath=/dev/null to every _git invocation (matching 419322e), and correct the docstring.

Blocking issue 2 — the committed/pushed tip is not proven green (unchanged)

The runner (embedded script, :286-317) re-runs only the check it just fixed (rerun_rc, rerun_out = run_cmd(check["command"]) at :303) and emits EGG_GREEN_GATE_VERDICT:{"checks": results} (:317) with no final full re-run. The gate autofix_ready = all(isinstance(c.get("fix"), dict) and c["fix"].get("check_ok_after_fix") for c in failed) (:1006-1008) trusts those per-check intermediate re-runs. Two divergences remain:

  • Sequential/greens divergence: a fix for check N can break an earlier-passing or earlier-fixed check M — check M is never re-run against the final tree, so the committed tip can be red while the gate reports it green. There is no final_verification field anywhere in the file.
  • git add -u drops fix-created untracked files: git add -u (:726) stages only tracked modifications. A fix that creates a new source file the green re-run relied on is silently dropped from the commit, so the pushed tip differs from the validated tree. There is no git ls-files --others --exclude-standard untracked detection.

Fix: after all fixes run, do one final full re-run of every configured check against the final tree and gate autofix_ready on that (final_verification.all_ok); and detect fix-created untracked paths, refusing to self-heal when any exist or the count is unknown (matching 419322e).

The one new change since my last review is fine

The test_artifact_spec.py conflict resolution (08b8f01) is correct and cosmetic: no leftover conflict markers; PIPELINES_PATH points at the decomposed orchestrator/routes/pipelines/ package (:417); pipelines_text reads sorted(PIPELINES_PATH.glob("*.py")) (:436), which covers the flat package's submodules; test_pipelines_py_is_readable asserts the dir exists and is non-empty (:438-444). No behavioral change. No concerns there.

Required to land

Cherry-pick / merge 419322e onto issue-3409-green-gate-autofix (or reland its two fixes directly) and let CI run it here. The main-merge did not address the blockers — the branch HEAD still needs the hook-neutralizing _git change and the final_verification + untracked-file gating before this can land. I'll re-review that delta once it's on the branch.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

Address review feedback on the #3409 Stage A autofix path:

- Disable git hooks on the orchestrator's autofix commit. The _git
  helper in _commit_and_push_autofix now runs every git invocation with
  -c core.hooksPath=/dev/null, matching the state-store precedent
  (StateStore._run_git / agent_salvage._run_git). --no-verify alone left
  post-commit and git add hooks free to execute agent-controlled
  worktree hooks on the orchestrator host.

- Make 'the committed tip is validated green' true in general. The
  runner now does one final full re-run of every configured check
  against the tree with all fixes applied, and reports any fix-created
  untracked files (git ls-files --others --exclude-standard). The
  orchestrator's new _autofix_ready gate self-heals only when that final
  re-run is green AND no new untracked files were created (git add -u
  would drop them). Unknown/degraded verdicts fail safe to a blocked
  slice.

Also documents the tmp_path 'not nested in a git repo' assumption in
test_no_git_repo_degrades_changed_files_to_none.
@james-in-a-box

james-in-a-box Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review feedback addressed — fix staged, blocked on a push-policy gate only a human can clear

Thanks for the persistent, precise re-reviews. Every actionable item is now fixed and verified in a single commit that fast-forwards the PR branch cleanly: 980d1fb1, on egg/issue-3409-green-gate-autofix-review-fixes-v2. Its parent is this PR's current HEAD 08b8f01, so no cherry-pick-across-the-main-merge is needed anymore — a plain fast-forward lands it.

⚠️ Why it isn't already on the PR branch (the recurring blocker)

The gateway refuses egg's push to issue-3409-green-gate-autofix:

ERROR: Push denied: Branch 'issue-3409-green-gate-autofix' is not owned by james-in-a-box or an authorized user. Use a bot-prefixed branch (egg-* or egg/*).

This PR is human-authored (jwbron), so egg cannot push to its branch and CI (which runs against the PR HEAD) never sees the fix. This is not a code problem I can resolve from inside the sandbox — it needs one human action:

# fast-forward (parent of 980d1fb1 IS the current PR HEAD 08b8f01):
git fetch origin
git push origin origin/egg/issue-3409-green-gate-autofix-review-fixes-v2:issue-3409-green-gate-autofix
# or equivalently, cherry-pick the single commit:
git checkout issue-3409-green-gate-autofix && git cherry-pick 980d1fb1 && git push

Disposition of each review item

Blocking 1 — orchestrator commits on an agent worktree without disabling hooksfixed (commit 980d1fb1). The _git helper in _commit_and_push_autofix now prepends -c core.hooksPath=/dev/null to every invocation (orchestrator/slice_green_gate.py:839), matching the state_store/_git.py / agent_salvage.py / gateway_client/_push.py precedent. The docstring is corrected to name core.hooksPath=/dev/null as the hook-neutralizing mechanism, with --no-verify as an additional layer — not the false claim that --no-verify "suppresses hooks the same way."

Blocking 2 — committed/pushed tip is not proven greenfixed (commit 980d1fb1). Both divergences are closed generically:

  • The runner now does one final full re-run of every configured check against the tree with all fixes applied and emits final_verification (orchestrator/slice_green_gate.py:375). _autofix_ready gates self-healing on final_verification.all_ok (:767), not the per-check intermediate re-runs — so a later fix that regresses an earlier check blocks the slice.
  • The runner captures non-ignored untracked paths via git ls-files --others --exclude-standard (:286,:296) before/after fixes and reports any the fixes created; _autofix_ready refuses to self-heal when that count is non-zero or unknown (fails safe to a blocked slice), so a fix-created file that git add -u would drop can't produce a false green.

Non-blocking 3 — test_no_git_repo_degrades_changed_files_to_none tmp_path assumptionfixed (commit 980d1fb1). Added a comment (orchestrator/tests/test_slice_green_gate.py:438-443) spelling out that pytest's tmp_path must not be nested inside a git repo for the runner's git diff/git ls-files to genuinely fail, and what would flip if the tmp root ever moved.

Non-blocking 4 — dead and repo_host_dir guarddisagree (harmless defense-in-depth, kept intentionally). You correctly noted wt_result.worktrees is asserted non-empty upstream so repo_host_dir is always set, and flagged it as harmless. Kept: it costs nothing and defends the autofix block against a falsy worktree path. Happy to drop it if you'd prefer.

make lint is clean for the changed files (ruff check + format pass; the 3 mypy errors reported by make lint are pre-existing in shared/egg_agent/client.py, untouched by this change and present on the base tree). New tests cover the final-re-run-red, fix-created-untracked, unknown-untracked, and missing-final_verification block paths plus the runner's final_verification output.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@jwbron

jwbron commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Refreshed against main and folded in the stranded review-fix commit.

  • Cherry-picked 980d1fb15 ("Green gate autofix: disable hooks + gate on final full re-run"), which was sitting on egg/issue-3409-green-gate-autofix-review-fixes-v2 and had never been pointed at this PR. It disables git hooks on the orchestrator's autofix commit (-c core.hooksPath=/dev/null; --no-verify alone left post-commit and git add hooks free to run agent-controlled worktree hooks on the orchestrator host) and makes "the committed tip is validated green" true in general by adding a final full re-run plus an untracked-files check.
  • Merged origin/main (the branch was 419 commits behind; no conflicts).
  • orchestrator/tests/test_slice_green_gate.py + tests/egg_config/test_validators.py: 147 passed. Ruff clean.

Sequencing note: this is throughput/toil reduction, not a correctness prerequisite for the gate. It also widens the trust surface (the orchestrator starts committing and pushing to the slice integration branch as egg-green-gate), and the autofix only commits in on mode. Suggest landing it after the gate has actually soaked in log mode.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: Green gate Stage A autofix (#3409) — both blocking issues fixed. Approving.

I re-reviewed the delta since my last review at 08b8f01. The PR advanced by exactly one PR-authored commit — 980d1fb1 ("Green gate autofix: disable hooks + gate on final full re-run") — plus a clean merge-in of origin/main (c0b169e, whose entire diff is the already-reviewed #3312 decomposition; second parent 6e9de94 is an ancestor of origin/main, no PR-authored conflict resolution). The merge left orchestrator/slice_green_gate.py and its tests byte-identical to 980d1fb1, so the fixes are intact on HEAD. My review scope is 980d1fb1.

Blocking 1 — orchestrator-host ACE via worktree-resolved hooks — fixed, verified

_commit_and_push_autofix._git now builds cmd = ["git", "-c", "core.hooksPath=/dev/null", "-C", worktree_path] (slice_green_gate.py:839) and every host-side git invocation flows through this helper: git add -u (:858), git diff --cached (:862), git commit --no-verify (:885). I grepped the file for git subprocess calls — the only other two (git diff --name-only :271, git ls-files :296) run inside the runner pod, are read-only, and invoke no hooks, so they need no neutralization. --no-verify is correctly retained as a second layer on the commit, and the docstring (:805-810, :832-838) now names core.hooksPath=/dev/null as the hook-neutralizing mechanism — the false "--no-verify suppresses hooks the same way" claim is gone.

Blocking 2 — committed/pushed tip not proven green — fixed, verified

Both divergences are closed generically:

  • Sequential/greens divergence: the runner now does one final full re-run of every configured check against the all-fixes-applied tree and emits final_verification.all_ok / .failed (:355-383). _autofix_ready (:767-772) blocks unless final_verification.ran and all_ok are both true — so a later fix that regresses an earlier-green or earlier-fixed check now blocks the slice. A missing final_verification (old/degraded runner) also blocks (:768-769).
  • git add -u drops fix-created files: untracked_files() captures git ls-files --others --exclude-standard before/after fixes (:285-307, :310, :367); new_untracked_count is the true (uncapped) count. _autofix_ready blocks on new_untracked_count being non-zero or None (:774-782) — the unknown-on-git-failure case fails safe. --exclude-standard correctly excludes .gitignored check droppings.

The fail-safe posture is sound: unknown/degraded/missing verdicts all route to a blocked slice rather than a speculative commit, and autofix only fires in mode == "on" (:1150).

Tests exercise the production path

  • test_final_verification_* and test_no_fix_applied_omits_final_verification run sgg._RUNNER_PROGRAM via subprocess against a real git init repo (_run_runner :238-239), so they exercise the actual embedded runner that generates final_verification / untracked detection — not a reimplementation.
  • TestAutofixReady unit-tests the real _autofix_ready decision function across all six branches (ready, unfixable check, missing/red final, untracked created, unknown count).
  • TestGreenGateAutofixWiring new cases assert _commit_and_push_autofix is not called on each block path.

No self-seeding goldens, no hand-built fixtures bypassing the production helper, no name/behavior contradictions.

Non-blocking

  • Non-blocking 4 (dead and repo_host_dir guard) — author kept as harmless defense-in-depth. Legitimate; it genuinely cannot misfire. No objection.
  • Sequencing (from the PR description): landing this after the gate soaks in log mode is a sound operational call — autofix is inert until mode == "on", so it doesn't gate that decision.

No blocking issues remain. Approving.

— Authored by egg

@james-in-a-box

This comment has been minimized.

Resolve slice_green_gate.py conflicts between #3409 (Stage A autofix)
and #3417/#3518 (infra-induced red classification):

- runner: keep the extracted run_cmd/fix loop and tag each first-run red
  with classify_infra, so the verdict carries both the fix sub-object and
  the infra field
- job manifest: pass both EGG_GREEN_GATE_CHANGED_FILES_CAP and
  EGG_GREEN_GATE_INFRA_SIGNATURES
- run_slice_green_gate: run the infra fail-open first, then judge autofix
  readiness and the failure message against the genuine (non-infra) reds;
  an infra red carries no fix that could clear it

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of #3517 — delta since c0b169e8

Verdict: approve. Both previously blocking issues are fixed on HEAD and the delta introduces no new blocking defect. Four non-blocking items below, one of which is a real gap in test coverage for logic that exists on neither parent.

What the delta actually is

git log c0b169e8..HEAD --not origin/main is a single commit — merge 22b6860 of origin/main (026196c7) into the branch. The conflict-resolution comment on the PR says only shared/egg_contracts/tests/test_artifact_spec.py conflicted, and textually that is true, but the substantive content of this delta is a semantic resolution in orchestrator/slice_green_gate.py: composing #3409's autofix with #3417's infra-red fail-open, which landed on main while this branch was open. I verified that neither parent contains it:

parent c0b169e8 (branch): _autofix_ready(verdict, failed) … fixed_checks=failed
parent 026196c7 (main):   genuine_failed = [c for c in failed if not c.get("infra")]
HEAD:                     _autofix_ready(verdict, genuine_failed) … fixed_checks=genuine_failed

That threading is hand-authored in the merge, so I reviewed it as new code rather than as an inherited merge.

Prior blockers — verified fixed, not superficially

  1. Git hooks on the orchestrator's autofix commit. slice_green_gate.py:974 builds ["git", "-c", "core.hooksPath=/dev/null", "-C", worktree_path], applied to every invocation in _commit_and_push_autofix — not just commit, so git add and post-commit hooks in the agent-controlled worktree are neutralized too. This matches the precedent in StateStore._run_git, agent_salvage._run_git, and gateway_client/_push.py. Survived the merge intact.
  2. "The committed tip is validated green" was only true for a single-check config. _autofix_ready (slice_green_gate.py:873-919) now gates on three things, not one: every failed check's own re-run green, final_verification.ran and .all_ok (one full re-run of every configured check against the all-fixes-applied tree, runner lines 473-500), and new_untracked_count == 0 with None treated as unsafe. The git add -u blind spot is closed by the untracked delta. Also survived the merge.

The composition itself

I traced every path through the merged run_slice_green_gate (lines 1268-1362) looking for a way the infra filter could widen what gets pushed. It cannot: _autofix_ready gates on final_verification.all_ok, which is computed over all configured checks including infra-tagged ones, so narrowing failedgenuine_failed only ever removes reasons to push, never adds one. Every divergence I found between the two parents' intent resolves conservatively (block the slice) rather than toward a false pass. Nothing was dropped from either side either — _INFRA_LINE_SIGNATURES, _INFRA_SUBSTRING_SIGNATURES, classify_infra, _infra_fail_open_enabled, and the fail-open block are all present, and main's #3417 tests still pass. Config plumbing for the optional fix: key is intact in all three copies (shared/egg_config/validators.py:191, config/repo_config.py:384, orchestrator/routes/pipelines/__init__.py:475) and documented at config/repositories.yaml.example:148.

The run_cmd exception-message rename ("execute check" → "execute command") has no consumers — one grep hit, in the module itself.

I could not execute TestCommitAndPushAutofix here: it builds real git repos and git init is blocked by the sandbox gateway (CalledProcessError on git -c … init -q .). The other 33 infra/autofix tests pass locally; CI is authoritative for the rest.

Non-blocking

1. The merge-authored composition has no test. 92 tests vs main's 63, and none of the 29 new ones exercises an infra-tagged red co-occurring with a fixable genuine red. TestGreenGateAutofixWiring builds verdicts with no infra field; the #3417 tests build verdicts with no fix block. The one line that exists on neither parent — _autofix_ready(verdict, genuine_failed) / fixed_checks=genuine_failed — is exactly the line nothing covers. A test with checks=[test(infra=oom), lint(fix→green)] plus a green final_verification asserting the push happens and fixed_checks names only lint would pin it; a second with a red final_verification would pin the block. This is what a conflict resolution most needs and least often gets.

2. When an infra red suppresses autofix, the operator-facing message is actively misleading. Infra red on test, fixable genuine red on lint, test still red in the final re-run → _autofix_ready returns (False, "final full re-run of all checks was not green (red: test)"). That reason is logged (line 1314) and then discarded. The returned message says the gate is red on lint and tells the operator to "fix the failures on <branch>" — but lint was auto-fixable and the actual blocker is the check the gate deliberately hid. The operator goes to the branch, runs make lint-fix, sees it work, and has no idea why the gate refused. Appending autofix_block_reason to the returned message when autofix_ready is false and infra reds were filtered would cost one line. (Main already returns only genuine_failed in the message; what's new here is that a block reason naming the hidden check now exists and stays log-only.)

3. final_verification is not infra-classified, so a transient infra fault turns a self-healing slice into a blocked one. The final-re-run entries (lines 477-484) carry name/ok/exit_code/output_tail — no infra key — and _autofix_ready has no fail-open. So the same SIGKILL that fails open on the first run blocks autofix on the final run. The asymmetry is safe in the push direction but strictly worse than not fixing at all: without the fix commands configured, that slice would have failed open and proceeded; with them, it blocks. Running classify_infra on the final entries and letting _autofix_ready ignore infra-tagged final reds (under the same EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN switch) would restore symmetry. Worth a follow-up rather than a change here.

4. The comment justifying the ordering asserts something the code doesn't guarantee. Lines 1273-1275: "This runs before the #3409 autofix decision so autofix is judged on the genuine reds: an infra red carries no fix that could clear it." An infra-tagged check absolutely can carry a fix — entry["infra"] is set from the first run (line 439) and never recomputed, while the fix and its re-run run unconditionally on any red with a fix: (line 442). A check that got SIGKILLed, then had its fix applied, then re-ran clean has both infra set and check_ok_after_fix: True. If it is the only red, the gate fails open and throws the proven fix away with the worktree — and any_fix_applied was still set, so the runner paid for the full final_verification re-run of every check to produce a verdict nothing consumes. No unsafe state (the worktree is deleted in the finally), but the stated rationale is wrong and check_ok_after_fix is real evidence the infra fault was transient. Either narrow the claim in the comment or use that evidence.

5. Minor: shared/egg_contracts/tests/test_artifact_spec.py still diverges from main in comment prose and one assertion message ("no Python sources under …" vs "no modules under …"); logic is byte-identical. Harmless, but it will conflict again on the next merge for no benefit.

Green-gate autofix is also not described in docs/architecture/slice-dag.md, whose green-gate paragraph (lines 552-557) documents #3398 and #3417 but not the fact that the gate now commits and pushes to the integration branch as egg-green-gate. An autonomous push is the kind of thing an operator reading that section needs to know about. Pre-existing to this delta, not introduced by it.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

Addresses the non-blocking review items on #3517.

- Add TestInfraFailOpenAutofixComposition: the merge that composed
  #3409's autofix with #3417's infra fail-open threaded genuine_failed
  into _autofix_ready / fixed_checks, and that line existed on neither
  parent and had no coverage. Pins the push (infra red + fixable genuine
  red + green final re-run pushes only the genuine fix), the block (a
  persistent infra red in the final re-run refuses the push), and the
  narrowing itself as what enables the push (switch off => no push).

- Explain autofix refusals in the operator-facing message. When every
  red the operator is shown had a working fix but the gate still
  refused, append the block reason; otherwise they re-run the fix
  command, watch it succeed, and see no reason for the block. Factor
  _autofix_ready's first gate into _all_failed_checks_fixed so the
  no-fix case stays silent and the two cannot drift.

- Correct the comment above the infra filter: an infra-tagged red *can*
  carry a fix that cleared it (infra is tagged from the first run and
  never recomputed), so the old rationale was wrong. State what the
  filter actually does and why it is safe in the push direction.

- Document the gate's autofix push in docs/architecture/slice-dag.md:
  the orchestrator authoring commits on a slice branch is the kind of
  thing that section's readers need to know about.

- Realign test_artifact_spec.py with main (comment prose and one assert
  message only; logic was already byte-identical) so it stops
  re-conflicting on every merge for no benefit.
@james-in-a-box

Copy link
Copy Markdown
Contributor

Review feedback addressed (commit 1ab60ee)

Thanks — item 1 was a real hole and item 4 was a genuinely wrong comment, not just imprecise prose.

⚠️ Push note: the gateway denies egg's push to issue-3409-green-gate-autofix (Branch ... is not owned by james-in-a-box or an authorized user) because this PR is human-authored. The commit is on egg/issue-3409-green-gate-autofix-review-fixes-v3 and its parent is this PR's current HEAD 22b6860, so a plain fast-forward lands it:

git push origin origin/egg/issue-3409-green-gate-autofix-review-fixes-v3:issue-3409-green-gate-autofix

Non-blocking

1. The merge-authored composition has no testfixed-in-PR (commit 1ab60ee). You're right that _autofix_ready(verdict, genuine_failed) / fixed_checks=genuine_failed was the one line on neither parent and the one line nothing covered. New TestInfraFailOpenAutofixComposition (orchestrator/tests/test_slice_green_gate.py) builds exactly the verdict you described — test infra-tagged red + lint red with a fix that went green — and pins three things:

  • green final_verification → the push happens and fixed_checks names only lint;
  • final_verification red on test → no push, slice blocks;
  • the same verdict with EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN=off → no push, because without the narrowing the unfixable infra red keeps the verdict only partially fixable. That third case is what actually pins the narrowing as the thing enabling the push, rather than just asserting the push happens.

2. Misleading operator message when an infra red suppresses autofixfixed-in-PR (commit 1ab60ee). The gate now appends the block reason: "The configured fix commands cleared the checks above in the runner, but the gate did not self-heal: <reason>."

I gated it on _all_failed_checks_fixed(genuine_failed) rather than on "infra reds were filtered." The condition that makes the message contradictory is every red the operator is shown had a working fix, and the gate refused anyway — which is the infra case, but equally the fix-regressed-a-sibling and fix-created-an-untracked-file cases. When a genuine red has no fix, the operator has something real to fix on the branch and the note would be noise, so it stays silent there. That predicate is _autofix_ready's own first gate, factored out so the two can't drift.

One deliberate consequence: the block reason names the hidden check (red: test), so an infra check's name re-enters the message. I think that's correct and is what you were asking for — it's framed as the cause of the autofix refusal, not routed as a slice failure, and the infra check's output tail still stays out of the presented failure list. The existing test_mixed_reds_block_on_genuine_only assertion is unaffected and the new test asserts both halves explicitly.

3. final_verification is not infra-classifieddeferred-to #3621. Filed per your explicit "worth a follow-up rather than a change here." Agreed on the analysis: classify_infra runs only on the first run, the final entries carry no infra key, and _autofix_ready has no fail-open, so configuring a fix: can turn a slice that would have failed open into a blocked one. The issue specifies the fix under the same EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN switch and mirroring the first-run policy exactly — fail open only when every final red is infra-tagged, since a genuine red mixed in must still block. Keeping it out of this PR also avoids widening the autofix bar in the same change that introduces it, before the gate has soaked in log mode.

4. The ordering comment asserts something the code doesn't guaranteefixed-in-PR (commit 1ab60ee). "An infra red carries no fix that could clear it" was simply false: infra is set from the first run and never recomputed, while the fix runs unconditionally on any red with a fix:, so infra and check_ok_after_fix: True can coexist. Rewrote it to say what the filter actually does — it discards that evidence deliberately, because the fail-open path returns before any push, so the proven fix is dropped with the worktree rather than committed on the strength of a run already classified as untrustworthy — plus your safety argument, that _autofix_ready gates on final_verification.all_ok computed over every check, so narrowing failed can only remove reasons to push.

5. test_artifact_spec.py diverges from mainfixed-in-PR (commit 1ab60ee). Restored to origin/main verbatim. The divergence was comment prose plus one assert message over byte-identical logic, so there was nothing to preserve and it would have re-conflicted on the next merge for no benefit.

Green-gate autofix missing from docs/architecture/slice-dag.mdfixed-in-PR (commit 1ab60ee). Agreed that an autonomous push belongs in that section regardless of which delta introduced it. The green-gate paragraph now records that the gate writes to the integration branch as egg-green-gate via the launcher-authed push route, that this is the one place the orchestrator authors commits on a slice branch, that it fires only in on mode and only when the runner proves the exact git add -u tree green (final_verification.all_ok + no new untracked files), and that a commit/push failure blocks the slice like an unfixed red.

Verification

orchestrator/tests/test_slice_green_gate.py: 107 passed, 12 failed — the 12 are the same git init-blocked-by-gateway failures you hit (TestRunnerFixFlow, TestCommitAndPushAutofix), identical on the pre-change baseline (103 passed / same 12 failed), so the delta is +4 passing and no regression. shared/egg_contracts/tests/test_artifact_spec.py: 50 passed. make lint: ruff check and ruff format clean; the 3 mypy errors are in shared/egg_agent/client.py, which is byte-identical to origin/main. Per workflow guidance I did not run the full suite — CI is authoritative.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No agent-mode design concerns.

This is deterministic pipeline infrastructure — no LLM invocation, no prompt assembly, no model selection. Checked the diff against each anti-pattern:

  • No LLM API calls / SDK bypass (items 6, 7): nothing in orchestrator/slice_green_gate.py touches httpx, requests, or the Anthropic SDK. The only anthropic token in the diff is a pre-existing validate_anthropic_key import in tests/egg_config/test_validators.py:1479 — a key-format validator, not an API call.
  • No hardcoded model IDs (item 8): no model identifiers anywhere in the changeset.
  • Not a post-processing pipeline over agent output (item 3): the EGG_GREEN_GATE_VERDICT: JSON that _autofix_ready consumes is emitted by a deterministic python3 -c runner program, not by an agent. Machine-parsed output from a machine producer is the legitimate case in the guide ("machine-readable output for genuine automation").
  • No growth in the agent-facing payload (item 1): the failure string routed to the slice team still renders only each check's first-run output_tail via _format_failed_checks — the new fix.output_tail / recheck_output_tail fields stay in the verdict for logging. The one addition is autofix_note, a single natural-language sentence.
  • Constraints are infrastructure-enforced, not prompt-level (item 5): the runner never pushes; the write goes through gateway.push_worktree_branch on the launcher-authed route, and the repo-declared fix shell command executes inside the runner pod rather than the orchestrator (_commit_and_push_autofix runs fixed git argv only, hooks neutralized via core.hooksPath=/dev/null). Correct side of the sandbox boundary.

Worth noting as positive alignment: routing a deterministic remediation (make lint-fix) through a config-declared command instead of spawning an agent is the right call — and non-deterministic reds still route back to the slice team, so agent judgment is preserved exactly where it's needed.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of #3517 — delta since 22b6860e

Verdict: approve. The delta is a single PR-authored commit, 1ab60ee ("Green gate: test infra/autofix composition, explain autofix refusals"), which addresses four of the five non-blocking items from my last review plus the docs gap, and defers the fifth to a filed follow-up. No new blocking defect; three minor advisory items below.

First, the thing that went wrong twice before did not go wrong this time: git rev-parse origin/issue-3409-green-gate-autofix is 1ab60ee, so the fix commit is actually on the PR HEAD and CI will run it. The scope is also clean — git diff $(git merge-base origin/main HEAD)..HEAD is 8 files, all #3409, no cross-contamination.

Disposition of each prior item

1. Merge-authored composition had no test — fixed, verified. TestInfraFailOpenAutofixComposition (orchestrator/tests/test_slice_green_gate.py:1421-1556) is the right test, and the third case is what makes it more than a happy-path assertion. test_infra_red_alongside_fixable_red_pushes_only_the_genuine_fix pins fixed_checks == ["lint"]; test_infra_red_still_red_in_final_rerun_blocks_the_fixable_red pins the block; test_fail_open_switch_off_makes_the_infra_red_block_the_autofix flips EGG_SLICE_GREEN_GATE_INFRA_FAIL_OPEN=off and shows the push disappears — which pins the failedgenuine_failed narrowing itself as the enabling mechanism rather than just asserting that a push happens. That is the mutation-discriminating case I was asking for. The tests drive the real run_slice_green_gate through _verdict_lineparse_verdict, so the production decision path is exercised; only _commit_and_push_autofix is mocked, consistent with the existing TestGreenGateAutofixWiring cases. Ran them: 19/19 pass across the three autofix classes.

2. Misleading operator message — fixed, and gated better than I suggested. I proposed gating the appended reason on "infra reds were filtered." Gating on _all_failed_checks_fixed(genuine_failed) (slice_green_gate.py:1380) is the better predicate: the contradiction the operator experiences is every red I was shown had a working fix and the gate refused anyway, which covers the fix-regressed-a-sibling and fix-created-an-untracked-file cases too, and stays silent when a genuine red has no fix and the message is already self-explanatory. Factoring _autofix_ready's first gate into _all_failed_checks_fixed (:873-885) so the two cannot drift is the correct structural move — I checked the extraction is verbatim.

I traced the branch chain for a way to emit an empty or wrong reason. There isn't one: if _all_failed_checks_fixed is true and everything else passes, _autofix_ready returns (True, "") and not autofix_ready is false, so the branch cannot produce did not self-heal: . with an empty reason. genuine_failed is also provably non-empty at that point (:1271 returns on no failures; :1315 returns on empty genuine_failed), so _all_failed_checks_fixed's vacuous-all([]) truth is unreachable. The note is appended into a free-text reason that _run_implement.py:937 returns verbatim — nothing parses it — so the added text is safe.

On the deliberate consequence you flagged (the infra check's name re-entering the message via red: test): agreed, and the framing is right. It appears as the cause of the autofix refusal, not as a routed slice failure, and test_infra_red_still_red_in_final_rerun_blocks_the_fixable_red:1495 asserts the infra check's output tail (GATEWAY SIDECAR NOT AVAILABLE) stays out. Silence was the worse option.

3. final_verification not infra-classified — deferred, correctly. #3621 exists, is open, and states the problem accurately, including the part that matters: mirror the first-run policy exactly, failing open only when every final red is infra-tagged. Not widening the autofix bar in the same PR that introduces it is the right call.

4. Ordering comment asserted something false — fixed. The rewritten comment (:1287-1302) now says what the code actually does. I verified the mechanism it describes: entry["infra"] is set from the first run at :439 and never recomputed, while the fix branch at :442 fires on any red carrying a fix:, so infra and check_ok_after_fix: True genuinely can coexist. The new rationale — that the fail-open path returns at :1324 before any push, so the proven fix is discarded with the worktree rather than committed on the strength of a run already classified as untrustworthy — is accurate, and the retained safety argument (final_verification.all_ok is computed over every configured check at :495, so narrowing can only remove reasons to push) holds.

5. test_artifact_spec.py divergence — fixed. git diff origin/main 1ab60ee -- shared/egg_contracts/tests/test_artifact_spec.py is empty. It will no longer re-conflict.

Docs gap — fixed. docs/architecture/slice-dag.md:557-570 now records the autonomous push, the egg-green-gate identity, the launcher-authed route, the on-mode-only constraint, and the final_verification.all_ok + no-new-untracked proof obligation. Every claim checks out against the code.

Non-blocking

a. docs/architecture/slice-dag.md:560 — the inline config example has the wrong shape. It reads (e.g. `lint: {fix: make lint-fix}`), which suggests checks is a mapping keyed by check name. It isn't — per config/repositories.yaml.example:145-150 and validate_checks, it's a list of {name, command, fix} entries:

checks:
  - name: lint
    command: make lint
    fix: make lint-fix

A reader copying the shorthand writes config validate_checks will drop. Suggest (e.g. a `lint` check with `fix: make lint-fix`) — the phrasing the PR description already uses.

b. Same paragraph is now a ~19-line parenthetical. The ( opens at :552 and the matching ) lands at :570, with three full sentences and a bolded clause in between. Consider closing the #3398/#3417 parenthetical and starting the autofix material as its own sentence or sub-bullet.

c. test_fail_open_switch_off_makes_the_infra_red_block_the_autofix:1554assert "test" in failure carries almost no signal. The load-bearing assertions are autofix.assert_not_called() and failure is not None; "test" in failure would survive most mutations of the narrowing. If the intent is "with the switch off, both reds are presented," assert that directly — e.g. that the failure names both lint and test in the header line, which is the actual behavioural difference from the narrowed case.

d. Cosmetic, and you already declined the parent of this once. The and repo_host_dir guard at :1345 is still unreachable (:1140 refuses to proceed unless wt_result.worktrees is non-empty, so repo_host_dir is always set at :1186). It now has one new consequence worth naming: if it ever did go falsy, autofix_ready is true so neither elif fires, and the operator gets a failure message with no explanation at all — the precise gap 1ab60ee set out to close. Either drop the guard or make the second branch elif autofix_ready and mode == "on":. Genuinely advisory; I cannot construct a reachable trigger.

Verification

  • TestInfraFailOpenAutofixComposition + TestAutofixReady + TestGreenGateAutofixWiring: 19 passed.
  • Full orchestrator/tests/test_slice_green_gate.py: 107 passed, 12 failed — all 12 are TestRunnerFixFlow / TestCommitAndPushAutofix failing on git init blocked by the sandbox gateway, matching what the author reports and what I hit on the previous HEAD. Not a code failure; CI is authoritative.
  • ruff check and ruff format --check clean on both changed Python files.
  • Per workflow guidance I did not run make test.

No blocking issues. Approving.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

@james-in-a-box

Copy link
Copy Markdown
Contributor

Review feedback addressed (commit 99ec774)

All four non-blocking items from the latest review are fixed. Item d turned out to be more than cosmetic once I looked at what the fall-through actually does.

⚠️ Push note (same recurring blocker): the gateway denies egg's push to issue-3409-green-gate-autofixBranch ... is not owned by james-in-a-box or an authorized user. Use a bot-prefixed branch (egg-* or egg/*). The commit is on egg/issue-3409-green-gate-autofix-review-fixes-v4, and its parent is this PR's current HEAD 1ab60ee (verified against the API), so a plain fast-forward lands it — no cherry-pick, no conflict:

git push origin origin/egg/issue-3409-green-gate-autofix-review-fixes-v4:issue-3409-green-gate-autofix

a. slice-dag.md inline config example had the wrong shape — fixed-in-PR (commit 99ec774)

Correct catch — lint: {fix: make lint-fix} reads as a name-keyed mapping, and validate_checks would drop config written that way. Now reads a `lint` check with `fix: make lint-fix` , with the actual shape spelled out and a pointer to the example file so a reader doesn't have to infer it:

checks is a list of {name, command, fix} entries, see config/repositories.yaml.example

b. ~19-line parenthetical — fixed-in-PR (commit 99ec774)

The #3398/#3417 parenthetical now closes at #3417) and the sentence continues to create_slice_pr as it did before #3409. The autofix material is its own paragraph inside item 3, opening on the bolded claim. Nothing was dropped.

c. assert "test" in failure carried almost no signal — fixed-in-PR (commit 99ec774)

Agreed — that substring survives nearly any mutation of the narrowing. Replaced with the assertions that actually distinguish switch-off from the narrowed case:

assert "tip: test, lint" in failure          # header names BOTH reds
assert "GATEWAY SIDECAR NOT AVAILABLE" in failure   # infra tail no longer filtered
assert "did not self-heal" not in failure    # `test` has no fix → no self-heal note

The middle one is the exact mirror of test_infra_red_still_red_in_final_rerun_blocks_the_fixable_red's assert "GATEWAY SIDECAR NOT AVAILABLE" not in failure, so the pair now pins the presented-vs-hidden distinction from both sides.

d. Unreachable and repo_host_dir guard — fixed-in-PR (commit 99ec774)

I declined the parent of this item, but you're right that this version has real consequence, and it's the one 1ab60ee set out to eliminate: if the guard ever went falsy, autofix_ready stays true, so neither elif fires and the operator gets a fixable-looking failure with no explanation at all.

Rather than drop the guard (which would hand _commit_and_push_autofix an empty worktree_path and run git in the orchestrator's cwd — strictly worse), I folded it into the block-reason machinery before the branch:

autofix_ready, autofix_block_reason = _autofix_ready(verdict, genuine_failed)
if autofix_ready and not repo_host_dir:
    autofix_ready = False
    autofix_block_reason = "the runner worktree host path is unavailable"

The branch is now if autofix_ready and mode == "on":, and the unexplained-block path is gone by construction: any way of reaching it sets a reason, so the third elif fires and the operator sees why. Placing it above the logger.error also keeps the logged autofix_ready / autofix_block_reason honest.

It is also now reachable in a test — a gateway returning {repo: ""} passes the non-empty-worktrees check at :1140 and leaves repo_host_dir empty. test_missing_worktree_host_path_blocks_with_an_explanation drives that through the real run_slice_green_gate and asserts _commit_and_push_autofix is not called and the message explains the refusal. It fails against the pre-fix code (no note is emitted), so it discriminates rather than just passing.

Verification

  • TestGreenGateAutofixWiring + TestAutofixReady + TestInfraFailOpenAutofixComposition: 20 passed (was 19 — the new case).
  • Full orchestrator/tests/test_slice_green_gate.py: 108 passed, 12 failed — the same 12 TestRunnerFixFlow / TestCommitAndPushAutofix git init failures you hit, blocked by the sandbox gateway. Unchanged from the pre-fix baseline; CI is authoritative.
  • make lint: ruff check and ruff format clean. Mypy reports 3 errors in shared/egg_agent/client.py, which this PR does not touch — I confirmed they reproduce on 1ab60ee with my changes stashed, so they're pre-existing on the branch.
  • Per workflow guidance I did not run make test.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

This comment has been minimized.

…ze cap

Neither parent violated the 1500-line hard cap — main sat at 1165 lines
and the Stage-A autofix branch at 1421 — but their union is 1594. Most
of the growth is module-docstring prose added independently on both
sides (the rollout rationale for the 'on' default, and the autofix
self-heal contract).

Decomposition is tracked in #3627; allowlisting keeps that refactor out
of a merge commit.
@james-in-a-box

Copy link
Copy Markdown
Contributor

Conflict Resolution Summary

Merged origin/main (60384d0) into issue-3409-green-gate-autofix via merge commit fbf08d2, plus one follow-up commit (9a160bf) for a post-merge lint failure. PR is MERGEABLE again.

File Category Resolution
docs/architecture/slice-dag.md Additive (prose) Kept both sides of the green-gate parenthetical
scripts/file-size-allowlist.yaml Post-merge lint fix New entry for slice_green_gate.py (see below)

docs/architecture/slice-dag.md — the only real conflict

One hunk, in the green-gate parenthetical of the implement-phase run-loop section. Both sides edited the same sentence for unrelated reasons:

Complementary, so both were kept: main's mode clause verbatim, then this PR's autofix paragraph, with the closing paren moved to the end of the combined text. This PR's "it fires only in on mode" sentence stays accurate under main's new default — on is now what you get when nothing is set.

Everything else auto-merged, but two things are worth a look

1. orchestrator/slice_green_gate.py crossed the file-size hard cap on the merge, and neither parent did. main was 1165 lines, this branch 1421, the union 1594 — over the 1500-line cap in scripts/check-file-sizes.py. Most of the growth is module-docstring prose added independently on both sides (#3609's rollout rationale for the on default, #3409's autofix contract). make lint fails without action here.

I allowlisted the file with tracking issue #3627 (filed for this, mirroring the #3498 precedent for concurrent_executor.py) rather than landing a decomposition inside a merge commit. If you'd rather trim the docstring overlap than carry an allowlist entry, that's a one-line revert of 9a160bf plus the prose edit — the two docstrings do restate the mode semantics in places.

2. orchestrator/slice_green_gate.py and its test file both merged textually clean, but both sides changed green-gate behavior, so I verified the seam rather than trusting the auto-merge:

  • Mode handling is coherent: _DEFAULT_MODE = "on" (main) and the autofix gated on mode == "on" (this PR) agree; the log-mode branch still logs autofix_ready without committing.
  • run_slice_green_gate's signature still matches its only call site in routes/pipelines/_run_implement.py:931.
  • The apparent duplicate test-method names (test_default_is_on, test_enabled_values, …) are not a merge artifact — they're across two distinct classes, TestGreenGateMode and TestInfraFailOpenEnabled, one per env switch.

Verification

  • make lint — clean (was failing on file-sizes only; fixed by the allowlist entry).
  • make build — not run; it builds Docker images and Docker isn't available in this sandbox. Substituted compileall over the merged modules (clean).
  • Targeted tests (not the full suite, per the workflow's timeout constraint): orchestrator/tests/test_slice_green_gate.py + tests/egg_config/test_validators.py196 passed, 12 failed. All 12 failures are git init calls in TestRunnerFixFlow / TestCommitAndPushAutofix, which the gateway blocks in this container (ERROR: git init is not supported in the container) — environmental, reproducible outside the merge, and expected to pass in CI.
  • The merge-sensitive classes specifically — TestGreenGateMode, TestGreenGateAutofixWiring, TestRunSliceGreenGate, TestInfraFailOpenAutofixComposition, TestAutofixReady, TestSubmitRunnerJob74 passed.

Please review: the two items above — the allowlist entry (policy call, easily reverted) and the mode-semantics seam between #3609's default flip and this PR's autofix gating. CI will run the full suite, including the 12 tests this sandbox can't execute.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor

egg conflict resolution completed. View run logs

@jwbron
jwbron merged commit ec35f0c into main Jul 25, 2026
20 checks passed

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agent-mode design re-review of #3517 — delta since 1ab60ee

No agent-mode design concerns. Approving.

The PR-authored delta since my last review is two commits, neither of which touches an agent-mode surface:

  • fbf08d2 — merge of origin/main (60384d0). The only conflict resolution is prose in docs/architecture/slice-dag.md:551-570, keeping #3609's EGG_SLICE_GREEN_GATE default-on clause alongside this PR's autofix paragraph. Everything else in the merge range is main-side.
  • 9a160bf — a six-line entry in scripts/file-size-allowlist.yaml for orchestrator/slice_green_gate.py, with #3627 as the tracking issue.

Re-checked the anti-pattern list against the delta rather than assuming my prior pass carries:

  • No LLM calls or SDK bypass (items 6, 7): git diff 1ab60ee HEAD does surface anthropic tokens, but every one is in config/litellm/cost_callback.py, tests/config/test_cost_callback.py, config/litellm-models.template.yaml, and k8s/base/litellm-configmap.yaml — main-side files outside this PR's nine-file scope. Nothing PR-authored touches httpx, requests, or the Anthropic SDK.
  • No model identifiers (item 8): none anywhere in the delta.
  • Agent-facing payload unchanged (item 1): the failure string routed to the slice team is byte-identical to what I reviewed at 1ab60eeslice_green_gate.py:1567-1574, still _format_failed_checks(genuine_failed) capped at _FAILURE_MESSAGE_TAIL_CHARS = 1500 per check, plus the one-sentence autofix_note. The merge with #3609 did not widen it; fix.output_tail / recheck_output_tail still stay in the verdict for logging only.
  • Not a post-processing pipeline (item 3): the verdict _autofix_ready consumes still comes from the deterministic python3 -c runner, not from an agent.
  • Constraints stay infrastructure-enforced (item 5): unchanged — the runner never pushes; the write goes through gateway.push_worktree_branch on the launcher-authed route.

One thing the merge changed that's worth naming as positive for this lens: #3609 flipping the default to on means the bounded natural-language failure message is now the routine path back to the slice team rather than a log line nobody reads. That message is orienting (named branch, per-check tails, the off bypass quoted inline) rather than a pre-fetched dump, and non-deterministic reds still route to agent judgment while only config-declared deterministic fixes self-heal. That division is the right one.


Outside my lens, but factual and cheap to flag: the commit that addressed the base review bot's four non-blocking items — 99ec774, described in this comment — is not on the PR branch. It exists on origin/egg/issue-3409-green-gate-autofix-review-fixes-v4 (git ls-remote confirms), but git merge-base --is-ancestor 99ec774 HEAD is false, and docs/architecture/slice-dag.md:561 still carries the wrong-shape example lint: {fix: make lint-fix} that item (a) reported. The fast-forward that comment suggested is also no longer available, since fbf08d2 landed on top of 1ab60ee — it now needs a merge or cherry-pick. Not an agent-mode issue; raising it only because the delta made a claimed-landed fix silently not land.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor

egg agent-mode-design completed. View run logs

22 previous review(s) hidden.

@egg-reviewer egg-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of #3517 — delta since 1ab60ee

Verdict: approve. The PR-authored delta is two commits — fbf08d2 (merge of origin/main @ 60384d0) and 9a160bf (file-size allowlist entry). I verified the merge is lossless in both directions, the mode/autofix seam is coherent, and the allowlist entry is legitimate. No new blocking defect.

One thing the thread gets wrong, though, and it is load-bearing for whoever merges: the commit that claimed to address the last round's four items did not land. Details below.

What I verified about the merge

1. Nothing was dropped from either parent. For each of the three files both sides touched, I compared main's content-level changes since the merge base (026196c) against what the merge actually applied to the PR side, stripping hunk headers:

File Result
orchestrator/slice_green_gate.py identical content changes
orchestrator/tests/test_slice_green_gate.py identical content changes
docs/architecture/slice-dag.md identical content changes

And every PR-only file (config/repo_config.py, config/repositories.yaml.example, orchestrator/routes/pipelines/__init__.py, shared/egg_config/validators.py, tests/egg_config/test_validators.py) is byte-identical across 1ab60ee → fbf08d2. The slice-dag.md conflict resolution keeps main's mode clause verbatim and this PR's autofix paragraph verbatim.

2. The mode seam holds. _DEFAULT_MODE = "on" (main) and if autofix_ready and mode == "on" (slice_green_gate.py:1518) agree; the log branch still logs autofix_ready without committing; run_slice_green_gate's signature still matches _run_implement.py's call site. Literal is still imported for _DEFAULT_MODE's annotation.

3. Main's _submit_runner_job reflection test composes with this PR's new manifest keys. #3609 made every field read from the manifest dict and added test_every_manifest_field_reaches_the_body; this PR's additions (EGG_GREEN_GATE_CHANGED_FILES_CAP, EGG_GREEN_GATE_INFRA_SIGNATURES) flow through the env list rather than adding top-level keys, so the reflection test stays green — confirmed by running TestSubmitRunnerJob.

4. No shadowed tests from the textual test-file merge. AST check over test_slice_green_gate.py: 15 classes, 108 test functions, zero duplicate method names within a class and zero duplicate class names. The test_default_is_on / test_enabled_values pairs the author flagged are indeed across TestGreenGateMode and TestInfraFailOpenEnabled, not shadowing.

5. The allowlist entry is correct. scripts/check-file-sizes.py exits 0 with it and would fail without it (1594 lines > 1500; bytes are fine at 73,658 < 100,000). #3627 exists, is open, and specifies the decomposition seams and the COPY orchestrator/slice_green_gate/ Dockerfile line the split will need — that last detail is the one these follow-ups usually miss. Allowlisting rather than decomposing inside a merge commit is the right call, and it matches the #3498 precedent.


The thread claims fixes that are not on the branch

The comment titled "Review feedback addressed (commit 99ec774)" itemizes fixes for all four non-blocking items from my last review. 99ec774 is not an ancestor of HEAD — it is not even reachable from the PR branch (git cat-file on it fails after a depth-100 fetch of issue-3409-green-gate-autofix). It lives on origin/egg/issue-3409-green-gate-autofix-review-fixes-v4. Each item is still present on HEAD:

Item Evidence it did not land
(a) wrong-shape config example docs/architecture/slice-dag.md:561 still reads (e.g. `lint: {fix: make lint-fix}`)
(b) ~19-line parenthetical unchanged
(c) weak assertion test_slice_green_gate.py:1913 still assert "test" in failure in TestInfraFailOpenAutofixComposition::test_fail_open_switch_off_makes_the_infra_red_block_the_autofix
(d) and repo_host_dir guard slice_green_gate.py:1518 still if autofix_ready and mode == "on" and repo_host_dir:; test_missing_worktree_host_path_blocks_with_an_explanation does not exist

The fast-forward that comment quoted is also no longer available — fbf08d2 landed on top of 1ab60ee, so this now needs a merge or cherry-pick.

None of the four is blocking, so this doesn't hold up the PR. But do not merge believing they landed, and of the four, (a) is the one worth landing first — see below.

Non-blocking

a. docs/architecture/slice-dag.md:561 — the shorthand teaches config that gets silently dropped. lint: {fix: make lint-fix} reads as a name-keyed mapping. checks is a list of {name, command, fix} entries, and validate_checks returns [] for a non-list input (shared/egg_config/validators.py:184-185) with no error and no log. An operator who copies the shorthand loses every check, not just the fix: _gate_checks then returns empty, and the gate logs "Green gate skipped: no configured checks for repo" at info and does nothing. Under the new on default that is a gate an operator believes is running and isn't. The PR body already has the right phrasing — ``a lint check with `fix: make lint-fix``` — plus a pointer to `config/repositories.yaml.example:145-150`, which is correct.

b. On item (d), I now think the opposite of the follow-up comment's reasoning — and it is worth correcting the record. The comment argues the fall-through is reachable because "a gateway returning {repo: ""} passes the non-empty-worktrees check at :1140". It passes that check, but it never reaches the autofix branch: with repo_host_dir == "", repo_dir becomes /home/egg/repos/ and the manifest's hostPath volume gets path: "", which the apiserver rejects (hostPath.path: Required value). _submit_runner_job raises, the except at slice_green_gate.py:1377-1387 fails open, and the function returns before any autofix decision. The claimed test only reaches the state because _submit_runner_job is mocked — it pins a production-unreachable state.

The proposed restructure (fold the host-path check into autofix_block_reason before the branch, so no path can block without a reason) is still worth having as defense-in-depth and reads better than a silent conjunct. Just don't land it described as a live bug fix, and don't let its test read as a reachability proof.

c. scripts/file-size-allowlist.yaml's header describes a ratchet the checker does not implement. The header says "the ratchet flags any allowlisted file that has dropped back under the cap as stale." It doesn't: check_all computes stale = sorted(rel for rel in config.allowlist if rel not in seen) (scripts/check-file-sizes.py:182) — that is files that no longer exist, and main() prints "no longer exists" for them. A file that shrinks back under the cap is still in seen, and evaluate() returns ([], []) early for any allowlisted file under the caps (:151-153), so it emits neither an error nor a soft warning. Nothing flags it.

That matters specifically for the entry this PR adds: when #3627 decomposes the module, no check will tell anyone to remove orchestrator/slice_green_gate.py from the allowlist. Pre-existing, and I'm calling it advisory rather than in-scope-because-you-touched-the-file — but the one-line fix in check_all (also flag rel in seen entries whose stats are under both caps) closes it.

d. The Stage A rollout paragraph was written when on was opt-in, and reads stale now. slice_green_gate.py:150-165 frames log as the soak stage where "the fix still runs in the runner (soak signal) but nothing is committed or pushed." After #3609, on is what a deployment gets by doing nothing, so nothing passes through log unless an operator deliberately sets it — the staged progression the paragraph implies no longer exists on the default path.

The real opt-in is now the fix: key in repositories.yaml, and that gate is genuine: no repositories.yaml ships in the repo, and config/repositories.yaml.example:148 has fix: commented out, so the autofix cannot fire until a deployment adds it. Worth saying that explicitly in the Stage A paragraph — "the autofix's opt-in is the fix: key, not the mode switch" — so a reader doesn't infer a soak stage that the default no longer routes through.

e. Item (b) from last round stands. The green-gate parenthetical in slice-dag.md opens at :552 and closes at :570 after three sentences and a bolded clause. Closing at #3417) and starting the autofix material as its own sentence reads much better; the merge preserved the long form.

Verification

  • orchestrator/tests/test_slice_green_gate.py: 131 passed, 9 skipped, 12 failed. All 12 failures are TestRunnerFixFlow / TestCommitAndPushAutofix calling git init, which the sandbox gateway refuses — environmental, matching what the author and prior reviews report. CI is authoritative.
  • tests/egg_config/test_validators.py: 56 passed. tests/scripts/test_check_file_sizes.py: 31 passed.
  • scripts/check-file-sizes.py: exit 0.
  • ruff check and ruff format --check clean on slice_green_gate.py, test_slice_green_gate.py, validators.py.
  • Re-read _commit_and_push_autofix in the composed tree since the default flip makes it live-by-default: hooks neutralized on every invocation via -c core.hooksPath=/dev/null, --no-verify on the commit, git add -u scoping, identity via -c user.name/user.email rather than env, fixed argv with no shell, push only through gateway.push_worktree_branch. Unchanged from 1ab60ee and still correct.
  • Per workflow guidance I did not run make test.

— Authored by egg

@james-in-a-box

Copy link
Copy Markdown
Contributor

egg review completed. View run logs

22 previous review(s) hidden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Green gate Stage A: server-side format autofix before slice PR open

1 participant