Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
English
Thank you for your first contribution to Maka, and for taking on this timing-sensitive CI flake. The change is focused and preserves the existing behavioral assertion. I found one ordering detail that should be tightened before merging:
Important: wait on the composer's pending state directly.
The steering acknowledgement does not strictly prove that
sendCurrent()has finished. The acknowledgement arrives through the session-event stream, whilesendPendingRefis released only afteripcRenderer.invoke('sessions:steer')resolves. Those are independent asynchronous delivery paths, so the acknowledgement may become visible whilesendPendingRef.currentis stilltrue.In that ordering, the test can still continue into
/side, and the final Enter can still be ignored by the overlapping-submit guard—the same failure this PR intends to eliminate.Please keep this assertion to verify that the message was routed as steering, then wait for the composer-owned pending signal before starting the
/sideflow. For example:aria-busyis driven directly bysendPending, so its removal establishes the completion boundary the test actually needs.简体中文
感谢你第一次为 Maka 贡献代码,也感谢你主动处理这个对时序敏感的 CI flaky test。这个改动很聚焦,并且保留了原有的行为断言。不过在合并前,还有一个异步顺序问题需要收紧:
重要:请直接等待 Composer 的 pending 状态结束。
Steering acknowledgement 并不能严格证明
sendCurrent()已经完成。Acknowledgement 通过 session event 流到达,而sendPendingRef只有在ipcRenderer.invoke('sessions:steer')返回后才会释放。这是两条独立的异步传递路径,因此 acknowledgement 可能已经渲染,但sendPendingRef.current仍然是true。在这种时序下,测试仍可能继续进入
/side流程,最终的 Enter 依然会被重叠提交保护逻辑忽略,也就是这个 PR 想解决的同一种失败。建议保留当前断言,用于验证消息确实被路由为 steering;然后在开始
/side流程前,等待 Composer 自己的 pending 信号结束。例如:aria-busy直接由sendPending驱动,因此它被移除才是该测试真正需要的完成边界。There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in efe541b. The test now keeps the steering acknowledgement assertion for routing, then waits for the same Insert message button to clear aria-busy before starting the /side flow. This makes the completion fence follow the composer-owned sendPending state, as requested. Verified with the focused Electron E2E repeated 5 times (5/5 passed), the complete slash-command-menu spec (4/4 passed), the Desktop typecheck, Biome, and git diff --check.