Skip to content

test(ci): qualify lease inspection on public Node - #4857

Merged
huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/qualify-task-lease-inspection-public-node
Sep 21, 2026
Merged

huangruiteng merged 1 commit into
loopx-project:mainfrom
Duang777:codex/qualify-task-lease-inspection-public-node

Conversation

@Duang777

Copy link
Copy Markdown
Contributor

Summary

  • Skip only the SQLite variants in the mixed task-lease inspection suite when the current Node/SQLite pair is not qualified.
  • Keep the legacy and File authority variants running on the public minimum Node, while qualified runtimes continue to exercise SQLite.

Root cause

task_lease_inspection.test.ts was added to the public-minimum suite with File, SQLite, and PostgreSQL variants. Node 22.18.0 embeds SQLite 3.50.2, which the authority boundary correctly rejects because it lacks the required WAL-reset fix. The test did not apply the runtime-qualified SQLite skip already used by adjacent mixed-provider suites, so node-minimum-compatibility failed independently of the PR under test. This was observed on #4855.

Validation

  • Before the fix on Node 22.18.0: 8 passed, 2 failed, 2 skipped.
  • After the fix on Node 22.18.0: 8 passed, 0 failed, 4 skipped.
  • On qualified Node 24.16.0: 10 passed, 0 failed, 2 PostgreSQL-only skips.
  • Exact public-minimum CI command: 2,038 tests, 2,018 passed, 20 skipped, 0 failed.
  • npm run typecheck:control-plane passed on Node 22.18.0.
  • loopx canary premerge --from-git-diff: 4/4 passed, 0 manual holds.
  • git diff --check passed.

This changes test qualification only; production authority admission remains fail-closed.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Contributor Author

Exact-head status for 1acf90d895f42e50a1d352a4dda9fa5e9bb589b0: the Node minimum and forward compatibility checks pass. The sole completed failure, test-shard (2), reproduces unchanged on current main@59e11118f because a project-lifecycle test fixture lacks the settlement progress contract added by #4852. #4859 contains the isolated fixture repair; no #4857 code change is needed.

@Job28703

Job28703 commented Sep 21, 2026

Copy link
Copy Markdown

Independent review of exact head 1acf90d895f42e50a1d352a4dda9fa5e9bb589b0 (adversarial review chain, reviewer PASS on re-run):

Verdict: APPROVE.

Root-cause fix done right: the skip condition reuses the production qualification probe (sqliteRuntimeIdentity().sqlite_authority_qualified, the same function backing the admission rejection at sqlite_runtime.ts:102) instead of inventing a second version comparison in the test. No production path is touched (+4/−1, single test file); legacy/file/postgresql variants keep running on the public minimum; qualified runtimes keep full SQLite coverage.

Direct effect verified at this head: node-minimum-compatibility passes with # fail 0, both sqlite mixed-head cases explicitly # SKIP — converting a false failure into an honest skip. This also closes the node-min attribution seen on #4808/#4855/#4856.

Reviewer-chain note (transparency): the first reviewer pass at this head timed out at 900s and emitted only a bare verdict line; it was discarded and a full re-run completed — PASS with minor editorial suggestions (line-number off-by-one in the review prose, immaterial to the verdict).

@huangruiteng huangruiteng 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.

动机

这个测试修复解决的是一条真实的 CI 信号问题:公开最低 Node 运行时没有满足 SQLite authority 的 WAL-reset 资格时,生产路径会 fail closed,但原测试仍强行执行 SQLite provider,于是把“不受支持”误报成产品回归。目标应当是只跳过不具备资格的 SQLite 行,而不是弱化整个 mixed-provider 测试。

改动思路

实现复用生产侧已有的 sqliteRuntimeIdentity().sqlite_authority_qualified,没有再维护 Node/SQLite 版本表,也没有复制第二份资格规则。skip 条件同时绑定 provider === "sqlite",所以 legacy/file 行继续运行;合格运行时上的 SQLite 行也继续运行。该模式与 goal_acceptance_authority.test.ts 的既有做法一致。

具体改动

关键内容讲解

  • 从既有 runtime qualification owner 导入 sqliteRuntimeIdentity,在测试注册前计算一次 sqliteQualified
  • 将原先只处理 PostgreSQL 环境缺失的 skip 条件扩展为两个独立条件:无 PostgreSQL URL 时跳过 PostgreSQL;SQLite 未资格化时只跳过 SQLite。
  • 不触碰 inspectTaskLease、authority store 或生产 admission 逻辑;4 additions / 1 deletion 全在单个测试文件内。

对主干的风险

最强反例是“为了让最低版本变绿而把合格运行时或其它 provider 的覆盖一起跳掉”。本地 Node 24.21.0 精确 head 验证实际执行了 SQLite:10 项通过,仅 2 个无连接的 PostgreSQL 行跳过;typecheck:control-planegit diff --check 通过。把当前 origin/main 以 no-commit 方式合入后,相同测试与 typecheck 也继续通过。远端 node-minimum-compatibilitynode-forward-compatibility 和 real-server PostgreSQL job 均通过,说明资格化/非资格化与真实 PostgreSQL 边界都有覆盖。

当前 blocker 仍是最终 CI:精确 head 1acf90d895f42e50a1d352a4dda9fa5e9bb589b0 落后主干,test-shard (2)pytestmerge-gate 失败。失败日志显示 shard 2 是 project-lifecycle fixture 的 SimpleNamespace.progress 缺失,不由本 PR 的测试文件触发;但 capability 要求 final CI,不能用“与本 diff 无关”代替当前 head 的绿色收据。最小修复是 rebase 当前 main 后重跑全部 required checks。

我的整体评价

这是一份正确、窄且可回滚的 test-only 修复:它复用现有资格事实,保留其它 provider 与合格 SQLite 的真实覆盖,没有引入生产语义或重复 smoke。代码层面没有额外 blocker;由于当前 exact head 没有成功的最终 CI,本轮结论仍为请求修改,待 rebase 后按新 head 复审。

English verdict: REQUEST_CHANGES - head 1acf90d implements the correct provider-specific qualification and passes focused, forward-runtime, PostgreSQL, and current-main integration checks, but it is behind main and lacks the required successful final CI; rebase and rerun all required checks.

@huangruiteng
huangruiteng merged commit 81ec1c9 into loopx-project:main Sep 21, 2026
20 of 23 checks passed
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.

3 participants