4.19: OLS-1188: Add the ability to attach Alert Rules and Silences to the prompt#1955
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThis PR extends the Lightspeed Console plugin to support attaching generated YAML for Silence and AlertingRule monitoring resources. It adds validation utilities for rule ID generation and silence ID validation, detects resource detail pages via URL patterns, and implements fetch-and-attach flows in the Prompt component with corresponding UI labels. ChangesSilence and AlertingRule YAML Attachment
Sequence DiagramsequenceDiagram
participant User
participant Prompt
participant LocationContext
participant SilenceAPI
participant AlertingRuleAPI
participant YAMLConverter
User->>LocationContext: Navigate to detail page
LocationContext->>Prompt: Set kind and name in context
Prompt->>User: Display attach menu
User->>Prompt: Select Silence or AlertingRule
alt Silence resource
Prompt->>SilenceAPI: Fetch silence by name
SilenceAPI-->>Prompt: Silence object
else AlertingRule resource
Prompt->>AlertingRuleAPI: Fetch rule definitions
AlertingRuleAPI-->>Prompt: Rule object
end
Prompt->>YAMLConverter: Convert to YAML
YAMLConverter-->>Prompt: YAML string
Prompt->>User: Dispatch attachment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/hooks/useLocationContext.ts`:
- Around line 193-199: The route matcher in useLocationContext currently uses
path.match(new RegExp('/monitoring/alertrules/([0-9]+)')) which will match
nested routes; update the regex used when computing alertRuleMatch (inside
useLocationContext) to anchor it to the detail-page URL (e.g. require start and
end or allow only an optional trailing slash) so only exact detail pages
setKind('AlertingRule'), setName(alertRuleMatch[1]) and setNamespace(undefined);
keep the same variables (alertRuleMatch, setKind, setName, setNamespace) but
tighten the RegExp to avoid false positives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: eb0a33ec-2c4f-48c1-8cbd-e5af8e8903a6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
locales/en/plugin__lightspeed-console-plugin.jsonpackage.jsonsrc/components/Prompt.tsxsrc/hooks/useLocationContext.tssrc/validation.tsunit-tests/validation.test.ts
| const alertRuleMatch = path.match(new RegExp('/monitoring/alertrules/([0-9]+)')); | ||
| if (alertRuleMatch) { | ||
| setKind('AlertingRule'); | ||
| setName(alertRuleMatch[1]); | ||
| setNamespace(undefined); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Tighten AlertingRule route matching to detail-page URLs only.
Line 193 matches any path containing /monitoring/alertrules/<digits>, so nested paths can be misclassified as a detail page context. Anchor the regex to avoid false positives.
Proposed fix
- const alertRuleMatch = path.match(new RegExp('/monitoring/alertrules/([0-9]+)'));
+ const alertRuleMatch = path.match(new RegExp('/monitoring/alertrules/([0-9]+)/?$'));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const alertRuleMatch = path.match(new RegExp('/monitoring/alertrules/([0-9]+)')); | |
| if (alertRuleMatch) { | |
| setKind('AlertingRule'); | |
| setName(alertRuleMatch[1]); | |
| setNamespace(undefined); | |
| return; | |
| } | |
| const alertRuleMatch = path.match(new RegExp('/monitoring/alertrules/([0-9]+)/?$')); | |
| if (alertRuleMatch) { | |
| setKind('AlertingRule'); | |
| setName(alertRuleMatch[1]); | |
| setNamespace(undefined); | |
| return; | |
| } |
🤖 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 `@src/hooks/useLocationContext.ts` around lines 193 - 199, The route matcher in
useLocationContext currently uses path.match(new
RegExp('/monitoring/alertrules/([0-9]+)')) which will match nested routes;
update the regex used when computing alertRuleMatch (inside useLocationContext)
to anchor it to the detail-page URL (e.g. require start and end or allow only an
optional trailing slash) so only exact detail pages setKind('AlertingRule'),
setName(alertRuleMatch[1]) and setNamespace(undefined); keep the same variables
(alertRuleMatch, setKind, setName, setNamespace) but tighten the RegExp to avoid
false positives.
5f62a48
into
openshift:release-4.19
Manual cherry pick of #1941
Summary by CodeRabbit
Release Notes