Fix scan/get --json dropping apply failures (#424) by mikolalysenko · Pull Request #955 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-06T18:28:29Z

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

Fixes #424

Summary

When scan --mode agent or get (agent mode) downloads a patch and the in-place apply then fails, --json now says what failed. Before this change the envelope said failed: 0 and applied: 0, listed the patch as added, and had no error text. Only the exit code and status: "partial_failure" hinted at the failure. The human output already printed the error.

Now the failed patch's record is {purl, uuid, action: "failed", errorCode, error}, using the same code/text pair the standalone apply --json emits (apply_failed, or package_not_installed when nothing is installed and the lockfiles don't resolve it). failed counts it, and applied counts only patches that really applied. A failure that no single patch explains (unreadable manifest, yarn PnP refusal, unavailable patch sources) is reported as top-level errorCode / error on the same object (apply in scan's envelope).

Root cause

run_nested_apply in crates/socket-patch-cli/src/commands/get.rs returned only a bool. The nested apply never prints JSON (one envelope per command), so its per-patch events were thrown away. download_and_apply_patches_with (behind both get and scan --mode agent) and the single-uuid get path then filled failed from download failures only. The code is shared by every ecosystem, which is why the report reproduced with Maven, PDM, Pipenv, npm and vlt.

Fix

apply::run_locked now returns an ApplyRunReport (exit code, per-patch failures, optional run-level error) instead of a bare exit code. apply itself still uses only the code, so its output is unchanged.
get.rs fold_apply_failures folds the report into the get/scan envelope. Records are matched by normalized purl, falling back to the base purl for qualified PyPI variants. A failing manifest patch that this run didn't select is added as its own failed record, because the nested apply covers the whole --ecosystems-scoped manifest.
CLI_CONTRACT.md documents the new failed records and counters.
No wrapper changes are needed: the npm, PyPI and gem wrappers only dispatch the binary.

Tests (red → green)

The regression tests were committed before the fix (9951d28) and failed on that commit with exactly the reported shape: "failed":0,...,"action":"added" and no error. All of them pass with the fix (94c993c and later).

Issue	Test	Before	After
#424 `scan --mode agent --json`	`covgap_commands_scan_mod::scan_agent_json_nested_apply_failure_reaches_the_apply_block`	FAIL (`apply.failed` 0)	pass
#424 `get <uuid> --json`	`covgap_commands_get::get_uuid_json_nested_apply_failure_names_the_patch`	FAIL	pass
#424 engine (`get` search path / scan)	`covgap_commands_get::engine_nested_apply_failure_reaches_the_json_envelope`	FAIL	pass
#424 not-installed variant	`covgap_commands_get::engine_nested_apply_not_installed_reaches_the_json_envelope`	FAIL	pass
control: clean apply unchanged	`covgap_commands_get::engine_nested_apply_success_keeps_added_and_counts_applied`	pass	pass
#424 uninstalled patches stay warnings beside a real failure	`apply::tests::collect_apply_failures_names_unresolved_purls_only_when_nothing_else_failed`	new	pass
fold/collect units	`get::tests::fold_apply_failures_` (4), `apply::tests::collect_apply_failures_` (2)	new	pass

The apply-failure tests use the first-party-link refusal from the issue thread (node_modules/<pkg> symlinked to packages/<pkg>), so they're deterministic and don't need a read-only filesystem, which root would bypass anyway. Because they use symlinks they're #[cfg(unix)].

Existing test updated: the composer and gem docker e2e verifiers (docker_e2e_composer.rs, docker_e2e_gem.rs) used to check that scan's JSON said "action": "added". In those fixtures, scan's own in-place apply fails on a hash mismatch and the later apply --force patches the file, so added was only there because of this bug. CI's coverage-docker (composer) caught the new failed/apply_failed record. The verifiers now check what "synced" actually means: the purl is recorded in .socket/manifest.json.

Local checks:

cargo clippy --workspace --all-features -- -D warnings: clean.
cargo test --workspace --all-features: not run in full locally: building every test binary ran this container out of disk. Run instead and all green: the socket-patch-cli lib tests (853), the get, scan and apply targets, in_process_get* (6 targets), covgap_commands_get (89), covgap_commands_scan_mod (52), and cli_{get,scan,apply}_silent. CI runs the full suite.
Formatting: CI has no rustfmt step, and cargo fmt --all -- --check already reports diffs in about 120 untouched files on main. The files this PR touches are clean under rustfmt --edition 2021 --check.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HjNH36TbmyXCpJPw3EyBZB

Note

Medium Risk
Changes JSON contract and exit/partial-failure semantics for agent-mode get/scan; apply internals now expose structured failures but standalone apply output is unchanged.

Overview
Fixes #424: when get or scan --mode agent runs download then nested apply, --json now reports apply failures instead of leaving patches as added with failed: 0.

apply::run_locked returns an ApplyRunReport (exit code, per-patch ApplyFailure list, optional run-level errorCode/error, and which purls actually applied). Standalone apply still only uses the exit code. collect_apply_failures maps apply results to apply_failed or package_not_installed, matching standalone apply --json.

get.rs folds that report via fold_apply_failures: selected patch rows become action: "failed" with metadata stripped; unselected manifest failures are appended; counters and top-level errors align with CLI_CONTRACT.md. PURL matching uses normalization and base-purl rules for qualified variants.

Tests cover engine, get <uuid> --json, and scan agent JSON; composer/gem docker e2e now treat “synced” as manifest presence when scan’s inline apply can show failed. Unrelated: digest.rs pending-inline list extended.

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

Generated by Claude Code

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

scan --mode agent --json and get --json report a failed nested apply as failed: 0 with the patch listed as added and no error text. These tests pin the expected envelope: the patch record carries action: failed, errorCode and error, and failed counts it. Refs #424 Assisted-by: Claude Code:claude-opus-5-5

When scan --mode agent or get downloads a patch and the in-place apply then fails, the --json output said failed: 0, listed the patch as added and carried no error, so automation reading the JSON could not tell what went wrong. Only the exit code and status hinted at it. The nested apply now hands its failures back to the caller instead of just a pass/fail flag. Each patch that failed to apply is reported as action: failed with the same errorCode/error pair that apply --json prints (apply_failed or package_not_installed), failed counts it, and applied counts only patches that really applied. A failure that no single patch explains (unreadable manifest, yarn PnP refusal, missing patch sources) is reported as a top-level errorCode/error. Fixes #424 Assisted-by: Claude Code:claude-opus-5-5

When one patch fails to apply, apply only warns about other patches that have no installed copy. The JSON report now matches that: those patches are reported as package_not_installed failures only when nothing else failed the run. Adds unit tests for the failure collection. Refs #424 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-06T18:57:24Z

BugBot review

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-06T18:58:56Z

[agent] CI on d2e44f3: the Bun and vlt patch-compatibility runs are red, but in cells this PR doesn't touch.

Bun patch compatibility (native ubuntu 1.0.0 / 1.1.0 / 1.1.38, macOS 0.8.1 / 1.0.0 / 1.0.36): every failing cell is hosted mode. Three cells failed with HTTP Error 503: Service Unavailable from the patch service. The rest failed *PatchedBytes checks with no error text, which means bun install didn't land the hosted bytes. The failures land on different shapes on each runner (direct, dev, optional, alias, peer, space-unicode), which doesn't look like a code regression.
vlt patch compatibility: native (ubuntu-latest, 1.2.0) had one cell, 1.2.0-vendored-dev, end in safe-refusal instead of patched. lock-diff follows from that: the Linux vlt-lock.json for that same cell differs from macOS and Windows.

Why these aren't this PR's: the diff only changes the agent-mode nested apply (apply::run_locked and get.rs run_nested_apply/fold_apply_failures) and how its result is reported in --json. Hosted and vendored runs never call either. Both workflows passed on other agent branches all afternoon (latest 18:34 UTC), and the patch service returned 503s during this run.

No code fix exists or is needed in this PR. I'll re-run the failed jobs once when each run completes. A second failure would be treated as real and investigated.

Generated by Claude Code

The composer and gem docker e2e scripts checked that scan's JSON said "action": "added". In these fixtures scan's own in-place apply fails (the later apply --force patches the file), and scan --json now reports that failure on the patch record (#424). So "added" was only there because of the bug. Check instead that the patch was recorded in .socket/manifest.json, which is what "synced" means here. Refs #424 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-06T19:02:20Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

The --json apply failure report could blame the wrong patch and miscount applied: - a failure on one PyPI release variant was pinned on a selected sibling variant that applied fine, via a base-purl fallback; - applied was "selected minus failed", so a selected patch that was never installed (only a warning next to a real failure) still counted as applied; - get <uuid> zeroed applied whenever any other manifest patch failed, and its extra failure records had no uuid. The nested apply now also reports which package keys it patched, and the envelope counts applied from that. A failure only marks records it covers: the same purl, or an unqualified key covering its variants. Refs #424 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-06T19:06:49Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

The digest guard test (#865) fails on main. Gradle support landed with inline sha256/sha1 computations in crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs, and the guard's pending list doesn't name them. List them as pending so CI is green until they move onto the utils::digest helpers. Open PRs #876 and #889 add only gradle_cache.rs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HjNH36TbmyXCpJPw3EyBZB

Mikola Lysenko (mikolalysenko) · 2026-10-06T19:27:40Z

[agent] test (macos-latest) on 9df5fab failed in socket-patch-core --lib: utils::digest::tests::production_digests_go_through_the_helpers. This isn't this PR's failure. It's red on main too (CI on 9c43dfc failed). The digest guard from #865 doesn't list the inline digests that Gradle support added in crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. I reproduced it locally.

I added a minimal fix in c51938b: those three files are added to PENDING_INLINE_DIGESTS, and the test passes locally. This change does nothing once main lists them itself. #876 and #889 each add only gradle_cache.rs, so they'd still fail on the other two files.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-06T21:18:37Z

bugbot run

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 c51938b. Configure here.}

Mikola Lysenko (mikolalysenko) · 2026-10-06T21:20:47Z

[agent] Ready for review at c51938b.

CI: every check suite on c51938b is green.
Bugbot: reviewed c51938b, no new issues. No open review threads.
Reviewer note: c51938b only ports main's digest-guard fix (adds the Gradle/Maven inline digests to the guard's pending list) so socket-patch-core --lib passes; it no-ops once main carries the same change. Approved earlier on this same head.

Generated by Claude Code

Merging main brought in #955's PENDING_INLINE_DIGESTS entries for gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs, but #690 had already moved those files onto the utils::digest helpers. The guard fails on stale entries, so coverage, test and test-release are red on main and on this PR. This is the same change as #1016, ported so this PR's CI can go green; it no-ops once #1016 lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012H7zqyRTeMzzAxit6xfV6r

The digest guard test is red on main: #955 added crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs to the pending list, and #690 had already moved them onto the utils::digest helpers. This ports the same three-line change as #1016, so it becomes a no-op once #1016 lands. Assisted-by: Claude Code:claude-opus-5-5

Main's test jobs are red: production_digests_go_through_the_helpers fails because PENDING_INLINE_DIGESTS still names gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs. #955 added them while those files computed digests inline, and #690 (sbt/Mill/scala-cli) then moved them onto utils::digest helpers; the two merged independently, so the ratchet now fails on stale entries on every platform. Remove the three entries so the list matches the production tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qq8uwZ9NTCZXZRygn7woy7 (cherry picked from commit 6cb46c0)

Main's test jobs are red: production_digests_go_through_the_helpers fails because PENDING_INLINE_DIGESTS still names gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs. #955 added them while those files computed digests inline, and #690 (sbt/Mill/scala-cli) then moved them onto utils::digest helpers; the two merged independently, so the ratchet now fails on stale entries on every platform. Remove the three entries so the list matches the production tree. Claude-Session: https://claude.ai/code/session_01Qq8uwZ9NTCZXZRygn7woy7 Co-authored-by: Claude <noreply@anthropic.com>

The digest guard test is red on main: #955 added crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs to the pending list, and #690 had already moved them onto the utils::digest helpers. This ports the same three-line change as #1016, so it becomes a no-op once #1016 lands. (cherry picked from commit d65c5c4)

* Start fix for #964 Assisted-by: Claude Code:claude-opus-5-5 * Stop uv projects scanning the system Python A fresh uv checkout (uv.lock with no .venv synced yet, or a UV_PROJECT_ENVIRONMENT that doesn't exist yet) and a directory holding only PEP 723 script locks fell back to the global site-packages. Every OS-Python package then joined the candidate set, so a vendored scan tried to vendor packages the project never depends on and exited 1 with pypi_uv_lock_package_missing. uv only ever installs such a project into its own env, and the lock already supplies the lock-only packages, so the crawl now returns no env for it. A uv.lock shared with Poetry, PDM or Pipenv files keeps the old fallback. Fixes #964 Assisted-by: Claude Code:claude-opus-5-5 * Route Gradle digests through utils::digest main's coverage job is red: the digest guard test from #865 requires production hashing to go through the utils::digest helpers, and the Gradle code from #646 still hashes inline. This is the same change as #878, ported so this PR's CI can go green; it no-ops once #878 lands. Assisted-by: Claude Code:claude-opus-5-5 * Revert rustfmt-only churn in files this fix doesn't touch 661c117 ran `cargo fmt --all`, which reformatted 118 files the uv fix never changes. main isn't rustfmt-clean and CI doesn't check formatting, so the sweep adds nothing. It also hides the real change and conflicts with every other open PR that touches those files. Each reverted file is byte-identical to rustfmt's output on the merge-base version, so this drops formatting only. The six files that carry the fix and the ported #878 change keep their formatting. Co-Authored-By: Claude <noreply@anthropic.com> * Drop stale digest pending-list entries Merging main brought in #955's PENDING_INLINE_DIGESTS entries for gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs, but #690 had already moved those files onto the utils::digest helpers. The guard fails on stale entries, so coverage, test and test-release are red on main and on this PR. This is the same change as #1016, ported so this PR's CI can go green; it no-ops once #1016 lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012H7zqyRTeMzzAxit6xfV6r --------- Co-authored-by: socket-patch agent <noreply@anthropic.com>

* Start fix for #907 Assisted-by: Claude Code:claude-opus-5-5 * Warn on hosted yarn classic pins berry drops A yarn 2+ install migrates a classic (v1) yarn.lock and re-resolves every entry from the registry, so a hosted pin is silently dropped and the package installs unpatched. Vendored mode already warned about this; hosted mode said nothing. The hosted engine now reads package.json beside a classic yarn.lock, and the classic rewriter warns redirect_yarn_classic_berry_migration_risk once per run when the lock carries a hosted pin, unless package.json pins yarn 1 through packageManager. The yarn 1 check is shared with the vendored probe so both modes agree. Fixes #907 Assisted-by: Claude Code:claude-opus-5-5 * Skip berry warning when no root package.json A yarn.lock with no root package.json is not a project yarn installs from, so there is nothing to warn about. This also keeps the advisory off rewriter fixtures that carry only a lock. Assisted-by: Claude Code:claude-opus-5-5 * Route Gradle digests through utils::digest main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c) * Pin yarn 1 in the yarn classic bench fixture The scan benchmark treats any warning as a failed scenario. Its yarn classic project declared no package manager, so a hosted scan now correctly warns that a yarn 2+ install would drop the pins. Declare packageManager yarn@1.22.22, as the berry fixture declares yarn 4 and as a real classic project would. The measured scan is unchanged. Assisted-by: Claude Code:claude-opus-5-5 * Drop stale entries from the digest pending list Main's test jobs are red: production_digests_go_through_the_helpers fails because PENDING_INLINE_DIGESTS still names gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs. #955 added them while those files computed digests inline, and #690 (sbt/Mill/scala-cli) then moved them onto utils::digest helpers; the two merged independently, so the ratchet now fails on stale entries on every platform. Remove the three entries so the list matches the production tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qq8uwZ9NTCZXZRygn7woy7 (cherry picked from commit 6cb46c0) --------- Co-authored-by: Claude <noreply@anthropic.com>

Start fix for #424

f18c7ee

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

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 6, 2026

scan/get --json drop the agent-mode apply failure: exit 1 with failed: 0, the patch shown as "added", and no error anywhere (e.g. a read-only global ~/.m2) #424

Closed

Claude (claude) added 3 commits October 6, 2026 18:34

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 6, 2026 18:57

cursor Bot reviewed Oct 6, 2026

View reviewed changes

Comment thread crates/socket-patch-cli/src/commands/get.rs

Comment thread crates/socket-patch-cli/src/commands/get.rs Outdated

Comment thread crates/socket-patch-cli/src/commands/get.rs

cursor Bot reviewed Oct 6, 2026

View reviewed changes

Tanmay Singla (Tanmay182003) approved these changes Oct 6, 2026

View reviewed changes

cursor Bot reviewed Oct 6, 2026

View reviewed changes

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

Mikola Lysenko (mikolalysenko) merged commit cd7dd92 into main Oct 7, 2026
465 checks passed

Mikola Lysenko (mikolalysenko) deleted the agent/fix-nested-apply-json-failures branch October 7, 2026 12:07

This was referenced Oct 7, 2026

Fix uv project with no env falling back to system Python (#964) #965

Merged

Run CI on the merge queue and stop cancelling main push runs #1018

Open

This was referenced Oct 7, 2026

Fix gem stale-install guard home selection (#1001, #729) #1002

Open

Fix .bundle/config values keeping a trailing # comment (#951) #953

Merged

Navigation Menu

Repository navigation

Fix scan/get --json dropping apply failures (#424) - #955

Fix scan/get --json dropping apply failures (#424)#955
Mikola Lysenko (mikolalysenko) merged 7 commits into
mainfrom
agent/fix-nested-apply-json-failures

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Fix

Tests (red → green)

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 6, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading