From c0a54b09079021c607c39a3e9369470dbdf3c4c6 Mon Sep 17 00:00:00 2001
From: fullsend-code
 <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
Date: Wed, 2 Sep 2026 15:45:17 +0000
Subject: [PATCH] docs(#3534): add review guidance for dynamic-plugins.yaml
 minimality

Add a "Reviewing dynamic-plugins.yaml Changes" subsection to the E2E
Testing section of AGENTS.md, after "Configuration Files". This gives
the review agent explicit criteria to flag unnecessary entries in
dynamic-plugins.yaml files:

- Entries setting disabled: true for plugins outside the workspace
  own metadata (redundant, auto-generation only includes workspace
  plugins)
- Entries duplicating what auto-generation would produce from
  metadata appConfigExamples
- Guidance that only entries overriding specific test-environment
  values justify maintaining the file

This addresses a gap identified during NFS (app-next) migration
reviews where legacy config entries carry forward unnecessarily.

Closes #3534
---
 AGENTS.md | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/AGENTS.md b/AGENTS.md
index 585e25d853..087fee6653 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -335,6 +335,16 @@ MY_TOKEN=abc123              →    MY_TOKEN: $MY_TOKEN        →   token: ${MY
 
 `envsubst` runs **only** on `rhdh-secrets.yaml`. Other config files reference the Secret values with `${VAR}` syntax — they are not substituted directly.
 
+### Reviewing dynamic-plugins.yaml Changes
+
+When reviewing PRs that add or modify `tests/config/dynamic-plugins.yaml`, verify that each entry is necessary:
+
+- **Entries setting `disabled: true`** for plugins not used by the workspace under test are redundant. The auto-generation from metadata only includes plugins defined in the workspace's own `metadata/*.yaml` files — plugins from other workspaces are not included. Explicitly disabling them is unnecessary coupling.
+- **Entries duplicating auto-generated defaults** (same package, same config as would be derived from `metadata/*.yaml` `appConfigExamples`) should be flagged. The recommended approach is to omit `dynamic-plugins.yaml` entirely and let auto-generation handle it.
+- **Only entries that override specific values** needed for the test environment (custom config not in metadata, integrity overrides, version pins for testing) justify maintaining a `dynamic-plugins.yaml`.
+
+This is especially important during NFS (app-next) migration, where the framework handles more configuration automatically and legacy config entries often carry forward unnecessarily.
+
 ### WorkspacePaths
 
 Config file paths are resolved from `test.info().project.testDir` (Playwright-provided absolute path), NOT from `process.cwd()`. This enables the same test code to work from both: