feat(ksef): emit FA(3) Platnosc from connection payment config

[norbert-kulus-blockydevs]

Summary

• Implements [FEATURE] Integration — emit FA(3) Platnosc (payment method, bank account, payment term, skonto) in KSeF invoices #1311: emit the FA(3) Platnosc element (payment method, bank account, payment term, skonto) as a per-connection, manually-entered config value in the KSeF integration.
• Started as a plan-only PR (implementation plan + design mockup); now carries the full implementation on the same branch to avoid a separate tech-debt PR (per docs/plans/implementation-plan-ksef-payment-platnosc.md).
• KSeF has no live bank-accounts API (unlike inFakt [TECH-DEBT] Integration — Infakt payment_method hardcoded and inconsistent between issueInvoice/issueCorrection #1303/[FEATURE] Integration+Frontend — Infakt live bank-account preview during wizard (locked-select variant) #1308) - the operator types the payment info in once per connection; it flows through the existing resolveSeller/resolveDefaultTaxRate factory -> adapter -> mapper -> pure-builder chain.
• Schema-audited XSD child order inside Platnosc: TerminPlatnosci -> FormaPlatnosci -> RachunekBankowy -> Skonto (NOT payment-method-first, despite that being the more intuitive reading order) - Platnosc itself is a sibling of FaWiersz under Fa, not nested inside it.
• No CORE changes - everything stays inside libs/integrations/ksef and its FE plugin section (ADR-026 compliant).
• Rides along, worth its own git log line: AES session-IV reuse fix in the KSeF session crypto (protocol-mandated by the KSeF v2 wire shape; fresh per-document IVs failed with status 430; documented CBC-determinism trade-off) - details under "Also included" below.
• Live Playwright smoke test (Phase 6) found and fixed a real bug: see verification artifact.

What's included

• Domain types: KsefPaymentConfig / KsefBankAccountConfig / KsefFormaPlatnosciValues on KsefConnectionConfig; Fa3PaymentInput / Fa3BankAccount / Fa3FormaPlatnosciValues for the builder.
• Builder: platnoscNode() in fa3-xml.builder.ts, wired into faNode() right after FaWiersz; omits Platnosc entirely when nothing is configured (existing connections keep byte-identical output).
• Wiring: KsefAdapterFactory.resolvePayment (mirrors resolveSeller/resolveDefaultTaxRate) threaded through the adapter constructor and Fa3MappingContext.
• Validation: KsefConnectionConfigShapeValidatorAdapter rejects an unknown formaPlatnosci, an empty bankAccount.nrRb, a negative/non-integer paymentTermDays, or an incomplete skonto (added after the smoke-test bug below) at connection-save time.
• Frontend: ksef-payment-config.ts assembly module (mirrors ksef-seller-config.ts) wired into edit-connection.schema.ts + EditConnectionForm.tsx; new fields in ksef-structured-section.tsx (payment method select + bank account/SWIFT/term/skonto inputs, each crediting its FA(3) target element in the description copy).
• Docs: FA3_IMPLEMENTATION_NOTES.md updated with the Platnosc mapping table and emit order.
• Design mockup: docs/plans/mockups/ksef-payment-platnosc.html (added in the plan commit; renamed from infakt-ksef-bank-account-payment-terms.html per review - the mockup is the KSeF payment section, the old prefix only credited the inFakt precedent).

Bug found via the live smoke test (fixed in this PR)

ksef-payment-config.ts's applyKsefPaymentToConfig originally deleted the whole bankAccount/skonto sub-object whenever a sibling field wasn't present in the same merge call. Since each field syncs to configText independently per keystroke, this silently discarded whatever the operator typed first — skonto could never actually be saved. Fixed: a sub-object is now dropped only when it's completely empty, never based on a missing sibling; the "required together" rules moved to the backend shape validator (save time) and the factory's resolvePayment (issuance time). Full writeup + before/after screenshots in the verification artifact.

Test plan

• pnpm --filter @openlinker/integrations-ksef test - 305+ pass, including full XSD structural validation of the new Platnosc block for both configured and unconfigured connections, in the correct child order.
• pnpm --filter @openlinker/web vitest - full suite pass, including new ksef-payment-config/ksef-structured-section coverage.
• pnpm check:invariants - clean (cross-context imports, service interfaces, etc.).
• pnpm --filter @openlinker/integrations-ksef lint / pnpm --filter @openlinker/web lint - no new errors or warnings in touched files.
• Live smoke test (Phase 6) against a running dev-stack KSeF connection with real Playwright screenshots — verification artifact. Live KSeF sandbox submission was attempted with a real sandbox token but blocked at the auth-handshake layer (unrelated to this PR — see the artifact's "Live sandbox attempt" section); the identical builder/mapper code path is covered by XSD-validated unit tests instead.

Closes #1311
Closes #1330 (assembly-relocation refactor merged into this branch via PR #1334; it targets the default branch through this PR)

Also included (bundled changes surfaced by tech review)

Two live-E2E bug fixes and one drive-by CI fix ride along on this branch; they are documented here so the trade-offs are auditable from the PR, not only from the diff:

• AES session-IV reuse (ksef-session-crypto.service.ts, ksef-crypto.types.ts): encryptDocument now reuses the session-declared IV instead of generating a fresh per-document IV. The KSeF v2 wire shape has no per-document IV field and the CIRFMF reference client confirms session-IV reuse is the protocol's intended model - a fresh IV made every submission fail with status 430. Trade-off: reusing one IV across documents within a session yields deterministic ciphertext for identical plaintexts in that session (a known CBC property). Accepted because the protocol mandates it, the session key is short-lived, and invoice payloads are practically never byte-identical (timestamps, invoice numbers).
• Required JST/GV elements in Podmiot2 (fa3-xml.builder.ts buyerNode): the FA(3) XSD declares these without minOccurs="0", so omitting them failed live clearance with status 450. Emitted with the 2 ("no") values for regular buyers.
• /v1/listings int-spec fix (fix(api): version listings invalid-path-id int-spec paths under /v1 #1328, merged into this branch): main's Integration Tests job was red due to a semantic merge conflict between feat(api): version the HTTP API under /v1 + runtime version surface (#1133) #1316 (global /v1 URI versioning) and fix(listings): reject non-UUID path ids with 400 instead of 500 #1313 (a new int-spec calling unprefixed /listings/...); the spec's paths are now versioned. Unrelated to Platnosc but required for this PR's CI to be green.

[norbert-kulus-blockydevs]

Tech Lead Review — PR #1317 (KSeF FA(3) Platnosc)

Summary

Solid, well-scoped PR. Everything stays inside libs/integrations/ksef and its FE plugin slice (ADR-026 compliant — no CORE changes), the factory→adapter→mapper→builder wiring mirrors the existing resolveSeller/resolveDefaultTaxRate chain exactly, naming/typing conventions (as const unions, *.types.ts, file headers) are followed, and test coverage is thorough (builder XSD-order cases, mapper, adapter, factory, shape validator, FE assembly + component). No architecture or CORE/Integration boundary violations found. Two SUGGESTION-level gaps below, neither blocking.

Issues

[SUGGESTION] — libs/integrations/ksef/src/infrastructure/adapters/ksef-connection-config-shape-validator.adapter.ts (payment.bankAccount.nrRb check) / apps/web/src/features/connections/components/edit-connection.schema.ts (paymentBankAccountNrRb schema)

The implementation plan documents NrRB as TNrRB, 10–34 chars per the XSD pattern (§4), but neither the shape validator nor the FE Zod schema enforce a minimum length — only non-emptiness (BE) / a max of 34 (FE). An operator could save an obviously truncated account number (e.g. 3 digits) that passes both client and server validation, only to fail at actual KSeF submission time with an opaque XSD error far removed from the config-save screen where the mistake was made. Since engineering-standards.md's validation guidance is to reject bad input at the boundary rather than let it surface downstream, consider adding a length/pattern check mirroring the XSD (10–34 chars) to the shape validator, with the FE schema following suit (.min(10)).

[SUGGESTION] — libs/integrations/ksef/src/application/factories/ksef-adapter.factory.ts:168-193 (resolvePayment)

The method's doc comment claims a defensive posture "mirroring resolveDefaultTaxRate's defensive posture," and it does re-validate bankAccount (drops it if nrRb is falsy) and skonto (drops it unless both sub-fields are present) — but formaPlatnosci and paymentTermDays are passed through unchecked (result.formaPlatnosci = payment.formaPlatnosci; / result.paymentTermDays = payment.paymentTermDays;). In the normal flow the shape validator already rejects an out-of-range formaPlatnosci or a negative/non-integer paymentTermDays at save time, so this is low-impact — but for a connection whose config predates the validator (or was written through another path), an invalid code/negative day-count would be emitted verbatim into FormaPlatnosci/TerminOpis/Ilosc rather than being defensively dropped like the other two fields. Minor inconsistency worth a one-line guard (or a code-comment caveat) if the "defensive" framing is meant to be complete.

Verdict

✅ Approve — both findings are non-blocking robustness suggestions; nothing here violates hexagonal boundaries, naming conventions, or the documented ADR-026 country-agnostic-core rule.

[norbert-kulus-blockydevs]

Addressed both SUGGESTION findings from the tech-lead review in 6d29c91:

• payment.bankAccount.nrRb now enforces the FA(3) TNrRB length bound (10-34 chars) in both the shape validator and the FE Zod schema, not just non-emptiness.
• resolvePayment now defensively drops an unknown formaPlatnosci code or a negative/non-integer paymentTermDays, mirroring the existing bankAccount/skonto guards, for connections whose config predates the ksef.publicapi.v2 shape validator.

Added corresponding unit tests in both the KSeF integration package and the web edit-connection schema. pnpm --filter @openlinker/integrations-ksef test (315 passed), pnpm --filter @openlinker/integrations-ksef lint/type-check, and pnpm --filter @openlinker/web lint/type-check all clean.

[piotrswierzy]

Reviewed against the vendored schemat_fa3_v1-0e.xsd in the repo. The wiring is clean and idiomatic — mirrors resolveSeller/resolveDefaultTaxRate end-to-end, no CORE changes (ADR-026 clean), as const unions, types in *.types.ts, no any, and the config-merge fix + validator prototype-safety both check out. I verified most XSD claims and they hold: the Platnosc position under Fa (after FaWiersz/Rozliczenie, both optional), the TerminPlatnosci → FormaPlatnosci → RachunekBankowy → Skonto order, the TerminOpis complex {Ilosc, Jednostka, ZdarzeniePoczatkowe} shape, and the TFormaPlatnosci 1–7 values are all correct.

One blocking issue and one process gap.

🔴 BLOCKING — RachunekBankowy emits NazwaBanku before SWIFT (XSD-invalid)

rachunekBankowyNode in fa3-xml.builder.ts builds keys as NrRB → NazwaBanku → SWIFT, but the vendored XSD TRachunekBankowy sequence is:

NrRB, SWIFT        (inner sequence)
RachunekWlasnyBanku?
NazwaBanku?
OpisRachunku?

SWIFT must come before NazwaBanku. When a connection has both bankName and swift set, the emitted document is XSD-invalid and KSeF clearance would reject it. Single-field paths (NrRB only, NrRB+swift, NrRB+bankName) are fine — only the both-set path breaks.

Fix: emit SWIFT before NazwaBanku in rachunekBankowyNode, and update fa3-xml.builder.spec.ts:485 — it currently asserts the wrong order (<NrRB><NazwaBanku><SWIFT>), which is why the bug reads as "passing."

🟠 IMPORTANT — the structural validator doesn't cover Platnosc, so the "validator passes" tests are false comfort

validateFa3Xml only checks root/namespace/Naglowek/Podmiot/Fa-required-children/FaWiersz — it never inspects Platnosc or child ordering. The two should pass the structural FA(3) validator … tests would (and do) pass even with the bug above. Combined with the blocked live sandbox submission, element-order correctness rests entirely on manual XSD reading plus tautological "emits-what-it-emits" regex assertions.

Please add a real order check for the Platnosc block (a targeted structural assertion, or an opt-in xmllint/libxmljs conformance pass in CI) so ordering regressions like the RachunekBankowy one are actually caught rather than co-signed by the test suite. This is the reason the bug slipped through.

🟡 SUGGESTIONS (non-blocking)

• The new payment constructor arg on KsefInvoicingAdapter is inserted before the pre-existing defaulted now param. Both internal call sites were updated so nothing breaks, but a mid-list positional insert is fragile — appending or an options object would be safer.
• The "required-together" invariants now live in FE assembly + shape validator + factory resolvePayment. Fine as defense-in-depth (matches the resolveSeller precedent), just noting the triplication.

Everything else — the merge-bug fix (sub-objects dropped only when fully empty, required-together moved to validator + factory), neutrality, and typing — looks good. Fixing the RachunekBankowy order + its test, and closing the validator coverage gap, clears this for merge.

[norbert-kulus-blockydevs]

@piotrswierzy re: your review (#1317 (review)) - all findings addressed:

• BLOCKING (RachunekBankowy emits NazwaBanku before SWIFT): fixed in c315e4d. rachunekBankowyNode now emits NrRB -> SWIFT -> NazwaBanku per the XSD TRachunekBankowy sequence, and the builder spec asserts the correct order (<NrRB><SWIFT><NazwaBanku>) instead of co-signing the wrong one.
• IMPORTANT (structural validator does not cover Platnosc): addressed in f11ea72. validateFa3Xml now enforces the XSD-mandated child order of Fa/Platnosc (TerminPlatnosci -> FormaPlatnosci -> RachunekBankowy -> Skonto) and of each RachunekBankowy (NrRB required, SWIFT before NazwaBanku). Negative tests include one that reproduces exactly the blocker shape (should reject a RachunekBankowy that emits NazwaBanku before SWIFT), so an ordering regression now fails the 'passes the structural validator' tests instead of sailing through.
• SUGGESTION (positional ctor insert): also in f11ea72 - the trailing optional params (payment, now) moved into a KsefInvoicingAdapterOptions bag, so future additions never shift positional call sites.
• SUGGESTION (required-together triplication): kept as intentional defense-in-depth (matches the resolveSeller precedent); the resolvePayment doc comment now names the three layers (FE assembly / shape validator / factory) explicitly.

Why PR #1328 was merged into this branch instead of main: main's Integration Tests job has been red since #1316 (URI versioning, global /v1 default) and #1313 (new listings-invalid-path-id.int-spec.ts written pre-versioning, calling unprefixed /listings/...) merged next to each other - a semantic merge conflict. Every request in that spec hit the router-level 404 ("Cannot GET"), so the three expect(400) ParseUUIDPipe assertions failed on main and on every PR based on it, including this one. The fix (#1328, prefixing the five spec paths with /v1) could not be merged to main directly - branch protection requires an approving review - so it was rebased onto this branch and squash-merged here (0d49d3f) to unblock this PR's CI. It will reach main together with this PR; the fix is a 5-line test-path change with no runtime surface.

CI is fully green on the current head (including Integration Tests). Ready for re-review.

[norbert-kulus-blockydevs]

Tech review (Claude, /tech-review)

Summary

Solid, well-layered implementation of #1311. The Platnosc chain (FE assembly -> shape validator -> factory resolvePayment -> mapper -> pure builder) mirrors the existing resolveSeller/resolveDefaultTaxRate seams exactly, stays entirely inside libs/integrations/ksef + its FE plugin section (no CORE changes, ADR-026 compliant), and the XSD child-order work is verified against the vendored schema with regression tests in the structural validator. The single most important concern is that the PR quietly bundles two significant changes that are not mentioned in the description.

Issues

[IMPORTANT] - libs/integrations/ksef/src/infrastructure/crypto/ksef-session-crypto.service.ts, ksef-crypto.types.ts, fa3-xml.builder.ts (buyerNode JST/GV)

The PR carries two live-E2E bug fixes that the description does not list: (1) the AES session-IV change - encryptDocument now reuses the session-declared IV instead of a fresh per-document IV, reversing an explicitly documented CBC IV-reuse security warning; and (2) the required JST/GV elements added to Podmiot2. Both look correct (the KSeF wire shape has no per-document IV field, and the CIRFMF reference confirms session-IV reuse; JST/GV have no minOccurs="0" in the XSD), but the IV change accepts deterministic ciphertext for identical plaintexts within a session - that trade-off deserves an explicit line in the PR body so it is auditable from the PR, not only from the test diff. Please add a "also included" section covering both, plus the unrelated /v1/listings int-spec drive-by fix (post-#1133 URI versioning).

[IMPORTANT] - apps/web/src/features/connections/components/ksef-payment-config.ts / ksef-structured-section.tsx

The FE placeholder actively suggests the spaced NRB format (61 1090 1014 0000 0000 9999 9999), and the value is persisted and emitted into NrRB verbatim, inner spaces included. The vendored XSD's TNrRB is length-only (10-34, no pattern facet), so it passes structural validation - but a spaced account number on the wire is fragile for downstream consumers and eats into the 34-char budget. Recommend normalizing (strip whitespace) at assembly time in applyKsefPaymentToConfig, following the normalizeNip precedent, and aligning the FE zod length check with the BE validator (FE counts raw length incl. spaces, BE counts trimmed length - after stripping they converge for free).

[SUGGESTION] - libs/integrations/ksef/src/infrastructure/adapters/ksef-connection-config-shape-validator.adapter.ts

A non-object payment (e.g. payment: "foo"), bankAccount, or skonto silently passes validation and is then silently dropped by resolvePayment at issuance. Since this validator is the strict save-time gate, pushing a must be an object issue for wrong-typed sub-objects would be more consistent than falling through.

[SUGGESTION] - apps/web/src/features/connections/components/ksef-setup.schema.ts vs ksef-connection.types.ts vs fa3-schema.types.ts

['1'..'7'] is now declared three times (FE, plugin connection-config layer, FA3 schema layer). FE/BE duplication is unavoidable; the two BE copies are intentionally layered but a one-line cross-reference comment already exists - fine as is, just flagging so a future 8th code gets added in all three places.

[SUGGESTION] - apps/web/e2e/ksef-payment-config.mjs

Hardcoded admin/admin123 dev-stack credentials; acceptable for a local smoke script, but reading them from env (with these as defaults) would match the WEB_BASE/KSEF_CONN_ID pattern already used.

What's good

• Correct XSD emit order (TerminPlatnosci -> FormaPlatnosci -> RachunekBankowy -> Skonto; SWIFT before NazwaBanku) with negative-order regression tests in fa3-xsd.validator.spec.ts - exactly the kind of check that keeps a future builder refactor honest.
• The per-keystroke merge-drop bug fix is the right call: persistence never gates on sibling completeness; required-together rules live in the shape validator (save time) and resolvePayment (issuance time), with the three-layer rationale documented at each layer.
• KsefInvoicingAdapterOptions options-bag refactor removes the positional-optional-arg trap before it bites.
• Byte-identical output for unconfigured connections is asserted, not just claimed.
• Naming, as const unions, *.types.ts placement, file headers, and the feature-barrel export all follow the documented standards.

Verdict

🔄 Approve with changes - mergeable after the two IMPORTANT items (document the bundled IV/JST-GV/int-spec changes in the PR body; normalize the NRB input).

[norbert-kulus-blockydevs]

Re: the second tech-review round (2 IMPORTANT + 3 SUGGESTION) - all findings addressed in 3a960dc + a PR-body update:

• IMPORTANT (undocumented bundled changes): the PR body now has an "Also included" section covering all three ride-alongs: the AES session-IV reuse change (with the deterministic-ciphertext trade-off spelled out and why the protocol mandates it), the required JST/GV elements in Podmiot2, and the /v1/listings int-spec drive-by fix (fix(api): version listings invalid-path-id int-spec paths under /v1 #1328) with the reason it had to land on this branch.
• IMPORTANT (spaced NRB persisted verbatim): fixed in 3a960dc. New normalizeNrRb helper (mirrors the normalizeNip precedent) strips all whitespace at assembly time in applyKsefPaymentToConfig, so the persisted config.payment.bankAccount.nrRb and the FA(3) NrRB wire value never carry inner spaces; the FE zod check now transforms through the same helper and counts the stripped length, converging with the BE validator's 10-34 bound. Tests cover the spaced-paste path on both the schema and the assembly (the old test that co-signed the spaced persistence now asserts the stripped value).
• SUGGESTION (non-object payment/bankAccount/skonto passes silently): the shape validator now pushes an explicit must be an object issue for a wrong-typed (or null) payment, payment.bankAccount, or payment.skonto instead of falling through to the issuance-time silent drop. Negative tests added for all three paths.
• SUGGESTION (tripled '1'..'7' list): all three declaration sites (ksef-setup.schema.ts, ksef-connection.types.ts, fa3-schema.types.ts) now carry a symmetrical cross-reference comment naming the other two, so a future 8th code gets added everywhere.
• SUGGESTION (hardcoded e2e credentials): ksef-payment-config.mjs reads WEB_USER / WEB_PASSWORD env vars with the dev-stack defaults preserved, matching the existing WEB_BASE / KSEF_CONN_ID pattern.

Verified: pnpm --filter @openlinker/integrations-ksef test (324 passed), KSeF lint + type-check clean, web vitest for the touched suites (edit-connection schema 76 passed, ksef plugin 54 passed), web lint (0 errors) and type-check clean.

[norbert-kulus-blockydevs]

Tech review summary

Summary

Well-executed, self-contained KSeF feature. The FA(3) Platnosc element is sourced from manually-entered per-connection config and threaded through the established resolveSeller-style chain (factory -> adapter -> mapper -> pure builder) with no CORE changes (ADR-026 respected). Test coverage is strong: XSD child-order regression tests for Platnosc and RachunekBankowy, shape-validator tests, FE sync-routing tests, plus a live Playwright smoke test that caught a real per-keystroke config-sync bug (incomplete sub-objects being silently discarded) - and the fix (persist partial, enforce required-together at save/issuance time) is the right layering.

Issues

[IMPORTANT] ksef-connection-config-shape-validator.adapter.ts - nrRb whitespace asymmetry between FE and BE
The FE (normalizeNrRb) strips all whitespace before the 10-34 length check and before persisting, but the BE validator counts nrRb.trim().length - inner spaces count toward the bound and are not rejected. A config written directly via the API (bypassing the FE) with a conventionally-spaced NRB ("61 1090 1014 ...") passes the validator, and resolvePayment + platnoscNode emit it verbatim, so spaces reach the FA(3) wire and will fail KSeF's TNrRB pattern at clearance. Since the validator's own header calls it "the strict save-time gate", it should either reject inner whitespace or apply the same normalization as the FE.

[IMPORTANT] fa3-xml.builder.ts buyerNode - hardcoded JST: 2 / GV: 2 limitation is undocumented
Correct for the common case and required by the XSD, but a buyer that actually is a JST subsidiary unit or a VAT-group member would get a false declaration on a fiscal document. Please record this in FA3_IMPLEMENTATION_NOTES.md under "Known limitations / deferred work" so it doesn't silently become "the behavior".

[SUGGESTION] Bundled changes - the session-IV fix and JST/GV fix were found by this PR's own live E2E and are well-documented in the body, so bundling is acceptable; the /v1/listings int-spec CI fix (#1328) would have been cleaner as its own fast-merge PR.

[SUGGESTION] KSeF-specific assembly keeps accumulating in the shared features/connections (ksef-payment-config.ts, ksef-nrb.ts, values in ksef-setup.schema.ts), following the ksef-seller-config.ts precedent. Consistent with the existing pattern, so fine here - but worth a follow-up issue to move plugin-specific config assembly behind a plugin slot before a third platform needs the same treatment.

[SUGGESTION] KsefFormaPlatnosciValues is declared three times by design with cross-references in all three places - honest trade-off. A small drift test asserting the three arrays are equal would make the "add the 8th code in all three places" instruction self-enforcing.

Notable positives

• The session-IV reuse change is correctly justified against the protocol (no per-document IV field on SendInvoiceRequest; CIRFMF reference client confirms session-IV reuse), with the CBC determinism trade-off documented in both the code and the PR body.
• The hand-rolled Platnosc / RachunekBankowy child-order checks in fa3-xsd.validator.ts guard exactly the regression class (element ordering) that the presence-only checks would miss, including the RachunekBankowyFaktora prefix-collision and the nested-FormaPlatnosci caveat.
• KsefInvoicingAdapterOptions bag for trailing optional constructor inputs is the right call to keep positional call sites stable.
• Byte-identical output guarantee for existing connections (omit Platnosc entirely when unconfigured) is explicitly tested.

Verdict

🔄 Approve with changes - fix the nrRb whitespace gap in the shape validator and record the JST/GV limitation; everything else is optional.

[norbert-kulus-blockydevs]

Heads-up: stacked PR #1334 (refactor(web): move plugin-specific connection-config assembly behind a plugin slot) has been merged into this branch, so its diff is now part of this PR.

Why it lands here rather than as a separate PR to main: the refactor extracts the KSeF payment/seller connection-config assembly (introduced by this PR) out of the shared connections feature and behind a generic plugin slot. It builds directly on files added here (ksef-payment-config.ts, ksef-seller-config.ts, edit-connection schema wiring), so it could not target main independently without conflicting with this branch.

Verification after the merge (stacked branches do not trigger CI, so this was checked locally on the merged tree): web type-check clean, all 1977 web unit tests green, web lint 0 errors, and the updated check-ksef-forma-platnosci-drift.mjs passes against the moved file path. The branch still merges cleanly into main.

[norbert-kulus-blockydevs]

Tech Lead Review — #1317

Summary

High-quality, well-documented PR. The KSeF Platnosc feature (#1311) follows the established resolve → adapter → mapper → builder chain exactly like resolveSeller/resolveDefaultTaxRate, stays fully inside libs/integrations/ksef (ADR-026 compliant, no CORE changes), and ships defense-in-depth validation at three independent layers (FE Zod, BE shape validator, adapter-factory resolvePayment) with matching bounds (0-999 days, TNrRB 10-34 chars) — each layer's rationale is explicit in comments, including why each is still needed. The bundled #1330 connection-config plugin-slot refactor is a genuine architectural improvement: it moves KSeF-specific fields out of the shared edit-connection.schema.ts into the plugin package via a ConnectionConfigContribution seam, with a compiler-enforced link between the Zod fragment and the declaration-merged form fields, and a runtime cross-plugin field-name collision guard. Test coverage is thorough at every layer (XSD child order, drift-guard, factory defensive-dropping, shape-validator rejection paths, FE per-keystroke sync). The main open concern is scope: this single PR bundles three separable changes (the KSeF payment feature, the FE plugin-slot architectural refactor, and an unrelated CI int-spec path fix), which makes it a heavy review unit — acceptable given the PR body's explicit rationale, but worth flagging.

Issues

[SUGGESTION] — PR scope (whole PR)

Three logically independent changes ride on one branch: the #1311 KSeF Platnosc feature, the #1330 FE connection-config plugin-slot refactor (touches every plugin's edit-connection wiring), and the unrelated #1328 /v1/listings int-spec path fix needed only because main's CI was red. The PR body justifies this well (avoiding a second tech-debt PR, unblocking CI), and each piece is independently well-tested, so this isn't blocking — but for a change this size, splitting the #1330 refactor into its own PR would have made the diff reviewable in isolation and given it a rollback boundary independent of the KSeF feature.

[SUGGESTION] — libs/integrations/ksef/src/infrastructure/crypto/ksef-session-crypto.service.ts

The AES-256-CBC session-IV reuse is well-justified (protocol-mandated, three independent sources cited) and the trade-off is documented in both the PR body and the type doc comment. Still, this is a real security-relevant property change (deterministic ciphertext for identical plaintexts within a session) — worth a one-line callout in FA3_IMPLEMENTATION_NOTES.md's "Known limitations" section alongside the JST/GV hardcoding, since that's where a future security reviewer would look first, rather than only in the crypto service's own docstring and the PR description.

[SUGGESTION] — scripts/check-ksef-forma-platnosci-drift.mjs

The regex-based extraction (export const ${constName} = \[([^\]]*)\] as const;) is brittle to reformatting (e.g. Prettier wrapping the array across multiple lines once an 8th code is added) — at that point the script would fail with the intended "declaration not found" error rather than silently passing, so this degrades safely, but consider parsing via the TS compiler API or a simpler line-based scan if the array ever wraps.

[SUGGESTION] — apps/web/src/plugins/ksef/ksef-connection-config.ts and edit-connection.schema.ts

readPatchString/readOptionalPatchString in ksef-connection-config.ts duplicate the same tiny helpers' shape used elsewhere in the file family (e.g. readString in EditConnectionForm.tsx). Not worth blocking on — the duplication is small and each copy is scoped to a different untyped bag shape — but if a fourth plugin contribution needs the same pattern, it's worth promoting to a shared helper exported from shared/plugins.

Nothing else surfaced: the CORE/Integration boundary is respected (no libs/core changes), naming conventions match (ksefConnectionConfig, KsefPaymentConfig, Fa3PaymentInput, applyKsefPaymentToConfig), the frontend dependency direction is correct (plugins/ksef owns its schema and lib, features/connections no longer re-exports KSeF-specific enums), state ownership rules are followed (form state via RHF, no raw fetch), and no secrets or credentials are touched by this change.

Verdict

✅ Approve

[norbert-kulus-blockydevs]

Re: the fifth tech-review round (4 SUGGESTION, 0 IMPORTANT) - all findings addressed in 36211ee:

• SUGGESTION (PR scope / bundled web: move plugin-specific connection-config assembly behind a plugin slot #1330 refactor): acknowledged as a process note - no code change possible at this point without unwinding the stack. The rationale for landing web: move plugin-specific connection-config assembly behind a plugin slot #1330 on this branch (it edits files this PR introduces, so it could not target main without conflicting) stays documented in the PR body and in the heads-up comment above; each ride-along remains independently well-tested, and the squash-merge message will carry the dedicated session-IV bullet for git-log findability.
• SUGGESTION (session-IV callout in FA3_IMPLEMENTATION_NOTES.md): added a dedicated entry under "Known limitations / deferred work", right next to the JST/GV hardcoding item, covering the protocol-mandated single-IV-per-session model, the deterministic-ciphertext trade-off (identical documents in one session produce identical ciphertext; shared prefixes observable at CBC block granularity), the bounded exposure (TLS-only transport to MF, public FA(3) header prefix), and why it cannot be fixed client-side (the status-430 rejection of per-document IVs). A future security reviewer now finds it where they would look first, not only in the crypto service docstring.
• SUGGESTION (brittle regex in check-ksef-forma-platnosci-drift.mjs): the extraction now tolerates an optional type annotation, arbitrary whitespace, a Prettier multi-line wrap of the array, and inline // / /* */ comments inside the array body - so an 8th code that makes Prettier wrap the declaration no longer trips the "declaration not found" exit. Verified against all three current declaration sites (script passes) and the full check:invariants chain is green.
• SUGGESTION (duplicated readPatchString/readString helpers): promoted to shared/plugins as readConfigString / readOptionalConfigString (new config-readers.ts, exported from the barrel). Both plugins/ksef/ksef-connection-config.ts and the host EditConnectionForm.tsx now consume the shared definition; the per-file copies are deleted. The differently-shaped readString in create-offer-request-to-form-values.ts (takes Record | undefined, different bag family) was left as is.

Verified on the new head: web type-check clean, web lint 0 errors, the touched vitest suites green (ksef-connection-config 127 tests across the 4 affected files), and pnpm check:invariants fully green including the hardened drift script.