Conversation
…ENDUM_FILE instructions: in mcp.yaml is a single scalar. A deployment that wants to add its own operating rules (data quality, duplicate-ticket handling, local workflow rules) currently has no way to add to the base text without copying it wholesale into a fork's own config - every wording change made here then has to be manually re-merged by hand, or drifts silently out of sync. Adds an optional APP_INSTRUCTIONS_ADDENDUM_FILE env var (empty by default, no behavior change). When set to a path relative to the project dir, its content is appended to the configured instructions when the server is built. Meant to be set in .env.local (or the deployment's own env), pointing at a file the deployment owns - no tracked file needs editing. iTopBuilder::setInstructions() now stores the base value instead of forwarding it immediately, and build() appends the addendum (if any) right before building the server - mirroring how getDiscoveryDirs() already defers its own computation to build() time.
There was a problem hiding this comment.
Pull request overview
Adds deployment-specific MCP instruction addenda while preserving upstream base instructions.
Changes:
- Adds optional addendum-file environment configuration.
- Defers instruction assembly until server build time.
- Appends readable, non-empty addendum content.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.env |
Defines the optional addendum file variable. |
config/services.yaml |
Injects addendum configuration into the builder. |
src/Capability/iTopBuilder.php |
Loads and appends addendum instructions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+120
to
+123
| if (!is_file($path) || !is_readable($path)) { | ||
| return $this->instructions; | ||
| } | ||
| $addendum = trim(file_get_contents($path)); |
| if ($addendum === '') { | ||
| return $this->instructions; | ||
| } | ||
| return trim((string)$this->instructions)."\n\n".$addendum; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #11
Problem
instructions:inconfig/packages/mcp.yamlis a single scalar. A deployment thatwants to add its own operating rules (data quality, duplicate-ticket handling, local
workflow rules — anything that spans several tools and belongs in server instructions
rather than repeated tool docblocks) currently has no way to add to the base text
without copying it wholesale into their own fork's config. Every wording change made
here then has to be manually re-merged by hand on the fork side, or drifts silently out
of sync.
Solution
Adds an optional
APP_INSTRUCTIONS_ADDENDUM_FILEenv var, empty by default (no behaviorchange for existing deployments). When set to a path relative to the project directory,
its content is appended to the configured
instructionswhen the server is built. It'smeant to be set in
.env.local(or the deployment's own environment), pointing at a filethe deployment owns — no tracked file needs editing.
iTopBuilder::setInstructions()now stores the base value instead of forwarding itimmediately to the underlying builder, and
build()appends the addendum (if any) rightbefore building the server — the same pattern
getDiscoveryDirs()already uses to deferits own computation to build time.
Testing
Verified via a real
/_mcpinitializerequest, both with the env var unset (base textonly, unchanged) and set to a test file (base + addendum, correctly concatenated).
Existing test suite passes/fails identically before and after (pre-existing failures are
unrelated fixture/environment issues).