diff --git a/README.md b/README.md index 56fe9ce..449a30d 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,24 @@ instant login # Log in to your instanode.dev account instant whoami # Show current account ``` +### Authentication + +`instant login` runs a browser device-flow and saves your credentials to +`~/.instant-config`. If that flow times out, or you're on a headless box, +skip it entirely with a **Personal Access Token**: mint one at +[instanode.dev/app/settings](https://instanode.dev/app/settings), then +authenticate any command in one of two ways: + +```bash +instant --token resources # per-invocation flag (highest priority) +export INSTANT_TOKEN= # environment variable for the session +instant resources +``` + +Resolution order is `--token` flag → `INSTANT_TOKEN` env var → saved +`instant login` credentials. Both PAT paths skip the browser entirely, so +they're the recommended auth for CI and agent scripts. + ### Targeting an environment Every `new` verb accepts an optional `--env` flag that the API honors diff --git a/cmd/deploy_stub.go b/cmd/deploy_stub.go index bd16d88..b49ef48 100644 --- a/cmd/deploy_stub.go +++ b/cmd/deploy_stub.go @@ -74,6 +74,12 @@ Track the upcoming native CLI support at: https://github.com/InstaNode-dev/cli/issues `, Args: cobra.NoArgs, + // B15-P2 follow-up: tolerate unknown flags so `instant deploy --name foo` + // (or any flag an agent reaches for) lands on the helpful MCP/curl pointer + // instead of dying with cobra's `unknown flag: --name` BEFORE RunE runs. + // These are stub commands with no real flags of their own; the whole point + // is that any invocation shape reaches the "use this instead" message. + FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}, RunE: func(cmd *cobra.Command, args []string) error { // Print the long help (covers the alternative-surface pointers) // and exit non-zero so scripts that test exit code don't proceed @@ -94,6 +100,13 @@ func newDeployStub(verb, extra string) *cobra.Command { Use: verb, Short: short, Args: cobra.ArbitraryArgs, + // Tolerate unknown flags (e.g. `instant deploy new --name foo --env + // production`) so the invocation reaches the MCP/curl pointer below + // instead of dying with cobra's `unknown flag: --name` pre-RunE. An + // agent that reflexively passes the real deploy flags must still land + // on the "use this instead" message — not a flag-parse error that + // looks like a bug in its own command construction. + FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}, RunE: func(cmd *cobra.Command, args []string) error { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "`instant deploy %s` is not yet implemented in the CLI.\n"+ diff --git a/cmd/discover.go b/cmd/discover.go index b15f64e..e80c387 100644 --- a/cmd/discover.go +++ b/cmd/discover.go @@ -96,11 +96,13 @@ func runResources(cmd *cobra.Command) error { if haveAuth() { return errSessionExpired() } - // In JSON mode the envelope is the only signal; skip the stderr - // hint so a `--json | jq` pipeline isn't disturbed. - if !jsonModeOn(cmd) { - fmt.Fprintln(os.Stderr, "Not logged in. Run `instant login` first.") - } + // De-dupe: previously this branch ALSO printed "Not logged in. Run + // `instant login` first." to stderr, then returned errAuthRequired — + // whose message main.go (run → Fprintln(stderr, err)) prints again. + // The user saw the not-logged-in guidance twice. Return the error + // silently and let main.go own the single print. (The errAuthRequired + // message already names `instant login`, so no guidance is lost; JSON + // mode is unaffected because nothing extra is written to stderr.) return errAuthRequired("authentication required — run `instant login` first") } diff --git a/cmd/dx_gaps_2026_06_10_test.go b/cmd/dx_gaps_2026_06_10_test.go new file mode 100644 index 0000000..24e9f22 --- /dev/null +++ b/cmd/dx_gaps_2026_06_10_test.go @@ -0,0 +1,272 @@ +package cmd + +// dx_gaps_2026_06_10_test.go — four contained DX-gap fixes (2026-06-10): +// +// 1. PAT fallback signpost on `instant login` poll timeout AND in +// `login --help`. A stranded device-flow user must learn the +// --token / INSTANT_TOKEN escape hatch. +// 2. `instant deploy new --name foo` must reach the helpful MCP/curl +// pointer instead of dying with cobra `unknown flag: --name`. +// 3. `instant resources` (no auth) must print the "not logged in" +// guidance EXACTLY ONCE (main.go owns the print; the handler no +// longer also writes to os.Stderr). +// 4. `instant --version` falls back to runtime/debug VCS metadata when +// ldflags are empty (the `go install` / `go build` path). + +import ( + "runtime/debug" + "strings" + "testing" +) + +// ── Fix 1: PAT fallback signpost ──────────────────────────────────────────── + +// TestLoginTimeout_PrintsPATWorkaround pins that the login-poll timeout error +// surfaces the Personal Access Token escape hatch (settings URL + both +// --token and INSTANT_TOKEN forms). A user whose browser flow stalls must be +// told how to authenticate without it. +func TestLoginTimeout_PrintsPATWorkaround(t *testing.T) { + withCleanState(t) + withShortPolls(t) + + prev := APIBaseURL + // Unroutable host → every poll attempt errors and the loop eventually + // hits the deadline, returning the timeout error. + APIBaseURL = "http://127.0.0.1:1" + t.Cleanup(func() { APIBaseURL = prev }) + + _, err := pollForAuthCompletion("s1") + if err == nil { + t.Fatal("expected timeout error") + } + msg := err.Error() + for _, want := range []string{ + "https://instanode.dev/app/settings", + "--token", + "INSTANT_TOKEN", + } { + if !strings.Contains(msg, want) { + t.Errorf("timeout error must mention %q for the PAT workaround; got %q", want, msg) + } + } +} + +// TestLoginHelp_DocumentsPATWorkaround pins that `instant login --help` +// (the Long text) tells the user about the PAT escape hatch before they +// even start a flow that might time out / fail on a headless box. +func TestLoginHelp_DocumentsPATWorkaround(t *testing.T) { + long := loginCmd.Long + for _, want := range []string{ + "https://instanode.dev/app/settings", + "--token", + "INSTANT_TOKEN", + } { + if !strings.Contains(long, want) { + t.Errorf("login --help Long must mention %q; got %q", want, long) + } + } +} + +// TestPATHint_SingleSource guards rule 16 (one token, all sites): the timeout +// error and the help text must both derive their core sentence from the same +// const so they can never drift apart. +func TestPATHint_SingleSource(t *testing.T) { + if !strings.Contains(patWorkaroundHint, "https://instanode.dev/app/settings") { + t.Errorf("patWorkaroundHint missing settings URL: %q", patWorkaroundHint) + } + if !strings.Contains(patWorkaroundHint, "--token") || + !strings.Contains(patWorkaroundHint, "INSTANT_TOKEN") { + t.Errorf("patWorkaroundHint missing one of the two PAT auth forms: %q", patWorkaroundHint) + } +} + +// ── Fix 2: deploy stub tolerates unknown flags ────────────────────────────── + +// TestDeployStub_UnknownFlagReachesPointer pins that an agent passing the +// REAL deploy flags (`--name`, `--env`) to the stub still lands on the +// helpful MCP/curl pointer rather than cobra's `unknown flag: --name` +// flag-parse error (which fires BEFORE RunE and looks like a bug in the +// agent's own command). +func TestDeployStub_UnknownFlagReachesPointer(t *testing.T) { + newITContext(t) + + cases := [][]string{ + {"deploy", "new", "--name", "foo"}, + {"deploy", "new", "--name", "foo", "--env", "production"}, + {"deploy", "logs", "some-id", "--follow"}, + {"deploy", "--name", "foo"}, // bare parent + unknown flag + } + for _, args := range cases { + args := args + t.Run(strings.Join(args, "_"), func(t *testing.T) { + _, stderr, err := run(args...) + if err == nil { + t.Fatalf("%v: MUST exit non-zero (stub not implemented)", args) + } + // The whole point: the error must be the not-implemented + // pointer, NOT an "unknown flag" flag-parse failure. + combined := strings.ToLower(stderr + err.Error()) + if strings.Contains(combined, "unknown flag") { + t.Errorf("%v: reached cobra unknown-flag error instead of the pointer; got %q", + args, combined) + } + if !strings.Contains(combined, "implement") { + t.Errorf("%v: error must mention not-implemented pointer; got %q", args, combined) + } + }) + } +} + +// ── Fix 3: single not-logged-in print ─────────────────────────────────────── + +// TestResources_NoAuth_NoDuplicateStderrHint pins that the resources 401 +// no-auth branch no longer ALSO writes "Not logged in. Run `instant login` +// first." to os.Stderr. Pre-fix the handler printed that sentence AND main.go +// printed the returned errAuthRequired message — two not-logged-in lines for +// one failure. main.go now owns the single print; the handler is silent. +// +// We capture the OS-level stderr (the pre-fix print used fmt.Fprintln(os.Stderr, +// …), which bypasses cobra's SetErr buffer that run() captures). +func TestResources_NoAuth_NoDuplicateStderrHint(t *testing.T) { + c := newITContext(t) + c.mock.mu.Lock() + c.mock.requireAuth = true + c.mock.mu.Unlock() + + resetJSONFlags() + + var err error + _, osStderr := captureStdout(t, func() { + _, _, err = run("resources") + }) + if err == nil { + t.Fatal("resources w/o auth: expected non-nil err (exit 3)") + } + if ExitCodeFor(err) != ExitAuthRequired { + t.Errorf("exit code = %d, want %d (ExitAuthRequired)", ExitCodeFor(err), ExitAuthRequired) + } + // The handler must no longer write the legacy "Not logged in." sentence + // to os.Stderr; main.go owns the single user-facing print. + if strings.Contains(osStderr, "Not logged in. Run") { + t.Errorf("handler must not print the legacy not-logged-in hint to os.Stderr (main.go owns it); got %q", + osStderr) + } + // The returned error still names `instant login` so no guidance is lost. + if !strings.Contains(err.Error(), "instant login") { + t.Errorf("returned error must still point at `instant login`; got %q", err.Error()) + } +} + +// ── Fix 4: --version VCS fallback ─────────────────────────────────────────── + +// fakeBuildInfo builds a *debug.BuildInfo carrying the given vcs settings so +// resolveBuildInfo's backfill path is exercised without a real git checkout. +func fakeBuildInfo(rev, vtime string) func() (*debug.BuildInfo, bool) { + return func() (*debug.BuildInfo, bool) { + bi := &debug.BuildInfo{} + if rev != "" { + bi.Settings = append(bi.Settings, debug.BuildSetting{Key: "vcs.revision", Value: rev}) + } + if vtime != "" { + bi.Settings = append(bi.Settings, debug.BuildSetting{Key: "vcs.time", Value: vtime}) + } + return bi, true + } +} + +// TestResolveBuildInfo_LdflagsWin: when ldflags are stamped, the VCS reader is +// never consulted — the stamped values pass straight through. +func TestResolveBuildInfo_LdflagsWin(t *testing.T) { + v, c, b := resolveBuildInfo("v1.2.3", "abc1234", "2026-06-10T00:00:00Z", + fakeBuildInfo("ffffffffffffffffffffffffffffffffffffffff", "2099-01-01T00:00:00Z")) + if v != "v1.2.3" || c != "abc1234" || b != "2026-06-10T00:00:00Z" { + t.Errorf("ldflag values must win, got (%q,%q,%q)", v, c, b) + } +} + +// TestResolveBuildInfo_VCSFallback: empty commit/buildTime (the go install / +// go build path) are backfilled from VCS metadata, and the revision is +// shortened to the platform's 7-char form. +func TestResolveBuildInfo_VCSFallback(t *testing.T) { + v, c, b := resolveBuildInfo("", "", "", + fakeBuildInfo("0123456789abcdef0123456789abcdef01234567", "2026-06-09T12:00:00Z")) + if v != "dev" { + t.Errorf("version = %q, want dev (no ldflag, no VCS version)", v) + } + if c != "0123456" { + t.Errorf("commit = %q, want short VCS sha 0123456", c) + } + if b != "2026-06-09T12:00:00Z" { + t.Errorf("buildTime = %q, want VCS time", b) + } +} + +// TestResolveBuildInfo_SentinelInputFallsBack pins the REAL production path: +// main.go declares the un-stamped defaults as the SENTINELS "dev"/"unknown" +// (not ""), so SetBuildInfo passes sentinels. The fallback must treat those +// as unset and still backfill from VCS — a naive `== ""` guard would not. +func TestResolveBuildInfo_SentinelInputFallsBack(t *testing.T) { + v, c, b := resolveBuildInfo("dev", "unknown", "unknown", + fakeBuildInfo("abcdef0123456789abcdef0123456789abcdef01", "2026-06-08T08:00:00Z")) + if v != "dev" { + t.Errorf("version = %q, want dev", v) + } + if c != "abcdef0" { + t.Errorf("commit = %q, want short VCS sha abcdef0 (sentinel must not block fallback)", c) + } + if b != "2026-06-08T08:00:00Z" { + t.Errorf("buildTime = %q, want VCS time (sentinel must not block fallback)", b) + } +} + +// TestResolveBuildInfo_NoVCSNoLdflags: nothing available anywhere → the +// pre-existing dev/unknown/unknown sentinel is preserved (e.g. `go run`). +func TestResolveBuildInfo_NoVCSNoLdflags(t *testing.T) { + v, c, b := resolveBuildInfo("", "", "", func() (*debug.BuildInfo, bool) { return nil, false }) + if v != "dev" || c != "unknown" || b != "unknown" { + t.Errorf("sentinel fallback broken, got (%q,%q,%q)", v, c, b) + } + // nil reader must also be tolerated. + v, c, b = resolveBuildInfo("", "", "", nil) + if v != "dev" || c != "unknown" || b != "unknown" { + t.Errorf("nil-reader fallback broken, got (%q,%q,%q)", v, c, b) + } +} + +// TestShortSHA_ShortInputUnchanged: a sub-7-char revision is returned as-is so +// an unexpected/dirty value still surfaces rather than being mangled. +func TestShortSHA_ShortInputUnchanged(t *testing.T) { + if got := shortSHA("abc"); got != "abc" { + t.Errorf("shortSHA(abc) = %q, want abc", got) + } + if got := shortSHA("0123456789"); got != "0123456" { + t.Errorf("shortSHA truncation = %q, want 0123456", got) + } +} + +// TestSetBuildInfo_VersionStringShape is an end-to-end smoke over SetBuildInfo +// (which calls resolveBuildInfo with the real debug.ReadBuildInfo): the +// resulting rootCmd.Version is the documented " (, )" shape and is +// never the empty-paren placeholder. +func TestSetBuildInfo_VersionStringShape(t *testing.T) { + prev := rootCmd.Version + t.Cleanup(func() { rootCmd.Version = prev }) + + SetBuildInfo("v9.9.9", "deadbee", "2026-06-10T00:00:00Z") + if rootCmd.Version != "v9.9.9 (deadbee, 2026-06-10T00:00:00Z)" { + t.Errorf("rootCmd.Version = %q", rootCmd.Version) + } + + // Sentinel ldflags (main.go's un-stamped defaults) → resolveBuildInfo + // runs against the test binary's real build info. The shape must still + // be well-formed (never "(, )") and must start with the dev sentinel. + // (Under `go test` the binary carries no runtime vcs settings, so this + // lands on dev (unknown, unknown) — still well-formed.) + SetBuildInfo("dev", "unknown", "unknown") + if !strings.HasPrefix(rootCmd.Version, "dev (") || !strings.HasSuffix(rootCmd.Version, ")") { + t.Errorf("sentinel-ldflag Version malformed: %q", rootCmd.Version) + } + if strings.Contains(rootCmd.Version, "(, )") { + t.Errorf("Version must never have empty commit+time fields: %q", rootCmd.Version) + } +} diff --git a/cmd/login.go b/cmd/login.go index 734a8e3..bec7a28 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -18,6 +18,20 @@ import ( "github.com/InstaNode-dev/cli/internal/tokens" ) +// patWorkaroundHint is the single source of truth for the "your login timed +// out — mint a PAT instead" signpost. It is surfaced in two places: the +// pollForAuthCompletion timeout error (so a stranded user sees it the moment +// the browser flow stalls) and the `instant login --help` long text (so a +// user who already knows the device-flow is flaky finds the escape hatch +// before they even start). The CLI already honours --token / INSTANT_TOKEN +// (root.go initConfig priority chain) and instanode-web mints PATs at +// /app/settings — but until this hint shipped the CLI never told a timed-out +// user that path existed. Kept as a package const (not an inline literal) so +// the two emitters can never drift (CLAUDE.md rule 16 — one token, all sites). +const patWorkaroundHint = "Login timed out. Workaround: mint a Personal Access Token at " + + "https://instanode.dev/app/settings and run `instant --token ...` or " + + "`export INSTANT_TOKEN=`." + // pollInterval is how often the CLI checks for auth completion. // // Declared as var (not const) so tests can lower it to milliseconds without @@ -47,6 +61,11 @@ Subsequent commands will use it automatically for authenticated API calls. If you upgrade to a paid plan, run `+"`instant login`"+` again to refresh your tier — or the CLI will detect it automatically on the next API call. + +If the browser flow times out or you're on a headless machine, skip it: +mint a Personal Access Token at https://instanode.dev/app/settings, then +authenticate any command with `+"`instant --token ...`"+` or by exporting +`+"`INSTANT_TOKEN=`"+` in your shell. `, RunE: runLogin, } @@ -244,7 +263,8 @@ func pollForAuthCompletion(sessionID string) (*authResult, error) { return nil, fmt.Errorf("unexpected status %d: %s", resp.StatusCode, raw) } - return nil, fmt.Errorf("timed out waiting for login after %.0f minutes; try again", pollTimeout.Minutes()) + return nil, fmt.Errorf("timed out waiting for login after %.0f minutes; try again.\n%s", + pollTimeout.Minutes(), patWorkaroundHint) } // pollForTierUpgrade polls GET /auth/me until the tier changes, up to 5 minutes. diff --git a/cmd/root.go b/cmd/root.go index 8afac6f..b2d7ae3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "os" + "runtime/debug" "strconv" "strings" "time" @@ -136,16 +137,90 @@ func ExecuteWithArgs(args []string) error { // gate is `instant --version | grep ` — which can only be satisfied // if the linker actually stamped these vars. func SetBuildInfo(version, commit, buildTime string) { - if version == "" { + version, commit, buildTime = resolveBuildInfo(version, commit, buildTime, debug.ReadBuildInfo) + rootCmd.Version = fmt.Sprintf("%s (%s, %s)", version, commit, buildTime) +} + +// buildInfoReader matches the signature of runtime/debug.ReadBuildInfo so +// resolveBuildInfo can be driven with a stub in tests (the real VCS settings +// only exist for binaries built from inside a git checkout, which a `go test` +// process is not). +type buildInfoReader func() (*debug.BuildInfo, bool) + +// resolveBuildInfo fills empty/sentinel ldflag values from the binary's +// embedded VCS metadata. +// +// Background: a release build stamps Version/Commit/BuildTime via +// `-ldflags -X`, so `instant --version` reads them directly. But a binary +// produced by `go install github.com/InstaNode-dev/cli@latest` or a bare +// `go build` has NO ldflags — pre-fix those printed the useless +// `dev (unknown, unknown)`, defeating CLAUDE.md rule 14's build-SHA gate for +// the most common install path (the README's `go install` one-liner). +// +// Go embeds VCS data (`vcs.revision`, `vcs.time`) in the BuildInfo of any +// binary built from inside a VCS checkout. When the commit/buildTime are +// unset we backfill from there, so `go install`/`go build` binaries print +// the real short SHA + commit time instead of "unknown". +// +// IMPORTANT: "unset" means empty OR the sentinel — main.go declares +// `Commit = "unknown"` / `BuildTime = "unknown"` / `Version = "dev"` as the +// un-stamped defaults, so SetBuildInfo receives the SENTINEL (not ""), and a +// naive `== ""` guard would never trigger the fallback. We treat the sentinels +// as unset here. Sentinels survive only when neither ldflags NOR VCS data are +// available (e.g. `go run`, or `-buildvcs=false`). +func resolveBuildInfo(version, commit, buildTime string, read buildInfoReader) (string, string, string) { + var vcsRev, vcsTime string + if read != nil { + if info, ok := read(); ok && info != nil { + for _, s := range info.Settings { + switch s.Key { + case "vcs.revision": + vcsRev = s.Value + case "vcs.time": + vcsTime = s.Value + } + } + } + } + + if isUnset(version, "dev") { version = "dev" } - if commit == "" { - commit = "unknown" + if isUnset(commit, "unknown") { + if vcsRev != "" { + commit = shortSHA(vcsRev) + } else { + commit = "unknown" + } } - if buildTime == "" { - buildTime = "unknown" + if isUnset(buildTime, "unknown") { + if vcsTime != "" { + buildTime = vcsTime + } else { + buildTime = "unknown" + } } - rootCmd.Version = fmt.Sprintf("%s (%s, %s)", version, commit, buildTime) + return version, commit, buildTime +} + +// isUnset reports whether an ldflag value should be treated as not-stamped: +// either the empty string or the package's sentinel default. main.go's +// un-stamped defaults are sentinels (not ""), so the VCS fallback must key +// off both forms. +func isUnset(v, sentinel string) bool { + return v == "" || v == sentinel +} + +// shortSHA truncates a full 40-char git revision to the 7-char short form the +// rest of the platform uses (api/worker/provisioner /healthz emit short SHAs, +// and rule 14 compares against `git rev-parse --short HEAD`). A shorter or +// non-hex value is returned unchanged so a dirty/unexpected revision string +// still surfaces rather than being silently mangled. +func shortSHA(rev string) string { + if len(rev) >= 7 { + return rev[:7] + } + return rev } func init() {