From e1966c7e835371f91b3dded3b98e78ec52f9ee21 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Tue, 28 Apr 2026 23:34:27 +0200 Subject: [PATCH] feat(approve-gate): wire 9 modifying tools through phobiCS-tui MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalizes runApproveGate beyond set_pou_code by adding: - runApproveGateOp({slug, oldText, newText}) — writes synthetic before/after files into a tmpdir, spawns 'phobiCS-tui approve' on them, cleans up the tmpdir on return. Used for ops that don't have a clean existing-file -> proposed-file mapping (create/delete/rename /add). - gateOpForTool({enabled, slug, oldText, newText}) — MCP-tool-shaped wrapper. Returns null when the op should proceed (gate disabled, accepted, or no-existing); returns a {content, isError} block- response otherwise. Lets each tool gate with one if-statement. Wired into the 9 modifying tools (with --approve-edits on, each one prompts via the TUI before applying): - create_pou all-green diff: name + type + language + parent - create_property all-green diff: name + type + parent FB - create_method all-green diff: name + return type + parent FB - create_dut all-green diff: name + DUT type + parent - create_gvl all-green diff: name + parent + (optional decl) - create_folder all-green diff: name + parent - delete_object all-red diff: object + project - rename_object del+add: old name -> new name - add_library all-green diff: library name + project set_pou_code keeps using the existing runApproveGate (which composes real merged file content via the IMPL_SENTINEL split, giving the nicest possible diff against the real mirror file). --- src/approve-gate.ts | 73 +++++++++++++++++++++++++++++++++ src/server.ts | 66 ++++++++++++++++++++++++++++- tests/unit/approve-gate.test.ts | 55 ++++++++++++++++++++++++- 3 files changed, 191 insertions(+), 3 deletions(-) diff --git a/src/approve-gate.ts b/src/approve-gate.ts index 8806501..034395b 100644 --- a/src/approve-gate.ts +++ b/src/approve-gate.ts @@ -1,4 +1,5 @@ import * as fs from 'fs/promises'; +import * as os from 'os'; import * as path from 'path'; import { spawn } from 'child_process'; @@ -93,6 +94,78 @@ export interface RunGateOpts { spawnFn?: typeof spawnApproveTui; } +/** + * Generic approve-gate for tools that don't have a single existing-file -> proposed-file + * mapping. Renders an arbitrary "before" + "after" snapshot through the TUI's approve + * mode, with a descriptive temp-file name conveying the operation. + * + * For pure-create ops, oldText is ''. For pure-delete ops, newText is ''. For rename, + * both sides are short identifier names. The diff display always communicates the op + * intent visually (all-green for create, all-red for delete, single line each for rename). + */ +export interface ApproveGateOp { + /** Short slug used as the synthetic filename — drives what the user sees as ".st". */ + slug: string; + /** Pre-state. Empty string for create operations. */ + oldText: string; + /** Post-state. Empty string for delete operations. */ + newText: string; + /** Test-only override for the spawn function. */ + spawnFn?: typeof spawnApproveTui; +} + +export async function runApproveGateOp(op: ApproveGateOp): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'phobics-gate-')); + const safeSlug = op.slug.replace(/[^A-Za-z0-9._-]+/g, '_'); + const oldPath = path.join(dir, `${safeSlug}.before.st`); + const newPath = path.join(dir, `${safeSlug}.after.st`); + await fs.writeFile(oldPath, op.oldText, 'utf8'); + await fs.writeFile(newPath, op.newText, 'utf8'); + try { + const spawnFn = op.spawnFn ?? spawnApproveTui; + return await spawnFn(oldPath, newPath); + } finally { + await fs.rm(dir, { recursive: true, force: true }); + } +} + +/** + * MCP-tool-shaped wrapper around runApproveGateOp. + * + * Returns null when the operation should proceed (gate disabled, accepted, or + * mirror file missing for a non-set_pou_code op). Returns an MCP-shaped + * response object when the call should short-circuit (rejected -> isError=false + * with the user-rejection message; error -> isError=true with the spawn error). + */ +export interface BlockResponse { + content: Array<{ type: 'text'; text: string }>; + isError: boolean; +} + +export async function gateOpForTool(opts: { + enabled: boolean; + slug: string; + oldText: string; + newText: string; + spawnFn?: typeof spawnApproveTui; +}): Promise { + if (!opts.enabled) return null; + const gate = await runApproveGateOp({ + slug: opts.slug, + oldText: opts.oldText, + newText: opts.newText, + spawnFn: opts.spawnFn, + }); + if (gate.status === 'accepted' || gate.status === 'no-existing') return null; + if (gate.status === 'rejected') { + return { content: [{ type: 'text', text: gate.message }], isError: false }; + } + return { + content: [{ type: 'text', text: `Approve gate error: ${gate.message}` }], + isError: true, + }; +} + export async function runApproveGate(opts: RunGateOpts): Promise { const existingPath = await findMirrorFile(opts.projectFilePath, opts.pouPath); if (!existingPath) { diff --git a/src/server.ts b/src/server.ts index 1df4f1c..329380e 100644 --- a/src/server.ts +++ b/src/server.ts @@ -26,7 +26,7 @@ import { inspectProjectFile } from './inspect'; import { parseProfileName } from './detect'; import { decideOpenProjectPreflight } from './preflight'; import { readSelection } from './state-read'; -import { runApproveGate } from './approve-gate'; +import { runApproveGate, gateOpForTool } from './approve-gate'; /** * Classifier for `bump_project_version --level=auto`. @@ -1046,6 +1046,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-pou-${sanParentPath.replace(/[/\\]/g, '_')}-${args.name}`, + oldText: '', + newText: `(* create POU *)\nname: ${args.name}\ntype: ${args.type}\nlanguage: ${args.language}\nparent: ${sanParentPath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1162,6 +1169,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-property-${sanParentPath.replace(/[/\\]/g, '_')}-${args.propertyName}`, + oldText: '', + newText: `(* create Property *)\nname: ${args.propertyName}\ntype: ${args.propertyType}\nparent FB: ${sanParentPath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1194,6 +1208,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-method-${sanParentPath.replace(/[/\\]/g, '_')}-${args.methodName}`, + oldText: '', + newText: `(* create Method *)\nname: ${args.methodName}\nreturn type: ${args.returnType ?? '(none)'}\nparent FB: ${sanParentPath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1363,6 +1384,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-dut-${sanParentPath.replace(/[/\\]/g, '_')}-${args.name}`, + oldText: '', + newText: `(* create DUT *)\nname: ${args.name}\ntype: ${args.dutType}\nparent: ${sanParentPath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1408,6 +1436,14 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-gvl-${sanParentPath.replace(/[/\\]/g, '_')}-${args.name}`, + oldText: '', + newText: `(* create GVL *)\nname: ${args.name}\nparent: ${sanParentPath}\n` + + (args.declarationCode ? `\nVAR_GLOBAL\n${args.declarationCode}\nEND_VAR\n` : ''), + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1438,6 +1474,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `create-folder-${sanParentPath.replace(/[/\\]/g, '_')}-${args.folderName}`, + oldText: '', + newText: `(* create Folder *)\nname: ${args.folderName}\nparent: ${sanParentPath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1466,6 +1509,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `delete-${sanObjPath.replace(/[/\\]/g, '_')}`, + oldText: `(* DELETE *)\nobject: ${sanObjPath}\nproject: ${args.projectFilePath}\n`, + newText: '', + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1501,6 +1551,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open', 'find_object_by_path'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `rename-${sanObjPath.replace(/[/\\]/g, '_')}`, + oldText: `(* old name *)\n${sanObjPath}\n`, + newText: `(* new name *)\n${sanObjPath.replace(/[^/\\]+$/, args.newName)}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, @@ -1969,6 +2026,13 @@ export async function startMcpServer(config: ServerConfig): Promise { }, ['ensure_project_open'] ); + const blocked = await gateOpForTool({ + enabled: !!config.approveEdits, + slug: `add-library-${args.libraryName.replace(/[^A-Za-z0-9._-]+/g, '_')}`, + oldText: '', + newText: `(* add Library *)\nname: ${args.libraryName}\nproject: ${args.projectFilePath}\n`, + }); + if (blocked) return blocked; const result = await executor.executeScript(script); return await formatModifyingResponse( result, diff --git a/tests/unit/approve-gate.test.ts b/tests/unit/approve-gate.test.ts index dd4d3bc..a08f30a 100644 --- a/tests/unit/approve-gate.test.ts +++ b/tests/unit/approve-gate.test.ts @@ -1,5 +1,6 @@ -import { describe, it, expect } from 'vitest'; -import { composeMergedContent, IMPL_SENTINEL } from '../../src/approve-gate'; +import { describe, it, expect, vi } from 'vitest'; +import * as fs from 'fs/promises'; +import { composeMergedContent, IMPL_SENTINEL, runApproveGateOp } from '../../src/approve-gate'; const existing = [ 'PROGRAM PLC_PRG', @@ -47,3 +48,53 @@ describe('composeMergedContent', () => { expect(out).toContain('x := 1;'); }); }); + +describe('runApproveGateOp', () => { + it('writes before/after files and spawns the TUI with both paths', async () => { + const captured: { existing?: string; staged?: string } = {}; + const spawnFn = vi.fn(async (existingPath: string, stagedPath: string) => { + captured.existing = existingPath; + captured.staged = stagedPath; + const oldText = await fs.readFile(existingPath, 'utf8'); + const newText = await fs.readFile(stagedPath, 'utf8'); + expect(oldText).toBe('OLD'); + expect(newText).toBe('NEW'); + return { status: 'accepted' as const }; + }); + const result = await runApproveGateOp({ + slug: 'create-Application/MyFB', + oldText: 'OLD', + newText: 'NEW', + spawnFn, + }); + expect(result.status).toBe('accepted'); + expect(spawnFn).toHaveBeenCalledOnce(); + }); + + it('passes the rejected status through', async () => { + const spawnFn = vi.fn(async () => ({ + status: 'rejected' as const, + message: 'nope', + })); + const result = await runApproveGateOp({ + slug: 'delete-Foo', + oldText: 'something', + newText: '', + spawnFn, + }); + expect(result.status).toBe('rejected'); + }); + + it('cleans up the temp dir after the spawn returns', async () => { + let dirSeen: string | undefined; + const spawnFn = vi.fn(async (existingPath: string) => { + dirSeen = existingPath.replace(/[\\/][^\\/]+$/, ''); + // dir should exist while we're in here + await fs.access(dirSeen); + return { status: 'accepted' as const }; + }); + await runApproveGateOp({ slug: 'x', oldText: '', newText: 'Y', spawnFn }); + // After: the dir must be gone + await expect(fs.access(dirSeen!)).rejects.toThrow(); + }); +});