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
14 changes: 9 additions & 5 deletions scripts/check_codex_comments.sh
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,11 @@ done
# more Running/Completed rows, fixed "About Codex" footer). The card may also
# carry one "### Security findings" section with one "#### Advisory findings
# (N)" list of exactly N links to review threads on this PR; it only counts
# as a non-finding when every linked thread is a resolved thread that the bot
# started. An unresolved or missing thread keeps the card blocking.
# as a non-finding when every finding is cleared. Codex appends
# " · **Resolved**" to a finding it considers resolved, which clears it.
# An unmarked finding is cleared only when its linked thread is a resolved
# thread that the bot started; an unresolved or missing thread keeps the
# card blocking. Unresolved bot threads also block on their own below.
# - the explicit clean security verdict ("No security issues were found")
# Recognition is structural and line-anchored: the summary and security shapes
# must match line for line, so a marker alone, a quoted marker, or a summary
Expand Down Expand Up @@ -226,14 +229,15 @@ def advisory_heading_regex: "^#### Advisory findings \\((?<n>[1-9][0-9]{0,2})\\)
# The link label may contain brackets (e.g. `args[0]` or escaped `\[`); only "](" ends it.
def finding_regex:
"^- [^ ]{1,4} \\[(?:[^\\]]|\\](?!\\())+\\]\\(https://github\\.com/(?<owner>[^/()]+)/(?<repo>[^/()]+)/pull/(?<pr>[0-9]+)"
+ "#discussion_r(?<id>[0-9]+)\\) · \\*\\*(Critical|High|Medium|Low)\\*\\*$";
+ "#discussion_r(?<id>[0-9]+)\\) · \\*\\*(Critical|High|Medium|Low)\\*\\*(?<mark> · \\*\\*Resolved\\*\\*)?$";

# True when the finding line links to a resolved, bot-started thread on this PR.
# True when the finding line links to this PR and either Codex marked it
# " · **Resolved**" or it links to a resolved, bot-started thread.
def is_resolved_finding:
(capture(finding_regex) // null) as $m
| $m != null
and $m.owner == $owner and $m.repo == $repo and $m.pr == $pr
and ($resolved | any(. == $m.id));
and ($m.mark == " · **Resolved**" or ($resolved | any(. == $m.id)));

# Input: the card lines between the status table's blank lines and the About
# footer. Valid when empty, or when it is exactly one security findings section
Expand Down
71 changes: 70 additions & 1 deletion scripts/check_codex_comments_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
# (human requester logins in the full-page fixture are replaced by "alice"),
# except summary-security-advisory-finding-pr89.txt: the PR #89 summary card
# (comment 5812405711) with its finding title replaced by "Example advisory
# finding". Its link, severity and layout are unchanged.
# finding". Its link, severity and layout are unchanged. And
# summary-security-advisory-findings-resolved-pr212.txt: the PR #212 summary
# card (comment 5970636463) with its two finding titles replaced by "Example
# advisory finding" and "Second example advisory finding". Codex marked the
# first finding " · **Resolved**"; links, severities and layout are unchanged.
#
# Usage: ./scripts/check_codex_comments_test.sh
set -euo pipefail
Expand All @@ -22,6 +26,12 @@ BOT="chatgpt-codex-connector"
PRIMARY_SUMMARY_SHA256="cb7624eb0869f631aca6e3efde64656369924c9e5741020e78fa11287a36af2e"
# sha256 of the PR #89 summary card with one advisory finding (see above).
FINDING_SUMMARY_SHA256="d46fcf1cc41042be58dd18b514e14e8e2fd6c6a57957d5b20ce2e90aac1733ee"
# sha256 of the PR #212 summary card with one finding marked Resolved (see above).
RESOLVED_SUMMARY_SHA256="c5a0153b7193277d220854109050d6d38100a1421a088a1447eefafe4f1f9363"
# Review comment IDs the PR #212 card's findings link to: the first is marked
# Resolved in the card, the second is not.
PR212_MARKED_ID=4173840086
PR212_UNMARKED_ID=4173883868
# Review comment ID the PR #89 card's finding links to (discussion_r<ID>).
FINDING_ID=4092909628
# 2^53 + 1: the smallest integer a JSON number cannot carry exactly through jq.
Expand Down Expand Up @@ -170,12 +180,28 @@ if [ "$actual_sha" != "$FINDING_SUMMARY_SHA256" ]; then
echo "❌ Assertion failed: summary-security-advisory-finding-pr89.txt sha256 ${actual_sha} != ${FINDING_SUMMARY_SHA256}"
exit 1
fi
actual_sha=$(sha256sum "${BODIES}/summary-security-advisory-findings-resolved-pr212.txt" | cut -d' ' -f1)
if [ "$actual_sha" != "$RESOLVED_SUMMARY_SHA256" ]; then
echo "❌ Assertion failed: summary-security-advisory-findings-resolved-pr212.txt sha256 ${actual_sha} != ${RESOLVED_SUMMARY_SHA256}"
exit 1
fi

# finding_card [sed-script] -> comments page holding the PR #89 card, optionally edited
finding_card() {
page comments "$(comment_node "$BOT" "$(body summary-security-advisory-finding-pr89 | sed -e "${1:-}")")"
}

# resolved_card [sed-script] -> comments page holding the PR #212 card, optionally edited
resolved_card() {
page comments "$(comment_node "$BOT" "$(body summary-security-advisory-findings-resolved-pr212 | sed -e "${1:-}")")"
}

# Appends a suffix to the PR #89 card's finding line (after its severity).
mark_finding_sed() {
printf '/discussion_r%s/s/$/%s/' "$FINDING_ID" "$1"
}
RESOLVED_SUFFIX=' · **Resolved**'

# Second finding line and count, for the two-finding cases.
TWO_FINDINGS_SED="s/^#### Advisory findings (1)\$/#### Advisory findings (2)/;/discussion_r${FINDING_ID}/{p;s/discussion_r${FINDING_ID}/discussion_r$((FINDING_ID + 1))/}"

Expand Down Expand Up @@ -242,6 +268,20 @@ CASE_PR=89 run_case summary_finding_full_id_string_above_2p53_resolved 0 "$CLEAN
"$(finding_card "s/discussion_r${FINDING_ID}/discussion_r${BIG_ID}/")" \
"$(page reviewThreads "$(thread_node "$BOT" true "$BIG_ID")")"

# Codex appends " · **Resolved**" to a finding once its thread is resolved
# (PR #212). Every linked thread is resolved here.
CASE_PR=212 run_case summary_findings_marked_resolved_pr212 0 "$CLEAN_MSG" \
"$(resolved_card)" \
"$(page reviewThreads "$(thread_node "$BOT" true "$PR212_MARKED_ID")" "$(thread_node "$BOT" true "$PR212_UNMARKED_ID")")"

CASE_PR=89 run_case summary_finding_marked_resolved_thread_resolved 0 "$CLEAN_MSG" \
"$(finding_card "$(mark_finding_sed "$RESOLVED_SUFFIX")")" \
"$(page reviewThreads "$(thread_node "$BOT" true "$FINDING_ID")")"

# The Resolved mark alone clears a finding, even when its thread is not listed.
CASE_PR=89 run_case summary_finding_marked_resolved_thread_missing 0 "$CLEAN_MSG" \
"$(finding_card "$(mark_finding_sed "$RESOLVED_SUFFIX")")" "$NO_THREADS"

# --- Findings and lookalikes (must stay blocking) --------------------------

run_case real_finding_comment 1 "$BLOCK_MSG" \
Expand Down Expand Up @@ -363,6 +403,35 @@ CASE_PR=89 run_case summary_finding_with_extra_text 1 "Found 1 unminimized regul
"$(finding_card "/discussion_r${FINDING_ID}/a **P1** Missing bounds check.")" \
"$(page reviewThreads "$(thread_node "$BOT" true "$FINDING_ID")")"

# The unmarked PR #212 finding still needs its own resolved thread.
CASE_PR=212 run_case summary_pr212_unmarked_finding_thread_unresolved 1 "Found 1 unminimized regular comment(s) from bot" \
"$(resolved_card)" \
"$(page reviewThreads "$(thread_node "$BOT" true "$PR212_MARKED_ID")" "$(thread_node "$BOT" false "$PR212_UNMARKED_ID")")"

CASE_PR=212 run_case summary_pr212_unmarked_finding_thread_missing 1 "Found 1 unminimized regular comment(s) from bot" \
"$(resolved_card)" "$(page reviewThreads "$(thread_node "$BOT" true "$PR212_MARKED_ID")")"

# A Resolved mark clears the card, but an unresolved thread still blocks on its own.
CASE_PR=89 run_case summary_finding_marked_resolved_thread_unresolved 1 "Found 1 unresolved review thread(s) from bot" \
"$(finding_card "$(mark_finding_sed "$RESOLVED_SUFFIX")")" \
"$(page reviewThreads "$(thread_node "$BOT" false "$FINDING_ID")")"

# The mark only counts on a finding that links to this PR.
run_case summary_finding_marked_resolved_links_other_pr 1 "Found 1 unminimized regular comment(s) from bot" \
"$(finding_card "$(mark_finding_sed "$RESOLVED_SUFFIX")")" "$NO_THREADS"

# Only the exact " · **Resolved**" suffix is a mark.
for case_suffix in \
'unresolved| · **Unresolved**' \
'lowercase| · **resolved**' \
'unbolded| · Resolved' \
'repeated| · **Resolved** · **Resolved**' \
'trailing_text| · **Resolved** **P1** Missing bounds check.'; do
CASE_PR=89 run_case "summary_finding_lookalike_mark_${case_suffix%%|*}" 1 \
"Found 1 unminimized regular comment(s) from bot" \
"$(finding_card "$(mark_finding_sed "${case_suffix#*|}")")" "$NO_THREADS"
done

run_case summary_plus_finding_comment 1 "Found 1 unminimized regular comment(s) from bot" \
"$(page comments "$(comment_node "$BOT" "$(body summary-completed-both)")" "$(comment_node "$BOT" "**P2** Unused parameter.")")" "$NO_THREADS"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!-- codex-pull-request-review-summary -->
<!-- codex-security-review:v1 {"blockingSeverityThreshold":"P0","headSha":"a4bf9f1180438d6ef9bcb91aa5449ca81b2829c8","mergeGateEnabled":false,"pullRequestNumber":212,"repository":"coder/coder-k8s","status":"completed"} -->
## Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

| Review | Status | Commit | Review trigger |
| --- | --- | --- | --- |
| 📝 **Code Review** | ✅ **Completed** <relative-time datetime="2026-10-03T16:42:03.042488Z">2026-10-03T16:42:03.042488Z</relative-time> | `a4bf9f1` | Manual request |
| 🔒 **Security Review** | ✅ **Completed** <relative-time datetime="2026-10-03T16:44:14.113174Z">2026-10-03T16:44:14.113174Z</relative-time> | `a4bf9f1` | Manual request |

### Security findings

#### Advisory findings (2)

- 🟡 [Example advisory finding](https://github.com/coder/coder-k8s/pull/212#discussion_r4173840086) · **Medium** · **Resolved**
- 🟡 [Second example advisory finding](https://github.com/coder/coder-k8s/pull/212#discussion_r4173883868) · **Medium**

<details> <summary>ℹ️ About Codex in GitHub</summary>
<br/>

[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

</details>
Loading