Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions integration-tests/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ export default defineConfig({
maxForks: 4,
},
},
// The worker->main `onTaskUpdate` RPC runs on a 60s budget; under the
// resource pressure of the macOS E2E lane a stall longer than that
// surfaces as an unhandled error and exits an all-green run red (the
// same failure class the core, cli, and scripts suites hit on these
// lanes). Test failures still fail the run; only unhandled errors stop
// being fatal, and only off Linux — the ubuntu shards and Linux local
// runs keep the unhandled-error signal.
dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux',
Comment on lines +43 to +44

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Accepted as evidence-backed — but left unresolved because the remaining remediation is outside this round's reach.

Verified locally the semantic claim this finding rests on (vitest 3.2.7 probes in a scratch project): a failing test with dangerouslyIgnoreUnhandledErrors: true still exits 1 (test failures stay fatal); an unhandled rejection with the flag on exits 0; the same rejection with the flag off exits 1. Combined with the log sweep reported in this thread — 401 API-key test failures on all 8 shards, zero onTaskUpdate signatures, four prior runs green — the exemption this PR adds cannot change run 32942410668's outcome, so merging with Fixes #10097 would auto-close the issue with the wrong root cause on record.

What this round cannot do: removing the Fixes #10097 linkage and correcting the PR title/description are PR-metadata edits; the address-review pass has no GitHub credentials and no PR-body output channel.

Maintainer decision needed: (a) drop the Fixes #10097 linkage and reframe the PR as standalone hardening — integration-tests was the only vitest project without this exemption (core, cli, and scripts all carry it) — leaving #10097 open for its real cause, the model endpoint rejecting the CI API key (transient, or a secret needing rotation), which no code change on this branch can address; or (b) close this PR. Recommendation: (a). Leaving this thread open until that decision is made.

中文说明

认可该发现,证据充分——但保持线程未解决,因为剩余的补救动作超出本轮的能力范围。

该发现所依赖的语义判断已在本地验证(vitest 3.2.7,临时项目探针):在 dangerouslyIgnoreUnhandledErrors: true 下,失败的测试依旧以退出码 1 结束(测试失败仍然致命);未处理的 rejection 在开启该标志时退出码为 0;同样的 rejection 在关闭该标志时退出码为 1。结合本线程中报告的日志扫描结果——全部 8 个分片都是 401 API key 导致的测试失败、零个 onTaskUpdate 超时签名、此前 4 次运行全绿——本 PR 添加的豁免不可能改变 run 32942410668 的结果,因此带着 Fixes #10097 合并会在记录错误根因的情况下自动关闭该 issue。

本轮无法完成的部分:移除 Fixes #10097 关联、更正 PR 标题/描述属于 PR 元数据编辑;address-review 流程没有 GitHub 凭据,也没有修改 PR body 的输出通道。

需要维护者决策:(a) 移除 Fixes #10097 关联,将本 PR 重新定位为独立加固——integration-tests 是唯一缺少该豁免的 vitest 项目(core、cli、scripts 都已带有)——让 #10097 保持打开以追踪其真实原因,即模型端点拒绝了 CI API key(瞬时故障,或需要轮换的 secret),本分支上的任何代码改动都无法解决该问题;或 (b) 关闭本 PR。建议:(a)。在做出该决定之前,本线程保持打开。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Maintainer decision received: option (a) — keep the change, fix the framing. This thread stays open until the PR-body edit lands.

@wenshao verified the change in real macOS + Linux environments and confirms the code is correct and merge-worthy; his remaining asks are all PR-body edits, which this loop cannot perform (no GitHub write access; the workflow owns network writes and address-review has no PR-body channel):

  1. Fixes #10097 → Refs #10097 (EN + 中文 sections) — the tracked run failed on API-key 401s across all 8 shards (six of them Linux, where this flag is inert); Fixes would auto-close the still-live issue whose real cause is the E2E lanes' credential.
  2. Replace the stale verify command with npx vitest run --config ./scripts/tests/vitest.config.ts integration-vitest-config (2 tests) — re-verified at HEAD: the printed command exits 1 ("No test files found"), the corrected form is green.
  3. Qualify "the only vitest project without it" to the suites the failing E2E workflow runs — 20 other workspace configs also lack the exemption (verified by enumeration at HEAD).

Ready-to-apply replacement text is in this round's report comment. The finding remains live until the edit lands: merging with Fixes #10097 still auto-closes #10097 with the wrong root cause on record.

中文说明

已收到维护者决策:方案 (a) —— 保留改动,修正表述。在 PR 正文编辑完成之前,本线程保持打开。

@wenshao 已在真实 macOS + Linux 环境中验证该改动,确认代码正确、可合入;其余诉求均为 PR 正文编辑,本循环无法执行(无 GitHub 写权限;工作流负责所有网络写入,且 address-review 没有修改 PR 正文的输出通道):

  1. Fixes #10097 → Refs #10097(英文与中文部分)—— 被追踪的那次运行是因 API key 401 而在全部 8 个分片上失败(其中 6 个是 Linux 分片,本标志在其上不起作用);Fixes 会自动关闭这个仍然存活的 issue,而其真实原因是 E2E 通道的凭据问题。
  2. 将过期的验证命令替换为 npx vitest run --config ./scripts/tests/vitest.config.ts integration-vitest-config(2 个测试)—— 已在 HEAD 上重新验证:正文中的命令以退出码 1 结束("No test files found"),纠正后的命令为绿。
  3. 将"唯一没有该豁免的 vitest 项目"限定为出问题的 E2E 工作流所运行的套件 —— 另有 20 个 workspace 配置同样缺少该豁免(已在 HEAD 上逐一枚举验证)。

可直接套用的替换文本见本轮的总结评论。在该编辑完成之前,本发现仍然成立:带着 Fixes #10097 合并仍会以错误的根因记录自动关闭 #10097。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed — and escalated to maintainers; this thread stays open for their decision.

The claim verifies from the code itself: dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux' evaluates to false on all Linux shards (six of the eight E2E shards), and the flag only reclassifies unhandled errors — it never suppresses real test failures. Your log sweep of run 32942410668 shows unhandled=0 across all 8 jobs with real test failures, so this PR cannot change that run's outcome; merging with Fixes #10097 would auto-close the issue while its real root cause (the rejected CI API key) goes untracked.

The remedy is either a PR-body edit or closing the PR — both outside this bot's authority in a review round (no GitHub write access; the workflow owns network writes, and choosing between the two directions is a maintainer call):

  • (a) Drop the Fixes #10097 linkage and reframe this PR as standalone hardening — integration-tests was the only vitest project without the house exemption — leaving Main CI failed: E2E Tests on 53b0e4b57b4f #10097 open for its credential root cause. The same edit should refresh the stale "How to verify" command to npm run test:scripts, where the consolidated witness now runs.
  • (b) Close this PR.

Recommendation: (a) — the code mirrors the established exemption in packages/core, packages/cli, and scripts/tests, and the consolidated witness (scripts/tests/integration-vitest-config.test.ts, 2 tests passing) pins it; only the issue linkage misrepresents the change. No further code changes will be made until a maintainer chooses.

中文说明

已确认——并已升级给维护者;该线程保持打开,等待维护者决策。

该结论可直接从代码验证:dangerouslyIgnoreUnhandledErrors: process.platform !== 'linux' 在所有 Linux 分片(8 个 E2E 分片中的 6 个)上取值为 false,且该标志仅改变未处理错误的致命性——从不抑制真实测试失败。你对 run 32942410668 的日志扫描显示全部 8 个作业 unhandled=0 且存在真实测试失败,因此本 PR 无法改变该次运行的结果;若带着 Fixes #10097 合并,该 issue 会被自动关闭,而其真正的根因(被拒绝的 CI API key)将无人追踪。

补救措施要么是编辑 PR 正文,要么是关闭 PR——两者都超出本机器人在审查轮次中的权限(无 GitHub 写权限;工作流负责所有网络写入,且两个方向之间的选择属于维护者决策):

  • (a) 移除 Fixes #10097 关联,将本 PR 重新定位为独立加固——integration-tests 是唯一缺少该豁免的 vitest 项目——让 Main CI failed: E2E Tests on 53b0e4b57b4f #10097 保持打开以追踪其凭据根因。同一次编辑还应把过期的"How to verify"命令更新为 npm run test:scripts,整合后的见证测试现由该命令运行。
  • (b) 关闭本 PR。

建议:(a)——代码与 packages/core、packages/cli、scripts/tests 中既有的豁免完全一致,且整合后的见证测试(scripts/tests/integration-vitest-config.test.ts,2 个测试通过)已将其钉住;只有 issue 关联错误地描述了该改动。在维护者做出选择之前,不会再做任何代码变更。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Disposition: escalated for a maintainer decision — thread left open. The re-verification holds against everything checkable from this checkout: the flag is gated by process.platform !== 'linux', so the six Linux shards of the tracked run are structurally unaffected by this change; the exemption mirrors the one already carried by packages/core, packages/cli, and scripts/tests (integration-tests was the only vitest project without it); and the deleted unhandled-error-policy test the body's How-to-verify command still names does not exist anywhere at HEAD — the consolidated pin is scripts/tests/integration-vitest-config.test.ts via npm run test:scripts, green at HEAD (2/2). The CI-log sweep and 401 counts cannot be re-derived here (this workflow carries no GitHub credentials) and are taken from the review's recorded witness. The requested remedy is not mine to apply: dropping the Fixes #10097 linkage or closing the PR both mutate PR metadata/fate, both are scope decisions about what this PR is and what happens to #10097, and both are GitHub writes this workflow does not perform. Question for the maintainer: (a) edit the PR body to drop Fixes #10097 (EN and 中文 sections), reframing the change as standalone hardening for the documented recurring off-Linux onTaskUpdate RPC-timeout flake class, leaving #10097 open for its credential root cause, and in the same edit replace the stale How-to-verify command with npm run test:scripts (scripts/tests/integration-vitest-config.test.ts) — or (b) close this PR? Recommendation: (a) — the exemption closes a real gap and the witness keeps test failures fatal; only the issue linkage is wrong for the tracked run.

中文说明

处置:已升级等待维护者决策——线程保持打开。 在本地可核查的范围内,复核结论成立:该标志由 process.platform !== 'linux' 门控,因此被追踪运行中的 6 个 Linux 分片在结构上不受本改动影响;此豁免与 packages/core、packages/cli、scripts/tests 中已有的豁免一致(integration-tests 是唯一缺少它的 vitest 项目);正文"如何验证"命令仍引用的已删除 unhandled-error-policy 测试在 HEAD 中任何位置都不存在——整合后的钉住测试是 scripts/tests/integration-vitest-config.test.ts,通过 npm run test:scripts 运行,在 HEAD 上为绿(2/2)。CI 日志扫描与 401 计数无法在此重新获取(本工作流不持有 GitHub 凭据),采用审查中记录的见证数据。所要求的补救措施不在我的权限内:移除 Fixes #10097 关联或关闭本 PR 都属于 PR 元数据/PR 命运的变更,都是关于本 PR 定位与 #10097 处置的范围决策,且都是本工作流不执行的 GitHub 写操作。给维护者的问题:(a)编辑 PR 正文,移除 Fixes #10097(英文与中文部分),将本改动重新定位为针对已记录的、非 Linux 通道反复出现的 onTaskUpdate RPC 超时抖动类别的独立加固,让 #10097 保持打开以追踪其凭据根因,并在同一次编辑中将过期的"如何验证"命令替换为 npm run test:scripts(scripts/tests/integration-vitest-config.test.ts)——还是 (b)关闭本 PR?建议选 (a):该豁免填补了真实缺口,且见证测试保证测试失败依旧致命;只是 issue 关联对于被追踪的那次运行是错误的。

},
resolve: {
alias: {
Expand Down
8 changes: 8 additions & 0 deletions scripts/tests/integration-vitest-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,12 @@ describe('integration Vitest config', () => {
});
expect(integrationConfig.test?.poolOptions?.threads).toBeUndefined();
});

it('keeps unhandled errors fatal only on Linux', () => {
// toBe, not toBeFalsy: a deleted flag is `undefined` and must fail
// this pin on every platform, including Linux where the value is false.
expect(integrationConfig.test?.dangerouslyIgnoreUnhandledErrors).toBe(
process.platform !== 'linux',
);
});
});
Loading