.github/workflows/test-failure-detector-sweep.yml (5)

39-39: 💤 Low value

Consider pinning checkout and setup-python actions to commit SHAs.

Static analysis flags the use of tag-based action references (@v6) instead of commit SHAs. While you've already pinned create-github-app-token to a SHA (line 55), the checkout and setup-python actions remain unpinned.

Pinning to commit SHAs provides supply-chain attack protection but requires more maintenance. This is a repository-wide policy decision.

Also applies to: 45-45

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml at line 39, Replace
tag-based GitHub Action references with pinned commit SHAs for supply-chain
safety: update the uses entries that reference "actions/checkout@v6" and
"actions/setup-python@v4" to their corresponding full commit SHA values
(matching the versions you intend to use), ensuring the workflow still functions
and updating any dependent inputs if necessary; verify the pinned SHAs are from
upstream actions/checkout and actions/setup-python repositories and run the
workflow to confirm no regressions.

---

24-36: ⚡ Quick win

Clarify permissions intent or remove unused grant.

The workflow-level issues: write (line 26) is overridden by the job-level permissions block (lines 35-36), which grants only contents: read. Since the detector uses the GitHub App token (TARGET_TOKEN) for issue operations rather than the default GITHUB_TOKEN, the workflow-level issues: write grant is unused.

Either remove issues: write from lines 24-26, or add a comment explaining that the App token (not GITHUB_TOKEN) handles issue operations.

📝 Option 1: Remove unused permission

 permissions: 
   contents: read
-  issues: write

📝 Option 2: Document the intent

 permissions: 
   contents: read
-  issues: write
+  issues: write  # Unused by workflow; detector uses GitHub App token for cross-repo issue ops

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml around lines 24 - 36, The
workflow defines a workflow-level permissions block granting "issues: write" but
the job "detect-failures" overrides permissions to only "contents: read" and the
detector actually uses the GitHub App token (TARGET_TOKEN) for issue operations,
so remove the unused workflow-level "issues: write" entry or add a clear comment
above the workflow-level permissions explaining that issue writes are handled by
the App token (TARGET_TOKEN) and not by GITHUB_TOKEN; update the "permissions"
block or add the comment near the top-level permissions and keep the job-level
permissions in the "detect-failures" job as-is to avoid confusion.

---

66-66: 💤 Low value

Consider simplifying the DRY_RUN expression for clarity.

The current expression ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run == true && 'true' || 'false' }} is correct but hard to parse. The logic ensures dry-run mode is only honored for manual dispatch (scheduled runs always create issues).

Consider splitting the logic or adding a comment to document the intent.

♻️ Alternative: add explanatory comment

+          # dry_run is only honored for manual dispatch; scheduled runs always create issues
           DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run == true && 'true' || 'false' }}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml at line 66, The DRY_RUN
expression is hard to read; replace the current expression with a ternary for
clarity and add a short comment: set DRY_RUN to ${{ github.event_name ==
'workflow_dispatch' && inputs.dry_run == 'true' ? 'true' : 'false' }} (and add a
one-line comment above explaining "only honor inputs.dry_run for manual
workflow_dispatch runs; scheduled runs always create issues"). Reference the
DRY_RUN env variable and the existing github.event_name and inputs.dry_run
symbols when making the change.

---

13-16: ⚡ Quick win

Use type: number for the run_id input.

The run_id input represents a workflow run ID, which is numeric. GitHub Actions supports type: number for workflow_dispatch inputs, which better documents the expected value type than type: string.

♻️ Proposed fix

       run_id:
         description: "Specific workflow run ID to analyze (empty = latest completed Daily run)"
         required: false
-        type: string
-        default: ""
+        type: number

Note: With type: number, omitting the input will pass null instead of "", so remove the empty string check on line 76.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml around lines 13 - 16,
Change the workflow_dispatch input "run_id" to use type: number instead of type:
string by updating the run_id input definition (replace type: string with type:
number); then remove or adjust the code that checks for an empty string value
for run_id (the check that compares run_id == ""), since with type: number an
omitted input will be null rather than ""; ensure any subsequent logic expects a
number or null and handle null appropriately.

---

29-34: 💤 Low value

Consider adding a job timeout for defense in depth.

The job currently has no timeout-minutes set, relying on the GitHub Actions default (360 minutes). While this workflow is expected to complete in minutes (artifact download + parsing + API calls), adding a modest timeout (e.g., 30-60 minutes) would catch hangs or API degradation scenarios.

🛡️ Proposed addition

   detect-failures:
     name: "Detect test failures"
+    timeout-minutes: 60
     if: >

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml around lines 29 - 34, Add
a defensive job timeout to the detect-failures job by setting the
timeout-minutes property (e.g., 30 or 60) under the detect-failures job
definition so the workflow won't hang indefinitely; update the job block that
defines name: "Detect test failures" / runs-on: ubuntu-latest to include
timeout-minutes: 30 (or 60) directly beneath runs-on.

tests/test_issue_manager.py (1)

109-115: ⚡ Quick win

Add a regression case for bodies without an Environments line.

Current coverage only proves replacement works when the marker is already present. Please add a case where an existing issue body has no **Environments:** section, then verify process_failures() appends the new environments instead of silently leaving the body unchanged.

Also applies to: 121-220

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_issue_manager.py` around lines 109 - 115, Add a regression test
for when the issue body lacks an existing "**Environments:**" line: create a new
test (e.g., in class TestUpdateEnvironments) that calls
_update_environments_in_body with a body like "Some text\nMore text" and
environments ["job-a", "job-b"] and assert the returned body now contains a new
"**Environments:** `job-a`, `job-b`" line while preserving the original text;
also add a parallel case calling process_failures() (or the higher-level wrapper
used in tests) to ensure it appends the environments instead of leaving the body
unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

.github/workflows/test-failure-detector-sweep.yml (1)

28-37: ⚡ Quick win

Consider adding a timeout to prevent runaway jobs.

The job inherits the default GitHub Actions timeout of 360 minutes, but the GitHub App token expires after 60 minutes. Adding an explicit timeout-minutes: 60 (or 30) would align the job lifecycle with the token lifetime and prevent unexpected long-running scenarios.

⏱️ Suggested addition

 jobs:
   detect-failures:
     name: "Detect test failures"
+    timeout-minutes: 60
     if: >
       (github.event_name == 'workflow_dispatch' ||
       github.repository == 'valkey-io/valkey-ci-agent')

Based on learnings: In the valkey-io/valkey-ci-agent repository, the fuzzer monitor workflow intentionally sets timeout-minutes: 180 as a worst-case safety net, with real runs expected to complete in 10–20 minutes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test-failure-detector-sweep.yml around lines 28 - 37, The
detect-failures job currently has no explicit timeout and can run up to the
GitHub default (360 minutes); add a timeout-minutes field to the job definition
(the jobs.detect-failures block) to align with the GitHub App token lifetime —
e.g., add timeout-minutes: 60 (or 30) directly under the detect-failures job
metadata (near name/runs-on/permissions) so the job will be forcibly cancelled
if it exceeds that limit.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes