Repository navigation
fix: [Agent] Clicking the "Go to Chat" icon on an agent card yields no response, and it is impossible to enter the chat page. - #7858
Closed
shaohuzhang1 wants to merge 1 commit into
Closed
shaohuzhang1 wants to merge 1 commit into
shaohuzhang1 wants to merge 1 commit into
Conversation
…o response, and it is impossible to enter the chat page.
shaohuzhang1
commented
Nov 12, 2026
| <el-button text @click="toChat"> | ||
| <MkIcon name="icon_new-chat_outlined" /> | ||
| </el-button> | ||
| </MkTooltip> |
Contributor
Author
There was a problem hiding this comment.
Here is a review of the provided code diff, highlighting irregularities, potential issues, and optimization suggestions.
1. Irregularities & Best Practices
A. Import Placement
- Issue: Imports are mixed throughout the script setup block.
import applicationAPI,useRoute/useRouter,computed, andCHAT_API_BASE_PATHare inserted interspersed with variable definitions anddefineOptions. - Recommendation: Move all
importstatements to the very top of the<script setup>block. This improves readability and follows standard ESLint/Vue formatting rules (e.g.,import/first).
B. Unused useRoute
- Issue:
const router = useRouter()is initialized, butrouteris never used in the logic.toChatuseswindow.open.useRouteis imported but not utilized either. - Recommendation: Remove unused imports and variables. If navigation is intended via Vue Router, use
router.push(). If opening in a new tab via URL construction is intended, remove the Vue Router imports.
C. Template Formatting
- Issue: The template changes involve collapsing multi-line component attributes into single lines. While valid, this reduces readability for long attribute lists and makes git diffs harder to read when attributes change later.
- Recommendation: Maintain consistent formatting. Ideally, one attribute per line when there are more than 3-4 attributes, or stick to a prettier/eslint-config-vue style guide.
2. Potential Issues
A. Security: Open Redirect / Command Injection
- Issue:
apiQueryis constructed using unvalidated data fromprops.application.work_flow.nodes...default_value. These values are joined directly into a query string (?key=value) and passed as a fragment/path part ofaccess_token.- If
variableordefault_valuecontain characters like&,#,?, or newlines, they can break the URL structure or inject unintended parameters. - The
access_tokenis appended before the query string. Ensure thatchatBasePath+access_tokenexpects a query string immediately after, and thataccess_tokenitself is URL-encoded if it contains special chars.
- If
- Recommendation:
- Use
URLSearchParamsto safely encode query parameters. - Validate/sanitize inputs from
application.work_flow.
const apiQuery = computed(() => { const nodes = props.application?.work_flow?.nodes; const baseNode = nodes?.find(n => n.id === 'base-node'); const fieldList = baseNode?.properties?.api_input_field_list; if (!fieldList || !Array.isArray(fieldList)) return ''; const params = new URLSearchParams(); fieldList.forEach((f: any) => { // Optional: Validate that f.variable is safe if (f.variable && f.default_value !== undefined) { params.append(f.variable, String(f.default_value)); } }); return params.toString() ? '?' + params.toString() : ''; });
- Use
B. Race Condition / Context Loss on Window.open
- Issue:
window.openmight be blocked by popup blockers if not triggered directly by a user gesture. Currently, it's inside a.then(())callback. While generally okay if the promise resolves immediately, if there's network latency, some browsers might flag this as non-user-triggered. - Recommendation: This is usually an acceptable pattern for "open in new tab" APIs, but ensure the click handler calls a function that might open synchronously, or accept that popup blockers might apply. Given it's a button click, it's likely fine, but be aware of edge cases where async operations delay
window.open.
C. Fragile Data Selection
- Issue: The code uses optional chaining heavily on deeply nested objects:
props.application?.work_flow?.nodes?.find(...). If the structure changes slightly, this silently fails returning empty strings. - Recommendation: This is defensive coding and good, but ensure
apiQuery.valueis always safe to append. The current implementation handles missing data correctly by returning''.
3. Optimization Suggestions
A. Computed Property Dependency
- The
apiQuerycomputed property depends onprops.application. This is correct and efficient as Vue will recompute only whenprops.applicationchanges.
B. String Construction for URL
- Instead of manual string concatenation:
Consider creating a clean builder function or ensuring
`${window.location.origin}${chatBasePath}/${ok.access_token}/${apiQuery.value}`access_tokenis URL-safe. Ifaccess_tokencontains slashes or spaces, this URL breaks.
C. Template Performance
- The
MkStatusLabelv-else block has inline logic:icon="application.is_publish ? undefined : 'icon_time_filled'". This is minor but fine. EnsureMkStatusLabelis optimized for frequent re-renders if this list is large.
Refactored Script Segment Example
<script setup lang="ts">
import type { ApplicationDetail } from '@/api/types'
import { isWorkFlow } from '@/utils/common'
import { dateFormat } from '@/utils/time'
import applicationAPI from "@/api/admin/workspace/application/application"
import { CHAT_API_BASE_PATH } from '@/api/constants'
// Only import useRouter if you plan to use router.push later.
// Since we are using window.open, remove useRouter/useRoute unless needed elsewhere.
// import { useRoute, useRouter } from 'vue-router';
import { computed } from 'vue';
defineOptions({ name: 'ApplicationCard' })
const props = defineProps<{
application: ApplicationDetail;
selectable?: boolean;
selected?: boolean;
}>()
const emit = defineEmits<{
click: [];
selected: [selected: boolean];
}>()
defineSlots<{ 'action-dropdown'?: () => unknown }>()
const chatBasePath = CHAT_API_BASE_PATH.replace(/\/api$/, '')
// Use URLSearchParams for safer query string generation
const apiQuery = computed(() => {
try {
const baseNode = props.application.work_flow?.nodes?.find(
(n: any) => n.id === 'base-node'
);
const fieldList = baseNode?.properties?.api_input_field_list;
if (!fieldList || !Array.isArray(fieldList)) {
return '';
}
const params = new URLSearchParams();
fieldList.forEach((f: any) => {
if (f.variable !== undefined && f.default_value !== undefined) {
params.append(String(f.variable), String(f.default_value));
}
});
const queryString = params.toString();
return queryString ? `?${queryString}` : '';
} catch (e) {
console.error("Error generating API query", e);
return '';
}
});
function handleOpen() {
if (!props.selectable) emit('click');
}
const toChat = async () => {
try {
// Wait for token
const response = await applicationAPI.getAccessToken(props.application.id);
if (!response.access_token) {
console.warn('No access token received');
return;
}
// Construct URL safely
// Note: Ensure access_token is URL-safe or encode it if necessary
const tokenPath = `/token-${encodeURIComponent(response.access_token)}`; // Example if path-based security requires it
const fullUrl = `${window.location.origin}${chatBasePath}/${response.access_token}${apiQuery.value}`;
window.open(fullUrl, '_blank', 'noopener,noreferrer');
} catch (error) {
console.error('Failed to get access token for chat', error);
}
};
</script>Summary of Actions
- Move imports to the top.
- Remove unused
useRouter/useRouteimports/variables. - Use
URLSearchParamsfor building the query string to prevent malformed URLs. - Add error handling (
try/catchor.catch) around the asyncgetAccessTokencall. - Review template formatting to match project standards (likely multi-line for complex components).
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.
fix: [Agent] Clicking the "Go to Chat" icon on an agent card yields no response, and it is impossible to enter the chat page.