Skip to content

Decide: warn on and then remove scan --apply/--vendor, and whether --vex stays embedded #966

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.

Kind: decision. Source: review R9 and R10 (§1 #13, Part 2); register C35. The SOCKET_FORCE half of R9 is already #615 and is out of scope here.

Questions

Q1. What happens to the legacy spellings? scan --apply (= --mode agent) and scan --vendor (= --mode vendored) are hidden and called "deprecated" in the code and in CLI_CONTRACT.md. But nothing ever tells a user they are deprecated: they are accepted silently, with no warning, and the contract has no removal date. get --no-apply, get's download alias and repair's gc alias are not called deprecated: the contract makes each one a MAJOR to remove.

Options:

  1. Warn now, remove in the next MAJOR (recommended for --apply/--vendor). In this release, scan --apply/--vendor print a stderr Warning: --apply is deprecated; use --mode agent and add a deprecated_flag entry to the --json warnings[] (additive, MINOR). The next MAJOR removes both, which deletes the boolean fold in resolve_mode_flags and its conflict rules. Keep --no-apply, download and gc as permanent aliases, as the contract already promises, and stop calling anything "deprecated" that isn't scheduled for removal.
  2. Remove --apply/--vendor in the next MAJOR with no warning release. This is less work, but scripts get no notice before they break with exit 2.
  3. Keep everything and call it supported. Delete the word "deprecated" from the code and the contract, and keep the fold. Nothing is deleted.
  4. Also retire --no-apply, download and gc (warn, then remove). This is the full R9 cleanup. The contract notes that --no-apply "is widely used in existing scripts".

Q2. Should --vex stay embedded in scan, apply and vendor? R10 proposes replacing it with <command> && vex -O <path>. Verifying on main found one capability the standalone command can't reproduce. In hosted mode, scan --vex attests before install: this run's confirmed rewrites go in assume_applied, so their bytes aren't verified on disk. A standalone vex run before install would verify those packages against the unpatched tree and omit them. So option 1 below needs a replacement for that.

Options:

  1. Remove embedded --vex in the next MAJOR, and give vex a --assume-hosted (or similar) to trust lockfile-confirmed hosted pins. This deletes 5 --vex-* flags × 3 commands and the per-command glue that E42 counts.
  2. Keep embedded --vex, but route it through one shared EmbeddedVex helper (recommended). That is register E42, owned by the ecosystems auditor. It's no contract change: it only removes the duplicate glue.
  3. Keep it as it is.

Problem (main @ 9c43dfc)

  • The legacy booleans are hidden fields, apply and vendor. They are folded into --mode by resolve_mode_flags. That function carries a cross-flag conflict table that exists only because of the booleans, plus its own error wording, which a contract test matches.
  • grep -rn deprecat crates/socket-patch-cli/src finds only comments. No code path emits a deprecation notice. Run twice on a debug build: scan --apply --dry-run --yes and scan --vendor --dry-run --yes in an npm project (API pointed at a closed port) exit 0, and neither stdout nor stderr contains "deprecat".
  • CLI_CONTRACT.md calls --apply a "deprecated spelling" (L97), and says scan --apply/--vendor are "hidden (still accepted)" (L424).
  • The contract also makes --no-apply "part of the contract" and keeps gc (L188-L190), and it classes removing any alias as MAJOR (L1603-L1612).``
  • The aliases: get's download, repair's gc and --save-only's no-apply.
  • Tests that use the spellings: "--vendor" appears in 27 CLI test files and "--apply" in 8, against 4 for "--no-apply".
  • Embedded VEX:
    • VexEmbedArgs has 5 flags, each with its own env var, and is flattened into scan, apply and vendor.
    • Hosted scan --vex fills assume_applied from this run's confirmed rewrites (scan/hosted.rs).`` The standalone vex always passes an empty list, as the `VexBuildParams` doc says.

Symptoms and impact

  • No bug today. The cost is a permanent second spelling for two of the three modes, plus a conflict table and its tests.
  • Users get no notice, so a later removal would break scripts without warning.
  • The command-model decision (C34, §4) assumes these spellings are gone.

Proposed change (after the decision; option 1 for Q1 and option 2 for Q2)

  • This release:
    • one warn_deprecated(flag, replacement) call in resolve_mode_flags, which emits a stderr line and adds a deprecated_flag entry to warnings[];
    • a contract note that names the removal release;
    • a test for each spelling.
  • Next MAJOR:
    • delete the apply and vendor fields;
    • delete the boolean arms and the conflict table in resolve_mode_flags, so only the global-scope check is left;
    • move the tests from the old spellings to --mode.
    • --sync stays: it is a documented shorthand for --mode agent --prune, not a deprecated spelling.
  • Q2: no contract work. E42 (ecosystems register) carries the refactor.

Size and scope

Acceptance criteria

  • An owner answers Q1 and Q2.
  • If Q1 is option 1: scan --apply and scan --vendor warn on stderr and in --json warnings[], with a test for each, and CLI_CONTRACT.md names the removal release.
  • The code and the contract use "deprecated" only for spellings that have a removal plan.
  • cargo test -p socket-patch-cli stays green, including the scan_vendor_e2e conflict tests.

Dependencies

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 6, 2026
  2. added a commit that references this issue on Oct 6, 2026
  3. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    Let's try to clean up the unused junk before v5 goes out.

    • Retire --no-apply, download and gc
    • Remove scan --apply/--vendor
  4. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Recording the decision from the triage comment above.

    Decision

    The legacy spellings are removed in v5, with no warning release first.

    Removed in v5 Use instead
    scan --apply scan --mode agent
    scan --vendor scan --mode vendored
    get --no-apply get --save-only (SOCKET_SAVE_ONLY is unchanged)
    socket-patch download … socket-patch get …
    socket-patch gc socket-patch repair

    After this change each removed spelling is a plain clap usage error (exit 2, no JSON envelope), like any other unknown flag or subcommand. The only argv shortcut is the bare-UUID rewrite to get (lib.rs parse_argv_with_shortcuts), so download and gc will not be swallowed as identifiers.

    Staying as-is:

    • --sync is a documented shorthand for --mode agent --prune, not a deprecated spelling. It keeps its check that --mode hosted|vendored --sync is exit 2.
    • --save-only, SOCKET_SAVE_ONLY and the repair subcommand.

    Q2 (embedded --vex) is still open. The triage comment doesn't cover it, so --vex and the four --vex-* knobs on scan, apply and vendor stay unchanged for now. The shared-helper cleanup is tracked separately as E42. If you want embedded --vex removed before v5 too, say so here. That needs a vex flag that trusts the hosted pins the lockfile confirms. Without it, hosted scan --vex can't attest before install (assume_applied, scan/hosted.rs).

    What changes (main @ db83f014)

    • crates/socket-patch-cli/src/commands/scan/mod.rs
      • Delete the hidden apply (L267-270) and vendor (L287-291) fields and their conflicts_with_all.
      • Shrink resolve_mode_flags (L184-246) to three things: the --sync vs different --mode check, --sync → agent, and the hosted default plus the global-scope check.
      • Drop the "deprecated spelling" comments on ScanMode and --mode.
    • crates/socket-patch-cli/src/commands/get.rs L420-427: drop alias = "no-apply".
    • crates/socket-patch-cli/src/lib.rs: drop visible_alias = "download" (L74) and visible_alias = "gc" (L109).
    • Comments in scan/{hosted,vendor_flow,discovery}.rs and apply.rs that say --apply/--vendor move to --mode agent/--mode vendored.
    • Tests:
      • Move every "--vendor" (27 files) and "--apply" (8 files) invocation to --mode vendored/--mode agent.
      • Same for the --no-apply uses in e2e_{npm,gem,pypi}.rs (move to --save-only).
      • Drop the apply/vendor fields from ScanArgs struct literals. The 3 that set true move to mode: Some(…).
      • Retire the alias tests in cli_parse_get.rs, cli_parse_main.rs and output_modes_e2e.rs, and the boolean conflict arms in covgap_commands_scan_mod.rs and scan_vendor_e2e.rs.
      • Add one test asserting that all five removed spellings exit 2.
    • Wrappers: none to change. The npm wrapper forwards argv untouched, and nothing in npm/, scripts/, .github/ or tests/ uses these spellings.
    • CLI_CONTRACT.md:
      • Clear the alias column for get/repair (L23, L27).
      • Remove the boolean spellings from the scan flag rows and the mode-resolution text (L96-99, L127, L135, L160, L429).
      • Drop the --no-apply and gc paragraphs (L190-192) and the get row's alias note (L105).
      • Rewrite the alias rows of the bump table (L1676-1677, L1684-1685) generically.
      • Replace the remaining scan --apply/scan --vendor references with the --mode spelling.
    • docs/migrating-to-v5.md: add the five rows above to "Retired spellings". docs/usage.md already uses only --mode/--save-only. It gets re-checked, with no edit expected.

    Coordination: #792 (--download-mode default) touches args.rs, the contract's globals and bump tables, and the same migration table, but not get's flags. Whichever PR lands second rebases the doc tables.

    Outside this repo, these consumers use the removed spellings and need updating when they bump to v5:

    • depscan: package.json scripts socket-patch download, and docs/patches.md (get --no-apply, gc).
    • socket-cli: integration tests socket patch download --dry-run.

    A PR implementing this will follow.


    Generated by Claude Code

  5. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Implemented in #1031.


    Generated by Claude Code

  6. added
    v5-blockerMust resolve before v5: public interface/migration or ordinary patch-install-undo failure.
    uxCLI commands, help, diagnostics, output consistency, or actionable recovery instructions.
    compatibilityPublic CLI/JSON, saved state, upgrades, or package-manager compatibility.
    and removed on Oct 9, 2026
  7. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    v5 release blocker (P1). The flag/alias removals landed in #1031, but current depscan scripts still invoke socket-patch download. Finish downstream migration before their v5 upgrade; retain embedded --vex as currently supported.

    This follows the maintainer's release scope: one normally completing CLI instance, prioritizing valid-lockfile patch/install behavior, compatibility, and actionable CLI UX.

    Verified current downstream calls: depscan package.json, local/staging scripts still invoke socket-patch download; socket-cli integration test still invokes patch download. Migrate the actual call sites and assert the expected command succeeds rather than accepting any nonnegative exit code. The v5 spelling removals are intentional; this does not propose restoring the old aliases.

  8. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming the downstream migration for v5 blocker burn-down (shared root cause: depscan and socket-cli still invoke the removed download alias). Branches: socket-patch-v5-migrate-download in SocketDev/depscan and SocketDev/socket-cli. Claim-ID: 2026-10-09T16:42:28Z-710d9d

  9. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Downstream migration PRs for the v5 spelling removals (embedded --vex unchanged):

    • SocketDev/depscan#27676: socket-patch-local/socket-patch-staging scripts now run socket-patch get instead of socket-patch download; docs/patches.md moves off get --no-apply, --one-off and the gc alias; one test comment moves from scan --vendor to scan --mode vendored.
    • test(patch): use get instead of the removed download alias socket-cli#1600: test/integration/binary/js.test.mts runs patch get --dry-run and now requires exit 0 plus the forwarded get express arguments, instead of accepting any exit code.

    Both spellings also work on the socket-patch versions those repos use today, so the PRs can land before the v5 bump. Neither PR is merged. This issue can close once both land.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:claimedagent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)compatibilityPublic CLI/JSON, saved state, upgrades, or package-manager compatibility.priority:p1refactorStructural change: duplicated code or logic, missing abstraction, layering, dead codeuxCLI commands, help, diagnostics, output consistency, or actionable recovery instructions.v5-blockerMust resolve before v5: public interface/migration or ordinary patch-install-undo failure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions