fix: HIGH quality-debt batch — logic bugs, exit codes, path typo, arg guards

[johnwaldo]

Summary

• model-registry-helper.sh: Merge duplicate if-blocks that made the print_info message unreachable — the first block (!= "0") always matched before the second (-gt 0) could run
• email-batch-convert-helper.sh: Return 1 when all conversions fail instead of always returning 0 — prevents cmd_batch from proceeding with stale/missing data
• system-cleanup.sh: Fix AGENT_DIR typo .agent → .agents to match LOG_DIR and TMP_DIR paths
• verify-brief.sh: Guard --repo-path against missing value under set -u to prevent unbound variable crash

Closes #3737
Closes #3697
Closes #3466
Closes #3253

Summary by CodeRabbit

• Bug Fixes

  • Reports an explicit failure when batch conversions were attempted but all failed
  • Validates repository path arguments to avoid silent misconfiguration
  • Logs when provider synchronization is skipped because prior syncs exist

• Chores

  • Changed default agent directory path
  • Code formatting and structural cleanups

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

• no-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d4f19a3f-6f57-4709-b867-d95fc4a64dc4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This pull request addresses four high-priority quality debt items identified in code reviews across shell automation scripts: adding failure detection when all email conversions fail, tightening numeric comparison logic with informational logging in provider syncing, standardizing the AGENT_DIR path constant, and adding argument validation to prevent silent empty parameter assignment.

Changes

Cohort / File(s)	Summary
Email Conversion Failure Detection .agents/scripts/email-batch-convert-helper.sh	Added exit code logic in cmd_convert to return 1 (non-zero) when all conversions fail (count > 0 && success == 0), ensuring pipeline short-circuits on total failure instead of returning success.
Provider Sync Logic Refinement .agents/scripts/model-registry-helper.sh	Changed opencode_synced pre-check from string inequality (!= "0") to numeric comparison (-gt 0) and added informational message explaining provider API probing skip reason.
Directory Path Standardization .agents/scripts/system-cleanup.sh	Updated AGENT_DIR constant from $HOME/.agent to $HOME/.agents for consistency with other directory paths (LOG_DIR, TMP_DIR); applied indentation and spacing formatting throughout.
Argument Validation Robustness .agents/scripts/verify-brief.sh	Added guard clause in parse_args to validate --repo-path argument has a value before assignment, returning exit code 2 with error message if missing instead of silently assigning empty value.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: add OpenCode model registry integration for daily model refresh (t194) #810: Modifies the same provider sync skip logic in .agents/scripts/model-registry-helper.sh, specifically the opencode_synced condition handling.

Suggested labels

bug, external-contributor

Poem

🛠️ Four scripts, four fixes, all debts now clear,
From conversions failing to paths shining here,
With numeric checks and validations sincere,
Our DevOps automation brings DevOps cheer! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	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 accurately summarizes the main changes: fixes to four shell scripts addressing logic bugs, exit codes, a path typo, and argument validation guards.
Linked Issues check	✅ Passed	All code changes directly address the requirements from linked issues #3737, #3697, #3466, and #3253, implementing the exact fixes specified.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the specific issues cited; no unrelated modifications or feature additions are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/scripts/email-batch-convert-helper.sh:
- Around line 102-105: The summary currently calls print_success before checking
the all-failed condition so a run can emit both success and failure; move the
print_success call so it occurs only after the all-failed check (i.e., check if
[[ "$count" -gt 0 && "$success" -eq 0 ]] then call print_error and return 1), or
change the prior summary print to a neutral status, ensuring print_success is
only invoked when conversions actually succeeded; update the section that uses
the count and success variables and the print_success / print_error calls
accordingly.

In @.agents/scripts/verify-brief.sh:
- Around line 81-85: The current arity check allows flag-like tokens as values
(e.g., "--repo-path --json") and sets REPO_PATH to a flag; update the validation
around the REPO_PATH assignment: after confirming $# >= 2, verify that the
candidate value ($2) is not a flag (does not start with '-') and if it is, call
log_fail "Missing value for --repo-path" and return 2; ensure REPO_PATH is only
assigned when the value passes this non-flag check so the parser is not left in
a broken state.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 079e458b-7b44-40c3-b786-56c44a317a85

📥 Commits

Reviewing files that changed from the base of the PR and between d932ae7 and 841a771.

📒 Files selected for processing (4)

• .agents/scripts/email-batch-convert-helper.sh
• .agents/scripts/model-registry-helper.sh
• .agents/scripts/system-cleanup.sh
• .agents/scripts/verify-brief.sh

💤 Files with no reviewable changes (1)

• .agents/scripts/model-registry-helper.sh

[johnwaldo]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[johnwaldo]

@coderabbitai review

Added 4 commits:

1. Review feedback fixes (success banner ordering, flag-like arg rejection)
2. 8 shell scripts: wappalyzer filename bug, comfy-cli arg guards, eeat-score empty URL guard, session-miner SCRIPT_DIR, watercrawl stderr, real-video-enhancer trap, ocr-receipt validation, test-supervisor-globals array safety
3. ttsr-rule-loader: session-stable state file, safe JSON with jq
4. package.json main entry, Python singular/plural grammar

[coderabbitai]

@johnwaldo Sure! Triggering a review of the new commits now. The expanded scope looks substantial — I'll cover the shell script fixes, the ttsr-rule-loader state file changes, and the package.json/Python grammar updates across all 4 commit batches.

---

🧠 Learnings used

Learnt from: marcusquinn
Repo: marcusquinn/aidevops PR: 0
File: :0-0
Timestamp: 2026-03-03T16:33:18.313Z
Learning: For the marcusquinn/aidevops repository: The daily quality sweep system (pulse-wrapper.sh) should only mention coderabbitai for full codebase review when there are significant changes (Quality Gate failure, +10 issues in one day, etc.), not on every automated sweep with stable metrics.

---

Learnt from: alex-solovyev
Repo: marcusquinn/aidevops PR: 0
File: :0-0
Timestamp: 2026-03-04T05:50:49.615Z
Learning: In the marcusquinn/aidevops repository: As of 2026-03-04, the chrome-webstore-helper.sh SC2119/SC2120 (load_credentials argument mismatch) and stash-audit-helper.sh SC2181 (indirect exit code checking) ShellCheck issues have been resolved. ShellCheck now reports 0 errors and 0 warnings for these files.

---

Learnt from: marcusquinn
Repo: marcusquinn/aidevops PR: 0
File: :0-0
Timestamp: 2026-03-08T11:00:09.839Z
Learning: In marcusquinn/aidevops, the correct npm package name for the Wappalyzer Node.js integration used in `.agents/tools/research/providers/wappalyzer.md` is `ryntab/wappalyzer-node` (not `AckeeCZ/wappalyzer` or similar). This was corrected in commit 253601d8 on PR `#3840`.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.