[SDLC] Add phase-based labels for SDLC workflow state tracking

[james-in-a-box]

Summary

Implements phase-based labeling for the SDLC pipeline to provide visual status
tracking and enable filtering issues by pipeline state. Replaces the single
egg-sdlc trigger label with phase labels (sdlc:refine, sdlc:plan,
sdlc:implement, sdlc:pr) plus an approval modifier (sdlc:awaiting-approval).

Closes #402.

---

Closes #402
Branch: egg/issue-402

This PR is managed by the SDLC pipeline. It will be marked ready for review once the implementation passes automated review.

[james-in-a-box]

Agent-mode design review: No concerns.

This PR implements standard infrastructure for SDLC label management. The changes are workflow automation (GitHub Actions YAML) and shell scripts for label operations—they don't involve agent prompts, pre-fetching, structured output requirements, or post-processing pipelines.

The label transition logic is straightforward infrastructure code that runs as part of the CI/CD pipeline, not agent-driven workflow design.

— Authored by egg

[james-in-a-box]

Review: Add phase-based labels for SDLC workflow state tracking

Overall this is a well-structured implementation. The phase-based labeling system is well-designed and the code is organized logically. However, I found several issues that need to be addressed before merging.

Issues Requiring Changes

1. Bug: URL encoding missing in setup-sdlc-labels.sh (setup-sdlc-labels.sh:36-41)

The script checks if a label exists using gh api "repos/${REPO}/labels/${name}", but labels containing : (like sdlc:refine) need URL encoding. The colon must be encoded as %3A.

# Current (broken for labels with colons):
if gh api "repos/${REPO}/labels/${name}" >/dev/null 2>&1; then

# Should be:
encoded_name="${name//:/%3A}"
if gh api "repos/${REPO}/labels/${encoded_name}" >/dev/null 2>&1; then

The same issue exists on line 38 for the PATCH request. Without this fix, the script will always create new labels instead of updating existing ones, and the PATCH requests will fail with 404.

2. Bug: transition-sdlc-label.sh is created but never used (transition-sdlc-label.sh)

The helper script transition-sdlc-label.sh is created and documented, but all the workflow files inline their own label transition logic instead of using this script. This violates DRY and means:

• The script is dead code that will rot
• Bug fixes need to be applied in multiple places
• The contract task TASK-4-3 claims this script is "used by both workflows" but that's not true

Either use the script in workflows or remove it. The acceptance criteria states "Reusable script handles label transitions with error handling; used by both workflows."

3. Logic error: Reading stale phase from contract (sdlc-hitl.yml, "Transition SDLC labels on decision resolution" step)

CONTRACT_PATH=".egg-state/contracts/${ISSUE_NUMBER}.json"
CURRENT_PHASE=$(jq -r '.current_phase' "$CONTRACT_PATH" 2>/dev/null || echo "")

This step runs AFTER the "Update contract phase" step, which already updated .current_phase to NEXT_PHASE. So CURRENT_PHASE will read the NEW phase, not the OLD phase. The OLD_LABEL will be computed from the new phase, causing incorrect label removal.

The step should either:

1. Run BEFORE "Update contract phase", or
2. Use PREVIOUS_PHASE from an earlier step output instead of reading from contract

4. Race condition in label application during init (sdlc-pipeline.yml:217-264)

The "Apply phase label" step removes incorrect phase labels and adds the correct one. However, if the issue already has sdlc:refine (which triggered the workflow), the step tries to remove all OTHER labels first, then add sdlc:refine. But sdlc:refine is already present, so the add will succeed (no-op) but is redundant.

More importantly, between the DELETE calls and the POST call, there's a window where the issue may have no phase label. This could cause issues if another workflow checks label state during this window.

Consider using a single atomic operation to set labels, or checking if the correct label is already present before making any API calls.

5. Documentation inconsistency: Label colors differ between docs and script

In the plan document (.egg-state/drafts/402-analysis.md), the proposed colors are:

• sdlc:refine: #c2e0c6 (green)
• sdlc:plan: #bfdadc (teal)
• sdlc:implement: #fef2c0 (yellow)
• sdlc:pr: #d4c5f9 (purple)

But the setup script uses:

• sdlc:refine: #0E8A16 (dark green)
• sdlc:plan: #1D76DB (blue)
• sdlc:implement: #D93F0B (red/orange)
• sdlc:pr: #5319E7 (purple)

This is not blocking but should be documented or reconciled. The script colors are more distinct which is probably better.

Minor Issues / Suggestions

6. Test file doesn't test actual behavior (test_label_transitions.py)

The tests only verify static properties (file exists, script is executable, strings appear in files). They don't test:

• The setup script actually creates labels correctly (mocked API calls)
• The transition script handles edge cases (label doesn't exist, API failures)
• Label transitions work end-to-end

Consider adding subprocess tests that actually run the scripts with mocked gh commands.

7. Missing cleanup of sdlc:awaiting-approval in on-issue-closed.yml condition

The if: condition on the cleanup job only checks for phase labels, but sdlc:awaiting-approval should also trigger cleanup if it's the only SDLC label present (edge case).

if: >-
  contains(github.event.issue.labels.*.name, 'sdlc:refine') ||
  contains(github.event.issue.labels.*.name, 'sdlc:plan') ||
  contains(github.event.issue.labels.*.name, 'sdlc:implement') ||
  contains(github.event.issue.labels.*.name, 'sdlc:pr') ||
  contains(github.event.issue.labels.*.name, 'sdlc:awaiting-approval')

This IS included, so no change needed - just confirming this is correct.

8. Contract state files included in PR

The .egg-state/contracts/402.json and .egg-state/drafts/*.md files contain SDLC pipeline metadata. These are expected for the SDLC process but note they'll be merged to main. Confirm this is intentional.

Summary

The core implementation is sound. Issues #1 and #3 are bugs that will cause incorrect behavior. Issue #2 represents dead code and an unmet acceptance criterion. Please address these before merging.

— Authored by egg

[james-in-a-box]

egg is investigating the Lint check failure...

[james-in-a-box]

egg autofix completed. View run logs

[james-in-a-box]

Agent-mode design re-review: No concerns.

The delta since last review (commits 3bd4aa9..HEAD) addresses feedback from the previous code review:

• URL encoding fix for labels with colons
• Stale phase reading fix (using step outputs instead of reading updated contract)
• Race condition mitigation (check if label already present before API calls)
• DRY improvement (using transition-sdlc-label.sh throughout)

These are standard GitHub Actions workflow changes and shell script fixes. No agent prompts, pre-fetching, structured output requirements, or post-processing pipelines are involved. The changes remain CI/CD infrastructure code, not agent-mode workflow design.

— Authored by egg

[james-in-a-box]

Re-review: Changes since commit 3bd4aa9

I've reviewed the delta from my previous review. The changes address the issues I raised.

Issues from Previous Review — Status

Issue	Status	Notes
#1: URL encoding missing in setup-sdlc-labels.sh	✅ Fixed	Labels with colons now URL-encoded as %3A in API paths
#2: transition-sdlc-label.sh unused	✅ Fixed	Script is now used by both sdlc-pipeline.yml (7 calls) and sdlc-hitl.yml (4 calls)
#3: Stale phase reading	✅ Fixed	previous_phase is now captured from step outputs BEFORE contract is updated
#4: Race condition in label application	✅ Fixed	Code now checks if correct label is present before making API calls

Code Quality Observations

Positive changes:

• The transition script provides a clean abstraction for label operations
• Proper input validation (validate_label) and URL encoding (url_encode_label)
• The issue number validation prevents path traversal
• Using --silent with fallthrough on failures is appropriate for non-critical label operations

Minor inconsistency (non-blocking):

• on-issue-closed.yml still uses inline label removal logic instead of the transition script. This is acceptable since that workflow doesn't checkout the repo (no need to for cleanup operations), but worth noting for future maintenance.

Verified Behavior

1. Phase transitions capture previous phase correctly: Both the "Check for phase advancement" step and "Determine next phase" step output previous_phase=${CURRENT_PHASE} before any contract modifications.

2. Label transitions are now atomic: The transition script adds the new label first, then removes the old one (add_label "$TO_LABEL" || true; remove_label "$FROM_LABEL" || true).

3. Error handling is appropriate: Label operations use || true to prevent workflow failures on non-critical operations while still logging warnings.

No blocking issues found in the new changes.

— Authored by egg

[james-in-a-box]

All review feedback has been addressed. The re-review confirms:

Issue	Status
#1: URL encoding in setup-sdlc-labels.sh	✅ Fixed
#2: transition-sdlc-label.sh unused	✅ Fixed
#3: Stale phase reading	✅ Fixed
#4: Race condition in label application	✅ Fixed

All CI checks are passing. Ready for human review.

— Authored by egg

[james-in-a-box]

Agent-mode design re-review: No concerns.

The delta since commit 12964804269a5f287cd7e2ad32442f2be574e55d consists of a single merge commit (3ce89e2) that brings in main branch changes. These changes refactor workflows to be reusable via workflow_call, which is standard CI/CD infrastructure—not agent-mode workflow design.

The reusable workflow pattern aligns with the "Composition over duplication" guideline in docs/guides/agent-mode-design.md. No agent prompts, pre-fetching, structured output requirements, or post-processing pipelines are introduced by this merge.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

[james-in-a-box]

Re-review: Changes since commit 1296480

I've reviewed the delta from my previous review (the merge commit 3ce89e2 incorporating commit 93e1395 which addressed my feedback).

Previous Issues — Status

Issue	Status	Verification
#1: URL encoding missing in setup-sdlc-labels.sh	✅ Fixed	Line 62: encoded_name="${name//:/%3A}" correctly URL-encodes colons before API calls
#2: transition-sdlc-label.sh unused	✅ Fixed	Script is now used 11 times: 7 calls in sdlc-pipeline.yml, 4 calls in sdlc-hitl.yml
#3: Stale phase reading	✅ Fixed	previous_phase is captured from CURRENT_PHASE at lines 440 and 714 BEFORE contract update
#4: Race condition in label application	✅ Fixed	Lines 325-330 check if correct label is present before making API calls

Implementation Quality

Positive observations:

• The transition script provides proper input validation (validate_label function) and numeric issue number validation
• Labels are transitioned atomically: add new label first, then remove old (lines 147-149 in transition script)
• Error handling uses || true for non-critical label operations to prevent workflow failures
• URL encoding is correctly implemented for both colons and spaces

Minor inconsistency (non-blocking):

• sdlc_label input in sdlc-pipeline.yml (line 53-57) and on-issue-closed.yml (line 28-32) defaults to "egg-sdlc" but is never used in the workflow logic. The trigger condition now hardcodes 'sdlc:refine' directly. This is dead configuration but doesn't affect functionality.

Verification

Traced the phase transition flow:

1. sdlc-hitl.yml handle-decisions job: previous_phase=${CURRENT_PHASE} captured at line 440 → contract updated at line 467-489 → labels transitioned at line 492-510 using PREVIOUS_PHASE from step outputs
2. sdlc-hitl.yml approve-phase job: Same pattern - previous_phase captured at line 714 before contract update

No blocking issues found in the new changes.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Feedback Status Update

Reviewed all feedback on this PR. The re-review at commit 3ce89e2 confirms all blocking issues have been addressed:

Issue	Status
#1: URL encoding in setup-sdlc-labels.sh	✅ Fixed
#2: transition-sdlc-label.sh unused	✅ Fixed
#3: Stale phase reading	✅ Fixed
#4: Race condition in label application	✅ Fixed

Non-blocking observations (no action required):

• Phase 3: Container extraction #5 (Color inconsistency): Script colors are more distinct, documented in code
• Phase 4: CLI implementation #6 (Test improvements): Suggestion noted for future enhancement
• Phase 6: Release preparation #7 (sdlc:awaiting-approval cleanup): Verified already correct
• Merge Phase 1.5 docs into main #8 (Contract state files): Expected for SDLC process

Minor dead code noted but non-blocking:

• sdlc_label input is defined but not used (trigger hardcodes 'sdlc:refine'). Can be cleaned up in a follow-up if desired.

All CI checks passing. Ready for human review and merge.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs