Updating from MistKit v1.0.0-alpha.5

[leogdion]

No description provided.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b812b8f7-4bf5-42d3-890c-f44d94d15b2c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mistkit-v1.0.0.alpha.2

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review

PR: Updating from MistKit v1.0.0-alpha.5
State: Draft — notes below reflect that some items may be intentionally in progress.

---

Overview

This PR switches the MistKit dependency from a pinned remote release (1.0.0-alpha.3) to a local path dependency (../..) for active development, adds a reusable setup-mistkit composite action to patch Package.swift in CI, cleans up workflow triggers, fixes a long-standing env-var name typo in README, and improves test idiom for UTF-8 data encoding.

---

✅ Good Changes

Test improvements (VirtualBuddyFetcherTests.swift)

• Replacing .data(using: .utf8)! with Data(string.utf8) is a clear improvement. UTF-8 encoding of a String can never fail, so the force-unwrap was always unnecessary noise. The new form is idiomatic Swift and removes 8 force-unwraps.

README env-var fix

• CLOUDKIT_KEY_FILE → CLOUDKIT_PRIVATE_KEY_PATH in four places is a real bug fix. The wrong name would silently produce "key file not found" errors for anyone following the README. Worth noting that CLAUDE.md already uses the correct name — README was just stale.

Workflow simplification (cloudkit-sync-dev.yml)

• Removing the workflow_run trigger and its conditional guard simplifies the dev-sync workflow meaningfully. The trigger was only useful while the 8-scheduled-job branch was active, so removing it reduces confusion without losing anything.

PR trigger scoping (bushel-cloud-build.yml)

• Adding branches: [main] to the pull_request trigger is the right call. Feature-to-feature PRs shouldn't burn CI minutes on the binary-build workflow.

---

⚠️ Concerns

1. setup-mistkit is called without a branch input in the main CI workflows

In both BushelCloud.yml and codeql.yml, the new step looks like:

- name: Setup MistKit
  uses: ./.github/actions/setup-mistkit

No with: branch: is provided. The action's if: inputs.branch != '' guard means both steps are unconditional no-ops in CI — Package.swift is left with path: "../..". That path doesn't exist in a standalone checkout of BushelCloud, so the subsequent build step would fail on package resolution unless skip-package-resolved: true somehow bypasses it. Please clarify: does swift-build@v1.4.2 with skip-package-resolved: true skip resolution entirely, or does it only skip the lockfile check?

If the path dep really does break CI, the fix is straightforward — pass the target branch:

- name: Setup MistKit
  uses: ./.github/actions/setup-mistkit
  with:
    branch: main   # or whichever branch tracks alpha.5

2. Shell injection risk in setup-mistkit/action.yml

run: |
  sed -i '' 's|...|.package(url: "...", branch: "'"${{ inputs.branch }}"'")|g' Package.swift

${{ inputs.branch }} is interpolated directly into the shell string before the shell sees it. A branch name containing |, \, ', or ; (unlikely but possible) would break the sed pattern or, in a context where the input is untrusted, could allow command injection. The fix is to use an env var:

env:
  MISTKIT_BRANCH: ${{ inputs.branch }}
run: |
  sed -i '' "s|...|.package(url: \"...\", branch: \"${MISTKIT_BRANCH}\")|g" Package.swift

This is lower priority since inputs.branch comes from the workflow (not from PR content), but it's a best practice worth following.

3. Local path dep in Package.swift should not land on main

Package.swift with path: "../.." only resolves correctly when BushelCloud is checked out inside MistKit's Examples/Bushel/ directory. Anyone who clones BushelCloud standalone and runs swift build without going through the setup-mistkit action will get an immediate resolution error. The intent seems to be that this stays as a development convenience (consistent with the "git subrepo" approach in CLAUDE.md), but it should be clearly gated — either:

• Don't merge this to main until a stable 1.0.0-alpha.5 tag is cut and the dep reverts to a versioned URL, or
• Add a // Dev only — CI rewrites this comment so the intent is obvious to contributors.

4. cloudkit-sync/action.yml — Setup MistKit also has no branch

Same as concern #1. The new step in the binary-build fallback path:

- name: Setup MistKit
  if: steps.download-binary.outcome != 'success'
  uses: ./.github/actions/setup-mistkit

…is also a no-op without a branch input, so the fallback build would hit the same path-dep resolution failure.

---

Minor Nits

• The inputs.branch description says "leave empty to keep the local path dependency" — that phrasing implies keeping the local dep is a valid CI scenario, which it isn't for standalone checkouts. Consider "leave empty to skip (no-op)" or add a required: true if it should always be provided.
• Package.resolved no longer pins MistKit (removed). That's correct for a path dep, but worth confirming the originHash change doesn't reflect any other unintended dependency churn.

---

Summary

The test and README fixes are clean and should ship. The CI setup around setup-mistkit needs a branch value wired in before the workflows will actually work, and the shell-injection nit in the sed command is a quick fix. Given the draft status, I'd expect these to be addressed before requesting a review.