I read through this change carefully. It does two concrete things: adds split:"false" to every []string flag that was being silently comma-split (fixing the root bug), and adds a rest:"true" positional-args field to EnvSet so users can write miren env set KEY=VALUE KEY=VALUE without -e. Both changes are wired correctly and consistently through the three affected commands (env set, deploy, auth provider add github).
What I checked:
split:"false" fix is applied to all four []string flags that needed it (Env, Sensitive in both env.go and deploy.go, and Orgs in auth_provider_github.go). I didn't find any other []string flag that handles comma-sensitive values that was left un-fixed.EnvSet of bare args into env via slices.Concat(opts.Env, opts.Args) is correct. The envSetReference path, the "nothing specified" guard, and ParseEnvVarSpecs all consistently use the merged slice.= (e.g. miren env set MYVAR) flows into parseEnvVarSpec, which already treats any arg without = as a "prompt for value" case — that's existing behavior, not a regression from this PR.env_set_flags_test.go are well-targeted: they exercise comma preservation in flag values, interleaved bare args and flag args being routed to the right fields, and the GitHub org team-list round-trip through buildGitHubConfigJSON. The TestAuthProviderGitHubOrgTeamsSurviveParsing test is especially useful because it validates the semantic impact of the split fix, not just the parsed slice.go.mod/go.sum) is what actually delivers the split:"false" feature; the hashes look consistent and there are no other unexpected dependency changes.commands.go, docs/docs/command/env-set.md) accurately describes both features.The change is clean and self-contained. I'm comfortable approving it.
Verdict: ready
{
"owner": "mirendev",
"repo": "runtime",
"number": 1197,
"verdict": "ready",
"event": "approve",
"summary": "I read through this change carefully. It does two concrete things: adds `split:\"false\"` to every `[]string` flag that was being silently comma-split (fixing the root bug), and adds a `rest:\"true\"` positional-args field to `EnvSet` so users can write `miren env set KEY=VALUE KEY=VALUE` without `-e`. Both changes are wired correctly and consistently through the three affected commands (`env set`, `deploy`, `auth provider add github`).\n\n**What I checked:**\n\n- The `split:\"false\"` fix is applied to all four `[]string` flags that needed it (`Env`, `Sensitive` in both `env.go` and `deploy.go`, and `Orgs` in `auth_provider_github.go`). I didn't find any other `[]string` flag that handles comma-sensitive values that was left un-fixed.\n- The merge in `EnvSet` of bare args into `env` via `slices.Concat(opts.Env, opts.Args)` is correct. The `envSetReference` path, the \"nothing specified\" guard, and `ParseEnvVarSpecs` all consistently use the merged slice.\n- A bare positional arg without `=` (e.g. `miren env set MYVAR`) flows into `parseEnvVarSpec`, which already treats any arg without `=` as a \"prompt for value\" case — that's existing behavior, not a regression from this PR.\n- The three new tests in `env_set_flags_test.go` are well-targeted: they exercise comma preservation in flag values, interleaved bare args and flag args being routed to the right fields, and the GitHub org team-list round-trip through `buildGitHubConfigJSON`. The `TestAuthProviderGitHubOrgTeamsSurviveParsing` test is especially useful because it validates the *semantic* impact of the split fix, not just the parsed slice.\n- The mflags dependency bump (`go.mod`/`go.sum`) is what actually delivers the `split:\"false\"` feature; the hashes look consistent and there are no other unexpected dependency changes.\n- Documentation (`commands.go`, `docs/docs/command/env-set.md`) accurately describes both features.\n\nThe change is clean and self-contained. I'm comfortable approving it.",
"posted_to_pr": true
}