From 9802f12203867aa89b5c29f2c651cc1c67e2879f Mon Sep 17 00:00:00 2001
From: Lucio Lelii <lucio.lelii@isti.cnr.it>
Date: Sun, 20 Sep 2026 16:08:54 +0200
Subject: [PATCH] Show the view-skill button on ungrouped array rows too

The node draws array rows from two near-identical template blocks - one
inside a group fieldset, one for ungrouped fields - and the button was
added to only the first. An LLM node renders its Skills list through the
second, so the button never appeared where it was actually wanted.

The test that missed this only exercised the component class, so it adds
a rendering test covering both blocks, and clears the stored user between
service tests: left behind, it made a sign-in a no-op transition and the
cache-reset tests failed intermittently.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/app/services/flows/flows.spec.ts          |  4 ++
 src/app/services/projects/projects.spec.ts    |  4 ++
 .../nodes/generic-node/generic-node.html      | 10 ++++
 .../nodes/generic-node/generic-node.spec.ts   | 49 +++++++++++++++++++
 4 files changed, 67 insertions(+)

diff --git a/src/app/services/flows/flows.spec.ts b/src/app/services/flows/flows.spec.ts
index fa69bd9..1eb3f27 100644
--- a/src/app/services/flows/flows.spec.ts
+++ b/src/app/services/flows/flows.spec.ts
@@ -53,6 +53,10 @@ describe('FlowsService', () => {
       createNewFlow: vi.fn()
     };
 
+    // See the same line in projects.spec.ts: a user left in storage by an earlier test would make
+    // the sign-in below a no-op transition, and the cache would have no reason to reset.
+    localStorage.removeItem(Authorization.USER_STORAGE_KEY);
+
     TestBed.configureTestingModule({});
     service = TestBed.inject(FlowsService);
     service.flowsCallService = callServiceSpy as unknown as FlowsCallServiceBase;
diff --git a/src/app/services/projects/projects.spec.ts b/src/app/services/projects/projects.spec.ts
index 20f4ac7..9ef62d7 100644
--- a/src/app/services/projects/projects.spec.ts
+++ b/src/app/services/projects/projects.spec.ts
@@ -43,6 +43,10 @@ describe('ProjectsService', () => {
       deleteProject: vi.fn()
     };
 
+    // Authorization restores the signed-in user from storage when it is constructed, and storage
+    // outlives a TestBed. Left over, it makes "bob signs in" a no-op transition in the test below.
+    localStorage.removeItem(Authorization.USER_STORAGE_KEY);
+
     TestBed.configureTestingModule({});
     service = TestBed.inject(ProjectsService);
     service.projectsCallService = callServiceSpy as unknown as ProjectsCallServiceBase;
diff --git a/src/app/shared/nodes/generic-node/generic-node.html b/src/app/shared/nodes/generic-node/generic-node.html
index 7cf540c..dda92fc 100644
--- a/src/app/shared/nodes/generic-node/generic-node.html
+++ b/src/app/shared/nodes/generic-node/generic-node.html
@@ -548,6 +548,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 706087b..ed5b88e 100644
--- a/src/app/shared/nodes/generic-node/generic-node.spec.ts
+++ b/src/app/shared/nodes/generic-node/generic-node.spec.ts
@@ -415,6 +415,23 @@ describe('GenericNodeComponent', () => {
       expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]);
     });
 
+    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.
+      const component = fixture.componentInstance as any;
+
+      const definitions = component.buildArrayFieldDefinitions({
+        type: 'object',
+        definitions: { SkillBinding: skillItemSchema },
+        properties: {
+          skills: { type: 'array', items: { $ref: '#/definitions/SkillBinding' } }
+        }
+      });
+
+      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;
 
@@ -440,6 +457,38 @@ describe('GenericNodeComponent', () => {
       expect(items).toEqual([expect.objectContaining({ index: 0, skillId: 'mcp-context-economy' })]);
     });
 
+    it('offers the button in every place the node renders an array row', () => {
+      // Regression: the node draws array rows from two near-identical template blocks - one for
+      // 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 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.
+      component.refreshParameterFields = () => {};
+      component.schemaReady = true;
+      component.parameterDisplaySections = [
+        {
+          key: 'grouped',
+          group: {
+            key: 'grouped',
+            legend: 'Group',
+            items: [{ path: 'skills', field: null, richContentField: null, arrayField, optionalGroupField: null }]
+          },
+          item: null
+        },
+        {
+          key: 'ungrouped',
+          group: null,
+          item: { path: 'skills', field: null, richContentField: null, arrayField, optionalGroupField: null }
+        }
+      ];
+      component.cdr.detectChanges();
+      const buttons = fixture.nativeElement.querySelectorAll('[title="View skill content"]');
+      expect(buttons.length).toBe(2);
+    });
+
     it('fetches the skill definition and opens it read-only', async () => {
       const component = fixture.componentInstance as any;
       const httpMock = TestBed.inject(HttpTestingController);