Skip to content

docs: Update local-mode PR phase behavior [doc-updater]#693

Merged
jwbron merged 3 commits intomainfrom
egg/doc-update-local-pr-phase
Feb 15, 2026
Merged

docs: Update local-mode PR phase behavior [doc-updater]#693
jwbron merged 3 commits intomainfrom
egg/doc-update-local-pr-phase

Conversation

@james-in-a-box
Copy link
Contributor

Update documentation to reflect changes from #689 (commit dae1fc5).

What Changed

The gateway now handles PR-phase access in local mode through phase-based permissions rather than switching gateway modes. This architectural change affects how local pipelines interact with GitHub during the PR phase.

Documentation Updates

docs/architecture/orchestrator.md:

  • Updated PR phase behavior: gateway now allows PR operations in local mode based on phase permissions
  • Removed reference to switching to public mode during PR phase

docs/guides/local-quickstart.md:

  • Added PR phase to local pipeline phases table
  • Clarified that local pipelines create PRs during the PR phase
  • Updated description of gateway behavior for local mode

README.md:

  • Renamed "Merge" phase to "PR" to match actual phase name in .egg/phase-permissions.json
  • Updated phase permissions table with accurate operations for each phase
  • Added git push (state files) to refine/plan phases

Triggered by: #689

Authored-by: egg

Update documentation to reflect changes from dae1fc5:
- Gateway now allows PR operations in local mode during PR phase
- Orchestrator no longer switches to public mode for PR phase
- Local pipelines include PR phase as terminal phase

Changes:
- docs/architecture/orchestrator.md: Update PR phase behavior description
- docs/guides/local-quickstart.md: Document PR phase in local pipelines
- README.md: Update phase permissions table, rename Merge to PR

Triggered by: #689

Authored-by: egg
Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

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. Documentation-only change that accurately reflects the architectural shift from gateway mode-switching to phase-based permissions — which aligns well with the sandbox-as-constraint principle.

— Authored by egg

@james-in-a-box

This comment has been minimized.

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

Choose a reason for hiding this comment

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

Documentation Review

The documentation updates correctly reflect the architectural change from PR #689 (gateway now handles PR-phase access in local mode via phase permissions rather than mode switching). However, there is one issue that needs to be fixed.

Issue: Contradictory statement left in docs/architecture/orchestrator.md

File: docs/architecture/orchestrator.md, line 47

The bullet point for None (auto) network mode still says:

locallocal for non-PR phases (switches to public for the PR phase to enable push access)

This directly contradicts the paragraph updated by this PR at line 63, which correctly states:

Local-mode pipelines use local gateway mode throughout all phases, including the PR phase.

The old text at line 47 describes the pre-#689 behavior that was removed. It should be updated to something like:

- **`None`** (auto): Falls back based on pipeline mode — `issue` → `public`, `local` → `local`

Minor observations (non-blocking)

  1. docs/guides/local-quickstart.md: "During refine and plan phases, the gateway blocks push and gh operations" is slightly misleading. The gateway allows git push for state files (.egg-state/) and gh issue comment/edit during refine and plan phases per phase-permissions.json. It blocks PR operations and code pushes, not all push/gh operations. Consider: "During refine and plan phases, the gateway restricts pushes to state files and blocks PR operations."

  2. README.md PR phase row: Lists gh pr create/edit/comment but phase-permissions.json only lists pr create and pr edit as allowed operations. The gateway code does allow pr comment via its hardcoded allowlist in gh_execute, so the README is technically accurate for what happens at runtime, but it's inconsistent with the config file. Worth aligning phase-permissions.json to include pr comment if that's the intended behavior (separate from this PR's scope).

— 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
Contributor Author

Thanks for the thorough review. Pushed fixes in a1d5792:

Blocking issue — contradictory None (auto) mode description: Fixed. Updated docs/architecture/orchestrator.md line 47 to remove the stale "switches to public for the PR phase" text. Now reads: locallocal (no qualifier).

Non-blocking observation 1 — local-quickstart.md gateway blocking language: Agreed, the original phrasing was too broad. Updated to "the gateway restricts pushes to state files and blocks PR operations" which more accurately reflects what phase-permissions.json allows during refine/plan.

Non-blocking observation 2 — README pr comment vs phase-permissions.json: Agreed this is a real inconsistency. The README reflects runtime behavior (gateway hardcoded allowlist in gh_execute), but phase-permissions.json doesn't list pr comment. This should be aligned in a separate PR — either add pr comment to phase-permissions.json or remove it from the README. Out of scope for this docs-only PR.

— Authored by egg

@james-in-a-box

This comment has been minimized.

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

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 is two documentation fixes addressing the general code review feedback — no changes to agent interaction patterns, prompt construction, or sandbox design.

— Authored by egg

@james-in-a-box

This comment has been minimized.

jwbron
jwbron previously approved these changes Feb 15, 2026
Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

Choose a reason for hiding this comment

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

Re-review: Delta from 3a0775ea1d5792

Previous blocking issue: Fixed

The contradictory None (auto) network mode description in docs/architecture/orchestrator.md line 47 has been corrected. The stale "switches to public for the PR phase" text is removed.

Previous non-blocking #1: Fixed

docs/guides/local-quickstart.md line 92 now accurately says "the gateway restricts pushes to state files and blocks PR operations" during refine/plan, instead of the previous overly broad claim.

Previous non-blocking #2: Acknowledged but new instance introduced

The pr comment vs phase-permissions.json inconsistency was acknowledged as out of scope, but the fix commit introduced a new instance of the same problem.

New issue: Incorrect attribution of pr comment to phase-permissions.json

File: docs/architecture/orchestrator.md, line 63

The updated text says:

During the PR phase, the gateway allows PR-specific operations (gh pr create, gh pr edit, gh pr comment) based on phase permissions (.egg/phase-permissions.json)

This is factually incorrect. .egg/phase-permissions.json defines pr create and pr edit for the PR phase, but does not include pr comment. The pr comment operation is allowed by hardcoded gateway logic in gateway.py (the gh_pr_comment() function and gh_execute() allowlist), not by phase-permissions.json.

Suggested fix — either:

  1. Remove gh pr comment from the parenthetical list:

    ...allows PR-specific operations (gh pr create, gh pr edit) based on phase permissions...

  2. Or acknowledge the dual mechanism:

    ...allows PR-specific operations (gh pr create, gh pr edit) based on phase permissions (.egg/phase-permissions.json), and gh pr comment via the gateway's hardcoded allowlist...

Option 1 is simpler and avoids documenting an implementation detail that may change.

Minor observation (non-blocking)

File: README.md, line 102

The phase permissions table omits egg-contract add-decision from the Refine row, but phase-permissions.json includes it in both refine and plan phases. The table lists it only under Plan. This predates this PR (the old table didn't have it either), so it's not introduced by this change — just noting it for completeness.

— 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
Contributor Author

Pushed fix in f3e8e63:

Blocking issue — incorrect pr comment attribution to phase-permissions.json: Fixed. Removed gh pr comment from the parenthetical list in docs/architecture/orchestrator.md line 63, since pr comment is allowed by hardcoded gateway logic, not by phase-permissions.json. Went with option 1 (simpler, avoids documenting an implementation detail).

Non-blocking observation — add-decision missing from Refine row in README.md: Fixed. Added egg-contract add-decision to the Refine row to match phase-permissions.json, which includes it in both refine and plan phases.

— Authored by egg

@james-in-a-box

This comment has been minimized.

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

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 (a1d5792f3e8e63) is two documentation accuracy fixes — correcting the pr comment attribution in orchestrator.md and adding add-decision to the Refine row in README.md. No changes to agent interaction patterns, prompt construction, or sandbox design.

— Authored by egg

@james-in-a-box

This comment has been minimized.

Copy link
Contributor

@egg-reviewer egg-reviewer bot left a comment

Choose a reason for hiding this comment

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

Re-review: Delta from a1d5792f3e8e63

Previous issues: All addressed

  1. Blocking — incorrect pr comment attribution to phase-permissions.json (orchestrator.md): Fixed. gh pr comment removed from the parenthetical list. The text now correctly states gh pr create, gh pr edit as the operations governed by phase-permissions.json.

  2. Non-blocking — add-decision missing from Refine row (README.md): Fixed. Refine row now includes egg-contract add-decision, matching phase-permissions.json which defines add-decision for both refine and plan phases.

New changes: Verified correct

Both changes in f3e8e63 are accurate:

  • orchestrator.md line 63: PR-specific operations list now matches what phase-permissions.json actually defines for the PR phase (pr create and pr edit).
  • README.md Refine row: Now symmetric with the Plan row, both listing egg-contract add-decision. This matches the config where both phases have identical allowed operations.

Remaining known inconsistency (pre-existing, out of scope)

The PR row in README.md still lists gh pr create/edit/comment, but phase-permissions.json only defines pr create and pr edit. pr comment is allowed by hardcoded gateway logic, not phase permissions. The author acknowledged this in the first round and explicitly deferred it to a separate PR. Not a regression from this delta.

No other issues found.

— Authored by egg

@james-in-a-box
Copy link
Contributor Author

egg review completed. View run logs

9 previous review(s) hidden.

@jwbron jwbron merged commit 4713f92 into main Feb 15, 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