From e61ed99139f77dc6a5d4fa7cc637584d0c3628c6 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Thu, 22 Oct 2026 22:28:47 +0200 Subject: [PATCH 1/2] feat(toolkit): the kind of an edge, when connect can make more than one A workflow connection is one thing, so `connect` never said what kind of edge it made. A specification's clauses are joined by typed edges (refines, verifies, depends-on), and a sidecar for them has to be told which one a drawn connection is. The sidecar lists the kinds in its capabilities (`diagnostic.edgeRoles`, `[{ id, label, description? }]`, parsed onto `SidecarCapabilities`). The create-edge handler asks which with a quick pick and sends it as `args.role`: a role given on the operation, or the only one listed, is taken without asking; a headless caller gets no pick and the sidecar's default applies; a dismissed pick makes no connection. The delete handler sends the edge's `cal:edgeRole` as `args.role`, so the right one of several edges between two nodes goes. A sidecar that lists no roles is treated exactly as before. Claude-Session: https://claude.ai/code/session_012yAjXyWw6JyWsXvHG3cG9c --- CHANGELOG.md | 8 + .../server/operations/create-edge-handler.ts | 49 ++++- .../src/server/operations/delete-handler.ts | 11 +- .../src/sidecar-capabilities.ts | 35 +++- .../sidecar-toolkit/test/edge-roles.test.ts | 183 ++++++++++++++++++ 5 files changed, 283 insertions(+), 3 deletions(-) create mode 100644 packages/sidecar-toolkit/test/edge-roles.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 07c90f5..1e71b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ platform API version. ### Added +- The kind of an edge, for a sidecar whose `connect` can make more than one. + A sidecar lists the kinds in its capabilities (`diagnostic.edgeRoles`, + `[{ id, label, description? }]`); the create-edge handler then asks which + (a role given on the operation, or the only one listed, is taken without + asking; a headless caller gets no pick) and sends it as `args.role`, and the + delete handler sends the edge's `cal:edgeRole` as `args.role` so the right + one of several edges between two nodes goes. A sidecar that lists none is + treated as before. - `clientBehavior.edgeCaptions`: `false` draws an edge as its line alone, with no carried label, queue badge, last-token button or index label on it; the tooltip on hover still says what it carries. Default `true`, as before. diff --git a/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts b/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts index 02f8a5a..e735452 100644 --- a/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts @@ -6,6 +6,7 @@ import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; import { SidecarInvoker } from './sidecar-invoker'; +import { parseCapabilitiesDiagnostic, type SidecarEdgeRole } from '../../sidecar-capabilities'; /** * Port reference resolved from GModel args (Python path). @@ -80,9 +81,13 @@ export class CreateEdgeOperationHandler extends OperationHandler { ? { type: 'workflow', name: targetRef.portName } : { type: 'actor', actor: targetRef.entityName, port: targetRef.portName }; + const role = await this.pickEdgeRole(sourceUri, operation); + if (role === null) { + return undefined; + } const ok = await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('connect'), - args: { workflow: workflowName, from_port: fromPort, to_port: toPort } + args: { workflow: workflowName, from_port: fromPort, to_port: toPort, ...(role ? { role } : {}) } }); if (!ok) { return undefined; @@ -92,6 +97,48 @@ export class CreateEdgeOperationHandler extends OperationHandler { }); } + /** + * The kind of edge to make, when the sidecar's `connect` can make more + * than one (`getCapabilities.edgeRoles`). + * + * A role given on the operation wins. Otherwise the sidecar is asked what + * it offers: nothing, and `connect` goes without a role, as it always + * has; one, and that is it; several, and the person picks (`undefined` + * when the pick is dismissed, which is the `null` here: no connection). + * A headless caller, an agent with no one to ask, gets no pick either: + * it names the role itself, or the sidecar's default applies. + */ + protected async pickEdgeRole(sourceUri: string, operation: CreateEdgeOperation): Promise { + const given = typeof operation.args?.['role'] === 'string' ? String(operation.args['role']).trim() : ''; + if (given !== '') { + return given; + } + if (operation.args?.['headless'] === true) { + return undefined; + } + const roles = await this.edgeRoles(sourceUri); + if (!roles || roles.length === 0) { + return undefined; + } + if (roles.length === 1) { + return roles[0].id; + } + const picked = await vscode.window.showQuickPick( + roles.map(role => ({ label: role.id, description: role.label, detail: role.description, id: role.id })), + { placeHolder: 'Which kind of edge?', matchOnDescription: true } + ); + return picked ? picked.id : null; + } + + /** The edge kinds the sidecar offers, from its capabilities; nothing when it says nothing. */ + protected async edgeRoles(sourceUri: string): Promise { + const result = await this.sidecar.sendSidecarCapabilitiesDetailed(sourceUri); + if (!result.ok || !result.response) { + return undefined; + } + return parseCapabilitiesDiagnostic(result.response.diagnostic, '')?.edgeRoles; + } + /** * Resolve a port reference for Python diagram ports. * diff --git a/packages/sidecar-toolkit/src/server/operations/delete-handler.ts b/packages/sidecar-toolkit/src/server/operations/delete-handler.ts index 021c628..13f8518 100644 --- a/packages/sidecar-toolkit/src/server/operations/delete-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/delete-handler.ts @@ -79,9 +79,18 @@ export class DeleteElementOperationHandler extends OperationHandler { if (!outPort || !inPort) continue; const fromExpr = this.edgeEndpointExpr(fromName, outPort); const toExpr = this.edgeEndpointExpr(toName, inPort); + // The kind of edge, when the producer said (`meta.role`): + // two nodes may be joined by edges of several kinds, and + // the sidecar then needs to know which one goes. + const role = elementArgs[WorkflowDiagramMetadata.EDGE_ROLE]; const ok = await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('deleteEdge'), - args: { workflow: workflowName, from_expr: fromExpr, to_expr: toExpr } + args: { + workflow: workflowName, + from_expr: fromExpr, + to_expr: toExpr, + ...(typeof role === 'string' && role.trim() !== '' ? { role: role.trim() } : {}) + } }); if (!ok) return undefined; continue; diff --git a/packages/sidecar-toolkit/src/sidecar-capabilities.ts b/packages/sidecar-toolkit/src/sidecar-capabilities.ts index 6064053..725c2d1 100644 --- a/packages/sidecar-toolkit/src/sidecar-capabilities.ts +++ b/packages/sidecar-toolkit/src/sidecar-capabilities.ts @@ -20,6 +20,38 @@ export interface SidecarCapabilities { supportedOps: Set; /** Declared feature flags (sourceTruthEdits, stableIds, checkConnection, …). */ features: Record; + /** + * The kinds of edge the sidecar's `connect` can make, when a connection is + * more than one thing (`diagnostic.edgeRoles`, `[{ id, label, description? }]`). + * Absent or empty: a connection is one thing, and `connect` is sent without + * a role. Present: the create-edge handler asks which, and sends `args.role`. + */ + edgeRoles?: SidecarEdgeRole[]; +} + +/** One kind of edge a sidecar's `connect` can make. */ +export interface SidecarEdgeRole { + id: string; + label: string; + description?: string; +} + +/** `diagnostic.edgeRoles` as the sidecar wrote it, else nothing: an entry + * without a non-empty string id is dropped, a missing label is the id. */ +export function parseEdgeRoles(raw: unknown): SidecarEdgeRole[] | undefined { + if (!Array.isArray(raw)) return undefined; + const out: SidecarEdgeRole[] = []; + for (const item of raw) { + if (!item || typeof item !== 'object') continue; + const { id, label, description } = item as Record; + if (typeof id !== 'string' || id.trim() === '') continue; + out.push({ + id: id.trim(), + label: typeof label === 'string' && label.trim() !== '' ? label.trim() : id.trim(), + ...(typeof description === 'string' && description.trim() !== '' ? { description: description.trim() } : {}), + }); + } + return out.length > 0 ? out : undefined; } interface CapabilityQueryOptions { @@ -68,7 +100,8 @@ export function parseCapabilitiesDiagnostic(diagnostic: any, prefix: string): Si : typeof nested?.sidecarProtocolVersion === 'number' ? nested.sidecarProtocolVersion : 1; - return { protocolVersion, supportedOps, features }; + const edgeRoles = parseEdgeRoles(diagnostic.edgeRoles); + return { protocolVersion, supportedOps, features, ...(edgeRoles ? { edgeRoles } : {}) }; } /** diff --git a/packages/sidecar-toolkit/test/edge-roles.test.ts b/packages/sidecar-toolkit/test/edge-roles.test.ts new file mode 100644 index 0000000..28ec7d2 --- /dev/null +++ b/packages/sidecar-toolkit/test/edge-roles.test.ts @@ -0,0 +1,183 @@ +/** + * The kind of an edge, when a sidecar's `connect` can make more than one. + * + * A workflow connection is one thing, so `connect` never said what kind of + * edge it made. A specification's clauses are joined by typed edges (refines, + * verifies, depends-on), and a sidecar for them lists the kinds in its + * capabilities (`edgeRoles`); the create-edge handler then asks which, and + * sends `args.role`. A sidecar that lists none is treated as before. + */ +import { describe, expect, it } from 'vitest'; +import * as vscode from 'vscode'; +import { WorkflowDiagramMetadata } from '@dialogram/shared'; + +import { parseCapabilitiesDiagnostic, parseEdgeRoles } from '../src/sidecar-capabilities.js'; +import { CreateEdgeOperationHandler } from '../src/server/operations/create-edge-handler.js'; + +const ROLES = [ + { id: 'refines', label: 'Refines: this clause narrows the target' }, + { id: 'verifies', label: 'Verifies: this clause checks the target' }, + { id: 'depends-on', label: 'Depends on', description: 'this clause needs the target' } +]; + +describe('edge roles in the capabilities', () => { + it('reads edgeRoles, dropping an entry without an id and defaulting a missing label to the id', () => { + expect(parseEdgeRoles([...ROLES, { label: 'no id' }, { id: ' bare ' }, 7])).toEqual([ + ...ROLES, + { id: 'bare', label: 'bare' } + ]); + expect(parseEdgeRoles(undefined)).toBeUndefined(); + expect(parseEdgeRoles([])).toBeUndefined(); + }); + + it('carries them on the parsed capabilities, and leaves them out when the sidecar lists none', () => { + const withRoles = parseCapabilitiesDiagnostic( + { protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: ROLES }, + 'ndf' + ); + expect(withRoles?.edgeRoles).toEqual(ROLES); + const without = parseCapabilitiesDiagnostic({ protocolVersion: 2, supportedOps: ['connect'], features: {} }, 'wfpy'); + expect(without).toBeDefined(); + expect(without?.edgeRoles).toBeUndefined(); + }); +}); + +/** A model with two clause nodes, each with one pin, as the NDF diagram draws them. */ +function modelState() { + const node = (name: string) => ({ + id: name, + type: 'node:actor', + args: { [WorkflowDiagramMetadata.ENTITY_NAME]: name } + }); + const port = (id: string, name: string, parent: unknown) => ({ + id, + type: 'port:in', + parent, + args: { [WorkflowDiagramMetadata.PORT_NAME]: name } + }); + const a = node('Frame admission'); + const b = node('Forwarding intent'); + const elements = new Map([ + ['a_out', port('a_out', 'out', a)], + ['b_in', port('b_in', 'in', b)] + ]); + return { + root: { args: { sourceUri: 'file:///spec/docs/forwarding.md', [WorkflowDiagramMetadata.WORKFLOW_NAME]: 'DOC-FORWARDING' } }, + index: { get: (id: string) => elements.get(id) } + }; +} + +function handlerWith(capabilities: unknown, sent: Array<{ op: string; args: Record }>) { + const handler = new CreateEdgeOperationHandler(); + (handler as any).modelState = modelState(); + (handler as any).sidecar = { + sidecarOp: (op: string) => `ndf.${op}`, + undoLabelSuffix: () => ' (ndf)', + sendSidecarCapabilitiesDetailed: async () => ({ ok: true, response: { status: 'ok', diagnostic: capabilities } }) + }; + (handler as any).sendSidecarOp = async (_uri: string, payload: { op: string; args: Record }) => { + sent.push(payload); + return true; + }; + return handler; +} + +async function connect(handler: CreateEdgeOperationHandler, args?: Record) { + const command = handler.createCommand({ + kind: 'createEdge', + elementTypeId: 'edge:connection', + sourceElementId: 'a_out', + targetElementId: 'b_in', + ...(args ? { args } : {}) + } as any); + expect(command).toBeTruthy(); + // The edits are computed when the command runs; that is where the sidecar is asked. + return (command as any).options?.computeEdits?.() ?? (command as any).computeEdits?.(); +} + +describe('the create-edge handler and the kind of edge', () => { + it('sends connect without a role when the sidecar lists no roles, as before', async () => { + const sent: Array<{ op: string; args: Record }> = []; + const windowAny = vscode.window as any; + const original = windowAny.showQuickPick; + let asked = false; + windowAny.showQuickPick = async () => { + asked = true; + return undefined; + }; + try { + await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {} }, sent)); + } finally { + windowAny.showQuickPick = original; + } + expect(asked).toBe(false); + expect(sent).toEqual([{ + op: 'ndf.connect', + args: { + workflow: 'DOC-FORWARDING', + from_port: { type: 'actor', actor: 'Frame admission', port: 'out' }, + to_port: { type: 'actor', actor: 'Forwarding intent', port: 'in' } + } + }]); + }); + + it('asks which kind when the sidecar lists several, and sends the pick as args.role', async () => { + const sent: Array<{ op: string; args: Record }> = []; + const windowAny = vscode.window as any; + const original = windowAny.showQuickPick; + let offered: Array<{ label: string; description?: string }> = []; + windowAny.showQuickPick = async (items: Array<{ label: string; description?: string; id: string }>) => { + offered = items; + return items.find(item => item.id === 'verifies'); + }; + try { + await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: ROLES }, sent)); + } finally { + windowAny.showQuickPick = original; + } + expect(offered.map(item => item.label)).toEqual(['refines', 'verifies', 'depends-on']); + expect(offered[0].description).toBe('Refines: this clause narrows the target'); + expect(sent).toHaveLength(1); + expect(sent[0].args.role).toBe('verifies'); + }); + + it('makes no connection when the pick is dismissed', async () => { + const sent: Array<{ op: string; args: Record }> = []; + const windowAny = vscode.window as any; + const original = windowAny.showQuickPick; + windowAny.showQuickPick = async () => undefined; + try { + const edits = await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: ROLES }, sent)); + expect(edits).toBeUndefined(); + } finally { + windowAny.showQuickPick = original; + } + expect(sent).toEqual([]); + }); + + it('takes a role given on the operation, and the only role offered, without asking', async () => { + const windowAny = vscode.window as any; + const original = windowAny.showQuickPick; + let asked = 0; + windowAny.showQuickPick = async () => { + asked += 1; + return undefined; + }; + try { + const given: Array<{ op: string; args: Record }> = []; + await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: ROLES }, given), { role: 'depends-on' }); + expect(given[0].args.role).toBe('depends-on'); + + const only: Array<{ op: string; args: Record }> = []; + await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: [ROLES[0]] }, only)); + expect(only[0].args.role).toBe('refines'); + + const headless: Array<{ op: string; args: Record }> = []; + await connect(handlerWith({ protocolVersion: 2, supportedOps: ['connect'], features: {}, edgeRoles: ROLES }, headless), { headless: true }); + expect(headless[0].args.role).toBeUndefined(); + } finally { + windowAny.showQuickPick = original; + } + expect(asked).toBe(0); + }); +}); From 28cce2ffdb4b8449695d3b5064d0a94ac3afde9a Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Fri, 23 Oct 2026 18:42:10 +0200 Subject: [PATCH 2/2] fix: ask a sidecar's edge kinds once per session; an agent's edges are headless Review of #96: - Every connect asked the sidecar for its capabilities, a process of its own, before the connect itself: two spawns per edge for every consumer, 2N for an agent's create-edges of N. `SidecarInvoker.edgeRoles` now asks once per sidecar command for the session (a failed ask is not kept), and both the create-edge and the delete handler use it. - The create-edges MCP tool passed the agent's args through without `headless`, so with a sidecar listing several kinds an agent's call opened a quick pick on the person's screen, and a dismissed pick made no edge while the tool reported success. The tool now marks each edge headless, and its description tells the agent to name the kind in `args.role` (else the sidecar's default kind is made). - Delete sent the edge's role even to a sidecar that lists no kinds; it now sends one only when the sidecar does. Tests: edge-roles.test.ts gains the once-per-session ask, the retry after a failed ask, the headless connect and the delete handler with and without listed kinds; mcp-diagram-module.test.ts expects the headless marker. Claude-Session: https://claude.ai/code/session_01E8WfqKknDNN6xpNVcJL5K8 --- CHANGELOG.md | 11 +- .../server/create-edges-mcp-tool-handler.ts | 8 +- .../test/mcp-diagram-module.test.ts | 5 +- .../server/operations/create-edge-handler.ts | 10 +- .../src/server/operations/delete-handler.ts | 12 ++- .../src/server/operations/sidecar-invoker.ts | 33 ++++++ .../sidecar-toolkit/test/edge-roles.test.ts | 101 ++++++++++++++++-- 7 files changed, 155 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e71b86..d8501f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,13 @@ platform API version. A sidecar lists the kinds in its capabilities (`diagnostic.edgeRoles`, `[{ id, label, description? }]`); the create-edge handler then asks which (a role given on the operation, or the only one listed, is taken without - asking; a headless caller gets no pick) and sends it as `args.role`, and the - delete handler sends the edge's `cal:edgeRole` as `args.role` so the right - one of several edges between two nodes goes. A sidecar that lists none is - treated as before. + asking) and sends it as `args.role`, and the delete handler sends the + edge's `cal:edgeRole` as `args.role` so the right one of several edges + between two nodes goes. The capabilities are asked once per sidecar command + for the session, not on every connect. An agent's `create-edges` call is + headless: it may name the kind in `args.role`, and is never asked; without + one the sidecar's default kind is made. A sidecar that lists none is + treated as before, on connect and on delete. - `clientBehavior.edgeCaptions`: `false` draws an edge as its line alone, with no carried label, queue badge, last-token button or index label on it; the tooltip on hover still says what it carries. Default `true`, as before. diff --git a/packages/diagram-server/src/server/create-edges-mcp-tool-handler.ts b/packages/diagram-server/src/server/create-edges-mcp-tool-handler.ts index 194c3e9..c37673b 100644 --- a/packages/diagram-server/src/server/create-edges-mcp-tool-handler.ts +++ b/packages/diagram-server/src/server/create-edges-mcp-tool-handler.ts @@ -61,7 +61,9 @@ export class AgentDispatchCreateEdgesMcpToolHandler extends CreateEdgesMcpToolHa "name via `args.source` / `args.target` as \"nodeName.portName\" (e.g. args.source " + '"producer.out", args.target "consumer.in") — the name form is resolved server-side and needs ' + 'no query-elements/diagram-model dump. After create-nodes, the confirmation lists each new ' + - "node's port names, so the very next create-edges can use nodeName.portName directly. Set " + + "node's port names, so the very next create-edges can use nodeName.portName directly. When " + + "the diagram's edges come in kinds, name the kind in `args.role`; without it the diagram's " + + 'default kind is made (never a question to the person). Set ' + '`dryRun: true` to validate without creating. Use `element-types` for valid edge type IDs. ' + 'This modifies the source and is undoable via the editor.'; @@ -84,12 +86,14 @@ export class AgentDispatchCreateEdgesMcpToolHandler extends CreateEdgesMcpToolHa if (source.ok && target.ok) { // Drop the name specs from args (they are addressing, not model metadata) and fill // the concrete ids the stock handler dispatches. + // An agent's edge is made headless: no one is at the screen to + // answer a question in the middle of its tool call. const { source: _s, target: _t, ...restArgs } = (edge.args ?? {}) as Record; resolvedEdges.push({ ...edge, sourceElementId: source.elementId, targetElementId: target.elementId, - args: Object.keys(restArgs).length > 0 ? restArgs : undefined + args: { ...restArgs, headless: true } }); } } diff --git a/packages/diagram-server/test/mcp-diagram-module.test.ts b/packages/diagram-server/test/mcp-diagram-module.test.ts index c003542..caa43a8 100644 --- a/packages/diagram-server/test/mcp-diagram-module.test.ts +++ b/packages/diagram-server/test/mcp-diagram-module.test.ts @@ -373,8 +373,9 @@ describe('AgentDispatchCreateEdgesMcpToolHandler (name-addressable endpoints)', const fwd = cap.forwarded(); expect(fwd.edges[0].sourceElementId).toBe('producer.out'); expect(fwd.edges[0].targetElementId).toBe('consumer.in'); - // The name specs are addressing, not model metadata — stripped from args. - expect(fwd.edges[0].args).toBeUndefined(); + // The name specs are addressing, not model metadata — stripped from args; an agent's + // edge is made headless, so no question reaches the person mid tool call. + expect(fwd.edges[0].args).toEqual({ headless: true }); } finally { cap.restore(); } diff --git a/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts b/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts index e735452..058ec3c 100644 --- a/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/create-edge-handler.ts @@ -6,7 +6,7 @@ import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared import { ReversibleWorkspaceEditCommand } from '@dialogram/diagram-server/operations/reversible-workspace-edit-command'; import { SidecarInvoker } from './sidecar-invoker'; -import { parseCapabilitiesDiagnostic, type SidecarEdgeRole } from '../../sidecar-capabilities'; +import { type SidecarEdgeRole } from '../../sidecar-capabilities'; /** * Port reference resolved from GModel args (Python path). @@ -130,13 +130,9 @@ export class CreateEdgeOperationHandler extends OperationHandler { return picked ? picked.id : null; } - /** The edge kinds the sidecar offers, from its capabilities; nothing when it says nothing. */ + /** The edge kinds the sidecar offers (asked once per session); nothing when it says nothing. */ protected async edgeRoles(sourceUri: string): Promise { - const result = await this.sidecar.sendSidecarCapabilitiesDetailed(sourceUri); - if (!result.ok || !result.response) { - return undefined; - } - return parseCapabilitiesDiagnostic(result.response.diagnostic, '')?.edgeRoles; + return this.sidecar.edgeRoles(sourceUri); } /** diff --git a/packages/sidecar-toolkit/src/server/operations/delete-handler.ts b/packages/sidecar-toolkit/src/server/operations/delete-handler.ts index 13f8518..85dd384 100644 --- a/packages/sidecar-toolkit/src/server/operations/delete-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/delete-handler.ts @@ -79,10 +79,14 @@ export class DeleteElementOperationHandler extends OperationHandler { if (!outPort || !inPort) continue; const fromExpr = this.edgeEndpointExpr(fromName, outPort); const toExpr = this.edgeEndpointExpr(toName, inPort); - // The kind of edge, when the producer said (`meta.role`): - // two nodes may be joined by edges of several kinds, and - // the sidecar then needs to know which one goes. - const role = elementArgs[WorkflowDiagramMetadata.EDGE_ROLE]; + // The kind of edge, when the producer said (`meta.role`) + // and the sidecar makes edges of several kinds: two nodes + // may then be joined by more than one, and the sidecar + // needs to know which one goes. A sidecar that lists no + // kinds gets no role, as before. + const role = (await this.sidecar.edgeRoles(sourceUri)) + ? elementArgs[WorkflowDiagramMetadata.EDGE_ROLE] + : undefined; const ok = await this.sendSidecarOp(sourceUri, { op: this.sidecar.sidecarOp('deleteEdge'), args: { diff --git a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts index 3d9b1af..52ba49a 100644 --- a/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts +++ b/packages/sidecar-toolkit/src/server/operations/sidecar-invoker.ts @@ -3,6 +3,7 @@ import * as vscode from 'vscode'; import { URI } from 'vscode-uri'; import { SidecarRuntimeService, type CreateNodeStrings, type CreateNodeBehavior, CreateNodeVariant } from '../sidecar-runtime-config'; import { describeChildFailure, runChildProcess } from '../../run-child-process'; +import { parseCapabilitiesDiagnostic, type SidecarEdgeRole } from '../../sidecar-capabilities'; export interface SidecarInvocationResult { ok: boolean; @@ -14,6 +15,9 @@ export interface SidecarInvocationResult { @injectable() export class SidecarInvoker { + /** The edge kinds each sidecar command offers, asked once per session. */ + private readonly edgeRolesByCommand = new Map>(); + constructor( @inject(SidecarRuntimeService) private readonly runtimeProfile: SidecarRuntimeService @@ -78,6 +82,35 @@ export class SidecarInvoker { }); } + /** + * The kinds of edge the sidecar's `connect` can make + * (`getCapabilities.edgeRoles`); nothing when it lists none. Asked once per + * sidecar command for the session, not on every connect: each ask is a + * process of its own, and what a sidecar offers does not change while it + * is running. A failed ask is not kept, so the next one tries again. + */ + async edgeRoles(sourceUri: string): Promise { + let key: string; + try { + key = this.sidecarCommand(vscode.Uri.parse(sourceUri)); + } catch { + key = sourceUri; + } + let pending = this.edgeRolesByCommand.get(key); + if (!pending) { + pending = this.sendSidecarCapabilitiesDetailed(sourceUri).then(result => { + if (!result.ok || !result.response) { + this.edgeRolesByCommand.delete(key); + return undefined; + } + return parseCapabilitiesDiagnostic(result.response.diagnostic, '')?.edgeRoles ?? []; + }); + this.edgeRolesByCommand.set(key, pending); + } + const roles = await pending; + return roles && roles.length > 0 ? roles : undefined; + } + async sendSidecarOp(sourceUri: string, payload: { op: string; args: Record }): Promise { const result = await this.invokeSidecar(sourceUri, payload); return result.ok; diff --git a/packages/sidecar-toolkit/test/edge-roles.test.ts b/packages/sidecar-toolkit/test/edge-roles.test.ts index 28ec7d2..38d8f6d 100644 --- a/packages/sidecar-toolkit/test/edge-roles.test.ts +++ b/packages/sidecar-toolkit/test/edge-roles.test.ts @@ -9,10 +9,16 @@ */ import { describe, expect, it } from 'vitest'; import * as vscode from 'vscode'; -import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { parseCapabilitiesDiagnostic, parseEdgeRoles } from '../src/sidecar-capabilities.js'; import { CreateEdgeOperationHandler } from '../src/server/operations/create-edge-handler.js'; +import { SidecarInvoker } from '../src/server/operations/sidecar-invoker'; +import { DeleteElementOperationHandler } from '../src/server/operations/delete-handler.js'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; const ROLES = [ { id: 'refines', label: 'Refines: this clause narrows the target' }, @@ -67,14 +73,27 @@ function modelState() { }; } -function handlerWith(capabilities: unknown, sent: Array<{ op: string; args: Record }>) { - const handler = new CreateEdgeOperationHandler(); - (handler as any).modelState = modelState(); - (handler as any).sidecar = { +/** A sidecar invoker whose capabilities answer is `capabilities`, counting the asks. */ +function invokerWith(capabilities: unknown, asks = { count: 0 }, answers: Array = []): any { + const invoker: any = { + edgeRolesByCommand: new Map(), sidecarOp: (op: string) => `ndf.${op}`, undoLabelSuffix: () => ' (ndf)', - sendSidecarCapabilitiesDetailed: async () => ({ ok: true, response: { status: 'ok', diagnostic: capabilities } }) + sidecarCommand: () => 'ndf-sidecar', + sendSidecarCapabilitiesDetailed: async () => { + asks.count++; + const ok = answers.length > 0 ? answers.shift()! : true; + return ok ? { ok: true, response: { status: 'ok', diagnostic: capabilities } } : { ok: false, message: 'no answer' }; + } }; + invoker.edgeRoles = SidecarInvoker.prototype.edgeRoles.bind(invoker); + return invoker; +} + +function handlerWith(capabilities: unknown, sent: Array<{ op: string; args: Record }>, invoker = invokerWith(capabilities)) { + const handler = new CreateEdgeOperationHandler(); + (handler as any).modelState = modelState(); + (handler as any).sidecar = invoker; (handler as any).sendSidecarOp = async (_uri: string, payload: { op: string; args: Record }) => { sent.push(payload); return true; @@ -180,4 +199,74 @@ describe('the create-edge handler and the kind of edge', () => { } expect(asked).toBe(0); }); + it('asks the sidecar what it offers once per session, not on every connect', async () => { + const asks = { count: 0 }; + const capabilities = { supportedOps: ['connect'], edgeRoles: [{ id: 'refines' }] }; + const invoker = invokerWith(capabilities, asks); + const sent: Array<{ op: string; args: Record }> = []; + await connect(handlerWith(capabilities, sent, invoker)); + await connect(handlerWith(capabilities, sent, invoker)); + expect(sent.map(payload => payload.args.role)).toEqual(['refines', 'refines']); + expect(asks.count).toBe(1); + }); + + it('asks again after an ask that failed', async () => { + const asks = { count: 0 }; + const invoker = invokerWith({ supportedOps: ['connect'], edgeRoles: [{ id: 'refines' }] }, asks, [false, true]); + expect(await invoker.edgeRoles('file:///spec/a.md')).toBeUndefined(); + expect((await invoker.edgeRoles('file:///spec/a.md'))?.map((r: any) => r.id)).toEqual(['refines']); + expect(asks.count).toBe(2); + }); + + it('never asks the person when an agent makes the edge (headless)', async () => { + const sent: Array<{ op: string; args: Record }> = []; + const capabilities = { supportedOps: ['connect'], edgeRoles: [{ id: 'refines' }, { id: 'verifies' }] }; + await connect(handlerWith(capabilities, sent), { headless: true }); + expect(sent).toHaveLength(1); + expect(sent[0].args.role).toBeUndefined(); + }); +}); + +describe('the delete handler and the kind of edge', () => { + async function deleteEdge(capabilities: unknown): Promise }>> { + const file = path.join(mkdtempSync(path.join(tmpdir(), 'edge-roles-')), 'doc.md'); + writeFileSync(file, '# doc\n'); + const edge = { + id: 'e1', + type: WorkflowDiagramTypes.EDGE_CONNECTION, + args: { 'wf:from': 'A', 'wf:to': 'B', 'wf:outPort': 'out', 'wf:inPort': 'in', [WorkflowDiagramMetadata.EDGE_ROLE]: 'refines' } + }; + const sent: Array<{ op: string; args: Record }> = []; + const handler = new DeleteElementOperationHandler() as any; + handler.modelState = { + root: { args: { sourceUri: pathToFileURL(file).href, 'wf:workflowName': 'DOC' } }, + index: { get: (id: string) => (id === 'e1' ? edge : undefined) } + }; + handler.sidecar = invokerWith(capabilities); + handler.sidecar.sendSidecarOp = async (_uri: string, payload: { op: string; args: Record }) => { + sent.push(payload); + return true; + }; + const workspaceAny = vscode.workspace as any; + const original = workspaceAny.openTextDocument; + workspaceAny.openTextDocument = async () => ({ getText: () => '# doc\n' }); + try { + const command = handler.createCommand({ kind: 'deleteElement', elementIds: ['e1'] }); + await ((command as any).options?.computeEdits?.() ?? (command as any).computeEdits?.()); + } finally { + workspaceAny.openTextDocument = original; + } + return sent; + } + + it("sends the edge's role when the sidecar makes edges of several kinds", async () => { + const sent = await deleteEdge({ supportedOps: ['deleteEdge'], edgeRoles: [{ id: 'refines' }, { id: 'verifies' }] }); + expect(sent.map(payload => payload.args.role)).toEqual(['refines']); + }); + + it('sends no role to a sidecar that lists none, as before', async () => { + const sent = await deleteEdge({ supportedOps: ['deleteEdge'] }); + expect(sent).toHaveLength(1); + expect(sent[0].args.role).toBeUndefined(); + }); });