Skip to content

Feat. Add. pkg installer distribution#49

Open
sebsto wants to merge 1 commit into
mainfrom
sebsto/add_pkg_distribution
Open

Feat. Add. pkg installer distribution#49
sebsto wants to merge 1 commit into
mainfrom
sebsto/add_pkg_distribution

Conversation

@sebsto
Copy link
Copy Markdown
Owner

@sebsto sebsto commented Apr 10, 2026

The .pkg installs to /Applications with a custom installer UI (branded background, welcome, readme, license). Two new Makefile targets: make pkg for local builds and make pkg-release VERSION=x.y.z to 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/.

Recreated from #16 after accidental branch deletion.

Copilot AI review requested due to automatic review settings April 10, 2026 08:11
Copy link
Copy Markdown
Contributor

@amazon-q-developer amazon-q-developer Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds comprehensive specification documentation for a new .pkg installer distribution channel. The documentation includes requirements, design, and implementation tasks that are well-structured and thorough.

After reviewing all files, the specifications appear sound with proper security considerations (secrets are already excluded via .gitignore), comprehensive error handling, and clear implementation guidance. The design appropriately reuses existing Makefile infrastructure and follows established patterns from the brew-release target.

The documentation is ready 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.

Comment on lines +115 to +116
├── secrets/
│ └── notarization.json # Now includes "installer_identity" field
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 Security Vulnerability: The design references secrets/notarization.json in the file layout (lines 115-116) without explicitly documenting that this file must be excluded from version control. The file contains sensitive credentials (Apple ID, team ID, signing identities) that could be exposed if committed.

Add explicit documentation that secrets/notarization.json must be added to .gitignore and specify validation in the build pipeline to fail if this file is tracked by git. This prevents accidental credential exposure.

Comment on lines +126 to +135
### `secrets/notarization.json` (extended)

```json
{
"apple_id": "[email]",
"team_id": "[team_id]",
"signing_identity": "Developer ID Application: [name] ([team_id])",
"installer_identity": "Developer ID Installer: [name] ([team_id])"
}
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 Security Vulnerability: The JSON schema includes sensitive credentials (Apple ID, signing identities) without documenting that this file must never be committed to version control. The design document should explicitly state that secrets/notarization.json must be listed in .gitignore.

Add a security note to this section specifying that this file contains sensitive credentials and must be excluded from version control.

<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>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 Logic Error: The pkg-ref version is hardcoded to 0 which conflicts with the requirement to set version via pkgbuild --version $(VERSION) (design.md line 166). This creates version inconsistency where the component package has the correct version but the distribution XML references version 0.

The pkg-ref version attribute should be set to match $(VERSION) dynamically during the build, or the design should clarify that version 0 is intentional and explain how version validation will work during installation.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a requirements-first specification package for introducing a signed/notarized .pkg installer distribution workflow to the existing Wispr Makefile-based release pipeline.

Changes:

  • Added a requirements document defining the .pkg installer build/sign/notarize/release acceptance criteria.
  • Added a design document detailing Makefile integration, data model changes, and the intended build flow.
  • Added an implementation task plan and Kiro spec configuration for tracking the work.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
.kiro/specs/pkg-installer/requirements.md Defines user stories and acceptance criteria for a .pkg installer distribution channel.
.kiro/specs/pkg-installer/design.md Documents proposed architecture, Makefile targets/variables, and build/notarization flow.
.kiro/specs/pkg-installer/tasks.md Breaks the work into actionable implementation steps and checkpoints.
.kiro/specs/pkg-installer/.config.kiro Adds Kiro workflow metadata for the new spec.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +5
# Implementation Plan: pkg-installer

## Overview

Extend the existing Makefile build pipeline with `pkg` and `pkg-release` targets that produce a signed, notarized `.pkg` installer with custom UI. All installer resources (`pkg/distribution.xml`, `pkg/resources/`) are created as static files. The `secrets/notarization.json` schema is extended with an `installer_identity` field. Property-based tests use Python/Hypothesis.
- _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)`
Comment on lines +38 to +45
- [ ] 3. Extend Makefile with `pkg` target
- [ ] 3.1 Add new Makefile variables for the pkg pipeline
- `INSTALLER_IDENTITY` read from `$(NOTARIZATION_JSON)` via `jq -r .installer_identity`
- `COMPONENT_PKG`, `PRODUCT_PKG`, `SIGNED_PKG`, `FINAL_PKG` derived paths in `$(EXPORT_DIR)`
- `PKG_RESOURCES` pointing to `$(CURDIR)/pkg/resources`
- `DISTRIBUTION_XML` pointing to `$(CURDIR)/pkg/distribution.xml`
- Reuse all existing variables (`BUNDLE_ID`, `SIGNING_IDENTITY`, `APP_PATH`, `EXPORT_DIR`, `ARCHIVE_PATH`, `API_KEY_PATH`, `API_KEY_ID`, `API_ISSUER`, `NOTARIZATION_JSON`)
- _Requirements: 5.6, 7.1, 7.2_
Comment on lines +115 to +118
- [ ] 8. Property-based tests (Python/Hypothesis)
- [ ]* 8.1 Write property test for installer identity extraction round trip
- **Property 1: Installer identity extraction round trip**
- Generate random valid JSON with a non-empty `installer_identity` string, write to temp file, extract via `jq -r .installer_identity`, assert output matches original

## Error Handling

All error handling follows the existing Makefile pattern: print a descriptive message to stderr and `exit 1`. No partial artifacts are left behind on failure.

| 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)` |

- [ ] 3.3 Add `installer_identity` validation step
- Read `INSTALLER_IDENTITY` from `$(NOTARIZATION_JSON)` and verify it is non-empty
- Print `Error: installer_identity not found in <path>` and `exit 1` if missing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants