feat(core,currency,orders): order-time FX rate snapshot + reporting-currency stamping

[norbert-kulus-blockydevs]

Implements ADR-040 (merged in #2050) end to end: every order is stamped at ingestion with the amount it represents in a system-wide reporting currency, plus an immutable reference to the rate used.

Executes docs/plans/implementation-plan-2049-order-fx-rate-snapshot.md, phases 0-5.

Sub-issues

Each lands on its own branch, merged into this epic branch with --no-ff, and is reviewed before it merges.

• [TASK] Shared — previousWorkingDay helper in @openlinker/shared/date #2122 - Phase 0: previousWorkingDay in @openlinker/shared/date
• [TASK] CORE + Integration — currency context, rate-provider port and the @openlinker/integrations-fx adapters #2123 - Phase 1: libs/core/src/currency/ + the new @openlinker/integrations-fx package
• [TASK] CORE — order_records FX columns, stamp-once repository methods and the migration #2124 - Phase 2: six order_records columns, stamp-once repository methods, the migration
• [TASK] CORE + Infrastructure — FX stamp on ingestion, retry job and hourly reconcile sweep #2125 - Phase 3: OrderFxStampService, marketplace.order.fxStamp, marketplace.order.fxStampSweep
• [TASK] Full-Stack — currency-settings API, reporting-currency tile and env passthrough #2126 - Phase 4: /currency-settings API, the Platform/Currency settings tile, OL_REPORTING_CURRENCY
• [TASK] DX — Currency bounded-context section in architecture-overview #2127 - Phase 5: docs/architecture-overview.md § Currency

UI reference

docs/plans/mockups/order-fx-stamping-2049.html (added in this PR) covers every surface the stamp touches, built against the real design system, and records the four design decisions taken outside ADR-040.

Warning while this branch is in flight

libs/core/src/currency/ ships two @Entity classes - exchange_rates and reporting_currency_setting - before their migration, which lands with #2124. They materialise only via synchronize until then, which is fine for dev and the integration harness.

The consequence to watch: apps/api/src/database/data-source.ts discovers entities by filesystem glob, so anyone running migration:generate on this branch for an unrelated change will silently fold both tables into their migration. If you need a migration here before #2124 merges, check the generated DDL and strip anything currency-related.

Status

Draft. Un-drafts only after: every sub-issue merged and reviewed to approval, then a live end-to-end run with Playwright screenshots posted as a comment here.

Closes #2049
Closes #2122
Closes #2123
Closes #2124
Closes #2125
Closes #2126
Closes #2127

[norbert-kulus-blockydevs]

Live E2E verification done

Booted the full epic branch (all six phases merged, plus the one final integration /pr-review pass) on a real stack — fresh Docker images, real Postgres, real PrestaShop, 81 real demo orders.

Report with screenshots: https://claude.ai/code/artifact/653b0213-91c2-4910-a909-e120051bf0dd

Three deploy-only bugs found and fixed — none catchable by lint, type-check, or code review

1. Dockerfile never learned @openlinker/integrations-fx exists. The image build hand-enumerates every workspace package for layer-caching COPY; the new FX package was missing from all three lists across all six phases. docker build failed outright.
2. OL_REPORTING_CURRENCY was set on the demo compose's api service but not worker. Order ingestion and the retry/sweep jobs all run in the worker process, and the settings resolution's env fallback is per-process — a fresh deployment would have silently stamped in EUR (the code default) instead of the intended PLN. Caught by the final review pass; confirmed live.
3. The migration's own unit spec crashed migration:run. TypeORM's CLI globs and require()s every file under migrations/ directly; a colocated __tests__/*.spec.ts hit a bare describe() with no Jest runtime present. Every migration attempt exited 1. Only a real boot could ever have found this — nothing in CI or the Testcontainers harness runs a migration.

Live proof

One real PrestaShop demo order (19.99 EUR) was converted through a genuine NBP API call and stamped 86.80 PLN — hand-verified: 19.99 × 4.342 = 86.79658 → 86.80. Rate: NBP table 137/A/NBP/2026, direct derivation. The exchange_rates registry deduplicated correctly live: 8 orders stamped, only 4 distinct rate rows.

Status

PR #2135 stays draft pending your own final check, per plan.

[piotrswierzy]

Tech Lead review — 🔄 Approve with changes

Unusually strong work. The highest-risk parts of this feature — quote direction, snapshot immutability, missing-rate handling — aren't merely correct, they're defended: the direction invariant is stated in the type, encoded structurally as from/to rather than by naming convention, and pinned by tests that assert an exact real-world magnitude together with the derivation kind. No BLOCKING defect. One IMPORTANT issue — a transient 429 classified as terminal, combined with a terminal marker that has no operator recovery path — is cheap to fix and I'd fix it before merge.

Scope note: no ADR file is in this diff. ADR-040 already landed on main separately; heading matches filename, no collision with 041–045.

The settled design decisions

#	Decision	Status	Evidence
1	System-level, no per-connection override	✅	Singleton reporting_currency_setting; no Connection.config read anywhere in libs/core/src/currency/
2	Chain is exactly row → env → 'EUR'	✅	reporting-currency-settings.service.ts:71-74, :88-93. No history-derived rung exists
3	The default converts; no post-first-stamp lock	✅	No lock guard in setReportingCurrency — validates shape + coverage only
4	Both providers in libs/integrations/fx/; no HTTP in core	✅	git grep -nE "fetch\(|axios|HttpService|undici" over libs/core/src/{currency,orders} → zero hits. libs/integrations/fx added to check-outbound-http.mjs:58
5	Invoicing computes its own rate	✅	No invoicing file touched at all

Quote direction

Direction is a contract, not a convention: rate is to units per one from unit (exchange-rate.types.ts:6-11, :88-102), so a consumer always multiplies — and the single conversion site does (order-fx-stamp.service.ts:168).

The part I most wanted to check is the pivot: the numerator/denominator order is deliberately opposite between the two adapters, because NBP and ECB publish mirror-image quote directions (nbp:183-191 vs ecb:245, documented at rate-arithmetic.ts:61-67). That's exactly where an inversion would hide, and it's right.

Tests would catch an inversion — exact value + kind + leg pair asserted together, at magnitudes where an inversion is unmistakable (4.25000000; 0.23529412 with kind:'inverted'; pivot 1.08974359 with the flipped-divide tell 0.917… named in the comment). Plus an explicit negative test: "should never divide — the rate is applied as a multiplier, not an inverse". That's the standard I'd want on every future FX consumer.

Snapshot immutability

The strongest instance of the toOrm rule so far — fifth application after #1984 / #2107 / #2141 / #2129. All six FX columns are excluded from the ingestion write set with the reason stated at the assignment block and in the upsert doc. The sole writers are two guarded single-statement conditional updates (claimFxIntentIfAbsent, stampFxIfAbsent), both answering via affected > 0. All five stamp columns move in one statement so the group can't half-apply, with the DB ck_order_records_fx_group CHECK as backstop, and applyStamp treats a false return as a normal outcome and adopts the winner rather than retrying.

Worth calling out: the intent-pinning design makes the currency immune to a setting change between the inline attempt and a sweep hours later. That's the subtle failure this feature would otherwise have shipped with, and it's pinned by two tests.

IMPORTANT

1. A 429 or 408 is classified TERMINAL, and terminal is permanent with no recovery path. nbp-exchange-rate.adapter.ts:291-305 throws RateUnsupportedPairError for any non-404 4xx; order-fx-stamp.service.ts:327-328 maps that to terminal; recordTerminal writes fxStampedAt; and findUnstampedFxOrderIds filters on fxStampedAt: IsNull() — so the row leaves the sweep frontier forever. There's no re-open (git grep "clearFx\|resetFx\|unmarkFx" → nothing).

NBP is a public unauthenticated API and 429 is exactly what it returns under a burst, which the sweep's sequential page walk can reach. Net effect: a transient throttle permanently costs those orders their reported figure, silently. Add 429 and 408 to the >= 500 transient arm in both adapters. The comment at :291-296 is honest that the 4xx class was never verified against the live API — that uncertainty is itself the argument for excluding the two codes that are unambiguously transient. Separately, consider an admin re-open, or letting the sweep revisit a terminal row older than N days, since no-rate-source has the same permanence.

2. No User-Agent on outbound FX requests. fx-http.client.ts:68-72 sends only accept. Public central-bank APIs commonly throttle or filter anonymous UA-less clients, and there's no local backoff to absorb the result — which is what makes finding 1 more than theoretical.

SUGGESTION

3. ecb-exchange-rate.adapter.ts has no logger at all while NBP debug-logs its walk-back — an ECB failure is currently silent at the adapter layer.
4. Migration header :17 says the tail on main was 1833000000005; it's 1833000000006. The claimed slot is still strictly greater so the invariant holds — but the note is what the next author will trust. Slot 1834000000000 is free and no longer contested, since the fiscalization stack moved to 1835000000000.
5. NBP_MAX_WALK_BACK_DAYS = 7 with a <= loop yields 8 attempts; the spec asserts <= 8, so intent is pinned but the constant name understates by one.
6. ECB accepts an observation stale by up to 10 days without a log line. Correct as a backstop, but a 6-day-stale rate stores silently; a warn above ~3 days would surface a degrading source.
7. enqueueRetry's key is fx:{internalOrderId} with no wave component — the shape #2039 had to fix for refreshSnapshot. Defensible here since the sweep is an unconditional backstop, but if a first retry dead-letters the key is spent and the sweep is the only remaining route. Worth saying that's intended.
8. A11y — the two reactive warnings in the edit dialog (currency-settings-dialog.tsx:114 era-split, :150 coverage-gap) appear on <select> change but sit in no live region, so a screen-reader user isn't told a warning materialised — and the second gates Save via a required acknowledgement. Add role="status" to at least the coverage-gap Alert.
9. No boot-time assertion that the provider registry is non-empty. This reinforces finding 1: if a host ever omits FxIntegrationModule, two things degrade silently and in opposite directions — every stamp classifies terminal and permanently marks the row answered, while listSelectableCurrencies returns [] so every settings PUT 422s. Both plugin lists carry the module today, so this is fragility rather than a live defect — but the assertion converts silent unrecoverable data loss into a loud startup failure.

Positive observations

• The registry is append-only by construction, not convention: insertIfAbsent builds an entity with no id, so save() can only INSERT. PG 23505 matched on the code, never the message.
• rate stays a string end-to-end into numeric(18,8) and is deliberately not Number()-ed in toDomain, against the repo's usual money convention, with the reason given. The stored artefact is the rate, so amounts stay recomputable rather than lossily pre-converted. Rounding happens once. And declining to reuse pricing-rule.types.ts's round2dp because its Math.max(0, …) clamp would turn a refund into 0.00 is exactly the reasoning that prevents a silent financial defect.
• toRateString throws RangeError on any non-positive or non-finite value, so the silent-0-or-1 fallback failure mode is structurally unreachable.
• resolveRateDate returning null rather than throwing for a missing placedAt is concretely motivated (WooCommerce orders arrive without it — cf. #2114), and the clamp-to-today is load-bearing against ECB answering a future endPeriod with a stale rate at HTTP 200.
• Keeping the Polish working-day calendar out of core and inside the NBP adapter, verified against the live API with a worked counter-example (Corpus Christi 2026-06-04), is the right layering backed by the right evidence.
• ADR-007 mapping is exhaustively switched with deferred correctly re-thrown as retryable rather than reported 'ok' — a new outcome kind fails type-check rather than falling through.
• The env rung ignores malformed and well-formed-but-unsupported values with a once-latched warning and never throws, which matters because resolve() runs on every stamp.
• check-cross-context-imports clean (1996 imports, 2555 files) — the new currency context is a true leaf with no orders back-edge. Controller @Roles('admin') on both verbs, both Cache-Control: no-store, and write-guard-coverage.spec.ts updated.
• Nothing in the FE renders a converted figure yet, so the "converted without provenance" risk doesn't arise — and the settings tile already distinguishes default / env / setting rather than collapsing them, and states that invoices compute their own rate.

CI: Lint, Type Check, Build, Docker smoke, PHP, Scaffolded Adapter Builds green on 242c1613. Test and Integration Tests were still in_progress — confirm both before merge. (The earlier run's cancelled jobs are the force-push superseding an in-flight run, not failures.)

Merge readiness: 🔄 ready pending findings 1 and 2 — two lines in each adapter plus a UA header — and green Test / Integration Tests. Everything the review set out to verify hard checks out.

[norbert-kulus-blockydevs]

Review addressed — all 9 findings, in e2a61ed

Both IMPORTANT and all seven SUGGESTIONs are implemented. Two went further than the literal ask, and one turned out to be already satisfied in the shipped code but genuinely broken for a different reason — both are called out below rather than folded in silently.

---

IMPORTANT 1 — a 429/408 was terminal, and terminal was permanent

Fixed in two layers, because the finding really describes two defects.

The classification. isTransientFxStatus(status) now lives in fx-http.client.ts and is status >= 500 || [408, 429].includes(status). Both adapters call it in place of their >= 500 arm, ordered before their >= 400 terminal arm, so the walk-back 404 and the "everything else is our bug" branch are untouched. Kept in the client rather than duplicated per adapter because both providers reach the same conclusion from these codes, and the cost of the two arms is asymmetric: a wrongly-transient status costs a retry, a wrongly-terminal one costs the order its figure forever. The >= 400 comment now says the two codes are excluded because the class was never verified against the live API — which is your argument, and it belongs next to the code that relies on it.

The permanence. You flagged that no-rate-source has the same shape and suggested an admin re-open or a sweep revisit. I took the sweep revisit, because it needs no new surface and it recovers the case where nobody is watching:

• findUnstampedFxOrderIds is now two OR'd arms — fxStampedAt IS NULL (unchanged) and fxStampedAt < terminalRetryBefore. reportingCurrency IS NULL sits in both, so a row that carries a figure is never re-entered whatever its marker says. That predicate, not the timestamp, is what makes a stamp immutable.
• markFxTerminalIfAbsent → markFxTerminal, guarded on reportingCurrency IS NULL alone. Dropping the fxStampedAt IS NULL predicate is required by the above: without it a re-answer writes nothing, the stale instant stays, and the row is re-tried on every tick instead of once per cooldown. The rename is deliberate — "if absent" would now be false about the marker.
• The cooldown is terminalRetryDays, default 7, payload-overridable per scheduler descriptor and clamped at 365. Shorter and a genuinely unstampable order burns provider calls; longer and a cleared condition outlives an operator's reaction time.

The reason a terminal answer had to become recoverable at all is the sentence worth keeping: it is terminal about the classification, not about the world. no-rate-source clears the moment a host is rewired; a throttle-induced unsupported-pair clears by itself.

IMPORTANT 2 — no User-Agent

FX_USER_AGENT (OpenLinker/1.0 (+repo URL; exchange-rate reader)) is sent on every fxGet alongside accept. Worth noting undici sends no default UA, so this is the only one either provider ever saw — and, as you said, there is no local backoff to absorb a filter, which is exactly what made finding 1 reachable.

---

SUGGESTIONS

3. ECB had no logger. Added, mirroring NBP: debug on the resolved observation, and debug on the empty-200 non-publication branch — the ECB analogue of NBP's walk-back line, i.e. the one branch that reports the API working correctly. Following the same thread one layer up: recordTerminal now takes the caught error's message, so ECB responded 400 for EXR.D… reaches the log. Previously the reason enum was the entire record of a permanent answer — an operator could see that an order was refused, never why.

4. Migration header. Corrected to 1833000000006 (add-order-record-sales-document-block), and it now states that 1834000000000 is uncontested since the fiscalization stack moved to 1835000000000. The correction is noted in place rather than quietly swapped, since the next author reads the note instead of re-deriving it.

5. Walk-back off-by-one. NBP_MAX_WALK_BACK_DAYS = 7 with <= → NBP_MAX_WALK_BACK_ATTEMPTS = 8 with <. Same 8 requests as before, now named for what it counts and matching the spec's <= 8. The doc comment says why it counts attempts.

6. Silent ECB staleness. OBSERVATION_LAG_WARN_DAYS = 3: above it the observation is still accepted (4 days is a real TARGET closing run, so refusing would be strictly worse) and WARN-logged naming the lag. MAX_OBSERVATION_LAG_DAYS = 10 stays the hard assertion. The band between them is where "legal" and "the source has stopped publishing" are indistinguishable.

7. Wave-less enqueueRetry key. Confirmed intended, and now documented at the method with the distinction from #2039 spelled out: there the delayed chain was the only route, so a spent key lost the read; here the hourly sweep reads the frontier predicate directly and needs no key. A wave component would add a second inline ladder on top of a mechanism that already covers the case.

8. A11y — this one was already half-true, and broken for a different reason. Alert (shared/ui/alert.tsx) already sets role="status" for every non-error tone, so the literal fix was in place. The real gap is that a live region inserted together with its content is not reliably announced — screen readers watch an existing region for mutations, so a conditionally-mounted Alert can appear silently regardless of its role. The dialog therefore mounts one persistent sr-only role="status" aria-live="polite" region that is empty until a warning applies. Its text is derived rather than duplicated: it names the warning and, for the coverage gap, the acknowledgement that gates Save — otherwise a screen-reader user meets a disabled button with no stated cause. Four tests cover it, including the region existing while still empty and returning to silence.

9. Boot assertion. CurrencyModule.onApplicationBootstrap throws NoExchangeRateProvidersRegisteredError (naming FxIntegrationModule) on an empty registry, and logs the registered sources otherwise. onApplicationBootstrap, not onModuleInit, because Nest runs every module's onModuleInit first — that is the earliest point at which "still empty" means "nobody will ever fill it". It lives on the core module on purpose: a host missing FxIntegrationModule is precisely the case that module cannot assert anything from. Core still learns nothing about which providers exist, only that the count is not zero. Both hosts and both integration harnesses boot the full AppModule, so nothing legitimately boots without providers.

---

Tests

New or extended, all passing locally:

• fx-http.client.spec.ts (new) — UA on every request, accept preserved, and the transient/terminal status table (408/429 and 5xx transient; 400/401/403/404/406/422 left to the adapter).
• Both adapter specs — it.each([429, 408]) transient.
• ECB spec — the warn band asserted at 3d (silent), 4d and 6d (warned), all three still resolving, so the warn can never be mistaken for a refusal.
• order-record.repository.spec.ts — markFxTerminal guards on reportingCurrency only and carries no fxStampedAt predicate; the frontier renders two arms with the invariant in both.
• marketplace-order-fx-stamp-sweep.handler.spec.ts (new) — both window bounds derived, cooldown default on four bad inputs, the 365-day clamp, payload rejection, retryable wrap.
• currency.module.spec.ts (new) — boot fails empty, names the module, passes with one provider.
• currency-settings-dialog.test.tsx (new) — the four live-region cases.

docs/architecture-overview.md § Currency restates the sweep predicate (it documented the old two-column one) and records why 429/408 are transient and why an empty registry fails the boot.

Gate: pnpm lint (incl. check:invariants) and pnpm type-check both clean. Targeted suites green: libs/integrations/fx 106, core currency + orders FX 167, worker sweep handler 8, api orders/currency controllers 42, web currency-settings 17. Full Test / Integration Tests are CI's to confirm on this commit.

[piotrswierzy]

Delta re-review (242c1613 → f42c4459) — ❌ Request changes

To be clear about the change in label: my previous review said "ready pending findings 1 and 2", so those were always merge prerequisites rather than optional. The new head is two files (+7/−3) and touches neither, so I'm making the gate explicit rather than leaving it as a comment someone could read as approval. No new problems — the delta is fine as far as it goes.

Prior finding	Status	Evidence
I1 429/408 classified terminal	❌	nbp-exchange-rate.adapter.ts:281 still >= 500 → transient, :290 still >= 400 → RateUnsupportedPairError, with the "ANY non-404 4xx is terminal, deliberately" comment intact. ecb- identical. No 429/408 literal in either adapter
I2 no User-Agent	❌	fx-http.client.ts:71 still sends accept as its only header
S3 ECB has no logger	❌	grep -c "new Logger" → ECB 0, NBP 1
S4 migration header comment	❌ (comment-only)	Slot re-verified free: main's tail is now 1833000000006, so 1834000000000 remains valid
S5 NBP_MAX_WALK_BACK_DAYS = 7 yields 8 attempts	❌	And the log at :220 renders attempt + 1/7, so attempt 8 prints "8/7"
S6 ECB 10-day staleness unlogged	❌	Blocked on S3 — no logger in the file
S7 enqueueRetry key has no wave component	❌	order-fx-stamp.service.ts:424
S8 reactive Alerts in no live region	❌	No web file in the delta
S9 no boot assertion on an empty FX registry	❌	Not present

I1, traced end to end — and the good news

I re-traced the whole chain, because the fix is only worth asking for if the consequence is real. It is:

• terminal → order-fx-stamp.service.ts:327 → recordTerminal (:391) → order-record.repository.ts:828 markFxTerminalIfAbsent writes { fxStampedAt };
• the sweep frontier (:853-854) filters fxStampedAt: IsNull(), reportingCurrency: IsNull(), so a terminal row leaves the frontier permanently, and git grep -E "clearFx|resetFx|unmarkFx" still finds no re-open path;
• transient → enqueueRetry (:195) never touches fxStampedAt, so a transiently-classified row correctly stays on the frontier.

That last point is the good news: the one-line move of 429/408 into the >= 500 arm is sufficient and needs no new recovery machinery. The surrounding design is already right — only the classification is wrong. And NBP is public and unauthenticated with no User-Agent (I2), which makes a burst-triggered 429 the single most likely real failure, currently costing those orders their reported figure forever.

Please land it with a spec asserting a 429 classifies transient and leaves fxStampedAt NULL — the second half is what actually pins the behaviour.

Highest-risk invariants: untouched, verified structurally. Neither adapter, the repository, nor the ORM entity appears in git diff 242c1613..f42c4459 --name-only, so the from/to quote-direction contract with its opposite pivot ordering, the six FX columns excluded from toOrm, and all three guarded conditional updates (claimFxIntentIfAbsent, stampFxIfAbsent, markFxTerminalIfAbsent) are byte-identical to the head I reviewed. No re-verification needed.

SUGGESTION

The rateSource change is a behaviour change, not a nit — worth saying so it isn't mistaken for a refactor in the merge log. currency-settings.controller.ts:142-159 now returns rateSource: null on the no-provider branch instead of resolveSourceKey(candidate). It's the more honest answer (no provider ⇒ no source) and it is covered — currency-settings.controller.spec.ts:135 asserts null when no provider is registered. No action.

Minor: the new cron key is appended out of alphabetical order in all four cronKeys arrays in scheduler.service.spec.ts, while 'order-fx-stamp-sweep' was correctly inserted alphabetically in the task-name list. Those arrays carry a comment telling the next author to register there, so keeping them sorted preserves their diff-scan value.

CI: fresh run on f42c4459 not yet conclusive — Docker smoke in progress; Lint, Build, Test, Integration Tests, Type Check all queued. Only PHP, path detect and Scaffolded Adapters have passed. Note the Test / Integration Tests pair is now unobserved across two consecutive reviews — and the scheduler-spec change in this very delta is exactly the class of fix whose correctness only CI proves. Please let it finish.

Merge readiness: ❌ two small changes away — 429/408 into the transient arm in both adapters, plus a User-Agent header. The machinery around them is already correct, so this should be a short round trip.

[norbert-kulus-blockydevs]

Re-review targeted a superseded head - all 9 findings are implemented, plus the minor and the rebase

The delta re-review reads 242c1613 -> f42c4459. f42c4459 stopped being the head at 09:04:44Z, when e2a61ed landed with all nine findings; the re-review submitted at 09:09:19Z. So the nine ❌ describe a head that was four minutes stale, not the code as it stands. Nothing in the review was wrong about f42c4459 - it simply is not what the branch contains.

New head is 08fc2f6. Two things happened since the re-review: the branch is rebased onto main (it was BEHIND), and the one genuinely open item - the cronKeys ordering minor - is fixed.

The nine findings, re-evidenced at 08fc2f6

Finding	Where it lives now
I1 429/408 terminal	fx-http.client.ts:57 FX_TRANSIENT_STATUS_CODES = [408, 429], :64-65 isTransientFxStatus = >= 500 || 408/429. Called at nbp:294 and ecb:307, so it is one choke point rather than two parallel ladders
I1 recovery path	The review asked whether a terminal marker should ever re-open, since no-rate-source had the same permanence. It does now: the frontier gained a second OR'd arm (order-record.repository.ts:873, fxStampedAt < terminalRetryBefore) that re-admits a terminal row still carrying no figure, cooldown terminalRetryDays default 7. markFxTerminalIfAbsent became markFxTerminal (:835), guarded on reportingCurrency IS NULL alone, so a re-answer moves the marker forward instead of the row re-entering on every tick. The stamp itself stays immutable - reportingCurrency IS NULL sits in both frontier arms and in the marker guard
I1 spec, both halves	it.each([429, 408]) in nbp-…spec.ts:300 and ecb-…spec.ts:366 for the transient half; order-fx-stamp.service.spec.ts:293 asserts markFxTerminal is not called on a transient failure, which is the half that pins fxStampedAt staying NULL
I2 User-Agent	fx-http.client.ts:40 FX_USER_AGENT, sent at :114 on every fxGet. undici sends no default UA at all, and this package deliberately carries no local retry loop to absorb a filter, so it is the only thing identifying the caller
S3 ECB logger	ecb-…adapter.ts:168. Debug on the resolved observation and on the empty-200 non-publication branch (the ECB analogue of NBP's walk-back line). The core terminal log also carries the provider's own message now, which previously died in the catch
S4 migration header	1834000000000-add-order-fx-stamp.ts:17-18 states the tail is 1833000000006 and that 1834000000000 is uncontested since the fiscalization stack moved to 1835000000000. Re-verified post-rebase by check-migration-timestamps against the current origin/main
S5 = 7 yields 8 attempts	NBP_MAX_WALK_BACK_ATTEMPTS = 8 with attempt < (nbp:67, :226). The 8/7 log is gone with it
S6 ECB staleness silent	OBSERVATION_LAG_WARN_DAYS = 3 (ecb:105), warn at :422. Above 3 days the observation is still accepted, because a 4-day run is a real TARGET closure, but it is no longer silent
S7 wave-less retry key	Confirmed intended and documented at order-fx-stamp.service.ts:436-446, including why #2039's refreshSnapshot fix does not transfer: there the delayed chain was the only route, here the hourly sweep re-reads straight from the frontier predicate and needs no key
S8 a11y live region	currency-settings-dialog.tsx:140. Alert already carried role="status", but a live region inserted together with its content is not reliably announced, so the dialog now mounts one persistent visually-hidden region naming whichever warning materialised and the acknowledgement it requires. Four tests
S9 empty FX registry	currency.module.ts:102 onApplicationBootstrap throws NoExchangeRateProvidersRegisteredError. onApplicationBootstrap rather than onModuleInit so plugin registration has run

New in 08fc2f6

The cronKeys minor, taken one step further than asked. The four arrays in scheduler.service.spec.ts were not alphabetical to begin with (INVENTORY, PRODUCT, PICKUP_POINT, REGULATORY, OFFLINE, PENDING, STALE_OFFER, TAXONOMY), so inserting only the new key would have left the diff-scan property the comment promises still broken. All four are sorted, the convention is stated in the comment, and the fourth array - which carried no comment at all - now has it. Test-only; the suite passes 35/35.

Rebase onto main. 23 commits replayed with no conflicts, all signed. The five commits picked up are #2144 inventory upsert column-scope, #2149 orders filtered-empty-state, the 0.7.0 release, the analytics design package and #2150 identity cells. Diffing the pre-rebase head against the new one shows exactly those five and nothing else, and the Dockerfile still carries its three libs/integrations/fx lines.

Local gate on the rebased tree: build, type-check, lint including all 22 invariant checks, all green.

CI

Point taken that Test and Integration Tests went unobserved across two reviews. Both were queued, not skipped - runner backlog on run 32235793483, which sat at queued for the four heavy jobs from 09:05 while Type Check, Docker smoke and PHP completed. A fresh run is now going on 08fc2f6. I will not ask for a re-review until Test and Integration Tests are both green on that run.

[norbert-kulus-blockydevs]

CI green on 08fc2f6, including the pair that went unobserved twice

All nine checks pass on the rebased head:

Check
Build	pass
Lint	pass
Type Check	pass
Test	pass
Integration Tests	pass
Docker Build Smoke Test	pass
PHP Unit Tests	pass
Scaffolded Adapter Builds	pass
Detect changed paths	pass

That closes the last item from the re-review. Test and Integration Tests had been queued rather than skipped on the previous run, and both are now observed green on the head that carries the nine findings, the cronKeys ordering fix and the rebase onto main.

The branch is no longer BEHIND. Ready for a re-review whenever you have a slot, @piotrswierzy.

[piotrswierzy]

Delta re-review (f42c4459/e2a61ed4 → 08e2c7f3) — ✅ Approve

Both merge prerequisites from the last round have landed, and the fix I did not ask for — a recovery path out of a terminal answer — is the one that most improves the feature. Lifting the gate.

Prior findings

#	Status	Evidence
I1 429/408 terminal	✅	fx-http.client.ts:57 FX_TRANSIENT_STATUS_CODES = [408, 429], isTransientFxStatus = >= 500 || includes(status); both adapters route through it (nbp:295, ecb:308) with the reasoning inline
I1b terminal had no recovery	✅ beyond ask	findUnstampedFxOrderIds is now two OR'd branches sharing reportingCurrency IS NULL — unanswered, or fxStampedAt < terminalRetryBefore. markFxTerminal deliberately drops the fxStampedAt IS NULL guard so a re-answer moves the marker forward instead of re-entering every tick. Default cooldown 7 days, clamped at 365
I2 no User-Agent	✅	fx-http.client.ts:114
S3 ECB had no logger	✅	ecb:168
S4 migration header timestamp claim	✅	Re-verified against origin/main: tail is 1835000000000 (fiscalization, #2137); 1836000000000 is strictly greater and uncontested. Header now says exactly that, including why it moved
S5 MAX_WALK_BACK_DAYS = 7 / "8/7" log	✅	Renamed NBP_MAX_WALK_BACK_ATTEMPTS = 8, loop is attempt < N, log renders /8
S6 ECB 10-day staleness silent	✅	ecb:411-422 warns, with the "max real non-publication run is 4 days" reasoning
S7 fx:{id} key has no wave	✅ (documented, as asked)	The enqueueRetry doc now states it is intended and why the #2039 precedent doesn't transfer: the sweep reads the frontier directly and needs no key
S8 reactive Alerts in no live region	✅	currency-settings-dialog.tsx:140 role="status" aria-live="polite"
S9 no boot assertion on an empty registry	⚠️ accepted	Still absent, but I1b removes its teeth: a host booted without FxIntegrationModule now produces terminal rows that the sweep re-admits after the cooldown once the wiring is fixed, instead of permanent silent loss. Fine as-is
cronKeys ordering	✅	08fc2f629

What I re-verified structurally

The invariants I care most about are unchanged or strengthened:

• Quote direction — rate is to per one from; the single conversion site multiplies (order-fx-stamp.service.ts), the opposite pivot ordering between NBP and ECB is intact, and the "should never divide" negative test still stands.
• Snapshot immutability — all six FX columns remain outside toOrm's write set; the three writers are still guarded single-statement conditional updates answering via affected > 0. stampFxIfAbsent keeps reportingCurrency: IsNull(), so the figure is untouchable regardless of the new marker semantics — which is exactly the right thing to have relaxed and the right thing to have kept.
• Migration — hand-authored with reasons; IF NOT EXISTS throughout, full down(), both CHECKs dropped-then-added idempotently, and the ck_order_records_fx_group first arm's deliberate omission of fxRule IS NULL is documented against claimFxIntentIfAbsent's intent row. The expression index matches NATIVE_CURRENCY_EXPR verbatim (kept as a shared constant precisely so it can't drift).
• Idempotency — registry get-or-create is insert-then-recover on PG 23505 matched by code; the stamp is stamp-once at the SQL level; the sweep walks sequentially and each stamp swallows its own failure; the ingestion seam never fails the persist.
• Boundaries — no HTTP anywhere in libs/core/src/{currency,orders}; libs/integrations/fx is registered in check-outbound-http.mjs; the new currency context is a leaf with no orders back-edge.

Minor, non-blocking: orders imports the pure helpers resolveRateDate / resolveRateSource from @openlinker/core/currency. They're pure, sit on the barrel, and the invariant checker is green — but they're plain functions rather than one of the enumerated cross-context shapes (I*Service / *Port / is* / entity / UPPER_SNAKE_CASE). Worth a line in architecture-overview.md § Cross-context dependencies if this becomes a pattern; not worth a round trip now.

CI: Type Check, Build, Docker smoke, PHP, Scaffolded Adapters green on 08e2c7f3. Lint, Test and Integration Tests are still in progress — that pair has now been unobserved across three reviews, so please confirm all three green before merging. Approving on the code; the CI gate is yours to hold.

Merge readiness: ✅ pending green Lint / Test / Integration Tests. Everything I asked for landed, and the terminal-recovery work exceeded the ask.