Skip to content

feat: 飞书凭证迁入 agents.json#212

Merged
lishuceo merged 9 commits into
mainfrom
feat/feishu-in-agents-json
Apr 8, 2026
Merged

feat: 飞书凭证迁入 agents.json#212
lishuceo merged 9 commits into
mainfrom
feat/feishu-in-agents-json

Conversation

@lishuceo
Copy link
Copy Markdown
Owner

@lishuceo lishuceo commented Apr 8, 2026

Summary

  • 每个 agent 内嵌 feishu: { appId, appSecret } 字段,一个文件看到全貌
  • 系统启动时从 agents.json 自动推导 bot 账号和路由规则(单/多 bot 自动判断)
  • 移除 .env 中的 FEISHU_APP_IDFEISHU_APP_SECRETBOT_ACCOUNTSAGENT_BINDINGS
  • Onboarding agent 将飞书凭证直接写入 agents.json,不再写 .env

Test plan

  • 新机器部署:setup.shnpm run onboard → 飞书 key 写入 agents.json → npm run dev
  • /config 命令显示正确的 feishu appId
  • npx vitest run 全部通过(1320 passed,1 pre-existing flaky failure)

🤖 Generated with Claude Code

每个 agent 内嵌 feishu 字段(appId + appSecret),一个文件看到全貌。
系统启动时从 agents.json 推导 bot 账号和路由规则。

主要改动:
- config-schema: 新增 FeishuAccountSchema + AgentBindingSchema
- config-loader: deriveBotAccounts/deriveBindings 从 agent config 推导
- config.ts: 移除 FEISHU_APP_ID/SECRET、BOT_ACCOUNTS、AGENT_BINDINGS
- index.ts: 启动顺序改为 agentConfig → deriveBotAccounts → accountManager
- client.ts: FeishuClient 构造函数 appId/appSecret 改为必传,增加 initDefaultClient
- server.ts: startServer 接收 primaryBot 参数
- oauth.ts: 从 deriveBotAccounts 获取凭证
- onboarding prompt: 飞书 keys 写入 agents.json 而非 .env

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 2m 5s —— View job


PR Review: feat: 飞书凭证迁入 agents.json

  • Check for previous review comments
  • Read PR diff and all changed files
  • Analyze for bugs, security, and architecture issues
  • Post inline comments and summary

⚠️ Issues Found

Overall: The migration direction is sound — consolidating Feishu credentials into agents.json simplifies configuration and makes the multi-agent setup more self-documenting. The schema additions, config-loader extensions, and startup flow rework are well-structured. However, there are a few issues to address:

# Severity File Issue Confidence
1 🔴 Critical src/feishu/client.ts:907 Multi-bot mode crashes: initDefaultClient() is only called in single-bot branch, but the feishuClient proxy throws when _defaultClient is undefined. Any code using feishuClient outside AsyncLocalStorage context in multi-bot mode (e.g., OAuth feishuClient.raw) will crash. 90
2 🟡 Warning src/feishu/oauth.ts:37-38 Empty HMAC key degrades security: getPrimaryAppSecret() returns '' if no bot accounts exist, silently signing OAuth states with an empty key. Should throw instead. 82
3 🟡 Warning src/feishu/oauth.ts:38 Repeated computation: deriveBotAccounts() rebuilds a full Map from all agents on every signState()/verifyState() call. Should cache. 80
4 🟡 Warning src/feishu/event-handler.ts:689 Per-message overhead: deriveBindings() iterates all agents and builds arrays on every incoming message. Should cache and invalidate on config reload. 78

What looks good:

  • Clean Zod schema additions for FeishuAccountSchema and AgentBindingSchema
  • Proper deduplication logic in deriveBotAccounts() (by appId)
  • Explicit bindings take precedence over auto-derived ones (spread order)
  • Test updates correctly reflect the new source of truth
  • startServer() parameter design cleanly separates single/multi-bot paths

Comment thread src/feishu/client.ts
Comment thread src/feishu/oauth.ts Outdated
Comment thread src/feishu/event-handler.ts
Comment thread src/feishu/oauth.ts Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 2m 51s —— View job


PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous review comments and resolve fixed issues
  • Read current state of all changed files
  • Analyze for new bugs, security, and architecture issues
  • Post summary

⚠️ Issues Found — 4 previous issues still unresolved

All 4 issues from the previous review remain unfixed after the latest push (2826257). Replied to each thread with current status.

# Severity File Issue Status
1 🔴 Critical src/feishu/client.ts:907 Multi-bot mode crashes: initDefaultClient() only called in single-bot branch. Any feishuClient usage outside AsyncLocalStorage context in multi-bot mode throws. Concrete crash paths: interrupted session notifications (index.ts:193), cron sendMessage fallback to 'default' (index.ts:131), OAuth feishuClient.raw (oauth.ts:113). Still exists
2 🟡 Warning src/feishu/oauth.ts:37-38 Empty HMAC key degrades security: getPrimaryAppSecret() returns '' if no bot accounts, silently signing with vacuous key. Still exists
3 🟡 Warning src/feishu/oauth.ts:38 Repeated computation: deriveBotAccounts() rebuilds full Map on every signState()/verifyState() call. Still exists
4 🟡 Warning src/feishu/event-handler.ts:689 Per-message overhead: deriveBindings() iterates all agents on every incoming message without caching. Still exists

No new issues found

The fix: 移除 index.ts 中未使用的 isMultiBotMode 导入 commit (2826257) is a clean import cleanup — no new issues introduced.

What looks good:

  • Clean Zod schema additions (FeishuAccountSchema, AgentBindingSchema)
  • Proper deduplication in deriveBotAccounts() by appId
  • Explicit bindings take precedence over auto-derived (spread order in index.ts:59)
  • Test updates correctly reflect the new source of truth (deriveBotAccounts mock)
  • .env.example cleanup is thorough — clear migration comment at top
  • No credential leakage risk in logging (all log calls only output accountId/agentId, never secrets)

Recommendation: Fix the 🔴 critical issue (#1) before merging — multi-bot mode will crash at runtime on several code paths. The simplest fix: always call initDefaultClient() with the primary bot's credentials, even in multi-bot mode.

- 多 bot 模式也调用 initDefaultClient(修复 AsyncLocalStorage 无值时崩溃)
- deriveBotAccounts/deriveBindings 改为缓存模式,reload 时刷新
- OAuth getPrimaryAppSecret 无账号时 throw 而非返回空字符串

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 2m 22s —— View job


PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous review comments and current diff
  • Read current state of all affected files
  • Check if 4 previous issues are resolved
  • Look for new issues in latest changes
  • Reply to previous comments and resolve fixed threads
  • Post final summary

✅ All 4 previous issues fixed — no new issues found

Commit 2237f40 ("fix: address PR review — multi-bot initDefaultClient + 缓存优化 + OAuth 安全") properly addresses all previously flagged issues:

# Previous Issue Fix Verified
1 🔴 Multi-bot mode _defaultClient uninitialized initDefaultClient() now called at index.ts:148 before the multi-bot/single-bot branch
2 🟡 Empty HMAC key in OAuth getPrimaryAppSecret() now throws instead of returning ''
3 🟡 deriveBotAccounts() recomputed per-call Returns cached cachedBotAccounts, recomputed only on load/reload
4 🟡 deriveBindings() per-message overhead Returns cached cachedDerivedBindings, recomputed only on load/reload

All 4 review threads have been resolved.

Overall assessment: The fix commit is clean and well-structured. The caching approach in config-loader.ts (lines 43-45, 203-204, 305-355) correctly invalidates on reload while eliminating per-request overhead. The initDefaultClient placement ensures both single-bot and multi-bot modes work correctly. LGTM.

description 字段解释 agent 的定位和特点:
- PM: Chat Bot — 只读、直接回复、轻量对话
- Dev: Dev Bot — 完整读写、创建话题、编码任务

/config 命令也展示 description。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous inline review comments and resolve fixed threads
  • Review new changes since last review (d7d81ab)
  • Read all affected files for context
  • Post summary

lishuceo and others added 2 commits April 8, 2026 19:51
- pm.example.md 已被 assistant.example.md 替代
- .gitkeep 不再需要(目录中已有 .example.md 文件)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 1m 22s —— View job


PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check for previous inline review comments and resolve fixed threads
  • Review new changes since last review (d7d81ab, 2ff51d5, a31922b)
  • Read all affected files for context
  • Post summary

✅ LGTM — All previous issues fixed, no new issues found

3 new commits since last review (all clean):

Commit Description Assessment
d7d81ab Add description field to agent config Schema, types, merge logic, and /config display all correctly wired. Optional field, no breaking change.
2ff51d5 Delete pm.example.md and .gitkeep files Clean removal of unused placeholder files.
a31922b Semantic placeholders in team.example.md Replaces fake Alice/Bob/Carol data with onboarding-friendly templates. Good UX improvement.

Previous 4 issues (all resolved in 2237f40):

  1. 🔴 Multi-bot _defaultClient uninitializedinitDefaultClient() now called before mode branch
  2. 🟡 Empty HMAC keygetPrimaryAppSecret() throws on missing accounts
  3. 🟡 deriveBotAccounts() per-call recomputation → Cached at load/reload time
  4. 🟡 deriveBindings() per-message overhead → Cached at load/reload time

What looks good across the full PR:

  • Clean description field addition: Zod schema → type → merge logic → /config display, all consistent
  • Caching strategy in config-loader.ts (lines 43-45, 203-205) correctly invalidates on reload
  • initDefaultClient placement at index.ts:148 ensures both single/multi-bot modes work
  • No credential leakage in logs (only accountId/agentId logged, never secrets)
  • Example config with semantic placeholders is a nice onboarding improvement

都是去飞书后台操作不需要回传值,没必要分两轮等用户回复。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous inline review comments and resolve fixed threads
  • Review latest changes since last review (cd415bf)
  • Read all affected files for context
  • Post summary

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 1m 46s —— View job


PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous inline review comments and resolve fixed threads
  • Review latest changes since last review (cd415bf, 84ac72c)
  • Read all affected files for context
  • Post summary

✅ LGTM — All 4 previous issues fixed, 1 minor issue found

4 previous issues (all resolved in 2237f40, threads already resolved):

  1. 🔴 Multi-bot _defaultClient uninitializedinitDefaultClient() now called before mode branch at index.ts:148
  2. 🟡 Empty HMAC keygetPrimaryAppSecret() throws on missing accounts
  3. 🟡 deriveBotAccounts() per-call recomputation → Cached at load/reload time (config-loader.ts:204)
  4. 🟡 deriveBindings() per-message overhead → Cached at load/reload time (config-loader.ts:205)

2 new commits since last review (both clean):

Commit Description Assessment
cd415bf Merge permission & event subscription into one onboarding step Cleaner UX — fewer round-trips. Required/recommended labels are a nice touch.
84ac72c Reorder onboarding: Bot persona before team info Sensible — personality is more immediately relevant than org structure.

1 new minor issue (not in diff, can't inline comment):

# Severity File Issue Confidence
1 🟡 Warning src/onboarding/bootstrap.ts:151 Stale Phase 6 validation: Step 1 says "读取 .env 确认关键配置已就绪(FEISHU_APP_ID, FEISHU_APP_SECRET, ANTHROPIC_API_KEY 非空非占位符)" — but this PR moved FEISHU_APP_ID/FEISHU_APP_SECRET from .env to agents.json. The onboarding agent will look for values that no longer exist in .env. Should check agents.json for feishu credentials instead. 88

Fix this →

What looks good across the full PR:

  • Clean caching strategy in config-loader.ts (lines 43-45, 203-205) — invalidates correctly on reload
  • initDefaultClient at index.ts:148 ensures both single/multi-bot modes work
  • Onboarding flow improvements: merged permission/event step reduces user round-trips; persona-first ordering is more natural
  • No credential leakage in logs (only accountId/agentId logged, never secrets)
  • Semantic placeholders in team.example.md and cleanup of unused .gitkeep/pm.example.md

Alice/Bob/Carol 已从模板中移除,测试改为检查 "占位" 关键词。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented Apr 8, 2026

Claude finished @lishuceo's task in 2m 26s —— View job


PR Re-review: feat: 飞书凭证迁入 agents.json

  • Check previous inline review comments and resolve fixed threads
  • Review latest changes since last review (94ed5b4)
  • Read all affected files for context
  • Post summary

✅ All 4 previous issues fixed, 1 minor issue persists

4 previous issues (all resolved in 2237f40, all threads already resolved):

# Previous Issue Fix Verified
1 🔴 Multi-bot _defaultClient uninitialized initDefaultClient() called at index.ts:148 before mode branch
2 🟡 Empty HMAC key in OAuth getPrimaryAppSecret() throws on missing accounts (oauth.ts:39)
3 🟡 deriveBotAccounts() per-call recomputation Cached at load/reload time (config-loader.ts:204)
4 🟡 deriveBindings() per-message overhead Cached at load/reload time (config-loader.ts:205)

1 issue still present (flagged in last review, not in diff so can't inline comment):

# Severity File Issue Confidence
1 🟡 Warning src/onboarding/bootstrap.ts:151 Stale Phase 6 validation: Step 1 says "读取 .env 确认关键配置已就绪(FEISHU_APP_ID, FEISHU_APP_SECRET, ANTHROPIC_API_KEY 非空非占位符)" — but this PR moved feishu credentials from .env to agents.json. The onboarding agent will incorrectly look for values that no longer exist in .env. Should check agents.json for feishu credentials and only check .env for ANTHROPIC_API_KEY. 85

Fix this →

3 new commits since last full review (all clean):

Commit Description Assessment
cd415bf Merge permission & event subscription into one onboarding step Cleaner UX, fewer round-trips
84ac72c Reorder onboarding: Bot persona before team info Sensible ordering
94ed5b4 Bootstrap test adapts to template placeholder changes Test-only, clean

What looks good across the full PR:

  • Clean Zod schema additions (FeishuAccountSchema, AgentBindingSchema, description field)
  • Caching in config-loader.ts correctly invalidates on reload while eliminating per-request overhead
  • initDefaultClient at index.ts:148 ensures both single/multi-bot modes work
  • No credential leakage in logs (only accountId/agentId logged, never secrets)
  • Onboarding flow improvements: merged permission/event step, persona-first ordering
  • Test updates correctly reflect new source of truth (deriveBotAccounts mock in oauth.test.ts)

Recommendation: Fix the 🟡 Phase 6 issue before merging — it's a low-risk prompt bug but will confuse the onboarding agent on fresh deploys.

@lishuceo lishuceo merged commit da2ac6d into main Apr 8, 2026
4 checks passed
@lishuceo lishuceo deleted the feat/feishu-in-agents-json branch April 8, 2026 13:21
lishuceo added a commit that referenced this pull request Apr 8, 2026
* feat: 飞书凭证迁入 agents.json,统一配置入口

每个 agent 内嵌 feishu 字段(appId + appSecret),一个文件看到全貌。
系统启动时从 agents.json 推导 bot 账号和路由规则。

主要改动:
- config-schema: 新增 FeishuAccountSchema + AgentBindingSchema
- config-loader: deriveBotAccounts/deriveBindings 从 agent config 推导
- config.ts: 移除 FEISHU_APP_ID/SECRET、BOT_ACCOUNTS、AGENT_BINDINGS
- index.ts: 启动顺序改为 agentConfig → deriveBotAccounts → accountManager
- client.ts: FeishuClient 构造函数 appId/appSecret 改为必传,增加 initDefaultClient
- server.ts: startServer 接收 primaryBot 参数
- oauth.ts: 从 deriveBotAccounts 获取凭证
- onboarding prompt: 飞书 keys 写入 agents.json 而非 .env

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: 移除 index.ts 中未使用的 isMultiBotMode 导入

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address PR review — multi-bot initDefaultClient + 缓存优化 + OAuth 安全

- 多 bot 模式也调用 initDefaultClient(修复 AsyncLocalStorage 无值时崩溃)
- deriveBotAccounts/deriveBindings 改为缓存模式,reload 时刷新
- OAuth getPrimaryAppSecret 无账号时 throw 而非返回空字符串

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: agent 配置新增 description 字段,示例配置加角色说明

description 字段解释 agent 的定位和特点:
- PM: Chat Bot — 只读、直接回复、轻量对话
- Dev: Dev Bot — 完整读写、创建话题、编码任务

/config 命令也展示 description。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: 删除 pm.example.md 和 .gitkeep

- pm.example.md 已被 assistant.example.md 替代
- .gitkeep 不再需要(目录中已有 .example.md 文件)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: team.example.md 占位符语义化,去掉 Alice/Bob/Carol 假数据

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: onboarding 权限和事件订阅合并为一步提示

都是去飞书后台操作不需要回传值,没必要分两轮等用户回复。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: onboarding 先收集 Bot 人格再收集团队信息

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: bootstrap 测试适配模板占位符改动

Alice/Bob/Carol 已从模板中移除,测试改为检查 "占位" 关键词。

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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