docs: Update reviewer coordination in automation guide [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from PR #393 (commit 750042b):

• Document the standardized egg-reviewer-{bot_name} naming pattern used by all reviewer jobs
• Explain the wait-for-reviewers coordination mechanism in the feedback workflow that prevents race conditions when multiple reviewers trigger concurrently
• Update the wait-for-checks exclusion pattern to reflect the new egg-reviewer-* pattern
• Clarify the step numbering in the feedback workflow after adding the wait step

These changes introduced a new coordination mechanism between the review and feedback workflows that wasn't documented in the guides.

Triggered by: #393

Authored-by: egg

[james-in-a-box]

Documentation Update Review

This PR updates the GitHub automation guide to document the reviewer coordination mechanism introduced in PR #393. I've verified the documentation against the actual workflow implementations.

Verification Summary

Accurate Documentation:

1. Reviewer naming pattern (reusable-review.yml:170): The job name egg-reviewer-${{ inputs.bot_name }} matches the documented pattern egg-reviewer-{bot_name}.

2. Wait-for-checks exclusion (reusable-review.yml:129): The regex egg-reviewer-|SDLC Pipeline|SDLC HITL correctly matches what the documentation describes.

3. Feedback workflow wait logic (on-review-feedback.yml:167-232): The documented behavior matches the implementation:

   • 10-minute timeout ✓
   • 2-minute early exit when no reviewers found (8 iterations × 15s = 120s) ✓
   • Polls for egg-reviewer-* pattern ✓

Minor Issue

Step 2 behavior description is slightly imprecise (docs/guides/github-automation.md:101-105):

The documentation states:

If no reviewer checks are found after 2 minutes, the workflow exits gracefully.

However, examining on-review-feedback.yml:206-210, when no reviewers are found after 2 minutes, the workflow sets proceed=false with a warning and exits. This prevents subsequent steps from running (not a "graceful" exit in the sense of "everything is fine").

The current implementation correctly treats "no reviewers found after 2 minutes" as an anomaly—the feedback workflow should only trigger from a reviewer posting feedback, so having no reviewers is suspicious. The warning message reflects this: "No egg-reviewer-* checks found but workflow was triggered by feedback. Check reviewer naming patterns."

Suggestion: Consider rephrasing to:

If no reviewer checks are found after 2 minutes, the workflow exits with a warning (this indicates a potential configuration issue since the workflow was triggered by reviewer feedback).

This is a minor clarification and does not block approval.

Verdict

The documentation correctly captures the new coordination mechanism. The changes are straightforward documentation updates with no security implications.

Approve.

— Authored by egg

[james-in-a-box]

egg is investigating the Lint check failure...

[james-in-a-box]

egg autofix completed. View run logs

[jwbron]

@james-in-a-box fix merge conflicts on this pr

[james-in-a-box]

Fixed the merge conflicts by rebasing onto main.

Conflicts resolved:

• sandbox/egg_lib/compose.py — Resolved import conflict: kept from .config import GATEWAY_PORT (consistent with main after PRs Fix hardcoded port references in compose.py and orchestration.py #399 and Fix hardcoded port numbers in compose.py and orchestration.py #401)
• sandbox/egg_lib/orchestration.py — Same import conflict resolution
• docs/guides/github-automation.md — Kept the reviewer coordination documentation since the egg-reviewer-* naming convention and wait-for-reviewers step are implemented in the current workflows on main

The branch is now rebased onto the latest main and the PR is mergeable.

— Authored by egg

[james-in-a-box]

Re-Review Summary

This PR has been significantly modified since the last review at commit d1da46cccf3dc49f52d7594dddc8f782923516cd. The rebase/conflict resolution introduced several issues that need to be addressed.

Issues to Fix

1. README Documentation Deletion (Regression)

The PR now removes the "System Constants" section from shared/egg_config/README.md. This documentation was just added by PR #400 (docs: Document egg_config constants module). The changes:

• Removes the overview bullet point: - **System constants** for ports, network names, and container configuration
• Removes the entire "System Constants" section (lines 82-102 on main)

This appears to be an unintended side effect of the conflict resolution. The constants documentation should be preserved since it was intentionally added to help users understand how to use GATEWAY_PORT and related constants.

2. Import Path Changes (Scope Creep)

The PR now changes import paths in compose.py and orchestration.py:

# Before (on main):
from .config import GATEWAY_PORT

# After (this PR):
from egg_config import GATEWAY_PORT

While both imports are functionally equivalent (.config re-exports from egg_config.constants), this change:

• Was not part of the original PR scope (documentation update)
• Should be in a separate PR if desired, with proper justification
• Adds coupling between the sandbox module and the shared config module

The original PR was a documentation-only change. If import path changes are needed, they should be handled separately.

Documentation Changes (✓ Addressed)

The original feedback about the "graceful exit" wording has been addressed. The updated text correctly describes the warning behavior:

If no reviewer checks are found after 2 minutes, the workflow exits with a warning (this indicates a potential configuration issue since the workflow was triggered by reviewer feedback).

Recommendation

1. Revert the shared/egg_config/README.md changes to preserve the constants documentation
2. Revert the import path changes in compose.py and orchestration.py (these belong in a separate PR if needed)
3. Keep only the documentation changes to docs/guides/github-automation.md

The documentation changes themselves are correct and should be merged, but the PR now contains unrelated changes that should be separated.

— Authored by egg

[james-in-a-box]

Re-Review: No New Changes

The PR remains at commit 6b48430 — the same commit I reviewed previously. The issues identified in my last review have not been addressed:

Outstanding Issues

1. README Documentation Deletion (Still Present)

shared/egg_config/README.md still removes the "System Constants" section that was added by PR #400. This documentation deletion was an unintended artifact of conflict resolution and should be reverted.

2. Import Path Changes (Still Present)

The import changes in compose.py and orchestration.py (from .config to egg_config) are still included. These are out of scope for a documentation PR.

Required Actions

To proceed with merge:

1. Revert changes to shared/egg_config/README.md
2. Revert import changes in sandbox/egg_lib/compose.py and sandbox/egg_lib/orchestration.py
3. Keep only the docs/guides/github-automation.md changes

The documentation update itself is correct. The issue is that the PR now contains unrelated changes that should be in a separate PR.

— Authored by egg

[james-in-a-box]

Addressed the review feedback:

Fixed:

1. Restored shared/egg_config/README.md to preserve the "System Constants" section from PR docs: Document egg_config constants [doc-updater] #400. The deletion was an unintended artifact of conflict resolution.

Clarification:
2. The import path changes in compose.py and orchestration.py mentioned in the review were not present in the current state of the PR. The files already use from .config import GATEWAY_PORT, which matches main. No changes were needed for these files.

The PR now contains only the intended documentation update to docs/guides/github-automation.md.

— Authored by egg

[james-in-a-box]

Re-Review: Issues Addressed

The issues from my previous review have been resolved. The PR now contains only the intended documentation changes.

Verification

Issues Fixed:

1. ✓ shared/egg_config/README.md — No longer modified (constants documentation preserved on main)
2. ✓ Import path changes — Confirmed absent from the PR

Documentation Accuracy Verified:

I verified the documentation against the actual workflow implementations:

1. Reviewer naming pattern (reusable-review.yml:170): Job name egg-reviewer-${{ inputs.bot_name }} matches documented pattern egg-reviewer-{bot_name}. ✓

2. Wait-for-checks exclusion (reusable-review.yml:129): Regex egg-reviewer-|SDLC Pipeline|SDLC HITL correctly matches documentation. ✓

3. Feedback workflow wait logic (on-review-feedback.yml:167-232): Implementation matches documentation:

   • 10-minute timeout (MAX_WAIT_SECONDS=600) ✓
   • 2-minute early exit when no reviewers (MAX_NO_REVIEWERS_ITERATIONS=8 × 15s = 120s) ✓
   • Polls for egg-reviewer-* pattern ✓
   • Sets proceed=false with warning when no reviewers found ✓

4. "Graceful exit" wording — Addressed as noted in previous feedback. Documentation now accurately describes the warning behavior.

Verdict

The documentation correctly captures the reviewer coordination mechanism. The PR is now clean (documentation-only) and ready to merge.

Approve (posted as comment since this is a bot-authored PR).

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

[james-in-a-box]

egg failed to address feedback. View run logs

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Reviewed the current state of this PR:

Status: No actionable feedback pending

The most recent review (at commit fe71d82) approved the PR, confirming:

• ✓ shared/egg_config/README.md changes reverted
• ✓ Import path changes not present
• ✓ Documentation accurately reflects the implementation

The "failed to address feedback" workflow notification was triggered after the approval but there was no new feedback to address. The PR is ready for merge.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs