mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
402f79e016
Implements SPEC-5 §Collection traits (approved v1.1) — the first unit of
PLAN-2656 phase 0. Three kernel behaviors were keyed on the literal collection
slugs "conventions" and "playbooks": what the agent bootstrap loads, which
items route by invocation slug, and which items export as portable artifacts.
Collections now DECLARE those behaviors and the kernel resolves them from the
declarations.
Fixes the KERNEL half of BUG-2702, which stays open for the rest (see below).
A slug is not a stable identifier —
UpdateCollection re-slugs on any name change, and renaming a collection is a
documented onboarding step (TASK-1510) — so renaming either collection silently
detached all three behaviors from it, with the items still present and no error
anywhere. Measured on origin/main before the fix: conventions and
convention_index dropped 1 -> 0, playbooks 1 -> 0, and GET /playbooks/{slug}
went 200 -> 404, so `/pad ship` stopped resolving with no sign the playbook
still existed. Both halves are locked by regression tests observed failing on
unfixed code.
BUG-2702 is NOT fully closed here, deliberately. Every kernel behavior follows
the trait, and library activation on the MCP dispatcher and CLI was converted
too — but the pack's own dedicated web routes (/conventions, /playbooks list and
detail, /library) still address their collection by literal slug and render
empty after a rename. Filed as BUG-2705 with the route paths and the likely fix
shape; 2702 closes when that lands. Degradation there is bounded: no data loss,
and the collection stays usable at its own /[collection] route and in the
sidebar.
SPEC-5 was amended to v1.1 BEFORE any code, per the spec tree's own discipline:
bootstrap_include becomes a LIST of {mode, filter, key} because v1.0 could not
express convention_index at all; the conventions filter is now normative and
includes status=active, which v1.0's shorthand omitted and which the
implementation does enforce (implementing v1.0 literally would have leaked
draft conventions into every agent's boot payload); v1 filters are field-
equality maps with query/1 named as the widening path, since SPEC-2 is phase 1
and PLAN-2656 forbids growing toward it; and invocation_field is constrained to
the literal `invocation_slug`, because any other field name falls outside the
partial unique indexes in migrations/054 and pgmigrations/033 that are the real
uniqueness guard.
Traits get their own column rather than a key inside the schema JSON. The
schema column is overwritten wholesale on update and every client rebuilds it
fields-only, so a traits key stored there is destroyed by one ordinary
collection edit — measured during this task, not assumed. Trait authority
cannot rest on a value an unrelated UI save deletes. UpdateCollection writes
traits only when explicitly supplied, so pre-existing clients leave them alone;
an explicit "{}" still clears.
Bootstrap keeps its three payload keys as first-party views fed from the
declarations, and gains a generic bootstrap_includes array for any other
declared key — so the boot surface is genuinely generic rather than three
hardcoded payloads, and no consumer breaks.
Existing workspaces are backfilled slug-keyed in both dialects, guarded on
traits='{}' so a re-run cannot clobber a workspace's own declarations. The
backfill inherits today's blind spot (a workspace that renamed the collection
before upgrading is not reached) but cannot do worse than the status quo, which
is itself slug-keyed; from the backfill forward the hazard is structurally gone.
Malformed declarations are refused at create and update rather than stored:
an unparseable blob degrades to "declares nothing", which is silently the wrong
behavior instead of a loud error (SPEC-0 L6).
Web groups agent-facing collections by bootstrap_include presence, replacing a
hardcoded two-slug array repeated at five call sites.
Not done, deliberately: no MCP catalog change (traits are first-party kernel
declarations, no agent needs to set them, and the separate column means
pad_collection.update passes through harmlessly — no ToolSurfaceVersion bump);
bootstrap's collections[] projection does not carry traits (PLAN-1410 trimmed
that payload and nothing consumes them there); prefix.go's NormalizeSlug is
untouched (a pure function with no workspace context, and de-hardcoding two of
its six slugs would make it less coherent, not more).
Eight Codex review rounds found nineteen real defects, all fixed here. The
serious one:
bootstrap_include filter keys FAIL OPEN. The item store's field-filter path
drops any key its sanitizer rejects, removing the predicate rather than matching
nothing, so a declaration filtering on `"stat us"` would narrow nothing and ship
every convention — drafts included — to every agent at boot, defeating the
status=active guarantee this change makes normative. Filter keys are now
validated against the store's own sanitizer shape and pinned by a cross-package
agreement test, since models cannot import store and a future divergence would
silently reopen it. SPEC-5 amended to v1.2 with the rule and its fail-open /
fail-closed asymmetry. Also fixed: an unknown declared artifact_kind reached
artifact.Encode and surfaced as a 500 (now a 400 at the export boundary, since
SPEC-5 permits unknown kinds as legal non-round-tripping declarations); and
workspace import validated traits as JSON only, so an archive could persist a
declaration that degrades to "declares nothing" (now validated, degrading to
"{}" with a warning rather than refusing an import that may be the only copy).
Later rounds found more, and several were defects this change itself created.
A hidden collection could SHADOW a visible one: resolution used to name exactly
one collection, so with several declaring, resolving across all of them and
rejecting afterwards on visibility made a visible playbook unreachable behind a
hidden one — candidates are now filtered by visibility before selection, in both
playbook resolution and artifact import. Importing a pre-traits archive produced
an INERT workspace: the migration backfill cannot reach rows inserted long after
it ran, so conventions/playbooks arrived declaring nothing, and canonical
declarations are now inferred from the slug when a collection declares none
(never overriding declarations that survived the round trip). The generic
include path had no L4 boot budget and is now capped with an overflow count.
Trait parsing claimed to be strict but json.Decoder ignores trailing bytes, so
`{...} garbage` parsed cleanly. First-party payload keys are now mode-pinned,
since their projections have fixed shapes and declaring the other mode would be
silently ignored. Duplicate artifact_kind / invocation_field declarations are
refused at the collection API, and a conflicting archive warns on import.
Agent-facing text was updated with the rest, not after it: SKILL.md,
instructions.md and the MCP catalog said the literal slugs, which is exactly the
artifact an agent acts on. ToolSurfaceVersion 0.24 -> 0.25 for the
pad_library.activate behaviour change.
Trait uniqueness is a documented BEST-EFFORT gate, not an invariant, by lead
ruling. The gate reads then writes without a lock, import bypasses it, and a
rename can mint a duplicate without touching that path. The database-level
enforcement (partial unique indexes on the extracted traits) cannot ship first:
existing deployments can already hold duplicates via rename-then-reseed, so the
index would fail the migration on precisely the databases that most need
repairing. TASK-2710 carries the de-duplication pass and the indexes; SPEC-5
v1.3 records the deferral and the reason. L6's requirement that conflicts fail
loud is met by the refusal plus the warning — the mechanism is deferred, the
principle is not.
Gates: build · make lint 0 issues · go test ./internal/... · make test-pg ·
svelte-check 0 errors · vitest 99 files / 1734 tests. Mutation-verified across
four matrices, 20 mutations, 19 caught; the survivor is a seeding path whose
trait-vs-slug difference is unreachable today (SeedCollectionsFromTemplate
creates any missing template collection before it seeds items), recorded on the
task trail rather than papered over with a test that proves nothing.
Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V
174 lines
6.7 KiB
Go
174 lines
6.7 KiB
Go
package mcp
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
|
|
"github.com/mark3labs/mcp-go/mcp"
|
|
|
|
"github.com/PerpetualSoftware/pad/internal/artifact"
|
|
"github.com/PerpetualSoftware/pad/internal/collections"
|
|
"github.com/PerpetualSoftware/pad/internal/models"
|
|
)
|
|
|
|
// --- library activate ---
|
|
|
|
// dispatchLibraryActivate reproduces `pad library activate <title>`:
|
|
// looks up a library entry by title (conventions first, then
|
|
// playbooks — same precedence the CLI uses), builds the right
|
|
// fields blob, and POSTs an item into the workspace's
|
|
// conventions/playbooks collection.
|
|
//
|
|
// Library data is sourced from internal/collections directly rather
|
|
// than via the /convention-library / /playbook-library endpoints.
|
|
// Both paths return the same data (the handlers wrap the same
|
|
// constants), and the in-process accessor avoids two extra HTTP
|
|
// round-trips per activate. The OAuth-scope hook (d.Apply) still
|
|
// runs on the eventual POST, so this isn't a scope bypass.
|
|
//
|
|
// Two minor divergences from the CLI:
|
|
//
|
|
// - The CLI uses `models.BuildConventionItemFields` for
|
|
// conventions (deals with surfaces/enforcement/commands metadata)
|
|
// but builds the playbook fields by hand. We match exactly.
|
|
// - The target collection, resolved from each entry's artifact kind via the
|
|
// collection that DECLARES it (SPEC-5), falling back to the canonical slug
|
|
// are hardcoded; we do the same. Workspaces from non-software
|
|
// templates may not have these collections, in which case the
|
|
// POST will 404 — same UX the CLI delivers.
|
|
func (d *HTTPHandlerDispatcher) dispatchLibraryActivate(
|
|
ctx context.Context,
|
|
input map[string]any,
|
|
user *models.User,
|
|
) (*mcp.CallToolResult, error) {
|
|
const cmdKey = "library activate"
|
|
workspace, _ := input["workspace"].(string)
|
|
if workspace == "" {
|
|
return validationFailedResult(cmdKey, "workspace is required",
|
|
"Pass `workspace=<slug>` or set a session default via pad_set_workspace."), nil
|
|
}
|
|
title, _ := input["title"].(string)
|
|
if title == "" {
|
|
return validationFailedResult(cmdKey, "title is required",
|
|
"Pass `title=<library-item-title>` matching an entry in the convention or playbook library."), nil
|
|
}
|
|
|
|
if conv := collections.GetLibraryConvention(title); conv != nil {
|
|
fieldsJSON, err := models.BuildConventionItemFields("active", &models.ItemConventionMetadata{
|
|
Category: conv.Category,
|
|
Trigger: conv.Trigger,
|
|
Surfaces: conv.Surfaces,
|
|
Enforcement: conv.Enforcement,
|
|
Commands: conv.Commands,
|
|
})
|
|
if err != nil {
|
|
return dispatcherErrorResult(cmdKey, "build convention fields", err), nil
|
|
}
|
|
target, terr := d.libraryTargetCollection(ctx, user, workspace, string(artifact.KindConvention), "conventions")
|
|
if terr != nil {
|
|
return dispatcherErrorResult(cmdKey, "resolve target collection", terr), nil
|
|
}
|
|
return d.postLibraryItem(ctx, user, workspace, target, cmdKey, conv.Title, conv.Content, fieldsJSON)
|
|
}
|
|
|
|
if pb := collections.GetLibraryPlaybook(title); pb != nil {
|
|
// Forward invocation_slug + arguments only when set so legacy
|
|
// library entries (none of which carry them) seed with the
|
|
// original three-field shape. Mirrors ShipPlaybook() and the
|
|
// CLI activate path in cmd/pad/main.go's libraryActivate.
|
|
fields := map[string]any{
|
|
"status": "active",
|
|
"trigger": pb.Trigger,
|
|
"scope": pb.Scope,
|
|
}
|
|
if pb.InvocationSlug != "" {
|
|
fields["invocation_slug"] = pb.InvocationSlug
|
|
}
|
|
if len(pb.Arguments) > 0 {
|
|
fields["arguments"] = pb.Arguments
|
|
}
|
|
fieldsJSON, err := json.Marshal(fields)
|
|
if err != nil {
|
|
return dispatcherErrorResult(cmdKey, "encode playbook fields", err), nil
|
|
}
|
|
target, terr := d.libraryTargetCollection(ctx, user, workspace, string(artifact.KindPlaybook), "playbooks")
|
|
if terr != nil {
|
|
return dispatcherErrorResult(cmdKey, "resolve target collection", terr), nil
|
|
}
|
|
return d.postLibraryItem(ctx, user, workspace, target, cmdKey, pb.Title, pb.Content, string(fieldsJSON))
|
|
}
|
|
|
|
return NewErrorResult(ErrorPayload{
|
|
Code: ErrNotFound,
|
|
Message: fmt.Sprintf("%s: %q not found in convention or playbook library", cmdKey, title),
|
|
Hint: "Use `pad_library action=list` to enumerate available titles.",
|
|
}), nil
|
|
}
|
|
|
|
// postLibraryItem POSTs an ItemCreate body into the named
|
|
// collection's items endpoint. Shared between conventions /
|
|
// playbooks branches of dispatchLibraryActivate so the URL +
|
|
// envelope shape stays in lockstep.
|
|
|
|
// libraryTargetCollection resolves where a library entry of the given artifact
|
|
// kind should be activated: whichever collection DECLARES that kind (SPEC-5
|
|
// artifact_kind), not the one that happens to be named "conventions" or
|
|
// "playbooks". Activating into a workspace whose collection was renamed used
|
|
// to fail not-found with the collection sitting right there ([[BUG-2702]]).
|
|
//
|
|
// A LOOKUP FAILURE IS NOT A FALLBACK CASE. Falling back on an error means
|
|
// writing to a slug we never confirmed anything about, and a workspace may
|
|
// legitimately have an ordinary collection sitting on the canonical slug, so
|
|
// the guess can land a library entry somewhere unrelated. Errors are returned;
|
|
// the fallback applies ONLY when the read SUCCEEDED and no collection declares
|
|
// the kind, which is the genuine pre-backfill case. Codex round 5.
|
|
func (d *HTTPHandlerDispatcher) libraryTargetCollection(
|
|
ctx context.Context,
|
|
user *models.User,
|
|
workspace, kind, fallback string,
|
|
) (string, error) {
|
|
req, err := d.buildAuthedRequest(ctx, http.MethodGet,
|
|
"/api/v1/workspaces/"+url.PathEscape(workspace)+"/collections", nil, user)
|
|
if err != nil {
|
|
return "", fmt.Errorf("resolve target collection: %w", err)
|
|
}
|
|
rec := httptest.NewRecorder()
|
|
d.Handler.ServeHTTP(rec, req)
|
|
if rec.Code != http.StatusOK {
|
|
return "", fmt.Errorf("resolve target collection: listing collections returned %d", rec.Code)
|
|
}
|
|
var colls []models.Collection
|
|
if err := json.Unmarshal(rec.Body.Bytes(), &colls); err != nil {
|
|
return "", fmt.Errorf("resolve target collection: decode collections: %w", err)
|
|
}
|
|
if slug := collections.SlugForArtifactKind(colls, kind); slug != "" {
|
|
return slug, nil
|
|
}
|
|
return fallback, nil
|
|
}
|
|
|
|
func (d *HTTPHandlerDispatcher) postLibraryItem(
|
|
ctx context.Context,
|
|
user *models.User,
|
|
workspace, collection, cmdKey, title, content, fieldsJSON string,
|
|
) (*mcp.CallToolResult, error) {
|
|
payload := map[string]any{
|
|
"title": title,
|
|
"fields": fieldsJSON,
|
|
}
|
|
if content != "" {
|
|
payload["content"] = content
|
|
}
|
|
body, err := json.Marshal(payload)
|
|
if err != nil {
|
|
return dispatcherErrorResult(cmdKey, "encode body", err), nil
|
|
}
|
|
urlPath := "/api/v1/workspaces/" + url.PathEscape(workspace) +
|
|
"/collections/" + url.PathEscape(collection) + "/items"
|
|
return d.executeRequest(ctx, cmdKey, user, http.MethodPost, urlPath, body)
|
|
}
|