Skip to content

feat(approve-builds): positional args, !pkg deny syntax, and auto-populate allowBuilds#11030

Merged
zkochan merged 30 commits intomainfrom
approve-builds
Mar 20, 2026
Merged

feat(approve-builds): positional args, !pkg deny syntax, and auto-populate allowBuilds#11030
zkochan merged 30 commits intomainfrom
approve-builds

Conversation

@zkochan
Copy link
Copy Markdown
Member

@zkochan zkochan commented Mar 19, 2026

Summary

pnpm approve-builds positional arguments

  • pnpm approve-builds foo — approves foo, leaves everything else untouched
  • pnpm approve-builds !bar — denies bar, leaves everything else untouched
  • pnpm approve-builds foo !bar — approves foo, denies bar
  • Only mentioned packages are modified; unmentioned packages remain pending
  • --all cannot be combined with positional arguments
  • Contradictory arguments (pkg !pkg) are rejected

Auto-populate allowBuilds during install

  • When pnpm install encounters packages with build scripts that aren't yet in allowBuilds, they are automatically written to pnpm-workspace.yaml with a 'set this to true or false' placeholder
  • Users can then edit the config directly instead of running approve-builds
  • The placeholder behaves like a missing entry: builds are skipped and strictDepBuilds still fails
  • Existing allowBuilds entries are preserved (only new packages get placeholders)

Test plan

  • Approve specific packages via positional args (no prompt, correct allowBuilds)
  • Deny packages via !pkg syntax
  • Mixed approve + deny in one command
  • Deny-only keeps other builds pending in ignoredBuilds
  • Unknown packages (approved or denied) throw error
  • Contradictory arguments throw error
  • --all with positional arguments throws error
  • Positional args preserve existing allowBuilds entries
  • Interactive and --all flows unchanged
  • Auto-populated placeholder entries appear in pnpm-workspace.yaml after install
  • Auto-populated placeholders merge with existing allowBuilds
  • Existing --allow-build test updated for placeholder behavior

🤖 Generated with Claude Code

zkochan and others added 10 commits March 19, 2026 15:47
… without prompt

Allow `pnpm approve-builds pkg1 pkg2 ...` to approve specific packages
directly without the interactive multiselect prompt. Only the listed
packages are added to allowBuilds; other pending packages are left
untouched for later approval.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prefix a package name with ! to explicitly deny it, e.g.
pnpm approve-builds foo !bar

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ckages

When positional args are used, packages not explicitly approved or denied
get allowBuilds: 'warn' instead of false. This skips their build but
does not cause strictDepBuilds to fail — only a warning is printed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Revert the 'warn' AllowBuild value and !pkg syntax. Instead:

- During install, packages with ignored builds that aren't yet in
  allowBuilds are automatically written as 'pending' entries in
  pnpm-workspace.yaml. Users can then manually edit them to true/false.
- 'pending' behaves like a missing entry: builds are skipped and
  strictDepBuilds still fails, but the entry is visible in config.
- Export dedupePackageNamesFromIgnoredBuilds from deps-installer.
- Extract handleIgnoredBuilds helper shared by installDeps and recursive.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
More self-documenting than 'pending' — it reads as a prompt telling
the user what values are expected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ages

When positional args are used, only the listed packages are modified:
- `pnpm approve-builds foo` sets foo to true
- `pnpm approve-builds !bar` sets bar to false
- Unmentioned packages are left untouched

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zkochan zkochan changed the title feat(approve-builds): accept positional arguments feat(approve-builds): positional args, !pkg deny syntax, and auto-populate allowBuilds Mar 20, 2026
@zkochan zkochan marked this pull request as ready for review March 20, 2026 00:27
Copilot AI review requested due to automatic review settings March 20, 2026 00:27
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 non-interactive workflows for pnpm approve-builds (positional approve/deny args) and introduces an install-time helper to persist ignored-build packages into allowBuilds (with a placeholder) so users can edit pnpm-workspace.yaml directly.

Changes:

  • Extend approve-builds to accept positional package args and !pkg deny syntax (skipping prompts when args are provided).
  • Add handleIgnoredBuilds() and route strict ignored-build handling through it from install/recursive flows.
  • Export dedupePackageNamesFromIgnoredBuilds() for reuse and document the behavior via a changeset.

Reviewed changes

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

Show a summary per file
File Description
installing/deps-installer/src/install/index.ts Exports the ignored-builds package-name dedupe helper for reuse.
installing/commands/src/recursive.ts Replaces direct strict ignored-build throwing with handleIgnoredBuilds() calls.
installing/commands/src/installDeps.ts Same refactor to handleIgnoredBuilds() for single-project install flows.
installing/commands/src/handleIgnoredBuilds.ts New helper: auto-writes placeholder allowBuilds entries and throws in strict mode.
building/commands/src/policy/approveBuilds.ts Adds positional args + !pkg deny support and updates help usages.
building/commands/test/policy/approveBuilds.test.ts Adds tests for positional approve/deny and unknown package args.
.changeset/approve-builds-positional-args.md Releases notes/versions for the new CLI + install behavior.

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

zkochan and others added 2 commits March 20, 2026 01:38
Collect all ignoredBuilds across concurrent project installs, then
write placeholder entries to allowBuilds once after all projects
complete. The strictDepBuilds error is still thrown per-project to
respect the bail option.

Also split handleIgnoredBuilds into two functions so the write and
throw concerns can be used independently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zkochan zkochan requested a review from Copilot March 20, 2026 00:57
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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.


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

zkochan and others added 6 commits March 20, 2026 02:08
- Guard modulesManifest.ignoredBuilds cleanup so it only happens in
  interactive/--all flows, not when positional args are used
- Write placeholders before throwing in recursive strict mode
- Add deny-only test to verify other builds remain pending

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The placeholder entries in allowBuilds were being written with exact
versions (e.g. esbuild@0.27.4) instead of just the package name.
Use parse() from @pnpm/deps.path to extract just the name.

Also unexport dedupePackageNamesFromIgnoredBuilds since it's no longer
used externally.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

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

- When positional args are used, remove only the decided packages from
  modulesManifest.ignoredBuilds instead of leaving them stale
- Throw an error when a package appears both approved and denied
  (e.g. pnpm approve-builds pkg !pkg)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zkochan and others added 3 commits March 20, 2026 12:23
- Remove per-project writeIgnoredBuildsToAllowBuilds and
  IgnoredBuildsError from the concurrent loop in recursive.ts
- Just collect allIgnoredBuilds during the loop, then call
  handleIgnoredBuilds once after Promise.all completes
- Unexport writeIgnoredBuildsToAllowBuilds (only used internally)
- Both installDeps.ts and recursive.ts now use the same pattern:
  call handleIgnoredBuilds once after install, which writes
  placeholders and throws IgnoredBuildsError if strictDepBuilds

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- !pkg with unknown package throws error
- Contradictory arguments (pkg !pkg) throw error
- --all with positional arguments throws error
- Positional args preserve existing allowBuilds entries and
  don't touch unmentioned packages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

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

zkochan and others added 4 commits March 20, 2026 12:53
rebuild.handler overwrites ignoredBuilds in .modules.yaml with only
the packages it processed. After rebuild, re-read the modules manifest
and restore ignoredBuilds for undecided packages so they remain
pending for future approve-builds runs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the fix from approve-builds into buildSelectedPkgs where the root
cause is. When rebuilding a subset of packages, merge the new
ignoredBuilds with existing ones, keeping entries for packages that
weren't part of this rebuild.

Simplify approve-builds to just re-read the manifest after rebuild and
remove decided entries.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the re-read workaround since rebuild now properly preserves
ignoredBuilds via mergeIgnoredBuilds. The approve-builds handler
goes back to the original pattern: return early after rebuild, or
clear ignoredBuilds in the no-rebuild interactive/--all path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

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

Denied packages left in .modules.yaml ignoredBuilds would be
re-merged by the next install, causing strictDepBuilds to keep
failing. Clean up ignoredBuilds for all decided packages (approved
and denied) before calling rebuild, so only truly pending packages
remain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

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

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zkochan zkochan merged commit 996284f into main Mar 20, 2026
10 of 12 checks passed
@zkochan zkochan deleted the approve-builds branch March 20, 2026 13:58
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