[shaohuzhang1]

Revert "feat: add application read permissions to homepage components" This reverts commit 1430103.
fix: chat bug

[shaohuzhang1]

Review of the diff

Main issue: removed permission gating (behavior regression)

The diff removes the four canRead* computed guards and always calls every aggregation API. Before, in system scope, requests were only issued for resources the user had *.system.read() permission for; unauthorized resources were short-circuited with zero-value objects.

Consequences to consider:

1. Unnecessary/unauthorized requests — users without system-level read permission now trigger 4 API calls that will likely return 401/403. They'll land in .catch() anyway, so UI result is undefined, but you've traded a cheap client-side guard for server round-trips + error logging noise.
2. Lost "zero state" rendering — previously authorized-but-denied resources got ZERO_* objects so the card could render 0s. Now they become undefined. Verify the template handles undefined for all four refs gracefully (if it dereferences .total, it may crash).
3. If this removal was intentional (e.g., backend now scopes data per workspace and returns zeros itself), document that rationale in the commit. Otherwise, restoring the permission checks is safer.

Other observations

• ZERO_* constants deleted — fine if no longer referenced, but confirm nothing else imports them. Also, they were shallow-copied ({ ...ZERO_X }); nested mutable fields (none here) would have been shared anyway.
• Empty catch handlers swallow errors silently. Keep at least a console.warn or set a per-resource error flag, otherwise debugging failures becomes hard — especially now that auth failures are expected.
• No handling of stale results — if loadResources() is called again while a prior call is in flight, the loading guard prevents overlap, good. But finally(() => loading.value = false) runs even for the first settled branch? No — Promise.allSettled(...).finally fires after all settle, so this is correct.
• Repetition — four near-identical blocks could be DRY'd:

  const tasks = [
    [application, props.api.getApplicationAggregation],
    [knowledge, props.api.getKnowledgeAggregation],
    [tool, props.api.getToolAggregation],
    [model, props.api.getModelAggregation],
  ] as const
  ...
  return Promise.allSettled(
    tasks.map(([ref, fn]) =>
      fn(props.workspaceId).then(v => { ref.value = v }, () => { ref.value = undefined })
    )
  ).finally(() => { loading.value = false })

• Race on prop change — props.workspaceId changes don't auto-reload; presumably handled by a watcher elsewhere. Worth double-checking there's a watch(() => props.workspaceId, loadResources).
• isSystemScope now appears unused — if nothing else references it, remove to avoid a lint warning.
• perm import may now be unused too — verify and clean up.

Recommendation

Unless the backend reliably returns zeroed aggregations for unauthorized resources, restore the canRead* guards. If it does, keep the simplification but add a brief comment explaining the backend guarantee and ensure the template renders undefined safely.

[shaohuzhang1]

Code Review

Did the diff apply cleanly?

The provided diff is a delta, not standalone code, so I reviewed the changes in context. The changes are logically consistent, but there are a few issues worth flagging.

---

Issues & Observations

1. portalName meta handling is over-defensive but inconsistent

const meta = authStore.authProfile?.meta
const name = meta && typeof meta === 'object' && 'portal_name' in meta ? meta.portal_name : undefined

• Improvement over the removed version (which did meta.portal_name on possibly undefined — that was a latent runtime crash). Good fix.
• However, consider typing authProfile.meta properly (e.g. { portal_name?: string; [k: string]: unknown }) so you can drop the runtime guards. The in + typeof chain is verbose here.
• Suggest (meta as Record<string, unknown> | undefined)?.portal_name.

2. groupConversations changed semantics — verify with consumers
Previously it always sliced to 5. Now it returns all conversations when total ≤ 20. It's used in:

• historyApplicationGroups filter (.length > 0) — fine.
• Template rendering of conversation items — now renders potentially more than 5 items per group. Ensure the UI/layout handles this (scroll, height). This looks intentional (matches "无需查看更多时展示全部已加载会话"), but confirm layout tests.

3. totalConversationCount > 20 threshold is a magic number
Extract constants for clarity:

const TOTAL_CONVERSATION_THRESHOLD = 20

And use in both the computed and template (v-if). Right now 20 is duplicated in script and template.

4. groupConversationCount may double-count the current application

Math.max(group.count, isCurrentApplication(group) ? conversations.value.length : group.conversations.length)

If applicationGroups includes a group whose count already reflects server-side totals, the sum in totalConversationCount could mismatch actual rendered counts. Confirm group.count semantics (server total vs. loaded). If the server count is larger than loaded, users may see "查看更多" even though only some are loaded — acceptable, but document intent.

5. Redundant import path change
@/stores/chat/index.ts → @/stores/chat is cleaner. ✅ But ensure no other file relies on the explicit .ts path consistency style; align project-wide.

6. RECENT_CONVERSATION_LIMIT comment now stale elsewhere?
The inline comment near line ~230 was updated ("总数超过 20 条…"). Good. But check that any docs/tests referencing "每个智能体最多展示最近 5 条会话" are updated to reflect the new conditional behavior.

7. Minor: template binding order
class="pl-8!" before v-if — put v-if first for readability (convention).

<MkListItem v-if="..." class="pl-8!" @click="...">

8. portalLogo type
authStore.authProfile?.logo — if logo can be undefined, <PortalIcon :icon="portalLogo" /> must tolerate undefined. Verify PortalIcon prop allows optional icon (else add default).

---

Suggested refined snippet

const TOTAL_CONVERSATION_THRESHOLD = 20

const { auth: authStore } = useStore()

const portalName = computed(() => {
  const name = (authStore.authProfile?.meta as Record<string, unknown> | undefined)?.portal_name
  return typeof name === 'string' && name ? name : '智能体门户'
})
const portalLogo = computed(() => authStore.authProfile?.logo)

const groupConversations = (group: PortalApplicationGroup) => {
  const list = isCurrentApplication(group) ? conversations.value : group.conversations
  return totalConversationCount.value > TOTAL_CONVERSATION_THRESHOLD
    ? list.slice(0, RECENT_CONVERSATION_LIMIT)
    : list
}

Template:

<MkListItem
  v-if="totalConversationCount > TOTAL_CONVERSATION_THRESHOLD && groupConversationCount(group) > RECENT_CONVERSATION_LIMIT"
  ...
>

---

Summary

The refactor is an improvement — it fixes a real null-access bug (meta.portal_name) and moves the name/logo logic into computed properties. Main follow-ups: define named constants, type meta, confirm rendering of >5 items per group, and verify group.count semantics to avoid incorrect "查看更多" visibility.

[shaohuzhang1]

Code Review

Potential Issues

1. No error handling in handleUserSearch
   The .then/.finally chain has no .catch. A rejected getWorkspaceMembers call will produce an unhandled promise rejection (though the global handler may catch it). Consider try/catch/await or adding a .catch.

2. Race condition / stale response overwrites
   loadingUsers guards against concurrent searches, but not against out-of-order responses. If the guard is ever bypassed (e.g., debounce removed later), a slow earlier request could overwrite a newer keyword's results. More importantly, rapid typing: since you early-return while loading, keystrokes during an in-flight request are silently dropped — the user's latest keyword may never be searched. Consider debouncing the input instead of dropping requests.

3. inert attribute on a Vue component

   • :inert="loadingUsers" — inert is a boolean HTML attribute; when false, Vue renders inert="false", which still activates inert behavior in browsers (presence-only semantics for older engines; modern browsers treat inert="false" as active because it's a valid attribute value string, not removed). Verify: Vue removes attributes bound to false only for known boolean attributes. inert is standardized as a boolean attribute, so this generally works correctly in Vue 3 — but confirm your Vue version and that it lands on the root element of UserAuthorizationList.
   • inert prevents focus/interaction but does not announce loading state to screen readers. Pair with aria-busy.

4. v-loading + inert overlap
   Using both may be redundant; v-loading already overlays. Ensure the loading mask itself isn't inside the inert subtree (otherwise the spinner blocks interaction but keyboard users get no cue either).

5. :key="selectedWorkspaceId" forces full re-render
   This destroys and recreates UserAuthorizationList on every workspace switch, resetting internal state (scroll position, local search input text, etc.). If that's intended, fine — but note that "search keyword" entered by the user will be lost on workspace change, which may surprise users. Also workspaceMembers isn't cleared here, so a stale list flashes before the new load finishes unless the parent reloads it.

6. selectedWorkspaceId.value may be empty on first search
   No guard for missing workspace ID; the API would be called with an empty id.

7. Stale workspaceMembers on workspace switch
   Recommend clearing workspaceMembers.value = [] and resetting selectedUserId when workspace changes, to avoid showing users from the previous workspace.

8. nick_name casing inconsistency
   Mixed snake_case (nick_name) vs. camelCase elsewhere (selectedUserId, userGroups). Confirm this matches the API contract and is consistent with other calls.

9. Inconsistent style
   Existing code uses async/await (seen in loadAuthorizationTargets), but this new function uses .then().finally(). Match project conventions.

Optimization Suggestions

let searchTimer: ReturnType<typeof setTimeout> | undefined

async function handleUserSearch(keyword: string) {
  clearTimeout(searchTimer)
  searchTimer = setTimeout(async () => {
    if (!selectedWorkspaceId.value) return
    loadingUsers.value = true
    try {
      workspaceMembers.value = await CommonSystemApi.getWorkspaceMembers(
        selectedWorkspaceId.value,
        keyword ? { nick_name: keyword } : undefined,
      )
    } catch (e) {
      // surface error / toast
    } finally {
      loadingUsers.value = false
    }
  }, 300)
}

Also consider:

• Add aria-busy="loadingUsers" alongside v-loading.
• Clear workspaceMembers in a watch(selectedWorkspaceId, ...).
• If UserAuthorizationList supports it, prefer debounced search over the inert/loadingUsers interaction guard.
• Avoid :key if you can reset internal state via a prop or exposed method instead.