diff --git a/src/scripts/add_library.py b/src/scripts/add_library.py index d7ba50d..a38896b 100644 --- a/src/scripts/add_library.py +++ b/src/scripts/add_library.py @@ -41,6 +41,8 @@ import sys, scriptengine as script_engine, os, traceback # bricking the next open. LIBRARY_NAME = "{LIBRARY_NAME}" +USE_DIRECT = "{USE_DIRECT}" == "1" +FORCE_DUP = "{FORCE_DUP}" == "1" def _resolve_in_repo(name): @@ -190,6 +192,26 @@ try: print("DEBUG: Library Manager found: %s" % getattr(lib_manager, 'get_name', lambda: '?')()) + # Step 0 (NEW per OPEN-BUGS-CROSS-REFERENCE Bug 4): dedup pre-check. + # If a reference with the same bare name already exists (whether + # placeholder or managed), no-op with a confirmation message rather + # than silently creating a duplicate. Bypass with FORCE_DUP=1. + existing_ref = _find_added_reference(lib_manager, LIBRARY_NAME) + if existing_ref is not None and not FORCE_DUP: + existing_name = getattr(existing_ref, 'name', '?') + is_ph = bool(getattr(existing_ref, 'is_placeholder', False)) + kind = "placeholder" if is_ph else "managed" + msg = ("Library '%s' is already referenced (%s, name=%r). " + "No-op (use force=true to add another reference)." + % (LIBRARY_NAME, kind, existing_name)) + print(msg) + print("Library Already Present: %s" % LIBRARY_NAME) + print("Project: %s" % project_name) + print("SCRIPT_SUCCESS: %s" % msg) + sys.exit(0) + if existing_ref is not None and FORCE_DUP: + print("DEBUG: dedup pre-check found existing reference for '%s' but FORCE_DUP=1 -- adding duplicate." % LIBRARY_NAME) + # Step 1: pre-resolve the library name against the installed repository. # If found, we will pass the ManagedLib to add_library() to get a # MANAGED reference instead of a placeholder reference. @@ -203,32 +225,70 @@ try: else: print("DEBUG: Pre-resolve via library_manager.find_library returned no hit for '%s'." % LIBRARY_NAME) - # Step 2: add the reference, preferring the managed overload. + # Step 2: add the reference. Default is add_placeholder() to match the + # modern ', * (System)' convention (placeholder resolves at + # compile time so transitive deps stay flexible). USE_DIRECT=1 opts + # into the legacy direct add_library() path (specific-version pin). + # Per docs: ScriptLibManObject exposes BOTH add_library(...) and + # add_placeholder(...) -- see helpme-codesys.com/ScriptLibManObject. added = False add_attempt_errors = [] - if resolved_lib is not None and hasattr(lib_manager, 'add_library'): + if not USE_DIRECT and hasattr(lib_manager, 'add_placeholder'): + # Default branch: placeholder add. Try (name, default_resolution) + # then (name) -- the default_resolution arg lets the IDE record + # which managed lib the placeholder should resolve to. try: - lib_manager.add_library(resolved_lib) - added = True - print("DEBUG: add_library(ManagedLib) succeeded.") + if resolved_lib is not None: + try: + lib_manager.add_placeholder(LIBRARY_NAME, resolved_lib) + added = True + print("DEBUG: add_placeholder(name, ManagedLib) succeeded.") + except Exception as e_pm: + add_attempt_errors.append("add_placeholder(name, ManagedLib): %s" % e_pm) + print("DEBUG: add_placeholder(name, ManagedLib) failed: %s" % e_pm) + if not added: + lib_manager.add_placeholder(LIBRARY_NAME) + added = True + print("DEBUG: add_placeholder(name) succeeded.") except Exception as e: - add_attempt_errors.append("add_library(ManagedLib): %s" % e) - print("DEBUG: add_library(ManagedLib) failed: %s" % e) + add_attempt_errors.append("add_placeholder(name): %s" % e) + print("DEBUG: add_placeholder(name) failed: %s" % e) + # Direct add_library path -- explicit opt-in OR fallback if the IDE + # doesn't expose add_placeholder. if not added and hasattr(lib_manager, 'add_library'): - try: - lib_manager.add_library(LIBRARY_NAME) - added = True - print("DEBUG: add_library(name) succeeded (placeholder overload).") - except Exception as e: - add_attempt_errors.append("add_library(name): %s" % e) - print("DEBUG: add_library(name) failed: %s" % e) + if resolved_lib is not None: + try: + lib_manager.add_library(resolved_lib) + added = True + print("DEBUG: add_library(ManagedLib) succeeded.") + except Exception as e: + add_attempt_errors.append("add_library(ManagedLib): %s" % e) + print("DEBUG: add_library(ManagedLib) failed: %s" % e) + if not added: + try: + lib_manager.add_library(LIBRARY_NAME) + added = True + print("DEBUG: add_library(name) succeeded (placeholder overload).") + except Exception as e: + add_attempt_errors.append("add_library(name): %s" % e) + print("DEBUG: add_library(name) failed: %s" % e) if not added: + # Per Bug 4 step 3: detect partial-success state. Neither + # add_library nor add_placeholder is exposed -- emit a clear + # error rather than silent-failing. + api_attrs = [] + try: + api_attrs = sorted([a for a in dir(lib_manager) if not a.startswith('_')]) + except Exception: + pass raise RuntimeError( - "Could not add library '%s'. add_library overloads failed: %s" - % (LIBRARY_NAME, "; ".join(add_attempt_errors) or "no add_library on libman")) + "Could not add library '%s'. Add overloads failed: %s. lm api: %s" + % (LIBRARY_NAME, + "; ".join(add_attempt_errors) or "neither add_library nor add_placeholder on libman", + ', '.join(api_attrs) if api_attrs else '')) # Step 3: verify the just-added reference actually resolved. If it did # not, REMOVE it and refuse to save -- saving an unresolvable diff --git a/src/server.ts b/src/server.ts index f8a5f26..a399d06 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1854,18 +1854,22 @@ export async function startMcpServer(config: ServerConfig): Promise { s.tool( 'add_library', - 'Adds a library reference to the CODESYS project. The library must be installed in the CODESYS library repository.', + "Adds a library reference to the CODESYS project. By default uses add_placeholder() to match the modern ', * (System)' convention so transitive deps resolve at compile time -- pass direct=true to opt into the legacy direct add_library() (specific version pin). Pre-checks lm.references for an existing reference with the same name and no-ops with a confirmation message unless force=true. The library must be installed in the CODESYS library repository.", { projectFilePath: z.string().describe("Path to the project file."), libraryName: z.string().describe("Name of the library to add (e.g., 'Standard', 'Util', 'CAA Memory')."), + direct: z.boolean().optional().describe("If true, use direct add_library() (specific-version pin) instead of the default add_placeholder() (resolves at compile)."), + force: z.boolean().optional().describe("If true, add even if a reference with the same name already exists (creates a duplicate). Default: dedup -- silently no-op with a confirmation message."), }, - async (args: { projectFilePath: string; libraryName: string }) => { + async (args: { projectFilePath: string; libraryName: string; direct?: boolean; force?: boolean }) => { const escaped = resolvePath(args.projectFilePath, workspaceDir); const script = scriptManager.prepareScriptWithHelpers( 'add_library', { PROJECT_FILE_PATH: escaped, LIBRARY_NAME: args.libraryName.trim(), + USE_DIRECT: args.direct ? '1' : '0', + FORCE_DUP: args.force ? '1' : '0', }, ['ensure_project_open'] ); diff --git a/tests/integration/e2e.test.ts b/tests/integration/e2e.test.ts index 0b121fd..cdcb80f 100644 --- a/tests/integration/e2e.test.ts +++ b/tests/integration/e2e.test.ts @@ -111,6 +111,8 @@ describe('E2E Script Preparation', () => { { PROJECT_FILE_PATH: 'C:\\test.project', LIBRARY_NAME: 'Util', + USE_DIRECT: '0', + FORCE_DUP: '0', }, ['ensure_project_open'] ); @@ -118,8 +120,13 @@ describe('E2E Script Preparation', () => { // Pre-resolve via the IDE-level library_manager expect(script).toContain('library_manager'); expect(script).toContain('find_library'); - // Managed-overload preference + // Managed-overload preference (when USE_DIRECT=1 or add_placeholder unavailable) expect(script).toContain('add_library(resolved_lib)'); + // Default-to-placeholder branch added per Bug 4 + expect(script).toContain('add_placeholder'); + // Dedup pre-check added per Bug 4 + expect(script).toContain('FORCE_DUP'); + expect(script).toContain('Library Already Present'); // Post-add resolution gate expect(script).toContain('effective_resolution'); expect(script).toContain('is_placeholder');