diff --git a/AGENTS.md b/AGENTS.md index b6982b6..516455d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -80,17 +80,24 @@ store.Repository**. `viewer | publisher | admin`, permissions `read | edit | delete`. A missing read permission is surfaced as 404, never 403, so frame existence does not leak. - `backend/internal/frames` owns the Frame content model: - - `schema.go` - the `Doc`/`Slots` YAML types. Parsing uses `KnownFields(true)`: the slot schema is - fixed and unknown keys are an error. - - `slots.go` - `SlotTable` is the single source of truth for slot keys, markdown headings, and - content shape (terms / list / prose). Add or rename a slot here only; the `.frame.md` codec and - the MCP composer both read it. - - `framemd.go` - the Frame Spec v0.2 `.frame.md` codec (YAML frontmatter plus one `##` section per - slot). Round-trip fidelity matters: `examples/*.yaml` and `examples/*.frame.md` are checked-in - conformance fixtures asserted by `examples_test.go`. - - `resolver.go` - inheritance merge over `extends`/`excludes`. Later parents win, the child's own - slots win last, cycles produce `CycleError`, and an unreadable ancestor propagates - `ErrParentUnreadable` rather than silently dropping content. + - `schema.go` - the `Doc` YAML type. A Frame's content is a single free-form markdown `Body`, + matching Frame Spec v0.2, which requires four frontmatter fields and defines no body structure. + Parsing uses `KnownFields(true)`: the document schema is fixed and unknown keys are an error. + - `legacy.go` - read-only support for documents published under the retired ten-slot schema. + `Parse` folds a `slots:` block into `Body`; nothing ever writes that shape again. Stored versions + are immutable, so this has to stay readable forever. It is mirrored once, in + `web/src/lib/frame-yaml.ts`, because the web app renders a stored legacy version without a + server round trip - and that render can become canonical content when a legacy version is + restored. Both sides are pinned to `testdata/legacy-slots/`; change one and you must + regenerate `expected.md` and update the other. + - `framemd.go` - the Frame Spec v0.2 `.frame.md` codec: YAML frontmatter, then the body verbatim. + The frontmatter delimiter matches only at column 0, since an indented `---` inside a multi-line + YAML value would otherwise truncate the document. Round-trip fidelity matters: `examples/*.yaml` + and `examples/*.frame.md` are checked-in conformance fixtures asserted by `examples_test.go`. + - `resolver.go` - inheritance merge over `extends`/`excludes`. Ancestors' bodies are concatenated + ancestors-first so the child's own guidance reads last, each `ref@version` is included once, + cycles produce `CycleError`, and an unreadable ancestor propagates `ErrParentUnreadable` rather + than silently dropping content. - `service.go` - `publish` is the single write path behind both front doors. It authorizes before parsing caller-supplied content, enforces `MaxContentBytes` on the stored document, refuses a version that does not advance `latest_version`, and refuses a write whose declared base version @@ -110,7 +117,7 @@ store.Repository**. Connect API uses, so the adapter itself performs no permission or validation logic. Two rules matter when changing it. `update_frame` merges onto the frame's own document from `SourceDoc` and never onto the composed form `get_frame` returns by default - merging onto a resolved document - would copy every parent's slots into the child and drop its `extends` edges. And the base version + would bake every ancestor's body into the child and drop its `extends` edges. And the base version it asserts against comes from the caller (`base_version`, read via `get_frame source=true`), never from a fresh server-side read, which would always match and make the check inert. Request bodies are capped at `mcp.MaxRequestBytes`; the cap must wrap the outermost handler, since the bearer @@ -141,8 +148,9 @@ store.Repository**. - A test that asserts only "this was rejected" usually proves nothing: an unrelated 401, or a parse failure, satisfies it just as well. Assert the specific code or message, and pair a rejection with a control that must succeed. Reflective guards in `backend/internal/mcp/resources_test.go` walk - `frames.SlotTable` and `frames.Doc`, so adding a slot without wiring it through the MCP input - fails rather than silently dropping data. + `frames.Doc`, so adding a document field without wiring it through the MCP input fails rather than + silently dropping data - the guard names the field and asks for a decision rather than defaulting + to one. - Comments in this repo explain *why* a constraint exists (pinned CI versions, fail-closed readiness, the vite `@bufbuild/protobuf` aliases). Preserve that rationale when editing near it, and keep new comments in the same register. diff --git a/README.md b/README.md index e2f7b28..92610b8 100644 --- a/README.md +++ b/README.md @@ -14,13 +14,13 @@ See [Background §1.1 in the migration design doc](docs/design/2026-05-21-nebari ### Frame Spec conformance -Frames are stored in a slot-typed YAML schema with inheritance and RBAC, which is richer than -[Frame Spec v0.2](https://github.com/openteams-ai/frame-spec) describes. The registry interoperates -with the spec rather than replacing it: every Frame **imports and exports as a conformant -`.frame.md` document** (`type: frame [0.2]`, YAML frontmatter, one `##` section per slot), and the -web app's authoring page offers that document as a second editor alongside the typed form. Exports -pass the spec project's own `tools/validate_frames.py`; `examples/*.frame.md` are checked-in -examples of the output. +A Frame's content is a single free-form markdown body, exactly as +[Frame Spec v0.2](https://github.com/openteams-ai/frame-spec) defines it; the registry adds +metadata, versioning, inheritance, and RBAC around that body. Every Frame **imports and exports as +a conformant `.frame.md` document** (`type: frame [0.2]`, YAML frontmatter, then the body +verbatim), and the web app's authoring page offers that document as a second editor alongside the +form. Exports pass the spec project's own `tools/validate_frames.py`; `examples/*.frame.md` are +checked-in examples of the output. The spec's optional `visibility`, `scope`, and `maintainer` fields travel with the document so a Frame survives a round trip through other tooling. **`visibility` is declared intent, not an access @@ -36,7 +36,7 @@ control** - who may read a Frame is decided by this registry's roles and grants. make dev ``` -Runs the backend (dev mode, no OIDC) on `:8080` and the Vite dev server on `:5173`, seeded with representative sample data (an org, members across roles, and frames with full slot content, multi-level inheritance, and versions). Open **http://localhost:5173**; UI edits hot-reload. Ctrl-C stops both. +Runs the backend (dev mode, no OIDC) on `:8080` and the Vite dev server on `:5173`, seeded with representative sample data (an org, members across roles, and frames with real body content, multi-level inheritance, and versions). Open **http://localhost:5173**; UI edits hot-reload. Ctrl-C stops both. There is no login step in this loop: dev mode disables OIDC and injects a fixed identity, so you land straight in the app as `dev-user`, an org admin - and never hit the "No organization access" screen (see [Troubleshooting](#troubleshooting)). diff --git a/backend/internal/devfixture/devfixture.go b/backend/internal/devfixture/devfixture.go index 81416dd..8b38dbb 100644 --- a/backend/internal/devfixture/devfixture.go +++ b/backend/internal/devfixture/devfixture.go @@ -45,6 +45,7 @@ type fixtureFrame struct { id string name string description string + isTemplate bool versions []frameVersion // oldest first } @@ -82,9 +83,8 @@ func mustMarshalDoc(doc *frames.Doc) string { // └── team-notebook (v1.0.0) extends pytorch-gpu@1.0.0 // standalone-frame (v1.0.0) no parents // -// Every frame fills real content slots (terminology, rules, skills, prompts, -// and the free-text goals/style/norms/architecture/business_process) so the -// detail view and the inheritance resolver have representative data to render. +// Every frame carries a real free-form markdown body so the detail view and +// the inheritance resolver have representative data to render. func buildFrames(orgSlug string) []fixtureFrame { return []fixtureFrame{ { @@ -98,26 +98,13 @@ func buildFrames(orgSlug string) []fixtureFrame { Visibility: "internal", Scope: "company", Maintainer: "platform engineering", - Slots: frames.Slots{ - Terminology: []frames.Term{ - {Term: "Frame", Definition: "A versioned, composable unit of environment and agent context that other frames can extend."}, - {Term: "Environment", Definition: "The reproducible set of pinned dependencies (conda/pip) a workload runs against."}, - }, - Rules: []string{ - "Pin every dependency to an exact version; never use unbounded ranges.", - "Reproducibility is non-negotiable: the same frame must resolve identically on every machine.", - "Prefer conda-forge as the primary channel for scientific packages.", - }, - Skills: []string{ - "Resolving and locking conda environments.", - "Reading and writing reproducible dependency manifests.", - }, - Goals: "Provide a stable, reproducible foundation so downstream frames can focus on their specialization rather than base setup.", - Style: "Terse, explicit, and reproducible. Favor declarative configuration over imperative setup scripts.", - Norms: "Changes to pinned tooling require a version bump and a changelog entry.", - }, + Body: `Pin every dependency to an exact version; never use unbounded ranges. Reproducibility is non-negotiable: the same frame must resolve identically on every machine. + +Prefer conda-forge as the primary channel for scientific packages. + +Changes to pinned tooling require a version bump and a changelog entry. Favor declarative configuration over imperative setup scripts.`, })}, - {version: "2.0.0", changelog: "Bump pinned tooling (Python 3.12) and add packaging skill", + {version: "2.0.0", changelog: "Bump pinned tooling (Python 3.12)", content: mustMarshalDoc(&frames.Doc{ Name: "base-ml-env", Description: "Base machine-learning environment: shared conventions every ML frame inherits.", @@ -125,27 +112,11 @@ func buildFrames(orgSlug string) []fixtureFrame { Visibility: "internal", Scope: "company", Maintainer: "platform engineering", - Slots: frames.Slots{ - Terminology: []frames.Term{ - {Term: "Frame", Definition: "A versioned, composable unit of environment and agent context that other frames can extend."}, - {Term: "Environment", Definition: "The reproducible set of pinned dependencies (conda/pip) a workload runs against."}, - {Term: "Lockfile", Definition: "A fully-resolved, hash-pinned snapshot of an environment used for byte-identical rebuilds."}, - }, - Rules: []string{ - "Pin every dependency to an exact version; never use unbounded ranges.", - "Reproducibility is non-negotiable: the same frame must resolve identically on every machine.", - "Prefer conda-forge as the primary channel for scientific packages.", - "Target Python 3.12 unless a downstream frame pins otherwise.", - }, - Skills: []string{ - "Resolving and locking conda environments.", - "Reading and writing reproducible dependency manifests.", - "Building and publishing conda packages to a private channel.", - }, - Goals: "Provide a stable, reproducible foundation so downstream frames can focus on their specialization rather than base setup.", - Style: "Terse, explicit, and reproducible. Favor declarative configuration over imperative setup scripts.", - Norms: "Changes to pinned tooling require a version bump and a changelog entry.", - }, + Body: `Pin every dependency to an exact version; never use unbounded ranges. Reproducibility is non-negotiable: the same frame must resolve identically on every machine. Rebuilds should be byte-identical from the lockfile. + +Prefer conda-forge as the primary channel for scientific packages. Target Python 3.12 unless a downstream frame pins otherwise. Build and publish internal conda packages to the private channel. + +Changes to pinned tooling require a version bump and a changelog entry. Favor declarative configuration over imperative setup scripts.`, })}, }, }, @@ -163,29 +134,11 @@ func buildFrames(orgSlug string) []fixtureFrame { Extends: []frames.ExtendRef{ {Ref: orgSlug + "/base-ml-env", Version: "2.0.0"}, }, - Slots: frames.Slots{ - Terminology: []frames.Term{ - {Term: "CUDA", Definition: "NVIDIA's parallel computing platform used to run PyTorch tensor ops on the GPU."}, - {Term: "Mixed precision", Definition: "Training with a mix of float16 and float32 to cut memory use and speed up compute."}, - }, - Rules: []string{ - "Pin the CUDA toolkit version to match the target driver; mismatches fail silently at runtime.", - "Always guard GPU code with a CPU fallback so tests run in CI without a GPU.", - "Enable mixed precision for training runs unless numerical stability requires float32.", - }, - Skills: []string{ - "Configuring PyTorch for a specific CUDA/cuDNN version.", - "Diagnosing out-of-memory errors and tuning batch size and gradient accumulation.", - "Profiling GPU utilization to find data-loading bottlenecks.", - }, - Prompts: []string{ - "Given a training script, suggest the largest batch size that fits in the available GPU memory.", - "Review this model code and flag any operations that will silently fall back to CPU.", - }, - ToolSpecs: "torch>=2.2, torchvision, cuda-toolkit 12.1, cudnn. Expose `nvidia-smi` for GPU introspection.", - Goals: "Give ML engineers a ready-to-train GPU environment with sane defaults for CUDA, so they iterate on models rather than plumbing.", - Architecture: "Single-node, single-or-multi-GPU. Data loaders run on CPU workers feeding the GPU; checkpoints written to the shared volume.", - }, + Body: `The stack is torch>=2.2 with torchvision on cuda-toolkit 12.1 and cudnn; ` + "`nvidia-smi`" + ` is available for GPU introspection. Single-node, single-or-multi-GPU: data loaders run on CPU workers feeding the GPU, and checkpoints are written to the shared volume. + +Pin the CUDA toolkit version to match the target driver; mismatches fail silently at runtime. Always guard GPU code with a CPU fallback so tests run in CI without a GPU. + +Enable mixed precision for training runs unless numerical stability requires float32. When memory is tight, tune batch size and gradient accumulation before reaching for a bigger GPU, and profile GPU utilization to find data-loading bottlenecks.`, }), extends: []store.ParentEdge{{ParentFrameID: idBaseMLEnv, ParentVersion: "2.0.0", OrderIndex: 0}}}, }, @@ -204,34 +157,18 @@ func buildFrames(orgSlug string) []fixtureFrame { Extends: []frames.ExtendRef{ {Ref: orgSlug + "/pytorch-gpu", Version: "1.0.0"}, }, - Slots: frames.Slots{ - Terminology: []frames.Term{ - {Term: "Notebook profile", Definition: "A JupyterLab configuration bundle (kernels, extensions, resource limits) applied to a team's servers."}, - }, - Rules: []string{ - "Commit notebooks with cleared outputs; large embedded outputs bloat the repo.", - "Shared datasets live under /shared/data (read-only); never copy them into a home directory.", - "Long-running jobs belong in the batch queue, not in an interactive notebook kernel.", - }, - Skills: []string{ - "Using the team's shared JupyterLab extensions and kernels.", - "Moving an exploratory notebook into a reproducible pipeline.", - }, - Prompts: []string{ - "Convert this exploratory notebook cell into a parameterized, testable function.", - "Suggest where in this notebook to checkpoint intermediate results to the shared volume.", - }, - Goals: "Let the data-science team share one reproducible, GPU-ready notebook environment with agreed-upon conventions.", - Style: "Collaborative and review-friendly: notebooks should read like documented experiments, not scratch pads.", - Norms: "New shared extensions are proposed in the team channel and added here via a version bump.", - BusinessProcess: "Exploration happens in notebooks; promising results are promoted to a tracked pipeline before any production use.", - }, + Body: `Commit notebooks with cleared outputs; large embedded outputs bloat the repo. Notebooks should read like documented experiments, not scratch pads. + +Shared datasets live under /shared/data (read-only); never copy them into a home directory. Long-running jobs belong in the batch queue, not in an interactive notebook kernel. + +Exploration happens in notebooks; promising results are promoted to a tracked, reproducible pipeline before any production use. New shared JupyterLab extensions are proposed in the team channel and added here via a version bump.`, }), extends: []store.ParentEdge{{ParentFrameID: idPyTorchGPU, ParentVersion: "1.0.0", OrderIndex: 0}}}, }, }, { id: idStandalone, name: "standalone-frame", description: "Standalone data-cleaning frame with no parents, for exercising the non-inheriting case.", + isTemplate: true, // exercises the "start from a template" picker versions: []frameVersion{ {version: "1.0.0", changelog: "Initial standalone frame", content: mustMarshalDoc(&frames.Doc{ @@ -241,25 +178,12 @@ func buildFrames(orgSlug string) []fixtureFrame { Visibility: "shared", Scope: "project", Maintainer: "data science", - Slots: frames.Slots{ - Terminology: []frames.Term{ - {Term: "Tidy data", Definition: "A table where each variable is a column, each observation a row, and each cell a single value."}, - }, - Rules: []string{ - "Never mutate the raw input in place; write cleaned output to a new location.", - "Record every transformation so the cleaning run is fully auditable.", - }, - Skills: []string{ - "Profiling a dataset for missing values, outliers, and type inconsistencies.", - "Writing idempotent, re-runnable data-cleaning transforms.", - }, - Prompts: []string{ - "Given this dataframe schema, propose a set of validation checks to run before cleaning.", - }, - Goals: "Provide a self-contained frame for tabular data cleaning that depends on nothing else.", - Style: "Defensive and explicit: validate assumptions loudly and fail fast on malformed input.", - BusinessProcess: "Raw data lands, is validated, is cleaned into a tidy table, and only then is handed to downstream analysis.", - }, + Template: true, + Body: `Never mutate the raw input in place; write cleaned output to a new location, and record every transformation so the cleaning run is fully auditable. + +Profile a dataset for missing values, outliers, and type inconsistencies before cleaning it, and write idempotent, re-runnable transforms. Validate assumptions loudly and fail fast on malformed input. + +Raw data lands, is validated, is cleaned into a tidy table (each variable a column, each observation a row), and only then is handed to downstream analysis.`, })}, }, }, @@ -317,6 +241,7 @@ func loadFrame(ctx context.Context, repo store.Repository, org *framesv1.Org, f Frame: &framesv1.Frame{ Id: f.id, OrgId: org.Id, Name: f.name, Description: f.description, OwnerSub: ownerSub, LatestVersion: v.version, CreatedAt: now, UpdatedAt: now, + IsTemplate: f.isTemplate, }, Version: &framesv1.FrameVersion{ Version: v.version, Changelog: v.changelog, Content: content, diff --git a/backend/internal/frames/admin_test.go b/backend/internal/frames/admin_test.go index 84f4ac8..5ac273c 100644 --- a/backend/internal/frames/admin_test.go +++ b/backend/internal/frames/admin_test.go @@ -68,9 +68,8 @@ func TestDeleteFrameBlockThenForce(t *testing.T) { publishFrame(t, ctx, svc, []byte(`name: parent description: Parent frame version: 1.0.0 -slots: - rules: - - Parent rule. +body: | + Parent rule. `)) // Publish child frame that extends parent. @@ -80,9 +79,8 @@ version: 1.0.0 extends: - ref: openteams/parent version: 1.0.0 -slots: - rules: - - Child rule. +body: | + Child rule. `)) // force=false should block and list the child. @@ -154,9 +152,8 @@ func TestDeleteFrameDeniedForViewer(t *testing.T) { publishFrame(t, adminCtx, svc, []byte(`name: secret-frame description: Admin frame version: 1.0.0 -slots: - rules: - - Only admins. +body: | + Only admins. `)) // Non-deleter attempts to delete. diff --git a/backend/internal/frames/framemd.go b/backend/internal/frames/framemd.go index 5a625f3..9fb3111 100644 --- a/backend/internal/frames/framemd.go +++ b/backend/internal/frames/framemd.go @@ -12,14 +12,21 @@ import ( // This file implements the .frame.md interchange format: the single-Markdown- // file-with-YAML-frontmatter shape defined by Frame Spec v0.2 // (https://github.com/openteams-ai/frame-spec). The canonical stored form -// remains the slot YAML in schema.go; this is a lossless codec on top of it. +// remains the YAML in schema.go; this is a codec on top of it. // -// Parsing is strict about *structure* (frontmatter delimiters, unknown keys, -// unknown section headings, malformed bullets) because guessing would silently -// mangle an author's content. It is deliberately lenient about *values*: -// an empty description or an unpinned `inherits` ref produces a Doc that -// Validate then rejects, so the error lands on the relevant form field where it -// can be fixed, rather than blocking the import outright. +// The spec requires four frontmatter fields and leaves the body entirely +// free-form, so the codec is simple: frontmatter maps to Doc metadata and +// everything after the closing --- is the body, verbatim. Parsing is strict +// about the frontmatter block (delimiters, unknown keys) because guessing +// would silently mangle an author's metadata; the body is never rejected. +// +// Round-tripping preserves content but normalizes in two known ways, both +// deliberate and both asserted in framemd_test.go. An empty visibility comes +// back as DefaultVisibility, because the spec requires the field and something +// has to be written. Leading and trailing blank lines around the body are +// dropped, because the body's offset from the closing --- is framing rather +// than content. Nothing else is normalized, and in particular no byte of the +// body between its first and last non-blank line is touched. // SpecVersion is the Frame Spec release this codec reads and writes. const SpecVersion = "0.2" @@ -34,9 +41,6 @@ const DefaultVisibility = "internal" // (tools/validate_frames.py): `frame` or `frame [.]`. var typeRe = regexp.MustCompile(`^frame(?: \[\d+\.\d+\])?$`) -// termRe matches a rendered terminology bullet body: "**term**: definition". -var termRe = regexp.MustCompile(`(?s)^\*\*(.+?)\*\*:[ \t]?(.*)$`) - // unknownKeyRe extracts the offending key from a yaml.v3 KnownFields error. var unknownKeyRe = regexp.MustCompile(`line (\d+): field (\S+) not found`) @@ -44,31 +48,8 @@ var unknownKeyRe = regexp.MustCompile(`line (\d+): field (\S+) not found`) // messages. Keep in sync with the frontmatter struct below. var frontmatterKeys = []string{ "type", "name", "description", "visibility", - "version", "scope", "maintainer", "inherits", "x-nebari-excludes", -} - -// headingHints points common headings from frames authored elsewhere at the -// closest slot. These only enrich the rejection message; nothing is ever mapped -// automatically, because silently relocating an author's content is worse than -// telling them where it belongs. -var headingHints = map[string]string{ - "ways of working": "Norms", - "how we work": "Norms", - "conventions": "Norms", - "constraints": "Rules", - "guardrails": "Rules", - "policies": "Rules", - "vocabulary": "Terminology", - "glossary": "Terminology", - "definitions": "Terminology", - "voice": "Style", - "tone": "Style", - "objectives": "Goals", - "purpose": "Goals", - "process": "Business Process", - "workflow": "Business Process", - "tools": "Tool Specifications", - "tooling": "Tool Specifications", + "version", "scope", "maintainer", "inherits", + "x-nebari-excludes", "x-nebari-template", } // stringOrSlice accepts either a scalar or a sequence, both of which Frame Spec @@ -106,12 +87,17 @@ type frontmatter struct { Scope string `yaml:"scope,omitempty"` Maintainer string `yaml:"maintainer,omitempty"` Inherits stringOrSlice `yaml:"inherits,omitempty"` - // Excludes has no Frame Spec equivalent, so it lives in an `x-` namespace - // as the spec advises implementations to do for their own fields. + // Excludes and Template have no Frame Spec equivalent, so they live in an + // `x-` namespace as the spec advises implementations to do for their own + // fields. Excludes stringOrSlice `yaml:"x-nebari-excludes,omitempty"` + Template bool `yaml:"x-nebari-template,omitempty"` } -// MarshalMarkdown renders a Doc as a spec-conformant .frame.md document. +// MarshalMarkdown renders a Doc as a spec-conformant .frame.md document. The +// body passes through verbatim between its first and last non-blank line; see +// the round-trip note at the top of this file for the two normalizations +// UnmarshalMarkdown applies on the way back. func MarshalMarkdown(doc *Doc) ([]byte, error) { fm := frontmatter{ Type: MarkdownType, @@ -122,6 +108,7 @@ func MarshalMarkdown(doc *Doc) ([]byte, error) { Scope: doc.Scope, Maintainer: doc.Maintainer, Excludes: doc.Excludes, + Template: doc.Template, } if fm.Visibility == "" { fm.Visibility = DefaultVisibility @@ -142,62 +129,30 @@ func MarshalMarkdown(doc *Doc) ([]byte, error) { var b strings.Builder b.WriteString("---\n") b.Write(head) - b.WriteString("---\n\n") - if doc.Name != "" { - fmt.Fprintf(&b, "# %s\n\n", doc.Name) - } - for _, d := range SlotTable { - writeSlot(&b, d, &doc.Slots) - } - return []byte(strings.TrimRight(b.String(), "\n") + "\n"), nil -} - -func writeSlot(b *strings.Builder, d SlotDescriptor, s *Slots) { - switch d.Kind { - case SlotTerms: - if len(s.Terminology) == 0 { - return - } - fmt.Fprintf(b, "## %s\n\n", d.Heading) - for _, t := range s.Terminology { - WriteBullet(b, fmt.Sprintf("**%s**: %s", t.Term, t.Definition)) - } + b.WriteString("---\n") + if body := strings.Trim(doc.Body, "\n"); body != "" { b.WriteString("\n") - case SlotList: - items := s.List(d.Key) - if len(items) == 0 { - return - } - fmt.Fprintf(b, "## %s\n\n", d.Heading) - for _, it := range items { - WriteBullet(b, it) - } + b.WriteString(body) b.WriteString("\n") - case SlotProse: - body := s.Prose(d.Key) - if strings.TrimSpace(body) == "" { - return - } - fmt.Fprintf(b, "## %s\n\n%s\n\n", d.Heading, strings.Trim(body, "\n")) } + return []byte(b.String()), nil } -// WriteBullet renders one list item as a markdown bullet. Continuation lines -// are indented two spaces so a multi-line item stays part of that item instead -// of terminating the list, and so it round-trips back to the same string. -func WriteBullet(b *strings.Builder, item string) { - lines := strings.Split(strings.Trim(item, "\n"), "\n") - fmt.Fprintf(b, "- %s\n", lines[0]) - for _, l := range lines[1:] { - if strings.TrimSpace(l) == "" { - b.WriteString("\n") - continue - } - fmt.Fprintf(b, " %s\n", l) - } +// isFence reports whether a line is a frontmatter delimiter. +// +// The comparison is anchored at column 0 on purpose. Matching TrimSpace(line) +// instead would let an indented --- inside a multi-line frontmatter value close +// the block early, silently truncating the document and dropping every field +// after it - and a YAML block scalar, which is the only way to write a +// multi-line value, is always indented. Trailing whitespace is tolerated +// because editors add it and it cannot occur inside a scalar without the +// indentation that already disqualifies the line. +func isFence(line string) bool { + return strings.TrimRight(line, " \t") == "---" } // UnmarshalMarkdown parses a .frame.md document into a Doc. Structural problems +// (all in the frontmatter block - the body is free-form and never rejected) // are returned as a *ValidationError whose paths are all "markdown" and whose // messages carry the 1-based line number, so the web editor can surface them // against the source. Value-level problems are left for Validate. @@ -219,14 +174,14 @@ func UnmarshalMarkdown(content []byte) (*Doc, error) { for i < len(lines) && strings.TrimSpace(lines[i]) == "" { i++ } - if i >= len(lines) || strings.TrimSpace(lines[i]) != "---" { + if i >= len(lines) || !isFence(lines[i]) { add(i+1, "a frame must begin with a YAML frontmatter block delimited by ---") return nil, fail() } start := i + 1 end := -1 for j := start; j < len(lines); j++ { - if strings.TrimSpace(lines[j]) == "---" { + if isFence(lines[j]) { end = j break } @@ -254,6 +209,9 @@ func UnmarshalMarkdown(content []byte) (*Doc, error) { case !typeRe.MatchString(fm.Type): add(start+1, "type must be \"frame\" or \"frame [.]\" (for example %q), got %q", MarkdownType, fm.Type) } + if len(errs) > 0 { + return nil, fail() + } doc := &Doc{ Name: fm.Name, @@ -263,90 +221,12 @@ func UnmarshalMarkdown(content []byte) (*Doc, error) { Scope: fm.Scope, Maintainer: fm.Maintainer, Excludes: fm.Excludes, + Template: fm.Template, + Body: strings.Trim(strings.Join(lines[end+1:], "\n"), "\n"), } for _, ref := range fm.Inherits { doc.Extends = append(doc.Extends, parseInherit(ref)) } - - // --- body --- - body := lines[end+1:] - bodyOffset := end + 2 // 1-based file line number of body[0] - - var heads []int - for idx, l := range body { - if strings.HasPrefix(l, "## ") { - heads = append(heads, idx) - } - } - - preEnd := len(body) - if len(heads) > 0 { - preEnd = heads[0] - } - seenH1 := false - for idx := 0; idx < preEnd; idx++ { - t := strings.TrimSpace(body[idx]) - if t == "" { - continue - } - if !seenH1 && strings.HasPrefix(t, "# ") { - seenH1 = true // the title heading; the name comes from frontmatter - continue - } - add(bodyOffset+idx, "content before the first section heading; frame body content must sit under a recognized \"## \" section") - break - } - - seen := map[string]bool{} - for h, hi := range heads { - stop := len(body) - if h+1 < len(heads) { - stop = heads[h+1] - } - heading := strings.TrimSpace(strings.TrimPrefix(body[hi], "## ")) - d, ok := SlotByHeading(heading) - if !ok { - add(bodyOffset+hi, "unknown section \"## %s\"%s — recognized sections are: %s", - heading, hint(heading), strings.Join(Headings(), ", ")) - continue - } - if seen[d.Key] { - add(bodyOffset+hi, "duplicate section \"## %s\"", heading) - continue - } - seen[d.Key] = true - - content := body[hi+1 : stop] - switch d.Kind { - case SlotTerms: - items, starts, stray := parseBullets(content, bodyOffset+hi+1) - for _, ln := range stray { - add(ln, "unexpected content in \"## %s\" — every entry must be a \"- **term**: definition\" bullet", heading) - } - for n, it := range items { - m := termRe.FindStringSubmatch(it) - if m == nil { - add(starts[n], "malformed terminology entry — expected \"- **term**: definition\"") - continue - } - doc.Slots.Terminology = append(doc.Slots.Terminology, Term{ - Term: strings.TrimSpace(m[1]), Definition: strings.TrimSpace(m[2]), - }) - } - case SlotList: - items, _, stray := parseBullets(content, bodyOffset+hi+1) - for _, ln := range stray { - add(ln, "unexpected content in \"## %s\" — every entry must be a \"- item\" bullet", heading) - } - doc.Slots.SetList(d.Key, items) - case SlotProse: - doc.Slots.SetProse(d.Key, strings.Trim(strings.Join(content, "\n"), "\n")) - } - } - - if len(errs) > 0 { - return nil, fail() - } return doc, nil } @@ -360,51 +240,6 @@ func parseInherit(ref string) ExtendRef { return ExtendRef{Ref: ref} } -// parseBullets splits a section body into list items. Lines indented two spaces -// continue the preceding item (the inverse of WriteBullet). base is the 1-based -// file line number of content[0]; stray holds lines that are neither. -func parseBullets(content []string, base int) (items []string, starts, stray []int) { - var cur []string - flush := func() { - if cur != nil { - items = append(items, strings.Trim(strings.Join(cur, "\n"), "\n")) - cur = nil - } - } - for i, l := range content { - switch { - case strings.HasPrefix(l, "- "): - flush() - starts = append(starts, base+i) - cur = []string{strings.TrimPrefix(l, "- ")} - case strings.TrimSpace(l) == "": - if cur != nil { - cur = append(cur, "") - } - case strings.HasPrefix(l, " ") && cur != nil: - cur = append(cur, strings.TrimPrefix(l, " ")) - default: - stray = append(stray, base+i) - } - } - flush() - return items, starts, stray -} - -// hint suggests the closest recognized section for a rejected heading. -func hint(heading string) string { - k := strings.ToLower(strings.TrimSpace(heading)) - for _, d := range SlotTable { - if strings.EqualFold(d.Heading, k) { - return fmt.Sprintf(" — did you mean \"## %s\"?", d.Heading) - } - } - if h, ok := headingHints[k]; ok { - return fmt.Sprintf(" — did you mean \"## %s\"?", h) - } - return "" -} - // frontmatterErr turns a yaml.v3 decode failure into a message that names the // file line and, for unknown keys, the recognized key set. func frontmatterErr(err error, offset int) string { diff --git a/backend/internal/frames/framemd_test.go b/backend/internal/frames/framemd_test.go index bcbd620..aede7ad 100644 --- a/backend/internal/frames/framemd_test.go +++ b/backend/internal/frames/framemd_test.go @@ -1,7 +1,6 @@ package frames_test import ( - "errors" "os" "path/filepath" "reflect" @@ -60,8 +59,8 @@ func TestExampleFrames_MarkdownRoundTrip(t *testing.T) { if err != nil { t.Fatalf("unmarshal markdown: %v", err) } - normalizeProse(doc) - normalizeProse(back) + normalizeBody(doc) + normalizeBody(back) if !reflect.DeepEqual(doc, back) { t.Errorf("round trip lost data.\noriginal: %+v\nround-tripped: %+v", doc, back) } @@ -84,6 +83,7 @@ func TestMarshalMarkdown_Frontmatter(t *testing.T) { {Ref: "industry/healthcare", Version: "2024.4"}, }, Excludes: []string{"openteams/legacy"}, + Body: "Lead with customer impact.", } md, err := frames.MarshalMarkdown(doc) if err != nil { @@ -98,7 +98,7 @@ func TestMarshalMarkdown_Frontmatter(t *testing.T) { " - openteams/company-core@1.2.0\n", " - industry/healthcare@2024.4\n", "x-nebari-excludes:\n", - "# brand-voice\n", + "Lead with customer impact.\n", } { if !strings.Contains(got, want) { t.Errorf("output missing %q\n---\n%s", want, got) @@ -118,6 +118,23 @@ func TestMarshalMarkdown_DefaultsVisibility(t *testing.T) { } } +// The body is emitted verbatim: no synthesized title heading, no section +// structure imposed on the author's markdown. +func TestMarshalMarkdown_BodyPassthrough(t *testing.T) { + doc := &frames.Doc{ + Name: "c", Description: "d", Version: "1.0.0", Visibility: "internal", + Body: "# My Own Title\n\nSome prose.\n\n## Any Heading At All\n\n- a bullet", + } + md, err := frames.MarshalMarkdown(doc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + wantTail := "---\n\n" + doc.Body + "\n" + if !strings.HasSuffix(string(md), wantTail) { + t.Errorf("body not passed through verbatim:\n%s", md) + } +} + func TestUnmarshalMarkdown_InheritsForms(t *testing.T) { tests := []struct { name string @@ -146,72 +163,87 @@ func TestUnmarshalMarkdown_InheritsForms(t *testing.T) { } } -// The spec's own examples/complete/frame.md must import: its sections all map -// to slots, and its bare `inherits` must surface as a fixable field error from -// Validate rather than blocking the conversion. -func TestUnmarshalMarkdown_SpecCompleteExample(t *testing.T) { +// Any body shape at all must import: the spec defines no sections, so headings, +// loose prose, bullets, and content before a heading are all just body. +func TestUnmarshalMarkdown_FreeFormBody(t *testing.T) { src := `--- type: frame -name: engineering-documentation-style -description: Writing guidance for engineering documentation. -visibility: internal +name: code-review-norms +description: How this team reviews pull requests. +visibility: shared version: 0.1.0 scope: department maintainer: engineering enablement -inherits: editorial-style-guide --- -# Engineering Documentation Style +# Code Review Norms -## Goals +Block on correctness, security, and data loss. -- Make technical guidance easy to scan and act on. - -## Terminology +## Whatever Heading - **abbreviation**: A shortened form defined before repeated use. -## Style - -- Lead with the task outcome before implementation detail. +Approve when the change is safe to merge, not when it is perfect. ` doc, err := frames.UnmarshalMarkdown([]byte(src)) if err != nil { - t.Fatalf("spec example must convert, got: %v", err) + t.Fatalf("free-form body must convert, got: %v", err) } if doc.Scope != "department" || doc.Maintainer != "engineering enablement" { t.Errorf("metadata lost: %+v", doc) } - if len(doc.Slots.Terminology) != 1 || doc.Slots.Terminology[0].Term != "abbreviation" { - t.Errorf("terminology not parsed: %+v", doc.Slots.Terminology) + for _, want := range []string{ + "# Code Review Norms", + "Block on correctness, security, and data loss.", + "## Whatever Heading", + "- **abbreviation**: A shortened form defined before repeated use.", + "Approve when the change is safe to merge, not when it is perfect.", + } { + if !strings.Contains(doc.Body, want) { + t.Errorf("body missing %q:\n%s", want, doc.Body) + } } - if !strings.Contains(doc.Slots.Goals, "easy to scan") { - t.Errorf("goals not parsed: %q", doc.Slots.Goals) + if err := frames.Validate(doc); err != nil { + t.Errorf("doc should validate: %v", err) } +} - verr := frames.Validate(doc) - if verr == nil { - t.Fatal("expected validation errors for the unpinned bare inherits ref") +// The template flag has no Frame Spec equivalent, so it travels in the +// x- namespace and must survive a round trip. +func TestMarkdown_TemplateFlagRoundTrip(t *testing.T) { + doc := &frames.Doc{ + Name: "starter", Description: "d", Version: "1.0.0", + Visibility: "internal", Template: true, Body: "Guidance.", } - var ve *frames.ValidationError - if !errors.As(verr, &ve) { - t.Fatalf("expected *ValidationError, got %T", verr) + md, err := frames.MarshalMarkdown(doc) + if err != nil { + t.Fatalf("marshal: %v", err) } - wantPaths := map[string]bool{"extends[0].ref": false, "extends[0].version": false} - for _, fe := range ve.Errors { - if _, ok := wantPaths[fe.Path]; ok { - wantPaths[fe.Path] = true - } + if !strings.Contains(string(md), "x-nebari-template: true\n") { + t.Errorf("template flag not exported:\n%s", md) } - for path, seen := range wantPaths { - if !seen { - t.Errorf("expected a fixable field error on %s, got %v", path, ve.Errors) - } + back, err := frames.UnmarshalMarkdown(md) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !back.Template { + t.Error("template flag lost on import") + } +} + +func TestUnmarshalMarkdown_EmptyBody(t *testing.T) { + src := "---\ntype: frame [0.2]\nname: c\ndescription: d\nvisibility: internal\n---\n" + doc, err := frames.UnmarshalMarkdown([]byte(src)) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if doc.Body != "" { + t.Errorf("expected empty body, got %q", doc.Body) } } func TestUnmarshalMarkdown_StructuralErrors(t *testing.T) { - const head = "---\ntype: frame [0.2]\nname: c\ndescription: d\nvisibility: internal\n---\n\n" tests := []struct { name string src string @@ -222,13 +254,6 @@ func TestUnmarshalMarkdown_StructuralErrors(t *testing.T) { {"unknown frontmatter key", "---\ntype: frame [0.2]\nname: c\nowner: bob\n---\n", "unknown frontmatter key \"owner\""}, {"missing type", "---\nname: c\ndescription: d\nvisibility: internal\n---\n", "missing required frontmatter field \"type\""}, {"bad type", "---\ntype: skill\nname: c\ndescription: d\nvisibility: internal\n---\n", "type must be"}, - {"unknown section", head + "## Ways of Working\n\n- a\n", "did you mean \"## Norms\"?"}, - {"unknown section no hint", head + "## Escalation Path\n\n- a\n", "recognized sections are: Terminology"}, - {"wrong case section", head + "## goals\n\ntext\n", "did you mean \"## Goals\"?"}, - {"duplicate section", head + "## Goals\n\na\n\n## Goals\n\nb\n", "duplicate section"}, - {"content before section", head + "Loose prose.\n\n## Goals\n\na\n", "content before the first section heading"}, - {"malformed terminology", head + "## Terminology\n\n- customer is an org\n", "malformed terminology entry"}, - {"stray content in list", head + "## Rules\n\nnot a bullet\n", "unexpected content in \"## Rules\""}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -246,65 +271,108 @@ func TestUnmarshalMarkdown_StructuralErrors(t *testing.T) { } } -// Multi-line list items are the one shape naive bullet rendering breaks: the -// continuation lines must be indented on the way out and dedented on the way in. -func TestMarkdown_MultiLineListItem(t *testing.T) { - doc := &frames.Doc{ - Name: "c", Description: "d", Version: "1.0.0", Visibility: "internal", - Slots: frames.Slots{Rules: []string{ - "First line of the rule.\nSecond line.\n\nA new paragraph.", - "A single-line rule.", - }}, - } - md, err := frames.MarshalMarkdown(doc) - if err != nil { - t.Fatalf("marshal: %v", err) - } - if !strings.Contains(string(md), "- First line of the rule.\n Second line.\n\n A new paragraph.\n") { - t.Errorf("continuation lines not indented:\n%s", md) - } - back, err := frames.UnmarshalMarkdown(md) - if err != nil { - t.Fatalf("unmarshal: %v", err) - } - if !reflect.DeepEqual(doc.Slots.Rules, back.Slots.Rules) { - t.Errorf("multi-line rule lost.\nwant %q\ngot %q", doc.Slots.Rules, back.Slots.Rules) - } +// normalizeBody strips trailing newlines from the body. A YAML block scalar +// ("body: |") always ends with one, whereas the markdown form trims it. That +// whitespace carries no meaning in either form, so it is the one difference +// the round trip does not preserve, and both sides are normalized before +// comparison. +func normalizeBody(d *frames.Doc) { + d.Body = strings.TrimRight(d.Body, "\n") } -// Slot bodies may not contain their own "## " headings, since those delimit -// sections. Authors must use "###" or deeper; this locks in the diagnostic. -func TestUnmarshalMarkdown_H2InsideProse(t *testing.T) { - src := "---\ntype: frame [0.2]\nname: c\ndescription: d\nvisibility: internal\n---\n\n## Goals\n\nIntro.\n\n## Sub Goal\n\nMore.\n" - _, err := frames.UnmarshalMarkdown([]byte(src)) - if err == nil { - t.Fatal("expected an error for an unrecognized ## inside prose") - } - if !strings.Contains(err.Error(), "unknown section \"## Sub Goal\"") { - t.Errorf("unexpected error: %v", err) - } -} +// A --- inside a multi-line frontmatter value must not close the block. +// +// This is the one place where being lenient about the delimiter is a data-loss +// bug rather than a convenience: YAML block scalars are always indented, so an +// indented --- that terminated the frontmatter would truncate the document and +// silently drop every field after it. The codec does this to its own output - +// a description containing a --- line exports cleanly and reimports as a +// different, shorter document - so a round-trip case is included alongside the +// direct parses. +func TestUnmarshalMarkdown_FenceOnlyAtColumnZero(t *testing.T) { + head := "---\ntype: frame [0.2]\nname: c\nversion: 1.0.0\nvisibility: internal\n" -func TestUnmarshalMarkdown_H3InsideProseIsKept(t *testing.T) { - src := "---\ntype: frame [0.2]\nname: c\ndescription: d\nvisibility: internal\n---\n\n## Goals\n\n### Near term\n\nShip it.\n" - doc, err := frames.UnmarshalMarkdown([]byte(src)) - if err != nil { - t.Fatalf("unmarshal: %v", err) + tests := []struct { + name string + src string + + wantDesc string + wantVer string + wantBody string + }{ + { + name: "indented --- inside a block scalar is content", + src: head + "description: |-\n Line one\n ---\n Line two\n---\n\nBody.\n", + wantDesc: "Line one\n---\nLine two", + wantVer: "1.0.0", + wantBody: "Body.", + }, + { + name: "a block scalar before the required fields does not swallow them", + src: "---\ntype: frame [0.2]\ndescription: |-\n Line one\n ---\n Line two\n" + + "name: c\nversion: 1.0.0\nvisibility: internal\n---\n\nBody.\n", + wantDesc: "Line one\n---\nLine two", + wantVer: "1.0.0", + wantBody: "Body.", + }, + { + name: "a --- in the body stays in the body", + src: head + "description: d\n---\n\nBefore.\n\n---\n\nAfter.\n", + wantDesc: "d", + wantVer: "1.0.0", + wantBody: "Before.\n\n---\n\nAfter.", + }, + { + name: "trailing whitespace on the closing fence still closes", + src: head + "description: d\n--- \n\nBody.\n", + wantDesc: "d", + wantVer: "1.0.0", + wantBody: "Body.", + }, } - if doc.Slots.Goals != "### Near term\n\nShip it." { - t.Errorf("prose body not preserved: %q", doc.Slots.Goals) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + doc, err := frames.UnmarshalMarkdown([]byte(tc.src)) + if err != nil { + t.Fatalf("unmarshal: %v", err) + } + if doc.Description != tc.wantDesc { + t.Errorf("description = %q, want %q", doc.Description, tc.wantDesc) + } + if doc.Version != tc.wantVer { + t.Errorf("version = %q, want %q: a field after the block scalar was lost", doc.Version, tc.wantVer) + } + if doc.Body != tc.wantBody { + t.Errorf("body = %q, want %q", doc.Body, tc.wantBody) + } + }) } -} -// normalizeProse strips trailing newlines from prose slots. A YAML block scalar -// ("goals: |") always ends with one, whereas a markdown section is delimited by -// the next heading rather than by trailing whitespace. That whitespace carries -// no meaning in either form, so it is the one difference the round trip does not -// preserve, and both sides are normalized before comparison. -func normalizeProse(d *frames.Doc) { - for _, sd := range frames.SlotTable { - if sd.Kind == frames.SlotProse { - d.Slots.SetProse(sd.Key, strings.TrimRight(d.Slots.Prose(sd.Key), "\n")) + t.Run("a document the codec itself wrote survives its own round trip", func(t *testing.T) { + orig := &frames.Doc{ + Name: "c", + Description: "Line one\n---\nLine two", + Version: "1.0.0", + Visibility: "internal", + Maintainer: "platform team", + Body: "## Rules\n\n- be kind", } - } + md, err := frames.MarshalMarkdown(orig) + if err != nil { + t.Fatalf("marshal: %v", err) + } + got, err := frames.UnmarshalMarkdown(md) + if err != nil { + t.Fatalf("reparse: %v\n%s", err, md) + } + if !reflect.DeepEqual(got, orig) { + t.Errorf("round trip lost content\n got: %+v\nwant: %+v\nmarkdown:\n%s", got, orig, md) + } + // The failure this guards against was silent: the reparsed document was + // invalid for a field the author never touched. + if err := frames.Validate(got); err != nil { + t.Errorf("reparsed document does not validate: %v", err) + } + }) } diff --git a/backend/internal/frames/legacy.go b/backend/internal/frames/legacy.go new file mode 100644 index 0000000..65edd42 --- /dev/null +++ b/backend/internal/frames/legacy.go @@ -0,0 +1,89 @@ +package frames + +import ( + "fmt" + "strings" +) + +// This file exists only to read documents published before the schema was +// reduced to a single free-form body (Frame Spec v0.2 defines no body +// structure). Stored versions are immutable, so the old ten-slot YAML shape +// must stay readable forever; Parse folds it into Doc.Body via renderMarkdown. +// Nothing ever writes this shape again. + +// legacyTerm is a vocabulary entry from the retired terminology slot. +type legacyTerm struct { + Term string `yaml:"term"` + Definition string `yaml:"definition"` +} + +// legacySlots is the retired ten-slot content schema. +type legacySlots struct { + Terminology []legacyTerm `yaml:"terminology,omitempty"` + Rules []string `yaml:"rules,omitempty"` + Skills []string `yaml:"skills,omitempty"` + Prompts []string `yaml:"prompts,omitempty"` + ToolSpecs string `yaml:"tool_specs,omitempty"` + Goals string `yaml:"goals,omitempty"` + Style string `yaml:"style,omitempty"` + Norms string `yaml:"norms,omitempty"` + Architecture string `yaml:"architecture,omitempty"` + BusinessProcess string `yaml:"business_process,omitempty"` +} + +// renderMarkdown renders the legacy slots as the markdown sections the old +// .frame.md codec emitted, in the old schema order, so a legacy document reads +// the same as its historical export. +func (s *legacySlots) renderMarkdown() string { + var b strings.Builder + + if len(s.Terminology) > 0 { + b.WriteString("## Terminology\n\n") + for _, t := range s.Terminology { + writeLegacyBullet(&b, fmt.Sprintf("**%s**: %s", t.Term, t.Definition)) + } + b.WriteString("\n") + } + writeLegacyList := func(heading string, items []string) { + if len(items) == 0 { + return + } + fmt.Fprintf(&b, "## %s\n\n", heading) + for _, it := range items { + writeLegacyBullet(&b, it) + } + b.WriteString("\n") + } + writeLegacyList("Rules", s.Rules) + writeLegacyList("Skills", s.Skills) + writeLegacyList("Prompts", s.Prompts) + + writeProse := func(heading, body string) { + if strings.TrimSpace(body) == "" { + return + } + fmt.Fprintf(&b, "## %s\n\n%s\n\n", heading, strings.Trim(body, "\n")) + } + writeProse("Tool Specifications", s.ToolSpecs) + writeProse("Goals", s.Goals) + writeProse("Style", s.Style) + writeProse("Norms", s.Norms) + writeProse("Architecture", s.Architecture) + writeProse("Business Process", s.BusinessProcess) + + return strings.TrimRight(b.String(), "\n") +} + +// writeLegacyBullet renders one list item as a markdown bullet, indenting +// continuation lines two spaces so a multi-line item stays part of the item. +func writeLegacyBullet(b *strings.Builder, item string) { + lines := strings.Split(strings.Trim(item, "\n"), "\n") + fmt.Fprintf(b, "- %s\n", lines[0]) + for _, l := range lines[1:] { + if strings.TrimSpace(l) == "" { + b.WriteString("\n") + continue + } + fmt.Fprintf(b, " %s\n", l) + } +} diff --git a/backend/internal/frames/legacy_test.go b/backend/internal/frames/legacy_test.go new file mode 100644 index 0000000..709778a --- /dev/null +++ b/backend/internal/frames/legacy_test.go @@ -0,0 +1,47 @@ +package frames_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nebari-dev/nebari-frames/backend/internal/frames" +) + +// The legacy `slots:` renderer exists twice - here and in +// web/src/lib/frame-yaml.ts - because the web app renders a stored legacy +// version without a round trip to the server. That mirror is not display-only: +// restoring a legacy version re-serializes the rendered body as the new +// canonical content, so a divergence between the two rewrites what is stored. +// +// testdata/legacy-slots is the one fixture both sides are pinned to, and it is +// asserted whole rather than by substring. Substring assertions are what let +// the two implementations drift on continuation-line indentation and on +// newline-only versus whitespace trimming while both suites stayed green. +// +// The web-side assertion lives in web/src/lib/frame-yaml.test.ts. Changing the +// rendering means regenerating expected.md and updating both. +func TestLegacySlots_SharedFixture(t *testing.T) { + dir := filepath.Join("..", "..", "..", "testdata", "legacy-slots") + + in, err := os.ReadFile(filepath.Join(dir, "input.yaml")) + if err != nil { + t.Fatalf("read fixture input: %v", err) + } + wantBytes, err := os.ReadFile(filepath.Join(dir, "expected.md")) + if err != nil { + t.Fatalf("read fixture expectation: %v", err) + } + // expected.md carries a trailing newline so it is a well-formed text file; + // the rendered body does not. + want := strings.TrimSuffix(string(wantBytes), "\n") + + doc, err := frames.Parse(in) + if err != nil { + t.Fatalf("parse fixture: %v", err) + } + if doc.Body != want { + t.Errorf("rendered body does not match the shared fixture\n--- got ---\n%s\n--- want ---\n%s", doc.Body, want) + } +} diff --git a/backend/internal/frames/resolver.go b/backend/internal/frames/resolver.go index abefdf2..46dd2bd 100644 --- a/backend/internal/frames/resolver.go +++ b/backend/internal/frames/resolver.go @@ -20,8 +20,20 @@ type ParentFetcher interface { FetchParent(ctx context.Context, ref, version string) (doc *Doc, extends []ExtendRef, excludes []string, err error) } -// Resolve merges the extends graph of doc (later parents win; doc wins last), -// honoring excludes. It detects cycles and propagates unreadable-ancestor errors. +// Resolve merges the extends graph of doc, honoring excludes. Bodies are +// concatenated in merge order - ancestors first, the doc's own body last - so +// the resolved body reads from the most general context to the most specific, +// and later guidance overrides earlier guidance for a reader. It detects cycles +// and propagates unreadable-ancestor errors. +// +// Reading order is the whole precedence model, and that is a deliberate trade +// rather than an omission: a free-form body has no addressable sections to +// override, so a child appends to its parents and `excludes` operates on a whole +// ancestor. What that costs - a child can no longer redefine a single term or +// replace a single prose section - and why the alternative would rebuild the +// retired slot schema is recorded in +// docs/design/2026-05-21-nebari-frames-migration.md, §3.4, under "Why precedence +// is reading order, and what that costs". func Resolve(ctx context.Context, fetcher ParentFetcher, doc *Doc, extends []ExtendRef, excludes []string) (*Doc, error) { excludeSet := map[string]bool{} for _, e := range excludes { @@ -34,14 +46,15 @@ func Resolve(ctx context.Context, fetcher ParentFetcher, doc *Doc, extends []Ext Visibility: doc.Visibility, Scope: doc.Scope, Maintainer: doc.Maintainer, } visiting := map[string]bool{} - if err := mergeParents(ctx, fetcher, extends, excludeSet, acc, visiting, []string{doc.Name}); err != nil { + merged := map[string]bool{} + if err := mergeParents(ctx, fetcher, extends, excludeSet, acc, visiting, merged, []string{doc.Name}); err != nil { return nil, err } - mergeInto(acc, doc) // doc's own slots override all parents + appendBody(acc, doc.Body) // doc's own body comes last return acc, nil } -func mergeParents(ctx context.Context, fetcher ParentFetcher, parents []ExtendRef, excludeSet map[string]bool, acc *Doc, visiting map[string]bool, path []string) error { +func mergeParents(ctx context.Context, fetcher ParentFetcher, parents []ExtendRef, excludeSet map[string]bool, acc *Doc, visiting, merged map[string]bool, path []string) error { for _, p := range parents { if excludeSet[p.Ref] { continue @@ -62,67 +75,30 @@ func mergeParents(ctx context.Context, fetcher ParentFetcher, parents []ExtendRe for _, e := range pexcludes { childExcludes[e] = true } - if err := mergeParents(ctx, fetcher, pextends, childExcludes, acc, visiting, append(path, p.Ref)); err != nil { + if err := mergeParents(ctx, fetcher, pextends, childExcludes, acc, visiting, merged, append(path, p.Ref)); err != nil { return err } - mergeInto(acc, pdoc) + // A parent reachable through more than one path (a diamond) contributes + // its body exactly once. + if !merged[key] { + merged[key] = true + appendBody(acc, pdoc.Body) + } delete(visiting, key) } return nil } -// mergeInto applies src's slots onto dst (src wins). -func mergeInto(dst, src *Doc) { - dst.Slots.Terminology = mergeTerms(dst.Slots.Terminology, src.Slots.Terminology) - dst.Slots.Rules = mergeStrings(dst.Slots.Rules, src.Slots.Rules) - dst.Slots.Skills = mergeStrings(dst.Slots.Skills, src.Slots.Skills) - dst.Slots.Prompts = mergeStrings(dst.Slots.Prompts, src.Slots.Prompts) - dst.Slots.ToolSpecs = replaceIfSet(dst.Slots.ToolSpecs, src.Slots.ToolSpecs) - dst.Slots.Goals = replaceIfSet(dst.Slots.Goals, src.Slots.Goals) - dst.Slots.Style = replaceIfSet(dst.Slots.Style, src.Slots.Style) - dst.Slots.Norms = replaceIfSet(dst.Slots.Norms, src.Slots.Norms) - dst.Slots.Architecture = replaceIfSet(dst.Slots.Architecture, src.Slots.Architecture) - dst.Slots.BusinessProcess = replaceIfSet(dst.Slots.BusinessProcess, src.Slots.BusinessProcess) -} - -// mergeTerms merges by term; src definition wins on collision; order = existing then new. -func mergeTerms(existing, incoming []Term) []Term { - idx := map[string]int{} - out := make([]Term, 0, len(existing)+len(incoming)) - for _, t := range existing { - idx[t.Term] = len(out) - out = append(out, t) +// appendBody appends a contribution to the accumulated body, separated by a +// blank line. Empty contributions are skipped. +func appendBody(dst *Doc, body string) { + body = strings.Trim(body, "\n") + if strings.TrimSpace(body) == "" { + return } - for _, t := range incoming { - if i, ok := idx[t.Term]; ok { - out[i].Definition = t.Definition - continue - } - idx[t.Term] = len(out) - out = append(out, t) - } - return out -} - -// mergeStrings concatenates then dedupes preserving the LAST occurrence. -func mergeStrings(existing, incoming []string) []string { - combined := append(append([]string{}, existing...), incoming...) - lastIndex := map[string]int{} - for i, s := range combined { - lastIndex[s] = i - } - out := make([]string, 0, len(combined)) - for i, s := range combined { - if lastIndex[s] == i { - out = append(out, s) - } - } - return out -} - -func replaceIfSet(existing, incoming string) string { - if incoming != "" { - return incoming + if dst.Body == "" { + dst.Body = body + return } - return existing + dst.Body += "\n\n" + body } diff --git a/backend/internal/frames/resolver_test.go b/backend/internal/frames/resolver_test.go index bc72180..93b0d2a 100644 --- a/backend/internal/frames/resolver_test.go +++ b/backend/internal/frames/resolver_test.go @@ -32,46 +32,99 @@ func (f fakeFetcher) FetchParent(_ context.Context, ref, version string) (*frame return d, d.Extends, d.Excludes, nil } -func TestResolve_MergeOrderAndOverride(t *testing.T) { - parent := &frames.Doc{Name: "base", Version: "1.0.0"} - parent.Slots.Rules = []string{"rule-a", "shared"} - parent.Slots.Terminology = []frames.Term{{Term: "x", Definition: "from-parent"}} - parent.Slots.Goals = "parent goals" +func TestResolve_BodiesConcatenateInMergeOrder(t *testing.T) { + grandparent := &frames.Doc{Name: "root", Version: "1.0.0", Body: "root guidance"} + parent := &frames.Doc{ + Name: "base", Version: "1.0.0", Body: "base guidance", + Extends: []frames.ExtendRef{{Ref: "org/root", Version: "1.0.0"}}, + } + child := &frames.Doc{ + Name: "child", Version: "1.0.0", Body: "child guidance", + Extends: []frames.ExtendRef{{Ref: "org/base", Version: "1.0.0"}}, + } - child := &frames.Doc{Name: "child", Version: "1.0.0", Extends: []frames.ExtendRef{{Ref: "org/base", Version: "1.0.0"}}} - child.Slots.Rules = []string{"shared", "rule-b"} - child.Slots.Terminology = []frames.Term{{Term: "x", Definition: "from-child"}} + f := newFakeFetcher(map[string]*frames.Doc{ + "org/root@1.0.0": grandparent, + "org/base@1.0.0": parent, + }) + got, err := frames.Resolve(context.Background(), f, child, child.Extends, child.Excludes) + if err != nil { + t.Fatalf("resolve: %v", err) + } + want := "root guidance\n\nbase guidance\n\nchild guidance" + if got.Body != want { + t.Errorf("body = %q, want %q", got.Body, want) + } +} +func TestResolve_EmptyBodiesAreSkipped(t *testing.T) { + parent := &frames.Doc{Name: "base", Version: "1.0.0", Body: "parent guidance"} + child := &frames.Doc{ + Name: "child", Version: "1.0.0", Body: "", + Extends: []frames.ExtendRef{{Ref: "org/base", Version: "1.0.0"}}, + } f := newFakeFetcher(map[string]*frames.Doc{"org/base@1.0.0": parent}) got, err := frames.Resolve(context.Background(), f, child, child.Extends, child.Excludes) if err != nil { t.Fatalf("resolve: %v", err) } + if got.Body != "parent guidance" { + t.Errorf("body = %q, want parent body only with no separator", got.Body) + } +} - t.Run("rules dedupe keeping last occurrence", func(t *testing.T) { - // rules: concat parent then child, dedupe keeping last occurrence -> [rule-a, shared, rule-b] - want := []string{"rule-a", "shared", "rule-b"} - if len(got.Slots.Rules) != len(want) { - t.Fatalf("rules = %v, want %v", got.Slots.Rules, want) - } - for i := range want { - if got.Slots.Rules[i] != want[i] { - t.Fatalf("rules[%d] = %q, want %q (full: %v, want %v)", i, got.Slots.Rules[i], want[i], got.Slots.Rules, want) - } - } - }) - - t.Run("terminology child overrides parent", func(t *testing.T) { - if got.Slots.Terminology[0].Definition != "from-child" { - t.Fatalf("terminology override failed: %v", got.Slots.Terminology) - } +// A parent reachable through more than one path (a diamond) must contribute +// its body exactly once. +func TestResolve_DiamondParentMergedOnce(t *testing.T) { + shared := &frames.Doc{Name: "shared", Version: "1", Body: "shared guidance"} + left := &frames.Doc{ + Name: "left", Version: "1", Body: "left guidance", + Extends: []frames.ExtendRef{{Ref: "org/shared", Version: "1"}}, + } + right := &frames.Doc{ + Name: "right", Version: "1", Body: "right guidance", + Extends: []frames.ExtendRef{{Ref: "org/shared", Version: "1"}}, + } + child := &frames.Doc{ + Name: "child", Version: "1", Body: "child guidance", + Extends: []frames.ExtendRef{ + {Ref: "org/left", Version: "1"}, + {Ref: "org/right", Version: "1"}, + }, + } + f := newFakeFetcher(map[string]*frames.Doc{ + "org/shared@1": shared, "org/left@1": left, "org/right@1": right, }) + got, err := frames.Resolve(context.Background(), f, child, child.Extends, child.Excludes) + if err != nil { + t.Fatalf("resolve: %v", err) + } + want := "shared guidance\n\nleft guidance\n\nright guidance\n\nchild guidance" + if got.Body != want { + t.Errorf("body = %q, want %q", got.Body, want) + } +} - t.Run("prose flows through from parent when child has none", func(t *testing.T) { - if got.Slots.Goals != "parent goals" { - t.Fatalf("goals = %q, want parent goals", got.Slots.Goals) - } - }) +// Spec metadata describes the child itself and is never inherited. +func TestResolve_MetadataCarriedFromChild(t *testing.T) { + parent := &frames.Doc{ + Name: "base", Version: "9.9.9", Visibility: "public", + Scope: "company", Maintainer: "platform", Body: "parent guidance", + } + child := &frames.Doc{ + Name: "child", Description: "child desc", Version: "1.0.0", + Visibility: "private", Scope: "project", Maintainer: "data science", + Extends: []frames.ExtendRef{{Ref: "org/base", Version: "9.9.9"}}, + } + f := newFakeFetcher(map[string]*frames.Doc{"org/base@9.9.9": parent}) + got, err := frames.Resolve(context.Background(), f, child, child.Extends, child.Excludes) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if got.Name != "child" || got.Description != "child desc" || got.Version != "1.0.0" || + got.Visibility != "private" || got.Scope != "project" || got.Maintainer != "data science" { + t.Errorf("child metadata not carried through: %+v", got) + } } func TestResolve_CycleDetected(t *testing.T) { @@ -86,10 +139,9 @@ func TestResolve_CycleDetected(t *testing.T) { } func TestResolve_Excludes(t *testing.T) { - parent := &frames.Doc{Name: "base", Version: "1"} - parent.Slots.Rules = []string{"excluded-rule"} + parent := &frames.Doc{Name: "base", Version: "1", Body: "excluded guidance"} child := &frames.Doc{ - Name: "child", Version: "1", + Name: "child", Version: "1", Body: "child guidance", Extends: []frames.ExtendRef{{Ref: "org/base", Version: "1"}}, Excludes: []string{"org/base"}, } @@ -98,8 +150,8 @@ func TestResolve_Excludes(t *testing.T) { if err != nil { t.Fatalf("resolve: %v", err) } - if len(got.Slots.Rules) != 0 { - t.Fatalf("excluded parent rules leaked: %v", got.Slots.Rules) + if got.Body != "child guidance" { + t.Fatalf("excluded parent body leaked: %q", got.Body) } } diff --git a/backend/internal/frames/schema.go b/backend/internal/frames/schema.go index fad3193..7fd0f15 100644 --- a/backend/internal/frames/schema.go +++ b/backend/internal/frames/schema.go @@ -5,6 +5,8 @@ package frames import ( "bytes" "fmt" + "regexp" + "strings" "gopkg.in/yaml.v3" ) @@ -15,28 +17,12 @@ type ExtendRef struct { Version string `yaml:"version"` } -// Term is a single vocabulary entry in the terminology slot. -type Term struct { - Term string `yaml:"term"` - Definition string `yaml:"definition"` -} - -// Slots holds the ten content slots defined by the Frame schema. -type Slots struct { - Terminology []Term `yaml:"terminology,omitempty"` - Rules []string `yaml:"rules,omitempty"` - Skills []string `yaml:"skills,omitempty"` - Prompts []string `yaml:"prompts,omitempty"` - ToolSpecs string `yaml:"tool_specs,omitempty"` - Goals string `yaml:"goals,omitempty"` - Style string `yaml:"style,omitempty"` - Norms string `yaml:"norms,omitempty"` - Architecture string `yaml:"architecture,omitempty"` - BusinessProcess string `yaml:"business_process,omitempty"` -} - // Doc is the parsed representation of a Frame YAML document. // +// The content of a Frame is a single free-form markdown Body, matching Frame +// Spec v0.2: the spec requires four frontmatter fields and defines no body +// structure at all. +// // Visibility, Scope, and Maintainer carry the Frame Spec v0.2 metadata fields // through the canonical form so the .frame.md codec round-trips them. They are // optional: documents published before these fields existed decode with the @@ -51,19 +37,60 @@ type Doc struct { Maintainer string `yaml:"maintainer,omitempty"` Extends []ExtendRef `yaml:"extends,omitempty"` Excludes []string `yaml:"excludes,omitempty"` - Slots Slots `yaml:"slots"` + // Template marks this Frame as a starting point offered by the authoring + // UI's template picker. Registry metadata, not spec metadata: it exports + // as x-nebari-template in the .frame.md form. + Template bool `yaml:"template,omitempty"` + Body string `yaml:"body,omitempty"` +} + +// docYAML is the on-disk decode shape. It accepts the legacy `slots:` key so +// documents published before the free-form body still parse; see legacy.go. +type docYAML struct { + Doc `yaml:",inline"` + Slots *legacySlots `yaml:"slots,omitempty"` } +// docKeys is the recognized top-level key set, named in unknown-key errors. +// Keep in sync with Doc and docYAML. +var docKeys = []string{ + "name", "description", "version", "visibility", "scope", + "maintainer", "extends", "excludes", "template", "body", +} + +// unknownFieldRe extracts the offending key from a yaml.v3 KnownFields error. +// The message yaml.v3 produces names the Go type it was decoding into, which +// here is the unexported docYAML - meaningless to an API client, and this error +// reaches one unwrapped through the publish and convert endpoints. +var unknownFieldRe = regexp.MustCompile(`field (\S+) not found in type \S+`) + // Parse decodes YAML content into a Doc. Unknown keys are rejected because the -// Frame schema is fixed and not extensible. +// Frame schema is fixed and not extensible. A legacy `slots:` block is folded +// into Body so versions published under the slot schema remain readable. func Parse(content []byte) (*Doc, error) { - var d Doc + var d docYAML dec := yaml.NewDecoder(newReader(content)) - dec.KnownFields(true) // reject unknown top-level/slot keys: schema is fixed + dec.KnownFields(true) // reject unknown top-level keys: schema is fixed if err := dec.Decode(&d); err != nil { - return nil, fmt.Errorf("parse frame yaml: %w", err) + return nil, fmt.Errorf("parse frame yaml: %s", parseErr(err)) } - return &d, nil + // A document carrying both keys is a legacy version someone has edited. The + // explicit body wins: `slots:` is only ever a fallback rendering of content + // nothing writes any more, so preferring it would discard the deliberate + // edit in favour of a reconstruction. + if d.Slots != nil && d.Body == "" { + d.Body = d.Slots.renderMarkdown() + } + return &d.Doc, nil +} + +// parseErr replaces the internal decode type in a yaml.v3 unknown-key error +// with the recognized key set, since the raw message reaches API clients. +func parseErr(err error) string { + msg := strings.TrimPrefix(err.Error(), "yaml: ") + msg = strings.ReplaceAll(msg, "unmarshal errors:\n ", "") + return unknownFieldRe.ReplaceAllString(msg, + `unknown field $1 - recognized fields are: `+strings.Join(docKeys, ", ")) } // Marshal encodes a Doc to YAML. diff --git a/backend/internal/frames/service.go b/backend/internal/frames/service.go index 8df6a8b..28f15db 100644 --- a/backend/internal/frames/service.go +++ b/backend/internal/frames/service.go @@ -213,6 +213,7 @@ func (s *Service) publish(ctx context.Context, caller rbac.Caller, doc *Doc, con frame = &framesv1.Frame{ Id: newID(), OrgId: caller.OrgID, Name: doc.Name, Description: doc.Description, OwnerSub: caller.Subject, LatestVersion: doc.Version, CreatedAt: now, UpdatedAt: now, + IsTemplate: doc.Template, } } else { // editing an existing frame requires edit permission @@ -248,6 +249,7 @@ func (s *Service) publish(ctx context.Context, caller rbac.Caller, doc *Doc, con frame.Description = doc.Description frame.LatestVersion = doc.Version frame.UpdatedAt = now + frame.IsTemplate = doc.Template } edges, err := s.resolveEdges(ctx, caller, org.Slug, doc.Extends) @@ -333,7 +335,7 @@ func (s *Service) ListFrames(ctx context.Context, _ *connect.Request[framesv1.Li } resp.Frames = append(resp.Frames, &framesv1.FrameSummary{ OrgSlug: org.Slug, Name: f.Name, Description: f.Description, OwnerSub: f.OwnerSub, - LatestVersion: f.LatestVersion, UpdatedAt: f.UpdatedAt, + LatestVersion: f.LatestVersion, UpdatedAt: f.UpdatedAt, IsTemplate: f.IsTemplate, Permissions: &framesv1.Permissions{CanEdit: canEdit, CanDelete: canDelete}, }) } @@ -641,8 +643,8 @@ func (f *readFetcher) FetchParent(ctx context.Context, ref, version string) (*Do // violationErr maps a *ValidationError onto an InvalidArgument Connect error // carrying FieldViolations, so a client can attach each failure to the input // that caused it. Shared by PublishFrame (value errors, paths like -// "slots.terminology[2].definition") and ConvertFrame (markdown structure -// errors, path "markdown"). +// "extends[0].version") and ConvertFrame (markdown structure errors, path +// "markdown"). func violationErr(err error) *connect.Error { cerr := connect.NewError(connect.CodeInvalidArgument, err) var ve *ValidationError @@ -660,7 +662,7 @@ func violationErr(err error) *connect.Error { return cerr } -// ConvertFrame translates between the canonical slot YAML and the .frame.md +// ConvertFrame translates between the canonical YAML and the .frame.md // interchange format defined by Frame Spec v0.2. It is a pure function of its // input - it touches no storage - so it requires only that the caller is a // member of an org. It backs the web app's markdown editor, import, and export. diff --git a/backend/internal/frames/service_test.go b/backend/internal/frames/service_test.go index 3152d93..97d4cb4 100644 --- a/backend/internal/frames/service_test.go +++ b/backend/internal/frames/service_test.go @@ -37,9 +37,8 @@ func seedSecondOrg(t *testing.T, repo *store.Memory, sub, role string) context.C const sampleFrame = `name: brand-voice description: OpenTeams brand voice version: 1.0.0 -slots: - rules: - - Cite benchmarks. +body: | + Cite benchmarks. ` func TestService_PublishThenGet(t *testing.T) { @@ -61,6 +60,51 @@ func TestService_PublishThenGet(t *testing.T) { } } +// The doc's `template` field denormalizes onto the frame record at publish +// time, in both directions, so template pickers can list without parsing +// content blobs. +func TestService_TemplateFlagDenormalized(t *testing.T) { + const asTemplate = `name: starter +description: A starting point +version: 1.0.0 +template: true +body: | + Guidance. +` + const notTemplate = `name: starter +description: A starting point +version: 1.1.0 +body: | + Guidance. +` + repo := store.NewMemory() + pubCtx := seedOrg(t, repo, "pub", "publisher") + svc := frames.NewService(repo) + + if _, err := svc.PublishFrame(pubCtx, connect.NewRequest(&framesv1.PublishFrameRequest{Content: []byte(asTemplate)})); err != nil { + t.Fatalf("publish: %v", err) + } + list, err := svc.ListFrames(pubCtx, connect.NewRequest(&framesv1.ListFramesRequest{})) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(list.Msg.Frames) != 1 || !list.Msg.Frames[0].IsTemplate { + t.Fatalf("expected the frame to list as a template: %+v", list.Msg.Frames) + } + + // Publishing a new version without the flag clears it. + if _, err := svc.PublishFrame(pubCtx, connect.NewRequest(&framesv1.PublishFrameRequest{Content: []byte(notTemplate)})); err != nil { + t.Fatalf("publish v1.1.0: %v", err) + } + got, err := svc.GetFrame(pubCtx, connect.NewRequest(&framesv1.GetFrameRequest{OrgSlug: "openteams", Name: "starter"})) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.Msg.Frame.IsTemplate { + t.Error("template flag should clear when the latest version drops it") + } +} + func TestService_ViewerCannotPublish(t *testing.T) { repo := store.NewMemory() viewerCtx := seedOrg(t, repo, "v", "viewer") @@ -92,9 +136,8 @@ func TestService_CrossOrgGetIs404(t *testing.T) { const parentFrame = `name: base-voice description: Base voice frame version: 1.0.0 -slots: - rules: - - Always cite sources. +body: | + Always cite sources. ` const childWithSameOrgRef = `name: brand-voice @@ -103,14 +146,13 @@ version: 1.0.0 extends: - ref: openteams/base-voice version: 1.0.0 -slots: - rules: - - Cite benchmarks. +body: | + Cite benchmarks. ` // TestService_ResolveSameOrgParent verifies that a child frame extending a // same-org parent resolves successfully and pulls in the parent's contributed -// slot content. The readFetcher resolves each parent ref against the caller's +// body content. The readFetcher resolves each parent ref against the caller's // org slug (mirroring PublishFrame); previously it used an empty fallback org // slug, a latent break for any same-org ref that omits the slug prefix. // @@ -135,13 +177,13 @@ func TestService_ResolveSameOrgParent(t *testing.T) { if err != nil { t.Fatalf("resolve: %v", err) } - // The resolved YAML must include the parent's contributed rule. + // The resolved YAML must include the parent's contributed guidance. if !bytes.Contains(resp.Msg.ResolvedContent, []byte("Always cite sources.")) { - t.Fatalf("resolved content missing parent rule; got:\n%s", resp.Msg.ResolvedContent) + t.Fatalf("resolved content missing parent guidance; got:\n%s", resp.Msg.ResolvedContent) } - // And the child's own rule. + // And the child's own guidance. if !bytes.Contains(resp.Msg.ResolvedContent, []byte("Cite benchmarks.")) { - t.Fatalf("resolved content missing child rule; got:\n%s", resp.Msg.ResolvedContent) + t.Fatalf("resolved content missing child guidance; got:\n%s", resp.Msg.ResolvedContent) } } @@ -154,9 +196,8 @@ func TestService_CrossOrgParentReadEnforcement(t *testing.T) { const secretFrameYAML = `name: secret description: Secret frame for org B version: 1.0.0 -slots: - rules: - - Internal only. +body: | + Internal only. ` // childExtending builds a publishable child frame YAML that extends the // given fully-qualified ref (e.g. "acme/secret") at version 1.0.0. @@ -167,9 +208,8 @@ version: 1.0.0 extends: - ref: ` + ref + ` version: 1.0.0 -slots: - rules: - - Some rule. +body: | + Some rule. `) } @@ -218,17 +258,14 @@ func TestListFrameVersions(t *testing.T) { const v1Frame = `name: brand-voice description: OpenTeams brand voice version: 1.0.0 -slots: - rules: - - Cite benchmarks. +body: | + Cite benchmarks. ` const v2Frame = `name: brand-voice description: OpenTeams brand voice version: 1.1.0 -slots: - rules: - - Cite benchmarks. - - Use data. +body: | + Cite benchmarks. Use data. ` tests := []struct { name string @@ -294,13 +331,13 @@ func TestService_GetMeReportsRole(t *testing.T) { } func TestPublishFrame_ValidationErrorDetail(t *testing.T) { - // An invalid doc: bad name (uppercase), empty description, empty version. + // An invalid doc: bad name (uppercase), empty description, empty version, + // unpinned extends ref. const badFrame = `name: Bad_Name description: "" version: "" -slots: - rules: - - "" +extends: + - ref: noslash ` tests := []struct { name string @@ -309,7 +346,7 @@ slots: {name: "bad name reported", wantField: "name"}, {name: "empty description reported", wantField: "description"}, {name: "empty version reported", wantField: "version"}, - {name: "empty rule reported", wantField: "slots.rules[0]"}, + {name: "bad extends ref reported", wantField: "extends[0].ref"}, } repo := store.NewMemory() @@ -365,7 +402,7 @@ func seedReadableFrameDirect(t *testing.T, repo *store.Memory, ctx context.Conte }, Version: &framesv1.FrameVersion{ Version: "1.0.0", - Content: []byte("name: alpha\ndescription: A\nversion: 1.0.0\nslots:\n rules:\n - r1\n"), + Content: []byte("name: alpha\ndescription: A\nversion: 1.0.0\nbody: |\n r1\n"), PublishedAt: timestamppb.Now(), }, Grants: []store.Grant{{SubjectType: "org", SubjectID: "o1", Permission: "read"}}, @@ -387,7 +424,7 @@ func seedUnreadableFrameDirect(t *testing.T, repo *store.Memory, ctx context.Con }, Version: &framesv1.FrameVersion{ Version: "1.0.0", - Content: []byte("name: secret\ndescription: S\nversion: 1.0.0\nslots:\n rules:\n - hidden\n"), + Content: []byte("name: secret\ndescription: S\nversion: 1.0.0\nbody: |\n hidden\n"), PublishedAt: timestamppb.Now(), }, Grants: []store.Grant{{SubjectType: "user", SubjectID: "someone-else", Permission: "read"}}, @@ -450,9 +487,10 @@ func TestConvertFrame_YamlToMarkdown(t *testing.T) { description: Voice guardrails. version: 1.0.0 visibility: internal -slots: - rules: - - Cite benchmarks. +body: | + ## Rules + + - Cite benchmarks. ` repo := store.NewMemory() ctx := seedOrg(t, repo, "pub", "publisher") @@ -520,11 +558,10 @@ type: frame [0.2] name: c description: d visibility: internal +owner: bob --- -## Ways of Working - -- something +Some guidance. ` repo := store.NewMemory() ctx := seedOrg(t, repo, "pub", "publisher") @@ -554,7 +591,7 @@ visibility: internal continue } for _, v := range fv.Violations { - if v.Field == "markdown" && strings.Contains(v.Message, "did you mean \"## Norms\"?") { + if v.Field == "markdown" && strings.Contains(v.Message, "unknown frontmatter key \"owner\"") { found = true } } @@ -570,21 +607,25 @@ func TestConvertFrame_RequiresMembership(t *testing.T) { repo := store.NewMemory() svc := frames.NewService(repo) _, err := svc.ConvertFrame(context.Background(), connect.NewRequest(&framesv1.ConvertFrameRequest{ - Source: &framesv1.ConvertFrameRequest_Yaml{Yaml: []byte("name: c\ndescription: d\nversion: 1.0.0\nslots: {}\n")}, + Source: &framesv1.ConvertFrameRequest_Yaml{Yaml: []byte("name: c\ndescription: d\nversion: 1.0.0\nbody: text\n")}, })) if err == nil { t.Fatal("want error for a caller with no org membership") } } -// docFor builds a minimal valid Doc for PublishDoc tests. +// docFor builds a minimal valid Doc for PublishDoc tests. Any rules supplied +// become bullets in the free-form body, which is all a Frame's content is now. func docFor(name, version string, rules ...string) *frames.Doc { - return &frames.Doc{ + d := &frames.Doc{ Name: name, Description: name + " description", Version: version, - Slots: frames.Slots{Rules: rules}, } + if len(rules) > 0 { + d.Body = "## Rules\n\n- " + strings.Join(rules, "\n- ") + } + return d } func TestService_PublishDoc(t *testing.T) { @@ -792,8 +833,8 @@ slots: if err != nil { t.Fatalf("SourceDoc: %v", err) } - if got := src.Slots.Rules; len(got) != 1 || got[0] != "from child" { - t.Errorf("rules = %v, want only the child's own rule (parent content must not be merged in)", got) + if !strings.Contains(src.Body, "from child") || strings.Contains(src.Body, "from parent") { + t.Errorf("body = %q, want only the child's own rule (parent content must not be merged in)", src.Body) } if len(src.Extends) != 1 || src.Extends[0].Ref != "openteams/base" { t.Errorf("extends = %+v, want the child's own pinned parent", src.Extends) @@ -808,8 +849,8 @@ slots: if err != nil { t.Fatalf("ResolveDoc: %v", err) } - if len(resolved.Slots.Rules) != 2 { - t.Errorf("resolved rules = %v, want both parent and child rules", resolved.Slots.Rules) + if !strings.Contains(resolved.Body, "from parent") || !strings.Contains(resolved.Body, "from child") { + t.Errorf("resolved body = %q, want both parent and child rules", resolved.Body) } } @@ -837,7 +878,7 @@ func TestService_PublishRejectsOversizedContent(t *testing.T) { repo := store.NewMemory() ctx := seedOrg(t, repo, "pub", "publisher") svc := frames.NewService(repo) - content := "name: big\ndescription: d\nversion: 1.0.0\nslots:\n goals: " + huge + "\n" + content := "name: big\ndescription: d\nversion: 1.0.0\nbody: " + huge + "\n" _, err := svc.PublishFrame(ctx, connect.NewRequest(&framesv1.PublishFrameRequest{Content: []byte(content)})) if connect.CodeOf(err) != connect.CodeInvalidArgument { t.Errorf("code = %v (err %v), want InvalidArgument", connect.CodeOf(err), err) @@ -849,7 +890,7 @@ func TestService_PublishRejectsOversizedContent(t *testing.T) { ctx := seedOrg(t, repo, "pub", "publisher") svc := frames.NewService(repo) doc := docFor("big", "1.0.0") - doc.Slots.Goals = huge + doc.Body = huge _, _, err := svc.PublishDoc(ctx, doc, "", frames.PublishCreate) if connect.CodeOf(err) != connect.CodeInvalidArgument { t.Errorf("code = %v (err %v), want InvalidArgument", connect.CodeOf(err), err) @@ -863,7 +904,7 @@ func TestService_PublishRejectsOversizedContent(t *testing.T) { // exactly the limit; YAML framing makes the offset awkward to hardcode. sizeFor := func(pad int) int { d := docFor("ok", "1.0.0") - d.Slots.Goals = strings.Repeat("y", pad) + d.Body = strings.Repeat("y", pad) b, err := frames.Marshal(d) if err != nil { t.Fatalf("marshal: %v", err) @@ -884,7 +925,7 @@ func TestService_PublishRejectsOversizedContent(t *testing.T) { } atLimit := docFor("ok", "1.0.0") - atLimit.Slots.Goals = strings.Repeat("y", lo) + atLimit.Body = strings.Repeat("y", lo) repo := store.NewMemory() ctx := seedOrg(t, repo, "pub", "publisher") if _, _, err := frames.NewService(repo).PublishDoc(ctx, atLimit, "", frames.PublishCreate); err != nil { @@ -892,7 +933,7 @@ func TestService_PublishRejectsOversizedContent(t *testing.T) { } over := docFor("ok", "1.0.0") - over.Slots.Goals = strings.Repeat("y", lo+1) + over.Body = strings.Repeat("y", lo+1) repo2 := store.NewMemory() ctx2 := seedOrg(t, repo2, "pub", "publisher") _, _, err := frames.NewService(repo2).PublishDoc(ctx2, over, "", frames.PublishCreate) diff --git a/backend/internal/frames/slots.go b/backend/internal/frames/slots.go deleted file mode 100644 index 9308007..0000000 --- a/backend/internal/frames/slots.go +++ /dev/null @@ -1,119 +0,0 @@ -package frames - -// SlotKind classifies the shape of a slot's content, which determines how the -// slot is rendered to and parsed back from the markdown (.frame.md) form. -type SlotKind int - -const ( - // SlotTerms is a []Term slot rendered as "- **term**: definition" bullets. - SlotTerms SlotKind = iota - // SlotList is a []string slot rendered as plain "- item" bullets. - SlotList - // SlotProse is a string slot rendered as a raw markdown body. - SlotProse -) - -// SlotDescriptor names one slot of the fixed schema. -type SlotDescriptor struct { - Key string // key under `slots:` in the canonical YAML - Heading string // canonical "## " heading in the markdown form - Kind SlotKind -} - -// SlotTable is the canonical slot list in schema order. It is the single source -// of truth for slot keys, markdown headings, and content shape: the .frame.md -// codec and the MCP markdown composer both read it, so a slot is added or -// renamed in exactly one place. -var SlotTable = []SlotDescriptor{ - {Key: "terminology", Heading: "Terminology", Kind: SlotTerms}, - {Key: "rules", Heading: "Rules", Kind: SlotList}, - {Key: "skills", Heading: "Skills", Kind: SlotList}, - {Key: "prompts", Heading: "Prompts", Kind: SlotList}, - {Key: "tool_specs", Heading: "Tool Specifications", Kind: SlotProse}, - {Key: "goals", Heading: "Goals", Kind: SlotProse}, - {Key: "style", Heading: "Style", Kind: SlotProse}, - {Key: "norms", Heading: "Norms", Kind: SlotProse}, - {Key: "architecture", Heading: "Architecture", Kind: SlotProse}, - {Key: "business_process", Heading: "Business Process", Kind: SlotProse}, -} - -// SlotByHeading resolves a markdown "## " heading to its descriptor. -func SlotByHeading(heading string) (SlotDescriptor, bool) { - for _, d := range SlotTable { - if d.Heading == heading { - return d, true - } - } - return SlotDescriptor{}, false -} - -// Headings returns every recognized section heading, in schema order. -func Headings() []string { - out := make([]string, len(SlotTable)) - for i, d := range SlotTable { - out[i] = d.Heading - } - return out -} - -// listPtr returns the addressable []string field for a SlotList key. -func (s *Slots) listPtr(key string) *[]string { - switch key { - case "rules": - return &s.Rules - case "skills": - return &s.Skills - case "prompts": - return &s.Prompts - } - return nil -} - -// prosePtr returns the addressable string field for a SlotProse key. -func (s *Slots) prosePtr(key string) *string { - switch key { - case "tool_specs": - return &s.ToolSpecs - case "goals": - return &s.Goals - case "style": - return &s.Style - case "norms": - return &s.Norms - case "architecture": - return &s.Architecture - case "business_process": - return &s.BusinessProcess - } - return nil -} - -// List reads a SlotList slot by key; nil for any other key. -func (s *Slots) List(key string) []string { - if p := s.listPtr(key); p != nil { - return *p - } - return nil -} - -// SetList writes a SlotList slot by key; a no-op for any other key. -func (s *Slots) SetList(key string, items []string) { - if p := s.listPtr(key); p != nil { - *p = items - } -} - -// Prose reads a SlotProse slot by key; "" for any other key. -func (s *Slots) Prose(key string) string { - if p := s.prosePtr(key); p != nil { - return *p - } - return "" -} - -// SetProse writes a SlotProse slot by key; a no-op for any other key. -func (s *Slots) SetProse(key, body string) { - if p := s.prosePtr(key); p != nil { - *p = body - } -} diff --git a/backend/internal/frames/validate.go b/backend/internal/frames/validate.go index b06d3db..5bd851d 100644 --- a/backend/internal/frames/validate.go +++ b/backend/internal/frames/validate.go @@ -40,7 +40,8 @@ func (e *ValidationError) Error() string { return strings.Join(parts, "; ") } -// Validate checks the fixed 10-slot schema. Returns *ValidationError (non-nil +// Validate checks a Doc's metadata fields and extends references. The body is +// free-form markdown and is never rejected. Returns *ValidationError (non-nil // .Errors) or nil. func Validate(doc *Doc) error { var errs []FieldError @@ -65,30 +66,6 @@ func Validate(doc *Doc) error { add("visibility", "must be one of "+strings.Join(VisibilityValues, ", ")) } - seenTerm := map[string]bool{} - for i, term := range doc.Slots.Terminology { - if strings.TrimSpace(term.Term) == "" { - add(fmt.Sprintf("slots.terminology[%d].term", i), "must not be empty") - } else if seenTerm[term.Term] { - add(fmt.Sprintf("slots.terminology[%d].term", i), "duplicate term within slot") - } - seenTerm[term.Term] = true - if strings.TrimSpace(term.Definition) == "" { - add(fmt.Sprintf("slots.terminology[%d].definition", i), "must not be empty") - } - } - - checkList := func(name string, items []string) { - for i, s := range items { - if strings.TrimSpace(s) == "" { - add(fmt.Sprintf("slots.%s[%d]", name, i), "must not be empty") - } - } - } - checkList("rules", doc.Slots.Rules) - checkList("skills", doc.Slots.Skills) - checkList("prompts", doc.Slots.Prompts) - for i, e := range doc.Extends { if !strings.Contains(e.Ref, "/") { add(fmt.Sprintf("extends[%d].ref", i), "must be org_slug/frame_name") diff --git a/backend/internal/frames/validate_test.go b/backend/internal/frames/validate_test.go index 43ceb99..ad835c0 100644 --- a/backend/internal/frames/validate_test.go +++ b/backend/internal/frames/validate_test.go @@ -13,6 +13,89 @@ func TestParseAndValidate_Valid(t *testing.T) { name: brand-voice description: OpenTeams brand voice version: 1.0.0 +body: | + Lead with customer impact. + + Never claim performance numbers without a benchmark citation. +`) + doc, err := frames.Parse(content) + if err != nil { + t.Fatalf("parse: %v", err) + } + if err := frames.Validate(doc); err != nil { + t.Fatalf("validate: %v", err) + } + if !strings.Contains(doc.Body, "Lead with customer impact.") { + t.Errorf("body not parsed: %q", doc.Body) + } +} + +// Documents published under the retired ten-slot schema must stay readable: +// Parse folds a legacy `slots:` block into the free-form body, rendered as the +// markdown sections the old .frame.md codec emitted. +// Both keys in one document is a legacy version somebody has since edited, and +// the precedence is silent - the discarded side produces no error and no +// warning - so it has to be pinned rather than left to be rediscovered. +func TestParse_BodyWinsOverLegacySlots(t *testing.T) { + tests := []struct { + name string + content string + wantBody string + }{ + { + name: "slots only fold into the body", + content: "name: c\ndescription: d\nversion: 1.0.0\nslots:\n rules:\n - from slots\n", + wantBody: "## Rules\n\n- from slots", + }, + { + name: "an explicit body wins and the legacy block is dropped", + content: "name: c\ndescription: d\nversion: 1.0.0\nbody: from body\n" + + "slots:\n rules:\n - from slots\n", + wantBody: "from body", + }, + { + name: "an explicitly empty body still falls back to slots", + content: "name: c\ndescription: d\nversion: 1.0.0\nbody: \"\"\n" + + "slots:\n rules:\n - from slots\n", + wantBody: "## Rules\n\n- from slots", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + doc, err := frames.Parse([]byte(tc.content)) + if err != nil { + t.Fatalf("parse: %v", err) + } + if doc.Body != tc.wantBody { + t.Errorf("body = %q, want %q", doc.Body, tc.wantBody) + } + }) + } +} + +// The decode error reaches API clients unwrapped, so it must name the schema +// rather than the unexported Go type yaml.v3 happens to be decoding into. +func TestParse_UnknownKeyErrorNamesTheSchema(t *testing.T) { + _, err := frames.Parse([]byte("name: c\ndescription: d\nversion: 1.0.0\nbogus: x\n")) + if err == nil { + t.Fatal("expected an error for an unknown key") + } + if strings.Contains(err.Error(), "docYAML") { + t.Errorf("error leaks an internal type name, which means nothing to a client: %v", err) + } + if !strings.Contains(err.Error(), "bogus") { + t.Errorf("error does not name the offending key: %v", err) + } + if !strings.Contains(err.Error(), "maintainer") { + t.Errorf("error does not list the recognized keys: %v", err) + } +} + +func TestParse_LegacySlotsFoldIntoBody(t *testing.T) { + content := []byte(` +name: brand-voice +description: OpenTeams brand voice +version: 1.0.0 slots: terminology: - term: customer @@ -29,6 +112,26 @@ slots: if err := frames.Validate(doc); err != nil { t.Fatalf("validate: %v", err) } + for _, want := range []string{ + "## Terminology", + "- **customer**: An enterprise organization.", + "## Rules", + "- Never claim performance numbers without a benchmark citation.", + "## Goals", + "Lead with customer impact.", + } { + if !strings.Contains(doc.Body, want) { + t.Errorf("legacy body missing %q:\n%s", want, doc.Body) + } + } + // The legacy shape is read-only: re-marshaling emits the new body form. + out, err := frames.Marshal(doc) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if strings.Contains(string(out), "slots:") { + t.Errorf("marshal must not emit the legacy slots key:\n%s", out) + } } func TestValidate_CollectsFieldErrors(t *testing.T) { @@ -74,97 +177,53 @@ func TestValidate_CollectsFieldErrors(t *testing.T) { wantErrorPaths: []string{"version"}, }, { - name: "empty terminology definition", + name: "bad visibility", doc: &frames.Doc{ Name: "good-name", Description: "valid description", Version: "1.0.0", + Visibility: "everyone", }, - // Slots.Terminology is set after the slice literal (see below tests[4]). - wantErrorPaths: []string{"slots.terminology[0].definition"}, - }, - { - name: "duplicate term", - doc: func() *frames.Doc { - d := &frames.Doc{ - Name: "good-name", - Description: "valid description", - Version: "1.0.0", - } - d.Slots.Terminology = []frames.Term{ - {Term: "foo", Definition: "first"}, - {Term: "foo", Definition: "second"}, - } - return d - }(), - wantErrorPaths: []string{"slots.terminology[1].term"}, - }, - { - name: "empty rule", - doc: func() *frames.Doc { - d := &frames.Doc{ - Name: "good-name", - Description: "valid description", - Version: "1.0.0", - } - d.Slots.Rules = []string{"valid rule", ""} - return d - }(), - wantErrorPaths: []string{"slots.rules[1]"}, + wantErrorPaths: []string{"visibility"}, }, { name: "extends missing slash in ref", - doc: func() *frames.Doc { - d := &frames.Doc{ - Name: "good-name", - Description: "valid description", - Version: "1.0.0", - } - d.Extends = []frames.ExtendRef{{Ref: "noslash", Version: "1.0.0"}} - return d - }(), + doc: &frames.Doc{ + Name: "good-name", + Description: "valid description", + Version: "1.0.0", + Extends: []frames.ExtendRef{{Ref: "noslash", Version: "1.0.0"}}, + }, wantErrorPaths: []string{"extends[0].ref"}, }, { name: "extends unpinned version", - doc: func() *frames.Doc { - d := &frames.Doc{ - Name: "good-name", - Description: "valid description", - Version: "1.0.0", - } - d.Extends = []frames.ExtendRef{{Ref: "org/frame", Version: ""}} - return d - }(), + doc: &frames.Doc{ + Name: "good-name", + Description: "valid description", + Version: "1.0.0", + Extends: []frames.ExtendRef{{Ref: "org/frame", Version: ""}}, + }, wantErrorPaths: []string{"extends[0].version"}, }, { name: "multiple errors collected at once", - doc: func() *frames.Doc { - d := &frames.Doc{ - Name: "Bad Name", - Description: "", - Version: "", - } - d.Slots.Terminology = []frames.Term{ - {Term: "x", Definition: ""}, - {Term: "x", Definition: "dupe"}, - } - return d - }(), + doc: &frames.Doc{ + Name: "Bad Name", + Description: "", + Version: "", + Extends: []frames.ExtendRef{{Ref: "noslash"}}, + }, wantErrorPaths: []string{ "name", "description", "version", - "slots.terminology[0].definition", - "slots.terminology[1].term", + "extends[0].ref", + "extends[0].version", }, }, } - // Set the empty-definition case's terminology (the entry above used a comment placeholder). - tests[4].doc.Slots.Terminology = []frames.Term{{Term: "x", Definition: ""}} - for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { err := frames.Validate(tc.doc) @@ -186,7 +245,7 @@ func TestValidate_CollectsFieldErrors(t *testing.T) { } func TestParse_RejectsUnknownKeys(t *testing.T) { - _, err := frames.Parse([]byte("name: x\nbogus: y\nslots: {}\n")) + _, err := frames.Parse([]byte("name: x\nbogus: y\nbody: text\n")) if err == nil { t.Fatal("expected error for unknown key, got nil") } diff --git a/backend/internal/mcp/compose.go b/backend/internal/mcp/compose.go index f887cfb..f541811 100644 --- a/backend/internal/mcp/compose.go +++ b/backend/internal/mcp/compose.go @@ -8,14 +8,16 @@ import ( "github.com/nebari-dev/nebari-frames/backend/internal/frames" ) -// composeMarkdown renders a resolved Frame Doc into the deterministic markdown -// format defined in the MCP design doc (section 3.4). Empty slots are omitted -// entirely. resolvedAt is passed in (not read from a clock) so the function is -// pure and testable. +// composeMarkdown renders a resolved Frame Doc into a deterministic markdown +// form. resolvedAt is passed in (not read from a clock) so the function is pure +// and testable. // -// Section headings and ordering come from frames.SlotTable, the same table the -// .frame.md codec uses, so the two markdown renderings cannot drift apart. The -// framing differs on purpose: this output describes an already-resolved frame +// This supersedes the per-slot rendering in section 3.4 of the MCP design doc, +// which is marked there as superseded: a Frame's content is a single free-form +// body, so there are no slots to render section by section. +// +// The body passes through verbatim. The framing differs from the .frame.md +// authoring form on purpose: this output describes an already-resolved frame // for an AI client, so it carries the "# Frame:" title and the provenance // blockquotes that an authoring document must not contain. func composeMarkdown(doc *frames.Doc, resolvedAt time.Time) string { @@ -32,41 +34,20 @@ func composeMarkdown(doc *frames.Doc, resolvedAt time.Time) string { if len(doc.Extends) > 0 { parts := make([]string, len(doc.Extends)) for i, e := range doc.Extends { - parts[i] = e.Ref + "@" + e.Version + // Matches MarshalMarkdown: an unpinned ref renders bare rather than + // with a dangling "@". + parts[i] = e.Ref + if e.Version != "" { + parts[i] += "@" + e.Version + } } fmt.Fprintf(&b, "> Inherits from: %s\n", strings.Join(parts, ", ")) } fmt.Fprintf(&b, "> Resolved at: %s\n\n", resolvedAt.UTC().Format(time.RFC3339)) - s := doc.Slots - for _, d := range frames.SlotTable { - switch d.Kind { - case frames.SlotTerms: - if len(s.Terminology) == 0 { - continue - } - fmt.Fprintf(&b, "## %s\n\n", d.Heading) - for _, t := range s.Terminology { - frames.WriteBullet(&b, fmt.Sprintf("**%s**: %s", t.Term, t.Definition)) - } - b.WriteString("\n") - case frames.SlotList: - items := s.List(d.Key) - if len(items) == 0 { - continue - } - fmt.Fprintf(&b, "## %s\n\n", d.Heading) - for _, it := range items { - frames.WriteBullet(&b, it) - } - b.WriteString("\n") - case frames.SlotProse: - body := s.Prose(d.Key) - if strings.TrimSpace(body) == "" { - continue - } - fmt.Fprintf(&b, "## %s\n\n%s\n\n", d.Heading, strings.Trim(body, "\n")) - } + if body := strings.Trim(doc.Body, "\n"); body != "" { + b.WriteString(body) + b.WriteString("\n") } return strings.TrimRight(b.String(), "\n") + "\n" diff --git a/backend/internal/mcp/compose_test.go b/backend/internal/mcp/compose_test.go index 8912303..debbe72 100644 --- a/backend/internal/mcp/compose_test.go +++ b/backend/internal/mcp/compose_test.go @@ -16,11 +16,7 @@ func TestComposeMarkdown(t *testing.T) { Description: "How we speak.", Version: "1.2.0", Extends: []frames.ExtendRef{{Ref: "openteams/base", Version: "1.0.0"}}, - Slots: frames.Slots{ - Terminology: []frames.Term{{Term: "Frame", Definition: "A context artifact."}}, - Rules: []string{"Be concise."}, - Goals: "Sound human.", - }, + Body: "Be concise.\n\n## House Style\n\nSound human.", } tests := []struct { @@ -30,22 +26,17 @@ func TestComposeMarkdown(t *testing.T) { mustNotHave []string }{ { - name: "renders populated slots and inheritance header", + name: "renders body verbatim with inheritance header", doc: full, mustContain: []string{ "# Frame: brand-voice", "How we speak.", "> Inherits from: openteams/base@1.0.0", "> Resolved at: 2026-06-26T12:00:00Z", - "## Terminology", - "- **Frame**: A context artifact.", - "## Rules", - "- Be concise.", - "## Goals", + "Be concise.", + "## House Style", "Sound human.", }, - // empty slots must be omitted entirely - mustNotHave: []string{"## Style", "## Norms", "## Skills", "## Prompts", "## Architecture", "## Business Process", "## Tool Specifications"}, }, { name: "no extends omits inherits header", diff --git a/backend/internal/mcp/integration_test.go b/backend/internal/mcp/integration_test.go index 3e2da35..feaf97c 100644 --- a/backend/internal/mcp/integration_test.go +++ b/backend/internal/mcp/integration_test.go @@ -88,7 +88,7 @@ func seedOrgAndReadableFrame(t *testing.T, mem *store.Memory) { }, Version: &framesv1.FrameVersion{ Version: "1.0.0", - Content: []byte("name: alpha\ndescription: Alpha frame\nversion: 1.0.0\nslots:\n rules:\n - r1\n"), + Content: []byte("name: alpha\ndescription: Alpha frame\nversion: 1.0.0\nbody: |\n r1\n"), PublishedAt: timestamppb.Now(), }, Grants: []store.Grant{{SubjectType: "org", SubjectID: "o1", Permission: "read"}}, @@ -273,7 +273,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { "name": "brand-voice", "description": "How we write", "version": "1.0.0", - "rules": []any{"Cite benchmarks."}, + "body": "## Rules\n\n- Cite benchmarks.", } t.Run("a viewer cannot create a frame", func(t *testing.T) { @@ -322,7 +322,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { // not the version-uniqueness check. second := map[string]any{ "name": "brand-voice", "description": "How we write", "version": "2.0.0", - "rules": []any{"Cite benchmarks."}, + "body": "## Rules\n\n- Cite benchmarks.", } text, isErr := callTool(t, cs, "create_frame", second) if !isErr { @@ -343,7 +343,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { "description": "How we write, revised", "version": "1.1.0", "base_version": "1.0.0", - "rules": []any{"Cite benchmarks.", "Avoid jargon."}, + "body": "## Rules\n\n- Cite benchmarks.\n- Avoid jargon.", "changelog": "added a rule", } text, isErr := callTool(t, cs, "update_frame", updated) @@ -367,7 +367,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { "description": "hijacked", "version": "2.0.0", "base_version": "1.0.0", - "rules": []any{"mine now"}, + "body": "## Rules\n\n- mine now", }) if !isErr { t.Fatalf("edit permission was bypassed: %q", text) @@ -382,7 +382,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { text, isErr := callTool(t, cs, "update_frame", map[string]any{ "name": "ghost", "description": "x", "version": "1.0.0", "base_version": "1.0.0", - "rules": []any{"r"}, + "body": "- r", }) if !isErr { t.Fatalf("update of an unknown frame succeeded: %q", text) @@ -398,7 +398,7 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { "name": "Not A Valid Name", "description": "x", "version": "1.0.0", - "rules": []any{"r"}, + "body": "- r", }) if !isErr { t.Fatalf("invalid name accepted: %q", text) @@ -414,8 +414,8 @@ func TestMCPWritesEnforceRBAC(t *testing.T) { // An update must not destroy what the caller did not mention. Absent fields keep // their current values; supplied fields replace them; an explicitly empty list -// clears. Without this, an AI that updates one slot silently wipes the Frame's -// visibility, maintainer, and - worst - its inheritance edges. +// clears. Without this, an AI that updates only the body silently wipes the +// Frame's visibility, maintainer, and - worst - its inheritance edges. func TestMCPUpdatePreservesOmittedFields(t *testing.T) { cs, mem := newWriteTestSession(t, "publisher") ctx := context.Background() @@ -423,26 +423,25 @@ func TestMCPUpdatePreservesOmittedFields(t *testing.T) { // A parent to inherit from, then a child that pins it and carries metadata. if _, isErr := callTool(t, cs, "create_frame", map[string]any{ "name": "base", "description": "Base", "version": "1.0.0", - "rules": []any{"from parent"}, + "body": "## Rules\n\n- from parent", }); isErr { t.Fatal("create base failed") } if _, isErr := callTool(t, cs, "create_frame", map[string]any{ "name": "child", "description": "Child", "version": "1.0.0", - "rules": []any{"from child"}, + "body": "## Rules\n\n- from child\n\n## Goals\n\nship the thing", "visibility": "private", "scope": "company", "maintainer": "platform team", "extends": []any{map[string]any{"ref": "openteams/base", "version": "1.0.0"}}, - "goals": "ship the thing", }); isErr { t.Fatal("create child failed") } - // Update only the rules. Everything else must survive. + // Update only the body. Everything else must survive. text, isErr := callTool(t, cs, "update_frame", map[string]any{ "name": "child", "version": "1.1.0", "base_version": "1.0.0", - "rules": []any{"from child", "and another"}, + "body": "## Rules\n\n- from child\n- and another\n\n## Goals\n\nship the thing", }) if isErr { t.Fatalf("update failed: %q", text) @@ -469,20 +468,18 @@ func TestMCPUpdatePreservesOmittedFields(t *testing.T) { if len(doc.Extends) != 1 || doc.Extends[0].Ref != "openteams/base" || doc.Extends[0].Version != "1.0.0" { t.Errorf("extends = %+v, want the pinned parent preserved: inheritance must survive an update", doc.Extends) } - if doc.Slots.Goals != "ship the thing" { - t.Errorf("goals = %q, want the original prose preserved", doc.Slots.Goals) + if !strings.Contains(doc.Body, "ship the thing") { + t.Errorf("body = %q, want the original prose preserved", doc.Body) } if doc.Description != "Child" { t.Errorf("description = %q, want Child", doc.Description) } // The parent's rule must NOT have been copied into the child. - if len(doc.Slots.Rules) != 2 { - t.Errorf("rules = %v, want exactly the two supplied (no inherited content flattened in)", doc.Slots.Rules) + if !strings.Contains(doc.Body, "and another") { + t.Errorf("body = %q, want the supplied rules", doc.Body) } - for _, r := range doc.Slots.Rules { - if r == "from parent" { - t.Errorf("parent content was flattened into the child: %v", doc.Slots.Rules) - } + if strings.Contains(doc.Body, "from parent") { + t.Errorf("parent content was flattened into the child: %q", doc.Body) } t.Run("supplied fields replace, and an explicit empty list clears", func(t *testing.T) { @@ -524,7 +521,7 @@ func mustFrameID(t *testing.T, mem *store.Memory, name string) string { } // The most likely instruction this tool will ever get is "add a rule to X". -// Doing that requires reading the Frame's current rules, and if the only read +// Doing that requires reading the Frame's current body, and if the only read // available returns the inheritance-composed form, the model has no choice but // to send the parent's content back as the child's own - which validates, looks // identical when composed, and silently detaches the child from its parent's @@ -535,14 +532,13 @@ func TestMCPReadModifyWriteDoesNotFlattenInheritance(t *testing.T) { if _, isErr := callTool(t, cs, "create_frame", map[string]any{ "name": "company-base", "description": "Company", "version": "1.0.0", - "rules": []any{"Use inclusive language."}, - "goals": "Grow the platform.", + "body": "## Rules\n\n- Use inclusive language.\n\n## Goals\n\nGrow the platform.", }); isErr { t.Fatal("create parent failed") } if _, isErr := callTool(t, cs, "create_frame", map[string]any{ "name": "team-api", "description": "API team", "version": "1.0.0", - "rules": []any{"Version every endpoint."}, + "body": "## Rules\n\n- Version every endpoint.", "extends": []any{map[string]any{"ref": "openteams/company-base", "version": "1.0.0"}}, }); isErr { t.Fatal("create child failed") @@ -560,7 +556,7 @@ func TestMCPReadModifyWriteDoesNotFlattenInheritance(t *testing.T) { t.Errorf("source read is missing the frame's own rule:\n%s", src) } if strings.Contains(src, "Grow the platform.") { - t.Errorf("source read leaked the parent's prose slot:\n%s", src) + t.Errorf("source read leaked the parent's prose:\n%s", src) } // The default read stays composed, which is what a consumer wants. @@ -575,7 +571,7 @@ func TestMCPReadModifyWriteDoesNotFlattenInheritance(t *testing.T) { // Editing from the source read leaves inheritance intact and un-flattened. if _, isErr := callTool(t, cs, "update_frame", map[string]any{ "name": "team-api", "version": "1.1.0", "base_version": "1.0.0", - "rules": []any{"Version every endpoint.", "Prefer cursor pagination."}, + "body": "## Rules\n\n- Version every endpoint.\n- Prefer cursor pagination.", }); isErr { t.Fatal("update failed") } @@ -587,13 +583,11 @@ func TestMCPReadModifyWriteDoesNotFlattenInheritance(t *testing.T) { if err != nil { t.Fatalf("parse: %v", err) } - for _, r := range doc.Slots.Rules { - if r == "Use inclusive language." { - t.Errorf("the parent's rule was copied into the child: %v", doc.Slots.Rules) - } + if strings.Contains(doc.Body, "Use inclusive language.") { + t.Errorf("the parent's rule was copied into the child: %q", doc.Body) } - if doc.Slots.Goals != "" { - t.Errorf("goals = %q, want empty: the parent's prose must not be frozen into the child", doc.Slots.Goals) + if strings.Contains(doc.Body, "Grow the platform.") { + t.Errorf("body = %q: the parent's prose must not be frozen into the child", doc.Body) } if len(doc.Extends) != 1 { t.Errorf("extends = %+v, want the parent still pinned", doc.Extends) diff --git a/backend/internal/mcp/resources_test.go b/backend/internal/mcp/resources_test.go index 65ebc61..007e3b7 100644 --- a/backend/internal/mcp/resources_test.go +++ b/backend/internal/mcp/resources_test.go @@ -53,7 +53,7 @@ func TestGetServer_DevModeBuildsServer(t *testing.T) { {OrgSlug: "openteams", OrgDisplay: "OpenTeams", Name: "alpha", Version: "1.0.0", Description: "A"}, }, docs: map[string]*frames.Doc{ - "openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Slots: frames.Slots{Rules: []string{"r1"}}}, + "openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Body: "- r1"}, }, } rs := &resourceServer{src: src, cfg: Config{DevMode: true}} @@ -67,7 +67,7 @@ func TestGetServer_DevModeBuildsServer(t *testing.T) { func TestReadHandler(t *testing.T) { src := stubSource{ docs: map[string]*frames.Doc{ - "openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Slots: frames.Slots{Rules: []string{"r1"}}}, + "openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Body: "- r1"}, }, } rs := &resourceServer{src: src, cfg: Config{DevMode: true}} @@ -156,7 +156,7 @@ func TestListFramesTool(t *testing.T) { func TestGetFrameTool(t *testing.T) { src := stubSource{ readable: []frames.ReadableFrame{{OrgSlug: "openteams", OrgDisplay: "OpenTeams", Name: "alpha", Version: "1.0.0", Description: "A"}}, - docs: map[string]*frames.Doc{"openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Slots: frames.Slots{Rules: []string{"r1"}}}}, + docs: map[string]*frames.Doc{"openteams/alpha": {Name: "alpha", Description: "A", Version: "1.0.0", Body: "- r1"}}, } rs := &resourceServer{src: src, cfg: Config{DevMode: true}} h := rs.getFrameTool(auth.DevClaims()) @@ -222,7 +222,7 @@ func (s *stubWriter) PublishDocFrom(_ context.Context, doc *frames.Doc, changelo } // ptr is shorthand for the optional string fields. -func ptr(s string) *string { return &s } +func ptr[T any](v T) *T { return &v } func TestWriteFrameTools(t *testing.T) { validInput := func() writeFrameInput { @@ -232,7 +232,7 @@ func TestWriteFrameTools(t *testing.T) { Version: "1.0.0", // Required by update_frame and ignored by create_frame. BaseVersion: "0.9.0", - Rules: []string{"Cite benchmarks."}, + Body: ptr("## Rules\n\n- Cite benchmarks."), } } @@ -327,21 +327,13 @@ func TestWriteFrameInputCarriesEveryField(t *testing.T) { in := writeFrameInput{ Name: "full", Description: ptr("d"), Version: "1.0.0", - Visibility: ptr("private"), - Scope: ptr("company"), - Maintainer: ptr("platform team"), - Terminology: []termInput{{Term: "Frame", Definition: "a context artifact"}}, - Rules: []string{"rule"}, - Skills: []string{"skill"}, - Prompts: []string{"prompt"}, - ToolSpecs: ptr("tools"), - Goals: ptr("goals"), - Style: ptr("style"), - Norms: ptr("norms"), - Architecture: ptr("architecture"), - BusinessProcess: ptr("process"), - Extends: []extendInput{{Ref: "openteams/base", Version: "1.0.0"}}, - Excludes: []string{"openteams/legacy"}, + Visibility: ptr("private"), + Scope: ptr("company"), + Maintainer: ptr("platform team"), + Body: ptr("## Rules\n\n- rule"), + Template: ptr(true), + Extends: []extendInput{{Ref: "openteams/base", Version: "1.0.0"}}, + Excludes: []string{"openteams/legacy"}, } if _, _, err := h(context.Background(), &gomcp.CallToolRequest{}, in); err != nil { t.Fatalf("create_frame: %v", err) @@ -355,18 +347,8 @@ func TestWriteFrameInputCarriesEveryField(t *testing.T) { Visibility: "private", Scope: "company", Maintainer: "platform team", Extends: []frames.ExtendRef{{Ref: "openteams/base", Version: "1.0.0"}}, Excludes: []string{"openteams/legacy"}, - Slots: frames.Slots{ - Terminology: []frames.Term{{Term: "Frame", Definition: "a context artifact"}}, - Rules: []string{"rule"}, - Skills: []string{"skill"}, - Prompts: []string{"prompt"}, - ToolSpecs: "tools", - Goals: "goals", - Style: "style", - Norms: "norms", - Architecture: "architecture", - BusinessProcess: "process", - }, + Template: true, + Body: "## Rules\n\n- rule", } if !reflect.DeepEqual(got, want) { t.Errorf("doc mismatch\n got: %+v\nwant: %+v", got, want) @@ -377,8 +359,8 @@ func TestWriteFrameInputCarriesEveryField(t *testing.T) { // hand-written literal, so a newly added slot or document field was zero on both // sides and passed - which is exactly how visibility, scope, and maintainer came // to be silently dropped. This walks the canonical definitions instead, so -// extending frames.Doc or frames.SlotTable without extending writeFrameInput -// fails here rather than in production. +// extending frames.Doc without extending writeFrameInput fails here rather than +// in production. func TestWriteFrameInputCoversDocFields(t *testing.T) { inputFields := map[string]bool{} inT := reflect.TypeOf(writeFrameInput{}) @@ -387,42 +369,27 @@ func TestWriteFrameInputCoversDocFields(t *testing.T) { inputFields[name] = true } - t.Run("every slot has an input field", func(t *testing.T) { - for _, d := range frames.SlotTable { - if !inputFields[d.Key] { - t.Errorf("slot %q has no writeFrameInput field: MCP writes would silently drop it", d.Key) - } + // Every field of frames.Doc, and the decision recorded for each. A new + // document field lands here as a failure, which is the point: the author has + // to say whether MCP writes carry it rather than letting it default to no. + expected := map[string]bool{ + "name": true, "description": true, "version": true, + "visibility": true, "scope": true, "maintainer": true, + "extends": true, "excludes": true, + // The content itself, and the flag that offers a Frame as a template. + "body": true, "template": true, + } + docT := reflect.TypeOf(frames.Doc{}) + for i := range docT.NumField() { + name, _, _ := strings.Cut(docT.Field(i).Tag.Get("yaml"), ",") + if !expected[name] { + t.Errorf("frames.Doc gained field %q: decide whether MCP writes must carry it, then add it here", name) + continue } - // Guard the other direction too: frames.Slots must not grow a field that - // SlotTable does not describe. - if got, want := reflect.TypeOf(frames.Slots{}).NumField(), len(frames.SlotTable); got != want { - t.Errorf("frames.Slots has %d fields but SlotTable describes %d", got, want) + if !inputFields[name] { + t.Errorf("document field %q has no writeFrameInput field: MCP writes would silently drop it", name) } - }) - - t.Run("every document field is accounted for", func(t *testing.T) { - // Doc-level fields that are not slots. "slots" is the container itself; - // the rest are the Frame Spec metadata plus inheritance. - expected := map[string]bool{ - "name": true, "description": true, "version": true, - "visibility": true, "scope": true, "maintainer": true, - "extends": true, "excludes": true, "slots": true, - } - docT := reflect.TypeOf(frames.Doc{}) - for i := range docT.NumField() { - name, _, _ := strings.Cut(docT.Field(i).Tag.Get("yaml"), ",") - if !expected[name] { - t.Errorf("frames.Doc gained field %q: decide whether MCP writes must carry it, then add it here", name) - continue - } - if name == "slots" { - continue // covered by the slot walk above - } - if !inputFields[name] { - t.Errorf("document field %q has no writeFrameInput field: MCP writes would silently drop it", name) - } - } - }) + } } // The write tools must be advertised, or a client has no way to call them. @@ -484,7 +451,14 @@ func TestApplyToWiresEveryInputField(t *testing.T) { f.SetString("sentinel-" + name) case reflect.Pointer: sv := reflect.New(f.Type().Elem()) - sv.Elem().SetString("sentinel-" + name) + switch sv.Elem().Kind() { + case reflect.String: + sv.Elem().SetString("sentinel-" + name) + case reflect.Bool: + sv.Elem().SetBool(true) + default: + t.Fatalf("field %s: unhandled pointer element kind %s", name, sv.Elem().Kind()) + } f.Set(sv) case reflect.Bool: f.SetBool(true) @@ -518,19 +492,8 @@ func TestApplyToWiresEveryInputField(t *testing.T) { docV := reflect.ValueOf(*got) docT := docV.Type() for i := range docT.NumField() { - name := docT.Field(i).Name - if name == "Slots" { - continue - } if docV.Field(i).IsZero() { - t.Errorf("Doc.%s is zero after applyTo: the input field exists but is not wired in", name) - } - } - slotsV := reflect.ValueOf(got.Slots) - slotsT := slotsV.Type() - for i := range slotsT.NumField() { - if slotsV.Field(i).IsZero() { - t.Errorf("Slots.%s is zero after applyTo: the input field exists but is not wired in", slotsT.Field(i).Name) + t.Errorf("Doc.%s is zero after applyTo: the input field exists but is not wired in", docT.Field(i).Name) } } } @@ -541,14 +504,14 @@ func TestApplyToWiresEveryInputField(t *testing.T) { func TestUpdateFrameSendsTheBaseVersionItRead(t *testing.T) { src := &stubWriter{current: &frames.Doc{ Name: "brand-voice", Description: "d", Version: "3.4.5", - Slots: frames.Slots{Rules: []string{"existing"}}, + Body: "## Rules\n\n- existing", }} rs := &resourceServer{src: src, cfg: Config{DevMode: true}} h := rs.updateFrameTool(auth.DevClaims()) if _, _, err := h(context.Background(), &gomcp.CallToolRequest{}, writeFrameInput{ Name: "brand-voice", Version: "3.5.0", BaseVersion: "3.4.5", - Rules: []string{"existing", "new"}, + Body: ptr("## Rules\n\n- existing\n- new"), }); err != nil { t.Fatalf("update_frame: %v", err) } @@ -566,7 +529,7 @@ func TestCreateFrameSendsNoBaseVersion(t *testing.T) { rs := &resourceServer{src: src, cfg: Config{DevMode: true}} h := rs.createFrameTool(auth.DevClaims()) if _, _, err := h(context.Background(), &gomcp.CallToolRequest{}, writeFrameInput{ - Name: "n", Description: ptr("d"), Version: "1.0.0", Rules: []string{"r"}, + Name: "n", Description: ptr("d"), Version: "1.0.0", Body: ptr("- r"), }); err != nil { t.Fatalf("create_frame: %v", err) } diff --git a/backend/internal/mcp/write.go b/backend/internal/mcp/write.go index 1e6ea96..f64712b 100644 --- a/backend/internal/mcp/write.go +++ b/backend/internal/mcp/write.go @@ -13,12 +13,6 @@ import ( "github.com/nebari-dev/nebari-frames/backend/internal/frames" ) -// termInput is one vocabulary entry in the terminology slot. -type termInput struct { - Term string `json:"term" jsonschema:"the term being defined"` - Definition string `json:"definition" jsonschema:"what the term means in this organization"` -} - // extendInput is a pinned reference to a parent Frame. type extendInput struct { Ref string `json:"ref" jsonschema:"parent Frame reference as org_slug/frame_name"` @@ -32,11 +26,11 @@ type extendInput struct { // Every optional field is a pointer or a slice so that "not mentioned" is // distinguishable from "set to empty". update_frame relies on that distinction: // an omitted field keeps the Frame's current value, while an explicitly empty -// one clears it. Without it, an AI updating a single slot would silently erase +// one clears it. Without it, an AI updating only the body would silently erase // the Frame's metadata and - far worse - its inheritance edges. // -// TestWriteFrameInputCoversDocFields walks frames.SlotTable and the frames.Doc -// field set, so adding a slot or a document field without adding it here fails. +// TestWriteFrameInputCoversDocFields walks the frames.Doc field set, so adding a +// document field without adding it here fails. type writeFrameInput struct { Name string `json:"name" jsonschema:"Frame name: lowercase letters, digits and dashes, e.g. brand-voice"` Version string `json:"version" jsonschema:"semantic version for the new revision, e.g. 1.1.0; must not already exist"` @@ -51,16 +45,16 @@ type writeFrameInput struct { Scope *string `json:"scope,omitempty" jsonschema:"who this Frame applies to, e.g. company or team-platform; omit to keep the current one, pass an empty string to clear it"` Maintainer *string `json:"maintainer,omitempty" jsonschema:"who owns this Frame; omit to keep the current one, pass an empty string to clear it"` - Terminology []termInput `json:"terminology,omitempty" jsonschema:"named concepts and their definitions; omit to keep the current list, pass an empty list to clear it"` - Rules []string `json:"rules,omitempty" jsonschema:"constraints that must be followed; omit to keep the current list, pass an empty list to clear it"` - Skills []string `json:"skills,omitempty" jsonschema:"capabilities this Frame expects; omit to keep, empty list to clear"` - Prompts []string `json:"prompts,omitempty" jsonschema:"reusable prompts; omit to keep, empty list to clear"` - ToolSpecs *string `json:"tool_specs,omitempty" jsonschema:"tool specifications, as markdown; omit to keep the current text, pass an empty string to clear it"` - Goals *string `json:"goals,omitempty" jsonschema:"what the organization is trying to achieve, as markdown; omit to keep the current text, pass an empty string to clear it"` - Style *string `json:"style,omitempty" jsonschema:"voice and formatting conventions, as markdown; omit to keep the current text, pass an empty string to clear it"` - Norms *string `json:"norms,omitempty" jsonschema:"team norms and expectations, as markdown; omit to keep the current text, pass an empty string to clear it"` - Architecture *string `json:"architecture,omitempty" jsonschema:"system architecture context, as markdown; omit to keep the current text, pass an empty string to clear it"` - BusinessProcess *string `json:"business_process,omitempty" jsonschema:"business process context, as markdown; omit to keep the current text, pass an empty string to clear it"` + // The whole content of a Frame. Frame Spec v0.2 defines no body structure, + // so there is nothing to break it into: headings, lists and prose are the + // author's choice. Callers that read a legacy slot-shaped Frame get it back + // already rendered as markdown, so editing and republishing it is a plain + // string edit rather than a schema migration. + Body *string `json:"body,omitempty" jsonschema:"the Frame's content as free-form markdown. Structure it however the guidance reads best - headings, lists, prose. Omit to keep the current body, pass an empty string to clear it"` + // Registry metadata rather than spec metadata, but MCP writes must carry it: + // without it create_frame could not produce a template at all, and an + // omitted-means-keep pointer stops update_frame de-listing one by accident. + Template *bool `json:"template,omitempty" jsonschema:"true to offer this Frame in the authoring UI's template picker; omit to keep the current setting"` Extends []extendInput `json:"extends,omitempty" jsonschema:"parent Frames this one inherits from, each pinned to a version; later parents win. Omit to keep the current inheritance, pass an empty list to remove all parents"` Excludes []string `json:"excludes,omitempty" jsonschema:"parent references to exclude from inheritance; omit to keep, empty list to clear"` @@ -80,29 +74,10 @@ func (in writeFrameInput) applyTo(base *frames.Doc) *frames.Doc { setString(&d.Visibility, in.Visibility) setString(&d.Scope, in.Scope) setString(&d.Maintainer, in.Maintainer) + setString(&d.Body, in.Body) - setString(&d.Slots.ToolSpecs, in.ToolSpecs) - setString(&d.Slots.Goals, in.Goals) - setString(&d.Slots.Style, in.Style) - setString(&d.Slots.Norms, in.Norms) - setString(&d.Slots.Architecture, in.Architecture) - setString(&d.Slots.BusinessProcess, in.BusinessProcess) - - if in.Rules != nil { - d.Slots.Rules = in.Rules - } - if in.Skills != nil { - d.Slots.Skills = in.Skills - } - if in.Prompts != nil { - d.Slots.Prompts = in.Prompts - } - if in.Terminology != nil { - terms := make([]frames.Term, len(in.Terminology)) - for i, t := range in.Terminology { - terms[i] = frames.Term{Term: t.Term, Definition: t.Definition} - } - d.Slots.Terminology = terms + if in.Template != nil { + d.Template = *in.Template } if in.Extends != nil { refs := make([]frames.ExtendRef, len(in.Extends)) @@ -139,8 +114,8 @@ func (rs *resourceServer) createFrameTool(claims *auth.Claims) gomcp.ToolHandler // permission on the target and rejects an unknown name. // // The merge base is SourceDoc - the Frame's OWN document - and not the composed -// form get_frame returns. Merging onto a resolved document would copy every -// parent's slots into the child and drop its extends edges, quietly destroying +// form get_frame returns. Merging onto a resolved document would bake every +// ancestor's body into the child and drop its extends edges, quietly destroying // the inheritance graph. func (rs *resourceServer) updateFrameTool(claims *auth.Claims) gomcp.ToolHandlerFor[writeFrameInput, any] { return func(ctx context.Context, _ *gomcp.CallToolRequest, in writeFrameInput) (*gomcp.CallToolResult, any, error) { diff --git a/backend/internal/store/sqlite/migrations/006_frame_is_template.sql b/backend/internal/store/sqlite/migrations/006_frame_is_template.sql new file mode 100644 index 0000000..14e3f11 --- /dev/null +++ b/backend/internal/store/sqlite/migrations/006_frame_is_template.sql @@ -0,0 +1,8 @@ +-- +goose Up +-- Frames flagged as templates are offered as starting points by the authoring +-- UI's "start from a template" picker. Denormalized from the latest version's +-- `template` doc field at publish time so listing never parses content blobs. +ALTER TABLE frames ADD COLUMN is_template INTEGER NOT NULL DEFAULT 0; + +-- +goose Down +ALTER TABLE frames DROP COLUMN is_template; diff --git a/backend/internal/store/sqlite/migrations/migrate_legacy_test.go b/backend/internal/store/sqlite/migrations/migrate_legacy_test.go index 5d6aeac..ea04c8f 100644 --- a/backend/internal/store/sqlite/migrations/migrate_legacy_test.go +++ b/backend/internal/store/sqlite/migrations/migrate_legacy_test.go @@ -16,6 +16,10 @@ import ( // that the case-sensitive unique index used to permit, and migrates forward. A // failure here is a crash-looping pod: migrations.Run's error reaches main, // which exits. +// +// The fixture carries every table a later migration touches, not just the ones +// with data to repair: an ALTER against a table the fixture forgot fails as +// "no such table", which looks like a broken migration rather than a stale test. func TestRunOnCaseVariantLegacyData(t *testing.T) { ctx := context.Background() db, err := sql.Open("sqlite", t.TempDir()+"/legacy.db") @@ -28,6 +32,9 @@ func TestRunOnCaseVariantLegacyData(t *testing.T) { `CREATE TABLE goose_db_version (id INTEGER PRIMARY KEY AUTOINCREMENT, version_id INTEGER NOT NULL, is_applied INTEGER NOT NULL, tstamp TIMESTAMP DEFAULT (datetime('now')))`, `INSERT INTO goose_db_version (version_id, is_applied) VALUES (0,1),(1,1),(2,1),(3,1),(4,1)`, `CREATE TABLE orgs (id TEXT PRIMARY KEY, slug TEXT NOT NULL UNIQUE, display_name TEXT NOT NULL, created_at TEXT NOT NULL)`, + // The frames table as of 002. Migration 006 alters it, so a fixture that + // omitted it would fail on a missing table rather than on the data. + `CREATE TABLE frames (id TEXT PRIMARY KEY, org_id TEXT NOT NULL REFERENCES orgs(id), name TEXT NOT NULL, description TEXT NOT NULL, owner_sub TEXT NOT NULL, latest_version TEXT NOT NULL, created_at TEXT NOT NULL, updated_at TEXT NOT NULL, UNIQUE (org_id, name))`, `CREATE TABLE org_memberships (org_id TEXT NOT NULL REFERENCES orgs(id), user_sub TEXT NOT NULL DEFAULT '', email TEXT, role TEXT NOT NULL, added_at TEXT NOT NULL)`, `CREATE UNIQUE INDEX idx_membership_sub ON org_memberships(user_sub) WHERE user_sub <> ''`, `CREATE UNIQUE INDEX idx_membership_email ON org_memberships(org_id, email) WHERE email IS NOT NULL`, @@ -40,6 +47,9 @@ func TestRunOnCaseVariantLegacyData(t *testing.T) { `INSERT INTO org_memberships (org_id,user_sub,email,role,added_at) VALUES ('o1','','CAROL@X.IO','publisher','2026-01-04T00:00:00Z')`, // Stray whitespace, no collision. `INSERT INTO org_memberships (org_id,user_sub,email,role,added_at) VALUES ('o1','',' dave@x.io ','viewer','2026-01-05T00:00:00Z')`, + // A pre-existing frame, so 006's ALTER runs against a populated table and + // its NOT NULL DEFAULT is exercised on real rows rather than none. + `INSERT INTO frames VALUES ('f1','o1','brand-voice','How we write','s2','1.0.0','2026-01-01T00:00:00Z','2026-01-01T00:00:00Z')`, } for _, q := range stmts { if _, err := db.ExecContext(ctx, q); err != nil { @@ -93,6 +103,16 @@ func TestRunOnCaseVariantLegacyData(t *testing.T) { t.Errorf("dave = %+v, want the trimmed address to survive", d) } + // 006 adds is_template with a default, so a frame that predates it must come + // forward as not-a-template rather than NULL. + var isTemplate sql.NullInt64 + if err := db.QueryRowContext(ctx, `SELECT is_template FROM frames WHERE id = 'f1'`).Scan(&isTemplate); err != nil { + t.Fatalf("read is_template on a pre-existing frame: %v", err) + } + if !isTemplate.Valid || isTemplate.Int64 != 0 { + t.Errorf("is_template = %v, want 0 for a frame published before the column existed", isTemplate) + } + // The new index must reject a case variant rather than storing both. if _, err := db.ExecContext(ctx, `INSERT INTO org_memberships (org_id,user_sub,email,role,added_at) VALUES ('o1','s3','BOSS@X.IO','viewer','2026-01-03T00:00:00Z')`, diff --git a/backend/internal/store/sqlite/sqlite.go b/backend/internal/store/sqlite/sqlite.go index 0aac913..e393eea 100644 --- a/backend/internal/store/sqlite/sqlite.go +++ b/backend/internal/store/sqlite/sqlite.go @@ -236,10 +236,10 @@ func (r *Repository) CreateFrameVersion(ctx context.Context, in store.CreateFram now := f.UpdatedAt.AsTime().UTC().Format(time.RFC3339) if in.IsNewFrame { if _, err := tx.ExecContext(ctx, - `INSERT INTO frames (id, org_id, name, description, owner_sub, latest_version, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + `INSERT INTO frames (id, org_id, name, description, owner_sub, latest_version, created_at, updated_at, is_template) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, f.Id, f.OrgId, f.Name, f.Description, f.OwnerSub, f.LatestVersion, - f.CreatedAt.AsTime().UTC().Format(time.RFC3339), now); err != nil { + f.CreatedAt.AsTime().UTC().Format(time.RFC3339), now, f.IsTemplate); err != nil { if isUnique(err) { return store.ErrAlreadyExists } @@ -247,8 +247,8 @@ func (r *Repository) CreateFrameVersion(ctx context.Context, in store.CreateFram } } else { if _, err := tx.ExecContext(ctx, - `UPDATE frames SET description=?, latest_version=?, updated_at=? WHERE id=?`, - f.Description, f.LatestVersion, now, f.Id); err != nil { + `UPDATE frames SET description=?, latest_version=?, updated_at=?, is_template=? WHERE id=?`, + f.Description, f.LatestVersion, now, f.IsTemplate, f.Id); err != nil { return err } } @@ -299,21 +299,21 @@ func (r *Repository) CreateFrameVersion(ctx context.Context, in store.CreateFram func (r *Repository) GetFrameBySlugName(ctx context.Context, orgSlug, name string) (*framesv1.Frame, error) { return r.scanFrame(r.db.QueryRowContext(ctx, - `SELECT f.id, f.org_id, f.name, f.description, f.owner_sub, f.latest_version, f.created_at, f.updated_at + `SELECT f.id, f.org_id, f.name, f.description, f.owner_sub, f.latest_version, f.created_at, f.updated_at, f.is_template FROM frames f JOIN orgs o ON o.id = f.org_id WHERE o.slug = ? AND f.name = ?`, orgSlug, name)) } func (r *Repository) GetFrameByID(ctx context.Context, id string) (*framesv1.Frame, error) { return r.scanFrame(r.db.QueryRowContext(ctx, - `SELECT id, org_id, name, description, owner_sub, latest_version, created_at, updated_at + `SELECT id, org_id, name, description, owner_sub, latest_version, created_at, updated_at, is_template FROM frames WHERE id = ?`, id)) } func (r *Repository) scanFrame(row *sql.Row) (*framesv1.Frame, error) { var f framesv1.Frame var created, updated string - if err := row.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated); err != nil { + if err := row.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated, &f.IsTemplate); err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, store.ErrNotFound } @@ -405,7 +405,7 @@ func (r *Repository) ListFrameVersions(ctx context.Context, frameID string) ([]* func (r *Repository) ListFramesByOrg(ctx context.Context, orgID string) ([]*framesv1.Frame, error) { rows, err := r.db.QueryContext(ctx, - `SELECT id, org_id, name, description, owner_sub, latest_version, created_at, updated_at + `SELECT id, org_id, name, description, owner_sub, latest_version, created_at, updated_at, is_template FROM frames WHERE org_id = ? ORDER BY updated_at DESC`, orgID) if err != nil { return nil, err @@ -415,7 +415,7 @@ func (r *Repository) ListFramesByOrg(ctx context.Context, orgID string) ([]*fram for rows.Next() { var f framesv1.Frame var created, updated string - if err := rows.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated); err != nil { + if err := rows.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated, &f.IsTemplate); err != nil { return nil, err } f.CreatedAt, f.UpdatedAt = ts(created), ts(updated) @@ -520,7 +520,7 @@ func (r *Repository) DeleteMembership(ctx context.Context, orgID, userSub, email func (r *Repository) FrameChildren(ctx context.Context, parentFrameID string) ([]*framesv1.Frame, error) { rows, err := r.db.QueryContext(ctx, - `SELECT DISTINCT f.id, f.org_id, f.name, f.description, f.owner_sub, f.latest_version, f.created_at, f.updated_at + `SELECT DISTINCT f.id, f.org_id, f.name, f.description, f.owner_sub, f.latest_version, f.created_at, f.updated_at, f.is_template FROM frames f JOIN frame_extends fe ON fe.frame_id = f.id WHERE fe.parent_frame_id = ? AND f.id <> ?`, parentFrameID, parentFrameID) @@ -532,7 +532,7 @@ func (r *Repository) FrameChildren(ctx context.Context, parentFrameID string) ([ for rows.Next() { var f framesv1.Frame var created, updated string - if err := rows.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated); err != nil { + if err := rows.Scan(&f.Id, &f.OrgId, &f.Name, &f.Description, &f.OwnerSub, &f.LatestVersion, &created, &updated, &f.IsTemplate); err != nil { return nil, err } f.CreatedAt, f.UpdatedAt = ts(created), ts(updated) diff --git a/backend/internal/store/sqlite/sqlite_test.go b/backend/internal/store/sqlite/sqlite_test.go index 915de8c..7efe061 100644 --- a/backend/internal/store/sqlite/sqlite_test.go +++ b/backend/internal/store/sqlite/sqlite_test.go @@ -790,3 +790,138 @@ func TestMembershipEmailsAreCanonical(t *testing.T) { } }) } + +// is_template is the one frame column this package writes from a flag rather +// than from content, and it is read back by four separate SELECT lists whose +// column order has to match four separate Scan calls. That is exactly the shape +// where a mismatch survives a green suite: the service-level tests run against +// store.NewMemory(), which round-trips the proto and executes none of this SQL. +// +// So every read path is asserted, and the false case is asserted alongside the +// true one - a scan that landed on the wrong column would still satisfy a +// true-only test whenever the neighbouring value happened to be truthy. +func TestSQLite_IsTemplateRoundTripsThroughEveryReadPath(t *testing.T) { + ctx := context.Background() + now := timestamppb.Now() + + r := newRepo(t) + seedOrg(t, r, "o1", "openteams") + + tmpl := baseInput(now) + tmpl.Frame.IsTemplate = true + if err := r.CreateFrameVersion(ctx, tmpl); err != nil { + t.Fatalf("publish template: %v", err) + } + + plain := baseInput(now) + plain.Frame.Id, plain.Frame.Name = "f2", "plain-frame" + plain.Frame.IsTemplate = false + if err := r.CreateFrameVersion(ctx, plain); err != nil { + t.Fatalf("publish non-template: %v", err) + } + + // A child of the template, so FrameChildren has something to return. + child := baseInput(now) + child.Frame.Id, child.Frame.Name = "f3", "child-frame" + child.Frame.IsTemplate = false + child.Extends = []store.ParentEdge{{ParentFrameID: "f1", ParentVersion: "1.0.0"}} + if err := r.CreateFrameVersion(ctx, child); err != nil { + t.Fatalf("publish child: %v", err) + } + + reads := []struct { + name string + get func(id string) (*framesv1.Frame, error) + }{ + { + name: "GetFrameByID", + get: func(id string) (*framesv1.Frame, error) { return r.GetFrameByID(ctx, id) }, + }, + { + name: "GetFrameBySlugName", + get: func(id string) (*framesv1.Frame, error) { + byID, err := r.GetFrameByID(ctx, id) + if err != nil { + return nil, err + } + return r.GetFrameBySlugName(ctx, "openteams", byID.Name) + }, + }, + { + name: "ListFramesByOrg", + get: func(id string) (*framesv1.Frame, error) { + list, err := r.ListFramesByOrg(ctx, "o1") + if err != nil { + return nil, err + } + for _, f := range list { + if f.Id == id { + return f, nil + } + } + return nil, errors.New("frame not in listing") + }, + }, + } + + for _, read := range reads { + t.Run(read.name, func(t *testing.T) { + for id, want := range map[string]bool{"f1": true, "f2": false} { + got, err := read.get(id) + if err != nil { + t.Fatalf("%s(%s): %v", read.name, id, err) + } + if got.IsTemplate != want { + t.Errorf("%s(%s).IsTemplate = %v, want %v", read.name, id, got.IsTemplate, want) + } + // A scan one column off would corrupt a neighbour rather than + // only the flag, so check the columns either side of it. + if got.Id != id { + t.Errorf("%s(%s).Id = %q: the scan is off by a column", read.name, id, got.Id) + } + if got.LatestVersion != "1.0.0" { + t.Errorf("%s(%s).LatestVersion = %q: the scan is off by a column", read.name, id, got.LatestVersion) + } + } + }) + } + + t.Run("FrameChildren", func(t *testing.T) { + children, err := r.FrameChildren(ctx, "f1") + if err != nil { + t.Fatalf("FrameChildren: %v", err) + } + if len(children) != 1 { + t.Fatalf("got %d children, want 1", len(children)) + } + if children[0].IsTemplate { + t.Errorf("child.IsTemplate = true, want false") + } + if children[0].Id != "f3" || children[0].LatestVersion != "1.0.0" { + t.Errorf("child = %+v: the scan is off by a column", children[0]) + } + }) + + // Republishing recomputes the flag, so the UPDATE arm has to carry it too - + // it is a different statement from the INSERT and would fail independently. + t.Run("republishing carries the flag through the UPDATE arm", func(t *testing.T) { + next := baseInput(timestamppb.Now()) + next.IsNewFrame = false + next.Frame.IsTemplate = false + next.Frame.LatestVersion = "1.1.0" + next.Version.Version = "1.1.0" + if err := r.CreateFrameVersion(ctx, next); err != nil { + t.Fatalf("republish: %v", err) + } + got, err := r.GetFrameByID(ctx, "f1") + if err != nil { + t.Fatalf("GetFrameByID: %v", err) + } + if got.IsTemplate { + t.Error("IsTemplate = true after republishing without the flag; the UPDATE did not carry it") + } + if got.LatestVersion != "1.1.0" { + t.Errorf("LatestVersion = %q, want 1.1.0", got.LatestVersion) + } + }) +} diff --git a/cli/cmd/resolve_test.go b/cli/cmd/resolve_test.go index 1fb85bd..eec67b0 100644 --- a/cli/cmd/resolve_test.go +++ b/cli/cmd/resolve_test.go @@ -13,7 +13,7 @@ import ( func TestResolve(t *testing.T) { url := testutil.NewStubServer(t, &testutil.StubService{ ResolveFn: func(_ context.Context, r *connect.Request[framesv1.ResolveFrameRequest]) (*connect.Response[framesv1.ResolveFrameResponse], error) { - return connect.NewResponse(&framesv1.ResolveFrameResponse{ResolvedContent: []byte("name: brand-voice\nslots:\n rules:\n - merged\n")}), nil + return connect.NewResponse(&framesv1.ResolveFrameResponse{ResolvedContent: []byte("name: brand-voice\nbody: |\n merged\n")}), nil }, }) out := runCmd(t, url, "resolve", "openteams/brand-voice@1.0.0") diff --git a/docs/design/2026-05-21-mcp-endpoint-design.md b/docs/design/2026-05-21-mcp-endpoint-design.md index 635b5ad..2b42a1c 100644 --- a/docs/design/2026-05-21-mcp-endpoint-design.md +++ b/docs/design/2026-05-21-mcp-endpoint-design.md @@ -138,15 +138,35 @@ Listing resources (`resources/list`) returns the user's full set of readable Fra ### 3.4 Resource content +> **Superseded by [#59](https://github.com/nebari-dev/nebari-frames/issues/59):** a Frame's content is now a single free-form markdown body, matching Frame Spec v0.2, which defines no body structure. There are no slots left to render section by section, so the per-slot composition format below no longer describes the server. The read path (steps 1-3) is unchanged; only step 4 and the format are. +> +> What the server emits now is the framing plus the body verbatim - see `composeMarkdown` in `backend/internal/mcp/compose.go`: +> +> ```markdown +> # Frame: +> +> +> +> > Version: +> > Inherits from: @, @ +> > Resolved at: +> +> +> ``` +> +> `Version:` is new and load-bearing: it is the value `update_frame` requires as `base_version`, so a client that intends to edit has to be able to see it. `Inherits from:` is omitted when the Frame has no parents, and the whole body section when it is empty. The format is still deterministic and stable across requests. + When a client reads a resource (`resources/read`), the server: 1. Parses the URI to extract org / name / version (or "latest"). 2. Calls `frames.Service.GetResolved(ctx, frameID, version)` - returns the inheritance-merged Frame. 3. `frames.Service` consults `rbac.Can(caller, Read, frame)` first; 404 if denied. -4. Server composes resolved Frame slots into markdown. +4. ~~Server composes resolved Frame slots into markdown.~~ Server emits the resolved Frame's body with the provenance framing above. **Composition format** (deterministic; same shape across all readers): +~~The ten-slot form below was the original design. It is retained as the record of what the endpoint used to emit, since versions published under the slot schema still exist in storage and are folded into a body on read.~~ + ```markdown # Frame: @@ -200,7 +220,7 @@ When a client reads a resource (`resources/read`), the server: ``` -Empty slots are **omitted from the rendered markdown** (no empty headers). The format is stable across requests so the AI can rely on consistent structure. +~~Empty slots are **omitted from the rendered markdown** (no empty headers).~~ The format is stable across requests so the AI can rely on consistent structure. ### 3.5 Per-provider compatibility notes diff --git a/docs/design/2026-05-21-nebari-frames-migration.md b/docs/design/2026-05-21-nebari-frames-migration.md index 79dc9a2..6da57ba 100644 --- a/docs/design/2026-05-21-nebari-frames-migration.md +++ b/docs/design/2026-05-21-nebari-frames-migration.md @@ -319,15 +319,46 @@ slots: 1. Walk `extends` graph depth-first from the requested Frame version. Abort with error on cycle. 2. Remove any node whose `frame_id` appears in the requested Frame's `excludes`. -3. Merge slots in `extends` order (later wins): - - `terminology`: merge by `term`; later definition wins on collision. - - Typed-list-of-strings (`rules`, `skills`, `prompts`): concatenate, dedupe preserving last occurrence. - - Prose slots (and `tool_specs`): later parent's content replaces earlier entirely. -4. Apply the Frame's own slot values last; they override all parents. +3. Append each ancestor's body in `extends` order, ancestors first. A parent reachable through more + than one path (a diamond) contributes once, keyed by `ref@version`. +4. Append the Frame's own body last, so its guidance reads after everything it inherits. 5. Return the resolved Frame. Resolution at read time (not publish time) keeps storage simple. Pinned parent refs mean read-time results are stable until the child re-publishes. +##### Why precedence is reading order, and what that costs + +> **Supersedes step 3 above** ([#59](https://github.com/nebari-dev/nebari-frames/issues/59)), which +> merged ten typed slots. This is the trade that came with the free-form body, recorded here because +> it is a deliberate loss rather than an oversight. + +Per-slot merging gave a child two kinds of override that concatenation does not: + +- **`terminology` merged by key.** A parent defining `customer` and a child redefining it produced + one entry, the child's. Now both definitions appear in the resolved body, adjacent. +- **Prose slots replaced outright.** A child's `style` erased its parent's. Now both paragraphs + appear, parent first. + +Two things did *not* change, and are worth naming so this is not read as broader than it is. Rules, +skills, and prompts were already append-with-last-wins-dedupe, so fine-grained **removal** never +existed on either side - a child could add to a parent's rules or restate one, never delete one. And +`excludes` still works, at whole-ancestor granularity. + +**Why not keep per-section override.** It requires the body to have addressable sections, which is +precisely what Frame Spec v0.2 does not define. Reintroducing a fixed section vocabulary to support +override would rebuild the ten-slot schema under another name and give back the authoring rigidity +the free-form body exists to remove. + +**What we are betting on instead.** Later guidance overriding earlier guidance is a convention the +reader honors, not a guarantee the format enforces. That is a real assertion about model behavior, +and `mcp/compose.go` emits the concatenation flat, so a model sees both the parent's and the child's +`customer` definition with no marker saying which wins. Ordering is the only signal. + +**If the bet turns out badly**, the fix is narrower than restoring slots: `excludes` scoped to a +heading path, or an explicit `## Overrides` convention the composer understands. Neither needs the +schema back. The signal to watch for is resolved Frames where a child's correction is visibly not +taking effect. + #### Frame Spec metadata and the `.frame.md` interchange format `visibility`, `scope`, and `maintainer` are the [Frame Spec v0.2](https://github.com/openteams-ai/frame-spec) @@ -347,20 +378,29 @@ stateless `ConvertFrame` RPC and used for the web app's Markdown editor, import, | `visibility` / `scope` / `maintainer` | same keys | | `inherits: ["org/name@1.2.0", ...]` | `extends: [{ref, version}]` - split on the last `@` | | `x-nebari-excludes` | `excludes` (no spec equivalent; namespaced as the spec advises) | -| `## Terminology` -> `- **term**: definition` | `slots.terminology` | -| `## Rules` / `## Skills` / `## Prompts` | the matching list slots | -| `## Goals`, `## Style`, ... | the matching prose slots | +| everything after the closing `---` | `body`, verbatim | +| ~~`## Terminology` -> `- **term**: definition`~~ | ~~`slots.terminology`~~ | +| ~~`## Rules` / `## Skills` / `## Prompts`~~ | ~~the matching list slots~~ | +| ~~`## Goals`, `## Style`, ...~~ | ~~the matching prose slots~~ | Inheritance order agrees with the spec by coincidence rather than adaptation: the spec says later `inherits` entries win, which is what `resolver.go` already did for `extends`. -Section headings and ordering come from `frames.SlotTable`, shared with `mcp/compose.go`, so the two -markdown renderings cannot drift. `examples/*.frame.md` are checked-in golden files asserting both -`yaml -> md` output and `yaml -> md -> yaml` identity; they also pass the frame-spec project's own +> **Superseded by [#59](https://github.com/nebari-dev/nebari-frames/issues/59):** a Frame's content +> is a single free-form markdown `body`, matching Frame Spec v0.2, which defines no body structure. +> The struck-through rows above describe the retired ten-slot schema. Documents published under it +> are still readable - `backend/internal/frames/legacy.go` folds a `slots:` block into a body on +> read, and nothing writes that shape again. + +`examples/*.frame.md` are checked-in golden files asserting both `yaml -> md` output and +`yaml -> md -> yaml` identity; they also pass the frame-spec project's own `tools/validate_frames.py`. -Adding a slot therefore means editing `SlotTable`, `Slots`, `validate.go`, the two zod mirrors in -`web/src/lib/`, and regenerating the goldens - the codec and the MCP composer follow automatically. +~~Adding a slot therefore means editing `SlotTable`, `Slots`, `validate.go`, the two zod mirrors in +`web/src/lib/`, and regenerating the goldens - the codec and the MCP composer follow automatically.~~ +There are no slots to add. The one place the retired rendering still exists twice - Go's +`legacy.go` and the web's `frame-yaml.ts`, which renders a stored legacy version without a server +round trip - is pinned to the shared fixture in `testdata/legacy-slots/`. ### 3.5 RBAC model diff --git a/docs/design/2026-05-21-web-app-design.md b/docs/design/2026-05-21-web-app-design.md index 5c73d81..528a31a 100644 --- a/docs/design/2026-05-21-web-app-design.md +++ b/docs/design/2026-05-21-web-app-design.md @@ -165,11 +165,19 @@ takes the version from the document's own frontmatter, so the publish dialog off changelog there. Import (`/frames/new?import=1`, reached from the catalog's "Import .frame.md" button) lands straight in the Markdown editor with paste/drop/Load file. -**Editor kinds per section** are unchanged: terminology is a two-column row editor, rules/skills/ +~~**Editor kinds per section** are unchanged: terminology is a two-column row editor, rules/skills/ prompts are single-column row editors, prose sections are markdown textareas with a preview toggle. The section list (keys, labels, editor kind, hints) lives once in `web/src/lib/slot-sections.ts`, mirroring the Go `SlotTable`; the read-only renderer -(`FrameSlots`) and the editor both consume it. +(`FrameSlots`) and the editor both consume it.~~ + +> **Superseded by [#59](https://github.com/nebari-dev/nebari-frames/issues/59):** there are no +> sections to have editor kinds for. A Frame's content is a single free-form markdown body, +> matching Frame Spec v0.2, so authoring is one markdown editor with starter templates and the +> `.frame.md` source mode described above. `web/src/lib/slot-sections.ts` and the Go `SlotTable` +> it mirrored are both gone; `web/src/lib/frame-templates.ts` supplies the starting points that +> the per-section hints used to. Versions published under the ten-slot schema are still readable: +> the backend folds a legacy `slots:` block into a body on read (`backend/internal/frames/legacy.go`). **The view page mirrors the editor.** Frame Detail leads with identity (name, version badges, visibility/scope badges, description, maintainer, inherits chips linking to parents) and then the @@ -180,24 +188,28 @@ a small menu (Download `.frame.md` / Copy as Markdown) available to anyone who c **Validation feedback.** Two phases, as before: zod client-side, then server `FieldViolations` on publish. Every field renders its own message through the shared `FieldError` component -(`components/form/FieldError.tsx`), which also wires `aria-invalid` / `aria-describedby`. Server +(`components/form/FieldError.tsx`), which also wires `aria-invalid` / `aria-describedby`. ~~Server paths use bracket notation (`slots.rules[0]`) while inputs register dotted paths (`slots.rules.0`); react-hook-form's `get` resolves both to the same node, so they meet on the -input that caused them. Cycle detection in `extends` remains a form-level banner. +input that caused them.~~ (Superseded by [#59](https://github.com/nebari-dev/nebari-frames/issues/59): +with one body field there are no indexed slot paths to reconcile.) Cycle detection in `extends` +remains a form-level banner. **The `.frame.md` codec.** Conversion lives only in Go (`backend/internal/frames/framemd.go`) and is -reached through one stateless `ConvertFrame` RPC, so the slot table is not mirrored into TypeScript -a fifth time. `frames.SlotTable` is the single source of slot keys, markdown headings, and content -shape, shared with `mcp/compose.go`. Round-tripping is covered by a golden corpus over `examples/`. +reached through one stateless `ConvertFrame` RPC, so the codec is not mirrored into TypeScript. +Round-tripping is covered by a golden corpus over `examples/`. -Parsing is strict about **structure** and lenient about **values**: +Parsing is strict about the **frontmatter** and never rejects the **body**: -- *Structural* (blocks conversion): unknown `##` heading, unknown frontmatter key, malformed - terminology bullet, missing/unterminated frontmatter, bad `type`. Errors name the line and - suggest the closest slot (`unknown section "## Ways of Working" - did you mean "## Norms"?`). +- *Structural* (blocks conversion): unknown frontmatter key, missing or unterminated frontmatter, + bad `type`. Errors name the line. The frontmatter delimiter is matched only at column 0, so an + indented `---` inside a multi-line YAML value is content rather than a terminator. - *Value* (does not block): an unpinned or unqualified `inherits`, an empty description. These convert successfully and land as fixable inline errors in the form. +The body itself is free-form under Frame Spec v0.2, so there are no headings to validate and no +"did you mean" suggestions to make - whatever is after the closing `---` is the content. + That split is what makes import usable: the spec's own `examples/complete/frame.md` uses `inherits: editorial-style-guide` - bare name, no org, no pinned version - which a single strict gate would reject outright. Because slot bodies are delimited by `##`, an author must use `###` or @@ -221,7 +233,7 @@ Frame Detail (`/frames/:org/:name`) is the highest-value reading screen: - Header: name, description, version, owner, "Edit" / "Delete" buttons (visible per server-returned permissions). - **"Use this Frame" panel** (right rail on desktop, top section on mobile): one-click links to per-provider Connect pages; code block showing the MCP resource URI for users who know what to do with it. - **Inheritance trail**: visual representation of the `extends` chain. Each parent is clickable and links to its detail page. -- **Slot rendering**: all populated slots rendered in their typed form (terminology as a definition list; rules / skills / prompts as bullet lists; prose slots as rendered markdown). Empty slots hidden. Each section collapsible. +- ~~**Slot rendering**: all populated slots rendered in their typed form (terminology as a definition list; rules / skills / prompts as bullet lists; prose slots as rendered markdown). Empty slots hidden. Each section collapsible.~~ **Superseded by [#59](https://github.com/nebari-dev/nebari-frames/issues/59):** the body renders as markdown, whatever structure its author gave it. The detail page is the authoring form rendered read-only, so reading and editing are one surface rather than two renderers to keep in step. - **Version history**: collapsed by default; expandable to see all published versions with timestamps and changelogs. Each version row links to its read-only detail page (no in-app diff in MVP; roadmap). ### 3.6 Per-provider Connect pages diff --git a/docs/qa/mcp-endpoint-manual-qa.md b/docs/qa/mcp-endpoint-manual-qa.md index 2c5c3db..7dda528 100644 --- a/docs/qa/mcp-endpoint-manual-qa.md +++ b/docs/qa/mcp-endpoint-manual-qa.md @@ -103,23 +103,28 @@ Notes: ### 1.2 Seed two readable frames (dev mode needs no token) The FrameService takes the Frame YAML as base64 bytes over Connect JSON. Seed a -content-rich frame (so the read shows multiple markdown sections) and a second +content-rich frame (so the read shows a real markdown body) and a second plain one (so the list shows more than one): ```bash -# Frame 1: brand-voice (terminology + rules + goals) +# Frame 1: brand-voice (multi-section markdown body) YAML1=$(cat <<'EOF' name: brand-voice description: How we speak to customers. version: 1.0.0 -slots: - terminology: - - term: Frame - definition: A scoped context artifact. - rules: - - Be concise and concrete. - - Prefer active voice. - goals: Sound human, not corporate. +body: | + ## Terminology + + - **Frame**: A scoped context artifact. + + ## Rules + + - Be concise and concrete. + - Prefer active voice. + + ## Goals + + Sound human, not corporate. EOF ) curl -s localhost:8080/frames.v1.FrameService/PublishFrame \ @@ -132,9 +137,8 @@ YAML2=$(cat <<'EOF' name: support-tone description: Tone for support replies. version: 1.0.0 -slots: - rules: - - Acknowledge the issue first. +body: | + Acknowledge the issue first. EOF ) curl -s localhost:8080/frames.v1.FrameService/PublishFrame \ @@ -202,9 +206,8 @@ In the Inspector UI: Sound human, not corporate. ``` - Confirm: named sections in fixed order; EMPTY slots (Skills, Style, Norms, - etc.) are OMITTED entirely (no empty headers); no `> Inherits from:` line for - this non-inheriting frame. + Confirm: the body renders verbatim after the provenance header; no + `> Inherits from:` line for this non-inheriting frame. > Alternative without Inspector (Claude Code as the client): > `claude mcp add --transport http frames-dev http://localhost:8080/mcp` @@ -367,7 +370,7 @@ Journeys 1, 6, 2 (negative), 7 (denied). - [ ] (T1, J5) `/.well-known/oauth-protected-resource` returns resource=`/mcp`, the issuer, and non-empty scopes. - [ ] (T1, J8) MCP Inspector connects to `/mcp` with no token. - [ ] (T1, J2+) List shows the seeded frames with `name (Org)` labels and `nebari-frame://` URIs. -- [ ] (T1, J3) Read returns `text/markdown` with named sections, empty slots omitted. +- [ ] (T1, J3) Read returns `text/markdown` with the frame body verbatim under the provenance header. - [ ] (T1, J7) Reading a nonexistent / malformed URI returns not-found. - [ ] (T2, J4) Tokenless POST `/mcp` -> 401 with `WWW-Authenticate` containing `resource_metadata`. - [ ] (T3, J1) Claude.ai connector completes OAuth and lists + reads frames (`docs/connect/claude-ai.md`). diff --git a/docs/site/src/content/docs/local-development.md b/docs/site/src/content/docs/local-development.md index 29927f2..c4aa846 100644 --- a/docs/site/src/content/docs/local-development.md +++ b/docs/site/src/content/docs/local-development.md @@ -12,7 +12,7 @@ Two loops cover local development, both documented in the project [Makefile](htt make dev ``` -Runs the backend in dev mode (no OIDC) on `:8080` and the Vite dev server on `:5173`, seeded with representative sample data: an org, members across roles, and Frames with full slot content, multi-level inheritance, and versions. Open **http://localhost:5173** - UI edits hot-reload. A single **Ctrl-C** stops both processes. +Runs the backend in dev mode (no OIDC) on `:8080` and the Vite dev server on `:5173`, seeded with representative sample data: an org, members across roles, and Frames with real body content, multi-level inheritance, and versions. Open **http://localhost:5173** - UI edits hot-reload. A single **Ctrl-C** stops both processes. There is no login step in this loop. Dev mode disables OIDC and injects a fixed identity, so you land straight in the app as `dev-user`, an org admin, and never hit the "No organization access" screen (see [Troubleshooting](/troubleshooting/)). diff --git a/examples/brand-voice.frame.md b/examples/brand-voice.frame.md index 2d421db..987d793 100644 --- a/examples/brand-voice.frame.md +++ b/examples/brand-voice.frame.md @@ -8,8 +8,6 @@ scope: company maintainer: marketing --- -# brand-voice - ## Terminology - **customer**: An enterprise organization that has deployed an Intelligence Hub. diff --git a/examples/brand-voice.yaml b/examples/brand-voice.yaml index 657f0cf..b0058f2 100644 --- a/examples/brand-voice.yaml +++ b/examples/brand-voice.yaml @@ -4,16 +4,21 @@ version: 1.0.0 visibility: internal scope: company maintainer: marketing -slots: - terminology: - - term: customer - definition: An enterprise organization that has deployed an Intelligence Hub. - - term: hub - definition: A deployed Nebari instance. - rules: +body: |- + ## Terminology + + - **customer**: An enterprise organization that has deployed an Intelligence Hub. + - **hub**: A deployed Nebari instance. + + ## Rules + - Never claim performance numbers without a benchmark citation. - Avoid the word "revolutionary" in customer-facing copy. - goals: | + + ## Goals + Communicate capability with precision and restraint. - style: | + + ## Style + Plain, direct, technically credible. Short sentences. diff --git a/examples/healthcare-compliance.frame.md b/examples/healthcare-compliance.frame.md index 380816b..233efc0 100644 --- a/examples/healthcare-compliance.frame.md +++ b/examples/healthcare-compliance.frame.md @@ -8,8 +8,6 @@ scope: department maintainer: legal --- -# healthcare-compliance - ## Rules - Never state or imply HIPAA compliance without legal sign-off. diff --git a/examples/healthcare-compliance.yaml b/examples/healthcare-compliance.yaml index e63d45a..d84abe0 100644 --- a/examples/healthcare-compliance.yaml +++ b/examples/healthcare-compliance.yaml @@ -4,11 +4,16 @@ version: 1.0.0 visibility: internal scope: department maintainer: legal -slots: - rules: +body: |- + ## Rules + - Never state or imply HIPAA compliance without legal sign-off. - Do not include PHI in any generated artifact. - norms: | + + ## Norms + When uncertain about a regulatory claim, flag it for SME review rather than asserting it. - business_process: | + + ## Business Process + All externally-facing compliance statements route through the compliance officer before release. diff --git a/examples/nebari-platform.frame.md b/examples/nebari-platform.frame.md index 5a103a6..2853561 100644 --- a/examples/nebari-platform.frame.md +++ b/examples/nebari-platform.frame.md @@ -8,8 +8,6 @@ scope: company maintainer: platform engineering --- -# nebari-platform - ## Terminology - **Nebari**: An open-source platform for deploying and operating data science and AI infrastructure on Kubernetes, inside the client's own cloud account or data center. Stewarded by OpenTeams and the nebari-dev community. diff --git a/examples/nebari-platform.yaml b/examples/nebari-platform.yaml index 37e937e..f1caa31 100644 --- a/examples/nebari-platform.yaml +++ b/examples/nebari-platform.yaml @@ -1,157 +1,58 @@ -# Frame: nebari-platform -# -# The first hand-authored example Frame (see docs/design/2026-05-21-nebari-frames-migration.md §5.3). -# Audience: sales teams giving Claude context when drafting proposals for potential clients. -# Source material: nebari-infrastructure-core design docs, nebari-operator API, -# software-pack-dashboard tracked-packs.yaml, and internal positioning documents. - name: nebari-platform -description: >- - What Nebari is today: Nebari Infrastructure Core, the Nebari Operator, and the - software pack catalog. Context for writing accurate client proposals: value - proposition, vocabulary, and guardrails for the today-vs-roadmap line. +description: 'What Nebari is today: Nebari Infrastructure Core, the Nebari Operator, and the software pack catalog. Context for writing accurate client proposals: value proposition, vocabulary, and guardrails for the today-vs-roadmap line.' version: 0.1.0 visibility: shared scope: company maintainer: platform engineering +body: |- + ## Terminology + + - **Nebari**: An open-source platform for deploying and operating data science and AI infrastructure on Kubernetes, inside the client's own cloud account or data center. Stewarded by OpenTeams and the nebari-dev community. + - **Nebari Infrastructure Core (NIC)**: The deployment engine. A Go CLI that reads a single config.yaml and provisions a production-ready Kubernetes cluster plus platform services using OpenTofu modules. Commands include deploy, destroy, status, and validate. + - **Software Pack**: A curated, versioned Helm chart that deploys a workload (JupyterHub, MLflow, LLM serving, Superset, and others) onto a Nebari cluster with platform integration included: routing, single sign-on, TLS, and landing-page registration. + - **Nebari Operator**: A Kubernetes operator that watches NebariApp resources and wires each application into the platform automatically: HTTPRoute creation through Envoy Gateway, OIDC authentication through Keycloak, TLS certificates, and landing-page registration. + - **NebariApp**: The custom resource a pack (or any application) creates to join the platform. Declaring a hostname and backend service is enough to get routing, auth, and TLS without writing any of that configuration by hand. + - **foundational software**: The platform services NIC installs on every cluster via GitOps: cert-manager, Envoy Gateway, Keycloak, the LGTM observability stack, and the Nebari Operator. + - **GitOps**: The operating model for everything above the infrastructure layer. The desired state of the platform lives in a Git repository the client owns; ArgoCD continuously reconciles the cluster against it. Every change is a reviewable commit. + - **OpenTofu**: The open-source infrastructure-as-code engine (a Terraform fork under the Linux Foundation) that NIC drives to provision cloud resources. State lives in standard backends the client controls. + - **LGTM stack**: The observability suite deployed on every cluster: Loki for logs, Grafana for dashboards, Tempo for traces, Mimir for metrics, fed by an OpenTelemetry Collector pipeline. + - **Keycloak**: The open-source identity provider deployed with the platform. Provides single sign-on across all deployed services and federates with the client's existing identity provider (Active Directory, Okta, and other OIDC or SAML systems). + - **Envoy Gateway**: The ingress layer, built on the Kubernetes Gateway API. All traffic to platform services routes through it, which is where authentication policies and TLS are enforced. + - **Intelligence Hub**: OpenTeams' product direction: a governed environment, built on Nebari, where an organization's AI context, workers, and workflows are owned and auditable artifacts. Direction, not a current deliverable; see rules before referencing it in proposals. + - **Nebari Classic**: The earlier generation of Nebari that ships the full data science stack as a single opinionated deployment. The current platform supersedes it with a composable architecture; avoid the term in proposals unless the client already uses Nebari Classic. + + ## Rules + + - Only present as deliverables what ships today: NIC-provisioned Kubernetes clusters, the foundational software, the Nebari Operator, and the tracked software packs. Nothing else is committable. + - Intelligence Hubs, Frames, Cogs, Ops, the marketplace, and the Desktop Application are product direction. Mention them only when the client asks where the platform is heading, clearly labeled as direction, with no dates and no commitments. + - Never reproduce confidential internal material in client-facing output: competitive moat analysis, network flywheel framing, competitor comparison tables, strategic roadmap timelines, or investor messaging. + - Never make performance, scale, or cost claims without a citable source. If no source exists, describe the capability without quantifying it. + - Never disparage competitors or name them negatively. Position Nebari on its own strengths: ownership, open source, composability, auditability. + - Pricing, delivery timelines, and contractual scope come from the account team. Leave them as clearly marked placeholders for humans to fill in. + - The data-engineering pack is work in progress. Do not present it as a turnkey deliverable. + - Every capability claim in a proposal must name the component that delivers it (NIC, the operator, a specific pack). If no component delivers it, the claim does not go in. + + ## Skills -slots: - terminology: - - term: Nebari - definition: >- - An open-source platform for deploying and operating data science and AI - infrastructure on Kubernetes, inside the client's own cloud account or - data center. Stewarded by OpenTeams and the nebari-dev community. - - term: Nebari Infrastructure Core (NIC) - definition: >- - The deployment engine. A Go CLI that reads a single config.yaml and - provisions a production-ready Kubernetes cluster plus platform services - using OpenTofu modules. Commands include deploy, destroy, status, and - validate. - - term: Software Pack - definition: >- - A curated, versioned Helm chart that deploys a workload (JupyterHub, - MLflow, LLM serving, Superset, and others) onto a Nebari cluster with - platform integration included: routing, single sign-on, TLS, and - landing-page registration. - - term: Nebari Operator - definition: >- - A Kubernetes operator that watches NebariApp resources and wires each - application into the platform automatically: HTTPRoute creation through - Envoy Gateway, OIDC authentication through Keycloak, TLS certificates, - and landing-page registration. - - term: NebariApp - definition: >- - The custom resource a pack (or any application) creates to join the - platform. Declaring a hostname and backend service is enough to get - routing, auth, and TLS without writing any of that configuration by - hand. - - term: foundational software - definition: >- - The platform services NIC installs on every cluster via GitOps: - cert-manager, Envoy Gateway, Keycloak, the LGTM observability stack, - and the Nebari Operator. - - term: GitOps - definition: >- - The operating model for everything above the infrastructure layer. The - desired state of the platform lives in a Git repository the client - owns; ArgoCD continuously reconciles the cluster against it. Every - change is a reviewable commit. - - term: OpenTofu - definition: >- - The open-source infrastructure-as-code engine (a Terraform fork under - the Linux Foundation) that NIC drives to provision cloud resources. - State lives in standard backends the client controls. - - term: LGTM stack - definition: >- - The observability suite deployed on every cluster: Loki for logs, - Grafana for dashboards, Tempo for traces, Mimir for metrics, fed by an - OpenTelemetry Collector pipeline. - - term: Keycloak - definition: >- - The open-source identity provider deployed with the platform. Provides - single sign-on across all deployed services and federates with the - client's existing identity provider (Active Directory, Okta, and other - OIDC or SAML systems). - - term: Envoy Gateway - definition: >- - The ingress layer, built on the Kubernetes Gateway API. All traffic to - platform services routes through it, which is where authentication - policies and TLS are enforced. - - term: Intelligence Hub - definition: >- - OpenTeams' product direction: a governed environment, built on Nebari, - where an organization's AI context, workers, and workflows are owned - and auditable artifacts. Direction, not a current deliverable; see - rules before referencing it in proposals. - - term: Nebari Classic - definition: >- - The earlier generation of Nebari that ships the full data science stack - as a single opinionated deployment. The current platform supersedes it - with a composable architecture; avoid the term in proposals unless the - client already uses Nebari Classic. - - rules: - - >- - Only present as deliverables what ships today: NIC-provisioned Kubernetes - clusters, the foundational software, the Nebari Operator, and the tracked - software packs. Nothing else is committable. - - >- - Intelligence Hubs, Frames, Cogs, Ops, the marketplace, and the Desktop - Application are product direction. Mention them only when the client asks - where the platform is heading, clearly labeled as direction, with no - dates and no commitments. - - >- - Never reproduce confidential internal material in client-facing output: - competitive moat analysis, network flywheel framing, competitor - comparison tables, strategic roadmap timelines, or investor messaging. - - >- - Never make performance, scale, or cost claims without a citable source. - If no source exists, describe the capability without quantifying it. - - >- - Never disparage competitors or name them negatively. Position Nebari on - its own strengths: ownership, open source, composability, auditability. - - >- - Pricing, delivery timelines, and contractual scope come from the account - team. Leave them as clearly marked placeholders for humans to fill in. - - >- - The data-engineering pack is work in progress. Do not present it as a - turnkey deliverable. - - >- - Every capability claim in a proposal must name the component that - delivers it (NIC, the operator, a specific pack). If no component - delivers it, the claim does not go in. - - skills: - proposal-writing - technical-writing - prompts: - - >- - Lead proposals with the client's problem stated in their own terms. - Introduce Nebari as the answer to that problem, not as the topic. - - >- - Where it honestly fits the client's situation, connect their pain to the - costs of rented AI and data infrastructure: vendor lock-in, compliance - exposure when data leaves their perimeter, no ability to audit or - reproduce AI behavior, and institutional knowledge accumulating in a - vendor's logs instead of their own systems. - - >- - When listing capabilities, tie each one to the named component that - delivers it, so the proposal reads as an engineering plan rather than a - brochure. - - >- - If the client asks about future direction, give the Intelligence Hub - story in one short paragraph, label it as direction, and return to what - ships today. + ## Prompts + + - Lead proposals with the client's problem stated in their own terms. Introduce Nebari as the answer to that problem, not as the topic. + - Where it honestly fits the client's situation, connect their pain to the costs of rented AI and data infrastructure: vendor lock-in, compliance exposure when data leaves their perimeter, no ability to audit or reproduce AI behavior, and institutional knowledge accumulating in a vendor's logs instead of their own systems. + - When listing capabilities, tie each one to the named component that delivers it, so the proposal reads as an engineering plan rather than a brochure. + - If the client asks about future direction, give the Intelligence Hub story in one short paragraph, label it as direction, and return to what ships today. + + ## Tool Specifications - tool_specs: | No tools are required to use this Frame. Web access is helpful for citing https://nebari.dev and the public pack repositories under https://github.com/nebari-dev when a proposal needs links or version details. - goals: | + ## Goals + A good Nebari proposal does four things: 1. Shows the client we understood their actual problem: the workloads they @@ -170,7 +71,8 @@ slots: wins is the goal; an inflated one that wins is a failure that surfaces three months later. - style: | + ## Style + Plain, concrete, and confident without hype. Short sentences. Name real components instead of using category buzzwords: "the operator creates an OAuth client in Keycloak" lands better than "seamless enterprise-grade @@ -180,7 +82,8 @@ slots: Spell out an acronym at first use. Use regular dashes and colons for punctuation, never em dashes. - norms: | + ## Norms + Engineering reviews the technical scope of every proposal before it goes to the client; the frame gives you accurate raw material, but a human who has deployed Nebari validates the specific commitments. Proposals state @@ -189,7 +92,8 @@ slots: When a proposal needs a capability that no current component delivers, flag it to the account team as a gap rather than writing around it. - architecture: | + ## Architecture + Nebari today is a layered, open-source platform. Each layer is independently useful and the client owns all of them. @@ -257,7 +161,8 @@ slots: platform is heading, that is the one-paragraph answer. It is direction, not a deliverable. - business_process: | + ## Business Process + A typical Nebari engagement runs in five stages: 1. **Discovery.** Understand the client's workloads (notebooks, model diff --git a/examples/q4-sales-playbook.frame.md b/examples/q4-sales-playbook.frame.md index 14edd1b..7ac67fd 100644 --- a/examples/q4-sales-playbook.frame.md +++ b/examples/q4-sales-playbook.frame.md @@ -10,8 +10,6 @@ inherits: - openteams/brand-voice@1.0.0 --- -# q4-sales-playbook - ## Rules - Qualify on deployment ownership before discussing pricing. diff --git a/examples/q4-sales-playbook.yaml b/examples/q4-sales-playbook.yaml index a0e9d09..7dbd9e8 100644 --- a/examples/q4-sales-playbook.yaml +++ b/examples/q4-sales-playbook.yaml @@ -5,12 +5,17 @@ visibility: internal scope: department maintainer: revenue extends: - - ref: openteams/brand-voice - version: 1.0.0 -slots: - prompts: - - When summarizing a release, lead with customer impact, not the feature list. - rules: + - ref: openteams/brand-voice + version: 1.0.0 +body: |- + ## Rules + - Qualify on deployment ownership before discussing pricing. - goals: | + + ## Prompts + + - When summarizing a release, lead with customer impact, not the feature list. + + ## Goals + Move qualified enterprise accounts from evaluation to signed pilot within the quarter. diff --git a/gen/go/frames/v1/frame.pb.go b/gen/go/frames/v1/frame.pb.go index 0350643..0074133 100644 --- a/gen/go/frames/v1/frame.pb.go +++ b/gen/go/frames/v1/frame.pb.go @@ -176,6 +176,9 @@ type Frame struct { LatestVersion string `protobuf:"bytes,6,opt,name=latest_version,json=latestVersion,proto3" json:"latest_version,omitempty"` CreatedAt *timestamppb.Timestamp `protobuf:"bytes,7,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` UpdatedAt *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=updated_at,json=updatedAt,proto3" json:"updated_at,omitempty"` + // Offered as a starting point in the "start from a template" picker. + // Denormalized from the latest version's `template` field at publish time. + IsTemplate bool `protobuf:"varint,9,opt,name=is_template,json=isTemplate,proto3" json:"is_template,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -266,6 +269,13 @@ func (x *Frame) GetUpdatedAt() *timestamppb.Timestamp { return nil } +func (x *Frame) GetIsTemplate() bool { + if x != nil { + return x.IsTemplate + } + return false +} + type ParentRef struct { state protoimpl.MessageState `protogen:"open.v1"` Ref string `protobuf:"bytes,1,opt,name=ref,proto3" json:"ref,omitempty"` // org_slug/frame_name @@ -471,6 +481,7 @@ type FrameSummary struct { LatestVersion string `protobuf:"bytes,5,opt,name=latest_version,json=latestVersion,proto3" json:"latest_version,omitempty"` UpdatedAt *timestamppb.Timestamp `protobuf:"bytes,6,opt,name=updated_at,json=updatedAt,proto3" json:"updated_at,omitempty"` Permissions *Permissions `protobuf:"bytes,7,opt,name=permissions,proto3" json:"permissions,omitempty"` + IsTemplate bool `protobuf:"varint,8,opt,name=is_template,json=isTemplate,proto3" json:"is_template,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -554,6 +565,13 @@ func (x *FrameSummary) GetPermissions() *Permissions { return nil } +func (x *FrameSummary) GetIsTemplate() bool { + if x != nil { + return x.IsTemplate + } + return false +} + type FrameVersionSummary struct { state protoimpl.MessageState `protogen:"open.v1"` Version string `protobuf:"bytes,1,opt,name=version,proto3" json:"version,omitempty"` @@ -639,7 +657,7 @@ const file_frames_v1_frame_proto_rawDesc = "" + "\buser_sub\x18\x02 \x01(\tR\auserSub\x12\x12\n" + "\x04role\x18\x03 \x01(\tR\x04role\x125\n" + "\badded_at\x18\x04 \x01(\v2\x1a.google.protobuf.TimestampR\aaddedAt\x12\x14\n" + - "\x05email\x18\x05 \x01(\tR\x05email\"\x9e\x02\n" + + "\x05email\x18\x05 \x01(\tR\x05email\"\xbf\x02\n" + "\x05Frame\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12\x15\n" + "\x06org_id\x18\x02 \x01(\tR\x05orgId\x12\x12\n" + @@ -650,7 +668,9 @@ const file_frames_v1_frame_proto_rawDesc = "" + "\n" + "created_at\x18\a \x01(\v2\x1a.google.protobuf.TimestampR\tcreatedAt\x129\n" + "\n" + - "updated_at\x18\b \x01(\v2\x1a.google.protobuf.TimestampR\tupdatedAt\"7\n" + + "updated_at\x18\b \x01(\v2\x1a.google.protobuf.TimestampR\tupdatedAt\x12\x1f\n" + + "\vis_template\x18\t \x01(\bR\n" + + "isTemplate\"7\n" + "\tParentRef\x12\x10\n" + "\x03ref\x18\x01 \x01(\tR\x03ref\x12\x18\n" + "\aversion\x18\x02 \x01(\tR\aversion\"\xf9\x01\n" + @@ -666,7 +686,7 @@ const file_frames_v1_frame_proto_rawDesc = "" + "\vPermissions\x12\x19\n" + "\bcan_edit\x18\x01 \x01(\bR\acanEdit\x12\x1d\n" + "\n" + - "can_delete\x18\x02 \x01(\bR\tcanDelete\"\x98\x02\n" + + "can_delete\x18\x02 \x01(\bR\tcanDelete\"\xb9\x02\n" + "\fFrameSummary\x12\x19\n" + "\borg_slug\x18\x01 \x01(\tR\aorgSlug\x12\x12\n" + "\x04name\x18\x02 \x01(\tR\x04name\x12 \n" + @@ -675,7 +695,9 @@ const file_frames_v1_frame_proto_rawDesc = "" + "\x0elatest_version\x18\x05 \x01(\tR\rlatestVersion\x129\n" + "\n" + "updated_at\x18\x06 \x01(\v2\x1a.google.protobuf.TimestampR\tupdatedAt\x128\n" + - "\vpermissions\x18\a \x01(\v2\x16.frames.v1.PermissionsR\vpermissions\"\xaf\x01\n" + + "\vpermissions\x18\a \x01(\v2\x16.frames.v1.PermissionsR\vpermissions\x12\x1f\n" + + "\vis_template\x18\b \x01(\bR\n" + + "isTemplate\"\xaf\x01\n" + "\x13FrameVersionSummary\x12\x18\n" + "\aversion\x18\x01 \x01(\tR\aversion\x12\x1c\n" + "\tchangelog\x18\x02 \x01(\tR\tchangelog\x12!\n" + diff --git a/gen/go/frames/v1/frame_service.pb.go b/gen/go/frames/v1/frame_service.pb.go index 4db8e32..cfa2cc8 100644 --- a/gen/go/frames/v1/frame_service.pb.go +++ b/gen/go/frames/v1/frame_service.pb.go @@ -23,7 +23,7 @@ const ( type PublishFrameRequest struct { state protoimpl.MessageState `protogen:"open.v1"` - Content []byte `protobuf:"bytes,1,opt,name=content,proto3" json:"content,omitempty"` // full YAML; name/version/extends/excludes/slots parsed server-side + Content []byte `protobuf:"bytes,1,opt,name=content,proto3" json:"content,omitempty"` // full YAML; metadata/extends/excludes/body parsed server-side Changelog string `protobuf:"bytes,2,opt,name=changelog,proto3" json:"changelog,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache @@ -662,7 +662,7 @@ func (x *ListFrameVersionsResponse) GetVersions() []*FrameVersionSummary { } // FieldViolation is one validation failure at a specific field path -// (e.g. "slots.terminology[2].definition", matching backend validate.go paths). +// (e.g. "extends[0].version", matching backend validate.go paths). type FieldViolation struct { state protoimpl.MessageState `protogen:"open.v1"` Field string `protobuf:"bytes,1,opt,name=field,proto3" json:"field,omitempty"` @@ -764,7 +764,7 @@ func (x *FieldViolations) GetViolations() []*FieldViolation { } // ConvertFrame translates between the two representations of the same frame: -// the canonical slot YAML stored in frame_versions.content, and the single +// the canonical YAML stored in frame_versions.content, and the single // Markdown file with YAML frontmatter defined by Frame Spec v0.2. It backs the // web app's Markdown editor, .frame.md import, and .frame.md export. type ConvertFrameRequest struct { diff --git a/gen/go/frames/v1/framesv1connect/frame_service.connect.go b/gen/go/frames/v1/framesv1connect/frame_service.connect.go index 4c4c754..f7308c0 100644 --- a/gen/go/frames/v1/framesv1connect/frame_service.connect.go +++ b/gen/go/frames/v1/framesv1connect/frame_service.connect.go @@ -84,7 +84,7 @@ type FrameServiceClient interface { ListFrameVersions(context.Context, *connect.Request[v1.ListFrameVersionsRequest]) (*connect.Response[v1.ListFrameVersionsResponse], error) // Write - delete a frame. Blocks if the frame is a parent unless force=true. DeleteFrame(context.Context, *connect.Request[v1.DeleteFrameRequest]) (*connect.Response[v1.DeleteFrameResponse], error) - // Pure conversion between the canonical slot YAML and the spec-conformant + // Pure conversion between the canonical YAML and the spec-conformant // .frame.md form. Stateless and unauthenticated beyond org membership. ConvertFrame(context.Context, *connect.Request[v1.ConvertFrameRequest]) (*connect.Response[v1.ConvertFrameResponse], error) // Admin only - list the caller's org members. @@ -275,7 +275,7 @@ type FrameServiceHandler interface { ListFrameVersions(context.Context, *connect.Request[v1.ListFrameVersionsRequest]) (*connect.Response[v1.ListFrameVersionsResponse], error) // Write - delete a frame. Blocks if the frame is a parent unless force=true. DeleteFrame(context.Context, *connect.Request[v1.DeleteFrameRequest]) (*connect.Response[v1.DeleteFrameResponse], error) - // Pure conversion between the canonical slot YAML and the spec-conformant + // Pure conversion between the canonical YAML and the spec-conformant // .frame.md form. Stateless and unauthenticated beyond org membership. ConvertFrame(context.Context, *connect.Request[v1.ConvertFrameRequest]) (*connect.Response[v1.ConvertFrameResponse], error) // Admin only - list the caller's org members. diff --git a/gen/ts/frames/v1/frame_pb.ts b/gen/ts/frames/v1/frame_pb.ts index 90e4dd1..7439bb2 100644 --- a/gen/ts/frames/v1/frame_pb.ts +++ b/gen/ts/frames/v1/frame_pb.ts @@ -12,7 +12,7 @@ import type { Message } from "@bufbuild/protobuf"; * Describes the file frames/v1/frame.proto. */ export const file_frames_v1_frame: GenFile = /*@__PURE__*/ - fileDesc("ChVmcmFtZXMvdjEvZnJhbWUucHJvdG8SCWZyYW1lcy52MSJlCgNPcmcSCgoCaWQYASABKAkSDAoEc2x1ZxgCIAEoCRIUCgxkaXNwbGF5X25hbWUYAyABKAkSLgoKY3JlYXRlZF9hdBgEIAEoCzIaLmdvb2dsZS5wcm90b2J1Zi5UaW1lc3RhbXAieQoKTWVtYmVyc2hpcBIOCgZvcmdfaWQYASABKAkSEAoIdXNlcl9zdWIYAiABKAkSDAoEcm9sZRgDIAEoCRIsCghhZGRlZF9hdBgEIAEoCzIaLmdvb2dsZS5wcm90b2J1Zi5UaW1lc3RhbXASDQoFZW1haWwYBSABKAki0QEKBUZyYW1lEgoKAmlkGAEgASgJEg4KBm9yZ19pZBgCIAEoCRIMCgRuYW1lGAMgASgJEhMKC2Rlc2NyaXB0aW9uGAQgASgJEhEKCW93bmVyX3N1YhgFIAEoCRIWCg5sYXRlc3RfdmVyc2lvbhgGIAEoCRIuCgpjcmVhdGVkX2F0GAcgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIuCgp1cGRhdGVkX2F0GAggASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcCIpCglQYXJlbnRSZWYSCwoDcmVmGAEgASgJEg8KB3ZlcnNpb24YAiABKAkirwEKDEZyYW1lVmVyc2lvbhIPCgd2ZXJzaW9uGAEgASgJEhEKCWNoYW5nZWxvZxgCIAEoCRIOCgZkaWdlc3QYAyABKAkSEgoKc2l6ZV9ieXRlcxgEIAEoAxIUCgxwdWJsaXNoZWRfYnkYBSABKAkSMAoMcHVibGlzaGVkX2F0GAYgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIPCgdjb250ZW50GAcgASgMIjMKC1Blcm1pc3Npb25zEhAKCGNhbl9lZGl0GAEgASgIEhIKCmNhbl9kZWxldGUYAiABKAgiywEKDEZyYW1lU3VtbWFyeRIQCghvcmdfc2x1ZxgBIAEoCRIMCgRuYW1lGAIgASgJEhMKC2Rlc2NyaXB0aW9uGAMgASgJEhEKCW93bmVyX3N1YhgEIAEoCRIWCg5sYXRlc3RfdmVyc2lvbhgFIAEoCRIuCgp1cGRhdGVkX2F0GAYgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIrCgtwZXJtaXNzaW9ucxgHIAEoCzIWLmZyYW1lcy52MS5QZXJtaXNzaW9ucyKBAQoTRnJhbWVWZXJzaW9uU3VtbWFyeRIPCgd2ZXJzaW9uGAEgASgJEhEKCWNoYW5nZWxvZxgCIAEoCRIUCgxwdWJsaXNoZWRfYnkYAyABKAkSMAoMcHVibGlzaGVkX2F0GAQgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcEKfAQoNY29tLmZyYW1lcy52MUIKRnJhbWVQcm90b1ABWj1naXRodWIuY29tL25lYmFyaS1kZXYvbmViYXJpLWZyYW1lcy9nZW4vZ28vZnJhbWVzL3YxO2ZyYW1lc3YxogIDRlhYqgIJRnJhbWVzLlYxygIJRnJhbWVzXFYx4gIVRnJhbWVzXFYxXEdQQk1ldGFkYXRh6gIKRnJhbWVzOjpWMWIGcHJvdG8z", [file_google_protobuf_timestamp]); + fileDesc("ChVmcmFtZXMvdjEvZnJhbWUucHJvdG8SCWZyYW1lcy52MSJlCgNPcmcSCgoCaWQYASABKAkSDAoEc2x1ZxgCIAEoCRIUCgxkaXNwbGF5X25hbWUYAyABKAkSLgoKY3JlYXRlZF9hdBgEIAEoCzIaLmdvb2dsZS5wcm90b2J1Zi5UaW1lc3RhbXAieQoKTWVtYmVyc2hpcBIOCgZvcmdfaWQYASABKAkSEAoIdXNlcl9zdWIYAiABKAkSDAoEcm9sZRgDIAEoCRIsCghhZGRlZF9hdBgEIAEoCzIaLmdvb2dsZS5wcm90b2J1Zi5UaW1lc3RhbXASDQoFZW1haWwYBSABKAki5gEKBUZyYW1lEgoKAmlkGAEgASgJEg4KBm9yZ19pZBgCIAEoCRIMCgRuYW1lGAMgASgJEhMKC2Rlc2NyaXB0aW9uGAQgASgJEhEKCW93bmVyX3N1YhgFIAEoCRIWCg5sYXRlc3RfdmVyc2lvbhgGIAEoCRIuCgpjcmVhdGVkX2F0GAcgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIuCgp1cGRhdGVkX2F0GAggASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBITCgtpc190ZW1wbGF0ZRgJIAEoCCIpCglQYXJlbnRSZWYSCwoDcmVmGAEgASgJEg8KB3ZlcnNpb24YAiABKAkirwEKDEZyYW1lVmVyc2lvbhIPCgd2ZXJzaW9uGAEgASgJEhEKCWNoYW5nZWxvZxgCIAEoCRIOCgZkaWdlc3QYAyABKAkSEgoKc2l6ZV9ieXRlcxgEIAEoAxIUCgxwdWJsaXNoZWRfYnkYBSABKAkSMAoMcHVibGlzaGVkX2F0GAYgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIPCgdjb250ZW50GAcgASgMIjMKC1Blcm1pc3Npb25zEhAKCGNhbl9lZGl0GAEgASgIEhIKCmNhbl9kZWxldGUYAiABKAgi4AEKDEZyYW1lU3VtbWFyeRIQCghvcmdfc2x1ZxgBIAEoCRIMCgRuYW1lGAIgASgJEhMKC2Rlc2NyaXB0aW9uGAMgASgJEhEKCW93bmVyX3N1YhgEIAEoCRIWCg5sYXRlc3RfdmVyc2lvbhgFIAEoCRIuCgp1cGRhdGVkX2F0GAYgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcBIrCgtwZXJtaXNzaW9ucxgHIAEoCzIWLmZyYW1lcy52MS5QZXJtaXNzaW9ucxITCgtpc190ZW1wbGF0ZRgIIAEoCCKBAQoTRnJhbWVWZXJzaW9uU3VtbWFyeRIPCgd2ZXJzaW9uGAEgASgJEhEKCWNoYW5nZWxvZxgCIAEoCRIUCgxwdWJsaXNoZWRfYnkYAyABKAkSMAoMcHVibGlzaGVkX2F0GAQgASgLMhouZ29vZ2xlLnByb3RvYnVmLlRpbWVzdGFtcEKfAQoNY29tLmZyYW1lcy52MUIKRnJhbWVQcm90b1ABWj1naXRodWIuY29tL25lYmFyaS1kZXYvbmViYXJpLWZyYW1lcy9nZW4vZ28vZnJhbWVzL3YxO2ZyYW1lc3YxogIDRlhYqgIJRnJhbWVzLlYxygIJRnJhbWVzXFYx4gIVRnJhbWVzXFYxXEdQQk1ldGFkYXRh6gIKRnJhbWVzOjpWMWIGcHJvdG8z", [file_google_protobuf_timestamp]); /** * @generated from message frames.v1.Org @@ -130,6 +130,14 @@ export type Frame = Message<"frames.v1.Frame"> & { * @generated from field: google.protobuf.Timestamp updated_at = 8; */ updatedAt?: Timestamp | undefined; + + /** + * Offered as a starting point in the "start from a template" picker. + * Denormalized from the latest version's `template` field at publish time. + * + * @generated from field: bool is_template = 9; + */ + isTemplate: boolean; }; /** @@ -274,6 +282,11 @@ export type FrameSummary = Message<"frames.v1.FrameSummary"> & { * @generated from field: frames.v1.Permissions permissions = 7; */ permissions?: Permissions | undefined; + + /** + * @generated from field: bool is_template = 8; + */ + isTemplate: boolean; }; /** diff --git a/gen/ts/frames/v1/frame_service_pb.ts b/gen/ts/frames/v1/frame_service_pb.ts index 6431d93..c11177d 100644 --- a/gen/ts/frames/v1/frame_service_pb.ts +++ b/gen/ts/frames/v1/frame_service_pb.ts @@ -19,7 +19,7 @@ export const file_frames_v1_frame_service: GenFile = /*@__PURE__*/ */ export type PublishFrameRequest = Message<"frames.v1.PublishFrameRequest"> & { /** - * full YAML; name/version/extends/excludes/slots parsed server-side + * full YAML; metadata/extends/excludes/body parsed server-side * * @generated from field: bytes content = 1; */ @@ -300,7 +300,7 @@ export const ListFrameVersionsResponseSchema: GenMessage = /*@__PURE__*/ /** * ConvertFrame translates between the two representations of the same frame: - * the canonical slot YAML stored in frame_versions.content, and the single + * the canonical YAML stored in frame_versions.content, and the single * Markdown file with YAML frontmatter defined by Frame Spec v0.2. It backs the * web app's Markdown editor, .frame.md import, and .frame.md export. * @@ -696,7 +696,7 @@ export const FrameService: GenService<{ output: typeof DeleteFrameResponseSchema; }, /** - * Pure conversion between the canonical slot YAML and the spec-conformant + * Pure conversion between the canonical YAML and the spec-conformant * .frame.md form. Stateless and unauthenticated beyond org membership. * * @generated from rpc frames.v1.FrameService.ConvertFrame diff --git a/proto/frames/v1/frame.proto b/proto/frames/v1/frame.proto index 6aeb4d5..9a1227d 100644 --- a/proto/frames/v1/frame.proto +++ b/proto/frames/v1/frame.proto @@ -29,6 +29,9 @@ message Frame { string latest_version = 6; google.protobuf.Timestamp created_at = 7; google.protobuf.Timestamp updated_at = 8; + // Offered as a starting point in the "start from a template" picker. + // Denormalized from the latest version's `template` field at publish time. + bool is_template = 9; } message ParentRef { @@ -59,6 +62,7 @@ message FrameSummary { string latest_version = 5; google.protobuf.Timestamp updated_at = 6; Permissions permissions = 7; + bool is_template = 8; } message FrameVersionSummary { diff --git a/proto/frames/v1/frame_service.proto b/proto/frames/v1/frame_service.proto index b4a9f3c..cbeddf9 100644 --- a/proto/frames/v1/frame_service.proto +++ b/proto/frames/v1/frame_service.proto @@ -20,7 +20,7 @@ service FrameService { rpc ListFrameVersions(ListFrameVersionsRequest) returns (ListFrameVersionsResponse); // Write - delete a frame. Blocks if the frame is a parent unless force=true. rpc DeleteFrame(DeleteFrameRequest) returns (DeleteFrameResponse); - // Pure conversion between the canonical slot YAML and the spec-conformant + // Pure conversion between the canonical YAML and the spec-conformant // .frame.md form. Stateless and unauthenticated beyond org membership. rpc ConvertFrame(ConvertFrameRequest) returns (ConvertFrameResponse); // Admin only - list the caller's org members. @@ -34,7 +34,7 @@ service FrameService { } message PublishFrameRequest { - bytes content = 1; // full YAML; name/version/extends/excludes/slots parsed server-side + bytes content = 1; // full YAML; metadata/extends/excludes/body parsed server-side string changelog = 2; } message PublishFrameResponse { @@ -88,7 +88,7 @@ message ListFrameVersionsResponse { } // FieldViolation is one validation failure at a specific field path -// (e.g. "slots.terminology[2].definition", matching backend validate.go paths). +// (e.g. "extends[0].version", matching backend validate.go paths). message FieldViolation { string field = 1; string message = 2; @@ -102,7 +102,7 @@ message FieldViolations { } // ConvertFrame translates between the two representations of the same frame: -// the canonical slot YAML stored in frame_versions.content, and the single +// the canonical YAML stored in frame_versions.content, and the single // Markdown file with YAML frontmatter defined by Frame Spec v0.2. It backs the // web app's Markdown editor, .frame.md import, and .frame.md export. message ConvertFrameRequest { diff --git a/testdata/legacy-slots/expected.md b/testdata/legacy-slots/expected.md new file mode 100644 index 0000000..e7751b1 --- /dev/null +++ b/testdata/legacy-slots/expected.md @@ -0,0 +1,47 @@ +## Terminology + +- **customer**: An enterprise organization +- **Frame**: A portable context artifact. + Versioned, and pinned when inherited. + +## Rules + +- Never claim performance numbers without data. +- Redact before logging: + - no names + - no account numbers +- Cite the benchmark. + + Link to the run that produced it. + +## Skills + +- Summarize a thread + +## Prompts + +- Draft a release note from a changelog. + +## Tool Specifications + +`search(query)` returns ranked results. + +## Goals + +Grow the platform. + +## Style + + Two leading spaces, and a trailing newline to strip. + +## Norms + +Ship small. + +## Architecture + +Go backend, React web app. + +## Business Process + +Quarterly planning. diff --git a/testdata/legacy-slots/input.yaml b/testdata/legacy-slots/input.yaml new file mode 100644 index 0000000..8fee761 --- /dev/null +++ b/testdata/legacy-slots/input.yaml @@ -0,0 +1,60 @@ +# Shared fixture for the legacy `slots:` -> body rendering, which exists in two +# places: renderMarkdown in backend/internal/frames/legacy.go and +# legacySlotsToMarkdown in web/src/lib/frame-yaml.ts. +# +# Both must produce expected.md byte for byte. The web copy is not display-only: +# restoring a legacy version re-serializes its rendered body as the new +# canonical content, so a divergence rewrites what is stored. Neither side's own +# tests caught the drift this fixture was added for, because both used +# single-line values and substring assertions. +# +# Every value here is a case where a plausible port drifts. Keep it that way: +# add a case when you find a new one, and do not simplify the awkward ones. +name: legacy-fixture +description: Every shape the legacy renderer has to get right +version: 1.0.0 +slots: + terminology: + # Single-line: the ordinary case. + - term: customer + definition: An enterprise organization + # Multi-line definition. Unreachable from the web form (it used an ), + # but reachable from a .frame.md import and from the CLI. + - term: Frame + definition: |- + A portable context artifact. + Versioned, and pinned when inherited. + rules: + - Never claim performance numbers without data. + # A multi-line item whose continuation is itself block-level markup. Without + # the two-space indent, CommonMark lazy continuation turns this one rule + # into three flat list items and loses the nesting. + - |- + Redact before logging: + - no names + - no account numbers + # A multi-line item with a blank line in the middle, which the renderer + # emits as a bare blank line rather than an indented one. + - |- + Cite the benchmark. + + Link to the run that produced it. + skills: + - Summarize a thread + prompts: + - Draft a release note from a changelog. + tool_specs: |- + `search(query)` returns ranked results. + goals: |+ + + Grow the platform. + + # Quoted so the leading spaces reach the renderer: markdown gives leading + # whitespace meaning, and a .trim() port would silently eat it. + style: " Two leading spaces, and a trailing newline to strip.\n" + norms: |- + Ship small. + architecture: |- + Go backend, React web app. + business_process: |- + Quarterly planning. diff --git a/web/package-lock.json b/web/package-lock.json index 88e404c..6ea7570 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -32,6 +32,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", + "@tailwindcss/typography": "^0.5.20", "@tailwindcss/vite": "^4.0.0", "@testing-library/jest-dom": "^6.6.0", "@testing-library/react": "^16.1.0", @@ -1962,6 +1963,19 @@ "node": ">= 20" } }, + "node_modules/@tailwindcss/typography": { + "version": "0.5.20", + "resolved": "https://registry.npmjs.org/@tailwindcss/typography/-/typography-0.5.20.tgz", + "integrity": "sha512-hwbzQuNUfcPvbegQFatVPl/MY/tcM9KLl963hQ5laJKPh81TEZ1+dNG9PirGvcaDBkp+BCshExAyKVPW91dozw==", + "dev": true, + "license": "MIT", + "dependencies": { + "postcss-selector-parser": "6.0.10" + }, + "peerDependencies": { + "tailwindcss": ">=3.0.0 || >=4.0.0 || insiders" + } + }, "node_modules/@tailwindcss/vite": { "version": "4.3.1", "resolved": "https://registry.npmjs.org/@tailwindcss/vite/-/vite-4.3.1.tgz", @@ -3036,6 +3050,19 @@ "dev": true, "license": "MIT" }, + "node_modules/cssesc": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/cssesc/-/cssesc-3.0.0.tgz", + "integrity": "sha512-/Tb/JcjK111nNScGob5MNtsntNM1aCNUDipB/TkwZFhyDrrE47SOx/18wF2bbjgc3ZzCSKW1T5nt5EbFoAz/Vg==", + "dev": true, + "license": "MIT", + "bin": { + "cssesc": "bin/cssesc" + }, + "engines": { + "node": ">=4" + } + }, "node_modules/cssstyle": { "version": "4.6.0", "resolved": "https://registry.npmjs.org/cssstyle/-/cssstyle-4.6.0.tgz", @@ -5714,6 +5741,20 @@ "node": "^10 || ^12 || >=14" } }, + "node_modules/postcss-selector-parser": { + "version": "6.0.10", + "resolved": "https://registry.npmjs.org/postcss-selector-parser/-/postcss-selector-parser-6.0.10.tgz", + "integrity": "sha512-IQ7TZdoaqbT+LCpShg46jnZVlhWD2w6iQYAcYXfHARZ7X1t/UGhhceQDs5X0cGqKvYlHNOuv7Oa1xmb0oQuA3w==", + "dev": true, + "license": "MIT", + "dependencies": { + "cssesc": "^3.0.0", + "util-deprecate": "^1.0.2" + }, + "engines": { + "node": ">=4" + } + }, "node_modules/prelude-ls": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/prelude-ls/-/prelude-ls-1.2.1.tgz", @@ -6560,6 +6601,13 @@ "react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" } }, + "node_modules/util-deprecate": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", + "integrity": "sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==", + "dev": true, + "license": "MIT" + }, "node_modules/vfile": { "version": "6.0.3", "resolved": "https://registry.npmjs.org/vfile/-/vfile-6.0.3.tgz", diff --git a/web/package.json b/web/package.json index de08626..04edf33 100644 --- a/web/package.json +++ b/web/package.json @@ -38,6 +38,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", + "@tailwindcss/typography": "^0.5.20", "@tailwindcss/vite": "^4.0.0", "@testing-library/jest-dom": "^6.6.0", "@testing-library/react": "^16.1.0", diff --git a/web/src/app/AppShell.tsx b/web/src/app/AppShell.tsx index de59a14..35b8ab7 100644 --- a/web/src/app/AppShell.tsx +++ b/web/src/app/AppShell.tsx @@ -5,7 +5,10 @@ export function AppShell() { return (
-
+ {/* A flex column so pages that want to fill the viewport (the frame + content editor) can stretch with flex-1 instead of subtracting the + chrome height with hardcoded pixel math. */} +
diff --git a/web/src/app/RequireMembership.tsx b/web/src/app/RequireMembership.tsx index 76c6d1a..9bfe368 100644 --- a/web/src/app/RequireMembership.tsx +++ b/web/src/app/RequireMembership.tsx @@ -16,7 +16,7 @@ export function RequireMembership() { if (code === Code.Unauthenticated) { return ; } - return
Something went wrong.
; + return
Something went wrong.
; } return ; } diff --git a/web/src/app/routes.test.tsx b/web/src/app/routes.test.tsx index c3531f1..df37b64 100644 --- a/web/src/app/routes.test.tsx +++ b/web/src/app/routes.test.tsx @@ -6,10 +6,15 @@ vi.mock("@/lib/auth/useAuth", () => ({ // AppShell, RequireMembership, and RequireAdmin all call useQuery(getMe). // Use a vi.fn() so individual tests can control the returned role/error. +// AdminMembersPage (the /admin index redirect target) additionally uses +// useMutation, createConnectQueryKey, and the TanStack query client. const useQueryMock = vi.fn(); vi.mock("@connectrpc/connect-query", () => ({ useQuery: () => useQueryMock(), + useMutation: () => ({ mutate: vi.fn(), isPending: false }), + createConnectQueryKey: () => ["k"], })); +vi.mock("@tanstack/react-query", () => ({ useQueryClient: () => ({ invalidateQueries: vi.fn() }) })); import { render, screen } from "@testing-library/react"; import { MemoryRouter } from "react-router"; @@ -35,7 +40,7 @@ it("renders a Connect link in the header", () => { expect(link).toHaveAttribute("href", "/connect"); }); -it("renders AdminHomePage at /admin for an admin user", () => { +it("renders the admin section (redirected to Members) at /admin for an admin user", () => { useQueryMock.mockReturnValue({ data: { role: "admin" }, isLoading: false, error: null }); render( diff --git a/web/src/app/routes.tsx b/web/src/app/routes.tsx index 1401fc3..64e1a36 100644 --- a/web/src/app/routes.tsx +++ b/web/src/app/routes.tsx @@ -1,4 +1,4 @@ -import { Route, Routes } from "react-router"; +import { Navigate, Route, Routes } from "react-router"; import { RequireAuth } from "./RequireAuth"; import { RequireMembership } from "./RequireMembership"; import { RequireAdmin } from "./RequireAdmin"; @@ -11,9 +11,9 @@ import { FrameDetailPage } from "@/pages/FrameDetailPage"; import { FrameAuthoringPage } from "@/pages/FrameAuthoringPage"; import { ConnectHubPage } from "@/pages/ConnectHubPage"; import { ConnectProviderPage } from "@/pages/ConnectProviderPage"; -import { AdminHomePage } from "@/pages/AdminHomePage"; +import { AdminLayout } from "@/pages/AdminLayout"; import { AdminMembersPage } from "@/pages/AdminMembersPage"; -import { AdminFramesPage } from "@/pages/AdminFramesPage"; +import { AdminTemplatesPage } from "@/pages/AdminTemplatesPage"; export function AppRoutes() { return ( @@ -31,9 +31,11 @@ export function AppRoutes() { } /> } /> }> - } /> - } /> - } /> + }> + } /> + } /> + } /> + diff --git a/web/src/components/connect/CopyField.tsx b/web/src/components/connect/CopyField.tsx index dc1be77..4588cfa 100644 --- a/web/src/components/connect/CopyField.tsx +++ b/web/src/components/connect/CopyField.tsx @@ -14,9 +14,11 @@ export function CopyField({ const [copied, setCopied] = useState(false); return (
- {label &&
{label}
} + {label &&
{label}
}
- {value} + + {value} +
diff --git a/web/src/components/connect/ProviderTile.tsx b/web/src/components/connect/ProviderTile.tsx index 832a37e..4b3da8d 100644 --- a/web/src/components/connect/ProviderTile.tsx +++ b/web/src/components/connect/ProviderTile.tsx @@ -1,26 +1,46 @@ import { Link } from "react-router"; -import { Card } from "@/components/ui/card"; +import { + Card, + CardAction, + CardDescription, + CardHeader, + CardTitle, +} from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import type { ConnectProvider } from "@/lib/connect-providers"; export function ProviderTile({ provider }: { provider: ConnectProvider }) { if (provider.status === "available") { return ( - - -
{provider.name}
-

{provider.blurb}

- - + + + + {/* The whole tile is the hit target; the anchor stretches over it so + the accessible name stays on a single real link. */} + + {provider.name} + + + {provider.blurb} + + ); } return ( - -
-
{provider.name}
- Coming soon -
-

{provider.blurb}

+ + + {provider.name} + + Coming soon + + {provider.blurb} + ); } diff --git a/web/src/components/document/AddSectionMenu.tsx b/web/src/components/document/AddSectionMenu.tsx deleted file mode 100644 index ca3a618..0000000 --- a/web/src/components/document/AddSectionMenu.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { Plus } from "lucide-react"; -import { - DropdownMenu, - DropdownMenuTrigger, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuPortal, -} from "@/components/ui/dropdown-menu"; -import type { SlotSectionDef } from "@/lib/slot-sections"; - -// The document grows section by section: only sections the author chose (or -// that already carry content) are on the page, and this menu offers the rest. -// Each item explains what the section is for, which doubles as the schema's -// documentation at the moment it is needed. -export function AddSectionMenu({ - available, - onAdd, -}: { - available: SlotSectionDef[]; - onAdd: (def: SlotSectionDef) => void; -}) { - if (available.length === 0) return null; - return ( - - - - Add section - - - {/* Ten items with hints outgrow the space around the trigger; cap the - popup at Base UI's measured --available-height and scroll inside it - so it never runs past the viewport edge. */} - - {available.map((def) => ( - onAdd(def)}> -
- {def.label} - {def.hint} -
- - ))} - - - - ); -} diff --git a/web/src/components/document/DocMetadataHeader.tsx b/web/src/components/document/DocMetadataHeader.tsx index 3b03bb5..271b770 100644 --- a/web/src/components/document/DocMetadataHeader.tsx +++ b/web/src/components/document/DocMetadataHeader.tsx @@ -1,84 +1,140 @@ -import { useFormContext, useWatch } from "react-hook-form"; +import { useId, type ReactNode } from "react"; +import { Controller, useFormContext, useWatch } from "react-hook-form"; +import { Checkbox } from "@/components/ui/checkbox"; import { Input } from "@/components/ui/input"; -import { Select } from "@/components/ui/select"; +import { Label } from "@/components/ui/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import { Textarea } from "@/components/ui/textarea"; import { FieldError, useFieldError, errorProps } from "@/components/form/FieldError"; import { VISIBILITY_VALUES } from "@/lib/frame-yaml"; -import { cn } from "@/lib/utils"; -// Inputs styled to read as document text until pointed at: the frame's name is -// its title and the description its subtitle, so the editor keeps the shape of -// the page the reader will see. A visible border appears on hover/focus (and on -// error) so the fields stay discoverable as fields. -const quiet = - "border-transparent bg-transparent shadow-none " + - "hover:border-input focus-visible:border-input aria-invalid:border-destructive"; +// One labeled field in the metadata column: label above control, and the +// field's validation error (looked up by form path) below. The control receives +// the generated id so the Label associates with it explicitly. +function Field({ + label, + name, + children, +}: { + label: string; + name?: string; + children: (id: string) => ReactNode; +}) { + const id = useId(); + return ( +
+ + {children(id)} + {name && } +
+ ); +} -// The metadata header of the document editor: title, description, and the -// spec metadata (visibility / scope / maintainer) as one compact row. +// The frame's identity and spec metadata as a standard labeled form column - +// the left side of the authoring layout. export function DocMetadataHeader({ nameReadOnly }: { nameReadOnly: boolean }) { const { register, control } = useFormContext(); - // Watched (not getValues) so the async edit-mode prefill re-renders the title. + // Watched (not getValues) so the async edit-mode prefill re-renders the name. const name = useWatch({ control, name: "name" }) as string; const nameError = useFieldError("name"); const descError = useFieldError("description"); const visibilityError = useFieldError("visibility"); return ( -
+
{nameReadOnly ? ( - // Identity is fixed after creation; render it as the plain title it is - // (the page label above is the document's h1). -
{name}
- ) : ( -
- - + // Identity is fixed after creation; render it as a plain value. +
+
Name
+
{name}
+ ) : ( + + {(id) => ( + + )} + )} -
- - -
+ + {(id) => ( +