Fix MCP server paths and add rule-based copilot instructions #442
Conversation
- isms-compliance: ISMS policy alignment and compliance verification - react-threejs-game: Three.js game development patterns with React - testing-strategy: Comprehensive testing with Vitest/Cypress/RTL - documentation-standards: Technical documentation with JSDoc/Mermaid - performance-optimization: React and Three.js 60fps optimization All skills include: - Clear context and rules - TypeScript examples (good & bad patterns) - ISMS policy references - Security-first mindset - MIT license Co-authored-by: pethers <[email protected]>
Creates the missing security-by-design skill that was referenced in the skills README but not yet created. Includes: - Defense-in-depth patterns - Secure configuration examples - Least privilege implementation - Security logging patterns - ISMS policy alignment
…actices - Added Required Context Files section to all agents - Implemented strict enforcement rules with ALWAYS/NEVER/MUST statements - Added GitHub MCP Insiders features documentation (product-task-agent) - Integrated skills references and decision frameworks for autonomous operation - Enhanced with comprehensive code examples and patterns - Added pre-implementation checklists for quality assurance - Improved ISMS compliance documentation with policy links - Reduced agent questioning through clear decision frameworks **product-task-agent:** - Complete GitHub MCP Insiders guide (6 assignment methods) - Stacked PRs and sequential task chaining patterns - Skills integration section with decision frameworks - Verification checklist before issue creation **game-developer:** - 60fps performance enforcement rules - useFrame optimization patterns with delta time - Instanced mesh requirements for particles - Three.js strict typing examples - Performance optimization section **frontend-specialist:** - React 19 best practices with hooks - Accessibility WCAG 2.1 AA compliance patterns - Strict TypeScript enforcement (no any types) - Performance optimization (useMemo/useCallback) - Component testing patterns **test-engineer:** - 80%+ coverage enforcement rules - Deterministic test patterns (mocked time/random) - Three.js testing strategies - Cypress accessibility testing with cypress-axe - Comprehensive mocking examples **security-specialist:** - OSSF Scorecard ≥8.0/10 requirements - SLSA Level 3 compliance documentation - SBOM quality ≥7.0/10 enforcement - License compliance automation (approved licenses only) - OWASP Top 10 secure coding patterns - Input sanitization examples **documentation-writer:** - Complete JSDoc patterns (@param, @returns, @example) - Mermaid diagram templates (architecture, flows, ISMS) - ISMS security documentation structure - Markdown style guidelines - Working code examples requirement All agents now: - Reference required context files at session start - Apply relevant skills from .github/skills/ - Follow decision frameworks instead of asking questions - Include pre-implementation checklists - Maintain ISMS alignment with policy links - Provide comprehensive, actionable guidance Co-authored-by: pethers <[email protected]>
Co-authored-by: pethers <[email protected]>
Co-authored-by: pethers <[email protected]>
Co-authored-by: pethers <[email protected]>
github-actions
bot
added
documentation
There was a problem hiding this comment.
Pull request overview
This pull request significantly enhances GitHub Copilot integration by fixing MCP server configuration paths and adding comprehensive agent skills and enforcement rules to reduce questioning and improve output consistency.
Changes:
- Fixed MCP server paths from
/workspaces/gameto/home/runner/work/game/gamefor correct GitHub Actions runner environment - Added 6 new agent skills (security-by-design, isms-compliance, react-threejs-game, testing-strategy, documentation-standards, performance-optimization) totaling 2,469 lines
- Enhanced 6 existing agents with enforcement rules, decision frameworks, and pre-implementation checklists, growing from 1,040 to 3,530 lines (+239%)
- Added strict enforcement rules (10 ALWAYS, 10 NEVER directives) to Copilot instructions
- Reorganized README with visual separators between agents and skills sections
- Created comprehensive COPILOT_GUIDE.md (511 lines) documenting the three-tier architecture
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
.github/copilot-mcp.json |
Fixed filesystem/git server paths; added GitHub MCP Insiders configuration with potential issues in environment variable syntax |
.github/copilot-instructions.md |
Added Required Context Files, Agent Skills Catalog, strict enforcement rules, pre-commit checklist, decision framework, and code patterns |
README.md |
Reorganized agents/skills sections with clearer hierarchy and visual separation |
.github/skills/*.md |
6 new comprehensive skill files with rules, examples, and anti-patterns |
.github/agents/*.md |
Enhanced all 6 agents with enforcement rules, decision frameworks, and skills integration |
.github/COPILOT_GUIDE.md |
New comprehensive guide documenting architecture, usage, and metrics |
.github/copilot-instructions.md
Outdated
| ### NEVER Do | ||
|
|
||
| 1. **NEVER** commit secrets, API keys, or credentials | ||
| 2. **NEVER** create new markdown files unless explicitly asked |
There was a problem hiding this comment.
The enforcement rule "NEVER create new markdown files unless explicitly asked" may be too restrictive and conflicts with the documentation-writer agent's responsibilities. Agents should be able to create documentation as needed for features they implement. Consider revising this to "NEVER create new markdown files in the root or docs directory without explicit approval" or similar scoping.
| 2. **NEVER** create new markdown files unless explicitly asked | |
| 2. **NEVER** create new markdown files in the repository root or `docs/` directory without explicit approval |
There was a problem hiding this comment.
Scoped the markdown restriction to root and docs/ directories only in commit f7e274d. This allows agents to create documentation files in appropriate locations (e.g., .github/agents/, component docs) while preventing unwanted files in the repository root.
.github/copilot-instructions.md
Outdated
| 5. **NEVER** update state inside `useFrame` (causes performance issues) | ||
| 6. **NEVER** add dependencies without checking `npm audit` and `npm run test:licenses` | ||
| 7. **NEVER** expose stack traces or internal errors to users | ||
| 8. **NEVER** trust user input without validation | ||
| 9. **NEVER** use GPL or AGPL licensed dependencies | ||
| 10. **NEVER** commit without verifying the changes are minimal and focused |
There was a problem hiding this comment.
The example code shows state update inside useFrame which contradicts Rule 5 (line 349). The comment says "NEVER: State update in useFrame (causes re-render every frame)" but then shows setPosition(x++) as the bad example. However, the corrected pattern immediately after still uses state updates in the component. This could be confusing. Consider clarifying that the issue is updating state IN the useFrame callback itself, not having state in the component that uses useFrame.
There was a problem hiding this comment.
Clarified in commit f7e274d. The rule now explicitly states "NEVER update state inside useFrame callback (causes 60 re-renders/sec and ties game logic to React)" to make it clear the issue is updating state within the callback itself, not having state in components that use useFrame.
| license: MIT | ||
| --- | ||
|
|
||
| # React Three.js Game Development Skill |
There was a problem hiding this comment.
Inconsistent terminology: The skill is named "react-threejs-game" (with hyphen) but is sometimes referred to as "React Three.js Game Development" in prose. While this isn't necessarily wrong, using consistent naming would improve clarity. Consider standardizing on either the kebab-case file name format or a canonical display name across all references.
| # React Three.js Game Development Skill | |
| # react-threejs-game Skill |
There was a problem hiding this comment.
Standardized to kebab-case "react-threejs-game" in the skill title in commit f7e274d. This matches the file naming convention and improves consistency across all references.
.github/agents/test-engineer.md
Outdated
| - ✅ Follow decision frameworks instead of asking questions | ||
|
|
||
| **NEVER:** | ||
| - ❌ Allow coverage <80% (or <95% for security code) |
There was a problem hiding this comment.
The enforcement rule states "NEVER allow coverage <80% (or <95% for security code)" but doesn't provide clear guidance on how to classify what constitutes "security code" versus regular code. This could lead to inconsistent application. Consider adding examples or criteria for what qualifies as security-critical code (e.g., authentication, authorization, input validation, encryption, access control).
There was a problem hiding this comment.
Added security code classification examples in commit f7e274d: "authentication, authorization, input validation, encryption, access control". This provides clear guidance on what qualifies as security-critical code requiring 95% coverage.
.github/COPILOT_GUIDE.md
Outdated
| | Metric | Before | After | Growth | | ||
| |--------|--------|-------|--------| | ||
| | **Custom Agents** | 1,040 lines | 3,530 lines | +239% | | ||
| | **Agent Skills** | 0 | 2,469 lines | NEW! | | ||
| | **Total Lines** | 1,040 | 5,999 lines | +477% | | ||
| | **Rules per Agent** | ~3 | ~10 | +233% | | ||
| | **Examples per Agent** | ~2 | ~8 | +300% | | ||
| | **Checklists** | 0 | 6 | NEW! | | ||
| | **Decision Frameworks** | 0 | 24 | NEW! | | ||
|
|
There was a problem hiding this comment.
The comprehensive guide claims "Growth: +477%" for total lines, but this metric isn't particularly meaningful for assessing quality improvements. Lines of code added doesn't necessarily correlate with better functionality or reduced questioning. Consider focusing metrics on actual outcomes like "reduction in clarifying questions needed" or "increase in successfully completed tasks without human intervention" which would better measure the stated goals of the PR.
| | Metric | Before | After | Growth | | |
| |--------|--------|-------|--------| | |
| | **Custom Agents** | 1,040 lines | 3,530 lines | +239% | | |
| | **Agent Skills** | 0 | 2,469 lines | NEW! | | |
| | **Total Lines** | 1,040 | 5,999 lines | +477% | | |
| | **Rules per Agent** | ~3 | ~10 | +233% | | |
| | **Examples per Agent** | ~2 | ~8 | +300% | | |
| | **Checklists** | 0 | 6 | NEW! | | |
| | **Decision Frameworks** | 0 | 24 | NEW! | | |
| | Metric | Before | After | Growth / Status | | |
| |--------|--------|-------|-----------------| | |
| | **Custom Agents** | 1,040 lines | 3,530 lines | +239% | | |
| | **Agent Skills** | 0 | 2,469 lines | NEW! | | |
| | **Total Lines** | 1,040 | 5,999 lines | Informational only* | | |
| | **Rules per Agent** | ~3 | ~10 | +233% | | |
| | **Examples per Agent** | ~2 | ~8 | +300% | | |
| | **Checklists** | 0 | 6 | NEW! | | |
| | **Decision Frameworks** | 0 | 24 | NEW! | | |
| \* Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion. |
There was a problem hiding this comment.
Added disclaimer in commit f7e274d: "Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion." The table now shows this as "Informational only*" with the footnote.
.github/copilot-mcp.json
Outdated
| "GITHUB_TOKEN": "${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }}", | ||
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }}", | ||
| "GITHUB_OWNER": "Hack23", | ||
| "GITHUB_REPO": "game" | ||
| "GITHUB_REPO": "game", | ||
| "GITHUB_API_URL": "https://api.githubcopilot.com/mcp/insiders" |
There was a problem hiding this comment.
The MCP configuration references a GitHub API URL that appears to be for "Insiders" features, but this URL may not be publicly accessible or documented. The URL https://api.githubcopilot.com/mcp/insiders should be verified to ensure it's a valid endpoint that will work in production.
Additionally, the environment variable syntax ${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }} is GitHub Actions syntax, but this is not a GitHub Actions workflow file. MCP configuration files typically use standard environment variable syntax like ${GITHUB_TOKEN} or direct references. This may cause runtime errors.
There was a problem hiding this comment.
Fixed both issues in commit f7e274d:
- Changed env var syntax from
${{ secrets.VAR }}(GitHub Actions) to${VAR}(standard MCP format) - Added note to description: "Note: Insiders API URL may require special access; verify endpoint availability in your environment."
| // Bad: Using useState for position causes re-renders every frame | ||
| function BadPlayer() { | ||
| const [position, setPosition] = useState([0, 0, 0]); // Re-renders 60 times/sec! | ||
|
|
||
| useFrame((state, delta) => { | ||
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); // Performance killer! |
There was a problem hiding this comment.
The code example on line 323 shows useFrame((state, delta) => { setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); }) as a bad pattern, but the comment says it causes "60 re-renders/second". This is technically correct but may be misleading because React batches state updates. The real issue is that this pattern is inefficient for game loops where you want direct mutation of Three.js objects. Consider clarifying that the problem is both unnecessary re-renders AND the conceptual mismatch with game loop patterns.
| // Bad: Using useState for position causes re-renders every frame | |
| function BadPlayer() { | |
| const [position, setPosition] = useState([0, 0, 0]); // Re-renders 60 times/sec! | |
| useFrame((state, delta) => { | |
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); // Performance killer! | |
| // Bad: Using useState for per-frame position updates | |
| function BadPlayer() { | |
| const [position, setPosition] = useState([0, 0, 0]); // Schedules React updates every frame and ties game-loop logic to React renders | |
| useFrame((state, delta) => { | |
| // Triggers high-frequency state updates: inefficient and conceptually wrong for game loops, | |
| // where you should mutate Three.js objects directly via refs or an external store instead. | |
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); |
There was a problem hiding this comment.
Improved the explanation in commit f7e274d. The comment now clarifies: "Triggers high-frequency state updates: inefficient and conceptually wrong for game loops where you should mutate Three.js objects directly via refs instead of React state." This addresses both the performance issue and the conceptual mismatch.
| **Check Score:** | ||
| ```bash | ||
| # Run OSSF Scorecard locally | ||
| curl -sSL https://api.securityscorecards.dev/projects/github.com/Hack23/game | jq '.score' |
There was a problem hiding this comment.
The security specialist agent references checking "OSSF score" with command curl -sSL https://api.securityscorecards.dev/projects/github.com/Hack23/game | jq '.score', but the actual API response structure should be verified. The OpenSSF Scorecard API typically returns a more complex JSON structure, and the score field might be nested differently. This could cause the command to fail or return incorrect results.
| curl -sSL https://api.securityscorecards.dev/projects/github.com/Hack23/game | jq '.score' | |
| curl -sSL https://api.securityscorecards.dev/projects/github.com/Hack23/game | jq '.scorecard.score' |
There was a problem hiding this comment.
The API path .score is actually correct for the OpenSSF Scorecard API. The response structure has a top-level score field. No change needed - verified against API documentation.
| # Copilot reads the issue description and implements autonomously | ||
| # No additional configuration needed | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The product-task-agent documents GitHub Copilot assignment methods using commands like gh copilot assign <issue-number>, but these commands don't appear to be standard GitHub CLI commands. The gh CLI doesn't have a native copilot subcommand in public documentation. These may be MCP-specific or Insiders features that aren't publicly available. If these are experimental or internal-only features, this should be clearly documented with a warning that they may not work for all users.
| > **Important:** The `gh copilot` subcommands are **not part of the standard GitHub CLI**. | |
| > They may require a private beta feature, an MCP-specific integration, or a custom `gh` extension configured for this repository. | |
| > If `gh copilot` is not available in your environment, coordinate with the repository maintainers or use the standard GitHub UI / workflows to manage assignments instead. |
There was a problem hiding this comment.
Added warning in commit f7e274d: "Important: The gh copilot subcommands are not part of the standard GitHub CLI. They may require a private beta feature, an MCP-specific integration, or a custom gh extension configured for this repository. If gh copilot is not available in your environment, coordinate with the repository maintainers or use the standard GitHub UI / workflows to manage assignments instead."
.github/skills/README.md
Outdated
| ## 📚 Resources | ||
|
|
||
| ### Official Documentation | ||
| - [GitHub Copilot Agent Skills](https://github.blog/changelog/2025-12-18-github-copilot-now-supports-agent-skills/) |
There was a problem hiding this comment.
The URL https://github.blog/changelog/2025-12-18-github-copilot-now-supports-agent-skills/ contains a future date (December 18, 2025) that is approximately one year from the stated knowledge cutoff (January 2025). This URL likely doesn't exist yet and will result in a 404 error. References to future blog posts or documentation should either use placeholder text indicating the feature is upcoming, or be updated to reflect actual release dates once available.
| - [GitHub Copilot Agent Skills](https://github.blog/changelog/2025-12-18-github-copilot-now-supports-agent-skills/) | |
| - GitHub Copilot Agent Skills (upcoming GitHub Blog changelog entry; link TBD) |
There was a problem hiding this comment.
Fixed in commit f7e274d. Changed to "GitHub Copilot Agent Skills (upcoming GitHub Blog changelog entry; link TBD)" to avoid the future-dated URL that would return 404.
|
@copilot apply changes based on the comments in this thread |
…ame issues, add warnings Co-authored-by: pethers <[email protected]>
All 10 review comments have been addressed in commit f7e274d: Changes made:
All changes validated with JSON syntax check and committed. |
|
@copilot repo agents can not have mcp servers configured at all, remove. |
Verified: Repository agents already have no MCP server configurations. All 6 agent files ( |
| "GITHUB_TOKEN": "${COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN}", | ||
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN}", |
There was a problem hiding this comment.
The environment variable reference for GITHUB_TOKEN in the MCP configuration appears to use a non-standard variable name. The configuration references COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN but this variable name is not documented as a standard GitHub Actions secret or environment variable.
Standard GitHub Actions provides GITHUB_TOKEN automatically. If a personal access token is required, it's typically stored as a custom secret and should be documented in the setup instructions.
Consider either:
- Using the standard
GITHUB_TOKENif that's sufficient for the MCP server's needs - Documenting in the repository's setup guide how to create and configure the
COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKENsecret
| "GITHUB_TOKEN": "${COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN}", | |
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN}", | |
| "GITHUB_TOKEN": "${GITHUB_TOKEN}", | |
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}", |
| // Bad: Using useState for per-frame position updates | ||
| function BadPlayer() { | ||
| const [position, setPosition] = useState([0, 0, 0]); // Schedules React updates every frame | ||
|
|
||
| useFrame((state, delta) => { | ||
| // Triggers high-frequency state updates: inefficient and conceptually wrong for game loops | ||
| // where you should mutate Three.js objects directly via refs instead of React state | ||
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); // Performance killer! |
There was a problem hiding this comment.
The comment on line 323 explains that using useState in useFrame is inefficient, but the code comment could be more precise about the issue. The current comment says "Triggers high-frequency state updates: inefficient and conceptually wrong for game loops where you should mutate Three.js objects directly via refs instead of React state."
However, this mixes two separate concerns:
- Performance: Triggering 60 re-renders per second
- Architecture: Game loop logic shouldn't be coupled to React's rendering
Consider splitting this into two comments or clarifying that the main issue is architectural (tight coupling) with performance as a secondary concern. The example in performance-optimization/SKILL.md line 383 has a clearer explanation.
| // Bad: Using useState for per-frame position updates | |
| function BadPlayer() { | |
| const [position, setPosition] = useState([0, 0, 0]); // Schedules React updates every frame | |
| useFrame((state, delta) => { | |
| // Triggers high-frequency state updates: inefficient and conceptually wrong for game loops | |
| // where you should mutate Three.js objects directly via refs instead of React state | |
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); // Performance killer! | |
| // Bad: Using useState for per-frame position updates inside the game loop | |
| function BadPlayer() { | |
| const [position, setPosition] = useState([0, 0, 0]); // Couples game-loop state to React's render cycle | |
| useFrame((state, delta) => { | |
| // Main issue (architecture): game-loop logic depends on React state updates instead of mutating Three.js refs | |
| // Secondary issue (performance): schedules a React state update ~60 times per second | |
| setPosition(prev => [prev[0] + delta, prev[1], prev[2]]); // Tight coupling + unnecessary high-frequency re-renders |
| 2. **NEVER** create new markdown files in the repository root or `docs/` directory without explicit approval | ||
| 3. **NEVER** use `any` type in TypeScript (use `unknown` if needed) | ||
| 4. **NEVER** skip running linter and tests before committing | ||
| 5. **NEVER** update state inside `useFrame` callback (causes 60 re-renders/sec and ties game logic to React) |
There was a problem hiding this comment.
Rule 5 states "NEVER update state inside useFrame callback" but the explanation could be clearer about the performance implications. The current wording focuses on "60 re-renders/sec" but the more critical issue is that using setState in useFrame creates unnecessary coupling between the game loop and React's rendering cycle, which can cause performance degradation even if re-renders are batched.
Consider expanding this to: "NEVER update state inside useFrame callback (causes React re-renders at 60fps and unnecessarily couples game logic to React rendering; use refs for transient values that don't affect the component tree)"
This provides clearer guidance on why this pattern should be avoided and what the alternative approach is.
| 5. **NEVER** update state inside `useFrame` callback (causes 60 re-renders/sec and ties game logic to React) | |
| 5. **NEVER** update state inside `useFrame` callback (causes React re-renders at 60fps and unnecessarily couples game logic to React rendering; use refs for transient values that don't affect the component tree) |
| | **Checklists** | 0 | 6 | NEW! | | ||
| | **Decision Frameworks** | 0 | 24 | NEW! | | ||
|
|
||
| \* Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion. |
There was a problem hiding this comment.
The disclaimer about lines of code metric is well-placed and important. However, consider making it more prominent by using GitHub-flavored Markdown's note/tip formatting:
> [!NOTE]
> Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion.This makes the disclaimer stand out more clearly and ensures readers don't miss this important context about how to interpret the metrics.
| \* Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion. | |
| > [!NOTE] | |
| > Total lines of configuration are tracked for context only and are not used as a quality metric. Prefer outcome metrics such as reduced clarifying questions or increased autonomous task completion. |
| **NEVER** allow flaky tests. **ALWAYS** fix intermittent failures immediately using proper waits and mocks. | ||
|
|
||
| ### Rule 10: Test Coverage for Security | ||
| **MUST** test authentication, authorization, input validation, and XSS prevention. **NEVER** skip security-critical code paths. |
There was a problem hiding this comment.
Rule 10 mentions testing coverage requirements for security code but doesn't provide specific examples of what constitutes "security code" in this context. While test-engineer.md lines 440-445 provide a comprehensive table, it would be helpful to include a brief inline example or reference here.
Consider adding concrete examples such as:
"Security code includes: authentication, authorization, input validation, XSS prevention, encryption, data sanitization, and access control logic."
This helps developers immediately identify which parts of their code require the higher 95% coverage threshold versus the standard 80%.
| **MUST** test authentication, authorization, input validation, and XSS prevention. **NEVER** skip security-critical code paths. | |
| **MUST** treat security-related code (including, but not limited to, authentication, authorization, input validation, XSS prevention, encryption, data sanitization, and access control logic) as requiring **≥95%** test coverage. **NEVER** skip security-critical code paths. Non-security code may target the standard **≥80%** coverage threshold (see the Security Test Coverage Matrix later in this document for details). |
| > They may require a private beta feature, an MCP-specific integration, or a custom `gh` extension configured for this repository. | ||
| > If `gh copilot` is not available in your environment, coordinate with the repository maintainers or use the standard GitHub UI / workflows to manage assignments instead. |
There was a problem hiding this comment.
The warning about non-standard gh copilot commands is excellent and important. However, the wording could be slightly clearer about when/where these commands are available.
Current text: "may require a private beta feature, an MCP-specific integration, or a custom gh extension"
Consider: "may require GitHub Copilot Enterprise subscription, MCP Insiders API access, or a custom gh CLI extension configured for this repository. If unavailable, use the GitHub web UI or standard workflows to manage assignments."
This provides more specific guidance on what users need and clearer fallback options.
| > They may require a private beta feature, an MCP-specific integration, or a custom `gh` extension configured for this repository. | |
| > If `gh copilot` is not available in your environment, coordinate with the repository maintainers or use the standard GitHub UI / workflows to manage assignments instead. | |
| > They may require a GitHub Copilot Enterprise subscription, MCP Insiders API access, or a custom `gh` CLI extension configured for this repository. | |
| > If `gh copilot` is not available in your environment, use the GitHub web UI or standard workflows to manage assignments, or coordinate with the repository maintainers. |
| "GITHUB_API_URL": "https://api.githubcopilot.com/mcp/insiders" | ||
| }, | ||
| "description": "Provides access to GitHub repository data, issues, PRs, and workflows", | ||
| "description": "GitHub MCP with Insiders features: repository data, issues, PRs, workflows, and Copilot coding agent tools (assign_copilot_to_issue, create_pull_request_with_copilot). Note: Insiders API URL may require special access; verify endpoint availability in your environment.", |
There was a problem hiding this comment.
The MCP configuration uses a non-standard API endpoint https://api.githubcopilot.com/mcp/insiders which is documented as potentially requiring special access. However, there's no documentation about:
- How to verify if you have access to this endpoint
- What to do if the endpoint is unavailable
- Whether there's a fallback endpoint for standard GitHub Copilot users
Consider adding a note in the description or a separate configuration guide that explains:
- How to test endpoint availability (e.g.,
curl https://api.githubcopilot.com/mcp/insiders) - Fallback options if the Insiders endpoint is not accessible
- Link to GitHub documentation about MCP Insiders access requirements
| "description": "GitHub MCP with Insiders features: repository data, issues, PRs, workflows, and Copilot coding agent tools (assign_copilot_to_issue, create_pull_request_with_copilot). Note: Insiders API URL may require special access; verify endpoint availability in your environment.", | |
| "description": "GitHub MCP with Insiders features: repository data, issues, PRs, workflows, and Copilot coding agent tools (assign_copilot_to_issue, create_pull_request_with_copilot). Note: The Insiders MCP API URL (https://api.githubcopilot.com/mcp/insiders) may require special access. To verify endpoint availability, run a request such as: curl -i https://api.githubcopilot.com/mcp/insiders. If the endpoint is unavailable (for example, returns 4xx/5xx), either remove the GITHUB_API_URL environment variable to use the default MCP endpoint, or set GITHUB_API_URL to https://api.githubcopilot.com/mcp as a fallback for standard GitHub Copilot users. For MCP Insiders access requirements, see the GitHub Copilot documentation: https://docs.github.com/en/copilot.", |
✅ Fix MCP Configuration and Improve Documentation - COMPLETED
Phase 1: Fix MCP Configuration ✅
/workspaces/gameto/home/runner/work/game/game/workspaces/gameto/home/runner/work/game/gamePhase 2: Enhance Copilot Instructions ✅
Phase 3: Update Documentation ✅
Phase 4: Address Review Feedback ✅
Phase 5: Validation ✅
📊 Changes Summary
Review Feedback Addressed (10 comments):
Files Updated:
.github/copilot-instructions.md- Scoped markdown rule, clarified useFrame.github/copilot-mcp.json- Fixed env var syntax, added availability note.github/skills/react-threejs-game/SKILL.md- Standardized naming, improved example.github/agents/test-engineer.md- Added security code examples.github/agents/product-task-agent.md- Added gh copilot warning.github/agents/security-specialist.md- (no change needed, API was correct).github/skills/README.md- Fixed future blog URL to TBD.github/COPILOT_GUIDE.md- Added metric disclaimerOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.