# Review Fixes Java Run Fail

> Review pull requests with the `fixes-java-run-fail` label in graalvm-reachability-metadata. Use when asked to review or triage a PR described as `fixes-java-run-fail` or one that fixes Java/JVM runtime test failures for an existing library version update. Focus on validating the Java runtime fix, keeping the diff scoped, and applying the repair coverage gate to overall and breakdown dynamic-access reports.

- Skill: `oracle/review-fixes-java-run-fail` (Agent Skill)
- Install (CLI): `npx skillmds@latest add oracle/review-fixes-java-run-fail`
- Raw SKILL.md: https://api.skillmd.com/api/skills/oracle/review-fixes-java-run-fail/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: Oracle (https://skillmd.com/u/oracle)
- Updated: 2026-09-10
- Page: https://skillmd.com/skills/oracle/review-fixes-java-run-fail

---


# Review `fixes-java-run-fail` PRs

These PRs repair tests, dependencies, or narrow runtime setup after an existing library version compiles, but the JVM-based test fails when it runs. Review them more lightly than `library-new-request` PRs: the library already exists, so the goal is to restore Java runtime behavior for the new version while preserving meaningful test and dynamic-access coverage.

The rules below implement the contribution contract: block only on a concrete violation of a must — the shape limits (§FS-contribution-contract.2), the coverage gates (§FS-contribution-contract.3), the cheating patterns (§FS-contribution-contract.4), and the test contract's musts (§FS-test-contract) — while shoulds stay advisory in review (§FS-contribution-contract.1).

## Chunked dynamic-access PRs

When the PR has the `chunked-dynamic-access` label, skip every repair coverage
comparison in this skill, including for the final chunk. Request changes for
coverage only when the reported `dynamicAccess.coveredCalls` is `0`. Keep every
non-coverage rule. §FS-contribution-contract.3

The PR number or URL can be passed as an optional argument (for example, `1234`, `https://github.com/oracle/graalvm-reachability-metadata/pull/1234`). If the user says "review this PR" without an argument, infer the PR from the surrounding conversation or `gh pr status`; only ask the user when it cannot be inferred. Use `gh pr view <pr>`, `gh pr diff <pr>`, and `gh pr checks <pr>` against the resolved PR throughout the workflow below.

## Review Principles

- Confirm the PR has label `fixes-java-run-fail`.
- Expect runtime-focused changes plus the normal generated support files for the newly tested version: test source updates, dependency adjustments, runtime configuration, a new metadata-version directory, stats, and metadata index changes.
- Be more relaxed than `library-new-request`: do not reject only because a test resembles older coverage, stays in an existing package layout, or contains compatibility branches for multiple supported versions.
- Do not accept changes that remove meaningful test coverage just to make `java` pass.
- Native-image execution should remain enabled by default when the repaired test reaches the native lane. Do not accept changes that skip or short-circuit native-image behavior. When a test exercises behavior that fundamentally relies on open-ended dynamic class loading that native image cannot support, such as loading classes, JARs, generated bytecode, plugin implementations, or other class definitions that are only discovered after the native executable is built, accept the `NativeImageSupport.isUnsupportedFeatureError(e)` catch pattern from `org.graalvm.internal.tck`. Reject raw native-only skips, bare `catch (Error)` blocks, and use of this pattern for ordinary reflection, resources, serialization, dynamic proxies, JNI, or missing reachability metadata.
- Treat dynamic-access coverage preservation as the main quality gate. Apply the
  ordered repair comparison in the workflow below independently to the overall
  report and every breakdown present in either version.
- Use the reported stats evidence as-is. Do not inspect generation filters,
  agent configuration, or metadata contents to second-guess the reported
  `totalCalls`, `coveredCalls`, or coverage ratios.
- Treat metadata entry counts as telemetry only. Never request changes or apply
  `human-intervention` because the new version reports fewer metadata entries.
- A new version that reports zero total dynamic-access calls has no comparable
  call surface and passes the numeric coverage gate. The Java runtime fix itself must still pass and must not be made green
  by swallowing the failure or disabling native-image behavior.
- Accept only `reachability-metadata.json` files as metadata files. Reject legacy native-image metadata config files such as `reflect-config.json`, `resource-config.json`, `proxy-config.json`, `serialization-config.json`, `jni-config.json`, or `predefined-classes-config.json`.
- Prefer small, targeted review comments. This label is for JVM runtime repair work, not a full redesign of historical tests.

## Workflow

1. Inspect the PR summary.
   - Resolve the target PR from the optional argument, or infer it from context when possible.
   - Confirm the PR has label `fixes-java-run-fail`.
   - Identify the target coordinate, the previous tested version, and the new tested version from the PR body, title, changed `index.json`, and changed test path.
   - Gather files, reviews, inline comments, and CI checks.

2. Validate the diff scope.
   - Expected files are usually limited to the target coordinate's generated new-version support:
     - `metadata/<group>/<artifact>/<new-version>/reachability-metadata.json`
     - `metadata/<group>/<artifact>/index.json`
     - `stats/<group>/<artifact>/<new-version>/stats.json`
     - `tests/src/<group>/<artifact>/<new-version>/**`
   - Accept compatibility edits that keep one test source working across multiple tested versions.
   - Treat a new `reachability-metadata.json` for the tested version as normal for this label, including `{}` when validation and stats are coherent.
   - Treat generated test project files such as `.gitignore`, `build.gradle`, `gradle.properties`, `settings.gradle`, and `user-code-filter.json` as normal when they live under the target version's test directory.
   - Accept narrow runtime setup changes when they are necessary for the JVM test to exercise the same library behavior, such as test resources, dependency updates, system properties, service loading, or initialization ordering.
   - Be suspicious of unrelated build logic, workflows, generated sources, other libraries, or broad refactors.
   - Reject legacy native-image metadata config files. Metadata for generated support and test-only metadata must use `reachability-metadata.json`.
   - Reject or request changes if the PR removes tests, disables test classes, catches and ignores the failing exception, or disables native-image behavior.
   - Accept the `NativeImageSupport.isUnsupportedFeatureError(e)` catch pattern for tests that exercise behavior fundamentally requiring unsupported open-ended dynamic class loading. Reject bare `catch (Error)` blocks without the `isUnsupportedFeatureError` verification.

3. Review the Java runtime fix.
   - Confirm the edit addresses the actual JVM runtime failure, such as changed runtime behavior, missing test resources, service loading changes, dependency conflicts, classpath/module issues, initialization order, changed exception types, or changed API semantics that only fail during execution.
   - Compatibility branches are acceptable when they keep older and newer tested versions covered by the same test.
   - Version-specific logic is acceptable when the upstream runtime behavior genuinely changed, but it should be narrow and documented by the code structure or assertions.
   - Test expectation changes are acceptable only when they still assert meaningful behavior for the new version instead of accepting any output or exception.
   - Do not require the stricter `library-new-request` rules about scaffold-only tests or test package placement unless the PR is also adding a new library.

4. Check dynamic-access coverage across versions.
   - Compare `stats/<group>/<artifact>/<old-metadata-version>/stats.json` and
     `stats/<group>/<artifact>/<new-metadata-version>/stats.json` when stats are
     present in the PR or available on the branch.
   - Apply the following ordered comparison independently to the overall
     `dynamicAccess` report and every `dynamicAccess.breakdown` scope present in
     either version. Treat `N/A` on either side as `0/0`: zero `totalCalls` and
     zero `coveredCalls`.
     1. Pass when the new scope reports `totalCalls == 0`.
     2. Fail when the new scope reports `totalCalls > 0` and `coveredCalls == 0`.
     3. Pass when new `coveredCalls` is at least old `coveredCalls`, or when the
        new coverage percentage is at least the old percentage.
     4. When both scopes report fewer than 10 `totalCalls`, fail only when old
        `coveredCalls - new coveredCalls > 2`.
     5. Otherwise, fail only when coverage drops by more than 20 percentage
        points.
   - Reach step 4 only after both covered calls and percentage decreased, so the
     subtraction is always positive. A failing scope is blocking unless the PR
     gives a concrete, credible explanation of the changed upstream surface.
   - For every failing scope, inspect the old and new stats and the test diff,
     then relevant upstream API/runtime or CI evidence, before deciding. State
     the evidence-backed cause. If the evidence does not establish one, say
     `Cause not established from the available evidence` and ask for an
     explanation. Never guess or present speculation as the cause.
   - For reference: `8/8 -> 7/9` and `8/8 -> 6/9` pass the small-report rule;
     `8/8 -> 5/9` fails it; `10/10 -> 15/20` passes because covered calls grew;
     and `20/20 -> 15/20` fails the large-report rule.
   - Do not compare metadata entry counts; they are telemetry, not a review gate.
   - If required old or new stats are missing or stale, ask for
     `generateLibraryStats` or the relevant CI stats job before approving.

5. Check CI before deciding.
   - Expected minimum: `javaTest` or equivalent changed-metadata JVM test checks are green for the target coordinate.
   - Compile and metadata validation checks should also be green; a Java runtime fix should not introduce a compile or metadata validation regression.
   - Prefer seeing the full target `test` lane green because a Java runtime fix can still reduce native-image coverage.
   - If current-defaults and future-defaults lanes both run, both should pass unless the PR clearly targets only one failing lane and the other failure is unrelated infrastructure noise.
   - If CI is flaky but the diff and coverage comparison are sound, ask for a rerun instead of blocking on speculation.

## Disposition

Finding a violation is half the review. **Read §FS-contribution-contract.5 and take the disposition it assigns** — `grund FS-contribution-contract.5 --full`, or section 5 of `docs/functional-spec/contribution-contract.md` in the checkout. That section is authoritative and this table is only a dispatch: it tells you which case you are in, not what the case permits. You are the party that acts, because the PR was generated and no author is waiting to answer a review comment.

| Case | When | What you do |
|---|---|---|
| 5.1 | The violation is in the PR's own `metadata/`, `stats/`, and `tests/src/` files | Repair it on the PR head, push, re-run the affected checks, and say what you changed and why |
| 5.2 | The only possible fix is build logic, harness code, workflows, or another coordinate | Do not repair there; revert such a change if the PR already carries one, then take 5.3 |
| 5.3 | The cause is a defect in shared repository code | Find or open one issue for the defect, comment that the PR is blocked on it rather than on its own content, then take 5.5 |
| 5.4 | The library's dynamic access is reachable only through behavior Native Image cannot support | Close the PR, label the linked issue `library-unsupported-version`, and close that issue |
| 5.5 | Anything else, uncertainty included | Label the PR `human-intervention` and comment with the rule at issue, what you tried, and what you could not decide |

Never close a PR under any case but 5.4. Read §FS-contribution-contract.5.1 before you repair: it bounds what a repair may do, and it carries the one exception that lets you drop a test scenario rather than fix it.

## Decision Rules

These decide the verdict. **Disposition** above decides what you then do with it: a violation you can correct inside the PR's own files is repaired and pushed, not requested from an author who is not there.

Approve when all of these are true:

- The PR is scoped to the target existing library and the Java runtime failure it fixes.
- Tests still execute and assert the same meaningful library behavior after the runtime repair.
- The overall report and every breakdown pass the ordered repair coverage gate, or any failing scope is convincingly explained by a changed upstream surface.
- Required metadata, compile, and Java runtime test checks are green.

Request changes when any of these are true:

- The fix makes JVM tests pass by deleting tests, skipping execution, swallowing the failing exception.
- The fix disables native-image behavior instead of using the `NativeImageSupport.isUnsupportedFeatureError(e)` catch pattern for unsupported open-ended dynamic class loading.
- The overall report or a breakdown fails the ordered repair coverage gate without a credible explanation and replacement coverage.
- CI failures indicate the Java runtime problem is not actually fixed.

Ask for follow-up instead of rejecting when:

- Stats needed for the old/new version comparison are missing or stale.
- CI failed in a way that looks like infrastructure noise. A transient external failure is waited out; a reproducible defect in the repository's own code is case 5.3 of **Disposition** (§FS-contribution-contract.5.3).
- A failing repair-coverage scope may reflect a plausible upstream API or runtime change, but the PR does not explain it.

## Output Style

Keep comments short and factual:

- For coverage failures: name the failing overall or breakdown scope, report old
  and new `coveredCalls`, `totalCalls`, and percentages, and identify whether it
  failed the zero-covered, small-report, or large-report rule. Ask for restored
  coverage or a concrete explanation. Explain why the regression happened
  using evidence from the stats, test diff, upstream changes, or CI. If the
  cause cannot be established, say so explicitly and request an explanation;
  never guess. Do not report a change that an earlier step of the ordered gate
  accepts.
- For deleted or bypassed coverage: say that the PR fixes Java runtime execution by removing coverage and should instead adapt the test to the new runtime behavior.
- For swallowed exceptions: say that catching or ignoring the failing exception hides the runtime failure instead of proving the library behavior works.
- For native skips that does not depend on the open-ended dynamic class loading: say that the PR avoids the failing native path instead of fixing it, so it does not demonstrate native-image runtime coverage.
- For unverified `catch (Error)`: say that dynamic class loading tests should verify Native Image failures with `NativeImageSupport.isUnsupportedFeatureError(e)` and re-throw any other error.
- For version-pinned tests: say that tests should not reference the exact library version because the same test should support multiple library versions.
- For unrelated changes: say the PR should stay scoped to the `fixes-java-run-fail` repair and remove unrelated files.
- For legacy metadata files: say that metadata must use `reachability-metadata.json` and ask for old config files such as `reflect-config.json` or `resource-config.json` to be replaced.
- For missing stats: ask for regenerated library stats or CI evidence before approval.

## Examples

Use these examples as representative patterns, not exhaustive matchers.

- Bad runtime repair that swallows the failure:

```java
@Test
void readsConfiguration() {
    try {
        Configuration config = Configuration.load("name=value");
        assertThat(config.get("name")).isEqualTo("value");
    } catch (RuntimeException ignored) {
    }
}
```

- Bad native-image skip:

```java
@Test
void readsConfiguration() {
    assumeFalse("runtime".equals(System.getProperty("org.graalvm.nativeimage.imagecode")));

    Configuration config = Configuration.load("name=value");
    assertThat(config.get("name")).isEqualTo("value");
}
```

- Approved dynamic-class-loading pattern:

```java
@Test
void loadsRuntimePluginJar() throws Exception {
    Path pluginJar = createPluginJar();
    try {
        Plugin plugin = PluginLoader.load(pluginJar, "example.Plugin");
        assertThat(plugin.name()).isEqualTo("example");
    } catch (Error e) {
        if (!NativeImageSupport.isUnsupportedFeatureError(e)) {
            throw e;
        }
        // Expected: Native Image cannot load plugin JARs discovered after build.
    }
}
```

- Bad version-pinned assertion:

```java
@Test
void reportsVersion() {
    assertThat(LibraryVersion.current()).isEqualTo("1.2.3");
}
```