Skip to content

CEXT-6160: store and expose Commerce system config during app associa…#460

Draft
vinayrao2000 wants to merge 3 commits into
mainfrom
CEXT-6160-commerce-system-config-on-association
Draft

CEXT-6160: store and expose Commerce system config during app associa…#460
vinayrao2000 wants to merge 3 commits into
mainfrom
CEXT-6160-commerce-system-config-on-association

Conversation

@vinayrao2000
Copy link
Copy Markdown
Contributor

Description

Adds two new routes to the existing installationRuntimeAction in aio-commerce-lib-app:

  • POST /installation/association — stores AIO_COMMERCE_BASE_URL and AIO_COMMERCE_ENV
    as OpenWhisk package parameters when an app is associated with a Commerce instance
  • DELETE /installation/association — removes those parameters on unassociation

Adds a new @adobe/aio-commerce-lib-app/runtime export with getCommerceSystemConfig(params),
a synchronous helper that reads the stored config from any runtime action's params.

Related Issue

CEXT-6160

Motivation and Context

Every Commerce app that needs to call the Commerce API was reimplementing the same logic to
store and retrieve the Commerce Base URL. This also stored it at install time rather than
keeping it in sync with the association lifecycle.

OpenWhisk package parameters are injected automatically into every action's params at
invocation time — storing the config there means runtime actions receive the Commerce URL
and env with zero extra code, and it is always in sync with the current association state.

How Has This Been Tested?

  • Unit tests added for POST /association and DELETE /association route handlers
  • Unit tests added for getCommerceSystemConfig covering all cases (present, null, paas, saas)
  • All 741 existing tests pass
  • End-to-end tested manually via App Management UI and DevTools:
  • Unit tests added for POST /association and DELETE /association route handlers
  • Unit tests added for getCommerceSystemConfig covering all cases (present, null, paas, saas)
  • All 741 existing tests pass
  • End-to-end tested manually via App Management UI and DevTools:
    • POST /installation/association returns 200 { baseUrl, env } after association
    • DELETE /installation/association returns 204 after unassociationd

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 20, 2026

⚠️ No Changeset found

Latest commit: 9f2d51e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added with-changeset The PR contains a Changeset file. pkg: aio-commerce-lib-app Includes changes in `packages/aio-commerce-lib-app` labels May 20, 2026
Comment on lines +66 to +75
"./runtime": {
"import": {
"types": "./dist/es/runtime.d.mts",
"default": "./dist/es/runtime.mjs"
},
"require": {
"types": "./dist/cjs/runtime.d.cts",
"default": "./dist/cjs/runtime.cjs"
}
},
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.

I would not export this from /runtime, just from the root entrypoint, so instead of:

import { ... } from "@adobe/aio-commerce-lib-app/runtime"

it would be:

import { ... } from "@adobe/aio-commerce-lib-app"

name: packageName,
package: { parameters: [...kept, ...added] },
});
}
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.

Probably such an approach would work. But I thought the idea was to store this configuration by using logic from aio-commerce-lib-config. So it can be retrieved in runtime actions from the configuration.

Copy link
Copy Markdown
Member

@obarcelonap obarcelonap left a comment

Choose a reason for hiding this comment

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

Thanks for writing a spec for these changes, I only reviewed this part.
Normally you should first pr only with the spec so you can get feedback and then once approved and merged you can go ahead with the implementation.
Spec workflow is defined here

## Workflow
A feature goes through two pull requests:
1. **Spec PR** — contains only the spec file. Reviewers align on the design
before any implementation begins. Once merged, the spec is considered
approved and the feature is ready to implement.
2. **Implementation PR** — contains the code. References the approved spec.
Once merged, the spec status is updated to _Implemented_.
This separation ensures design decisions are made explicitly and are not
shaped retroactively by implementation details.

**Non-goals:**

- Storing any Commerce system configuration beyond Base URL and deployment type.
- Replacing the `AIO_COMMERCE_API_BASE_URL` and `AIO_COMMERCE_API_FLAVOR` package params set
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't be better to use these params names? Otherwise the params resolver won't work and might be a bit confusing when to use what.
For instance, how the dev will decide when to use AIO_COMMERCE_API_BASE_URL vs AIO_COMMERCE_BASE_URL

import { getCommerceSystemConfig } from "@adobe/aio-commerce-lib-app/runtime";

export async function main(params) {
const config = getCommerceSystemConfig(params);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While this kind of helper function is needed, since the main purpose of these values is to be able to do http requests to the commerce instance, I think we need also to use it in some way in http client param resolution

export function resolveCommerceHttpClientParams(
params: Record<string, unknown>,
options: ResolveCommerceHttpClientParamsOptions = {},
): CommerceHttpClientParams {
if (allNonEmpty(params, ["AIO_COMMERCE_API_BASE_URL"])) {
const baseUrl = String(params.AIO_COMMERCE_API_BASE_URL);
const flavor = isFlavor(params.AIO_COMMERCE_API_FLAVOR)
? params.AIO_COMMERCE_API_FLAVOR
: resolveCommerceFlavorFromApiUrl(baseUrl);

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.

But then we would be coupling lib-app to lib-api? Unless I am not understanding what to mean not sure if this is a good idea.

- `AIO_COMMERCE_BASE_URL` — the Commerce API base URL
- `AIO_COMMERCE_ENV` — the deployment type (`"saas"` or `"paas"`)

OpenWhisk injects all package parameters into every action's `params` object at invocation time,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am afraid that this is coupling the implementation too much to OpenWhisk. Have you aligned this approach with the appbuilder team?

`CommerceSystemConfig`, or `null` if either value is absent. The function is synchronous — no
async call is needed because the values are already present in `params`.

### Changes to `commerce-app-management`
Copy link
Copy Markdown
Member

@obarcelonap obarcelonap May 21, 2026

Choose a reason for hiding this comment

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

I believe we should not have commerce-app-management implementation details here. Maybe you can just describe broadly how a client of app-management api is impacted and the expected flows.

* Returns the Commerce system configuration populated during association,
* or null if the app is not currently associated with a Commerce instance.
*/
export function getCommerceSystemConfig(
Copy link
Copy Markdown
Member

@obarcelonap obarcelonap May 21, 2026

Choose a reason for hiding this comment

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

Not sure if the concept system config is too broad. I'd prefer something like getAssossociationDetails or getAssociatedCommerceInstance or similar

Comment on lines +29 to +32
The SDK is already passed `commerceBaseUrl` and `commerceEnv` at every association and
unassociation event — the data apps need is already flowing through the system. The missing piece
is persistence tied to the association lifecycle and a standard way for runtime actions to consume
it.
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.

In order to implement some of the other requested features (e.g. helpers to publish events into the Event Ingress API) we'll also need to store data about the project/workspace we're associating with.


**Goals:**

- Store the Commerce Base URL and deployment type (PaaS/SaaS) as OpenWhisk package parameters
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.

I am not very much aware of how package parameters work, but given that we're applying this "save" under the context of the app-management package, does this work for actions that are set under other packages? The app-management package is only ours, but developers have their own.

Comment on lines +58 to +64
A runtime action can read these directly:

```js
export async function main(params) {
const baseUrl = params.AIO_COMMERCE_BASE_URL;
const env = params.AIO_COMMERCE_ENV;
}
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.

While having in params is somehow standard, it also forces you to move the params object everywhere you need this data. So if you are for example in a 4-level deep function call hierachy and you need this data only at that level, you will be forced to drill it down from the first layer down to the fourth.

Business configuration would allow retrieving this without coupling to params

import { getCommerceSystemConfig } from "@adobe/aio-commerce-lib-app/runtime";

export async function main(params) {
const config = getCommerceSystemConfig(params);
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.

But then we would be coupling lib-app to lib-api? Unless I am not understanding what to mean not sure if this is a good idea.

Comment on lines +205 to +207
- **Association before this feature existed.** Apps associated before `POST /association` was
introduced have no stored parameters. `getCommerceSystemConfig` returns `null`. Apps must handle
this explicitly.
Copy link
Copy Markdown
Collaborator

@iivvaannxx iivvaannxx May 21, 2026

Choose a reason for hiding this comment

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

@asalloum5 This sounds to me an acceptable tradeoff. But was wondering if we could do something similar to the "Sync Commerce Scopes"? For those apps that do not have this data maybe a "Sync Association" or something? So that they do not need to: Unassociate + Associate

Might also be a "solution" (probably not the best) to the edge case that @oshmyheliuk mentioned about a Commerce Instance changing URLs.

@github-actions github-actions Bot added without-changeset The PR does not contain a Changeset file and removed with-changeset The PR contains a Changeset file. pkg: aio-commerce-lib-app Includes changes in `packages/aio-commerce-lib-app` labels May 22, 2026

- The stored shape can be extended with `projectId` and `workspaceId` at association time,
providing data needed by other planned SDK features without requiring a new association step.
- `getAssociatedCommerceInstance` could become the foundation for a higher-level helper that
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you include this in the scope and change the fetch example to be using resolver function? otherwise code will be repeated to create the commerce api client from the association details which we can already encapsulate.

Copy link
Copy Markdown
Contributor Author

@vinayrao2000 vinayrao2000 May 26, 2026

Choose a reason for hiding this comment

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

Yes good point, as suggested I will update the spec to include getAssociatedCommerceClient in scope and replace the fetch example with the resolver function pattern. This will return a ready to use AdobeCommerceHttpClient built from the stored association data, so developers don't have to wire it up themselves. I will implement it in this PR.

Comment on lines +86 to +94
The association data is stored using the infrastructure already established in
**`aio-commerce-lib-config`** — specifically, the shared `getSharedState()` utility from
`aio-commerce-lib-config/source/utils/repository.ts`, which provides a lazy-initialized
Adobe I/O State client shared across the SDK.

A new module is added to `aio-commerce-lib-config` specifically for association data,
separate from the existing Business Configuration module. It uses the same `getSharedState()`
client but stores under a dedicated reserved key (`association`) rather than the
`configuration.{scopeCode}` keys used by Business Configuration.
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.

I think it should be mentioned that the cache TTL should be specified for this configuration, as by default, TTL is 24 hours in aio lib state https://github.com/adobe/aio-lib-state/blob/main/doc/api.md#adobestateputoptions--object.
The max TTL of 1 year can be set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will update accordingly. The default TTL of 24 hours is not appropriate here association data is long-lived and must not expire on its own. It should only be cleared explicitly on unassociation. We'll set it to the max TTL of 1 year, after 1 year without re-association the data will expire.

The shape is designed to extend future fields (e.g. `projectId`, `workspaceId`) can be
added without a breaking change.

### New `association` runtime action
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.

Please mention require-adobe-auth: true in specs for those new actions, as they must be protected the same way as others.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure I will add auth to the action

Comment on lines +237 to +238
- **Reserved config key naming.** The exact key used to store the data in `aio-commerce-lib-config`
needs to be defined as a reserved SDK concern so that apps do not accidentally collide with it.
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.

This contradicts line 93, as the key naming was already mentioned:

client but stores under a dedicated reserved key (`association`) rather than the

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry for the confusion, I will remove here

Comment on lines +182 to +183
**On association** — after the app is successfully registered with the Extension Manager,
the client calls `POST /` with the Commerce instance details. This step is best effort a failure does not roll back the registration.
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.

If a POST/ request to the association endpoint fails, it may lead to a broken application if runtime actions depend on that configuration. I think we can't mark the app as successfully associated if the configuration wasn't saved. Probably the association should fail in such a case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. However, I have a suggestion, please let me know if this approach sounds okay. Since the association with the Extension Manager and the config storage are separate concerns, we feel that a transient network error on the association endpoint should not block the entire association flow. Instead of failing the association completely, we plan to surface the failure in the App Management UI so developers are aware of it and can re-associate the app if needed to resolve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

without-changeset The PR does not contain a Changeset file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants