Guard load_repo_tools Against an Empty install.linux Set (#916)

[ptr727]

Summary

host-setup/linux/install-tools.sh --repo PATH died on a host-tools.json that declares
tools but zero install.linux entries, instead of treating "nothing to add" as a no-op.

Root cause

In load_repo_tools(), when no .tools[] entry has .install.linux, the jq filter
produces no output, so rows is the empty string. <<< "$rows" still feeds the
while read loop one line (a here-string always appends a trailing newline, even for
an empty string), so the loop ran once with name/manager/package all empty and
immediately died on the "non-empty tool name" check — even though the declaration was
otherwise valid and simply had nothing to add.

Fix

Guard the loop on an empty $rows before entering it, per the issue's suggested fix.

Testing

• Repro from the issue now reports normally instead of dying.
• Verified the normal path (repo host-tools.json with a real install.linux entry)
  still works.
• python3 -m unittest scripts.tests.test_host_gate — 83 tests pass.
• scripts/repo_gate.py and scripts/prose_lint.py — clean.

Fixes #916

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved Linux tool setup handling when no repository-specific tools are available.
  • Avoided unnecessary processing during installation in this scenario.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d22f9d0c-5124-4494-b20a-185f7bc2329a

📥 Commits

Reviewing files that changed from the base of the PR and between b1fec3e and e3957a7.

📒 Files selected for processing (1)

• host-setup/linux/install-tools.sh

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

load_repo_tools now returns successfully when repository metadata has no Linux installation entries. This prevents an empty here-string from entering the validation loop and producing an error.

Changes

Linux tool loading

Layer / File(s)	Summary
Empty metadata guard host-setup/linux/install-tools.sh	load_repo_tools exits successfully when no Linux repository tool metadata exists.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to e3957

This change makes an empty Linux tool set a no-op while preserving the existing path for declared tools; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the change: guarding load_repo_tools against an empty install.linux set.
Linked Issues check	✅ Passed	The change adds the requested empty-result guard, so repositories without install.linux entries are treated as a no-op.
Out of Scope Changes check	✅ Passed	The only reported change directly supports issue #916 and introduces no unrelated modifications.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-fix-916-install-tools-empty-rows

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[qodo-code-review]

PR Summary by Qodo

Guard repo tool loading when install.linux yields no rows

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Prevent load_repo_tools() from treating an empty install.linux set as an error.
• Return early when jq produces no rows, avoiding a spurious empty-iteration while read.
• Keeps valid host-tools.json declarations with “nothing to add” as a no-op.

Diagram

graph TD
  A["host-setup/linux/install-tools.sh"] --> B("load_repo_tools()") --> C("jq extracts rows") --> D{ "rows non-empty?" }
  D -->|"no"| E("return 0")
  D -->|"yes"| F("while read name/manager/package") --> G("validate fields") --> H("add linux installs")

  subgraph Legend
    direction LR
    _file["File"] ~~~ _proc("Process") ~~~ _dec{ "Decision" }
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Avoid here-string; stream jq output into the loop

• ➕ Eliminates the special here-string behavior (always appends a newline), so the loop naturally runs zero times on empty output
• ➕ Avoids storing potentially large output in a shell variable
• ➖ Slightly more complex shell plumbing (process substitution / pipe) and error-handling can be trickier in pipelines
• ➖ Requires care to preserve die behavior on jq failure across subshell/pipeline contexts

2. Make jq always emit an explicit sentinel row or count

• ➕ Makes “empty set” explicit and testable at the JSON-processing layer
• ➕ Could support richer diagnostics (e.g., count of rows)
• ➖ Adds complexity to the jq program and couples shell logic to jq output conventions
• ➖ Still needs special-case handling to avoid treating sentinel as a real tool row

Recommendation: The current approach (early return when $rows is empty) is the simplest, lowest-risk fix that directly addresses the here-string edge case without changing the jq contract or shell execution model. Consider streaming jq output only if row volume becomes large or if similar here-string pitfalls appear elsewhere.

Files changed (1) +1 / -0

Bug fix (1) +1 / -0

install-tools.shEarly-return when repo tool rows are empty +1/-0

Early-return when repo tool rows are empty

• Adds a guard that returns immediately when the 'jq' extraction produces no tab-separated rows. This prevents the subsequent 'while read' loop from executing once with empty fields due to here-string newline semantics.

host-setup/linux/install-tools.sh

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.