Repository navigation
chore(loop): expose the engine model to workflow_dispatch, and fix the push-after-skipped-pull failure (D34) - #44
Merged
Merged
Conversation
Contributor
Coverage
|
The D19 guard exists because pi silently falls back to its default model for an unknown `--model`, so a typo would run every run with the wrong model. The guard was never exercised by a real bad value, and there was no way to produce one: `LOOP_MODEL` was a hardcoded job-level env value with no dispatch input. Add an optional `model` input, defaulting to the same id as before, so the guard can be tested end to end and a model can be switched for debugging without editing the workflow. Validation is unchanged and still fails loudly. Verified by dispatching with a deliberately unknown model; see the PR.
D34, found by the D19 guard test (two dispatches with a deliberately unknown model). Both runs failed correctly at `Install engine (pi)`, but then `Push state to R2` failed too: aws: [ERROR]: The user-provided path /home/runner/work/tiny-oss/tiny-oss/state does not exist. `state/` is gitignored and only ever appears from a pull, so when an earlier step fails there is nothing to upload. The push is `if: always()` so a run that crashes or times out still records its processing state — that intent is right, but it needs the pull to have happened. The job was already failing, so the verdict did not change. It still matters: a second red step buries the first cause and reads like R2 itself broke, and this is the phase where a human reads these logs daily. Condition on `steps.pull.outcome != 'skipped'` rather than `== 'success'`: a failed pull may still leave a usable tree, and post-crash state is precisely what `always()` is for. Also records what the guard test did and did not establish: - serial lock: verified — run 2's *job* was created exactly when run 1's job completed, a 4s queue that workflow-level timestamps hide - crash path and failure classification: still unexercised, and now provably not testable this way — the failure precedes `entry.mjs`, so nothing is ever claimed and no run row is written
Also reconciles the A1 checklist with the guard test's findings, and keeps 'recorded runs' distinct from 'workflow executions': the three guard failures die at `Install engine (pi)`, before `entry.mjs`, so they write no run row — 21 executions, 18 run records. Conflating the two would have made the engine stability count wrong.
Alex1990
force-pushed
the
chore/loop-model-input
branch
from
September 11, 2026 14:22
1191b1a to
ed735a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Executes the A1 sample-gathering test I proposed, and it paid for itself: the guard works, the serial lock is proven, and the test exposed a real defect (D34).
1. The guard holds — D19 verified for the first time
workflow_dispatchhad no model input (LOOP_MODELwas a hardcoded job env), so a bad model could not be produced at all. Added an optionalmodelinput, defaulting to the same id as before; validation is unchanged.Dispatching with
nonexistent/model-xyz:Fails loudly, names the want/actual, prints the catalog. Zero LLM tokens — the guard trips before the agent. This closes the "the guard was never tested with a real bad value" gap.
2. Serial lock — proven, with a caveat about how you read the timestamps
Two dispatches 25s apart:
createdAtcreated_atcompleted_atRun 2 entered the queue at 14:03:03, but its job was not created until 14:03:07 — exactly run 1's job
completed_at— and "Set up job" logs at 14:03:10. A 4s queue.The trap: at workflow level
created == startedfor both runs, so reading those alone would look like they ran concurrently. Only the job-level timestamps show the serialization. Worth recording, because "R2 has no concurrent writes" rests on this.3. D34 — a second red step that buried the first cause
Every one of the failing runs showed:
state/is gitignored and only materialises from a pull. The push isif: always()— correct intent (a crashed run must still record itsprocessingstate for the next sweep) — but withPull state from R2skipped there is nothing to upload.The verdict did not change (the job was already failing), but this is the phase where a human reads these logs daily: two red steps read like R2 also broke, and the second failure is pure noise on top of the real one.
Fix: the pull gets an
id, the push is conditioned on!= 'skipped'rather than== 'success'deliberately — a failed pull may still leave a usable tree, and recording post-crash state is precisely whatalways()is for.Verified on the same bad model after the fix (
34608064576): step 9 is nowskipped, step 6 staysfailure. The only red step is the one that genuinely failed.4. What this test did not establish
Recording these because I proposed this test partly on the expectation it would cover them, and it does not:
processing→ sweep reclaims by TTL)Install engine (pi), beforeRun stage, so no task is ever claimed — steps 7 and 8 were bothskipped. Needs a job cancelled afterentry.mjsclaims.retryvs inbox)entry.mjsnever ran, so no run row exists and the classification table was never consulted.Both are now written into the A1 checklist with the reason they cannot be tested this way, so the next person does not repeat the attempt.
Verification
actionlintclean on both workflows.both PRs had extended the same list. Resolved by keeping both intents
rather than picking a side: chore(loop): expose the engine model to workflow_dispatch, and fix the push-after-skipped-pull failure (D34) #44's Crash-path and new Failure-classification
items, plus chore(loop): record what A1 structurally cannot validate #43's fuller Metrics entry.
records — they die at
Install engine (pi), beforeentry.mjs, so no run rowis ever written. That is 21 workflow executions but still 18 run records, and
the engine-stability count now says exactly that.
mergeable=CLEAN.