Remove pulumi-bin and add @pulumi/pulumi dependency - #411
Conversation
…ependency to package.json
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces Doppler secret management targets to the Makefile, expands Linux-specific package configuration in home-manager by removing pulumi-bin from base packages and conditionally adding numerous Linux utilities and applications, and adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the project's dependency management by replacing an indirect Pulumi binary with a direct '@pulumi/pulumi' package dependency, aiming to simplify the project's setup. Additionally, it integrates new automation for Doppler secrets, providing dedicated 'make' commands to manage environment variables for development and production. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRRemoved What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request streamlines Pulumi dependency management by replacing pulumi-bin from home-manager with the @pulumi/pulumi npm package. It also introduces new Makefile targets, doppler-sync and doppler-upload, to manage Doppler secrets. The changes are logical and well-implemented. I have a couple of suggestions for the Makefile to improve the robustness and user experience of the new Doppler targets.
| @echo "🔐 Syncing Doppler secrets to .env..." | ||
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env |
There was a problem hiding this comment.
The doppler-sync command overwrites the .env file without any warning. If a user has made local changes to .env that they haven't uploaded, running this target will cause them to lose those changes. It would be safer to update the informational message to warn the user that the file will be overwritten.
@echo "🔐 Syncing Doppler secrets to .env (this will overwrite the existing file)..."
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). | ||
| @echo "🔐 Uploading .env to Doppler..." | ||
| @doppler secrets upload --project dotfiles --config prd .env | ||
| @echo "✅ .env uploaded to Doppler (dotfiles/prd)" |
There was a problem hiding this comment.
The doppler-upload target attempts to upload the .env file without first checking if it exists. If the file is missing (for example, if make doppler-sync hasn't been run), the command will fail with an error from the doppler CLI.
It would be more user-friendly and robust to add a check for the file's existence and provide a helpful error message if it's not found. This would guide the user on the correct workflow.
doppler-upload: ## Upload .env file to Doppler (dotfiles/prd).
@if [ ! -f .env ]; then \
echo "❌ .env file not found. Run 'make doppler-sync' first."; \
exit 1; \
fi
@echo "🔐 Uploading .env to Doppler..."
@doppler secrets upload --project dotfiles --config prd .env
@echo "✅ .env uploaded to Doppler (dotfiles/prd)"
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Makefile(1 hunks)home-manager/packages/default.nix(0 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (1)
- home-manager/packages/default.nix
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome with 2-space indentation, 80-character line width, double quotes, and trailing commas (ES5) for JSON/JavaScript/TypeScript files
Files:
package.json
**/*.{json,yaml,yml,toml}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
**/*.{json,yaml,yml,toml}: Use consistent indentation (2 spaces) in configuration files
Sort keys alphabetically when possible in configuration files
Use clear, descriptive names in configuration files
Files:
package.json
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
**/*.{js,jsx,ts,tsx,json}: Format JavaScript/TypeScript/JSON files using Biome with 2-space indentation and 80 character line width
Enable Biome linter with recommended rules for JavaScript/TypeScript/JSON files
Files:
package.json
**/*.{js,jsx,ts,tsx,json,jsonc,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Use Biome for code formatting as configured in biome.json
Files:
package.json
🪛 checkmake (0.2.2)
Makefile
[warning] 658-658: Missing required phony target "all"
(minphony)
[warning] 658-658: Missing required phony target "clean"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Agent
- GitHub Check: cubic · AI code reviewer
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: lua-neovim-test
- GitHub Check: lua-hammerspoon
- GitHub Check: shellspec
- GitHub Check: lua-neovim
- GitHub Check: shellcheck
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: nix-nixos
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
🔇 Additional comments (1)
package.json (1)
13-26: Verify Pulumi + buntrustedDependenciesusage and version policy (lockfile will pin).
Adding@pulumi/pulumiin bothdependenciesandtrustedDependenciesis plausible, but please confirm (a) this repo’s bun version (bun@1.3.0) actually relies ontrustedDependenciesfor this package, and (b)^3.212.0is the intended version range (and consistent with your lockfile / Pulumi tooling).Bun "trustedDependencies" what does it do, and when should a package be added to it?What is the recommended/current @pulumi/pulumi version and any breaking changes around 3.212.0?Also applies to: 27-39
| ##@ Doppler | ||
|
|
||
| .PHONY: doppler-sync | ||
| doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file. | ||
| @echo "🔐 Syncing Doppler secrets to .env..." | ||
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env | ||
| @echo "✅ .env file updated from Doppler (dotfiles/prd)" | ||
|
|
||
| .PHONY: doppler-upload | ||
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). | ||
| @echo "🔐 Uploading .env to Doppler..." | ||
| @doppler secrets upload --project dotfiles --config prd .env | ||
| @echo "✅ .env uploaded to Doppler (dotfiles/prd)" |
There was a problem hiding this comment.
Prevent accidental secret leakage / clobbering when syncing Doppler to .env.
doppler-sync overwrites .env in the repo root and leaves secrets at rest; it’s easy to accidentally commit it. Consider: (1) ensure .env is in .gitignore, (2) write with restrictive permissions (e.g., umask 077), (3) optional backup/confirmation, and/or (4) fail early if doppler isn’t installed/authenticated.
Suggested tweak:
doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file.
@echo "🔐 Syncing Doppler secrets to .env..."
- @doppler secrets download --project dotfiles --config prd --format env --no-file > .env
+ @command -v doppler >/dev/null 2>&1 || (echo "❌ doppler CLI not found" && exit 1)
+ @umask 077; doppler secrets download --project dotfiles --config prd --format env --no-file > .env
@echo "✅ .env file updated from Doppler (dotfiles/prd)"🧰 Tools
🪛 checkmake (0.2.2)
[warning] 658-658: Missing required phony target "all"
(minphony)
[warning] 658-658: Missing required phony target "clean"
(minphony)
🤖 Prompt for AI Agents
In Makefile around lines 650 to 662, the doppler-sync target currently
overwrites .env in the repo root which risks leaking or clobbering secrets;
update the target to (1) check doppler is installed and authenticated and fail
early if not, (2) ensure .env is listed in .gitignore (or at least warn and
refuse to proceed if not), (3) write secrets to a temporary file then set
restrictive permissions (e.g., create with umask 077 or chmod 600) and
atomically move it into place to avoid leaving secrets in world-readable temp
files, and (4) optionally backup an existing .env (e.g., rename to
.env.backup.TIMESTAMP) or require a confirmation flag before overwriting;
implement these checks and atomic/temp-file + chmod + move flow in the Makefile
recipe so doppler-sync is safe by default.
| .PHONY: doppler-sync | ||
| doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file. | ||
| @echo "🔐 Syncing Doppler secrets to .env..." | ||
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env | ||
| @echo "✅ .env file updated from Doppler (dotfiles/prd)" | ||
|
|
||
| .PHONY: doppler-upload | ||
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). | ||
| @echo "🔐 Uploading .env to Doppler..." | ||
| @doppler secrets upload --project dotfiles --config prd .env | ||
| @echo "✅ .env uploaded to Doppler (dotfiles/prd)" |
There was a problem hiding this comment.
Address checkmake “missing phony all/clean” warnings if you want a clean lint run.
checkmake is asking for conventional all and clean phony targets; if your repo treats these as required, add lightweight aliases (even if they just point at existing targets).
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 658-658: Missing required phony target "all"
(minphony)
[warning] 658-658: Missing required phony target "clean"
(minphony)
🤖 Prompt for AI Agents
In Makefile around lines 652 to 662, checkmake warns about missing conventional
phony targets `all` and `clean`; add lightweight .PHONY targets named `all` and
`clean` (placed near the doppler-* rules) and implement them as simple aliases
that invoke your real default/build target or perform a no-op/cleanup (e.g.,
`all` can depend on the main build target or echo a message; `clean` can remove
generated artifacts or be an empty-but-valid target) so the lint stops
complaining while preserving current behavior.
There was a problem hiding this comment.
Pull request overview
This pull request modernizes dependency management by transitioning from system-level Pulumi installation to npm package management, while also introducing Doppler secret management automation through new Makefile targets.
Key Changes:
- Replaces
pulumi-binsystem package with@pulumi/puluminpm dependency (v3.212.0) - Adds Makefile targets (
doppler-syncanddoppler-upload) for automating Doppler secret synchronization with local.envfiles - Updates lockfile (bun.lock) with all transitive dependencies for the new Pulumi package
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Adds @pulumi/pulumi npm dependency and includes it in trustedDependencies array for secure installation |
| home-manager/packages/default.nix | Removes pulumi-bin from the system packages list (line 59 deleted) |
| bun.lock | Updates lockfile with @pulumi/pulumi and its extensive dependency tree including gRPC, OpenTelemetry, protobuf, and npm CLI tooling |
| Makefile | Introduces two new Doppler targets for bidirectional .env file synchronization with the dotfiles/prd project configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @echo "✅ .env file updated from Doppler (dotfiles/prd)" | ||
|
|
||
| .PHONY: doppler-upload | ||
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). |
There was a problem hiding this comment.
The doppler-upload target should verify that the .env file exists before attempting to upload to avoid misleading error messages. Consider adding a file existence check before the upload command.
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). | |
| doppler-upload: ## Upload .env file to Doppler (dotfiles/prd). | |
| @if [ ! -f .env ]; then \ | |
| echo "❌ .env file does not exist. Please create it before uploading."; \ | |
| exit 1; \ | |
| fi |
|
|
||
| .PHONY: doppler-sync | ||
| doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file. | ||
| @echo "🔐 Syncing Doppler secrets to .env..." |
There was a problem hiding this comment.
The doppler-sync target overwrites the .env file without any backup or warning. If a user has local changes in their .env file, they will be lost without notification. Consider adding a warning message or creating a backup before overwriting.
| @echo "🔐 Syncing Doppler secrets to .env..." | |
| @echo "🔐 Syncing Doppler secrets to .env..." | |
| @if [ -f .env ]; then \ | |
| echo "⚠️ .env file exists. Creating backup as .env.bak before overwriting."; \ | |
| cp .env .env.bak; \ | |
| fi |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="Makefile">
<violation number="1" location="Makefile:655">
P2: Using direct stdout redirection can cause data loss if the doppler command fails. Consider writing to a temp file first and moving on success:
```makefile
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .env
```</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| .PHONY: doppler-sync | ||
| doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file. | ||
| @echo "🔐 Syncing Doppler secrets to .env..." | ||
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env |
There was a problem hiding this comment.
P2: Using direct stdout redirection can cause data loss if the doppler command fails. Consider writing to a temp file first and moving on success:
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .envPrompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 655:
<comment>Using direct stdout redirection can cause data loss if the doppler command fails. Consider writing to a temp file first and moving on success:
```makefile
@doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .env
```</comment>
<file context>
@@ -646,3 +646,17 @@ shell-check: ## Run ShellCheck on shell scripts.
+.PHONY: doppler-sync
+doppler-sync: ## Sync Doppler secrets (dotfiles/prd) to .env file.
+ @echo "🔐 Syncing Doppler secrets to .env..."
+ @doppler secrets download --project dotfiles --config prd --format env --no-file > .env
+ @echo "✅ .env file updated from Doppler (dotfiles/prd)"
+
</file context>
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env | |
| @doppler secrets download --project dotfiles --config prd --format env --no-file > .env.tmp && mv .env.tmp .env |
Eliminate the pulumi-bin package from home-manager and include the @pulumi/pulumi dependency in package.json to streamline project dependencies. Additionally, introduce make targets for syncing and uploading Doppler secrets.
Summary by cubic
Remove pulumi-bin from home-manager and add the @pulumi/pulumi npm dependency to simplify Pulumi usage. Add make targets to sync and upload Doppler secrets via a .env file.
New Features
Dependencies
Written for commit 3e518df. Summary will update automatically on new commits.