diff --git a/README.md b/README.md index b1ff59f..4d4015b 100644 --- a/README.md +++ b/README.md @@ -5,15 +5,18 @@ projects as a git submodule. The repository carries three things: -- `rules/` — the rules themselves, as plain markdown. Single source of truth. +- `rules/` — common rules and opt-in profiles, as plain markdown. Single source + of truth. - `hooks/` — scripts wired into agent lifecycle events (Claude Code and Codex). - `install.sh` / `check.sh` — wire the above into a consuming project, idempotently. ## What belongs here -Only rules that hold for the whole organization. Anything tied to one service — -its packages, its build quirks, its local conventions — stays in that service's -own `AGENTS.md` / `CLAUDE.md`, outside the synced block. +Top-level files in `rules/` hold rules that apply to the whole organization. +Rules shared by one family of services live in `rules/profiles/` and are selected +by the consuming project. Anything tied to one service — its build quirks and +local conventions — stays in that service's own `AGENTS.md` / `CLAUDE.md`, +outside the synced block. ## Adding to a project @@ -25,7 +28,7 @@ git submodule add .agent-rules `install.sh` is idempotent and touches only what it owns: - registers the Kotlin format hook in `.claude/settings.json` and `.codex/hooks.json` -- writes `@`-imports of `rules/*` into `CLAUDE.md` +- writes `@`-imports of the selected rule files into `CLAUDE.md` - syncs the rule text into `AGENTS.md` between `` and `` @@ -33,6 +36,32 @@ Everything outside those markers is yours and is never rewritten. Commit the resulting changes together with the submodule pointer. +## Rule profiles + +Without configuration, `install.sh` applies only the common rules. A consuming +project can commit `.agent-rules-profile` with one of these values: + +- `common` — common rules only; +- `openapi` — common rules and contract-first OpenAPI conventions; +- `adapter` — common rules and external-adapter conventions. + +For example: + +```text +openapi +``` + +The profile can be overridden for a single command. The same option is accepted +by `check.sh`: + +```bash +./.agent-rules/install.sh --profile openapi +./.agent-rules/check.sh --profile openapi +``` + +The command-line value takes precedence over `.agent-rules-profile`. Unknown or +empty profile values are rejected. + ## Updating ```bash @@ -45,8 +74,9 @@ change under a project without a commit in it. ## Keeping projects honest -`check.sh` is `install.sh --check`: it writes nothing and exits non-zero when a -project has drifted from the submodule it pins. Wire it into CI with +`check.sh` runs `install.sh --check` with the configured profile: it writes +nothing and exits non-zero when a project has drifted from the submodule it pins. +Wire it into CI with `ci/github-actions/agent-rules-drift.yml` — note the `submodules: true` on checkout, without it the check runs against an empty directory. diff --git a/check.sh b/check.sh index 11cf16b..72f769b 100755 --- a/check.sh +++ b/check.sh @@ -1,3 +1,3 @@ #!/usr/bin/env bash # CI entry point: fails when the project has drifted from the rules it pins. -exec "$(cd -- "$(dirname -- "$0")" && pwd)/install.sh" --check +exec "$(cd -- "$(dirname -- "$0")" && pwd)/install.sh" --check "$@" diff --git a/code-conventions.md b/code-conventions.md deleted file mode 100644 index 3af2592..0000000 --- a/code-conventions.md +++ /dev/null @@ -1,36 +0,0 @@ -## Structure and dependencies - -- Code is organized into the `config`, `config.properties`, `resource`, - `servlet`, `service`, `repository`, `repository.model`, `scheduler`, `client`, - `client.model`, `converter`, and `extensions` packages. -- Standalone classes and models are placed in separate files. - -## DTOs and converters - -- External API requests and responses are represented by typed DTOs, without - `Map`. -- JSON property names are specified with Jackson annotations, and closed sets of - values are represented by enums. -- Model conversion is performed by dedicated `@Component` classes implementing - Spring's `Converter`. -- Requests and responses are created by converters. -- Concrete converters are provided through constructor injection. - -## Kotlin style - -- Calls to regular functions and methods use positional arguments. -- Named arguments are allowed for constructors and annotations. -- Constants belonging to a single class are placed in its `private companion object`. -- Shared constants are placed in the appropriate `constants/*.kt` file. - -## Configuration - -- External integrations are replaced with mock or stub beans in tests. -- Settings are grouped into typed `@ConfigurationProperties`. - -## External clients - -- The client is responsible for transport, the converter for mapping, and the service - for the business scenario. - -Changes are verified with `mvn ktlint:check` and `mvn clean test`. diff --git a/database-conventions.md b/database-conventions.md deleted file mode 100644 index 69d3ee2..0000000 --- a/database-conventions.md +++ /dev/null @@ -1,24 +0,0 @@ -## Stack and migrations - -- Migrations are run by Flyway from `src/main/resources/db/migration`. -- `IF NOT EXISTS` is used for supported PostgreSQL objects. -- Primary keys, constraints, foreign keys, and indexes are defined explicitly and - given meaningful names. -- Flyway and ShedLock tables are excluded from jOOQ code generation. - -## jOOQ - -- Flyway runs before jOOQ code generation. -- Generated classes are created in `target/generated-sources/jooq`. -- Generated tables, records, and enums are used; generated code is not edited - manually. -- Repositories use `DSLContext`. - -## Time and secrets - -- `TIMESTAMP WITHOUT TIME ZONE` and `LocalDateTime` are used; values are - interpreted as UTC. -- Test credentials are allowed only for embedded PostgreSQL and Testcontainers. - -Database changes are verified with Flyway, jOOQ code generation, and repository -integration tests using a PostgreSQL Testcontainer. diff --git a/install.sh b/install.sh index a201810..c377f2f 100755 --- a/install.sh +++ b/install.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Wires the shared rules into the project that mounts this submodule. # -# ./.agent-rules/install.sh apply -# ./.agent-rules/install.sh --check report drift, write nothing, exit 1 on drift +# ./.agent-rules/install.sh apply common rules +# ./.agent-rules/install.sh --profile openapi apply a rule profile +# ./.agent-rules/install.sh --check [--profile ...] report drift, write nothing # # Everything here is idempotent and owns a bounded piece of each file: the hook # entries it registered, and the text between the agent-rules markers. Whatever @@ -18,15 +19,36 @@ HOOK_MARKER="format-kotlin.sh" CHECK_ONLY=0 DRIFT=0 +PROFILE_OVERRIDE="" -case "${1:-}" in - --check) CHECK_ONLY=1 ;; - "") ;; - *) - printf 'usage: %s [--check]\n' "$0" >&2 - exit 64 - ;; -esac +usage() { + printf 'usage: %s [--check] [--profile common|openapi|adapter]\n' "$0" >&2 +} + +while [ "$#" -gt 0 ]; do + case "$1" in + --check) + CHECK_ONLY=1 + shift + ;; + --profile) + case "${2:-}" in + ""|--*) + printf 'agent-rules: --profile requires a value\n' >&2 + usage + exit 64 + ;; + esac + PROFILE_OVERRIDE="$2" + shift 2 + ;; + *) + printf 'agent-rules: unknown argument: %s\n' "$1" >&2 + usage + exit 64 + ;; + esac +done command -v jq >/dev/null 2>&1 || { printf 'agent-rules: jq is required\n' >&2 @@ -47,6 +69,30 @@ case "$RULES_DIR" in ;; esac +PROFILE="common" +PROFILE_FILE="$PROJECT_ROOT/.agent-rules-profile" + +if [ -f "$PROFILE_FILE" ]; then + PROFILE="$(cat "$PROFILE_FILE")" +fi + +if [ -n "$PROFILE_OVERRIDE" ]; then + PROFILE="$PROFILE_OVERRIDE" +fi + +case "$PROFILE" in + common|openapi|adapter) ;; + "") + printf 'agent-rules: profile in %s is empty\n' "$PROFILE_FILE" >&2 + exit 64 + ;; + *) + printf 'agent-rules: unknown profile: %s\n' "$PROFILE" >&2 + printf 'agent-rules: expected common, openapi, or adapter\n' >&2 + exit 64 + ;; +esac + report() { if [ "$CHECK_ONLY" -eq 1 ]; then printf 'drift: %s\n' "$1" >&2 @@ -118,6 +164,15 @@ merge_hooks() { rule_files() { find "$RULES_DIR/rules" -maxdepth 1 -name '*.md' -not -name 'index.md' | sort + + case "$PROFILE" in + openapi) + printf '%s\n' "$RULES_DIR/rules/profiles/openapi.md" + ;; + adapter) + printf '%s\n' "$RULES_DIR/rules/profiles/adapter.md" + ;; + esac } # Path of a rule file relative to the project root, e.g. .agent-rules/rules/x.md diff --git a/rules/code-conventions.md b/rules/code-conventions.md new file mode 100644 index 0000000..ead56d3 --- /dev/null +++ b/rules/code-conventions.md @@ -0,0 +1,104 @@ +# Code conventions + +## Architecture and dependencies + +- Transport adapters handle protocol concerns only: they validate the transport + contract, delegate to a service, and translate failures into protocol errors. +- Services implement business scenarios, define the order of operations, and own + transaction boundaries. +- Complex changes to aggregate parts are delegated to focused handlers instead of + growing a single service class. +- Repositories encapsulate persistence and return database or domain models. They do + not build transport responses. +- External systems are hidden behind local client or service interfaces; generated + stubs and retry mechanics do not leak into business code. +- Spring dependencies are provided through constructor injection and stored in + `final`/`val` fields. Java components use Lombok's `@RequiredArgsConstructor` + instead of handwritten constructors when no custom initialization is required. +- Code is organized into the `config`, `config.properties`, `resource`, + `servlet`, `service`, `repository`, `repository.model`, `scheduler`, `client`, + `client.model`, `converter`, and `extensions` packages. +- Standalone classes and models are placed in separate files. + +## DTOs and converters + +- External API requests and responses are represented by typed DTOs, without + `Map`. +- Transport models are converted before reaching repositories. Simple entities may + use generated persistence models; aggregates use local domain models. +- JSON property names are specified with Jackson annotations, and closed sets of + values are represented by enums. +- Model conversion is performed by dedicated `@Component` classes implementing + Spring's `Converter`. +- Requests and responses are created by converters. +- Converters map data but do not write to the database or call external systems. +- Optional fields are set only when present. An omitted value and an explicitly + empty value remain distinct when the API contract distinguishes them. +- Unsupported conversion directions fail explicitly instead of returning `null`. +- Concrete converters are provided through constructor injection. +- Related data for collections is loaded in batches before conversion; converters + must not introduce N+1 calls. + +## Business operations + +- Writes that form one business operation run in one transaction. +- Collection equality ignores order when order has no business meaning. +- One operation timestamp is reused for the persisted changes. + +## REST-to-gRPC gateways + +- Generated REST interfaces define the transport contract. Controllers and resources + implement them, validate transport concerns, and delegate without duplicating the + contract or containing business orchestration. +- Orchestration services build typed gRPC requests, invoke generated clients, and use + dedicated converters for REST-to-Protobuf and Protobuf-to-REST mapping. +- A request or correlation identifier received at the public boundary is propagated to + every downstream request and included in logs and typed error responses. +- gRPC failures are mapped centrally to the API's declared error model. At minimum, + invalid input, unauthenticated, forbidden, not found, conflict, throttling, deadline, + downstream unavailability, and unexpected internal failures remain distinguishable. +- Transport failures never produce an untyped or accidentally empty error response. + +## Kotlin style + +- Calls to regular functions and methods use positional arguments. +- Named arguments are allowed for constructors and annotations. +- Constants belonging to a single class are placed in its `private companion object`. +- Shared constants are placed in the appropriate `constants/*.kt` file. + +## Configuration + +- External integrations are replaced with mock or stub beans in tests. +- Settings are grouped into typed `@ConfigurationProperties`. +- Retry policies, backoff, and asynchronous executors are configured centrally and + injected by name. + +## External clients + +- The client is responsible for transport, the converter for mapping, and the service + for the business scenario. +- A client owns its generated stub and applies the configured retry policy in one + place. +- Missing recipients or input for an optional side effect causes an early return + without an external call. +- Asynchronous entry points catch and log failures that cannot be returned to the + caller. + +## Errors and logging + +- Expected domain failures use specific exception types and are mapped to transport + statuses at the transport boundary. +- Logs use parameterized placeholders instead of string concatenation and include + available request and domain identifiers. +- Large payloads and user content are logged only at `DEBUG` or `TRACE`. +- Transport adapters log request boundaries; services and handlers log business + steps without duplicating the full payload. + +## Testing + +- Pure converters and external-client orchestration are covered by unit tests, + including optional values, empty collections, invalid input, retries, and early + returns. +- External calls use mocks or stubs and assert the generated request as well as the + returned result. +- Asynchronous tests wait for an observable event instead of using a fixed `sleep`. diff --git a/rules/database-conventions.md b/rules/database-conventions.md new file mode 100644 index 0000000..7de38e4 --- /dev/null +++ b/rules/database-conventions.md @@ -0,0 +1,98 @@ +# Database conventions + +## Stack and migrations + +- Migrations are run by Flyway from `src/main/resources/db/migration`. +- Every schema change is introduced by a new immutable migration; an applied + migration is never rewritten. +- Migration names follow `V__.sql` and describe one + complete schema or index change. +- `IF NOT EXISTS` is used for supported PostgreSQL objects. +- Primary keys, constraints, and indexes are defined explicitly and + given meaningful names. +- Storage invariants use database defaults and `NOT NULL` constraints and are also + represented consistently in converters and repositories. +- Flyway and ShedLock tables are excluded from jOOQ code generation. +- No foreign keys are used. + +## jOOQ + +- Flyway runs before jOOQ code generation. +- Generated classes are created in `target/generated-sources/jooq`. +- Generated tables, records, POJOs, and enums are used; generated code is not edited + manually. +- A schema change is traced through migration, generated model, input conversion, + write query, read model, output conversion, and tests. +- Repositories use `DSLContext` and keep jOOQ queries out of services and transport + adapters. +- Inserts populate the complete model and map it to a generated record. Updates set + only fields that the operation is allowed to change. +- Insert, update, and upsert operations set their audit timestamp in UTC. +- Query aliases match read-model property names when results are mapped with + `fetchInto`. +- Empty collections are handled before `IN` queries and collection writes. +- Upsert is used only with a defined business key. Conflict columns and the minimal + set of updated columns are listed explicitly. +- Related data for result collections is fetched in batches to avoid N+1 queries. +- Type-safe jOOQ DSL is preferred. PostgreSQL-specific plain SQL uses bind values or + `inline(...)`, never string concatenation of user input. + +## Data lifecycle + +- When designing, priority is given to soft-delete +- Replacing related records and updating the owning aggregate happen in one transaction. +- Absence from a single-row query is represented consistently. + +## State transitions + +- A state transition, its validation, and all resulting writes run in one transaction. +- Repeating the same transition with the same business data is idempotent. A transition + that conflicts with an existing final state fails with a specific domain error. +- Business keys are protected by explicit unique constraints. Upsert or conflict + handling complements domain validation when concurrent requests may race. +- Batch state changes verify that the number of affected rows matches the expected + number; a partial update fails the transaction. + +## Transactional event delivery + +- The domain write and insertion of its delivery event are committed in the same + database transaction. A failure rolls back both. +- Every event has a stable identifier, a deterministic sequence or ordering key, a + delivery status, an attempt count, and the time at which it became eligible. +- The event payload contains the immutable data required for delivery; a retry does not + rebuild a materially different event from current mutable state. +- Concurrent workers claim disjoint events with a short transaction, for example by + using a lease or `FOR UPDATE SKIP LOCKED`. A database row lock is not held while a + remote call is in flight. +- Delivery is idempotent by event identifier. A worker records success only after the + recipient accepts the event and safely retries an ambiguous outcome. +- Retries are bounded and use configured backoff and next-attempt time. Exhausted events + move to an explicit terminal or dead-letter state and remain observable. + +## Time and secrets + +- `TIMESTAMP WITHOUT TIME ZONE` and `LocalDateTime` are used; values are + interpreted as UTC. +- Current timestamps are created explicitly in UTC and shared across all writes in + one business operation. +- Test credentials are allowed only for embedded PostgreSQL and Testcontainers. + +## Integration testing + +- Migration, query, filter, search, and transactional changes are tested against a + real PostgreSQL instance provided by embedded PostgreSQL or Testcontainers. +- Tests clean only the data they own and do not depend on execution order. +- Repository tests assert persisted values, conflict/update behavior, and empty-result + boundaries, not only affected-row counts. +- CRUD scenarios verify create, read, update, logical deletion, and the values in both + the database and returned model. +- Filtering and search rules include positive, negative, and boundary cases; + pagination also covers page boundaries and continuation tokens. +- Stateful-operation tests cover idempotent replay, conflicting final states, + concurrent requests, affected-row mismatches, and full transactional rollback. +- Event-delivery tests cover atomic domain/event rollback, concurrent workers, ordered + delivery, duplicate replay, ambiguous responses, process restart, retry backoff, and + exhaustion of the attempt limit. + +Database changes are verified with Flyway, jOOQ code generation, and repository +integration tests using PostgreSQL. diff --git a/rules/index.md b/rules/index.md index 61785de..254ff87 100644 --- a/rules/index.md +++ b/rules/index.md @@ -1,7 +1,15 @@ # Shared engineering rules -Organization-wide rules. They apply to every repository that mounts this -submodule; anything specific to a single service belongs in that service. +Top-level rules apply to every repository that mounts this submodule. Profiles +extend them for a selected family of services; anything specific to a single +service belongs in that service. - [Code generation](code-generation.md) +- [Code conventions](code-conventions.md) +- [Database conventions](database-conventions.md) - [Protobuf](protobuf.md) + +## Profiles + +- [OpenAPI contract](profiles/openapi.md) +- [Adapter](profiles/adapter.md) diff --git a/rules/profiles/adapter.md b/rules/profiles/adapter.md new file mode 100644 index 0000000..e7ce7e7 --- /dev/null +++ b/rules/profiles/adapter.md @@ -0,0 +1,73 @@ +# Adapter conventions + +These rules extend the common rules for services that integrate with external +providers. + +## Architecture and flow + +- Transport entry points validate the transport contract and delegate to services. +- Services coordinate the integration scenario; step-specific behavior is placed in + focused handlers selected by an explicit state or operation type. +- State transitions and transport intents are built centrally instead of being + assembled independently by handlers. +- Provider request and response models, converters, constants, and error handling + stay behind the provider client boundary. +- Runtime configuration keys, provider method names, URL paths, statuses, and error + codes are declared centrally as constants or enums. +- Callback handlers dispatch by an explicit callback type and are idempotent. A + repeated callback must not overwrite completed state or repeat a side effect. + +## Configuration and clients + +- Application settings use typed `@ConfigurationProperties` with `@Validated` and + field constraints for required values. +- Per-operation runtime options are validated by a dedicated validator before a + converter or handler accesses them as non-null values. +- Provider calls use the application's configured `RestClient` and `ObjectMapper`. +- Base URLs, environment selection, request paths, and authorization headers are + resolved centrally. +- Provider requests and responses use typed DTOs. Internal configuration and helper + fields that are not part of the wire contract are excluded from serialization. +- An empty response body or a body that cannot be parsed is handled explicitly and + mapped to a stable integration error. +- HTTP status errors, provider errors, and response parsing errors are distinguished + before being mapped to domain failures. + +## Secrets and logging + +- Provider credentials and tokens are obtained through the configured secret service, + such as Vault. They are never hardcoded or included in logs. +- Kotlin files use a file-level `private val log = KotlinLogging.logger {}` and lazy + logging blocks. +- PANs, phone numbers, bank accounts, tokens, and other sensitive fields are masked + before logging or storing diagnostic metadata. +- External request, response, and callback payloads pass through the shared log + sanitizer before being logged. +- DTOs containing sensitive values provide a safe `toString()` or are never logged as + complete objects. + +## State and polling + +- Multi-step operation state is held in a dedicated context and serialized into the + transport's continuation state through one serializer. +- Missing continuation state creates a new context; malformed state fails explicitly. +- Serialized context changes are backward compatible with states produced by the + previous deployed version and are covered by compatibility tests. +- Polling metadata, including the deadline and next interval, is stored with the + operation state. +- Polling is bounded by a deadline. Pending and unknown non-final statuses schedule + the next attempt using the configured backoff instead of looping immediately. +- Final success, final failure, timeout, transport failure, and malformed provider + responses produce distinct, deterministic outcomes. + +## Testing + +- Provider HTTP integration tests use WireMock with the application context and real + client serialization. +- Every provider method covers success, provider failure, HTTP failure, empty body, + malformed body, and required-field validation where applicable. +- Stateful flows cover pending-to-success, pending-to-failure, polling timeout, and + callback replay. +- Tests assert outbound method, path, headers, and body as well as the mapped result. +- Shared flow fixtures and builders contain transport mechanics; test cases describe + scenario-specific mocks, actions, and assertions. diff --git a/rules/profiles/openapi.md b/rules/profiles/openapi.md new file mode 100644 index 0000000..a49a137 --- /dev/null +++ b/rules/profiles/openapi.md @@ -0,0 +1,17 @@ +# OpenAPI contract conventions + +These rules extend the common rules for repositories that own an OpenAPI contract and +publish generated server or client artifacts. + +## Contract structure + +- One root OpenAPI document is the source entry point. Paths and reusable components + are split into focused files and connected through local `$ref` references. +- Every operation has a stable, unique `operationId`, an appropriate tag, and explicit + request parameters, request body, responses, and security requirements. +- Common parameters, error responses, schemas, and security schemes are defined once + under `components` and reused instead of being copied between operations. +- Public operations require and document a request or correlation identifier and use a + shared typed error schema. +- Schema fields declare `required`, `nullable`, formats, enums, bounds, and collection + constraints explicitly whenever they are part of the contract. diff --git a/rules/protobuf.md b/rules/protobuf.md index c42e9ed..cf270e1 100644 --- a/rules/protobuf.md +++ b/rules/protobuf.md @@ -1,3 +1,4 @@ # Protobuf -Use versioned packages and directories. Keep wire-compatible field types, reserve removed fields, and add compatibility tests when a consumer may receive a newly added oneof variant or enum value. +Use versioned packages and directories. Keep wire-compatible field types, reserve removed fields, +and add compatibility tests when a consumer may receive a newly added oneof variant or enum value.