fix(apps): include app_builder_id in the missing-URL warning

app_builder_id is present whenever this warning fires, except for a narrow
DB invariant violation in the backend's upload no-op path (documented in
app-builder-code's service.go) that shouldn't occur in normal operation.
Surfacing it alongside the display name gives an unambiguous reference:
disambiguates if multiple apps share a name, and is directly usable by
anyone who already knows their org's domain.

Extracted the message construction into buildMissingAppUrlWarning to
avoid duplicating the ID-suffix logic across the upload and release call
sites. Added a test for the edge case where app_builder_id is also absent,
confirming the suffix is omitted rather than printing something broken.

Author: tyffical

packages/plugins/apps/src/upload.test.ts

@@ -300,17 +300,40 @@ describe('Apps Plugin - upload', () => {
             // But it also must not go silent — surfaced as a warning so it's visible in CI
             // output (see handle-upload.ts, which logs and aggregates `warnings` without
             // failing the build, unlike `errors`). Names the app by its display name (not
-            // context.identifier, which is an opaque hash — see identifier.ts) so there's a
-            // concrete next step (find it in the apps list), not just a generic warning.
+            // context.identifier, which is an opaque hash — see identifier.ts) and includes
+            // the app_builder_id, so there's a concrete, unambiguous next step (find it in
+            // the apps list, disambiguated by ID if names collide), not just a generic warning.
             expect(warnings).toHaveLength(1);
             expect(warnings[0]).toContain('Could not resolve the App Builder URL');
             expect(warnings[0]).toContain(context.name);
+            expect(warnings[0]).toContain('builder123');
 
             // Reset — other tests in this file rely on getDDEnvValueMock's default
             // (undefined) behavior and beforeEach doesn't reset this particular mock.
             getDDEnvValueMock.mockReset();
         });
 
+        test('Should omit the app ID suffix when app_builder_id is also absent', async () => {
+            getDDEnvValueMock.mockImplementation((key) =>
+                key === 'APPS_PUBLISH' ? 'false' : undefined,
+            );
+            // Matches the backend's own no-op-path edge case (app-builder-code's
+            // TestAppBuilderURL_EmptyAppBuilderID) — both app_builder_id and
+            // app_builder_url absent, not just the URL.
+            doAuthenticatedRequestMock.mockResolvedValueOnce({
+                version_id: 'v123',
+                application_id: 'app123',
+            } as any);
+
+            const { warnings } = await uploadArchive(archive, context, logger);
+
+            expect(warnings).toHaveLength(1);
+            expect(warnings[0]).toContain(context.name);
+            expect(warnings[0]).not.toContain('app ID');
+
+            getDDEnvValueMock.mockReset();
+        });
+
         test('Should upload archive using the supplied request function', async () => {
             const doUploadAuthenticatedRequestMock = jest.fn().mockResolvedValue({
                 version_id: 'v123',
@@ -419,8 +442,9 @@ describe('Apps Plugin - upload', () => {
                 })
                 // The backend couldn't resolve the org's app URL for this release — a real,
                 // designed-for degradation path (e.g. a transient org-lookup failure), not a
-                // hypothetical. The release itself still succeeded.
-                .mockResolvedValueOnce({});
+                // hypothetical. The release itself still succeeded. app_builder_id is still
+                // present, though — it's set from the DB independently of the URL lookup.
+                .mockResolvedValueOnce({ app_builder_id: 'builder123' });
 
             const { errors, warnings } = await uploadArchive(archive, context, logger);
 
@@ -433,10 +457,11 @@ describe('Apps Plugin - upload', () => {
             expect(releaseLog?.[0]).toContain('to live.');
             expect(releaseLog?.[0]).not.toContain('\n');
             // Surfaced as a warning rather than a blank/malformed log line — names the app
-            // by its display name so there's a concrete next step (find it in the apps list).
+            // by its display name and includes the app_builder_id for unambiguous lookup.
             expect(warnings).toHaveLength(1);
             expect(warnings[0]).toContain('Could not resolve the App Builder URL');
             expect(warnings[0]).toContain(context.name);
+            expect(warnings[0]).toContain('builder123');
         });
 
         test.each(['false', '0', 'False', 'FALSE', 'off', 'no'])(

packages/plugins/apps/src/upload.ts

@@ -41,6 +41,19 @@ export const getReleaseUrl = (site: string, appId: string) => {
     return `https://api.${site}/${APPS_API_PATH}/${appId}/release/live`;
 };
 
+// Builds the warning shown when the backend couldn't resolve an org's App Builder URL.
+// Names the app (context.name — always human-readable, unlike context.identifier's opaque
+// hash) and its app_builder_id when present, which disambiguates same-named apps and lets
+// anyone who knows their org's domain construct the link directly.
+const buildMissingAppUrlWarning = (
+    action: 'upload' | 'release',
+    name: string,
+    appBuilderId?: string,
+) => {
+    const idSuffix = appBuilderId ? ` (app ID: ${appBuilderId})` : '';
+    return `Could not resolve the App Builder URL for this ${action} — find "${name}"${idSuffix} in your App Builder apps list to view it.`;
+};
+
 export const getData =
     (archivePath: string, defaultHeaders: Record<string, string> = {}, name: string) =>
     async (): Promise<DataResponse> => {
@@ -126,10 +139,14 @@ Would have uploaded ${summary}`,
         } else {
             // The backend couldn't resolve this org's App Builder URL (e.g. a transient
             // lookup failure) — the upload itself still succeeded, so this doesn't fail the
-            // build. Point at the apps list by name (not context.identifier, which is an
-            // opaque hash in the default case — see identifier.ts's buildIdentifier — and
-            // not something anyone would recognize or search for).
-            const message = `Could not resolve the App Builder URL for this upload — find "${context.name}" in your App Builder apps list to view it.`;
+            // build. Uses context.name, not context.identifier, since the latter is an
+            // opaque hash in the default case (see identifier.ts's buildIdentifier) — not
+            // something anyone would recognize or search for.
+            const message = buildMissingAppUrlWarning(
+                'upload',
+                context.name,
+                response.app_builder_id,
+            );
             warnings.push(message);
             log.warn(message);
         }
@@ -164,7 +181,11 @@ Would have uploaded ${summary}`,
                 );
             } else {
                 log.info(`Published uploaded version ${bold(response.version_id)} to live.`);
-                const message = `Could not resolve the App Builder URL for this release — find "${context.name}" in your App Builder apps list to view it.`;
+                const message = buildMissingAppUrlWarning(
+                    'release',
+                    context.name,
+                    releaseResponse.app_builder_id,
+                );
                 warnings.push(message);
                 log.warn(message);
             }