From fb3e92e378ecb3495cc9ecb64887753e888f0170 Mon Sep 17 00:00:00 2001 From: Ludovic Champenois Date: Sat, 24 Oct 2026 00:42:46 -0700 Subject: [PATCH 1/4] Use grpc-okhttp instead of grpc-netty-shaded in the runtime jars The Cloud Tasks client bundled in runtime_impl_jetty12/121 only needs a gRPC transport found through the ManagedChannelProvider SPI; nothing in gax, gax-grpc, grpc-auth or the Cloud Tasks stubs references netty directly. Replace grpc-netty-shaded (about 4 MB compressed in each jar) with grpc-okhttp. grpc-okhttp depends on okio 3.x, which is written in Kotlin and pulls in kotlin-stdlib (another 1.5 MB). Exclude it and use okio 1.17.6 (pure Java, 92 KB) instead: every okio method and field referenced by grpc-okhttp 1.85.0 exists in okio 1.17.6, which also has the fix for CVE-2023-3635. runtime-impl-jetty121.jar: 33.7 MB with netty -> 29.3 MB. --- pom.xml | 7 +++++++ runtime/runtime_impl_jetty12/pom.xml | 27 ++++++++++++++++++++------- runtime/runtime_impl_jetty121/pom.xml | 27 ++++++++++++++++++++------- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 09168b5a..0ce5281d 100644 --- a/pom.xml +++ b/pom.xml @@ -771,6 +771,13 @@ javax.annotation-api 1.3.2 + + + com.squareup.okio + okio + 1.17.6 + com.google.auth google-auth-library-oauth2-http diff --git a/runtime/runtime_impl_jetty12/pom.xml b/runtime/runtime_impl_jetty12/pom.xml index e4482f81..35dd12c2 100644 --- a/runtime/runtime_impl_jetty12/pom.xml +++ b/runtime/runtime_impl_jetty12/pom.xml @@ -58,6 +58,24 @@ appengine-apis true + + + io.grpc + grpc-okhttp + true + + + + com.squareup.okio + okio + + + + + com.squareup.okio + okio + true + com.google.appengine runtime-util @@ -335,12 +353,6 @@ - - io.grpc:grpc-netty-shaded - - META-INF/native/** - - *:* @@ -515,7 +527,8 @@ io.grpc:grpc-auth io.grpc:grpc-context io.grpc:grpc-core - io.grpc:grpc-netty-shaded + io.grpc:grpc-okhttp + com.squareup.okio:okio io.grpc:grpc-protobuf io.grpc:grpc-protobuf-lite io.grpc:grpc-stub diff --git a/runtime/runtime_impl_jetty121/pom.xml b/runtime/runtime_impl_jetty121/pom.xml index c1fa8034..d06153ae 100644 --- a/runtime/runtime_impl_jetty121/pom.xml +++ b/runtime/runtime_impl_jetty121/pom.xml @@ -58,6 +58,24 @@ appengine-apis true + + + io.grpc + grpc-okhttp + true + + + + com.squareup.okio + okio + + + + + com.squareup.okio + okio + true + com.google.appengine runtime-util @@ -358,12 +376,6 @@ - - io.grpc:grpc-netty-shaded - - META-INF/native/** - - *:* @@ -543,7 +555,8 @@ io.grpc:grpc-auth io.grpc:grpc-context io.grpc:grpc-core - io.grpc:grpc-netty-shaded + io.grpc:grpc-okhttp + com.squareup.okio:okio io.grpc:grpc-protobuf io.grpc:grpc-protobuf-lite io.grpc:grpc-stub From e8f915874799f3af8e6819ee8ef3592127c66505 Mon Sep 17 00:00:00 2001 From: Ludovic Champenois Date: Sat, 24 Oct 2026 00:51:27 -0700 Subject: [PATCH 2/4] Keep okio on 1.x: ignore okio 2+ updates and enforce okio.version The runtime jars bundle grpc-okhttp with the pure Java okio 1.17.6. okio 2+ is written in Kotlin and needs kotlin-stdlib, which the runtime jars don't bundle (the shade artifactSet is an allowlist), so an okio upgrade would break the Cloud Tasks client at runtime. - Move the okio version to an okio.version property in the root pom. - Add a maven-enforcer requireProperty rule to the runtime_impl poms that fails the build, with an explanation, unless okio.version is 1.x. - Tell Dependabot to ignore okio >= 2.0.0. Majors are already ignored globally; this makes the okio case explicit. Mirror it in the (disabled) renovate.json. --- .github/dependabot.yml | 5 +++++ pom.xml | 4 +++- renovate.json | 6 ++++++ runtime/runtime_impl_jetty12/pom.xml | 21 +++++++++++++++++++++ runtime/runtime_impl_jetty121/pom.xml | 21 +++++++++++++++++++++ 5 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index edc83c64..bc6fcdad 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -51,6 +51,11 @@ updates: - dependency-name: "org.eclipse.jetty.*:*" - dependency-name: "org.springframework.boot:spring-boot-starter-parent" - dependency-name: "org.springframework.boot:spring-boot-starter-web" + # The runtime jars use the pure Java okio 1.x with grpc-okhttp; okio 2+ is written in Kotlin + # and needs kotlin-stdlib. okio.version is also checked by the runtime_impl poms. + - dependency-name: "com.squareup.okio:okio" + versions: + - ">= 2.0.0" - package-ecosystem: "github-actions" directory: "/" diff --git a/pom.xml b/pom.xml index 0ce5281d..16196b3a 100644 --- a/pom.xml +++ b/pom.xml @@ -69,6 +69,8 @@ 1.1.1 1.11.1 1.85.0 + + 1.17.6 12.0.40 12.1.14 2.0.20 @@ -776,7 +778,7 @@ Kotlin stdlib. All okio APIs used by grpc-okhttp are present in 1.17.6. --> com.squareup.okio okio - 1.17.6 + ${okio.version} com.google.auth diff --git a/renovate.json b/renovate.json index 89cdfbe4..a89f6221 100644 --- a/renovate.json +++ b/renovate.json @@ -28,6 +28,12 @@ ], "enabled": false }, + { + "matchPackageNames": [ + "com.squareup.okio:okio" + ], + "allowedVersions": "<2.0.0" + }, { "major": { "enabled": false diff --git a/runtime/runtime_impl_jetty12/pom.xml b/runtime/runtime_impl_jetty12/pom.xml index 35dd12c2..de6f99b5 100644 --- a/runtime/runtime_impl_jetty12/pom.xml +++ b/runtime/runtime_impl_jetty12/pom.xml @@ -335,6 +335,27 @@ + + org.apache.maven.plugins + maven-enforcer-plugin + + + require-okio-1 + + enforce + + + + + okio.version + 1\.\d+(\.\d+)* + okio.version must stay 1.x: the runtime jar bundles grpc-okhttp with the pure Java okio 1.x, while okio 2+ is written in Kotlin and needs kotlin-stdlib, which this jar does not bundle. + + + + + + org.apache.maven.plugins maven-shade-plugin diff --git a/runtime/runtime_impl_jetty121/pom.xml b/runtime/runtime_impl_jetty121/pom.xml index d06153ae..5162fffd 100644 --- a/runtime/runtime_impl_jetty121/pom.xml +++ b/runtime/runtime_impl_jetty121/pom.xml @@ -358,6 +358,27 @@ + + org.apache.maven.plugins + maven-enforcer-plugin + + + require-okio-1 + + enforce + + + + + okio.version + 1\.\d+(\.\d+)* + okio.version must stay 1.x: the runtime jar bundles grpc-okhttp with the pure Java okio 1.x, while okio 2+ is written in Kotlin and needs kotlin-stdlib, which this jar does not bundle. + + + + + + org.apache.maven.plugins maven-shade-plugin From 16a96ef465c29c47c28fccfdc88966e779d48fc1 Mon Sep 17 00:00:00 2001 From: Ludovic Champenois Date: Sat, 24 Oct 2026 01:13:10 -0700 Subject: [PATCH 3/4] Add a runtime jar dependency closure test and agent skills for runtime deps RuntimeJarDependencyClosureTest runs jdeps on runtime-impl-jetty12.jar and runtime-impl-jetty121.jar, with the other runtime deployment jars on the class path, follows every reference from com.google.appengine / com.google.apphosting classes through the bundled libraries, and fails on any missing class that is not a known, unused optional dependency. It reports the chain that reaches each missing class. On the main branch it reports 88 classes per jar, including com.google.auth.Credentials <- CloudTasksClientWrapper. Agent skills in .agent/skills (read by Antigravity; .claude/skills is a symlink to it for Claude Code): - runtime-jar-deps: keep the runtime jars' bundled libraries complete. - internal-runtime-usage: find API calls made by the runtime itself and test them end to end, including behind feature flags. - pom-dependency-change: check history, dependency trees, shaded jars and size before changing pom dependencies (with scripts/jar_size_diff.sh). - runtime-e2e-tests: build and run runtime/test, and known local pitfalls. --- .agent/skills/internal-runtime-usage/SKILL.md | 56 +++++ .agent/skills/pom-dependency-change/SKILL.md | 68 ++++++ .../scripts/jar_size_diff.sh | 63 ++++++ .agent/skills/runtime-e2e-tests/SKILL.md | 59 +++++ .agent/skills/runtime-jar-deps/SKILL.md | 67 ++++++ .claude/skills | 1 + .../RuntimeJarDependencyClosureTest.java | 210 ++++++++++++++++++ 7 files changed, 524 insertions(+) create mode 100644 .agent/skills/internal-runtime-usage/SKILL.md create mode 100644 .agent/skills/pom-dependency-change/SKILL.md create mode 100755 .agent/skills/pom-dependency-change/scripts/jar_size_diff.sh create mode 100644 .agent/skills/runtime-e2e-tests/SKILL.md create mode 100644 .agent/skills/runtime-jar-deps/SKILL.md create mode 120000 .claude/skills create mode 100644 runtime/test/src/test/java/com/google/apphosting/runtime/RuntimeJarDependencyClosureTest.java diff --git a/.agent/skills/internal-runtime-usage/SKILL.md b/.agent/skills/internal-runtime-usage/SKILL.md new file mode 100644 index 00000000..97c9c2ad --- /dev/null +++ b/.agent/skills/internal-runtime-usage/SKILL.md @@ -0,0 +1,56 @@ +--- +name: internal-runtime-usage +description: Use when changing the behavior, dependencies or feature flags of public App Engine API classes (QueueImpl, QueueFactory, DatastoreServiceImpl, MemcacheServiceImpl, URLFetchService, UserService, ApiProxy, ...), or when adding an environment-variable switch to them such as APPENGINE_USE_CLOUDTASK_PUSH_QUEUE. Finds the places where the runtime itself calls these APIs from its private class loader and makes sure they are tested end to end in both modes. +--- + +# API classes used by the runtime itself + +Applications use the App Engine APIs through `appengine-api-1.0-sdk.jar`. The **runtime** also +uses some of them, from its own class loader, with the copies bundled in +`runtime-impl-jetty12.jar` / `runtime-impl-jetty121.jar`. A change that works for applications can +therefore still break the runtime, and unit tests in `api/` do not cover it. + +## 1. Find runtime callers + +Search the code that runs in the runtime class loader: + +``` +grep -rln --include=*.java -e 'QueueFactory\|QueueImpl\|DatastoreServiceFactory\|MemcacheServiceFactory\|URLFetchServiceFactory\|UserServiceFactory\|ApiProxy' \ + shared_sdk shared_sdk_jetty12 shared_sdk_jetty121 runtime/impl runtime/runtime_impl_jetty12 runtime/runtime_impl_jetty121 runtime/lite runtime/util api/src/main/java/com/google/apphosting/utils/servlet +``` + +Narrow the pattern to the class you changed. Known runtime users include: + +| Runtime code | API used | Enabled by | +|---|---|---| +| `DeferredDatastoreSessionStore` (shared_sdk_jetty12/121) | Task Queue (`QueueFactory`, `DeferredTask`) | `` + `` | +| `DatastoreSessionStore`, `MemcacheSessionStore` | Datastore, Memcache | `true` | +| `DeferredTaskServlet` (bundled in the runtime jars) | runs `DeferredTask` payloads (Task Queue) | deferred tasks, including async session writes | +| `SessionCleanupServlet` (bundled in the runtime jars) | Datastore | expired session cleanup | +| `SweeperServlet` (bundled in the runtime jars) | `TaskProcessor` → Cloud Tasks | `APPENGINE_USE_CLOUDTASK_PUSH_QUEUE` | +| `AppEngineLoginService`, `AppEngineAuthentication`, `AppEngineUserIdentity` | Users API | `` in `web.xml` | + +Also check which API packages are bundled: the `` for `com.google.appengine:appengine-apis` +in `runtime/runtime_impl_jetty12/pom.xml` and `runtime/runtime_impl_jetty121/pom.xml`. Any class in +those packages, including new ones you add, ends up in the runtime jars. + +## 2. Test the runtime path end to end + +Unit tests are not enough: add or extend a test in `runtime/test` that starts the real runtime +(`JavaRuntimeViaHttpBase`) with an app that triggers the runtime caller. If the change is behind a +flag, run the test **with the flag on** (and keep the existing test for the flag off). +`CloudTasksSessionTest` is the template: it copies the allinone app, enables +`` in its `appengine-web.xml`, sets +`APPENGINE_USE_CLOUDTASK_PUSH_QUEUE=true` through `setEnvironmentEntries`, and asserts that the +requests succeed and that `outputLinesContaining(...)` finds no `ClassNotFoundException` / +`NoClassDefFoundError`. See the `runtime-e2e-tests` skill to build and run it. + +## 3. Check the bundled dependencies + +If the change makes the API classes use a new library, follow the `runtime-jar-deps` skill. + +## 4. Say what the internal build needs + +The internal (google3) build uses BUILD files, not these poms. In the PR, list the libraries the +runtime jars now need so the matching BUILD deps can be added internally, otherwise the change +can pass here and fail internally. diff --git a/.agent/skills/pom-dependency-change/SKILL.md b/.agent/skills/pom-dependency-change/SKILL.md new file mode 100644 index 00000000..fa7800ea --- /dev/null +++ b/.agent/skills/pom-dependency-change/SKILL.md @@ -0,0 +1,68 @@ +--- +name: pom-dependency-change +description: Use before removing or adding a Maven dependency, an artifactSet include, a relocation or a filter in any pom.xml of this repo, or when bumping a library version (including reviewing Dependabot PRs). Checks why the entry exists, what depends on it transitively, and how the shaded jars (appengine-api-1.0-sdk, remoteapi, runtime_impl) change, including their size. +--- + +# Changing Maven dependencies safely + +Many pom entries look unused but exist for a shaded jar or a runtime-only code path. For example +`io.grpc:grpc-api` in `appengine-api-1.0-sdk/pom.xml` was added for issue #381 (shaded OpenCensus +in remote_api needs `io.grpc.Context`); removing it later only kept working because another +library happened to pull it in. + +## 1. Find why the entry exists + +``` +git log --format='%h %ad %an %s' --date=short -S'ARTIFACT' -- '*pom.xml' +git log --format='%h %ad %s' --date=short -G'ARTIFACT' -- '*pom.xml' +``` + +Read the commit that added it (`git show `), and any XML comment next to the entry. Search the +issue tracker for referenced issue numbers. + +## 2. Compare the dependency trees + +Run before and after the change, for every module whose pom you touch and for the shaded modules +that depend on it: + +``` +./mvnw -q dependency:tree -pl MODULE -Dincludes=GROUP_ID -DoutputFile=/tmp/tree-before.txt +``` + +The tree omits duplicates, so a library that disappears from one branch may still be resolved +elsewhere; check the final resolved version, not only the node you edited. + +## 3. Check the shaded jars + +| Jar | Pom | What to check | +|---|---|---| +| `appengine-api-1.0-sdk.jar` (used by apps) | `appengine-api-1.0-sdk/pom.xml` | `` includes and `com.google.appengine.repackaged.*` relocations | +| `appengine-remote-api.jar` | `remoteapi/pom.xml` | `./mvnw verify -pl remoteapi` (runs `ShadingIT`) | +| `runtime-impl-jetty12.jar`, `runtime-impl-jetty121.jar` | `runtime/runtime_impl_jetty12*/pom.xml` | the `runtime-jar-deps` skill (`RuntimeJarDependencyClosureTest`) | + +Shade only bundles artifacts listed in an `artifactSet`; a library that is resolved but not listed +is silently left out of the jar. + +## 4. Report the size impact + +Build the jar before and after (use a `git worktree` of `origin/main` for "before", with +`./mvnw package` so `~/.m2` is not overwritten), then: + +``` +.agent/skills/pom-dependency-change/scripts/jar_size_diff.sh OLD.jar NEW.jar +``` + +Put the totals and the largest packages in the PR description or a PR comment. + +## 5. Version pins with constraints + +Some versions are deliberately held back and guarded, for example `okio.version` must stay 1.x +(okio 2+ needs kotlin-stdlib, which the runtime jars do not bundle): it is enforced by a +`requireProperty` rule in the runtime_impl poms and ignored for `>= 2.0.0` in +`.github/dependabot.yml`. When you add such a pin, add the same guard and the Dependabot ignore, +and do not "fix" a failing guard by loosening it. + +## 6. Internal build + +The internal (google3) build uses BUILD files, not these poms. When a shaded jar gains or loses a +library, say so in the PR so the internal BUILD deps can be updated. diff --git a/.agent/skills/pom-dependency-change/scripts/jar_size_diff.sh b/.agent/skills/pom-dependency-change/scripts/jar_size_diff.sh new file mode 100755 index 00000000..f9d5f0e2 --- /dev/null +++ b/.agent/skills/pom-dependency-change/scripts/jar_size_diff.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Compares two builds of the same jar: total size, entry and class counts, and the compressed +# bytes of added and removed entries grouped by package (first three path segments). +# +# Usage: jar_size_diff.sh OLD.jar NEW.jar +set -euo pipefail +export LC_ALL=C + +if [[ $# -ne 2 ]]; then + echo "Usage: $0 OLD.jar NEW.jar" >&2 + exit 1 +fi +old=$1 +new=$2 +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT + +# Writes "pathcompressed-size" for each file entry of a jar, sorted by path. +entries() { + zipinfo -l "$1" | awk 'NF >= 10 && $1 ~ /^-/ { print $NF "\t" $6 }' | sort +} +entries "$old" > "$tmp/old" +entries "$new" > "$tmp/new" + +size_old=$(stat -c%s "$old") +size_new=$(stat -c%s "$new") +classes_old=$(grep -c '\.class ' "$tmp/old" || true) +classes_new=$(grep -c '\.class ' "$tmp/new" || true) +printf '%-10s %12s %12s %12s\n' "" "old" "new" "delta" +printf '%-10s %12d %12d %+12d\n' "bytes" "$size_old" "$size_new" $((size_new - size_old)) +printf '%-10s %12d %12d %+12d\n' "entries" "$(wc -l < "$tmp/old")" "$(wc -l < "$tmp/new")" \ + $(($(wc -l < "$tmp/new") - $(wc -l < "$tmp/old"))) +printf '%-10s %12d %12d %+12d\n' "classes" "$classes_old" "$classes_new" \ + $((classes_new - classes_old)) + +# Prints compressed bytes and entry counts per package for entries only in the first file. +by_package() { + join -t "$(printf '\t')" -v 1 "$1" <(cut -f1 "$2") | + awk -F'\t' '{ n = split($1, p, "/"); k = p[1]; for (i = 2; i <= 3 && i < n; i++) k = k "/" p[i]; + bytes[k] += $2; count[k]++ } + END { for (k in bytes) printf "%10d B %6d entries %s\n", bytes[k], count[k], k }' | + sort -rn +} +echo +echo "Added (compressed bytes by package):" +by_package "$tmp/new" "$tmp/old" +echo +echo "Removed (compressed bytes by package):" +by_package "$tmp/old" "$tmp/new" diff --git a/.agent/skills/runtime-e2e-tests/SKILL.md b/.agent/skills/runtime-e2e-tests/SKILL.md new file mode 100644 index 00000000..ac7d107f --- /dev/null +++ b/.agent/skills/runtime-e2e-tests/SKILL.md @@ -0,0 +1,59 @@ +--- +name: runtime-e2e-tests +description: Use when building the runtime deployment or running or writing end-to-end tests in runtime/test (JavaRuntimeViaHttpBase, JavaRuntimeAllInOneTest, CloudTasksSessionTest, RuntimeJarDependencyClosureTest, ...). Covers the build commands, JDK requirements, how test parameters depend on the JDK, and common local failures that are not real regressions. +--- + +# Runtime end-to-end tests (runtime/test) + +Tests in `runtime/test` start the real runtime as a separate process from +`runtime/deployment/target/runtime-deployment-/` (the `appengine.runtime.dir` system +property, set by the root pom) and talk to it over HTTP, with a fake API server +(`HttpApiServer` / `DummyApiServer`). + +## Build + +- A full JDK is required (`javac` and `lib/ct.sym`); a JRE fails with + `release version 17 not supported`. Set `JAVA_HOME` to the JDK. +- Build what the tests need, skipping tests and integration tests: + + ``` + ./mvnw install -DskipTests -DskipITs -pl runtime/deployment,runtime/test -am + ``` + + `-pl runtime/test -am` alone does **not** rebuild the runtime jars or the deployment, so the + tests would run an old runtime. After changing a runtime_impl pom, rebuild with + `-pl runtime/runtime_impl_jetty12,runtime/runtime_impl_jetty121,runtime/deployment`. +- `-DskipTests` does not skip failsafe ITs (e.g. `remoteapi` `ShadingIT`); add `-DskipITs`. +- Do not resume a parallel (`-T`) build with `-rf`: modules that were not built yet are skipped. + +## Run + +``` +./mvnw test -pl runtime/test -Dtest=CloudTasksSessionTest -Dsurefire.failIfNoSpecifiedTests=false +``` + +`JavaRuntimeViaHttpBase.allVersions()` only returns the runtime/Jetty/EE combinations supported by +the JDK running the tests: on JDK 25 only `java25` (Jetty 12.1, EE8/EE11), which uses +`runtime-impl-jetty121.jar`. `runtime-impl-jetty12.jar` (Jetty 12.0) is only exercised on JDK 17 +and 21 (CI runs 17, 21, 25 and 28-ea). On JDK 24+ the build adds JVM flags that JDK 21 rejects, so +`-Djvm=/bin/java` does not work; rely on CI or build with JDK 21. + +## Local failures that are not regressions + +- `FailureFilterTest`, `AnnotationScanningTest`, `NoGaeApisTest`, `AsyncServletAppTest`: + `expected to be true` on `appRoot.isDirectory()` means their test web app (e.g. + `runtime/failinitfilterwebapp/target/failinitfilterwebapp-`) was not built for the + current version. Build those modules too, or run the full build. +- `GuestBookTest` runs a nested `mvnw clean install` and needs `sdk_assembly`. +- Cloud Tasks: there is no endpoint override, so with `APPENGINE_USE_CLOUDTASK_PUSH_QUEUE=true` the + client calls the real `cloudtasks.googleapis.com` and gets an auth error (`UNAUTHENTICATED` in + CI, `PERMISSION_DENIED` with local credentials). Tests must not depend on the task being + delivered. + +## Writing a test + +Extend `JavaRuntimeViaHttpBase`, copy an app with `copyAppToDir(...)`, patch its +`WEB-INF/appengine-web.xml` if needed, pass environment variables with +`RuntimeContext.Config.builder(...).setEnvironmentEntries(...)`, and check the runtime's +stdout/stderr with `RuntimeContext.outputLinesContaining(Pattern)` or +`awaitStdoutLineMatching`/`awaitStderrLineMatching`. `CloudTasksSessionTest` is a short example. diff --git a/.agent/skills/runtime-jar-deps/SKILL.md b/.agent/skills/runtime-jar-deps/SKILL.md new file mode 100644 index 00000000..0109ecbf --- /dev/null +++ b/.agent/skills/runtime-jar-deps/SKILL.md @@ -0,0 +1,67 @@ +--- +name: runtime-jar-deps +description: Use when changing App Engine API classes that are bundled into the runtime jars (com.google.appengine.api.* such as taskqueue, datastore, memcache, urlfetch, users, appidentity, oauth; com.google.apphosting.utils.servlet.*), when adding a library dependency to those API classes, or when editing the shade configuration of runtime/runtime_impl_jetty12 or runtime/runtime_impl_jetty121. Verifies that every class the runtime can reach is actually bundled, so the runtime does not fail with NoClassDefFoundError. +--- + +# Runtime jar dependency closure + +The runtime jars `runtime-impl-jetty12.jar` and `runtime-impl-jetty121.jar` are built by the +maven-shade-plugin in `runtime/runtime_impl_jetty12/pom.xml` and +`runtime/runtime_impl_jetty121/pom.xml`. They copy selected App Engine API classes out of +`appengine-apis` (see the `` for `com.google.appengine:appengine-apis`, e.g. +`com/google/appengine/api/taskqueue/*`) and bundle third-party libraries from an explicit +allowlist (the ``). + +The runtime uses those API classes **from its own private class loader**, not the application's +(for example `DeferredDatastoreSessionStore` adds Task Queue tasks for async session +persistence). Nothing relocates or bundles a library automatically: if an API class starts using a +new library and the library is not in the `artifactSet`, the runtime fails only at run time: + +``` +NoClassDefFoundError: com/google/auth/Credentials + at com.google.appengine.api.taskqueue.QueueImpl.isCloudTaskBackendEnabled(...) + at com.google.apphosting.runtime.jetty.DeferredDatastoreSessionStore.doStore(...) +``` + +`appengine-api-1.0-sdk.jar` (what applications use) is a different shade with its own +`artifactSet` and relocations; fixing one does not fix the other. + +## Check + +1. Build the runtime deployment (needs a full JDK with `javac`; see the `runtime-e2e-tests` + skill for build details): + + ``` + ./mvnw install -DskipTests -DskipITs -pl runtime/deployment,runtime/test -am + ``` + +2. Run the closure test: + + ``` + ./mvnw test -pl runtime/test -Dtest=RuntimeJarDependencyClosureTest -Dsurefire.failIfNoSpecifiedTests=false + ``` + + `runtime/test/src/test/java/com/google/apphosting/runtime/RuntimeJarDependencyClosureTest.java` + runs `jdeps` on each runtime-impl jar with the other deployment jars on the class path, follows + every reference from `com.google.appengine.*` / `com.google.apphosting.*` classes through the + bundled libraries, and lists each missing class with the chain that reaches it, e.g. + `com.google.auth.Credentials <- com.google.appengine.api.taskqueue.CloudTasksClientWrapper`. + +## Fix a failure + +- **The code path is used by the runtime** (default assumption): add the artifact that contains + the missing class to the `` of **both** runtime_impl poms. Find the + artifact with `./mvnw dependency:tree -pl runtime/runtime_impl_jetty121 -Dincludes=` + or by looking at the same library in `appengine-api-1.0-sdk/pom.xml`. Repeat until the test + passes; libraries often need further libraries (e.g. `CloudTasksClient` needs `gax-httpjson` + even for gRPC). +- Libraries that register services in `META-INF/services` (gRPC providers, etc.) rely on the + `ServicesResourceTransformer` in the shade configuration; keep it. +- **The code path is never used by the runtime**: add the package to `KNOWN_MISSING_PACKAGES` in + the test, with the reason. Do not use this to hide a library the runtime needs. +- Check the size impact with `.agent/skills/pom-dependency-change/scripts/jar_size_diff.sh` and + report it in the PR. +- The internal (google3) build does not use these poms. Say in the PR that the same libraries must + be added to the internal BUILD deps of the runtime jars. +- Add or extend an end-to-end test for the runtime code path (see the `internal-runtime-usage` + skill). diff --git a/.claude/skills b/.claude/skills new file mode 120000 index 00000000..9b058317 --- /dev/null +++ b/.claude/skills @@ -0,0 +1 @@ +../.agent/skills \ No newline at end of file diff --git a/runtime/test/src/test/java/com/google/apphosting/runtime/RuntimeJarDependencyClosureTest.java b/runtime/test/src/test/java/com/google/apphosting/runtime/RuntimeJarDependencyClosureTest.java new file mode 100644 index 00000000..edda2cdb --- /dev/null +++ b/runtime/test/src/test/java/com/google/apphosting/runtime/RuntimeJarDependencyClosureTest.java @@ -0,0 +1,210 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.apphosting.runtime; + +import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assume.assumeTrue; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.io.File; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.spi.ToolProvider; +import java.util.stream.Collectors; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +/** + * Checks that every class reachable from the App Engine classes bundled in a runtime-impl jar can + * be loaded from the runtime class path. + * + *

The runtime jars bundle App Engine API classes (e.g. all of {@code + * com.google.appengine.api.taskqueue}) that the runtime uses from its own class loader, for + * instance for async session persistence. When those classes start using a new library, the + * library must also be added to the jar's shade {@code artifactSet}, otherwise the runtime fails + * at run time with a {@code NoClassDefFoundError}. This test runs {@code jdeps} on the jar, with + * the other jars of the runtime deployment on the class path, follows every reference starting + * from {@code com.google.appengine} and {@code com.google.apphosting} classes, and fails on any + * missing class that is not a known, unused optional dependency. + */ +@RunWith(Parameterized.class) +public final class RuntimeJarDependencyClosureTest { + + /** + * Packages that are known to be referenced but not on the runtime class path, on code paths the + * runtime does not use. Matches the package itself and its subpackages. + */ + private static final ImmutableMap KNOWN_MISSING_PACKAGES = + ImmutableMap.builder() + .put("org.antlr.runtime", "borgcron Groc schedule parser from appengine-utils") + .put( + "com.google.api.client.auth.oauth2", + "Cloud Datastore v1 client (CloudDatastoreV1ClientImpl, com.google.datastore.v1)") + .put( + "com.google.api.client.googleapis", + "Cloud Datastore v1 client (CloudDatastoreV1ClientImpl, com.google.datastore.v1)") + .put("io.grpc.alts", "gax-grpc DirectPath, which is not enabled") + .put("org.eclipse.jetty.alpn.client", "ALPN support of the Jetty HTTP client") + .put("javax.transaction", "Jetty JNDI UserTransaction, needs a transaction manager") + .put("jakarta.transaction", "Jetty JNDI UserTransaction, needs a transaction manager") + .buildOrThrow(); + + private static final ImmutableList ROOT_PREFIXES = + ImmutableList.of("com.google.appengine.", "com.google.apphosting."); + + /** A {@code jdeps -verbose:class} dependency line: source, target, and where it was found. */ + private static final Pattern DEPENDENCY_LINE = + Pattern.compile("^\\s+(\\S+)\\s+->\\s+(\\S+)\\s+(.+?)\\s*$"); + + @Parameters(name = "{0}") + public static List jars() { + return Arrays.asList( + new Object[][] {{"runtime-impl-jetty12.jar"}, {"runtime-impl-jetty121.jar"}}); + } + + private final String jarName; + + public RuntimeJarDependencyClosureTest(String jarName) { + this.jarName = jarName; + } + + @Test + public void allReachableClassesAreOnRuntimeClassPath() { + Optional jdeps = ToolProvider.findFirst("jdeps"); + assumeTrue("jdeps is only available in a full JDK", jdeps.isPresent()); + String runtimeDirProperty = System.getProperty("appengine.runtime.dir"); + assumeTrue("appengine.runtime.dir is not set", runtimeDirProperty != null); + File runtimeDir = new File(runtimeDirProperty); + File jar = new File(runtimeDir, jarName); + assertWithMessage("Runtime jar %s should exist", jar).that(jar.isFile()).isTrue(); + + // The jar is loaded together with runtime-main.jar and one runtime-shared jar; use all of them + // so that only classes missing from the whole deployment are reported. + File[] otherJars = + runtimeDir.listFiles( + (dir, name) -> name.endsWith(".jar") && !name.startsWith("runtime-impl-")); + String classPath = + Arrays.stream(otherJars) + .map(File::getAbsolutePath) + .collect(Collectors.joining(File.pathSeparator)); + + StringWriter out = new StringWriter(); + StringWriter err = new StringWriter(); + int exitCode = + jdeps + .get() + .run( + new PrintWriter(out), + new PrintWriter(err), + "--multi-release", + Integer.toString(Runtime.version().feature()), + "-verbose:class", + "--ignore-missing-deps", + "--class-path", + classPath, + jar.getAbsolutePath()); + assertWithMessage("jdeps failed: %s", err).that(exitCode).isEqualTo(0); + + Map> bundledEdges = new HashMap<>(); + Map> missingEdges = new HashMap<>(); + Set classes = new HashSet<>(); + for (String line : out.toString().split("\n")) { + Matcher m = DEPENDENCY_LINE.matcher(line); + if (!m.matches()) { + continue; + } + String from = m.group(1); + String to = m.group(2); + String location = m.group(3); + classes.add(from); + if (location.equals("not found")) { + missingEdges.computeIfAbsent(from, k -> new HashSet<>()).add(to); + } else if (location.equals(jarName)) { + bundledEdges.computeIfAbsent(from, k -> new HashSet<>()).add(to); + } + } + + // Breadth-first walk over the classes of this jar, starting from the App Engine classes and + // remembering how each class was reached, to explain any failure. + Map reachedFrom = new HashMap<>(); + ArrayDeque queue = new ArrayDeque<>(); + for (String c : classes) { + if (ROOT_PREFIXES.stream().anyMatch(c::startsWith)) { + reachedFrom.put(c, null); + queue.add(c); + } + } + assertWithMessage("No App Engine classes found in %s", jar).that(queue).isNotEmpty(); + Map unexpectedMissing = new TreeMap<>(); + while (!queue.isEmpty()) { + String c = queue.remove(); + for (String missing : missingEdges.getOrDefault(c, Set.of())) { + if (!isKnownMissing(missing)) { + unexpectedMissing.putIfAbsent(missing, c); + } + } + for (String next : bundledEdges.getOrDefault(c, Set.of())) { + if (!reachedFrom.containsKey(next)) { + reachedFrom.put(next, c); + queue.add(next); + } + } + } + + List report = new ArrayList<>(); + unexpectedMissing.forEach( + (missing, from) -> report.add(missing + " <- " + referenceChain(from, reachedFrom))); + assertWithMessage( + "Classes referenced from App Engine code in %s are missing from the runtime class" + + " path. Add the library that provides them to the shade artifactSet of the" + + " runtime_impl pom (and to the internal BUILD deps), or, if the code path is" + + " never used by the runtime, to KNOWN_MISSING_PACKAGES with a reason", + jarName) + .that(report) + .isEmpty(); + } + + private static boolean isKnownMissing(String className) { + int lastDot = className.lastIndexOf('.'); + String pkg = lastDot < 0 ? "" : className.substring(0, lastDot); + return KNOWN_MISSING_PACKAGES.keySet().stream() + .anyMatch(known -> pkg.equals(known) || pkg.startsWith(known + ".")); + } + + /** Returns {@code from}'s reference chain back to an App Engine class, innermost first. */ + private static String referenceChain(String from, Map reachedFrom) { + List chain = new ArrayList<>(); + for (String c = from; c != null && chain.size() < 10; c = reachedFrom.get(c)) { + chain.add(c); + } + return String.join(" <- ", chain); + } +} From 85377602c9002f8380cd1b3247f0879bd950a509 Mon Sep 17 00:00:00 2001 From: Ludovic Champenois Date: Sat, 24 Oct 2026 01:21:44 -0700 Subject: [PATCH 4/4] Add copyright headers to the agent skill files SKILL.md must start with its YAML frontmatter for Claude Code and Antigravity to read the skill name and description, so the header follows the frontmatter. --- .agent/skills/internal-runtime-usage/SKILL.md | 16 ++++++++++++++++ .agent/skills/pom-dependency-change/SKILL.md | 16 ++++++++++++++++ .agent/skills/runtime-e2e-tests/SKILL.md | 16 ++++++++++++++++ .agent/skills/runtime-jar-deps/SKILL.md | 16 ++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/.agent/skills/internal-runtime-usage/SKILL.md b/.agent/skills/internal-runtime-usage/SKILL.md index 97c9c2ad..1db489b3 100644 --- a/.agent/skills/internal-runtime-usage/SKILL.md +++ b/.agent/skills/internal-runtime-usage/SKILL.md @@ -3,6 +3,22 @@ name: internal-runtime-usage description: Use when changing the behavior, dependencies or feature flags of public App Engine API classes (QueueImpl, QueueFactory, DatastoreServiceImpl, MemcacheServiceImpl, URLFetchService, UserService, ApiProxy, ...), or when adding an environment-variable switch to them such as APPENGINE_USE_CLOUDTASK_PUSH_QUEUE. Finds the places where the runtime itself calls these APIs from its private class loader and makes sure they are tested end to end in both modes. --- + + # API classes used by the runtime itself Applications use the App Engine APIs through `appengine-api-1.0-sdk.jar`. The **runtime** also diff --git a/.agent/skills/pom-dependency-change/SKILL.md b/.agent/skills/pom-dependency-change/SKILL.md index fa7800ea..287fb581 100644 --- a/.agent/skills/pom-dependency-change/SKILL.md +++ b/.agent/skills/pom-dependency-change/SKILL.md @@ -3,6 +3,22 @@ name: pom-dependency-change description: Use before removing or adding a Maven dependency, an artifactSet include, a relocation or a filter in any pom.xml of this repo, or when bumping a library version (including reviewing Dependabot PRs). Checks why the entry exists, what depends on it transitively, and how the shaded jars (appengine-api-1.0-sdk, remoteapi, runtime_impl) change, including their size. --- + + # Changing Maven dependencies safely Many pom entries look unused but exist for a shaded jar or a runtime-only code path. For example diff --git a/.agent/skills/runtime-e2e-tests/SKILL.md b/.agent/skills/runtime-e2e-tests/SKILL.md index ac7d107f..eae12a17 100644 --- a/.agent/skills/runtime-e2e-tests/SKILL.md +++ b/.agent/skills/runtime-e2e-tests/SKILL.md @@ -3,6 +3,22 @@ name: runtime-e2e-tests description: Use when building the runtime deployment or running or writing end-to-end tests in runtime/test (JavaRuntimeViaHttpBase, JavaRuntimeAllInOneTest, CloudTasksSessionTest, RuntimeJarDependencyClosureTest, ...). Covers the build commands, JDK requirements, how test parameters depend on the JDK, and common local failures that are not real regressions. --- + + # Runtime end-to-end tests (runtime/test) Tests in `runtime/test` start the real runtime as a separate process from diff --git a/.agent/skills/runtime-jar-deps/SKILL.md b/.agent/skills/runtime-jar-deps/SKILL.md index 0109ecbf..5949cdc2 100644 --- a/.agent/skills/runtime-jar-deps/SKILL.md +++ b/.agent/skills/runtime-jar-deps/SKILL.md @@ -3,6 +3,22 @@ name: runtime-jar-deps description: Use when changing App Engine API classes that are bundled into the runtime jars (com.google.appengine.api.* such as taskqueue, datastore, memcache, urlfetch, users, appidentity, oauth; com.google.apphosting.utils.servlet.*), when adding a library dependency to those API classes, or when editing the shade configuration of runtime/runtime_impl_jetty12 or runtime/runtime_impl_jetty121. Verifies that every class the runtime can reach is actually bundled, so the runtime does not fail with NoClassDefFoundError. --- + + # Runtime jar dependency closure The runtime jars `runtime-impl-jetty12.jar` and `runtime-impl-jetty121.jar` are built by the