feat: update Copilot instructions and enhance testing guidelines

chrisreddington · chrisreddington · commit 6565e9f56446 · 2025-11-27T11:30:01.000Z
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
@@ -1,77 +1,78 @@
-# Turn-Based Games Platform - AI Developer Guide
+# Turn-Based Games Platform
 
-## Architecture Overview
+## Purpose
 
-This is a **monorepo workspace** with three packages that work together to create a turn-based games platform:
+These instructions guide Copilot across all files in this monorepo. Language-specific and framework-specific rules are in separate instruction files under `.github/instructions/`.
 
-- **`shared/`** - Core game logic, types, and SQLite storage (TypeScript library)
-- **`web/`** - Next.js 15 frontend with API routes (React + TailwindCSS)  
-- **`mcp-server/`** - Model Context Protocol server providing AI opponents (Node.js service)
+## Repository Structure
 
-**Key Integration Pattern**: The MCP server communicates with the web app via HTTP calls to `/api/games/*` endpoints, not direct database access. This maintains clear service boundaries.
+- `shared/` - Core game logic, types, and SQLite storage (TypeScript library)
+- `web/` - Next.js 15 frontend with API routes (React + TailwindCSS)
+- `mcp-server/` - Model Context Protocol server providing AI opponents (Node.js service)
 
-## Development Workflow
+## Code Standards
 
-**Essential build order** (shared must be built first):
-```bash
-npm run build --workspace=shared  # Always run first
-npm run dev --workspace=web       # Starts Next.js dev server (port 3000)
-npm run dev --workspace=mcp-server # Starts MCP server (stdio)
-```
+### Required Before Each Commit
+
+- Run `npm run lint` to check for linting issues
+- Run `npm run test` to ensure all tests pass
+- Build shared package first when making cross-package changes
+
+### Development Flow
+
+- Build shared: `npm run build --workspace=shared` (always run first)
+- Dev web: `npm run dev --workspace=web` (starts Next.js on port 3000)
+- Dev MCP: `npm run dev --workspace=mcp-server` (starts MCP server via stdio)
+- Full test: `npm run test` (runs all workspace tests)
+- Full lint: `npm run lint` (runs all workspace linting)
 
-**Database location**: SQLite file is at `web/games.db` by default, controlled by `GAMES_DB_PATH` env var.
+### Database
 
-## Core Architecture Patterns
+- SQLite file location: `web/games.db` (default)
+- Controlled by `GAMES_DB_PATH` environment variable
 
-### Game Implementation Pattern
-All games follow the `Game<TGameState, TMove>` interface in `shared/src/types/game.ts`:
-- `validateMove()` - Check if move is legal
-- `applyMove()` - Apply move and return new state  
-- `checkGameEnd()` - Determine win/draw/continue
-- `getValidMoves()` - Get available moves
-- `getInitialState()` - Create starting game state
+## Architecture Guidelines
 
-### Storage Pattern
-Games use a dual-storage approach:
-- **Web app**: Direct SQLite access via `shared/src/storage/sqlite-storage.ts`
-- **MCP server**: HTTP calls to web API endpoints (no direct DB access)
+### Service Boundaries
 
-### AI Integration Pattern
-The MCP server exposes tools like `play_tic_tac_toe` and `create_tic_tac_toe_game`. It calls the web API to:
-1. Fetch current game state
-2. Calculate AI move using algorithms in `mcp-server/src/ai/`
-3. Submit move back via API POST
+- MCP server communicates with web app via HTTP calls to `/api/games/*` endpoints
+- MCP server never accesses the database directly
+- Web app uses direct SQLite access via `shared/src/storage/sqlite-storage.ts`
 
-## File Organization Conventions
+### Game Interface
 
-### API Routes Structure
-- `web/src/app/api/games/[game-type]/route.ts` - Create/list games
-- `web/src/app/api/games/[game-type]/[id]/move/route.ts` - Make moves
+All games implement `Game<TGameState, TMove>` interface in `shared/src/types/game.ts`:
 
-### Game-Specific Types
-Game states extend `BaseGameState` and are defined in `shared/src/types/games.ts`:
-- `TicTacToeGameState` includes `board: Board` and `playerSymbols`
-- `RPSGameState` includes `rounds` and `scores`
+```typescript
+// Required methods for all game implementations
+validateMove(state, move)   // Check if move is legal
+applyMove(state, move)      // Apply move and return new state
+checkGameEnd(state)         // Determine win/draw/continue
+getValidMoves(state)        // Get available moves
+getInitialState(config)     // Create starting game state
+```
+
+### API Routes Pattern
 
-### Component Organization
-- `web/src/components/games/` - Game-specific UI components
-- `web/src/app/games/[game-type]/` - Game-specific pages
+- Create/list games: `web/src/app/api/games/[game-type]/route.ts`
+- Make moves: `web/src/app/api/games/[game-type]/[id]/move/route.ts`
 
-## Development Guidelines
+## Adding New Games
 
-### Adding New Games
 1. Define types in `shared/src/types/games.ts`
 2. Implement game class in `shared/src/games/`
 3. Add API routes in `web/src/app/api/games/[new-game]/`
 4. Create AI implementation in `mcp-server/src/ai/`
 5. Add MCP tools in `mcp-server/src/server.ts`
 6. Build UI components in `web/src/components/games/`
 
-### Environment Variables
+## Environment Variables
+
 - `WEB_API_BASE` - MCP server's web app URL (default: `http://localhost:3000`)
 - `GAMES_DB_PATH` - SQLite database location (default: `./games.db`)
 
-### Key Dependencies
-- `@modelcontextprotocol/sdk` - MCP server framework
-- `sqlite3` - Database (shared between web/mcp-server)
-- `@turn-based-mcp/shared` - Internal workspace dependency
+## Security Considerations
+
+- Never hardcode credentials or API keys
+- Validate all user inputs in API routes
+- Sanitize game IDs and player names before database operations
diff --git a/.github/instructions/copilot-instructions.instructions.md b/.github/instructions/copilot-instructions.instructions.md
@@ -0,0 +1,138 @@
+---
+applyTo: "**/{copilot-instructions.md,*.instructions.md,AGENTS.md,CLAUDE.md,GEMINI.md}"
+description: Guidelines for writing GitHub Copilot custom instruction files
+---
+
+# Writing GitHub Copilot Custom Instructions
+
+Follow these guidelines when creating or modifying Copilot instruction files.
+
+## File Types and Their Purpose
+
+### Repository-Wide Instructions (`copilot-instructions.md`)
+
+Use for:
+- General project standards that apply to all files
+- Cross-cutting concerns (error handling philosophy, architecture patterns)
+- Build, test, and lint commands
+- Security requirements
+- Environment variable documentation
+
+Do NOT include:
+- Language-specific coding standards
+- Framework-specific patterns
+- Rules that only apply to certain file types
+
+### Path-Specific Instructions (`*.instructions.md`)
+
+Use for:
+- Language-specific coding standards (TypeScript, Python, etc.)
+- Framework-specific patterns (React, Next.js, etc.)
+- Technology-specific concerns (testing, API routes, etc.)
+- Different rules for different parts of the codebase
+
+Always include frontmatter:
+```yaml
+---
+applyTo: "glob/pattern/**/*.{ts,tsx}"
+description: Brief description of what these instructions cover
+---
+```
+
+## Writing Effective Instructions
+
+### Structure and Formatting
+
+- Use distinct headings to separate topics
+- Use bullet points for easy scanning
+- Write short, imperative directives (not narrative paragraphs)
+- Keep any single file under 1,000 lines
+
+```markdown
+<!-- ❌ Avoid: Narrative style -->
+When you're reviewing code, it would be good if you could try to look
+for situations where developers might have accidentally left in
+sensitive information like passwords or API keys.
+
+<!-- ✅ Prefer: Imperative bullet points -->
+## Security
+- Check for hardcoded secrets, API keys, or credentials
+- Validate all user inputs
+- Use parameterized queries to prevent SQL injection
+```
+
+### Provide Concrete Examples
+
+Include code snippets showing correct and incorrect patterns:
+
+```markdown
+## Naming Conventions
+
+```typescript
+// ❌ Avoid
+const d = new Date();
+const x = users.filter(u => u.active);
+
+// ✅ Prefer
+const currentDate = new Date();
+const activeUsers = users.filter(user => user.isActive);
+```
+```
+
+### Be Specific and Actionable
+
+```markdown
+<!-- ❌ Vague -->
+- Write good tests
+- Use proper error handling
+
+<!-- ✅ Specific -->
+- Test names should follow: "should [expected behavior] when [condition]"
+- Wrap async operations in try/catch and return appropriate error responses
+```
+
+## What NOT to Include
+
+Instructions that Copilot cannot follow:
+
+- Formatting changes: "Use bold text for critical issues"
+- External links: "Follow standards at https://example.com" (copy content instead)
+- Vague quality requests: "Be more accurate", "Don't miss any issues"
+- UI modifications: "Add emoji to comments"
+
+## Glob Pattern Examples
+
+```yaml
+# All TypeScript files
+applyTo: "**/*.{ts,tsx}"
+
+# Test files (both patterns)
+applyTo: "**/{*.test.{ts,tsx,js,jsx},__tests__/**/*.{ts,tsx,js,jsx}}"
+
+# Specific directory
+applyTo: "web/src/components/**/*.{tsx,ts}"
+
+# API routes
+applyTo: "web/src/app/api/**/*.{ts,js}"
+
+# Multiple specific paths
+applyTo: "{shared,mcp-server}/src/**/*.ts"
+```
+
+## Recommended Section Order
+
+1. **Purpose** - Brief statement of what the file covers
+2. **Naming Conventions** - How to name things
+3. **Code Style** - Formatting and structure rules
+4. **Patterns** - Common patterns to follow (with examples)
+5. **Error Handling** - How to handle errors
+6. **Security** - Security considerations
+7. **Testing** - Testing expectations
+8. **Performance** - Performance considerations
+
+## Iteration Process
+
+1. Start with 10-20 specific instructions
+2. Note which instructions are followed or missed
+3. Refine wording for missed instructions
+4. Add new instructions incrementally based on needs
diff --git a/.github/instructions/testing.instructions.md b/.github/instructions/testing.instructions.md
@@ -1,5 +1,5 @@
 ---
-applyTo: "**/*.test.{ts,tsx,js,jsx}"
+applyTo: "**/{*.test.{ts,tsx,js,jsx},__tests__/**/*.{ts,tsx,js,jsx}}"
 description: Testing guidelines and patterns for the turn-based games platform
 ---
 
@@ -94,3 +94,10 @@ const localMockGameState = { /* duplicated data */ }  // Use shared mocks instea
 - Test loading states and error boundaries
 - Include integration tests for complex workflows
 - Verify all game states (playing, finished, error)
+
+## General Principles
+
+- New features require unit tests
+- Tests should cover edge cases and error conditions
+- Test names should clearly describe what they test
+- Use table-driven tests when testing multiple similar scenarios
