Bugs surfaced by core-logic tests (PR #1245)

[chhoumann]

While adding unit tests in #1245, the tests surfaced these suspected bugs. They were deliberately not locked into the tests (the suite asserts correct behavior). Ordered by severity.

Medium

• quickAddApi.executeChoice proceeds with an undefined choice — src/quickAddApi.ts:215-228. When plugin.getChoiceByName returns undefined, it calls reportError (which only logs) but then still await choiceExecutor.execute(choice) with choice === undefined instead of returning early. Also the const choice: IChoice annotation is dishonest (value can be undefined). Fix: return after reporting.

Low

• duplicateChoice drops fields for Multi choices — src/services/choiceService.ts:42-49. The Multi early-return copies only choices/placeholder/collapsed (+ new name/id); unlike the non-Multi path it doesn't copy command/onePageInput, so duplicating a Multi with command: true yields command: false. Asymmetric vs other choice types.
• Dead schema-version check — src/services/packageImportService.ts:114-118. isQuickAddPackage() already requires schemaVersion === CURRENT exactly, so a newer-version package fails validation first and the user gets the generic "not a valid package" error instead of the helpful version-mismatch message.
• isChoiceImportable cycle guard skips bookkeeping — src/services/packageImportService.ts:169-213. On a parent cycle it returns true without finalizeImportable, leaving a stale entry in importableVisiting. Not reachable with valid (acyclic) input; low impact.
• Asset-kind dedup precedence — src/services/packageExportService.ts:115-132. Script paths are assigned last-write-wins while template/capture paths use if (!has), so a path used as both user-script and template is classified user-script. Likely intended; flagging for confirmation.
• utility.getSelectedText returns undefined on error branches — src/quickAddApi.ts:499-519 — while getSelection returns "". Minor API-shape inconsistency for user scripts.

Test coverage for each path now exists (#1245), so fixes can be verified by adjusting the corresponding assertion.

[chhoumann]

Verified all 6 empirically (reproduce + independent refutation). Outcome:

Confirmed real → fixed in #1247 (low severity): duplicateChoice dropping command/onePageInput for Multi; the unreachable package version-error message; getSelectedText returning undefined instead of "".

Verified NOT bugs → left unchanged:

• executeChoice 'proceeds with undefined choice' — unreachable: plugin.getChoiceByName throws on a miss and is typed : IChoice, so the if (!choice) branch is dead code. (The original 'medium' flag was based on a wrong assumption about getChoiceByName's contract.)
• import isChoiceImportable cycle-guard bookkeeping — harmless: proven via a 5000-trial differential test (zero observable difference vs a finalize-in-guard variant).
• export asset-kind dedup precedence — intended: the export/import round-trip is lossless regardless of the kind label.