fix(tools): resolve --builtin-tools not registering tools and surface skipped UDTF columns

The --builtin-tools CLI flag was not registering built-in tools due to
an ES module evaluation timing bug: allToolDefinitions was computed as a
top-level constant at import time, before applyCliOverrides() could set
config.ibmi_enableDefaultTools = true. Changed to a function
(getAllToolDefinitions) evaluated at registration time.

Also fixed executeSql.tool.ts enabled callback to check live config
values as fallback, since toolConfig.enabled was captured at load time.

Added a `skipped` field to validate_query's column validation output so
UDTF/CTE unqualified columns are explicitly reported rather than
silently ignored. This surfaces column names like bogus typos that
callers should manually verify.

Signed-off-by: Adam Shedivy <ajshedivyaj@gmail.com>

Author: ajshedivy

server/src/ibmi-mcp-server/tools/executeSql.tool.ts

@@ -477,5 +477,8 @@ export const executeSqlTool = defineTool({
     destructiveHint: !(toolConfig.security?.readOnly ?? true), // Destructive if not read-only
     openWorldHint: !(toolConfig.security?.readOnly ?? true), // Open world if not read-only
   },
-  enabled: () => toolConfig.enabled, // Use function to dynamically check enabled state
+  enabled: () =>
+    toolConfig.enabled ||
+    config.ibmi_enableExecuteSql ||
+    config.ibmi_enableDefaultTools, // Check both toolConfig and live config for CLI override support
 });

server/src/ibmi-mcp-server/tools/index.ts

@@ -18,32 +18,37 @@ import { validateQueryTool } from "./validateQuery.tool.js";
 import { getRelatedObjectsTool } from "./getRelatedObjects.tool.js";
 
 /**
- * Default text-to-SQL toolset, enabled via IBMI_ENABLE_DEFAULT_TOOLS or --builtin-tools CLI flag (default: false).
- * These tools provide schema discovery and query validation for LLM workflows:
+ * Returns all tool definitions for automated registration.
+ *
+ * This is a function (not a constant) because the config values it depends on
+ * (e.g., ibmi_enableDefaultTools) may be set by CLI argument overrides that
+ * run after ES module evaluation. A top-level constant would capture the
+ * pre-override defaults and miss CLI flags like --builtin-tools.
+ *
+ * The default text-to-SQL toolset is enabled via IBMI_ENABLE_DEFAULT_TOOLS
+ * or --builtin-tools CLI flag (default: false). These tools provide schema
+ * discovery and query validation for LLM workflows:
  *   list_schemas → list_tables_in_schema → get_table_columns → get_related_objects → validate_query → execute_sql
- */
-const defaultTools = config.ibmi_enableDefaultTools
-  ? [
-      listSchemasTool,
-      listTablesInSchemaTool,
-      getTableColumnsTool,
-      getRelatedObjectsTool,
-      validateQueryTool,
-    ]
-  : [];
-
-/**
- * Array of all tool definitions for automated registration.
  *
  * To add a new tool:
  * 1. Create the tool definition file (e.g., myTool.tool.ts)
  * 2. Import it above
- * 3. Add it to this array
- *
- * The ToolRegistry will automatically register all tools in this array.
+ * 3. Add it to the returned array
  */
-export const allToolDefinitions = [
-  executeSqlTool, // Controlled by IBMI_ENABLE_EXECUTE_SQL or IBMI_ENABLE_DEFAULT_TOOLS
-  generateSqlTool, // Always available (describe_sql_object)
-  ...defaultTools, // Conditionally included default toolset
-];
+export function getAllToolDefinitions() {
+  const defaultTools = config.ibmi_enableDefaultTools
+    ? [
+        listSchemasTool,
+        listTablesInSchemaTool,
+        getTableColumnsTool,
+        getRelatedObjectsTool,
+        validateQueryTool,
+      ]
+    : [];
+
+  return [
+    executeSqlTool, // Controlled by IBMI_ENABLE_EXECUTE_SQL or IBMI_ENABLE_DEFAULT_TOOLS
+    generateSqlTool, // Always available (describe_sql_object)
+    ...defaultTools, // Conditionally included default toolset
+  ];
+}

server/src/ibmi-mcp-server/tools/validateQuery.tool.ts

@@ -40,7 +40,7 @@ interface RoutineRef {
 
 interface ObjectValidation {
   tables: { valid: string[]; invalid: string[] };
-  columns: { valid: string[]; invalid: string[] };
+  columns: { valid: string[]; invalid: string[]; skipped?: string[] };
   routines: { valid: string[]; invalid: string[] };
 }
 
@@ -83,6 +83,12 @@ const ObjectValidationSchema = z.object({
     invalid: z
       .array(z.string())
       .describe("Columns not found in any referenced table."),
+    skipped: z
+      .array(z.string())
+      .optional()
+      .describe(
+        "Columns skipped during validation (e.g., unqualified columns from UDTFs or CTE aliases that cannot be verified against the system catalog).",
+      ),
   }),
   routines: z.object({
     valid: z
@@ -394,16 +400,28 @@ export async function validateQueryLogic(
             validateRoutines(refs.routines, appContext),
           ]);
 
-          // CTE aliases and UDTF output columns appear as unqualified COLUMNs.
-          // When virtual columns are possible, only validate qualified columns
-          // (where PARSE_STATEMENT resolved SCHEMA + NAME to a physical table).
+          // CTE aliases and UDTF output columns appear as unqualified COLUMNs
+          // in PARSE_STATEMENT results. These cannot be verified against
+          // SYSCOLUMNS, so we skip them but report which columns were skipped.
           const hasVirtualColumns =
             refs.hasVirtualColumns || CTE_REGEX.test(params.sql_statement);
 
+          let skippedColumns: string[] | undefined;
           const columnsToValidate = hasVirtualColumns
             ? refs.columns.filter((c) => c.schema && c.tableName)
             : refs.columns;
 
+          if (hasVirtualColumns) {
+            const skipped = refs.columns.filter(
+              (c) => !c.schema || !c.tableName,
+            );
+            if (skipped.length > 0) {
+              skippedColumns = [
+                ...new Set(skipped.map((c) => c.columnName)),
+              ];
+            }
+          }
+
           const columnValidation = await validateColumns(
             columnsToValidate,
             validTableRefs,
@@ -412,7 +430,10 @@ export async function validateQueryLogic(
 
           objectValidation = {
             tables: tableValidation,
-            columns: columnValidation,
+            columns: {
+              ...columnValidation,
+              ...(skippedColumns ? { skipped: skippedColumns } : {}),
+            },
             routines: routineValidation,
           };
         }
@@ -506,6 +527,7 @@ const validateQueryResponseFormatter = (
   const invalidTables = ov?.tables.invalid ?? [];
   const invalidColumns = ov?.columns.invalid ?? [];
   const invalidRoutines = ov?.routines.invalid ?? [];
+  const skippedColumns = ov?.columns.skipped ?? [];
 
   const parts: string[] = [];
 
@@ -535,6 +557,13 @@ const validateQueryResponseFormatter = (
     parts.push(warnings.join("\n"));
   }
 
+  // Report columns that were skipped during validation
+  if (skippedColumns.length > 0) {
+    parts.push(
+      `Skipped column validation: ${skippedColumns.join(", ")} (from UDTF output or CTE alias — not present in SYSCOLUMNS). Verify these column names match the function's result set.`,
+    );
+  }
+
   parts.push(`Execution time: ${result.executionTime}ms`);
 
   const resultJson = JSON.stringify(result.data, null, 2);

server/src/mcp-server/tools/index.ts

@@ -8,7 +8,7 @@
  */
 
 import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
-import { allToolDefinitions } from "@/ibmi-mcp-server/tools/index.js";
+import { getAllToolDefinitions } from "@/ibmi-mcp-server/tools/index.js";
 import { registerToolFromDefinition } from "./utils/tool-factory.js";
 import { logger, requestContextService } from "../../utils/index.js";
 import {
@@ -19,19 +19,23 @@ import { config } from "@/config/index.js";
 
 /**
  * Registers all factory pattern tools with the MCP server.
- * Tools are defined in the allToolDefinitions array.
+ * Tools are defined by getAllToolDefinitions().
  */
 export async function registerAllTools(server: McpServer): Promise<void> {
   const context = requestContextService.createRequestContext({
     operation: "RegisterAllTools",
   });
 
+  // Evaluate tool definitions at registration time (not module load time)
+  // so that CLI overrides like --builtin-tools are reflected.
+  const toolDefinitions = getAllToolDefinitions();
+
   logOperationStart(
     context,
-    `Registering ${allToolDefinitions.length} factory pattern tools`,
+    `Registering ${toolDefinitions.length} factory pattern tools`,
   );
 
-  for (const toolDef of allToolDefinitions) {
+  for (const toolDef of toolDefinitions) {
     if (
       toolDef.name === "execute_sql" &&
       !config.ibmi_enableExecuteSql &&
@@ -49,6 +53,6 @@ export async function registerAllTools(server: McpServer): Promise<void> {
 
   logOperationSuccess(
     context,
-    `Successfully registered ${allToolDefinitions.length} factory pattern tools`,
+    `Successfully registered ${toolDefinitions.length} factory pattern tools`,
   );
 }

server/tests/unit/tools/defaultTools.test.ts

@@ -1285,6 +1285,84 @@ describe("Default Tools - validate_query", () => {
     ]);
     // Unqualified UDTF output columns should NOT be flagged as invalid
     expect(result.objectValidation!.columns.invalid).toEqual([]);
+    // They should be reported as skipped so callers know they weren't verified
+    expect(result.objectValidation!.columns.skipped).toEqual([
+      "OBJNAME",
+      "OBJSIZE",
+    ]);
+  });
+
+  it("should report bogus UDTF column names as skipped", async () => {
+    const { validateQueryTool } =
+      await import("../../../src/ibmi-mcp-server/tools/validateQuery.tool.js");
+
+    // Simulates a query with a typo in a UDTF output column name
+    // e.g., ENTRY_TIMESTAMPdsadasdas instead of ENTRY_TIMESTAMP
+    const parseData = [
+      {
+        NAME_TYPE: "FUNCTION",
+        NAME: "AUDIT_JOURNAL_CP",
+        SCHEMA: "SYSTOOLS",
+        COLUMN_NAME: null,
+        USAGE_TYPE: "QUERY",
+        SQL_STATEMENT_TYPE: "QUERY",
+      },
+      {
+        NAME_TYPE: "COLUMN",
+        NAME: null,
+        SCHEMA: null,
+        COLUMN_NAME: "ENTRY_TIMESTAMPdsadasdas",
+        USAGE_TYPE: "QUERY",
+        SQL_STATEMENT_TYPE: "QUERY",
+      },
+      {
+        NAME_TYPE: "COLUMN",
+        NAME: null,
+        SCHEMA: null,
+        COLUMN_NAME: "USER_NAME",
+        USAGE_TYPE: "QUERY",
+        SQL_STATEMENT_TYPE: "QUERY",
+      },
+      {
+        NAME_TYPE: "COLUMN",
+        NAME: null,
+        SCHEMA: null,
+        COLUMN_NAME: "CHANGED_PROFILE",
+        USAGE_TYPE: "QUERY",
+        SQL_STATEMENT_TYPE: "QUERY",
+      },
+    ];
+    const routinesData = [
+      { ROUTINE_SCHEMA: "SYSTOOLS", ROUTINE_NAME: "AUDIT_JOURNAL_CP" },
+    ];
+
+    mockExecuteQuery
+      .mockResolvedValueOnce(createMockQueryResult(parseData))
+      .mockResolvedValueOnce(createMockQueryResult(routinesData));
+
+    const result = await validateQueryTool.logic(
+      {
+        sql_statement:
+          "SELECT ENTRY_TIMESTAMPdsadasdas, USER_NAME, CHANGED_PROFILE FROM TABLE(SYSTOOLS.AUDIT_JOURNAL_CP(STARTING_TIMESTAMP => CURRENT_TIMESTAMP - 180 DAYS))",
+      },
+      context,
+      mockSdkContext,
+    );
+
+    expect(result.success).toBe(true);
+    expect(result.objectValidation!.routines.valid).toEqual([
+      "SYSTOOLS.AUDIT_JOURNAL_CP",
+    ]);
+    // No columns should be invalid (they're skipped, not validated)
+    expect(result.objectValidation!.columns.invalid).toEqual([]);
+    // All unqualified UDTF columns — including the bogus one — should be skipped
+    expect(result.objectValidation!.columns.skipped).toContain(
+      "ENTRY_TIMESTAMPdsadasdas",
+    );
+    expect(result.objectValidation!.columns.skipped).toContain("USER_NAME");
+    expect(result.objectValidation!.columns.skipped).toContain(
+      "CHANGED_PROFILE",
+    );
   });
 
   it("should skip unqualified column validation for CTE queries", async () => {
@@ -1417,8 +1495,10 @@ describe("Default Tools - validate_query", () => {
     expect(result.objectValidation!.columns.valid).toContain("CUSNUM");
     // Qualified FAKE_COL is still caught as invalid
     expect(result.objectValidation!.columns.invalid).toContain("FAKE_COL");
-    // Unqualified OBJNAME (UDTF output) is NOT flagged
+    // Unqualified OBJNAME (UDTF output) is NOT flagged as invalid
     expect(result.objectValidation!.columns.invalid).not.toContain("OBJNAME");
+    // It should be reported as skipped instead
+    expect(result.objectValidation!.columns.skipped).toContain("OBJNAME");
   });
 });
 
@@ -1659,10 +1739,10 @@ describe("Default Tools - get_related_objects", () => {
 
 describe("Default Tools - Tool Registry", () => {
   it("should include default tools when IBMI_ENABLE_DEFAULT_TOOLS is true", async () => {
-    const { allToolDefinitions } =
+    const { getAllToolDefinitions } =
       await import("../../../src/ibmi-mcp-server/tools/index.js");
 
-    const toolNames = allToolDefinitions.map((t) => t.name);
+    const toolNames = getAllToolDefinitions().map((t) => t.name);
     expect(toolNames).toContain("list_schemas");
     expect(toolNames).toContain("list_tables_in_schema");
     expect(toolNames).toContain("get_table_columns");
@@ -1673,14 +1753,14 @@ describe("Default Tools - Tool Registry", () => {
   });
 
   it("should have 7 total tools when defaults are enabled", async () => {
-    const { allToolDefinitions } =
+    const { getAllToolDefinitions } =
       await import("../../../src/ibmi-mcp-server/tools/index.js");
 
-    expect(allToolDefinitions).toHaveLength(7);
+    expect(getAllToolDefinitions()).toHaveLength(7);
   });
 
   it("should mark all default tools as read-only", async () => {
-    const { allToolDefinitions } =
+    const { getAllToolDefinitions } =
       await import("../../../src/ibmi-mcp-server/tools/index.js");
 
     const defaultToolNames = [
@@ -1692,7 +1772,7 @@ describe("Default Tools - Tool Registry", () => {
     ];
 
     for (const name of defaultToolNames) {
-      const tool = allToolDefinitions.find((t) => t.name === name);
+      const tool = getAllToolDefinitions().find((t) => t.name === name);
       expect(tool, `Tool ${name} should exist`).toBeDefined();
       expect(
         tool?.annotations?.readOnlyHint,