Greptile Summary

This PR resolves human-readable group chat names from the Feishu getChatInfo API and uses them to populate GroupSubject and ConversationLabel in the agent context payload, replacing the raw chat_id (e.g. oc_***) in session labels. The implementation is well-structured — multi-account cache keying, negative caching, and insertion-order-aware two-phase eviction are all solid — with good test coverage across 7 unit tests.

Key findings:

• TTL eviction gap: evictGroupNameCache() (which runs Phase 1 — removing expired entries) is only invoked when groupNameCache.size > GROUP_NAME_CACHE_MAX_SIZE. In typical deployments with far fewer than 500 group chats, expired entries will never be garbage-collected from the Map, even though they are correctly skipped on read. This is a memory leak for long-running processes.
• No account.configured guard: bot-sender-name.ts (the explicit reference pattern) short-circuits with if (!account.configured) { return {}; } before making any API call. resolveGroupName omits this check, which could cause unnecessary API calls (and a 30-minute negative-cache penalty) for unconfigured accounts.
• Minor style issues: A missing blank line before export type FeishuMessageEvent and a double blank line at the call site.

Confidence Score: 3/5

• Safe to merge with the caveat that stale cache entries will accumulate in memory over time in long-running deployments with fewer than 500 group chats.
• The core feature works correctly — group names are resolved, cached, and fall back gracefully to the raw chat ID on any failure. However, the TTL-based cleanup path is effectively dead code in most deployments (only runs when the cache hits 500 entries), which is a genuine memory leak for long-running servers. The missing account.configured guard is a minor divergence from the established pattern but won't cause visible breakage. These issues prevent a higher confidence score.
• extensions/feishu/src/bot.ts — specifically the eviction trigger condition at line 162 and the missing configured guard at line 120.

This is a comment left during a code review.
Path: extensions/feishu/src/bot.ts
Line: 162-164

Comment:
**TTL-based eviction only runs when cache is at capacity**

`evictGroupNameCache()` (which includes Phase 1 — removing expired entries) is only called when `groupNameCache.size > GROUP_NAME_CACHE_MAX_SIZE`. This means stale entries that have exceeded their 30-minute TTL will remain in memory indefinitely as long as the cache never reaches 500 entries. In a typical single-account deployment with far fewer than 500 group chats, expired entries will never be purged.

This is the opposite of the intended TTL behaviour: the cache will always serve entries within their 30-minute window (since `resolveGroupName` checks `expiresAt` on read), but the underlying `Map` will silently accumulate stale objects forever.

Consider calling `evictGroupNameCache()` unconditionally (or periodically), and only making the Phase 2 eviction conditional on the size check:

```ts
// Always run Phase 1 (TTL cleanup); only gate Phase 2 on size
evictGroupNameCache();
```

Or, alternatively, call Phase 1 unconditionally at the top of `resolveGroupName` and only trigger the full eviction when the cap is exceeded.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/feishu/src/bot.ts
Line: 167-168

Comment:
**Missing blank line before `export type FeishuMessageEvent`**

There is no blank line separating the end of `resolveGroupName` from the `FeishuMessageEvent` type declaration, making the module-level code hard to scan visually.

```suggestion
  return result;
}

export type FeishuMessageEvent = {
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/feishu/src/bot.ts
Line: 120-126

Comment:
**No `account.configured` guard before making the API call**

`bot-sender-name.ts` (the pattern this PR is explicitly modelled after) guards its API call with:

```ts
if (!account.configured) {
  return {};
}
```

`resolveGroupName` makes no such check. If an unconfigured account somehow reaches this code path (e.g., account added but keys not yet set), `createFeishuClient` may produce a client that issues requests with invalid credentials, causing a noisy `getChatInfo` failure that gets negative-cached for 30 minutes. Adding the guard here keeps the behaviour consistent and avoids an unnecessary API round-trip.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/feishu/src/bot.ts
Line: 1002-1005

Comment:
**Double blank line is a minor style nit**

There are two consecutive blank lines between the `resolveGroupName` call block and the `buildCtxPayloadForAgent` comment. A single blank line is the convention in the rest of the file.

```suggestion
    }

    // --- Shared context builder for dispatch ---
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "feat(feishu): displa..."}

Hi @BunsDev — this PR includes a detailed design write-up covering cache strategy, eviction correctness, and multi-account isolation. I've also verified the end-to-end behavior on a live Feishu deployment. Would love your review when you free. Thanks!

CI note: The 1 remaining failed checks are pre-existing and unrelated to this PR.

Codex automated review: keeping this open.

Current main still uses the raw Feishu chat_id as the group session subject and does not resolve inbound Feishu group names for session labels. This PR remains a plausible focused implementation candidate, but it overlaps with open PR #38958 and still needs maintainer review/cleanup before merge.

Best possible solution:

Keep this PR open for maintainer review. The best path is to compare it with #38958, pick the canonical Feishu display-name implementation, remove unrelated root guidance changes, preserve thread-label priority, place group-name lookup after broadcast dedup, and land the focused Feishu code/tests if that remains the chosen approach.

What I checked:

• Current Feishu inbound context still labels groups by ID: buildCtxPayloadForAgent sets GroupSubject to ctx.chatId for group messages and does not supply ConversationLabel, so current main still produces raw Feishu group IDs as the derived label input. (extensions/feishu/src/bot.ts:1065, d3fd275aa5fc)
• Session label pipeline would consume a friendly group name if provided: Label resolution prefers ConversationLabel, then ThreadLabel, then group metadata including GroupSubject; session metadata persists that resolved label into origin.label. The missing piece is Feishu populating a friendly group name before finalizing inbound context. (src/channels/conversation-label.ts:27, d3fd275aa5fc)
• Existing Feishu API wrapper can retrieve chat names: getChatInfo already calls client.im.chat.get and returns name, which supports the PR's proposed implementation path without a new core API. (extensions/feishu/src/chat.ts:16, d3fd275aa5fc)
• Current main has no group-name resolver/test surface from this PR: A targeted search found no resolveGroupName, clearGroupNameCache, or bot-group-name test on current main; only the generic conversation-label code and unrelated Feishu comment-handler ConversationLabel usage matched. (d3fd275aa5fc)
• Related PR context shows overlapping but not landed work: The provided GitHub context shows this PR resolves closed feature request Feature: Auto-populate session label with chat/group name from messaging providers #35675, while open PR feat(feishu): configurable group and DM display-name resolution #38958 separately tracks broader Feishu group/DM display-name resolution. Since neither PR is merged into current main, this PR should not be closed as implemented.

Remaining risk / open question:

• The PR overlaps with open PR feat(feishu): configurable group and DM display-name resolution #38958, so maintainers should choose one canonical Feishu display-name path before merging.
• The provided PR file list includes an unrelated root AGENTS.md deletion outside the Feishu feature surface.
• Unresolved review feedback says the group-name lookup should move behind the broadcast dedup claim to avoid unnecessary getChatInfo calls in multi-account broadcast groups.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against d3fd275aa5fc.