Skip to content

docs: Update readonly mount docs for reviewer exemption [doc-updater] - #825

Merged
jwbron merged 2 commits into
mainfrom
egg/doc-update-reviewer-exemption
Feb 20, 2026
Merged

docs: Update readonly mount docs for reviewer exemption [doc-updater]#825
jwbron merged 2 commits into
mainfrom
egg/doc-update-reviewer-exemption

Conversation

@james-in-a-box

Copy link
Copy Markdown
Contributor

docs: Update readonly mount docs for reviewer exemption [doc-updater]

Update documentation to reflect changes from e5f7d11 (Fix feedback iteration limit never enforcing #820):

What changed:

  • Reviewer agents (roles starting with "reviewer") are now exempted from .egg-state/reviews/ readonly mount during implement phase
  • They need write access to create verdict files in the reviews directory
  • Other implement phase agents (coder, tester, documenter, integrator) still have readonly access

Documentation updates:

  • docs/architecture/orchestrator.md: Updated phase-based readonly mounts table and added explanation of reviewer exemption
  • docs/guides/sdlc-pipeline.md: Clarified filesystem-level readonly mount principle to mention the reviewer exception

Triggered by: e5f7d11b

Authored-by: egg

Update documentation to reflect changes from e5f7d11 (Fix feedback
iteration limit never enforcing):

- Reviewer agents are now exempted from .egg-state/reviews/ readonly
  mount during implement phase, as they need to write verdict files
- Updated orchestrator.md to clarify the exemption in the phase-based
  readonly mounts table and explanation
- Updated sdlc-pipeline.md to mention the reviewer exception in the
  structural enforcement principles

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.

No agent-mode design concerns.

This is a documentation-only change that accurately describes the reviewer exemption from .egg-state/reviews/ readonly mounts. The underlying mechanism aligns well with agent-mode design principles — the constraint (readonly mounts) and its exemption (for reviewer roles) are enforced at the filesystem/sandbox level, not via prompt instructions.

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

Docs-only change. Verified all claims against the implementation.

Verification summary:

  1. phase_readonly_mounts() exemption (shared/egg_container/__init__.py:230-239): Confirmed. Reviewer roles (identified by agent_role.startswith("reviewer")) skip the reviews directory in the readonly mount list. Matches the doc update in orchestrator.md.

  2. ensure_egg_state_dirs() marker skip (shared/egg_container/__init__.py:165-178): Confirmed. The .egg-readonly marker file is not placed in reviews/ for reviewer agents. Matches the added sentence in orchestrator.md line 99.

  3. Reviewer role examples (orchestrator/models.py:80-84): reviewer_code and reviewer_contract both exist as AgentRole enum values. Examples in the docs are accurate.

  4. sdlc-pipeline.md change: The old text listed all four directories as unconditionally readonly. The new text correctly separates reviews/ from the other three and explains the reviewer exception. Accurate.

No issues found. The documentation accurately reflects the code behavior introduced in e5f7d11.

One minor suggestion (non-blocking): In orchestrator.md line 97, the parenthetical (roles starting with "reviewer", e.g., reviewer_code, reviewer_contract) — consider adding backtick formatting around the role names for consistency with the rest of the doc (they appear as inline code in the surrounding paragraphs).

— 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 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Changed "reviewer" to backtick-formatted reviewer in orchestrator.md for consistency with the surrounding inline code formatting. Pushed in 8149e89.

— 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-review delta is a single formatting fix ("reviewer"`reviewer`) addressing the previous reviewer's inline code consistency suggestion. No substantive changes since last review. Previous approval still applies.

— 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-reviewed delta: single commit 8149e89e — formatting fix only.

Previous feedback addressed: The code-correctness reviewer suggested backtick formatting for reviewer in the parenthetical on line 97 of orchestrator.md. This has been correctly applied: "reviewer"`reviewer`. The reviewer_code and reviewer_contract examples already had backtick formatting from the original commit, so the parenthetical is now fully consistent.

Verification against implementation: Re-confirmed shared/egg_container/__init__.py:165-239startswith("reviewer") prefix check, marker file skip for reviewer roles, and readonly mount exemption all match the documentation.

No issues found. Previous approval still holds.

— Authored by egg

@james-in-a-box

This comment has been minimized.

@james-in-a-box

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

@james-in-a-box

Copy link
Copy Markdown
Contributor Author

egg feedback addressed. View run logs

6 previous review(s) hidden.

@jwbron
jwbron merged commit e77124e into main Feb 20, 2026
27 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