Skip to content

Bound crawler and tool probes through one spawn deadline (#845) - #886

Open
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
arch-refactor/845-bounded-probes
Open

Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
mainfrom
arch-refactor/845-bounded-probes

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #845

Summary

Every crawler probe (gem env gemdir, python3 --version, npm root -g, composer global config home, ...) now runs under a 10 s deadline through one utils::process::output_within primitive. A wedged toolchain shim used to hang scan, and every other crawling command, forever with no output. The three sites that hand-rolled their own tokio::time::timeout + kill_on_drop (pipenv, hatch, self-update sanity_exec) now use the same primitive, and their inline blocks are deleted.

Why

What changed

  • utils::process::output_within(Command, budget) -> Result<Output, BoundedError>: null stdin, captured stdout, discarded stderr. stdout is read on a thread under recv_timeout, then the child's exit is polled until the deadline. At the deadline the child is killed and reaped, and the call returns TimedOut without waiting for a grandchild that still holds the pipe (a #!/bin/sh shim's sleep).
  • PROBE_TIMEOUT = 10 s. That is the budget pipenv, hatch and sanity_exec already used.
  • run_resolved, behind SystemCommandRunner and GlobalProbeRunner, goes through output_within. A probe that times out answers None ("no information"), the same as a failed probe, and logs one --debug line.
  • pipenv::installed_major, pypi_hatch::require_environment_context_support_with and update::download::sanity_exec build a std::process::Command and run it through run_blocking(output_within). sanity_exec keeps its ETXTBSY retry loop around the call and its "hung during its --version self-check" error.
  • Base-red port (7b2eabd): Route Gradle digests through utils::digest #878's fix, verbatim, so main's failing digest ratchet passes here. It moves the inline digests in gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs onto utils::digest, and no-ops once Route Gradle digests through utils::digest #878 merges.

Deleted

Refactor commit only (excluding the #878 port): 4 files, +229 / −34.

  • Production: ≈ +124 / −34. The deleted lines are the three tokio::time::timeout(..., cmd.output()) + kill_on_drop blocks and the unbounded command.output() in run_resolved.
  • Tests: ≈ +105.

Behavior

  • Crawler probes: used to wait with no limit. Now a probe that runs longer than 10 s is killed and treated as absent.
  • Children's stderr: now goes to the null device instead of a pipe that was captured and then dropped. pipenv and hatch already threw their stderr away, and sanity_exec already nulled it.
  • Unchanged: output, exit codes, JSON and the CLI contract.

Test evidence

  • New tests in utils::process:
    • a_hung_probe_answers_none_within_its_budget: a gem shim that runs exec sleep 30, under a 300 ms budget. This can't be expressed on main, which has no budget.
    • output_within_does_not_wait_for_a_grandchild_holding_stdout.
    • output_within_reports_status_stdout_and_spawn_errors: stdout, stderr dropped, nonzero exit, null stdin, missing program.
  • New test in pypi_hatch: a_hung_hatch_is_refused_within_the_probe_budget.
  • The existing pipenv, hatch, sanity_exec (hang, ETXTBSY retry, wrong program) and process tests still pass.
  • cargo test -p socket-patch-core --lib: everything passes except the 4 tests that fail on main too because the sandbox runs as root (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…). utils::digest::tests::production_digests_go_through_the_helpers is red on main @ 9c43dfc and green here after the Route Gradle digests through utils::digest #878 port.
  • cargo test -p socket-patch-cli --all-features --test in_process_gem_apply --test cli_global_args --test in_process_alternate_installers: all pass.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • CLI repro (debug build): a Bundler project (rack 2.2.8), with a gem shim on PATH that logs its args and runs exec sleep 3600, and the API pointed at a closed port. scan --json exits 1 after 10 s with the full JSON envelope (scannedPackages: 1). The shim log shows env gemdir and env gempath, and no sleep 3600 process is left afterwards. The issue's run of the same setup on main hung past 90 s with empty stdout.

Risk

M. Every probe's spawn path changes. The budgets match the old per-site values, and the crawler probes only gain a bound.

Remaining (#845 later slices)

🤖 Generated with Claude Code

https://claude.ai/code/session_01Mr9d6u7vNrWPgoibe1ZRXd


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Oct 5, 2026
A version-manager shim that never answers (`gem`, `python3`, `npm`,
`composer` behind rbenv/asdf, a Ruby waiting on a network gem home)
used to hang `scan`, `apply`, `vex` and every other crawling command
forever with no output: the crawler probes waited on `output()` with
no deadline.

Every probe now runs through one `utils::process::output_within`
primitive: null stdin, captured stdout, dropped stderr, and the child
killed and reaped at the deadline without waiting on a grandchild
that still holds the pipe. Crawler probes get the same 10 s budget
that the Pipenv and Hatch version probes and the self-update
`--version` check already used, and those three sites drop their
hand-rolled `tokio::time::timeout` + `kill_on_drop` blocks for it.

Refs #845.

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 5, 2026
Assisted-by: Claude Code:claude-opus-5-5

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

`main` fails `utils::digest::tests::production_digests_go_through_the_
helpers` because #646 left inline sha1/sha256 calls in
`gradle_cache.rs`, `jvm_jar.rs` and `sidecars/maven.rs`, which turns
`test`, `test-release` and `coverage` red on every PR. This is #878's
change verbatim; it no-ops once #878 merges.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] coverage, test (macos-latest), test (windows-latest) and test-release failed on b036f7d at a single test, utils::digest::tests::production_digests_go_through_the_helpers. It is red on main @ 9c43dfc as well: #646 left inline digests in crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs, and this PR touches none of them. I ported #878's fix verbatim in 7b2eabd. It no-ops once #878 merges. Locally the digest, gradle_cache, jvm_jar and sidecars tests pass, and clippy is clean.

The cancelled e2e (windows-latest, e2e_safety_vlt, …) cell ran no steps and left no logs, which points to a lost runner rather than a test failure. The new push re-runs it.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 7b2eabd. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] lock-diff (vlt patch compatibility) failed on 7b2eabd with "no cross-OS lock set … locks from no OS". The cause sits upstream of it: build (ubuntu-latest) was cancelled after 30 minutes without ever getting a runner (no runner name, no steps ran). That skipped native, canary, downgrade and install-proof, so lock-diff had no locks to compare. Nothing ran from this diff, so this isn't a failure in this PR. I re-ran the failed jobs once.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Update on lock-diff: the one re-run failed the same way. build (ubuntu-latest) was cancelled again after about 16 minutes with no runner assigned and no steps run, so lock-diff again compared only the Windows and macOS locks against a missing Linux set.

This isn't this PR's. Every recent vlt patch compatibility run shows the same cancelled, runner-less build (ubuntu-latest): #724, #777, #768, #657, #825 and others since about 20:10 UTC. It's a runner-capacity problem for that job, not a code failure. I've used the re-run and won't spend another. The check should go green once ubuntu runners pick that job up again; the next push or a maintainer re-run will show it.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] gradle 7.6.6 / jdk 17 / vendor / ubuntu-latest failed on 7b2eabd in gradle_multi_project_vendor_locked_offline_tamper_and_byte_exact_revert. Gradle itself failed before socket-patch ran: Could not GET 'https://repo.maven.apache.org/maven2/org/apache/commons/commons-text/1.10.0/commons-text-1.10.0.pom'. Received status code 429 from server: Too Many Requests while resolving the buildscript classpath. That's Maven Central rate-limiting the runner, not this diff; the PR doesn't touch Gradle, Maven or the vendor flow beyond the already-merged digest port, and the other 7.6.6 cells (fake-central smoke, both DSLs) passed in the same job. I've already spent this PR's one re-run on the runner-less vlt build, so I'm not re-running this one. A maintainer re-run, or the next push, should clear it.


Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 6, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Ready for review at 7b2eabd. CI: the two infra failures are now green after one re-run each. The vlt lock-diff job had failed because build (ubuntu-latest) never got a runner, and the Gradle 7.6.6 vendor cell had hit a Maven Central 429. All workflow checks are now green. The only non-green items are the CodeQL default-setup Analyze (actions/javascript-typescript) jobs, which the outage cancelled. They aren't required and the API won't re-run them. Bugbot reviewed 7b2eabd with no findings, and there are no open review threads.


Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 6, 2026
No slot free (#876, #886, #889 ready); main and ranking unchanged.

Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 6, 2026
Capacity full (#876, #886, #889 ready). New tracking issue #930
and child #931 ranked; both skipped on file overlap.

Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 6, 2026
No free slot: #876, #886 and #889 are ready and approved. New #960
is skipped because commands/vendor.rs is changed by open PRs.

Assisted-by: Claude Code:claude-opus-5-5

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crawler probes spawn gem, python and npm with no timeout, so a hung shim hangs scan forever

3 participants