mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
fix(api): children, starred and shared-collection lists are bounded at 1000 and paged; MCP starred caps at 300 (BUG-3789, BUG-3790, BUG-3792) (#2153)
* fix(api): the children list, the starred list and a shared collection are bounded at 1000 and paged; MCP starred caps at 300 (BUG-3789, BUG-3790, BUG-3792) Each returned every row with its body: the children of an item, the caller's starred items, and every item of a shared collection to an anonymous reader. They now take the BUG-3704 contract: limit defaults to and is capped at 1000, offset skips rows, and X-Pad-Has-More / X-Pad-Next-Offset carry the paging state (the share payload, a JSON object, also says has_more and next_offset in the body). Each order ends on the id, so offset pages neither repeat nor skip rows that tie. Children: store.ListChildItems applies the caller's visibility in the query, so the window and has-more count only visible children, and the has_children probe reads one visible grandchild instead of every grandchild with its body. Starred: read without bodies, filtered for visibility and terminal status (which stays in Go), windowed, and bodies are loaded only for the page (store.ItemBodies). Share link: ListItems takes the window. Callers walk every page: the web children and starred lists by offset until a short page (request() exposes no headers; rows de-duplicated by id), the web share page by next_offset (merging items and attachment refs), including the password retry, and pad item starred by the server's next-offset. Each walk fails past 50 pages rather than return a partial list. MCP pad_item action=starred never pages (lead ruling, as BUG-3704 for list): both transports ask for one row past the 300 cap (remote ?limit=301, stdio the new `pad item starred --limit`) and say has_more with the truncation note. ToolSurfaceVersion 0.99. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS * test(api): an offset past the end of the starred and children lists is [] (BUG-3789, BUG-3790; lead check) Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
@@ -402,7 +402,7 @@ directory for `claude-code`, and an `[mcp_servers.pad]` table in
|
||||
project-scoped, it's install-on-request only — `--all` and `pad mcp status` cover
|
||||
the per-user clients (including Codex) and skip it.
|
||||
|
||||
**Tool catalog (v0.98)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
|
||||
**Tool catalog (v0.99)** — ten resource × action tools plus `pad_set_workspace`. Undeclared input keys are rejected with a structured error rather than silently dropped, and field values are typed against the collection schema on the server:
|
||||
|
||||
| Tool | Actions |
|
||||
|---|---|
|
||||
@@ -426,7 +426,7 @@ Plus resources at `pad://workspaces`, `pad://workspace/{ws}/dashboard`,
|
||||
and `pad://_meta/version`.
|
||||
|
||||
**Stability contract.** The catalog is versioned
|
||||
(`tool_surface_version: "0.98"`) and advertised in the initialize handshake and at
|
||||
(`tool_surface_version: "0.99"`) and advertised in the initialize handshake and at
|
||||
`pad://_meta/version`, so external agents can pin against it; the per-version
|
||||
changelog lives in [`internal/mcp/version.go`](internal/mcp/version.go). Errors
|
||||
come back as structured envelopes (`{error: {code, message, hint, ...}}`) from a
|
||||
|
||||
+15
-1
@@ -4400,6 +4400,7 @@ func unstarCmd() *cobra.Command {
|
||||
|
||||
func starredCmd() *cobra.Command {
|
||||
var all bool
|
||||
var limit int
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "starred",
|
||||
@@ -4410,7 +4411,19 @@ func starredCmd() *cobra.Command {
|
||||
client, _ := getClient()
|
||||
ws := getWorkspace()
|
||||
|
||||
items, err := client.ListStarredItems(ws, all)
|
||||
// --limit asks for one page from the start and says on stderr when
|
||||
// more exist; without it every page is read (BUG-3790).
|
||||
var items []models.Item
|
||||
var err error
|
||||
if limit > 0 {
|
||||
var page cli.ItemListPage
|
||||
items, page, err = client.ListStarredItemsPage(ws, all, limit)
|
||||
if err == nil && page.HasMore {
|
||||
fmt.Fprintf(os.Stderr, "More starred items exist than --limit %d shows.\n", limit)
|
||||
}
|
||||
} else {
|
||||
items, err = client.ListStarredItems(ws, all)
|
||||
}
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -4438,6 +4451,7 @@ func starredCmd() *cobra.Command {
|
||||
}
|
||||
|
||||
cmd.Flags().BoolVar(&all, "all", false, "include completed/terminal items")
|
||||
cmd.Flags().IntVar(&limit, "limit", 0, "at most this many, from the most recently starred (default: all)")
|
||||
|
||||
return cmd
|
||||
}
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+1
-1
@@ -339,7 +339,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"limit": {
|
||||
"description": "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions.",
|
||||
"description": "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions. starred takes no limit: it returns at most 300, most recently starred first, with has_more when more exist.",
|
||||
"type": "number"
|
||||
},
|
||||
"lines": {
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+1
-1
@@ -339,7 +339,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"limit": {
|
||||
"description": "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions.",
|
||||
"description": "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions. starred takes no limit: it returns at most 300, most recently starred first, with has_more when more exist.",
|
||||
"type": "number"
|
||||
},
|
||||
"lines": {
|
||||
|
||||
+3
-1
@@ -73,7 +73,9 @@ REST API at `/api/v1/`. Key endpoints:
|
||||
- `GET/POST/DELETE /workspaces/{ws}/webhooks` — webhook management
|
||||
- `PATCH/DELETE /workspaces/{ws}/items/{slug}/comments/{id}` — comment edit/delete addressed through its item (TASK-2695): 404 unless the comment is on that item, then the same handler, ACL and events as `/workspaces/{ws}/comments/{id}` (edit author-only, delete item-edit). A delete of a comment that still has replies leaves a TOMBSTONE on either route (BUG-3252, 204 like any delete): the body is emptied and the reactions removed, author and timestamps are kept, the comment serialises with `deleted: true`, and its replies keep their parent. An edit, reply or reaction addressed to a tombstone answers 409 `comment_deleted`, deleting it again answers 404, and deleting its last reply removes it in the same transaction. Tombstones are left out of the decision trail and the workspace export. A server before the tombstone answered 409 `comment_has_replies` instead, and the CLI and MCP still classify that code. The CLI and every MCP transport use this form; `GET /server/capabilities` advertises it as `item_scoped_comment_writes`. Every serialised comment carries a derived `edited` bool
|
||||
- `GET /workspaces/{ws}/changes?since=<unix_ms>` — the web sync delta: items updated at or after `since` (archived ones included; their ids are also in `deleted`), with full bodies, and `server_time`, the next `since`. **Bounded and paged by a cursor** (BUG-3791). `limit` defaults to, and is capped at, 1000, as on the item lists. When more remain the body says `has_more: true` with `next_cursor`; ask again with `?cursor=<next_cursor>` alone (a `since` beside it must equal the walk's). The cursor is a position in the workspace's write order (`seq`, then id), so a change committed during the walk comes back in a later page, and a row can appear twice. Every page repeats the FIRST page's `server_time`, which is a safe next `since` only once a page says `has_more: false`. Visibility is applied before the window, so a page's count and `has_more` never reflect rows the caller cannot see. **Contract change:** a client that ignores `has_more` gets the first 1000 changes of a longer delta and loses the rest when it advances to `server_time`; the web client (`walkChanges`) walks every page, and fails to a full refresh past 50
|
||||
- `GET /workspaces/{ws}/items/{slug}/children` — child items linked to a parent
|
||||
- `GET /workspaces/{ws}/items/{slug}/children` — child items linked to a parent, in the manual order (sort_order, created_at, id). **Bounded and paged like the item lists** (BUG-3789): `limit` defaults to and is capped at 1000, `offset` skips rows, and `X-Pad-Has-More` / `X-Pad-Next-Offset` carry the paging state; the body stays a bare array. Visibility is applied before the window, so the page and has-more count only children the caller may see. The web client walks every page
|
||||
- `GET /workspaces/{ws}/starred` — the caller's starred items, most recently starred first (ties by id); `include_terminal=true` keeps closed ones, `summary=true` leaves out bodies. **Bounded and paged** the same way (BUG-3790), after the visibility and terminal filters; bodies are loaded only for the page returned. `pad item starred` and the web client walk every page
|
||||
- `GET /api/v1/s/{token}` — public share link resolution. For a COLLECTION share the items are **bounded and paged** (BUG-3792): `limit` (default and cap 1000) and `offset` query parameters, and the body carries `has_more` and `next_offset` (the headers are set too). The web share page follows `next_offset` until `has_more` is false and merges the pages' items and `attachment_refs`
|
||||
- `GET /workspaces/{ws}/items/{slug}/progress` — child item completion progress
|
||||
- `GET/POST /workspaces/{ws}/items/{slug}/links` — item relationships (blocks/blocked-by, parent/child)
|
||||
- `GET /workspaces/{ws}/items/{slug}/context?intent=&role=&budget=` (PLAN-3598) — one deterministic brief for working an item: `item` (fields, body, seq), `parents` (the visible ancestor chain, nearest first, each with `acceptance_criteria` parsed by `internal/acceptance` and the nearest one's body), `conventions` (active ones whose trigger is `always` or the intent's, with bodies, sorted must → should → nice-to-have then ref), `playbooks` (metadata only), `deps` (`blocks` / `blocked_by` with statuses), `decisions` (TASK-3618: only the answers their set would surface, i.e. the question is current and the Noul reaches the set's threshold, `decision.AttentionThreshold` or `decision.ConventionThreshold`; each carries `current`, and a surfaced answer whose item has moved on stays, with `current: false` and a `stale_decision` conflict. The rest are only counted: `decisions_below_threshold`, and `decisions_stale_question` for answers to a question no longer asked as it was), `links` (other link types, ref/title/status, no bodies), `omitted`, and `conflicts`. Every part carries its ref and seq. **Intent** resolves against the conventions collection's declared trigger options: `on-<intent>` if declared, else `<intent>`; neither is 400 `unknown_intent` with `details.valid_intents`. No intent means `always` only. A caller who cannot see the conventions collection gets no validation and no list (the intent is taken as `on-<intent>`), so the 400 cannot reveal its declared triggers. **Role** must be a role slug in the workspace (400 `unknown_role`); with one, a convention scoped to another role (by its agent-role column, the one bootstrap labels with) is left out, and without one every role's are included, as bootstrap does. **Budget** (characters of serialized parts, default 40000, capped at 400000) admits whole parts in a fixed order: parents, conventions, playbooks, deps, decisions, links. The first part that does not fit and every part after it are listed in `omitted` (`part`, `ref`, `seq`, `size`, `reason: "budget"`), so a later small part never slips in after a refused one. The item always ships, even past the budget. **Visibility:** the item uses the single-item read check, so an archived item is served read-only to a caller who can read it, exactly as the single-item GET serves it, and an unreadable item is the same 404 as a missing one; every ancestor, dependency and linked item uses `itemVisibleFor` plus the app token's collection ceiling and live rows only; conventions and playbooks use bootstrap's visibility sets. A part the caller cannot see is absent and NOT counted in `omitted`. **`conflicts`** is structural only (no semantic comparison of rules): `open_blocker`, `open_under_closed_parent`, `stale_decision`, and `role_and_global_convention` (a role-scoped and a global convention matching the same trigger under the same title)
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
+41
-6
@@ -623,14 +623,49 @@ func (c *Client) UnstarItem(wsSlug, itemSlug string) error {
|
||||
return c.delete("/workspaces/" + url.PathEscape(wsSlug) + "/items/" + url.PathEscape(itemSlug) + "/star")
|
||||
}
|
||||
|
||||
// ListStarredItems returns the current user's starred items in a workspace.
|
||||
func (c *Client) ListStarredItems(wsSlug string, includeTerminal bool) ([]models.Item, error) {
|
||||
var result []models.Item
|
||||
path := "/workspaces/" + url.PathEscape(wsSlug) + "/starred"
|
||||
// ListStarredItemsPage returns at most limit of the current user's starred
|
||||
// items, from the start of the list, and whether more exist (BUG-3790: MCP
|
||||
// asks for one row past its cap and states the truncation; it never pages).
|
||||
func (c *Client) ListStarredItemsPage(wsSlug string, includeTerminal bool, limit int) ([]models.Item, ItemListPage, error) {
|
||||
q := url.Values{}
|
||||
if includeTerminal {
|
||||
path += "?include_terminal=true"
|
||||
q.Set("include_terminal", "true")
|
||||
}
|
||||
q.Set("limit", strconv.Itoa(limit))
|
||||
var page []models.Item
|
||||
h, err := c.getWithHeader("/workspaces/"+url.PathEscape(wsSlug)+"/starred?"+q.Encode(), &page)
|
||||
return page, itemListPage(h), err
|
||||
}
|
||||
|
||||
// ListStarredItems returns the current user's starred items in a workspace,
|
||||
// following the server's next-offset until has-more is false (BUG-3790: the
|
||||
// list is bounded per page).
|
||||
func (c *Client) ListStarredItems(wsSlug string, includeTerminal bool) ([]models.Item, error) {
|
||||
q := url.Values{}
|
||||
if includeTerminal {
|
||||
q.Set("include_terminal", "true")
|
||||
}
|
||||
var all []models.Item
|
||||
for offset := 0; ; {
|
||||
if offset > 0 {
|
||||
q.Set("offset", strconv.Itoa(offset))
|
||||
}
|
||||
var page []models.Item
|
||||
path := "/workspaces/" + url.PathEscape(wsSlug) + "/starred"
|
||||
if len(q) > 0 {
|
||||
path += "?" + q.Encode()
|
||||
}
|
||||
h, err := c.getWithHeader(path, &page)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
all = append(all, page...)
|
||||
p := itemListPage(h)
|
||||
if !p.HasMore || p.NextOffset <= offset {
|
||||
return all, nil
|
||||
}
|
||||
offset = p.NextOffset
|
||||
}
|
||||
return result, c.get(path, &result)
|
||||
}
|
||||
|
||||
// CreateWatch creates (or replaces the predicate on) a durable watch for
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/mark3labs/mcp-go/mcp"
|
||||
)
|
||||
|
||||
// BUG-3790 (lead ruling): pad_item starred never pages. Both transports ask
|
||||
// for one row past the list cap, and a result that had the extra row comes
|
||||
// back trimmed to the cap with has_more and the truncation note.
|
||||
|
||||
func bug3790Check(t *testing.T, res *mcp.CallToolResult, wantItems int, wantMore bool) {
|
||||
t.Helper()
|
||||
obj, ok := res.StructuredContent.(map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("structured content %T", res.StructuredContent)
|
||||
}
|
||||
if got := len(obj["items"].([]any)); got != wantItems {
|
||||
t.Fatalf("items %d, want %d", got, wantItems)
|
||||
}
|
||||
more, _ := obj["has_more"].(bool)
|
||||
if more != wantMore {
|
||||
t.Fatalf("has_more %v, want %v", more, wantMore)
|
||||
}
|
||||
note := len(res.Content) == 2 && strings.Contains(res.Content[1].(mcp.TextContent).Text, "More items match")
|
||||
if note != wantMore {
|
||||
t.Fatalf("note present %v, want %v", note, wantMore)
|
||||
}
|
||||
}
|
||||
|
||||
// Stdio: the CLI is asked for --limit 301.
|
||||
func TestBUG3790_StdioStarredAsksOnePastTheCapAndSaysTruncation(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
rows, want int
|
||||
more bool
|
||||
}{{3, 3, false}, {300, 300, false}, {301, 300, true}} {
|
||||
t.Run(fmt.Sprintf("rows=%d", tc.rows), func(t *testing.T) {
|
||||
disp := &listRowsDispatcher{rows: tc.rows}
|
||||
env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: disp}
|
||||
res, err := actionItemStarred(context.Background(), map[string]any{"all": true}, env)
|
||||
if err != nil || res == nil || res.IsError {
|
||||
t.Fatalf("actionItemStarred: %v %v", res, err)
|
||||
}
|
||||
args := strings.Join(disp.gotArgs, " ")
|
||||
if !strings.Contains(args, "--limit 301") || !strings.Contains(args, "--all") {
|
||||
t.Fatalf("args %q: want --limit 301 and --all", args)
|
||||
}
|
||||
bug3790Check(t, res, tc.want, tc.more)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Remote: the HTTP dispatcher sends ?limit=301 to /starred.
|
||||
func TestBUG3790_RemoteStarredAsksOnePastTheCapAndSaysTruncation(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
rows, want int
|
||||
more bool
|
||||
}{{3, 3, false}, {301, 300, true}} {
|
||||
t.Run(fmt.Sprintf("rows=%d", tc.rows), func(t *testing.T) {
|
||||
var gotURL string
|
||||
h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
gotURL = r.URL.String()
|
||||
items := make([]map[string]any, tc.rows)
|
||||
for i := range items {
|
||||
items[i] = map[string]any{"ref": fmt.Sprintf("TASK-%d", i+1)}
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(items)
|
||||
})
|
||||
disp := &HTTPHandlerDispatcher{
|
||||
Handler: h,
|
||||
UserResolver: func(context.Context) *models.User { return &models.User{ID: "u1", Name: "U"} },
|
||||
}
|
||||
env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: disp}
|
||||
res, err := actionItemStarred(context.Background(), map[string]any{}, env)
|
||||
if err != nil || res == nil || res.IsError {
|
||||
t.Fatalf("actionItemStarred: %v %v", res, err)
|
||||
}
|
||||
if !strings.HasPrefix(gotURL, "/api/v1/workspaces/docapp/starred?") || !strings.Contains(gotURL, "limit=301") {
|
||||
t.Fatalf("remote asked %q; want /starred with limit=301", gotURL)
|
||||
}
|
||||
bug3790Check(t, res, tc.want, tc.more)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// CONVE-19: the tool's starred action IS the truncating action, not a bare
|
||||
// pass-through to the CLI.
|
||||
func TestBUG3790_PadItemStarredIsBoundToTheTruncatingAction(t *testing.T) {
|
||||
handler, ok := padItemTool.Actions["starred"]
|
||||
if !ok {
|
||||
t.Fatal("pad_item has no starred action")
|
||||
}
|
||||
disp := &listRowsDispatcher{rows: 301}
|
||||
env := ActionEnv{Doc: liveCmdhelpDoc(t), Workspace: NewWorkspaceState("docapp"), Dispatcher: disp}
|
||||
res, err := handler(context.Background(), map[string]any{}, env)
|
||||
if err != nil || res == nil || res.IsError {
|
||||
t.Fatalf("starred: %v %v", res, err)
|
||||
}
|
||||
if !strings.Contains(strings.Join(disp.gotArgs, " "), "--limit 301") {
|
||||
t.Fatalf("args %v: the bound handler did not ask one past the cap", disp.gotArgs)
|
||||
}
|
||||
bug3790Check(t, res, 300, true)
|
||||
}
|
||||
@@ -115,7 +115,7 @@ var padItemTool = ToolDef{
|
||||
// Stars
|
||||
"star": passThrough([]string{"item", "star"}),
|
||||
"unstar": passThrough([]string{"item", "unstar"}),
|
||||
"starred": passThrough([]string{"item", "starred"}),
|
||||
"starred": actionItemStarred,
|
||||
|
||||
// Comments. reply-comment isn't a separate CLI verb — the
|
||||
// `comment` command takes a --reply-to flag, exposed here as
|
||||
@@ -291,7 +291,7 @@ var padItemSchemaParams = []ParamDef{
|
||||
|
||||
// ── List / starred ──
|
||||
{Name: "all", Type: "bool", Description: "Include archived/done items in list responses. Optional for: list, starred."},
|
||||
{Name: "limit", Type: "number", Description: "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions."},
|
||||
{Name: "limit", Type: "number", Description: "Maximum results. Optional for: list, backlinks, history. Each defaults to 50, max 300. For history the window is the NEWEST N versions. starred takes no limit: it returns at most 300, most recently starred first, with has_more when more exist."},
|
||||
{Name: "offset", Type: "number", Description: "Skip the first N results (paging). Optional for: backlinks."},
|
||||
{Name: "sort", Type: "string", Description: "Sort field. Optional for: list."},
|
||||
{Name: "group_by", Type: "string", Description: "Group-by field. Optional for: list."},
|
||||
@@ -900,6 +900,24 @@ func actionItemList(ctx context.Context, input map[string]any, env ActionEnv) (*
|
||||
return markItemListTruncation(res, limit), nil
|
||||
}
|
||||
|
||||
// actionItemStarred handles pad_item.action=starred. Like list it never pages
|
||||
// (BUG-3790, lead ruling, as BUG-3704 ruled for list): it asks for one row past
|
||||
// the list cap, on both transports (the HTTP mapper sends it as ?limit=, stdio
|
||||
// as pad item starred --limit), and a result that had the extra row says
|
||||
// has_more with the truncation note.
|
||||
func actionItemStarred(ctx context.Context, input map[string]any, env ActionEnv) (*CallToolResult, error) {
|
||||
out := make(map[string]any, len(input)+1)
|
||||
for k, v := range input {
|
||||
out[k] = v
|
||||
}
|
||||
out["limit"] = mcpItemListMaxLimit + 1
|
||||
res, err := env.Dispatch(ctx, []string{"item", "starred"}, out)
|
||||
if err != nil || res == nil || res.IsError {
|
||||
return res, err
|
||||
}
|
||||
return markItemListTruncation(res, mcpItemListMaxLimit), nil
|
||||
}
|
||||
|
||||
// itemListTruncatedNote is the text an agent sees when its list was cut short.
|
||||
const itemListTruncatedNote = "More items match than this list shows (has_more). Narrow the filters, or raise limit (max %d)."
|
||||
|
||||
|
||||
@@ -757,6 +757,7 @@ func liveCmdhelpDoc(t *testing.T) *cmdhelp.Document {
|
||||
Flags: map[string]cmdhelp.Flag{
|
||||
"workspace": {Type: "string"},
|
||||
"all": {Type: "bool"},
|
||||
"limit": {Type: "int"}, // BUG-3790
|
||||
},
|
||||
},
|
||||
"item comment": {
|
||||
|
||||
@@ -783,9 +783,17 @@ func mapItemStarred(input map[string]any) (string, string, []byte, error) {
|
||||
if workspace == "" {
|
||||
return "", "", nil, errWorkspaceRequired
|
||||
}
|
||||
urlPath := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/starred"
|
||||
q := url.Values{}
|
||||
if all, _ := input["all"].(bool); all {
|
||||
urlPath += "?include_terminal=true"
|
||||
q.Set("include_terminal", "true")
|
||||
}
|
||||
// actionItemStarred always sets it, one past the MCP cap (BUG-3790).
|
||||
if n, ok := numericInput(input["limit"]); ok && int(n) > 0 {
|
||||
q.Set("limit", strconv.Itoa(int(n)))
|
||||
}
|
||||
urlPath := "/api/v1/workspaces/" + url.PathEscape(workspace) + "/starred"
|
||||
if len(q) > 0 {
|
||||
urlPath += "?" + q.Encode()
|
||||
}
|
||||
return http.MethodGet, urlPath, nil, nil
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ Pad is a project tracker for developers and AI agents — issues (TASK, BUG), pl
|
||||
|
||||
If the user is asking general code questions with no project-management thread, you don't need this server.
|
||||
|
||||
## Tool surface (v0.98)
|
||||
## Tool surface (v0.99)
|
||||
|
||||
Ten resource × action tools, plus `pad_set_workspace`. Eleven tools total.
|
||||
|
||||
|
||||
+11
-1
@@ -1738,6 +1738,16 @@ const CmdhelpVersion = "0.1"
|
||||
// caller cannot read embeds nothing; a failed fetch stays non-fatal,
|
||||
// as on stdio. Stdio is unchanged.
|
||||
//
|
||||
// 0.99 — BUG-3790, starred truncation. ADDITIVE bump on the v0.53
|
||||
// grounds: `pad_item.action=starred` returns at most 300 items, most
|
||||
// recently starred first, and when more exist says `has_more: true`
|
||||
// on the structured result plus the same truncation note as list. It
|
||||
// never pages (lead ruling, as for list). Both transports ask for one
|
||||
// row past the cap: remote as GET /starred?limit=301, stdio as
|
||||
// `pad item starred --limit 301` (a new CLI flag; without it the CLI
|
||||
// walks every page). Starred used to return everything. The `limit`
|
||||
// param's description names starred; no name, enum or param moved.
|
||||
//
|
||||
// 0.98 — BUG-3704, item list truncation. ADDITIVE bump on the v0.53
|
||||
// grounds: `pad_item.action=list` asks for one row past its limit and,
|
||||
// when that row comes back, returns `limit` rows with `has_more: true`
|
||||
@@ -2177,7 +2187,7 @@ const CmdhelpVersion = "0.1"
|
||||
// this surface can receive it; the entry exists so a future action does
|
||||
// not collapse it to permission_denied. When an action that can reach
|
||||
// it is added, that addition is the contract change and owns the bump.
|
||||
const ToolSurfaceVersion = "0.98"
|
||||
const ToolSurfaceVersion = "0.99"
|
||||
|
||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||
|
||||
@@ -0,0 +1,398 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"sort"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3789, BUG-3790, BUG-3792: the children list, the starred list and a
|
||||
// shared collection are bounded like the item lists (BUG-3704). A missing or
|
||||
// invalid limit is the maximum, an explicit one is clamped, and the paging
|
||||
// state comes back in headers (and, on the share payload, in the body).
|
||||
|
||||
type bug3789Page struct {
|
||||
items []models.Item
|
||||
more string
|
||||
next string
|
||||
rawBody []byte
|
||||
}
|
||||
|
||||
func bug3789Get(t *testing.T, srv *Server, path, cookie string) bug3789Page {
|
||||
t.Helper()
|
||||
rr := doRequest(srv, "GET", path, nil)
|
||||
if cookie != "" {
|
||||
rr = doRequestWithCookie(srv, "GET", path, nil, cookie)
|
||||
}
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("%s: %d %s", path, rr.Code, rr.Body.String())
|
||||
}
|
||||
var items []models.Item
|
||||
_ = json.Unmarshal(rr.Body.Bytes(), &items)
|
||||
return bug3789Page{items, rr.Header().Get(itemListHasMoreHeader), rr.Header().Get(itemListNextOffsetHeader), rr.Body.Bytes()}
|
||||
}
|
||||
|
||||
// walk follows next-offset from offset 0, asking for pageSize rows a page.
|
||||
func bug3789Walk(t *testing.T, srv *Server, base, cookie string, pageSize int) ([]models.Item, int) {
|
||||
t.Helper()
|
||||
sep := "?"
|
||||
if containsByte(base, '?') {
|
||||
sep = "&"
|
||||
}
|
||||
var all []models.Item
|
||||
offset, pages := 0, 0
|
||||
for {
|
||||
p := bug3789Get(t, srv, fmt.Sprintf("%s%slimit=%d&offset=%d", base, sep, pageSize, offset), cookie)
|
||||
pages++
|
||||
all = append(all, p.items...)
|
||||
if p.more == "false" {
|
||||
if p.next != "" {
|
||||
t.Fatalf("last page carries next-offset %q", p.next)
|
||||
}
|
||||
return all, pages
|
||||
}
|
||||
if p.more != "true" || p.next != fmt.Sprint(offset+pageSize) {
|
||||
t.Fatalf("page at %d: has-more %q next %q", offset, p.more, p.next)
|
||||
}
|
||||
offset += pageSize
|
||||
if pages > 100 {
|
||||
t.Fatal("walk did not end")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func containsByte(s string, b byte) bool {
|
||||
for i := 0; i < len(s); i++ {
|
||||
if s[i] == b {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func TestBUG3789_ChildrenAreBoundedAndPaged(t *testing.T) {
|
||||
bothBackends(t, func(t *testing.T, srv *Server) {
|
||||
slug, wsID := task3462Workspace(t, srv, "Children 3789", "startup")
|
||||
tasks, _ := srv.store.GetCollectionBySlug(wsID, "tasks")
|
||||
parent, err := srv.store.CreateItem(wsID, tasks.ID, models.ItemCreate{Title: "Parent", Fields: `{"status":"open"}`})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
n := maxItemListQueryLimit + 1
|
||||
for i := 0; i < n; i++ {
|
||||
child, err := srv.store.CreateItem(wsID, tasks.ID, models.ItemCreate{Title: fmt.Sprintf("Child %04d", i), Fields: `{"status":"open"}`})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := srv.store.CreateItemLink(wsID, models.ItemLinkCreate{TargetID: parent.ID, LinkType: "parent"}, child.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
// Every child ties on the list order's leading keys.
|
||||
if _, err := srv.store.DB().Exec(srv.store.D().Rebind(`UPDATE items SET sort_order = 0, created_at = '2026-01-01T00:00:00Z' WHERE collection_id = ? AND id <> ?`), tasks.ID, parent.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
base := "/api/v1/workspaces/" + slug + "/items/" + parent.Slug + "/children"
|
||||
for _, q := range []string{"", "?limit=0", "?limit=-5", "?limit=abc", "?limit=5000"} {
|
||||
p := bug3789Get(t, srv, base+q, "")
|
||||
if len(p.items) != maxItemListQueryLimit || p.more != "true" || p.next != fmt.Sprint(maxItemListQueryLimit) {
|
||||
t.Fatalf("%s: %d rows, has-more %q, next %q", q, len(p.items), p.more, p.next)
|
||||
}
|
||||
}
|
||||
all, _ := bug3789Walk(t, srv, base, "", 400)
|
||||
seen := map[string]int{}
|
||||
for _, it := range all {
|
||||
seen[it.ID]++
|
||||
}
|
||||
if len(seen) != n || len(all) != n {
|
||||
t.Fatalf("walk returned %d rows, %d distinct; want %d each", len(all), len(seen), n)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// A restricted member's children window counts only children they may see,
|
||||
// and has_children reflects only visible grandchildren.
|
||||
func TestBUG3789_ChildrenWindowIsOverVisibleRows(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
ownerCookie := bootstrapFirstUser(t, srv, "owner@test.com", "Owner")
|
||||
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces", map[string]string{"name": "Vis 3789"}, ownerCookie)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var ws models.Workspace
|
||||
parseJSON(t, rr, &ws)
|
||||
tasks, _ := srv.store.GetCollectionBySlug(ws.ID, "tasks")
|
||||
ideas, _ := srv.store.GetCollectionBySlug(ws.ID, "ideas")
|
||||
mk := func(collID, title string, parent *models.Item) *models.Item {
|
||||
it, err := srv.store.CreateItem(ws.ID, collID, models.ItemCreate{Title: title})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if parent != nil {
|
||||
if _, err := srv.store.CreateItemLink(ws.ID, models.ItemLinkCreate{TargetID: parent.ID, LinkType: "parent"}, it.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
return it
|
||||
}
|
||||
parent := mk(tasks.ID, "Parent", nil)
|
||||
visible := mk(tasks.ID, "Visible child", parent)
|
||||
for i := 0; i < 3; i++ {
|
||||
mk(ideas.ID, fmt.Sprintf("Hidden child %d", i), parent)
|
||||
}
|
||||
mk(ideas.ID, "Hidden grandchild", visible)
|
||||
if _, err := srv.store.DB().Exec(srv.store.D().Rebind(`UPDATE items SET sort_order = 0, created_at = '2026-01-01T00:00:00Z' WHERE workspace_id = ?`), ws.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The visible child sorts first.
|
||||
if _, err := srv.store.DB().Exec(srv.store.D().Rebind(`UPDATE items SET sort_order = -1 WHERE id = ?`), visible.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
m, err := srv.store.CreateUser(models.UserCreate{Email: "restricted@test.com", Name: "Restricted", Password: "correct-horse-battery-staple"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.AddWorkspaceMember(ws.ID, m.ID, "viewer"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.SetMemberCollectionAccess(ws.ID, m.ID, "specific", []string{tasks.ID}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cookie, err := srv.store.CreateSession(m.ID, "web-test", "192.0.2.1", "", webSessionTTL)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
base := "/api/v1/workspaces/" + ws.Slug + "/items/" + parent.Slug + "/children"
|
||||
p := bug3789Get(t, srv, base+"?limit=1", cookie)
|
||||
if len(p.items) != 1 || p.items[0].ID != visible.ID || p.more != "false" {
|
||||
t.Fatalf("restricted page: %d rows, has-more %q; want only the visible child and has-more false", len(p.items), p.more)
|
||||
}
|
||||
if p.items[0].HasChildren {
|
||||
t.Fatal("has_children counts a grandchild the member cannot see")
|
||||
}
|
||||
owner := bug3789Get(t, srv, base, ownerCookie)
|
||||
if len(owner.items) != 4 {
|
||||
t.Fatalf("owner sees %d children, want 4", len(owner.items))
|
||||
}
|
||||
for _, it := range owner.items {
|
||||
if it.ID == visible.ID && !it.HasChildren {
|
||||
t.Fatal("owner: has_children misses the grandchild")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBUG3790_StarredIsBoundedPagedAndLoadsBodiesForThePage(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
ownerCookie := bootstrapFirstUser(t, srv, "owner@test.com", "Owner")
|
||||
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces", map[string]string{"name": "Stars 3790"}, ownerCookie)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var ws models.Workspace
|
||||
parseJSON(t, rr, &ws)
|
||||
owner, _ := srv.store.GetUserByEmail("owner@test.com")
|
||||
tasks, _ := srv.store.GetCollectionBySlug(ws.ID, "tasks")
|
||||
n := maxItemListQueryLimit + 1
|
||||
for i := 0; i < n; i++ {
|
||||
it, err := srv.store.CreateItem(ws.ID, tasks.ID, models.ItemCreate{Title: fmt.Sprintf("Star %04d", i), Content: fmt.Sprintf("body %04d", i), Fields: `{"status":"open"}`})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.StarItem(owner.ID, it.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
// Every star ties on starred-at.
|
||||
if _, err := srv.store.DB().Exec(`UPDATE item_stars SET created_at = '2026-01-01T00:00:00Z'`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
base := "/api/v1/workspaces/" + ws.Slug + "/starred"
|
||||
for _, q := range []string{"", "?limit=0", "?limit=abc", "?limit=5000"} {
|
||||
p := bug3789Get(t, srv, base+q, ownerCookie)
|
||||
if len(p.items) != maxItemListQueryLimit || p.more != "true" || p.next != fmt.Sprint(maxItemListQueryLimit) {
|
||||
t.Fatalf("%s: %d rows, has-more %q, next %q", q, len(p.items), p.more, p.next)
|
||||
}
|
||||
}
|
||||
all, _ := bug3789Walk(t, srv, base, ownerCookie, 300)
|
||||
seen := map[string]bool{}
|
||||
for _, it := range all {
|
||||
if seen[it.ID] {
|
||||
t.Fatalf("%s returned twice", it.Title)
|
||||
}
|
||||
seen[it.ID] = true
|
||||
if it.Content != "body "+it.Title[len("Star "):] {
|
||||
t.Fatalf("%s: body %q", it.Title, it.Content)
|
||||
}
|
||||
}
|
||||
if len(seen) != n {
|
||||
t.Fatalf("walk returned %d distinct, want %d", len(seen), n)
|
||||
}
|
||||
summary := bug3789Get(t, srv, base+"?summary=true&limit=5", ownerCookie)
|
||||
for _, it := range summary.items {
|
||||
if it.Content != "" {
|
||||
t.Fatalf("summary carries a body: %q", it.Content)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBUG3792_SharedCollectionIsBoundedAndPaged(t *testing.T) {
|
||||
bothBackends(t, func(t *testing.T, srv *Server) {
|
||||
_, wsID := task3462Workspace(t, srv, "Share 3792", "startup")
|
||||
tasks, _ := srv.store.GetCollectionBySlug(wsID, "tasks")
|
||||
n := maxItemListQueryLimit + 1
|
||||
for i := 0; i < n; i++ {
|
||||
if _, err := srv.store.CreateItem(wsID, tasks.ID, models.ItemCreate{Title: fmt.Sprintf("Shared %04d", i), Fields: `{"status":"open"}`}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
creator, err := srv.store.CreateUser(models.UserCreate{Email: "sharer@test.com", Name: "Sharer", Password: "correct-horse-battery-staple"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
link, err := srv.store.CreateShareLink(wsID, "collection", tasks.ID, "view", creator.ID, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
get := func(q string) (items []map[string]any, hasMore bool, next float64) {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest("GET", "/api/v1/s/"+link.Token+q, nil)
|
||||
rr := httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, req)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("%s: %d %s", q, rr.Code, rr.Body.String())
|
||||
}
|
||||
var body struct {
|
||||
Items []map[string]any `json:"items"`
|
||||
HasMore bool `json:"has_more"`
|
||||
NextOffset float64 `json:"next_offset"`
|
||||
}
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &body); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return body.Items, body.HasMore, body.NextOffset
|
||||
}
|
||||
for _, q := range []string{"", "?limit=0", "?limit=abc", "?limit=5000"} {
|
||||
items, more, next := get(q)
|
||||
if len(items) != maxItemListQueryLimit || !more || next != maxItemListQueryLimit {
|
||||
t.Fatalf("%s: %d items, has_more %v, next %v", q, len(items), more, next)
|
||||
}
|
||||
}
|
||||
seen := map[string]bool{}
|
||||
for offset := 0; ; {
|
||||
items, more, next := get(fmt.Sprintf("?limit=400&offset=%d", offset))
|
||||
for _, it := range items {
|
||||
ref := fmt.Sprint(it["ref"])
|
||||
if seen[ref] {
|
||||
t.Fatalf("%s returned twice", ref)
|
||||
}
|
||||
seen[ref] = true
|
||||
}
|
||||
if !more {
|
||||
break
|
||||
}
|
||||
offset = int(next)
|
||||
}
|
||||
if len(seen) != n {
|
||||
t.Fatalf("walk returned %d items, want %d", len(seen), n)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Rows that tie on every leading sort key come back in id order. The links and
|
||||
// stars are written in DESCENDING id order, so the scan order the database
|
||||
// would fall back to without the id tie-break is the reverse: an order that is
|
||||
// not total lets offset pages repeat or skip rows (SQLite happens to keep
|
||||
// paging consistent by scan order; Postgres does not promise it).
|
||||
func TestBUG3789_TiesComeBackInIDOrder(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
ownerCookie := bootstrapFirstUser(t, srv, "owner@test.com", "Owner")
|
||||
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces", map[string]string{"name": "Ties 3789"}, ownerCookie)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var ws models.Workspace
|
||||
parseJSON(t, rr, &ws)
|
||||
owner, _ := srv.store.GetUserByEmail("owner@test.com")
|
||||
tasks, _ := srv.store.GetCollectionBySlug(ws.ID, "tasks")
|
||||
parent, err := srv.store.CreateItem(ws.ID, tasks.ID, models.ItemCreate{Title: "Parent"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var ids []string
|
||||
for i := 0; i < 6; i++ {
|
||||
it, err := srv.store.CreateItem(ws.ID, tasks.ID, models.ItemCreate{Title: fmt.Sprintf("Tie %d", i)})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ids = append(ids, it.ID)
|
||||
}
|
||||
sort.Strings(ids)
|
||||
for i := len(ids) - 1; i >= 0; i-- {
|
||||
if _, err := srv.store.CreateItemLink(ws.ID, models.ItemLinkCreate{TargetID: parent.ID, LinkType: "parent"}, ids[i]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.StarItem(owner.ID, ids[i]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if _, err := srv.store.DB().Exec(srv.store.D().Rebind(`UPDATE items SET sort_order = 0, created_at = '2026-01-01T00:00:00Z' WHERE collection_id = ? AND id <> ?`), tasks.ID, parent.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := srv.store.DB().Exec(`UPDATE item_stars SET created_at = '2026-01-01T00:00:00Z'`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, path := range []string{
|
||||
"/api/v1/workspaces/" + ws.Slug + "/items/" + parent.Slug + "/children",
|
||||
"/api/v1/workspaces/" + ws.Slug + "/starred",
|
||||
} {
|
||||
all, pages := bug3789Walk(t, srv, path, ownerCookie, 2)
|
||||
var got []string
|
||||
for _, it := range all {
|
||||
got = append(got, it.ID)
|
||||
}
|
||||
if pages != 3 || fmt.Sprint(got) != fmt.Sprint(ids) {
|
||||
t.Fatalf("%s: %d pages, order %v; want 3 pages in id order %v", path, pages, got, ids)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Lead check (#2153): an offset past the end is an empty page that serialises
|
||||
// as [] (not null), with has-more false, on both bare-array lists.
|
||||
func TestBUG3789_AnOffsetPastTheEndIsAnEmptyArray(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
ownerCookie := bootstrapFirstUser(t, srv, "owner@test.com", "Owner")
|
||||
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces", map[string]string{"name": "End 3789"}, ownerCookie)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var ws models.Workspace
|
||||
parseJSON(t, rr, &ws)
|
||||
owner, _ := srv.store.GetUserByEmail("owner@test.com")
|
||||
tasks, _ := srv.store.GetCollectionBySlug(ws.ID, "tasks")
|
||||
parent, _ := srv.store.CreateItem(ws.ID, tasks.ID, models.ItemCreate{Title: "Parent"})
|
||||
child, _ := srv.store.CreateItem(ws.ID, tasks.ID, models.ItemCreate{Title: "Child"})
|
||||
if _, err := srv.store.CreateItemLink(ws.ID, models.ItemLinkCreate{TargetID: parent.ID, LinkType: "parent"}, child.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.StarItem(owner.ID, child.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, path := range []string{
|
||||
"/api/v1/workspaces/" + ws.Slug + "/starred?offset=5",
|
||||
"/api/v1/workspaces/" + ws.Slug + "/items/" + parent.Slug + "/children?offset=5",
|
||||
} {
|
||||
p := bug3789Get(t, srv, path, ownerCookie)
|
||||
if body := string(p.rawBody); body != "[]\n" && body != "[]" {
|
||||
t.Fatalf("%s: body %q, want []", path, body)
|
||||
}
|
||||
if p.more != "false" || p.next != "" {
|
||||
t.Fatalf("%s: has-more %q next %q", path, p.more, p.next)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -3921,16 +3921,8 @@ func (s *Server) handleGetItemChildren(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
children, err := s.store.GetChildItems(item.ID)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
if children == nil {
|
||||
children = []models.Item{}
|
||||
}
|
||||
|
||||
// Filter children by collection visibility and item-level grants
|
||||
// Visibility is applied in the query and the list is windowed (BUG-3789),
|
||||
// so the has-more probe counts only children the caller may see.
|
||||
visibleIDs, visErr := s.visibleCollectionIDs(r, workspaceID)
|
||||
if visErr != nil {
|
||||
writeInternalError(w, visErr)
|
||||
@@ -3941,36 +3933,36 @@ func (s *Server) handleGetItemChildren(w http.ResponseWriter, r *http.Request) {
|
||||
writeInternalError(w, grantErr)
|
||||
return
|
||||
}
|
||||
limit, offset := parseListWindow(r)
|
||||
params := store.ChildListParams{Limit: limit + 1, Offset: offset}
|
||||
if visibleIDs != nil {
|
||||
filtered := children[:0]
|
||||
for _, child := range children {
|
||||
if !isCollectionVisible(child.CollectionID, visibleIDs) {
|
||||
continue
|
||||
}
|
||||
if !s.isItemVisibleToGuest(r, workspaceID, &child, fullCollIDs, grantedItemIDs) {
|
||||
continue
|
||||
}
|
||||
filtered = append(filtered, child)
|
||||
}
|
||||
children = filtered
|
||||
params.CollectionIDs = visibleIDs
|
||||
params.FullCollectionIDs, params.GrantedItemIDs = fullCollIDs, grantedItemIDs
|
||||
}
|
||||
children, err := s.store.ListChildItems(item.ID, params)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
children = setListWindowHeaders(w, children, limit, offset)
|
||||
if children == nil {
|
||||
children = []models.Item{}
|
||||
}
|
||||
|
||||
s.enrichItemsWithParent(r, workspaceID, children, visibleIDs)
|
||||
if visibleIDs != nil {
|
||||
// Visibility-aware has_children: only count visible grandchildren
|
||||
// One visible grandchild decides it; the probe reads one row
|
||||
// (BUG-3789: it read every grandchild, with bodies, per child).
|
||||
probe := params
|
||||
probe.Limit, probe.Offset = 1, 0
|
||||
for i := range children {
|
||||
grandchildren, _ := s.store.GetChildItems(children[i].ID)
|
||||
children[i].HasChildren = false
|
||||
for _, gc := range grandchildren {
|
||||
if !isCollectionVisible(gc.CollectionID, visibleIDs) {
|
||||
continue
|
||||
}
|
||||
if !s.isItemVisibleToGuest(r, workspaceID, &gc, fullCollIDs, grantedItemIDs) {
|
||||
continue
|
||||
}
|
||||
children[i].HasChildren = true
|
||||
break
|
||||
grandchildren, err := s.store.ListChildItems(children[i].ID, probe)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
children[i].HasChildren = len(grandchildren) > 0
|
||||
}
|
||||
} else {
|
||||
s.store.PopulateHasChildren(children)
|
||||
@@ -4565,6 +4557,33 @@ func (s *Server) listItemsPage(w http.ResponseWriter, workspaceID string, params
|
||||
return items, nil
|
||||
}
|
||||
|
||||
// parseListWindow reads ?limit= and ?offset= for a bounded list that is not an
|
||||
// item list (BUG-3789, BUG-3790, BUG-3792), as parseItemListParams reads them
|
||||
// for the item lists (BUG-3704): a missing, zero, negative or invalid limit is
|
||||
// the maximum, an explicit one is clamped to it, and a bad offset is 0.
|
||||
func parseListWindow(r *http.Request) (limit, offset int) {
|
||||
limit = maxItemListQueryLimit
|
||||
if l, err := strconv.Atoi(r.URL.Query().Get("limit")); err == nil && l > 0 && l < limit {
|
||||
limit = l
|
||||
}
|
||||
if o, err := strconv.Atoi(r.URL.Query().Get("offset")); err == nil && o > 0 {
|
||||
offset = o
|
||||
}
|
||||
return limit, offset
|
||||
}
|
||||
|
||||
// setListWindowHeaders takes rows read one past limit, trims the probe row and
|
||||
// sets the paging headers the item lists set (BUG-3704).
|
||||
func setListWindowHeaders[T any](w http.ResponseWriter, rows []T, limit, offset int) []T {
|
||||
if len(rows) > limit {
|
||||
w.Header().Set(itemListHasMoreHeader, "true")
|
||||
w.Header().Set(itemListNextOffsetHeader, strconv.Itoa(offset+limit))
|
||||
return rows[:limit]
|
||||
}
|
||||
w.Header().Set(itemListHasMoreHeader, "false")
|
||||
return rows
|
||||
}
|
||||
|
||||
// maxListFilterKeys bounds the field filters (field filters and any-of
|
||||
// filters together) one list request may carry (BUG-3703).
|
||||
const maxListFilterKeys = 50
|
||||
|
||||
@@ -659,14 +659,22 @@ func (s *Server) handleResolveShareLink(w http.ResponseWriter, r *http.Request)
|
||||
// share page renders items in the order they arrive, because the public
|
||||
// payload carries no sort columns. ListItems' default (pinned, then
|
||||
// updated_at DESC) reshuffled a shared collection on every edit.
|
||||
// Bounded and paged (BUG-3792): an anonymous reader used to get every
|
||||
// item with its body in one response. The order ends on the id, so
|
||||
// offset pages neither repeat nor skip rows.
|
||||
limit, offset := parseListWindow(r)
|
||||
items, err := s.store.ListItems(link.WorkspaceID, models.ItemListParams{
|
||||
ScopeCollectionID: coll.ID,
|
||||
Sort: "sort_order:asc,created_at:asc,_id:asc",
|
||||
Limit: limit + 1,
|
||||
Offset: offset,
|
||||
})
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
hasMore := len(items) > limit
|
||||
items = setListWindowHeaders(w, items, limit, offset)
|
||||
// Build public item list with only safe fields. content is the
|
||||
// item's markdown body, included so the public viewer can render an
|
||||
// inline read-only row expand (TASK-1678 / TASK-1684) without a
|
||||
@@ -758,7 +766,11 @@ func (s *Server) handleResolveShareLink(w http.ResponseWriter, r *http.Request)
|
||||
"type": "collection",
|
||||
"collection": publicCollection,
|
||||
"items": publicItems,
|
||||
"permission": "view",
|
||||
// The page's place in the list (BUG-3792); the viewer asks again
|
||||
// with ?offset=next_offset until has_more is false.
|
||||
"has_more": hasMore,
|
||||
"next_offset": offset + len(items),
|
||||
"permission": "view",
|
||||
"share_link": map[string]interface{}{
|
||||
"target_type": link.TargetType,
|
||||
},
|
||||
|
||||
@@ -133,7 +133,7 @@ func (s *Server) handleUnstarItem(w http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
}
|
||||
|
||||
// handleListStarredItems returns all starred items for the authenticated user in a workspace.
|
||||
// handleListStarredItems returns one bounded page of the authenticated user's starred items in a workspace (BUG-3790).
|
||||
// GET /api/v1/workspaces/{slug}/starred
|
||||
// Query params:
|
||||
// - include_terminal=true — include items in terminal statuses (default: false)
|
||||
@@ -156,7 +156,8 @@ func (s *Server) handleListStarredItems(w http.ResponseWriter, r *http.Request)
|
||||
|
||||
includeTerminal := r.URL.Query().Get("include_terminal") == "true"
|
||||
|
||||
items, err := s.store.ListStarredItems(userID, workspaceID, includeTerminal)
|
||||
// Bodies are loaded below for the returned page only (BUG-3790).
|
||||
items, err := s.store.ListStarredItemsWithoutBodies(userID, workspaceID, includeTerminal)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
@@ -208,12 +209,33 @@ func (s *Server) handleListStarredItems(w http.ResponseWriter, r *http.Request)
|
||||
}
|
||||
items = filtered
|
||||
|
||||
// The list is bounded and paged like the item lists (BUG-3790). The
|
||||
// window comes after the visibility and terminal filters, which run in Go,
|
||||
// so has-more counts only rows this caller is served.
|
||||
limit, offset := parseListWindow(r)
|
||||
if offset >= len(items) {
|
||||
items = nil
|
||||
} else {
|
||||
items = items[offset:min(len(items), offset+limit+1)]
|
||||
}
|
||||
items = setListWindowHeaders(w, items, limit, offset)
|
||||
|
||||
// Hydrate items with parent links and computed refs
|
||||
s.enrichItemsWithParent(r, workspaceID, items, visibleIDs)
|
||||
|
||||
if r.URL.Query().Get("summary") == "true" {
|
||||
if r.URL.Query().Get("summary") != "true" {
|
||||
ids := make([]string, len(items))
|
||||
for i := range items {
|
||||
items[i].Content = ""
|
||||
ids[i] = items[i].ID
|
||||
}
|
||||
bodies, err := s.store.ItemBodies(ids)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
for i := range items {
|
||||
b := bodies[items[i].ID]
|
||||
items[i].Content, items[i].ContentState = b.Content, b.ContentState
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -94,8 +94,25 @@ func (s *Store) AreItemsStarred(userID string, itemIDs []string) (map[string]boo
|
||||
// collection and assignment info. Only non-deleted items are returned.
|
||||
// If includeTerminal is false, items in terminal statuses are excluded.
|
||||
func (s *Store) ListStarredItems(userID, workspaceID string, includeTerminal bool) ([]models.Item, error) {
|
||||
return s.listStarredItems(userID, workspaceID, includeTerminal, true)
|
||||
}
|
||||
|
||||
// ListStarredItemsWithoutBodies is ListStarredItems with every body (and its
|
||||
// content_state) left empty (BUG-3790): the starred door windows the list after
|
||||
// filtering it, and loads bodies only for the page it returns (ItemBodies).
|
||||
func (s *Store) ListStarredItemsWithoutBodies(userID, workspaceID string, includeTerminal bool) ([]models.Item, error) {
|
||||
return s.listStarredItems(userID, workspaceID, includeTerminal, false)
|
||||
}
|
||||
|
||||
func (s *Store) listStarredItems(userID, workspaceID string, includeTerminal, withBodies bool) ([]models.Item, error) {
|
||||
// No content_state beside an empty body: it would describe content this
|
||||
// query does not return (BUG-3000).
|
||||
bodyCols := "'', ''"
|
||||
if withBodies {
|
||||
bodyCols = "i.content, " + contentStateSQL
|
||||
}
|
||||
query := `
|
||||
SELECT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.content, ` + contentStateSQL + `, i.fields, i.tags,
|
||||
SELECT i.id, i.workspace_id, i.collection_id, i.title, i.slug, ` + bodyCols + `, i.fields, i.tags,
|
||||
i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order,
|
||||
i.created_by, i.last_modified_by, i.source,
|
||||
i.item_number, i.seq, i.created_at, i.updated_at,
|
||||
@@ -110,7 +127,7 @@ func (s *Store) ListStarredItems(userID, workspaceID string, includeTerminal boo
|
||||
WHERE s.user_id = ?
|
||||
AND i.workspace_id = ?
|
||||
AND i.deleted_at IS NULL
|
||||
ORDER BY s.created_at DESC
|
||||
ORDER BY s.created_at DESC, i.id ASC
|
||||
`
|
||||
|
||||
rows, err := s.db.Query(s.q(query), userID, workspaceID)
|
||||
@@ -234,3 +251,36 @@ func (s *Store) DeleteStarsForItem(itemID string) error {
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ItemBody is one item's body and its content_state.
|
||||
type ItemBody struct {
|
||||
Content string
|
||||
ContentState string
|
||||
}
|
||||
|
||||
// ItemBodies loads the body and content_state of the given items, keyed by id
|
||||
// (BUG-3790: a door that windows a body-less list fills in only its page).
|
||||
func (s *Store) ItemBodies(ids []string) (map[string]ItemBody, error) {
|
||||
out := make(map[string]ItemBody, len(ids))
|
||||
if len(ids) == 0 {
|
||||
return out, nil
|
||||
}
|
||||
args := make([]any, len(ids))
|
||||
for i, id := range ids {
|
||||
args[i] = id
|
||||
}
|
||||
rows, err := s.db.Query(s.q(`SELECT i.id, i.content, `+contentStateSQL+` FROM items i WHERE i.id IN (`+placeholders(len(ids))+`)`), args...)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("item bodies: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var id string
|
||||
var b ItemBody
|
||||
if err := rows.Scan(&id, &b.Content, &b.ContentState); err != nil {
|
||||
return nil, fmt.Errorf("item bodies: %w", err)
|
||||
}
|
||||
out[id] = b
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
@@ -5478,6 +5478,83 @@ func (s *Store) getChildItems(q childQueryer, parentItemID string, includeDelete
|
||||
return scanItems(rows)
|
||||
}
|
||||
|
||||
// ChildListParams selects one page of ListChildItems (BUG-3789).
|
||||
type ChildListParams struct {
|
||||
// Limit and Offset window the children in their list order. Limit <= 0
|
||||
// returns every child: in-process callers (progress, guards) need them
|
||||
// whole, and only the HTTP door bounds the list.
|
||||
Limit, Offset int
|
||||
// CollectionIDs nil means every collection; non-nil restricts to those
|
||||
// (empty: none). With FullCollectionIDs or GrantedItemIDs non-nil, a child
|
||||
// must also be a granted item or sit in a full-grant collection, as
|
||||
// isItemVisibleToGuest decides it.
|
||||
CollectionIDs []string
|
||||
FullCollectionIDs []string
|
||||
GrantedItemIDs []string
|
||||
}
|
||||
|
||||
// ListChildItems is GetChildItems with the caller's visibility applied in the
|
||||
// query and an optional window over it (BUG-3789). Visibility is in the
|
||||
// query, not after it, so a window and its has-more probe count only rows the
|
||||
// caller may see. The order ends on the id, so offset pages neither repeat
|
||||
// nor skip children that tie on sort_order and created_at.
|
||||
func (s *Store) ListChildItems(parentItemID string, p ChildListParams) ([]models.Item, error) {
|
||||
if p.CollectionIDs != nil && len(p.CollectionIDs) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
query := `
|
||||
SELECT DISTINCT i.id, i.workspace_id, i.collection_id, i.title, i.slug, i.content, ` + contentStateSQL + `, i.fields, i.tags,
|
||||
i.pinned, i.sort_order, i.parent_id, i.assigned_user_id, i.agent_role_id, i.role_sort_order,
|
||||
i.created_by, i.last_modified_by, i.source,
|
||||
i.item_number, i.seq, i.created_at, i.updated_at,
|
||||
c.slug, c.name, c.icon, c.prefix,
|
||||
COALESCE(au.name, ''), COALESCE(au.email, ''),
|
||||
COALESCE(ar.name, ''), COALESCE(ar.slug, ''), COALESCE(ar.icon, ''), i.deleted_at
|
||||
FROM items i
|
||||
JOIN collections c ON c.id = i.collection_id
|
||||
JOIN item_links il ON il.source_id = i.id AND il.link_type IN (` + childLinkTypeSQL() + `) AND il.target_id = ?
|
||||
LEFT JOIN users au ON au.id = i.assigned_user_id
|
||||
LEFT JOIN agent_roles ar ON ar.id = i.agent_role_id
|
||||
WHERE i.deleted_at IS NULL AND c.deleted_at IS NULL`
|
||||
args := []interface{}{parentItemID}
|
||||
if p.CollectionIDs != nil {
|
||||
query += " AND i.collection_id IN (" + placeholders(len(p.CollectionIDs)) + ")"
|
||||
for _, id := range p.CollectionIDs {
|
||||
args = append(args, id)
|
||||
}
|
||||
if p.FullCollectionIDs != nil || p.GrantedItemIDs != nil {
|
||||
var alts []string
|
||||
if len(p.FullCollectionIDs) > 0 {
|
||||
alts = append(alts, "i.collection_id IN ("+placeholders(len(p.FullCollectionIDs))+")")
|
||||
for _, id := range p.FullCollectionIDs {
|
||||
args = append(args, id)
|
||||
}
|
||||
}
|
||||
if len(p.GrantedItemIDs) > 0 {
|
||||
alts = append(alts, "i.id IN ("+placeholders(len(p.GrantedItemIDs))+")")
|
||||
for _, id := range p.GrantedItemIDs {
|
||||
args = append(args, id)
|
||||
}
|
||||
}
|
||||
if len(alts) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
query += " AND (" + strings.Join(alts, " OR ") + ")"
|
||||
}
|
||||
}
|
||||
query += " ORDER BY i.sort_order ASC, i.created_at ASC, i.id ASC"
|
||||
if p.Limit > 0 {
|
||||
query += " LIMIT ? OFFSET ?"
|
||||
args = append(args, p.Limit, max(p.Offset, 0))
|
||||
}
|
||||
rows, err := s.db.Query(s.q(query), args...)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list child items: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
return scanItems(rows)
|
||||
}
|
||||
|
||||
// GetChildItemsForParents returns the live child items for each of the given
|
||||
// parent item IDs, grouped by parent ID, in ONE query — collapsing the
|
||||
// per-parent GetChildItems N+1 the dashboard used to run once per active plan
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { api } from './client';
|
||||
import { walkOffsetPages } from './listWalk';
|
||||
import { walkSharePages, type SharePage } from './shareWalk';
|
||||
|
||||
// BUG-3789 / BUG-3790 / BUG-3792: the children list, the starred list and a
|
||||
// shared collection are bounded per page, so the client walks every page.
|
||||
|
||||
const row = (id: string) => ({ id });
|
||||
const coll = (items: string[], more: boolean, next?: number, refs?: Record<string, unknown>): SharePage =>
|
||||
({ type: 'collection', collection: { name: 'C' }, items: items.map((ref) => ({ ref, title: ref })), has_more: more, next_offset: next, attachment_refs: refs }) as unknown as SharePage;
|
||||
|
||||
describe('walkOffsetPages', () => {
|
||||
it('asks page after page until a short page, de-duplicating by id', async () => {
|
||||
const asked: [number, number][] = [];
|
||||
const pages = [[row('a'), row('b')], [row('b'), row('c')], [row('d')]];
|
||||
const got = await walkOffsetPages(async (limit, offset) => {
|
||||
asked.push([limit, offset]);
|
||||
return pages[asked.length - 1];
|
||||
}, 2);
|
||||
expect(asked).toEqual([[2, 0], [2, 2], [2, 4]]);
|
||||
expect(got.map((r) => r.id)).toEqual(['a', 'b', 'c', 'd']);
|
||||
});
|
||||
|
||||
it('an exact multiple costs one empty page', async () => {
|
||||
const pages = [[row('a'), row('b')], []];
|
||||
let n = 0;
|
||||
const got = await walkOffsetPages(async () => pages[n++], 2);
|
||||
expect(n).toBe(2);
|
||||
expect(got).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('fails past the page bound', async () => {
|
||||
let n = 0;
|
||||
await expect(walkOffsetPages(async () => [row(`r${n++}`), row(`s${n++}`)], 2, 3)).rejects.toThrow('3 pages');
|
||||
});
|
||||
});
|
||||
|
||||
describe('walkSharePages', () => {
|
||||
it('follows next_offset and merges items and attachment refs', async () => {
|
||||
const asked: number[] = [];
|
||||
const pages = [coll(['A', 'B'], true, 2, { x: { mime_type: 'image/png', filename: 'x' } }), coll(['C'], false, 3, { y: { mime_type: 'image/png', filename: 'y' } })];
|
||||
const got = (await walkSharePages(async (o) => {
|
||||
asked.push(o);
|
||||
return pages[asked.length - 1];
|
||||
})) as SharePage;
|
||||
expect(asked).toEqual([0, 2]);
|
||||
expect(got.items?.map((i) => i.ref)).toEqual(['A', 'B', 'C']);
|
||||
expect(Object.keys(got.attachment_refs ?? {})).toEqual(['x', 'y']);
|
||||
expect(got.has_more).toBe(false);
|
||||
expect(got.next_offset).toBeUndefined();
|
||||
});
|
||||
|
||||
it('an item share, a refusal or an older server is one page', async () => {
|
||||
for (const first of [{ type: 'item', item: { title: 't' } }, { require_password: true }, { type: 'collection', items: [] }] as SharePage[]) {
|
||||
let n = 0;
|
||||
expect(await walkSharePages(async () => (n++, first))).toBe(first);
|
||||
expect(n).toBe(1);
|
||||
}
|
||||
});
|
||||
|
||||
it('fails past the bound, and on has_more without a usable next_offset', async () => {
|
||||
await expect(walkSharePages(async (o) => coll([`I${o}`], true, o + 1), 3)).rejects.toThrow('3 pages');
|
||||
await expect(walkSharePages(async () => coll(['A'], true, undefined))).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
// CONVE-19: the API methods the pages call are the walks.
|
||||
describe('API wiring (BUG-3789 / BUG-3790 / BUG-3792)', () => {
|
||||
afterEach(() => vi.unstubAllGlobals());
|
||||
|
||||
function stub(bodies: unknown[]) {
|
||||
const urls: string[] = [];
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn(async (url: string) => {
|
||||
urls.push(String(url));
|
||||
return new Response(JSON.stringify(bodies[urls.length - 1]), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
||||
})
|
||||
);
|
||||
return urls;
|
||||
}
|
||||
const full = (p: string) => Array.from({ length: 1000 }, (_, i) => row(`${p}${i}`));
|
||||
const tail = (u: string) => u.slice(u.indexOf('/workspaces/') >= 0 ? u.indexOf('/workspaces/') : u.indexOf('/s/'));
|
||||
|
||||
it('children walks by offset', async () => {
|
||||
const urls = stub([full('c'), [row('last')]]);
|
||||
const got = await api.items.children('ws', 'p');
|
||||
expect(got).toHaveLength(1001);
|
||||
expect(urls.map(tail)).toEqual(['/workspaces/ws/items/p/children?limit=1000&offset=0', '/workspaces/ws/items/p/children?limit=1000&offset=1000']);
|
||||
});
|
||||
|
||||
it('starred walks by offset and keeps its params', async () => {
|
||||
const urls = stub([full('s'), []]);
|
||||
await api.items.starred('ws', { include_terminal: true, summary: true });
|
||||
expect(urls.map(tail)).toEqual([
|
||||
'/workspaces/ws/starred?include_terminal=true&summary=true&limit=1000&offset=0',
|
||||
'/workspaces/ws/starred?include_terminal=true&summary=true&limit=1000&offset=1000'
|
||||
]);
|
||||
});
|
||||
|
||||
it('share.get walks a shared collection', async () => {
|
||||
const urls = stub([coll(['A'], true, 1000), coll(['B'], false, 1001)]);
|
||||
const got = await api.share.get('tok');
|
||||
expect(got.items?.map((i) => i.ref)).toEqual(['A', 'B']);
|
||||
expect(urls.map(tail)).toEqual(['/s/tok', '/s/tok?offset=1000']);
|
||||
});
|
||||
});
|
||||
@@ -111,6 +111,8 @@ import { runSessionExpiryKeepers } from '$lib/stores/sessionExpiryKeep';
|
||||
import { reportWorkspaceWrite } from './workspaceWrites';
|
||||
import { noteServerDate } from './serverClock';
|
||||
import { walkChanges, type ChangesPageQuery } from './changesWalk';
|
||||
import { walkSharePages, type SharePage } from './shareWalk';
|
||||
import { walkOffsetPages } from './listWalk';
|
||||
import { uploadImportBundle } from './importUpload';
|
||||
import { reserveKeepalive } from './keepaliveBudget';
|
||||
|
||||
@@ -2220,17 +2222,19 @@ export const api = {
|
||||
})}`
|
||||
),
|
||||
|
||||
/** Get child items linked to a parent item */
|
||||
/** Get EVERY child item linked to a parent item, walking the bounded
|
||||
* pages (BUG-3789). */
|
||||
children: (ws: string, slug: string) =>
|
||||
request<Item[]>(`/workspaces/${ws}/items/${slug}/children`),
|
||||
walkOffsetPages((limit, offset) =>
|
||||
request<Item[]>(`/workspaces/${ws}/items/${slug}/children${qs({ limit, offset })}`)
|
||||
),
|
||||
|
||||
/** Get completion progress for an item's children */
|
||||
progress: (ws: string, slug: string) =>
|
||||
request<{total: number; done: number; percentage: number}>(`/workspaces/${ws}/items/${slug}/progress`),
|
||||
|
||||
/** @deprecated Use children() */
|
||||
tasks: (ws: string, slug: string) =>
|
||||
request<Item[]>(`/workspaces/${ws}/items/${slug}/children`),
|
||||
tasks: (ws: string, slug: string) => api.items.children(ws, slug),
|
||||
|
||||
/** @deprecated Use progress() per-item instead */
|
||||
plansProgress: (ws: string) =>
|
||||
@@ -2308,7 +2312,9 @@ export const api = {
|
||||
/** List all starred items in a workspace for the current user, most
|
||||
* recently starred first. `summary` leaves out each body (TASK-2231). */
|
||||
starred: (ws: string, params?: {include_terminal?: boolean; summary?: boolean}) =>
|
||||
request<Item[]>(`/workspaces/${ws}/starred${qs(params)}`),
|
||||
walkOffsetPages((limit, offset) =>
|
||||
request<Item[]>(`/workspaces/${ws}/starred${qs({ ...params, limit, offset })}`)
|
||||
),
|
||||
|
||||
/**
|
||||
* Push an instruction about this item to the caller's OWN agent
|
||||
@@ -2925,19 +2931,24 @@ export const api = {
|
||||
// ── Public Share (no auth) ──────────────────────────────────────────────
|
||||
|
||||
share: {
|
||||
get: (token: string, password?: string): Promise<SharePayload> => {
|
||||
/** A shared item, or EVERY page of a shared collection (BUG-3792). */
|
||||
get: (token: string, password?: string): Promise<SharePayload> =>
|
||||
walkSharePages((offset) => api.share.page(token, password, offset)),
|
||||
/** One page of a share link; a collection's items start at `offset`. */
|
||||
page: (token: string, password?: string, offset = 0): Promise<SharePage> => {
|
||||
const headers: Record<string, string> = {};
|
||||
if (password) headers['X-Share-Password'] = password;
|
||||
const qs = offset > 0 ? `?offset=${offset}` : '';
|
||||
// Under the API deadline (BUG-3216). A GET, so a timeout is idempotent.
|
||||
return withRequestDeadline(
|
||||
async (signal) => {
|
||||
const resp = await fetch(`${BASE}/s/${token}`, { credentials: 'same-origin', headers, signal });
|
||||
const resp = await fetch(`${BASE}/s/${token}${qs}`, { credentials: 'same-origin', headers, signal });
|
||||
if (!resp.ok) {
|
||||
const body = await resp.json().catch(() => null);
|
||||
if (body?.error) throw new PadApiError(body.error);
|
||||
throw new Error(`API error: ${resp.status}`);
|
||||
}
|
||||
return (await resp.json()) as SharePayload;
|
||||
return (await resp.json()) as SharePage;
|
||||
},
|
||||
{ idempotent: true }
|
||||
);
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
/** The server's page ceiling for bounded lists (BUG-3704, BUG-3789). */
|
||||
export const LIST_PAGE_SIZE = 1000;
|
||||
|
||||
/** The most pages one list walk follows before failing (50,000 rows). */
|
||||
export const MAX_LIST_PAGES = 50;
|
||||
|
||||
/**
|
||||
* Pull every page of a bounded bare-array list by offset (BUG-3789, BUG-3790).
|
||||
* The body carries no paging state, and `request` does not expose the
|
||||
* has-more headers, so a page shorter than the size asked for is the last;
|
||||
* a list of exactly a multiple of the size costs one empty request. Rows are
|
||||
* de-duplicated by id, first occurrence kept, since a row that moves between
|
||||
* requests can come back in a later page.
|
||||
*/
|
||||
export async function walkOffsetPages<T extends { id: string }>(
|
||||
fetchPage: (limit: number, offset: number) => Promise<T[]>,
|
||||
pageSize = LIST_PAGE_SIZE,
|
||||
maxPages = MAX_LIST_PAGES
|
||||
): Promise<T[]> {
|
||||
const out: T[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (let page = 0; page < maxPages; page++) {
|
||||
const rows = await fetchPage(pageSize, page * pageSize);
|
||||
for (const row of rows) {
|
||||
if (seen.has(row.id)) continue;
|
||||
seen.add(row.id);
|
||||
out.push(row);
|
||||
}
|
||||
if (rows.length < pageSize) return out;
|
||||
}
|
||||
throw new Error(`list did not finish within ${maxPages} pages`);
|
||||
}
|
||||
@@ -0,0 +1,43 @@
|
||||
import type { SharePayload } from '$lib/types';
|
||||
|
||||
/**
|
||||
* The most pages one shared-collection load follows (BUG-3792). At the
|
||||
* server's 1000-item page that is 50,000 items; past it the load fails rather
|
||||
* than rendering a collection with items silently missing.
|
||||
*/
|
||||
export const MAX_SHARE_PAGES = 50;
|
||||
|
||||
/** A shared collection's page: SharePayload plus its place in the list. */
|
||||
export type SharePage = SharePayload & { has_more?: boolean; next_offset?: number };
|
||||
|
||||
/**
|
||||
* Load every page of a shared collection and merge them (BUG-3792). The
|
||||
* server bounds each page and says `has_more` with the `next_offset` to ask
|
||||
* for; items arrive in the owner's order, so pages are appended. Attachment
|
||||
* refs are minted per page and merged. A single-item share, a refusal
|
||||
* (require_auth / require_password) or an older server without `has_more` is
|
||||
* one page.
|
||||
*/
|
||||
export async function walkSharePages(
|
||||
fetchPage: (offset: number) => Promise<SharePage>,
|
||||
maxPages = MAX_SHARE_PAGES
|
||||
): Promise<SharePayload> {
|
||||
const first = await fetchPage(0);
|
||||
if (first.type !== 'collection' || !first.has_more) return first;
|
||||
const items = [...(first.items ?? [])];
|
||||
const refs = { ...(first.attachment_refs ?? {}) };
|
||||
let page = first;
|
||||
for (let n = 1; page.has_more; n++) {
|
||||
const next = page.next_offset;
|
||||
if (n >= maxPages || typeof next !== 'number' || next <= 0) {
|
||||
throw new Error(`shared collection did not finish within ${maxPages} pages`);
|
||||
}
|
||||
page = await fetchPage(next);
|
||||
items.push(...(page.items ?? []));
|
||||
Object.assign(refs, page.attachment_refs ?? {});
|
||||
}
|
||||
const merged: SharePage = { ...first, items, has_more: false };
|
||||
delete merged.next_offset;
|
||||
if (Object.keys(refs).length > 0) merged.attachment_refs = refs;
|
||||
return merged;
|
||||
}
|
||||
@@ -2,7 +2,7 @@
|
||||
import { safeText } from '$lib/fields/fieldShape';
|
||||
import { page } from '$app/state';
|
||||
import { onMount } from 'svelte';
|
||||
import { api, withRequestDeadline } from '$lib/api/client';
|
||||
import { api, PadApiError } from '$lib/api/client';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import { renderMarkedWithAttachments, sanitizeMarkdownHtml } from '$lib/utils/markdown';
|
||||
import { renderAttachmentUnavailable } from '$lib/markdown/attachments';
|
||||
@@ -501,21 +501,18 @@
|
||||
passwordError = '';
|
||||
passwordLoading = true;
|
||||
try {
|
||||
// Under the API deadline (BUG-3216): `passwordLoading` is held across it.
|
||||
const { resp, data } = await withRequestDeadline(
|
||||
async (signal) => {
|
||||
const resp = await fetch(`/api/v1/s/${token}`, {
|
||||
credentials: 'same-origin',
|
||||
headers: { 'X-Share-Password': passwordInput },
|
||||
signal
|
||||
});
|
||||
return { resp, data: await resp.json() };
|
||||
},
|
||||
{ idempotent: true }
|
||||
);
|
||||
if (!resp.ok) {
|
||||
passwordError = data?.error?.message ?? 'Incorrect password';
|
||||
return;
|
||||
// Through the share loader, so a collection past one page arrives
|
||||
// whole (BUG-3792, Codex r1); each page is under the API deadline
|
||||
// (BUG-3216), and `passwordLoading` is held across them.
|
||||
let data: SharePayload;
|
||||
try {
|
||||
data = await api.share.get(token, passwordInput);
|
||||
} catch (e: any) {
|
||||
if (e instanceof PadApiError) {
|
||||
passwordError = e.message || 'Incorrect password';
|
||||
return;
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
if (data.require_password) {
|
||||
passwordError = 'Incorrect password';
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
// BUG-3792 (Codex r1 Medium): a password-protected share's retry goes through
|
||||
// api.share.get, the loader that walks every page of a shared collection. It
|
||||
// used to fetch the first page by hand, so a protected collection past one page
|
||||
// rendered incomplete.
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { cleanup, fireEvent, render } from '@testing-library/svelte';
|
||||
|
||||
const share = vi.hoisted(() => ({ calls: [] as unknown[][] }));
|
||||
|
||||
vi.mock('$lib/api/client', () => {
|
||||
class PadApiError extends Error {}
|
||||
return {
|
||||
PadApiError,
|
||||
withRequestDeadline: vi.fn(),
|
||||
api: {
|
||||
share: {
|
||||
get: vi.fn(async (...args: unknown[]) => {
|
||||
share.calls.push(args);
|
||||
if (args[1] === undefined) return { require_password: true };
|
||||
return { type: 'collection', collection: { name: 'Protected' }, items: [{ ref: 'TASK-1', title: 'One' }] };
|
||||
})
|
||||
}
|
||||
}
|
||||
};
|
||||
});
|
||||
|
||||
import { page } from '$app/state';
|
||||
import SharePage from './+page.svelte';
|
||||
|
||||
async function settle() {
|
||||
for (let i = 0; i < 20; i++) await Promise.resolve();
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
share.calls = [];
|
||||
});
|
||||
|
||||
describe('share page: the password retry walks the share through api.share.get (BUG-3792)', () => {
|
||||
it('submits the password to the paged loader', async () => {
|
||||
page.params = { token: 'tok' };
|
||||
const { container } = render(SharePage);
|
||||
await settle();
|
||||
const input = container.querySelector('form.password-form input') as HTMLInputElement;
|
||||
expect(input, 'the password form did not render').not.toBeNull();
|
||||
await fireEvent.input(input, { target: { value: 'pw' } });
|
||||
await fireEvent.submit(container.querySelector('form.password-form')!);
|
||||
await settle();
|
||||
expect(share.calls).toEqual([['tok'], ['tok', 'pw']]);
|
||||
expect(container.querySelector('form.password-form')).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user