Skip to content

feat(feishu): enrich observed contact labels - #8569

Merged
wenshao merged 11 commits into
QwenLM:mainfrom
BenGuanRan:feat/feishu-observed-contact-labels
Aug 7, 2026
Merged

feat(feishu): enrich observed contact labels#8569
wenshao merged 11 commits into
QwenLM:mainfrom
BenGuanRan:feat/feishu-observed-contact-labels

Conversation

@BenGuanRan

@BenGuanRan BenGuanRan commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This change enriches Feishu observed-contact entries with the sender's display name and the group's name. It records the existing ID-based observation first, resolves both names in the background, and writes an enriched observation when either lookup succeeds. Completed OpenAPI lookups are cached for the lifetime of the channel instance so repeated messages do not repeatedly call Feishu OpenAPI, while a token acquisition failure before the lookup can retry.

Why it's needed

Feishu message callbacks identify senders and chats by ID, so daemon contact discovery currently exposes opaque labels such as ou_… and oc_…. Recognizable labels make discovered contacts usable in channel-management interfaces while preserving message delivery when permissions are absent or Feishu OpenAPI is unavailable.

Reviewer Test Plan

How to verify

Configure a Feishu bot with contact:user.basic_profile:readonly and im:chat:readonly, start the daemon, and mention the bot in a group. Confirm the message reaches the Agent immediately, the initial observation remains ID-based, and a later observation updates the sender and group labels. Send a second message from the same sender in the same group and confirm no additional name requests are issued. Then remove either permission and repeat with a new contact; message processing should continue, the unresolved ID should remain as the label, and no lookup-specific error should be logged.

Automated verification passed with 561 channel-base tests and 80 Feishu adapter tests. npm run build, npm run typecheck, and npm run lint also completed successfully. A full npm test run exposed 15 failures in two untouched CLI test files (14 AuthDialog interaction assertions and one memory-diagnostics symlink-cleanup assertion); both the tests and their production code are identical to origin/main, and the failures reproduce when those files run alone on this macOS/Node.js 25 environment.

Evidence (Before & After)

Before: the observed user and group labels equal their callback IDs (ou_… and oc_…).

After: the ID observation is still available immediately, followed asynchronously by an observation whose labels contain the Feishu user and group names. Automated tests hold the lookups unresolved while confirming that the Agent prompt has already started, delay a second reply until after enrichment and confirm its stale snapshot is healed, and verify token-acquisition retry plus 401 invalidation.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node.js 25.2.1, native macOS environment without a sandbox.

Risk & Scope

  • Main risk or tradeoff: Name and completed lookup caches live until the daemon recreates the channel, so renamed contacts and failures after an actual lookup request are not refreshed during the same process lifetime.
  • Not validated / out of scope: Windows/Linux runtime behavior and the review-fix commit were not exercised against live Feishu; an earlier branch-tip live daemon run verified real callback delivery and successful user/group name enrichment, while the review fixes are covered by deterministic unit tests.
  • Breaking changes / migration notes: None. Missing permissions or lookup failures retain the existing ID labels and do not change message delivery.

Linked Issues

Closes #8566

中文说明

本 PR 做了什么

此变更为飞书已观测联系人补全发送者显示名和群名。它先记录现有的 ID 观测结果,再在后台并行查询两个名称;任一查询成功后,就补写一条带名称的观测结果。已实际执行的 OpenAPI 查询会在 channel 实例生命周期内缓存,因此重复消息不会反复调用飞书 OpenAPI;若在查询前获取 token 失败,后续消息仍可重试。

为什么需要

飞书消息回调使用 ID 标识发送者和会话,因此 daemon 联系人探测目前只能展示 ou_…oc_… 之类的不透明标签。可识别的标签能让探测到的联系人直接用于 channel 管理界面,同时在权限未开通或飞书 OpenAPI 不可用时继续保障消息投递。

Reviewer 测试计划

如何验证

为飞书机器人配置 contact:user.basic_profile:readonlyim:chat:readonly,启动 daemon,并在群里艾特机器人。确认消息立即进入 Agent,首次观测仍使用 ID,随后出现一条更新了发送者和群标签的观测结果。由同一发送者在同一群再次发送消息,确认不会新增名称查询。然后移除其中一个权限,换一个新联系人重复验证;消息处理应继续进行,未解析的 ID 应保留为标签,并且不应输出名称查询专属错误。

自动验证已通过 561 条 channel-base 测试和 80 条飞书适配器测试。npm run buildnpm run typechecknpm run lint 也均已成功完成。全量 npm test 暴露了两个未改动 CLI 测试文件中的 15 条失败(14 条 AuthDialog 交互断言和 1 条内存诊断符号链接清理断言);这些测试及其生产代码与 origin/main 完全一致,并且在当前 macOS/Node.js 25 环境中单独运行相应文件也能复现。

证据(变更前后)

变更前:已观测用户和群的标签等于回调中的 ID(ou_…oc_…)。

变更后:ID 观测结果仍会立即可用,随后异步出现一条以飞书用户名和群名作为标签的观测结果。自动化测试会在查询未完成时确认 Agent prompt 已开始;还会把第二条回复延迟到补全写入之后,确认旧快照会被再次修复,并验证 token 获取失败重试与 401 失效处理。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

Node.js 25.2.1,macOS 原生环境,未启用 sandbox。

风险与范围

  • 主要风险或取舍:名称缓存和已完成查询的结果会保留到 daemon 重建 channel 为止,因此同一进程生命周期内不会刷新联系人改名或实际查询请求返回的失败。
  • 未验证或不在范围内:未在 Windows/Linux 上验证,也未对本次评审修复提交重新执行真实飞书联调;此前的分支提交已通过真实 daemon 验证回调投递及用户/群名称补全,本次评审修复由确定性单元测试覆盖。
  • 破坏性变更或迁移说明:无。权限缺失或查询失败时继续保留现有 ID 标签,不改变消息投递。

关联 Issue

Closes #8566

@BenGuanRan

Copy link
Copy Markdown
Collaborator Author

Feishu Observed-Contact Label Enrichment E2E

Baseline

  • Start the released global qwen daemon with a Feishu channel whose bot can receive group messages.
  • Mention the bot in a group and inspect the daemon's observed-contact output.
  • Expected before this change: sender and group labels equal their Feishu IDs.

Verification

  • Grant the app contact:user.basic_profile:readonly and im:chat:readonly.
  • Start a daemon built from this branch and mention the bot in a group.
  • Confirm the Agent receives the message without waiting for either name lookup.
  • Confirm the first observation may contain IDs and a later observation replaces them with the sender and group names.
  • Send another message from the same sender in the same group and confirm no additional user or group lookup is issued.
  • Revoke either permission and repeat with a new ID; confirm the message still reaches the Agent, the ID label remains, and no lookup-specific error is logged.

Current result

  • Baseline callback and both OpenAPI response shapes were exercised before implementation.
  • Branch behavior is covered by focused unit tests; a post-build live daemon run remains for reviewer verification because credentials are not stored in the repository.

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 5, 2026
@BenGuanRan

Copy link
Copy Markdown
Collaborator Author

Live Feishu E2E report

Tested commit c82d3f131b783bcb7126a95f7c754435f262cb5a using a freshly built and bundled qwen serve --channel daemon with an isolated Qwen home and a real Feishu WebSocket connection.

Verified

  • Feishu app authentication and WebSocket connection succeeded; the bot identity was resolved.
  • Real im.message.receive_v1 callbacks were received for both P2P and group messages.
  • A real group @mention passed the mention gate.
  • P2P and group messages both reached the daemon Agent path: sessions were spawned and prompts were accepted with HTTP 202.
  • Contact enrichment was asynchronous and did not block or reject the inbound prompt path.
  • The first user lookup called contact/v3/users/basic_batch once. The first group lookup called im/v1/chats/:chat_id once.
  • Both lookups returned HTTP 400 / Feishu code 99991672 because the test app lacked the required API scopes. The observed-contact endpoint therefore correctly retained full platform IDs as labels.
  • A second P2P callback and a second group callback for the same user/group caused no additional user or group lookup. This verifies the per-process lookup cache, including failed lookups.
  • Failure feedback was delivered back to Feishu successfully; each send returned HTTP 200 / code 0.

Not yet verified

  • Successful replacement of user and group ID labels with display names. To complete this case, grant and publish contact:user.basic_profile:readonly and either im:chat:readonly or im:chat:read, restart the channel worker (failed lookups are cached for the process lifetime), then send another group @mention.
  • A successful model-generated reply. The local model credential used by this test returned 401 after prompt acceptance; this is independent of Feishu callback and contact enrichment behavior.

No application credentials were written to the repository or included in this report.

@BenGuanRan
BenGuanRan marked this pull request as ready for review August 5, 2026 13:17
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Gate re-run — four review-fix commits from the autofix loop have landed since the last triage pass, so this re-checks the gate against the current head.

  • Template: complete ✓ — all required sections present, bilingual.
  • Problem: real, not theoretical. Linked feat(feishu): enrich observed contact labels asynchronously #8566 describes observable current behavior: Feishu callbacks identify senders and chats by ID, so daemon contact discovery exposes ou_… / oc_… as labels. Self-reported by the author, but concrete and verifiable by inspection of the current code.
  • Direction: aligned. Feishu is an actively maintained channel in this repo (a dozen CHANGELOG entries, including previously merged work from this same author), and readable labels are what make discovered contacts usable in channel-management surfaces.
  • Size: cross-package (channels base + feishu + a 9-line cli wire-up), so the Stage 0 two-tier gate applies. Not a refactor, so no hard block; production logic is 291 lines (ChannelBase 29 + FeishuAdapter 253 + daemon-worker 9) vs 1263 test lines and 413 docs lines — under the 500-line awareness threshold. The base-package change is purely additive (optional list?(), a default no-op hook, private→protected visibility), and daemon-worker is the only production caller that constructs observedContacts, so every downstream consumer is named.
  • Approach: matches what the issue prescribed — ID observation persisted first, background lookup, silent failure, ID labels retained when the optional scopes are absent. Design/plan docs and the user-facing scope docs follow repo conventions. No unrelated changes in the diff.
  • Risk: no elevated risk signals — none of the changed files match the revert-correlated path list.

Moving on to code review. 🔍

中文说明

Gate 复查 —— 自上次 triage 以来 autofix 循环已合入四个 review-fix commit,本次按当前 head 重新过 gate。

  • 模板:完整 ✓ —— 必填章节齐全,中英双语。
  • 问题:真实存在而非理论问题。关联的 feat(feishu): enrich observed contact labels asynchronously #8566 描述的是当前可观察的行为:飞书回调以 ID 标识发送者和会话,daemon 联系人探测因此只能展示 ou_… / oc_… 这样的标签。虽为作者自报,但具体且可通过现有代码直接确认。
  • 方向:对齐。飞书是本仓库持续维护的渠道(CHANGELOG 有十余条记录,作者此前也有已合入的工作),可读的联系人标签正是让探测结果能用于渠道管理界面的前提。
  • 规模:跨包改动(channels base + feishu + cli 9 行接线),适用 Stage 0 两级 gate。非 refactor,不触发硬阻断;生产逻辑 291 行(ChannelBase 29 + FeishuAdapter 253 + daemon-worker 9),测试 1263 行、文档 413 行 —— 低于 500 行的维护者关注阈值。base 包的改动纯增量(可选 list?()、默认空实现钩子、private→protected 可见性调整),生产代码中只有 daemon-worker 构造 observedContacts,下游消费方可以逐一点名。
  • 方案:与 issue 规定的一致 —— 先落 ID 观测、后台查询、失败静默、未开通可选权限时保留 ID 标签。design/plan 文档与用户文档均符合仓库约定,diff 中无无关改动。
  • 风险:无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。

进入代码审查。🔍

Qwen Code · qwen3.8-max

Reviewed at 119e565317ad821368f64aac48877cb12d0d1464 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Code review

Re-run at the current head — this pass covers the four review-fix commits landed since the last full review (bounded caches, observation-recency hydration, write de-duplication, silent token-refresh logging).

My independent baseline for this problem was exactly the shape the PR uses: persist the ID-based observation immediately, then fire-and-forget a background lookup with per-instance singleflight caches, silent failures, and sanitized names. The PR matches it, and goes further in ways that each answer a real failure mode rather than speculate: restart hydration from the persisted registry so known labels don't regress to raw IDs after a daemon restart, a 500-entry cap on every runtime cache with re-hydration after eviction, a observedContactWrites map that skips redundant observation writes when labels are unchanged, and a core-waiter flag on token refresh so enrichment-initiated refreshes stay silent while core delivery callers still see token errors. Each of these has dedicated tests.

Checks I ran against the code, all clean:

  • Singleflight lifecycle — a token-acquisition failure before the request and 401 responses delete the lookup entry (retryable on the next message; a 401 also drops the cached token), while failures whose request reached Feishu stay cached until restart — matching the issue's "at most once per instance".
  • Core-waiter flag race — the flag is set before joining an in-flight refresh and read at failure time inside refreshToken, so a core caller that joins a silent-initiated refresh still gets its error logged.
  • Channel isolation — hydration filters the persisted graph by channelName, so labels don't leak between channel instances hosted by the same daemon.
  • Injection surface — resolved names pass through the shared sanitizeSenderName before caching, chatId is encodeURIComponent-ed into the URL path, and the persistence layer already falls back to ID labels when a name sanitizes to unknown.

No blockers, no convention violations.

sequenceDiagram
    participant P1 as Feishu WebSocket event
    participant P2 as ChannelBase processInbound
    participant P3 as ObservedContactStore
    participant P4 as FeishuChannel enrichment
    participant P5 as Feishu OpenAPI
    P1->>P2: preflighted envelope
    P2->>P3: persist ID-based observation
    P2->>P4: onObservedContact hook, not awaited
    P4->>P5: user and chat lookups, singleflight cached
    P5-->>P4: names or silent failure
    P4->>P3: enriched observation, same contact key
Loading

Testing — CI evidence via API (no PR code executed in this run)

The macOS, Windows, and CLI-integration jobs are gated on merge_group in ci.yml — they never run on PR push and will run when this PR enters the merge queue, so the skips below are structural, not caused by this PR. The per-PR Ubuntu gate is green at the reviewed head, with the 24 new Feishu adapter tests and 2 new ChannelBase tests included.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
precheck-pr / precheck ✅ success
Test (macos-latest, Node 22.x) ⏭️ skipped — merge queue only
Test (windows-latest, Node 22.x) ⏭️ skipped — merge queue only
Integration Tests (CLI, No Sandbox) ⏭️ skipped — merge queue only

Sandboxed verification would settle the one claim still resting on the author's word: that live enrichment works against the real Feishu OpenAPI at the current head — the live E2E report in this thread ran against the pre-review-fix commit c82d3f13, and the four fix commits since are covered by mocked unit tests only. @qwen-code /verify would A/B the enrichment path against the base build (author holds write access, so this is a regular run, not a sponsored one).

Not verified: live Feishu OpenAPI behavior (no credentials available here, and triage never executes PR code); Windows/Linux runtime behavior (author tested on macOS). The earlier maintainer change-request about the skipped integration suite is now moot by design — that job runs in the merge queue, not on PR push.

中文说明

代码审查

本次在当前 head 复审,覆盖上次完整审查之后合入的四个 review-fix commit(缓存上限、按观测时间回填、写入去重、静默 token 刷新日志)。

我独立构思的方案与 PR 的形状完全一致:先立即落盘 ID 观测,再以 fire-and-forget 方式后台查询,配合实例内 singleflight 缓存、失败静默和名称净化。PR 在此基础上更进一步,且每一点都对应真实故障模式而非臆测:daemon 重启后从持久化记录回填已知名称,避免标签退化为原始 ID;所有运行时缓存设 500 条上限,驱逐后重新回填;observedContactWrites 在标签未变时跳过重复的观测写入;token 刷新引入 core-waiter 标记,使补全发起的刷新保持静默,同时核心投递调用方仍能看到 token 错误。每一项都有专门测试覆盖。

逐项检查均无问题:singleflight 生命周期(请求发出前的 token 获取失败与 401 会删除查询条目、下一条消息可重试,401 同时失效缓存 token;请求已送达飞书的失败则缓存至重启,符合 issue 的"每实例至多一次");core-waiter 标记的竞态(标记在加入既有刷新之前设置、在 refreshToken 失败时读取,因此加入静默刷新的核心调用方仍能看到错误日志);渠道隔离(回填按 channelName 过滤,同一 daemon 上多个渠道实例互不泄漏标签);注入面(解析出的名称先经共享的 sanitizeSenderName 净化再缓存,chatId 经 encodeURIComponent 进入 URL 路径,持久化层在名称净化为 unknown 时本就会回退为 ID 标签)。

无阻断问题,无规范违规。上方时序图展示了关键路径:预检通过 → 立即落盘 ID 观测 → 钩子异步触发补全(不被等待)→ 查询成功后以相同联系人键补写一条带名称的观测。

测试 —— 通过 API 获取的 CI 证据(本次运行未执行任何 PR 代码)

macOS、Windows 与 CLI 集成测试在 ci.yml 中限定 merge_group 事件触发 —— PR 推送时本就不运行,进入合并队列时才会运行,因此下表中的 skipped 是机制使然,与本 PR 无关。逐 PR 的 Ubuntu 门禁在受审 head 上为绿,包含新增的 24 条飞书适配器测试与 2 条 ChannelBase 测试。

沙盒验证可以了结目前仍依赖作者自述的唯一主张:当前 head 对真实飞书 OpenAPI 的在线补全 —— 本帖中的在线 E2E 报告是在 review-fix 之前的 commit c82d3f1 上跑的,此后四个修复 commit 仅有 mock 单测覆盖。@qwen-code /verify 可对补全路径做 A/B 加载性验证(作者有 write 权限,属常规运行而非托管运行)。

未验证:真实飞书 OpenAPI 行为(此处无凭据,且 triage 从不执行 PR 代码);Windows/Linux 运行时行为(作者仅在 macOS 上测试)。此前维护者关于集成测试被跳过的 change-request 现已因机制澄清而消解 —— 该任务在合并队列中运行,而非 PR 推送时。

Qwen Code · qwen3.8-max

Reviewed at 119e565317ad821368f64aac48877cb12d0d1464 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean across every stage at the current head; the only gap is that the live OpenAPI success path is covered at this head by mocked tests plus an author E2E from an earlier commit, which is exactly what the /verify line in my review comment is for.

This is the third triage pass on this PR, and the progression is the opposite of review-bloat: each round removed a real gap. The original pass approved an unbounded-cache version; the review rounds since produced bounded caches, restart hydration that honors observation recency, redundant-write de-duplication, and silent-but-core-visible token logging — all small, each tested, none speculative. The result is now tighter than my independent proposal, not looser.

Stepping back: the problem is genuinely observed (raw ou_…/oc_… labels in discovered contacts), the fix is the minimal shape the issue prescribed, and the diff is proportionate — 291 production lines against 1263 test lines and full docs coverage. The base-class surface stays purely additive, so the other channel adapters are untouched by construction. If I were maintaining this in six months, the lookup-lifecycle comment block and the per-failure-mode tests are exactly what I'd want to find. A maintainer has already approved this exact head, and the earlier change-request about the skipped integration suite is resolved by design — that job is merge-queue-only in ci.yml.

The one honest reservation, carried from Stage 2: the live success path against real Feishu OpenAPI is not independently verified at this head. That's a verification gap, not a code doubt — approving, with /verify named above if anyone wants the claim settled before the merge queue does.

中文说明

置信度:4/5 —— 当前 head 在各阶段均无问题;唯一的缺口是真实 OpenAPI 在线成功路径在此 head 上只有 mock 测试与作者在更早 commit 上的 E2E 覆盖,这正是审查评论中 /verify 一行的用途。

这是本 PR 的第三轮 triage,走向与"审查膨胀"相反:每一轮都在消除真实缺口。首轮通过的是缓存无上限的版本;此后的审查轮次补齐了缓存上限、按观测时间回填的重启水合、冗余写入去重、以及静默但对核心调用方可见的 token 日志 —— 每项都小、都有测试、都不臆测。最终实现比我独立构思的方案更收敛而非更发散。

整体看:问题是真实观测到的(探测出的联系人只有 ou_…/oc_… 标签),修复是 issue 规定的最小形状,diff 比例得当 —— 291 行生产代码对应 1263 行测试与完整文档。base 类接口保持纯增量,其余渠道适配器从构造上就不受影响。若半年后由我维护,查询生命周期的注释块与按故障模式划分的测试正是我希望看到的东西。维护者已在同一 head 上批准;此前关于集成测试被跳过的 change-request 也已因机制澄清而消解 —— 该任务在 ci.yml 中限定合并队列触发。

唯一保留意见(承接 Stage 2):真实飞书 OpenAPI 的在线成功路径尚未在此 head 上独立验证。这是验证缺口而非代码疑虑 —— 予以批准;如有人在进入合并队列前想坐实该主张,上方已给出 /verify 触发方式。

Qwen Code · qwen3.8-max

Reviewed at 119e565317ad821368f64aac48877cb12d0d1464 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max via Qwen Code /review (v0.21.5)

Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread docs/design/feishu-observed-contact-label-enrichment.md
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
@QwenLM QwenLM deleted a comment from danialzivehdadr Aug 5, 2026
@QwenLM QwenLM deleted a comment from danialzivehdadr Aug 5, 2026
@QwenLM QwenLM deleted a comment from danialzivehdadr Aug 5, 2026

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): npm testexit 1.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):npm testexit 1

— qwen3.8-max via Qwen Code /review (v0.21.6)

Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
@wenshao

wenshao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 6, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 3 finishedview run. See this round's report below.

中文说明

AutoFix 第 3 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #8569

No base conflict was reported (--conflict false); no merge was performed. All changes are in commit d2dec013a on the PR branch.

Feedback addressed this round

  • [rc:3727592884] 401 permanently poisons the per-ID lookup cache — Resolved. A 401 lookup now evicts the per-ID promise entry in addition to clearing tokenCache, so the victim ID is retried on the next observation once the token refresh succeeds. Because the lookup lifecycle was extracted this round (below), the fix lives in one place and covers both the user and chat lookups. Pinned by the extended 401 test and the new retries a 401 lookup for the same ID once the token refreshes test; a mutation probe removing the eviction makes the test fail.
  • [rc:3727592897] Name caches never hydrated from the persisted store — Resolved. The observedContacts channel option gained an optional read path (list()), wired in the daemon worker to the observed-contact store at the maximum freshness window. ChannelBase exposes the persisted graph for the current channel, and FeishuChannel hydrates both name caches from it once, on the first inbound message. After a daemon restart the envelope therefore carries the persisted display name, the initial write no longer reverts enriched labels to raw IDs, and a failed re-lookup can no longer lose a known name. Pinned by the new hydrates label caches from persisted observations after a restart test (429 lookups, labels survive on both messages, registry read happens once); a mutation probe skipping hydration makes it fail.
  • [rc:3727592901] K in-flight messages trigger K redundant full-file enrichment writes — Resolved. The adapter now tracks the last persisted labels per contact key (updated both by the post-initial-write hook and by enrichment writes) and skips the healing write when the resolved labels already match what is persisted. A burst of K messages from a new sender while the shared lookup is pending now produces exactly one enrichment write; the stale-snapshot and collect-drain healing paths still write because the intervening initial write resets the tracked labels. Pinned by the new shares one in-flight lookup across concurrent messages test, which asserts exactly 3 observations (2 initial + 1 enrichment) — it fails when the dedup is removed.
  • [rc:3727592914] Resolved display names never pass through sanitizeSenderName — Resolved. Resolved names are sanitized before entering the in-memory name caches (a name collapsing to unknown is treated as unresolved), so the envelope/pairing sinks no longer receive raw platform names. Pinned by the new sanitizes resolved display names before caching them test.
  • [rc:3727592927] observedChatName is a near-copy of observedUserName — Resolved. Both now delegate to one shared observedNameLookup helper that owns inflight-promise dedup, token-acquisition retry eviction, 401 token-cache invalidation plus entry eviction, silent failure, name sanitization, and name caching; the two wrappers only supply the endpoint request and response extraction. The lifecycle fixes above land in a single body, removing the drift risk this finding described.
  • [rc:3727592943] Chat-side retry eviction unpinned — Resolved. Added retries a chat lookup when token acquisition fails before the request, mirroring the user-side test. With the shared lifecycle, the user-side and chat-side tests together pin the single eviction site from both directions.
  • [rc:3727592955] Envelope-side label attachment unpinned — Resolved. Added attaches cached labels to later envelopes and prompts, asserting the second message's prompt carries the [Alice] attribution and its observation carries the enriched group label. Mutation probe: reverting the cache reads at envelope-construction time makes this test fail.
  • [rc:3721908753] Partial-success path untested (round 1) — Resolved. Added writes an enriched observation when only the user lookup resolves: user lookup succeeds, chat lookup returns a non-zero Feishu code; asserts the second observation carries the enriched user label with the ID group label, and the follow-up message adds no write or fetch.
  • [rc:3721908758] Direct-chat enrichment untested (round 1) — Resolved. Added enriches direct-chat senders without issuing chat lookups: two p2p messages, one user fetch, zero /im/v1/chats/ fetches, one observation per message plus the single enrichment.
  • [rc:3721908767] Only the open_id branch of the ID-type mapping exercised (round 1) — Resolved. Added maps non-open_id senders to the matching user_id_type with union_id-only and user_id-only sender variants, asserting the user_id_type query parameter per branch.
  • [rc:3721908776] In-flight lookup sharing untested (round 1) — Resolved. Covered by the new shares one in-flight lookup across concurrent messages test: two group messages are delivered before the deferred lookups resolve; after settlement each endpoint was fetched exactly once and the final state is enriched.
  • [rc:3727592906] basic_batch batching / 429 caching interaction — Declined. See the threaded reply: batching would require delaying lookups to batch IDs, which conflicts with this feature's no-delay design goal; per-ID promise dedup already bounds the cost to one request per concurrently arriving new ID, and the finding itself notes leaving this as-is is defensible. The restart-hydration fix this round also removes the worst consequence of permanent negative caching, since persisted labels are no longer overwritten with raw IDs.

Findings re-verified as already fixed (commit 8ef1b1e)

These threads were fixed in the previous round but remained open after the merge with main; each was re-verified against the current HEAD and is listed in resolved-comments.txt:

  • [rc:3721908721] 401 token-cache invalidation at the lookup sites — present at both sites (now inside the shared helper).
  • [rc:3721908745] Optional scopes documented in the Feishu setup guide — present in docs/users/features/channels/feishu.md.
  • [rc:3721908771] Delayed-second-message label revert — healed by the resolved-label comparison; covered by the deterministic delayed-reply regression test, which still passes under this round's reworked comparison.
  • [rc:3721908779] Collect-drain revert — same healing mechanism; synthetic drain envelopes reset the tracked labels and get a healing write.
  • [rc:3722654408] Round-2 Critical (stale ID-labeled observations reverting enriched labels) — fixed; all three probe variants remain covered by the delayed-reply test and the shared comparison logic.
  • [rc:3722654418] Transient token-acquisition failure cached permanently — the !token branch deletes the per-ID entry for both lookups; pinned by the user-side and chat-side retry tests.

Other changes

  • Design doc (docs/design/feishu-observed-contact-label-enrichment.md, both languages) updated to describe the shared lookup lifecycle, the retryable failure classes (token-acquisition failures and 401), name sanitization, and restart hydration.
  • packages/cli/src/commands/channel/daemon-worker.test.ts updated for the new list wiring (mock store, options assertion, delegation check).

Verification

  • npm run build — passed (exit 0)
  • npm run typecheck — passed (exit 0)
  • npx eslint on the five changed source/test files — passed, no findings
  • npx prettier --check on the six changed files — passed (two files auto-formatted first)
  • vitest run src/adapter.test.ts (packages/channels/feishu) — 89/89 passed
  • vitest run src/ChannelBase.test.ts (packages/channels/base) — 561/561 passed
  • vitest run src/commands/channel/daemon-worker.test.ts src/commands/channel/observed-contact-store.test.ts (packages/cli) — 88/88 passed
  • Mutation probes (test efficacy): removing the 401 eviction, skipping hydration, removing the write dedup, and reverting the envelope cache reads each make the corresponding new test fail; the committed code passes all of them.
  • Live Feishu E2E: unchanged from earlier rounds — it requires app credentials that are not stored in the repository, so it remains for reviewer verification per the PR's E2E report. Channel integration tests were not run because the touched behavior is fully exercised by the unit suites above (adapter, ChannelBase hook, daemon-worker wiring), not only through the bundled CLI.
中文说明

Autofix 评审轮次 — PR #8569

未报告与 base 的冲突(--conflict false),未执行 merge。所有变更位于 PR 分支的提交 d2dec013a

本轮处理的反馈

  • [rc:3727592884] 401 永久毒化按 ID 的查询缓存 — 已修复。 401 查询现在除了清除 tokenCache 之外,还会删除该 ID 的 promise 缓存条目,使受害 ID 在 token 刷新成功后可在下一次观测时重试。由于本轮已抽取查询生命周期(见下),该修复只落在一处,同时覆盖用户查询和群查询。由扩展后的 401 测试与新增的 retries a 401 lookup for the same ID once the token refreshes 测试钉住;变异探针(删除该 delete)会使测试失败。
  • [rc:3727592897] 名称缓存从不从持久化存储回填 — 已修复。 observedContacts 渠道选项新增可选读取路径(list()),由 daemon worker 以最大时间窗口接入 observed-contact 存储。ChannelBase 暴露当前渠道的持久化观测图,FeishuChannel 在首条入站消息时一次性回填两个名称缓存。daemon 重启后 envelope 即携带已持久化的显示名,首次写入不再把已补全的标签回退为原始 ID,重新查询失败也不会再丢失已知名称。由新增的 hydrates label caches from persisted observations after a restart 测试钉住(lookup 返回 429、两条消息标签均保持补全、注册表只读取一次);跳过回填的变异探针会使该测试失败。
  • [rc:3727592901] K 条在途消息触发 K 次冗余的整文件补全写入 — 已修复。 适配器现在按联系人 key 跟踪最近一次持久化的标签(首次写入钩子与补全写入都会更新),当已解析标签与持久化内容一致时跳过补全写入。新发送者在共享查询挂起期间连发 K 条消息时,现在只产生一次补全写入;过期快照与 collect 排空的修复路径不受影响,因为 intervening 的首次写入会重置跟踪的标签。由新增的 shares one in-flight lookup across concurrent messages 测试钉住:断言恰好 3 次观测(2 次首次写入 + 1 次补全)——移除去重后该测试失败。
  • [rc:3727592914] 解析出的显示名从未经过 sanitizeSenderName — 已修复。 解析出的名称在进入进程内名称缓存前先做净化(净化结果为 unknown 的名称按未解析处理),envelope/pairing 汇聚点不再收到原始平台名称。由新增的 sanitizes resolved display names before caching them 测试钉住。
  • [rc:3727592927] observedChatNameobservedUserName 的近似拷贝 — 已修复。 两者现在都委托给共享的 observedNameLookup 辅助方法,由它统一负责在途 promise 去重、token 获取失败删除重试、401 token 缓存失效与条目删除、静默失败、名称净化与名称缓存;两个包装方法只提供端点请求与响应提取。上述生命周期修复只需落在一处,消除了本发现指出的分叉风险。
  • [rc:3727592943] 群聊侧重试驱逐未被测试钉住 — 已修复。 新增 retries a chat lookup when token acquisition fails before the request,与用户侧测试对称。共享生命周期之后,用户侧与群聊侧测试共同从两个方向钉住唯一的驱逐点。
  • [rc:3727592955] envelope 侧标签附着未被测试钉住 — 已修复。 新增 attaches cached labels to later envelopes and prompts,断言第二条消息的 prompt 携带 [Alice] 归属标签、其观测携带补全后的群标签。变异探针:回退 envelope 构造处的缓存读取会使该测试失败。
  • [rc:3721908753] 部分成功路径无测试(第一轮)— 已修复。 新增 writes an enriched observation when only the user lookup resolves:用户查询成功、群查询返回非零飞书 code;断言第二次观测携带补全的用户标签与 ID 群标签,且后续消息不再产生写入或请求。
  • [rc:3721908758] 私聊补全无测试(第一轮)— 已修复。 新增 enriches direct-chat senders without issuing chat lookups:两条 p2p 消息、一次用户查询、零次 /im/v1/chats/ 查询、每条消息一次观测外加唯一一次补全。
  • [rc:3721908767] ID 类型映射只覆盖了 open_id 分支(第一轮)— 已修复。 新增 maps non-open_id senders to the matching user_id_type,使用仅 union_id 与仅 user_id 的发送者变体,按分支断言 user_id_type 查询参数。
  • [rc:3721908776] 在途查询共享无测试(第一轮)— 已修复。 由新增的 shares one in-flight lookup across concurrent messages 测试覆盖:在延迟的查询解析之前送达两条群消息;全部稳定后每个端点恰好 fetch 一次,且最终状态为补全标签。
  • [rc:3727592906] basic_batch 批量化 / 429 缓存叠加 — 拒绝。 见线程内回复:批量化需要延迟查询以累积 ID,与本功能「不延迟」的设计目标冲突;按 ID 的 promise 去重已把代价限制为每个并发到达的新 ID 一次请求,且该发现本身也认可维持现状可以辩护。本轮的重启回填修复也消除了永久负缓存的最坏后果——已持久化的标签不会再被回退为原始 ID。

复核确认已在 8ef1b1e 修复的发现

这些线程在上一轮已修复,但在与 main 合并后仍处于打开状态;已逐一在当前 HEAD 上复核,并列入 resolved-comments.txt

  • [rc:3721908721] 两个查询点的 401 token 缓存失效 — 两处均存在(现位于共享辅助方法内)。
  • [rc:3721908745] 用户文档补充可选权限 — docs/users/features/channels/feishu.md 中已存在。
  • [rc:3721908771] 延迟第二条消息导致的标签回退 — 由已解析标签比较修复;确定性延迟回复回归测试覆盖,且在本轮重构比较逻辑后依然通过。
  • [rc:3721908779] collect 排空回退 — 同一修复机制;合成排空 envelope 会重置跟踪的标签并获得补全写入。
  • [rc:3722654408] 第二轮 Critical(过期 ID 标签观测回退已补全标签)— 已修复;三个探针变体仍由延迟回复测试与共享比较逻辑覆盖。
  • [rc:3722654418] 瞬时 token 获取失败被永久缓存 — !token 分支对用户与群查询均删除对应 ID 条目;由用户侧与群聊侧重试测试钉住。

其他变更

  • 设计文档(docs/design/feishu-observed-contact-label-enrichment.md,中英文)更新:描述共享查询生命周期、可重试的失败类别(token 获取失败与 401)、名称净化,以及重启回填。
  • packages/cli/src/commands/channel/daemon-worker.test.ts 随新的 list 接入更新(mock 存储、选项断言、委托检查)。

验证

  • npm run build — 通过(exit 0)
  • npm run typecheck — 通过(exit 0)
  • 对 5 个变更的源码/测试文件运行 npx eslint — 通过,无发现
  • 对 6 个变更文件运行 npx prettier --check — 通过(其中 2 个文件先做了自动格式化)
  • vitest run src/adapter.test.ts(packages/channels/feishu)— 89/89 通过
  • vitest run src/ChannelBase.test.ts(packages/channels/base)— 561/561 通过
  • vitest run src/commands/channel/daemon-worker.test.ts src/commands/channel/observed-contact-store.test.ts(packages/cli)— 88/88 通过
  • 变异探针(测试有效性验证):删除 401 驱逐、跳过回填、删除写入去重、回退 envelope 缓存读取,均会使对应的新增测试失败;提交代码全部通过。
  • 飞书真实 E2E:与前几轮一致——需要未存放在仓库中的应用凭据,按 PR 的 E2E 报告留给评审者验证。未运行渠道集成测试,因为本次变更的行为已由上述单元测试(适配器、ChannelBase 钩子、daemon-worker 接入)完整覆盖,并非只经由打包后的 CLI 才能验证。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Disposition of this round's feedback

The eleven new inline replies from @BenGuanRan each contain only a file reference (@r_T3.txt, @r_T4.txt, @r_T5.txt, @r_T7.txt, @r_T11.txt@r_T13.txt, @r_T15.txt@r_T18.txt). Those referenced files were not delivered to this run's working directory, so their content could not be read. Rather than guess their intent, this round re-verified the underlying finding of every replied thread against the current PR head (3d1b501d6d). Result: all eleven findings are already resolved in the code by the previous round's lifecycle-hardening commit (d2dec013a6 fix(feishu): harden observed-contact label enrichment lifecycle), and each fix is pinned by a focused regression test. No further code change is needed. If an attachment disagreed with one of these fixes or carried new counter-evidence, please restate the essential content as plain text in the thread — the next round will pick it up and act on it.

Point-by-point evidence, all verified at the current head:

  • Partial-success path untested (user lookup resolves, chat lookup fails) — resolved: writes an enriched observation when only the user lookup resolves asserts the first observation keeps both ID labels, the second carries the resolved user name with the ID group label, and a later message issues no further lookups. A regression to an || guard would skip the second write and fail this test.
  • Direct-chat (DM) enrichment untested — resolved: enriches direct-chat senders without issuing chat lookups sends two chat_type: 'p2p' messages and asserts one user fetch, zero /im/v1/chats/ fetches, and exactly one observation per message.
  • Only the ou_open_id ID-type branch exercised — resolved: maps non-open_id senders to the matching user_id_type sends union_id-only and user_id-only senders and asserts the user_id_type query parameter per branch, plus that no open_id-typed call is made.
  • Concurrent in-flight lookup sharing untested — resolved: shares one in-flight lookup across concurrent messages delivers two group messages before resolving the deferred mock fetches, then asserts exactly one fetch per endpoint and both observations ending enriched. Moving the lookup-cache .set() below the first await would duplicate the fetches and fail this test.
  • A 401 lookup permanently poisoned the ID's cache entry — resolved: the shared lookup now deletes the cached promise and clears the token cache on HTTP 401 (both user and chat paths, since both share one helper). retries a 401 lookup for the same ID once the token refreshes proves the same ID re-resolves after a token refresh, and refreshes the token after a label lookup returns 401 covers the cross-ID path.
  • Name caches never hydrated from the persisted store after a restart — resolved: the adapter now hydrates its label caches from persisted observations (new list read path wired through the daemon worker) before constructing envelopes, so a failed re-lookup no longer overwrites known names with raw IDs. hydrates label caches from persisted observations after a restart reproduces the exact reported scenario — persisted names, empty caches, fetch mocked to 429 — and asserts the observations keep the known labels.
  • K messages during a pending lookup caused K enrichment writes — resolved: enrichment writes are deduplicated by comparing against the last persisted labels per contact key. shares one in-flight lookup across concurrent messages pins this directly (two concurrent messages produce one enrichment write, not two), while repairs labels from a delayed message with a stale snapshot pins that a message whose envelope still carried stale ID labels still gets its enrichment write.
  • Resolved display names never passed through sanitizeSenderName — resolved: the shared lookup sanitizes every resolved name before caching it (user and chat symmetrically) and treats an all-control-character name (unknown after sanitization) as unresolved. sanitizes resolved display names before caching them pins the stripping of CR/control sequences.
  • observedChatName was a near-copy of observedUserName — resolved: both now delegate to a single observedNameLookup helper; only the endpoint request and response extraction differ, so lifecycle fixes (401 eviction, token-failure retry, sanitization) can no longer drift between the two.
  • Chat-side retry eviction untested — resolved: retries a chat lookup when token acquisition fails before the request is the chat twin of the user-side pin; deleting the failed-lookup eviction would fail it.
  • Envelope-side label attachment untested — resolved: attaches cached labels to later envelopes and prompts asserts the first prompt carries the raw ID while the second carries the resolved name, and that the later observation is enriched from the envelope caches. Reverting the cache reads at envelope construction would fail it.

Check status note: the review-pr: CANCELLED entry in this round's feedback is a cancelled automated-review run, not a code failure — the test checks on the current head are green and no check remains red.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest run src/adapter.test.ts (packages/channels/feishu) — 89 passed
  • vitest run src/ChannelBase.test.ts (packages/channels/base) — 561 passed
  • vitest run src/commands/channel/daemon-worker.test.ts (packages/cli) — 77 passed
中文说明

本轮反馈的处理

@BenGuanRan 新提出的 11 条行内回复每条都只包含一个文件引用(@r_T3.txt@r_T4.txt@r_T5.txt@r_T7.txt@r_T11.txt@r_T13.txt@r_T15.txt@r_T18.txt)。这些被引用的文件并未随本轮运行提供到工作目录中,因此无法读取其内容。本轮没有猜测其意图,而是对每条回复所在线程的底层发现逐一在当前 PR head(3d1b501d6d)上重新核验。结果:全部 11 条发现均已在代码中解决——由上一轮的生命周期加固提交(d2dec013a6 fix(feishu): harden observed-contact label enrichment lifecycle)完成,且每项修复都有聚焦的回归测试钉住。无需再做代码改动。如果某个附件对下列修复持有异议或包含新的反证,请在对应线程中以纯文本重述关键内容——下一轮会读取并据此处理。

逐条证据如下,均在当前 head 上核验:

  • 部分成功路径无测试(用户查询成功、群查询失败) —— 已解决:writes an enriched observation when only the user lookup resolves 断言第一次观测保留双 ID 标签、第二次观测携带解析出的用户名与 ID 群标签,且后续消息不再发起查询。若守卫退化为 ||,第二次写入会被跳过,该测试将失败。
  • 私聊(DM)补全无测试 —— 已解决:enriches direct-chat senders without issuing chat lookups 发送两条 chat_type: 'p2p' 消息,断言仅一次用户查询、零次 /im/v1/chats/ 查询、每条消息恰好一次观测。
  • ou_open_id 分支被覆盖 —— 已解决:maps non-open_id senders to the matching user_id_type 分别发送仅含 union_id 与仅含 user_id 的发送者,按分支断言 user_id_type 查询参数,并断言未发起任何 open_id 类型调用。
  • 并发在途查询共享无测试 —— 已解决:shares one in-flight lookup across concurrent messages 在解析延迟的 mock fetch 之前送达两条群消息,随后断言每个端点恰好一次 fetch、且两次观测最终均被补全。若把查询缓存的 .set() 移到第一个 await 之后,fetch 会重复,该测试将失败。
  • 401 查询永久毒化该 ID 的缓存条目 —— 已解决:共享查询在 HTTP 401 时会删除已缓存的 promise 并清除 token 缓存(用户与群聊路径共用同一 helper,均已覆盖)。retries a 401 lookup for the same ID once the token refreshes 证明同一 ID 在 token 刷新后可重新解析;refreshes the token after a label lookup returns 401 覆盖跨 ID 场景。
  • 重启后名称缓存从不从持久化存储回填 —— 已解决:适配器现在会在构造 envelope 之前从持久化观测回填标签缓存(daemon worker 接入了新的 list 读取路径),因此重新查询失败不会再用原始 ID 覆盖已知姓名。hydrates label caches from persisted observations after a restart 精确复现了报告的场景——已持久化姓名、空缓存、fetch mock 为 429——并断言观测保留已知标签。
  • 查询未决期间的 K 条消息触发 K 次补全写入 —— 已解决:补全写入改为与每个联系人 key 的上次持久化标签比较去重。shares one in-flight lookup across concurrent messages 直接钉住该行为(两条并发消息只产生一次补全写入,而非两次);repairs labels from a delayed message with a stale snapshot 则钉住 envelope 仍携带过期 ID 标签的消息依然会获得其补全写入。
  • 解析出的显示名从未经过 sanitizeSenderName —— 已解决:共享查询在缓存任何解析出的名称前先做净化(用户与群聊对称处理),并把全控制字符的名称(净化后为 unknown)视为未解析。sanitizes resolved display names before caching them 钉住了 CR/控制字符的剥离。
  • observedChatNameobservedUserName 的近似拷贝 —— 已解决:两者现在都委托给同一个 observedNameLookup helper,仅端点请求与响应提取不同,因此生命周期修复(401 驱逐、token 失败重试、净化)不会再在两份代码间漂移。
  • 群聊侧重试驱逐无测试 —— 已解决:retries a chat lookup when token acquisition fails before the request 是用户侧钉住测试的群聊孪生版本;删除失败查询驱逐逻辑会导致其失败。
  • envelope 侧标签附着无测试 —— 已解决:attaches cached labels to later envelopes and prompts 断言第一次 prompt 携带原始 ID、第二次携带解析出的姓名,且后续观测从 envelope 缓存获得补全。若回退 envelope 构造处的缓存读取,该测试将失败。

检查状态说明:本轮反馈中的 review-pr: CANCELLED 是一次被取消的自动评审运行,并非代码失败——当前 head 上的测试检查均为绿色,也没有保持红色的检查。

验证

  • npm run build —— 通过
  • npm run typecheck —— 通过
  • npm run lint —— 通过
  • vitest run src/adapter.test.ts(packages/channels/feishu)—— 89 通过
  • vitest run src/ChannelBase.test.ts(packages/channels/base)—— 561 通过
  • vitest run src/commands/channel/daemon-worker.test.ts(packages/cli)—— 77 通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

中文说明

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

— gpt-5.6-sol via Qwen Code /review (v0.21.7)

Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/feishu/src/FeishuAdapter.ts
Comment thread packages/channels/feishu/src/FeishuAdapter.ts Outdated
Comment thread packages/channels/base/src/ChannelBase.test.ts
…nrichment token failures (QwenLM#8569)

- hydrate runtime label caches from the newest observation per contact so
  stale group membership labels cannot overwrite more recent ones
- cap the user/chat label, in-flight lookup, and write-dedup maps at 500
  entries (matching the persisted registry) and evict oldest entries
- route best-effort label lookups through a silent token refresh path so
  enrichment failures no longer write to stderr
- add tests for silent token refresh, newest-label hydration, cache cap,
  and the persisted-observation reject path in hook ordering
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no code change needed

Every finding in this round's feedback is already resolved at the current PR head (93bd433da, "fix(feishu): bound label caches, honor observation recency, silence enrichment token failures"). The review snapshot targets the pre-fix tree: the anchored line numbers and described behavior only match the code before that commit, and the regression tests it demands did not exist there but exist now. This round re-verified each finding against HEAD instead of re-applying fixes that are already in.

Inline findings — all verified resolved at HEAD

  • [Critical] R1-1 — restart hydration could replace a newer label with an older observation. Resolved. hydrateObservedNames() now selects the newest non-ID label per user/chat across top-level users and all group members by comparing lastObservedAt before writing the runtime caches. Covered by the passing test prefers the newest persisted label when hydrating overlapping contacts, which feeds an older group-membership label plus a newer top-level label for the same user and asserts the newer one wins.
  • [Critical] R1-2 — name/lookup/write maps grew without bound. Resolved. All five runtime maps (user names, chat names, user lookups, chat lookups, write-dedup state) are capped at OBSERVED_LABEL_CACHE_LIMIT = 500 — the same bound as the persisted observed-contact registry — with oldest-first eviction via capObservedCache(). The write-dedup entries are retained up to the cap rather than deleted on completion on purpose: they are the cross-message dedup cache that stops every inbound message from re-persisting unchanged enriched labels, and the cap alone removes the unbounded-growth/OOM scenario. Covered by the passing test bounds runtime label caches to the observed-contact retention cap.
  • [Critical] R1-3 — best-effort enrichment leaked token failures to stderr. Resolved. getTenantAccessToken()/refreshToken() accept { silent: true }; the enrichment lookup path is the only caller of the silent mode, core delivery paths keep logging. Covered by the passing test keeps enrichment silent when tenant token acquisition fails, which fails the token endpoint, asserts delivery and ID fallback continue, and asserts stderr was never written.
  • [Suggestion] R1-4 — hook-order test covered only successful persistence. Resolved. ChannelBase.test.ts now includes still notifies the adapter after a rejected contact persistence, which makes observe() reject and asserts inbound handling completes, the hook receives the envelope, the prompt runs, and the observed contact persistence failed diagnostic is emitted. Passing.

Failed check: Test (ubuntu-latest Node 22.x)

Not reproducible. Every locally runnable step of that CI job was re-run on the current head and passes (full list in Verification). The two steps that cannot run here are unaffected by this PR: the runtime-dependency audit (this PR changes no dependencies — the lockfile check passes and no package.json is touched) and the actionlint/yamllint/shellcheck binaries (this PR touches no workflow/YAML/shell files; shellcheck passes locally). Evidence that the failing snapshot predates the fix commit: it arrives in the same feedback batch as a review whose findings match only the pre-fix tree, and the fix commit's own message and diff correspond one-to-one to those findings. If the check is still red on the current head, the deterministic gate that re-runs build/typecheck/lint/tests on any new commit — plus a fresh CI run — is the arbiter; no code-level defect survives local reproduction.

Review-level note: "Integration Tests (CLI, No Sandbox) was skipped in CI"

That job is merge-queue-only by design (if: github.event_name == 'merge_group' in ci.yml), so it is skipped on every PR push and cannot be made to run on PR CI without changing CI policy. To close the review gap locally, the suite was run here after npm run build && npm run bundle: 185 passed, 18 skipped, 1 environment-only failureqwen-config-dir.test.ts 1d fails only because this sandbox's /home/github-runner is root-owned while the tests run as uid 1000, so the CLI cannot mkdir ~/.qwen when QWEN_HOME is unset. That is a sandbox-home artifact, not a code defect, and it exercises config-dir bootstrap, which this PR does not touch. All channel/daemon suites in the run pass, including channel-plugin, daemon-invocation-context, qwen-serve-routes, and qwen-serve-streaming.

Verification

Commands actually run this round on head 93bd433da (all exit 0 unless noted):

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint (ESLint, repo + integration-tests) — passed
  • npx prettier --check on all touched files — passed
  • npm run generate:settings-schema + git status on the schema — up to date, no diff
  • npm run check:lockfile — passed
  • npm run check-i18n — passed (warnings are pre-existing coverage notes)
  • npm run check:desktop-isolation — passed
  • npm run check:voice-guard-sync — passed
  • npm run check:serve-fast-path-bundle — passed
  • node scripts/lint.js --sensitive-keywords — passed
  • node --test <13 .github/scripts helper test files> — 206/206 passed
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 passed
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 passed
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 passed
  • npm run test:ci (full unit suite, all workspaces) — passed
  • npm run bundle — passed
  • npm run test:integration:no-ak:sandbox:none (part of the CI Test job) — 129/129 passed
  • npx vitest run --root ./integration-tests cli channel-plugin.test.ts — 185 passed, 18 skipped, 1 environment-only failure described above
  • node scripts/lint.js --shellcheck — passed; --yamllint/--actionlint binaries are CI-installed and irrelevant here (no YAML/action/shell changes in this PR)
  • npm run audit:runtime:critical — not run (networked registry command, disallowed for this agent); moot: no dependency changes in this PR

No code change was made this round; the branch head is unchanged.

中文说明

Autofix 审查轮次 — 无需修改代码

本轮反馈中的每一条发现都已在当前 PR head93bd433da,"fix(feishu): bound label caches, honor observation recency, silence enrichment token failures")中解决。该审查快照针对的是修复前的代码树:其锚定的行号与描述的行为只与那次提交之前的代码吻合,而它所要求的回归测试当时不存在、现在已存在。因此本轮没有重复套用已经存在的修复,而是逐条对照 HEAD 重新验证。

行内发现 — 均已确认在 HEAD 中解决

  • [Critical] R1-1 — 重启恢复缓存时较旧观测可能覆盖较新标签。 已解决。hydrateObservedNames() 现在会在顶层用户和所有群成员之间按 lastObservedAt 比较,为每个用户/群选取最新的非 ID 标签,然后才写入运行时缓存。由通过的测试 prefers the newest persisted label when hydrating overlapping contacts 覆盖:该测试为同一用户同时提供较旧的群成员标签和较新的顶层标签,断言较新者生效。
  • [Critical] R1-2 — 名称/查询/写入 Map 无界增长。 已解决。全部五个运行时 Map(用户名、群名、用户查询、群查询、写入去重状态)均以 OBSERVED_LABEL_CACHE_LIMIT = 500 为上限——与持久化 observed-contact 注册表一致——并通过 capObservedCache() 按最旧优先淘汰。写入去重条目有意保留至上限、而不是在写入完成后删除:它们是跨消息的去重缓存,防止每条入站消息都重复持久化未变化的补全标签;仅上限本身就已消除无限增长/OOM 场景。由通过的测试 bounds runtime label caches to the observed-contact retention cap 覆盖。
  • [Critical] R1-3 — 尽力而为的补全把 token 失败泄漏到 stderr。 已解决。getTenantAccessToken()/refreshToken() 支持 { silent: true };补全查询路径是静默模式的唯一调用方,核心投递路径继续记录日志。由通过的测试 keeps enrichment silent when tenant token acquisition fails 覆盖:该测试令 token 接口失败,断言投递与 ID 回退照常进行,且 stderr 从未被写入。
  • [Suggestion] R1-4 — hook 顺序测试只覆盖持久化成功。 已解决。ChannelBase.test.ts 新增 still notifies the adapter after a rejected contact persistence:令 observe() 拒绝,断言入站处理完成、hook 收到 envelope、prompt 正常执行,且输出 observed contact persistence failed 诊断。测试通过。

失败检查:Test (ubuntu-latest Node 22.x)

无法复现。该 CI job 中所有可在本地运行的步骤都已在当前 head 上重跑并通过(完整列表见 Verification)。仅有两步无法在此运行,且均不受本 PR 影响:运行时依赖审计(本 PR 未改动任何依赖——lockfile 检查通过且未触碰任何 package.json)以及 actionlint/yamllint/shellcheck 二进制(本 PR 未改动任何 workflow/YAML/shell 文件;shellcheck 本地已通过)。该失败快照早于修复提交的证据是:它与一份发现只匹配修复前代码树的审查同批到达,且修复提交的提交信息与 diff 和这些发现一一对应。若该检查在当前 head 上仍然为红,则以对任何新提交重跑 build/typecheck/lint/测试的确定性门禁及新一轮 CI 为准;本地复现未发现任何代码级缺陷。

审查级意见:"Integration Tests (CLI, No Sandbox) was skipped in CI"

该 job 按设计仅在 merge queue 中运行(ci.ymlif: github.event_name == 'merge_group'),因此每次 PR push 都会被跳过;不改变 CI 策略就无法让它在 PR CI 中运行。为在本地弥补该审查缺口,本环境在 npm run build && npm run bundle 之后运行了该套件:185 通过、18 跳过、1 个仅环境导致的失败 —— qwen-config-dir.test.ts 1d 失败仅因为本沙箱的 /home/github-runner 属主为 root,而测试以 uid 1000 运行,未设置 QWEN_HOME 时 CLI 无法 mkdir ~/.qwen。这是沙箱 home 目录问题,不是代码缺陷,且该用例验证的是配置目录初始化,本 PR 并未触碰。本次运行中所有 channel/daemon 套件均通过,包括 channel-plugindaemon-invocation-contextqwen-serve-routesqwen-serve-streaming

Verification(验证)

本轮在 head 93bd433da 上实际执行的命令(除特别说明外均以退出码 0 结束):

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint(ESLint,仓库 + integration-tests)— 通过
  • 对所有改动文件执行 npx prettier --check — 通过
  • npm run generate:settings-schema + 对该文件的 git status — 已是最新、无 diff
  • npm run check:lockfile — 通过
  • npm run check-i18n — 通过(警告为既有的覆盖率提示)
  • npm run check:desktop-isolation — 通过
  • npm run check:voice-guard-sync — 通过
  • npm run check:serve-fast-path-bundle — 通过
  • node scripts/lint.js --sensitive-keywords — 通过
  • node --test <13 个 .github/scripts 辅助测试文件> — 206/206 通过
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 通过
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 通过
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 通过
  • npm run test:ci(全量单元测试,全部 workspace)— 通过
  • npm run bundle — 通过
  • npm run test:integration:no-ak:sandbox:none(CI Test job 的一部分)— 129/129 通过
  • npx vitest run --root ./integration-tests cli channel-plugin.test.ts — 185 通过、18 跳过、1 个如上所述仅环境导致的失败
  • node scripts/lint.js --shellcheck — 通过;--yamllint/--actionlint 的二进制仅在 CI 安装,且与本 PR 无关(未改动 YAML/action/shell)
  • npm run audit:runtime:critical — 未运行(需访问注册表的网络命令,本 agent 不允许执行);无实际影响:本 PR 无依赖变更

本轮未做任何代码修改;分支 head 保持不变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no code change needed

This round's feedback window (comments newer than the last evaluation, 2026-08-07T04:13:09Z) contains sixteen inline comments and one still-red CI check. All sixteen comments are per-finding disposition records — "fixed in 93bd433 / pinned by test X / decline confirmed" notes on threads from the R1 batch and the earlier suggestion batches. None of them requests a new change. This round re-verified each claimed disposition against the code and tests at the current PR head (93bd433da, "fix(feishu): bound label caches, honor observation recency, silence enrichment token failures") and every claim holds, so no code change was made and the branch head is unchanged. The still-red Test (ubuntu-latest, Node 22.x) check was re-diagnosed from evidence: it failed inside a window of the CI job that this PR's diff cannot influence, and every locally runnable part of that job passes on the current head under a CI-equivalent environment.

Inline comments — all sixteen verified against HEAD

  • R1-1 hydration recency (thread at FeishuAdapter.ts:713) — verified resolved. hydrateObservedNames() selects the newest non-ID label per contact by comparing lastObservedAt across top-level users and all group members before writing the runtime caches (FeishuAdapter.ts:712-749). Test prefers the newest persisted label when hydrating overlapping contacts present (adapter.test.ts:1132) and passing.
  • R1-2 unbounded caches (FeishuAdapter.ts:154) — verified resolved. OBSERVED_LABEL_CACHE_LIMIT = 500 (FeishuAdapter.ts:92) with oldest-first capObservedCache() applied on every write path and after hydration for all five caches (user names, chat names, user lookups, chat lookups, write-dedup state). Test bounds runtime label caches to the observed-contact retention cap present (adapter.test.ts:1193) and passing.
  • R1-3 stderr noise from enrichment token failures — verified resolved. getTenantAccessToken()/refreshToken() accept { silent } (FeishuAdapter.ts:647-703); the enrichment lookup is the only silent caller (line 822), core delivery paths keep logging. Test keeps enrichment silent when tenant token acquisition fails present (adapter.test.ts:1090) and passing.
  • R1-4 hook order after rejected persistence (ChannelBase.test.ts) — verified resolved. recordObservedContact() catches observe() rejections internally (ChannelBase.ts:4913-4920), so onObservedContact() always runs after it (ChannelBase.ts:4975-4979). Test still notifies the adapter after a rejected contact persistence present (ChannelBase.test.ts:936) and passing.
  • Partial-success enrichment (FeishuAdapter.ts:877) — verified pinned. writes an enriched observation when only the user lookup resolves (adapter.test.ts:748) asserts exactly one enriched observation carrying only the resolved user label; passing.
  • DM enrichment without chat lookups (FeishuAdapter.ts:774) — verified pinned. enriches direct-chat senders without issuing chat lookups (adapter.test.ts:797) asserts one user lookup and zero /im/v1/chats requests; passing.
  • Non-open_id ID-type mapping (FeishuAdapter.ts:700) — verified pinned. maps non-open_id senders to the matching user_id_type (adapter.test.ts:845) asserts union_id/user_id for on_-prefixed and bare IDs; passing.
  • Concurrent lookup sharing (FeishuAdapter.ts:691) — verified pinned. shares one in-flight lookup across concurrent messages (adapter.test.ts:909) delivers two messages while both lookups are pending and asserts exactly one fetch per endpoint and exactly three observe calls (two raw + one enriched write); passing. The write guard compares resolved labels, not envelope identity (FeishuAdapter.ts:880-893).
  • 401 cache poisoning (FeishuAdapter.ts:726) — verified pinned. The 401 branch clears tokenCache and deletes the poisoned lookup entry (FeishuAdapter.ts:827-833); refreshes the token after a label lookup returns 401 (adapter.test.ts:601) and retries a 401 lookup for the same ID once the token refreshes (adapter.test.ts:679) both pass.
  • Restart hydration (FeishuAdapter.ts:2177) — verified pinned. hydrates label caches from persisted observations after a restart (adapter.test.ts:964) passes, alongside the newest-label test above.
  • Write-dedup convergence (FeishuAdapter.ts:897) — verified pinned by the concurrent-messages test above (exactly three observe calls, not K+2).
  • Name sanitization (FeishuAdapter.ts:729) — verified pinned. sanitizes resolved display names before caching them (adapter.test.ts:719) passes; names sanitizing to unknown are dropped (FeishuAdapter.ts:837-839).
  • Shared lookup helper (FeishuAdapter.ts:818) — verified resolved. observedUserName/observedChatName delegate to a single observedNameLookup owning dedup, 401 retry, silent failure, sanitization, and cache capping (FeishuAdapter.ts:810-857); both token-failure retry tests pass (adapter.test.ts:563 user side, :642 chat side).
  • Chat-side retry eviction (FeishuAdapter.ts:749) — verified pinned. retries a chat lookup when token acquisition fails before the request (adapter.test.ts:642) passes.
  • Envelope label attachment (FeishuAdapter.ts:2224) — verified pinned. attaches cached labels to later envelopes and prompts (adapter.test.ts:1036) passes.
  • Batching decline (FeishuAdapter.ts:779) — remains declined, unchanged this round. The maintainer's decline is recorded on the thread (2026-08-06) and confirmed in this window: batching would require delaying lookups, which conflicts with the feature's no-delay design goal, and per-ID in-flight dedup already bounds concurrent duplicates to one request per ID. This thread is deliberately left open so the recorded reason stays visible; no new reply was posted because the reason is already on the thread.

The fifteen verified-resolved findings are also listed in this round's resolved-comments.txt for the workflow's thread-resolution record.

Still-red check Test (ubuntu-latest, Node 22.x) — environmental, not addressable by this PR's code

Evidence:

  1. The failing run (actions/runs/31146687953/job/92767493937) lasted 13 seconds (started 2026-08-07T04:12:56Z, completed 04:13:09Z).
  2. Per ci.yml, the Test job's steps are: workspace-ownership restore → stale-.qwen cleanup → checkout (depth 1) → verify-checkout-head guard → CI-profile classify → Node setup → npm config → npm ci → only then the audit/lint/schema/unit-test/integration steps. Every step that reads project code or dependencies runs after npm ci, which alone takes far longer than 13 seconds in this monorepo. The job therefore failed inside the pre-dependency-install window: checkout/runner/proxy infrastructure, or the deliberate fail-fast verify-checkout-head guard, which ci.yml documents as protection against a caching egress proxy serving a stale ref.
  3. This PR's diff is nine files (three docs, four channel source/test files, two channel-daemon files). It changes no workflow, action, .github/scripts, dependency manifest, YAML, or shell file — nothing in the pre-npm ci window reads a file this PR touches, so no code change in this branch can affect that failure window.
  4. The repository runs a dedicated CI failure patrol (qwen-ci-flaky-rerun.yml, every 10 minutes) that classifies stale PR check failures and re-runs them; a CI re-run is the arbiter for this check.
  5. Every locally runnable part of the Test job was re-run on the current head with a CI-equivalent environment and passes (Verification below), including the full unit suite and the no-AK integration gate.

Transparency note on local runs: a first npm run test:ci inside this autofix sandbox showed ~111 failures. Each cluster was traced to the sandbox harness environment, not to the code: (a) the harness exports its own QWEN_HOME while the sandbox home is root-owned and tests run as uid 1000 — this breaks home-path-dependent suites (settings, config, storage, logger, memoryDiscovery, token-storage, etc.), mirroring the dedicated-HOME setup CI uses (HOME: runner.temp/qwen-ci-home); (b) the harness exports SANDBOX/QWEN_CODE_* — this breaks the editor/sandbox-config/docs/extensions/ide/footer suites that assume no sandbox markers; (c) one AuthDialog TUI-navigation test runs only outside CI (itWhenTuiInputReliable skips when CI=true) and trips over a provider-list order introduced on main on 2026-07-14 (Grok preset inserted between DeepSeek and MiniMax in all-providers.ts while the test still navigates one step down to MiniMax) — pre-existing on main, untouched by this PR, and never executed in CI. Under a CI-equivalent environment (fresh writable HOME, harness QWEN_HOME/SANDBOX unset, CI=true), every suite passes: the affected files were re-run individually and all pass or skip exactly as they do in CI.

Verification

Commands actually run this round on head 93bd433da (all exit 0 unless noted):

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint (ESLint, repo + integration-tests) — passed
  • npx prettier --check on all nine PR-touched files — passed
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 passed
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 passed
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 passed
  • npm run check:lockfile — passed
  • npm run generate:settings-schema + git status on the schema — up to date, no diff (no settings source is touched by this PR)
  • npm run check:desktop-isolation — passed
  • npm run check:voice-guard-sync — passed
  • npm run check:serve-fast-path-bundle — passed
  • node scripts/lint.js --sensitive-keywords — passed
  • npm run test:ci (all workspaces) — all workspaces pass under the CI-equivalent environment. In the sandbox's inherited environment only, core/cli showed the env-artifact failures described above; targeted re-runs with the harness markers stripped: packages/core editor.test.ts 152/152; packages/cli focused re-run of the seven affected files 425/426, and the one remainder (AuthDialog) passes with CI=true exactly as in CI (7 passed, 18 skipped, exit 0). All other workspaces (channels, web-shell, sdk, vscode-companion, etc.) passed in both environments.
  • npm run test:integration:no-ak:sandbox:none (the Test job's required integration gate, CI-equivalent env) — 129/129 passed
  • git status --porcelain — clean; no working-tree changes were made this round and the branch head is unchanged
中文说明

Autofix 审查轮次 — 无需修改代码

本轮反馈窗口(晚于上次评估 2026-08-07T04:13:09Z 的评论)包含十六条行内评论和一个仍为红色的 CI 检查。十六条评论全部是逐条处置记录——即针对 R1 批次与更早建议批次的线程所留下的"已在 93bd433 修复 / 已由测试 X 钉住 / 确认拒绝"说明,没有任何一条提出新的改动要求。本轮已对照当前 PR head(93bd433da,"fix(feishu): bound label caches, honor observation recency, silence enrichment token failures")逐条重新验证这些处置声明,全部成立,因此未做任何代码修改,分支 head 保持不变。仍为红色的 Test (ubuntu-latest, Node 22.x) 检查已依据证据重新诊断:它失败在 CI job 中本 PR diff 无法影响的窗口内,且该 job 所有可在本地运行的部分在 CI 等价环境下均于当前 head 上通过。

行内评论 — 十六条均已对照 HEAD 验证

  • R1-1 恢复缓存的新旧顺序(FeishuAdapter.ts:713 线程) — 已验证解决。hydrateObservedNames() 在写入运行时缓存前,会跨顶层用户与所有群成员按 lastObservedAt 比较,为每个联系人选取最新的非 ID 标签(FeishuAdapter.ts:712-749)。测试 prefers the newest persisted label when hydrating overlapping contacts 存在(adapter.test.ts:1132)且通过。
  • R1-2 无界缓存(FeishuAdapter.ts:154) — 已验证解决。OBSERVED_LABEL_CACHE_LIMIT = 500(FeishuAdapter.ts:92),capObservedCache() 按最旧优先淘汰,应用于全部五个缓存(用户名、群名、用户查询、群查询、写入去重状态)的每个写入路径及恢复之后。测试 bounds runtime label caches to the observed-contact retention cap 存在(adapter.test.ts:1193)且通过。
  • R1-3 补全 token 失败泄漏到 stderr — 已验证解决。getTenantAccessToken()/refreshToken() 支持 { silent }(FeishuAdapter.ts:647-703);补全查询是静默模式的唯一调用方(第 822 行),核心投递路径继续记录日志。测试 keeps enrichment silent when tenant token acquisition fails 存在(adapter.test.ts:1090)且通过。
  • R1-4 持久化被拒后的 hook 顺序(ChannelBase.test.ts) — 已验证解决。recordObservedContact() 内部捕获 observe() 的拒绝(ChannelBase.ts:4913-4920),因此 onObservedContact() 总是在其后执行(ChannelBase.ts:4975-4979)。测试 still notifies the adapter after a rejected contact persistence 存在(ChannelBase.test.ts:936)且通过。
  • 部分成功的补全(FeishuAdapter.ts:877) — 已验证钉住。writes an enriched observation when only the user lookup resolves(adapter.test.ts:748)断言恰好写入一条只携带已解析用户标签的补全观测;通过。
  • 私聊补全不发起群查询(FeishuAdapter.ts:774) — 已验证钉住。enriches direct-chat senders without issuing chat lookups(adapter.test.ts:797)断言一次用户查询且零次 /im/v1/chats 请求;通过。
  • 非 open_id 的 ID 类型映射(FeishuAdapter.ts:700) — 已验证钉住。maps non-open_id senders to the matching user_id_type(adapter.test.ts:845)断言 on_ 前缀与裸 ID 分别使用 union_id/user_id;通过。
  • 并发查询共享(FeishuAdapter.ts:691) — 已验证钉住。shares one in-flight lookup across concurrent messages(adapter.test.ts:909)在两个查询均未决时投递两条消息,断言每个端点恰好一次 fetch、恰好三次 observe 调用(两次原始 + 一次补全写入);通过。写入守卫比较的是解析后的标签而非 envelope 标识(FeishuAdapter.ts:880-893)。
  • 401 缓存投毒(FeishuAdapter.ts:726) — 已验证钉住。401 分支清除 tokenCache 并删除被投毒的查询条目(FeishuAdapter.ts:827-833);refreshes the token after a label lookup returns 401(adapter.test.ts:601)与 retries a 401 lookup for the same ID once the token refreshes(adapter.test.ts:679)均通过。
  • 重启后恢复缓存(FeishuAdapter.ts:2177) — 已验证钉住。hydrates label caches from persisted observations after a restart(adapter.test.ts:964)通过,与上述最新标签测试共同覆盖。
  • 写入去重收敛(FeishuAdapter.ts:897) — 已由上述并发消息测试钉住(恰好三次 observe 调用,而非 K+2)。
  • 名称净化(FeishuAdapter.ts:729) — 已验证钉住。sanitizes resolved display names before caching them(adapter.test.ts:719)通过;净化为 unknown 的名称被丢弃(FeishuAdapter.ts:837-839)。
  • 共享查询助手(FeishuAdapter.ts:818) — 已验证解决。observedUserName/observedChatName 委托给单一的 observedNameLookup,由其统一负责去重、401 重试、静默失败、净化与缓存上限(FeishuAdapter.ts:810-857);两侧 token 失败重试测试均通过(adapter.test.ts:563 用户侧,:642 群侧)。
  • 群侧重试逐出(FeishuAdapter.ts:749) — 已验证钉住。retries a chat lookup when token acquisition fails before the request(adapter.test.ts:642)通过。
  • envelope 标签附加(FeishuAdapter.ts:2224) — 已验证钉住。attaches cached labels to later envelopes and prompts(adapter.test.ts:1036)通过。
  • 批量查询拒绝(FeishuAdapter.ts:779) — 维持拒绝,本轮无变化。维护者的拒绝已记录在该线程(2026-08-06),本窗口内再次确认:批量处理需要延迟查询以累积 ID,与本功能的零延迟设计目标冲突,且按 ID 的在途去重已把并发重复限制为每个 ID 一次请求。该线程有意保持打开,以便拒绝理由持续可见;由于理由已在线程中,未再发布新回复。

上述十五条已验证解决的发现同时列入本轮 resolved-comments.txt,作为工作流线程解析记录。

仍红检查 Test (ubuntu-latest, Node 22.x) — 环境问题,本 PR 代码无法影响

证据:

  1. 失败的一次运行(actions/runs/31146687953/job/92767493937)总时长仅 13 秒(2026-08-07T04:12:56Z 开始,04:13:09Z 结束)。
  2. ci.yml,Test job 的步骤依次为:恢复 workspace 属主 → 清理遗留 .qwen → checkout(depth 1)→ verify-checkout-head 守卫 → CI profile 分类 → Node 安装 → npm 配置 → npm ci → 之后才是审计/lint/schema/单测/集成步骤。所有读取项目代码或依赖的步骤都在 npm ci 之后,而该 monorepo 的 npm ci 本身就远超 13 秒。因此失败发生在安装依赖之前的窗口:checkout/runner/代理等基础设施问题,或 verify-checkout-head 守卫的有意快速失败——ci.yml 明确记载该守卫用于防止缓存出口代理提供过期 ref。
  3. 本 PR 的 diff 共九个文件(三个文档、四个 channel 源码/测试文件、两个 channel-daemon 文件),未改动任何 workflow、action、.github/scripts、依赖清单、YAML 或 shell 文件——npm ci 之前窗口读取的每个文件都与本 PR 无关,因此本分支的任何代码改动都不可能影响该失败窗口。
  4. 仓库有专门的 CI 失败巡检(qwen-ci-flaky-rerun.yml,每 10 分钟一次),负责分类并重跑过期的 PR 检查失败;该检查以 CI 重跑为准。
  5. Test job 所有可在本地运行的部分都已在当前 head 上以 CI 等价环境重跑并通过(见下方 Verification),包括完整单测套件与 no-AK 集成门禁。

关于本地运行的透明说明:在本 autofix 沙箱中首次运行 npm run test:ci 时出现约 111 个失败。逐簇排查后均归因于沙箱宿主环境而非代码:(a) 宿主导出了自己的 QWEN_HOME,而沙箱 home 属主为 root、测试以 uid 1000 运行——这破坏了依赖 home 路径的套件(settings、config、storage、logger、memoryDiscovery、token-storage 等),与 CI 使用专用 HOME(HOME: runner.temp/qwen-ci-home)的做法对应;(b) 宿主导出了 SANDBOX/QWEN_CODE_*——这破坏了假定无沙箱标记的 editor/sandbox-config/docs/extensions/ide/footer 套件;(c) 一个 AuthDialog TUI 导航测试只在非 CI 环境运行(CI=trueitWhenTuiInputReliable 跳过),它踩中了 2026-07-14 main 上引入的提供者列表顺序(Grok preset 被插入 DeepSeek 与 MiniMax 之间,而该测试仍只向下移动一步到 MiniMax)——该问题在 main 上既有、本 PR 未触碰、且在 CI 中从不执行。在 CI 等价环境(全新可写 HOME、取消宿主 QWEN_HOME/SANDBOXCI=true)下所有套件通过:受影响文件已单独重跑,全部通过与按 CI 中完全相同的方式跳过。

Verification(验证)

本轮在 head 93bd433da 上实际执行的命令(除特别说明外均以退出码 0 结束):

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint(ESLint,仓库 + integration-tests)— 通过
  • 对全部九个 PR 改动文件执行 npx prettier --check — 通过
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 通过
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 通过
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 通过
  • npm run check:lockfile — 通过
  • npm run generate:settings-schema + 对该文件的 git status — 已是最新、无 diff(本 PR 未触碰任何 settings 源)
  • npm run check:desktop-isolation — 通过
  • npm run check:voice-guard-sync — 通过
  • npm run check:serve-fast-path-bundle — 通过
  • node scripts/lint.js --sensitive-keywords — 通过
  • npm run test:ci(全部 workspace)— 在 CI 等价环境下全部 workspace 通过。仅在沙箱继承环境中,core/cli 出现上述环境伪影失败;剥离宿主标记后的定点重跑:packages/core editor.test.ts 152/152;packages/cli 七个受影响文件的定点重跑 425/426,其余一条(AuthDialog)在 CI=true 下与 CI 完全一致地通过(7 通过、18 跳过,退出码 0)。其余所有 workspace(channels、web-shell、sdk、vscode-companion 等)在两种环境下均通过。
  • npm run test:integration:no-ak:sandbox:none(Test job 的必跑集成门禁,CI 等价环境)— 129/129 通过
  • git status --porcelain — 干净;本轮未做任何工作树改动,分支 head 保持不变

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round: no code changes needed

This round's feedback contains no inline or review-comment findings — only two
failed checks and the bot comment about the aborted review. After
evidence-based triage, neither failure is attributable to this PR's code, and
neither has a code-level fix. The working tree is left unchanged
(git status clean, HEAD still at the reviewed merge commit).

Triage of feedback points

1. Test (ubuntu-latest, Node 22.x) — FAILURE (run 31149158319): transient runner/checkout-stage failure, no code fix possible.

Evidence:

  • The job ran for only 13 seconds (05:00:12Z → 05:00:25Z). In the full
    CI profile every content-dependent step (ESLint, settings-schema freshness,
    notices, unit and integration tests) is gated behind npm ci — which also
    runs the prepare build — so none of them can have executed. The failure
    happened in the pre-install window, where the only failable steps are
    actions/checkout and the Verify checkout includes expected head commit
    guard (the workspace-restore/cleanup and profile-classification steps have
    warning-only fallbacks and cannot fail the job).
  • Runner routing is asymmetric in exactly the expected way: this was the only
    job in the run that does a git checkout on the self-hosted ECS pool.
    classify_pr (same ECS pool, no checkout) passed; Desktop Shell
    (hosted ubuntu-22.04, with checkout) passed.
  • The run was triggered by a force-push only 20 seconds before it started
    (merge commit authored 04:59:52Z; the force-push reminder job fired in the
    same window). That is precisely the race where the caching egress proxy can
    serve a stale refs/pull/8569/head — the condition the
    verify-checkout-head guard exists to fail loudly on rather than silently
    testing the wrong tree (see the comments around both steps in ci.yml).
  • A fast npm ci lockfile-mismatch failure was ruled out: this branch's diff
    against main touches no package.json or package-lock.json at all.
  • The tree itself verifies clean locally (see Verification below), so a
    re-run of the Test job should pass.

Action: re-run the failed Test job. No commit can influence the proxy/cache
behavior that tripped it.

2. review-pr — FAILURE (run 31149158191): transient review-workflow API error, nothing to address in code.

Per the bot's own comment on this PR, the Qwen review aborted with an API
error before posting comments; transient errors are retried automatically, or
a maintainer can retry with @qwen-code /review. This failure is internal to
the review workflow and unrelated to the PR's changes.

3. Issue-level comment from qwen-code-ci-bot: informational, covers the
same aborted review as point 2. Handled by the same retry path.

No base-conflict resolution was needed (--conflict false; the branch
already contains the current main tip as its merge base).

Verification

Commands actually run this round, on the unchanged tree (HEAD = the reviewed
merge commit):

  • npm run build — passed
  • npm run typecheck — passed
  • node scripts/lint.js --eslint (repo-wide, --max-warnings 0) — passed
  • npx prettier --check on all 9 files touched by this PR — all formatted
  • npx vitest run in packages/channels/base — 19 files / 973 tests passed
  • npx vitest run in packages/channels/feishu — 3 files / 130 tests passed
  • npx vitest run src/commands/channel/daemon-worker.test.ts in
    packages/cli — 77 tests passed

Note: node scripts/lint.js --prettier is a write step (prettier --write .)
and reformatted 38 pre-existing unformatted files outside this PR's scope
before that was noticed; those files were restored to the committed state and
are not part of this round.

中文说明

Autofix 审查轮次:无需代码改动

本轮反馈中没有任何行内评论或审查意见——只有两个失败的 check 和一条关于审查中止的机器人评论。经过基于证据的分类处理,这两个失败都不是本 PR 代码导致的,也都没有代码层面的修复手段。工作区保持不变(git status 干净,HEAD 仍停留在被审查的合并提交上)。

反馈点分类处理

1. Test (ubuntu-latest, Node 22.x) — 失败(运行 31149158319):运行器/检出阶段的瞬时故障,无法通过代码修复。

证据:

  • 该 job 只运行了 13 秒(05:00:12Z → 05:00:25Z)。在 full CI profile 中,所有依赖内容的步骤(ESLint、settings-schema 新鲜度检查、NOTICES、单元测试和集成测试)都在 npm ci 之后才会执行——而 npm ci 还会运行 prepare 构建——因此这些步骤都不可能执行过。失败发生在安装依赖之前的窗口内,该窗口中唯一可能失败的步骤是 actions/checkoutVerify checkout includes expected head commit 守卫(工作区权限恢复/清理和 profile 分类步骤只有告警回退逻辑,不会使 job 失败)。
  • 运行器路由的差异恰好符合预期:这是本次运行中唯一一个在自托管 ECS 机器池上执行 git checkout 的 job。classify_pr(同一 ECS 机器池,无 checkout)通过;Desktop Shell(托管 ubuntu-22.04,有 checkout)通过。
  • 本次运行由一次 force-push 触发,且距运行开始仅 20 秒(合并提交时间 04:59:52Z;force-push 提醒 job 在同一窗口内触发)。这正是缓存出口代理可能返回过期 refs/pull/8569/head 的竞争窗口——也就是 verify-checkout-head 守卫存在的意义:宁可大声失败,也不静默地测试错误的代码树(参见 ci.yml 中这两处步骤附近的注释)。
  • 已排除 npm ci 因 lockfile 不匹配而快速失败的可能:本分支相对 main 的 diff 完全没有改动任何 package.jsonpackage-lock.json
  • 代码树本身在本地验证是干净的(见下方 Verification 一节),因此重新运行 Test job 应当通过。

处理方式:重新运行失败的 Test job。任何提交都无法影响导致其失败的代理/缓存行为。

2. review-pr — 失败(运行 31149158191):审查工作流的瞬时 API 错误,代码层面无需处理。

按照机器人在本 PR 上的评论所述,Qwen 审查在发布评论之前因 API 错误中止;瞬时错误会自动重试,维护者也可以用 @qwen-code /review 手动重试。该失败是审查工作流内部的问题,与本 PR 的改动无关。

3. qwen-code-ci-bot 的 issue 级评论:信息性内容,与第 2 点是同一次审查中止,通过同样的重试路径处理。

无需解决与 base 分支的冲突(--conflict false;分支已包含当前 main 顶端作为其合并基点)。

Verification(验证)

本轮在未改动的代码树(HEAD = 被审查的合并提交)上实际运行的命令:

  • npm run build — 通过
  • npm run typecheck — 通过
  • node scripts/lint.js --eslint(全仓库,--max-warnings 0)— 通过
  • npx prettier --check 检查本 PR 改动的全部 9 个文件 — 全部符合格式
  • packages/channels/basenpx vitest run — 19 个文件 / 973 个测试通过
  • packages/channels/feishunpx vitest run — 3 个文件 / 130 个测试通过
  • packages/clinpx vitest run src/commands/channel/daemon-worker.test.ts — 77 个测试通过

说明:node scripts/lint.js --prettier 是写入步骤(prettier --write .),在发现这一点之前,它已对本 PR 范围之外的 38 个原本未格式化的文件做了重新格式化;这些文件已被恢复到已提交的状态,不属于本轮改动。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), review-pr] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x), review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no code change needed

This round's feedback contains no reviews, inline comments, or issue-level comments — only one item: the still-red Test (ubuntu-latest, Node 22.x) check (run 31157845183, job ran 07:29:37Z → 07:48:55Z). After evidence-based triage, the failure is not attributable to this PR's code and has no code-level fix, so the working tree is left unchanged (git status clean; HEAD unchanged at the reviewed merge commit 85f4d07886). No base-conflict resolution was needed (--conflict false; the branch already contains the current main tip).

Triage of the failed check

Test (ubuntu-latest, Node 22.x) — not reproducible; every locally runnable step of that CI job passes on the current head under a CI-equivalent environment.

Evidence:

  1. Full step-by-step reproduction. Every step of the Test job that can run in this sandbox was re-run on the exact commit CI tested (85f4d07886), with a CI-equivalent environment (fresh writable HOME, the harness's QWEN_HOME/SANDBOX/QWEN_* variables unset, API keys cleared, mirroring the job's HOME: runner.temp/qwen-ci-home setup). All passed — full list in Verification below.
  2. The full unit suite passes. npm run test:ci was run across all 19 workspaces in that clean environment. Every suite passes except four tests that provably cannot fail in CI:
    • Two src/serve/server.test.ts cases bind the IPv6 loopback [::1]. This diagnosis container has no IPv6 stack at all (/proc/net/if_inet6 is absent), while GitHub's ubuntu runners have ::1. These fail only in this container.
    • Two src/ui/auth/AuthDialog.test.tsx TUI-navigation cases are wrapped in itWhenTuiInputReliable, which is it.skip whenever process.env.CI === 'true' (AuthDialog.test.tsx:238-239) — they never execute in CI. They only failed here because this sandbox run does not set CI=true.
  3. The PR cannot influence the failure window. This PR's diff is nine files (three docs, four channel source/test files, two channel-daemon files). It changes no workflow, action, .github/scripts, dependency manifest (package.json/package-lock.json untouched — lockfile check passes), YAML, or shell file. The remaining Test-job steps that cannot run here (runtime-dependency audit, actionlint/yamllint/shellcheck binaries, Chrome-extension artifact scan) only read inputs this PR does not touch.
  4. History of this check on this PR. Earlier rounds recorded this same check failing from infrastructure causes — one instance was a 13-second job that died in the pre-npm ci checkout window on the self-hosted pool (a caching-egress-proxy/stale-ref race that the verify-checkout-head guard exists to catch), and each round's local reproduction of every runnable step was green. The repository also runs a CI flake patrol (qwen-ci-flaky-rerun.yml) that classifies and re-runs such failures; a CI re-run remains the arbiter for this check.

Disposition

  • Failed check Test (ubuntu-latest, Node 22.x): declined as not addressable by this PR's code — reason recorded here so it is visible in the thread: not reproducible across two rounds of full step-by-step local reproduction; the only local failures are container-specific artifacts (no IPv6, no CI=true) that do not exist on the CI runners.

Verification

Commands actually run this round on head 85f4d07886 (all exit 0 unless noted):

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint (ESLint, repo + integration-tests) — passed
  • npx prettier --check on all 9 files touched by this PR — passed
  • npm run check:lockfile — passed
  • npm run check:desktop-isolation — passed
  • npm run check:voice-guard-sync — passed
  • npm run check-i18n — passed
  • npm run generate:settings-schema + git status on the schema — up to date, no diff
  • npm run generate:notices --workspace=qwen-code-vscode-ide-companion + git status — up to date, no diff
  • npm run check:serve-fast-path-bundle — passed
  • node --test on the 13 .github/scripts helper test files — 206/206 passed
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 passed
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 passed
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 passed
  • npm run test:ci equivalent (all workspaces, clean CI-equivalent env) — all passed except the 4 container-specific/CI-skipped tests itemized above
  • npm run test:integration:no-ak:sandbox:none (the required no-AK gate inside the Test job) — 129/129 passed
  • npm run audit:runtime:critical — not run (networked registry command, disallowed for this agent); moot: this PR changes no dependencies

No code change was made this round; the branch head is unchanged.

中文说明

Autofix 审查轮次 — 无需修改代码

本轮反馈中没有任何审查意见、行内评论或 issue 级评论——只有一项内容:仍然为红的 Test (ubuntu-latest, Node 22.x) 检查(运行 31157845183,job 运行时间 07:29:37Z → 07:48:55Z)。经过基于证据的分类处理后,该失败不能归因于本 PR 的代码,也没有代码层面的修复手段,因此工作区保持不变(git status 干净;HEAD 仍停留在被审查的合并提交 85f4d07886)。无需解决与 base 分支的冲突(--conflict false;分支已包含当前 main 顶端)。

失败检查的分类处理

Test (ubuntu-latest, Node 22.x) — 无法复现;在 CI 等价环境下,该 CI job 中所有可在本地运行的步骤都在当前 head 上通过。

证据:

  1. 逐步完整复现。 该 Test job 中所有能在本沙箱运行的步骤,都已在 CI 实际测试的同一个提交(85f4d07886)上、以 CI 等价环境(全新可写的 HOME,解除运行框架自带的 QWEN_HOME/SANDBOX/QWEN_* 变量,清空 API key,与该 job 的 HOME: runner.temp/qwen-ci-home 配置一致)重新执行。全部通过——完整清单见下方 Verification。
  2. 完整单元测试套件通过。 在上述干净环境中对全部 19 个 workspace 运行了 npm run test:ci。除四个在 CI 中不可能失败的测试外,所有套件均通过:
    • src/serve/server.test.ts 中两个用例绑定 IPv6 回环地址 [::1]。本诊断容器完全没有 IPv6 协议栈(/proc/net/if_inet6 不存在),而 GitHub 的 ubuntu 运行器具有 ::1。这两个用例只在本容器中失败。
    • src/ui/auth/AuthDialog.test.tsx 中两个 TUI 导航用例由 itWhenTuiInputReliable 包裹,当 process.env.CI === 'true' 时它等价于 it.skip(AuthDialog.test.tsx:238-239)——在 CI 中从不执行。它们在此失败仅因为本沙箱运行未设置 CI=true
  3. 本 PR 无法影响该失败窗口。 本 PR 的 diff 共九个文件(三个文档、四个 channel 源码/测试文件、两个 channel-daemon 文件)。未改动任何 workflow、action、.github/scripts、依赖清单(package.json/package-lock.json 未被触碰——lockfile 检查通过)、YAML 或 shell 文件。其余无法在此运行的 Test job 步骤(运行时依赖审计、actionlint/yamllint/shellcheck 二进制、Chrome 扩展产物扫描)只读取本 PR 未触碰的输入。
  4. 该检查在本 PR 上的历史。 此前的轮次已记录过该检查因基础设施原因失败——其中一次是一个仅运行 13 秒的 job,在 npm ci 之前的检出窗口内死于自托管机器池(缓存出口代理/过期 ref 的竞争,正是 verify-checkout-head 守卫要捕获的场景),且每一轮的本地逐步复现均为绿色。仓库还运行着 CI flake 巡检(qwen-ci-flaky-rerun.yml),会分类并重跑此类失败;CI 重跑仍是该检查的最终裁决。

处理结论

  • 失败检查 Test (ubuntu-latest, Node 22.x)拒绝处理,理由是本 PR 代码无法解决——理由记录于此以便在话题中可见:经两轮完整的本地逐步复现均无法复现;仅有的本地失败是容器特有的环境因素(无 IPv6、无 CI=true),在 CI 运行器上并不存在。

Verification(验证)

本轮在 head 85f4d07886 上实际执行的命令(除特别说明外均以退出码 0 结束):

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint(ESLint,仓库 + integration-tests)— 通过
  • 对本 PR 改动的全部 9 个文件执行 npx prettier --check — 通过
  • npm run check:lockfile — 通过
  • npm run check:desktop-isolation — 通过
  • npm run check:voice-guard-sync — 通过
  • npm run check-i18n — 通过
  • npm run generate:settings-schema + 对 schema 文件执行 git status — 已是最新,无 diff
  • npm run generate:notices --workspace=qwen-code-vscode-ide-companion + git status — 已是最新,无 diff
  • npm run check:serve-fast-path-bundle — 通过
  • 对 13 个 .github/scripts 辅助测试文件执行 node --test — 206/206 通过
  • cd packages/channels/feishu && npx vitest run src/adapter.test.ts — 92/92 通过
  • cd packages/channels/base && npx vitest run src/ChannelBase.test.ts — 562/562 通过
  • cd packages/cli && npx vitest run src/commands/channel/daemon-worker.test.ts — 77/77 通过
  • npm run test:ci 等价运行(全部 workspace,干净的 CI 等价环境)— 除上文逐项列出的 4 个容器特有/CI 跳过测试外全部通过
  • npm run test:integration:no-ak:sandbox:none(Test job 内的必需 no-AK 门禁)— 129/129 通过
  • npm run audit:runtime:critical — 未运行(联网 registry 命令,本 agent 不允许执行);无实际影响:本 PR 未改动任何依赖

本轮未做任何代码改动;分支 head 保持不变。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): npm testexit 1.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):npm testexit 1

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +653 to +654
if (this.tokenRefreshPromise) return this.tokenRefreshPromise;
this.tokenRefreshPromise = this.refreshToken();
this.tokenRefreshPromise = this.refreshToken(options);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-1: The silent flag is latched into the shared tokenRefreshPromise by whichever caller starts the refresh. An enrichment-initiated (silent) refresh therefore suppresses the token-error log lines for concurrent core delivery callers that join it — the comment this diff adds ("core delivery paths keep logging token errors") holds only when a core path initiates the refresh. — Failure scenario: with the tenant-token cache expired, the inbound enrichment hook calls getTenantAccessToken({ silent: true }) before the reply path and starts refreshToken({ silent: true }); while it is in flight (up to the 15 s timeout), sendMessageInternal / media download / card paths join the shared promise; if the token endpoint hangs or returns 5xx, both stderr write sites are skipped and delivery fails with only generic downstream errors ("Cannot send: no access token."), with the root-cause line suppressed for as long as steady inbound traffic keeps re-opening silent refreshes. Probe-verified at this commit: a joined core caller observes zero stderr writes after the shared failure, while a core-initiated refresh logs. Suggested fix: track non-silent joiners (e.g. set a tokenRefreshHasCoreWaiters flag in the early-return branch when !options?.silent, reset it in the finally alongside tokenRefreshPromise = undefined) and log in refreshToken's failure branches when a non-silent waiter exists.

中文说明

[Suggestion] R4-1:silent 标志由发起刷新的调用方锁定进共享的 tokenRefreshPromise。因此由补全发起的(静默)刷新会抑制并发加入该 promise 的核心投递调用方的 token 错误日志——本 diff 新增的注释("core delivery paths keep logging token errors")只有在核心路径发起刷新时才成立。—— 失败场景:tenant token 缓存过期时,入站补全 hook 在回复路径之前调用 getTenantAccessToken({ silent: true }) 并启动 refreshToken({ silent: true });在其在途期间(最长 15 秒超时),sendMessageInternal / 媒体下载 / 卡片路径加入共享 promise;若 token 接口挂起或返回 5xx,两处 stderr 写入均被跳过,投递仅以泛化的下游错误("Cannot send: no access token.")失败,且在持续的入站流量不断重新发起静默刷新期间,根因日志会一直被抑制。已在本提交上用探针验证:加入共享刷新的核心调用方在失败后观测到 0 条 stderr 输出,而核心路径发起的刷新会正常记录日志。建议修复:记录非静默的加入方(例如在提前返回分支中当 !options?.silent 时设置 tokenRefreshHasCoreWaiters 标志,并在 finally 中与 tokenRefreshPromise = undefined 一起重置),在 refreshToken 的失败分支中当存在非静默等待者时输出日志。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +817 to +818
const existing = options.lookups.get(options.id);
if (existing) return existing;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-2: observedNameLookup dedups only on the lookups promise map, never the resolved names map. capObservedCache FIFO-evicts both maps at 500, and lookup entries are inserted before their name entries resolve, so after cache churn an already-resolved ID gets a fresh HTTP lookup even though its label is still cached. — Failure scenario: a long-running daemon observes >500 unique sender/chat IDs; the oldest (successfully resolved) lookup entry is evicted; when that ID sends another message, a fresh basic_batch request is issued even though observedUserNames still holds the label and the envelope already uses it — redundant rate-limited API traffic, contrary to the plan's "at most one query per ID per instance". Probe-verified: after simulated churn the PR code issues 1 fetch for the warm-cached ID; consulting the names map first removes it.

Suggested change
const existing = options.lookups.get(options.id);
if (existing) return existing;
const cached = options.names.get(options.id);
if (cached) return Promise.resolve(cached);
const existing = options.lookups.get(options.id);
if (existing) return existing;
中文说明

[Suggestion] R4-2:observedNameLookup 只对 lookups promise map 做去重,从不查询已解析的 names map。capObservedCache 对两个 map 都按 500 条 FIFO 淘汰,且 lookup 条目先于其 name 条目写入,因此缓存周转之后,即使标签仍在缓存中,已解析的 ID 也会触发一次新的 HTTP 查询。—— 失败场景:长期运行的 daemon 观测到超过 500 个不同的发送者/会话 ID;最早的(已成功解析的)lookup 条目被淘汰;当该 ID 再次发消息时,即使 observedUserNames 中仍存有标签且 envelope 已在使用它,也会发起一次新的 basic_batch 请求——多余的受限 API 流量,与计划的"每个实例每个 ID 至多查询一次"相悖。探针验证:模拟缓存周转后,PR 代码对该仍在缓存中的 ID 发起了 1 次请求;先查询 names map 可将其消除。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +4938 to +4942
users: graph.users.filter((user) => user.channelName === this.name),
groups: graph.groups.filter((group) => group.channelName === this.name),
};
} catch {
return undefined;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-4: The channel-isolation filter and the swallowing catch in persistedObservedContacts() have no test that can detect their removal. Mutation-tested at this commit: deleting both .filter(...) conditions, or deleting the try/catch, leaves ChannelBase 562/562 and feishu 92/92 green. — Failure scenario: the hydrate fixture includes a foreign-channel group with member ou_foreign, but no message is ever sent from it — so removing the filters ships green, and labels persisted by another channel (the store is workspace-shared) hydrate into this channel's caches and mislabel prompts. Removing the catch also ships green: list() throws on a corrupted registry, the throw propagates through hydrateObservedNames() into FeishuChannel.onMessage, whose outer catch drops the first inbound message after restart instead of silently skipping hydration. Suggested fix: in the hydrate test, also deliver a message from the foreign-channel contact and assert labels stay raw IDs; add a case where list throws and assert the message is still processed.

中文说明

[Suggestion] R4-4:persistedObservedContacts() 中的 channel 隔离过滤与吞错 catch 没有任何测试能检测到它们被删除。已在本提交上做变异测试:删除两个 .filter(...) 条件,或删除 try/catch,ChannelBase 562/562 与 feishu 92/92 均保持绿色。—— 失败场景:hydration 测试 fixture 中包含一个外 channel 的群(成员 ou_foreign),但从未有该成员的消息——因此删除过滤后测试仍绿,而另一个 channel 持久化的标签(store 按 workspace 共享)会 hydrate 进本 channel 的缓存并导致提示词标注错误。删除 catch 同样能绿着上线:list() 在注册表损坏时会抛错,该异常会经 hydrateObservedNames() 传播到 FeishuChannel.onMessage,其外层 catch 会丢弃重启后的首条入站消息,而不是静默跳过 hydration。建议修复:在 hydration 测试中让外 channel 联系人也发一条消息并断言其标签保持原始 ID;再新增一个 list 抛错的用例,断言消息仍被正常处理。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +683 to +687
if (!silent) {
process.stderr.write(
`[Feishu:${this.name}] getTenantAccessToken failed: HTTP ${resp.status}\n`,
);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-5: This silent-mode HTTP-error branch is exercised by the retry tests (retries a label lookup when token acquisition fails before the request, adapter.test.ts:563, and its chat twin) but never asserted silent — the tests install stderrSpy and only restore it in finally. The only silence assertion (keeps enrichment silent when tenant token acquisition fails, adapter.test.ts:1090) covers the catch branch, not this one. — Failure scenario: deleting the if (!silent) wrapper here leaves the suite 92/92 green, and a silent background lookup hitting a 503 then writes [Feishu:...] getTenantAccessToken failed: HTTP 503 to stderr (probe-verified: the mutant emits the line, PR code emits nothing) — violating the design's "lookup failures produce no log output" with the suite green. Suggested fix: add expect(stderrSpy).not.toHaveBeenCalled() before the finally in both retry tests — the spies are already installed and unused.

中文说明

[Suggestion] R4-5:这个静默模式的 HTTP 错误分支被重试测试(retries a label lookup when token acquisition fails before the request,adapter.test.ts:563,及其 chat 对应测试)执行到,但从未断言其静默——测试安装了 stderrSpy 却只在 finally 中恢复。唯一的静默断言(keeps enrichment silent when tenant token acquisition fails,adapter.test.ts:1090)覆盖的是 catch 分支,而非此分支。—— 失败场景:删除此处的 if (!silent) 包装后测试套件仍为 92/92 绿,而一次命中 503 的静默后台查询会向 stderr 写入 [Feishu:...] getTenantAccessToken failed: HTTP 503(探针验证:变异体输出了该行,PR 代码无任何输出)——在套件全绿的情况下违反设计文档的"查询失败不产生日志输出"。建议修复:在两个重试测试的 finally 之前加上 expect(stderrSpy).not.toHaveBeenCalled()——spy 已经安装且未被使用。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +839 to +840
const label = sanitizeSenderName(name);
if (label === 'unknown') return undefined;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-6: The 'unknown'-label guard has no test — no enrichment test supplies a name that sanitizes to 'unknown'; deleting the guard leaves the suite 92/92 green (mutation-tested at this commit). — Failure scenario: with the guard removed, a Feishu user whose display name consists solely of strippable characters (e.g. only zero-width characters) sanitizes to 'unknown', gets cached in observedUserNames and reused on every later envelope — prompt attribution renders [unknown] instead of the ID, contradicting the design's "keep ID labels when lookup is unavailable". Probe-verified: the mutant caches 'unknown'; PR code resolves undefined. Suggested fix: add a lookup test where the API returns a name of only zero-width characters and assert the lookup resolves undefined and nothing is cached.

中文说明

[Suggestion] R4-6:'unknown' 标签守卫没有测试——没有任何补全测试提供会净化为 'unknown' 的名称;删除该守卫后套件仍为 92/92 绿(已在本提交上做变异测试)。—— 失败场景:删除守卫后,若某个飞书用户的显示名仅由可剥离字符(例如全是零宽字符)组成,净化结果为 'unknown',会被缓存进 observedUserNames 并在之后每条 envelope 中复用——提示词署名会渲染为 [unknown] 而非 ID,与设计文档的"查询不可用时保留 ID 标签"相悖。探针验证:变异体缓存了 'unknown';PR 代码解析为 undefined。建议修复:新增一个查询测试,令 API 返回仅含零宽字符的名称,断言查询解析为 undefined 且不写入缓存。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +2175 to +2177
this.hydrateObservedNames();
const senderName = this.observedUserNames.get(senderId) || senderId;
const chatName = isGroup ? this.observedChatNames.get(chatId) : undefined;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R4-8: After in-lifetime cache eviction, the next message from a previously enriched contact rebuilds the envelope with the raw ID fallback, and the initial recordObservedContact write then overwrites the registry's persisted name label with the ID. If the guaranteed re-lookup fails, the persisted name is destroyed and hydration cannot restore it (label === id entries are skipped) — hydration protects restarts only, not in-lifetime eviction. — Failure scenario: a long-running daemon resolves names for >500 distinct sender IDs; FIFO eviction drops an enriched contact's cache entries (lookup entries evict first, so a re-lookup fires); the contact's next message writes (user=ou_X, label='ou_X') over (user=ou_X, label='Alice'); if the re-lookup then fails — transient outage, 429 storm, or the optional scope revoked mid-lifetime — the failure is retained until restart, and the next restart's hydration skips the entry, so the persisted name is gone. Probe-verified at this commit (including a flip check: re-hydrating on actual eviction restores the label). Pre-PR this downgrade could not occur because Feishu labels were always IDs. Suggested fix: consult the persisted registry before falling back to the raw ID on a cold-cache miss (or reset hydratedObservedNames when capObservedCache actually evicts, so the next message re-hydrates from the still-intact registry).

中文说明

[Suggestion] R4-8:运行期缓存淘汰之后,此前已补全的联系人的下一条消息会以原始 ID 回退重建 envelope,而首次 recordObservedContact 写入会用 ID 覆盖注册表中已持久化的名称标签。若随后必然触发的重新查询失败,已持久化的名称即被销毁且 hydration 无法恢复(label === id 的条目会被跳过)——hydration 只保护重启场景,不覆盖运行期淘汰。—— 失败场景:长期运行的 daemon 为超过 500 个不同发送者解析了名称;FIFO 淘汰掉了某个已补全联系人的缓存条目(lookup 条目先被淘汰,因此必然触发重新查询);该联系人的下一条消息把 (user=ou_X, label='Alice') 覆盖为 (user=ou_X, label='ou_X');若重新查询随后失败——瞬时故障、429 风暴、或可选权限在运行中被回收——该失败会被保留到重启,而下次重启的 hydration 会跳过该条目,已持久化的名称就此丢失。已在本提交上用探针验证(含翻转验证:在实际淘汰时重新 hydration 可恢复标签)。PR 之前不会发生这种降级,因为彼时飞书标签恒为 ID。建议修复:冷缓存未命中时先查询持久化注册表再回退到原始 ID(或在 capObservedCache 实际发生淘汰时重置 hydratedObservedNames,让下一条消息从仍然完好的注册表重新 hydration)。

— qwen3.8-max via Qwen Code /review (v0.21.7)

)

* Track core (non-silent) waiters on the shared tenant-token refresh so a
  silent-initiated refresh still logs token errors for joined delivery
  callers.
* Short-circuit label lookups on the resolved names cache so evicted
  lookup entries do not trigger redundant API requests.
* Re-hydrate label caches from the persisted registry after an
  in-lifetime cache eviction so the next initial write cannot clobber a
  persisted label with the raw ID.
* Add mutation-proof regression tests for the channel-isolation filter,
  the list-failure swallow, the silent HTTP-error branch, and the
  'unknown' label guard.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Autofix review-round summary — PR #8569 (round 4 feedback)

All six inline findings from the automated reviewer were verified against the
exact reviewed commit and addressed in code (three production fixes, plus
regression tests that were mutation-proven to detect each regression). No
finding was declined or escalated; no conflict resolution was needed
(--conflict false, base branch was not merged).

Findings and dispositions

  • [Suggestion] R4-1 (token refresh latches the silent flag) — Addressed.
    Verified: refreshToken(options) captured the initiator's silence, so an
    enrichment-initiated (silent) refresh suppressed the token-error log lines
    for concurrent core delivery callers that joined the shared
    tokenRefreshPromise. Fix: added a tokenRefreshHasCoreWaiters flag — set
    by any non-silent caller in getTenantAccessToken (initiator or joiner),
    reset in the finally that clears the shared promise; both failure branches
    in refreshToken now log when the flag is set. New regression test
    (logs token errors for core callers that join a silent-initiated refresh)
    asserts a core joiner of a silent 503 refresh sees exactly one stderr line.
  • [Suggestion] R4-2 (lookup dedup never consults the resolved names map)
    Addressed with the suggested change: observedNameLookup now returns the
    cached label from options.names before checking options.lookups, so an
    ID whose lookup entry was FIFO-evicted but whose label is still cached no
    longer issues a redundant rate-limited request. New regression test
    (reuses a cached label without a new lookup after lookup entries evict).
    Note: this also changes the restart-hydration test's fetch expectations —
    hydrated labels now short-circuit enrichment lookups (0 fetches until truly
    unknown IDs appear), which matches the plan's "at most one query per ID".
  • [Suggestion] R4-4 (channel-isolation filter / swallowing catch untested)
    Addressed with tests (no production change). The hydrate test now also
    delivers a message from the foreign-channel contact (ou_foreign in
    oc_foreign) and asserts labels stay raw IDs in both the persisted
    observation and the prompt; removing either .filter(...) condition makes
    it fail. A new test
    (still processes messages when persisted observations cannot be listed)
    makes list() throw and asserts the message is still processed with zero
    stderr output; removing the try/catch makes it fail.
  • [Suggestion] R4-5 (silent HTTP-error branch never asserted silent)
    Addressed: both retry tests (retries a label lookup... and its chat twin)
    already installed stderrSpy; each now asserts
    expect(stderrSpy).not.toHaveBeenCalled(). Removing the silence condition
    on the HTTP-error branch now fails the suite.
  • [Suggestion] R4-6 ('unknown'-label guard untested) — Addressed: new
    test (keeps the ID label when a resolved name sanitizes to unknown)
    returns a name consisting only of zero-width characters, asserts the lookup
    resolves undefined and nothing is cached. Removing the guard fails it.
  • [Suggestion] R4-8 (in-lifetime eviction can destroy persisted labels)
    Addressed with the reviewer's re-hydration variant. capObservedCache now
    reports whether it evicted; when a resolved label is evicted from a names
    cache, hydratedObservedNames is reset so the next inbound message
    re-hydrates from the still-intact persisted registry before building the
    envelope — the initial write can no longer clobber a persisted name with
    the raw ID. (The registry holds ≤500 observations total, so re-hydration
    itself never evicts and cannot loop.) New regression test
    (re-hydrates persisted labels after in-lifetime cache eviction) churns
    the cache past the cap, fails the guaranteed re-lookup (429), and asserts
    the contact's next envelope/observation still carries Alice.

All seven mutations (each fix removed, one at a time) were verified to fail
their paired test, and the restored tree passes the full suite.

Changed files

  • packages/channels/feishu/src/FeishuAdapter.ts — the three production
    fixes above (+23/−11).
  • packages/channels/feishu/src/adapter.test.ts — five new tests, one
    extended test, two added silence assertions (+240).

Conflict notes

None — --conflict false; no merge performed.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx vitest run src/adapter.test.ts (packages/channels/feishu) — 97 passed (92 prior + 5 new)
  • npx vitest run (packages/channels/feishu, full package) — 135 passed
  • npx vitest run (packages/channels/base, full package) — 973 passed
  • npx vitest run src/commands/channel/daemon-worker.test.ts (packages/cli) — 77 passed
  • Mutation check (7 mutants: each fix removed individually) — every mutant failed its paired test; restored tree green
  • npx prettier --check on both changed files — clean

No settings source changed (no schema regeneration needed). The touched
behavior is exercised by the package unit tests, so no bundled/integration
run was required.

中文说明

Autofix 审查轮次总结 — PR #8569(第 4 轮反馈)

自动审查者的六条行内发现均已在被审查的确切提交上逐条核实,并已在代码中处理(三处生产代码修复,以及经变异测试证明能检测对应回归的回归测试)。没有拒绝或升级任何发现;无需解决冲突(--conflict false,未合并 base 分支)。

发现与处理

  • [Suggestion] R4-1(token 刷新锁存了 silent 标志) — 已处理。已核实:refreshToken(options) 捕获的是发起方的静默状态,因此由补全发起的(静默)刷新会抑制并发加入共享 tokenRefreshPromise 的核心投递调用方的 token 错误日志。修复:新增 tokenRefreshHasCoreWaiters 标志——由 getTenantAccessToken 中任一非静默调用方(发起方或加入方)设置,并在清除共享 promise 的 finally 中重置;refreshToken 的两个失败分支在该标志被设置时输出日志。新增回归测试(logs token errors for core callers that join a silent-initiated refresh),断言核心调用方加入一次静默发起的 503 刷新时恰好输出一条 stderr 日志。
  • [Suggestion] R4-2(查询去重从不查询已解析的 names map) — 已按建议修复:observedNameLookup 现在先检查 options.lookups 之前先从 options.names 返回缓存标签,因此 lookup 条目被 FIFO 淘汰但标签仍在缓存中的 ID 不会再发起多余的受限请求。新增回归测试(reuses a cached label without a new lookup after lookup entries evict)。注意:这也改变了重启 hydration 测试的 fetch 预期——hydrated 标签现在会短路补全查询(在出现真正未知 ID 之前 fetch 为 0 次),这与计划中"每个 ID 至多查询一次"一致。
  • [Suggestion] R4-4(channel 隔离过滤 / 吞错 catch 无测试) — 已通过测试处理(无生产代码改动)。hydration 测试现在还会投递一条来自外 channel 联系人(oc_foreign 群中的 ou_foreign)的消息,并断言其标签在持久化观测与提示词中均保持原始 ID;删除任一 .filter(...) 条件都会使测试失败。新增测试(still processes messages when persisted observations cannot be listed)令 list() 抛错,并断言消息仍被正常处理且 stderr 零输出;删除 try/catch 会使测试失败。
  • [Suggestion] R4-5(静默 HTTP 错误分支从未断言静默) — 已处理:两个重试测试(retries a label lookup... 及其 chat 对应测试)本就安装了 stderrSpy;现在各自断言 expect(stderrSpy).not.toHaveBeenCalled()。删除 HTTP 错误分支上的静默条件现在会使测试套件失败。
  • [Suggestion] R4-6('unknown' 标签守卫无测试) — 已处理:新增测试(keeps the ID label when a resolved name sanitizes to unknown),令 API 返回仅由零宽字符组成的名称,断言查询解析为 undefined 且不写入缓存。删除该守卫会使测试失败。
  • [Suggestion] R4-8(运行期缓存淘汰可能销毁已持久化的标签) — 已按审查者建议的重新 hydration 方案修复。capObservedCache 现在返回是否发生了淘汰;当 names 缓存中已解析的标签被淘汰时,重置 hydratedObservedNames,使下一条入站消息在构建 envelope 之前先从仍然完好的持久化注册表重新 hydration——首次写入不再会用原始 ID 覆盖已持久化的名称。(注册表总共最多保存 500 条观测,因此重新 hydration 本身不会触发淘汰,也不会形成循环。)新增回归测试(re-hydrates persisted labels after in-lifetime cache eviction):将缓存搅动至超过上限、令必然触发的重新查询失败(429),并断言该联系人的下一条 envelope/观测仍携带 Alice

以上七种变异(每次移除一处修复)均已验证会使对应测试失败,恢复后的代码树通过全部测试。

变更文件

  • packages/channels/feishu/src/FeishuAdapter.ts — 上述三处生产代码修复(+23/−11)。
  • packages/channels/feishu/src/adapter.test.ts — 五个新测试、一个扩展测试、两条新增静默断言(+240)。

冲突说明

无 — --conflict false;未执行合并。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • npx vitest run src/adapter.test.ts(packages/channels/feishu)— 97 通过(原 92 + 新增 5)
  • npx vitest run(packages/channels/feishu 整包)— 135 通过
  • npx vitest run(packages/channels/base 整包)— 973 通过
  • npx vitest run src/commands/channel/daemon-worker.test.ts(packages/cli)— 77 通过
  • 变异检查(7 个变异体:逐个移除每处修复)— 每个变异体均使其对应测试失败;恢复后的代码树全绿
  • 对两个变更文件执行 npx prettier --check — 干净

未变更任何 settings 源文件(无需重新生成 schema)。本次触及的行为由包内单元测试覆盖,因此无需运行 bundle/集成测试。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): npm testexit 1.

中文说明

已审查——无阻断问题。 建议见行内评论。 Test Plan(非阻断):npm testexit 1

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +749 to +751
this.capObservedCache(this.observedUserNames);
this.capObservedCache(this.observedChatNames);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-3: Sibling-guard asymmetry: hydrateObservedNames() discards the return value of these capObservedCache calls, while the sibling eviction site in observedNameLookup resets hydratedObservedNames on eviction precisely because evicting a resolved label risks the next initial write clobbering the persisted name with the raw ID. — Failure scenario: a daemon that has resolved >500 distinct labels evicts an entry in observedNameLookup and resets the flag; the next message's hydration re-inserts registry labels into the now-full cache, the cap evicts another label, but the return value is discarded and the flag stays true, so no re-hydration is scheduled; the evicted contact's next envelope is built with the raw ID and recordObservedContact overwrites the persisted name label; if its lookup entry was also evicted and the fresh lookup fails (e.g. 429, retained until restart by design), the name stays reverted until daemon restart. Probe-reproduced end-to-end at this commit. Suggested fix: mirror the sibling guard here (reset hydratedObservedNames when either cap evicts), ideally together with the insertion-order fix in the R5-10 thread — under sustained churn a bare flag reset re-runs a full synchronous registry read on the next message.

中文说明

R5-3: 兄弟守卫不对称:hydrateObservedNames() 丢弃了这些 capObservedCache 调用的返回值,而 observedNameLookup 中的另一个淘汰点在淘汰时会重置 hydratedObservedNames——正因为淘汰已解析标签后,下一次初始写入可能会用原始 ID 覆盖已持久化的名称。失败场景:当 daemon 解析过 500+ 个不同标签后,observedNameLookup 淘汰一个条目并重置标志;下一条消息的 hydration 把注册表标签重新插入已满的缓存,上限逻辑又淘汰另一个标签,但返回值被丢弃、标志仍为 true,于是不会安排再次 hydration;被淘汰联系人的下一条 envelope 以原始 ID 构造,recordObservedContact 覆盖已持久化的名称标签;若该联系人的查询条目也已被淘汰且新查询失败(如 429,按设计保留到重启),名称将一直保持回退状态直到 daemon 重启。已在本提交上用探针端到端复现。建议修复:在此处镜像兄弟守卫(任一 cap 发生淘汰时重置 hydratedObservedNames),最好与 R5-10 线程中的插入顺序修复一并处理——在持续搅动下,单纯重置标志会让下一条消息重新执行一次完整的同步注册表读取。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +836 to +842
if (!response.ok) {
if (response.status === 401) {
this.tokenCache = undefined;
options.lookups.delete(options.id);
}
return undefined;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-4: The 401-vs-non-401 branch distinction in this block is pinned by no test: forcing if (response.status === 401) to always-true survives all 97 adapter tests (mutant measured with a validated harness), and moving options.lookups.delete(options.id) outside the 401 check also survives. — Failure scenario: a regression widening the condition would clear tokenCache on every failed lookup response — a rate-limited (429) or flapping (500) endpoint would invalidate the cached tenant token on every failed background lookup, forcing a full tenant_access_token refresh for each subsequent silent lookup (auth-endpoint request amplification in the exact hot path this PR adds); symmetrically, making non-401 failures retryable would re-request a persistently failing endpoint on every inbound message from each affected contact. Suggested fix: add a case beside the existing 401 tests — lookup returns 429/500, then assert the next lookup reuses the cached token (a fetch mock counting /auth/v3/tenant_access_token/internal calls expects exactly one) and that no second lookup is issued for the same ID.

中文说明

R5-4: 该代码块中 401 与非 401 的分支区分没有任何测试固定:把 if (response.status === 401) 变异为恒真后,全部 97 个适配器测试仍然通过(变异体已经过校验的测试框架实测);把 options.lookups.delete(options.id) 移到 401 判断之外同样能存活。失败场景:若回归放宽该条件,任何失败的查询响应都会清除 tokenCache——被限流(429)或不稳定(500)的接口会导致每次后台查询失败都使缓存的 tenant token 失效,后续每次静默查询都要完整重新获取 tenant_access_token(恰在本 PR 新增的热路径上造成认证接口请求放大);对称地,若使非 401 失败可重试,则每个受影响联系人的每条入站消息都会反复请求一个持续失败的接口。建议修复:在现有 401 测试旁新增用例——查询返回 429/500,然后断言下一次查询复用缓存 token(统计 /auth/v3/tenant_access_token/internal 调用次数的 fetch mock 应恰好为 1 次),且同一 ID 不会发起第二次查询。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +730 to +731
if (label === id) return;
const current = best.get(id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-5: The if (label === id) return; guard is load-bearing but unpinned: no hydration fixture feeds an ID-label entry, which is the common real registry state after any run where enrichment failed. Deleting the guard leaves all 97 tests passing (mutation measured; flip probe verified at this commit). — Failure scenario: with the guard gone, after a daemon restart hydration seeds observedUserNames/observedChatNames with raw IDs, and observedNameLookup's names.get short-circuit returns the ID as a "resolved" name — so no real lookup is ever issued for precisely the never-enriched contacts until cache eviction or another restart, silently disabling enrichment for them. Suggested fix: extend a hydration fixture with an ID-label entry (e.g. { id: 'ou_pending', label: 'ou_pending', lastObservedAt: ... }) and assert a lookup IS issued for it (user_ids: ['ou_pending']) and that the raw ID is not cached as a name.

中文说明

R5-5: if (label === id) return; 守卫起实际作用但没有测试固定:所有 hydration 测试夹具都没有提供「标签即 ID」的条目,而这正是任何一次补全失败后注册表的常见真实状态。删除该守卫后全部 97 个测试仍通过(已实测变异体,并在本提交上用翻转探针验证)。失败场景:守卫被删除后,daemon 重启时 hydration 会把原始 ID 播进 observedUserNames/observedChatNames,而 observedNameLookupnames.get 短路会把这个 ID 当作「已解析」名称返回——于是恰好是那些从未补全成功的联系人,在缓存淘汰或再次重启之前永远不会发起真正的查询,补全被静默禁用。建议修复:在 hydration 夹具中增加一条「标签即 ID」的条目(如 { id: 'ou_pending', label: 'ou_pending', lastObservedAt: ... }),断言会为其发起查询(user_ids: ['ou_pending']),且原始 ID 不会被缓存为名称。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +889 to +890
if (!senderName && !chatName) return;
const key = this.observedContactKey(envelope);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-6: Chat-only enrichment (user lookup fails, group name resolves) is untested — only the user-only mirror case exists, while this PR's own docs anticipate the harm case ("cross-application IDs and external users may remain unresolved"). Mutating the guard to if (!senderName) return; passes the entire suite (flip-verified by probe at this commit). — Failure scenario: if that regression ships, group labels stop being persisted whenever the sender lookup fails — the common real case for external/cross-app senders in a readable group — and no test catches it. Suggested fix: add the mirror test — user lookup returns a non-zero API code while the chat lookup resolves 'Project Group'; assert the enriched observation keeps the ID user label and carries the resolved group label.

中文说明

R5-6: 仅群名补全(用户查询失败、群名解析成功)没有测试——目前只有「仅用户解析成功」的镜像用例,而本 PR 自己的文档明确预期了这种受损场景(「跨应用 ID 与外部用户可能无法解析」)。把守卫变异为 if (!senderName) return; 后整个测试套件仍全部通过(已在本提交上用翻转探针验证)。失败场景:若该回归上线,只要发送者查询失败,群标签就不再被持久化——这正是可读群组中外部/跨应用发送者的常见真实场景——而且没有任何测试能发现。建议修复:补充镜像测试——用户查询返回非 0 API code,而群查询解析出 'Project Group';断言补全观测保留 ID 用户标签并携带已解析的群标签。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines 661 to 664
} finally {
this.tokenRefreshPromise = undefined;
this.tokenRefreshHasCoreWaiters = false;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-7: The tokenRefreshHasCoreWaiters = false reset in this finally block is the only thing preventing a permanent stderr-noise regression, and it is unpinned: the existing core-waiter test runs exactly one refresh cycle and the silence test starts from a fresh flag — deleting the reset passes both. — Failure scenario: after a core-waited failed refresh (logged once), the flag stays true forever, so every later enrichment-only (silent) refresh failure writes getTenantAccessToken failed/error noise to stderr — re-introducing exactly the behavior an earlier round's Critical fixed. Harm-probed at this commit (stderr written twice under the mutant). Suggested fix: in the existing core-waiter test, after the first logged failure clear tokenCache again and trigger a second refresh via getTenantAccessToken({ silent: true }) only, asserting the stderr spy was still called exactly once.

中文说明

R5-7:finally 块中的 tokenRefreshHasCoreWaiters = false 重置是防止 stderr 噪声永久性回归的唯一屏障,但没有测试固定:现有的核心等待方测试只运行一个刷新周期,静默测试又从全新标志开始——删除该重置后两个测试都通过。失败场景:在一次有核心等待方的失败刷新(输出一次日志)之后,标志永远保持为 true,此后每次仅补全(静默)的刷新失败都会向 stderr 输出 getTenantAccessToken failed/error 噪声——恰好复现了前几轮评审中 Critical 所修复的行为。已在本提交上做危害探针(变异体下 stderr 被写入两次)。建议修复:在现有核心等待方测试中,第一次失败日志之后再次清除 tokenCache,仅通过 getTenantAccessToken({ silent: true }) 触发第二次刷新,断言 stderr spy 仍只被调用一次。

— qwen3.8-max via Qwen Code /review (v0.21.7)

}
});

it('retries a 401 lookup for the same ID once the token refreshes', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-8: The 24-line token-refresh/401 fetch-routing mock in this test is byte-identical to the one in 'refreshes the token after a label lookup returns 401' (lines 605-628 vs 684-707, diff-verified), and a near-identical success-routing mock repeats in at least three more tests (~14 URL-routing mocks total in this file). — Failure scenario: if the token endpoint path, the token response shape (tenant_access_token/expire), or the batch response shape changes, every copy must be located and edited separately; a missed copy keeps stubbing the old contract, so that test either fails for the wrong reason or silently passes against an outdated stub. This PR already extracts repeated setup into jsonResponse/feishuGroupMessage/feishuDmMessage helpers for exactly this reason; the fetch routers were left behind that pattern. Suggested fix: extract one helper next to jsonResponse (e.g. mockLabelFetchRoutes({ staleToken401, userName, chatName })) that installs the URL-routing implementation, and have the enrichment tests call it with per-test overrides.

中文说明

R5-8: 本测试中 24 行的 token 刷新/401 fetch 路由 mock 与 'refreshes the token after a label lookup returns 401' 中的一字不差(605-628 行对 684-707 行,已用 diff 校验),且近似相同的成功路由 mock 在至少另外三个测试中重复出现(本文件共约 14 个 URL 路由 mock)。失败场景:若 token 接口路径、token 响应结构(tenant_access_token/expire)或批量查询响应结构发生变化,每一处副本都必须逐一找到并分别修改;漏改的副本会继续按旧契约打桩,导致该测试要么因错误原因失败,要么对着过期桩静默通过。本 PR 已经为此把重复的测试准备抽成了 jsonResponse/feishuGroupMessage/feishuDmMessage 辅助函数;fetch 路由器被落在了这一模式之外。建议修复:在 jsonResponse 旁抽取一个辅助函数(如 mockLabelFetchRoutes({ staleToken401, userName, chatName }))安装 URL 路由实现,让补全测试以各自的覆盖参数调用它。

— qwen3.8-max via Qwen Code /review (v0.21.7)

Comment on lines +743 to +745
for (const [id, entry] of newestUser) {
this.observedUserNames.set(id, entry.label);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R5-10: Hydrated labels are inserted newest-first (list() sorts by lastObservedAt descending and Map preserves insertion order), so once the cache exceeds the cap, FIFO eviction removes the NEWEST/hottest hydrated labels first — the inverse of what the eviction mitigation relies on. Probe-verified deterministically at this commit. — Failure scenario: with a near-full registry plus a few newly resolved contacts, every new successful lookup's capObservedCache deletes cache.keys().next() — the newest hydrated label — while older ones survive; the hottest contacts fall out of the cache first, and each eviction resets hydratedObservedNames, forcing a full synchronous registry re-read + parse on the next onMessage. Harm is modest (small file, bounded churn), but the ordering is mechanically inverted. Suggested fix: insert hydrated entries oldest-first so FIFO eviction removes the stalest labels — e.g. iterate newestUser/newestChat in reverse before set (both loops).

中文说明

R5-10: hydration 以「最新在前」的顺序插入标签(list()lastObservedAt 降序排序,Map 保持插入顺序),因此缓存一旦超过上限,FIFO 淘汰会先移除最新/最热的 hydration 标签——与淘汰缓解机制所依赖的顺序恰好相反。已在本提交上用探针确定性验证。失败场景:注册表接近占满且新解析了若干联系人时,每次新的成功查询触发的 capObservedCache 都会删除 cache.keys().next()——即最新的 hydration 标签——而更旧的条目反而留存;最热的联系人最先掉出缓存,且每次淘汰都重置 hydratedObservedNames,迫使下一条 onMessage 重新同步读取并解析整个注册表。危害不大(文件小、搅动有界),但顺序在机制上是反的。建议修复:以「最旧在前」插入 hydration 条目,使 FIFO 淘汰移除最陈旧的标签——例如在 set 之前反向遍历 newestUser/newestChat(两个循环都改)。

— qwen3.8-max via Qwen Code /review (v0.21.7)

@wenshao

wenshao commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Maintainer local verification — ✅ merge-ready

49/49 scripted assertions passed (0 failed). Verified head: 119e5653 (merge ref be66a220, control = merge-ref parent 63a8ed43). Real qwen serve daemon + real adapter code on both arms; open.feishu.cn redirected at the undici dispatcher layer to a loopback fake peer; signed im.message.receive_v1 webhook events; isolated QWEN_HOME.

中文摘要

结论:可合并 —— 49/49 条脚本断言全部通过(验证 head 119e5653)。

  • A/B:base 控制组标签始终为 ou_…/oc_… 原始 ID 且 lookup 请求为 0;head 先落 ID 标签、随后异步补全为真实姓名/群名,第二条消息不再发起新 lookup(缓存去重),机器人回复端到端送达。
  • 降级:lookup 返回 403(模拟未开通权限)时消息投递不受影响、标签保持 ID、无 token 错误日志。
  • token 失败重试:首次 token 401 跳过本次 lookup 且不缓存为永久失败,下一条消息重试成功并完成补全。
  • 重启水合:重启 daemon 后已知联系人首条消息直接带水合名称标签(不回退 ID)且不产生新 lookup —— R1 评审的 restart-hydration Critical 修复已验证。
  • 测试门:feishu 135/135、channel-base 1014/1014 通过;删除 enrichment 触发调用后恰有 9 条测试失败(新测试非空转)。
  • 未覆盖:未连真实飞书服务(无测试 bot);WebSocket 长连接模式、DM 私聊、user_id/union_id 分支、缓存淘汰、Windows/Linux 未跑 E2E。

A/B — central claim is load-bearing

Cell Oracle BASE 63a8ed43 HEAD 119e5653
Initial observation observed-contacts.json ou_alice / oc_group1 ou_alice / oc_group1
After enrichment settles same file still raw IDs after 3 s Alice Zhang / Verify Group
basic_batch + im/v1/chats lookups on wire fake-peer log 0 + 0 1 + 1
2nd message (same sender/group) wire counts n/a no new lookups (still 1 + 1)
Bot reply delivered POST /im/v1/messages yes yes

store-and-wire-ab

Scenario matrix

Scenario Assertions Result
head / enrich (happy path + cache dedup + reply) 11/11
base / enrich (control) 9/9
head / degrade (lookups → 403) 8/8
head / token401 (token 401 → skip lookup → retry next message) 9/9
head / restart (hydration from persisted store) 11/11
mutation check (delete enrichment trigger → 9 adapter tests fail) 1/1

Unit gates: packages/channels/feishu 135/135, packages/channels/base 1014/1014 (vitest, head tree).

The restart row directly covers the R1 review Critical (restart hydration clobbering newer labels): after a daemon restart, the first message from a known contact is persisted with the hydrated name labels — no revert to ou_… — and issues zero lookups.

head enrich run base control run
head-enrich base-enrich
degrade (403) token401 retry restart hydration
degrade token401 restart

Notes

  • With a single token-401 injected, a racing core caller (bot-info) absorbs it and the same message still enriches — observed behavior is at least as good as the claimed "retry next message".
  • The two-phase write (ID row replaced in place by the enriched row) is inherent to the async design and matches the PR description.

Not covered

No live Feishu bot was available — wire evidence comes from the loopback fake peer running the PR's real adapter code (token format/rate-limit specifics simulated). Not exercised: WebSocket long-connection mode, DM (p2p) enrichment, user_id/union_id sender branches, cache-cap (500) eviction, Windows/Linux.

— maintainer local verification run (wire-oracle harness, macOS, Node v22.22.2); full report + harness in tmp/pr8569-verify-20260807-235053/

@wenshao

wenshao commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge August 7, 2026 16:24
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 115 passed · 0 failed · 115 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:115 通过 · 0 失败 · 115 总计

Verification report

Verdict: merge-ready — 115/115 scripted assertions passed (0 unexpected failures).
Verified head: 119e565317ad821368f64aac48877cb12d0d1464 (git rev-parse HEAD^2), A/B base: 63a8ed433 (HEAD^1, the merge-ref base tip — the metadata snapshot's baseRefOid 5fdcdb28… had drifted behind the regenerated merge ref).

中文摘要

结论:merge-ready。核心主张(飞书观测联系人标签异步富化:先写 ID 观测,后台解析用户/群名后补写富化观测)通过 A/B 实证:head 侧 6 个场景全部出现预期的富化/缓存/重试行为,base 侧零 OpenAPI 请求、标签恒为 ID,差异完全归因于本 PR(表 1,证据图 01-ab-central-cells-head-vs-base.png)。

A/B 结论:12 个单元(head/base × 6 场景)共 96 条脚本断言全部通过:富化写入顺序正确、二次消息不再发起查询(wire 计数冻结)、权限缺失保留 ID 标签且不阻塞投递、token 获取失败可重试、401 后失效重取、重启后从持久化注册表水合(零查询即出名称)、跨频道标签隔离、DM 场景只查用户不查群。边界探针 11 条全过(union_id/user_id 类型映射、恶意名称消毒、'unknown' 折叠拒绝、64 码点截断无孤立代理项、部分失败时半富化,图 03-boundary-probes-head.png)。

测试有效性:5 个定点变异全部被测试套件击杀(9/7/3/4/1 条测试变红),无幸存者(图 02-mutation-matrix-kills-all-five.png)。目标门禁全绿:feishu 97/97、channel-base 586/586、daemon-worker 77/77。

Findings:仅 1 条 nit——静默发起的 token 刷新被核心调用者中途加入时,刷新级错误日志会被抑制(窗口极窄,且发送路径仍有 Cannot send: no access token. 兜底日志,原因仍可观测)。

未覆盖:逐提交归因(浅克隆仅 3 个提交可达)、真实飞书联调、Windows/Linux 实机、全仓 lint/typecheck(CI 覆盖)、作者所述 macOS/Node 25 下 15 条未改动 CLI 测试失败(本容器 Linux/Node 22 无法归因)、docs 文案审阅。

Central claim and A/B proof

Central claim: Feishu observed-contact entries are enriched with sender display names and group names — an ID-based observation is persisted immediately, lookups run in the background, an enriched observation is written when either lookup succeeds, completed lookups are cached for the channel lifetime, and token-acquisition failures before a lookup remain retryable — all without blocking message delivery.

Environment per cell: node:22-bookworm container; compiled dist/ builds driven through the adapter's real onMessage inbound path. Head tree = merged HEAD build; base tree = worktree at HEAD^1 with only packages/channels/base + packages/channels/feishu rebuilt. Workspace-link purity asserted by realpath before trusting the control: readlink -f node_modules/@qwen-code/channel-base → head tree /__w/qwen-code/qwen-code/packages/channels/base, base tree /__w/qwen-code/qwen-code/tmp/base-tree/packages/channels/base; the base dist contains zero enrichObservedContact symbols. The lockfile is untouched by the PR, so sharing root node_modules is a clean control. The fake Feishu OpenAPI is a real loopback HTTP server; the only interposition is a fetch wrapper rewriting the hardcoded https://open.feishu.cn/open-apis prefix to loopback — every request the adapter constructs (method, path, user_id_type query, Authorization header, JSON body) travels over a real socket and is recorded server-side (the wire oracle). Raw logs: logs/ab-<arm>-<scenario>.log.

# Scenario Observable oracle Head Base (control)
A Group msg ×2, lookups succeed observation label sequence; wire counts ou_user|oc_group → Alice|Project Group, msg2 label enriched immediately; token=1 user=1 chat=1 (msg2 adds 0 requests) ID labels only; 0 wire requests
B Both lookups answer code≠0 (no permission) labels stay IDs; delivery continues; stderr clean; failures cached (no retry on msg2) 2 ID writes, user=1 chat=1 total, stderr=0, promptCalls=2 ID labels; 0 requests
C Token endpoint 503 then success retry-after-token-failure msg1 ID-only (lookups retried), msg2 enriched; token=2 user=1 chat=1, stderr=0 ID labels; 0 requests
D list() seam returns persisted enriched labels (restart) hydrated labels, zero lookups first write already Alice|Project Group; 0 wire requests across 2 msgs list() seam absent in base options: ID labels; 0 requests
E Lookups 401 with stale cached token token invalidation + re-fetch tokenCache dropped, fresh token, retries succeed: token=1 user=2 chat=2, final write enriched ID labels; 0 requests
F p2p DM user lookup only, no chat lookup sender enriched, no group key; chat requests = 0 ID labels; 0 requests

12/12 cells pass; 96 scripted assertions. The base cells encode the expected absence (zero enrichment, zero wire traffic) and pass as predicted — the flip from "IDs only, silent" on base to "enriched, cached, retrying" on head is wholly attributable to this PR. Witness: 01-ab-central-cells-head-vs-base.png (scenario A on both arms as printed).

Secondary claims verified: delivery is never blocked by lookups (promptCalls=2 in every cell including total-failure B); the PR's accepted tradeoff — lookup failures after a real request are cached for the process lifetime — was measured, not assumed (cell B: second message issued no new lookups).

Boundary probes on the changed surface (11 assertions, all pass; 03-boundary-probes-head.png): on_/raw-user_id senders map to user_id_type=union_id / user_id with the ID in the batch body; a hostile name (Al]ice\u0007[[X) is sanitized to Al ice X and survives the double pass through sanitizeSenderName idempotently; a name collapsing to unknown is rejected (ID retained, single write); a 70-emoji name truncates to exactly 64 code points with no lone surrogate; when the chat lookup is down but the user lookup succeeds, a partial enrichment lands (sender resolved, chat keeps its ID) and delivery is unaffected; stderr stayed clean throughout.

Corrections

None — no prior review round exists for this PR (no previous-report.md), and no inaccurate bot claims were in scope.

Findings

Nit: refresh-level token error can be silenced when a core caller joins a silent-initiated refresh

getTenantAccessToken sets tokenRefreshHasCoreWaiters only when it initiates a refresh (if (!options?.silent) …, FeishuAdapter.ts:656); a non-silent caller that arrives while a silent-initiated refresh is already in flight joins this.tokenRefreshPromise without setting the flag. If that refresh then fails, refreshToken sees the flag false and suppresses the getTenantAccessToken failed/error line even though a core delivery caller is waiting on it. The window is narrow (silent refreshes are only initiated by best-effort enrichment), and the failure cause is not lost: the send path logs its own [Feishu:<name>] Cannot send: no access token. (FeishuAdapter.ts:954, verified present), so this is degraded refresh-level detail, not an unobservable failure. One-line direction if the author cares: set the core-waiter flag when a non-silent caller joins an existing promise as well. No code change requested.

Not covered

  • Per-commit attribution: the checkout is depth-2 (shallow); only 1 of the PR's 11 commits is reachable (git rev-list HEAD^1..HEAD^2 = 1 vs 11 in the metadata). The aggregate HEAD^1..HEAD diff was verified; individual commits were not.
  • Live Feishu end-to-end: no credentials in this sandbox. The harnesses reproduce the OpenAPI wire shape (endpoints, id-type mapping, token flow), not real Feishu-side behavior; the author likewise notes the review-fix commits were not live-tested.
  • Repo-wide gates: full npm test / lint / typecheck are CI-covered and were not re-run; targeted suites only (below).
  • The author's 15 CLI test failures (AuthDialog / memory-diagnostics) on macOS + Node 25: those files are untouched by this PR and the failures are environment-specific; they cannot be attributed or reproduced in this Linux + Node 22 container.
  • Windows/Linux runtime behavior (flagged unvalidated by the author).
  • Docs content (docs/design/…, docs/plans/…, docs/users/features/channels/feishu.md): not reviewed for accuracy.
  • Other channel adapters (telegram/dingtalk) against the new ChannelBase hook: the default onObservedContact is a no-op and recordObservedContact only changed visibility, so no behavior change is possible; their suites run in CI but not here.
  • Long-lifetime cache churn (500-entry caps): exercised only by the PR's own eviction/re-hydration test, not independently load-tested.

Targeted gates (head)

Suite Result
packages/channels/feishu adapter.test.ts 97/97 passed
packages/channels/base ChannelBase.test.ts 586/586 passed
packages/cli daemon-worker.test.ts 77/77 passed (pins list wiring incl. freshWithinSeconds: 365d cap)

Suite liveness proven by mutation rather than assumed — see below.

Vacuity / mutation matrix (5/5 killed, no survivors)

Each mutant is a single-point source edit applied via git apply, run, and reverted (live replay: matrix-replay.sh, patches in patches/):

# Mutation Red tests Verdict
m1 remove onObservedContact hook call (ChannelBase) 9 enrichment tests + the 2 new ChannelBase hook tests (quoted failure: expected [ 'persisted' ] to deeply equal [ 'persisted', 'hook' ]) KILLED — positive control; reverts fail the intended behavioral assertions
m2 disable duplicate-write suppression guard (persistedLabels equality) 7 (write-count assertions) KILLED
m3 silent token refresh always logs (core-waiter flag unconditional) 3 (retry/silence stderr assertions) KILLED
m4 remove hydrateObservedNames() call on the inbound path 4 (hydration tests) KILLED
m5 remove channelName filter in persistedObservedContacts 1 (cross-channel isolation: foreign labels must not hydrate) KILLED

Unmutated controls green on both sides (97/97, 586/586). No mutant regressed a kill; no survivors to adjudicate. Witness: 02-mutation-matrix-kills-all-five.png.

Methodology

Ran in the CI verify container (node:22-bookworm, shared runner) against refs/pull/8569/merge (depth 2): HEAD be66a220d, base 63a8ed433, PR head 119e56531. The head tree was pre-built by the workflow; the base control was a scratch git worktree at HEAD^1 rebuilt for the two affected workspaces only, with node_modules/@qwen-code/channel-base re-linked into the base tree and verified by realpath. Two mock-free harnesses (harness-ab.mjs, harness-probe.mjs, kept in this artifact dir) drove compiled dist/ output through the adapter's real onMessage path against a loopback Feishu OpenAPI server that records every request server-side; the only interposition was host-rewriting the adapter's hardcoded open.feishu.cn fetch prefix. Mutations were applied as git apply patches, tested, and reverted; the working tree ended clean. Per-cell stdout/stderr live in logs/; evidence PNGs were produced with scripts/verify-capture.mjs.

Evidence images

01-ab-central-cells-head-vs-base

02-mutation-matrix-kills-all-five

03-boundary-probes-head

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@wenshao
wenshao added this pull request to the merge queue Aug 7, 2026
Merged via the queue into QwenLM:main with commit 028747a Aug 7, 2026
121 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(feishu): enrich observed contact labels asynchronously

4 participants