Repository navigation
Add agent skills for recurring repo workflows - #1486
cristianrgreco wants to merge 7 commits into
Conversation
Move step-by-step workflows out of AGENTS.md into skills under .agents/skills (read by Codex), symlinked into .claude/skills (read by Claude Code): open-pr, review-pr, add-module, triage-issue, diagnose-ci, update-dependencies and publish-release. AGENTS.md keeps the rules that apply to every task, adds verification, image pinning and CommonJS dependency rules, and fixes the Vitest sequential guidance for Vitest 5. The bug report template now asks for DEBUG logs, the container runtime, the test runner and the last working version.
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@claude Push access is fixed (#1493). Please apply the pending changes from these review threads in one run and push. Where threads overlap, follow the order below.
|
|
Claude finished @cristianrgreco's task in 4m 4s —— View job Applied and pushed the review changes in
|
- Replace publish-release with a read-only review-release skill; agents never publish - Add a Releases section to AGENTS.md - open-pr: ask for a maintainer dry run instead of dispatching it, add a commit and push step with the signing and force-push rules, drop the --body-file note - triage-issue: replace the hardcoded failing-phase and known-causes tables with searching the source, issues, PRs and docs - update-dependencies and diagnose-ci: point at the signing rules and open-pr Co-authored-by: Cristian Greco <10348798+cristianrgreco@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add the .claude/skills symlink the review run could not create, and align two leftover triage-issue phrases with the removed known-causes table.
|
|
||
| ## Docs and finish | ||
|
|
||
| - Adapt the mosquitto docs page. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line. |
There was a problem hiding this comment.
@claude Not sure about pointing to a specific module. Why was it chosen over for example including an inline template within the skill?
There was a problem hiding this comment.
Claude finished @cristianrgreco's task in 2m 6s —— View job
It points at a real module so the boilerplate stays current, but it doesn't need to name one
- Read
add-moduleandpackages/modules/mosquitto - Compare the boilerplate across all 43 modules
- Weigh pointing at a module against an inline template
- Answer with a proposed change. Nothing is committed, because you asked a question.
Why it points at a real module
Line 9 sums this up as "Copying keeps the boilerplate current". In more detail:
- The release rewrites part of the boilerplate.
npm-publish.yml:55-59bumpsversionin every workspace and setsdependencies.testcontainersto^<version>in every module. Copy a real module and both values are already correct. In an inline template they would say12.2.0until someone updated the skill, or they'd be placeholders the agent has to look up anyway. - CI checks a real module, but nothing checks a template. A module has to build, lint and pass its tests on every CI run, so its files can't drift away from something that works. A template under
.agents/is never built or run, so nothing would flag it when it falls behind. - Length, the weakest of the three. A full template would add about 150 lines:
package.json, two tsconfigs,Dockerfile,index.ts, the container class, a test and a docs page. That's still under the 500-line limit in Claude's skill guidance. But it would make a 40-line skill mostly boilerplate that the agent can copy from disk anyway.
Where you're right
- The boilerplate doesn't depend on which module you copy.
tsconfig.json: 42 of 43 are byte-identical. Opensearch differs only in formatting.tsconfig.build.json: 36 of 43 are identical. The rest only add an extra test-only exclude.- Docs pages: 42 have the same "substitute
IMAGE" line.
- A single example passes on its quirks. Mosquitto writes a config file in
start()and changes the entrypoint to/bin/shso it can runmosquitto_passwd. An agent copying it could bring that pattern into a module that doesn't need it. - The pointer can go stale without anyone noticing. "Recent" won't stay true. If mosquitto changes shape, every new module still starts from it.
Proposed change
Don't name a module, and don't add a template. For the files that are identical everywhere, point at any module. For the parts that differ, point at the closest one.
Line 9:
Copy `package.json`, `tsconfig.json`, `tsconfig.build.json`, `Dockerfile` and `src/index.ts` from any existing module; they're the same in every module. In `package.json`, change the name, description, keywords and `devDependencies`, and leave `version` and the `testcontainers` dependency as copied, because the release workflow sets both. For the container class, tests and docs page, start from the module closest in shape to the new one (same kind of wait, auth or number of ports). The rules below are what reviewers keep flagging on module PRs.Line 36:
-- Adapt the mosquitto docs page. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.
+- Copy the docs page of the module you started from. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.These edits don't touch line 13, which the other open thread on this file is changing.
Other options I considered
- Template files in the skill folder (
.agents/skills/add-module/template/). This keepsSKILL.mdshort, but the files go stale just like an inline template: nothing builds or tests them, and the version fields still need placeholders. - A generator script with a CI test. This is the only kind of template that can't go stale. testcontainers-go has one,
modulegen, which generates a module from templates and is tested in CI. I'm going from memory here, because this run can't open external sites, so check it before citing it. It would be real code to maintain, so I'd do it in a separate PR if you want it.
Reply "go ahead" and I'll push the two edits above to this branch.
· Branch claude/repo-skills-audit-83c819
There was a problem hiding this comment.
Claude finished @cristianrgreco's task in 1m 54s —— View job
Made the add-module edits, but the push was rejected because another push landed first
- Find the thread for this "Go ahead":
add-moduleline 36, which asked why the skill points at mosquitto - Re-check the "same in every module" claims
- Edit line 9 and line 36
- Push: committed locally as
d42a4c2, but the push was rejected
Why the push failed
! [rejected] HEAD -> claude/repo-skills-audit-83c819 (fetch first)
This isn't the old 403. Something was pushed to the branch at 18:15, after this run checked it out. That was probably another @claude run on one of the other threads. This run can't run git fetch or git pull, so it can't rebase onto that push. I didn't force-push.
To land the change, reply "@claude try the push again" on this thread. A new run checks out the latest branch. Or apply the diff below yourself. It only touches lines 9 and 36, so it shouldn't conflict with the line 13 or lines 38–40 threads.
The change
--- a/.agents/skills/add-module/SKILL.md
+++ b/.agents/skills/add-module/SKILL.md
@@ -6,7 +6,9 @@ argument-hint: "[module name]"
# Add a module
-Start by copying a small, recent module (`packages/modules/mosquitto` and `docs/modules/mosquitto.md`) and adapting it. Copying keeps the boilerplate current. The rules below are what reviewers keep flagging on module PRs.
+Copy `package.json`, `tsconfig.json` and `tsconfig.build.json` from any existing module. They have the same shape in every module. In `package.json`, change the name, description, keywords and `devDependencies`. Leave `version` and the `testcontainers` dependency as copied, because the release workflow sets both.
+
+For the container class, tests and docs page, start from the module closest in shape to the new one (same kind of wait, auth or number of ports). The rules below are what reviewers keep flagging on module PRs.
## Before writing code
@@ -33,7 +35,7 @@
## Docs and finish
-- Adapt the mosquitto docs page. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.
+- Copy the docs page of the module you started from. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.
- Add the page to the `mkdocs.yml` Modules nav in alphabetical order.Where this differs from what I proposed
- I dropped
Dockerfileandsrc/index.tsfrom the copy list. The proposal said they were "the same in every module", and they aren't. TheDockerfileis the module's own image, which line 14 andAGENTS.mdalready cover.src/index.tsexports the module's own classes. - "Same shape" instead of "the same".
package.jsonreally does differ (name, description, keywords,devDependencies), and the sentence after it lists those fields. I re-checked the tsconfig files on this branch:tsconfig.jsonis byte-identical in 42 of 43 modules, andtsconfig.build.jsonin 36 of 43. - Two paragraphs instead of one. The skill goes from 40 to 42 lines, still inside the 30–66 range in the PR body.
- I checked the version claim against
npm-publish.yml:55-59. The release workflow sets every workspace'sversion, and each module'sdependencies.testcontainers, to^<version>.
No other skill and nothing in AGENTS.md points at mosquitto as a template. The only other mention is the example title Add Mosquitto module in open-pr. The PR description doesn't mention it either, so it doesn't need updating.
npm isn't available in the Action, so format and lint haven't been run. The change is Markdown only.
· Branch claude/repo-skills-audit-83c819
There was a problem hiding this comment.
Claude finished @cristianrgreco's task in 1m 3s —— View job
Pushed in 580c9cf: add-module no longer points at mosquitto as the template
I took "Try again" to mean a retry of the push that was rejected on the line 36 thread (d42a4c2). That was the only approved edit on this PR that hadn't reached the branch. If you meant a different thread, tell me which one.
- Check the branch head and the current
add-modulefile. The head was1d7567f, andd42a4c2wasn't on the branch. - Apply the line 9 and line 36 edits on top of
1d7567f - Re-check the claims the edits make
- Commit and push
The change
This is the same diff as d42a4c2, applied on top of 1d7567f. It doesn't touch line 13, which 1d7567f changed.
-Start by copying a small, recent module (`packages/modules/mosquitto` and `docs/modules/mosquitto.md`) and adapting it. Copying keeps the boilerplate current. The rules below are what reviewers keep flagging on module PRs.
+Copy `package.json`, `tsconfig.json` and `tsconfig.build.json` from any existing module. They have the same shape in every module. In `package.json`, change the name, description, keywords and `devDependencies`. Leave `version` and the `testcontainers` dependency as copied, because the release workflow sets both.
+
+For the container class, tests and docs page, start from the module closest in shape to the new one (same kind of wait, auth or number of ports). The rules below are what reviewers keep flagging on module PRs.-- Adapt the mosquitto docs page. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.
+- Copy the docs page of the module you started from. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line.Claims re-checked on this branch
tsconfig.json: identical in 42 of 43 modules.opensearchis the exception.tsconfig.build.json: identical in 36 of 43 modules. The other seven add test-only excludes, which AGENTS.md already requires.- The release workflow sets
versionand thetestcontainersdependency:npm-publish.yml:55-59runsnpm version --wsand setsdependencies.testcontainers=^$VERSIONin each module.
add-module is now 42 lines, inside the 30–66 range in the PR body. Nothing else in the skills or AGENTS.md uses mosquitto as a template. The only other mention is the example title in open-pr.
Still open
These threads got a proposal but no "go ahead", so I haven't changed them:
add-modulelines 38–40: addcheck-compilesand merge the lines into one hand-off toopen-pr.diagnose-ciline 32: replace the pattern table, plus theupdate-dependencies:13tweak.
Checks
npm isn't available in the Action, so I couldn't run format or lint. The change is Markdown only. As in earlier runs, the .claude/skills symlinks show as deleted in this checkout. I committed only .agents/skills/add-module/SKILL.md, so the symlinks on the branch are unchanged.
· Branch claude/repo-skills-audit-83c819
|
|
||
| ## Docs and finish | ||
|
|
||
| - Adapt the mosquitto docs page. Examples come only from test blocks via `codeinclude`. Keep the "substitute `IMAGE`" line. |
…d Go Co-authored-by: Cristian Greco <10348798+cristianrgreco@users.noreply.github.com>
Co-authored-by: Cristian Greco <10348798+cristianrgreco@users.noreply.github.com>
Summary
Moves the step-by-step workflows out of
AGENTS.mdinto seven repository skills. They live in.agents/skills/, where Codex reads them, and.claude/skills/symlinks each one so Claude Code reads them too:open-prreview-pradd-moduletriage-issuediagnose-ciupdate-dependenciesreview-releaseThe skill content comes from:
triage-issuedeliberately has no list of known causes. Such lists go stale, as the Bun/Ryuk workaround already had, so the skill sends the agent to the source, past issues and PRs, anddocs/instead.AGENTS.mdnow holds only the rules that apply to every task:open-prandreview-pr.open-prandupdate-dependenciesrepeat the signing and force-push rules where they commit.npm-publish.yml(including its dry run), and never edit releases. A maintainer releases by publishing the draft.open-prasks the author to flag changes that need a maintainer dry run.FROMlines for the same image..sequentialmodifier, so it now says{ concurrent: false }.The bug report template now asks for:
DEBUG=testcontainers*logs, and how to collect themVerification
Skill format. Checked against Claude's skill authoring best practices:
SKILL.mdfrontmatter parses with js-yaml, each name matches its folder, and each has a.claude/skillssymlink that resolves.Symlinks. Claude Code's skills docs say a
.claude/skills/<name>entry can be a symlink to a directory elsewhere. The symlinks are committed as git symlinks (mode120000). Codex reads.agents/skillsdirectly.Test runs. Fresh subagents ran two skills, read-only, using the first version of each. The skills were then tightened from what they reported.
triage-issuewas reworked again in review, when its known-causes tables were replaced with searching for an existing answer.review-pron PR 1447 independently raised the points from the maintainer's own review:restart()path is untouchedIt also flagged:
triage-issueon issue 1442 reproduced the leak with testcontainers-python 4.15.0 alongside a Node suite. It traced the cause to the Python binding force-removing its Ryuk, which corrects the session-id explanation in the report. It also checked a one-linelang=nodefilter red-green inreaper.test.ts, then reverted it.CI.
changed-modules.mjsselects no packages for these paths, so Checks runs no package jobs.Not yet verified: that a fresh Claude Code session lists the skills. The session that wrote them started before
.claude/skillsexisted, and/reload-skillsdidn't pick them up.Not breaking
This only touches docs, agent configuration and the issue template. No package source, manifest or lockfile changed.