Repository navigation
Conversation
Return a configurable instructions string in both the legacy initialize result (2025-11-25) and the modern server/discover result (2026-07-28). Behaviour: - MCP_INSTRUCTIONS unset or absent → default text: "Fast Time Server. Use get_system_time for the current time. Use convert_time to convert a time between timezones." - MCP_INSTRUCTIONS set to a non-empty value → that value - MCP_INSTRUCTIONS set to empty string → field omitted from both The default text was wired directly into get_info() via with_instructions(); the rmcp SDK already propagates instructions from ServerConfig (InitializeResult) into DiscoverResult through DiscoverResult::from_server_info(), so both eras are covered by the single field. Six acceptance-criteria tests added: - default text present in initialize - default text present in server/discover - empty env var omits field in initialize - empty env var omits field in server/discover - custom value returned in initialize - custom value returned in server/discover README updated with MCP_INSTRUCTIONS variable documentation. Closes #57 Signed-off-by: Jonathan Springer <jps@s390x.com>
lucarlig
approved these changes
Oct 15, 2026
lucarlig
left a comment
Contributor
There was a problem hiding this comment.
Reviewed commit 457061e. No blocking issues found.
Verified top-level instructions in both initialize and server/discover: exact required default, custom value, and complete field omission for empty MCP_INSTRUCTIONS. Confirmed configuration and discovery propagation against the locked rmcp 3.5.0 source.
Validation: cargo test --locked (51 passed), cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, and cargo build --locked. Ran the built server with MCP_INSTRUCTIONS unset, empty, and custom and checked both endpoints over HTTP (6 passed). All 7 GitHub checks passed.
jonpspri
added a commit
to IBM/mcp-context-forge
that referenced
this pull request
Oct 16, 2026
Add black-box tests for the `instructions` text of a virtual server. Each scenario runs in the legacy era (`initialize`) and the modern era (`server/discover`). The scenarios cover a single upstream server, a subset of its tools, a mix of upstream and local tools, an override, an empty override, an update through PUT, and the global endpoint. Pin the fast-time-server image in `docker-compose.yml` to the build that sends `instructions` (IBM/contextforge-examples#58, closes #57). The image exposes one more tool, `whoami`, which needs no change to the benchmark denylist. The tests expect the fast-time-server text from a single-upstream virtual server. `FAST_TIME_INSTRUCTIONS` selects another expected text. Document the resolution order, the single-upstream rule, and the meaning of `null` and an empty string in the REST API guide. State that upstream text changes at the next refresh and that a client can put it in a model prompt. Add a CHANGELOG entry. Part of #4693 Signed-off-by: Jonathan Springer <jps@s390x.com>
Open
9 tasks done
jonpspri
added a commit
to IBM/mcp-context-forge
that referenced
this pull request
Oct 16, 2026
Add black-box tests for the `instructions` text of a virtual server. Each scenario runs in the legacy era (`initialize`) and the modern era (`server/discover`). The scenarios cover a single upstream server, a subset of its tools, a mix of upstream and local tools, an override, an empty override, an update through PUT, and the global endpoint. Pin the fast-time-server image in `docker-compose.yml` to the build that sends `instructions` (IBM/contextforge-examples#58, closes #57). The image exposes one more tool, `whoami`, which needs no change to the benchmark denylist. The tests expect the fast-time-server text from a single-upstream virtual server. `FAST_TIME_INSTRUCTIONS` selects another expected text. Document the resolution order, the single-upstream rule, and the meaning of `null` and an empty string in the REST API guide. State that upstream text changes at the next refresh and that a client can put it in a model prompt. Add a CHANGELOG entry. Part of #4693 Signed-off-by: Jonathan Springer <jps@s390x.com>
jonpspri
added a commit
to IBM/mcp-context-forge
that referenced
this pull request
Oct 16, 2026
Add black-box tests for the `instructions` text of a virtual server. Each scenario runs in the legacy era (`initialize`) and the modern era (`server/discover`). The scenarios cover a single upstream server, a subset of its tools, a mix of upstream and local tools, an override, an empty override, an update through PUT, and the global endpoint. Pin the fast-time-server image in `docker-compose.yml` to the build that sends `instructions` (IBM/contextforge-examples#58, closes #57). The image exposes one more tool, `whoami`, which needs no change to the benchmark denylist. The tests expect the fast-time-server text from a single-upstream virtual server. `FAST_TIME_INSTRUCTIONS` selects another expected text. Document the resolution order, the single-upstream rule, and the meaning of `null` and an empty string in the REST API guide. State that upstream text changes at the next refresh and that a client can put it in a model prompt. Add a CHANGELOG entry. Part of #4693 Signed-off-by: Jonathan Springer <jps@s390x.com>
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 #57
Summary
Returns a configurable
instructionsstring in both the legacyinitializeresult (2025-11-25) and the modernserver/discoverresult (2026-07-28).Behaviour
MCP_INSTRUCTIONSinstructionsfield"Fast Time Server. Use get_system_time for the current time. Use convert_time to convert a time between timezones."""Implementation notes
DEFAULT_INSTRUCTIONSconstant.instructions_from_env()readsMCP_INSTRUCTIONSat startup and returnsOption<String>.FastTimeServerand used inget_info()viawith_instructions().instructionsfromServerConfig(alias forInitializeResult) intoDiscoverResult::from_server_info(), so both eras are covered by a single field with no extra code.Tests added (6 new)
initializeserver/discoverinitializeserver/discoverinitializeserver/discoverAll 51 tests pass.
cargo clippy --all-targetsandcargo fmtare clean.