Fix review findings in the agent instructions - #1454
Conversation
The adoption instructions ran `npx skills` with no version and installed from a repo's default branch. Both move on their own, so a skill could change the instructions an agent follows here after someone had read and approved it, and the installer itself could change underneath that. Pin the installer and require a reviewed tag or commit for the skill, recorded in the provenance table so an update means re-reading rather than bumping a number.
The skill claimed to run the same checks as CI while leaving out both PyInstaller builds, treating the agent hook tests as conditional, and listing cargo test without saying no workflow runs it. Someone following it could pass every gate locally and still fail the build. Add the frozen builds to this skill and to the backend endpoint playbook, make the hook tests unconditional, and say plainly that cargo test is extra rather than a gate. Pin markdownlint to the version the CI action uses, since bare npx currently resolves to a newer release than CI runs.
The area AGENTS.md files gave commands with no working directory while the root file says everything runs from the repository root, so the backend one only worked from inside backend/ and the others quietly left you in the wrong directory. Wrap them the way the root file does. Also: Codex does support hooks, through hooks.json or an inline [hooks] table, so say this repository configures none rather than that the feature does not exist. And the hook test named the workflow the Linting job lived in before it moved to lint.yml.
|
|
|
Warning Review limit reached
Next review available in: 6 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Addresses the 7 review comments left on #1453. They were all about the agent
instruction files from #1408, which is already merged, so they are fixed here against
main instead of in the album PR.
What changed:
npx skills addran unpinned and installed from a repo's default branch. Both move ontheir own, so a skill could change the instructions an agent follows here after someone
had approved it. The installer is pinned now and the skill needs a reviewed tag or
commit, recorded in the provenance table.
treated the agent hook tests as optional, and listed
cargo testwithout saying noworkflow runs it. You could pass everything locally and still fail the build. The backend
endpoint playbook was missing the same builds.
markdownlint-cli2is pinned to 0.22.1, the version CI's action uses. Barenpxresolvesto 0.23.2 today, so the documented local command was checking against a different release
than the one deciding the build.
everything runs from the repository root. The backend one only worked from inside
backend/. All wrapped now.hooks.jsonor an inline[hooks]table, so the note nowsays this repository configures none rather than that the feature does not exist.
lint.yml.Docs only, plus one comment line in
scripts/agent-format-hook.test.mjs. Hook tests stillpass 53/53, and markdownlint at the pinned version reports nothing in committed files.
One nitpick from that review is not taken: moving the
Agent hook testsstep belowInstall frontend dependenciesinlint.yml. The test only asserts excluded paths are leftalone, which needs no formatter, and running it before
npm ciis deliberate so it failsfast.