Skip to content

Commit 7ca980a

Browse files
nficanoclaude
nficano
and
claude
committed
docs: align CONTRIBUTING.md with shared SDK template
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0ab7480 commit 7ca980a

1 file changed

Lines changed: 145 additions & 79 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 145 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,90 +1,156 @@
1-
# Contributing
1+
# Contributing to dev.arcp:arcp
2+
3+
Thanks for your interest in improving the Java SDK for ARCP. This
4+
document covers how to report issues, propose changes, and get a change merged.
5+
6+
By participating you agree to the [Code of Conduct](CODE_OF_CONDUCT.md).
7+
8+
## Where changes belong
9+
10+
ARCP is two things in two places, and a change belongs to exactly one of them:
11+
12+
- **The protocol** — the wire format, message semantics, lease rules, error
13+
taxonomy, feature flags. These live in the
14+
[specification repository](https://github.com/agentruntimecontrolprotocol/spec).
15+
If your idea changes what goes *on the wire* or what a conformant runtime must
16+
do, it is a spec change — open it there, not here. This SDK implements the
17+
spec; it does not define it.
18+
- **This SDK** — how the protocol is expressed idiomatically in Java:
19+
bugs, ergonomics, performance, missing-but-specified features, docs, tests.
20+
Those belong here.
21+
22+
When in doubt, open an issue here and we'll redirect if it's really a protocol
23+
question.
24+
25+
## The golden rule: conform, don't extend
26+
27+
A change to this SDK must keep it a faithful client of
28+
[ARCP v1.1 (draft)](https://github.com/agentruntimecontrolprotocol/spec/blob/main/docs/draft-arcp-1.1.md).
29+
Concretely:
30+
31+
- **Don't invent wire behavior.** No envelope fields, event kinds, error codes,
32+
or feature flags that the spec doesn't define. If you need one, it's a spec
33+
proposal first.
34+
- **Negotiate honestly.** Only advertise a feature flag in `session.hello` once
35+
the SDK actually implements it. The feature matrix in the README must match
36+
what the code negotiates — a row marked `Supported` is a promise.
37+
- **Respect the semantics.** Sequence numbers stay gap-free and monotonic;
38+
`LEASE_EXPIRED` and `BUDGET_EXHAUSTED` stay non-retryable; the effective
39+
feature set is the intersection of client and runtime advertisements. Tests
40+
must not paper over a semantic the spec requires.
41+
- **Stay layered.** This SDK controls runtimes. It does not expose tools (that's
42+
MCP) or export telemetry (that's OpenTelemetry). PRs that blur those layers
43+
will be asked to move the logic out.
244

3-
Thanks for considering a contribution. This SDK tracks
4-
[`draft-arcp-1.1.md`](../spec/docs/draft-arcp-1.1.md). Behavior changes that
5-
diverge from the spec are out of scope unless they fix a clear interpretation
6-
bug; please open an issue first to discuss.
45+
## Reporting bugs
46+
47+
Open an issue with: the SDK version and Java version, the runtime you
48+
connected to, a minimal reproduction (the smallest program that triggers it),
49+
what you expected, and what happened. A failing test is the best possible bug
50+
report. Wire-level traces (the envelopes exchanged) help enormously for protocol
51+
behavior — redact any `auth.token` or provisioned-credential `value` first.
52+
53+
## Proposing a change
754

8-
## Prerequisites
55+
For anything beyond a small fix, open an issue describing the problem before
56+
writing code, so we can agree on the approach. Small, focused PRs review faster
57+
than large ones; if a change is big, say so early and we'll help break it down.
958

10-
- JDK 21+ (Temurin is what CI uses). Set `JAVA_HOME` accordingly.
11-
- Gradle 9.x via the bundled wrapper — do not install Gradle separately.
12-
- Graphviz on `PATH` if you intend to touch `docs/diagrams/`.
59+
## Development setup
1360

14-
## Local checks
61+
The build targets JDK 21 LTS (`--release 21`) and is driven entirely by the
62+
bundled Gradle wrapper — do not install Gradle separately. CI runs on Temurin
63+
21 and 25; either works locally. Clone, point `JAVA_HOME` at a JDK 21+, and
64+
let the wrapper resolve everything else:
1565

16-
```bash
17-
./gradlew build # compile + test all 10 modules
18-
./gradlew :examples:cancel:run # run any of the 10 examples
19-
./gradlew :arcp-core:pitest # opt-in mutation analysis
20-
make -C docs/diagrams # render SVGs after editing .dot
66+
```sh
67+
git clone https://github.com/nficano/arpc.git
68+
cd arpc/java-sdk
69+
export JAVA_HOME=$(/usr/libexec/java_home -v 21) # macOS; use your distro's equivalent
70+
./gradlew help # bootstraps the wrapper + toolchain
2171
```
2272

23-
`./gradlew build` runs ~140 tasks and finishes in about a minute on a fresh
24-
checkout. All 18 test suites must stay green; broken tests block the PR.
25-
26-
## Coding conventions
27-
28-
- **JDK 21 floor.** `--release 21` is set in the root build; bytecode must
29-
stay consumable on JDK 21 even though the toolchain may compile on 25.
30-
- **No `--enable-preview`** on consumer-facing artifacts. `StructuredTaskScope`
31-
is preview in 21 and final in 25 with a different shape; the runtime uses
32-
plain virtual threads on `Executors.newVirtualThreadPerTaskExecutor()`.
33-
- **Records over Lombok.** All value types are records; the canonical
34-
constructor enforces invariants.
35-
- **Sealed types for taxonomies.** `Message`, `EventBody`, `ArcpException`
36-
are sealed so dispatch is exhaustive at compile time. New wire types or
37-
error codes touch the sealed permits list.
38-
- **No `null` returns without `@Nullable`.** JSpecify is on the compile
39-
classpath; mark anything that may return null.
40-
- **Thread safety on shared mutable state.** Counters use
41-
`AtomicReference` / `AtomicLong` with CAS loops; idempotency uses
42-
`ConcurrentHashMap`. Avoid `synchronized` on hot paths.
43-
- **Spec § citations in Javadoc.** Every public type implementing a
44-
protocol concept should cite the matching spec section.
45-
46-
## Test discipline
47-
48-
- A change to runtime FSM logic requires a test that exercises the
49-
transition.
50-
- A change to wire format requires an envelope round-trip assertion
51-
([`EnvelopeRoundTripPropertyTest`](arcp-core/src/test/java/dev/arcp/core/EnvelopeRoundTripPropertyTest.java)
52-
covers most of it via jqwik).
53-
- A change visible to `Lease.contains`, `BudgetCounters`, or
54-
`LeaseGuard.authorize` requires either a property or a focused unit test.
55-
56-
## Diagrams
57-
58-
Light and dark variants must remain structurally identical — only color hex
59-
codes legitimately differ. The `diagrams` workflow re-runs `dot -Tsvg` against
60-
every `.dot` source and fails the PR if a checked-in SVG diverges from the
61-
fresh render. Edit both variants together; commit both `.dot` and rendered
62-
`.svg`.
63-
64-
## Conformance
65-
66-
`CONFORMANCE.md` rows that flip from `Deferred` to `Implemented` MUST cite a
67-
`path:LineNumber` for the load-bearing code, and the PR MUST include a test
68-
that exercises that line. PRs that mark a row Implemented without an
69-
exercising test will be rejected at review.
70-
71-
## Commit + PR style
72-
73-
- Imperative summaries: "Wire heartbeat watchdog into SessionLoop", not
74-
"Added heartbeat watchdog".
75-
- One topic per PR — split mechanical refactors from behavior changes.
76-
- Reference the issue number in the PR description if one exists.
77-
- New public types or methods carry Javadoc and at minimum one test.
73+
The build is a multi-module Gradle project (`arcp-core`, `arcp-client`,
74+
`arcp-runtime`, the `arcp` umbrella, transport adapters, middleware, the TCK,
75+
and example/recipe projects); `settings.gradle.kts` is the canonical list.
7876

79-
## Reporting bugs
77+
## Tests and conformance
78+
79+
Two layers must pass before a PR merges:
80+
81+
- **Unit tests** — this SDK's own suite:
82+
83+
```sh
84+
./gradlew build
85+
```
86+
87+
`build` compiles every module, runs Spotless, and executes the JUnit 5 +
88+
jqwik suites across all subprojects. Use `./gradlew test` (or
89+
`./gradlew :arcp-core:test`) to skip compilation of unrelated modules during
90+
iteration.
91+
92+
- **Conformance** — the SDK's behavior against the reference runtime. New
93+
protocol-facing code (session negotiation, event sequencing, lease handling,
94+
error mapping) needs a test that exercises the real exchange, not a mock that
95+
assumes the answer. The reusable conformance suite lives in `arcp-tck` as a
96+
JUnit 5 `@TestFactory` (`dev.arcp.tck.ConformanceSuite` + `TckProvider`);
97+
point it at any `Transport` pair — `MemoryTransport.pair()` for the in-process
98+
reference runtime, or a live WebSocket pair via `arcp-runtime-jetty` +
99+
`arcp-client`'s `WebSocketTransport.connect(uri)`. Run with
100+
`./gradlew :arcp-tck:test`.
101+
102+
CI runs both on every PR. A PR that changes which feature flags the SDK
103+
negotiates must also update the README feature matrix in the same change.
104+
105+
## Coding standards
106+
107+
Formatting is enforced by [Spotless](https://github.com/diffplug/spotless)
108+
configured with Google Java Format and unused-import removal. The same JDK 21
109+
`javac` settings (`-Xlint:all`, `-parameters`, UTF-8) and Javadoc generation
110+
that CI uses are applied to every `java-library` subproject:
111+
112+
```sh
113+
./gradlew spotlessApply # auto-format
114+
./gradlew spotlessCheck # verify (CI gate on JDK 21)
115+
./gradlew build # compile + lint warnings + tests
116+
./gradlew javadoc # Javadoc for the published modules
117+
```
118+
119+
Beyond formatting, the house style is captured in the existing code: records
120+
for value types, sealed hierarchies (`Message`, `EventBody`, `ArcpException`)
121+
for exhaustive dispatch, JSpecify `@Nullable` on anything that may return null,
122+
`AtomicReference` / `AtomicLong` / `ConcurrentHashMap` for shared mutable state,
123+
and a spec `§` citation in the Javadoc of public types that implement a
124+
protocol concept.
125+
126+
Match the surrounding code. Public API changes need doc comments and an entry in
127+
the changelog. Prefer clarity over cleverness in a library others build on.
128+
129+
## Commit and pull-request conventions
130+
131+
- Write focused commits with present-tense, imperative subjects
132+
(`add result_chunk reassembly`, not `added` / `adds`).
133+
- Reference the issue a PR closes (`Closes #123`).
134+
- Keep the PR description honest about scope and any spec sections touched.
135+
- Rebase on the default branch and ensure CI is green before requesting review.
136+
- Sign off your commits to certify the [Developer Certificate of Origin](https://developercertificate.org/):
137+
138+
```sh
139+
git commit -s -m "your message"
140+
```
141+
142+
## Releases
80143

81-
Open a GitHub issue. Include:
144+
Releases are cut by maintainers. The `release` GitHub Actions workflow is
145+
dispatched manually with a version input; it builds, signs with the project
146+
PGP key, publishes the aggregated set of `dev.arcp:*` artifacts to Maven
147+
Central via the `com.gradleup.nmcp` plugin, and pushes a `vX.Y.Z` tag.
148+
Detailed operator steps live in [RELEASING.md](RELEASING.md). The SDK is
149+
versioned with semantic versioning independently of the protocol version it
150+
speaks; a protocol version bump is noted in the changelog when the negotiated
151+
ARCP version changes.
82152

83-
- JDK version (`java --version`).
84-
- Whether the bug reproduces under `MemoryTransport` (the in-process
85-
pair), and if not, which transport.
86-
- A failing test or a minimal client + runtime snippet — the
87-
[`SmokeRoundTripTest`](arcp-client/src/test/java/dev/arcp/client/SmokeRoundTripTest.java)
88-
shape is a good template.
153+
## License
89154

90-
Security-sensitive reports follow [SECURITY.md](SECURITY.md) instead.
155+
By contributing, you agree that your contributions are licensed under the
156+
project's [Apache-2.0](LICENSE) license.

0 commit comments

Comments
 (0)