fix(themes): several fixes for themes:preview and themes:migrate

[luis-almeida]

Description

Five latent bugs in zcli themes, each as its own commit:

1. Validation errors at line/column 0 rendered without the location suffix, so users couldn't jump to the source.
2. themes:preview skipped validation on file add/delete, so deleting a referenced partial left the preview silently broken.
3. themes:migrate crashed on any theme with templates under subfolders.
4. themes:migrate sent http://undefined:undefined/... URLs to the migrations endpoint — preview-server URLs leaked into a command that has no preview server.
5. Rapid saves during themes:preview fired concurrent uploads whose reload broadcasts raced, so the browser could reload on a stale state.

References

N/A

Risks

Low, scoped to zcli themes.

[Copilot]

Pull request overview

This PR fixes several latent issues in the zcli themes area, primarily improving correctness and reliability for themes:preview (validation + upload serialization) and themes:migrate (nested template handling + removing preview-server URL leakage).

Changes:

• themes:preview: watch add/unlink events and serialize preview uploads to avoid concurrent reload races; functional tests added to cover add/delete and rapid changes.
• Validation error rendering: include :line:column even when either value is 0, plus unit tests.
• themes:migrate: support nested template paths during rewrite and stop generating http://host:port/... URLs by making preview-server flags optional in asset/variable URL generation.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/zcli-themes/tests/functional/preview.test.ts	Adds functional coverage for add/delete revalidation and serialized uploads during rapid changes.
packages/zcli-themes/src/commands/themes/preview.ts	Serializes preview runs and listens to add/unlink/change to keep validation and preview consistent.
packages/zcli-themes/src/lib/validationErrorsToString.ts	Fixes location suffix rendering when line/column are 0.
packages/zcli-themes/src/lib/validationErrorsToString.test.ts	Adds test cases for 0 location values and missing location data.
packages/zcli-themes/src/lib/rewriteTemplates.ts	Creates intermediate directories before writing nested template paths.
packages/zcli-themes/src/lib/rewriteTemplates.test.ts	Updates tests to account for directory creation and nested paths.
packages/zcli-themes/src/lib/migrate.ts	Removes preview-server flags dependency and uses basename assets/variables for migration payload.
packages/zcli-themes/src/lib/migrate.test.ts	Updates migration tests to match new payload behavior and signature.
packages/zcli-themes/src/lib/getVariables.ts	Makes flags optional; returns filename when not in preview context.
packages/zcli-themes/src/lib/getVariables.test.ts	Adds tests for optional flags behavior; updates stubbing.
packages/zcli-themes/src/lib/getAssets.ts	Makes flags optional; returns filename when not in preview context.
packages/zcli-themes/src/lib/getAssets.test.ts	Adds tests for optional flags behavior; updates stubbing.
packages/zcli-themes/src/commands/themes/migrate.ts	Removes parsing/passing of flags now that migrate no longer depends on preview-server settings.
package.json	Forces mocha to exit after functional tests complete.

Comments suppressed due to low confidence (5)

packages/zcli-themes/src/lib/rewriteTemplates.ts:16

• The new mkdirSync call is inside the same try/catch as writeFileSync, but the thrown error message still says "Failed to write template file". If directory creation fails, this message is misleading and also drops the underlying error details. Consider updating the message to mention directory creation and/or include the original error message (or set it as the cause) to improve diagnosability.

      try {
        fs.mkdirSync(path.dirname(filePath), { recursive: true })
        fs.writeFileSync(filePath, content)
      } catch (error) {
        throw new CLIError(chalk.red(`Failed to write template file: ${filePath}`))
      }

packages/zcli-themes/src/lib/getVariables.test.ts:52

• Same as above: getVariables() uses fs.readdirSync() in its string-returning overload, so stubbing it with a fs.Dirent[] type cast is misleading. Use a string[] cast (or adjust implementation to request Dirent objects) to keep the test aligned with actual behavior.

    readdirSyncStub.returns(['logo.png', 'favicon.png'] as unknown as fs.Dirent[])

packages/zcli-themes/src/lib/getVariables.test.ts:69

• Same issue here: fs.readdirSync() is used without { withFileTypes: true } and returns string[], so casting the stub return to fs.Dirent[] is misleading and not representative of the production call signature.

    readdirSyncStub.returns(['logo.png'] as unknown as fs.Dirent[])

packages/zcli-themes/src/lib/getAssets.test.ts:51

• Same as above: getAssets() uses the string-returning fs.readdirSync() overload, so typing the stubbed return value as fs.Dirent[] is misleading. Use string[] (or change the implementation to request Dirent values if that’s intended).

    readdirSyncStub.returns(['foo.png', 'bar.png'] as unknown as fs.Dirent[])

packages/zcli-themes/src/lib/getAssets.test.ts:75

• Same stub typing issue here: readdirSync() is called without { withFileTypes: true } in getAssets(), so the runtime return type is string[]. Casting the stubbed return value to fs.Dirent[] is misleading.

    readdirSyncStub.returns(['unsuported file name.png'] as unknown as fs.Dirent[])

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

packages/zcli-themes/src/commands/themes/preview.ts:110

• handleThemeChange can run concurrently for rapid sequences of file events (chokidar doesn’t await async handlers). That means multiple preview() uploads and subsequent reload broadcasts can overlap and race, so the browser may reload a stale upload state. Consider serializing these operations (e.g., maintain a single in-flight promise/queue or debounce+coalesce events) so only one upload+reload cycle runs at a time, and newer changes can’t be overwritten by earlier completions.

    const handleThemeChange = async (path: string) => {
      this.log(chalk.bold('Change'), path)
      try {
        await preview(themePath, flags)
        wss.clients.forEach((client) => {
          if (client.readyState === WebSocket.OPEN) {
            client.send('reload')
          }
        })

[melzreal]

lgtm but may want an approval from someone more JS savy