Repository navigation
Description
Description
The Go SDK has 14 functional option types applied at 16 call sites across six
packages. Only one of them, ApplyCreateOptions, guards against a nil option.
Every other apply loop calls opt(&cfg) unconditionally and panics with a nil
pointer dereference if a caller passes a nil option.
Two things should be decided and then applied consistently:
-
Nil policy. Either ignore nil options everywhere, or document that a nil
option is a programming error and let it panic. Right now the SDK does one
thing inCreateand the opposite everywhere else, which is the worst of
both options because callers cannot rely on either behavior. -
Apply-loop duplication. Six
Apply*Optionshelpers live intypes/,
whileoidc,fake,gateway,edge, and thev1transport clients each
hand-roll the same loop inline. A single generic helper would collapse all of
them and make the nil policy a one-line decision instead of a 16-site edit.
Context
This came out of review on #3198, which replaced the positional labels
parameter on Create() and CreateFromTemplate() with functional options.
Because the removed parameter sat immediately before the variadic, existing
calls like Create(ctx, ws, name, spec, nil) still compiled and began panicking
at runtime. That was fixed in #3198 by nil-guarding ApplyCreateOptions only,
since it was the one type with an actual migration hazard.
The remaining types have no such hazard, so there is no urgency here. This is a
consistency and API-design question rather than a bug.
Current state
| Location | Applier | Nil-guarded |
|---|---|---|
types/options.go:34 |
ApplyCreateOptions |
yes |
types/log.go:72 |
ApplyLogOptions |
no |
types/policy.go:267,304,348,400 |
ApplyGetDraftOptions, ApplyApproveAllOptions, ApplyGetStatusOptions, ApplyListPolicyOptions |
no |
oidc/oidc.go:36, oidc/device.go:49, oidc/credentials.go:40 |
inline | no |
fake/fake.go:100 |
inline | no |
gateway/gateway.go:30 |
inline | no |
ssh_client.go:67, tcp_client.go:46,106 |
inline | no |
edge/tunnel.go:110, auth_refresh.go:163 |
inline | no |
Definition of Done
- A single nil policy is chosen and recorded in the SDK docs
- The policy is applied uniformly across all 16 apply sites
- Duplicated apply loops are consolidated behind a shared helper, or a note
explains why per-package loops are kept - Tests cover the chosen behavior for at least one option type per package
Activity
krishicks commentedon Sep 8, 2026
I think this doesn't really need an issue; if it's a problem, I would just fix it.
rhuss commentedon Sep 9, 2026
@krishicks good point, I've just added #3232 as fix, should close this issue when merged.
refactor(sdk/go): unify functional-option handling with shared applier
refactor(sdk/go): unify functional-option handling with shared applier (