Skip to content

Add coding agent anti-patterns instructions - #6264

Closed
piekwerk wants to merge 1 commit into
github:mainfrom
piekwerk:add-coding-agent-anti-patterns
Closed

piekwerk wants to merge 1 commit into
github:mainfrom
piekwerk:add-coding-agent-anti-patterns

Conversation

@piekwerk

@piekwerk piekwerk commented Nov 5, 2026

Copy link
Copy Markdown

Adds instructions/coding-agent-anti-patterns.instructions.md: detection-and-prevention rules for mistakes AI coding agents make when editing an existing codebase (silent test deletion, secret leaks, N+1 queries, blocking calls on an async runtime, unparameterized SQL, oversized diffs).

Why this is a different gap than agent-safety.instructions.md

agent-safety.instructions.md covers building governed agent frameworks (tool allowlists, audit trails, multi-agent trust). This file covers the opposite direction: constraints on an agent editing someone else's codebase. No existing file in instructions/ addresses that.

Why this is not duplicating existing model strengths

Each rule pairs a prevention instruction with a grep/git command that makes the violation mechanically checkable, rather than restating generic best practice. These patterns come from a kit I maintain and test against real agent-produced diffs, not hypothetical cases.

Checklist

  • Follows CONTRIBUTING.md naming convention (instructions/*.instructions.md, lowercase-hyphenated)
  • Frontmatter matches the documented format (description + applyTo)
  • New file, single topic, no duplicate of existing instructions
  • Targets main
  • Type of contribution: Instruction file

Add rules for detecting and preventing common AI coding agent mistakes.
@piekwerk
piekwerk requested a review from a team as a code owner November 5, 2026 09:37
Copilot AI auto review requested due to automatic review settings November 5, 2026 09:37
@github-actions github-actions Bot added instructions PR touches instructions new-submission PR adds at least one new contribution labels Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

🔒 PR Risk Scan Results

Scanned 1 changed file(s).

Severity Count
🔴 High 0
🟠 Medium 0
ℹ️ Info 0

✅ No matching risk patterns were detected in changed files.

This is an automated soft-gate report. Findings indicate review targets and do not block merge by themselves.

@github-actions github-actions Bot added merge-risk:medium requires-submitter-fixes Submission has quality-gate findings that submitter must fix before maintainer review labels Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

🚦 Submission status: 🛠️ Requires submitter fixes

Risk tier: merge-risk:medium — New or substantially changed resource without privileged execution
Required to merge: passing submission-gate checks plus 1 approval from reviewers with write access.

Why this tier
  • Adds 1 file(s), e.g. instructions/coding-agent-anti-patterns.instructions.md
  • Changes 49 lines (low-risk limit is 40)

Automated checks

Check Status Details
Line endings ✅ Passed Passed · logs
Spelling ✅ Passed Passed · logs
Generated README consistency ❌ Contribution failure Failed step: Fail workflow if files need updating · logs
Risk scan ✅ Passed Passed · logs
Contributor reputation ✅ Passed Passed · logs
Duplicate resource scan ✅ Passed Passed · logs
PR quality signal ⏭️ Skipped Skipped by its workflow · logs

Action needed

  • Generated README consistency failed. Run npm start locally and commit the regenerated files.

Review

  • Approvals: 0/1
  • Assigned reviewer: not assigned yet — comment /request-review to ask for one
  • Review target date: not set
  • Still needed: 1 more approval(s)
  • No staffed reviewer pool owns these files yet; any reviewer with write access counts as the domain reviewer.

Commands

Command Who What it does
/rerun-checks PR author, maintainers Re-runs failed or incomplete checks and re-evaluates this gate
/request-review PR author, maintainers Asks the review rotation to assign a reviewer (adds needs-reviewer)

Updated for a386f5f · gate run · This comment is maintained automatically — see submission gate docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new instruction contains multiple moderate correctness and scope issues in its detection rules and is also missing from the required README catalog.

9 open issues
What changed in this PR

Adds a repository-wide instruction file to help coding agents avoid common mistakes when modifying existing codebases.

Changes:

  • Adds guidance for test integrity, secret handling, SQL and migration safety, async-runtime behavior, ORM performance, and error handling.
  • Provides command-based detection checks for several anti-patterns.
  • Adds diff-hygiene guidance covering lockfiles, commits, debugging output, and oversized changes.
File Description
instructions/​coding-agent-anti-patterns.instructions.md New global coding-agent safety and editing guidance, including prevention rules and detection examples.

Review effort: Lite (auto)

Comment on lines +1 to +3
---
description: 'Detection-and-prevention rules for the most common mistakes AI coding agents make when editing an existing codebase: silently deleting tests, leaking secrets, breaking async/ORM/SQL safety, and oversized diffs. Apply when generating or editing code in any language.'
applyTo: '**'
## Test integrity

- Never delete or skip an existing test to make a suite pass. Fix the code under test, not the test, unless the test itself has a verified bug. State the bug before changing it.
- Detect: a diff that removes test files or lowers the test count (`git diff --stat` on `test*/`, `*_test.*`, `*.test.*` paths).
## Secrets

- Never commit `.env` files, API keys, tokens, or connection strings. If a secret is staged, unstage it immediately; if it was already committed, rotate the secret before cleaning history.
- Detect: `git diff --cached --name-only | grep -E '\.env|secret|credential'`.
## Database and query safety

- All queries must use parameterized queries or the ORM's query builder. Never concatenate user input into a SQL string, including via f-strings or template interpolation.
- Detect: `grep -rnE '"\s*\+.*(SELECT|INSERT|UPDATE|DELETE)'` and `grep -rn 'f".*SELECT.*{'`.
Comment on lines +28 to +29
- Every relation access in an ORM loop needs eager loading (`select_related`/`prefetch_related` in Django, `includes`/`eager_load` in Rails, `.with` in Ecto, etc.) or it becomes an N+1 query.
- Detect: query-count assertions in tests, or `EXPLAIN`/debug-toolbar output showing one query per row.
Comment on lines +33 to +34
- Inside async functions, use only async I/O. Blocking calls (sync file reads, `thread::sleep`, blocking HTTP clients) stall the whole runtime, not just the caller.
- Detect: in Rust/Tokio, `grep -rn 'std::fs::\|thread::sleep\|reqwest::blocking' src/` inside `async fn` bodies. In Node, `grep -rn 'fs\.readFileSync\|execSync'` inside async handlers.
- Detect: in Rust/Tokio, `grep -rn 'std::fs::\|thread::sleep\|reqwest::blocking' src/` inside `async fn` bodies. In Node, `grep -rn 'fs\.readFileSync\|execSync'` inside async handlers.
- Never use `.unwrap()` (Rust) or an equivalent force-unwrap in a path reachable from a network request. A `None`/`Err` there panics or crashes the handler. Use `?`, `.map_err()`, or an explicit fallback.
- Catch specific exception/error types, not a bare `Exception`/`Error`. Re-raise what you cannot handle (`KeyboardInterrupt`, `SystemExit`, OOM signals).
- Every external call (HTTP, database, filesystem, third-party API) needs a timeout, error handling that does not crash the caller, and a fallback or explicit error surfaced to the user.
- Make the smallest change that solves the problem. To fix one function, edit that function. Do not rewrite or reformat the whole file.
- Detect: `git diff --stat` shows a full file rewritten for what the task described as a small fix.
- One logical change per commit. Do not combine a refactor with a feature or a style pass; commit the refactor first, verify, then commit the feature.
- After changing a dependency manifest, run the install command and commit the updated lockfile in the same commit. A manifest change without a matching lockfile change breaks CI (`npm ci`, `uv sync --locked`, etc.) for every other contributor.
- Detect: `git diff --stat` shows a full file rewritten for what the task described as a small fix.
- One logical change per commit. Do not combine a refactor with a feature or a style pass; commit the refactor first, verify, then commit the feature.
- After changing a dependency manifest, run the install command and commit the updated lockfile in the same commit. A manifest change without a matching lockfile change breaks CI (`npm ci`, `uv sync --locked`, etc.) for every other contributor.
- Remove debugging output (`console.log`, `print`, `println!`, `dbg!`) before the final commit; use the project's logging framework for anything that should stay.
@aaronpowell aaronpowell closed this Nov 6, 2026
@aaronpowell

Copy link
Copy Markdown
Contributor

Thanks for the contribution. We're not able to take this one, for the same reason as #5190: almost all of the rules are general practice that current models already follow without an instruction file. Examples include "Never delete or skip an existing test to make a suite pass", "All queries must use parameterized queries", "Make the smallest change that solves the problem", and "Catch specific exception/error types". Applying them globally with applyTo: '**' adds context to every request without changing much. The secrets and SQL guidance is also already covered by instructions/security-and-owasp.instructions.md.

The detection commands are meant to be the differentiator, but they're too coarse to check the rules they're attached to:

  • git diff --stat on test paths won't catch skipped tests or removed assertions.
  • The secrets check only matches file names, not staged content.
  • The SQL regex misses most real query construction.

There's also a factual slip: .with is Laravel Eloquent, not Ecto, which uses preload.

If you'd like to contribute again, the best fit is something narrowly scoped that a model genuinely gets wrong without help. Examples include version-specific breaking changes, non-obvious tool behaviour, or a script that deterministically performs one of these checks, paired with a concrete before/after showing the difference it makes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

instructions PR touches instructions merge-risk:medium new-submission PR adds at least one new contribution requires-submitter-fixes Submission has quality-gate findings that submitter must fix before maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants