mirror of
https://github.com/docker/docker-agent.git
synced 2026-11-04 10:02:15 +00:00
feat(plan): file-based revisions, free-form status, and optimistic locking
Implements the three improvements requested in issue #3263. File-based revisions: - export_plan_to_file writes a plan's content to disk and returns only metadata, never the body, so materialising a plan costs no output tokens. - update_plan_from_file commits new content read from a file. File reads reject directories, non-regular files (devices, named pipes) and oversized files, and are bounded by an io.LimitReader. Free-form status: - Plan and Summary gain a free-form status string (no fixed vocabulary). - set_plan_status and get_plan_status read and write the status without the body; status is also accepted on write_plan and update_plan_from_file. - A new TUI renderer surfaces the status and title next to single-plan calls. Optimistic locking: - The existing auto-incremented revision is the version number. Reads return it; write_plan, update_plan_from_file, set_plan_status and delete_plan accept an optional last_known_revision that rejects the write with a conflict error on mismatch. The compare-and-set is atomic under the storage mutex. Storage.Upsert now takes an UpsertRequest (nil fields preserve their previous value) and Storage.Delete takes an optional expected revision. Docs and the shared_plan example are updated to cover the new tools.
This commit is contained in:
9 files changed
+1384
-78
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
+424
-37
@@ -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)
|
||||
}
|
||||
if author != "" {
|
||||
plan.Author = author
|
||||
// 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 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
|
||||
|
||||
@@ -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 author != "" {
|
||||
p.Author = author
|
||||
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 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)
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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, " ")
|
||||
}
|
||||
@@ -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))
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user