Repository navigation
fix(ci): write back only the MariaDB lane's Maven delta and size its ceiling (#15000) - #15064
Conversation
…ng (#15000) Run 39563394993's mysql-integration job was cancelled at the 200-minute ceiling: 54 minutes queued for the host lock, 75 minutes of Maven work, then a 60-minute unbounded tar of the host-shared ~/.m2. The cache is an optimization, so skip it after five minutes; the ceiling now covers the 90-minute wait, the measured Maven pass, the bounded write-back and setup headroom.
|
Thanks for the PR! Re-run at Template looks good ✓ — all nine headings present, and Evidence carries named run/job identifiers I could actually look up. Problem: observed, and I checked the citation rather than taking it on faith. Job Direction: aligned. Bounding an optimization so it cannot eat a gate's budget is the right shape of fix, and nothing here touches auth, sandbox, model selection, telemetry, or a public contract. One inherited consumer worth keeping in view: Size: not applicable — no core paths, no cross-package change. 19 production lines in the workflow, 14 test lines in the meta-test; far under every threshold. Approach: the scope is right and genuinely minimal — one workflow step plus the one meta-test that pins it, no drive-by edits. The interesting move is the delta predicate: the seed hardlinks host → job, so One alternative I would still put to you, as a question and not a blocker: the write-back could move outside the lock into an Risk: no elevated risk signals — neither changed file matches the high-risk path set. Moving on to code review. 🔍 中文说明感谢贡献！本次是在 模板完整 ✓ —— 九个标题齐全，Evidence 里给的是可查证的 run/job 标识，而不是泛泛描述。 问题： 已观测，而且我核对了引用而非直接采信。run 方向： 对齐。给一个"优化"加上界、让它不能吃掉头门禁的预算，是正确的修法形态；这里也没有触及 auth、sandbox、模型选择、telemetry 或任何公开契约。有一个继承型下游需要留意： 规模： 不适用——未触及核心路径，也不是跨包改动。workflow 里 19 行生产代码，meta-test 里 14 行测试代码，远低于所有阈值。 方案： 范围合理且确实最小——一个 workflow 步骤，加上唯一钉住它的那个 meta-test，没有夹带无关改动。关键的一步是增量判据：播种是把宿主机文件硬链接进 job，因此 仍然想向你提一个问题（不是阻断项）：写回其实可以移到锁外面，放在释放锁之后的 风险： 无升级风险信号——两个改动文件都不匹配高风险路径集合。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI wrote my own proposal before reading the diff, and it converged on the same two moves: publish only the delta, and re-derive the ceiling from measured components rather than from the old number plus a guess. So the review below is about whether the delta argument actually holds, not about whether the direction is right. The delta is complete, not just cheap. This is the claim the whole change rests on, so I checked it independently rather than accepting the PR's framing. Any file in The truncation containment is real. Precedence and exit codes check out. On the Two non-blocking notes, neither worth a round on its own:
I also re-checked the AGENTS.md meta-test obligation, since a workflow edit is exactly where it gets missed. Six files under CI test evidenceThis run is on the CI path ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Read from the 172 check-runs on The green that matters is
The gap that CI cannot close: the changed lane was skipped, because the tier selector chose fast. Nothing on this PR has observed the new write-back executing. The meta-tests pin the predicate's text, and a string assertion passes identically whether or not Sandboxed verification would settle this: 中文说明代码审查我在读 diff 之前先写了自己的方案，结论收敛到同样两步：只发布增量，以及用实测分量重新推导上限、而不是在旧数字上加个猜测。所以下面的审查关注的是"增量这个论证到底成不成立"，而不是方向对不对。 增量是完整的，不只是省时的。 整个改动都压在这条论证上，所以我独立核过，没有直接采信 PR 的说法。 截断隔离是真的。 优先级与退出码都成立。
两条非阻断的意见，都不值得单独走一轮：
我也重新核了 AGENTS.md 的 meta-test 义务，因为改工作流正是最容易漏这一步的地方。 CI 测试证据本次运行在 CI 路径上（ （CI 表格见上方标记区域。） 读自 真正要紧的绿色是
CI 关不上的缺口： 被改动的 lane 是 skipped，因为分层选择器选了 fast。这个 PR 上没有任何东西观察到新的写回真正执行过。meta-test 钉的是判据的文本，而字符串断言无论 沙箱验证可以把这件事定下来： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — the previous round's one real reservation is closed by a mechanism stronger than the fix I asked for, the reasoning behind the delta checks out independently, and the meta-tests are green on the reviewed commit; the missing fifth is that the changed lane was skipped, so the write-back has never been observed running. Stepping back. Last round I deferred rather than approved, for one substantive reason: bounding the producer I want to be explicit about what convinced me, because "the author says it's atomic" is not a reason to approve. The load-bearing claim is that The On the evidence. The gap from last round is genuinely closed on the side I can see:
Verdict: approve, pinned to One housekeeping item, which resolved itself as I posted rather than staying outstanding: the 中文说明Confidence: 4/5 —— 上一轮唯一实质性的顾虑，被一个比我要求的修法更强的机制关闭了；增量背后的论证独立核过是成立的，meta-test 也在被审查的 commit 上是绿的。少的那一分是：被改动的 lane 被跳过了，所以这个写回从未被观察到真正运行过。 退一步看。上一轮我选择保留意见而不是批准，实质原因只有一个：把生产者 我想说清楚是什么说服了我，因为"作者说它是原子的"不构成批准的理由。承重的论点是
关于证据。我能看见的那一侧，上一轮的缺口确实关上了：
结论：批准，钉在 有一件收尾事项，它在我发布批准时自行解决了，而不是仍然悬着：本 bot 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Resolve .github/workflows/sdk-java.yml: the two sides are additive. Keep this branch's raised mysql-integration ceiling (timeout-minutes: 280) and its bounded cache write-back, and take main's #14936 replacement of the mariadb service container with the job-managed MySQL 8.4.6 container (env MYSQL_CONTAINER, "Start MySQL without native AIO", always() removal). Verified: npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/sdk-java-workflow.test.js scripts/tests/workflow-size.test.js -> 332 passed, 1 skipped. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmvi84hl9nv
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — SDK Java (fast) / Runtime Broker and Managed Agent MariaDB / Java 21 was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 3a": none — all checks above ran to completion (~11 of 34 tool calls used)..
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/sdk-java.yml — actionlint embedded-shell source mapping is not yet supported.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：build-and-test — SDK Java (fast) / Runtime Broker and Managed Agent MariaDB / Java 21 was skipped in CI and its suite did not run locally.
未探索到全部深度（达到工具调用预算）："agent 3a"：none — all checks above ran to completion (~11 of 34 tool calls used).。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未检查（工具限制，非阻断）：.github/workflows/sdk-java.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
— qwen3.8-max via Qwen Code /review (v0.25.3)
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical-only review at c9dfb7d0. One blocking finding. The timeout placement and the ceiling raise are both correct; the problem is the destination of the bounded write-back. Not approving until the truncation is contained or the trade is explicitly accepted.
Critical — a timed-out write-back leaves a permanently corrupt artifact in the host-shared Maven cache, and this change makes that the expected case rather than a rare one
The code at head. Line 735–736 of .github/workflows/sdk-java.yml:
timeout 300 tar -C "${RUNNER_TEMP}/m2-repo" --exclude=… -cf - . \
| tar -C "${HOME}/.m2/repository" --skip-old-files -xf - \
|| echo "::warning::Maven cache write-back skipped after 300 s or failed; …"timeout 300 binds the producer. When it fires, the consumer sees EOF in the middle of a member it has already created on disk, and it extracts into ${HOME}/.m2/repository — the host-shared tree on the self-hosted pool, not job-local scratch.
I verified the two properties this depends on rather than reasoning about them. Feeding an archive stream that ends mid-member into tar -k -xf - (-k is the --skip-old-files equivalent) and then running a second, complete write-back over the same destination:
f1.bin size=8388608 sha=75e6985b… src=75e6985b… -> MATCH
f2.bin size=8388608 sha=2c438bb2… src=2c438bb2… -> MATCH
f3.bin size=8388608 sha=245e758a… src=de6aa1ea… -> CORRUPT ← the member in flight at EOF
The member that was being written when the stream ended survives in the destination, and the subsequent complete write-back does not repair it, because --skip-old-files skips any path that already exists. The corruption is sticky for the life of that host. On bsdtar it is worse than a short file: the size is the full declared size (the extractor sizes the file from the header), so only the bytes are wrong and a size or mtime sanity check passes. GNU tar on the Linux runner writes sequentially and leaves a genuinely short file instead — different mechanism, same outcome class: one corrupt member in the shared tree that nothing later repairs.
Why it reaches other jobs. The same workflow seeds every Maven lane from that shared tree by hardlink — lines 710, 1368 and 1606 all run cp -aln "${HOME}/.m2/repository" "${RUNNER_TEMP}/m2-repo", and -Dmaven.repo.local=${RUNNER_TEMP}/m2-repo (lines 684, 1365, 1603, 1674) points Maven at the result. A corrupt jar in the shared cache is therefore hardlinked into the local repository of the next job on that host. I did not execute Maven here, so I am not claiming a tested failure mode for this last link; the well-known behaviour is that Maven does not re-verify checksums of artifacts already present in the local repository, so the artifact is used and surfaces as a ZipException / "invalid LOC header" / compile failure in a lane that has nothing to do with this change.
Why this PR changes the risk rather than only moving it. Before this commit, a partial write-back required the job to be cancelled mid-tar at its ceiling — rare, though run 39563394993 did exactly that. After it, the write-back is bounded at 300 s while the PR's own measurement of that same tar is ~60 minutes, a 12× overrun. On a loaded host the timeout firing becomes the normal outcome, and each firing deposits one corrupt member into a shared, never-repaired cache. The step stays green by design, so nothing reports it.
What the Risk & Scope section covers. "A slow write-back now leaves the host cache partially refreshed, so a later job may resolve more artifacts through the proxy" describes missing artifacts, and missing is safe — Maven re-resolves. Corrupt is not safe, and that half is not covered.
The cheap containment. Extract into job-local scratch and publish only on success, so a killed tar leaves its damage in RUNNER_TEMP, which the runner discards:
rm -rf "${RUNNER_TEMP}/m2-stage" && mkdir -p "${RUNNER_TEMP}/m2-stage"
if timeout 300 tar -C "${RUNNER_TEMP}/m2-repo" --exclude=./com/alibaba/qwencode-sdk --exclude=./com/alibaba/qwen-managed-agent-server -cf - . \
| tar -C "${RUNNER_TEMP}/m2-stage" --skip-old-files -xf -; then
cp -aln "${RUNNER_TEMP}/m2-stage/." "${HOME}/.m2/repository/" 2>/dev/null || true
else
echo "::warning::Maven cache write-back skipped after 300 s or failed; the gate result above is unaffected"
fiThe publish stays hardlink-based, so the happy path costs inode entries rather than a second copy of the bytes. Dropping --skip-old-files instead would let a later successful write-back overwrite and repair a truncated member, but that trades away whatever the flag was added to protect, so staging is the better fit. If neither is wanted, the alternative is an explicit decision recorded in the workflow comment that a timed-out write-back may poison the host cache — but that should be a choice, not a side effect.
Parts I checked and found sound. timeout 300 on the producer is the right placement: the consumer sees EOF, exits non-zero, pipefail propagates it, and the || echo "::warning::…" arm keeps the step green with the gate result untouched — and the reworded warning honestly covers both exit paths. timeout(1) is already the established idiom for bounding shell work in this repo's workflows. timeout-minutes: 280 covers the stated arithmetic (90-minute lock wait + ~75 minutes Maven + 5 minutes bounded write-back + ~30 minutes setup = 200, with margin for a loaded host), and hosted lanes inherit it as a bound only. The meta-test was updated in the same commit, as the workflow-edit rule requires: it pins the timeout 300 tar string and the reworded warning, and asserts a floor on the ceiling.
Non-blocking, noted only: the new test asserts toBeGreaterThanOrEqual(240) while the workflow sets 280. A floor matches how this file already guards the job, but 240 sits below the arithmetic the adjacent comment documents, so a future edit down to 241 would keep the test green while the stated budget no longer fits.
CI: Lint & Static, Integration Tests (no-AK), Classify PR, SDK Java (fast) / Real daemon E2E, SDK Java (fast) / Flyway migration version uniqueness and Select the Managed Agent tier pass; Test (ubuntu-latest, Node 22.x) and SDK Java (fast) / ubuntu-latest / Java 21 were still pending at read time. No failure is attributable to this PR. Note that the lane this commit changes is skipped under the fast tier, so no CI check exercises the write-back — the meta-tests pin its text, and they pass identically whether or not the truncation is contained.
Critical-only 评审，基于 c9dfb7d0：发现一个阻塞问题，已提交 Comment，未批准。timeout 加在生产端、上限提到 280 都是对的，但被超时打断的写回会把一个残缺构件留在宿主机共享的 Maven 缓存里；我用实验证明该文件会残留，且因为 --skip-old-files，后续一次完整写回也永远不会修复它。缓存又是用硬链接播种到后续任务的本地仓库，于是故障会出现在无关的任务里。改动前这只在极少见的取消场景下发生，改动后写回上限是 300 秒而实测耗时约 60 分钟，触发会成为常态。建议先解包到任务本地临时目录、成功后再发布进共享缓存。
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
The bounded tar pipeline extracted straight into the host-shared ~/.m2, so a timeout mid-member left a truncated artifact that --skip-old-files never repaired and the next seed hardlinked into its job. Its 300 s cap was also below the measured 26-61 minute whole-tree cost, so it would have tripped on every run. Publish only the files Maven fetched in this job (nlink == 1 after the hardlink seed) with per-file cp -ln, keeping the two in-house excludes. Each link is atomic and never replaces a host file, so the 300 s backstop can only leave artifacts missing. Distinguish the timeout from other failures, and fold the backstop into the derived ceiling floor instead of a second hardcoded one.
yiliang114
changed the title
|
@qqqys Thanks for the containment experiment. df8c4c1 takes a narrower route than staging. The write-back no longer extracts anything into the shared cache. It publishes only the files Maven fetched in this job: the hardlink seed gives every host file nlink ≥ 2, so the delta is nlink == 1. Each one goes in with a no-clobber hardlink ( Probe in GNU coreutils 8.32 / findutils containers (Ubuntu 22.04 and Oracle Linux):
The meta-tests now pin the delta predicate, 中文说明感谢您的复现实验。df8c4c11 没有采用暂存方案，而是收得更窄：写回不再向共享缓存解包任何东西，只发布本次 job 中 Maven 新下载的文件。硬链接播种让宿主机文件 nlink ≥ 2，所以增量就是 nlink == 1 的那些。每个文件都用不覆盖的硬链接发布（ 容器验证（GNU coreutils 8.32 / findutils，Ubuntu 22.04 与 Oracle Linux）：正常增量只发布新文件、druid 照常回收、两个自研坐标被跳过、被 Maven 以 rename 替换的已播种文件在宿主机上保持不变；在 2 万个新文件上强制兜底触发（3 次）：退出码 124，发布约 5000 个，部分写入 0 个，下一次运行补齐其余文件。 meta-test 现在钉住增量判据、 |
|
Scope ledger — round 1 at |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — SDK Java (fast) / Runtime Broker and Managed Agent MySQL / Java 21 was skipped in CI (fast tier) and its suite did not run locally, so the rewritten write-back has never executed in CI; the meta-tests and a fixture smoke of the shell are the only execution evidence.
Not explored to full depth (tool budget reached): "agent 6b": could not execute npx vitest run scripts/tests/sdk-java-workflow.test.js — neither the review worktree nor the main checkout has node_modules , so the change….
Not linted (tool limitation, not a blocker): .github/workflows/sdk-java.yml — actionlint embedded-shell source mapping is not yet supported.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查（原文为英文）：build-and-test — SDK Java (fast) / Runtime Broker and Managed Agent MySQL / Java 21 was skipped in CI (fast tier) and its suite did not run locally, so the rewritten write-back has never executed in CI; the meta-tests and a fixture smoke of the shell are the only execution evidence.
未探索到全部深度（达到工具调用预算）："agent 6b"：could not execute npx vitest run scripts/tests/sdk-java-workflow.test.js — neither the review worktree nor the main checkout has node_modules , so the change…。
未检查（工具限制，非阻断）：.github/workflows/sdk-java.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
— qwen3.8-max via Qwen Code /review (v0.25.3)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 72 passed · 0 failed · 72 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 0 失败 · 72 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #15064 deep verification —
|
| cell | arm | oracle | base | head |
|---|---|---|---|---|
| C1 selectivity | both | final host-cache file set vs expected | 30200 files, 0 missing, 0 extra, exit 0 | 30200 files, 0 missing, 0 extra, exit 0 |
| C1 cost (30000 seeded × 400 B + 200 new) | both | wall ms of the write-back | 895 ms | 463 ms |
| C2a cancel mid-stream | base | truncated files in the shared cache | ≥1 on 4/4 landed cancels | n/a (no unbounded stream) |
C2b backstop fires (60000 delta files, timeout 300→4 s) |
head | step exit, warning text, published count, partial files | n/a | exit 0, "stopped after 300 s" printed, 38333/60000 published, 0 truncated |
| C3 seeded file replaced by rename | both | host content + host inode | HOST-ORIGINAL, inode unchanged |
HOST-ORIGINAL, inode unchanged |
| C4 excludes | both | published set | [druid, fastjson], no in-house |
[druid, fastjson], no in-house |
| C5 seed failed (no hardlinks) | head | published count, exit | n/a | 400/400 (degrades to whole tree), exit 0 |
| C6 job-local repo absent | both | step exit + warning | exit 0, warning | exit 1, no warning |
| C7 residue after a head backstop | head | empty dirs, truncated files | n/a | 33015 dirs, 0 empty, 0 truncated |
| C8 seeded file modified in place | both | host content after | MODIFIED-IN-PLACE |
MODIFIED-IN-PLACE |
| C9 cross-device (host cache on tmpfs) | head | published count, step exit | n/a | 0/5, generic warning, exit 0 |
| C9b same-fs control | head | published count | n/a | 5/5 |
Witness: 01-ab-safety-base-truncates-head-zero-partial.png (C2a vs C2b, live run), 02-ab-equivalence-and-cd-failure-path.png (C3/C4/C6, live run), 04-canonical-ab-ledger-cost-cells.png (C1/C2b/C5/C7 ledger).
C2a's four landed cancels each truncated exactly one member, with the byte deficit recorded: mod39-1.0.jar 849920≠8388608, mod51-1.0.jar 6949888≠8388608, mod51-1.0.jar 7675904≠8388608, mod10-1.0.jar 1992704≠8388608. This is the failure mode the PR exists to remove, and it reproduces on the base build.
C2a determinism note (harness, not PR). Whether a cancel lands, and whether a landed cancel truncates, are both timing properties. My first two attempts at this cell were unreliable: a fixed 2 s kill let base's tar finish 960 MB from page cache in 4 of 5 runs (a green no-op that looked like a pass), and truncation then reproduced on only 1 of 3 landed cancels because a kill landing exactly on a member boundary leaves files missing rather than partial. The final cell halves the kill point until exit 124 is observed, asserts that precondition explicitly, and samples four landed cancels: 4/4 landed, 4/4 after a first write, 4/4 truncated. One full canonical run was discarded and re-run because of this (stability.log, c2a-stability.log); it was a harness defect, and no result in this report comes from a cell whose precondition did not hold.
2.3 Cost scaling — what the mechanism does and does not remove
| fixture | seeded tree | base | head | ratio |
|---|---|---|---|---|
| 30000 files × 400 B | 11 MiB | 1189 ms | 567 ms | 2.1× |
| 4000 files × 256 KiB | 1000 MiB | 752 ms | 78 ms | 9.6× |
| 1500 files × 1 MiB | 1500 MiB | 777 ms | 40 ms | 19.4× |
Read down the head column: 567 → 78 → 40 ms tracks file count (30000 → 4000 → 1500), while the seeded tree grew 11 MiB → 1500 MiB. So head's residual cost is the metadata walk (find must stat every file to find the nlink == 1 delta), and the data copy is gone entirely — which is why the ratio widens as files get larger. On this container's NVMe both arms are file-count-bound, so at equal count the win is ~2×, not orders of magnitude. See F3 and 05-cost-scaling-base-tracks-tree-head-tracks-delta.png; raw data c1-scaling.log.
2.4 Ceiling derivation, recomputed independently
The meta-test asserts timeout-minutes × 60 ≥ seconds + 100×60 + writeBackBound + 30×60, with seconds read from sdk_java_lock_acquire 5400 and writeBackBound from /timeout (\d+) find \. /. Recomputed outside the test:
| ceiling | seconds | floor (5400+6000+300+1800) | satisfied | margin | |
|---|---|---|---|---|---|
| base | 200 | 12000 | 13500 (225 min) | no | −1500 s (−25 min) |
| head | 280 | 16800 | 13500 (225 min) | yes | +3300 s (+55 min) |
Smallest integer ceiling satisfying the floor is 225 min, so 280 is a deliberate margin rather than a tight fit. writeBackBound fails closed: if the regex stops matching, Number(undefined) is NaN, expect(NaN).toBe(300) goes red, and the ≥ comparison with NaN also goes red.
3. Corrections to the PR description
These are corrections to the description, not requests to change code.
- "it exited 124 after publishing ~5 000 of them" — 124 is the value captured into
write_back; the step exits 0 and prints the distinct warning, which is precisely what keeps the gate green. Measured over 60000 delta files with the productiontimeout 300shimmed to 4 s: step exit 0, warning present, 38333 published, 0 partial files. A reader could otherwise take "exited 124" as the step failing. - "the delta publish copies only new files" — it hardlinks only new files; no file content is ever copied. That distinction is the load-bearing half of the safety argument (a
link()is atomic and writes no data, so there is no member to truncate), and it is what C2b demonstrates.
4. Findings
F1 (Low, latent) — the new cd turns a best-effort write-back into a hard failure, contradicting the step's own comment
The step comment states: "the warning arms keep a copy failure from reddening the gate after mvn has already passed." But cd "${RUNNER_TEMP}/m2-repo" executes before any guarded command, and under the runner's bash -e a failed cd aborts the step with no warning at all.
Reproduce (ab-writeback.mjs, cell C6; also 02-ab-equivalence-and-cd-failure-path.png):
ART_DIR=tmp/pr15064-verify-20261021-192443 ONLY=C6 node tmp/ab-writeback.mjs
# RESULT C6 base exit=0 stderr="tar: …/m2-repo: Cannot open: No such file or directory"
# stdout="::warning::Maven cache write-back failed; the gate result above is unaffected"
# RESULT C6 head exit=1 stderr="bash: line 3: cd: …/m2-repo: No such file or directory"
# stdout=""shellcheck 0.11.0 independently flags the new line as SC2164 (Use 'cd ... || exit'). Comparing full step bodies, base carries 1× SC2164 (the pre-existing cd packages/managed-agent-server) and head carries 2× — the PR adds exactly one. This is advisory only, not a gate: the repo runs actionlint with -shellcheck= (scripts/lint.js:202), and its shellcheck gate selects git ls-files entries by shell-script MIME type, which excludes YAML run: blocks.
Reachability — bounded, and low. The cd runs only after both mvn invocations succeeded, and Maven creates -Dmaven.repo.local, so ${RUNNER_TEMP}/m2-repo exists on every path that reaches it. It becomes reachable only if the runner fails to provide a searchable RUNNER_TEMP; note the job's ownership-restore preamble covers $GITHUB_WORKSPACE and not RUNNER_TEMP. I could not construct a production-realistic trigger, so I report this as a latent narrowing of the step's stated best-effort contract, not as a live break.
Measured one-line fix (preserves the commit's intent; no test change needed)
- cd "${RUNNER_TEMP}/m2-repo"
+ cd "${RUNNER_TEMP}/m2-repo" || { echo "::warning::Maven cache write-back skipped; the gate result above is unaffected"; exit 0; }exit 0 is safe here because the write-back is the last thing in the step body. Measured (measure-fix.mjs, 8/8 assertions, fix-measurement.log):
- hostile fixture (repo absent): unfixed exit 1, no warning → fixed exit 0, warning emitted
- benign fixture: published set byte-identical (
["com/example/a/1.0/a-1.0.jar"]) on both - meta-test suite with the fix applied to the real workflow: 96 passed / 96 (
fix-metatest.log) — the existingtoContain('cd "${RUNNER_TEMP}/m2-repo"')andindexOf(...)pins are substrings of the fixed line, so no test edit is required. (Workflow restored afterwards; sha256 verified against pristine.)
F2 (Low) — hardlink publish cannot cross filesystems, and fails silently where base's tar would have worked
cp -l returns EXDEV across mounts. Measured with the host cache on tmpfs and the job repo on overlay (c9-crossdev.log), running the verbatim head block:
cp: cannot create hard link '…/repository/./com/example/m1/1.0/m1-1.0.jar' …: Invalid cross-device link (×5)
::warning::Maven cache write-back failed; the gate result above is unaffected
step exit=0
published into host cache: 0 of 5
Same-filesystem control: 5 of 5. So the failure mode is: gate stays green, one generic warning, and the shared cache silently never grows — every subsequent job re-resolves through the egress proxy, which is exactly the outcome the mkdir -p comment directly above exists to prevent. Base's tar pipeline had no such constraint.
Bounded: not reachable on the current layout (HOME=/home/runner, RUNNER_TEMP=/home/runner/work/_temp, same filesystem), and I could not inspect the ECS pool's mounts from this container. No data loss and no gate failure — a silent performance regression only, and one that would announce itself in the warning text if it ever happened.
F3 (Informational — coverage limit, not a defect) — the mechanism is proven; production's magnitude is not reproducible here
Head still walks the entire seeded tree; what it removes is the data copy (§2.3). My largest fixture (1500 MiB) puts base at 777 ms, whereas the PR reports 26–61 minutes for the same phase in production. That gap is 4–5 orders of magnitude, so the production cache must be far larger than anything I built and/or live on much slower storage than this container's NVMe. I therefore prove the direction and the scaling exponent (head's cost is independent of seeded bytes; base's is not) and not the absolute "seconds vs an hour" claim. The lane itself remains the only runtime check for that number, as the PR's own test plan says.
F4 (Informational, pre-existing) — "never replaces a host file" holds for rename-replace, not for in-place writes
Cell C8: a tool that truncates and rewrites a seeded file modifies the host cache through the shared inode before the write-back ever runs, and both arms end at MODIFIED-IN-PLACE. Cell C3 confirms the PR's claim for the rename case (host content and inode both preserved, identically on both arms). This is a property of the pre-existing cp -aln seed, unchanged by this PR — worth stating only because the step comment's "never replaces a host file" is true of the write-back but not of the seed it depends on.
F5 (Nit) — set -o pipefail is now dead in this step, and its pin was removed in the same change
The only pipeline in the step was the tar | tar write-back this PR deletes; the two remaining |-bearing lines are || operators (cp -aln … || true, timeout 300 find … || write_back=$?). The PR also drops expect(step.run).toContain('set -o pipefail') from the meta-test, so nothing now asserts it either way. Harmless — but it is leftover scaffolding, and removing the line would make the step's shell contract match what it actually needs.
F6 (Nit, benign) — a killed head run can leave an empty directory behind
cp --parents creates parent directories before linking, so a backstop landing between the two would leave an empty directory in the host cache. Not observed: three killed-backstop runs measured 0 empty directories (33015 dirs canonical, 33015 and 25661 in two repeats — c7-repeat.log), with 0 truncated files in all three. The mechanism admits it and it is benign either way: Maven re-resolves a directory with no artifact, and that is strictly better than base's truncated file.
5. Mutation matrix — the meta-tests are not vacuous
Baseline (unmutated) run through the identical runner, environment and test selection: 96 tests, 0 failed. Every mutant anchor matched exactly once (13/13); every restore was sha256-verified against the pristine file (13/13), and the tree is clean at round end. 13/13 killed, 0 survivors. Witness: 03-mutation-matrix-13-of-13-killed.png; raw mutation-ledger.txt, mutation-matrix.json.
| # | mutation to .github/workflows/sdk-java.yml |
result | red test |
|---|---|---|---|
| M0 | positive control: reword the timeout warning | KILLED (1 red) | installs and verifies against one job-local Maven repository |
| M1 | drop -links 1 (delta predicate) |
KILLED (1) | same |
| M2 | drop timeout 300 (backstop) |
KILLED (2) | + holds the per-host sdk-java lock once around both Maven runs |
| M3 | cp -ln → cp -l (drop no-clobber) |
KILLED (1) | installs and verifies… |
| M4 | drop the -eq 124 branch |
KILLED (1) | installs and verifies… |
| M5 | timeout-minutes: 280 → 200 |
KILLED (1) | holds the per-host sdk-java lock… |
| M6 | drop the qwencode-sdk exclude path |
KILLED (1) | installs and verifies… |
| M7 | drop --parents |
KILLED (1) | installs and verifies… |
| M8 | invert predicate: -links 1 → -links 2 |
KILLED (1) | installs and verifies… |
| M9 | drop mkdir -p "${HOME}/.m2/repository" |
KILLED (1) | installs and verifies… |
| M10 | drop cd "${RUNNER_TEMP}/m2-repo" |
KILLED (1) | installs and verifies… |
| M11 | re-add a tar … --skip-old-files -xf - line |
KILLED (1) | installs and verifies… |
| M12 | backstop 300 → 600, ceiling unchanged |
KILLED (2) | both tests above |
M0 is the positive control the matrix needs: it lands in the same file as every other mutant and turns exactly one test red, so "the suite does not cover this" and "my runner never collected the file" are distinguishable. M5 and M12 both redden the lock/ceiling test, confirming the derived floor in §2.4 is live rather than decorative. No survivors to adjudicate, so no classification (coverage gap / dead code / redundant defence / real defect) is required.
The mutation runs in reverse too: the F1 fix in §4 was applied to a scratch copy and the suite stayed green at 96/96 — i.e. nothing pins whether the write-back is best-effort when the job-local repo is absent. The fixture that would go red is the C6 cell (absent ${RUNNER_TEMP}/m2-repo), which is a shell-level fixture, not something this string-matching meta-test can express.
6. Targeted gates
| gate | command | result |
|---|---|---|
| Named in the PR test plan | npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/sdk-java-workflow.test.js scripts/tests/workflow-size.test.js |
2 files passed; 331 passed, 1 skipped (332) |
| Every other meta-test parsing this workflow (AGENTS.md rule 3) | same runner over event-transport-multi-node-ci, main-ci-failure-issue-workflow, managed-agent-server-e2e, managed-agent-gate-workflow, spotbugs-gate, qwen-triage-workflow, event-transport-broker-ci, hosted-process-ci |
8 files passed; 451 passed |
| actionlint (repo's own flags) | node scripts/lint.js --actionlint |
exit 0 |
| actionlint positive control | dangling needs: planted in a scratch copy, same flags |
exit 1, job "mysql-integration" needs job "job-that-does-not-exist" which does not exist → gate is live |
bash -n |
4 extracted blocks (base/head × write-back/full step) | all syntax OK |
| shellcheck positive control | planted unused variable | exit 1, SC2034 reported → binary is live |
| workflow size | wc -c vs WORKFLOW_SIZE_GATE_BYTES |
113621 B vs 470000 B gate |
Nine meta-test files parse sdk-java.yml; all nine were run (AGENTS.md: "update every scripts/tests/*.test.js that parses the changed file"). None needed updating — the PR updated its own.
The 1 skipped test in workflow-size.test.js is pre-existing and unrelated to this diff.
7. Not covered
- No real MariaDB/MySQL lane run. The lane itself is the runtime check for the pool, as the PR states. No Java toolchain was needed — the diff touches no Java — and
QWEN_VERIFY_JAVAis unset in this container. - The shell replay is uncalibrated. This is a first round with no GitHub token and no
previous-report.md, so no real artifact the production step emitted (a write-back step log, a posted warning line) was retrievable to check the replay against. My harness reproduces the write-back's shape — realfind/cp/tar, real hardlinks, real filesystems, the step's own shell contract — but was never validated against a production step log. What would calibrate it: one job log from the pool showing the write-back step's output and duration. - Per-commit attribution out of reach. Depth-2 shallow checkout:
git rev-list HEAD^1..HEAD^2returns 1 commit while the snapshot lists 3 (7849600,c9dfb7dmerge,df8c4c1). I verified the aggregateHEAD^1..HEADdiff. The intermediate bounded-tarvariant introduced by7849600and superseded bydf8c4c1was never executed, and the merge resolution inc9dfb7d(which took main's job-managed MySQL container) was not separately probed. - yamllint not run.
node scripts/lint.js --setupfailed atpip3: Permission denied. actionlint and shellcheck installed and ran; the repo's--yamllintgate is therefore unmeasured here. - Toolchain versions. Measured on GNU coreutils 9.1, findutils 4.9.0, tar 1.34, bash 5.2.15 (Debian bookworm). The author reports coreutils 8.32 on Ubuntu 22.04 / Oracle Linux.
cp -ln --parents -t,find -links, andtimeout's process-group signalling were not exercised on 8.32, and I cannot confirm the ECS pool's versions from this container. Note coreutils 9.2 changed some-nsemantics upstream; 9.1 here behaved as the PR expects. - Base-arm meta-test gate not run. That would need
node_modulesin the base worktree. Attribution instead rests on extracting base's workflow directly and reading its ceiling (200) and write-back (313-bytetarpipeline) from the YAML. - No concurrent-job race. The per-host
sdk_java_lock_acquire 5400serialises the write-back, so I did not construct two jobs publishing into one host cache simultaneously. - Hosted-runner (
ubuntu-latest) fallback for this job — reachable via theruns-onexpression — was not exercised; F2's same-filesystem assumption was verified only for the pool layout's path shape, not on a hosted runner. - Production cache size. See F3: the 26–61 minute figure was not reproduced and my fixtures are 1–3 orders of magnitude smaller.
8. Methodology
Ran in the CI verify lane: node:22-bookworm container, 64 cores, Debian 12, node v22.23.4, working tree at refs/pull/15064/merge (depth 2), npm ci and npm run build already complete. The base arm needed no rebuild — the artifact under test is a shell block inside a YAML file, so both arms were extracted from their respective revisions with a YAML parser (extract-writeback.mjs) and executed directly against scratch fixtures; a tmp/base-tree worktree at fbeb188 was created for reference and removed at round end. Harnesses (ab-writeback.mjs, mutate.py, measure-fix.mjs, tally.mjs) drive real child processes over real filesystems — ext4/overlay for the main fixtures, tmpfs (/dev/shm) for the cross-device cell — and every assertion is a scripted comparison. Raw logs live beside this report: ab-canonical-ledger.txt (canonical A/B), c1-scaling.log, c2a-stability.log, stability.log, c2a-final.log, c9-crossdev.log, mutation-ledger.txt, mutation-matrix.json, fix-measurement.log, gate-two-files.log, other-metatests.log, lint-gates.log, lint-shell.log, lint-shellcheck-ab.log, assertion-tally.log. assertions.json (72/72) is generated from those files by tally.mjs, never hand-counted; the repo's own vitest counts (331 + 451) are reported separately as gate results and deliberately excluded from that total. Rig hygiene: every fixture was removed, the mutated workflow was restored and sha256-verified after each of the 13 mutants, the base worktree was removed, git status is clean, and zero orphaned processes remain under the rig paths.
Flakiness gate log
rounds=5 files=1 skipped=0
file scripts/tests/sdk-java-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/sdk-java-workflow.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
scripts/tests/sdk-java-workflow.test.js: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · scripts/tests/sdk-java-workflow.test.js: P (exit 0)
round 2 · scripts/tests/sdk-java-workflow.test.js: P (exit 0)
round 3 · scripts/tests/sdk-java-workflow.test.js: P (exit 0)
round 4 · scripts/tests/sdk-java-workflow.test.js: P (exit 0)
round 5 · scripts/tests/sdk-java-workflow.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
…ignment Picks up the base-side fix for the three MySQL replay assertions that were reddening this PR's `SDK Java (full) / Hosted process fault gates / MySQL 8.4` leg: main 13a4fea test(managed-agent): align Hosted fault gates with shared ownership (#15066) That commit touches both IT classes this PR was failing on (HostedWorkspaceToolTurnIT.java, HostedProcessCrashIT.java), and this branch's merge-base (f036ab2) predates it, so the aligned expectations were missing here. With #15066 on main, those three tests no longer fail there. No PR-side code changes. This PR still only carries its own .github/workflows/sdk-java.yml budget/write-back change plus its workflow test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmviqccpiop
… spec fix The `web-shell E2E Smoke (ubuntu-latest, Node 22.x)` leg was red on this branch's head 4795e60 (run 39696432612) on a base-side desync, not on anything this PR changes: client/e2e/managed-ssh-workspace-registration.spec.ts:130 expect(getByRole('button', { name: 'Workspace', exact: true })) .toHaveText('Remote project') Expected: "Remote project" Received: "Remote project · Managed" 2 failed / 349 passed (@1280px and @390px, plus Retry #1) main has since carried the matching spec update: main d03b311 feat(managed-agent): Add durable manual context compression (#15104) which rewrote that assertion to `.toHaveText('Remote project · Managed')`. This branch's merge-base (7924267) predates it, so the failing run still checked out the stale expectation. After this merge the suffixed assertion is present in the tree, so rerunning that leg has something to converge to. No PR-side code changes: `git diff --name-only origin/main HEAD` is still exactly this PR's own two files (.github/workflows/sdk-java.yml and scripts/tests/sdk-java-workflow.test.js), and the merged tree is byte-for-byte the plain `git merge origin/main` result. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmviumo8how
|
CI attribution — The only failure annotation on this PR's head is the no-AK membership guard: The identical annotation is red on Cause: three interactive TUI-parity suites landed on Tracked in #15139. Fixes are already in flight in #15141 (@wenshao) and #15143 (autofix), each adding exactly those three 中文本 PR 头上 根因：三个交互式 TUI-parity 套件落到 已由 #15139 跟踪，修复在 #15141（@wenshao）与 #15143（autofix）在途，各加这 3 条 |
|
Correction to my previous CI-attribution comment. The attribution itself stands (the
Both were closed as superseded by #15142, which merged as It takes the opposite route from #15141/#15143 — it enrolls the three parity suites in the So the main-side breakage tracked in #15139 is fixed on 中文更正我上一条 CI 归因评论。归因本身成立（
两张都因被 #15142 取代而关闭，而 #15142 已于 2026-10-22T03:16:42Z 合并为 它走的是与 #15141/#15143 相反的路线——把三个 parity 套件登记进 所以 #15139 跟踪的 main 侧破坏已在 main 修好。但本分支早于那个提交，它自己那份 |
Inherit two main-side CI fixes that were reddening this branch's lanes: - #15142 enrolls the OpenTUI parity suites in the no-AK lane, clearing the no-ak-integration-ci membership guard assertion. - #15146 removes the duplicate markApiHistoryPrompt import that broke 'tsc --build' in packages/core. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmvizzkp6p3
This PR grows .github/workflows/sdk-java.yml from 113473 to 114180 bytes: the MariaDB lane's timeout ceiling moves 200 -> 280 with the arithmetic that justifies it, and the Maven write-back is rewritten to publish only this job's own downloads (nlink == 1) under a 300 s backstop instead of taring the whole tree. The growth is the change, so record it rather than move the step out. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmvj37bcsp8
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the diff at e7e218c (sdk-java.yml + size-baseline + workflow meta-test).
No blocking findings. Verified the fixes against the round-1 findings on the current head:
- R1-1 / R1-2 (Criticals): the tar-pipeline write-back is gone entirely. The replacement publishes only the nlink == 1 delta with per-file
cp -ln— each link is atomic and-nnever replaces an existing host file, so a timed-out backstop can leave artifacts missing for the next job to re-resolve but never truncated at a final path. The 300 s bound now covers a metadata-only operation, so it is a real backstop rather than below the mechanism's normal duration. - R1-6: the two failure modes are distinguishable now —
write_back=$?is captured, 124 emits "stopped after 300 s" and any other non-zero emits "failed". - R1-4: the ceiling decomposition now names the seed explicitly (job 118848599935: 71-minute seed, 26 of Maven) and 280 = 90 + 100 + 5 + 30 with margin; the old 200 ceiling was hit by run 39563394993.
- R1-3 / R1-5: the meta-test derives
writeBackBoundfrom the parsed step (step.run.match(/timeout (\d+) find /)) instead of a second hardcoded floor, and all pins resolve the job/step first. - R2-1 (declined): the decline rests on a mutation run executed one mutant at a time against this suite, which is the correct oracle for "the pin is live"; accepting the author's measurement here.
Zero unresolved threads; the round-1 CR was dismissed after the df8c4c1 rewrite. COMMENT rather than APPROVE: GitHub forbids self-approval on your own PR. Required CI lanes are currently running on this head.
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical-only re-review at e7e218c4. The head moved since my Comment at c9dfb7d0, and the write-back mechanism was replaced rather than patched. Every blocking issue on record is fixed, and I found no new one. Approving.
Historical blocking issues — all confirmed fixed at this head
-
Review ledger round 1,
R1-1(sev C,sdk-java.yml:720) — fixed by construction. The finding was thattimeout 300bound only the producertar, so when it fired the extractor published a partially written member into the host-shared cache. There is no longer any extraction into the shared tree: the wholetar -cf - . | tar -C "${HOME}/.m2/repository" --skip-old-files -xf -pipeline is gone, and the meta-test now pins its absence withexpect(step.run).not.toMatch(/tar [^\n]*-xf/), so a future edit cannot reintroduce the shape silently. -
Review ledger round 1,
R1-2(sev C, line 715) — fixed. The 300 s bound sat below the write-back's measured 26–61 minute whole-tree cost, so the recapture would have been cut off on every run. The write-back is now metadata-only —findover the delta plus onelink(2)per file — which the PR's own probe puts in seconds, leaving 300 s as a genuine backstop rather than the expected outcome. Exit 124 is also distinguished from other failures now, which closes ledgerR1-6's ambiguous-signal point at the same time. -
My Comment at
c9dfb7d0— fixed. I had proved experimentally that a killed producer leaves the in-flight member in the destination at full declared size but with wrong bytes, that--skip-old-filesmeans no later successful write-back ever repairs it, and that thecp -alnseed then hardlinks the corrupt artifact into subsequent jobs. All three links are broken here: nothing is streamed into the shared tree,--skip-old-filesis gone with the extractor, and publication iscp -ln— an atomiclink(2)with no-clobber — so the backstop can leave an artifact missing for the next job to re-resolve, and cannot leave one truncated, wrong-byte, or overwritten.
What I verified in the replacement mechanism
-
-links 1selects exactly this job's downloads. The seed iscp -aln "${HOME}/.m2/repository" "${RUNNER_TEMP}/m2-repo", so every pre-existing host file enters the job-local repo with nlink ≥ 2; an inode at nlink 1 is one Maven created in this job and nothing else references. Publishing only those means the write-back can never alias a file another job is mid-writing, which was the reachability question behind my earlier finding. If the seed itself failed (it is|| true), every file reads as nlink 1 and the whole tree is published — still correct, still bounded by the backstop. -
-nmakes publication non-destructive.cp -lnwill not replace an existing host file, so a concurrent or earlier artifact in the shared cache is untouched regardless of interleaving. Combined withlink(2)atomicity, a partial run is indistinguishable from a run that simply published fewer files. -
The excludes and the layout survive the mechanism change. The two in-house coordinates are still pruned —
find . \( -path ./com/alibaba/qwencode-sdk -o -path ./com/alibaba/qwen-managed-agent-server \) -prune— so third-party artifacts undercom/alibabasuch as druid still recapture, and--parents -t "${HOME}/.m2/repository"with cwd at${RUNNER_TEMP}/m2-reporeproduces the repository-relative depth the extractor used to give. -
Best-effort semantics are preserved and now more precise.
set -o pipefailis no longer load-bearing for this block (there is no pipeline), the|| write_back=$?capture keeps a failure from reddening a gate whose Maven work already passed, and the two warning arms name the two distinct causes. -
The ceiling and its guard agree.
timeout-minutes: 280covers the arithmetic the comment states — the 5400 s lock wait, up to ~100 minutes from acquisition to the end of verify, the 300 s backstop and ~30 minutes of setup, i.e. 225 minutes. The meta-test now deriveswriteBackBoundby parsingtimeout (\d+) find \.from the step text and assertstimeout-minutes * 60 >= seconds + 100*60 + writeBackBound + 30*60, so the floor tracks the bound instead of duplicating a second hardcoded number — which also settles ledgerR1-3andR1-5. Phase ordering is still pinned, withwriteBackAtcorrectly re-derived fromcd "${RUNNER_TEMP}/m2-repo"now that the tar anchor is gone. -
The size ratchet is exact.
.size-baselinerecords114180forsdk-java.yml, and the blob at this head is 114180 bytes — byte-exact, not an estimate. Worth stating becauseLint & Staticis still pending here and cannot confirm it yet.
Non-blocking, noted only: cd "${RUNNER_TEMP}/m2-repo" is unguarded, so were that directory absent the find . would walk the previous working directory. Maven creates it through -Dmaven.repo.local and has already run by this point, so the state is not reachable in practice. The hardlink publication also assumes the job-local temp and the shared cache sit on one filesystem — an assumption the existing cp -aln seed already depends on, so this adds no new environmental requirement.
CI: at this head Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK) and the SDK Java (fast) legs were still pending; Classify PR, Select the Managed Agent tier, Flyway migration version uniqueness, assign and label pass, and nothing is failing. The red Test lane seen on earlier heads was the main-side no-AK membership guard, red on main itself and since fixed there; this branch has merged that fix, so it is not attributable to this PR. Note that mysql-integration is gated on the non-fast tier, so the changed step is again not exercised by this PR's own runs — the coverage is the static pins above, which I read rather than assumed.
Critical-only 复审，基于 e7e218c4：记录在案的两个 Critical 与我此前提出的阻塞问题全部确认已修复，未发现新问题，已提交 Approve。修复方式是替换机制而非打补丁：共享缓存不再有任何解包动作，写回改为只发布本任务新下载的增量，用原子且不改写既有文件的硬链接完成，因此超时最多让某个构件缺失并由后续任务重新解析，绝不会留下残缺内容；元数据级写回通常只需数秒，300 秒退化为兜底，且超时与其他失败已分别报告。另已核对增量筛选条件与种子硬链接的引用计数关系成立、两处自研坐标仍被排除、目录层级保持不变、上限与其派生断言自洽、体积基线与真实文件逐字节相符。本次 CI 主要通道仍在排队，无失败项。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Review: fix(ci) write-back delta publish — Round 1
No blocking findings. Approval blockers: none.
Tier: Standard (CI/workflow change; no persisted user-data format, no auth, reversible).
What I checked
Shell script correctness (sdk-java.yml lines 725–750)
-
Delta predicate correctness: The seed (
cp -aln) hardlinks every host file into${RUNNER_TEMP}/m2-repo, leavingnlink≥2on all seeded files. Maven writes new downloads as fresh inodes (nlink==1).find … -links 1therefore selects exactly the files this job fetched. If the seed fails (best-effort, guarded2>/dev/null || true), all files havenlink==1and the write-back degrades to the whole tree, bounded by the 300 s backstop — the PR's stated degradation path is correct. -
Atomicity under backstop:
cp -lncreates a hard link for each file individually. Eachlink(2)is atomic — either the link is created (host sees a complete file) or it is not (host sees nothing). A SIGTERM fromtimeoutlanding mid-findcannot produce a partially-written artifact. This definitively closes the truncation failure mode that previous reviewers found on the oldtar | tarpipeline. -
Exit-code handling:
write_back=0; timeout 300 find … || write_back=0— with no pipeline,0istimeout's own exit code. Exit 124 (timeout) reaches the firstifbranch; any other non-zero reaches theelif; success falls through withwrite_back=0. The two warning messages are distinct and correctly routed. -
Exclusion logic:
-path ./com/alibaba/qwencode-sdk -pruneprunes the directory and all files under it;-path ./com/alibaba/qwen-managed-agent-server -prunelikewise. Third-partycom/alibabaartifacts (druid) are unaffected, matching the old--excludesemantics.find -pathmatches the exact named paths, not prefixes, so no collateral exclusion. -
cp --parentspath mapping:find .emits paths like./path/to/file;cp --parents -t ${HOME}/.m2/repository ./path/to/filecreates${HOME}/.m2/repository/./path/to/filewhich resolves to the correct destination. -
Ceiling derivation: 280 min × 60 = 16 800 s ≥ 5 400 (90 min lock wait) + 6 000 (100 min seed+Maven bound) + 300 (backstop) + 1 800 (setup) = 13 500 s, with ~55 min margin. Well-justified by the measured 71-min seed and 26–42-min Maven figures in the PR description.
Meta-test changes (sdk-java-workflow.test.js)
The ceiling test is updated to use a single derived assertion: seconds + 100*60 + writeBackBound + 30*60, where writeBackBound is extracted from the workflow text by regex. With 280 min this evaluates to ≥225 min, which correctly rejects a regression to the old 200-min ceiling. The write-back assertions are scoped to step.run, not the whole file. The assertions cover: cd into the temp repo, the full find … -exec cp -ln invocation, absence of tar … -xf, both warning branches.
Cross-check against prior reviews
- R1-1 (qwen-code-review-bot, CHANGES_REQUESTED): truncated artifacts via
timeoutkilling the left-handtarmid-stream. Refuted at current head — thetar | tarpipeline is gone;cp -lnis atomic per file. - R1-2 (qwen-code-review-bot): 300 s bound below the full-tree
tar's normal duration. Refuted at current head — the write-back now processes onlynlink==1delta files; the bound is a backstop, not the expected duration. - R1-6 (qwen-code-review-bot):
$?underset -o pipefailconflates timeout with other failures. Refuted at current head — no pipeline;|| write_back=$?capturestimeout's direct exit code. - R1-3, R1-5 (qwen-code-review-bot, Suggestions): duplicate literal floor and unscoped
expect(workflow)assertion. Not present at current head — the diff uses a single derived ceiling expression andstep.run-scoped assertions. - R1-4 (qwen-code-review-bot, Suggestion): seed duration not named in the ceiling comment. Noted; the PR description supplies the evidence, and the comment is substantially more accurate than the one it replaces. Not a blocker.
- R2-1 (qwen-code-review-bot, Suggestion): two-site invariant (seed
cp -aln+ write-back-links 1) not executably tied together. Author declined with mutation evidence showing both M1 (cp -aln → cp -a) and M2 (drop-links 1) turn existing tests red. That rebuttal is sound — thestep-scopedtoContainassertions at :1033/:1064 catch M1, and the full command assertion catches M2. - qqqys review at c9dfb7d: destination of bounded write-back. Refuted at current head — current write-back never extracts into the shared cache; it publishes atomic hardlinks to the host repo.
Unreviewed dimensions
- Local execution of the meta-tests was not performed (no matching Node/vitest environment available). The assertions read correctly and the mutation argument in the author's R2-1 rebuttal is independently verifiable.
- Rung 3 (terminal/environment-dependent behaviour) not applicable — no terminal geometry or OS-specific path logic changed.
Reviewed with AI assistance.
Conflicts resolved: - sdk-java.yml: keep this PR's lock-scope narrowing (seed and both Maven runs unlocked; only the cache write-back acquires the per-host lock) on top of main's bounded delta write-back (#15064), closing fd 9 on the write-back's mkdir and timeout children. Allow 310 minutes for the 90-minute wait, the ~3-hour Maven run, the 5-minute write-back backstop and 30 minutes of setup. - Design docs (EN + zh-CN): update the ceiling and the write-back children described by the Hold scope section. - sdk-java-workflow test: adapt main's write-back assertions to the fd 9 closures and carry #15121's surviving-child runtime pin onto the write-back. - .size-baseline: record the merged workflow's byte size. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
|
Released in v0.26.1. |
What this PR does
Makes the MariaDB lane's Maven cache write-back publish only the artifacts Maven fetched in this job, by per-file hardlink, with a five-minute backstop, and raises that job's ceiling from 200 to 280 minutes. The host-shared
~/.m2still gains whatever the job downloaded; it can never receive a partially written file, and a write-back that hits the backstop only leaves artifacts missing for the next job to re-resolve. The gate result is unaffected either way.Why it's needed
Run 39563394993 (PR #15000) had its
Runtime Broker and Managed Agent MariaDB / Java 21job cancelled at the 200-minute ceiling. The single locked step split into three costs:tarof the shared~/.m2The write-back streamed the entire seeded repository through
taronly for--skip-old-filesto discard almost all of it, so its cost grew with the host's accumulated cache (26, 51 and 61 minutes in three recent jobs) — the known #14384 cost. Simply putting a wall-clock cap on that pipeline would not work: the cap would trip on every run, and a producer killed mid-member leaves a truncated artifact in the shared cache that--skip-old-filesnever repairs and the next job's hardlink seed picks up.Because the seed hardlinks every host file into the job-local repository, the files Maven fetched during this job are exactly the ones with a single link. Publishing only those, with no-clobber hardlinks, turns the write-back into a metadata-only merge of the delta; each link is atomic, so stopping it early cannot publish a partial file. The step distinguishes "stopped at the backstop" from other failures in its warning.
The ceiling has to accommodate the 90-minute lock wait, up to ~100 minutes from lock acquisition to the end of verify (one job spent 71 minutes in the seed alone), the backstop and setup; 200 does not.
The other red job in that run,
Hosted process fault gates, failed on two known flaky Hosted tests; that is tracked separately and untouched here.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/sdk-java-workflow.test.js scripts/tests/workflow-size.test.js. The meta-tests pin the delta predicate, the two in-house excludes, the no-clobber hardlink publish, the separate timeout warning, the absence of anytar … -xfinto the shared cache, and a ceiling derived from the lock wait plus the measured hold, backstop and setup. On the pool, the next full-tier MariaDB job should finish its write-back in seconds; aMaven cache write-back stopped after 300 swarning means the backstop fired.Evidence (Before & After)
Before: run 39563394993 job 119117818954 — step 08:29:11 → 11:43:47, cancelled at 200 minutes,
Terminate orphan process: tar.After (GNU coreutils 8.32 / findutils, Ubuntu 22.04 and Oracle Linux containers): the delta publish copies only new files, keeps seeded host files untouched even when Maven replaced them by rename, skips both in-house coordinates and still recaptures third-party
com/alibabaartifacts. With the backstop forced to fire mid-run over 20 000 new files it exited 124 after publishing ~5 000 of them with zero partial files, and the next run published the rest. Meta-test mutations (dropping the delta predicate, the backstop,-n, the 124 arm, or lowering the ceiling to 200) each turn the suite red.Tested on
Environment (optional)
N/A — workflow budget and one shell step.
Risk & Scope
Linked Issues
Refs #15000
Refs #14384
中文说明
本 PR 做了什么
让 MariaDB lane 的 Maven 缓存写回只把本次 job 中 Maven 新下载的构件逐个硬链接回宿主机共享的
~/.m2，并设 5 分钟兜底；同时把该 job 的上限从 200 分钟提到 280 分钟。共享缓存仍会获得本次下载的内容，但永远不会收到写了一半的文件；即使触发兜底，也只是少了一些构件，由下一个 job 重新解析。门禁结果不受影响。为什么需要
run 39563394993（PR #15000）的
Runtime Broker and Managed Agent MariaDB / Java 21在 200 分钟上限被 cancel。持锁步骤拆成三段：~/.m2原写回会把整个播种出来的仓库用
tar流一遍，再由--skip-old-files丢掉几乎全部内容，所以耗时随宿主机缓存累积而增长（最近三次分别为 26、51、61 分钟），即 #14384 的已知成本。单纯给这条管道加墙钟上限行不通：每次都会触发；而且生产端在某个成员写到一半时被杀，会在共享缓存里留下截断的构件，--skip-old-files永远不会修复它，下一个 job 的硬链接播种还会把它带进去。播种是把宿主机每个文件硬链接进 job 本地仓库，因此本次 Maven 新下载的文件恰好是链接数为 1 的那些。只发布这些文件、且用不覆盖的硬链接，写回就变成只涉及元数据的增量合并；每个链接都是原子的，提前停止也不会发布半个文件。warning 也区分了“兜底超时”和其他失败。
上限要覆盖 90 分钟锁等待、从拿到锁到 verify 结束最多约 100 分钟（有一次仅播种就用了 71 分钟）、兜底时间和 setup；200 分钟不够。
同一次 run 里另一个红的
Hosted process fault gates是两个已知 flaky 的 Hosted 测试，单独跟踪，本次不动。评审测试计划
如何验证
npx vitest run scripts/tests/sdk-java-workflow.test.js scripts/tests/workflow-size.test.js。meta-test 钉住了增量判据、两个自研坐标的排除、不覆盖的硬链接发布、单独的超时 warning、不再有任何tar … -xf解包进共享缓存，以及由锁等待 + 实测持锁时间 + 兜底 + setup 推导出的上限。池上下一次 full-tier MariaDB job 的写回应在数秒内完成；若出现Maven cache write-back stopped after 300 s，说明兜底触发。证据（前后对比）
Before：run 39563394993 job 119117818954 —— 步骤 08:29:11 → 11:43:47，在 200 分钟被 cancel，
Terminate orphan process: tar。After（GNU coreutils 8.32 / findutils，Ubuntu 22.04 与 Oracle Linux 容器）：增量发布只复制新文件；即使 Maven 以 rename 方式替换了已播种文件，宿主机文件也保持不变；两个自研坐标被跳过，
com/alibaba下的第三方构件仍会回收。在 2 万个新文件上强制兜底中途触发：退出码 124，已发布约 5000 个，部分写入文件为 0；下一次运行补齐其余文件。meta-test 变异（去掉增量判据、兜底、-n、124 分支，或把上限降回 200）均会让测试变红。测试环境
环境（可选）
N/A —— 工作流预算与一个 shell 步骤。
风险与范围
关联 Issue
Refs #15000
Refs #14384