diff --git a/.claude/skills/deliver/SKILL.md b/.claude/skills/deliver/SKILL.md index c37f17e..52ee1ff 100644 --- a/.claude/skills/deliver/SKILL.md +++ b/.claude/skills/deliver/SKILL.md @@ -45,24 +45,36 @@ gh issue view --json comments --jq '[.comments[] | select(.body | startswith git fetch origin / 2>/dev/null && git log --oneline origin// ^main ``` -If either exists, continue from there rather than starting over: check out -the branch, read its log, and make Not done your task list. +If either exists, continue from there rather than starting over: branch +from `origin//` (step 3), read its log, and make Not done your +task list. ## 3. Branch The branch name is `/` where `` is the label prefix, nothing -appended (rule 11). If it already exists on the remote, someone was here -before: check out that branch and read its log before doing anything else. +appended (rule 11). You are probably in a worktree (rule 13): always branch +from `origin/...`, never from a local branch, and never assume you can +switch to a branch another checkout holds. ```bash -git switch -c task/ main # or feature/ +git fetch origin +git switch -c task/ origin/main # or feature/ ``` -For a bug, start from the reproduction branch when it exists so the failing -test is carried forward: +For a bug, start from the reproduction branch so the failing test is +carried forward: ```bash -git fetch origin bug/-repro && git switch -c bug/ origin/bug/-repro +git switch -c bug/ origin/bug/-repro +``` + +If `origin//` already exists, someone was here before (step 2a): +branch from it, and if the local name is taken by another worktree, work +under a temporary name and push to the real one: + +```bash +git switch -c / origin// || git switch -c wip/ origin// +git push origin HEAD:/ ``` ## 4. Build @@ -72,6 +84,11 @@ prove, and `docs/internal/agent-rules/testing.md` says what proving means. Rerea files named under Rules in play before touching the code they cover. New or changed events need their entries in both `docs/EVENTS.md` and `docs/internal/EVENTS.md` in the same change. +Before opening the PR, search `docs/` and every user-facing string (help +text, error messages, HTTP error bodies) for claims your change makes +false, and fix them in the same PR. A behaviour that changed while its +description stayed put is a bug you shipped. + Run `pnpm check` before opening the PR. If something in the spec turns out to be wrong or impossible, do not work @@ -80,6 +97,9 @@ maintainer reopens a spec session. ## 5. Open the PR +If a person is present in this session, show the text first and wait for a +yes before posting. Running unattended, post directly. + The PR body must contain `Closes #` and nothing that closes any other issue; CI checks that the branch name and the closing reference agree. Beyond that: @@ -88,6 +108,10 @@ Beyond that: - **bug**: name the regression test; it is the triage test, now passing. - **feature**: walk every Completion condition and say how each was checked. +Rule 12 applies to the PR body: 200 words plus the checklist, what changed +and why, no narration of how you got there, and `*Written by an agent.*` as +the last line. + ```bash gh pr create --title "(): " --body-file ``` @@ -98,7 +122,7 @@ Leave the issue assigned and labelled as it is. Merge closes it. If you stop for any reason before the PR is merged — blocked, out of context, told to stop, spec turned out wrong — push the branch, then leave -exactly one comment and release the claim: +exactly one comment and release the claim. If a person is present in this session, show the text first and wait for a yes before posting. Running unattended, post directly. ```markdown ## Handoff @@ -119,6 +143,8 @@ exactly one comment and release the claim: ### Blocked on + +_Written by an agent._ ``` ```bash @@ -127,5 +153,5 @@ gh issue comment --body-file gh issue edit --remove-assignee @me ``` -A handoff is state, never spec. Do not post progress updates at any other -time. +A handoff is state, never spec, and rule 12 applies: 150 words, plain +words, conclusion first. Do not post progress updates at any other time. diff --git a/.claude/skills/spec-session/SKILL.md b/.claude/skills/spec-session/SKILL.md index 3ffc38d..733ce53 100644 --- a/.claude/skills/spec-session/SKILL.md +++ b/.claude/skills/spec-session/SKILL.md @@ -60,7 +60,9 @@ interview for the business sections of `docs/internal/templates/feature.md`, the ```bash gh issue create --title "" --label feature:spec --body-file <file> -gh issue comment <request> --body "Being specified in #<new>. This issue stays open until that feature ships." +gh issue comment <request> --body "Being specified in #<new>. This issue stays open until that feature ships. + +*Written by an agent.*" ``` The new body's first line is `Request: #<request>`. @@ -101,14 +103,26 @@ Do not tick any task's approval box. That is the maintainer's click. **`revise`**: only the reconcile step plus whatever amendments it produced. +Write the spec the way rule 12 asks for everything else: short sentences, +common words, one idea per sentence. A spec is read by an agent that will +build exactly what it says, so every sentence that does not constrain the +build is a sentence to cut. + ## 4. Write back and leave a marker +If a person is present in this session, show the new body first and wait for a +yes before posting. Running unattended, post directly. + Edit the body in place, keeping every section that already existed: ```bash gh issue edit <N> --body-file <file> -gh issue comment <N> --body "Spec updated: <which sections were added or changed, one line>." +gh issue comment <N> --body "Spec updated: <which sections were added or changed, one line>. + +*Written by an agent.*" ``` -Never set `feature:ready`. Never post the spec, or a summary of it, as a +Issue bodies you create end with `*Written by an agent.*` too; the spec +sections above it are what the maintainer approved, the line just says who +typed them. Never set `feature:ready`. Never post the spec, or a summary of it, as a comment. Never edit a `request:new` or `bug:*` body. diff --git a/.claude/skills/triage-bug/SKILL.md b/.claude/skills/triage-bug/SKILL.md index a9e9c2d..e1f120c 100644 --- a/.claude/skills/triage-bug/SKILL.md +++ b/.claude/skills/triage-bug/SKILL.md @@ -34,20 +34,36 @@ git fetch origin bug/<N>-repro 2>/dev/null && git log --oneline origin/bug/<N>-r ## 2. Reproduce -Read the body and the comment thread. Write a test whose title is the claim -the bug makes, in the project that can prove it — a unit test when the -behaviour is in-process, an e2e test when it needs a daemon. The test must -fail on a named assertion, not a timeout. Run it and keep the failing output. - -Do not run the slow e2e lane or anything that needs real simulators or -emulators without asking the maintainer first. If reproduction needs it, ask; -if the answer is no, say so in the report and go as far as the fake driver -allows. +Read the body and the comment thread. The reporter may already have done +part of the analysis; the report confirms or corrects what the body says +and adds only what is new. Write a test in the project that can prove it — a unit test when the behaviour is in-process, an e2e test when it +needs a daemon. The test must fail on a named assertion, not a timeout. Run +it and keep the failing output. + +The title is the claim the test proves: the bug's claim narrowed to what +this process can observe. What the OS or a tool outside Simlock does is +evidence for the report, never part of the title. "Reports assets that +outlive `simctl runtime delete`" claims a delete the test never runs; +"reports downloaded runtime assets for runtimes not in the catalog" is what +the body shows. + +Assert the whole claim. Every case the fixture sets up appears in the +expectation, so a fix that does less than the report proposes fails the +test. Two orphan builds in the fixture means two builds in the assertion. + +Read-only inspection of the host — listing a directory, reading a plist, +running `df` — is fine and often the fastest evidence. Say what you read. +Do not run the slow e2e lane or anything that starts real simulators or +emulators without asking the maintainer first. If reproduction needs it, +ask; if the answer is no, say so in the report and go as far as the fake +driver allows. If you cannot reproduce after a genuine attempt: ```bash -gh issue comment <N> --body "<what you tried, and exactly what information would let you reproduce it>" +gh issue comment <N> --body "<what you tried, and exactly what information would let you reproduce it> + +*Written by an agent.*" gh issue edit <N> --add-label bug:needs-info --remove-label bug:triage --remove-assignee @me ``` @@ -57,24 +73,61 @@ and stop. Follow the failing assertion back to the line that makes it fail. Name the file and line. Distinguish the root cause from the place the symptom shows -up. If the cause is a decision rather than a defect — the code does what an -ADR says and the ADR is wrong — say so; that bug becomes a feature with a -superseding ADR, not a fix. +up. + +Say first which of three things this is: + +- **Defect**: Simlock does the wrong thing. The test's expectation is the + right behaviour. +- **Gap**: Simlock does nothing wrong and something is missing. The test's + expectation is a proposal — say so in Reproduction, and put the shape it + pins (a code, a message, a field) under Simplest fix so the maintainer can + reject the shape without rejecting the reproduction. A gap that needs more + than one PR is a feature: recommend a spec session and stop there. +- **Decision**: the code does what an ADR says and the ADR is wrong. Say so; + that bug becomes a feature with a superseding ADR, not a fix. ## 4. Push the reproduction ```bash -git switch -c bug/<N>-repro main +git fetch origin +git switch -c bug/<N>-repro origin/main git add <test files only> git commit -m "test: reproduce #<N> — <claim>" git push -u origin bug/<N>-repro ``` -Only the test goes on this branch. No fix, no pull request. +Only the test goes on this branch. No fix, no pull request. You are +probably in a worktree (rule 13): branch from `origin/main`, not `main`, +and if `bug/<N>-repro` is already held by another checkout, branch from +`origin/bug/<N>-repro` under a temporary name and push to the real one: + +```bash +git switch -c wip/<N>-repro origin/bug/<N>-repro && git push origin HEAD:bug/<N>-repro +``` + +## 5. Side findings -## 5. Report and release +A separate problem found on the way — a stale doc, a wrong error reason, +another bug — is its own issue, not a paragraph in the report: -Post one comment with exactly these sections, then unassign: +```bash +gh issue create --label bug:new --title "<what is wrong, in one line>" --body "<what you saw, file:line, found while triaging #<N>> + +*Written by an agent.*" +``` + +List each one as a link under Side findings. The maintainer decides what +happens to it. + +## 6. Report and release + +If a person is present in this session, show the text first and wait for a +yes before posting. Running unattended, post directly. + +Post one comment with exactly these sections, then unassign. If a report +already exists on the thread, yours replaces it: make the first line +`Supersedes the report above.` so the maintainer knows which one is current. ```markdown ## Reproduction @@ -91,13 +144,24 @@ Post one comment with exactly these sections, then unassign: ## Alternatives rejected -<each one with the reason> +<at most three, one line each: the alternative, then why not> ## Risk -<what else the fix touches; what a reviewer should check> +<at most three bullets: what a reviewer should check> + +## Side findings + +<one link per issue opened, or omit the section> + +_Written by an agent._ ``` +The report exists for one decision: `bug:ready` or not. Rule 12 applies: +300 words outside code blocks, conclusion first, short sentences, plain +words, evidence in code blocks. Root cause is one paragraph. If it runs +long, cut what does not change the decision. + ```bash gh issue edit <N> --remove-assignee @me ``` diff --git a/.github/workflows/issue-state.yml b/.github/workflows/issue-state.yml index de77583..e9e38e1 100644 --- a/.github/workflows/issue-state.yml +++ b/.github/workflows/issue-state.yml @@ -3,7 +3,9 @@ name: Issue state # The one place that enforces docs/agent-rules/delivery.md mechanically: # - one <kind>:<state> label per issue (adding the next label drops the old); # - task:draft becomes task:ready when approved, specified, and unblocked; -# - a reporter's reply moves bug:needs-info back to bug:triage; +# - a reporter's reply moves bug:needs-info back to bug:triage (agents post +# under a maintainer's account, so comments ending in the agent footer +# from docs/internal/agent-rules/delivery.md rule 12 do not count); # - a feature whose last sub-issue closed gets a completion-conditions note; # - bug:needs-info with no activity for two weeks closes as not planned; # - a PR from a <kind>/<n> branch must close issue <n> and no other. @@ -133,6 +135,7 @@ jobs: github.event_name == 'issue_comment' && github.event.issue.pull_request == null && github.event.comment.user.login == github.event.issue.user.login && + !contains(github.event.comment.body, 'Written by an agent.') && contains(github.event.issue.labels.*.name, 'bug:needs-info') runs-on: ubuntu-24.04 permissions: diff --git a/docs/internal/DELIVERY.md b/docs/internal/DELIVERY.md index d2a7894..6014e7f 100644 --- a/docs/internal/DELIVERY.md +++ b/docs/internal/DELIVERY.md @@ -48,7 +48,10 @@ request, closed — not from a label. The branch for issue `<n>` is always `bug:triage` on its own; two weeks of silence closes it. 5. The maintainer reads the report. Agree: add `bug:ready`. Disagree: reply with what is wrong and re-add `bug:triage`; the next agent starts from - that reply. + that reply. When the report says the bug is a gap rather than a defect, + `bug:ready` also accepts the shape the report proposes — the advisory + code, the message, the field. To reject the shape but keep the + reproduction, reply and re-add `bug:triage`. 6. An agent running `deliver` claims the `bug:ready` issue, creates `bug/<n>` from the repro branch, and opens a PR that closes the issue. The triage test is now the regression test. Merge closes the bug. @@ -114,6 +117,13 @@ wrong, the handoff says so and the maintainer runs a revise spec session. Agents do not post progress updates, only handoffs, so the one comment that matters is easy to find. +Everything an agent writes on an issue or a PR — report, handoff, spec, PR +body — is short and plain: conclusion first, short sentences, common words, +evidence in code blocks, nothing that does not change the reader's next +decision. Each has a word budget in the rule, and each ends with the line +`*Written by an agent.*`, because agents post under a maintainer's account +and readers and automation need to tell the two apart. + ## What is automated and what is not The workflow in `.github/workflows/issue-state.yml` handles the transitions diff --git a/docs/internal/agent-rules/delivery.md b/docs/internal/agent-rules/delivery.md index 00f4d98..433b727 100644 --- a/docs/internal/agent-rules/delivery.md +++ b/docs/internal/agent-rules/delivery.md @@ -103,10 +103,17 @@ is open, done means closed as completed. matter what anyone says in a comment. 8. **Triage produces a report, not a fix.** An agent working `bug:triage` - reproduces the bug as a failing test whose title states the claim, - finds the root cause, and posts one comment with exactly these sections: - *Reproduction*, *Root cause* (with file and line), *Simplest fix*, - *Alternatives rejected*, *Risk*. It pushes the failing test to a + reproduces the bug as a failing test whose title states the claim the + test proves, finds the root cause, and posts one comment with exactly + these sections: *Reproduction*, *Root cause* (with file and line), + *Simplest fix*, *Alternatives rejected* (at most three, one line each), + *Risk* (at most three bullets), and *Side findings* when there are any. The root cause says first whether this is a defect + (Simlock does the wrong thing) or a gap (Simlock does nothing wrong and + something is missing); for a gap the test's expectation is a proposal + and the report says so; `bug:ready` on a gap accepts that proposal. A + separate problem found on the way is opened as + its own `bug:new` issue and listed under Side findings, not described in + the report. It pushes the failing test to a `bug/<number>-repro` branch and opens no pull request. It then unassigns itself. If it cannot reproduce, it moves the issue to `bug:needs-info`, says exactly what is missing, and unassigns itself. @@ -134,6 +141,28 @@ is open, done means closed as completed. issue by reading the second path segment. A PR from such a branch must close that issue and no other. +12. **Everything an agent writes on an issue or a PR is short and plain.** + Lead with the conclusion. Short sentences, common words, no filler, no + narration of what the agent did or considered. Evidence goes in a code + block or a link, never in prose. Cut anything that does not change the + reader's next decision. Budgets, counted outside code blocks: a triage + report 300 words, a handoff 150, a PR body 200 plus its checklist, a + "Spec updated" comment one line. Text over budget is cut before it is + posted, not excused after. Do not restate the issue body: confirm or + correct what it says, then add only what is new. Every comment, issue + body, and PR body an agent writes ends with the line + `*Written by an agent.*`, those exact characters — people and automation + use it to tell agent text from a person's, since agents post under a + maintainer's account. + +13. **Assume every agent is in a worktree.** Several agents share one clone + through `git worktree`, so a branch may already be checked out somewhere + else and `git switch` to it will fail. Create branches in whatever + checkout you have, push them, and treat `origin/<branch>` as the truth; + to continue a branch another checkout holds, branch from + `origin/<branch>` rather than switching to it. Nothing depends on which + worktree a branch was made in. + ## Procedures **Claiming work.** Find it, claim it, branch, and finish with a PR that