Author SHA1 Message Date
christianandClaude Opus 5.5 f666bdf6b3 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>
2026-10-21 00:56:51 +02:00
christianandClaude Opus 5.5 ed197eeeb1 Address review round 3: safe API snippets, PR opened once, pinned review
- Titles and bodies go through --rawfile files, never inline shell quotes
- pipefail + curl -f + jq -e so a failed call can't look successful
- Fix rounds merge latest main and skip re-opening the PR
- Check the branch isn't main before pushing; never bypass the hooks
- Review at a pinned level; only correctness findings checked against the
  code block approval

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 00:50:38 +02:00
christianandClaude Opus 5.5 fa45c58c0c Address review round 2: no-track branches, honest enforcement, safer API calls
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 00:32:15 +02:00
christianandClaude Opus 5.5 962c1aa9a1 Address review: fresh base, named review target, round cap, Gitea API
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 00:22:48 +02:00
christianandClaude Opus 5.5 82ba704957 Document branch-and-PR workflow; never write to main
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 00:12:37 +02:00
9 changed files with 57 additions and 159 deletions

No files matched your search

+1 -5
View File
@@ -16,11 +16,7 @@
"Read(~/.aws/**)", "Read(~/.aws/**)",
"Read(~/.gnupg/**)", "Read(~/.gnupg/**)",
"Edit(.claude/settings.json)", "Edit(.claude/settings.json)",
"Edit(.claude/settings.local.json)", "Edit(.claude/settings.local.json)"
"Bash(git push origin main:*)",
"Bash(git push origin HEAD:main:*)",
"Bash(git push * main:*)",
"Bash(git push *:main:*)"
], ],
"blockReadsOutsideWorkingDirectories": true, "blockReadsOutsideWorkingDirectories": true,
"ask": [ "ask": [
-10
View File
@@ -1,10 +0,0 @@
#!/bin/sh
# Refuse commits (and, via pre-merge-commit, merge commits) while main is checked out.
# Cherry-pick, revert and fast-forward merges don't run these hooks; pre-push still stops
# their result from reaching the remote.
name=${1:-pre-commit}
if [ "$(git symbolic-ref --short -q HEAD)" = "main" ]; then
echo "$name: committing on main is not allowed; create a branch first." >&2
exit 1
fi
exit 0
-2
View File
@@ -1,2 +0,0 @@
#!/bin/sh
exec sh "$(dirname "$0")/pre-commit" pre-merge-commit
-11
View File
@@ -1,11 +0,0 @@
#!/bin/sh
# Reject any push that would create, update or delete main on the remote, however the
# command is spelled (bare `git push`, HEAD:main, +main, refs/heads/main, --all, --mirror).
# Only a human merges into main, through a PR in Gitea.
while read -r local_ref local_sha remote_ref remote_sha; do
if [ "$remote_ref" = "refs/heads/main" ]; then
echo "pre-push: pushing to main is not allowed; push a branch and open a PR instead." >&2
exit 1
fi
done
exit 0
+55 -2
View File
@@ -3,8 +3,7 @@
A tiny zero-dependency Node.js todo CLI, used as a sandbox for trying Claude Code features. A tiny zero-dependency Node.js todo CLI, used as a sandbox for trying Claude Code features.
## Commands ## Commands
- `npm install` — once per clone (no dependencies): installs the git hooks that block commits and pushes to `main`. If it fails in the sandbox, ask the human to run it. - `npm test` — run the test suite (built-in `node:test`, no install needed)
- `npm test` — run the test suite (built-in `node:test`)
- `npm start -- add "something"` / `npm start -- ls` — run the CLI (data in `todos.json`, override with `TODO_FILE`) - `npm start -- add "something"` / `npm start -- ls` — run the CLI (data in `todos.json`, override with `TODO_FILE`)
## Conventions ## Conventions
@@ -13,5 +12,59 @@ A tiny zero-dependency Node.js todo CLI, used as a sandbox for trying Claude Cod
- Every behavior change gets a test in `test/`. - Every behavior change gets a test in `test/`.
- Commit messages: short imperative subject line. - Commit messages: short imperative subject line.
## Workflow (required)
Never commit, merge, or push to `main`. Only a human (Christian) merges into `main`, in Gitea.
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`).
2. **Commit, test, push.** `npm test` must pass. Check that `git branch --show-current` isn't `main`, then run `git push -u origin HEAD`.
3. **Open the PR**, once per branch (see "Gitea API").
4. **Review** with `/code-review <branch> medium`.
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.
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._`
- Then the blocking findings, then the notes, then the findings you rejected, each with the reason.
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. **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
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`. 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:
- Make each call one self-contained command, because shell state doesn't carry over between commands.
- 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.
- 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
# 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
(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" \
'{head:$head, base:"main", title:($title|rtrimstr("\n")), body:$body}' \
| curl -fsS -X POST -H @"$TMPDIR/gitea-auth" -H "Content-Type: application/json" -d @- "$API/pulls" \
| 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.
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}' \
| curl -fsS -X POST -H @"$TMPDIR/gitea-auth" -H "Content-Type: application/json" -d @- "$API/issues/<number>/comments" \
| jq -er .html_url
```
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`.
-9
View File
@@ -3,7 +3,6 @@
A tiny todo CLI that exists to try out [Claude Code](https://docs.claude.com/en/docs/claude-code) features. A tiny todo CLI that exists to try out [Claude Code](https://docs.claude.com/en/docs/claude-code) features.
```sh ```sh
npm install # once per clone, and after changing .githooks/: installs the git hooks
npm test npm test
npm start -- add "try Claude Code" npm start -- add "try Claude Code"
npm start -- done 1 npm start -- done 1
@@ -19,14 +18,6 @@ npm start -- ls
| `.claude/skills/changelog/` | A custom skill — run `/changelog` | | `.claude/skills/changelog/` | A custom skill — run `/changelog` |
| `.claude/agents/test-writer.md` | A custom subagent — "use the test-writer agent to…" | | `.claude/agents/test-writer.md` | A custom subagent — "use the test-writer agent to…" |
### Protecting `main`
Changes reach `main` only through a PR that a human merges. Three layers, strongest first:
1. **Gitea branch protection** on `main` (set in the repo settings on code.westerkam.org): the only layer that can't be bypassed from a clone.
2. **Git hooks** from `.githooks/`, copied into `.git/hooks` by `npm install` (`scripts/install-hooks.sh`). `pre-push` rejects any push that would update `main`, however it's spelled. `pre-commit` / `pre-merge-commit` refuse ordinary and merge commits while `main` is checked out; cherry-pick, revert and fast-forwards skip them, but pre-push still stops the result leaving the machine. The hooks are copied, not linked with `core.hooksPath`, because Claude Code's sandbox can't write inside `.git/`: a sandboxed command can't rewrite the installed hooks, which matters because `git commit` / `git push` run outside the sandbox. Anyone can still skip them with `--no-verify`.
3. **Claude Code deny rules** in `.claude/settings.json` for `git push … main`: a quick early stop, but they match command text, so they're easy to get around.
Layers 2 and 3 live on your machine and can be bypassed by whoever runs the commands; only layer 1 can't.
## Things to try ## Things to try
- *"There's a bug in how ids are assigned after removing a todo — find it and fix it with a test."* - *"There's a bug in how ids are assigned after removing a todo — find it and fix it with a test."*
+1 -2
View File
@@ -6,8 +6,7 @@
"bin": { "todo": "./src/cli.js" }, "bin": { "todo": "./src/cli.js" },
"scripts": { "scripts": {
"start": "node src/cli.js", "start": "node src/cli.js",
"test": "node --test", "test": "node --test"
"prepare": "sh scripts/install-hooks.sh"
}, },
"engines": { "node": ">=20" }, "engines": { "node": ">=20" },
"license": "MIT" "license": "MIT"
-20
View File
@@ -1,20 +0,0 @@
#!/bin/sh
# Copy the hooks in .githooks/ into git's own hooks directory. They are copied rather than
# enabled with core.hooksPath on purpose: Claude Code's sandbox can't write inside .git/, so
# a sandboxed command can't rewrite the installed hooks, even though git commit and git push
# (and so their hooks) run outside the sandbox. Re-run after changing anything in .githooks/.
set -e
cd "$(dirname "$0")/.."
# Use the repo's own hooks directory, not `git rev-parse --git-path hooks`: that follows a
# (possibly global) core.hooksPath and would install these hooks for every repo.
if ! common=$(git rev-parse --git-common-dir 2>/dev/null); then
echo "install-hooks: not a git checkout; skipping." >&2
exit 0
fi
hooks="$common/hooks"
mkdir -p "$hooks"
for hook in .githooks/*; do
cp "$hook" "$hooks/"
chmod +x "$hooks/$(basename "$hook")"
done
echo "install-hooks: installed $(ls .githooks | tr '\n' ' ')into $hooks"
-98
View File
@@ -1,98 +0,0 @@
import { test, after } from "node:test";
import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import { mkdtempSync, rmSync, cpSync, statSync, writeFileSync, existsSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { fileURLToPath } from "node:url";
const ROOT = fileURLToPath(new URL("../", import.meta.url));
const HOOKS = join(ROOT, ".githooks");
const SHA = "1".repeat(40);
const ZERO = "0".repeat(40);
// Run git and the hooks without inheriting GIT_DIR & co, e.g. when tests run from inside a hook.
const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !k.startsWith("GIT_")));
const run = (cmd, args, opts = {}) => spawnSync(cmd, args, { env, encoding: "utf8", ...opts });
const dirs = [];
after(() => dirs.forEach((d) => rmSync(d, { recursive: true, force: true })));
function repoOn(branch) {
const dir = mkdtempSync(join(tmpdir(), "todo-hooks-"));
dirs.push(dir);
const init = run("git", ["init", "-q", "-b", branch, dir]);
assert.equal(init.status, 0, `git init failed: ${init.stderr}`);
return dir;
}
function prePush(...lines) {
return run("sh", [join(HOOKS, "pre-push"), "origin", "git@example:repo.git"], {
input: lines.map((l) => l + "\n").join(""),
});
}
for (const [label, line] of [
["an update", `refs/heads/fix/x ${SHA} refs/heads/main ${SHA}`],
["a new ref", `refs/heads/fix/x ${SHA} refs/heads/main ${ZERO}`],
["a deletion", `(delete) ${ZERO} refs/heads/main ${SHA}`],
]) {
test(`pre-push rejects ${label} of main`, () => {
const res = prePush(line);
assert.equal(res.status, 1);
assert.match(res.stderr, /pushing to main is not allowed/);
});
}
test("pre-push rejects --all style pushes that include main", () => {
const res = prePush(`refs/heads/fix/x ${SHA} refs/heads/fix/x ${SHA}`, `refs/heads/main ${SHA} refs/heads/main ${SHA}`);
assert.equal(res.status, 1);
});
test("pre-push allows other branches, including ones named like main", () => {
const res = prePush(`refs/heads/fix/x ${SHA} refs/heads/fix/x ${ZERO}`, `refs/heads/maintenance ${SHA} refs/heads/maintenance ${SHA}`);
assert.equal(res.status, 0);
});
for (const hook of ["pre-commit", "pre-merge-commit"]) {
test(`${hook} rejects main, names itself, and allows other branches`, () => {
const onMain = run("sh", [join(HOOKS, hook)], { cwd: repoOn("main") });
assert.equal(onMain.status, 1);
assert.match(onMain.stderr, new RegExp(`^${hook}: committing on main is not allowed`));
assert.equal(run("sh", [join(HOOKS, hook)], { cwd: repoOn("fix/x") }).status, 0);
});
}
test("install-hooks copies every hook into .git/hooks as an executable", () => {
const repo = repoOn("fix/x");
cpSync(join(ROOT, "scripts"), join(repo, "scripts"), { recursive: true });
cpSync(HOOKS, join(repo, ".githooks"), { recursive: true });
const res = run("sh", [join(repo, "scripts", "install-hooks.sh")]);
assert.equal(res.status, 0, res.stderr);
for (const hook of ["pre-push", "pre-commit", "pre-merge-commit"]) {
assert.ok(statSync(join(repo, ".git", "hooks", hook)).mode & 0o100, `${hook} is not executable`);
}
});
test("install-hooks ignores a global core.hooksPath", () => {
const repo = repoOn("fix/x");
const elsewhere = mkdtempSync(join(tmpdir(), "todo-hooks-"));
dirs.push(elsewhere);
const globalConfig = join(elsewhere, "gitconfig");
writeFileSync(globalConfig, `[core]\n\thooksPath = ${elsewhere}\n`);
cpSync(join(ROOT, "scripts"), join(repo, "scripts"), { recursive: true });
cpSync(HOOKS, join(repo, ".githooks"), { recursive: true });
const res = run("sh", [join(repo, "scripts", "install-hooks.sh")], { env: { ...env, GIT_CONFIG_GLOBAL: globalConfig } });
assert.equal(res.status, 0, res.stderr);
assert.ok(existsSync(join(repo, ".git", "hooks", "pre-push")));
assert.equal(existsSync(join(elsewhere, "pre-push")), false);
});
test("install-hooks skips cleanly outside a git checkout", () => {
const dir = mkdtempSync(join(tmpdir(), "todo-hooks-"));
dirs.push(dir);
cpSync(join(ROOT, "scripts"), join(dir, "scripts"), { recursive: true });
const res = run("sh", [join(dir, "scripts", "install-hooks.sh")], { env: { ...env, GIT_CEILING_DIRECTORIES: tmpdir() } });
assert.equal(res.status, 0);
assert.match(res.stderr, /not a git checkout; skipping/);
});