fix: use tomlq to parse dependency-groups.tools in uv-globals - #1368
Conversation
The script was reading project.dependencies (which doesn't exist) using dasel v2 syntax incompatible with dasel v3. Switch to tomlq (jq for TOML) and read from the correct dependency-groups.tools section. Also fix coverage spec and shellcheck warning in tmux-bridge.
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request refactors the UV globals installation script to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Mesa DescriptionTL;DRRefactored 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 refactors the uv-globals installation script to use tomlq for dependency extraction from dependency-groups.tools, replacing the previous dasel and jq implementation. It also simplifies the installation loop and updates the test suite and coverage specifications accordingly. A minor fix was applied to tmux-bridge.sh to ignore an unused variable. Feedback was provided to ensure the tomlq command handles missing keys gracefully by adding || true to prevent premature script termination if the dependency key is absent.
|
|
||
| # Parse dependencies from standard pyproject.toml format | ||
| DEPS=$(dasel -f "$PYPROJECT" -r toml -w json 'project.dependencies' 2>/dev/null | jq -r '.[]' 2>/dev/null || true) | ||
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) |
There was a problem hiding this comment.
Because the script uses set -e, if tomlq fails (for example, if the dependency-groups key is missing from pyproject.toml), the script will exit immediately with a non-zero status. This bypasses the graceful check on line 34. Adding || true ensures the script continues so it can handle the empty result gracefully, adhering to the principle of handling failures without blocking execution.
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) | |
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null || true) |
References
- Scripts should handle failures gracefully (e.g., during command execution or in retry loops) to avoid blocking execution flow, especially when set -e is in effect.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
home-manager/modules/uv-globals/install-uv-globals.sh (1)
32-36:⚠️ Potential issue | 🟠 MajorUse the optional operator to prevent early exit when
dependency-groups.toolsis missing.Line 32 will cause the script to exit immediately if the key is absent (before the empty-check at line 34), because
tomlqinherits jq's behavior and.["dependency-groups"].tools[]errors when the key is missing. Underset -e, this terminates the script. Redirect the key absence safely with?:Proposed fix
-DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) +DEPS=$(tomlq -r '.["dependency-groups"].tools[]?' "$PYPROJECT" 2>/dev/null || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@home-manager/modules/uv-globals/install-uv-globals.sh` around lines 32 - 36, The tomlq invocation assigning DEPS will fail under set -e if the .["dependency-groups"].tools path is missing; update the query in the DEPS assignment to use jq's optional operator so missing keys don't cause an error (e.g., change the query that references dependency-groups.tools to use the ? operator on the missing portions), leaving the rest of the logic (the empty-check and message using DEPS, PYPROJECT and tomlq) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@home-manager/modules/uv-globals/install-uv-globals.sh`:
- Around line 23-25: The script install-uv-globals.sh requires tomlq (checked
via command -v tomlq and used to parse pyproject.toml) but default.nix only
exports uv, dasel, and jq to PATH; update the PATH export in default.nix (the
line constructing export PATH=...) to include ${pkgs.tomlq}/bin so tomlq is
available at runtime, ensuring install-uv-globals.sh can find and run tomlq.
---
Outside diff comments:
In `@home-manager/modules/uv-globals/install-uv-globals.sh`:
- Around line 32-36: The tomlq invocation assigning DEPS will fail under set -e
if the .["dependency-groups"].tools path is missing; update the query in the
DEPS assignment to use jq's optional operator so missing keys don't cause an
error (e.g., change the query that references dependency-groups.tools to use the
? operator on the missing portions), leaving the rest of the logic (the
empty-check and message using DEPS, PYPROJECT and tomlq) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9c2ed73-439e-4636-97a7-788a824ae871
📒 Files selected for processing (4)
home-manager/modules/local-scripts/tmux-bridge.shhome-manager/modules/uv-globals/install-uv-globals.shspec/coverage_spec.shspec/uv_globals_spec.sh
| if ! command -v tomlq &>/dev/null; then | ||
| echo "tomlq not found, skipping uv globals install" | ||
| exit 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Verify the runtime check in installer/spec:
rg -n 'command -v tomlq|tomlq' home-manager/modules/uv-globals/install-uv-globals.sh spec/uv_globals_spec.sh
# Verify PATH/exported tools for uv-globals module:
rg -n 'export PATH=' home-manager/modules/uv-globals/default.nix
rg -n 'tomlq|dasel|jq' home-manager/modules/uv-globals/default.nixRepository: shunkakinoki/dotfiles
Length of output: 933
🏁 Script executed:
# Check if tomlq is referenced elsewhere in default.nix or related files
rg -n 'tomlq|pkgs.toml' home-manager/modules/uv-globals/default.nix
# Also check if there are other related nix files
fd -e nix home-manager/modules/uv-globals/ -exec rg -l 'tomlq' {} \;Repository: shunkakinoki/dotfiles
Length of output: 303
🏁 Script executed:
# Check if tomlq is referenced anywhere in the nix module files
rg -n 'tomlq' home-manager/modules/uv-globals/
# Also check the full default.nix to see buildInputs/dependencies
cat -n home-manager/modules/uv-globals/default.nixRepository: shunkakinoki/dotfiles
Length of output: 909
🏁 Script executed:
export PATH=${pkgs.uv}/bin:${pkgs.dasel}/bin:${pkgs.jq}/bin:${pkgs.tomlq}/bin:$PATHRepository: shunkakinoki/dotfiles
Length of output: 178
tomlq dependency is not provisioned in the module configuration.
Line 23 hard-requires tomlq, and line 32 uses it to parse pyproject.toml. However, home-manager/modules/uv-globals/default.nix (line 5) exports only uv, dasel, and jq to PATH. The script will always skip execution silently since command -v tomlq will fail.
Add ${pkgs.tomlq}/bin to the PATH export in default.nix line 5:
export PATH=${pkgs.uv}/bin:${pkgs.dasel}/bin:${pkgs.jq}/bin:${pkgs.tomlq}/bin:$PATH
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@home-manager/modules/uv-globals/install-uv-globals.sh` around lines 23 - 25,
The script install-uv-globals.sh requires tomlq (checked via command -v tomlq
and used to parse pyproject.toml) but default.nix only exports uv, dasel, and jq
to PATH; update the PATH export in default.nix (the line constructing export
PATH=...) to include ${pkgs.tomlq}/bin so tomlq is available at runtime,
ensuring install-uv-globals.sh can find and run tomlq.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the uv-globals installer to parse pyproject.toml using tomlq and to read tool dependencies from dependency-groups.tools, while also correcting coverage/spec entries and a ShellCheck issue.
Changes:
- Replace
dasel+jqTOML parsing withtomlqin the uv-globals installer and corresponding spec. - Fix dependency source key from
project.dependenciestodependency-groups.tools. - Update coverage spec to include
tmux-bridge.shand remove a duplicate entry; rename an unused read variable intmux-bridge.sh.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| spec/uv_globals_spec.sh | Updates expectations to require tomlq and the new dependency key path. |
| spec/coverage_spec.sh | Fixes coverage list (remove duplicate, add tmux-bridge.sh). |
| home-manager/modules/uv-globals/install-uv-globals.sh | Switches TOML parsing to tomlq and installs tools from dependency-groups.tools. |
| home-manager/modules/local-scripts/tmux-bridge.sh | Resolves unused variable warning by renaming read field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Parse dependencies from standard pyproject.toml format | ||
| DEPS=$(dasel -f "$PYPROJECT" -r toml -w json 'project.dependencies' 2>/dev/null | jq -r '.[]' 2>/dev/null || true) | ||
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) |
There was a problem hiding this comment.
The filter .["dependency-groups"].tools[] will error when the dependency-groups table or tools array is missing (or non-array). That can cause the script to exit early in environments that run bash with set -e, and it also makes the behavior more brittle than necessary. Use a non-throwing filter (e.g., optional iteration like []? / default-to-empty) and ensure failures don’t abort the script so the later -z check can handle the 'no deps' case deterministically.
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) | |
| DEPS=$(tomlq -r '.["dependency-groups"]?.tools? // [] | .[]?' "$PYPROJECT" 2>/dev/null || true) |
| echo "$DEPS" | while read -r pkg; do | ||
| if [ -n "$pkg" ]; then | ||
| echo "Installing $pkg..." | ||
| # Try default Python first, fall back to version from pyproject.toml | ||
| if ! uv tool install "$pkg" --force 2>/dev/null; then | ||
| if uv tool install "$pkg" --python "$PYTHON_VERSION" --force 2>/dev/null; then | ||
| echo "Installed $pkg with Python $PYTHON_VERSION" | ||
| else | ||
| echo "Failed to install $pkg, skipping..." | ||
| fi | ||
| fi | ||
| uv tool install "$pkg" --force 2>/dev/null || echo "Failed to install $pkg, skipping..." | ||
| fi | ||
| done |
There was a problem hiding this comment.
Piping via echo is less robust than printf because echo can treat leading -n/-e-like content as options and can be implementation-dependent. Prefer printf '%s\n' \"$DEPS\" | while ... (or, even better, avoid storing the whole list and stream directly from tomlq) to make the loop resilient to unusual dependency strings.
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) | ||
|
|
||
| if [ -z "$DEPS" ]; then | ||
| echo "No dependencies found in pyproject.toml" |
There was a problem hiding this comment.
The message is now misleading because the script no longer looks for general dependencies; it specifically reads dependency-groups.tools. Suggest updating the message to reflect the actual source (e.g., 'No dependency-groups.tools entries found in pyproject.toml') so failures are easier to diagnose.
| echo "No dependencies found in pyproject.toml" | |
| echo "No dependency-groups.tools entries found in pyproject.toml" |
| When run bash -c "grep 'dependency-groups' '$SCRIPT'" | ||
| The output should include 'dependency-groups' |
There was a problem hiding this comment.
This assertion is quite broad: it will pass if the script contains the text dependency-groups anywhere, even if it doesn’t actually parse .dependency-groups.tools. Consider grepping for the specific tomlq filter/path used (e.g., .[\"dependency-groups\"].tools / dependency-groups.tools) to make the test accurately match the behavior described by the test name.
| When run bash -c "grep 'dependency-groups' '$SCRIPT'" | |
| The output should include 'dependency-groups' | |
| When run bash -c "grep -E '\\[\"dependency-groups\"\\]\\.tools|dependency-groups\\.tools' '$SCRIPT'" | |
| The output should include '.tools' |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/modules/uv-globals/install-uv-globals.sh">
<violation number="1" location="home-manager/modules/uv-globals/install-uv-globals.sh:23">
P0: `tomlq` is not provisioned in the Nix module. `home-manager/modules/uv-globals/default.nix` only exports `uv`, `dasel`, and `jq` to `PATH`. Since `tomlq` is never on `PATH`, `command -v tomlq` will always fail and the script will silently skip execution every time. Add `${pkgs.tomlq}/bin` to the `PATH` export in `default.nix`.</violation>
<violation number="2" location="home-manager/modules/uv-globals/install-uv-globals.sh:32">
P2: Guard the `tomlq` extraction against missing `dependency-groups.tools`; otherwise `set -e` can terminate the script before the empty-deps check runs.</violation>
</file>
<file name="spec/uv_globals_spec.sh">
<violation number="1" location="spec/uv_globals_spec.sh:74">
P2: The spec claims to verify `dependency-groups.tools`, but it only matches `dependency-groups`, making the test too weak and prone to false positives.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| if ! command -v jq &>/dev/null; then | ||
| echo "jq not found, skipping uv globals install" | ||
| if ! command -v tomlq &>/dev/null; then |
There was a problem hiding this comment.
P0: tomlq is not provisioned in the Nix module. home-manager/modules/uv-globals/default.nix only exports uv, dasel, and jq to PATH. Since tomlq is never on PATH, command -v tomlq will always fail and the script will silently skip execution every time. Add ${pkgs.tomlq}/bin to the PATH export in default.nix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/uv-globals/install-uv-globals.sh, line 23:
<comment>`tomlq` is not provisioned in the Nix module. `home-manager/modules/uv-globals/default.nix` only exports `uv`, `dasel`, and `jq` to `PATH`. Since `tomlq` is never on `PATH`, `command -v tomlq` will always fail and the script will silently skip execution every time. Add `${pkgs.tomlq}/bin` to the `PATH` export in `default.nix`.</comment>
<file context>
@@ -8,40 +8,28 @@ if ! timeout 3 bash -c 'exec 3<>/dev/tcp/1.1.1.1/53' 2>/dev/null; then
-
-if ! command -v jq &>/dev/null; then
- echo "jq not found, skipping uv globals install"
+if ! command -v tomlq &>/dev/null; then
+ echo "tomlq not found, skipping uv globals install"
exit 0
</file context>
|
|
||
| # Parse dependencies from standard pyproject.toml format | ||
| DEPS=$(dasel -f "$PYPROJECT" -r toml -w json 'project.dependencies' 2>/dev/null | jq -r '.[]' 2>/dev/null || true) | ||
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) |
There was a problem hiding this comment.
P2: Guard the tomlq extraction against missing dependency-groups.tools; otherwise set -e can terminate the script before the empty-deps check runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/uv-globals/install-uv-globals.sh, line 32:
<comment>Guard the `tomlq` extraction against missing `dependency-groups.tools`; otherwise `set -e` can terminate the script before the empty-deps check runs.</comment>
<file context>
@@ -8,40 +8,28 @@ if ! timeout 3 bash -c 'exec 3<>/dev/tcp/1.1.1.1/53' 2>/dev/null; then
-
-# Parse dependencies from standard pyproject.toml format
-DEPS=$(dasel -f "$PYPROJECT" -r toml -w json 'project.dependencies' 2>/dev/null | jq -r '.[]' 2>/dev/null || true)
+DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null)
if [ -z "$DEPS" ]; then
</file context>
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]' "$PYPROJECT" 2>/dev/null) | |
| DEPS=$(tomlq -r '.["dependency-groups"].tools[]?' "$PYPROJECT" 2>/dev/null || true) |
| When run bash -c "grep 'project.dependencies' '$SCRIPT'" | ||
| The output should include 'project.dependencies' | ||
| It 'parses dependency-groups.tools' | ||
| When run bash -c "grep 'dependency-groups' '$SCRIPT'" |
There was a problem hiding this comment.
P2: The spec claims to verify dependency-groups.tools, but it only matches dependency-groups, making the test too weak and prone to false positives.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/uv_globals_spec.sh, line 74:
<comment>The spec claims to verify `dependency-groups.tools`, but it only matches `dependency-groups`, making the test too weak and prone to false positives.</comment>
<file context>
@@ -75,9 +70,9 @@ When run bash -c "grep -- '--force' '$SCRIPT'"
-When run bash -c "grep 'project.dependencies' '$SCRIPT'"
-The output should include 'project.dependencies'
+It 'parses dependency-groups.tools'
+When run bash -c "grep 'dependency-groups' '$SCRIPT'"
+The output should include 'dependency-groups'
End
</file context>
…e-swap version in pyproject.toml
…on flag to uv tool install
Summary
dasel+jqwithtomlqfor TOML parsing in uv-globals installerproject.dependencies->dependency-groups.tools)tmux-bridge.sh, remove duplicateauto-switch.sh)wnamevariable intmux-bridge.shTest plan
make shell-testpasses (961 examples, 0 failures)make shell-lintpasses (shellcheck clean)tomlqcorrectly parses all 5 tools from pyproject.tomlSummary by cubic
Switch
uv-globalsto usetomlqand readdependency-groups.toolsfrompyproject.toml. Always install tools with the Python version fromproject.requires-pythonto fix failed global installs, and update tests and a shellcheck warning.Bug Fixes
dependency-groups.toolswithtomlq; removeddasel/jq.--pythonfromproject.requires-python(default3.13) touv tool install --force.requires-pythonand passing--python; update specs to expecttomlq; coverage cleanup; fix unused var intmux-bridge.sh.Dependencies
claude-swapto>=0.7.1inpyproject.toml.Written for commit 980e697. Summary will update on new commits.