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
4 changes: 3 additions & 1 deletion .github/scripts/agents-guard.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,9 @@ function evaluateGuard({
const hasCodeownerApproval = hasExternalApproval || authorIsCodeowner;

const hasProtectedChanges = modifiedProtectedPaths.size > 0;
const needsApproval = hasProtectedChanges && !hasCodeownerApproval;
// Allow label to bypass approval for automated PRs (dependabot, renovate)
const isAutomatedPR = normalizedAuthor && (normalizedAuthor === 'dependabot[bot]' || normalizedAuthor === 'renovate[bot]');
const needsApproval = hasProtectedChanges && !hasCodeownerApproval && !(hasAllowLabel && isAutomatedPR);
Comment on lines +450 to +452
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic allows automated PRs (dependabot, renovate) to bypass approval requirements if they have the allow label. However, this creates a potential security risk where an automated PR with the allow label can modify protected workflow files without human review. Consider whether this bypass should require both the label AND some form of approval, or if there should be additional constraints on what types of changes automated PRs can make to protected paths.

Suggested change
// Allow label to bypass approval for automated PRs (dependabot, renovate)
const isAutomatedPR = normalizedAuthor && (normalizedAuthor === 'dependabot[bot]' || normalizedAuthor === 'renovate[bot]');
const needsApproval = hasProtectedChanges && !hasCodeownerApproval && !(hasAllowLabel && isAutomatedPR);
// Automated PRs (dependabot, renovate) must always get approval for protected changes,
// even if they have the allow label. The label can still bypass approval for
// human-authored PRs.
const isAutomatedPR = normalizedAuthor && (normalizedAuthor === 'dependabot[bot]' || normalizedAuthor === 'renovate[bot]');
const automatedBypassAllowed = hasAllowLabel && !isAutomatedPR;
const needsApproval = hasProtectedChanges && !hasCodeownerApproval && !automatedBypassAllowed;

Copilot uses AI. Check for mistakes.
const needsLabel = hasProtectedChanges && !hasAllowLabel && !hasCodeownerApproval;

const failureReasons = [];
Expand Down
7 changes: 7 additions & 0 deletions .github/scripts/error_classifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ const TRANSIENT_PATTERNS = [
'bad gateway',
'gateway timeout',
'eai_again',
// Git workspace state issues - agent encountered unexpected changes
'unexpected changes',
'untracked',
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern 'untracked' at line 46 is too generic and could match legitimate error messages about untracked files that are not transient infrastructure issues. Consider making this pattern more specific, such as 'untracked.*workflows-lib' or 'untracked.*codex-session', to avoid false positives where actual code issues involve untracked files.

Suggested change
'untracked',
'untracked.*workflows-lib',
'untracked.*codex-session',

Copilot uses AI. Check for mistakes.
'.workflows-lib is modified',
'codex-session',
'existing changes',
'how would you like me to proceed',
];

const AUTH_PATTERNS = [
Expand Down
149 changes: 103 additions & 46 deletions .github/scripts/keepalive_instruction_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,63 +2,109 @@

const fs = require('fs');
const path = require('path');
const { resolvePromptMode } = require('./keepalive_prompt_routing');

/**
* Path to the canonical keepalive instruction template.
* Edit .github/templates/keepalive-instruction.md to change the instruction text.
* Path to the fallback keepalive instruction template.
* Edit .github/templates/keepalive-instruction.md to change the fallback text.
*/
const TEMPLATE_PATH = path.resolve(__dirname, '../templates/keepalive-instruction.md');
const NEXT_TASK_TEMPLATE_PATH = path.resolve(__dirname, '../codex/prompts/keepalive_next_task.md');
const FIX_TEMPLATE_PATH = path.resolve(__dirname, '../codex/prompts/fix_ci_failures.md');
const VERIFY_TEMPLATE_PATH = path.resolve(__dirname, '../codex/prompts/verifier_acceptance_check.md');

const TEMPLATE_PATHS = {
normal: NEXT_TASK_TEMPLATE_PATH,
fix_ci: FIX_TEMPLATE_PATH,
verify: VERIFY_TEMPLATE_PATH,
};

/**
* Cached instruction text (loaded once per process).
* @type {string|null}
* @type {Map<string,string>}
*/
let cachedInstruction = null;
const instructionCache = new Map();

/**
* Returns the canonical keepalive instruction directive text.
* The text is loaded from .github/templates/keepalive-instruction.md.
*
* @returns {string} The instruction directive (without @agent prefix)
*/
function getKeepaliveInstruction() {
if (cachedInstruction !== null) {
return cachedInstruction;
function normalise(value) {
return String(value ?? '').trim();
}

function resolveTemplatePath({ templatePath, mode, action, reason, scenario } = {}) {
const explicit = normalise(templatePath);
if (explicit) {
return { mode: 'custom', path: explicit };
}
const resolvedMode = resolvePromptMode({ mode, action, reason, scenario });
return { mode: resolvedMode, path: TEMPLATE_PATHS[resolvedMode] || TEMPLATE_PATH };
}

function getFallbackInstruction() {
return [
'Your objective is to satisfy the **Acceptance Criteria** by completing each **Task** within the defined **Scope**.',
'',
'**This round you MUST:**',
'1. Implement actual code or test changes that advance at least one incomplete task toward acceptance.',
'2. Commit meaningful source code (.py, .yml, .js, etc.)—not just status/docs updates.',
'3. **UPDATE THE CHECKBOXES** in the Tasks and Acceptance Criteria sections below to mark completed items.',
'4. Change `- [ ]` to `- [x]` for items you have completed and verified.',
'',
'**CRITICAL - Checkbox Updates:**',
'When you complete a task or acceptance criterion, update its checkbox directly in this prompt file.',
'Change the `[ ]` to `[x]` for completed items. The automation will read these checkboxes and update the PR status summary.',
'',
'**Example:**',
'Before: `- [ ] Add validation for user input`',
'After: `- [x] Add validation for user input`',
'',
'**DO NOT:**',
'- Commit only status files, markdown summaries, or documentation when tasks require code.',
'- Mark checkboxes complete without actually implementing and verifying the work.',
'- Close the round without source-code changes when acceptance criteria require them.',
'- Change the text of checkboxes—only change `[ ]` to `[x]`.',
'',
'Review the Scope/Tasks/Acceptance below, identify the next incomplete task that requires code, implement it, then **update the checkboxes** to mark completed items.',
].join('\n');
}

function loadInstruction(templatePath, { allowDefaultFallback = true } = {}) {
const resolvedPath = templatePath || TEMPLATE_PATH;
if (instructionCache.has(resolvedPath)) {
return instructionCache.get(resolvedPath);
}

let content = '';
try {
cachedInstruction = fs.readFileSync(TEMPLATE_PATH, 'utf8').trim();
content = fs.readFileSync(resolvedPath, 'utf8').trim();
} catch (err) {
// Fallback if template file is missing
console.warn(`Warning: Could not load keepalive instruction template from ${TEMPLATE_PATH}: ${err.message}`);
cachedInstruction = [
'Your objective is to satisfy the **Acceptance Criteria** by completing each **Task** within the defined **Scope**.',
'',
'**This round you MUST:**',
'1. Implement actual code or test changes that advance at least one incomplete task toward acceptance.',
'2. Commit meaningful source code (.py, .yml, .js, etc.)—not just status/docs updates.',
'3. **UPDATE THE CHECKBOXES** in the Tasks and Acceptance Criteria sections below to mark completed items.',
'4. Change `- [ ]` to `- [x]` for items you have completed and verified.',
'',
'**CRITICAL - Checkbox Updates:**',
'When you complete a task or acceptance criterion, update its checkbox directly in this prompt file.',
'Change the `[ ]` to `[x]` for completed items. The automation will read these checkboxes and update the PR status summary.',
'',
'**Example:**',
'Before: `- [ ] Add validation for user input`',
'After: `- [x] Add validation for user input`',
'',
'**DO NOT:**',
'- Commit only status files, markdown summaries, or documentation when tasks require code.',
'- Mark checkboxes complete without actually implementing and verifying the work.',
'- Close the round without source-code changes when acceptance criteria require them.',
'- Change the text of checkboxes—only change `[ ]` to `[x]`.',
'',
'Review the Scope/Tasks/Acceptance below, identify the next incomplete task that requires code, implement it, then **update the checkboxes** to mark completed items.',
].join('\n');
if (allowDefaultFallback && resolvedPath !== TEMPLATE_PATH) {
try {
content = fs.readFileSync(TEMPLATE_PATH, 'utf8').trim();
} catch (fallbackError) {
console.warn(
`Warning: Could not load keepalive instruction template from ${resolvedPath}: ${fallbackError.message}`
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning message at line 84 displays resolvedPath in the error, but the error actually originated from trying to read TEMPLATE_PATH (the fallback). This could be confusing for debugging. Consider updating the message to clarify that both the requested path and the fallback path failed to load.

Suggested change
`Warning: Could not load keepalive instruction template from ${resolvedPath}: ${fallbackError.message}`
`Warning: Could not load keepalive instruction template. `
+ `Primary path "${resolvedPath}" failed with: ${err.message}. `
+ `Fallback path "${TEMPLATE_PATH}" also failed with: ${fallbackError.message}`

Copilot uses AI. Check for mistakes.
);
content = getFallbackInstruction();
}
} else {
console.warn(`Warning: Could not load keepalive instruction template from ${resolvedPath}: ${err.message}`);
content = getFallbackInstruction();
}
}

return cachedInstruction;
instructionCache.set(resolvedPath, content);
return content;
}

/**
* Returns the canonical keepalive instruction directive text.
* The text is loaded from .github/templates/keepalive-instruction.md.
*
* @returns {string} The instruction directive (without @agent prefix)
*/
function getKeepaliveInstruction(options = {}) {
const params = options && typeof options === 'object' ? options : {};
const resolved = resolveTemplatePath(params);
return loadInstruction(resolved.path, { allowDefaultFallback: true });
}

/**
Expand All @@ -67,20 +113,31 @@ function getKeepaliveInstruction() {
* @param {string} [agent='codex'] - The agent alias to mention
* @returns {string} The full instruction with @agent prefix
*/
function getKeepaliveInstructionWithMention(agent = 'codex') {
const alias = String(agent || '').trim() || 'codex';
return `@${alias} ${getKeepaliveInstruction()}`;
function getKeepaliveInstructionWithMention(agent = 'codex', options = {}) {
let resolvedAgent = agent;
let params = options;

if (agent && typeof agent === 'object') {
params = agent;
resolvedAgent = params.agent;
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature change for getKeepaliveInstructionWithMention makes the first parameter accept either a string (agent name) or an object (options). However, the backward compatibility logic at lines 120-123 has a potential issue: if agent is an object with a falsy agent property (e.g., { agent: '' }), the resolved agent will be falsy and fall back to 'codex', but this might not be the intended behavior. Consider explicitly checking if agent.agent exists in the object rather than just using its value.

Suggested change
resolvedAgent = params.agent;
if (Object.prototype.hasOwnProperty.call(params, 'agent')) {
resolvedAgent = params.agent;
}

Copilot uses AI. Check for mistakes.
}

const alias = String(resolvedAgent || '').trim() || 'codex';
return `@${alias} ${getKeepaliveInstruction(params)}`;
}

/**
* Clears the cached instruction (useful for testing).
*/
function clearCache() {
cachedInstruction = null;
instructionCache.clear();
}

module.exports = {
TEMPLATE_PATH,
NEXT_TASK_TEMPLATE_PATH,
FIX_TEMPLATE_PATH,
VERIFY_TEMPLATE_PATH,
getKeepaliveInstruction,
getKeepaliveInstructionWithMention,
clearCache,
Expand Down
Loading
Loading