Populate contract tasks from plan before implement phase

[james-in-a-box]

Fix empty contract causing infinite implement loop

The plan parser (plan_parser.py) exists but is never called. The plan
phase posts a markdown plan as an issue comment, but the contract's
phases[] array stays empty. The review status check treats 0/0 tasks as
"incomplete", causing an infinite loop until the circuit breaker trips.

This adds a populate-contract-tasks.py script that runs at the start of
the implement job. It fetches the plan comment from the issue, runs
plan_parser.parse_plan(), and writes phases/tasks into the contract JSON.
The script is idempotent — if tasks already exist, it exits immediately.

Changes:

• Add action/populate-contract-tasks.py — fetches plan comment, parses
  tasks, populates contract
• Add "Populate contract tasks from plan" step to implement job, after
  checkout and before the implementer runs
• Save shared/ as trusted scripts alongside action/ from main checkout
• Add git identity config to implement job for the commit

Issue: #262

Test plan:

• Run the SDLC pipeline on an issue through plan→implement
• Verify the contract is populated with tasks before the implementer runs
• Verify review status check sees actual tasks (not 0/0)
• On re-dispatch cycles, verify the populate step is skipped (idempotent)

Authored-by: egg

[james-in-a-box]

egg is investigating the Lint check failure...

[james-in-a-box]

Agent-Mode Design Review: No concerns.

This is pre-agent infrastructure, not an agent workflow. The script runs before the agent starts and populates a contract file with task context. This provides orienting metadata that helps the agent understand what work is expected — it doesn't constrain how the agent explores or operates.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

[james-in-a-box]

egg autofix completed. View run logs

[james-in-a-box]

Agent-Mode Design Review: No concerns (re-review).

The only change since my previous review is ruff auto-formatting of argument lists. No functional changes were made.

My previous assessment stands: this is pre-agent infrastructure that provides orienting metadata to help the agent understand what work is expected. It doesn't constrain how the agent explores or operates.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

[james-in-a-box]

Code Review: PR #274 - Populate contract tasks from plan

Summary

This PR adds a populate-contract-tasks.py script that runs at the start of the implement job to extract tasks from the plan comment and populate the contract JSON. The fix addresses the root cause of infinite implement loops (issue #262) by ensuring the contract has actual tasks before the implementer runs.

---

Critical Issues

1. JSON Schema Mismatch for Task IDs — Will cause validation failures

Location: .egg/schemas/contract.schema.json:175 vs shared/egg_contracts/models.py:103-106

The JSON schema enforces pattern ^task-[0-9]+$ (single number), but the plan parser and this script produce task IDs in task-{phase}-{task} format (e.g., task-1-1).

// contract.schema.json line 175
"pattern": "^task-[0-9]+$"

# models.py allows both formats:
pattern=r"^task-[0-9]+(-[0-9]+)?$"

The Pydantic model correctly allows both formats, but any external JSON schema validation (e.g., pre-commit hooks, CI validation) will reject the task IDs produced by this script.

Fix: Update contract.schema.json line 175 to match the Pydantic model pattern:

"pattern": "^task-[0-9]+(-[0-9]+)?$"

2. Missing escalated field in task dict — Schema violation

Location: action/populate-contract-tasks.py:112-120

The script constructs task dictionaries missing the escalated field, which is defined in the schema (line 227-230). While the schema has a default, explicit JSON writes should include all required fields for clarity and to avoid issues if schema defaults change.

task_dict = {
    "id": task.id,
    "description": task.description,
    "status": str(task.status),
    "acceptance_criteria": task.acceptance_criteria,
    "files_affected": task.files_affected,
    "commit": None,
    "review_cycles": 0,
    "max_cycles": 3,
    "notes": "",
    # Missing: "escalated": False
}

Fix: Add "escalated": False to the task_dict.

---

Medium Issues

3. Missing phase fields in phase dict — Inconsistent with schema

Location: action/populate-contract-tasks.py:103-109

The script constructs phase dictionaries missing several fields that exist in the schema and Pydantic model:

• review_cycles (default: 0)
• max_cycles (default: 3)
• escalated (default: false)
• escalation_reason (default: null)
• review_feedback (default: [])

phase_dict = {
    "id": contract_phase.id,
    "name": contract_phase.name,
    "status": str(contract_phase.status),
    "tasks": [],
}

Fix: Include all phase fields for consistency:

phase_dict = {
    "id": contract_phase.id,
    "name": contract_phase.name,
    "status": str(contract_phase.status),
    "tasks": [],
    "review_cycles": 0,
    "max_cycles": 3,
    "escalated": False,
    "escalation_reason": None,
    "review_feedback": [],
}

4. No validation of parsed plan before writing — Silent corruption risk

Location: action/populate-contract-tasks.py:97-130

The script writes phases directly to the contract JSON without validating against the Pydantic models or JSON schema. If parse_plan produces malformed data, it will be written to the contract and discovered only when later operations fail.

Suggested fix: After constructing phases_data, validate it:

from egg_contracts import Contract
# After building phases_data, validate by loading through Pydantic
contract["phases"] = phases_data
try:
    Contract.model_validate(contract)
except ValidationError as e:
    print(f"Generated contract is invalid: {e}", file=sys.stderr)
    sys.exit(1)

5. pip install failure suppressed silently

Location: .github/workflows/sdlc-pipeline.yml:279

pip install -q pyyaml pydantic 2>/dev/null || true

If pyyaml or pydantic fail to install, the script will fail with an unclear ImportError rather than a clear dependency installation failure. The || true and stderr suppression hide the actual failure mode.

Fix: Remove || true and stderr redirection, or install dependencies in a prior setup step like the init job does.

---

Minor Issues

6. Hardcoded max_cycles: 3 — Inconsistent with default

The task dict hardcodes max_cycles: 3, matching the current schema default. However, if the schema default changes, this will become inconsistent. Consider importing the default from the model:

from egg_contracts.models import Task
# Use Task.model_fields["max_cycles"].default

7. No handling for partial write failures

If json.dump succeeds but the file is only partially written (disk full, etc.), the contract JSON could be corrupted. Consider using atomic write pattern:

import tempfile
with tempfile.NamedTemporaryFile(mode="w", dir=os.path.dirname(contract_path), delete=False) as f:
    json.dump(contract, f, indent=2)
    temp_path = f.name
os.replace(temp_path, contract_path)

---

Design Observations (non-blocking)

1. Idempotency check is effective: The early exit when total_tasks > 0 ensures re-runs are safe.

2. Comment search heuristic is reasonable: Looking for [TASK- and phase markers in reversed order (most recent first) should find the plan comment reliably.

3. Trusted script pattern is correct: Copying scripts from main checkout before switching to issue branch prevents untrusted branch code from modifying pipeline behavior.

---

Required Changes Before Approval

1. Fix JSON schema pattern for task IDs (Critical #1)
2. Add missing escalated field to task dict (Critical #2)
3. Add missing phase fields (Medium #3)

Recommended Changes

4. Add Pydantic validation before writing (Medium #4)
5. Remove silent pip failure suppression (Medium #5)

---

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

[james-in-a-box]

Agreed. Addressed all required and recommended changes:

1. Schema pattern fixed — contract.schema.json now allows task-1-1 format: ^task-[0-9]+(-[0-9]+)?$
2. Missing escalated field — added to task dicts
3. Missing phase fields — added review_cycles, max_cycles, escalated, escalation_reason, review_feedback
4. Pydantic validation — contract is validated via Contract.model_validate() before writing
5. pip install — removed || true and stderr suppression

Skipped #6 (importing model defaults) and #7 (atomic writes) — the model defaults are unlikely to change, and the git commit after write provides a recovery point.

— Authored by egg

[james-in-a-box]

Agent-Mode Design Review: No concerns (re-review).

The changes since my last review (d15386a) address code quality feedback from the base review bot:

• Fixed JSON schema pattern mismatch
• Removed silent pip install failure handling
• Added missing phase/task fields
• Added contract validation before write

My previous agent-mode assessment stands: this is pre-agent infrastructure that provides orienting metadata to help the agent understand what work is expected. It populates task context into the contract before the agent starts — this is helpful orientation, not constraint on agent exploration.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

[james-in-a-box]

Re-Review: Changes Since d15386a

All required and recommended changes from my previous review have been properly addressed:

Verified Fixes

1. Schema pattern fixed ✓ — contract.schema.json now uses ^task-[0-9]+(-[0-9]+)?$, matching the Pydantic model pattern exactly.

2. Missing escalated field ✓ — Added to task dicts at line 129.

3. Missing phase fields ✓ — All phase fields now included:

   • review_cycles: 0
   • max_cycles: 3
   • escalated: False
   • escalation_reason: None
   • review_feedback: []

4. Pydantic validation ✓ — Contract.model_validate(contract) is called before writing, with proper error handling that exits with code 1 on validation failure.

5. pip install failure handling ✓ — Removed || true and stderr suppression. Pip failures will now cause the step to fail clearly.

No New Issues

The changes are minimal and focused. No new concerns introduced.

Status: Ready for merge.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs