Skip to content

docs: fix stale .egg-state/contracts/ note [doc-updater] - #3419

Merged
jwbron merged 7 commits into
mainfrom
egg/doc-update-hard-block-egg-state
Jul 2, 2026
Merged

docs: fix stale .egg-state/contracts/ note [doc-updater]#3419
jwbron merged 7 commits into
mainfrom
egg/doc-update-hard-block-egg-state

Conversation

@james-in-a-box

@james-in-a-box james-in-a-box Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Update documentation to reflect changes from #3396 (Hard-block whole
.egg-state/ tree with anchored carve-backs):

Triggered by: 2e29233 (#3406)

Authored-by: egg

@james-in-a-box

This comment has been minimized.

@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 a documentation-only change correcting a stale blocklist path (.egg-state/contracts/ → the whole .egg-state/ tree) in two docs — no agent workflows, prompts, output formats, or model references are affected.

— 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.

ACK — accurate, minimal, well-scoped documentation fix. No blocking issues.

Verification

I traced both changed sentences to the canonical source (shared/egg_restrictions/patterns.py) and confirmed the new wording is correct:

  • The whole .egg-state/ tree is hard-blocked for the coder (patterns.py:324, hard_blocked_patterns) and tester (patterns.py:423), with only .egg-state/agent-outputs/ (and agent-anchors/ for the coder) carved back via hard_block_exempt_patterns. These are the two roles that gained the **/fixtures/ / **/testdata/ block-exemptions in #3396, which is exactly why the block had to be widened from contracts/ to the whole tree — otherwise a fixtures//testdata/ path could punch through into any other .egg-state/ subdir.
  • Both edited sentences are now consistent with the per-role descriptions already updated in #3396 in the same file: coder (docs/reference/agent-roles.md:250,280-284) and tester (:349-353) both say "whole .egg-state/ tree".

Confirmed the PR didn't miss sibling stale references

I checked every other .egg-state/contracts/ mention in docs/. The remaining ones are correct as-is and should NOT be changed, because those roles genuinely block only contracts/ (no fixtures carve-out, no whole-tree hard block):

  • refiner (agent-roles.md:55), simplifier (:117), architect (:137) — plan/refine agents, blocklist is _PLAN_AGENT_BLOCKED (patterns.py:504).
  • documenter (:382, :462) — blocked_patterns is .egg-state/contracts/ only (patterns.py:462), no hard_blocked_patterns.
  • autofixer (:478) — blocked_patterns is .egg-state/contracts/ only (patterns.py:794).

So the two sentences this PR touched were precisely the remaining stale ones. Good targeting.

Non-blocking follow-up

The same stale wording that this PR fixes in the docs also lives in the canonical source's own comments/docstring, which this docs-only PR (correctly) doesn't touch:

  • shared/egg_restrictions/patterns.py:165-167# Security-relevant blocklists (.egg-state/contracts/, .github/) are NOT overridable — this flat enumeration is now stale for the coder/tester tier (should reference the whole .egg-state/ tree).
  • shared/egg_restrictions/patterns.py:954-956 — same phrasing, though softened with "etc.", so less misleading.

Worth a one-line follow-up so the canonical source's comments stay in sync with the behavior #3396 shipped. Not blocking this PR.

— 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 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. One actionable item; disposition below.

Non-blocking follow-up — sync the canonical source's own comments (patterns.py:165-167, :954-956) with the .egg-state/ widening: fixed-in-PR (commit 1ef5c1f).

I fixed both in this PR rather than deferring — it's a two-line comment sync with no HITL decision needed, and the feedback contract biases toward in-PR fixes. Both spots now spell out the tiered reality instead of the flat .egg-state/contracts/ enumeration: "the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ for the other roles, and .github/". This keeps the canonical source's docstring/comment consistent with the per-role behavior #3396 shipped and with the two doc sentences this PR already corrected.

make lint ruff check/format pass. The mypy errors in shared/egg_agent/client.py are pre-existing and unrelated (comment-only change to patterns.py).

— Authored by egg

@james-in-a-box

This comment has been minimized.

@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.

The delta since my last review (1ef5c1f) is exactly the non-blocking follow-up I flagged: the canonical-source comment sync in shared/egg_restrictions/patterns.py:165-167 and :954-956, now matching the tiered reality ("whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ for the other roles, and .github/"). Comment/docstring-only — no impact on prompts, agent orientation, output shape, or sandbox-vs-prompt boundaries. Good call fixing it in-PR.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

Check Failure: Python / Custom Checks (Lint workflow)

What's failing: ruff format on 6 files (config/repo_config.py, gateway/gateway.py, gateway/tests/test_repo_visibility.py, orchestrator/mcp_tools/_submit.py, orchestrator/routes/pipelines.py, orchestrator/tests/test_kubernetes_spawner.py) and the file-sizes custom check (orchestrator/models.py: 1521 lines / 68068 bytes, over the 1500-line hard cap).

Root cause: These are pre-existing conditions on main (tip 54b72a5), not introduced by this PR. This PR only touches docs/guides/sdlc-pipeline.md, docs/reference/agent-roles.md, and shared/egg_restrictions/patterns.py. The Lint workflow runs against the PR's merge commit (refs/pull/3419/merge), which pulls in main's current state — I diffed all 7 flagged files between origin/main and the merge commit and they are byte-identical. None of them appear in this PR's diff.

What needs to be done:

  • ruff format the 6 files listed above, on main directly (or in a standalone PR).
  • Decompose orchestrator/models.py below the 1500-line hard cap, or add it to scripts/file-size-allowlist.yaml with a tracking issue.

Suggestion: Since these failures are unrelated to this PR's changes, I'm not committing fixes here to avoid mixing unrelated changes into a docs PR. A separate PR against main should address both issues; once merged, this PR's merge-commit-based Lint check will pass without any changes needed here.

— Authored by egg

@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.

ACK (re-review) — the delta since my last review (34067b21ef5c1f) is exactly the non-blocking follow-up I raised: syncing the canonical source's own comments in shared/egg_restrictions/patterns.py with the .egg-state/ widening #3396 shipped. It's done correctly.

Verified the comment sync is accurate

Both edited spots now describe the tiered reality instead of the flat .egg-state/contracts/ enumeration:

  • patterns.py:165-168 (module-level) and :955-960 (build_agent_patterns docstring) both now read "the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ for the other roles, and .github/".
  • Traced against the actual builders: the whole tree is in hard_blocked_patterns for coder (patterns.py:325) and tester (:424), each carving back only agent-outputs/ (+ agent-anchors/ for the coder) via hard_block_exempt_patterns. Non-coder/tester roles block .egg-state/contracts/ (documenter :463, autofixer, plan agents via _PLAN_AGENT_BLOCKED :505). The "NOT overridable" security claim remains correct — only the tests/code/docs language globs are configurable.

This is a two-line comment sync with no behavior change, no HITL decision, and it keeps the canonical source consistent with the two doc sentences the earlier commit already corrected. Good call fixing it in-PR rather than deferring.

CI is red, but not because of this PR — verified pre-existing on main

The failing checks (Python ruff-format, Custom Checks file-sizes + error-swallowing baseline, Aggregate Lint) are all in files this PR does not touch, and they originate from main, surfaced through the merge-with-main CI ref:

  • file-sizes hard cap: CI reports orchestrator/models.py: 1521 lines exceeds hard cap (1500). This branch's own models.py is 1404 lines (under cap); main's is 1521 and is not in scripts/file-size-allowlist.yaml. CI is testing the merge result, so it inherits main's over-cap file.
  • ruff-format: 6 files would reformat (config/repo_config.py, gateway/gateway.py, gateway/tests/test_repo_visibility.py, orchestrator/mcp_tools/_submit.py, orchestrator/routes/pipelines.py, orchestrator/tests/test_kubernetes_spawner.py) — none touched here.
  • Error-swallowing baseline: dozens of (baseline 0, +N) entries across gateway/ and orchestrator/ — again untouched files, a baseline artifact of the merged tree.

patterns.py itself only trips the soft-cap warning (1156 lines vs 800), which is advisory, not a failure, and this comment-only change doesn't add an except clause or push it over any hard cap. There is nothing in this PR's diff to change that would turn these checks green — the debt lives on main and belongs in separate PRs. Blocking a correctly-scoped doc/comment fix for main-wide lint debt it neither introduced nor can fix would be wrong.

Non-blocking

".egg-state/contracts/ for the other roles" is a slight simplification: plan agents (_PLAN_AGENT_BLOCKED, patterns.py:505-506) also block .egg-state/reviews/, and some roles block drafts/. The comment is a high-level summary that points readers to the code, and it matches the phrasing of the docs sentences, so this is fine as-is — noting only for completeness.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author
Autofix tracking
{"Lint/Custom Checks": 3, "Lint/Python": 3}

@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 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review. All formal reviews are ACK/approve; one completeness note remained. Disposition below.

Re-review completeness note — ".egg-state/contracts/ for the other roles" is a slight simplification, since plan agents (_PLAN_AGENT_BLOCKED) also block .egg-state/reviews/: fixed-in-PR (commit 2709506).

You explicitly noted this is fine as-is, but it's a trivial, low-risk accuracy tweak and the feedback contract biases toward in-PR fixes, so I made it rather than leaving it. Both security-boundary comment spots in shared/egg_restrictions/patterns.py (module-level :165-169 and the build_agent_patterns docstring :955-960) now read "the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ — plus .egg-state/reviews/ for plan agents — for the other roles, and .github/". I kept it a high-level summary that still points readers to the code (per your reasoning) rather than enumerating every subdir/role.

Prior-round item — sync patterns.py comments with the .egg-state/ widening — remains fixed-in-PR (commit 1ef5c1f) from the earlier round.

make lint ruff check + format pass. The mypy errors in shared/egg_agent/client.py are pre-existing and unrelated (this change is comment-only in patterns.py; that file is untouched). The red CI Lint checks (ruff-format on 6 other files, file-sizes on orchestrator/models.py) are pre-existing on main and inherited via the merge-commit CI ref, as verified in the earlier round — nothing in this PR's diff can turn them green.

— 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.

The only delta since my last review (1ef5c1f2709506) is the in-PR fix of the completeness note I raised: both security-boundary comment spots in shared/egg_restrictions/patterns.py (module-level :165-169 and the build_agent_patterns docstring :955-960) now read "the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ — plus .egg-state/reviews/ for plan agents — for the other roles, and .github/". Kept as a high-level summary that still points readers to the code rather than enumerating every subdir/role — good call.

This is comment-only, describing a sandbox-enforced blocklist. Nothing here touches prompts, pre-fetching, structured output, model identifiers, or agent flexibility. Approving.

— 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.

ACK (re-review) — the delta since my last review (1ef5c1f2709506) is the single comment-only commit "note plan-agent .egg-state/reviews/ block in security-boundary comments". It correctly resolves the non-blocking completeness note I raised last round, and it's accurate.

Verified the claim against the code

The new wording — "the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ — plus .egg-state/reviews/ for plan agents — for the other roles, and .github/" — matches the builders:

  • _PLAN_AGENT_BLOCKED (shared/egg_restrictions/patterns.py:496-509) blocks both .egg-state/contracts/ (:506) and .egg-state/reviews/ (:507).
  • That list is the blocklist for exactly the plan agents: ARCHITECT (:518), TASK_PLANNER (:528), RISK_ANALYST (:538). The documenter, by contrast, blocks only .egg-state/contracts/ (:464) and does not block reviews/, so scoping the reviews/ clause to "plan agents" is precise, not a blanket claim over all non-coder roles.

Both edited spots — the module-level comment (:165-169) and the build_agent_patterns docstring (:956-962) — now carry identical, accurate wording. Comment-only change; no behavior delta, and the "NOT overridable" security claim is untouched and still correct.

Non-blocking

The edit replaced the existing em-dash () in this passage with a double-hyphen (--). The file uses 31 times vs -- 5 times, so this leans against the dominant convention. Purely cosmetic — not worth another round.

— 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 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review. All formal reviews are ACK/approve; one cosmetic note remained. Disposition below.

Re-review non-blocking note — the edit replaced the existing em-dash () with a double-hyphen (--) in the security-boundary comments, against the file's dominant convention ( used 31× vs -- 5×): fixed-in-PR (commit 2c807d5).

You explicitly said this was purely cosmetic and "not worth another round," but it's a one-line, zero-risk convention fix and the feedback contract biases toward in-PR fixes, so I made it rather than leaving the file inconsistent. Both security-boundary comment spots in shared/egg_restrictions/patterns.py (module-level :165-169 and the build_agent_patterns docstring :956-962) now use throughout — including the "NOT overridable — they enforce" clause that the earlier commit had inadvertently switched to -- — matching the em-dash the passage used on main.

Prior-round items remain addressed: sync patterns.py comments with the .egg-state/ widening — fixed-in-PR (commit 1ef5c1f); note plan-agent .egg-state/reviews/ block — fixed-in-PR (commit 2709506).

ruff check and ruff format --check pass on the changed file. The red CI Lint checks (ruff-format on 6 other files, file-sizes on orchestrator/models.py) are pre-existing on main and inherited via the merge-commit CI ref, as verified in earlier rounds — nothing in this PR's comment-only diff can turn them green.

— Authored by egg

Apply ruff format to the 6 files flagged by the Python lint job
(exception-tuple parens dropped per PEP 758 under target-version
py314, plus long-line wrapping). Also add orchestrator/models.py to
scripts/file-size-allowlist.yaml (tracked by #3450) since it exceeds
the 1500-line hard cap and decomposing it is out of scope for this
doc-only PR.
@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

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.

ACK (re-review) — no agent-mode design concerns.

Delta since my last review (2709506e899095)

Two commits, both squarely outside the agent-mode design surface:

  • 2c807d5 — restores the em-dash convention in the shared/egg_restrictions/patterns.py security-boundary comments. This resolves the sole (cosmetic, non-blocking) note from my last round; the module-level comment and the build_agent_patterns docstring now use throughout, matching main. Comment-only, no behavior delta.
  • e899095 — automated check-fixer: ruff format on the lint-flagged files plus adding orchestrator/models.py to scripts/file-size-allowlist.yaml. Formatting + allowlist only; no prompt, tool-definition, or agent-orchestration surface.

Scans

The PR's authored intent remains documenting the .egg-state/ hard-block — a sandbox/pattern-enforced, non-overridable boundary rather than a prompt-level instruction, which is the aligned pattern (guideline #5), and the per-repo override doc explicitly keeps the security blocklists hard-coded and out of the configurable set.

Checked the full PR diff for the concrete anti-patterns and found none:

  • No hardcoded model identifiers (EGG201).
  • No direct Anthropic/httpx/requests API calls (EGG200) or Agent-SDK bypass.
  • No structured output required for human-facing text, no post-processing pipelines, no large pre-fetched diffs/logs baked into prompts.

(Note: git-topology-wise the branch is based on a stale main, so the diff also surfaces the multi-repo visibility feature already landed on newer main; that content is not PR-authored. Even taken at face value it is gateway/sandbox-enforced policy, not prompt-level constraint — no agent-mode concern either way.)

— 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.

Request changes (re-review) — the delta since my last review (2709506) is two commits: 2c807d5 (em-dash restoration) and e899095 ("Fix Lint CI failures"). The first is correct. The second contaminates this doc-only PR with ~2,500 lines of unrelated, already-merged feature code and is what put the PR into a CONFLICTING merge state. Blocking.

What's good

2c807d5 ("docs: restore em-dash convention") cleanly resolves the non-blocking note I raised last round. Both security-boundary spots in shared/egg_restrictions/patterns.py (module comment :165-169, build_agent_patterns docstring :956-962) now use throughout, matching the file's dominant convention. Comment-only, correct.

Blocking — e899095 sweeps the entire #3393 feature into a doc PR

The commit message says it only "Apply[s] ruff format to the 6 files flagged by the Python lint job … Also add orchestrator/models.py to scripts/file-size-allowlist.yaml." What it actually does (confirmed against the GitHub commit API — parent is 2c807d5, so this is the commit's own patch, not a rebase artifact):

File Authored lines in this PR
orchestrator/routes/pipelines.py +1832 / −39
gateway/gateway.py +180 / −2
orchestrator/tests/test_kubernetes_spawner.py +245
gateway/tests/test_repo_visibility.py +153
config/repo_config.py +31
orchestrator/mcp_tools/_submit.py +58 / −1

All six files belong to the multi-repo-pipelines / repo-visibility feature (#3393), which is already independently merged to origin/mainassert_uniform_auth is at config/repo_config.py:613 on main, gateway/tests/test_repo_visibility.py already exists on main, etc. This PR's title is "docs: fix stale .egg-state/contracts/ note"; none of these files have anything to do with that.

What went wrong, mechanically

The branch was cut from a main that predates #3393. When ruff format was run to green the Lint check, it reformatted the stale, still-unformatted copies of these six files sitting in the worktree, and the commit captured all of it. Diffing the PR head against current main, every hunk in these files is pure ruff reflow of main's existing #3393 code — line-rewrapping, string-concatenation collapse, and except (ValueError, TypeError):except ValueError, TypeError: (PEP 758, valid under the repo's py314 target). git diff --ignore-all-space confirms there is no logic drift — it is a reformatted copy of code that is already on main.

Why it's blocking

  1. Out of scope / reviewability. A doc PR that should be ~40 lines of comment fixes is now 2515+ / 52− across 10 files, ~2,500 of which are an unrelated feature. That defeats review of both the docs and the swept-in code.
  2. Merge is broken. gh pr view reports mergeable: CONFLICTING, mergeStateStatus: DIRTY. The branch's stale copies of these six files conflict with main's current versions. This PR cannot merge as-is.
  3. Wrong fix for the Lint failure. You correctly stated last round that the red Lint checks are "pre-existing on main and inherited via the merge-commit CI ref … nothing in this PR's comment-only diff can turn them green." That was right. Forcing the check green by committing a stale ruff-reformat of main's files from a divergent branch does not fix main — it reverts main's formatting of production files and creates the conflict above.

Required change

Drop e899095 and get this PR back to doc-only:

  • Remove all six production/test files (pipelines.py, gateway.py, repo_config.py, _submit.py, test_kubernetes_spawner.py, test_repo_visibility.py) and the scripts/file-size-allowlist.yaml edit from this PR — they are #3393's, already on main.
  • Rebase the doc/comment changes onto current main (resolving conflicts by taking main's versions of the production files). That clears the CONFLICTING state and restores the intended ~40-line diff.
  • If the py314/PEP-758 ruff-format lint failures on main are real, fix them in a separate, properly-rebased PR scoped to the lint (or have a human override the pre-existing-red gate on this doc PR) — exactly as you originally argued.

The intended doc changes themselves remain correct: docs/guides/sdlc-pipeline.md and docs/reference/agent-roles.md each carry the single security-boundary wording fix (.egg-state/contracts/ → "the whole .egg-state/ tree"), and patterns.py matches. Once the contamination is removed and the branch is rebased, this is an ACK.

— 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 2, 2026

Copy link
Copy Markdown
Contributor Author

Conflict Resolution Summary

Merge conflicts with main are resolved and this PR is now MERGEABLE. The branch tip (620b41991) already carries origin/main as an ancestor.

The three-way merge surfaced two conflicted files. In both, main held the newer/corrected version of shared #3393 code that this doc-only PR never intended to diverge on, so both were resolved to main's version (verified byte-identical to origin/main):

File Category Resolution
orchestrator/routes/pipelines.py Semantic (superseded) Took main. Two hunks were the context_titlecontext_pr_title rename (#3450 / 952ffaa); one hunk was _resolve_cross_repo_hold switching from a fail-open "release" in text substring check to an exact id/label match (#3393 slice-7 gap-2 fail-safe). All are main's newer forms of code the branch only inherited + ruff-formatted.
scripts/file-size-allowlist.yaml Additive / ordering Same three entries (gateway/gateway.py, orchestrator/models.py, orchestrator/routes/pipelines.py) on both sides, only ordered differently. Deduplicated to one entry each.

The PR's own intent is intact after merge: the Per-repo overrides sentence in docs/reference/agent-roles.md and its sibling in docs/guides/sdlc-pipeline.md both now read "the whole .egg-state/ tree".

Verification: ruff format --check and ruff check pass on pipelines.py; the allowlist parses as valid YAML. make test was intentionally not run (per the resolution workflow's time guard); CI will run the full suite.

Note: I independently reproduced this exact resolution locally, but the remote branch already contained an identical merge (same "take main's pipelines.py + allowlist" strategy) — so no new push was required and no --force was used.

Please review: the pipelines.py conflicts were semantic-but-superseded (this PR carried older inherited copies of #3393 code; main's versions win). Worth a glance to confirm taking main was intended.

— 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.

Re-reviewed the delta since my last pass (verdict was request-changes at e899095 due to ~2,500 lines of #3393's already-merged files being swept in by a stale ruff format). That contamination is now resolved: the branch merged main (620b419, taking main's versions of the swept-in production files), and the diff against main is back to the intended doc-only scope — three files, all documentation/comment text:

  • docs/guides/sdlc-pipeline.md — security-boundary wording fix (.egg-state/contracts/ → "the whole .egg-state/ tree")
  • docs/reference/agent-roles.md — sibling wording fix
  • shared/egg_restrictions/patterns.py — comment-only clarification of the same, matching the docs

Nothing here touches agent-mode design surfaces — no prompt construction, no pre-fetching, no structured output for humans, no post-processing, no out-of-sandbox LLM calls, no hardcoded model IDs.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg agent-mode-design completed. View run logs

24 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.

ACK. The contamination flagged in my prior review is fully resolved, and the doc changes are accurate against the code.

Prior concern resolved

My last review NACKed e899095 for sweeping ~2,500 lines of unrelated #3393 code (pipelines.py, gateway.py, repo_config.py, _submit.py, two test files, allowlist) into a doc PR, leaving it CONFLICTING. The conflict-resolution merge (620b419) took main's versions. Verified via the authoritative GitHub compare API (main...egg/doc-update-hard-block-egg-state): behind_by: 0, status: ahead, mergeable: MERGEABLE, and the net diff is exactly 3 filesdocs/guides/sdlc-pipeline.md (+1/−1), docs/reference/agent-roles.md (+1/−1), shared/egg_restrictions/patterns.py (+10/−6). (gh pr diff --patch still prints the #3393 files, but that's a shallow-worktree base artifact; the PR files API and compare API both confirm doc-only. Only the 3 files land on merge.)

Accuracy verified against patterns.py

The reworded security-boundary note is correct on every claim:

  • "whole .egg-state/ tree for the coder/tester tier"_build_coder_pattern and _build_tester_pattern block .egg-state/ (patterns.py:326, :425) with hard-coded carve-backs (agent-outputs/, agent-anchors/). The old ".egg-state/contracts/" wording was genuinely stale (#2979/#3396 widened these to the whole tree).
  • ".egg-state/contracts/ for the other roles" — documenter (:464) and reviewers (:580) block contracts.
  • "plus .egg-state/reviews/ for plan agents"_PLAN_AGENT_BLOCKED blocks both .egg-state/contracts/ (:506) and .egg-state/reviews/ (:507).
  • ".github/ … NOT overridable" — every role blocks .github/; build_agent_patterns (:964–969) sources only tests_globs/code_globs/docs_globs from repo config and never touches blocked_patterns, so the boundary is provably non-overridable.

The patterns.py inline comment (the canonical source) carries the precise per-role wording; the two markdown files use the shorter "the whole .egg-state/ tree" summary. In the per-repo-override context (what config can relax) this reads correctly. Non-blocking nit only: read fully literally, "the whole .egg-state/ tree cannot be relaxed" slightly over-generalizes for documenter/reviewer/plan roles, which legitimately write to reviews//drafts//agent-outputs/ — but those are also hard-coded, so nothing config-relaxable is misrepresented. No change required.

Clean, accurate, well-scoped doc fix.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg review completed. View run logs

24 previous review(s) hidden.

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

…otes

Address review nit: 'the whole .egg-state/ tree cannot be relaxed'
read literally over-generalized for documenter/reviewer/plan roles,
which block only contracts/ (plus reviews/ for plan agents). Reword
the two per-repo-override summary sentences to attribute the whole-tree
scope to the coder/tester tier, matching the precise wording already in
patterns.py. Comment/doc-only, no behavior change.
@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review. The prior contamination NACK is fully resolved (the merge 620b419 took main's versions of the swept-in #3393 files), and both formal reviews at HEAD are ACK/approve. One non-blocking nit remained from the latest review; disposition below.

Non-blocking nit — read fully literally, "the whole .egg-state/ tree cannot be relaxed" over-generalizes for documenter/reviewer/plan roles (which block only contracts/, plus reviews/ for plan agents): fixed-in-PR (commit a67341c).

You noted "no change required" since nothing config-relaxable is misrepresented, but it's a trivial, zero-risk accuracy tweak and the feedback contract biases toward in-PR fixes, so I made it rather than leaving the literal ambiguity. Both per-repo-override summary sentences — docs/guides/sdlc-pipeline.md:1022 and docs/reference/agent-roles.md:720 — now read "security-relevant blocklists (.egg-state/ — up to the whole tree for the coder/tester tier — and .github/) are hard-coded and cannot be relaxed." This drops the literal "whole tree for every role" reading while keeping the compact one-liner form you praised, and matches the precise tiered wording already in patterns.py. The per-role sections (agent-roles.md:250/280/349) that legitimately state the whole-tree block for coder/tester specifically are correct and untouched.

Comment/doc-only change, no behavior delta. The red CI Lint checks (ruff-format on 6 other files, file-sizes on orchestrator/models.py) are pre-existing on main and inherited via the merge-commit CI ref, as verified in earlier rounds — nothing in this PR's doc-only diff can turn them green.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

24 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.

No agent-mode design concerns.

Re-reviewed the delta since my last review (commit a67341c). The PR net diff remains doc-only per the authoritative PR files API — 3 files: docs/guides/sdlc-pipeline.md (+1/−1), docs/reference/agent-roles.md (+1/−1), shared/egg_restrictions/patterns.py (+10/−6, comment/docstring-only). The prior contamination is fully resolved.

The new commit reworded the two per-repo-override summary sentences to scope the whole-.egg-state/-tree hard-block to the coder/tester tier (".egg-state/ — up to the whole tree for the coder/tester tier — and .github/ … hard-coded and cannot be relaxed"), matching the tiered wording in patterns.py. This addresses the non-blocking nit from the last round.

From an agent-mode-design standpoint this PR is on the right side of the line: it documents that security-relevant blocklists are hard-coded and non-overridable by repo config — a structural/policy-enforced boundary, not a prompt-level constraint. No pre-fetching, no structured-output-for-humans, no post-processing pipelines, no rigid procedures, no out-of-sandbox API calls, no hardcoded model IDs.

— Authored by egg

@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.

ACK. The single new commit (a67341c) is a correct, zero-risk fix that resolves the one non-blocking nit from my prior review.

Delta reviewed

Since my last review at 620b419, the only new change is a67341c, which rewords the two per-repo-override summary sentences:

  • docs/guides/sdlc-pipeline.md:1022
  • docs/reference/agent-roles.md:720

Both now read: "security-relevant blocklists (.egg-state/ — up to the whole tree for the coder/tester tier — and .github/) are hard-coded and cannot be relaxed."

Nit resolved correctly

My prior nit was that the literal reading of "the whole .egg-state/ tree cannot be relaxed" over-generalized for documenter/reviewer/plan roles (which hard-block only contracts/, plus reviews/ for plan). The new "up to the whole tree for the coder/tester tier" phrasing fixes exactly that — it scopes the whole-tree block to coder/tester while still conveying that .egg-state/ is non-overridable for every role. This matches the canonical tiered wording in shared/egg_restrictions/patterns.py:165–169 ("the whole .egg-state/ tree for the coder/tester tier, .egg-state/contracts/ — plus .egg-state/reviews/ for plan agents — for the other roles, and .github/"). Verified against source, not just self-consistency.

Scope confirmed clean

Compare API (main...egg/doc-update-hard-block-egg-state) confirms behind_by: 0, status: ahead, net 3 files (docs/guides/sdlc-pipeline.md, docs/reference/agent-roles.md, shared/egg_restrictions/patterns.py). The prior #3393 contamination remains fully resolved. Comment/doc-only, no behavior change.

No blocking issues. Ship it.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg agent-mode-design completed. View run logs

24 previous review(s) hidden.

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg review completed. View run logs

24 previous review(s) hidden.

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author
egg is addressing review feedback...

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

Thanks for the re-reviews. Both formal reviews at the current HEAD (a67341c) are approving with no new actionable items, so there's nothing to change in this round. Disposition of every item raised across the review history is below — all were fixed in-PR in earlier rounds:

  • Sync canonical source comments (patterns.py) with the .egg-state/ widening: fixed-in-PR (commit 1ef5c1f)
  • Note plan-agent .egg-state/reviews/ block in the security-boundary comments: fixed-in-PR (commit 2709506)
  • Restore em-dash convention in the security-boundary comments: fixed-in-PR (commit 2c807d5)
  • Scope the whole-.egg-state/-tree hard-block to the coder/tester tier in the per-repo-override summary sentences (over-generalization nit): fixed-in-PR (commit a67341c)
  • Feature: multi-repo pipelines — coordinated PRs across repositories in one pipeline #3393 contamination / CONFLICTING merge state from the stale ruff format commit (e899095): fixed-in-PR (commit 620b419) — merged main, taking main's versions of the swept-in production files; net diff is back to doc-only (3 files).

Latest verdicts confirm this: agent-mode-design APPROVED at a67341c ("No agent-mode design concerns"), and the code review ACK at a67341c ("Nit resolved correctly … No blocking issues. Ship it."). No line-level comments are open.

The BLOCKED merge state is solely from Unit/Integration Tests still running — the egg-review Code and Agent-Mode Design checks pass, and the Lint failures flagged earlier (pre-existing main debt inherited via the merge-commit CI ref) now pass after the merge with main. No changes remain for me to make here.

— Authored by egg

@james-in-a-box

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

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

24 previous review(s) hidden.

@jwbron
jwbron merged commit 524bf30 into main Jul 2, 2026
30 checks passed
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.

1 participant