Skip to content

fix: PR #55/#58 review — mutation-testing skill review rounds 2-3 (resurrected from closed #58) - #60

Merged
Bonobo791 merged 2 commits into
mainfrom
fix-mutation-skill-review
Aug 3, 2026
Merged

fix: PR #55/#58 review — mutation-testing skill review rounds 2-3 (resurrected from closed #58)#60
Bonobo791 merged 2 commits into
mainfrom
fix-mutation-skill-review

Conversation

@Bonobo791

@Bonobo791 Bonobo791 commented Aug 3, 2026

Copy link
Copy Markdown
Owner

User description

Why

PR #58 was closed unmerged during branch cleanup; its review findings were still valid against main. This PR resurrects them on a fresh branch from current origin/main:

Verification

  • Hook behavior suite (echo-based): plural "weak assertions" fires; "are my tests passing?" stays silent; "are my tests actually good" fires; unreadable skill file now emits the error detail on BOTH stdout WARNING and stderr.
  • codacy-analysis analyze --files on all 6 changed files — 0 issues.
  • npm run test — 371 passed. npm run check — 0/0. npm run build — clean.
  • Installed copy re-synced to ~/.agents/skills/mutation-testing (diff-verified).

CodeAnt-AI Description

Make mutation-testing guidance accurate and hook failures visible

What Changed

  • Mutation-testing hook failures now show a warning and the specific error in user-visible output while still allowing prompts to continue.
  • The hook only activates for explicit mutation-testing or meaningful test-quality questions, avoiding unrelated prompts such as “are my tests passing?”
  • Guidance now matches current mutmut 3.x, PIT, and Stryker behavior, including updated commands, result locations, diff-run support, and CI caching.
  • Research claims and equivalent-mutant estimates are qualified with source context instead of presenting fixed results as universal.
  • Skill files and repository guidance now include the required licensing notices and clarify that hook errors are reported through both output channels.

Impact

✅ Visible mutation-testing hook failures
✅ Fewer unintended skill injections
✅ Accurate current-tool setup guidance

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…tmut 3.x/PIT/Stryker corrections, cited research claims, license headers
…-assertion trigger, CI-cache wording for Stryker incremental
@cla-bot cla-bot Bot added the cla-signed label Aug 3, 2026
@codeant-ai

codeant-ai Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR d03a6d6 Aug 03, 2026 · 16:52 16:54

@netlify

netlify Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploy Preview for moderaty ready!

Name Link
🔨 Latest commit d03a6d6
🔍 Latest deploy log https://app.netlify.com/projects/moderaty/deploys/6a70c76975bd1f00089ced19
😎 Deploy Preview https://deploy-preview-60--moderaty.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 91
Accessibility: 97
Best Practices: 100
SEO: 100
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Bonobo791, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0cd3ca60-5b60-4ae8-8d58-794f742ef545

📥 Commits

Reviewing files that changed from the base of the PR and between 52ab395 and d03a6d6.

📒 Files selected for processing (6)
  • .agents/skills-src/mutation-testing/SKILL.md
  • .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py
  • .agents/skills-src/mutation-testing/references/agent-mutation-loop.md
  • .agents/skills-src/mutation-testing/references/surviving-mutant-triage.md
  • .agents/skills-src/mutation-testing/references/tools-by-language.md
  • AGENTS.md

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

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Aug 3, 2026
@codeant-ai

codeant-ai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sequence Diagram

This PR tightens when the mutation-testing skill is injected and makes skill-loading failures visible to both the user-facing context and server logs while preserving prompt flow.

sequenceDiagram
    participant User
    participant Hook
    participant SkillFile
    participant Harness

    User->>Harness: Submit prompt
    Harness->>Hook: Run prompt hook
    Hook->>Hook: Match mutation testing trigger

    alt Trigger matches and skill file is readable
        Hook->>SkillFile: Read skill content
        SkillFile-->>Hook: Return skill content
        Hook-->>Harness: Inject skill guidance
    else Trigger matches and skill file cannot be read
        Hook-->>Harness: Show warning and fallback path
        Hook-->>Harness: Log failure to server output
    end
Loading

Generated by CodeAnt AI

@codeant-ai

codeant-ai Bot commented Aug 3, 2026

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: d03a6d68
Scan Time: 2026-08-03 16:54:29 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 1.3% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED No IAC issues

View Full Results

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR successfully resurrects the review fixes from closed PR #58 with clean improvements to the mutation-testing skill. All changes are correct and well-implemented:

Key improvements:

  • License headers properly added to all mutation-testing skill files per repository standards
  • Research citations refined with precise attributions (Yao, Harman & Jia study; Dakhel et al. MuTAP reference)
  • Tool documentation updated for current versions (mutmut 3.x CLI changes, PIT's removed scmMutationCoverage goal)
  • Error handling enhanced in prehook to ensure failures are visible on both stdout and stderr
  • Trigger patterns tightened (plural "weak assertions" support, more precise "are my tests" matching)
  • CI guidance clarified on caching state files rather than committing them

The changes align with the repo's "fail loudly" rule and maintain consistency across all skill documentation. No defects found that would block merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0 (≤ 100 complexity)
Duplication 0 (≤ 1 duplication)

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix mutation-testing skill hook warnings and refresh mutation-tool guidance

🐞 Bug fix 📝 Documentation ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Make mutation-testing prehook failures visible on stdout (and still logged to stderr).
• Tighten prehook trigger phrases to avoid noisy matches while catching “weak assertions”.
• Update mutation-testing docs for current mutmut/PIT/Stryker behavior and cite research/limits.
Diagram

graph TD
  U{{"User prompt"}} --> H["Prehook script"] --> SK["Skill doc"] --> IC(("Injected context"))
  H --> SO(("Stdout WARNING"))
  H --> SE(("Stderr log"))
  R["Reference docs"] --> SK
  subgraph Legend
    direction LR
    _ext{{"External"}} ~~~ _mod["Hook/module"] ~~~ _doc["Markdown doc"] ~~~ _io(("Output channel"))
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Shared hook error-reporting wrapper
  • ➕ Removes duplicated try/except + warning formatting across multiple skills
  • ➕ Enforces consistent stdout/stderr behavior and message format repo-wide
  • ➖ Requires introducing and distributing a shared helper module (and updating other hooks to use it)
  • ➖ Slightly higher coordination cost than a targeted fix in one hook
2. Structured (JSON) diagnostic output channel
  • ➕ Machine-parseable warnings (severity/code/path) for tooling/CI aggregation
  • ➕ Avoids mixing human warnings into injected text streams if a runner can route them separately
  • ➖ Only useful if the hook runner/tooling is prepared to handle a structured side-channel
  • ➖ More complexity than needed for a simple visibility fix

Recommendation: Current approach (emit a concise stdout WARNING in addition to stderr) is the best fit given stdout-injecting harnesses may hide stderr. If multiple skills adopt similar logic, consider extracting a small shared wrapper to standardize formatting and reduce repetition.

Files changed (6) +128 / -42

Enhancement (1) +36 / -29
tools-by-language.mdUpdate mutation-tool docs (mutmut v3, PIT diff mode, Stryker incremental cache) +36/-29

Update mutation-tool docs (mutmut v3, PIT diff mode, Stryker incremental cache)

• Adds AGPL license comment and expands guidance on tool-specific mutant status semantics. Updates Stryker incremental docs to recommend CI caching of the incremental JSON rather than committing it. Refreshes mutmut guidance for 3.x config keys and workflow (browse/TUI, mutants/ state), and corrects PIT PR-diff guidance to reflect removal of scmMutationCoverage and the need for alternative git integration or version pinning. Updates the universal CI pattern to prefer CI caches over repo commits for state files.

.agents/skills-src/mutation-testing/references/tools-by-language.md

Bug fix (1) +35 / -6
skill_prehook.pyMake prehook failures visible on stdout and tighten trigger regexes +35/-6

Make prehook failures visible on stdout and tighten trigger regexes

• Adds license header and clarifies stdout contract to include loud warnings on failures. Tightens keyword triggers (avoids broad "are my tests" matches; supports plural "weak assertions"). On file-read and runtime failures, now emits detailed WARNING messages to stdout while still logging to stderr; exit behavior remains non-breaking (always 0).

.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py

Documentation (4) +57 / -7
SKILL.mdAdd license header and refine equivalent-mutant ceiling guidance +19/-1

Add license header and refine equivalent-mutant ceiling guidance

• Adds an AGPL license notice comment after YAML front matter (to preserve loader requirements). Updates the equivalent-mutant discussion to avoid a single hard-coded rate and references the classic manual-analysis study contextually.

.agents/skills-src/mutation-testing/SKILL.md

agent-mutation-loop.mdAdd license header and polish research/cost-control guidance +18/-3

Add license header and polish research/cost-control guidance

• Adds AGPL license comment and updates the research-support bullets to be less numerically assertive while naming sources. Adjusts fenced block to explicit text formatting and refines cost-control wording, including updated mutmut guidance.

.agents/skills-src/mutation-testing/references/agent-mutation-loop.md

surviving-mutant-triage.mdAdd license header and soften equivalent-mutant rate claim +16/-1

Add license header and soften equivalent-mutant rate claim

• Adds AGPL license comment and reframes equivalent-mutant expectations as a codebase/operator-dependent estimate rather than a fixed percentage.

.agents/skills-src/mutation-testing/references/surviving-mutant-triage.md

AGENTS.mdDocument dual-channel (stdout+stderr) loud failure policy for hooks +4/-2

Document dual-channel (stdout+stderr) loud failure policy for hooks

• Updates repo guidance to specify that hook failures must be visible to users via a stdout WARNING line in addition to stderr logging, while still always exiting 0 to avoid breaking prompt flow.

AGENTS.md

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR successfully updates the mutation-testing skill to align with modern toolchains (mutmut 3.x, PIT 1.7+, and StrykerJS) and improves reliability through a 'fail-loudly' error reporting mechanism in the pre-hook. While the implementation of the dual-channel error reporting (stdout/stderr) and the placement of license headers are correct, there is a significant gap in verification: no unit or integration tests were provided to validate the updated regex triggers or the exception-handling logic. Additionally, the qualitative triggers should be expanded to handle singular nouns to ensure consistent behavior when users query the quality of a single test.

About this PR

  • No new or updated test files were included in the diff to verify the refined regex triggers or the dual-channel error reporting logic, despite the verification steps listed in the PR description.

Test suggestions

  • Verify that the keyword trigger matches plural 'weak assertions'\n- [ ] Verify that the trigger stays silent for 'are my tests passing?' while firing for 'are my tests actually good'\n- [ ] Verify that a FileNotFoundError during skill read emits a WARNING with error details to stdout\n- [ ] Verify that a generic Exception in the hook's main loop emits a WARNING to stdout
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that the keyword trigger matches plural 'weak assertions'\n- [ ] Verify that the trigger stays silent for 'are my tests passing?' while firing for 'are my tests actually good'\n- [ ] Verify that a FileNotFoundError during skill read emits a WARNING with error details to stdout\n- [ ] Verify that a generic Exception in the hook's main loop emits a WARNING to stdout

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

r"\bequivalent mutants?\b", r"\bkill(ing)? (the |those |these |that )?mutants?\b",
r"\btest(-| )suite quality\b",
r"\bare my tests\b", r"\bweak assertion",
r"\bare my tests (actually )?(good|catching|enough|worth|strong)\b", r"\bweak assertions?\b",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: The trigger for evaluating test quality only matches the plural form ("are my tests"). Users often ask questions in the singular (e.g., "is my test good?"). Expanding the regex to handle both singular and plural forms ensures the mutation-testing skill is correctly injected for both cases.\n\nsuggestion\n r"\b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b", r"\bweak assertions?\b",\n

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 77 rules

Grey Divider


Remediation recommended

1. Hook may exit non-zero 🐞 Bug ☼ Reliability
Description
The top-level exception handler writes to stdout/stderr without guarding write failures; if the
original error was caused by a broken output stream (e.g., stdout pipe closed), the handler can
raise again before sys.exit(0), violating the documented “hooks always exit 0” contract.
Code

.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[R155-157]

+        sys.stdout.write(
+            f"[mutation-testing prehook WARNING: hook failed: {type(exc).__name__}: {exc}]\n"
+        )
Relevance

●● Moderate

Hooks-exit-0/log-loudly concerns were accepted before, but no history on guarding stdout write
failures (BrokenPipe).

PR-#55
PR-#51

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new exception handler unconditionally writes a WARNING to stdout during failure handling, so if
stdout is the source of the failure the handler can throw again and never reach sys.exit(0). The
repo documentation explicitly states hooks must always exit 0 and failures should be loud on
stdout/stderr.

.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[147-158]
AGENTS.md[51-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The mutation-testing prehook promises to always exit 0, but the new failure-reporting code writes to stdout/stderr inside exception paths without handling write errors. If stdout/stderr is broken/closed (or the triggering exception is itself a BrokenPipeError from an earlier write), the handler can throw again and terminate before reaching `sys.exit(0)`.

## Issue Context
This PR intentionally makes failures “loud on both channels” by adding `sys.stdout.write(...)` warnings in error paths. That increases the chance of a second exception during error handling.

## Fix Focus Areas
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[122-134]
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[147-158]

## Suggested fix
- Wrap stdout/stderr writes in a small helper like `safe_write(stream, text)` that catches `BrokenPipeError`/`OSError` and returns.
- Use that helper for **all** diagnostic writes (both the OSError fallback warning and the top-level exception handler), so the process reliably reaches `sys.exit(0)` even when output streams are unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. skill_prehook.py uses space indentation 📘 Rule violation ⚙ Maintainability
Description
The updated prehook script includes indented lines that start with spaces rather than tabs. This
violates the repository rule requiring tab-only indentation in changed source files.
Code

.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[R130-133]

+            sys.stdout.write(
+                f"[mutation-testing prehook WARNING: could not read the skill "
+                f"file at {skill_path} ({type(file_error).__name__}: {file_error}); "
+                "falling back to a path directive.]\n"
Relevance

● Weak

Similar “replace spaces with tabs” findings were rejected (tabs-only enforcement not followed).

PR-#14
PR-#11
PR-#7

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2401137 requires that changed source files use tabs for indentation, not spaces.
The modified block around sys.stdout.write(...) in skill_prehook.py is indented with spaces.

Rule 2401137: Use tabs for indentation, never spaces
.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[130-134]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The file `.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py` contains indentation using leading spaces on indented lines, but the repo compliance rule requires tabs for indentation.

## Issue Context
This PR modified the file and added/changed indented lines in the `except OSError` block; those lines currently begin with spaces.

## Fix Focus Areas
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[117-158]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment on lines +155 to +157
sys.stdout.write(
f"[mutation-testing prehook WARNING: hook failed: {type(exc).__name__}: {exc}]\n"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Hook may exit non-zero 🐞 Bug ☼ Reliability

The top-level exception handler writes to stdout/stderr without guarding write failures; if the
original error was caused by a broken output stream (e.g., stdout pipe closed), the handler can
raise again before sys.exit(0), violating the documented “hooks always exit 0” contract.
Agent Prompt
## Issue description
The mutation-testing prehook promises to always exit 0, but the new failure-reporting code writes to stdout/stderr inside exception paths without handling write errors. If stdout/stderr is broken/closed (or the triggering exception is itself a BrokenPipeError from an earlier write), the handler can throw again and terminate before reaching `sys.exit(0)`.

## Issue Context
This PR intentionally makes failures “loud on both channels” by adding `sys.stdout.write(...)` warnings in error paths. That increases the chance of a second exception during error handling.

## Fix Focus Areas
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[122-134]
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[147-158]

## Suggested fix
- Wrap stdout/stderr writes in a small helper like `safe_write(stream, text)` that catches `BrokenPipeError`/`OSError` and returns.
- Use that helper for **all** diagnostic writes (both the OSError fallback warning and the top-level exception handler), so the process reliably reaches `sys.exit(0)` even when output streams are unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@Bonobo791
Bonobo791 merged commit 0786d1d into main Aug 3, 2026
17 of 20 checks passed
@Bonobo791
Bonobo791 deleted the fix-mutation-skill-review branch August 3, 2026 17:07
Bonobo791 added a commit that referenced this pull request Aug 5, 2026
…guards; match singular test-quality triggers
@Bonobo791

Copy link
Copy Markdown
Owner Author

Post-merge review triage — all bot comments addressed

This PR merged before every finding was triaged; the valid ones are fixed in follow-up PR #96.

@qodo-code-review

  • Hook may exit non-zero (bug, skill_prehook.py) — valid, fixed in fix: PR #59/#60 review — FAQ channel-read disclosure; prehook broken-stream guard #96 (81866bb). Reproduced before the fix: a triggering prompt with stdout closed exited 1, breaking the documented always-exit-0 contract — the exception handler's own stream writes were unguarded. Both writes are now individually guarded; verified broken-stdout → exit 0, broken-stderr → exit 0, and the trigger / no-match / garbage-stdin paths unchanged. Installed copy re-synced to ~/.agents/skills/mutation-testing.
  • skill_prehook.py uses spaces, not tabs (rule violation) — declined. It is a Python file, space-indented per PEP 8 since its creation in chore: mutation-testing skill source + pre-hook + role docs #55; the repo's tabs convention targets the Svelte/TypeScript codebase.

@codacy-productionTrigger only matches the plural "are my tests" (low) — valid, fixed in #96 (72d102f). The trigger is now \b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b. Verified: "is my test good?" and the plural forms inject the skill; "how are my tests today" and unrelated prompts stay silent.

@CodeAnt-AI — quality gate passed; sequence diagram and review status comments are informational, nothing actionable. @coderabbitai — rate-limited, no actionable comments. @netlify — deploy preview, informational. @sonarqubecloud — quality gate passed.

@codeant-ai

codeant-ai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Question: a triggering prompt with stdout closed exited 1, breaking the documented always-exit-0 contract — the exception handler's own stream writes were unguarded. both writes are now individually guarded; verified broken-stdout → exit 0, broken-stderr → exit 0, and the trigger / no-match / garbage-stdin paths unchanged. installed copy re-synced to ~/.agents/skills/mutation-testing.

@codacy-productiontrigger only matches the plural "are my tests" (low) — valid, fixed in #96 (72d102f). the trigger is now \b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b. verified: "is my test good?" and the plural forms inject the skill; "how are my tests today" and unrelated prompts stay silent.

@CodeAnt-AI — quality gate passed; sequence diagram and review status comments are informational, nothing actionable. @coderabbitai — rate-limited, no actionable comments. @netlify — deploy preview, informational. @sonarqubecloud — quality gate passed.

Answer:
The always exit 0 contract is still broken for a triggering prompt when stdout is closed.

In the outer exception handler, both writes remain unguarded:

sys.stderr.write(...)
sys.stdout.write(...)

If main() raises because stdout is unavailable, the first stderr write may succeed, but the subsequent stdout.write() raises BrokenPipeError. That exception escapes the handler, so Python exits nonzero instead of reaching sys.exit(0). If stderr is also unavailable, the first write can fail for the same reason.

Guard each diagnostic stream independently, for example:

try:
    sys.stderr.write(
        f"mutation-testing prehook failed: {type(exc).__name__}: {exc}\n"
    )
except OSError:
    pass

try:
    sys.stdout.write(
        f"[mutation-testing prehook WARNING: hook failed: "
        f"{type(exc).__name__}: {exc}]\n"
    )
except OSError:
    pass

The same principle should be applied to the OSError fallback branch if the hook must guarantee exit 0 under broken stdout/stderr in all failure paths. After that, the final sys.exit(0) can execute reliably.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="Bonobo791/Moderaty"

printf '%s\n' '== PR `#96` metadata =='
gh pr view 96 --repo "$repo" --json number,state,mergedAt,url,title,commits \
  --jq '{number, state, mergedAt, url, title, commits: [.commits[] | {oid: .oid, messageHeadline: .messageHeadline}]}'

printf '%s\n' '== PR `#96` changed files =='
gh pr view 96 --repo "$repo" --json files \
  --jq '.files[] | {path, additions, deletions}'

printf '%s\n' '== Stated commits =='
for sha in 81866bb 72d102f; do
  gh api "repos/${repo}/commits/${sha}" \
    --jq '{sha: .sha, message: .commit.message, files: [.files[] | {filename, status, patch}]}'
done

Length of output: 7147


@Bonobo791 Confirmed from the PR #96 diffs.

  • Commit 81866bb guards each output-stream write in skill_prehook.py. The handler can now reach sys.exit(0) when either stdout or stderr is broken.
  • Commit 72d102f supports both is my test good? and plural forms with \b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b.
  • The space indentation in this Python file is appropriate. It does not require a tabs-based change.

PR #96 is still open. The fixes are not merged yet, but the inspected changes address the stated findings.


✏️ Learnings added
Learnt from: Bonobo791
URL: https://github.com/Bonobo791/Moderaty/pull/60

Timestamp: 2026-08-05T14:10:21.672Z
Learning: In the Moderaty repository, the tabs convention applies to the Svelte/TypeScript codebase. Python files, including `.agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py`, use spaces according to PEP 8.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@qodo-code-review

Copy link
Copy Markdown

Post-merge review triage — all bot comments addressed

This PR merged before every finding was triaged; the valid ones are fixed in follow-up PR #96.

@qodo-code-review

  • Hook may exit non-zero (bug, skill_prehook.py) — valid, fixed in fix: PR #59/#60 review — FAQ channel-read disclosure; prehook broken-stream guard #96 (81866bb). Reproduced before the fix: a triggering prompt with stdout closed exited 1, breaking the documented always-exit-0 contract — the exception handler's own stream writes were unguarded. Both writes are now individually guarded; verified broken-stdout → exit 0, broken-stderr → exit 0, and the trigger / no-match / garbage-stdin paths unchanged. Installed copy re-synced to ~/.agents/skills/mutation-testing.
  • skill_prehook.py uses spaces, not tabs (rule violation) — declined. It is a Python file, space-indented per PEP 8 since its creation in chore: mutation-testing skill source + pre-hook + role docs #55; the repo's tabs convention targets the Svelte/TypeScript codebase.

@codacy-productionTrigger only matches the plural "are my tests" (low) — valid, fixed in #96 (72d102f). The trigger is now \b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b. Verified: "is my test good?" and the plural forms inject the skill; "how are my tests today" and unrelated prompts stay silent.

@CodeAnt-AI — quality gate passed; sequence diagram and review status comments are informational, nothing actionable. @coderabbitai — rate-limited, no actionable comments. @netlify — deploy preview, informational. @sonarqubecloud — quality gate passed.

Thanks for the post-merge triage. Based on the details provided:

The remaining bot comments are informational or already covered by the follow-up fixes.

Bonobo791 added a commit that referenced this pull request Aug 5, 2026
fix: PR #59/#60 review — FAQ channel-read disclosure; prehook broken-stream guard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant