Skip to content

test(core): stub resident-agent methods in the agent test registry - #7540

Closed
he-yufeng wants to merge 1 commit into
QwenLM:mainfrom
he-yufeng:fix/agent-test-resident-stub
Closed

test(core): stub resident-agent methods in the agent test registry#7540
he-yufeng wants to merge 1 commit into
QwenLM:mainfrom
he-yufeng:fix/agent-test-resident-stub

Conversation

@he-yufeng

@he-yufeng he-yufeng commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Un-reds main's ubuntu CI: agent.test.ts > runs a non-interactive fork through the background registry fails on every current main run.

Why it's needed

#7460 added an assertion that the fork completes through registry.complete, but the shared stub registry in beforeEach lacks registerResidentAgent / unregisterResidentAgent. The background turn hits the missing method, the turn promise rejects into reportUnexpectedBackgroundError, and complete never runs. I reproduced it locally: execute() is called once, then the turn dies at the resident-agent line. Two stub methods fix it. The production path is fine; forks are supposed to stay resident.

Reviewer Test Plan

How to verify

On current main the focused test fails 1/197. With this change npx vitest run packages/core/src/tools/agent/agent.test.ts is 197/197.

Evidence (Before & After)

N/A (test-only change)

Tested on

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

Main's ubuntu CI has been red since QwenLM#7460 on
"runs a non-interactive fork through the background registry". That
commit added a completion assertion (registry.complete called with the
fork's final text), but the shared stub BackgroundTaskRegistry lacks
registerResidentAgent/unregisterResidentAgent. The fork's background
turn reaches the resident-agent branch, hits the missing stub method,
and the turn promise rejects into reportUnexpectedBackgroundError
before registry.complete ever runs.

The production path is fine (forks are meant to stay resident); the
harness stub was just two methods short. Full agent.test.ts: 197/197.

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ (minor: "Risk & Scope" and "Linked Issues" sections are missing, but for a 2-line test stub fix the essential sections are all present and well-filled — not worth a round-trip).

Problem: observed CI failure on main — agent.test.ts > runs a non-interactive fork through the background registry fails deterministically. Root cause is clear: #7460 added a stubRegistry.complete assertion, but the shared beforeEach stub lacks registerResidentAgent / unregisterResidentAgent, so the fork's background turn dies before complete is ever called. I reproduced the failure locally on current main (1/197 fails, complete spy has 0 calls).

Direction: fixing broken CI is unambiguously aligned. The production code is correct — forks are meant to stay resident — and only the test harness stub was incomplete.

Size: 2 additions, 0 deletions, 1 file (agent.test.ts). All changes are in a test file — 0 production logic lines. Not applicable for core module escalation.

Approach: this is the minimal possible fix — two vi.fn() stubs added to the existing shared registry stub, following the exact same pattern as every other method in that object. Nothing to cut, nothing extraneous.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓(小问题:缺少 "Risk & Scope" 和 "Linked Issues" 部分,但对于一个 2 行测试 stub 修复,核心部分都已完整填写——不值得为此来回修改)。

问题:main 分支上的 CI 失败——agent.test.ts > runs a non-interactive fork through the background registry 确定性失败。根因清晰:#7460 添加了 stubRegistry.complete 断言,但共享的 beforeEach stub 缺少 registerResidentAgent / unregisterResidentAgent,导致 fork 的后台任务在调用 complete 之前就中断了。我在当前 main 上本地复现了该失败(1/197 失败,complete spy 调用次数为 0)。

方向:修复 CI 失败毫无疑问是对齐的。生产代码是正确的——fork 本应保持 resident——只是测试 harness 的 stub 不完整。

规模:2 行新增,0 行删除,1 个文件(agent.test.ts)。所有改动都在测试文件中——0 行生产逻辑。不需要核心模块升级。

方案:这是最小化的修复——在现有的共享 registry stub 中添加两个 vi.fn() stub,与该对象中其他方法的模式完全一致。没有可删减的,没有多余的。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal before reading the diff: the fork path in agent.ts calls registry.registerResidentAgent() (line 3584) when canStayResident && !needsAutoPermissionLease(). The shared beforeEach stub at the top of agent.test.ts doesn't have that method, so the background turn throws and registry.complete never fires. The fix is to add registerResidentAgent: vi.fn() and unregisterResidentAgent: vi.fn() to the stub — exactly what this PR does.

The diff matches my independent proposal line-for-line. Two vi.fn() stubs, same pattern as the 18 other methods in that object, placed right after appendActivity. No correctness issues, no convention violations, nothing extraneous. The later describe block (line ~4444) already has its own local stubs for these methods — the shared beforeEach stub was simply never updated when the resident-agent feature landed.

No findings. Clean.

Testing

This is a test-only change with no user-visible behavior — the unit test itself is the real scenario. Ran the specific test on current main (before) and with the two stub lines added (after):

Before (current main)

 FAIL  src/tools/agent/agent.test.ts > AgentTool > Fork dispatch (subagent_type: "fork") > runs a non-interactive fork through the background registry
AssertionError: expected "spy" to be called with arguments: [ Any<String>, …(2) ]

Number of calls: 0

 ❯ src/tools/agent/agent.test.ts:3157:37
    3155| 
    3156|       await vi.runAllTimersAsync();
    3157|       expect(stubRegistry.complete).toHaveBeenCalledWith(
       |                                     ^
    3158|         expect.any(String),
    3159|         'headless fork result',

 Test Files  1 failed (1)
      Tests  1 failed | 196 skipped (197)

After (with this PR's change applied)

 ✓ src/tools/agent/agent.test.ts (197 tests | 196 skipped) 18ms

 Test Files  1 passed (1)
      Tests  1 passed | 196 skipped (197)

Full file run with the fix: 197/197 passed, no regressions.

中文说明

代码审查

在读 diff 之前的独立方案:agent.ts 中的 fork 路径在 canStayResident && !needsAutoPermissionLease() 时调用 registry.registerResidentAgent()(第 3584 行)。agent.test.ts 顶部的共享 beforeEach stub 没有这个方法,所以后台任务抛异常,registry.complete 永远不会被调用。修复方法是在 stub 中添加 registerResidentAgent: vi.fn()unregisterResidentAgent: vi.fn()——与这个 PR 完全一致。

Diff 与我的独立方案逐行匹配。两个 vi.fn() stub,与该对象中其他 18 个方法的模式相同,放在 appendActivity 之后。没有正确性问题,没有规范违反,没有多余内容。后面的 describe 块(约第 4444 行)已经有这些方法的本地 stub——只是共享的 beforeEach stub 在 resident-agent 功能落地时没有同步更新。

无发现。干净。

测试

这是一个纯测试改动,没有用户可见行为——单元测试本身就是真实场景。在当前 main 上运行了特定测试(修改前),以及添加两行 stub 后(修改后):

修改前:1/197 失败,stubRegistry.complete 调用次数为 0。
修改后:197/197 全部通过,无回归。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is exactly the kind of PR you want to see: a clear root-cause explanation, a two-line fix that matches the existing stub pattern, and a verified before/after. The author correctly identified that the production code is fine and only the test harness was incomplete — the shared beforeEach stub was never updated when the resident-agent feature landed, and #7460's new assertion exposed the gap. I reproduced the failure on current main and confirmed the fix resolves it (197/197). Nothing to maintain, nothing to regret in six months.

中文说明

置信度:5/5 —— 每个阶段都很干净;毫不犹豫地合并。

这正是你想看到的 PR:清晰的根因分析、符合现有 stub 模式的两行修复、以及经过验证的 before/after。作者正确地指出生产代码没有问题,只是测试 harness 不完整——共享的 beforeEach stub 在 resident-agent 功能落地时没有同步更新,而 #7460 的新断言暴露了这个缺口。我在当前 main 上复现了失败,并确认修复解决了问题(197/197)。没有维护负担,六个月后也不会后悔。

Qwen Code · qwen3.8-max-preview

Reviewed at b290977e02bf40d7324339c14d86c2f60171ce05 · 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.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

@he-yufeng

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #7538, which was opened about two hours earlier and stubs the same registry methods plus the rest of the set agent.ts can reach (getQueuedCount, restartCompletedAgent, waitForMessages, bridgeApprovalEvents) and the fork-dispatch assertion. That superset covers this one's ground.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants