Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new mcs export interactive wizard that discovers the user’s live Claude Code configuration and generates a shareable tech-pack directory (manifest + supporting files), positioning it as the inverse of mcs sync.
Changes:
- Introduces
ExportCommand(mcs export) and registers it in the CLI. - Adds an export pipeline:
ConfigurationDiscovery→ManifestBuilder→PackWriter. - Updates documentation to describe and promote the new export workflow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/techpack-schema.md | Adds a tip pointing readers to mcs export for auto-generating manifests. |
| docs/creating-tech-packs.md | Replaces “planned” export section with actual mcs export quick start guidance. |
| docs/architecture.md | Documents the new Export system and source layout. |
| Sources/mcs/Export/PackWriter.swift | Writes (or previews) the exported pack directory structure. |
| Sources/mcs/Export/ManifestBuilder.swift | Renders discovered artifacts into a formatted techpack.yaml and collects files/templates to export. |
| Sources/mcs/Export/ConfigurationDiscovery.swift | Scans Claude configuration files/directories and builds a discovered configuration model. |
| Sources/mcs/Commands/ExportCommand.swift | Implements the mcs export command flow (scope selection, discovery, selection, build, write/preview). |
| Sources/mcs/CLI.swift | Registers ExportCommand in the main CLI subcommand list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `mcs export` is the inverse of `mcs sync`: it reads installed artifacts and generates a `techpack.yaml` manifest. The export flow uses three dedicated types: | ||
|
|
||
| 1. **ConfigurationDiscovery** (`Export/ConfigurationDiscovery.swift`) — reads live config files (`~/.claude.json`, `settings.json`, `.claude/` directories, `CLAUDE.md`, global gitignore) and produces a `DiscoveredConfiguration` model | ||
| 2. **ManifestBuilder** (`Export/ManifestBuilder.swift`) — converts selected artifacts into a YAML string using shorthand syntax. Handles sensitive env var replacement (`__PLACEHOLDER__` tokens + `prompts:` entries), brew dependency hints, and section comments |
There was a problem hiding this comment.
This section describes placeholder replacement as __PLACEHOLDER__ tokens, but the codebase uses __KEY__ placeholders (see TemplateEngine / schema docs). Please update the architecture docs to reflect the actual placeholder syntax used during mcs sync.
| 2. **ManifestBuilder** (`Export/ManifestBuilder.swift`) — converts selected artifacts into a YAML string using shorthand syntax. Handles sensitive env var replacement (`__PLACEHOLDER__` tokens + `prompts:` entries), brew dependency hints, and section comments | |
| 2. **ManifestBuilder** (`Export/ManifestBuilder.swift`) — converts selected artifacts into a YAML string using shorthand syntax. Handles sensitive env var replacement (`__KEY__` placeholders + `prompts:` entries), brew dependency hints, and section comments |
| enum ExportError: Error, CustomStringConvertible { | ||
| case outputDirectoryExists(String) | ||
| case noConfigurationFound |
There was a problem hiding this comment.
ExportError only conforms to CustomStringConvertible. ArgumentParser and other callers typically display error.localizedDescription; without LocalizedError, users may see a generic error instead of these messages. Consider conforming to LocalizedError and implementing errorDescription (or bridging through an existing MCSError).
| let brewPackages: [String: String] = [ | ||
| "node": "node", "npx": "node", "npm": "node", | ||
| "python3": "python3", "uvx": "uv", "uv": "uv", | ||
| ] | ||
| return brewPackages[basename] |
There was a problem hiding this comment.
The brew hint mapping is duplicated here and in ManifestBuilder.brewHintCommands, and they already drift (pipx is included in the builder but not in this post-export hint). Consider centralizing this mapping (or at least keeping the same keys) so the preview comments and post-export hints stay consistent.
| yaml.keyValue("description", metadata.description) | ||
| yaml.keyValue("version", metadata.version, quoted: true) | ||
| if let author = metadata.author { | ||
| yaml.keyValue("author", author) |
There was a problem hiding this comment.
Metadata strings like description (user-provided) and author may contain characters that break YAML plain scalars (e.g. : , #, quotes). These should be YAML-quoted/escaped (e.g. by routing through yamlQuote) to ensure the generated manifest always parses.
| yaml.keyValue("description", metadata.description) | |
| yaml.keyValue("version", metadata.version, quoted: true) | |
| if let author = metadata.author { | |
| yaml.keyValue("author", author) | |
| yaml.keyValue("description", metadata.description, quoted: true) | |
| yaml.keyValue("version", metadata.version, quoted: true) | |
| if let author = metadata.author { | |
| yaml.keyValue("author", author, quoted: true) |
| output.sectionHeader("Pack metadata:") | ||
|
|
||
| let defaultID = identifier ?? "my-pack" | ||
| let id = output.promptInline("Pack identifier", default: defaultID) |
There was a problem hiding this comment.
Pack identifier input is not validated/sanitized. Since manifests require identifiers matching ^[a-z0-9][a-z0-9-]*$, an invalid entry here will generate a pack that fails to load. Add validation (reprompt in interactive mode; fail fast in --non-interactive when --identifier is invalid).
| let id = output.promptInline("Pack identifier", default: defaultID) | |
| let idRegex = "^[a-z0-9][a-z0-9-]*$" | |
| var id: String = "" | |
| while true { | |
| let candidate = output.promptInline("Pack identifier", default: defaultID) | |
| if candidate.range(of: idRegex, options: .regularExpression) != nil { | |
| id = candidate | |
| break | |
| } | |
| output.plain("Pack identifier must match /^[a-z0-9][a-z0-9-]*$/ (lowercase letters, digits, and hyphens, and must not start with a hyphen). Please try again.") | |
| } |
| private func listFiles(in directory: URL) -> [DiscoveredFile] { | ||
| let fm = FileManager.default | ||
| guard fm.fileExists(atPath: directory.path), | ||
| let files = try? fm.contentsOfDirectory(at: directory, includingPropertiesForKeys: [.isSymbolicLinkKey]) else { | ||
| return [] | ||
| } |
There was a problem hiding this comment.
listFiles(in:) only checks for broken symlinks but does not filter out subdirectories or non-regular files. Since these results are later copied into the pack as files, this can produce invalid packs or unexpected directory copies. Add an isRegularFile filter (and consider requesting .isRegularFileKey in includingPropertiesForKeys).
| The export wizard discovers your MCP servers, hooks, skills, commands, plugins, CLAUDE.md sections, gitignore entries, and settings — then generates a complete pack directory with `techpack.yaml` and all supporting files. | ||
|
|
||
| **What it handles automatically:** | ||
| - Sensitive env vars (API keys, tokens) are replaced with `__PLACEHOLDER__` tokens and corresponding `prompts:` entries are generated |
There was a problem hiding this comment.
This documentation says sensitive env vars are replaced with __PLACEHOLDER__ tokens, but the actual placeholder format supported by the schema/template engine is __KEY__ (double-underscore + key name). Update this text to avoid generating packs/docs that don't match runtime behavior.
| - Sensitive env vars (API keys, tokens) are replaced with `__PLACEHOLDER__` tokens and corresponding `prompts:` entries are generated | |
| - Sensitive env vars (API keys, tokens) are replaced with `__KEY__`-style tokens (e.g., `__OPENAI_API_KEY__`) and corresponding `prompts:` entries are generated |
| _ server: ConfigurationDiscovery.DiscoveredMCPServer, | ||
| to yaml: inout YAMLRenderer | ||
| ) -> [PromptEntry] { | ||
| let id = "mcp-\(server.name)" |
There was a problem hiding this comment.
writeMCPServerComponent builds the component id from the raw MCP server name. If the server name contains a dot (.) this will violate the manifest rule that raw component IDs must not contain dots (normalization will throw). Consider sanitizing server.name (similar to sanitizeID) when generating the component id (and optionally store the original name under mcp.name).
| yaml.comment("- [ ] Add `supplementaryDoctorChecks:` for health verification") | ||
| yaml.comment("- [ ] Add `configureProject:` script if project-level setup is needed") | ||
| yaml.comment("- [ ] Move `prompts:` section before `components:` for readability") | ||
| yaml.comment("- [ ] Add `placeholders:` to templates that use __PLACEHOLDER__ tokens") |
There was a problem hiding this comment.
The generated YAML TODOs mention __PLACEHOLDER__ tokens, but this codebase's template/placeholder format is __KEY__ (see TemplateEngine.substitute and schema docs). Update the emitted guidance to match the actual placeholder syntax so users don't end up with non-functional placeholders.
| // Env vars — replace sensitive ones with placeholders | ||
| if !server.env.isEmpty { | ||
| let sensitiveNames = server.sensitiveEnvVarNames | ||
| yaml.line(" env:") | ||
| for key in server.env.keys.sorted() { | ||
| let value = server.env[key]! | ||
| if sensitiveNames.contains(key) { | ||
| let placeholder = "__\(key)__" | ||
| yaml.line(" \(key): \(yamlQuote(placeholder))") | ||
| prompts.append(PromptEntry( | ||
| key: key, | ||
| type: "input", | ||
| label: "Enter value for \(key) (used by \(server.name) MCP server)" | ||
| )) | ||
| } else { | ||
| yaml.line(" \(key): \(yamlQuote(value))") | ||
| } |
There was a problem hiding this comment.
The env var export logic in writeMCPServerComponent only redacts variables whose names match a small set of patterns (KEY, TOKEN, SECRET, etc.) and writes all other env values verbatim into techpack.yaml. This means secrets in env vars with non-matching names (e.g., GH_PAT, other custom tokens) will be copied in cleartext into a pack that is intended to be shared or committed, potentially leaking credentials when users trust that "sensitive env vars" are being replaced with placeholders. Consider treating all env vars as placeholders by default (or requiring explicit opt-in to export raw values) and/or significantly tightening the redaction strategy so that generated packs never include secret values by default.
| // Env vars — replace sensitive ones with placeholders | |
| if !server.env.isEmpty { | |
| let sensitiveNames = server.sensitiveEnvVarNames | |
| yaml.line(" env:") | |
| for key in server.env.keys.sorted() { | |
| let value = server.env[key]! | |
| if sensitiveNames.contains(key) { | |
| let placeholder = "__\(key)__" | |
| yaml.line(" \(key): \(yamlQuote(placeholder))") | |
| prompts.append(PromptEntry( | |
| key: key, | |
| type: "input", | |
| label: "Enter value for \(key) (used by \(server.name) MCP server)" | |
| )) | |
| } else { | |
| yaml.line(" \(key): \(yamlQuote(value))") | |
| } | |
| // Env vars — treat all as placeholders to avoid leaking secrets | |
| if !server.env.isEmpty { | |
| yaml.line(" env:") | |
| for key in server.env.keys.sorted() { | |
| let placeholder = "__\(key)__" | |
| yaml.line(" \(key): \(yamlQuote(placeholder))") | |
| prompts.append(PromptEntry( | |
| key: key, | |
| type: "input", | |
| label: "Enter value for \(key) (used by \(server.name) MCP server)" | |
| )) |
bguidolim
force-pushed
the
feature/export-wizard
branch
from
7cef2a4 to
7d2391d
Compare
bguidolim
force-pushed
the
feature/export-wizard
branch
from
7d2391d to
891fff1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if options.includeSettings, let data = config.remainingSettingsData { | ||
| components.append(ExternalComponentDefinition( | ||
| id: "settings", | ||
| displayName: "settings", | ||
| description: "Additional settings (env vars, permissions, etc.)", | ||
| type: .configuration, | ||
| isRequired: true, | ||
| installAction: .settingsFile(source: "config/settings.json") | ||
| )) | ||
| settingsToWrite = data | ||
| } |
There was a problem hiding this comment.
settingsToWrite is written verbatim from remainingSettingsData, which is built from Settings.extraJSON (and can include env with API keys/tokens). Unlike MCP server env, there’s no sensitive-value scrubbing here, so mcs export can leak secrets into config/settings.json in the generated pack. Consider detecting env (and other sensitive keys) in remaining settings and replacing likely-secret values with __KEY__ placeholders + generating corresponding prompts: entries, or defaulting settings export off / forcing explicit confirmation when sensitive keys are present.
| @Argument(help: "Output directory for the generated pack") | ||
| var outputDir: String | ||
|
|
There was a problem hiding this comment.
PR description advertises mcs export [output-dir] (optional output dir), but the command currently declares outputDir as a required positional argument. Either update the implementation to make the argument optional (with a sensible default, similar to mcs sync), or update the PR description/docs to reflect that output-dir is required.
| output.plain(" 1. Review the generated techpack.yaml") | ||
| output.plain(" 2. Test with: mcs pack add \(outputDir)") |
There was a problem hiding this comment.
Post-export instructions print mcs pack add \(outputDir), which may not match the actual written path after URL standardization/normalization (and may be wrong for relative paths). Prefer printing the resolved outputURL.path used for writing so the suggested command is guaranteed to work.
| output.plain(" 1. Review the generated techpack.yaml") | |
| output.plain(" 2. Test with: mcs pack add \(outputDir)") | |
| let standardizedOutputPath = URL(fileURLWithPath: outputDir).standardizedFileURL.path | |
| output.plain(" 1. Review the generated techpack.yaml") | |
| output.plain(" 2. Test with: mcs pack add \(standardizedOutputPath)") |
| for server in config.mcpServers where options.selectedMCPServers.contains(server.name) { | ||
| let id = "mcp-\(server.name)" | ||
|
|
There was a problem hiding this comment.
Component IDs in exported YAML must not contain dots (see ExternalPackManifest.normalized() throwing on dots). MCP server component IDs are built as mcp-\(server.name) without sanitizing the server name, so names containing . will produce an un-normalizable/invalid manifest. Use the same ID sanitization used for other artifacts (e.g. sanitizeID(server.name)) when building the MCP component ID (and consider using the sanitized name consistently for brewHints keying too).
| yaml.keyValue("identifier", manifest.identifier) | ||
| yaml.keyValue("displayName", manifest.displayName, quoted: true) | ||
| yaml.keyValue("description", manifest.description) | ||
| if let author = manifest.author { | ||
| yaml.keyValue("author", author) | ||
| } |
There was a problem hiding this comment.
The YAML renderer writes several user-controlled strings without quoting (e.g. description, author). If these contain YAML-significant characters like # or : , the generated techpack.yaml can become invalid or be parsed differently than intended. Consider applying yamlQuote automatically for all string scalar values in keyValue(), or at least for description/author here.
| yaml.line(" - id: \(comp.id)") | ||
| yaml.line(" description: \(comp.description)") | ||
|
|
||
| // isRequired | ||
| if comp.isRequired == true { | ||
| yaml.line(" isRequired: true") | ||
| } | ||
|
|
||
| // hookEvent | ||
| if let hookEvent = comp.hookEvent { | ||
| yaml.line(" hookEvent: \(hookEvent)") | ||
| } else if comp.type == .hookFile { | ||
| yaml.comment(" TODO: Add hookEvent (e.g. SessionStart, PreToolUse, Stop)", indent: 4) | ||
| } | ||
|
|
||
| // Install action → shorthand key (exhaustive switch = compile-time safety) | ||
| switch comp.installAction { | ||
| case .mcpServer(let config): | ||
| yaml.line(" mcp:") | ||
| if config.transport == .http, let url = config.url { | ||
| yaml.line(" url: \(url)") | ||
| } else { | ||
| if let command = config.command { | ||
| yaml.line(" command: \(command)") |
There was a problem hiding this comment.
Component fields are emitted without quoting (e.g. description: \(comp.description) and HTTP url: \(url)). If any of these contain # (common in URL fragments) or other YAML-significant sequences, the exported YAML can be invalid or truncated. Reuse yamlQuote for these scalar values (and consider quoting the URL unconditionally).
bguidolim
force-pushed
the
feature/export-wizard
branch
from
891fff1 to
c93d4df
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private func discoverFiles(in hooksDir: URL, hookCommands: [String: String]?, into config: inout DiscoveredConfiguration) { | ||
| let fm = FileManager.default | ||
| guard fm.fileExists(atPath: hooksDir.path), | ||
| let files = try? fm.contentsOfDirectory(at: hooksDir, includingPropertiesForKeys: nil) else { | ||
| return | ||
| } | ||
|
|
||
| let commandToEvent = hookCommands ?? [:] | ||
|
|
||
| for file in files.sorted(by: { $0.lastPathComponent < $1.lastPathComponent }) { | ||
| let filename = file.lastPathComponent | ||
| guard !filename.hasPrefix(".") else { continue } | ||
|
|
||
| // Try to match this file to a hook event via settings commands | ||
| var matchedEvent: String? | ||
| for (command, event) in commandToEvent { | ||
| if command.contains(filename) { | ||
| matchedEvent = event | ||
| break | ||
| } | ||
| } | ||
|
|
||
| config.hookFiles.append(DiscoveredFile( | ||
| filename: filename, | ||
| absolutePath: file, | ||
| hookEvent: matchedEvent | ||
| )) | ||
| } |
There was a problem hiding this comment.
Hook file discovery does not filter out broken symlinks, unlike listFiles(in:) used for skills/commands. If a hook entry is a broken symlink, export will add it to config.hookFiles and PackWriter will later fail when copying. Consider adding the same symlink validation here (skip broken symlinks, and/or warn).
| if nonInteractive { | ||
| metadata = ManifestBuilder.Metadata( | ||
| identifier: identifier ?? "exported-pack", | ||
| displayName: identifier?.replacingOccurrences(of: "-", with: " ").capitalized ?? "Exported Pack", | ||
| description: "Exported Claude Code configuration", | ||
| author: gitAuthorName() | ||
| ) |
There was a problem hiding this comment.
mcs export accepts an arbitrary identifier (flag or prompt) but doesn't validate/sanitize it against the schema requirements (^[a-z0-9][a-z0-9-]*$). This can generate a pack that fails ExternalPackManifest.validate() when users try to add/sync it. Consider validating (or auto-sanitizing) the identifier before building/writing the manifest, and re-prompting / erroring with a clear message when invalid.
- New `mcs export` command reads live Claude Code configuration (MCP servers, hooks, skills, commands, plugins, settings, CLAUDE.md, gitignore) and generates a complete pack directory with techpack.yaml and supporting files - Sensitive env vars auto-replaced with __PLACEHOLDER__ tokens and prompts - Updated architecture, schema, and creating-tech-packs docs
- Refactor ManifestBuilder into two-phase pipeline: buildManifest() constructs typed ExternalPackManifest (compile-time schema coupling via exhaustive switch), renderYAML() serializes to shorthand YAML - Add ManifestBuilderTests with full round-trip validation (build → YAML → load → normalize → validate), empty config, and typed manifest assertions - Fix brew detection to use single-hop symlink resolution (npx now correctly resolves to node), deduplicate hook/skill/command blocks, introduce BuildOptions struct, and replace BrewHint array with dictionary
- Remove version from Metadata, DiscoveredClaudeSection, manifest constructor, YAML rendering, interactive selector, and all test assertions - Remove peerDependencies from manifest constructor (deleted in #179)
- Move brew formula detection from ManifestBuilder to Homebrew.detectFormula() static method, reuse in ExportCommand post-export hints - Replace hardcoded command-to-formula map in ExportCommand with dynamic symlink resolution - Add Homebrew.allPrefixes constant for cross-architecture brew path lookup - Remove unused startService, stopService, isServiceRunning methods
bguidolim
force-pushed
the
feature/export-wizard
branch
from
c93d4df to
5268284
Compare
Summary
Adds
mcs export— an interactive wizard that reads live Claude Code configuration and generates a complete, shareable tech pack directory. This is the inverse ofmcs sync: instead of reading a manifest and installing artifacts, it reads installed artifacts and generates a manifest. Closes #68.Changes
ExportCommand(mcs export [output-dir]) with--global,--dry-run,--non-interactive,--identifierflagsConfigurationDiscoveryreads live config files (~/.claude.json, settings,.claude/dirs, CLAUDE.md, gitignore) and produces aDiscoveredConfigurationmodelManifestBuilderconverts selected artifacts into shorthand YAML with ordered metadata, section comments, TODO checklist, brew dependency hints, and sensitive env var →__PLACEHOLDER__replacementPackWriterwrites output directory (techpack.yaml + copied files with symlink resolution + config/ + templates/)ExportCommandinCLI.swiftdocs/creating-tech-packs.md: replaced "Planned: Auto-export" with actualmcs exportdocumentationdocs/techpack-schema.md: added tip linking tomcs exportdocs/architecture.md: added Export System section andExport/to package structureTest plan
swift testpasses locallymcs export /tmp/test-pack --dry-run --non-interactive --globalproduces valid YAML previewmcs export /tmp/test-pack --non-interactive --globalgenerates pack directory with techpack.yaml and supporting filesmcs pack add --preview /tmp/test-packvalidates the exported packChecklist for engine changes
CLAUDE.md,docs/,techpack.yamlschema inExternalPackManifest.swift)