Add 20 skills from marketplace#8
Conversation
…, web-design-guidelines, agent-browser, shadcn, systematic-debugging, writing-plans, verification-before-completion, test-driven-development, requesting-code-review, receiving-code-review, executing-plans, dispatching-parallel-agents, subagent-driven-development, using-git-worktrees, finishing-a-development-branch, webapp-testing, mcp-builder, github-actions-docs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| **Helper Scripts Available**: | ||
| - `scripts/with_server.py` - Manages server lifecycle (supports multiple servers) | ||
|
|
||
| **Always run scripts with `--help` first** to see usage. DO NOT read the source until you try running the script first and find that a customized solution is abslutely necessary. These scripts can be very large and thus pollute your context window. They exist to be called directly as black-box scripts rather than ingested into your context window. |
There was a problem hiding this comment.
SUGGESTION: Typo — abslutely should be absolutely
| **Always run scripts with `--help` first** to see usage. DO NOT read the source until you try running the script first and find that a customized solution is abslutely necessary. These scripts can be very large and thus pollute your context window. They exist to be called directly as black-box scripts rather than ingested into your context window. | |
| **Always run scripts with `--help` first** to see usage. DO NOT read the source until you try running the script first and find that a customized solution is absolutely necessary. These scripts can be very large and thus pollute your context window. They exist to be called directly as black-box scripts rather than ingested into your context window. |
|
|
||
| # Get list of test files | ||
| TEST_FILES=$(find . -path "$TEST_PATTERN" | sort) | ||
| TOTAL=$(echo "$TEST_FILES" | wc -l | tr -d ' ') |
There was a problem hiding this comment.
WARNING: find . -path "$TEST_PATTERN" does not support ** glob syntax and will silently match nothing when the pattern contains ** (e.g. src/**/*.test.ts). find's -path flag matches against the full relative path including ./ prefix using basic fnmatch, which doesn't expand ** for recursive descent.
For a pattern like src/**/*.test.ts, the practical fix is to split on the extension and use -name:
TEST_FILES=$(find . -name "*.test.ts" | sort)Or use find with -path "./src/**/*.test.ts" and rely on the shell to expand ** before passing it to find, which only works when the shell itself supports globstar (i.e. shopt -s globstar in bash). As written, the pattern is passed as a literal string to find -path and ** is never expanded, so the loop body never executes and the script always reports "No polluter found" regardless of the actual situation.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)
Several skills (
The note on line 16 tells agents to use Multiple files missing trailing newline The following new files end without a trailing newline, which can cause noisy diffs in future PRs:
Files Reviewed (139 files)
Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 1,275,485 tokens |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| while response.stop_reason == "tool_use": | ||
| tool_use = next(block for block in response.content if block.type == "tool_use") | ||
| tool_name = tool_use.name | ||
| tool_input = tool_use.input | ||
|
|
||
| tool_start_ts = time.time() | ||
| try: | ||
| tool_result = await connection.call_tool(tool_name, tool_input) | ||
| tool_response = json.dumps(tool_result) if isinstance(tool_result, (dict, list)) else str(tool_result) | ||
| except Exception as e: | ||
| tool_response = f"Error executing tool {tool_name}: {str(e)}\n" | ||
| tool_response += traceback.format_exc() | ||
| tool_duration = time.time() - tool_start_ts | ||
|
|
||
| if tool_name not in tool_metrics: | ||
| tool_metrics[tool_name] = {"count": 0, "durations": []} | ||
| tool_metrics[tool_name]["count"] += 1 | ||
| tool_metrics[tool_name]["durations"].append(tool_duration) | ||
|
|
||
| messages.append({ | ||
| "role": "user", | ||
| "content": [{ | ||
| "type": "tool_result", | ||
| "tool_use_id": tool_use.id, | ||
| "content": tool_response, | ||
| }] | ||
| }) |
There was a problem hiding this comment.
🔴 Agent loop only processes first tool_use block, silently dropping parallel tool calls
The agent_loop function uses next(block for block in response.content if block.type == "tool_use") at line 110, which only extracts the first tool_use block from the response. The Anthropic Messages API can return multiple tool_use blocks in a single response when Claude wants to call several tools in parallel. Per the API contract, all corresponding tool_result blocks must be sent back in a single user message. The current code only executes and returns one tool result per loop iteration, so additional tool calls are silently ignored. This causes the model to either error out or be forced to re-request the dropped tool calls, leading to incorrect evaluation metrics (inflated tool call counts, longer durations) and potentially wrong answers.
Prompt for agents
In agent_loop() at evaluation.py:109-135, the while loop processes only the first tool_use block per response via next(). The Anthropic API can return multiple tool_use blocks in one response (parallel tool calls), and ALL tool_result blocks must be returned in a single user message.
The fix is to:
1. Collect ALL tool_use blocks from response.content (not just the first via next())
2. Execute each tool call (sequentially or concurrently via asyncio.gather)
3. Build a single user message containing ALL tool_result entries
4. Track metrics for each tool call
Relevant code: the while loop at line 109 and the message append at lines 128-135. The tool_use extraction at line 110 should become a list comprehension filtering all blocks with type == 'tool_use', and lines 111-135 should iterate over all of them, accumulating results into a single content list for the user message.
Was this helpful? React with 👍 or 👎 to provide feedback.
| process = subprocess.Popen( | ||
| server['cmd'], | ||
| shell=True, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE | ||
| ) |
There was a problem hiding this comment.
🔴 subprocess.PIPE on long-running server processes causes deadlock
At lines 69-74, server processes are started with stdout=subprocess.PIPE, stderr=subprocess.PIPE, but the pipe output is never consumed. For long-running servers that continuously produce log output (e.g., npm run dev), the OS pipe buffer (typically 64KB on Linux) will fill up, causing the server process to block on its next write to stdout/stderr. This deadlock makes the server stop responding to requests, causing the automation command to hang or fail. The pipes are never read before cleanup at the end.
| process = subprocess.Popen( | |
| server['cmd'], | |
| shell=True, | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE | |
| ) | |
| process = subprocess.Popen( | |
| server['cmd'], | |
| shell=True, | |
| stdout=subprocess.DEVNULL, | |
| stderr=subprocess.DEVNULL | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
| tool_metrics = {} | ||
|
|
||
| while response.stop_reason == "tool_use": | ||
| tool_use = next(block for block in response.content if block.type == "tool_use") |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
agent_loop only executes the first tool_use block in each assistant message even though Anthropic tool-use responses can contain one or more tool_use blocks in the same turn. That means any evaluation task that triggers multiple tool calls in one response will send back only one tool_result, leaving the remaining requested calls unanswered and producing incorrect failures or stalled tool loops. Iterate over every tool_use block in response.content and return a matching tool_result for each before asking Claude for the next turn.
# .agents/skills/mcp-builder/scripts/evaluation.py
while response.stop_reason == "tool_use":
tool_use = next(block for block in response.content if block.type == "tool_use")
tool_name = tool_use.name
tool_input = tool_use.input| process = subprocess.Popen( | ||
| server['cmd'], | ||
| shell=True, | ||
| stdout=subprocess.PIPE, |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The helper starts each server with both stdout and stderr redirected to pipes, but it never reads from either stream. Long-running dev servers like Vite/Next/Webpack routinely emit enough startup/runtime output to fill those OS pipe buffers; once that happens the child blocks on write, and this wrapper hangs even though the port check and test workflow are otherwise valid. Either inherit the parent stdio or actively drain both pipes while the server is running.
# .agents/skills/webapp-testing/scripts/with_server.py
process = subprocess.Popen(
server['cmd'],
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE
)| - **MCP Best Practices**: [📋 View Best Practices](./reference/mcp_best_practices.md) - Core guidelines | ||
|
|
||
| **For TypeScript (recommended):** | ||
| - **TypeScript SDK**: Use WebFetch to load `https://raw.githubusercontent.com/modelcontextprotocol/typescript-sdk/main/README.md` |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
This skill tells readers to load the TypeScript SDK docs from the typescript-sdk main branch even though the official README on that branch now says main contains the v2 pre-alpha SDK, while the rest of this skill still teaches the v1 package layout (@modelcontextprotocol/sdk and @modelcontextprotocol/sdk/server/...). That sends users to a different SDK generation than the examples they are supposed to follow, so they can easily end up mixing incompatible package names and APIs. Point this link at the stable v1 docs (or the v1.x branch) instead.
# .agents/skills/mcp-builder/SKILL.md
**For TypeScript (recommended):**
- **TypeScript SDK**: Use WebFetch to load `https://raw.githubusercontent.com/modelcontextprotocol/typescript-sdk/main/README.md`
- [⚡ TypeScript Guide](./reference/node_mcp_server.md) - TypeScript patterns and examples| - **TypeScript SDK**: Use WebFetch to load `https://raw.githubusercontent.com/modelcontextprotocol/typescript-sdk/main/README.md` | |
| - **TypeScript SDK**: Use WebFetch to load `https://ts.sdk.modelcontextprotocol.io/` |
|
|
||
| # Streamable HTTP transport (for remote servers) | ||
| if __name__ == "__main__": | ||
| mcp.run(transport="streamable_http", port=8000) |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The FastMCP example uses transport="streamable_http", but the upstream Python SDK docs use the hyphenated transport name streamable-http. Readers who copy this example will not start the advertised HTTP transport successfully, so the documentation currently teaches a non-working invocation for remote servers.
# .agents/skills/mcp-builder/reference/python_mcp_server.md
# Streamable HTTP transport (for remote servers)
if __name__ == "__main__":
mcp.run(transport="streamable_http", port=8000)| mcp.run(transport="streamable_http", port=8000) | |
| mcp.run(transport="streamable-http", port=8000) |
|
|
||
| ```bash | ||
| # Try common base branches | ||
| git merge-base HEAD main 2>/dev/null || git merge-base HEAD master 2>/dev/null |
There was a problem hiding this comment.
[🟠 High] [🔵 Bug]
The step that is supposed to determine <base-branch> uses git merge-base, but that command returns a common-ancestor commit ID, not main/master. Following the later instructions literally would detach HEAD before git pull/git merge, so the merge workflow no longer operates on the intended branch.
# .agents/skills/finishing-a-development-branch/SKILL.md
git merge-base HEAD main 2>/dev/null || git merge-base HEAD master 2>/dev/null
...
git checkout <base-branch>Use a real branch name here (or require explicit confirmation of the base branch) before reusing it in checkout/pull/merge commands.
| path="$LOCATION/$BRANCH_NAME" | ||
| ;; | ||
| ~/.config/superpowers/worktrees/*) | ||
| path="~/.config/superpowers/worktrees/$project/$BRANCH_NAME" |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The global-location branch assigns path using a quoted ~, but shells do not perform tilde expansion inside double quotes. If an agent follows this snippet, the "global" option resolves to a literal ~/... path instead of the user's home directory, so git worktree add targets the wrong location.
# .agents/skills/using-git-worktrees/SKILL.md
~/.config/superpowers/worktrees/*)
path="~/.config/superpowers/worktrees/$project/$BRANCH_NAME"
;;Use $HOME (or leave ~ unquoted) when constructing the path.
| path="~/.config/superpowers/worktrees/$project/$BRANCH_NAME" | |
| path="$HOME/.config/superpowers/worktrees/$project/$BRANCH_NAME" |
|
|
||
| ## Requirements/Plan | ||
|
|
||
| {PLAN_REFERENCE} |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The surrounding skill docs and examples tell callers to populate PLAN_OR_REQUIREMENTS, but the template’s main Requirements section uses {PLAN_REFERENCE} instead. As a result, the rendered reviewer prompt leaves the primary requirements block blank/unexpanded, which removes the spec context the reviewer is supposed to compare the diff against.
# .agents/skills/requesting-code-review/code-reviewer.md
**Your task:**
1. Review {WHAT_WAS_IMPLEMENTED}
2. Compare against {PLAN_OR_REQUIREMENTS}
...
## Requirements/Plan
{PLAN_REFERENCE}Make the template placeholder match the documented caller contract (or update all callers consistently).
| {PLAN_REFERENCE} | |
| {PLAN_OR_REQUIREMENTS} |
|
|
||
| Do not use this skill for: | ||
|
|
||
| - A specific failing PR check, missing workflow log, or CI failure triage. Use `gh-fix-ci`. |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The skill tells the reader to hand off common GitHub Actions requests to other skills that are not installed anywhere under this repo’s .agents/skills tree (gh-fix-ci, github, codeql, dependabot). That makes the routing guidance unusable in this environment: a user asking about a failing PR check or CodeQL/Dependabot config will be sent to a dead end instead of getting help. Update these lines to point to skills that actually exist in this repo, or make the routing conditional with a fallback to handle the request here.
# .agents/skills/github-actions-docs/SKILL.md
- A specific failing PR check, missing workflow log, or CI failure triage. Use `gh-fix-ci`.
- General GitHub pull request, branch, or repository operations. Use `github`.
- CodeQL-specific configuration or code scanning guidance. Use `codeql`.
- Dependabot configuration, grouping, or dependency update strategy. Use `dependabot`.| │ └─ Fails/Incomplete → Treat as dynamic (below) | ||
| │ | ||
| └─ No (dynamic webapp) → Is the server already running? | ||
| ├─ No → Run: python scripts/with_server.py --help |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The skill’s primary dynamic-app workflow tells the reader to run python scripts/with_server.py --help, but this repo does not ship that helper at repo-root scripts/with_server.py; the actual file is checked in as .agents/skills/webapp-testing/scripts/with_server.py. Following the documented command from the normal repo root therefore fails before testing can begin, which breaks the main happy path for this skill. Point the docs at the shipped helper location or explain that the command must be run relative to the skill directory.
# .agents/skills/webapp-testing/SKILL.md
└─ No → Run: python scripts/with_server.py --help
Then use the helper + write simplified Playwright script| setResults(results) | ||
|
|
||
| // Defer non-critical work to idle periods | ||
| requestIdleCallback(() => { |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The rule’s main “Correct” example uses requestIdleCallback() directly even though MDN marks the API as limited-availability and unsupported in some widely used browsers. Because this skill is meant to drive automated refactoring/code generation, agents are likely to copy the first “correct” snippet and ship code that throws at runtime where window.requestIdleCallback is undefined; the fallback should be part of the primary recommendation, not an optional appendix.
// .agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md
requestIdleCallback(() => {
analytics.track('search', { query })
})Use a guarded helper (feature detection + fallback) in the main example so the generated code is safe across browsers.
1 participant
Added from Capy skills marketplace:
vercel-labs/skillsanthropics/skillsvercel-labs/agent-skillsvercel-labs/agent-skillsvercel-labs/agent-browsershadcn/uiobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersobra/superpowersanthropics/skillsanthropics/skillsxixu-me/skills