You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
On PR #2946, the human reviewer jrichter1 commented on Aug 14 that disablePlugins in rhdh.configure() does nothing unless GIT_PR_NUMBER is set, making the conditional nightly-mode guard unnecessary complexity. The PR author had added branching logic (isNightlyMode ? {} : { disablePlugins: [...] }) to avoid calling disablePlugins in nightly mode — but the option is already a no-op there. This led to an add-then-revert cycle (commits 4a78646 → d867a24) that added noise to the PR's 38-day iteration history.
The review agent ran 14 successful review cycles on this PR and never identified this issue. It commented on disablePlugins configuration multiple times but only analyzed it at the YAML/config level — never recognizing the mode-dependent runtime behavior. The agent lacked the framework knowledge that disablePlugins works by modifying auto-generated plugin configurations during PR mode and has no effect in nightly or local dev modes.
This gap is not isolated. Multiple open E2E failure issues stem from mode-dependent configuration misunderstandings: #3458 (notifications disabled in nightly), #3455 (extensions use local paths, no OCI artifacts for nightly), #3303 (bulk-import module stripped by nightly config merge), #3044 (quickstart plugin disabled in nightly).
What could go better
The existing AGENTS.md E2E section documents the three deployment modes (PR, nightly, local dev) in the Plugin Metadata Resolution table, and issue #3112 proposes guidance for the dynamicPlugins option specifically (baked-in plugin case). However, no guidance covers the broader set of configure() options and their mode-dependent behaviors — particularly disablePlugins.
Confidence: High that this is a real gap. The human reviewer caught it immediately from experience; the agent missed it across 14 review cycles. The pattern of E2E failures linked to mode-dependent config misunderstandings (#3458, #3455, #3303, #3044) suggests this is a recurring knowledge gap affecting both agents and human contributors.
Uncertainty: The full set of mode-dependent configure() options may extend beyond disablePlugins and dynamicPlugins. An exhaustive inventory would require reviewing the e2e-test-utils source, which is in a separate repo.
Proposed change
Add a subsection to the AGENTS.md E2E Testing section titled "Mode-Dependent configure() Options" that documents which rhdh.configure() options behave differently across deployment modes. At minimum, document:
disablePlugins — Only effective in PR mode (when GIT_PR_NUMBER is set). In PR mode, it removes named plugins from the auto-generated dynamic-plugins configuration. In nightly and local dev modes, plugins come from fixed OCI refs or local paths respectively, and disablePlugins is silently ignored. Reviewers should flag conditional guards around disablePlugins based on mode (e.g., isNightlyMode ? {} : { disablePlugins: [...] }) as unnecessary complexity.
dynamicPlugins — (Complements Add AGENTS.md guidance for baked-in plugin E2E test configuration #3112) Passing dynamicPlugins in nightly mode causes the framework to generate disabled: true for baked-in plugins. Reviewers should flag dynamicPlugins usage without a nightly-mode guard.
This guidance should be placed near the existing "Plugin Metadata Resolution" table to create a coherent reference for mode-dependent behaviors. Add a review instruction: "When reviewing E2E test configure() calls, verify that mode-dependent options are used in the correct mode context."
Validation criteria
On the next 3 PRs that modify rhdh.configure() calls in E2E tests with mode-dependent options (disablePlugins, dynamicPlugins), the review agent should either: (a) correctly identify unnecessary mode guards, or (b) flag mode-dependent options used outside their effective mode. The review agent should not produce findings asking for conditional guards around options that are already no-ops in certain modes.
What happened
On PR #2946, the human reviewer jrichter1 commented on Aug 14 that
disablePluginsinrhdh.configure()does nothing unlessGIT_PR_NUMBERis set, making the conditional nightly-mode guard unnecessary complexity. The PR author had added branching logic (isNightlyMode ? {} : { disablePlugins: [...] }) to avoid callingdisablePluginsin nightly mode — but the option is already a no-op there. This led to an add-then-revert cycle (commits4a78646→d867a24) that added noise to the PR's 38-day iteration history.The review agent ran 14 successful review cycles on this PR and never identified this issue. It commented on
disablePluginsconfiguration multiple times but only analyzed it at the YAML/config level — never recognizing the mode-dependent runtime behavior. The agent lacked the framework knowledge thatdisablePluginsworks by modifying auto-generated plugin configurations during PR mode and has no effect in nightly or local dev modes.This gap is not isolated. Multiple open E2E failure issues stem from mode-dependent configuration misunderstandings: #3458 (notifications disabled in nightly), #3455 (extensions use local paths, no OCI artifacts for nightly), #3303 (bulk-import module stripped by nightly config merge), #3044 (quickstart plugin disabled in nightly).
What could go better
The existing AGENTS.md E2E section documents the three deployment modes (PR, nightly, local dev) in the Plugin Metadata Resolution table, and issue #3112 proposes guidance for the
dynamicPluginsoption specifically (baked-in plugin case). However, no guidance covers the broader set ofconfigure()options and their mode-dependent behaviors — particularlydisablePlugins.Confidence: High that this is a real gap. The human reviewer caught it immediately from experience; the agent missed it across 14 review cycles. The pattern of E2E failures linked to mode-dependent config misunderstandings (#3458, #3455, #3303, #3044) suggests this is a recurring knowledge gap affecting both agents and human contributors.
Uncertainty: The full set of mode-dependent
configure()options may extend beyonddisablePluginsanddynamicPlugins. An exhaustive inventory would require reviewing thee2e-test-utilssource, which is in a separate repo.Proposed change
Add a subsection to the AGENTS.md E2E Testing section titled "Mode-Dependent configure() Options" that documents which
rhdh.configure()options behave differently across deployment modes. At minimum, document:disablePlugins— Only effective in PR mode (whenGIT_PR_NUMBERis set). In PR mode, it removes named plugins from the auto-generated dynamic-plugins configuration. In nightly and local dev modes, plugins come from fixed OCI refs or local paths respectively, anddisablePluginsis silently ignored. Reviewers should flag conditional guards arounddisablePluginsbased on mode (e.g.,isNightlyMode ? {} : { disablePlugins: [...] }) as unnecessary complexity.dynamicPlugins— (Complements Add AGENTS.md guidance for baked-in plugin E2E test configuration #3112) PassingdynamicPluginsin nightly mode causes the framework to generatedisabled: truefor baked-in plugins. Reviewers should flagdynamicPluginsusage without a nightly-mode guard.This guidance should be placed near the existing "Plugin Metadata Resolution" table to create a coherent reference for mode-dependent behaviors. Add a review instruction: "When reviewing E2E test
configure()calls, verify that mode-dependent options are used in the correct mode context."Validation criteria
On the next 3 PRs that modify
rhdh.configure()calls in E2E tests with mode-dependent options (disablePlugins, dynamicPlugins), the review agent should either: (a) correctly identify unnecessary mode guards, or (b) flag mode-dependent options used outside their effective mode. The review agent should not produce findings asking for conditional guards around options that are already no-ops in certain modes.Generated by retro agent from #2946