fix(ksef): resolve secretRef/secret credentials mismatch on connection create

[norbert-kulus-blockydevs]

Summary

• KSeF connection creation always failed with BadRequestException: Invalid KSeF credentials: must include a non-empty \secretRef` stringbecause three layers disagreed on the credentials shape: the FE form sends{authType, secret}, the credentials-shape validator required secretRef`, and the adapter factory assumed an unreachable second credentials indirection that nothing in the system ever populates.
• Renames KsefCredentials.secretRef to secret, updates the shape validator to match, and removes the dead second-resolver call in ksef-adapter.factory.ts - contextNip (needed for the KSeF XML handshake) is now sourced from connection.config.seller.nip, which the setup form already collects.
• No FE changes needed; ksef-setup.schema.ts already sent the correct shape.

Closes #1318

Test plan

• pnpm --filter @openlinker/integrations-ksef type-check - clean
• pnpm --filter @openlinker/integrations-ksef test - all 26 suites / 274 tests pass, including an end-to-end factory-creation test exercising {authType: 'ksef-token', secret: '...'} through createAdapters with no exception thrown
• Confirmed no other package (apps/api, apps/worker) references secretRef/KsefCredentials/KsefResolvedTokenSecret

Back-compat note

Wizard-created connections never worked with the old shape, so there is nothing to migrate for them. However, any hand-provisioned credential row in the old { authType, secretRef } + nested-secret shape (e.g. rows created during early E2E testing) will now fail at adapter boot with KsefConfigException: KSeF credentials missing authType or secret. The fix is to re-save the connection credentials through the wizard (or re-provision the row as { authType, secret }).

[norbert-kulus-blockydevs]

Tech review

Summary

The fix direction is correct: the FE wizard has always sent credentials: { authType, secret }, while the BE shape validator and adapter factory expected { authType, secretRef } plus a second nested credentials lookup, so every wizard-created KSeF connection failed at create with a 400. Collapsing to a single resolver call on connection.credentialsRef and sourcing contextNip from config.seller.nip matches how the wizard actually persists data, and the change stays fully inside the plugin package (no core surface touched). Tests were updated consistently. The single most important gap: the operator-facing setup guide still documents the old secretRef contract, which would reproduce exactly the bug this PR fixes.

Issues

[IMPORTANT] - docs/integrations/ksef/setup-guide.md (lines ~68, ~90, ~121)

Not updated in this PR. The credentials table still lists secretRef as a required field, and the "Obtaining credentials" section calls the platform-assigned reference "the opaque secretRef". After this PR secretRef no longer exists anywhere in the code - anyone hand-provisioning credentials from this guide will hit InvalidCredentialsShapeException ("must include a non-empty secret"), i.e. the exact failure mode being fixed. This guide is referenced from architecture-overview.md as the canonical KSeF doc, so it should ship in the same PR. (Side note: the "Planned addition (C5) - seller profile" callout is also stale now that config.seller exists and is load-bearing for contextNip, worth fixing while in there.)

[SUGGESTION] - apps/web/src/features/connections/components/ksef-setup.schema.ts (line ~31)

Header comment still says the API "assigns a db:<uuid> reference (the BE's opaque secretRef)". That reference is connection.credentialsRef; secretRef is gone. One-line comment fix to avoid resurrecting the dead term.

[SUGGESTION] - libs/integrations/ksef/src/application/factories/ksef-adapter.factory.ts (resolveContextNip)

Validates with nip.trim().length === 0 but returns the untrimmed nip. The FE normalizes NIP to bare digits so this is theoretical today, but returning nip.trim() costs nothing and protects hand-edited config rows (the value lands verbatim in the <ContextNip> XML element).

[SUGGESTION] - back-compat for hand-provisioned credential rows

Wizard-created connections never worked with the old shape, so there is nothing to migrate there. But any hand-provisioned payload in the old { authType, secretRef } + nested-secret shape (e.g. the row used for the June 30 E2E confirmation) will now throw KsefConfigException("KSeF credentials missing authType or secret") at adapter boot with no hint that re-saving credentials fixes it. Worth a line in the PR body / release note; no code change required.

[SUGGESTION] - libs/integrations/ksef/src/application/factories/__tests__/ksef-adapter.factory.spec.ts

The test "should throw when the seller profile is missing" (config { env: 'test' }) now trips resolveContextNip inside resolveAuthMaterial, which runs before resolveSeller - so resolveSeller's own rejection branch (nip present, address malformed) is no longer exercised by this spec. Consider one test with seller: { nip: '1234567890' } and no address to keep the seller-profile guard covered, and optionally rename the existing test to say it covers the missing context NIP.

[SUGGESTION] - config.contextIdentifier divergence

The FE wizard also collects config.contextIdentifier ("display + future scoping" per the FE schema header). After this change the handshake always uses seller.nip as the session context, silently ignoring contextIdentifier even when the operator sets a different value. The factory doc comment justifies the default well; consider either consuming contextIdentifier when present or stating explicitly (in the FE field help / setup guide) that it does not affect authentication.

Verdict

🔄 Approve with changes - the code change is sound and well-tested; fix the setup-guide docs (IMPORTANT) before merge. CI was still pending at review time.

[norbert-kulus-blockydevs]

Review addressed in da0000f - all findings (1 IMPORTANT + 5 SUGGESTIONS) resolved:

• [IMPORTANT] docs/integrations/ksef/setup-guide.md - purged the dead secretRef contract: the Authentication section and "Obtaining credentials" now describe the real { authType, secret } payload persisted behind the connection's credentialsRef (db:<uuid>), and the credentials table lists secret instead of secretRef. Also fixed the stale "Planned addition (C5) - seller profile" callout: the config table now documents the shipped config.seller block (including defaultTaxRate), with a note that seller.nip is also the token-auth session context (<ContextNip>); the outdated limitations row ("config only carries env") was corrected as well.
• [SUGGESTION] ksef-setup.schema.ts header comment - no longer resurrects the secretRef term; the db:<uuid> reference is described as the connection's credentialsRef.
• [SUGGESTION] resolveContextNip - now returns the trimmed NIP, so a hand-edited config row with stray whitespace can't leak into the <ContextNip> XML element.
• [SUGGESTION] back-compat for hand-provisioned rows - documented in the PR body (new "Back-compat note" section): old-shape rows fail at adapter boot with KsefConfigException; re-saving credentials through the wizard fixes it. No code change, as suggested.
• [SUGGESTION] factory spec coverage - renamed the existing test to "should throw when the seller profile is missing (context NIP unresolvable)" and added a new case with seller: { nip, name } but no address, so resolveSeller's own rejection branch stays exercised now that resolveContextNip runs first.
• [SUGGESTION] config.contextIdentifier divergence - went with the documentation option: the wizard field help now states it does not affect authentication (sessions always authenticate in the seller NIP context), the FE schema header says the same, and the setup guide's new session-context note covers it too. Consuming contextIdentifier in the handshake is deliberately left out - changing auth behavior is out of scope for this fix.

Scoped quality gate: @openlinker/integrations-ksef tests (10/10 in the factory spec) + type-check, @openlinker/web type-check, ESLint on all touched files - all clean.

[norbert-kulus-blockydevs]

Tech review (round 2)

Summary

The fix is correct and complete. All three layers (FE schema, credentials-shape validator, adapter factory) now agree on { authType, secret }, the dead second-resolver indirection is gone, and contextNip is sourced from the canonical config.seller.nip. No stale secretRef remains anywhere in source (only doc/comment updates track the rename consistently). Removing the inline KsefResolvedTokenSecret interface from the factory also fixes a latent types-in-separate-files violation. Tests cover the new happy path, both seller-guard rejection branches, and the no-secret-echo invariant. One inconsistency worth fixing before merge.

Issues

[IMPORTANT] - libs/integrations/ksef/src/application/factories/ksef-adapter.factory.ts (resolveSeller vs resolveContextNip)

resolveContextNip trims the NIP with an explicit comment defending against a hand-edited config row with stray whitespace leaking into <ContextNip>. But resolveSeller returns nip: seller.nip untrimmed (it only trims for the emptiness check), so the same hand-edited row would put the whitespace-padded NIP into the FA(3) Podmiot1 block - the exact failure mode the new comment says must not happen, now split across two representations of the same value (session context trimmed, invoice subject not). Return seller.nip.trim() from resolveSeller (and arguably trim name the same way) so both consumers see one canonical value.

[SUGGESTION] - libs/integrations/ksef/src/application/factories/__tests__/ksef-adapter.factory.spec.ts ("malformed address" test)

The test comment explains the rejection must come from resolveSeller's guard rather than resolveContextNip, but the assertion is only toBeInstanceOf(KsefConfigException) - both guards throw that type, so the test would still pass if the ordering claim were wrong. Assert on the message (/seller profile/) to actually pin which guard fired.

[SUGGESTION] - libs/integrations/ksef/src/domain/types/ksef-connection.types.ts (KsefCredentials.secret doc)

For qualified-seal the doc describes secret as "a qualified-seal reference", which quietly reintroduces reference semantics into a field just renamed away from secretRef. Harmless today (the factory throws for qualified-seal until C4), but when C4 lands this wording will invite the same shape drift this PR fixes. Consider "placeholder until C4 defines the seal material shape" instead.

[SUGGESTION] - back-compat note

The PR body documents that hand-provisioned old-shape rows now fail at adapter boot with KsefConfigException. Since wizard-created connections never worked, this is the right call - but consider dropping a one-line entry into docs/lessons.md (contract drift between FE payload, shape validator, and factory went unnoticed because each layer had green unit tests against its own assumed shape; only a cross-layer test caught it).

Verdict

🔄 Approve with changes - fix the NIP trim inconsistency in resolveSeller; the rest is optional.

[norbert-kulus-blockydevs]

Round-2 review addressed in f25f879 - all findings (1 IMPORTANT + 3 SUGGESTIONS) resolved:

• [IMPORTANT] resolveSeller vs resolveContextNip trim inconsistency - resolveSeller now returns seller.nip.trim() and seller.name.trim(), so a hand-edited config row with stray whitespace can no longer split into two representations of the same value (trimmed session context vs untrimmed FA(3) Podmiot1). A comment at the return site ties the two consumers to the one canonical value.
• [SUGGESTION] malformed-address spec assertion - the test now asserts rejects.toThrow(/seller profile/) instead of the bare toBeInstanceOf(KsefConfigException), pinning that the rejection comes from resolveSeller's guard (whose message contains "seller profile") rather than resolveContextNip ("seller NIP"), so the ordering claim in the comment is actually enforced.
• [SUGGESTION] KsefCredentials.secret doc for qualified-seal - reworded per the suggestion: secret is described per auth type (ksef-token = the KSeF authorization token; qualified-seal = a placeholder until C4 defines the seal material shape), no longer calling it "a qualified-seal reference" that would reintroduce reference semantics.
• [SUGGESTION] docs/lessons.md entry - added the cross-layer contract-drift lesson: per-layer green unit tests against each layer's own assumed payload shape hid the FE/validator/factory divergence; the rule requires one cross-layer test (or a single shared fixture) for any wire payload consumed by multiple layers. Scoped to connection credentials/config validators, adapter factories, and FE connection-wizard schemas.

Scoped quality gate: @openlinker/integrations-ksef factory spec (10/10) + package type-check, ESLint on all touched files - all clean.

[piotrswierzy]

Review — Approve

Verified the three-layer reconciliation end-to-end against the branch — the fix is correct and minimal.

What I confirmed

• FE already sends the right shape: ksef-setup.schema.ts → toCreateConnectionInput emits credentials: { authType, secret }. No FE change needed, as claimed.
• Validator now gates a non-empty secret, uses neutral error text, and never echoes the value.
• The removed second resolver (KsefResolvedTokenSecret + the async credentialsResolver.get on secretRef) was genuinely dead — nothing ever populated that nested row. Dropping it loses no behavior.
• contextNip now sourced from config.seller.nip is correct for the <ContextNip> token handshake (seller's own NIP in the self-issuance case); config.contextIdentifier stays display/future-scoping only, and that's documented.
• No lingering secretRef in runtime code — remaining hits are the unrelated webhookSecretRef helper and historical docs/plans/*.
• Back-compat is acknowledged (hand-provisioned old-shape rows fail fast at boot with KsefConfigException).

Cross-reference to my #1284 note: I previously flagged that the KSeF README documented credentials as { token, authType, secretRef }, which was already wrong vs. code. This PR settles the canonical shape as { authType, secret } — please make sure #1284's README (and any other docs) documents { authType, secret }, not secretRef or token.

Suggestions (non-blocking)

1. ksef-adapter.factory.ts — seller.nip is now trimmed + guarded independently in both resolveContextNip and resolveSeller, and resolveAuthMaterial runs before resolveSeller (hence the "(context NIP unresolvable)" test rename). Consider resolving the seller once and deriving contextNip from the validated profile, so resolveSeller stays the single seller-validation seam.
2. The new lessons.md entry recommends one cross-layer test (shared fixture, or an apps/api integration test creating a KSeF connection). This PR aligns the per-layer specs but each still builds its own {authType, secret} literal — the same drift class the lesson warns about. Worth a follow-up so the lesson is enforced, not just documented.
3. docs/plans/implementation-plan-invoicing-fe-redesign.md:138 still says secretRef — minor doc sweep.

Nothing blocking. Good, well-scoped fix with the back-compat impact called out.