docs: architecture decision records

CLAUDE.md

@@ -42,7 +42,11 @@ on change. Use `task compose -- logs --tail 0 --follow node` to see compilation
 1. `task prepare-code` — normalize, apply standards, PHPStan, tests.
 2. `task composer -- normalize` if you touched `composer.json`.
 3. Add a bullet to `CHANGELOG.md` under `## [Unreleased]`, keyed by PR link. **CI fails a PR whose
-   `CHANGELOG.md` is identical to the base branch**, so this is not optional.
+   `CHANGELOG.md` is identical to the base branch**, so this is not optional. Keep it to 1–2 lines,
+   3 at the most: what changed and the consequence, nothing else. The long prose entries already in
+   the file are not the style to copy. No entries for tests added or coverage moved, and no
+   rationale — reasoning that is worth keeping belongs in an ADR under `docs/adr/`
+   (see `docs/adr/README.md`).
 4. If you touched Markdown or YAML, they have their own CI gates:
 
    ```shell

docs/adr/001-single-data-provider-abstraction.md

@@ -0,0 +1,84 @@
+# 001: One data provider abstraction, Leantime as the only implementation
+
+| Field | Value |
+| --- | --- |
+| **Created By** | Troels Ugilt Jensen |
+| **Date** | 2026-08-25 |
+| **Decision Maker** | ITK Dev Economics team |
+| **Stakeholders** | ITK Dev Economics team |
+| **Status** | Accepted |
+
+## Context
+
+Economics owns no source data. Projects, issues, worklogs, accounts and milestones all originate in
+an external project tracker, and everything the application does — invoices, project billing,
+reports — is derived from that synchronised copy. Which tracker that is has changed once already:
+the system was built against Jira and now runs against Leantime.
+
+At the point Leantime replaced Jira there was exactly one implementation left, which raised the
+question of whether the abstraction still pays for itself.
+
+### Drivers
+
+- **Functional:** synchronisation must be able to target a different tracker, and more than one
+  instance of the same tracker, without the invoicing and reporting code being aware of it.
+- **Functional:** the Jira era must remain migratable — existing installations carry Jira-shaped data.
+- **Non-functional:** a tracker swap should be a contained change, not a rewrite of the sync layer.
+- **Non-functional:** one indirection is acceptable; a plugin framework for a single provider is not.
+
+### Options Considered
+
+1. **Keep the interface with a single implementation.** `App\Interface\DataProviderInterface` stays,
+   `LeantimeApiService` implements it, and `DataProviderService::IMPLEMENTATIONS` is the registry.
+   Costs one level of indirection that no second implementation currently justifies; buys a seam that
+   the Jira removal has already been pushed through once.
+2. **Collapse the abstraction and call Leantime directly.** Removes the indirection and the interface
+   drift described below. Makes the sync layer, the commands and the admin UI all name Leantime
+   explicitly, so the next tracker change touches every one of them.
+3. **Keep Jira alive alongside Leantime.** Preserves a real second implementation, and with it the
+   proof that the abstraction works. Means maintaining an integration with a tracker no installation
+   uses any more, including its custom-field configuration.
+
+## Decision
+
+Option 1. `DataProviderInterface` is retained, `LeantimeApiService` is its only implementation, and
+`DataProviderService::IMPLEMENTATIONS` (`src/Service/DataProviderService.php`) is the registry — a
+one-entry array today.
+
+The abstraction is kept on evidence rather than on principle: removing Jira was an edit to that
+registry plus the deletion of one service, not a change to any consumer of the synchronised data.
+The `DataProvider` entity is what makes the seam load-bearing even with one implementation — several
+rows can point at different Leantime instances, each enabled or disabled independently, which is a
+multiple-provider case in production regardless of how many classes implement the interface.
+
+Jira is retained only as a migration path: `src/Command/MigrateFromJiraEconomicsCommand.php` and
+`docs/migration-from-jira-economics.md`. It is not a data provider and does not appear in the
+registry.
+
+## Consequences
+
+### Positive
+
+- Invoicing, billing and the report suite consume synchronised entities and never name a tracker.
+- Adding or retiring a provider is a registry edit plus one service.
+- Several tracker instances can be synchronised at once, and enabled or disabled per row, without a
+  deploy — see [002](002-data-provider-credentials-in-database.md).
+
+### Negative / Trade-offs
+
+- An interface with one implementation is an abstraction the compiler cannot check against a second
+  shape. It may have drifted towards Leantime's model in ways only a real second provider would
+  reveal.
+- **Known interface drift, deliberately left alone.** `DataProviderInterface` declares only
+  `updateAll()` and `update()`, and neither declaration carries the `$disableModifiedAtCheck`
+  argument that `LeantimeApiService` and `SyncCommand` both use. `deleteAll()` is not declared at
+  all. The call sites are correct and the declaration is what lags. Do not "fix" this by dropping
+  arguments at the call sites — that removes working behaviour to satisfy a stale signature.
+- Reading the sync layer means following one more hop than a direct client would.
+
+### Follow-up Actions
+
+- [ ] Widen `DataProviderInterface` to match the implementation: add `$disableModifiedAtCheck` to
+      `update()` and `updateAll()`, and declare `deleteAll()`.
+- [ ] Reassess this ADR if a second tracker is ever added — that is the point at which the abstraction
+      is tested rather than assumed.

docs/adr/002-data-provider-credentials-in-database.md

@@ -0,0 +1,111 @@
+# 002: Data provider credentials live in the database
+
+| Field | Value |
+| --- | --- |
+| **Created By** | Troels Ugilt Jensen |
+| **Date** | 2026-08-25 |
+| **Decision Maker** | ITK Dev Economics team |
+| **Stakeholders** | ITK Dev Economics team |
+| **Status** | Accepted |
+
+## Context
+
+A data provider needs a base URL and an API token. The Symfony default is to put both in the
+environment and to configure a scoped HTTP client against them, so that every request to that host
+carries the right headers automatically.
+
+That does not work here, because a scoped client keys on `base_uri`, and Economics does not know its
+base URIs at container-build time: they are rows in a table. The consequence shows up as a hand-built
+client in `config/services.yaml`, which reads like an oversight unless the reason is written down.
+
+### Drivers
+
+- **Functional:** more than one provider must be configurable, and each enabled or disabled
+  independently, by an administrator rather than by a deploy.
+- **Functional:** a token has to be replaceable when it is rotated at the tracker, without a release.
+- **Non-functional:** one request must never hold a Messenger worker indefinitely.
+- **Non-functional:** credentials should stay out of the codebase and out of version control.
+
+### Options Considered
+
+1. **Environment variables plus a scoped HTTP client.** The framework default. Gives per-host
+   defaults, headers and retry configuration for free, and puts the token in the usual place for a
+   secret. Fixes the set of providers at deploy time, and makes a second Leantime instance a
+   configuration release.
+2. **Database rows plus one hand-built client.** A `DataProvider` row holds the URL and token; a
+   single client is defined with timeout options only, and the URL and token are applied per request
+   from the row. Providers become runtime data. Loses the scoped-client conveniences.
+3. **Database rows plus a scoped client built per provider at runtime.** Keeps the scoped-client
+   behaviour by constructing one client per row on boot. Means a service graph that depends on the
+   database being readable before the container is usable, and a cache invalidation problem whenever a
+   row changes.
+
+## Decision
+
+Option 2. Credentials live in the `DataProvider` entity, and providers are created and managed
+through the console — `app:data-provider:create`, `app:data-provider:list`,
+`app:data-provider:set-enable` — or through the admin interface.
+
+Because the base URI is only known at runtime, `app.leantime.http_client`
+(`config/services.yaml`) is hand-built from `@http_client` with `withOptions`, carrying nothing but
+the two timeout bounds, and is injected into `LeantimeApiService` as `$httpClient`:
+
+```yaml
+app.leantime.http_client:
+    class: Symfony\Contracts\HttpClient\HttpClientInterface
+    factory: ["@http_client", withOptions]
+    arguments:
+        - { timeout: 5, max_duration: 30 }
+```
+
+Both numbers are deliberate. `timeout: 5` is the idle gap between response chunks, not the whole
+request, which is generous for an API that answers in milliseconds. `max_duration: 30` bounds one
+page; Symfony caps neither by default, and an uncapped request holds a worker forever, because
+`messenger:consume` only checks its `--time-limit` between messages and never during one.
+
+**If a page ever needs longer than 30s, lower `LeantimeApiService::LIMIT` instead of raising the
+cap.** The cap is what guarantees a worker comes back; the page size is the adjustable part. See
+[003](003-messenger-paged-synchronization.md).
+
+### The stored URL is normalized where it is read
+
+Because the URL is data rather than configuration, nothing validates its shape on the way in, and a
+provider stored as `https://leantime.example.com/` is as legitimate a row as one stored without the
+trailing slash. `LeantimeApiService::API_PATH_DATA` carries its own leading slash, so that row was
+asked for `//APIData/API/projects`, and the deep links written onto issues and projects came out as
+`//errorpage/…` and `//projects/showProject/…`.
+
+The slash is stripped where the URL is **read**, not where it is written. `LeantimeUrlGenerator::baseUrl()`
+— already used by the `leantime_url` Twig function and by `ProjectRepository` — is injected into
+`LeantimeApiService` and applied at each of the three places it concatenates the URL.
+
+Normalizing on read is what covers the rows already stored with a trailing slash. A setter or a form
+constraint would only cover rows written after it, because Doctrine hydrates properties directly and
+never calls the setter when loading an entity.
+
+## Consequences
+
+### Positive
+
+- Providers are runtime data: an administrator can add a tracker instance, rotate a token, or disable
+  a provider without a deploy.
+- Several providers are synchronised in the same run, each with its own credentials — the sync methods
+  iterate `getEnabledLeantimeDataProviders()`.
+- No credential is in the repository or in a compiled container.
+- The timeout pair gives a hard bound on how long one message can occupy a worker.
+
+### Negative / Trade-offs
+
+- Tokens sit in application table rows, outside whatever secret management the platform offers, and
+  are readable by anyone with database access or the admin role.
+- The client forgoes the scoped-client conveniences: per-host default headers, `base_uri` resolution
+  and framework-level retry configuration must all be handled in `LeantimeApiService` instead.
+- The definition looks unmotivated in `config/services.yaml` without the comment that sits above it —
+  which is why this ADR exists.
+- URL normalization is applied per call site rather than once at the boundary, so a fourth place that
+  concatenates a provider URL can silently forget it and reintroduce the double slash.
+
+### Follow-up Actions
+
+- [ ] Consider encrypting the token column at rest, or moving it behind a secrets backend while
+      keeping the URL in the row.

docs/adr/003-messenger-paged-synchronization.md

@@ -0,0 +1,124 @@
+# 003: Synchronisation is Messenger-paged, and the transport choice is semantic
+
+| Field | Value |
+| --- | --- |
+| **Created By** | Troels Ugilt Jensen |
+| **Date** | 2026-08-25 |
+| **Decision Maker** | ITK Dev Economics team |
+| **Stakeholders** | ITK Dev Economics team |
+| **Status** | Accepted |
+
+## Context
+
+A full synchronisation reads every project, milestone, ticket and timesheet the tracker holds for the
+included projects. That is far more than one HTTP request can carry and far more than one PHP process
+should attempt: a single long request has no progress, no partial success, and no way to resume after
+a failure halfway through.
+
+The tracker's API answers a page at a time. The question is how those pages are driven, and — less
+obviously — what the choice between the `sync` and `async` Messenger transports actually means, since
+in this codebase it is not a performance knob but an ordering guarantee.
+
+### Drivers
+
+- **Functional:** a full sync must complete across many requests without any single request being
+  long-running.
+- **Functional:** deletions must be applied children-before-parents, or a parent removal orphans rows
+  that are still being read.
+- **Functional:** a failed page must be retried without re-reading the pages that already succeeded.
+- **Non-functional:** sync runs hourly by cron, so the whole run must finish comfortably inside an hour.
+- **Non-functional:** one message must never hold a worker indefinitely — see
+  [002](002-data-provider-credentials-in-database.md).
+
+### Options Considered
+
+1. **One request per entity type.** Simplest to read. Needs an unbounded timeout, gives no partial
+   progress, and loses the entire type's work on any failure.
+2. **Paged, with all pages queued up front.** Maximum parallelism. Requires knowing the total count
+   before starting, and queues work for pages that a failure means should never run. Pages of the same
+   type interleave, so ordering is gone.
+3. **Paged, with each page dispatching its successor only once it has succeeded.** A self-propagating
+   chain: the handler does its work and, at the end, queues the next page. A failure stops the chain at
+   the failed page rather than skipping past it.
+
+## Decision
+
+Option 3, with the transport chosen per dispatch, not per application.
+
+`LeantimeApiService::LIMIT = 100`. Each `LeantimeUpdateMessage` / `LeantimeDeleteMessage` handler
+reads its page, and only at the end of the handler — after the page's own work is done — dispatches
+the successor, when `resultsCount === $limit` indicates a page was full. The cursor is the highest id
+seen on the page, not an offset, so pages cannot be skipped or repeated as rows shift underneath.
+A full page whose cursor cannot advance stops the chain and logs, rather than re-reading the same page
+until the queue starves.
+
+Every dispatch carries an explicit `TransportNamesStamp`, selecting `async` or `sync` from the
+`$asyncJobQueue` flag threaded through the whole call chain.
+
+### The transport is an ordering guarantee, not a speed setting
+
+`delete()` fans all four `DELETED_TYPES` out up front, in the order timesheets → tickets →
+milestones → projects. That fan-out is only safe because `deleteAll()` passes `$asyncJobQueue` as
+`false`: on the `sync` transport every handler runs inline, so a type's every page — removals and
+next-page dispatch alike — completes before the next type is dispatched at all. Children are gone
+before their parents are touched.
+
+**`deleteAll()` must therefore stay on `sync`.** On the `async` queue the four types would interleave,
+and a project could be reached while its timesheets were still a page behind. That path is not a
+matter of being slower; it is wrong.
+
+### A bad row is skipped and logged; a bad page still stops the chain
+
+The rule above is about pages. There is a second, opposite rule about the rows inside one, and the two
+are easy to confuse: a page that cannot advance stops the chain, but a single row that cannot be
+mapped does not. It logs `Skipping <class> id <id>: <reason>` and the sync moves on. One malformed row
+out of a hundred thousand should not cost the run.
+
+The catches that allow this are deliberately narrow. A `TypeError` from a null field, and a handler's
+`UnrecoverableMessageHandlingException`, are skippable. Nothing catches `\Throwable`, so a dead
+database or an unreachable tracker still halts the run loudly rather than being logged away as a bad
+row — the failure reaches the retry ladder in [004](004-layered-retry-policy.md) instead.
+
+The mappers are null-safe to match, and the placeholders they substitute are chosen rather than
+incidental:
+
+- a deleted user is attributed to `deleted-user-<userId>`;
+- a missing name becomes `(no name) <id>` (`LeantimeApiService::NAME_MISSING`);
+- a row with no `ticketId`, `projectId` or `id` is skipped, since there is nothing to key it on.
+
+**The tracker id is part of the placeholder because names are used as lookup keys elsewhere** —
+`ProjectBillingService` resolves a client by version name — so a bare `(no name)` would collide two
+unrelated rows into one. For the same reason a `deleted-user-<userId>` attribution never overwrites a
+worker name an earlier sync already stored: the placeholder is a fallback for missing data, not a
+correction of good data.
+
+## Consequences
+
+### Positive
+
+- No request is long-running, and no page's failure discards a page that already succeeded.
+- Progress is durable: a run interrupted mid-chain has committed everything up to the failed page.
+- Deletion ordering is guaranteed by the transport rather than by hope, and the guarantee is one line
+  (`$asyncJobQueue` false) rather than a scheduler.
+- Page size is the tuning knob for a slow tracker, which keeps the HTTP timeout cap intact.
+
+### Negative / Trade-offs
+
+- A type's pages are strictly sequential, so a full sync is latency-bound on page count × round trip
+  rather than parallelised. This is why the retry spacing cannot be widened without bound — see
+  [004](004-layered-retry-policy.md).
+- The chain is invisible in the code: nothing in `delete()` shows that the four dispatches are
+  ordered. The guarantee lives in the transport argument, which is why the reasoning is commented at
+  the dispatch site as well as recorded here.
+- A page that fails past its retry budget silently ends that type's sync for the run; the next hourly
+  run is what heals it.
+- A skipped row is visible only in the log. Nothing counts skips, and a run that dropped rows still
+  reports success.
+- A persistent mapping fault — a field the tracker started sending differently — drops the same rows
+  on every run without ever failing, so it is discoverable only by reading the log or noticing the
+  missing data downstream.
+
+### Follow-up Actions
+
+- [ ] If `deleteAll()` ever needs the `async` queue, the four types must be **chained** — each
+      dispatched only once the previous is exhausted — not fanned out. Do not simply flip the flag.