docs(agents): consolidate AGENTS.md guidance into skills and rules#4355
docs(agents): consolidate AGENTS.md guidance into skills and rules#4355pandemicsyn wants to merge 5 commits into
Conversation
Reduce always-loaded agent context by moving conditionally-relevant and duplicated guidance out of AGENTS.md files into the mechanisms that load on demand or by reference. - Move duplicated Cloudflare Worker/DO conventions (file naming, DO stub helpers, sub-module splitting, IO boundaries, SQL, HTTP routes, withDORetry) into a shared durable-objects skill reference; trim gastown/wasteland/ mcp-gateway/cloud-agent-next AGENTS.md to pointers + genuine deltas. - Move the module-scope DB-client-caching rule into workers-best-practices skill; drop the Workers & Durable Objects section from root AGENTS.md. - Consolidate coding standards into .kilo/rules/coding-style.md (reconciling the `as`-operator rule to the nuanced version); remove restatements from root, extension, mobile, and kiloclaw AGENTS.md. Fix stale .kilocode path. - Dedup GDPR/PII rule (kept in .kilo/rules/gdpr-pii.md). - Move PR conventions into a /cloud-pr command. - Move the spec index into a specs skill (rebuilt from disk, all specs except template.md); add per-service spec pointers to kiloclaw, kiloclaw-billing, and mcp-gateway. - Delete the CI-enforced Markdown Tables prose. - Rescope the Domain Context section to state CONTEXT.md covers only the Code Reviewer and Security Agent domains.
…self-contained Follow-up to the consolidation: the shared durable-objects repo-conventions doc had over-generalized Gastown/Wasteland-specific patterns. - Trim repo-conventions.md to genuinely shared Worker/DO conventions (DO call retries, DO stub helper, sub-modules, IO boundaries, DB clients); drop the SQL/table and HTTP-route sections that are not universal. - Revert services/gastown/AGENTS.md and services/wasteland/AGENTS.md to their original main content (they keep their own raw-query() SQL conventions rather than pointing at a shared doc). - mcp-gateway: HTTP routes section lists only its own route surface. - Update durable-objects SKILL.md description to match repo-conventions.md.
| `CONTEXT.md` is the domain-language and ownership contract for the **Code Reviewer** and **Security Agent** domains only (review execution/analytics, Security Findings, Security Sync, notifications, remediation, and their email delivery — mainly `apps/web/src/lib/{code-reviews,security-agent}/`, `services/security-sync/`). It does not cover other areas of the monorepo. | ||
|
|
||
| When working in those domains, read `CONTEXT.md` first and use its canonical terms in code, docs, task descriptions, tests, and agent outputs. Do not introduce synonyms for its concepts without updating `CONTEXT.md` first, and do not duplicate the full contract inside `AGENTS.md`. |
There was a problem hiding this comment.
Previously its seems like we instructed the agent to load this all the time. But this doc actually seems like it only applies to Code/Security reviewer. I'm guessing because the agent confuses the terminology used by the two?
This might work better as a skill or something? But hopefully this cuts down on eager loading.
There was a problem hiding this comment.
This is an artifact of the /grill-with-docs skill.
It think it has long-term value, but it needs to be back-filled with context about the rest of the repo.
We might also want to consider whether we want this to live separately in each app/service/package and maybe just have a context map at the root of the repo?
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| routes in v1. | ||
|
|
||
| ## File naming | ||
| ## Specs |
There was a problem hiding this comment.
WARNING: Conventions dropped for this service with no replacement
This removes mcp-gateway's own "File naming" section, and (further down in this same file's diff) the "table interpolator objects / no raw table or column strings" SQL-safety rule and the "gateway is stricter than Gastown" IO-boundary detail — on the assumption these are now covered by the shared durable-objects skill (references/repo-conventions.md). However, this PR's later commit also removes the "File naming", "Column naming", "SQL queries", and "HTTP routes" sections from repo-conventions.md. services/gastown/AGENTS.md and services/wasteland/AGENTS.md are unaffected by this PR (net-zero diff) and still carry those conventions verbatim, but mcp-gateway now has none of them documented anywhere — including the no-table-aliasing/no-raw-string SQL-safety rules and the no-Hono-sub-app-mounting HTTP rule.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| - Never use `as` to cast IO data. If the shape is known, define a Zod schema; if | ||
| not, use `.passthrough()` or a catch-all schema. | ||
|
|
||
| ## DB clients |
There was a problem hiding this comment.
WARNING: File naming/Column naming/SQL queries/HTTP routes sections removed with no home
This deletes the "File naming", "Column naming", "SQL queries" (table-interpolator, no-table-aliasing, Zod row-parsing rules), and "HTTP routes" (no Hono sub-app mounting) sections that an earlier commit in this same PR pointed to as the shared home for these conventions (see this file's own earlier wording and services/mcp-gateway/AGENTS.md's removed "File naming" section, which now assumes this file covers it). services/gastown/AGENTS.md and services/wasteland/AGENTS.md are untouched by this PR and still carry these conventions directly, but services/mcp-gateway/AGENTS.md removed its own copies expecting this file to retain them — leaving those rules (notably the SQL table-aliasing/raw-string guardrails) undocumented for mcp-gateway.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe consolidation drops several DO/service conventions (file naming, SQL query safety rules, HTTP-routing rules) from both the shared Overview
Issue Details (click to expand)WARNING
Files Reviewed (14 files)
Note: Incremental update: since the previous review (commit |
| @@ -0,0 +1,84 @@ | |||
| # Repo Conventions — Worker/DO Services in This Monorepo | |||
There was a problem hiding this comment.
Comment/question, not review feedback:
Something I ran into in the on-call repo was the split between where we put custom-authored skills and 3rd party skills, and does it even matter. For instance we tend to use npx skills as our "skill package manager" for 3rd party skills and install that into .agents/skills as its mostly supported across all agents.
In the on-call repo we had a mixture of custom and 3rd party skills and ended up with:
- .agents/skills → 3rd party skills
- .kilo/skills → custom skills
It feels duplicative though to have skills in two places and maybe we should just put it all in .agents/skills like it seems we're going for in this repo.
No strong opinions, just think it would be good to be consistent across all repos so we don't have to think about it.
There was a problem hiding this comment.
i actually struggled with this more broadly as well - because i also have the actual official cloudflare skill + cloudflare mcp set up globally.
I'm down for copying the setup in the on-call repo.
| - `./references/rules.md` - Core rules, storage, concurrency, RPC, alarms | ||
| - `./references/testing.md` - Vitest setup, unit/integration tests, alarm testing | ||
| - `./references/workers.md` - Workers handlers, types, wrangler config, observability | ||
| - `./references/repo-conventions.md` - This repo's Worker/DO conventions: DO call |
There was a problem hiding this comment.
Ok this is actual review feedback and here's where we should make a decision on how we want to handle 3rd party vs custom skills.
If we want to use npx skills as our "skill package manager" to easily be able to update skills to their latest version, then I think we should withhold from making any changes to those skills like we are doing here.
An alternative: No 3rd party skills in repo by default
- We could have a recommended list of skills that might be useful but require the user to install it separately.
- This puts extra burden on the developer to make sure they have the right "tools" in place when working on the repo, but one could argue that's how we've done things in the past with which IDE you use, etc.
@marius-kilocode might have some thoughts on this as I believe he favors a more bare repo with limited agent pre-configuration to allow for more flexibility in personal development workflows and control.
There was a problem hiding this comment.
Mentioned this above as well but:
i actually struggled with this more broadly as well - because i also have the actual official cloudflare skill + cloudflare mcp set up globally.
I'd be up for this. I already have my own review agents, pr skills, etc. So this doesn't feel un-natural.
Some of these custom additions for Cloudflare specifically could also go in REVIEWS.md. e.g. insisting on withDORetry
| @@ -0,0 +1,30 @@ | |||
| --- | |||
There was a problem hiding this comment.
I think we might be good to delete this command. I imagine most folks have their own workflow for this and the repo template serves as a guide for external contributors.
RSO
left a comment
RSO
left a comment
There was a problem hiding this comment.
I think this is an improvement. I do want to second some of @jeanduplessis's comments regarding the wording of the specs. If we tell the bot that the specs are the authoritative source of truth, it will be reluctant to suggest a change when my prompt conflicts with a spec. Usually, when this happens, the bot will twist itself into all kinds of weird corners to somehow figure out how to comply with my prompt while keeping the spec as is.
Personally, I'd much rather have the bot just make a change to the spec file when this happens, because that makes it very clear to me that what I'm asking conflicts with something we decided earlier. It also fits much more with our iterative approach to software development, as opposed to the waterfall-y approach of treating specs as set-in-stone.
Co-authored-by: Jean du Plessis <jeandp@gmail.com>
Co-authored-by: Jean du Plessis <jeandp@gmail.com>
Co-authored-by: Jean du Plessis <jeandp@gmail.com>
3 participants
Summary
Clean up
Key moves:
durable-objectsskill (references/repo-conventions.md): DO call retries (withDORetry), DO stub helpers, sub-module splitting, IO boundaries, DB clients. The module-scope DB-client-caching rule moved intoworkers-best-practices..kilo/rules/coding-style.md(reconciling theas-operator rule to the nuanced version); per-package restatements removed. Stale.kilocode/rules/...path fixed..kilo/rules/gdpr-pii.md). Theres also a pre-existing ci check./cloud-prcommand. We already had one a pr slash command at some point and I'm guessing most folks have their own workflow.specsskill (rebuilt from disk — all.specs/excepttemplate.md); per-service spec pointers added tokiloclaw,kiloclaw-billing,mcp-gateway.CONTEXT.mdcovers only the Code Reviewer + Security Agent domains.AGENTS.md.Root
AGENTS.mdshrank from 134 to ~75 lines.Verification
Visual Changes
N/A
Reviewer Notes