Skip to content

test(cli): reuse shared wait budget in tui-mcp-control - #4106

Open
bferanmi806-sketch wants to merge 1 commit into
apache:mainfrom
bferanmi806-sketch:test/4094-tui-mcp-wait-budget
Open

test(cli): reuse shared wait budget in tui-mcp-control#4106
bferanmi806-sketch wants to merge 1 commit into
apache:mainfrom
bferanmi806-sketch:test/4094-tui-mcp-wait-budget

Conversation

@bferanmi806-sketch

@bferanmi806-sketch bferanmi806-sketch commented Aug 28, 2026

Copy link
Copy Markdown

Summary

Closes #4094

Replaces the local iteration-count waitFor in packages/cli/src/__tests__/tui-mcp-control.test.ts with the existing shared wall-clock waitFor from tui-terminal-mock.ts.

The shared helper uses the repository's established wait-budget policy: 250 ms locally, 5 seconds in CI, with MAKA_TEST_WAIT_BUDGET_MS available as an override. This follows the pattern introduced in #2304 for #2221.

All 27 waits now include descriptive timeout messages, so failures report what state was being awaited instead of ending with an opaque predicate assertion.

No production MCP behavior changed.

Verification

  • npm --workspace maka-agent run build — passed after dependencies were built
  • node --test "dist/__tests__/tui-mcp-control.test.js"19/19 passed
  • git diff --check — clean
  • Biome check/lint on the changed file — passed

The targeted suite includes same-server credential retirement stays inside the shared config transaction, the test that exposed the CI race reported in #4094.

A wider maka-agent run on this Windows environment encountered filesystem/runtime-host failures outside the changed test. Those are noted separately and are not attributed to this patch.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Muse Spark (opencode)was used to inspect the existing test infrastructure, audit the affected wait sites, implement reuse of the shared helper, add descriptive timeout messages, and run validation. The resulting change was reviewed by me.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behaviour?

  • Yes — described under Summary above
  • No

Replace the local iteration-count waitFor (1000 * setImmediate) with
the existing shared wall-clock helper from tui-terminal-mock.ts
(WAIT_BUDGET_MS = 250ms local / 5000ms CI / MAKA_TEST_WAIT_BUDGET_MS).
Add descriptive timeout messages to all 27 waits and remove the flaky
local helper. Fixes the race that failed the same-server credential
retirement transaction under CI load.

Closes apache#4094

Generated-by: Muse Spark (opencode/muse-spark-1.2-contributor-free)
@bferanmi806-sketch
bferanmi806-sketch marked this pull request as ready for review August 28, 2026 17:57
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 28, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this at the waiting primitive. The old helper counted scheduler turns rather than elapsed time, so under CI load it could give up before the file transaction completed. Reusing the shared waitFor removes duplicated timing policy and gives failures useful timeout descriptions.

I verified exact head b8cb035c611fcafeee7460b5817482fd706cb33d: the focused CLI suite passes 19/19, together with the CLI build, typecheck, Biome, and git diff --check. I found no blocking issues.

One pre-existing ten-turn setImmediate loop remains around the second transaction attempt. It is nonblocking and not introduced here; if that path flakes later, an observable synchronization point would be clearer.

The exact head does not have a test check yet, so this approval does not by itself make the PR merge-ready.

Review analysis was assisted by Codex; Astro-Han reviewed the result and owns this approval.

简体中文

谢谢从等待机制本身解决这个问题。旧 helper 计算的是调度轮次,而不是实际经过的时间,因此 CI 负载较高时,可能在文件事务完成前就提前放弃。复用共享的 waitFor 删除了重复的超时策略,也让失败信息更容易定位。

我验证了精确 head b8cb035c611fcafeee7460b5817482fd706cb33d:CLI focused suite 19/19 通过,build、typecheck、Biome 和 git diff --check 也通过。没有阻塞问题。

测试中仍有一处旧的十次 setImmediate 循环,用于等待第二次事务尝试。这不是本次引入的,也不阻塞;如果后续仍发生 flake,可以改成可观测的同步点。

当前 head 尚未出现 test check,因此本次 Approve 本身不代表已经可以合并。

本次审查分析由 Codex 协助;Astro-Han 审阅了结果并对本次 Approve 负责。

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

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(cli): tui-mcp-control's local waitFor spins 1000 immediates with no wall-clock budget, flaky under CI load

2 participants