GeezerVison is showing github.com — Remove the SOCKET_FORCE env binding; --force is flag-only (#615) by mikolalysenko · Pull Request #1021 · SocketDev/socket-patch
Conversation
What and why
Removes the SOCKET_FORCE environment variable. --force stays on apply, vendor and --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, remove SOCKET_FORCE in v5, keep --force as a flag. Reasons:
Removal fails closed. A stale SOCKET_FORCE=1 export now does nothing, so hash checks and the managed-install refusal stay on. Nothing warns when the variable is set.
User-visible changes
SOCKET_FORCE is ignored by apply, vendor and --update. Pass --force to the command that needs it.
- No change to flags, defaults or JSON output.
Code
- Dropped
env = "SOCKET_FORCE" from the force arg in commands/apply.rs, commands/vendor.rs and commands/update.rs.
args.rs: removed SOCKET_FORCE from LOCAL_ARG_ENV_VARS and its three rows from BOOL_BINDINGS. New unit test socket_force_env_is_ignored_and_force_flag_still_works (env set to 1/true/yes leaves force=false on apply, vendor, self-update and --update; --force still sets it).
tests/cli_parse_vendor.rs: the four env-wiring tests are replaced by env_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 --force rows have no env var; --update --force is "Flag only"; the env-var table row is removed; SOCKET_FORCE is listed under "Removed env vars" and in the env-var section intro.
docs/migrating-to-v5.md: new row in "Retired spellings".
CHANGELOG.md is 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
- The CLI_CONTRACT env-var intro still said only the three v3/v4 aliases were removed in v5. It now names
SOCKET_FORCE too (436149f).
- Left as is:
value_parser = parse_bool_flag on the three --force flags is now redundant but harmless; kept to keep the diff small.
Coordination
Closes #615
🤖 Generated with Claude Code
Note Low Risk
Intentional v5 contract tightening with fail-closed behavior for stale env; explicit --force behavior is unchanged.
Overview
v5 removes SOCKET_FORCE: --force on apply, vendor, and --update is flag-only—a stale shell export no longer bypasses beforeHash checks, variant probes, or managed-install refusal across unrelated commands.
Clap env = "SOCKET_FORCE" bindings are dropped from those three subcommands; LOCAL_ARG_ENV_VARS and bool-env tests are updated, with new coverage that the env var is ignored while --force still works. CLI_CONTRACT and migrating-to-v5 document the retirement; vendor parse tests now assert ignorance instead of env wiring.
No change to flag semantics when you pass --force explicitly; JSON and defaults stay the same.
Reviewed by Cursor Bugbot for commit 2793363. Configure here.
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
`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) <[email protected]>
…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) <[email protected]>
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) <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Resolve the CLI_CONTRACT.md per-subcommand table conflict: keep main's
expanded apply --check description and drop the SOCKET_FORCE env column
from the apply/vendor --force rows (#615).
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
[agent] Merged main, CI green; ready for review.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 2793363. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent).
- Head:
27933631d4c51dc7d3de4390ee89f063b3ce8482
- CI: 472/472 check runs green (success/skipped/neutral) on this head, mergeable, no conflicts.
- Bugbot: reviewed this head (Cursor Bugbot check: success), no unresolved review threads.
- Changelog: untouched.
Nothing specific flagged for the reviewer beyond the PR description.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
arch-refactor
PR opened by the scheduled architecture refactor routine
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
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.
|