fix: four CLI and MCP bugs (#91, #74, #87, #73)

pkg/cli/cmd_namespace.go

@@ -45,16 +45,26 @@ func newNamespaceListCmd(deps *Deps) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "list",
 		Short: "list the namespaces you belong to",
-		Args:  cobra.NoArgs,
+		Long: `List the namespaces you belong to as NAMESPACE, KIND, ROLE, HUB.
+
+With no --hub every configured hub is queried and each row names its source
+hub, so the same namespace name on two hubs stays two distinct rows. A hub that
+cannot be reached is reported on stderr and the remaining hubs still list.`,
+		Args: cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) error {
 			// --hub is the global keg-resolution flag (default: the resolved default hub).
 			opts := tapper.NamespaceListOptions{Hub: globalKegTarget(deps).Hub}
-			nss, err := deps.Tap.NamespaceList(cmd.Context(), opts)
+			res, err := deps.Tap.NamespaceList(cmd.Context(), opts)
 			if err != nil {
 				return err
 			}
-			for _, ns := range nss {
-				fmt.Fprintf(cmd.OutOrStdout(), "@%s\t%s\t%s\n", ns.Name, ns.Kind, ns.Role)
+			for _, ns := range res.Namespaces {
+				fmt.Fprintf(cmd.OutOrStdout(), "@%s\t%s\t%s\t%s\n", ns.Name, ns.Kind, ns.Role, ns.Hub)
+			}
+			// Warnings go to stderr so the rows stay pipeable, and name the hub
+			// that failed so a partial listing is never mistaken for a full one.
+			for _, w := range res.Warnings {
+				fmt.Fprintf(cmd.ErrOrStderr(), "warning: %s\n", w)
 			}
 			return nil
 		},

pkg/keg/keg_integer_schema_test.go

@@ -0,0 +1,84 @@
+package keg_test
+
+import (
+	"testing"
+
+	"github.com/jlrickert/tapper/pkg/keg"
+	"github.com/stretchr/testify/require"
+)
+
+// changeSchema is the schema from tapper#91: a required field typed integer.
+const changeSchema = `type: change
+meta:
+  type: object
+  required: [type, contract_count]
+  properties:
+    type: {const: change}
+    contract_count: {type: integer, minimum: 1}
+markdown:
+  requireTitle: true
+`
+
+// TestCreate_IntegerFrontmatterSatisfiesSchema is the issue's reproduction, end
+// to end through the real create path rather than at the metadata layer alone.
+// Before the fix this failed with:
+//
+//	validating /properties/contract_count: type: 1 has type "string", want "integer"
+//
+// which was a closed loop: the node could not be created, and meta could not
+// repair it because meta validates the whole node including markdown sections.
+func TestCreate_IntegerFrontmatterSatisfiesSchema(t *testing.T) {
+	fx := NewSandbox(t)
+	ctx := fx.Context()
+	k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime())
+	require.NoError(t, k.Init(ctx))
+	require.NoError(t, k.CreateSchema(ctx, "change", []byte(changeSchema)))
+
+	body := "---\ntype: change\ncontract_count: 1\n---\n\n# Widen the contract\n\nBody text.\n"
+	results, err := k.CreateNodes(ctx, []keg.NodeCreate{
+		{Key: "n", Schema: "change", Body: []byte(body)},
+	})
+	require.NoError(t, err)
+	require.Len(t, results, 1)
+	if v := results[0].Validation; v != nil {
+		require.True(t, v.Valid, "schema validation failed: %+v", v)
+	}
+
+	// And the value is stored as a number, not a quoted string.
+	meta, err := k.GetMeta(ctx, results[0].ID)
+	require.NoError(t, err)
+	require.Contains(t, meta.ToYAML(), "contract_count: 1")
+	require.NotContains(t, meta.ToYAML(), `contract_count: "1"`)
+}
+
+// TestUpdate_IntegerFrontmatterSurvivesEdit covers the edit half of the issue:
+// a body carrying inline frontmatter runs through the same metadata path.
+func TestUpdate_IntegerFrontmatterSurvivesEdit(t *testing.T) {
+	fx := NewSandbox(t)
+	ctx := fx.Context()
+	k := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime())
+	require.NoError(t, k.Init(ctx))
+	require.NoError(t, k.CreateSchema(ctx, "change", []byte(changeSchema)))
+
+	results, err := k.CreateNodes(ctx, []keg.NodeCreate{{
+		Key:    "n",
+		Schema: "change",
+		Body:   []byte("---\ntype: change\ncontract_count: 1\n---\n\n# Title\n\nBody.\n"),
+	}})
+	require.NoError(t, err)
+
+	_, err = k.UpdateNode(ctx, keg.NodeUpdateOptions{
+		ID:         results[0].ID,
+		Schema:     "change",
+		Content:    []byte("---\ntype: change\ncontract_count: 7\n---\n\n# Title\n\nEdited.\n"),
+		HasContent: true,
+		// Writes are guarded; carry the hash the create returned.
+		ExpectedHash: results[0].Hash,
+	})
+	require.NoError(t, err)
+
+	meta, err := k.GetMeta(ctx, results[0].ID)
+	require.NoError(t, err)
+	require.Contains(t, meta.ToYAML(), "contract_count: 7")
+	require.NotContains(t, meta.ToYAML(), `contract_count: "7"`)
+}

pkg/keg/node_meta.go

@@ -4,7 +4,9 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"math"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -524,11 +526,12 @@ func valueToYAMLNode(v any) *yaml.Node {
 	case bool:
 		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!bool", Value: fmt.Sprint(t)}
 	case int, int8, int16, int32, int64:
-		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: fmt.Sprint(t)}
+		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!int", Value: fmt.Sprint(t)}
 	case uint, uint8, uint16, uint32, uint64:
-		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: fmt.Sprint(t)}
+		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!int", Value: fmt.Sprint(t)}
 	case float32, float64:
-		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: fmt.Sprint(t)}
+		tag, text := formatYAMLFloat(t)
+		return &yaml.Node{Kind: yaml.ScalarNode, Tag: tag, Value: text}
 	case time.Time:
 		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: t.Format(time.RFC3339)}
 	case []string:
@@ -546,13 +549,45 @@ func valueToYAMLNode(v any) *yaml.Node {
 		return seq
 	case map[string]any:
 		mnode := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"}
-		for k, v2 := range t {
+		keys := make([]string, 0, len(t))
+		for k := range t {
+			keys = append(keys, k)
+		}
+		// Sorted so repeated writes of the same value produce byte-identical
+		// YAML; ranging a Go map directly would reorder keys run to run.
+		sort.Strings(keys)
+		for _, k := range keys {
 			mnode.Content = append(mnode.Content,
 				&yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: k},
-				valueToYAMLNode(v2))
+				valueToYAMLNode(t[k]))
 		}
 		return mnode
 	default:
 		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: fmt.Sprint(v)}
 	}
 }
+
+// formatYAMLFloat renders a float as a YAML scalar and reports the tag it
+// should carry. JSON has no integer type, so a value that arrived over the MCP
+// wire as `1` reaches us as float64(1). Tagging that !!float and writing "1"
+// would make the emitter spell out an explicit `!!float` tag, and writing "1.0"
+// would fail a JSON Schema `type: integer` check that the same value satisfies
+// in JSON. Integral floats therefore serialize as plain integers, which is the
+// representation both YAML and JSON Schema agree on.
+func formatYAMLFloat(v any) (string, string) {
+	var f float64
+	switch t := v.(type) {
+	case float32:
+		f = float64(t)
+	case float64:
+		f = t
+	}
+	if math.IsInf(f, 0) || math.IsNaN(f) {
+		// Not representable as a plain YAML scalar of either numeric tag.
+		return "!!str", fmt.Sprint(f)
+	}
+	if f == math.Trunc(f) && math.Abs(f) < 1e15 {
+		return "!!int", strconv.FormatInt(int64(f), 10)
+	}
+	return "!!float", strconv.FormatFloat(f, 'g', -1, 64)
+}

pkg/keg/node_meta_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/jlrickert/tapper/pkg/keg"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 )
 
 func TestParseMeta_EmptyReturnsEmptyMeta(t *testing.T) {
@@ -181,3 +182,69 @@ tags:
 	require.Contains(t, out, "- gamma")
 	require.NotContains(t, out, "- alpha")
 }
+
+// TestSetAttrs_PreservesScalarTypes covers tapper#91: SetAttrs used to tag every
+// numeric scalar !!str, so a schema field typed `integer` could never be
+// satisfied through create or through an edit carrying inline frontmatter.
+func TestSetAttrs_PreservesScalarTypes(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	m, err := keg.ParseMeta(ctx, []byte("# initial\ntype: change\n"))
+	require.NoError(t, err)
+
+	require.NoError(t, m.SetAttrs(ctx, map[string]any{
+		// YAML frontmatter decodes an integer literal as int.
+		"contract_count": 1,
+		// JSON (the MCP attrs path) has no integer type, so the same value
+		// arrives as float64 and must still write as an integer.
+		"json_count": float64(2),
+		"ratio":      0.75,
+		"enabled":    true,
+		"label":      "text",
+	}))
+
+	out := m.ToYAML()
+	require.Contains(t, out, "contract_count: 1")
+	require.Contains(t, out, "json_count: 2")
+	require.Contains(t, out, "ratio: 0.75")
+	require.Contains(t, out, "enabled: true")
+	require.Contains(t, out, "label: text")
+	// The bug's signature was a quoted scalar where a number belonged.
+	require.NotContains(t, out, `"1"`)
+	require.NotContains(t, out, `"0.75"`)
+
+	// Round-trip through YAML to confirm the emitted tags actually resolve back
+	// to numbers rather than to strings that merely look unquoted.
+	var back map[string]any
+	require.NoError(t, yaml.Unmarshal([]byte(out), &back))
+	require.Equal(t, 1, back["contract_count"])
+	require.Equal(t, 2, back["json_count"])
+	require.Equal(t, 0.75, back["ratio"])
+	require.Equal(t, true, back["enabled"])
+	require.Equal(t, "text", back["label"])
+}
+
+// TestSetAttrs_NestedMapsAreDeterministic guards the sorted key order in
+// valueToYAMLNode: ranging a Go map directly reordered nested keys run to run,
+// which made otherwise identical writes produce different meta.yaml bytes.
+func TestSetAttrs_NestedMapsAreDeterministic(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	render := func() string {
+		m, err := keg.ParseMeta(ctx, []byte("# initial\n"))
+		require.NoError(t, err)
+		require.NoError(t, m.SetAttrs(ctx, map[string]any{
+			"nested": map[string]any{
+				"zulu": 1, "alpha": 2, "mike": 3, "delta": 4, "papa": 5,
+			},
+		}))
+		return m.ToYAML()
+	}
+
+	first := render()
+	for i := 0; i < 8; i++ {
+		require.Equal(t, first, render())
+	}
+}

pkg/mcp/auth_error_internal_test.go

@@ -0,0 +1,116 @@
+package mcp
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/jlrickert/tapper/pkg/keg"
+	"github.com/jlrickert/tapper/pkg/tapper"
+)
+
+// TestErrorResult_AuthFailuresCarryGuidance covers tapper#87: an auth failure
+// used to fall through to bare error text with no structured content, leaving
+// an agent no indication that reorienting is the refresh boundary.
+func TestErrorResult_AuthFailuresCarryGuidance(t *testing.T) {
+	tests := []struct {
+		name         string
+		err          error
+		wantCode     string
+		wantReorient bool
+	}{
+		{
+			name:         "unauthorized",
+			err:          fmt.Errorf("hub call: %w", keg.ErrUnauthorized),
+			wantCode:     "UNAUTHORIZED",
+			wantReorient: true,
+		},
+		{
+			name:         "token rejected",
+			err:          fmt.Errorf("hub: %w (401 Unauthorized)", tapper.ErrTokenRejected),
+			wantCode:     "UNAUTHORIZED",
+			wantReorient: true,
+		},
+		{
+			// A grant the user does not have is not fixed by logging in, so
+			// this must not send the agent into a login/reorient loop.
+			name:         "forbidden",
+			err:          fmt.Errorf("hub call: %w", keg.ErrForbidden),
+			wantCode:     "FORBIDDEN",
+			wantReorient: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			res := errorResult(tc.err)
+			if !res.IsError {
+				t.Fatalf("IsError = false, want true")
+			}
+			structured, ok := res.StructuredContent.(map[string]any)
+			if !ok {
+				t.Fatalf("StructuredContent = %T, want map[string]any", res.StructuredContent)
+			}
+			if got := structured["code"]; got != tc.wantCode {
+				t.Fatalf("code = %v, want %v", got, tc.wantCode)
+			}
+			if got := structured["reorientRequired"]; got != tc.wantReorient {
+				t.Fatalf("reorientRequired = %v, want %v", got, tc.wantReorient)
+			}
+			if got := structured["operationPerformed"]; got != false {
+				t.Fatalf("operationPerformed = %v, want false", got)
+			}
+			action, _ := structured["action"].(string)
+			if strings.TrimSpace(action) == "" {
+				t.Fatalf("action is empty; the agent gets no actionable guidance")
+			}
+			if tc.wantReorient {
+				if !strings.Contains(action, "tap auth login") || !strings.Contains(action, "reorient") {
+					t.Fatalf("action %q does not mention logging in and reorienting", action)
+				}
+				// The issue explicitly asks that guidance never tell a caller
+				// to kill a host-owned process.
+				for _, banned := range []string{"restart the host", "kill", "restart your"} {
+					if strings.Contains(strings.ToLower(action), banned) &&
+						!strings.Contains(action, "restarting the host is not required") {
+						t.Fatalf("action %q tells the caller to restart or kill the host", action)
+					}
+				}
+			}
+		})
+	}
+}
+
+// TestErrorResult_AuthFailuresLeakNoCredentials guards the acceptance criterion
+// that no credential detail reaches the agent transcript.
+func TestErrorResult_AuthFailuresLeakNoCredentials(t *testing.T) {
+	const secret = "thub_supersecrettokenvalue"
+	res := errorResult(fmt.Errorf("request with token %s: %w", secret, keg.ErrUnauthorized))
+
+	structured, _ := res.StructuredContent.(map[string]any)
+	if action, _ := structured["action"].(string); strings.Contains(action, secret) {
+		t.Fatalf("action leaked the credential")
+	}
+	if code, _ := structured["code"].(string); strings.Contains(code, secret) {
+		t.Fatalf("code leaked the credential")
+	}
+}
+
+// TestErrorResult_NonAuthErrorsAreUnchanged confirms the new branch did not
+// swallow the precondition paths that sit next to it.
+func TestErrorResult_NonAuthErrorsAreUnchanged(t *testing.T) {
+	res := errorResult(errors.New("something ordinary went wrong"))
+	if res.StructuredContent != nil {
+		t.Fatalf("StructuredContent = %v, want nil for a plain error", res.StructuredContent)
+	}
+
+	res = errorResult(fmt.Errorf("write: %w", keg.ErrPreconditionRequired))
+	structured, ok := res.StructuredContent.(map[string]any)
+	if !ok {
+		t.Fatalf("precondition error lost its structured content")
+	}
+	if structured["code"] != keg.RemoteCodePreconditionRequired {
+		t.Fatalf("code = %v, want %v", structured["code"], keg.RemoteCodePreconditionRequired)
+	}
+}