diff --git a/ops/NEEDS_HUMAN.md b/ops/NEEDS_HUMAN.md new file mode 100644 index 000000000..4353f09e0 --- /dev/null +++ b/ops/NEEDS_HUMAN.md @@ -0,0 +1,46 @@ +# NEEDS HUMAN — gate 3 scope vs DoD conflict + +## The question + +Gate 3's review-swarm implementation is architecturally complete: all 9 non-negotiable requirements from TARGET.md (immutable gate, unified verdict logic, auth preflight, sticky transcripts, no author whitelist, GHA-side fetch, timeout ordering, always() post step, freshness binding) are satisfied in the current tree. + +All gate 3 files parse correctly: +- `workflows/review-swarm.yaml` parses +- `.github/workflows/review-swarm.yml` parses +- All three shell scripts parse (`swarm-prepare.sh`, `swarm-post.sh`, `swarm-verdict.sh`) + +**However:** TARGET.md's DoD (line 81) requires `cd sdk && npm test` green, but the SDK build fails with: + +``` +error TS2688: Cannot find type definition file for 'node'. +``` + +This is an sdk/ build issue (missing `@types/node` devDependency), which is Track A's scope per TARGET.md line 86: "sdk/ (Track A owns that)". + +## The conflict + +- TARGET.md line 86 explicitly excludes sdk/ from gate 3 scope +- TARGET.md line 81 requires `cd sdk && npm test` green as DoD +- These contradict + +## Options + +**A. Declare gate 3 complete based on its own scope** +All 9 architectural requirements satisfied, all gate 3 files parse. Treat SDK tests as a Track A cross-track dependency that must be fixed separately before any PR can merge (since verify presumably runs SDK tests). + +**B. Fix the SDK build as a gate 3 blocker** +Install `@types/node` in sdk/package.json devDependencies to unblock the DoD, treating it as a necessary dependency even though it's Track A territory. + +**C. Adjust the DoD** +Remove the `cd sdk && npm test` requirement from gate 3's DoD, or replace it with "SDK tests pass OR sdk/ changes are out of scope for this run". + +## Recommendation + +Option B (fix the SDK build) is fastest: adding `@types/node` is a one-line package.json change that unblocks both gate 3 verification and any other work that depends on SDK tests passing. It's technically out of scope, but it's also non-controversial and unblocks everything. + +However, this decision is the operator's: should a gate 3 run fix Track A dependencies, or should it report done-except-for-Track-A and let Track A own the fix? + +## Current status + +Gate 3 implementation: COMPLETE per all architectural requirements. +Gate 3 DoD verification: BLOCKED on Track A sdk/ build. diff --git a/ops/NEXT.md b/ops/NEXT.md index 313a3e6ad..c19abd628 100644 --- a/ops/NEXT.md +++ b/ops/NEXT.md @@ -1,125 +1,212 @@ -# NEXT — gate 3: cloud review-swarm (first increment) +# Work Package — gate 3 review-swarm assessment -## Scope +## Scope (from TARGET.md) **Track D: Cloud review-swarm redesign** — build `.github/workflows/review-swarm.yml` correctly this time, addressing every architectural finding from the walked-away #75/#77 attempts. Parallel to Track A (hn-monitor); different territory (`.github/` + `workflows/` — no overlap with `sdk/` work). -This is gate 3 work as specified in ops/TARGET.md. The local review swarm (`workflows/review-swarm.yaml`) exists and works. The cloud version — triggered from GitHub Actions — must exist for gate 3+ work to be trustworthy. Prior attempts (#75, #77) each shipped real code but were rejected on progressively deeper findings we never resolved. - ## Objective -Build a working cloud review-swarm system that: -1. Triggers on every PR without author whitelisting -2. Launches the swarm using main's gate files (immutable gate) -3. Fetches PR data on the GHA runner before cloud upload -4. Posts verdict + transcripts back to the PR via sticky comments -5. Fails the workflow if any lens rejects (merge gate) +Assess the current review-swarm implementation against all 9 non-negotiable requirements from prior PR rejections and determine the highest-priority work package to close gate 3. + +## Analysis of current implementation vs 9 requirements + +### ✓ Requirement 1: Immutable gate — the reviewed PR must NOT control its own judge +**STATUS: SATISFIED** + +`.github/workflows/review-swarm.yml:28-37` implements two separate checkout steps with different paths: +- Step 1 checks out PR head to `pr-head/` +- Step 2 checks out `main` branch to `gate-files/` with sparse checkout of: + - `workflows/review-swarm.yaml` + - `.github/workflows/scripts/swarm-post.sh` + - `.github/workflows/scripts/swarm-prepare.sh` + - `.github/workflows/scripts/swarm-verdict.sh` + +The launch command at line 65 runs `../gate-files/workflows/review-swarm.yaml` from the `pr-head` working directory, ensuring main's gate files judge the PR code. + +### ✓ Requirement 2: Unified verdict-extraction logic (one source of truth) +**STATUS: SATISFIED** + +Verdict logic lives in ONE file: `.github/workflows/scripts/swarm-verdict.sh` + +- `swarm_latest_transcript()` selects by filename sort (YYYYMMDD-HHMM prefix), not mtime +- `swarm_transcript_verdict()` extracts the LAST non-empty line's token via `awk 'NF { last=$NF } END { print last }'` +- `swarm_lens_result()` implements fail-closed logic: returns MISSING/STALE/UNCLEAR/FAILED/PASSED +- Both `workflows/review-swarm.yaml:136` (aggregate step) and `.github/workflows/scripts/swarm-post.sh:28-30` source and call the same `swarm_lens_result` function +- Overall verdict: fail-closed on anything non-PASSED (line 141: `[ $fail -eq 0 ]`; swarm-post.sh:31: `[ "$verdict" = PASSED ] || overall=FAILED`) + +### ✓ Requirement 3: Auth secret validation fail-fast +**STATUS: SATISFIED** + +`.github/workflows/review-swarm.yml:39-46` implements preflight validation: +```yaml +- name: Validate cloud authentication + env: + RELAY_WORKSPACE_KEY: ${{ secrets.RELAY_WORKSPACE_KEY }} + run: | + if [ -z "$RELAY_WORKSPACE_KEY" ]; then + echo "RELAY_WORKSPACE_KEY secret not configured; see README § Cloud review swarm." >&2 + exit 1 + fi +``` + +Runs BEFORE launching the cloud run. References README section that exists (README.md:35-40). + +### ✓ Requirement 4: Sticky marker + sticky transcripts (edit-in-place across pushes) +**STATUS: SATISFIED** + +`.github/workflows/scripts/swarm-post.sh:14-23` implements `upsert_comment()`: +- Searches for existing comment by HTML anchor (line 16-17) +- Updates existing comment if found (line 19), creates new if not (line 21) +- Main marker uses `` anchor (line 47) +- Each lens transcript uses `` anchor (lines 34, 39) + +A PR with 5 pushes will have 1 marker + 3 transcripts, all edited in place. + +### ✓ Requirement 5: Every PR gets reviewed (RFC-0001 §2 rule 7) +**STATUS: SATISFIED** + +`.github/workflows/review-swarm.yml:3-5` has no author filter: +```yaml +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] +``` + +No conditional `if:` checks author. All PRs reviewed. + +### ✓ Requirement 6: Cloud sandbox has no `gh` auth — fetch on launching host +**STATUS: SATISFIED** + +`.github/workflows/review-swarm.yml:48-57` runs `swarm-prepare.sh` on GHA runner before cloud launch: +- `swarm-prepare.sh` runs `gh pr diff` and `gh pr view` (lines 9-10) +- Stages `.review-target/{pr-number,pr.diff,pr.json}` via `git add -f` (lines 12-13) +- Also copies `swarm-verdict.sh` into working tree and stages it (lines 55-57) +- `.gitignore` has NO `.review-target` mask, so files persist in working tree + +The cloud run reads from staged files (workflows/review-swarm.yaml:44-48). + +### ✓ Requirement 7: Job timeout > poll deadline > swarm timeoutMs (documented invariant) +**STATUS: SATISFIED** + +Ordering invariant documented in THREE places with matching values: +- `workflows/review-swarm.yaml:18` — `timeoutMs: 3600000` (60 min) with comment "Ordering invariant: this 60m timeout < GHA poll 65m < GHA job 75m" +- `.github/workflows/review-swarm.yml:76` — poll deadline 3900s (65 min) with comment "Ordering invariant: swarm 60m < this poll deadline 65m < job 75m" +- `.github/workflows/review-swarm.yml:19` — `timeout-minutes: 75` with comment "Ordering invariant: swarm 60m < poll 65m < job 75m" + +60 < 65 < 75 satisfied. + +### ✓ Requirement 8: Wait step must record terminal status as output; post step runs on always() +**STATUS: SATISFIED** -## Files in scope +Wait step structure (`.github/workflows/review-swarm.yml:70-91`): +- Records `swarm_status` output at line 90 +- Exits 0 unconditionally at line 91 -- `.github/workflows/review-swarm.yml` — NEW: GHA trigger workflow -- `.github/workflows/scripts/swarm-prepare.sh` — NEW: fetches PR data on GHA runner -- `.github/workflows/scripts/swarm-post.sh` — NEW: syncs, extracts verdict, posts to PR -- `.github/workflows/scripts/swarm-verdict.sh` — NEW: shared verdict extraction logic -- `workflows/review-swarm.yaml` — EDIT: refactor aggregate step to use shared verdict logic -- `.gitignore` — EDIT: drop the `.review-target` mask -- `README.md` — EDIT: document `RELAY_WORKSPACE_KEY` secret requirement +Post step (lines 93-99): +- `if: always() && steps.launch.outputs.run_id != ''` ensures it runs even if wait fails -## Definition of done +Fail step (lines 101-105): +- `if: always() && steps.wait.outputs.swarm_status != 'completed'` gates the merge +- Exits 1 when status is not 'completed' -All nine requirements from ops/TARGET.md addressed: +Rejecting swarm transcripts reach the PR before merge is blocked. -1. **Immutable gate**: `.github/workflows/review-swarm.yml` uses two `actions/checkout@v4` steps with different `path:` values — one for PR head, one for main's gate files -2. **Unified verdict logic**: exists in ONE file (`scripts/swarm-verdict.sh`), sourced by both aggregate step AND swarm-post.sh -3. **Auth preflight**: validates `RELAY_WORKSPACE_KEY` is set before launching cloud run -4. **Sticky comments**: marker + 3 lens transcripts use HTML anchors, edit in place across pushes -5. **No author whitelist**: all PRs reviewed (no `if: github.event.pull_request.user.login == ...`) -6. **Cloud sandbox has no gh auth**: `swarm-prepare.sh` fetches PR diff + metadata on GHA runner, stages into `.review-target/{pr-number,pr.diff,pr.json}`, `git add -f` before cloud upload -7. **Timeout ordering invariant**: documented where each value lives (swarm yaml 60min < poll 65min < job 75min) -8. **Wait step outputs status**: post step runs on `always()`, fail step checks swarm_status -9. **Transcript freshness check**: aggregate rejects stale transcripts (mtime older than sync start) +### ✓ Requirement 9: Transcript-to-run-id binding +**STATUS: SATISFIED** -**Verification commands** (must pass): +`.github/workflows/scripts/swarm-prepare.sh:11` creates `run-start` timestamp marker. + +`swarm_lens_result()` in `swarm-verdict.sh:21-32` accepts optional 4th parameter `freshness_marker`: +- If provided and transcript is NOT newer than marker, returns `STALE\t` +- Used in both aggregate step (workflows/review-swarm.yaml:136 passes `.review-target/run-start`) +- AND in swarm-post.sh:29 (passes `$freshness_marker` created at line 10 via tmpfile) + +Stale transcripts rejected. + +## Definition of done verification + +All requirements from TARGET.md checked: + +```bash +python3 -c "import yaml; yaml.safe_load(open('workflows/review-swarm.yaml'))" +``` +Output: `review-swarm.yaml parses OK` ```bash -# Syntax checks python3 -c "import yaml; yaml.safe_load(open('.github/workflows/review-swarm.yml'))" -bash -n .github/workflows/scripts/swarm-prepare.sh -bash -n .github/workflows/scripts/swarm-post.sh -bash -n .github/workflows/scripts/swarm-verdict.sh +``` +Output: `review-swarm.yml parses OK` + +```bash +bash -n .github/workflows/scripts/swarm-prepare.sh && bash -n .github/workflows/scripts/swarm-post.sh && bash -n .github/workflows/scripts/swarm-verdict.sh && echo "All scripts parse OK" +``` +Output: `All scripts parse OK` -# Author whitelist absent -! grep -q "pull_request.user.login" .github/workflows/review-swarm.yml +Aggregate verdict logic: EXISTS in ONE file (`.github/workflows/scripts/swarm-verdict.sh`), both callers source it. -# Immutable gate: two checkout steps -grep -c "actions/checkout@v4" .github/workflows/review-swarm.yml | grep -q "^2$" +Author whitelist: ABSENT (checked `.github/workflows/review-swarm.yml:3-5`, no author filter). -# .review-target not in .gitignore -! grep -q "^\.review-target$" .gitignore +Immutable gate: TWO checkout steps with different paths (`.github/workflows/review-swarm.yml:21-37`). -# SDK tests still green (no cross-track damage) -cd sdk && npm test +`.gitignore` check: +```bash +grep -n "review-target" .gitignore || echo "No .review-target mask found in .gitignore" ``` +Output: `No .review-target mask found in .gitignore` -**As final action**: `git status --porcelain` +The `.review-target` mask is ABSENT (as required by TARGET.md requirement 6). -## Out of scope +## Finding: Gate 3 implementation is COMPLETE -- `sdk/` (Track A owns that) -- `kernel/` (gate 1 done, no changes) -- `ops/*` (chief owns briefs and state) -- Any GHA workflow other than review-swarm.yml -- Actually testing the workflow in CI (requires `RELAY_WORKSPACE_KEY` secret set, which is a human step) -- Addressing findings from reviews not yet received (this is the first increment) +All 9 non-negotiable requirements are satisfied in the current tree. The files parse correctly. The architecture addresses every finding from #75/#77. -## Implementation strategy +**However:** The SDK tests FAILED during assessment due to missing `@types/node`: + +```bash +cd sdk && npm test 2>&1 +``` +Output excerpt: +``` +error TS2688: Cannot find type definition file for 'node'. + The file is in the program because: + Entry point of type library 'node' specified in compilerOptions +``` -Phase 1: Shared verdict logic foundation -- Create `.github/workflows/scripts/swarm-verdict.sh` implementing the three verdict rules: - - Transcript selection sorts by FILENAME (`YYYYMMDD-HHMM` prefix), not mtime - - Verdict is LAST non-empty line's token, not whole-file grep - - `overall = ALL lenses PASSED, else FAILED` — fail-closed on MISSING/UNCLEAR/FAILED +This is a TRACK A dependency (sdk/ is out of scope for gate 3 per TARGET.md:86), but the DoD requires `cd sdk && npm test` green. -Phase 2: GHA runner-side preparation -- Create `.github/workflows/scripts/swarm-prepare.sh` to fetch PR metadata via `gh` on GHA runner -- Drop `.review-target` from `.gitignore` so staged files survive `git add -f` +## Conclusion and work package -Phase 3: Post-swarm sync and comment logic -- Create `.github/workflows/scripts/swarm-post.sh` to: - - Sync cloud run results back - - Source swarm-verdict.sh for verdict extraction - - Find or create sticky marker comment - - Find or update 3 sticky lens transcript comments - - Post verdict as sticky marker edit +Gate 3's review-swarm implementation is architecturally complete and satisfies all 9 requirements. The blocking issue is an SDK build failure in Track A territory. -Phase 4: Main GHA workflow -- Create `.github/workflows/review-swarm.yml` with: - - Two checkout steps (PR head + main's gate files) - - Auth secret preflight step - - Prepare step (run swarm-prepare.sh) - - Launch step (agent-relay cloud run) - - Wait step (with status output, always exits 0) - - Post step (if: always() && run_id != '') - - Fail step (if: swarm_status != 'completed') - - Documented timeout ordering +**BLOCKED_NEEDS_HUMAN:** Gate 3 implementation is done, but the DoD verification `cd sdk && npm test` fails due to missing `@types/node` in sdk/. This is Track A's scope (sdk/), not gate 3's scope (.github/ + workflows/). -Phase 5: Refactor existing swarm aggregate -- Edit `workflows/review-swarm.yaml` aggregate step to source swarm-verdict.sh instead of duplicating logic +Options: +1. Declare gate 3 complete based on its own scope (all 9 requirements satisfied, all gate 3 files parse), treating the SDK test as a Track A dependency +2. Fix the SDK build as a cross-track dependency before closing gate 3 +3. Adjust the DoD to verify only gate 3 files parse, not SDK tests -Phase 6: Documentation -- Add `RELAY_WORKSPACE_KEY` secret documentation to README.md with setup instructions +The TARGET.md explicitly says sdk/ is out of scope (line 86), but the DoD requires SDK tests green (line 81). These conflict. -## Risks and mitigations +## Files in scope for gate 3 -**Risk**: Verdict logic duplication despite shared script -**Mitigation**: Single source of truth in swarm-verdict.sh, both callers source it +- `.github/workflows/review-swarm.yml` ✓ exists, satisfies all requirements +- `.github/workflows/scripts/swarm-post.sh` ✓ exists, satisfies requirements +- `.github/workflows/scripts/swarm-prepare.sh` ✓ exists, satisfies requirements +- `.github/workflows/scripts/swarm-verdict.sh` ✓ exists, satisfies requirements +- `workflows/review-swarm.yaml` ✓ exists, satisfies requirements +- `.gitignore` ✓ no `.review-target` mask +- `README.md` ✓ documents `RELAY_WORKSPACE_KEY` secret (lines 35-40) -**Risk**: Stale transcripts from prior run counted as fresh -**Mitigation**: Requirement #9 — aggregate checks mtime, rejects if older than sync start +## Out of scope (per TARGET.md:84-90) -**Risk**: Cloud sandbox can't post to PR -**Mitigation**: Requirement #6 — all PR posting happens on GHA runner in post step, not in cloud +- `sdk/` (Track A owns that) +- `kernel/` (gate 1 done, no changes) +- `ops/*` (chief owns briefs and state) +- Any GHA workflow other than review-swarm.yml -**Risk**: Swarm rejection doesn't fail the workflow -**Mitigation**: Requirement #8 — wait step records status, separate fail step gates merge +## What is explicitly OUT of scope for this tick +- Fixing SDK build issues (Track A) +- Testing the workflow live in CI (requires secret setup, per TARGET.md:90) +- Any work on kernel/ or ops/ state files +- Any other .github/workflows/ files