Open
Conversation
…eatures/add-agent-rule-action
…eatures/add-agent-rule-action
…eatures/graph-rule-engine
Review Summary by QodoImplement graph rule engine with collapsible UI components and MCP tools management
WalkthroughsDescription• Add agent rule engine with trigger configuration and topology support • Implement collapsible UI components for rules, utilities, and knowledge bases • Refactor knowledge base and LLM config components into modular item components • Add MCP tools management with server configuration and function selection • Enhance CodeScript editor with line number hiding and editability control Diagramflowchart LR
A["Agent Components"] --> B["Rules Engine"]
A --> C["Utilities"]
A --> D["Knowledge Bases"]
A --> E["MCP Tools"]
B --> F["Rule Items with Topology"]
C --> G["Utility Items with Visibility"]
D --> H["Knowledge Base Items"]
E --> I["MCP Tool Items with Functions"]
J["LLM Configs"] --> K["Collapsible Headers"]
L["CodeScript Editor"] --> M["Line Numbers & Editability Control"]
File Changes1. src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte
|
Code Review by Qodo
1. Rule config undefined crash
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+384
to
+386
| const config = ruleConfigs[found.config.topology_provider]; | ||
| const customParam = config.customParameters || {}; | ||
|
|
There was a problem hiding this comment.
1. Rule config undefined crash 🐞 Bug ✓ Correctness
openRuleConfigModal dereferences config.customParameters without verifying that ruleConfigs[topology_provider] exists, so clicking the config gear can throw and break the Rules UI if the provider key is missing/mismatched.
Agent Prompt
### Issue description
`openRuleConfigModal` assumes `ruleConfigs[topology_provider]` always exists and immediately dereferences `config.customParameters`. If the API doesn’t return a matching key, this throws and breaks the Rules UI.
### Issue Context
This is triggered by clicking the rule config gear icon; it should be safe even when the backend returns partial/mismatched config options.
### Fix Focus Areas
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[378-404]
Suggested direction:
- Add `if (!config) { return; }` (or show a user-facing error) before accessing `config.customParameters`.
- Consider also guarding `customParam.url` and `customParam.htmlTag` existence before constructing the URL.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+393
to
+395
| const url = new URL(customParam.url, window.location.origin); | ||
| url.searchParams.set(customParam.appendParameterName || 'parameters', encodeURIComponent(params)); | ||
|
|
There was a problem hiding this comment.
2. Double-encoded iframe params 🐞 Bug ✓ Correctness
openRuleConfigModal passes encodeURIComponent(params) into URLSearchParams.set, which causes double-encoding and breaks parsing of the JSON parameters in the embedded config page.
Agent Prompt
### Issue description
The code uses `encodeURIComponent(params)` before `url.searchParams.set(...)`, which leads to double-encoding of the JSON payload.
### Issue Context
The embedded iframe page expects to receive a query parameter containing JSON (or at least a once-encoded JSON string).
### Fix Focus Areas
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[388-395]
Suggested direction:
- Change to: `url.searchParams.set(name, params)`.
- Ensure the receiving page decodes exactly once.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+422
to
+431
| {#if isOpenConfigModal} | ||
| <PlainModal | ||
| containerClasses={'rule-config-modal'} | ||
| title={selectedRuleConfig?.rule?.trigger_name || ''} | ||
| size={'xl'} | ||
| isOpen={isOpenConfigModal} | ||
| toggleModal={() => isOpenConfigModal = !isOpenConfigModal} | ||
| > | ||
| <iframe src={selectedRuleConfig.url} title={selectedRuleConfig.title} width="100%" height="100%" /> | ||
| </PlainModal> |
There was a problem hiding this comment.
3. Untrusted iframe url embed 🐞 Bug ⛨ Security
The code embeds a server-provided URL into an iframe without any origin/allowlist validation, enabling arbitrary pages to be framed and receiving agentId/trigger data via query parameters.
Agent Prompt
### Issue description
A server-driven `customParam.url` is framed directly. Without validating `url.origin` (or restricting to known safe paths), this can embed arbitrary origins and leak agent metadata via query parameters.
### Issue Context
Even if only admins can open the modal, the embedded content can still perform unwanted actions (tracking, phishing UI, data exfil) within the iframe context.
### Fix Focus Areas
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[387-404]
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[422-431]
Suggested direction:
- Reject URLs whose `origin` is not `window.location.origin` (or not in an allowlist).
- Add `sandbox` attributes to the iframe (tightest possible), e.g. start with `sandbox` and selectively allow what’s needed.
- Consider not passing `agentId`/`trigger` to third-party origins.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+118
to
+126
| /** | ||
| * Get agent rule criteria providers | ||
| * @returns {Promise<any[]>} | ||
| */ | ||
| export async function getAgentRuleConfigOptions() { | ||
| const url = endpoints.agentRuleConfigOptionsUrl; | ||
| const response = await axios.get(url); | ||
| return response.data; | ||
| } |
There was a problem hiding this comment.
4. Rule config shape mismatch 🐞 Bug ✓ Correctness
getAgentRuleConfigOptions is documented as returning an array, but the rules UI treats it as an object keyed by provider name; if the API actually returns an array, configOptions/ruleConfigs will be built incorrectly and rule config modal cannot resolve provider configs.
Agent Prompt
### Issue description
The UI logic requires `ruleConfigs` to be a map keyed by provider name, but the service is documented to return an array. If the backend response is an array, `ruleConfigs[providerName]` will be `undefined` and the config modal flow breaks.
### Issue Context
This is a contract mismatch between `getAgentRuleConfigOptions()` and `agent-rule.svelte`.
### Fix Focus Areas
- src/lib/services/agent-service.js[118-126]
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[123-136]
- src/routes/page/agent/[agentId]/agent-components/rules/agent-rule.svelte[378-386]
Suggested direction:
- If backend returns a map: update JSDoc to `Promise<Record<string, any>>` (or a typed interface).
- If backend returns an array: convert it in `loadAgentRuleConfigOptions()` into `{ [name]: config }` and build `configOptions` from those names.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…eatures/graph-rule-engine
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
1 participant
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.
No description provided.