Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions .claude/guardrails.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# Guardrails — the long version

`CLAUDE.md` states each working rule as one line: the rule and its shortest
reason. This file holds what does not fit there — the incident that produced
the rule, the recipe it implies, the numbers that make it credible.

The split exists because `CLAUDE.md` is loaded into **every** session, so its
cost is paid on every turn, while a retro narrative is needed only when
someone actually hits the situation. Nothing here is a new rule. If this file
and `CLAUDE.md` ever disagree, `CLAUDE.md` is the rule and this is the
commentary that fell behind.

Sections follow the order in which `CLAUDE.md` states the rules.

---

## Delegated agents run on Opus by default

Owner directive, 2026-08-11, refined 2026-08-16.

Pass `model: opus` when spawning subagents or workflows — the tier the
`agentic/README.md` model table already uses for large tasks. Escalate to
Fable for genuinely hard reasoning, drop to Sonnet or Haiku for mechanical
grinding, but Opus is the default answer.

Escalation is for judgment calls, not for every detail. Within its briefed
scope a delegate decides routine matters itself and documents them; otherwise
delegation gains nothing. What comes back to the main loop, which keeps the
overview, is anything that changes scope, contradicts the brief or the docs,
or would be expensive to redo. Say that split explicitly in the prompt:
decide-and-document versus return-as-finding.

**Two lines every brief carries** (2026-09-05). A delegate reads `CLAUDE.md`
like anyone else, but it also receives harness reminders that arrive LATER in
its context and therefore read as the more recent instruction. In auto mode
one of them prescribes editing files through `sed`, heredocs and short
scripts. In the sibling repository three agents in one day followed it against
the Edit/Write rule, two of them for the single-token `sed -i` that fills the
PR number into a changelog fragment — the same fragment format this repository
uses. A fourth ran `git checkout -b` in the SHARED checkout, which moves a ref
the owner's own working tree is sitting on. So spell both out in the brief, in
the brief's own words:

1. Repo files are modified ONLY with Edit/Write — `CLAUDE.md` wins over the
auto-mode reminder.
2. All `git` happens in the agent's own worktree; the shared checkout is left
on the branch it was found on.

Neither is new policy. Both are precedence a brief has to make explicit,
because an agent cannot infer precedence from the order messages arrived in.

## External-system writes need explicit, named authorization

The rule is the asking. This section is about what to hand over once the
answer is yes and the agent still cannot run the command itself.

Some prod runbooks are refused by the harness permission classifier rather
than by the owner — the `gcloud run services update` plus `update-traffic`
pair that arms the site's origin gate is one, observed 2026-09-04. Reaching
for a different phrasing of the same command is the wrong move: the classifier
is not an obstacle to route around. Stopping half-way is not the danger
either, when the runbook is built for it — the pair in
`infra/cloudflare/README.md` § "Arming, in full" stages the new revision and
promotes it by name, so a config that fails to render leaves the revision
never ready and traffic on the old one, which the README calls a safe failure
and the reason its step 4 is not optional. The real danger is reading the
STAGED revision as the finished one and reporting an arm that never took
traffic.

The pattern that works: write the runbook as ONE script in the scratchpad —
never into the repo — and hand the owner a single line to paste:

```
! bash /tmp/…/scratchpad/arm-origin-gate.sh
```

The script does the whole pair under `set -euo pipefail`, echoes what it is
about to change, and ends by reading the state back — for that gate, the
`x-origin-gate` response header AND `status.traffic`, because only the second
says which revision answered. Rules for it: one action per script, and no
secret values in its output.

**The reverse direction is its own script, not the same one with a flag.**
`infra/cloudflare/README.md` § "Rolling back" is explicit about why: a
rollback has to run in the worst state the service can be in, which includes
the secret having been disabled during the incident — so it never looks the
SECRET up. It still resolves the serving revision and its image, because it
has to; what it must not do is depend on anything the incident may have taken
away.

Afterwards, VERIFY from the session with a read the agent is allowed to make,
instead of trusting a "done" in chat. A runbook that shipped this way belongs
in the owning README, so the next round starts from a reviewed text rather
than a fresh improvisation.

## Modify repo files only with the Edit/Write tools

Appending with `>>` counts — appending at the end of a file is exactly the
forbidden path, however little it feels like editing.

When a Bash command legitimately mutates a tracked file (a formatter, codegen,
`git checkout`), read the file again before the next edit; stale-state errors
cascade otherwise.

The moment this rule gets broken is when an edit ANCHOR fails — "string not
found", "file modified since read". The answer is a fresh targeted read plus a
longer anchor, never a regex rewrite from a heredoc. The other moment is a
change small enough to feel exempt: a one-token substitution reads like a
`sed` job, and `sed -i` on a changelog fragment is how three delegated agents
in one day broke the rule in the sibling repository (2026-09-05). Size is not
the criterion; the tool is.

The exemptions are narrow and stated in `CLAUDE.md`: GitHub Actions workflows
and codegen scripts write files by design. An interactive session is never one
of those.
115 changes: 108 additions & 7 deletions .claude/skills/open-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,36 @@ format (Summary / Plan / Test plan), the changelog gate, the push,
and the PR-ref follow-up. English throughout, no
"Generated with..." lines in the body.

**A multi-paragraph commit message goes through a file — and that
file belongs to this branch alone.** `git commit -F` keeps the prose
out of shell quoting, but the scratchpad is shared by every agent of
one session, so a generic `commitmsg.txt` gets overwritten by a
parallel agent and a later re-read commits someone else's text
(sibling repo, 2026-09-05: both a message and a body file were
clobbered mid-run). Take a private directory, which needs no
sanitising at all:

```bash
D=$(mktemp -d) # or, if you name it: BRANCH=$(git branch --show-current)
MSG="$D/commitmsg.txt" # SLUG=${BRANCH//\//-}; MSG="$D/commitmsg-$SLUG.txt"
```

A branch name is not a filename — `release/v1.2.3` turns the slash
into a directory that does not exist — so substitute the separators
if you derive the name, and never write into a `$SCRATCH` you have
not set yourself.

Write the file with the Write tool, then `git commit -F "$MSG"` in
the SAME step that wrote it — never re-read one a turn later to reuse
it, because between the two it may belong to another agent. These are
scratch input to one command, not a record; the record is the commit.

The same holds for a PR body you pass as `--body-file`. The mandated
`/pull_request` command does not take that path — it builds the body
inline with a quoted heredoc (`agentic/commands/pull_request.md`
step 6), which has no collision to avoid — so this applies when you
write a body file yourself.

## 3 · After opening: pipeline + review loop (do not skip)

Repeat until **both** hold: all checks pass AND zero unresolved
Expand All @@ -135,15 +165,32 @@ On failure: `gh run view --log-failed`, fix, push to the same branch,
keep watching.

**b. Wait for the Copilot review.** Bot login:
`copilot-pull-request-reviewer[bot]`. It usually lands within ~2 min;
if `gh pr view <num> --json reviewRequests,reviews` shows neither a
request nor a review after the checks pass, request it explicitly
(verified working):
`copilot-pull-request-reviewer[bot]`. It arrives a few minutes after
the PR is OPENED — not after each push; see "One review per PR"
below. If `gh pr view <num> --json reviewRequests,reviews` shows
neither a request nor a review after the checks pass, request it
explicitly (verified working):

```bash
gh api -X POST repos/{owner}/{repo}/pulls/<num>/requested_reviewers -f "reviewers[]=copilot-pull-request-reviewer[bot]"
```

**One review per PR is the normal case now.** The ruleset "Automated
Copilot Code Review" (anyplot 10370785, kurrentschrift 18516317)
carries `review_on_push: false` since 2026-09-03 — the owner asked
for the churn to stop, and the setting, not any skill, was what
re-reviewed. Two consequences for this loop. A FIX push starts no new
Copilot run, so a `copilot-*` check on the new head SHA is
legitimately ABSENT; waiting for one that will never come is the
failure mode to avoid — see §3e for what to require instead. And a
fresh review is requested only after a SUBSTANTIVE rework (new
behaviour, a reworked mechanism), never after every push: each
request re-reads the whole diff and surfaces "previously missed"
findings in files the push never touched, which draws another push
(kurrentschrift#406 collected ~15 requests in a day over a one-line
docstring fix). Stop once a round yields no new inline comments but
only carried-over items.

Fetch all three comment surfaces — they carry different content:

```bash
Expand Down Expand Up @@ -180,6 +227,57 @@ gh api graphql -f query='mutation($id:ID!){resolveReviewThread(input:{threadId:$
report the PR URL and final state. **Do not merge unless explicitly
authorized.**

**Merging on request: wait for the review, not just for green.**
When the owner does ask for the merge in this session, four
conditions, all read on the CURRENT head SHA — re-read it after every
push, `gh pr view <num> --json headRefOid`:

1. A draft is not reviewable — `gh pr ready <num>` first. Copilot
does not review a draft, so a draft merged "green" was never
reviewed, and `gh pr merge` on a draft fails anyway. Check
`isDraft`.
2. Every non-Copilot check on the head SHA is `completed` and green.
Dedupe the check runs **by name, newest wins**: a superseded run
(a label re-trigger, a cancelled first attempt) stays beside the
current one and reads as a red check that is not there any more.
Dedupe on `.id`, which grows with creation and is always set — a
check run carries no `created_at`, and `started_at` stays null
until the run begins, so a `max_by(.started_at)` would hand the
row to the OLD completed attempt while the new one is still
queued, which is the failure this step exists to prevent.
```bash
gh api repos/{owner}/{repo}/commits/$(gh pr view <num> --json headRefOid --jq .headRefOid)/check-runs \
--jq '[.check_runs[]] | group_by(.name) | map(max_by(.id)) | .[] | "\(.name): \(.status) \(.conclusion // "")"'
```
3. **A Copilot review actually exists on the PR** — `gh pr view <num>
--json reviews`, author `copilot-pull-request-reviewer`. The
head-SHA check run does not prove one: a run reaches `completed`
with conclusion `cancelled` and delivers nothing. So read the
check run only to learn whether a round is still RUNNING
(`queued`/`in_progress` means wait) and read the review list to
learn whether one was ever delivered. Since `review_on_push` is
off (§3b), the normal state after a fix push is no run on the head
at all with the first round's review standing — that is reviewed,
not unreviewed. If no review exists and the run was cancelled, one
re-request is the whole budget; after that report
green-and-unreviewed and let the owner decide, never loop.
4. Zero unresolved review threads (step c), outdated ones included.

**Merge state is two different fields; read each by its own name.**
`mergeable` (`gh pr view --json mergeable`) is `MERGEABLE`,
`CONFLICTING` or `UNKNOWN` — `UNKNOWN` right after another merge is
GitHub still computing, so keep polling. `mergeStateStatus` is the
richer enum, where the conflicting case is `DIRTY`. A conflict is not
transient and has a symptom worth knowing: GitHub starts no CI at
all, so the PR shows no red check, just none (#11212 and
kurrentschrift#524, 2026-09-04, both read as "checks pending" for a
while). Report it and merge `origin/main` into the branch instead of
waiting it out.

Poll all of this from ONE script rather than by hand, and kill a
stale wait loop with the bracket trick (`pkill -f "x[.]y"`), or
`pkill` matches its own calling shell.

## 4 · After merge (when it happens): watch the deploy

Merges to `main` touching `api/**`, `core/**`, or `pyproject.toml`
Expand All @@ -206,9 +304,12 @@ a 20-minute poll on the global list never saw the builds). Match the
- **`isOutdated` ≠ `isResolved`.** A fix-push can outdate a Copilot
thread while it stays unresolved; outdated threads still count
against review-clean — resolve them explicitly.
- **Copilot reviews every push round.** New threads on changed lines
are the loop working, not noise — but don't chase cosmetic nits
past a couple of rounds; surface stalemates to the user.
- **A fix push no longer starts a review round.** `review_on_push` is
`false` since 2026-09-03 (§3b), so only an explicit — and
substantive — re-request opens another one. When a round does run,
new threads on the changed lines are the loop working, not noise —
but don't chase cosmetic nits past a couple of rounds; surface
stalemates to the user.
- **Stacked PRs die when their base squash-merges.** Don't stack; if
work depends on an unmerged PR, wait for its merge (or do the work
and rebase before opening).
Expand Down
21 changes: 21 additions & 0 deletions .claude/skills/verify-frontend/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,27 @@ than restating values here. Method:
Style questions are **findings to report**, not things to silently
fix — palette and typography decisions are settled in the style guide.

## 3b · Numeric rules: measure the result, not the plan

**A numeric UI rule is verified against the MEASURED result in the
browser, never against the planned one.** A floor, a minimum size, a
cap, a hit target, the ~13 px legibility floor above — the
verification names the rule and the number it measured, on every
surface the rule reaches, at both viewports.

The case for it is a sibling-repo PR that shipped a 14 px x-height
floor for rendered lines whose planner sized them from the average
advance per character. The plan met the floor and the widest real
line did not, because the frame's own padding scales with the content
and was never in the budget: the page came out at **13.9 px** — a
rule broken by the code that enforces it, and only the measurement on
the page could say so.

So read the number off the element (`getBoundingClientRect()`,
`getComputedStyle`), not off the code that computed it, and quote the
measurement in the PR. When the fix is to re-plan from the measured
value, say plainly which cases still fall outside the rule.

## 4 · Performance

Only when the change can plausibly move performance (data loading,
Expand Down
29 changes: 29 additions & 0 deletions .claude/skills/write-docs/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,35 @@ the house guide, repository docs follow Google style. Existing docs
migrate **on touch** (the "fix formatting" rule below), no bulk
rewrites required.

## Shortening a text that carries claims

Applies wherever the text lives — a `docs/` reference, `README.md`,
the site's legal and about copy (`app/src/pages/LegalPage.tsx`,
`app/src/pages/AboutPage.tsx`) — whenever the edit makes an existing
text SHORTER: a legal section, a licensing paragraph, a factsheet,
any prose a reader may rely on.

**Diff claim by claim against the previous version, not paragraph by
paragraph.** Shortening drops qualifiers before it drops sentences,
and a qualifier is what makes a claim true: "only", "unless", "up
to", a condition on a right, the last item of a list. In the sibling
repository on 2026-09-03 a privacy section lost exactly three that
way — an overstated retention period for the rate-limit counters, the
condition on the right to object, and part of a list of rights —
inside an edit that was otherwise a genuine improvement. The check
that catches it is mechanical: list the claims of the old text, then
tick each one off against the new text as kept, deliberately dropped,
or narrowed.

Two rules on top:

- **The owner's own sentences stay verbatim.** Where he supplied the
wording, it is quoted, not paraphrased and not tightened. Say in
the PR body that it is his sentence.
- **A claim you cannot support is removed, never softened** into a
vaguer version of itself: an unproven statement made fuzzy is still
an unproven statement.

## Formatting

- Actually FIX formatting issues while editing (headings, lists, code
Expand Down
Loading
Loading