add additional mcp validation and error handling

This commit is contained in:
david 2026-09-03 14:29:05 +03:00
parent 76af2a47db
commit b31a0dfde7
35 changed files with 1541 additions and 138 deletions

View File

@ -210,6 +210,12 @@ interface, which would be a second fact to keep in step. What a tool is, it stat
does.
- `Destructive` keeps it outside an "approve all" answer. Only `delete_document` and
`delete_block`.
- `Overwrites` says the write replaces content the caller did not name — the target's
nested blocks, and the uids comments and hooks hang off, go with it.
`update_block_text` and `replace_block`. It is what MCP's destructive hint reports
alongside `Destructive`, and is deliberately separate from it: a client deciding
whether to auto-approve needs to know, while an approve-all inside a chat turn is
meant to cover exactly these edits.
- `DataSource` says the tool reads an outbound connection rather than the organisation's
documents. Over MCP those answer to their own `data-sources:read` scope and are
annotated open-world.
@ -221,7 +227,7 @@ a write cannot skip it by forgetting to ask.
The MCP surface (`internal/server/internal/mcp`) serves the same registry **ungated** via
`Set.Entries`, minus the internal ones — MCP clients own the approval story, and the
write/destructive facts become MCP tool annotations. It builds each `mcp.Tool` inline
write/destructive/overwrites facts become MCP tool annotations. It builds each `mcp.Tool` inline
from `Entry.Info` and calls `Entry.Tool.Run`, so it never touches eino; every document in
`Result.Documents` comes back as a resource link. Adding a tool to `tools.Set` extends
the MCP server for free.

View File

@ -449,6 +449,18 @@ describe("createAuth", () => {
expect(auth.options.basePath).toBe("/api/auth")
})
// the default is the origin root, which serves the frontend's
// document page and reads no error query
it("points failed requests at the frontend's error page", ({
expect,
}) => {
const { auth } = buildAuth()
expect(auth.options.onAPIError.errorURL).toBe(
"http://localhost:8080/auth-error",
)
})
it("takes better-auth's level from the configured one", ({
expect,
}) => {

View File

@ -266,6 +266,13 @@ export function createAuth({
// redirect URIs below).
baseURL: env.authOrigin,
basePath: "/api/auth",
// without this better-auth's /api/auth/error route redirects to
// the origin root, which the reverse proxy serves the frontend
// from — a failed authorization would land on a document and
// its error query would go unread.
onAPIError: {
errorURL: `${env.frontendUrl}/auth-error`,
},
logger: {
level: betterAuthLevels[env.logLevel],
log: (level, message) => {

View File

@ -46,6 +46,22 @@ function sampleBlocks(): PMNode[] {
]
}
// a bullet list carrying one item, whose paragraph holds the text. The
// three uids let a test address the list, the item, or the paragraph.
function bulletList(listUID: string, itemUID: string, text: string): PMNode {
return {
type: "bulletList",
attrs: { uid: listUID },
content: [
{
type: "listItem",
attrs: { uid: itemUID },
content: [textParagraph("text", text)],
},
],
}
}
// builds a live document through the same path the Hocuspocus server
// uses, so every block carries the uid the uid-addressed operations
// look up.
@ -750,6 +766,127 @@ describe("applyOperations", () => {
expect(blockByUid(doc, "intro").length).toBe(0)
})
it("writes into a callout's first paragraph, keeping its other children", ({
expect,
}) => {
const doc = docWith([
{
type: "calloutBlock",
attrs: {
uid: "callout",
icon: "lucide:zap",
},
content: [
textParagraph(
"first",
"First line",
),
textParagraph(
"second",
"Second line",
),
],
},
])
const result = applyOperations(doc, [
{
kind: "update_text",
block_uid: "callout",
content: [
{
type: "text",
text: "Rewritten",
},
],
},
])
const callout = blockByUid(doc, "callout")
expect(result.applied).toBe(1)
expect(callout.length).toBe(2)
expect(inlineDelta(blockByUid(doc, "first"))).toEqual([
{ insert: "Rewritten" },
])
expect(inlineDelta(blockByUid(doc, "second"))).toEqual([
{ insert: "Second line" },
])
})
it("reports an error when a callout holds no paragraph to write in", ({
expect,
}) => {
const doc = docWith([
{
type: "calloutBlock",
attrs: {
uid: "callout",
icon: "lucide:zap",
},
content: [
bulletList(
"list",
"item",
"Item",
),
],
},
])
const result = applyOperations(doc, [
{
kind: "update_text",
block_uid: "callout",
content: [
{
type: "text",
text: "Rewritten",
},
],
},
])
expect(result.applied).toBe(0)
expect(result.errors[0]?.message).toContain(
"no paragraph to write in",
)
})
it.for([
{ name: "a bullet list", uid: "list" },
{ name: "a list item", uid: "item" },
])(
"refuses to overwrite $name, which carries blocks rather than text",
({ uid }, { expect }) => {
const doc = docWith([
bulletList("list", "item", "Item"),
])
const result = applyOperations(doc, [
{
kind: "update_text",
block_uid: uid,
content: [
{
type: "text",
text: "Rewritten",
},
],
},
])
expect(result.applied).toBe(0)
expect(result.errors[0]?.message).toContain(
"carries blocks rather than text",
)
// the item, and the uid any comment or hook hangs off,
// both survive the refusal.
expect(
inlineDelta(blockByUid(doc, "text")),
).toEqual([{ insert: "Item" }])
},
)
it("reports an error when the block is absent", ({
expect,
}) => {

View File

@ -71,9 +71,10 @@ export interface ReplaceOp {
/**
* Replaces the inline content of a text-bearing block in place,
* preserving the block's type, attrs, and uid. Children that are
* themselves blocks (e.g. paragraphs inside a callout) are not
* affected.
* preserving the block's type, attrs, and uid. A callout or blockquote
* is written through to its first paragraph, since their schemas hold
* blocks rather than text; any other block-carrying target is refused
* rather than flattened.
*/
export interface UpdateTextOp {
kind: "update_text"
@ -298,25 +299,79 @@ function opReplace(doc: Y.Doc, op: ReplaceOp): void {
found.parent.insert(found.index, [xml])
}
// the blocks whose children are inline content, so replacing them
// wholesale with a text run is the whole edit.
const TEXT_LEAF_NODES = new Set([
"paragraph",
"heading",
"codeBlock",
"mermaidBlock",
])
// the blocks that hold their text one level down, in a first child
// paragraph. Their schemas admit block children only, so writing a text
// run into them directly produces a node the editor cannot parse and
// the canonical model cannot read back.
const TEXT_WRAPPER_NODES = new Set(["calloutBlock", "blockquote"])
function opUpdateText(doc: Y.Doc, op: UpdateTextOp): void {
const found = findByUid(doc.getXmlFragment("content"), op.block_uid)
if (!found) {
throw new Error(`block_uid not found: ${op.block_uid}`)
}
// Replace the block's children with a fresh Y.XmlText carrying
// the new inline content. Block-level children (e.g. paragraphs
// nested inside a list item) would also be wiped — update_text
// is only meant for text-bearing leaf blocks (paragraph, heading,
// codeBlock, mermaidBlock, …) so that's intended.
found.element.delete(0, found.element.length)
const nodeName = found.element.nodeName
// the edit replaces everything under the target, so a block holding
// other blocks would lose them — and lose the uids comments and
// hooks are anchored to — while reporting success. Only the caller
// knows that was not meant, so refuse instead of guessing.
if (
!TEXT_LEAF_NODES.has(nodeName) &&
!TEXT_WRAPPER_NODES.has(nodeName)
) {
throw new Error(
`update_text does not apply to ${nodeName}: it carries ` +
`blocks rather than text, and the edit would discard ` +
`them. Use replace_block to rewrite it whole, or ` +
`update_text on the block holding the text.`,
)
}
const target = TEXT_WRAPPER_NODES.has(nodeName)
? firstParagraph(found.element)
: found.element
if (!target) {
throw new Error(
`update_text found no paragraph to write in ${nodeName}`,
)
}
target.delete(0, target.length)
const text = buildInlineText(op.content)
if (text) {
found.element.insert(0, [text])
target.insert(0, [text])
}
}
// firstParagraph returns the wrapper's first paragraph child, which is
// where its text lives.
function firstParagraph(el: Y.XmlElement): Y.XmlElement | null {
for (let i = 0; i < el.length; i++) {
const child = el.get(i)
if (
child instanceof Y.XmlElement &&
child.nodeName === "paragraph"
) {
return child
}
}
return null
}
function opUpdateAttrs(doc: Y.Doc, op: UpdateAttrsOp): void {
const found = findByUid(doc.getXmlFragment("content"), op.block_uid)
if (!found) {

View File

@ -25,10 +25,10 @@ const (
// Metric enum values. They mirror the editor's enums in
// web/app/components/editor/blocks/metrics/utils.ts (TimeRangePreset,
// RefreshInterval, the Visualization*Unit enums, MetricBlockWidth) and
// web/app/utils/api/data-source/generic-query.ts (GenericQueryChartType);
// a value added there has to be added here or the assistant cannot author
// it.
// RefreshInterval, the Visualization*Unit enums, MetricBlockWidth,
// MetricSimulationPreset) and web/app/utils/api/data-source/generic-query.ts
// (GenericQueryChartType); a value added there has to be added here or the
// assistant cannot author it.
var (
// _metricVisualizationTypes are the chart kinds; they equal the
// datasource processor's ChartType values, which the tools package
@ -62,6 +62,13 @@ var (
// _metricWidths are the block widths inside a metric_grid.
_metricWidths = []string{"compact", "standard", "wide"}
// _metricSimulationPresets are the generated series a block draws
// while the metric it documents has no real data to answer with.
_metricSimulationPresets = []string{
"cpu_usage", "memory_usage", "disk_usage",
"http_requests", "http_latency", "error_rate",
}
// _metricEnums maps each enum-valued attribute to its values, which
// is the table validateMetric checks against and MetricEnums
// publishes.
@ -71,6 +78,7 @@ var (
document.AttrRefreshInterval: _metricRefreshIntervals,
document.AttrUnitType: _metricUnitTypes,
document.AttrWidth: _metricWidths,
document.AttrSimulationPreset: _metricSimulationPresets,
}
)
@ -98,14 +106,22 @@ func validateMetric(b Block, path string) error {
return err
}
return validateMetricAttrs(b.Attrs, path)
}
// validateMetricAttrs checks a metric block's attributes: every enum
// holds one of its values, every string is a string, every number a
// number, and the queries and thresholds arrays are well-formed.
func validateMetricAttrs(attrs document.Attributes, path string) error {
for _, key := range []string{
document.AttrVisualizationType,
document.AttrTimeRange,
document.AttrRefreshInterval,
document.AttrUnitType,
document.AttrWidth,
document.AttrSimulationPreset,
} {
if err := validateMetricEnum(b.Attrs, key, path); err != nil {
if err := validateMetricEnum(attrs, key, path); err != nil {
return err
}
}
@ -115,9 +131,8 @@ func validateMetric(b Block, path string) error {
document.AttrDataSourceID,
document.AttrUnitCustom,
document.AttrBaseThresholdColor,
document.AttrSimulationPreset,
} {
if err := validateMetricString(b.Attrs, key, path); err != nil {
if err := validateMetricString(attrs, key, path); err != nil {
return err
}
}
@ -127,16 +142,16 @@ func validateMetric(b Block, path string) error {
document.AttrAxisBoundsMin,
document.AttrAxisBoundsMax,
} {
if err := validateMetricNumber(b.Attrs, key, path); err != nil {
if err := validateMetricNumber(attrs, key, path); err != nil {
return err
}
}
if err := validateMetricQueries(b.Attrs, path); err != nil {
if err := validateMetricQueries(attrs, path); err != nil {
return err
}
return validateMetricThresholds(b.Attrs, path)
return validateMetricThresholds(attrs, path)
}
// validateMetricEnum checks that the named attribute, when set, is one

View File

@ -14,9 +14,10 @@ func Test_MetricEnums(t *testing.T) {
got := MetricEnums()
require.Len(t, got, 5)
require.Len(t, got, 6)
assert.Equal(t, []string{"line_chart", "bar_chart", "gauge_chart"}, got[document.AttrVisualizationType])
assert.Equal(t, []string{"compact", "standard", "wide"}, got[document.AttrWidth])
assert.Len(t, got[document.AttrSimulationPreset], 6)
assert.Len(t, got[document.AttrTimeRange], 28)
assert.Len(t, got[document.AttrRefreshInterval], 10)
assert.Len(t, got[document.AttrUnitType], 20)
@ -109,6 +110,11 @@ func Test_validateMetric(t *testing.T) {
Err: assert.AnError,
ExpectedPath: "attrs.width",
},
"Unknown simulation preset": {
Attrs: document.Attributes{document.AttrSimulationPreset: "solar_flares"},
Err: assert.AnError,
ExpectedPath: "attrs.simulationPreset",
},
"Non-string title": {
Attrs: document.Attributes{document.AttrTitle: 1},
Err: assert.AnError,

View File

@ -27,6 +27,56 @@ var _canonicalTypes = map[document.BlockNodeType]Type{
document.BlockNodeParamList: BlockParamList,
}
// _typeOrder lists every canonical type once, in the order the block
// model documents them. Callers publishing the set — a tool schema's
// enum — take it from here so the order is the same on every build,
// which is what keeps a provider's prompt cache warm across restarts.
var _typeOrder = []Type{
BlockParagraph,
BlockHeading,
BlockBlockquote,
BlockBulletList,
BlockOrderedList,
BlockTaskList,
BlockCallout,
BlockCode,
BlockTitledCode,
BlockMermaid,
BlockHorizontalRule,
BlockImage,
BlockFigma,
BlockMetric,
BlockMetricGrid,
BlockSplitDoc,
BlockParamList,
}
// Types returns every canonical block type, in documentation order.
func Types() []string {
out := make([]string, 0, len(_typeOrder))
for _, t := range _typeOrder {
out = append(out, string(t))
}
return out
}
// RootTypes returns the canonical block types that may sit directly
// under the document root, in documentation order. The rest reach the
// document inside a macro and are never placed on their own.
func RootTypes() []string {
out := make([]string, 0, len(_allowedAtRoot))
for _, t := range _typeOrder {
if _allowedAtRoot[t] {
out = append(out, string(t))
}
}
return out
}
// CanonicalType returns the canonical name of a ProseMirror node type. The
// second return value reports whether the node type has one at all.
func CanonicalType(pm document.BlockNodeType) (Type, bool) {

View File

@ -16,9 +16,12 @@ var (
// emit any of it back out of a stored blockquote.
_allowedBlockquoteItems = _allowedAtRoot
// _allowedListItemContent is the set of block types that can sit
// inside a list/task-list item. Mirrors the web editor's
// ListItem default content.
// _allowedListItemContent is the set of block types that can be
// nested under a list or task-list entry — the "block*" half of
// the editor's "paragraph block*" content for ListItem and a
// nested TaskItem. The "paragraph" half is the entry itself, which
// validateListEntry checks; the two positions take different types
// and do not share a rule.
_allowedListItemContent = map[Type]bool{
BlockParagraph: true,
BlockBulletList: true,
@ -153,11 +156,44 @@ func ValidateAsRoot(b Block) error {
return err
}
if !_allowedAtRoot[b.Type] {
return verr("", fmt.Sprintf(
"%s is not allowed at the document root; it must appear inside %s",
b.Type, containerForType(b.Type),
))
return AllowedInContainer(document.BlockNodeDoc, b.Type)
}
// ValidateAttrs checks the attribute rules for a block of the given
// type, asking nothing about its content. An attribute-only update
// names no content, so the content rules have nothing to say about it,
// while the attributes it does set answer to exactly the rules a whole
// block's would. Types carrying no attribute rules pass.
//
// Callers merge the update over the block's current attributes before
// calling: a rule reads the attributes the block will end up with, not
// the subset one call happened to name.
func ValidateAttrs(t Type, attrs document.Attributes) error {
switch t {
case BlockHeading:
return validateHeadingAttrs(attrs, "")
case BlockTitledCode:
return validateTitledCodeAttrs(attrs, "")
case BlockImage, BlockFigma:
return validateSrcAttrs(t, attrs, "")
case BlockMetric:
return validateMetricAttrs(attrs, "")
case BlockParagraph,
BlockBlockquote,
BlockBulletList,
BlockOrderedList,
BlockTaskList,
BlockCallout,
BlockCode,
BlockMermaid,
BlockHorizontalRule,
BlockMetricGrid,
BlockSplitDoc,
BlockParamList:
// these carry no attribute the canonical model constrains:
// callout's icon and code's language are free strings, and the
// rest have none at all.
return nil
}
return nil
@ -170,23 +206,39 @@ func ValidateAsRoot(b Block) error {
// direct children are wrapper items — a list, a macro internal —
// accepts no canonical block at all.
func ValidateInContainer(container document.BlockNodeType, b Block) error {
if container == document.BlockNodeDoc {
return ValidateAsRoot(b)
}
if err := validateBlock(b, ""); err != nil {
return err
}
return AllowedInContainer(container, b.Type)
}
// AllowedInContainer reports whether a block of type t may sit among
// the direct children of the given ProseMirror node, asking nothing
// about the block's content. A move carries a block that is already in
// the document, so its content is not in question — only where it is
// allowed to land.
func AllowedInContainer(container document.BlockNodeType, t Type) error {
if container == document.BlockNodeDoc {
if !_allowedAtRoot[t] {
return verr("", fmt.Sprintf(
"%s is not allowed at the document root; it must appear inside %s",
t, containerForType(t),
))
}
return nil
}
allowed, ok := _allowedInContainer[container]
if !ok {
return verr("", fmt.Sprintf("blocks cannot be placed directly inside %s", container))
}
if !allowed[b.Type] {
if !allowed[t] {
return verr("", fmt.Sprintf(
"%s is not allowed inside %s, which holds only %s",
b.Type, container, listAllowed(allowed),
t, container, listAllowed(allowed),
))
}
@ -243,7 +295,10 @@ func validateBlock(b Block, path string) error {
return validateParamList(b, path)
}
return verr(path, fmt.Sprintf("unknown block type %q", b.Type))
return verr(path, fmt.Sprintf(
"unknown block type %q; the types are %s",
b.Type, strings.Join(Types(), ", "),
))
}
// validateTextBearing checks that the block carries only inline Text
@ -269,7 +324,12 @@ func validateHeading(b Block, path string) error {
return err
}
if a, ok := b.Attrs.Get(document.AttrLevel); !ok || a.Int() < 1 || a.Int() > 3 {
return validateHeadingAttrs(b.Attrs, path)
}
// validateHeadingAttrs checks a heading's level.
func validateHeadingAttrs(attrs document.Attributes, path string) error {
if a, ok := attrs.Get(document.AttrLevel); !ok || a.Int() < 1 || a.Int() > 3 {
return verr(joinPath(path, "attrs.level"), "heading level must be 1, 2, or 3")
}
@ -339,10 +399,8 @@ func validateTaskList(b Block, path string) error {
return err
}
if !_allowedListItemContent[ti.Block.Type] {
return verr(joinPath(itemPath, "block"),
fmt.Sprintf("task_item content must be one of %s, got %s", listAllowed(_allowedListItemContent), ti.Block.Type),
)
if err := validateListEntry(ti.Block.Type, joinPath(itemPath, "block")); err != nil {
return err
}
if err := validateItemsAllowed(
@ -384,7 +442,12 @@ func validateTitledCode(b Block, path string) error {
return err
}
if a, ok := b.Attrs.Get(document.AttrTitle); !ok || strings.TrimSpace(a.String()) == "" {
return validateTitledCodeAttrs(b.Attrs, path)
}
// validateTitledCodeAttrs checks that a titled_code carries a title.
func validateTitledCodeAttrs(attrs document.Attributes, path string) error {
if a, ok := attrs.Get(document.AttrTitle); !ok || strings.TrimSpace(a.String()) == "" {
return verr(joinPath(path, "attrs.title"), "titled_code requires a non-empty title")
}
@ -414,8 +477,13 @@ func validateAtomWithSrc(b Block, path string) error {
return err
}
if a, ok := b.Attrs.Get(document.AttrSrc); !ok || strings.TrimSpace(a.String()) == "" {
return verr(joinPath(path, "attrs.src"), fmt.Sprintf("%s requires a non-empty src", b.Type))
return validateSrcAttrs(b.Type, b.Attrs, path)
}
// validateSrcAttrs checks that an image or figma block carries a src.
func validateSrcAttrs(t Type, attrs document.Attributes, path string) error {
if a, ok := attrs.Get(document.AttrSrc); !ok || strings.TrimSpace(a.String()) == "" {
return verr(joinPath(path, "attrs.src"), fmt.Sprintf("%s requires a non-empty src", t))
}
return nil
@ -645,9 +713,8 @@ func validateListItems(items []Block, path string) error {
children := item.Children
item.Children = nil
if !_allowedListItemContent[item.Type] {
return verr(itemPath, fmt.Sprintf("type %s not allowed here; expected one of %s",
item.Type, listAllowed(_allowedListItemContent)))
if err := validateListEntry(item.Type, itemPath); err != nil {
return err
}
if err := validateBlock(item, itemPath); err != nil {
@ -662,6 +729,22 @@ func validateListItems(items []Block, path string) error {
return nil
}
// validateListEntry checks the block a list or task-list entry leads
// with. The editor's ListItem and nested TaskItem both hold
// "paragraph block*", so the entry is the paragraph and anything else
// belongs under it — expanding a list as the entry would produce an
// item with no leading paragraph, which the schema does not admit.
func validateListEntry(t Type, path string) error {
if t != BlockParagraph {
return verr(path, fmt.Sprintf(
"a list entry is a %s, got %s; nest other blocks under the entry with children",
BlockParagraph, t,
))
}
return nil
}
// validateItemsAllowed validates a slice of items, requiring each
// to be of a type in allowed.
func validateItemsAllowed(items []Block, path string, allowed map[Type]bool) error {

View File

@ -230,6 +230,30 @@ func Test_Validate(t *testing.T) {
Err: assert.AnError,
ExpectedPath: "items[0]",
},
"Bullet list whose entry is a well-formed list is rejected": {
Input: Block{
Type: BlockBulletList,
Items: []Block{{
Type: BlockBulletList,
Items: []Block{{Type: BlockParagraph, Text: "nested"}},
}},
},
Err: assert.AnError,
ExpectedPath: "items[0]",
},
"Task list whose entry is a well-formed list is rejected": {
Input: Block{
Type: BlockTaskList,
TaskItems: []TaskItem{{
Block: Block{
Type: BlockBulletList,
Items: []Block{{Type: BlockParagraph, Text: "nested"}},
},
}},
},
Err: assert.AnError,
ExpectedPath: "task_items[0]/block",
},
"List item children are accepted": {
Input: Block{
Type: BlockBulletList,

View File

@ -29,7 +29,7 @@ You have tools for reading and writing documents in the user's organisation. Rea
Read first, then edit: search_documents to find candidates across the org, read_document_summary to look inside one, read_block only when you need a block's full inner structure.
You also have read-only tools for the organisation's data sources. Answer a question about the data from the query results themselves; author a metric block only when the user asks for a chart, a metric or a dashboard in a document. Discover what exists before you query — get_prometheus_metadata, the label and series tools, get_sql_metadata — rather than guessing metric, label or table names, and run the query with the chart_type you intend before you insert or edit a metric block, so you know it renders.
You also have read-only tools for the organisation's data sources. Answer a question about the data from the query results themselves; author a metric block only when the user asks for a chart, a metric or a dashboard in a document. Discover what exists before you query, using get_prometheus_metadata, the label and series tools and get_sql_metadata, rather than guessing metric, label or table names, and run the query with the chart_type you intend before you insert or edit a metric block, so you know it renders.
`
@ -56,7 +56,7 @@ Multi-paragraph content is multiple blocks, not one block with newlines. Inside
| bullet_list | items: [Block] | - |
| ordered_list | items: [Block] | - |
| task_list | task_items: [{checked, block}] | - |
| (any list entry) | the entry Block, plus children: [Block] for what is indented under it | - |
| (any list entry) | a paragraph, plus children: [Block] for what is indented under it | - |
| callout | text (shorthand) **or** items: [Block] | icon (defaults to lucide:text) |
| code | text (raw) | language (optional, default empty) |
| titled_code | text (raw code body) | title (required), language (optional) |
@ -73,18 +73,15 @@ Multi-paragraph content is multiple blocks, not one block with newlines. Inside
A metric block renders one chart of one data source. It never sits at the document root: wrap it in a metric_grid, which is what you insert, or put it in a split_doc's right side. Its attrs are:
- dataSourceId — the id from list_data_sources. Required for the block to render.
- visualizationType — line_chart, bar_chart, gauge_chart.
- queries — [{name, query, legendFormat}]. query is PromQL or SQL depending on the data source; legendFormat may be empty. A SQL chart selects a time column aliased "time" plus one or more numeric columns, and may use the $__ macros ($__timeFilter, $__timeGroupAlias).
- timeRange — last_5_minutes, last_15_minutes, last_30_minutes, last_1_hour, last_3_hours, last_6_hours, last_12_hours, last_24_hours, last_2_days, last_7_days, last_30_days, last_90_days, last_6_months, last_1_year, last_2_years, last_5_years, today, yesterday, today_so_far, this_week, this_week_so_far, this_month, this_month_so_far, this_year, this_year_so_far, previous_week, previous_month, previous_year.
- refreshInterval — 5s, 10s, 30s, 1m, 5m, 15m, 30m, 1h, 2h, 1d.
- unitType — custom, nanoseconds, microseconds, milliseconds, seconds, minutes, hours, days, bytes, kilobytes, megabytes, gigabytes, terabytes, bits, kilobits, megabits, gigabits, terabits, percent0to100, percent0to1. With custom, put the label in unitCustom.
- width — compact, standard, wide.
- title, decimals, thresholds ([{value, label, color}]), baseThresholdColor, axisBoundsMin, axisBoundsMax — optional; omit them to take the block's own defaults.
- dataSourceId is the id from list_data_sources. Required for the block to render.
- queries is [{name, query, legendFormat}]. query is PromQL or SQL depending on the data source; legendFormat may be empty. A SQL chart selects a time column aliased "time" plus one or more numeric columns, and may use the $__ macros ($__timeFilter, $__timeGroupAlias).
- width is compact, standard or wide.
- visualizationType, timeRange, refreshInterval, unitType and simulationPreset take a fixed set of values each, which the block tools' schema lists. With unitType custom, put the label in unitCustom. simulationPreset draws a generated series in place of the query's own result, for a block documenting a metric that has no real data yet; omit it, or set it to null, to chart the query.
- title, decimals, thresholds ([{value, label, color}]), baseThresholdColor, axisBoundsMin and axisBoundsMax are optional; omit them to take the block's own defaults.
### Nested lists
items are the list's own entries; anything nested under an entry goes in that entry block's children, never in a second entry. Reads report existing nesting the same way, so a block read with children has to be written back with them or the nested content is lost.
items are the list's own entries, and an entry is a paragraph. A list, callout or anything else nested under one goes in that entry's children, never in the entry itself and never in a second entry. Reads report existing nesting the same way, so a block read with children has to be written back with them or the nested content is lost.
### Compound blocks
@ -157,7 +154,7 @@ const _basePrompt = _personaSection + _blockModelSection + _etiquetteSection + _
// instead of waiting on a confirmation.
const _mcpIntroSection = `You are connected to Oxynote, a collaborative product for writing technical documentation. The tools operate on the documents of one organization, which are also listed as resources; blocks are the unit of content, addressed by uid, and comments, hooks, and files hang off those uids. Documents are written for humans first, balancing prose with technical detail where it sharpens meaning.
Read first, then edit: list_documents or search_documents to find a document, read_document_summary to look inside one, read_block only when you need a block's full inner structure. Data-source tools are read-only; discover what a source exposes — get_prometheus_metadata, the label and series tools, get_sql_metadata — before querying it. Write tools apply immediately.
Read first, then edit: list_documents or search_documents to find a document, read_document_summary to look inside one, read_block only when you need a block's full inner structure. Data-source tools are read-only; discover what a source exposes with get_prometheus_metadata, the label and series tools and get_sql_metadata before querying it. Write tools apply immediately.
`

View File

@ -24,15 +24,12 @@ func Test_buildSystemPrompt(t *testing.T) {
assert.Contains(t, got, "## Current context")
assert.Contains(t, got, "`doc-123`")
// the prompt states exactly the metric values Validate accepts. The
// model is told the metric configuration here rather than in a tool
// schema — a block's attrs depend on its type, which the shared
// block schema cannot express — so this is what keeps the two from
// drifting apart.
for attr, values := range block.MetricEnums() {
for _, v := range values {
assert.Contains(t, got, v, "%s value %q is missing from the prompt", attr, v)
}
// width is the one metric enum the prompt is still the only source
// for: attrs is a single field shared by every block type, and
// width means something else on an image, so the tool schema cannot
// publish it. The rest moved there, where Test_attrProps pins them.
for _, v := range block.MetricEnums()[document.AttrWidth] {
assert.Contains(t, got, v, "width value %q is missing from the prompt", v)
}
for _, attr := range []string{
@ -48,6 +45,7 @@ func Test_buildSystemPrompt(t *testing.T) {
document.AttrUnitCustom,
document.AttrAxisBoundsMin,
document.AttrAxisBoundsMax,
document.AttrSimulationPreset,
} {
assert.Contains(t, got, attr, "attr %q is missing from the prompt", attr)
}

View File

@ -41,7 +41,7 @@ type readDocumentSummary struct {
func (readDocumentSummary) Info() Info {
return Info{
Name: NameReadDocumentSummary,
Description: "Return an ordered, compact summary of a document: per-block uid, kind, flattened source_text, surrounding_context (ancestor headings, list intro, doc title), and tags. Use this as your default way to read a document — it's ~5-10x cheaper than fetching full content.",
Description: "Return an ordered, compact summary of a document: one row per block with its uid, kind, flattened text, depth and parent_uid, plus the few attrs that matter for reads (heading level, callout icon, code language, task checked). Use this as your default way to read a document — it's ~5-10x cheaper than fetching full content. Rows carrying has_children hold nested blocks the summary does not list; read_block returns those.",
Properties: documentIDProp(_descDocumentID),
Required: []string{_keyDocumentID},
}
@ -203,7 +203,7 @@ func (insertBlock) Info() Info {
"reference_block_uid": stringProp("The uid of the block to insert relative to."),
"position": map[string]any{
_keyType: _typeString,
"enum": []string{
_keyEnum: []string{
string(positionBefore),
string(positionAfter),
},
@ -382,7 +382,7 @@ func (appendBlock) Info() Info {
Description: "Append a single canonical block at the end of a document. Block uid is generated server-side if omitted.",
Properties: map[string]any{
_keyDocumentID: stringProp(_descTargetDocumentID),
_keyBlock: _blockSchema,
_keyBlock: _rootBlockSchema,
},
Required: []string{
_keyDocumentID,
@ -462,7 +462,7 @@ func (prependBlock) Info() Info {
Description: "Prepend a single canonical block at the start of a document. Block uid is generated server-side if omitted.",
Properties: map[string]any{
_keyDocumentID: stringProp(_descTargetDocumentID),
_keyBlock: _blockSchema,
_keyBlock: _rootBlockSchema,
},
Required: []string{
_keyDocumentID,
@ -582,9 +582,10 @@ func (replaceBlock) Info() Info {
}
}
// Traits reports a write.
// Traits reports a write that overwrites: the replacement takes the
// target's place whole, so every nested block and uid under it goes.
func (replaceBlock) Traits() Traits {
return Traits{Write: true}
return Traits{Write: true, Overwrites: true}
}
// Title announces which document is being updated.
@ -695,9 +696,10 @@ func (updateBlockText) Info() Info {
}
}
// Traits reports a write.
// Traits reports a write that overwrites: the new text replaces the
// block's whole content, nested blocks and their uids included.
func (updateBlockText) Traits() Traits {
return Traits{Write: true}
return Traits{Write: true, Overwrites: true}
}
// Title announces which document is being updated.
@ -871,6 +873,10 @@ func (updateBlockAttrs) Execute(inp Input) (string, error) {
}
}
if err := inp.ValidateAttrUpdate(in.DocumentID, in.BlockUID, in.Attrs); err != nil {
return "", fmt.Errorf("update_block_attrs: %w", err)
}
return inp.ApplyEdit(in.DocumentID, []edit.Operation{edit.UpdateAttrs(in.BlockUID, in.Attrs)})
}
@ -1023,7 +1029,7 @@ func (moveBlock) Info() Info {
_keyBlockUID: stringProp("The uid of the block to move."),
"position": map[string]any{
_keyType: _typeString,
"enum": []string{
_keyEnum: []string{
string(positionBefore),
string(positionAfter),
},
@ -1095,5 +1101,9 @@ func (moveBlock) Execute(inp Input) (string, error) {
op = edit.MoveBefore(in.BlockUID, in.ReferenceBlockUID)
}
if err := inp.ValidateMove(in.DocumentID, in.BlockUID, in.ReferenceBlockUID); err != nil {
return "", fmt.Errorf("move_block: %w", err)
}
return inp.ApplyEdit(in.DocumentID, []edit.Operation{op})
}

View File

@ -613,7 +613,7 @@ func Test_replaceBlock_Info(t *testing.T) {
func Test_replaceBlock_Traits(t *testing.T) {
t.Parallel()
assert.Equal(t, Traits{Write: true}, replaceBlock{}.Traits())
assert.Equal(t, Traits{Write: true, Overwrites: true}, replaceBlock{}.Traits())
}
func Test_replaceBlock_Title(t *testing.T) {
@ -717,7 +717,7 @@ func Test_updateBlockText_Info(t *testing.T) {
func Test_updateBlockText_Traits(t *testing.T) {
t.Parallel()
assert.Equal(t, Traits{Write: true}, updateBlockText{}.Traits())
assert.Equal(t, Traits{Write: true, Overwrites: true}, updateBlockText{}.Traits())
}
func Test_updateBlockText_Title(t *testing.T) {

View File

@ -36,7 +36,7 @@ const (
// _descChartType describes the optional chart-type property shared
// by the two query tools.
_descChartType = "Optional. One of line_chart, bar_chart, gauge_chart. When set, the result is the transformed series the metric block renders, plus a status, instead of the raw result — use it to check a query before putting it in a metric block."
_descChartType = "Optional. One of line_chart, bar_chart, gauge_chart. When set, the result describes what the metric block would draw — render status, series count, and each series' labels, point count and endpoints — instead of the raw data. Use it to check a query before putting it in a metric block; omit it when you need the values themselves."
)
// errUnknownDataSource is what a lookup reports for an id that names
@ -593,7 +593,7 @@ type queryPrometheus struct {
func (queryPrometheus) Info() Info {
return Info{
Name: NameQueryPrometheus,
Description: "Run a PromQL range query against a Prometheus data source over the given window. Returns the raw result by default, or the series a metric block would render when chart_type is set.",
Description: "Run a PromQL range query against a Prometheus data source over the given window. Returns the raw result by default, or a description of what a metric block would render when chart_type is set.",
Properties: dataSourceProps(map[string]any{
_keyQuery: stringProp("The PromQL expression to run."),
_keyChartType: stringProp(_descChartType),
@ -665,7 +665,74 @@ func (queryPrometheus) Execute(inp Input) (string, error) {
return result(&processor.QueryResult{Status: processor.QueryStatusNoData})
}
return result(res.Transform(in.ChartType))
return result(newChartPreview(res.Transform(in.ChartType)))
}
// _maxPreviewSeries caps how many series a chart check describes. A
// query behind a metric block draws a handful; one answering with more
// is already the wrong query, and listing them all would cost more than
// the check saves.
const _maxPreviewSeries = 10
// chartPreview is what a query answers with when a chart type was
// named. Naming one asks whether the query renders, not what it
// contains — so the shape of the answer is reported and the points
// behind it are not. A caller that wants the data omits chart_type and
// gets the raw result.
type chartPreview struct {
// Status is the render outcome: whether the data fits the chart.
Status processor.QueryStatus `json:"status"`
// SeriesCount is how many series the chart would draw, including
// any beyond the ones described.
SeriesCount int `json:"series_count"`
// Series describes the first few, each by its labels and extent.
Series []chartPreviewSeries `json:"series,omitempty"`
}
// chartPreviewSeries is one series as a chart check reports it.
type chartPreviewSeries struct {
// Labels are the series' labels, which become its legend entry.
Labels map[string]string `json:"labels,omitempty"`
// PointCount is how many points the series carries.
PointCount int `json:"point_count"`
// First and Last are its endpoints as [timestamp, value], enough
// to see the series covers the window and holds real values.
First [2]any `json:"first,omitempty"`
Last [2]any `json:"last,omitempty"`
}
// newChartPreview summarises a transformed result for a chart check.
func newChartPreview(qr *processor.QueryResult) chartPreview {
if qr == nil {
// NOCOV: Transform never returns nil; the guard keeps a future
// caller from panicking on one.
return chartPreview{Status: processor.QueryStatusNoData}
}
out := chartPreview{
Status: qr.Status,
SeriesCount: len(qr.Data),
}
for _, sr := range qr.Data[:min(len(qr.Data), _maxPreviewSeries)] {
row := chartPreviewSeries{
Labels: sr.Labels,
PointCount: len(sr.Metrics),
}
if len(sr.Metrics) > 0 {
row.First = sr.Metrics[0]
row.Last = sr.Metrics[len(sr.Metrics)-1]
}
out.Series = append(out.Series, row)
}
return out
}
// getSQLMetadataArgs is what get_sql_metadata is called with.
@ -879,7 +946,7 @@ type querySQL struct {
func (querySQL) Info() Info {
return Info{
Name: NameQuerySQL,
Description: "Run a read-only query against a PostgreSQL, MariaDB or MySQL data source. Returns columns and rows by default, or the series a metric block would render when chart_type is set. $__ macros ($__timeFilter, $__timeGroupAlias, …) are expanded against the window, and the row count is capped.",
Description: "Run a read-only query against a PostgreSQL, MariaDB or MySQL data source. Returns columns and rows by default, or a description of what a metric block would render when chart_type is set. $__ macros ($__timeFilter, $__timeGroupAlias, …) are expanded against the window, and the row count is capped.",
Properties: dataSourceProps(map[string]any{
_keyQuery: stringProp("The SQL query to run. For a chart, select a time column aliased \"time\" plus one or more numeric columns."),
_keyChartType: stringProp(_descChartType),
@ -965,7 +1032,7 @@ func runPostgreSQLQuery(
return result(&processor.QueryResult{Status: processor.QueryStatusNoData})
}
return result(res.Transform(ct))
return result(newChartPreview(res.Transform(ct)))
}
// runMySQLQuery serves query_sql for a MySQL or MariaDB data source.
@ -994,5 +1061,5 @@ func runMySQLQuery(
return result(&processor.QueryResult{Status: processor.QueryStatusNoData})
}
return result(res.Transform(ct))
return result(newChartPreview(res.Transform(ct)))
}

View File

@ -3,6 +3,7 @@ package tools
import (
"context"
"maps"
"strconv"
"testing"
"time"
@ -1437,3 +1438,82 @@ func Test_dataSourceProps(t *testing.T) {
assert.Contains(t, props, _keyDataSourceID)
assert.Contains(t, props, _keyQuery)
}
func Test_newChartPreview(t *testing.T) {
t.Parallel()
series := func(n int) []processor.QueryResultSeries {
out := make([]processor.QueryResultSeries, 0, n)
for i := range n {
out = append(out, processor.QueryResultSeries{
Labels: map[string]string{"i": strconv.Itoa(i)},
Metrics: [][2]any{{1, 1.5}, {2, 2.5}, {3, 3.5}},
})
}
return out
}
cc := map[string]struct {
Input *processor.QueryResult
Result chartPreview
}{
"Nil result reads as no data": {
Input: nil,
Result: chartPreview{Status: processor.QueryStatusNoData},
},
"Status without data carries no series": {
Input: &processor.QueryResult{Status: processor.QueryStatusNoData},
Result: chartPreview{Status: processor.QueryStatusNoData},
},
"Series report their labels and endpoints, never their points": {
Input: &processor.QueryResult{
Status: processor.QueryStatusOK,
Data: series(1),
},
Result: chartPreview{
Status: processor.QueryStatusOK,
SeriesCount: 1,
Series: []chartPreviewSeries{{
Labels: map[string]string{"i": "0"},
PointCount: 3,
First: [2]any{1, 1.5},
Last: [2]any{3, 3.5},
}},
},
},
"Empty series reports its count without endpoints": {
Input: &processor.QueryResult{
Status: processor.QueryStatusOK,
Data: []processor.QueryResultSeries{{Labels: map[string]string{"i": "0"}}},
},
Result: chartPreview{
Status: processor.QueryStatusOK,
SeriesCount: 1,
Series: []chartPreviewSeries{{Labels: map[string]string{"i": "0"}}},
},
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
assert.Equal(t, c.Result, newChartPreview(c.Input))
})
}
// the cap bounds the answer without hiding how much there was.
t.Run("More series than the cap are counted but not listed", func(t *testing.T) {
t.Parallel()
got := newChartPreview(&processor.QueryResult{
Status: processor.QueryStatusOK,
Data: series(_maxPreviewSeries + 5),
})
assert.Equal(t, _maxPreviewSeries+5, got.SeriesCount)
assert.Len(t, got.Series, _maxPreviewSeries)
})
}

View File

@ -11,8 +11,10 @@ import (
)
// _defaultDocumentIcon is the icon assigned to assistant-created
// documents when the model doesn't pick one.
const _defaultDocumentIcon = "lucide:file"
// documents when the model doesn't pick one. It matches what the
// editor gives a document created by hand, so a document's origin is
// not visible in the sidebar.
const _defaultDocumentIcon = "mingcute:document-2-fill"
// listDocumentsArgs is what list_documents is called with.
type listDocumentsArgs struct {
@ -213,7 +215,7 @@ func (createDocument) Info() Info {
Description: "Create a new document. Returns {document_id, branch_id}. The document starts with one empty paragraph; immediately follow up with append_block / insert_block calls to populate it.",
Properties: map[string]any{
_keyName: stringProp("Display name for the new document."),
document.AttrIcon: stringProp("Lucide icon identifier (e.g. \"lucide:file-text\"). Defaults to \"lucide:file\" when empty."),
document.AttrIcon: stringProp("Iconify identifier, as \"collection:name\". The product's own icons are MingCute fills (e.g. \"mingcute:file-code-fill\"); prefer one so the document matches the rest of the sidebar. Defaults to \"mingcute:document-2-fill\" when empty."),
"parent_id": stringProp("Optional parent document id. Omit to create at the org root."),
},
Required: []string{_keyName},
@ -305,7 +307,7 @@ type deleteDocument struct{}
func (deleteDocument) Info() Info {
return Info{
Name: NameDeleteDocument,
Description: "Delete a document. This is destructive — the user is always asked to confirm and there is no auto-approve.",
Description: "Delete a document and every document nested under it — the whole subtree goes, and none of it can be restored. This is destructive — the user is always asked to confirm and there is no auto-approve.",
Properties: documentIDProp("The document id to delete."),
Required: []string{_keyDocumentID},
}
@ -346,14 +348,31 @@ func (deleteDocument) Summary(inp DescribeInput) (ActionSummary, error) {
return ActionSummary{}, fmt.Errorf("%s: fetch document: %w", NameDeleteDocument, err)
}
summary := "Delete " + doc.DocumentName
// the delete cascades, so a card naming only the document would
// have the user approve a subtree they were never shown.
if n, err := inp.DescendantCount(in.DocumentID); err == nil && n > 0 {
summary += fmt.Sprintf(" and the %s nested under it", pluralPages(n))
}
return ActionSummary{
Tool: NameDeleteDocument,
DocumentID: doc.ID,
DocumentName: doc.DocumentName,
Summary: "Delete " + doc.DocumentName,
Summary: summary,
}, nil
}
// pluralPages renders a nested-document count for the confirm card.
func pluralPages(n int) string {
if n == 1 {
return "1 page"
}
return fmt.Sprintf("%d pages", n)
}
// Execute deletes the document and refreshes the tree.
func (deleteDocument) Execute(inp Input) (string, error) {
var in deleteDocumentArgs
@ -512,7 +531,7 @@ type setDocumentIcon struct{}
func (setDocumentIcon) Info() Info {
return Info{
Name: NameSetDocumentIcon,
Description: "Change a document's icon identifier (Lucide-style, e.g. \"lucide:rocket\").",
Description: "Change a document's icon. Takes an Iconify identifier, as \"collection:name\"; the product's own icons are MingCute fills (e.g. \"mingcute:rocket-fill\").",
Properties: map[string]any{
_keyDocumentID: stringProp(_descDocumentID),
document.AttrIcon: stringProp("The new icon identifier."),

View File

@ -449,6 +449,56 @@ func Test_deleteDocument_Summary(t *testing.T) {
assert.Equal(t, "Runbook", got.DocumentName)
assert.Equal(t, "Delete Runbook", got.Summary)
// the delete cascades, so a card naming only the document would
// have the user approve a subtree they were never shown.
withKids := stubDocumentDB()
withKids.FetchDocumentTreeFunc = func(context.Context, string) (document.Summaries, error) {
return document.Summaries{{
ID: _testDocID,
Children: document.Summaries{
{ID: xid.New()},
{ID: xid.New(), Children: document.Summaries{{ID: xid.New()}}},
},
}}, nil
}
got, err = deleteDocument{}.Summary(testInput(
testDeps(withKids, nil, nil), NameDeleteDocument,
`{"document_id":"`+_testDocID.String()+`"}`,
))
require.NoError(t, err)
assert.Equal(t, "Delete Runbook and the 3 pages nested under it", got.Summary)
// one reads as one.
withOneKid := stubDocumentDB()
withOneKid.FetchDocumentTreeFunc = func(context.Context, string) (document.Summaries, error) {
return document.Summaries{{
ID: _testDocID,
Children: document.Summaries{{ID: xid.New()}},
}}, nil
}
got, err = deleteDocument{}.Summary(testInput(
testDeps(withOneKid, nil, nil), NameDeleteDocument,
`{"document_id":"`+_testDocID.String()+`"}`,
))
require.NoError(t, err)
assert.Equal(t, "Delete Runbook and the 1 page nested under it", got.Summary)
// a tree that cannot be read still names the document: the count is
// worth having, never worth failing the confirmation over.
treeFails := stubDocumentDB()
treeFails.FetchDocumentTreeFunc = func(context.Context, string) (document.Summaries, error) {
return nil, assert.AnError
}
got, err = deleteDocument{}.Summary(testInput(
testDeps(treeFails, nil, nil), NameDeleteDocument,
`{"document_id":"`+_testDocID.String()+`"}`,
))
require.NoError(t, err)
assert.Equal(t, "Delete Runbook", got.Summary)
// the document it names has to resolve; the failure is passed on
// rather than described around.
_, err = deleteDocument{}.Summary(testInput(testDeps(failingDocumentDB(), nil, nil), NameDeleteDocument, requiredArgs(t, NameDeleteDocument)))

View File

@ -42,6 +42,13 @@ type docSummaryEntry struct {
// (heading.level, callout.icon, code.language, list task
// item.checked). Opaque attrs (metric configs) are omitted.
Attrs map[string]any `json:"attrs,omitempty"`
// HasChildren marks an entry holding addressable blocks the walk
// does not emit — a sublist under a list item, the internals of a
// macro. Their text is folded into this entry's Text, so without
// the flag the summary reads as though this row were the whole of
// it, and the uids an edit would need are invisible.
HasChildren bool `json:"has_children,omitempty"`
}
// walkDocForAssistant builds the flat list of summary entries the
@ -102,7 +109,7 @@ func (w *docWalker) walkLevel(blocks []document.Block, depth int, parentUID stri
document.BlockNodeTaskItem,
document.BlockNodeSplitDoc,
document.BlockNodeParamList:
w.emit(b, uid, depth, parentUID)
w.emit(b, uid, depth, parentUID, hasAddressableChildren(b))
case document.BlockNodeBulletList,
document.BlockNodeOrderedList,
@ -110,28 +117,60 @@ func (w *docWalker) walkLevel(blocks []document.Block, depth int, parentUID stri
document.BlockNodeCalloutBlock,
document.BlockNodeBlockquote,
document.BlockNodeMetricGrid:
w.emit(b, uid, depth, parentUID)
// the walk lists this block's children itself, so there is
// nothing hidden below it to point the reader at.
w.emit(b, uid, depth, parentUID, false)
w.walkLevel(b.Content, depth+1, uid)
default:
}
}
}
// emit creates the standard summary entry for a block and appends
// it to the walker's output.
func (w *docWalker) emit(b document.Block, uid string, depth int, parentUID string) {
// emit creates the standard summary entry for a block and appends it to
// the walker's output. hasChildren says whether the block holds
// addressable blocks this walk is not going to emit.
func (w *docWalker) emit(
b document.Block,
uid string,
depth int,
parentUID string,
hasChildren bool,
) {
entry := docSummaryEntry{
UID: uid,
Kind: canonicalKindForPM(b.Type),
Text: b.Flatten(),
Depth: depth,
ParentUID: parentUID,
Attrs: summaryAttrs(b),
UID: uid,
Kind: canonicalKindForPM(b.Type),
Text: b.Flatten(),
Depth: depth,
ParentUID: parentUID,
Attrs: summaryAttrs(b),
HasChildren: hasChildren,
}
w.out = append(w.out, entry)
}
// hasAddressableChildren reports whether the block holds addressable
// blocks beyond the text the entry already shows. A leading paragraph
// is that text — every list item has one — so it is not what the flag
// is for; anything after it is structure the summary does not list.
func hasAddressableChildren(b document.Block) bool {
for i, cb := range b.Content {
if i == 0 && cb.Type == document.BlockNodeParagraph {
continue
}
if uid, ok := cb.UID(); ok && uid != "" {
return true
}
if hasAddressableChildren(cb) {
return true
}
}
return false
}
// canonicalKindForPM maps a ProseMirror node type to the canonical kind
// name the assistant uses elsewhere. Keeping the AI on one vocabulary
// (snake_case canonical names) avoids surprises when it reads a summary,

View File

@ -58,6 +58,31 @@ func Test_walkDocForAssistant(t *testing.T) {
{UID: "li1", Kind: "list_item", Text: "one", Depth: 1, ParentUID: "l1"},
},
},
"List item holding a sublist is flagged, since the walk stops there": {
Blocks: []document.Block{
pmBlock(document.BlockNodeBulletList, "l1", nil,
pmBlock(document.BlockNodeListItem, "li1", nil,
pmBlock(document.BlockNodeParagraph, "p1", nil, pmText("parent")),
pmBlock(document.BlockNodeBulletList, "l2", nil,
pmBlock(document.BlockNodeListItem, "li2", nil,
pmBlock(document.BlockNodeParagraph, "p2", nil, pmText("child")),
),
),
),
),
},
Result: []docSummaryEntry{
{UID: "l1", Kind: "bullet_list", Text: "parent child"},
{
UID: "li1",
Kind: "list_item",
Text: "parent child",
Depth: 1,
ParentUID: "l1",
HasChildren: true,
},
},
},
"Task list surfaces checked attr on items": {
Blocks: []document.Block{
pmBlock(document.BlockNodeTaskList, "t1", nil,
@ -91,7 +116,7 @@ func Test_walkDocForAssistant(t *testing.T) {
),
},
Result: []docSummaryEntry{
{UID: "s1", Kind: "split_doc", Text: "API", Attrs: map[string]any{"inversed": true}},
{UID: "s1", Kind: "split_doc", Text: "API", Attrs: map[string]any{"inversed": true}, HasChildren: true},
},
},
"Param list emits a single entry without descending": {
@ -101,7 +126,7 @@ func Test_walkDocForAssistant(t *testing.T) {
),
},
Result: []docSummaryEntry{
{UID: "pl1", Kind: "split_doc_param_list", Text: "Body"},
{UID: "pl1", Kind: "split_doc_param_list", Text: "Body", HasChildren: true},
},
},
"Blocks without uid are skipped": {

View File

@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"log/slog"
"maps"
"slices"
"github.com/guregu/null/v5"
@ -15,6 +16,7 @@ import (
"github.com/oxynote/oxynote/server/core/internal/datasource"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/oxynote/oxynote/server/core/internal/search"
"github.com/oxynote/oxynote/server/core/pkg/errutil"
"github.com/oxynote/oxynote/server/core/pkg/sqlutil"
"github.com/rs/xid"
)
@ -233,9 +235,20 @@ func (i *input) DataSourceRunner(id xid.ID) (datasource.Runner, error) {
return i.runners.Runner(*ds), nil
}
// Document returns the document's default branch.
// Document returns the document's default branch. An id naming nothing
// in the session's organisation is reported as such rather than as the
// driver's own words, which say nothing a caller can act on.
func (i *input) Document(id xid.ID) (*document.Document, error) {
return i.db.FetchDocument(i.ctx, id, i.orgID, document.DefaultBranch)
doc, err := i.db.FetchDocument(i.ctx, id, i.orgID, document.DefaultBranch)
if err != nil {
if errutil.IsNotFound(err) {
return nil, errUnknownDocument
}
return nil, err
}
return doc, nil
}
// DocumentContent returns the document's parsed main-branch content.
@ -243,6 +256,24 @@ func (i *input) DocumentContent(id xid.ID) (document.Content, error) {
return i.db.FetchMainBranchContent(i.ctx, id, i.orgID)
}
// DescendantCount reports how many documents sit under the named one at
// any depth. A document that is not in the tree has none, which is what
// a caller asking about a missing id should hear.
func (i *input) DescendantCount(id xid.ID) (int, error) {
tree, err := i.DocumentTree()
if err != nil {
return 0, fmt.Errorf("fetching document tree: %w", err)
}
for _, s := range tree.Descendants() {
if s.ID == id {
return len(s.Children.Descendants()), nil
}
}
return 0, nil
}
// DocumentTree returns every document in the organisation as a nested
// summary tree.
func (i *input) DocumentTree() (document.Summaries, error) {
@ -391,6 +422,11 @@ func (i *input) ReadOffloaded(path string) (string, error) {
return i.offload.Read(i.ctx, path)
}
// errBranchProtected is what a content write reports for a document
// whose branch is protected. The message names the tool the caller
// still has, since reading a protected document stays allowed.
var errBranchProtected = errors.New("this document's branch is protected and takes no edits; read it with read_document_summary or ask someone with access to unprotect it")
// resolveDoc loads the default branch of the given document and returns
// the ids the edit client needs. The lookup also acts as the cross-org
// safety check — Document scopes by orgID so a docID from another
@ -401,6 +437,15 @@ func (i *input) resolveDoc(documentID xid.ID) (docRef, error) {
return docRef{}, fmt.Errorf("fetching document: %w", err)
}
// a protected branch takes no write but core's own, and this is not
// one: the operations endpoint applies the batch to the live Y.Doc
// regardless, and only the persist behind it is refused. Without
// this the call reports success, the change shows up in every open
// editor, and it is gone at the next load.
if doc.Protected {
return docRef{}, errBranchProtected
}
return docRef{DocumentID: doc.ID, BranchID: doc.BranchID}, nil
}
@ -434,6 +479,16 @@ func (i *input) ApplyEdit(documentID xid.ID, ops []edit.Operation) (string, erro
return "", fmt.Errorf("applying edit: %w", err)
}
// nothing committed and something to say why is a failed call, not
// a result describing a no-op: every write tool ships a single
// operation, so there is no partial success to report. Returning an
// error is what makes the surfaces treat it as one — the MCP bridge
// marks the result isError, and the assistant sees a failure it can
// correct rather than a success it will summarise.
if res.Applied == 0 && len(res.Errors) > 0 {
return "", fmt.Errorf("applying edit: %s", joinOpErrors(res.Errors))
}
if len(res.Errors) > 0 {
i.log.Warn(
"edit partial failure",
@ -481,6 +536,83 @@ func (i *input) ValidatePlacement(documentID xid.ID, referenceUID string, b bloc
return block.ValidateInContainer(parent, b)
}
// ValidateAttrUpdate checks the attributes an update would leave on the
// block. The update names some attributes and preserves the rest, so
// the rules run against the merge rather than the payload — otherwise
// setting one attribute would be judged as if every other were absent.
//
// A block whose ProseMirror type has no canonical counterpart is a
// wrapper item (a list item, a macro internal). Those carry attributes
// the canonical model does not describe, so there is nothing to check.
func (i *input) ValidateAttrUpdate(documentID xid.ID, blockUID string, attrs map[string]any) error {
content, err := i.DocumentContent(documentID)
if err != nil {
return fmt.Errorf("fetching content: %w", err)
}
target, ok := content.Content.FindByUID(blockUID)
if !ok {
// the uid does not resolve; the edit backend reports that as
// the call's result.
return nil
}
t, ok := block.CanonicalType(target.Type)
if !ok {
return nil
}
merged := make(document.Attributes, len(target.Attrs)+len(attrs))
maps.Copy(merged, target.Attrs)
maps.Copy(merged, attrs)
return block.ValidateAttrs(t, merged)
}
// ValidateMove checks that the moved block may live where the move
// would put it: among the children of whatever holds the reference.
// Only placement is in question — the block is already in the document,
// so its content has been through validation once already and may
// contain shapes this layer can no longer express.
//
// A wrapper item (a list item, a macro internal) has no canonical type
// to check against a container's allowed set, so it is held to a
// narrower rule instead: it may only land in a container of the kind it
// already sits in, which permits reordering and moving between two
// lists while keeping a list item from landing at the document root.
func (i *input) ValidateMove(documentID xid.ID, blockUID, referenceUID string) error {
content, err := i.DocumentContent(documentID)
if err != nil {
return fmt.Errorf("fetching content: %w", err)
}
moved, ok := content.Content.FindByUID(blockUID)
if !ok {
// neither uid resolving is the edit backend's report to make.
return nil
}
target, ok := content.Content.FindParentTypeByUID(referenceUID)
if !ok {
return nil
}
t, ok := block.CanonicalType(moved.Type)
if !ok {
source, ok := content.Content.FindParentTypeByUID(blockUID)
if !ok || source == target {
return nil
}
return fmt.Errorf(
"%s cannot move from %s into %s; it belongs to the block that holds it",
moved.Type, source, target,
)
}
return block.AllowedInContainer(target, t)
}
// NotifyTreeChange invokes the tree notifier when one is configured.
// Safe to call from any tool — a nil notifier silently no-ops so tests
// that don't wire one don't trip.

View File

@ -178,6 +178,24 @@ func stubDocumentDB() *DBMock {
}
}
// protectedDocumentDB answers with a document whose branch is
// protected, which is the one branch a tool's write may not reach.
func protectedDocumentDB() *DBMock {
branchID := xid.New()
return &DBMock{
FetchDocumentFunc: func(_ context.Context, id xid.ID, orgID, _ string) (*document.Document, error) {
return &document.Document{
BranchID: branchID,
DocumentName: _stubDocumentName,
ID: id,
OrganizationID: orgID,
Protected: true,
}, nil
},
}
}
// failingDocumentDB refuses every document lookup.
func failingDocumentDB() *DBMock {
return &DBMock{
@ -634,6 +652,7 @@ func Test_input_ApplyEdit(t *testing.T) {
Applier *EditApplierMock
Result string
Touched []xid.ID
Applies int
Err error
}{
"Error returned by db.FetchDocument": {
@ -652,7 +671,25 @@ func Test_input_ApplyEdit(t *testing.T) {
return edit.Result{}, assert.AnError
},
},
Err: assert.AnError,
Applies: 1,
Err: assert.AnError,
},
"Protected branch is refused before anything is applied": {
DB: protectedDocumentDB(),
Applier: stubApplier(),
Err: errBranchProtected,
},
"Nothing applied is a failure, not a result": {
DB: stubDocumentDB(),
Applier: &EditApplierMock{
ApplyFunc: func(context.Context, xid.ID, xid.ID, []edit.Operation, bool) (edit.Result, error) {
return edit.Result{
Errors: []edit.OpError{{Index: 0, Message: "uid not found"}},
}, nil
},
},
Applies: 1,
Err: errors.New("applying edit: uid not found"),
},
"Partial failure still reports the outcome": {
DB: stubDocumentDB(),
@ -666,12 +703,14 @@ func Test_input_ApplyEdit(t *testing.T) {
},
Result: `{"applied":1,"errors":[{"index":1,"message":"uid not found"}]}`,
Touched: []xid.ID{docID},
Applies: 1,
},
"Applied": {
DB: stubDocumentDB(),
Applier: stubApplier(),
Result: `{"applied":1,"errors":[]}`,
Touched: []xid.ID{docID},
Applies: 1,
},
}
@ -687,6 +726,11 @@ func Test_input_ApplyEdit(t *testing.T) {
// an edit that never landed has changed nothing to report.
assert.Equal(t, c.Touched, inp.touched)
// a refusal the tools own — a protected branch — stops
// before the batch is shipped, rather than letting it land
// and be undone by the persist behind it.
assert.Len(t, c.Applier.ApplyCalls(), c.Applies)
if err != nil {
return
}
@ -705,6 +749,239 @@ func Test_input_ApplyEdit(t *testing.T) {
}
}
// contentWithBlocks builds a DB whose main-branch content is the given
// top-level blocks, for the validators that read the document to decide.
func contentWithBlocks(blocks ...document.Block) *DBMock {
return &DBMock{
FetchMainBranchContentFunc: func(context.Context, xid.ID, string) (document.Content, error) {
return document.Content{
Content: document.RootBlock{Content: blocks},
}, nil
},
}
}
func Test_input_ValidateAttrUpdate(t *testing.T) {
t.Parallel()
// a heading at level 2, a metric inside its grid, and a task item —
// a typed block with attr rules, a typed block with enum rules, and
// a wrapper item the canonical model does not name.
contentDB := contentWithBlocks(
document.Block{
Type: document.BlockNodeHeading,
Attrs: document.Attributes{
document.AttrUID: "head",
document.AttrLevel: 2,
},
},
document.Block{
Type: document.BlockNodeMetricGrid,
Attrs: document.Attributes{document.AttrUID: "grid"},
Content: []document.Block{{
Type: document.BlockNodeMetricBlock,
Attrs: document.Attributes{
document.AttrUID: "metric",
document.AttrSimulationPreset: "cpu_usage",
},
}},
},
document.Block{
Type: document.BlockNodeTaskList,
Attrs: document.Attributes{document.AttrUID: "tasks"},
Content: []document.Block{{
Type: document.BlockNodeTaskItem,
Attrs: document.Attributes{document.AttrUID: "task"},
}},
},
)
cc := map[string]struct {
DB *DBMock
UID string
Attrs map[string]any
Err error
}{
"Error returned by db.FetchMainBranchContent": {
DB: &DBMock{
FetchMainBranchContentFunc: func(context.Context, xid.ID, string) (document.Content, error) {
return document.Content{}, assert.AnError
},
},
UID: "head",
Attrs: map[string]any{document.AttrLevel: 2},
Err: assert.AnError,
},
"Unresolved uid is left to the backend": {
DB: contentDB,
UID: "missing",
Attrs: map[string]any{document.AttrLevel: 9},
},
"Wrapper item carries attrs the canonical model does not name": {
DB: contentDB,
UID: "task",
Attrs: map[string]any{"checked": true},
},
"Allowed heading level": {
DB: contentDB,
UID: "head",
Attrs: map[string]any{document.AttrLevel: 3},
},
"Heading level outside the range": {
DB: contentDB,
UID: "head",
Attrs: map[string]any{document.AttrLevel: 9},
Err: assert.AnError,
},
"Unrelated attr keeps the level already on the block": {
DB: contentDB,
UID: "head",
Attrs: map[string]any{document.AttrIcon: "lucide:hash"},
},
"Unknown simulation preset": {
DB: contentDB,
UID: "metric",
Attrs: map[string]any{document.AttrSimulationPreset: "solar_flares"},
Err: assert.AnError,
},
"Known simulation preset": {
DB: contentDB,
UID: "metric",
Attrs: map[string]any{document.AttrSimulationPreset: "error_rate"},
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
inp := testInput(testDeps(c.DB, nil, nil), NameUpdateBlockAttrs, `{}`)
err := inp.ValidateAttrUpdate(_testDocID, c.UID, c.Attrs)
testutil.AssertEqualError(t, c.Err, err)
})
}
}
func Test_input_ValidateMove(t *testing.T) {
t.Parallel()
// two lists, so a wrapper item has both a same-kind destination and
// a root one to be tested against, plus a split_doc whose right side
// holds the one block type that may not sit at the root.
contentDB := contentWithBlocks(
document.Block{
Type: document.BlockNodeParagraph,
Attrs: document.Attributes{document.AttrUID: "root-p"},
},
document.Block{
Type: document.BlockNodeBulletList,
Attrs: document.Attributes{document.AttrUID: "list-a"},
Content: []document.Block{
{
Type: document.BlockNodeListItem,
Attrs: document.Attributes{document.AttrUID: "item-a1"},
},
{
Type: document.BlockNodeListItem,
Attrs: document.Attributes{document.AttrUID: "item-a2"},
},
},
},
document.Block{
Type: document.BlockNodeBulletList,
Attrs: document.Attributes{document.AttrUID: "list-b"},
Content: []document.Block{{
Type: document.BlockNodeListItem,
Attrs: document.Attributes{document.AttrUID: "item-b1"},
}},
},
document.Block{
Type: document.BlockNodeSplitDoc,
Attrs: document.Attributes{document.AttrUID: "split"},
Content: []document.Block{{
Type: document.BlockNodeSplitDocRight,
Content: []document.Block{{
Type: document.BlockNodeTitledCodeBlock,
Attrs: document.Attributes{document.AttrUID: "right-code"},
}},
}},
},
)
cc := map[string]struct {
DB *DBMock
UID string
Ref string
Err error
}{
"Error returned by db.FetchMainBranchContent": {
DB: &DBMock{
FetchMainBranchContentFunc: func(context.Context, xid.ID, string) (document.Content, error) {
return document.Content{}, assert.AnError
},
},
UID: "root-p",
Ref: "list-a",
Err: assert.AnError,
},
"Unresolved moved uid is left to the backend": {
DB: contentDB,
UID: "missing",
Ref: "root-p",
},
"Unresolved reference is left to the backend": {
DB: contentDB,
UID: "root-p",
Ref: "missing",
},
"Root block beside a root reference": {
DB: contentDB,
UID: "root-p",
Ref: "list-a",
},
"Macro internal out to the document root is refused": {
DB: contentDB,
UID: "right-code",
Ref: "root-p",
Err: assert.AnError,
},
"Root block onto a split_doc right side is refused": {
DB: contentDB,
UID: "root-p",
Ref: "right-code",
Err: assert.AnError,
},
"List item reordered within its own list": {
DB: contentDB,
UID: "item-a1",
Ref: "item-a2",
},
"List item moved into another list of the same kind": {
DB: contentDB,
UID: "item-a1",
Ref: "item-b1",
},
"List item out to the document root is refused": {
DB: contentDB,
UID: "item-a1",
Ref: "root-p",
Err: assert.AnError,
},
}
for cn, c := range cc {
t.Run(cn, func(t *testing.T) {
t.Parallel()
inp := testInput(testDeps(c.DB, nil, nil), NameMoveBlock, `{}`)
err := inp.ValidateMove(_testDocID, c.UID, c.Ref)
testutil.AssertEqualError(t, c.Err, err)
})
}
}
func Test_input_ValidatePlacement(t *testing.T) {
t.Parallel()

View File

@ -1,5 +1,10 @@
package tools
import (
"github.com/oxynote/oxynote/server/core/internal/assistant/block"
"github.com/oxynote/oxynote/server/core/internal/document"
)
// JSON-schema literals shared by the tool schemas.
const (
// _keyType is the JSON-schema type key.
@ -14,9 +19,16 @@ const (
// _typeArray is the JSON-schema array type.
_typeArray = "array"
// _typeInteger is the JSON-schema integer type.
_typeInteger = "integer"
// _keyDescription is the JSON-schema description key.
_keyDescription = "description"
// _keyEnum is the JSON-schema enum key, naming the values a
// property accepts.
_keyEnum = "enum"
// _keyDocumentID is the shared document-id property name.
_keyDocumentID = "document_id"
@ -59,27 +71,126 @@ const (
_descTargetDocumentID = "The target document id."
)
// _blockSchema is the JSON schema shared by every write tool that takes
// a canonical Block argument. The full shape is documented in the
// system prompt; this object only enumerates the top-level fields so
// the model surfaces the correct argument names.
var _blockSchema = map[string]any{
_keyType: _typeObject,
_keyDescription: "A canonical block. See the system prompt for the per-type schema.",
"properties": map[string]any{
_keyType: map[string]any{_keyType: _typeString},
"uid": map[string]any{_keyType: _typeString},
"text": map[string]any{_keyType: _typeString},
"attrs": map[string]any{_keyType: _typeObject},
"items": map[string]any{_keyType: _typeArray},
"children": map[string]any{_keyType: _typeArray},
"task_items": map[string]any{_keyType: _typeArray},
"left": map[string]any{_keyType: _typeArray},
"right": map[string]any{_keyType: _typeArray},
"header": map[string]any{_keyType: _typeString},
"params": map[string]any{_keyType: _typeArray},
},
"required": []string{_keyType},
// The block argument's schema, in the two shapes the write tools need.
var (
// _blockSchema is for the tools that place a block relative to
// another, where the reference's parent decides which types are
// legal — so the enum is every canonical type and the container
// rule is checked server-side.
_blockSchema = blockSchema(block.Types())
// _rootBlockSchema is for the tools that place a block at the
// document root, where the legal set is fixed and can be stated
// outright.
_rootBlockSchema = blockSchema(block.RootTypes())
)
// blockSchema builds the block argument's schema with the given set of
// legal types. The per-field prose is deliberately terse: this schema is
// published to MCP clients, which may never receive the server
// instructions, and to the assistant, whose system prompt already
// carries the full block model and pays for every word on every turn.
func blockSchema(types []string) map[string]any {
return map[string]any{
_keyType: _typeObject,
_keyDescription: "A canonical block. Which fields apply depends on type; supply only that type's own.",
"properties": map[string]any{
_keyType: map[string]any{
_keyType: _typeString,
_keyDescription: "The block's canonical type.",
_keyEnum: types,
},
"uid": map[string]any{
_keyType: _typeString,
_keyDescription: "Leave unset. Uids are generated server-side and are the handle later edits use.",
},
"text": map[string]any{
_keyType: _typeString,
_keyDescription: "Inline text for paragraph, heading, blockquote, code, titled_code, mermaid and callout. Raw inside code, titled_code and mermaid; minimal markdown elsewhere. One block is one paragraph — never embed newlines to make several.",
},
"attrs": map[string]any{
_keyType: _typeObject,
_keyDescription: "Per-type attributes. Required where the type says so: heading level, titled_code title, image and figma src. Also code language, callout icon, image alt and width, and a metric's chart configuration, whose width is compact, standard or wide.",
"properties": attrProps(),
},
"items": map[string]any{
_keyType: _typeArray,
_keyDescription: "Entries of bullet_list and ordered_list, which are paragraphs; items of metric_grid, and of blockquote or callout when not using text.",
},
"children": map[string]any{
_keyType: _typeArray,
_keyDescription: "Blocks nested under a single list entry. A list entry is a paragraph; anything else nested under it goes here, never in the entry itself and never in a second entry.",
},
"task_items": map[string]any{
_keyType: _typeArray,
_keyDescription: "Rows of task_list, each {checked, block}.",
},
"left": map[string]any{
_keyType: _typeArray,
_keyDescription: "split_doc concept side. Must start with a level-1 heading; split_doc_param_list is legal only here.",
},
"right": map[string]any{
_keyType: _typeArray,
_keyDescription: "split_doc example side. Holds titled_code or metric only, and is the one place titled_code is legal.",
},
"header": map[string]any{
_keyType: _typeString,
_keyDescription: "split_doc_param_list heading, as plain text.",
},
"params": map[string]any{
_keyType: _typeArray,
_keyDescription: "split_doc_param_list rows, each {name, type, description}.",
},
},
"required": []string{_keyType},
}
}
// _enumAttrs are the metric attributes whose legal values are a fixed
// set, listed in the order the block model introduces them.
//
// width is deliberately absent. attrs is one field shared by every
// block type, so a name means the same thing everywhere it appears,
// and width is a metric's size but an image's pixel width. It carries
// no enum here and stays described in the block model instead.
var _enumAttrs = []struct {
// Name is the attribute's key inside attrs.
Name string
// Desc names the block the attribute belongs to and what it sets.
Desc string
}{
{document.AttrVisualizationType, "metric: the chart kind."},
{document.AttrTimeRange, "metric: the window queried."},
{document.AttrRefreshInterval, "metric: how often the chart re-queries."},
{document.AttrUnitType, "metric: the unit values are read in. With custom, put the label in unitCustom."},
{document.AttrSimulationPreset, "metric: draws this generated series in place of the query's own result, for a block documenting a metric that has no real data yet. Omit it to chart the query."},
}
// attrProps builds the attrs sub-schema. Only attributes with a fixed
// value set appear: publishing the set as an enum is what lets a client
// reject a bad value itself, rather than learning the values from prose
// it may never have been given.
func attrProps() map[string]any {
enums := block.MetricEnums()
out := map[string]any{
document.AttrLevel: map[string]any{
_keyType: _typeInteger,
_keyDescription: "heading: the level. Required on a heading.",
_keyEnum: []int{1, 2, 3},
},
}
for _, a := range _enumAttrs {
out[a.Name] = map[string]any{
_keyType: _typeString,
_keyDescription: a.Desc,
_keyEnum: enums[a.Name],
}
}
return out
}
// stringProp builds a JSON-schema string property with a description.

View File

@ -3,7 +3,10 @@ package tools
import (
"testing"
"github.com/oxynote/oxynote/server/core/internal/assistant/block"
"github.com/oxynote/oxynote/server/core/internal/document"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func Test_stringProp(t *testing.T) {
@ -19,3 +22,38 @@ func Test_documentIDProp(t *testing.T) {
map[string]any{"document_id": map[string]any{"type": "string", "description": "a doc"}},
documentIDProp("a doc"))
}
func Test_attrProps(t *testing.T) {
t.Parallel()
got := attrProps()
// heading level is the one non-metric enum, and the only integer
// one: a client sending "2" rather than 2 is sending the wrong type.
level, ok := got[document.AttrLevel].(map[string]any)
require.True(t, ok)
assert.Equal(t, _typeInteger, level[_keyType])
assert.Equal(t, []int{1, 2, 3}, level[_keyEnum])
// every metric enum is published except width, whose name means a
// size on an image and so cannot carry one set of values here.
enums := block.MetricEnums()
delete(enums, document.AttrWidth)
require.Len(t, got, len(enums)+1)
for attr, values := range enums {
prop, pok := got[attr].(map[string]any)
require.True(t, pok, "attr %q is missing from the schema", attr)
assert.Equal(t, _typeString, prop[_keyType])
assert.Equal(t, values, prop[_keyEnum], "attr %q publishes the wrong values", attr)
assert.NotEmpty(t, prop[_keyDescription], "attr %q has no description", attr)
}
// the sub-schema is reachable from the block argument itself, which
// is what a client actually reads.
attrs, ok := _blockSchema["properties"].(map[string]any)["attrs"].(map[string]any)
require.True(t, ok)
assert.Equal(t, got, attrs["properties"])
}

View File

@ -45,7 +45,7 @@ type searchDocuments struct {
func (searchDocuments) Info() Info {
return Info{
Name: NameSearchDocuments,
Description: "Full-text search across all documents in the organisation for blocks whose text matches the query. Returns hits with document_id, document_name, block_uid, text. Use this to find relevant context before editing or to surface candidate documents the user might be asking about.",
Description: "Full-text search across all documents in the organisation for blocks whose text matches the query. Returns hits with document_id, document_name, block_uid, text. Use this to find relevant context before editing or to surface candidate documents the user might be asking about. A hit addresses the innermost block holding the text, which may sit below anything read_document_summary lists — read_block resolves it.",
Properties: map[string]any{
_keyQuery: stringProp("The full-text search query (typo-tolerant, matches block text)."),
"limit": map[string]any{

File diff suppressed because one or more lines are too long

View File

@ -75,6 +75,15 @@ type Traits struct {
// for text edits is not consent to delete a document.
Destructive bool
// Overwrites indicates the write replaces content the caller did
// not name: the target's nested blocks go with it, and so do the
// uids comments and hooks hang off. It is what MCP's destructive
// hint reports, and it is deliberately not Destructive — a client
// deciding whether to auto-approve needs to know, while an
// approve-all inside a chat turn is meant to cover exactly these
// edits.
Overwrites bool
// DataSource indicates the tool reads one of the organisation's
// outbound data-source connections rather than its documents. A
// surface that gates access by scope asks it: over MCP these tools
@ -168,6 +177,11 @@ type DescribeInput interface {
// DataSource should return the data source the id names, for a
// description that has to name its subject.
DataSource(dataSourceID xid.ID) (*datasource.DataSource, error)
// DescendantCount should report how many documents sit under the
// named one, at any depth, for a description that has to say how
// far a cascade reaches.
DescendantCount(id xid.ID) (int, error)
}
// DataSources is the organisation's outbound data-source connections as
@ -244,6 +258,14 @@ type Editor interface {
// ValidatePlacement should check that a block is legal next to, or
// in place of, the block referenceUID names.
ValidatePlacement(documentID xid.ID, referenceUID string, b block.Block) error
// ValidateAttrUpdate should check that setting attrs on the block
// blockUID names leaves it with attributes its type allows.
ValidateAttrUpdate(documentID xid.ID, blockUID string, attrs map[string]any) error
// ValidateMove should check that the block blockUID names may sit
// where a move relative to referenceUID would put it.
ValidateMove(documentID xid.ID, blockUID, referenceUID string) error
}
// Input is everything a tool needs to do its work: the call's arguments

View File

@ -6,6 +6,7 @@ import (
"strings"
"github.com/oxynote/oxynote/server/core/internal/assistant/block"
"github.com/oxynote/oxynote/server/core/internal/assistant/edit"
"github.com/oxynote/oxynote/server/core/pkg/strutil"
)
@ -20,6 +21,19 @@ func result(v any) (string, error) {
return string(data), nil
}
// joinOpErrors renders per-operation failures as one message. The
// index is left out: a tool sends a single operation, so naming its
// position says nothing the reader can act on.
func joinOpErrors(errs []edit.OpError) string {
msgs := make([]string, 0, len(errs))
for _, e := range errs {
msgs = append(msgs, e.Message)
}
return strings.Join(msgs, "; ")
}
// errRequired reports an argument the tool cannot act without.
func errRequired(key string) error {
return fmt.Errorf("%s is required", key)

View File

@ -14,9 +14,12 @@ const _resourceURIPrefix = "oxynote://documents/"
// annotations declares a tool's intent so MCP clients can gate calls:
// reads are read-only, writes say whether they can destroy content. A
// document tool works on the organization's own documents, so its world
// is closed; a data-source tool reaches whatever the connection points
// at, which is not.
// write counts as destructive here when it removes content outright and
// also when it overwrites content the caller did not name — both lose
// work a client would want to confirm first, which is the question the
// hint answers. A document tool works on the organization's own
// documents, so its world is closed; a data-source tool reaches whatever
// the connection points at, which is not.
func annotations(e tools.Entry) *mcp.ToolAnnotations {
out := &mcp.ToolAnnotations{
ReadOnlyHint: !e.Write,
@ -24,7 +27,7 @@ func annotations(e tools.Entry) *mcp.ToolAnnotations {
}
if e.Write {
out.DestructiveHint = new(e.Destructive)
out.DestructiveHint = new(e.Destructive || e.Overwrites)
}
return out

View File

@ -53,6 +53,13 @@ func Test_annotations(t *testing.T) {
return &v
}(),
},
"Overwriting write tool is destructive to a client": {
Entry: tools.Entry{Traits: tools.Traits{Write: true, Overwrites: true}},
Destructive: func() *bool {
v := true
return &v
}(),
},
}
for cn, c := range cc {

View File

@ -102,7 +102,11 @@ describe("01.redirect.global", { concurrent: false }, () => {
useDocumentAPIMock.mockReset()
})
it.for([{ page: "desktop-auth" }, { page: "verify-email" }])(
it.for([
{ page: "desktop-auth" },
{ page: "verify-email" },
{ page: "auth-error" },
])(
"lets the $page page pass without touching the session",
async ({ page }, { expect }) => {
const { fetchAuthSession } = arrange({})

View File

@ -6,8 +6,14 @@ export default defineNuxtRouteMiddleware(async (to) => {
// the deep link or fall back to /login itself. The verify-email info
// page must render for signed-out visitors (fresh email-password
// signups) and signed-in ones (change-email confirmations) alike, so
// neither the auth gate nor the skipAuth redirect may touch it.
if (to.name === "desktop-auth" || to.name === "verify-email") {
// neither the auth gate nor the skipAuth redirect may touch it. The
// auth-error page reports a failed authorization to whoever landed on
// it, signed in or not, and either redirect would drop that report.
if (
to.name === "desktop-auth" ||
to.name === "verify-email" ||
to.name === "auth-error"
) {
return
}

View File

@ -0,0 +1,48 @@
import { mockNuxtImport, mountSuspended } from "@nuxt/test-utils/runtime"
import { beforeEach, describe, it, vi } from "vitest"
import AuthError from "./auth-error.vue"
import { t } from "../components/test-helpers"
// route.query is a readonly proxy, so the route itself is what a test
// arranges
const { useRouteMock } = vi.hoisted(() => ({ useRouteMock: vi.fn() }))
mockNuxtImport("useRoute", () => useRouteMock)
async function mountError(query = "?error=invalid_client") {
useRouteMock.mockReturnValue({
query: Object.fromEntries(new URLSearchParams(query).entries()),
})
return await mountSuspended(AuthError)
}
// the useRoute module mock is an app-wide singleton every mount in the
// file shares
describe("<auth-error>", { concurrent: false }, () => {
// restoreMocks does not touch hand-made vi.fn() singletons in vitest 4
beforeEach(() => {
useRouteMock.mockReset()
})
it("tells the visitor the authorization failed", async ({ expect }) => {
const wrapper = await mountError()
expect(wrapper.text()).toContain(t("oauth.error.heading"))
expect(wrapper.text()).toContain(t("oauth.error.description"))
})
it("names the error code the server reported", async ({ expect }) => {
const wrapper = await mountError()
expect(wrapper.text()).toContain("invalid_client")
})
it("leaves the error description out of the page", async ({ expect }) => {
const wrapper = await mountError(
"?error=invalid_client&error_description=call+1-800-not-oxynote",
)
expect(wrapper.text()).not.toContain("1-800-not-oxynote")
})
})

View File

@ -0,0 +1,54 @@
<script lang="ts" setup>
// better-auth redirects here (its onAPIError.errorURL) when an auth or
// OAuth request fails before it can reach the client's redirect_uri —
// most often an MCP client presenting a client_id this deployment no
// longer knows. Only the error code is shown: error_description is
// arbitrary text taken straight from the query.
definePageMeta({
name: "auth-error",
middleware: (to) => {
if (!to.query.error || typeof to.query.error !== "string") {
return navigateTo("/")
}
},
})
const { t } = useI18n({ useScope: "global" })
useHead({
title: () => t("oauth.error.page-title"),
})
const route = useRoute()
const errorCode = computed(() => route.query.error as string)
</script>
<template>
<main
class="flex min-h-svh min-w-svw items-center justify-center bg-background text-foreground"
>
<div class="flex w-67 flex-col items-center gap-5">
<div class="flex flex-col items-center gap-6">
<Icon name="custom-icons:main-logo" class="size-12" />
<div class="text-lg font-semibold">
{{ $t("oauth.error.heading") }}
</div>
</div>
<div class="flex w-full flex-col gap-3">
<div class="text-center text-xs text-accent-foreground">
{{ $t("oauth.error.description") }}
</div>
<div class="text-center font-mono text-xs text-muted-foreground">
{{ $t("oauth.error.code", { code: errorCode }) }}
</div>
<ShadcnUiButton
type="button"
size="lg"
variant="ghost"
class="h-10 w-full text-muted-foreground"
@click="navigateTo('/')"
>
{{ $t("oauth.error.button") }}
</ShadcnUiButton>
</div>
</div>
</main>
</template>

View File

@ -16,6 +16,13 @@
"invalid-request": "This authorization request is invalid or has expired.",
"decision-failed": "Could not complete the authorization. Please try again."
}
},
"error": {
"page-title": "Authorization failed",
"heading": "Authorization failed",
"description": "Oxynote could not complete this request. Start the authorization again from the application you were connecting.",
"code": "Error code: {code}",
"button": "Back to Oxynote"
}
}
}