fix: validate recordId before split in googledns deleteRecord (#454)

[nikos1005]

Summary

Add validation for recordId before destructuring split to avoid TS18048 under noUncheckedIndexedAccess.

Fixes #454

[greptile-apps]

Greptile Summary

Adds a pre-split guard to deleteRecord in the Google Cloud DNS provider that throws a descriptive error when recordId is not a string or is missing the required / separator, preventing downstream crashes or confusing API errors.

• The runtime check correctly prevents use of an invalid recordId, but the validation condition (includes('/')) still admits malformed values with empty segments (e.g. /example.com. or A/), and the TypeScript TS18048 errors on the destructured type/name variables may not be fully suppressed since TypeScript cannot narrow string[] destructuring positions to non-undefined based on a runtime guard alone.

Confidence Score: 4/5

The change is a small, focused guard that improves error messaging and prevents crashes on bad input; it does not alter the happy path.

The guard correctly handles the most common invalid input cases at runtime. Two edge cases remain: the validation accepts recordId values with empty segments (e.g. /example.com.), and the TypeScript TS18048 errors the PR targets may not be fully suppressed because TypeScript cannot narrow destructured array positions to non-undefined based solely on a runtime string check.

packages/dns/googledns/src/index.ts — the validation condition and whether it truly resolves the downstream TypeScript errors.

Important Files Changed

Filename	Overview
packages/dns/googledns/src/index.ts	Adds a runtime guard in deleteRecord to reject non-string or slash-free recordId values; the guard improves error messaging but may not fully resolve TS18048 for the destructured type/name variables downstream, and doesn't catch edge cases with empty segments.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[deleteRecord zoneId recordId config] --> B{recordId is not string OR has no slash}
    B -- Yes --> C[throw Error Invalid recordId]
    B -- No --> D[split recordId into type and name]
    D --> E[listRecords then filter by type and name]
    E --> F{existing.length === 0}
    F -- Yes --> G[return early]
    F -- No --> H[POST changes with deletions]
    H --> I{res.ok or 404}
    I -- No --> J[throw Error deleteRecord status]
    I -- Yes --> K[done]

Loading

Comments Outside Diff (1)

1. packages/dns/googledns/src/index.ts, line 129-132 (link)

   TS18048 may persist for type and name after the split

   The guard narrows recordId to string, but recordId.split('/') still returns string[]. Under noUncheckedIndexedAccess, destructuring a string[] yields string | undefined for each position — TypeScript cannot infer from the runtime includes('/') check that the two slots are populated. This means name.endsWith('.') (line 135), name.replace(…) (line 132), and the type comparison remain TS18048 sites. To actually close those errors you would need to either assert as [string, string] or add a post-split guard that narrows type and name to string.

_{Reviews (1): Last reviewed commit: "fix: validate recordId before split in g..." | Re-trigger Greptile}

[greptile-apps]

Empty segment edge cases pass validation

recordId.includes('/') accepts values like /example.com. or A/ — both contain a / but produce an empty type or name after the split. An empty type silently matches no records and the function returns without error; an empty name produces fqdn = '.' which is syntactically wrong and may generate a confusing API error. Adding a stricter pattern check would catch these at the validation boundary.

Suggested change

	if (typeof recordId !== 'string' || !recordId.includes('/')) {
	throw new Error(`Invalid recordId: ${recordId}. Expected format: "<type>/<FQDN>"`);
	}
	if (typeof recordId !== 'string' || !/^[^/]+\/[^/]+/.test(recordId)) {
	throw new Error(`Invalid recordId: ${recordId}. Expected format: "<type>/<FQDN>"`);
	}