Skip to content

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
v3from
pr@v3@fix_to_chat
Closed

shaohuzhang1 wants to merge 1 commit into
v3from
pr@v3@fix_to_chat

Conversation

@shaohuzhang1

Copy link
Copy Markdown
Contributor

fix: [Agent] Clicking the "Go to Chat" icon on an agent card yields no response, and it is impossible to enter the chat page.

…o response, and it is impossible to enter the chat page.
@shaohuzhang1
shaohuzhang1 deleted the pr@v3@fix_to_chat branch November 12, 2026 06:06
@shaohuzhang1
shaohuzhang1 restored the pr@v3@fix_to_chat branch November 12, 2026 06:06
@shaohuzhang1
shaohuzhang1 deleted the pr@v3@fix_to_chat branch November 12, 2026 06:07
<el-button text @click="toChat">
<MkIcon name="icon_new-chat_outlined" />
</el-button>
</MkTooltip>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and CHAT_API_BASE_PATH are inserted interspersed with variable definitions and defineOptions.
  • Recommendation: Move all import statements 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, but router is never used in the logic. toChat uses window.open. useRoute is 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: apiQuery is constructed using unvalidated data from props.application.work_flow.nodes...default_value. These values are joined directly into a query string (?key=value) and passed as a fragment/path part of access_token.
    • If variable or default_value contain characters like &, #, ?, or newlines, they can break the URL structure or inject unintended parameters.
    • The access_token is appended before the query string. Ensure that chatBasePath + access_token expects a query string immediately after, and that access_token itself is URL-encoded if it contains special chars.
  • Recommendation:
    1. Use URLSearchParams to safely encode query parameters.
    2. 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() : '';
    });

B. Race Condition / Context Loss on Window.open

  • Issue: window.open might 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.value is always safe to append. The current implementation handles missing data correctly by returning ''.

3. Optimization Suggestions

A. Computed Property Dependency

  • The apiQuery computed property depends on props.application. This is correct and efficient as Vue will recompute only when props.application changes.

B. String Construction for URL

  • Instead of manual string concatenation:
    `${window.location.origin}${chatBasePath}/${ok.access_token}/${apiQuery.value}`
    Consider creating a clean builder function or ensuring access_token is URL-safe. If access_token contains slashes or spaces, this URL breaks.

C. Template Performance

  • The MkStatusLabel v-else block has inline logic :icon="application.is_publish ? undefined : 'icon_time_filled'". This is minor but fine. Ensure MkStatusLabel is 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

  1. Move imports to the top.
  2. Remove unused useRouter/useRoute imports/variables.
  3. Use URLSearchParams for building the query string to prevent malformed URLs.
  4. Add error handling (try/catch or .catch) around the async getAccessToken call.
  5. Review template formatting to match project standards (likely multi-line for complex components).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant