Skip to content

Add NotForCommand helper and fix Exercism plugin auth gating - #665

Merged
AJaccP merged 1 commit into
mainfrom
aditya/619-exercism-needs-auth-new-helper
Sep 17, 2026
Merged

AJaccP merged 1 commit into
mainfrom
aditya/619-exercism-needs-auth-new-helper

Conversation

@AJaccP

@AJaccP AJaccP commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Overview

The Exercism plugin passed four subcommand names to a single NotForExactArgs call, 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> and exercism troubleshoot --full-api-key gated, 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 via ForCommand. This PR adds NotForCommand as its negation and uses one call per exempt subcommand. It's purely additive — no existing helper changed, so no other plugin's NeedsAuth decision can shift. Anchoring at command position rather than contains-matching also keeps exercism submit completion gated, since submit takes free-form file arguments and completion is an ordinary word.

Also included: on current main, TestForCommand never calls ForCommand — its body is a copy of the NotWhenContainsArgs test above it — leaving the SDK's only prefix matcher without direct coverage. It's rewritten here against ForCommand, with TestForNestedCommand added for the multi-token form.

Type of change

  • Created a new plugin
  • Improved an existing plugin
  • Fixed a bug in an existing plugin
  • Improved contributor utilities or experience

Related Issue(s)

How To Test

Unit tests, including a regression test for the reported bug:

go test ./sdk/needsauth/ ./plugins/exercism/ -v

TestExercismCLINeedsAuth covers the previously-broken invocations, the argument-bearing forms, and guards that the API-backed subcommands stay gated.

End to end with the CLI:

op plugin init exercism

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 init will 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:

exercism completion bash
exercism upgrade
exercism workspace
exercism workspace --verbose
exercism troubleshoot
exercism troubleshoot --full-api-key

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 prompts

That 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, and troubleshoot subcommands.

@AJaccP
AJaccP requested a review from JillRegan September 17, 2026 15:30
@AJaccP
AJaccP merged commit 343f43d into main Sep 17, 2026
3 checks passed
@AJaccP
AJaccP deleted the aditya/619-exercism-needs-auth-new-helper branch September 17, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exercism: NotForExactArgs called with multiple args skips auth for no real subcommand

3 participants