Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/patch-prevent-safeoutput-pr-probing.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

237 changes: 157 additions & 80 deletions .github/workflows/pr-description-caveman.lock.yml

Large diffs are not rendered by default.

149 changes: 149 additions & 0 deletions actions/setup/js/intent_probe.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// @ts-check

const TRIVIAL_PROBE_VALUES = new Set(["test", "testing", "test no base", "probe", "placeholder", "dummy", "temp", "temporary", "todo", "tbd", "wip", "example"]);

/**
* @param {unknown} value
* @returns {string}
*/
function normalizeProbeValue(value) {
if (typeof value !== "string") return "";
return value.trim().toLowerCase().replace(/\s+/g, " ");
}

/**
* @param {unknown} value
* @returns {boolean}
*/
function isTrivialProbeValue(value) {
return TRIVIAL_PROBE_VALUES.has(normalizeProbeValue(value));
}

/**
* @param {unknown} value
* @returns {boolean}
*/
function looksLikeExploratoryBranch(value) {
const branch = normalizeProbeValue(value);
const hasProbeBranchMarker = /(^|[-/])probe([-/]|$)/.test(branch);
return branch.includes("test-from-main") || TRIVIAL_PROBE_VALUES.has(branch) || hasProbeBranchMarker;
}

/**
* @param {{title?: unknown, body?: unknown}} entry
* @returns {string}
*/
function resolveIssueTitleForValidation(entry) {
if (typeof entry.title === "string" && entry.title.trim()) {
return entry.title;
}
if (typeof entry.body === "string" && entry.body.trim()) {
return entry.body;
}
// Mirror create_issue's own fallback so probe validation sees the same
// effective title the handler would ultimately use when both fields are absent.
return "Agent Output";
}

/**
* Detects obviously exploratory PR payloads so the agent can fail fast
* instead of recording a stray real-world PR intent.
* @param {{title?: unknown, body?: unknown, branch?: unknown}} entry
* @returns {string|null}
*/
function validateCreatePullRequestIntent(entry) {
const title = normalizeProbeValue(entry.title);
const body = normalizeProbeValue(entry.body);
const branch = normalizeProbeValue(entry.branch);

const looksLikeProbeTitle = TRIVIAL_PROBE_VALUES.has(title);
const looksLikeProbeBody = TRIVIAL_PROBE_VALUES.has(body);
const looksLikeTestFromMainBranch = branch.includes("test-from-main");
const looksLikeProbeBranch = looksLikeExploratoryBranch(branch);

if (looksLikeTestFromMainBranch || (looksLikeProbeTitle && looksLikeProbeBody) || (looksLikeProbeBranch && (looksLikeProbeTitle || looksLikeProbeBody))) {
return (
"Refusing to record an exploratory pull request. " +
"create_pull_request is for a real intended PR only and successful calls can lead to an externally visible pull request. " +
"Do not use placeholder values like 'test' or probe branches. " +
"If you are not ready to open the real PR, use noop or report_incomplete instead."
);
}

return null;
}

/**
* Detects obviously exploratory issue payloads so the agent can fail fast
* instead of recording a stray real-world issue intent.
* @param {{title?: unknown, body?: unknown}} entry
* @returns {string|null}
*/
function validateCreateIssueIntent(entry) {
const rawResolvedTitle = resolveIssueTitleForValidation(entry);
const body = normalizeProbeValue(entry.body);

if (isTrivialProbeValue(rawResolvedTitle) && (body === "" || isTrivialProbeValue(body))) {
return (
"Refusing to record an exploratory issue. " +
"create_issue is for a real intended issue only and successful calls can lead to an externally visible issue. " +
"Do not use placeholder titles or bodies like 'test'. " +
"If you are not ready to open the real issue, use noop or report_incomplete instead."
);
}

return null;
}

/**
* Detects obviously exploratory comment payloads so the agent can fail fast
* instead of recording a stray real-world comment intent.
* @param {{body?: unknown}} entry
* @returns {string|null}
*/
function validateAddCommentIntent(entry) {
if (isTrivialProbeValue(entry.body)) {
return (
"Refusing to record an exploratory comment. " +
"add_comment is for a real intended comment only and successful calls can lead to an externally visible comment. " +
"Do not use placeholder bodies like 'test'. " +
"If you are not ready to post the real comment, use noop or report_incomplete instead."
);
}

return null;
}

/**
* Detects obviously exploratory PR-branch push payloads so the agent can fail fast
* instead of recording a stray real-world branch update intent.
* @param {{branch?: unknown, message?: unknown}} entry
* @returns {string|null}
*/
function validatePushToPullRequestBranchIntent(entry) {
const branch = normalizeProbeValue(entry.branch);
const looksLikeTestFromMainBranch = branch.includes("test-from-main");
const looksLikeProbeBranch = looksLikeExploratoryBranch(branch);
const looksLikeProbeMessage = isTrivialProbeValue(entry.message);

if (looksLikeTestFromMainBranch || TRIVIAL_PROBE_VALUES.has(branch) || (looksLikeProbeBranch && looksLikeProbeMessage)) {
return (
"Refusing to record an exploratory pull request branch update. " +
"push_to_pull_request_branch is for a real intended PR update only and successful calls can lead to externally visible branch changes. " +
"Do not use probe branches, '*-test-from-main-*' branches, or placeholder commit messages like 'test'. " +
"If you are not ready to push the real update, use noop or report_incomplete instead."
);
}

return null;
}

module.exports = {
looksLikeExploratoryBranch,
normalizeProbeValue,
resolveIssueTitleForValidation,
validateAddCommentIntent,
validateCreateIssueIntent,
validateCreatePullRequestIntent,
validatePushToPullRequestBranchIntent,
};
51 changes: 49 additions & 2 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,31 @@ const { getOrGenerateTemporaryId } = require("./temporary_id.cjs");
const { parseAllowedExtensionsEnv } = require("./allowed_extensions_helpers.cjs");
const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs");
const { parseDeduplicateByTitle, normalizeTitleForDedup, findDuplicateByTitle } = require("./issue_title_dedup.cjs");
const {
validateCreatePullRequestIntent,
validatePushToPullRequestBranchIntent,
validateCreateIssueIntent,
validateAddCommentIntent,
} = require("./intent_probe.cjs");

/**
* @param {string} error
* @returns {{content: Array<{type: "text", text: string}>, isError: true}}
*/
function buildIntentErrorResponse(error) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error,
}),
},
],
isError: true,
};
}

/**
* Create handlers for safe output tools
Expand Down Expand Up @@ -228,6 +253,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
* Handler for create_pull_request tool
* Spec cross-reference: Safe Output Outcome Evaluation §1 (`create_pull_request`).
* Resolves the current branch if branch is not provided or is the base branch
* Validates exploratory probe payloads against the resolved effective branch
* Generates git patch for the changes (unless allow-empty is true)
* Supports multi-repo scenarios via the optional 'repo' parameter
*/
Expand Down Expand Up @@ -331,6 +357,11 @@ function createHandlers(server, appendSafeOutput, config = {}) {
entry.branch = detectedBranch;
}

const intentValidationError = validateCreatePullRequestIntent(entry);
if (intentValidationError) {
return buildIntentErrorResponse(intentValidationError);
}

// Check if allow-empty is enabled in configuration
const allowEmpty = config.create_pull_request?.allow_empty === true;

Expand Down Expand Up @@ -603,6 +634,11 @@ function createHandlers(server, appendSafeOutput, config = {}) {
entry.branch = detectedBranch;
}

const intentValidationError = validatePushToPullRequestBranchIntent(entry);
if (intentValidationError) {
return buildIntentErrorResponse(intentValidationError);
}

// Determine transport format: "bundle" (default) uses git bundle (preserves merge topology),
// "am" uses git format-patch / git am (good for linear histories).
// Use ?? (nullish coalescing) so an empty-string resolved value is preserved and
Expand Down Expand Up @@ -976,6 +1012,10 @@ function createHandlers(server, appendSafeOutput, config = {}) {
*/
const createIssueHandler = args => {
const entry = { ...(args || {}), type: "create_issue" };
const intentValidationError = validateCreateIssueIntent(entry);
if (intentValidationError) {
return buildIntentErrorResponse(intentValidationError);
}

const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(createIssueConfig);
const repoResult = resolveAndValidateRepo(entry, defaultTargetRepo, allowedRepos, "issue");
Expand Down Expand Up @@ -1109,6 +1149,10 @@ function createHandlers(server, appendSafeOutput, config = {}) {

// Build the entry with a temporary_id
const entry = { ...(args || {}), type: "add_comment" };
const intentValidationError = validateAddCommentIntent(entry);
if (intentValidationError) {
return buildIntentErrorResponse(intentValidationError);
}

// Use helper to validate or generate temporary_id
const tempIdResult = getOrGenerateTemporaryId(entry, "add_comment");
Expand Down Expand Up @@ -1178,7 +1222,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
*/
const submitPullRequestReviewHandler = args => {
const body = (args && typeof args.body === "string" ? args.body : "").trim();
const event = (args && args.event ? String(args.event).toUpperCase() : "COMMENT");
const event = args && args.event ? String(args.event).toUpperCase() : "COMMENT";

const VALID_REVIEW_EVENTS = ["APPROVE", "REQUEST_CHANGES", "COMMENT"];
if (!VALID_REVIEW_EVENTS.includes(event)) {
Expand Down Expand Up @@ -1330,4 +1374,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
};
}

module.exports = { createHandlers };
module.exports = {
buildIntentErrorResponse,
createHandlers,
};
Loading