feat(approve-gate): wire 9 modifying tools through phobiCS-tui
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).
This commit is contained in:
parent
3da09f8aba
commit
e1966c7e83
3 changed files with 191 additions and 3 deletions
|
|
@ -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 "<file>.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<GateResult> {
|
||||
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<BlockResponse | null> {
|
||||
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<GateResult> {
|
||||
const existingPath = await findMirrorFile(opts.projectFilePath, opts.pouPath);
|
||||
if (!existingPath) {
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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<void> {
|
|||
},
|
||||
['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,
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue