GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — CI: hermetic spawns in cli/covgap tests (#823) by mikolalysenko · Pull Request #1441 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

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

Refs #823 (slice 4 of the hermetic-spawn migration; the issue stays open for the remaining files).

What was wrong

20 CLI test files (cli/, cli_*, coverage_fix_*, covgap_*) spawned socket-patch with a bare Command::new and their own hand-rolled SOCKET_* scrub. The copies disagreed. Several removed only the global flag vars, so SOCKET_API_TOKEN, SOCKET_API_URL or a per-command var like SOCKET_SKIP_ROLLBACK from a developer shell still reached the child. Two plain list runs in cli_parse_list scrubbed nothing, and none forced the update notifier off.

There was also a race in the shared builder. hermetic::command removed only the SOCKET_* names present when it swept the parent env. Test binaries that set flag-bound vars in-process (apply_env_toggles, save/restore helpers) could leak one set between the sweep and the spawn, and each suite patched that with its own removal list. Those lists had already drifted: 4 vars in one place, 2 in another, the full list in a third.

Change (test-only)

  • All 20 files spawn through tests/common/hermetic.rs and leave PENDING_RAW_SPAWNS (126 → 106), so the ratchet keeps them hermetic. Deliberate opt-ins stay as caller env: the config-layer suites still set SOCKET_NO_CONFIG=0, and the telemetry suite still re-enables telemetry.
  • hermetic::command now removes every flag-bound var (GLOBAL_ARG_ENV_VARS ∪ LOCAL_ARG_ENV_VARS) by name, which closes the race once and deletes the per-file lists. SOCKET_CWD joins the hostile seeds (the telemetry suite used to seed it locally). New contract test: command_drops_flag_bound_vars_set_after_it_is_built.
  • No workflow change. A lean PR run keeps the same job count (~28 Linux jobs) and job-minutes.

Evidence

  • Hostile shell (SOCKET_DRY_RUN=true SOCKET_JSON=true SOCKET_API_TOKEN=… SOCKET_API_URL=http://127.0.0.1:1 SOCKET_PROXY_URL=… SOCKET_SKIP_ROLLBACK=true SOCKET_ALL_RELEASES=true SOCKET_ORG_SLUG=… SOCKET_NO_CONFIG=0) over the 19 affected targets:
    • main: 11 failures. 6 spawn tests in cli_parse_list (*_via_binary), covgap_commands_vendor::npm_only_human_vendor_does_not_read_composer_lock, plus 4 noted below.
    • this branch: the 7 spawn tests pass. What remains is cli_parse_list::defaults_match_contract (an in-process clap parse, not a spawn; out of scope here) and the 3 root-only failures below.
  • command_drops_flag_bound_vars_set_after_it_is_built fails against main's hermetic.rs (SOCKET_OFFLINE set after build leaked) and passes with this change. spawn_env_hygiene: 15/15.
  • cargo test --workspace --all-features --no-fail-fast, 249 targets: everything passes except 13 tests that need a non-root user to make a file unwritable (*_write_failure_*, *unremovable*, relax_loop_must_not_traverse_symlinked_root) and e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed. My container runs as root. Each of these fails the same way with main's hermetic.rs and test files, so none are from this PR. CI runs as non-root.
  • cargo fmt --all -- --check and cargo clippy --locked --workspace --all-features -- -D warnings: clean.

/code-review high: findings not fixed here

  • PTY spawns (portable_pty::CommandBuilder::new(binary()), 7 sites, including covgap_commands_scan_mod.rs and covgap_commands_rollback.rs) keep their own scrubs, and the ratchet does not detect them. That needs a CommandBuilder counterpart of the builder, so it is a separate slice, recorded on Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823.
  • spawn_scrubbed (rollback) doesn't apply jvm_env::isolate_cli. No caller spawns a JVM fixture, and its doc now says so.

Follow-ups for #823: the 106 remaining PENDING_RAW_SPAWNS files (mostly e2e_*, in_process_*, vex_*), the PTY spawns, and the in-process parse in cli_parse_list.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CQp9sCPnUPTSFrEGsvTQmQ


Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@claude
Start fix for #823 (cli/covgap spawns)
8e02bca
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 12, 2026
Spawn every CLI test child through one hermetic Command builder; 10 test files inherit ambient SOCKET_* today #823
Closed
5 tasks
Claude (claude) added 2 commits October 12, 2026 18:00
@claude
Make cli and covgap tests ignore SOCKET_*
e974944
Twenty CLI test files spawned socket-patch with a bare Command::new and
a hand-rolled SOCKET_* scrub. The copies disagreed: several only removed
the global flag vars, so SOCKET_API_TOKEN, SOCKET_API_URL or a
per-command var like SOCKET_SKIP_ROLLBACK from a developer shell still
reached the child, cli_parse_list's two plain list runs scrubbed
nothing, and none forced the update notifier off.

All 20 now spawn through the shared hermetic builder in
tests/common/hermetic.rs and leave PENDING_RAW_SPAWNS, so the ratchet
keeps them hermetic. Deliberate opt-ins stay as caller env: the config
layer suites still set SOCKET_NO_CONFIG=0 and the telemetry suite still
re-enables telemetry. Suites whose in-process tests mutate SOCKET_* keep
their unconditional removals for the spawn race.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@claude
Close the in-process env race in the builder
6d44a29
Some test binaries set flag-bound SOCKET_* vars in their own process
(apply_env_toggles, env save/restore helpers). The hermetic builder only
removed names present when it swept the parent environment, so a var
set by a parallel test before the spawn still reached the child, and
suites patched that with their own, already diverging, removal lists.

hermetic::command now removes every flag-bound var by name, so the
per-file lists go away, and SOCKET_CWD joins the hostile seeds. A new
contract test sets vars after the builder runs and checks none leak.

Refs #823

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 12, 2026 18:46
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

cursor[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor[bot]
cursor Bot reviewed Oct 12, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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.

✅ 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 6d44a29. Configure here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 12, 2026
Swarm health manager log #1382
Open
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 12, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent)

  • Head: 6d44a29a
  • CI: 86/86 settled on the current head (36 success, 50 path-skipped); no main-wide failures.
  • Mergeable against main, no CHANGELOG.md change.
  • Bugbot: reviewed 6d44a29, no findings; no open review threads.
  • Reviewer focus: the shared tests/common/hermetic.rs helper that the CLI/covgap tests now use to spawn the binary with a scrubbed environment.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay182003
Tanmay Singla (Tanmay182003) approved these changes Oct 12, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 12, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 12, 2026
CI: hermetic spawns in scan/vendor tests (#823) #1448
Merged
Merged via the queue into main with commit 6ad6add Oct 12, 2026
86 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-ci-hermetic-cli-covgap branch October 12, 2026 20:59
This was referenced Oct 12, 2026
chore(release): bump to 5.0.1 with release notes #1446
Closed
chore(release): 5.0.1 #1485
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@Tanmay182003 Tanmay Singla (Tanmay182003) Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mikolalysenko @Tanmay182003 @claude
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.