Conversation
WalkthroughA new container named Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant ShellScript
participant Podman
participant mcphost Container
User->>Makefile: make mcphost
Makefile->>ShellScript: ./scripts/mcphost.sh
ShellScript->>Podman: podman attach assisted-chat-pod-mcphost
Podman->>mcphost Container: Attach session
User-->>mcphost Container: Interact via attached session
sequenceDiagram
participant mcphost Container
participant Config File
participant System Prompt File
participant Assisted MCP Server
mcphost Container->>Config File: Read /mcpconfig.json (SSE URL)
mcphost Container->>System Prompt File: Read /systemprompt.txt
mcphost Container->>Assisted MCP Server: Connect via SSE (http://assisted-service-mcp:8000/sse)
Assisted MCP Server-->>mcphost Container: Stream events/responses
mcphost Container-->>User: Provide assistant responses per system prompt
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
assisted-chat-pod.yaml (1)
60-64:hostPathvolumes reduce portability and require elevated privilegesThe relative
./confighostPath ties the pod to a specific node filesystem and blocks scheduling on other nodes. A ConfigMap is better suited for static, text-based configuration files and works on any cluster.
🧹 Nitpick comments (7)
config/mcphost-mcp.json (1)
4-5: Externalize the MCP endpoint to avoid hard-coded URLsHard-coding the transport & URL couples the image to one specific deployment and prevents reuse in other environments (staging, prod, different namespaces). Mounting this JSON from a ConfigMap or templating the value from an env var keeps the container image generic.
config/mcphost-systemprompt.txt (2)
1-6: Remove duplicate identity sentences to reduce prompt tokensLines 1-3 and 4-6 repeat (almost) the same identity statement, wasting context window tokens and increasing latency/cost for every request. Consolidate the wording into a single paragraph.
13-15: Add commas to clarify the conditional clausesMissing commas after the introductory “If …” clauses makes the long sentence harder to parse and trips basic grammar checks.
-If the user provides a specific value for a parameter (for example provided in quotes), make sure to use that value EXACTLY. -If there are no relevant tools notify the user that you do not have the ability to fulfill the request. -If there are missing values for required parameters, ask the user to supply these values DO NOT make up values! +If the user provides a specific value for a parameter (for example, provided in quotes), make sure to use that value EXACTLY. +If there are no relevant tools, notify the user that you can’t fulfill the request. +If any required parameters are missing, ask the user to supply them — DO NOT make up values!mcphost.sh (1)
3-5: Gracefully handle a missing or restarting container
podman attachexits with 125 when the container is not running, which is common given the PR note about automatic restarts. Consider a tiny loop that waits for the container to exist & be running before attaching, or fall back topodman logs -ffor quick inspection.assisted-chat-pod.yaml (3)
40-40: Strip trailing whitespace to satisfy yaml-lint- image: quay.io/otuchfel/mcphost:0.9.2 + image: quay.io/otuchfel/mcphost:0.9.2
53-59: Mount the config files read-onlyNothing in the container should need to mutate
/mcpconfig.jsonor/systemprompt.txt. Marking the mounts read-only protects against accidental writes and potential privilege-escalation vectors.- - mountPath: /mcpconfig.json + - mountPath: /mcpconfig.json name: config subPath: mcphost-mcp.json + readOnly: true - - mountPath: /systemprompt.txt + - mountPath: /systemprompt.txt name: config subPath: mcphost-systemprompt.txt + readOnly: true
51-52: Consider sourcingGEMINI_API_KEYfrom a Secret instead of env-substPlacing the API key directly into the rendered pod spec (even via
${…}substitution) risks accidental commit or log exposure. KubernetesSecretobjects +envFromorvalueFrom: secretKeyRefprovide safer handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assisted-chat-pod.yaml(1 hunks)config/mcphost-mcp.json(1 hunks)config/mcphost-systemprompt.txt(1 hunks)mcphost.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
config/mcphost-systemprompt.txt
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...value EXACTLY. If there are no relevant tools notify the user that you do not have th...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~14-~14: The phrase ‘do not have the ability to’ might be wordy. Consider using “can't”.
Context: ...relevant tools notify the user that you do not have the ability to fulfill the request. If there are missi...
(HAS_THE_ABILITY_TO)
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ave the ability to fulfill the request. If there are missing values for required p...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~15-~15: Possible missing comma found.
Context: ...arameters, ask the user to supply these values DO NOT make up values! Refuse to answe...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 YAMLlint (1.37.1)
assisted-chat-pod.yaml
[error] 40-40: trailing spaces
(trailing-spaces)
mcphost is a wrapper around a model to detect and make MCP calls, similar to how lightspeed-core would do it, except it comes with a simple TUI for developers to use. We can use it to play around with our system prompt / MCP / model combinations, without having to go through lightspeed-core or the assisted UI. This commit adds a new container to the assisted-chat-pod, which runs mcphost and configures it to use the assisted-service MCP server and Gemini, with the API keys and everything. Use `make mcphost` to attach to the mcphost container and interact with it. Note that if you attach to late, you might get a weird broken TUI that if you try to use it would crash mcphost. This is actually a good thing, because then it will automatically restart, at this point try to attach again (this time early enough) and wait until it starts and the TUI should be good to go.
| all things related to Openshift. Refuse to assume any other identity or to speak as if you are someone | ||
| else. | ||
|
|
||
| Example Input: |
There was a problem hiding this comment.
I added this for granite, gemini doesn't need this.
I think we can remove 13-15 as well
There was a problem hiding this comment.
This is just lorem ipsum for now as far as I'm concerned, can you please modify it in a separate PR?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
assisted-chat-pod.yaml (3)
40-40: Strip trailing whitespace on theimageline
YAML-lint is flagging a trailing space after the image tag (quay.io/otuchfel/mcphost:0.9.2␠). It is harmless at runtime but fails many linters and pre-commit hooks.- image: quay.io/otuchfel/mcphost:0.9.2␠ + image: quay.io/otuchfel/mcphost:0.9.2
50-52: Handle secrets viavalueFrominstead of literal${VAR}
Kubernetes does not interpolate shell-style${…}placeholders. The literal string${GEMINI_API_KEY}will be passed to the container unless the manifest is pre-processed externally (e.g.envsubst).
Use a Secret or ConfigMap reference instead:env: - name: GEMINI_API_KEY valueFrom: secretKeyRef: name: gemini-api-key # kubectl create secret … key: token
54-59: Mount config files read-only
These files are configuration, not writable state. Protect them by addingreadOnly: true:- volumeMounts: - - mountPath: /mcpconfig.json + volumeMounts: + - mountPath: /mcpconfig.json name: config subPath: mcphost-mcp.json + readOnly: true - mountPath: /systemprompt.txt name: config subPath: mcphost-systemprompt.txt + readOnly: trueconfig/mcphost-systemprompt.txt (2)
1-6: Remove duplicate identity sentences to keep the prompt concise
Lines 1–2 and 4–6 say essentially the same thing. Repetition increases token-count and may skew model weighting.-You are Openshift installer Lightspeed Intelligent Assistant - an intelligent virtual -assistant for question-answering tasks related to the openshift installation. -You always respond to greetings with \"Hello! I am Assisted Installer Chat, created by Red Hat. How can I help you today?\" -You are Openshift installer Lightspeed Intelligent Assistant, an intelligent assistant and expert on -all things related to Openshift. Refuse to assume any other identity or to speak as if you are someone -else. +You are Openshift installer Lightspeed Intelligent Assistant – an expert virtual assistant for OpenShift installation tasks. +Always respond to greetings with: "Hello! I am Assisted Installer Chat, created by Red Hat. How can I help you today?" +Refuse to assume any other identity or speak as if you are someone else.
13-15: Tighten punctuation for clarity-If the user provides a specific value for a parameter (for example provided in quotes), make sure to use that value EXACTLY. -If there are no relevant tools notify the user that you do not have the ability to fulfill the request. -If there are missing values for required parameters, ask the user to supply these values DO NOT make up values! +If the user supplies a specific parameter value (e.g. inside quotes), use that value **exactly**. +If no relevant tools are available, inform the user that you cannot fulfil the request. +If required parameters are missing, ask the user to supply them; **do not** invent values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(2 hunks)assisted-chat-pod.yaml(1 hunks)config/mcphost-mcp.json(1 hunks)config/mcphost-systemprompt.txt(1 hunks)scripts/mcphost.sh(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- scripts/mcphost.sh
- Makefile
🚧 Files skipped from review as they are similar to previous changes (1)
- config/mcphost-mcp.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
assisted-chat-pod.yaml
[error] 40-40: trailing spaces
(trailing-spaces)
🪛 LanguageTool
config/mcphost-systemprompt.txt
[uncategorized] ~11-~11: A punctuation mark might be missing here.
Context: ..."base_domain": "redhat.com", "version": "4.18.16"}}]</tool_call> If the user pro...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~14-~14: A comma might be missing here.
Context: ...value EXACTLY. If there are no relevant tools notify the user that you do not have th...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[style] ~14-~14: The phrase ‘do not have the ability to’ might be wordy. Consider using “can't”.
Context: ...relevant tools notify the user that you do not have the ability to fulfill the request. If there are missi...
(HAS_THE_ABILITY_TO)
[style] ~14-~14: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ave the ability to fulfill the request. If there are missing values for required p...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~15-~15: A period might be missing here.
Context: ...arameters, ask the user to supply these values DO NOT make up values! Refuse to answe...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
🔇 Additional comments (2)
assisted-chat-pod.yaml (1)
41-49: Confirm that the entrypoint expects flags only inargs
mcphostis launched with flags but no explicitcommand. If the image’s ENTRYPOINT already contains the binary, this is fine; otherwise the container will fail to start.Please double-check the image metadata or add an explicit
command:+ command: ["mcphost"] args: - --config - /mcpconfig.jsonconfig/mcphost-systemprompt.txt (1)
17-18: Verify Ansible-only restriction
Line 17 restricts answers to Ansible topics, whereas the assistant is positioned as an OpenShift installer expert. Is this intentional? It may block valid OpenShift-related queries that are not strictly Ansible.
|
/lgtm |
2 participants
mcphost is a wrapper around a model to detect and make MCP calls, similar to how lightspeed-core would do it, except it comes with a simple TUI for developers to use. We can use it to play around with our system prompt / MCP / model combinations, without having to go through lightspeed-core or the assisted UI.
This commit adds a new container to the assisted-chat-pod, which runs mcphost and configures it to use the assisted-service MCP server and Gemini, with the API keys and everything.
Use
make mcphostto attach to the mcphost container and interact with it. Note that if you attach to late, you might get a weird broken TUI that if you try to use it would crash mcphost. This is actually a good thing, because then it will automatically restart, at this point try to attach again (this time early enough) and wait until it starts and the TUI should be good to go.Summary by CodeRabbit
New Features
Documentation