Skip to content

qodo: unbreak the config loader, and stop the gate blocking clean PRs #2269

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
widgetii merged 2 commits into from
Aug 16, 2026
Merged

qodo: unbreak the config loader, and stop the gate blocking clean PRs #2269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7caba7a
qodo: stop the config tripping the loader's directive check
widgetii Aug 16, 2026
e829edf
qodo-gate: count a zero-finding review as a review
widgetii Aug 16, 2026
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
33 changes: 29 additions & 4 deletions .github/workflows/qodo-gate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,20 @@ jobs:
exit 0
fi

# Any review by the bot counts — the first pass reviews the whole
# diff, and its later in-place updates edit the same review object,
# so one review object existing == the PR has been Qodo-reviewed.
# Qodo signals a finished review in one of two ways, and a clean PR
# only ever produces the second:
#
# 1. a review OBJECT — created when it has inline comments to hang
# on the diff. Its later in-place updates edit the same object,
# so one existing == the PR has been reviewed.
# 2. an issue COMMENT headed "Code Review by Qodo" — which is ALL
# a zero-finding PR gets. Verified on #2269: "Bugs (0), Rule
# violations (0), Requirement gaps (0)", and no review object
# at all.
#
# Counting only (1) therefore blocks every clean PR forever — the
# gate can never go green precisely when there is nothing to fix.
#
# Paginated: a busy PR accumulates well over 100 review objects
# (every inline reply wraps itself in one), and the bot's first
# review is the OLDEST — exactly what a last-100 window loses
Expand All @@ -96,13 +107,27 @@ jobs:
--paginate --jq ".[]
| select(.user.login | startswith(\"$BOT\"))
| .id" | wc -l)
if [ "$reviewed" -eq 0 ]; then

# Match the review header specifically, NOT merely "a comment by the
# bot". Qodo also posts "Qodo is busy working" (a placeholder before
# it has read anything), "PR Summary by Qodo" (/describe output, not
# a review), and "failed to apply 'local' repo settings" (an error).
# Treating any of those as a review would let the gate pass on a PR
# the agent never actually reviewed.
commented=$(gh api "repos/$REPO_OWNER/$REPO_NAME/issues/$PR/comments" \
--paginate --jq ".[]
| select(.user.login | startswith(\"$BOT\"))
| select(.body | test(\"Code Review by Qodo\"; \"i\"))
Comment on lines +117 to +120

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Aug 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Gate won’t rerun on comment 🐞 Bug ☼ Reliability

qodo-gate now accepts a Qodo issue comment headed “Code Review by Qodo” as proof of review, but the
workflow is not triggered by issue-comment creation/edit events. For clean PRs (no review object),
the initial run will usually fail before Qodo posts that comment and won’t automatically rerun in
response to it, so the required check can still block until a manual rerun or another unrelated
trigger occurs.
Agent Prompt 
## Issue description
The gate now treats a Qodo *issue comment* (“Code Review by Qodo”) as a valid completion signal, but the workflow is only triggered by PR/review/review-comment events. When Qodo’s only output is an issue comment (clean PR), the workflow often fails before Qodo posts it and will not rerun because the comment itself doesn’t trigger this workflow.

## Issue Context
- The workflow’s `on:` section does not include `issue_comment`, so posting the Qodo summary comment won’t schedule a new run.
- The new logic explicitly relies on that comment (`commented=... test("Code Review by Qodo")`) to pass clean PRs.

## Fix Focus Areas
Implement one of the following (prefer the polling option to avoid broad `issue_comment` triggers):
1) **Polling in the existing run (recommended):**
   - Before failing on `reviewed==0 && commented==0`, sleep-and-retry for a bounded time (e.g., up to 5–10 minutes), re-querying reviews/comments each iteration.
   - Only fail after the timeout if both signals are still absent.

2) **Alternative:** add an `issue_comment` trigger and ensure it cannot create a false PASS on non-Qodo comments.

### Code locations to modify
- .github/workflows/qodo-gate.yml[41-47]
- .github/workflows/qodo-gate.yml[86-131]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

| .id" | wc -l)

if [ "$reviewed" -eq 0 ] && [ "$commented" -eq 0 ]; then
echo "FAIL: no Qodo review on this PR yet — it reviews new PRs"
echo "automatically within a couple of minutes; comment /review to"
echo "summon one, then re-run this check once it answers. (Outage?"
echo "Apply the skip-qodo-gate label.)"
exit 1
fi
echo "review found: $reviewed review object(s), $commented review comment(s)"

# Unresolved Qodo threads, paginated (a long-lived PR can exceed one
# 100-thread page; a truncated read must never produce a false pass).
Expand Down
28 changes: 23 additions & 5 deletions .pr_agent.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
# pr_compliance_checklist.yaml - binary hard gates
#
# Reference: https://docs.qodo.ai/install-and-configure/configuration-overview/configuration-file
#
# CAUTION - do not name the dynamic linker's library-injection environment
# variable in this file, in any case, and not inside a longer word either.
# PR-Agent parses this file with a hardened Dynaconf loader that rejects the
# directives capable of executing code or reading arbitrary files, and that
# variable's name ends in one of them. Matching is on the bare token, so the
# full variable name trips it even though it is only prose in a guideline.
#
# The failure mode is silent in the worst way: the agent posts "failed to apply
# 'local' repo settings / Forbidden directive", then reviews the PR with its
# stock prompt - so every guideline below is ignored while the review still
# looks normal. That is exactly what happened on #2268, which added this file.
#
# Write such rules in best_practices.md instead. It is plain markdown, is under
# no such restriction, and already states this one in full in section 4.1.

[github_app]
pr_commands = [
Expand Down Expand Up @@ -59,9 +74,11 @@ Prioritise, in order:
majestic streamer belong to majestic's maintainers. Redirecting a contributor is a
normal, useful review outcome; say which repo and why.

5. Monkey-patching in place of a fix. LD_PRELOAD shims are not acceptable anywhere in the
OpenIPC tree, in firmware or in builder. Neither is a kernel module that rewrites a
vendor blob's memory at runtime, nor a generated facade library checked in as a binary.
5. Monkey-patching in place of a fix. Dynamic-linker injection shims - a library forced
ahead of the real one through the linker's LD_* environment knob, spelled out in
best_practices.md section 4.1 - are not acceptable anywhere in the OpenIPC tree, in
firmware or in builder. Neither is a kernel module that rewrites a vendor blob's memory
at runtime, nor a generated facade library checked in as a binary.
Each of these is bound to one exact build of one blob and fails silently at the next
vendor drop.

Expand Down Expand Up @@ -89,6 +106,7 @@ Do not raise a finding for pre-existing code that the diff merely moves or reind
Note especially: a device-specific value added to general/overlay/ or to a shared
load_<vendor> script fails the blast-radius gate even when it is correct for the board the
contributor tested, because it changes the default for cameras already in the field. And
LD_PRELOAD has no sanctioned use in this tree - a majestic bug that appears to need one is
a majestic issue, not a firmware change.
dynamic-linker injection shims (best_practices.md section 4.1) have no sanctioned use in
this tree - a majestic bug that appears to need one is a majestic issue, not a firmware
change.
"""
Loading