SED-4859 Support Automation Package file resources in the Frontend by dvladir · Pull Request #1440 · exense/step-frontend

dvladir · 2026-08-20T15:48:23Z

No description provided.

gemini-code-assist

Code Review

This pull request introduces a new file picker modal and service to support selecting and downloading resources directly from automation packages, integrating this capability into the dynamic resource and resource input components. The review feedback highlights several important issues: extracting the automation package resource path by splitting on colons can truncate paths containing colons (such as Windows paths); the location input signal can get out of sync when the form control value is set programmatically without emitting events; the method name downloadAutomationPackageResourceUrl is misleading as it triggers a download rather than returning a URL; and using a non-null assertion on preview.resourceId is risky and should be replaced with a defensive check.

gemini-code-assist · 2026-08-20T15:50:31Z

+export const extractApResourcePath = (id: string): string => {
+  if (!isApResourceId(id)) {
+    return id;
+  }
+  const [prefix, apId, path] = id.split(':');
+  return path;
+};


Splitting the ID by : and destructuring it into [prefix, apId, path] will fail if the path itself contains colons (such as absolute Windows paths like C:/path/to/file). In such cases, the path will be truncated. Using slice(2).join(':') ensures that any colons within the path are preserved correctly.

Suggested change

export const extractApResourcePath = (id: string): string => {

if (!isApResourceId(id)) {

return id;

}

const [prefix, apId, path] = id.split(':');

return path;

};

export const extractApResourcePath = (id: string): string => {

if (!isApResourceId(id)) {

return id;

}

const parts = id.split(':');

return parts.slice(2).join(':');

};

Path format is known

gemini-code-assist · 2026-08-20T15:50:31Z

+  protected readonly location = signalFromFormControl<string>(this.locationControl);
+  protected readonly isLocationButtonDisabled = computed(() => {
+    const location = this.location();
+    return !location.trim();
+  });


Since location is a signal wrapping locationControl.valueChanges, it will not emit or update when this.locationControl.setValue is called with { emitEvent: false } (which happens in showRoots() and updateStateFromResult()). This causes isLocationButtonDisabled to become out of sync with the actual input value. Replacing the signal and computed property with a simple getter resolves this synchronization issue cleanly.

protected get isLocationButtonDisabled(): boolean { return !this.locationControl.value?.trim(); }

Use get in bindings is not a good practice.
Fixed in other way. isLocationButtonDisabled was switched to linked signal

gemini-code-assist · 2026-08-20T15:50:31Z

+  downloadAutomationPackageResourceUrl(apId: string, path: string, fileName: string): void {
+    const url = this.getDownloadAutomationPackageResourceUrl(apId, path);
+    downloadFile(url, fileName);
+  }


The method name downloadAutomationPackageResourceUrl is misleading because it actually triggers the file download (by calling downloadFile) rather than returning a URL. Renaming it to downloadAutomationPackageResource better reflects its behavior.

Suggested change

downloadAutomationPackageResourceUrl(apId: string, path: string, fileName: string): void {

const url = this.getDownloadAutomationPackageResourceUrl(apId, path);

downloadFile(url, fileName);

}

downloadAutomationPackageResource(apId: string, path: string, fileName: string): void {

const url = this.getDownloadAutomationPackageResourceUrl(apId, path);

downloadFile(url, fileName);

}

gemini-code-assist · 2026-08-20T15:50:31Z

+    }
+    const list = path.split('/');
+    const fileName = list[list.length - 1];
+    this._apApiService.downloadAutomationPackageResourceUrl(this.apId, path, fileName);


Update the method call to match the renamed downloadAutomationPackageResource method in AugmentedAutomationPackagesService.

Suggested change

this._apApiService.downloadAutomationPackageResourceUrl(this.apId, path, fileName);

this._apApiService.downloadAutomationPackageResource(this.apId, path, fileName);

gemini-code-assist · 2026-08-20T15:50:31Z

        for (const preview of previews) {
-          this.setPreviewContent(preview.resourceId, preview.success ? preview.content : undefined);
+          this.setPreviewContent(preview.resourceId!, preview.success ? preview.content : undefined);
        }


Using the non-null assertion operator preview.resourceId! is risky here because resourceId is defined as optional on the ResourcePreview type. It is safer to add a defensive check to ensure resourceId is defined before calling setPreviewContent.

Suggested change

for (const preview of previews) {

this.setPreviewContent(preview.resourceId, preview.success ? preview.content : undefined);

this.setPreviewContent(preview.resourceId!, preview.success ? preview.content : undefined);

}

for (const preview of previews) {

if (preview.resourceId) {

this.setPreviewContent(preview.resourceId, preview.success ? preview.content : undefined);

}

}

neogucky · 2026-08-24T06:27:46Z

-    <div class="directory-list">
-      @for (item of directories(); track item.absolutePath) {
-        @if (item.isDirectory && (showingRoots() || !item.isHidden)) {
+    <div class="file-list">


We have a function scrollDirectoryIntoView() which is used to scroll to newly created folders. But this one still uses directory-list so it is not working anymore.

…urce dialog

…841-automation-package-file-system-resource-resolver # Conflicts: # projects/step-core/src/lib/client/generated/models/FetchBucketsRequest.ts # projects/step-core/src/lib/client/generated/models/TwoStageAggregation.ts

neogucky · 2026-08-27T13:22:53Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces support for selecting, browsing, and downloading resources from automation packages. It refactors the file picker into a generic, reusable modal component with a data provider abstraction, integrates the new AutomationPackageResourceComponent into the standard resource input, and adds the RESOURCE_AP_ID token to dynamically resolve package IDs. Feedback highlights several improvement opportunities: ensuring extractApResourcePath correctly handles paths containing colons, making the IDE_MODE injection optional to prevent runtime injector errors in non-IDE contexts, allowing the clear button to be displayed for legacy standard resources even when an automation package is configured, and fixing a typo in a script file tooltip.

gemini-code-assist · 2026-08-27T13:25:02Z

+export const extractApResourcePath = (id: string): string => {
+  if (!isApResourceId(id)) {
+    return id;
+  }
+  const [prefix, apId, path] = id.split(':');
+  return path;
+};


The current implementation of extractApResourcePath splits the entire ID string by : and only takes the third element. If the resource path itself contains one or more colons (which is possible in some paths or filenames), the path will be truncated and parts of it will be lost.

Using parts.slice(2).join(':') is a more robust approach that correctly reconstructs the full path even if it contains colons.

Suggested change

export const extractApResourcePath = (id: string): string => {

if (!isApResourceId(id)) {

return id;

}

const [prefix, apId, path] = id.split(':');

return path;

};

export const extractApResourcePath = (id: string): string => {

if (!isApResourceId(id)) {

return id;

}

const parts = id.split(':');

return parts.slice(2).join(':');

};

Non issue as we don't have collons in paths

gemini-code-assist · 2026-08-27T13:25:02Z

+    const _injector = inject(Injector);
+    const _ideMode = inject(IDE_MODE);


Injecting IDE_MODE without making it optional can cause a NullInjectorError at runtime in non-IDE contexts (such as the standard keyword configuration or plan editor) if the token is not provided globally.

To ensure defensive programming and prevent runtime crashes, inject IDE_MODE with { optional: true } and default to false.

Suggested change

const _injector = inject(Injector);

const _ideMode = inject(IDE_MODE);

const _injector = inject(Injector);

const _ideMode = inject(IDE_MODE, { optional: true }) ?? false;

IDE_MODE is safely defined

gemini-code-assist · 2026-08-27T13:25:02Z


 <ng-template #clearButtonTemplate>
-  @if (_config.withClearButton()) {
+  @if (conf.withClearButton) {


When automationPackageId is defined, conf.withClearButton is forced to false by the configuration directive. However, if the current resource is a legacy standard resource (i.e., isApResource() is false), the user will be unable to clear it to switch to the automation package resource mode.

We should allow showing the clear button in this scenario so that users can clear standard resources and select an AP resource instead.

@if (conf.withClearButton || (automationPackageId() && !isApResource())) {

This would enable the removal of non-ap resources which can never be recreated, therefor it should not be allowed.

gemini-code-assist · 2026-08-27T13:25:02Z

    <step-dynamic-resource
      label="Script file"
      type="functions"
      tooltip="Leave empty if you wan\'t step to generate it for you otherwise specify a file accessible to the controller. This file will be transferred automatically to the agents."
      [formControl]="formGroup.controls.scriptFile"
-    ></step-dynamic-resource>
-  </ng-container>
-</ng-container>
+    />


The tooltip contains a typo and unnecessary backslash escaping: wan\'t. This should be corrected to want for proper English and to avoid rendering the backslash literally in the UI.

Suggested change

<step-dynamic-resource

label="Script file"

type="functions"

tooltip="Leave empty if you wan\'t step to generate it for you otherwise specify a file accessible to the controller. This file will be transferred automatically to the agents."

[formControl]="formGroup.controls.scriptFile"

></step-dynamic-resource>

</ng-container>

</ng-container>

/>

<step-dynamic-resource

label="Script file"

type="functions"

tooltip="Leave empty if you want step to generate it for you otherwise specify a file accessible to the controller. This file will be transferred automatically to the agents."

[formControl]="formGroup.controls.scriptFile"

/>

SED-4859 Support Automation Package file resources in the Frontend

cd646eb

gemini-code-assist Bot reviewed Aug 20, 2026

View reviewed changes

dvladir added 3 commits August 21, 2026 11:46

SED-4859 Adapt resource input to IDE mode

355cf8c

SED-4859 Fix resource input

f2a5d08

SED-4589 Fixes

e9ba004

dvladir requested a review from neogucky August 21, 2026 16:05

neogucky approved these changes Aug 24, 2026

View reviewed changes

neogucky added 4 commits August 24, 2026 08:36

SED-4841 Fixed scrollDirectoryIntoView

ce54a1d

SED-4841 added apResource support to keyword library

2b6ef07

SED-4841 added clear apResource button and file icon to select apReso…

be60ef8

…urce dialog

SED-4859 hide download / clear buttons if there is no apResource

f78db6a

neogucky marked this pull request as ready for review August 27, 2026 13:02

gemini-code-assist Bot reviewed Aug 27, 2026

View reviewed changes

SED-4841 review comments

c2cbb18

neogucky approved these changes Aug 27, 2026

View reviewed changes

	this._apApiService.downloadAutomationPackageResourceUrl(this.apId, path, fileName);
	this._apApiService.downloadAutomationPackageResource(this.apId, path, fileName);

		const _injector = inject(Injector);
		const _ideMode = inject(IDE_MODE);

Uh oh!

Conversation

dvladir commented Aug 20, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

dvladir Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

dvladir Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

neogucky Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

neogucky commented Aug 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

neogucky Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

neogucky Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

neogucky Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants