diff --git a/.egg-state/contracts/333.json b/.egg-state/contracts/333.json new file mode 100644 index 0000000000..b20e35f8e2 --- /dev/null +++ b/.egg-state/contracts/333.json @@ -0,0 +1,318 @@ +{ + "schemaVersion": "1.0", + "issue": { + "number": 333, + "title": "In the SDLC workflow, include a reviewer on the PR for contract verification", + "url": "https://github.com/jwbron/egg/issues/333" + }, + "current_phase": "implement", + "acceptance_criteria": [ + { + "id": "ac-1", + "description": "[TASK-1-1] Script generates prompt for contract verification, outputs prompt-file and model", + "verified": false + }, + { + "id": "ac-2", + "description": "[TASK-1-2] Workflow triggers on egg-sdlc labeled PRs, uses reusable-review.yml", + "verified": false + }, + { + "id": "ac-3", + "description": "[TASK-1-3] Contains rules for task/phase consistency and acceptance criteria verification", + "verified": false + }, + { + "id": "ac-4", + "description": "[TASK-2-1] SDLC PRs have egg-sdlc label applied", + "verified": false + }, + { + "id": "ac-5", + "description": "[TASK-2-2] Draft PRs remain draft until all participating reviewers approve", + "verified": false + }, + { + "id": "ac-6", + "description": "[TASK-2-3] Contract Verification excluded from check filter to prevent deadlock", + "verified": false + }, + { + "id": "ac-7", + "description": "[TASK-3-1] Prompt includes CLI instructions with REVIEWER role examples", + "verified": false + }, + { + "id": "ac-8", + "description": "[TASK-3-2] New CLI command updates acceptance_criteria.*.verified field", + "verified": false + }, + { + "id": "ac-9", + "description": "[TASK-3-3] Workflow sets EGG_AGENT_ROLE=reviewer for status mutations", + "verified": false + }, + { + "id": "ac-10", + "description": "[TASK-4-1] Reviews include egg-automated-review marker with bot=contract-verification", + "verified": false + }, + { + "id": "ac-11", + "description": "[TASK-4-2] Contract verification reviews trigger feedback loop on request-changes", + "verified": false + }, + { + "id": "ac-12", + "description": "[TASK-4-3] Re-review uses LAST_REVIEW_COMMIT for incremental verification", + "verified": false + } + ], + "phases": [ + { + "id": "phase-1", + "name": "Core Contract Review Infrastructure", + "status": "pending", + "tasks": [ + { + "id": "task-1-1", + "description": "Create action/build-contract-verification-prompt.sh", + "status": "pending", + "acceptance_criteria": "Script generates prompt for contract verification, outputs prompt-file and model", + "files_affected": [ + "action/build-contract-verification-prompt.sh" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-1-2", + "description": "Create .github/workflows/on-pull-request-contract-verify.yml", + "status": "pending", + "acceptance_criteria": "Workflow triggers on egg-sdlc labeled PRs, uses reusable-review.yml", + "files_affected": [ + ".github/workflows/on-pull-request-contract-verify.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-1-3", + "description": "Create .egg/contract-rules.md for verification rules", + "status": "pending", + "acceptance_criteria": "Contains rules for task/phase consistency and acceptance criteria verification", + "files_affected": [ + ".egg/contract-rules.md" + ], + "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": "PR Label and Draft-Ready Requirements", + "status": "pending", + "tasks": [ + { + "id": "task-2-1", + "description": "Add egg-sdlc label to PR in finalize-pr job", + "status": "pending", + "acceptance_criteria": "SDLC PRs have egg-sdlc label applied", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-2", + "description": "Update wait-for-checks to require all auto-reviewer approvals", + "status": "pending", + "acceptance_criteria": "Draft PRs remain draft until all participating reviewers approve", + "files_affected": [ + ".github/workflows/sdlc-pipeline.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-2-3", + "description": "Update check filter in reusable-review.yml for Contract Verification", + "status": "pending", + "acceptance_criteria": "Contract Verification excluded from check filter to prevent deadlock", + "files_affected": [ + ".github/workflows/reusable-review.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": "Contract Update Integration", + "status": "pending", + "tasks": [ + { + "id": "task-3-1", + "description": "Extend prompt to instruct agent on egg-contract mark-task usage", + "status": "pending", + "acceptance_criteria": "Prompt includes CLI instructions with REVIEWER role examples", + "files_affected": [ + "action/build-contract-verification-prompt.sh" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-2", + "description": "Add egg-contract verify-criterion command for acceptance criteria", + "status": "pending", + "acceptance_criteria": "New CLI command updates acceptance_criteria.*.verified field", + "files_affected": [ + "sandbox/egg_lib/contract_cli.py", + "tests/sandbox/test_contract_cli.py" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-3-3", + "description": "Ensure gateway session passes REVIEWER role for contract verification", + "status": "pending", + "acceptance_criteria": "Workflow sets EGG_AGENT_ROLE=reviewer for status mutations", + "files_affected": [ + ".github/workflows/on-pull-request-contract-verify.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": "Re-review and Feedback Loop Integration", + "status": "pending", + "tasks": [ + { + "id": "task-4-1", + "description": "Add automated review marker to contract verification reviews", + "status": "pending", + "acceptance_criteria": "Reviews include egg-automated-review marker with bot=contract-verification", + "files_affected": [ + "action/build-contract-verification-prompt.sh" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-4-2", + "description": "Integrate with on-review-feedback.yml for contract violations", + "status": "pending", + "acceptance_criteria": "Contract verification reviews trigger feedback loop on request-changes", + "files_affected": [ + ".github/workflows/on-review-feedback.yml" + ], + "commit": null, + "review_cycles": 0, + "max_cycles": 3, + "escalated": false, + "notes": "" + }, + { + "id": "task-4-3", + "description": "Update prompt for re-review with delta-focused verification", + "status": "pending", + "acceptance_criteria": "Re-review uses LAST_REVIEW_COMMIT for incremental verification", + "files_affected": [ + "action/build-contract-verification-prompt.sh" + ], + "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" + }, + "audit_log": [ + { + "timestamp": "2026-02-08T08:03:18Z", + "actor": "jwbron", + "role": "human", + "action": "transition", + "field_path": "current_phase", + "old_value": "refine", + "new_value": "plan", + "reason": "Phase approved by human" + }, + { + "timestamp": "2026-02-08T08:09:35Z", + "actor": "jwbron", + "role": "human", + "action": "transition", + "field_path": "current_phase", + "old_value": "plan", + "new_value": "implement", + "reason": "Phase approved by human" + } + ] +} diff --git a/.egg/contract-rules.md b/.egg/contract-rules.md new file mode 100644 index 0000000000..390a17c5fe --- /dev/null +++ b/.egg/contract-rules.md @@ -0,0 +1,91 @@ +# Contract Verification Rules + +Guidelines for verifying SDLC contract compliance. + +## Task Verification + +For each task in the contract, verify: + +### 1. Implementation Completeness +- The described functionality exists in the codebase +- All files listed in `files_affected` have been modified +- The implementation matches the task description + +### 2. Acceptance Criteria +- Each task has specific acceptance criteria +- Verify the criteria are objectively met, not partially or loosely +- If criteria mentions tests, verify tests exist and pass + +### 3. Commit Linkage +- If a commit is linked to a task, verify the commit's changes relate to the task +- Commits should be atomic and focused on their linked task +- Multiple commits per task is acceptable if they build on each other + +## Phase Consistency + +Check that phase status reflects task completion: + +- **pending**: No tasks started +- **in_progress**: At least one task started, not all complete +- **complete**: All tasks marked complete and verified +- **blocked**: Task(s) cannot proceed due to external dependency + +### Red Flags +- Phase marked complete but tasks are pending +- Tasks marked complete but code is missing +- Orphaned code not covered by any task + +## Acceptance Criteria Verification + +The contract contains top-level `acceptance_criteria` that map to tasks: + +``` +[TASK-1-1] Description here +``` + +For each criterion: + +1. **Read carefully**: Understand exactly what must be true +2. **Examine evidence**: Find the code, test, or artifact that proves compliance +3. **Verify objectively**: Don't assume—check that it actually works +4. **Mark verified**: Use `egg-contract verify-criterion --criterion ac-N` + +### Verification Standards + +**Verified** means: +- Code exists and is syntactically correct +- Logic matches the requirement +- Edge cases are handled where specified +- Tests pass (if testing is part of criteria) + +**Not Verified** means: +- Code is missing or incomplete +- Logic doesn't match requirement +- Obvious bugs or missing error handling +- Tests fail or don't exist where required + +## Contract Integrity on Re-review + +When re-reviewing after changes: + +1. **Check regressions**: Ensure previously verified criteria still hold +2. **Verify new work**: Apply full verification to newly completed tasks +3. **Flag violations**: If a change breaks a verified criterion, flag it clearly + +## Review Verdict Guidelines + +### Approve +- All acceptance criteria are verified +- All tasks are complete and correctly implemented +- No contract violations found + +### Request Changes +- One or more acceptance criteria not met +- Task implementation doesn't match description +- Contract violations found +- Missing tests for tasks that require them + +### Comment +- Minor suggestions that don't block approval +- Questions about implementation choices +- Notes for human reviewer diff --git a/.github/workflows/on-pull-request-contract-verify.yml b/.github/workflows/on-pull-request-contract-verify.yml new file mode 100644 index 0000000000..8eb9e0a346 --- /dev/null +++ b/.github/workflows/on-pull-request-contract-verify.yml @@ -0,0 +1,86 @@ +name: "egg: Contract Verification" + +# Trigger on PRs with the egg-sdlc label +# This workflow verifies that implementation matches the SDLC contract +on: + pull_request: + types: [opened, synchronize, ready_for_review, reopened, labeled] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to verify' + required: true + type: number + +jobs: + should-run: + name: Check if verification should run + runs-on: ubuntu-latest + outputs: + run: ${{ steps.check.outputs.run }} + issue_number: ${{ steps.check.outputs.issue_number }} + steps: + - name: Generate bot token + id: bot-token + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ secrets.BOT_APP_ID }} + private-key: ${{ secrets.BOT_APP_PRIVATE_KEY }} + + - name: Check for egg-sdlc label and extract issue number + id: check + env: + GH_TOKEN: ${{ steps.bot-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} + run: | + set -euo pipefail + + # For workflow_dispatch, always run + if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + # Check if PR has egg-sdlc label + labels=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}" --jq '.labels[].name') + if echo "$labels" | grep -q "^egg-sdlc$"; then + echo "PR has egg-sdlc label, running contract verification" + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "PR does not have egg-sdlc label, skipping" + echo "run=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + fi + + # Extract issue number from branch name (egg/issue-{number}) + branch_name=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}" --jq '.head.ref') + if [[ "$branch_name" =~ ^egg/issue-([0-9]+)$ ]]; then + issue_number="${BASH_REMATCH[1]}" + echo "Extracted issue number from branch: $issue_number" + echo "issue_number=${issue_number}" >> "$GITHUB_OUTPUT" + else + echo "Could not extract issue number from branch name: $branch_name" + echo "issue_number=" >> "$GITHUB_OUTPUT" + fi + + verify: + name: Contract Verification + needs: should-run + if: needs.should-run.outputs.run == 'true' + permissions: + contents: read + pull-requests: write + uses: ./.github/workflows/reusable-review.yml + with: + pr_number: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} + bot_name: contract-verification + prompt_script: "action/build-contract-verification-prompt.sh" + timeout: "15" + pr_title: ${{ github.event.pull_request.title || '' }} + event_name: ${{ github.event_name }} + agent_role: reviewer + issue_number: ${{ needs.should-run.outputs.issue_number }} + secrets: + BOT_APP_ID: ${{ secrets.BOT_APP_ID }} + BOT_APP_PRIVATE_KEY: ${{ secrets.BOT_APP_PRIVATE_KEY }} + BOT_APP_INSTALLATION_ID: ${{ secrets.BOT_APP_INSTALLATION_ID }} + ANTHROPIC_OAUTH_TOKEN: ${{ secrets.ANTHROPIC_OAUTH_TOKEN }} diff --git a/.github/workflows/reusable-review.yml b/.github/workflows/reusable-review.yml index ec13e959c0..b3b634417f 100644 --- a/.github/workflows/reusable-review.yml +++ b/.github/workflows/reusable-review.yml @@ -31,6 +31,16 @@ on: required: false type: string default: "pull_request" + agent_role: + description: 'Agent role for contract mutations (implementer or reviewer)' + required: false + type: string + default: "" + issue_number: + description: 'Issue number for contract operations (extracted from branch name if not provided)' + required: false + type: string + default: "" secrets: BOT_APP_ID: required: true @@ -115,9 +125,10 @@ jobs: # - "Design Review" matches "egg: Agent-Mode Design Review / Agent-Mode Design Review" # - "SDLC Pipeline" matches "egg: SDLC Pipeline / Initialize pipeline", etc. # - "SDLC HITL" matches "egg: SDLC HITL Decision Handler / Handle HITL decision", etc. + # - "Contract Verification" matches "egg: Contract Verification / ..." checks=$(gh api "repos/${{ github.repository }}/commits/${HEAD_SHA}/check-runs" \ --jq '.check_runs | [.[] | select( - (.name | test("Code Review|Design Review|SDLC Pipeline|SDLC HITL") | not) + (.name | test("Code Review|Design Review|SDLC Pipeline|SDLC HITL|Contract Verification") | not) ) | {name, status, conclusion}]') total=$(echo "$checks" | jq 'length') @@ -337,6 +348,7 @@ jobs: GH_TOKEN: ${{ steps.bot-token.outputs.token }} PR_NUMBER: ${{ env.PR_NUMBER }} LAST_REVIEW_COMMIT: ${{ steps.last-review.outputs.commit-sha }} + COMMIT_SHA: ${{ steps.pr-meta.outputs.head-sha }} - name: Checkout PR code for review uses: actions/checkout@v4 @@ -359,6 +371,8 @@ jobs: timeout: ${{ inputs.timeout }} env: EGG_BOT_NAME: ${{ inputs.bot_name }} + EGG_AGENT_ROLE: ${{ inputs.agent_role }} + EGG_ISSUE_NUMBER: ${{ inputs.issue_number }} - name: Post workflow run status to PR if: always() && !cancelled() diff --git a/.github/workflows/sdlc-pipeline.yml b/.github/workflows/sdlc-pipeline.yml index 14d63cc3d1..83d949a3bd 100644 --- a/.github/workflows/sdlc-pipeline.yml +++ b/.github/workflows/sdlc-pipeline.yml @@ -487,6 +487,11 @@ jobs: echo "Created draft PR #${PR_NUMBER}" echo "pr_number=${PR_NUMBER}" >> "$GITHUB_OUTPUT" + # Add egg-sdlc label to trigger contract verification workflow + gh pr edit "${PR_NUMBER}" --add-label "egg-sdlc" || { + echo "::warning::Failed to add egg-sdlc label (label may not exist)" + } + - name: Post status comment if: always() env: @@ -551,7 +556,7 @@ jobs: app-id: ${{ secrets.BOT_APP_ID }} private-key: ${{ secrets.BOT_APP_PRIVATE_KEY }} - - name: Wait for all check runs to pass + - name: Wait for all check runs to complete id: wait env: GH_TOKEN: ${{ steps.bot-token.outputs.token }} @@ -566,39 +571,34 @@ jobs: exit 0 fi + # Use the final HEAD SHA captured after implementation completes, + # rather than fetching via API, to avoid race conditions where new + # commits push a different SHA between poll iterations + HEAD_SHA="${{ needs.implement.outputs.head_sha }}" + if [[ -z "$HEAD_SHA" ]]; then + # Fallback: fetch from API if implement didn't produce a SHA + HEAD_SHA=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}" --jq '.head.sha') + echo "::warning::Using HEAD SHA from API (implement job did not output head_sha)" + fi + echo "Waiting for checks on commit: ${HEAD_SHA}" + + # Wait for all check runs to complete (CI, reviews, etc.) # Excludes this workflow's own checks to avoid self-deadlock SELF_WORKFLOW="egg: SDLC Pipeline" - # Maximum wait time (30 minutes) + # Maximum wait time (30 minutes — reviews can take a while) MAX_WAIT_SECONDS=1800 POLL_INTERVAL=30 start_time=$(date +%s) - prev_sha="" zero_checks_iterations=0 # After 10 minutes with no checks, assume none are configured and pass MAX_ZERO_CHECKS_ITERATIONS=20 while true; do + # Use wall-clock elapsed time (accounts for API latency, not just sleep) elapsed=$(( $(date +%s) - start_time )) if [[ $elapsed -ge $MAX_WAIT_SECONDS ]]; then - echo "::warning::Timeout waiting for checks after ${MAX_WAIT_SECONDS} seconds" - echo "passed=false" >> "$GITHUB_OUTPUT" - echo "timed_out=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - - # Fetch current HEAD SHA of the PR on each iteration. - # Autofix workflows may push new commits, changing the HEAD. - HEAD_SHA=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}" --jq '.head.sha') - - if [[ "$HEAD_SHA" != "$prev_sha" ]]; then - if [[ -n "$prev_sha" ]]; then - echo "HEAD SHA changed: ${prev_sha:0:7} -> ${HEAD_SHA:0:7} (new commit detected)" - else - echo "Watching checks on commit: ${HEAD_SHA:0:7}" - fi - prev_sha="$HEAD_SHA" - zero_checks_iterations=0 + break fi # Get all check runs for this commit @@ -614,7 +614,7 @@ jobs: failed=$(echo "$external_checks" | jq '[.[] | select(.status == "completed" and .conclusion != "success" and .conclusion != "skipped" and .conclusion != "neutral" and .conclusion != "cancelled")] | length') pending=$(echo "$external_checks" | jq -r '[.[] | select(.status != "completed")] | map(.name) | join(", ")') - echo "Checks: ${completed}/${total} completed, ${failed} failed (elapsed: ${elapsed}s)" + echo "Checks: ${completed}/${total} completed, ${failed} failed" if [[ "$total" -eq 0 ]]; then zero_checks_iterations=$((zero_checks_iterations + 1)) @@ -628,27 +628,58 @@ jobs: sleep $POLL_INTERVAL continue fi + # Reset counter once checks appear zero_checks_iterations=0 - # All checks completed and all passed — success - if [[ "$completed" -eq "$total" && "$failed" -eq 0 ]]; then - echo "All ${total} checks passed" + # All checks completed + if [[ "$completed" -eq "$total" ]]; then + # All checks completed but some failed — keep waiting, autofix may push a new commit + if [[ "$failed" -gt 0 ]]; then + echo "Checks completed with ${failed} failure(s) — waiting for autofix..." + echo "$external_checks" | jq -r '.[] | select(.status == "completed" and .conclusion != "success" and .conclusion != "skipped" and .conclusion != "neutral" and .conclusion != "cancelled") | " FAILED: \(.name) (\(.conclusion))"' + sleep $POLL_INTERVAL + continue + fi + + # All checks passed - now verify automated reviewers have approved + # Get all reviews with egg-automated-review marker + reviews=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/reviews" \ + --jq '[.[] | select(.body | contains("egg-automated-review"))]') + + # Check for any request-changes reviews that haven't been superseded + # Group by user, take latest review per user + latest_reviews=$(echo "$reviews" | jq ' + group_by(.user.login) | + map(sort_by(.submitted_at) | last) | + [.[] | {user: .user.login, state: .state}] + ') + + # Check if any automated reviewer has requested changes + changes_requested=$(echo "$latest_reviews" | jq '[.[] | select(.state == "CHANGES_REQUESTED")] | length') + + if [[ "$changes_requested" -gt 0 ]]; then + echo "::warning::Automated reviewer(s) requested changes" + echo "$latest_reviews" | jq -r '.[] | select(.state == "CHANGES_REQUESTED") | " \(.user): CHANGES_REQUESTED"' + echo "passed=false" >> "$GITHUB_OUTPUT" + echo "timed_out=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + echo "All ${total} checks passed and no automated reviewers blocking" echo "passed=true" >> "$GITHUB_OUTPUT" echo "timed_out=false" >> "$GITHUB_OUTPUT" exit 0 fi - # All checks completed but some failed — keep waiting, autofix may push a new commit - if [[ "$completed" -eq "$total" && "$failed" -gt 0 ]]; then - echo "Checks completed with ${failed} failure(s) — waiting for autofix..." - echo "$external_checks" | jq -r '.[] | select(.status == "completed" and .conclusion != "success" and .conclusion != "skipped" and .conclusion != "neutral" and .conclusion != "cancelled") | " FAILED: \(.name) (\(.conclusion))"' - else - echo "Waiting for: ${pending}" - fi + echo "Waiting for: ${pending}" sleep $POLL_INTERVAL done + echo "::warning::Timeout waiting for checks after ${MAX_WAIT_SECONDS} seconds" + echo "passed=false" >> "$GITHUB_OUTPUT" + echo "timed_out=true" >> "$GITHUB_OUTPUT" + # ============================================================ # Finalize PR: Mark draft PR ready for human review # ============================================================ @@ -887,7 +918,7 @@ jobs: --- Authored by egg" fi - gh issue comment "${ISSUE_NUMBER}" --repo "${{ github.repository }}" --body "$BODY" + gh issue comment "${ISSUE_NUMBER}" --body "$BODY" # ============================================================ # Refine: Analyze issue and produce analysis document diff --git a/action/build-contract-verification-prompt.sh b/action/build-contract-verification-prompt.sh new file mode 100755 index 0000000000..d68a5b462c --- /dev/null +++ b/action/build-contract-verification-prompt.sh @@ -0,0 +1,230 @@ +#!/usr/bin/env bash +# build-contract-verification-prompt.sh — Build prompt for contract verification review +# +# This script creates a prompt for a contract verification reviewer agent. The agent +# verifies that the implementation matches the contract (tasks completed, acceptance +# criteria met), marks verified items via egg-contract CLI, and identifies gaps. +# +# Environment variables: +# PR_NUMBER — Pull request number +# GITHUB_REPOSITORY — owner/repo +# RUNNER_TEMP — Temp directory for prompt file +# LAST_REVIEW_COMMIT — (Optional) Commit SHA of last review, for incremental verification +# COMMIT_SHA — Current PR head commit SHA (for review marker) +# EGG_ISSUE_NUMBER — Issue number for the contract +# +# Output: +# Sets 'prompt-file' and 'model' in $GITHUB_OUTPUT + +set -euo pipefail + +# --------------------------------------------------------------------------- +# Fetch contract rules (or use defaults) +# --------------------------------------------------------------------------- + +fetch_contract_rules() { + local rules_file=".egg/contract-rules.md" + + if [[ -f "$rules_file" ]]; then + cat "$rules_file" + else + # Default contract verification rules when no repo-specific rules exist + cat <<'EOF' +## Default Contract Verification Rules + +### Task Verification + +For each task in the contract, verify: + +1. **Implementation exists**: The described functionality is present in the code +2. **Acceptance criteria met**: The specific acceptance criteria for the task is satisfied +3. **Commit linked**: If a commit is linked, verify it relates to the task +4. **Tests present**: Where applicable, tests cover the new functionality + +### Phase Consistency + +Check that: +- All tasks in completed phases are actually implemented +- Phase status matches task completion state +- No orphaned code exists that isn't covered by any task + +### Acceptance Criteria Verification + +For each acceptance criterion in the contract: +1. Read the criterion description +2. Examine the implementation to verify it meets the criterion +3. If verified, mark it using: `egg-contract verify-criterion --criterion ` +4. If not verified, note the gap in your review + +### Contract Integrity + +Verify: +- No implementation changes violate previously verified criteria +- New changes don't break existing contract compliance +- All required files listed in tasks are present +EOF + fi +} + +# --------------------------------------------------------------------------- +# Build the prompt +# --------------------------------------------------------------------------- + +build_prompt() { + local contract_rules + contract_rules=$(fetch_contract_rules) + + # Load review conventions if available + local conventions_file + conventions_file="$(dirname "$0")/review-conventions.md" + local conventions="" + if [[ -f "$conventions_file" ]]; then + conventions=$(cat "$conventions_file") + fi + + local prompt + local is_rereview=false + + # Check if this is a re-review (we have a previous review commit) + if [[ -n "${LAST_REVIEW_COMMIT:-}" ]]; then + is_rereview=true + prompt="Re-verify contract compliance for PR #${PR_NUMBER} in ${GITHUB_REPOSITORY}. + +This is a **re-review** — you previously verified this PR at commit \`${LAST_REVIEW_COMMIT}\`. + +## Your Task + +Perform **incremental contract verification**. Focus on changes since your last review and verify they don't violate the contract. + +1. **Review the delta**: Use \`git diff ${LAST_REVIEW_COMMIT}..HEAD\` to see what changed since your last review. +2. **Check contract state**: Run \`egg-contract show\` to see the current contract with all tasks and acceptance criteria. +3. **Verify new changes comply**: Ensure new code doesn't break any previously verified acceptance criteria. +4. **Verify newly completed tasks**: If any tasks were completed since last review, verify their implementation. +5. **Mark verified criteria**: For any acceptance criteria now fully verified, run: + \`egg-contract verify-criterion --criterion ac-N\` +6. **Identify gaps**: Note any contract violations or incomplete implementations. + +### CLI Commands (REVIEWER role) + +You are running with REVIEWER role, which allows you to mark task and criterion status: + +\`\`\`bash +# View contract state +egg-contract show + +# Mark acceptance criterion as verified +egg-contract verify-criterion --criterion ac-1 + +# Mark task status (complete, incomplete, blocked) +egg-contract mark-task --task task-1-1 --status complete + +# Mark phase status +egg-contract mark-phase --phase phase-1 --passed true +\`\`\` + +For full PR context if needed: \`gh pr diff ${PR_NUMBER}\` + +## Contract Rules + +${contract_rules} + +## Review Conventions + +${conventions:-Post your review using \`gh pr review ${PR_NUMBER}\` with \`--body-file\`. Always write your review to a temp file first, then use --body-file to post it. Do NOT use --body with inline content — long reviews will fail due to shell escaping. Use --approve, --request-changes, or --comment as appropriate. Sign your review with: — Authored by egg} + +## Review Marker + +Your review MUST include this HTML comment at the end of your review body for tracking: + +\`\`\` + +\`\`\` +" + else + prompt="Verify contract compliance for PR #${PR_NUMBER} in ${GITHUB_REPOSITORY}. + +## Your Task + +Perform **comprehensive contract verification**. Verify that the implementation matches the contract and all acceptance criteria are met. + +### How to Proceed + +1. **Get the contract**: Run \`egg-contract show\` to see all tasks and acceptance criteria. +2. **Get the PR diff**: Run \`gh pr diff ${PR_NUMBER}\` to see all code changes. +3. **Verify each task**: + - For each task, check that the implementation matches the task description + - Verify the acceptance criteria for each task is satisfied + - Check that linked commits relate to their tasks +4. **Mark verified criteria**: For each acceptance criterion that is fully verified, run: + \`egg-contract verify-criterion --criterion ac-N\` +5. **Identify gaps**: Note any tasks that are incomplete or don't meet their criteria. +6. **Check for violations**: Ensure no code changes violate contract requirements. + +### CLI Commands (REVIEWER role) + +You are running with REVIEWER role, which allows you to mark task and criterion status: + +\`\`\`bash +# View contract state +egg-contract show + +# Mark acceptance criterion as verified +egg-contract verify-criterion --criterion ac-1 + +# Mark task status (complete, incomplete, blocked) +egg-contract mark-task --task task-1-1 --status complete + +# Mark phase status +egg-contract mark-phase --phase phase-1 --passed true +\`\`\` + +### Be Thorough + +Verify every task and every acceptance criterion. This is the last check before human review. If something doesn't match the contract, flag it clearly. + +## Contract Rules + +${contract_rules} + +## Review Conventions + +${conventions:-Post your review using \`gh pr review ${PR_NUMBER}\` with \`--body-file\`. Always write your review to a temp file first, then use --body-file to post it. Do NOT use --body with inline content — long reviews will fail due to shell escaping. Use --approve, --request-changes, or --comment as appropriate. Sign your review with: — Authored by egg} + +## Review Marker + +Your review MUST include this HTML comment at the end of your review body for tracking: + +\`\`\` + +\`\`\` +" + fi + + # Write prompt to temp file + local prompt_file="${RUNNER_TEMP:-/tmp}/contract-verification-prompt-${PR_NUMBER}.txt" + echo "$prompt" > "$prompt_file" + + # Always use opus for contract verification (needs thorough reasoning) + local model="opus" + + # Write outputs + { + echo "prompt-file=${prompt_file}" + echo "model=${model}" + } >> "${GITHUB_OUTPUT:-/dev/null}" + + local review_type="initial" + if [[ "$is_rereview" == "true" ]]; then + review_type="re-review (since ${LAST_REVIEW_COMMIT:0:7})" + fi + echo "Contract verification prompt built: ${#prompt} chars, model=${model}, type=${review_type}" +} + +# --------------------------------------------------------------------------- +# Main +# --------------------------------------------------------------------------- + +: "${PR_NUMBER:?PR_NUMBER is required}" +: "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}" + +build_prompt diff --git a/sandbox/egg_lib/cli.py b/sandbox/egg_lib/cli.py index 03b3657e7f..15479f71e3 100644 --- a/sandbox/egg_lib/cli.py +++ b/sandbox/egg_lib/cli.py @@ -277,6 +277,11 @@ def gha_exec() -> int: if issue_number: extra_env["EGG_ISSUE_NUMBER"] = issue_number + # Pass agent role for gateway authorization (e.g., reviewer role) + agent_role = os.environ.get("EGG_AGENT_ROLE") + if agent_role: + extra_env["EGG_AGENT_ROLE"] = agent_role + success_flag = exec_in_new_container( command=command, timeout_minutes=timeout, diff --git a/sandbox/egg_lib/contract_cli.py b/sandbox/egg_lib/contract_cli.py index c232991da7..ae16ea2ff8 100755 --- a/sandbox/egg_lib/contract_cli.py +++ b/sandbox/egg_lib/contract_cli.py @@ -105,6 +105,29 @@ def parse_task_id(task_id: str) -> tuple[int, int]: raise ValueError(f"Invalid task ID '{task_id}': expected numeric values") from e +def parse_criterion_id(criterion_id: str) -> int: + """Parse criterion ID and return criterion_idx. + + Args: + criterion_id: Criterion ID in format "ac-N" + + Returns: + Criterion index as 0-based + + Raises: + ValueError: If criterion ID format is invalid or number is out of range + """ + try: + criterion_num = int(criterion_id.lower().replace("ac-", "")) + if criterion_num < 1: + raise ValueError(f"Criterion number must be >= 1: {criterion_id}") + return criterion_num - 1 + except ValueError as e: + if "must be >= 1" in str(e): + raise + raise ValueError(f"Invalid criterion ID '{criterion_id}': expected format 'ac-N'") from e + + def parse_phase_id(phase_id: str) -> int: """Parse phase ID and return phase_idx. @@ -490,6 +513,47 @@ def format_decision_markdown(decision_id: str, question: str, options: list[dict return "\n".join(lines) +def cmd_verify_criterion(args: argparse.Namespace) -> int: + """Mark an acceptance criterion as verified. + + Note: This operation requires REVIEWER role. Agents running as IMPLEMENTER + will receive a role authorization error from the gateway. This command is + used by contract verification reviewers to mark criteria as verified. + """ + issue_number = args.issue or get_issue_number() + if not issue_number: + print("Error: Issue number required", file=sys.stderr) + return 1 + + try: + criterion_idx = parse_criterion_id(args.criterion) + except ValueError as e: + print(f"Error: {e}", file=sys.stderr) + return 1 + + field_path = f"acceptance_criteria.{criterion_idx}.verified" + + result = make_gateway_request( + "/api/v1/contract/mutate", + method="POST", + data={ + "issue_number": issue_number, + "repo_path": args.repo_path or get_repo_path(), + "field_path": field_path, + "new_value": True, + "actor": "egg", + "reason": f"Verified criterion {args.criterion}", + }, + ) + + if result.get("success"): + print(f"Verified criterion {args.criterion}") + return 0 + else: + print(f"Error: {result.get('message')}", file=sys.stderr) + return 1 + + def cmd_add_decision(args: argparse.Namespace) -> int: """Create a HITL decision point. @@ -639,6 +703,15 @@ def create_parser() -> argparse.ArgumentParser: ) mark_phase_parser.set_defaults(func=cmd_mark_phase) + # verify-criterion command (requires REVIEWER role) + verify_criterion_parser = subparsers.add_parser( + "verify-criterion", help="Mark acceptance criterion as verified (requires REVIEWER role)" + ) + verify_criterion_parser.add_argument( + "--criterion", required=True, help="Criterion ID (e.g., ac-1)" + ) + verify_criterion_parser.set_defaults(func=cmd_verify_criterion) + # add-decision command decision_parser = subparsers.add_parser("add-decision", help="Create HITL decision point") decision_parser.add_argument("--question", required=True, help="Decision question") diff --git a/tests/sandbox/test_contract_cli.py b/tests/sandbox/test_contract_cli.py index f4a6a3443b..d973f6e8f6 100644 --- a/tests/sandbox/test_contract_cli.py +++ b/tests/sandbox/test_contract_cli.py @@ -21,6 +21,7 @@ get_issue_number, get_repo_path, main, + parse_criterion_id, parse_phase_id, parse_task_id, validate_commit_sha, @@ -142,6 +143,13 @@ def test_add_decision_format_default_json(self): args = parser.parse_args(["add-decision", "--question", "Which approach?"]) assert args.format == "json" + def test_verify_criterion_command(self): + """Test parsing verify-criterion command.""" + parser = create_parser() + args = parser.parse_args(["verify-criterion", "--criterion", "ac-1"]) + assert args.command == "verify-criterion" + assert args.criterion == "ac-1" + class TestEnvironmentHelpers: """Tests for environment variable helpers.""" @@ -243,6 +251,32 @@ def test_task_id_negative_numbers(self): parse_task_id("task--1") +class TestCriterionIdParsing: + """Tests for criterion ID parsing.""" + + def test_valid_criterion_id(self): + """Test parsing valid criterion ID.""" + criterion_idx = parse_criterion_id("ac-1") + assert criterion_idx == 0 + + def test_criterion_id_case_insensitive(self): + """Test that criterion ID parsing is case insensitive.""" + criterion_idx = parse_criterion_id("AC-2") + assert criterion_idx == 1 + + def test_criterion_id_non_numeric(self): + """Test that non-numeric criterion ID raises ValueError.""" + with pytest.raises(ValueError) as exc_info: + parse_criterion_id("ac-abc") + assert "Invalid criterion ID" in str(exc_info.value) + + def test_criterion_id_zero(self): + """Test that criterion number 0 raises ValueError.""" + with pytest.raises(ValueError) as exc_info: + parse_criterion_id("ac-0") + assert "must be >= 1" in str(exc_info.value) + + class TestPhaseIdParsing: """Tests for phase ID parsing.""" @@ -485,6 +519,30 @@ def test_add_commit_too_short_sha(self, capsys): captured = capsys.readouterr() assert "Invalid commit SHA" in captured.err + def test_verify_criterion_no_issue_number(self, capsys): + """Test verify-criterion without issue number.""" + with patch.dict("os.environ", {}, clear=True): + result = main(["verify-criterion", "--criterion", "ac-1"]) + assert result == 1 + captured = capsys.readouterr() + assert "Issue number required" in captured.err + + def test_verify_criterion_invalid_criterion_id(self, capsys): + """Test verify-criterion with invalid criterion ID.""" + with patch.dict("os.environ", {"EGG_ISSUE_NUMBER": "123"}): + result = main(["verify-criterion", "--criterion", "ac-abc"]) + assert result == 1 + captured = capsys.readouterr() + assert "Invalid criterion ID" in captured.err + + def test_verify_criterion_zero_criterion(self, capsys): + """Test verify-criterion with zero criterion number.""" + with patch.dict("os.environ", {"EGG_ISSUE_NUMBER": "123"}): + result = main(["verify-criterion", "--criterion", "ac-0"]) + assert result == 1 + captured = capsys.readouterr() + assert "must be >= 1" in captured.err + class TestCommitShaValidation: """Tests for commit SHA validation."""