Skip to content

fix: merge execute load options with dir - #1626

Open
liyimil wants to merge 1 commit into
oclif:mainfrom
liyimil:agent/merge-execute-load-options
Open

liyimil wants to merge 1 commit into
oclif:mainfrom
liyimil:agent/merge-execute-load-options

Conversation

@liyimil

@liyimil liyimil commented Jul 18, 2026

Copy link
Copy Markdown

What changed

  • merge execute()'s dir with partial loadOptions
  • preserve explicitly supplied load options such as ignoreManifest
  • normalize file URL roots before loading the CLI
  • add a regression test covering dir plus {ignoreManifest: true}

Why

Passing loadOptions currently replaces dir entirely, so inferred values such as root are lost. Supplying only ignoreManifest then reaches the loader without a root and fails before the option can take effect.

Fixes #1533.

Checks

  • yarn compile --noEmit
  • focused Mocha test on Node 22: test/execute.test.ts
  • yarn lint (passes with existing warnings)
  • ESLint and Prettier checks for the changed files

Prepared with Codex assistance; the diff was reviewed and the checks above were run locally.

@salesforce-cla

Copy link
Copy Markdown

Thanks for the contribution! Before we can merge this, we need @liyimil to sign the Salesforce Inc. Contributor License Agreement.

@liyimil
liyimil marked this pull request as ready for review August 5, 2026 11:58
@jfeingold35 jfeingold35 closed this Aug 5, 2026
@jfeingold35 jfeingold35 reopened this Aug 5, 2026
@jfeingold35

Copy link
Copy Markdown
Contributor

Closed and re-opened to re-evaluate the CLA.
I will review this, conduct any necessary QA, and merge if it's ready to go. Thank you for the contribution, @liyimil.

@jfeingold35

Copy link
Copy Markdown
Contributor

@liyimil , this looks like it resolves the issue where you needed to have root, but it still seems like the description is being pulled from the manifest instead of the file.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ignoreManifest option in loadOptions not respected by execute()

2 participants