From 36c3b74eace13680589efeea116c206f7d097b58 Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Tue, 8 Sep 2026 11:43:23 +0000 Subject: [PATCH 1/2] structured errors --- internal/cli/card_get.go | 8 +- internal/cli/cli_test.go | 39 ++++++ internal/cli/client.go | 10 +- internal/cli/root.go | 52 +++++++- internal/cli/send.go | 15 ++- internal/cli/serve.go | 5 +- internal/clierr/clierr.go | 187 +++++++++++++++++++++++++++ internal/clierr/clierr_test.go | 166 ++++++++++++++++++++++++ internal/flagparse/urlorpath.go | 53 +++++++- internal/flagparse/urlorpath_test.go | 46 +++++++ 10 files changed, 559 insertions(+), 22 deletions(-) create mode 100644 internal/clierr/clierr.go create mode 100644 internal/clierr/clierr_test.go diff --git a/internal/cli/card_get.go b/internal/cli/card_get.go index 913b23b..f7298f9 100644 --- a/internal/cli/card_get.go +++ b/internal/cli/card_get.go @@ -20,6 +20,7 @@ import ( "github.com/spf13/cobra" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-go/v2/a2a" "github.com/a2aproject/a2a-go/v2/a2aclient/agentcard" ) @@ -82,7 +83,10 @@ func getExtendedAgentCard(ctx context.Context, cfg *globalConfig) (*a2a.AgentCar func getPublicAgentCard(ctx context.Context, cfg *globalConfig) (*a2a.AgentCard, error) { if !cfg.agentCard.IsSet() { - return nil, fmt.Errorf("specify the agent card URL as an argument or with --agent-card") + return nil, clierr.Usage("specify the agent card URL as an argument or with --agent-card") + } + if err := cfg.agentCard.Validate(); err != nil { + return nil, clierr.Usage(err.Error()) } ref := cfg.agentCard.URL() @@ -94,7 +98,7 @@ func getPublicAgentCard(ctx context.Context, cfg *globalConfig) (*a2a.AgentCard, card, err := compatCardResolver.Resolve(ctx, ref, resolveOpts...) if err != nil { - return nil, fmt.Errorf("failed to resolve agent card: %w", err) + return nil, clierr.CardResolution(err) } return card, nil } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 4a00667..a55ebfa 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "iter" "net/http" @@ -31,6 +32,7 @@ import ( "github.com/spf13/pflag" "github.com/a2aproject/a2a-cli/internal/clicfg" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/localsrv" "github.com/a2aproject/a2a-cli/internal/output" @@ -715,6 +717,43 @@ func TestGetTask(t *testing.T) { }) } +func TestUsageErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + }{ + {"unknown flag", []string{"card", "get", "--bogus", "http://x"}}, + {"missing positional arg", []string{"task", "get", "-a", "http://x"}}, + {"too many positional args", []string{"task", "get", "-a", "http://x", "one", "two"}}, + {"unknown top-level command", []string{"bogus"}}, + {"mutually exclusive targets", []string{"send", "-a", "http://x", "-e", "http://y", "hi"}}, + {"malformed agent-card url", []string{"card", "get", "-a", "http://exa mple.com"}}, + {"missing agent-card file", []string{"card", "get", "-a", "/no/such/card.json"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := runCMD(t, tt.args...) + if err == nil { + t.Fatalf("runCMD(%v) error = nil, want a usage error", tt.args) + } + var ce *clierr.Error + if !errors.As(err, &ce) { + t.Fatalf("runCMD(%v) error = %v, want *clierr.Error", tt.args, err) + } + if ce.Code != clierr.CodeUsage { + t.Errorf("runCMD(%v) code = %q, want %q", tt.args, ce.Code, clierr.CodeUsage) + } + if ce.Exit != 2 { + t.Errorf("runCMD(%v) exit = %d, want 2", tt.args, ce.Exit) + } + }) + } +} + func TestServe_ModeValidation(t *testing.T) { t.Parallel() for _, tt := range []struct { diff --git a/internal/cli/client.go b/internal/cli/client.go index d973aa4..4f5a8b4 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -25,6 +25,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/transportplugin" "github.com/a2aproject/a2a-go/v2/a2a" @@ -44,13 +45,16 @@ var compatCardResolver = func() *agentcard.Resolver { func newAgentClient(ctx context.Context, cfg *globalConfig, extraOpts ...a2aclient.FactoryOption) (*a2aclient.Client, error) { switch { case cfg.url != "" && cfg.agentCard.IsSet(): - return nil, fmt.Errorf("--endpoint and --agent-card are mutually exclusive") + return nil, clierr.Usage("--endpoint and --agent-card are mutually exclusive") case cfg.url != "": return newClientFromEndpoint(ctx, cfg, cfg.url, extraOpts...) case cfg.agentCard.IsSet(): + if err := cfg.agentCard.Validate(); err != nil { + return nil, clierr.Usage(err.Error()) + } return newClientFromCard(ctx, cfg, cfg.agentCard.URL(), extraOpts...) default: - return nil, fmt.Errorf("either '--agent-card ' or '--endpoint --transport ' must be provided") + return nil, clierr.Usage("either '--agent-card ' or '--endpoint --transport ' must be provided") } } @@ -101,7 +105,7 @@ func newClientFromCard(ctx context.Context, cfg *globalConfig, ref string, extra } card, err := compatCardResolver.Resolve(ctx, ref, resolveOpts...) if err != nil { - return nil, fmt.Errorf("resolving agent card: %w", err) + return nil, clierr.CardResolution(err) } factoryOpts := append(clientFactoryOpts(cfg), extraOpts...) diff --git a/internal/cli/root.go b/internal/cli/root.go index 078005c..2e76005 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" "github.com/a2aproject/a2a-cli/internal/clicfg" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/output" "github.com/a2aproject/a2a-cli/internal/polling" @@ -76,12 +77,26 @@ func Execute() int { } root := newRootCmd(cfg, deps{}) if err := root.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - return 1 + return cfg.renderError(err) } return 0 } +func (g *globalConfig) renderError(err error) int { + ce := clierr.Classify(err) + if g.Mode == output.ModeJson { + if perr := g.PrintJSON(ce); perr != nil { + _, _ = fmt.Fprintf(os.Stderr, "Error: %v\n", err) + } + return ce.Exit + } + fmt.Fprintf(os.Stderr, "Error: %s\n", ce.Message) + if ce.Hint != "" { + fmt.Fprintf(os.Stderr, "hint: %s\n", ce.Hint) + } + return ce.Exit +} + func newRootCmd(cfg *globalConfig, deps deps) *cobra.Command { deps.setDefaults() @@ -91,6 +106,13 @@ func newRootCmd(cfg *globalConfig, deps deps) *cobra.Command { Version: buildVersionInfo().Version, SilenceUsage: true, SilenceErrors: true, + Args: cobra.ArbitraryArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return cmd.Help() + } + return clierr.Usage(fmt.Sprintf("unknown command %q for %q", args[0], cmd.CommandPath())) + }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { store, err := deps.cfgLoader(clicfg.LoadOpts{ConfigPath: cfg.configPath}) if err != nil { @@ -106,7 +128,7 @@ func newRootCmd(cfg *globalConfig, deps deps) *cobra.Command { case output.ModeText, output.ModeJson, output.ModeJSONL: cfg.Mode = output.Mode(cfg.output) default: - return fmt.Errorf("invalid --output %q (want text, json, or jsonl)", cfg.output) + return clierr.Usage(fmt.Sprintf("invalid --output %q (want text or json)", cfg.output)) } return nil }, @@ -136,10 +158,34 @@ func newRootCmd(cfg *globalConfig, deps deps) *cobra.Command { ) cmd.SetUsageTemplate(rootUsageTemplate) + markUsageErrors(cmd) return cmd } +// markUsageErrors makes cobra's flag- and argument-validation failures surface +// as clierr usage errors. +func markUsageErrors(cmd *cobra.Command) { + cmd.SetFlagErrorFunc(func(_ *cobra.Command, err error) error { + return clierr.Usage(err.Error()) + }) + wrapArgsUsage(cmd) +} + +func wrapArgsUsage(cmd *cobra.Command) { + if validate := cmd.Args; validate != nil { + cmd.Args = func(c *cobra.Command, args []string) error { + if err := validate(c, args); err != nil { + return clierr.Usage(err.Error()) + } + return nil + } + } + for _, sub := range cmd.Commands() { + wrapArgsUsage(sub) + } +} + // rootUsageTemplate is cobra's default usage template with one change: the root // command (the one with no parent) lists each command's immediate subcommands // indented beneath it, so the full command surface (e.g. `task get`, `card get`) diff --git a/internal/cli/send.go b/internal/cli/send.go index 0132a67..e8591b7 100644 --- a/internal/cli/send.go +++ b/internal/cli/send.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/cobra" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/utils" "github.com/a2aproject/a2a-go/v2/a2a" @@ -53,7 +54,7 @@ func newSendCmd(cfg *globalConfig, poller pollerFunc) *cobra.Command { Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { if flags.stream && flags.async { - return fmt.Errorf("--stream is incompatible with --async") + return clierr.Usage("--stream is incompatible with --async") } baseCtx := withServiceParams(cmd.Context(), cfg) @@ -182,13 +183,13 @@ func buildMessage(positional []string, flags *sendFlags) (*a2a.Message, error) { return nil, err } if len(positional) > 1 { - return nil, fmt.Errorf("at most one positional argument is allowed, use --text-part for multi-part messages") + return nil, clierr.Usage("at most one positional argument is allowed, use --text-part for multi-part messages") } if len(positional) == 1 { // a2a send "check it out" --file-part -> [TextPart("check it out"), FilePart("")] parts = append([]*a2a.Part{a2a.NewTextPart(positional[0])}, parts...) } if len(parts) == 0 { - return nil, fmt.Errorf("provide a message as text, or via --text-part, --file-part, --data-part, or --request-payload") + return nil, clierr.Usage("provide a message as text, or via --text-part, --file-part, --data-part, or --request-payload") } msg := a2a.NewMessage(a2a.MessageRoleUser, parts...) if flags.taskID != "" { @@ -205,10 +206,10 @@ func buildMessage(positional []string, flags *sendFlags) (*a2a.Message, error) { func parseRequestPayload(ref string) (*a2a.SendMessageRequest, error) { req := new(a2a.SendMessageRequest) if err := json.Unmarshal(flagparse.RawOrInline(ref), req); err != nil { - return nil, fmt.Errorf("--request-payload %q is not a readable file or valid JSON: %w", ref, err) + return nil, clierr.Usage(fmt.Sprintf("--request-payload %q is not a readable file or valid JSON: %v", ref, err)) } if req.Message == nil { - return nil, fmt.Errorf("--request-payload must include a message") + return nil, clierr.Usage("--request-payload must include a message") } if req.Message.ID == "" { req.Message.ID = a2a.NewMessageID() @@ -220,11 +221,11 @@ func parseRequestPayload(ref string) (*a2a.SendMessageRequest, error) { // conflict with a --request-payload, which already carries the whole request. func ensureNoPayloadOverrides(cmd *cobra.Command, positional []string) error { if len(positional) > 0 { - return fmt.Errorf("--request-payload cannot be combined with a positional message") + return clierr.Usage("--request-payload cannot be combined with a positional message") } for _, name := range []string{"text-part", "file-part", "data-part", "task-id", "context-id", "metadata", "history", "async"} { if cmd.Flags().Changed(name) { - return fmt.Errorf("--request-payload cannot be combined with --%s", name) + return clierr.Usage(fmt.Sprintf("--request-payload cannot be combined with --%s", name)) } } return nil diff --git a/internal/cli/serve.go b/internal/cli/serve.go index 5574f4d..81617dc 100644 --- a/internal/cli/serve.go +++ b/internal/cli/serve.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/localsrv" "github.com/a2aproject/a2a-cli/internal/transportplugin" @@ -66,10 +67,10 @@ func newServeCmd(cfg *globalConfig) *cobra.Command { modes++ } if modes > 1 { - return fmt.Errorf("--echo, --proxy, and --exec are mutually exclusive") + return clierr.Usage("--echo, --proxy, and --exec are mutually exclusive") } if modes == 0 { - return fmt.Errorf("specify --echo, --proxy , or --exec ") + return clierr.Usage("specify --echo, --proxy , or --exec ") } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) defer stop() diff --git a/internal/clierr/clierr.go b/internal/clierr/clierr.go new file mode 100644 index 0000000..ac0ee3f --- /dev/null +++ b/internal/clierr/clierr.go @@ -0,0 +1,187 @@ +// Copyright 2026 The A2A Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package clierr classifies errors into the a2a-cli error contract, +package clierr + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net" + "syscall" + + "github.com/a2aproject/a2a-go/v2/a2a" +) + +// Code is a symbolic error identifier. +type Code string + +const ( + // CodeUsage marks invalid arguments, flags, or flag combinations; exit 2. + CodeUsage = "A2ACLI_ERR_USAGE" + // CodeProtocol marks a failure the agent reported as an a2a.Error; the + // specific A2A reason travels in the A2ACode field. Exit 1. + CodeProtocol = "A2ACLI_ERR_PROTOCOL" + // CodeIO marks a transport failure reaching or reading from the agent — + // DNS, connection, TLS, reset, broken pipe. Exit 3. + CodeIO = "A2ACLI_ERR_IO" + // CodeCardInvalid marks a card that was reached but is not usable — a + // non-OK status or a malformed body. Exit 4. + CodeCardInvalid = "A2ACLI_ERR_CARD_INVALID" + // CodeTimeout marks a --timeout that expired before a terminal state; exit 5. + CodeTimeout = "A2ACLI_ERR_TIMEOUT" + // CodeInternal marks an unexpected CLI-side failure, or any condition with + // no better code. Exit 1. + CodeInternal = "A2ACLI_ERR_INTERNAL" +) + +// Error is a classified CLI failure. +type Error struct { + // Code is the symbolic identifier reported to callers. + Code string + // Message is the human-readable, single-line failure description. + Message string + // Hint is optional remediation advice shown to the user. + Hint string + // A2ACode is the A2A protocol error reason, set only for protocol failures. + A2ACode string + // Exit is the process exit status for this failure. + Exit int + + // err is the wrapped cause, exposed via Unwrap. + err error +} + +// Error returns the failure message, satisfying the error interface. +func (e *Error) Error() string { return e.Message } + +// Unwrap returns the wrapped cause so errors.Is and errors.As can inspect it. +func (e *Error) Unwrap() error { return e.err } + +// Usage builds a usage error for invalid arguments, flags, or flag combinations. +func Usage(msg string) *Error { + return &Error{Code: CodeUsage, Message: msg, Exit: 2} +} + +// CardResolution builds an error for a failure to resolve an --agent-card. +// +// It is called only from the card-resolution path, where the reference itself +// is already known to be well-formed (see flagparse.URLOrPath.Validate), so the +// outcome is binary: either the agent could not be reached (an IO failure), or +// it was reached but did not yield a usable card. +func CardResolution(err error) *Error { + msg := fmt.Sprintf("failed to resolve agent card: %v", err) + if isIOError(err) { + return &Error{ + Code: CodeIO, + Message: msg, + Exit: 3, + err: err, + Hint: "check the agent is running and the --agent-card reference is correct", + } + } + return &Error{ + Code: CodeCardInvalid, + Message: msg, + Exit: 4, + Hint: "check the --agent-card reference (host, full card URL, or local file path)", + err: err, + } +} + +// MarshalJSON implements [json.Marshaler]. +func (e *Error) MarshalJSON() ([]byte, error) { + type body struct { + Code string `json:"code"` + Message string `json:"message"` + Hint string `json:"hint,omitempty"` + A2ACode string `json:"a2aCode,omitempty"` + } + type wrapper struct { + Error body `json:"error"` + } + return json.Marshal(wrapper{Error: body{ + Code: e.Code, + Message: e.Message, + A2ACode: e.A2ACode, + Hint: e.Hint, + }}) +} + +// Classify maps an arbitrary error to a classified CLI Error with a stable code +// and exit status. It returns nil for a nil error and passes an already +// classified *Error (e.g. from Usage or CardResolution) through unchanged. +func Classify(err error) *Error { + if err == nil { + return nil + } + + var classified *Error + if errors.As(err, &classified) { + return classified + } + + msg := err.Error() + + if errors.Is(err, context.DeadlineExceeded) { + return &Error{ + Code: CodeTimeout, + Message: msg, + Exit: 5, + err: err, + Hint: "increase --timeout, or start the task with --async and follow it later", + } + } + + var a2aErr *a2a.Error + if errors.As(err, &a2aErr) { + return &Error{ + Code: CodeProtocol, + Message: msg, + A2ACode: a2a.ErrorReason(a2aErr.Err), + Exit: 1, + err: err, + } + } + + if isIOError(err) { + return &Error{ + Code: CodeIO, + Message: msg, + Exit: 3, + Hint: "check connectivity or whether the agent is running", + err: err, + } + } + + return &Error{Code: CodeInternal, Message: msg, Exit: 1, err: err} +} + +// isIOError reports whether err is a transport-level failure — a network error, +// a broken stream, or a reset/closed connection — identified by type and +// sentinel rather than by matching message text. +func isIOError(err error) bool { + var netErr net.Error + if errors.As(err, &netErr) { + return true + } + return errors.Is(err, io.ErrUnexpectedEOF) || + errors.Is(err, io.ErrClosedPipe) || + errors.Is(err, syscall.ECONNREFUSED) || + errors.Is(err, syscall.ECONNRESET) || + errors.Is(err, syscall.EPIPE) +} diff --git a/internal/clierr/clierr_test.go b/internal/clierr/clierr_test.go new file mode 100644 index 0000000..a349116 --- /dev/null +++ b/internal/clierr/clierr_test.go @@ -0,0 +1,166 @@ +// Copyright 2026 The A2A Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package clierr + +import ( + "context" + "errors" + "fmt" + "net" + "syscall" + "testing" + + "github.com/a2aproject/a2a-go/v2/a2a" +) + +func TestClassify(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + wantCode string + wantA2A string + wantExit int + }{ + { + name: "already-classified usage error passes through", + err: Usage("bad flag combo"), + wantCode: CodeUsage, + wantExit: 2, + }, + { + name: "already-classified card error passes through", + err: CardResolution(errors.New("not found")), + wantCode: CodeCardInvalid, + wantExit: 4, + }, + { + name: "deadline exceeded is a timeout", + err: fmt.Errorf("waiting: %w", context.DeadlineExceeded), + wantCode: CodeTimeout, + wantExit: 5, + }, + { + name: "protocol error carries the A2A reason in a2aCode, exit 1", + err: fmt.Errorf("failed to get task x: %w", a2a.NewError(a2a.ErrTaskNotFound, "task not found")), + wantCode: CodeProtocol, + wantA2A: "TASK_NOT_FOUND", + wantExit: 1, + }, + { + name: "auth is a protocol error, not a special case", + err: fmt.Errorf("send: %w", a2a.NewError(a2a.ErrUnauthenticated, "unauthenticated")), + wantCode: CodeProtocol, + wantA2A: "UNAUTHENTICATED", + wantExit: 1, + }, + { + name: "net error is io, exit 3", + err: fmt.Errorf("resolving agent card: %w", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}), + wantCode: CodeIO, + wantExit: 3, + }, + { + name: "connection reset sentinel is io, exit 3", + err: fmt.Errorf("reading stream: %w", syscall.ECONNRESET), + wantCode: CodeIO, + wantExit: 3, + }, + { + name: "unclassified error is internal, exit 1", + err: errors.New("something odd happened"), + wantCode: CodeInternal, + wantExit: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := Classify(tt.err) + if got == nil { + t.Fatalf("Classify(%v) = nil, want a classified error", tt.err) + } + if got.Code != tt.wantCode { + t.Errorf("Classify(%v).Code = %q, want %q", tt.err, got.Code, tt.wantCode) + } + if got.A2ACode != tt.wantA2A { + t.Errorf("Classify(%v).A2ACode = %q, want %q", tt.err, got.A2ACode, tt.wantA2A) + } + if got.Exit != tt.wantExit { + t.Errorf("Classify(%v).Exit = %d, want %d", tt.err, got.Exit, tt.wantExit) + } + }) + } +} + +func TestClassifyNil(t *testing.T) { + t.Parallel() + if got := Classify(nil); got != nil { + t.Fatalf("Classify(nil) = %v, want nil", got) + } +} + +func TestUsage(t *testing.T) { + t.Parallel() + got := Usage("bad flags") + if got.Code != CodeUsage { + t.Errorf("Usage().Code = %q, want %q", got.Code, CodeUsage) + } + if got.Exit != 2 { + t.Errorf("Usage().Exit = %d, want 2", got.Exit) + } +} + +func TestCardResolution(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + wantCode string + wantExit int + }{ + { + name: "transport failure is io", + err: fmt.Errorf("card request failed: %w", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}), + wantCode: CodeIO, + wantExit: 3, + }, + { + name: "non-transport failure is card-invalid", + err: errors.New("card request failed, status: 404 Not Found"), + wantCode: CodeCardInvalid, + wantExit: 4, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := CardResolution(tt.err) + if got.Code != tt.wantCode { + t.Errorf("CardResolution(%v).Code = %q, want %q", tt.err, got.Code, tt.wantCode) + } + if got.Exit != tt.wantExit { + t.Errorf("CardResolution(%v).Exit = %d, want %d", tt.err, got.Exit, tt.wantExit) + } + if !errors.Is(got, tt.err) { + t.Errorf("CardResolution(%v) does not wrap the cause", tt.err) + } + }) + } +} diff --git a/internal/flagparse/urlorpath.go b/internal/flagparse/urlorpath.go index e6a683c..f4f3a13 100644 --- a/internal/flagparse/urlorpath.go +++ b/internal/flagparse/urlorpath.go @@ -15,7 +15,9 @@ package flagparse import ( + "fmt" "net" + "net/url" "os" "path/filepath" "strings" @@ -44,8 +46,42 @@ func (u *URLOrPath) Type() string { return "host|url|path" } func (u *URLOrPath) IsSet() bool { return u.raw != "" } // URL returns the reference normalized to a URL with an explicit scheme. -func (u *URLOrPath) URL() string { - ref := u.raw +func (u *URLOrPath) URL() string { return normalize(u.raw) } + +// Validate reports whether the reference is well-formed: a local file path must +// exist, and a host/URL form must parse to a URL with a host. +func (u *URLOrPath) Validate() error { + if u.raw == "" { + return nil + } + parsed, err := url.Parse(normalize(u.raw)) + if err != nil { + return fmt.Errorf("invalid agent card reference %q: %w", u.raw, err) + } + if parsed.Scheme == "file" { + path := parsed.Path + if path == "" { + path = parsed.Opaque + } + if path == "" { + return nil + } + return statCardFile(u.raw, path) + } + if parsed.Host == "" { + return fmt.Errorf("invalid agent card reference %q: missing host", u.raw) + } + return nil +} + +func statCardFile(ref, path string) error { + if _, err := os.Stat(path); err != nil { + return fmt.Errorf("agent card file %q: %w", ref, err) + } + return nil +} + +func normalize(ref string) string { if ref == "" || strings.Contains(ref, "://") { return ref } @@ -63,14 +99,21 @@ func (u *URLOrPath) URL() string { } func maybeFilePath(ref string) bool { + if hasFilePrefix(ref) { + return true + } + if _, err := os.Stat(ref); err == nil { + return true + } + return false +} + +func hasFilePrefix(ref string) bool { for _, prefix := range []string{"/", "./", "../"} { if strings.HasPrefix(ref, prefix) { return true } } - if _, err := os.Stat(ref); err == nil { - return true - } return false } diff --git a/internal/flagparse/urlorpath_test.go b/internal/flagparse/urlorpath_test.go index 9ce355c..0c7cd9f 100644 --- a/internal/flagparse/urlorpath_test.go +++ b/internal/flagparse/urlorpath_test.go @@ -15,6 +15,7 @@ package flagparse import ( + "errors" "os" "path/filepath" "testing" @@ -63,6 +64,51 @@ func TestURLOrPathURL(t *testing.T) { } } +func TestURLOrPathValidate(t *testing.T) { + t.Parallel() + + existing := filepath.Join(t.TempDir(), "card.json") + if err := os.WriteFile(existing, []byte("{}"), 0o600); err != nil { + t.Fatalf("os.WriteFile() error = %v", err) + } + + tests := []struct { + name string + ref string + wantErr bool + wantErrIs error + }{ + {name: "empty is valid", ref: ""}, + {name: "bare host is valid", ref: "agent.example"}, + {name: "loopback host:port is valid", ref: "localhost:9000"}, + {name: "full https url is valid", ref: "https://agent.example/card.json"}, + {name: "existing absolute path is valid", ref: existing}, + {name: "existing file url is valid", ref: "file://" + existing}, + {name: "missing absolute path is a usage error", ref: "/no/such/card.json", wantErr: true, wantErrIs: os.ErrNotExist}, + {name: "missing relative path is a usage error", ref: "./no-such-card.json", wantErr: true, wantErrIs: os.ErrNotExist}, + {name: "missing file url is a usage error", ref: "file:///no/such/card.json", wantErr: true, wantErrIs: os.ErrNotExist}, + {name: "malformed url is a usage error", ref: "http://exa mple.com", wantErr: true}, + {name: "url without host is a usage error", ref: "https://", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var u URLOrPath + if err := u.Set(tt.ref); err != nil { + t.Fatalf("URLOrPath.Set(%q) error = %v", tt.ref, err) + } + err := u.Validate() + if tt.wantErr != (err != nil) { + t.Fatalf("URLOrPath.Validate() error = %v, want error = %v", err, tt.wantErr) + } + if tt.wantErrIs != nil && !errors.Is(err, tt.wantErrIs) { + t.Errorf("URLOrPath.Validate() error = %v, want errors.Is %v", err, tt.wantErrIs) + } + }) + } +} + func TestURLOrPathIsSet(t *testing.T) { t.Parallel() From 837ebbe5e9d50d891bfd4c99a8be6938fd7d7839 Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Tue, 8 Sep 2026 12:16:00 +0000 Subject: [PATCH 2/2] fix import --- internal/flagparse/svcparams.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/flagparse/svcparams.go b/internal/flagparse/svcparams.go index 2d99519..6c18dc6 100644 --- a/internal/flagparse/svcparams.go +++ b/internal/flagparse/svcparams.go @@ -16,6 +16,7 @@ package flagparse import ( "fmt" + "strings" "github.com/spf13/pflag"