Skip to content

Commit a272d1a

Browse files
committed
Improve code-review-and-fix agent: shared-module safety checks
- Expand sibling-validation rule from :testing-only to all shared/common modules (directory names containing -common, :library, :bootstrap, testing-common) — run :check for all sibling javaagent/library modules after modifying a shared module - Add visibility-reduction guard for shared modules: search sibling callers before reducing access; fix only when no sibling references the member, report otherwise to prevent IllegalAccessError in ByteBuddy advice
1 parent 9061ef6 commit a272d1a

1 file changed

Lines changed: 17 additions & 9 deletions

File tree

.github/agents/code-review-and-fix.agent.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -397,22 +397,30 @@ Execute these steps strictly in order — do not reorder:
397397
pre-existing failure — note it in the final output but do not block the commit.
398398
5. Never commit code that fails tests you can reproduce locally.
399399

400-
**Testing-module dependent validation**: when any modified module is a `testing` module
401-
(its Gradle path ends with `:testing`), you must **also** run `:check` (both normal and
402-
`-PtestLatestDeps=true`) for every sibling `library` and `javaagent` module under the
403-
same instrumentation parent. `testing` modules contain shared abstract test base classes
404-
consumed by those siblings — changes to visibility, method signatures, or class structure
405-
in the `testing` module can break compilation or tests in dependent modules.
406-
407-
To find siblings, list the parent directory of the `testing` module and look for
400+
**Shared-module dependent validation**: when any modified module is a shared module
401+
consumed by sibling instrumentation modules, you must **also** run `:check` (both normal
402+
and `-PtestLatestDeps=true`) for every sibling `library` and `javaagent` module under the
403+
same instrumentation parent. A module is shared if its Gradle path ends with `:testing`,
404+
or its directory name contains `-common` (e.g., `couchbase-2-common`, `netty-common`),
405+
or it is named `library`, `bootstrap`, or `testing-common`. Changes to visibility,
406+
method signatures, or class structure in a shared module can break compilation or tests
407+
in dependent sibling modules — including failures that compile cleanly but throw
408+
`IllegalAccessError` at runtime inside ByteBuddy advice.
409+
410+
To find siblings, list the parent directory of the shared module and look for
408411
`library/`, `javaagent/`, and any version-variant directories that contain `library/`
409-
or `javaagent/` submodules. Run `:check` for each.
412+
or `javaagent/` submodules. Also check `settings.gradle.kts` for every module under
413+
the same instrumentation group. Run `:check` for each sibling that transitively
414+
depends on the modified shared module.
410415

411416
Example: if you modify files in
412417
`:instrumentation:foo:foo-1.0:testing`, also run `:check` for
413418
`:instrumentation:foo:foo-1.0:library`,
414419
`:instrumentation:foo:foo-1.0:javaagent`, and any version-variant siblings such as
415420
`:instrumentation:foo:foo-2.0:library` if it depends on the `foo-1.0:testing` module.
421+
Likewise, if you modify files in `:instrumentation:couchbase:couchbase-2-common:javaagent`,
422+
also run `:check` for `:instrumentation:couchbase:couchbase-2.0:javaagent` and
423+
`:instrumentation:couchbase:couchbase-2.6:javaagent`.
416424

417425
Do not move on to step 2 until every required `:check` run from this step, including
418426
sibling-module validation and any re-runs after fixes or reverts, has fully completed

0 commit comments

Comments
 (0)