Repository navigation
feat: add abgen as an opt-in conversion platform - #396
Conversation
Routes jobs to the abgen generator's queue via ab-admin (platform=abgen). Deliberately excluded from the default platform set so default and mass reconversions (snapshot runs) don't feed abgen until explicitly requested. Requires the ab-admin side (ops-lambdas MR 408) deployed with TASK_QUEUE_ABGEN configured in the target environment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
decentraland-bot
left a comment
There was a problem hiding this comment.
Review: feat: add abgen as an opt-in conversion platform
PR: #396 · Branch: feat/abgen-platform → main
Files changed: 5 (+18/−9) · CI: ✅ build passing
Summary
Clean, minimal change that adds ABGEN = 'abgen' to the Platform enum and replaces the Object.values(Platform) default with an explicit DEFAULT_PLATFORMS = [Platform.WINDOWS, Platform.MAC] constant. The design is sound — abgen is opt-in only, never included in default or mass reconversion runs, and the existing runtime validation (assert(Object.values(Platform).includes(platform))) correctly accepts the new value.
All three command files (queue-ab-conversion, -about, -snapshot) are updated consistently. README documentation is thorough.
Findings
No P0 or P1 issues found.
[P2] DEFAULT_PLATFORMS could be readonly — src/helpers/asset-bundles.ts
The exported array is mutable; any consumer could call .push() and corrupt the default for subsequent invocations. Free to fix:
export const DEFAULT_PLATFORMS: readonly Platform[] = [Platform.WINDOWS, Platform.MAC]Low risk for an internal CLI, but good TypeScript hygiene for module-level constants.
[P2] Silent no-op flags for abgen — all command files
--prioritize, --animation, and --doISS are accepted but have no effect when --platform abgen is the target. A console.warn would be a nice quality-of-life improvement, but the companion skill documentation (agent-server#265) covers these differences thoroughly. Not blocking.
[P2] Pre-existing: unsafe as Platform[] cast — all command files (informational)
The (args['--platform'] as Platform[]) cast bypasses compile-time type checking, but the downstream assert() validation catches invalid values at runtime. Not introduced by this PR.
[P2] Test coverage — repo-wide (pre-existing)
The repo's test suite is a single smoke test (test/unit.spec.ts). No unit tests cover the asset-bundles module or platform validation. Not introduced by this PR, but worth noting for future work.
Consumer Impact
The Platform enum is internal to opscli. External consumers invoke the CLI via npx @dcl/opscli@latest queue-ab-conversion. The change is fully backward-compatible: default behavior (windows, mac) is preserved, and --platform abgen is purely additive. Checked across decentraland/sdk7-goerli-plaza workflows, decentraland/agent-server skill, and decentraland/unity-explorer docs — no consumers are affected.
Git Conventions (ADR-6)
- PR title:
feat: add abgen as an opt-in conversion platform✅ - Branch:
feat/abgen-platform✅ - Commits follow semantic format ✅
Security
- No secrets or credentials committed
- Token references remain as env vars (
$AB_ADMIN_TOKEN_*) - Platform input validated at runtime via
assert() DEFAULT_PLATFORMSpattern is a net security improvement overObject.values(Platform)— prevents accidental inclusion of experimental platforms in batch jobs- No new network endpoints or auth changes
DEPENDENCY_REVIEW: PASS
Verdict: ✅ APPROVE
No P0 or P1 findings. The design is pragmatic, the change is minimal, and the opt-in guard is well-placed. P2 suggestions (readonly, console warnings) are improvements for a follow-up.
Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni (<@U03JSUQ5Z7U>) via Slack
What
Adds
abgenas a valid--platformvalue onqueue-ab-conversion,queue-ab-conversion-snapshotandqueue-ab-conversion-about, routing the job to the abgen generator (the Rust asset-bundle pipeline) through ab-admin's/enqueue-task.Details
DEFAULT_PLATFORMS = [windows, mac]instead ofObject.values(Platform), so omitting--platformbehaves exactly as today and mass snapshot reconversions never feed abgen implicitly.--prioritizeis accepted but lands on the same queue).--forceworks: abgen honors it by bypassing its already-converted-at-AB_VERSION check.--animation/--doISSare Unity-only and ignored by abgen.organdzonehave an abgen queue;todayreturns a 400 forplatform=abgen.Backend status — all landed
platform=abgensupport: ops-lambdas MR 408 (https://dcl.tools/ops/ops-lambdas/-/merge_requests/408), merged and deployed to dev; promoted to prd via MR 422 (https://dcl.tools/ops/ops-lambdas/-/merge_requests/422)./enqueue-task?platform=abgencall; the abgen registry showed the entity go queued → complete.Merging publishes
@dcl/opscli@latestwith the new flag value. The agent-serverab-reconvertskill update that documents--platform abgenshould merge after this is published.🤖 Generated with Claude Code