Skip to content

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

Merged
widgetii merged 2 commits into
masterfrom
fix-qodo-config-preload
Aug 16, 2026
Merged

qodo: unbreak the config loader, and stop the gate blocking clean PRs#2269
widgetii merged 2 commits into
masterfrom
fix-qodo-config-preload

Conversation

@widgetii

@widgetii widgetii commented Aug 16, 2026

Copy link
Copy Markdown
Member

Problem

Since #2268 merged, Qodo has been rejecting .pr_agent.toml on every PR (example):

PR-Agent failed to apply 'local' repo settings
Error message: Forbidden directive: preload

PR-Agent parses the repo-local config with a hardened Dynaconf loader that rejects directives capable of executing code or reading arbitrary files. It matches on the bare token — and the dynamic linker's library-injection environment variable, which two of the guidelines named as something contributors must not use, ends in one of those tokens. It tripped the check while being nothing but prose inside a string.

The irony is that the rule banning the thing is what broke the config.

This failure mode is nastier than a wrong rule. The agent does not fall back to a partial config — it drops the local settings entirely and reviews with its stock prompt. So every guideline added in #2268 has been inactive since it merged, while the reviews still looked perfectly normal. Nothing on the PR indicates the standards were skipped except that one comment.

Fix

Both guidelines now describe the mechanism and point at best_practices.md section 4.1, which states the rule in full and is not affected:

-5. Monkey-patching in place of a fix. LD_PRELOAD shims are not acceptable anywhere in the
-   OpenIPC tree, in firmware or in builder.
+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.

best_practices.md and pr_compliance_checklist.yaml are untouched and keep the literal variable name — they are not parsed by that loader. The rule itself is unchanged in substance, and the agent still reaches the full wording through the standards file.

A header comment records the constraint for whoever edits this next. It is worded so that it does not name the variable either — I checked the raw file has zero occurrences of any rejected token, comments included, since I could not confirm whether the scan ignores comments.

Evidence

No hardware involved; this is review configuration.

  • Reproduced: the bot comment above, on the PR that introduced the file.
  • Mechanism confirmed against PR-Agent's documented secure loader, which rejects "includes, preloads, custom loaders, and other directives that could execute code or read arbitrary files".
  • .pr_agent.toml parses under tomllib, and the guidelines still carry the rule ('injection shims' and the section 4.1 cross-reference both present).
  • Raw-file scan for every rejected token returns zero matches.

This PR cannot test itself, and the failure comment on it is expected. PR-Agent reads the repo-local config from the default branch, not from the PR head — sensible, since otherwise a PR could inject config for its own review. Two observations pin this down:

  • On qodo: review standards, compliance gates, and a PR template #2268 the error did not appear while the PR was open; it appeared 4 seconds after merge (merged 14:20:10Z, comment 14:20:14Z). Before the merge master had no .pr_agent.toml, so there was nothing to reject.
  • On this PR the same error appears again, because master still carries the old file. The fix on this branch is invisible to the loader.

Note also that the bot renders an empty "Configuration content" block, consistent with rejecting the file before it can echo anything.

So the fix cannot be proven from here; it is verified by inspection (zero occurrences of any rejected token in the raw file, comments included). The real check is the first PR opened after this merges: if it draws a normal Qodo review with no "failed to apply 'local' repo settings" comment, the config is live. If the comment persists, the token match is broader than assumed and the remaining guidelines need the same treatment.

Second fix: qodo-gate blocked every clean PR

Found while trying to merge this PR — its own gate failed, and would have failed forever.

The gate looked only for a review object by the bot. Qodo creates one when it has inline comments to attach to the diff, so any PR it finds something in has one. That is why it passed on #2268 and on every OpenIPC/devourer PR I checked — all of them had findings.

A clean PR gets no review object at all. Qodo reported Bugs (0), Rule violations (0), Requirement gaps (0) on this PR as a plain issue comment and created nothing else, so the gate reported "no Qodo review on this PR yet" and could never go green. The check blocked hardest exactly when there was nothing to fix.

It now accepts 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 before pushing:

PR review objects review comments verdict
#2269 (clean) 0 1 PASS — was FAIL
#2268 (had findings) 1 1 PASS — unchanged

All three noise comments are correctly rejected. Thread resolution is unaffected: a zero-finding PR opens no threads, so the unresolved count is 0 and it falls through to PASS.

Unlike the config fix, this one is testable here — pull_request workflows run from the merge commit, so this PR's own qodo-gate used the fixed file and passed with review found: 0 review object(s), 1 review comment(s).

Scope

  • No kernel patches, no single-board files, no debugging tooling
  • Nothing under general/overlay/; no package sources changed
  • No code — review configuration only, does not enter any image

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR-Agent failed to apply 'local' repo settings

The configuration file needs to be a valid TOML, please fix it.


Error message:
Forbidden directive: preload

Configuration content:

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Qodo: avoid directive-token match that breaks .pr_agent.toml loading

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a header warning about Dynaconf secure-loader token scanning in repo-local config.
• Reword two guidelines to avoid the dynamic-linker env var token that triggers rejection.
• Point readers to best_practices.md §4.1 for the unchanged full policy wording.
Diagram

graph TD
  A["PR-Agent (Qodo)"] --> B["Dynaconf secure loader"] --> C[".pr_agent.toml"] --> D["Guidelines strings"] --> E["best_practices.md §4.1"]
  A --> F["pr_compliance_checklist.yaml"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Move all prose guidelines out of .pr_agent.toml
  • ➕ Eliminates risk of future token-triggered loader rejection
  • ➕ Keeps .pr_agent.toml minimal and less error-prone
  • ➖ May be constrained by what Qodo expects to be embedded vs referenced
  • ➖ Requires reorganizing how guidelines are surfaced to the agent
2. Adjust/override PR-Agent secure-loader settings (allowlist tokens)
  • ➕ Preserves the exact wording in config without rephrasing
  • ➕ Fixes the root cause rather than working around it
  • ➖ May not be possible for a hosted GitHub app / security policy
  • ➖ Higher risk: loosening loader constraints could reintroduce unsafe directives
3. Encode sensitive tokens in a way the loader won’t match (escaping/formatting)
  • ➕ Keeps literal wording without semantic rephrasing
  • ➖ Unclear whether the loader normalizes/decodes before scanning
  • ➖ More brittle and less readable than plain rewording + standards link

Recommendation: Keep the PR’s current approach as the best pragmatic fix: rephrase the two guideline mentions to avoid the directive-token substring and link to best_practices.md §4.1 for the literal policy. It fixes the immediate failure mode (config dropped entirely) without weakening the secure loader or relying on brittle escaping tricks; consider the longer-term alternative of moving more prose standards into best_practices.md if token scanning continues to bite.

Files changed (1) +23 / -5

Other (1) +23 / -5
.pr_agent.tomlPrevent secure-loader token rejection and preserve standards via best_practices.md link +23/-5

Prevent secure-loader token rejection and preserve standards via best_practices.md link

• Adds an explicit header comment documenting Dynaconf secure-loader token scanning and the failure mode when local settings are rejected. Rewords two guideline sections to avoid naming the dynamic-linker injection environment variable directly, instead referencing best_practices.md section 4.1 for the unchanged full rule.

.pr_agent.toml

@qodo-free-for-open-source-projects

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Gate won’t rerun on comment 🐞 Bug ☼ Reliability
Description
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.
Code

.github/workflows/qodo-gate.yml[R117-120]

+          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\"))
Evidence
The workflow adds a new success path based on scanning PR issue comments for “Code Review by Qodo”,
but the workflow’s triggers do not include issue_comment. Therefore, when Qodo posts only an issue
comment (clean PR case), that post will not itself retrigger this workflow to observe commented>0
and clear the gate.

.github/workflows/qodo-gate.yml[41-47]
.github/workflows/qodo-gate.yml[86-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


Grey Divider

Context
Review mode: ⚖️ Balanced: This changes executable CI gate logic and security-sensitive PR-Agent configuration parsing, with multiple independent paths and meaningful false-pass/false-fail risks; a careful single-pass review is warranted.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous review results

Review updated until commit e829edf ⚖️ Balanced

Results up to commit 7caba7a


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Qodo Logo

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 <noreply@anthropic.com>
@widgetii widgetii changed the title qodo: stop the config tripping the loader's directive check qodo: unbreak the config loader, and stop the gate blocking clean PRs Aug 16, 2026
@widgetii
widgetii merged commit d1b91f7 into master Aug 16, 2026
103 checks passed
@widgetii
widgetii deleted the fix-qodo-config-preload branch August 16, 2026 15:51
@widgetii

Copy link
Copy Markdown
Member Author

/review

Comment on lines +117 to +120
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\"))

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

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e829edf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant