refactor: use wrangler.example.toml pattern for configuration #50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace tracked wrangler.toml with wrangler.example.toml template pattern. This makes it easier to modify wrangler configuration locally without affecting tracked files and follows standard practice for config files. Changes: - Create wrangler.example.toml from wrangler.toml - Add wrangler.toml to .gitignore (both root and packages/api) - Update deploy.sh: create wrangler.toml from example on each deployment - Update migrate-d1.sh: create wrangler.toml from example before migrations - Update CI/CD action: rename to "Create wrangler.toml from example" - Simplify workflows: no need to backup/restore wrangler.toml anymore - Update docs: deployment.md and packages/api/README.md Benefits: - No more backup/restore dance during deployments - Developers create their own wrangler.toml locally from example - CI/CD creates fresh wrangler.toml for each deployment - Zero risk of accidentally committing sensitive values - Cleaner git history without config file churn Workflow: - Local dev: Copy wrangler.example.toml or use wrangler.toml.local - CI/CD: Action creates wrangler.toml from example with substitution - Scripts: Create wrangler.toml from example before each operation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
=======================================
Coverage ? 67.31%
=======================================
Files ? 108
Lines ? 4479
Branches ? 1190
=======================================
Hits ? 3015
Misses ? 1457
Partials ? 7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Wrangler configuration management to use a template pattern (wrangler.example.toml) instead of tracking a wrangler.toml file directly. This approach improves security by preventing accidental commits of sensitive configuration values and simplifies local development workflows.
Key Changes:
- Introduced
wrangler.example.tomlas a tracked template with placeholder values - Added
wrangler.tomlto .gitignore (both root and packages/api) - Updated deployment scripts to generate
wrangler.tomlfrom the template on each run - Renamed and updated CI/CD action to reflect the new "create from example" pattern
- Removed backup/restore logic that is no longer needed
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/wrangler.example.toml | New template file with placeholders for environment-specific values |
| packages/api/scripts/migrate-d1.sh | Creates wrangler.toml from example before running migrations; removes backup/restore logic |
| packages/api/scripts/deploy.sh | Creates wrangler.toml from example before deployment; removes backup/restore logic |
| packages/api/README.md | Updated documentation to explain the new configuration workflow options |
| packages/api/.gitignore | Added wrangler.toml to ignore list |
| .gitignore | Added wrangler.toml to root ignore list |
| .github/workflows/deploy-dev.yml | Updated step name and removed restore logic; simplified workflow |
| .github/workflows/deploy-cloudflare.yml | Updated step name to reflect new pattern |
| .github/actions/substitute-d1-database-id/action.yml | Renamed action and updated to create wrangler.toml from example template |
| docs/deployment.md | Comprehensive documentation updates explaining both local development options and the new workflow |
Review Summary:
The refactoring is well-executed and consistent across all files. The changes follow best practices for configuration management:
- ✅ Security: Prevents accidental commits of sensitive values
- ✅ Consistency: All scripts and workflows follow the same pattern
- ✅ Documentation: Clear instructions for both local development options and CI/CD setup
- ✅ Maintainability: Simpler workflow without backup/restore dance
- ✅ Backward compatibility: Scripts gracefully handle both D1_DATABASE_ID env var and wrangler.toml.local
No critical issues were identified. The implementation is clean, well-documented, and ready for merge.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@sentry review |
Comment on lines
+78
to
+81
| ) | ||
| print(" Continuing anyway, but this may cause issues", file=sys.stderr) | ||
|
|
There was a problem hiding this comment.
When validating the database ID format, the warning is printed but execution continues. This could lead to silently accepting invalid database IDs that would fail later during deployment. Consider either enforcing strict validation or adding additional checks at deployment time. If you want to be lenient, consider using a different approach: try to deploy first and catch errors rather than guessing at validation.
Severity: MEDIUM
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/api/scripts/create-wrangler-config.py#L78-L80
Potential issue: When validating the database ID format, the warning is printed but
execution continues. This could lead to silently accepting invalid database IDs that
would fail later during deployment. Consider either enforcing strict validation or
adding additional checks at deployment time. If you want to be lenient, consider using a
different approach: try to deploy first and catch errors rather than guessing at
validation.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 4672852
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace fragile sed-based substitution with Python script that provides
comprehensive validation similar to CI/CD action.
Changes:
- Remove packages/api/wrangler.toml from version control (now gitignored)
- Add create-wrangler-config.py with comprehensive validation
- Update deploy.sh and migrate-d1.sh to use Python script
- Fix Python 3.9 compatibility: use Optional[str] instead of str | None
- Remove redundant error checks (set -e handles exit on failure)
Critical fixes:
- Validate D1_DATABASE_ID format (UUID pattern)
- Verify placeholder exists before substitution
- Confirm placeholder was successfully replaced
- Check database_id line format in final file
- Prevent deployment with malformed configuration
Python script validation (create-wrangler-config.py):
- Validates database ID is not empty and looks like a UUID
- Performs substitution with multiple verification steps
- Cleans up database_id line formatting
- Provides detailed error messages on failure
- Compatible with Python 3.9+ (uses Optional from typing)
Updated bash scripts (deploy.sh, migrate-d1.sh):
- Call Python script for validated config creation
- Rely on set -e for automatic exit on failure
- Simpler and more maintainable
Benefits:
- wrangler.toml no longer tracked, preventing accidental commits
- Prevents cryptic TOML parsing errors from malformed config
- Catches configuration issues before wrangler commands run
- Consistent validation between local scripts and CI/CD
- Better error messages for debugging
Fixes critical bug where sed substitution could fail silently,
leaving ${D1_DATABASE_ID} placeholder in wrangler.toml.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Ignore __pycache__ and compiled Python files (.pyc, .pyo, .pyd, .so) generated by create-wrangler-config.py script. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
KyleTryon
force-pushed
the
refactor/wrangler-example-toml
branch
from
443097a to
ecb85d5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace tracked wrangler.toml with wrangler.example.toml template pattern. This makes it easier to modify wrangler configuration locally without affecting tracked files and follows standard practice for config files.
Changes:
Benefits:
Workflow:
🤖 Generated with Claude Code
Description
Type of Change
Related Issues
Fixes #
Relates to #
Changes Made
Testing
pnpm test)pnpm type-check)pnpm lint)Screenshots/Videos
Documentation
Checklist
Additional Notes