t1323: Fix TTSR shell-local-params rule false-positives on currency/pricing text#2296
Conversation
…alse-positives The shell-local-params TTSR rule regex $[1-9] was matching dollar amounts like $5/mo, $9 per month in markdown tables and pricing text. Expanded negative lookaheads across all three enforcement points (TTSR plugin, linters-local.sh, pre-commit-hook.sh) to exclude: - $[1-9] followed by pipe (markdown table cell boundary) - $[1-9] followed by common currency unit words (per, mo, month, year, flat, each, off, fee, plan, tier, user, seat, unit, addon, setup, trial, credit, annual, quarterly, monthly) Closes #2194
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThree shell script linting files are updated to fix false positives in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Feb 25 17:29:25 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![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 effectively addresses false-positives in the shell-local-params rule by enhancing the detection regex to ignore common currency and pricing patterns. The changes are consistently applied across the TTSR plugin, local linter, and pre-commit hook. My review focuses on improving the maintainability of these new regular expressions and shell script logic, applying repository-specific optimization rules. I've identified opportunities to reduce code duplication and improve efficiency in linters-local.sh and pre-commit-hook.sh, which will make the code more efficient and easier to manage in the future. I've also suggested minor simplifications to the regular expressions themselves for conciseness.
| if [[ -f "$file" ]] && grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' >/dev/null; then | ||
| print_error "Direct positional parameter usage in $file" | ||
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | head -3 | ||
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' | head -3 | ||
| ((violations++)) | ||
| fi |
There was a problem hiding this comment.
The long grep pipeline is executed twice: once in the if condition to check for violations, and then again to print the first few violating lines. This is inefficient and creates significant code duplication, making the script harder to maintain. A better approach is to run the pipeline once, store its output in a variable, and then use that variable for both the check and the output. This improves performance and eliminates the duplicated code.
| if [[ -f "$file" ]] && grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' >/dev/null; then | |
| print_error "Direct positional parameter usage in $file" | |
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | head -3 | |
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' | head -3 | |
| ((violations++)) | |
| fi | |
| if [[ -f "$file" ]]; then | |
| local violations_output | |
| violations_output=$(grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|y(ea)?r|day|week|h(ou)?r|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b') | |
| if [[ -n "$violations_output" ]]; then | |
| print_error "Direct positional parameter usage in $file" | |
| echo "$violations_output" | head -3 | |
| ((violations++)) | |
| fi | |
| fi |
References
- The principle of consolidating multiple executions of a command/pipeline into a single pass to improve performance and efficiency applies here, even though it's a
greppipeline instead ofjq. - Storing the output of the
greppipeline inviolations_outputbefore using it multiple times is analogous to moving a loop-invariant calculation outside a loop, as it avoids redundant computation/execution and improves efficiency.
| // - $[1-9] followed by digits, decimal, comma, or slash (e.g. $28/mo, $1.99, $1,000) | ||
| // - $[1-9] followed by pipe (markdown table cell boundary) | ||
| // - $[1-9] followed by common currency/pricing unit words (per, mo, month, flat, etc.) | ||
| pattern: "^\\s+(?:echo|printf|return|if|\\[\\[).*\\$[1-9](?![0-9.,/])(?!\\s*[|])(?!\\s+(?:per|mo(?:nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\\b)(?!.*local\\s+\\w+=)", |
There was a problem hiding this comment.
For improved readability and maintainability of this complex regular expression, you can make it more concise by grouping related alternatives. For example, year|yr can be combined into y(?:ea)?r, and hr|hour can be h(?:ou)?r. This makes the pattern slightly shorter and easier to parse visually.
| pattern: "^\\s+(?:echo|printf|return|if|\\[\\[).*\\$[1-9](?![0-9.,/])(?!\\s*[|])(?!\\s+(?:per|mo(?:nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\\b)(?!.*local\\s+\\w+=)", | |
| pattern: "^\\s+(?:echo|printf|return|if|\\[\\[).*\\$[1-9](?![0-9.,/])(?!\\s*[|])(?!\\s+(?:per|mo(?:nth)?|y(?:ea)?r|day|week|h(?:ou)?r|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\\b)(?!.*local\\s+\\w+=)", |
| /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)[[:space:][:punct:]]/ { next } | ||
| /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)$/ { next } |
There was a problem hiding this comment.
These two lines are nearly identical, with the only difference being the match at the end of the line ([[:space:][:punct:]] vs. $). You can combine them into a single, more maintainable rule by using an alternation ([[:space:][:punct:]]|$). This avoids duplicating the long list of keywords and makes future updates easier. I've also included a small simplification for the keywords themselves.
| /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)[[:space:][:punct:]]/ { next } | |
| /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)$/ { next } | |
| /\$[1-9][[:space:]]+(per|mo(nth)?|y(ea)?r|day|week|h(ou)?r|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)([[:space:][:punct:]]|$)/ { next } |
🤖 Augment PR SummarySummary: Reduces false-positives in the 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| // - $[1-9] followed by digits, decimal, comma, or slash (e.g. $28/mo, $1.99, $1,000) | ||
| // - $[1-9] followed by pipe (markdown table cell boundary) | ||
| // - $[1-9] followed by common currency/pricing unit words (per, mo, month, flat, etc.) | ||
| pattern: "^\\s+(?:echo|printf|return|if|\\[\\[).*\\$[1-9](?![0-9.,/])(?!\\s*[|])(?!\\s+(?:per|mo(?:nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\\b)(?!.*local\\s+\\w+=)", |
There was a problem hiding this comment.
The new (?!\\s*[|]) exclusion will also suppress matches for real shell pipelines like echo $1 | ..., potentially turning true violations into false negatives. Is that tradeoff intended, given | is common shell syntax and not unique to markdown tables?
Severity: medium
Other Locations
.agents/scripts/linters-local.sh:194.agents/scripts/pre-commit-hook.sh:83
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| if [[ -f "$file" ]] && grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' >/dev/null; then | ||
| # slash (e.g. $28/mo, $1.99, $1,000), pipe (markdown table cell), or common | ||
| # currency/pricing unit words (per, mo, month, flat, etc.). | ||
| if [[ -f "$file" ]] && grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' >/dev/null; then |
There was a problem hiding this comment.
grep -E (ERE/POSIX) doesn’t reliably support escapes like \\s and \\b, so these new exclusions may not work and could reintroduce the currency false-positives this PR is trying to eliminate. This affects both the if check and the diagnostic pipeline below.
Severity: high
Other Locations
.agents/scripts/pre-commit-hook.sh:85
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![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)
.agents/plugins/opencode-aidevops/index.mjs (1)
787-811:⚠️ Potential issue | 🟠 Major
validatePositionalParamsis missing the new pipe and pricing-unit exclusions added to the TTSR rule.The TTSR
shell-local-paramspattern now skips lines that contain$N |(pipe) and$N per/mo/month/…(pricing units), but the inlinevalidatePositionalParamsfunction (used by the hard quality gate intool.execute.before) still only has the old digit/decimal exclusion on line 800:if (/\$[1-9][0-9.,/]/.test(trimmed)) { continue; }The pipe and pricing-unit cases will continue to trigger false-positive violations in the inline gate even though they are now suppressed in the TTSR rule — the two enforcement paths are out of sync.
🐛 Proposed fix — bring validatePositionalParams in line with the TTSR rule
- // Skip currency/pricing patterns: $[1-9] followed by digits, decimal, comma, - // or slash (e.g. $28/mo, $1.99, $1,000) — false-positives in markdown tables. - if (/\$[1-9][0-9.,/]/.test(trimmed)) { - continue; - } + // Skip currency/pricing patterns: $[1-9] followed by digits, decimal, comma, slash, + // pipe (markdown table boundary), or a common pricing-unit word (per, mo, month, …). + if (/\$[1-9][0-9.,/]/.test(trimmed)) continue; + if (/\$[1-9]\s*\|/.test(trimmed)) continue; + if (/\$[1-9]\s+(?:per|mo(?:nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b/.test(trimmed)) continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/plugins/opencode-aidevops/index.mjs around lines 787 - 811, The inline validatePositionalParams function is out-of-sync with the TTSR rule: replace the current exclusion that checks /\$[1-9][0-9.,/]/ (the check inside validatePositionalParams) with an updated pattern that also ignores pipes and pricing-unit suffixes (e.g. "$N |" and "$N per/mo/month/yr/year" variants) so the function continues when a positional-parameter token is followed by digits, decimal, comma, slash, a pipe character, or common pricing unit words; keep the existing continue behavior and update only the regex used in validatePositionalParams to mirror the TTSR shell-local-params exclusions.
🧹 Nitpick comments (1)
.agents/scripts/linters-local.sh (1)
194-196: LGTM — new awk exclusion rules are correct and idiomatic.Using POSIX character classes (
[[:space:]],[[:punct:]]) and the EREmo(nth)?optional group is portable across macOS and Linux awk. The two-pattern approach (trailing space/punct vs. EOL) correctly handles both mid-line and end-of-line pricing phrases.As a minor optional nicety, the two unit-word patterns could be merged into one:
♻️ Optional: collapse lines 195-196 into a single pattern
- /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)[[:space:][:punct:]]/ { next } - /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)$/ { next } + /\$[1-9][[:space:]]+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)([[:space:][:punct:]]|$)/ { next }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/linters-local.sh around lines 194 - 196, Merge the two awk exclusion patterns that match dollar amounts with unit words (the two patterns in .agents/scripts/linters-local.sh that begin with the price matcher for unit words) into a single pattern: keep the same unit-word alternation and POSIX classes ([[:space:]] and [[:punct:]]) but combine the "followed by space/punct" and "end-of-line" cases into one expression by making the trailing separator a single alternation/optional group (separator or EOL), then replace the two separate / { next } rules with one consolidated / { next } rule so behavior is unchanged but the rule is collapsed.
🤖 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/pre-commit-hook.sh:
- Around line 83-85: Replace the duplicated grep pipeline by capturing its
output into a variable (e.g., matches) and reusing that variable for the
existence check and printing; specifically, run the pipeline that starts with
grep -n '\$[1-9]' "$file" and the subsequent grep -v/grep -vE filters once,
assign its (possibly empty) result to matches (use "|| true" to avoid non-zero
exit), then change the if condition to check [[ -f "$file" ]] && [[ -n
"$matches" ]], call print_error "Direct positional parameter usage in $file",
echo "$matches" | head -3, and increment ((violations++)); update references to
the original duplicated pipeline locations to use matches instead.
---
Outside diff comments:
In @.agents/plugins/opencode-aidevops/index.mjs:
- Around line 787-811: The inline validatePositionalParams function is
out-of-sync with the TTSR rule: replace the current exclusion that checks
/\$[1-9][0-9.,/]/ (the check inside validatePositionalParams) with an updated
pattern that also ignores pipes and pricing-unit suffixes (e.g. "$N |" and "$N
per/mo/month/yr/year" variants) so the function continues when a
positional-parameter token is followed by digits, decimal, comma, slash, a pipe
character, or common pricing unit words; keep the existing continue behavior and
update only the regex used in validatePositionalParams to mirror the TTSR
shell-local-params exclusions.
---
Nitpick comments:
In @.agents/scripts/linters-local.sh:
- Around line 194-196: Merge the two awk exclusion patterns that match dollar
amounts with unit words (the two patterns in .agents/scripts/linters-local.sh
that begin with the price matcher for unit words) into a single pattern: keep
the same unit-word alternation and POSIX classes ([[:space:]] and [[:punct:]])
but combine the "followed by space/punct" and "end-of-line" cases into one
expression by making the trailing separator a single alternation/optional group
(separator or EOL), then replace the two separate / { next } rules with one
consolidated / { next } rule so behavior is unchanged but the rule is collapsed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.agents/plugins/opencode-aidevops/index.mjs.agents/scripts/linters-local.sh.agents/scripts/pre-commit-hook.sh
| if [[ -f "$file" ]] && grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' >/dev/null; then | ||
| print_error "Direct positional parameter usage in $file" | ||
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | head -3 | ||
| grep -n '\$[1-9]' "$file" | grep -v 'local.*=.*\$[1-9]' | grep -vE '\$[1-9][0-9.,/]' | grep -vE '\$[1-9]\s*\|' | grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' | head -3 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the pre-commit-hook.sh file
find . -name "pre-commit-hook.sh" -type fRepository: marcusquinn/aidevops
Length of output: 100
🏁 Script executed:
# Read the file around the reported lines
if [ -f ".agents/scripts/pre-commit-hook.sh" ]; then
wc -l ".agents/scripts/pre-commit-hook.sh"
fiRepository: marcusquinn/aidevops
Length of output: 102
🏁 Script executed:
# Get context around lines 80-85
if [ -f ".agents/scripts/pre-commit-hook.sh" ]; then
sed -n '75,90p' ".agents/scripts/pre-commit-hook.sh"
fiRepository: marcusquinn/aidevops
Length of output: 1083
🌐 Web query:
macOS BSD grep \s \b support extended regular expressions ERE compatibility
💡 Result:
On macOS’s default (BSD/Apple) grep, -E enables POSIX ERE, but grep also enables Apple’s “enhanced” regex features from re_format(7), which include the shortcuts and assertions you’re asking about:
\s/\Sare supported as enhanced shortcuts (\s≡[[:space:]]). [1]\b/\Bare supported as enhanced word-boundary assertions (and\</\>are available too, with[[:<:]]/[[:>:]]equivalents). [1]- This is not POSIX ERE portability: many greps treat
\sand\bas non-special (Autoconf explicitly warns they’re nonportable). [2]
If you need portable ERE, use:
- whitespace:
[[:space:]] - word boundaries (BSD-style):
[[:<:]]word[[:>:]](or usegrep -wfor whole words when that’s sufficient). [1]
Also note: other macOS tools (notably sed, find -E -regex) may not be compiled with the enhanced flag even though re_format(7) documents it, so \s/\b can fail there—grep is the common one that does work. [3]
Sources:
[1] macOS re_format(7) (“ENHANCED FEATURES”: \s, \b, \</\> and their [[:...:]] equivalents) https://www.unix.com/man-page/osx/7/re_format
[2] GNU Autoconf manual: portability warning to avoid \b, \s, etc. https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Limitations-of-Usual-Tools.html
[3] Discussion of macOS enhanced vs extended support and that grep supports enhanced features while some other tools don’t https://stackoverflow.com/questions/12178924/os-x-sed-e-doesnt-accept-extended-regular-expressions
🏁 Script executed:
# Check if ShellCheck is available and run it on the file
which shellcheck || echo "ShellCheck not available"
if command -v shellcheck &> /dev/null; then
shellcheck ".agents/scripts/pre-commit-hook.sh" || true
fiRepository: marcusquinn/aidevops
Length of output: 149
🏁 Script executed:
# Also check the specific lines for context and verify the duplication more clearly
sed -n '76,90p' ".agents/scripts/pre-commit-hook.sh" | cat -nRepository: marcusquinn/aidevops
Length of output: 1168
Refactor the grep pipeline to eliminate duplication and improve readability, but note that \s and \b work on macOS.
While \s and \b are supported by macOS BSD grep as enhanced regex features (per re_format(7)), the grep pipeline duplicated on lines 83 and 85 violates the DRY principle and creates a maintenance liability. If either line changes, the other must be manually updated or behavior diverges silently.
Extract the grep result into a variable to eliminate duplication:
local matches
matches=$(grep -n '\$[1-9]' "$file" \
| grep -v 'local.*=.*\$[1-9]' \
| grep -vE '\$[1-9][0-9.,/]' \
| grep -vE '\$[1-9]\s*\|' \
| grep -vE '\$[1-9]\s+(per|mo(nth)?|year|yr|day|week|hr|hour|flat|each|off|fee|plan|tier|user|seat|unit|addon|setup|trial|credit|annual|quarterly|monthly)\b' \
|| true)
if [[ -f "$file" ]] && [[ -n "$matches" ]]; then
print_error "Direct positional parameter usage in $file"
echo "$matches" | head -3
((violations++))
fiThis also improves readability by breaking the long pipeline into logical sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/pre-commit-hook.sh around lines 83 - 85, Replace the
duplicated grep pipeline by capturing its output into a variable (e.g., matches)
and reusing that variable for the existence check and printing; specifically,
run the pipeline that starts with grep -n '\$[1-9]' "$file" and the subsequent
grep -v/grep -vE filters once, assign its (possibly empty) result to matches
(use "|| true" to avoid non-zero exit), then change the if condition to check [[
-f "$file" ]] && [[ -n "$matches" ]], call print_error "Direct positional
parameter usage in $file", echo "$matches" | head -3, and increment
((violations++)); update references to the original duplicated pipeline
locations to use matches instead.
1 participant
Summary
shell-local-paramsTTSR rule regex to exclude currency/pricing patterns that were causing false-positivesindex.mjs),linters-local.sh, andpre-commit-hook.shProblem
The regex
$[1-9]in the shell-local-params rule was matching dollar amounts like$5 per month,$9 mo,$3 monthlyin markdown tables and pricing documentation. The existing negative lookahead(?![0-9.,/])only excluded multi-digit amounts ($28) and amounts with decimals/commas/slashes ($1.99,$1,000,$5/mo), but not standalone single-digit amounts followed by currency unit words.Testing
Verified with 28 test cases covering:
$1-$9in shell commands (still correctly detected)$5 per month,$9 mo,$3 monthly,$7 annual,$5 flat,$1 trial, pipe-delimited table cells, etc.All 28 tests pass with zero failures. ShellCheck and Node syntax checks pass clean.
Closes #2194
Summary by CodeRabbit
Release Notes