Skip to content

Add @CopilotExperimental compile-time gate for experimental APIs#1601

Merged
edburns merged 12 commits into
mainfrom
edburns/dd-3012834-experimental-annotation
Jun 9, 2026
Merged

Add @CopilotExperimental compile-time gate for experimental APIs#1601
edburns merged 12 commits into
mainfrom
edburns/dd-3012834-experimental-annotation

Conversation

@edburns

@edburns edburns commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

This change adds @CopilotExperimental and a processor that rejects
unapproved references to experimental SDK APIs at compile time unless the
consumer opts in with -Acopilot.experimental.allowed=true.

What changed

New files

File Purpose
java/src/main/java/com/github/copilot/CopilotExperimental.java Public annotation for experimental API types and methods
java/src/main/java/com/github/copilot/CopilotExperimentalProcessor.java JSR 269 processor that enforces the opt-in gate
java/src/main/resources/META-INF/services/javax.annotation.processing.Processor Processor service registration
java/docs/adr/adr-004-copilotexperimental.md ADR documenting the design choice and tradeoffs
java/src/test/java/com/github/copilot/CopilotExperimentalProcessorTest.java Compile-time tests for fail-by-default and opt-in behavior

Updated files

File Change
java/scripts/codegen/java.ts Emits @CopilotExperimental alongside existing experimental @apiNote Javadocs
java/src/generated/java/** Regenerated to annotate experimental types and methods
java/pom.xml Compiler config updated for internal opt-in build usage
java/src/main/java/module-info.java Removed the extra compiler-module dependency
java/README.md Documents opt-in usage, caught cases, and known declaration-level limitations

How it works

The processor is intentionally declaration-level only:

  1. It scans standard Java elements with javax.lang.model.*.
  2. It checks field types, method parameters, return types, supertypes,
    thrown types, and generic type usage.
  3. If any referenced type or method is annotated @CopilotExperimental,
    compilation fails unless -Acopilot.experimental.allowed=true is set.

This keeps the implementation portable across Java compilers while still
covering the main ways the SDK’s generated experimental APIs are used.

Important review points

  1. The processor no longer depends on compiler-specific AST APIs.
  2. The module descriptor stays simple; no extra compiler-module wiring is required.
  3. The README explicitly documents what is and is not caught, so the tradeoff is visible to users.
  4. The generator still preserves existing experimental @apiNote Javadocs; the annotation is additive.
  5. The tests prove both failure-by-default and success-with-opt-in behavior using in-memory compilation.

Test coverage

  • Fails when an experimental type appears in declarations
  • Fails when a class extends an experimental type
  • Passes when the compiler option -Acopilot.experimental.allowed=true is present

Outcome

Experimental APIs are now opt-in at compile time, with a portable implementation and explicit documentation of the remaining limitations.

edburns added 2 commits June 6, 2026 13:36
## Summary

Introduces a compile-time enforcement mechanism that prevents accidental use of
experimental APIs in the Java SDK. Consumer code that references experimental
types or methods now fails to compile by default, with explicit opt-in via
`-Acopilot.experimental.allowed=true`.

## Motivation

Generated SDK APIs marked as `stability: "experimental"` in the schema were
previously only documented via `@apiNote` in Javadoc — invisible to the compiler
and easy to miss. This change makes the experimental contract enforceable at
build time, giving consumers a clear signal and an explicit opt-in path.

## What changed

### New files

| File | Purpose |
|------|---------|
| `java/src/main/java/com/github/copilot/CopilotExperimental.java` | Annotation: `@Documented`, `@Retention(CLASS)`, `@Target({TYPE, METHOD})` |
| `java/src/main/java/com/github/copilot/CopilotExperimentalProcessor.java` | Annotation processor using Trees API + `TreePathScanner` |
| `java/src/main/resources/META-INF/services/javax.annotation.processing.Processor` | Service registration for classpath-based discovery |
| `java/src/test/java/com/github/copilot/CopilotExperimentalProcessorTest.java` | 3 tests proving fail-by-default and allow-when-opted-in |

### Modified files

| File | Change |
|------|--------|
| `java/src/main/java/module-info.java` | Added `requires static jdk.compiler;` and `provides` directive |
| `java/pom.xml` | Compiler plugin: `<proc>none</proc>` + `-Acopilot.experimental.allowed=true`; surefire args for `jdk.compiler` exports |
| `java/scripts/codegen/java.ts` | Emits `@CopilotExperimental` + import at 3 codegen sites |
| `java/src/generated/java/**` | Regenerated — experimental types/methods now annotated |
| `java/README.md` | New "Using experimental APIs" section with examples |

## How it works

### Processor behavior

1. Runs with `@SupportedAnnotationTypes("*")` — processes all compilation units.
2. Checks option `copilot.experimental.allowed`:
   - If `"true"`: processor is a no-op (all experimental use permitted).
   - Otherwise: scans source trees for forbidden references.
3. Uses `com.sun.source.util.Trees` + `TreePathScanner` to visit:
   - `IdentifierTree` — direct type/variable references
   - `MemberSelectTree` — qualified access (`obj.method`)
   - `MemberReferenceTree` — method references (`Type::method`)
   - `NewClassTree` — constructor calls (`new ExperimentalType()`)
4. For each resolved element, checks if it or its enclosing type carries
   `@CopilotExperimental`. If so, emits `Diagnostic.Kind.ERROR`.

### Diagnostic message

```
error: Use of experimental API 'symbolName' is not allowed.
       Add compiler option -Acopilot.experimental.allowed=true to opt in.
```

### Bootstrap handling

The processor lives in the same module it protects. To avoid the bootstrap
problem (javac trying to load the processor before it's compiled):

- Main compile uses `<proc>none</proc>` — annotation processing is disabled
  during this module's own compilation.
- Consumer builds discover the processor via `META-INF/services` (classpath) or
  `provides` directive (module path).

### Codegen integration

The TypeScript codegen (`java/scripts/codegen/java.ts`) was updated at three
sites where `stability === "experimental"` is detected:

1. **Session event classes** — `@CopilotExperimental` on the class
2. **RPC params/result records** — `@CopilotExperimental` on the class
3. **API wrapper methods** — `@CopilotExperimental` on individual methods

Import (`com.github.copilot.CopilotExperimental`) is added to the generated
file's import set only when needed. Existing `@apiNote` Javadoc is preserved.

## Key review points for a Java SME

1. **Trees API usage** — The processor uses only `com.sun.source.*` (exported
   from `jdk.compiler`), not `com.sun.tools.javac.*` internals. This is the
   standard supported API for source-level annotation processors.

2. **Module wiring** — `requires static jdk.compiler` means the dependency is
   compile-time only; the module works at runtime without `jdk.compiler` on the
   module graph. The `provides` directive enables modular service discovery.

3. **Surefire JVM args** — Tests that instantiate the processor directly need
   `--add-modules jdk.compiler --add-exports jdk.compiler/com.sun.source.util=ALL-UNNAMED`
   because surefire runs on the classpath (unnamed module) while `Trees` is in
   `jdk.compiler`.

4. **proc=none trade-off** — The SDK module itself never runs its own processor.
   Enforcement is consumer-facing only. This is intentional: the SDK must
   compile its own generated experimental code without error.

5. **Retention=CLASS** — The annotation is not available via reflection at
   runtime (`RUNTIME` is unnecessary), but is preserved in `.class` files so the
   processor can read it from compiled dependencies on the classpath.

6. **Enclosing-type propagation** — When a type is annotated, all member
   accesses through that type are treated as experimental, even if the member
   itself is not annotated. This covers field access, method calls, and
   constructor invocations on experimental types.

## Test coverage

| Test | Asserts |
|------|---------|
| `failsByDefault_whenReferencingExperimentalType` | ERROR diagnostic with "experimental API" for type usage |
| `failsByDefault_whenInvokingExperimentalMethod` | ERROR diagnostic for method-level annotation |
| `passes_whenOptInFlagIsProvided` | Zero errors when `-Acopilot.experimental.allowed=true` is set |

Tests use `javax.tools.JavaCompiler` with in-memory sources and explicit
`setProcessors()` to avoid environment sensitivity.

## How to verify

```bash
cd java
mvn generate-sources -Pcodegen   # regenerate with annotations
mvn test -Dtest=CopilotExperimentalProcessorTest  # run gate tests
```
Copilot AI review requested due to automatic review settings June 8, 2026 17:39
@edburns edburns requested a review from a team as a code owner June 8, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@edburns edburns force-pushed the edburns/dd-3012834-experimental-annotation branch from 622004b to b7378ae Compare June 8, 2026 17:44
@edburns edburns marked this pull request as draft June 8, 2026 17:54
@github-actions

This comment has been minimized.

…m.sun APIs)

Replace Trees/TreePathScanner-based implementation with standard
javax.lang.model.* declaration-level checking. This removes the
dependency on jdk.compiler and makes the processor portable across
all Java compilers (javac, ECJ, etc.).

- Remove requires static jdk.compiler from module-info
- Remove --add-exports surefire hacks from pom.xml
- Update tests to validate declaration-level detection
- Document known limitations and workarounds in README
- Add ADR-004 explaining the design decision and trade-offs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment thread java/src/main/java/com/github/copilot/CopilotExperimentalProcessor.java Outdated
modified:   src/main/java/com/github/copilot/CopilotExperimentalProcessor.java

- Address comment from John Oliver.
@github-actions

This comment has been minimized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@JonathanGiles JonathanGiles left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'll note is that we played around with similar concepts in the past and they didn't buy us as much as we hoped. This isn't to say it's a bad idea, it's just that we found most customers would be oblivious to both the annotation (because they don't read docs as much as we like), nor do they run the tools we would like them to run. Having said that, if you can generate this and apply it with ease, I think it is better to have it rather than not - but it won't protect you from customers being furious when something you've marked as experimental is ripped away from them in a new release (see our lived experience of com.sun.* for reference!)

The only thing I note (without fully knowing your code base) is that it seems odd to me that the annotation lives in the root package.

@github-actions

This comment has been minimized.

@edburns

edburns commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator Author

@JonathanGiles wrote:

The only thing I note (without fully knowing your code base) is that it seems odd to me that the annotation lives in the root package.

There are very few packages in this API:

image

I tend to oppose creating a new annotations package just for these things.

Is that ok?

@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Jun 9, 2026
@github-actions

This comment has been minimized.

@JonathanGiles JonathanGiles left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Looks good to me.

@github-actions

This comment has been minimized.

@edburns edburns marked this pull request as ready for review June 9, 2026 16:11
@edburns

edburns commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

@SteveSandersonMS wrote:

From my point of view if this has been reviewed by Java SMEs then it's good

…834-experimental-annotation

deleted:    dd-3012834-remove-before-merge/20260606-prompts.md
deleted:    dd-3012834-remove-before-merge/dd-3012835-plan-to-implement-CopilotExperimental.md
deleted:    dd-3012834-remove-before-merge/dd-3012835-sketch.md
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR is Java-only and adds @CopilotExperimental + a JSR 269 annotation processor for compile-time enforcement of experimental API usage. Here's how it maps to the other SDKs:

SDK Mechanism Enforcement
Java (this PR) @CopilotExperimental + JSR 269 processor ❌ Compile error by default; opt-in via flag or @AllowCopilotExperimental
.NET (existing) [Experimental("GHCP001")] attribute ⚠️ Compile warning; suppress via #pragma warning disable GHCP001
Go // Experimental: doc comments 📖 Documentation only
Node.js/TS /** @experimental */ JSDoc 📖 Documentation only
Python # Experimental: ... comments 📖 Documentation only
Rust /// **Experimental.** doc comments 📖 Documentation only

API Surface Consistency ✅

The same experimental API surface (Canvas, Plugins, AgentRegistry types) is correctly marked experimental across all SDKs. The Java codegen update in java.ts emitting @CopilotExperimental alongside the existing @apiNote Javadoc is consistent with how other SDK codegens handle the stability: "experimental" schema flag.

Enforcement Parity Observation

Java now has error-level enforcement (fail-by-default), while .NET has warning-level enforcement — a mild asymmetry, but both are idiomatic for their ecosystems. The other four SDKs (Go, Python, TypeScript, Rust) have documentation-only markers with no compile-time gate, which is expected given language constraints:

  • TypeScript has no first-class annotation processor mechanism; an ESLint custom rule could enforce /** @experimental */ tags, but this would be a significant addition.
  • Go/Python/Rust similarly lack analogous enforcement mechanisms.

This asymmetry is acceptable — the Java PR follows the same spirit as .NET's [Experimental] attribute while using the idiomatic Java approach (annotation processor). The ADR (adr-004) clearly documents the design decisions and limitations.

Summary

No cross-SDK consistency issues to block this PR. The experimental API marking is consistent across all SDKs; the compile-time enforcement is Java-specific and mirrors the existing .NET approach. The documentation (README + ADR) is thorough.

Generated by SDK Consistency Review Agent for issue #1601 · sonnet46 1.4M ·

@stephentoub stephentoub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@edburns edburns added this pull request to the merge queue Jun 9, 2026
Merged via the queue into main with commit 9e9cfbe Jun 9, 2026
23 checks passed
@edburns edburns deleted the edburns/dd-3012834-experimental-annotation branch June 9, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants