fix: expose perchAgent url#551
Conversation
Signed-off-by: yashodgayashan <yashodgayashan@gmail.com>
📝 WalkthroughWalkthroughThis PR enables the Perch Agent URL configuration by uncommenting and activating the ChangesPerch Agent Configuration Activation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app-config.yaml (1)
168-178: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUpdate comments to match the new configuration behavior.
The comments (lines 168-174) explicitly state: "Optional — leave commented to keep the plugin disabled by default. Uncomment AND set OPENCHOREO_PERCH_AGENT_URL only when the assistant feature is enabled."
However, this PR uncomments the configuration unconditionally. Either:
- The comments should be updated to reflect that
perchAgentUrlis now always active (controlled solely by whether the env var is set during deployment), or- The configuration should remain commented and be uncommented only in environments where the assistant feature is deployed.
The current state creates confusion about the intended deployment pattern.
📝 Suggested comment update
If the new intended pattern is to control this via environment variable presence (set by Helm only when assistant is enabled):
- # Upstream URL the openchoreo-perch-backend plugin forwards to (Perch - # AI assistant). Optional — leave commented to keep the plugin disabled - # by default. Uncomment AND set OPENCHOREO_PERCH_AGENT_URL only when - # the assistant feature is enabled - # (openchoreo.features.assistant.enabled = true) and the perch-agent - # service is deployed. + # Upstream URL the openchoreo-perch-backend plugin forwards to (Perch + # AI assistant). The plugin uses this URL when the assistant feature + # is enabled (openchoreo.features.assistant.enabled = true). + # Set OPENCHOREO_PERCH_AGENT_URL via Helm when the perch-agent service + # is deployed. If unset, the plugin should gracefully disable itself. # # For local k3d via port-forward: http://localhost:8088 # For in-cluster Backstage: http://perch-agent.openchoreo-control-plane.svc.cluster.local:8080 - # For external access: configure via OPENCHOREO_PERCH_AGENT_URL env var. perchAgentUrl: ${OPENCHOREO_PERCH_AGENT_URL}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-config.yaml` around lines 168 - 178, Update the explanatory comments around the perchAgentUrl setting to reflect the new behavior: state that perchAgentUrl is now set unconditionally from the OPENCHOREO_PERCH_AGENT_URL environment variable (so the plugin is controlled by whether the env var is provided at deploy time), and remove the guidance about "uncommenting" to enable; alternatively, if you intend to keep the previous deployment pattern, revert to keeping perchAgentUrl commented and document that it should be uncommented only when openchoreo.features.assistant.enabled is true. Reference the perchAgentUrl key, the OPENCHOREO_PERCH_AGENT_URL env var, and the openchoreo.features.assistant.enabled flag when making the comment changes.app-config.production.yaml (1)
172-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConfiguration contradicts the documented self-disable pattern.
The comment on line 176 states: "The perch-backend plugin self-disables when this key is absent, matching the thunder.token pattern."
However:
- The
thunder.tokenpattern (line 226) keeps the configuration commented when optional- This PR uncomments
perchAgentUrl, making the key always present- If
OPENCHOREO_PERCH_AGENT_URLis undefined, the key is still present (not absent), which may prevent the self-disable mechanism from working correctlyThis creates the same runtime risk as in
app-config.yaml: if the environment variable is not set, the behavior is undefined and may not follow the intended self-disable pattern.🔧 Suggested fix to maintain self-disable pattern
Option 1: Keep the configuration commented and document that it should be uncommented in deployment overlays:
- # Upstream URL the openchoreo-perch-backend plugin forwards to (Perch - # AI assistant). Optional — uncomment only when the assistant feature - # is enabled (openchoreo.features.assistant.enabled = true) and the - # perch-agent service is deployed. The perch-backend plugin self- - # disables when this key is absent, matching the thunder.token pattern. - # OPENCHOREO_PERCH_AGENT_URL: e.g. http://perch-agent.openchoreo-control-plane.svc.cluster.local:8080 - perchAgentUrl: ${OPENCHOREO_PERCH_AGENT_URL} + # Upstream URL the openchoreo-perch-backend plugin forwards to (Perch + # AI assistant). Optional — uncomment only when the assistant feature + # is enabled (openchoreo.features.assistant.enabled = true) and the + # perch-agent service is deployed. The perch-backend plugin self- + # disables when this key is absent, matching the thunder.token pattern. + # OPENCHOREO_PERCH_AGENT_URL: e.g. http://perch-agent.openchoreo-control-plane.svc.cluster.local:8080 + # perchAgentUrl: ${OPENCHOREO_PERCH_AGENT_URL}Option 2: Update comments to clarify the plugin handles undefined values gracefully (if verified):
# Upstream URL the openchoreo-perch-backend plugin forwards to (Perch - # AI assistant). Optional — uncomment only when the assistant feature - # is enabled (openchoreo.features.assistant.enabled = true) and the - # perch-agent service is deployed. The perch-backend plugin self- - # disables when this key is absent, matching the thunder.token pattern. - # OPENCHOREO_PERCH_AGENT_URL: e.g. http://perch-agent.openchoreo-control-plane.svc.cluster.local:8080 + # AI assistant). Set OPENCHOREO_PERCH_AGENT_URL only when the assistant + # feature is enabled (openchoreo.features.assistant.enabled = true) and + # the perch-agent service is deployed. The plugin gracefully handles + # undefined values by disabling itself. + # Example: http://perch-agent.openchoreo-control-plane.svc.cluster.local:8080 perchAgentUrl: ${OPENCHOREO_PERCH_AGENT_URL}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-config.production.yaml` around lines 172 - 178, The config uncomments perchAgentUrl which breaks the documented self-disable pattern: ensure the key is absent unless intentionally enabled by either 1) reverting to a commented-out entry for perchAgentUrl (so deploy overlays uncomment when enabling) matching the thunder.token pattern used elsewhere (e.g. app-config.yaml), or 2) if you intend to keep the key present, update the plugin and docs to handle undefined/empty OPENCHOREO_PERCH_AGENT_URL gracefully (verify perch-backend reads EMPTY/undefined and self-disables) and adjust the comment to reflect that behavior; reference perchAgentUrl and OPENCHOREO_PERCH_AGENT_URL in your change set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app-config.production.yaml`:
- Around line 172-178: The config uncomments perchAgentUrl which breaks the
documented self-disable pattern: ensure the key is absent unless intentionally
enabled by either 1) reverting to a commented-out entry for perchAgentUrl (so
deploy overlays uncomment when enabling) matching the thunder.token pattern used
elsewhere (e.g. app-config.yaml), or 2) if you intend to keep the key present,
update the plugin and docs to handle undefined/empty OPENCHOREO_PERCH_AGENT_URL
gracefully (verify perch-backend reads EMPTY/undefined and self-disables) and
adjust the comment to reflect that behavior; reference perchAgentUrl and
OPENCHOREO_PERCH_AGENT_URL in your change set.
In `@app-config.yaml`:
- Around line 168-178: Update the explanatory comments around the perchAgentUrl
setting to reflect the new behavior: state that perchAgentUrl is now set
unconditionally from the OPENCHOREO_PERCH_AGENT_URL environment variable (so the
plugin is controlled by whether the env var is provided at deploy time), and
remove the guidance about "uncommenting" to enable; alternatively, if you intend
to keep the previous deployment pattern, revert to keeping perchAgentUrl
commented and document that it should be uncommented only when
openchoreo.features.assistant.enabled is true. Reference the perchAgentUrl key,
the OPENCHOREO_PERCH_AGENT_URL env var, and the
openchoreo.features.assistant.enabled flag when making the comment changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22ece0a5-a251-4d5e-a189-b6645bea44bb
📒 Files selected for processing (2)
app-config.production.yamlapp-config.yaml
Purpose
perchAgent url was commented out causing the call failed.
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit