fix(DicomWebDataSource): set correct cross-service URLs on DICOMweb clients

[galenzo17]

Summary

• After client construction, assigns the correct cross-service URLs (wadoURL, qidoURL, stowURL) on each DICOMweb client so requests are routed to the right endpoint when qidoRoot and wadoRoot differ (e.g. /qidors/ vs /wadors/)
• Adds optional stowRoot config field for deployments with a separate STOW endpoint
• Adds unit tests covering cross-service URL assignment, explicit stowRoot, same-root (no-op), and stowRoot fallback scenarios

Closes #5820

Test plan

• Cross-service URLs are correctly assigned when qidoRoot and wadoRoot differ
• Explicit stowRoot is used when provided
• Backward-compatible: no-op when qidoRoot === wadoRoot
• stowURL defaults to wadoRoot when stowRoot is omitted

Greptile Summary

This PR fixes cross-service URL routing in the DicomWebDataSource by introducing applyServiceUrls, a utility that post-construction assigns the correct wadoURL, qidoURL, and stowURL on each DICOMweb client when qidoRoot and wadoRoot differ. It also adds an optional stowRoot config field and a dedicated test suite for the new utility.

• applyServiceUrls.ts guards assignments with !== undefined checks and uses ?? for stowRoot fallback — both are improvements over naive falsy checks.
• index.ts wires the utility into the initialize block immediately after client construction and extends DicomWebConfig with stowRoot.
• The new test file covers four representative scenarios but misses the edge case where qidoRoot is provided while both wadoRoot and stowRoot are absent, which would cause wadoClient.stowURL to be set to undefined at runtime.

Confidence Score: 4/5

Safe to merge after addressing the effectiveStowRoot undefined-assignment edge case in applyServiceUrls.ts

When config.qidoRoot is defined but both config.wadoRoot and config.stowRoot are absent, effectiveStowRoot is undefined and wadoClient.stowURL is overwritten with it — silently corrupting the client for any STOW operation. The guard on qidoRoot only protects wadoClient.qidoURL, not the stowURL assignment that follows it.

extensions/default/src/DicomWebDataSource/utils/applyServiceUrls.ts — the effectiveStowRoot computation needs to move inside each if guard

Important Files Changed

Filename	Overview
extensions/default/src/DicomWebDataSource/utils/applyServiceUrls.ts	New utility that cross-assigns service URLs; has a type-unsafe effectiveStowRoot that can be undefined inside the qidoRoot guard block, corrupting wadoClient.stowURL
extensions/default/src/DicomWebDataSource/utils/applyServiceUrls.test.ts	Unit tests covering four scenarios; no test for the edge case where qidoRoot is defined but wadoRoot and stowRoot are both absent
extensions/default/src/DicomWebDataSource/index.ts	Wires applyServiceUrls into the initialize hook and adds stowRoot to DicomWebConfig; the integration point itself is straightforward

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
extensions/default/src/DicomWebDataSource/utils/applyServiceUrls.ts:14-24
When `config.qidoRoot` is defined but both `config.wadoRoot` and `config.stowRoot` are undefined, `effectiveStowRoot` resolves to `undefined`. The second `if` block guard only checks `config.qidoRoot !== undefined`, so `wadoClient.stowURL` is overwritten with `undefined`, corrupting the client state. TypeScript strict mode also flags the assignment since `effectiveStowRoot: string | undefined` is not assignable to `stowURL: string`. Moving `effectiveStowRoot` inside each guard (where the relevant root is guaranteed defined) eliminates both the runtime risk and the type error.

```suggestion
  if (config.wadoRoot !== undefined) {
    const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
    qidoClient.wadoURL = config.wadoRoot;
    qidoClient.stowURL = effectiveStowRoot;
  }

  if (config.qidoRoot !== undefined) {
    const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
    wadoClient.qidoURL = config.qidoRoot;
    wadoClient.stowURL = effectiveStowRoot;
  }
```

_{Reviews (3): Last reviewed commit: "fix(DicomWebDataSource): guard against u..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[netlify]

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	bfcb2d1
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/69ff667dc791540008b822f7

[greptile-apps]

Test logic decoupled from production code

applyServiceUrls is a hand-copied extract of the assignment block in index.ts. If the production logic changes (e.g., a guard is added, a URL property is renamed, or a new client is introduced), these tests will continue to pass while the real behavior regresses silently. The tests should import and exercise the actual createDicomWebApi initializer (or at minimum import a shared utility), not duplicate its logic inline.

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/default/src/DicomWebDataSource/index.test.ts
Line: 27-42

Comment:
**Test logic decoupled from production code**

`applyServiceUrls` is a hand-copied extract of the assignment block in `index.ts`. If the production logic changes (e.g., a guard is added, a URL property is renamed, or a new client is introduced), these tests will continue to pass while the real behavior regresses silently. The tests should import and exercise the actual `createDicomWebApi` initializer (or at minimum import a shared utility), not duplicate its logic inline.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

When config.qidoRoot is defined but both config.wadoRoot and config.stowRoot are undefined, effectiveStowRoot resolves to undefined. The second if block guard only checks config.qidoRoot !== undefined, so wadoClient.stowURL is overwritten with undefined, corrupting the client state. TypeScript strict mode also flags the assignment since effectiveStowRoot: string | undefined is not assignable to stowURL: string. Moving effectiveStowRoot inside each guard (where the relevant root is guaranteed defined) eliminates both the runtime risk and the type error.

Suggested change

	const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
	if (config.wadoRoot !== undefined) {
	qidoClient.wadoURL = config.wadoRoot;
	qidoClient.stowURL = effectiveStowRoot;
	}
	if (config.qidoRoot !== undefined) {
	wadoClient.qidoURL = config.qidoRoot;
	wadoClient.stowURL = effectiveStowRoot;
	}
	if (config.wadoRoot !== undefined) {
	const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
	qidoClient.wadoURL = config.wadoRoot;
	qidoClient.stowURL = effectiveStowRoot;
	}
	if (config.qidoRoot !== undefined) {
	const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
	wadoClient.qidoURL = config.qidoRoot;
	wadoClient.stowURL = effectiveStowRoot;
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/default/src/DicomWebDataSource/utils/applyServiceUrls.ts
Line: 14-24

Comment:
When `config.qidoRoot` is defined but both `config.wadoRoot` and `config.stowRoot` are undefined, `effectiveStowRoot` resolves to `undefined`. The second `if` block guard only checks `config.qidoRoot !== undefined`, so `wadoClient.stowURL` is overwritten with `undefined`, corrupting the client state. TypeScript strict mode also flags the assignment since `effectiveStowRoot: string | undefined` is not assignable to `stowURL: string`. Moving `effectiveStowRoot` inside each guard (where the relevant root is guaranteed defined) eliminates both the runtime risk and the type error.

```suggestion
  if (config.wadoRoot !== undefined) {
    const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
    qidoClient.wadoURL = config.wadoRoot;
    qidoClient.stowURL = effectiveStowRoot;
  }

  if (config.qidoRoot !== undefined) {
    const effectiveStowRoot = config.stowRoot ?? config.wadoRoot;
    wadoClient.qidoURL = config.qidoRoot;
    wadoClient.stowURL = effectiveStowRoot;
  }
```

How can I resolve this? If you propose a fix, please make it concise.