diff --git a/packages/cli/src/__tests__/scripts/core-flow-smoke.test.ts b/packages/cli/src/__tests__/scripts/core-flow-smoke.test.ts index 0752879..1c06e85 100644 --- a/packages/cli/src/__tests__/scripts/core-flow-smoke.test.ts +++ b/packages/cli/src/__tests__/scripts/core-flow-smoke.test.ts @@ -1654,6 +1654,103 @@ describe('core flow smoke tests (bash scripts)', () => { expect(worker26.stdout).not.toContain('NIGHT_WATCH_RESULT:skip_locked'); }); + it('reviewer should not include a PR that merged before its worker starts in completed notification metadata', () => { + const projectDir = mkTempDir('nw-smoke-reviewer-merged-worker-'); + initGitRepo(projectDir); + fs.mkdirSync(path.join(projectDir, 'logs'), { recursive: true }); + + const fakeBin = mkTempDir('nw-smoke-reviewer-merged-worker-bin-'); + writeFakeClaude(fakeBin); + + fs.writeFileSync( + path.join(fakeBin, 'gh'), + '#!/usr/bin/env bash\n' + + 'args="$*"\n' + + 'pr_number="${3:-}"\n' + + 'if [[ "$1" == "repo" && "$2" == "view" ]]; then\n' + + " echo 'owner/repo'\n" + + ' exit 0\n' + + 'fi\n' + + 'if [[ "$1" == "pr" && "$2" == "list" ]]; then\n' + + ' if [[ "$args" == *"number,headRefName"* ]]; then\n' + + " echo -e '61\\tnight-watch/merged\\t\\n62\\tnight-watch/open\\t'\n" + + ' else\n' + + " echo -e 'night-watch/merged\\nnight-watch/open'\n" + + ' fi\n' + + ' exit 0\n' + + 'fi\n' + + 'if [[ "$1" == "pr" && "$2" == "view" ]]; then\n' + + ' if [[ "$args" == *"state"* ]]; then\n' + + ' if [[ "$pr_number" == "61" ]]; then echo "MERGED"; else echo "OPEN"; fi\n' + + ' exit 0\n' + + ' fi\n' + + ' if [[ "$args" == *"mergeStateStatus"* ]]; then\n' + + " echo 'DIRTY'\n" + + ' exit 0\n' + + ' fi\n' + + ' if [[ "$args" == *"headRefOid"* ]]; then\n' + + " echo 'abc123'\n" + + ' exit 0\n' + + ' fi\n' + + ' echo \'{"number":\'"${pr_number:-62}"\'}\'\n' + + ' exit 0\n' + + 'fi\n' + + 'if [[ "$1" == "pr" && "$2" == "comment" ]]; then\n' + + ' exit 0\n' + + 'fi\n' + + 'if [[ "$1" == "api" ]]; then\n' + + " echo '[]'\n" + + ' exit 0\n' + + 'fi\n' + + 'exit 0\n', + { encoding: 'utf-8', mode: 0o755 }, + ); + + const result = runScript(reviewerScript, projectDir, { + PATH: `${fakeBin}:${process.env.PATH}`, + NW_PROVIDER_CMD: 'claude', + NW_DEFAULT_BRANCH: 'main', + NW_BRANCH_PATTERNS: 'night-watch/', + NW_AUTO_MERGE: '0', + NW_REVIEWER_PARALLEL: '1', + NW_REVIEWER_WORKER_STAGGER: '0', + NW_REVIEWER_MAX_RUNTIME: '20', + NW_QUEUE_ENABLED: '0', + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('NIGHT_WATCH_RESULT:success_reviewed|prs=#62'); + expect(result.stdout).not.toContain('NIGHT_WATCH_RESULT:success_reviewed|prs=#61,#62'); + }); + + it('reviewer target mode should treat a merged target PR as no longer open', () => { + const projectDir = mkTempDir('nw-smoke-reviewer-target-merged-'); + initGitRepo(projectDir); + fs.mkdirSync(path.join(projectDir, 'logs'), { recursive: true }); + + const fakeBin = mkTempDir('nw-smoke-reviewer-target-merged-bin-'); + fs.writeFileSync( + path.join(fakeBin, 'gh'), + '#!/usr/bin/env bash\n' + + 'if [[ "$1" == "pr" && "$2" == "view" && "$*" == *"state"* ]]; then\n' + + " echo 'MERGED'\n" + + ' exit 0\n' + + 'fi\n' + + 'exit 1\n', + { encoding: 'utf-8', mode: 0o755 }, + ); + + const result = runScript(reviewerScript, projectDir, { + PATH: `${fakeBin}:${process.env.PATH}`, + NW_PROVIDER_CMD: 'claude', + NW_TARGET_PR: '61', + NW_QUEUE_ENABLED: '0', + }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('NIGHT_WATCH_RESULT:skip_no_open_prs'); + }); + it('executor should emit success_already_merged when PR is already merged before execution', () => { const projectDir = mkTempDir('nw-smoke-executor-already-merged-'); initGitRepo(projectDir); diff --git a/scripts/night-watch-pr-reviewer-cron.sh b/scripts/night-watch-pr-reviewer-cron.sh index 86e7ae7..06e9c58 100755 --- a/scripts/night-watch-pr-reviewer-cron.sh +++ b/scripts/night-watch-pr-reviewer-cron.sh @@ -89,6 +89,26 @@ emit_result() { fi } +is_pr_open() { + local pr_number="${1:?PR number required}" + local pr_state="" + + pr_state=$(gh pr view "${pr_number}" --json state --jq '.state // ""' 2>/dev/null || echo "") + case "${pr_state}" in + OPEN) + return 0 + ;; + MERGED|CLOSED) + return 1 + ;; + esac + + # Backward-compatible fallback for older gh versions or tests that do not + # support the state field. Treat an unknown/empty state as open only when the + # PR is still viewable by number. + gh pr view "${pr_number}" --json number >/dev/null 2>&1 +} + require_provider_on_path() { if ! ensure_provider_on_path "${PROVIDER_CMD}"; then echo "ERROR: Provider '${PROVIDER_CMD}' not found in PATH or common installation locations" >&2 @@ -720,7 +740,7 @@ fi if [ -n "${TARGET_PR}" ]; then OPEN_PRS=$( - if gh pr view "${TARGET_PR}" --json number >/dev/null 2>&1; then + if is_pr_open "${TARGET_PR}"; then echo "1" else echo "0" @@ -971,6 +991,7 @@ if [ -z "${TARGET_PR}" ] && [ "${WORKER_MODE}" != "1" ] && [ "${PARALLEL_ENABLED EXIT_CODE=0 AUTO_MERGED_PRS="" AUTO_MERGE_FAILED_PRS="" + ACTUAL_REVIEWED_PRS="" NO_CHANGES_PRS="" MAX_WORKER_ATTEMPTS=1 MAX_WORKER_FINAL_SCORE="" @@ -1015,6 +1036,7 @@ if [ -z "${TARGET_PR}" ] && [ "${WORKER_MODE}" != "1" ] && [ "${PARALLEL_ENABLED worker_status=$(printf '%s' "${worker_result}" | sed -n 's/^NIGHT_WATCH_RESULT:\([^|]*\).*$/\1/p') worker_auto_merged=$(printf '%s' "${worker_result}" | grep -oP '(?<=auto_merged=)[^|]+' || true) worker_auto_merge_failed=$(printf '%s' "${worker_result}" | grep -oP '(?<=auto_merge_failed=)[^|]+' || true) + worker_reviewed_prs=$(printf '%s' "${worker_result}" | grep -oP '(?<=prs=)[^|]+' || true) worker_attempts=$(printf '%s' "${worker_result}" | grep -oP '(?<=attempts=)[^|]+' || true) worker_final_score=$(printf '%s' "${worker_result}" | grep -oP '(?<=final_score=)[^|]+' || true) worker_no_changes=$(printf '%s' "${worker_result}" | grep -oP '(?<=no_changes_needed=)[^|]+' || true) @@ -1022,6 +1044,9 @@ if [ -z "${TARGET_PR}" ] && [ "${WORKER_MODE}" != "1" ] && [ "${PARALLEL_ENABLED AUTO_MERGED_PRS=$(append_csv "${AUTO_MERGED_PRS}" "${worker_auto_merged}") AUTO_MERGE_FAILED_PRS=$(append_csv "${AUTO_MERGE_FAILED_PRS}" "${worker_auto_merge_failed}") + if [ "${worker_status}" = "success_reviewed" ]; then + ACTUAL_REVIEWED_PRS=$(append_csv "${ACTUAL_REVIEWED_PRS}" "${worker_reviewed_prs}") + fi NO_CHANGES_PRS=$(append_csv "${NO_CHANGES_PRS}" "${worker_no_changes_prs}") if [ -z "${worker_no_changes_prs}" ] && [ "${worker_no_changes}" = "1" ]; then NO_CHANGES_PRS=$(append_csv "${NO_CHANGES_PRS}" "#${worker_pr}") @@ -1066,7 +1091,7 @@ if [ -z "${TARGET_PR}" ] && [ "${WORKER_MODE}" != "1" ] && [ "${PARALLEL_ENABLED # worker runs may have left behind. cleanup_reviewer_worktrees - emit_final_status "${EXIT_CODE}" "${PRS_NEEDING_WORK_CSV}" "${AUTO_MERGED_PRS}" "${AUTO_MERGE_FAILED_PRS}" "${MAX_WORKER_ATTEMPTS}" "${MAX_WORKER_FINAL_SCORE}" "0" "${NO_CHANGES_PRS}" + emit_final_status "${EXIT_CODE}" "${ACTUAL_REVIEWED_PRS}" "${AUTO_MERGED_PRS}" "${AUTO_MERGE_FAILED_PRS}" "${MAX_WORKER_ATTEMPTS}" "${MAX_WORKER_FINAL_SCORE}" "0" "${NO_CHANGES_PRS}" exit "${EXIT_CODE}" fi