Skip to content

fix(fiori-migration-writer): remove unnecessary path manipulation in launch config generation - #5400

Open
korotkovao wants to merge 3 commits into
mainfrom
fix/launch-config-path-duplication
Open

korotkovao wants to merge 3 commits into
mainfrom
fix/launch-config-path-duplication

Conversation

@korotkovao

Copy link
Copy Markdown
Contributor

Description

Fixes path duplication bug where migration fails when generating VS Code launch configuration due to duplicated paths.

Problem

Multiple users reported migration failures with duplicated paths (sanitized examples):

Error committing changes while configuring launch json file: Error: ENOENT: no such file or directory, 
mkdir 'c:\Users\<user>\Desktop\c:\Users\<user>\Desktop\<project>\.vscode'

Notice the path appears twice before .vscode.

Root Cause

Location: packages/fiori-migration-writer/src/config/launch-config.ts lines 32-34

The code was unnecessarily deconstructing and reconstructing the path:

const projectName = projectInfo.rootPath.split(pathSep).pop() as string;
const targetFolder = join(projectInfo.rootPath, '..');  
const projectPath = join(targetFolder, projectName);

This caused path duplication on Windows when join() operations were performed.

Solution

Since projectInfo.rootPath is already the correct full path to the project, use it directly:

const projectPath = projectInfo.rootPath;
const projectName = basename(projectPath);

Testing

  • ✅ All existing tests pass (498 tests including 195 snapshots)
  • ✅ Lint checks pass
  • ✅ Integration tests verify the fix works correctly

Impact

  • Fixes migration failures for users with paths containing spaces on Windows
  • Prevents .vscode directory creation errors
  • Allows launch configuration generation to complete successfully

Related Issues

…launch config generation

Fixes path duplication bug where projectInfo.rootPath was being
unnecessarily deconstructed and reconstructed, causing duplicated
paths on Windows:

Before: 'c:\Users\<user>\workspace\c:\Users\<user>\workspace\project\.vscode'
After:  'c:\Users\<user>\workspace\project\.vscode'

The issue was in generateLaunchConfiguration() which:
1. Extracted project name from rootPath
2. Got parent folder with join(rootPath, '..')
3. Rejoined them with join(parent, name)

This path manipulation was unnecessary since projectInfo.rootPath
is already the correct full path to the project.

Solution: Use projectInfo.rootPath directly and extract projectName
with basename() only when needed.

Relates-To: https://github.wdf.sap.corp/ux-engineering/tools-suite/issues/39900
@changeset-bot

changeset-bot Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0871f48

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sap-ux/fiori-migration-writer Patch
@sap-ux/create Patch

Not sure what this means? Click here to learn what changesets are.

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

@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


Fix launch configuration path duplication during migration

Description

Fixes VS Code launch configuration generation during Fiori migration by using the existing project root path directly.

The previous path reconstruction could duplicate absolute Windows paths, causing .vscode directory creation to fail with an ENOENT error. This change:

  • Uses projectInfo.rootPath as the project path without modification.
  • Derives the project name with basename().
  • Removes unnecessary path utilities.

Related issue: https://github.wdf.sap.corp/ux-engineering/tools-suite/issues/39900

Type of change

  • Bug (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a new feature)
  • Breaking change (Bug or New feature that would cause existing functionality/consumers to not work as expected)
  • Non-Breaking chores (Changes to tools, libraries, build process, documentation, etc)
  • None of the above (Reviewers might ask for more clarification)

How have you tested?

  • Ran the existing test suite: 498 tests and 195 snapshots pass.
  • Ran lint checks successfully.
  • Verified through integration tests that launch configuration generation uses the correct project path without duplication.

Checklist:

  • The code conforms to the general development principles
  • Supplied as many details as possible on this change
  • The code is easy to read and maintainable by others
  • Corresponding changes to the documentation has been done
  • Already existing and new unit tests pass locally
  • I have reviewed and addressed all Hyperspace bot findings (or explicitly explained dismissals)
  • I have done an Agentic review

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.33.44

  • Output Template: Repository PR Template
  • LLM: gpt-5.6-sol
  • Correlation ID: dd549640-dd26-11f1-8c96-d504aae56225
  • Summary Prompt: Default Prompt
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content

@hyperspace-pr-bot hyperspace-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The path simplification is sound, but the PR needs minor cleanup and a required patch changeset before merging.

PR Bot Information

Version: 1.33.44

  • LLM: gpt-5.6-sol
  • Correlation ID: dd549640-dd26-11f1-8c96-d504aae56225
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • Experiment Variant: control
  • Agent Instructions:

const projectPath = join(targetFolder, projectName);
// projectInfo.rootPath is already the full path to the project
const projectPath = projectInfo.rootPath;
const projectName = basename(projectPath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: projectName is now unused

This declaration is no longer referenced because fioriOptions.name calls basename(projectPath) directly, so the package's TypeScript/lint checks may fail on the unused local. Consider removing it.

Suggested change
const projectName = basename(projectPath);

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

const targetFolder = join(projectInfo.rootPath, '..');
const projectPath = join(targetFolder, projectName);
// projectInfo.rootPath is already the full path to the project
const projectPath = projectInfo.rootPath;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Best Practices: This published source fix has no changeset

Repository policy requires a patch changeset for every src/ bug fix, and .changeset currently contains no entry for @sap-ux/fiori-migration-writer, so changeset validation will fail. Please add a patch changeset whose summary starts with FIX:.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

@sonarqubecloud

Copy link
Copy Markdown

This branch was previously deployed

1 inactive deployment
ci-secrets — 0871f480 Deployed Nov 10, 2026 by korotkovao via build (ubuntu-latest, 24.x) #79053
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.

1 participant