Skip to content

fix(#2332): respect tool-owned filenames - #1096

Merged
rh-hemartin merged 2 commits into
fullsend-ai:mainfrom
shairevivo:fix/2332-tool-filenames
Sep 1, 2026
Merged

fix(#2332): respect tool-owned filenames#1096
rh-hemartin merged 2 commits into
fullsend-ai:mainfrom
shairevivo:fix/2332-tool-filenames

Conversation

@shairevivo

@shairevivo shairevivo commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • treat tool-recognized filenames as compatibility contracts
  • require repository evidence before recommending an alternative filename
  • add a deterministic review eval for the .codecov.yml regression

Addresses fullsend-ai/fullsend#2332.

Reproduction: konflux-ci/mintmaker-schedule-calculator#25 (comment)

Testing

  • pre-commit run --from-ref origin/main --to-ref HEAD
  • git diff --check origin/main...HEAD
  • synthetic finding judge check rejects [file-naming-convention] and accepts clean output

Functional tests require a maintainer to apply ok-to-test after the latest push.

@shairevivo
shairevivo requested a review from a team as a code owner August 31, 2026 14:40
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Functional tests are running

Authorization passed for this commit. See the Functional Tests workflow for results.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Respect external tool-owned filenames during style review

🐞 Bug fix 📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Treat tool-recognized filenames as compatibility contracts during style review.
• Require repository evidence before recommending alternative filenames.
• Clarify filename examples are illustrative rather than exhaustive.
Diagram

sequenceDiagram
  actor Reviewer
  participant Agent as Style Agent
  participant Tools as Repository Tools
  participant Output as Review Output
  Reviewer->>Agent: Review filename
  Agent->>Tools: Verify alternative support
  alt Support confirmed
    Tools-->>Agent: Return repository evidence
    Agent->>Output: Report finding
  else Support unavailable
    Agent->>Output: Suppress finding
  end
Loading
High-Level Assessment

The evidence-based compatibility rule is the appropriate approach. A static allowlist was considered but would inevitably miss valid tool-owned filenames, while unconditional convention enforcement could recommend breaking integrations.

Files changed (1) +12 / -0

Bug fix (1) +12 / -0
style-conventions.mdProtect tool-owned filenames from unsupported rename findings +12/-0

Protect tool-owned filenames from unsupported rename findings

• Adds guidance that externally recognized filenames are compatibility contracts. Filename-convention findings now require repository evidence that an alternative is supported, and the provided examples are explicitly non-exhaustive.

skills/pr-review/sub-agents/style-conventions.md

@shairevivo
shairevivo force-pushed the fix/2332-tool-filenames branch from 1caf036 to a2c5c88 Compare August 31, 2026 14:47
@qodo-code-review

qodo-code-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Protected skills/ path modified ✗ Dismissed 📜 Skill insight § Compliance
Description
This PR modifies skills/pr-review/sub-agents/style-conventions.md, which is an explicitly
protected governance path. Although issue #2332 provides justification, the change still requires
human approval and must not be auto-approved.
Code

skills/pr-review/sub-agents/style-conventions.md[R23-25]

+## Tool-owned filenames
+
+Treat filenames recognized by external tools as compatibility contracts. Do
Relevance

●●● Strong

Protected skills/ changes require human approval; similar compliance findings were raised for PRs
#59 and #157.

PR-#59
PR-#157

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538392 requires a finding whenever any file under skills/ is modified; the added
section is in that protected path, while the PR title and description link the change to issue
#2332.

skills/pr-review/sub-agents/style-conventions.md[23-25]
Skill: pr-review


2. Filename behavior lacks tests 📜 Skill insight ▣ Testability
Description
The change alters reviewer behavior for tool-owned filenames, but the PR contains no corresponding
test change exercising the new compatibility-contract and repository-evidence rules. Without a
behavioral test, regressions could restore the false-positive filename recommendations addressed by
#2332.
Code

skills/pr-review/sub-agents/style-conventions.md[R25-29]

+Treat filenames recognized by external tools as compatibility contracts. Do
+not recommend renaming a tool configuration file solely to match repository
+conventions. Only raise a filename-convention finding when repository evidence
+confirms that the proposed alternative is supported; if support cannot be
+established with the available tools, report no finding.
Relevance

●● Moderate

Testing expectations for Markdown skill behavior are unclear; historical guidance changes include
both accepted and undetermined outcomes.

PR-#1038
PR-#56

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538339 requires every behavioral change to have a corresponding test change. The
diff adds explicit new decision behavior in the style-conventions agent, while the PR diff contains
only this skill file and no test or evaluation modification.

skills/pr-review/sub-agents/style-conventions.md[25-29]
Skill: code-implementation

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 new filename-review behavior has no corresponding test change.

## Issue Context
Add a behavioral evaluation that proves recognized tool-owned filenames are not flagged merely for repository style, and that alternatives are recommended only when repository evidence confirms tool support. Assert specific finding outcomes rather than only successful execution.

## Fix Focus Areas
- skills/pr-review/sub-agents/style-conventions.md[25-29]

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



Informational

3. Agent instructions added 📜 Skill insight ⛨ Security
Description
The new imperative text directly instructs a review agent when to raise or suppress findings. This
introduces an agent-instruction pattern in a changed Markdown/configuration artifact, which the
checklist expressly prohibits.
Code

skills/pr-review/sub-agents/style-conventions.md[R25-29]

+Treat filenames recognized by external tools as compatibility contracts. Do
+not recommend renaming a tool configuration file solely to match repository
+conventions. Only raise a filename-convention finding when repository evidence
+confirms that the proposed alternative is supported; if support cannot be
+established with the available tools, report no finding.
Relevance

● Weak

The imperative text implements the PR’s stated agent-guidance intent, matching accepted guidance
additions in PRs #1038 and #172.

PR-#1038
PR-#172

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538322 prohibits system-prompt or role-instruction patterns in changed comments,
strings, or configuration values. The cited lines imperatively direct the agent to suppress or emit
review findings.

skills/pr-review/sub-agents/style-conventions.md[25-29]
Skill: code-review

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 added text contains direct agent instructions such as `Do not recommend` and `report no finding`, violating the prohibition on agent-instruction patterns in changed strings or configuration content.

## Issue Context
Preserve the intended filename compatibility behavior without committing imperative agent-prompt content that matches the prohibited pattern.

## Fix Focus Areas
- skills/pr-review/sub-agents/style-conventions.md[25-29]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 56 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread skills/pr-review/sub-agents/style-conventions.md
Comment thread skills/pr-review/sub-agents/style-conventions.md
@rh-hemartin

Copy link
Copy Markdown
Member

Skillsaw problems are being fixed at #1101

@shairevivo

Copy link
Copy Markdown
Contributor Author

Skillsaw problems are being fixed at #1101

@rh-hemartin should I close this PR? and can you close issue - #2332 ?

@rh-hemartin

Copy link
Copy Markdown
Member

Skillsaw problems are being fixed at #1101

@rh-hemartin should I close this PR? and can you close issue - #2332 ?

Why would you close this PR? Your PR has nothing to do with Skillsaw problems. I'm just telling you that the skillsaw problems you are having on CI are being solved, so you will need to rebase when those land. And this day and age... are you reading this or this is an agent?

@shairevivo

Copy link
Copy Markdown
Contributor Author

@rh-hemartin LOL, it is me. I misinterpreted your response. thanks for clarifying.
Thanks!

@rh-hemartin

Copy link
Copy Markdown
Member

Sorry, I am having bad experiencies with folks answering with AI and I'm starting to get paranoid. The PR will be merged soon: #1101, so you will be able to rebase soon.

shairevivo and others added 2 commits September 1, 2026 14:01
Signed-off-by: Shai Revivo <srevivo@redhat.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Shai Revivo <srevivo@redhat.com>
Co-authored-by: Codex <noreply@openai.com>
@shairevivo
shairevivo force-pushed the fix/2332-tool-filenames branch from 888ca0b to 00f637e Compare September 1, 2026 11:02
@rh-hemartin
rh-hemartin added this pull request to the merge queue Sep 1, 2026
Merged via the queue into fullsend-ai:main with commit c4e059f Sep 1, 2026
32 of 33 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Sep 1, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:27 PM UTC · Completed 12:39 PM UTC

Commit: 00f637e · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.34

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1096fix(#2332): respect tool-owned filenames

What happened

PR #1096 was a human-authored fix by shairevivo (external contributor, read permission) that adds tool-owned filename guidance to the style-conventions review sub-agent and a deterministic eval test case. It addresses fullsend-ai/fullsend#2332, which reported the review agent incorrectly flagging .codecov.yml for not matching a repo's .yaml naming convention.

Timeline:

  • Aug 31 14:40 — PR opened with initial commit (co-authored with Codex)
  • Aug 31 14:53 — qodo-code-review flagged missing tests for the new behavior
  • Aug 31 15:01 — Author pushed test commit adding a finding_expectations eval judge and a .codecov.yml regression fixture
  • Sep 1 09:25rh-hemartin approved the PR
  • Sep 1 12:25 — PR merged via merge queue

CI failures during the window were not flaky: the CI/skillsaw failure was a pre-existing violation in main fixed by PR #1101, and functional test failures were expected behavior (the detect job only authorizes on labeled events, not synchronize).

Agent involvement: none

No fullsend agents (code, review, fix) ran on this PR. The review agent was intentionally skipped by the routing logic because shairevivo has read-only repository access — dispatch requires at least triage permission. The only agent dispatched was the retro agent post-merge.

Assessment

The workflow went well. The PR is clean and well-scoped — it adds the right guidance to the right sub-agent, includes a deterministic eval test, and follows conventional commit format. The qodo-code-review bot provided useful signal (flagging missing tests), and the author responded promptly. The human reviewer approved after the skillsaw CI blocker was resolved in a parallel PR.

Existing issues with new supporting evidence

  • #349 (RICE 5.4) — Pre-retro should skip retro on PRs with zero fullsend agent involvement. This retro run is a direct instance of the waste described: the retro agent was dispatched, consumed compute and tokens, and found no agent workflow to analyze because no agents ran on this PR.

  • fullsend-ai/fullsend#2332 remains open despite PR merge. The PR body uses "Addresses" (not a closing keyword), and cross-repo auto-closing is a known GitHub limitation. This is tracked by fullsend#5924, fullsend#1302, and agents#678.

No new proposals warranted — existing issues already cover the identified improvement areas.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants