Give the PR workflow a hard stop
Only confirmed correctness bugs block approval; everything else is a note. At most one fix round, then hand off. Don't widen scope to satisfy a review. Merge main only on conflict or request. The token goes in a private header file, and PR responses are checked for a number. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ed197eeeb1
commit
f666bdf6b3
1 file changed
+33
-19
@@ -13,44 +13,58 @@ A tiny zero-dependency Node.js todo CLI, used as a sandbox for trying Claude Cod
|
|||||||
- Commit messages: short imperative subject line.
|
- Commit messages: short imperative subject line.
|
||||||
|
|
||||||
## Workflow (required)
|
## Workflow (required)
|
||||||
Never commit, merge, or push to `main`. Only a human (Christian) merges into `main`.
|
Never commit, merge, or push to `main`. Only a human (Christian) merges into `main`, in Gitea.
|
||||||
This is enforced by the git hooks that `npm install` installs and by Gitea branch protection (see "Protecting `main`" in the README). Never get around them: no `--no-verify`, no changing `core.hooksPath`, no editing `.git/`.
|
The git hooks installed by `npm install` and Gitea branch protection enforce this (see "Protecting `main`" in the README). Never get around them: no `--no-verify`, no changing `core.hooksPath`, no editing `.git/`.
|
||||||
1. **Branch.** Run `git fetch origin` on its own, then `git switch --no-track -c <type>/<short-name> origin/main` (e.g. `fix/duplicate-ids`, `docs/readme`). `--no-track` stops the branch from tracking `main`.
|
|
||||||
2. **Commit and test.** Commit on the branch; `npm test` must pass before every push.
|
1. **Branch.** Run `git fetch origin` on its own, then `git switch --no-track -c <type>/<short-name> origin/main` (e.g. `fix/duplicate-ids`).
|
||||||
3. **Push.** Check that `git branch --show-current` isn't `main`, then run `git push -u origin HEAD`.
|
2. **Commit, test, push.** `npm test` must pass. Check that `git branch --show-current` isn't `main`, then run `git push -u origin HEAD`.
|
||||||
4. **Open the PR**, once per branch (see "Gitea API" below).
|
3. **Open the PR**, once per branch (see "Gitea API").
|
||||||
5. **Review** with `/code-review <branch> medium`. Name both the branch and the level, so every round diffs against `main` at the same depth.
|
4. **Review** with `/code-review <branch> medium`.
|
||||||
6. **Comment** on the PR with the verdict.
|
5. **Triage.** The review lists unverified candidates, and it will always find some. Check each one against the code. A finding **blocks** only if it is a correctness bug in this PR's change that you have confirmed, ideally with a test that fails. Everything else is a **note**: cleanups, judgement calls, hardening ideas and unconfirmed findings.
|
||||||
- First line: **Needs fixes** if any correctness finding still holds after you check it against the code; otherwise **Approved**. Cleanup and judgement-call findings are listed but don't block.
|
6. **Comment** on the PR:
|
||||||
|
- First line: **Needs fixes** if anything blocks, otherwise **Approved**.
|
||||||
- Second line: `_Automated self-review by Claude; not an independent review._`
|
- Second line: `_Automated self-review by Claude; not an independent review._`
|
||||||
- Then the findings, including any you checked and rejected, and why.
|
- Then the blocking findings, then the notes, then the findings you rejected, each with the reason.
|
||||||
7. **Fix rounds.** If it needs fixes, first bring in the latest `main`: run `git fetch origin`, then `git merge --no-ff --no-commit origin/main`, then `git commit --no-edit`, each as its own command. Don't rebase, because force pushes are denied. Fix the correctness findings, then repeat steps 2, 3, 5 and 6 (not step 4: the PR already exists). Do at most 2 fix rounds, then post what's left and hand off.
|
7. **At most one fix round.** If the PR needs fixes, fix only the blocking findings, each with a test. Push, review once more, and post the verdict from steps 5–6. Then stop, whatever that second review says. Findings about the fixes themselves go to the human as notes; don't fix them.
|
||||||
8. **Stop.** Never merge, even if approved; tell the human it's ready.
|
8. **Hand off.** Never merge, even if the PR is approved. Tell the human it's ready, with the verdict and any notes.
|
||||||
|
|
||||||
|
Don't widen scope beyond the PR's purpose to satisfy a review. If a fix would add significant new code (a new mechanism, not a correction), list it as a note for the human instead.
|
||||||
|
|
||||||
|
Bring in the latest `main` only when the PR conflicts or the human asks:
|
||||||
|
1. Run `git fetch origin`.
|
||||||
|
2. Run `git merge --no-ff --no-commit origin/main`.
|
||||||
|
3. If it says "Already up to date", stop here. Otherwise resolve any conflicts and run `npm test`.
|
||||||
|
4. Run `git commit --no-edit`.
|
||||||
|
|
||||||
|
Run each as its own command, and never rebase or force-push.
|
||||||
|
|
||||||
### Gitea API
|
### Gitea API
|
||||||
No `gh`/`tea`: use `curl` with `$GITEA_TOKEN`.
|
No `gh`/`tea`: use `curl` with `$GITEA_TOKEN`.
|
||||||
|
|
||||||
One-time human setup in the git-ignored `.claude/settings.local.json`: set `env.GITEA_TOKEN`, and add `code.westerkam.org` to `sandbox.network.allowedDomains`. Settings load at session start, so restart the session after adding them (`claude --continue`). If the token is unset, stop and ask the human; don't work around it.
|
One-time human setup in the git-ignored `.claude/settings.local.json`: set `env.GITEA_TOKEN`, and add `code.westerkam.org` to `sandbox.network.allowedDomains`. Then restart the session (`claude --continue`); settings load only at start. If the token is unset, stop and ask the human.
|
||||||
|
|
||||||
Rules for every API call:
|
Rules for every API call:
|
||||||
- Make each call one self-contained command, because shell state doesn't carry over between commands.
|
- Make each call one self-contained command, because shell state doesn't carry over between commands.
|
||||||
- Write titles and bodies to files in `$TMPDIR` (never in the repo) and pass them with `--rawfile`. Never put them inline in double quotes, where backticks and `$(…)` would run.
|
- Put titles and bodies in files in `$TMPDIR` (never in the repo) and pass them with `--rawfile`, never inline in double quotes, where backticks and `$(…)` would run.
|
||||||
- `pipefail`, `curl -f` and `jq -e` make every failure exit non-zero.
|
- The token goes in a private header file, so it doesn't show up in the process list.
|
||||||
|
- `pipefail`, `curl -f` and the `jq` checks make every failure exit non-zero.
|
||||||
```sh
|
```sh
|
||||||
# Open a PR for the current branch (once). Write $TMPDIR/pr-title.txt and $TMPDIR/pr-body.md first.
|
# Open a PR for the current branch (once). Write $TMPDIR/pr-title.txt and $TMPDIR/pr-body.md first.
|
||||||
set -o pipefail; API=https://code.westerkam.org/api/v1/repos/christian/claude-playground
|
set -o pipefail; API=https://code.westerkam.org/api/v1/repos/christian/claude-playground
|
||||||
|
(umask 077; printf 'Authorization: token %s\n' "$GITEA_TOKEN" > "$TMPDIR/gitea-auth")
|
||||||
jq -n --arg head "$(git branch --show-current)" --rawfile title "$TMPDIR/pr-title.txt" --rawfile body "$TMPDIR/pr-body.md" \
|
jq -n --arg head "$(git branch --show-current)" --rawfile title "$TMPDIR/pr-title.txt" --rawfile body "$TMPDIR/pr-body.md" \
|
||||||
'{head:$head, base:"main", title:($title|rtrimstr("\n")), body:$body}' \
|
'{head:$head, base:"main", title:($title|rtrimstr("\n")), body:$body}' \
|
||||||
| curl -fsS -X POST -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" -d @- "$API/pulls" \
|
| curl -fsS -X POST -H @"$TMPDIR/gitea-auth" -H "Content-Type: application/json" -d @- "$API/pulls" \
|
||||||
| jq -er '"#\(.number) \(.html_url)"'
|
| jq -er 'if .number then "#\(.number) \(.html_url)" else error("no PR number in response") end'
|
||||||
|
|
||||||
# Comment on PR <number>. Write $TMPDIR/comment.md first.
|
# Comment on PR <number>. Write $TMPDIR/comment.md first.
|
||||||
set -o pipefail; API=https://code.westerkam.org/api/v1/repos/christian/claude-playground
|
set -o pipefail; API=https://code.westerkam.org/api/v1/repos/christian/claude-playground
|
||||||
|
(umask 077; printf 'Authorization: token %s\n' "$GITEA_TOKEN" > "$TMPDIR/gitea-auth")
|
||||||
jq -n --rawfile body "$TMPDIR/comment.md" '{body:$body}' \
|
jq -n --rawfile body "$TMPDIR/comment.md" '{body:$body}' \
|
||||||
| curl -fsS -X POST -H "Authorization: token $GITEA_TOKEN" -H "Content-Type: application/json" -d @- "$API/issues/<number>/comments" \
|
| curl -fsS -X POST -H @"$TMPDIR/gitea-auth" -H "Content-Type: application/json" -d @- "$API/issues/<number>/comments" \
|
||||||
| jq -er .html_url
|
| jq -er .html_url
|
||||||
```
|
```
|
||||||
Run `git fetch`, `git commit` and `git push` as standalone commands, with no `cd … &&` before them and nothing chained after. Only then do they run outside the sandbox, which they need to reach the remote and to sign commits.
|
Commands listed under `sandbox.excludedCommands` in `.claude/settings.json` (`git fetch`, `git commit`, `git push` and others) run outside the sandbox only when they are run standalone: no `cd … &&` before them and nothing chained after. They need that to reach the remote and to sign commits.
|
||||||
|
|
||||||
## Remote
|
## Remote
|
||||||
Gitea at code.westerkam.org — `origin` = `git@code.westerkam.org:christian/claude-playground.git`.
|
Gitea at code.westerkam.org — `origin` = `git@code.westerkam.org:christian/claude-playground.git`.
|
||||||
Reference in new issue
Block a user