Skip to content

drive: cloud run 579149fc #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kjgbot wants to merge 1 commit into from
Closed

drive: cloud run 579149fc #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions ops/NEEDS_HUMAN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
265 changes: 176 additions & 89 deletions ops/NEXT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 `<!-- review-swarm -->` anchor (line 47)
- Each lens transcript uses `<!-- swarm-lens: <lens> -->` 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<transcript>`
- 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
Loading