diff --git a/src/app/shared/nodes/generic-node/generic-node.html b/src/app/shared/nodes/generic-node/generic-node.html index dda92fc..ccea647 100644 --- a/src/app/shared/nodes/generic-node/generic-node.html +++ b/src/app/shared/nodes/generic-node/generic-node.html @@ -392,13 +392,13 @@
{{ entry.summary }}
- @if (entry.skillId; as skillId) { + @if (entry.definition; as definition) { } @@ -548,13 +548,13 @@
{{ entry.summary }}
- @if (entry.skillId; as skillId) { + @if (entry.definition; as definition) { } diff --git a/src/app/shared/nodes/generic-node/generic-node.spec.ts b/src/app/shared/nodes/generic-node/generic-node.spec.ts index ed5b88e..d636f29 100644 --- a/src/app/shared/nodes/generic-node/generic-node.spec.ts +++ b/src/app/shared/nodes/generic-node/generic-node.spec.ts @@ -392,33 +392,39 @@ describe('GenericNodeComponent', () => { }); }); - describe('viewing a skill\'s content', () => { - /** The shape `SkillBinding` produces: one property carrying the `@FieldRetriever(name = "Skills")`. */ + describe('viewing what a chosen value stands for', () => { + /** The shape `SkillBinding` produces: a value that is an id, plus where to read what it names. */ const skillItemSchema = { type: 'object', required: ['skillId'], properties: { - skillId: { type: 'string', 'x-retriever-name': 'Skills' } + skillId: { + type: 'string', + 'x-retriever-name': 'Skills', + 'x-retriever-definition-url': '/retriever/Skills/definitions' + } } }; - it('finds the item property backed by the skills catalog from the schema alone', () => { + it('takes the property and the endpoint from the schema, whatever the binding is called', () => { const component = fixture.componentInstance as any; const definitions = component.buildArrayFieldDefinitions({ type: 'object', properties: { - skills: { type: 'array', items: skillItemSchema } + anything: { type: 'array', items: skillItemSchema } } }); - expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]); + expect(definitions).toEqual([expect.objectContaining({ + path: 'anything', + definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' } + })]); }); it('finds it through a $ref, which is the shape the server actually sends', () => { - // Regression: SkillBinding is used by exactly one block type, so the bundler leaves it in - // this schema's own `definitions` and the array references it by $ref rather than inlining - // it - unlike the synthetic schema in the test above. + // Regression: a binding used by one block type stays in that schema's own `definitions`, and + // the array references it by $ref rather than inlining it. const component = fixture.componentInstance as any; const definitions = component.buildArrayFieldDefinitions({ @@ -429,32 +435,56 @@ describe('GenericNodeComponent', () => { } }); - expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]); + expect(definitions[0].definitionSource).toEqual({ property: 'skillId', url: '/retriever/Skills/definitions' }); }); - it('leaves skillIdProperty null for an array field with no skills-catalog property', () => { + it('offers nothing for a binding whose value declares no definition to read', () => { + // An MCP server's endpoint answers with a JSON schema, not readable text, so it declares none. const component = fixture.componentInstance as any; const definitions = component.buildArrayFieldDefinitions({ type: 'object', properties: { - mcpServers: { type: 'array', items: { type: 'object', properties: { serverName: { type: 'string' } } } } + mcpServers: { + type: 'array', + items: { + type: 'object', + properties: { serverName: { type: 'string', 'x-retriever-name': 'MCPServers' } } + } + } } }); - expect(definitions[0].skillIdProperty).toBeNull(); + expect(definitions[0].definitionSource).toBeNull(); }); - it('carries each row\'s skill id into its array item view', () => { + it('carries each row\'s own endpoint and value into its view', () => { const component = fixture.componentInstance as any; const definition = { - path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null, skillIdProperty: 'skillId', + path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null, + definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' }, ui: { structural: false, visibleWhen: [], enabledWhen: [] } }; const items = component.toArrayFieldItems(definition, [{ skillId: 'mcp-context-economy' }]); - expect(items).toEqual([expect.objectContaining({ index: 0, skillId: 'mcp-context-economy' })]); + expect(items).toEqual([expect.objectContaining({ + index: 0, + definition: { url: '/retriever/Skills/definitions', value: 'mcp-context-economy' } + })]); + }); + + it('leaves a row without the view when its value is missing', () => { + const component = fixture.componentInstance as any; + const definition = { + path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null, + definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' }, + ui: { structural: false, visibleWhen: [], enabledWhen: [] } + }; + + const items = component.toArrayFieldItems(definition, [{}]); + + expect(items[0].definition).toBeNull(); }); it('offers the button in every place the node renders an array row', () => { @@ -462,7 +492,11 @@ describe('GenericNodeComponent', () => { // fields inside a group fieldset, one for ungrouped fields - and the button was first added // to only one of them, so it never appeared on an LLM node's ungrouped Skills list. const component = fixture.componentInstance as any; - const row = { index: 0, summary: 'mcp-context-economy', skillId: 'mcp-context-economy' }; + const row = { + index: 0, + summary: 'mcp-context-economy', + definition: { url: '/retriever/Skills/definitions', value: 'mcp-context-economy' } + }; const arrayField = { path: 'skills', label: 'Skills', items: [row] }; // The node recomputes its display model from the (absent) schema on every check, which would // wipe the rows this test is here to render. @@ -485,17 +519,20 @@ describe('GenericNodeComponent', () => { } ]; component.cdr.detectChanges(); - const buttons = fixture.nativeElement.querySelectorAll('[title="View skill content"]'); + const buttons = fixture.nativeElement.querySelectorAll('[title="View content"]'); expect(buttons.length).toBe(2); }); - it('fetches the skill definition and opens it read-only', async () => { + it('reads the endpoint the row carries and opens the answer read-only', async () => { const component = fixture.componentInstance as any; const httpMock = TestBed.inject(HttpTestingController); const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType; open.mockResolvedValue(null); - const pending = component.viewSkillContent('mcp-context-economy'); + const pending = component.viewItemDefinition({ + url: '/retriever/Skills/definitions', + value: 'mcp-context-economy' + }); httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/mcp-context-economy`).flush({ id: 'mcp-context-economy', name: 'MCP Context Economy', @@ -509,13 +546,16 @@ describe('GenericNodeComponent', () => { expect(dialog.initial.value).toBe('Prefer write_file over apply_patch when creating a file.'); }); - it('shows a message rather than throwing when the skill cannot be loaded', async () => { + it('shows a message rather than throwing when the definition cannot be loaded', async () => { const component = fixture.componentInstance as any; const httpMock = TestBed.inject(HttpTestingController); const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType; open.mockResolvedValue(null); - const pending = component.viewSkillContent('missing-skill'); + const pending = component.viewItemDefinition({ + url: '/retriever/Skills/definitions', + value: 'missing-skill' + }); httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/missing-skill`) .flush('not found', { status: 404, statusText: 'Not Found' }); await pending; diff --git a/src/app/shared/nodes/generic-node/generic-node.ts b/src/app/shared/nodes/generic-node/generic-node.ts index 58a6df0..a46e7fa 100644 --- a/src/app/shared/nodes/generic-node/generic-node.ts +++ b/src/app/shared/nodes/generic-node/generic-node.ts @@ -81,13 +81,28 @@ type EditableFieldView = SchemaParameterFieldView; type OptionalGroupFieldDefinition = SchemaOptionalGroupFieldDefinition; +/** + * An item property whose chosen value is only an id, with the thing it names readable elsewhere. + * The schema says so and says where (`x-retriever-definition-url`), so a row offers to show it + * without the editor knowing what kind of binding it is looking at. + */ +type ItemDefinitionSource = { + property: string; + url: string; +}; + +/** What one row needs to fetch its own definition: the endpoint, and the value to read there. */ +type ArrayItemDefinition = { + url: string; + value: string; +}; + type ArrayFieldDefinition = { path: string; label: string; itemSchema: Record | null; uniqueBy: string | null; - /** The item property backed by the skills catalog, if any - the one a "view content" button reads. */ - skillIdProperty: string | null; + definitionSource: ItemDefinitionSource | null; ui: { structural: boolean; visibleWhen: UiConditionRule[]; @@ -99,7 +114,7 @@ type ArrayFieldDefinition = { type ArrayFieldItemView = { index: number; summary: string; - skillId: string | null; + definition: ArrayItemDefinition | null; }; type ArrayFieldView = { @@ -928,7 +943,7 @@ export class GenericNodeComponent implements OnDestroy { uniqueBy: typeof childResolved?.['x-ui-unique-by'] === 'string' && String(childResolved['x-ui-unique-by']).trim().length > 0 ? String(childResolved['x-ui-unique-by']).trim() : null, - skillIdProperty: this.findSkillIdProperty(itemSchema, schema ?? {}), + definitionSource: this.findItemDefinitionSource(itemSchema, schema ?? {}), ui: { structural: ui.structural, visibleWhen: ui.visibleWhen, @@ -941,11 +956,15 @@ export class GenericNodeComponent implements OnDestroy { }); } - /** The one item property, if any, that a `@FieldRetriever(name = "Skills")` binds - see `SkillBinding`. */ - private findSkillIdProperty(itemSchema: Record | null, rootSchema: Record): string | null { + /** The first item property, if any, that declares where the value it holds can be read in full. */ + private findItemDefinitionSource( + itemSchema: Record | null, + rootSchema: Record + ): ItemDefinitionSource | null { if (!itemSchema) return null; for (const { key, schema } of orderedSchemaPropertyEntries(itemSchema, rootSchema)) { - if (schema?.['x-retriever-name'] === 'Skills') return key; + const url = toStringOrNull(schema?.['x-retriever-definition-url']); + if (url) return { property: key, url: url.trim() }; } return null; } @@ -1546,23 +1565,28 @@ export class GenericNodeComponent implements OnDestroy { this.maybeCreateBlockOnServer(); } - /** Always available, even read-only: seeing a skill's instructions never needs edit rights. */ - async viewSkillContent(skillId: string, event?: Event) { + /** Always available, even read-only: reading what a chosen value stands for needs no edit rights. */ + async viewItemDefinition(definition: ArrayItemDefinition, event?: Event) { event?.preventDefault(); event?.stopPropagation(); try { - const skill = await firstValueFrom( - this.http.get<{ id: string; name?: string; content?: string }>( - `${environment.apiUrl}/retriever/Skills/definitions/${encodeURIComponent(skillId)}` + const loaded = await firstValueFrom( + this.http.get<{ name?: string; content?: string }>( + `${this.toApiUrl(definition.url)}/${encodeURIComponent(definition.value)}` ) ); - await this.openReadonlyTextDialog(skill.name ?? skillId, skill.content ?? ''); + await this.openReadonlyTextDialog(loaded.name ?? definition.value, loaded.content ?? ''); } catch { - await this.openReadonlyTextDialog(skillId, 'Could not load this skill\'s content.'); + await this.openReadonlyTextDialog(definition.value, 'Could not load this content.'); } } + private toApiUrl(url: string): string { + if (/^https?:\/\//.test(url)) return url; + return `${environment.apiUrl}${url.startsWith('/') ? url : `/${url}`}`; + } + private async openArrayItemEditor(path: string, index: number | null) { const definition = this.arrayFieldDefinitions.find((field) => field.path === path); if (!definition || !this.isPathVisible(path)) return; @@ -1921,12 +1945,16 @@ export class GenericNodeComponent implements OnDestroy { return value.map((item, index) => ({ index, summary: this.toArrayItemSummary(definition, item, index), - skillId: definition.skillIdProperty && item && typeof item === 'object' && !Array.isArray(item) - ? toStringOrNull((item as Record)[definition.skillIdProperty]) - : null + definition: this.toArrayItemDefinition(definition.definitionSource, item) })); } + private toArrayItemDefinition(source: ItemDefinitionSource | null, item: unknown): ArrayItemDefinition | null { + if (!source || !item || typeof item !== 'object' || Array.isArray(item)) return null; + const value = toStringOrNull((item as Record)[source.property]); + return value ? { url: source.url, value } : null; + } + private toArrayItemSummary(definition: ArrayFieldDefinition, item: unknown, index: number) { if (!item || typeof item !== 'object' || Array.isArray(item)) { return `Item ${index + 1}`;