fix(cli): item-envelope detail decode + provision-timeout orphan guidance (#29)

* fix(cli): item-envelope detail decode + provision-timeout orphan guidance

Two bugs from the live durability sweep.

BUG 1 (silent): `instant resource <token>` printed an all-empty object.
runResourceDetail only unwrapped a bare object or a `"resource"` envelope,
but prod GET /api/v1/resources/:token wraps the object under `"item"`
({"item":{...},"ok":true}) — neither branch matched, so every field but the
path-token fallback rendered empty with exit 0. Teach the decoder the "item"
key (preferred), keeping "resource" + bare-object fallbacks for back-compat.

BUG 2 (durability UX): on a provision timeout the CLI exited 1 with a bare
"context deadline exceeded" — but provisioning is synchronous server-side, so
the resource may have already landed as an orphan the user can't see. The 60s
default timeout is intentional (TestBugHunt_T16_P2_2) and unchanged; instead,
detect the client/context-deadline timeout in provisionResource and surface
actionable recovery guidance (run `instant resources` / `instant resource
delete <token> --yes`) via a named const, preserving the cause with %w.

Tests: TestRunResourceDetail_ItemEnvelopeRenders (decodes {"item":{...}} and
asserts fields render), TestProvisionTimeout_SurfacesOrphanGuidance (forces a
real client timeout, asserts the guidance). Both proven red-without-fix /
green-with-fix. Gate green: go build/vet/test -short, gofmt, golangci-lint 0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(cli): pin every isTimeoutErr branch for 100% patch coverage

The diff-cover gate flagged cmd/monitor.go 297-298 (nil-error guard) and
304-305 (net.Error Timeout()==true branch) as uncovered: the existing
provision-timeout regression test only reaches the errors.Is(DeadlineExceeded)
path because http.Client.Timeout errors match DeadlineExceeded first.

Add TestIsTimeoutErr — a table-driven unit test with a synthetic net.Error
(fakeNetTimeoutErr, message via named const) covering nil, bare/wrapped
DeadlineExceeded, bare/wrapped timeout net.Error, non-timeout net.Error,
and plain errors. isTimeoutErr is now 100% covered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: mastermanas805

cmd/bughunt_p2_test.go

@@ -21,6 +21,7 @@ package cmd
 import (
 	"encoding/json"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
@@ -141,6 +142,44 @@ func TestBugHunt_T16_P2_2_ProvisionTimeoutAtLeast60s(t *testing.T) {
 	}
 }
 
+// TestProvisionTimeout_SurfacesOrphanGuidance is the BUG 2 durability
+// regression: a provision that hits the client/context deadline must NOT exit
+// with a bare "context deadline exceeded" — the resource may have already
+// landed server-side (provisioning is synchronous on the api), so the user
+// needs an actionable next step to find + clean up the potential orphan.
+//
+// We point a short-timeout client at a server that never responds, forcing a
+// real client-timeout (*url.Error with Timeout()==true) through
+// provisionResource, and assert the named-const guidance is surfaced.
+func TestProvisionTimeout_SurfacesOrphanGuidance(t *testing.T) {
+	block := make(chan struct{})
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		<-block // hang until the client times out
+	}))
+	defer srv.Close()
+	defer close(block)
+
+	prevURL, prevClient := APIBaseURL, HTTPClient
+	APIBaseURL = srv.URL
+	HTTPClient = &http.Client{Timeout: 50 * time.Millisecond}
+	t.Cleanup(func() { APIBaseURL, HTTPClient = prevURL, prevClient })
+
+	_, err := provisionResource("/db/new", "orphan-on-timeout", "")
+	if err == nil {
+		t.Fatal("provision against a hanging server must error (client timeout)")
+	}
+	msg := err.Error()
+	if !strings.Contains(msg, provisionTimeoutGuidance) {
+		t.Errorf("timeout error must surface orphan guidance %q; got %q", provisionTimeoutGuidance, msg)
+	}
+	// The guidance must name the recovery commands so an agent/user can act.
+	for _, want := range []string{"instant resources", "instant resource delete"} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("timeout guidance must mention %q; got %q", want, msg)
+		}
+	}
+}
+
 // ── T16 P2-3 — `up` env default is "development", not "production".
 
 // TestBugHunt_T16_P2_3_UpDefaultsToDevelopmentNotProd asserts an `up` run with

cmd/extras.go

@@ -141,15 +141,23 @@ func runResourceDetail(cmd *cobra.Command, token string) error {
 		return parseAPIError(resp.StatusCode, raw)
 	}
 
-	// The API may return the bare resource object OR an envelope:
-	// {ok:true, resource:{...}}. Accept either shape.
+	// The API may return the bare resource object OR an envelope. Prod
+	// GET /api/v1/resources/:token wraps the object under "item"
+	// ({"item":{...},"ok":true}); older/alternate shapes use "resource".
+	// Accept "item" first (today's prod), then "resource" (back-compat),
+	// then fall back to the bare object — so a future un-enveloped response
+	// still renders.
 	var envelope struct {
 		OK       bool            `json:"ok"`
+		Item     json.RawMessage `json:"item"`
 		Resource json.RawMessage `json:"resource"`
 	}
 	_ = json.Unmarshal(raw, &envelope)
 	body := raw
-	if len(envelope.Resource) > 0 {
+	switch {
+	case len(envelope.Item) > 0:
+		body = envelope.Item
+	case len(envelope.Resource) > 0:
 		body = envelope.Resource
 	}
 

cmd/extras_test.go

@@ -131,6 +131,58 @@ func TestRunResourceDetail_Success_JSON(t *testing.T) {
 	}
 }
 
+// TestRunResourceDetail_ItemEnvelopeRenders is the BUG 1 regression: prod
+// GET /api/v1/resources/:token wraps the object under "item"
+// ({"item":{...},"ok":true}). Before the fix the detail decoder only knew
+// the bare-object and "resource"-envelope shapes, so it rendered an all-empty
+// object with exit 0 (silent). This asserts the "item" key is unwrapped and
+// the fields actually render to stdout — mirroring how discover.go's list
+// path keys off "items".
+func TestRunResourceDetail_ItemEnvelopeRenders(t *testing.T) {
+	withCleanState(t)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"ok":true,"item":{
+			"token":"tok-item","id":"id-item","resource_type":"postgres","name":"app-item",
+			"env":"production","tier":"pro","status":"active",
+			"connection_url":"postgres://u:p@x/db"
+		}}`))
+	}))
+	defer srv.Close()
+	prev := APIBaseURL
+	APIBaseURL = srv.URL
+	t.Cleanup(func() { APIBaseURL = prev })
+
+	// runResourceDetail short-circuits with errAuthRequired unless haveAuth()
+	// is true — wire an authTransport so the request actually reaches the mock.
+	prevClient := HTTPClient
+	HTTPClient = &http.Client{Transport: &authTransport{base: http.DefaultTransport, apiKey: "x"}}
+	t.Cleanup(func() { HTTPClient = prevClient })
+
+	stdout, _ := captureStdout(t, func() {
+		if err := runResourceDetail(nil, "tok-item"); err != nil {
+			t.Fatalf("runResourceDetail (item envelope): %v", err)
+		}
+	})
+
+	// The fields nested under "item" must render — an all-empty object is the
+	// pre-fix bug.
+	for _, want := range []string{
+		"tok-item",            // TOKEN
+		"id-item",             // ID
+		"postgres",            // TYPE
+		"app-item",            // NAME
+		"production",          // ENV
+		"pro",                 // TIER
+		"active",              // STATUS
+		"postgres://u:p@x/db", // URL
+	} {
+		if !strings.Contains(stdout, want) {
+			t.Errorf("item-envelope detail must render %q; stdout=%q", want, stdout)
+		}
+	}
+}
+
 func TestRunResourceDelete_EmptyToken(t *testing.T) {
 	err := runResourceDelete(nil, "")
 	if err == nil || !strings.Contains(err.Error(), "token is required") {

cmd/monitor.go

@@ -2,9 +2,12 @@ package cmd
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"os"
 	"regexp"
@@ -67,20 +70,22 @@ func validateResourceName(name string) error {
 // storage / webhook / vector) MUST reject unknown sub-sub-commands with a
 // non-zero exit. The previous behaviour was:
 //
-//   instant db delete <id>   → prints help, exits 0
+//	instant db delete <id>   → prints help, exits 0
 //
 // which silently hid typo bugs in agent scripts and let `... | xargs instant`
 // pipelines look successful. The pattern below combines:
 //
-//   1. Args: cobra.NoArgs            — refuses any positional arg
-//   2. RunE: showGroupHelp           — when called with zero args, shows
-//                                      help and exits 0 (the legacy path)
-//   3. cobra's built-in "did you mean?" suggestions surface for typos that
-//      are within 2 edits of a valid subcommand (cobra default).
+//  1. Args: cobra.NoArgs            — refuses any positional arg
+//  2. RunE: showGroupHelp           — when called with zero args, shows
+//     help and exits 0 (the legacy path)
+//  3. cobra's built-in "did you mean?" suggestions surface for typos that
+//     are within 2 edits of a valid subcommand (cobra default).
 //
 // Together, `instant db delete <id>` now errors with:
-//   Error: unknown command "delete" for "instant db"
-//   Run 'instant db --help' for usage.
+//
+//	Error: unknown command "delete" for "instant db"
+//	Run 'instant db --help' for usage.
+//
 // and exits 1.
 func showGroupHelp(cmd *cobra.Command, args []string) error {
 	return cmd.Help()
@@ -248,6 +253,13 @@ func provisionResource(endpoint, name, env string) (*provisionResponse, error) {
 
 	resp, err := HTTPClient.Post(url, "application/json", bytes.NewReader(body))
 	if err != nil {
+		// A provision that hit the client/context deadline may have ALREADY
+		// landed server-side (provisioning is synchronous on the api) — exit 1
+		// alone hides a possible orphan. Surface actionable durability guidance
+		// while preserving the underlying cause for %w-aware callers.
+		if isTimeoutErr(err) {
+			return nil, fmt.Errorf("%s (%w)", provisionTimeoutGuidance, err)
+		}
 		return nil, err
 	}
 	defer func() { _ = resp.Body.Close() }()
@@ -273,6 +285,27 @@ func provisionResource(endpoint, name, env string) (*provisionResponse, error) {
 	return &result, nil
 }
 
+// isTimeoutErr reports whether err is a request timeout — either the
+// http.Client.Timeout firing (surfaces as a net.Error with Timeout()==true,
+// wrapped in a *url.Error) or a context deadline being exceeded. Both mean the
+// provision request was abandoned client-side, so the resource may still be
+// landing server-side. Kept separate from json_error.go's network classifier
+// because that path emits a generic "network_error"; here we want the orphan
+// durability hint specifically on the timeout case.
+func isTimeoutErr(err error) bool {
+	if err == nil {
+		return false
+	}
+	if errors.Is(err, context.DeadlineExceeded) {
+		return true
+	}
+	var netErr net.Error
+	if errors.As(err, &netErr) && netErr.Timeout() {
+		return true
+	}
+	return false
+}
+
 // ── status command ────────────────────────────────────────────────────────────
 
 // statusJSON is the --json flag for `instant status`. T16 P3: machine-readable

cmd/monitor_test.go

@@ -6,6 +6,10 @@ package cmd
 
 import (
 	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -140,3 +144,47 @@ func withTestAPI(t *testing.T, baseURL string) {
 		HTTPClient = prevClient
 	})
 }
+
+// ── isTimeoutErr unit coverage ────────────────────────────────────────────────
+
+// fakeNetTimeoutErrMsg is the message carried by fakeNetTimeoutErr; named so
+// tests don't scatter string literals.
+const fakeNetTimeoutErrMsg = "dial tcp: i/o timeout (synthetic)"
+
+// fakeNetTimeoutErr is a minimal net.Error with a configurable Timeout().
+// It deliberately does NOT wrap context.DeadlineExceeded, so it exercises the
+// errors.As(net.Error) branch of isTimeoutErr rather than the errors.Is one
+// (an http.Client.Timeout error matches DeadlineExceeded first, leaving the
+// net.Error branch unreachable through provisionResource alone).
+type fakeNetTimeoutErr struct{ timeout bool }
+
+func (e fakeNetTimeoutErr) Error() string   { return fakeNetTimeoutErrMsg }
+func (e fakeNetTimeoutErr) Timeout() bool   { return e.timeout }
+func (e fakeNetTimeoutErr) Temporary() bool { return e.timeout }
+
+// TestIsTimeoutErr pins every branch of isTimeoutErr: the nil guard, the
+// context.DeadlineExceeded path, the net.Error-with-Timeout() path (including
+// when wrapped, as *url.Error does), and the non-timeout fallthrough.
+func TestIsTimeoutErr(t *testing.T) {
+	// Compile-time proof the fake satisfies net.Error.
+	var _ net.Error = fakeNetTimeoutErr{}
+
+	cases := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{"nil error is not a timeout", nil, false},
+		{"context deadline exceeded", context.DeadlineExceeded, true},
+		{"wrapped context deadline exceeded", fmt.Errorf("post: %w", context.DeadlineExceeded), true},
+		{"net.Error with Timeout()==true", fakeNetTimeoutErr{timeout: true}, true},
+		{"wrapped net.Error with Timeout()==true", fmt.Errorf("post: %w", fakeNetTimeoutErr{timeout: true}), true},
+		{"net.Error with Timeout()==false", fakeNetTimeoutErr{timeout: false}, false},
+		{"plain non-timeout error", errors.New("connection refused"), false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, isTimeoutErr(tc.err))
+		})
+	}
+}

cmd/root.go

@@ -9,9 +9,9 @@ import (
 	"strings"
 	"time"
 
-	"github.com/spf13/cobra"
 	"github.com/InstaNode-dev/cli/internal/cliconfig"
 	"github.com/InstaNode-dev/cli/internal/secretstore"
+	"github.com/spf13/cobra"
 )
 
 var _ = httpListTimeout // documented constant; referenced in tests / future refactor
@@ -64,6 +64,15 @@ const httpListTimeout = 10 * time.Second
 // CLI simplicity; <=0 falls back to the default).
 const httpProvisionTimeout = 60 * time.Second
 
+// provisionTimeoutGuidance is appended to a provision error when the request
+// hit the client/context deadline. Provisioning is synchronous server-side, so
+// a timeout does NOT mean the resource was not created — it may have landed and
+// become an orphan the user can't see. Surface an actionable next step rather
+// than a bare "context deadline exceeded". Named const (not an inline literal)
+// so the message is greppable + asserted by the timeout regression test.
+const provisionTimeoutGuidance = "Request timed out, but the resource may still be provisioning — " +
+	"run `instant resources` to check (and `instant resource delete <token> --yes` if it's an orphan)."
+
 // HTTPClient is the shared HTTP client used by all subcommands.
 // It is configured with the auth transport during init.
 //
@@ -348,4 +357,3 @@ func initConfig() {
 		},
 	}
 }
-