diff --git a/.claude/skills/gemini-task/SKILL.md b/.claude/skills/gemini-task/SKILL.md index cc2b5d55c41..d61b90f1d54 100644 --- a/.claude/skills/gemini-task/SKILL.md +++ b/.claude/skills/gemini-task/SKILL.md @@ -88,6 +88,18 @@ covers the whole task. and a sample in its test, because an autouse fixture in `tests/unit/agents/data/conftest.py` fails every test that emits an unknown warning. A "change only" list without them contradicts the gates. +- For tests, Constraints name the files where the behavior lives and allow + test files next to them, and Acceptance carries a mutation table: per + behavior, one deliberate break in the real code (file and change) that must + turn a test red, plus one harmless refactor that must stay green. Gemini + runs the table on a clean tree after committing the step (section 2, step + 6), so `git restore ` drops only the break: it applies each row, runs + the tests, restores, and reports the result per row. Green gates do not show what a test would + catch: on 2026-10-30 the section-tests briefs allowed new files only under + `app/src/sections/`, while the state those sections show lives in + `app/src/pages/*.tsx`. Gemini copied the page logic into test harnesses, + every gate passed, and breaks in the real pages went unnoticed until the + review applied them. - Write each Acceptance check as one plain command (`yarn test`, a `grep` that must print nothing, `wc -l ` with the limit in words), never a chain with `( … )`, `$( … )`, or `|| true`: agy asks the owner for every chain. @@ -136,8 +148,8 @@ well, and the repository rules that matter most here. ## Acceptance The exact commands that must pass, the convention checks that must find -nothing, the cases where the existing rules must still win, and the behavior to -check by hand, if any. +nothing, the cases where the existing rules must still win, for tests the +mutation table, and the behavior to check by hand, if any. ## Decide yourself / report back The routine choices Gemini makes and documents, and the questions it returns @@ -304,6 +316,15 @@ This section is for the Gemini agent in the owner's agy session. changelog fragment for the task, push, and open one draft PR (step 7) whose `## Plan` lists each package with its merge commit. A package that failed stays out; name it under open steps. + - **A review round** is an owner's comment on the draft PR that lists + packages the way briefs do (section 3, step 4). Work it as above, with + these differences: round ``'s packages branch from + `origin/gemini/` as `gemini/-r-` and merge back into + `gemini/` the same way; path checks diff against + `origin/gemini/` instead of `origin/main`; and the fragment and the + draft PR exist already, so update the fragment only if the round changes + what it says. After the push, comment on the PR with each package's merge + commit and the result of every mutation-table row. - Do not use `/teamwork` for this repository: it builds in its own folder (`~/teamwork_projects/`) without branches or PRs. 2. **Work in the dedicated clone on your own branch.** That clone is @@ -476,6 +497,11 @@ This section is for the Gemini agent in the owner's agy session. - Add Fable for high-risk diffs: security, production data, sandboxing, privacy. - Gemini never reviews its own work. + - **New tests need their mutation table run** by the subagent: apply each + row to the real code, check that a test turns red, and revert; add a + break or two of its own in the logic the tests claim to cover. On + 2026-10-30 this found copied page logic in two of three packages that had + passed every gate. - **A refactor that must keep behaviour identical needs proof**, and green CI is not proof: on 2026-10-29 a Jules split of DebugPage passed CI and a browser comparison and still changed visible text. Run the checks yourself, @@ -512,9 +538,16 @@ This section is for the Gemini agent in the owner's agy session. 4. **Fix or send back.** Fix small defects yourself in the worktree and push them to the branch; Gemini pulls them before it continues. Larger ones go - back as a PR comment (`gh pr comment`), and the owner tells Gemini to - continue. Bring in a newer `origin/main` by merging it into the branch, never - by rebasing a pushed branch. + back as a review round: one PR comment (`gh pr comment --body-file`) that + reads like a set of briefs, with one section per package naming its files, + the fixes, its mutation table, its path check, and whether it is + `independent` or `coupled`. The owner starts Gemini with + `/gemini-task ` and tells it to work the comment; Gemini runs the + packages from the task branch and comments with the results (section 2, + step 1). Review the round as in step 3, per merge commit. Keep the PR a + draft until the last round is in, because marking it ready starts its one + Copilot review. Bring in a newer `origin/main` by merging it into the + branch, never by rebasing a pushed branch. 5. **Ship.** Run the open-pr gates, add the PR number to the changelog fragment, and add one Summary bullet to the PR body, which Gemini wrote in the repository's format (section 2, step 7): the change was implemented by @@ -529,11 +562,17 @@ This section is for the Gemini agent in the owner's agy session. Delete the brief (`rm -f ~/projects/anyplot-briefs/.md`, or for a task in packages `find ~/projects/anyplot-briefs -maxdepth 1 -regex '.*/-[0-9]+\.md' -delete`); - the merged PR keeps its copy. Delete - the package branches the merge left behind: - `git push origin --delete gemini/-1 gemini/-2 …`. The branch in - the owner's clone - (`~/projects/anyplot-agy`) is his to delete. + the merged PR keeps its copy. The package branches the merge left behind, + review rounds' `gemini/-r-` included, go only with the owner's + OK, because only the PR's own worktree and local branch go without asking + (`CLAUDE.md`). Run `git fetch --prune origin` first, so no stale ref hides + a newer commit, then check that each is part of the merged head + (`git merge-base --is-ancestor origin/ `) and ask + with the branch names in the question: + `git push origin --delete gemini/-1 gemini/-2 …`. On + 2026-10-30 a general OK to the cleanup plan did not get past the permission + classifier; if it refuses, give the owner the command to run with `!`. The + branch in the owner's clone (`~/projects/anyplot-agy`) is his to delete. 7. **Record the task in the ledger.** ## Ledger diff --git a/changelog.d/gemini-task-test-lessons.md b/changelog.d/gemini-task-test-lessons.md new file mode 100644 index 00000000000..9ab355fe823 --- /dev/null +++ b/changelog.d/gemini-task-test-lessons.md @@ -0,0 +1,13 @@ +### Changed + +- **Gemini test tasks prove what their tests catch.** In the `gemini-task` + skill, a test brief names the files where the behavior lives and carries a + mutation table in Acceptance: one deliberate break per behavior that must + turn a test red, and one harmless refactor that must stay green. The review + subagent runs the table and adds breaks of its own. Larger findings go back + as a review round, one PR comment with a section per package; Gemini works + the independent ones in parallel on `gemini/-r-` branches from + the task branch and merges them back. In the first parallel run, the section tests passed every + gate while two of three packages tested copies of the page logic instead of + the pages. Cleanup now asks before deleting package branches, as + `CLAUDE.md` requires. (#12474)