[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #1968 (register T60, living document Part 5). This is the first slice. The issue stays open for the rest (listed under "Remaining").

Why

Each docker_e2e_* suite carried its own copy of the container scaffolding. 9 copies of cov_docker_args and 8 of assert_vex_agent_attested were byte-identical (md5-checked), and skip_if_no_image had drifted: 7 copies never read the required switch, and deno read SOCKET_PATCH_REQUIRE_DOCKER, which nothing sets.

Changes (test-only)

• New tests/docker_common/mod.rs, the one home for:
  • cov_docker_args, with sbt's SOCKET_PATCH_DOCKER_BIN branch folded in;
  • skip_if_no_image(image) / skip_or_require_image, moved from docker_vendor_common;
  • run_container(image, script);
  • assert_vex_agent_attested(stdout, purl, ghsa).
• Deleted per-suite copies:
  • cov_docker_args ×10, counting docker_vendor_common and sbt's binary_mounts;
  • skip_if_no_image ×10, counting npm's skip_if_no_docker_image and sbt's required();
  • run_container ×5 and npm's run_in_container;
  • the inline docker run blocks in cargo, golang and maven;
  • assert_vex_agent_attested ×8;
  • plain_sha256 ×3 and pypi's sha256_hex (use common::sha256_hex).
• Other deletions:
  • deno's SOCKET_PATCH_REQUIRE_DOCKER gate;
  • the dead docker_vendor_common::run_in_image;
  • npm's unused Duration import and its const _ suppressor.
• docker_vendor_common builds its docker run on crate::docker_common::cov_docker_args. Its includers (5 vendor suites, docker_e2e_npm, tests/vendor/main.rs) also declare docker_common. The selftest imports skip_or_require_image from there.

Diff: +260 / −1,113, all in tests (net −853). No production lines change.

Behavior

• The required gate. Every agent suite now honours SOCKET_PATCH_DOCKER_E2E_REQUIRED=1 the way sbt and the vendor capstones already did. The variable is set only for:

  • the sbt) arm of both docker jobs;
  • composer-compatibility.yml, which runs only docker_e2e_vendor_composer;
  • scripts/sbt-compat-matrix.sh, which runs only docker_e2e_sbt.

  So no CI leg changes outcome.

• SOCKET_PATCH_DOCKER_BIN. It now overrides the binary in every suite, not only in sbt. Only sbt-compat-matrix.sh sets it.

• Skip messages. Their wording is now uniform. npm's no-docker message no longer mentions SOCKET_PATCH_TEST_HOST, which the module doc still documents.

Verification

• All 15 docker_e2e_* binaries build and pass with --features docker-e2e. Without images, the container tests skip and the host-side tests run: 17–31 per binary.
• tests/vendor's docker_vendor_common selftests pass 8/8, including a_required_docker_suite_fails_on_a_missing_image.
• cargo clippy --workspace --all-features -- -D warnings is clean.
• The containers themselves only run in the coverage-docker / e2e-docker jobs, so this needs a full-scope CI dispatch to prove them.

/code-review high

It raised 10 findings. None were fixed, for these reasons:

1. Deno lost SOCKET_PATCH_REQUIRE_DOCKER. This is intended (Share one docker test harness: cov_docker_args ×10, skip_if_no_image ×11 and assert_vex_agent_attested ×8 are copied, and 8 docker suites never honour SOCKET_PATCH_DOCKER_E2E_REQUIRED #1968, step 3): nothing sets the variable, and the shared gate reads SOCKET_PATCH_DOCKER_E2E_REQUIRED.
2. SOCKET_PATCH_DOCKER_BIN now applies everywhere and skips the coverage mounts. Its only setter runs sbt alone, so it keeps sbt's precedence.
3. Agent suites now honour REQUIRED. This is intended, and as shown above no runner sets it across suites.
4. npm's host-mode hint is gone from the skip message. The module doc still documents host mode.
5. run_container takes no extra args. Folding in sbt (-m 2g, -e), docker_vendor_common::docker_command and npm's vlt smoke needs an extra parameter. That is left for the follow-up, together with items 6 and 7.
6. host.docker.internal URL ×15 and api_url_for_container ×2: follow-up.
7. assert_api_path_exercised ×4, drifted: follow-up, as the issue's assert_api_exercised.
8. crate::docker_common is a crate-root requirement. A missing declaration fails compilation at that line.
9. A docker image inspect runs per test. That was already the case and is cheap.
10. A ci.yml comment names the per-suite files. ci.yml is left alone because ci-janitor PRs Retry pinned bundler installs in the e2e legs #1854 and Annotate failing tests in macOS/Windows test legs #1890 change it.

Remaining on #1968

• export REQUIRED for every docker leg in ci.yml;
• the shared assert_api_exercised and container URL;
• the GIT_SHA256_FN copies ×3 → git_blob_sha;
• the bash "applied": 1 greps → assert_summary (gem matches 10);
• run_container with extra args.

Risk

Low to medium. The change is test-only and compile-checked, but the docker containers can't run in this sandbox.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeP9dHgNGnDr1A5rmCp

---

Note

Low Risk
Changes are confined to integration tests and shared test modules; production code is untouched, with minor env-gate alignment that the PR notes should not alter current CI legs.

Overview
Introduces tests/docker_common/mod.rs as the shared home for Docker integration-test plumbing, and wires every docker_e2e_* (and vendor suites that already used overlapping helpers) through it instead of copy-pasted locals.

Consolidated helpers: cov_docker_args (now also honors SOCKET_PATCH_DOCKER_BIN before coverage mounts), skip_if_no_image / skip_or_require_image, run_container(image, script), and assert_vex_agent_attested(stdout, purl, ghsa) with an explicit GHSA argument. docker_vendor_common drops its duplicate skip/coverage logic and calls crate::docker_common::cov_docker_args; the unused run_in_image helper is removed.

Suite cleanup: Per-file duplicates of coverage mounts, skip gates, container runners, VEX oracles, and local plain_sha256 / pypi sha256_hex are deleted in favor of common::sha256_hex. Agent suites pass an IMAGE constant into the shared skip/run APIs; sbt swaps binary_mounts for cov_docker_args.

Intentional test behavior alignment: All agent docker suites now respect SOCKET_PATCH_DOCKER_E2E_REQUIRED=1 (replacing deno’s unused SOCKET_PATCH_REQUIRE_DOCKER gate). Skip messaging is unified (npm no longer mentions host mode in the skip line).

^{Reviewed by Cursor Bugbot for commit 22c76cc. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 22c76cc. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 22c76cc8 (burn-down agent).

• CI: 82/82 settled green (32 success, 50 skipped).
• Bugbot: reviewed 22c76cc8, no findings.
• Mergeable, no conflicts; no CHANGELOG change.
• Reviewer focus: first slice of Share one docker test harness: cov_docker_args ×10, skip_if_no_image ×11 and assert_vex_agent_attested ×8 are copied, and 8 docker suites never honour SOCKET_PATCH_DOCKER_E2E_REQUIRED #1968. The docker e2e suites now share tests/docker_common/mod.rs; check that each suite's container setup behaves the same as before.
• Slack announcement: not sent this run (Slack send unavailable); the next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Adds a shared tests/docker_common module (cov_docker_args, skip_if_no_image / skip_or_require_image, run_container, assert_vex_agent_attested) and deletes the per-suite copies from the 10 agent docker suites and docker_vendor_common. This is the first slice of #1968. It changes test code only.

Risk: low. I compared every deleted copy line by line with the shared code. The images, docker flags (sbt keeps -m 2g and SOCKET_PATCH_JDK), coverage mounts, env vars and VEX checks are all unchanged. Two behaviours change, and the description states both: every suite now honours SOCKET_PATCH_DOCKER_E2E_REQUIRED=1 and SOCKET_PATCH_DOCKER_BIN. The only CI legs that set those already ran suites that honoured them, so no CI outcome changes.

Look here:

• docker_common/mod.rs:16-34: cov_docker_args. SOCKET_PATCH_DOCKER_BIN comes first and skips the coverage mounts.
• docker_common/mod.rs:47-87: the skip/require gate and run_container
• docker_e2e_sbt.rs:445-461: sbt on the shared helpers
• docker_vendor_common/mod.rs:49:`` vendor helpers build on docker_common

Verified: I read the full diff against #1968. cargo test -p socket-patch-cli --features docker-e2e --no-run builds all 15 docker_e2e_* targets plus --test vendor. Without docker, all 15 binaries pass with their container tests skipped, and the docker_vendor_common selftests pass 8/8. I diffed --list output for every binary against the merge-base: the test names are identical, none lost or added. Clippy with --all-targets shows no lints in the touched code; the existing lints in untouched files are the same at base and head. CI is green (ci-ok, clippy). Bugbot is neutral. CHANGELOG.md is untouched. The containers themselves were not run here, since there is no docker in this environment.

Changes I made: none.

Open questions / nits (non-blocking, for the follow-up slices):

• Deno's old gate accepted 1 or true (any case); the shared gate accepts only 1. Nothing sets that variable today. Confirm this matches the truthiness rule from Gate toolchain e2e suites through one common::toolchain_gate: ten *_E2E_REQUIRED readers disagree on whether =0 and =true mean required #1940.
• With both SOCKET_PATCH_DOCKER_BIN and SOCKET_PATCH_COV_BIN set, coverage is dropped silently in every suite. No CI leg sets both, but an eprintln! would help.
• The ci.yml:829 comment is stale. The PR defers that fix to avoid conflicts with Retry pinned bundler installs in the e2e legs #1854 and Annotate failing tests in macOS/Windows test legs #1890.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 22c76cc86f38c0d8f4e0be42971dbcda0fb31a87. Checked shared Docker image gates, coverage mounts, execution flags and agent VEX assertions against migrated suites. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.