Add configuration file and session-default persistance #191
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
cameroncooke
force-pushed
the
01-28-add_configuration_file_and_session-default_persistance
branch
from
e8db325 to
2d9923d
Compare
| .string() | ||
| .default('') | ||
| .describe('Comma-separated list of workflows to load at startup.'), | ||
| sentryDisabled: z.boolean().default(false).describe('Disable Sentry error reporting.'), |
There was a problem hiding this comment.
Smithery sentryDisabled option silently ignored after removal
Medium Severity
The sentryDisabled option was removed from the Smithery config schema without any warning or migration path. Zod's default behavior strips unknown keys, so Smithery users who configured sentryDisabled: true to opt out of telemetry will have their setting silently ignored—Sentry will be enabled even though they explicitly requested it be disabled. The old code set XCODEBUILDMCP_SENTRY_DISABLED env var based on this config option; now that mapping is gone with no replacement in the Smithery path.
There was a problem hiding this comment.
This is a false positive. The sentryDisabled option was intentionally removed from Smithery config because Sentry must be initialized before the config store is loaded in order to capture startup crashes. Deferring Sentry initialization would lose visibility into early bootstrap failures. Users can still disable Sentry via the XCODEBUILDMCP_SENTRY_DISABLED environment variable.
cameroncooke
force-pushed
the
01-28-add_configuration_file_and_session-default_persistance
branch
from
2d9923d to
1667473
Compare
cameroncooke
force-pushed
the
01-27-feat_xcodebuildmcp_add_support_for_session-default_persistance
branch
from
626a80b to
a754339
Compare
cameroncooke
force-pushed
the
01-28-add_configuration_file_and_session-default_persistance
branch
from
1667473 to
07529f9
Compare
cameroncooke
force-pushed
the
01-28-add_configuration_file_and_session-default_persistance
branch
from
07529f9 to
92dbdc9
Compare
WalkthroughThis pull request implements a runtime configuration system that transitions from environment-variable-based configuration to a file-based approach using 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/smithery.ts (1)
10-44: Config defaults unintentionally override project configuration.Because
debugandincrementalBuildsEnabledare defaulted tofalse, they are always included in the overrides object passed toinitConfigStore. SinceresolveFromLayersprioritises overrides before checking project config, project-level settings for these fields cannot take effect unless Smithery explicitly provides them. Consider making these fields optional and only adding them to overrides when explicitly provided.Proposed fix
export const configSchema = z.object({ - incrementalBuildsEnabled: z - .boolean() - .default(false) - .describe('Enable incremental builds via xcodemake (true/false).'), - enabledWorkflows: z - .string() - .default('') - .describe('Comma-separated list of workflows to load at startup.'), - debug: z.boolean().default(false).describe('Enable debug logging.'), + incrementalBuildsEnabled: z + .boolean() + .optional() + .describe('Enable incremental builds via xcodemake (true/false).'), + enabledWorkflows: z + .string() + .optional() + .describe('Comma-separated list of workflows to load at startup.'), + debug: z.boolean().optional().describe('Enable debug logging.'), }); -function parseEnabledWorkflows(value: string): string[] | undefined { - const normalized = value +function parseEnabledWorkflows(value?: string): string[] | undefined { + if (!value) return undefined; + const normalized = value .split(',') .map((name) => name.trim().toLowerCase()) .filter(Boolean); return normalized.length > 0 ? normalized : undefined; } function buildOverrides(config: SmitheryConfig): RuntimeConfigOverrides { - const overrides: RuntimeConfigOverrides = { - incrementalBuildsEnabled: config.incrementalBuildsEnabled, - debug: config.debug, - }; + const overrides: RuntimeConfigOverrides = {}; + if (config.incrementalBuildsEnabled !== undefined) { + overrides.incrementalBuildsEnabled = config.incrementalBuildsEnabled; + } + if (config.debug !== undefined) { + overrides.debug = config.debug; + } const enabledWorkflows = parseEnabledWorkflows(config.enabledWorkflows); if (enabledWorkflows) { overrides.enabledWorkflows = enabledWorkflows; } return overrides; }
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 6-14: Fix the minor grammar and typo issues in the CHANGELOG
entry: change "This a change" to "This is a change", change "requires the user
to opt-in" to "requires the user to opt in", replace "The reason the simulator
workflow is the default is because" with "The reason the simulator workflow is
the default is that" (or reword to "This is because" if preferred), correct
"self explanatory" to "self-explanatory", and fix the typo "redune" to "reduce";
apply these edits to the paragraph that begins with "By default when the
`enabledWorkflows`..." and the subsequent sentence about tool names and
descriptions.
In `@docs/CONFIGURATION.md`:
- Around line 91-99: Edit the CONFIGURATION.md text that describes session-aware
opt-out to correct grammar, spelling, and phrasing: change "At time of writing
neither Cursor, Claude Code or Codex support tools changed notifications." to
"At the time of writing, neither Cursor, Claude Code, nor Codex support
tool-changed notifications."; fix "explictly" to "explicitly"; ensure the config
key/flag references remain exact (disableSessionDefaults and
XCODEBUILDMCP_DISABLE_SESSION_DEFAULTS=true); rephrase the last sentence for
clarity/readability to something like "This is not recommended because it
increases token usage per call; enable it only if build, device, and simulator
settings must change frequently within a single coding session (for example,
monorepos with multiple projects)." Ensure parallel structure and consistent
punctuation in the related paragraphs (also update the similar text at the other
occurrences referenced).
- Around line 20-25: The YAML example for enabledWorkflows is currently a single
quoted string; change enabledWorkflows to be a YAML sequence of strings so each
workflow is a separate item (e.g., enabledWorkflows: [ "simulator",
"ui-automation", "debugging" ] or multiline with a dash list). Locate the
example block containing enabledWorkflows and update its value to a proper list
of strings, preserving other keys like schemaVersion,
experimentalWorkflowDiscovery, disableSessionDefaults, and
incrementalBuildsEnabled.
- Around line 54-61: The README currently contradicts itself about default
behavior; update the text so it consistently states that XcodeBuildMCP loads
only the default "simulator" workflow when no configuration is provided: replace
the opening sentence "By default, XcodeBuildMCP loads all workflows at startup."
with a clear statement like "By default, XcodeBuildMCP loads only the default
'simulator' workflow at startup." Keep the rest explaining configuration via
enabledWorkflows (in config.yaml) and the XCODEBUILDMCP_ENABLED_WORKFLOWS env
var (comma-separated), and ensure the Notes section references enabledWorkflows
and XCODEBUILDMCP_ENABLED_WORKFLOWS and the default "simulator" behavior to
remove the contradiction.
- Around line 124-129: Replace the bare URL link to the AXe GitHub repo with a
descriptive link label (e.g., change the line "For more information about AXe
see:
[https://github.com/cameroncooke/axe](https://github.com/cameroncooke/axe)." to
use a labeled link like "For more information about AXe see: [AXe GitHub
repository](https://github.com/cameroncooke/axe).") so markdownlint no longer
flags a bare URL; update the "AXe" reference in the same paragraph accordingly.
In `@docs/dev/PROJECT_CONFIG_PLAN.md`:
- Around line 120-122: In the "Smithery overrides" note update the sentence
"Pass overrides into bootstrap/config store so Smithery is highest precedence."
by inserting a comma before "so" for readability; edit the text under the
heading (reference: Smithery overrides, File: src/smithery.ts) to read "Pass
overrides into bootstrap/config store, so Smithery is highest precedence." and
save the change.
In `@docs/GETTING_STARTED.md`:
- Around line 37-42: Add a language tag to the fenced code block that shows the
config path in the "Project config (optional)" section of GETTING_STARTED.md:
replace the opening triple-backtick for the block containing
"<workspace-root>/.xcodebuildmcp/config.yaml" with "```text" so the block
becomes a fenced "text" code block to satisfy markdownlint MD040 and keep
formatting consistent.
In `@example_projects/iOS/.xcodebuildmcp/config.yaml`:
- Around line 3-5: The config uses a hardcoded absolute projectPath and
simulatorId which break portability; update the projectPath key to a relative
path (e.g., set projectPath to "./MCPTest.xcodeproj") and remove the simulatorId
entry (or replace it with a more portable simulatorName key such as
simulatorName: "iPhone 14") so other developers can clone and run without
machine-specific UUIDs; ensure scheme remains unchanged (scheme: MCPTest) and
validate that any code reading projectPath/simulatorId supports a relative path
or the new simulatorName.
In `@src/server/bootstrap.ts`:
- Around line 33-44: The code sets overrides.enabledWorkflows to [] whenever
options has an own property 'enabledWorkflows' even if it's undefined, which can
unintentionally clear existing configOverrides; change the logic so you only
create or mutate overrides when options.enabledWorkflows is explicitly defined.
Specifically, update the hasLegacyEnabledWorkflows check or add a guard to
require options.enabledWorkflows !== undefined before doing overrides ??= {} and
assigning overrides.enabledWorkflows; reference the symbols
hasLegacyEnabledWorkflows, overrides, options.enabledWorkflows, and
configOverrides to locate and modify the conditional so undefined does not
override existing values.
🧹 Nitpick comments (5)
config.example.yaml (1)
6-12: XOR options both populated may cause confusion or validation errors.The example shows both
projectPathandworkspacePath, as well as bothsimulatorNameandsimulatorId. While the comments indicate these are mutually exclusive (XOR), users copying this file verbatim may encounter validation errors.Consider either commenting out one of each XOR pair or adding a clearer note at the top explaining that users must choose one or the other.
Suggested improvement
sessionDefaults: - projectPath: "./MyApp.xcodeproj" # xor workspacePath - workspacePath: "./MyApp.xcworkspace" # xor projectPath + projectPath: "./MyApp.xcodeproj" # Use this OR workspacePath (not both) + # workspacePath: "./MyApp.xcworkspace" # Uncomment to use workspace instead of project scheme: "MyApp" configuration: "Debug" - simulatorName: "iPhone 16" # xor simulatorId - simulatorId: "<UUID>" # xor simulatorName + simulatorName: "iPhone 16" # Use this OR simulatorId (not both) + # simulatorId: "<UUID>" # Uncomment to use specific simulator UUIDsrc/utils/config-store.ts (2)
77-83: RenamehasOwnPropertyto avoid shadowing the global.The function name shadows
Object.prototype.hasOwnProperty, which can cause confusion about the origin of the identifier.♻️ Suggested rename
-function hasOwnProperty<T extends object, K extends PropertyKey>( +function hasOwn<T extends object, K extends PropertyKey>( obj: T | undefined, key: K, ): obj is T & Record<K, unknown> { if (!obj) return false; return Object.prototype.hasOwnProperty.call(obj, key); }Then update all usages from
hasOwnProperty(...)tohasOwn(...).
444-447: Consider preserving the originalenvreference when re-resolving config.When
persistSessionDefaultsPatchre-resolves the configuration, it doesn't pass anenvparameter toresolveConfig, meaning it will default to reading fromprocess.envat call time. If the originalinitConfigStorewas called with a customenv(e.g., in tests), this could lead to inconsistent resolution.Consider storing the
envreference instoreStateand passing it through:♻️ Suggested improvement
type ConfigStoreState = { initialized: boolean; cwd?: string; fs?: FileSystemExecutor; overrides?: RuntimeConfigOverrides; + env?: NodeJS.ProcessEnv; fileConfig?: ProjectConfig; resolved: ResolvedRuntimeConfig; };Then in
initConfigStore:storeState.overrides = opts.overrides; + storeState.env = opts.env;And in
persistSessionDefaultsPatch:storeState.resolved = resolveConfig({ fileConfig: storeState.fileConfig, overrides: storeState.overrides, + env: storeState.env, });src/mcp/tools/doctor/doctor.ts (1)
226-230: Update warning message to reflect config file option.The warning still only mentions the environment variable approach. Since this PR introduces config file support as the preferred method, consider updating the guidance:
♻️ Suggested update
...(dapSelected && !lldbDapAvailable ? [ - `- Warning: DAP backend selected but lldb-dap not available. Set XCODEBUILDMCP_DEBUGGER_BACKEND=lldb-cli to use the CLI backend.`, + `- Warning: DAP backend selected but lldb-dap not available. Set debuggerBackend: lldb-cli in .xcodebuildmcp/config.yaml or XCODEBUILDMCP_DEBUGGER_BACKEND=lldb-cli to use the CLI backend.`, ] : []),src/utils/axe-helpers.ts (1)
83-87: Consider updating the source label for accuracy.The
source: 'env'label is now semantically inaccurate since the value comes from the config store (which may source from a config file, not just environment variables). This could cause confusion in diagnostic output or logging that uses this source indicator.Consider either:
- Renaming the source to
'config'and updatingAxeBinarySourcetype- Documenting that
'env'represents "user-configured" paths (both env vars and config file)♻️ Suggested clarification
-export type AxeBinarySource = 'env' | 'bundled' | 'path'; +export type AxeBinarySource = 'config' | 'bundled' | 'path';const configPath = resolveAxePathFromConfig(); if (configPath) { - return { path: configPath, source: 'env' }; + return { path: configPath, source: 'config' }; }
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Previously when enabledWorkflows was empty or not set, all workflows would be loaded. The documentation stated only the simulator workflow should load by default, but the code did the opposite. This aligns the implementation with the documented breaking change: - Default to simulator workflow when enabledWorkflows is empty - Remove the ability to load all workflows implicitly - Users must explicitly list all desired workflows to load them
Changed absolute path to relative path for portability across different development machines.
Matches config-store.ts behavior - no tri-state handling. Empty input normalizes to [] which triggers default simulator workflow.
Array items should be separate strings, not a single comma-separated string.
- CHANGELOG.md: "This a change" -> "This is a change" - CHANGELOG.md: "redune" -> "reduce" - CHANGELOG.md: "self explanatory" -> "self-explanatory" - CHANGELOG.md: "opt-in" -> "opt in" (verb form) - CHANGELOG.md: Simplify "is because" redundancy - CONFIGURATION.md: "At time of writing" -> "At the time of writing" - CONFIGURATION.md: "or Codex" -> "nor Codex" - CONFIGURATION.md: "explictly" -> "explicitly" - CONFIGURATION.md: "overridens" -> "overrides" - CONFIGURATION.md: Use descriptive link text for AXe URL - PROJECT_CONFIG_PLAN.md: Add comma before "so" - GETTING_STARTED.md: Add language tag to fenced code block
bootstrapServer already calls initConfigStore internally, so the explicit call in smithery.ts was redundant and could cause issues. Now Smithery matches main entry point: initSentry -> bootstrapServer.
Smithery entry point now behaves identically to index.ts: - No CLI flag config options - All configuration via config.yaml and env vars - Same precedence: config.yaml > env vars > defaults This aligns with the vision of config.yaml as canonical truth. Also removes unused xcodemake imports from index.ts.
commit: |
cameroncooke
deleted the
01-28-add_configuration_file_and_session-default_persistance
branch
TL;DR
Comprehensive overhaul of XcodeBuildMCP configuration system, introducing a unified config file approach with layered precedence and improved documentation.
What changed?
.xcodebuildmcp/config.yamlfiles for runtime configurationenabledWorkflowsis not set, only loads thesimulatorworkflow (breaking change)How to test?
.xcodebuildmcp/config.yamlfile in your project root with desired settingsWhy make this change?
This change provides a more robust and flexible configuration system that:
The simulator workflow is now the default because it's the most common use case based on analytics data, requiring users to explicitly opt-in to additional workflows as needed.
Note
Medium Risk
Medium risk because it changes startup workflow selection defaults (now defaults to only
simulator) and centralizes many runtime settings behind a new config-store, which can affect tool availability and behavior across the server.Overview
Adds a new project-scoped config file (
.xcodebuildmcp/config.yaml) and a globalconfig-storethat resolves runtime settings with layered precedence (overrides > config file > env > defaults) and supports persistingsessionDefaultspatches.Updates server boot to initialize the config store, seed
sessionStorefrom the file, and changes workflow selection behavior so missing/emptyenabledWorkflowsdefaults to onlysimulator(plus required/auto workflows likesession-management,workflow-discovery,doctor).Refactors multiple subsystems (workflow selection, debugger backend + DAP tuning, log capture wait, template overrides, AXe path, incremental builds opt-in) to read configuration via
getConfig(); updates tool metadata text (e.g., log capture stop tools) and adjusts tests/docs with new config examples and guidance.Written by Cursor Bugbot for commit 52a0b3a. This will update automatically on new commits. Configure here.