From 7caba7a6df6163adc58a152e4b723f77f9224cdd Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Sun, 16 Aug 2026 17:24:56 +0300 Subject: [PATCH 1/2] qodo: stop the config tripping the loader's directive check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo has been rejecting .pr_agent.toml outright since #2268 merged: ❌ PR-Agent failed to apply 'local' repo settings Error message: Forbidden directive: preload PR-Agent parses the file with a hardened Dynaconf loader that refuses the directives able to execute code or read arbitrary files. Matching is on the bare token, so the dynamic linker's library-injection variable — named in two of the guidelines, as something contributors must not use — trips the check even though it is only prose inside a string. The result is worse than a broken rule: the whole local config is dropped and the agent reviews with its stock prompt, so every guideline has been silently inactive since the merge while the reviews still looked normal. Both guidelines now describe the mechanism instead of naming the variable and point at best_practices.md section 4.1, which states the rule in full. best_practices.md and pr_compliance_checklist.yaml are unaffected — they are not parsed by that loader and keep the literal name. A header comment records the constraint, worded so it does not name the variable either. Co-Authored-By: Claude Opus 4.8 --- .pr_agent.toml | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/.pr_agent.toml b/.pr_agent.toml index 12aa2c13fa..e87a332d4f 100644 --- a/.pr_agent.toml +++ b/.pr_agent.toml @@ -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 = [ @@ -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. @@ -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_ 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. """ From e829edfd28f50752068c37380997d3229b20ee97 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Sun, 16 Aug 2026 17:32:49 +0300 Subject: [PATCH 2/2] qodo-gate: count a zero-finding review as a review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate looked only for a review OBJECT by the bot. Qodo creates one when it has inline comments to hang on the diff, so every PR it finds something in has one — which is why this passed on #2268 and on every devourer PR checked. A clean PR gets no review object at all. On #2269 Qodo reported "Bugs (0), Rule violations (0), Requirement gaps (0)" as a plain issue comment and created nothing else, so the gate failed with "no Qodo review on this PR yet" and could never go green — the check blocked hardest precisely when there was nothing to fix. Accept either signal: a review object, or an issue comment headed "Code Review by Qodo". The header is matched specifically rather than "any comment by the bot", because Qodo also posts "Qodo is busy working" (a placeholder written before it has read anything), "PR Summary by Qodo" (/describe output), and the "failed to apply 'local' repo settings" error. Counting those would pass a PR the agent never reviewed. Verified against the live API: #2269 now 0 objects + 1 comment -> PASS, #2268 1 + 1 -> PASS, and the three noise comments are all rejected. Thread resolution is unaffected: a zero-finding PR opens no threads, so the unresolved count is 0 and the gate falls through to PASS. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/qodo-gate.yml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/qodo-gate.yml b/.github/workflows/qodo-gate.yml index 9ba5a2a7b0..8af61d4583 100644 --- a/.github/workflows/qodo-gate.yml +++ b/.github/workflows/qodo-gate.yml @@ -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 @@ -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\")) + | .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).