Conversation
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds comprehensive specification documents for a new .pkg installer distribution feature. The specifications are well-structured with requirements, design, and implementation tasks.
Critical Issue:
- The distribution.xml example contains a hardcoded filename that must exactly match the component package filename used in the build pipeline, creating a potential runtime failure point.
Once the filename consistency issue is resolved, the specification structure appears sound for implementation.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| <choice id="wispr" visible="false"> | ||
| <pkg-ref id="com.stormacq.mac.wispr"/> | ||
| </choice> | ||
| <pkg-ref id="com.stormacq.mac.wispr" version="0" onConclusion="none">wispr-component.pkg</pkg-ref> |
There was a problem hiding this comment.
🛑 Logic Error: The pkg-ref filename wispr-component.pkg is hardcoded in the distribution.xml example, but the design specifies COMPONENT_PKG variable as $(EXPORT_DIR)/wispr-component.pkg (line 54). The productbuild command requires the pkg-ref filename to match exactly what's in the --package-path directory. If implementation follows the variable definition, this mismatch will cause productbuild to fail with "package not found" error.
There was a problem hiding this comment.
Pull request overview
This PR adds a Kiro feature specification for a .pkg installer distribution channel for the Wispr macOS app. The specification consists of requirements, design, and implementation task documents under .kiro/specs/pkg-installer/. It describes how to extend the existing Makefile build pipeline with pkg and pkg-release targets that produce a signed, notarized .pkg installer with a custom installer UI.
Changes:
- Added requirements document defining 8 requirement areas covering component package building, custom installer UI, signing, notarization, Makefile targets, identity configuration, and resource file management.
- Added design document detailing the build flow architecture, new Makefile variables/targets, distribution XML schema, and testing strategy (property-based tests with Python/Hypothesis).
- Added implementation tasks document with a step-by-step plan for creating installer resources, extending the Makefile, and writing tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.kiro/specs/pkg-installer/requirements.md |
Feature requirements for .pkg installer: build flow, signing, notarization, Makefile targets, and resource management |
.kiro/specs/pkg-installer/design.md |
Technical design: build flow, variables, distribution XML, JSON schema extension, error handling, and testing strategy |
.kiro/specs/pkg-installer/tasks.md |
Implementation task checklist with requirement traceability and checkpoints |
.kiro/specs/pkg-installer/.config.kiro |
Kiro spec configuration (feature type, requirements-first workflow) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| | Step | Tool | Key Arguments | | ||
| |------|------|---------------| | ||
| | Component pkg | `pkgbuild` | `--root` (app path parent), `--component-plist` (not needed, single app), `--install-location /Applications`, `--identifier $(BUNDLE_ID)`, `--version $(VERSION)` | |
There was a problem hiding this comment.
The design specifies pkgbuild --root with the app path parent directory, which would be $(EXPORT_DIR). However, after the notarize target runs, $(EXPORT_DIR) contains not just Wispr.app but also wispr-notarized.zip (see Makefile line 72: ZIP_PATH := $(EXPORT_DIR)/wispr-notarized.zip). Using pkgbuild --root $(EXPORT_DIR) would package the zip file into the installer as well.
The design should either:
- Use
pkgbuild --component $(APP_PATH) --install-location /Applicationsinstead of--root, which packages only the.appbundle, or - Add a step to remove/move the zip file before running
pkgbuild, or - Use a separate staging directory for the
--rootargument containing only the.app.
| | Component pkg | `pkgbuild` | `--root` (app path parent), `--component-plist` (not needed, single app), `--install-location /Applications`, `--identifier $(BUNDLE_ID)`, `--version $(VERSION)` | | |
| | Component pkg | `pkgbuild` | `--component $(APP_PATH)`, `--install-location /Applications`, `--identifier $(BUNDLE_ID)`, `--version $(VERSION)` | |
| - _Requirements: 7.2, 7.3_ | ||
|
|
||
| - [ ] 3.4 Add `pkgbuild` step to create the component package | ||
| - `pkgbuild --root <app parent> --install-location /Applications --identifier $(BUNDLE_ID) --version $(VERSION) $(COMPONENT_PKG)` |
There was a problem hiding this comment.
The pkgbuild --root <app parent> command would use $(EXPORT_DIR) as the root, which after the notarize prerequisite also contains wispr-notarized.zip. This would cause the zip file to be included in the component package. Consider using pkgbuild --component $(APP_PATH) --install-location /Applications instead, which targets only the .app bundle.
| - `pkgbuild --root <app parent> --install-location /Applications --identifier $(BUNDLE_ID) --version $(VERSION) $(COMPONENT_PKG)` | |
| - `pkgbuild --component $(APP_PATH) --install-location /Applications --identifier $(BUNDLE_ID) --version $(VERSION) $(COMPONENT_PKG)` |
| 5. Runs `pkgbuild` to create the component package | ||
| 6. Runs `productbuild` to create the product package with custom UI | ||
| 7. Runs `productsign` to sign the product package | ||
| 8. Runs `notarytool` to notarize the signed package (reuses `_setup-api-key` / `_cleanup-api-key`) |
There was a problem hiding this comment.
The design says the pkg target depends on notarize and then "reuses _setup-api-key / _cleanup-api-key" for the pkg notarization step. However, the existing notarize target already calls _cleanup-api-key at its end (Makefile line 83), which deletes the API key file. The design should explicitly note that _setup-api-key must be called again within the pkg recipe before the notarytool submit step for the .pkg file, since the key will have been cleaned up by the notarize prerequisite.
| 8. Runs `notarytool` to notarize the signed package (reuses `_setup-api-key` / `_cleanup-api-key`) | |
| 8. Before submitting the `.pkg`, calls `_setup-api-key` again (because the `notarize` target's `_cleanup-api-key` removed the key file), then runs `notarytool` to notarize the signed package, and finally calls `_cleanup-api-key` |
sebsto
changed the title
The
.pkginstalls to/Applicationswith a custom installer UI (branded background, welcome, readme, license).Two new
Makefiletargets:make pkgfor local builds andmake pkg-release VERSION=x.y.zto upload to GitHub Releases. Reuses the existing notarize pipeline end-to-end, no duplicated build logic.Spec covers requirements, design, and implementation tasks in
.kiro/specs/pkg-installer/.