diff --git a/.github/actions/setup-api-client/action.yml b/.github/actions/setup-api-client/action.yml index 3e43bc863..5fead358b 100644 --- a/.github/actions/setup-api-client/action.yml +++ b/.github/actions/setup-api-client/action.yml @@ -266,7 +266,10 @@ runs: npm_err=$(cat "$npm_output") rm -f "$npm_output" - echo "::warning::npm install attempt $attempt/$NPM_MAX_RETRIES failed: $npm_err" + echo "::warning::npm install attempt $attempt/$NPM_MAX_RETRIES failed (see logs)" + echo "::group::npm stderr (attempt $attempt)" + echo "$npm_err" + echo "::endgroup::" # On first failure, also try --legacy-peer-deps in case it's a peer dep conflict if [ "$attempt" -eq 1 ]; then @@ -279,7 +282,10 @@ runs: fi npm_err_legacy=$(cat "$npm_output") rm -f "$npm_output" - echo "::warning::npm install with --legacy-peer-deps failed: $npm_err_legacy" + echo "::warning::npm install with --legacy-peer-deps failed (see logs)" + echo "::group::npm stderr (--legacy-peer-deps)" + echo "$npm_err_legacy" + echo "::endgroup::" fi if [ "$attempt" -lt "$NPM_MAX_RETRIES" ]; then diff --git a/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js b/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js index 673a5d13c..64cec070d 100644 --- a/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js +++ b/.github/scripts/__tests__/agents-pr-meta-keepalive.test.js @@ -3,7 +3,7 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { detectKeepalive } = require('../agents_pr_meta_keepalive.js'); +const { detectKeepalive, extractIssueNumberFromPull } = require('../agents_pr_meta_keepalive.js'); function createCore(outputs) { return { @@ -543,3 +543,64 @@ test('detectKeepalive does not cache empty pull responses', async () => { assert.equal(outputsFirst.reason, 'pull-fetch-failed'); assert.equal(outputsSecond.reason, 'pull-fetch-failed'); }); + +// --- extractIssueNumberFromPull tests --- + +test('extractIssueNumberFromPull returns null for null input', () => { + assert.equal(extractIssueNumberFromPull(null), null); +}); + +test('extractIssueNumberFromPull extracts from meta comment', () => { + const pull = { body: 'Some text more text', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), 42); +}); + +test('extractIssueNumberFromPull extracts from branch name', () => { + const pull = { body: '', head: { ref: 'codex/issue-99' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), 99); +}); + +test('extractIssueNumberFromPull extracts from title', () => { + const pull = { body: '', head: { ref: 'feature' }, title: 'fix: resolve #55' }; + assert.equal(extractIssueNumberFromPull(pull), 55); +}); + +test('extractIssueNumberFromPull extracts from body hash ref', () => { + const pull = { body: 'Fixes #123', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), 123); +}); + +test('extractIssueNumberFromPull skips "Run #NNN" in body', () => { + const pull = { body: 'Run #2615 timed out after 45 minutes', head: { ref: 'claude/fix-something' }, title: 'fix: pre-timeout watchdog' }; + assert.equal(extractIssueNumberFromPull(pull), null); +}); + +test('extractIssueNumberFromPull skips "run #NNN" case-insensitive', () => { + const pull = { body: 'The run #500 failed', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), null); +}); + +test('extractIssueNumberFromPull skips "attempt #N" in body', () => { + const pull = { body: 'attempt #3 was successful', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), null); +}); + +test('extractIssueNumberFromPull skips "step #N" in body', () => { + const pull = { body: 'step #2 completed', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), null); +}); + +test('extractIssueNumberFromPull skips "version #N" in body', () => { + const pull = { body: 'Upgraded to version #4', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), null); +}); + +test('extractIssueNumberFromPull prefers meta comment over "Run #NNN"', () => { + const pull = { body: ' Run #2615 timed out', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), 77); +}); + +test('extractIssueNumberFromPull finds real issue after skipping Run ref', () => { + const pull = { body: 'Run #2615 timed out. Relates to #88', head: { ref: 'feature' }, title: 'stuff' }; + assert.equal(extractIssueNumberFromPull(pull), 88); +}); diff --git a/.github/scripts/agents_pr_meta_keepalive.js b/.github/scripts/agents_pr_meta_keepalive.js index 7a2197536..10eab2c81 100644 --- a/.github/scripts/agents_pr_meta_keepalive.js +++ b/.github/scripts/agents_pr_meta_keepalive.js @@ -240,6 +240,11 @@ function extractIssueNumberFromPull(pull) { if (match.index > 0 && /\w/.test(bodyText[match.index - 1])) { continue; } + // Skip non-issue refs like "Run #123", "run #123", "attempt #2" + const preceding = bodyText.slice(Math.max(0, match.index - 20), match.index); + if (/\b(?:run|attempt|step|job|check|task|version|v)\s*$/i.test(preceding)) { + continue; + } candidates.push(match[1]); } diff --git a/.github/scripts/agents_pr_meta_update_body.js b/.github/scripts/agents_pr_meta_update_body.js index ec9a585aa..3e987c10c 100644 --- a/.github/scripts/agents_pr_meta_update_body.js +++ b/.github/scripts/agents_pr_meta_update_body.js @@ -404,7 +404,7 @@ function parseCheckboxStates(block) { if (inCodeBlock) { continue; } - const match = line.match(/^- \[(x| )\]\s*(.+)$/i); + const match = line.match(/^\s*- \[(x| )\]\s*(.+)$/i); if (match) { const checked = match[1].toLowerCase() === 'x'; const text = match[2].trim(); @@ -461,12 +461,13 @@ function mergeCheckboxStates(newContent, existingStates) { updated.push(line); continue; } - const match = line.match(/^- \[( )\]\s*(.+)$/); + const match = line.match(/^(\s*)- \[( )\]\s*(.+)$/); if (match) { - const text = match[2].trim(); + const indent = match[1]; + const text = match[3].trim(); const normalized = text.replace(/^-\s*/, '').trim().toLowerCase(); if (existingStates.has(normalized)) { - updated.push(`- [x] ${text}`); + updated.push(`${indent}- [x] ${text}`); continue; } } diff --git a/.github/workflows/reusable-codex-run.yml b/.github/workflows/reusable-codex-run.yml index 11b6cad0c..f5dfa67be 100644 --- a/.github/workflows/reusable-codex-run.yml +++ b/.github/workflows/reusable-codex-run.yml @@ -124,6 +124,9 @@ on: error-recovery: description: 'Suggested recovery action if failure occurred' value: ${{ jobs.codex.outputs.error-recovery }} + watchdog-saved: + description: 'Whether the pre-timeout watchdog saved uncommitted work (true/false)' + value: ${{ jobs.codex.outputs.watchdog-saved }} # LLM task analysis outputs llm-analysis-run: description: 'Whether LLM analysis was performed' @@ -188,6 +191,7 @@ jobs: error-type: ${{ steps.classify_failure.outputs.error_type }} error-recovery: ${{ steps.classify_failure.outputs.error_recovery }} error-summary: ${{ steps.classify_failure.outputs.error_summary }} + watchdog-saved: ${{ steps.run_codex.outputs.watchdog-saved }} # LLM analysis outputs llm-analysis-run: ${{ steps.llm_analysis.outputs.llm-analysis-run }} llm-completed-tasks: ${{ steps.llm_analysis.outputs.completed-tasks }} @@ -938,6 +942,74 @@ jobs: echo "Extra args: provided (${#EXTRA_ARGS[@]} arg(s))" fi + # --- Pre-timeout watchdog --- + # When the job approaches the timeout limit, this background process + # commits and pushes any uncommitted work so it isn't lost to the + # job cancellation. It fires once, 5 minutes before max_runtime. + MAX_RUNTIME_MIN=${{ inputs.max_runtime_minutes }} + GRACE_MIN=5 + WATCHDOG_DELAY=$(( (MAX_RUNTIME_MIN - GRACE_MIN) * 60 )) + echo "watchdog-saved=false" >> "$GITHUB_OUTPUT" + if [ "$WATCHDOG_DELAY" -gt 60 ]; then + ( + sleep "$WATCHDOG_DELAY" + echo "::warning::Pre-timeout watchdog fired (${GRACE_MIN}m before ${MAX_RUNTIME_MIN}m limit)" + + TARGET_BRANCH="${{ inputs.pr_ref }}" + TARGET_BRANCH="${TARGET_BRANCH#refs/heads/}" + PUSH_TOKEN="${{ steps.auth_token.outputs.push_token }}" + REMOTE_URL="https://x-access-token:${PUSH_TOKEN}@github.com/${{ github.repository }}" + + # Fetch to get current FETCH_HEAD before checking for unpushed work + git fetch "${REMOTE_URL}" "${TARGET_BRANCH}" 2>/dev/null || true + + CHANGED=$(git status --porcelain | wc -l) + UNPUSHED=0 + if git rev-parse FETCH_HEAD >/dev/null 2>&1; then + UNPUSHED=$(git rev-list FETCH_HEAD..HEAD --count 2>/dev/null || echo 0) + fi + + if [ "$CHANGED" -gt 0 ] || [ "$UNPUSHED" -gt 0 ]; then + echo "::notice::Watchdog saving ${CHANGED} uncommitted file(s) and ${UNPUSHED} unpushed commit(s)" + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + if [ "$CHANGED" -gt 0 ]; then + git add -A + git commit -m "chore(codex-keepalive): pre-timeout checkpoint (PR #${PR_NUM:-})" --no-verify || true + fi + # Rebase onto remote before pushing to avoid non-fast-forward rejection + if git rev-parse FETCH_HEAD >/dev/null 2>&1; then + if ! git rebase FETCH_HEAD 2>/dev/null; then + echo "::warning::Watchdog rebase failed; attempting merge fallback." + git rebase --abort 2>/dev/null || true + git pull --no-rebase "${REMOTE_URL}" "${TARGET_BRANCH}" \ + --allow-unrelated-histories 2>/dev/null || true + fi + fi + # Push with one retry + if ! git push "${REMOTE_URL}" "HEAD:${TARGET_BRANCH}" 2>/dev/null; then + echo "::warning::Watchdog push failed (attempt 1), retrying after fetch/rebase..." + sleep 3 + git fetch "${REMOTE_URL}" "${TARGET_BRANCH}" 2>/dev/null || true + if git rev-parse FETCH_HEAD >/dev/null 2>&1; then + git rebase FETCH_HEAD 2>/dev/null || { + git rebase --abort 2>/dev/null || true + git pull --no-rebase "${REMOTE_URL}" "${TARGET_BRANCH}" \ + --allow-unrelated-histories 2>/dev/null || true + } + fi + git push "${REMOTE_URL}" "HEAD:${TARGET_BRANCH}" 2>/dev/null || \ + echo "::warning::Watchdog push failed after retry" + fi + echo "watchdog-saved=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::Watchdog: no uncommitted or unpushed work to save" + fi + ) & + WATCHDOG_PID=$! + echo "::notice::Pre-timeout watchdog started (PID ${WATCHDOG_PID}, fires in $((WATCHDOG_DELAY/60))m)" + fi + # Run codex exec with --json to capture rich session data # JSONL events stream to stdout, final message still goes to OUTPUT_FILE CODEX_EXIT=0 @@ -954,6 +1026,12 @@ jobs: prompt_content="$(cat "$PROMPT_FILE")" "${cmd[@]}" "$prompt_content" > "$SESSION_JSONL" 2>&1 || CODEX_EXIT=$? + # Kill watchdog if Codex finished before the timer fired + if [ -n "${WATCHDOG_PID:-}" ]; then + kill "$WATCHDOG_PID" 2>/dev/null || true + wait "$WATCHDOG_PID" 2>/dev/null || true + fi + echo "exit-code=${CODEX_EXIT}" >> "$GITHUB_OUTPUT" if [ "$CODEX_EXIT" -ne 0 ]; then @@ -1008,17 +1086,29 @@ jobs: # Basic parsing (always available) python3 << 'PYEOF' + import importlib.util import os import sys - # Add .workflows-lib to path for tools imports - sys.path.insert(0, '.workflows-lib') - sys.path.insert(0, '.') session_file = os.environ.get("SESSION_JSONL", "codex-session.jsonl") github_output = os.environ.get("GITHUB_OUTPUT", "/dev/null") try: - from tools.codex_jsonl_parser import parse_codex_jsonl_file + # Load codex_jsonl_parser from the Workflows checkout by exact path. + # Consumer repos (e.g. Counter_Risk) have their own tools/ package with + # __init__.py, which shadows the Workflows tools/ on sys.path. Using + # importlib.util.spec_from_file_location bypasses sys.path entirely. + _parser_path = os.path.join( + ".workflows-lib", "tools", "codex_jsonl_parser.py" + ) + _spec = importlib.util.spec_from_file_location( + "codex_jsonl_parser", _parser_path + ) + if _spec is None or _spec.loader is None: + raise ImportError(f"Cannot load spec from {_parser_path}") + _mod = importlib.util.module_from_spec(_spec) + _spec.loader.exec_module(_mod) + parse_codex_jsonl_file = _mod.parse_codex_jsonl_file session = parse_codex_jsonl_file(session_file) @@ -1150,6 +1240,7 @@ jobs: - name: Commit and push changes id: commit + if: always() env: MODE: ${{ inputs.mode }} PR_NUMBER: ${{ inputs.pr_number }} diff --git a/docs/ci/WORKFLOW_OUTPUTS.md b/docs/ci/WORKFLOW_OUTPUTS.md index 543d15099..7e971cf5b 100644 --- a/docs/ci/WORKFLOW_OUTPUTS.md +++ b/docs/ci/WORKFLOW_OUTPUTS.md @@ -78,6 +78,7 @@ that only emit artifacts, see the "Workflows without workflow_call outputs" sect | `reusable-codex-run.yml` | `error-category` | string | Error category if failure occurred (transient/auth/resource/logic/unknown) | `needs.codex.outputs.error-category` | | `reusable-codex-run.yml` | `error-type` | string | Error type if failure occurred (codex/infrastructure/auth/unknown) | `needs.codex.outputs.error-type` | | `reusable-codex-run.yml` | `error-recovery` | string | Suggested recovery action if failure occurred | `needs.codex.outputs.error-recovery` | +| `reusable-codex-run.yml` | `watchdog-saved` | string (boolean-like) | Whether the pre-timeout watchdog saved uncommitted work (true/false) | `needs.codex.outputs.watchdog-saved` | | `reusable-codex-run.yml` | `llm-analysis-run` | string (boolean-like) | Whether LLM analysis was performed | `needs.codex.outputs.llm-analysis-run` | | `reusable-codex-run.yml` | `llm-provider` | string | LLM provider used for analysis (github-models, openai, regex-fallback) | `needs.codex.outputs.llm-provider` | | `reusable-codex-run.yml` | `llm-model` | string | Specific model used for analysis (e.g., gpt-4o, claude-3-5-sonnet) | `needs.codex.outputs.llm-model` | diff --git a/templates/consumer-repo/.github/actions/setup-api-client/action.yml b/templates/consumer-repo/.github/actions/setup-api-client/action.yml index b912fe4ad..24736497c 100644 --- a/templates/consumer-repo/.github/actions/setup-api-client/action.yml +++ b/templates/consumer-repo/.github/actions/setup-api-client/action.yml @@ -239,29 +239,65 @@ runs: # Install with pinned versions for consistency. # lru-cache is an explicit transitive dep of @octokit/auth-app required for - # GitHub App token minting; pin it here so npm always hoists it to the top - # level even if a prior cached node_modules state is missing it. - # Capture stderr for debugging if the command fails - npm_output=$(mktemp) - npm_cmd=(npm install --no-save --location=project \ - @octokit/rest@20.0.2 \ - @octokit/plugin-retry@6.0.1 \ - @octokit/plugin-paginate-rest@9.1.5 \ - @octokit/auth-app@6.0.3 \ - lru-cache@^10.0.0) - if "${npm_cmd[@]}" 2>"$npm_output"; then - rm -f "$npm_output" - else - echo "::warning::npm install failed with: $(cat "$npm_output")" - echo "::warning::Retrying with --legacy-peer-deps" + # GitHub App token minting; pin it here so npm always hoists a specific version + # even if a prior cached node_modules state is missing it. + # + # Retry with exponential backoff to survive transient npm registry errors + # (e.g. 403 Forbidden from CDN/rate-limit on safe-buffer, undici, etc.). + NPM_PACKAGES=( + @octokit/rest@20.0.2 + @octokit/plugin-retry@6.0.1 + @octokit/plugin-paginate-rest@9.1.5 + @octokit/auth-app@6.0.3 + lru-cache@10.4.3 + ) + NPM_MAX_RETRIES=3 + NPM_BACKOFF=5 # seconds; doubles each retry (5, 10) + npm_installed=false + + for (( attempt=1; attempt<=NPM_MAX_RETRIES; attempt++ )); do + npm_output=$(mktemp) + + if npm install --no-save --location=project "${NPM_PACKAGES[@]}" 2>"$npm_output"; then + rm -f "$npm_output" + npm_installed=true + break + fi + + npm_err=$(cat "$npm_output") rm -f "$npm_output" - npm_cmd=(npm install --no-save --legacy-peer-deps --location=project \ - @octokit/rest@20.0.2 \ - @octokit/plugin-retry@6.0.1 \ - @octokit/plugin-paginate-rest@9.1.5 \ - @octokit/auth-app@6.0.3 \ - lru-cache@^10.0.0) - "${npm_cmd[@]}" + echo "::warning::npm install attempt $attempt/$NPM_MAX_RETRIES failed (see logs)" + echo "::group::npm stderr (attempt $attempt)" + echo "$npm_err" + echo "::endgroup::" + + # On first failure, also try --legacy-peer-deps in case it's a peer dep conflict + if [ "$attempt" -eq 1 ]; then + echo "::warning::Retrying with --legacy-peer-deps" + npm_output=$(mktemp) + if npm install --no-save --legacy-peer-deps --location=project "${NPM_PACKAGES[@]}" 2>"$npm_output"; then + rm -f "$npm_output" + npm_installed=true + break + fi + npm_err_legacy=$(cat "$npm_output") + rm -f "$npm_output" + echo "::warning::npm install with --legacy-peer-deps failed (see logs)" + echo "::group::npm stderr (--legacy-peer-deps)" + echo "$npm_err_legacy" + echo "::endgroup::" + fi + + if [ "$attempt" -lt "$NPM_MAX_RETRIES" ]; then + echo "::notice::Waiting ${NPM_BACKOFF}s before retry..." + sleep "$NPM_BACKOFF" + NPM_BACKOFF=$((NPM_BACKOFF * 2)) + fi + done + + if [ "$npm_installed" != "true" ]; then + echo "::error::npm install failed after $NPM_MAX_RETRIES attempts" + exit 1 fi # Restore vendored package metadata that npm may have overwritten diff --git a/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js b/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js index 7a2197536..10eab2c81 100644 --- a/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js +++ b/templates/consumer-repo/.github/scripts/agents_pr_meta_keepalive.js @@ -240,6 +240,11 @@ function extractIssueNumberFromPull(pull) { if (match.index > 0 && /\w/.test(bodyText[match.index - 1])) { continue; } + // Skip non-issue refs like "Run #123", "run #123", "attempt #2" + const preceding = bodyText.slice(Math.max(0, match.index - 20), match.index); + if (/\b(?:run|attempt|step|job|check|task|version|v)\s*$/i.test(preceding)) { + continue; + } candidates.push(match[1]); } diff --git a/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js b/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js index ec9a585aa..3e987c10c 100644 --- a/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js +++ b/templates/consumer-repo/.github/scripts/agents_pr_meta_update_body.js @@ -404,7 +404,7 @@ function parseCheckboxStates(block) { if (inCodeBlock) { continue; } - const match = line.match(/^- \[(x| )\]\s*(.+)$/i); + const match = line.match(/^\s*- \[(x| )\]\s*(.+)$/i); if (match) { const checked = match[1].toLowerCase() === 'x'; const text = match[2].trim(); @@ -461,12 +461,13 @@ function mergeCheckboxStates(newContent, existingStates) { updated.push(line); continue; } - const match = line.match(/^- \[( )\]\s*(.+)$/); + const match = line.match(/^(\s*)- \[( )\]\s*(.+)$/); if (match) { - const text = match[2].trim(); + const indent = match[1]; + const text = match[3].trim(); const normalized = text.replace(/^-\s*/, '').trim().toLowerCase(); if (existingStates.has(normalized)) { - updated.push(`- [x] ${text}`); + updated.push(`${indent}- [x] ${text}`); continue; } }