Skip to content

fix(config): derive enum flag usage text from validator values - #406

Merged
xvzc merged 1 commit into
xvzc:mainfrom
ayzabar:fix/enum-flag-usage-from-validator
Sep 28, 2026
Merged

xvzc merged 1 commit into
xvzc:mainfrom
ayzabar:fix/enum-flag-usage-from-validator

Conversation

@ayzabar

@ayzabar ayzabar commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Problem

The usage hints for the enum-valued flags are hardcoded string literals, and some
have drifted away from the values their validators accept. Following --help
fails outright:

$ spoofdpi --dns-mode doh
application failed to start
invalid value "doh" for flag -dns-mode: value 'doh' is invalid (allowed: udp, https, system)
Flag --help advertises Validator accepts
--dns-mode <'udp'|'doh'|'sys'> udp, https, system
--https-split-mode <"sni"|"random"|"chunk"|"sni"|"custom"|"none"> sni, random, chunk, first-byte, custom, none

For --dns-mode, only udp is correct — DoH is https and sys is system.
docs/user-guide/dns.md already documents --dns-mode "https", so the help
output is the odd one out. For --https-split-mode, "sni" is listed twice
while first-byte is missing entirely.

Separately, the --https-chunk-size description refers twice to a flag named
https-split-default, which does not exist; the flag is https-split-mode.

Fix

Render the hints from the same available*Values slices the validators check
against, through a small enumUsage() helper, so the text cannot drift away
from the behaviour again. Applied to all four enum flags — --app-mode and
--dns-qtype were already accurate, but they can no longer rot either.

-  --dns-mode string <'udp'|'doh'|'sys'>
+  --dns-mode string <"udp"|"https"|"system">
-  --https-split-mode string <"sni"|"random"|"chunk"|"sni"|"custom"|"none">
+  --https-split-mode string <"sni"|"random"|"chunk"|"first-byte"|"custom"|"none">

No behaviour changes — help text only.

Tests

TestCreateCommand_EnumUsageMatchesValidator asserts that each enum flag's
usage hint lists exactly the values its validator accepts, reading the usage
back through cli.DocGenerationFlag. I verified it is a real regression test by
restoring the old <'udp'|'doh'|'sys'> literal and watching it fail.

make test, make lint and make fmt-check all pass locally (Go 1.27.1, darwin/arm64).

🤖 Generated with Claude Code

The usage hints for the enum-valued flags were hardcoded and had drifted
away from the values their validators actually accept:

  --dns-mode          says <'udp'|'doh'|'sys'>, accepts udp, https, system
  --https-split-mode  lists "sni" twice and omits "first-byte"

Following the help text fails: `--dns-mode doh` is rejected with
`allowed: udp, https, system`, and docs/user-guide/dns.md correctly
documents `--dns-mode "https"`.

Render the hints from the same available*Values slices the validators
check against, so the two cannot drift apart again, and apply it to
--app-mode and --dns-qtype as well. Also fix two references to a
nonexistent 'https-split-default' flag in the --https-chunk-size text;
the flag is named --https-split-mode.

Add a regression test asserting that every enum flag's usage hint lists
exactly the values its validator accepts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ayzabar

ayzabar commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Heads up that the red commitlint check here is failing inside the workflow, not on
this PR's contents — flagging it so it doesn't read as a message-format problem.

Lint PR runs on pull_request_target and then checks out the fork's head SHA, and
actions/checkout now refuses that combination:

##[error]Refusing to check out fork pull request code from a 'pull_request_target'
workflow. This workflow runs with the base repository's GITHUB_TOKEN, secrets,
default-branch cache scope, and runner access. ... To opt in, review the risks at
https://gh.io/securely-using-pull_request_target and set 'allow-unsafe-pr-checkout:
true' on the actions/checkout step.

(failing step)

It dies at the checkout step, before commitlint ever runs, so every fork PR hits it
right now. #405 passed the same workflow back in July — actions/checkout@v4 is a
floating tag, so the guard landed in a v4 patch release since then.

Running this repo's own .commitlintrc.js and the workflow's two commands locally:

$ echo "fix(config): derive enum flag usage text from validator values" | npx commitlint --verbose
✔   found 0 problems, 0 warnings

$ npx commitlint --from 90441f46c36ba1a23ee391573138e80ec5a18eb2 --to e23ec59bcf9191430256f81cb0031629e07d6571 --verbose
✔   found 0 problems, 0 warnings

Happy to send a separate PR for the workflow if that's useful. commitlint only needs
the PR title and the commits' metadata, so the job never needs the fork's working
tree — it can lint from fetched refs without checking out fork code, which avoids
opting into allow-unsafe-pr-checkout at all. Entirely your call on the approach
though, it's your CI and your threat model.

The CI run is waiting on first-time-contributor approval; make test, make lint
and make fmt-check all pass locally (Go 1.27.1, darwin/arm64).

@xvzc

xvzc commented Sep 27, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR and for flagging the commitlint CI issue! I'll fix the workflow soon.

@xvzc
xvzc self-requested a review September 27, 2026 13:00
@xvzc

xvzc commented Sep 27, 2026

Copy link
Copy Markdown
Owner

LGTM

@xvzc
xvzc merged commit ee86fa2 into xvzc:main Sep 28, 2026
7 of 8 checks passed
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.

2 participants