SP-1383: gate early-access commands behind feature flag, replace beta#393
SP-1383: gate early-access commands behind feature flag, replace beta#393A. Jashari (admirjashari) wants to merge 3 commits into
Conversation
Add an earlyAccess(featureKey?) marker (replacing beta): commands with a key are checked against the backend feature flag at run time and blocked with a clear message when not enabled for the team. No labels shown in help. Consolidates the old beta marker and removes the (beta) help decoration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A. Jashari (admirjashari)
force-pushed
the
SP-1383-early-access-feature-flag
branch
from
c2886f8 to
5c19af6
Compare
…de lines Sonar counts pre-existing module-handler code (discoverAndRegisterModules, argument) as new code for this PR due to the project's New Code Definition. Cover it with tests to clear the coverage gate. No source changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
|
||
| public register(context: Context, configurator: Configurator): void { | ||
| const deploymentCommand = configurator.command("deployment").beta() | ||
| const deploymentCommand = configurator.command("deployment").earlyAccess() |
There was a problem hiding this comment.
FYI, the deployment feature is GA, so this beta() marker was already a leftover, and moving it off here is the right cleanup. One thing still hanging around: docs/user-guide/deployment-commands.md is titled "Deployment Commands (beta)", so the docs and the CLI now disagree. We know fully removing the beta tag isn't the point of this PR, so feel free to leave it for a followup PR if it's too much hassle to fold in here.
| */ | ||
| public earlyAccess(featureKey?: string): this { | ||
| const command = this.cmd as any | ||
| command.isEarlyAccess = true; |
There was a problem hiding this comment.
Small thing that might trip us up later: isEarlyAccess gets set here but nothing reads it anymore. It was only ever used by the old CustomHelp to render the (beta) label, and that's gone in this PR. The doc comment also says "not shown in help", but that isn't actually true now: these commands still appear in --help, just without a label. Could we pick one direction, either genuinely hide them (cmd.hidden = true) or drop the unused flag?
(nit: missing semicolon after const command = this.cmd as any.)
|
|
||
| let enabled: boolean; | ||
| try { | ||
| enabled = await new FeatureFlagService(this.ctx).isEnabled(featureKey); |
There was a problem hiding this comment.
Bigger-picture question on the approach: this turns the CLI into a second source of truth for "is this command allowed". It reads /api/team/features and decides, while the command's real authorization still lives on the backend. If those two ever drift (flag present but the call still 403s, or the command works without the flag), the user is back to a confusing experience. Did we weigh the other direction, letting the command run and translating its actual 403 into the friendly "not enabled, contact support" message? That keeps a single source of truth, avoids the extra round-trip, and can't fall out of sync with the backend. This approach isn't wrong, but since nothing uses a key yet, it feels like a good moment to settle on which model we want.
| throw error; | ||
| } | ||
| throw new GracefulError( | ||
| `Could not verify whether '${this.cmd.name()}' is enabled for your team. Please try again later or contact support.` |
There was a problem hiding this comment.
A couple of edge cases worth thinking through here:
- This is fail-closed, so if
/api/team/featuresis down or slow, even a team that is entitled gets blocked. For an early-access gate that's probably an acceptable trade-off, but let's make it a deliberate one. - An expired or invalid token comes back as a 401 and gets reported as "could not verify whether X is enabled", which points the user at the wrong problem. Might be worth letting real auth errors surface as auth errors.
| * Marks a command as early access (internal, not shown in help). With a feature | ||
| * key, the command is gated at run time on that backend flag. | ||
| */ | ||
| public earlyAccess(featureKey?: string): this { |
There was a problem hiding this comment.
Zooming out from deployments: collapsing beta() and betaOption() into earlyAccess() also removes our only way to tell a user "this command (or option) is beta and may change". Deployment is GA so it doesn't need that, but I think the beta/betaOption capability (or something equivalent) is worth keeping around for future commands that genuinely are early-access or unstable. betaOption in particular was unused, but the ability to flag a single option that way is the kind of thing we'll likely want again rather than rebuild later. Could we keep the mechanism even if nothing uses it today?
4 participants
Description
Gate early-access (non-GA) commands behind a backend feature flag. If a team runs a gated command that isn't enabled for them, they get a clear "not enabled — contact support" message instead of a generic 403.
Also consolidates the old
betamarker into a singleearlyAccess:earlyAccess(featureKey?)replacesbeta()/betaOption()— with a key it's gated at run time, without one it's just an internal marker.(beta)label anymore; early-access status is internal (nothing shown in--help)./api/team/featuresendpoint with the profile token (flag enabled = key present).deploymentcommands migrated to.earlyAccess()(ungated, behavior unchanged).Relevant links
Checklist