Skip to content

Comments

t318.5: End-to-end PR task ID validation testing#1260

Merged
marcusquinn merged 1 commit intomainfrom
feature/t318.5
Feb 12, 2026
Merged

t318.5: End-to-end PR task ID validation testing#1260
marcusquinn merged 1 commit intomainfrom
feature/t318.5

Conversation

@marcusquinn
Copy link
Owner

@marcusquinn marcusquinn commented Feb 12, 2026

Summary

End-to-end testing of the PR task ID validation system (t318). Also implements the CI check (t318.1) since it had no existing PR or branch.

Ref #1239

What is included

1. Local test harness (test-pr-task-check.sh)

23 test cases validating all CI check logic:

Test Group Cases Result
PRs without task ID 3 All correctly rejected
PRs with valid task ID 4 All correctly accepted
Non-existent task IDs 3 All correctly rejected
Exempted branches 5 All correctly exempted
Edge cases (boundary, declined, subtasks) 6 All pass
Supervisor PR patterns 2 All correctly accepted
Total 23 23/23 pass

2. CI workflow patch (pr-task-check-ci.patch)

Adds pr-task-check job to code-quality.yml that:

  • Extracts tNNN task ID from PR title or branch name
  • Verifies task exists in TODO.md on the base branch
  • Fails with clear error message + PR comment if missing
  • Exempts dependabot/, auto-fix/, release/, hotfix/-emergency-* branches
  • Rejects declined [-] tasks

Apply with: git apply .agents/scripts/pr-task-check-ci.patch

3. Real PR audit

Audited 30 most recent PRs:

  • 28/30 have task IDs (100% of post-t318 PRs)
  • 2/30 pre-t318 merged PRs lack task IDs (would be correctly blocked today)
  • All supervisor-created PRs follow tNNN: Description pattern

Blocker: workflow scope

The GH_TOKEN (OAuth) lacks workflow scope, which GitHub requires to push changes to .github/workflows/. The CI workflow change is provided as a patch file.

Test evidence

All 23 local tests pass. See test-pr-task-check.sh for full test suite.

Summary by CodeRabbit

  • Chores
    • Implemented automated pull request validation workflow to ensure proper task tracking compliance
    • Added comprehensive test harness to verify PR validation logic across various scenarios

@marcusquinn marcusquinn marked this pull request as ready for review February 12, 2026 14:11
@gemini-code-assist
Copy link

Summary of Changes

Hello @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 introduces a robust system for validating PR task IDs, ensuring that all pull requests are linked to existing and active tasks defined in TODO.md. It includes a new CI check to enforce this policy and a comprehensive local test suite to verify the validation logic across various scenarios, thereby improving traceability and planning adherence within the development workflow.

Highlights

  • New CI Check Implementation: A new GitHub Actions job, pr-task-check, has been introduced to automatically validate PR task IDs. This job extracts task IDs from PR titles or branch names and verifies their existence and status in TODO.md on the base branch.
  • Comprehensive Local Test Harness: A local shell script, test-pr-task-check.sh, was added, containing 23 test cases to thoroughly validate the PR task ID check logic. These tests cover scenarios such as PRs without task IDs, valid task IDs, non-existent task IDs, exempted branches, and various edge cases.
  • Exemption Logic: The CI check includes exemption patterns for specific branch types (e.g., dependabot/, auto-fix/, release/, hotfix/*-emergency-*), allowing certain automated or critical PRs to bypass the task ID validation.
  • Declined Task Handling: The validation now explicitly rejects PRs associated with tasks marked as declined ([-]) in TODO.md, preventing work on deprecated or cancelled items.
  • Automated Feedback: Failed PR task ID checks will automatically post clear error messages and guidance as comments on the pull request, helping authors quickly understand and resolve issues.
  • Workflow Deployment Strategy: Due to a GH_TOKEN lacking workflow scope, the CI workflow change is provided as a patch file (pr-task-check-ci.patch) to be applied manually, rather than directly modifying .github/workflows/code-quality.yml.
Changelog
  • .agents/scripts/test-pr-task-check.sh
    • Introduced a comprehensive local test suite for the PR task ID validation logic.
    • Included 23 test cases covering various scenarios like valid/invalid IDs, exempted branches, and edge cases.
  • .github/workflows/code-quality.yml
    • Added a new pr-task-check job to validate PR task IDs.
    • Implemented logic to extract task IDs from PR titles or branch names.
    • Configured checks against TODO.md for task existence and status (e.g., declined).
    • Defined exemption patterns for specific branch types (e.g., dependabot/, release/).
    • Added automated PR comments for failed checks.
Activity
  • All 23 local test cases for the PR task ID validation logic passed successfully.
  • An audit of the 30 most recent PRs was conducted, revealing that 28/30 had task IDs, with the two missing IDs being pre-t318 PRs that would now be correctly blocked.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 15 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Thu Feb 12 14:11:32 UTC 2026: Code review monitoring started
Thu Feb 12 14:11:32 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 15

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 15
  • VULNERABILITIES: 0

Generated on: Thu Feb 12 14:11:35 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

Walkthrough

This PR adds a new GitHub Actions workflow job "PR Task ID Check" and corresponding Bash test harness to validate that PR titles and branches contain task IDs (tNNN or tNNN.N format) that exist in TODO.md on the base branch. Includes exemption logic for dependabot, auto-fix, release, and emergency hotfix branches.

Changes

Cohort / File(s) Summary
Workflow Implementation
.github/workflows/code-quality.yml
Introduces new "PR Task ID Check" job with sparse checkout for TODO.md, task ID extraction from PR title/branch, branch exemption patterns, task existence validation, and detailed PR comment posting on validation failures.
Test Harness
.agents/scripts/test-pr-task-check.sh
Comprehensive Bash test script with 319 lines covering exemption patterns, task ID extraction logic, multiple result states (pass, fail-no-id, fail-not-found, fail-declined, exempt), edge-case boundary testing, and colored output reporting.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub
    participant WF as PR Task Check Job
    participant Git as Git
    participant TODO as TODO.md
    participant Comment as PR Comment
    participant Exit as Exit Code

    GH->>WF: Pull request event triggered
    WF->>WF: Extract task ID from PR title/branch
    WF->>WF: Check branch exemption patterns<br/>(dependabot, auto-fix, release, hotfix)
    
    alt Exempt Branch
        WF->>Exit: Exit 0 (success)
    else Non-Exempt
        WF->>Git: Sparse checkout base branch TODO.md
        Git->>TODO: Retrieve TODO.md file
        
        alt Task ID Found
            WF->>TODO: Search for task ID
            
            alt Task Exists & Not Declined
                WF->>Exit: Exit 0 (success)
            else Task Missing or Declined
                WF->>Comment: Post detailed fix instructions
                WF->>Exit: Exit 1 (failure)
            end
        else No Task ID Found
            WF->>Comment: Post task ID requirement comment
            WF->>Exit: Exit 1 (failure)
        end
        
        alt TODO.md Missing
            WF->>WF: Log warning and skip check
            WF->>Exit: Exit 0 (success)
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Poem

🚀 A task-ID guardian now watches the gate,
Validating each PR before it's too late,
With TODO.md as truth and branches exempt,
The workflow ensures no rogue commits are sent,
While bash tests keep the logic pristine and tight—
Zero debt maintained, DevOps done right.

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (13 files):

⚔️ .agents/scripts/coderabbit-pulse-helper.sh (content)
⚔️ .agents/scripts/content-fanout-helper.sh (content)
⚔️ .agents/scripts/document-extraction-helper.sh (content)
⚔️ .agents/scripts/install-hooks.sh (content)
⚔️ .agents/scripts/issue-sync-helper.sh (content)
⚔️ .agents/scripts/objective-runner-helper.sh (content)
⚔️ .agents/scripts/pipecat-helper.sh (content)
⚔️ .agents/scripts/real-video-enhancer-helper.sh (content)
⚔️ .agents/scripts/rosetta-audit-helper.sh (content)
⚔️ .agents/scripts/test-ocr-extraction-pipeline.sh (content)
⚔️ TODO.md (content)
⚔️ sonar-project.properties (content)
⚔️ todo/PLANS.md (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 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 change: introducing end-to-end testing for PR task ID validation, which aligns with the primary additions of test-pr-task-check.sh and the accompanying CI workflow patch.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/t318.5
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch feature/t318.5
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs 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

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

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 15 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Thu Feb 12 14:15:12 UTC 2026: Code review monitoring started
Thu Feb 12 14:15:12 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 15

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 15
  • VULNERABILITIES: 0

Generated on: Thu Feb 12 14:15:14 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a valuable CI check for validating task IDs in PRs, along with a comprehensive test harness. The implementation is solid, and the test script covers an impressive range of scenarios and edge cases. My feedback focuses on aligning the new shell scripts with the repository's style guide to enhance robustness and maintainability. I've suggested adding set -euo pipefail to the CI script, and in the test script, ensuring functions have explicit return statements and temporary files are cleaned up using trap.

+ PR_NUMBER: ${{ github.event.pull_request.number }}
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+ echo "PR Task ID Validation"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The script within the run block should start with set -euo pipefail to ensure robustness. The repository style guide (line 10) requires all scripts to use it. While GitHub Actions enables -e by default, -u (for unbound variables) and -o pipefail are crucial for preventing subtle bugs.

        set -euo pipefail
        echo "PR Task ID Validation"
References
  1. All scripts should use set -euo pipefail for strict error checking. (link)

Comment on lines 82 to 83
fi
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The repository style guide (line 12) requires all functions to have an explicit return statement. The check_pr_task_id function is missing one.

Suggested change
fi
}
fi
return 0
}
References
  1. All functions must have an explicit return statement. (link)

Comment on lines 215 to 222
TEMP_TODO=$(mktemp)
echo "- [ ] t318.5 Test task" >"$TEMP_TODO"
check_pr_task_id \
"t318: Parent task" \
"feature/t318" \
"$TEMP_TODO" \
"fail-not-found"
rm -f "$TEMP_TODO"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block creates a temporary file without using a trap for cleanup, which violates the repository style guide (line 33). If the script were to exit unexpectedly between lines 215 and 222, the temporary file would be left behind.

This pattern is repeated multiple times in the script. A better approach would be to create a helper function that encapsulates the creation of the temp file, setting a trap, running the test, and cleaning up. This would make the code safer and more readable.

References
  1. Temporary files created with mktemp must have a trap for cleanup to ensure they are removed on script exit or return. (link)

Implements end-to-end testing for the PR task ID enforcement system (t318).

1. test-pr-task-check.sh: 25 test cases validating all CI check logic:
   - PRs without task ID -> rejected (3 cases)
   - PRs with valid task ID -> accepted (4 cases)
   - Non-existent task IDs -> rejected (3 cases)
   - Exempted branches: dependabot, auto-fix, release, hotfix-emergency (5 cases)
   - Edge cases: boundary matching, declined tasks, subtasks (6 cases)
   - Supervisor PR patterns -> accepted (2 cases)
   - Real-world PR patterns -> accepted (2 cases)
   All 25 tests pass.

2. pr-task-check-ci.patch: GitHub Actions workflow change for code-quality.yml
   Adds pr-task-check job that validates every PR has a task ID (tNNN) in its
   title or branch name, and that the task exists in TODO.md on the base branch.
   Exemptions: dependabot/*, auto-fix/*, release/*, hotfix/*-emergency-*.
   Posts PR comment with fix instructions on failure.

   Apply with: git apply .agents/scripts/pr-task-check-ci.patch
   Requires token with workflow scope to push .github/workflows/ changes.

BLOCKER: GH_TOKEN (OAuth) lacks workflow scope — cannot push .github/workflows/
changes. User must apply patch and push with a PAT that has workflow scope.
@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 0 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Thu Feb 12 20:13:42 UTC 2026: Code review monitoring started
Thu Feb 12 20:13:43 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 0

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 0
  • VULNERABILITIES: 0

Generated on: Thu Feb 12 20:13:45 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @.agents/scripts/pr-task-check-ci.patch:
- Around line 78-92: The multi-line string passed to gh pr comment "$PR_NUMBER"
--body is indented so GitHub will render it as a code block; fix both gh pr
comment invocations by dedenting the message text or switching to a heredoc so
the body has no leading spaces—for example, replace the inline quoted, indented
literal with a construct that injects an unindented multi-line string (e.g., gh
pr comment "$PR_NUMBER" --body "$(cat <<'EOF'\n...your unindented
message...\nEOF\n)") ensuring the same change is applied to the second gh pr
comment block as well.

In @.agents/scripts/test-pr-task-check.sh:
- Around line 27-83: The function check_pr_task_id currently exits by falling
through; add an explicit return code at the end of the function (e.g., return 0)
to satisfy the shell scripting guideline requiring explicit returns. Locate the
closing brace of check_pr_task_id and append a single return 0 just before it so
the function always returns an explicit success status when it finishes.
- Around line 22-23: The verbose helper uses the unsafe `[[ "$VERBOSE" ==
"--verbose" ]] && echo -e ... || true` pattern flagged as SC2015; refactor the
`verbose()` function into an explicit if/then/else that calls `echo -e " 
${YELLOW}$1${NC}"` when VERBOSE equals "--verbose" and otherwise returns
success, and add explicit return statements to both `log()` and `verbose()` so
failures from `echo -e` are not silently swallowed; update the `log()` function
to also include an explicit return after the `echo -e "$1"` call.
🧹 Nitpick comments (4)
.agents/scripts/test-pr-task-check.sh (3)

22-22: log() function also lacks explicit return.

Same guideline applies here. Minor, but for zero-debt consistency:

🔧 Proposed fix
-log() { echo -e "$1"; }
+log() { echo -e "$1"; return 0; }

As per coding guidelines, "Include explicit returns in shell scripts."


214-271: Temp files leak if set -e triggers an early exit between mktemp and rm.

If check_pr_task_id (or any command in-between) fails unexpectedly under set -e, the temp file won't be cleaned up. Consider using a trap for cleanup or grouping each temp-file test in a subshell.

🔧 Proposed fix — trap-based cleanup

Add near the top of the script (after set -euo pipefail):

+TEMP_FILES=()
+cleanup_temp() {
+    for f in "${TEMP_FILES[@]}"; do
+        rm -f "$f"
+    done
+}
+trap cleanup_temp EXIT

Then replace each mktemp / rm pair like:

 TEMP_TODO=$(mktemp)
+TEMP_FILES+=("$TEMP_TODO")
 echo "- [ ] t318.5 Test task" >"$TEMP_TODO"
 check_pr_task_id \
     "t318: Parent task" \
     "feature/t318" \
     "$TEMP_TODO" \
     "fail-not-found"
-rm -f "$TEMP_TODO"

The EXIT trap handles cleanup regardless of how the script terminates.


67-73: Dots in task_id are unescaped in grep regex — minor false-match risk.

task_id values like t318.5 are interpolated directly into the grep ERE pattern. The . matches any character, so t318.5 would also match a hypothetical t31825. Given the controlled TODO.md format and trailing-space anchor, false positives are extremely unlikely, but for correctness purity you could escape dots.

🔧 Proposed fix
+    # Escape dots in task_id for grep regex
+    local escaped_id="${task_id//./\\.}"
+
-    elif grep -qE "^[[:space:]]*- \[-\] ${task_id} " "$todo_file"; then
+    elif grep -qE "^[[:space:]]*- \[-\] ${escaped_id} " "$todo_file"; then
         result="fail-declined"
-    elif grep -qE "^[[:space:]]*- \[[ x]\] ${task_id} " "$todo_file"; then
+    elif grep -qE "^[[:space:]]*- \[[ x]\] ${escaped_id} " "$todo_file"; then
         result="pass"
.agents/scripts/pr-task-check-ci.patch (1)

103-113: Same unescaped-dot-in-grep note applies here too.

${task_id} containing dots (e.g., t318.5) is used unescaped in the ERE pattern, identical to the test harness. Low risk given the controlled format, but worth noting for consistency if you fix it in the test script.

Comment on lines +78 to +92
+ gh pr comment "$PR_NUMBER" --body "## PR Task ID Check Failed
+
+ This PR does not reference a task ID (\`tNNN\`) in its title or branch name.
+
+ Every PR must be traceable to a planned task in TODO.md. This ensures:
+ - All work is planned before implementation
+ - Every PR can be traced back to its task
+ - Every task can be traced forward to its PR
+
+ **How to fix:**
+ 1. If a task exists, add its ID to the PR title (e.g., \`t001: Fix the thing\`)
+ 2. If no task exists, create one in TODO.md first
+ 3. Then update the PR title to include the task ID
+
+ **Exempted branches:** \`dependabot/*\`, \`auto-fix/*\`, \`release/*\`, \`hotfix/*-emergency-*\`" || echo "Warning: Could not post PR comment"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

PR comment body will render with unwanted leading whitespace in GitHub Markdown.

The body text on lines 80–92 is indented to match the YAML nesting, but gh pr comment --body will preserve that whitespace verbatim. GitHub Markdown will render the leading spaces as a code block or preformatted text, making the comment look broken.

Dedent the comment body so lines start at column 1 (or use a heredoc approach). The same issue applies to the second gh pr comment block on lines 129–137.

🔧 Proposed fix (first comment block)
           # Post comment on PR with fix instructions
-          gh pr comment "$PR_NUMBER" --body "## PR Task ID Check Failed
-
-        This PR does not reference a task ID (\`tNNN\`) in its title or branch name.
-
-        Every PR must be traceable to a planned task in TODO.md. This ensures:
-        - All work is planned before implementation
-        - Every PR can be traced back to its task
-        - Every task can be traced forward to its PR
-
-        **How to fix:**
-        1. If a task exists, add its ID to the PR title (e.g., \`t001: Fix the thing\`)
-        2. If no task exists, create one in TODO.md first
-        3. Then update the PR title to include the task ID
-
-        **Exempted branches:** \`dependabot/*\`, \`auto-fix/*\`, \`release/*\`, \`hotfix/*-emergency-*\`" || echo "Warning: Could not post PR comment"
+          gh pr comment "$PR_NUMBER" --body "## PR Task ID Check Failed
+
+This PR does not reference a task ID (\`tNNN\`) in its title or branch name.
+
+Every PR must be traceable to a planned task in TODO.md. This ensures:
+- All work is planned before implementation
+- Every PR can be traced back to its task
+- Every task can be traced forward to its PR
+
+**How to fix:**
+1. If a task exists, add its ID to the PR title (e.g., \`t001: Fix the thing\`)
+2. If no task exists, create one in TODO.md first
+3. Then update the PR title to include the task ID
+
+**Exempted branches:** \`dependabot/*\`, \`auto-fix/*\`, \`release/*\`, \`hotfix/*-emergency-*\`" || echo "Warning: Could not post PR comment"
🤖 Prompt for AI Agents
In @.agents/scripts/pr-task-check-ci.patch around lines 78 - 92, The multi-line
string passed to gh pr comment "$PR_NUMBER" --body is indented so GitHub will
render it as a code block; fix both gh pr comment invocations by dedenting the
message text or switching to a heredoc so the body has no leading spaces—for
example, replace the inline quoted, indented literal with a construct that
injects an unindented multi-line string (e.g., gh pr comment "$PR_NUMBER" --body
"$(cat <<'EOF'\n...your unindented message...\nEOF\n)") ensuring the same change
is applied to the second gh pr comment block as well.

Comment on lines +22 to +23
log() { echo -e "$1"; }
verbose() { [[ "$VERBOSE" == "--verbose" ]] && echo -e " ${YELLOW}$1${NC}" || true; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

SC2015: A && B || C is not a safe if-then-else substitute.

The || true guard helps, but if echo -e fails (e.g., write error to a closed fd), the || true silently swallows it. ShellCheck flags this as SC2015. Since the coding guidelines mandate zero ShellCheck violations, refactor to a proper conditional.

🔧 Proposed fix
-verbose() { [[ "$VERBOSE" == "--verbose" ]] && echo -e "  ${YELLOW}$1${NC}" || true; }
+verbose() {
+    if [[ "$VERBOSE" == "--verbose" ]]; then
+        echo -e "  ${YELLOW}$1${NC}"
+    fi
+    return 0
+}

As per coding guidelines, "Run ShellCheck with zero violations on all scripts in .agents/scripts/" and "Include explicit returns in shell scripts."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
log() { echo -e "$1"; }
verbose() { [[ "$VERBOSE" == "--verbose" ]] && echo -e " ${YELLOW}$1${NC}" || true; }
log() { echo -e "$1"; }
verbose() {
if [[ "$VERBOSE" == "--verbose" ]]; then
echo -e " ${YELLOW}$1${NC}"
fi
return 0
}
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis

[warning] 23-23: .agents/scripts/test-pr-task-check.sh#L23
Note that A && B || C is not if-then-else. C may run when A is true.

🤖 Prompt for AI Agents
In @.agents/scripts/test-pr-task-check.sh around lines 22 - 23, The verbose
helper uses the unsafe `[[ "$VERBOSE" == "--verbose" ]] && echo -e ... || true`
pattern flagged as SC2015; refactor the `verbose()` function into an explicit
if/then/else that calls `echo -e "  ${YELLOW}$1${NC}"` when VERBOSE equals
"--verbose" and otherwise returns success, and add explicit return statements to
both `log()` and `verbose()` so failures from `echo -e` are not silently
swallowed; update the `log()` function to also include an explicit return after
the `echo -e "$1"` call.

Comment on lines +27 to +83
check_pr_task_id() {
local pr_title="$1"
local pr_branch="$2"
local todo_file="$3"
local expected="$4" # "pass", "fail-no-id", "fail-not-found", "fail-declined", "exempt"

TOTAL=$((TOTAL + 1))
local result=""
local task_id=""

# Exempted branch patterns (must match code-quality.yml exactly)
local exempt_patterns=(
"^dependabot/"
"^auto-fix/"
"^release/"
"^hotfix/.*-emergency-"
)

for pattern in "${exempt_patterns[@]}"; do
if [[ "$pr_branch" =~ $pattern ]]; then
result="exempt"
verbose "Branch '${pr_branch}' matches exemption '${pattern}'"
break
fi
done

if [[ -z "$result" ]]; then
# Extract task ID from PR title first, then branch name
if [[ "$pr_title" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in PR title"
elif [[ "$pr_branch" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in branch name"
fi

if [[ -z "$task_id" ]]; then
result="fail-no-id"
elif [[ ! -f "$todo_file" ]]; then
result="pass" # No TODO.md = skip check (graceful)
elif grep -qE "^[[:space:]]*- \[-\] ${task_id} " "$todo_file"; then
result="fail-declined"
elif grep -qE "^[[:space:]]*- \[[ x]\] ${task_id} " "$todo_file"; then
result="pass"
else
result="fail-not-found"
fi
fi

if [[ "$result" == "$expected" ]]; then
PASS=$((PASS + 1))
log "${GREEN}PASS${NC} [$result] title='$pr_title' branch='$pr_branch'"
else
FAIL=$((FAIL + 1))
log "${RED}FAIL${NC} expected=$expected got=$result title='$pr_title' branch='$pr_branch'"
fi
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Missing explicit return statements in check_pr_task_id.

The function falls through without an explicit return. The coding guidelines require explicit returns in shell scripts. Add return 0 at the end of the function.

🔧 Proposed fix
     else
         FAIL=$((FAIL + 1))
         log "${RED}FAIL${NC} expected=$expected got=$result title='$pr_title' branch='$pr_branch'"
     fi
+    return 0
 }

As per coding guidelines, "Include explicit returns in shell scripts."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
check_pr_task_id() {
local pr_title="$1"
local pr_branch="$2"
local todo_file="$3"
local expected="$4" # "pass", "fail-no-id", "fail-not-found", "fail-declined", "exempt"
TOTAL=$((TOTAL + 1))
local result=""
local task_id=""
# Exempted branch patterns (must match code-quality.yml exactly)
local exempt_patterns=(
"^dependabot/"
"^auto-fix/"
"^release/"
"^hotfix/.*-emergency-"
)
for pattern in "${exempt_patterns[@]}"; do
if [[ "$pr_branch" =~ $pattern ]]; then
result="exempt"
verbose "Branch '${pr_branch}' matches exemption '${pattern}'"
break
fi
done
if [[ -z "$result" ]]; then
# Extract task ID from PR title first, then branch name
if [[ "$pr_title" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in PR title"
elif [[ "$pr_branch" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in branch name"
fi
if [[ -z "$task_id" ]]; then
result="fail-no-id"
elif [[ ! -f "$todo_file" ]]; then
result="pass" # No TODO.md = skip check (graceful)
elif grep -qE "^[[:space:]]*- \[-\] ${task_id} " "$todo_file"; then
result="fail-declined"
elif grep -qE "^[[:space:]]*- \[[ x]\] ${task_id} " "$todo_file"; then
result="pass"
else
result="fail-not-found"
fi
fi
if [[ "$result" == "$expected" ]]; then
PASS=$((PASS + 1))
log "${GREEN}PASS${NC} [$result] title='$pr_title' branch='$pr_branch'"
else
FAIL=$((FAIL + 1))
log "${RED}FAIL${NC} expected=$expected got=$result title='$pr_title' branch='$pr_branch'"
fi
}
check_pr_task_id() {
local pr_title="$1"
local pr_branch="$2"
local todo_file="$3"
local expected="$4" # "pass", "fail-no-id", "fail-not-found", "fail-declined", "exempt"
TOTAL=$((TOTAL + 1))
local result=""
local task_id=""
# Exempted branch patterns (must match code-quality.yml exactly)
local exempt_patterns=(
"^dependabot/"
"^auto-fix/"
"^release/"
"^hotfix/.*-emergency-"
)
for pattern in "${exempt_patterns[@]}"; do
if [[ "$pr_branch" =~ $pattern ]]; then
result="exempt"
verbose "Branch '${pr_branch}' matches exemption '${pattern}'"
break
fi
done
if [[ -z "$result" ]]; then
# Extract task ID from PR title first, then branch name
if [[ "$pr_title" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in PR title"
elif [[ "$pr_branch" =~ (t[0-9]+(\.[0-9]+)*) ]]; then
task_id="${BASH_REMATCH[1]}"
verbose "Found task ID '${task_id}' in branch name"
fi
if [[ -z "$task_id" ]]; then
result="fail-no-id"
elif [[ ! -f "$todo_file" ]]; then
result="pass" # No TODO.md = skip check (graceful)
elif grep -qE "^[[:space:]]*- \[-\] ${task_id} " "$todo_file"; then
result="fail-declined"
elif grep -qE "^[[:space:]]*- \[[ x]\] ${task_id} " "$todo_file"; then
result="pass"
else
result="fail-not-found"
fi
fi
if [[ "$result" == "$expected" ]]; then
PASS=$((PASS + 1))
log "${GREEN}PASS${NC} [$result] title='$pr_title' branch='$pr_branch'"
else
FAIL=$((FAIL + 1))
log "${RED}FAIL${NC} expected=$expected got=$result title='$pr_title' branch='$pr_branch'"
fi
return 0
}
🤖 Prompt for AI Agents
In @.agents/scripts/test-pr-task-check.sh around lines 27 - 83, The function
check_pr_task_id currently exits by falling through; add an explicit return code
at the end of the function (e.g., return 0) to satisfy the shell scripting
guideline requiring explicit returns. Locate the closing brace of
check_pr_task_id and append a single return 0 just before it so the function
always returns an explicit success status when it finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants