From c2180a9e98509579d762f483f63417894382ad66 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 28 May 2026 16:56:42 +0000 Subject: [PATCH] chore: sync actions from gh-aw@v0.77.0 --- setup/action.yml | 4 + setup/index.js | 3 + setup/js/action_setup_otlp.cjs | 57 + setup/js/add_comment.cjs | 22 +- setup/js/add_labels.cjs | 24 +- setup/js/add_reaction.cjs | 8 +- setup/js/add_reviewer.cjs | 136 +- setup/js/check_workflow_timestamp_api.cjs | 49 +- setup/js/commit_sha_helpers.cjs | 20 + setup/js/constants.cjs | 20 + setup/js/copilot_harness.cjs | 85 + setup/js/create_forecast_issue.cjs | 150 +- setup/js/create_pull_request.cjs | 39 +- setup/js/effective_tokens.cjs | 99 +- setup/js/emit_outcome_spans.cjs | 19 +- setup/js/evaluate_outcomes.cjs | 1618 ++++++++++++++++- setup/js/extra_empty_commit.cjs | 22 +- .../extract_base_branch_from_agent_output.cjs | 57 + setup/js/frontmatter_hash_pure.cjs | 125 +- setup/js/fuzz_template_branch_harness.cjs | 5 +- setup/js/generate_aw_info.cjs | 106 +- setup/js/generate_git_patch.cjs | 35 +- setup/js/git_helpers.cjs | 15 +- setup/js/handle_agent_failure.cjs | 25 +- setup/js/interpolate_prompt.cjs | 116 +- setup/js/merge_awf_model_multipliers.cjs | 212 +++ setup/js/messages_footer.cjs | 30 + setup/js/package.json | 8 +- setup/js/parse_mcp_gateway_log.cjs | 2 +- setup/js/parse_threat_detection_results.cjs | 216 ++- setup/js/pi_provider.cjs | 50 + setup/js/pr_review_buffer.cjs | 96 +- setup/js/push_to_pull_request_branch.cjs | 97 +- setup/js/redact_secrets.cjs | 7 +- setup/js/render_template.cjs | 45 +- setup/js/safe_output_execution_metadata.cjs | 252 +++ setup/js/safe_output_handler_manager.cjs | 50 +- setup/js/safe_output_manifest.cjs | 33 +- setup/js/safe_outputs_handlers.cjs | 84 +- setup/js/safe_outputs_mcp_server.cjs | 2 +- setup/js/safe_outputs_mcp_server_http.cjs | 2 +- setup/js/safe_outputs_tools.json | 4 +- setup/js/safe_outputs_tools_loader.cjs | 65 +- setup/js/send_otlp_span.cjs | 40 +- setup/js/submit_pr_review.cjs | 1 + setup/js/update_handler_factory.cjs | 12 +- setup/js/update_issue.cjs | 5 + setup/js/update_pull_request.cjs | 18 +- setup/md/forecast_issue.md | 12 + setup/md/threat_detection.md | 6 + setup/setup.sh | 13 - setup/sh/mask_otlp_headers.sh | 21 +- setup/sh/setup_cache_memory_git.sh | 40 + setup/sh/setup_cache_memory_git_test.sh | 18 + setup/sh/start_cli_proxy.sh | 1 + setup/sh/start_difc_proxy.sh | 1 + 56 files changed, 3751 insertions(+), 551 deletions(-) create mode 100644 setup/js/commit_sha_helpers.cjs create mode 100644 setup/js/extract_base_branch_from_agent_output.cjs create mode 100644 setup/js/merge_awf_model_multipliers.cjs create mode 100644 setup/js/safe_output_execution_metadata.cjs create mode 100644 setup/md/forecast_issue.md diff --git a/setup/action.yml b/setup/action.yml index d70cc20d..bf9b64ec 100644 --- a/setup/action.yml +++ b/setup/action.yml @@ -22,6 +22,10 @@ inputs: description: 'OTLP parent span ID (16-character hexadecimal string) for the setup span. Pass the setup-span-id output of the upstream setup step so job setup spans form a single tree.' required: false default: '' + otlp-oidc-token: + description: 'Optional pre-minted OIDC bearer token used for OTLP Authorization headers.' + required: false + default: '' outputs: files_copied: diff --git a/setup/index.js b/setup/index.js index b8084853..7a37e776 100644 --- a/setup/index.js +++ b/setup/index.js @@ -16,6 +16,7 @@ const safeOutputArtifactClient = getActionInput("SAFE_OUTPUT_ARTIFACT_CLIENT") | const inputTraceId = getActionInput("TRACE_ID"); const inputParentSpanId = getActionInput("PARENT_SPAN_ID"); const inputJobName = getActionInput("JOB_NAME"); +const inputOTLPOIDCToken = getActionInput("OTLP_OIDC_TOKEN"); const result = spawnSync(path.join(__dirname, "setup.sh"), [], { stdio: "inherit", @@ -25,6 +26,7 @@ const result = spawnSync(path.join(__dirname, "setup.sh"), [], { INPUT_TRACE_ID: inputTraceId, INPUT_PARENT_SPAN_ID: inputParentSpanId, INPUT_JOB_NAME: inputJobName, + INPUT_OTLP_OIDC_TOKEN: inputOTLPOIDCToken, // Tell setup.sh to skip the OTLP span: in action mode index.js sends it // after setup.sh returns so that the startMs captured here is used. GH_AW_SKIP_SETUP_OTLP: "1", @@ -53,6 +55,7 @@ if (result.status !== 0) { process.env.INPUT_TRACE_ID = inputTraceId; process.env.INPUT_PARENT_SPAN_ID = inputParentSpanId; process.env.INPUT_JOB_NAME = inputJobName; + process.env.INPUT_OTLP_OIDC_TOKEN = inputOTLPOIDCToken; const { run } = require(path.join(__dirname, "js", "action_setup_otlp.cjs")); await run(); } catch { diff --git a/setup/js/action_setup_otlp.cjs b/setup/js/action_setup_otlp.cjs index 2fc39549..04c80d3b 100644 --- a/setup/js/action_setup_otlp.cjs +++ b/setup/js/action_setup_otlp.cjs @@ -44,6 +44,47 @@ function writeEnvLine(filePath, key, value, logLabel, fileLabel) { console.log(`[otlp] ${logLabel} written to ${fileLabel}`); } +/** + * @param {string} headers + * @returns {boolean} + */ +function hasAuthorizationHeader(headers) { + return /(^|,)\s*authorization\s*=/i.test(headers); +} + +/** + * @param {string} headers + * @param {string} token + * @returns {string} + */ +function mergeAuthorizationHeader(headers, token) { + if (hasAuthorizationHeader(headers)) return headers; + return (headers ? `${headers},` : "") + "Authorization=Bearer " + token; +} + +/** + * @param {string} endpointsRaw + * @param {string} token + * @returns {string} + */ +function mergeAuthorizationIntoOTLPEndpoints(endpointsRaw, token) { + if (!endpointsRaw) return endpointsRaw; + let parsed; + try { + parsed = JSON.parse(endpointsRaw); + } catch { + return endpointsRaw; + } + if (!Array.isArray(parsed)) return endpointsRaw; + const updated = parsed.map(entry => { + if (!entry || typeof entry !== "object") return entry; + const currentHeaders = typeof entry.headers === "string" ? entry.headers : ""; + const mergedHeaders = mergeAuthorizationHeader(currentHeaders, token); + return { ...entry, headers: mergedHeaders }; + }); + return JSON.stringify(updated); +} + /** * Send the OTLP job-setup span and propagate trace context via GITHUB_OUTPUT / * GITHUB_ENV. Non-fatal: all errors are silently swallowed. @@ -86,6 +127,22 @@ async function run() { process.env.INPUT_PARENT_SPAN_ID = inputParentSpanId; } + const inputOTLPOIDCToken = getActionInput("OTLP_OIDC_TOKEN"); + if (inputOTLPOIDCToken) { + const existingHeaders = process.env.OTEL_EXPORTER_OTLP_HEADERS || ""; + const mergedHeaders = mergeAuthorizationHeader(existingHeaders, inputOTLPOIDCToken); + + process.env.OTEL_EXPORTER_OTLP_HEADERS = mergedHeaders; + writeEnvLine(process.env.GITHUB_ENV, "OTEL_EXPORTER_OTLP_HEADERS", mergedHeaders, "OTEL_EXPORTER_OTLP_HEADERS", "GITHUB_ENV"); + + const existingEndpoints = process.env.GH_AW_OTLP_ENDPOINTS || ""; + const mergedEndpoints = mergeAuthorizationIntoOTLPEndpoints(existingEndpoints, inputOTLPOIDCToken); + if (mergedEndpoints && mergedEndpoints !== existingEndpoints) { + process.env.GH_AW_OTLP_ENDPOINTS = mergedEndpoints; + writeEnvLine(process.env.GITHUB_ENV, "GH_AW_OTLP_ENDPOINTS", mergedEndpoints, "GH_AW_OTLP_ENDPOINTS", "GITHUB_ENV"); + } + } + if (!endpoints) { console.log("[otlp] GH_AW_OTLP_ENDPOINTS not set, skipping setup span"); } else { diff --git a/setup/js/add_comment.cjs b/setup/js/add_comment.cjs index aaf92121..0377990f 100644 --- a/setup/js/add_comment.cjs +++ b/setup/js/add_comment.cjs @@ -825,9 +825,17 @@ async function main(config = {}) { return recordComment(comment, isDiscussion); } catch (error) { const errorMessage = getErrorMessage(error); + const normalizedErrorMessage = errorMessage.toLowerCase(); + // Known GitHub lock-related message fragments observed from REST/GraphQL comment APIs. + const lockPhrases = ["issue is locked", "conversation is locked", "resource is locked", "resource locked"]; + const hasKnownLockPhrase = lockPhrases.some(phrase => normalizedErrorMessage.includes(phrase)); // Check if this is a 404 error (discussion/issue was deleted or wrong type) - const is404 = error?.status === 404 || errorMessage.includes("404") || errorMessage.toLowerCase().includes("not found"); + const is404 = error?.status === 404 || errorMessage.includes("404") || normalizedErrorMessage.includes("not found"); + const isHttp423Locked = error?.status === 423; + const isHttp403WithLockedMessage = error?.status === 403 && normalizedErrorMessage.includes("locked"); + const isLockedByKnownMessageWithoutStatus = error?.status == null && hasKnownLockPhrase; + const isLocked = isHttp423Locked || isHttp403WithLockedMessage || isLockedByKnownMessageWithoutStatus; // If 404 and item_number was explicitly provided and we tried as issue/PR, // retry as a discussion (the user may have provided a discussion number) @@ -879,7 +887,17 @@ async function main(config = {}) { }; } - // For non-404 errors, fail as before + if (isLocked) { + // Treat locked targets as warnings - locked PRs/issues are a valid repository state + core.warning(`Target is locked, skipping comment: ${errorMessage}`); + return { + success: true, + warning: `Target is locked: ${errorMessage}`, + skipped: true, + }; + } + + // For all other errors, propagate the failure core.error(`Failed to add comment: ${errorMessage}`); return { success: false, diff --git a/setup/js/add_labels.cjs b/setup/js/add_labels.cjs index da229a83..a2c4865c 100644 --- a/setup/js/add_labels.cjs +++ b/setup/js/add_labels.cjs @@ -28,6 +28,7 @@ const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { resolveSafeOutputIssueTarget } = require("./temporary_id.cjs"); +const { attachExecutionState, fetchIssueState, normalizeLabelNames } = require("./safe_output_execution_metadata.cjs"); const { MAX_LABELS } = require("./constants.cjs"); const { createCountGatedHandler } = require("./handler_scaffold.cjs"); const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs"); @@ -178,7 +179,8 @@ const main = createCountGatedHandler({ } try { - await withRetry( + const beforeState = await fetchIssueState(githubClient, repoParts, itemNumber); + const { data: labels } = await withRetry( () => githubClient.rest.issues.addLabels({ owner: repoParts.owner, @@ -191,12 +193,20 @@ const main = createCountGatedHandler({ ); core.info(`Successfully added ${uniqueLabels.length} labels to ${contextType} #${itemNumber} in ${itemRepo}`); - return { - success: true, - number: itemNumber, - labelsAdded: uniqueLabels, - contextType, - }; + return attachExecutionState( + { + success: true, + number: itemNumber, + repo: itemRepo, + labelsAdded: uniqueLabels, + contextType, + }, + beforeState, + { + ...beforeState, + labels: normalizeLabelNames(labels), + } + ); } catch (error) { const errorMessage = getErrorMessage(error); core.error(`Failed to add labels: ${errorMessage}`); diff --git a/setup/js/add_reaction.cjs b/setup/js/add_reaction.cjs index 1c50697e..ad857c63 100644 --- a/setup/js/add_reaction.cjs +++ b/setup/js/add_reaction.cjs @@ -63,10 +63,11 @@ async function main() { /** * Resolve the REST API endpoint for non-discussion events. - * Returns null for discussion/discussion_comment/unsupported events (handled separately). + * Returns null for discussion/discussion_comment/pull_request_review/unsupported events (handled separately). * @param {string} eventName * @param {string} owner * @param {string} repo + * @param {Record} payload * @returns {string | null} */ function resolveRestEndpoint(eventName, owner, repo, payload) { @@ -108,6 +109,10 @@ function resolveRestEndpoint(eventName, owner, repo, payload) { return `/repos/${owner}/${repo}/pulls/comments/${reviewCommentId}/reactions`; } + case "pull_request_review": + // Reactions are not supported on review objects; skip silently. + return null; + default: return null; } @@ -126,6 +131,7 @@ function isRestReactionEvent(eventName) { * @param {string} eventName * @param {string} owner * @param {string} repo + * @param {Record} payload * @param {string} reaction */ async function handleGraphQLOrUnknownEvent(eventName, owner, repo, payload, reaction) { diff --git a/setup/js/add_reviewer.cjs b/setup/js/add_reviewer.cjs index 969c8c46..82cb2bda 100644 --- a/setup/js/add_reviewer.cjs +++ b/setup/js/add_reviewer.cjs @@ -9,7 +9,7 @@ */ /** - * @typedef {{ reviewers?: Array, team_reviewers?: Array, pull_request_number?: number|string }} AddReviewerMessage + * @typedef {{ reviewers?: Array, team_reviewers?: Array, pull_request_number?: number|string, repo?: string }} AddReviewerMessage */ /** @type {string} Safe output type handled by this module */ @@ -21,7 +21,9 @@ const { getPullRequestNumber } = require("./pr_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { isStagedMode, checkRequiredFilter } = require("./safe_output_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); -const { COPILOT_REVIEWER_BOT } = require("./constants.cjs"); +const { attachExecutionState, extractReviewStateFromData, fetchPullRequestReviewState } = require("./safe_output_execution_metadata.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { COPILOT_REVIEWER_BOT, COPILOT_REVIEWER_BOT_ID } = require("./constants.cjs"); /** * Main handler factory for add_reviewer @@ -32,6 +34,7 @@ async function main(config = {}) { const allowedReviewers = config.allowed ?? []; const allowedTeamReviewers = config.allowed_team_reviewers ?? []; const maxCount = config.max ?? 10; + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); const isStaged = isStagedMode(config); @@ -41,6 +44,10 @@ async function main(config = {}) { if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`); core.info(`Add reviewer configuration: max=${maxCount}`); + core.info(`Default target repo: ${defaultTargetRepo}`); + if (allowedRepos.size > 0) { + core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); + } if (allowedReviewers.length > 0) { core.info(`Allowed reviewers: ${allowedReviewers.join(", ")}`); } @@ -48,6 +55,34 @@ async function main(config = {}) { core.info(`Allowed team reviewers: ${allowedTeamReviewers.join(", ")}`); } + /** @type {string|null} Copilot reviewer bot node ID, resolved once and cached per handler instance */ + let copilotBotNodeIdCache = null; + + /** + * Resolves the Copilot reviewer bot's GraphQL node ID for the current GitHub instance. + * Uses the REST users API so the result is correct on GitHub.com and GHES alike. + * Caches the resolved ID for the lifetime of this handler to avoid redundant requests. + * Falls back to the built-in GitHub.com constant when the API call fails. + * @returns {Promise} GraphQL node ID for the Copilot reviewer bot + */ + async function resolveCopilotBotNodeId() { + if (copilotBotNodeIdCache !== null) { + return copilotBotNodeIdCache; + } + try { + const response = await githubClient.rest.users.getByUsername({ username: COPILOT_REVIEWER_BOT }); + const nodeId = response?.data?.node_id; + if (nodeId) { + copilotBotNodeIdCache = nodeId; + return nodeId; + } + } catch (err) { + core.warning(`Could not resolve Copilot reviewer bot node ID at runtime (${getErrorMessage(err)}); using built-in fallback`); + } + copilotBotNodeIdCache = COPILOT_REVIEWER_BOT_ID; + return copilotBotNodeIdCache; + } + let processedCount = 0; /** @@ -82,7 +117,17 @@ async function main(config = {}) { }; } - const repoParts = { owner: context.repo.owner, repo: context.repo.repo }; + const repoResult = resolveAndValidateRepo(message, defaultTargetRepo, allowedRepos, "pull request reviewer"); + if (!repoResult.success) { + core.warning(`Skipping add_reviewer: ${repoResult.error}`); + return { + success: false, + error: repoResult.error, + }; + } + const { repo: itemRepo, repoParts } = repoResult; + core.info(`Target repository: ${itemRepo}`); + const filterResult = await checkRequiredFilter(githubClient, repoParts, prNumber, requiredLabels, requiredTitlePrefix, HANDLER_TYPE); if (filterResult) return filterResult; @@ -100,6 +145,7 @@ async function main(config = {}) { core.info("No reviewers to add"); return { success: true, + skipped: true, prNumber, reviewersAdded: [], teamReviewersAdded: [], @@ -124,35 +170,72 @@ async function main(config = {}) { } try { + const beforeState = await fetchPullRequestReviewState(githubClient, repoParts, prNumber); + let latestPullRequest = null; + // Special handling for "copilot" reviewer - separate it from other reviewers const hasCopilot = uniqueReviewers.includes("copilot"); const otherReviewers = uniqueReviewers.filter(r => r !== "copilot"); - + const manifestReviewers = hasCopilot ? [...otherReviewers, COPILOT_REVIEWER_BOT] : otherReviewers; // Add non-copilot reviewers first if (otherReviewers.length > 0 || uniqueTeamReviewers.length > 0) { /** @type {{ owner: string, repo: string, pull_number: number, reviewers: string[], team_reviewers?: string[] }} */ const reviewerRequest = { - owner: context.repo.owner, - repo: context.repo.repo, + owner: repoParts.owner, + repo: repoParts.repo, pull_number: prNumber, reviewers: otherReviewers, }; if (uniqueTeamReviewers.length > 0) { reviewerRequest.team_reviewers = uniqueTeamReviewers; } - await githubClient.rest.pulls.requestReviewers(reviewerRequest); + const response = await githubClient.rest.pulls.requestReviewers(reviewerRequest); + latestPullRequest = response?.data || latestPullRequest; core.info(`Successfully added reviewers to PR #${prNumber}: reviewers=${JSON.stringify(otherReviewers)}, team_reviewers=${JSON.stringify(uniqueTeamReviewers)}`); } // Add copilot reviewer separately if requested if (hasCopilot) { try { - await githubClient.rest.pulls.requestReviewers({ - owner: context.repo.owner, - repo: context.repo.repo, + const pullRequestQuery = ` + query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + id + } + } + } + `; + const pullRequestResponse = await githubClient.graphql(pullRequestQuery, { + owner: repoParts.owner, + repo: repoParts.repo, + number: prNumber, + }); + const pullRequestId = pullRequestResponse?.repository?.pullRequest?.id; + if (!pullRequestId) { + throw new Error(`Could not resolve pull request node ID for ${repoParts.owner}/${repoParts.repo}#${prNumber}`); + } + + const requestReviewsMutation = ` + mutation($pullRequestId: ID!, $botIds: [ID!]!) { + requestReviews(input: { pullRequestId: $pullRequestId, botIds: $botIds, union: true }) { + pullRequest { + id + } + } + } + `; + await githubClient.graphql(requestReviewsMutation, { + pullRequestId, + botIds: [await resolveCopilotBotNodeId()], + }); + + const response = await githubClient.rest.pulls.get({ + owner: repoParts.owner, + repo: repoParts.repo, pull_number: prNumber, - reviewers: [COPILOT_REVIEWER_BOT], }); + latestPullRequest = response?.data || latestPullRequest; core.info(`Successfully added copilot as reviewer to PR #${prNumber}`); } catch (copilotError) { core.warning(`Failed to add copilot as reviewer: ${getErrorMessage(copilotError)}`); @@ -160,12 +243,31 @@ async function main(config = {}) { } } - return { - success: true, - prNumber, - reviewersAdded: uniqueReviewers, - teamReviewersAdded: uniqueTeamReviewers, - }; + const afterState = latestPullRequest + ? { + ...extractReviewStateFromData(latestPullRequest, []), + reviews: beforeState.reviews, + } + : await fetchPullRequestReviewState(githubClient, repoParts, prNumber); + + return attachExecutionState( + { + success: true, + prNumber, + number: prNumber, + repo: itemRepo, + pull_request_number: prNumber, + pull_request_url: `https://github.com/${repoParts.owner}/${repoParts.repo}/pull/${prNumber}`, + reviewersAdded: uniqueReviewers, + teamReviewersAdded: uniqueTeamReviewers, + metadata: { + requested_reviewers: manifestReviewers, + requested_team_reviewers: uniqueTeamReviewers, + }, + }, + beforeState, + afterState + ); } catch (error) { const errorMessage = getErrorMessage(error); core.error(`Failed to add reviewers: ${errorMessage}`); diff --git a/setup/js/check_workflow_timestamp_api.cjs b/setup/js/check_workflow_timestamp_api.cjs index 07d0db5f..5ba0d3f7 100644 --- a/setup/js/check_workflow_timestamp_api.cjs +++ b/setup/js/check_workflow_timestamp_api.cjs @@ -18,7 +18,7 @@ const fs = require("fs"); const path = require("path"); const { getErrorMessage } = require("./error_helpers.cjs"); -const { extractHashFromLockFile, computeFrontmatterHash, createGitHubFileReader } = require("./frontmatter_hash_pure.cjs"); +const { extractHashFromLockFile, extractBodyHashFromLockFile, computeFrontmatterHash, computeBodyHash, createGitHubFileReader } = require("./frontmatter_hash_pure.cjs"); const { getFileContent } = require("./github_api_helpers.cjs"); const { ERR_CONFIG } = require("./error_codes.cjs"); @@ -34,12 +34,15 @@ async function main() { return; } + // Determine if full stale check mode is enabled (checks both frontmatter and body hashes). + const fullCheckMode = process.env.GH_AW_STALE_CHECK_FULL === "true"; + // Construct file paths const workflowBasename = workflowFile.replace(".lock.yml", ""); const workflowMdPath = `.github/workflows/${workflowBasename}.md`; const lockFilePath = `.github/workflows/${workflowFile}`; - core.info(`Checking for stale lock file using frontmatter hash:`); + core.info(`Checking for stale lock file using ${fullCheckMode ? "frontmatter + body" : "frontmatter"} hash:`); core.info(` Source: ${workflowMdPath}`); core.info(` Lock file: ${lockFilePath}`); @@ -179,6 +182,34 @@ async function main() { // The activation job's "Checkout .github and .agents folders" step always runs before // this check and places the workflow source files in $GITHUB_WORKSPACE, so the local // files are always available at this point. + + /** + * Compares the stored body hash in the lock file content against the body hash + * recomputed from the workflow source. Returns true if they match (or if the lock + * file predates body hash support), false if they differ. + * @param {string} lockFileContent - Content of the .lock.yml file + * @param {string} mdPath - Path to the .md file to compute the body hash from + * @param {Object} [options] - Options forwarded to computeBodyHash + * @param {Function} [options.fileReader] - Custom file reader (defaults to local filesystem) + * @param {string} [label] - Optional label appended to log lines for context (e.g. "local filesystem fallback") + * @returns {Promise} true if match or no body hash present, false if mismatch + */ + async function compareBodyHashes(lockFileContent, mdPath, { fileReader } = {}, label) { + const suffix = label ? ` (${label})` : ""; + const storedBodyHash = extractBodyHashFromLockFile(lockFileContent); + if (!storedBodyHash) { + core.info(`No body hash found in lock file; skipping body hash check${suffix} (lock file may predate body hash support)`); + return true; + } + const recomputedBodyHash = await computeBodyHash(mdPath, { fileReader }); + const match = storedBodyHash === recomputedBodyHash; + core.info(`Body hash comparison${suffix}:`); + core.info(` Lock file body hash: ${storedBodyHash}`); + core.info(` Recomputed body hash: ${recomputedBodyHash}`); + core.info(` Status: ${match ? "✅ Body hashes match" : "⚠️ Body hashes differ"}`); + return match; + } + async function compareFrontmatterHashesFromLocalFiles() { const workspace = process.env.GITHUB_WORKSPACE; if (!workspace) { @@ -227,13 +258,18 @@ async function main() { // computeFrontmatterHash uses the local filesystem reader by default const recomputedHash = await computeFrontmatterHash(localMdFilePath); - const match = storedHash === recomputedHash; + let match = storedHash === recomputedHash; core.info(`Frontmatter hash comparison (local filesystem fallback):`); core.info(` Lock file hash: ${storedHash}`); core.info(` Recomputed hash: ${recomputedHash}`); core.info(` Status: ${match ? "✅ Hashes match" : "⚠️ Hashes differ"}`); + // When full check mode is enabled, also compare body hashes. + if (match && fullCheckMode) { + match = await compareBodyHashes(localLockContent, localMdFilePath, undefined, "local filesystem fallback"); + } + return { match, storedHash, recomputedHash }; } catch (error) { core.info(`Could not compute frontmatter hash from local files: ${getErrorMessage(error)}`); @@ -280,7 +316,7 @@ async function main() { const fileReader = createGitHubFileReader(github, owner, repo, ref); const recomputedHash = await computeFrontmatterHash(workflowMdPath, { fileReader }); - const match = storedHash === recomputedHash; + let match = storedHash === recomputedHash; // Log hash comparison core.info(`Frontmatter hash comparison:`); @@ -288,6 +324,11 @@ async function main() { core.info(` Recomputed hash: ${recomputedHash}`); core.info(` Status: ${match ? "✅ Hashes match" : "⚠️ Hashes differ"}`); + // When full check mode is enabled, also compare body hashes. + if (match && fullCheckMode) { + match = await compareBodyHashes(lockFileContent, workflowMdPath, { fileReader }); + } + return { result: { match, storedHash, recomputedHash }, crossRepoAuthFailure: null }; } catch (error) { const errorMessage = getErrorMessage(error); diff --git a/setup/js/commit_sha_helpers.cjs b/setup/js/commit_sha_helpers.cjs new file mode 100644 index 00000000..f3f34c3c --- /dev/null +++ b/setup/js/commit_sha_helpers.cjs @@ -0,0 +1,20 @@ +/** + * @fileoverview Shared helpers for commit SHA normalization. + */ + +const GIT_COMMIT_SHA_PATTERN = /^[0-9a-fA-F]{7,40}$/; + +/** + * Normalize and validate a git commit SHA. + * + * @param {unknown} value + * @returns {string} + */ +function normalizeCommitSHA(value) { + const normalized = String(value ?? "").trim(); + return GIT_COMMIT_SHA_PATTERN.test(normalized) ? normalized : ""; +} + +module.exports = { + normalizeCommitSHA, +}; diff --git a/setup/js/constants.cjs b/setup/js/constants.cjs index 21f0222c..bed082de 100644 --- a/setup/js/constants.cjs +++ b/setup/js/constants.cjs @@ -31,6 +31,15 @@ const TMP_GH_AW_PATH = "/tmp/gh-aw"; */ const COPILOT_REVIEWER_BOT = "copilot-pull-request-reviewer[bot]"; +/** + * GitHub.com GraphQL node ID for the Copilot pull request reviewer bot. + * Used as a fallback when the node ID cannot be resolved at runtime (e.g. network error). + * For GHES and other GitHub instances the node ID differs; prefer runtime resolution via + * the REST users API ({@link https://docs.github.com/en/rest/users/users#get-a-user}). + * @type {string} + */ +const COPILOT_REVIEWER_BOT_ID = "BOT_kgDOCnlnWA"; + // --------------------------------------------------------------------------- // Documentation URLs // --------------------------------------------------------------------------- @@ -112,10 +121,20 @@ const GITHUB_RATE_LIMITS_JSONL_PATH = `${TMP_GH_AW_PATH}/github_rate_limits.json */ const DETECTION_LOG_FILENAME = "detection.log"; +/** + * Filename of the structured threat detection result written by the Codex engine + * via `--output-last-message`. When present, the parser reads this file directly + * instead of scraping the detection log, eliminating false parse_error warnings + * caused by noisy SSE/tracing output in the log stream. + * @type {string} + */ +const DETECTION_RESULT_FILENAME = "detection_result.json"; + module.exports = { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH, COPILOT_REVIEWER_BOT, + COPILOT_REVIEWER_BOT_ID, FAQ_CREATE_PR_PERMISSIONS_URL, MAX_LABELS, MAX_ASSIGNEES, @@ -126,4 +145,5 @@ module.exports = { OTEL_JSONL_PATH, GITHUB_RATE_LIMITS_JSONL_PATH, DETECTION_LOG_FILENAME, + DETECTION_RESULT_FILENAME, }; diff --git a/setup/js/copilot_harness.cjs b/setup/js/copilot_harness.cjs index d515ef6a..f98aae1f 100644 --- a/setup/js/copilot_harness.cjs +++ b/setup/js/copilot_harness.cjs @@ -150,6 +150,74 @@ function isModelNotSupportedError(output) { return MODEL_NOT_SUPPORTED_PATTERN.test(output); } +/** + * Determine whether the current run phase is threat detection. + * @param {string | undefined | null} phase + * @returns {boolean} + */ +function isDetectionPhase(phase) { + return ( + String(phase || "") + .trim() + .toLowerCase() === "detection" + ); +} + +/** + * Check whether a model is present in AWF /reflect endpoint data. + * @param {string} model + * @param {unknown} reflectData + * @returns {boolean} + */ +function isModelAvailableInReflectData(model, reflectData) { + const normalizedModel = typeof model === "string" ? model.trim() : ""; + if (!normalizedModel) return false; + if (!reflectData || typeof reflectData !== "object") return false; + + // TypeScript needs explicit 'in' check or cast before property access on narrowed object type + const endpoints = "endpoints" in reflectData && Array.isArray(reflectData.endpoints) ? reflectData.endpoints : []; + for (const endpoint of endpoints) { + if (!endpoint || endpoint.configured !== true || !Array.isArray(endpoint.models)) { + continue; + } + if (endpoint.models.includes(normalizedModel)) { + return true; + } + } + return false; +} + +/** + * Load saved AWF /reflect data and check whether a model is present. + * @param {string} model + * @param {{ + * reflectPath?: string, + * readFileSync?: (path: string, encoding: string) => string, + * logger?: (msg: string) => void, + * }} [options] + * @returns {boolean} + */ +function isModelAvailableInReflectFile(model, options) { + const normalizedModel = typeof model === "string" ? model.trim() : ""; + const reflectPath = (options && options.reflectPath) || AWF_REFLECT_OUTPUT_PATH; + const readFile = (options && options.readFileSync) || fs.readFileSync; + const logger = (options && options.logger) || log; + if (!normalizedModel) { + logger("awf-reflect: model availability check skipped (model is empty)"); + return false; + } + + try { + const raw = readFile(reflectPath, "utf8"); + const reflectData = JSON.parse(raw); + return isModelAvailableInReflectData(normalizedModel, reflectData); + } catch (error) { + const err = /** @type {Error} */ error; + logger(`awf-reflect: unable to read model availability from ${reflectPath}: ${err.message}`); + return false; + } +} + /** * Determines if the collected output contains a "No authentication information found" error. * This means no auth token (COPILOT_GITHUB_TOKEN, GH_TOKEN, or GITHUB_TOKEN) is available @@ -470,6 +538,7 @@ async function main() { let scheduledExit2Retries = 0; let scheduledExit2RetryAttempted = false; let useContinueOnRetry = false; + let modelNotSupportedReflectRetryAttempted = false; // Once set to true, --continue is never re-enabled for the remainder of this run. // This prevents a broken --continue recovery from resurrecting --continue on the next attempt. let continueDisabledPermanently = false; @@ -563,6 +632,19 @@ async function main() { // Model-not-supported errors are persistent — retrying will not help. if (isModelNotSupported) { + if (!modelNotSupportedReflectRetryAttempted && attempt < MAX_RETRIES && isDetectionPhase(process.env.GH_AW_PHASE) && process.env.AWF_REFLECT_ENABLED === "1") { + const configuredModel = process.env.COPILOT_MODEL || ""; + modelNotSupportedReflectRetryAttempted = true; + log(`attempt ${attempt + 1}: model not supported during detection — refreshing awf-reflect to rule out startup registry race`); + await fetchAWFReflect({ logger: log }); + if (isModelAvailableInReflectFile(configuredModel, { logger: log })) { + useContinueOnRetry = false; + continueDisabledPermanently = true; + log(`attempt ${attempt + 1}: refreshed awf-reflect now includes model '${configuredModel}' — retrying once as fresh run`); + continue; + } + log(`attempt ${attempt + 1}: refreshed awf-reflect does not include model '${configuredModel || "(none)"}' — treating as non-retryable`); + } log(`attempt ${attempt + 1}: model not supported — not retrying (the requested model is unavailable for this subscription tier; specify a supported model in the workflow frontmatter)`); break; } @@ -662,6 +744,9 @@ if (typeof module !== "undefined" && module.exports) { extractDeniedCommands, fetchAWFReflect, fetchModelsFromUrl, + isDetectionPhase, + isModelAvailableInReflectData, + isModelAvailableInReflectFile, countPermissionDeniedIssues, detectCopilotErrors, hasNumerousPermissionDeniedIssues, diff --git a/setup/js/create_forecast_issue.cjs b/setup/js/create_forecast_issue.cjs index fd95aeca..8d528d9d 100644 --- a/setup/js/create_forecast_issue.cjs +++ b/setup/js/create_forecast_issue.cjs @@ -2,9 +2,13 @@ /// const fs = require("node:fs"); +const { getPromptPath, renderTemplateFromFile } = require("./messages_core.cjs"); const FORECAST_REPORT_PATH = "./.cache/gh-aw/forecast/report.json"; +const FORECAST_ERROR_PATH = "./.cache/gh-aw/forecast/error.json"; const FORECAST_ISSUE_TITLE = "[aw] workflow forecast report"; +const FORECAST_ERROR_ISSUE_TITLE = "[aw] workflow forecast report (error)"; +const FORECAST_ISSUE_TEMPLATE = "forecast_issue.md"; /** * @param {unknown} value @@ -27,104 +31,137 @@ function formatET(value) { } /** - * @param {Record} report - * @param {{owner: string, repo: string, serverUrl: string, runID?: string, generatedAtISO?: string}} options + * @param {Record|null} report + * @param {{owner: string, repo: string, serverUrl: string, runID?: string, generatedAtISO?: string, outcome?: string, errorMessage?: string}} options * @returns {string} */ function buildForecastIssueBody(report, options) { - const workflows = Array.isArray(report.workflows) ? report.workflows : []; + const workflows = Array.isArray(report?.workflows) ? report.workflows : []; const rows = workflows.map(workflow => { const p50 = workflow?.monte_carlo?.p50_projected_effective_tokens ?? workflow?.projected_effective_tokens ?? 0; return [escapeCell(workflow.workflow_id), workflow.sampled_runs ?? 0, Number(p50)]; }); - const allProjectedZero = rows.every(([, , p50]) => Number(p50) === 0); + const allProjectedZero = rows.length > 0 && rows.every(([, , p50]) => Number(p50) === 0); const zeroProjectedWithSamples = rows.filter(([, sampledRuns, p50]) => Number(sampledRuns) > 0 && Number(p50) === 0).length; const zeroWorkflowWord = zeroProjectedWithSamples === 1 ? "workflow" : "workflows"; const zeroWorkflowVerb = zeroProjectedWithSamples === 1 ? "has" : "have"; - const reportTable = ["| Workflow | Sampled runs | Forecast ET (P50) |", "| --- | ---: | ---: |", ...rows.map(([workflowID, sampledRuns, p50]) => `| ${workflowID} | ${sampledRuns} | ${formatET(p50)} |`)].join("\n"); + const reportTable = + rows.length > 0 + ? ["| Workflow | Sampled runs | Forecast ET (P50) |", "| --- | ---: | ---: |", ...rows.map(([workflowID, sampledRuns, p50]) => `| ${workflowID} | ${sampledRuns} | ${formatET(p50)} |`)].join("\n") + : "_No forecast rows were produced._"; const repoSlug = `${options.owner}/${options.repo}`; - const period = report.period || "month"; + const period = report?.period || "month"; const runID = options.runID || ""; const runURL = runID ? `${options.serverUrl}/${repoSlug}/actions/runs/${runID}` : ""; - - return [ - "### Agentic workflow forecast report", - "", - `Repository: ${repoSlug}`, - `Generated at: ${options.generatedAtISO || new Date().toISOString()}`, - `Period: ${period}`, - "", - reportTable, - "", - ...(allProjectedZero - ? [ - "> [!NOTE]", - "> All projected ET values are 0 even after cache warm-up. This usually means cached run summaries do not include token usage for sampled runs.", - "> Verify gh aw logs fetched recent runs and that run_summary.json files include token usage.", - "", - ] - : []), - ...(zeroProjectedWithSamples > 0 + const outcome = (options.outcome || "success").toLowerCase(); + + const allProjectedZeroNote = allProjectedZero + ? [ + "> [!NOTE]", + "> All projected ET values are 0 even after cache warm-up. This usually means cached run summaries do not include token usage for sampled runs.", + "> Verify gh aw logs fetched recent runs and that run_summary.json files include token usage.", + "", + ].join("\n") + : ""; + const zeroProjectedTip = + zeroProjectedWithSamples > 0 ? [ "> [!TIP]", `> ${zeroProjectedWithSamples} ${zeroWorkflowWord} ${zeroWorkflowVerb} sampled runs but forecast ET is 0. This usually indicates missing token usage in cached run summaries for sampled runs.`, "> Increase the warm-up scope with `gh aw logs --start-date -30d --count ` if this persists.", "", - ] - : []), - ...(runURL ? [`_Forecast source run: [#${runID}](${runURL})._`] : []), - ].join("\n"); + ].join("\n") + : ""; + const sourceRunLine = runURL ? `_Forecast source run: [#${runID}](${runURL})._` : ""; + const errorSection = outcome === "success" ? "" : ["> [!WARNING]", `> Forecast outcome: ${outcome}.`, `> ${options.errorMessage || "Forecast computation did not complete successfully."}`].join("\n"); + + return renderTemplateFromFile(getPromptPath(FORECAST_ISSUE_TEMPLATE), { + repository: repoSlug, + generated_at: options.generatedAtISO || new Date().toISOString(), + period, + report_table: reportTable, + all_projected_zero_note: allProjectedZeroNote, + zero_projected_tip: zeroProjectedTip, + error_section: errorSection, + source_run_line: sourceRunLine, + }).trim(); } /** * @returns {Promise} */ async function main() { - if (!fs.existsSync(FORECAST_REPORT_PATH)) { - core.warning(`Forecast report JSON not found at ${FORECAST_REPORT_PATH}; skipping issue creation.`); - return; + /** @type {Record|null} */ + let report = null; + let outcome = "success"; + let errorMessage = ""; + + if (fs.existsSync(FORECAST_REPORT_PATH)) { + let reportBody = ""; + try { + reportBody = fs.readFileSync(FORECAST_REPORT_PATH, "utf8").trim(); + } catch (error) { + outcome = "error"; + errorMessage = `Failed to read forecast report JSON at ${FORECAST_REPORT_PATH}: ${error.message}`; + core.warning(errorMessage); + } + + if (reportBody) { + try { + report = JSON.parse(reportBody); + } catch (error) { + outcome = "error"; + errorMessage = `Failed to parse forecast report JSON at ${FORECAST_REPORT_PATH}: ${error.message}`; + core.warning(errorMessage); + } + } else if (!errorMessage) { + outcome = "error"; + errorMessage = `Forecast report JSON is empty at ${FORECAST_REPORT_PATH}.`; + core.warning(errorMessage); + } + } else { + outcome = "error"; + errorMessage = `Forecast report JSON not found at ${FORECAST_REPORT_PATH}.`; + core.warning(errorMessage); } - let reportBody = ""; - try { - reportBody = fs.readFileSync(FORECAST_REPORT_PATH, "utf8").trim(); - } catch (error) { - core.warning(`Failed to read forecast report JSON at ${FORECAST_REPORT_PATH}: ${error.message}`); - return; + if (fs.existsSync(FORECAST_ERROR_PATH)) { + try { + const errorPayload = JSON.parse(fs.readFileSync(FORECAST_ERROR_PATH, "utf8")); + outcome = String(errorPayload?.outcome || outcome).toLowerCase(); + if (typeof errorPayload?.message === "string" && errorPayload.message.trim() !== "") { + errorMessage = errorPayload.message.trim(); + } + } catch (error) { + core.warning(`Failed to parse forecast error JSON at ${FORECAST_ERROR_PATH}: ${error.message}`); + } } - if (!reportBody) { - core.warning(`Forecast report JSON is empty at ${FORECAST_REPORT_PATH}; skipping issue creation.`); - return; + if (process.env.FORECAST_STEP_OUTCOME && outcome === "success") { + const stepOutcome = process.env.FORECAST_STEP_OUTCOME.toLowerCase(); + if (stepOutcome !== "success") { + outcome = stepOutcome; + errorMessage = errorMessage || `Forecast step finished with outcome: ${stepOutcome}.`; + } } - /** @type {Record} */ - let report = {}; - try { - report = JSON.parse(reportBody); - } catch (error) { - core.warning(`Failed to parse forecast report JSON at ${FORECAST_REPORT_PATH}: ${error.message}`); - return; - } - - if (!Array.isArray(report.workflows) || report.workflows.length === 0) { - core.warning("Forecast report contains no workflows; skipping issue creation."); - return; - } + const isErrorOutcome = outcome !== "success"; const body = buildForecastIssueBody(report, { owner: context.repo.owner, repo: context.repo.repo, serverUrl: context.serverUrl, runID: process.env.GITHUB_RUN_ID || "", + outcome, + errorMessage, }); const createdIssue = await github.rest.issues.create({ owner: context.repo.owner, repo: context.repo.repo, - title: FORECAST_ISSUE_TITLE, + title: isErrorOutcome ? FORECAST_ERROR_ISSUE_TITLE : FORECAST_ISSUE_TITLE, body, labels: ["agentic-workflows"], }); @@ -138,5 +175,8 @@ module.exports = { formatET, escapeCell, FORECAST_REPORT_PATH, + FORECAST_ERROR_PATH, FORECAST_ISSUE_TITLE, + FORECAST_ERROR_ISSUE_TITLE, + FORECAST_ISSUE_TEMPLATE, }; diff --git a/setup/js/create_pull_request.cjs b/setup/js/create_pull_request.cjs index 66f88044..313840ce 100644 --- a/setup/js/create_pull_request.cjs +++ b/setup/js/create_pull_request.cjs @@ -30,6 +30,7 @@ const { checkFileProtection } = require("./manifest_file_helpers.cjs"); const { renderTemplateFromFile, renderFilesList, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs"); const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL } = require("./constants.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); +const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs"); const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs"); const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs"); const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, linearizeRangeAsCommit } = require("./git_helpers.cjs"); @@ -204,7 +205,7 @@ async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBran // commit objects, then retry with the original bundle ref. // This handles the race where main advanced between agent-time and safe_outputs-time: // the bundle's base commit may not be reachable from a fetch-depth:1 shallow clone - // even after --unshallow (e.g. when the commit is on a ref not in the fetch refspec). + // (e.g. when the commit is on a ref not in the fetch refspec). const prerequisiteCommits = extractBundlePrerequisiteCommits(initialFetchErrorOutput); if (prerequisiteCommits.length > 0) { core.warning(`Bundle fetch with ${bundleBranchRef} failed due to ${prerequisiteCommits.length} missing prerequisite commit(s); fetching prerequisites from origin and retrying`); @@ -867,6 +868,11 @@ async function main(config = {}) { // Always require patch content for policy enforcement, even when bundle transport // is used for apply-time commit transport. const hasBundleFile = !!(bundleFilePath && fs.existsSync(bundleFilePath)); + const applyTransport = hasBundleFile ? "bundle" : "patch"; + core.info(`Apply transport mode: ${applyTransport} (bundle file present: ${hasBundleFile})`); + if (bundleFilePath && !hasBundleFile) { + core.warning(`Bundle file path was provided but file is not present on disk: ${bundleFilePath}; falling back to patch transport`); + } const hasPatchFile = !!(patchFilePath && fs.existsSync(patchFilePath)); if (!hasPatchFile) { // If allow-empty is enabled, we can proceed without a patch file @@ -1501,9 +1507,32 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo } // Handle branch creation/checkout - core.info(`Branch should not exist locally, creating new branch from base: ${branchName}`); - await exec.exec(`git checkout -b ${branchName}`); - core.info(`Created new branch from base: ${branchName}`); + let branchBaseRef = baseBranch; + const recordedBaseCommit = normalizeCommitSHA(pullRequestItem.base_commit); + if (recordedBaseCommit) { + core.info(`Patch route base_commit resolved: ${recordedBaseCommit}`); + core.info(`Using base_commit from safe output entry for patch apply: ${recordedBaseCommit}`); + try { + try { + await exec.exec("git", ["fetch", "origin", recordedBaseCommit, "--depth=1"]); + } catch (fetchError) { + core.info(`Note: could not fetch base commit ${recordedBaseCommit} explicitly (${fetchError instanceof Error ? fetchError.message : String(fetchError)}); will verify local availability next`); + } + await exec.exec("git", ["cat-file", "-e", recordedBaseCommit]); + const ancestryCheck = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", recordedBaseCommit, `origin/${baseBranch}`], { ignoreReturnCode: true }); + if (ancestryCheck.exitCode !== 0) { + throw new Error(`recorded base_commit ${recordedBaseCommit} is not an ancestor of origin/${baseBranch}; falling back to ${baseBranch}`); + } + branchBaseRef = recordedBaseCommit; + } catch (baseCommitError) { + core.warning(`Recorded base_commit ${recordedBaseCommit} is not available in this checkout (${baseCommitError instanceof Error ? baseCommitError.message : String(baseCommitError)}); falling back to ${baseBranch}`); + } + } else if (String(pullRequestItem.base_commit ?? "").trim()) { + core.warning(`Ignoring invalid base_commit value for patch apply: ${String(pullRequestItem.base_commit).trim()}`); + } + core.info(`Branch should not exist locally, creating new branch from base: ${branchName} (${branchBaseRef})`); + await exec.exec("git", ["checkout", "-b", branchName, branchBaseRef]); + core.info(`Created new branch from base: ${branchName} (${branchBaseRef})`); // Apply the patch using git CLI (skip if empty) if (!isEmpty) { @@ -1579,7 +1608,7 @@ gh pr create --title '${title}' --base ${baseBranch} --head ${branchName} --repo // Use the base commit recorded at patch generation time. // The From header in format-patch output contains the agent's new commit SHA // which does not exist in this checkout, so we cannot derive the base from it. - const originalBaseCommit = pullRequestItem.base_commit; + const originalBaseCommit = normalizeCommitSHA(pullRequestItem.base_commit); if (!originalBaseCommit) { core.warning("No base_commit recorded in safe output entry - fallback not possible"); } else { diff --git a/setup/js/effective_tokens.cjs b/setup/js/effective_tokens.cjs index cd465451..6bec72a9 100644 --- a/setup/js/effective_tokens.cjs +++ b/setup/js/effective_tokens.cjs @@ -2,6 +2,8 @@ /// const fs = require("fs"); +const path = require("path"); +const { TMP_GH_AW_PATH } = require("./constants.cjs"); /** * Effective Tokens (ET) computation module. @@ -21,13 +23,26 @@ const fs = require("fs"); * Reasoning (R): w_reason = 4.0 * Cache Write (W): w_cache_write = 1.0 (implementation extension) * - * The per-model multiplier (m) is loaded from the GH_AW_MODEL_MULTIPLIERS - * environment variable, which contains the JSON content of model_multipliers.json. + * The per-model multiplier (m) is loaded from model_multipliers.json data. + * Lookup order: + * 1. /tmp/gh-aw/model_multipliers.json (merged in activation job) + * 2. GH_AW_MODEL_MULTIPLIERS env var (compatibility fallback) + * 3. Built-in model_multipliers.json in the setup action folder * Falls back to m=1.0 (reference baseline) for unknown models. */ /** @type {{ token_class_weights: { input: number, cached_input: number, output: number, reasoning: number, cache_write: number }, multipliers: Record } | null | undefined} */ let _parsedMultipliers = undefined; // undefined = not yet parsed; null = parsed but unavailable +const DEFAULT_MERGED_MULTIPLIERS_PATH = `${TMP_GH_AW_PATH}/model_multipliers.json`; +const BUILTIN_MULTIPLIERS_PATH = path.join(__dirname, "model_multipliers.json"); + +/** + * @returns {string} + */ +function getMergedMultipliersPath() { + const override = process.env.GH_AW_MERGED_MODEL_MULTIPLIERS_PATH; + return override && override.trim() ? override : DEFAULT_MERGED_MULTIPLIERS_PATH; +} /** * Default token class weights from the ET specification (Section 4.2). @@ -44,7 +59,7 @@ function defaultTokenClassWeights() { } /** - * Loads and parses the model multipliers from the GH_AW_MODEL_MULTIPLIERS env var. + * Loads and parses model multipliers from file-based sources or env fallback. * Caches the result after first parse (including null when unavailable). Returns null if not available or invalid. * @returns {{ token_class_weights: { input: number, cached_input: number, output: number, reasoning: number, cache_write: number }, multipliers: Record } | null | undefined} */ @@ -53,23 +68,64 @@ function getMultipliersData() { return _parsedMultipliers; } - const raw = process.env.GH_AW_MODEL_MULTIPLIERS; - if (!raw || !raw.trim()) { - _parsedMultipliers = null; + const parsedFromMergedFile = parseMultipliersFile(getMergedMultipliersPath()); + if (parsedFromMergedFile) { + _parsedMultipliers = parsedFromMergedFile; + return _parsedMultipliers; + } + + if (Object.prototype.hasOwnProperty.call(process.env, "GH_AW_MODEL_MULTIPLIERS")) { + const raw = process.env.GH_AW_MODEL_MULTIPLIERS; + if (raw && raw.trim()) { + const parsedFromEnv = parseMultipliersJSON(raw); + if (parsedFromEnv) { + _parsedMultipliers = parsedFromEnv; + return _parsedMultipliers; + } + } + } + + const parsedFromBuiltinFile = parseMultipliersFile(BUILTIN_MULTIPLIERS_PATH); + if (parsedFromBuiltinFile) { + _parsedMultipliers = parsedFromBuiltinFile; + return _parsedMultipliers; + } + + _parsedMultipliers = null; + return null; +} + +/** + * @param {string} filePath + * @returns {{ token_class_weights: { input: number, cached_input: number, output: number, reasoning: number, cache_write: number }, multipliers: Record } | null} + */ +function parseMultipliersFile(filePath) { + try { + if (!fs.existsSync(filePath)) { + return null; + } + const raw = fs.readFileSync(filePath, "utf8"); + return parseMultipliersJSON(raw); + } catch { return null; } +} +/** + * @param {string} raw + * @returns {{ token_class_weights: { input: number, cached_input: number, output: number, reasoning: number, cache_write: number }, multipliers: Record } | null} + */ +function parseMultipliersJSON(raw) { try { const parsed = JSON.parse(raw); - if (!parsed || typeof parsed !== "object") { - _parsedMultipliers = null; + if (!isPlainObject(parsed)) { return null; } const defaults = defaultTokenClassWeights(); - const weights = { ...defaults, ...(parsed.token_class_weights || {}) }; + const rawWeights = isPlainObject(parsed.token_class_weights) ? parsed.token_class_weights : {}; + const weights = { ...defaults, ...rawWeights }; - // Ensure missing or invalid weights fall back to defaults, but preserve explicit 0 overrides for (const key of Object.keys(defaults)) { const value = weights[key]; if (value == null || !Number.isFinite(value)) { @@ -79,21 +135,30 @@ function getMultipliersData() { /** @type {Record} */ const multipliers = {}; - for (const [model, mult] of Object.entries(parsed.multipliers || {})) { - multipliers[model.toLowerCase()] = Number(mult); + if (isPlainObject(parsed.multipliers)) { + for (const [model, mult] of Object.entries(parsed.multipliers)) { + multipliers[model.toLowerCase()] = Number(mult); + } } - _parsedMultipliers = { token_class_weights: weights, multipliers }; - return _parsedMultipliers; + return { token_class_weights: weights, multipliers }; } catch { - _parsedMultipliers = null; return null; } } +/** + * @param {unknown} value + * @returns {value is Record} + */ +function isPlainObject(value) { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + /** * Returns the token class weights in use. - * Uses values from GH_AW_MODEL_MULTIPLIERS if available, otherwise defaults. + * Uses values from merged/built-in model_multipliers.json when available, + * otherwise falls back to defaults. * @returns {{ input: number, cached_input: number, output: number, reasoning: number, cache_write: number }} */ function getTokenClassWeights() { @@ -105,7 +170,7 @@ function getTokenClassWeights() { * Returns the per-model cost multiplier for the given model name. * * Lookup order: - * 1. Exact case-insensitive match in GH_AW_MODEL_MULTIPLIERS + * 1. Exact case-insensitive match in loaded model multipliers * 2. Longest prefix match (e.g. "claude-sonnet-4.6-preview" → "claude-sonnet-4.6") * 3. Default: 1.0 (unknown model treated as reference baseline) * diff --git a/setup/js/emit_outcome_spans.cjs b/setup/js/emit_outcome_spans.cjs index 928f3072..4b2ed4a7 100644 --- a/setup/js/emit_outcome_spans.cjs +++ b/setup/js/emit_outcome_spans.cjs @@ -39,6 +39,9 @@ const { const AW_INFO_PATH = "/tmp/gh-aw/aw_info.json"; const EVALUATIONS_PATH = "/tmp/gh-aw/outcome-evaluations.jsonl"; const SUMMARY_PATH = "/tmp/gh-aw/outcome-summary.json"; +const OTLP_STATUS_UNSET = 0; +const OTLP_STATUS_OK = 1; +const OTLP_STATUS_ERROR = 2; /** * Read a JSONL file, returning an array of parsed objects. @@ -136,6 +139,11 @@ async function main() { for (const eval_ of evaluations) { const type = typeof eval_.type === "string" ? eval_.type : ""; const result = typeof eval_.result === "string" ? eval_.result : "unknown"; + // Fall back to the legacy result field so older JSONL artifacts still render + // useful spans while newer artifacts carry explicit normalized fields. + const outcomeStatus = typeof eval_.outcome_status === "string" ? eval_.outcome_status : result; + const evidenceStrength = typeof eval_.evidence_strength === "string" ? eval_.evidence_strength : "weak"; + const signal = typeof eval_.signal === "string" ? eval_.signal : ""; const detail = typeof eval_.detail === "string" ? eval_.detail : ""; const workflow = typeof eval_.workflow === "string" ? eval_.workflow : ""; const sourceRunId = typeof eval_.run_id === "number" ? eval_.run_id : 0; @@ -159,6 +167,8 @@ async function main() { buildAttr("gh-aw.exporter.name", "outcome-collector"), buildAttr("gh-aw.outcome.type", type), buildAttr("gh-aw.outcome.result", result), + buildAttr("gh-aw.outcome.outcome_status", outcomeStatus), + buildAttr("gh-aw.outcome.evidence_strength", evidenceStrength), buildAttr("gh-aw.outcome.workflow", workflow), buildAttr("gh-aw.outcome.run_id", sourceRunId), buildAttr("gh-aw.outcome.repo", repo), @@ -166,6 +176,7 @@ async function main() { if (url) attributes.push(buildAttr("gh-aw.outcome.url", url)); if (detail) attributes.push(buildAttr("gh-aw.outcome.detail", detail)); + if (signal) attributes.push(buildAttr("gh-aw.outcome.signal", signal)); if (timestamp) attributes.push(buildAttr("gh-aw.outcome.created_at", timestamp)); if (event) attributes.push(buildAttr("gh-aw.outcome.event", event)); if (resolutionSec !== null) attributes.push(buildAttr("gh-aw.outcome.resolution_sec", resolutionSec)); @@ -180,8 +191,8 @@ async function main() { if (comments !== null) attributes.push(buildAttr("gh-aw.outcome.comments", comments)); if (zeroTouch) attributes.push(buildAttr("gh-aw.outcome.zero_touch", true)); - // Map result to OTLP status: accepted=OK, rejected=ERROR, noop=UNSET, pending/ignored=UNSET - const statusCode = result === "rejected" ? 2 : result === "accepted" ? 1 : 0; + // Map normalized outcome_status to OTLP status: accepted=OK, rejected=ERROR, all others=UNSET + const statusCode = outcomeStatus === "rejected" ? OTLP_STATUS_ERROR : outcomeStatus === "accepted" ? OTLP_STATUS_OK : OTLP_STATUS_UNSET; itemSpans.push( buildOTLPSpan({ @@ -213,6 +224,10 @@ async function main() { buildAttr("gh-aw.outcome.ignored", getSummaryNumber("ignored", 0)), buildAttr("gh-aw.outcome.pending", getSummaryNumber("pending", 0)), buildAttr("gh-aw.outcome.noop", getSummaryNumber("noop", 0)), + buildAttr("gh-aw.outcome.accepted_strong", getSummaryNumber("accepted_strong", 0)), + buildAttr("gh-aw.outcome.accepted_medium", getSummaryNumber("accepted_medium", 0)), + buildAttr("gh-aw.outcome.accepted_weak", getSummaryNumber("accepted_weak", 0)), + buildAttr("gh-aw.outcome.fallback_exists_only_count", getSummaryNumber("fallback_exists_only_count", 0)), buildAttr("gh-aw.outcome.acceptance_rate", getSummaryNumber("acceptance_rate", 0)), buildAttr("gh-aw.outcome.waste_rate", getSummaryNumber("waste_rate", 0)), buildAttr("gh-aw.outcome.noop_rate", getSummaryNumber("noop_rate", 0)), diff --git a/setup/js/evaluate_outcomes.cjs b/setup/js/evaluate_outcomes.cjs index 3fd30d87..52be0fc3 100644 --- a/setup/js/evaluate_outcomes.cjs +++ b/setup/js/evaluate_outcomes.cjs @@ -26,6 +26,7 @@ const fs = require("fs"); const path = require("path"); +const crypto = require("crypto"); const { execFileSync } = require("child_process"); // --------------------------------------------------------------------------- @@ -41,6 +42,29 @@ const SUMMARY_PATH = "/tmp/gh-aw/outcome-summary.json"; // Noop types that are tracked but not counted as actionable // --------------------------------------------------------------------------- const NOOP_TYPES = new Set(["noop", "missing_tool", "missing_data", "report_incomplete"]); +const CLOSING_LABEL_KEYWORDS = ["not planned", "not_planned", "wontfix", "won't fix", "duplicate", "invalid", "declined", "rejected"]; +const CLOSING_COMMENT_KEYWORDS = ["not planned", "won't fix", "wontfix", "duplicate", "invalid", "declined", "rejected", "closing as", "closed as", "closing this"]; + +const DEFAULT_ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC = 60 * 60; +const DEFAULT_LABEL_RETENTION_WINDOW_SEC = 24 * 60 * 60; + +const POSITIVE_REACTIONS = ["+1", "heart", "hooray", "rocket"]; +const NEGATIVE_REACTIONS = ["-1", "confused"]; + +/** + * Read a positive integer from env with fallback. + * @param {string} key + * @param {number} fallback + * @returns {number} + */ +function getEnvPositiveIntOrDefault(key, fallback) { + const raw = process.env[key]; + const parsed = Number.parseInt(raw || "", 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; +} + +const ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC = getEnvPositiveIntOrDefault("OUTCOME_ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC", DEFAULT_ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC); +const LABEL_RETENTION_WINDOW_SEC = getEnvPositiveIntOrDefault("OUTCOME_LABEL_RETENTION_WINDOW_SEC", DEFAULT_LABEL_RETENTION_WINDOW_SEC); // --------------------------------------------------------------------------- // Helpers @@ -90,100 +114,1053 @@ function readJSON(filePath, fallback) { } } -/** - * Read a JSONL file, returning an array of parsed objects. - * @param {string} filePath - * @returns {object[]} - */ -function readJSONL(filePath) { - try { - return fs - .readFileSync(filePath, "utf8") - .split("\n") - .filter(l => l.trim()) - .map(l => { - try { - return JSON.parse(l); - } catch { - return null; +/** + * Read a JSONL file, returning an array of parsed objects. + * @param {string} filePath + * @returns {object[]} + */ +function readJSONL(filePath) { + try { + return fs + .readFileSync(filePath, "utf8") + .split("\n") + .filter(l => l.trim()) + .map(l => { + try { + return JSON.parse(l); + } catch { + return null; + } + }) + .filter(Boolean); + } catch { + return []; + } +} + +/** + * Atomically write JSON to a file using a tmp+rename swap. + * @param {string} filePath + * @param {any} data + */ +function writeJSONAtomic(filePath, data) { + const tmp = filePath + ".tmp"; + fs.writeFileSync(tmp, JSON.stringify(data, null, 2) + "\n"); + fs.renameSync(tmp, filePath); +} + +/** + * Parse an ISO-8601 timestamp to epoch seconds. Returns null on failure. + * @param {string} ts + * @returns {number | null} + */ +function isoToEpoch(ts) { + if (!ts) return null; + const ms = Date.parse(ts); + return Number.isFinite(ms) ? Math.floor(ms / 1000) : null; +} + +/** + * Compute seconds between two ISO timestamps. Returns null if either is invalid. + * @param {string} from + * @param {string} to + * @returns {number | null} + */ +function secondsBetween(from, to) { + const a = isoToEpoch(from); + const b = isoToEpoch(to); + if (a === null || b === null) return null; + return b - a; +} + +// --------------------------------------------------------------------------- +// Item evaluation +// --------------------------------------------------------------------------- + +/** + * @typedef {object} EvalResult + * @property {string} result + * @property {"accepted"|"rejected"|"pending"|"ignored"|"skipped"|"unknown"} outcome_status + * @property {"strong"|"medium"|"weak"|"none"} evidence_strength + * @property {string} signal + * @property {string} detail + * @property {number | null} resolution_sec + * @property {number | null} pending_age_sec + * @property {number | null} review_comments + * @property {number | null} changed_files + * @property {number | null} additions + * @property {number | null} deletions + * @property {number | null} reactions_total + * @property {number | null} reactions_positive + * @property {number | null} reactions_negative + * @property {number | null} comments + * @property {boolean} zero_touch + */ + +/** + * @typedef {object} EvaluateDeps + * @property {(endpoint: string) => any} [ghAPI] + * @property {number} [nowMs] + */ + +/** + * Convert issue/PR reaction summary into aggregate counts. + * @param {any} reactions + * @returns {{total: number|null, positive: number|null, negative: number|null}} + */ +function summarizeReactions(reactions) { + if (!reactions || typeof reactions !== "object") { + return { total: null, positive: null, negative: null }; + } + const positive = POSITIVE_REACTIONS.reduce((sum, key) => sum + Number(reactions[key] || 0), 0); + const negative = NEGATIVE_REACTIONS.reduce((sum, key) => sum + Number(reactions[key] || 0), 0); + const total = reactions.total_count != null ? reactions.total_count : positive + negative + (reactions.laugh || 0) + (reactions.eyes || 0); + return { total, positive, negative }; +} + +/** + * @param {unknown} value + * @returns {string[]} + */ +function normalizeLabels(value) { + if (!Array.isArray(value)) return []; + return value.map(l => String(l || "").trim()).filter(Boolean); +} + +/** + * @param {string} url + * @returns {number|null} + */ +function parseIssueNumberFromURL(url) { + const match = String(url || "").match(/\/(?:issues|pull)\/(\d+)/); + if (!match) return null; + const num = Number.parseInt(match[1], 10); + return Number.isInteger(num) && num > 0 ? num : null; +} + +/** + * @param {string} url + * @returns {string} + */ +function parseCommentIDFromURL(url) { + const text = String(url || ""); + const issueCommentMatch = text.match(/#issuecomment-(\d+)/); + if (issueCommentMatch) return issueCommentMatch[1]; + const pathMatch = text.match(/\/comments\/(\d+)/); + return pathMatch ? pathMatch[1] : ""; +} + +/** + * @param {any} issue + * @returns {boolean} + */ +function hasIssueReactions(issue) { + const summary = summarizeReactions(issue?.reactions); + return typeof summary.total === "number" && summary.total > 0; +} + +/** + * Evaluate `create_issue`. + * @param {object} item + * @param {string} itemRepo + * @param {string} timestamp + * @param {EvalResult} out + * @param {(endpoint: string) => any} apiGet + * @param {number} nowMs + * @returns {EvalResult} + */ +function evaluateCreateIssue(item, itemRepo, timestamp, out, apiGet, nowMs) { + const num = parseIssueNumberFromURL(item.url || ""); + if (!num) { + out.result = "unknown"; + out.detail = "unknown: issue number not found"; + setPendingAge(out, timestamp, nowMs); + return out; + } + const issue = apiGet(`repos/${itemRepo}/issues/${num}`); + if (!issue || !issue.state) { + out.result = "unknown"; + out.detail = "unknown: issue api error"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + out.comments = typeof issue.comments === "number" ? issue.comments : null; + const reactionSummary = summarizeReactions(issue.reactions); + out.reactions_total = reactionSummary.total; + out.reactions_positive = reactionSummary.positive; + out.reactions_negative = reactionSummary.negative; + + const authorLogin = issue.user && typeof issue.user.login === "string" ? issue.user.login : ""; + const comments = apiGet(`repos/${itemRepo}/issues/${num}/comments`); + const hasNonAuthorComment = Array.isArray(comments) && comments.some(c => c && c.user && typeof c.user.login === "string" && c.user.login !== authorLogin && c.user.login !== ""); + + const timeline = apiGet(`repos/${itemRepo}/issues/${num}/timeline`); + const timelineEvents = Array.isArray(timeline) ? timeline : []; + let hasMergedPRReference = false; + let hasCommitReference = false; + let hasClosingActionReference = false; + let closeActor = ""; + for (const event of timelineEvents) { + if (!event || typeof event !== "object") continue; + const eventType = typeof event.event === "string" ? event.event : ""; + if (eventType === "closed") { + hasClosingActionReference = true; + const actorLogin = event.actor && typeof event.actor.login === "string" ? event.actor.login : ""; + if (actorLogin) closeActor = actorLogin; + } + if (eventType === "referenced" && event.commit_id) { + hasCommitReference = true; + } + if (eventType !== "cross-referenced") continue; + const sourceIssue = event.source && event.source.issue; + const prNumber = sourceIssue && typeof sourceIssue.number === "number" ? sourceIssue.number : null; + if (!prNumber) continue; + const pr = apiGet(`repos/${itemRepo}/pulls/${prNumber}`); + if (pr && pr.merged === true) { + hasMergedPRReference = true; + } + } + + if (issue.state === "open" && (hasMergedPRReference || hasCommitReference)) { + out.result = "accepted"; + out.detail = "accepted:strong"; + return out; + } + + if (issue.state === "open" && (hasNonAuthorComment || hasIssueReactions(issue))) { + out.result = "accepted"; + out.detail = "accepted:medium"; + return out; + } + + if (issue.state === "closed" && issue.created_at && issue.closed_at) { + out.resolution_sec = secondsBetween(issue.created_at, issue.closed_at); + const closedByDifferentUser = closeActor !== "" && closeActor !== authorLogin; + if (typeof out.resolution_sec === "number" && out.resolution_sec <= ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC && closedByDifferentUser) { + out.result = "rejected"; + out.detail = "rejected:strong"; + return out; + } + } + + if (issue.state === "closed") { + const hasActivity = (typeof issue.comments === "number" && issue.comments > 0) || hasIssueReactions(issue) || hasMergedPRReference || hasCommitReference; + if (!hasActivity) { + out.result = "rejected"; + out.detail = "rejected:medium"; + return out; + } + out.result = "unknown"; + out.detail = "unknown: closed with activity"; + return out; + } + + if (issue.state === "open") { + out.result = "pending"; + out.detail = "pending: open with no engagement"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + out.result = "unknown"; + out.detail = "unknown: unsupported issue state"; + setPendingAge(out, timestamp, nowMs); + return out; +} + +/** + * Evaluate `add_comment`. + * @param {object} item + * @param {string} itemRepo + * @param {string} timestamp + * @param {EvalResult} out + * @param {(endpoint: string) => any} apiGet + * @param {number} nowMs + * @returns {EvalResult} + */ +function evaluateAddComment(item, itemRepo, timestamp, out, apiGet, nowMs) { + const commentID = parseCommentIDFromURL(item.url || ""); + const issueNum = parseIssueNumberFromURL(item.url || ""); + if (!commentID || !issueNum) { + out.result = "unknown"; + out.detail = "unknown: missing comment or issue id"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + const comment = apiGet(`repos/${itemRepo}/issues/comments/${commentID}`); + if (!comment || !comment.id) { + out.result = "unknown"; + out.detail = "unknown: failed to fetch comment from API (may be deleted or inaccessible)"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + const commentAuthor = comment.user && typeof comment.user.login === "string" ? comment.user.login : ""; + const commentCreatedAt = typeof comment.created_at === "string" ? comment.created_at : ""; + const reactionSummary = summarizeReactions(comment.reactions); + out.reactions_total = reactionSummary.total; + out.reactions_positive = reactionSummary.positive; + out.reactions_negative = reactionSummary.negative; + + const issueComments = apiGet(`repos/${itemRepo}/issues/${issueNum}/comments`); + const commentURL = String(item.url || ""); + let hasReply = false; + let hasQuote = false; + let threadActedOn = false; + if (Array.isArray(issueComments)) { + for (const c of issueComments) { + if (!c || typeof c !== "object") continue; + if (typeof c.created_at === "string" && commentCreatedAt && c.created_at > commentCreatedAt) { + threadActedOn = true; + } + const body = typeof c.body === "string" ? c.body : ""; + const cAuthor = c.user && typeof c.user.login === "string" ? c.user.login : ""; + if (cAuthor && cAuthor !== commentAuthor && typeof c.created_at === "string" && commentCreatedAt && c.created_at > commentCreatedAt) { + hasReply = true; + } + if (body.includes(`#issuecomment-${commentID}`) || (commentURL && body.includes(commentURL))) { + hasQuote = true; + } + } + } + + if ((typeof reactionSummary.total === "number" && reactionSummary.total > 0) || hasReply || hasQuote) { + out.result = "accepted"; + out.detail = "accepted:strong"; + return out; + } + + if (threadActedOn) { + out.result = "accepted"; + out.detail = "accepted:medium"; + return out; + } + + out.result = "pending"; + out.detail = "pending: no follow-up"; + setPendingAge(out, timestamp, nowMs); + return out; +} + +/** + * Evaluate `add_labels`. + * @param {object} item + * @param {string} itemRepo + * @param {string} timestamp + * @param {EvalResult} out + * @param {(endpoint: string) => any} apiGet + * @param {number} nowMs + * @returns {EvalResult} + */ +function evaluateAddLabels(item, itemRepo, timestamp, out, apiGet, nowMs) { + const num = parseIssueNumberFromURL(item.url || ""); + if (!num) { + out.result = "unknown"; + out.detail = "unknown: issue number not found"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + const hasLabelsBefore = Object.hasOwn(item, "labelsBefore") && Array.isArray(item.labelsBefore); + const labelsBefore = hasLabelsBefore ? normalizeLabels(item.labelsBefore) : []; + const labelsAdded = normalizeLabels(item.labelsAdded); + const fallbackLabels = normalizeLabels(item.labels); + const effectiveLabelsAdded = labelsAdded.length > 0 ? labelsAdded : fallbackLabels; + + if (!hasLabelsBefore) { + out.result = "unknown"; + out.detail = "unknown: missing persisted label before state"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + if (effectiveLabelsAdded.length === 0) { + out.result = "unknown"; + out.detail = "unknown: no labels added"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + const labels = apiGet(`repos/${itemRepo}/issues/${num}/labels`); + if (!Array.isArray(labels)) { + out.result = "unknown"; + out.detail = "unknown: labels api error"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + const currentLabels = new Set(labels.map(l => (l && typeof l.name === "string" ? l.name : "")).filter(Boolean)); + const trackedAdded = effectiveLabelsAdded.filter(l => !labelsBefore.includes(l)); + const removed = trackedAdded.filter(l => !currentLabels.has(l)); + + const nowEpoch = Math.floor(nowMs / 1000); + const createdEpoch = isoToEpoch(timestamp || ""); + const elapsedSec = createdEpoch === null ? null : nowEpoch - createdEpoch; + if (elapsedSec === null || elapsedSec < LABEL_RETENTION_WINDOW_SEC) { + out.result = "pending"; + out.detail = "pending: retention window not elapsed"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + if (removed.length === 0) { + out.result = "accepted"; + out.detail = "accepted:strong"; + return out; + } + + const issue = apiGet(`repos/${itemRepo}/issues/${num}`); + const issueAuthor = issue && issue.user && typeof issue.user.login === "string" ? issue.user.login : ""; + const events = apiGet(`repos/${itemRepo}/issues/${num}/events`); + const eventList = Array.isArray(events) ? events : []; + const removedByNonAuthor = eventList.some(event => { + if (!event || event.event !== "unlabeled") return false; + const removedLabel = event.label && typeof event.label.name === "string" ? event.label.name : ""; + if (!removed.includes(removedLabel)) return false; + const actor = event.actor && typeof event.actor.login === "string" ? event.actor.login : ""; + return actor !== "" && actor !== issueAuthor; + }); + + if (removedByNonAuthor) { + out.result = "rejected"; + out.detail = "rejected:strong"; + return out; + } + + out.result = "unknown"; + out.detail = "unknown: labels removed with ambiguous actor"; + return out; +} + +/** + * Normalize legacy result/detail pairs into the shared outcome model. + * @param {string} result + * @param {string} detail + * @returns {{ outcome_status: "accepted"|"rejected"|"pending"|"ignored"|"skipped"|"unknown", evidence_strength: "strong"|"medium"|"weak"|"none", signal: string }} + */ +function normalizeOutcome(result, detail) { + const normalizedDetail = String(detail || "") + .toLowerCase() + .trim(); + + if (result === "noop") { + return { outcome_status: "skipped", evidence_strength: "none", signal: "noop" }; + } + if (normalizedDetail === "object still exists") { + return { outcome_status: "unknown", evidence_strength: "weak", signal: "target_exists_only" }; + } + if (normalizedDetail === "review approved") { + return { outcome_status: "accepted", evidence_strength: "strong", signal: "review_approved" }; + } + if (normalizedDetail === "review submitted") { + return { outcome_status: "accepted", evidence_strength: "medium", signal: "review_submitted" }; + } + if (normalizedDetail === "review request removed") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "review_request_removed" }; + } + if (normalizedDetail === "review dismissed") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "review_dismissed" }; + } + if (normalizedDetail === "changes requested addressed and merged") { + return { outcome_status: "accepted", evidence_strength: "medium", signal: "changes_requested_addressed" }; + } + if (normalizedDetail === "closed without merge after review") { + return { outcome_status: "rejected", evidence_strength: "medium", signal: "closed_without_merge_after_review" }; + } + if (normalizedDetail === "latest review awaiting outcome") { + return { outcome_status: "pending", evidence_strength: "medium", signal: "latest_review_pending" }; + } + if (normalizedDetail === "awaiting review") { + return { outcome_status: "pending", evidence_strength: "medium", signal: "awaiting_review" }; + } + if (normalizedDetail === "update retained and merged") { + return { outcome_status: "accepted", evidence_strength: "strong", signal: "state_retained_and_merged" }; + } + if (normalizedDetail === "update retained") { + return { outcome_status: "accepted", evidence_strength: "medium", signal: "state_retained" }; + } + if (normalizedDetail === "update reverted") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "state_reverted" }; + } + if (normalizedDetail === "update replaced") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "state_replaced" }; + } + if (normalizedDetail === "missing execution state") { + return { outcome_status: "unknown", evidence_strength: "none", signal: "missing_execution_state" }; + } + if (normalizedDetail === "no persisted state delta") { + return { outcome_status: "unknown", evidence_strength: "none", signal: "no_state_delta" }; + } + if (result === "accepted" && normalizedDetail.startsWith("merged")) { + return { outcome_status: "accepted", evidence_strength: "strong", signal: "merged" }; + } + if (result === "rejected" && normalizedDetail === "closed") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "closed" }; + } + if (result === "pending" && normalizedDetail === "open") { + return { outcome_status: "pending", evidence_strength: "medium", signal: "open" }; + } + switch (result) { + case "accepted": + return { outcome_status: "accepted", evidence_strength: "medium", signal: "acted_on" }; + case "rejected": + return { outcome_status: "rejected", evidence_strength: "medium", signal: "rejected" }; + case "ignored": + return { outcome_status: "ignored", evidence_strength: "medium", signal: "ignored" }; + case "pending": + return { outcome_status: "pending", evidence_strength: "medium", signal: "pending" }; + default: + return { outcome_status: "unknown", evidence_strength: "weak", signal: "unknown" }; + } +} + +/** + * @param {object} item + * @returns {number | null} + */ +function getItemNumber(item) { + if (typeof item.number === "number" && Number.isFinite(item.number)) return item.number; + const url = item.url || ""; + const issueMatch = url.match(/\/(?:issues|pull)\/(\d+)/); + if (issueMatch) return Number(issueMatch[1]); + return null; +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @returns {string} + */ +function getItemRepo(item, defaultRepo) { + if (item.repo) return item.repo; + const url = item.url || ""; + const match = url.match(/github\.com\/([^/]+\/[^/]+)/); + return match ? match[1] : defaultRepo; +} + +/** + * @param {object} item + * @param {string} key + * @returns {string[]} + */ +function getMetadataStringArray(item, key) { + const raw = item?.metadata?.[key]; + if (!Array.isArray(raw)) return []; + return raw.map(value => String(value || "").trim()).filter(Boolean); +} + +/** + * @param {object} item + * @param {string} key + * @returns {number | null} + */ +function getMetadataNumber(item, key) { + const raw = item?.metadata?.[key]; + if (typeof raw === "number" && Number.isFinite(raw)) return raw; + if (typeof raw === "string" && raw.trim() !== "") { + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : null; + } + return null; +} + +/** + * @param {string} key + * @param {any} value + * @returns {any} + */ +function normalizeStateValue(key, value) { + if (key === "labels" || key === "assignees") { + if (!Array.isArray(value)) return []; + return value + .map(entry => String(entry || "").trim()) + .filter(Boolean) + .sort(); + } + if (key === "draft") { + return value === true; + } + if (typeof value === "string") { + return value.trim(); + } + return value ?? ""; +} + +function hashOutcomeBody(body) { + const normalized = String(body || "") + .replace(/\r\n/g, "\n") + .split("\n") + .map(line => line.replace(/[ \t]+$/g, "")) + .join("\n") + .trim(); + return crypto.createHash("sha256").update(normalized, "utf8").digest("hex"); +} + +/** + * @param {string} key + * @param {any} left + * @param {any} right + * @returns {boolean} + */ +function stateValuesEqual(key, left, right) { + const normalizedLeft = normalizeStateValue(key, left); + const normalizedRight = normalizeStateValue(key, right); + return JSON.stringify(normalizedLeft) === JSON.stringify(normalizedRight); +} + +/** + * @param {Record | null | undefined} beforeState + * @param {Record | null | undefined} afterState + * @param {Record} currentState + * @param {string[]} fields + * @returns {{changed: string[], retained: string[], reverted: string[], replaced: string[]}} + */ +function compareRetainedState(beforeState, afterState, currentState, fields) { + const changed = []; + const retained = []; + const reverted = []; + const replaced = []; + + for (const field of fields) { + if (!afterState || !(field in afterState)) continue; + const beforeValue = beforeState ? beforeState[field] : undefined; + const afterValue = afterState[field]; + if (stateValuesEqual(field, beforeValue, afterValue)) continue; + changed.push(field); + const currentValue = currentState[field]; + if (stateValuesEqual(field, currentValue, afterValue)) { + retained.push(field); + continue; + } + if (beforeState && field in beforeState && stateValuesEqual(field, currentValue, beforeValue)) { + reverted.push(field); + continue; + } + replaced.push(field); + } + + return { changed, retained, reverted, replaced }; +} + +function extractIssueUpdateState(issue) { + return { + title: typeof issue?.title === "string" ? issue.title : "", + body_hash: hashOutcomeBody(issue?.body), + state: typeof issue?.state === "string" ? issue.state : "", + labels: Array.isArray(issue?.labels) + ? issue.labels + .map(label => { + if (typeof label === "string") return label; + if (label && typeof label.name === "string") return label.name; + return ""; + }) + .filter(Boolean) + : [], + assignees: Array.isArray(issue?.assignees) + ? issue.assignees + .map(assignee => { + if (typeof assignee === "string") return assignee; + if (assignee && typeof assignee.login === "string") return assignee.login; + return ""; + }) + .filter(Boolean) + : [], + }; +} + +function extractPullRequestUpdateState(pullRequest) { + return { + title: typeof pullRequest?.title === "string" ? pullRequest.title : "", + body_hash: hashOutcomeBody(pullRequest?.body), + state: typeof pullRequest?.state === "string" ? pullRequest.state : "", + base: typeof pullRequest?.base?.ref === "string" ? pullRequest.base.ref : "", + draft: pullRequest?.draft === true, + head_sha: typeof pullRequest?.head?.sha === "string" ? pullRequest.head.sha : "", + }; +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @param {{fields: string[], loadCurrent: (repo: string, number: number) => { currentState: Record, merged?: boolean } | null}} options + * @returns {EvalResult} + */ +function evaluateRetainedUpdate(item, defaultRepo, api, options) { + const repo = getItemRepo(item, defaultRepo); + const number = getItemNumber(item); + /** @type {EvalResult} */ + const out = { + result: "unknown", + outcome_status: "unknown", + evidence_strength: "none", + signal: "missing_execution_state", + detail: "", + resolution_sec: null, + pending_age_sec: null, + review_comments: null, + changed_files: null, + additions: null, + deletions: null, + reactions_total: null, + reactions_positive: null, + reactions_negative: null, + comments: null, + zero_touch: false, + }; + + if (!repo || !number) { + out.detail = "missing execution state"; + return out; + } + + if (!item.before_state || !item.after_state) { + out.detail = "missing execution state"; + return out; + } + + const loaded = options.loadCurrent(repo, number); + if (!loaded || !loaded.currentState) { + out.signal = "unknown"; + out.detail = "api error"; + out.evidence_strength = "none"; + return out; + } + + const comparison = compareRetainedState(item.before_state, item.after_state, loaded.currentState, options.fields); + if (comparison.changed.length === 0) { + out.detail = "no persisted state delta"; + out.signal = "no_state_delta"; + return out; + } + + if (comparison.retained.length === comparison.changed.length) { + out.result = "accepted"; + if (loaded.merged) { + out.outcome_status = "accepted"; + out.evidence_strength = "strong"; + out.signal = "state_retained_and_merged"; + out.detail = "update retained and merged"; + return out; + } + out.outcome_status = "accepted"; + out.evidence_strength = "medium"; + out.signal = "state_retained"; + out.detail = "update retained"; + return out; + } + + out.result = "rejected"; + out.outcome_status = "rejected"; + out.evidence_strength = "strong"; + if (comparison.reverted.length === comparison.changed.length) { + out.signal = "state_reverted"; + out.detail = "update reverted"; + return out; + } + out.signal = "state_replaced"; + out.detail = "update replaced"; + return out; +} + +/** + * @param {string | undefined} timestamp + * @param {string | undefined} threshold + * @returns {boolean} + */ +function isOnOrAfter(timestamp, threshold) { + if (!timestamp) return false; + if (!threshold) return true; + const a = Date.parse(timestamp); + const b = Date.parse(threshold); + if (!Number.isFinite(a) || !Number.isFinite(b)) return false; + return a >= b; +} + +/** + * @param {any} review + * @returns {boolean} + */ +function isSubmittedReview(review) { + const state = String(review?.state || "").toUpperCase(); + return Boolean(review?.submitted_at) && state !== "" && state !== "PENDING"; +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @returns {EvalResult} + */ +function evaluateAddReviewer(item, defaultRepo, api = ghAPI) { + const repo = getItemRepo(item, defaultRepo); + const number = getItemNumber(item); + const timestamp = item.timestamp || ""; + /** @type {EvalResult} */ + const out = { + result: "unknown", + outcome_status: "unknown", + evidence_strength: "weak", + signal: "unknown", + detail: "", + resolution_sec: null, + pending_age_sec: null, + review_comments: null, + changed_files: null, + additions: null, + deletions: null, + reactions_total: null, + reactions_positive: null, + reactions_negative: null, + comments: null, + zero_touch: false, + }; + + if (!repo || !number) { + out.detail = "missing review request reference"; + return out; + } + + const requestedReviewers = new Set(getMetadataStringArray(item, "requested_reviewers").map(login => login.toLowerCase())); + const requestedTeams = new Set(getMetadataStringArray(item, "requested_team_reviewers").map(team => team.toLowerCase())); + const reviews = api(`repos/${repo}/pulls/${number}/reviews`); + const requested = api(`repos/${repo}/pulls/${number}/requested_reviewers`); + + if (!Array.isArray(reviews) || !requested) { + out.detail = "api error"; + setPendingAge(out, timestamp); + return out; + } + + const latestReviewByRequestedReviewer = new Map(); + for (const review of reviews) { + const state = String(review?.state || "").toUpperCase(); + const submittedAt = review?.submitted_at; + if (!state || state === "PENDING" || !submittedAt || !isOnOrAfter(submittedAt, timestamp)) continue; + const login = String(review?.user?.login || "").toLowerCase(); + if (!requestedReviewers.has(login)) continue; + const previous = latestReviewByRequestedReviewer.get(login); + if (!previous || isOnOrAfter(submittedAt, previous?.submitted_at)) { + latestReviewByRequestedReviewer.set(login, review); + } + } + const relevantReviews = Array.from(latestReviewByRequestedReviewer.values()); + + if (relevantReviews.some(review => String(review?.state || "").toUpperCase() === "APPROVED")) { + out.result = "accepted"; + out.detail = "review approved"; + return out; + } + + if (relevantReviews.length > 0) { + out.result = "accepted"; + out.detail = "review submitted"; + return out; + } + + // We cannot cheaply verify team membership for each reviewer from this endpoint, + // so any submitted post-request review counts as medium-evidence team activity. + const anyReviewAfterRequest = reviews.some(review => isSubmittedReview(review) && isOnOrAfter(review?.submitted_at, timestamp)); + if (requestedTeams.size > 0 && anyReviewAfterRequest) { + out.result = "accepted"; + out.detail = "review submitted"; + return out; + } + + const pendingUsers = new Set((requested.users || []).map(user => String(user?.login || "").toLowerCase())); + const pendingTeams = new Set((requested.teams || []).map(team => String(team?.slug || team?.name || "").toLowerCase())); + const stillPending = Array.from(requestedReviewers).some(login => pendingUsers.has(login)) || Array.from(requestedTeams).some(team => pendingTeams.has(team)); + + if (stillPending) { + out.result = "pending"; + out.detail = "awaiting review"; + setPendingAge(out, timestamp); + return out; + } + + if (requestedReviewers.size > 0 || requestedTeams.size > 0) { + out.result = "rejected"; + out.detail = "review request removed"; + return out; + } + + out.detail = "unknown review request state"; + return out; +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @returns {EvalResult} + */ +function evaluateUpdateIssue(item, defaultRepo, api = ghAPI) { + return evaluateRetainedUpdate(item, defaultRepo, api, { + fields: ["title", "body_hash", "state", "labels", "assignees"], + loadCurrent: (repo, number) => { + const issue = api(`repos/${repo}/issues/${number}`); + if (!issue || !issue.state) return null; + return { currentState: extractIssueUpdateState(issue) }; + }, + }); +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @returns {EvalResult} + */ +function evaluateUpdatePullRequest(item, defaultRepo, api = ghAPI) { + return evaluateRetainedUpdate(item, defaultRepo, api, { + fields: ["title", "body_hash", "state", "base", "draft", "head_sha"], + loadCurrent: (repo, number) => { + const pullRequest = api(`repos/${repo}/pulls/${number}`); + if (!pullRequest || !pullRequest.state) return null; + return { + currentState: extractPullRequestUpdateState(pullRequest), + merged: pullRequest.merged === true, + }; + }, + }); +} + +/** + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @returns {EvalResult} + */ +function evaluateSubmitPullRequestReview(item, defaultRepo, api = ghAPI) { + const repo = getItemRepo(item, defaultRepo); + const number = getItemNumber(item); + const timestamp = item.timestamp || ""; + /** @type {EvalResult} */ + const out = { + result: "unknown", + outcome_status: "unknown", + evidence_strength: "weak", + signal: "unknown", + detail: "", + resolution_sec: null, + pending_age_sec: null, + review_comments: null, + changed_files: null, + additions: null, + deletions: null, + reactions_total: null, + reactions_positive: null, + reactions_negative: null, + comments: null, + zero_touch: false, + }; + + if (!repo || !number) { + out.detail = "missing review reference"; + return out; + } + + const reviewId = getMetadataNumber(item, "review_id"); + const pr = api(`repos/${repo}/pulls/${number}`); + const reviews = api(`repos/${repo}/pulls/${number}/reviews`); + + if (!pr || !Array.isArray(reviews) || !pr.state) { + out.detail = "api error"; + setPendingAge(out, timestamp); + return out; + } + + const submittedReviews = reviews.filter(candidate => isSubmittedReview(candidate)); + const review = submittedReviews.find(candidate => Number(candidate?.id) === reviewId) || submittedReviews.filter(candidate => isOnOrAfter(candidate?.submitted_at, timestamp)).slice(-1)[0]; + + if (!review) { + out.detail = "review not found"; + return out; + } + + const reviewState = String(review?.state || item?.metadata?.review_state || "").toUpperCase(); + const reviewSubmittedAt = review?.submitted_at || timestamp; + const latestReview = submittedReviews.sort((a, b) => Date.parse(a.submitted_at) - Date.parse(b.submitted_at)).slice(-1)[0]; + + out.review_comments = typeof pr.review_comments === "number" ? pr.review_comments : null; + out.changed_files = typeof pr.changed_files === "number" ? pr.changed_files : null; + out.additions = typeof pr.additions === "number" ? pr.additions : null; + out.deletions = typeof pr.deletions === "number" ? pr.deletions : null; + out.comments = typeof pr.comments === "number" ? pr.comments : null; + + if (reviewState === "DISMISSED") { + out.result = "rejected"; + out.detail = "review dismissed"; + return out; + } + + if (pr.merged === true) { + if (reviewState === "APPROVED") { + out.result = "accepted"; + out.detail = "review approved"; + if (pr.created_at && pr.merged_at) { + out.resolution_sec = secondsBetween(pr.created_at, pr.merged_at); + } + return out; + } + + if (reviewState === "CHANGES_REQUESTED") { + const commits = api(`repos/${repo}/pulls/${number}/commits`); + const hasPushAfterReview = Array.isArray(commits) ? commits.some(commit => isOnOrAfter(commit?.commit?.committer?.date || commit?.commit?.author?.date, reviewSubmittedAt)) : false; + if (hasPushAfterReview) { + out.result = "accepted"; + out.detail = "changes requested addressed and merged"; + if (pr.created_at && pr.merged_at) { + out.resolution_sec = secondsBetween(pr.created_at, pr.merged_at); } - }) - .filter(Boolean); - } catch { - return []; + return out; + } + } } -} -/** - * Atomically write JSON to a file using a tmp+rename swap. - * @param {string} filePath - * @param {any} data - */ -function writeJSONAtomic(filePath, data) { - const tmp = filePath + ".tmp"; - fs.writeFileSync(tmp, JSON.stringify(data, null, 2) + "\n"); - fs.renameSync(tmp, filePath); -} + if (pr.state === "closed" && pr.merged !== true) { + out.result = "rejected"; + out.detail = "closed without merge after review"; + if (pr.created_at && pr.closed_at) { + out.resolution_sec = secondsBetween(pr.created_at, pr.closed_at); + } + return out; + } -/** - * Parse an ISO-8601 timestamp to epoch seconds. Returns null on failure. - * @param {string} ts - * @returns {number | null} - */ -function isoToEpoch(ts) { - if (!ts) return null; - const ms = Date.parse(ts); - return Number.isFinite(ms) ? Math.floor(ms / 1000) : null; -} + if (pr.state === "open" && latestReview && Number(latestReview.id) === Number(review.id)) { + out.result = "pending"; + out.detail = "latest review awaiting outcome"; + setPendingAge(out, timestamp); + return out; + } -/** - * Compute seconds between two ISO timestamps. Returns null if either is invalid. - * @param {string} from - * @param {string} to - * @returns {number | null} - */ -function secondsBetween(from, to) { - const a = isoToEpoch(from); - const b = isoToEpoch(to); - if (a === null || b === null) return null; - return b - a; + out.detail = "unknown review lifecycle"; + return out; } -// --------------------------------------------------------------------------- -// Item evaluation -// --------------------------------------------------------------------------- - -/** - * @typedef {object} EvalResult - * @property {string} result - * @property {string} detail - * @property {number | null} resolution_sec - * @property {number | null} pending_age_sec - * @property {number | null} review_comments - * @property {number | null} changed_files - * @property {number | null} additions - * @property {number | null} deletions - * @property {number | null} reactions_total - * @property {number | null} reactions_positive - * @property {number | null} reactions_negative - * @property {number | null} comments - * @property {boolean} zero_touch - */ - /** * Evaluate a single safe-output item against the GitHub API. * @param {object} item * @param {string} defaultRepo + * @param {((endpoint: string) => any) | EvaluateDeps} [apiOrOptions] * @returns {EvalResult} */ -function evaluateItem(item, defaultRepo) { +function evaluateItem(item, defaultRepo, apiOrOptions) { const url = item.url || ""; const itemRepo = item.repo || defaultRepo; const timestamp = item.timestamp || ""; + const type = item.type || ""; + const ghAPIFn = typeof apiOrOptions === "function" ? apiOrOptions : typeof apiOrOptions?.ghAPI === "function" ? apiOrOptions.ghAPI : ghAPI; + const nowMs = typeof apiOrOptions === "object" && typeof apiOrOptions?.nowMs === "number" ? apiOrOptions.nowMs : Date.now(); /** @type {EvalResult} */ const out = { result: "pending", + outcome_status: "pending", + evidence_strength: "medium", + signal: "pending", detail: "", resolution_sec: null, pending_age_sec: null, @@ -198,20 +1175,55 @@ function evaluateItem(item, defaultRepo) { zero_touch: false, }; + if (type === "create_pull_request") { + return evaluateCreatePullRequestOutcome(item, itemRepo, out, ghAPIFn); + } + if (type === "push_to_pull_request_branch") { + return evaluatePushToPullRequestBranchOutcome(item, itemRepo, out, ghAPIFn); + } + if (type === "update_issue") { + return evaluateUpdateIssue(item, defaultRepo, ghAPIFn); + } + if (type === "update_pull_request") { + return evaluateUpdatePullRequest(item, defaultRepo, ghAPIFn); + } + if (!url) { + if (item.type === "add_reviewer") { + return evaluateAddReviewer(item, defaultRepo, ghAPIFn); + } + if (item.type === "submit_pull_request_review") { + return evaluateSubmitPullRequestReview(item, defaultRepo, ghAPIFn); + } out.detail = "no url"; - setPendingAge(out, timestamp); + setPendingAge(out, timestamp, nowMs); return out; } + if (type === "add_reviewer") { + return evaluateAddReviewer(item, defaultRepo, ghAPIFn); + } + if (type === "submit_pull_request_review") { + return evaluateSubmitPullRequestReview(item, defaultRepo, ghAPIFn); + } + if (type === "create_issue") { + return evaluateCreateIssue(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } + if (type === "add_comment") { + return evaluateAddComment(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } + if (type === "add_labels") { + return evaluateAddLabels(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } + // Issues / issue-comments const issueMatch = url.match(/\/(?:issues|pull)\/(\d+)/); if (/\/issues\/\d+|\/issuecomment-/.test(url) && issueMatch) { const num = issueMatch[1]; - const data = ghAPI(`repos/${itemRepo}/issues/${num}`); + const data = ghAPIFn(`repos/${itemRepo}/issues/${num}`); if (!data || !data.state) { out.detail = "api error"; - setPendingAge(out, timestamp); + setPendingAge(out, timestamp, nowMs); return out; } out.result = "accepted"; @@ -220,12 +1232,10 @@ function evaluateItem(item, defaultRepo) { // Reactions on issues if (data.reactions && typeof data.reactions === "object") { - const r = data.reactions; - const positive = (r["+1"] || 0) + (r.heart || 0) + (r.hooray || 0) + (r.rocket || 0); - const negative = (r["-1"] || 0) + (r.confused || 0); - out.reactions_total = r.total_count != null ? r.total_count : positive + negative + (r.laugh || 0) + (r.eyes || 0); - out.reactions_positive = positive; - out.reactions_negative = negative; + const summary = summarizeReactions(data.reactions); + out.reactions_total = summary.total; + out.reactions_positive = summary.positive; + out.reactions_negative = summary.negative; } if (data.state === "closed" && data.created_at && data.closed_at) { @@ -238,10 +1248,10 @@ function evaluateItem(item, defaultRepo) { const prMatch = url.match(/\/pull\/(\d+)/); if (prMatch) { const num = prMatch[1]; - const data = ghAPI(`repos/${itemRepo}/pulls/${num}`); + const data = ghAPIFn(`repos/${itemRepo}/pulls/${num}`); if (!data || !data.state) { out.detail = "api error"; - setPendingAge(out, timestamp); + setPendingAge(out, timestamp, nowMs); return out; } @@ -254,12 +1264,10 @@ function evaluateItem(item, defaultRepo) { // Reactions if (data.reactions && typeof data.reactions === "object") { - const r = data.reactions; - const positive = (r["+1"] || 0) + (r.heart || 0) + (r.hooray || 0) + (r.rocket || 0); - const negative = (r["-1"] || 0) + (r.confused || 0); - out.reactions_total = r.total_count != null ? r.total_count : positive + negative + (r.laugh || 0) + (r.eyes || 0); - out.reactions_positive = positive; - out.reactions_negative = negative; + const summary = summarizeReactions(data.reactions); + out.reactions_total = summary.total; + out.reactions_positive = summary.positive; + out.reactions_negative = summary.negative; } // Zero-touch: merged with no human review comments and no issue-level comments @@ -282,30 +1290,389 @@ function evaluateItem(item, defaultRepo) { } else if (data.state === "open") { out.result = "pending"; out.detail = "open"; - setPendingAge(out, timestamp); + setPendingAge(out, timestamp, nowMs); } else { out.detail = "api error"; - setPendingAge(out, timestamp); + setPendingAge(out, timestamp, nowMs); } return out; } // Comments, labels, etc. — if URL exists, the item was created - out.result = "accepted"; - out.detail = "object exists"; + out.result = "unknown"; + out.detail = "object still exists"; + Object.assign(out, normalizeOutcome(out.result, out.detail)); + return out; +} + +/** + * Evaluate outcome for create_pull_request. + * @param {object} item + * @param {string} itemRepo + * @param {EvalResult} out + * @param {(endpoint: string) => any} [ghAPIFn] + * @returns {EvalResult} + */ +function evaluateCreatePullRequestOutcome(item, itemRepo, out, ghAPIFn = ghAPI) { + const num = resolvePRNumber(item); + const timestamp = item.timestamp || ""; + + if (!num || !itemRepo) { + out.result = "unknown"; + out.detail = "missing pull request reference"; + setPendingAge(out, timestamp); + return out; + } + + const data = ghAPIFn(`repos/${itemRepo}/pulls/${num}`); + if (!data || !data.state) { + out.result = "unknown"; + out.detail = "api error"; + setPendingAge(out, timestamp); + return out; + } + + out.review_comments = typeof data.review_comments === "number" ? data.review_comments : null; + out.changed_files = typeof data.changed_files === "number" ? data.changed_files : null; + out.additions = typeof data.additions === "number" ? data.additions : null; + out.deletions = typeof data.deletions === "number" ? data.deletions : null; + out.comments = typeof data.comments === "number" ? data.comments : null; + + if (data.merged === true) { + out.result = "accepted"; + out.detail = "merged (strong)"; + if (data.created_at && data.merged_at) { + out.resolution_sec = secondsBetween(data.created_at, data.merged_at); + } + if (out.review_comments === 0 && out.comments === 0) { + out.zero_touch = true; + } + return out; + } + + if (data.state === "closed") { + const closingSignal = hasClosingSignal(itemRepo, num, data, ghAPIFn); + out.result = "rejected"; + out.detail = closingSignal ? "closed without merge (strong)" : "closed without merge"; + if (data.created_at && data.closed_at) { + out.resolution_sec = secondsBetween(data.created_at, data.closed_at); + } + return out; + } + + if (data.state === "open") { + const reviewsRaw = ghAPIFn(`repos/${itemRepo}/pulls/${num}/reviews`); + if (reviewsRaw === null) { + out.result = "unknown"; + out.detail = "reviews api error"; + setPendingAge(out, timestamp); + return out; + } + const reviews = Array.isArray(reviewsRaw) ? reviewsRaw : []; + const hasApproved = reviews.some(r => (r?.state || "").toUpperCase() === "APPROVED"); + const hasChangesRequested = reviews.some(r => (r?.state || "").toUpperCase() === "CHANGES_REQUESTED"); + + if (hasApproved && !hasChangesRequested) { + out.result = "accepted"; + out.detail = "approved without requested changes"; + return out; + } + if (hasChangesRequested && !hasApproved) { + out.result = "pending"; + out.detail = "open with changes requested"; + setPendingAge(out, timestamp); + return out; + } + if (reviews.length === 0) { + setPendingAge(out, timestamp); + if (isStalePending(out.pending_age_sec)) { + out.result = "ignored"; + out.detail = "open and stale"; + } else { + out.result = "pending"; + out.detail = "open with no reviews"; + } + return out; + } + out.result = "unknown"; + out.detail = "open with mixed review state"; + setPendingAge(out, timestamp); + return out; + } + + out.result = "unknown"; + out.detail = "unknown pull request state"; + setPendingAge(out, timestamp); + return out; +} + +/** + * Evaluate outcome for push_to_pull_request_branch. + * @param {object} item + * @param {string} itemRepo + * @param {EvalResult} out + * @param {(endpoint: string) => any} [ghAPIFn] + * @returns {EvalResult} + */ +function evaluatePushToPullRequestBranchOutcome(item, itemRepo, out, ghAPIFn = ghAPI) { + const num = resolvePRNumber(item); + const timestamp = item.timestamp || ""; + const pushedShas = extractPushedCommitSHAs(item); + const beforeHead = extractBeforeHeadSHA(item); + + if (!num || !itemRepo) { + out.result = "unknown"; + out.detail = "missing pull request reference"; + setPendingAge(out, timestamp); + return out; + } + + const data = ghAPIFn(`repos/${itemRepo}/pulls/${num}`); + if (!data || !data.state) { + out.result = "unknown"; + out.detail = "api error"; + setPendingAge(out, timestamp); + return out; + } + + const currentHead = normalizeCommitSHA(data?.head?.sha); + + const pushedStillHead = currentHead ? pushedShas.some(sha => shaMatches(sha, currentHead)) : false; + const commitRetentionResults = currentHead && pushedShas.length > 0 ? pushedShas.map(sha => isCommitInBranchHistory(itemRepo, sha, currentHead, ghAPIFn)) : []; + const pushedIncluded = commitRetentionResults.some(inHistory => inHistory === true); + const allPushedCommitsMissingFromHistory = commitRetentionResults.length > 0 && commitRetentionResults.every(inHistory => inHistory === false); + + if (data.merged === true) { + out.result = "accepted"; + out.detail = pushedIncluded ? "merged with pushed commit retained (strong)" : "merged"; + if (data.created_at && data.merged_at) { + out.resolution_sec = secondsBetween(data.created_at, data.merged_at); + } + return out; + } + + if (data.state === "closed") { + out.result = "rejected"; + out.detail = "closed without merge"; + if (data.created_at && data.closed_at) { + out.resolution_sec = secondsBetween(data.created_at, data.closed_at); + } + return out; + } + + if (data.state !== "open") { + out.result = "unknown"; + out.detail = "unknown pull request state"; + setPendingAge(out, timestamp); + return out; + } + + if (pushedStillHead) { + out.result = "accepted"; + out.detail = "pushed commit is current branch head"; + return out; + } + + // A strong rejection requires before-head metadata from execution time so we + // can distinguish "commit not retained" from "insufficient history context". + if (pushedShas.length > 0 && allPushedCommitsMissingFromHistory && beforeHead) { + out.result = "rejected"; + out.detail = "pushed commits were force-pushed away or branch reset"; + return out; + } + + const reviewsRaw = ghAPIFn(`repos/${itemRepo}/pulls/${num}/reviews`); + if (reviewsRaw === null) { + out.result = "unknown"; + out.detail = "reviews api error"; + setPendingAge(out, timestamp); + return out; + } + const reviews = Array.isArray(reviewsRaw) ? reviewsRaw : []; + const hasReviewOnPushedCommit = + pushedShas.length > 0 && + reviews.some(r => { + const reviewCommit = normalizeCommitSHA(r?.commit_id); + return reviewCommit ? pushedShas.some(sha => shaMatches(sha, reviewCommit)) : false; + }); + + if (!hasReviewOnPushedCommit) { + setPendingAge(out, timestamp); + if (isStalePending(out.pending_age_sec)) { + out.result = "ignored"; + out.detail = "open and stale with no review on pushed commits"; + } else { + out.result = "pending"; + out.detail = "open with no review on pushed commits"; + } + return out; + } + + out.result = "unknown"; + out.detail = "open with reviewed pushed commits"; + setPendingAge(out, timestamp); return out; } +/** + * @param {object} item + * @returns {number} + */ +function resolvePRNumber(item) { + if (typeof item.number === "number" && item.number > 0) return item.number; + const candidates = [item.pull_request_number, item.pr_number, item.pr, item.pull_number, item.item_number]; + for (const candidate of candidates) { + const n = Number.parseInt(String(candidate || ""), 10); + if (Number.isInteger(n) && n > 0) return n; + } + const url = item.url || ""; + const prMatch = url.match(/\/pull\/(\d+)/); + if (!prMatch) return 0; + const n = Number.parseInt(prMatch[1], 10); + return Number.isInteger(n) && n > 0 ? n : 0; +} + +/** + * @param {string | null | undefined} sha + * @returns {string} + */ +function normalizeCommitSHA(sha) { + if (!sha || typeof sha !== "string") return ""; + const normalized = sha.trim().toLowerCase(); + return /^[0-9a-f]{7,40}$/.test(normalized) ? normalized : ""; +} + +/** + * Match SHAs across short/full representations (7-40 hex chars). + * Returns true for exact matches and when the longer SHA starts with the + * shorter SHA prefix (minimum 7 chars). + * + * @param {string} a + * @param {string} b + * @returns {boolean} + */ +function shaMatches(a, b) { + const left = normalizeCommitSHA(a); + const right = normalizeCommitSHA(b); + if (!left || !right) return false; + if (left === right) return true; + const leftIsShorterOrEqual = left.length <= right.length; + const shorter = leftIsShorterOrEqual ? left : right; + const longer = leftIsShorterOrEqual ? right : left; + return shorter.length >= 7 && longer.startsWith(shorter); +} + +/** + * @param {object} item + * @returns {string[]} + */ +function extractPushedCommitSHAs(item) { + /** @type {string[]} */ + const shas = []; + // Intentionally exclude `item.head_sha`: it is ambiguous (tip-at-observation) + // and not a reliable indicator of what commit(s) were pushed in this action. + const candidates = [item.commit_sha, item.pushed_commit_sha, item?.metadata?.commit_sha, item?.metadata?.pushed_commit_sha]; + for (const candidate of candidates) { + const normalized = normalizeCommitSHA(candidate); + if (normalized) shas.push(normalized); + } + const listCandidates = [item.commit_shas, item.pushed_commit_shas, item?.metadata?.commit_shas, item?.metadata?.pushed_commit_shas]; + for (const list of listCandidates) { + if (!Array.isArray(list)) continue; + for (const value of list) { + const normalized = normalizeCommitSHA(value); + if (normalized) shas.push(normalized); + } + } + return [...new Set(shas)]; +} + +/** + * @param {object} item + * @returns {string} + */ +function extractBeforeHeadSHA(item) { + const candidates = [item.before_head_sha, item.previous_head_sha, item.head_sha_before, item.branch_head_before, item.pre_push_head_sha, item?.metadata?.before_head_sha, item?.metadata?.previous_head_sha, item?.metadata?.head_sha_before]; + for (const candidate of candidates) { + const normalized = normalizeCommitSHA(candidate); + if (normalized) return normalized; + } + return ""; +} + +/** + * @param {string} repo + * @param {number} number + * @param {any} prData + * @param {(endpoint: string) => object | null} ghAPIFn + * @returns {boolean} + */ +function hasClosingSignal(repo, number, prData, ghAPIFn) { + const labels = Array.isArray(prData?.labels) ? prData.labels : []; + const hasClosingLabel = labels.some(label => { + const name = String(label?.name || "").toLowerCase(); + return CLOSING_LABEL_KEYWORDS.some(keyword => name.includes(keyword)); + }); + if (hasClosingLabel) return true; + + const commentsRaw = ghAPIFn(`repos/${repo}/issues/${number}/comments`); + if (!Array.isArray(commentsRaw)) return false; + return commentsRaw.some(comment => { + const body = String(comment?.body || "").toLowerCase(); + return CLOSING_COMMENT_KEYWORDS.some(keyword => body.includes(keyword)); + }); +} + +/** + * @param {string} repo + * @param {string} commitSHA + * @param {string} branchHeadSHA + * @param {(endpoint: string) => object | null} ghAPIFn + * @returns {boolean | null} + */ +function isCommitInBranchHistory(repo, commitSHA, branchHeadSHA, ghAPIFn) { + if (!commitSHA || !branchHeadSHA) return null; + if (shaMatches(commitSHA, branchHeadSHA)) return true; + const compareData = ghAPIFn(`repos/${repo}/compare/${commitSHA}...${branchHeadSHA}`); + if (!compareData || typeof compareData.status !== "string") return null; + const status = compareData.status.toLowerCase(); + // compare base...head semantics: + // - ahead/identical => base commit is in head history + // - behind => evaluated head is behind base, so base is not retained at this tip + // - diverged => evaluated head diverged from base, so base is not retained + if (status === "ahead" || status === "identical") return true; + if (status === "behind" || status === "diverged") return false; + return null; +} + +/** + * @returns {number} + */ +function staleThresholdSec() { + const raw = Number.parseInt(String(process.env.GH_AW_OUTCOME_STALE_AFTER_SECONDS || ""), 10); + if (Number.isInteger(raw) && raw > 0) return raw; + return 7 * 24 * 60 * 60; +} + +/** + * @param {number | null} pendingAgeSec + * @returns {boolean} + */ +function isStalePending(pendingAgeSec) { + return typeof pendingAgeSec === "number" && pendingAgeSec >= staleThresholdSec(); +} + /** * Set pending_age_sec on the result if the item has a timestamp. * @param {EvalResult} out * @param {string} timestamp + * @param {number} [nowMs] */ -function setPendingAge(out, timestamp) { +function setPendingAge(out, timestamp, nowMs = Date.now()) { if (!timestamp) return; const itemEpoch = isoToEpoch(timestamp); if (itemEpoch === null) return; - out.pending_age_sec = Math.floor(Date.now() / 1000) - itemEpoch; + out.pending_age_sec = Math.floor(nowMs / 1000) - itemEpoch; } // --------------------------------------------------------------------------- @@ -349,11 +1716,15 @@ function main() { let checked = 0; let accepted = 0; let rejected = 0; - const ignored = 0; + let ignored = 0; let pending = 0; let total = 0; let noop = 0; let zeroTouchCount = 0; + let acceptedStrong = 0; + let acceptedMedium = 0; + let acceptedWeak = 0; + let fallbackExistsOnlyCount = 0; /** @type {number[]} */ const resolutionTimes = []; @@ -398,6 +1769,7 @@ function main() { // Write noop entries for (const n of noops) { + const normalized = normalizeOutcome("noop", n.type || ""); fs.appendFileSync( EVAL_JSONL, JSON.stringify({ @@ -405,6 +1777,9 @@ function main() { url: "", repo, result: "noop", + outcome_status: normalized.outcome_status, + evidence_strength: normalized.evidence_strength, + signal: normalized.signal, detail: n.type, workflow, run_id: runId, @@ -430,10 +1805,22 @@ function main() { // Evaluate each actionable item for (const item of actionable) { const evalResult = evaluateItem(item, repo); + const normalized = normalizeOutcome(evalResult.result, evalResult.detail); - switch (evalResult.result) { + switch (normalized.outcome_status) { case "accepted": accepted++; + switch (normalized.evidence_strength) { + case "strong": + acceptedStrong++; + break; + case "medium": + acceptedMedium++; + break; + case "weak": + acceptedWeak++; + break; + } if (evalResult.zero_touch === true) { zeroTouchCount++; } @@ -441,10 +1828,16 @@ function main() { case "rejected": rejected++; break; - default: + case "ignored": + ignored++; + break; + case "pending": pending++; break; } + if (normalized.signal === "target_exists_only") { + fallbackExistsOnlyCount++; + } if (typeof evalResult.resolution_sec === "number" && evalResult.resolution_sec > 0) { resolutionTimes.push(evalResult.resolution_sec); } @@ -456,6 +1849,9 @@ function main() { url: item.url || "", repo: item.repo || repo, result: evalResult.result, + outcome_status: normalized.outcome_status, + evidence_strength: normalized.evidence_strength, + signal: normalized.signal, detail: evalResult.detail, workflow, run_id: runId, @@ -511,6 +1907,10 @@ function main() { ignored, pending, noop, + accepted_strong: acceptedStrong, + accepted_medium: acceptedMedium, + accepted_weak: acceptedWeak, + fallback_exists_only_count: fallbackExistsOnlyCount, acceptance_rate: Math.round(acceptanceRate * 10000) / 10000, waste_rate: Math.round(wasteRate * 10000) / 10000, noop_rate: Math.round(noopRate * 10000) / 10000, @@ -534,4 +1934,20 @@ if (require.main === module) { main(); } -module.exports = { main, evaluateItem, readJSONL, secondsBetween, isoToEpoch }; +module.exports = { + main, + evaluateItem, + evaluateAddReviewer, + evaluateUpdateIssue, + evaluateUpdatePullRequest, + evaluateSubmitPullRequestReview, + evaluateCreateIssue, + evaluateAddComment, + evaluateAddLabels, + evaluateCreatePullRequestOutcome, + evaluatePushToPullRequestBranchOutcome, + normalizeOutcome, + readJSONL, + secondsBetween, + isoToEpoch, +}; diff --git a/setup/js/extra_empty_commit.cjs b/setup/js/extra_empty_commit.cjs index ddb88e13..a5386939 100644 --- a/setup/js/extra_empty_commit.cjs +++ b/setup/js/extra_empty_commit.cjs @@ -93,12 +93,15 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes const MAX_EMPTY_COMMITS = 30; const COMMITS_TO_CHECK = 60; let emptyCommitCount = 0; + let mergeCommitCount = 0; + let analyzedCommitCount = 0; + core.info(`Cycle check: analyzing up to ${COMMITS_TO_CHECK} recent commits on ${branchName}`); try { let logOutput = ""; - // List last N commits: for each, output "COMMIT:" then changed file names. + // List last N commits: for each, output "COMMIT: " then changed file names. // Empty commits will have no files listed after the hash line. - await exec.exec("git", ["log", `--max-count=${COMMITS_TO_CHECK}`, "--format=COMMIT:%H", "--name-only", "HEAD"], { + await exec.exec("git", ["log", `--max-count=${COMMITS_TO_CHECK}`, "--format=COMMIT:%H %P", "--name-only", "HEAD"], { listeners: { stdout: data => { logOutput += data.toString(); @@ -109,8 +112,19 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes // Split by COMMIT: markers; each chunk starts with the hash, followed by filenames const chunks = logOutput.split("COMMIT:").filter(c => c.trim()); for (const chunk of chunks) { + analyzedCommitCount++; const lines = chunk.split("\n").filter(l => l.trim()); - // First line is the hash, remaining lines are changed files + // First line is hash + parent SHAs, remaining lines are changed files. + // Ignore merge commits (2+ parents) so they aren't mistaken for CI-trigger empty commits. + // git log format is "COMMIT: ..." + const hashAndParents = (lines[0] || "").trim().split(" ").filter(Boolean); + const parentCount = Math.max(0, hashAndParents.length - 1); + if (parentCount >= 2) { + mergeCommitCount++; + continue; + } + + // Non-merge commit with no changed files -> empty commit if (lines.length <= 1) { emptyCommitCount++; } @@ -118,6 +132,7 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes } catch { // If we can't check, default to allowing the push emptyCommitCount = 0; + core.warning(`Cycle check unavailable: failed to inspect git history for ${branchName}. Continuing with empty commit count set to 0.`); } if (emptyCommitCount >= MAX_EMPTY_COMMITS) { @@ -125,6 +140,7 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes return { success: true, skipped: true }; } + core.info(`Cycle check details: analyzed ${analyzedCommitCount} commit(s), ignored ${mergeCommitCount} merge commit(s), counted ${emptyCommitCount} empty non-merge commit(s)`); core.info(`Cycle check passed: ${emptyCommitCount} empty commit(s) in last ${COMMITS_TO_CHECK} (limit: ${MAX_EMPTY_COMMITS})`); // Configure git remote with the token for authentication diff --git a/setup/js/extract_base_branch_from_agent_output.cjs b/setup/js/extract_base_branch_from_agent_output.cjs new file mode 100644 index 00000000..c7165de0 --- /dev/null +++ b/setup/js/extract_base_branch_from_agent_output.cjs @@ -0,0 +1,57 @@ +// @ts-check +/// + +const fs = require("fs"); + +const AGENT_OUTPUT_PATH = "/tmp/gh-aw/agent_output.json"; +const SAFE_BRANCH_NAME_REGEX = /^[a-zA-Z0-9/_.-]+$/; + +/** + * @param {string} itemRepo + * @param {string} workflowRepo + * @returns {boolean} + */ +function isSameWorkflowRepo(itemRepo, workflowRepo) { + if (!itemRepo) return true; + if (!workflowRepo) return false; + if (itemRepo === workflowRepo) return true; + + // Safe-output repo values may be a bare repo name and get qualified at runtime. + // Match bare names against the repository suffix from owner/repo. + if (!itemRepo.includes("/")) { + return workflowRepo.endsWith(`/${itemRepo}`); + } + + return false; +} + +/** + * @param {{ agentOutputPath?: string, workflowRepo?: string }} [opts] + * @returns {string} + */ +function extractBaseBranchFromAgentOutput(opts = {}) { + const agentOutputPath = opts.agentOutputPath || AGENT_OUTPUT_PATH; + const workflowRepo = (opts.workflowRepo || process.env.GITHUB_REPOSITORY || "").trim(); + + try { + const data = JSON.parse(fs.readFileSync(agentOutputPath, "utf8")); + const item = (data.items || []).find(i => { + const itemRepo = (i.repo || "").trim(); + const sameRepo = isSameWorkflowRepo(itemRepo, workflowRepo); + return (i.type === "create_pull_request" || i.type === "push_to_pull_request_branch") && i.base_branch && sameRepo; + }); + return typeof item?.base_branch === "string" ? item.base_branch : ""; + } catch { + return ""; + } +} + +async function main() { + const baseBranch = extractBaseBranchFromAgentOutput(); + if (!baseBranch) return; + if (!SAFE_BRANCH_NAME_REGEX.test(baseBranch) || baseBranch.length > 255) return; + core.setOutput("base-branch", baseBranch); + core.info(`Extracted base branch from safe output: ${baseBranch}`); +} + +module.exports = { extractBaseBranchFromAgentOutput, isSameWorkflowRepo, main }; diff --git a/setup/js/frontmatter_hash_pure.cjs b/setup/js/frontmatter_hash_pure.cjs index cb0f0012..039e3e34 100644 --- a/setup/js/frontmatter_hash_pure.cjs +++ b/setup/js/frontmatter_hash_pure.cjs @@ -5,6 +5,8 @@ const path = require("path"); const crypto = require("crypto"); const { ERR_PARSE, ERR_SYSTEM } = require("./error_codes.cjs"); +const MAX_FRONTMATTER_HASH_INPUT_BYTES = 1 << 20; // 1 MiB + /** * Default file reader using Node.js fs module * @param {string} filePath - Path to the file @@ -86,6 +88,8 @@ async function computeFrontmatterHash(workflowPath, options = {}) { // Add the main frontmatter text as-is (trimmed and normalized) const normalizedFrontmatter = normalizeFrontmatterText(frontmatterText); + const normalizedImportedFrontmatterTexts = importedFrontmatterTexts.map(t => normalizeFrontmatterText(t)); + validateNormalizedFrontmatterHashInputSize(normalizedFrontmatter, normalizedImportedFrontmatterTexts); canonical["frontmatter-text"] = normalizedFrontmatter; log(`Normalized frontmatter-text:\n${normalizedFrontmatter}`); @@ -98,7 +102,7 @@ async function computeFrontmatterHash(workflowPath, options = {}) { // Add sorted imported frontmatter texts (concatenated with delimiter) if (importedFrontmatterTexts.length > 0) { - const sortedTexts = importedFrontmatterTexts.map(t => normalizeFrontmatterText(t)).sort(); + const sortedTexts = normalizedImportedFrontmatterTexts.sort(); canonical["imported-frontmatters"] = sortedTexts.join("\n---\n"); log(`canonical.imported-frontmatters:\n${canonical["imported-frontmatters"]}`); } @@ -280,6 +284,24 @@ function normalizeFrontmatterText(text) { return text.trim().replace(/\r\n/g, "\n"); } +/** + * Validates combined normalized frontmatter input size for hash computation. + * Enforces the 1 MiB ceiling used by FH-TV-NEG-001 across main and imported frontmatter text. + * @param {string} normalizedFrontmatterText + * @param {string[]} normalizedImportedFrontmatterTexts + * @throws {Error} When total normalized input exceeds MAX_FRONTMATTER_HASH_INPUT_BYTES. + */ +function validateNormalizedFrontmatterHashInputSize(normalizedFrontmatterText, normalizedImportedFrontmatterTexts) { + let totalBytes = Buffer.byteLength(normalizedFrontmatterText, "utf8"); + for (const text of normalizedImportedFrontmatterTexts) { + totalBytes += Buffer.byteLength(text, "utf8"); + } + + if (totalBytes > MAX_FRONTMATTER_HASH_INPUT_BYTES) { + throw new Error(`frontmatter hash input exceeds ${MAX_FRONTMATTER_HASH_INPUT_BYTES} bytes after normalization`); + } +} + /** * Extract template expressions containing env. or vars. * @param {string} markdown - The markdown body @@ -349,6 +371,104 @@ function marshalSorted(data) { return JSON.stringify(data); } +/** + * Process imports from frontmatter to collect body texts of all transitively imported files. + * Used to include imported file bodies in the body hash. + * @param {string} frontmatterText - The frontmatter text (used to find imports) + * @param {string} baseDir - Base directory for resolving imports + * @param {Set} visited - Set of visited files for cycle detection + * @param {Function} fileReader - File reader function (async (filePath) => content) + * @returns {Promise} Array of imported body texts + */ +async function collectImportedBodies(frontmatterText, baseDir, visited = new Set(), fileReader = defaultFileReader) { + const importedBodyTexts = []; + + const imports = extractImportsFromText(frontmatterText); + if (imports.length === 0) { + return importedBodyTexts; + } + + const sortedImports = [...imports].sort(); + + for (const importPath of sortedImports) { + const fullPath = path.join(baseDir, importPath); + + if (visited.has(fullPath)) continue; + visited.add(fullPath); + + try { + const importContent = await fileReader(fullPath); + const { frontmatterText: importFrontmatterText, markdown: importBody } = extractFrontmatterAndBody(importContent); + + importedBodyTexts.push(importBody); + + const importBaseDir = path.dirname(fullPath); + const nestedBodies = await collectImportedBodies(importFrontmatterText, importBaseDir, visited, fileReader); + importedBodyTexts.push(...nestedBodies); + } catch (err) { + continue; + } + } + + return importedBodyTexts; +} + +/** + * Computes a SHA-256 hash of the markdown body (after frontmatter) including the bodies + * of all transitively imported files. This hash covers changes to the prompt body that are + * not captured by the frontmatter hash. + * + * @param {string} workflowPath - Path to the workflow file + * @param {Object} [options] - Optional configuration + * @param {Function} [options.fileReader] - Custom file reader function (async (filePath) => content) + * @returns {Promise} The SHA-256 hash as a lowercase hexadecimal string (64 characters) + */ +async function computeBodyHash(workflowPath, options = {}) { + const fileReader = options.fileReader || defaultFileReader; + + const content = await fileReader(workflowPath); + const { frontmatterText, markdown } = extractFrontmatterAndBody(content); + + const baseDir = path.dirname(workflowPath); + + const importedBodies = await collectImportedBodies(frontmatterText, baseDir, undefined, fileReader); + + const allParts = [normalizeFrontmatterText(markdown)]; + + if (importedBodies.length > 0) { + const sortedBodies = importedBodies.map(b => normalizeFrontmatterText(b)).sort(); + allParts.push(...sortedBodies); + } + + const combined = allParts.join("\n---\n"); + const hash = crypto.createHash("sha256").update(combined, "utf8").digest("hex"); + return hash; +} + +/** + * Extract body hash from lock file content. + * Only the new JSON metadata format (# gh-aw-metadata: {...}) contains the body hash. + * @param {string} lockFileContent - Content of the .lock.yml file + * @returns {string} The extracted body hash or empty string if not found + */ +function extractBodyHashFromLockFile(lockFileContent) { + const lines = lockFileContent.split("\n"); + for (const line of lines) { + const metadataMatch = line.match(/^#\s*gh-aw-metadata:\s*(\{.+\})/); + if (metadataMatch) { + try { + const metadata = JSON.parse(metadataMatch[1]); + if (metadata.body_hash) { + return metadata.body_hash; + } + } catch (err) { + // Invalid JSON + } + } + } + return ""; +} + /** * Extract hash from lock file content * Supports both formats: @@ -414,15 +534,18 @@ function createGitHubFileReader(github, owner, repo, ref) { module.exports = { computeFrontmatterHash, + computeBodyHash, extractFrontmatterAndBody, extractImportsFromText, extractRelevantTemplateExpressions, marshalCanonicalJSON, marshalSorted, extractHashFromLockFile, + extractBodyHashFromLockFile, normalizeFrontmatterText, parseBoolFromFrontmatter, processImportsTextBased, + collectImportedBodies, defaultFileReader, createGitHubFileReader, }; diff --git a/setup/js/fuzz_template_branch_harness.cjs b/setup/js/fuzz_template_branch_harness.cjs index 7a43ee88..37618e2e 100644 --- a/setup/js/fuzz_template_branch_harness.cjs +++ b/setup/js/fuzz_template_branch_harness.cjs @@ -25,10 +25,7 @@ if (!global.core) { }; } -const renderTemplateScript = require("fs").readFileSync(require("path").join(__dirname, "render_template.cjs"), "utf8"); -const renderMarkdownTemplateMatch = renderTemplateScript.match(/function renderMarkdownTemplate\(markdown\)\s*{[\s\S]*?return result;[\s\S]*?}/); -if (!renderMarkdownTemplateMatch) throw new Error("Could not extract renderMarkdownTemplate"); -const renderMarkdownTemplate = eval(`(${renderMarkdownTemplateMatch[0]})`); +const { renderMarkdownTemplate } = require("./render_template.cjs"); if (require.main === module) { let input = ""; diff --git a/setup/js/generate_aw_info.cjs b/setup/js/generate_aw_info.cjs index a1225028..951cf48c 100644 --- a/setup/js/generate_aw_info.cjs +++ b/setup/js/generate_aw_info.cjs @@ -2,6 +2,7 @@ /// const fs = require("fs"); +const path = require("path"); const { TMP_GH_AW_PATH } = require("./constants.cjs"); const { generateWorkflowOverview } = require("./generate_workflow_overview.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); @@ -117,22 +118,9 @@ async function main(core, ctx) { awInfo.workflow_run_conclusion = workflowRunConclusion; } - // Include custom token weights when set (engine.token-weights in workflow frontmatter). - // Deep structure validation is intentionally minimal here: the JSON schema and Go parser - // already validate the structure at compile time. We only verify the top-level type to - // guard against unexpected env-var values at runtime. - const tokenWeightsEnv = process.env.GH_AW_INFO_TOKEN_WEIGHTS; - if (tokenWeightsEnv) { - try { - const tokenWeights = JSON.parse(tokenWeightsEnv); - if (tokenWeights !== null && typeof tokenWeights === "object" && !Array.isArray(tokenWeights)) { - awInfo.token_weights = tokenWeights; - } else { - core.warning(`GH_AW_INFO_TOKEN_WEIGHTS must be a JSON object, ignoring`); - } - } catch { - core.warning(`Failed to parse GH_AW_INFO_TOKEN_WEIGHTS: ${tokenWeightsEnv}`); - } + const tokenWeights = parseTokenWeightsFromEnv(core); + if (tokenWeights) { + awInfo.token_weights = tokenWeights; } // Include aw_context when the workflow was triggered by a caller that relayed @@ -171,6 +159,7 @@ async function main(core, ctx) { // Write to /tmp/gh-aw directory to avoid inclusion in PR fs.mkdirSync(TMP_GH_AW_PATH, { recursive: true }); + writeMergedModelMultipliers(core, tokenWeights); const tmpPath = TMP_GH_AW_PATH + "/aw_info.json"; fs.writeFileSync(tmpPath, JSON.stringify(awInfo, null, 2)); @@ -178,6 +167,72 @@ async function main(core, ctx) { logStagedPreviewInfo("Generating workflow info in staged mode — no changes applied"); } + /** + * Parse optional custom token weights from GH_AW_INFO_TOKEN_WEIGHTS. + * @param {typeof import('@actions/core')} core + * @returns {Record | null} + */ + function parseTokenWeightsFromEnv(core) { + const tokenWeightsEnv = process.env.GH_AW_INFO_TOKEN_WEIGHTS; + if (!tokenWeightsEnv) { + return null; + } + try { + const tokenWeights = JSON.parse(tokenWeightsEnv); + if (tokenWeights !== null && typeof tokenWeights === "object" && !Array.isArray(tokenWeights)) { + return tokenWeights; + } + core.warning(`GH_AW_INFO_TOKEN_WEIGHTS must be a JSON object, ignoring`); + return null; + } catch { + core.warning(`Failed to parse GH_AW_INFO_TOKEN_WEIGHTS: ${tokenWeightsEnv}`); + return null; + } + } + + /** + * Load and merge model multiplier data, then persist it to /tmp/gh-aw for downstream jobs. + * Base data is read from the setup action directory; custom workflow overrides (if any) + * are merged on top. + * @param {typeof import('@actions/core')} core + * @param {Record | null} tokenWeights + */ + function writeMergedModelMultipliers(core, tokenWeights) { + const builtInPath = path.join(__dirname, "model_multipliers.json"); + /** @type {Record} */ + let builtIn = {}; + if (fs.existsSync(builtInPath)) { + try { + const parsed = JSON.parse(fs.readFileSync(builtInPath, "utf8")); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + builtIn = parsed; + } else { + core.warning(`Built-in model_multipliers.json is not a JSON object: ${builtInPath}`); + } + } catch (error) { + core.warning(`Failed to parse built-in model_multipliers.json: ${String(error)}`); + } + } else { + core.warning(`Built-in model_multipliers.json not found at ${builtInPath}`); + } + + const builtInTokenClassWeights = getPlainObjectOrEmpty(builtIn.token_class_weights); + const builtInMultipliers = getPlainObjectOrEmpty(builtIn.multipliers); + + const customTokenClassWeights = getPlainObjectOrEmpty(tokenWeights?.token_class_weights); + const customMultipliers = getPlainObjectOrEmpty(tokenWeights?.multipliers); + + const merged = { + ...builtIn, + token_class_weights: { ...builtInTokenClassWeights, ...customTokenClassWeights }, + multipliers: { ...builtInMultipliers, ...customMultipliers }, + }; + + const mergedPath = `${TMP_GH_AW_PATH}/model_multipliers.json`; + fs.writeFileSync(mergedPath, JSON.stringify(merged, null, 2)); + core.info(`Generated merged model multipliers at: ${mergedPath}`); + } + core.info("Generated aw_info.json at: " + tmpPath); core.info(JSON.stringify(awInfo, null, 2)); @@ -190,3 +245,22 @@ async function main(core, ctx) { } module.exports = { main }; + +/** + * @param {unknown} value + * @returns {Record} + */ +function getPlainObjectOrEmpty(value) { + if (isPlainObject(value)) { + return value; + } + return {}; +} + +/** + * @param {unknown} value + * @returns {value is Record} + */ +function isPlainObject(value) { + return Boolean(value) && typeof value === "object" && !Array.isArray(value); +} diff --git a/setup/js/generate_git_patch.cjs b/setup/js/generate_git_patch.cjs index 8d8723fe..7a4a844e 100644 --- a/setup/js/generate_git_patch.cjs +++ b/setup/js/generate_git_patch.cjs @@ -40,6 +40,8 @@ function debugLog(message) { * In incremental mode, origin/branchName is fetched explicitly and merge-base fallback is disabled. * @param {string} [options.cwd] - Working directory for git commands. Defaults to GITHUB_WORKSPACE or process.cwd(). * Use this for multi-repo scenarios where repos are checked out to subdirectories. + * @param {string} [options.workspacePath] - Path relative to GITHUB_WORKSPACE used as git working directory. + * When set, this takes precedence over options.cwd and must resolve to a directory under GITHUB_WORKSPACE. * @param {string} [options.repoSlug] - Repository slug (owner/repo) to include in patch filename for disambiguation. * Required for multi-repo scenarios to prevent patch file collisions. * @param {string} [options.token] - GitHub token for git authentication. Falls back to GITHUB_TOKEN env var. @@ -51,8 +53,8 @@ function debugLog(message) { */ async function generateGitPatch(branchName, baseBranch, options = {}) { const mode = options.mode || "full"; - // Support custom cwd for multi-repo scenarios - const cwd = options.cwd || process.env.GITHUB_WORKSPACE || process.cwd(); + const workspaceRoot = process.env.GITHUB_WORKSPACE || process.cwd(); + let cwd = options.cwd || workspaceRoot; // Include repo slug in patch path for multi-repo disambiguation // Build :(exclude) pathspec arguments from the excludedFiles option. @@ -70,6 +72,35 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { } const patchPath = options.repoSlug ? getPatchPathForRepo(branchName, options.repoSlug) : getPatchPath(branchName); + if (options.workspacePath !== undefined && options.workspacePath !== null && String(options.workspacePath).trim() !== "") { + const root = path.resolve(workspaceRoot); + const candidate = path.resolve(root, String(options.workspacePath)); + const relative = path.relative(root, candidate); + if (relative.startsWith("..") || path.isAbsolute(relative)) { + const errorMessage = `Invalid workspacePath '${String(options.workspacePath)}': path must stay under GITHUB_WORKSPACE`; + return { + success: false, + error: errorMessage, + patchPath, + }; + } + if (!fs.existsSync(candidate)) { + return { + success: false, + error: `Invalid workspacePath '${String(options.workspacePath)}': directory does not exist`, + patchPath, + }; + } + if (!fs.statSync(candidate).isDirectory()) { + return { + success: false, + error: `Invalid workspacePath '${String(options.workspacePath)}': path is not a directory`, + patchPath, + }; + } + cwd = candidate; + } + // Validate baseBranch early to avoid confusing git errors (e.g., origin/undefined) if (typeof baseBranch !== "string" || baseBranch.trim() === "") { const errorMessage = "baseBranch is required and must be a non-empty string (received: " + String(baseBranch) + ")"; diff --git a/setup/js/git_helpers.cjs b/setup/js/git_helpers.cjs index 308acc52..b6a6c33b 100644 --- a/setup/js/git_helpers.cjs +++ b/setup/js/git_helpers.cjs @@ -152,13 +152,15 @@ function hasMergeCommitsInRange(baseRef, headRef, options = {}) { } /** - * Ensure the current repository has full history before fetching a git bundle. + * Probe shallow-repository status before fetching a git bundle. * * Bundles generated from a commit range can declare prerequisite commits. A * depth-1 checkout may not contain those prerequisites, and `git fetch ` - * rejects the bundle before the caller can update refs. Unshallowing first makes - * the prerequisites available while avoiding a no-op network fetch for full - * checkouts. + * can reject the bundle before the caller can update refs. + * + * IMPORTANT: Do not unshallow here. Full-history fetches are prohibitively + * expensive for large monorepos. Callers recover from prerequisite failures by + * fetching only the missing commit objects from origin and retrying. * * @param {{ getExecOutput: Function, exec: Function }} execApi - Exec API to run git commands. * @param {Object} [options] - Options passed through to exec calls. @@ -170,12 +172,11 @@ async function ensureFullHistoryForBundle(execApi, options = {}) { ({ stdout } = await execApi.getExecOutput("git", ["rev-parse", "--is-shallow-repository"], options)); } catch (error) { const message = error instanceof Error ? error.message : String(error); - core.warning(`Could not determine shallow repository status; skipping unshallow: ${message}`); + core.warning(`Could not determine shallow repository status; skipping full-history fetch probe: ${message}`); return; } if (stdout.trim() === "true") { - core.info("Repository is shallow; fetching full history before applying bundle"); - await execApi.exec("git", ["fetch", "--unshallow", "origin"], options); + core.info("Repository is shallow; skipping full-history fetch and relying on prerequisite recovery"); } } diff --git a/setup/js/handle_agent_failure.cjs b/setup/js/handle_agent_failure.cjs index 9bdc6f74..9a3ca6bb 100644 --- a/setup/js/handle_agent_failure.cjs +++ b/setup/js/handle_agent_failure.cjs @@ -241,12 +241,26 @@ function isReusableFailureIssue(body, options) { return (failureMarker.failure_categories || "") === options.failureCategories.join("|"); } +/** + * Escape a GitHub search phrase for safe inclusion inside double quotes. + * GitHub search phrases are wrapped in double quotes, so embedded backslashes and + * quotes must be escaped, and newlines are normalized to spaces to keep the query + * on a single line. + * @param {string} value - Raw phrase value + * @returns {string} Escaped phrase + */ +function escapeGitHubSearchPhrase(value) { + return value + .replace(/\\/g, "\\\\") + .replace(/"/g, '\\"') + .replace(/\r?\n|\r/g, " "); +} + /** * Find an existing open failure issue that exactly matches the current failure metadata. * @param {Object} options - Search options * @param {string} options.owner - Repository owner * @param {string} options.repo - Repository name - * @param {string} options.issueTitle - Failure issue title * @param {string} options.workflowId - Workflow identifier * @param {string} options.branch - Triggering branch * @param {number|undefined} options.pullRequestNumber - Triggering pull request number @@ -254,10 +268,10 @@ function isReusableFailureIssue(body, options) { * @returns {Promise<{number: number, html_url: string} | null>} Matching issue or null */ async function findExistingFailureIssue(options) { - const { owner, repo, issueTitle, workflowId, branch, pullRequestNumber, failureCategories } = options; - const searchQuery = `repo:${owner}/${repo} is:issue is:open label:agentic-workflows in:title "${issueTitle}"`; + const { owner, repo, workflowId, branch, pullRequestNumber, failureCategories } = options; + const escapedWorkflowId = escapeGitHubSearchPhrase(workflowId); + const searchQuery = `repo:${owner}/${repo} is:issue is:open label:agentic-workflows ` + `"gh-aw-agentic-workflow:" "workflow_id: ${escapedWorkflowId}" in:body`; const perPage = 100; - for (let page = 1; ; page += 1) { const searchResult = await github.rest.search.issuesAndPullRequests({ q: searchQuery, @@ -2239,13 +2253,12 @@ async function main() { hasStaleLockFileFailed, }); - core.info(`Checking for existing issue with precise metadata match for title: "${issueTitle}"`); + core.info(`Checking for existing issue with precise failure metadata for title: "${issueTitle}"`); try { const existingIssue = await findExistingFailureIssue({ owner, repo, - issueTitle, workflowId: workflowID, branch: currentBranch, pullRequestNumber: pullRequest?.number, diff --git a/setup/js/interpolate_prompt.cjs b/setup/js/interpolate_prompt.cjs index 36c63cec..b80cfcfb 100644 --- a/setup/js/interpolate_prompt.cjs +++ b/setup/js/interpolate_prompt.cjs @@ -10,13 +10,12 @@ const fs = require("fs"); const path = require("path"); -const { isTruthy } = require("./is_truthy.cjs"); -const { selectBranch } = require("./template_branch.cjs"); const { processRuntimeImports } = require("./runtime_import.cjs"); const { writeInlineSubAgents } = require("./extract_inline_sub_agents.cjs"); const { writeInlineSkills } = require("./extract_inline_skills.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_API, ERR_CONFIG, ERR_VALIDATION } = require("./error_codes.cjs"); +const { renderMarkdownTemplate } = require("./render_template.cjs"); /** * @typedef {Object} ImportTreeNode @@ -63,119 +62,6 @@ function interpolateVariables(content, variables) { return result; } -/** - * Renders a Markdown template by processing {{#if}} conditional blocks. - * When a conditional block is removed (falsy condition) and the template tags - * were on their own lines, the empty lines are cleaned up to avoid - * leaving excessive blank lines in the output. - * @param {string} markdown - The markdown content to process - * @returns {string} - The processed markdown content - */ -function renderMarkdownTemplate(markdown) { - core.info(`[renderMarkdownTemplate] Starting template rendering`); - core.info(`[renderMarkdownTemplate] Input length: ${markdown.length} characters`); - - // Preserve fenced code blocks to avoid processing {{#if}} markers inside them - const _codeBlocks = []; - const _FENCE_PH = "\x00FENCE\x00"; - const _stripped = markdown.replace(/`{3,}[^\n]*\n[\s\S]*?\n`{3,}[ \t]*/g, m => { - _codeBlocks.push(m); - return `${_FENCE_PH}${_codeBlocks.length - 1}${_FENCE_PH}`; - }); - if (_codeBlocks.length > 0) { - core.info(`[renderMarkdownTemplate] Preserved ${_codeBlocks.length} fenced code block(s) from template processing`); - } - - // Count conditionals before processing - const blockConditionals = (_stripped.match(/(\n?)([ \t]*{{#if\s+([^}]*)}}[ \t]*\n)([\s\S]*?)([ \t]*(?:{{#endif}}|{{\/if}})[ \t]*)(\n?)/g) || []).length; - const inlineConditionals = (_stripped.match(/{{#if\s+([^}]*)}}([\s\S]*?)(?:{{#endif}}|{{\/if}})/g) || []).length - blockConditionals; - - core.info(`[renderMarkdownTemplate] Found ${blockConditionals} block conditional(s) and ${inlineConditionals} inline conditional(s)`); - - let blockCount = 0; - let keptBlocks = 0; - let removedBlocks = 0; - - // First pass: Handle blocks where tags are on their own lines - // Captures: (leading newline)(opening tag line)(condition)(body)(closing tag line)(trailing newline) - // Closing tag: {{#endif}} (primary) or {{/if}} (alternate) - let result = _stripped.replace(/(\n?)([ \t]*{{#if\s+([^}]*)}}[ \t]*\n)([\s\S]*?)([ \t]*(?:{{#endif}}|{{\/if}})[ \t]*)(\n?)/g, (match, leadNL, openLine, cond, body, closeLine, trailNL) => { - blockCount++; - const condTrimmed = cond.trim(); - const bodyPreview = body.substring(0, 60).replace(/\n/g, "\\n"); - - core.info(`[renderMarkdownTemplate] Block ${blockCount}: condition="${condTrimmed}" -> evaluating branches`); - core.info(`[renderMarkdownTemplate] Body preview: "${bodyPreview}${body.length > 60 ? "..." : ""}"`); - - // Evaluate the full branch chain (if / elseif* / else?) - const selectedContent = selectBranch(cond, body); - - if (selectedContent !== null) { - keptBlocks++; - core.info(`[renderMarkdownTemplate] Action: Keeping selected branch with leading newline=${!!leadNL}`); - return leadNL + selectedContent; - } else { - removedBlocks++; - core.info(`[renderMarkdownTemplate] Action: Removing entire block`); - return ""; - } - }); - - core.info(`[renderMarkdownTemplate] First pass complete: ${keptBlocks} kept, ${removedBlocks} removed`); - - let inlineCount = 0; - let keptInline = 0; - let removedInline = 0; - - // Second pass: Handle inline conditionals (tags not on their own lines) - // Closing tag: {{#endif}} (primary) or {{/if}} (alternate) - result = result.replace(/{{#if\s+([^}]*)}}([\s\S]*?)(?:{{#endif}}|{{\/if}})/g, (_, cond, body) => { - inlineCount++; - const condTrimmed = cond.trim(); - const bodyPreview = body.substring(0, 40).replace(/\n/g, "\\n"); - - const selectedContent = selectBranch(cond, body); - - core.info(`[renderMarkdownTemplate] Inline ${inlineCount}: condition="${condTrimmed}" -> ${selectedContent !== null ? "KEEP" : "REMOVE"}`); - core.info(`[renderMarkdownTemplate] Body preview: "${bodyPreview}${body.length > 40 ? "..." : ""}"`); - - if (selectedContent !== null) { - keptInline++; - return selectedContent; - } else { - removedInline++; - return ""; - } - }); - - core.info(`[renderMarkdownTemplate] Second pass complete: ${keptInline} kept, ${removedInline} removed`); - - // Clean up excessive blank lines (more than one blank line = 2 newlines) - const beforeCleanup = result.length; - const excessiveLines = (result.match(/\n{3,}/g) || []).length; - result = result.replace(/\n{3,}/g, "\n\n"); - - if (excessiveLines > 0) { - core.info(`[renderMarkdownTemplate] Cleaned up ${excessiveLines} excessive blank line sequence(s)`); - core.info(`[renderMarkdownTemplate] Length change from cleanup: ${beforeCleanup} -> ${result.length} characters`); - } - // Restore fenced code blocks - if (_codeBlocks.length > 0) { - result = result.replace(/\x00FENCE\x00(\d+)\x00FENCE\x00/g, (_, i) => _codeBlocks[+i]); - } - - // Runtime assertion: number of fence markers must be the same before and after processing - const _inputFenceCount = (markdown.match(/`{3,}/g) || []).length; - const _outputFenceCount = (result.match(/`{3,}/g) || []).length; - if (_inputFenceCount !== _outputFenceCount) { - core.warning(`[renderMarkdownTemplate] Fence count mismatch: input had ${_inputFenceCount} fence marker(s), output has ${_outputFenceCount}`); - } - - core.info(`[renderMarkdownTemplate] Final output length: ${result.length} characters`); - - return result; -} - /** * Main function for prompt variable interpolation and template rendering */ diff --git a/setup/js/merge_awf_model_multipliers.cjs b/setup/js/merge_awf_model_multipliers.cjs new file mode 100644 index 00000000..d7c471e9 --- /dev/null +++ b/setup/js/merge_awf_model_multipliers.cjs @@ -0,0 +1,212 @@ +// @ts-check +/// + +const fs = require("fs"); +const path = require("path"); +const { TMP_GH_AW_PATH } = require("./constants.cjs"); + +const DEFAULT_MODEL_MULTIPLIERS_PATH = `${TMP_GH_AW_PATH}/model_multipliers.json`; + +/** + * @param {unknown} value + * @returns {value is Record} + */ +function isPlainObject(value) { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** + * @param {Record} rawMultipliers + * @returns {Record} + */ +function normalizeMultipliers(rawMultipliers) { + /** @type {Record} */ + const normalized = {}; + for (const [key, value] of Object.entries(rawMultipliers)) { + if (typeof value === "number" && Number.isFinite(value) && value > 0) { + normalized[key] = value; + } + } + return normalized; +} + +/** + * @param {unknown} rawModels + * @returns {Array<{alias: string, targets: string[]}>} + */ +function normalizeAliasRows(rawModels) { + if (!isPlainObject(rawModels)) { + return []; + } + + /** @type {Array<{alias: string, targets: string[]}>} */ + const rows = []; + for (const [alias, rawTargets] of Object.entries(rawModels)) { + if (!Array.isArray(rawTargets)) { + continue; + } + const trimmedTargets = rawTargets.map(target => (typeof target === "string" ? target.trim() : "")); + const targets = trimmedTargets.filter(target => target !== ""); + if (targets.length === 0) { + continue; + } + rows.push({ alias, targets }); + } + + rows.sort((a, b) => { + // Sort the default alias ("") last for readability in step-summary tables. + // We substitute it with U+ffff only for comparison; it is a noncharacter and + // reliably sorts after typical printable alias strings in localeCompare. + const left = a.alias || "\uffff"; + const right = b.alias || "\uffff"; + return left.localeCompare(right); + }); + return rows; +} + +/** + * @param {string} value + * @returns {string} + */ +function escapeTableValue(value) { + return value.replace(/\|/g, "\\|"); +} + +/** + * @param {Array<{alias: string, targets: string[]}>} aliasRows + * @returns {string} + */ +function renderModelAliasSummary(aliasRows) { + const lines = []; + lines.push("
"); + lines.push(`AWF model aliases (${aliasRows.length})`); + lines.push(""); + lines.push("| Alias | Resolution order |"); + lines.push("|-------|------------------|"); + for (const row of aliasRows) { + const aliasLabel = row.alias === "" ? "(default)" : `\`${escapeTableValue(row.alias)}\``; + const resolutionOrder = row.targets.map(target => `\`${escapeTableValue(target)}\``).join(" → "); + lines.push(`| ${aliasLabel} | ${resolutionOrder} |`); + } + lines.push(""); + lines.push("
"); + lines.push(""); + return lines.join("\n"); +} + +/** + * @param {Array<{alias: string, targets: string[]}>} aliasRows + * @param {(message: string) => void} warn + */ +function writeAliasSummary(aliasRows, warn) { + if (aliasRows.length === 0) { + return; + } + + const summaryPath = process.env.GITHUB_STEP_SUMMARY; + if (!summaryPath) { + return; + } + + try { + fs.appendFileSync(summaryPath, renderModelAliasSummary(aliasRows), "utf8"); + } catch (error) { + warn(`warning: failed to write AWF model alias summary: ${String(error)}`); + } +} + +/** + * @param {object} options + * @param {string} options.configPath + * @param {string} options.multipliersPath + * @param {(message: string) => void} options.warn + */ +function mergeModelMultipliers({ configPath, multipliersPath, warn }) { + if (!fs.existsSync(configPath) || !fs.existsSync(multipliersPath)) { + return; + } + + /** @type {Record | null} */ + let multipliersDoc = null; + try { + const parsed = JSON.parse(fs.readFileSync(multipliersPath, "utf8")); + if (isPlainObject(parsed)) { + multipliersDoc = parsed; + } + } catch (error) { + warn(`warning: failed to parse model multipliers file: ${String(error)}`); + return; + } + if (!multipliersDoc || !isPlainObject(multipliersDoc.multipliers)) { + return; + } + + const normalized = normalizeMultipliers(multipliersDoc.multipliers); + + /** @type {Record | null} */ + let configDoc = null; + try { + const parsed = JSON.parse(fs.readFileSync(configPath, "utf8")); + if (isPlainObject(parsed)) { + configDoc = parsed; + } + } catch (error) { + warn(`warning: failed to parse awf-config.json before model multiplier merge: ${String(error)}`); + return; + } + if (!configDoc) { + return; + } + + const apiProxy = isPlainObject(configDoc.apiProxy) ? configDoc.apiProxy : {}; + const aliasRows = normalizeAliasRows(apiProxy.models); + if (Object.keys(normalized).length > 0) { + apiProxy.modelMultipliers = normalized; + } else { + delete apiProxy.modelMultipliers; + } + configDoc.apiProxy = apiProxy; + + fs.writeFileSync(configPath, JSON.stringify(configDoc), "utf8"); + writeAliasSummary(aliasRows, warn); +} + +/** + * @param {object} [options] + * @param {string} [options.runnerTemp] + * @param {string} [options.configPath] + * @param {string} [options.multipliersPath] + * @param {(message: string) => void} [options.warn] + */ +function main(options = {}) { + const runnerTemp = options.runnerTemp ?? process.env.RUNNER_TEMP; + if (!runnerTemp) { + throw new Error("RUNNER_TEMP is required"); + } + + const configPath = options.configPath ?? path.join(runnerTemp, "gh-aw", "awf-config.json"); + const multipliersPath = options.multipliersPath ?? process.env.GH_AW_MODEL_MULTIPLIERS_PATH ?? DEFAULT_MODEL_MULTIPLIERS_PATH; + const warn = options.warn ?? (message => process.stderr.write(`${message}\n`)); + + mergeModelMultipliers({ configPath, multipliersPath, warn }); +} + +if (require.main === module) { + try { + main(); + } catch (error) { + process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); + process.exit(1); + } +} + +module.exports = { + DEFAULT_MODEL_MULTIPLIERS_PATH, + isPlainObject, + normalizeMultipliers, + normalizeAliasRows, + renderModelAliasSummary, + writeAliasSummary, + mergeModelMultipliers, + main, +}; diff --git a/setup/js/messages_footer.cjs b/setup/js/messages_footer.cjs index 2e9497c2..af4e49c2 100644 --- a/setup/js/messages_footer.cjs +++ b/setup/js/messages_footer.cjs @@ -73,6 +73,8 @@ function buildModelPrefix(modelName) { * @property {number} [effectiveTokens] - Total effective token count for the run (shown as N when > 0, in compact format) * @property {string} [model] - Model name used for the run, used to build a compact model identifier in ET suffixes * @property {string} [emoji] - Optional emoji representing the workflow (from frontmatter) + * @property {string} [slashCommand] - Slash command name (without leading slash) for the run-again hint, when applicable + * @property {string} [slashCommandPlaceholder] - Custom hint text appended after the command name (replaces default "to run again") */ /** @@ -135,6 +137,11 @@ function getFooterMessage(ctx) { if (ctx.historyUrl) { defaultFooter += " · [◷]({history_url})"; } + // Append slash command hint when applicable (workflow has a slash command trigger) + if (ctx.slashCommand) { + const hintText = ctx.slashCommandPlaceholder || "to run again"; + defaultFooter += `\n> Comment /{slash_command} ${hintText}`; + } return renderTemplate(defaultFooter, templateContext); } @@ -422,6 +429,27 @@ function generateFooterWithMessages(workflowName, runUrl, workflowSource, workfl // Read workflow emoji from environment variable if available. const emoji = process.env.GH_AW_WORKFLOW_EMOJI || undefined; + // Read slash command from GH_AW_COMMANDS (JSON array) when available. + // Use the first command as the hint. This is only set when the workflow has a slash command trigger. + let slashCommand; + const commandsJSON = process.env.GH_AW_COMMANDS; + if (commandsJSON) { + try { + const commands = JSON.parse(commandsJSON); + if (Array.isArray(commands) && commands.length > 0 && typeof commands[0] === "string") { + slashCommand = commands[0]; + } + } catch { + // Silently ignore malformed GH_AW_COMMANDS; the hint is a non-critical enhancement + // and omitting it is always safe. The value is compiler-generated JSON, so this + // path should not occur in practice. + } + } + + // Read optional footer hint placeholder from GH_AW_COMMAND_PLACEHOLDER. + // When set, it replaces the default "to run again" suffix in the slash command hint. + const slashCommandPlaceholder = process.env.GH_AW_COMMAND_PLACEHOLDER; + const ctx = { workflowName, runUrl, @@ -431,6 +459,8 @@ function generateFooterWithMessages(workflowName, runUrl, workflowSource, workfl historyUrl: historyUrl || undefined, effectiveTokens, emoji, + slashCommand, + slashCommandPlaceholder, }; const { skipDetectionCaution = false } = options || {}; diff --git a/setup/js/package.json b/setup/js/package.json index 3694f70e..44d48d62 100644 --- a/setup/js/package.json +++ b/setup/js/package.json @@ -8,13 +8,13 @@ "@actions/glob": "^0.7.0", "@actions/io": "^3.0.2", "@types/node": "^25.9.1", - "@vitest/coverage-v8": "^4.1.4", - "@vitest/ui": "^4.1.6", + "@vitest/coverage-v8": "^4.1.7", + "@vitest/ui": "^4.1.7", "minimatch": ">=3.1.3", "prettier": "^3.8.3", "typescript": "^6.0.3", - "vite": "^8.0.13", - "vitest": "^4.1.6" + "vite": "^8.0.14", + "vitest": "^4.1.7" }, "overrides": { "undici": "^6.23.0" diff --git a/setup/js/parse_mcp_gateway_log.cjs b/setup/js/parse_mcp_gateway_log.cjs index 82512c8c..2ee5964c 100644 --- a/setup/js/parse_mcp_gateway_log.cjs +++ b/setup/js/parse_mcp_gateway_log.cjs @@ -49,7 +49,7 @@ function formatDurationMs(ms) { /** * Parses token-usage.jsonl content and returns an aggregated summary. - * Computes effective tokens (ET) per model using the GH_AW_MODEL_MULTIPLIERS env var. + * Computes effective tokens (ET) per model using merged multipliers, env fallback, then built-in multipliers. * @param {string} jsonlContent - The token-usage.jsonl file content * @returns {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, totalEffectiveTokens: number, byModel: Object, entries: Array} | null} */ diff --git a/setup/js/parse_threat_detection_results.cjs b/setup/js/parse_threat_detection_results.cjs index 990cb421..6edc9404 100644 --- a/setup/js/parse_threat_detection_results.cjs +++ b/setup/js/parse_threat_detection_results.cjs @@ -20,7 +20,7 @@ const fs = require("fs"); const path = require("path"); const { getErrorMessage } = require("./error_helpers.cjs"); const { listFilesRecursively } = require("./file_helpers.cjs"); -const { DETECTION_LOG_FILENAME } = require("./constants.cjs"); +const { DETECTION_LOG_FILENAME, DETECTION_RESULT_FILENAME } = require("./constants.cjs"); const { ERR_SYSTEM, ERR_PARSE, ERR_VALIDATION } = require("./error_codes.cjs"); const RESULT_PREFIX = "THREAT_DETECTION_RESULT:"; @@ -79,6 +79,34 @@ function extractResultFromText(text) { return RESULT_PREFIX + text.substring(jsonStartPos, jsonEndPos + 1); } +/** Required fields that must be present for a valid structured threat detection object. */ +const STRUCTURED_OUTPUT_REQUIRED_FIELDS = ["prompt_injection", "secret_leak", "malicious_patch"]; + +/** + * Try to extract a threat detection verdict from Codex structured output. + * When Codex runs with -c response_schema enabled for detection jobs, it outputs the + * JSON verdict object directly — without the THREAT_DETECTION_RESULT: prefix that + * non-structured (text-mode) engines use. This function detects such output by + * checking for the expected threat detection fields and wraps it with the prefix so + * the existing parsing pipeline can handle it uniformly. + * + * @param {string} text - Text that may be a raw JSON threat detection object + * @returns {string|null} RESULT_PREFIX + JSON string if the text is a valid + * threat detection object without the prefix, null otherwise + */ +function extractStructuredOutput(text) { + const trimmed = text.trim(); + if (!trimmed.startsWith("{")) return null; + try { + const parsed = JSON.parse(trimmed); + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) return null; + if (!STRUCTURED_OUTPUT_REQUIRED_FIELDS.every(field => field in parsed)) return null; + return RESULT_PREFIX + trimmed; + } catch { + return null; + } +} + /** * Try to extract a THREAT_DETECTION_RESULT value from a stream-json line. * Stream-json output from Claude wraps the result in JSON envelopes like: @@ -87,6 +115,10 @@ function extractResultFromText(text) { * The same result also appears in {"type":"assistant"} messages, but we only * extract from "type":"result" which is the authoritative final summary. * + * When Codex runs with structured outputs (response_schema), the Responses API + * emits the verdict as a plain JSON object in response events (no prefix). + * extractStructuredOutput handles that case as a fallback. + * * @param {string} line - A single line from the detection log * @returns {string|null} The raw THREAT_DETECTION_RESULT:... string if found, null otherwise */ @@ -144,14 +176,16 @@ function extractFromStreamJson(line) { } // Codex responses API emits final output text in response events. + // Try the prefixed format first; fall back to structured output (no prefix) + // when Codex runs with -c response_schema in detection mode. if (obj.type === "response.output_text.done" && typeof obj.text === "string") { - return extractPrefixedResult(obj.text); + return extractPrefixedResult(obj.text) || extractStructuredOutput(obj.text); } if (obj.type === "response.content_part.done" && obj.part && typeof obj.part.text === "string") { - return extractPrefixedResult(obj.part.text); + return extractPrefixedResult(obj.part.text) || extractStructuredOutput(obj.part.text); } if (obj.type === "item.completed" && obj.item && typeof obj.item.text === "string") { - return extractPrefixedResult(obj.item.text); + return extractPrefixedResult(obj.item.text) || extractStructuredOutput(obj.item.text); } } catch { // Not valid JSON — not a stream-json line @@ -305,6 +339,63 @@ function parseDetectionLog(content) { } } +/** + * Try to parse the threat detection verdict from a Codex structured output file. + * + * When Codex runs with `--output-last-message `, it writes the final model + * response directly to that file as plain text. Combined with `--output-schema`, + * the content is the JSON verdict object (no THREAT_DETECTION_RESULT: prefix, no + * log noise). This function reads and validates the file, returning the verdict + * or an error object exactly like `parseDetectionLog`. + * + * @param {string} resultFilePath - Absolute path to the structured result file + * @returns {{ verdict?: { prompt_injection: boolean, secret_leak: boolean, malicious_patch: boolean, reasons: string[] }, error?: string } | null} + * Returns null if the file does not exist (caller should fall back to log parsing). + * Returns an error object if the file exists but is malformed (conservative: report parse_error). + */ +function parseStructuredResultFile(resultFilePath) { + if (!fs.existsSync(resultFilePath)) { + return null; + } + + let content; + try { + content = fs.readFileSync(resultFilePath, "utf8"); + } catch (/** @type {any} */ readError) { + return { error: `Failed to read structured result file: ${getErrorMessage(readError)}` }; + } + + const trimmed = content.trim(); + if (!trimmed) { + return { error: "Structured result file is empty" }; + } + + try { + const parsed = JSON.parse(trimmed); + + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + return { error: `Structured result must be a JSON object, got ${parsed === null ? "null" : Array.isArray(parsed) ? "array" : typeof parsed}` }; + } + + for (const field of ["prompt_injection", "secret_leak", "malicious_patch"]) { + if (typeof parsed[field] !== "boolean") { + return { error: `Invalid type for "${field}" in structured result: expected boolean, got ${typeof parsed[field]} (${JSON.stringify(parsed[field])})` }; + } + } + + return { + verdict: { + prompt_injection: parsed.prompt_injection, + secret_leak: parsed.secret_leak, + malicious_patch: parsed.malicious_patch, + reasons: Array.isArray(parsed.reasons) ? parsed.reasons : [], + }, + }; + } catch (/** @type {any} */ parseError) { + return { error: `Failed to parse JSON from structured result file: ${getErrorMessage(parseError)}` }; + } +} + /** * Main entry point for parsing threat detection results and concluding the detection job. * @@ -349,6 +440,48 @@ async function main() { } } + /** + * Log and evaluate a parsed verdict, then set step outputs/failure state. + * @param {{ prompt_injection: boolean, secret_leak: boolean, malicious_patch: boolean, reasons: string[] }} verdict + * @param {string} sourceLabel + */ + function evaluateAndReportVerdict(verdict, sourceLabel) { + core.info(`📋 Threat detection verdict${sourceLabel ? ` (${sourceLabel})` : ""}:`); + core.info(` prompt_injection : ${verdict.prompt_injection}`); + core.info(` secret_leak : ${verdict.secret_leak}`); + core.info(` malicious_patch : ${verdict.malicious_patch}`); + const hasReasons = verdict.reasons.length > 0; + if (hasReasons) { + core.info(` reasons (${verdict.reasons.length}):`); + verdict.reasons.forEach((reason, i) => core.info(` [${i + 1}] ${reason}`)); + } else { + core.info(" reasons : (none)"); + } + + const threatsDetected = verdict.prompt_injection || verdict.secret_leak || verdict.malicious_patch; + if (threatsDetected) { + const threats = []; + if (verdict.prompt_injection) threats.push("prompt injection"); + if (verdict.secret_leak) threats.push("secret leak"); + if (verdict.malicious_patch) threats.push("malicious patch"); + const reasonsText = hasReasons ? "\nReasons: " + verdict.reasons.join("; ") : ""; + core.error("🚨 Security threats detected: " + threats.join(", ")); + if (hasReasons) { + core.error(" Reasons: " + verdict.reasons.join("; ")); + } + setDetectionFailure("threat_detected", `${ERR_VALIDATION}: ❌ Security threats detected: ${threats.join(", ")}${reasonsText}`); + } else { + core.info("✅ No security threats detected. Safe outputs may proceed."); + core.setOutput("conclusion", "success"); + core.setOutput("success", "true"); + core.setOutput("reason", ""); + } + + core.info("════════════════════════════════════════════════════════"); + core.info("🛡️ Threat detection conclusion complete."); + core.info("════════════════════════════════════════════════════════"); + } + // Top-level try/catch ensures outputs are always set and the step never throws // unexpectedly. Any unanticipated runtime error (e.g. I/O error outside the guarded // paths) is caught here and surfaced as a parse_error warning (in warn mode) or @@ -375,7 +508,9 @@ async function main() { core.info(`📋 continue-on-error: ${continueOnError}`); core.info(`📋 detection execution outcome: ${JSON.stringify(detectionExecutionOutcome)}`); core.info(`📁 Threat detection directory: ${threatDetectionDir}`); + const resultFilePath = path.join(threatDetectionDir, DETECTION_RESULT_FILENAME); core.info(`📄 Detection log path: ${logPath}`); + core.info(`📄 Structured result path: ${resultFilePath}`); // ── Step 1: Check whether detection was needed ────────────────────────── if (runDetection !== "true") { @@ -389,9 +524,31 @@ async function main() { return; } - core.info("🔍 Detection is required. Proceeding to parse detection log..."); + core.info("🔍 Detection is required. Proceeding to parse detection result..."); + + // ── Step 2: Try the Codex structured result file first ─────────────────── + // When Codex runs with --output-schema and --output-last-message the final + // model response is written directly to detection_result.json as clean JSON, + // eliminating log-scraping parse failures caused by SSE/trace noise. + core.info(`🔎 Checking for structured result file: ${resultFilePath}`); + const structuredResult = parseStructuredResultFile(resultFilePath); + + if (structuredResult !== null) { + if (structuredResult.error) { + core.warning(`⚠️ Structured result file exists but could not be parsed: ${structuredResult.error}`); + core.info(" Falling back to detection log parsing..."); + } else if (!structuredResult.verdict) { + core.warning("⚠️ Structured result parsed but verdict was missing. Falling back to detection log parsing..."); + } else { + core.info("✔️ Structured result file found and parsed successfully."); + evaluateAndReportVerdict(structuredResult.verdict, "from structured result file"); + return; + } + } else { + core.info("ℹ️ No structured result file found. Falling back to detection log parsing."); + } - // ── Step 2: Verify the detection log file exists ───────────────────────── + // ── Step 3: Verify the detection log file exists ───────────────────────── if (!fs.existsSync(logPath)) { core.error("❌ Detection log file not found at: " + logPath); core.info("📁 Listing all files in artifact directory for diagnosis: " + threatDetectionDir); @@ -412,7 +569,7 @@ async function main() { core.info("✔️ Detection log file exists: " + logPath); - // ── Step 3: Read the detection log ─────────────────────────────────────── + // ── Step 4: Read the detection log ─────────────────────────────────────── let logContent; try { logContent = fs.readFileSync(logPath, "utf8"); @@ -434,7 +591,7 @@ async function main() { core.info(`📄 No lines containing THREAT_DETECTION_RESULT found in ${logLines.length} lines`); } - // ── Step 4: Parse the detection result ─────────────────────────────────── + // ── Step 5: Parse the detection result from log ─────────────────────────── core.info("🔎 Parsing THREAT_DETECTION_RESULT from detection log..."); const { verdict, error } = parseDetectionLog(logContent); @@ -447,46 +604,9 @@ async function main() { return; } - // ── Step 5: Log the full verdict ───────────────────────────────────────── - core.info("📋 Threat detection verdict:"); - core.info(` prompt_injection : ${verdict.prompt_injection}`); - core.info(` secret_leak : ${verdict.secret_leak}`); - core.info(` malicious_patch : ${verdict.malicious_patch}`); - if (verdict.reasons && verdict.reasons.length > 0) { - core.info(` reasons (${verdict.reasons.length}):`); - verdict.reasons.forEach((reason, i) => core.info(` [${i + 1}] ${reason}`)); - } else { - core.info(" reasons : (none)"); - } - - // ── Step 6: Evaluate verdict and set conclusion ─────────────────────────── - const threatsDetected = verdict.prompt_injection || verdict.secret_leak || verdict.malicious_patch; - - if (threatsDetected) { - const threats = []; - if (verdict.prompt_injection) threats.push("prompt injection"); - if (verdict.secret_leak) threats.push("secret leak"); - if (verdict.malicious_patch) threats.push("malicious patch"); - - const reasonsText = verdict.reasons && verdict.reasons.length > 0 ? "\nReasons: " + verdict.reasons.join("; ") : ""; - - core.error("🚨 Security threats detected: " + threats.join(", ")); - if (verdict.reasons && verdict.reasons.length > 0) { - core.error(" Reasons: " + verdict.reasons.join("; ")); - } - - setDetectionFailure("threat_detected", `${ERR_VALIDATION}: ❌ Security threats detected: ${threats.join(", ")}${reasonsText}`); - } else { - core.info("✅ No security threats detected. Safe outputs may proceed."); - core.setOutput("conclusion", "success"); - core.setOutput("success", "true"); - core.setOutput("reason", ""); - } - - core.info("════════════════════════════════════════════════════════"); - core.info("🛡️ Threat detection conclusion complete."); - core.info("════════════════════════════════════════════════════════"); + // ── Step 6: Log and evaluate verdict ───────────────────────────────────── + evaluateAndReportVerdict(verdict, ""); } // end runMain } -module.exports = { main, parseDetectionLog, extractFromStreamJson, extractResultFromText }; +module.exports = { main, parseDetectionLog, extractFromStreamJson, extractResultFromText, extractStructuredOutput, parseStructuredResultFile }; diff --git a/setup/js/pi_provider.cjs b/setup/js/pi_provider.cjs index 12f5db9d..f9a83be5 100644 --- a/setup/js/pi_provider.cjs +++ b/setup/js/pi_provider.cjs @@ -28,6 +28,8 @@ "use strict"; const { fetchAWFReflect, AWF_API_PROXY_REFLECT_URL, AWF_REFLECT_OUTPUT_PATH, AWF_REFLECT_TIMEOUT_MS, AWF_MODELS_URL_TIMEOUT_MS } = require("./awf_reflect.cjs"); +const fs = require("fs"); +const path = require("path"); // Default logger: prefixed with "[gh-aw/pi-provider]" for easy grepping. // prettier-ignore @@ -138,6 +140,51 @@ function formatResponseHeaderNames(headers) { return names.length > 0 ? names.join(",") : "none"; } +/** + * Build a structured report_incomplete payload for infrastructure failures. + * + * @param {string} details + * @returns {string} + */ +function buildInfrastructureIncompletePayload(details) { + return JSON.stringify({ + type: "report_incomplete", + reason: "infrastructure_error", + details, + }); +} + +/** + * Append a report_incomplete safe output when provider infrastructure fails + * before any safe outputs have been recorded. + * + * @param {string} details + * @param {(msg: string) => void} logger + * @returns {void} + */ +function emitInfrastructureIncompleteIfNoSafeOutputs(details, logger) { + const safeOutputsPath = process.env.GH_AW_SAFE_OUTPUTS || ""; + if (!safeOutputsPath) { + logger("report_incomplete skipped: GH_AW_SAFE_OUTPUTS is not set"); + return; + } + + try { + const existing = fs.existsSync(safeOutputsPath) ? fs.readFileSync(safeOutputsPath, "utf8").trim() : ""; + if (existing) { + logger(`report_incomplete skipped: safe outputs already recorded at ${safeOutputsPath}`); + return; + } + + fs.mkdirSync(path.dirname(safeOutputsPath), { recursive: true }); + fs.appendFileSync(safeOutputsPath, buildInfrastructureIncompletePayload(details) + "\n", { encoding: "utf8" }); + logger(`report_incomplete emitted: ${safeOutputsPath}`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger(`report_incomplete emission failed: ${message}`); + } +} + /** * Log extra context when the AWF /reflect call does not produce a snapshot. * @@ -297,6 +344,7 @@ function piProviderExtension(pi) { log( `provider_error provider=${message.provider || "(unknown provider)"} model=${message.model || "(unknown model)"} api=${request.api} status=${status} method=${request.method} url=${request.url} response_headers=${responseHeaders} error=${JSON.stringify(message.errorMessage)}` ); + emitInfrastructureIncompleteIfNoSafeOutputs(`Pi provider request failed before safe outputs were emitted: ${message.errorMessage}`, log); }); pi.on("agent_start", async () => { @@ -355,4 +403,6 @@ module.exports.resolveGatewayUrl = resolveGatewayUrl; module.exports.registerConfiguredProviders = registerConfiguredProviders; module.exports.resolveProviderRequestTarget = resolveProviderRequestTarget; module.exports.formatResponseHeaderNames = formatResponseHeaderNames; +module.exports.buildInfrastructureIncompletePayload = buildInfrastructureIncompletePayload; +module.exports.emitInfrastructureIncompleteIfNoSafeOutputs = emitInfrastructureIncompleteIfNoSafeOutputs; module.exports.logReflectFailure = logReflectFailure; diff --git a/setup/js/pr_review_buffer.cjs b/setup/js/pr_review_buffer.cjs index d9291e05..efcaa71b 100644 --- a/setup/js/pr_review_buffer.cjs +++ b/setup/js/pr_review_buffer.cjs @@ -23,6 +23,7 @@ const { generateFooterWithMessages, getDetectionCautionAlert } = require("./mess const { getErrorMessage } = require("./error_helpers.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); const { generateWorkflowCallIdMarker, matchesWorkflowId } = require("./generate_footer.cjs"); +const { attachExecutionState, fetchPullRequestReviewState } = require("./safe_output_execution_metadata.cjs"); const SUPERSEDE_REVIEW_MESSAGE = "Superseded by updated review from same workflow."; const MAX_SUPERSEDE_REVIEW_PAGES = 10; @@ -244,6 +245,7 @@ function createReviewBuffer() { } const { repo, repoParts, pullRequestNumber, pullRequest } = reviewContext; + const beforeState = await fetchPullRequestReviewState(github, repoParts, pullRequestNumber); if (!pullRequest || !pullRequest.head || !pullRequest.head.sha) { core.warning("Pull request head SHA not available - cannot submit review"); @@ -508,15 +510,26 @@ function createReviewBuffer() { core.info(`Created PR review #${review.id}: ${review.html_url}`); - return { - success: true, - review_id: review.id, - review_url: review.html_url, - pull_request_number: pullRequestNumber, - repo: repo, - event: event, - comment_count: comments.length, - }; + return attachExecutionState( + { + success: true, + url: review.html_url, + number: pullRequestNumber, + review_id: review.id, + review_url: review.html_url, + pull_request_number: pullRequestNumber, + repo: repo, + event: event, + comment_count: comments.length, + metadata: { + review_id: review.id, + review_event: event, + ...(review.state ? { review_state: review.state } : {}), + }, + }, + beforeState, + await fetchPullRequestReviewState(github, repoParts, pullRequestNumber) + ); } catch (error) { const errorMessage = getErrorMessage(error); @@ -531,15 +544,26 @@ function createReviewBuffer() { const { data: review } = await github.rest.pulls.createReview(requestParams); await maybeSupersedeOlderReviews(review.id); core.info(`Created PR review #${review.id}: ${review.html_url}`); - return { - success: true, - review_id: review.id, - review_url: review.html_url, - pull_request_number: pullRequestNumber, - repo: repo, - event: "COMMENT", - comment_count: comments.length, - }; + return attachExecutionState( + { + success: true, + url: review.html_url, + number: pullRequestNumber, + review_id: review.id, + review_url: review.html_url, + pull_request_number: pullRequestNumber, + repo: repo, + event: "COMMENT", + comment_count: comments.length, + metadata: { + review_id: review.id, + review_event: "COMMENT", + ...(review.state ? { review_state: review.state } : {}), + }, + }, + beforeState, + await fetchPullRequestReviewState(github, repoParts, pullRequestNumber) + ); } catch (retryError) { core.error(`Failed to submit PR review on retry: ${getErrorMessage(retryError)}`); return { @@ -549,9 +573,10 @@ function createReviewBuffer() { } } - // When the API cannot resolve a line reference in an inline comment, retry as a body-only - // review so that the overall review (and its footer body) is still submitted successfully. - if (errorMessage.includes("Line could not be resolved") && comments.length > 0) { + // When the API cannot resolve a line or path reference in an inline comment, retry as a + // body-only review so that the overall review (and its footer body) is still submitted + // successfully. Matches both "Line could not be resolved" and "Path could not be resolved". + if ((errorMessage.includes("Line could not be resolved") || errorMessage.includes("Path could not be resolved")) && comments.length > 0) { core.warning(`PR review submission failed due to unresolvable comment line(s): ${errorMessage}. Retrying as body-only review.`); try { const bodyOnlyParams = { ...requestParams }; @@ -560,15 +585,26 @@ function createReviewBuffer() { const { data: review } = await github.rest.pulls.createReview(bodyOnlyParams); await maybeSupersedeOlderReviews(review.id); core.info(`Created PR review #${review.id} (body-only fallback): ${review.html_url}`); - return { - success: true, - review_id: review.id, - review_url: review.html_url, - pull_request_number: pullRequestNumber, - repo: repo, - event: event, - comment_count: 0, - }; + return attachExecutionState( + { + success: true, + url: review.html_url, + number: pullRequestNumber, + review_id: review.id, + review_url: review.html_url, + pull_request_number: pullRequestNumber, + repo: repo, + event: event, + comment_count: 0, + metadata: { + review_id: review.id, + review_event: event, + ...(review.state ? { review_state: review.state } : {}), + }, + }, + beforeState, + await fetchPullRequestReviewState(github, repoParts, pullRequestNumber) + ); } catch (retryError) { core.error(`Failed to submit body-only PR review: ${getErrorMessage(retryError)}`); return { diff --git a/setup/js/push_to_pull_request_branch.cjs b/setup/js/push_to_pull_request_branch.cjs index 49c5ce5f..dbe627a8 100644 --- a/setup/js/push_to_pull_request_branch.cjs +++ b/setup/js/push_to_pull_request_branch.cjs @@ -18,8 +18,10 @@ const { checkFileProtection } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplateFromFile, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs"); const { ensureFullHistoryForBundle, getGitAuthEnv, extractBundlePrerequisiteCommits, linearizeRangeAsCommit } = require("./git_helpers.cjs"); +const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs"); const { findRepoCheckout } = require("./find_repo_checkout.cjs"); const { getThreatDetectedMarker } = require("./threat_detection_warning.cjs"); +const { attachExecutionState } = require("./safe_output_execution_metadata.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -37,7 +39,6 @@ const MISSING_REMOTE_REF_PATTERNS = [ "fatal: couldn't find remote ref", "exit code 128", ]; - /** * @param {unknown} value * @returns {boolean} @@ -195,6 +196,11 @@ async function main(config = {}) { // Check if bundle or patch file exists const hasBundleFile = !!(bundleFilePath && fs.existsSync(bundleFilePath)); const hasPatchFile = !!(patchFilePath && fs.existsSync(patchFilePath)); + const applyTransport = hasBundleFile ? "bundle" : "patch"; + core.info(`Apply transport mode: ${applyTransport} (patch file present: ${hasPatchFile}, bundle file present: ${hasBundleFile})`); + if (bundleFilePath && !hasBundleFile) { + core.warning(`Bundle file path was provided but file is not present on disk: ${bundleFilePath}; falling back to patch transport`); + } // Always require a patch file for policy enforcement. Bundle is used for apply-time // transport, but allowed-files/protected-files checks must run on patch content @@ -397,6 +403,7 @@ async function main(config = {}) { let branchName; let prTitle = ""; let prLabels = []; + let branchStateBefore = null; if (!pullNumber) { return { success: false, error: "Pull request number is required but not found" }; @@ -445,6 +452,9 @@ async function main(config = {}) { branchName = pullRequest.head.ref; prTitle = pullRequest.title || ""; prLabels = pullRequest.labels.map(label => label.name); + if (typeof pullRequest.head?.sha === "string" && pullRequest.head.sha) { + branchStateBefore = { head_sha: pullRequest.head.sha }; + } } catch (error) { core.info(`Warning: Could not fetch PR ${pullNumber} from ${itemRepo}: ${getErrorMessage(error)}`); return { success: false, error: `Failed to determine branch name for PR ${pullNumber} in ${itemRepo}` }; @@ -552,12 +562,16 @@ async function main(config = {}) { ); core.info(`Created manifest-protection review issue #${issue.number}: ${issue.html_url}`); await updateActivationComment(github, context, core, issue.html_url, issue.number, "issue"); - return { - success: true, - fallback_used: true, - issue_number: issue.number, - issue_url: issue.html_url, - }; + return attachExecutionState( + { + success: true, + fallback_used: true, + issue_number: issue.number, + issue_url: issue.html_url, + }, + branchStateBefore, + branchStateBefore + ); } catch (issueError) { const error = `Manifest file protection: failed to create review issue. Error: ${issueError instanceof Error ? issueError.message : String(issueError)}`; core.error(error); @@ -650,15 +664,55 @@ async function main(config = {}) { let newCommitCount = 0; let remoteHeadBeforePatch = ""; let pushedCommitSha = ""; + let rangeBaseRef = `origin/${branchName}`; if (hasChanges) { // Capture HEAD before applying changes to compute new-commit count later try { const { stdout } = await exec.getExecOutput("git", ["rev-parse", "HEAD"], baseGitOpts); remoteHeadBeforePatch = stdout.trim(); + if (remoteHeadBeforePatch) { + rangeBaseRef = remoteHeadBeforePatch; + core.info(`Remote branch HEAD before apply: ${remoteHeadBeforePatch}`); + } } catch { // Non-fatal - extra empty commit will be skipped } + // Pin patch application to the recorded base commit captured at patch-generation time. + // This avoids applying a patch generated from an older branch tip onto a newer remote tip. + // If the commit is unavailable (e.g. cross-repo/missing object), continue with current HEAD. + if (!hasBundleFile && message.base_commit) { + const recordedBaseCommit = normalizeCommitSHA(message.base_commit); + if (recordedBaseCommit) { + core.info(`Patch route base_commit resolved: ${recordedBaseCommit}`); + try { + try { + await exec.exec("git", ["fetch", "origin", recordedBaseCommit, "--depth=1"], { + env: { ...process.env, ...gitAuthEnv }, + ...baseGitOpts, + }); + } catch (fetchError) { + core.info(`Note: could not fetch base_commit ${recordedBaseCommit} explicitly (${getErrorMessage(fetchError)}); will verify local availability next`); + } + await exec.exec("git", ["cat-file", "-e", recordedBaseCommit], baseGitOpts); + const ancestryCheck = await exec.getExecOutput("git", ["merge-base", "--is-ancestor", recordedBaseCommit, `origin/${branchName}`], { ...baseGitOpts, ignoreReturnCode: true }); + if (ancestryCheck.exitCode !== 0) { + throw new Error(`recorded base_commit ${recordedBaseCommit} is not an ancestor of origin/${branchName}; cannot safely re-anchor patch apply`); + } + if (remoteHeadBeforePatch && remoteHeadBeforePatch !== recordedBaseCommit) { + core.warning(`Remote PR branch advanced since patch generation (remote HEAD ${remoteHeadBeforePatch}, patch base ${recordedBaseCommit}); applying patch from recorded base commit`); + } + await exec.exec("git", ["reset", "--hard", recordedBaseCommit], baseGitOpts); + rangeBaseRef = recordedBaseCommit; + core.info(`Reset branch to recorded base_commit before patch apply: ${recordedBaseCommit}`); + } catch (baseCommitError) { + core.warning(`Unable to use recorded base_commit ${recordedBaseCommit}; applying patch on current branch HEAD: ${getErrorMessage(baseCommitError)}`); + } + } else if (String(message.base_commit).trim()) { + core.warning(`Ignoring invalid base_commit value for patch apply: ${String(message.base_commit).trim()}`); + } + } + if (hasBundleFile) { // Bundle transport: fetch commits directly from the bundle file. // This preserves merge commit topology and per-commit metadata. @@ -683,7 +737,7 @@ async function main(config = {}) { // commit objects, then retry with the original bundle ref. // This handles the race where main advanced between agent-time and safe_outputs-time: // the bundle's base commit may not be reachable from a fetch-depth:1 shallow clone - // even after --unshallow (e.g. when the commit is on a ref not in the fetch refspec). + // (e.g. when the commit is on a ref not in the fetch refspec). const prerequisiteCommits = extractBundlePrerequisiteCommits(initialFetchErrorOutput); if (prerequisiteCommits.length > 0) { core.warning(`Bundle fetch failed due to ${prerequisiteCommits.length} missing prerequisite commit(s); fetching prerequisites from origin and retrying`); @@ -702,13 +756,14 @@ async function main(config = {}) { // checkouts, merge --ff-only can fail to discover the ancestry even // when the bundle tip is based on the current branch tip and the // prerequisite exists locally. + core.info(`Updating local branch ref refs/heads/${branchName} to ${bundleRef} (expected previous tip: ${remoteHeadBeforePatch || "unknown"})`); const updateRefArgs = ["update-ref", `refs/heads/${branchName}`, bundleRef]; if (remoteHeadBeforePatch) { updateRefArgs.push(remoteHeadBeforePatch); } await exec.exec("git", updateRefArgs, baseGitOpts); await exec.exec("git", ["reset", "--hard"], baseGitOpts); - core.info("Updated branch to bundle tip"); + core.info(`Updated branch to bundle tip from ${bundleRef}`); // Clean up the temporary ref try { @@ -832,6 +887,7 @@ async function main(config = {}) { } } } // end else (patch path) + core.info(`Apply transport completed; signed-push base ref: ${rangeBaseRef}`); // When threat detection produced a warning, create a review PR instead of pushing // directly to the existing PR branch. This allows manual review of the changes @@ -919,7 +975,7 @@ async function main(config = {}) { // be signed. A warning is emitted so workflow authors and agents know that rebase // should be preferred over merge in future runs. if (signedCommits && hasChanges) { - const squashBase = remoteHeadBeforePatch || `origin/${branchName}`; + const squashBase = rangeBaseRef; try { const { stdout: mergeCountOut } = await exec.getExecOutput("git", ["rev-list", "--merges", "--count", `${squashBase}..HEAD`], baseGitOpts); const mergeCount = parseInt(mergeCountOut.trim(), 10); @@ -954,7 +1010,7 @@ async function main(config = {}) { owner: repoParts.owner, repo: repoParts.repo, branch: branchName, - baseRef: remoteHeadBeforePatch || `origin/${branchName}`, + baseRef: rangeBaseRef, cwd: repoCwd || process.cwd(), gitAuthEnv, signedCommits, @@ -1139,12 +1195,19 @@ async function main(config = {}) { } } - return { - success: true, - branch_name: branchName, - commit_sha: commitSha, - commit_url: commitUrl, - }; + return attachExecutionState( + { + success: true, + number: pullNumber, + repo: itemRepo, + url: `${repoUrl}/pull/${pullNumber}`, + branch_name: branchName, + commit_sha: commitSha, + commit_url: commitUrl, + }, + branchStateBefore || (remoteHeadBeforePatch ? { head_sha: remoteHeadBeforePatch } : null), + commitSha ? { head_sha: commitSha } : null + ); }; } diff --git a/setup/js/redact_secrets.cjs b/setup/js/redact_secrets.cjs index b456b36f..98080c26 100644 --- a/setup/js/redact_secrets.cjs +++ b/setup/js/redact_secrets.cjs @@ -49,9 +49,10 @@ function findFiles(dir, extensions) { const BUILT_IN_PATTERNS = [ // GitHub tokens { name: "GitHub Personal Access Token (classic)", pattern: /ghp_[0-9a-zA-Z]{36}/g }, - // Match both legacy 36-character installation tokens and the newer long - // JWT-like stateless tokens with 3+ dot-separated base64url-ish segments. - { name: "GitHub Server-to-Server Token", pattern: /ghs_(?:[0-9a-zA-Z]{36}(?![0-9A-Za-z._-])|[0-9A-Za-z_-]{10,}(?:\.[0-9A-Za-z_-]{10,}){2,})/g }, + // New stateless ghs_ token format allows dots, underscores, and dashes + // and uses variable length (minimum 36 chars after prefix). + // https://github.blog/changelog/2026-05-15-github-app-installation-tokens-per-request-override-header/ + { name: "GitHub Server-to-Server Token", pattern: /ghs_[0-9A-Za-z._-]{36,}/g }, { name: "GitHub OAuth Access Token", pattern: /gho_[0-9a-zA-Z]{36}/g }, { name: "GitHub User Access Token", pattern: /ghu_[0-9a-zA-Z]{36}/g }, { name: "GitHub Fine-grained PAT", pattern: /github_pat_[0-9a-zA-Z_]{82}/g }, diff --git a/setup/js/render_template.cjs b/setup/js/render_template.cjs index e87d12d8..162a07c9 100644 --- a/setup/js/render_template.cjs +++ b/setup/js/render_template.cjs @@ -13,6 +13,14 @@ const { ERR_API, ERR_CONFIG } = require("./error_codes.cjs"); const { isTruthy } = require("./is_truthy.cjs"); const { selectBranch } = require("./template_branch.cjs"); +// Closing tag: {{#endif}} (primary) or {{/if}} (alternate) +const BLOCK_CONDITIONAL_REGEX = /(\n?)([ \t]*{{#if\s+([^}]*)}}[ \t]*\n)([\s\S]*?)([ \t]*(?:{{#endif}}|{{\/if}})[ \t]*)(\n?)/; +const INLINE_CONDITIONAL_REGEX = /{{#if\s+([^}]*)}}([\s\S]*?)(?:{{#endif}}|{{\/if}})/; + +// Max characters shown in body preview log lines +const BLOCK_BODY_PREVIEW_LENGTH = 60; +const INLINE_BODY_PREVIEW_LENGTH = 40; + /** * Renders a Markdown template by processing {{#if}} conditional blocks. * When a conditional block is removed (falsy condition) and the template tags @@ -37,8 +45,8 @@ function renderMarkdownTemplate(markdown) { } // Count conditionals before processing - const blockConditionals = (_stripped.match(/(\n?)([ \t]*{{#if\s+(.*?)\s*}}[ \t]*\n)([\s\S]*?)([ \t]*{{\/if}}[ \t]*)(\n?)/g) || []).length; - const inlineConditionals = (_stripped.match(/{{#if\s+(.*?)\s*}}([\s\S]*?){{\/if}}/g) || []).length - blockConditionals; + const blockConditionals = (_stripped.match(new RegExp(BLOCK_CONDITIONAL_REGEX.source, "g")) || []).length; + const inlineConditionals = (_stripped.match(new RegExp(INLINE_CONDITIONAL_REGEX.source, "g")) || []).length - blockConditionals; core.info(`[renderMarkdownTemplate] Found ${blockConditionals} block conditional(s) and ${inlineConditionals} inline conditional(s)`); @@ -48,19 +56,23 @@ function renderMarkdownTemplate(markdown) { // First pass: Handle blocks where tags are on their own lines // Captures: (leading newline)(opening tag line)(condition)(body)(closing tag line)(trailing newline) - // Uses .*? (non-greedy) with \s* to handle expressions with or without trailing spaces - let result = _stripped.replace(/(\n?)([ \t]*{{#if\s+(.*?)\s*}}[ \t]*\n)([\s\S]*?)([ \t]*{{\/if}}[ \t]*)(\n?)/g, (match, leadNL, openLine, cond, body) => { + let result = _stripped.replace(new RegExp(BLOCK_CONDITIONAL_REGEX.source, "g"), (match, leadNL, openLine, cond, body) => { blockCount++; + const condTrimmed = cond.trim(); + const bodyPreview = body.substring(0, BLOCK_BODY_PREVIEW_LENGTH).replace(/\n/g, "\\n"); - core.info(`[renderMarkdownTemplate] Block ${blockCount}: condition="${cond.trim()}" -> evaluating branches`); + core.info(`[renderMarkdownTemplate] Block ${blockCount}: condition="${condTrimmed}" -> evaluating branches`); + core.info(`[renderMarkdownTemplate] Body preview: "${bodyPreview}${body.length > BLOCK_BODY_PREVIEW_LENGTH ? "..." : ""}"`); const selectedContent = selectBranch(cond, body); if (selectedContent !== null) { keptBlocks++; + core.info(`[renderMarkdownTemplate] Action: Keeping selected branch with leading newline=${!!leadNL}`); return leadNL + selectedContent; } else { removedBlocks++; + core.info(`[renderMarkdownTemplate] Action: Removing entire block`); return ""; } }); @@ -72,12 +84,14 @@ function renderMarkdownTemplate(markdown) { let removedInline = 0; // Second pass: Handle inline conditionals (tags not on their own lines) - // Uses .*? (non-greedy) with \s* to handle expressions with or without trailing spaces - result = result.replace(/{{#if\s+(.*?)\s*}}([\s\S]*?){{\/if}}/g, (_, cond, body) => { + result = result.replace(new RegExp(INLINE_CONDITIONAL_REGEX.source, "g"), (_, cond, body) => { inlineCount++; + const condTrimmed = cond.trim(); + const bodyPreview = body.substring(0, INLINE_BODY_PREVIEW_LENGTH).replace(/\n/g, "\\n"); const selectedContent = selectBranch(cond, body); - core.info(`[renderMarkdownTemplate] Inline ${inlineCount}: condition="${cond.trim()}" -> ${selectedContent !== null ? "KEEP" : "REMOVE"}`); + core.info(`[renderMarkdownTemplate] Inline ${inlineCount}: condition="${condTrimmed}" -> ${selectedContent !== null ? "KEEP" : "REMOVE"}`); + core.info(`[renderMarkdownTemplate] Body preview: "${bodyPreview}${body.length > INLINE_BODY_PREVIEW_LENGTH ? "..." : ""}"`); if (selectedContent !== null) { keptInline++; @@ -91,17 +105,28 @@ function renderMarkdownTemplate(markdown) { core.info(`[renderMarkdownTemplate] Second pass complete: ${keptInline} kept, ${removedInline} removed`); // Clean up excessive blank lines (more than one blank line = 2 newlines) + const beforeCleanup = result.length; + const excessiveLines = (result.match(/\n{3,}/g) || []).length; result = result.replace(/\n{3,}/g, "\n\n"); + if (excessiveLines > 0) { + core.info(`[renderMarkdownTemplate] Cleaned up ${excessiveLines} excessive blank line sequence(s)`); + core.info(`[renderMarkdownTemplate] Length change from cleanup: ${beforeCleanup} -> ${result.length} characters`); + } + + // Count which placeholders survived to detect code blocks removed in false conditional branches + const _survivedIndices = new Set([...result.matchAll(/\x00FENCE\x00(\d+)\x00FENCE\x00/g)].map(m => +m[1])); + const _removedFenceMarkerCount = _codeBlocks.reduce((sum, block, i) => (_survivedIndices.has(i) ? sum : sum + (block.match(/`{3,}/g) || []).length), 0); // Restore fenced code blocks if (_codeBlocks.length > 0) { result = result.replace(/\x00FENCE\x00(\d+)\x00FENCE\x00/g, (_, i) => _codeBlocks[+i]); } - // Runtime assertion: number of fence markers must be the same before and after processing + // Runtime assertion: number of fence markers must be the same before and after processing, + // accounting for any fenced code blocks intentionally removed inside false conditional branches. const _inputFenceCount = (markdown.match(/`{3,}/g) || []).length; const _outputFenceCount = (result.match(/`{3,}/g) || []).length; - if (_inputFenceCount !== _outputFenceCount) { + if (_inputFenceCount - _removedFenceMarkerCount !== _outputFenceCount) { core.warning(`[renderMarkdownTemplate] Fence count mismatch: input had ${_inputFenceCount} fence marker(s), output has ${_outputFenceCount}`); } diff --git a/setup/js/safe_output_execution_metadata.cjs b/setup/js/safe_output_execution_metadata.cjs new file mode 100644 index 00000000..3af7cac3 --- /dev/null +++ b/setup/js/safe_output_execution_metadata.cjs @@ -0,0 +1,252 @@ +// @ts-check + +const crypto = require("crypto"); + +function normalizeBodyForHash(body) { + const normalized = String(body || "").replace(/\r\n/g, "\n"); + return normalized + .split("\n") + .map(line => line.replace(/[ \t]+$/g, "")) + .join("\n") + .trim(); +} + +function hashNormalizedBody(body) { + return crypto.createHash("sha256").update(normalizeBodyForHash(body), "utf8").digest("hex"); +} + +function normalizeLabelNames(labels) { + if (!Array.isArray(labels)) { + return []; + } + return labels + .map(label => { + if (typeof label === "string") { + return label; + } + if (label && typeof label.name === "string") { + return label.name; + } + return ""; + }) + .filter(Boolean); +} + +function normalizeAssigneeLogins(assignees) { + if (!Array.isArray(assignees)) { + return []; + } + return assignees + .map(assignee => { + if (typeof assignee === "string") { + return assignee; + } + if (assignee && typeof assignee.login === "string") { + return assignee.login; + } + return ""; + }) + .filter(Boolean); +} + +function normalizeRequestedReviewers(reviewers) { + if (!Array.isArray(reviewers)) { + return []; + } + return reviewers + .map(reviewer => { + if (typeof reviewer === "string") { + return reviewer; + } + if (reviewer && typeof reviewer.login === "string") { + return reviewer.login; + } + return ""; + }) + .filter(Boolean); +} + +function normalizeRequestedTeams(teams) { + if (!Array.isArray(teams)) { + return []; + } + return teams + .map(team => { + if (typeof team === "string") { + return team; + } + if (team && typeof team.slug === "string") { + return team.slug; + } + if (team && typeof team.name === "string") { + return team.name; + } + return ""; + }) + .filter(Boolean); +} + +function normalizeReviews(reviews) { + if (!Array.isArray(reviews)) { + return []; + } + return reviews + .map(review => ({ + ...(review?.id != null ? { id: review.id } : {}), + ...(review?.user?.login ? { user: review.user.login } : {}), + ...(review?.state ? { state: review.state } : {}), + ...(review?.submitted_at ? { submitted_at: review.submitted_at } : {}), + })) + .filter(review => Object.keys(review).length > 0); +} + +function extractIssueStateFromData(issue) { + return { + title: typeof issue?.title === "string" ? issue.title : "", + body_hash: hashNormalizedBody(issue?.body), + state: typeof issue?.state === "string" ? issue.state : "", + labels: normalizeLabelNames(issue?.labels), + assignees: normalizeAssigneeLogins(issue?.assignees), + }; +} + +function mergeIssueState(baseState, issue) { + const nextState = { + title: "", + body_hash: "", + state: "", + labels: [], + assignees: [], + ...(baseState || {}), + }; + if (!issue || typeof issue !== "object") { + return nextState; + } + if ("title" in issue && typeof issue.title === "string") { + nextState.title = issue.title; + } + if ("body" in issue) { + nextState.body_hash = hashNormalizedBody(issue.body); + } + if ("state" in issue && typeof issue.state === "string") { + nextState.state = issue.state; + } + if ("labels" in issue) { + nextState.labels = normalizeLabelNames(issue.labels); + } + if ("assignees" in issue) { + nextState.assignees = normalizeAssigneeLogins(issue.assignees); + } + return nextState; +} + +function extractPullRequestStateFromData(pullRequest) { + return { + title: typeof pullRequest?.title === "string" ? pullRequest.title : "", + body_hash: hashNormalizedBody(pullRequest?.body), + state: typeof pullRequest?.state === "string" ? pullRequest.state : "", + base: typeof pullRequest?.base?.ref === "string" ? pullRequest.base.ref : "", + draft: pullRequest?.draft === true, + head_sha: typeof pullRequest?.head?.sha === "string" ? pullRequest.head.sha : "", + }; +} + +function mergePullRequestState(baseState, pullRequest) { + const nextState = { + title: "", + body_hash: "", + state: "", + base: "", + draft: false, + head_sha: "", + ...(baseState || {}), + }; + if (!pullRequest || typeof pullRequest !== "object") { + return nextState; + } + if ("title" in pullRequest && typeof pullRequest.title === "string") { + nextState.title = pullRequest.title; + } + if ("body" in pullRequest) { + nextState.body_hash = hashNormalizedBody(pullRequest.body); + } + if ("state" in pullRequest && typeof pullRequest.state === "string") { + nextState.state = pullRequest.state; + } + if (pullRequest.base && typeof pullRequest.base.ref === "string") { + nextState.base = pullRequest.base.ref; + } + if ("draft" in pullRequest) { + nextState.draft = pullRequest.draft === true; + } + if (pullRequest.head && typeof pullRequest.head.sha === "string") { + nextState.head_sha = pullRequest.head.sha; + } + return nextState; +} + +async function fetchIssueState(github, repoParts, issueNumber) { + const { data: issue } = await github.rest.issues.get({ + owner: repoParts.owner, + repo: repoParts.repo, + issue_number: issueNumber, + }); + return extractIssueStateFromData(issue); +} + +function extractReviewStateFromData(pullRequest, reviews) { + return { + requested_reviewers: normalizeRequestedReviewers(pullRequest?.requested_reviewers), + requested_team_reviewers: normalizeRequestedTeams(pullRequest?.requested_teams ?? pullRequest?.requested_team_reviewers), + reviews: normalizeReviews(reviews), + }; +} + +async function fetchPullRequestReviewState(github, repoParts, pullRequestNumber) { + const [{ data: pullRequest }, { data: reviews }] = await Promise.all([ + github.rest.pulls.get({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + }), + github.rest.pulls.listReviews({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + per_page: 100, + }), + ]); + return extractReviewStateFromData(pullRequest, reviews); +} + +async function fetchPullRequestState(github, repoParts, pullRequestNumber) { + const { data: pullRequest } = await github.rest.pulls.get({ + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + }); + return extractPullRequestStateFromData(pullRequest); +} + +function attachExecutionState(result, beforeState, afterState) { + return { + ...result, + ...(beforeState ? { before_state: beforeState } : {}), + ...(afterState ? { after_state: afterState } : {}), + }; +} + +module.exports = { + attachExecutionState, + extractIssueStateFromData, + extractPullRequestStateFromData, + extractReviewStateFromData, + fetchIssueState, + fetchPullRequestState, + fetchPullRequestReviewState, + hashNormalizedBody, + mergePullRequestState, + mergeIssueState, + normalizeBodyForHash, + normalizeLabelNames, +}; diff --git a/setup/js/safe_output_handler_manager.cjs b/setup/js/safe_output_handler_manager.cjs index fe9eed58..f3dd28bd 100644 --- a/setup/js/safe_output_handler_manager.cjs +++ b/setup/js/safe_output_handler_manager.cjs @@ -492,6 +492,27 @@ function formatManifestLogMessage(item) { return `📝 Manifest: logged ${item.type}`; } +function logCreatedItemFromResult(onItemCreated, messageType, result) { + if (!onItemCreated) { + return; + } + if (Array.isArray(result)) { + for (const item of result) { + const createdItem = extractCreatedItemFromResult(messageType, item); + if (createdItem) { + core.info(formatManifestLogMessage(createdItem)); + onItemCreated(createdItem); + } + } + return; + } + const createdItem = extractCreatedItemFromResult(messageType, result); + if (createdItem) { + core.info(formatManifestLogMessage(createdItem)); + onItemCreated(createdItem); + } +} + /** * Retroactively mark buffered review results as failed when the finalization POST fails. * Both submit_pull_request_review and create_pull_request_review_comment return @@ -863,23 +884,7 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) }); // Log to manifest if this was a create operation - if (onItemCreated) { - if (Array.isArray(result)) { - for (const item of result) { - const createdItem = extractCreatedItemFromResult(messageType, item); - if (createdItem) { - core.info(formatManifestLogMessage(createdItem)); - onItemCreated(createdItem); - } - } - } else { - const createdItem = extractCreatedItemFromResult(messageType, result); - if (createdItem) { - core.info(formatManifestLogMessage(createdItem)); - onItemCreated(createdItem); - } - } - } + logCreatedItemFromResult(onItemCreated, messageType, result); core.info(`✓ Message ${i + 1} (${messageType}) completed successfully`); } catch (error) { @@ -981,13 +986,7 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) } // Log to manifest after deferred retry success - if (onItemCreated) { - const createdItem = extractCreatedItemFromResult(deferred.type, result); - if (createdItem) { - core.info(formatManifestLogMessage(createdItem)); - onItemCreated(createdItem); - } - } + logCreatedItemFromResult(onItemCreated, deferred.type, result); } } catch (error) { core.error(`✗ Retry of message ${deferred.messageIndex + 1} (${deferred.type}) failed: ${getErrorMessage(error)}`); @@ -1376,6 +1375,7 @@ async function main() { try { const reviewResult = await prReviewBuffer.submitReview(); if (reviewResult.success && !reviewResult.skipped) { + logCreatedItemFromResult(logCreatedItem, "submit_pull_request_review", reviewResult); core.info(`✓ PR review submitted successfully: ${reviewResult.review_url}`); } else if (!reviewResult.success) { reviewFailureError = reviewResult.error || "PR review finalization failed"; @@ -1574,4 +1574,4 @@ async function main() { } } -module.exports = { main, loadConfig, loadHandlers, processMessages, buildCommentMemoryMessagesFromFiles, rollbackReviewResults }; +module.exports = { main, loadConfig, loadHandlers, processMessages, buildCommentMemoryMessagesFromFiles, rollbackReviewResults, logCreatedItemFromResult }; diff --git a/setup/js/safe_output_manifest.cjs b/setup/js/safe_output_manifest.cjs index c00a1bce..500b7f72 100644 --- a/setup/js/safe_output_manifest.cjs +++ b/setup/js/safe_output_manifest.cjs @@ -47,6 +47,10 @@ const NOT_LOGGED_TYPES = new Set(["noop", "missing_tool", "missing_data", "repor * @property {number} [number] - Issue/PR/discussion number if applicable * @property {string} [repo] - Repository slug (owner/repo) if applicable * @property {string} [temporaryId] - Temporary ID assigned to this item, if any + * @property {Record} [metadata] - Persisted outcome metadata captured at execution time + * @property {Object} [before_state] - Execution-time state snapshot captured before mutation + * @property {Object} [after_state] - Execution-time state snapshot captured after mutation + * @property {string[]} [labelsAdded] - Labels added by add_labels handler * @property {string} timestamp - ISO 8601 timestamp of creation */ @@ -57,7 +61,7 @@ const NOT_LOGGED_TYPES = new Set(["noop", "missing_tool", "missing_data", "repor * It is designed to be easily testable by accepting the file path as a parameter. * * @param {string} [manifestFile] - Path to the manifest file (defaults to MANIFEST_FILE_PATH) - * @returns {(item: {type: string, url?: string, number?: number, repo?: string, temporaryId?: string}) => void} Logger function + * @returns {(item: {type: string, url?: string, number?: number, repo?: string, temporaryId?: string, metadata?: Record, before_state?: Object, after_state?: Object, labelsAdded?: string[], labelsBefore?: string[]}) => void} Logger function */ function createManifestLogger(manifestFile = MANIFEST_FILE_PATH) { // Touch the file immediately so it exists for artifact upload @@ -67,7 +71,7 @@ function createManifestLogger(manifestFile = MANIFEST_FILE_PATH) { /** * Log an executed safe output item to the manifest file. * - * @param {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}} item - Executed item details + * @param {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string, metadata?: Record, before_state?: Object, after_state?: Object, labelsAdded?: string[], labelsBefore?: string[]}} item - Executed item details */ return function logCreatedItem(item) { if (!item) return; @@ -79,6 +83,11 @@ function createManifestLogger(manifestFile = MANIFEST_FILE_PATH) { ...(item.number != null ? { number: item.number } : {}), ...(item.repo ? { repo: item.repo } : {}), ...(item.temporaryId ? { temporaryId: item.temporaryId } : {}), + ...(item.metadata && Object.keys(item.metadata).length > 0 ? { metadata: item.metadata } : {}), + ...(item.before_state ? { before_state: item.before_state } : {}), + ...(item.after_state ? { after_state: item.after_state } : {}), + ...(Array.isArray(item.labelsAdded) ? { labelsAdded: item.labelsAdded } : {}), + ...(Array.isArray(item.labelsBefore) ? { labelsBefore: item.labelsBefore } : {}), timestamp: new Date().toISOString(), }; @@ -120,23 +129,35 @@ function ensureManifestExists(manifestFile = MANIFEST_FILE_PATH) { * * @param {string} type - The handler type (e.g., "create_issue") * @param {any} result - The handler result object - * @returns {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}|null} + * @returns {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string, metadata?: Record, before_state?: Object, after_state?: Object, labelsAdded?: string[], labelsBefore?: string[]}|null} */ function extractCreatedItemFromResult(type, result) { if (!result || NOT_LOGGED_TYPES.has(type)) return null; + // PR reviews are buffered first and only gain durable identity fields after the + // final submitReview() call, so skip logging placeholder buffer results here. + if (type === "submit_pull_request_review" && !result.review_url && !result.pull_request_number && !result.repo) { + return null; + } + // In staged mode (🎭 Staged Mode Preview), no item was actually modified in GitHub — skip logging - if (result.staged === true) return null; + if (result.staged === true || result.skipped === true || result.deferred_manifest === true) return null; // Normalize URL from different result shapes (present for creation types) - const url = result.url || result.projectUrl || result.html_url; + const url = result.url || result.projectUrl || result.html_url || result.pull_request_url || result.review_url || result.issue_url; + const number = result.number ?? result.pull_request_number ?? result.prNumber ?? result.issue_number ?? result.itemNumber; return { type, ...(url ? { url } : {}), - ...(result.number != null ? { number: result.number } : {}), + ...(number != null ? { number } : {}), ...(result.repo ? { repo: result.repo } : {}), ...(result.temporaryId ? { temporaryId: result.temporaryId } : {}), + ...(result.metadata && Object.keys(result.metadata).length > 0 ? { metadata: result.metadata } : {}), + ...(result.before_state ? { before_state: result.before_state } : {}), + ...(result.after_state ? { after_state: result.after_state } : {}), + ...(Array.isArray(result.labelsAdded) ? { labelsAdded: result.labelsAdded } : {}), + ...(Array.isArray(result.labelsBefore) ? { labelsBefore: result.labelsBefore } : {}), }; } diff --git a/setup/js/safe_outputs_handlers.cjs b/setup/js/safe_outputs_handlers.cjs index ba942cbf..190d5018 100644 --- a/setup/js/safe_outputs_handlers.cjs +++ b/setup/js/safe_outputs_handlers.cjs @@ -96,6 +96,31 @@ function isAllowedBranch(branch, allowedPatterns) { return false; } +/** + * Resolve and validate a workspace-relative patch path. + * @param {string|undefined} workspacePath + * @returns {{success: true, absolutePath: string} | {success: false, error: string}} + */ +function resolvePatchWorkspacePath(workspacePath) { + const candidatePath = typeof workspacePath === "string" ? workspacePath.trim() : ""; + if (!candidatePath) { + return { success: false, error: "patch_workspace_path is empty" }; + } + const workspaceRoot = path.resolve(process.env.GITHUB_WORKSPACE || process.cwd()); + const resolved = path.resolve(workspaceRoot, candidatePath); + const relative = path.relative(workspaceRoot, resolved); + if (relative.startsWith("..") || path.isAbsolute(relative)) { + return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': path must stay under GITHUB_WORKSPACE` }; + } + if (!fs.existsSync(resolved)) { + return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': directory does not exist` }; + } + if (!fs.statSync(resolved).isDirectory()) { + return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': path is not a directory` }; + } + return { success: true, absolutePath: resolved }; +} + /** * Create handlers for safe output tools * @param {Object} server - The MCP server instance for logging @@ -341,8 +366,32 @@ function createHandlers(server, appendSafeOutput, config = {}) { // If repo is specified or configured, find where it's checked out let repoCwd = null; let repoSlug = null; + const patchWorkspacePath = typeof prConfig.patch_workspace_path === "string" ? prConfig.patch_workspace_path.trim() : ""; + const currentCheckoutRepo = typeof prConfig.current_checkout_repo === "string" ? prConfig.current_checkout_repo.trim() : ""; + const patchWorkspaceMatchesTargetRepo = patchWorkspacePath && (!currentCheckoutRepo || currentCheckoutRepo === repoResult.repo); - if ((entry.repo && entry.repo.trim()) || prConfig["target-repo"]) { + if (patchWorkspaceMatchesTargetRepo) { + const patchWorkspaceResult = resolvePatchWorkspacePath(patchWorkspacePath); + if (!patchWorkspaceResult.success) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: patchWorkspaceResult.error, + }), + }, + ], + isError: true, + }; + } + repoCwd = patchWorkspaceResult.absolutePath; + repoSlug = repoResult.repo; + server.debug(`Using configured patch_workspace_path for create_pull_request: ${patchWorkspacePath} -> ${repoCwd}`); + } + + if (((entry.repo && entry.repo.trim()) || prConfig["target-repo"]) && !repoCwd) { // Use the validated/qualified repo slug from repoResult to avoid divergence // between the raw user input and the normalized/qualified repo name repoSlug = repoResult.repo; @@ -510,6 +559,9 @@ function createHandlers(server, appendSafeOutput, config = {}) { server.debug(`Generating patch for create_pull_request with branch: ${entry.branch}${repoCwd ? ` in ${repoCwd} baseBranch: ${baseBranch}` : ""}`); /** @type {Record} */ const patchOptions = { ...transportOptions }; + if (patchWorkspaceMatchesTargetRepo) { + patchOptions.workspacePath = patchWorkspacePath; + } // Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time. if (Array.isArray(prConfig.excluded_files) && prConfig.excluded_files.length > 0) { patchOptions.excludedFiles = prConfig.excluded_files; @@ -668,7 +720,32 @@ function createHandlers(server, appendSafeOutput, config = {}) { // generation runs from the correct directory when the target repo is checked out in a subdirectory. let repoCwd = null; const itemRepo = repoResult.repo; - if ((entry.repo && entry.repo.trim()) || pushConfig["target-repo"]) { + const pushPatchWorkspacePath = typeof pushConfig.patch_workspace_path === "string" ? pushConfig.patch_workspace_path.trim() : ""; + const pushCurrentCheckoutRepo = typeof pushConfig.current_checkout_repo === "string" ? pushConfig.current_checkout_repo.trim() : ""; + const pushPatchWorkspaceMatchesTargetRepo = pushPatchWorkspacePath && (!pushCurrentCheckoutRepo || pushCurrentCheckoutRepo === itemRepo); + + if (pushPatchWorkspaceMatchesTargetRepo) { + const patchWorkspaceResult = resolvePatchWorkspacePath(pushPatchWorkspacePath); + if (!patchWorkspaceResult.success) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: patchWorkspaceResult.error, + }), + }, + ], + isError: true, + }; + } + repoCwd = patchWorkspaceResult.absolutePath; + entry.repo_cwd = repoCwd; + server.debug(`Using configured patch_workspace_path for push_to_pull_request_branch: ${pushPatchWorkspacePath} -> ${repoCwd}`); + } + + if (((entry.repo && entry.repo.trim()) || pushConfig["target-repo"]) && !repoCwd) { server.debug(`Looking for checkout of target repo: ${itemRepo}`); const checkoutResult = findRepoCheckout(itemRepo); if (!checkoutResult.success) { @@ -804,6 +881,9 @@ function createHandlers(server, appendSafeOutput, config = {}) { server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`); /** @type {Record} */ const pushPatchOptions = { ...pushTransportOptions }; + if (pushPatchWorkspaceMatchesTargetRepo) { + pushPatchOptions.workspacePath = pushPatchWorkspacePath; + } // Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time. if (Array.isArray(pushConfig.excluded_files) && pushConfig.excluded_files.length > 0) { pushPatchOptions.excludedFiles = pushConfig.excluded_files; diff --git a/setup/js/safe_outputs_mcp_server.cjs b/setup/js/safe_outputs_mcp_server.cjs index 48c390ee..ddfb107d 100644 --- a/setup/js/safe_outputs_mcp_server.cjs +++ b/setup/js/safe_outputs_mcp_server.cjs @@ -45,7 +45,7 @@ function startSafeOutputsServer(options = {}) { const { defaultHandler } = handlers; // Attach handlers to tools - const toolsWithHandlers = attachHandlers(ALL_TOOLS, handlers); + const toolsWithHandlers = attachHandlers(ALL_TOOLS, handlers, server); server.debug(` output file: ${outputFile}`); server.debug(` config: ${JSON.stringify(safeOutputsConfig)}`); diff --git a/setup/js/safe_outputs_mcp_server_http.cjs b/setup/js/safe_outputs_mcp_server_http.cjs index 3257ebf3..6e5643e4 100644 --- a/setup/js/safe_outputs_mcp_server_http.cjs +++ b/setup/js/safe_outputs_mcp_server_http.cjs @@ -92,7 +92,7 @@ function createMCPServer(options = {}) { const { defaultHandler } = handlers; // Attach handlers to tools - const toolsWithHandlers = attachHandlers(ALL_TOOLS, handlers); + const toolsWithHandlers = attachHandlers(ALL_TOOLS, handlers, logger); // Register predefined tools that are enabled in configuration logger.debug(`Registering predefined tools...`); diff --git a/setup/js/safe_outputs_tools.json b/setup/js/safe_outputs_tools.json index 30407052..27c34bda 100644 --- a/setup/js/safe_outputs_tools.json +++ b/setup/js/safe_outputs_tools.json @@ -387,13 +387,13 @@ }, { "name": "submit_pull_request_review", - "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Preferred default: use COMMENT for informative, non-blocking bot feedback. Use REQUEST_CHANGES only when merge-blocking is intentionally required. If you don't call this tool, review comments are still submitted as a COMMENT review.", + "description": "Submit a pull request review with a status decision. REQUIRED: every call must include either a non-empty body or be preceded by at least one create_pull_request_review_comment call; calling with no body and no prior comments is rejected with ERR_VALIDATION. All preceding create_pull_request_review_comment outputs are automatically attached as inline comments. If this tool is not called, buffered review comments are submitted as a COMMENT review at workflow end. Use COMMENT for non-blocking feedback; use REQUEST_CHANGES only for merge-blocking. Example (inline-only review): call create_pull_request_review_comment one or more times, then call this tool with event: COMMENT and no body.", "inputSchema": { "type": "object", "properties": { "body": { "type": "string", - "description": "Overall review summary in Markdown. Provide a high-level assessment of the changes. Required for REQUEST_CHANGES; optional for APPROVE and COMMENT." + "description": "Overall review summary in Markdown. Provide a high-level assessment of the changes. Required for REQUEST_CHANGES. For APPROVE or COMMENT, body is optional only when at least one create_pull_request_review_comment was called earlier in the same run; otherwise body is required." }, "event": { "type": "string", diff --git a/setup/js/safe_outputs_tools_loader.cjs b/setup/js/safe_outputs_tools_loader.cjs index ca3a45af..76d9fcc9 100644 --- a/setup/js/safe_outputs_tools_loader.cjs +++ b/setup/js/safe_outputs_tools_loader.cjs @@ -5,6 +5,53 @@ const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require( const fs = require("fs"); +/** + * Check whether a schema enforces strict object keys. + * @param {any} inputSchema - Tool input schema + * @returns {boolean} True when additional properties are disallowed + */ +function isStrictSchema(inputSchema) { + if (!inputSchema || typeof inputSchema !== "object") { + return false; + } + if (inputSchema.additionalProperties !== false) { + return false; + } + const { properties } = inputSchema; + return !!properties && typeof properties === "object" && !Array.isArray(properties); +} + +/** + * Strip unknown keys from tool arguments when schema is strict. + * @param {any} args - Tool call arguments + * @param {any} inputSchema - Tool input schema + * @param {(keys: string[]) => void} [onUnknownKeysStripped] - Optional callback for stripped keys + * @returns {any} Sanitized args + */ +function sanitizeArgsBySchema(args, inputSchema, onUnknownKeysStripped) { + if (!args || typeof args !== "object" || Array.isArray(args)) { + return args; + } + if (!isStrictSchema(inputSchema)) { + return args; + } + + const allowedKeys = new Set(Object.keys(inputSchema.properties)); + const sanitizedArgs = {}; + const strippedKeys = []; + for (const [key, value] of Object.entries(args)) { + if (allowedKeys.has(key)) { + sanitizedArgs[key] = value; + } else { + strippedKeys.push(key); + } + } + if (strippedKeys.length > 0 && typeof onUnknownKeysStripped === "function") { + onUnknownKeysStripped(strippedKeys); + } + return sanitizedArgs; +} + /** * Load tools from tools.json file * @param {Object} server - The MCP server instance for logging @@ -68,9 +115,10 @@ function loadTools(server) { * Attach handlers to tools * @param {Array} tools - Array of tool definitions * @param {Object} handlers - Object containing handler functions + * @param {{ debug?: Function }} [logger] - Optional logger * @returns {Array} Tools with handlers attached */ -function attachHandlers(tools, handlers) { +function attachHandlers(tools, handlers, logger) { const handlerMap = { create_issue: handlers.createIssueHandler, create_pull_request: handlers.createPullRequestHandler, @@ -89,6 +137,8 @@ function attachHandlers(tools, handlers) { const handler = handlerMap[tool.name]; if (handler) { tool.handler = handler; + } else if (typeof handlers.defaultHandler === "function") { + tool.handler = handlers.defaultHandler(tool.name); } // Check if this is a dispatch_workflow tool (dynamic tool with workflow metadata) @@ -127,6 +177,16 @@ function attachHandlers(tools, handlers) { }); }; } + + if (typeof tool.handler === "function" && isStrictSchema(tool.inputSchema)) { + const originalHandler = tool.handler; + tool.handler = args => + originalHandler( + sanitizeArgsBySchema(args, tool.inputSchema, strippedKeys => { + logger?.debug?.(`Stripped unknown keys for strict schema tool '${tool.name}': ${JSON.stringify(strippedKeys)}`); + }) + ); + } }); return tools; @@ -158,6 +218,9 @@ function registerPredefinedTools(server, tools, config, registerTool, normalizeT // Enrich create_pull_request tool description when target-repo is configured if (safetyWarning || isCreatePullRequestTool) { toolToRegister = JSON.parse(JSON.stringify(tool)); + if (tool.handler) { + toolToRegister.handler = tool.handler; + } if (safetyWarning) { toolToRegister.description += safetyWarning; } diff --git a/setup/js/send_otlp_span.cjs b/setup/js/send_otlp_span.cjs index 39231578..86d2f231 100644 --- a/setup/js/send_otlp_span.cjs +++ b/setup/js/send_otlp_span.cjs @@ -372,9 +372,9 @@ function buildGitHubActionsResourceAttributes({ runAttempt = "1", }) { const resourceAttributes = [buildAttr("github.repository", repository), buildAttr("github.run_id", runId), buildAttr("github.run_attempt", runAttempt)]; - if (repository && runId && repository.includes("/")) { - const [owner, repo] = repository.split("/"); - resourceAttributes.push(buildAttr("github.actions.run_url", buildWorkflowRunUrl({ runId }, { owner, repo }))); + const runUrlAttr = buildGitHubActionsRunUrlAttribute(repository, runId); + if (runUrlAttr) { + resourceAttributes.push(runUrlAttr); } if (eventName) { resourceAttributes.push(buildAttr("github.event_name", eventName)); @@ -422,6 +422,21 @@ function buildGitHubActionsResourceAttributes({ return resourceAttributes; } +/** + * Build github.actions.run_url attribute when repository and run ID are available. + * + * @param {string} repository + * @param {string} runId + * @returns {{ key: string, value: object } | null} + */ +function buildGitHubActionsRunUrlAttribute(repository, runId) { + if (!repository || !runId || !repository.includes("/")) { + return null; + } + const [owner, repo] = repository.split("/"); + return buildAttr("github.actions.run_url", buildWorkflowRunUrl({ runId }, { owner, repo })); +} + /** * Wrap one or more OTLP span objects in a single traces payload. * @@ -1158,6 +1173,10 @@ async function sendJobSetupSpan(options = {}) { buildAttr("gh-aw.run.actor", actor), buildAttr("gh-aw.repository", repository), ]; + const runUrlAttr = buildGitHubActionsRunUrlAttribute(repository, runId); + if (runUrlAttr) { + attributes.push(runUrlAttr); + } if (scopeVersion !== "unknown") { attributes.push(buildAttr("gh-aw.cli.version", scopeVersion)); } @@ -1523,7 +1542,6 @@ function getErrorMessage(errorEntry) { /** * @typedef {Object} AgentRuntimeMetrics * @property {number | undefined} turns - * @property {number | undefined} estimatedCostUsd * @property {string | undefined} stopReason * @property {string | undefined} resolvedModel * @property {{input_tokens?: number, output_tokens?: number, cache_read_tokens?: number, cache_write_tokens?: number} | undefined} tokenUsage @@ -1576,13 +1594,13 @@ function normalizeRuntimeTokenUsage(rawUsage) { } /** - * Read turns, estimated cost, token usage, and warning volume from agent-stdio.log. + * Read turns, token usage, and warning volume from agent-stdio.log. * * @returns {AgentRuntimeMetrics} */ function readAgentRuntimeMetrics() { /** @type {AgentRuntimeMetrics} */ - const metrics = { turns: undefined, estimatedCostUsd: undefined, stopReason: undefined, resolvedModel: undefined, tokenUsage: undefined, warningCount: 0 }; + const metrics = { turns: undefined, stopReason: undefined, resolvedModel: undefined, tokenUsage: undefined, warningCount: 0 }; try { const content = fs.readFileSync(AGENT_STDIO_LOG_PATH, "utf8"); @@ -1609,9 +1627,6 @@ function readAgentRuntimeMetrics() { if (typeof parsed.num_turns === "number" && parsed.num_turns >= 0) { metrics.turns = parsed.num_turns; } - if (typeof parsed.total_cost_usd === "number" && Number.isFinite(parsed.total_cost_usd) && parsed.total_cost_usd >= 0) { - metrics.estimatedCostUsd = parsed.total_cost_usd; - } if (typeof parsed.stop_reason === "string" && parsed.stop_reason) { metrics.stopReason = parsed.stop_reason; } @@ -1847,6 +1862,10 @@ async function sendJobConclusionSpan(spanName, options = {}) { } const attributes = [buildAttr("gh-aw.workflow.name", workflowName), buildAttr("gh-aw.run.id", runId), buildAttr("gh-aw.run.attempt", runAttempt), buildAttr("gh-aw.run.actor", actor), buildAttr("gh-aw.repository", repository)]; + const runUrlAttr = buildGitHubActionsRunUrlAttribute(repository, runId); + if (runUrlAttr) { + attributes.push(runUrlAttr); + } if (version !== "unknown") { attributes.push(buildAttr("gh-aw.cli.version", version)); } @@ -1889,9 +1908,6 @@ async function sendJobConclusionSpan(spanName, options = {}) { if (typeof runtimeMetrics.turns === "number") { attributes.push(buildAttr("gh-aw.turns", runtimeMetrics.turns)); } - if (typeof runtimeMetrics.estimatedCostUsd === "number") { - attributes.push(buildAttr("gh-aw.estimated_cost_usd", runtimeMetrics.estimatedCostUsd)); - } if (jobName === "agent") { // Emit OTel GenAI semantic attributes on agent conclusion spans even when the // dedicated agent sub-span is missing, so LLM activity remains discoverable. diff --git a/setup/js/submit_pr_review.cjs b/setup/js/submit_pr_review.cjs index 8694d50d..2fffdcda 100644 --- a/setup/js/submit_pr_review.cjs +++ b/setup/js/submit_pr_review.cjs @@ -205,6 +205,7 @@ async function main(config = {}) { success: true, event: event, body_length: body.length, + deferred_manifest: true, }; }; } diff --git a/setup/js/update_handler_factory.cjs b/setup/js/update_handler_factory.cjs index 0103d2aa..1630e3ae 100644 --- a/setup/js/update_handler_factory.cjs +++ b/setup/js/update_handler_factory.cjs @@ -11,6 +11,7 @@ const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); +const { attachExecutionState } = require("./safe_output_execution_metadata.cjs"); const { withRetry, isTransientError } = require("./error_recovery.cjs"); const { loadTemporaryIdMapFromResolved, resolveRepoIssueTarget } = require("./temporary_id.cjs"); @@ -25,6 +26,7 @@ const { loadTemporaryIdMapFromResolved, resolveRepoIssueTarget } = require("./te * @property {Function} formatSuccessResult - Function to format success result * @property {Object} [additionalConfig] - Additional configuration options specific to the handler * @property {Function} [itemFilter] - Optional async filter function called before update; returns null to proceed or a result object to skip + * @property {{captureBefore?: Function, captureAfter?: Function}} [captureExecutionMetadata] - Optional execution metadata hooks */ /** @@ -114,7 +116,7 @@ function createStandardFormatResult(fieldMapping) { * @returns {HandlerFactoryFunction} Handler factory function */ function createUpdateHandlerFactory(handlerConfig) { - const { itemType, itemTypeName, supportsPR, resolveItemNumber, buildUpdateData, executeUpdate, formatSuccessResult, additionalConfig = {}, itemFilter = null } = handlerConfig; + const { itemType, itemTypeName, supportsPR, resolveItemNumber, buildUpdateData, executeUpdate, formatSuccessResult, additionalConfig = {}, itemFilter = null, captureExecutionMetadata = null } = handlerConfig; /** * Main handler factory @@ -285,11 +287,17 @@ function createUpdateHandlerFactory(handlerConfig) { // effectiveContext.repo contains the target repo owner/name for cross-repo routing. // Retry on transient errors (e.g. GitHub API returning HTML instead of JSON on 500 crashes). try { + const beforeState = captureExecutionMetadata?.captureBefore ? await captureExecutionMetadata.captureBefore(githubClient, effectiveContext, itemNumber, updateData) : null; const updatedItem = await withRetry(() => executeUpdate(githubClient, effectiveContext, itemNumber, updateData), { maxRetries: 1, initialDelayMs: 2000, shouldRetry: isTransientError }, `update ${itemTypeName} #${itemNumber}`); core.info(`Successfully updated ${itemTypeName} #${itemNumber}: ${updatedItem.html_url || updatedItem.url}`); // Format and return success result - return formatSuccessResult(itemNumber, updatedItem); + const result = { + ...formatSuccessResult(itemNumber, updatedItem), + repo: `${effectiveContext.repo.owner}/${effectiveContext.repo.repo}`, + }; + const afterState = captureExecutionMetadata?.captureAfter ? await captureExecutionMetadata.captureAfter(updatedItem, beforeState, updateData) : null; + return attachExecutionState(result, beforeState, afterState); } catch (error) { const errorMessage = getErrorMessage(error); core.error(`Failed to update ${itemTypeName} #${itemNumber}: ${errorMessage}`); diff --git a/setup/js/update_issue.cjs b/setup/js/update_issue.cjs index 86c96ea5..5abe7f11 100644 --- a/setup/js/update_issue.cjs +++ b/setup/js/update_issue.cjs @@ -18,6 +18,7 @@ const { ERR_VALIDATION } = require("./error_codes.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { generateHistoryUrl } = require("./generate_history_link.cjs"); +const { fetchIssueState, mergeIssueState } = require("./safe_output_execution_metadata.cjs"); const { MAX_LABELS, MAX_ASSIGNEES } = require("./constants.cjs"); /** @@ -211,6 +212,10 @@ const main = createUpdateHandlerFactory({ buildUpdateData: buildIssueUpdateData, executeUpdate: executeIssueUpdate, formatSuccessResult: formatIssueSuccessResult, + captureExecutionMetadata: { + captureBefore: async (githubClient, effectiveContext, issueNumber) => fetchIssueState(githubClient, effectiveContext.repo, issueNumber), + captureAfter: async (updatedIssue, beforeState) => mergeIssueState(beforeState, updatedIssue), + }, itemFilter: async (githubClient, repoParts, issueNumber, config) => { const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : []; const requiredTitlePrefix = config.required_title_prefix || ""; diff --git a/setup/js/update_pull_request.cjs b/setup/js/update_pull_request.cjs index a10f1c3a..7917ef87 100644 --- a/setup/js/update_pull_request.cjs +++ b/setup/js/update_pull_request.cjs @@ -16,6 +16,7 @@ const { parseBoolTemplatable } = require("./templatable.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { generateHistoryUrl } = require("./generate_history_link.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); +const { fetchPullRequestState, mergePullRequestState } = require("./safe_output_execution_metadata.cjs"); const { withRetry, isTransientError } = require("./error_recovery.cjs"); /** @@ -146,10 +147,15 @@ async function executePRUpdate(github, context, prNumber, updateData) { } if (Object.keys(apiData).length === 0) { - return { - number: prNumber, - html_url: `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/pull/${prNumber}`, - }; + // update_branch-only operations need the authoritative post-update PR state so the + // manifest can persist after_state fields such as head_sha/base/draft for later + // retained-update evaluation. A synthetic {number, html_url} result is not enough. + const { data: pullRequest } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + return pullRequest; } const { data: pr } = await github.rest.pulls.update({ @@ -259,6 +265,10 @@ const main = createUpdateHandlerFactory({ buildUpdateData: buildPRUpdateData, executeUpdate: executePRUpdate, formatSuccessResult: formatPRSuccessResult, + captureExecutionMetadata: { + captureBefore: async (githubClient, effectiveContext, prNumber) => fetchPullRequestState(githubClient, effectiveContext.repo, prNumber), + captureAfter: async (updatedPullRequest, beforeState) => mergePullRequestState(beforeState, updatedPullRequest), + }, additionalConfig: { allow_title: true, allow_body: true, diff --git a/setup/md/forecast_issue.md b/setup/md/forecast_issue.md new file mode 100644 index 00000000..a5f782fa --- /dev/null +++ b/setup/md/forecast_issue.md @@ -0,0 +1,12 @@ +### Agentic workflow forecast report + +Repository: {repository} +Generated at: {generated_at} +Period: {period} + +{report_table} + +{all_projected_zero_note} +{zero_projected_tip} +{error_section} +{source_run_line} diff --git a/setup/md/threat_detection.md b/setup/md/threat_detection.md index f81f78c5..16b22729 100644 --- a/setup/md/threat_detection.md +++ b/setup/md/threat_detection.md @@ -68,6 +68,12 @@ Analyze the above content for the following security threats, using the workflow - **Backdoor Installation**: Hidden remote access mechanisms, unauthorized authentication bypass, or persistent access methods - **Encoded Strings**: Base64, hex, or other encoded strings that appear to hide secrets, commands, or malicious payloads without legitimate purpose - **Suspicious Dependencies**: Addition of unknown packages, dependencies from untrusted sources, or libraries with known vulnerabilities + - **npm Lockfile Validation** (for `package-lock.json` changes): Before flagging an npm dependency update as malicious, verify against current npm metadata: + - Fetch current npm metadata using `npm view @ --json` (or the corresponding `registry.npmjs.org` version endpoint) + - Confirm the package version exists in the npm registry + - Confirm `resolved` matches npm's official tarball URL for that exact version + - Confirm `integrity` matches npm's official integrity hash for that exact version + - Treat stale-version claims as false positives when current npm metadata confirms the version exists and both `resolved` and `integrity` match ## Response Format diff --git a/setup/setup.sh b/setup/setup.sh index 98460a63..ea9bb338 100755 --- a/setup/setup.sh +++ b/setup/setup.sh @@ -178,19 +178,6 @@ fi echo "Successfully copied ${FILE_COUNT} files to ${DESTINATION}" -# Export model multipliers JSON as GH_AW_MODEL_MULTIPLIERS environment variable. -# This makes the per-model effective token multipliers available to JavaScript -# actions running in subsequent steps so they can compute Effective Tokens (ET). -MODEL_MULTIPLIERS_FILE="${DESTINATION}/model_multipliers.json" -if [ -f "${MODEL_MULTIPLIERS_FILE}" ] && [ -n "${GITHUB_ENV:-}" ]; then - { - echo "GH_AW_MODEL_MULTIPLIERS<> "${GITHUB_ENV}" - debug_log "Exported GH_AW_MODEL_MULTIPLIERS from ${MODEL_MULTIPLIERS_FILE}" -fi - # Copy prompt markdown files to their expected directory PROMPTS_DEST="${GH_AW_ROOT}/prompts" debug_log "Copying prompt markdown files to ${PROMPTS_DEST}" diff --git a/setup/sh/mask_otlp_headers.sh b/setup/sh/mask_otlp_headers.sh index 15363d05..7ec01907 100644 --- a/setup/sh/mask_otlp_headers.sh +++ b/setup/sh/mask_otlp_headers.sh @@ -11,13 +11,14 @@ set +o histexpand # When GH_AW_OTLP_ALL_HEADERS is set (multi-endpoint configuration), the same # masking is applied to all endpoint headers combined in that variable. # -# Three levels of masking are applied to each headers string: +# Up to three levels of masking are applied to each headers string: # 1. The entire comma-separated header pairs string. # 2. Each individual header value extracted from the pairs, so that a token # appearing without its header name prefix is also redacted. # 3. For Authorization-style "Bearer " credentials, the raw token after # stripping the "Bearer " scheme prefix, so it is masked even when it appears # without the scheme (e.g. in downstream tool logs). +# Values shorter than MIN_MASK_LENGTH are skipped at all levels. # # Mixed quoting ('::add-mask::' followed by "$VAR") is used so the directive prefix # is treated as a literal string while the variable values are expanded at runtime. @@ -27,6 +28,18 @@ set +o histexpand set -euo pipefail +# Ignore mask values shorter than 4 characters because GHES may over-mask +# subsequent logs when receiving very short ::add-mask:: entries. +MIN_MASK_LENGTH=4 + +# emit_mask emits ::add-mask:: only for non-empty values at or above MIN_MASK_LENGTH. +emit_mask() { + local _value="${1:-}" + [ -z "$_value" ] && return + [ "${#_value}" -lt "$MIN_MASK_LENGTH" ] && return + echo '::add-mask::'"$_value" +} + # mask_headers masks all values in a comma-separated key=value headers string. mask_headers() { local _headers="$1" @@ -35,7 +48,7 @@ mask_headers() { [ -z "$_headers" ] && return # Level 1: mask the entire comma-separated headers string. - echo '::add-mask::'"$_headers" + emit_mask "$_headers" # Levels 2 & 3: split on commas, extract each value, and mask it individually. # For "Bearer " values, also mask the raw token without the scheme prefix. @@ -43,10 +56,10 @@ mask_headers() { mapfile -t _pairs < <(printf '%s' "$_headers" | tr ',' '\n') for _pair in "${_pairs[@]}"; do _val="${_pair#*=}" - [ -n "$_val" ] && echo '::add-mask::'"$_val" + emit_mask "$_val" _no_bearer="${_val#Bearer }" if [ "$_no_bearer" != "$_val" ]; then - echo '::add-mask::'"$_no_bearer" + emit_mask "$_no_bearer" fi done } diff --git a/setup/sh/setup_cache_memory_git.sh b/setup/sh/setup_cache_memory_git.sh index 7e049740..0df3e6e7 100644 --- a/setup/sh/setup_cache_memory_git.sh +++ b/setup/sh/setup_cache_memory_git.sh @@ -28,6 +28,40 @@ INTEGRITY="${GH_AW_MIN_INTEGRITY:-none}" # All integrity levels in descending order (highest first) LEVELS=("merged" "approved" "unapproved" "none") +ensure_writable_dir() { + local dir="$1" + local purpose="$2" + local probe_file="" + local mkdir_err + local chmod_err + local write_err + mkdir_err="$(mktemp /tmp/gh-aw-cache-mkdir-err.XXXXXX)" + chmod_err="$(mktemp /tmp/gh-aw-cache-chmod-err.XXXXXX)" + write_err="$(mktemp /tmp/gh-aw-cache-write-err.XXXXXX)" + + if ! mkdir -p "$dir" 2>"$mkdir_err"; then + echo "ERROR: cache-memory setup error: failed to create ${purpose} (${dir})" >&2 + cat "$mkdir_err" >&2 || true + rm -f "$mkdir_err" "$chmod_err" "$write_err" 2>/dev/null || true + exit 1 + fi + + if ! chmod u+rwx "$dir" 2>"$chmod_err"; then + echo "ERROR: cache-memory setup error: ${purpose} is not writable (${dir})" >&2 + cat "$chmod_err" >&2 || true + rm -f "$mkdir_err" "$chmod_err" "$write_err" 2>/dev/null || true + exit 1 + fi + + if ! probe_file="$(mktemp "${dir}/gh-aw-write-check.XXXXXX" 2>"$write_err")"; then + echo "ERROR: cache-memory setup error: ${purpose} is not writable (${dir})" >&2 + cat "$write_err" >&2 || true + rm -f "$mkdir_err" "$chmod_err" "$write_err" 2>/dev/null || true + exit 1 + fi + rm -f "$probe_file" "$mkdir_err" "$chmod_err" "$write_err" 2>/dev/null || true +} + initialize_cache_memory_git_repo() { # No git repo yet — either a fresh cache or a legacy flat-file cache. # Initialize a git repository with an empty baseline commit on the highest-trust @@ -262,3 +296,9 @@ if [ "$IS_CACHE_HIT" = "true" ]; then "$_run_id" "$_timestamp" "$_post_file_count" > "cache-hit-history.json" echo "Cache hit history updated (run: $_run_id, files: $_post_file_count)" fi + +# Preflight write checks for known cache-memory paths required by daily planners. +# Fail fast here so agent runs do not continue after a hidden permission problem. +ensure_writable_dir "$CACHE_DIR" "cache-memory root directory" +ensure_writable_dir "${CACHE_DIR}/spdd-daily" "Daily SPDD rotation cache directory" +echo "Cache memory preflight write checks passed" diff --git a/setup/sh/setup_cache_memory_git_test.sh b/setup/sh/setup_cache_memory_git_test.sh index 64ab79f3..efc07c45 100644 --- a/setup/sh/setup_cache_memory_git_test.sh +++ b/setup/sh/setup_cache_memory_git_test.sh @@ -249,6 +249,24 @@ assert "integrity branch exists after recovery" \ "git -C '${D}' rev-parse --verify none >/dev/null 2>&1" echo "" +# ── Test 14: Daily SPDD rotation directory preflight is created and writable ── +echo "Test 14: Daily SPDD rotation cache directory is writable" +D="${WORKSPACE}/test14" +make_cache_dir "${D}" "data.json" +set +e +OUTPUT="$(run_script "${D}" none)" +EXIT_CODE=$? +set -e +assert "preflight exits successfully" \ + "[ '${EXIT_CODE}' -eq 0 ]" +assert "spdd-daily directory created" \ + "[ -d '${D}/spdd-daily' ]" +assert "spdd-daily directory writable" \ + "[ -w '${D}/spdd-daily' ]" +assert "preflight success message logged" \ + "printf '%s' \"${OUTPUT}\" | grep -q 'Cache memory preflight write checks passed'" +echo "" + # ── Summary ────────────────────────────────────────────────────────────────── echo "Tests passed: ${TESTS_PASSED}" echo "Tests failed: ${TESTS_FAILED}" diff --git a/setup/sh/start_cli_proxy.sh b/setup/sh/start_cli_proxy.sh index da104b16..fe6f8f63 100644 --- a/setup/sh/start_cli_proxy.sh +++ b/setup/sh/start_cli_proxy.sh @@ -41,6 +41,7 @@ if [ -n "$POLICY" ]; then fi docker run -d --name awmg-cli-proxy --network host \ + --user "$(id -u):$(id -g)" \ -e GH_TOKEN \ -e GITHUB_SERVER_URL \ -e DEBUG='*' \ diff --git a/setup/sh/start_difc_proxy.sh b/setup/sh/start_difc_proxy.sh index 3016e3e9..6a60995f 100644 --- a/setup/sh/start_difc_proxy.sh +++ b/setup/sh/start_difc_proxy.sh @@ -40,6 +40,7 @@ mkdir -p "$PROXY_LOG_DIR" "$MCP_LOG_DIR" echo "Starting DIFC proxy container: $CONTAINER_IMAGE" docker run -d --name awmg-proxy --network host \ + --user "$(id -u):$(id -g)" \ -e GH_TOKEN \ -e GITHUB_SERVER_URL \ -e DEBUG='*' \