diff --git a/.github/workflows/loop.yml b/.github/workflows/loop.yml index eee4beb..0710ebd 100644 --- a/.github/workflows/loop.yml +++ b/.github/workflows/loop.yml @@ -42,6 +42,9 @@ on: description: 'Infrastructure only: exercise checkout/install/R2 pull+push, skip the agent' required: false default: 'false' + model: + description: 'Override the engine model id (default deepseek/deepseek-v4-flash). A bad value must fail the D19 guard.' + required: false concurrency: group: loop @@ -91,7 +94,7 @@ jobs: # For non-dispatch events github.event.inputs is null → default to report (report-only) LOOP_EXECUTE_WRITES: ${{ github.event.inputs.execute_writes || 'false' }} LOOP_SMOKE: ${{ github.event.inputs.smoke || 'false' }} - LOOP_MODEL: deepseek/deepseek-v4-flash + LOOP_MODEL: ${{ github.event.inputs.model || 'deepseek/deepseek-v4-flash' }} LOOP_RUN_TIMEOUT_MS: '3300000' # 55min, leaving headroom for push (job timeout 60min) GH_TOKEN: ${{ secrets.LOOP_GH_TOKEN || github.token }} # Credentials: secrets only @@ -166,6 +169,7 @@ jobs: echo "model $want is available" - name: Pull state from R2 + id: pull run: node scripts/loop/r2-sync.mjs pull - name: Run stage @@ -174,9 +178,18 @@ jobs: run: node scripts/loop/entry.mjs # Push the state layer back even on crash/timeout (the task stays in - # processing and is reclaimed by the next sweep via TTL) + # processing and is reclaimed by the next sweep via TTL). + # + # Guarded on `pull` having *run*: `state/` is gitignored, so it exists only + # after a pull. If an earlier step failed, pull is skipped, the directory + # never appears, and an unguarded push dies on + # `aws: [ERROR]: The user-provided path ... does not exist` — a second red + # step that buries the real cause and reads like R2 itself broke (D34). + # `!= 'skipped'` rather than `== 'success'` on purpose: a *failed* pull may + # still have left a usable tree, and the point of `always()` is to record + # the processing state after a mid-run crash. - name: Push state to R2 - if: always() + if: always() && steps.pull.outcome != 'skipped' run: node scripts/loop/r2-sync.mjs push - name: Upload session transcripts diff --git a/scripts/loop/README.md b/scripts/loop/README.md index c1c8038..5278dc2 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -92,7 +92,7 @@ GitHub event ─▶ loop.yml (concurrency group `loop` = platform-level single w | `pull_request_target` closed (other PR) | task → `accepted` / `rejected` only; **no metric** (not loop-produced) | | `release` published | metrics: released | | `schedule` daily / weekly | system run: sweep / retro-scheduled | -| `workflow_dispatch` | run per inputs (`task`/`stage`); `execute_writes=true` = L2 rehearsal | +| `workflow_dispatch` | run per inputs (`task`/`stage`); `model` overrides the engine model; `execute_writes=true` = L2 rehearsal | Anything else is a no-op that still exits 0 — event storms cost nothing. @@ -307,10 +307,25 @@ was *correct*, and whether new events produce proposals that match reality. as *skipped* (not failed), while a same-repo PR and a loop PR still run. Verify by pushing to a same-repo branch, and by observing the next Dependabot PR arrive. -- [ ] Serial lock: two consecutive dispatches queue, never run concurrently - (run timestamps prove it) +- [x] Serial lock: two consecutive dispatches queue, never run concurrently — + verified 2026-09-11 with two `workflow_dispatch` runs 25s apart + (`34607797259`, `34607838545`). The evidence is at **job** level, not + workflow: run 2's workflow `created_at` is 14:03:03 but its job + `created_at` is 14:03:07 — exactly run 1's job `completed_at` — and its + "Set up job" logs at 14:03:10. Workflow-level `created == started` on both, + so read alone it would have looked like concurrency; the job timestamps + show the 4s queue. - [ ] Crash path: cancel a job mid-run → task stays `processing` → next sweep - reclaims it by TTL + reclaims it by TTL. Still unsampled — and the model-guard runs show why the + cheap version of this test cannot work: the failure lands in `Install + engine (pi)`, *before* `Run stage`, so no task is ever claimed (`Pull state` + and `Run stage` both skipped). It needs a job actually cancelled after + `entry.mjs` has claimed a task. +- [ ] Failure classification (`retry` vs inbox) — **still unexercised even though + a failing run now exists.** The D19 guard failure is a *workflow step* + failure: `entry.mjs` never ran, so no run row was written and the + classification table was never consulted. Only an in-agent failure (engine + crash, non-zero exit after a claim) exercises it. - [ ] Metrics — **unreachable under A1; accepted unverified (2026-09-11)**: a human merge writes one `acceptance` row, no duplicates; `released` backfills to the intended task. A1 produces no loop PR, so the `metrics` @@ -329,10 +344,13 @@ was *correct*, and whether new events produce proposals that match reality. (outcome, exit code, token usage, model, proposed actions) without downloading anything - [ ] Engine stability: N consecutive headless pi runs without hanging — all 18 - recorded runs reached an end row with no hang (A0 + A1 combined). Note the - inverse gap: **every run so far exited 0**, so the failure-classification - table (machine/config failure → `retry`; agent-judged → inbox) has never - been exercised by a real failure. + **recorded** runs reached an end row with no hang (A0 + A1 combined). Note + the inverse gap: every recorded run exited 0, so the classification table + (machine/config failure → `retry`; agent-judged → inbox) has never been + consulted. The three guard failures do not change this: they die at + `Install engine (pi)`, before `entry.mjs`, so they write no run row at all + — see the failure-classification item above. (Distinction worth keeping: + 21 workflow executions, 18 run records.) - [ ] No drift **and** no incorrect proposal after a week → human decides on L2 ### Not reachable under A1 (structural — decide before the L2 switchover) @@ -572,6 +590,24 @@ that has never met production. the 09-11 daily sweep (`r-20260911-080231-zja0`), which only found it because the #40 triage report had mentioned it in passing. That is the accidental path D30/D33 exist to remove. +- [x] D34 (A1) `Push state to R2` failed whenever `Pull state from R2` was + skipped. The push is `if: always()` so that a run which crashes or times + out still records its `processing` state for the next sweep — but + `state/` is gitignored and materialises only from a pull, so when an + earlier step fails there is no directory to upload and the step dies on + `aws: [ERROR]: The user-provided path ... does not exist` (exit 255, + observed on `34607797259` and `34607838545`, both after the D19 model + guard rejected a deliberately bad model). The job was already failing, so + the verdict was unchanged — but a second red step buries the first cause + and reads like R2 itself broke, which is exactly the kind of noise a + report-only phase must not generate while a human is reading logs daily. + Now the pull has an `id` and the push is conditioned on + `always() && steps.pull.outcome != 'skipped'`. Deliberately not + `== 'success'`: a *failed* pull may still leave a usable tree, and + recording post-crash state is the whole point of `always()`. + Verified on `34608064576` (same bad model after the fix): step 9 is now + `skipped` while step 6 stays `failure`, so the only red step is the one + that actually failed. ## Environment facts