mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 05:49:50 +00:00
fix: blank workspaces accept the library's own triggers and scopes (BUG-3446) (#1836)
* fix: a blank workspace's conventions and playbooks take the library's own triggers and scopes (BUG-3446) The blank template seeds Conventions with trigger [always] / scope [all] (Playbooks [manual] / [all]) and nothing widened them, so all 18 library conventions were refused on every door: CLI and MCP library activate, the web Activate button, and the onboard playbook's literal B3 create. A create in a system collection declaring the convention or playbook artifact kind now adds a missing trigger/scope value to the select's options when the LIBRARY itself uses that word (collections.LibraryOptionVocabulary, computed from the library). The write goes through UpdateCollection with its concurrency token, ahead of validation, and is reported in warnings.options_added (CLI: stderr) and in the created activity's metadata, which the item timeline renders. Any other value is refused as before. Editors may trigger it (lead ruling). The onboard playbook's B3 now says how an owner adds an invented trigger. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS * chore(mcp): ToolSurfaceVersion 0.67 for BUG-3446 (additive) Lead ruling: a create in a system conventions/playbooks collection now adds a library trigger/scope word instead of refusing it, and its result gains an omitempty warnings.options_added. No tool name, enum or param moved; tools/list golden bytes are unchanged, initialize moves for the version and the instructions note. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
@@ -390,7 +390,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.66)** — 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.67)** — 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 |
|
||||
|---|---|
|
||||
@@ -414,7 +414,7 @@ Plus resources at `pad://workspaces`, `pad://workspace/{ws}/dashboard`,
|
||||
and `pad://_meta/version`.
|
||||
|
||||
**Stability contract.** The catalog is versioned
|
||||
(`tool_surface_version: "0.66"`) and advertised in the initialize handshake and at
|
||||
(`tool_surface_version: "0.67"`) 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
|
||||
|
||||
@@ -365,6 +365,7 @@ Run with --help-collections to see available collections and their status values
|
||||
// into a parser — which is exactly the caller most likely to have
|
||||
// sent a mistyped key and least likely to notice (codex round 2).
|
||||
warnUndeclaredFields(item)
|
||||
warnOptionsAdded(item)
|
||||
warnContentPendingFlush(item)
|
||||
|
||||
if formatFlag == "json" {
|
||||
@@ -4327,6 +4328,25 @@ func warnUndeclaredFields(item *models.Item) {
|
||||
strings.Join(item.Warnings.UndeclaredFields, ", "))
|
||||
}
|
||||
|
||||
// warnOptionsAdded prints one line to STDERR when the create added library
|
||||
// trigger or scope words to its collection's options (BUG-3446), so the
|
||||
// schema change is never silent. Stderr for the same reason as above.
|
||||
func warnOptionsAdded(item *models.Item) {
|
||||
if item == nil || item.Warnings == nil || len(item.Warnings.OptionsAdded) == 0 {
|
||||
return
|
||||
}
|
||||
keys := make([]string, 0, len(item.Warnings.OptionsAdded))
|
||||
for k := range item.Warnings.OptionsAdded {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
parts := make([]string, 0, len(keys))
|
||||
for _, k := range keys {
|
||||
parts = append(parts, k+": "+strings.Join(item.Warnings.OptionsAdded[k], ", "))
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "note: added to this collection's options (library vocabulary): %s\n", strings.Join(parts, "; "))
|
||||
}
|
||||
|
||||
// warnContentStale prints one line to STDERR when a READ served a body the server
|
||||
// knows is behind the item's live collaborative document (BUG-3000).
|
||||
//
|
||||
|
||||
@@ -400,6 +400,7 @@ Examples:
|
||||
if err := verifyConventionLanded(item); err != nil {
|
||||
return err
|
||||
}
|
||||
warnOptionsAdded(item)
|
||||
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(item)
|
||||
@@ -468,6 +469,8 @@ Examples:
|
||||
return err
|
||||
}
|
||||
|
||||
warnOptionsAdded(item)
|
||||
|
||||
if formatFlag == "json" {
|
||||
return cli.PrintJSON(item)
|
||||
}
|
||||
|
||||
+2
-2
File diff suppressed because one or more lines are too long
+2
-2
File diff suppressed because one or more lines are too long
@@ -0,0 +1,60 @@
|
||||
package collections
|
||||
|
||||
import (
|
||||
"sort"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/artifact"
|
||||
)
|
||||
|
||||
// LibraryOptionVocabulary returns, per select field key, the values the
|
||||
// library writes into a collection of the given artifact kind when one of
|
||||
// its entries is activated (BUG-3446). It is computed from the library
|
||||
// itself, never from a hand list, so a new library entry brings its own
|
||||
// words along.
|
||||
//
|
||||
// The `blank` template seeds its system collections with one trigger and
|
||||
// one scope on purpose (templates_blank.go), so every triggered library
|
||||
// entry used to be refused there. A write that carries one of these values
|
||||
// may add it to the collection's options; any other value is still refused,
|
||||
// so the options grow only by words the library itself uses.
|
||||
//
|
||||
// Convention activation stores `scope = surfaces[0]`
|
||||
// (models.ApplyItemConventionMetadata), so every surface is a scope word.
|
||||
func LibraryOptionVocabulary(kind string) map[string][]string {
|
||||
words := map[string]map[string]bool{"trigger": {}, "scope": {}}
|
||||
add := func(key, v string) {
|
||||
if v != "" {
|
||||
words[key][v] = true
|
||||
}
|
||||
}
|
||||
switch kind {
|
||||
case string(artifact.KindConvention):
|
||||
for _, cat := range ConventionLibrary() {
|
||||
for _, c := range cat.Conventions {
|
||||
add("trigger", c.Trigger)
|
||||
for _, s := range c.Surfaces {
|
||||
add("scope", s)
|
||||
}
|
||||
}
|
||||
}
|
||||
case string(artifact.KindPlaybook):
|
||||
for _, cat := range PlaybookLibrary() {
|
||||
for _, p := range cat.Playbooks {
|
||||
add("trigger", p.Trigger)
|
||||
add("scope", p.Scope)
|
||||
}
|
||||
}
|
||||
default:
|
||||
return nil
|
||||
}
|
||||
out := make(map[string][]string, len(words))
|
||||
for key, set := range words {
|
||||
vals := make([]string, 0, len(set))
|
||||
for v := range set {
|
||||
vals = append(vals, v)
|
||||
}
|
||||
sort.Strings(vals)
|
||||
out[key] = vals
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -133,6 +133,8 @@ For each rewritten convention, propose it to the user. Confirm. Create via ` + "
|
||||
|
||||
If the library has nothing close to what the user needs, INVENT a convention.
|
||||
|
||||
**Triggers and scopes.** A blank workspace starts with one trigger (` + "`always`" + `) and one scope (` + "`all`" + `). Any trigger or scope the library uses is added to the collection's options automatically the first time you use it; the response notes what was added. A trigger you INVENT (e.g. ` + "`on-experiment-run`" + `) is refused until it is in the options, so add it first: read the schema (` + "`pad collection list --format json`" + `), append the word to the ` + "`trigger`" + ` field's ` + "`options`" + `, and send it back with ` + "`pad collection update conventions --schema '<json>'`" + ` (or ` + "`pad_collection`" + ` with ` + "`action: update`" + `). That is owner-only; if you are not the owner, ask the user to do it or pick a library trigger.
|
||||
|
||||
**Independent code reviewer (software projects).** One library convention is worth
|
||||
calling out specially: **"Independent AI code review."** The principle — a reviewer
|
||||
model DIFFERENT from your implementer model catches far more than self-review;
|
||||
|
||||
@@ -15,12 +15,16 @@ package collections
|
||||
// for conventions (universal "follow this" rule), `manual` for
|
||||
// playbooks (the seeded onboard playbook itself is
|
||||
// manual-triggered), and `all` for both scopes.
|
||||
// - The /pad onboard playbook broadens these via `pad collection
|
||||
// update` (TASK-1510) once the interview reveals the workspace's
|
||||
// actual domain — software adds `on-commit`/`on-implement`,
|
||||
// hiring adds `on-candidate-advance`, research adds
|
||||
// `on-experiment-run`, etc. Keeping the seed minimal forces that
|
||||
// conversation up-front rather than carrying baked-in assumptions.
|
||||
// - They grow two ways. A trigger or scope the convention/playbook
|
||||
// LIBRARY uses is added by the server the first time a write uses
|
||||
// it, and reported (BUG-3446, collections.LibraryOptionVocabulary):
|
||||
// the onboard playbook used to say nothing about widening, so every
|
||||
// triggered library convention was refused here. A word the library
|
||||
// does not use (hiring's `on-candidate-advance`, research's
|
||||
// `on-experiment-run`) is still added deliberately, by the owner,
|
||||
// via `pad collection update` (TASK-1510); the onboard playbook's B3
|
||||
// says how. Keeping the seed minimal still means only the words a
|
||||
// workspace actually uses end up listed.
|
||||
|
||||
var (
|
||||
BlankConventionTriggers = []string{"always"}
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
package mcp
|
||||
|
||||
// BUG-3446, the MCP door: an agent running the onboard playbook over remote
|
||||
// MCP in a `blank` workspace activates every library convention and playbook
|
||||
// with pad_library.activate, and B3's literal create (pad_item create with a
|
||||
// `field: ["trigger=…"]`) succeeds. Driven through the real dispatcher and the
|
||||
// real server and store, for the reason given in
|
||||
// dispatch_http_collection_alias_test.go; that file's SCOPE note applies here
|
||||
// too (no transport, no OAuth).
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/collections"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/server"
|
||||
"github.com/PerpetualSoftware/pad/internal/store/storetest"
|
||||
)
|
||||
|
||||
func TestBUG3446_MCPActivatesEveryLibraryEntryInABlankWorkspace(t *testing.T) {
|
||||
s := storetest.NewSQLite(t)
|
||||
srv := server.New(s)
|
||||
t.Cleanup(srv.Stop)
|
||||
|
||||
owner, err := s.CreateUser(models.UserCreate{Email: "blank-3446@example.com", Name: "Owner", Password: "correct-horse-battery-staple"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser: %v", err)
|
||||
}
|
||||
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Blank 3446", Slug: "blank-3446", OwnerID: owner.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
if err := s.SeedCollectionsFromTemplate(ws.ID, "blank"); err != nil {
|
||||
t.Fatalf("SeedCollectionsFromTemplate: %v", err)
|
||||
}
|
||||
d := &HTTPHandlerDispatcher{Handler: srv, UserResolver: func(context.Context) *models.User { return owner }}
|
||||
|
||||
call := func(args []string, input map[string]any) (string, bool) {
|
||||
t.Helper()
|
||||
res, err := d.Dispatch(WithDispatchInput(context.Background(), input), args, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Dispatch %v: %v", args, err)
|
||||
}
|
||||
return textOf(res), res.IsError
|
||||
}
|
||||
|
||||
// A word outside the library is still refused over MCP.
|
||||
text, isErr := call([]string{"item", "create"}, map[string]any{
|
||||
"workspace": ws.Slug, "collection": "conventions", "title": "Invented",
|
||||
"field": []any{"trigger=on-experiment-run", "status=active"},
|
||||
})
|
||||
if !isErr || !strings.Contains(text, "on-experiment-run") {
|
||||
t.Errorf("an out-of-vocabulary trigger over MCP: isError=%v %s", isErr, text)
|
||||
}
|
||||
|
||||
seeded := map[string]bool{}
|
||||
items, err := s.ListItems(ws.ID, models.ItemListParams{})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems: %v", err)
|
||||
}
|
||||
for _, it := range items {
|
||||
seeded[it.Title] = true
|
||||
}
|
||||
|
||||
for _, cat := range collections.ConventionLibrary() {
|
||||
for _, c := range cat.Conventions {
|
||||
if text, isErr := call([]string{"library", "activate"}, map[string]any{"workspace": ws.Slug, "title": c.Title}); isErr {
|
||||
t.Errorf("pad_library.activate %q (trigger %s): %s", c.Title, c.Trigger, text)
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, cat := range collections.PlaybookLibrary() {
|
||||
for _, p := range cat.Playbooks {
|
||||
if seeded[p.Title] {
|
||||
continue // the seeded onboard playbook; the library shows it as active
|
||||
}
|
||||
if text, isErr := call([]string{"library", "activate"}, map[string]any{"workspace": ws.Slug, "title": p.Title}); isErr {
|
||||
t.Errorf("pad_library.activate playbook %q (trigger %s): %s", p.Title, p.Trigger, text)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// B3's literal step over MCP. The activations above already added
|
||||
// on-task-complete, so this only has to succeed; the report on the first
|
||||
// adding write is TestBUG3446_MCPCreateReportsOptionsAdded below.
|
||||
text, isErr = call([]string{"item", "create"}, map[string]any{
|
||||
"workspace": ws.Slug, "collection": "conventions", "title": "Run make test before done",
|
||||
"field": []any{"trigger=on-task-complete", "status=active"},
|
||||
})
|
||||
if isErr {
|
||||
t.Fatalf("pad_item create with a library trigger: %s", text)
|
||||
}
|
||||
|
||||
coll, err := s.GetCollectionBySlug(ws.ID, "conventions")
|
||||
if err != nil || coll == nil {
|
||||
t.Fatalf("GetCollectionBySlug: %v", err)
|
||||
}
|
||||
var schema models.CollectionSchema
|
||||
if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
|
||||
t.Fatalf("schema: %v", err)
|
||||
}
|
||||
for _, f := range schema.Fields {
|
||||
if f.Key == "trigger" {
|
||||
for _, o := range f.Options {
|
||||
if o == "on-experiment-run" {
|
||||
t.Errorf("an out-of-vocabulary word reached the options: %v", f.Options)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The first MCP write that adds a word tells the agent so.
|
||||
func TestBUG3446_MCPCreateReportsOptionsAdded(t *testing.T) {
|
||||
s := storetest.NewSQLite(t)
|
||||
srv := server.New(s)
|
||||
t.Cleanup(srv.Stop)
|
||||
owner, err := s.CreateUser(models.UserCreate{Email: "blank-3446b@example.com", Name: "Owner", Password: "correct-horse-battery-staple"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser: %v", err)
|
||||
}
|
||||
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Blank 3446b", Slug: "blank-3446b", OwnerID: owner.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
if err := s.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
if err := s.SeedCollectionsFromTemplate(ws.ID, "blank"); err != nil {
|
||||
t.Fatalf("SeedCollectionsFromTemplate: %v", err)
|
||||
}
|
||||
d := &HTTPHandlerDispatcher{Handler: srv, UserResolver: func(context.Context) *models.User { return owner }}
|
||||
res, err := d.Dispatch(WithDispatchInput(context.Background(), map[string]any{
|
||||
"workspace": ws.Slug, "collection": "conventions", "title": "Commit often",
|
||||
"field": []any{"trigger=on-commit", "status=active"},
|
||||
}), []string{"item", "create"}, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("Dispatch: %v", err)
|
||||
}
|
||||
if res.IsError {
|
||||
t.Fatalf("create: %s", textOf(res))
|
||||
}
|
||||
if text := textOf(res); !strings.Contains(text, `"options_added"`) || !strings.Contains(text, "on-commit") {
|
||||
t.Errorf("the create result does not report the option it added: %s", text)
|
||||
}
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
+14
-1
@@ -1738,6 +1738,19 @@ const CmdhelpVersion = "0.1"
|
||||
// caller cannot read embeds nothing; a failed fetch stays non-fatal,
|
||||
// as on stdio. Stdio is unchanged.
|
||||
//
|
||||
// 0.67 — BUG-3446 (lead ruling, day 89). ADDITIVE bump on the v0.62 /
|
||||
// v0.64 grounds: in a SYSTEM conventions or playbooks collection, a
|
||||
// create whose `trigger` / `scope` is a word the convention or playbook
|
||||
// LIBRARY itself uses now ADDS that word to the select's options instead
|
||||
// of being refused, so `pad_library.activate` and `pad_item.action=create`
|
||||
// succeed in a `blank` workspace, where every triggered library
|
||||
// convention used to be refused. The create's result gains an
|
||||
// `omitempty` `warnings.options_added` (keyed by field) on both
|
||||
// transports. A word the library does not use is refused exactly as
|
||||
// before, so the options grow only by library words. Absent when nothing
|
||||
// was added, so a consumer that ignores it is unaffected. No name, enum
|
||||
// or param moved.
|
||||
//
|
||||
// 0.66 — BUG-2136 (lead ruling, day 87). BEHAVIOR bump on the v0.52 /
|
||||
// v0.43 grounds: `pad_workspace.action=invite` no longer ADDS an existing
|
||||
// account to the workspace. Every invite now creates a pending
|
||||
@@ -1792,7 +1805,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.66"
|
||||
const ToolSurfaceVersion = "0.67"
|
||||
|
||||
// MetaVersionURI is the canonical URI of the queryable version document.
|
||||
// Lives outside the pad://workspace/{ws}/... namespace because it's a
|
||||
|
||||
@@ -101,6 +101,10 @@ type ItemWriteWarnings struct {
|
||||
UndeclaredFields []string `json:"undeclared_fields,omitempty"`
|
||||
DroppedFields []string `json:"dropped_fields,omitempty"`
|
||||
UnresolvedRelations []string `json:"unresolved_relations,omitempty"`
|
||||
// OptionsAdded names the select options this write added to its
|
||||
// collection's schema (BUG-3446): a library trigger or scope a `blank`
|
||||
// workspace's system collection did not list yet. Keyed by field.
|
||||
OptionsAdded map[string][]string `json:"options_added,omitempty"`
|
||||
// ContentOutcome names where a content write ended up, in the same vocabulary
|
||||
// the content_not_applied error uses. On a 200 it takes exactly one value,
|
||||
// "applied_pending_flush": the content went to the collaborative document rather
|
||||
|
||||
@@ -0,0 +1,350 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/artifact"
|
||||
"github.com/PerpetualSoftware/pad/internal/collections"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// BUG-3446: the `blank` template seeds Conventions with trigger [always] and
|
||||
// scope [all], and Playbooks with trigger [manual] and scope [all]. Every
|
||||
// triggered library convention was refused there, through every door, so an
|
||||
// agent following the onboard playbook literally failed at B3. These tests
|
||||
// replay the playbook's B3 and B5 writes against a blank workspace, in the
|
||||
// request shapes each door sends:
|
||||
//
|
||||
// - CLI `pad library activate` / remote MCP `pad_library.activate`:
|
||||
// models.BuildConventionItemCreate + the typed `convention` member;
|
||||
// - the web Library page's Activate button: its own fields blob
|
||||
// (web/src/lib/api/client.ts `library.activate`), which also names scope;
|
||||
// - B3's literal `pad item create conventions --field trigger=<t>
|
||||
// --field status=active`.
|
||||
//
|
||||
// The MCP door is also driven through the real dispatcher in
|
||||
// internal/mcp/bug3446_blank_library_activate_test.go.
|
||||
|
||||
func bug3446BlankWorkspace(t *testing.T, srv *Server, name string) (slug, wsID string) {
|
||||
t.Helper()
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces", map[string]string{"name": name, "template": "blank"})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("create blank workspace: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var ws models.Workspace
|
||||
parseJSON(t, rr, &ws)
|
||||
return ws.Slug, ws.ID
|
||||
}
|
||||
|
||||
func bug3446Options(t *testing.T, srv *Server, wsID, collSlug, key string) []string {
|
||||
t.Helper()
|
||||
coll, err := srv.store.GetCollectionBySlug(wsID, collSlug)
|
||||
if err != nil || coll == nil {
|
||||
t.Fatalf("GetCollectionBySlug %s: %v", collSlug, err)
|
||||
}
|
||||
var schema models.CollectionSchema
|
||||
if err := json.Unmarshal([]byte(coll.Schema), &schema); err != nil {
|
||||
t.Fatalf("schema: %v", err)
|
||||
}
|
||||
for _, f := range schema.Fields {
|
||||
if f.Key == key {
|
||||
out := append([]string(nil), f.Options...)
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
}
|
||||
t.Fatalf("%s has no %q field", collSlug, key)
|
||||
return nil
|
||||
}
|
||||
|
||||
// cliConventionBody is what `pad library activate` and pad_library.activate
|
||||
// post for a library convention.
|
||||
func cliConventionBody(t *testing.T, c collections.LibraryConvention) map[string]any {
|
||||
t.Helper()
|
||||
fields, conv, err := models.BuildConventionItemCreate("active", &models.ItemConventionMetadata{
|
||||
Category: c.Category, Trigger: c.Trigger, Surfaces: c.Surfaces, Enforcement: c.Enforcement, Commands: c.Commands,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("BuildConventionItemCreate: %v", err)
|
||||
}
|
||||
return map[string]any{"title": c.Title, "content": c.Content, "fields": fields, "convention": conv}
|
||||
}
|
||||
|
||||
// webConventionBody mirrors web/src/lib/api/client.ts `library.activate`.
|
||||
func webConventionBody(c collections.LibraryConvention) map[string]any {
|
||||
scope := "all"
|
||||
if len(c.Surfaces) > 0 {
|
||||
scope = c.Surfaces[0]
|
||||
}
|
||||
commands := c.Commands
|
||||
if commands == nil {
|
||||
commands = []string{}
|
||||
}
|
||||
fields, _ := json.Marshal(map[string]any{
|
||||
"status": "active", "category": c.Category, "trigger": c.Trigger, "scope": scope,
|
||||
"priority": c.Enforcement, "enforcement": c.Enforcement, "surfaces": c.Surfaces, "commands": commands,
|
||||
})
|
||||
return map[string]any{
|
||||
"title": c.Title, "content": c.Content, "fields": string(fields),
|
||||
"convention": map[string]any{
|
||||
"category": c.Category, "trigger": c.Trigger, "surfaces": c.Surfaces,
|
||||
"enforcement": c.Enforcement, "commands": commands,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func playbookBody(p collections.LibraryPlaybook) map[string]any {
|
||||
f := map[string]any{"status": "active", "trigger": p.Trigger, "scope": p.Scope}
|
||||
if p.InvocationSlug != "" {
|
||||
f["invocation_slug"] = p.InvocationSlug
|
||||
}
|
||||
if len(p.Arguments) > 0 {
|
||||
f["arguments"] = p.Arguments
|
||||
}
|
||||
fields, _ := json.Marshal(f)
|
||||
return map[string]any{"title": p.Title, "content": p.Content, "fields": string(fields)}
|
||||
}
|
||||
|
||||
func libraryConventions() []collections.LibraryConvention {
|
||||
var out []collections.LibraryConvention
|
||||
for _, cat := range collections.ConventionLibrary() {
|
||||
out = append(out, cat.Conventions...)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// usedWords is the set of values for key the given writes carry, plus what
|
||||
// the blank template seeded: the options must end EXACTLY there.
|
||||
func usedWords(seed []string, vals []string) []string {
|
||||
set := map[string]bool{}
|
||||
for _, v := range append(append([]string(nil), seed...), vals...) {
|
||||
set[v] = true
|
||||
}
|
||||
out := make([]string, 0, len(set))
|
||||
for v := range set {
|
||||
out = append(out, v)
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// Every library convention and playbook activates in a blank workspace, in
|
||||
// the CLI/MCP shape and in the web shape, and the options end at exactly the
|
||||
// words those activations used.
|
||||
func TestBUG3446_EveryLibraryEntryActivatesInABlankWorkspace(t *testing.T) {
|
||||
bothBackends(t, func(t *testing.T, srv *Server) {
|
||||
for _, door := range []struct {
|
||||
name string
|
||||
body func(*testing.T, collections.LibraryConvention) map[string]any
|
||||
}{
|
||||
{"cli-mcp", cliConventionBody},
|
||||
{"web", func(_ *testing.T, c collections.LibraryConvention) map[string]any { return webConventionBody(c) }},
|
||||
} {
|
||||
t.Run(door.name, func(t *testing.T) {
|
||||
slug, wsID := bug3446BlankWorkspace(t, srv, "Blank 3446 "+door.name)
|
||||
var triggers, scopes []string
|
||||
for _, c := range libraryConventions() {
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/conventions/items", door.body(t, c))
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Errorf("activate convention %q (trigger %s): %d %s", c.Title, c.Trigger, rr.Code, rr.Body.String())
|
||||
continue
|
||||
}
|
||||
triggers = append(triggers, c.Trigger)
|
||||
if len(c.Surfaces) > 0 {
|
||||
scopes = append(scopes, c.Surfaces[0])
|
||||
}
|
||||
}
|
||||
if got, want := bug3446Options(t, srv, wsID, "conventions", "trigger"), usedWords(collections.BlankConventionTriggers, triggers); strings.Join(got, ",") != strings.Join(want, ",") {
|
||||
t.Errorf("conventions trigger options = %v, want exactly the seed plus the words used %v", got, want)
|
||||
}
|
||||
if got, want := bug3446Options(t, srv, wsID, "conventions", "scope"), usedWords(collections.BlankConventionScopes, scopes); strings.Join(got, ",") != strings.Join(want, ",") {
|
||||
t.Errorf("conventions scope options = %v, want exactly the seed plus the words used %v", got, want)
|
||||
}
|
||||
|
||||
// B5. A library playbook whose title the workspace already
|
||||
// holds (the seeded onboard playbook) is what the UI shows as
|
||||
// "Active"; it is not activated twice.
|
||||
existing := map[string]bool{}
|
||||
rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/playbooks/items", nil)
|
||||
var seeded []models.Item
|
||||
parseJSON(t, rr, &seeded)
|
||||
for _, it := range seeded {
|
||||
existing[it.Title] = true
|
||||
}
|
||||
for _, cat := range collections.PlaybookLibrary() {
|
||||
for _, p := range cat.Playbooks {
|
||||
if existing[p.Title] {
|
||||
continue
|
||||
}
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/playbooks/items", playbookBody(p))
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Errorf("activate playbook %q (trigger %s): %d %s", p.Title, p.Trigger, rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// B3's literal step, the report, and the bound: the first write that needs a
|
||||
// word adds it and says so in its warnings and its activity; the next write
|
||||
// with the same word adds nothing; a word the library does not use is refused
|
||||
// as before and adds nothing.
|
||||
func TestBUG3446_LiteralItemCreateWidensOnlyByLibraryWordsAndRecordsIt(t *testing.T) {
|
||||
bothBackends(t, func(t *testing.T, srv *Server) {
|
||||
slug, wsID := bug3446BlankWorkspace(t, srv, "Blank 3446 literal")
|
||||
path := "/api/v1/workspaces/" + slug + "/collections/conventions/items"
|
||||
create := func(title, trigger string) (*models.Item, int, string) {
|
||||
fields, _ := json.Marshal(map[string]string{"trigger": trigger, "status": "active"})
|
||||
rr := doRequest(srv, "POST", path, map[string]any{"title": title, "content": "x", "fields": string(fields)})
|
||||
if rr.Code != http.StatusCreated {
|
||||
return nil, rr.Code, rr.Body.String()
|
||||
}
|
||||
var it models.Item
|
||||
parseJSON(t, rr, &it)
|
||||
return &it, rr.Code, ""
|
||||
}
|
||||
|
||||
first, code, body := create("Run make test before done", "on-task-complete")
|
||||
if first == nil {
|
||||
t.Fatalf("B3 literal create with a library trigger: %d %s", code, body)
|
||||
}
|
||||
if first.Warnings == nil || strings.Join(first.Warnings.OptionsAdded["trigger"], ",") != "on-task-complete" {
|
||||
t.Errorf("first create: warnings.options_added = %+v, want trigger: [on-task-complete]", first.Warnings)
|
||||
}
|
||||
acts, err := srv.store.ListDocumentActivity(first.ID, models.ActivityListParams{})
|
||||
if err != nil {
|
||||
t.Fatalf("ListDocumentActivity: %v", err)
|
||||
}
|
||||
recorded := false
|
||||
for _, a := range acts {
|
||||
// Decoded, not substring-matched: Postgres stores metadata as
|
||||
// JSONB and re-serialises it with its own spacing.
|
||||
var meta map[string]string
|
||||
if a.Action != "created" || json.Unmarshal([]byte(a.Metadata), &meta) != nil {
|
||||
continue
|
||||
}
|
||||
if meta["options_added"] == "trigger: on-task-complete" && meta["options_added_collection"] == "Conventions" {
|
||||
recorded = true
|
||||
}
|
||||
}
|
||||
if !recorded {
|
||||
t.Errorf("the created activity does not record the options added: %+v", acts)
|
||||
}
|
||||
|
||||
second, code, body := create("Summarize on done", "on-task-complete")
|
||||
if second == nil {
|
||||
t.Fatalf("second create: %d %s", code, body)
|
||||
}
|
||||
if second.Warnings != nil && len(second.Warnings.OptionsAdded) > 0 {
|
||||
t.Errorf("second create with an already-listed word reported options_added %+v", second.Warnings.OptionsAdded)
|
||||
}
|
||||
|
||||
if it, code, body := create("Invented rule", "on-experiment-run"); it != nil || code != http.StatusBadRequest || !strings.Contains(body, "on-experiment-run") {
|
||||
t.Errorf("a trigger outside the library vocabulary: got %d %s, want 400 naming it", code, body)
|
||||
}
|
||||
if got := bug3446Options(t, srv, wsID, "conventions", "trigger"); strings.Join(got, ",") != "always,on-task-complete" {
|
||||
t.Errorf("trigger options = %v, want [always on-task-complete]", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Ruling (a): an EDITOR's create may widen, bounded the same way.
|
||||
func TestBUG3446_EditorCreateWidens(t *testing.T) {
|
||||
bothBackends(t, func(t *testing.T, srv *Server) {
|
||||
slug, wsID := bug3446BlankWorkspace(t, srv, "Blank 3446 editor")
|
||||
owner, err := srv.store.CreateUser(models.UserCreate{Email: "owner-3446@example.com", Name: "Owner", Username: "owner-3446", Password: "pw-test-12345"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser owner: %v", err)
|
||||
}
|
||||
editor, err := srv.store.CreateUser(models.UserCreate{Email: "editor-3446@example.com", Name: "Editor", Username: "editor-3446", Password: "pw-test-12345"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser editor: %v", err)
|
||||
}
|
||||
if err := srv.store.AddWorkspaceMember(wsID, owner.ID, "owner"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember owner: %v", err)
|
||||
}
|
||||
if err := srv.store.AddWorkspaceMember(wsID, editor.ID, "editor"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember editor: %v", err)
|
||||
}
|
||||
token, err := srv.store.CreateSession(editor.ID, "go-test", "192.0.2.1", "go-test", 24*time.Hour)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateSession: %v", err)
|
||||
}
|
||||
c := collections.GetLibraryConvention("Conventional commit format")
|
||||
if c == nil {
|
||||
t.Fatal("library convention missing")
|
||||
}
|
||||
rr := doRequestWithCookie(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/conventions/items", cliConventionBody(t, *c), token)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("editor activate: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := bug3446Options(t, srv, wsID, "conventions", "trigger"); strings.Join(got, ",") != "always,"+c.Trigger {
|
||||
t.Errorf("trigger options after the editor's activate = %v", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// The artifact import door keeps its documented clear-and-warn for a select
|
||||
// value the destination does not list; it never widens (codex round 1).
|
||||
func TestBUG3446_ArtifactImportDoesNotWiden(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
slug, wsID := bug3446BlankWorkspace(t, srv, "Blank 3446 import")
|
||||
data, err := artifact.Encode(artifact.Artifact{
|
||||
Kind: artifact.KindConvention,
|
||||
FormatVersion: artifact.FormatVersion,
|
||||
Title: "Imported commit rule",
|
||||
Fields: map[string]any{"status": "active", "trigger": "on-commit", "scope": "all"},
|
||||
Body: "body\n",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("encode: %v", err)
|
||||
}
|
||||
rr := doArtifactRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/import-artifact", data)
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("import: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if !strings.Contains(rr.Body.String(), `cleared on import`) {
|
||||
t.Errorf("import did not report clearing the unlisted trigger: %s", rr.Body.String())
|
||||
}
|
||||
if got := bug3446Options(t, srv, wsID, "conventions", "trigger"); strings.Join(got, ",") != "always" {
|
||||
t.Errorf("an import widened the trigger options: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The schema rewrite touches the FIRST definition of a key, the one
|
||||
// validation reads, and never duplicates a value (codex round 1).
|
||||
func TestBUG3446_AppendSelectOptionsJSON(t *testing.T) {
|
||||
raw := `{"fields":[{"key":"trigger","type":"select","options":["always"],"x_extra":1},{"key":"trigger","type":"select","options":["always","on-commit"]}]}`
|
||||
out, err := appendSelectOptionsJSON(raw, map[string][]string{"trigger": {"on-commit", "always"}})
|
||||
if err != nil {
|
||||
t.Fatalf("appendSelectOptionsJSON: %v", err)
|
||||
}
|
||||
want := `{"fields":[{"key":"trigger","options":["always","on-commit"],"type":"select","x_extra":1},{"key":"trigger","options":["always","on-commit"],"type":"select"}]}`
|
||||
if out != want {
|
||||
t.Errorf("got %s\nwant %s", out, want)
|
||||
}
|
||||
}
|
||||
|
||||
// A create that the widened schema would still refuse writes nothing to the
|
||||
// schema: it is validated against the widened schema before the widening is
|
||||
// stored (codex round 2). Here the trigger is a library word but the status
|
||||
// is not a valid option.
|
||||
func TestBUG3446_RefusedCreateDoesNotWiden(t *testing.T) {
|
||||
srv := testServer(t)
|
||||
slug, wsID := bug3446BlankWorkspace(t, srv, "Blank 3446 refused")
|
||||
fields, _ := json.Marshal(map[string]string{"trigger": "on-commit", "status": "not-a-status"})
|
||||
rr := doRequest(srv, "POST", "/api/v1/workspaces/"+slug+"/collections/conventions/items", map[string]any{"title": "Refused", "fields": string(fields)})
|
||||
if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "not-a-status") {
|
||||
t.Fatalf("create with a bad status: got %d %s, want 400 naming it", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := bug3446Options(t, srv, wsID, "conventions", "trigger"); strings.Join(got, ",") != "always" {
|
||||
t.Errorf("a refused create widened the trigger options: %v", got)
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"maps"
|
||||
"net/http"
|
||||
"sort"
|
||||
"strconv"
|
||||
@@ -894,6 +895,27 @@ const (
|
||||
)
|
||||
|
||||
func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *models.Collection, schema models.CollectionSchema, input models.ItemCreate, fieldMap map[string]any, parentValue string, posture relationPosture) (*models.Item, *itemCreateError) {
|
||||
// BUG-3446: a library trigger/scope a blank workspace's system collection
|
||||
// does not list yet is added before validation, and reported below. An
|
||||
// artifact import (relationsCarry) keeps its own clear-and-warn handling
|
||||
// of foreign select values, which runs before this and leaves nothing
|
||||
// unlisted; the posture check makes that a rule rather than a sequence.
|
||||
var optionsAdded map[string][]string
|
||||
if posture == relationsRefuse {
|
||||
if wanted := libraryOptionsWanted(coll, schema, fieldMap); len(wanted) > 0 {
|
||||
// Validate against the widened schema first, on a copy (the
|
||||
// pipeline injects defaults into its map), so a create that would
|
||||
// be refused anyway never changes the collection (codex round 2).
|
||||
if _, _, _, _, cerr := s.prepareCreateFields(r, workspaceID, coll, withSelectOptions(schema, wanted), maps.Clone(fieldMap), posture); cerr != nil {
|
||||
return nil, cerr
|
||||
}
|
||||
var werr *itemCreateError
|
||||
coll, schema, optionsAdded, werr = s.widenLibraryOptions(r, workspaceID, coll, schema, fieldMap)
|
||||
if werr != nil {
|
||||
return nil, werr
|
||||
}
|
||||
}
|
||||
}
|
||||
fieldMap, droppedDefaults, unresolved, undeclared, cerr := s.prepareCreateFields(r, workspaceID, coll, schema, fieldMap, posture)
|
||||
if cerr != nil {
|
||||
return nil, cerr
|
||||
@@ -992,7 +1014,17 @@ func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *mo
|
||||
|
||||
actor, source := actorFromRequest(r)
|
||||
actorNameForCreate := actorNameFromRequest(r)
|
||||
s.logActivity(workspaceID, item.ID, "created", r)
|
||||
if len(optionsAdded) > 0 {
|
||||
// BUG-3446: the record of the schema change, where the owner reads
|
||||
// it — the item's own timeline and the workspace activity feed.
|
||||
meta, _ := json.Marshal(map[string]string{
|
||||
"options_added": formatOptionsAdded(optionsAdded),
|
||||
"options_added_collection": coll.Name,
|
||||
})
|
||||
s.logActivityWithMeta(workspaceID, item.ID, "created", r, string(meta))
|
||||
} else {
|
||||
s.logActivity(workspaceID, item.ID, "created", r)
|
||||
}
|
||||
s.publishItemEventWithName(sseItemCreated, workspaceID, item.ID, item.Title, coll.Slug, actor, actorNameForCreate, source, item.Seq)
|
||||
|
||||
// Assignment-at-creation (TASK-2533): unlike an update, a freshly
|
||||
@@ -1019,11 +1051,12 @@ func (s *Server) createItemChecked(r *http.Request, workspaceID string, coll *mo
|
||||
|
||||
// Attach after the write succeeded: these are advisory notes about a
|
||||
// stored item, not a reason to refuse one (BUG-2850).
|
||||
if len(undeclared) > 0 || len(droppedDefaults) > 0 || len(unresolved) > 0 {
|
||||
if len(undeclared) > 0 || len(droppedDefaults) > 0 || len(unresolved) > 0 || len(optionsAdded) > 0 {
|
||||
item.Warnings = &models.ItemWriteWarnings{
|
||||
UndeclaredFields: undeclared,
|
||||
DroppedFields: droppedDefaults,
|
||||
UnresolvedRelations: unresolved,
|
||||
OptionsAdded: optionsAdded,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,233 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/collections"
|
||||
"github.com/PerpetualSoftware/pad/internal/events"
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// libraryWidenAttempts bounds the optimistic retry when another writer moves
|
||||
// the collection between our read and our write.
|
||||
const libraryWidenAttempts = 3
|
||||
|
||||
// widenLibraryOptions adds to a system convention or playbook collection's
|
||||
// select options any value this create carries that the LIBRARY itself uses
|
||||
// for that field and the options do not yet list (BUG-3446). The `blank`
|
||||
// template seeds those collections with one trigger and one scope on purpose,
|
||||
// so before this every triggered library entry was refused there — through
|
||||
// `pad library activate`, `pad_library.activate`, the web Activate button and
|
||||
// the onboard playbook's own `pad item create conventions` step alike.
|
||||
//
|
||||
// Bounded on purpose: a value outside collections.LibraryOptionVocabulary is
|
||||
// left for validation to refuse as before, so the options never become a
|
||||
// free-for-all. An editor may trigger it (lead ruling, BUG-3446): the words
|
||||
// are ones every non-blank template already ships. What was added is returned
|
||||
// so the create can report it, in its write warnings and in its activity.
|
||||
// Not on the artifact import door (relationsCarry): an import clears a
|
||||
// select value the destination does not list and warns
|
||||
// (preprocessArtifactFields), and keeps doing so; createItemChecked calls
|
||||
// this only for an ordinary create.
|
||||
//
|
||||
// The caller validates the create against withSelectOptions(schema, wanted)
|
||||
// BEFORE calling this, so a create the widened schema would still refuse
|
||||
// writes nothing (codex round 2). The schema write goes through
|
||||
// Store.UpdateCollection with the collection's own concurrency token, so it
|
||||
// takes the same locks as any schema edit. It is its own transaction, ahead
|
||||
// of the item insert, so a refusal after it (the plan cap at insert, a title
|
||||
// the store rejects, a race lost to a unique value, or another writer's
|
||||
// schema edit landing between the dry run and this write, codex round 3)
|
||||
// leaves the library word listed and recorded only in the log; it is a word
|
||||
// the library uses, and nothing else depends on it. The store re-validates the item under its
|
||||
// lock if the schema moved again in between (BUG-3407).
|
||||
func (s *Server) widenLibraryOptions(r *http.Request, workspaceID string, coll *models.Collection, schema models.CollectionSchema, fieldMap map[string]any) (*models.Collection, models.CollectionSchema, map[string][]string, *itemCreateError) {
|
||||
vocab := libraryVocabularyFor(coll)
|
||||
if len(vocab) == 0 || len(missingLibraryOptions(schema, vocab, fieldMap)) == 0 {
|
||||
return coll, schema, nil, nil
|
||||
}
|
||||
|
||||
for attempt := 0; attempt < libraryWidenAttempts; attempt++ {
|
||||
fresh, err := s.store.GetCollection(coll.ID)
|
||||
if err != nil || fresh == nil {
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusInternalServerError, code: "internal_error", message: "Failed to read collection"}
|
||||
}
|
||||
var freshSchema models.CollectionSchema
|
||||
if err := models.UnmarshalItemFieldSchema([]byte(fresh.Schema), &freshSchema); err != nil {
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusInternalServerError, code: "internal_error", message: "Failed to parse collection schema"}
|
||||
}
|
||||
missing := missingLibraryOptions(freshSchema, vocab, fieldMap)
|
||||
if len(missing) == 0 {
|
||||
// Another writer added them first.
|
||||
return fresh, freshSchema, nil, nil
|
||||
}
|
||||
widened, err := appendSelectOptionsJSON(fresh.Schema, missing)
|
||||
if err != nil {
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusInternalServerError, code: "internal_error", message: "Failed to extend collection schema"}
|
||||
}
|
||||
updated, err := s.store.UpdateCollection(fresh.ID, models.CollectionUpdate{
|
||||
Schema: &widened,
|
||||
ExpectedUpdatedAt: fresh.UpdatedAt.UTC().Format(time.RFC3339Nano),
|
||||
})
|
||||
if _, conflict := asCollectionUpdateConflictError(err); conflict {
|
||||
continue
|
||||
}
|
||||
if err != nil || updated == nil {
|
||||
slog.Error("widenLibraryOptions: UpdateCollection failed", "collection", fresh.ID, "error", err)
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusInternalServerError, code: "internal_error", message: "Failed to extend collection schema"}
|
||||
}
|
||||
var updatedSchema models.CollectionSchema
|
||||
if err := models.UnmarshalItemFieldSchema([]byte(updated.Schema), &updatedSchema); err != nil {
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusInternalServerError, code: "internal_error", message: "Failed to parse collection schema"}
|
||||
}
|
||||
s.publishCollectionEvent(events.CollectionUpdated, workspaceID, updated.ID, updated.Slug, "", false)
|
||||
slog.Info("widened library options", "workspace", workspaceID, "collection", updated.Slug, "added", missing, "actor", actorNameFromRequest(r))
|
||||
return updated, updatedSchema, missing, nil
|
||||
}
|
||||
return coll, schema, nil, &itemCreateError{status: http.StatusConflict, code: "conflict", message: fmt.Sprintf("The %s collection changed while its options were being extended; retry.", coll.Name)}
|
||||
}
|
||||
|
||||
// libraryVocabularyFor is the library vocabulary for a SYSTEM collection
|
||||
// declaring the convention or playbook artifact kind, nil for any other.
|
||||
func libraryVocabularyFor(coll *models.Collection) map[string][]string {
|
||||
if coll == nil || !coll.IsSystem {
|
||||
return nil
|
||||
}
|
||||
traits, err := models.ParseCollectionTraits(coll.Traits)
|
||||
if err != nil || traits.ArtifactKind == nil {
|
||||
return nil
|
||||
}
|
||||
return collections.LibraryOptionVocabulary(traits.ArtifactKind.Kind)
|
||||
}
|
||||
|
||||
// libraryOptionsWanted is what this create would add, read without writing.
|
||||
func libraryOptionsWanted(coll *models.Collection, schema models.CollectionSchema, fieldMap map[string]any) map[string][]string {
|
||||
vocab := libraryVocabularyFor(coll)
|
||||
if len(vocab) == 0 {
|
||||
return nil
|
||||
}
|
||||
return missingLibraryOptions(schema, vocab, fieldMap)
|
||||
}
|
||||
|
||||
// withSelectOptions returns a copy of schema with add appended to the first
|
||||
// definition of each key, the in-memory twin of appendSelectOptionsJSON.
|
||||
func withSelectOptions(schema models.CollectionSchema, add map[string][]string) models.CollectionSchema {
|
||||
out := models.CollectionSchema{Fields: append([]models.FieldDef(nil), schema.Fields...)}
|
||||
done := map[string]bool{}
|
||||
for i := range out.Fields {
|
||||
f := &out.Fields[i]
|
||||
if done[f.Key] || len(add[f.Key]) == 0 {
|
||||
continue
|
||||
}
|
||||
done[f.Key] = true
|
||||
opts := append([]string(nil), f.Options...)
|
||||
for _, v := range add[f.Key] {
|
||||
if !hasOption(opts, v) {
|
||||
opts = append(opts, v)
|
||||
}
|
||||
}
|
||||
f.Options = opts
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// missingLibraryOptions returns, per select field, the values fieldMap
|
||||
// carries that the library vocabulary allows and the field's options lack.
|
||||
func missingLibraryOptions(schema models.CollectionSchema, vocab map[string][]string, fieldMap map[string]any) map[string][]string {
|
||||
out := map[string][]string{}
|
||||
seen := map[string]bool{}
|
||||
for _, f := range schema.Fields {
|
||||
if seen[f.Key] {
|
||||
continue
|
||||
}
|
||||
seen[f.Key] = true
|
||||
if f.Type != "select" {
|
||||
continue
|
||||
}
|
||||
words, ok := vocab[f.Key]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
v, ok := fieldMap[f.Key].(string)
|
||||
if !ok || v == "" || hasOption(f.Options, v) || !hasOption(words, v) {
|
||||
continue
|
||||
}
|
||||
out[f.Key] = append(out[f.Key], v)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// appendSelectOptionsJSON appends values to the named fields' options in the
|
||||
// stored schema JSON, decoded generically so keys models.FieldDef does not
|
||||
// declare survive the rewrite.
|
||||
func appendSelectOptionsJSON(raw string, add map[string][]string) (string, error) {
|
||||
var doc map[string]any
|
||||
if err := models.DecodeJSONKeepingNumbers([]byte(raw), &doc); err != nil {
|
||||
return "", err
|
||||
}
|
||||
fields, ok := doc["fields"].([]any)
|
||||
if !ok {
|
||||
return "", fmt.Errorf("schema has no fields array")
|
||||
}
|
||||
done := map[string]bool{}
|
||||
for _, f := range fields {
|
||||
fm, ok := f.(map[string]any)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
key, _ := fm["key"].(string)
|
||||
vals := add[key]
|
||||
if len(vals) == 0 || done[key] {
|
||||
continue
|
||||
}
|
||||
// The first definition of a key is the one validation reads; a
|
||||
// duplicate definition is left alone rather than given a copy.
|
||||
done[key] = true
|
||||
opts, _ := fm["options"].([]any)
|
||||
for _, v := range vals {
|
||||
present := false
|
||||
for _, o := range opts {
|
||||
if o == v {
|
||||
present = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !present {
|
||||
opts = append(opts, v)
|
||||
}
|
||||
}
|
||||
fm["options"] = opts
|
||||
}
|
||||
out, err := json.Marshal(doc)
|
||||
return string(out), err
|
||||
}
|
||||
|
||||
// formatOptionsAdded renders the additions for an activity row, keys sorted:
|
||||
// "scope: backend; trigger: on-commit".
|
||||
func formatOptionsAdded(added map[string][]string) string {
|
||||
keys := make([]string, 0, len(added))
|
||||
for k := range added {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
parts := make([]string, 0, len(keys))
|
||||
for _, k := range keys {
|
||||
parts = append(parts, k+": "+strings.Join(added[k], ", "))
|
||||
}
|
||||
return strings.Join(parts, "; ")
|
||||
}
|
||||
|
||||
func hasOption(list []string, v string) bool {
|
||||
for _, x := range list {
|
||||
if x == v {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -122,6 +122,17 @@
|
||||
field unique: the server writes one sentence per drop, naming the value
|
||||
and, when the actor may see it, the item that holds it.
|
||||
-->
|
||||
<!--
|
||||
BUG-3446. A create can add a library trigger or scope to its collection's
|
||||
options (a blank workspace seeds one of each). This row is the record the
|
||||
owner reads: which words were added, to which collection, by this write.
|
||||
-->
|
||||
{#if activity.action === 'created' && metadata.options_added}
|
||||
<div class="dropped">
|
||||
<span class="dropped-label">Added to {metadata.options_added_collection || 'collection'} options:</span>
|
||||
<span class="dropped-keys">{metadata.options_added}</span>
|
||||
</div>
|
||||
{/if}
|
||||
{#if activity.action === 'moved' && metadata.not_unique}
|
||||
<div class="dropped">
|
||||
<span class="dropped-label">Not unique:</span>
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { render } from '@testing-library/svelte';
|
||||
import TimelineActivityCard from './TimelineActivityCard.svelte';
|
||||
import type { Activity } from '$lib/types';
|
||||
|
||||
// BUG-3446. A create in a blank workspace's Conventions or Playbooks
|
||||
// collection can add a library trigger or scope to the collection's options.
|
||||
// The created activity records which words and where; this row is how the
|
||||
// owner sees the options grew and why. Negative legs: a row keyed on the
|
||||
// metadata alone, or rendered on every action, would pass a presence test.
|
||||
|
||||
function activity(overrides: Partial<Activity> = {}): Activity {
|
||||
return {
|
||||
id: 'act-1',
|
||||
workspace_id: 'ws-1',
|
||||
document_id: 'item-1',
|
||||
action: 'created',
|
||||
actor: 'agent',
|
||||
actor_name: 'Dave',
|
||||
source: 'cli',
|
||||
metadata: JSON.stringify({
|
||||
options_added: 'scope: backend; trigger: on-implement',
|
||||
options_added_collection: 'Conventions'
|
||||
}),
|
||||
created_at: new Date('2026-10-06T12:00:00Z').toISOString(),
|
||||
...overrides
|
||||
} as Activity;
|
||||
}
|
||||
|
||||
describe('TimelineActivityCard options-added row', () => {
|
||||
it('names the collection and the exact words a create added', () => {
|
||||
const { getByText } = render(TimelineActivityCard, { activity: activity() });
|
||||
expect(getByText('Added to Conventions options:')).toBeTruthy();
|
||||
expect(getByText('scope: backend; trigger: on-implement')).toBeTruthy();
|
||||
});
|
||||
|
||||
it('renders nothing on a create that added nothing', () => {
|
||||
const { queryByText } = render(TimelineActivityCard, { activity: activity({ metadata: '' }) });
|
||||
expect(queryByText(/options:/)).toBeNull();
|
||||
});
|
||||
|
||||
it('renders nothing for another action carrying the same key', () => {
|
||||
const { queryByText } = render(TimelineActivityCard, {
|
||||
activity: activity({ action: 'updated' })
|
||||
});
|
||||
expect(queryByText(/options:/)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,60 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { api } from '$lib/api/client';
|
||||
import { conventionCreatePayload } from './createPayload';
|
||||
|
||||
// BUG-3446: a blank workspace's Conventions collection seeds trigger [always]
|
||||
// and scope [all], and the server now adds a library word a write needs. The
|
||||
// Go test that proves the web Activate path
|
||||
// (internal/server/bug3446_blank_library_options_test.go, webConventionBody)
|
||||
// replays the body these two web writers send. This pins that body, so the
|
||||
// Go mirror cannot drift from what the browser actually posts without one of
|
||||
// the two going red. The entry is the library's one multi-surface convention:
|
||||
// its scope is surfaces[0], a word blank does not list either.
|
||||
|
||||
const entry = {
|
||||
title: 'Update docs on API changes',
|
||||
content: 'body',
|
||||
category: 'docs',
|
||||
trigger: 'on-implement',
|
||||
surfaces: ['backend', 'docs'],
|
||||
enforcement: 'should' as const,
|
||||
commands: [] as string[]
|
||||
};
|
||||
|
||||
const expectedFields = {
|
||||
status: 'active',
|
||||
category: 'docs',
|
||||
trigger: 'on-implement',
|
||||
scope: 'backend',
|
||||
priority: 'should',
|
||||
enforcement: 'should',
|
||||
surfaces: ['backend', 'docs'],
|
||||
commands: []
|
||||
};
|
||||
|
||||
describe('web convention writers: the body BUG-3446 replays', () => {
|
||||
afterEach(() => vi.unstubAllGlobals());
|
||||
|
||||
it('Library page Activate posts trigger and scope = surfaces[0] to conventions', async () => {
|
||||
let url = '';
|
||||
let sent: Record<string, unknown> | undefined;
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn(async (u: string, init: RequestInit) => {
|
||||
url = u;
|
||||
sent = JSON.parse(String(init.body));
|
||||
return { status: 201, ok: true, json: async () => ({ id: 'i1' }) };
|
||||
})
|
||||
);
|
||||
await api.library.activate('ws', entry);
|
||||
|
||||
expect(url).toContain('/workspaces/ws/collections/conventions/items');
|
||||
expect(JSON.parse(String(sent?.fields))).toEqual(expectedFields);
|
||||
});
|
||||
|
||||
it('Conventions page create sends the same fields', () => {
|
||||
const { title, content, ...meta } = entry;
|
||||
const payload = conventionCreatePayload(title, content, meta);
|
||||
expect(JSON.parse(String(payload.fields))).toEqual(expectedFields);
|
||||
});
|
||||
});
|
||||
@@ -871,6 +871,13 @@ export interface ItemWriteWarnings {
|
||||
* which is about the key rather than what it points at.
|
||||
*/
|
||||
unresolved_relations?: string[];
|
||||
/**
|
||||
* Select options this create ADDED to its collection's schema, keyed by
|
||||
* field (BUG-3446): a library trigger or scope a `blank` workspace's
|
||||
* Conventions or Playbooks collection did not list yet. Only words the
|
||||
* library itself uses are ever added; anything else is refused.
|
||||
*/
|
||||
options_added?: Record<string, string[]>;
|
||||
/**
|
||||
* Carried values a move or copy dropped because the destination declares
|
||||
* the field unique and another item already holds the value (BUG-2367).
|
||||
|
||||
Reference in New Issue
Block a user