Add --script mode to run command for inline metadata scripts#21
Conversation
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
|
@cubic-dev-ai review |
@pirate I have started the AI code review. It will take a few minutes to complete. |
41370ca to
74ae064
Compare
1608e20 to
e9523c0
Compare
f5604fd to
d41aec6
Compare
40dcd80 to
78aafd7
Compare
| try: | ||
| return self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
shouldnt this be in the base method not the override?
78aafd7 to
130e145
Compare
| def default_abspath_handler( | ||
| self, | ||
| bin_name: BinName | HostBinPath, | ||
| no_cache: bool = False, | ||
| **context, | ||
| ) -> HostBinPath | None: | ||
| if str(bin_name) == self.INSTALLER_BIN: | ||
| return self.INSTALLER_BIN_ABSPATH | ||
| try: | ||
| return self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath | ||
| except Exception: | ||
| return None | ||
| bin_dir = self.bin_dir | ||
| assert bin_dir is not None | ||
| link_path = bin_dir / str(bin_name) |
There was a problem hiding this comment.
cant this just be on the base? then we dont need this same override everywhere.
Checking bin_dir for the bin_name seems like the most common pattern
8aad844 to
8536a0f
Compare
8536a0f to
d899150
Compare
d899150 to
bbe308c
Compare
bbe308c to
d168e0d
Compare
- parse_script_metadata(): comment-syntax-agnostic /// script parser - --script flag on run command with automatic dep resolution - BinProvider.ENV computed property + apply_env() for runtime env vars - ENV overrides on all 14 providers - [tool.abx-pkg] values passed through as env vars verbatim - Removed MANAGED_PROVIDER_ROOTS — providers resolve install_root from ABX_PKG_LIB_DIR via default_factory - abx_pkg_install_root_default reads env vars fresh (not cached) - Playwright/puppeteer: 3-mode npm resolution (global/managed/hermetic) https://claude.ai/code/session_0125eDj24UMFUhMUN8x2zmHB
d168e0d to
48996d7
Compare
d5c5c17 to
64f8e5c
Compare
There was a problem hiding this comment.
🔴 Yarn Berry install drops --force flag when postinstall_scripts=True and no_cache=True
In default_install_handler, the condition for adding --force was changed from if no_cache and "--force" not in cmd to if no_cache and not is_berry and "--force" not in cmd. For Yarn Berry with postinstall_scripts=True, the code never enters the Berry-specific if is_berry and not postinstall_scripts: branch (which is the only other place --force is added for Berry). This means --no-cache no longer forces a fresh install for Yarn Berry when postinstall scripts are enabled, silently serving stale cached packages.
Old vs new behavior trace
Old code path (Berry, postinstall_scripts=True, no_cache=True):
cmd = ["add", ...]no_cache=True→--forceaddedis_berry and not postinstall_scripts= False → skip- Result:
--forcepresent ✅
New code path:
cmd = ["add", ...]no_cache and not is_berry= False →--forceNOT addedis_berry and not postinstall_scripts= False → skip- Result:
--forcemissing ❌
(Refers to line 296)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if not postinstall_scripts: | ||
| cmd = [ | ||
| "up", | ||
| *self.yarn_install_args, | ||
| *( | ||
| ["--force"] | ||
| if no_cache and "--force" not in self.yarn_install_args | ||
| else [] | ||
| ), | ||
| "--mode", | ||
| "skip-build", | ||
| *install_args, |
There was a problem hiding this comment.
🔴 Yarn Berry update handler completely drops --force for no_cache=True
The default_update_handler for Yarn Berry entirely removes the --force flag logic. In the old code, --force was added for Berry when no_cache=True in both the postinstall_scripts=True and postinstall_scripts=False branches. The new code removes the --force insertion for Berry in both branches, so --no-cache on update has no effect for Berry Yarn. The not postinstall_scripts branch also lost its --force injection (abx_pkg/binprovider_yarn.py:383-398).
(Refers to lines 376-383)
Prompt for agents
In binprovider_yarn.py default_update_handler, the Berry branch (is_berry=True) no longer adds --force when no_cache=True. The old code had:
if is_berry:
cmd = ["up", *self.yarn_install_args, *install_args]
if no_cache and "--force" not in cmd:
cmd.insert(1, "--force")
The new code removes the --force insertion entirely for Berry. Need to restore --force for no_cache=True in the Berry update path. This affects both the postinstall_scripts=True path (line 376, where cmd is built but --force never added) and the postinstall_scripts=False path (lines 378-383, where the cmd is rebuilt without --force).
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
This PR adds a new
--scriptmode to theabx-pkg runcommand that enables executing scripts with inline/// scriptmetadata blocks. The feature automatically parses script metadata, resolves declared dependencies, and executes the script with a PATH that includes all resolved binaries.Key Changes
New
parse_script_metadata()function: Extracts TOML-formatted metadata from/// scriptblocks in script files. Supports multiple comment styles (#,//,--,;, etc.) and handles multi-line TOML structures with proper indentation preservation.--scriptflag for run command: When enabled, treats the binary name as an interpreter and the first argument as a script file path. Parses the script's metadata, resolves all declared dependencies, and executes the script with an augmented PATH.Metadata-driven dependency resolution: Scripts can declare dependencies as strings or objects with additional options (custom binproviders, min_version). The resolver collects PATH entries from all successfully resolved binaries.
Tool configuration support: Scripts can include
[tool.abx-pkg]sections in metadata to set defaults forpostinstall_scripts,min_release_age, andmin_version. CLI flags take precedence over metadata settings.Comprehensive test coverage: Added 16 unit tests for metadata parsing (various comment styles, edge cases, TOML structures) and 7 integration tests for the full
run --scriptworkflow (argument passing, error handling, exit code propagation, CLI override behavior).Implementation Details
///marker; unclosed blocks returnNoneabx-pkgunchanged--dry-run) resolves dependencies but skips actual script executionhttps://claude.ai/code/session_0125eDj24UMFUhMUN8x2zmHB
Summary by cubic
Adds
--scriptmode toabx-pkg runto execute scripts with inline/// scriptmetadata, auto-resolve dependencies, and set PATH/ENV for the chosen interpreter. Also unifies installer discovery and runtime ENV across providers, and makes version reporting resilient to missing installers.New Features
run --script INTERPRETER SCRIPT_PATH [ARGS...](shebang:#!/usr/bin/env -S abx-pkg run --script node). Implies--install;--dry-runonly resolves./// scriptTOML block.[tool.abx-pkg]values export to env vars (CLI flags still override). README adds examples.playwright/puppeteer: global/managed/hermeticnpmresolution.Refactors
INSTALLER_BINARY(), guarding abspath lookups and avoidingdetect_euidrecursion; version report skips missing installer binaries.ENV;apply_env()merges vars. Sets common vars likeVIRTUAL_ENV,PYTHONPATH,NODE_PATH,HOMEBREW_PREFIX,GEM_HOME,GOPATH,DENO_DIR,PLAYWRIGHT_BROWSERS_PATH,PUPPETEER_CACHE_DIR.abx_pkg_install_root_default()which readsABX_PKG_LIB_DIRat runtime; removedMANAGED_PROVIDER_ROOTS.tests,release, anddeploy-pages.Written for commit 18eeb66. Summary will update on new commits.