refactor(snapshot-admin): migrate to shared helix-admin client

[shsteimer]

Summary

• Adds admin.snapshot({org,site}) (GET/update/remove) and admin.sidekick({org,site}) (GET-only) namespaces to scripts/helix-admin.js
• Rewrites tools/snapshot-admin/utils.js: removes module-level state and setOrgSite(), adds org/site as explicit params to all functions, replaces direct fetch calls with admin.snapshot() + executeAdminRequest (PREFLIGHT_AND_RETRY on reads, RETRY_ON_401 on writes per auth policy)
• Rewrites snapshot-utils.js: replaces all admin.hlx.page fetch functions with admin.snapshot()/admin.status() calls; keeps updateScheduledPublish + isRegisteredForSnapshotScheduler as direct fetch (external scheduler service, not admin.hlx.page)
• Updates snapshot-admin.js and snapshot-details.js: removes setOrgSite() calls, passes org/site to each utils function, adds null guards for login-cancelled returns, migrates the sidekick config fetch to admin.sidekick().get()
• Rewrites popover.js: removes inline duplicated admin.hlx.page functions, imports shared helpers from snapshot-utils.js, uses admin.snapshot().get() directly for listing (sidekick iframe — no AEM pipeline, no executeAdminRequest)
• palette.js unchanged — snapshot-utils.js preserves existing function signatures

Preview

https://refactor-snapshot-admin-helix-admin--helix-tools-website--adobe.aem.live/tools/snapshot-admin/index.html

Test plan

• Load the snapshot admin page with valid org/site — login modal appears if not signed in, snapshots list loads after sign-in
• Create a new snapshot — POST succeeds, list refreshes
• Open snapshot details — manifest loads, URLs/title/description populate correctly
• Save edits to a snapshot manifest
• Lock/unlock a snapshot
• Request/approve/reject a review
• Delete a snapshot
• Open popover.html?owner=...&repo=...&referrer=... — snapshot list loads without login modal, add/remove/review actions work
• Open palette.html?owner=...&repo=...&referrer=... — same as popover

🤖 Generated with Claude Code

[aem-code-sync]

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

• Re-run PSI checks
• Re-sync branch

Commits

• 49e444c ✅ (latest)
• 8c278a8 ✅
• cba760a ✅
• 8a13e68 ✅
• 618a83a ✅
• 15c8751 ✅

[aem-code-sync]

	Page	Scores	Audits	Google
📱	/tools/snapshot-admin/index.html
🖥️	/tools/snapshot-admin/index.html

[claude]

SHOULD FIX: The async reduce-with-sentinel pattern is correct but hard to reason about — undefined means "keep going", null means "login cancelled", anything else means "early error return". A for...of loop expresses the same logic without the mental overhead.

Suggested change

	export async function deleteSnapshotUrls(org, site, name, paths = ['/*']) {
	const earlyExit = await paths.reduce(async (chain, path) => {
	const prev = await chain;
	if (prev !== undefined) return prev;
	const result = await executeAdminRequest(
	() => admin.snapshot({ org, site }).remove(`${name}${path}`),
	{ org, site },
	);
	if (!result) return null;
	if (!result.ok) return { error: formatError(result), status: result.status };
	return undefined;
	}, Promise.resolve(undefined));
	return earlyExit !== undefined ? earlyExit : { success: true };
	}
	export async function deleteSnapshotUrls(org, site, name, paths = ['/*']) {
	for (const path of paths) {
	const result = await executeAdminRequest(
	() => admin.snapshot({ org, site }).remove(`${name}${path}`),
	{ org, site },
	);
	if (!result) return null;
	if (!result.ok) return { error: formatError(result), status: result.status };
	}
	return { success: true };
	}

[claude]

CONSIDER: The check order means x-error header value now wins over the friendly 401/403 fallbacks. In the old code, 401/403 always showed the actionable human copy regardless of x-error. In practice admin.hlx.page does set x-error on auth failures (e.g. [admin] not authenticated), so users who exhaust their login retries will see the technical header value instead of "Please make sure you are logged in."

If the intent is to surface admin's own message when available, that's reasonable — but it's worth confirming this is intentional rather than an accidental inversion.

[claude]

Code Review

Clean refactor overall — the shift from module-level org/site state to explicit params is a clear improvement, the network error handling in helix-admin.js is well-tested, and the duplication removal in popover.js is welcome.

Issues Found

SHOULD FIX

• utils.js:94–107 — deleteSnapshotUrls uses an async reduce with a undefined/null/error sentinel to implement sequential early-exit. The logic is correct but unnecessarily hard to follow. A for...of loop expresses the same intent without the indirection. Inline suggestion posted.

CONSIDER

• utils.js:25 — formatError now checks result.error (the x-error response header) before the friendly 401/403 fallbacks, inverting the old priority. When admin.hlx.page returns a technical x-error value on auth failures (which it does in practice), users who exhaust login retries will see that header string rather than the actionable copy. Worth confirming this is intentional. Inline note posted.

• tests/scripts/helix-admin.test.js — The new snapshot() and sidekick() namespaces are not explicitly tested. A small test asserting the constructed URL (/snapshot/{org}/{site}/main/...) would ensure a typo in the opBase call doesn't silently break them — similar to the existing URL-structure tests for preview, live, etc.

Verdict

REQUEST CHANGES — one SHOULD FIX (clarity) and two CONSIDERs. No security or correctness issues found.