Skip to content

Drop stale release-channel and Statistics API preview language - #22

Merged
zmofei merged 6 commits into
mainfrom
docs/release-channel-serving
Sep 15, 2026
Merged

zmofei merged 6 commits into
mainfrom
docs/release-channel-serving

Conversation

@zmofei

@zmofei zmofei commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Summary

  • The README said the cli.mapbox.com install channel was not serving yet. Verified it now is: install.sh/install.ps1 return 200, and latest/manifest.json lists a live 0.2.1 release with checksummed artifacts for all five targets. Removed the stale note (the working commands already show usage).
  • The Statistics API (mapbox usage) is generally available now, not gated behind a Mapbox-support enablement step — confirmed directly by the Statistics API team. Dropped the "private preview" / support-enablement language from the OpenAPI spec, docs/commands.md, the CLI help text, and the 403 error remedy.
  • Removed the now-unused ACCOUNT_USAGE feature flag: it was a no-op (switch already true everywhere) now that the API it gated is GA. statistics:read is folded directly into scopes, mapbox usage registers unconditionally, and the feature_flags module is gone since nothing used it anymore.
  • Bumped the version to 0.2.2 (patch — no breaking change per CONTRIBUTING.md#compatibility) via scripts/prepare-release.sh 0.2.2.
  • Fixed the 401/403 mismatch in the mapbox usage error remedy (see review discussion): the API answers 403, not 401, when a token is only missing statistics:read — confirmed live, including that 401 is reachable for a genuinely invalid token. The 403 fix now leads with "run mapbox auth login again" before falling back to Mapbox support.

Test plan

  • curl -fsSL https://cli.mapbox.com/install.sh returns 200 with real script content
  • curl https://cli.mapbox.com/latest/manifest.json returns a populated manifest for 0.2.1
  • curl https://api.mapbox.com/statistics/v1?access_token=<invalid> returns 401 Not Authorized - Invalid Token, confirming 401 is reachable and distinct from the scope/access 403s
  • cargo test (full suite, including docs_contract and account_usage)
  • cargo clippy --locked --all-targets -- -D warnings

The install channel at cli.mapbox.com started serving real builds
with the 0.2.1 release (manifest confirmed live), so the "not
serving yet" note is stale.
The version that first started serving is history, not usage
guidance; the commands above are enough.
The Statistics API is generally available now, not gated behind
Mapbox support enabling an account. Update the spec, docs, CLI help
text, and error remedy accordingly; a 403 is no longer explained as
a preview-enablement gate.
@zmofei zmofei changed the title Update README: release channel is now serving Update README: release channel serving; drop stale statistics API preview note Sep 15, 2026
It was a no-op (switch already true everywhere) now that the
Statistics API it gated is generally available and `mapbox usage`
ships unconditionally. Fold statistics:read directly into
DEFAULT_SCOPES and register the command unconditionally; drop the
feature_flags module since nothing uses it anymore.
@zmofei zmofei changed the title Update README: release channel serving; drop stale statistics API preview note Drop stale release-channel and Statistics API preview language Sep 15, 2026
@zmofei
zmofei marked this pull request as ready for review September 15, 2026 16:07
@zmofei
zmofei requested a review from a team as a code owner September 15, 2026 16:07

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good change, and the test plan is the right shape — claims about a live
channel and a live API, each checked rather than asserted. I verified the ones
I could: 595 tests pass, clippy clean, Cargo.toml at 0.2.2, feature_flags
is gone with no dangling references, and statistics:read is in
DEFAULT_SCOPES.

Removing the flag is clearly right. It was the only entry in flags::ALL with
its switch already true, so the mechanism had no live user and its module
docs described a gate that gated nothing.

Requesting one change, on the strength of running it.

The 403 remedy misdiagnoses the case the API actually uses 403 for

mapbox usage on this branch, with a token that predates the scope:

{"code":"http_403",
 "message":"This API requires a token with statistics:read scope.",
 "fix":"This account doesn't have access to the Statistics API; contact Mapbox support if that's unexpected.",
 "status":403}

The API says the problem is a scope. The fix sitting next to it says the
problem is account access, and sends the reader to support — for something
they can fix themselves with mapbox auth login.

The advice for that case does exist, but it is on the wrong branch:

401 => Remedy::default().with_fix(
    "Check the token has the `statistics:read` scope — run `mapbox auth login` again \
     if it predates that scope, or pass one from account.mapbox.com with --token.",
),
403 => Remedy::default().with_fix(
    "This account doesn't have access to the Statistics API; contact Mapbox support …",
),

The API returns 403 for a missing scope, not 401. So the scope remedy is
attached to a status this endpoint doesn't use for that, and the one that
fires gives advice that cannot help.

To be fair: this PR doesn't introduce it. The old 403 text — "a private
preview: ask Mapbox support to enable it" — was also support-only, so the
mismatch predates you. Two things make it worth fixing here anyway. This is
the PR touching that remedy. And the change removes what used to compensate
for it: with the preview framing gone, support is now the whole of the
advice, where before a reader had the surrounding "needs the statistics:read
scope" language nearby to reinterpret it.

The help text still carries the right explanation, and it's good —

the token needs the statistics:read scope. mapbox auth login requests it
by default; log in again if your stored token predates that.

— but that's the text someone reads before running the command, not the line
they get when it fails.

I'd have the 403 lead with the scope and keep support as the second half:
something like "Check the token has the statistics:read scope — run mapbox auth login again if it predates it. If the token has the scope, the account
may not have access; contact Mapbox support." That covers the common case
first and still lands the real one, and it stops fix contradicting message
on the same line of output.

Worth checking whether 401 is reachable here at all. If the endpoint only ever
answers 403 for both, the two remedies could collapse into one; if it uses 401
for a bad token and 403 for scope-or-access, then the 401 text should lose its
scope sentence, since it would then be describing the other branch's failure.

Smaller

  • The changelog is in the right place now, which I checked because it
    didn't used to be: the entry goes here under ## 0.2.2, and this repo's
    CHANGELOG.md is now the canonical one — the private copy is gone and the
    release tag no longer gates on it. Nothing to do; noting it because a
    reviewer with older context would flag it.
  • docs_contract passing is worth calling out as more than a formality:
    mapbox usage now registers unconditionally rather than behind a
    build-gated flag, so the documented surface and the real one had to move
    together, and they did.

And one claim I could not confirm either way

That the Statistics API is "generally available now, not gated behind a
Mapbox-support enablement step". My own 403 is a scope problem rather than an
access one, so it neither supports nor contradicts it — I can't distinguish
"GA for everyone" from "enabled for this account long ago" from here. The
error path is safe either way, since a genuine access failure still points at
support, so this isn't a blocker. But if the GA claim came from the Statistics
API team rather than from the endpoint's behaviour, it's worth a line in the
PR saying so, because it's the premise the rest of the change rests on.

The Statistics API answers 403, not 401, when a token is only
missing statistics:read — confirmed against a live account. The 403
remedy now leads with "run mapbox auth login again" before falling
back to Mapbox support, and the 401 remedy no longer claims a scope
problem it doesn't cause. Updated the OpenAPI spec and
docs/commands.md to match.

Also split DEFAULT_SCOPES into a DEFAULT_SCOPES_LIST array joined
into a string on first use, so the scope list reads as one per line
instead of one long literal.
@zmofei

zmofei commented Sep 15, 2026 •

Copy link
Copy Markdown
Member Author

Good catch, thanks. Pushed a fix: 403 now leads with "run mapbox auth login again" (the case your repro actually hit) and only falls back to Mapbox support once the scope is confirmed present. 401 no longer mentions scope at all.

On whether 401 is reachable at all: yes. Hit the endpoint directly with a garbage token:

$ curl "https://api.mapbox.com/statistics/v1?access_token=pk.invalidtoken12345"
401 {"message":"Not Authorized - Invalid Token"}

Same message the existing test already mocks. So the split is real: 401 = bad/missing token, 403 = valid token but missing scope, or (rarer) account has no access. Updated the OpenAPI spec and docs/commands.md to describe that instead of the old (wrong) 401=scope/403=account-not-enabled model.

Also folded DEFAULT_SCOPES into a DEFAULT_SCOPES_LIST array (one scope per line) joined into a string on first use, since you were reading through it anyway.

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The 403/401 fix is right, and I checked it the same way I found the
problem — by running it rather than by reading it.

Live, with a token that predates the scope:

Error: This API requires a token with statistics:read scope. (HTTP 403)
Fix: Check the token has the `statistics:read` scope — run `mapbox auth login`
     again if it predates that scope. If it already has the scope, this account
     doesn't have access to the Statistics API; contact Mapbox support.

The fix now agrees with the message beside it, leads with the case a
reader can act on, and still lands the real access problem second.

You also answered the follow-on question rather than leaving it. 401 loses
its scope sentence and becomes "the token is missing or invalid", which is what
that status actually means here. Having both remedies name the scope would have
left the same ambiguity pointing the other way.

And the test fixture now uses the API's real message. It asserted against
"Statistics API feature is not enabled for this account", which was invented;
it now uses "This API requires a token with statistics:read scope.", which is
what the endpoint returns. That is worth more than the remedy change — a test
built on a made-up response shape agrees with itself forever. The comment
recording why 403 rather than 401 ("confirmed live against a real account")
is the part that stops someone swapping them back.

The DEFAULT_SCOPES change is a real improvement, and I verified it changes nothing

Going from one long space-joined string to DEFAULT_SCOPES_LIST plus a cached
default_scopes() removes three call sites that were doing .split(' ') to get
back a list the constant used to be. OnceLock for the joined form is the right
weight for something that cannot change at runtime.

Since a silently dropped scope would break commands only after someone's next
login — the kind of thing no test here would notice — I compared the sets rather
than reading the list:

the refactor commit alone: 16 -> 16
identical, same order: True

And across the PR as a whole, against main: 15 → 16, statistics:read added,
nothing removed. Exactly what folding the flag in should do.

Verified

  • 595 tests, cargo clippy --locked --all-targets clean, cargo fmt --check
    clean.
  • mapbox usage registers unconditionally and docs_contract still passes, so
    the documented surface and the real one moved together.
  • feature_flags is gone with no dangling references.
  • The changelog entry is in this repo under ## 0.2.2, which is now the right
    place — the private copy is gone and the release tag no longer gates on one.

The GA premise I could not confirm from here still stands as the thing the rest
rests on, and it no longer worries me: whichever way it goes, a 403 now tells
the reader the two things that could be wrong in the order they can act on
them.

@zmofei
zmofei merged commit f6ff6e4 into main Sep 15, 2026
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