chore(loop): expose the engine model to workflow_dispatch, and fix the push-after-skipped-pull failure (D34)

.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

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