Skip to content

Conversation

@tommaso-moro
Copy link
Contributor

@tommaso-moro tommaso-moro commented Jan 22, 2026

Summary

This PR moves server instructions generation from NewMCPServer into the Inventory pattern, so instructions are computed at inventory build time rather than server creation time.

Note: This carries over changes from #1863 (which targets http-stack-2) to main, excluding the HTTP handler change which depends on the HTTP stack.

Changes

  • Added instructions field to Inventory struct with Instructions() getter method
  • Added InstructionsFunc field to ToolsetMetadata allowing each toolset to define its own instructions
  • Added WithServerInstructions() builder method that enables instruction generation during Build()
  • Created pkg/github/toolset_instructions.go with instruction functions for each toolset (context, issues, pull_requests, discussions, projects)
  • Moved instruction generation to pkg/inventory/instructions.go , which first adds base instructions and then iterates over toolsets and calls their InstructionsFunc
  • Updated STDIO path to use .WithServerInstructions() when building inventory
  • Updated NewMCPServer to use inventory.Instructions() instead of generating instructions inline

Testing

  • Unit tests moved to pkg/inventory/instructions_test.go
  • Manually verified STDIO server with/without .WithServerInstructions()

Demo

Screen.Recording.2026-01-22.at.17.24.48.mov

Why

This aligns with the architectural direction where the Inventory holds all pre-computed configuration, and the MCP server receives a fully-configured inventory rather than creating one itself. Instructions are generated based on the resolved enabled toolsets (after processing "default", "all" keywords, etc.)

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

@tommaso-moro tommaso-moro requested a review from a team as a code owner January 22, 2026 17:26
Copilot AI review requested due to automatic review settings January 22, 2026 17:26
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 refactors server instructions generation to align with the Inventory pattern, moving instruction computation from server creation time (NewMCPServer) to inventory build time. Instructions are now generated based on resolved enabled toolsets and accessed via a getter method.

Changes:

  • Added InstructionsFunc field to ToolsetMetadata allowing each toolset to define context-aware instructions
  • Introduced WithServerInstructions() builder method enabling instruction generation during Build()
  • Moved instruction generation logic from pkg/github to pkg/inventory with corresponding test migration
  • Updated toolset metadata to include instruction functions for context, issues, pull_requests, discussions, and projects toolsets
  • Modified NewMCPServer to retrieve instructions from the built inventory instead of generating them inline

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/inventory/server_tool.go Adds InstructionsFunc field to ToolsetMetadata for per-toolset instruction generation
pkg/inventory/registry.go Adds instructions field and Instructions() getter method to Inventory
pkg/inventory/builder.go Adds WithServerInstructions() method to enable instruction generation during build
pkg/inventory/instructions.go New file containing generateInstructions() logic that combines base instructions with toolset-specific instructions
pkg/inventory/instructions_test.go New comprehensive test file covering instruction generation with various toolset configurations
pkg/github/toolset_instructions.go Refactored from monolithic function to individual per-toolset instruction functions
pkg/github/tools.go Updated toolset metadata to include InstructionsFunc assignments for applicable toolsets
pkg/github/instructions_test.go Deleted - tests migrated to pkg/inventory
internal/ghmcp/server.go Updated to build inventory with .WithServerInstructions() and retrieve instructions via inventory.Instructions()
pkg/github/toolsnaps/assign_copilot_to_issue.snap Minor field ordering change (description/type swap) from toolsnap update
Comments suppressed due to low confidence (1)

pkg/github/toolset_instructions.go:106

  • The instruction functions (generateContextToolsetInstructions, generateIssuesToolsetInstructions, generatePullRequestsToolsetInstructions, generateDiscussionsToolsetInstructions, generateProjectsToolsetInstructions) are unexported (lowercase first letter), but they are used as function pointers in the toolset metadata that is exported and used by other parts of the codebase. According to the project guidelines, this repository is used as a library by the remote server, so functions that could be called by other repositories should be exported (capitalized). These functions should be exported to maintain consistency with the library usage pattern and allow potential external use of the toolset metadata with custom inventory builders.

@tommaso-moro tommaso-moro merged commit 15e66b3 into main Jan 23, 2026
16 checks passed
@tommaso-moro tommaso-moro deleted the tommy/inventory-instructions branch January 23, 2026 09:57
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.

4 participants