Swiftpkgr, a friendly UI for swiftpkg - #5
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (62)
📝 WalkthroughWalkthroughThis PR extracts shared package logic into a new SwiftPkgCore library, introduces a SwiftUI macOS app (Swiftpkgr) for editing and building package projects, migrates the CLI to use async build APIs, updates the Xcode project/CI accordingly, and adds a signing/notarization release automation script with supporting installer template and documentation updates. ChangesSwiftpkgr Frontend and Shared Core
Signed and Notarized Release Automation
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectEditorView
participant ProjectEditorModel
participant PackageOperationService
participant PackageBuildCoordinator
User->>ProjectEditorView: choose project or trigger build
ProjectEditorView->>ProjectEditorModel: invoke action
ProjectEditorModel->>PackageOperationService: buildPackage(options, reporter)
PackageOperationService->>PackageBuildCoordinator: buildPackage(project, options)
PackageBuildCoordinator-->>PackageOperationService: ConsoleEvent updates / result
PackageOperationService-->>ProjectEditorModel: completion or error
ProjectEditorModel-->>ProjectEditorView: update status, logs, or alert
sequenceDiagram
participant Maintainer
participant PublishScript as publish-xcode-release.sh
participant Xcodebuild
participant Notarytool
participant GitHubCLI
Maintainer->>PublishScript: run --check/--build/--publish
PublishScript->>Xcodebuild: build swiftpkg and Swiftpkgr
PublishScript->>Notarytool: notarize and staple artifacts
PublishScript->>PublishScript: assemble signed .pkg into dist/
PublishScript->>GitHubCLI: create/update release with pkg and SHA256SUMS
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
swiftpkg/Support.swift (1)
56-92: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake process launch atomic with cancellation registration.
Line 70 exposes
activeProcessbefore Line 77 starts it. A concurrentcancel()seesisRunning == false, returns, and the process then launches despite cancellation.Proposed fix
- lock.withLock { activeProcess = process } defer { lock.withLock { if activeProcess === process { activeProcess = nil } } } do { - try process.run() + try lock.withLock { + try process.run() + activeProcess = process + } } catch {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@swiftpkg/Support.swift` around lines 56 - 92, Update SystemProcessRunner.run so process registration and launch are atomic with respect to cancel(), preventing cancellation from observing an unstarted activeProcess and returning before launch. Coordinate the lock around activeProcess assignment and process.run(), while preserving cleanup in the existing defer and ensuring launch errors still clear registration and propagate as MunkiPkgError.
🧹 Nitpick comments (1)
swiftpkg/PackageSettingsDraft.swift (1)
82-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the newly exposed public API surface.
swiftpkg/PackageSettingsDraft.swift#L82-L153: document the defaults and validation conversion contracts.swiftpkg/ProjectOperations.swift#L5-L19: documentProjectCreatorandcreateProject.swiftpkg/ProjectOperations.swift#L49-L66: document the public BOM export and synchronization operations.swiftpkg/PackageImporter.swift#L17-L29: add a concise type-level comment forPackageImporter.swiftpkgCLI/SwiftPkg.swift#L4-L9: documentSwiftPkg.run, including its exit-status contract.As per coding guidelines, “Use fluent, role-based Swift names, concise documentation comments for new nontrivial types and entry points.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@swiftpkg/PackageSettingsDraft.swift` around lines 82 - 153, Document the newly exposed public APIs with concise role-based Swift documentation comments: in swiftpkg/PackageSettingsDraft.swift lines 82-153, document validatedConfiguration() and its defaults and validation-to-PackageConfiguration contract; in swiftpkg/ProjectOperations.swift lines 5-19, document ProjectCreator and createProject; in swiftpkg/ProjectOperations.swift lines 49-66, document the public BOM export and synchronization operations; in swiftpkg/PackageImporter.swift lines 17-29, add a type-level comment for PackageImporter; and in swiftpkgCLI/SwiftPkg.swift lines 4-9, document SwiftPkg.run and its exit-status contract.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 121: Update the documented Swiftpkgr build command in README.md lines
121-121 and docs/SWIFTPKGR_IMPLEMENTATION_PLAN.md lines 379-380 to append
CODE_SIGNING_ALLOWED=NO, matching the unsigned build configuration used by CI
and VERIFICATION.md.
In `@swiftpkg/PackageBuilder.swift`:
- Around line 212-217: Update the timeout handling in
waitForAcceptance(_:configuration:) to replace the direct stderr write with
console.warning(...). Preserve the existing timeout message and ensure it is
emitted through Console so the event reporter and UI receive the
stapling-skipped warning.
In `@Swiftpkgr/NotarizationSettingsView.swift`:
- Around line 25-26: Prevent notarizationPassword from being persisted in
build-info: update the PackageSettingsDraft, validatedConfiguration(), and
BuildInfoStore flow used by NotarizationSettingsView so the Apple ID password
remains ephemeral and is excluded from serialized configuration. Prefer reusing
the existing keychain-profile flow if available, while preserving the
SecureField input behavior.
In `@Swiftpkgr/ProjectEditorModel.swift`:
- Around line 250-256: The project action flow must reject transitions while an
operation is running. In Swiftpkgr/ProjectEditorModel.swift lines 250-256,
update requestProjectAction to return when isRunning before queuing or
performing the action; in Swiftpkgr/SwiftpkgrCommands.swift lines 7-13, disable
the new/open/import commands when model.isRunning.
- Around line 132-140: Enable YAML format parity across settings workflows: in
Swiftpkgr/ProjectEditorModel.swift lines 132-140, allow importSettings to load
YAML settings; in Swiftpkgr/ProjectEditorModel.swift lines 152-159, accept the
YAML BuildInfoFormat during export; and in Swiftpkgr/ProjectPanelService.swift
lines 36-52, include .yaml and .yml content types in both file panels.
- Around line 167-188: Separate ProjectEditorModel.build() into a request method
that checks whether configured signing or notarization credentials require user
confirmation and a confirmed execution method containing the existing build
logic. Update Swiftpkgr/ProjectEditorModel.swift lines 167-188 accordingly, and
change the build triggers in Swiftpkgr/ProjectEditorView.swift lines 41-43,
Swiftpkgr/SwiftpkgrCommands.swift lines 26-29, and Swiftpkgr/BuildView.swift
lines 15-17 to call the shared gated request method so every entry point follows
the confirmation flow.
---
Outside diff comments:
In `@swiftpkg/Support.swift`:
- Around line 56-92: Update SystemProcessRunner.run so process registration and
launch are atomic with respect to cancel(), preventing cancellation from
observing an unstarted activeProcess and returning before launch. Coordinate the
lock around activeProcess assignment and process.run(), while preserving cleanup
in the existing defer and ensuring launch errors still clear registration and
propagate as MunkiPkgError.
---
Nitpick comments:
In `@swiftpkg/PackageSettingsDraft.swift`:
- Around line 82-153: Document the newly exposed public APIs with concise
role-based Swift documentation comments: in swiftpkg/PackageSettingsDraft.swift
lines 82-153, document validatedConfiguration() and its defaults and
validation-to-PackageConfiguration contract; in swiftpkg/ProjectOperations.swift
lines 5-19, document ProjectCreator and createProject; in
swiftpkg/ProjectOperations.swift lines 49-66, document the public BOM export and
synchronization operations; in swiftpkg/PackageImporter.swift lines 17-29, add a
type-level comment for PackageImporter; and in swiftpkgCLI/SwiftPkg.swift lines
4-9, document SwiftPkg.run and its exit-status contract.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e54e5f28-fb44-4201-b944-33c2b9df6194
📒 Files selected for processing (48)
.github/workflows/ci.ymlAGENTS.mdPackage.swiftREADME.mdSwiftpkgr/AppAlert.swiftSwiftpkgr/BuildView.swiftSwiftpkgr/DistributionSettingsView.swiftSwiftpkgr/GeneralSettingsView.swiftSwiftpkgr/NotarizationSettingsView.swiftSwiftpkgr/OptionLabels.swiftSwiftpkgr/PackageBehaviorView.swiftSwiftpkgr/PendingProjectAction.swiftSwiftpkgr/ProjectContentsView.swiftSwiftpkgr/ProjectEditorModel.swiftSwiftpkgr/ProjectEditorView.swiftSwiftpkgr/ProjectPanelService.swiftSwiftpkgr/ProjectSection.swiftSwiftpkgr/ProjectSectionView.swiftSwiftpkgr/SigningSettingsView.swiftSwiftpkgr/SwiftpkgrApp.swiftSwiftpkgr/SwiftpkgrCommands.swiftSwiftpkgr/WelcomeView.swiftVERIFICATION.mddocs/SWIFTPKGR_IMPLEMENTATION_PLAN.mdswiftpkg.xcodeproj/project.pbxprojswiftpkg.xcodeproj/xcshareddata/xcschemes/SwiftPkgCore.xcschemeswiftpkg.xcodeproj/xcshareddata/xcschemes/Swiftpkgr.xcschemeswiftpkg/BuildInfo.swiftswiftpkg/BuildInfoDocument.swiftswiftpkg/ConsoleEvent.swiftswiftpkg/NotarizationAuthenticationMode.swiftswiftpkg/PackageBuildOptions.swiftswiftpkg/PackageBuilder.swiftswiftpkg/PackageImporter.swiftswiftpkg/PackageOperationService.swiftswiftpkg/PackageSettingsDraft.swiftswiftpkg/ProjectOperations.swiftswiftpkg/SigningTimestampMode.swiftswiftpkg/Support.swiftswiftpkg/Version.swiftswiftpkg/main.swiftswiftpkgCLI/CLI.swiftswiftpkgCLI/SwiftPkg.swiftswiftpkgCLI/main.swiftswiftpkgTests/BuildInfoTests.swiftswiftpkgTests/CLITests.swiftswiftpkgTests/ProjectOperationsTests.swiftswiftpkgTests/TestSupport.swift
💤 Files with no reviewable changes (1)
- swiftpkg/main.swift
| SecureField("App-specific password", text: $model.draft.notarizationPassword) | ||
| Text("CLI-compatible exports store this password in plaintext. Prefer a keychain profile.") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant files first
git ls-files 'Swiftpkgr/**/*.swift' | rg 'NotarizationSettingsView|ProjectEditorModel|build-info|Notarization|notarization' || true
# Show outline for the target file if present
for f in $(git ls-files 'Swiftpkgr/**/*.swift' | rg 'NotarizationSettingsView\.swift$'); do
echo "=== OUTLINE: $f ==="
ast-grep outline "$f" --view expanded || true
doneRepository: codecarton/swiftpkg
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Swiftpkgr swift files ==="
git ls-files 'Swiftpkgr/**/*.swift' | sed -n '1,200p'
echo "=== Search for notarization/build-info references ==="
rg -n --hidden --glob 'Swiftpkgr/**/*.swift' \
'notarizationPassword|notarization|build-info|keychain|SecureField|staplingTimeout|ProjectEditorModel|draft\.' \
Swiftpkgr || trueRepository: codecarton/swiftpkg
Length of output: 6478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect likely files once located.
for f in \
"$(git ls-files 'Swiftpkgr/**/*.swift' | rg 'NotarizationSettingsView\.swift$' | head -n 1)" \
"$(git ls-files 'Swiftpkgr/**/*.swift' | rg 'ProjectEditorModel\.swift$' | head -n 1)" \
"$(git ls-files 'Swiftpkgr/**/*.swift' | rg 'build-info|BuildInfo|PackageOperationService|Export' | head -n 5)"
do
[ -n "${f:-}" ] || continue
echo "=== FILE: $f ==="
wc -l "$f"
ast-grep outline "$f" --view expanded || true
doneRepository: codecarton/swiftpkg
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact files and inspect the relevant regions with line numbers.
for f in $(git ls-files 'Swiftpkgr/**/*.swift' | rg 'NotarizationSettingsView\.swift$|ProjectEditorModel\.swift$|PackageOperationService\.swift$|build-info|BuildInfo|Notarization'); do
echo "=== $f ==="
sed -n '1,260p' "$f" | cat -n
echo
doneRepository: codecarton/swiftpkg
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact files referenced by the search.
for f in \
Swiftpkgr/NotarizationSettingsView.swift \
Swiftpkgr/ProjectEditorModel.swift \
Swiftpkgr/ProjectEditorView.swift \
Swiftpkgr/SwiftpkgrCommands.swift
do
if [ -f "$f" ]; then
echo "=== $f ==="
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
echo
fi
done
# Also inspect any build-info or draft serialization code.
rg -n --hidden --glob 'Swiftpkgr/**/*.swift' \
'notarizationPassword|notarizationAppleID|notarizationTeamID|notarizationMode|staplingTimeout|validatedConfiguration|build-info|encode|serialize|PropertyListEncoder|JSONEncoder|FileManager|write' \
Swiftpkgr || trueRepository: codecarton/swiftpkg
Length of output: 21447
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search the whole repo for the draft/configuration definitions and the notarization password field.
rg -n --hidden \
'notarizationPassword|notarizationAppleID|notarizationTeamID|notarizationMode|staplingTimeout|PackageSettingsDraft|validatedConfiguration|BuildInfoStore|BuildInfoDocument|PackageConfiguration|appleID|keychainProfile' \
.
# Locate likely source files outside Swiftpkgr if present.
git ls-files | rg 'SwiftPkgCore|BuildInfo|PackageSettingsDraft|PackageConfiguration|Store|Draft|Settings' || trueRepository: codecarton/swiftpkg
Length of output: 16605
Don't persist the Apple ID password in build-info. SecureField only masks the UI; PackageSettingsDraft.notarizationPassword is carried into validatedConfiguration() and written out by BuildInfoStore, so Apple ID exports still store the credential in plaintext. Keep it ephemeral or switch this flow to a keychain profile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Swiftpkgr/NotarizationSettingsView.swift` around lines 25 - 26, Prevent
notarizationPassword from being persisted in build-info: update the
PackageSettingsDraft, validatedConfiguration(), and BuildInfoStore flow used by
NotarizationSettingsView so the Apple ID password remains ephemeral and is
excluded from serialized configuration. Prefer reusing the existing
keychain-profile flow if available, while preserving the SecureField input
behavior.
| func importSettings() { | ||
| guard let url = panels.chooseSettingsFile(), let projectURL else { return } | ||
| do { | ||
| let configuration = try BuildInfoStore.loadTemplate(from: url, defaultsFor: projectURL) | ||
| draft = PackageSettingsDraft(configuration: configuration) | ||
| statusMessage = "Imported settings" | ||
| activityLog.append("Imported settings from \(url.path)") | ||
| } catch { | ||
| present(error, title: "Could Not Import Settings") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep YAML available in settings import and export.
The app can create/open YAML build-info projects, but the settings panels filter YAML out and export explicitly rejects it. This breaks format parity with CLI-compatible projects.
Swiftpkgr/ProjectEditorModel.swift#L132-L140: allow imported YAML settings.Swiftpkgr/ProjectEditorModel.swift#L152-L159: accept the YAMLBuildInfoFormat.Swiftpkgr/ProjectPanelService.swift#L36-L52: add.yaml/.ymlcontent types to both panels.
📍 Affects 2 files
Swiftpkgr/ProjectEditorModel.swift#L132-L140(this comment)Swiftpkgr/ProjectEditorModel.swift#L152-L159Swiftpkgr/ProjectPanelService.swift#L36-L52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Swiftpkgr/ProjectEditorModel.swift` around lines 132 - 140, Enable YAML
format parity across settings workflows: in Swiftpkgr/ProjectEditorModel.swift
lines 132-140, allow importSettings to load YAML settings; in
Swiftpkgr/ProjectEditorModel.swift lines 152-159, accept the YAML
BuildInfoFormat during export; and in Swiftpkgr/ProjectPanelService.swift lines
36-52, include .yaml and .yml content types in both file panels.
| func build() { | ||
| guard let projectURL, let format = buildInfoDocument?.format else { return } | ||
| do { | ||
| try saveDraft() | ||
| } catch { | ||
| present(error, title: "Could Not Build Package") | ||
| return | ||
| } | ||
| let options = PackageBuildOptions( | ||
| requestedFormat: format, | ||
| exportsBOM: exportsBOM, | ||
| isQuiet: false, | ||
| skipsSigning: skipsSigning, | ||
| skipsNotarization: skipsNotarization, | ||
| skipsStapling: skipsStapling | ||
| ) | ||
| let reporter = makeReporter() | ||
| runOperation("Building package…") { [operations] in | ||
| try await operations.buildPackage(in: projectURL, options: options, reporter: reporter) | ||
| return projectURL | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require confirmation before credential-based builds.
Every Build entry point invokes build() immediately, including when signing or notarization will use configured credentials. Add a model-level request/confirmation flow so all entry points satisfy the PR’s credential-action confirmation requirement.
Swiftpkgr/ProjectEditorModel.swift#L167-L188: separate build request/confirmation from confirmed execution.Swiftpkgr/ProjectEditorView.swift#L41-L43: call the gated request method.Swiftpkgr/SwiftpkgrCommands.swift#L26-L29: call the same gated request method.Swiftpkgr/BuildView.swift#L15-L17: call the same gated request method.
📍 Affects 4 files
Swiftpkgr/ProjectEditorModel.swift#L167-L188(this comment)Swiftpkgr/ProjectEditorView.swift#L41-L43Swiftpkgr/SwiftpkgrCommands.swift#L26-L29Swiftpkgr/BuildView.swift#L15-L17
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Swiftpkgr/ProjectEditorModel.swift` around lines 167 - 188, Separate
ProjectEditorModel.build() into a request method that checks whether configured
signing or notarization credentials require user confirmation and a confirmed
execution method containing the existing build logic. Update
Swiftpkgr/ProjectEditorModel.swift lines 167-188 accordingly, and change the
build triggers in Swiftpkgr/ProjectEditorView.swift lines 41-43,
Swiftpkgr/SwiftpkgrCommands.swift lines 26-29, and Swiftpkgr/BuildView.swift
lines 15-17 to call the shared gated request method so every entry point follows
the confirmation flow.
| private func requestProjectAction(_ action: PendingProjectAction) { | ||
| if hasUnsavedChanges { | ||
| pendingProjectAction = action | ||
| showsDiscardConfirmation = true | ||
| } else { | ||
| perform(action) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Block project transitions while an operation is running.
New/open/import commands remain active during a build. Opening another project can replace model state, then the completed operation reopens its original project and may discard edits made meanwhile.
Swiftpkgr/ProjectEditorModel.swift#L250-L256: reject project actions whileisRunning.Swiftpkgr/SwiftpkgrCommands.swift#L7-L13: disable these commands whilemodel.isRunning.
📍 Affects 2 files
Swiftpkgr/ProjectEditorModel.swift#L250-L256(this comment)Swiftpkgr/SwiftpkgrCommands.swift#L7-L13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Swiftpkgr/ProjectEditorModel.swift` around lines 250 - 256, The project
action flow must reject transitions while an operation is running. In
Swiftpkgr/ProjectEditorModel.swift lines 250-256, update requestProjectAction to
return when isRunning before queuing or performing the action; in
Swiftpkgr/SwiftpkgrCommands.swift lines 7-13, disable the new/open/import
commands when model.isRunning.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 10 file(s) based on 6 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 10 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
26f7879 to
dbfa982
Compare
Adds Swiftpkgr, a native SwiftUI frontend for swiftpkg, while preserving the existing CLI and its macOS 13+ compatibility.
Swiftpkgr targets macOS 15+ and supports creating, importing, editing, building, and maintaining the same package projects as the command-line tool.
Closes #4
Closes #2
Summary by CodeRabbit