Compact copilot instructions, agents, and skills (~84% size reduction)#546
Compact copilot instructions, agents, and skills (~84% size reduction)#546
Conversation
…duction) Reduce total size from ~256KB to ~42KB while preserving all key guidance: - copilot-instructions.md: 16KB → 4KB (project-specific, no redundant code examples) - Agent files: ~137KB → ~17KB (role + rules + skill refs, no duplicated patterns) - Skill files: ~90KB → ~16KB (focused rules + key examples + anti-patterns) - READMEs: ~23KB → ~5KB (streamlined tables and structure) Adapted to current game state (Target Shooter with React 19, TS 6, Three.js, Vite 8) Agent-Logs-Url: https://github.com/Hack23/game/sessions/436af475-db9a-49ed-8066-5e75930ff999 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
github-actions
Bot
added
documentation
There was a problem hiding this comment.
Pull request overview
This PR significantly reduces duplication across GitHub Copilot custom instructions, agents, and skills by introducing a clearer layering: project-specific defaults in copilot-instructions.md, reusable patterns in skills, and concise role/rule definitions in agent files.
Changes:
- Refactors
.github/copilot-instructions.mdto be project-specific and removes large duplicated examples. - Compresses agent definitions in
.github/agents/to focus on role, key rules, decision frameworks, and pointers to real repo files. - Streamlines skill docs in
.github/skills/to concise rules + minimal examples/anti-patterns, plus a simplified skills README.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/copilot-instructions.md | Rewritten as a concise, project-specific reference (commands, structure, standards). |
| .github/skills/README.md | Simplified skills catalog + hierarchy overview + resources. |
| .github/skills/testing-strategy/SKILL.md | Condensed testing rules/examples; added minimal mock/test snippets. |
| .github/skills/security-by-design/SKILL.md | Condensed security principles; minimal examples + anti-patterns. |
| .github/skills/isms-compliance/SKILL.md | Condensed compliance rules; adds key policy links + short example. |
| .github/skills/react-threejs-game/SKILL.md | Condensed R3F rules; minimal typed example + anti-patterns. |
| .github/skills/performance-optimization/SKILL.md | Condensed perf rules; minimal memo/ref examples + anti-patterns. |
| .github/skills/documentation-standards/SKILL.md | Condensed doc rules; JSDoc + Mermaid examples + anti-patterns. |
| .github/agents/README.md | Streamlined agent catalog, skill mapping table, and workflow diagram. |
| .github/agents/game-developer.md | Reduced to key rules + references to real game source files. |
| .github/agents/frontend-specialist.md | Reduced to key rules + references to real UI/hook patterns. |
| .github/agents/test-engineer.md | Reduced to key rules + references to existing test patterns/commands. |
| .github/agents/security-specialist.md | Reduced to key rules + minimal security checks/decision frameworks. |
| .github/agents/documentation-writer.md | Reduced to key rules + pointers to existing repo docs/examples. |
| .github/agents/product-task-agent.md | Reduced issue template/workflow and Copilot assignment snippets. |
| ```mermaid | ||
| graph TB |
There was a problem hiding this comment.
The Mermaid example appears to contain a leading zero-width character before the opening code fence (line begins with an invisible character before mermaid). This can break Markdown rendering/copy-paste; remove the hidden character so the fence starts exactly with mermaid.
| ``` | ||
| ``` |
There was a problem hiding this comment.
The closing Mermaid code fence also appears to include a leading zero-width character before the backticks. Remove the hidden character so the closing fence is exactly ``` to avoid broken rendering.
| ```typescript | ||
| import { render, screen, fireEvent, waitFor } from '@testing-library/react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { Player } from './Player'; | ||
| import type { PlayerProps } from './Player.types'; | ||
|
|
There was a problem hiding this comment.
This example uses JSX but the code block is labeled as typescript. For React component tests, this should be tsx (or avoid JSX) to keep the example directly usable.
| it('should display current score', () => { | ||
| render(<HUD gameState={{ score: 42, timeLeft: 30 }} />); | ||
| expect(screen.getByText(/42/)).toBeInTheDocument(); |
There was a problem hiding this comment.
The HUD test example doesn't match the actual HUD component props in this repo: HUD expects gameState: GameState (with fields like level, targets, successfulHits, totalClicks, etc.), so { score, timeLeft } alone won't type-check. Update the example to construct a valid GameState object (or reuse a helper/fixture used by existing HUD/App tests).
| vi.mock('@react-three/fiber', () => ({ | ||
| Canvas: ({ children }: { children: React.ReactNode }) => <div>{children}</div>, | ||
| useFrame: vi.fn(), | ||
| useThree: () => ({ camera: {}, scene: {}, gl: {} }), | ||
| })); |
There was a problem hiding this comment.
The Three.js mock example references React.ReactNode and returns JSX, but the snippet doesn't import React types (and the code block is labeled typescript). Use import type { ReactNode } from 'react' (or JSX.Element) and label the block as tsx so the mock is copy/pasteable.
| const element = screen.getByClassName('player-component'); // Bad! | ||
| const button = screen.getByTagName('button'); // Bad! | ||
| // BAD: Vague name | ||
| it('works', () => { ... }); |
There was a problem hiding this comment.
This anti-pattern example contains ... inside a function body, which is not valid TypeScript and can confuse readers. Replace it with a compilable placeholder (e.g., an empty body or a comment) while still conveying the anti-pattern about vague test names.
| it('works', () => { ... }); | |
| it('works', () => { /* test omitted */ }); |
| import { useRef } from 'react'; | ||
| import { useFrame } from '@react-three/fiber'; | ||
| import * as THREE from 'three'; | ||
|
|
||
| /** | ||
| * Player component optimized for 60fps updates | ||
| * Uses refs instead of state for position/velocity to avoid re-renders | ||
| */ | ||
| export function OptimizedPlayer(): JSX.Element { | ||
| function AnimatedObject(): JSX.Element { | ||
| const meshRef = useRef<THREE.Mesh>(null); | ||
|
|
||
| // Store velocity in ref - no re-renders on update | ||
| const velocityRef = useRef(new THREE.Vector3(0, 0, 0)); | ||
| const keysPressed = useRef<Set<string>>(new Set()); | ||
|
|
||
| // Handle keyboard input without re-renders | ||
| const handleKeyDown = useCallback((e: KeyboardEvent) => { | ||
| keysPressed.current.add(e.key); | ||
| }, []); | ||
|
|
||
| const handleKeyUp = useCallback((e: KeyboardEvent) => { | ||
| keysPressed.current.delete(e.key); | ||
| }, []); | ||
|
|
||
| // Game loop - runs 60fps without triggering React re-renders | ||
| useFrame((state, delta) => { | ||
| if (!meshRef.current) return; | ||
|
|
||
| const speed = 5; | ||
| const velocity = velocityRef.current; | ||
|
|
||
| // Update velocity based on input | ||
| velocity.set(0, 0, 0); | ||
| if (keysPressed.current.has('w')) velocity.z -= speed * delta; | ||
| if (keysPressed.current.has('s')) velocity.z += speed * delta; | ||
| if (keysPressed.current.has('a')) velocity.x -= speed * delta; | ||
| if (keysPressed.current.has('d')) velocity.x += speed * delta; | ||
|
|
||
| // Update position directly on Three.js object | ||
| meshRef.current.position.add(velocity); | ||
| }); | ||
|
|
||
| return ( | ||
| <mesh ref={meshRef}> | ||
| <sphereGeometry args={[0.5, 32, 32]} /> | ||
| <meshStandardMaterial color="#00ff00" /> | ||
| </mesh> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### ✅ Good Pattern: Instanced Mesh for Performance | ||
|
|
||
| ```typescript | ||
| import { useRef, useMemo } from 'react'; | ||
| import { useFrame } from '@react-three/fiber'; | ||
| import * as THREE from 'three'; | ||
|
|
||
| interface BulletSystemProps { | ||
| maxBullets: number; | ||
| } | ||
|
|
||
| /** | ||
| * Optimized bullet system using instancing | ||
| * Renders 1000+ bullets with single draw call | ||
| */ | ||
| export function BulletSystem({ maxBullets }: BulletSystemProps): JSX.Element { | ||
| const meshRef = useRef<THREE.InstancedMesh>(null); | ||
|
|
||
| // Pre-allocate bullet data (no allocation in game loop) | ||
| const bullets = useMemo(() => { | ||
| return Array.from({ length: maxBullets }, () => ({ | ||
| active: false, | ||
| position: new THREE.Vector3(), | ||
| velocity: new THREE.Vector3(), | ||
| lifetime: 0 | ||
| })); | ||
| }, [maxBullets]); | ||
|
|
||
| // Reusable objects to avoid GC pressure | ||
| const matrix = useMemo(() => new THREE.Matrix4(), []); | ||
| const tempObject = useMemo(() => new THREE.Object3D(), []); | ||
|
|
||
| useFrame((state, delta) => { | ||
| if (!meshRef.current) return; | ||
|
|
||
| let activeCount = 0; | ||
|
|
||
| bullets.forEach((bullet, i) => { | ||
| if (!bullet.active) return; | ||
|
|
||
| // Update bullet position | ||
| bullet.position.addScaledVector(bullet.velocity, delta); | ||
| bullet.lifetime -= delta; | ||
|
|
||
| // Deactivate if expired | ||
| if (bullet.lifetime <= 0) { | ||
| bullet.active = false; | ||
| return; | ||
| } | ||
|
|
||
| // Update instance matrix | ||
| tempObject.position.copy(bullet.position); | ||
| tempObject.updateMatrix(); | ||
| meshRef.current!.setMatrixAt(activeCount, tempObject.matrix); | ||
| activeCount++; | ||
| }); | ||
|
|
||
| // Set visible instance count | ||
| meshRef.current.count = activeCount; | ||
| meshRef.current.instanceMatrix.needsUpdate = true; | ||
| }); | ||
|
|
||
| return ( | ||
| <instancedMesh ref={meshRef} args={[undefined, undefined, maxBullets]}> | ||
| <sphereGeometry args={[0.1, 8, 8]} /> | ||
| <meshBasicMaterial color="#ffff00" /> | ||
| </instancedMesh> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### ✅ Good Pattern: Code Splitting | ||
|
|
||
| ```typescript | ||
| import { lazy, Suspense } from 'react'; | ||
| import { Canvas } from '@react-three/fiber'; | ||
|
|
||
| // Lazy load heavy game components | ||
| const GameWorld = lazy(() => import('./GameWorld')); | ||
| const GameUI = lazy(() => import('./GameUI')); | ||
| const GameAudio = lazy(() => import('./GameAudio')); | ||
|
|
||
| /** | ||
| * Main game component with code splitting | ||
| * Loads components on-demand to reduce initial bundle size | ||
| */ | ||
| export function Game(): JSX.Element { | ||
| return ( | ||
| <div className="game-container"> | ||
| <Suspense fallback={<LoadingScreen />}> | ||
| <Canvas> | ||
| <GameWorld /> | ||
| </Canvas> | ||
|
|
||
| <GameUI /> | ||
| <GameAudio /> | ||
| </Suspense> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function LoadingScreen(): JSX.Element { | ||
| return ( | ||
| <div className="loading-screen"> | ||
| <div className="spinner" /> | ||
| <p>Loading game...</p> | ||
| </div> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### ✅ Good Pattern: Debounced Event Handlers | ||
|
|
||
| ```typescript | ||
| import { useCallback, useRef } from 'react'; | ||
|
|
||
| /** | ||
| * Custom hook for debouncing expensive operations | ||
| * Prevents performance issues from rapid event firing | ||
| */ | ||
| export function useDebounce<T extends (...args: any[]) => any>( | ||
| callback: T, | ||
| delay: number | ||
| ): (...args: Parameters<T>) => void { | ||
| const timeoutRef = useRef<NodeJS.Timeout>(); | ||
|
|
||
| return useCallback((...args: Parameters<T>) => { | ||
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
|
|
||
| timeoutRef.current = setTimeout(() => { | ||
| callback(...args); | ||
| }, delay); | ||
| }, [callback, delay]); | ||
| } | ||
|
|
||
| // Usage in component | ||
| export function GameSettings(): JSX.Element { | ||
| const saveSettings = useCallback((settings: GameSettings) => { | ||
| // Expensive operation | ||
| localStorage.setItem('settings', JSON.stringify(settings)); | ||
| }, []); | ||
|
|
||
| // Debounce saves to avoid excessive localStorage writes | ||
| const debouncedSave = useDebounce(saveSettings, 500); | ||
|
|
||
| const handleVolumeChange = (volume: number): void => { | ||
| debouncedSave({ volume }); | ||
| }; | ||
|
|
||
| return ( | ||
| <input | ||
| type="range" | ||
| onChange={(e) => handleVolumeChange(Number(e.target.value))} | ||
| /> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### ✅ Good Pattern: Web Worker for Heavy Computation | ||
|
|
||
| ```typescript | ||
| // pathfinding.worker.ts | ||
| self.addEventListener('message', (e: MessageEvent) => { | ||
| const { start, end, obstacles } = e.data; | ||
|
|
||
| // Heavy A* pathfinding computation | ||
| const path = calculatePath(start, end, obstacles); | ||
|
|
||
| self.postMessage({ path }); | ||
| }); | ||
|
|
||
| // Game component | ||
| import { useCallback, useEffect, useRef } from 'react'; | ||
|
|
||
| export function usePathfinding() { | ||
| const workerRef = useRef<Worker>(); | ||
|
|
||
| useEffect(() => { | ||
| // Create worker on mount | ||
| workerRef.current = new Worker( | ||
| new URL('./pathfinding.worker.ts', import.meta.url), | ||
| { type: 'module' } | ||
| ); | ||
|
|
||
| return () => { | ||
| workerRef.current?.terminate(); | ||
| }; | ||
| }, []); | ||
|
|
||
| const findPath = useCallback(( | ||
| start: Vector3, | ||
| end: Vector3, | ||
| obstacles: Vector3[] | ||
| ): Promise<Vector3[]> => { | ||
| return new Promise((resolve) => { | ||
| if (!workerRef.current) { | ||
| resolve([]); | ||
| return; | ||
| } | ||
|
|
||
| const handler = (e: MessageEvent) => { | ||
| workerRef.current?.removeEventListener('message', handler); | ||
| resolve(e.data.path); | ||
| }; | ||
|
|
||
| workerRef.current.addEventListener('message', handler); | ||
| workerRef.current.postMessage({ start, end, obstacles }); | ||
| }); | ||
| }, []); | ||
|
|
||
| return { findPath }; | ||
| } | ||
| ``` | ||
|
|
||
| ### ❌ Bad Pattern: Unnecessary Re-renders | ||
|
|
||
| ```typescript | ||
| // Bad: Component re-renders every frame | ||
| function BadGameHUD({ gameState }: { gameState: GameState }) { | ||
| // Expensive computation on every render | ||
| const healthColor = gameState.health > 50 ? '#00ff00' : '#ff0000'; | ||
|
|
||
| return ( | ||
| <div> | ||
| <div style={{ color: healthColor }}> | ||
| Health: {gameState.health} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // This HUD will re-render 60 times per second if parent updates! | ||
| ``` | ||
|
|
||
| ### ❌ Bad Pattern: useState for High-Frequency Updates | ||
|
|
||
| ```typescript | ||
| // Bad: Triggers 60 re-renders per second | ||
| function BadPlayer() { | ||
| const [position, setPosition] = useState({ x: 0, y: 0, z: 0 }); | ||
|
|
||
| useFrame((state, delta) => { | ||
| // DON'T DO THIS - causes 60 re-renders/second! | ||
| setPosition(prev => ({ | ||
| x: prev.x + delta, | ||
| y: prev.y, | ||
| z: prev.z | ||
| })); | ||
| // Use ref, not state — avoids 60 re-renders/sec | ||
| useFrame((_, delta) => { | ||
| if (meshRef.current) meshRef.current.rotation.y += delta; | ||
| }); | ||
|
|
||
| return <mesh position={[position.x, position.y, position.z]} />; | ||
| return <mesh ref={meshRef}><boxGeometry /><meshStandardMaterial /></mesh>; |
There was a problem hiding this comment.
The AnimatedObject example uses useRef<THREE.Mesh>(null) but doesn't import THREE (the import * as THREE from 'three' line was removed). Add an appropriate type import (import * as THREE from 'three' or import type { Mesh } from 'three') so the snippet type-checks.
| ```typescript | ||
| // Bad: 1000 individual meshes = 1000 draw calls | ||
| function BadBullets({ bullets }: { bullets: Bullet[] }) { | ||
| return ( | ||
| <> | ||
| {bullets.map((bullet, i) => ( | ||
| <mesh key={i} position={bullet.position}> | ||
| <sphereGeometry args={[0.1]} /> | ||
| <meshBasicMaterial color="#ffff00" /> | ||
| </mesh> | ||
| ))} | ||
| </> | ||
| ); | ||
| } | ||
| ``` | ||
| // BAD: useState in useFrame — 60 re-renders/sec! | ||
| useFrame(() => { setRotation(r => r + 0.01); }); | ||
|
|
||
| ### ❌ Bad Pattern: Memory Allocation in Game Loop | ||
| // BAD: New object every render | ||
| <mesh position={[0, 0, 0]}> // Creates new array each render | ||
| // GOOD: const POS = [0, 0, 0] as const; outside component | ||
|
|
There was a problem hiding this comment.
This anti-pattern block is labeled typescript but includes JSX (<mesh ...>), which won't type-check as-is in a .ts file. Either change the fence language to tsx or rewrite the example without JSX (e.g., demonstrate array allocation in plain TS).
|
@copilot apply changes based on the comments in this thread |
- documentation-standards: Remove zero-width chars from Mermaid code fences - testing-strategy: Use tsx for JSX examples, fix HUD test to use full GameState, import ReactNode properly, fix invalid ... syntax - performance-optimization: Use tsx for JSX examples, add THREE import Agent-Logs-Url: https://github.com/Hack23/game/sessions/12cabc77-5d5c-49b0-870d-9e384d87f6c3 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 8 review comments in
|
| import type { GameState } from '../hooks/useGameState'; | ||
|
|
There was a problem hiding this comment.
In this example, the imports don’t match the repo’s actual layout: GameState lives in src/hooks/useGameState.ts, so ../hooks/useGameState is not a valid path from this SKILL.md. Consider using a repo-accurate import (or explicitly note that the snippet is schematic) to avoid copy/paste failures.
| import type { GameState } from '../hooks/useGameState'; | |
| type GameState = { | |
| score: number; | |
| isPlaying: boolean; | |
| timeLeft: number; | |
| combo: number; | |
| highScore: number; | |
| targetSize: number; | |
| level: number; | |
| isNewHighScore: boolean; | |
| targets: unknown[]; | |
| totalClicks: number; | |
| successfulHits: number; | |
| }; |
| beforeEach(() => { vi.clearAllMocks(); }); | ||
|
|
||
| describe('AuthService', () => { | ||
| let authService: AuthService; | ||
|
|
||
| beforeEach(() => { | ||
| authService = new AuthService(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('should authenticate user with valid credentials', async () => { | ||
| // Arrange | ||
| const credentials: UserCredentials = { | ||
| username: 'testuser', | ||
| password: 'SecureP@ssw0rd!' | ||
| }; | ||
|
|
||
| const mockResponse: AuthResult = { | ||
| success: true, | ||
| token: 'jwt_token_here', | ||
| userId: '12345' | ||
| }; | ||
|
|
||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: true, | ||
| json: async () => mockResponse | ||
| } as Response); | ||
|
|
||
| // Act | ||
| const result = await authService.login(credentials); | ||
|
|
||
| // Assert | ||
| expect(result.success).toBe(true); | ||
| expect(result.token).toBeDefined(); | ||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| expect.stringContaining('/api/auth/login'), | ||
| expect.objectContaining({ | ||
| method: 'POST', | ||
| headers: expect.objectContaining({ | ||
| 'Content-Type': 'application/json' | ||
| }), | ||
| body: JSON.stringify(credentials) | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle authentication failure', async () => { | ||
| // Arrange | ||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: false, | ||
| status: 401, | ||
| json: async () => ({ error: 'Invalid credentials' }) | ||
| } as Response); | ||
|
|
||
| // Act & Assert | ||
| await expect( | ||
| authService.login({ username: 'bad', password: 'wrong' }) | ||
| ).rejects.toThrow('Authentication failed'); | ||
| }); | ||
|
|
||
| it('should handle network errors', async () => { | ||
| // Arrange | ||
| mockFetch.mockRejectedValueOnce(new Error('Network error')); | ||
|
|
||
| // Act & Assert | ||
| await expect( | ||
| authService.login({ username: 'user', password: 'pass' }) | ||
| ).rejects.toThrow('Network error'); | ||
| it('should display current score', () => { | ||
| render(<HUD gameState={mockGameState} />); | ||
| expect(screen.getByText(/42/)).toBeInTheDocument(); |
There was a problem hiding this comment.
The test snippet renders <HUD ... /> but never imports HUD, so the example as written won’t run if copied into a test file. Add the HUD import (matching the real component location) to keep the “✅” example executable.
|
|
||
| ### ❌ Bad Pattern: No Examples | ||
| // BAD: No @param or @returns | ||
| /** Does calculation */ export function calc(x: number) { ... } |
There was a problem hiding this comment.
This anti-pattern example uses { ... } as a function body, which isn’t valid TypeScript and can confuse readers/tools that parse code blocks. Prefer a syntactically valid placeholder like { /* ... */ } or a minimal implementation while still illustrating the missing @param/@returns.
| /** Does calculation */ export function calc(x: number) { ... } | |
| /** Does calculation */ export function calc(x: number) { /* ... */ } |
| /** Validates and sanitizes user input — ISMS Policy SC-002 */ | ||
| function sanitizeInput(input: unknown): string { | ||
| if (typeof input !== 'string') throw new Error('Input must be a string'); | ||
| if (input.length > 100) throw new Error('Input too long'); | ||
| return input.replace(/[<>'"&]/g, ''); // Strip dangerous chars | ||
| } |
There was a problem hiding this comment.
The sanitizeInput() example is a simplistic character-stripping sanitizer. For security guidance, this can be misleading because safe handling is context-dependent (HTML vs attribute vs URL vs text) and typically requires output encoding and/or a proven sanitizer (e.g., DOMPurify for HTML). Consider renaming the example to clarify it’s only for plain-text contexts, and/or show a safer pattern (validate + encode for the output context) to avoid encouraging an unsafe “roll-your-own” sanitizer.
| /** Validates and sanitizes user input — ISMS Policy SC-002 */ | |
| function sanitizeInput(input: unknown): string { | |
| if (typeof input !== 'string') throw new Error('Input must be a string'); | |
| if (input.length > 100) throw new Error('Input too long'); | |
| return input.replace(/[<>'"&]/g, ''); // Strip dangerous chars | |
| } | |
| /** Validates plain-text user input — ISMS Policy SC-002 */ | |
| function validatePlainTextInput(input: unknown): string { | |
| if (typeof input !== 'string') throw new Error('Input must be a string'); | |
| const normalizedInput = input.trim(); | |
| if (normalizedInput.length === 0) throw new Error('Input is required'); | |
| if (normalizedInput.length > 100) throw new Error('Input too long'); | |
| return normalizedInput; | |
| } | |
| /** Encode untrusted data before inserting it into an HTML context */ | |
| function escapeHtml(input: string): string { | |
| return input | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, '''); | |
| } | |
| const plainText = validatePlainTextInput(userInput); | |
| const safeHtml = `<p>${escapeHtml(plainText)}</p>`; | |
| // For rich HTML, use a proven sanitizer such as DOMPurify instead of | |
| // relying on ad-hoc character stripping. |
| | Agent | Expertise | Tools | | ||
| |-------|-----------|-------| | ||
| | 🎮 **[game-developer](game-developer.md)** | Three.js + @react-three/fiber, game loops, 60fps performance | view, edit, create, bash | | ||
| | 🎨 **[frontend-specialist](frontend-specialist.md)** | React 19, strict TypeScript, component architecture | view, edit, create, bash | | ||
| | 🧪 **[test-engineer](test-engineer.md)** | Vitest, Cypress, React Testing Library, 80%+ coverage | view, edit, create, bash, search_code | | ||
| | 🔒 **[security-specialist](security-specialist.md)** | OSSF Scorecard, SLSA, OWASP, ISMS compliance | view, edit, bash, search_code | | ||
| | 📝 **[documentation-writer](documentation-writer.md)** | JSDoc, READMEs, Mermaid diagrams, ISMS docs | view, edit, create, search_code | | ||
| | 🎯 **[product-task-agent](product-task-agent.md)** | Product analysis, issue creation, agent coordination | view, edit, create, bash, search_code | |
There was a problem hiding this comment.
The “Tools” column here doesn’t match the actual tools: lists in the agent frontmatter files (e.g., game-developer.md includes custom-agent, but it’s omitted in this table). Either update the table to reflect the real tool sets or rename the column to indicate it’s a simplified/representative list.
|
@copilot apply changes based on the comments in this thread |
- testing-strategy: inline GameState type, add HUD import, note schematic paths
- documentation-standards: fix invalid `{ ... }` to `{ /* ... */ }`
- security-by-design: replace simplistic sanitizer with validate+encode pattern
- agents/README.md: add missing custom-agent tool to all agents
Agent-Logs-Url: https://github.com/Hack23/game/sessions/06fb86cf-2def-49bc-ab58-ddae655f26f1
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 5 review comments in
|
| .replace(/"/g, '"') | ||
| .replace(/'/g, '''); | ||
| } | ||
|
|
There was a problem hiding this comment.
The example uses userInput but it isn’t defined anywhere in the snippet, so it can’t be copy/pasted as-is. Consider either declaring a const userInput = ... in the example or rewriting the last two lines to take an explicit input parameter so the validation/escaping pipeline is self-contained.
| const userInput = ' Hello <script>alert("xss")</script> '; |
| ```tsx | ||
| import type { ReactNode } from 'react'; | ||
|
|
||
| ### ❌ Bad Pattern: No Type Safety in Mocks | ||
|
|
||
| ```typescript | ||
| // Bad: Untyped mocks lose TypeScript benefits | ||
| const mockFn = vi.fn(); // No type information! | ||
|
|
||
| mockFn.mockReturnValue('string'); // Should return AuthResult, but no error! | ||
|
|
||
| // Good: Typed mocks | ||
| const mockFn = vi.fn<[UserCredentials], Promise<AuthResult>>(); | ||
| vi.mock('@react-three/fiber', () => ({ | ||
| Canvas: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| useFrame: vi.fn(), | ||
| useThree: () => ({ camera: {}, scene: {}, gl: {} }), | ||
| })); |
There was a problem hiding this comment.
The Three.js mock example calls vi.mock(...) but doesn’t import vi from Vitest, so the snippet is incomplete. Add the vi import (or explicitly note it relies on a shared test setup) to keep the example runnable when copied into a test file.
| expect(handleClick).toHaveBeenCalledOnce(); | ||
| }); | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
This fenced block doesn’t specify a language. Since other docs in this repo explicitly require language identifiers for code fences, consider using something like ````text` here to keep formatting consistent and improve readability.
| ``` | |
| ```text |
|
|
||
| ### Skill Structure | ||
| Each skill follows this directory structure: | ||
| ``` |
There was a problem hiding this comment.
The directory tree is in a fenced block without a language identifier. If we’re enforcing “always specify code block languages” elsewhere, consider using a language like text for consistency.
| ``` | |
| ```text |
| ``` | ||
| @workspace Use the product-task-agent to analyze the codebase and create improvement issues | ||
|
|
||
| @workspace Ask the product-task-agent to review UI/UX and create accessibility issues | ||
|
|
||
| @workspace Use the game-developer agent to create a new Three.js sprite component | ||
|
|
||
| @workspace Ask the security-specialist to review this dependency for vulnerabilities | ||
|
|
||
| @workspace Have the test-engineer write Vitest tests for this component | ||
|
|
||
| @workspace Request the frontend-specialist to refactor this with React hooks | ||
|
|
||
| @workspace Use the documentation-writer to create JSDoc comments for this API | ||
| @workspace Use the game-developer agent to add a new particle effect | ||
| @workspace Ask the test-engineer to improve test coverage for GameScene | ||
| @workspace Have the security-specialist review dependencies | ||
| ``` |
There was a problem hiding this comment.
This example block doesn’t specify a language. Consider using a text (or similar) language tag on the fence to match the repo’s documentation standards around labeled code blocks.
|
@copilot apply changes based on the comments in this thread |
…xt language to code fences Agent-Logs-Url: https://github.com/Hack23/game/sessions/0eab4b08-04a5-40ed-b90d-38b720826eab Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 5 review comments in
Applied all 5 review comments in
|
| catch (error: unknown) { | ||
| console.error('Internal error:', error); | ||
| return { error: 'An error occurred. Please try again.' }; |
There was a problem hiding this comment.
The “Secure Error Handling” example is not valid TypeScript as written because it starts with a standalone catch block without a corresponding try. Please rewrite the snippet to be syntactically complete (e.g., show the surrounding try { ... } catch { ... }), or adjust it to a complete function example so readers can copy/paste it without errors.
| catch (error: unknown) { | |
| console.error('Internal error:', error); | |
| return { error: 'An error occurred. Please try again.' }; | |
| function handleRequest(): { success?: true; error?: string } { | |
| try { | |
| // Perform the requested operation here | |
| return { success: true }; | |
| } catch (error: unknown) { | |
| console.error('Internal error:', error); | |
| return { error: 'An error occurred. Please try again.' }; | |
| } |
| // Bad: Committing .env file with secrets | ||
| // .env file should be in .gitignore! | ||
| // BAD: Hardcoded secret | ||
| const API_KEY = "sk-abc123"; |
There was a problem hiding this comment.
Even though this is an anti-pattern example, using a realistic-looking API key string (e.g., "sk-abc123") can trigger secret-scanning or internal regex detectors and create noisy security alerts. Consider replacing it with an obviously fake placeholder (e.g., "<API_KEY>" or "sk_test_example") that won’t match common secret patterns.
| const API_KEY = "sk-abc123"; | |
| const API_KEY = "<API_KEY>"; |
| ```markdown | ||
| # Game Template - React + TypeScript + Three.js | ||
|
|
||
| > Secure, high-performance 3D game template with React and Three.js | ||
|
|
||
| [](https://github.com/Hack23/game/actions) | ||
| [](LICENSE) | ||
| [](https://codecov.io/gh/Hack23/game) | ||
|
|
||
| ## 🔒 Security & Compliance | ||
|
|
||
| This project implements security controls aligned with [Hack23 AB's ISMS](https://github.com/Hack23/ISMS-PUBLIC): | ||
|
|
||
| - **ISO 27001:2022** - Information security management | ||
| - **NIST CSF 2.0** - Cybersecurity framework | ||
| - **CIS Controls v8.1** - Security best practices | ||
| - **OWASP Top 10** - Web application security | ||
|
|
||
| For detailed security information, see [SECURITY.md](SECURITY.md). | ||
|
|
||
| ## 🚀 Quick Start | ||
|
|
||
| ### Prerequisites | ||
|
|
||
| - Node.js 20.x or later | ||
| - npm 10.x or later | ||
| - Modern browser with WebGL 2.0 support | ||
|
|
||
| ### Installation | ||
|
|
||
| \`\`\`bash | ||
| # Clone repository | ||
| git clone https://github.com/Hack23/game.git | ||
| cd game | ||
|
|
||
| # Install dependencies | ||
| npm install | ||
|
|
||
| # Start development server | ||
| npm run dev | ||
| \`\`\` | ||
|
|
||
| Visit http://localhost:5173 to see the game. | ||
|
|
||
| ## 📦 Available Scripts | ||
|
|
||
| | Script | Description | ISMS Policy | | ||
| |--------|-------------|-------------| | ||
| | `npm run dev` | Start development server | - | | ||
| | `npm run build` | Production build | SC-001 | | ||
| | `npm run preview` | Preview production build | - | | ||
| | `npm run lint` | Run ESLint | SC-002 | | ||
| | `npm run test` | Run Vitest unit tests | SC-002 | | ||
| | `npm run test:e2e` | Run Cypress E2E tests | SC-002 | | ||
| | `npm run coverage` | Generate coverage report | SC-002 | | ||
| | `npm audit` | Check dependencies for vulnerabilities | SC-003 | | ||
|
|
||
| ## 🏗️ Architecture | ||
|
|
||
| \`\`\`mermaid | ||
| ```mermaid | ||
| graph TB | ||
| A[User Browser] --> B[React App] | ||
| B --> C[@react-three/fiber] | ||
| C --> D[Three.js] | ||
| B --> E[Game State Management] | ||
| E --> F[React Context] | ||
| E --> G[Local Storage] | ||
| B --> H[Security Layer] | ||
| H --> I[Input Validation] | ||
| H --> J[CSP Headers] | ||
| H --> K[XSS Protection] | ||
| \`\`\` | ||
|
|
||
| ## 🎮 Game Development | ||
|
|
||
| ### Creating a 3D Component | ||
|
|
||
| \`\`\`typescript | ||
| import { useRef } from 'react'; | ||
| import { useFrame } from '@react-three/fiber'; | ||
| import * as THREE from 'three'; | ||
|
|
||
| interface PlayerProps { | ||
| position: [number, number, number]; | ||
| } | ||
|
|
||
| export function Player({ position }: PlayerProps): JSX.Element { | ||
| const meshRef = useRef<THREE.Mesh>(null); | ||
|
|
||
| useFrame((state, delta) => { | ||
| if (meshRef.current) { | ||
| meshRef.current.rotation.y += delta; | ||
| } | ||
| }); | ||
|
|
||
| return ( | ||
| <mesh ref={meshRef} position={position}> | ||
| <sphereGeometry args={[0.5, 32, 32]} /> | ||
| <meshStandardMaterial color="#00ff00" /> | ||
| </mesh> | ||
| ); | ||
| } | ||
| \`\`\` | ||
|
|
||
| See [Game Development Guide](docs/GAME_DEVELOPMENT.md) for more examples. | ||
|
|
||
| ## 🧪 Testing | ||
|
|
||
| We maintain 80%+ code coverage with three testing layers: | ||
|
|
||
| 1. **Unit Tests** (Vitest) - Component and logic testing | ||
| 2. **Integration Tests** (Vitest) - Multi-component interactions | ||
| 3. **E2E Tests** (Cypress) - Full game flow testing | ||
|
|
||
| \`\`\`bash | ||
| # Run all tests with coverage | ||
| npm run coverage | ||
|
|
||
| # Run specific test file | ||
| npm run test -- Player.test.tsx | ||
|
|
||
| # Run E2E tests in interactive mode | ||
| npm run test:e2e:open | ||
| \`\`\` | ||
|
|
||
| ## 📊 Code Quality | ||
|
|
||
| | Metric | Target | Current | | ||
| |--------|--------|---------| | ||
| | Code Coverage | ≥ 80% | 85% | | ||
| | TypeScript Strict | ✅ Enabled | ✅ | | ||
| | Security Headers | ✅ All | ✅ | | ||
| | Performance (FPS) | ≥ 60 | 60 | | ||
|
|
||
| ## 🔐 Security | ||
|
|
||
| ### Reporting Vulnerabilities | ||
|
|
||
| See [SECURITY.md](SECURITY.md) for vulnerability reporting process. | ||
|
|
||
| ### Security Features | ||
|
|
||
| - ✅ Content Security Policy (CSP) | ||
| - ✅ HTTPS-only (HSTS) | ||
| - ✅ XSS Protection | ||
| - ✅ Input validation and sanitization | ||
| - ✅ Dependency scanning (npm audit) | ||
| - ✅ License compliance checking | ||
|
|
||
| ## 📝 License | ||
|
|
||
| MIT License - see [LICENSE](LICENSE) file for details. | ||
|
|
||
| ## 🤝 Contributing | ||
|
|
||
| See [CONTRIBUTING.md](CONTRIBUTING.md) for contribution guidelines. | ||
|
|
||
| ## 📚 Documentation | ||
|
|
||
| - [Game Development Guide](docs/GAME_DEVELOPMENT.md) | ||
| - [API Reference](docs/API.md) | ||
| - [Architecture Decision Records](docs/adr/) | ||
| - [Security Documentation](SECURITY.md) | ||
| App[App.tsx] --> Canvas[Canvas] | ||
| App --> HUD[HUD] | ||
| App --> Overlay[GameOverlay] | ||
| Canvas --> Scene[GameScene] | ||
| Scene --> Target[TargetSphere] | ||
| Scene --> Particles[BackgroundParticles] | ||
| App --> useGameState[useGameState] | ||
| App --> useAudio[useAudioManager] | ||
| ``` | ||
|
|
||
| ### ✅ Good Pattern: Mermaid Diagram for Security Flow | ||
|
|
||
| ```markdown | ||
| ## Authentication Flow | ||
|
|
||
| \`\`\`mermaid | ||
| sequenceDiagram | ||
| participant U as User | ||
| participant F as Frontend | ||
| participant A as Auth Service | ||
| participant D as Database | ||
| participant L as Audit Log | ||
|
|
||
| U->>F: Enter credentials | ||
| F->>F: Validate input format | ||
| F->>A: POST /api/auth/login | ||
|
|
||
| Note over A: ISMS Policy AC-001 | ||
| A->>D: Check credentials | ||
| D-->>A: User data | ||
|
|
||
| alt Valid Credentials | ||
| A->>A: Generate JWT token | ||
| A->>L: Log success | ||
| A-->>F: {token, user} | ||
| F-->>U: Redirect to game | ||
| else Invalid Credentials | ||
| A->>L: Log failure | ||
| A->>A: Increment fail count | ||
| alt Account Locked | ||
| A-->>F: 423 Account Locked | ||
| F-->>U: Show error + contact support | ||
| else Not Locked | ||
| A-->>F: 401 Unauthorized | ||
| F-->>U: Show error | ||
| end | ||
| end | ||
|
|
||
| Note over L: Compliance: ISO 27001 A.5.15 | ||
| \`\`\` | ||
| ``` |
There was a problem hiding this comment.
The Mermaid example uses nested fenced code blocks (markdown containing an inner mermaid). In Markdown this terminates the outer fence early, so the example won’t render correctly when copied. Prefer showing just a single ```mermaid block, or use a longer outer fence (e.g., ````markdown) so the inner triple-backticks remain intact.
|
@copilot apply changes based on the comments in this thread and Test and Report / e2e-tests (pull_request) |
…older, fix nested Mermaid fences Agent-Logs-Url: https://github.com/Hack23/game/sessions/954525ae-9b03-4517-b6d7-5dfb60b77c3a Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 3 review comments in
The e2e-tests CI failure is unrelated to this PR — no source code, tests, or config files were changed (only |
| ```typescript | ||
| // BAD: Hardcoded secret | ||
| const API_KEY = "<API_KEY>"; | ||
|
|
||
| ## Remember | ||
| // BAD: Leaking stack trace | ||
| res.status(500).json({ error: error.stack }); | ||
|
|
||
| - **Security is Everyone's Responsibility**: Every developer must understand and implement security controls | ||
| - **Assume Breach**: Design for compromise - limit blast radius and enable detection | ||
| - **Defense in Depth**: Never rely on a single security control - layer multiple controls | ||
| - **Fail Securely**: When things break, they should break in a secure state | ||
| - **Least Privilege**: Grant only the minimum permissions required | ||
| - **Zero Trust**: Verify everything, trust nothing | ||
| - **Secure Defaults**: Out-of-the-box configuration must be secure | ||
| - **No Secrets in Code**: Use environment variables and secret management systems | ||
| - **Encrypt Everything**: Data at rest (AES-256) and in transit (TLS 1.3+) | ||
| - **Validate All Input**: Client-side UX, server-side security | ||
| - **Audit Everything**: Log security events for compliance and forensics | ||
| - **Test Security**: Security tests are as important as functional tests | ||
| - **Stay Updated**: Patch dependencies regularly to fix known vulnerabilities | ||
| - **Document Decisions**: Threat models, security architecture, and risk decisions | ||
| - **Compliance is Continuous**: Security is not a one-time activity but an ongoing process | ||
| // BAD: No input validation | ||
| document.innerHTML = userInput; // XSS! |
There was a problem hiding this comment.
In the Anti-Patterns snippet, res, error, and userInput are undefined and document.innerHTML is not a valid DOM API. Even for “bad” examples, keeping snippets syntactically valid reduces copy/paste confusion—consider wrapping in a small handler with declared variables and using element.innerHTML/document.body.innerHTML to demonstrate the XSS issue accurately.
| } catch (error: unknown) { | ||
| console.error('Internal error:', error); |
There was a problem hiding this comment.
The “Secure Error Handling” example logs the full error via console.error, which is not “internal” in a browser context and can leak details to end users. Consider adjusting the guidance/snippet to reflect the intended environment (e.g., server-side logging/monitoring) or to redact details in client logs while keeping user-facing errors generic.
| } catch (error: unknown) { | |
| console.error('Internal error:', error); | |
| } catch { | |
| // In browser code, avoid logging raw error details to the console because | |
| // end users can access them. Send detailed diagnostics to restricted | |
| // server-side logs or monitoring instead. | |
| console.warn('Request failed'); |
| // which ISMS policy it implements, or compliance mapping | ||
| return encrypt(data); | ||
| } | ||
| // BAD: Broken/untested example in JSDoc |
There was a problem hiding this comment.
The Anti-Patterns block ends right after “Broken/untested example in JSDoc” without actually showing an example of a broken JSDoc snippet. Either add a minimal broken JSDoc example (so the reader knows what to avoid) or remove that line to avoid confusion.
| // BAD: Broken/untested example in JSDoc | |
| // BAD: Broken/untested example in JSDoc | |
| /** | |
| * Doubles a number. | |
| * | |
| * @example | |
| * ```typescript | |
| * calc("2"); // 4 | |
| * ``` | |
| */ | |
| export function calc(value: number): number { return value * 2; } |
| interface HUDProps { | ||
| score: number; | ||
| health: number; | ||
| ammo: number; | ||
| onPause: () => void; | ||
| } | ||
|
|
||
| /** | ||
| * Memoized HUD component - only re-renders when props change | ||
| * Prevents expensive re-renders during 60fps game loop | ||
| */ | ||
| export const GameHUD: FC<GameHUDProps> = memo(({ | ||
| score, | ||
| health, | ||
| ammo, | ||
| onPause | ||
| }) => { | ||
| // Memoize expensive color calculation | ||
| const healthColor = useMemo(() => { | ||
| if (health > 75) return '#00ff00'; | ||
| if (health > 25) return '#ffff00'; | ||
| return '#ff0000'; | ||
| }, [health]); | ||
|
|
||
| // Memoize event handler to prevent child re-renders | ||
| const handlePause = useCallback(() => { | ||
| onPause(); | ||
| }, [onPause]); | ||
|
|
||
| export const GameHUD = memo(({ score, health, onPause }: HUDProps): JSX.Element => { | ||
| const healthColor = useMemo(() => health > 50 ? '#0f0' : '#f00', [health]); | ||
| const handlePause = useCallback(() => onPause(), [onPause]); | ||
|
|
||
| return ( | ||
| <div className="game-hud"> | ||
| <div className="score">Score: {score}</div> | ||
| <div className="health" style={{ color: healthColor }}> | ||
| Health: {health}% | ||
| </div> | ||
| <div className="ammo">Ammo: {ammo}</div> | ||
| <div className="hud"> | ||
| <span>Score: {score}</span> | ||
| <span style={{ color: healthColor }}>Health: {health}%</span> | ||
| <button onClick={handlePause}>Pause</button> | ||
| </div> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The “Memoized Component” example exports a component as export const ... = memo(...), which conflicts with the repo’s documented preference for function Component(): JSX.Element style components in .github/copilot-instructions.md and existing components (e.g., src/components/HUD.tsx). Consider rewriting the example to match the project’s component style to avoid teaching two different conventions.
|
@copilot apply changes based on the comments in this thread |
…ror handling, broken JSDoc example, component style Agent-Logs-Url: https://github.com/Hack23/game/sessions/dbbb1a76-2efc-451a-837e-1baef34791dc Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 4 review comments in
|
| ### ✅ Good Pattern: Typed 3D Component with Game Logic | ||
| ### ✅ Typed Game Object with useFrame | ||
|
|
||
| ```typescript |
There was a problem hiding this comment.
The example uses a TSX/JSX snippet but the code fence is labeled typescript, which breaks syntax highlighting and encourages copy/paste into the wrong file type. Change this fence to tsx to match the JSX usage in the example.
| ```typescript | |
| ```tsx |
| <mesh position={[0, 0, 0]}> {/* Creates new array each render */} | ||
| {/* GOOD: const POS = [0, 0, 0] as const; outside component */} |
There was a problem hiding this comment.
This anti-pattern JSX snippet is not syntactically valid: the <mesh> opening tag is never closed, and the following {/* GOOD: ... */} comment is outside any JSX element. Even for anti-patterns, keep snippets parsable so readers can understand them and so the docs don’t inadvertently teach broken syntax.
| <mesh position={[0, 0, 0]}> {/* Creates new array each render */} | |
| {/* GOOD: const POS = [0, 0, 0] as const; outside component */} | |
| <mesh position={[0, 0, 0]}> | |
| {/* Creates new array each render */} | |
| {/* GOOD: const POS = [0, 0, 0] as const; outside component */} | |
| </mesh> |
| 7. **Test Edge Cases**: Boundary conditions, empty inputs, null/undefined | ||
| 8. **React Testing Library**: Query by role/text, not implementation details | ||
| 9. **Test Logic Not Rendering**: For Three.js, test state/logic, mock renderer | ||
| 10. **Isolate Tests**: Independent, no shared state, `vi.clearAllMocks()` in beforeEach |
There was a problem hiding this comment.
Rule 8 recommends querying by role/text and avoiding implementation details, but this repository’s existing React tests predominantly use data-testid + getByTestId (e.g. src/components/HUD.test.tsx, src/App.test.tsx). To keep the skill aligned with established repo patterns, consider explicitly allowing/prefering data-testid selectors when components expose stable test IDs.
| Canvas: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| useFrame: vi.fn(), | ||
| useThree: () => ({ camera: {}, scene: {}, gl: {} }), | ||
| })); |
There was a problem hiding this comment.
The Three.js unit-test mock example only mocks @react-three/fiber, but many components in this repo import from @react-three/drei (e.g. TargetSphere uses Sparkles/Trail). Add a companion mock for @react-three/drei (as done in src/App.test.tsx / src/components/GameScene.test.tsx) so the snippet is complete and copy/pasteable.
| })); | |
| })); | |
| vi.mock('@react-three/drei', () => ({ | |
| Sparkles: ({ children }: { children?: ReactNode }) => <>{children}</>, | |
| Trail: ({ children }: { children?: ReactNode }) => <>{children}</>, | |
| })); |
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/Hack23/game/sessions/70d82f00-9ba0-49be-bda1-ad250a470700 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied the requested thread updates in
Validation: UI screenshot: N/A — documentation-only changes, no UI/runtime changes. |
.github/skills/react-threejs-game/SKILL.mdcode fence language for JSX snippet (typescript→tsx).github/skills/performance-optimization/SKILL.mdto remain parsable.github/skills/testing-strategy/SKILL.mdrule guidance with repo practice by explicitly allowing stabledata-testidselectors@react-three/dreicompanion mock example in.github/skills/testing-strategy/SKILL.mdCI note: The previously referenced e2e failure was in run
24005530950on commitb08b05c; newer runs on this branch (e.g. run24006953517onc4ed74f) are passing.