diff --git a/agent/example/example.go b/agent/example/example.go index 2fa171493..0b5606fb1 100644 --- a/agent/example/example.go +++ b/agent/example/example.go @@ -7,8 +7,8 @@ // // 1. A copy-start point for new integrators — copy the whole package and rename // it; every key decision point carries a teaching comment from the -// "integrator's perspective" (how to fill registration fields, why typed -// errors are required, how to make capability-matrix trade-offs); +// "integrator's perspective" (how to fill registration fields, which +// capabilities to wire, how to make capability trade-offs); // 2. The command tree's offline demo backend — the full agent // list/card/send/task/context chain runs for real without any platform // configuration; @@ -17,9 +17,14 @@ // Minimal checklist for onboarding a new provider (each item is demonstrated in // this package): // - register metadata via agent.Register in init() (see the per-field comments below); -// - implement the 9 methods of agent.Provider (internal/agent/provider.go); -// - a catalog type (KindCatalog) must implement agent.Discoverer (asserted at registration time); -// - add a blank import in cmd/agent/agent.go to trigger init registration; +// - construct a *agent.Provider in the Factory, wiring one func field per +// capability you support — the core Send/GetTask are mandatory, every other +// field is optional and "not wired = not supported" (the framework returns a +// unified unsupported_capability error and derives the card matrix from what +// is wired, so there is no bool matrix to keep in sync and no capability- +// refusal code to write); +// - a catalog type (KindCatalog) must wire ListAgents (asserted at registration); +// - add a blank import under agent/register.go to trigger init registration; // - run agenttest.RunConformance in tests to lock down implicit contracts. package example @@ -37,74 +42,33 @@ import ( const scheme = "example" // catalog is the full agent set known at registration time. The catalog -// boilerplate (enumeration / per-agent rich Card / typed error for unknown ids) -// is entirely handled by the framework's StaticCatalog; the integrator only -// declares the data. -// -// Teaching focus — the capability matrix must be "declared honestly": -// - The two agents' matrices differ on purpose: echo is the minimal set (no -// artifact, no file input, not cancelable), reporter has everything on. The -// AI decides its next command from the card, so whatever is declared must be -// delivered. -// - The asymmetry is directional: declaring something you "can't do" as true -// leads the AI into a dead end (e.g. declaring input_required true while the -// task never stops in that state, so the AI waits forever for a state that -// never appears); conversely, declaring an "unused" capability as true when -// the state never appears (like reporter's input_required) merely means the -// AI never uses it and does not go wrong. Only the former warrants being -// conservatively false. -// - task_cancel being true for one and false for the other is not arbitrary: -// echo=false demonstrates the command layer's card gating -// (cmd/agent/task.go intercepts and reports unsupported_capability before -// sending any request), while reporter=true demonstrates actually reaching -// the provider's CancelTask. +// boilerplate (enumeration / per-agent Card metadata / typed error for unknown +// ids) is handled by the framework's StaticCatalog; the integrator only declares +// the descriptive data. Capabilities are NOT declared here — see newProvider, +// where each agent's supported capabilities are expressed by which Provider func +// fields the Factory wires. var catalog = agent.NewStaticCatalog(scheme, []agent.CatalogEntry{ { ID: "echo", Name: "复读机", Description: "把你发的话原样复读一遍(同一会话续发时带轮次,证明上下文记忆)。最小能力集示范。", - Capabilities: agent.Capabilities{ - TaskGet: true, - TaskList: true, - // echo is not cancelable: once the command layer reads false it - // rejects task cancel outright, so the provider is never even called - // (gate-before-network). - TaskCancel: false, - // The mock task completes instantly and never stops in - // input_required; echo honestly declares false per the minimal set. - InputRequired: false, - // echo takes no files and produces no artifact — Send returns - // agent.ErrUnsupported for --file, consistent with the false here - // (declaration and behavior single-sourced). - FileInput: false, - ArtifactDownload: false, - MultiTurn: true, - }, }, { ID: "reporter", Name: "报表生成器", Description: "对任意请求产出一份内联 CSV 报表 artifact,示范 artifact 下载与任务取消链路。", - Capabilities: agent.Capabilities{ - TaskGet: true, - TaskList: true, - TaskCancel: true, - InputRequired: true, - FileInput: true, - ArtifactDownload: true, - MultiTurn: true, - }, }, }) func init() { // Registration contract (internal/agent/registry.go): everything except - // RequiredScopes is required; missing / invalid values panic. At - // registration time it also constructs a Provider once via a zero-value Deps - // probe — so Factory must accept zero-value Deps and have no side effects - // during construction (no network, no disk). + // RequiredScopes is required; missing / invalid values panic. At registration + // time it also constructs a Provider once via a zero-value Deps probe — so the + // Factory must accept zero-value Deps and an empty agentID, have no side + // effects during construction (no network, no disk), and wire the mandatory + // core fields (Send/GetTask) plus, for a catalog type, ListAgents. agent.Register(scheme, agent.ProviderInfo{ - Factory: newExampleProvider, + Factory: newProvider, // Label: the user-facing provider name (the LABEL column in agent list). Label: "Example 演示 agent(内存 mock,零网络)", // AgentRefFormat: the written format of agent_ref, must start with ":" (validated at registration). @@ -113,9 +77,8 @@ func init() { // information for AI-guided onboarding, referenced by the unknown-id hint // and the not-discoverable list hint. AgentIDSource: "运行 lark-cli agent list example 查看内置演示 agent 及其 agent_ref(无需任何平台配置)", - // Kind: catalog type. Registration asserts the instance implements - // Discoverer, so `agent list example` can enumerate (an instance-type - // provider that does not support enumeration returns unsupported_capability). + // Kind: catalog type. Registration asserts the provider wires ListAgents, + // so `agent list example` can enumerate. Kind: agent.KindCatalog, // RequiredScopes: the full set of scopes this provider's real API calls // need. example has zero network and calls no OAPI, so it is empty — @@ -134,45 +97,75 @@ func init() { }) } -// exampleProvider addresses one agent in the catalog. agentID may be empty — the -// Discoverer path (agent list example) constructs an instance without an id only -// to enumerate the catalog. -type exampleProvider struct { +// state addresses one agent in the catalog. agentID may be empty — the +// enumeration path (agent list example) and the registration probe construct a +// state without an id. +type state struct { deps agent.Deps agentID string } -// newExampleProvider is the registered Factory. Teaching point: it does pure -// assignment only. -// - It does not validate agentID: an unknown id is rejected by catalog.Lookup -// inside "the verbs that actually use the id" (the empty-id enumeration -// instance must construct successfully); -// - It does not touch deps: the mock has no use for Client/As; a real provider -// uses deps.Client to send requests and deps.As to pick the identity in its -// methods, but construction must still have no side effects (the zero-value -// Deps probe contract). -func newExampleProvider(deps agent.Deps, agentID string) (agent.Provider, error) { - return &exampleProvider{deps: deps, agentID: agentID}, nil +// newProvider is the registered Factory. It assembles a *agent.Provider by +// wiring the func fields for the capabilities this agent supports. +// +// Teaching focus — capability is expressed as wiring, per agent: +// - Core Send/GetTask are wired unconditionally (mandatory). +// - The always-on optionals (ListTasks, the context trio, ListAgents, Describe) +// are wired for every agent. +// - reporter additionally wires CancelTask + DownloadArtifact and sets +// FileInput — echo does not, so echo's card honestly shows task_cancel / +// artifact_download / file_input = false. There is no bool matrix: the card +// is derived from exactly these fields (internal/agent/card.go DeriveCapabilities). +// - A capability you do not wire needs zero refusal code: the command layer +// gates on the nil field and returns unified unsupported_capability before +// any provider method runs. +// +// Teaching point — the Factory does pure assignment only: it does not validate +// agentID (an unknown id is rejected by catalog.Lookup inside the verbs that use +// it, and by Describe on the card path; the empty-id probe/enumeration instance +// must construct successfully) and does not touch deps (the mock has no use for +// Client/As, but construction must have no side effects either way — the +// zero-value Deps probe contract). +func newProvider(deps agent.Deps, agentID string) (*agent.Provider, error) { + s := &state{deps: deps, agentID: agentID} + p := &agent.Provider{ + Send: s.send, + GetTask: s.getTask, + ListTasks: s.listTasks, + ListContexts: s.listContexts, + GetContext: s.getContext, + DeleteContext: s.deleteContext, + ListAgents: s.listAgents, + Describe: s.describe, + } + // Per-agent capability: reporter can be canceled and produces a downloadable + // artifact, accepts file input, and may pause a task in input_required; echo + // (minimal set) does none of these, so those fields stay nil/false and the + // framework reports them unsupported. + if agentID == "reporter" { + p.CancelTask = s.cancelTask + p.DownloadArtifact = s.downloadArtifact + p.FileInput = true + p.InputRequired = true + } + return p, nil } -// Card returns the agent's capability card. Synthesis is fully offline: NewCard -// fills in registration-time fields (Provider/Label/Identity/AgentIDSource/empty -// Parameters), and the catalog entry adds Name/Description/Capabilities. An -// unknown id returns a typed validation error from StaticCatalog.Lookup -// (invalid_argument/exit 2, hint pointing to agent list example) — the -// integrator need not build its own unknown-agent error. -func (p *exampleProvider) Card(ctx context.Context) (*agent.AgentCard, error) { - return catalog.Card(p.agentID) +// describe supplies the per-agent Card metadata and validates the agent_id +// (StaticCatalog.Describe returns a typed unknown-id error). Capabilities are +// derived by the framework from the wired fields, so Describe never touches them. +func (s *state) describe(ctx context.Context) (*agent.CardInfo, error) { + return catalog.Describe(s.agentID) } -// ListAgents enumerates the catalog (Discoverer): `agent list example` goes here. -func (p *exampleProvider) ListAgents(ctx context.Context) ([]agent.AgentSummary, error) { +// listAgents enumerates the catalog: `agent list example` goes here. +func (s *state) listAgents(ctx context.Context) ([]agent.AgentSummary, error) { return catalog.ListAgents(ctx) } -// Send sends one message: the first turn generates a context_id to start a new +// send sends one message: the first turn generates a context_id to start a new // conversation, and --context-id continues within the same conversation. The -// mock task has no async execution body, so Send immediately returns in the +// mock task has no async execution body, so send immediately returns in the // completed terminal state — the command layer's meta.next therefore directly // gives the terminal-state suggestion "view task detail and artifacts" rather // than a polling command. @@ -180,26 +173,18 @@ func (p *exampleProvider) ListAgents(ctx context.Context) ([]agent.AgentSummary, // Teaching point (IsTerminal): IsTerminal is filled in here for convenience, but // leaving it out would be fine — the command layer's normalizeTask always // re-derives this field from State (single source), so a provider filling it in -// wrong does not affect the watch exit code. The integrator need not worry about it. -func (p *exampleProvider) Send(ctx context.Context, in agent.SendInput) (*agent.AgentTask, error) { - entry, err := catalog.Lookup(p.agentID) +// wrong does not affect the watch exit code. +func (s *state) send(ctx context.Context, in agent.SendInput) (*agent.AgentTask, error) { + entry, err := catalog.Lookup(s.agentID) if err != nil { return nil, err } - // Capability declaration and behavior are single-sourced: if the card says it - // takes no files, the behavior must reject them. The command layer does not - // card-gate --file (only cancel is gated), so the provider returns the - // agent.ErrUnsupported sentinel, and the command layer's convertUnsupported - // turns it into a typed unsupported_capability (exit 2) — the full - // sentinel→typed chain. - if len(in.Files) > 0 && !entry.Capabilities.FileInput { - return nil, fmt.Errorf("example:%s 不接收文件输入: %w", p.agentID, agent.ErrUnsupported) - } // The mock task is instantly terminal, so there is no "feed input to a running // task" scenario. Continuing via --task-id returns failed_precondition: the // request itself is valid but the target resource's state does not satisfy it // — reading this subtype, the AI knows to "try a different way" (start a new - // task) rather than retry as-is. + // task) rather than retry as-is. (This is a genuine runtime precondition, not + // a capability gate — hence a typed error here, not an unwired field.) if in.TaskID != "" { return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "example 的任务发出即完成(终态),无法向已有任务续发"). @@ -211,7 +196,7 @@ func (p *exampleProvider) Send(ctx context.Context, in agent.SendInput) (*agent. if ctxID == "" { // First turn: generate a context_id (the anchor for the multi-turn // context; later sends use it to continue the conversation). - ctxID, err = store.createContext(p.agentID, truncateTitle(in.Text)) + ctxID, err = store.createContext(s.agentID, truncateTitle(in.Text)) if err != nil { return nil, err } @@ -221,7 +206,7 @@ func (p *exampleProvider) Send(ctx context.Context, in agent.SendInput) (*agent. // cross-agent context id is rejected inside with a typed validation error), // computes the round, and inserts atomically; the build callback only // assembles the task body according to the round. - task, err := store.createTask(p.agentID, ctxID, func(round int) agent.AgentTask { + task, err := store.createTask(s.agentID, ctxID, func(round int) agent.AgentTask { var reply string switch entry.ID { case "echo": @@ -263,78 +248,71 @@ func (p *exampleProvider) Send(ctx context.Context, in agent.SendInput) (*agent. return &task, nil } -// GetTask queries a single task's state and artifacts (reads the in-memory state machine). -func (p *exampleProvider) GetTask(ctx context.Context, taskID string) (*agent.AgentTask, error) { - if _, err := catalog.Lookup(p.agentID); err != nil { +// getTask queries a single task's state and artifacts (reads the in-memory state machine). +func (s *state) getTask(ctx context.Context, taskID string) (*agent.AgentTask, error) { + if _, err := catalog.Lookup(s.agentID); err != nil { return nil, err } - task, err := store.getTask(p.agentID, taskID) + task, err := store.getTask(s.agentID, taskID) if err != nil { return nil, err } return &task, nil } -// ListTasks lists tasks, optionally filtered by contextID (empty string means no filter). -func (p *exampleProvider) ListTasks(ctx context.Context, contextID string) ([]agent.TaskSummary, error) { - if _, err := catalog.Lookup(p.agentID); err != nil { +// listTasks lists tasks, optionally filtered by contextID (empty string means no filter). +func (s *state) listTasks(ctx context.Context, contextID string) ([]agent.TaskSummary, error) { + if _, err := catalog.Lookup(s.agentID); err != nil { return nil, err } - return store.listTasks(p.agentID, contextID), nil + return store.listTasks(s.agentID, contextID), nil } -// CancelTask cancels a task. Both paths are demonstrated here: -// - echo (task_cancel=false): returns the agent.ErrUnsupported sentinel. Via -// the CLI this is unreachable (cmd/agent/task.go's card gating comes first), -// but tests calling the SPI directly / future callers that bypass the gate -// still get the correct semantics — belt-and-braces. -// - reporter (task_cancel=true): actually dispatched. But the mock task is -// completed the moment it is sent, so canceling a terminal task returns a -// failed_precondition typed error (state not satisfied, exit 2) rather than -// pretending success — honest error semantics matter as much as honest -// capability declaration. -func (p *exampleProvider) CancelTask(ctx context.Context, taskID string) error { - entry, err := catalog.Lookup(p.agentID) - if err != nil { +// cancelTask cancels a task. It is wired only for reporter (task_cancel=true), so +// echo never reaches it — the command layer gates echo's cancel on the nil field +// and returns unsupported_capability before any provider code runs. The mock +// task is completed the moment it is sent, so canceling a terminal task returns a +// failed_precondition typed error (state not satisfied, exit 2) rather than +// pretending success — honest error semantics matter as much as honest capability +// wiring. +func (s *state) cancelTask(ctx context.Context, taskID string) error { + if _, err := catalog.Lookup(s.agentID); err != nil { return err } - if !entry.Capabilities.TaskCancel { - return agent.ErrUnsupported - } - task, err := store.getTask(p.agentID, taskID) + task, err := store.getTask(s.agentID, taskID) if err != nil { return err } if task.State.IsTerminal() { return errs.NewValidationError(errs.SubtypeFailedPrecondition, "任务 '%s' 已处于终态 %s,无法取消", taskID, task.State). - WithHint("终态任务不可取消;用 lark-cli agent task get example:%s %s 查看结果", p.agentID, taskID) + WithHint("终态任务不可取消;用 lark-cli agent task get example:%s %s 查看结果", s.agentID, taskID) } return store.setTaskState(taskID, agent.StateCanceled) } -// ListContexts lists multi-turn contexts. -func (p *exampleProvider) ListContexts(ctx context.Context) ([]agent.ContextSummary, error) { - if _, err := catalog.Lookup(p.agentID); err != nil { +// listContexts lists multi-turn contexts. +func (s *state) listContexts(ctx context.Context) ([]agent.ContextSummary, error) { + if _, err := catalog.Lookup(s.agentID); err != nil { return nil, err } - return store.listContexts(p.agentID), nil + return store.listContexts(s.agentID), nil } -// GetContext returns a single context's detail (including its task list). -func (p *exampleProvider) GetContext(ctx context.Context, ctxID string) (*agent.ContextDetail, error) { - if _, err := catalog.Lookup(p.agentID); err != nil { +// getContext returns a single context's detail (including its task list). +func (s *state) getContext(ctx context.Context, ctxID string) (*agent.ContextDetail, error) { + if _, err := catalog.Lookup(s.agentID); err != nil { return nil, err } - return store.getContext(p.agentID, ctxID) + return store.getContext(s.agentID, ctxID) } -// DeleteContext deletes a context (a destructive operation; the --yes gate is in the command layer). -func (p *exampleProvider) DeleteContext(ctx context.Context, ctxID string) error { - if _, err := catalog.Lookup(p.agentID); err != nil { +// deleteContext deletes a context (a destructive operation; the --yes gate is in the command layer). +func (s *state) deleteContext(ctx context.Context, ctxID string) error { + if _, err := catalog.Lookup(s.agentID); err != nil { return err } - return store.deleteContext(p.agentID, ctxID) + return store.deleteContext(s.agentID, ctxID) } // reportCSV is the fixed content of the reporter artifact (inline text, demonstrating a Bytes-type artifact). @@ -342,26 +320,22 @@ const reportCSV = "quarter,revenue,cost,margin\n" + "2026Q1,1250,830,0.336\n" + "2026Q2,1410,905,0.358\n" -// DownloadArtifact fetches artifact data. example uses the inline Bytes type -// (the command layer writes it to disk directly); the URL type (a real -// provider's signed URL) fills the URL field, and SSRF validation plus the -// download are handled uniformly by the command layer. +// downloadArtifact fetches artifact data. It is wired only for reporter +// (artifact_download=true); echo never reaches it (gated on the nil field). +// example uses the inline Bytes type (the command layer writes it to disk +// directly); the URL type (a real provider's signed URL) fills the URL field, and +// SSRF validation plus the download are handled uniformly by the command layer. // // Teaching point (suggested_name): ArtifactData.Name is the "server-suggested // file name", echoed back only as a suggested_name for the caller to reference // when choosing -o — it is untrusted input and must never participate in // constructing the local save path (the contract.go rule; the save path is // always determined by -o/SafeOutputPath). -func (p *exampleProvider) DownloadArtifact(ctx context.Context, taskID, artifactID string) (*agent.ArtifactData, error) { - entry, err := catalog.Lookup(p.agentID) - if err != nil { +func (s *state) downloadArtifact(ctx context.Context, taskID, artifactID string) (*agent.ArtifactData, error) { + if _, err := catalog.Lookup(s.agentID); err != nil { return nil, err } - if !entry.Capabilities.ArtifactDownload { - // Keep behavior consistent with the card's artifact_download=false (the sentinel→typed chain). - return nil, fmt.Errorf("example:%s 不产出可下载产物: %w", p.agentID, agent.ErrUnsupported) - } - task, err := store.getTask(p.agentID, taskID) + task, err := store.getTask(s.agentID, taskID) if err != nil { return nil, err } @@ -376,7 +350,7 @@ func (p *exampleProvider) DownloadArtifact(ctx context.Context, taskID, artifact } return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "任务 '%s' 名下没有产物 '%s'", taskID, artifactID). - WithHint("运行 lark-cli agent task get example:%s %s 查看该任务的 artifacts", p.agentID, taskID) + WithHint("运行 lark-cli agent task get example:%s %s 查看该任务的 artifacts", s.agentID, taskID) } // truncateTitle takes the first few characters of the message as the diff --git a/agent/example/example_test.go b/agent/example/example_test.go index 8954290d3..8486dd24b 100644 --- a/agent/example/example_test.go +++ b/agent/example/example_test.go @@ -5,7 +5,6 @@ package example import ( "context" - "errors" "path/filepath" "strings" "testing" @@ -24,12 +23,12 @@ func swapStore(t *testing.T) { t.Cleanup(func() { store = old }) } -// newProvider builds an example provider with zero-value Deps (the mock never needs a Client). -func newProvider(t *testing.T, agentID string) agent.Provider { +// buildProvider builds an example *Provider with zero-value Deps (the mock never needs a Client). +func buildProvider(t *testing.T, agentID string) *agent.Provider { t.Helper() - p, err := newExampleProvider(agent.Deps{}, agentID) + p, err := newProvider(agent.Deps{}, agentID) if err != nil { - t.Fatalf("newExampleProvider: %v", err) + t.Fatalf("newProvider: %v", err) } return p } @@ -50,15 +49,11 @@ func TestConformanceReporter(t *testing.T) { // capability matrices (the core of the teaching demo: honest capability declaration // plus task_cancel true for one and false for the other). func TestCapabilityMatrixDiverges(t *testing.T) { - echoCard, err := newProvider(t, "echo").Card(context.Background()) - if err != nil { - t.Fatal(err) - } - reporterCard, err := newProvider(t, "reporter").Card(context.Background()) - if err != nil { - t.Fatal(err) - } - ec, rc := echoCard.Capabilities, reporterCard.Capabilities + // The card matrix is derived from which Provider fields the Factory wires per + // agent, so DeriveCapabilities over the two constructed providers is the + // single source under test. + ec := agent.DeriveCapabilities(buildProvider(t, "echo")) + rc := agent.DeriveCapabilities(buildProvider(t, "reporter")) if ec.ArtifactDownload || ec.FileInput || ec.TaskCancel { t.Errorf("echo should be the minimal capability set (no artifact/file/cancel), got %+v", ec) } @@ -75,7 +70,7 @@ func TestCapabilityMatrixDiverges(t *testing.T) { // echoes with a turn marker. func TestEchoMultiTurn(t *testing.T) { swapStore(t) - p := newProvider(t, "echo") + p := buildProvider(t, "echo") ctx := context.Background() t1, err := p.Send(ctx, agent.SendInput{Text: "hello"}) @@ -139,7 +134,7 @@ func TestEchoMultiTurn(t *testing.T) { // is still queryable -- the offline demo chain depends on this. func TestStateSurvivesReload(t *testing.T) { swapStore(t) - p := newProvider(t, "echo") + p := buildProvider(t, "echo") task, err := p.Send(context.Background(), agent.SendInput{Text: "persist"}) if err != nil { t.Fatal(err) @@ -159,7 +154,7 @@ func TestStateSurvivesReload(t *testing.T) { // and DownloadArtifact returns inline Bytes + suggested_name. func TestReporterArtifactFlow(t *testing.T) { swapStore(t) - p := newProvider(t, "reporter") + p := buildProvider(t, "reporter") ctx := context.Background() task, err := p.Send(ctx, agent.SendInput{Text: "本季度报表"}) @@ -196,22 +191,20 @@ func TestReporterArtifactFlow(t *testing.T) { } } -// TestEchoCancelSentinel verifies the sentinel chain: echo (task_cancel=false) -// returns agent.ErrUnsupported from CancelTask, which the command layer's -// convertUnsupported recognizes via errors.Is. The same holds for echo's -// DownloadArtifact and a Send with files. -func TestEchoCancelSentinel(t *testing.T) { - swapStore(t) - p := newProvider(t, "echo") - ctx := context.Background() - if err := p.CancelTask(ctx, "task_whatever"); !errors.Is(err, agent.ErrUnsupported) { - t.Fatalf("echo CancelTask should return the ErrUnsupported sentinel, got %v", err) +// TestEchoUnwiredCapabilities verifies the new capability model: echo (the +// minimal set) simply leaves CancelTask / DownloadArtifact unwired and FileInput +// false. There is no capability-refusal code — the command layer gates on the +// nil fields and returns unsupported_capability before any provider method runs. +func TestEchoUnwiredCapabilities(t *testing.T) { + p := buildProvider(t, "echo") + if p.CancelTask != nil { + t.Error("echo should not wire CancelTask (task_cancel=false)") } - if _, err := p.DownloadArtifact(ctx, "task_x", "art_x"); !errors.Is(err, agent.ErrUnsupported) { - t.Fatalf("echo DownloadArtifact should return the ErrUnsupported sentinel, got %v", err) + if p.DownloadArtifact != nil { + t.Error("echo should not wire DownloadArtifact (artifact_download=false)") } - if _, err := p.Send(ctx, agent.SendInput{Text: "hi", Files: []string{"a.txt"}}); !errors.Is(err, agent.ErrUnsupported) { - t.Fatalf("echo Send with --file should return the ErrUnsupported sentinel, got %v", err) + if p.FileInput { + t.Error("echo should not accept file input (file_input=false)") } } @@ -220,7 +213,7 @@ func TestEchoCancelSentinel(t *testing.T) { // as soon as it is sent). func TestReporterCancelTerminal(t *testing.T) { swapStore(t) - p := newProvider(t, "reporter") + p := buildProvider(t, "reporter") ctx := context.Background() task, err := p.Send(ctx, agent.SendInput{Text: "报表"}) if err != nil { @@ -230,9 +223,6 @@ func TestReporterCancelTerminal(t *testing.T) { if err == nil { t.Fatal("canceling a terminal task should return an error") } - if errors.Is(err, agent.ErrUnsupported) { - t.Fatal("reporter supports cancel and should not return ErrUnsupported") - } prob, ok := errs.ProblemOf(err) if !ok { t.Fatalf("terminal cancel should be a typed error, got %T: %v", err, err) @@ -246,10 +236,10 @@ func TestReporterCancelTerminal(t *testing.T) { // typed error (invalid_argument, with a hint pointing to agent list example). func TestUnknownCatalogID(t *testing.T) { swapStore(t) - p := newProvider(t, "nonexistent") + p := buildProvider(t, "nonexistent") ctx := context.Background() - if _, err := p.Card(ctx); err == nil { - t.Fatal("Card with an unknown catalog id should return an error") + if _, err := agent.BuildCard(ctx, scheme, "nonexistent", p); err == nil { + t.Fatal("BuildCard with an unknown catalog id should return an error (Describe validates the id)") } _, err := p.Send(ctx, agent.SendInput{Text: "hi"}) if err == nil { @@ -265,7 +255,7 @@ func TestUnknownCatalogID(t *testing.T) { // semantics) and an unknown context id. func TestSendGuards(t *testing.T) { swapStore(t) - p := newProvider(t, "echo") + p := buildProvider(t, "echo") ctx := context.Background() _, err := p.Send(ctx, agent.SendInput{Text: "hi", ContextID: "ctx_x", TaskID: "task_x"}) @@ -282,7 +272,7 @@ func TestSendGuards(t *testing.T) { // TestDeleteContext verifies deleting a context also cleans up the tasks under it. func TestDeleteContext(t *testing.T) { swapStore(t) - p := newProvider(t, "echo") + p := buildProvider(t, "echo") ctx := context.Background() task, err := p.Send(ctx, agent.SendInput{Text: "bye"}) if err != nil { diff --git a/cmd/agent/card.go b/cmd/agent/card.go index 15b521516..bc483b6c4 100644 --- a/cmd/agent/card.go +++ b/cmd/agent/card.go @@ -71,7 +71,11 @@ func agentCardRun(opts *cardOptions) error { if err != nil { return err } - card, err := p.Card(opts.Cmd.Context()) + r, err := iagent.ParseRef(opts.Ref) + if err != nil { + return wrapRefResolveError(err) + } + card, err := iagent.BuildCard(opts.Cmd.Context(), r.Scheme, r.AgentID, p) if err != nil { return err } diff --git a/cmd/agent/common.go b/cmd/agent/common.go index f8fdd671f..7a08487d4 100644 --- a/cmd/agent/common.go +++ b/cmd/agent/common.go @@ -50,7 +50,7 @@ var sleep = func(ctx context.Context, d time.Duration) bool { // (Card) may be called on it. A malformed ref or unknown provider scheme is // wrapped into a validation typed error (subtype invalid_argument, exit 2), so // those surface before (not behind) the config gate. -func resolveProviderNoClient(f *cmdutil.Factory, cmd *cobra.Command, ref, asStr string) (iagent.Provider, core.Identity, error) { +func resolveProviderNoClient(f *cmdutil.Factory, cmd *cobra.Command, ref, asStr string) (*iagent.Provider, core.Identity, error) { id := f.ResolveAs(cmd.Context(), cmd, core.Identity(asStr)) if err := f.CheckIdentity(id, supportedIdentities); err != nil { return nil, "", err @@ -91,7 +91,7 @@ func wrapRefResolveError(err error) error { // Wiring rule: every verb that calls the real API MUST run preflightScopesForRef // right after this succeeds and before the API call, so a new verb is // never silently exempt from the local scope preflight. -func resolveProvider(f *cmdutil.Factory, cmd *cobra.Command, ref, asStr string) (iagent.Provider, core.Identity, error) { +func resolveProvider(f *cmdutil.Factory, cmd *cobra.Command, ref, asStr string) (*iagent.Provider, core.Identity, error) { _, id, err := resolveProviderNoClient(f, cmd, ref, asStr) if err != nil { return nil, "", err @@ -240,24 +240,6 @@ func capabilityError(ref, capHuman, capKey string) error { ).WithHint("%s", cardHint(ref, "支持的能力")) } -// convertUnsupported converts the agent.ErrUnsupported sentinel a provider -// method may return (SPI contract, internal/agent/registry.go) into the typed -// unsupported_capability validation error (exit 2), mirroring capabilityError's -// wording: ref is the user-supplied agent_ref, action the human-facing verb -// (e.g. "task cancel"). Any other error — including an already-typed one and -// nil — passes through unchanged, so wrapping every provider call site is -// side-effect free. Without this wiring a bare sentinel would fall through to -// the internal-error fallback (exit 5) and break the documented semantics. -func convertUnsupported(ref, action string, err error) error { - if err == nil || !errors.Is(err, iagent.ErrUnsupported) { - return err - } - return errs.NewValidationError( - errs.SubtypeUnsupportedCapability, - "agent '%s' 不支持 '%s'(provider 返回能力不支持)", ref, action, - ).WithCause(err).WithHint("%s", cardHint(ref, "支持的能力")) -} - // normalizeTask derives the redundant IsTerminal flag from State — the single // source of truth — the moment a task enters the command layer, so a provider // that forgets (or mis-fills) the flag can never skew watch exit codes or an @@ -283,7 +265,7 @@ func normalizeTaskSummaries(ts []iagent.TaskSummary) []iagent.TaskSummary { // or ctx is done. A timeout is not a failure: it returns the most recent // task with a nil error, letting the caller print the current state (exit 0). A // provider GetTask error is surfaced. -func pollToStop(ctx context.Context, p iagent.Provider, taskID string) (*iagent.AgentTask, error) { +func pollToStop(ctx context.Context, p *iagent.Provider, taskID string) (*iagent.AgentTask, error) { const ( initialDelay = time.Second maxDelay = 5 * time.Second diff --git a/cmd/agent/common_test.go b/cmd/agent/common_test.go index 324efd025..8a0420c07 100644 --- a/cmd/agent/common_test.go +++ b/cmd/agent/common_test.go @@ -220,25 +220,30 @@ func TestSemanticExitError(t *testing.T) { } } -// fakePollProvider drives pollToStop through a scripted state sequence. +// fakePollProvider drives pollToStop through a scripted state sequence. It is +// not registered, so provider() only wires GetTask (the sole field pollToStop +// touches); calls/err stay observable on the struct after the poll. type fakePollProvider struct { - iagent.Provider states []iagent.TaskState calls int err error } -func (f *fakePollProvider) GetTask(ctx context.Context, taskID string) (*iagent.AgentTask, error) { - if f.err != nil { - return nil, f.err +func (f *fakePollProvider) provider() *iagent.Provider { + return &iagent.Provider{ + GetTask: func(ctx context.Context, taskID string) (*iagent.AgentTask, error) { + if f.err != nil { + return nil, f.err + } + i := f.calls + if i >= len(f.states) { + i = len(f.states) - 1 + } + f.calls++ + s := f.states[i] + return &iagent.AgentTask{TaskID: taskID, State: s, IsTerminal: s.IsTerminal()}, nil + }, } - i := f.calls - if i >= len(f.states) { - i = len(f.states) - 1 - } - f.calls++ - s := f.states[i] - return &iagent.AgentTask{TaskID: taskID, State: s, IsTerminal: s.IsTerminal()}, nil } // TestPollToStop_ReachesTerminal stops once a terminal state is observed. @@ -247,7 +252,7 @@ func TestPollToStop_ReachesTerminal(t *testing.T) { defer restore() p := &fakePollProvider{states: []iagent.TaskState{iagent.StateWorking, iagent.StateWorking, iagent.StateCompleted}} - task, err := pollToStop(context.Background(), p, "chat_1") + task, err := pollToStop(context.Background(), p.provider(), "chat_1") if err != nil { t.Fatalf("should not error: %v", err) } @@ -265,7 +270,7 @@ func TestPollToStop_StopsOnInputRequired(t *testing.T) { defer restore() p := &fakePollProvider{states: []iagent.TaskState{iagent.StateWorking, iagent.StateInputRequired}} - task, err := pollToStop(context.Background(), p, "chat_1") + task, err := pollToStop(context.Background(), p.provider(), "chat_1") if err != nil { t.Fatalf("should not error: %v", err) } @@ -283,7 +288,7 @@ func TestPollToStop_ContextTimeoutNotFailure(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // expire immediately p := &fakePollProvider{states: []iagent.TaskState{iagent.StateWorking}} - task, err := pollToStop(ctx, p, "chat_1") + task, err := pollToStop(ctx, p.provider(), "chat_1") if err != nil { t.Fatalf("timeout should not be treated as failure: %v", err) } @@ -298,7 +303,7 @@ func TestPollToStop_GetTaskError(t *testing.T) { defer restore() p := &fakePollProvider{states: []iagent.TaskState{iagent.StateWorking}, err: errors.New("boom")} - if _, err := pollToStop(context.Background(), p, "chat_1"); err == nil { + if _, err := pollToStop(context.Background(), p.provider(), "chat_1"); err == nil { t.Fatal("a GetTask error should propagate") } } @@ -349,7 +354,7 @@ func TestPollToStop_ClampsDelayToMax(t *testing.T) { iagent.StateWorking, iagent.StateWorking, iagent.StateWorking, iagent.StateWorking, iagent.StateWorking, iagent.StateCompleted, }} - task, err := pollToStop(context.Background(), p, "chat_1") + task, err := pollToStop(context.Background(), p.provider(), "chat_1") if err != nil { t.Fatalf("should not error: %v", err) } @@ -379,7 +384,7 @@ func TestPollToStop_SleepCanceledDuringBackoff(t *testing.T) { defer restore() p := &fakePollProvider{states: []iagent.TaskState{iagent.StateWorking, iagent.StateCompleted}} - task, err := pollToStop(context.Background(), p, "chat_1") + task, err := pollToStop(context.Background(), p.provider(), "chat_1") if err != nil { t.Fatalf("an interrupted sleep should not be treated as failure: %v", err) } diff --git a/cmd/agent/context.go b/cmd/agent/context.go index 0e9c32eca..a6b39c9f0 100644 --- a/cmd/agent/context.go +++ b/cmd/agent/context.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" + iagent "github.com/larksuite/cli/internal/agent" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/output" ) @@ -128,13 +129,18 @@ func agentContextListRun(opts *contextOptions) error { if err != nil { return err } + // Capability gate before the API call: multi_turn is derived from ListContexts + // being wired, so a provider without it returns unsupported_capability. + if p.ListContexts == nil { + return capabilityError(opts.Ref, "context list", iagent.CapMultiTurn) + } // Local scope preflight: after resolveProvider, before the API call. if err := preflightScopesForRef(f, id, opts.Ref); err != nil { return err } contexts, err := p.ListContexts(opts.Cmd.Context()) if err != nil { - return convertUnsupported(opts.Ref, "context list", err) + return err } // pretty is a human view only; a --jq expression implies structured JSON. if opts.Format == "pretty" && jqExpr(opts.Cmd) == "" { @@ -163,13 +169,17 @@ func agentContextGetRun(opts *contextOptions) error { if err != nil { return err } + // Capability gate before the API call. + if p.GetContext == nil { + return capabilityError(opts.Ref, "context get", iagent.CapMultiTurn) + } // Local scope preflight: after resolveProvider, before the API call. if err := preflightScopesForRef(f, id, opts.Ref); err != nil { return err } detail, err := p.GetContext(opts.Cmd.Context(), opts.CtxID) if err != nil { - return convertUnsupported(opts.Ref, "context get", err) + return err } if detail != nil { // Derive IsTerminal from State (single source of truth) for the embedded @@ -208,12 +218,16 @@ func agentContextDeleteRun(opts *contextOptions) error { if err != nil { return err } + // Capability gate before the API call. + if p.DeleteContext == nil { + return capabilityError(opts.Ref, "context delete", iagent.CapMultiTurn) + } // Local scope preflight: after resolveProvider, before the API call. if err := preflightScopesForRef(f, id, opts.Ref); err != nil { return err } if err := p.DeleteContext(opts.Cmd.Context(), opts.CtxID); err != nil { - return convertUnsupported(opts.Ref, "context delete", err) + return err } // pretty is a human view only; a --jq expression implies structured JSON. if opts.Format == "pretty" && jqExpr(opts.Cmd) == "" { diff --git a/cmd/agent/list.go b/cmd/agent/list.go index 926af162b..8a6982cc1 100644 --- a/cmd/agent/list.go +++ b/cmd/agent/list.go @@ -4,7 +4,6 @@ package agent import ( - "errors" "fmt" "github.com/spf13/cobra" @@ -138,17 +137,8 @@ func agentListSchemeRun(opts *listOptions) error { if err != nil { return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err.Error()).WithCause(err) } - agents, err := p.(iagent.Discoverer).ListAgents(opts.Cmd.Context()) + agents, err := p.ListAgents(opts.Cmd.Context()) if err != nil { - // A Discoverer that still returns the agent.ErrUnsupported sentinel at - // call time maps to the same typed unsupported_capability as the - // probe-negative path above (SPI contract, internal/agent/registry.go). - if errors.Is(err, iagent.ErrUnsupported) { - return errs.NewValidationError(errs.SubtypeUnsupportedCapability, - "provider '%s' 暂不支持列举 agent", opts.Scheme). - WithCause(err). - WithHint("%s", info.AgentIDSource) - } return err } @@ -176,9 +166,9 @@ func agentListSchemeRun(opts *listOptions) error { return nil } -// probeDiscoverer reports whether the provider built by info implements -// Discoverer. The probe instance is constructed with empty Deps and an empty -// agentID — no client is needed to answer a type assertion, which keeps the +// probeDiscoverer reports whether the provider built by info can enumerate its +// agents (wires ListAgents). The probe instance is constructed with empty Deps +// and an empty agentID — no client is needed to read a field, which keeps the // probe usable before config init. A factory error means the capability cannot // be confirmed, so it degrades to not discoverable. func probeDiscoverer(info iagent.ProviderInfo) bool { @@ -186,8 +176,7 @@ func probeDiscoverer(info iagent.ProviderInfo) bool { if err != nil || p == nil { return false } - _, ok := p.(iagent.Discoverer) - return ok + return p.ListAgents != nil } // listProviders builds the provider descriptors from the built-in registry so diff --git a/cmd/agent/list_test.go b/cmd/agent/list_test.go index bcd5ef388..8d3f1a37f 100644 --- a/cmd/agent/list_test.go +++ b/cmd/agent/list_test.go @@ -231,15 +231,26 @@ func TestAgentListScheme_UnknownScheme(t *testing.T) { } } -// fakeDiscProvider is a test-only provider implementing Discoverer, to pin the -// `agent list ` positive path without a real catalog provider. -type fakeDiscProvider struct{ iagent.Provider } +// stubCore wires the mandatory core fields onto a test *Provider; the list +// tests never dispatch Send/GetTask (they only exercise ListAgents), but +// Register requires both non-nil. +func stubCore(p *iagent.Provider) *iagent.Provider { + p.Send = func(ctx context.Context, in iagent.SendInput) (*iagent.AgentTask, error) { return nil, nil } + p.GetTask = func(ctx context.Context, taskID string) (*iagent.AgentTask, error) { return nil, nil } + return p +} -func (p *fakeDiscProvider) ListAgents(ctx context.Context) ([]iagent.AgentSummary, error) { - return []iagent.AgentSummary{ - {AgentRef: "fakedisc:a1", Name: "Agent One", Description: "第一个"}, - {AgentRef: "fakedisc:a2", Name: "Agent Two"}, - }, nil +// newFakeDisc is a test-only enumerable provider (wires ListAgents), to pin the +// `agent list ` positive path without a real catalog provider. +func newFakeDisc() *iagent.Provider { + return stubCore(&iagent.Provider{ + ListAgents: func(ctx context.Context) ([]iagent.AgentSummary, error) { + return []iagent.AgentSummary{ + {AgentRef: "fakedisc:a1", Name: "Agent One", Description: "第一个"}, + {AgentRef: "fakedisc:a2", Name: "Agent Two"}, + }, nil + }, + }) } // registerFakeDisc registers the fakedisc scheme. Like fakepause in @@ -248,7 +259,7 @@ func (p *fakeDiscProvider) ListAgents(ctx context.Context) ([]iagent.AgentSummar // provider set or provider count. func registerFakeDisc() { iagent.Register("fakedisc", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { return &fakeDiscProvider{}, nil }, + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { return newFakeDisc(), nil }, Label: "test fake (discoverer)", AgentRefFormat: "fakedisc:", AgentIDSource: "test only", @@ -296,9 +307,9 @@ func TestAgentListScheme_DiscovererListsAgents(t *testing.T) { func TestAgentListScheme_PropagatesIdentity(t *testing.T) { var captured iagent.Deps iagent.Register("fakedeps", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { captured = deps - return &fakeDiscProvider{}, nil + return newFakeDisc(), nil }, Label: "test fake (deps capture)", AgentRefFormat: "fakedeps:", @@ -324,14 +335,16 @@ func TestAgentListScheme_PropagatesIdentity(t *testing.T) { } } -// dirtyNameProvider is a Discoverer whose agent names carry ANSI escapes, to -// pin the pretty-path sanitization of agent-controlled fields. -type dirtyNameProvider struct{ iagent.Provider } - -func (p *dirtyNameProvider) ListAgents(ctx context.Context) ([]iagent.AgentSummary, error) { - return []iagent.AgentSummary{ - {AgentRef: "fakedirty:a1", Name: "\x1b[31mEvil\x1b[0m One", Description: "d\x1b[2Jesc"}, - }, nil +// newDirtyName is an enumerable provider whose agent names carry ANSI escapes, +// to pin the pretty-path sanitization of agent-controlled fields. +func newDirtyName() *iagent.Provider { + return stubCore(&iagent.Provider{ + ListAgents: func(ctx context.Context) ([]iagent.AgentSummary, error) { + return []iagent.AgentSummary{ + {AgentRef: "fakedirty:a1", Name: "\x1b[31mEvil\x1b[0m One", Description: "d\x1b[2Jesc"}, + }, nil + }, + }) } // TestAgentListScheme_PrettyStripsANSI pins the Task 10 review item: `agent list @@ -339,7 +352,7 @@ func (p *dirtyNameProvider) ListAgents(ctx context.Context) ([]iagent.AgentSumma // Name/Description before they reach the terminal. func TestAgentListScheme_PrettyStripsANSI(t *testing.T) { iagent.Register("fakedirty", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { return &dirtyNameProvider{}, nil }, + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { return newDirtyName(), nil }, Label: "test fake (dirty names)", AgentRefFormat: "fakedirty:", AgentIDSource: "test only", diff --git a/cmd/agent/scripted_provider_test.go b/cmd/agent/scripted_provider_test.go index 6d9bcc56e..6228b3fb4 100644 --- a/cmd/agent/scripted_provider_test.go +++ b/cmd/agent/scripted_provider_test.go @@ -12,7 +12,7 @@ import ( ) // scriptedHooks scripts a fake provider's behavior per test. Each hook maps to -// one Provider method; an unset hook that gets called panics — a tripwire +// one Provider func field; an unset hook that gets called panics — a tripwire // against a test reaching an unexpected provider path. This replaces the old // pattern of driving the (removed) real-OAPI adapter through httpmock stubs: // the command-layer contracts under test (envelope shape, watch exit codes, @@ -21,14 +21,13 @@ type scriptedHooks struct { send func(in iagent.SendInput) (*iagent.AgentTask, error) getTask func(taskID string) (*iagent.AgentTask, error) listTasks func(contextID string) ([]iagent.TaskSummary, error) - cancelTask func(taskID string) error listContexts func() ([]iagent.ContextSummary, error) getContext func(ctxID string) (*iagent.ContextDetail, error) deleteContext func(ctxID string) error downloadArtifact func(taskID, artifactID string) (*iagent.ArtifactData, error) } -// scripted is the package-level hook set shared by every scriptedProvider +// scripted is the package-level hook set shared by every scripted provider // instance (the registry factory cannot be re-pointed per test, the hooks can). var scripted scriptedHooks @@ -40,80 +39,59 @@ func setScripted(t *testing.T, h scriptedHooks) { t.Cleanup(func() { scripted = scriptedHooks{} }) } -// scriptedProvider delegates every Provider method to the scripted hooks. Its -// Card is synthesized statically (offline, zero-Deps safe) with a fixed honest -// capability matrix: task_cancel=false so the command-layer cancel gate is -// exercisable, everything else true except input_required. -type scriptedProvider struct{ scheme string } - -func (p *scriptedProvider) Card(ctx context.Context) (*iagent.AgentCard, error) { - card := iagent.NewCard(p.scheme, "agt_x") - card.Capabilities = iagent.Capabilities{ - TaskGet: true, - TaskList: true, - TaskCancel: false, - InputRequired: false, - FileInput: true, - ArtifactDownload: true, - MultiTurn: true, +// newScriptedProvider builds a scripted *Provider. Its capability surface is +// fixed by which fields are wired (the framework derives the card from this): +// CancelTask is deliberately left unwired so task_cancel=false (the command +// layer's cancel gate is exercised via example:echo); everything else the +// command tests drive is wired, and FileInput=true so the --file gate/confirm +// path is reachable. Each wired func delegates to the per-test hook and panics +// if that hook was not set (tripwire against an unexpected provider path). +func newScriptedProvider() *iagent.Provider { + return &iagent.Provider{ + Send: func(ctx context.Context, in iagent.SendInput) (*iagent.AgentTask, error) { + if scripted.send == nil { + panic("scripted provider: Send hook not set") + } + return scripted.send(in) + }, + GetTask: func(ctx context.Context, taskID string) (*iagent.AgentTask, error) { + if scripted.getTask == nil { + panic("scripted provider: GetTask hook not set") + } + return scripted.getTask(taskID) + }, + ListTasks: func(ctx context.Context, contextID string) ([]iagent.TaskSummary, error) { + if scripted.listTasks == nil { + panic("scripted provider: ListTasks hook not set") + } + return scripted.listTasks(contextID) + }, + ListContexts: func(ctx context.Context) ([]iagent.ContextSummary, error) { + if scripted.listContexts == nil { + panic("scripted provider: ListContexts hook not set") + } + return scripted.listContexts() + }, + GetContext: func(ctx context.Context, ctxID string) (*iagent.ContextDetail, error) { + if scripted.getContext == nil { + panic("scripted provider: GetContext hook not set") + } + return scripted.getContext(ctxID) + }, + DeleteContext: func(ctx context.Context, ctxID string) error { + if scripted.deleteContext == nil { + panic("scripted provider: DeleteContext hook not set") + } + return scripted.deleteContext(ctxID) + }, + DownloadArtifact: func(ctx context.Context, taskID, artifactID string) (*iagent.ArtifactData, error) { + if scripted.downloadArtifact == nil { + panic("scripted provider: DownloadArtifact hook not set") + } + return scripted.downloadArtifact(taskID, artifactID) + }, + FileInput: true, } - return card, nil -} - -func (p *scriptedProvider) Send(ctx context.Context, in iagent.SendInput) (*iagent.AgentTask, error) { - if scripted.send == nil { - panic("scripted provider: Send hook not set") - } - return scripted.send(in) -} - -func (p *scriptedProvider) GetTask(ctx context.Context, taskID string) (*iagent.AgentTask, error) { - if scripted.getTask == nil { - panic("scripted provider: GetTask hook not set") - } - return scripted.getTask(taskID) -} - -func (p *scriptedProvider) ListTasks(ctx context.Context, contextID string) ([]iagent.TaskSummary, error) { - if scripted.listTasks == nil { - panic("scripted provider: ListTasks hook not set") - } - return scripted.listTasks(contextID) -} - -func (p *scriptedProvider) CancelTask(ctx context.Context, taskID string) error { - if scripted.cancelTask == nil { - panic("scripted provider: CancelTask hook not set") - } - return scripted.cancelTask(taskID) -} - -func (p *scriptedProvider) ListContexts(ctx context.Context) ([]iagent.ContextSummary, error) { - if scripted.listContexts == nil { - panic("scripted provider: ListContexts hook not set") - } - return scripted.listContexts() -} - -func (p *scriptedProvider) GetContext(ctx context.Context, ctxID string) (*iagent.ContextDetail, error) { - if scripted.getContext == nil { - panic("scripted provider: GetContext hook not set") - } - return scripted.getContext(ctxID) -} - -func (p *scriptedProvider) DeleteContext(ctx context.Context, ctxID string) error { - if scripted.deleteContext == nil { - panic("scripted provider: DeleteContext hook not set") - } - return scripted.deleteContext(ctxID) -} - -func (p *scriptedProvider) DownloadArtifact(ctx context.Context, taskID, artifactID string) (*iagent.ArtifactData, error) { - if scripted.downloadArtifact == nil { - panic("scripted provider: DownloadArtifact hook not set") - } - return scripted.downloadArtifact(taskID, artifactID) } // fakescopedAllScopes is the full RequiredScopes set of the fakescoped test @@ -127,7 +105,7 @@ var fakescopedAllScopes = []string{ } // fakeflowAgentIDSource is the AgentIDSource text of the fakeflow provider — -// the non-Discoverer `agent list ` error surfaces it as the hint. +// the non-enumerable `agent list ` error surfaces it as the hint. const fakeflowAgentIDSource = "在 fakeflow 测试控制台获取 agent_id(形如 agt_xxx)" // registerScripted registers the two scripted schemes exactly once (Register @@ -144,8 +122,8 @@ var registerScriptedOnce sync.Once func registerScripted() { registerScriptedOnce.Do(func() { iagent.Register("fakeflow", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { - return &scriptedProvider{scheme: "fakeflow"}, nil + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { + return newScriptedProvider(), nil }, Label: "test fake (scripted flow)", AgentRefFormat: "fakeflow:", @@ -154,8 +132,8 @@ func registerScripted() { Identities: []iagent.IdentitySpec{{Type: iagent.IdentityUser}, {Type: iagent.IdentityBot}}, }) iagent.Register("fakescoped", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { - return &scriptedProvider{scheme: "fakescoped"}, nil + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { + return newScriptedProvider(), nil }, Label: "test fake (scoped)", AgentRefFormat: "fakescoped:", diff --git a/cmd/agent/send.go b/cmd/agent/send.go index 205859c84..4c539fb4c 100644 --- a/cmd/agent/send.go +++ b/cmd/agent/send.go @@ -111,7 +111,11 @@ func agentSendRun(opts *sendOptions) error { return err } - card, err := p.Card(opts.Cmd.Context()) + r, err := iagent.ParseRef(opts.Ref) + if err != nil { + return wrapRefResolveError(err) + } + card, err := iagent.BuildCard(opts.Cmd.Context(), r.Scheme, r.AgentID, p) if err != nil { return err } @@ -134,17 +138,24 @@ func agentSendRun(opts *sendOptions) error { return emitDryRun(f, opts.Cmd, opts.Ref, in, opts.Format) } - // --file exfiltrates local file content off this machine (the provider reads - // the file and uploads it to the remote agent). That is an irreversible, - // CLI-enforced high-risk write: a real send that would upload requires --yes, - // returning confirmation_required (exit 10) before any network access. Gated - // on the Card's file_input so a provider that cannot upload (the send would - // be rejected as unsupported anyway) does not prompt for a confirmation that - // buys nothing. dry-run above is exempt — it never uploads. - if len(in.Files) > 0 && card.Supports(iagent.CapFileInput) && !opts.Yes { - return errs.NewConfirmationRequiredError(errs.RiskHighRiskWrite, "agent send --file", - "--file 会把本地文件外发上传到远端 agent(内容离开本机,不可撤回)"). - WithHint("确认要外发这些文件后,加 --yes 重发") + if len(in.Files) > 0 { + // An agent that does not declare file_input cannot take an upload, so + // --file against it is unsupported_capability — gated before any network + // access, so the user is not told "confirm the upload" for a send that + // would be rejected anyway. + if !card.Supports(iagent.CapFileInput) { + return capabilityError(opts.Ref, "send with --file", iagent.CapFileInput) + } + // --file exfiltrates local file content off this machine (the provider + // reads the file and uploads it to the remote agent). That is an + // irreversible, CLI-enforced high-risk write: a real send that would upload + // requires --yes, returning confirmation_required (exit 10) before any + // network access. dry-run above is exempt — it never uploads. + if !opts.Yes { + return errs.NewConfirmationRequiredError(errs.RiskHighRiskWrite, "agent send --file", + "--file 会把本地文件外发上传到远端 agent(内容离开本机,不可撤回)"). + WithHint("确认要外发这些文件后,加 --yes 重发") + } } // A real send calls the API, so it needs a configured client; resolve it now @@ -163,7 +174,7 @@ func agentSendRun(opts *sendOptions) error { task, err := pc.Send(opts.Cmd.Context(), in) if err != nil { - return convertUnsupported(opts.Ref, "send", err) + return err } normalizeTask(task) diff --git a/cmd/agent/task.go b/cmd/agent/task.go index 368b3d3da..a16ab0611 100644 --- a/cmd/agent/task.go +++ b/cmd/agent/task.go @@ -52,6 +52,11 @@ var resolveDownload = func(opts *taskOptions) (*iagent.ArtifactData, error) { if err != nil { return nil, err } + // Capability gate before the API call: a provider that does not wire + // DownloadArtifact (card artifact_download=false) returns unsupported_capability. + if p.DownloadArtifact == nil { + return nil, capabilityError(opts.Ref, "artifact download", iagent.CapArtifactDownload) + } if err := preflightScopesForRef(opts.Factory, id, opts.Ref); err != nil { return nil, err } @@ -229,7 +234,7 @@ func agentTaskGetRun(opts *taskOptions) error { ctx := opts.Cmd.Context() task, err := p.GetTask(ctx, opts.TaskID) if err != nil { - return convertUnsupported(opts.Ref, "task get", err) + return err } if opts.Watch && !task.State.ShouldStopPolling() { @@ -246,7 +251,7 @@ func agentTaskGetRun(opts *taskOptions) error { } final, perr := pollToStop(pollCtx, p, opts.TaskID) if perr != nil { - return convertUnsupported(opts.Ref, "task get", perr) + return perr } if final != nil { task = final @@ -275,13 +280,18 @@ func agentTaskListRun(opts *taskOptions) error { if err != nil { return err } + // Capability gate before the API call: a provider that does not wire + // ListTasks (card task_list=false) returns unsupported_capability. + if p.ListTasks == nil { + return capabilityError(opts.Ref, "task list", iagent.CapTaskList) + } // Local scope preflight: after resolveProvider, before the API call. if err := preflightScopesForRef(f, id, opts.Ref); err != nil { return err } tasks, err := p.ListTasks(opts.Cmd.Context(), opts.ContextID) if err != nil { - return convertUnsupported(opts.Ref, "task list", err) + return err } tasks = normalizeTaskSummaries(tasks) // pretty is a human view only; a --jq expression implies structured JSON. @@ -309,11 +319,15 @@ func agentTaskListRun(opts *taskOptions) error { // or API call. Only a supporting provider reaches resolveProvider + // CancelTask. func agentTaskCancelRun(opts *taskOptions) error { - card, err := cardForRef(opts.Ref) + // Gate before requiring a Factory / network: resolve with zero Deps and read + // the CancelTask capability (a wired field == card task_cancel=true). An agent + // that does not support cancel (e.g. example:echo) returns + // unsupported_capability with no Factory or API access. + probe, err := iagent.Resolve(opts.Ref, iagent.Deps{}) if err != nil { - return err + return wrapRefResolveError(err) } - if !card.Supports(iagent.CapTaskCancel) { + if probe.CancelTask == nil { return capabilityError(opts.Ref, "task cancel", iagent.CapTaskCancel) } @@ -330,10 +344,7 @@ func agentTaskCancelRun(opts *taskOptions) error { return err } if err := p.CancelTask(opts.Cmd.Context(), opts.TaskID); err != nil { - // Belt-and-braces behind the card gate above: a provider that declares - // task_cancel=true but still returns the sentinel maps to the same typed - // unsupported_capability instead of the internal-error fallback. - return convertUnsupported(opts.Ref, "task cancel", err) + return err } // pretty is a human view only; a --jq expression implies structured JSON. if opts.Format == "pretty" && jqExpr(opts.Cmd) == "" { @@ -353,19 +364,6 @@ func agentTaskCancelRun(opts *taskOptions) error { return nil } -// cardForRef resolves the capability Card for ref without a Factory or network. -// It relies on the adapter cards being statically synthesized (Card needs no -// client), so capability gating can run before the API path is even built. A -// malformed ref / unknown scheme is promoted to an invalid_argument validation -// error (exit 2), matching resolveProvider's wrapping. -func cardForRef(ref string) (*iagent.AgentCard, error) { - p, err := iagent.Resolve(ref, iagent.Deps{}) - if err != nil { - return nil, wrapRefResolveError(err) - } - return p.Card(context.Background()) -} - // downloadArtifact resolves the artifact descriptor and writes it to opts.Output // under vfs. A URL-type artifact is SSRF-validated and fetched over a // download-hardened client; an inline-bytes artifact is written directly. The @@ -394,7 +392,7 @@ func downloadArtifact(opts *taskOptions) error { ctx := opts.Cmd.Context() art, err := resolveDownload(opts) if err != nil { - return convertUnsupported(opts.Ref, "artifact download", err) + return err } data := art.Bytes diff --git a/cmd/agent/unsupported_test.go b/cmd/agent/unsupported_test.go index 13f2bf33f..0f486a41a 100644 --- a/cmd/agent/unsupported_test.go +++ b/cmd/agent/unsupported_test.go @@ -6,7 +6,6 @@ package agent import ( "context" "encoding/json" - "errors" "strings" "sync" "testing" @@ -18,43 +17,38 @@ import ( "github.com/larksuite/cli/internal/output" ) -// unsupProvider is a stub provider driving the slice-2 command-layer wirings -// without any HTTP: ListContexts/DeleteContext return the bare -// agent.ErrUnsupported sentinel (convertUnsupported must promote it to the -// typed unsupported_capability), and GetTask returns a task whose IsTerminal -// deliberately mismatches its State (normalizeTask must re-derive it). Any -// other Provider method call panics via the nil embedded interface — a -// tripwire against the test reaching an unexpected path. -type unsupProvider struct{ iagent.Provider } - -func (p *unsupProvider) Card(ctx context.Context) (*iagent.AgentCard, error) { - return iagent.NewCard("fakeunsup", "a1"), nil -} - -func (p *unsupProvider) ListContexts(ctx context.Context) ([]iagent.ContextSummary, error) { - return nil, iagent.ErrUnsupported -} - -func (p *unsupProvider) DeleteContext(ctx context.Context, ctxID string) error { - return iagent.ErrUnsupported -} - -func (p *unsupProvider) GetTask(ctx context.Context, taskID string) (*iagent.AgentTask, error) { - // Deliberate mismatch: State is terminal but IsTerminal=false (simulating a - // provider that forgot to set it or set it wrong). - return &iagent.AgentTask{TaskID: taskID, State: iagent.StateCompleted, IsTerminal: false}, nil +// newUnsupProvider builds a stub *Provider driving the command-layer +// capability-gate wirings without any HTTP: ListContexts / DeleteContext are +// left UNWIRED (nil), so the command layer's nil-gate must return the typed +// unsupported_capability before any network access. GetTask is wired to return a +// task whose IsTerminal deliberately mismatches its State (normalizeTask must +// re-derive it). Send is wired (core, required by Register) but never called +// here. There is no capability-refusal code in the provider — "unsupported" is +// expressed purely by the absent fields. +func newUnsupProvider() *iagent.Provider { + return &iagent.Provider{ + Send: func(ctx context.Context, in iagent.SendInput) (*iagent.AgentTask, error) { + panic("unsup provider: Send should not be called") + }, + GetTask: func(ctx context.Context, taskID string) (*iagent.AgentTask, error) { + // Deliberate mismatch: State is terminal but IsTerminal=false (simulating + // a provider that forgot to set it or set it wrong). + return &iagent.AgentTask{TaskID: taskID, State: iagent.StateCompleted, IsTerminal: false}, nil + }, + // ListContexts / DeleteContext intentionally unwired ⇒ unsupported. + } } // registerFakeUnsup registers the fakeunsup scheme exactly once (Register -// panics on duplicates). Like fakedisc/fakepause it leaks into the -// package-level registry for the remaining tests of this package run. +// panics on duplicates). Like the other fakes it leaks into the package-level +// registry for the remaining tests of this package run. var registerFakeUnsupOnce sync.Once func registerFakeUnsup() { registerFakeUnsupOnce.Do(func() { iagent.Register("fakeunsup", iagent.ProviderInfo{ - Factory: func(deps iagent.Deps, agentID string) (iagent.Provider, error) { return &unsupProvider{}, nil }, - Label: "test fake (ErrUnsupported)", + Factory: func(deps iagent.Deps, agentID string) (*iagent.Provider, error) { return newUnsupProvider(), nil }, + Label: "test fake (unwired optional capabilities)", AgentRefFormat: "fakeunsup:", AgentIDSource: "test only", Kind: iagent.KindInstance, @@ -63,7 +57,7 @@ func registerFakeUnsup() { }) } -// assertUnsupportedCapability pins the full convertUnsupported contract on err: +// assertUnsupportedCapability pins the full capability-gate contract on err: // validation typed, subtype unsupported_capability, exit 2, hint pointing at // `agent card `, and — because the Factory's httpmock registry has zero // stubs — no HTTP was issued (any network attempt would have surfaced as an @@ -71,7 +65,7 @@ func registerFakeUnsup() { func assertUnsupportedCapability(t *testing.T, err error, ref string) { t.Helper() if err == nil { - t.Fatal("a provider returning ErrUnsupported should error") + t.Fatal("an unsupported capability should error") } if !errs.IsValidation(err) { t.Fatalf("want validation error, got %T (%v)", err, err) @@ -91,10 +85,10 @@ func assertUnsupportedCapability(t *testing.T, err error, ref string) { } } -// TestContextListConvertsErrUnsupported pins the ErrUnsupported wiring on -// `context list`: a provider sentinel maps to typed unsupported_capability +// TestContextListUnsupportedGated pins the capability gate on `context list`: a +// provider that does not wire ListContexts returns typed unsupported_capability // (exit 2) with the agent-card hint, without any HTTP. -func TestContextListConvertsErrUnsupported(t *testing.T) { +func TestContextListUnsupportedGated(t *testing.T) { registerFakeUnsup() f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{AppID: "cli_x", AppSecret: "fake-secret", Brand: core.BrandFeishu}) opts := &contextOptions{ @@ -103,9 +97,9 @@ func TestContextListConvertsErrUnsupported(t *testing.T) { assertUnsupportedCapability(t, agentContextListRun(opts), "fakeunsup:a1") } -// TestContextDeleteConvertsErrUnsupported pins the same wiring on the confirmed -// `context delete` path (--yes passes, provider then returns the sentinel). -func TestContextDeleteConvertsErrUnsupported(t *testing.T) { +// TestContextDeleteUnsupportedGated pins the same gate on the confirmed +// `context delete` path (--yes passes, provider does not wire DeleteContext). +func TestContextDeleteUnsupportedGated(t *testing.T) { registerFakeUnsup() f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{AppID: "cli_x", AppSecret: "fake-secret", Brand: core.BrandFeishu}) opts := &contextOptions{ @@ -114,30 +108,6 @@ func TestContextDeleteConvertsErrUnsupported(t *testing.T) { assertUnsupportedCapability(t, agentContextDeleteRun(opts), "fakeunsup:a1") } -// TestConvertUnsupported_Passthrough pins the transparency contract: nil and -// non-sentinel errors (including already-typed ones) pass through unchanged, so -// wrapping every provider call site is side-effect free. -func TestConvertUnsupported_Passthrough(t *testing.T) { - if err := convertUnsupported("example:agt_x", "send", nil); err != nil { - t.Errorf("nil should pass through unchanged, got %v", err) - } - plain := errors.New("boom") - if err := convertUnsupported("example:agt_x", "send", plain); err != plain { - t.Errorf("a non-sentinel error should pass through unchanged, got %v", err) - } - typed := errs.NewValidationError(errs.SubtypeInvalidArgument, "bad input") - if err := convertUnsupported("example:agt_x", "send", typed); !errors.Is(err, typed) { - t.Errorf("an already-typed error should pass through unchanged, got %v", err) - } - // A wrapped sentinel must also match (errors.Is semantics). - wrapped := errs.NewInternalError(errs.SubtypeUnknown, "wrapped").WithCause(iagent.ErrUnsupported) - converted := convertUnsupported("example:agt_x", "send", wrapped) - p, ok := errs.ProblemOf(converted) - if !ok || p.Subtype != errs.SubtypeUnsupportedCapability { - t.Errorf("a wrapped sentinel should also convert to unsupported_capability, got %+v", p) - } -} - // TestTaskGetDerivesIsTerminalFromState pins the normalizeTask wiring: a // provider returning a State/IsTerminal-mismatched task (completed + // is_terminal=false) must emit is_terminal=true — the command layer derives diff --git a/internal/agent/agenttest/agenttest.go b/internal/agent/agenttest/agenttest.go index 4195954d8..68ea0ae9a 100644 --- a/internal/agent/agenttest/agenttest.go +++ b/internal/agent/agenttest/agenttest.go @@ -65,6 +65,14 @@ func RunConformance(t *testing.T, scheme, sampleAgentID string) { if p == nil { t.Fatal("conformance: Factory must not return a nil provider") } + // Core fields are mandatory (the command layer dispatches them without a + // nil-check); Register enforces this at registration, re-assert here. + if p.Send == nil { + t.Error("conformance: Provider.Send (core) must be wired") + } + if p.GetTask == nil { + t.Error("conformance: Provider.GetTask (core) must be wired") + } }) t.Run("card", func(t *testing.T) { @@ -74,9 +82,9 @@ func RunConformance(t *testing.T, scheme, sampleAgentID string) { if err != nil { t.Fatalf("conformance: Factory(zero-value Deps) returned error: %v", err) } - card, err := p.Card(context.Background()) + card, err := agent.BuildCard(context.Background(), scheme, sampleAgentID, p) if err != nil { - t.Fatalf("conformance: Card should be available offline (expected nil error), got %v", err) + t.Fatalf("conformance: BuildCard should be available offline (expected nil error), got %v", err) } if card == nil { t.Fatal("conformance: Card must not return nil") @@ -115,11 +123,10 @@ func RunConformance(t *testing.T, scheme, sampleAgentID string) { if err != nil { t.Fatalf("conformance: Factory(zero-value Deps) returned error: %v", err) } - d, ok := p.(agent.Discoverer) - if !ok { - t.Fatal("conformance: catalog-type provider instances must implement Discoverer") + if p.ListAgents == nil { + t.Fatal("conformance: catalog-type provider must wire ListAgents") } - list, err := d.ListAgents(context.Background()) + list, err := p.ListAgents(context.Background()) if err != nil { t.Fatalf("conformance: catalog-type ListAgents should be available offline (expected nil error), got %v", err) } @@ -144,7 +151,7 @@ func RunConformance(t *testing.T, scheme, sampleAgentID string) { if !found { t.Errorf("conformance: sampleAgentID should appear in the enumeration (expected to contain %q), got %+v", wantRef, list) } - list2, err := d.ListAgents(context.Background()) + list2, err := p.ListAgents(context.Background()) if err != nil { t.Fatalf("conformance: second ListAgents returned error: %v", err) } diff --git a/internal/agent/card.go b/internal/agent/card.go index d8e1013c4..2655bf93b 100644 --- a/internal/agent/card.go +++ b/internal/agent/card.go @@ -3,6 +3,8 @@ package agent +import "context" + // capability key constants (the JSON key names in capabilities, also the // capability identifiers used by Supports / capabilityError). Only capabilities // that "can change the AI's next command line and are currently deliverable" are @@ -68,6 +70,49 @@ func NewCard(scheme, agentID string) *AgentCard { } } +// DeriveCapabilities computes the capability matrix from which Provider fields +// are wired — the single source of truth. The method-backed capabilities are +// derived from the corresponding func field being non-nil (implement it = +// support it); file_input / input_required are behavioral flags with no backing +// method and are read straight from the struct. Send/GetTask are mandatory +// (Register enforces), so task_get is always true. +func DeriveCapabilities(p *Provider) Capabilities { + return Capabilities{ + TaskGet: p.GetTask != nil, + TaskList: p.ListTasks != nil, + TaskCancel: p.CancelTask != nil, + ArtifactDownload: p.DownloadArtifact != nil, + MultiTurn: p.ListContexts != nil, + FileInput: p.FileInput, + InputRequired: p.InputRequired, + } +} + +// BuildCard synthesizes an agent's full Card: NewCard fills the +// registration-time fields, DeriveCapabilities fills the matrix from the wired +// fields, and Describe (if the provider set it) supplies the per-agent +// Name/Description/Parameters/Skills and validates the agent_id. A provider +// therefore never assembles its own card or declares its own capability bools. +func BuildCard(ctx context.Context, scheme, agentID string, p *Provider) (*AgentCard, error) { + card := NewCard(scheme, agentID) + card.Capabilities = DeriveCapabilities(p) + if p.Describe != nil { + info, err := p.Describe(ctx) + if err != nil { + return nil, err + } + if info != nil { + card.Name = info.Name + card.Description = info.Description + if info.Parameters != nil { + card.Parameters = info.Parameters + } + card.Skills = info.Skills + } + } + return card, nil +} + // CardParam is one input parameter declared by a Card (used for --param validation). type CardParam struct { Name string `json:"name"` diff --git a/internal/agent/card_test.go b/internal/agent/card_test.go index 5630489fb..edaf190e0 100644 --- a/internal/agent/card_test.go +++ b/internal/agent/card_test.go @@ -43,7 +43,7 @@ func TestCardSupports(t *testing.T) { // registration-known field and panics on an unregistered scheme. func TestNewCardFillsRegistrationFields(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - info := testInfo("nc", func(Deps, string) (Provider, error) { return nil, nil }) + info := testInfo("nc", okFactory()) info.Identities = []IdentitySpec{{Type: IdentityBot, Precondition: "需要白名单"}} Register("nc", info) diff --git a/internal/agent/catalog.go b/internal/agent/catalog.go index 0eff6954a..a6b4929b7 100644 --- a/internal/agent/catalog.go +++ b/internal/agent/catalog.go @@ -11,23 +11,25 @@ import ( ) // CatalogEntry is the provider-neutral description of one predefined agent in a -// catalog-type provider. It holds only fields the framework can consume -// (enumeration / Card synthesis); a provider's private business fields (such as -// the execution backend it points to) are maintained alongside in the -// integrator's own package and do not enter framework types. +// catalog-type provider. It holds only descriptive fields the framework can +// consume (enumeration / Card metadata); capabilities are NOT declared here — +// they are derived from which Provider func fields the integrator's Factory +// wires for this agent (see agent/example). A provider's private business +// fields (such as the execution backend it points to) are maintained alongside +// in the integrator's own package and do not enter framework types. type CatalogEntry struct { - ID string - Name string - Description string - Capabilities Capabilities + ID string + Name string + Description string } // StaticCatalog carries the common boilerplate of a catalog-type provider: -// catalog enumeration (Discoverer semantics), per-agent rich Card (NewCard fills -// in registration-time fields, the entry adds Name/Description/Capabilities), and -// a typed validation error for unknown ids. Business differences (such as the -// execution backend) are composed by the provider itself on the outer layer. -// It is read-only after construction and safe for concurrent use. +// catalog enumeration (the ListAgents field), per-agent Card metadata (Describe +// returns the entry's Name/Description), and a typed validation error for +// unknown ids. Capabilities are derived by the framework from the Provider +// fields the integrator's Factory wires, not stored here. Business differences +// (such as the execution backend) are composed by the provider itself on the +// outer layer. It is read-only after construction and safe for concurrent use. type StaticCatalog struct { scheme string entries map[string]CatalogEntry @@ -61,20 +63,17 @@ func (c *StaticCatalog) ListAgents(ctx context.Context) ([]AgentSummary, error) return out, nil } -// Card synthesizes the agent's rich card: NewCard fills in registration-time -// fields (Provider/Label/Identity/AgentIDSource/empty Parameters), and the entry -// adds the per-agent Name/Description/Capabilities. An unknown id returns the -// typed error from Lookup. -func (c *StaticCatalog) Card(agentID string) (*AgentCard, error) { +// Describe returns the per-agent Card metadata (Name/Description) for agentID, +// suitable as a Provider.Describe implementation: it validates the id (an +// unknown id returns the typed error from Lookup) and leaves capability +// derivation to the framework. Parameters/Skills are left empty; a provider +// with declared parameters composes them on top. +func (c *StaticCatalog) Describe(agentID string) (*CardInfo, error) { e, err := c.Lookup(agentID) if err != nil { return nil, err } - card := NewCard(c.scheme, agentID) - card.Name = e.Name - card.Description = e.Description - card.Capabilities = e.Capabilities - return card, nil + return &CardInfo{Name: e.Name, Description: e.Description}, nil } // Lookup fetches a catalog entry by id. An unknown id returns a typed diff --git a/internal/agent/catalog_test.go b/internal/agent/catalog_test.go index 9677b8e2b..7d0f86f39 100644 --- a/internal/agent/catalog_test.go +++ b/internal/agent/catalog_test.go @@ -15,10 +15,8 @@ import ( // testCatalogEntries is declared out of order to verify ListAgents sorting stability. func testCatalogEntries() []CatalogEntry { return []CatalogEntry{ - {ID: "zeta", Name: "Zeta 助手", Description: "z desc", - Capabilities: Capabilities{TaskGet: true}}, - {ID: "alpha", Name: "Alpha 助手", Description: "a desc", - Capabilities: Capabilities{TaskGet: true, MultiTurn: true}}, + {ID: "zeta", Name: "Zeta 助手", Description: "z desc"}, + {ID: "alpha", Name: "Alpha 助手", Description: "a desc"}, } } @@ -47,37 +45,21 @@ func TestStaticCatalogListAgentsSorted(t *testing.T) { } } -// TestStaticCatalogCard asserts Card = NewCard pre-filling registration-time fields -// plus the entry filling in Name/Description/Capabilities. -func TestStaticCatalogCard(t *testing.T) { - swapRegistry(t, map[string]ProviderInfo{}) - Register("cattest", testInfo("cattest", - func(Deps, string) (Provider, error) { return &stubProvider{}, nil })) - info, _ := Info("cattest") - +// TestStaticCatalogDescribe asserts Describe returns the entry's per-agent +// Name/Description (the framework fills registration fields and derives +// capabilities from the wired Provider fields, so those are not Describe's job). +func TestStaticCatalogDescribe(t *testing.T) { c := NewStaticCatalog("cattest", testCatalogEntries()) - card, err := c.Card("alpha") + info, err := c.Describe("alpha") if err != nil { t.Fatal(err) } - if card.Provider != "cattest" || card.AgentID != "alpha" { - t.Fatalf("provider/agent_id: %+v", card) + if info.Name != "Alpha 助手" || info.Description != "a desc" { + t.Fatalf("Describe should return the entry Name/Description, got %+v", info) } - if card.ProviderLabel != info.Label || card.AgentIDSource != info.AgentIDSource { - t.Fatalf("registration-time fields should be pre-filled by NewCard: %+v", card) - } - if !reflect.DeepEqual(card.Identity, info.Identities) { - t.Fatalf("Identity should match the registered value: %+v", card.Identity) - } - if card.Parameters == nil { - t.Fatal("Parameters must not be nil (always emitted, empty is [])") - } - if card.Name != "Alpha 助手" || card.Description != "a desc" { - t.Fatalf("entry should fill in Name/Description: %+v", card) - } - wantCaps := Capabilities{TaskGet: true, MultiTurn: true} - if card.Capabilities != wantCaps { - t.Fatalf("entry should fill in Capabilities, want %+v got %+v", wantCaps, card.Capabilities) + // Describe carries no capabilities/parameters — those are the framework's job. + if len(info.Parameters) != 0 || len(info.Skills) != 0 { + t.Fatalf("Describe should not populate Parameters/Skills, got %+v", info) } } @@ -103,9 +85,9 @@ func TestStaticCatalogUnknownID(t *testing.T) { if want := "运行 lark-cli agent list cattest 查看可用 agent"; ve.Hint != want { t.Fatalf("hint should be %q, got %q", want, ve.Hint) } - // Card goes through the same Lookup path. - if _, err := c.Card("nonexistent"); !errors.As(err, &ve) { - t.Fatalf("Card with an unknown id should return the same typed error, got %v", err) + // Describe goes through the same Lookup path. + if _, err := c.Describe("nonexistent"); !errors.As(err, &ve) { + t.Fatalf("Describe with an unknown id should return the same typed error, got %v", err) } } diff --git a/internal/agent/provider.go b/internal/agent/provider.go index 50c06180f..d730130ff 100644 --- a/internal/agent/provider.go +++ b/internal/agent/provider.go @@ -14,26 +14,76 @@ type SendInput struct { TaskID string } -// Provider is a remote agent adapter: it translates the unified commands into a -// specific vendor's OAPI. -type Provider interface { - // Card returns the agent's capability description (may be synthesized - // statically, not necessarily via an API call). - Card(ctx context.Context) (*AgentCard, error) - // Send sends one message, starting a new task or continuing an existing one. - Send(ctx context.Context, in SendInput) (*AgentTask, error) - // GetTask queries a single task's state and artifacts. - GetTask(ctx context.Context, taskID string) (*AgentTask, error) - // ListTasks lists tasks, optionally filtered by contextID (empty string means no filter). - ListTasks(ctx context.Context, contextID string) ([]TaskSummary, error) - // CancelTask cancels (interrupts) a task; returns ErrUnsupported when unsupported. - CancelTask(ctx context.Context, taskID string) error - // ListContexts lists multi-turn contexts. - ListContexts(ctx context.Context) ([]ContextSummary, error) - // GetContext returns a single context's detail. - GetContext(ctx context.Context, ctxID string) (*ContextDetail, error) - // DeleteContext deletes a context (a destructive operation). - DeleteContext(ctx context.Context, ctxID string) error - // DownloadArtifact fetches artifact data: the URL type returns URL, the inline type returns Bytes. - DownloadArtifact(ctx context.Context, taskID, artifactID string) (*ArtifactData, error) +// CardInfo is the per-agent descriptive metadata a provider supplies for its +// Card (everything the framework cannot fill from registration data or derive +// from capabilities): the display Name/Description, declared input Parameters, +// and Skills. It is returned by Provider.Describe. +type CardInfo struct { + Name string + Description string + Parameters []CardParam + Skills []CardSkill +} + +// Provider is a remote agent adapter: it translates the unified commands into a +// specific vendor's OAPI. It is a struct of function fields rather than a fat +// interface, mirroring the events KeyDefinition / shortcuts Shortcut convention: +// a provider fills only the capabilities it supports, and a nil optional field +// means "unsupported" — the command layer gates on it and returns a unified +// unsupported_capability error before any network access, so a provider never +// writes capability-refusal code itself. The Card capability matrix is derived +// by the framework from which fields are non-nil (see BuildCard), so declaration +// and behavior are single-sourced and cannot drift. +// +// Because a Provider is constructed per (deps, agentID) by its Factory, a +// catalog provider whose agents differ in capability wires different fields per +// agentID (see agent/example) — capability is expressed as code, not a +// hand-maintained bool matrix. +type Provider struct { + // ── Core (Register validates both non-nil for every provider) ── + + // Send sends one message, starting a new task or continuing an existing one. + Send func(ctx context.Context, in SendInput) (*AgentTask, error) + // GetTask queries a single task's state and artifacts. + GetTask func(ctx context.Context, taskID string) (*AgentTask, error) + + // ── Optional capabilities (nil = unsupported; framework gates) ── + + // ListTasks lists tasks, optionally filtered by contextID (empty = no filter). + // nil ⇒ card task_list=false. + ListTasks func(ctx context.Context, contextID string) ([]TaskSummary, error) + // CancelTask cancels (interrupts) a task. nil ⇒ card task_cancel=false. + CancelTask func(ctx context.Context, taskID string) error + // ListContexts lists multi-turn contexts. nil ⇒ card multi_turn=false (the + // multi_turn capability is derived from this, the enumeration entry point). + ListContexts func(ctx context.Context) ([]ContextSummary, error) + // GetContext returns a single context's detail. nil ⇒ context get unsupported. + GetContext func(ctx context.Context, ctxID string) (*ContextDetail, error) + // DeleteContext deletes a context (destructive). nil ⇒ context delete unsupported. + DeleteContext func(ctx context.Context, ctxID string) error + // DownloadArtifact fetches artifact data: the URL type returns URL, the inline + // type returns Bytes. nil ⇒ card artifact_download=false. + DownloadArtifact func(ctx context.Context, taskID, artifactID string) (*ArtifactData, error) + // ListAgents enumerates the provider's own agents (catalog discovery). nil ⇒ + // `agent list ` reports the provider is not enumerable. A KindCatalog + // provider must wire it (asserted at Register time). + ListAgents func(ctx context.Context) ([]AgentSummary, error) + + // ── Optional descriptive metadata ── + + // Describe supplies the per-agent Card metadata (Name/Description/Parameters/ + // Skills) and is the place to validate an unknown agent_id (return a typed + // error). nil ⇒ the card carries only registration fields + derived + // capabilities. Called at card-display time (may hit the network for an + // instance provider that fetches its card remotely). + Describe func(ctx context.Context) (*CardInfo, error) + + // ── Behavioral flags (not derivable from method presence) ── + + // FileInput reports whether Send accepts SendInput.Files (drives card + // file_input and the --file off-machine-upload confirmation gate). + FileInput bool + // InputRequired reports whether the agent may pause a task in the + // input_required state awaiting more input (drives card input_required). + InputRequired bool } diff --git a/internal/agent/registry.go b/internal/agent/registry.go index 62f4cd28d..596ffe883 100644 --- a/internal/agent/registry.go +++ b/internal/agent/registry.go @@ -4,7 +4,6 @@ package agent import ( - "errors" "fmt" "sort" "strings" @@ -13,10 +12,6 @@ import ( "github.com/larksuite/cli/internal/core" ) -// ErrUnsupported indicates a capability is unsupported by the current provider -// (converted by the command layer into an unsupported_capability typed error). -var ErrUnsupported = errors.New("capability not supported") - // Deps are the dependencies a provider factory needs (injected by the command // layer to avoid internal/agent depending on cmd). type Deps struct { @@ -25,7 +20,7 @@ type Deps struct { } // Factory constructs a Provider from an agentID plus dependencies. -type Factory func(deps Deps, agentID string) (Provider, error) +type Factory func(deps Deps, agentID string) (*Provider, error) // ProviderKind is the closed set of provider forms (validated at Register time // to guard against cast typos). @@ -33,7 +28,7 @@ type ProviderKind string const ( // KindCatalog is the catalog type: the full agent set is known at - // registration time, and it must implement Discoverer. + // registration time, and it must wire Provider.ListAgents. KindCatalog ProviderKind = "catalog" // KindInstance is the instance type: agents are created by users on the // platform and cannot be enumerated by the CLI in advance. @@ -49,8 +44,9 @@ type ProviderInfo struct { // zero-value Deps and have no side effects during construction — this // contract is enforced at registration time by Register's zero-value Deps // probe (a violation panics), and agent list also constructs a probe - // instance with empty Deps to assert the Discoverer capability - // (cmd/agent/list.go probeDiscoverer). + // instance with empty Deps to read the ListAgents capability + // (cmd/agent/list.go probeDiscoverer). Because the probe passes zero Deps and + // empty agentID, capability wiring must not depend on either. Factory Factory // Label is the user-facing provider name. Label string @@ -61,7 +57,7 @@ type ProviderInfo struct { // for AI-guided onboarding). AgentIDSource string // Kind is the provider form: KindCatalog (catalog type) or KindInstance - // (instance type). Catalog types must implement Discoverer (asserted at + // (instance type). Catalog types must wire Provider.ListAgents (asserted at // Register time). Kind ProviderKind // RequiredScopes is the full (flat) set of scopes needed by any real API @@ -117,10 +113,22 @@ func Register(scheme string, info ProviderInfo) { if err != nil { panic("agent: provider factory must accept zero-value Deps: " + scheme + ", got error: " + err.Error()) } - if info.Kind == KindCatalog { - if _, ok := p.(Discoverer); !ok { - panic("agent: catalog provider must implement Discoverer: " + scheme) - } + if p == nil { + panic("agent: provider factory returned nil Provider: " + scheme) + } + // Core capabilities are mandatory for every provider — a provider you cannot + // send to or read a task back from is not usable. The command layer relies on + // these never being nil (no nil-check before dispatch), so enforce it here. + switch { + case p.Send == nil: + panic("agent: provider missing core Send: " + scheme) + case p.GetTask == nil: + panic("agent: provider missing core GetTask: " + scheme) + } + // A catalog provider's full agent set is known offline, so it must be + // enumerable (wire ListAgents); an instance provider need not be. + if info.Kind == KindCatalog && p.ListAgents == nil { + panic("agent: catalog provider must wire ListAgents: " + scheme) } providerRegistry[scheme] = info } @@ -135,7 +143,7 @@ func Info(scheme string) (ProviderInfo, bool) { // providerFor fetches the factory for a scheme and constructs a Provider. An // unknown scheme returns an error listing the available options. -func providerFor(scheme, agentID string, deps Deps) (Provider, error) { +func providerFor(scheme, agentID string, deps Deps) (*Provider, error) { info, ok := providerRegistry[scheme] if !ok { return nil, fmt.Errorf("未知的 agent provider '%s',当前支持: %s", scheme, KnownSchemes()) @@ -155,7 +163,7 @@ func KnownSchemes() string { } // Resolve parses a ref and constructs the corresponding Provider (command-layer entry point). -func Resolve(ref string, deps Deps) (Provider, error) { +func Resolve(ref string, deps Deps) (*Provider, error) { r, err := ParseRef(ref) if err != nil { return nil, err diff --git a/internal/agent/registry_test.go b/internal/agent/registry_test.go index 27c1a68b4..bb4728949 100644 --- a/internal/agent/registry_test.go +++ b/internal/agent/registry_test.go @@ -4,6 +4,7 @@ package agent import ( + "context" "errors" "reflect" "strings" @@ -20,6 +21,22 @@ func swapRegistry(t *testing.T, m map[string]ProviderInfo) { t.Cleanup(func() { providerRegistry = saved }) } +// okProvider is a minimal valid provider: it wires the two mandatory core fields +// (Send/GetTask) so it passes Register's zero-Deps probe. Tests that need extra +// capabilities set the fields on the returned struct. +func okProvider() *Provider { + return &Provider{ + Send: func(context.Context, SendInput) (*AgentTask, error) { return nil, nil }, + GetTask: func(context.Context, string) (*AgentTask, error) { return nil, nil }, + } +} + +// okFactory returns a Factory yielding okProvider — the default for cases that +// only care about metadata/registry behavior, not capabilities. +func okFactory() Factory { + return func(Deps, string) (*Provider, error) { return okProvider(), nil } +} + // testInfo builds a minimal ProviderInfo that passes Register validation // (AgentRefFormat is generated from the scheme so it satisfies the prefix check), // reused by cases that only care about the Factory. @@ -54,9 +71,9 @@ func mustPanic(t *testing.T, wantMsg string, fn func()) { // on metadata fields: missing Factory / Label / AgentRefFormat / AgentIDSource / // Identities, an invalid Kind, an invalid Identity Type, and an AgentRefFormat // that does not start with ":" (panic messages must carry the actual -// offending value). +// offending value). Metadata validation runs before the probe, so a valid +// okFactory keeps the probe from firing first. func TestRegisterPanicBranches(t *testing.T) { - nop := func(Deps, string) (Provider, error) { return nil, nil } cases := []struct { name string mutate func(info *ProviderInfo) @@ -78,7 +95,7 @@ func TestRegisterPanicBranches(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - info := testInfo("bad", nop) + info := testInfo("bad", okFactory()) tc.mutate(&info) mustPanic(t, tc.wantMsg, func() { Register("bad", info) }) }) @@ -88,39 +105,62 @@ func TestRegisterPanicBranches(t *testing.T) { // TestRegisterEmptyScheme pins the empty-scheme fail-fast branch. func TestRegisterEmptyScheme(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - nop := func(Deps, string) (Provider, error) { return nil, nil } - mustPanic(t, "empty scheme", func() { Register("", testInfo("", nop)) }) + mustPanic(t, "empty scheme", func() { Register("", testInfo("", okFactory())) }) } // TestRegisterDuplicateScheme pins the sql.Register-style dup panic. func TestRegisterDuplicateScheme(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - nop := func(Deps, string) (Provider, error) { return nil, nil } - Register("dup", testInfo("dup", nop)) - mustPanic(t, "called twice for scheme: dup", func() { Register("dup", testInfo("dup", nop)) }) + Register("dup", testInfo("dup", okFactory())) + mustPanic(t, "called twice for scheme: dup", func() { Register("dup", testInfo("dup", okFactory())) }) } // TestRegisterFactoryZeroDepsProbe pins the registration-time zero-Deps probe: // a factory erroring under zero-value Deps is a contract violation and panics. func TestRegisterFactoryZeroDepsProbe(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - bad := func(Deps, string) (Provider, error) { return nil, errors.New("need client") } + bad := func(Deps, string) (*Provider, error) { return nil, errors.New("need client") } mustPanic(t, "must accept zero-value Deps", func() { Register("zd", testInfo("zd", bad)) }) } -// TestRegisterCatalogRequiresDiscoverer pins the catalog-archetype MUST: -// a KindCatalog provider whose probe instance lacks Discoverer panics. -func TestRegisterCatalogRequiresDiscoverer(t *testing.T) { +// TestRegisterNilProvider pins the probe's nil-Provider branch. +func TestRegisterNilProvider(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - info := testInfo("cat", func(Deps, string) (Provider, error) { return &stubProvider{}, nil }) + nilP := func(Deps, string) (*Provider, error) { return nil, nil } + mustPanic(t, "returned nil Provider", func() { Register("np", testInfo("np", nilP)) }) +} + +// TestRegisterMissingCore pins that the mandatory core fields are enforced at +// registration: a provider missing Send or GetTask panics fail-fast. +func TestRegisterMissingCore(t *testing.T) { + swapRegistry(t, map[string]ProviderInfo{}) + noSend := func(Deps, string) (*Provider, error) { + return &Provider{GetTask: func(context.Context, string) (*AgentTask, error) { return nil, nil }}, nil + } + mustPanic(t, "missing core Send", func() { Register("ns", testInfo("ns", noSend)) }) + + swapRegistry(t, map[string]ProviderInfo{}) + noGet := func(Deps, string) (*Provider, error) { + return &Provider{Send: func(context.Context, SendInput) (*AgentTask, error) { return nil, nil }}, nil + } + mustPanic(t, "missing core GetTask", func() { Register("ng", testInfo("ng", noGet)) }) +} + +// TestRegisterCatalogRequiresListAgents pins the catalog-archetype MUST: +// a KindCatalog provider whose probe instance does not wire ListAgents panics. +// The factory wires the core fields so the panic is specifically about ListAgents +// (not a missing-core panic firing first). +func TestRegisterCatalogRequiresListAgents(t *testing.T) { + swapRegistry(t, map[string]ProviderInfo{}) + info := testInfo("cat", okFactory()) // okProvider wires Send/GetTask but not ListAgents info.Kind = KindCatalog - mustPanic(t, "must implement Discoverer", func() { Register("cat", info) }) + mustPanic(t, "must wire ListAgents", func() { Register("cat", info) }) } func TestInfoReturnsRegisteredMetadata(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) Register("t1", ProviderInfo{ - Factory: func(Deps, string) (Provider, error) { return nil, nil }, + Factory: okFactory(), Label: "测试 provider", AgentRefFormat: "t1:", AgentIDSource: "在 T1 控制台获取", @@ -148,10 +188,12 @@ func TestRegistryUnknownScheme(t *testing.T) { func TestRegistryKnownScheme(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - // The factory passes the zero-value Deps probe (empty agentID) and only errors on a real construction, staying compatible with the registration-time probe. - Register("stub", testInfo("stub", func(f Deps, agentID string) (Provider, error) { + // The factory passes the zero-value Deps probe (empty agentID → a valid + // provider) and only errors on a real construction, staying compatible with + // the registration-time probe. + Register("stub", testInfo("stub", func(f Deps, agentID string) (*Provider, error) { if agentID == "" { - return nil, nil + return okProvider(), nil } return nil, errors.New("stub called") })) @@ -170,11 +212,10 @@ func TestKnownSchemesEmpty(t *testing.T) { func TestRegisteredSchemesSorted(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - nop := func(Deps, string) (Provider, error) { return nil, nil } // Register out of order to verify enumeration + sort stability. - Register("gamma", testInfo("gamma", nop)) - Register("alpha", testInfo("alpha", nop)) - Register("beta", testInfo("beta", nop)) + Register("gamma", testInfo("gamma", okFactory())) + Register("alpha", testInfo("alpha", okFactory())) + Register("beta", testInfo("beta", okFactory())) got := RegisteredSchemes() want := []string{"alpha", "beta", "gamma"} if !reflect.DeepEqual(got, want) { @@ -209,10 +250,10 @@ func TestResolveUnknownScheme(t *testing.T) { func TestResolveSuccess(t *testing.T) { swapRegistry(t, map[string]ProviderInfo{}) - sentinel := &stubProvider{} + sentinel := okProvider() var gotDeps Deps var gotAgentID string - Register("demo", testInfo("demo", func(deps Deps, agentID string) (Provider, error) { + Register("demo", testInfo("demo", func(deps Deps, agentID string) (*Provider, error) { gotDeps = deps gotAgentID = agentID return sentinel, nil @@ -232,6 +273,3 @@ func TestResolveSuccess(t *testing.T) { t.Fatalf("factory should receive the passed-in Deps, got %+v", gotDeps) } } - -// stubProvider is an empty Provider implementation used only for Resolve success-path assertions. -type stubProvider struct{ Provider } diff --git a/internal/agent/spi.go b/internal/agent/spi.go index 127123434..36b09f502 100644 --- a/internal/agent/spi.go +++ b/internal/agent/spi.go @@ -3,8 +3,6 @@ package agent -import "context" - // IdentityType is the closed set of values for IdentitySpec.Type (validated at // Register time to guard against typos). type IdentityType string @@ -26,12 +24,3 @@ type AgentSummary struct { Name string `json:"name"` Description string `json:"description,omitempty"` } - -// Discoverer is implemented by providers that can enumerate their own agents -// (required for catalog types, asserted at Register time; instance types -// implement it once the server-side List API is ready). -// The implementer's factory must support construction with zero-value Deps -// (a prerequisite for probing; see the ProviderInfo.Factory contract). -type Discoverer interface { - ListAgents(ctx context.Context) ([]AgentSummary, error) -}