From d09ec47be686c867f3273916f9b0de1230e9a4c5 Mon Sep 17 00:00:00 2001
From: Lucio Lelii <lucio.lelii@isti.cnr.it>
Date: Sun, 20 Sep 2026 11:29:49 +0200
Subject: [PATCH] Let a skill binding's content be viewed from the node editor

Skills were selectable only by bare id, with no way to see the
instructions a SKILL.md actually carries. Adds a "view content" button
next to each skill row, backed by the already-existing
/retriever/Skills/definitions/{id} endpoint and the previewOnly dialog
mode, so no backend change or new dialog infrastructure was needed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .../nodes/generic-node/generic-node.html      | 10 +++
 .../nodes/generic-node/generic-node.spec.ts   | 90 +++++++++++++++++++
 .../shared/nodes/generic-node/generic-node.ts | 41 ++++++++-
 3 files changed, 139 insertions(+), 2 deletions(-)

diff --git a/src/app/shared/nodes/generic-node/generic-node.html b/src/app/shared/nodes/generic-node/generic-node.html
index 7bb5905..7cf540c 100644
--- a/src/app/shared/nodes/generic-node/generic-node.html
+++ b/src/app/shared/nodes/generic-node/generic-node.html
@@ -392,6 +392,16 @@
               <div class="llm-array-item">
                 <span class="llm-array-item-summary">{{ entry.summary }}</span>
                 <div class="llm-array-item-actions">
+                  @if (entry.skillId; as skillId) {
+                  <button
+                    type="button"
+                    class="llm-edit-btn"
+                    title="View skill content"
+                    (pointerdown)="$event.stopPropagation()"
+                    (click)="viewSkillContent(skillId, $event)">
+                    <i class="bi bi-eye"></i>
+                  </button>
+                  }
                   @if (!isReadonly) {
                   <button
                     type="button"
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 ae9db4e..706087b 100644
--- a/src/app/shared/nodes/generic-node/generic-node.spec.ts
+++ b/src/app/shared/nodes/generic-node/generic-node.spec.ts
@@ -2,7 +2,10 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 // Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
 
+import { provideHttpClient } from '@angular/common/http';
+import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { environment } from '@environment';
 import { DEFAULT_NODE_CAPABILITIES } from '@models/flow';
 import { BlocksService } from '@services/blocks/blocks';
 import { NodeSettingsDialogService } from '@services/dialogs/node-settings-dialog';
@@ -22,6 +25,8 @@ describe('GenericNodeComponent', () => {
     await TestBed.configureTestingModule({
       imports: [GenericNodeComponent],
       providers: [
+        provideHttpClient(),
+        provideHttpClientTesting(),
         {
           provide: NodeSettingsDialogService,
           useValue: {
@@ -387,6 +392,91 @@ describe('GenericNodeComponent', () => {
     });
   });
 
+  describe('viewing a skill\'s content', () => {
+    /** The shape `SkillBinding` produces: one property carrying the `@FieldRetriever(name = "Skills")`. */
+    const skillItemSchema = {
+      type: 'object',
+      required: ['skillId'],
+      properties: {
+        skillId: { type: 'string', 'x-retriever-name': 'Skills' }
+      }
+    };
+
+    it('finds the item property backed by the skills catalog from the schema alone', () => {
+      const component = fixture.componentInstance as any;
+
+      const definitions = component.buildArrayFieldDefinitions({
+        type: 'object',
+        properties: {
+          skills: { type: 'array', items: skillItemSchema }
+        }
+      });
+
+      expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]);
+    });
+
+    it('leaves skillIdProperty null for an array field with no skills-catalog property', () => {
+      const component = fixture.componentInstance as any;
+
+      const definitions = component.buildArrayFieldDefinitions({
+        type: 'object',
+        properties: {
+          mcpServers: { type: 'array', items: { type: 'object', properties: { serverName: { type: 'string' } } } }
+        }
+      });
+
+      expect(definitions[0].skillIdProperty).toBeNull();
+    });
+
+    it('carries each row\'s skill id into its array item view', () => {
+      const component = fixture.componentInstance as any;
+      const definition = {
+        path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null, skillIdProperty: 'skillId',
+        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' })]);
+    });
+
+    it('fetches the skill definition and opens it read-only', async () => {
+      const component = fixture.componentInstance as any;
+      const httpMock = TestBed.inject(HttpTestingController);
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue(null);
+
+      const pending = component.viewSkillContent('mcp-context-economy');
+      httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/mcp-context-economy`).flush({
+        id: 'mcp-context-economy',
+        name: 'MCP Context Economy',
+        content: 'Prefer write_file over apply_patch when creating a file.'
+      });
+      await pending;
+
+      const dialog = open.mock.calls.at(-1)?.[0];
+      expect(dialog.title).toBe('MCP Context Economy');
+      expect(dialog.previewOnly).toBe(true);
+      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 () => {
+      const component = fixture.componentInstance as any;
+      const httpMock = TestBed.inject(HttpTestingController);
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue(null);
+
+      const pending = component.viewSkillContent('missing-skill');
+      httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/missing-skill`)
+        .flush('not found', { status: 404, statusText: 'Not Found' });
+      await pending;
+
+      const dialog = open.mock.calls.at(-1)?.[0];
+      expect(dialog.title).toBe('missing-skill');
+      expect(dialog.initial.value).toContain('Could not load');
+    });
+  });
+
   describe('a retriever whose values are an incomplete list', () => {
     /** What the editor holds for a retriever-backed scalar, as openParameterEditor builds it. */
     function modelFieldDefinition() {
diff --git a/src/app/shared/nodes/generic-node/generic-node.ts b/src/app/shared/nodes/generic-node/generic-node.ts
index aafdd9c..58a6df0 100644
--- a/src/app/shared/nodes/generic-node/generic-node.ts
+++ b/src/app/shared/nodes/generic-node/generic-node.ts
@@ -3,9 +3,11 @@
 // Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
 
 import { CommonModule } from '@angular/common';
+import { HttpClient } from '@angular/common/http';
 import { ChangeDetectionStrategy, ChangeDetectorRef, Component, effect, ElementRef, HostBinding, HostListener, inject, Input, OnDestroy, viewChild } from '@angular/core';
 import { FormsModule } from '@angular/forms';
 import { MatTooltipModule } from '@angular/material/tooltip';
+import { environment } from '@environment';
 import { BiasAnnotation, BiasAnnotationsDescriptor, BlockType, currentFlowPortValueKind, flowValueKindLabel, FlowBlock, FlowData, FlowGlobalInput, FlowPort, FlowValueKind, FLOW_DEPENDANT_PORT_KEY, FLOW_DEPENDENCY_PORT_KEY, isProbeExecutable, normalizeFlowPortValueKinds } from '@models/flow';
 import { BiasAnnotationsComponent } from '../../bias-annotations/bias-annotations';
 import { NodeFocusModalController } from '../node-focus-modal-controller';
@@ -84,6 +86,8 @@ type ArrayFieldDefinition = {
   label: string;
   itemSchema: Record<string, any> | null;
   uniqueBy: string | null;
+  /** The item property backed by the skills catalog, if any - the one a "view content" button reads. */
+  skillIdProperty: string | null;
   ui: {
     structural: boolean;
     visibleWhen: UiConditionRule[];
@@ -95,6 +99,7 @@ type ArrayFieldDefinition = {
 type ArrayFieldItemView = {
   index: number;
   summary: string;
+  skillId: string | null;
 };
 
 type ArrayFieldView = {
@@ -137,6 +142,7 @@ type RenderedSocketPort = {
 })
 export class GenericNodeComponent implements OnDestroy {
   private settingsDialog = inject(NodeSettingsDialogService);
+  private http = inject(HttpClient);
   private editorState = inject(EditorStateHolder);
   private fieldRetriever = inject(FieldRetriever);
   private blocksService = inject(BlocksService);
@@ -914,13 +920,15 @@ export class GenericNodeComponent implements OnDestroy {
       if (childResolved?.['type'] !== 'array') return null;
       if (key === 'type' || key === 'name' || key.startsWith('__')) return null;
 
+      const itemSchema = this.resolveArrayItemSchema(childResolved, schema ?? {});
       return {
         path,
         label: schemaFieldLabel(path, childResolved),
-        itemSchema: this.resolveArrayItemSchema(childResolved, schema ?? {}),
+        itemSchema,
         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 ?? {}),
         ui: {
           structural: ui.structural,
           visibleWhen: ui.visibleWhen,
@@ -933,6 +941,15 @@ export class GenericNodeComponent implements OnDestroy {
     });
   }
 
+  /** The one item property, if any, that a `@FieldRetriever(name = "Skills")` binds - see `SkillBinding`. */
+  private findSkillIdProperty(itemSchema: Record<string, any> | null, rootSchema: Record<string, any>): string | null {
+    if (!itemSchema) return null;
+    for (const { key, schema } of orderedSchemaPropertyEntries(itemSchema, rootSchema)) {
+      if (schema?.['x-retriever-name'] === 'Skills') return key;
+    }
+    return null;
+  }
+
   /**
    * A group that holds values must not look like an empty one: starting a run with settings you
    * cannot see is exactly what a collapsed control risks.
@@ -1529,6 +1546,23 @@ 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) {
+    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)}`
+        )
+      );
+      await this.openReadonlyTextDialog(skill.name ?? skillId, skill.content ?? '');
+    } catch {
+      await this.openReadonlyTextDialog(skillId, 'Could not load this skill\'s content.');
+    }
+  }
+
   private async openArrayItemEditor(path: string, index: number | null) {
     const definition = this.arrayFieldDefinitions.find((field) => field.path === path);
     if (!definition || !this.isPathVisible(path)) return;
@@ -1886,7 +1920,10 @@ export class GenericNodeComponent implements OnDestroy {
 
     return value.map((item, index) => ({
       index,
-      summary: this.toArrayItemSummary(definition, item, index)
+      summary: this.toArrayItemSummary(definition, item, index),
+      skillId: definition.skillIdProperty && item && typeof item === 'object' && !Array.isArray(item)
+        ? toStringOrNull((item as Record<string, unknown>)[definition.skillIdProperty])
+        : null
     }));
   }