feat!: CLI v2 - #43
Merged
Merged
Conversation
Remove the TypeScript/oclif CLI entirely and start over in Go + cobra, per the installation & auth design (docs/design/001-installation-and-auth.md). The PoC implements interactive auth against the core OAuth 2.1 server (Flagsmith/flagsmith#8029): - `flagsmith login`: authorization-code + PKCE on a loopback listener (literal 127.0.0.1/[::1], RFC 8252), endpoints discovered via RFC 8414 metadata, explicit `management-api` scope, `--no-browser` for headless - credentials in the OS keychain, 0600-file fallback - `flagsmith auth status` / `auth token`: transparent refresh-token rotation (server grace period 120s) - `flagsmith logout`: server-side revocation + local removal Verified end-to-end against ghcr.io/flagsmith/flagsmith:pr-8029. beep boop
Tracks the scope rename in Flagsmith/flagsmith#8029. Verified e2e against the rebuilt pr-8029 image (metadata now advertises admin-api). beep boop
- oauth: full PKCE login against an in-process fake authorization server (asserts the exact authorization request and token-exchange contract, including verifier→challenge), state-mismatch rejection, access_denied, context cancellation, exchange failure, discovery errors, refresh rotation form, and revocation - store: keychain round-trip (go-keyring mock), forced file fallback with 0600 perms, corrupt-file and not-logged-in paths; make keychain removal in Delete best-effort so a broken keychain can't fail logout - cmd: drives login --no-browser → auth status → auth token → logout end-to-end in-process, with the test playing the browser beep boop
Implements the 02-authentication design (bar OIDC), red-green TDD: - FLAGSMITH_API_KEY resolved ahead of stored logins, classified by shape: dotted -> Master API key (Api-Key header), dotless -> bearer token; ser.* keys and legacy 40-hex authtokens rejected with targeted errors - flagsmith login --token / --token-stdin to store a Master API key (verified against /organisations/ before saving) - credential store keyed per instance API URL (keychain account = URL, file fallback becomes a URL-keyed map); logout --api targets one instance and only revokes OAuth sessions - auth status shows organisations and the active credential source; plaintext file fallback now warns visibly - auth token prints whichever Admin API credential is active Verified against staging (real browser PKCE login, refresh-token rotation, per-instance isolation) and against a local pr-8029 stack for the master-key paths with a real generated key. beep boop
`flagsmith auth login` / `flagsmith auth logout` now work identically to the top-level spellings (gh-style muscle memory). Commands are built by constructors and registered under both parents; the flow tests are parametrised over both shapes. beep boop
…y doc - `environment` now holds the client-side SDK key (env names have no server-side uniqueness constraint; keys are unique and public); `environmentKey` removed; schema anyOf(project, environment) so SDK-only configs are one field. Names come from a cosmetic name cache (os.UserCacheDir), never from the file. - Sources collapse to flag/env/default; schema fields map 1:1 to flags and FLAGSMITH_* env vars; config --json works credless/offline. - New 02-output-and-interactivity.md for cross-cutting conventions (TTY rules, --yes/--no-input/FLAGSMITH_NO_INPUT, --json, exit codes); auth and project-config docs renumbered to 03/04. beep boop
The plaintext file store is no longer a silent fallback. When the OS keychain is unavailable (headless Linux, containers, SSH), login fails closed before starting any flow — browser logins probe storage first so we never mint tokens we can't keep — and the error names both ways out: FLAGSMITH_API_KEY, or --insecure-storage to opt into the 0600 plaintext file. Refreshed sessions persist to whichever store they were loaded from (never migrating between stores), and auth status labels the opt-in store as "file (plaintext)". beep boop
Matches the flagsmith.json field name (apiUrl) and the --config-path convention from the project-config design. The alias is implemented via cobra's global flag-name normalization, so it works in any position and never appears in help. Also specs out-of-repo config discovery (cwd only) and keys-only -e/FLAGSMITH_ENVIRONMENT in 04. beep boop
…ONMENT (context) FLAGSMITH_ENVIRONMENT_KEY is the SDK credential — takes precedence and is the only home for server-side ser.* keys. FLAGSMITH_ENVIRONMENT is the default-environment context (client-side key, mirroring flagsmith.json). Also: 04 rewrite cleanups (env-key diff fix, typos, cache path, out-of-repo discovery, keys-only -e), matrix rows updated for the removed name-derivation path. beep boop
Nearest-file discovery walking up to the git toplevel (cwd only outside a repository), forward-compatible parsing (unknown fields warn, not fail), and rejection of server-side keys in the environment field with an error pointing at FLAGSMITH_ENVIRONMENT_KEY. A schema-drift guard test cross-checks the parser's field list against schema/flagsmith.json — and immediately caught $schema missing from the schema's properties, which with additionalProperties:false would make editors reject the $schema line that flagsmith init writes. Restored. beep boop
- Global context flags (-p/--project, --organisation, -e/--environment, --sdk-api-url, -c/--config-path) with cli > env > config > default precedence per value; sdkApiUrl follows a non-default apiUrl, else Edge; ser.* keys rejected in context with a pointer at FLAGSMITH_ENVIRONMENT_KEY - flagsmith config shows the resolved context with per-value sources; --json is the scripting interface; credless and offline, names are best-effort enrichment from the local cache - name cache at os.UserCacheDir()/flagsmith/cache.json keyed by instance; auth status seeds organisation names opportunistically - auth commands resolve their instance through the full chain, so a committed apiUrl finally works without --api-url beep boop
Implements the rest of 04-project-config: - flagsmith init: interactive flow (inline browser login when needed, org picker for multi-org users with the choice recorded, project picker with inline creation defaulting to the cwd name, environment picker writing the client-side key) and non-interactive flow (--project/--environment/--yes; missing input is a usage error). Values from an existing flagsmith.json act as prompt defaults, never as decisions; re-init shows a -/+ diff and confirms. The environments call doubles as the access check and seeds the name cache. $schema is pinned to the writing CLI's version tag. - Admin API client: Projects, CreateProject, Environments, with paginated-or-bare-array list decoding. - Exit codes per 02: usage errors (a prompt would have collected it) exit 2, everything else 1. --yes/--no-input aliases with FLAGSMITH_NO_INPUT; prompts require a real TTY. - Bare `flagsmith` nudges towards init when there is neither project context nor credentials. Verified live against staging (non-interactive init against a real project). Also fixes a test-harness bug where cobra's sticky --help local flag leaked across Execute calls. beep boop
- --json is a global flag with FLAGSMITH_JSON_OUTPUT, now covering auth status (identity, organisations, source, expiry) and auth token alongside config - browser rule enforced: without a TTY, login never opens a browser (implicit --no-browser); with --yes/--no-input it refuses outright, pointing at --token-stdin and FLAGSMITH_API_KEY instead of hanging CI for the login timeout - logout's revoke warning moves to stderr; stream discipline pinned by tests with split stdout/stderr capture - invalid promptable input (e.g. bad FLAGSMITH_PROJECT) is a usage error: exit 2, not 1 - fixes the FLASGMITH_JSON_OUTPUT typo in 02 beep boop
Replace the hand-rolled selector with huh: on a real terminal, pickers are full-terminal selects with arrow keys, j/k, and type-to-/-filter for long lists (a real win for the org picker); everywhere else huh's accessible mode gives numbered, line-based, screen-reader-friendly prompts that re-prompt on invalid input and terminate on EOF (never hang). Colors come from termenv and respect NO_COLOR. internal/prompt keeps its Select/Text/Confirm API and the stdinIsTTY/rawTerminal cmd seam, so the accessible path is exactly what the command tests drive. Accessible input is fed one byte at a time because huh spins up a fresh scanner per prompt and a buffered source would let one prompt swallow the next's input (regression-tested). Verified live against staging in tmux: arrow-key org selection, /-filtering the project list, and the ←/→ confirm on re-init. Docs transcripts in 04 updated to the real huh rendering. beep boop
Two bugs in how `flagsmith init` treated an existing file's organisation: - It was dropped entirely: organisationID started at 0 and was only set by the multi-org picker, so any invocation that skipped the picker (single-org user, or explicit --project) rewrote the file without its organisation, showing `- "organisation": N` in the diff. Seed it from the resolved context. - A config-file organisation suppressed the picker, so a multi-org user could never change it via re-init. Now only a flag/env organisation skips the picker (explicit decision); a config value is just the picker's pre-selected default, so re-init always re-offers the choice. Verified live against staging in tmux: re-init re-offers the org picker pre-selected to the current org. beep boop
A project with no environments (typically one just created inline) had nothing to pick from, so init wrote a config with no environment. Now, interactively, it prompts to create one (default name Development), creates it via the new api.CreateEnvironment, and records its key. Non-interactively an empty project is written without an environment rather than creating one silently. The test fake now distinguishes an accessible-but-empty project (200, empty list) from a no-access project (403), which the access check relies on. Verified live against staging: a freshly created project reports [] environments and init creates Development end-to-end. beep boop
New internal/output package owns the JSON-vs-human decision once: output.Render(w, data, opts, human) marshals the same data value that feeds the human view — so the two can't drift — and Table/Detail/Success give commands a consistent, NO_COLOR-aware, pipe-friendly vocabulary. - --jq <expr> (global, implies --json) filters JSON output through a jq expression via itchyny/gojq: raw for string results, compact JSON otherwise. Works on every command that outputs data. - Result model (02): stdout is the data result; ✓ confirmations, progress, warnings and prompts go to stderr. Migrated config, auth status (now a Detail view, single UsersMe call) and auth token onto Render; moved login/init/logout ✓ lines to stderr so mutations leave stdout empty (delete-style contract). - JSON mirrors the resource shape (bare object/array); config keeps its bespoke keyed shape. Verified live against staging: --jq filtering, migrated rendering, and the stdout/stderr split. beep boop
Answers the 02 gap where two init prompts (project name, environment name) had no flag equivalent and the overwrite-confirm exited 1 instead of 2. - Prompt primitives now take the flag that supplies the same value and self-guard: called without a TTY they return a usage error (exit 2) naming that flag. A prompt can no longer be written without linking a flag, and can no longer hang non-interactively — the contract is structural, not per-call-site discipline. confirmOrYes centralises the yes/no case (--yes answers it; no TTY without it is exit 2 naming --yes). - --create-project <name> / --create-environment <name> make creation flag-drivable, each mutually exclusive (exit 2) with its select counterpart; org resolution factored into resolveOrganisation so both the create and select paths share it. - Re-init now carries a config-file environment forward non-interactively (it was silently dropped, the same class of bug as the earlier org drop). Verified live against staging: non-interactive create of project + environment, mutual-exclusion and overwrite-without-yes both exit 2. beep boop
Move the result model out of 02 (it is CRUD-shaped, not general output) into a new 05-crud.md defining the shared shape every resource command follows: command layout, addressing, result model (human list shows a count; JSON is a bare array), mutations, errors, and pagination (list fetches all pages, --limit caps). Also fixes a typo in 02. beep boop
Scope flags to feature states (the SDK view); project-level feature definitions are a separate `features` resource, deferred. Resource definition plus `flags list` as the minimum viable command: reads via the SDK API with just an environment key (GET /api/v1/flags/), so it works right after `init` and makes the init nudge real. get and the enable/disable/set mutations (Admin API, v2-versioning branch) noted as later work. Also lands 05-crud.md (CRUD conventions) from the prior step. beep boop
Implements the minimum from 06-flags.md, making the `flagsmith init` next-step nudge real. `flags list` reads feature states from the SDK API (GET /api/v1/flags/, X-Environment-Key) using the environment key from context — FLAGSMITH_ENVIRONMENT_KEY (may be server-side) or the client-side key in flagsmith.json — so it works with no Admin API credentials at all. Human output is a NAME/ENABLED/VALUE table with a count; --json/--jq get the bare array as the API returns it (per 05's result model). Verified live against staging with only a committed environment key. beep boop
Colour the human tables after tabwriter has aligned them, not before: Table bolds the header line and Detail colours the label column once the plain text is laid out, so the ANSI bytes never count toward column width. Restores the bold header (and cyan detail labels) while keeping `flags list` and the detail views aligned; piped output stays plain. beep boop
So a flagsmith.json written by a branch build resolves its schema (main has no schema/flagsmith.json yet). Revert to main / a release tag before merge. beep boop
Point $schema at the feat/cli-v2 ref so it resolves like the files init now writes. Revert to main / a release tag before merge. beep boop
Adopt singular resource nouns for CRUD commands (gh/kubectl style): `flagsmith flag list`, not `flags`. Renames the command and its cobra identifiers, the init next-step nudge, tests, and the command examples in 05-crud.md / 06-flags.md. beep boop
Leftover from the rename to 04-project-config.md; accidentally re-added in the previous commit by `git add -A`. 03 is authentication; the project-config doc lives at 04. beep boop
Themis spotted that aa55c22 only covered half the set: tabwriter's Write switches on '\t', '\v', '\n', '\f' (text/tabwriter.go:532), so a value carrying a vertical tab or form feed still desynchronised output lines from input rows and re-triggered the same index-out-of-range panic. Reproduced before fixing — "a\fb" in a description panicked exactly as before. Both now flatten to spaces alongside \n and \t, and the test asserts line count == field count over all four. Two other leftovers from the same review round: the redirect cap's message hardcoded 10 next to the maxRedirects const it must agree with, and the doc comment for TestFeatureVariantListEmptyIsArray had been orphaned onto TestFalseyEnvSwitches when that test was inserted above it. beep boop
02-output-and-interactivity described the switches only as settable / =1, but c1289c6 made 0/false/no/off explicitly turn them off — a promise the tests now enforce, so it belongs in the doc rather than being discoverable only from the code. FLAGSMITH_DEBUG is mentioned there too, having had no documentation anywhere. 05-crud's `--limit N` sentence read as shipped behaviour with nothing distinguishing it, unlike `--weight` in 07-flags which names its upstream dependency. Removed rather than annotated: pagination already returns the aggregate, and a flag that does not exist is better absent from the spec than present with a caveat. Addresses the residuals Themis raised on #43 (root.go:101, docs/design/12-evaluate.md). beep boop
khvn26
marked this pull request as ready for review
July 28, 2026 10:38
TestCheckRedirectCapsHops asserted only err != nil, so nothing stated what the cap actually reports. The expectation is hardcoded rather than built from maxRedirects: derived from the const, a changed cap would still pass its own test, which is the behaviour change worth catching. Verified by mutation — 10 → 5 fails it. Note for the record that this does not guard the bug that prompted it: `go test` runs vet's printf check itself, so the %s/int mismatch fails the build before any assertion runs. What the line does buy is the cap's value and wording, which vet cannot see. Addresses a residual Themis raised on #43 (httpx/client.go:62). beep boop
Contributor
|
@CodeRabbit review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThe repository is migrated from a Node/Oclif CLI to a Go-based CLI. The change adds typed API access, OAuth and keychain authentication, configuration discovery, caching, prompts, output formatting, resource commands, flag and override management, environment operations, API passthroughs, tests, documentation, schema definitions, and Go-focused CI. Obsolete Node entrypoints, package metadata, workflows, and example application files are removed. Estimated code review effort: 5 (Critical) | ~180 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 035ecd54-c534-49a3-ab68-22d9e0826ca7
⛔ Files ignored due to path filters (5)
example/package-lock.jsonis excluded by!**/package-lock.jsonexample/public/favicon.icois excluded by!**/*.icoexample/public/logo192.pngis excluded by!**/*.pngexample/public/logo512.pngis excluded by!**/*.pnggo.sumis excluded by!**/*.sum
📒 Files selected for processing (101)
.editorconfig.eslintignore.eslintrc.github/workflows/publish.yml.github/workflows/pull-request.yml.github/workflows/release-please.yml.gitignore.golangci.yml.mocharc.json.nvmrc.pre-commit-config.yaml.release-please-manifest.jsonCHANGELOG.mdREADME.mdbin/devbin/dev.cmdbin/runbin/run.cmddocs/design/01-installation.mddocs/design/02-output-and-interactivity.mddocs/design/03-authentication.mddocs/design/04-project-config.mddocs/design/05-crud.mddocs/design/06-api.mddocs/design/07-flags.mddocs/design/08-segments.mddocs/design/09-features.mddocs/design/10-projects-organisations.mddocs/design/11-environments.mddocs/design/12-evaluate.mdexample/.gitignoreexample/README.mdexample/package.jsonexample/public/index.htmlexample/public/manifest.jsonexample/public/robots.txtexample/src/App.tsxexample/src/index.cssexample/src/index.tsxexample/tsconfig.jsonflagsmith.jsongo.modinternal/api/client.gointernal/api/client_test.gointernal/auth/kind.gointernal/auth/kind_test.gointernal/auth/oauth.gointernal/auth/oauth_test.gointernal/auth/store.gointernal/auth/store_test.gointernal/bug/bug.gointernal/bug/bug_test.gointernal/cache/cache.gointernal/cache/cache_test.gointernal/cmd/api.gointernal/cmd/auth.gointernal/cmd/client.gointernal/cmd/cmd_test.gointernal/cmd/config.gointernal/cmd/config_test.gointernal/cmd/context.gointernal/cmd/envcred.gointernal/cmd/envcred_test.gointernal/cmd/environment.gointernal/cmd/errors.gointernal/cmd/errors_test.gointernal/cmd/feature.gointernal/cmd/flag_identity.gointernal/cmd/flag_reorder.gointernal/cmd/flag_update.gointernal/cmd/flags.gointernal/cmd/init.gointernal/cmd/label.gointernal/cmd/label_test.gointernal/cmd/login.gointernal/cmd/logout.gointernal/cmd/organisation.gointernal/cmd/project.gointernal/cmd/prompts.gointernal/cmd/render.gointernal/cmd/resolve.gointernal/cmd/root.gointernal/cmd/segment.gointernal/cmd/sentinel_test.gointernal/config/config.gointernal/config/config_test.gointernal/httpx/client.gointernal/httpx/client_test.gointernal/output/output.gointernal/output/output_test.gointernal/prompt/prompt.gointernal/prompt/prompt_test.gointernal/version/version.gointernal/version/version_test.gomain.gopackage.jsonrelease-please-config.jsonschema/flagsmith.jsonsrc/commands/get/index.tssrc/index.tstsconfig.json
💤 Files with no reviewable changes (28)
- bin/run.cmd
- bin/dev
- example/public/manifest.json
- example/tsconfig.json
- example/public/robots.txt
- .nvmrc
- example/src/index.css
- package.json
- src/index.ts
- .github/workflows/release-please.yml
- .mocharc.json
- .editorconfig
- example/public/index.html
- tsconfig.json
- .eslintignore
- example/README.md
- example/.gitignore
- CHANGELOG.md
- example/src/index.tsx
- example/src/App.tsx
- .github/workflows/publish.yml
- release-please-config.json
- example/package.json
- bin/run
- .eslintrc
- src/commands/get/index.ts
- bin/dev.cmd
- .release-please-manifest.json
No job needs the token after checkout, but actions/checkout persists it for later steps to reuse. On a public repo every PR build runs untrusted code next to it. beep boop
checkout v4.4.0 -> v7.0.1, setup-go v5.6.0 -> v7.0.0, golangci-lint-action v8.0.0 -> v9.3.0. The majors are node20 -> node24 runtime moves; setup-go v6's toolchain-selection change is moot with no toolchain line in go.mod. beep boop
beep boop
A next link that cycles spins the walk forever. Nearly every command has a 60s deadline to fall back on, but the ones that opt out of it (init) do not, and "context deadline exceeded" says nothing about which page broke. beep boop
The callback handler sent on a one-slot channel unconditionally. Only the first send is ever read, so a third delivery — a refreshed success tab, or anything else that can reach the loopback port — parked its handler, and Shutdown waits for active connections, so the deferred teardown never returned either. Login hung after already succeeding, with no invocation deadline to catch it. beep boop
Every keyring.Get failure became ErrNotLoggedIn, so a locked keychain sent the user through the whole browser login only to fail at Save. Only ErrNotFound means "no credential"; anything else means the keychain itself is unusable, which already hints at FLAGSMITH_API_KEY. beep boop
The fake serves requests concurrently — fsPeak exists to measure it — but workflowGated and segmentMissing were read outside f.mu, and four fields were set without it. Safe only because every write happens to precede its request, which the next mid-flight toggle would break silently. beep boop
Confirming first meant answering "Delete max_items override for identifier ghost?" and only then hearing the identity does not exist — and without a TTY, hearing to pass --yes instead. runIdentityUpdate already resolves first. beep boop
A missing feature-state row read as the zero value, so the request echoed the override as off with an empty value. Since update-flag-v2 takes the whole config, moving priorities would have rewritten it. beep boop
Only an explicit --organisation narrowed the list, while resolveProjectRefID honours the context. In an org-pinned directory, list showed projects that project get then refused to resolve by name. beep boop
renderSegment rebuilt what toSegmentView already returns, so the curated JSON shape existed twice — once for segment list, once for get/create/update. beep boop
An anonymous errors.New meant callers could only string-match "the user pressed Ctrl-C". The sentinel self-test forces the hint decision, and the answer is none: there is nothing to recover from. beep boop
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4478406a-fdb8-4f85-8665-5847743a5262
📒 Files selected for processing (51)
.github/workflows/pull-request.ymldocs/design/01-installation.mddocs/design/05-crud.mdinternal/api/client.gointernal/api/client_test.gointernal/auth/kind.gointernal/auth/kind_test.gointernal/auth/oauth.gointernal/auth/oauth_test.gointernal/auth/store.gointernal/auth/store_test.gointernal/bug/bug.gointernal/cache/cache.gointernal/cache/cache_test.gointernal/cmd/api.gointernal/cmd/auth.gointernal/cmd/client.gointernal/cmd/cmd_test.gointernal/cmd/context.gointernal/cmd/envcred.gointernal/cmd/envcred_test.gointernal/cmd/environment.gointernal/cmd/errors.gointernal/cmd/errors_test.gointernal/cmd/feature.gointernal/cmd/flag_identity.gointernal/cmd/flag_reorder.gointernal/cmd/flag_update.gointernal/cmd/flags.gointernal/cmd/init.gointernal/cmd/label.gointernal/cmd/label_test.gointernal/cmd/login.gointernal/cmd/organisation.gointernal/cmd/project.gointernal/cmd/prompts.gointernal/cmd/render.gointernal/cmd/resolve.gointernal/cmd/root.gointernal/cmd/segment.gointernal/cmd/sentinel_test.gointernal/config/config.gointernal/config/config_test.gointernal/httpx/client.gointernal/httpx/client_test.gointernal/output/output.gointernal/output/output_test.gointernal/prompt/prompt.gointernal/prompt/prompt_test.gointernal/version/version.gointernal/version/version_test.go
💤 Files with no reviewable changes (2)
- internal/version/version_test.go
- internal/auth/kind_test.go
matthewelwell
approved these changes
Jul 28, 2026
matthewelwell
left a comment
matthewelwell
left a comment
Contributor
There was a problem hiding this comment.
Stamping based on:
- Acceptance that this is greenfield code for the CLI v2
- Comprehensive review cycles from CodeRabbit and Themis
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #42.
To try it, make sure you've got Go 1.26+ installed and: