fix(config): save a minimal target unless defaults are asked for

[sasjs-dev]

What

saveToLocalConfig and saveToGlobalConfig defaulted saveWithDefaultValues to true, so any caller that omitted the argument wrote every non-mandatory attribute into the saved sasjsconfig.

One caller does exactly that: the token-refresh handler at src/utils/config.ts:816, which passes only the target and isDefault. So every token refresh rewrote the global target with a full set of defaults - including a deployConfig the user never wrote. Both defaults are now false.

That is the other half of #221. The constructor and toJson halves are in sasjs/utils#277.

sasjs add is unaffected: addTarget.ts and addCredential.ts already pass false explicitly, so this changes nothing on that path. The specs that relied on the default are unaffected too - they assert that the target is present and whether it is the default, not the JSON shape, which is why they pass unchanged.

Verification

• npx jest src/utils/spec/config.spec.ts: 39 pass
• npx jest src/commands/add: 3 suites, 34 tests pass
• npx tsc -p tsconfig.json --noEmit: clean
• npm run lint: clean

Ordering

This takes full effect once this repo's @sasjs/utils pin moves to the release carrying the constructor fix - utils 3.7.1, from sasjs/utils#277. Until then the constructor still inserts deployConfig before toJson is reached, so the saved config stays minimal in everything except that one attribute. Bumping the pin is a follow-up for after 3.7.1 is published, per the usual order: utils, then cli, then vscode-extension.

[github-actions]

Coverage report

Total coverage

Status	Category	Percentage	Covered / Total
🟡	Statements	75.74%	3575/4720
🟡	Branches	63.38%	1464/2310
🟡	Functions	75.55%	717/949
🟢	Lines	84.29%	9094/10789

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status	Filename	Statements	Branches	Functions	Lines
🟢	src/commands/compile/compileSingleFile.ts	95.83% (-2.08% 🔻)	84% (-4% 🔻)	100%	100%
🟢	src/commands/deploy/deploy.ts	92.86%	79.17% (-2.65% 🔻)	75%	96.88%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 604c423

[sasjs-dev]

Reviewed the full base...head diff (one file, two default flips in src/utils/config.ts) against the CLI's call sites, the specs, and the @sasjs/utils Target contract this interacts with. The intent is right: the only production caller that relied on the old default is the token-refresh path at config.ts:816-818, which passes just (target, isDefault) and was rewriting the user's saved target with a full set of defaults on every refresh; sasjs add passes explicit values at addTarget.ts:33 and :93, so it is unaffected; the remaining no-flag callers are test helpers. Two findings:

• src/utils/config.ts:312 and :425 - flipping the default changes what every future caller that omits the argument persists, and the flipped signatures still write the minimal JSON into an existing target slot unconditionally (config.ts:343-347 and :443-447 replace the stored target whole). If a saved target predates this change and carries defaults, a refresh now strips them from the stored JSON even where the loaded Target instance still materialises them - fine - but the same replace means any caller refreshing a target loaded through new Target(rawConfig) will now persist the constructor-normalised shape rather than what the user wrote. That is the intent of the change (half of #221), but neither flipped function carries a doc comment saying the new contract: add a one-line JSDoc stating that omitting saveWithDefaultValues saves a minimal target and drops undeclared defaults, since this is a public-CLI-codebase function whose behaviour just inverted.

• the change is only half-effective until the pin moves: package.json still pins @sasjs/utils 3.6.5, whose Target constructor inserts a default deployConfig for any target that declares none (verified in the published 3.6.5 tarball - types/target.js validates { deployServicePack: true, deployScripts: [] } in the else branch). toJson(false) therefore still emits that injected deployConfig after this flip, exactly as the commit message acknowledges ("Takes full effect once this repo's @sasjs/utils pin moves to the release carrying the constructor fix"). The adapter has already bumped to the fixed 3.7.1 in sasjs/adapter#919 - follow with the same bump here, or the refresh path keeps writing deployConfig for one more cycle.

2 findings above for review.

[sasjs-dev]

Both findings addressed in c8560b7.

1. The JSDoc. Added to both flipped functions. It states the contract rather than leaving it to be inferred: omitting saveWithDefaultValues persists a minimal target - the mandatory attributes only - and passing true writes the full set of injected defaults, including a deployConfig the caller never declared. You are right that a public function whose behaviour just inverted should say so in the signature.

2. The pin. Moved to @sasjs/utils 3.7.1, which carries the constructor fix from sasjs/utils#277. Your diagnosis was exactly right - without it the flip is half-effective, because 3.6.5's constructor inserts a default deployConfig for any target that declares none, so toJson(false) still emits it. sasjs/adapter#919 had already moved to 3.7.1, so this follows it. package.json and the lockfile root both read 3.7.1, tsc is clean, and config.spec passes 39 tests.

Worth noting for the record: there are still nested 3.6.4 copies of utils in the tree, from the sibling packages that pin it themselves. They do not break this - tsc is clean, no TS2345 - but those pins will need the same treatment when their own PRs land.

[sasjs-dev]

Your second finding did more than require a pin bump - moving the pin exposed a user-facing regression, and it is now fixed in 604c423.

What happened. With @sasjs/utils 3.7.1 the Target constructor no longer injects { deployServicePack: true, deployScripts: [] } into a target that declares no deployConfig (sasjs/utils#277). deploy() branches on that value and throws when it is absent, so the CI job died at the SAS9 mocks deploy:

[error] Error deploying project:  Deployment failed.
Please either enable the 'deployServicePack' option or add deployment
script paths to 'deployScripts' in your target's 'deployConfig'.

The unit tests all passed - 668 of them - because this is a runtime path, not a type error. The job that caught it is the mocks deploy against a real sasjs/server, which is exactly why that step earns its keep.

Why it matters beyond this repo. Any existing sasjsconfig.json that omits deployConfig deployed before and would stop deploying once a consumer moved to 3.7.1. The blast radius is limited today because the CLI pins 3.6.5, but the adapter has already moved to 3.7.1 in sasjs/adapter#919 and others will follow.

The fix, in src/commands/deploy/deploy.ts. An absent deployConfig now means the historical default - deploy the service pack. An explicit deployServicePack: false is still honoured, and the guard that throws uses the same resolved value instead of re-reading the raw config:

const deployServicePack = target.deployConfig?.deployServicePack ?? true

That is the right layer for it. The utils change was about not writing defaults into the user's saved config; it was never meant to change runtime behaviour, and this restores that.

Also in this PR, from your first finding: the JSDoc on both flipped functions, stating the new contract.

Verified locally: tsc clean, deploy specs 28 passing, config and add specs 73 passing. The mocks deploy job re-runs on this head and is the end-to-end proof.

[sasjs-dev]

Re-review at the new head 604c423; the delta since the review at 7b5725f is the JSDoc on both save functions and the @sasjs/utils pin moving to 3.7.1 (both prior findings addressed), plus the new absent-deployConfig default in deploy. The deploy change is verified against the published tarballs: utils 3.7.1 no longer injects a default deployConfig in the Target constructor, toJson(false) now emits a genuinely minimal target, and the "?? true" at deploy.ts:34 restores the pre-3.7.1 behaviour for a target that declares no deployConfig, while a declared deployConfig still passes through validateDeployConfig, which coerces deployServicePack to a boolean, so the scripts-only case is unchanged. CI (test (22)) is green at the head. Two findings:

• package.json:5 - engines.node is still "^14.18.0 || >=16.10.0" while the runtime dependency @sasjs/utils is now pinned to 3.7.1, which itself requires node >=22 (verified in the published tarball). The manifest claims node 16/18/20 support that the dependency tree no longer honours: installs on those runtimes produce EBADENGINE warnings and fail outright under engine-strict. Raise engines.node to >=22 here; the cli branch of #1479 already carries that change, so landing #1479 first or folding the engines bump into this PR both work. Regenerate the lockfile root (package-lock.json packages "" engines, still the old range) in the same step.

• src/commands/deploy/deploy.ts:34 - the absent-deployConfig default is the behavioural point of this commit, but no spec exercises it: every Target in src/commands/deploy/spec/deploy.spec.ts and the cbd specs declares a deployConfig, and the deployConfig-less Target at deployCommand.spec.ts:9-14 is only ever passed to a mocked deploy(). Add one spec beside the existing "should deploy services to SAS Viya as a service pack" case in deploy.spec.ts: a Target that declares no deployConfig, asserting the service-pack deploy path runs. That pins the 3.7.1 contract and fails if the default regresses to refusing minimal targets again.

2 findings above for review.

[sasjs-dev]

Rolled into #1482. All three commits are carried there with git cherry-pick -x (the only conflict was the @sasjs/utils pin, resolved to 3.7.2). Both findings from the final review at 604c423 are addressed there: engines.node is >=22 with the lockfile root regenerated, and the absent-deployConfig default now has a spec in deploy.spec.ts - a target that declares no deployConfig, asserting the service-pack deploy path runs. Closing as superseded.