Skip to content

Conversation

@chrisreddington
Copy link
Contributor

This pull request makes significant improvements to type safety, code consistency, and developer tooling across the project, especially focusing on centralizing constants and types, removing duplication, and refactoring the testing and linting infrastructure. The most important changes are summarized below.

Type Safety and Constants Consolidation

  • Centralizes key constants (DIFFICULTIES, GAME_TYPES, PLAYER_IDS, etc.) in the shared library, and enforces that all types like Difficulty, GameType, and PlayerId are derived from these constants using as const assertions. Updates copilot instructions to import these types instead of redefining union types.
  • Updates function signatures and AI logic to use the shared Difficulty type, ensuring consistent difficulty handling across all games.

Testing Infrastructure Refactoring

  • Migrates the MCP server package from Jest to Vitest for testing, removing all Jest-related configuration and setup files, and updating test scripts in package.json.
  • Documents the use of shared testing utilities and mock data from the shared package, discouraging local duplication of test data.

Linting and Developer Tooling

  • Adds an ESLint configuration at the root and in the MCP server package, using typescript-eslint for improved TypeScript linting and code quality enforcement. Customizes rules for the MCP server and adds ignore patterns.

Code Consistency and Minor Refactoring

  • Cleans up test files to use imported types and constants, removes unnecessary type imports, and clarifies variable usage for better readability and maintainability.

Build and Runtime Adjustments

  • Updates server entry points and script references in both the VSCode workspace and package.json to use server.js instead of index.js, aligning with the new build output.

These changes collectively improve maintainability and type safety across the codebase.

chrisreddington and others added 30 commits August 4, 2025 16:49
Initial elicitation implementation and tool call consolidation.
Tech Debt: Consolidate hardcoded constants and eliminate duplicate difficulty styling
Consolidate duplicate code patterns and remove dead code
chrisreddington and others added 9 commits August 7, 2025 18:56
Fix ESLint Errors in Shared Library Workspace
Bumps the npm-development group with 4 updates: [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node), [typescript](https://github.com/microsoft/TypeScript), [eslint-config-next](https://github.com/vercel/next.js/tree/HEAD/packages/eslint-config-next) and [undici](https://github.com/nodejs/undici).


Updates `@types/node` from 24.1.0 to 24.2.0
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

Updates `typescript` from 5.8.3 to 5.9.2
- [Release notes](https://github.com/microsoft/TypeScript/releases)
- [Changelog](https://github.com/microsoft/TypeScript/blob/main/azure-pipelines.release-publish.yml)
- [Commits](microsoft/TypeScript@v5.8.3...v5.9.2)

Updates `eslint-config-next` from 15.4.5 to 15.4.6
- [Release notes](https://github.com/vercel/next.js/releases)
- [Changelog](https://github.com/vercel/next.js/blob/canary/release.js)
- [Commits](https://github.com/vercel/next.js/commits/v15.4.6/packages/eslint-config-next)

Updates `undici` from 7.12.0 to 7.13.0
- [Release notes](https://github.com/nodejs/undici/releases)
- [Commits](nodejs/undici@v7.12.0...v7.13.0)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-version: 24.2.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: npm-development
- dependency-name: typescript
  dependency-version: 5.9.2
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: npm-development
- dependency-name: eslint-config-next
  dependency-version: 15.4.6
  dependency-type: direct:development
  update-type: version-update:semver-patch
  dependency-group: npm-development
- dependency-name: undici
  dependency-version: 7.13.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: npm-development
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps the npm-production group with 2 updates: [next](https://github.com/vercel/next.js) and [@modelcontextprotocol/sdk](https://github.com/modelcontextprotocol/typescript-sdk).


Updates `next` from 15.4.5 to 15.4.6
- [Release notes](https://github.com/vercel/next.js/releases)
- [Changelog](https://github.com/vercel/next.js/blob/canary/release.js)
- [Commits](vercel/next.js@v15.4.5...v15.4.6)

Updates `@modelcontextprotocol/sdk` from 1.17.0 to 1.17.1
- [Release notes](https://github.com/modelcontextprotocol/typescript-sdk/releases)
- [Commits](modelcontextprotocol/typescript-sdk@1.17.0...1.17.1)

---
updated-dependencies:
- dependency-name: next
  dependency-version: 15.4.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: npm-production
- dependency-name: "@modelcontextprotocol/sdk"
  dependency-version: 1.17.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: npm-production
...

Signed-off-by: dependabot[bot] <[email protected]>
…pm-development-1fad86d8f6

Bump the npm-development group with 4 updates
…pm-production-1b125f1a35

Bump the npm-production group with 2 updates
Copilot AI review requested due to automatic review settings August 7, 2025 19:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR significantly modernizes the codebase by migrating from Jest to Vitest, improving type safety through centralized constants, and refactoring testing infrastructure across all packages. The main focus is on eliminating duplication and establishing consistent patterns.

Key changes include:

  • Migration from Jest to Vitest testing framework with shared test utilities
  • Centralization of game constants and types in the shared package
  • Addition of ESLint configuration and interactive elicitation features

Reviewed Changes

Copilot reviewed 93 out of 96 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
web/vitest.setup.ts New Vitest setup replacing Jest configuration
web/vitest.config.ts Vitest configuration with coverage thresholds
web/package.json Updated dependencies from Jest to Vitest
shared/src/constants/game-constants.ts New centralized constants module
shared/src/testing/vitest-setup.ts Shared test setup utilities
mcp-server/src/handlers/elicitation-handlers.ts New interactive user preference collection
mcp-server/src/handlers/tool-handlers.ts Refactored to use centralized constants
Comments suppressed due to low confidence (1)

mcp-server/src/utils/http-client.ts:48

  • The type casting '(games as GenericGameStateWrapper[])' assumes the API response structure without validation. Consider adding runtime type checking or using a type guard to ensure the response matches the expected structure before casting.
export async function getGameViaAPI(gameType: string, gameId: string): Promise<GenericGameStateWrapper | undefined> {
  try {
    const games = await httpGet(`${WEB_API_BASE}/api/games/${gameType}/mcp`)
  return (games as GenericGameStateWrapper[]).find((game) => game.gameState?.id === gameId)
  } catch (error) {
    console.error(`Error fetching ${gameType} game via API:`, error)
    return undefined
  }
}

@chrisreddington chrisreddington merged commit fe1d54d into github-samples:main Aug 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant