Extract MistKitConfiguration; converge all three examples on typed config keys - #455
Merged
Conversation
… CLI flags
Delete the hand-rolled `read(_:)` glue duplicated in BushelCloud and
CelestraCloud in favor of ConfigKeyKit 1.0.0-beta.2's `ConfigValueReading`,
whose protocol extension supplies every overload both files declared. Four of
them were character-for-character identical between the two examples.
Standardize Bushel's ConfigKey bases on dash-case, matching Celestra.
`CLIKeyEncoder` joins a key's components verbatim, so snake_case bases
generated `--cloudkit-key_id` while Bushel's own error messages,
`secretsSpecifier` and docs all advertise `--cloudkit-key-id`. The documented
flags therefore never resolved, and secret redaction never matched, so a
private key passed by flag was not marked secret. ENV names are unchanged
(the environment provider normalizes `-` and `.` to `_`), so deployments and
CI are unaffected.
Fail closed on an unparseable CLOUDKIT_ENVIRONMENT in Celestra, which
previously degraded silently to .development, matching Bushel and MistDemo.
Expose Bushel's loader test seam unconditionally and add the same to
Celestra, whose loader was previously untestable without mutating process
environment.
Rebuild the Bushel test double on the real providers instead of
InMemoryProvider. The double matched keys literally and served only the
stored case, so it diverged from production on key normalization and numeric
coercion. It also stored bare flags as `.string("true")`, masking that
valueless flags never resolved through the real CLI provider — a gap that
predates this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Converge BushelCloud and CelestraCloud on one CloudKit config shape, so the shared surface can be extracted into a package. Breaking for both subrepos. Replace Bushel's `CloudKitAuthMethod` with MistKit's `PrivateKeyMaterial`, which it re-declared. `ValidatedCloudKitConfiguration` now carries a single `privateKey: PrivateKeyMaterial` instead of a `privateKeyPath` plus optional `privateKey` plus an empty-string sentinel, collapsing the two-branch auth resolution duplicated across five Bushel command sites. Migrate both examples off the legacy `ServerToServerAuthManager` path onto `CloudKitService(containerIdentifier:credentials:environment:)`. MistKit defers reading a `.file(path:)` key until the credentials are consumed, so construction no longer does file IO. Celestra gains the inline-PEM path it lacked, which is what CI needs to avoid writing a temporary key file. Share PEMValidator and KeyIDValidator with Celestra, which validated neither. They throw a new app-neutral `CredentialValidationError`, so both copies are byte-identical. A Server-to-Server key ID is 64 hex characters; correct .env.example, SECRETS_SETUP.md and CLOUDKIT_SYNC_SETUP.md, which documented 32. Unify the configuration error on one `ConfigurationError` conforming to LocalizedError. Bushel's was a bare Error whose message never reached users. Delete `CelestraConfig`, whose sole factory is now `ValidatedCloudKitConfiguration.makeCloudKitService()`. BushelCloudKitService.swift drops from 314 to 268 lines, clearing a file_length lint error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Captures PR 1 (unify, done and unpushed), the push/merge commands, and what PR 2 (extract into brightdigit/MistKitConfiguration) still needs — including the repo creation and subrepo pushes left as user actions. Records why the issue changed shape: most of #407's premises were stale (the ConfigKeyKit#1 blocker resolved in-core, and the Environment/credential/factory targets already exist in MistKit), so the work was reordered to converge the examples first and extract second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings MistDemo onto the typed-key pattern PR 1 gave BushelCloud and CelestraCloud, so all three examples share one key convention before the shared surface is extracted into MistKitConfiguration. The five CloudKit credential keys adopt the names the package will own (`cloudkit.container-id`, `cloudkit.key-id`, `cloudkit.private-key`, `cloudkit.private-key-path`, `cloudkit.environment`). Every other key keeps its historical base and gains `envPrefix: "CLOUDKIT"`, faithfully reproducing the blanket `prefixKeys(with: "cloudkit")` the provider stack applied to the whole key space — so no other flag or variable moves. Environment variables: four unchanged, one repaired, none broken. `container.identifier` resolved to CLOUDKIT_CONTAINER_IDENTIFIER, which nothing ever set; MistDemo-Integration.yml supplies CLOUDKIT_CONTAINER_ID on all four jobs and docs/cloudkit-guide documents that name, so the value CI passed was silently ignored and every run fell back to the built-in default. It only went unnoticed because that default equals the secret's value. CLI: five credential flags gain a `cloudkit-` prefix. CI passes credentials by environment, not flags, so no workflow changes are needed. Fixes bare boolean flags. `optionalBool` probed `string(forKey:)` first, and swift-configuration surfaces a valueless flag only through `bool(forKey:)`, so `--zone-wide`, `--numbers-as-strings` and `--fetch-root-record` only worked as `--flag true`. Measured: string(forKey:"verbose") is nil while bool(forKey:"verbose") is true. ConfigKeyKit's own resolvedBool has the same flaw, so MistDemoConfiguration keeps a Bool path over `bool(forKey:)` — that both fixes the three broken sites and avoids regressing the twelve `bool(forKey:default:)` flags that already worked. output.format collapses from 25 sites with three different defaults to one key defaulting to `table`; `--output-format json` opts back in. No CI step parses command output, and all four formats have renderers. Also: deletes the 30-constant ConfigKeys enum (now fully dead) and Defaults.database, which said "private" while the runtime default was "public" and was never read; rebuilds the test doubles on the real environment provider keyed by typed keys, which surfaced that MistDemoConfig+Testing seeded `private.key.file` — a key production never read, so `privateKeyFile:` silently did nothing; removes README claims about a `--config-file` that no provider implements. 1010 tests / 299 suites pass (1002 baseline + 8 new pinning the env-var names, dash-case bases, secret flags and bare-flag resolution). SwiftLint is at parity: two pre-existing errors in untouched files, two warnings resolved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd
Extracts the CloudKit credential configuration surface that BushelCloud and CelestraCloud converged on in PR 1 into its own package, scaffolded from the ConfigKeyKit repo template. Lives at Packages/MistKitConfiguration so it can be co-developed with MistKit and pushed to brightdigit/MistKitConfiguration as a subrepo. MistKit's own Package.swift is untouched — the dependency arrow points one way, which is why this is a separate repo rather than a product. Errors are generic and identifiable, carrying no prose. CloudKitConfigurationError, KeyIDValidationFailure and PEMValidationFailure are Equatable enums that deliberately do not conform to LocalizedError: all three consumers already own an error type with its own wording, remediation advice and key names, so package-authored text would contradict every one of them. Errors name a CloudKitConfigurationField rather than a key string, because the same field is spelled differently per application; CloudKitConfigurationKeys.subscript(_:) maps a field back to that application's own key. This retires CredentialValidationError, whose two cases each carried reason/suggestion strings that flattened four distinct key-ID failures into one unswitchable case and hardcoded CLOUDKIT_KEY_ID into a package that cannot know it. ValidatedCloudKitConfiguration's memberwise initializer is throwing and runs both validators, so no value of that type can exist whose credentials skipped format checking — the property that lets callers delete hand-rolled checks. validated() checks presence before format and prefers an inline PEM over a path. Reading stays non-throwing so it composes into any application's loader. secretCommandLineFlags is derived from each key's isSecret rather than hand-listed, structurally preventing the drift that previously let a private key passed by flag be logged unredacted; a regression test asserts it. Package.swift uses .package(name: "MistKit", path: "../..") rather than a tagged url:. Verified empirically: a path package takes its identity from the directory name, so pairing it with a sibling depending on MistKit by url: resolves two distinct packages and fails with "multiple similar targets 'MistKit', 'MistKitOpenAPI'". swift package resolve still succeeds, so only a build catches it. That line is a monorepo-local overlay; the standalone repo carries the url: form, swapped by setup-mistkit in CI. CI follows CelestraCloud rather than ConfigKeyKit because tools-version 6.4 has no Linux or Windows release toolchain (verified: Docker Hub lists 88 swift:6.2 tags, 83 swift:6.3, zero swift:6.4). Ubuntu runs the single nightly-6.4.x entry, Windows is commented out, Android is omitted, macOS runs on xcode-27, and dependency-policy.yml gates main on tagged dependencies. 35 tests in 6 suites pass; swift-format and SwiftLint are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd
ConfigKeyKit resolved booleans through `string(forKey:)`, which cannot see a valueless command-line flag, and coerced unrecognized environment values to false instead of ignoring them. Fixed in brightdigit/ConfigKeyKit#8; pin by revision until a release carrying it is tagged. Verified the pin coexists with the `from: "1.0.0-beta.2"` requirement the three examples declare: SwiftPM resolves the revision for the whole graph, so once they depend on MistKitConfiguration they inherit the fix without editing their own ConfigKeyKit line. Note the revision requirement is deliberately incompatible with dependency-policy.yml, which rejects non-tagged dependencies on PRs to main — that gate is what will force the swap to a tagged release before merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v1.0.0-beta.5 #455 +/- ##
=================================================
- Coverage 81.83% 79.76% -2.07%
=================================================
Files 197 207 +10
Lines 4778 4942 +164
=================================================
+ Hits 3910 3942 +32
- Misses 868 1000 +132
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tion.git --branch=initial-extraction Packages/MistKitConfiguration subrepo: subdir: "Packages/MistKitConfiguration" merged: "cc8743f" upstream: origin: "git@github.com:brightdigit/MistKitConfiguration.git" branch: "initial-extraction" commit: "none" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "0942cac2ed"
Points Packages/MistKitConfiguration at brightdigit/MistKitConfiguration (branch initial-extraction), where the package now lives as PR #1. The repository was seeded by hand rather than by `git subrepo push`: it was empty, and GitHub cannot open a PR between unrelated histories, so `main` needed a LICENSE-only initial commit first to give initial-extraction a merge base. `.gitrepo` therefore records an empty `commit =` — the first `git subrepo push` will believe nothing has been pushed. That matters, because the two copies of Package.swift deliberately differ on one line: the monorepo needs `path: "../.."` (a path package takes its identity from the directory name, so a transitive `url:` MistKit resolves as a second package and fails the build), while the standalone repo needs the tagged `url:` or a tag of this package is unusable downstream. `git subrepo push` copies the subdir verbatim and would clobber that line, so both Package.swift and CLAUDE.md now say so at the site, and a memory records it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd
The previous handoff was written before any of this work and is stale in every section: it says nothing is pushed, that the MistKitConfiguration repo does not exist, that PR 2 has not started, and that Swift 6.4 needs a snapshot toolchain. Records what is actually true now: three open PRs and the merge order they force, the nine commits on the branch, Parts 0-2 as landed, and the two hazards that fail silently — `git subrepo push` overwriting the standalone Package.swift overlay, and a `path:` MistKit coexisting with a `url:` one, where `swift package resolve` succeeds and only a build catches it. Also carries forward the boolean correction, since it inverts my earlier reasoning: typed keys do not fix bare flags, and migrating MistDemo naively onto ConfigKeyKit's `read(_:)` would have regressed twelve working flags rather than fixing three. `MistDemoConfiguration.resolveBool` exists for that reason and should not be "simplified" away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd
…istDemo Rewire the three examples onto the shared package (Parts 3–5 of #407), pin ConfigKeyKit to main until it is tagged, and add MistKitConfiguration CI wiring. Co-authored-by: Cursor <cursoragent@cursor.com>
Mirror the standalone package gate so draft integration can pin ConfigKeyKit to branch main until a release tag exists. Co-authored-by: Cursor <cursoragent@cursor.com>
PR #455 targets v1.0.0-beta.5, so branches: [main] never scheduled the MistDemo/Bushel/Celestra/MistKitConfiguration matrix. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
Author
|
Temporarily closing/reopening to re-trigger Actions after synchronize events stopped scheduling PR workflows. |
leogdion
marked this pull request as ready for review
August 31, 2026 18:36
leogdion
marked this pull request as draft
August 31, 2026 18:36
leogdion
marked this pull request as ready for review
August 31, 2026 18:37
Synchronize events after f671c91 stopped scheduling MistKit/MistDemo workflow runs; Package.swift is in both path filters. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve Package.resolved conflicts by regenerating against the MistKitConfiguration path dependency. Unblocks PR Actions that were stuck while the PR was CONFLICTING. Co-authored-by: Cursor <cursoragent@cursor.com>
leogdion
marked this pull request as draft
August 31, 2026 18:39
That package gitignores the lockfile (standalone CI already passes skip-package-resolved); without the flag swift-build --force-resolved-versions fails immediately in the Examples matrix. Co-authored-by: Cursor <cursoragent@cursor.com>
MistDemo now path-depends on MistKitConfiguration (tools 6.4), so 6.2/6.3, Xcode 26.6, Windows, and Android lanes cannot parse the graph. Align MistDemo with MistKitConfiguration's nightly/Xcode 27 matrix, and run MKC lint inside the 6.4 nightly container so `swift build` in lint.sh succeeds. Co-authored-by: Cursor <cursoragent@cursor.com>
ConfigKeyKit 1.0.0-beta.3 ships PR #8 (boolean resolution via a bool primitive), the fix MistKitConfiguration was tracking off `branch: "main"`. Swap the temporary branch pin for the released tag. The pinned revision is unchanged (3c8ae38), so this is a provenance change only — no behavior difference. `dependency-policy.yml` rejects branch and revision requirements on non-draft PRs to `main`, so this is a prerequisite for MistKitConfiguration#1 leaving draft. 35 tests pass on Swift 6.4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`.gitrepo` carried an empty `commit =` because the standalone repo was seeded by hand rather than by `git subrepo push` (GitHub cannot open a PR between unrelated histories, so `main` needed a LICENSE-only base commit first). Record the commit the ConfigKeyKit bump landed on so future subrepo operations have a diff base. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MistDemo reached ConfigKeyKit transitively through MistKitConfiguration, which pinned `branch: "main"`, so the lockfile recorded a floating branch. With MistKitConfiguration on `from: "1.0.0-beta.3"` the same revision (3c8ae38) now locks as a tagged version instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two facts learned while unblocking the MistKitConfiguration release: - A workflow job gated on `draft == false` never runs on the draft → ready transition unless `types:` lists `ready_for_review`. It reports `skipped`, not `failure`, so the gate silently fails open on exactly the PRs it guards. - `git subrepo push` for Packages/MistKitConfiguration would carry the monorepo `path:` MistKit overlay into the standalone repo; push via a clone instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tagging MistKitConfiguration does not mean the Examples switch to `from:`. They dogfood unreleased MistKit, so every in-monorepo package must be reached the same way — `path:` locally, rewritten to a branch-HEAD `revision:` pin in CI by `setup-mistkitconfiguration`, which takes both branch inputs precisely because a `path:` MistKit and a `url:` MistKit cannot coexist. Verified both failure modes: pointing only MistKitConfiguration at its tag reproduces the duplicate-target error, and additionally moving MistKit to 1.0.0-beta.4 makes MistDemo fail to compile on `ZoneType` / `ZoneInfo.deleted` (#444, branch-only). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ndoff doc `.gitrepo` pointed at `initial-extraction`, which was squash-merged as 6df3ad4 — its commits are not on `main`, so a subrepo pull would diff against history that no longer exists. Repoint at `mistkit-beta.5` (a70afee), the integration branch pinning MistKit's unreleased `v1.0.0-beta.5` so MistKitConfiguration can be exercised against #444 before the tag exists. That branch must never merge to `main` or be tagged; `main` stays tag-only for downstream consumers. Delete `.claude/HANDOFF-407.md`: its merge gate is resolved (ConfigKeyKit 1.0.0-beta.3, MistKitConfiguration#1 merged, 1.0.0-beta.1 tagged) and its durable hazards live in `.claude/memory/`. The five unfiled follow-ups it carried are preserved on issue #407 rather than dropped. Also supersedes the `resolveBool` directive — ConfigKeyKit#8 shipped in beta.3, so that simplification is unblocked pending verification. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Packages/MistKitConfiguration` rides the `v1.0.0-beta.5` release branch so MistKit and MistKitConfiguration can be developed together, then comes out in the `v1.0.0-beta.5` → `main` release PR. Keeping it would be circular: a MistKit release carrying a package whose `.gitrepo` tracks a branch pinning that same unreleased release. Records the retirement order so the removal isn't missed at release time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
leogdion
marked this pull request as ready for review
August 31, 2026 22:23
…stra Both lockfiles still recorded ConfigKeyKit as `branch: "main"`, so once MistKitConfiguration moved to `from: "1.0.0-beta.3"` the requirement no longer matched what was pinned. CI resolves with automatic resolution disabled and failed with "an out-of-date resolved file was detected". Same revision (3c8ae38) — only the pin's provenance changes, branch → version. MistDemo was relocked in bc3c671; these two were missed. Verified with `swift build --disable-automatic-resolution` (CI's flag) in all three Examples. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #407 when complete. Draft — Parts 0–2 are done, Parts 3–5 are in progress.
Why this shape
#407 proposed extracting five things into a new package. Verifying each against the code showed most no longer held —
Environmentparsing, the credential types and the service factory are all already public MistKit API, and theConfigKeyKitConfigurationpackage the issue’s diagram assumed never existed (that bridge shipped in ConfigKeyKit’s dependency-free core asConfigValueReading). So the order is inverted: converge the examples first, then extract what is genuinely identical. Extracting first would have packaged code that convergence deletes.What is done
PR 1 work (3 commits, previously unpushed) — BushelCloud and CelestraCloud converged onto MistKit’s
CredentialsAPI and ConfigKeyKit’sConfigValueReading, deleting ~180 lines of character-identical glue.Part 0 — MistDemo onto typed
ConfigKeys (5338914)MistDemo had zero typed keys (Bushel 32, Celestra 12) and read config through an untyped façade at ~160 call sites. The five CloudKit credential keys now use the names the package owns; every other key keeps its base and gains
envPrefix: "CLOUDKIT", reproducing the blanketprefixKeys(with:)the provider stack applied to the whole key space — so no other flag or variable moves.Part 1–2 — the package (
8603e60) atPackages/MistKitConfiguration/, scaffolded from the ConfigKeyKit repo template.ConfigKeyKit fix + pin (
cc8743f) — see below.Bugs this fixes
CLOUDKIT_CONTAINER_IDnever worked.MistDemo-Integration.ymlsets it on all four jobs anddocs/cloudkit-guide/documents it, but the code readCLOUDKIT_CONTAINER_IDENTIFIER. Every integration run silently fell back to the built-in default — masked only because that default equals the secret’s value. Point it at a different container and it was ignored.--flagread as its default and--flag falseread astrue, while an unrecognized env value collapsed tofalseinstead of being ignored. Fixed in Resolve booleans via a bool primitive, not string parsing ConfigKeyKit#8 and pinned here by revision. BushelCloud alone has ~14 affectedConfigKey<Bool>values.MistDemoConfig+Testingseededprivate.key.file; production readsprivate.key.path, soprivateKeyFile:never reached the parsing path.ConfigKeysenum andDefaults.database, which saidprivatewhile the runtime default waspublicand was never read.Design notes worth review
Equatableenums and deliberately does not conform toLocalizedError: all three consumers already own an error type with its own wording, remediation advice and key names. Errors name aCloudKitConfigurationFieldrather than a key string, because the same field is spelled differently per app. This retiresCredentialValidationError, whosereason/suggestionstrings flattened four distinct key-ID failures into one unswitchable case and hardcodedCLOUDKIT_KEY_IDinto a package that cannot know it.ValidatedCloudKitConfiguration’s initializer is throwing and runs both validators, so no value of that type can exist whose credentials skipped format checking.secretCommandLineFlagsis derived from each key’sisSecret, structurally preventing the drift that let a private key passed by flag be logged unredacted.output.formatcollapses from 25 sites with three different defaults to one key defaulting totable.The MistKit dependency is a
path:, deliberatelyA tagged
url:breaks the monorepo build. Apath:package takes its identity from the directory name, so pairing it with a sibling depending on MistKit byurl:makes SwiftPM resolve two packages and fail withmultiple similar targets MistKit, MistKitOpenAPI.swift package resolvestill succeeds — only a build catches it. Every package in this monorepo must reach MistKit the same way. That line is a monorepo-local overlay; the standalone repo carries theurl:form, swapped bysetup-mistkitin CI.Verification
ConfigReadergit diff origin/v1.0.0-beta.5 -- Sources/ Package.swiftis emptySwiftLint is at parity for MistDemo: two pre-existing errors in files this PR never touches, two warnings resolved.
Remaining
git subrepowiring,setup-mistkitconfiguration, monorepo CI lane, close out Create MistKitConfiguration package for shared CloudKit config glue #407.dependency-policy.ymlgates exactly this.🤖 Generated with Claude Code
https://claude.ai/code/session_01BB4QwYjmEPMC2Fo5HW4cKd