diff --git a/.egg-state/contracts/363.json b/.egg-state/contracts/363.json new file mode 100644 index 0000000000..9f4aa217d8 --- /dev/null +++ b/.egg-state/contracts/363.json @@ -0,0 +1,517 @@ +{ + "schemaVersion": "1.0", + "issue": { + "number": 363, + "title": "this comment was improperly hidden", + "url": "https://github.com/jwbron/egg/issues/363" + }, + "current_phase": "implement", + "acceptance_criteria": [ + { + "id": "ac-1", + "description": "[TASK-1-1] Init job no longer calls minimizeComment; workflow file validates", + "verified": false + }, + { + "id": "ac-2", + "description": "[TASK-1-2] Refine job no longer calls minimizeComment", + "verified": false + }, + { + "id": "ac-3", + "description": "[TASK-1-3] Plan job no longer calls minimizeComment", + "verified": false + }, + { + "id": "ac-4", + "description": "[TASK-2-1] All status comments in sdlc-pipeline.yml include the marker", + "verified": false + }, + { + "id": "ac-5", + "description": "[TASK-2-2] All status comments in sdlc-hitl.yml include the marker", + "verified": false + }, + { + "id": "ac-6", + "description": "[TASK-2-3] Status comments include marker; review content does NOT", + "verified": false + }, + { + "id": "ac-7", + "description": "[TASK-2-4] Status comments include marker", + "verified": false + }, + { + "id": "ac-8", + "description": "[TASK-2-5] Status comments include marker", + "verified": false + }, + { + "id": "ac-9", + "description": "[TASK-2-6] Status comments include marker", + "verified": false + }, + { + "id": "ac-10", + "description": "[TASK-2-7] Status comments include marker", + "verified": false + }, + { + "id": "ac-11", + "description": "[TASK-3-1] Hiding uses contains marker instead of content patterns", + "verified": false + }, + { + "id": "ac-12", + "description": "[TASK-3-2] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-13", + "description": "[TASK-3-3] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-14", + "description": "[TASK-3-4] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-15", + "description": "[TASK-3-5] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-16", + "description": "[TASK-3-6] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-17", + "description": "[TASK-3-7] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-18", + "description": "[TASK-3-8] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-19", + "description": "[TASK-3-9] Hiding uses marker-based selection", + "verified": false + }, + { + "id": "ac-20", + "description": "[TASK-4-1] Status message includes \"N previous review(s) hidden\" if N > 0", + "verified": false + }, + { + "id": "ac-21", + "description": "[TASK-4-2] Status message includes hidden count", + "verified": false + } + ], + "phases": [ + { + "id": "phase-1", + "name": "Remove Comment Hiding from SDLC Issue Phases", + "status": "pending", + "tasks": [ + { + "id": "task-1-1", + "description": "Remove \"Minimize previous status comments\" step from init job", + "status": "pending", + "acceptance_criteria": "Init job no longer calls minimizeComment; workflow file validates", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-1-2", + "description": "Remove \"Minimize previous pipeline comments\" step from refine job", + "status": "pending", + "acceptance_criteria": "Refine job no longer calls minimizeComment", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-1-3", + "description": "Remove \"Minimize previous pipeline comments\" step from plan job", + "status": "pending", + "acceptance_criteria": "Plan job no longer calls minimizeComment", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + } + ], + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "escalation_reason": null, + "review_feedback": [] + }, + { + "id": "phase-2", + "name": "Add Semantic Marker to Status Comments", + "status": "pending", + "tasks": [ + { + "id": "task-2-1", + "description": "Add marker to SDLC pipeline status comments", + "status": "pending", + "acceptance_criteria": "All status comments in sdlc-pipeline.yml include the marker", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-2", + "description": "Add marker to SDLC HITL status comments", + "status": "pending", + "acceptance_criteria": "All status comments in sdlc-hitl.yml include the marker", + "files_affected": [ + ".github/workflows/sdlc-hitl.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-3", + "description": "Add marker to reusable-review.yml status comments", + "status": "pending", + "acceptance_criteria": "Status comments include marker; review content does NOT", + "files_affected": [ + ".github/workflows/reusable-review.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-4", + "description": "Add marker to on-check-failure.yml status comments", + "status": "pending", + "acceptance_criteria": "Status comments include marker", + "files_affected": [ + ".github/workflows/on-check-failure.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-5", + "description": "Add marker to on-merge-conflict.yml status comments", + "status": "pending", + "acceptance_criteria": "Status comments include marker", + "files_affected": [ + ".github/workflows/on-merge-conflict.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-6", + "description": "Add marker to on-mention.yml status comments", + "status": "pending", + "acceptance_criteria": "Status comments include marker", + "files_affected": [ + ".github/workflows/on-mention.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-7", + "description": "Add marker to on-review-feedback.yml status comments", + "status": "pending", + "acceptance_criteria": "Status comments include marker", + "files_affected": [ + ".github/workflows/on-review-feedback.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + } + ], + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "escalation_reason": null, + "review_feedback": [] + }, + { + "id": "phase-3", + "name": "Update PR Workflow Hiding to Use Markers", + "status": "pending", + "tasks": [ + { + "id": "task-3-1", + "description": "Update implement job hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses contains marker instead of content patterns", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-2", + "description": "Update finalize-pr job hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-3", + "description": "Update checks-failed job hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-4", + "description": "Update reusable-review.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/reusable-review.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-5", + "description": "Update on-check-failure.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/on-check-failure.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-6", + "description": "Update on-merge-conflict.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/on-merge-conflict.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-7", + "description": "Update on-mention.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/on-mention.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-8", + "description": "Update on-review-feedback.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/on-review-feedback.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-9", + "description": "Update sdlc-hitl.yml hiding logic to use marker", + "status": "pending", + "acceptance_criteria": "Hiding uses marker-based selection", + "files_affected": [ + ".github/workflows/sdlc-hitl.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + } + ], + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "escalation_reason": null, + "review_feedback": [] + }, + { + "id": "phase-4", + "name": "Add Hidden-Comments Counter for Reviews", + "status": "pending", + "tasks": [ + { + "id": "task-4-1", + "description": "Update reusable-review.yml to count hidden comments", + "status": "pending", + "acceptance_criteria": "Status message includes \"N previous review(s) hidden\" if N > 0", + "files_affected": [ + ".github/workflows/reusable-review.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-4-2", + "description": "Update on-review-feedback.yml to count hidden comments", + "status": "pending", + "acceptance_criteria": "Status message includes hidden count", + "files_affected": [ + ".github/workflows/on-review-feedback.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + } + ], + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "escalation_reason": null, + "review_feedback": [] + } + ], + "decisions": [], + "circuit_breaker": { + "total_cycles": 0, + "max_total_cycles": 10, + "status": "closed" + }, + "workflow_owner": "jwbron", + "audit_log": [ + { + "timestamp": "2026-02-09T02:14:51Z", + "actor": "system", + "role": "reviewer", + "action": "update", + "field_path": "refine_review_cycles", + "old_value": 0, + "new_value": 1, + "reason": "Refine review cycle 1: approved" + }, + { + "timestamp": "2026-02-09T02:48:15Z", + "actor": "jwbron", + "role": "human", + "action": "transition", + "field_path": "current_phase", + "old_value": "refine", + "new_value": "plan", + "reason": "Phase approved by human" + }, + { + "timestamp": "2026-02-09T02:53:49Z", + "actor": "system", + "role": "reviewer", + "action": "update", + "field_path": "plan_review_cycles", + "old_value": 0, + "new_value": 1, + "reason": "Plan review cycle 1: approved" + }, + { + "timestamp": "2026-02-09T02:58:01Z", + "actor": "jwbron", + "role": "human", + "action": "transition", + "field_path": "current_phase", + "old_value": "plan", + "new_value": "implement", + "reason": "Phase approved by human" + } + ], + "refine_review_cycles": 1, + "refine_review_feedback": "", + "plan_review_cycles": 1, + "plan_review_feedback": "", + "pr": { + "title": "Fix comment hider to only hide status on PRs", + "description": "Fixes #363. The comment hider was improperly hiding substantive content\n(like analysis documents) on issues because it used pattern matching that\nwas too broad.\n\nThis PR implements role-based hiding with semantic markers:\n- Removes comment hiding from SDLC issue phases (refine, plan)\n- Adds `` marker to status-only comments\n- Updates hiding logic to target the marker instead of content patterns\n- Adds a counter showing how many prior reviews were hidden" + } +} diff --git a/.egg-state/drafts/363-analysis.md b/.egg-state/drafts/363-analysis.md new file mode 100644 index 0000000000..99488bdb0f --- /dev/null +++ b/.egg-state/drafts/363-analysis.md @@ -0,0 +1,177 @@ +# Analysis: Comment Hider Improperly Hiding Issue Comments + +> Issue: #363 | Phase: refine + +## Problem Statement + +In issue #359, the SDLC pipeline's review phase improperly hid/minimized comments that should have been preserved. Specifically, the analysis document posted to the issue was minimized (collapsed) when it should have remained visible. + +The comment hider logic was designed for PR workflows where status comments should be hidden to reduce clutter, but it's being applied too broadly to issue comments in the SDLC pipeline, resulting in substantive content (like analysis documents) being hidden. + +**Current state:** Comment minimization runs on issue comments in the SDLC pipeline using patterns that can match substantive content. + +**Desired outcome:** Comment hiding should: +1. Only apply to PR workflows (not issue workflows in the refine/plan phases) +2. Hide only status/notification comments, not substantive analysis or review content +3. Review bots should hide their prior reviews +4. Fixer bots should hide their old status comments + +## Current Behavior + +The codebase has comment minimization logic in 7 workflow files using GitHub's GraphQL `minimizeComment` mutation with the `OUTDATED` classifier. Here's how it currently works: + +### Where Comment Hiding is Implemented + +| Workflow | Location | Pattern Matched | +|----------|----------|-----------------| +| `sdlc-pipeline.yml` | Lines 229-243 | `SDLC Pipeline\|phase completed\|phase encountered` | +| `sdlc-pipeline.yml` | Lines 593-607 | `SDLC Pipeline\|phase completed\|phase encountered\|Pull request ready` | +| `sdlc-pipeline.yml` | Lines 983-997 | `SDLC Pipeline\|phase completed\|phase encountered\|Pull request ready\|Checks timed out\|Checks failed` | +| `sdlc-pipeline.yml` | Lines 1193-1207 | `SDLC Pipeline\|phase completed\|phase encountered` | +| `sdlc-pipeline.yml` | Lines 1914-1928 | `SDLC Pipeline\|phase completed\|phase encountered` | +| `reusable-review.yml` | Lines 383-397 | `egg (completed\|failed)` | +| `on-review-feedback.yml` | Lines 234-254 | `egg is addressing\|egg feedback` | +| `on-check-failure.yml` | Lines 84-103 | `egg is investigating\|egg autofix` | +| `on-mention.yml` | Lines 154-171 | `egg run\|egg finished\|Working on it` | +| `on-merge-conflict.yml` | Lines 171-181, 331-341 | `egg is resolving\|egg conflict resolution` | + +### The Problem with Issue #359 + +Looking at issue #359's comments, the comment minimization step ran on **issue comments** (not PR comments) and used a pattern that was too broad. The pattern `SDLC Pipeline|phase completed|phase encountered` matches: + +1. **Intended targets**: "SDLC Pipeline initialized for this issue" (status message) +2. **Unintended targets**: Any comment containing "phase" or similar keywords + +The analysis document itself starts with `# Analysis: Investigate strongdm/attractor` and contains extensive content. While this specific content shouldn't match the pattern, the underlying issue is that: + +1. **Comment hiding runs on issues, not just PRs** - The SDLC pipeline operates on issues during refine/plan phases, but comment hiding was designed for PR clutter reduction +2. **Pattern matching is fragile** - Using regex patterns to identify "status-only" comments can accidentally match substantive content +3. **No distinction between comment types** - There's no semantic marker distinguishing status comments from content comments + +## Constraints + +### Technical Constraints +- **GitHub API limitations**: `minimizeComment` mutation hides comments but they can be expanded by users +- **Pattern matching risks**: Regex patterns can match unintended content; need precise targeting +- **Workflow structure**: Comment hiding typically runs as a step before posting new status, meaning it runs on every phase transition + +### Business Constraints +- **Preserve substantive content**: Analysis documents, review feedback, and human-facing content must never be hidden +- **Reduce clutter**: Status/notification comments should still be minimized to keep discussions focused +- **Backward compatibility**: Existing PR review workflows should continue to work + +### Dependencies +- SDLC pipeline phases (refine, plan, implement, pr) each have different comment contexts +- PR-based workflows (`reusable-review.yml`, `on-review-feedback.yml`) operate on PRs, not issues +- Issue-based workflows (early SDLC phases) should preserve more content + +## Options Considered + +### Option A: Scope Comment Hiding to PR Workflows Only + +**Approach**: Remove or disable comment minimization from SDLC pipeline jobs that operate on issues (refine, plan phases). Only apply comment hiding in the PR/implement phase. + +**Pros**: +- Simple to implement - remove/disable a few workflow steps +- Eliminates risk of hiding issue content entirely +- Clear separation: issues keep all comments, PRs get cleaned up + +**Cons**: +- Issue threads may get cluttered with status messages over multiple cycles +- Inconsistent behavior between phases + +### Option B: Use Semantic Markers to Identify Hideable Comments + +**Approach**: Add HTML comment markers (e.g., ``) to status-only comments, then only minimize comments containing this marker. + +**Pros**: +- Precise targeting - only comments explicitly marked get hidden +- Self-documenting - marker indicates intent +- Future-proof - new comment types can opt-in or out of hiding + +**Cons**: +- Requires updating all status comment posting steps to include the marker +- Existing unmarked comments won't be hidden (may need migration) +- More changes across multiple workflows + +### Option C: Restrict Patterns and Add Negative Matches + +**Approach**: Tighten regex patterns and add negative lookahead to exclude content-rich comments. For example: +```bash +select(.body | test("SDLC Pipeline initialized|phase completed")) | +select(.body | test("# Analysis|## Problem Statement|## Recommended") | not) +``` + +**Pros**: +- Can be done incrementally without marker changes +- Preserves existing behavior for true status comments + +**Cons**: +- Negative patterns are fragile and grow over time +- Doesn't address the root cause (hiding on issues vs PRs) +- Hard to maintain as content formats evolve + +### Option D: Role-Based Hiding (Review Bots Hide Reviews, Fixers Hide Status) + +**Approach**: Each bot type only hides its own prior output of the same type: +- Review bots (refine reviewer, code reviewer) hide their previous reviews before posting new ones +- Fixer bots (autofixer, conflict resolver) hide their previous status comments +- Implementation agents don't hide anything on issues + +Combined with semantic markers for precise targeting. + +**Pros**: +- Aligns with the stated goal: "review bots hide prior reviews, fixer bots hide old comments" +- Each workflow owns its hiding logic +- Clear responsibility boundaries + +**Cons**: +- Requires auditing each workflow to ensure correct behavior +- May need coordination when multiple bot types operate on the same thread + +## Recommended Approach + +**Option D (Role-Based Hiding with Semantic Markers)** combined with **Option A (Scope to PR Workflows)** for the SDLC pipeline. + +### Rationale + +1. **Addresses the root cause**: The issue is that comment hiding was applied too broadly to issue comments. By restricting SDLC pipeline comment hiding to PR-phase operations and using semantic markers, we eliminate the risk of hiding substantive content. + +2. **Matches stated requirements**: The issue explicitly states "review bots should hide prior reviews and fixer bots should hide their old comments" - this is role-based hiding. + +3. **Precise targeting via markers**: Using `` or similar markers ensures only intended comments are hidden, regardless of content patterns. + +4. **Incremental implementation**: Can be done in phases: + - Phase 1: Remove comment hiding from SDLC issue-phase jobs (quick fix) + - Phase 2: Add semantic markers to status comments across workflows + - Phase 3: Update hiding logic to use markers instead of content patterns + +### Suggested Implementation + +1. **SDLC Pipeline Changes** (`.github/workflows/sdlc-pipeline.yml`): + - Remove the "Minimize previous status comments" steps from jobs that operate on issues (init, refine, refine-review, plan phases) + - Keep comment minimization only in PR-phase jobs (implement-pr-ready, check-impl-pr, etc.) + +2. **Add Semantic Markers**: + - Status comments (e.g., "SDLC Pipeline initialized", "Refine phase completed") should include `` + - Content comments (analysis, reviews, feedback) should NOT include this marker + +3. **Update Hiding Logic**: + ```bash + # Instead of pattern matching on content: + .[] | select(.body | contains("")) | .node_id + ``` + +4. **PR Review Workflows** (`reusable-review.yml`): + - Already correctly scoped to PRs + - Consider adding markers for future-proofing + +## Open Questions + +1. Should the refine phase status messages (e.g., "Refine phase completed. [View run logs]...") also be hidden on issues, or should they remain visible as a timeline of what happened? + +2. For the review bot's reviews that get hidden, should we keep a count or summary visible (e.g., "2 previous reviews hidden") so users know prior review cycles occurred? + +--- + +*Authored-by: egg* diff --git a/.egg-state/drafts/363-plan.md b/.egg-state/drafts/363-plan.md new file mode 100644 index 0000000000..f2204f4fed --- /dev/null +++ b/.egg-state/drafts/363-plan.md @@ -0,0 +1,277 @@ +# Plan: Fix Comment Hider Logic for Issue Comments + +> Issue: #363 | Phase: plan + +## Summary + +The comment hider logic improperly hid an analysis document in issue #359 because the hiding logic was designed for PR workflows but applied too broadly to issue comments. This plan implements role-based hiding with semantic markers: SDLC pipeline phase jobs will stop hiding comments on issues (preserving all substantive content), while PR-based workflows will use explicit `` markers for precise targeting. Review bots will continue hiding prior reviews but with a counter showing how many were hidden. + +The implementation follows the recommended approach from the analysis (Option D + Option A), incorporating the human feedback to keep refine status messages visible and add a hidden-comments counter for review cycles. + +## Implementation Phases + +### Phase 1: Remove Comment Hiding from SDLC Issue Phases + +**Goal**: Stop the SDLC pipeline from hiding any comments during issue-based phases (init, refine, plan). This is the immediate fix that prevents substantive content from being hidden. + +**Tasks**: +- [TASK-1-1] Remove "Minimize previous status comments" step from init job (line ~218-243) — Acceptance: Init job no longer calls minimizeComment; workflow file validates +- [TASK-1-2] Remove "Minimize previous pipeline comments" step from refine job (line ~1184-1207) — Acceptance: Refine job no longer calls minimizeComment +- [TASK-1-3] Remove "Minimize previous pipeline comments" step from plan job (line ~1905-1928) — Acceptance: Plan job no longer calls minimizeComment + +**Dependencies**: None + +**Exit criteria**: All three comment-minimizing steps are removed from issue-phase jobs in sdlc-pipeline.yml. The implement and finalize-pr jobs retain their comment hiding (they operate on PRs). + +### Phase 2: Add Semantic Marker to Status Comments + +**Goal**: Add the `` marker to all status-only comments across workflows, enabling precise targeting for future hiding logic. + +**Tasks**: +- [TASK-2-1] Add marker to SDLC pipeline status comments (init, phase completed, etc.) — Acceptance: All status comments in sdlc-pipeline.yml include the marker +- [TASK-2-2] Add marker to SDLC HITL status comments (decision resolved, phase approved) — Acceptance: All status comments in sdlc-hitl.yml include the marker +- [TASK-2-3] Add marker to reusable-review.yml status comments — Acceptance: Status comments include marker; review content does NOT +- [TASK-2-4] Add marker to on-check-failure.yml status comments — Acceptance: Status comments include marker +- [TASK-2-5] Add marker to on-merge-conflict.yml status comments — Acceptance: Status comments include marker +- [TASK-2-6] Add marker to on-mention.yml status comments — Acceptance: Status comments include marker +- [TASK-2-7] Add marker to on-review-feedback.yml status comments — Acceptance: Status comments include marker + +**Dependencies**: Phase 1 (conceptually independent but should be sequenced for clean commits) + +**Exit criteria**: All status-only comments across all workflows include the `` marker. Substantive content (analysis docs, reviews, PR descriptions) does NOT include the marker. + +### Phase 3: Update PR Workflow Hiding to Use Markers + +**Goal**: Update the comment hiding logic in PR-based workflows to target the semantic marker instead of pattern-matching on content. + +**Tasks**: +- [TASK-3-1] Update implement job hiding logic to use marker — Acceptance: Hiding uses `contains("")` instead of content patterns +- [TASK-3-2] Update finalize-pr job hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-3] Update checks-failed job hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-4] Update reusable-review.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-5] Update on-check-failure.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-6] Update on-merge-conflict.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-7] Update on-mention.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-8] Update on-review-feedback.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection +- [TASK-3-9] Update sdlc-hitl.yml hiding logic to use marker — Acceptance: Hiding uses marker-based selection + +**Dependencies**: Phase 2 (markers must exist before hiding logic can target them) + +**Exit criteria**: All comment hiding logic across all workflows uses the semantic marker for targeting. No pattern-matching on content remains. + +### Phase 4: Add Hidden-Comments Counter for Reviews + +**Goal**: When review bots hide prior reviews, display a count so users know review cycles occurred (per human feedback). + +**Tasks**: +- [TASK-4-1] Update reusable-review.yml to count hidden comments — Acceptance: When posting new review status, include "N previous review(s) hidden" if N > 0 +- [TASK-4-2] Update on-review-feedback.yml to count hidden comments — Acceptance: Include hidden count in status message + +**Dependencies**: Phase 3 + +**Exit criteria**: When prior reviews are hidden, the new status comment includes a count of how many were hidden. + +## Test Strategy + +- **Unit tests**: Not applicable (workflow YAML changes) +- **Integration tests**: + - Trigger SDLC pipeline on a test issue and verify no comments are hidden during refine/plan phases + - Create a PR and verify status comments ARE hidden (using marker-based logic) + - Trigger multiple review cycles and verify hidden-count appears +- **Manual testing**: + 1. Create issue with `egg-sdlc` label + 2. Let pipeline run through refine phase — verify analysis NOT hidden + 3. Approve to plan phase — verify plan NOT hidden + 4. Approve to implement phase — verify only marker-tagged status comments hidden + 5. Review PR multiple times — verify hidden count shows in status + +## Rollback Plan + +If issues arise after deployment: + +1. **Immediate rollback**: Revert the commit(s) on main via `git revert ` +2. **Partial rollback**: If only marker logic is problematic, revert Phase 3 commits while keeping Phases 1-2 +3. **Emergency fix**: If comments are being hidden incorrectly, temporarily remove all `minimizeComment` calls by reverting to pre-change state + +Commands: +```bash +# View recent commits +git log --oneline -10 + +# Revert specific commit +git revert --no-edit +git push origin main +``` + +## Risk Assessment + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Marker accidentally added to substantive content | Low | Medium | Clear documentation; use marker only in explicit status messages | +| Existing hidden comments stay hidden | Low | Low | Already hidden comments cannot be un-hidden automatically; acceptable | +| Hiding logic fails silently | Low | Low | Keep `|| true` error handling; comments just won't hide | +| Workflow validation failures | Medium | Low | Test YAML syntax before commit; use `yamllint` | + +## Migration Notes + +- **Breaking changes**: None. This is purely behavioral — no API or interface changes +- **Database migrations**: None +- **Config changes**: None +- **User impact**: Users will see more comments visible on issues (intended behavior). Status comments on PRs will continue to be hidden as before. + +--- + +## Structured Task Appendix + +The following YAML block is machine-readable and will be extracted into the contract. +It must accurately reflect the tasks described above. The `pr:` section provides the +title and description that will be used when creating the pull request. + +```yaml +# yaml-tasks +pr: + title: "Fix comment hider to only hide status on PRs" + description: | + Fixes #363. The comment hider was improperly hiding substantive content + (like analysis documents) on issues because it used pattern matching that + was too broad. + + This PR implements role-based hiding with semantic markers: + - Removes comment hiding from SDLC issue phases (refine, plan) + - Adds `` marker to status-only comments + - Updates hiding logic to target the marker instead of content patterns + - Adds a counter showing how many prior reviews were hidden +phases: + - id: 1 + name: Remove Comment Hiding from SDLC Issue Phases + goal: Stop hiding comments during issue-based phases to prevent substantive content from being hidden + tasks: + - id: TASK-1-1 + description: Remove "Minimize previous status comments" step from init job + acceptance: Init job no longer calls minimizeComment; workflow file validates + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-1-2 + description: Remove "Minimize previous pipeline comments" step from refine job + acceptance: Refine job no longer calls minimizeComment + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-1-3 + description: Remove "Minimize previous pipeline comments" step from plan job + acceptance: Plan job no longer calls minimizeComment + files: + - .github/workflows/sdlc-pipeline.yml + - id: 2 + name: Add Semantic Marker to Status Comments + goal: Add marker to status-only comments for precise targeting + tasks: + - id: TASK-2-1 + description: Add marker to SDLC pipeline status comments + acceptance: All status comments in sdlc-pipeline.yml include the marker + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-2-2 + description: Add marker to SDLC HITL status comments + acceptance: All status comments in sdlc-hitl.yml include the marker + files: + - .github/workflows/sdlc-hitl.yml + - id: TASK-2-3 + description: Add marker to reusable-review.yml status comments + acceptance: Status comments include marker; review content does NOT + files: + - .github/workflows/reusable-review.yml + - id: TASK-2-4 + description: Add marker to on-check-failure.yml status comments + acceptance: Status comments include marker + files: + - .github/workflows/on-check-failure.yml + - id: TASK-2-5 + description: Add marker to on-merge-conflict.yml status comments + acceptance: Status comments include marker + files: + - .github/workflows/on-merge-conflict.yml + - id: TASK-2-6 + description: Add marker to on-mention.yml status comments + acceptance: Status comments include marker + files: + - .github/workflows/on-mention.yml + - id: TASK-2-7 + description: Add marker to on-review-feedback.yml status comments + acceptance: Status comments include marker + files: + - .github/workflows/on-review-feedback.yml + - id: 3 + name: Update PR Workflow Hiding to Use Markers + goal: Update hiding logic to target semantic marker instead of content patterns + tasks: + - id: TASK-3-1 + description: Update implement job hiding logic to use marker + acceptance: Hiding uses contains marker instead of content patterns + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-3-2 + description: Update finalize-pr job hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-3-3 + description: Update checks-failed job hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/sdlc-pipeline.yml + - id: TASK-3-4 + description: Update reusable-review.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/reusable-review.yml + - id: TASK-3-5 + description: Update on-check-failure.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/on-check-failure.yml + - id: TASK-3-6 + description: Update on-merge-conflict.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/on-merge-conflict.yml + - id: TASK-3-7 + description: Update on-mention.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/on-mention.yml + - id: TASK-3-8 + description: Update on-review-feedback.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/on-review-feedback.yml + - id: TASK-3-9 + description: Update sdlc-hitl.yml hiding logic to use marker + acceptance: Hiding uses marker-based selection + files: + - .github/workflows/sdlc-hitl.yml + - id: 4 + name: Add Hidden-Comments Counter for Reviews + goal: Display count of hidden prior reviews so users know review cycles occurred + tasks: + - id: TASK-4-1 + description: Update reusable-review.yml to count hidden comments + acceptance: Status message includes "N previous review(s) hidden" if N > 0 + files: + - .github/workflows/reusable-review.yml + - id: TASK-4-2 + description: Update on-review-feedback.yml to count hidden comments + acceptance: Status message includes hidden count + files: + - .github/workflows/on-review-feedback.yml +``` + +--- + +## Phase Approval + + +- [ ] Approve and advance to implement phase + +--- + +*Authored-by: egg* diff --git a/.egg-state/reviews/363-refine-review.json b/.egg-state/reviews/363-refine-review.json new file mode 100644 index 0000000000..512a249f20 --- /dev/null +++ b/.egg-state/reviews/363-refine-review.json @@ -0,0 +1,6 @@ +{ + "verdict": "approved", + "summary": "The analysis meets quality standards and is ready for the plan phase.", + "feedback": "", + "timestamp": "2026-02-09T02:14:10Z" +} diff --git a/.github/workflows/on-mention.yml b/.github/workflows/on-mention.yml index e990ecfdbf..982d5bd79a 100644 --- a/.github/workflows/on-mention.yml +++ b/.github/workflows/on-mention.yml @@ -306,6 +306,7 @@ jobs: timeout: ${{ inputs.timeout || '30' }} # Minimize previous status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous status comments if: always() env: @@ -313,11 +314,11 @@ jobs: run: | set -euo pipefail - # Find previous bot comments that are status updates - # Match: "egg run", "egg finished", "👀 Working on it" + # Find previous bot comments with status marker + # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${{ env.ISSUE_OR_PR_NUMBER }}/comments" \ | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("egg run|egg finished|👀 Working on it")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -353,17 +354,26 @@ jobs: if [[ "${EXISTING_COMMENTS:-0}" -gt 0 ]]; then # Agent already posted a substantive comment — only add run link on failure if [[ "${EGG_EXIT_CODE:-1}" != "0" ]]; then - BODY="egg run had errors (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL})" + BODY=" + egg run had errors (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL})" + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api repos/${{ github.repository }}/issues/${{ env.ISSUE_OR_PR_NUMBER }}/comments \ -X POST -f body="${BODY}" fi else # No substantive comment posted — post fallback if [[ "${EGG_EXIT_CODE:-1}" == "0" ]]; then - BODY="egg finished successfully but did not post a response. [View run logs](${RUN_URL})" + BODY=" + egg finished successfully but did not post a response. [View run logs](${RUN_URL})" else - BODY="egg run failed (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL})" + BODY=" + egg run failed (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL})" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api repos/${{ github.repository }}/issues/${{ env.ISSUE_OR_PR_NUMBER }}/comments \ -X POST -f body="${BODY}" fi diff --git a/.github/workflows/on-review-feedback.yml b/.github/workflows/on-review-feedback.yml index f749dc3c25..3a04c449d6 100644 --- a/.github/workflows/on-review-feedback.yml +++ b/.github/workflows/on-review-feedback.yml @@ -337,24 +337,30 @@ jobs: done # Minimize previous feedback status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous feedback comments + id: minimize if: steps.should-run.outputs.run == 'true' && steps.wait-for-reviewers.outputs.proceed == 'true' run: | set -euo pipefail - gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | test(\"egg is addressing|egg feedback\")) | .node_id" \ - | while read -r node_id; do - # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash - if ! gh api graphql -f query=' - mutation($id: ID!) { - minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { - minimizedComment { isMinimized } - } + HIDDEN_COUNT=0 + # Use process substitution to avoid subshell variable scope issues + while read -r node_id; do + # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash + if gh api graphql -f query=' + mutation($id: ID!) { + minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { + minimizedComment { isMinimized } } - ' -f id="$node_id"; then - echo "Warning: Failed to minimize comment $node_id (may lack permissions)" - fi - done + } + ' -f id="$node_id"; then + HIDDEN_COUNT=$((HIDDEN_COUNT + 1)) + else + echo "Warning: Failed to minimize comment $node_id (may lack permissions)" + fi + done < <(gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ + --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | contains(\"\")) | .node_id") + echo "hidden_count=${HIDDEN_COUNT}" >> "$GITHUB_OUTPUT" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} @@ -364,7 +370,7 @@ jobs: run: | set -euo pipefail gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - -f body="egg is addressing review feedback..." + -f body="egg is addressing review feedback..." env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} @@ -414,15 +420,31 @@ jobs: run: | set -euo pipefail RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + HIDDEN_COUNT="${{ steps.minimize.outputs.hidden_count }}" + HIDDEN_COUNT="${HIDDEN_COUNT:-0}" + + # Build hidden count suffix + HIDDEN_MSG="" + if [[ "$HIDDEN_COUNT" -gt 0 ]]; then + HIDDEN_MSG=" + + *${HIDDEN_COUNT} previous review(s) hidden.*" + fi if [[ "${{ steps.egg.outcome }}" == "success" ]]; then - BODY="egg feedback addressed. [View run logs](${RUN_URL})" + BODY=" + egg feedback addressed. [View run logs](${RUN_URL})${HIDDEN_MSG}" elif [[ "${{ steps.egg.outcome }}" == "failure" ]]; then - BODY="egg failed to address feedback. [View run logs](${RUN_URL})" + BODY=" + egg failed to address feedback. [View run logs](${RUN_URL})${HIDDEN_MSG}" else - BODY="Workflow failed before running egg. [View run logs](${RUN_URL})" + BODY=" + Workflow failed before running egg. [View run logs](${RUN_URL})${HIDDEN_MSG}" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" -f body="$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} diff --git a/.github/workflows/reusable-autofix.yml b/.github/workflows/reusable-autofix.yml index b8fcf4867a..5b59c78bab 100644 --- a/.github/workflows/reusable-autofix.yml +++ b/.github/workflows/reusable-autofix.yml @@ -110,9 +110,9 @@ jobs: - name: Minimize previous autofix comments if: steps.skip-check.outputs.skip != 'true' run: | - # Find previous bot comments that are "investigating" status updates + # Find previous bot comments that are status comments (marked with semantic marker) gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | test(\"egg is investigating|egg autofix\")) | .node_id" \ + --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | contains(\"\")) | .node_id" \ | while read -r node_id; do # Minimize the comment using GraphQL # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash @@ -133,10 +133,15 @@ jobs: run: | if [[ -n "${{ env.FAILED_RUN_ID }}" ]]; then RUN_LINK="[${FAILED_WORKFLOW}](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ env.FAILED_RUN_ID }})" - BODY="egg is investigating the ${RUN_LINK} check failure..." + BODY=" + egg is investigating the ${RUN_LINK} check failure..." else - BODY="egg is investigating check failures for this PR..." + BODY=" + egg is investigating check failures for this PR..." fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" -f body="$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} @@ -190,11 +195,16 @@ jobs: RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" if [[ "${{ steps.egg.outcome }}" == "success" ]]; then - BODY="egg autofix completed. [View run logs](${RUN_URL})" + BODY=" + egg autofix completed. [View run logs](${RUN_URL})" else - BODY="egg autofix encountered an issue. [View run logs](${RUN_URL})" + BODY=" + egg autofix encountered an issue. [View run logs](${RUN_URL})" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" -f body="$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} diff --git a/.github/workflows/reusable-conflict-resolve.yml b/.github/workflows/reusable-conflict-resolve.yml index 292524f776..251fd46e70 100644 --- a/.github/workflows/reusable-conflict-resolve.yml +++ b/.github/workflows/reusable-conflict-resolve.yml @@ -117,13 +117,13 @@ jobs: if: steps.fork-check.outputs.skip != 'true' && steps.skip-check.outputs.skip == 'true' run: echo "Skipping conflict resolution due to [skip-conflict-fix] marker" - # Minimize previous conflict resolution comments + # Minimize previous conflict resolution comments (status comments marked with semantic marker) - name: Minimize previous conflict comments if: steps.fork-check.outputs.skip != 'true' && steps.skip-check.outputs.skip != 'true' run: | set -euo pipefail gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | test(\"egg is resolving|egg conflict resolution\")) | .node_id" \ + --jq ".[] | select(.user.login == \"$BOT_USERNAME\" or .user.login == \"${BOT_USERNAME}[bot]\") | select(.body | contains(\"\")) | .node_id" \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash gh api graphql -f query=' @@ -141,7 +141,11 @@ jobs: if: steps.fork-check.outputs.skip != 'true' && steps.skip-check.outputs.skip != 'true' run: | set -euo pipefail - BODY="egg is resolving merge conflicts for this PR..." + BODY=" + egg is resolving merge conflicts for this PR..." + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" -f body="$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} @@ -193,11 +197,16 @@ jobs: RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" if [[ "${{ steps.egg.outcome }}" == "success" ]]; then - BODY="egg conflict resolution completed. [View run logs](${RUN_URL})" + BODY=" + egg conflict resolution completed. [View run logs](${RUN_URL})" else - BODY="egg conflict resolution encountered an issue. [View run logs](${RUN_URL})" + BODY=" + egg conflict resolution encountered an issue. [View run logs](${RUN_URL})" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" -f body="$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} diff --git a/.github/workflows/reusable-review.yml b/.github/workflows/reusable-review.yml index 56e01615bd..71cc3992d7 100644 --- a/.github/workflows/reusable-review.yml +++ b/.github/workflows/reusable-review.yml @@ -477,39 +477,67 @@ jobs: EGG_ISSUE_NUMBER: ${{ inputs.issue_number }} # Minimize previous status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous status comments + id: minimize if: always() && !cancelled() run: | set -euo pipefail - # Find previous bot comments that are status updates for this bot - # Match: "egg completed" or "egg failed" + HIDDEN_COUNT=0 + # Find previous bot comments with status marker + # Use process substitution to avoid subshell variable scope issues # shellcheck disable=SC2016 - gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ - | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("egg ${{ inputs.bot_name }} (completed|failed)")) | .node_id' \ - | while read -r node_id; do - # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash - if ! gh api graphql -f query=' - mutation($id: ID!) { - minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { - minimizedComment { isMinimized } - } + while read -r node_id; do + # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash + if gh api graphql -f query=' + mutation($id: ID!) { + minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { + minimizedComment { isMinimized } } - ' -f id="$node_id"; then - echo "Warning: Failed to minimize comment $node_id (may lack permissions)" - fi - done + } + ' -f id="$node_id"; then + HIDDEN_COUNT=$((HIDDEN_COUNT + 1)) + else + echo "Warning: Failed to minimize comment $node_id (may lack permissions)" + fi + done < <(gh api "repos/${{ github.repository }}/issues/${{ env.PR_NUMBER }}/comments" \ + | jq -r --arg bot "$BOT_USERNAME" \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id') + echo "hidden_count=${HIDDEN_COUNT}" >> "$GITHUB_OUTPUT" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} - name: Post workflow run status to PR if: always() && !cancelled() run: | + HIDDEN_COUNT="${{ steps.minimize.outputs.hidden_count }}" + HIDDEN_COUNT="${HIDDEN_COUNT:-0}" + RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + if [[ "${{ steps.egg.outcome }}" == "success" ]]; then - BODY="egg ${{ inputs.bot_name }} completed. [View run logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + if [[ "$HIDDEN_COUNT" -gt 0 ]]; then + BODY=" + egg ${{ inputs.bot_name }} completed. [View run logs](${RUN_URL}) + + *${HIDDEN_COUNT} previous review(s) hidden.*" + else + BODY=" + egg ${{ inputs.bot_name }} completed. [View run logs](${RUN_URL})" + fi else - BODY="egg ${{ inputs.bot_name }} failed. [View run logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" + if [[ "$HIDDEN_COUNT" -gt 0 ]]; then + BODY=" + egg ${{ inputs.bot_name }} failed. [View run logs](${RUN_URL}) + + *${HIDDEN_COUNT} previous review(s) hidden.*" + else + BODY=" + egg ${{ inputs.bot_name }} failed. [View run logs](${RUN_URL})" + fi fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh pr comment "${{ env.PR_NUMBER }}" --repo "${{ github.repository }}" --body "$BODY" env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} diff --git a/.github/workflows/sdlc-hitl.yml b/.github/workflows/sdlc-hitl.yml index 11eb822661..14704f7934 100644 --- a/.github/workflows/sdlc-hitl.yml +++ b/.github/workflows/sdlc-hitl.yml @@ -510,17 +510,18 @@ jobs: --remove "sdlc:awaiting-approval" # Minimize previous HITL status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous HITL comments if: steps.update.outputs.contract_updated == 'true' env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} run: | set -euo pipefail - # Find previous bot comments that are HITL decision status updates + # Find previous bot comments with status marker # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ | jq -r --arg bot "${{ env.BOT_USERNAME }}" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("Decision .* resolved|Phase .* approved|Advancing to")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -546,10 +547,10 @@ jobs: set -euo pipefail # Use printf to safely construct comment body (prevents command injection) if [[ "$SHOULD_ADVANCE" == "true" ]]; then - BODY=$(printf 'Decision **%s** resolved: %s\n\nAll pending decisions resolved. Advancing to **%s** phase.\n\n--- Authored by egg' \ + BODY=$(printf '\nDecision **%s** resolved: %s\n\nAll pending decisions resolved. Advancing to **%s** phase.\n\n--- Authored by egg' \ "$DECISION_ID" "$SELECTED_OPTION" "$NEXT_PHASE") else - BODY=$(printf 'Decision **%s** resolved: %s\n\n--- Authored by egg' \ + BODY=$(printf '\nDecision **%s** resolved: %s\n\n--- Authored by egg' \ "$DECISION_ID" "$SELECTED_OPTION") fi @@ -821,17 +822,18 @@ jobs: echo "Transitioned labels: sdlc:${PREVIOUS_PHASE} -> sdlc:${NEXT_PHASE}" # Minimize previous HITL status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous HITL comments if: steps.approve.outputs.approved == 'true' env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} run: | set -euo pipefail - # Find previous bot comments that are HITL decision/approval status updates + # Find previous bot comments with status marker # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ | jq -r --arg bot "${{ env.BOT_USERNAME }}" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("Decision .* resolved|Phase .* approved|Advancing to")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -855,7 +857,7 @@ jobs: run: | set -euo pipefail # Use printf to safely construct comment body (prevents command injection) - BODY=$(printf 'Phase **%s** approved by @%s.\n\nAdvancing to **%s** phase.\n\n--- Authored by egg' \ + BODY=$(printf '\nPhase **%s** approved by @%s.\n\nAdvancing to **%s** phase.\n\n--- Authored by egg' \ "$PREVIOUS_PHASE" "$SENDER_LOGIN" "$NEXT_PHASE") gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" @@ -865,23 +867,52 @@ jobs: # ============================================================ handle-feedback: name: Handle feedback submission + needs: resolve-inputs runs-on: ubuntu-latest # Trigger on feedback comments (with egg-feedback marker) where the submit # checkbox has been checked. - # Authorization: Only jwbron can submit feedback, and bot cannot trigger itself + # NOTE: Authorization is checked dynamically in the first step using authorized_users input. + # Bot self-trigger prevention is hardcoded here because job-level `if:` conditions cannot + # access `needs` outputs (GitHub Actions limitation). This is a safety check; the step-level + # auth check handles the full authorization logic including configured bot_username. if: >- contains(github.event.comment.body, '\nFeedback **%s** submitted by @%s.\n\nResuming pipeline with feedback.\n\n--- Authored by egg' \ "$FEEDBACK_ID" "$SENDER_LOGIN") gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" diff --git a/.github/workflows/sdlc-pipeline.yml b/.github/workflows/sdlc-pipeline.yml index 03305750de..a01dd95b09 100644 --- a/.github/workflows/sdlc-pipeline.yml +++ b/.github/workflows/sdlc-pipeline.yml @@ -347,32 +347,9 @@ jobs: echo "Applied phase label: ${PHASE_LABEL}" - # Minimize previous pipeline status comments to reduce clutter - - name: Minimize previous pipeline comments - if: steps.setup.outputs.contract_exists == 'false' - env: - GH_TOKEN: ${{ steps.bot-token.outputs.token }} - BOT_USERNAME: ${{ needs.resolve-inputs.outputs.bot_username }} - run: | - set -euo pipefail - # Find previous bot comments that are pipeline status updates - # Match: "SDLC Pipeline", "phase completed", "phase encountered" - # shellcheck disable=SC2016 - gh api "repos/${{ github.repository }}/issues/${{ steps.setup.outputs.issue_number }}/comments" \ - | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered")) | .node_id' \ - | while read -r node_id; do - # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash - if ! gh api graphql -f query=' - mutation($id: ID!) { - minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { - minimizedComment { isMinimized } - } - } - ' -f id="$node_id"; then - echo "Warning: Failed to minimize comment $node_id (may lack permissions)" - fi - done + # NOTE: Comment hiding removed from init job (issue-phase) per #363 + # Status comments on issues should remain visible for audit trail + # Comment hiding is now only done in PR-phase jobs using semantic markers - name: Post initialization comment if: steps.setup.outputs.contract_exists == 'false' @@ -380,7 +357,8 @@ jobs: GH_TOKEN: ${{ steps.bot-token.outputs.token }} run: | set -euo pipefail - BODY="SDLC Pipeline initialized for this issue. + BODY=" + SDLC Pipeline initialized for this issue. **Branch:** \`${{ steps.setup.outputs.branch_name }}\` **Starting phase:** \`${{ steps.setup.outputs.current_phase }}\` @@ -389,6 +367,9 @@ jobs: --- Authored by egg" + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${{ steps.setup.outputs.issue_number }}" --repo "${{ github.repository }}" --body "$BODY" - name: Close stale PRs @@ -720,17 +701,18 @@ jobs: --to "sdlc:pr" # Minimize previous pipeline status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous pipeline comments if: always() env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} run: | set -euo pipefail - # Find previous bot comments that are pipeline status updates + # Find previous bot comments with status marker # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered|Pull request ready")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -754,16 +736,21 @@ jobs: RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" if [[ "${EGG_EXIT_CODE:-1}" == "0" ]]; then - BODY="Implementation phase completed. [View run logs](${RUN_URL}) + BODY=" + Implementation phase completed. [View run logs](${RUN_URL}) --- Authored by egg" else - BODY="Implementation phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). + BODY=" + Implementation phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL}) --- Authored by egg" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" - name: Capture final HEAD SHA @@ -1013,7 +1000,8 @@ jobs: set -euo pipefail PR_URL="${{ github.server_url }}/${{ github.repository }}/pull/${PR_NUMBER}" - BODY="**Merge conflicts detected** on draft PR #${PR_NUMBER}. + BODY=" + **Merge conflicts detected** on draft PR #${PR_NUMBER}. The branch has conflicts with \`main\` that must be resolved before the PR can be marked ready for review: ${PR_URL} @@ -1021,6 +1009,9 @@ jobs: --- Authored by egg" + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" - name: Update PR title and mark ready @@ -1110,17 +1101,18 @@ jobs: fi # Minimize previous pipeline status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous pipeline comments env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} BOT_USERNAME: ${{ needs.resolve-inputs.outputs.bot_username }} run: | set -euo pipefail - # Find previous bot comments that are pipeline status updates + # Find previous bot comments with status marker # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered|Pull request ready|Checks timed out|Checks failed")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -1141,12 +1133,16 @@ jobs: run: | set -euo pipefail PR_URL="${{ github.server_url }}/${{ github.repository }}/pull/${PR_NUMBER}" - BODY="Pull request ready for review: ${PR_URL} + BODY=" + Pull request ready for review: ${PR_URL} All automated checks passed. The PR is ready for human review and merge. --- Authored by egg" + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" # ============================================================ @@ -1177,17 +1173,18 @@ jobs: private-key: ${{ secrets.BOT_APP_PRIVATE_KEY }} # Minimize previous pipeline status comments to reduce clutter + # Uses semantic marker instead of content patterns per #363 - name: Minimize previous pipeline comments env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} BOT_USERNAME: ${{ needs.resolve-inputs.outputs.bot_username }} run: | set -euo pipefail - # Find previous bot comments that are pipeline status updates + # Find previous bot comments with status marker # shellcheck disable=SC2016 gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered|Pull request ready|Checks timed out|Checks failed")) | .node_id' \ + '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | contains("")) | .node_id' \ | while read -r node_id; do # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash if ! gh api graphql -f query=' @@ -1209,7 +1206,8 @@ jobs: PR_URL="${{ github.server_url }}/${{ github.repository }}/pull/${PR_NUMBER}" if [[ "$TIMED_OUT" == "true" ]]; then - BODY="**Checks timed out** while waiting for all check runs to complete. + BODY=" + **Checks timed out** while waiting for all check runs to complete. This may indicate workflows are queued or running slowly. Please check the PR for status: ${PR_URL} @@ -1217,7 +1215,8 @@ jobs: --- Authored by egg" else - BODY="**Checks failed** on draft PR #${PR_NUMBER}. + BODY=" + **Checks failed** on draft PR #${PR_NUMBER}. Please review the failing checks: ${PR_URL} @@ -1226,6 +1225,9 @@ jobs: --- Authored by egg" fi + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" # ============================================================ @@ -1319,30 +1321,9 @@ jobs: echo "No draft file found at ${DRAFT_FILE}" fi - # Minimize previous pipeline status comments to reduce clutter - - name: Minimize previous pipeline comments - if: always() - env: - GH_TOKEN: ${{ steps.bot-token.outputs.token }} - run: | - set -euo pipefail - # Find previous bot comments that are pipeline status updates - # shellcheck disable=SC2016 - gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ - | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered")) | .node_id' \ - | while read -r node_id; do - # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash - if ! gh api graphql -f query=' - mutation($id: ID!) { - minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { - minimizedComment { isMinimized } - } - } - ' -f id="$node_id"; then - echo "Warning: Failed to minimize comment $node_id (may lack permissions)" - fi - done + # NOTE: Comment hiding removed from refine job (issue-phase) per #363 + # Status comments on issues should remain visible for audit trail + # Comment hiding is now only done in PR-phase jobs using semantic markers - name: Post failure comment if: always() && (steps.egg.outputs.exit-code != '0' || steps.validate-refine.outputs.found != 'true') @@ -1355,19 +1336,24 @@ jobs: RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" if [[ "${EGG_EXIT_CODE:-1}" == "0" ]] && [[ "${ANALYSIS_FOUND:-}" != "true" ]]; then - BODY="**Warning:** Refine phase exited successfully but no analysis draft was found. The agent may have failed to write to \`.egg-state/drafts/\`. [View run logs](${RUN_URL}) + BODY=" + **Warning:** Refine phase exited successfully but no analysis draft was found. The agent may have failed to write to \`.egg-state/drafts/\`. [View run logs](${RUN_URL}) Please check the run logs and retry if needed. --- Authored by egg" elif [[ "${EGG_EXIT_CODE:-1}" != "0" ]]; then - BODY="Refine phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL}) + BODY=" + Refine phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL}) --- Authored by egg" fi # Only post if BODY is set (success cases don't post here - they wait for review) if [[ -n "${BODY:-}" ]]; then + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" fi @@ -2052,30 +2038,9 @@ jobs: gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body-file "$COMMENT_FILE" echo "Posted PR metadata comment" - # Minimize previous pipeline status comments to reduce clutter - - name: Minimize previous pipeline comments - if: always() - env: - GH_TOKEN: ${{ steps.bot-token.outputs.token }} - run: | - set -euo pipefail - # Find previous bot comments that are pipeline status updates - # shellcheck disable=SC2016 - gh api "repos/${{ github.repository }}/issues/${ISSUE_NUMBER}/comments" \ - | jq -r --arg bot "$BOT_USERNAME" \ - '.[] | select(.user.login == $bot or .user.login == ($bot + "[bot]")) | select(.body | test("SDLC Pipeline|phase completed|phase encountered")) | .node_id' \ - | while read -r node_id; do - # shellcheck disable=SC2016 # $id is a GraphQL variable, not bash - if ! gh api graphql -f query=' - mutation($id: ID!) { - minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { - minimizedComment { isMinimized } - } - } - ' -f id="$node_id"; then - echo "Warning: Failed to minimize comment $node_id (may lack permissions)" - fi - done + # NOTE: Comment hiding removed from plan job (issue-phase) per #363 + # Status comments on issues should remain visible for audit trail + # Comment hiding is now only done in PR-phase jobs using semantic markers - name: Post failure comment if: always() && (steps.egg.outputs.exit-code != '0' || steps.validate-plan.outputs.found != 'true') @@ -2088,19 +2053,24 @@ jobs: RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" if [[ "${EGG_EXIT_CODE:-1}" == "0" ]] && [[ "${PLAN_FOUND:-}" != "true" ]]; then - BODY="**Warning:** Plan phase exited successfully but no plan draft was found. The agent may have failed to write to \`.egg-state/drafts/\`. [View run logs](${RUN_URL}) + BODY=" + **Warning:** Plan phase exited successfully but no plan draft was found. The agent may have failed to write to \`.egg-state/drafts/\`. [View run logs](${RUN_URL}) Please check the run logs and retry if needed. --- Authored by egg" elif [[ "${EGG_EXIT_CODE:-1}" != "0" ]]; then - BODY="Plan phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL}) + BODY=" + Plan phase encountered issues (exit code: ${EGG_EXIT_CODE:-unknown}). [View run logs](${RUN_URL}) --- Authored by egg" fi # Only post if BODY is set (success cases don't post here - they wait for review) if [[ -n "${BODY:-}" ]]; then + # Strip leading whitespace from each line (caused by YAML indentation) + # shellcheck disable=SC2001 # sed is needed for regex-based multiline substitution + BODY=$(echo "$BODY" | sed 's/^[[:space:]]*//') gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" fi diff --git a/docs/guides/reusable-workflows.md b/docs/guides/reusable-workflows.md index 370d39f3b0..10d17c2db0 100644 --- a/docs/guides/reusable-workflows.md +++ b/docs/guides/reusable-workflows.md @@ -135,6 +135,7 @@ The pipeline is triggered by applying the `sdlc:refine` label to an issue. You c .github/scripts/setup-sdlc-labels.sh --repo owner/repo ``` + ## Common Parameters | Parameter | Description | Default |