Skip to content

refactor: restructure AGENTS.md using best practices skill#1145

Merged
myakove merged 3 commits into
mainfrom
refactor/agents-md-best-practices
Jul 1, 2026
Merged

refactor: restructure AGENTS.md using best practices skill#1145
myakove merged 3 commits into
mainfrom
refactor/agents-md-best-practices

Conversation

@myakove

@myakove myakove commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Refactored AGENTS.md from 221 → 105 lines (52% reduction) following the update-agents-md skill's research-backed best practices.

Changes

Before After
Commands buried mid-file Commands first (highest ROI)
No Definition of Done Added — 5 specific exit-code criteria
No escalation rules Added When Blocked section
Topic-organized Task-organized (When Writing Code, When Adding a Handler, etc.)
Verbose architecture essay 3 sentences + bullet points, references docs/
Inline Sidecar/AI/Test Oracle details Condensed to one-line references
No Boundaries section Added ✅/⚠️/🚫 tiers

What's Preserved

  • Both critical ❌/✅ examples (PyGithub github_api_call() + anti-defensive programming)
  • All technical facts — no content accuracy changes
  • Security warnings (unauthenticated log viewer)
  • docsfy docs/ warning

Principles Applied

Based on GitHub 2,500-repo analysis + Augment Code measured eval:

  • Fewer lines = more compliance
  • Commands first = highest ROI
  • Task-organized > topic-organized
  • Every "don't" paired with a "do"
  • Progressive disclosure via referenced files

Refactored AGENTS.md from 221 to 105 lines following update-agents-md skill:
- Commands section first (highest ROI)
- Added Definition of Done with specific exit codes
- Added When Blocked escalation rules
- Task-organized sections (When Writing Code, When Adding a Handler, etc.)
- Condensed architecture to 3 sentences + references
- Added Boundaries section (✅/⚠️/🚫 tiers)
- Kept critical ❌/✅ examples (PyGithub, anti-defensive)
@myakove-bot

Copy link
Copy Markdown
Collaborator

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest python-module-install - Test Python package installation
  • /retest pre-commit - Run pre-commit hooks and checks
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 1 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge
AI Features
  • Conventional Title: Mode: fix (claude/claude-opus-4-6-1m)
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)
  • Test Oracle: Triggers: approved (claude/claude-opus-4-6[1m]); /test-oracle can be used anytime
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refactor AGENTS.md for task-first best-practice guidance

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Reorder AGENTS.md to put high-ROI commands and completion criteria first.
• Add Definition of Done, escalation rules, and explicit ✅/⚠️/🚫 boundaries.
• Condense architecture/patterns into task-oriented sections with references to docs/.
Diagram

graph TD
A([Contributor]) --> B["AGENTS.md"] --> C["Commands"] --> D["Definition of Done"] --> E["Task Guides"] --> F["Boundaries & Security"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split guidance into multiple focused docs (keep AGENTS.md as index)
  • ➕ Keeps AGENTS.md very short while allowing deep dives per topic
  • ➕ Reduces churn/conflicts when different areas evolve
  • ➖ More navigation overhead for first-time contributors
  • ➖ Risk of fragmentation if cross-links drift
2. Adopt a standardized contributor template (CONTRIBUTING.md-style)
  • ➕ Familiar structure for external contributors
  • ➕ Easier to compare/maintain across repos
  • ➖ May not fit agent-specific operational guidance needs
  • ➖ Can reintroduce verbosity if template is too generic
3. Generate AGENTS.md from a structured source (YAML)
  • ➕ Consistent formatting and ordering
  • ➕ Enables linting/validation of required sections
  • ➖ Adds tooling complexity for a doc-only problem
  • ➖ Harder to make quick edits in-place

Recommendation: The chosen approach (task-first, commands-first, progressive disclosure via references) is optimal for compliance and day-to-day usability. If AGENTS.md grows again, consider splitting deeper sections into dedicated docs while keeping AGENTS.md as an indexed entry point.

Files changed (1) +85 / -201

Documentation (1) +85 / -201
AGENTS.mdRestructure AGENTS.md into commands-first, task-oriented guidance +85/-201

Restructure AGENTS.md into commands-first, task-oriented guidance

• Reorders the document to surface commands and a Definition of Done first, adds escalation rules for when blocked, and introduces explicit boundaries and security guardrails. Condenses architecture/details into a short project overview plus task-based sections while preserving key PyGithub and fail-fast examples.

AGENTS.md

@qodo-code-review

qodo-code-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Verify leaves artifacts ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
AGENTS.md labels the “Full verify (read-only)” command as read-only, but pytest -n auto is
configured to always generate an HTML coverage report under .tests_coverage/, which can leave the
working tree dirty. This can mislead contributors and increases the risk of accidentally committing
generated coverage output.
Code

AGENTS.md[12]

+- Full verify (read-only): `uv run ruff check --no-fix && uv run ruff format --check && uv run mypy webhook_server/ && uv run --group tests pytest -n auto`
Relevance

⭐⭐ Medium

No direct precedent; team has accepted adding gitignore entries for generated artifacts
(.coverage_report.txt) in PR #804.

PR-#804
PR-#1141

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The “Full verify” command runs pytest -n auto; repo-level pytest defaults always enable HTML
coverage output and coverage config writes that HTML to .tests_coverage/, which is not ignored by
git, so the command is not actually side-effect-free.

AGENTS.md[10-13]
pytest.ini[1-7]
pyproject.toml[1-9]
.gitignore[40-54]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AGENTS.md` says the “Full verify (read-only)” command is read-only, but the repo’s pytest/coverage configuration writes an HTML coverage report to `.tests_coverage/`, which is not gitignored. Running “Full verify” can therefore produce untracked files and a non-clean working tree.

### Issue Context
- `pytest.ini` always includes `--cov-report=html`.
- `pyproject.toml` configures coverage HTML output directory as `.tests_coverage`.
- `.gitignore` ignores `htmlcov/` but does not ignore `.tests_coverage/`.

### Fix Focus Areas
- AGENTS.md[10-13]
- pytest.ini[1-7]
- pyproject.toml[1-9]
- .gitignore[40-54]

### Suggested fix options (pick one)
1. **Add `.tests_coverage/` to `.gitignore`** so “Full verify” stays effectively read-only with respect to git status.
2. **Change pytest defaults** to not emit HTML coverage by default (e.g., remove `--cov-report=html` from `pytest.ini` addopts), and keep HTML as an explicit opt-in.
3. **Update AGENTS.md wording** to remove “read-only” or clarify it creates `.tests_coverage/` unless cleaned/ignored.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Verify step mutates files ✓ Resolved 🐞 Bug ☼ Reliability
Description
AGENTS.md’s “Lint + format” / “Full verify” commands run ruff check (auto-fix enabled in
pyproject.toml) and ruff format (write mode), which can silently modify files while still
exiting 0. This conflicts with the Definition of Done requiring ruff format --check and can leave
a dirty working tree or uncommitted formatting/lint fixes.
Code

AGENTS.md[R10-19]

+- Lint + format: `uv run ruff check && uv run ruff format`
+- Type check: `uv run mypy webhook_server/`
+- Full verify: `uv run ruff check && uv run ruff format && uv run mypy webhook_server/ && uv run --group tests pytest -n auto`
+
+## Definition of Done
+A task is complete when ALL pass:
+1. `uv run ruff check` exits 0
+2. `uv run ruff format --check` exits 0
+3. `uv run mypy webhook_server/` exits 0
+4. `uv run --group tests pytest -n auto` exits 0 — 90% coverage required, new code without tests fails CI
Relevance

⭐⭐ Medium

No direct history on ruff format --check in docs; team values detecting auto-fix mutations (PR1108)
but rejected some AGENTS.md tweaks (PR1141).

PR-#1108
PR-#1141

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
AGENTS.md defines verification commands using ruff format (write mode) and ruff check, while the
same file’s Definition of Done requires ruff format --check. Additionally, repo configuration
enables ruff auto-fixing by default, meaning ruff check can also mutate files.

AGENTS.md[3-20]
pyproject.toml[11-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AGENTS.md` documents “verify” commands that can *modify* files (`ruff format`, and `ruff check` with auto-fix enabled via `pyproject.toml`), while the Definition of Done expects a non-mutating format check (`ruff format --check`). This mismatch can mislead users/agents and leave uncommitted changes.

## Issue Context
- `pyproject.toml` sets `[tool.ruff] fix = true`, so `uv run ruff check` may rewrite code.
- `uv run ruff format` rewrites files; it is not a verification-only command.

## Fix Focus Areas
- AGENTS.md[3-20]
- pyproject.toml[11-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread AGENTS.md Outdated
Separate mutating (development) from non-mutating (verification) commands:
- Lint fix + format: ruff check/format in write mode (for development)
- Full verify: ruff check --no-fix && ruff format --check (read-only)
- Definition of Done: ruff check --no-fix (consistent with verify)
@myakove

myakove commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

AGENTS.md:10 (qodo bug) — Verify step mutates files

Addressed: Fixed in 4e1fcc6 — separated mutating (development) from non-mutating (verification) commands:

  • Lint fix + format: ruff check && ruff format (mutates files — for development)
  • Full verify: ruff check --no-fix && ruff format --check (read-only)
  • Definition of Done: uses ruff check --no-fix consistently with Full verify

This resolves the mismatch where Commands used write-mode ruff but Definition of Done expected --check mode.

@qodo-code-review

Copy link
Copy Markdown

code-review[bot]

The following review comments were reviewed and a decision was made:

AGENTS.md:10 (qodo bug) — Verify step mutates files

Addressed: Fixed in 4e1fcc6 — separated mutating (development) from non-mutating (verification) commands:

  • Lint fix + format: ruff check && ruff format (mutates files — for development)
  • Full verify: ruff check --no-fix && ruff format --check (read-only)
  • Definition of Done: uses ruff check --no-fix consistently with Full verify

This resolves the mismatch where Commands used write-mode ruff but Definition of Done expected --check mode.

Thanks — that looks resolved.

  • finding 1 is already marked implemented.
  • The commands now clearly separate mutation (ruff check && ruff format) from verification (ruff check --no-fix && ruff format --check), so the docs are consistent.

Comment thread AGENTS.md
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4e1fcc6

pytest generates HTML coverage reports in .tests_coverage/ (configured
in pyproject.toml), which was not gitignored. This made the "Full verify
(read-only)" command in AGENTS.md leave untracked files in the working tree.
@myakove

myakove commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

AGENTS.md:12 (qodo bug) — Verify leaves artifacts

Addressed: Fixed in 235c0c8 — added .tests_coverage/ to .gitignore so the Full verify command is effectively side-effect-free with respect to git status. The .tests_coverage/ directory (configured in pyproject.toml as coverage HTML output dir) was previously unignored, causing untracked files after running pytest with coverage.

@qodo-code-review

Copy link
Copy Markdown

code-review[bot]

The following review comments were reviewed and a decision was made:

AGENTS.md:12 (qodo bug) — Verify leaves artifacts

Addressed: Fixed in 235c0c8 — added .tests_coverage/ to .gitignore so the Full verify command is effectively side-effect-free with respect to git status. The .tests_coverage/ directory (configured in pyproject.toml as coverage HTML output dir) was previously unignored, causing untracked files after running pytest with coverage.

Thanks — that matches finding 1, and it’s already implemented in 235c0c87.

  • .tests_coverage/ is now ignored via .gitignore
  • the full verify command should no longer leave Git-status noise from coverage output

So this one is closed.

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 235c0c8

@myakove myakove merged commit 527d63d into main Jul 1, 2026
8 of 10 checks passed
@myakove myakove deleted the refactor/agents-md-best-practices branch July 1, 2026 04:41
@myakove-bot

Copy link
Copy Markdown
Collaborator

New container for ghcr.io/myk-org/github-webhook-server:latest published

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants