diff --git a/docs/tools/plan/index.md b/docs/tools/plan/index.md index 4efecc069..32d06feff 100644 --- a/docs/tools/plan/index.md +++ b/docs/tools/plan/index.md @@ -25,12 +25,44 @@ No additional options are required. All agents that include `type: plan` in thei ## Available Tools -| Tool | Description | -| ------------- | ------------------------------------------------------------------------------------------------- | -| `write_plan` | Create or update a shared plan by name. Replaces the entire plan content — read it first to preserve what you want to keep. Each write bumps the revision number. | -| `read_plan` | Read a shared plan by name, including its title, content, author, revision number, and last-updated timestamp. | -| `list_plans` | List all shared plans with their name, title, author, revision, and last-updated timestamp. | -| `delete_plan` | Delete a shared plan by name. | +| Tool | Description | +| ----------------------- | ------------------------------------------------------------------------------------------------- | +| `write_plan` | Create or update a shared plan by name. Replaces the entire plan content — read it first to preserve what you want to keep. Each write bumps the revision number. | +| `read_plan` | Read a shared plan by name, including its title, content, author, status, revision number, and last-updated timestamp. | +| `list_plans` | List all shared plans with their name, title, author, status, revision, and last-updated timestamp. | +| `delete_plan` | Delete a shared plan by name. | +| `update_plan_from_file` | Create or update a plan, taking the new content from a file on disk instead of inline. Use it with `export_plan_to_file` to edit a large plan without re-sending its whole body. | +| `export_plan_to_file` | Write a plan's content to a file. The content goes to disk and is **not** returned as tool output, so materialising a plan costs no tokens. | +| `set_plan_status` | Set a plan's free-form status without rewriting its body. The plan must already exist. | +| `get_plan_status` | Read a plan's status and current revision without fetching its body. | + +### Cheap edits with file-based revisions + +Re-sending a whole plan on every revision is expensive. The file-based tools let +an agent edit a plan without paying input-token cost for its body: + +1. `export_plan_to_file` writes the current plan content to a path. The content + is written to disk and is **not** returned. +2. The agent edits that file in place with its filesystem tools. +3. `update_plan_from_file` commits the file's new contents as the next revision. + +### Free-form status + +Each plan carries a free-form `status` string. There is no fixed vocabulary: +define your own in the system prompt (e.g. `idle`, `in-progress`, `blocked`, +`done`, `canceled`). Read and write it independently of the body with +`get_plan_status` and `set_plan_status`, or pass `status` to `write_plan` and +`update_plan_from_file`. The TUI surfaces the status next to the plan title. + +### Optimistic locking + +When several sessions edit the same plan, concurrent writes could silently +overwrite each other. Every read returns a `revision` number; pass the value you +last read as `last_known_revision` to `write_plan`, `update_plan_from_file`, +`set_plan_status`, or `delete_plan`. If the plan changed since (its current +revision no longer matches), the write is rejected with a version-conflict +error and the caller should re-read the plan and retry. Omit +`last_known_revision` to write unconditionally (last writer wins). ### Plan Names @@ -46,7 +78,8 @@ Each plan document contains: | `title` | A short human-readable title (optional) | | `content` | The full Markdown or free-form plan text | | `author` | Free-form label identifying who last wrote the plan | -| `revision` | Monotonically increasing counter, bumped on every write | +| `status` | Free-form lifecycle label (optional), e.g. `in-progress` | +| `revision` | Monotonically increasing version counter, bumped on every write | | `updatedAt`| ISO 8601 timestamp of the last write | ## Example diff --git a/examples/shared_plan.yaml b/examples/shared_plan.yaml index 9809269ee..68287c37c 100644 --- a/examples/shared_plan.yaml +++ b/examples/shared_plan.yaml @@ -26,8 +26,12 @@ agents: You are a software architect. Use list_plans to see existing plans and read_plan to inspect one, then use write_plan to create or revise a plan by name. Always read before writing so you preserve content you want to - keep. Set the title and use "architect" as the author. When the - high-level plan is ready, hand off to the builder. + keep. Set the title and use "architect" as the author. Note the revision + you read and pass it as last_known_revision when you write, so a + concurrent edit by the builder is detected instead of silently lost; on a + conflict, re-read and retry. Use set_plan_status to mark the plan + "drafting" while you work and "ready-for-build" when the high-level plan + is done, then hand off to the builder. toolsets: - type: plan handoffs: @@ -37,12 +41,15 @@ agents: model: openai/gpt-4o description: Reviews plans and adds concrete implementation steps instruction: | - You are an implementation engineer. Use read_plan to read the architect's - plan, then use write_plan to append concrete, actionable implementation - steps. Always read before writing so you don't lose the architect's work. - Use "builder" as the author. Delete obsolete plans with delete_plan. When - done, hand off back to root. + You are an implementation engineer. For large plans, edit cheaply: call + export_plan_to_file to write the current plan to a scratch file, edit that + file in place to add concrete implementation steps, then call + update_plan_from_file to commit it (passing last_known_revision so you + don't clobber a concurrent edit). Use "builder" as the author and + set_plan_status to "done" when the plan is complete. Delete obsolete plans + with delete_plan. When done, hand off back to root. toolsets: - type: plan + - type: filesystem handoffs: - root diff --git a/pkg/tools/builtin/plan/plan.go b/pkg/tools/builtin/plan/plan.go index 99f1082cb..5ab5d8777 100644 --- a/pkg/tools/builtin/plan/plan.go +++ b/pkg/tools/builtin/plan/plan.go @@ -23,6 +23,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "regexp" @@ -37,12 +38,41 @@ import ( ) const ( - ToolNameWritePlan = "write_plan" - ToolNameReadPlan = "read_plan" - ToolNameListPlans = "list_plans" - ToolNameDeletePlan = "delete_plan" + ToolNameWritePlan = "write_plan" + ToolNameReadPlan = "read_plan" + ToolNameListPlans = "list_plans" + ToolNameDeletePlan = "delete_plan" + ToolNameUpdatePlanFromFile = "update_plan_from_file" + ToolNameExportPlanToFile = "export_plan_to_file" + ToolNameSetPlanStatus = "set_plan_status" + ToolNameGetPlanStatus = "get_plan_status" ) +// maxPlanFileSize caps how much update_plan_from_file will read from disk, so a +// pathological or wrong path cannot make the agent pull an arbitrarily large +// file into a plan (and into the model's context). 10 MiB is far above any +// realistic plan while still bounding memory. +const maxPlanFileSize = 10 << 20 + +// ErrPlanNotFound is returned by write operations that require an existing plan +// (set_plan_status) when the named plan does not exist, so callers can tell a +// missing plan apart from a real backend failure. +var ErrPlanNotFound = errors.New("plan not found") + +// VersionConflictError is returned by a write when the caller's +// last_known_revision does not match the plan's current revision, signalling +// that another writer changed the plan in the meantime. The caller should +// re-read the plan and retry against the new revision. +type VersionConflictError struct { + Name string + Expected int + Current int +} + +func (e *VersionConflictError) Error() string { + return fmt.Sprintf("version conflict on plan %q: last_known_revision %d does not match current revision %d; re-read the plan and retry", e.Name, e.Expected, e.Current) +} + // namePattern defines the accepted plan-name format: a lowercase slug made of // alphanumerics, '-' and '_'. Names are validated against it rather than being // silently rewritten, so two different inputs can never collapse onto the same @@ -58,7 +88,15 @@ type Plan struct { // Author is a free-form label identifying who last wrote the plan // (typically the agent name). It helps collaborators see who made the // most recent change. - Author string `json:"author,omitempty"` + Author string `json:"author,omitempty"` + // Status is a free-form lifecycle label (e.g. "idle", "in-progress", + // "blocked", "done"). There is no fixed vocabulary: agents and users define + // their own in the system prompt. + Status string `json:"status,omitempty"` + // Revision is the plan's monotonically increasing version number, bumped on + // every write. Callers capture it on a read and pass it back as + // last_known_revision to detect concurrent modifications (optimistic + // locking). Revision int `json:"revision"` UpdatedAt string `json:"updatedAt"` } @@ -68,10 +106,50 @@ type Summary struct { Name string `json:"name"` Title string `json:"title,omitempty"` Author string `json:"author,omitempty"` + Status string `json:"status,omitempty"` Revision int `json:"revision"` UpdatedAt string `json:"updatedAt"` } +// StatusView is the lightweight result of set_plan_status and get_plan_status: +// just the status and the revision, without the plan body, so reading or +// writing the status never costs the tokens of the full content. +type StatusView struct { + Name string `json:"name"` + Status string `json:"status"` + Revision int `json:"revision"` +} + +// ExportResult is the result of export_plan_to_file. It deliberately omits the +// plan content: the content is written to disk, not returned, so materialising +// a plan on disk before an edit costs no output tokens. +type ExportResult struct { + Name string `json:"name"` + Path string `json:"path"` + Title string `json:"title,omitempty"` + Status string `json:"status,omitempty"` + Revision int `json:"revision"` + BytesWritten int `json:"bytesWritten"` +} + +// UpsertRequest describes a create-or-update operation on a plan. Content, +// Title, Author and Status are pointers so an omitted field (nil) preserves the +// previous value while an explicit value (including "") overwrites it; this lets +// a caller change just the status without rewriting the body. ExpectedRevision +// enables optimistic locking: when non-nil, the write is rejected with a +// *VersionConflictError unless it equals the plan's current revision. MustExist +// makes the write fail with ErrPlanNotFound when the plan does not already +// exist (used by set_plan_status, which must not create a plan). +type UpsertRequest struct { + Name string + Content *string + Title *string + Author *string + Status *string + ExpectedRevision *int + MustExist bool +} + // ListResult is the output of list_plans. Warnings lists plan files that could // not be read or decoded, so a caller can tell "no plans exist" apart from // "some plans failed to load" — important because an agent that mistakes a @@ -90,18 +168,22 @@ type Storage interface { // such plan exists, so callers can tell a missing plan apart from a real // read failure (returned as a non-nil error). Get(ctx context.Context, name string) (Plan, bool, error) - // Upsert creates or replaces the named plan's content and, when non-empty, - // its title and author; omitted title/author are preserved from the - // previous revision. It bumps the revision and stamps UpdatedAt, returning - // the stored plan. - Upsert(ctx context.Context, name, content, title, author string) (Plan, error) + // Upsert creates or updates a plan as described by req: nil fields are + // preserved from the previous revision, the optimistic-lock check and the + // must-exist guard are honoured atomically with the write, the revision is + // bumped and UpdatedAt stamped. It returns a *VersionConflictError on a + // revision mismatch and ErrPlanNotFound when req.MustExist is set but the + // plan is absent. + Upsert(ctx context.Context, req UpsertRequest) (Plan, error) // List returns a summary of every stored plan. Warnings carries entries // that could not be read, so a caller can tell "no plans" apart from "some // plans failed to load". List(ctx context.Context) (plans []Summary, warnings []string, err error) // Delete removes the named plan. The bool is false with a nil error when - // there was no such plan to delete. - Delete(ctx context.Context, name string) (deleted bool, err error) + // there was no such plan to delete. When expectedRevision is non-nil the + // delete is rejected with a *VersionConflictError unless it matches the + // plan's current revision. + Delete(ctx context.Context, name string, expectedRevision *int) (deleted bool, err error) } type ToolSet struct { @@ -190,15 +272,50 @@ global shared folder, so every agent using this toolset sees the same plans. latest change. Each write bumps the revision number. - Use delete_plan to remove a plan once it is no longer needed. +### Editing a large plan cheaply + +Re-sending the whole plan body on every edit is expensive. To avoid it: + +1. Call export_plan_to_file to write the current plan content to a file. The + content is written to disk and is NOT returned, so this costs no tokens. +2. Edit that file in place with your filesystem tools. +3. Call update_plan_from_file with the same path to commit the new content. + +### Status + +Each plan has a free-form status string (you define the vocabulary, e.g. +"idle", "in-progress", "blocked", "done"). Use set_plan_status and +get_plan_status to write and read it independently of the body, or pass status +to write_plan / update_plan_from_file. + +### Concurrent edits (optimistic locking) + +Reads return a revision number. When several sessions edit one plan, pass the +revision you last read as last_known_revision to write_plan, update_plan_from_file, +set_plan_status or delete_plan. If the plan changed in the meantime the write is +rejected with a version-conflict error: re-read the plan and retry. + Plan names must be lowercase and may contain only letters, digits, '-' and '_' (for example "release-2025" or "db_migration").` } +// optStr maps an omitted (empty) tool argument to a nil pointer, so an UpsertRequest +// preserves the previous value rather than overwriting it with "". A non-empty +// value is passed through as an explicit set. +func optStr(s string) *string { + if s == "" { + return nil + } + return &s +} + type WritePlanArgs struct { - Name string `json:"name" jsonschema:"The plan name. Lowercase letters, digits, '-' and '_' only (e.g. 'release', 'db-migration')."` - Content string `json:"content" jsonschema:"The full plan content (markdown). Replaces the existing plan."` - Title string `json:"title,omitempty" jsonschema:"Optional human-readable title. Preserved from the previous revision when omitted."` - Author string `json:"author,omitempty" jsonschema:"Optional label identifying who is writing the plan (typically the agent name). Preserved from the previous revision when omitted."` + Name string `json:"name" jsonschema:"The plan name. Lowercase letters, digits, '-' and '_' only (e.g. 'release', 'db-migration')."` + Content string `json:"content" jsonschema:"The full plan content (markdown). Replaces the existing plan."` + Title string `json:"title,omitempty" jsonschema:"Optional human-readable title. Preserved from the previous revision when omitted."` + Author string `json:"author,omitempty" jsonschema:"Optional label identifying who is writing the plan (typically the agent name). Preserved from the previous revision when omitted."` + Status string `json:"status,omitempty" jsonschema:"Optional free-form lifecycle status (e.g. 'in-progress', 'blocked', 'done'). Preserved from the previous revision when omitted."` + LastKnownRevision *int `json:"last_known_revision,omitempty" jsonschema:"Optional optimistic-lock guard. When set, the write is rejected with a conflict error unless it matches the plan's current revision. Obtain it by reading the plan first."` } func (t *ToolSet) writePlan(ctx context.Context, params WritePlanArgs) (*tools.ToolCallResult, error) { @@ -206,7 +323,14 @@ func (t *ToolSet) writePlan(ctx context.Context, params WritePlanArgs) (*tools.T return tools.ResultError("content must not be empty"), nil } - plan, err := t.storage.Upsert(ctx, params.Name, params.Content, params.Title, params.Author) + plan, err := t.storage.Upsert(ctx, UpsertRequest{ + Name: params.Name, + Content: ¶ms.Content, + Title: optStr(params.Title), + Author: optStr(params.Author), + Status: optStr(params.Status), + ExpectedRevision: params.LastKnownRevision, + }) if err != nil { return tools.ResultError(err.Error()), nil } @@ -214,6 +338,115 @@ func (t *ToolSet) writePlan(ctx context.Context, params WritePlanArgs) (*tools.T return tools.ResultJSON(plan), nil } +type UpdatePlanFromFileArgs struct { + Name string `json:"name" jsonschema:"The plan name. Lowercase letters, digits, '-' and '_' only."` + Path string `json:"path" jsonschema:"Path to a file whose contents become the new plan content. Lets you commit edits without re-sending the full plan body."` + Title string `json:"title,omitempty" jsonschema:"Optional human-readable title. Preserved from the previous revision when omitted."` + Author string `json:"author,omitempty" jsonschema:"Optional label identifying who is writing the plan. Preserved from the previous revision when omitted."` + Status string `json:"status,omitempty" jsonschema:"Optional free-form lifecycle status. Preserved from the previous revision when omitted."` + LastKnownRevision *int `json:"last_known_revision,omitempty" jsonschema:"Optional optimistic-lock guard. When set, the write is rejected with a conflict error unless it matches the plan's current revision."` +} + +func (t *ToolSet) updatePlanFromFile(ctx context.Context, params UpdatePlanFromFileArgs) (*tools.ToolCallResult, error) { + if params.Path == "" { + return tools.ResultError("path must not be empty"), nil + } + + content, err := readPlanFile(params.Path) + if err != nil { + return tools.ResultError(err.Error()), nil + } + if content == "" { + return tools.ResultError("file is empty; plan content must not be empty"), nil + } + + plan, err := t.storage.Upsert(ctx, UpsertRequest{ + Name: params.Name, + Content: &content, + Title: optStr(params.Title), + Author: optStr(params.Author), + Status: optStr(params.Status), + ExpectedRevision: params.LastKnownRevision, + }) + if err != nil { + return tools.ResultError(err.Error()), nil + } + + return tools.ResultJSON(plan), nil +} + +type ExportPlanToFileArgs struct { + Name string `json:"name" jsonschema:"The name of the plan to export."` + Path string `json:"path" jsonschema:"Path to write the plan content to. The content is written to disk and is NOT returned as tool output, so you can materialise the plan cheaply before editing it."` +} + +func (t *ToolSet) exportPlanToFile(ctx context.Context, params ExportPlanToFileArgs) (*tools.ToolCallResult, error) { + if params.Path == "" { + return tools.ResultError("path must not be empty"), nil + } + + plan, ok, err := t.storage.Get(ctx, params.Name) + if err != nil { + return tools.ResultError(err.Error()), nil + } + if !ok { + return tools.ResultError(fmt.Sprintf("plan %q not found", params.Name)), nil + } + + if err := writePlanFile(params.Path, plan.Content); err != nil { + return tools.ResultError(err.Error()), nil + } + + return tools.ResultJSON(ExportResult{ + Name: params.Name, + Path: params.Path, + Title: plan.Title, + Status: plan.Status, + Revision: plan.Revision, + BytesWritten: len(plan.Content), + }), nil +} + +type SetPlanStatusArgs struct { + Name string `json:"name" jsonschema:"The name of the plan whose status to set."` + Status string `json:"status" jsonschema:"The new free-form status string (e.g. 'in-progress', 'blocked', 'done'). Defined by your own vocabulary."` + LastKnownRevision *int `json:"last_known_revision,omitempty" jsonschema:"Optional optimistic-lock guard. When set, the write is rejected with a conflict error unless it matches the plan's current revision."` +} + +func (t *ToolSet) setPlanStatus(ctx context.Context, params SetPlanStatusArgs) (*tools.ToolCallResult, error) { + if params.Status == "" { + return tools.ResultError("status must not be empty"), nil + } + + plan, err := t.storage.Upsert(ctx, UpsertRequest{ + Name: params.Name, + Status: ¶ms.Status, + MustExist: true, + ExpectedRevision: params.LastKnownRevision, + }) + if err != nil { + return tools.ResultError(err.Error()), nil + } + + return tools.ResultJSON(StatusView{Name: plan.Name, Status: plan.Status, Revision: plan.Revision}), nil +} + +type GetPlanStatusArgs struct { + Name string `json:"name" jsonschema:"The name of the plan whose status to read."` +} + +func (t *ToolSet) getPlanStatus(ctx context.Context, params GetPlanStatusArgs) (*tools.ToolCallResult, error) { + plan, ok, err := t.storage.Get(ctx, params.Name) + if err != nil { + return tools.ResultError(err.Error()), nil + } + if !ok { + return tools.ResultError(fmt.Sprintf("plan %q not found", params.Name)), nil + } + + return tools.ResultJSON(StatusView{Name: params.Name, Status: plan.Status, Revision: plan.Revision}), nil +} + type ReadPlanArgs struct { Name string `json:"name" jsonschema:"The name of the plan to read."` } @@ -245,11 +478,12 @@ func (t *ToolSet) listPlans(ctx context.Context, _ tools.ToolCall) (*tools.ToolC } type DeletePlanArgs struct { - Name string `json:"name" jsonschema:"The name of the plan to delete."` + Name string `json:"name" jsonschema:"The name of the plan to delete."` + LastKnownRevision *int `json:"last_known_revision,omitempty" jsonschema:"Optional optimistic-lock guard. When set, the delete is rejected with a conflict error unless it matches the plan's current revision."` } func (t *ToolSet) deletePlan(ctx context.Context, params DeletePlanArgs) (*tools.ToolCallResult, error) { - deleted, err := t.storage.Delete(ctx, params.Name) + deleted, err := t.storage.Delete(ctx, params.Name, params.LastKnownRevision) if err != nil { return tools.ResultError(err.Error()), nil } @@ -260,6 +494,73 @@ func (t *ToolSet) deletePlan(ctx context.Context, params DeletePlanArgs) (*tools return tools.ResultJSON(map[string]string{"deleted": params.Name}), nil } +// readPlanFile reads plan content from a file for update_plan_from_file. It +// rejects directories, non-regular files, and oversized files up front so a +// wrong path fails with a clear message instead of pulling unexpected data into +// a plan. The non-regular check matters for safety as well as clarity: a device +// (e.g. /dev/zero) or a named pipe reports size 0 from stat yet would stream +// unbounded data or block forever if opened, so it is rejected before any open. +// The read itself goes through an io.LimitReader rather than trusting the stat +// size, which closes the race where the file grows between stat and read. +func readPlanFile(path string) (string, error) { + clean := filepath.Clean(path) + + info, err := os.Stat(clean) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("file %q does not exist", path) + } + return "", fmt.Errorf("reading plan file: %w", err) + } + if info.IsDir() { + return "", fmt.Errorf("path %q is a directory, not a file", path) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("path %q is not a regular file (e.g. a device or named pipe)", path) + } + if info.Size() > maxPlanFileSize { + return "", fmt.Errorf("file %q is too large (%d bytes; max %d)", path, info.Size(), maxPlanFileSize) + } + + f, err := os.Open(clean) + if err != nil { + return "", fmt.Errorf("reading plan file: %w", err) + } + defer f.Close() + + // Read one byte past the cap so an over-cap file is detected even if it grew + // since the stat above. + data, err := io.ReadAll(io.LimitReader(f, maxPlanFileSize+1)) + if err != nil { + return "", fmt.Errorf("reading plan file: %w", err) + } + if len(data) > maxPlanFileSize { + return "", fmt.Errorf("file %q is too large (max %d bytes)", path, maxPlanFileSize) + } + return string(data), nil +} + +// writePlanFile writes plan content to a file for export_plan_to_file. It +// creates any missing parent directories and writes atomically (temp + rename), +// so a reader never sees a partial export and the agent can point at a fresh +// path without first creating its directory. +func writePlanFile(path, content string) error { + clean := filepath.Clean(path) + + if info, err := os.Stat(clean); err == nil && info.IsDir() { + return fmt.Errorf("path %q is a directory, not a file", path) + } + if dir := filepath.Dir(clean); dir != "" { + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("creating directory for plan file: %w", err) + } + } + if err := atomicfile.Write(clean, strings.NewReader(content), 0o600); err != nil { + return fmt.Errorf("writing plan file: %w", err) + } + return nil +} + func (t *ToolSet) Tools(context.Context) ([]tools.Tool, error) { return []tools.Tool{ { @@ -276,7 +577,7 @@ func (t *ToolSet) Tools(context.Context) ([]tools.Tool, error) { { Name: ToolNameReadPlan, Category: "plan", - Description: "Read a shared plan by name, including its title, content, author, and revision number.", + Description: "Read a shared plan by name, including its title, content, author, status, and revision number.", Parameters: tools.MustSchemaFor[ReadPlanArgs](), OutputSchema: tools.MustSchemaFor[Plan](), Handler: tools.NewHandler(t.readPlan), @@ -285,10 +586,55 @@ func (t *ToolSet) Tools(context.Context) ([]tools.Tool, error) { ReadOnlyHint: true, }, }, + { + Name: ToolNameUpdatePlanFromFile, + Category: "plan", + Description: "Create or update a shared plan, taking the new content from a file on disk instead of inline. Use it together with export_plan_to_file to edit a large plan without re-sending its whole body. Each write bumps the revision number.", + Parameters: tools.MustSchemaFor[UpdatePlanFromFileArgs](), + OutputSchema: tools.MustSchemaFor[Plan](), + Handler: tools.NewHandler(t.updatePlanFromFile), + Annotations: tools.ToolAnnotations{ + Title: "Update Plan From File", + }, + }, + { + Name: ToolNameExportPlanToFile, + Category: "plan", + Description: "Write a shared plan's content to a file on disk. The content is written to the file and is NOT returned, so you can materialise a plan cheaply before editing it and committing the result with update_plan_from_file.", + Parameters: tools.MustSchemaFor[ExportPlanToFileArgs](), + OutputSchema: tools.MustSchemaFor[ExportResult](), + Handler: tools.NewHandler(t.exportPlanToFile), + Annotations: tools.ToolAnnotations{ + Title: "Export Plan To File", + }, + }, + { + Name: ToolNameSetPlanStatus, + Category: "plan", + Description: "Set a shared plan's free-form status (e.g. 'in-progress', 'blocked', 'done') without rewriting its body. The plan must already exist. Each call bumps the revision number.", + Parameters: tools.MustSchemaFor[SetPlanStatusArgs](), + OutputSchema: tools.MustSchemaFor[StatusView](), + Handler: tools.NewHandler(t.setPlanStatus), + Annotations: tools.ToolAnnotations{ + Title: "Set Plan Status", + }, + }, + { + Name: ToolNameGetPlanStatus, + Category: "plan", + Description: "Read a shared plan's free-form status and current revision without fetching its body.", + Parameters: tools.MustSchemaFor[GetPlanStatusArgs](), + OutputSchema: tools.MustSchemaFor[StatusView](), + Handler: tools.NewHandler(t.getPlanStatus), + Annotations: tools.ToolAnnotations{ + Title: "Get Plan Status", + ReadOnlyHint: true, + }, + }, { Name: ToolNameListPlans, Category: "plan", - Description: "List all shared plans with their name, title, author, and revision.", + Description: "List all shared plans with their name, title, author, status, and revision.", OutputSchema: tools.MustSchemaFor[ListResult](), Handler: t.listPlans, Annotations: tools.ToolAnnotations{ @@ -390,11 +736,19 @@ func (s *FilesystemStorage) Get(_ context.Context, name string) (Plan, bool, err s.mu.Lock() defer s.mu.Unlock() - return s.load(path) + plan, ok, err := s.load(path) + if ok { + // The filename is the authoritative key (List and Upsert key off it), so + // normalise the returned Name to the requested one. A plan file whose + // stored "name" field drifted from its filename then reads back under the + // name the caller can actually use to write or delete it. + plan.Name = name + } + return plan, ok, err } -func (s *FilesystemStorage) Upsert(_ context.Context, name, content, title, author string) (Plan, error) { - path, err := s.planPath(name) +func (s *FilesystemStorage) Upsert(_ context.Context, req UpsertRequest) (Plan, error) { + path, err := s.planPath(req.Name) if err != nil { return Plan{}, err } @@ -402,20 +756,35 @@ func (s *FilesystemStorage) Upsert(_ context.Context, name, content, title, auth s.mu.Lock() defer s.mu.Unlock() - plan, _, err := s.load(path) + plan, exists, err := s.load(path) if err != nil { return Plan{}, err } - plan.Name = name - plan.Content = content - // Title and author are preserved across revisions when omitted, so an - // agent updating only the content does not wipe the collaboration metadata - // set by a previous writer. - if title != "" { - plan.Title = title + if req.MustExist && !exists { + return Plan{}, fmt.Errorf("%w: %q", ErrPlanNotFound, req.Name) + } + // Optimistic-lock check: reject the write if another writer bumped the + // revision since the caller last read it. Checked under the same lock as + // the load+save below so the compare-and-set is atomic within the process. + if req.ExpectedRevision != nil && plan.Revision != *req.ExpectedRevision { + return Plan{}, &VersionConflictError{Name: req.Name, Expected: *req.ExpectedRevision, Current: plan.Revision} + } + + plan.Name = req.Name + // Nil fields are preserved across revisions, so an agent updating only the + // content (or only the status) does not wipe metadata set by a previous + // writer. + if req.Content != nil { + plan.Content = *req.Content } - if author != "" { - plan.Author = author + if req.Title != nil { + plan.Title = *req.Title + } + if req.Author != nil { + plan.Author = *req.Author + } + if req.Status != nil { + plan.Status = *req.Status } plan.Revision++ plan.UpdatedAt = time.Now().UTC().Format(time.RFC3339) @@ -465,6 +834,7 @@ func (s *FilesystemStorage) List(_ context.Context) ([]Summary, []string, error) Name: name, Title: plan.Title, Author: plan.Author, + Status: plan.Status, Revision: plan.Revision, UpdatedAt: plan.UpdatedAt, }) @@ -477,7 +847,7 @@ func (s *FilesystemStorage) List(_ context.Context) ([]Summary, []string, error) return plans, warnings, nil } -func (s *FilesystemStorage) Delete(_ context.Context, name string) (bool, error) { +func (s *FilesystemStorage) Delete(_ context.Context, name string, expectedRevision *int) (bool, error) { path, err := s.planPath(name) if err != nil { return false, err @@ -486,8 +856,25 @@ func (s *FilesystemStorage) Delete(_ context.Context, name string) (bool, error) s.mu.Lock() defer s.mu.Unlock() - // Remove the file directly and treat a missing file as "not deleted". We - // don't pre-load the plan: a corrupt plan should still be deletable. + // With an optimistic-lock guard we must read the current revision first, so + // the compare-and-delete is atomic under the lock. This means a guarded + // delete of a corrupt plan fails (its revision can't be read to compare); + // recovering from a corrupt plan is done with an unguarded delete, which + // removes the file directly without pre-loading it. + if expectedRevision != nil { + plan, ok, err := s.load(path) + if err != nil { + return false, err + } + if !ok { + return false, nil + } + if plan.Revision != *expectedRevision { + return false, &VersionConflictError{Name: name, Expected: *expectedRevision, Current: plan.Revision} + } + } + + // Remove the file directly and treat a missing file as "not deleted". if err := os.Remove(path); err != nil { if errors.Is(err, os.ErrNotExist) { return false, nil diff --git a/pkg/tools/builtin/plan/plan_test.go b/pkg/tools/builtin/plan/plan_test.go index a3d59f744..7ff15e8ad 100644 --- a/pkg/tools/builtin/plan/plan_test.go +++ b/pkg/tools/builtin/plan/plan_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "path/filepath" "sort" @@ -483,12 +484,13 @@ func runStorageConformance(t *testing.T, s Storage) { assert.Empty(t, warnings) // Upsert creates the plan at revision 1 and stamps a timestamp. - p, err := s.Upsert(ctx, "release", "v1", "Release", "alice") + p, err := s.Upsert(ctx, UpsertRequest{Name: "release", Content: new("v1"), Title: new("Release"), Author: new("alice"), Status: new("draft")}) require.NoError(t, err) assert.Equal(t, "release", p.Name) assert.Equal(t, "v1", p.Content) assert.Equal(t, "Release", p.Title) assert.Equal(t, "alice", p.Author) + assert.Equal(t, "draft", p.Status) assert.Equal(t, 1, p.Revision) assert.NotEmpty(t, p.UpdatedAt) @@ -498,32 +500,73 @@ func runStorageConformance(t *testing.T, s Storage) { require.True(t, ok) assert.Equal(t, p, got) - // Upsert bumps the revision and preserves omitted title/author. - p2, err := s.Upsert(ctx, "release", "v2", "", "") + // Upsert with nil fields bumps the revision and preserves title, author, + // and status; only the content changes. + p2, err := s.Upsert(ctx, UpsertRequest{Name: "release", Content: new("v2")}) require.NoError(t, err) assert.Equal(t, 2, p2.Revision) assert.Equal(t, "v2", p2.Content) assert.Equal(t, "Release", p2.Title) assert.Equal(t, "alice", p2.Author) + assert.Equal(t, "draft", p2.Status) - // A non-empty author overwrites the previous one. - p3, err := s.Upsert(ctx, "release", "v3", "", "bob") + // A non-nil author overwrites the previous one. + p3, err := s.Upsert(ctx, UpsertRequest{Name: "release", Content: new("v3"), Author: new("bob")}) require.NoError(t, err) assert.Equal(t, 3, p3.Revision) assert.Equal(t, "bob", p3.Author) - // List reflects every plan, sorted by name. - _, err = s.Upsert(ctx, "alpha", "a", "", "") + // Status can be set independently, preserving the content. + p4, err := s.Upsert(ctx, UpsertRequest{Name: "release", Status: new("done")}) + require.NoError(t, err) + assert.Equal(t, 4, p4.Revision) + assert.Equal(t, "done", p4.Status) + assert.Equal(t, "v3", p4.Content) + + // MustExist fails with ErrPlanNotFound for a plan that does not exist. + _, err = s.Upsert(ctx, UpsertRequest{Name: "ghost", Status: new("x"), MustExist: true}) + require.ErrorIs(t, err, ErrPlanNotFound) + + // A matching ExpectedRevision succeeds; a stale one is a version conflict. + p5, err := s.Upsert(ctx, UpsertRequest{Name: "release", Content: new("v5"), ExpectedRevision: new(4)}) + require.NoError(t, err) + assert.Equal(t, 5, p5.Revision) + + _, err = s.Upsert(ctx, UpsertRequest{Name: "release", Content: new("stale"), ExpectedRevision: new(4)}) + require.Error(t, err) + var conflict *VersionConflictError + require.ErrorAs(t, err, &conflict) + assert.Equal(t, 4, conflict.Expected) + assert.Equal(t, 5, conflict.Current) + + // The conflicting write left the plan untouched at revision 5. + got, ok, err = s.Get(ctx, "release") + require.NoError(t, err) + require.True(t, ok) + assert.Equal(t, 5, got.Revision) + assert.Equal(t, "v5", got.Content) + + // List reflects every plan, sorted by name, and carries the status. + _, err = s.Upsert(ctx, UpsertRequest{Name: "alpha", Content: new("a")}) require.NoError(t, err) plans, warnings, err = s.List(ctx) require.NoError(t, err) require.Len(t, plans, 2) assert.Equal(t, "alpha", plans[0].Name) assert.Equal(t, "release", plans[1].Name) + assert.Equal(t, "done", plans[1].Status) assert.Empty(t, warnings) - // Delete removes the plan and reports it was deleted. - deleted, err := s.Delete(ctx, "release") + // Delete with a stale revision is rejected as a conflict and removes nothing. + deleted, err := s.Delete(ctx, "release", new(1)) + require.ErrorAs(t, err, &conflict) + assert.False(t, deleted) + _, ok, err = s.Get(ctx, "release") + require.NoError(t, err) + assert.True(t, ok) + + // Delete with the matching revision removes the plan and reports success. + deleted, err = s.Delete(ctx, "release", new(5)) require.NoError(t, err) assert.True(t, deleted) _, ok, err = s.Get(ctx, "release") @@ -531,7 +574,7 @@ func runStorageConformance(t *testing.T, s Storage) { assert.False(t, ok) // Deleting again reports not-deleted, without an error. - deleted, err = s.Delete(ctx, "release") + deleted, err = s.Delete(ctx, "release", nil) require.NoError(t, err) assert.False(t, deleted) } @@ -557,21 +600,32 @@ func (s *memoryStorage) Get(_ context.Context, name string) (Plan, bool, error) return p, ok, nil } -func (s *memoryStorage) Upsert(_ context.Context, name, content, title, author string) (Plan, error) { +func (s *memoryStorage) Upsert(_ context.Context, req UpsertRequest) (Plan, error) { s.mu.Lock() defer s.mu.Unlock() - p := s.plans[name] - p.Name = name - p.Content = content - if title != "" { - p.Title = title + p, exists := s.plans[req.Name] + if req.MustExist && !exists { + return Plan{}, fmt.Errorf("%w: %q", ErrPlanNotFound, req.Name) + } + if req.ExpectedRevision != nil && p.Revision != *req.ExpectedRevision { + return Plan{}, &VersionConflictError{Name: req.Name, Expected: *req.ExpectedRevision, Current: p.Revision} + } + p.Name = req.Name + if req.Content != nil { + p.Content = *req.Content + } + if req.Title != nil { + p.Title = *req.Title } - if author != "" { - p.Author = author + if req.Author != nil { + p.Author = *req.Author + } + if req.Status != nil { + p.Status = *req.Status } p.Revision++ p.UpdatedAt = "2024-01-01T00:00:00Z" - s.plans[name] = p + s.plans[req.Name] = p return p, nil } @@ -580,18 +634,22 @@ func (s *memoryStorage) List(_ context.Context) ([]Summary, []string, error) { defer s.mu.Unlock() out := make([]Summary, 0, len(s.plans)) for name, p := range s.plans { - out = append(out, Summary{Name: name, Title: p.Title, Author: p.Author, Revision: p.Revision, UpdatedAt: p.UpdatedAt}) + out = append(out, Summary{Name: name, Title: p.Title, Author: p.Author, Status: p.Status, Revision: p.Revision, UpdatedAt: p.UpdatedAt}) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out, nil, nil } -func (s *memoryStorage) Delete(_ context.Context, name string) (bool, error) { +func (s *memoryStorage) Delete(_ context.Context, name string, expectedRevision *int) (bool, error) { s.mu.Lock() defer s.mu.Unlock() - if _, ok := s.plans[name]; !ok { + p, ok := s.plans[name] + if !ok { return false, nil } + if expectedRevision != nil && p.Revision != *expectedRevision { + return false, &VersionConflictError{Name: name, Expected: *expectedRevision, Current: p.Revision} + } delete(s.plans, name) return true, nil } @@ -604,13 +662,26 @@ var _ Storage = noBumpStorage{} func (noBumpStorage) Get(context.Context, string) (Plan, bool, error) { return Plan{}, false, nil } -func (noBumpStorage) Upsert(_ context.Context, name, content, title, author string) (Plan, error) { - return Plan{Name: name, Content: content, Title: title, Author: author, Revision: 0}, nil +func (noBumpStorage) Upsert(_ context.Context, req UpsertRequest) (Plan, error) { + p := Plan{Name: req.Name, Revision: 0} + if req.Content != nil { + p.Content = *req.Content + } + if req.Title != nil { + p.Title = *req.Title + } + if req.Author != nil { + p.Author = *req.Author + } + if req.Status != nil { + p.Status = *req.Status + } + return p, nil } func (noBumpStorage) List(context.Context) ([]Summary, []string, error) { return nil, nil, nil } -func (noBumpStorage) Delete(context.Context, string) (bool, error) { return false, nil } +func (noBumpStorage) Delete(context.Context, string, *int) (bool, error) { return false, nil } // errStorage is a backend whose every method fails, used to verify the // handlers surface a real backend error as an IsError result. @@ -625,7 +696,7 @@ func (errStorage) Get(context.Context, string) (Plan, bool, error) { return Plan{}, false, errBackend } -func (errStorage) Upsert(context.Context, string, string, string, string) (Plan, error) { +func (errStorage) Upsert(context.Context, UpsertRequest) (Plan, error) { return Plan{}, errBackend } @@ -633,6 +704,539 @@ func (errStorage) List(context.Context) ([]Summary, []string, error) { return nil, nil, errBackend } -func (errStorage) Delete(context.Context, string) (bool, error) { +func (errStorage) Delete(context.Context, string, *int) (bool, error) { return false, errBackend } + +// --- Status feature --------------------------------------------------------- + +func TestPlanTool_WriteWithStatus(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "x", Status: "in-progress"}) + require.NoError(t, err) + assert.False(t, result.IsError) + + var plan Plan + require.NoError(t, json.Unmarshal([]byte(result.Output), &plan)) + assert.Equal(t, "in-progress", plan.Status) +} + +func TestPlanTool_StatusPreservedWhenOmitted(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1", Status: "blocked"}) + require.NoError(t, err) + + // A subsequent write that omits status keeps the previous one. + result, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2"}) + require.NoError(t, err) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(result.Output), &plan)) + assert.Equal(t, "blocked", plan.Status) + assert.Equal(t, "v2", plan.Content) +} + +func TestPlanTool_SetPlanStatus(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "the body", Title: "T", Author: "alice"}) + require.NoError(t, err) + + result, err := tool.setPlanStatus(t.Context(), SetPlanStatusArgs{Name: "p", Status: "done"}) + require.NoError(t, err) + assert.False(t, result.IsError) + + var view StatusView + require.NoError(t, json.Unmarshal([]byte(result.Output), &view)) + assert.Equal(t, "p", view.Name) + assert.Equal(t, "done", view.Status) + // Setting the status is a write, so it bumps the revision. + assert.Equal(t, 2, view.Revision) + + // The status write preserves the body, title, and author untouched. + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(read.Output), &plan)) + assert.Equal(t, "done", plan.Status) + assert.Equal(t, "the body", plan.Content) + assert.Equal(t, "T", plan.Title) + assert.Equal(t, "alice", plan.Author) +} + +// TestPlanTool_SetStatusIsTokenLight proves set_plan_status returns only the +// lightweight status view, never the plan body, so updating the status is cheap. +func TestPlanTool_SetStatusIsTokenLight(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "SECRET-BODY-MARKER"}) + require.NoError(t, err) + + result, err := tool.setPlanStatus(t.Context(), SetPlanStatusArgs{Name: "p", Status: "done"}) + require.NoError(t, err) + assert.False(t, result.IsError) + assert.NotContains(t, result.Output, "SECRET-BODY-MARKER") + assert.NotContains(t, result.Output, "content") +} + +func TestPlanTool_SetStatusNotFound(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.setPlanStatus(t.Context(), SetPlanStatusArgs{Name: "ghost", Status: "done"}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "not found") +} + +func TestPlanTool_SetStatusEmptyRejected(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "x"}) + require.NoError(t, err) + + result, err := tool.setPlanStatus(t.Context(), SetPlanStatusArgs{Name: "p", Status: ""}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "status must not be empty") +} + +func TestPlanTool_GetPlanStatus(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "SECRET-BODY-MARKER", Status: "in-progress"}) + require.NoError(t, err) + + result, err := tool.getPlanStatus(t.Context(), GetPlanStatusArgs{Name: "p"}) + require.NoError(t, err) + assert.False(t, result.IsError) + + var view StatusView + require.NoError(t, json.Unmarshal([]byte(result.Output), &view)) + assert.Equal(t, "p", view.Name) + assert.Equal(t, "in-progress", view.Status) + assert.Equal(t, 1, view.Revision) + // Reading the status never fetches the body. + assert.NotContains(t, result.Output, "SECRET-BODY-MARKER") +} + +func TestPlanTool_GetStatusNotFound(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.getPlanStatus(t.Context(), GetPlanStatusArgs{Name: "missing"}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "not found") +} + +func TestPlanTool_ListIncludesStatus(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "x", Status: "review"}) + require.NoError(t, err) + + result, err := tool.listPlans(t.Context(), tools.ToolCall{}) + require.NoError(t, err) + var list ListResult + require.NoError(t, json.Unmarshal([]byte(result.Output), &list)) + require.Len(t, list.Plans, 1) + assert.Equal(t, "review", list.Plans[0].Status) +} + +// --- File-based revisions --------------------------------------------------- + +func TestPlanTool_ExportPlanToFile(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "SECRET-BODY-MARKER", Title: "T", Status: "draft"}) + require.NoError(t, err) + + dest := filepath.Join(t.TempDir(), "export.md") + result, err := tool.exportPlanToFile(t.Context(), ExportPlanToFileArgs{Name: "p", Path: dest}) + require.NoError(t, err) + assert.False(t, result.IsError) + + // The full content is written to disk... + data, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "SECRET-BODY-MARKER", string(data)) + + // ...but is NOT returned as tool output (the whole point of the tool). + assert.NotContains(t, result.Output, "SECRET-BODY-MARKER") + + var export ExportResult + require.NoError(t, json.Unmarshal([]byte(result.Output), &export)) + assert.Equal(t, "p", export.Name) + assert.Equal(t, dest, export.Path) + assert.Equal(t, "T", export.Title) + assert.Equal(t, "draft", export.Status) + assert.Equal(t, 1, export.Revision) + assert.Equal(t, len("SECRET-BODY-MARKER"), export.BytesWritten) +} + +func TestPlanTool_ExportCreatesParentDirs(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "body"}) + require.NoError(t, err) + + dest := filepath.Join(t.TempDir(), "nested", "deeper", "export.md") + result, err := tool.exportPlanToFile(t.Context(), ExportPlanToFileArgs{Name: "p", Path: dest}) + require.NoError(t, err) + assert.False(t, result.IsError) + + data, err := os.ReadFile(dest) + require.NoError(t, err) + assert.Equal(t, "body", string(data)) +} + +func TestPlanTool_ExportNotFound(t *testing.T) { + tool := newTestPlanTool(t) + + dest := filepath.Join(t.TempDir(), "export.md") + result, err := tool.exportPlanToFile(t.Context(), ExportPlanToFileArgs{Name: "missing", Path: dest}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "not found") + assert.NoFileExists(t, dest) +} + +func TestPlanTool_ExportEmptyPath(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "body"}) + require.NoError(t, err) + + result, err := tool.exportPlanToFile(t.Context(), ExportPlanToFileArgs{Name: "p", Path: ""}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "path must not be empty") +} + +func TestPlanTool_UpdatePlanFromFile(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1", Title: "T", Author: "alice"}) + require.NoError(t, err) + + src := filepath.Join(t.TempDir(), "edit.md") + require.NoError(t, os.WriteFile(src, []byte("brand new content"), 0o600)) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: src, Author: "bob"}) + require.NoError(t, err) + assert.False(t, result.IsError) + + var plan Plan + require.NoError(t, json.Unmarshal([]byte(result.Output), &plan)) + assert.Equal(t, "brand new content", plan.Content) + assert.Equal(t, 2, plan.Revision) + // Title is preserved (omitted); author is updated. + assert.Equal(t, "T", plan.Title) + assert.Equal(t, "bob", plan.Author) +} + +// TestPlanTool_FileRoundTrip exercises the cheap edit loop the feature exists +// for: export to disk, edit the file, commit it back with update_plan_from_file. +func TestPlanTool_FileRoundTrip(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "original", Title: "Roadmap"}) + require.NoError(t, err) + + scratch := filepath.Join(t.TempDir(), "plan.md") + exp, err := tool.exportPlanToFile(t.Context(), ExportPlanToFileArgs{Name: "p", Path: scratch}) + require.NoError(t, err) + require.False(t, exp.IsError) + + // Edit the materialised file in place, then commit it back. + require.NoError(t, os.WriteFile(scratch, []byte("original\nplus an appended line"), 0o600)) + + _, err = tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: scratch}) + require.NoError(t, err) + + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(read.Output), &plan)) + assert.Equal(t, "original\nplus an appended line", plan.Content) + assert.Equal(t, "Roadmap", plan.Title) + assert.Equal(t, 2, plan.Revision) +} + +func TestPlanTool_UpdateFromFileEmptyPath(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: ""}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "path must not be empty") +} + +func TestPlanTool_UpdateFromFileMissingFile(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: filepath.Join(t.TempDir(), "nope.md")}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "does not exist") +} + +func TestPlanTool_UpdateFromFileEmptyFile(t *testing.T) { + tool := newTestPlanTool(t) + + src := filepath.Join(t.TempDir(), "empty.md") + require.NoError(t, os.WriteFile(src, []byte(""), 0o600)) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: src}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "empty") +} + +func TestPlanTool_UpdateFromFileDirectory(t *testing.T) { + tool := newTestPlanTool(t) + + dir := t.TempDir() + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: dir}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "directory") +} + +// TestPlanTool_UpdateFromFileTooLarge proves the size cap is enforced: a file +// over the limit is rejected rather than slurped into a plan. +func TestPlanTool_UpdateFromFileTooLarge(t *testing.T) { + tool := newTestPlanTool(t) + + src := filepath.Join(t.TempDir(), "big.md") + require.NoError(t, os.WriteFile(src, make([]byte, maxPlanFileSize+1), 0o600)) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: src}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "too large") +} + +// TestPlanTool_ReadNormalizesNameFromFilename proves read_plan returns the name +// the caller can actually use (the filename), not a drifted "name" field stored +// inside the file, so a read -> write round-trip can't land on the wrong plan. +func TestPlanTool_ReadNormalizesNameFromFilename(t *testing.T) { + tool, dir := newTestPlanToolWithDir(t) + + drifted := Plan{Name: "wrong-name", Content: "x", Revision: 1} + data, err := json.Marshal(drifted) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, "real-name.json"), data, 0o600)) + + result, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "real-name"}) + require.NoError(t, err) + require.False(t, result.IsError) + + var plan Plan + require.NoError(t, json.Unmarshal([]byte(result.Output), &plan)) + assert.Equal(t, "real-name", plan.Name) +} + +// TestPlanTool_GuardedDeleteOnCorrupt documents the interaction between the +// optimistic-lock guard and a corrupt plan: a guarded delete can't read the +// revision to compare so it fails, while an unguarded delete still recovers it. +func TestPlanTool_GuardedDeleteOnCorrupt(t *testing.T) { + tool, dir := newTestPlanToolWithDir(t) + require.NoError(t, os.WriteFile(filepath.Join(dir, "broken.json"), []byte("{nope"), 0o600)) + + guarded, err := tool.deletePlan(t.Context(), DeletePlanArgs{Name: "broken", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.True(t, guarded.IsError, "a guarded delete cannot verify a corrupt plan's revision") + + unguarded, err := tool.deletePlan(t.Context(), DeletePlanArgs{Name: "broken"}) + require.NoError(t, err) + assert.False(t, unguarded.IsError, "an unguarded delete still recovers a corrupt plan") +} + +// --- Optimistic locking ----------------------------------------------------- + +func TestPlanTool_WriteWithMatchingVersion(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + + result, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.False(t, result.IsError) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(result.Output), &plan)) + assert.Equal(t, 2, plan.Revision) +} + +func TestPlanTool_WriteWithStaleVersionConflicts(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + _, err = tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2"}) + require.NoError(t, err) + + // The plan is now at revision 2; a writer that still thinks it is at 1 loses. + result, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "racing", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "conflict") + + // The losing write did not touch the plan. + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(read.Output), &plan)) + assert.Equal(t, "v2", plan.Content) + assert.Equal(t, 2, plan.Revision) +} + +// TestPlanTool_CreateWithVersionZero documents that a fresh plan can be guarded +// with last_known_revision 0 ("I expect this not to exist yet"). +func TestPlanTool_CreateWithVersionZero(t *testing.T) { + tool := newTestPlanTool(t) + + result, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1", LastKnownRevision: new(0)}) + require.NoError(t, err) + assert.False(t, result.IsError) + + // Creating it again with last_known_revision 0 now conflicts (it is at 1). + result, err = tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "again", LastKnownRevision: new(0)}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "conflict") +} + +func TestPlanTool_UpdateFromFileWithStaleVersionConflicts(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + _, err = tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2"}) + require.NoError(t, err) + + src := filepath.Join(t.TempDir(), "edit.md") + require.NoError(t, os.WriteFile(src, []byte("new"), 0o600)) + + result, err := tool.updatePlanFromFile(t.Context(), UpdatePlanFromFileArgs{Name: "p", Path: src, LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "conflict") +} + +func TestPlanTool_SetStatusWithStaleVersionConflicts(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + _, err = tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2"}) + require.NoError(t, err) + + result, err := tool.setPlanStatus(t.Context(), SetPlanStatusArgs{Name: "p", Status: "done", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "conflict") +} + +func TestPlanTool_DeleteWithStaleVersionConflicts(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + _, err = tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v2"}) + require.NoError(t, err) + + result, err := tool.deletePlan(t.Context(), DeletePlanArgs{Name: "p", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "conflict") + + // The conflicting delete left the plan in place. + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + assert.False(t, read.IsError) +} + +func TestPlanTool_DeleteWithMatchingVersion(t *testing.T) { + tool := newTestPlanTool(t) + + _, err := tool.writePlan(t.Context(), WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + + result, err := tool.deletePlan(t.Context(), DeletePlanArgs{Name: "p", LastKnownRevision: new(1)}) + require.NoError(t, err) + assert.False(t, result.IsError) + + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + assert.True(t, read.IsError) +} + +// TestPlanTool_ConcurrentWritesOptimisticLock proves the lock under real +// contention: many goroutines that all read revision 1 race to write with +// last_known_revision=1. Exactly one wins and the rest get a conflict, so the +// plan can never silently lose a concurrent edit and lands at exactly one bump. +func TestPlanTool_ConcurrentWritesOptimisticLock(t *testing.T) { + tool := newTestPlanTool(t) + + ctx := t.Context() + _, err := tool.writePlan(ctx, WritePlanArgs{Name: "p", Content: "v1"}) + require.NoError(t, err) + + const n = 8 + results := make([]*tools.ToolCallResult, n) + errs := make([]error, n) + var wg sync.WaitGroup + for i := range n { + wg.Add(1) + go func(i int) { + defer wg.Done() + results[i], errs[i] = tool.writePlan(ctx, WritePlanArgs{ + Name: "p", + Content: fmt.Sprintf("by-%d", i), + LastKnownRevision: new(1), + }) + }(i) + } + wg.Wait() + + success, conflicts := 0, 0 + for i := range n { + require.NoError(t, errs[i]) + if results[i].IsError { + assert.Contains(t, results[i].Output, "conflict") + conflicts++ + } else { + success++ + } + } + assert.Equal(t, 1, success, "exactly one writer starting from revision 1 should win") + assert.Equal(t, n-1, conflicts, "every other writer should get a conflict") + + read, err := tool.readPlan(t.Context(), ReadPlanArgs{Name: "p"}) + require.NoError(t, err) + var plan Plan + require.NoError(t, json.Unmarshal([]byte(read.Output), &plan)) + assert.Equal(t, 2, plan.Revision, "the plan should advance by exactly one revision") +} + +// TestPlanTool_NewToolsRegistered locks in that the new tools are exposed with +// their documented names so a config or client referencing them keeps working. +func TestPlanTool_NewToolsRegistered(t *testing.T) { + tool := newTestPlanTool(t) + + all, err := tool.Tools(t.Context()) + require.NoError(t, err) + + names := map[string]bool{} + for _, tl := range all { + names[tl.Name] = true + } + for _, want := range []string{ + ToolNameWritePlan, ToolNameReadPlan, ToolNameListPlans, ToolNameDeletePlan, + ToolNameUpdatePlanFromFile, ToolNameExportPlanToFile, ToolNameSetPlanStatus, ToolNameGetPlanStatus, + } { + assert.True(t, names[want], "tool %q should be registered", want) + } +} diff --git a/pkg/tools/builtin/plan/plan_unix_test.go b/pkg/tools/builtin/plan/plan_unix_test.go new file mode 100644 index 000000000..aa2a2f5ff --- /dev/null +++ b/pkg/tools/builtin/plan/plan_unix_test.go @@ -0,0 +1,46 @@ +//go:build unix + +package plan + +import ( + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPlanTool_UpdateFromFileRejectsNamedPipe proves a non-regular file (here a +// named pipe with no writer) is rejected at the stat stage and never opened. +// Opening such a pipe would block forever and reading a device like /dev/zero +// would stream unbounded data, so the implementation must refuse it up front. +// The timeout guards against a regression that opens the file and hangs. +func TestPlanTool_UpdateFromFileRejectsNamedPipe(t *testing.T) { + tool := newTestPlanTool(t) + + fifo := filepath.Join(t.TempDir(), "pipe") + require.NoError(t, syscall.Mkfifo(fifo, 0o600)) + + ctx := t.Context() + done := make(chan *struct { + out string + isError bool + }, 1) + go func() { + res, _ := tool.updatePlanFromFile(ctx, UpdatePlanFromFileArgs{Name: "p", Path: fifo}) + done <- &struct { + out string + isError bool + }{res.Output, res.IsError} + }() + + select { + case res := <-done: + assert.True(t, res.isError) + assert.Contains(t, res.out, "regular file") + case <-time.After(5 * time.Second): + t.Fatal("updatePlanFromFile blocked on a named pipe; it must reject non-regular files without opening them") + } +} diff --git a/pkg/tui/components/tool/factory.go b/pkg/tui/components/tool/factory.go index 6f59d0a43..ecc863c66 100644 --- a/pkg/tui/components/tool/factory.go +++ b/pkg/tui/components/tool/factory.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker-agent/pkg/tools/builtin/fetch" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" handofftool "github.com/docker/docker-agent/pkg/tools/builtin/handoff" + "github.com/docker/docker-agent/pkg/tools/builtin/plan" shelltool "github.com/docker/docker-agent/pkg/tools/builtin/shell" "github.com/docker/docker-agent/pkg/tools/builtin/todo" transfertasktool "github.com/docker/docker-agent/pkg/tools/builtin/transfertask" @@ -21,6 +22,7 @@ import ( "github.com/docker/docker-agent/pkg/tui/components/tool/editfile" "github.com/docker/docker-agent/pkg/tui/components/tool/handoff" "github.com/docker/docker-agent/pkg/tui/components/tool/listdirectory" + "github.com/docker/docker-agent/pkg/tui/components/tool/plantool" "github.com/docker/docker-agent/pkg/tui/components/tool/readfile" "github.com/docker/docker-agent/pkg/tui/components/tool/readmultiplefiles" "github.com/docker/docker-agent/pkg/tui/components/tool/searchfilescontent" @@ -58,6 +60,16 @@ var builders = map[string]Builder{ todo.ToolNameCreateTodos: todotool.New, todo.ToolNameUpdateTodos: todotool.New, todo.ToolNameListTodos: todotool.New, + // Single-plan write/status tools surface the plan's status/title in a + // compact header. read_plan, list_plans and delete_plan intentionally keep + // the default renderer: read_plan's job is to show the full plan body (the + // default renderer prints it, with the status still in the JSON), list_plans + // returns many plans, and delete_plan has no status to show. + plan.ToolNameWritePlan: plantool.New, + plan.ToolNameSetPlanStatus: plantool.New, + plan.ToolNameGetPlanStatus: plantool.New, + plan.ToolNameUpdatePlanFromFile: plantool.New, + plan.ToolNameExportPlanToFile: plantool.New, } // custom holds renderers registered by embedders via Register. They are diff --git a/pkg/tui/components/tool/factory_test.go b/pkg/tui/components/tool/factory_test.go index cc7fc3ec3..a23a16b5b 100644 --- a/pkg/tui/components/tool/factory_test.go +++ b/pkg/tui/components/tool/factory_test.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker-agent/pkg/tools" "github.com/docker/docker-agent/pkg/tools/builtin/filesystem" + "github.com/docker/docker-agent/pkg/tools/builtin/plan" shelltool "github.com/docker/docker-agent/pkg/tools/builtin/shell" "github.com/docker/docker-agent/pkg/tui/core/layout" "github.com/docker/docker-agent/pkg/tui/service" @@ -124,3 +125,27 @@ func TestNew_Dispatch(t *testing.T) { assert.False(t, byCategory, "no category renderer registered") }) } + +// TestPlanToolsRouting locks in which plan tools get the status-surfacing +// renderer: the single-plan write/status tools do, while read_plan (shows the +// full body), list_plans (many plans) and delete_plan (no status) intentionally +// fall through to the default renderer. +func TestPlanToolsRouting(t *testing.T) { + withCleanToolRegistry(t) + + for _, name := range []string{ + plan.ToolNameWritePlan, + plan.ToolNameSetPlanStatus, + plan.ToolNameGetPlanStatus, + plan.ToolNameUpdatePlanFromFile, + plan.ToolNameExportPlanToFile, + } { + _, ok := resolve(name) + assert.True(t, ok, "%q should have a dedicated plan renderer", name) + } + + for _, name := range []string{plan.ToolNameReadPlan, plan.ToolNameListPlans, plan.ToolNameDeletePlan} { + _, ok := resolve(name) + assert.False(t, ok, "%q should fall through to the default renderer", name) + } +} diff --git a/pkg/tui/components/tool/plantool/plantool.go b/pkg/tui/components/tool/plantool/plantool.go new file mode 100644 index 000000000..3e5d8ebf4 --- /dev/null +++ b/pkg/tui/components/tool/plantool/plantool.go @@ -0,0 +1,70 @@ +// Package plantool renders the single-plan write/status plan tool calls +// (write_plan, set_plan_status, get_plan_status, update_plan_from_file, +// export_plan_to_file). It surfaces the plan's status (and title) right next to +// the tool call, so a reader can see a plan's lifecycle at a glance instead of +// hunting for it in the JSON body. +// +// read_plan, list_plans and delete_plan intentionally keep the default +// renderer: read_plan is meant to show the full plan body (which the default +// renderer prints, with the status still present in the JSON), list_plans +// returns many plans, and delete_plan has no status to surface. +package plantool + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/docker/docker-agent/pkg/tui/components/toolcommon" + "github.com/docker/docker-agent/pkg/tui/core/layout" + "github.com/docker/docker-agent/pkg/tui/service" + "github.com/docker/docker-agent/pkg/tui/types" +) + +// New builds the plan tool view: the plan name from the call arguments, plus a +// compact title/status/revision summary from the result. +func New(msg *types.Message, sessionState service.SessionStateReader) layout.Model { + return toolcommon.NewBase(msg, sessionState, toolcommon.SimpleRendererWithResult(extractName, extractSummary)) +} + +type nameArg struct { + Name string `json:"name"` +} + +func extractName(args string) string { + return toolcommon.ExtractField(func(a nameArg) string { return a.Name })(args) +} + +// planResult captures the result fields worth surfacing in the header. Plan, +// StatusView and ExportResult all share this shape, so one decode covers every +// single-plan tool. +type planResult struct { + Title string `json:"title"` + Status string `json:"status"` + Revision int `json:"revision"` +} + +func extractSummary(msg *types.Message) string { + // On failure, surface the backend message verbatim (e.g. a version + // conflict): that is exactly what the reader needs in order to react. + if msg.ToolStatus == types.ToolStatusError { + return msg.Content + } + + var r planResult + if err := json.Unmarshal([]byte(msg.Content), &r); err != nil { + return "" + } + + var parts []string + if r.Title != "" { + parts = append(parts, fmt.Sprintf("%q", r.Title)) + } + if r.Status != "" { + parts = append(parts, "["+r.Status+"]") + } + if r.Revision > 0 { + parts = append(parts, fmt.Sprintf("rev %d", r.Revision)) + } + return strings.Join(parts, " ") +} diff --git a/pkg/tui/components/tool/plantool/plantool_test.go b/pkg/tui/components/tool/plantool/plantool_test.go new file mode 100644 index 000000000..9b38c30e9 --- /dev/null +++ b/pkg/tui/components/tool/plantool/plantool_test.go @@ -0,0 +1,122 @@ +package plantool + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/docker/docker-agent/pkg/tools" + "github.com/docker/docker-agent/pkg/tui/service" + "github.com/docker/docker-agent/pkg/tui/types" +) + +var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m") + +func stripANSI(s string) string { + return ansiEscape.ReplaceAllString(s, "") +} + +func planMessage(status types.ToolStatus, args, result string) *types.Message { + return &types.Message{ + Type: types.MessageTypeToolCall, + ToolStatus: status, + Content: result, + ToolCall: tools.ToolCall{ + Function: tools.FunctionCall{Arguments: args}, + }, + ToolDefinition: tools.Tool{ + Name: "write_plan", + Annotations: tools.ToolAnnotations{Title: "Write Plan"}, + }, + } +} + +func render(t *testing.T, msg *types.Message) string { + t.Helper() + view := New(msg, service.StaticSessionState{}) + view.SetSize(120, 1) + return stripANSI(view.View()) +} + +// TestPlanTool_SurfacesStatusAndTitle is the core requirement: a completed +// plan call shows the plan name (from args) and the status and title (from the +// result) next to each other. +func TestPlanTool_SurfacesStatusAndTitle(t *testing.T) { + t.Parallel() + + out := render(t, planMessage( + types.ToolStatusCompleted, + `{"name":"release","content":"body"}`, + `{"name":"release","title":"Release plan","status":"in-progress","revision":3}`, + )) + + assert.Contains(t, out, "release", "plan name should be shown") + assert.Contains(t, out, "Release plan", "plan title should be surfaced") + assert.Contains(t, out, "in-progress", "plan status should be surfaced") + assert.Contains(t, out, "rev 3", "plan revision should be surfaced") +} + +// TestPlanTool_StatusViewWithoutTitle covers get_plan_status / set_plan_status, +// whose lightweight result has a status and revision but no title. +func TestPlanTool_StatusViewWithoutTitle(t *testing.T) { + t.Parallel() + + out := render(t, planMessage( + types.ToolStatusCompleted, + `{"name":"release"}`, + `{"name":"release","status":"done","revision":5}`, + )) + + assert.Contains(t, out, "done") + assert.Contains(t, out, "rev 5") +} + +// TestPlanTool_ShowsConflictError verifies a version conflict (or any error) is +// surfaced verbatim, since that is what the reader must act on. +func TestPlanTool_ShowsConflictError(t *testing.T) { + t.Parallel() + + out := render(t, planMessage( + types.ToolStatusError, + `{"name":"release","content":"x","last_known_revision":1}`, + `version conflict on plan "release": last_known_revision 1 does not match current revision 4; re-read the plan and retry`, + )) + + assert.Contains(t, out, "version conflict") + assert.Contains(t, out, "release") +} + +func TestExtractName(t *testing.T) { + t.Parallel() + + assert.Equal(t, "release", extractName(`{"name":"release","content":"x"}`)) + assert.Empty(t, extractName(`not json`)) + assert.Empty(t, extractName(`{"other":"field"}`)) +} + +func TestExtractSummary(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + status types.ToolStatus + result string + want string + }{ + {"full plan", types.ToolStatusCompleted, `{"title":"T","status":"draft","revision":2}`, `"T" [draft] rev 2`}, + {"status only", types.ToolStatusCompleted, `{"status":"done","revision":5}`, `[done] rev 5`}, + {"no status", types.ToolStatusCompleted, `{"title":"T","revision":1}`, `"T" rev 1`}, + {"empty result", types.ToolStatusCompleted, `{}`, ``}, + {"unparseable", types.ToolStatusCompleted, `not json`, ``}, + {"error passthrough", types.ToolStatusError, `boom`, `boom`}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + msg := &types.Message{ToolStatus: tc.status, Content: tc.result} + assert.Equal(t, tc.want, extractSummary(msg)) + }) + } +}