Skip to content

chore(loop): fix two gaps the loop's first real run exposed - #51

Merged
Alex1990 merged 2 commits into
mainfrom
chore/loop-report-actions
Sep 15, 2026
Merged

Alex1990 merged 2 commits into
mainfrom
chore/loop-report-actions

Conversation

@Alex1990

Copy link
Copy Markdown
Owner

Two small fixes, both found by watching the loop do real work for the first time end to end: issue #48 → triage → bugfix → branch → PR #50. Neither is speculative.

1. The run report hid the two writes that mattered

That bugfix run logged:

[loop] git: pushed loop/48-azure-oracle-ci
[loop] gh: opened PR #50 from loop/48-azure-oracle-ci
[loop] gh: #48 +label ready-for-human, -label ready-for-agent
[loop] gh: #48 commented
[loop] GitHub actions executed: 2

Four actions, reported as two. applyActions runs the push and the PR creation in finishRun's pre-flight (so a failure can downgrade the outcome before it is recorded), and only the label and comment were left in the actions list the log line and Step Summary count from. The summary of a run that opened a PR read as if it had only tagged an issue — exactly backwards for anyone auditing what the loop touched.

finishRun now keeps what the pre-flight executed and prepends it, so actions and applied describe the whole run in the order it happened: push → pr-create → label → comment.

The report-boundary path is unaffected: validatePush refuses pr-opened there, so the pre-flight never runs and nothing is prepended.

2. The docs told you to self-review

A loop PR creates two approval-gated runs, and I had written that a loop PR's checks are simply "waiting for the click". PR #50 showed the useful detail: approval is per run. Approving only CI ran it to green while leaving Loop untouched — the loop verifies its own change without reviewing its own work.

[loop:gh] ... CI in_progress          ← approved
[loop:gh] ... Loop action_required    ← left alone, so pr-review never fires

The UI's Approve workflows to run approves both; the API does them one at a time. scripts/loop/README.md now names the API path and says which to pick when.

Verification

  • A focused harness drives finishRun through the real pr-opened path against a local bare repo (git's url.insteadOf standing in for GitHub, a stubbed gh through the exec seam): asserts all four actions are reported in order, that applied matches, that the branch really lands, and that the task still reaches waiting-merge. Green.
  • The doc correction is not theory — it is what was actually done to PR chore(ci): run the Azure signer oracle in CI #50, whose CI then passed with the oracle in the log (All azure oracle checks passed. / Test Files 22 passed).
  • pnpm lint, pnpm fmt:check, node --check clean.

Note for the reviewer

This PR was written by me (the session), not by the loop, so it has no loop-task: marker and is not a loop PR — pr-review does not apply. Its own CI runs without approval.

… two

The first real `pr-opened` run (task #48, PR #50) logged `GitHub actions executed: 2`
while having executed four: `applyActions` ran the push and the PR creation in
`finishRun`'s pre-flight, and only the label and comment were left in `actions` —
the list the log line and the Step Summary count from. The two writes that matter
most were the two the report did not mention, so the summary of a run that opened a
PR read as if it had only tagged an issue.

`finishRun` now keeps what the pre-flight executed and prepends it to the plan for
the rest, so `actions` and `applied` describe the whole run in the order it happened
(push → pr-create → label → comment).

The report-boundary path is unaffected: `validatePush` refuses `pr-opened` there, so
the pre-flight never runs and nothing is prepended.
…fault

The first real loop PR (#50) showed the approval-required state is per *run*, not per
PR: `CI` and `Loop` both sat at `completed/action_required`, and approving just the
first ran CI to green while leaving `Loop` untouched — that is, the loop verified its
own change without reviewing its own work. The UI button approves both, so the text
now names the API path (`POST /actions/runs/<id>/approve`) and says which to pick when.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

Metric % Covered / Total
Lines 90.19% 1003/1112
Statements 89.06% 1083/1216
Functions 91.03% 193/212
Branches 76.43% 519/679

@github-actions github-actions Bot added the needs-triage Maintainer needs to evaluate (loop can't decide; too risky; external PR) label Sep 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Loop triage — needs-triage (human review + merge).

PR #51 edits the loop host's own machinery (scripts/loop/shared/state.mjs, scripts/loop/README.md) on a same-repo branch chore/loop-report-actions with no loop-task: marker. loopTaskOf() returns null, so the router sends owner repo PRs to triage: it is not a loop PR, and the loop must not be maker or checker for the change it runs on. The change is already written and green; what remains is a maintainer review/merge decision. Unlike #49 it does not touch .github/workflows/**, so the job token could push it — but there is nothing left for the loop to do, and self-review of its own host is the line that matters.

Both claims check out read-only:

  • Report fix. finishRun now keeps the pre-flight applyActions result (executed) and prepends it to the label/comment plan, so actions/applied are push → pr-create → label → comment (4, not 2). The report-boundary path is unaffected: the pre-flight only runs under writeLevel === 'auto' && outcome === 'pr-opened' && push, and validatePush refuses pr-opened there, so nothing is prepended.
  • Approval docs. PR chore(ci): run the Azure signer oracle in CI #50's head loop/48-azure-oracle-ci has exactly two pull_request runs: CI completed/success and Loop completed/action_required — so approving only CI left the self-review (Loop) unrun, exactly the per-run detail now documented. The ?status=action_required query returns the Loop run, and POST .../actions/runs/<id>/approve is the correct per-run endpoint.

Verified: base sha 31aa850 == current main head (not stacked); 2 files, +44/−9, 2 commits; node --check scripts/loop/shared/state.mjs clean; Lint pass, Test pass; mergeable: MERGEABLE, mergeStateStatus: BLOCKED (review required), reviewDecision: REVIEW_REQUIRED.

Points for the reviewer.

  1. The "focused harness" is not committed: the diff adds no test file and a tree-wide search finds no loop-script tests, so the asserted behaviour (4 actions in order, applied matches, branch lands, task reaches waiting-merge) is not reproducible from the tree. If it is meant as the regression guard, commit it.
  2. On the failure path actions still omits the failed pre-flight action: applyActions returns only successfully applied actions, so a failed push/pr-create shows up only as a warning while the Step Summary header still reads "GitHub actions executed: N". Minor, not a regression, but the PR's "describe the whole run" holds only on the success path.
  3. Commit 2 is typed docs(loop):, but AGENTS.md reserves docs for user-facing docs (README.md, UPGRADING.md); a change to scripts/loop/README.md is loop maintenance and should be chore(loop): (the type is rendered into the user-facing CHANGELOG.md). The PR title is already chore(loop):.

Proposed action: label needs-triage; a human reviews and merges. The loop must not merge it.

@Alex1990
Alex1990 merged commit 9929d03 into main Sep 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-triage Maintainer needs to evaluate (loop can't decide; too risky; external PR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant