Repository navigation
Remove the SOCKET_FORCE env binding; --force is flag-only (#615) - #1021
Draft
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Conversation
`apply --force`, `vendor --force` and `--update --force` all bound the same SOCKET_FORCE variable, so exporting it for one command quietly weakened checks in the others. Nothing sets it: no hook, wrapper, installer, workflow or other SocketDev repo. Drop the env binding from all three flags and from LOCAL_ARG_ENV_VARS. The variable is now ignored without a warning, like the other env vars v5 retires; a stale export leaves the beforeHash check and the managed-install refusal on. Replace the vendor env-wiring tests with one that pins SOCKET_FORCE as ignored, and add an args.rs regression test covering apply, vendor, self-update and --update (env ignored, --force still works). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…615) Set the env column of the apply/vendor --force rows to none, drop the env note from `--update --force` and the env-var table row, and list SOCKET_FORCE under the contract's removed env vars and the migration guide's retired spellings. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The intro still said only the three v3/v4 aliases were removed in v5, while the Removed env vars section it links to now also lists SOCKET_FORCE. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4 tasks
Collaborator
Author
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 436149f. Configure here.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What and why
Removes the
SOCKET_FORCEenvironment variable.--forcestays onapply,vendorand--update(self-update) as a flag only.The maintainer asked on #615 whether we still need
SOCKET_FORCE. The answer was no, and the decision was: no per-command force variables, removeSOCKET_FORCEin v5, keep--forceas a flag. Reasons:apply --force,vendor --force,--update --force). Core never reads it.install.sh, CI workflows,scripts/and depscan have no reference to it. An org-wide search finds it only in this repo.--force.Removal fails closed. A stale
SOCKET_FORCE=1export now does nothing, so hash checks and the managed-install refusal stay on. Nothing warns when the variable is set.User-visible changes
SOCKET_FORCEis ignored byapply,vendorand--update. Pass--forceto the command that needs it.Code
env = "SOCKET_FORCE"from theforcearg incommands/apply.rs,commands/vendor.rsandcommands/update.rs.args.rs: removedSOCKET_FORCEfromLOCAL_ARG_ENV_VARSand its three rows fromBOOL_BINDINGS. New unit testsocket_force_env_is_ignored_and_force_flag_still_works(env set to 1/true/yes leavesforce=falseon apply, vendor, self-update and--update;--forcestill sets it).tests/cli_parse_vendor.rs: the four env-wiring tests are replaced byenv_socket_force_is_ignored("1", "true", "yes", ""). The scrub-list entries stay for hermeticity.Docs updated
crates/socket-patch-cli/CLI_CONTRACT.md:apply/vendor--forcerows have no env var;--update --forceis "Flag only"; the env-var table row is removed;SOCKET_FORCEis listed under "Removed env vars" and in the env-var section intro.docs/migrating-to-v5.md: new row in "Retired spellings".CHANGELOG.mdis not touched (written at release time).Tests run
cargo test -p socket-patch-cli --lib args::: 43 passed.cargo test -p socket-patch-cli --test cli_parse_vendor --test cli_parse_vex --test cli_parse_apply --test cli_parse_main --test help_text_hygiene: all passed.cargo clippy -p socket-patch-cli --all-targets: no new warnings in changed files.Review findings fixed
SOCKET_FORCEtoo (436149f).value_parser = parse_bool_flagon the three--forceflags is now redundant but harmless; kept to keep the diff small.Coordination
vendor --forcehelp text and edits the same contract row. That rewrite is not in this PR.args.rstables. Whichever lands second needs a rebase.Closes #615
🤖 Generated with Claude Code
Generated by Claude Code
Note
Low Risk
Intentional v5 contract change with migration docs; behavior fails closed when the env var is set, and flags are unchanged.
Overview
v5 removes
SOCKET_FORCE:--forceonapply,vendor, and--updateis flag-only; the env var is ignored with no warning, so a stale export no longer bypasses beforeHash checks or managed-install refusal on unrelated commands.Implementation drops
env = "SOCKET_FORCE"from the three clapforceargs, removes the var fromLOCAL_ARG_ENV_VARS, and replaces env-wiring tests with coverage thatSOCKET_FORCEis ignored while--forcestill parses. CLI_CONTRACT.md and migrating-to-v5.md document the retirement.Reviewed by Cursor Bugbot for commit 436149f. Configure here.