build: setup build infrastructure - #278
Conversation
Cloudflare Pages DeploymentEvent Name: pull_request Wrangler Output⛅️ wrangler 4.87.0 🌎 Deploying... |
📝 WalkthroughWalkthroughMigrates domains and repository ownership (deepink.io → deepink.app, vitonsky → DeepinkApp), replaces AGPL with Apache-2.0, restructures CI/CD with a dedicated wasm job and Node-based packaging, replaces Makefile packaging with npm/electron-forge flows, and updates package/site metadata and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant WasmJob as GitHub Actions (wasm)
participant NPM as npm (packages/twofish)
participant ArtifactStore as Actions Artifact
participant BuildJob as GitHub Actions (build)
participant Packager as electron-forge / npm scripts
participant Publisher as tsx publish script
WasmJob->>NPM: npm ci && npm run build (packages/twofish)
NPM-->>ArtifactStore: upload `packages/twofish/dist` as "twofish"
BuildJob->>ArtifactStore: download "twofish" -> `packages/twofish/dist`
BuildJob->>NPM: npm ci (repo)
BuildJob->>Packager: npx electron-forge package && npm run make
Packager-->>BuildJob: produce artifacts (out/make)
BuildJob->>Publisher: npx tsx ./scripts/publish --dir ./out/make
Publisher-->>GitHub: publish release assets
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/app/src/features/App/useGetAppUpdates.tsx (1)
66-75: ⚡ Quick winUse one host constant for both update check and download URL.
Line 66 and Line 75 duplicate the same base host. Centralizing avoids future drift.
Proposed refactor
- const appReleases = new AppUpdatesChecker({ host: 'https://deepink.app' }); + const updateHost = 'https://deepink.app'; + const appReleases = new AppUpdatesChecker({ host: updateHost }); ... - const updateUrl = 'https://deepink.app/download'; + const updateUrl = `${updateHost}/download`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/features/App/useGetAppUpdates.tsx` around lines 66 - 75, The code duplicates the base host string in the AppUpdatesChecker instantiation and the updateUrl; introduce a single constant (e.g., const DEEPINK_HOST = 'https://deepink.app') at the top of the module or inside useGetAppUpdates, replace the literal in new AppUpdatesChecker({ host: ... }) and construct updateUrl by `${DEEPINK_HOST}/download`, and ensure any other occurrences (download URL or host) in this module use the same constant so both AppUpdatesChecker and the updateUrl remain in sync.packages/twofish/compose.yml (1)
3-3: ⚡ Quick winConsider making the pinned Docker platform overridable.
Line 3 hard-pins
linux/amd64; keeping a default while allowing override helps ARM developers without changing CI behavior.Proposed refactor
- platform: linux/amd64 + platform: ${DOCKER_PLATFORM:-linux/amd64}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/twofish/compose.yml` at line 3, Replace the hard-coded platform value "platform: linux/amd64" in compose.yml with a compose/environment variable so callers can override it (for example use "platform: ${DOCKER_PLATFORM:-linux/amd64}"); update any README/CI that relies on the current file to set DOCKER_PLATFORM when needed and ensure default remains linux/amd64 so CI behavior is unchanged.packages/app/package.json (1)
34-34: 💤 Low valueRelease script depends on
jqwhich may not be universally available.The
releasescript usesjqto parsepackage.json. Whilejqis commonly available on Linux/macOS CI runners, it may not be installed on Windows or all developer machines.Consider using Node.js for portability, e.g., via inline script or a dedicated release script file:
♻️ Portable alternative using Node
-"release": "git tag -a \"v`jq -r .version package.json`\" -m \"build: publish v`jq -r .version package.json`\" && git push && git push --tags", +"release": "git tag -a \"v$(node -p \"require('./package.json').version\")\" -m \"build: publish v$(node -p \"require('./package.json').version\")\" && git push && git push --tags",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/package.json` at line 34, The release npm script currently uses jq to read package.json ("release" script) which breaks on systems without jq; replace it with a Node-based solution: change the "release" script to compute the version using node (e.g., a node -e inline command or call a small JS script like scripts/get-version.js) and use that value in the git tag and message so the script no longer depends on jq; update the "release" entry to reference the new Node invocation or script name and ensure any new helper script exports or prints the version for reuse..github/workflows/release.yml (1)
113-118: Remove the redundantelectron-forge packagecall.
electron-forge makeinternally runspackagebefore creating distributables. The separatenpx electron-forge packagecall beforenpm run makeis unnecessary.♻️ Suggested simplification
- name: Build app working-directory: packages/app run: | npm run build - npx electron-forge package npm run make🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 113 - 118, Remove the redundant packaging step in the "Build app" job by deleting the explicit npx electron-forge package invocation; keep npm run build and then run npm run make (since electron-forge make will invoke package internally), i.e., remove the line containing "npx electron-forge package" so the sequence is only "npm run build" followed by "npm run make".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 26-45: The "Build WASM" step always runs even when the twofish
cache hits; update the Build WASM step to include a conditional based on the
cache step output (twofish-cache) so it only runs when cache miss (e.g., check
steps.twofish-cache.outputs.cache-hit != 'true'); reference the cache step by
its id twofish-cache and the step label "Build WASM" to locate the YAML entry
and add the appropriate if condition to skip the rebuild when the cache
indicates a hit.
In `@packages/site/src/content/legal/privacy.md`:
- Line 216: The privacy text uses LaTeX-style math `$100\%$` which will render
literally in standard Markdown; update the sentence containing the literal
`$100\%$` to use plain text "100%" (e.g., replace `$100\%$` with `100%`) so the
percentage displays correctly, keeping the rest of the sentence and the security
contact `security@deepink.app` unchanged.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 113-118: Remove the redundant packaging step in the "Build app"
job by deleting the explicit npx electron-forge package invocation; keep npm run
build and then run npm run make (since electron-forge make will invoke package
internally), i.e., remove the line containing "npx electron-forge package" so
the sequence is only "npm run build" followed by "npm run make".
In `@packages/app/package.json`:
- Line 34: The release npm script currently uses jq to read package.json
("release" script) which breaks on systems without jq; replace it with a
Node-based solution: change the "release" script to compute the version using
node (e.g., a node -e inline command or call a small JS script like
scripts/get-version.js) and use that value in the git tag and message so the
script no longer depends on jq; update the "release" entry to reference the new
Node invocation or script name and ensure any new helper script exports or
prints the version for reuse.
In `@packages/app/src/features/App/useGetAppUpdates.tsx`:
- Around line 66-75: The code duplicates the base host string in the
AppUpdatesChecker instantiation and the updateUrl; introduce a single constant
(e.g., const DEEPINK_HOST = 'https://deepink.app') at the top of the module or
inside useGetAppUpdates, replace the literal in new AppUpdatesChecker({ host:
... }) and construct updateUrl by `${DEEPINK_HOST}/download`, and ensure any
other occurrences (download URL or host) in this module use the same constant so
both AppUpdatesChecker and the updateUrl remain in sync.
In `@packages/twofish/compose.yml`:
- Line 3: Replace the hard-coded platform value "platform: linux/amd64" in
compose.yml with a compose/environment variable so callers can override it (for
example use "platform: ${DOCKER_PLATFORM:-linux/amd64}"); update any README/CI
that relies on the current file to set DOCKER_PLATFORM when needed and ensure
default remains linux/amd64 so CI behavior is unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 16d8226b-bcd8-4a22-8de1-bc21444a42fd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/workflows/codeql.yml.github/workflows/release.yml.github/workflows/site.ymlpackage.jsonpackages/app/Makefilepackages/app/package.jsonpackages/app/src/features/App/VaultCreator/index.tsxpackages/app/src/features/App/useGetAppUpdates.tsxpackages/site/astro.config.mjspackages/site/src/components/analytics/index.tspackages/site/src/content/legal/privacy.mdpackages/site/src/content/legal/terms.mdpackages/site/src/features/404/index.tsxpackages/twofish/compose.yml
💤 Files with no reviewable changes (1)
- packages/app/Makefile
Cloudflare Pages DeploymentEvent Name: pull_request Wrangler Output⛅️ wrangler 4.87.0 🌎 Deploying... |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 7: The image markdown lacks alt text; update the image tag
'' to include a
concise descriptive alt string (e.g., '') so screen readers can describe the
screenshot—replace the empty brackets with an appropriate description matching
the screenshot content.
- Line 1: The README currently contains a mismatch between the license badge
(AGPL-3.0) and a paragraph that claims "Apache 2.0"; update the sentence that
mentions Apache 2.0 so it instead states AGPL-3.0 to match the LICENSE file and
the badge, ensuring the README consistently references AGPL-3.0 in the license
declaration and any adjacent license summary text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5946a53-d9bb-4893-be8c-9d0f63fc15d7
📒 Files selected for processing (12)
README.mdpackage.jsonpackages/app/package.jsonpackages/app/src/electron/updates/__tests__/AppUpdatesChecker.test.tspackages/app/src/electron/updates/__tests__/versions.jsonpackages/site/astro.config.mjspackages/site/src/components/Layout/Layout.tsxpackages/site/src/content/docs/introduction/security.mdpackages/site/src/content/legal/privacy.mdpackages/site/src/content/legal/terms.mdpackages/site/src/features/Landing/index.tsxpackages/site/src/features/releases.ts
✅ Files skipped from review due to trivial changes (8)
- packages/site/src/features/Landing/index.tsx
- packages/app/src/electron/updates/tests/AppUpdatesChecker.test.ts
- packages/site/src/content/docs/introduction/security.md
- packages/site/astro.config.mjs
- packages/site/src/content/legal/terms.md
- package.json
- packages/site/src/components/Layout/Layout.tsx
- packages/app/src/electron/updates/tests/versions.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app/package.json
Cloudflare Pages DeploymentEvent Name: pull_request Wrangler Output⛅️ wrangler 4.87.0 🌎 Deploying... |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LICENSE (1)
1-202:⚠️ Potential issue | 🟠 MajorUpdate license metadata in packages/site and fix contradictory license statement in terms.md.
The license migration to Apache 2.0 is incomplete and contains critical inconsistencies:
packages/site/package.json is missing a
"license"field (currently shows as MISSING). Add"license": "Apache 2.0"to align with the root project.packages/site/src/content/legal/terms.md contains a contradictory statement:
"Deepink is open source and distributed under the GNU Affero General Public License v3.0 (Apache 2.0)."
This statement conflates AGPL-3.0 with Apache 2.0, which are mutually exclusive. Replace this with a clear statement that the project is licensed solely under Apache 2.0.
The root package.json, README.md, and LICENSE file have been correctly updated. However, the documentation inconsistencies above must be resolved to ensure legal clarity and avoid confusing contributors and users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LICENSE` around lines 1 - 202, packages/site is missing a license declaration and the terms document contains a contradictory license string; add a "license": "Apache 2.0" entry to packages/site/package.json and update the sentence in packages/site/src/content/legal/terms.md that currently reads "Deepink is open source and distributed under the **GNU Affero General Public License v3.0 (Apache 2.0)**." to state the project is licensed solely under "Apache 2.0" (or "Apache License, Version 2.0") so the package metadata and the human-readable terms are consistent with the root LICENSE and package.json.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 31-33: The cache key only hashes files under packages/twofish/**
which can miss dependency changes; update the GitHub Actions cache key
expression (the key: twofish-${{ hashFiles(...) }}) to also include top-level
lockfile(s) and the package manifest(s) that affect dependencies (e.g.,
pnpm-lock.yaml, package-lock.json/yarn.lock if present, and
packages/twofish/package.json and root package.json) so changes to resolved deps
invalidate the twofish cache and force rebuilding packages/twofish/dist.
In `@packages/site/src/content/legal/terms.md`:
- Line 31: Replace the ambiguous license string "Deepink is open source and
distributed under the **GNU Affero General Public License v3.0 (Apache 2.0)**."
with a single, correct license name: either state "GNU Affero General Public
License v3.0 (AGPL-3.0)" or "Apache License 2.0" consistently; update the
sentence in terms.md to use the chosen full license name (e.g., "Deepink is open
source and distributed under the GNU Affero General Public License v3.0
(AGPL-3.0)." or "Deepink is open source and distributed under the Apache
License, Version 2.0.") and ensure no other text in the document references the
conflicting license combination.
---
Outside diff comments:
In `@LICENSE`:
- Around line 1-202: packages/site is missing a license declaration and the
terms document contains a contradictory license string; add a "license": "Apache
2.0" entry to packages/site/package.json and update the sentence in
packages/site/src/content/legal/terms.md that currently reads "Deepink is open
source and distributed under the **GNU Affero General Public License v3.0
(Apache 2.0)**." to state the project is licensed solely under "Apache 2.0" (or
"Apache License, Version 2.0") so the package metadata and the human-readable
terms are consistent with the root LICENSE and package.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d87b364-be62-43dc-bd76-7cba8d24fe9f
📒 Files selected for processing (7)
.github/workflows/release.ymlLICENSEREADME.mdpackage.jsonpackages/app/package.jsonpackages/site/src/content/legal/privacy.mdpackages/site/src/content/legal/terms.md
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app/package.json
Cloudflare Pages DeploymentEvent Name: pull_request Wrangler Output⛅️ wrangler 4.87.0 🌎 Deploying... |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/site/src/content/legal/terms.md (1)
29-31: ⚡ Quick winUse the canonical Apache license name consistently in legal text.
On Line 31 and related references, consider using “Apache License, Version 2.0” (instead of shorthand “Apache 2.0”) for legal precision and consistency with
LICENSE.Proposed wording cleanup
-## 3) Open source software license (Apache 2.0) +## 3) Open source software license (Apache License, Version 2.0) -Deepink is open source and distributed under the **Apache 2.0**. +Deepink is open source and distributed under the **Apache License, Version 2.0**. -**Important:** The Apache 2.0 governs the App’s source code and your rights to use, modify, and redistribute that code. +**Important:** The Apache License, Version 2.0 governs the App’s source code and your rights to use, modify, and redistribute that code. -The App’s source code is licensed under Apache 2.0, but the **Deepink name, logo, and other branding** are **not** automatically granted for your use. +The App’s source code is licensed under the Apache License, Version 2.0, but the **Deepink name, logo, and other branding** are **not** automatically granted for your use. -You may truthfully state that your fork/build is “based on Deepink” or “built from Deepink (Apache 2.0),” but you may not: +You may truthfully state that your fork/build is “based on Deepink” or “built from Deepink (Apache License, Version 2.0),” but you may not: -- These Terms are the entire agreement between you and Deepink regarding the App and Hosted Services, except where the Apache 2.0 license governs the software code. +- These Terms are the entire agreement between you and Deepink regarding the App and Hosted Services, except where the Apache License, Version 2.0 governs the software code.Also applies to: 36-36, 102-102, 104-104, 185-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/site/src/content/legal/terms.md` around lines 29 - 31, Replace the shorthand license name with the canonical phrase "Apache License, Version 2.0" wherever the page currently says "Apache 2.0" — specifically update the heading "## 3) Open source software license (Apache 2.0)" and the sentence "Deepink is open source and distributed under the **Apache 2.0**." as well as the other occurrences noted (lines referenced 36, 102, 104, 185) to use "Apache License, Version 2.0" so the legal text matches the LICENSE file and is consistent across the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/site/src/content/legal/terms.md`:
- Around line 29-31: Replace the shorthand license name with the canonical
phrase "Apache License, Version 2.0" wherever the page currently says "Apache
2.0" — specifically update the heading "## 3) Open source software license
(Apache 2.0)" and the sentence "Deepink is open source and distributed under the
**Apache 2.0**." as well as the other occurrences noted (lines referenced 36,
102, 104, 185) to use "Apache License, Version 2.0" so the legal text matches
the LICENSE file and is consistent across the document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d0233170-c632-43df-9bb9-1a1101afee58
📒 Files selected for processing (1)
packages/site/src/content/legal/terms.md
1 participant
Closes #277
Summary by CodeRabbit
Chores
Documentation
Refactor
UI/Behavior