You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/09/18 04:51:55 UTC
[PR] test(runtime-host): bound the PTY integrity-failure burst [maka]
Astro-Han opened a new pull request, #5473:
URL: https://github.com/apache/maka/pull/5473
## Summary
The `treats acquire during PTY integrity failure` regression from #5406 times out in CI (`Timed out waiting for PTY control to close`, run 35305985449). The test flooded \\033[c DA queries without a limit; once the collector's pending queue crossed its 1MB budget, evictions dropped most admissions before parsing, so accumulating 1MB of protocol replies became a throughput race with the parser. Under parallel `node --test` load that race can exceed any fixed timeout.
The burst now emits ~800KB of queries in one shot — under the pending budget, so nothing is evicted and every query parses — then the process idles alive behind the same TERM trap. Reply accumulation no longer depends on outrunning an endless flood; it is bounded by parsing a fixed 800KB, which any live runner completes in seconds.
## Verification
- `node --test --test-concurrency=4 dist/__tests__/runtime-resource-process.test.js` in `packages/runtime-host`: 4/4 pass; the integrity-failure case now finishes in ~2.2s (previously up to the 60s probe ceiling).
- `npm run format` and `npm run lint`: clean.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Devin diagnosed the CI failure and authored the test change.
## Checklist
- [x] Tests cover the change and fail without it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(runtime-host): deflake the PTY integrity burst and the two-client CAS race [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #5473:
URL: https://github.com/apache/maka/pull/5473#issuecomment-5726263262
@jackwener P1 确认成立——"机制会触发 ≠ 测试变确定"正是病根。把 60s 的去向查清后，第三个缺陷藏在**探针本身**，不在触发机制。
复现 + 定位：stall 时 `manager.live` 里已无该 run（`live=none`），`writeStdin` 每次都正常返回。机制是：
1. `writeStdin` 走 `collector.mutateAtCut`，排在整条解析链后面；
2. collector fail 瞬间已在队列中的那个 poll 被 `throwIfUnavailable` 抛出 collector failure（普通 Error，不是 `ShellRunPtyControlClosedError`）→ writeStdin 的 catch 不在重抛白名单 → `operationFailed` → `await live.finished.join()` 泊车；
3. 它要睡过整个 teardown（TERM 被 trap → 2s 宽限 → SIGKILL → finalize → evict），resolve 时 live 已移出 live map；
4. 之后所有 poll 走 `writeStdinWithoutLive` → 正常返回 → `return false` → 60s。
串行循环被自己的 in-flight 调用睡过了唯一能观察到 closed 的窗口——burst 快慢只改变 fail 时机，不改变这个竞态结构。这解释了为什么 head/base 失败率一致，也解释了为什么本机 84ms 的隔离验证没证明测试确定。
修法（a79e5fb）：探针换成 `manager.getLivePtySnapshot(...) === null`——`integrityFailure` 置位即 null、teardown/evict 全程保持 null（单调）、同步无队列不泊车，而且是 acquire handler 查的同一个谓词，wait 观察的状态迁移和断言同源。没有改 writeStdin 的 post-failure 语义：返回 `{failed: true}` 对真实调用方是合理的，丢 closed 信号只发生在把它当探针用时。
验证：snapshot 探针 16/16 CLOSED ~170ms（含 10/10 CPU 饱和）；同一条件下旧 writeStdin 探针 1/3 stall；测试文件重复 4/4 过。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(runtime-host): deflake the PTY integrity burst and the two-client CAS race [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #5473:
URL: https://github.com/apache/maka/pull/5473
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] test(runtime-host): deflake the PTY integrity burst and the two-client CAS race [maka]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #5473:
URL: https://github.com/apache/maka/pull/5473#discussion_r4044158567
##########
packages/runtime-host/src/__tests__/session-catalog-two-client-uds.test.ts:
##########
@@ -301,7 +301,7 @@ test('two Clients share stable Session creation, CAS configuration, and catalog
sessionId: created.id,
expectedRevision: configurationRevision,
patch: {
- permissionMode: 'bypass',
+ permissionMode: 'ask',
Review Comment:
This one is sound, and the reasoning is the part worth keeping.
The Session is created with `permissionMode: 'bypass'` and the default `orchestrationMode`, so the racing patch here was `bypass` + `default` — identical to the existing configuration. When it was admitted first, the idempotent-commit short-circuit returned `committed` without writing, leaving the revision untouched, so the desktop's patch then also committed. `['committed', 'committed']` was not a race the test failed to pin down; it was the correct outcome of a no-op patch, and passing depended on admission order.
Sending a patch that genuinely changes configuration (`'ask'` + `'default'` against the desktop's `'bypass'` + `'graph'`) makes the second arrival hit the revision check whichever it is. That is a strictly stronger test than before: the previous version could pass while never exercising the CAS path at all.
Three consecutive runs of the file pass here (3/3 each). Worth noting that this change makes the assertion mean what it says, rather than relaxing it — the opposite of what a deflake usually does, and the reason it reads as a fix rather than a suppression.
<details><summary>简体中文</summary>
这一处是站得住的,而且**值得留下的是它的推理**。
Session 创建时用的是 `permissionMode: 'bypass'` 加默认 `orchestrationMode`,所以这里参与竞争的补丁 `bypass` + `default` **与既有配置完全相同**。当它先被准入时,幂等提交短路会直接返回 `committed` 而不写入、revision 不动,于是桌面端那次也跟着提交成功。`['committed','committed']` 不是测试没能钉住的竞争,而是**空补丁的正确结果**;能不能通过取决于准入顺序。
改成真正会改变配置的补丁(`'ask'` + `'default'` 对桌面端的 `'bypass'` + `'graph'`)之后,无论谁第二个到达都必然撞上 revision 检查。**这是比原来更强的测试**:旧版本可能在从未走过 CAS 路径的情况下通过。
我在本机连跑三次该文件,每次 3/3 通过。值得点出的是:这处改动让断言**名副其实**,而不是放松它 —— 与 deflake 通常的做法相反,这也是它读起来像修复而不是掩盖的原因。
</details>
##########
packages/runtime-host/src/__tests__/runtime-resource-process.test.ts:
##########
@@ -253,12 +253,17 @@ describe('real Host Runtime Resource process lifecycle', {
});
test('treats acquire during PTY integrity failure as a conflict without draining Host', async () => {
- // A DA flood makes the emulator emit protocol replies past the 1MB
- // boundary, which fails the collector and requests termination; the TERM
- // trap keeps the process alive so acquire lands inside the kill grace.
+ // A bounded DA burst makes the emulator emit protocol replies past the
+ // 1MB boundary, which fails the collector and requests termination; the
+ // TERM trap keeps the process alive so acquire lands inside the kill
+ // grace. The command runs under /bin/sh (dash on CI), where {1..N} brace
+ // expansion is literal and emits one query total; seq stays portable.
+ // The burst must stay under the parser's 1MB pending budget — larger
+ // emissions get evicted before parsing and the reply total goes back to
+ // a throughput race.
const background = await coordinator.runBackgroundBash({
...bashInput(
- 'trap "" TERM; while :; do printf "\\033[c%.0s" {1..200}; done',
+ 'trap "" TERM; printf "\\033[c%.0s" $(seq 240000); while :; do sleep 1; done',
Review Comment:
[P1] The test this PR deflakes still times out with the original error — on this PR's own CI run, and on my machine at the same rate as the base.
Run 35311226409 at `9770ea4419347a7ae6b5b3fbb0e8f8f1b53524ce`, step `Run Runtime Host tests`, sole failure:
```
✖ treats acquire during PTY integrity failure as a conflict without draining Host (60010.792183ms)
Error: Timed out waiting for PTY control to close
```
That is the same test and the same message the Summary opens with.
I checked the diagnosis first, because a wrong fix built on a right diagnosis is a different conversation from a wrong diagnosis. **The diagnosis is right, and I confirmed every number in it independently:**
```
$ /bin/dash -c 'printf "\033[c%.0s" {1..200}' | wc -c → 3
$ /bin/dash -c 'printf "[%s]\n" {1..200}' → [{1..200}]
$ /bin/dash -c 'printf "\033[c%.0s" $(seq 240000)' | wc -c → 720000
```
`PTY_PROTOCOL_REPLY_MAX_BYTES` and `PTY_PARSER_HIGH_WATER_BYTES` are both `1024 * 1024`, so the window you picked is real: 240000 queries emit 720 KB of input, comfortably under the eviction budget, while ~8-byte DA replies total ~1.9 MB, comfortably over the reply limit. The burst is portable and bounded, and it is a genuine improvement over a `while :` loop whose output depended on shell dialect.
The gap is between that and determinism of the test. Your end-to-end check — "collector fails with `PTY protocol replies exceeded the 1048576-byte process limit` in 84ms" — establishes that the mechanism fires in isolation. It does not establish that the test reaches that state within its 60-second budget under `--test-concurrency=4` on a loaded runner, and that is precisely where the throughput race you identified lived.
My own runs say the rate has not moved. On macOS at this head: 2 failures in 11 runs, same message. Reverting only `runtime-resource-process.test.ts` to the base and rebuilding: 1 failure in 6. Different platform from CI and small samples, so I would not publish those numbers as a rate — I report them because they point the same way as your CI run rather than against it.
One run is not proof that the fix is ineffective; flaky tests fail intermittently by definition. But the claim here is that replies now cross the boundary *deterministically*, and a deterministic mechanism that still times out means either the determinism does not survive load, or the timeout is being spent somewhere other than reaching the boundary. Worth finding out which before this lands, since a deflake that does not deflake costs more than the flake — it removes the signal that something is still wrong.
The two-client change in this PR is a separate matter and looks sound; I have written that up under it.
<details><summary>简体中文</summary>
本 PR 要 deflake 的那条测试,**在本 PR 自己的 CI 上仍然以原样的报错超时**,在我机器上的失败率也与 base 没有区别。
`9770ea441` 的 run 35311226409,`Run Runtime Host tests` 步骤,**唯一失败**就是 `treats acquire during PTY integrity failure as a conflict without draining Host`,报 `Timed out waiting for PTY control to close`,60 秒 —— 与 Summary 开头写的是同一条测试、同一句报错。
**我先核了诊断**,因为"诊断对但修法不对"和"诊断就错了"是两件事。**诊断是对的,而且里面每个数字我都独立验过**(用真实 dash:`{1..200}` 得 3 字节、字面量就是 `{1..200}`;`$(seq 240000)` 得 720000 字节)。`PTY_PROTOCOL_REPLY_MAX_BYTES` 与 `PTY_PARSER_HIGH_WATER_BYTES` 都是 `1024*1024`,所以你选的窗口是真实存在的:720 KB 输入稳在驱逐预算之下,而约 8 字节的 DA 回复合计约 1.9 MB 稳在回复上限之上。这个 burst 可移植、有界,相比依赖 shell 方言的 `while :` 循环是实打实的改进。
**差距在于这一点与"测试确定性"之间。** 你的端到端核验("collector 在 84ms 内以 `exceeded the 1048576-byte process limit` 失败")证明的是**机制在隔离条件下会触发**;它没有证明**在 `--test-concurrency=4`、负载中的 runner 上,测试能在 60 秒预算内走到那个状态** —— 而这恰恰是你自己指出的吞吐竞争所在的位置。
我自己的数据说失败率没有变化:本 head 的 macOS 上 11 次跑挂 2 次,报错相同;只把 `runtime-resource-process.test.ts` 还原到 base 再重建,6 次挂 1 次。平台与 CI 不同、样本也小,所以我不会把这两个数字当作"失败率"发布 —— 我报告它们,是因为它们与你 CI 那次的指向一致,而不是相反。
**一次 CI 红不能证明修法无效**,间歇性失败本来就间歇。但这里的主张是回复"确定性地"越过边界;一个确定性的机制仍然超时,说明要么确定性在负载下不成立,要么那 60 秒花在了别的地方而不是走到边界。**落地之前值得先弄清是哪一种** —— 一个没能 deflake 的 deflake,代价比 flake 本身更大:它拿走了"这里还有问题"这个信号。
本 PR 里 two-client 那处改动是另一回事,看起来是站得住的,我写在它下面。
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org