Skip to content

fix(#914): space before Skill in tools frontmatter - #928

Merged
waynesun09 merged 1 commit into
mainfrom
fix/914-skill-tools-spacing
Aug 22, 2026
Merged

fix(#914): space before Skill in tools frontmatter#928
waynesun09 merged 1 commit into
mainfrom
fix/914-skill-tools-spacing

Conversation

@ascerra

@ascerra ascerra commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fix(#914): add Skill to triage and prioritize tools #917 added Skill as tools: Bash(gh,curl,jq),Skill (no space after the comma).
  • Live runs with that form boot as tools: ["Bash"] only — Skill is dropped by Claude frontmatter parsing.
  • Spaced form Bash(gh,curl,jq), Skill correctly yields tools: ["Bash", "Skill"] (proven on the proof tip).

Test plan

  • New triage run init tools include Skill
  • Skill tool_use observed for labeling / forge skills

@ascerra
ascerra requested a review from a team as a code owner August 21, 2026 19:26
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix tools frontmatter spacing so Skill is parsed correctly

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 5 minutes

Grey Divider

AI Description

• Add a space after the comma in tools: frontmatter for prioritize and triage agents.
• Prevent Claude frontmatter parsing from dropping Skill from the tool list.
• Ensure agent boots with both Bash and Skill tools enabled.
Diagram

graph TD
  A["agents/*.md"] --> B["Frontmatter parser"] --> C["Parsed tools list"] --> D["Agent runtime"] --> E["Skill-based actions"]
  D --> F["Bash tool"]

  subgraph Legend
    direction LR
    _file["File"] ~~~ _cmp["Component"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Make frontmatter tools parser whitespace-tolerant
  • ➕ Fixes this class of issues across all agents, even if formatting regresses
  • ➕ More robust to minor authoring differences
  • ➖ Requires changing and validating parsing logic
  • ➖ Potentially wider blast radius if other parsing edge-cases exist
2. Use explicit YAML list syntax for tools
  • ➕ Avoids reliance on comma parsing/spacing rules
  • ➕ Clearer intent and less error-prone in reviews
  • ➖ May require updating existing conventions and documentation
  • ➖ Could require parser support if only string form is currently accepted

Recommendation: Given the minimal scope and immediate impact, the formatting fix is the best short-term solution. If this issue recurs, consider hardening the tools parser to trim tokens or migrating to an explicit YAML list representation to prevent whitespace-sensitive parsing bugs.

Files changed (2) +2 / -2

Bug fix (2) +2 / -2
prioritize.mdFix tools frontmatter spacing to include 'Skill' +1/-1

Fix tools frontmatter spacing to include 'Skill'

• Adds a space after the comma in the 'tools:' frontmatter entry so 'Skill' is not dropped during Claude frontmatter parsing. Ensures prioritize runs initialize with both 'Bash' and 'Skill'.

agents/prioritize.md

triage.mdFix tools frontmatter spacing to include 'Skill' +1/-1

Fix tools frontmatter spacing to include 'Skill'

• Adds a space after the comma in the 'tools:' frontmatter entry so 'Skill' is correctly parsed and included. Ensures triage runs initialize with both 'Bash' and 'Skill'.

agents/triage.md

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:28 PM UTC · Completed 7:38 PM UTC

Commit: bc91a1d · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Protected agents/ files modified 📜 Skill insight § Compliance
Description
This PR changes agent definitions under agents/, a protected governance/infrastructure path that
requires explicit human review and must not be auto-approved. Ensure appropriate owners review this
change and that the issue context/justification is captured in the PR.
Code

agents/prioritize.md[9]

+tools: Bash(gh,curl,jq), Skill
Relevance

●●● Strong

Precedent shows agents/ frontmatter changes get scrutinized/flagged as protected path in recent PR.

PR-#855

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The compliance checklist requires raising a finding for any modifications under protected paths
(including agents/). The diff shows changes to agents/prioritize.md and agents/triage.md on
the tools: frontmatter line, placing this PR squarely in the protected-path category.

agents/prioritize.md[1-11]
agents/triage.md[1-11]
Skill: pr-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
Files under `agents/` are protected governance/infrastructure paths and changes must not be auto-approved.

## Issue Context
This PR modifies `tools:` frontmatter lines in agent definitions.

## Fix Focus Areas
- agents/prioritize.md[9-9]
- agents/triage.md[9-9]

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


2. No test for tools: fix 📜 Skill insight ▣ Testability
Description
The PR changes tools: frontmatter to alter runtime tool initialization (ensuring Skill is
included), but includes no corresponding test change. Add or extend an automated test/lint check to
prevent regressions in frontmatter parsing expectations.
Code

agents/triage.md[9]

+tools: Bash(gh,curl,jq), Skill
Relevance

●● Moderate

Test-coverage findings on trivial frontmatter typo fixes are uncertain; no close precedent found.

PR-#855

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist requires every behavioral change to have a corresponding test change. The diff
modifies agent tools: frontmatter (behavioral/runtime effect) and contains no test-file
modifications, indicating missing test coverage for the changed behavior.

agents/prioritize.md[8-10]
agents/triage.md[8-10]
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
This PR changes agent `tools:` frontmatter behavior (comma spacing) but does not add/update any automated test to constrain the new expected behavior.

## Issue Context
A regression in `tools:` formatting can cause `Skill` to be dropped by the frontmatter parser, changing the tools available at runtime.

## Fix Focus Areas
- agents/prioritize.md[9-9]
- agents/triage.md[9-9]

ⓘ 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

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread agents/prioritize.md
Comment thread agents/triage.md
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Previous run

Review

Findings

Medium

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 21, 2026
@ascerra

ascerra commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

/ok-to-test

Signed-off-by: Adam Scerra <ascerra@redhat.com>
@ascerra
ascerra force-pushed the fix/914-skill-tools-spacing branch from bc91a1d to 6a17440 Compare August 22, 2026 14:09
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:11 PM UTC · Completed 2:21 PM UTC

Commit: 6a17440 · View workflow run →

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Human approval for the protected agents/ path.

The change is a one-character space after the comma on triage and prioritize tools: lines so Claude’s frontmatter parser keeps Skill (Bash(gh,curl,jq), Skill["Bash","Skill"]). Those are the only two fleet agents with this ,Skill form; CI (including the triage functional suite) is green.

Qodo’s missing-lint note is already tracked as #927 and is not blocking for this surgical fix.

@waynesun09
waynesun09 added this pull request to the merge queue Aug 22, 2026
Merged via the queue into main with commit 6414483 Aug 22, 2026
29 of 30 checks passed
@waynesun09
waynesun09 deleted the fix/914-skill-tools-spacing branch August 22, 2026 18:46
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 22, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:48 PM UTC · Completed 6:58 PM UTC

Commit: 6a17440 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #928 — space before Skill in tools frontmatter

Timeline

  1. 2026-07-06: Claude Code v2.1.119 enforced tools: frontmatter as a hard clamp on agent tool access. The triage and prioritize agents silently lost Skill tool access because their tools: field listed only Bash(gh,curl,jq) without Skill. This went undetected for 6 weeks.
  2. 2026-08-21 11:12 UTC: ascerra filed issue #914 with detailed evidence (22/22 sampled runs were Bash-only with zero Skill opens). The issue recommended the fix tools: Bash(gh,curl,jq),Skill (no space after comma).
  3. 2026-08-21 11:26 UTC: The code agent created PR #917 implementing the issue's recommended fix verbatim: Bash(gh,curl,jq),Skill.
  4. 2026-08-21 ~11:40 UTC: The review agent on PR fix(#914): add Skill to triage and prioritize tools #917 flagged only protected-path (correct). Neither the review agent nor the two human reviewers (ben-alkov: "LGTM", ascerra: empty approval) caught the spacing issue.
  5. 2026-08-21 18:56 UTC: PR fix(#914): add Skill to triage and prioritize tools #917 merged — but the fix was ineffective. Claude's frontmatter parser tokenizes Bash(gh,curl,jq),Skill as ["Bash"] only, silently dropping Skill.
  6. 2026-08-21 19:26 UTC: ascerra discovered the parsing quirk and created PR #928 adding a space: , Skill → parses as ["Bash", "Skill"].
  7. 2026-08-22 18:36 UTC: waynesun09 approved PR fix(#914): space before Skill in tools frontmatter #928 with thorough justification, noting the missing-lint concern is tracked in Add CI test for agent tool-skill declaration consistency #927.
  8. 2026-08-22 18:46 UTC: PR fix(#914): space before Skill in tools frontmatter #928 merged. Skill tool access restored.

Workflow quality assessment

Review quality: The review agent correctly flagged protected-path on both PRs — appropriate for agents/ modifications. However, neither the review agent nor human reviewers on PR #917 detected that the tools: field would not parse correctly. This is understandable: the parsing behavior (comma without trailing space silently drops subsequent tokens) is undocumented and non-obvious.

Rework rate: The fix required two PRs (code agent PR #917 + human follow-up PR #928), adding ~23 hours to resolution. The rework was caused by the issue recommending incorrect syntax that the code agent faithfully implemented.

Token cost: Low — both PRs were 2-line changes. Review agent costs were proportionate.

Root cause: The issue's recommended fix used a format (Bash(gh,curl,jq),Skill) that Claude Code's frontmatter parser silently mishandles. No documentation exists for the tools: field syntax requirements in agent definitions. AGENTS.md section 7 documents skill frontmatter fields but says nothing about agent definition frontmatter conventions.

Existing issue coverage

Proposals filed

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

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants