diff --git a/agent/example/example_test.go b/agent/example/example_test.go index 06599b69a..f712147d1 100644 --- a/agent/example/example_test.go +++ b/agent/example/example_test.go @@ -174,6 +174,48 @@ func TestEchoMultiTurn(t *testing.T) { } } +// TestCrossAgentIsolation pins the load-bearing per-agent isolation guard: echo +// and reporter share one package-global store, so a task/context created under +// one agent MUST be invisible to the other agent's runtime (get/delete return a +// not-found error; list returns nothing). Without this guard +// `agent task get example:reporter ` would leak echo's data. +func TestCrossAgentIsolation(t *testing.T) { + swapStore(t) + ctx := context.Background() + echo := fakeRuntime{"echo"} + reporter := fakeRuntime{"reporter"} + + t1, err := echoSend(ctx, echo, agent.SendInput{Text: "secret"}) + if err != nil { + t.Fatalf("echo send: %v", err) + } + + // reporter must not read/delete echo's task or context. + if _, err := getTask(ctx, reporter, t1.TaskID); err == nil { + t.Error("reporter must not read echo's task (cross-agent leak)") + } + if _, err := getContext(ctx, reporter, t1.ContextID); err == nil { + t.Error("reporter must not read echo's context (cross-agent leak)") + } + if err := deleteContext(ctx, reporter, t1.ContextID); err == nil { + t.Error("reporter must not delete echo's context (cross-agent leak)") + } + if tasks, _ := listTasks(ctx, reporter, ""); len(tasks) != 0 { + t.Errorf("reporter should see no echo tasks, got %d", len(tasks)) + } + if ctxs, _ := listContexts(ctx, reporter); len(ctxs) != 0 { + t.Errorf("reporter should see no echo contexts, got %d", len(ctxs)) + } + + // echo still sees its own data, and its context survived reporter's delete. + if _, err := getTask(ctx, echo, t1.TaskID); err != nil { + t.Errorf("echo must still read its own task: %v", err) + } + if _, err := getContext(ctx, echo, t1.ContextID); err != nil { + t.Errorf("echo's context must survive a cross-agent delete attempt: %v", err) + } +} + // TestStateSurvivesReload pins the cross-process semantics via the shared snapshot. func TestStateSurvivesReload(t *testing.T) { swapStore(t) diff --git a/agent/example/state.go b/agent/example/state.go index e90cf301a..d9ec78ca8 100644 --- a/agent/example/state.go +++ b/agent/example/state.go @@ -26,10 +26,9 @@ import ( // naturally stateless; example is a pure mock and must manage state itself. Two // disciplines the integrator needs to know: // -// 1. Concurrency safety: provider instances may be constructed / called -// concurrently (e.g. list's probe alongside the real call), so package-level -// mutable state must be locked. A single coarse-grained Mutex covers all -// reads and writes here — the mock does not chase throughput; correctness comes first. +// 1. Concurrency safety: package-level mutable state must be locked. A single +// coarse-grained Mutex covers all reads and writes here — the mock does not +// chase throughput; correctness comes first. // 2. CLI process boundary: every lark-cli command is a fresh process, so a pure // in-memory map does not survive a single command — after `send`, a // `task get` would find nothing. So a lazy JSON snapshot layer sits beneath @@ -38,9 +37,10 @@ import ( // have this layer — it is a mock-only demo device. // // Note that the snapshot is loaded lazily (only on the first real read/write of -// state): Register's zero-value Deps probe constructs a provider once at -// registration time, and construction must have no side effects (the registry.go -// contract), so Factory / Card / ListAgents must not touch store. +// state): provider registration is a pure declarative Register(Provider) call +// (see agent/register.go) with no construction and no side effects, so nothing +// touches store at registration time — the snapshot is read on the first hook +// invocation, not at init. // ============================================================================ // taskRecord is a task's storage form: a full AgentTask snapshot + owning agent diff --git a/cmd/agent/common.go b/cmd/agent/common.go index 806014e14..57269cc45 100644 --- a/cmd/agent/common.go +++ b/cmd/agent/common.go @@ -83,6 +83,14 @@ func runtimeFor(f *cmdutil.Factory, id core.Identity, agentID string) (iagent.Ru // shape; an unknown scheme points at `agent list` to discover the available // providers. Both hints are copy-pasteable next steps, not just wording. func wrapRefResolveError(err error) error { + // LookupSpec's unknown-catalog-id case is ALREADY a typed validation error + // carrying a scheme-scoped hint (`agent list `); pass it through + // instead of flattening it via err.Error() and overwriting that hint with the + // generic provider-list one. Only the untyped ParseRef sentinel / unknown- + // scheme errors need wrapping. + if _, ok := errs.ProblemOf(err); ok { + return err + } e := errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err.Error()).WithCause(err) if errors.Is(err, iagent.ErrInvalidRef) { return e.WithHint("agent_ref 形如 :，如 example:echo") diff --git a/cmd/agent/context.go b/cmd/agent/context.go index 51d9bcc0a..d4edb64ee 100644 --- a/cmd/agent/context.go +++ b/cmd/agent/context.go @@ -154,6 +154,9 @@ func agentContextListRun(opts *contextOptions) error { // preserves the provider's relative order for equal timestamps, and contexts // with no timestamp sort last. sort.SliceStable(contexts, func(i, j int) bool { return contexts[i].UpdatedAt > contexts[j].UpdatedAt }) + if contexts == nil { + contexts = []iagent.ContextSummary{} // always emit [] not null (matches the Card.Parameters array convention) + } return scanAndEmitData(f, opts.Cmd, opts.Format, map[string]interface{}{"contexts": contexts}, &output.Meta{Count: len(contexts)}, @@ -198,7 +201,7 @@ func agentContextGetRun(opts *contextOptions) error { // agentContextDeleteRun runs `context delete`. The --yes confirmation guard runs // first so a missing confirmation returns confirmation_required (exit 10) before // any provider is built and holds even under a nil Factory. Only a -// confirmed delete reaches resolveProvider + DeleteContext. +// confirmed delete reaches resolveSpec + DeleteContext. func agentContextDeleteRun(opts *contextOptions) error { if !opts.Yes { return cmdutil.RequireConfirmation("agent context delete") diff --git a/cmd/agent/context_test.go b/cmd/agent/context_test.go index 161d23cf8..637c7d9b8 100644 --- a/cmd/agent/context_test.go +++ b/cmd/agent/context_test.go @@ -309,16 +309,53 @@ func TestContextGetInvalidRef(t *testing.T) { } } -// TestContextListWithJq exercises the --jq output branch for list. +// TestContextListWithJq pins the --jq output branch for list: the filtered +// value (not the full envelope) is what reaches stdout. func TestContextListWithJq(t *testing.T) { opts, _ := contextTestOpts(t, "list") opts.Cmd.Flags().String("jq", ".data.contexts | length", "") setScripted(t, scriptedHooks{listContexts: func() ([]iagent.ContextSummary, error) { return []iagent.ContextSummary{{ContextID: "sess_1"}}, nil }}) + out := opts.Factory.IOStreams.Out.(interface{ Bytes() []byte }) if err := agentContextListRun(opts); err != nil { t.Fatalf("context list --jq should not error: %v", err) } + got := strings.TrimSpace(string(out.Bytes())) + if got != "1" { + t.Errorf("--jq .data.contexts | length should output 1, got %q", got) + } + if strings.Contains(got, `"ok"`) { + t.Errorf("--jq output should be the filtered value, not the full envelope, got %q", got) + } +} + +// TestContextListEmptyEmitsArray pins the array convention: an empty context +// list serializes as [] (never null), matching Card.Parameters. +func TestContextListEmptyEmitsArray(t *testing.T) { + opts, _ := contextTestOpts(t, "list") + setScripted(t, scriptedHooks{listContexts: func() ([]iagent.ContextSummary, error) { + return nil, nil + }}) + out := opts.Factory.IOStreams.Out.(interface{ Bytes() []byte }) + if err := agentContextListRun(opts); err != nil { + t.Fatalf("context list should not error: %v", err) + } + var env output.Envelope + if err := json.Unmarshal(out.Bytes(), &env); err != nil { + t.Fatalf("output should be valid envelope JSON: %v (%s)", err, string(out.Bytes())) + } + data, _ := env.Data.(map[string]interface{}) + v, present := data["contexts"] + if !present { + t.Fatal("data.contexts key should be present") + } + if _, ok := v.([]interface{}); !ok { + t.Errorf("empty context list should emit a JSON array (not null), got %T: %v", v, v) + } + if env.Meta == nil || env.Meta.Count != 0 { + t.Errorf("meta.count should be 0, got %+v", env.Meta) + } } // TestContextListPretty exercises the --format pretty human-view branch for diff --git a/cmd/agent/format.go b/cmd/agent/format.go index ec7f5b82c..6cac45005 100644 --- a/cmd/agent/format.go +++ b/cmd/agent/format.go @@ -93,7 +93,7 @@ func printTaskPretty(w io.Writer, task *iagent.AgentTask) { fmt.Fprintln(w, "(no task)") return } - fmt.Fprintf(w, "state: %s\n", task.State) + fmt.Fprintf(w, "state: %s\n", kvValue(string(task.State))) fmt.Fprintf(w, "task_id: %s\n", kvValue(task.TaskID)) if task.ContextID != "" { fmt.Fprintf(w, "context_id: %s\n", kvValue(task.ContextID)) @@ -118,7 +118,7 @@ func printTaskSummariesTSV(w io.Writer, tasks []iagent.TaskSummary) { fmt.Fprintf(w, "TASK_ID\tCONTEXT_ID\tSTATE\tIS_TERMINAL\tUPDATED_AT\tSUMMARY\n") for _, t := range tasks { fmt.Fprintf(w, "%s\t%s\t%s\t%t\t%s\t%s\n", - stripANSI(t.TaskID), stripANSI(t.ContextID), t.State, t.IsTerminal, t.UpdatedAt, kvValue(t.Summary)) + stripANSI(t.TaskID), stripANSI(t.ContextID), stripANSI(string(t.State)), t.IsTerminal, stripANSI(t.UpdatedAt), kvValue(t.Summary)) } } @@ -129,7 +129,7 @@ func printContextsTSV(w io.Writer, contexts []iagent.ContextSummary) { fmt.Fprintf(w, "CONTEXT_ID\tCREATED_AT\tUPDATED_AT\tTITLE\tTASK_COUNT\tAWAITING_INPUT\n") for _, c := range contexts { fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\t%t\n", - stripANSI(c.ContextID), c.CreatedAt, c.UpdatedAt, stripANSI(c.Title), c.TaskCount, c.AwaitingInput) + stripANSI(c.ContextID), stripANSI(c.CreatedAt), stripANSI(c.UpdatedAt), stripANSI(c.Title), c.TaskCount, c.AwaitingInput) } } @@ -147,10 +147,10 @@ func printContextDetailPretty(w io.Writer, detail *iagent.ContextDetail) { } fmt.Fprintf(w, "context_id: %s\n", kvValue(detail.ContextID)) if detail.CreatedAt != "" { - fmt.Fprintf(w, "created_at: %s\n", detail.CreatedAt) + fmt.Fprintf(w, "created_at: %s\n", kvValue(detail.CreatedAt)) } if detail.UpdatedAt != "" { - fmt.Fprintf(w, "updated_at: %s\n", detail.UpdatedAt) + fmt.Fprintf(w, "updated_at: %s\n", kvValue(detail.UpdatedAt)) } if detail.Title != "" { fmt.Fprintf(w, "title: %s\n", kvValue(detail.Title)) @@ -158,7 +158,7 @@ func printContextDetailPretty(w io.Writer, detail *iagent.ContextDetail) { fmt.Fprintf(w, "task_count: %d\n", detail.TaskCount) fmt.Fprintf(w, "awaiting_input: %t\n", detail.AwaitingInput) if at := detail.ActiveTask; at != nil { - fmt.Fprintf(w, "active_task: %s · %s · %s\n", at.State, at.UpdatedAt, kvValue(at.Summary)) + fmt.Fprintf(w, "active_task: %s · %s · %s\n", kvValue(string(at.State)), kvValue(at.UpdatedAt), kvValue(at.Summary)) } } diff --git a/cmd/agent/list.go b/cmd/agent/list.go index 50187ad84..a156d70c3 100644 --- a/cmd/agent/list.go +++ b/cmd/agent/list.go @@ -15,7 +15,7 @@ import ( ) // providerInfo describes a registered provider adapter in `agent list` output. -// Every field is sourced from the registered iagent.ProviderInfo (the single +// Every field is sourced from the registered iagent.Provider (the single // source of truth). type providerInfo struct { Scheme string `json:"scheme"` @@ -36,9 +36,9 @@ type listOptions struct { // NewCmdAgentList builds `agent list [scheme]`. Without an argument it // enumerates the registered provider adapters with their metadata — a // pure, API-free listing. With a scheme it performs second-level discovery: -// providers implementing Discoverer enumerate their agents; -// others return unsupported_capability with the agent_id_source -// guidance. Risk=read. +// catalog providers enumerate offline from their static set; instance providers +// enumerate via their optional ListAgents hook (absent ⇒ unsupported_capability +// with the agent_id_source guidance). Risk=read. func NewCmdAgentList(f *cmdutil.Factory) *cobra.Command { opts := &listOptions{Factory: f} cmd := &cobra.Command{ @@ -120,6 +120,7 @@ func agentListSchemeRun(opts *listOptions) error { } var agents []iagent.AgentSummary + var identity string // set only on the online (instance) path, which resolves one if prov.Kind() == iagent.KindCatalog { agents = prov.ListCatalog() // offline } else { @@ -132,6 +133,7 @@ func agentListSchemeRun(opts *listOptions) error { // The enumeration call carries the resolved identity; agentID is empty // (enumeration is not scoped to a single agent). id := f.ResolveAs(opts.Cmd.Context(), opts.Cmd, "") + identity = string(id) rt, err := runtimeFor(f, id, "") if err != nil { return err @@ -141,6 +143,9 @@ func agentListSchemeRun(opts *listOptions) error { return err } } + if agents == nil { + agents = []iagent.AgentSummary{} // always emit [] not null + } // pretty is a human view only; a --jq expression implies structured JSON. if opts.Format == "pretty" && jqExpr(opts.Cmd) == "" { @@ -154,10 +159,11 @@ func agentListSchemeRun(opts *listOptions) error { } env := output.Envelope{ - OK: true, - Data: map[string]interface{}{"agents": agents}, - Meta: &output.Meta{Count: len(agents)}, - Notice: output.GetNotice(), + OK: true, + Identity: identity, // empty for the offline catalog path (omitempty) + Data: map[string]interface{}{"agents": agents}, + Meta: &output.Meta{Count: len(agents)}, + Notice: output.GetNotice(), } if jq := jqExpr(opts.Cmd); jq != "" { return output.JqFilter(f.IOStreams.Out, env, jq) diff --git a/cmd/agent/preflight.go b/cmd/agent/preflight.go index 99d5c522d..826bb6cc3 100644 --- a/cmd/agent/preflight.go +++ b/cmd/agent/preflight.go @@ -135,7 +135,7 @@ func scopeRemediationHint(id core.Identity, missing []string) string { // seam, and runs the all-or-nothing preflight. Any gap in its own inputs (nil // Factory, unparsable ref, unregistered scheme) yields nil — the preflight is // an accelerator, never a new failure mode; the paths that validate ref/scheme -// for real have already run inside resolveProvider. +// for real have already run inside resolveSpec. func preflightScopesForRef(f *cmdutil.Factory, id core.Identity, ref string) error { if f == nil { return nil diff --git a/cmd/agent/task.go b/cmd/agent/task.go index 8e04cc831..f74ee1cd8 100644 --- a/cmd/agent/task.go +++ b/cmd/agent/task.go @@ -245,6 +245,14 @@ func agentTaskGetRun(opts *taskOptions) error { if err != nil { return err } + // A provider that decodes an empty "data" via Call[*AgentTask] legitimately + // returns (nil, nil) (see internal/agent decodeData). Surface that as a typed + // error rather than dereferencing task.State below (the --watch branch would + // otherwise panic; the sibling consumers all nil-guard). + if task == nil { + return errs.NewInternalError(errs.SubtypeInvalidResponse, + "provider 未返回任务数据（响应无 data）") + } if opts.Watch && !task.State.ShouldStopPolling() { // A positive --timeout bounds the poll: pollToStop returns the most recent @@ -315,6 +323,9 @@ func agentTaskListRun(opts *taskOptions) error { // recently active task heads the list; a stable sort preserves the provider's // relative order for equal timestamps, and tasks with no timestamp sort last. sort.SliceStable(tasks, func(i, j int) bool { return tasks[i].UpdatedAt > tasks[j].UpdatedAt }) + if tasks == nil { + tasks = []iagent.TaskSummary{} // always emit [] not null (matches the Card.Parameters array convention) + } return scanAndEmitData(f, opts.Cmd, opts.Format, map[string]interface{}{"tasks": tasks}, &output.Meta{Count: len(tasks)}, @@ -397,6 +408,19 @@ func downloadArtifact(opts *taskOptions) error { if err != nil { return err } + // A provider decoding an empty "data" via Call[*ArtifactData] can return + // (nil, nil); and a non-nil descriptor with neither inline bytes nor a URL + // carries no downloadable content. Both are provider-response defects — fail + // with a typed error instead of dereferencing nil or writing a 0-byte file + // (which under --force would clobber an existing local file with emptiness). + if art == nil { + return errs.NewInternalError(errs.SubtypeInvalidResponse, + "provider 未返回产物数据（响应无 data）") + } + if len(art.Bytes) == 0 && art.URL == "" { + return errs.NewInternalError(errs.SubtypeInvalidResponse, + "产物 '%s' 无可下载内容（provider 既未提供内联字节也未提供下载 URL）", opts.ArtifactID) + } data := art.Bytes if art.URL != "" { diff --git a/cmd/agent/task_test.go b/cmd/agent/task_test.go index 0da069737..4d9215857 100644 --- a/cmd/agent/task_test.go +++ b/cmd/agent/task_test.go @@ -279,6 +279,34 @@ func TestTaskListEmitsCount(t *testing.T) { } } +// TestTaskListEmptyEmitsArray pins the array convention: an empty task list +// serializes as [] (never null), matching Card.Parameters. +func TestTaskListEmptyEmitsArray(t *testing.T) { + opts, _ := taskTestOpts(t, "list") + setScripted(t, scriptedHooks{listTasks: func(string) ([]iagent.TaskSummary, error) { + return nil, nil + }}) + out := opts.Factory.IOStreams.Out.(interface{ Bytes() []byte }) + if err := agentTaskListRun(opts); err != nil { + t.Fatalf("list should not error: %v", err) + } + var env output.Envelope + if err := json.Unmarshal(out.Bytes(), &env); err != nil { + t.Fatalf("output should be valid envelope JSON: %v (%s)", err, string(out.Bytes())) + } + data, _ := env.Data.(map[string]interface{}) + v, present := data["tasks"] + if !present { + t.Fatal("data.tasks key should be present") + } + if _, ok := v.([]interface{}); !ok { + t.Errorf("empty task list should emit a JSON array (not null), got %T: %v", v, v) + } + if env.Meta == nil || env.Meta.Count != 0 { + t.Errorf("meta.count should be 0, got %+v", env.Meta) + } +} + // TestTaskListError surfaces a provider ListTasks failure. func TestTaskListError(t *testing.T) { opts, _ := taskTestOpts(t, "list") @@ -505,6 +533,28 @@ func TestTaskListPrettyFormat(t *testing.T) { } } +// TestTaskListPrettySanitizesStateAndTimestamp pins that the agent-controlled +// State and UpdatedAt fields are ANSI-stripped on the pretty/TSV path, not just +// the ids/summary — a malicious provider must not inject terminal escapes via a +// forged state string or timestamp. +func TestTaskListPrettySanitizesStateAndTimestamp(t *testing.T) { + opts, _ := taskTestOpts(t, "list") + opts.Format = "pretty" + setScripted(t, scriptedHooks{listTasks: func(string) ([]iagent.TaskSummary, error) { + return []iagent.TaskSummary{ + {TaskID: "chat_1", State: iagent.TaskState("completed\x1b[2J"), IsTerminal: true, + UpdatedAt: "2026-07-05T12:00:00Z\x1b]0;pwned\x07"}, + }, nil + }}) + out := opts.Factory.IOStreams.Out.(interface{ Bytes() []byte }) + if err := agentTaskListRun(opts); err != nil { + t.Fatalf("task list --format pretty should not error: %v", err) + } + if text := string(out.Bytes()); strings.Contains(text, "\x1b") { + t.Errorf("ANSI/OSC sequences in State/UpdatedAt must be stripped, got %q", text) + } +} + // TestTaskListSortedByUpdatedAtDesc pins the ordering + enriched-field // contract: the provider returns tasks out of order, and the command emits them // newest-first by updated_at while carrying updated_at + summary on each. diff --git a/internal/agent/card.go b/internal/agent/card.go index c2372b549..a04a83d2f 100644 --- a/internal/agent/card.go +++ b/internal/agent/card.go @@ -134,11 +134,11 @@ type CardSkill struct { // Supports reports whether a capability is declared as supported (an unknown key // or a nil card is treated as unsupported). -func (c *AgentCard) Supports(cap string) bool { +func (c *AgentCard) Supports(capKey string) bool { if c == nil { return false } - switch cap { + switch capKey { case CapArtifactDownload: return c.Capabilities.ArtifactDownload case CapFileInput: