Add NotForCommand helper and fix Exercism plugin auth gating - #665
Merged
Merged
Conversation
rr3khan
approved these changes
Sep 17, 2026
JillRegan
approved these changes
Sep 17, 2026
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.
Overview
The Exercism plugin passed four subcommand names to a single
NotForExactArgscall, which matches the whole argv as one ordered command rather than as a set of alternatives. Auth was therefore skipped only for an invalid four-word invocation, while every real setup subcommand still demanded a 1Password item it never reads.Splitting that into one call per subcommand still leaves commands like
exercism completion <shell>andexercism troubleshoot --full-api-keygated, because exact matching rejects any invocation with trailing arguments. What these subcommands need is prefix matching, which the SDK only had in the auth-required direction viaForCommand. This PR addsNotForCommandas its negation and uses one call per exempt subcommand. It's purely additive — no existing helper changed, so no other plugin'sNeedsAuthdecision can shift. Anchoring at command position rather than contains-matching also keepsexercism submit completiongated, sincesubmittakes free-form file arguments andcompletionis an ordinary word.Also included: on current
main,TestForCommandnever callsForCommand— its body is a copy of theNotWhenContainsArgstest above it — leaving the SDK's only prefix matcher without direct coverage. It's rewritten here againstForCommand, withTestForNestedCommandadded for the multi-token form.Type of change
Related Issue(s)
How To Test
Unit tests, including a regression test for the reported bug:
go test ./sdk/needsauth/ ./plugins/exercism/ -vTestExercismCLINeedsAuthcovers the previously-broken invocations, the argument-bearing forms, and guards that the API-backed subcommands stay gated.End to end with the CLI:
No real Exercism token is needed for this. The exempt subcommands never read the credential, and the gated ones only need to reach 1Password, so a dummy item is enough —
op plugin initwill walk you through creating one. The API key field expects 36 characters of lowercase letters, digits, and hyphens, so any UUID-shaped value works, e.g.11111111-2222-3333-4444-555555555555. What we're checking is when 1Password is asked for the credential, not whether the command itself succeeds.These should run with no 1Password prompt:
These should still prompt:
exercism download --exercise=two-fer --track=go exercism submit solution.go exercism submit completion # prompts, then fails on the missing file — the point is that it promptsThat last one is the regression guard for the over-exemption described above.
Changelog
The Exercism plugin no longer requires a 1Password item for the
completion,upgrade,workspace, andtroubleshootsubcommands.