ref: Centralize dry-run logic with Proxy-based abstraction #685
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
Replaces scattered isDryRun() checks with a centralized abstraction layer that automatically intercepts destructive operations: - Add src/utils/dryRun.ts with Proxy wrappers for SimpleGit, Octokit, and fs - Update getGitClient()/createGitClient() to return dry-run-aware git clients - Update getGitHubClient() to return dry-run-aware Octokit instance - Add dryRunFs for file write operations (writeFile, unlink, mkdir, rename) - Add dryRunExec() helper for custom destructive operations - Migrate commands and targets to use wrapped APIs - Add ESLint rules to enforce using dry-run wrapped APIs - Add comprehensive tests for proxy behavior - Document the pattern in AGENTS.md
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Changelog
Other
Build / dependencies / internal 🔧
🤖 This preview updates automatically when you update the PR. |
BYK
changed the title
BYK
commented
Dec 29, 2025
- Remove repetitive dry-run proxy comments - Remove isDryRun() checks from brew.ts, awsLambdaLayer.ts, github.ts (use proxied APIs instead) - Allow clone() through dry-run proxy (safe local operation) - Remove eslint-disable comments for clone operations - Update Octokit proxy to return mock status for status-based checks
BYK
commented
Dec 29, 2025
- Add cloneRepo() helper for cleaner clone+createGitClient pattern - Rename dryRunFs to safeFs, dryRunExec to safeExec - Remove remaining isDryRun() checks from github.ts using safeExec - Remove repetitive dry-run proxy comments from registry.ts and ghPages.ts - Update AGENTS.md with new naming
- Fix GCS download to return null in dry-run mode (instead of invalid path) - Use explicit isDryRun() check in createDraftRelease for mock data - Cache wrapped git instances in proxy to avoid recreation on chaining - Add copyFile/copyFileSync to safeFs - Fix import ordering in prepare.ts - Add tests for chained git operations and proxy caching
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
…mode In dry-run mode, git methods like commit() now return proper mock result objects instead of the proxy itself. This fixes issues where code expects to access properties like commitResult.commit to get the commit hash.
Remove pull and push from GIT_MOCK_RESULTS because they are used in method chains like git.pull().merge().push() in publish.ts. Returning a mock object instead of the proxy breaks these chains. Only commit needs a mock result because upm.ts accesses commitResult.commit.
MathurAditya724
approved these changes
Dec 31, 2025
Refactor safeFs to use the same Proxy pattern as Git and Octokit for consistency. This also exports safeFsPromises and safeFsSync for direct access to the full proxied fs modules.
Return proxy directly instead of Promise.resolve(proxy) to support method chaining like git.pull().merge().push(). The proxy wraps SimpleGit which is thenable, so await still works correctly. Only methods in GIT_MOCK_RESULTS (like commit) return a Promise with mock data since their return values are actually accessed.
This was referenced Dec 31, 2025
7 tasks
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.
Summary
Replaces scattered
isDryRun()checks throughout the codebase with a centralized abstraction layer that automatically intercepts destructive operations using JavaScript Proxies.Changes
New Module:
src/utils/dryRun.tscreateDryRunGit()- Proxy wrapper for SimpleGit that blocks mutating operations (push, commit, checkout, merge, etc.)createDryRunOctokit()- Proxy wrapper for Octokit that blocks mutating API calls (create*, update*, delete*, upload*)dryRunFs- Object with file system operations that respect dry-run (writeFile, unlink, mkdir, rename)dryRunExec()/dryRunExecSync()- Generic action wrappers for custom operationslogDryRun()- Consistent log formattingIntegration
getGitClient()and newcreateGitClient()return dry-run-aware git clientsgetGitHubClient()returns dry-run-aware Octokit instanceprepare.ts,publish.ts) and all targets to use wrapped APIsESLint Enforcement
Added
no-restricted-syntaxrules to catch:simpleGit()calls → usecreateGitClient()new Octokit()→ usegetGitHubClient()Testing & Documentation
Benefits
[dry-run] Would execute: ...log formatting