awml
Sign in

chrismdp / fix-review-findings

A failed review is fixed

Public ·
Version 3 of 3
  1. #3 · chrismdp · · 0.8ShowingLatest
  2. #2 · chrismdp · · 0.6
  3. #1 · chrismdp · · 0.6
Fork
Who may see each partWhere each part runsYAMLChecksNo mistakes

GitHub Actions, fix.yml

GitHub: pull requests, checks and commentsActions job: decide, reading only GitHub, with a token that only reads, in no concurrency groupActions job: fix, with a token that only reads, one at a time on the pull request decidedActions job: push or comment, running nothing from the change, one at a time on the pull request decidedClaude Code, claude-opus-5-5, on the maintainer's subscriptionCI's run and its committhe pull request and its headthe branch's commits[open, not moved, fewer thantwo fix commits][two fix commits]the findingsthe patchthe fix[none of those files, notmoved][within limits]each fix commit, as pushed[touches .github/ or Claude'sfiles, not moved][gave up]TriggerCI finishes on a pull requestfrom a branch of thisrepository with the testspassed and Bug review orSecurity review failedDecided in #160's follow-up: the tests own a run where both failed, so retry-a-build acts then, not this. A fix commit that breaks a test wakes retry, which redrafts and uses a try; the tries cap stops it Loosely coupled: it reads only the checks on the head commit and the branch. Builds and landing don't call it or wait for it, so a person's pull request is fixed the same way as a build's2Linked to chrismdp/issue-to-pull-request#tokenConnectionThe repository, through itsown Actions tokenLinked to chrismdp/issue-to-pull-request#appConnectionThe repository, through itsown GitHub AppLinked to chrismdp/ci-a-commit#checksData · untrustedThe head commit's checks:test (1.22), test (stable), Bugreview, Security review andClassifyData · untrustedThe pull request's patchagainst main, and the branch'scommits, from gitCode · sortSort by GitHub's record,passing on only the reviewedcommit's sha and a count: isthe pull request open, its headstill the reviewed commit, andhow many fix commits doesthe branch carry?Its tries are derived, not stored: the fix commits on the branch, counted by the line each one's message carries, which survives a rebase. Someone pushed since means the head isn't the commit that was judged (#160 rule 3) A person's push since the review, or a closed pull request, means there is nothing to fix: it exits with no comment. Only giving up, after two fix commits, or a fix it may not push, posts one, since a person must act (#160 rule 5) A stranger's pull request from the same branch starts CI, and so the fixer, too, and GitHub keeps only one run waiting in a concurrency group, so a newer one would displace the real one's. The decide job takes no turns, and a stranger's run leaves there; the fix and write jobs take turns per pull request, never cancelled3Code · bring-inBring in both reviews' findingsfrom their checks, and thepatch, as read-only files forthe sessionDataThe findings and the patch,made read-onlyDataMain's fix prompt, AGENTS.mdand REVIEW.md, copied outbefore the session startsAI · draftFix every finding of bothreviews, Important ones andNits, test firstThe findings came from sessions that read the patch, and the patch is the branch's, so text in either can steer this session. Nothing can stop it before the push, so the push goes under a standing approval. The offer and giving up quote none of it The session runs its commands, tests included, only in its sandbox, keeping the login token from them, loading no settings or servers from the tree. The job runs nothing of its own beside the secret (awml #121, #160) Read is limited to the workspace and the session's own input folder, with Read(//proc/**) denied, and Edit, which covers writing a file, is denied on .git/**. Git after the session runs with core.hooksPath=/dev/null and core.fsmonitor=false (#160)3DataThe fix, as a patchThe fix is an artifact within this run only, because the job split keeps the app's token away from the change's code. The fix job gives its sha256 as an output, the write job checks it, and everything the session wrote is scanned for a token before it's uploaded, pushed or posted. It's uploaded with retention-days: 30, so an offered fix can be downloaded for 30 days (#160 rule 6)Code · sortSort by git: does the fix touch.github/ or Claude Code's ownfiles, and is the branch still atthe reviewed commit?The app may not change a workflow, and Claude Code's own files, .claude/, .mcp.json or a CLAUDE.md, are never pushed by a session, so a fix touching either is offered for a person: decided in #160's follow-up, the comment carries the gh run download command for the run's fix artifact, not the patch, so nothing is escaped or cappedCode · doPush the fix as one commitcarrying the fix line in itsmessage, with--force-with-lease at thereviewed commitPerson · standing approvalThe maintainer's standingapproval: one fix commit afailed review, to the pullrequest's own branch, up totwo a branch, accepted by themaintainer in #160, under rule4Outside actionPush one commit to the pullrequest's branchAskIt pushes and doesn't judge. The push starts CI, so a fix that breaks a test or brings its own Important finding fails there and comes back here, up to two tries. A bad push costs a CI run (#160 rule 4)Code · doOffer the fix in a newcomment from fixed text: ghrun download <run> -n fix, thegit apply command, and thatthe artifact lasts 30 days,quoting none of the fixOutside actionComment on the pull requestwith the commandsAllowSurfaceThe pull request's commitsand thread, where each pushand comment is listedCode · doSay in a new comment, fromfixed text, that the findingsneed a person, linking thefailed checks and the runOutside actionComment on the pull requestAllow