Skip to content

feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation#1399

Merged
norbert-kulus-blockydevs merged 3 commits into
epic/1381-erli-allegro-category-catalogfrom
1383-erli-adapter-capability-wiring
Jul 7, 2026
Merged

feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation#1399
norbert-kulus-blockydevs merged 3 commits into
epic/1381-erli-allegro-category-catalogfrom
1383-erli-adapter-capability-wiring

Conversation

@norbert-kulus-blockydevs

Copy link
Copy Markdown
Collaborator

Summary

Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387).

Implements the issue #1383 spec on top of libs/integrations/erli/src/infrastructure/http/allegro-category-catalog-client.ts (#1382):

  • ErliOfferManagerAdapter: adds an optional constructor param allegroCategoryCatalog?: AllegroCategoryCatalogClient. fetchCategories/fetchCategoryParameters are declared as optional instance properties, assigned only in the constructor when the catalog client is provided — the class's static implements clause deliberately does NOT include CategoryBrowser/CategoryParametersReader (ADR-031). This keeps isCategoryBrowser/isCategoryParametersReader per-connection-accurate instead of a static, connection-independent capability.
  • ErliAdapterFactory: createAdapters checks the resolved credentials for a non-empty allegroClientId and allegroClientSecret pair; when both are present it constructs an AllegroCategoryCatalogClient (environment from config.allegroEnvironment ?? 'production') and passes it into the offer-manager constructor. Otherwise undefined is passed.
  • Credentials shape validator: enforces "both or neither" for allegroClientId/allegroClientSecret — rejects a payload carrying exactly one.
  • Config shape validator: allegroEnvironment, when present, must be exactly 'sandbox' or 'production'.
  • Verified (not assumed) that CategoriesCacheService and ListingsController's GET /listings/connections/:connectionId/categories/:categoryId/parameters need no code changes — both already resolve capabilities generically via getCapabilityAdapter + isCategoryBrowser/isCategoryParametersReader guards, so a misconfigured Erli connection behaves exactly like today's "adapter doesn't implement this capability" case (empty array / 422), not a new error path.

A note on connection.supportedCapabilities in the acceptance criteria

The issue's acceptance criteria mentions asserting connection.supportedCapabilities includes/excludes CategoryBrowser/CategoryParametersReader. That field is populated from the static, per-adapterKey AdapterMetadata.supportedCapabilities (see IntegrationsService.resolveAdapterMetadata / ConnectionController.toResponse) — it is never connection-instance-aware. Declaring these capabilities in erliAdapterManifest.supportedCapabilities would advertise them for every Erli connection regardless of Allegro-credential configuration, which is exactly the regression ADR-031 rules out (and would falsely trip the #1367 bulk-wizard gate for unconfigured Erli connections). Per ADR-031's explicit "do NOT add to the static implements/manifest" constraint, this PR keeps erliAdapterManifest.supportedCapabilities unchanged (['OfferManager', 'OrderSource']) and the real per-connection signal is exposed via isCategoryBrowser/isCategoryParametersReader on the resolved adapter instance, and via the real HTTP endpoint's 200-with-data vs 422 behaviour — both of which the integration test exercises directly, alongside confirming supportedCapabilities stays identical (and thus non-regressed) across configured/unconfigured connections.

Test plan

  • pnpm --filter @openlinker/integrations-erli lint — 0 errors
  • pnpm --filter @openlinker/integrations-erli type-check — clean
  • pnpm --filter @openlinker/integrations-erli test — 331/331 passed (new: adapter per-instance wiring specs, factory with/without/partial Allegro-credential specs, credentials/config validator specs)
  • pnpm --filter @openlinker/api type-check — clean
  • pnpm --filter @openlinker/api lint — 0 errors (2 pre-existing unrelated warnings)
  • pnpm --filter @openlinker/api test — 675/675 passed
  • New integration test apps/api/test/integration/listings/erli-category-catalog.int-spec.ts — 3/3 passed against real Postgres/Testcontainers, exercising the REAL ErliAdapterFactory + ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through the production adapter-resolution seam and the real HTTP endpoint (configured / unconfigured / partially-configured connections)
  • apps/web bulk-edit-modal test ([BUG] Frontend — bulk offer wizard hides category-parameter step for Allegro (required "Stan" unsettable) #1367 gate) — 8/8 passed, unmodified
  • node scripts/check-cross-context-imports.mjs — clean
  • node scripts/check-service-interfaces.mjs — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01RpsVFTpKZ1HoowEUKzbnWu

… credential/config validation (#1383)

ErliOfferManagerAdapter now wires fetchCategories/fetchCategoryParameters as
optional instance properties (never a static implements clause) so
isCategoryBrowser/isCategoryParametersReader reflect whether a specific Erli
connection has configured a valid Allegro app credential pair, per ADR-031.
ErliAdapterFactory constructs the shared AllegroCategoryCatalogClient only
when both allegroClientId and allegroClientSecret resolve to non-empty
strings, resolving config.allegroEnvironment (default 'production').

The credentials shape validator now enforces "both or neither" for the
Allegro credential pair, and the config shape validator restricts
allegroEnvironment to 'sandbox' | 'production'.

Verified CategoriesCacheService and ListingsController need no changes: both
already resolve capabilities generically via getCapabilityAdapter + is* type
guards, so a misconfigured Erli connection behaves like today's
"adapter doesn't implement this capability" case.

Added an integration test exercising the real ErliAdapterFactory +
ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through
the production adapter-resolution seam and the real HTTP endpoint, covering
configured / unconfigured / partially-configured connections. Confirmed the
existing #1367 bulk-wizard capability-gate test passes unmodified.

Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387).

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…ible signal

connection.supportedCapabilities turned out to be a static, per-adapterKey
manifest value rather than computed per-connection-instance, so it can't
distinguish an Erli connection with configured Allegro app credentials from
one without. Add a non-secret ErliConnectionConfig.allegroCategoryAccessEnabled
boolean for the frontend to read instead (see ADR-031 "Correction"); the
write path that sets allegroClientId/allegroClientSecret must set/clear this
flag atomically (tracked in #1384's scope).

Also fixes stale ADR-030 references (renumbered to ADR-031 to avoid a
collision with #1307) in allegro-category-catalog-client.ts and
erli-connection.types.ts.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
@norbert-kulus-blockydevs

Copy link
Copy Markdown
Collaborator Author

Follow-up commit pushed (07bb2b38): during doc review we found connection.supportedCapabilities is a static, per-adapterKey manifest value — not computed per connection instance — so it can't distinguish a configured-vs-unconfigured Erli connection. Added a new non-secret ErliConnectionConfig.allegroCategoryAccessEnabled?: boolean as the actual FE-visible signal instead (validated in the config shape validator, tests added). This doesn't change any of the capability-wiring logic already reviewed here — ErliOfferManagerAdapter/ErliAdapterFactory are untouched by this commit. Also fixed stale ADR-030 references to ADR-031 (renumbered to avoid a collision with #1307). ADR-031 and the implementation plan have been updated accordingly in PR #1380. Quality gate re-verified green (335/335 tests, lint, type-check).

@piotrswierzy piotrswierzy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/pr-review — feat(erli): per-connection Allegro category-catalog capability + validators (#1383)

This is the wiring sub-task I flagged during the #1380 plan review, and it lands the design correctly. The per-connection capability model is implemented exactly as ADR-031 prescribes, verified end-to-end. Backend is correct and mergeable. Two items: one concrete perf regression worth fixing here or as a fast-follow, and one forward-gate risk the FE sub-task (#1384) must not walk into.

Verified correct (the load-bearing checks)

  • Sub-capability resolution is coherent — both consumers resolve by the base OfferManager capability, then narrow: ListingsController:433 getCapabilityAdapter<OfferManagerPort>(…'OfferManager'):438 isCategoryParametersReader, and CategoriesCacheService:53/56 the same. Since CategoryBrowser/CategoryParametersReader are sub-capabilities (never top-level registry capabilities), Erli's OfferManager-only manifest resolves fine and the guard reads the per-instance property. Keeping them out of erliAdapterManifest.supportedCapabilities isn't just acceptable — it's required; adding a sub-capability to the registry manifest would be a category error. ✅
  • Per-instance wiring is textbook ADR-031fetchCategories?/fetchCategoryParameters? are optional instance properties (not in implements), assigned only when the catalog client is present; the is* guards are structural (typeof … === 'function'), so unconfigured → undefined → guard false. Per-connection accurate. ✅
  • Validators are both-or-neither + enum, defended twice — the credentials validator rejects an incomplete allegroClientId/allegroClientSecret pair at write time ("must include both … or neither"), the config validator constrains allegroEnvironment to sandbox|production, and the factory re-checks both (.trim()) at construction. Write-time + construction-time defense in depth. ✅
  • Tests — the int-spec exercises all three credential states (configured → guards true + 200 data; unconfigured → guards false + 422; partial pair → guards false) through the real ErliAdapterFactory + adapter + client via the production resolution seam and the real HTTP endpoint. That's a proper vertical slice, not a mock dance. No any. ADR-031 boundary intact.

🟡 IMPORTANT — forward note for #1384 (FE): don't gate the Erli wizard on supportedCapabilities

Your PR description nails why connection.supportedCapabilities (static, per-adapterKey manifest) can't reflect the per-connection capability — correct. But there's an asymmetry the FE sub-task has to handle deliberately: Allegro (#1370) advertises its sub-capabilities statically in its manifest, so the existing FE bulk-wizard gate (#1367) reads supportedCapabilities.includes('CategoryBrowser'). Erli, by design, never will. So if #1384 reuses that same gate for the Erli offer wizard, a configured Erli connection will silently not show the category/parameters steps — the feature ships "wired but invisible." #1384 must drive the Erli gate off a genuine per-connection signal (probe the parameters endpoint: 200 → show, 422 → plain-text fallback; or a per-connection effective-capability read) — not the static manifest. Flagging here so the seam is on record before the FE PR; nothing to change in this PR.

🟡 IMPORTANT — token cache is per-request (realizes my #1388 forward note)

buildAllegroCategoryCatalog constructs a fresh AllegroCategoryCatalogClient on every createAdapters call, and adapters are built fresh per getCapabilityAdapter call (no memoization). So the client's per-instance app-token cache — the single-flight + expiry-aware logic added in #1388 — lives exactly one request and delivers zero cross-request benefit. Impact is asymmetric:

  • fetchCategories is shielded by CategoriesCacheService's 24h DB cache → underlying client (and its OAuth round-trip) is hit only on cache-miss. Low.
  • fetchCategoryParameters is uncached at the controller (pre-existing gap shared with Allegro) → every parameters request pays a full extra client_credentials OAuth round-trip (2 upstream calls instead of 1). That's a latency + Allegro-rate-limit cost, and a regression vs Allegro's shared AllegroConnectionTokenState.

The fix is cheap and the seam is already here: createAdapters already receives host.cache (CachePort param). Hoist the app-token cache to it (or a module-scope map keyed by clientId) so the token survives across per-request adapter instances. Fine as a fast-follow if you'd rather not widen this PR, but it should be tracked, not lost.

🟢 Minor

  • The two plugin validators are named *.adapter.ts and implement the host *ShapeValidatorPorts — consistent with the existing Allegro/PrestaShop validator precedent. Good.
  • Nice touch re-checking the credential pair at construction even though the write-time validator already guards it — belt-and-suspenders against a connection whose credentials were written before the validator existed.

Verdict

🔄 Approve with changes — the backend design is correct and directly implements the ADR-031 per-instance model I asked for in the #1380 review; it's mergeable onto the epic branch as-is (mergeable_state: clean). Priority items: (1) carry the FE-gating note into #1384 so a configured Erli connection actually shows the category UI, and (2) hoist the app-token cache to host.cache (here or a tracked fast-follow) so category-parameter browsing doesn't pay an OAuth round-trip per request.

@norbert-kulus-blockydevs norbert-kulus-blockydevs left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consolidated tech-review + pr-review (two independent passes)

Two independent automated reviews were run against the current HEAD (07bb2b38), each verifying the actual code (not just the PR description) and running the quality gate live in an isolated checkout. Both converge on the same verdict.

On Piotr's prior review

Confirmed via the GitHub API that his review's commit_id matches the current HEAD — it is not stale, it already reflects the allegroCategoryAccessEnabled fix commit. His two IMPORTANT items remain open by design (deferred, not blockers for this PR):

  1. The bulk-wizard's supportedCapabilities gate won't surface the Erli category UI even when configured — informational note for #1384, nothing to fix here.
  2. AllegroCategoryCatalogClient's token cache is per-request (no adapter-instance caching in IIntegrationsService.getCapabilityAdapter anywhere in this codebase), so every fetchCategoryParameters call pays a full OAuth round-trip. Real, confirmed independently, worth a tracked fast-follow via host.cache.

Independently verified claims

  • connection.supportedCapabilities is genuinely static per-adapterKey — confirmed by reading connection.controller.ts's toResponse, which resolves it from resolveAdapterMetadata({ platformType, adapterKey }), never per-connection-instance. The allegroCategoryAccessEnabled config flag is the correct, narrow, plugin-local fix.
  • ADR-031's static-implements constraint holds in codeErliOfferManagerAdapter's implements clause has no CategoryBrowser/CategoryParametersReader; fetchCategories?/fetchCategoryParameters? are optional instance properties assigned only in the constructor when allegroCategoryCatalog is provided. isCategoryBrowser/isCategoryParametersReader are structural, so this is genuinely per-instance.
  • No CORE changes; hexagonal boundary respected — everything confined to libs/integrations/erli + one new int-spec.

Quality gate — run live in isolated worktrees by both reviews (not trusted from the PR description)

  • @openlinker/integrations-erli: lint clean, type-check clean, 335/335 tests passed.
  • @openlinker/api: lint/type-check clean; new erli-category-catalog.int-spec.ts 3/3 passed against real Testcontainers Postgres, genuinely exercising the real getCapabilityAdapter seam and HTTP endpoint.
  • bulk-edit-modal.test.tsx (#1367 gate), unmodified file, run directly — passed (one pass ran the full FE suite: 2029/2029).
  • Repo invariants (check-cross-context-imports, check-service-interfaces, check-migration-timestamps) all green.

New findings from this pass (neither blocking)

  1. ErliAdapterFactory.createAdapters resolves credentials twice per call (createHttpClient resolves for apiKey, then buildAllegroCategoryCatalog resolves again for the Allegro pair) — a second DB round-trip + decrypt on every adapter construction. Same class of issue as Piotr's token-cache point; worth fixing together, here or as a fast-follow.
  2. allegroCategoryAccessEnabled has zero producers today — by design, scoped to #1384 per the commit message and ADR-031's "Correction" section. Flagging explicitly as a hard dependency to verify when #1384 lands: the write path there MUST set/clear this flag atomically alongside allegroClientId/allegroClientSecret, or the two state sources can drift.
  3. (Minor, non-blocking) the epic branch hasn't picked up ADR-031 from main yet (still on PR #1380) — worth a sync before/shortly after merge, not a reason to hold this PR.

Verdict

Approve with changes. No blocking issues on either independent pass. Mergeable onto the epic branch as-is; the token-cache hoist + double-credential-resolve are good candidates to bundle into one fast-follow rather than reopening this PR.

…credential resolve (#1399 review)

AllegroCategoryCatalogClient built a fresh in-memory token cache per adapter
instance, but ErliAdapterFactory builds a fresh instance per
getCapabilityAdapter call — so fetchCategoryParameters paid a full
client_credentials OAuth round-trip on every request. The client now
persists the acquired token in the optional CachePort (host.cache), keyed by
clientId, with a TTL matching the token's remaining lifetime, so it survives
across per-request instances and processes.

Also collapses ErliAdapterFactory.createAdapters's two credentialsResolver.get
calls (one for apiKey, one for the Allegro pair) into one shared resolve.

Addresses both IMPORTANT findings from the PR #1399 review round (Piotr's
token-cache note and the self-review's double-resolve finding).

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
@norbert-kulus-blockydevs

Copy link
Copy Markdown
Collaborator Author

Review follow-up (commit 771c1279)

Addressed both open IMPORTANT findings from the review round (Piotr's pass + the consolidated tech-review/pr-review pass). No BLOCKER or SUGGESTION items were left on the table — everything actionable in this PR's scope has been fixed; the remaining items were explicitly informational/deferred by design and are called out below.

Fixed

  1. Token cache was per-request, not shared (Piotr, IMPORTANT)AllegroCategoryCatalogClient cached its client_credentials app token only in-memory, but ErliAdapterFactory builds a fresh client per getCapabilityAdapter call, so every fetchCategoryParameters request paid a full extra OAuth round-trip. The client now accepts an optional CachePort (host.cache) and persists the acquired token there, keyed by clientId with a TTL matching the token's remaining lifetime, so it survives across per-request instances and across processes. ErliAdapterFactory threads its existing cache param through to the client. Falls back to the pre-existing in-memory-only behaviour when no cache is supplied (backward compatible). New unit tests cover: write-through on acquire, reuse across two separate client instances sharing one CachePort, and re-acquisition when the cached entry is expired.

  2. createAdapters resolved credentials twice per call (self-review, IMPORTANT)createHttpClient resolved credentials for apiKey, then a second resolveCredentials call resolved again for the Allegro pair — a second credentialsRef DB round-trip + decrypt on every adapter construction. Collapsed into a single resolve, shared by both the Erli http client and the optional Allegro category-catalog client via a new private buildHttpClient helper. New test asserts credentialsResolver.get is called exactly once per createAdapters call.

Not changed (informational / explicitly deferred by design, confirmed still correct)

Quality gate (re-verified on HEAD 771c1279)

  • @openlinker/integrations-erli: lint clean, type-check clean, 340/340 tests passed (5 new: cache-hoist behaviour + single-resolve).
  • @openlinker/api: lint clean (2 pre-existing unrelated warnings), type-check clean.
  • erli-category-catalog.int-spec.ts3/3 passed against real Testcontainers Postgres, exercising the real adapter-resolution seam end-to-end.
  • check-cross-context-imports / check-service-interfaces — clean.

@norbert-kulus-blockydevs norbert-kulus-blockydevs merged commit a02a0c7 into epic/1381-erli-allegro-category-catalog Jul 7, 2026
norbert-kulus-blockydevs added a commit that referenced this pull request Jul 7, 2026
… credential/config validation (#1399)

* feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation (#1383)

ErliOfferManagerAdapter now wires fetchCategories/fetchCategoryParameters as
optional instance properties (never a static implements clause) so
isCategoryBrowser/isCategoryParametersReader reflect whether a specific Erli
connection has configured a valid Allegro app credential pair, per ADR-031.
ErliAdapterFactory constructs the shared AllegroCategoryCatalogClient only
when both allegroClientId and allegroClientSecret resolve to non-empty
strings, resolving config.allegroEnvironment (default 'production').

The credentials shape validator now enforces "both or neither" for the
Allegro credential pair, and the config shape validator restricts
allegroEnvironment to 'sandbox' | 'production'.

Verified CategoriesCacheService and ListingsController need no changes: both
already resolve capabilities generically via getCapabilityAdapter + is* type
guards, so a misconfigured Erli connection behaves like today's
"adapter doesn't implement this capability" case.

Added an integration test exercising the real ErliAdapterFactory +
ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through
the production adapter-resolution seam and the real HTTP endpoint, covering
configured / unconfigured / partially-configured connections. Confirmed the
existing #1367 bulk-wizard capability-gate test passes unmodified.

Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387).

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): add allegroCategoryAccessEnabled config flag as the FE-visible signal

connection.supportedCapabilities turned out to be a static, per-adapterKey
manifest value rather than computed per-connection-instance, so it can't
distinguish an Erli connection with configured Allegro app credentials from
one without. Add a non-secret ErliConnectionConfig.allegroCategoryAccessEnabled
boolean for the frontend to read instead (see ADR-031 "Correction"); the
write path that sets allegroClientId/allegroClientSecret must set/clear this
flag atomically (tracked in #1384's scope).

Also fixes stale ADR-030 references (renumbered to ADR-031 to avoid a
collision with #1307) in allegro-category-catalog-client.ts and
erli-connection.types.ts.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): hoist Allegro category-token cache to host.cache + dedupe credential resolve (#1399 review)

AllegroCategoryCatalogClient built a fresh in-memory token cache per adapter
instance, but ErliAdapterFactory builds a fresh instance per
getCapabilityAdapter call — so fetchCategoryParameters paid a full
client_credentials OAuth round-trip on every request. The client now
persists the acquired token in the optional CachePort (host.cache), keyed by
clientId, with a TTL matching the token's remaining lifetime, so it survives
across per-request instances and processes.

Also collapses ErliAdapterFactory.createAdapters's two credentialsResolver.get
calls (one for apiKey, one for the Allegro pair) into one shared resolve.

Addresses both IMPORTANT findings from the PR #1399 review round (Piotr's
token-cache note and the self-review's double-resolve finding).

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
norbert-kulus-blockydevs added a commit that referenced this pull request Jul 8, 2026
… credential/config validation (#1399)

* feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation (#1383)

ErliOfferManagerAdapter now wires fetchCategories/fetchCategoryParameters as
optional instance properties (never a static implements clause) so
isCategoryBrowser/isCategoryParametersReader reflect whether a specific Erli
connection has configured a valid Allegro app credential pair, per ADR-031.
ErliAdapterFactory constructs the shared AllegroCategoryCatalogClient only
when both allegroClientId and allegroClientSecret resolve to non-empty
strings, resolving config.allegroEnvironment (default 'production').

The credentials shape validator now enforces "both or neither" for the
Allegro credential pair, and the config shape validator restricts
allegroEnvironment to 'sandbox' | 'production'.

Verified CategoriesCacheService and ListingsController need no changes: both
already resolve capabilities generically via getCapabilityAdapter + is* type
guards, so a misconfigured Erli connection behaves like today's
"adapter doesn't implement this capability" case.

Added an integration test exercising the real ErliAdapterFactory +
ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through
the production adapter-resolution seam and the real HTTP endpoint, covering
configured / unconfigured / partially-configured connections. Confirmed the
existing #1367 bulk-wizard capability-gate test passes unmodified.

Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387).

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): add allegroCategoryAccessEnabled config flag as the FE-visible signal

connection.supportedCapabilities turned out to be a static, per-adapterKey
manifest value rather than computed per-connection-instance, so it can't
distinguish an Erli connection with configured Allegro app credentials from
one without. Add a non-secret ErliConnectionConfig.allegroCategoryAccessEnabled
boolean for the frontend to read instead (see ADR-031 "Correction"); the
write path that sets allegroClientId/allegroClientSecret must set/clear this
flag atomically (tracked in #1384's scope).

Also fixes stale ADR-030 references (renumbered to ADR-031 to avoid a
collision with #1307) in allegro-category-catalog-client.ts and
erli-connection.types.ts.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): hoist Allegro category-token cache to host.cache + dedupe credential resolve (#1399 review)

AllegroCategoryCatalogClient built a fresh in-memory token cache per adapter
instance, but ErliAdapterFactory builds a fresh instance per
getCapabilityAdapter call — so fetchCategoryParameters paid a full
client_credentials OAuth round-trip on every request. The client now
persists the acquired token in the optional CachePort (host.cache), keyed by
clientId, with a TTL matching the token's remaining lifetime, so it survives
across per-request instances and processes.

Also collapses ErliAdapterFactory.createAdapters's two credentialsResolver.get
calls (one for apiKey, one for the Allegro pair) into one shared resolve.

Addresses both IMPORTANT findings from the PR #1399 review round (Piotr's
token-cache note and the self-review's double-resolve finding).

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
piotrswierzy pushed a commit that referenced this pull request Jul 8, 2026
…t a required Allegro connection (#1407)

* feat(erli): Erli-owned Allegro client_credentials category-catalog client (#1388)

* feat(erli): add Erli-owned Allegro client_credentials category-catalog client

Adds AllegroCategoryCatalogClient, a self-contained HTTP client that lets
an Erli connection browse Allegro's public /sale/categories and
/sale/categories/{id}/parameters catalog via an Allegro app's
grant_type=client_credentials token, with no dependency on
@openlinker/integrations-allegro (per ADR-030) and no requirement for the
operator to own a real Allegro seller connection.

Extends ErliConnectionConfig with an optional allegroEnvironment field and
ErliCredentials with optional allegroClientId/allegroClientSecret fields
to carry the Allegro app credentials alongside Erli's own apiKey.

Part of #1381, sub-task 1 of 3 (see #1383, #1384).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RpsVFTpKZ1HoowEUKzbnWu
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): address PR #1388 review findings on AllegroCategoryCatalogClient

- Extract inline wire-shape types to allegro-category-catalog-client.types.ts,
  matching the erli-http-client.ts / erli-http-client.types.ts split.
- Add a single-flight guard around token acquisition so two concurrent
  fetchCategories/fetchCategoryParameters calls on a cold cache issue one
  grant_type=client_credentials request, not two.
- Treat a token response missing expires_in as already-expired instead of
  caching it indefinitely.
- Add tests for the 401/403 branch on a data call (categories/parameters),
  distinct from the token-endpoint rejection branch already covered.
- Add a cross-plugin mapper parity test suite that runs Allegro's real
  sandbox category-parameters fixture through this client's copy of
  toNeutralCategoryParameter, mirroring assertions from
  allegro-category-parameter.mapper.spec.ts, with cross-referencing comments
  in both spec files so a future mapper change prompts updating both.
- Replace the inline 'sandbox' | 'production' union with an as const +
  runtime-array AllegroCatalogEnvironment type in erli-connection.types.ts,
  matching AllegroConnectionConfig's own convention.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation (#1399)

* feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation (#1383)

ErliOfferManagerAdapter now wires fetchCategories/fetchCategoryParameters as
optional instance properties (never a static implements clause) so
isCategoryBrowser/isCategoryParametersReader reflect whether a specific Erli
connection has configured a valid Allegro app credential pair, per ADR-031.
ErliAdapterFactory constructs the shared AllegroCategoryCatalogClient only
when both allegroClientId and allegroClientSecret resolve to non-empty
strings, resolving config.allegroEnvironment (default 'production').

The credentials shape validator now enforces "both or neither" for the
Allegro credential pair, and the config shape validator restricts
allegroEnvironment to 'sandbox' | 'production'.

Verified CategoriesCacheService and ListingsController need no changes: both
already resolve capabilities generically via getCapabilityAdapter + is* type
guards, so a misconfigured Erli connection behaves like today's
"adapter doesn't implement this capability" case.

Added an integration test exercising the real ErliAdapterFactory +
ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through
the production adapter-resolution seam and the real HTTP endpoint, covering
configured / unconfigured / partially-configured connections. Confirmed the
existing #1367 bulk-wizard capability-gate test passes unmodified.

Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387).

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): add allegroCategoryAccessEnabled config flag as the FE-visible signal

connection.supportedCapabilities turned out to be a static, per-adapterKey
manifest value rather than computed per-connection-instance, so it can't
distinguish an Erli connection with configured Allegro app credentials from
one without. Add a non-secret ErliConnectionConfig.allegroCategoryAccessEnabled
boolean for the frontend to read instead (see ADR-031 "Correction"); the
write path that sets allegroClientId/allegroClientSecret must set/clear this
flag atomically (tracked in #1384's scope).

Also fixes stale ADR-030 references (renumbered to ADR-031 to avoid a
collision with #1307) in allegro-category-catalog-client.ts and
erli-connection.types.ts.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli): hoist Allegro category-token cache to host.cache + dedupe credential resolve (#1399 review)

AllegroCategoryCatalogClient built a fresh in-memory token cache per adapter
instance, but ErliAdapterFactory builds a fresh instance per
getCapabilityAdapter call — so fetchCategoryParameters paid a full
client_credentials OAuth round-trip on every request. The client now
persists the acquired token in the optional CachePort (host.cache), keyed by
clientId, with a TTL matching the token's remaining lifetime, so it survives
across per-request instances and processes.

Also collapses ErliAdapterFactory.createAdapters's two credentialsResolver.get
calls (one for apiKey, one for the Allegro pair) into one shared resolve.

Addresses both IMPORTANT findings from the PR #1399 review round (Piotr's
token-cache note and the self-review's double-resolve finding).

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* feat(erli,web): Allegro credentials checkbox + offer wizard category/parameters steps (#1401)

* feat(erli,web): checkbox-reveal Allegro credentials panel + offer wizard category/parameters steps

Adds the operator-facing half of the Erli-owned Allegro category-catalog
feature (#1384, part of epic #1381, depends on #1382/#1383):

- ErliCredentialsPanel gains a "Browse Allegro categories when creating
  Erli offers" checkbox that reveals Client ID / Client Secret fields
  (masked, show/hide toggle). Saving sequences two existing mutations from
  one click: useUpdateConnectionCredentialsMutation (credentials pair,
  merged with apiKey) then useUpdateConnectionMutation (config patch for
  allegroCategoryAccessEnabled) — credentials first so a failure never
  leaves the flag advertising access the backend can't serve.

- ErliCreateOfferWizard renders the reused Allegro CategoryPicker +
  CategoryParametersStep (fed by the existing useCategoryParametersQuery)
  as dedicated Category / Category-parameters steps when
  connection.config.allegroCategoryAccessEnabled is true — per ADR-031's
  correction, this per-connection-instance config flag is the FE gating
  signal, not the static, per-adapterKey connection.supportedCapabilities.
  Falls back to today's plain-text field + a link to the connection's edit
  page otherwise. Stepper labels and needsProductParameters become
  capability-conditional; erliCreateOfferSchema gains a parameters slice
  serialized into overrides.parameters on submit (no BE change needed).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RpsVFTpKZ1HoowEUKzbnWu
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli,web): address PR #1401 review findings

- Gate `canSubmit` in ErliCredentialsPanel on `allegroEnabled` so
  unchecking the Allegro-access box after typing (but not saving)
  Client ID/Secret disables Save instead of showing a false-positive
  "Credentials saved" toast while discarding the typed secret.
- Drop the checkbox's invalid inline `accentColor: var(--accent)`
  (token doesn't exist) — `index.css` already applies
  `accent-color: var(--accent-primary)` globally.
- Restore category-parameter values on Erli offer retry by reusing
  the Allegro retry mapper's `readParameters` heuristic (exported)
  instead of always prefilling an empty `parameters` object — both
  platforms persist the same neutral `overrides.parameters` shape.

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* fix(erli,connections): merge credential rotation + enforce category pick (#1401 second review)

- ConnectionService.updateCredentials now merges the submitted payload onto
  the existing stored credentialsJson instead of replacing it wholesale, so
  rotating the plain Erli apiKey no longer silently wipes a previously
  configured allegroClientId/allegroClientSecret pair (and enabling Allegro
  access without touching apiKey no longer fails shape validation).
- ErliCreateOfferWizard now blocks Next on the Category step until a
  category is actually selected, mirroring AllegroCreateOfferWizard's
  required categoryId field, instead of silently falling through to
  Category-parameters/Review with an empty category.
- Carried over the approved mockup's helper copy under the Allegro Client ID
  / Client Secret fields and restored the checkbox's original hint text.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

* test(erli,docs): cover mutation-sequencing failure paths + close ADR-031 documentation gaps (#1401 third review)

- erli-credentials-panel.test.tsx: add regression tests asserting the config
  patch never fires when the credentials write rejects, and that a config-patch
  rejection after a successful credentials write leaves the flag/fields intact
  with a retryable inline error - the two claims the PR description makes about
  "the trickiest part of this issue" were previously unverified by the suite.
- ADR-031: add a second Correction noting the sequencing is two FE-orchestrated
  mutations, not a single backend write, since no endpoint accepts both the
  credential pair and the config flag together; also record the wizard's
  3-step-vs-5-step topology and the deferred credential-verification indicator
  as explicit, confirmed scope cuts rather than implicit deviations from the
  approved mockup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* feat(erli): reuse an existing Allegro connection's credentials for category access (#1387) (#1405)

Extends the #1384 Erli credentials panel with a reuse-vs-manual choice: when
the operator already has an Allegro connection, its clientId/clientSecret can
be copied server-side into the Erli connection instead of registering a
second Allegro app. The raw clientSecret never round-trips through the
browser.

The credential-copy logic is dispatched through a new, platform-neutral
ConnectionCredentialsRewriterPort + registry (mirroring the existing
ConnectionConfigShapeValidatorPort/ConnectionCredentialsShapeValidatorPort
pair) so the generic ConnectionService.updateCredentials stays unaware that
Allegro or Erli exist. The Erli-specific resolution lives entirely in
ErliAllegroCredentialsRewriterAdapter, registered from a companion
ErliCredentialsRewriterModule (mirrors ErliWebhookProvisioningModule) since
it needs ConnectionPort, which is intentionally outside the plugin-neutral
HostServices bag.

Closes #1387

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>

---------

Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
Signed-off-by: Norbert Kulus <42zeroo@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants