diff --git a/.changeset/patch-clean-safe-outputs-loader.md b/.changeset/patch-clean-safe-outputs-loader.md new file mode 100644 index 00000000000..54e80ffa9f0 --- /dev/null +++ b/.changeset/patch-clean-safe-outputs-loader.md @@ -0,0 +1,9 @@ +--- +"gh-aw": patch +--- + +Clean and modernize `pkg/workflow/js/safe_outputs_tools_loader.cjs` by refactoring +internal functions (`loadTools`, `attachHandlers`, `registerDynamicTools`) to use +modern JavaScript patterns (optional chaining, nullish coalescing, handler map) +and reduce nesting and complexity. No behavioral changes. + diff --git a/pkg/workflow/js/safe_outputs_tools_loader.cjs b/pkg/workflow/js/safe_outputs_tools_loader.cjs index 69eb19e315d..88c1975e942 100644 --- a/pkg/workflow/js/safe_outputs_tools_loader.cjs +++ b/pkg/workflow/js/safe_outputs_tools_loader.cjs @@ -9,30 +9,28 @@ const fs = require("fs"); */ function loadTools(server) { const toolsPath = process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH || "/tmp/gh-aw/safeoutputs/tools.json"; - let ALL_TOOLS = []; - + server.debug(`Reading tools from file: ${toolsPath}`); + if (!fs.existsSync(toolsPath)) { + server.debug(`Tools file does not exist at: ${toolsPath}`); + server.debug(`Using empty tools array`); + return []; + } + try { - if (fs.existsSync(toolsPath)) { - server.debug(`Tools file exists at: ${toolsPath}`); - const toolsFileContent = fs.readFileSync(toolsPath, "utf8"); - server.debug(`Tools file content length: ${toolsFileContent.length} characters`); - server.debug(`Tools file read successfully, attempting to parse JSON`); - ALL_TOOLS = JSON.parse(toolsFileContent); - server.debug(`Successfully parsed ${ALL_TOOLS.length} tools from file`); - } else { - server.debug(`Tools file does not exist at: ${toolsPath}`); - server.debug(`Using empty tools array`); - ALL_TOOLS = []; - } + server.debug(`Tools file exists at: ${toolsPath}`); + const toolsFileContent = fs.readFileSync(toolsPath, "utf8"); + server.debug(`Tools file content length: ${toolsFileContent.length} characters`); + server.debug(`Tools file read successfully, attempting to parse JSON`); + const tools = JSON.parse(toolsFileContent); + server.debug(`Successfully parsed ${tools.length} tools from file`); + return tools; } catch (error) { server.debug(`Error reading tools file: ${error instanceof Error ? error.message : String(error)}`); server.debug(`Falling back to empty tools array`); - ALL_TOOLS = []; + return []; } - - return ALL_TOOLS; } /** @@ -42,15 +40,19 @@ function loadTools(server) { * @returns {Array} Tools with handlers attached */ function attachHandlers(tools, handlers) { + const handlerMap = { + create_pull_request: handlers.createPullRequestHandler, + push_to_pull_request_branch: handlers.pushToPullRequestBranchHandler, + upload_asset: handlers.uploadAssetHandler, + }; + tools.forEach(tool => { - if (tool.name === "create_pull_request") { - tool.handler = handlers.createPullRequestHandler; - } else if (tool.name === "push_to_pull_request_branch") { - tool.handler = handlers.pushToPullRequestBranchHandler; - } else if (tool.name === "upload_asset") { - tool.handler = handlers.uploadAssetHandler; + const handler = handlerMap[tool.name]; + if (handler) { + tool.handler = handler; } }); + return tools; } @@ -84,76 +86,64 @@ function registerDynamicTools(server, tools, config, outputFile, registerTool, n const normalizedKey = normalizeTool(configKey); // Skip if it's already a predefined tool - if (server.tools[normalizedKey]) { + if (server.tools[normalizedKey] || tools.find(t => t.name === normalizedKey)) { return; } - // Check if this is a safe-job (not in ALL_TOOLS) - if (!tools.find(t => t.name === normalizedKey)) { - const jobConfig = config[configKey]; - - // Create a dynamic tool for this safe-job - const dynamicTool = { - name: normalizedKey, - description: jobConfig && jobConfig.description ? jobConfig.description : `Custom safe-job: ${configKey}`, - inputSchema: { - type: "object", - properties: {}, - additionalProperties: true, // Allow any properties for flexibility - }, - handler: args => { - // Create a generic safe-job output entry - const entry = { - type: normalizedKey, - ...args, - }; - - // Write the entry to the output file in JSONL format - // CRITICAL: Use JSON.stringify WITHOUT formatting parameters for JSONL format - // Each entry must be on a single line, followed by a newline character - const entryJSON = JSON.stringify(entry); - fs.appendFileSync(outputFile, entryJSON + "\n"); - - // Use output from safe-job config if available - const outputText = jobConfig && jobConfig.output ? jobConfig.output : `Safe-job '${configKey}' executed successfully with arguments: ${JSON.stringify(args)}`; - - return { - content: [ - { - type: "text", - text: JSON.stringify({ result: outputText }), - }, - ], - }; - }, - }; - - // Add input schema based on job configuration if available - if (jobConfig && jobConfig.inputs) { - dynamicTool.inputSchema.properties = {}; - dynamicTool.inputSchema.required = []; - - Object.keys(jobConfig.inputs).forEach(inputName => { - const inputDef = jobConfig.inputs[inputName]; - const propSchema = { - type: inputDef.type || "string", - description: inputDef.description || `Input parameter: ${inputName}`, - }; - - if (inputDef.options && Array.isArray(inputDef.options)) { - propSchema.enum = inputDef.options; - } - - dynamicTool.inputSchema.properties[inputName] = propSchema; - - if (inputDef.required) { - dynamicTool.inputSchema.required.push(inputName); - } - }); - } - - registerTool(server, dynamicTool); + const jobConfig = config[configKey]; + + // Create a dynamic tool for this safe-job + const dynamicTool = { + name: normalizedKey, + description: jobConfig?.description ?? `Custom safe-job: ${configKey}`, + inputSchema: { + type: "object", + properties: {}, + additionalProperties: true, // Allow any properties for flexibility + }, + handler: args => { + // Create a generic safe-job output entry + const entry = { type: normalizedKey, ...args }; + + // Write the entry to the output file in JSONL format + // CRITICAL: Use JSON.stringify WITHOUT formatting parameters for JSONL format + // Each entry must be on a single line, followed by a newline character + fs.appendFileSync(outputFile, `${JSON.stringify(entry)}\n`); + + // Use output from safe-job config if available + const outputText = jobConfig?.output ?? `Safe-job '${configKey}' executed successfully with arguments: ${JSON.stringify(args)}`; + + return { + content: [{ type: "text", text: JSON.stringify({ result: outputText }) }], + }; + }, + }; + + // Add input schema based on job configuration if available + if (jobConfig?.inputs) { + dynamicTool.inputSchema.properties = {}; + dynamicTool.inputSchema.required = []; + + Object.keys(jobConfig.inputs).forEach(inputName => { + const inputDef = jobConfig.inputs[inputName]; + const propSchema = { + type: inputDef.type || "string", + description: inputDef.description || `Input parameter: ${inputName}`, + }; + + if (Array.isArray(inputDef.options)) { + propSchema.enum = inputDef.options; + } + + dynamicTool.inputSchema.properties[inputName] = propSchema; + + if (inputDef.required) { + dynamicTool.inputSchema.required.push(inputName); + } + }); } + + registerTool(server, dynamicTool); }); }