diff --git a/README.md b/README.md index b1bc9ce6..42c9ec9d 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,16 @@ make logs # Tail logs (goclaw service) make reset # Wipe volumes and rebuild from scratch ``` +**Operator CLI:** + +The main `goclaw` binary can also inspect local or remote gateways: + +```bash +goclaw traces list --status error +goclaw traces get -o json +goclaw --server https://goclaw.example.com --token "$GOCLAW_GATEWAY_TOKEN" traces follow --session +``` + **Optional services** — enable with `WITH_*` flags: | Flag | Service | What it does | diff --git a/cmd/gateway_http_client.go b/cmd/gateway_http_client.go index 9508fa69..39dbc2ec 100644 --- a/cmd/gateway_http_client.go +++ b/cmd/gateway_http_client.go @@ -7,10 +7,7 @@ import ( "io" "net/http" "os" - "strings" "time" - - "github.com/nextlevelbuilder/goclaw/internal/config" ) // gatewayHTTPError represents a structured error from the gateway HTTP API. @@ -28,38 +25,7 @@ var httpClient = &http.Client{Timeout: 10 * time.Second} // healthClient has a shorter timeout for quick health checks. var healthClient = &http.Client{Timeout: 3 * time.Second} -// resolveGatewayBaseURL reads host/port from config and returns http://host:port. -func resolveGatewayBaseURL() string { - if server := strings.TrimSpace(os.Getenv("GOCLAW_SERVER")); server != "" { - return strings.TrimRight(server, "/") - } - cfg, err := config.Load(resolveConfigPath()) - if err != nil { - return "http://127.0.0.1:18790" - } - host := cfg.Gateway.Host - if host == "" || host == "0.0.0.0" { - host = "127.0.0.1" - } - port := cfg.Gateway.Port - if port == 0 { - port = 18790 - } - return fmt.Sprintf("http://%s:%d", host, port) -} - -// resolveGatewayToken returns the gateway auth token. -// Priority: GOCLAW_GATEWAY_TOKEN env → config file token. -func resolveGatewayToken() string { - if t := os.Getenv("GOCLAW_GATEWAY_TOKEN"); t != "" { - return t - } - cfg, _ := config.Load(resolveConfigPath()) - if cfg != nil { - return cfg.Gateway.Token - } - return "" -} +const gatewayHTTPResponseLimit = 1 << 20 // gatewayHTTPDo sends an HTTP request to the gateway with auth and returns the parsed JSON response. func gatewayHTTPDo(method, path string, body any) (map[string]any, error) { @@ -99,6 +65,10 @@ func gatewayHTTPPut(path string, body any) (map[string]any, error) { return gatewayHTTPDo(http.MethodPut, path, body) } +func gatewayHTTPPatch(path string, body any) (map[string]any, error) { + return gatewayHTTPDo(http.MethodPatch, path, body) +} + func gatewayHTTPDelete(path string) error { _, err := gatewayHTTPDo(http.MethodDelete, path, nil) return err @@ -107,6 +77,10 @@ func gatewayHTTPDelete(path string) error { // gatewayHTTPDoRaw executes an HTTP request and returns the raw response bytes. // Shared by both map-based and typed response functions. func gatewayHTTPDoRaw(method, path string, body any) ([]byte, int, error) { + return gatewayHTTPDoRawWithLimit(method, path, body, gatewayHTTPResponseLimit) +} + +func gatewayHTTPDoRawWithLimit(method, path string, body any, limit int64) ([]byte, int, error) { base := resolveGatewayBaseURL() var bodyReader io.Reader @@ -134,7 +108,13 @@ func gatewayHTTPDoRaw(method, path string, body any) ([]byte, int, error) { } defer resp.Body.Close() - raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + raw, err := io.ReadAll(io.LimitReader(resp.Body, limit+1)) + if err != nil { + return nil, resp.StatusCode, fmt.Errorf("read gateway response: %w", err) + } + if int64(len(raw)) > limit { + return nil, resp.StatusCode, fmt.Errorf("gateway response exceeds %d bytes", limit) + } return raw, resp.StatusCode, nil } diff --git a/cmd/gateway_http_client_resolution.go b/cmd/gateway_http_client_resolution.go new file mode 100644 index 00000000..37204dbf --- /dev/null +++ b/cmd/gateway_http_client_resolution.go @@ -0,0 +1,88 @@ +package cmd + +import ( + "fmt" + "net/url" + "os" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/config" +) + +// resolveGatewayBaseURL reads host/port from config and returns http://host:port. +func resolveGatewayBaseURL() string { + if base := firstNonEmpty(gatewayServerOverride, os.Getenv("GOCLAW_SERVER"), os.Getenv("GOCLAW_GATEWAY_URL")); base != "" { + return normalizeGatewayBaseURL(base) + } + + cfg, err := config.Load(resolveConfigPath()) + if err != nil { + return "http://127.0.0.1:18790" + } + host := cfg.Gateway.Host + if host == "" || host == "0.0.0.0" { + host = "127.0.0.1" + } + port := cfg.Gateway.Port + if port == 0 { + port = 18790 + } + return fmt.Sprintf("http://%s:%d", host, port) +} + +// resolveGatewayToken returns the gateway auth token. +// Priority: --token flag -> GOCLAW_GATEWAY_TOKEN env -> config file token. +func resolveGatewayToken() string { + if t := strings.TrimSpace(gatewayTokenOverride); t != "" { + return t + } + if t := os.Getenv("GOCLAW_GATEWAY_TOKEN"); t != "" { + return t + } + cfg, _ := config.Load(resolveConfigPath()) + if cfg != nil { + return cfg.Gateway.Token + } + return "" +} + +func resolveGatewayWebSocketURL() (string, error) { + baseURL := resolveGatewayBaseURL() + parsed, err := url.Parse(baseURL) + if err != nil { + return "", fmt.Errorf("parse gateway URL %q: %w", baseURL, err) + } + switch parsed.Scheme { + case "https": + parsed.Scheme = "wss" + case "http": + parsed.Scheme = "ws" + case "ws", "wss": + default: + return "", fmt.Errorf("unsupported gateway URL scheme %q", parsed.Scheme) + } + parsed.Path = strings.TrimRight(parsed.Path, "/") + "/ws" + parsed.RawQuery = "" + parsed.Fragment = "" + return parsed.String(), nil +} + +func normalizeGatewayBaseURL(raw string) string { + base := strings.TrimSpace(raw) + if base == "" { + return "" + } + if !strings.HasPrefix(base, "http://") && !strings.HasPrefix(base, "https://") { + base = "http://" + base + } + return strings.TrimRight(base, "/") +} + +func firstNonEmpty(values ...string) string { + for _, v := range values { + if trimmed := strings.TrimSpace(v); trimmed != "" { + return trimmed + } + } + return "" +} diff --git a/cmd/gateway_http_client_test.go b/cmd/gateway_http_client_test.go index e41c783e..1552fc00 100644 --- a/cmd/gateway_http_client_test.go +++ b/cmd/gateway_http_client_test.go @@ -1,12 +1,136 @@ package cmd -import "testing" +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestResolveGatewayClientOverrides(t *testing.T) { + t.Setenv("GOCLAW_GATEWAY_TOKEN", "env-token") + t.Setenv("GOCLAW_SERVER", "") + t.Setenv("GOCLAW_GATEWAY_URL", "") + + oldServer := gatewayServerOverride + oldToken := gatewayTokenOverride + oldCfg := cfgFile + t.Cleanup(func() { + gatewayServerOverride = oldServer + gatewayTokenOverride = oldToken + cfgFile = oldCfg + }) + + cfgFile = "/path/that/does/not/exist" + gatewayServerOverride = "https://goclaw.example.com/" + gatewayTokenOverride = "flag-token" + + if got := resolveGatewayBaseURL(); got != "https://goclaw.example.com" { + t.Fatalf("resolveGatewayBaseURL() = %q, want trimmed override", got) + } + if got := resolveGatewayToken(); got != "flag-token" { + t.Fatalf("resolveGatewayToken() = %q, want flag token", got) + } + + gatewayTokenOverride = "" + if got := resolveGatewayToken(); got != "env-token" { + t.Fatalf("resolveGatewayToken() = %q, want env token", got) + } + + gatewayServerOverride = "" + t.Setenv("GOCLAW_SERVER", "remote.example.com:18790/") + if got := resolveGatewayBaseURL(); got != "http://remote.example.com:18790" { + t.Fatalf("resolveGatewayBaseURL() = %q, want normalized env URL", got) + } -func TestResolveGatewayBaseURLUsesGOCLAWServer(t *testing.T) { t.Setenv("GOCLAW_SERVER", "http://127.0.0.1:19999/") + if got := resolveGatewayBaseURL(); got != "http://127.0.0.1:19999" { + t.Fatalf("resolveGatewayBaseURL() = %q, want trimmed GOCLAW_SERVER URL", got) + } +} - got := resolveGatewayBaseURL() - if got != "http://127.0.0.1:19999" { - t.Fatalf("resolveGatewayBaseURL()=%q, want GOCLAW_SERVER without trailing slash", got) +func TestGatewayHTTPDoRawUsesServerAndTokenOverride(t *testing.T) { + oldServer := gatewayServerOverride + oldToken := gatewayTokenOverride + oldClient := httpClient + t.Cleanup(func() { + gatewayServerOverride = oldServer + gatewayTokenOverride = oldToken + httpClient = oldClient + }) + + var sawRequest bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawRequest = true + if r.URL.Path != "/v1/traces" { + t.Errorf("path = %q, want /v1/traces", r.URL.Path) + } + if got := r.Header.Get("Authorization"); got != "Bearer flag-token" { + t.Errorf("Authorization = %q, want Bearer flag-token", got) + } + if got := r.Header.Get("X-GoClaw-User-Id"); got != "system" { + t.Errorf("X-GoClaw-User-Id = %q, want system", got) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer srv.Close() + + gatewayServerOverride = srv.URL + gatewayTokenOverride = "flag-token" + httpClient = srv.Client() + + raw, status, err := gatewayHTTPDoRaw(http.MethodGet, "/v1/traces", nil) + if err != nil { + t.Fatalf("gatewayHTTPDoRaw: %v", err) + } + if status != http.StatusOK { + t.Fatalf("status = %d, want 200", status) + } + if string(raw) != `{"ok":true}` { + t.Fatalf("raw = %s", raw) + } + if !sawRequest { + t.Fatal("test server did not receive request") + } +} + +func TestResolveGatewayWebSocketURLUsesServerOverride(t *testing.T) { + oldServer := gatewayServerOverride + oldCfg := cfgFile + t.Cleanup(func() { + gatewayServerOverride = oldServer + cfgFile = oldCfg + }) + + cfgFile = "/path/that/does/not/exist" + gatewayServerOverride = "https://goclaw.example.com/base/" + + got, err := resolveGatewayWebSocketURL() + if err != nil { + t.Fatalf("resolveGatewayWebSocketURL: %v", err) + } + if got != "wss://goclaw.example.com/base/ws" { + t.Fatalf("resolveGatewayWebSocketURL() = %q", got) + } +} + +func TestGatewayHTTPDoRawWithLimitRejectsOversizedResponse(t *testing.T) { + oldServer := gatewayServerOverride + oldClient := httpClient + t.Cleanup(func() { + gatewayServerOverride = oldServer + httpClient = oldClient + }) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("12345")) + })) + defer srv.Close() + + gatewayServerOverride = srv.URL + httpClient = srv.Client() + + if _, _, err := gatewayHTTPDoRawWithLimit(http.MethodGet, "/big", nil, 4); err == nil { + t.Fatal("expected oversized response error") } } diff --git a/cmd/pairing.go b/cmd/pairing.go index aed8b9a0..d2d652e4 100644 --- a/cmd/pairing.go +++ b/cmd/pairing.go @@ -3,14 +3,12 @@ package cmd import ( "encoding/json" "fmt" - "net/url" "os" "time" "github.com/gorilla/websocket" "github.com/spf13/cobra" - "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/pkg/protocol" ) @@ -187,26 +185,20 @@ func pairingRevokeCmd() *cobra.Command { // gatewayRPC connects to the running gateway, authenticates, sends an RPC call, and returns the response. func gatewayRPC(method string, params json.RawMessage) (*protocol.ResponseFrame, error) { - cfg, err := config.Load(resolveConfigPath()) + wsURL, err := resolveGatewayWebSocketURL() if err != nil { - return nil, fmt.Errorf("load config: %w", err) + return nil, err } - host := cfg.Gateway.Host - if host == "0.0.0.0" { - host = "127.0.0.1" - } - - u := url.URL{Scheme: "ws", Host: fmt.Sprintf("%s:%d", host, cfg.Gateway.Port), Path: "/ws"} - conn, _, err := websocket.DefaultDialer.Dial(u.String(), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) if err != nil { - return nil, fmt.Errorf("connect to gateway at %s: %w", u.String(), err) + return nil, fmt.Errorf("connect to gateway at %s: %w", wsURL, err) } defer conn.Close() // Step 1: Send connect handshake connectParams, _ := json.Marshal(map[string]any{ - "token": cfg.Gateway.Token, + "token": resolveGatewayToken(), "protocol": protocol.ProtocolVersion, }) connectReq := protocol.RequestFrame{ diff --git a/cmd/root.go b/cmd/root.go index 8f4fd9a4..4e812115 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -15,6 +15,10 @@ var Version = "dev" var ( cfgFile string verbose bool + + gatewayServerOverride string + gatewayTokenOverride string + gatewayOutputFormat string ) var rootCmd = &cobra.Command{ @@ -29,6 +33,8 @@ var rootCmd = &cobra.Command{ func init() { rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default: config.json or $GOCLAW_CONFIG)") rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "enable debug logging") + rootCmd.PersistentFlags().StringVar(&gatewayServerOverride, "server", "", "gateway server URL override") + rootCmd.PersistentFlags().StringVar(&gatewayTokenOverride, "token", "", "gateway bearer token override") rootCmd.AddCommand(onboardCmd()) rootCmd.AddCommand(versionCmd()) @@ -42,6 +48,7 @@ func init() { rootCmd.AddCommand(cronCmd()) rootCmd.AddCommand(skillsCmd()) rootCmd.AddCommand(sessionsCmd()) + rootCmd.AddCommand(tracesCmd()) rootCmd.AddCommand(migrateCmd()) rootCmd.AddCommand(upgradeCmd()) rootCmd.AddCommand(backupCmd()) diff --git a/cmd/skills_cmd.go b/cmd/skills_cmd.go index d87e1706..d34285af 100644 --- a/cmd/skills_cmd.go +++ b/cmd/skills_cmd.go @@ -22,6 +22,10 @@ func skillsCmd() *cobra.Command { } cmd.AddCommand(skillsListCmd()) cmd.AddCommand(skillsShowCmd()) + cmd.AddCommand(skillsDepsCmd()) + cmd.AddCommand(skillsAccessCmd()) + cmd.AddCommand(skillsGrantCmd()) + cmd.AddCommand(skillsRevokeCmd()) return cmd } diff --git a/cmd/skills_lifecycle_cmd.go b/cmd/skills_lifecycle_cmd.go new file mode 100644 index 00000000..bf9fc27c --- /dev/null +++ b/cmd/skills_lifecycle_cmd.go @@ -0,0 +1,166 @@ +package cmd + +import ( + "fmt" + "net/http" + "net/url" + + "github.com/spf13/cobra" +) + +var ( + skillsGatewayDo = gatewayHTTPDo + skillsGatewayDelete = gatewayHTTPDelete + skillsRequireGateway = requireRunningGatewayHTTP +) + +func skillsDepsCmd() *cobra.Command { + cmd := &cobra.Command{Use: "deps", Short: "Scan, check, and install skill dependencies"} + cmd.AddCommand(skillsDepsReadCmd("status", http.MethodGet, "Show dependency status")) + cmd.AddCommand(skillsDepsReadCmd("scan", http.MethodPost, "Scan dependency declarations")) + cmd.AddCommand(skillsDepsReadCmd("check", http.MethodPost, "Check dependency availability")) + cmd.AddCommand(skillsDepsInstallCmd()) + return cmd +} + +func skillsDepsReadCmd(name, method, short string) *cobra.Command { + var jsonOutput bool + cmd := &cobra.Command{ + Use: name + " [skill-id-or-path]", + Short: short, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if pathExists(args[0]) { + return runLocalSkillDepsStatus(cmd.OutOrStdout(), args[0], jsonOutput) + } + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/dependencies" + if method == http.MethodPost { + path += "/" + name + } + return runSkillsGateway(cmd.OutOrStdout(), method, path, nil, jsonOutput) + }, + } + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsDepsInstallCmd() *cobra.Command { + var jsonOutput bool + cmd := &cobra.Command{ + Use: "install [skill-id]", + Short: "Install missing dependencies for a managed skill", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/dependencies/install" + return runSkillsGateway(cmd.OutOrStdout(), http.MethodPost, path, nil, jsonOutput) + }, + } + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsAccessCmd() *cobra.Command { + cmd := &cobra.Command{Use: "access", Short: "Manage skill access mode and effective access"} + cmd.AddCommand(skillsAccessGetCmd()) + cmd.AddCommand(skillsAccessSetCmd()) + cmd.AddCommand(skillsAccessEffectiveCmd()) + return cmd +} + +func skillsAccessGetCmd() *cobra.Command { + var jsonOutput bool + cmd := &cobra.Command{ + Use: "get [skill-id]", + Short: "Show skill access mode and grants", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/access" + return runSkillsGateway(cmd.OutOrStdout(), http.MethodGet, path, nil, jsonOutput) + }, + } + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsAccessSetCmd() *cobra.Command { + var mode string + var jsonOutput bool + cmd := &cobra.Command{ + Use: "set [skill-id]", + Short: "Set skill access mode", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if mode == "" { + return fmt.Errorf("--mode is required") + } + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/access" + return runSkillsGateway(cmd.OutOrStdout(), http.MethodPatch, path, map[string]any{"mode": mode}, jsonOutput) + }, + } + cmd.Flags().StringVar(&mode, "mode", "", "access mode: private, internal, public") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsAccessEffectiveCmd() *cobra.Command { + var agentID, userID string + var jsonOutput bool + cmd := &cobra.Command{ + Use: "effective [skill-id]", + Short: "Inspect effective access for an agent and user", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if agentID == "" || userID == "" { + return fmt.Errorf("--agent and --user are required") + } + skillsRequireGateway() + values := url.Values{"agent_id": {agentID}, "user_id": {userID}} + path := "/v1/skills/access/effective" + if len(args) == 1 { + path = "/v1/skills/" + url.PathEscape(args[0]) + "/access/effective" + } + return runSkillsGateway(cmd.OutOrStdout(), http.MethodGet, path+"?"+values.Encode(), nil, jsonOutput) + }, + } + cmd.Flags().StringVar(&agentID, "agent", "", "agent ID") + cmd.Flags().StringVar(&userID, "user", "", "user ID") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsGrantCmd() *cobra.Command { + cmd := &cobra.Command{Use: "grant", Short: "Grant skill access"} + cmd.AddCommand(skillsGrantAgentCmd()) + cmd.AddCommand(skillsGrantUserCmd()) + return cmd +} + +func skillsGrantAgentCmd() *cobra.Command { + var canManage, jsonOutput bool + var pinnedVersion int + cmd := &cobra.Command{ + Use: "agent [skill-id] [agent-id]", + Short: "Grant a skill to an agent", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + body := map[string]any{"agent_id": args[1]} + if canManage { + body["can_manage"] = true + } + if pinnedVersion > 0 { + body["pinned_version"] = pinnedVersion + } + path := "/v1/skills/" + url.PathEscape(args[0]) + "/grants/agents" + return runSkillsGateway(cmd.OutOrStdout(), http.MethodPost, path, body, jsonOutput) + }, + } + cmd.Flags().BoolVar(&canManage, "can-manage", false, "grant manage permission") + cmd.Flags().IntVar(&pinnedVersion, "pinned-version", 0, "pin a specific skill version") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} diff --git a/cmd/skills_lifecycle_cmd_test.go b/cmd/skills_lifecycle_cmd_test.go new file mode 100644 index 00000000..b3a80f6a --- /dev/null +++ b/cmd/skills_lifecycle_cmd_test.go @@ -0,0 +1,159 @@ +package cmd + +import ( + "bytes" + "io" + "net/http" + "os" + "reflect" + "testing" +) + +func TestSkillsDepsInstallUsesPerSkillInstallEndpoint(t *testing.T) { + calls := captureSkillGatewayCalls(t) + + cmd := skillsDepsCmd() + cmd.SetArgs([]string{"install", "skill-123", "--json"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + + if len(calls.requests) != 1 { + t.Fatalf("requests = %d, want 1", len(calls.requests)) + } + got := calls.requests[0] + if got.method != http.MethodPost || got.path != "/v1/skills/skill-123/dependencies/install" { + t.Fatalf("request = %+v", got) + } +} + +func TestSkillsAccessSetUsesPatchModeBody(t *testing.T) { + calls := captureSkillGatewayCalls(t) + + cmd := skillsAccessCmd() + cmd.SetArgs([]string{"set", "skill-123", "--mode", "public", "--json"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + + if len(calls.requests) != 1 { + t.Fatalf("requests = %d, want 1", len(calls.requests)) + } + got := calls.requests[0] + if got.method != http.MethodPatch || got.path != "/v1/skills/skill-123/access" { + t.Fatalf("request = %+v", got) + } + if !reflect.DeepEqual(got.body, map[string]any{"mode": "public"}) { + t.Fatalf("body = %#v", got.body) + } +} + +func TestSkillsGrantAgentUsesPluralGrantEndpoint(t *testing.T) { + calls := captureSkillGatewayCalls(t) + + cmd := skillsGrantCmd() + cmd.SetArgs([]string{"agent", "skill-123", "agent-456", "--can-manage", "--pinned-version", "7", "--json"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + + if len(calls.requests) != 1 { + t.Fatalf("requests = %d, want 1", len(calls.requests)) + } + got := calls.requests[0] + if got.method != http.MethodPost || got.path != "/v1/skills/skill-123/grants/agents" { + t.Fatalf("request = %+v", got) + } + want := map[string]any{"agent_id": "agent-456", "can_manage": true, "pinned_version": 7} + if !reflect.DeepEqual(got.body, want) { + t.Fatalf("body = %#v, want %#v", got.body, want) + } +} + +func TestSkillsAccessEffectiveBuildsOptionalSkillURL(t *testing.T) { + calls := captureSkillGatewayCalls(t) + + cmd := skillsAccessCmd() + cmd.SetArgs([]string{"effective", "skill-123", "--agent", "agent-456", "--user", "user-789", "--json"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + + if len(calls.requests) != 1 { + t.Fatalf("requests = %d, want 1", len(calls.requests)) + } + got := calls.requests[0] + wantPath := "/v1/skills/skill-123/access/effective?agent_id=agent-456&user_id=user-789" + if got.method != http.MethodGet || got.path != wantPath { + t.Fatalf("request = %+v, want path %s", got, wantPath) + } +} + +type skillGatewayCall struct { + method string + path string + body any +} + +type capturedSkillGatewayCalls struct { + requests []skillGatewayCall +} + +func captureSkillGatewayCalls(t *testing.T) *capturedSkillGatewayCalls { + t.Helper() + calls := &capturedSkillGatewayCalls{} + prevDo := skillsGatewayDo + prevDelete := skillsGatewayDelete + prevRequire := skillsRequireGateway + prevStdout := os.Stdout + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + t.Cleanup(func() { + _ = w.Close() + _, _ = io.Copy(io.Discard, r) + _ = r.Close() + os.Stdout = prevStdout + skillsGatewayDo = prevDo + skillsGatewayDelete = prevDelete + skillsRequireGateway = prevRequire + }) + + skillsRequireGateway = func() {} + skillsGatewayDo = func(method, path string, body any) (map[string]any, error) { + calls.requests = append(calls.requests, skillGatewayCall{method: method, path: path, body: body}) + return map[string]any{"ok": true}, nil + } + skillsGatewayDelete = func(path string) error { + calls.requests = append(calls.requests, skillGatewayCall{method: http.MethodDelete, path: path}) + return nil + } + + return calls +} + +func TestRunLocalSkillDepsStatusPrintsJSON(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(dir+"/SKILL.md", []byte("---\nname: Demo\ndeps:\n - system:goclaw-missing-test-bin\n---\n"), 0o644); err != nil { + t.Fatalf("write SKILL.md: %v", err) + } + + var out bytes.Buffer + if err := runLocalSkillDepsStatus(&out, dir, true); err != nil { + t.Fatalf("runLocalSkillDepsStatus: %v", err) + } + if !bytes.Contains(out.Bytes(), []byte(`"missing_count": 1`)) { + t.Fatalf("output = %s", out.String()) + } +} diff --git a/cmd/skills_lifecycle_helpers.go b/cmd/skills_lifecycle_helpers.go new file mode 100644 index 00000000..2953c54b --- /dev/null +++ b/cmd/skills_lifecycle_helpers.go @@ -0,0 +1,134 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/nextlevelbuilder/goclaw/internal/skills" +) + +func skillsGrantUserCmd() *cobra.Command { + var jsonOutput bool + cmd := &cobra.Command{ + Use: "user [skill-id] [user-id]", + Short: "Grant a skill to a user", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/grants/users" + return runSkillsGateway(cmd.OutOrStdout(), http.MethodPost, path, map[string]any{"user_id": args[1]}, jsonOutput) + }, + } + cmd.Flags().BoolVar(&jsonOutput, "json", false, "output as JSON") + return cmd +} + +func skillsRevokeCmd() *cobra.Command { + cmd := &cobra.Command{Use: "revoke", Short: "Revoke skill access"} + cmd.AddCommand(skillsRevokeAgentCmd()) + cmd.AddCommand(skillsRevokeUserCmd()) + return cmd +} + +func skillsRevokeAgentCmd() *cobra.Command { + return &cobra.Command{ + Use: "agent [skill-id] [agent-id]", + Short: "Revoke a skill from an agent", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/grants/agents/" + url.PathEscape(args[1]) + return runSkillsGatewayDelete(cmd.OutOrStdout(), path) + }, + } +} + +func skillsRevokeUserCmd() *cobra.Command { + return &cobra.Command{ + Use: "user [skill-id] [user-id]", + Short: "Revoke a skill from a user", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + skillsRequireGateway() + path := "/v1/skills/" + url.PathEscape(args[0]) + "/grants/users/" + url.PathEscape(args[1]) + return runSkillsGatewayDelete(cmd.OutOrStdout(), path) + }, + } +} + +func runSkillsGateway(w io.Writer, method, path string, body any, jsonOutput bool) error { + resp, err := skillsGatewayDo(method, path, body) + if err != nil { + return err + } + if jsonOutput { + return writePrettyJSON(w, resp) + } + if ok, _ := resp["ok"].(bool); ok { + _, _ = fmt.Fprintln(w, "ok") + return nil + } + return writePrettyJSON(w, resp) +} + +func runSkillsGatewayDelete(w io.Writer, path string) error { + if err := skillsGatewayDelete(path); err != nil { + return err + } + _, _ = fmt.Fprintln(w, "ok") + return nil +} + +func runLocalSkillDepsStatus(w io.Writer, target string, jsonOutput bool) error { + dir := target + if filepath.Base(target) == "SKILL.md" { + dir = filepath.Dir(target) + } + manifest := skills.ScanSkillDeps(dir) + ok, missing := skills.CheckSkillDeps(manifest) + resp := map[string]any{ + "skill": map[string]any{"path": dir}, + "ok": ok, + "status": localDepsStatus(ok), + "manifest": manifest, + "missing": missing, + "missing_count": len(missing), + } + if jsonOutput { + return writePrettyJSON(w, resp) + } + if ok { + _, _ = fmt.Fprintln(w, "all deps satisfied") + return nil + } + _, _ = fmt.Fprintf(w, "missing dependencies: %s\n", skills.FormatMissing(missing)) + return nil +} + +func writePrettyJSON(w io.Writer, v any) error { + data, err := json.MarshalIndent(v, "", " ") + if err != nil { + return err + } + _, err = fmt.Fprintln(w, string(data)) + return err +} + +func pathExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +func localDepsStatus(ok bool) string { + if ok { + return "ok" + } + return "missing" +} diff --git a/cmd/traces_cmd.go b/cmd/traces_cmd.go new file mode 100644 index 00000000..456c023e --- /dev/null +++ b/cmd/traces_cmd.go @@ -0,0 +1,122 @@ +package cmd + +import "github.com/spf13/cobra" + +const traceExportResponseLimit = 64 << 20 + +func tracesCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "traces", + Short: "Inspect gateway traces", + } + cmd.PersistentFlags().StringVarP(&gatewayOutputFormat, "output", "o", "table", "output format (table|json)") + cmd.AddCommand(tracesListCmd()) + cmd.AddCommand(tracesGetCmd()) + cmd.AddCommand(tracesExportCmd()) + cmd.AddCommand(tracesFollowCmd()) + cmd.AddCommand(tracesTimelineCmd()) + return cmd +} + +func tracesListCmd() *cobra.Command { + var opts traceListOptions + cmd := &cobra.Command{ + Use: "list", + Short: "List traces", + RunE: func(cmd *cobra.Command, args []string) error { + requireRunningGatewayHTTP() + return runTracesList(opts) + }, + } + addTraceListFlags(cmd, &opts) + return cmd +} + +func tracesGetCmd() *cobra.Command { + return &cobra.Command{ + Use: "get ", + Short: "Get trace details with spans", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + requireRunningGatewayHTTP() + return runTracesGet(args[0]) + }, + } +} + +func tracesExportCmd() *cobra.Command { + var filePath string + cmd := &cobra.Command{ + Use: "export ", + Short: "Export a gzipped trace tree", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + requireRunningGatewayHTTP() + return runTracesExport(args[0], filePath) + }, + } + cmd.Flags().StringVar(&filePath, "file", "", "write gzip export to file (use - for stdout)") + return cmd +} + +func tracesFollowCmd() *cobra.Command { + var opts traceFollowOptions + cmd := &cobra.Command{ + Use: "follow", + Short: "Poll trace changes for a session or agent", + RunE: func(cmd *cobra.Command, args []string) error { + requireRunningGatewayHTTP() + return runTracesFollow(opts) + }, + } + cmd.Flags().StringVar(&opts.SessionKey, "session", "", "filter by session key") + cmd.Flags().StringVar(&opts.AgentID, "agent-id", "", "filter by agent UUID") + cmd.Flags().StringVar(&opts.UserID, "user", "", "filter by user ID for admin callers") + cmd.Flags().StringVar(&opts.Status, "status", "", "filter by trace status") + cmd.Flags().StringVar(&opts.Channel, "channel", "", "filter by raw channel") + cmd.Flags().StringVar(&opts.Since, "since", "", "RFC3339 lower bound for changed traces") + cmd.Flags().IntVar(&opts.Limit, "limit", 0, "page size, max 200") + cmd.Flags().BoolVar(&opts.IncludeSpans, "include-spans", false, "include spans grouped by trace ID") + return cmd +} + +func tracesTimelineCmd() *cobra.Command { + var opts traceTimelineOptions + cmd := &cobra.Command{ + Use: "timeline ", + Short: "Show the run timeline linked to a trace", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + requireRunningGatewayHTTP() + return runTracesTimeline(args[0], opts) + }, + } + cmd.Flags().IntVar(&opts.Limit, "limit", 0, "page size, max 500") + cmd.Flags().IntVar(&opts.Offset, "offset", 0, "pagination offset") + return cmd +} + +func addTraceListFlags(cmd *cobra.Command, opts *traceListOptions) { + cmd.Flags().StringVarP(&opts.Query, "query", "q", "", "search trace text, IDs, labels, and span previews") + cmd.Flags().StringVar(&opts.AgentID, "agent-id", "", "filter by agent UUID") + cmd.Flags().StringVar(&opts.UserID, "user", "", "filter by user ID for admin callers") + cmd.Flags().StringVar(&opts.SessionKey, "session", "", "filter by session key") + cmd.Flags().StringVar(&opts.Status, "status", "", "filter by trace status") + cmd.Flags().StringVar(&opts.Channel, "channel", "", "filter by raw channel") + cmd.Flags().StringVar(&opts.AgentQuery, "agent", "", "search agent display name or key") + cmd.Flags().StringVar(&opts.ChannelQuery, "channel-query", "", "search channel instance labels") + cmd.Flags().StringVar(&opts.ToolName, "tool", "", "search span tool names") + cmd.Flags().StringVar(&opts.From, "from", "", "start time lower bound, RFC3339") + cmd.Flags().StringVar(&opts.To, "to", "", "start time upper bound, RFC3339") + cmd.Flags().StringVar(&opts.Since, "since", "", "alias for --from") + cmd.Flags().StringVar(&opts.Until, "until", "", "alias for --to") + cmd.Flags().StringVar(&opts.HasToolCalls, "has-tool-calls", "", "filter true or false") + cmd.Flags().IntVar(&opts.MinInputTokens, "min-input-tokens", 0, "minimum input tokens") + cmd.Flags().IntVar(&opts.MaxInputTokens, "max-input-tokens", 0, "maximum input tokens") + cmd.Flags().IntVar(&opts.MinOutputTokens, "min-output-tokens", 0, "minimum output tokens") + cmd.Flags().IntVar(&opts.MaxOutputTokens, "max-output-tokens", 0, "maximum output tokens") + cmd.Flags().IntVar(&opts.MinToolCalls, "min-tool-calls", 0, "minimum tool calls") + cmd.Flags().IntVar(&opts.MaxToolCalls, "max-tool-calls", 0, "maximum tool calls") + cmd.Flags().IntVar(&opts.Limit, "limit", 0, "page size, max 200") + cmd.Flags().IntVar(&opts.Offset, "offset", 0, "pagination offset") +} diff --git a/cmd/traces_cmd_test.go b/cmd/traces_cmd_test.go new file mode 100644 index 00000000..9cba5db8 --- /dev/null +++ b/cmd/traces_cmd_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "net/url" + "testing" +) + +func TestBuildTraceListPathIncludesFilters(t *testing.T) { + path := buildTraceListPath(traceListOptions{ + Query: "provider fail", + SessionKey: "session A", + Status: "running", + AgentQuery: "coder", + HasToolCalls: "true", + Limit: 25, + Offset: 10, + }) + + u, err := url.Parse(path) + if err != nil { + t.Fatalf("parse path: %v", err) + } + if u.Path != "/v1/traces" { + t.Fatalf("path = %q, want /v1/traces", u.Path) + } + q := u.Query() + assertQueryValue(t, q, "q", "provider fail") + assertQueryValue(t, q, "session_key", "session A") + assertQueryValue(t, q, "status", "running") + assertQueryValue(t, q, "agent", "coder") + assertQueryValue(t, q, "has_tool_calls", "true") + assertQueryValue(t, q, "limit", "25") + assertQueryValue(t, q, "offset", "10") +} + +func TestBuildTraceFollowPathRequiresScope(t *testing.T) { + if _, err := buildTraceFollowPath(traceFollowOptions{}); err == nil { + t.Fatal("expected missing scope error") + } + + path, err := buildTraceFollowPath(traceFollowOptions{ + SessionKey: "session A", + Since: "2026-06-12T01:00:00Z", + Limit: 20, + IncludeSpans: true, + }) + if err != nil { + t.Fatalf("buildTraceFollowPath: %v", err) + } + u, err := url.Parse(path) + if err != nil { + t.Fatalf("parse path: %v", err) + } + if u.Path != "/v1/traces/follow" { + t.Fatalf("path = %q, want /v1/traces/follow", u.Path) + } + q := u.Query() + assertQueryValue(t, q, "session_key", "session A") + assertQueryValue(t, q, "since", "2026-06-12T01:00:00Z") + assertQueryValue(t, q, "limit", "20") + assertQueryValue(t, q, "include_spans", "true") +} + +func TestTraceTimelinePathUsesRunIDFromDetail(t *testing.T) { + runID, err := traceRunIDFromDetail(traceDetailResponse{ + Trace: traceDataForCLI{RunID: "run-123"}, + }) + if err != nil { + t.Fatalf("traceRunIDFromDetail: %v", err) + } + if runID != "run-123" { + t.Fatalf("runID = %q, want run-123", runID) + } + + if _, err := traceRunIDFromDetail(traceDetailResponse{}); err == nil { + t.Fatal("expected missing run_id error") + } +} + +func TestValidateTraceOutputFormatRejectsUnsupportedValues(t *testing.T) { + oldOutput := gatewayOutputFormat + t.Cleanup(func() { gatewayOutputFormat = oldOutput }) + + gatewayOutputFormat = "yaml" + if err := validateTraceOutputFormat(); err == nil { + t.Fatal("expected unsupported output format error") + } +} + +func assertQueryValue(t *testing.T, q url.Values, key, want string) { + t.Helper() + if got := q.Get(key); got != want { + t.Fatalf("query %s = %q, want %q", key, got, want) + } +} diff --git a/cmd/traces_output.go b/cmd/traces_output.go new file mode 100644 index 00000000..b83c9c11 --- /dev/null +++ b/cmd/traces_output.go @@ -0,0 +1,177 @@ +package cmd + +import ( + "bytes" + "compress/gzip" + "encoding/json" + "fmt" + "io" + "os" + "strings" + "text/tabwriter" + "time" + + "github.com/google/uuid" +) + +func outputFormatIsJSON() bool { + return strings.EqualFold(strings.TrimSpace(gatewayOutputFormat), "json") +} + +func validateTraceOutputFormat() error { + switch strings.ToLower(strings.TrimSpace(gatewayOutputFormat)) { + case "", "table", "json": + return nil + default: + return fmt.Errorf("unsupported output format %q; use table or json", gatewayOutputFormat) + } +} + +func printJSON(value any) error { + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return err + } + fmt.Println(string(data)) + return nil +} + +func printTraceList(resp traceListResponse) error { + if outputFormatIsJSON() { + return printJSON(resp) + } + if len(resp.Traces) == 0 { + fmt.Println("No traces found.") + return nil + } + tw := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "TRACE\tSTATUS\tAGENT\tSESSION\tRUN\tTOKENS\tSTARTED") + for _, tr := range resp.Traces { + fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%d/%d\t%s\n", + shortTraceID(tr.ID.String()), + tr.Status, + shortOptionalUUID(tr.AgentID), + truncateStr(tr.SessionKey, 36), + truncateStr(tr.RunID, 24), + tr.TotalInputTokens, + tr.TotalOutputTokens, + formatTraceTime(tr.StartTime), + ) + } + return tw.Flush() +} + +func printTraceDetail(resp traceDetailResponse) error { + if outputFormatIsJSON() { + return printJSON(resp) + } + tr := resp.Trace + fmt.Printf("Trace: %s\n", tr.ID) + fmt.Printf("Status: %s\n", tr.Status) + fmt.Printf("Agent: %s\n", shortOptionalUUID(tr.AgentID)) + fmt.Printf("Session: %s\n", tr.SessionKey) + fmt.Printf("Run: %s\n", tr.RunID) + fmt.Printf("Tokens: %d input / %d output\n", tr.TotalInputTokens, tr.TotalOutputTokens) + if tr.Error != "" { + fmt.Printf("Error: %s\n", tr.Error) + } + if len(resp.Spans) == 0 { + fmt.Println("\nNo spans found.") + return nil + } + fmt.Println() + tw := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "SPAN\tTYPE\tSTATUS\tPROVIDER\tMODEL\tDURATION\tNAME") + for _, sp := range resp.Spans { + fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%dms\t%s\n", + shortTraceID(sp.ID.String()), + sp.SpanType, + sp.Status, + sp.Provider, + sp.Model, + sp.DurationMS, + truncateStr(sp.Name, 48), + ) + } + return tw.Flush() +} + +func printTraceFollow(resp traceFollowResponse) error { + if outputFormatIsJSON() { + return printJSON(resp) + } + if resp.NextSince != "" { + fmt.Printf("Next since: %s\n\n", resp.NextSince) + } + return printTraceList(traceListResponse{ + Traces: resp.Traces, + Total: len(resp.Traces), + Limit: resp.Limit, + }) +} + +func printTraceTimeline(resp traceTimelineResponse) error { + if outputFormatIsJSON() { + return printJSON(resp) + } + if len(resp.Items) == 0 { + fmt.Println("No timeline items found.") + return nil + } + tw := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "SEQ\tTYPE\tSTATUS\tTITLE\tPREVIEW\tCREATED") + for _, item := range resp.Items { + fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%s\n", + item.Seq, + item.ItemType, + item.Status, + truncateStr(item.Title, 32), + truncateStr(firstNonEmpty(item.Preview, item.Content), 60), + formatTraceTime(item.CreatedAt), + ) + } + return tw.Flush() +} + +func printGzipJSON(raw []byte) error { + gr, err := gzip.NewReader(bytes.NewReader(raw)) + if err != nil { + return err + } + defer gr.Close() + data, err := io.ReadAll(gr) + if err != nil { + return err + } + var value any + if err := json.Unmarshal(data, &value); err != nil { + return err + } + return printJSON(value) +} + +func defaultTraceExportPath(traceID string) string { + short := shortTraceID(traceID) + return fmt.Sprintf("trace-%s-%s.json.gz", short, time.Now().UTC().Format("20060102")) +} + +func shortTraceID(id string) string { + if len(id) <= 8 { + return id + } + return id[:8] +} + +func shortOptionalUUID(id *uuid.UUID) string { + if id == nil { + return "-" + } + return shortTraceID(id.String()) +} + +func formatTraceTime(t time.Time) string { + if t.IsZero() { + return "-" + } + return t.Local().Format(time.DateTime) +} diff --git a/cmd/traces_paths.go b/cmd/traces_paths.go new file mode 100644 index 00000000..edc7765a --- /dev/null +++ b/cmd/traces_paths.go @@ -0,0 +1,169 @@ +package cmd + +import ( + "fmt" + "net/url" + "strconv" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +type traceDataForCLI = store.TraceData +type spanDataForCLI = store.SpanData +type timelineItemForCLI = store.RunTimelineItem + +type traceListOptions struct { + Query string + AgentID string + UserID string + SessionKey string + Status string + Channel string + AgentQuery string + ChannelQuery string + ToolName string + From string + To string + Since string + Until string + HasToolCalls string + MinInputTokens int + MaxInputTokens int + MinOutputTokens int + MaxOutputTokens int + MinToolCalls int + MaxToolCalls int + Limit int + Offset int +} + +type traceFollowOptions struct { + AgentID string + SessionKey string + Status string + Channel string + UserID string + Since string + Limit int + IncludeSpans bool +} + +type traceTimelineOptions struct { + Limit int + Offset int +} + +type traceListResponse struct { + Traces []traceDataForCLI `json:"traces"` + Total int `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` +} + +type traceDetailResponse struct { + Trace traceDataForCLI `json:"trace"` + Spans []spanDataForCLI `json:"spans"` +} + +type traceFollowResponse struct { + Traces []traceDataForCLI `json:"traces"` + SpansByTraceID map[string][]spanDataForCLI `json:"spans_by_trace_id"` + ServerTime string `json:"server_time"` + NextSince string `json:"next_since"` + Limit int `json:"limit"` +} + +type traceTimelineResponse struct { + RunID string `json:"run_id"` + SessionKey string `json:"session_key"` + Items []timelineItemForCLI `json:"items"` + Limit int `json:"limit"` + Offset int `json:"offset"` +} + +func buildTraceListPath(opts traceListOptions) string { + values := url.Values{} + addQuery(values, "q", opts.Query) + addQuery(values, "agent_id", opts.AgentID) + addQuery(values, "user_id", opts.UserID) + addQuery(values, "session_key", opts.SessionKey) + addQuery(values, "status", opts.Status) + addQuery(values, "channel", opts.Channel) + addQuery(values, "agent", opts.AgentQuery) + addQuery(values, "channel_query", opts.ChannelQuery) + addQuery(values, "tool_name", opts.ToolName) + addQuery(values, "from", firstNonEmpty(opts.From, opts.Since)) + addQuery(values, "to", firstNonEmpty(opts.To, opts.Until)) + addQuery(values, "has_tool_calls", opts.HasToolCalls) + addIntQuery(values, "min_input_tokens", opts.MinInputTokens) + addIntQuery(values, "max_input_tokens", opts.MaxInputTokens) + addIntQuery(values, "min_output_tokens", opts.MinOutputTokens) + addIntQuery(values, "max_output_tokens", opts.MaxOutputTokens) + addIntQuery(values, "min_tool_calls", opts.MinToolCalls) + addIntQuery(values, "max_tool_calls", opts.MaxToolCalls) + addIntQuery(values, "limit", opts.Limit) + addIntQuery(values, "offset", opts.Offset) + return pathWithQuery("/v1/traces", values) +} + +func buildTraceFollowPath(opts traceFollowOptions) (string, error) { + if strings.TrimSpace(opts.SessionKey) == "" && strings.TrimSpace(opts.AgentID) == "" { + return "", fmt.Errorf("traces follow requires --session or --agent-id") + } + values := url.Values{} + addQuery(values, "agent_id", opts.AgentID) + addQuery(values, "session_key", opts.SessionKey) + addQuery(values, "status", opts.Status) + addQuery(values, "channel", opts.Channel) + addQuery(values, "user_id", opts.UserID) + addQuery(values, "since", opts.Since) + addIntQuery(values, "limit", opts.Limit) + if opts.IncludeSpans { + values.Set("include_spans", "true") + } + return pathWithQuery("/v1/traces/follow", values), nil +} + +func buildTraceTimelinePath(runID, sessionKey string, opts traceTimelineOptions) string { + values := url.Values{} + addQuery(values, "session_key", sessionKey) + addIntQuery(values, "limit", opts.Limit) + addIntQuery(values, "offset", opts.Offset) + return pathWithQuery("/v1/runs/"+url.PathEscape(runID)+"/timeline", values) +} + +func traceDetailPath(traceID string) string { + return "/v1/traces/" + url.PathEscape(traceID) +} + +func traceExportPath(traceID string) string { + return "/v1/traces/" + url.PathEscape(traceID) + "/export" +} + +func traceRunIDFromDetail(detail traceDetailResponse) (string, error) { + runID := strings.TrimSpace(detail.Trace.RunID) + if runID == "" { + return "", fmt.Errorf("trace has no run_id; timeline is unavailable") + } + return runID, nil +} + +func addQuery(values url.Values, key, value string) { + if trimmed := strings.TrimSpace(value); trimmed != "" { + values.Set(key, trimmed) + } +} + +func addIntQuery(values url.Values, key string, value int) { + if value > 0 { + values.Set(key, strconv.Itoa(value)) + } +} + +func pathWithQuery(path string, values url.Values) string { + if len(values) == 0 { + return path + } + return path + "?" + values.Encode() +} diff --git a/cmd/traces_run.go b/cmd/traces_run.go new file mode 100644 index 00000000..ed93ce85 --- /dev/null +++ b/cmd/traces_run.go @@ -0,0 +1,93 @@ +package cmd + +import ( + "fmt" + "net/http" + "os" +) + +func runTracesList(opts traceListOptions) error { + if err := validateTraceOutputFormat(); err != nil { + return err + } + resp, err := gatewayHTTPGetTyped[traceListResponse](buildTraceListPath(opts)) + if err != nil { + return err + } + return printTraceList(resp) +} + +func runTracesGet(traceID string) error { + if err := validateTraceOutputFormat(); err != nil { + return err + } + resp, err := gatewayHTTPGetTyped[traceDetailResponse](traceDetailPath(traceID)) + if err != nil { + return err + } + return printTraceDetail(resp) +} + +func runTracesExport(traceID, filePath string) error { + if err := validateTraceOutputFormat(); err != nil { + return err + } + raw, status, err := gatewayHTTPDoRawWithLimit(http.MethodGet, traceExportPath(traceID), nil, traceExportResponseLimit) + if err != nil { + return err + } + if status >= 400 { + return parseHTTPError(raw, status) + } + if outputFormatIsJSON() { + return printGzipJSON(raw) + } + if filePath == "-" { + _, err = os.Stdout.Write(raw) + return err + } + if filePath == "" { + filePath = defaultTraceExportPath(traceID) + } + if err := os.WriteFile(filePath, raw, 0o600); err != nil { + return err + } + fmt.Printf("Exported trace to %s\n", filePath) + return nil +} + +func runTracesFollow(opts traceFollowOptions) error { + if err := validateTraceOutputFormat(); err != nil { + return err + } + path, err := buildTraceFollowPath(opts) + if err != nil { + return err + } + resp, err := gatewayHTTPGetTyped[traceFollowResponse](path) + if err != nil { + return err + } + return printTraceFollow(resp) +} + +func runTracesTimeline(traceID string, opts traceTimelineOptions) error { + if err := validateTraceOutputFormat(); err != nil { + return err + } + detail, err := gatewayHTTPGetTyped[traceDetailResponse](traceDetailPath(traceID)) + if err != nil { + return err + } + runID, err := traceRunIDFromDetail(detail) + if err != nil { + return err + } + resp, err := gatewayHTTPGetTyped[traceTimelineResponse]( + buildTraceTimelinePath(runID, detail.Trace.SessionKey, opts), + ) + if err != nil { + return err + } + return printTraceTimeline(resp) +} diff --git a/cmd/traces_run_test.go b/cmd/traces_run_test.go new file mode 100644 index 00000000..50b430ba --- /dev/null +++ b/cmd/traces_run_test.go @@ -0,0 +1,160 @@ +package cmd + +import ( + "bytes" + "compress/gzip" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRunTracesGetJSONOutput(t *testing.T) { + traceID := "11111111-1111-1111-1111-111111111111" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/traces/"+traceID { + t.Fatalf("path = %q", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "trace": { + "id": "` + traceID + `", + "run_id": "run-123", + "start_time": "2026-06-12T01:00:00Z", + "created_at": "2026-06-12T01:00:00Z", + "status": "completed", + "total_input_tokens": 10, + "total_output_tokens": 20 + }, + "spans": [] + }`)) + })) + defer srv.Close() + withTraceTestGateway(t, srv) + gatewayOutputFormat = "json" + + out, err := captureStdout(t, func() error { + return runTracesGet(traceID) + }) + if err != nil { + t.Fatalf("runTracesGet: %v", err) + } + if !strings.Contains(out, `"run_id": "run-123"`) { + t.Fatalf("output missing run_id: %s", out) + } +} + +func TestRunTracesExportWritesFileAndPrintsJSON(t *testing.T) { + var gzipPayload bytes.Buffer + gz := gzip.NewWriter(&gzipPayload) + _, _ = gz.Write([]byte(`{"trace":{"id":"trace-1"},"spans":[],"sub_traces":[]}`)) + if err := gz.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/traces/trace-1/export" { + t.Fatalf("path = %q", r.URL.Path) + } + w.Header().Set("Content-Type", "application/gzip") + _, _ = w.Write(gzipPayload.Bytes()) + })) + defer srv.Close() + withTraceTestGateway(t, srv) + + gatewayOutputFormat = "table" + outFile := filepath.Join(t.TempDir(), "trace.json.gz") + if _, err := captureStdout(t, func() error { + return runTracesExport("trace-1", outFile) + }); err != nil { + t.Fatalf("runTracesExport file: %v", err) + } + written, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read export: %v", err) + } + if !bytes.Equal(written, gzipPayload.Bytes()) { + t.Fatal("written gzip payload mismatch") + } + + gatewayOutputFormat = "json" + out, err := captureStdout(t, func() error { + return runTracesExport("trace-1", "") + }) + if err != nil { + t.Fatalf("runTracesExport json: %v", err) + } + if !strings.Contains(out, `"trace"`) || !strings.Contains(out, `"sub_traces"`) { + t.Fatalf("json output missing trace tree fields: %s", out) + } +} + +func TestRunTracesFollowJSONOutput(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/traces/follow" { + t.Fatalf("path = %q", r.URL.Path) + } + if got := r.URL.Query().Get("session_key"); got != "session-1" { + t.Fatalf("session_key = %q", got) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "traces": [], + "spans_by_trace_id": {}, + "server_time": "2026-06-12T01:00:00Z", + "next_since": "2026-06-12T01:00:00Z", + "limit": 50 + }`)) + })) + defer srv.Close() + withTraceTestGateway(t, srv) + gatewayOutputFormat = "json" + + out, err := captureStdout(t, func() error { + return runTracesFollow(traceFollowOptions{SessionKey: "session-1"}) + }) + if err != nil { + t.Fatalf("runTracesFollow: %v", err) + } + if !strings.Contains(out, `"next_since": "2026-06-12T01:00:00Z"`) { + t.Fatalf("output missing next_since: %s", out) + } +} + +func withTraceTestGateway(t *testing.T, srv *httptest.Server) { + t.Helper() + oldServer := gatewayServerOverride + oldToken := gatewayTokenOverride + oldOutput := gatewayOutputFormat + oldClient := httpClient + t.Cleanup(func() { + gatewayServerOverride = oldServer + gatewayTokenOverride = oldToken + gatewayOutputFormat = oldOutput + httpClient = oldClient + }) + gatewayServerOverride = srv.URL + gatewayTokenOverride = "test-token" + httpClient = srv.Client() +} + +func captureStdout(t *testing.T, fn func() error) (string, error) { + t.Helper() + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + runErr := fn() + _ = w.Close() + os.Stdout = oldStdout + out, readErr := io.ReadAll(r) + if readErr != nil { + t.Fatalf("read stdout: %v", readErr) + } + return string(out), runErr +} diff --git a/docs/10-tracing-observability.md b/docs/10-tracing-observability.md index f69f998f..6ed99ed6 100644 --- a/docs/10-tracing-observability.md +++ b/docs/10-tracing-observability.md @@ -187,7 +187,10 @@ worker.Stop() | Method | Path | Description | |--------|------|-------------| | GET | `/v1/traces` | List traces with pagination and filters | +| GET | `/v1/traces/follow` | Poll trace changes for one session or agent | | GET | `/v1/traces/{id}` | Get trace details with all spans | +| GET | `/v1/traces/{id}/export` | Export a gzipped trace tree with spans and sub-traces | +| GET | `/v1/runs/{runID}/timeline` | Get persisted run archive timeline items | ### Query Filters @@ -210,6 +213,35 @@ worker.Stop() | `limit` | int | Page size (default 50) | | `offset` | int | Pagination offset | +### Operator CLI + +The main `goclaw` binary can also act as a thin operator client for trace +inspection: + +```bash +goclaw traces list --status error --limit 20 +goclaw traces get -o json +goclaw traces export --file trace.json.gz +goclaw traces follow --session --since 2026-06-12T01:00:00Z +goclaw traces timeline +``` + +By default, commands use the same local gateway config and +`GOCLAW_GATEWAY_TOKEN` behavior as existing admin commands. For remote +operations, use explicit overrides: + +```bash +goclaw --server https://goclaw.example.com --token "$GOCLAW_GATEWAY_TOKEN" traces get -o json +``` + +`--server` also applies to existing WebSocket/RPC-backed admin commands such as +`sessions`, `cron`, and `pairing`. The URL can also come from `GOCLAW_SERVER` +or `GOCLAW_GATEWAY_URL`; `--server` wins when both are set. + +The standalone `nextlevelbuilder/goclaw-cli` can remain a compatibility tool, +but first-party trace operator workflows are now available from the main +server/runtime binary. + --- ## 8. Delegation History diff --git a/docs/18-http-api.md b/docs/18-http-api.md index f8c24449..745b22d9 100644 --- a/docs/18-http-api.md +++ b/docs/18-http-api.md @@ -1393,6 +1393,26 @@ Follow response: } ``` +Main binary operator commands wrap the same endpoints: + +```bash +goclaw traces list --query "provider fail" --status error +goclaw traces get -o json +goclaw traces export --file trace.json.gz +goclaw traces follow --session +goclaw traces timeline +``` + +Remote gateways use the shared client overrides: + +```bash +goclaw --server https://goclaw.example.com --token "$GOCLAW_GATEWAY_TOKEN" traces list -o json +``` + +The same `--server` / `--token` resolver is shared with WebSocket/RPC-backed +admin commands. `GOCLAW_SERVER` or `GOCLAW_GATEWAY_URL` can provide the base URL +when the flag is omitted. + ### Run Timeline `GET /v1/runs/{runID}/timeline` returns display-safe archive entries for one diff --git a/docs/21-agent-evolution-and-skill-management.md b/docs/21-agent-evolution-and-skill-management.md index b535df5c..d1daf187 100644 --- a/docs/21-agent-evolution-and-skill-management.md +++ b/docs/21-agent-evolution-and-skill-management.md @@ -345,10 +345,18 @@ All endpoints require authentication (`authMiddleware`). Mutation endpoints requ | `PUT` | `/v1/skills/{id}` | Update metadata (owner/admin) | | `DELETE` | `/v1/skills/{id}` | Delete/archive skill (owner/admin) | | `POST` | `/v1/skills/{id}/toggle` | Enable/disable skill (owner/admin) | +| `GET` | `/v1/skills/{id}/dependencies` | Structured dependency status by source | +| `POST` | `/v1/skills/{id}/dependencies/scan` | Re-scan skill dependencies | +| `POST` | `/v1/skills/{id}/dependencies/check` | Check missing skill dependencies | +| `POST` | `/v1/skills/{id}/dependencies/install` | Install missing deps for one skill (master tenant) | +| `GET` | `/v1/skills/{id}/access` | Read visibility and grants | +| `PATCH` | `/v1/skills/{id}/access` | Set visibility/access mode | +| `GET` | `/v1/skills/{id}/access/effective` | Explain access for one skill/agent/user | +| `GET` | `/v1/skills/access/effective` | Explain effective access across skills | | `POST` | `/v1/skills/{id}/grants/agent` | Grant skill to agent (owner/admin) | -| `DELETE` | `/v1/skills/{id}/grants/agent` | Revoke agent grant (owner/admin) | +| `DELETE` | `/v1/skills/{id}/grants/agent/{agentID}` | Revoke agent grant (owner/admin) | | `POST` | `/v1/skills/{id}/grants/user` | Grant skill to user (owner/admin) | -| `DELETE` | `/v1/skills/{id}/grants/user` | Revoke user grant (owner/admin) | +| `DELETE` | `/v1/skills/{id}/grants/user/{userID}` | Revoke user grant (owner/admin) | | `POST` | `/v1/skills/upload` | Upload custom skill ZIP | | `POST` | `/v1/skills/rescan-deps` | Re-scan all enabled skills | | `POST` | `/v1/skills/install-deps` | Install all missing deps | diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 3b820c20..fce3d99a 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,42 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-06-12 +### Operator trace CLI (issue #158) + +**Changes** + +- Added first-class `goclaw traces` operator commands to the main binary: + `list`, `get`, `export`, `follow`, and `timeline`. +- Added remote client overrides with `--server` and `--token`; trace commands + support trace-scoped output selection via `--output` / `-o`. +- Shared the URL/token resolver with WebSocket/RPC-backed admin commands so + `sessions`, `cron`, and `pairing` can use the same remote gateway overrides. +- Kept trace commands as thin wrappers over existing HTTP endpoints so the + server/runtime and operator CLI share one binary without new API contracts. + +**Tests** + +- Added command/client regressions for gateway URL and token overrides, trace + query serialization, follow scope validation, and timeline run ID handling. + +### Skill lifecycle API and CLI (issue #159) + +**Changes** + +- Added per-skill dependency scan/check/install endpoints and CLI commands. +- Added access read/update, agent/user grant aliases, and effective-access + inspection for operator workflows. +- Dependency status now reports system, pip, npm, and GitHub release deps in + structured output. +- Skill user grants are now tenant-scoped so shared system skills can be + granted to the same user ID in different tenants without conflicts. + +**Tests** + +- Added HTTP, CLI, PostgreSQL, and SQLite coverage for dependency lifecycle, + access mode changes, grants, effective access, grant-driven visibility, and + tenant-scoped user grants. + ### Mid-flight request preservation (issue #137) **Fixes** diff --git a/internal/http/skills.go b/internal/http/skills.go index 7d04a4e5..f9d93f3c 100644 --- a/internal/http/skills.go +++ b/internal/http/skills.go @@ -92,12 +92,26 @@ func (h *SkillsHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("POST /v1/skills/upload", h.adminMiddleware(h.handleUpload)) mux.HandleFunc("PUT /v1/skills/{id}", h.adminMiddleware(h.handleUpdate)) mux.HandleFunc("DELETE /v1/skills/{id}", h.adminMiddleware(h.handleDelete)) + mux.HandleFunc("GET /v1/skills/{id}/dependencies", h.adminMiddleware(h.handleSkillDependenciesStatus)) + mux.HandleFunc("POST /v1/skills/{id}/dependencies/scan", h.adminMiddleware(h.handleSkillDependenciesStatus)) + mux.HandleFunc("POST /v1/skills/{id}/dependencies/check", h.adminMiddleware(h.handleSkillDependenciesStatus)) + mux.HandleFunc("POST /v1/skills/{id}/dependencies/install", h.adminMiddleware(h.handleSkillDependenciesInstall)) + mux.HandleFunc("GET /v1/skills/{id}/access", h.adminMiddleware(h.handleGetSkillAccess)) + mux.HandleFunc("PATCH /v1/skills/{id}/access", h.adminMiddleware(h.handlePatchSkillAccess)) + mux.HandleFunc("GET /v1/skills/{id}/access/effective", h.adminMiddleware(h.handleGetSkillEffectiveAccess)) + mux.HandleFunc("GET /v1/skills/access/effective", h.adminMiddleware(h.handleListEffectiveAccess)) // Skill grants (admin+) mux.HandleFunc("GET /v1/skills/{id}/grants/agent", h.adminMiddleware(h.handleListAgentGrants)) mux.HandleFunc("POST /v1/skills/{id}/grants/agent", h.adminMiddleware(h.handleGrantAgent)) mux.HandleFunc("DELETE /v1/skills/{id}/grants/agent/{agentID}", h.adminMiddleware(h.handleRevokeAgent)) + mux.HandleFunc("GET /v1/skills/{id}/grants/agents", h.adminMiddleware(h.handleListAgentGrants)) + mux.HandleFunc("POST /v1/skills/{id}/grants/agents", h.adminMiddleware(h.handleGrantAgent)) + mux.HandleFunc("DELETE /v1/skills/{id}/grants/agents/{agentID}", h.adminMiddleware(h.handleRevokeAgent)) + mux.HandleFunc("GET /v1/skills/{id}/grants/users", h.adminMiddleware(h.handleListUserGrants)) mux.HandleFunc("POST /v1/skills/{id}/grants/user", h.adminMiddleware(h.handleGrantUser)) mux.HandleFunc("DELETE /v1/skills/{id}/grants/user/{userID}", h.adminMiddleware(h.handleRevokeUser)) + mux.HandleFunc("POST /v1/skills/{id}/grants/users", h.adminMiddleware(h.handleGrantUser)) + mux.HandleFunc("DELETE /v1/skills/{id}/grants/users/{userID}", h.adminMiddleware(h.handleRevokeUser)) // System-level operations: admin + master tenant only. // These execute shell commands (pip/npm install) and affect the entire server. mux.HandleFunc("POST /v1/skills/rescan-deps", h.adminMiddleware(h.handleRescanDeps)) @@ -128,19 +142,7 @@ func (h *SkillsHandler) adminMiddleware(next http.HandlerFunc) http.HandlerFunc // System skill management (install packages, rescan deps) is a server-wide operation // that should only be accessible to the master tenant or cross-tenant admins. func (h *SkillsHandler) requireMasterTenant(w http.ResponseWriter, r *http.Request) bool { - ctx := r.Context() - if store.IsOwnerRole(ctx) { - return true - } - tid := store.TenantIDFromContext(ctx) - if tid == store.MasterTenantID { - return true - } - locale := store.LocaleFromContext(ctx) - writeJSON(w, http.StatusForbidden, map[string]string{ - "error": i18n.T(locale, i18n.MsgPermissionDenied, "system skill management"), - }) - return false + return requireMasterScope(w, r) } func (h *SkillsHandler) handleList(w http.ResponseWriter, r *http.Request) { @@ -503,7 +505,7 @@ func stringSlicesEqual(a, b []string) bool { // bundled dir if the managed copy's scripts/ directory is missing or empty. // If a fallback scan succeeds, re-copies the bundled scripts to the managed dir. func (h *SkillsHandler) scanWithFallback(sk store.SkillInfo) *skills.SkillManifest { - manifest := skills.ScanSkillDeps(sk.BaseDir) + manifest := scanSkillDeps(sk.BaseDir) if manifest != nil && !manifest.IsEmpty() { return manifest } @@ -520,7 +522,7 @@ func (h *SkillsHandler) scanWithFallback(sk store.SkillInfo) *skills.SkillManife } bundledSkillDir := filepath.Join(h.bundledDir, sk.Slug) - bundledManifest := skills.ScanSkillDeps(bundledSkillDir) + bundledManifest := scanSkillDeps(bundledSkillDir) if bundledManifest == nil || bundledManifest.IsEmpty() { return manifest } diff --git a/internal/http/skills_access.go b/internal/http/skills_access.go new file mode 100644 index 00000000..b7ff91c0 --- /dev/null +++ b/internal/http/skills_access.go @@ -0,0 +1,152 @@ +package http + +import ( + "net/http" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/i18n" + "github.com/nextlevelbuilder/goclaw/internal/skills" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +type skillAccessResponse struct { + Skill skillRef `json:"skill"` + Visibility string `json:"visibility"` + AgentGrants []store.SkillAgentGrantInfo `json:"agent_grants"` + UserGrants []store.SkillUserGrantInfo `json:"user_grants"` +} + +type skillEffectiveAccessResponse struct { + Skill skillRef `json:"skill"` + Accessible bool `json:"accessible"` + Reason string `json:"reason"` + CanManage bool `json:"can_manage"` + PinnedVersion *int `json:"pinned_version,omitempty"` +} + +func (h *SkillsHandler) handleGetSkillAccess(w http.ResponseWriter, r *http.Request) { + sk, ok := h.lifecycleSkill(w, r, false) + if !ok { + return + } + agentGrants, err := h.skills.ListAgentGrantsForSkill(r.Context(), mustParseUUID(sk.ID)) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + userGrants, err := h.skills.ListUserGrantsForSkill(r.Context(), mustParseUUID(sk.ID)) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + writeJSON(w, http.StatusOK, skillAccessResponse{ + Skill: refForSkill(sk), + Visibility: sk.Visibility, + AgentGrants: agentGrants, + UserGrants: userGrants, + }) +} + +func (h *SkillsHandler) handlePatchSkillAccess(w http.ResponseWriter, r *http.Request) { + locale := store.LocaleFromContext(r.Context()) + sk, ok := h.lifecycleSkill(w, r, false) + if !ok { + return + } + if sk.IsSystem && !h.requireMasterTenant(w, r) { + return + } + var req struct { + Mode string `json:"mode"` + Visibility string `json:"visibility"` + } + if !bindJSON(w, r, locale, &req) { + return + } + visibility := req.Visibility + if visibility == "" { + visibility = req.Mode + } + if err := skills.ValidateVisibility(visibility); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidVisibility, visibility)}) + return + } + visibility = skills.NormalizeVisibility(visibility) + id := mustParseUUID(sk.ID) + updateCtx := r.Context() + if sk.IsSystem { + updateCtx = store.WithCrossTenant(store.WithTenantID(updateCtx, store.MasterTenantID)) + } + if err := h.skills.UpdateSkill(updateCtx, id, map[string]any{"visibility": visibility}); err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + h.skills.BumpVersion() + h.emitCacheInvalidate(bus.CacheKindSkills, sk.ID, uuid.Nil) + emitAudit(h.msgBus, r, "skill.access_updated", "skill", sk.ID) + writeJSON(w, http.StatusOK, map[string]any{"ok": true, "visibility": visibility}) +} + +func (h *SkillsHandler) handleGetSkillEffectiveAccess(w http.ResponseWriter, r *http.Request) { + sk, ok := h.lifecycleSkill(w, r, false) + if !ok { + return + } + agentID, userID, ok := parseEffectiveAccessParams(w, r) + if !ok { + return + } + if !h.requireTenantUser(w, r, userID) { + return + } + idx, err := h.buildEffectiveAccessIndex(r, agentID, userID) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + resp := effectiveAccessForSkill(r.Context(), sk, idx, userID) + writeJSON(w, http.StatusOK, resp) +} + +func (h *SkillsHandler) handleListEffectiveAccess(w http.ResponseWriter, r *http.Request) { + agentID, userID, ok := parseEffectiveAccessParams(w, r) + if !ok { + return + } + if !h.requireTenantUser(w, r, userID) { + return + } + idx, err := h.buildEffectiveAccessIndex(r, agentID, userID) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + skills := h.skills.ListAllSkills(r.Context()) + out := make([]skillEffectiveAccessResponse, 0, len(skills)) + for _, sk := range skills { + out = append(out, effectiveAccessForSkill(r.Context(), sk, idx, userID)) + } + writeJSON(w, http.StatusOK, map[string]any{"skills": out}) +} + +func parseEffectiveAccessParams(w http.ResponseWriter, r *http.Request) (uuid.UUID, string, bool) { + locale := store.LocaleFromContext(r.Context()) + agentID, err := uuid.Parse(r.URL.Query().Get("agent_id")) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "agent")}) + return uuid.Nil, "", false + } + userID := r.URL.Query().Get("user_id") + if err := store.ValidateUserID(userID); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) + return uuid.Nil, "", false + } + return agentID, userID, true +} + +func mustParseUUID(raw string) uuid.UUID { + id, _ := uuid.Parse(raw) + return id +} diff --git a/internal/http/skills_access_effective.go b/internal/http/skills_access_effective.go new file mode 100644 index 00000000..20a7f2b5 --- /dev/null +++ b/internal/http/skills_access_effective.go @@ -0,0 +1,111 @@ +package http + +import ( + "context" + "fmt" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/skills" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +type effectiveAccessIndex struct { + actorID string + accessibleBySlug map[string]bool + agentGrantByID map[string]store.SkillWithGrantStatus + agentGrantBySlug map[string]store.SkillWithGrantStatus +} + +func (h *SkillsHandler) buildEffectiveAccessIndex(r interface{ Context() context.Context }, agentID uuid.UUID, userID string) (effectiveAccessIndex, error) { + ctx := r.Context() + accessStore, ok := h.skills.(store.SkillAccessStore) + if !ok { + return effectiveAccessIndex{}, fmt.Errorf("skill access store not available") + } + accessible, err := accessStore.ListAccessible(ctx, agentID, userID) + if err != nil { + return effectiveAccessIndex{}, err + } + grantStatus, err := h.skills.ListWithGrantStatus(ctx, agentID) + if err != nil { + return effectiveAccessIndex{}, err + } + idx := effectiveAccessIndex{ + actorID: store.ActorIDFromContext(ctx), + accessibleBySlug: make(map[string]bool, len(accessible)), + agentGrantByID: make(map[string]store.SkillWithGrantStatus, len(grantStatus)), + agentGrantBySlug: make(map[string]store.SkillWithGrantStatus, len(grantStatus)), + } + if idx.actorID == "" { + idx.actorID = userID + } + for _, sk := range accessible { + idx.accessibleBySlug[sk.Slug] = true + } + for _, grant := range grantStatus { + idx.agentGrantByID[grant.ID.String()] = grant + idx.agentGrantBySlug[grant.Slug] = grant + } + return idx, nil +} + +func effectiveAccessForSkill(ctx context.Context, sk store.SkillInfo, idx effectiveAccessIndex, userID string) skillEffectiveAccessResponse { + resp := skillEffectiveAccessResponse{Skill: refForSkill(sk), Reason: "none"} + if sk.Status != "active" || !sk.Enabled { + resp.Reason = "inactive" + return resp + } + if !idx.accessible(sk) { + return resp + } + if sk.IsSystem { + resp.Accessible = true + resp.Reason = "system" + return resp + } + if sk.Visibility == skills.VisibilityPublic { + resp.Accessible = true + resp.Reason = "public" + return resp + } + actorID := idx.actorID + if actorID == "" { + actorID = store.ActorIDFromContext(ctx) + } + if actorID == "" { + actorID = userID + } + if sk.Visibility == skills.VisibilityPrivate && (sk.OwnerID == userID || sk.OwnerID == actorID) { + resp.Accessible = true + resp.Reason = "owner" + return resp + } + if sk.Visibility != skills.VisibilityInternal { + return resp + } + if grant, ok := idx.agentGrant(sk); ok && grant.Granted { + resp.Accessible = true + resp.Reason = "agent_grant" + resp.CanManage = grant.CanManage + resp.PinnedVersion = grant.PinnedVer + return resp + } + resp.Accessible = true + resp.Reason = "user_grant" + return resp +} + +func (idx effectiveAccessIndex) accessible(sk store.SkillInfo) bool { + return idx.accessibleBySlug[sk.Slug] +} + +func (idx effectiveAccessIndex) agentGrant(sk store.SkillInfo) (store.SkillWithGrantStatus, bool) { + if sk.ID != "" { + if grant, ok := idx.agentGrantByID[sk.ID]; ok { + return grant, true + } + } + grant, ok := idx.agentGrantBySlug[sk.Slug] + return grant, ok +} diff --git a/internal/http/skills_grants.go b/internal/http/skills_grants.go index 60ce877b..3b307325 100644 --- a/internal/http/skills_grants.go +++ b/internal/http/skills_grants.go @@ -50,6 +50,25 @@ func (h *SkillsHandler) handleListAgentGrants(w http.ResponseWriter, r *http.Req writeJSON(w, http.StatusOK, map[string]any{"grants": grants}) } +func (h *SkillsHandler) handleListUserGrants(w http.ResponseWriter, r *http.Request) { + locale := store.LocaleFromContext(r.Context()) + idStr := r.PathValue("id") + skillID, err := uuid.Parse(idStr) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "skill")}) + return + } + + grants, err := h.skills.ListUserGrantsForSkill(r.Context(), skillID) + if err != nil { + slog.Error("failed to list skill user grants", "skill_id", skillID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToList, "skill grants")}) + return + } + + writeJSON(w, http.StatusOK, map[string]any{"grants": grants}) +} + func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) { locale := store.LocaleFromContext(r.Context()) userID := store.UserIDFromContext(r.Context()) @@ -70,9 +89,10 @@ func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) } var req struct { - AgentID string `json:"agent_id"` - Version int `json:"version"` - CanManage *bool `json:"can_manage"` + AgentID string `json:"agent_id"` + Version int `json:"version"` + PinnedVersion int `json:"pinned_version"` + CanManage *bool `json:"can_manage"` } if !bindJSON(w, r, locale, &req) { return @@ -84,6 +104,9 @@ func (h *SkillsHandler) handleGrantAgent(w http.ResponseWriter, r *http.Request) return } + if req.PinnedVersion > 0 { + req.Version = req.PinnedVersion + } if req.Version <= 0 { req.Version = 1 } @@ -175,6 +198,9 @@ func (h *SkillsHandler) handleGrantUser(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()}) return } + if !h.requireTenantUser(w, r, req.UserID) { + return + } if err := h.skills.GrantToUser(r.Context(), skillID, req.UserID, userID); err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) @@ -222,4 +248,31 @@ func (h *SkillsHandler) handleRevokeUser(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusOK, map[string]string{"ok": "true"}) } +func (h *SkillsHandler) requireTenantUser(w http.ResponseWriter, r *http.Request, userID string) bool { + locale := store.LocaleFromContext(r.Context()) + if h.tenantStore == nil { + writeJSON(w, http.StatusNotImplemented, map[string]string{"error": i18n.T(locale, i18n.MsgNotImplemented, "tenant store")}) + return false + } + tenantID := store.TenantIDFromContext(r.Context()) + if tenantID == uuid.Nil { + tenantID = store.MasterTenantID + } + role, err := h.tenantStore.GetUserRole(r.Context(), tenantID, userID) + if err != nil { + slog.Error("skill_grants: tenant user membership check failed", "tenant_id", tenantID, "target_user_id", userID, "error", err) + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": i18n.T(locale, i18n.MsgFailedToList, "tenant users")}) + return false + } + if role == "" { + slog.Warn("security.skill_grant_user_not_tenant_member", + "tenant_id", tenantID, + "target_user_id", userID, + "user_id", store.UserIDFromContext(r.Context())) + writeJSON(w, http.StatusForbidden, map[string]string{"error": i18n.T(locale, i18n.MsgPermissionDenied, "tenant user")}) + return false + } + return true +} + // --- Helpers --- diff --git a/internal/http/skills_lifecycle.go b/internal/http/skills_lifecycle.go new file mode 100644 index 00000000..34dd63b4 --- /dev/null +++ b/internal/http/skills_lifecycle.go @@ -0,0 +1,144 @@ +package http + +import ( + "context" + "net/http" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/i18n" + "github.com/nextlevelbuilder/goclaw/internal/skills" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +var ( + scanSkillDeps = skills.ScanSkillDeps + checkSkillDeps = skills.CheckSkillDeps + githubSkillDependencyInstalled = skillGitHubDependencyInstalled +) + +type skillRef struct { + ID string `json:"id"` + Slug string `json:"slug"` + Name string `json:"name"` + Status string `json:"status"` + Version int `json:"version"` +} + +type skillDependencyItem struct { + Source string `json:"source"` + Name string `json:"name"` + Status string `json:"status"` +} + +type skillDependencyStatusResponse struct { + Skill skillRef `json:"skill"` + Dependencies []skillDependencyItem `json:"dependencies"` + OK bool `json:"ok"` + Status string `json:"status"` + Missing []string `json:"missing,omitempty"` + MissingCount int `json:"missing_count"` +} + +func (h *SkillsHandler) handleSkillDependenciesStatus(w http.ResponseWriter, r *http.Request) { + if !h.requireMasterTenant(w, r) { + return + } + sk, ok := h.lifecycleSkill(w, r, false) + if !ok { + return + } + writeJSON(w, http.StatusOK, h.buildSkillDependencyStatus(sk)) +} + +func (h *SkillsHandler) handleSkillDependenciesInstall(w http.ResponseWriter, r *http.Request) { + if !h.requireMasterTenant(w, r) { + return + } + sk, ok := h.lifecycleSkill(w, r, true) + if !ok { + return + } + manifest := h.scanWithFallback(sk) + if manifest == nil || manifest.IsEmpty() { + h.persistSkillDependencyState(r.Context(), sk, true, nil) + writeJSON(w, http.StatusOK, h.buildSkillDependencyStatus(sk)) + return + } + _, missing := lifecycleCheckSkillDeps(manifest) + if len(missing) == 0 { + h.persistSkillDependencyState(r.Context(), sk, true, nil) + writeJSON(w, http.StatusOK, h.buildSkillDependencyStatus(sk)) + return + } + result, err := installLifecycleDeps(r.Context(), manifest, missing) + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + return + } + okAfter, missingAfter := lifecycleCheckSkillDeps(manifest) + h.persistSkillDependencyState(r.Context(), sk, okAfter, missingAfter) + resp := h.buildSkillDependencyStatus(sk) + resp.OK = okAfter + resp.Missing = missingAfter + resp.MissingCount = len(missingAfter) + resp.Status = dependencyStatus(okAfter) + writeJSON(w, http.StatusOK, map[string]any{"result": result, "dependencies": resp}) +} + +func (h *SkillsHandler) lifecycleSkill(w http.ResponseWriter, r *http.Request, crossTenant bool) (store.SkillInfo, bool) { + locale := store.LocaleFromContext(r.Context()) + id, err := uuid.Parse(r.PathValue("id")) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidID, "skill")}) + return store.SkillInfo{}, false + } + ctx := r.Context() + if crossTenant { + ctx = store.WithCrossTenant(store.WithTenantID(ctx, store.MasterTenantID)) + } + sk, ok := h.skills.GetSkillByID(ctx, id) + if !ok { + writeJSON(w, http.StatusNotFound, map[string]string{"error": i18n.T(locale, i18n.MsgNotFound, "skill", id.String())}) + return store.SkillInfo{}, false + } + return sk, true +} + +func (h *SkillsHandler) buildSkillDependencyStatus(sk store.SkillInfo) skillDependencyStatusResponse { + manifest := h.scanWithFallback(sk) + if manifest == nil || manifest.IsEmpty() { + return skillDependencyStatusResponse{Skill: refForSkill(sk), OK: true, Status: "ok"} + } + ok, missing := lifecycleCheckSkillDeps(manifest) + return skillDependencyStatusResponse{ + Skill: refForSkill(sk), + Dependencies: dependencyItems(manifest, missing), + OK: ok, + Status: dependencyStatus(ok), + Missing: missing, + MissingCount: len(missing), + } +} + +func (h *SkillsHandler) persistSkillDependencyState(ctx context.Context, sk store.SkillInfo, ok bool, missing []string) { + id, err := uuid.Parse(sk.ID) + if err != nil { + return + } + updateCtx := skillTenantContext(ctx, sk) + _ = h.skills.StoreMissingDeps(updateCtx, id, missing) + wantStatus := "archived" + if ok { + wantStatus = "active" + } + if sk.Status != wantStatus { + _ = h.skills.UpdateSkill(updateCtx, id, map[string]any{"status": wantStatus}) + h.emitCacheInvalidate(bus.CacheKindSkills, sk.ID, uuid.Nil) + } +} + +func refForSkill(sk store.SkillInfo) skillRef { + return skillRef{ID: sk.ID, Slug: sk.Slug, Name: sk.Name, Status: sk.Status, Version: sk.Version} +} diff --git a/internal/http/skills_lifecycle_deps.go b/internal/http/skills_lifecycle_deps.go new file mode 100644 index 00000000..f32a447e --- /dev/null +++ b/internal/http/skills_lifecycle_deps.go @@ -0,0 +1,164 @@ +package http + +import ( + "context" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/skills" +) + +func lifecycleCheckSkillDeps(m *skills.SkillManifest) (bool, []string) { + ok, missing := checkSkillDeps(m) + missing = appendUniqueDeps(missing, missingGitHubSkillDeps(m)...) + return ok && len(missing) == 0, missing +} + +func missingGitHubSkillDeps(m *skills.SkillManifest) []string { + if m == nil { + return nil + } + var missing []string + for _, raw := range m.Explicit { + if strings.HasPrefix(raw, "github:") && !githubSkillDependencyInstalled(raw) { + missing = append(missing, raw) + } + } + return missing +} + +func skillGitHubDependencyInstalled(raw string) bool { + spec, err := skills.ParseGitHubSpec(raw) + if err != nil { + return false + } + installer := skills.DefaultGitHubInstaller() + if installer == nil { + return false + } + entries, err := installer.List() + if err != nil { + return false + } + wantRepo := spec.Owner + "/" + spec.Repo + for _, entry := range entries { + if strings.EqualFold(entry.Repo, wantRepo) && (spec.Tag == "" || entry.Tag == spec.Tag) { + return true + } + } + return false +} + +func installLifecycleDeps(ctx context.Context, manifest *skills.SkillManifest, missing []string) (*skills.InstallResult, error) { + bulkMissing, githubMissing := splitGitHubMissingDeps(missing) + result := &skills.InstallResult{} + if len(bulkMissing) > 0 { + bulkResult, err := installManagedDeps(ctx, manifest, bulkMissing) + if err != nil { + return nil, err + } + mergeInstallResult(result, bulkResult) + } + for _, dep := range githubMissing { + ok, errMsg := installSingleDep(ctx, dep) + name := strings.TrimPrefix(dep, "github:") + if !ok { + result.Errors = append(result.Errors, "github "+name+": "+errMsg) + continue + } + result.GitHub = append(result.GitHub, name) + } + return result, nil +} + +func splitGitHubMissingDeps(missing []string) ([]string, []string) { + var bulkMissing []string + var githubMissing []string + for _, dep := range missing { + if strings.HasPrefix(dep, "github:") { + githubMissing = append(githubMissing, dep) + continue + } + bulkMissing = append(bulkMissing, dep) + } + return bulkMissing, githubMissing +} + +func mergeInstallResult(dst, src *skills.InstallResult) { + if dst == nil || src == nil { + return + } + dst.System = append(dst.System, src.System...) + dst.Pip = append(dst.Pip, src.Pip...) + dst.Npm = append(dst.Npm, src.Npm...) + dst.GitHub = append(dst.GitHub, src.GitHub...) + dst.Errors = append(dst.Errors, src.Errors...) +} + +func dependencyItems(m *skills.SkillManifest, missing []string) []skillDependencyItem { + missingSet := dependencyMissingSet(missing) + var out []skillDependencyItem + add := func(source, name string) { + status := "installed" + if missingSet[dependencyKey(source, name)] { + status = "missing" + } + out = append(out, skillDependencyItem{Source: source, Name: name, Status: status}) + } + for _, name := range m.Requires { + add("system", name) + } + for _, name := range m.RequiresPython { + add("pip", name) + } + for _, name := range m.RequiresNode { + add("npm", name) + } + for _, raw := range m.Explicit { + if after, ok := strings.CutPrefix(raw, "github:"); ok { + add("github", after) + } + } + return out +} + +func dependencyMissingSet(missing []string) map[string]bool { + out := make(map[string]bool, len(missing)) + for _, dep := range missing { + source, name := splitDependency(dep) + out[dependencyKey(source, name)] = true + } + return out +} + +func splitDependency(dep string) (string, string) { + for _, prefix := range []string{"pip:", "npm:", "github:", "system:"} { + if after, ok := strings.CutPrefix(dep, prefix); ok { + return strings.TrimSuffix(prefix, ":"), after + } + } + return "system", dep +} + +func dependencyKey(source, name string) string { return source + ":" + name } + +func appendUniqueDeps(dst []string, src ...string) []string { + seen := make(map[string]bool, len(dst)+len(src)) + for _, dep := range dst { + seen[dep] = true + } + for _, dep := range src { + if seen[dep] { + continue + } + dst = append(dst, dep) + seen[dep] = true + } + return dst +} + +func dependencyStatus(ok bool) string { + if ok { + return "ok" + } + return "missing" +} diff --git a/internal/http/skills_lifecycle_test.go b/internal/http/skills_lifecycle_test.go new file mode 100644 index 00000000..de260a33 --- /dev/null +++ b/internal/http/skills_lifecycle_test.go @@ -0,0 +1,437 @@ +package http + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/crypto" + "github.com/nextlevelbuilder/goclaw/internal/skills" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +func TestHandleSkillDependenciesStatus_ReturnsStructuredMissingBySource(t *testing.T) { + handler, skillStore, ctx, root := newTestUploadHandler(t) + skillDir := filepath.Join(root, "skills-store", "dep-skill", "1") + skillID := skillStore.seedCustomSkill("dep-skill", skillDir, "archived", []string{"ffmpeg", "pip:requests"}) + + prevScan := scanSkillDeps + prevCheck := checkSkillDeps + prevGitHubInstalled := githubSkillDependencyInstalled + scanSkillDeps = func(string) *skills.SkillManifest { + return &skills.SkillManifest{ + Requires: []string{"ffmpeg"}, + RequiresPython: []string{"requests"}, + RequiresNode: []string{"tsx"}, + Explicit: []string{"github:cli/cli@v2.40.0"}, + } + } + checkSkillDeps = func(*skills.SkillManifest) (bool, []string) { + return false, []string{"ffmpeg", "pip:requests"} + } + githubSkillDependencyInstalled = func(string) bool { return false } + t.Cleanup(func() { + scanSkillDeps = prevScan + checkSkillDeps = prevCheck + githubSkillDependencyInstalled = prevGitHubInstalled + }) + + req := httptest.NewRequest(http.MethodGet, "/v1/skills/"+skillID.String()+"/dependencies", http.NoBody).WithContext(ctx) + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handleSkillDependenciesStatus(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + var resp struct { + OK bool `json:"ok"` + MissingCount int `json:"missing_count"` + Dependencies []struct { + Source string `json:"source"` + Name string `json:"name"` + Status string `json:"status"` + } `json:"dependencies"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp.OK { + t.Fatal("ok = true, want false") + } + if resp.MissingCount != 3 { + t.Fatalf("missing_count = %d, want 3", resp.MissingCount) + } + got := map[string]string{} + for _, dep := range resp.Dependencies { + got[dep.Source+":"+dep.Name] = dep.Status + } + want := map[string]string{ + "system:ffmpeg": "missing", + "pip:requests": "missing", + "npm:tsx": "installed", + "github:cli/cli@v2.40.0": "missing", + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("dependencies = %#v, want %#v", got, want) + } +} + +func TestSkillDependencyRoutesRequireAdmin(t *testing.T) { + handler, skillStore, _, root := newTestUploadHandler(t) + skillID := skillStore.seedCustomSkill("dep-skill", filepath.Join(root, "skills-store", "dep-skill", "1"), "active", nil) + tenantID := uuid.New() + setupTestCache(t, map[string]*store.APIKeyData{ + crypto.HashAPIKey("read-token"): {ID: uuid.New(), Scopes: []string{"operator.read"}}, + crypto.HashAPIKey("tenant-admin-token"): { + ID: uuid.New(), + Scopes: []string{"operator.admin"}, + TenantID: tenantID, + OwnerID: "tenant-admin", + }, + }) + + prevScan := scanSkillDeps + scanCalled := false + scanSkillDeps = func(string) *skills.SkillManifest { + scanCalled = true + return &skills.SkillManifest{} + } + t.Cleanup(func() { scanSkillDeps = prevScan }) + + mux := http.NewServeMux() + handler.RegisterRoutes(mux) + req := httptest.NewRequest(http.MethodGet, "/v1/skills/"+skillID.String()+"/dependencies", http.NoBody) + req.Header.Set("Authorization", "Bearer read-token") + w := httptest.NewRecorder() + + mux.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body = %s", w.Code, w.Body.String()) + } + if scanCalled { + t.Fatal("dependency scan ran for non-admin caller") + } + + scanCalled = false + req = httptest.NewRequest(http.MethodGet, "/v1/skills/"+skillID.String()+"/dependencies", http.NoBody) + req.Header.Set("Authorization", "Bearer tenant-admin-token") + w = httptest.NewRecorder() + + mux.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("tenant admin status = %d, want 403; body = %s", w.Code, w.Body.String()) + } + if scanCalled { + t.Fatal("dependency scan ran for non-master tenant admin") + } +} + +func TestHandleSkillDependenciesInstall_SplitsGitHubDepsToSingleInstaller(t *testing.T) { + handler, skillStore, ctx, root := newTestUploadHandler(t) + skillDir := filepath.Join(root, "skills-store", "dep-skill", "1") + skillID := skillStore.seedCustomSkill("dep-skill", skillDir, "archived", []string{"pip:requests", "github:cli/cli@v2.40.0"}) + + prevScan := scanSkillDeps + prevCheck := checkSkillDeps + prevInstallManaged := installManagedDeps + prevInstallSingle := installSingleDep + prevGitHubInstalled := githubSkillDependencyInstalled + githubInstalled := false + scanSkillDeps = func(string) *skills.SkillManifest { + return &skills.SkillManifest{ + RequiresPython: []string{"requests"}, + Explicit: []string{"github:cli/cli@v2.40.0"}, + } + } + checkSkillDeps = func(*skills.SkillManifest) (bool, []string) { + if githubInstalled { + return true, nil + } + return false, []string{"pip:requests"} + } + githubSkillDependencyInstalled = func(raw string) bool { + if raw != "github:cli/cli@v2.40.0" { + t.Fatalf("github status checked for %q", raw) + } + return githubInstalled + } + installManagedDeps = func(_ context.Context, _ *skills.SkillManifest, missing []string) (*skills.InstallResult, error) { + if !reflect.DeepEqual(missing, []string{"pip:requests"}) { + t.Fatalf("managed missing = %#v, want pip only", missing) + } + return &skills.InstallResult{Pip: []string{"requests"}}, nil + } + installSingleDep = func(_ context.Context, dep string) (bool, string) { + if dep != "github:cli/cli@v2.40.0" { + t.Fatalf("single dep = %q, want github spec", dep) + } + githubInstalled = true + return true, "" + } + t.Cleanup(func() { + scanSkillDeps = prevScan + checkSkillDeps = prevCheck + installManagedDeps = prevInstallManaged + installSingleDep = prevInstallSingle + githubSkillDependencyInstalled = prevGitHubInstalled + }) + + req := httptest.NewRequest(http.MethodPost, "/v1/skills/"+skillID.String()+"/dependencies/install", http.NoBody).WithContext(ctx) + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handleSkillDependenciesInstall(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + var resp struct { + Result skills.InstallResult `json:"result"` + Deps struct { + OK bool `json:"ok"` + MissingCount int `json:"missing_count"` + } `json:"dependencies"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if !reflect.DeepEqual(resp.Result.Pip, []string{"requests"}) || !reflect.DeepEqual(resp.Result.GitHub, []string{"cli/cli@v2.40.0"}) { + t.Fatalf("install result = %+v", resp.Result) + } + if !resp.Deps.OK || resp.Deps.MissingCount != 0 { + t.Fatalf("dependencies = %+v", resp.Deps) + } +} + +func TestHandleSkillDependenciesInstall_RejectsNonMasterTenant(t *testing.T) { + handler, skillStore, _, root := newTestUploadHandler(t) + tenantID := uuid.New() + ctx := store.WithTenantID(context.Background(), tenantID) + skillDir := filepath.Join(root, "tenants", tenantID.String(), "skills-store", "dep-skill", "1") + skillID := skillStore.seedCustomSkillForTenant(tenantID, "dep-skill", skillDir, "archived", []string{"pip:requests"}) + + installCalled := false + prevInstall := installManagedDeps + installManagedDeps = func(context.Context, *skills.SkillManifest, []string) (*skills.InstallResult, error) { + installCalled = true + return &skills.InstallResult{}, nil + } + t.Cleanup(func() { installManagedDeps = prevInstall }) + + req := httptest.NewRequest(http.MethodPost, "/v1/skills/"+skillID.String()+"/dependencies/install", http.NoBody).WithContext(ctx) + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handleSkillDependenciesInstall(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body = %s", w.Code, w.Body.String()) + } + if installCalled { + t.Fatal("installManagedDeps was called for non-master tenant") + } +} + +func TestSkillGrantUserRouteRejectsNonTenantMember(t *testing.T) { + handler, skillStore, _, root := newTestUploadHandler(t) + tenantID := uuid.New() + skillID := skillStore.seedCustomSkillForTenant(tenantID, "access-skill", filepath.Join(root, "tenants", tenantID.String(), "skills-store", "access-skill", "1"), "active", nil) + tenants := newMockTenantStore() + tenants.addTenant(tenantID, "tenant-a") + tenants.setUserRole(tenantID, "admin-user", store.TenantRoleAdmin) + handler.tenantStore = tenants + setupTestCache(t, map[string]*store.APIKeyData{ + crypto.HashAPIKey("admin-token"): { + ID: uuid.New(), + Scopes: []string{"operator.admin"}, + TenantID: tenantID, + OwnerID: "admin-user", + }, + }) + + mux := http.NewServeMux() + handler.RegisterRoutes(mux) + req := httptest.NewRequest(http.MethodPost, "/v1/skills/"+skillID.String()+"/grants/users", strings.NewReader(`{"user_id":"outside-user"}`)) + req.Header.Set("Authorization", "Bearer admin-token") + w := httptest.NewRecorder() + + mux.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body = %s", w.Code, w.Body.String()) + } + if len(skillStore.userGrantCalls) != 0 { + t.Fatalf("user grant calls = %+v, want none", skillStore.userGrantCalls) + } +} + +func TestHandleSkillAccessGetListsAgentAndUserGrants(t *testing.T) { + handler, skillStore, ctx, root := newTestUploadHandler(t) + skillID := skillStore.seedCustomSkill("access-skill", filepath.Join(root, "skills-store", "access-skill", "1"), "active", nil) + agentID := uuid.New() + skillStore.agentGrants[skillID] = []store.SkillAgentGrantInfo{{ + AgentID: agentID, + PinnedVersion: 3, + GrantedBy: "owner-user", + CanManage: true, + }} + skillStore.userGrants[skillID] = []store.SkillUserGrantInfo{{ + UserID: "target-user", + GrantedBy: "owner-user", + }} + + req := httptest.NewRequest(http.MethodGet, "/v1/skills/"+skillID.String()+"/access", http.NoBody).WithContext(ctx) + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handleGetSkillAccess(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + var resp struct { + Visibility string `json:"visibility"` + AgentGrants []store.SkillAgentGrantInfo `json:"agent_grants"` + UserGrants []store.SkillUserGrantInfo `json:"user_grants"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if len(resp.AgentGrants) != 1 || !resp.AgentGrants[0].CanManage || resp.AgentGrants[0].PinnedVersion != 3 { + t.Fatalf("agent grants = %+v", resp.AgentGrants) + } + if len(resp.UserGrants) != 1 || resp.UserGrants[0].UserID != "target-user" { + t.Fatalf("user grants = %+v", resp.UserGrants) + } +} + +func TestHandleSkillEffectiveAccessReportsAgentGrant(t *testing.T) { + handler, skillStore, ctx, root := newTestUploadHandler(t) + skillID := skillStore.seedCustomSkill("effective-skill", filepath.Join(root, "skills-store", "effective-skill", "1"), "active", nil) + agentID := uuid.New() + tenants := newMockTenantStore() + tenants.addTenant(store.MasterTenantID, "master") + tenants.setUserRole(store.MasterTenantID, "target-user", store.TenantRoleMember) + handler.tenantStore = tenants + skillStore.skills[skillID] = store.SkillInfo{ + ID: skillID.String(), + TenantID: store.MasterTenantID.String(), + Name: "Effective Skill", + Slug: "effective-skill", + Path: filepath.Join(root, "skills-store", "effective-skill", "1", "SKILL.md"), + BaseDir: filepath.Join(root, "skills-store", "effective-skill", "1"), + Visibility: "internal", + Status: "active", + Enabled: true, + } + skillStore.agentGrants[skillID] = []store.SkillAgentGrantInfo{{ + AgentID: agentID, + PinnedVersion: 2, + GrantedBy: "owner-user", + CanManage: true, + }} + + req := httptest.NewRequest(http.MethodGet, "/v1/skills/"+skillID.String()+"/access/effective?agent_id="+agentID.String()+"&user_id=target-user", http.NoBody).WithContext(ctx) + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handleGetSkillEffectiveAccess(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + var resp struct { + Accessible bool `json:"accessible"` + Reason string `json:"reason"` + CanManage bool `json:"can_manage"` + PinnedVersion *int `json:"pinned_version"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if !resp.Accessible || resp.Reason != "agent_grant" || !resp.CanManage || resp.PinnedVersion == nil || *resp.PinnedVersion != 2 { + t.Fatalf("effective access = %+v", resp) + } +} + +func TestEffectiveAccessTreatsAccessibleIndexAsSourceOfTruthForSystemSkills(t *testing.T) { + ctx := store.WithTenantID(context.Background(), uuid.New()) + sk := store.SkillInfo{ + ID: uuid.NewString(), + Name: "System Skill", + Slug: "system-skill", + Status: "active", + Enabled: true, + IsSystem: true, + } + resp := effectiveAccessForSkill(ctx, sk, effectiveAccessIndex{accessibleBySlug: map[string]bool{}}, "target-user") + if resp.Accessible || resp.Reason != "none" { + t.Fatalf("effective access = %+v, want inaccessible none", resp) + } + resp = effectiveAccessForSkill(ctx, sk, effectiveAccessIndex{accessibleBySlug: map[string]bool{"system-skill": true}}, "target-user") + if !resp.Accessible || resp.Reason != "system" { + t.Fatalf("effective access = %+v, want system access", resp) + } +} + +func TestHandlePatchSkillAccessRejectsNonMasterSystemSkillUpdate(t *testing.T) { + handler, skillStore, _, root := newTestUploadHandler(t) + tenantID := uuid.New() + skillID := skillStore.seedSystemSkill("system-skill", filepath.Join(root, "bundled", "system-skill")) + setupTestCache(t, map[string]*store.APIKeyData{ + crypto.HashAPIKey("tenant-admin-token"): { + ID: uuid.New(), + Scopes: []string{"operator.admin"}, + TenantID: tenantID, + OwnerID: "tenant-admin", + }, + }) + + mux := http.NewServeMux() + handler.RegisterRoutes(mux) + req := httptest.NewRequest(http.MethodPatch, "/v1/skills/"+skillID.String()+"/access", strings.NewReader(`{"mode":"public"}`)) + req.Header.Set("Authorization", "Bearer tenant-admin-token") + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + mux.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body = %s", w.Code, w.Body.String()) + } + if updates := skillStore.lastUpdates[skillID]; updates != nil { + t.Fatalf("system skill updated by non-master tenant: %+v", updates) + } +} + +func TestHandlePatchSkillAccessAcceptsModeAlias(t *testing.T) { + handler, skillStore, ctx, root := newTestUploadHandler(t) + skillID := skillStore.seedCustomSkill("access-skill", filepath.Join(root, "skills-store", "access-skill", "1"), "active", nil) + + req := httptest.NewRequest(http.MethodPatch, "/v1/skills/"+skillID.String()+"/access", bytes.NewBufferString(`{"mode":"PUBLIC"}`)).WithContext(ctx) + req.Header.Set("Content-Type", "application/json") + req.SetPathValue("id", skillID.String()) + w := httptest.NewRecorder() + + handler.handlePatchSkillAccess(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", w.Code, w.Body.String()) + } + info, _ := skillStore.GetSkillByID(ctx, skillID) + if info.Visibility != "public" { + t.Fatalf("visibility = %q, want public", info.Visibility) + } +} diff --git a/internal/http/skills_upload_test.go b/internal/http/skills_upload_test.go index de45592c..0974784d 100644 --- a/internal/http/skills_upload_test.go +++ b/internal/http/skills_upload_test.go @@ -857,15 +857,18 @@ func skillMarkdown(name, slug string) string { } type skillManageStoreStub struct { - baseDir string - version int64 - nextBySlug map[string]int - skills map[uuid.UUID]store.SkillInfo - systemDirs map[string]string - hashBySlug map[string]string // slug -> SKILL.md content hash (most recent) - grantCalls []skillGrantCall - grantErrors map[uuid.UUID]error - lastUpdates map[uuid.UUID]map[string]any + baseDir string + version int64 + nextBySlug map[string]int + skills map[uuid.UUID]store.SkillInfo + systemDirs map[string]string + hashBySlug map[string]string // slug -> SKILL.md content hash (most recent) + grantCalls []skillGrantCall + userGrantCalls []skillUserGrantCall + grantErrors map[uuid.UUID]error + lastUpdates map[uuid.UUID]map[string]any + agentGrants map[uuid.UUID][]store.SkillAgentGrantInfo + userGrants map[uuid.UUID][]store.SkillUserGrantInfo } type skillUploadSystemConfigStore struct { @@ -904,6 +907,12 @@ type skillGrantCall struct { CanManage bool } +type skillUserGrantCall struct { + SkillID uuid.UUID + UserID string + GrantedBy string +} + func newSkillManageStoreStub(baseDir string) *skillManageStoreStub { return &skillManageStoreStub{ baseDir: baseDir, @@ -913,10 +922,12 @@ func newSkillManageStoreStub(baseDir string) *skillManageStoreStub { hashBySlug: map[string]string{}, grantErrors: map[uuid.UUID]error{}, lastUpdates: map[uuid.UUID]map[string]any{}, + agentGrants: map[uuid.UUID][]store.SkillAgentGrantInfo{}, + userGrants: map[uuid.UUID][]store.SkillUserGrantInfo{}, } } -func (s *skillManageStoreStub) seedSystemSkill(slug, dir string) { +func (s *skillManageStoreStub) seedSystemSkill(slug, dir string) uuid.UUID { id := uuid.New() s.skills[id] = store.SkillInfo{ ID: id.String(), @@ -931,6 +942,7 @@ func (s *skillManageStoreStub) seedSystemSkill(slug, dir string) { IsSystem: true, } s.systemDirs[slug] = dir + return id } func (s *skillManageStoreStub) seedCustomSkill(slug, dir, status string, missing []string) uuid.UUID { @@ -1143,15 +1155,89 @@ func (s *skillManageStoreStub) GrantToAgent(_ context.Context, skillID uuid.UUID func (s *skillManageStoreStub) RevokeFromAgent(context.Context, uuid.UUID, uuid.UUID) error { return nil } -func (s *skillManageStoreStub) GrantToUser(context.Context, uuid.UUID, string, string) error { +func (s *skillManageStoreStub) GrantToUser(_ context.Context, skillID uuid.UUID, userID, grantedBy string) error { + s.userGrantCalls = append(s.userGrantCalls, skillUserGrantCall{SkillID: skillID, UserID: userID, GrantedBy: grantedBy}) return nil } func (s *skillManageStoreStub) RevokeFromUser(context.Context, uuid.UUID, string) error { return nil } -func (s *skillManageStoreStub) ListWithGrantStatus(context.Context, uuid.UUID) ([]store.SkillWithGrantStatus, error) { - return nil, nil +func (s *skillManageStoreStub) ListAccessible(ctx context.Context, agentID uuid.UUID, userID string) ([]store.SkillInfo, error) { + actorID := store.ActorIDFromContext(ctx) + if actorID == "" { + actorID = userID + } + out := make([]store.SkillInfo, 0, len(s.skills)) + for id, skill := range s.skills { + if skill.Status != "active" || !skill.Enabled || !s.canAccessSkill(ctx, skill) { + continue + } + switch skill.Visibility { + case skills.VisibilityPublic: + out = append(out, skill) + case skills.VisibilityPrivate: + if skill.OwnerID == userID || skill.OwnerID == actorID { + out = append(out, skill) + } + case skills.VisibilityInternal: + if s.hasAgentGrant(id, agentID) || s.hasUserGrant(id, userID) || s.hasUserGrant(id, actorID) { + out = append(out, skill) + } + default: + if skill.IsSystem { + out = append(out, skill) + } + } + } + return out, nil } -func (s *skillManageStoreStub) ListAgentGrantsForSkill(context.Context, uuid.UUID) ([]store.SkillAgentGrantInfo, error) { - return nil, nil +func (s *skillManageStoreStub) ListWithGrantStatus(ctx context.Context, agentID uuid.UUID) ([]store.SkillWithGrantStatus, error) { + out := make([]store.SkillWithGrantStatus, 0, len(s.skills)) + for id, skill := range s.skills { + if skill.Status != "active" || !s.canAccessSkill(ctx, skill) { + continue + } + row := store.SkillWithGrantStatus{ + ID: id, + Name: skill.Name, + Slug: skill.Slug, + Description: skill.Description, + Visibility: skill.Visibility, + Version: skill.Version, + IsSystem: skill.IsSystem, + } + for _, grant := range s.agentGrants[id] { + if grant.AgentID == agentID { + row.Granted = true + row.CanManage = grant.CanManage + pinned := grant.PinnedVersion + row.PinnedVer = &pinned + break + } + } + out = append(out, row) + } + return out, nil +} +func (s *skillManageStoreStub) hasAgentGrant(skillID, agentID uuid.UUID) bool { + for _, grant := range s.agentGrants[skillID] { + if grant.AgentID == agentID { + return true + } + } + return false +} +func (s *skillManageStoreStub) hasUserGrant(skillID uuid.UUID, userID string) bool { + for _, grant := range s.userGrants[skillID] { + if grant.UserID == userID { + return true + } + } + return false +} +func (s *skillManageStoreStub) ListAgentGrantsForSkill(_ context.Context, skillID uuid.UUID) ([]store.SkillAgentGrantInfo, error) { + return append([]store.SkillAgentGrantInfo(nil), s.agentGrants[skillID]...), nil +} +func (s *skillManageStoreStub) ListUserGrantsForSkill(_ context.Context, skillID uuid.UUID) ([]store.SkillUserGrantInfo, error) { + return append([]store.SkillUserGrantInfo(nil), s.userGrants[skillID]...), nil } func (s *skillManageStoreStub) AgentCanManageSkill(context.Context, uuid.UUID, uuid.UUID) (bool, error) { return false, nil diff --git a/internal/skills/dep_installer.go b/internal/skills/dep_installer.go index 0628b0ba..528b0216 100644 --- a/internal/skills/dep_installer.go +++ b/internal/skills/dep_installer.go @@ -50,6 +50,7 @@ type InstallResult struct { System []string `json:"system,omitempty"` Pip []string `json:"pip,omitempty"` Npm []string `json:"npm,omitempty"` + GitHub []string `json:"github,omitempty"` Errors []string `json:"errors,omitempty"` } diff --git a/internal/skills/dep_scanner.go b/internal/skills/dep_scanner.go index e828ab89..c0b10e39 100644 --- a/internal/skills/dep_scanner.go +++ b/internal/skills/dep_scanner.go @@ -24,7 +24,7 @@ type SkillManifest struct { // IsEmpty returns true if the manifest has no dependencies. func (m *SkillManifest) IsEmpty() bool { - return len(m.Requires) == 0 && len(m.RequiresPython) == 0 && len(m.RequiresNode) == 0 + return len(m.Requires) == 0 && len(m.RequiresPython) == 0 && len(m.RequiresNode) == 0 && len(m.Explicit) == 0 } // ScanSkillDeps auto-detects dependencies by statically analyzing the scripts/ directory, diff --git a/internal/store/pg/skills_grants.go b/internal/store/pg/skills_grants.go index 04a710a4..8374fe20 100644 --- a/internal/store/pg/skills_grants.go +++ b/internal/store/pg/skills_grants.go @@ -321,17 +321,37 @@ func (s *PGSkillStore) GrantToUser(ctx context.Context, skillID uuid.UUID, userI if err := store.ValidateUserID(grantedBy); err != nil { return err } + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } _, err := s.db.ExecContext(ctx, `INSERT INTO skill_user_grants (id, skill_id, user_id, granted_by, created_at, tenant_id) VALUES ($1, $2, $3, $4, $5, $6) - ON CONFLICT (skill_id, user_id) DO NOTHING`, - store.GenNewID(), skillID, userID, grantedBy, time.Now(), tenantIDForInsert(ctx), + ON CONFLICT (skill_id, user_id, tenant_id) DO NOTHING`, + store.GenNewID(), skillID, userID, grantedBy, time.Now(), tid, ) - return err + if err != nil { + return err + } + _, err = s.db.ExecContext(ctx, + `UPDATE skills + SET visibility = 'internal', updated_at = NOW() + WHERE id = $1 AND visibility = 'private' AND (is_system = true OR tenant_id = $2)`, + skillID, tid) + if err != nil { + slog.Warn("skill_grants: failed to auto-promote visibility for user grant", "skill_id", skillID, "error", err) + } + s.BumpVersion() + return nil } // RevokeFromUser revokes a skill grant from a user. func (s *PGSkillStore) RevokeFromUser(ctx context.Context, skillID uuid.UUID, userID string) error { + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } tClause, tArgs, _, err := scopeClause(ctx, 3) if err != nil { return err @@ -339,7 +359,42 @@ func (s *PGSkillStore) RevokeFromUser(ctx context.Context, skillID uuid.UUID, us _, err = s.db.ExecContext(ctx, "DELETE FROM skill_user_grants WHERE skill_id = $1 AND user_id = $2"+tClause, append([]any{skillID, userID}, tArgs...)...) - return err + if err != nil { + return err + } + _, err = s.db.ExecContext(ctx, + `UPDATE skills SET visibility = 'private', updated_at = NOW() + WHERE id = $1 AND visibility = 'internal' AND (is_system = true OR tenant_id = $2) + AND NOT EXISTS (SELECT 1 FROM skill_agent_grants WHERE skill_id = $1) + AND NOT EXISTS (SELECT 1 FROM skill_user_grants WHERE skill_id = $1)`, + skillID, tid) + if err != nil { + slog.Warn("skill_grants: failed to auto-demote visibility for user grant", "skill_id", skillID, "error", err) + } + s.BumpVersion() + return nil +} + +// ListUserGrantsForSkill returns all user grants for one skill. +func (s *PGSkillStore) ListUserGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillUserGrantInfo, error) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return nil, err + } + tClause, tArgs, _, err := scopeClauseAlias(ctx, 2, "sug") + if err != nil { + return nil, err + } + var result []store.SkillUserGrantInfo + err = pkgSqlxDB.SelectContext(ctx, &result, + `SELECT sug.user_id, sug.granted_by + FROM skill_user_grants sug + WHERE sug.skill_id = $1`+tClause+` + ORDER BY sug.created_at DESC`, + append([]any{skillID}, tArgs...)...) + if err != nil { + return nil, err + } + return result, nil } // ListAccessible returns skills accessible to a given agent+user combination. @@ -364,9 +419,13 @@ func (s *PGSkillStore) ListAccessible(ctx context.Context, agentID uuid.UUID, us return nil, err } tenantCond := "" + agentGrantTenantCond := "" + userGrantTenantCond := "" if tc != "" { // tc is " AND tenant_id = $4"; we need it as an OR condition inside the WHERE tenantCond = fmt.Sprintf(" AND (s.is_system = true OR s.tenant_id = $%d)", 4) + agentGrantTenantCond = fmt.Sprintf(" AND sag.tenant_id = $%d", 4) + userGrantTenantCond = fmt.Sprintf(" AND sug.tenant_id = $%d", 4) _ = tc // tcArgs carries the value } // LEFT JOIN skill_tenant_configs to exclude per-tenant disabled skills. @@ -379,8 +438,8 @@ func (s *PGSkillStore) ListAccessible(ctx context.Context, agentID uuid.UUID, us } rows, err := s.db.QueryContext(ctx, `SELECT DISTINCT s.name, s.slug, s.description, s.version, s.file_path FROM skills s - LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = $1 - LEFT JOIN skill_user_grants sug ON s.id = sug.skill_id AND (sug.user_id = $2 OR sug.user_id = $3)`+stcJoin+` + LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = $1`+agentGrantTenantCond+` + LEFT JOIN skill_user_grants sug ON s.id = sug.skill_id AND (sug.user_id = $2 OR sug.user_id = $3)`+userGrantTenantCond+stcJoin+` WHERE s.status = 'active'`+tenantCond+stcFilter+` AND ( s.is_system = true OR s.visibility = 'public' diff --git a/internal/store/skill_store.go b/internal/store/skill_store.go index 7ac897fd..5d554d80 100644 --- a/internal/store/skill_store.go +++ b/internal/store/skill_store.go @@ -114,6 +114,12 @@ type SkillAgentGrantInfo struct { CanManage bool `json:"can_manage" db:"can_manage"` } +// SkillUserGrantInfo is a user grant row for one skill. +type SkillUserGrantInfo struct { + UserID string `json:"user_id" db:"user_id"` + GrantedBy string `json:"granted_by" db:"granted_by"` +} + // SkillManageStore extends SkillStore with CRUD, ownership, and grant operations // needed by HTTP upload handlers and agent tools (skill_manage, publish_skill). // Implemented by both PGSkillStore and SQLiteSkillStore. @@ -146,6 +152,7 @@ type SkillManageStore interface { RevokeFromUser(ctx context.Context, skillID uuid.UUID, userID string) error ListWithGrantStatus(ctx context.Context, agentID uuid.UUID) ([]SkillWithGrantStatus, error) ListAgentGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]SkillAgentGrantInfo, error) + ListUserGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]SkillUserGrantInfo, error) AgentCanManageSkill(ctx context.Context, skillID, agentID uuid.UUID) (bool, error) // Files GetSkillFilePath(ctx context.Context, id uuid.UUID) (filePath string, slug string, version int, isSystem bool, ok bool) diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index e7791b79..af8816e0 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 46 +const SchemaVersion = 47 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -831,6 +831,23 @@ CREATE TABLE IF NOT EXISTS secure_cli_agent_credentials ( CREATE INDEX IF NOT EXISTS idx_scac_tenant ON secure_cli_agent_credentials(tenant_id); CREATE INDEX IF NOT EXISTS idx_scac_binary ON secure_cli_agent_credentials(binary_id); CREATE INDEX IF NOT EXISTS idx_scac_agent ON secure_cli_agent_credentials(agent_id);`, + // Version 46 → 47: scope skill user-grant uniqueness by tenant. + 46: `CREATE TABLE skill_user_grants_new ( + id TEXT NOT NULL PRIMARY KEY, + skill_id TEXT NOT NULL REFERENCES skills(id) ON DELETE CASCADE, + user_id VARCHAR(255) NOT NULL, + granted_by VARCHAR(255) NOT NULL, + tenant_id TEXT NOT NULL REFERENCES tenants(id), + created_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + UNIQUE(skill_id, user_id, tenant_id) +); +INSERT OR IGNORE INTO skill_user_grants_new (id, skill_id, user_id, granted_by, tenant_id, created_at) + SELECT id, skill_id, user_id, granted_by, tenant_id, created_at + FROM skill_user_grants; +DROP TABLE skill_user_grants; +ALTER TABLE skill_user_grants_new RENAME TO skill_user_grants; +CREATE INDEX IF NOT EXISTS idx_skill_user_grants_user ON skill_user_grants(user_id); +CREATE INDEX IF NOT EXISTS idx_skill_user_grants_tenant ON skill_user_grants(tenant_id);`, } const addChannelMemoryExtractionTables = ` diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index 28989c07..df8eb703 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -428,7 +428,7 @@ CREATE TABLE IF NOT EXISTS skill_user_grants ( granted_by VARCHAR(255) NOT NULL, tenant_id TEXT NOT NULL REFERENCES tenants(id), created_at TEXT DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - UNIQUE(skill_id, user_id) + UNIQUE(skill_id, user_id, tenant_id) ); CREATE INDEX IF NOT EXISTS idx_skill_user_grants_user ON skill_user_grants(user_id); diff --git a/internal/store/sqlitestore/skills.go b/internal/store/sqlitestore/skills.go index ff6d8182..62b89236 100644 --- a/internal/store/sqlitestore/skills.go +++ b/internal/store/sqlitestore/skills.go @@ -123,18 +123,18 @@ func (s *SQLiteSkillStore) ListAllSkills(ctx context.Context) []store.SkillInfo var err error if store.IsCrossTenant(ctx) { rows, err = s.db.QueryContext(ctx, - `SELECT id, tenant_id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path - FROM skills WHERE enabled = 1 AND status != 'deleted' - ORDER BY name`) + `SELECT id, tenant_id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path + FROM skills WHERE enabled = 1 AND status != 'deleted' + ORDER BY name`) } else { tid := store.TenantIDFromContext(ctx) if tid == uuid.Nil { tid = store.MasterTenantID } rows, err = s.db.QueryContext(ctx, - `SELECT id, tenant_id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path - FROM skills WHERE enabled = 1 AND status != 'deleted' AND (is_system = 1 OR tenant_id = ?) - ORDER BY name`, tid) + `SELECT id, tenant_id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path + FROM skills WHERE enabled = 1 AND status != 'deleted' AND (is_system = 1 OR tenant_id = ?) + ORDER BY name`, tid) } if err != nil { return nil @@ -145,9 +145,9 @@ func (s *SQLiteSkillStore) ListAllSkills(ctx context.Context) []store.SkillInfo func (s *SQLiteSkillStore) ListAllSystemSkills(ctx context.Context) []store.SkillInfo { rows, err := s.db.QueryContext(ctx, - `SELECT id, tenant_id, name, slug, description, visibility, tags, version, is_system, status, enabled, deps, file_path - FROM skills WHERE is_system = 1 AND enabled = 1 AND status != 'deleted' - ORDER BY name`) + `SELECT id, tenant_id, name, slug, description, visibility, owner_id, tags, version, is_system, status, enabled, deps, file_path + FROM skills WHERE is_system = 1 AND enabled = 1 AND status != 'deleted' + ORDER BY name`) if err != nil { return nil } @@ -160,20 +160,21 @@ func (s *SQLiteSkillStore) scanSkillInfoList(rows *sql.Rows) []store.SkillInfo { for rows.Next() { var id uuid.UUID var tenantID uuid.UUID - var name, slug, visibility, status string + var name, slug, visibility, ownerID, status string var desc *string var tagsJSON []byte var version int var isSystem, enabled bool var depsRaw []byte var filePath *string - if err := rows.Scan(&id, &tenantID, &name, &slug, &desc, &visibility, &tagsJSON, &version, + if err := rows.Scan(&id, &tenantID, &name, &slug, &desc, &visibility, &ownerID, &tagsJSON, &version, &isSystem, &status, &enabled, &depsRaw, &filePath); err != nil { continue } info := buildSkillInfo(id.String(), name, slug, desc, version, s.baseDir, filePath) info.TenantID = tenantID.String() info.Visibility = visibility + info.OwnerID = ownerID scanJSONStringArray(tagsJSON, &info.Tags) info.IsSystem = isSystem info.Status = status diff --git a/internal/store/sqlitestore/skills_grants.go b/internal/store/sqlitestore/skills_grants.go index 823b5ff7..9385c5a7 100644 --- a/internal/store/sqlitestore/skills_grants.go +++ b/internal/store/sqlitestore/skills_grants.go @@ -370,18 +370,38 @@ func (s *SQLiteSkillStore) GrantToUser(ctx context.Context, skillID uuid.UUID, u if err := store.ValidateUserID(grantedBy); err != nil { return err } + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } id := store.GenNewID() _, err := s.db.ExecContext(ctx, `INSERT INTO skill_user_grants (id, skill_id, user_id, granted_by, created_at, tenant_id) VALUES (?, ?, ?, ?, ?, ?) - ON CONFLICT (skill_id, user_id) DO NOTHING`, - id, skillID, userID, grantedBy, time.Now().UTC(), tenantIDForInsert(ctx), + ON CONFLICT (skill_id, user_id, tenant_id) DO NOTHING`, + id, skillID, userID, grantedBy, time.Now().UTC(), tid, ) - return err + if err != nil { + return err + } + _, err = s.db.ExecContext(ctx, + `UPDATE skills + SET visibility = 'internal', updated_at = ? + WHERE id = ? AND visibility = 'private' AND (is_system = 1 OR tenant_id = ?)`, + time.Now().UTC(), skillID, tid) + if err != nil { + slog.Warn("skill_grants: failed to auto-promote visibility for user grant", "skill_id", skillID, "error", err) + } + s.BumpVersion() + return nil } // RevokeFromUser revokes a skill grant from a user. func (s *SQLiteSkillStore) RevokeFromUser(ctx context.Context, skillID uuid.UUID, userID string) error { + tid := tenantIDForInsert(ctx) + if err := s.verifySkillInGrantScope(ctx, skillID, tid); err != nil { + return err + } tClause, tArgs, err := scopeClause(ctx) if err != nil { return err @@ -389,7 +409,52 @@ func (s *SQLiteSkillStore) RevokeFromUser(ctx context.Context, skillID uuid.UUID _, err = s.db.ExecContext(ctx, "DELETE FROM skill_user_grants WHERE skill_id = ? AND user_id = ?"+tClause, append([]any{skillID, userID}, tArgs...)...) - return err + if err != nil { + return err + } + _, err = s.db.ExecContext(ctx, + `UPDATE skills SET visibility = 'private', updated_at = ? + WHERE id = ? AND visibility = 'internal' AND (is_system = 1 OR tenant_id = ?) + AND NOT EXISTS (SELECT 1 FROM skill_agent_grants WHERE skill_id = ?) + AND NOT EXISTS (SELECT 1 FROM skill_user_grants WHERE skill_id = ?)`, + time.Now().UTC(), skillID, tid, skillID, skillID) + if err != nil { + slog.Warn("skill_grants: failed to auto-demote visibility for user grant", "skill_id", skillID, "error", err) + } + s.BumpVersion() + return nil +} + +// ListUserGrantsForSkill returns all user grants for one skill. +func (s *SQLiteSkillStore) ListUserGrantsForSkill(ctx context.Context, skillID uuid.UUID) ([]store.SkillUserGrantInfo, error) { + if err := s.verifySkillInGrantScope(ctx, skillID, tenantIDForInsert(ctx)); err != nil { + return nil, err + } + tClause, tArgs, err := scopeClause(ctx) + if err != nil { + return nil, err + } + rows, err := s.db.QueryContext(ctx, + `SELECT sug.user_id, sug.granted_by + FROM skill_user_grants sug + WHERE sug.skill_id = ?`+strings.ReplaceAll(tClause, "tenant_id", "sug.tenant_id")+` + ORDER BY sug.created_at DESC`, + append([]any{skillID}, tArgs...)...) + if err != nil { + return nil, err + } + defer rows.Close() + + var result []store.SkillUserGrantInfo + for rows.Next() { + var g store.SkillUserGrantInfo + if err := rows.Scan(&g.UserID, &g.GrantedBy); err != nil { + slog.Warn("skill_grants: scan error in ListUserGrantsForSkill", "error", err) + continue + } + result = append(result, g) + } + return result, rows.Err() } // ListAccessible returns skills accessible to a given agent+user combination. @@ -404,27 +469,35 @@ func (s *SQLiteSkillStore) ListAccessible(ctx context.Context, agentID uuid.UUID return nil, err } tenantCond := "" + agentGrantTenantCond := "" + userGrantTenantCond := "" stcJoin := "" stcFilter := "" if len(tArgs) > 0 { tenantCond = " AND (s.is_system = 1 OR s.tenant_id = ?)" + agentGrantTenantCond = " AND sag.tenant_id = ?" + userGrantTenantCond = " AND sug.tenant_id = ?" stcJoin = " LEFT JOIN skill_tenant_configs stc ON s.id = stc.skill_id AND stc.tenant_id = ?" stcFilter = " AND (stc.enabled IS NULL OR stc.enabled = 1)" } - // Positional args: agentID, userID, actorID, [tenantID x2 if tenant-scoped], userID, actorID (private-owner clause) - queryArgs := []any{agentID, userID, actorID} + queryArgs := []any{agentID} if len(tArgs) > 0 { - queryArgs = append(queryArgs, tArgs...) // tenant cond + queryArgs = append(queryArgs, tArgs...) // sag tenant + } + queryArgs = append(queryArgs, userID, actorID) + if len(tArgs) > 0 { + queryArgs = append(queryArgs, tArgs...) // sug tenant queryArgs = append(queryArgs, tArgs...) // stc join + queryArgs = append(queryArgs, tArgs...) // tenant cond } // Remove tClause (aliased scope) — we handle it manually above. _ = tClause rows, err := s.db.QueryContext(ctx, `SELECT DISTINCT s.name, s.slug, s.description, s.version, s.file_path FROM skills s - LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = ? - LEFT JOIN skill_user_grants sug ON s.id = sug.skill_id AND (sug.user_id = ? OR sug.user_id = ?)`+stcJoin+` + LEFT JOIN skill_agent_grants sag ON s.id = sag.skill_id AND sag.agent_id = ?`+agentGrantTenantCond+` + LEFT JOIN skill_user_grants sug ON s.id = sug.skill_id AND (sug.user_id = ? OR sug.user_id = ?)`+userGrantTenantCond+stcJoin+` WHERE s.status = 'active'`+tenantCond+stcFilter+` AND ( s.is_system = 1 OR s.visibility = 'public' diff --git a/internal/store/sqlitestore/skills_test.go b/internal/store/sqlitestore/skills_test.go index 76fcf03c..93be73cf 100644 --- a/internal/store/sqlitestore/skills_test.go +++ b/internal/store/sqlitestore/skills_test.go @@ -250,6 +250,89 @@ func TestSQLiteSkillStore_RevokeFromAgentKeepsInternalWhenUserGrantRemains(t *te } } +func TestSQLiteSkillStore_UserGrantListPromotesAndDemotesVisibility(t *testing.T) { + _, skillStore, db := newTestSQLiteSkillStoreWithDB(t) + tenantID, _ := seedSQLiteTenantAgent(t, db) + ctx := store.WithTenantID(context.Background(), tenantID) + + skillID, err := skillStore.CreateSkillManaged(ctx, store.SkillCreateParams{ + Name: "User Shared Skill", + Slug: "user-shared-skill-" + tenantID.String()[:8], + OwnerID: "owner-user", + Visibility: "private", + FilePath: filepath.Join(t.TempDir(), "user-shared-skill", "1"), + }) + if err != nil { + t.Fatalf("CreateSkillManaged error: %v", err) + } + if err := skillStore.GrantToUser(ctx, skillID, "granted-user", "owner-user"); err != nil { + t.Fatalf("GrantToUser error: %v", err) + } + + grants, err := skillStore.ListUserGrantsForSkill(ctx, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill error: %v", err) + } + if len(grants) != 1 || grants[0].UserID != "granted-user" || grants[0].GrantedBy != "owner-user" { + t.Fatalf("user grants = %+v", grants) + } + got, ok := skillStore.GetSkillByID(ctx, skillID) + if !ok { + t.Fatal("GetSkillByID returned !ok") + } + if got.Visibility != "internal" { + t.Fatalf("visibility after user grant = %q, want internal", got.Visibility) + } + if err := skillStore.RevokeFromUser(ctx, skillID, "granted-user"); err != nil { + t.Fatalf("RevokeFromUser error: %v", err) + } + got, ok = skillStore.GetSkillByID(ctx, skillID) + if !ok { + t.Fatal("GetSkillByID returned !ok after revoke") + } + if got.Visibility != "private" { + t.Fatalf("visibility after last user grant revoke = %q, want private", got.Visibility) + } +} + +func TestSQLiteSkillStore_UserGrantsAreTenantScopedForSystemSkill(t *testing.T) { + _, skillStore, db := newTestSQLiteSkillStoreWithDB(t) + tenantA, _ := seedSQLiteTenantAgent(t, db) + tenantB, _ := seedSQLiteTenantAgent(t, db) + ctxA := store.WithTenantID(context.Background(), tenantA) + ctxB := store.WithTenantID(context.Background(), tenantB) + skillID := uuid.New() + slug := "system-user-grant-" + skillID.String()[:8] + if _, err := db.Exec( + `INSERT INTO skills (id, name, slug, owner_id, visibility, version, status, file_path, is_system, tenant_id) + VALUES (?, 'System User Grant Skill', ?, 'system', 'private', 1, 'active', ?, 1, ?)`, + skillID.String(), slug, filepath.Join(t.TempDir(), "system-user-grant", "1"), store.MasterTenantID.String(), + ); err != nil { + t.Fatalf("insert system skill: %v", err) + } + + if err := skillStore.GrantToUser(ctxA, skillID, "same-user", "tenant-a-admin"); err != nil { + t.Fatalf("GrantToUser tenant A error: %v", err) + } + if err := skillStore.GrantToUser(ctxB, skillID, "same-user", "tenant-b-admin"); err != nil { + t.Fatalf("GrantToUser tenant B error: %v", err) + } + grantsA, err := skillStore.ListUserGrantsForSkill(ctxA, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill tenant A error: %v", err) + } + grantsB, err := skillStore.ListUserGrantsForSkill(ctxB, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill tenant B error: %v", err) + } + if len(grantsA) != 1 || grantsA[0].GrantedBy != "tenant-a-admin" { + t.Fatalf("tenant A grants = %+v", grantsA) + } + if len(grantsB) != 1 || grantsB[0].GrantedBy != "tenant-b-admin" { + t.Fatalf("tenant B grants = %+v", grantsB) + } +} + func TestSQLiteSkillStore_ListWithGrantStatusIgnoresForeignTenantGrant(t *testing.T) { _, skillStore, db := newTestSQLiteSkillStoreWithDB(t) tenantA, _ := seedSQLiteTenantAgent(t, db) @@ -365,6 +448,30 @@ func TestSQLiteSkillStore_ListAccessibleHonorsAccessModes(t *testing.T) { } } +func TestSQLiteSkillStore_ListAllSkillsIncludesOwnerID(t *testing.T) { + ctx, skillStore := newTestSQLiteSkillStore(t) + skillID, err := skillStore.CreateSkillManaged(ctx, store.SkillCreateParams{ + Name: "Owner Projection Skill", + Slug: "owner-projection-skill", + OwnerID: "owner-user", + Visibility: "private", + FilePath: filepath.Join(t.TempDir(), "owner-projection", "1"), + }) + if err != nil { + t.Fatalf("CreateSkillManaged error: %v", err) + } + + for _, skill := range skillStore.ListAllSkills(ctx) { + if skill.ID == skillID.String() { + if skill.OwnerID != "owner-user" { + t.Fatalf("OwnerID = %q, want owner-user", skill.OwnerID) + } + return + } + } + t.Fatalf("created skill %s not found", skillID) +} + func listAccessibleSlugs(t *testing.T, skillStore *SQLiteSkillStore, ctx context.Context, agentID uuid.UUID, userID string) map[string]bool { t.Helper() skills, err := skillStore.ListAccessible(ctx, agentID, userID) diff --git a/internal/tools/skill_manage_files_test.go b/internal/tools/skill_manage_files_test.go index 298df992..4e7fa96d 100644 --- a/internal/tools/skill_manage_files_test.go +++ b/internal/tools/skill_manage_files_test.go @@ -594,6 +594,9 @@ func (s *skillManageFilesStore) ListWithGrantStatus(context.Context, uuid.UUID) func (s *skillManageFilesStore) ListAgentGrantsForSkill(context.Context, uuid.UUID) ([]store.SkillAgentGrantInfo, error) { return nil, nil } +func (s *skillManageFilesStore) ListUserGrantsForSkill(context.Context, uuid.UUID) ([]store.SkillUserGrantInfo, error) { + return nil, nil +} func (s *skillManageFilesStore) AgentCanManageSkill(context.Context, uuid.UUID, uuid.UUID) (bool, error) { return false, nil } diff --git a/internal/upgrade/version.go b/internal/upgrade/version.go index 36f249f9..67b4ce6e 100644 --- a/internal/upgrade/version.go +++ b/internal/upgrade/version.go @@ -2,4 +2,4 @@ package upgrade // RequiredSchemaVersion is the schema migration version this binary requires. // Bump this whenever adding a new SQL migration file. -const RequiredSchemaVersion uint = 77 +const RequiredSchemaVersion uint = 78 diff --git a/migrations/000078_skill_user_grants_tenant_unique.down.sql b/migrations/000078_skill_user_grants_tenant_unique.down.sql new file mode 100644 index 00000000..d1a8bb95 --- /dev/null +++ b/migrations/000078_skill_user_grants_tenant_unique.down.sql @@ -0,0 +1,6 @@ +ALTER TABLE skill_user_grants + DROP CONSTRAINT IF EXISTS skill_user_grants_skill_id_user_id_tenant_id_key; + +ALTER TABLE skill_user_grants + ADD CONSTRAINT skill_user_grants_skill_id_user_id_key + UNIQUE (skill_id, user_id); diff --git a/migrations/000078_skill_user_grants_tenant_unique.up.sql b/migrations/000078_skill_user_grants_tenant_unique.up.sql new file mode 100644 index 00000000..936a749a --- /dev/null +++ b/migrations/000078_skill_user_grants_tenant_unique.up.sql @@ -0,0 +1,9 @@ +ALTER TABLE skill_user_grants + DROP CONSTRAINT IF EXISTS skill_user_grants_skill_id_user_id_key; + +ALTER TABLE skill_user_grants + DROP CONSTRAINT IF EXISTS skill_user_grants_skill_id_user_id_tenant_id_key; + +ALTER TABLE skill_user_grants + ADD CONSTRAINT skill_user_grants_skill_id_user_id_tenant_id_key + UNIQUE (skill_id, user_id, tenant_id); diff --git a/tests/integration/v3_skills_store_test.go b/tests/integration/v3_skills_store_test.go index 0ae48a34..1bb75fb1 100644 --- a/tests/integration/v3_skills_store_test.go +++ b/tests/integration/v3_skills_store_test.go @@ -346,6 +346,81 @@ func TestStoreSkill_GrantToAgent(t *testing.T) { } } +func TestStoreSkill_UserGrantListPromotesAndDemotesVisibility(t *testing.T) { + db := testDB(t) + tenantID, _ := seedTenantAgent(t, db) + ctx := tenantCtx(tenantID) + s := newSkillStore(t) + + skillID := seedSkill(t, s, ctx, "user-grant-skill-"+tenantID.String()[:8], "User Grant Skill") + if err := s.GrantToUser(ctx, skillID, "granted-user", "test-owner"); err != nil { + t.Fatalf("GrantToUser: %v", err) + } + grants, err := s.ListUserGrantsForSkill(ctx, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill: %v", err) + } + if len(grants) != 1 || grants[0].UserID != "granted-user" || grants[0].GrantedBy != "test-owner" { + t.Fatalf("user grants = %+v", grants) + } + got, ok := s.GetSkillByID(ctx, skillID) + if !ok { + t.Fatal("GetSkillByID returned false") + } + if got.Visibility != "internal" { + t.Fatalf("visibility after user grant = %q, want internal", got.Visibility) + } + if err := s.RevokeFromUser(ctx, skillID, "granted-user"); err != nil { + t.Fatalf("RevokeFromUser: %v", err) + } + got, ok = s.GetSkillByID(ctx, skillID) + if !ok { + t.Fatal("GetSkillByID returned false after revoke") + } + if got.Visibility != "private" { + t.Fatalf("visibility after last user grant revoke = %q, want private", got.Visibility) + } +} + +func TestStoreSkill_UserGrantsAreTenantScopedForSystemSkill(t *testing.T) { + db := testDB(t) + tenantA, _ := seedTenantAgent(t, db) + tenantB, _ := seedTenantAgent(t, db) + ctxA := tenantCtx(tenantA) + ctxB := tenantCtx(tenantB) + s := newSkillStore(t) + skillID := uuid.New() + slug := "system-user-grant-" + skillID.String()[:8] + if _, err := db.Exec( + `INSERT INTO skills (id, name, slug, owner_id, visibility, version, status, file_path, is_system, tenant_id) + VALUES ($1, 'System User Grant Skill', $2, 'system', 'private', 1, 'active', $3, true, $4)`, + skillID, slug, "/tmp/skills/system-user-grant/1", store.MasterTenantID, + ); err != nil { + t.Fatalf("insert system skill: %v", err) + } + + if err := s.GrantToUser(ctxA, skillID, "same-user", "tenant-a-admin"); err != nil { + t.Fatalf("GrantToUser tenant A: %v", err) + } + if err := s.GrantToUser(ctxB, skillID, "same-user", "tenant-b-admin"); err != nil { + t.Fatalf("GrantToUser tenant B: %v", err) + } + grantsA, err := s.ListUserGrantsForSkill(ctxA, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill tenant A: %v", err) + } + grantsB, err := s.ListUserGrantsForSkill(ctxB, skillID) + if err != nil { + t.Fatalf("ListUserGrantsForSkill tenant B: %v", err) + } + if len(grantsA) != 1 || grantsA[0].GrantedBy != "tenant-a-admin" { + t.Fatalf("tenant A grants = %+v", grantsA) + } + if len(grantsB) != 1 || grantsB[0].GrantedBy != "tenant-b-admin" { + t.Fatalf("tenant B grants = %+v", grantsB) + } +} + func TestStoreSkill_GrantToAgentRejectsCrossTenantSkill(t *testing.T) { db := testDB(t) tenantA, agentA := seedTenantAgent(t, db)