diff --git a/.claude/rules/tests.md b/.claude/rules/tests.md index 681557c689..2701f17264 100644 --- a/.claude/rules/tests.md +++ b/.claude/rules/tests.md @@ -27,6 +27,11 @@ This exists because inside a **git worktree** any `bunx` call creates a partial ## Test traps +- **Never start an executable the test just made and then rename or delete it.** macOS checks every new executable in + `syspolicyd` after it starts; a check that found the file gone crashed `syspolicyd` five times in ten minutes on + 2026-10-14 and froze the whole Mac (WindowServer watchdog). Make a test program once with `persistentFixture` from + `src/utils/test/persistent-fixture.ts` (kept, never renamed or deleted), and after the first run of such a test check + that `pgrep -x syspolicyd` still prints the same pid. - **Rewriting untested code: a characterization test first.** Pin the current behaviour, prove the test passes on the old code, then rewrite. When the test fails after the rewrite, assume the rewrite is wrong. Change a test only for an intended behaviour change, and say in the commit what changed and why. - **Red today, green yesterday, no relevant commit: check the calendar.** A dated fixture crosses a window when one input still resolves against the real clock: `--since=N days ago` for git, `find -mtime`, `date -v-7d`. Derive every cutoff from the injected `now`. - **`bun test --timeout` bounds a test body only.** A hang at module scope (an import that never settles) blocks forever. The signal is the wrapper's `[test] suite complete` line missing. Do not grep bun's own "Ran N tests across M files": it prints once per process, and `bun run test` runs two. diff --git a/.claude/skills/babysit/SKILL.md b/.claude/skills/babysit/SKILL.md index 552a85bc57..59441a3552 100644 --- a/.claude/skills/babysit/SKILL.md +++ b/.claude/skills/babysit/SKILL.md @@ -42,6 +42,20 @@ Write it to `/tmp/cc///babysit/run.json` at the start; the scr | `post` | ask | Post the round summary and replies after the push (review-loop step 4), or hold them in the vault. | | `report` | vault note | Optionally also a bus message: `{ "session": "grok-dev", "to": "grokbot-developer" }`. | | `stopAfter` | the user's words | A phase or round to stop at. Stop there even if everything is green. | +| `vault` | `/vault` | The notes folder of this run: reply files, summaries, the rendered ledger. Set it. | +| `judge` | `Claude` | The name the round summary credits as judge ("… from judged by "). | +| `worktrees` | `/worktrees` | Parent of the per-PR worker worktrees `bs-`. | + +**Codex model: Sol unless a finding needs Astra.** A large PR is not a hard PR: its findings are mostly Medium and Low +fixes that Sol handles. Use `gpt-6-astra` only when the user names it for a PR, when a Sol worker missed the same +finding twice after the judge's correction, or for a finding that crosses systems (process lifecycle, concurrency, +kernel or OS APIs, a stacked chain that must stay consistent). Steering the same session keeps its model, so decide +again at each round: an Astra session that finished its hard finding hands the next round to a Sol session. On +2026-10-14 the overnight run gave Astra every large PR by size: 47 of the 62 worker rounds, and $903 of the $924 +API-equivalent spend of the `bs-*` workers (an Astra worker cost $201 to $217 for about 20 turns; the Sol worker $21 +for 25 turns; `tools ai-spend workers --backend codex --by model,name`). + +Every script below takes `--run ` (or `BABYSIT_RUN`) and reads this card through `scripts/run.ts`. ## 2. Mode: a worktree per worker, or the unified branch @@ -76,15 +90,42 @@ the stop point. After the confirmation period, state the plan in one line and pr the hidden channels, `expand` every thread (the L1 table can scramble files). Keep each thread's `threadId`. 4. Read the source at every cited line before you brief: a stale or wrong finding is briefed as such, so the worker does not "fix" it. -5. **Round policy.** review-loop decides what gets fixed in each round (rounds 1 and 2: every valid finding; later - rounds: critical and high). When the user's own words ask for a fix the policy would only answer ("fix the open - threads"), the user's words win; say so in the plan. +5. **Round policy** (Martin, 2026-10-17 12:16; it replaces "fix every finding in every round", because eve opened + new Medium and Low findings on every push of #536 for 28 rounds). The executable rule is `scripts/policy.ts`; + `fill-brief.ts` derives it from the round, GitHub's changed-file count and `--dedicated-review`. Both brief + templates and judging use that generated policy. Count the PR's fix rounds; a follow-up PR starts at round 1. + - A PR with more than 100 changed files: rounds 1 to 3 fix every valid finding, Medium and Low included. From + round 4, fix only Critical and High. + - A PR with 100 changed files or fewer: rounds 1 and 2 fix every valid finding. From round 3, fix only Critical + and High. + - In the rounds that fix only Critical and High, a Medium or Low finding gets a reply (valid or not, and why). + It is not fixed in that PR, and its thread stays OPEN: resolve only a thread that was acted on (fixed, or + rejected with a cited file:line). Record a policy deferral with the reply transport's supported + `"verdict": "belongs-elsewhere"` and explain the policy in the reply; do not add unsupported schema fields. `integrate.ts` keeps it in the summary but partitions it into a separate + `post-replies.ts --no-resolve` batch, never the batch that resolves threads (Martin, 2026-10-17 12:45). + - **A valid security finding is always fixed**, at any severity and in any round (Martin, 2026-10-17 12:45: + "security valid always"). Read the reviewer's category (`🔒 Security`), not only its severity. + - **A dedicated code review the user asks for** (an independent worker review such as `review-brief.ts` on Astra + or Sol) is fixed in full: every valid finding, Medium and Low included, whatever round the PR is in. Fix it + yourself or steer the PR's fixing worker, then judge it as any round (Martin, 2026-10-17 13:05). + - Inside the fix-everything rounds, a deferral is not accepted ("nothing is a follow up or out of scope", Martin, + 2026-10-17), and every file a valid fix needs is in scope. A `no-change` or rejection needs a cited file:line + that proves the finding wrong. + - **Merge bar:** when the last round's Critical and High findings are fixed and judged, merge without waiting + for eve to review that last fix. The PR still needs at least one CodeRabbit review, green CI and a reply on + every thread. 6. Scope: `git fetch origin `, then `bun .claude/skills/babysit/scripts/pr-scope.ts --compare HEAD`. It lists the PR's files, the test file of each one (a sibling `.test.ts`, or a neighbour test that imports it), - and whether unified differs from the PR branch per file. That list is the outer bound. The brief allows the files - the findings cite plus their tests, and any other PR file a fix provably needs. Missing a test file in the scope + and whether unified differs from the PR branch per file. That list is the starting context, not a scope limit. + The brief allows every file a valid fix needs, including files outside the original PR diff; explain those files + in the reply. Missing a test file in the scope put Grok's tests in the wrong file on 2026-10-14. -7. Baseline the cost: `tools grok usage --json --fresh` (the SuperGrok weekly percentage) for Grok workers. Every +7. **Worktree** (worktree mode): `bun .claude/skills/babysit/scripts/setup-wt.ts --run --pr `. Detached at + the PR head, `bun install --ignore-scripts` plus the repo's patches, the test wrapper's install stamp (else + `scripts/test.ts` reinstalls everything inside the worker's sandbox), native files linked to the main checkout's + twins (`link-native.ts`: on 2026-10-14 a stuck syspolicyd hung every NEW native binary at `_dyld_start`), nested + app `node_modules` linked. About 7 s with a warm bun cache. +8. Baseline the cost: `tools grok usage --json --fresh` (the SuperGrok weekly percentage) for Grok workers. Every worker turn is booked in the cost ledger with its ai-spend price, login and label, and its report (Codex: the `turn_completed` bus event and `status --json` `cost`) ends with one line: API-equivalent, billed, and a Claude Opus/Sonnet estimate for the same tokens. The API-credit @@ -94,7 +135,17 @@ the stop point. After the confirmation period, state the plan in one line and pr ## 4. Brief and dispatch -Fill [references/brief-template.md](references/brief-template.md) per PR (the findings pasted in full, the scope, +A Codex worker's sandbox has no network, so `tools github review` and `gh` fail inside it (2026-10-16: two round 2 +fixers stopped before their first edit). Before every spawn or steer of a round, run +`bun .claude/skills/babysit/scripts/fetch-threads.ts --run --pr --round `: it writes the threads (listed +and expanded, with Thread IDs), the review bodies and the bot PR comments into the worktree, and the brief points there. + +Two brief forms. **Self-fetch** (the default since 2026-10-14): +`bun .claude/skills/babysit/scripts/fill-brief.ts --run --pr --round --model "