fix: all sasjs tests should be passing on sas9

[sabhas]

Issue

closes #688

Checks

No PR (that involves a non-trivial code change) should be merged, unless all items below are confirmed! If an urgent fix is needed - use a tar file.

• All sasjs-cli unit tests are passing (npm test).
• (CI Runs this) All sasjs-tests are passing. If you want to run it manually (instructions available here).
• Data Controller builds and is functional on both SAS 9 and Viya

[4gl-reviewer]

Good catch. loginCallback is typed as () => Promise<void>, so this await is necessary — the other three call sites (lines 45, 82, 112) already await it. Without it, this.userName = loginParams.username on the next line could execute before the callback completes, causing a race condition.

[4gl-reviewer]

This unconditional getValidJson call applies to all server types, not just SAS9. Note that FileUploader.ts (line 106) does the same but guards with else if (this.serverType !== ServerType.Sasjs). For Sasjs, SasjsRequestClient.parseResponse already calls getValidJson, so res.result should already be an object — the typeof === 'string' check protects against that. However, consider whether a Sasjs guard is warranted here for consistency with FileUploader and to avoid double-parsing if the request client ever returns a string.

Also note: getValidJson throws JsonParseArrayError if the parsed JSON is a top-level array (see src/utils/getValidJson.ts:11). Before this PR, non-debug SAS9 string responses passed through as-is. If a SAS9 stored process can return a top-level JSON array, this will now throw where it previously did not. If array responses are not expected from SAS9, this is fine; if they are, this is a behaviour change worth documenting.

[4gl-reviewer]

This relaxation is correct: extractUserName in AuthManager.ts (line 286) returns '' for ServerType.Sas9, so the userName assertion can never pass on SAS9. Keeping the isLoggedIn check is the right level of validation for SAS9.

Minor: consider adding a brief comment explaining why the assertion differs for SAS9 (no username extraction), so future readers don't mistake this for incomplete testing.

[4gl-reviewer]

Hermes Agent Code Review

Verdict: Approve — with minor notes

Overview

This PR fixes three issues to make sasjs tests pass on SAS9:

1. src/auth/AuthManager.ts — Awaits this.loginCallback() at line 144, which was the only un-awaited call site out of four. This is a genuine bug fix — loginCallback is typed () => Promise<void>, and without await, this.userName is set on the next line before the callback completes, creating a race condition.

2. src/job-execution/WebJobExecutor.ts — Adds getValidJson() parsing for string responses in the non-debug path. Before this change, non-debug SAS9 string responses passed through raw to appendExtraResponseAttributes, inconsistent with FileUploader.ts which already parses them. This brings parity.

3. sasjs-tests/src/testSuites/Basic.ts — Relaxes the session check assertion for SAS9 to only check isLoggedIn (not userName), since extractUserName returns '' for SAS9. Correct.

Issues Found

Minor (no action required to merge):

• WebJobExecutor.ts:188 — The getValidJson call is unconditional across all server types, whereas FileUploader.ts guards with serverType !== ServerType.Sasjs. The typeof === 'string' check mitigates this for Sasjs (since SasjsRequestClient pre-parses), but a guard would improve consistency. Additionally, getValidJson throws JsonParseArrayError for top-level JSON arrays — if SAS9 stored processes can return array JSON, this is a new failure mode. Likely acceptable given the intent, but worth being aware of.

• Basic.ts:58 — Consider adding a brief comment explaining why the assertion differs for SAS9 (no username extraction), to aid future readers.

Verification

• ✅ npm run build — compiles successfully (webpack, 0 errors)
• ✅ Unit tests — 188 passed, 4 failed. The 4 failures are in RequestClient.spec.ts (TLS certificate tests), pre-existing and unrelated to this PR's changes.
• ✅ getValidJson edge cases verified: empty string → {}, null/undefined → throws InvalidJsonError, arrays → throws JsonParseArrayError.

Security

No security concerns. No injection vectors, no secret leakage, no auth bypass.

Reviewed by Hermes Agent (GitHub App)