Repository navigation
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
DRU-1044: overview, decisions, tests, and open questionsAll results are from OverviewThe problem. A build could hand a person code that nobody had tested:
So a PR could look ready to merge while its CI was failing. The solution. The build now checks the work before anyone is asked to look at it:
The approach. The build learns about CI the way it learns about the review. GitHub's What changed, and why
Two things to know:
TestsAutomated, on
Live, and why. The change sits between GitHub and a durable run: webhooks, a wait that has to survive restarts, and CI that finishes on its own schedule. No unit test spans that, so each case ran a real build on a local install against The test repository's CI has these jobs:
Its
DSE-66 stopped with Not run live, and why:
Pending Questions
|
czpython
left a comment
There was a problem hiding this comment.
Requesting changes on the direction, not on the work. This follows DRU-1044 closely, and the live runs are thorough. The changes come from rethinking the ticket.
The new shape: the implementer pushes and its call ends. The build waits on ci.updated with the VM released, wakes on a webhook or the 60-second timer, and reads only whether CI on the head has finished and whether it is green. When CI is red, the implementer reads the failures itself.
implement lint, typecheck, tests for its diff, commit, push
|
v
head changed? -- no -----------------------> review it pushed nothing
| yes
v
read CI <-----------------+
| |
+-- running --> wait -+ a webhook or the 60 s timer wakes it;
| the VM is released, and no agent runs
+-- green -----------------------------> review
+-- nothing registered after 3 min ----> review
+-- still running after 1 h -----------> review the park label says so
+-- red -+-- revisions left -----------> implement "CI on <sha> is red"
+-- none left ----------------> review
review
|
+-- request changes, revisions left --> implement
+-- approve, or none left ------------> park: the PR leaves draft only if
CI is green and the review after it approved
-
The implementer reads CI; the build only waits. The build reads two facts from GitHub: whether CI on the head has finished, so it knows when to wake the implementer, and whether it is green, for the draft rule. Remove
Check,CheckFailure, the per-check rules inCiReport.judge,get_job_log,list_check_runs,CI_LOG_LINES, and the default-branch read. On red, the revision request is one line: "CI on<sha>is red. Read it withgh pr checksandgh run view --log-failed, fix what your diff caused, and report the rest inchecks."ghin the box acts as the operator App, which can read Actions logs. The box's GitHub MCP server loads only its default toolsets, which leave outactions. -
Read "finished" from check suites, not check runs. Actions creates the check run of a
needs:job only when the jobs it needs finish. On astral-sh/uv run 42286852249, 67 jobs appeared in 9 waves over about 8 minutes. The webhook that wakes the read is an upstream job's completion, so "every check run finished" can be true between waves. Actions opens one check suite per workflow run, and the suite stays open until the run completes.- Finished: every suite with check runs is
completed, and no commit status ispending. - A suite with no check runs counts as running for 3 minutes after the push, then is ignored. Some apps leave a
queuedsuite with no runs for good. Theclaudeapp has one on this PR's head. - Green: every suite is
success,neutral, orskipped, and every status issuccess. - githubkit has
checks.async_list_suites_for_ref.
- Finished: every suite with check runs is
-
Drop the red-on-main rule. It matches check names, and on this repo it fails both ways. The test workflows run only on
pull_request, so main never has their checks. On main,backendis the release-image build, not the tests. Instead, the implementer decides. When a red check is not from its diff, it pushes nothing and reports the check incheckswith the reason. In_implement_phase, a delivery whosehead_shaequals the previous delivery's skips the CI wait and goes to review. The PR stays a draft, and the park label says CI is red. The implement prompt must allow that round with no push. -
Release the VM during the CI wait. Add
release_sandbox: bool = FalsetoWorkflow.wait, and passTruefrom the CI wait. It calls_reap_run(), as a gate park does.releaserevokes the box identity before it deletes the VM, so the next implement call boots a new VM, andRepoWorkspacere-clones before every call anyway. Update thewaitsection ofdocs/writing-an-app.md. -
Drop
pre_push_tests. A fixed list is the same for every diff, so in a large repo it is either too slow or it tests the wrong area. Keep the prompt change: before every push, run lint, typecheck, and the tests narrowed to the files you changed. Remove the key fromdocs/configuration.md. -
Narrow
_approved_work(your question 1). Clear the draft before a merge only on a person's approval. Leaveimplementation_approval: noneto DRU-614. As it is now, withnoneandon_approval: merge,merge_when_readysquash-merges a capped red round on a repo without required checks.
Keep as they are: the check_run and status webhooks, ci.updated and its subscriber, the 60-second timer, _now and _read_ci as steps, the 3-minute and 1-hour limits, the revision cap, is_ready_for_review, and the park label.
Tests: drop the Check.get_log and red-on-default tests. Add one for the suite read, with a needs: wave and a suite with no runs, and one for the unchanged head.
fb6640c to
c3b2903
Compare
The new direction: how each point was done, and the live tests
Each point of the review
Automated checks
Live testsEach test ran a real build on a local install of this commit against The test repository's CI has these parts:
The repository also has three apps (Netlify, Supabase, and the operator App) that leave a queued suite with no check runs on every commit.
Not run live: the review agent requesting changes after green CI. Every live review approved, and a unit test covers that loop. Test 1 (PR #94) ran while the install's public tunnel was down. GitHub's |
czpython
left a comment
There was a problem hiding this comment.
The rework does everything the last review asked, and the live runs cover the cases that worried me: the needs: wave, a status held pending, the unchanged head, and the merge after a person's approval. One behavior change is left, and four small fixes. I updated the PR description to this version myself.
-
A
commentreview also takes the PR out of draft. The review returnsapprove,comment, orrequest_changes, andcommentmeans findings with none blocking._implement_phasealready letscommentgo on to the park, butis_ready_for_reviewasks forapprove. So a build with green CI and a review that only leaves notes parks with a draft PR, which every park onmaincleared. Use the loop's rule injournal.py: the last review after green CI is notrequest_changes. The ticket said "approving review", so this one is on the ticket. Update the property's docstring, its test, and the draft sentence in the PR description. -
Rename the
runningverdict tounfinished.CiReport.judgereturnsNonewhile CI runs, andrunningonce the build stops waiting for it, so the word has two meanings. TheCI_TTL_SECONDScomment already says "unfinished", and the label says "did not finish". -
Name the
CiReportvariableci_report. It ischecksin_implement_phase,_work_gate, and both journal properties, andreportin_wait_for_ci.checksis alsoImplementationOutput.checks, the field the new prompt and the revision request tell the implementer to fill. -
Docs.
docs/writing-an-app.md: drop "such as one on the checks of a commit". Core docs give no app's use case: "A wait that can last long can passrelease_sandbox=True."docs/configuration.md: there is no new key now, so two sentences are enough: "After each push, the build waits for CI on the head before the review, and red CI goes back to the implementer. The pull request leaves draft when the review of a head with green CI requests no changes, or when a person approves it."
-
Test helpers in
test_build_plan_phase.py._implementing_buildand_parked_buildreturn a tuple of the flow and a list, but their annotations say-> listand-> list[bool]._parked_buildhas one caller, so inline it.
One thing to know, with no change asked: because the operator App leaves a suite with no check runs on every push, each CI wait lasts at least three minutes, as your live test 1 showed. That rule came from my review. It stays for now, and the PR description says so.
c3b2903 to
89dbb81
Compare
The second review: each point and the tests
Each point
Automated checks
Live tests on this commitEach test ran a real build on a local install of
The live tests from my last comment still cover everything else. This change only renames those paths. A question: a network blip fails the buildOne live run of case 1 failed before its review, with no change made. I dropped and reconnected my Wi-Fi during the test. A The cause was my network. But the CI wait reads GitHub about once a minute for up to an hour, so it is more exposed to a brief network error than a single step is.
|
A build reviewed and offered a pull request whose CI had not finished or was red, and
ReviewWork.on_waittook the PR out of draft whatever the round's outcome. Now the build waits for CI on each pushed head before the review, and a PR that is not ready stays a draft.checks.ci.updated. GitHub'scheck_runandstatuswebhooks publish it, and the wait also ends every minute. Each wake reads the check suites and the commit statuses on the head.Workflow.waittakesrelease_sandbox. The CI wait passes it, so no sandbox runs while the build waits, and the next agent call boots a new one.needs:job only when the jobs it needs finish, but the suite stays open until the workflow run completes.success,neutral, orskippedon every suite, andsuccesson every status.max_implementation_revisions. The implementer reads the failures itself withghand fixes what its diff caused. When no red check comes from its diff, it pushes nothing and reports each one incheckswith the reason. A delivery that leaves the head unchanged skips the CI wait and goes to the review.on_approval: mergeclears the draft before the merge, because GitHub merges no draft.The CI wait adds steps after
implement, so a build that is past its first delivery when this deploys fails on replay and needs a new start.