Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,31 @@ Every command supports `--json` for structured output:

Breadcrumbs suggest next commands, making it easy for humans and agents to navigate.

Errors use the same envelope with `ok: false`, a stable `code`, and `retryable`,
which says whether a retry can change the outcome:

```json
{
"ok": false,
"error": "Gateway error (503)",
"code": "api_error",
"retryable": true,
"hint": "..."
}
```

Key order within the envelope is not part of the contract — the interactive
(TTY) path re-encodes through a map and alphabetizes keys — so match on key
names, never on position.

`retryable` is present on every error envelope — `true` when the CLI classified
the failure transient (network, timeout, rate limit, circuit open, and most
5xx/gateway responses — not all: 507 and some 500s are verdicts), `false` for a
verdict (usage, not found, auth, forbidden, validation, account limit) and for
any error nothing classified — and never on a success envelope. Key on it rather
than on the code or message when deciding whether to retry; `false` means no
known reason a retry would help, not a guarantee the failure is permanent.

## Authentication

OAuth 2.1 with automatic token refresh. First login opens your browser.
Expand Down
84 changes: 84 additions & 0 deletions e2e/errors.bats
Original file line number Diff line number Diff line change
Expand Up @@ -481,5 +481,89 @@ load test_helper
assert_failure
assert_json_value '.ok' 'false'
assert_json_value '.code' 'usage'
assert_json_value '.retryable' 'false'
assert_output_contains '"error"'
}

# Every error envelope carries "retryable" so a consumer can tell a transient
# failure from a verdict without parsing the code or message. A stub API that
# answers 503 exercises the SDK classification end to end; a mutation is used
# because the SDK retries idempotent reads on its own schedule.

start_unavailable_api_stub() {
UNAVAILABLE_STUB_PORT_FILE="$TEST_TEMP_DIR/unavailable-stub.port"
local log_file="$TEST_TEMP_DIR/unavailable-stub.log"

local python_bin
if command -v python3 >/dev/null 2>&1; then
python_bin=python3
elif command -v python >/dev/null 2>&1; then
python_bin=python
else
echo "Error: neither python3 nor python is available in PATH; cannot start unavailable API stub" >&2
return 1
fi

"$python_bin" - <<'PY' "$UNAVAILABLE_STUB_PORT_FILE" >"$log_file" 2>&1 3>&- &
import http.server
import socketserver
import sys

port_file = sys.argv[1]

class Handler(http.server.BaseHTTPRequestHandler):
def _unavailable(self):
self.send_response(503)
self.send_header('Content-Type', 'application/json')
self.end_headers()
self.wfile.write(b'{"error":"service unavailable"}')

do_GET = _unavailable
do_POST = _unavailable

def log_message(self, format, *args):
pass

with socketserver.TCPServer(('127.0.0.1', 0), Handler) as server:
with open(port_file, 'w', encoding='utf-8') as f:
f.write(str(server.server_address[1]))
server.serve_forever()
PY
UNAVAILABLE_STUB_PID=$!

for _ in $(seq 1 50); do
[[ -s "$UNAVAILABLE_STUB_PORT_FILE" ]] && break
sleep 0.1
done

if [[ ! -s "$UNAVAILABLE_STUB_PORT_FILE" ]]; then
echo "failed to start unavailable API stub. Log:" >&2
cat "$log_file" >&2
stop_unavailable_api_stub
return 1
fi

export BASECAMP_BASE_URL="http://127.0.0.1:$(cat "$UNAVAILABLE_STUB_PORT_FILE")"
}

stop_unavailable_api_stub() {
if [[ -n "${UNAVAILABLE_STUB_PID:-}" ]]; then
kill "$UNAVAILABLE_STUB_PID" 2>/dev/null || true
wait "$UNAVAILABLE_STUB_PID" 2>/dev/null || true
unset UNAVAILABLE_STUB_PID
fi
}

@test "transient API failure returns retryable JSON envelope" {
start_unavailable_api_stub
create_credentials
create_global_config '{"account_id": 99999}'

run basecamp projects create "Launch plan" --json
stop_unavailable_api_stub

assert_failure
assert_json_value '.ok' 'false'
assert_json_value '.code' 'api_error'
assert_json_value '.retryable' 'true'
}
40 changes: 40 additions & 0 deletions internal/commands/projects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"

Expand Down Expand Up @@ -150,3 +151,42 @@ type projectUpdateEnvelope struct {
UpdatedAt string `json:"updated_at"`
} `json:"data"`
}

// A failed command reaches the JSON error envelope with a top-level
// "retryable" that carries the SDK's classification end to end (SDK error →
// convertSDKError → app.Err): a transient 503 says retry, a 404 verdict says
// don't. Consumers deciding whether to retry key on that field rather than
// on the code or message. A mutation is used because the generated client
// retries idempotent operations on its own schedule, which is not the
// behavior under test here.
func TestProjectsCreateErrorEnvelopeCarriesRetryable(t *testing.T) {
for _, tc := range []struct {
name string
status int
code string
retryable bool
}{
{"gateway 503", http.StatusServiceUnavailable, basecamp.CodeAPI, true},
{"not found 404", http.StatusNotFound, basecamp.CodeNotFound, false},
} {
t.Run(tc.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(tc.status)
fmt.Fprint(w, `{"error":"upstream said no"}`)
}))
t.Cleanup(server.Close)
app, buf := newRequestLevelApp(t, server.URL)

err := executeCommand(NewProjectsCmd(), app, "create", "Launch plan")
require.Error(t, err)
require.NoError(t, app.Err(err))

var decoded map[string]any
require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded), "envelope: %s", buf.String())
assert.Equal(t, false, decoded["ok"])
assert.Equal(t, tc.code, decoded["code"])
assert.Equal(t, tc.retryable, decoded["retryable"], "envelope: %s", buf.String())
})
}
}
29 changes: 20 additions & 9 deletions internal/output/envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,22 @@ type Breadcrumb struct {
}

// ErrorResponse is the error envelope for JSON output.
//
// Retryable is always present on an error envelope and never on a success
// one: true when the failure was classified transient (network, timeout,
// rate limit, circuit open, bulkhead full, and most 5xx/gateway responses —
// not all: 507 and a raw-client 500 are verdicts), false for a verdict
// (usage, not found, auth, forbidden, validation, account limit) and for any
// error nothing classified. Consumers key on the field rather than on the
// code or message, which describe the failure and not whether a retry can
// change it.
type ErrorResponse struct {
OK bool `json:"ok"`
Error string `json:"error"`
Code string `json:"code"`
Hint string `json:"hint,omitempty"`
Meta map[string]any `json:"meta,omitempty"`
OK bool `json:"ok"`
Error string `json:"error"`
Code string `json:"code"`
Retryable bool `json:"retryable"`
Comment thread
jeremy marked this conversation as resolved.
Hint string `json:"hint,omitempty"`
Meta map[string]any `json:"meta,omitempty"`
}

// Format specifies the output format.
Expand Down Expand Up @@ -156,10 +166,11 @@ func (w *Writer) OK(data any, opts ...ResponseOption) error {
func (w *Writer) Err(err error, opts ...ErrorResponseOption) error {
e := AsError(err)
resp := &ErrorResponse{
OK: false,
Error: e.Message,
Code: e.Code,
Hint: e.Hint,
OK: false,
Error: e.Message,
Code: e.Code,
Retryable: e.Retryable,
Hint: e.Hint,
}
if requestID := RequestID(err); requestID != "" {
if resp.Meta == nil {
Expand Down
119 changes: 119 additions & 0 deletions internal/output/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,125 @@ func TestWriterErrIncludesRequestIDMeta(t *testing.T) {
assert.Equal(t, "req-cli-123", resp.Meta["request_id"])
}

// The error envelope carries a top-level "retryable" that mirrors the
// error's Retryable classification. A consumer deciding whether to retry keys
// on it, not on the code or message — so it must be present (as false) on
// every error envelope, not just the transient ones, and never leak onto a
// success envelope.
func TestWriterErrEmitsRetryableForTransientError(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

err := w.Err(&basecamp.Error{
Code: basecamp.CodeAPI,
Message: "Gateway error (503)",
HTTPStatus: 503,
Retryable: true,
})
require.NoError(t, err, "Err() failed")

decoded := decodeErrorEnvelope(t, buf.Bytes())
assert.Equal(t, false, decoded["ok"])
assert.Equal(t, CodeAPI, decoded["code"])
assert.Equal(t, true, decoded["retryable"])
}

func TestWriterErrEmitsRetryableFalseForVerdict(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

require.NoError(t, w.Err(ErrNotFound("project", "123")))

decoded := decodeErrorEnvelope(t, buf.Bytes())
retryable, present := decoded["retryable"]
assert.True(t, present, "retryable must be present on every error envelope, false included")
assert.Equal(t, false, retryable)
}

func TestWriterErrRetryableForCLIClassifiedErrors(t *testing.T) {
for name, err := range map[string]error{
"rate limit": ErrRateLimit(30),
"network": ErrNetwork(errors.New("dial tcp: connection refused")),
"wrapped": fmt.Errorf("listing projects: %w", ErrNetwork(errors.New("timeout"))),
} {
t.Run(name, func(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

require.NoError(t, w.Err(err))

assert.Equal(t, true, decodeErrorEnvelope(t, buf.Bytes())["retryable"])
})
}
}

func TestWriterErrRetryableFalseForUnclassifiedError(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

require.NoError(t, w.Err(errors.New("something unexpected")))

decoded := decodeErrorEnvelope(t, buf.Bytes())
assert.Equal(t, CodeAPI, decoded["code"])
assert.Equal(t, false, decoded["retryable"], "no positive signal means not retryable")
}

// Field order is part of the piped-envelope contract: retryable sits between
// code and hint, and the existing fields keep their positions. The pin covers
// the non-TTY representation machine consumers read; on a TTY, writeJSON
// re-encodes the envelope through map-backed sanitization, so keys come out
// alphabetized there — that path serves a human reader and carries no order
// contract.
Comment thread
jeremy marked this conversation as resolved.
func TestWriterErrRetryableFieldOrder(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

require.NoError(t, w.Err(ErrRateLimit(30)))

out := buf.String()
positions := []int{
strings.Index(out, `"ok"`),
strings.Index(out, `"error"`),
strings.Index(out, `"code"`),
strings.Index(out, `"retryable"`),
strings.Index(out, `"hint"`),
}
for i, pos := range positions {
require.NotEqual(t, -1, pos, "field %d missing from %s", i, out)
if i > 0 {
assert.Greater(t, pos, positions[i-1], "field order drifted in %s", out)
}
}
}

func TestWriterErrQuietModeCarriesRetryable(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatQuiet, Writer: &buf})

require.NoError(t, w.Err(ErrRateLimit(30)))

assert.Equal(t, true, decodeErrorEnvelope(t, buf.Bytes())["retryable"])
}

func TestWriterOKOmitsRetryable(t *testing.T) {
var buf bytes.Buffer
w := New(Options{Format: FormatJSON, Writer: &buf})

require.NoError(t, w.OK(map[string]string{"id": "123"}))

var decoded map[string]any
require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded))
_, present := decoded["retryable"]
assert.False(t, present, "retryable is an error-envelope field only")
}

func decodeErrorEnvelope(t *testing.T, raw []byte) map[string]any {
t.Helper()
var decoded map[string]any
require.NoError(t, json.Unmarshal(raw, &decoded), "Failed to unmarshal %s", raw)
return decoded
}

func TestWriterQuietFormat(t *testing.T) {
var buf bytes.Buffer
w := New(Options{
Expand Down
15 changes: 12 additions & 3 deletions skills/basecamp/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ Full CLI coverage: 155 endpoints across todos, cards, messages, files, schedule,
|------|------|--------|
| Filter/extract JSON data | `--jq '<expr>'` | Built-in jq filter (no external jq needed). Implies `--json`; filter runs on the envelope. |
| Filter in agent mode | `--agent --jq '<expr>'` | Filter runs on data-only payload (no envelope), matching `--agent` contract. |
| Full JSON output | `--json` | JSON envelope: `{ok, data, summary, breadcrumbs, meta}` |
| Full JSON output | `--json` | JSON envelope: `{ok, data, summary, breadcrumbs, meta}`; errors: `{ok:false, error, code, retryable, hint, meta}` |
| Show results to a user | `--md` / `-m` | GFM tables, task lists, structured Markdown |
| Automation / scripting | `--agent` | Success: raw JSON data (no envelope); errors: `{ok:false,...}` object; no interactive prompts |

Expand Down Expand Up @@ -1349,15 +1349,24 @@ the specific argument. Use this for elicitation:

```bash
$ basecamp todos create --json
{"ok": false, "error": "<content> required", "code": "usage",
{"ok": false, "error": "<content> required", "code": "usage", "retryable": false,
"hint": "Usage: basecamp todos create <content>"}

$ basecamp comments create 123 --json
{"ok": false, "error": "<content> required", "code": "usage", ...}
{"ok": false, "error": "<content> required", "code": "usage", "retryable": false, ...}
```

The `error` field names the missing `<arg>` — use it to prompt the user for the specific value.

**Retryable errors (`retryable`):** every error envelope carries a boolean `retryable` —
`true` when the CLI classified the failure transient (network, timeout, rate limit,
circuit open, most 5xx/gateway responses — not all: 507 and some 500s are verdicts) and
a retry can change the outcome, `false` for a verdict (usage, not found, auth, forbidden,
validation, account limit) and for any error nothing classified. Key on it rather than
on `code` or `error` when deciding whether to retry — `false` means no known reason a
retry would help, not a guarantee of permanence; it is never present on a success
envelope.

**URL malformed (curl exit 3):** Special characters in content. Use plain text or properly escaped HTML.

## Built-in jq Filtering
Expand Down
Loading