Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .claude/agents/reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ First read `CLAUDE.md` and every **accepted** ADR in `docs/adr/` that relates to

**ADR compliance**
- ADR-0002: no `catch (Exception)` or bare `catch`. Expected failures (HTTP status, payload, network, timeout) are returned as `OpenUrzednikResult` errors. Only caller cancellation, argument errors and fatal errors are thrown; in provider client methods, a `null` argument or an undefined enum value throws synchronously, before the task is returned (flag an `async` public client method that checks them, or a `ValidationError` for them), while a non-null value of the right type that fails a format or range check is a returned `ValidationError`. Timeouts are distinguished from caller cancellation. Errors implement `CreateException()` returning `OpenUrzednikException` subtypes; exceptions keep their error in `Error`/`Errors`. Several validation errors are thrown as one `OpenUrzednikValidationException` listing all of them, never `AggregateException`; several errors of other kinds as the first error's exception with all of them in `Errors`. Resilience-handler rejections are translated in DI packages, not caught in providers.
- ADR-0003: no `ILogger`/`ActivitySource` in Core or provider packages, except Core's internal `Telemetry/Activities` (the .NET fallback trace source, compiled on .NET only). Members of those types that other packages call through `InternalsVisibleTo` are a binary contract: flag a change or removal outside a major. Span and tag naming; HTTP tags use the OpenTelemetry names (`http.request.method`, `url.path`, `http.response.status_code`); errors set `error.type`. Activity sources are named `OpenUrzednik.<Provider>`. Every log call passes an event id; a new log event has a new id in `RestLogEvents` (HTTP, 1xxx) or the provider's `<Provider>LogEvents` (2xxx); flag a renumbered, reused or missing id, and a `RestLogEvents` entry not mirrored in `<Provider>LogEvents`. Flag any change to `IOpenUrzednikLogger`, `IOpenUrzednikTraceSource` or `IOpenUrzednikSpan` after 1.0 (frozen; new capabilities are separate interfaces), and any change to a stable name (source, span, documented tag, `error.type`, logger category, event id) outside a major. `IsEnabled` guards. No secrets or personal identifiers in logs or tags.
- ADR-0003: no `ILogger`/`ActivitySource` in Core or provider packages, except Core's internal `Telemetry/Activities` (the .NET fallback trace source, compiled on .NET only). Members of those types that other packages call through `InternalsVisibleTo` are a binary contract: flag a change or removal outside a major. Span and tag naming; HTTP tags use the OpenTelemetry names (`http.request.method`, `url.path`, `http.response.status_code`); errors set `error.type`. Activity sources are named `OpenUrzednik.<Provider>`. Every log call passes an event id; a new log event has a new id in `RestLogEvents` (HTTP, 1xxx) or the provider's `<Provider>LogEvents` (2xxx); flag a renumbered, reused or missing id, and a `RestLogEvents` entry not mirrored in `<Provider>LogEvents`. Flag any change to `IOpenUrzednikLogger`, `IOpenUrzednikTraceSource` or `IOpenUrzednikSpan` after 1.0 (frozen; new capabilities are separate interfaces), and any change to a stable name (source, span, documented tag, `error.type`, logger category, event id) outside a major. `IsEnabled` guards. No secrets (API keys, session ids, tokens) or personal identifiers (PESEL, NIP, REGON, bank account numbers) in logs, span names or tags, error and exception messages, error metadata or `Exception.Data`; at most the last 4 characters.
- ADR-0004: no new `PackageReference` in Core or provider projects, apart from netstandard2.0-only official Microsoft BCL packages (currently `System.Text.Json`, `System.Net.Http.Json`, `Microsoft.Bcl.TimeProvider`, `Microsoft.Bcl.HashCode`; any other one needs approval). `System.Diagnostics.DiagnosticSource` is allowed only in `OpenUrzednik.Diagnostics`. No `Version=` on package references (central package management). A dependency of a published library moves only by patch unless the PR says why a minor or major raise is needed; the DI package's `Microsoft.Extensions.*` keep the `net8.0` override (8.0.x), and no package gets two conditional `Include` rows. DI packages don't add a resilience handler themselves (the app chains `AddStandardResilienceHandler()`), and they translate its rejection exceptions into errors.
- ADR-0005: code builds for netstandard2.0 and passes `test (net472)`. Package validation stays on with no suppressions: flag a `CompatibilitySuppressions.xml`, a `CPxxxx` in `NoWarn`, or `EnablePackageValidation` turned off (the only exception is ADR-0011's CP0006 for a new client-interface method). `OpenUrzednik.snk`, `SignAssembly`, `PublicKey` and `assemblyVersion.precision` don't change; a project doesn't turn signing off; a third-party `InternalsVisibleTo` friend has its own `Key`. `#if` kept inside helpers (polyfills in `src/Polyfills`, `src/Http/Polyfills`, `Nbp/Polyfills`, `Extensions.Logging/Polyfills`). Every build has the same public signatures for what it shares: flag a public `DateOnly` (parameter, property or return type) and any interface member that exists only on .NET; public dates are `DateTime`, with `DateOnly` overloads only as .NET-only extension methods. Internally, dates are written as `DateOnly` (an alias for `DateTime` on netstandard2.0) and compared by `DayNumber`. A .NET target that isn't the lowest one may be removed in a minor version once Microsoft ends its support; the lowest .NET target and netstandard2.0 are removed only in a major. Targets are netstandard2.0 plus the .NET LTS releases in support.
- ADR-0006: provider HTTP goes through `RestRequestExecutor` (shared source in `src/Http`); providers never call `HttpClient.SendAsync`, and no public signature uses a type from `OpenUrzednik.Http.Infrastructure`. Request and response are disposed.
- ADR-0007: public types follow the namespace layout (clients, interfaces and options in the provider root namespace, models in area namespaces, no public `.Extensions`/`.Options` namespaces in Core or providers; `OpenUrzednik.Extensions.Logging` and internal namespaces are exempt); the default constructor stays simple; nothing that shapes requests (URL builders, path enums) is public or a public constructor parameter: customization goes through `NbpOptions` and `DelegatingHandler`s; `CancellationToken cancellationToken = default` comes last in both interface and implementation.
- ADR-0006: REST/JSON provider HTTP goes through `RestRequestExecutor` (shared source in `src/Http`); those providers never call `HttpClient.SendAsync` (a SOAP or other transport follows its provider ADR), and a request URI (path or query) carrying a personal identifier must not reach `url.path`, logs, error messages or `ErrorResponseContext.RequestPath` (display path; a 400's server message must be masked or dropped), and no public signature uses a type from `OpenUrzednik.Http.Infrastructure`. Request and response are disposed.
- ADR-0007: public types follow the namespace layout (clients, interfaces and options in the provider root namespace, models in area namespaces, no public `.Extensions`/`.Options` namespaces in Core or providers; `OpenUrzednik.Extensions.Logging` and internal namespaces are exempt); the default constructor stays simple; nothing that shapes requests (URL builders, path enums) is public or a public constructor parameter: customization goes through `NbpOptions` and `DelegatingHandler`s (auth that needs protocol calls, or credentials passed as API arguments, belongs in the provider, per its provider ADR); `CancellationToken cancellationToken = default` comes last in both interface and implementation.
- ADR-0008: all packages share the version of the root `version.json`; no project has a `version.json` of its own. The `paths` filter of `nuget-pre-release.yml` and `pathFilters` in `version.json` list the same package inputs.
- ADR-0009: docs are in English, and English wins over Polish. If a PR changes an informational part of a README (description, "unofficial" notice, installation, quick start), `README.pl.md` changes too. A `*.pl.md` file gets no contract or reference content (API limits, data availability, errors, telemetry, configuration rules, dependencies, versioning, support), and no `*.en.md` file comes back. Renaming or removing a section of an English README updates the links to it in `README.pl.md`.
- ADR-0010: no work on GUS/KRS/MF beyond what that ADR allows.
- ADR-0010: no work on GUS/KRS/MF beyond what that ADR allows; a provider ADR answers the ADR-0010 readiness checklist, and package and namespace names follow its casing rule.
- If a change contradicts an ADR and has no new ADR, report it as **blocking**.

**Performance** (CLAUDE.md "Performance")
Expand All @@ -36,7 +36,7 @@ First read `CLAUDE.md` and every **accepted** ADR in `docs/adr/` that relates to
- Dependencies of the published (`src/`) libraries: abstractions stay at the lowest supported version, implementations at the latest patch of the chosen minor (a minor or major raise needs a stated reason). Test and tooling packages always take the latest version. A change that raises an abstraction is **blocking** until the maintainer has explicitly approved it.

**Correctness**
- DTO `required` properties match real API responses. Hand-written WireMock bodies are suspect: suggest `/verify-api`.
- DTO `required` properties match real API responses. Hand-written WireMock bodies are suspect: suggest `/verify-api` (allowed only for cases the real API can't produce, such as an empty `[]` or malformed JSON). Flag committed fixtures that contain real personal identifiers, API keys, session ids or tokens.
- Validators match the documented API limits. Messages use invariant formatting.
- Disposal, cancellation-token propagation, `ConfigureAwait(false)` in library code.
- Public behaviour changes are covered by unit tests and WireMock tests, including every error path in ADR-0002's Confirmation.
Expand Down
2 changes: 1 addition & 1 deletion .claude/skills/new-endpoint/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Work through every step. Do not skip tests. If a step conflicts with an ADR, sto
- Use `Faker().WithConstantSeed()`, the DTO fakers, `StubHttpMessageHandler`, `FakeTimeProvider`.
- **Mapper** tests in `MapperTest.<MapMethod>.cs`; a **validator** test per new validator.
- **WireMock** (`tests/OpenUrzednik.IntegrationTests/<Provider>/WireMock/`):
- success with the captured real body
- success with the captured real body; if responses can contain personal data, capture from the provider's test environment or from legal-person records, replace remaining personal fields with fake values of the same format, and never commit API keys, session ids or tokens. Hand-written bodies are only for cases the real API can't produce (an empty answer `[]`, malformed JSON)
- every error path in the canonical list in [ADR-0002](../../../docs/adr/0002-result-pattern-and-error-handling.md)'s Confirmation
- Assert the request path and `Accept` header.
- **Real API** test with `[ManualFact]` (explicit: runs with `--explicit on` or when `OPEN_URZEDNIK_INTEGRATION_TEST_ENABLED` is set).
Expand Down
2 changes: 1 addition & 1 deletion .claude/skills/verify-api/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Goal: our DTOs and test payloads must match what the API really returns, so that
- A `required` property missing from any real response is a **bug**: deserialization will fail.
- Report extra fields we ignore, type mismatches (number vs string, date formats) and casing.
4. **Compare with the WireMock tests** in `tests/OpenUrzednik.IntegrationTests/<Provider>/WireMock/`. Flag every hand-written body that differs in shape from the real one.
5. **Capture fixtures** when asked, or when fixing a mismatch: save real bodies under `tests/OpenUrzednik.IntegrationTests/<Provider>/Fixtures/<endpoint-shape>.json` (trim long arrays to 2–3 items, keep the real structure) and load them in the WireMock tests instead of inline JSON.
5. **Capture fixtures** when asked, or when fixing a mismatch: save real bodies under `tests/OpenUrzednik.IntegrationTests/<Provider>/Fixtures/<endpoint-shape>.json` (trim long arrays to 2–3 items, keep the real structure) and load them in the WireMock tests instead of inline JSON. If responses can contain personal data, capture from the provider's test environment or from legal-person records, replace any remaining personal fields with fake values of the same format, and never commit API keys, session ids or tokens. Hand-written bodies are only for cases the real API can't produce (an empty answer `[]`, malformed JSON).
6. **Report** a table: endpoint | status | fields in response | DTO verdict (OK / missing required / extra / type mismatch) | test payload verdict. List the limits you observed (max `last/{n}`, max date range, earliest date) next to the values used in `Validation/`.

Don't change validators based on one observation without saying so: point out differences from the documented limits and let the maintainer decide.
28 changes: 28 additions & 0 deletions .github/ISSUE_TEMPLATE/provider_proposal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,31 @@ body:
description: Do czego potrzebujesz tego API? / What do you need it for?
validations:
required: true
- type: dropdown
id: quota
attributes:
label: Rodzaj limitu / Quota type
description: Opcjonalne. / Optional.
options: [ "Brak limitu / No limit", "Ograniczanie tempa / Throttling", "Limit dzienny lub na adres IP / Daily or per-IP quota", "Nie wiem / Unknown" ]
- type: dropdown
id: session
attributes:
label: Sesja lub logowanie / Stateful session or login
description: Opcjonalne. Czy trzeba się zalogować osobnym wywołaniem? / Optional. Is a separate login call needed?
options: [ "Tak / Yes", "Nie / No", "Nie wiem / Unknown" ]
- type: dropdown
id: personal-data
attributes:
label: Dane osobowe w odpowiedziach / Personal data in responses
description: Opcjonalne. / Optional.
options: [ "Tak / Yes", "Nie / No", "Nie wiem / Unknown" ]
- type: textarea
id: test-environment
attributes:
label: Środowisko testowe / Test environment
description: Opcjonalne. Adres lub opis. / Optional. URL or description.
- type: textarea
id: dependencies
attributes:
label: Wymagane zależności / Required dependencies
description: Opcjonalne. Np. biblioteka do archiwów, certyfikaty. / Optional. E.g. an archive library, certificates.
Loading
Loading