nikon/Codesys-MCP-SP21-plus
0
0
Fork 0
Code Activity Actions

fix(create_folder): try project.create_folder(name, SV_POU) first; drop ScriptManager cache

Browse source 
create_folder v2 (positional foldername) returned None silently against
the SP22 Application object -- no exception raised, no folder created.
Investigation showed:
  - ScriptObject.create_folder(foldername) is documented to "create a
    folder in the structured view of the parent node", but on Application
    specifically it's a silent no-op (the structured view isn't pinned to
    POU view there).
  - ScriptProject.create_folder(foldername, structured_view=None) on the
    project itself with explicit SV_POU GUID
    ({21AF5390-2942-461a-BF89-951AAF6999F1}) is the documented and
    reliable pathway -- the resulting folder appears under Application
    in the IDE tree because that's where SV_POU lives.

v3 fix: try strategies in order until one returns non-None:
  (1) primary_project.create_folder(name, SV_POU_GUID) -- new, primary
  (2) parent.create_folder(name) positional -- pre-SP21 path
  (3) parent.create_folder(foldername=name) -- alt keyword
  (4) primary_project.create_folder(name) -- default view
  (5) parent.create_object(typeUuid='85d1215e-...') -- alt factory
  (6) parent.add(script_engine.types.IecFolder, name=name) -- legacy
Each strategy guards on hasattr + return-value-not-None, so a silent
no-op falls through instead of being mistaken for success.

ScriptManager: dropped the in-memory template cache. Each loadTemplate
call now reads the .py from disk fresh. Cost: ~1ms per call vs ~1.5s
of CODESYS execution time -- invisible. Win: edits to dist/scripts/
take effect without an MCP restart, which makes iterating on script-
side fixes (like this very create_folder loop) much faster. Existing
"cache hit" unit test rewritten as "two loads return equal content".

tests/test-fixes.mjs: standalone harness that drives a single persistent
CODESYS through HeadlessExecutor + CodesysLauncher to verify the four
broken-tool fixes end-to-end. Useful for regression-testing without
needing a vsc reboot loop. Currently only smoke-tests
create_folder + compile + cross-project; expand as more fixes need
verification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Karstein Kvistad 2026-04-26 17:16:44 +02:00
parent e07f281fd0
commit 32e612000d
4 changed files with 224 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/script-manager.ts
View file

@ -1,6 +1,11 @@
/**
* Python script template loading and interpolation.
* Loads .py templates from src/scripts/, caches them, and performs {PARAM} replacement.
* Loads .py templates from src/scripts/ (or dist/scripts/) and performs
* {PARAM} replacement. No caching: a tool call is ~1.5 s of CODESYS time,
* so the few-ms cost of re-reading a small .py file each call is invisible
* AND it means edits to dist/scripts/ are picked up live without an MCP
* restart. This makes iterating on script-side fixes much faster
* (relevant for the SP21+ scripting-engine drift bugs we hit on this fork).
*/
import * as fs from 'fs';
@ -9,28 +14,19 @@ import { ScriptParams } from './types';
export class ScriptManager {
private scriptsDir: string;
private cache: Map<string, string> = new Map();
constructor(scriptsDir?: string) {
this.scriptsDir = scriptsDir ?? path.join(__dirname, 'scripts');
}
/** Synchronously load a template file and cache it */
/** Synchronously read a template file. Re-reads on every call -- no cache. */
loadTemplate(name: string): string {
const fileName = name.endsWith('.py') ? name : `${name}.py`;
const cached = this.cache.get(fileName);
if (cached !== undefined) {
return cached;
}
const filePath = path.join(this.scriptsDir, fileName);
if (!fs.existsSync(filePath)) {
throw new Error(`Script template not found: ${filePath}`);
}
const content = fs.readFileSync(filePath, 'utf-8');
this.cache.set(fileName, content);
return content;
return fs.readFileSync(filePath, 'utf-8');
}
/**

87
src/scripts/create_folder.py
View file

@ -46,38 +46,84 @@ try:
parent_name = getattr(parent_object, 'get_name', lambda: str(parent_object))()
print("DEBUG: Using parent object: %s" % parent_name)
# Create the folder. The keyword changed between docs and stubs:
# Per the SP22 stub Stubs/scriptengine/ScriptObject.pyi, the signature is
# def create_folder(self, foldername): ...
# NOT `name=...` (which the original fork code used and got
# "create_folder() got an unexpected keyword argument 'name'" against
# SP22). Use the positional form first to be agnostic to the keyword
# name across SP releases. Fall through to alternate factories if
# create_folder is unavailable on the parent at all.
# Create the folder. Per the SP22 stubs:
# ScriptObject.create_folder(foldername) -- on POUs / sub-objects
# ScriptProject.create_folder(foldername, structured_view=None)
# -- on the project itself,
# accepts an explicit view GUID
# On SP22 specifically, calling .create_folder('X') on an Application object
# returns None silently (no exception, no folder created). The reliable
# pathway is to call create_folder on the PROJECT with an explicit
# structured_view GUID -- the SV_POU view is where Application's children
# live, so a folder there will appear under Application in the IDE tree.
#
# SV_POU GUID = {21AF5390-2942-461a-BF89-951AAF6999F1}. (Documented in the
# ScriptProject.pyi stub; constant since SP3.5.2.0.)
SV_POU_GUID_STR = '21AF5390-2942-461a-BF89-951AAF6999F1'
sv_pou_guid = None
try:
from System import Guid
sv_pou_guid = Guid(SV_POU_GUID_STR)
except Exception as guid_e:
print("WARN: Could not construct System.Guid for SV_POU: %s" % guid_e)
new_folder = None
if hasattr(parent_object, 'create_folder'):
# Strategy 1: project-level create_folder with explicit POU view. This is
# the only call shape that reliably works on SP22 Application children.
if hasattr(primary_project, 'create_folder') and sv_pou_guid is not None:
try:
print("DEBUG: Calling parent.create_folder('%s') [positional]" % FOLDER_NAME)
new_folder = parent_object.create_folder(FOLDER_NAME)
print("DEBUG: Trying primary_project.create_folder('%s', SV_POU)" % FOLDER_NAME)
new_folder = primary_project.create_folder(FOLDER_NAME, sv_pou_guid)
if new_folder is not None:
print("DEBUG: project.create_folder(SV_POU) succeeded.")
except Exception as e:
print("WARN: parent.create_folder('%s') raised: %s -- trying foldername= kwarg." % (FOLDER_NAME, e))
print("WARN: primary_project.create_folder('%s', SV_POU) raised: %s" % (FOLDER_NAME, e))
new_folder = None
# Strategy 2: parent.create_folder (positional). Works pre-SP21 and on
# parents whose factories haven't been pinned to project-level.
if new_folder is None and hasattr(parent_object, 'create_folder'):
try:
print("DEBUG: Trying parent.create_folder('%s') [positional]" % FOLDER_NAME)
new_folder = parent_object.create_folder(FOLDER_NAME)
if new_folder is not None:
print("DEBUG: parent.create_folder() positional succeeded.")
except Exception as e:
print("WARN: parent.create_folder('%s') raised: %s" % (FOLDER_NAME, e))
new_folder = None
if new_folder is None:
try:
new_folder = parent_object.create_folder(foldername=FOLDER_NAME)
if new_folder is not None:
print("DEBUG: parent.create_folder(foldername=) succeeded.")
except Exception as e2:
print("WARN: parent.create_folder(foldername='%s') raised: %s -- trying alternate factories." % (FOLDER_NAME, e2))
print("WARN: parent.create_folder(foldername='%s') raised: %s" % (FOLDER_NAME, e2))
new_folder = None
# Strategy 3: project-level create_folder default view (POU).
if new_folder is None and hasattr(primary_project, 'create_folder'):
try:
print("DEBUG: Trying primary_project.create_folder('%s') [default view]" % FOLDER_NAME)
new_folder = primary_project.create_folder(FOLDER_NAME)
if new_folder is not None:
print("DEBUG: project.create_folder() default-view succeeded.")
except Exception as e:
print("WARN: primary_project.create_folder('%s') raised: %s" % (FOLDER_NAME, e))
new_folder = None
# Strategy 4: generic create_object with the folder type UUID. Last-ditch
# for non-standard parent types.
if new_folder is None and hasattr(parent_object, 'create_object'):
# CODESYS folder type UUID -- documented "generic IEC folder" type.
# Tried as a fallback for parents that don't expose create_folder.
FOLDER_TYPE_UUID = '85d1215e-6520-4983-9a55-2d39d1f24cb4'
try:
print("DEBUG: parent.create_folder() unavailable. Trying parent.create_object(typeUuid=%s, name='%s')" % (FOLDER_TYPE_UUID, FOLDER_NAME))
print("DEBUG: Trying parent.create_object(typeUuid=%s, name='%s')" % (FOLDER_TYPE_UUID, FOLDER_NAME))
new_folder = parent_object.create_object(typeUuid=FOLDER_TYPE_UUID, name=FOLDER_NAME)
except Exception as e:
print("WARN: parent.create_object(typeUuid=%s) raised: %s" % (FOLDER_TYPE_UUID, e))
new_folder = None
# Strategy 5: types.IecFolder + parent.add (very old API).
if new_folder is None and hasattr(script_engine, 'types') and hasattr(script_engine.types, 'IecFolder') and hasattr(parent_object, 'add'):
try:
print("DEBUG: Trying parent.add(script_engine.types.IecFolder, name='%s')" % FOLDER_NAME)
@ -88,9 +134,12 @@ try:
if new_folder is None:
raise TypeError(
"Parent object '%s' of type %s does not support any known folder-creation factory: "
"tried create_folder() positional, create_folder(foldername=...), "
"create_object(typeUuid=...), and add(script_engine.types.IecFolder)." % (
"Parent object '%s' of type %s -- folder creation failed for all known strategies: "
"(1) primary_project.create_folder(name, SV_POU), "
"(2) parent.create_folder(name), "
"(3) primary_project.create_folder(name) default view, "
"(4) parent.create_object(typeUuid='85d1215e-...'), "
"(5) parent.add(script_engine.types.IecFolder)." % (
parent_name, type(parent_object).__name__))
if new_folder:

146
tests/test-fixes.mjs Normal file
View file

@ -0,0 +1,146 @@
#!/usr/bin/env node
// Verifies the four broken-tool fixes (commit 2607063) end-to-end.
// Drives a single persistent CODESYS instance, runs each test against
// a *copy* of MCPTest2 so the source binary isn't mutated, and reports
// pass/fail with the relevant slice of script output for inspection.
import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
import { fileURLToPath } from 'url';
import { CodesysLauncher } from '../dist/launcher.js';
import { ScriptManager } from '../dist/script-manager.js';
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const repoRoot = path.resolve(__dirname, '..');
const MCPTEST2 = '\\\\files\\karstein.kvistad\\Documents\\Claude\\PLC\\MCPTest2\\MCPTest2.project';
const MARINER = '\\\\files\\karstein.kvistad\\Documents\\Claude\\PLC\\mariner40206\\MRCodesysMarinerMK2.6_012.project';
const config = {
codesysPath: 'C:\\Program Files\\CODESYS 3.5.22.10\\CODESYS\\Common\\CODESYS.exe',
profileName: 'CODESYS V3.5 SP22 Patch 1',
};
// Set up a working copy of MCPTest2.
const workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codesys-mcp-testfixes-'));
const workMCP = path.join(workDir, 'MCPTest2.project');
fs.copyFileSync(MCPTEST2, workMCP);
console.log(`copy: ${workMCP}`);
const sm = new ScriptManager(path.join(repoRoot, 'src', 'scripts'));
const launcher = new CodesysLauncher(config);
console.log('launching persistent CODESYS...');
await launcher.launch();
console.log(' ready.');
const results = [];
async function run(name, scriptArgs, helpers, projectPath) {
const params = { PROJECT_FILE_PATH: projectPath, ...scriptArgs };
const script = sm.prepareScriptWithHelpers(name, params, helpers);
const t0 = process.hrtime.bigint();
const r = await launcher.executeScript(script);
const ms = Number(process.hrtime.bigint() - t0) / 1e6;
const ok = r?.success && (r.output || '').includes('SCRIPT_SUCCESS');
// ScriptManager caches templates after first load, so re-instantiate to
// force fresh reads if we mutate src/scripts/. Not needed here -- we
// only load each name once per test.
return { ok, ms, output: r?.output || '', error: r?.error || '' };
}
// --- TEST 1: create_folder fix ---
console.log('\n[1/3] create_folder fix...');
{
const r = await run('create_folder',
{ FOLDER_NAME: 'Test_Bench_Folder', PARENT_PATH: 'PLCWinNT/Plc Logic/Application' },
['ensure_project_open', 'find_object_by_path'], workMCP);
console.log(` ${r.ms.toFixed(0)} ms ${r.ok ? 'PASS' : 'FAIL'}`);
if (!r.ok) {
console.log(' ---output tail---');
console.log(r.output.slice(-1500).split('\n').map(l => ' ' + l).join('\n'));
}
// Cleanup: delete the folder so MCPTest2 stays unchanged for next runs.
if (r.ok) {
const cleanup = await run('delete_object',
{ OBJECT_PATH: 'PLCWinNT/Plc Logic/Application/Test_Bench_Folder' },
['ensure_project_open', 'find_object_by_path'], workMCP);
console.log(` cleanup delete_object: ${cleanup.ok ? 'OK' : 'FAIL'}`);
}
results.push({ name: 'create_folder', ok: r.ok, ms: r.ms });
}
// --- TEST 2: compile_project + get_compile_messages fix ---
console.log('\n[2/3] compile_project + get_compile_messages fix...');
{
const r = await run('compile_project', {}, ['ensure_project_open'], workMCP);
console.log(` compile_project: ${r.ms.toFixed(0)} ms ${r.ok ? 'PASS' : 'FAIL'}`);
if (!r.ok) {
console.log(' ---output tail---');
console.log(r.output.slice(-1500).split('\n').map(l => ' ' + l).join('\n'));
} else {
// Verify the JSON markers + parseable JSON in the output (this is
// exactly what would have failed pre-fix).
const m = r.output.match(/### COMPILE_MESSAGES_START ###\n([\s\S]*?)\n### COMPILE_MESSAGES_END ###/);
if (!m) {
console.log(' WARN: no markers found in output (still passed -- but JSON emit may be missing)');
} else {
try {
const parsed = JSON.parse(m[1]);
console.log(` json parsed OK: ${parsed.length} messages`);
if (parsed.length > 0) {
console.log(' first message:', JSON.stringify(parsed[0]));
}
} catch (e) {
console.log(` FAIL: json parse error: ${e.message}`);
}
}
}
results.push({ name: 'compile_project', ok: r.ok, ms: r.ms });
const r2 = await run('get_compile_messages', {}, ['ensure_project_open'], workMCP);
console.log(` get_compile_messages: ${r2.ms.toFixed(0)} ms ${r2.ok ? 'PASS' : 'FAIL'}`);
if (!r2.ok) {
console.log(' ---output tail---');
console.log(r2.output.slice(-1500).split('\n').map(l => ' ' + l).join('\n'));
}
results.push({ name: 'get_compile_messages', ok: r2.ok, ms: r2.ms });
}
// --- TEST 3: ensure_project_open cross-project switch ---
console.log('\n[3/3] ensure_project_open cross-project switch fix...');
{
// Currently MCPTest2 working copy is primary. Switch to mariner40206.
// Pre-fix: this would either fail or leave MCPTest2 still primary.
const r = await run('open_project', {}, ['ensure_project_open'], MARINER);
console.log(` open_project(mariner40206): ${r.ms.toFixed(0)} ms ${r.ok ? 'PASS' : 'FAIL'}`);
if (!r.ok) {
console.log(' ---output tail---');
console.log(r.output.slice(-1500).split('\n').map(l => ' ' + l).join('\n'));
}
// Verify: list_project_libraries should show mariner's libs (~64), not MCPTest2's (5).
if (r.ok) {
const verify = await run('list_project_libraries', {}, ['ensure_project_open'], MARINER);
const libCount = (verify.output.match(/(\d+) library reference\(s\)/) || [])[1];
console.log(` verify list_project_libraries: lib count = ${libCount} (expected ~64 for mariner40206)`);
const switchOk = libCount && parseInt(libCount, 10) > 50;
console.log(` cross-project switch verified: ${switchOk ? 'PASS' : 'FAIL'}`);
results.push({ name: 'cross_project_switch', ok: r.ok && switchOk, ms: r.ms });
} else {
results.push({ name: 'cross_project_switch', ok: false, ms: r.ms });
}
}
console.log('\n=== summary ===');
for (const t of results) {
console.log(` ${t.ok ? '✓' : '✗'} ${t.name} (${t.ms.toFixed(0)} ms)`);
}
console.log('\nshutting down...');
await launcher.shutdown();
try { fs.rmSync(workDir, { recursive: true, force: true }); } catch {}
console.log('done.');
const allOk = results.every(r => r.ok);
process.exit(allOk ? 0 : 1);

4
tests/unit/script-manager.test.ts
View file

@ -41,10 +41,10 @@ describe('ScriptManager', () => {
expect(result).toBe('x and y');
});
it('cache hit - second load returns same content', () => {
it('two loads return identical content (no cache, fresh file read each call)', () => {
const first = mgr.loadTemplate('check_status');
const second = mgr.loadTemplate('check_status');
expect(first).toBe(second); // Same reference from cache
expect(first).toEqual(second);
});
it('combineScripts concatenates with double newlines', () => {