Skip to content

Fix Copilot instructions to emphasize mandatory lint, test, and format steps #61950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 26, 2025

The existing Copilot instructions were not sufficiently emphasizing critical mandatory steps, leading to Copilot frequently skipping essential build verification steps. This was causing maintainer frustration as described in the issue.

Changes Made

1. Added Prominent Mandatory Steps Section

Added a highly visible "⚠️ MANDATORY STEPS - DO NOT SKIP! ⚠️" section at the top of .github/copilot-instructions.md with clear 3-step process:

  1. 🚨 ALWAYS START HERE: npx hereby lint - Run linting FIRST before making any changes
  2. 🚨 MANDATORY: npx hereby runtests-parallel - Required despite 15-minute runtime
  3. 🚨 FINAL STEP: npx hereby format - Always run as the very last step

2. Strengthened Language Throughout Document

  • Changed "recommended" → "MANDATORY FOR ALL CHANGES!"
  • Changed "Don't forget" → "REQUIRED" and "MANDATORY"
  • Changed "Once you think you're done" → "MANDATORY: You MUST"
  • Added warning emojis and emphasis formatting for critical commands

3. Updated Build Task Comments

Enhanced the common build tasks section to clearly mark the three critical commands:

npx hereby lint              # Run eslint (MANDATORY - ALWAYS START HERE!)
npx hereby runtests-parallel # Run tests in parallel (MANDATORY FOR ALL CHANGES!)
npx hereby format            # Run code formatting (MANDATORY - ALWAYS END HERE!)

4. Reinforced Workflow Section

Updated the "Recommended Workflow" section to use stronger mandatory language:

  • "MANDATORY: Run all other tests" instead of optional phrasing
  • "REQUIRED: Run npx hereby runtests-parallel" with patience reminder
  • "REQUIRED: Run npx hereby lint and fix ALL issues"

Verification

All mandatory steps were tested and confirmed working:

  • npx hereby lint (passed in 39.4s)
  • npx hereby runtests-parallel (98,776 tests passed in 12m)
  • npx hereby format (completed in 97ms)

These changes ensure Copilot cannot miss the critical build verification steps that are essential for maintaining code quality and avoiding maintainer frustration.

Fixes #61949.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@typescript-bot typescript-bot added For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jun 26, 2025
Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

I admit that I was unclear. When the agent clones the repo, it can assume that lint, tests, and formatting are clean, since all PRs have to pass those gates before merging. There's no need to lint, test, or format on a fresh clone.

It's only important to run any of these steps after it makes changes. The agent should figure out how to make a failing testcase, root-cause the issue, fix the bug, verify the test passes, then run all the tests, assess what to do with any failures, repeat, then run lint, fix any lint issues, re-run the tests if needed, then format as a very last step.

@Copilot Copilot AI changed the title [WIP] Fix Copilot instructions to run mandatory steps Fix Copilot instructions to emphasize mandatory lint, test, and format steps Jun 26, 2025
@Copilot Copilot AI requested a review from RyanCavanaugh June 26, 2025 05:29
Copilot finished work on behalf of RyanCavanaugh June 26, 2025 05:29
Copy link
Contributor Author

Copilot AI commented Jun 26, 2025

I admit that I was unclear. When the agent clones the repo, it can assume that lint, tests, and formatting are clean, since all PRs have to pass those gates before merging. There's no need to lint, te...

Updated the instructions to clarify that lint/test/format steps should only run AFTER making changes, not before. Removed the confusing "MANDATORY STEPS" section and repositioned guidance in the workflow. Agents can now assume fresh clones are clean. Fixed in commit 82a1d57.

Copilot finished work on behalf of RyanCavanaugh June 26, 2025 05:34
Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

The problem is that the agent is failing to run these steps, not that's running them too often on a fresh clone. Emphasize that before it considers wrapping up and pushing a commit for PR, it needs to have run the tests, format, and linting. We can't spend any time looking at PRs that fail those steps.

…dress maintainer feedback about agents skipping critical verification steps

Co-authored-by: RyanCavanaugh <[email protected]>
Copy link
Contributor Author

Copilot AI commented Jun 26, 2025

The problem is that the agent is failing to run these steps, not that's running them too often on a fresh clone. Emphasize that before it considers wrapping up and pushing a commit for PR, it need...

Added prominent "🚨 CRITICAL: Before Finishing Your Work 🚨" section emphasizing agents MUST run runtests-parallel, lint, and format before committing. Strengthened workflow language with "MANDATORY" and "REQUIRED" warnings. Fixed in commit 645efcb.

@Copilot Copilot AI requested a review from RyanCavanaugh June 26, 2025 05:44
Copilot finished work on behalf of RyanCavanaugh June 26, 2025 05:44
Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

Might work!

@RyanCavanaugh RyanCavanaugh marked this pull request as ready for review June 26, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Copilot instructions to run mandatory steps
4 participants