Skip to content

ci: refine security reports and fix off-by-one in update-pins #377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 29, 2025
Merged

ci: refine security reports and fix off-by-one in update-pins #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74b6fd6
ci: mark PR required for change review via workflow_dispatch
xenoscopic Oct 29, 2025
b3db82b
ci: fix off-by-one error in update-pins PR count restrictions
xenoscopic Oct 29, 2025
b8950c0
ci: relocate beta preamble to check summary
xenoscopic Oct 29, 2025
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
26 changes: 10 additions & 16 deletions .github/workflows/security-review-changes.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ on:
workflow_dispatch:
inputs:
pull_request_number:
description: "Optional pull request number to review"
required: false
description: "Pull request number to review"
required: true
default: ""
agent:
description: "Optional reviewer agent (claude or codex)."
Expand Down Expand Up @@ -402,11 +402,13 @@ jobs:
# Review succeeded - determine conclusion from labels.
conclusion=$(determine_conclusion "$labels_path")

# Build summary text.
# Build summary text with beta preamble.
beta_notice=$'**⚠️ Beta Feature:** This automated security review is designed to aid human assessment and may contain spurious findings. Please use your judgment when evaluating the results.\n\n'

if [ "$review_type" = "differential" ]; then
summary="Differential review completed (${base_commit:0:7}...${head_commit:0:7})"
summary="${beta_notice}Differential review completed (${base_commit:0:7}...${head_commit:0:7})"
else
summary="Full code review completed at ${head_commit:0:7}"
summary="${beta_notice}Full code review completed at ${head_commit:0:7}"
fi

# Read labels for summary.
Expand All @@ -416,26 +418,18 @@ jobs:
fi

# Read report and truncate if necessary.
beta_preamble=$'**⚠️ Beta Feature:** This automated security review is designed to aid human assessment and may contain spurious findings. Please use your judgment when evaluating the results.\n\n---\n\n'

if [ -s "$report_path" ]; then
report_text=$(cat "$report_path")
report_size=${#report_text}

# Account for preamble size.
preamble_size=${#beta_preamble}
max_report_size=$((max_check_output_size - preamble_size))

if [ "$report_size" -gt "$max_report_size" ]; then
if [ "$report_size" -gt "$max_check_output_size" ]; then
# Truncate and add notice.
truncate_at=$((max_report_size - 200))
truncate_at=$((max_check_output_size - 200))
report_text="${report_text:0:$truncate_at}"
report_text=$''"${report_text}"$'\n\n---\n\n**Note:** Report truncated due to size limits. Full report available in workflow artifacts.'
fi

report_text="${beta_preamble}${report_text}"
else
report_text="${beta_preamble}No report generated."
report_text="No report generated."
fi

# Update check with results.
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/update-pins.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,6 @@ jobs:
fi
fi

# Check PR limit for new branches only.
if [ "$branch_exists" = false ] && [ -n "$new_pr_limit" ] && [ "$new_pr_count" -ge "$new_pr_limit" ]; then
echo "New PR quota reached ($new_pr_limit); skipping $server."
continue
fi

# Apply the patch onto a fresh branch for this server.
git checkout -B "$branch" origin/main
if ! git apply "$patch"; then
Expand Down Expand Up @@ -182,6 +176,12 @@ jobs:
failed_servers+=("$server (update)")
fi
else
# Check PR limit before creating new PR.
if [ -n "$new_pr_limit" ] && [ "$new_pr_count" -ge "$new_pr_limit" ]; then
echo "New PR quota reached ($new_pr_limit); skipping $server."
continue
fi
Comment on lines 176 to +183
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Oct 29, 2025

Choose a reason for hiding this comment

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

P1 Badge Avoid pushing branches when PR quota prevents creation

The new PR limit is now enforced only after the branch is pushed. When the quota has been reached, the loop still applies the patch, commits, and force-pushes automation/update-pin-${server}, but then skips gh pr create. On the next run the code sees the pushed branch, finds that servers/${server}/server.yaml already pins the target commit, and immediately skips the server, so a PR is never opened for that update. Previously the check happened before pushing, allowing a later run to create the PR when the quota permitted. This regression causes updates processed after hitting the quota to be silently dropped until a newer upstream commit appears.

Useful? React with 👍 / 👎.


if gh pr create \
--title "chore: update pin for ${server}" \
--body "Automated commit pin update for ${server}." \
Expand Down