[#186] Replace preconditionFailure with graceful fallback

[bguidolim]

Context

ManifestBuilder.renderComponent used preconditionFailure for the .generic/.none cases of ExternalCopyFileType. This crashes the CLI in both debug and release builds. While the export wizard never produces .generic components, renderYAML accepts any ExternalPackManifest — so a manifest loaded from disk with an omitted or .generic fileType would crash with no recovery.

Closes #186

Changes

• Replace preconditionFailure with assertionFailure (stripped in release builds) plus a key = "hook" fallback
• Debug builds still get a loud assertion signal if .generic reaches the render path
• Release builds gracefully produce valid hook: YAML instead of crashing

Testing Steps

• swift build succeeds
• All 585 tests pass (swift test)
• ManifestBuilder round-trip tests pass

[Copilot]

Pull request overview

This PR updates ManifestBuilder.renderComponent to avoid crashing the CLI when encountering .generic or missing fileType values while rendering YAML, replacing a hard preconditionFailure with a debug-only assertion and a fallback YAML key.

Changes:

• Replace preconditionFailure with assertionFailure in the .generic/.none branch.
• Add a fallback YAML key (hook) so release builds render valid YAML instead of terminating.

Comments suppressed due to low confidence (1)

Sources/mcs/Export/ManifestBuilder.swift:499

• case .generic, .none: triggers assertionFailure for both .generic and a missing fileType (nil). Since fileType is optional in ExternalCopyPackFileConfig, a manifest loaded from disk with an omitted fileType would assert in debug builds even though it should be a normal, recoverable case. Consider splitting the cases so only .generic asserts, and .none just falls back to a default key without an assertion.

            case .generic, .none:
                assertionFailure("Export does not produce .generic file components")
                key = "hook"

---

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

Sources/mcs/Export/ManifestBuilder.swift:505

• There are two separate skip paths for .generic/.none copy-pack files: the early-return guard at the top of renderComponent and the case .generic, .none: return inside the fileType switch. This duplication makes it easy for the handling to drift, and the inner return is currently redundant given the guard. Suggest consolidating the behavior into a single place (preferably where you can decide before emitting any YAML for the component) so the output is deterministic and easier to maintain.

            switch config.fileType {
            case .hook: key = "hook"
            case .skill: key = "skill"
            case .command: key = "command"
            case .generic, .none:
                return
            }

Sources/mcs/Export/ManifestBuilder.swift:442

• This newly introduced edge-case handling for .copyPackFile with fileType == nil / .generic isn’t covered by the existing ManifestBuilderTests. Since the change is meant to prevent crashes / define fallback behavior, it would be good to add a regression test that constructs a manifest containing such a component and verifies the renderer’s output (fallback key, verbose rendering, or explicit skip) matches the intended behavior.

        // Skip .generic file components — no YAML shorthand key exists for this type
        if case .copyPackFile(let config) = comp.installAction,
           config.fileType == .generic || config.fileType == nil {
            return
        }

---

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

[Copilot]

This change silently skips .copyPackFile components when fileType is .generic or nil, but the PR description/Issue #186 state we should emit an assertionFailure in debug and fall back to rendering valid YAML (e.g., defaulting the shorthand key to hook in release). As written, these components will be dropped from the exported YAML with no signal, which is a behavioral change and may lose data. Consider implementing the described assertionFailure + fallback (or rendering the verbose type + installAction form / emitting an explicit YAML warning comment) instead of an unconditional early return.