clean up skills#912
Conversation
📝 WalkthroughWalkthroughThis PR reorganizes and expands the agent-relay skill documentation ecosystem by removing the generic headless-orchestrator skill, splitting its guidance into explicit orchestrator and participant skills in both ChangesAgent-Relay Skill Documentation Reorganization
Manifest Configuration Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .step('implementation-reconcile', { | ||
| type: 'deterministic', | ||
| dependsOn: ['runtime-implementation', 'adapter-implementation'], | ||
| dependsOn: ['context'], |
There was a problem hiding this comment.
🔴 Example implementation-reconcile step lost its dependency on implementation agents, creating a race condition
The dependsOn for the implementation-reconcile step was changed from ['runtime-implementation', 'adapter-implementation'] to ['context']. Since both implementation agents also depend on ['context'], this makes the deterministic reconcile step (which runs git status and test -f checks for files the agents create) start in parallel with the implementation agents instead of after them. The reconcile check finishes almost instantly and will always report files as missing because the agents haven't produced output yet. The repair agent then starts while implementations are still in progress, and E2E tests (run-e2e) run before any implementation work completes.
The section's intent ("Agent Transport Must Not Be The First Hard Gate") is that transport failures shouldn't crash the workflow, but the correct pattern would keep the dependency on agent steps (so the reconcile waits for them) while making agent steps non-fatal — not remove the ordering constraint entirely. The identical incorrect change appears in .agents/skills/writing-agent-relay-workflows/SKILL.md:1303.
| dependsOn: ['context'], | |
| dependsOn: ['runtime-implementation', 'adapter-implementation'], |
Was this helpful? React with 👍 or 👎 to provide feedback.
| .step('implementation-reconcile', { | ||
| type: 'deterministic', | ||
| dependsOn: ['runtime-implementation', 'adapter-implementation'], | ||
| dependsOn: ['context'], |
There was a problem hiding this comment.
🔴 Same broken dependsOn in the .agents/ copy of the workflow skill
The .agents/ copy of the workflow skill has the same incorrect dependsOn: ['context'] change as the .claude/ copy. The implementation-reconcile step should depend on ['runtime-implementation', 'adapter-implementation'] so it runs after the implementation agents produce files, not in parallel with them.
| dependsOn: ['context'], | |
| dependsOn: ['runtime-implementation', 'adapter-implementation'], |
Was this helpful? React with 👍 or 👎 to provide feedback.
| | `pattern('single')` on cloud runner | Not supported — use `dag` | | ||
| | `pattern('supervisor')` with one agent | Same agent is owner + specialist. Use `dag` | | ||
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, and `pr_url` are valid | | ||
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | |
There was a problem hiding this comment.
🟡 Common Mistakes table drops pr_url from valid verification types, contradicting the Verification Gates section in the same file
The Common Mistakes table row for invalid verification types was changed to list only four valid types: exit_code, output_contains, file_exists, custom — dropping pr_url. However, in the same file at .claude/skills/writing-agent-relay-workflows/SKILL.md:1096, the text explicitly states: "Only these five types are valid: exit_code, output_contains, file_exists, custom, pr_url." The code example at line 1093 still shows pr_url, and lines 1098 contain detailed usage guidance for pr_url. This contradictory guidance will confuse workflow authors — one section says pr_url is valid and explains how to use it, while the mistakes table tells them it's invalid.
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | | |
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, and `pr_url` are valid | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| | Thinking `agent-relay run` inspects exports | It executes the file as a subprocess. Only `.run()` invocations trigger steps | | ||
| | `pattern('single')` on cloud runner | Not supported — use `dag` | | ||
| | `pattern('supervisor')` with one agent | Same agent is owner + specialist. Use `dag` | | ||
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | |
There was a problem hiding this comment.
🟡 Same pr_url inconsistency in the .agents/ copy — code example shows pr_url but mistakes table excludes it
The .agents/ version of the workflow skill still shows verification: { type: 'pr_url', value: 'owner/repo' } in the code example at .agents/skills/writing-agent-relay-workflows/SKILL.md:710, but the Common Mistakes table at line 1434 was changed to omit pr_url from the valid types list. The explanatory paragraph that previously documented pr_url (between lines 712-715 in the old version) was also removed, leaving an undocumented but still-shown verification type.
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | | |
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, and `pr_url` are valid | |
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/skills/orchestrating-agent-relay/SKILL.md (1)
267-279:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate sections at the end of the file.
The file contains duplicate "Overview" and "Prerequisites" sections at the end (lines 267-279) that appear to be leftover content from a refactor. These topics are already covered earlier in the document and should be removed to avoid confusion.
🧹 Proposed fix
-### Overview - -Self-bootstrap agent-relay infrastructure and manage a team of agents autonomously. - -### Prerequisites - -#### 1. **agent-relay CLI installed** (required) - -```bash -npm install -g agent-relay - # Or use npx without installing: npx agent-relay <command> -``` -🤖 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 @.agents/skills/orchestrating-agent-relay/SKILL.md around lines 267 - 279, Remove the duplicated tail content that repeats the "### Overview" and "### Prerequisites" sections (including the npm install / npx code block) so only the original earlier occurrences remain; locate the duplicate headings "Overview" and "Prerequisites" near the end of SKILL.md and delete that entire repeated block (the repeated headings plus the fenced code block) to avoid redundancy.
🧹 Nitpick comments (4)
.agents/skills/review-fix-signoff-loop/SKILL.md (2)
17-17: ⚡ Quick winClarify "expected token" language.
The phrase "if it cannot return the expected token" is ambiguous in the context of a
--jsoncommand. Consider clarifying what constitutes success—perhaps "if it cannot return valid JSON output" or "if authentication fails" would be clearer for workflow authors implementing this preflight check.🤖 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 @.agents/skills/review-fix-signoff-loop/SKILL.md at line 17, Clarify the ambiguous "expected token" wording by specifying the success criteria for the preflight check: update the sentence referencing `codex login status` and `codex exec --ephemeral --json --sandbox read-only -m <supported-model>` to say the command must produce valid JSON output that includes a valid authentication token (e.g., a specific "token" field) and otherwise treat it as an authentication failure; instruct workflow authors to fail early with a clear re-login message when the command either returns non-JSON output, malformed JSON, or JSON that lacks the expected token field (or contains an explicit auth error).
134-134: ⚡ Quick winAdd trailing newline.
The file is missing a trailing newline at the end. POSIX standards and most style guides recommend files end with a newline character.
🤖 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 @.agents/skills/review-fix-signoff-loop/SKILL.md at line 134, The file SKILL.md is missing a trailing newline; open SKILL.md and add a single newline character at the end of the file (ensure the final line ends with '\n') so the file ends with a trailing newline per POSIX/style guidelines..claude/skills/review-fix-signoff-loop/SKILL.md (2)
19-19: ⚡ Quick winClarify "expected token" language.
The phrase "if it cannot return the expected token" is ambiguous in the context of a
--jsoncommand. Consider clarifying what constitutes success—perhaps "if it cannot return valid JSON output" or "if authentication fails" would be clearer for workflow authors implementing this preflight check.🤖 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 @.claude/skills/review-fix-signoff-loop/SKILL.md at line 19, The wording "if it cannot return the expected token" is ambiguous; update the sentence that describes the Codex preflight check to explicitly state the success criteria for the `codex exec --ephemeral --json --sandbox read-only -m <supported-model>` command: require that the command returns valid JSON and contains the authentication field(s) you expect (for example a token key or an explicit authenticated status), and fail early with a clear re-login instruction if the output is not valid JSON or indicates authentication failure (e.g., missing token key or an auth=false flag) instead of using the vague phrase "expected token".
203-203: ⚡ Quick winAdd trailing newline.
The file is missing a trailing newline at the end. POSIX standards and most style guides recommend files end with a newline character.
🤖 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 @.claude/skills/review-fix-signoff-loop/SKILL.md at line 203, The file SKILL.md is missing a trailing newline; open SKILL.md and ensure the file ends with a single newline character (add a final '\n' at EOF), then save and commit the change so the file ends with a newline per POSIX/style guidelines.
🤖 Prompt for all review comments with 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.
Inline comments:
In @.agents/skills/setting-up-relayfile/SKILL.md:
- Around line 42-43: Remove the malformed markdown headers by replacing the
literal sequence "#### ```bash" with a proper code fence "```bash" so the blocks
render as code; update both occurrences referenced by the snippet "#### ```bash"
(around the relayfile setup and relayfile status examples) to remove the "#### "
prefix so each block begins with "```bash" and ends with "```".
In @.agents/skills/writing-agent-relay-workflows/SKILL.md:
- Line 1288: The markdown heading "Interactive lead-and-worker teams are useful,
but they are still process" is truncated; update the heading to the full
sentence from the .claude version (e.g., "Interactive lead-and-worker teams are
useful, but they are still process sessions. A long-running PTY can go idle...")
so the section reads coherently; locate the same heading text in SKILL.md and
replace the incomplete line with the full explanatory text (or copy the
corresponding paragraph from
`.claude/skills/writing-agent-relay-workflows/SKILL.md`) to restore the intended
content.
In @.claude/skills/writing-agent-relay-workflows/SKILL.md:
- Line 2019: The Common Mistakes table list of valid verification types is
inconsistent with the Verification Gates section and examples that include
pr_url; update the Common Mistakes entry so it lists all five valid types:
exit_code, output_contains, file_exists, custom, pr_url (or alternatively remove
pr_url from the Verification Gates section if you intend only four types),
ensuring the term "pr_url" appears alongside exit_code, output_contains,
file_exists, and custom in the Common Mistakes text to match the documented
examples and the Verification Gates section.
---
Outside diff comments:
In @.agents/skills/orchestrating-agent-relay/SKILL.md:
- Around line 267-279: Remove the duplicated tail content that repeats the "###
Overview" and "### Prerequisites" sections (including the npm install / npx code
block) so only the original earlier occurrences remain; locate the duplicate
headings "Overview" and "Prerequisites" near the end of SKILL.md and delete that
entire repeated block (the repeated headings plus the fenced code block) to
avoid redundancy.
---
Nitpick comments:
In @.agents/skills/review-fix-signoff-loop/SKILL.md:
- Line 17: Clarify the ambiguous "expected token" wording by specifying the
success criteria for the preflight check: update the sentence referencing `codex
login status` and `codex exec --ephemeral --json --sandbox read-only -m
<supported-model>` to say the command must produce valid JSON output that
includes a valid authentication token (e.g., a specific "token" field) and
otherwise treat it as an authentication failure; instruct workflow authors to
fail early with a clear re-login message when the command either returns
non-JSON output, malformed JSON, or JSON that lacks the expected token field (or
contains an explicit auth error).
- Line 134: The file SKILL.md is missing a trailing newline; open SKILL.md and
add a single newline character at the end of the file (ensure the final line
ends with '\n') so the file ends with a trailing newline per POSIX/style
guidelines.
In @.claude/skills/review-fix-signoff-loop/SKILL.md:
- Line 19: The wording "if it cannot return the expected token" is ambiguous;
update the sentence that describes the Codex preflight check to explicitly state
the success criteria for the `codex exec --ephemeral --json --sandbox read-only
-m <supported-model>` command: require that the command returns valid JSON and
contains the authentication field(s) you expect (for example a token key or an
explicit authenticated status), and fail early with a clear re-login instruction
if the output is not valid JSON or indicates authentication failure (e.g.,
missing token key or an auth=false flag) instead of using the vague phrase
"expected token".
- Line 203: The file SKILL.md is missing a trailing newline; open SKILL.md and
ensure the file ends with a single newline character (add a final '\n' at EOF),
then save and commit the change so the file ends with a newline per POSIX/style
guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b0b5c45-fc7a-44de-a009-9577c17be787
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonprpm.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.agents/skills/orchestrating-agent-relay/SKILL.md.agents/skills/review-fix-signoff-loop/SKILL.md.agents/skills/setting-up-relayfile/SKILL.md.agents/skills/using-agent-relay/SKILL.md.agents/skills/writing-agent-relay-workflows/SKILL.md.claude/skills/orchestrating-agent-relay/SKILL.md.claude/skills/review-fix-signoff-loop/SKILL.md.claude/skills/running-headless-orchestrator/SKILL.md.claude/skills/setting-up-relayfile/SKILL.md.claude/skills/using-agent-relay/SKILL.md.claude/skills/writing-agent-relay-workflows/SKILL.mdprpm.json
💤 Files with no reviewable changes (1)
- .claude/skills/running-headless-orchestrator/SKILL.md
| #### ```bash | ||
|
|
There was a problem hiding this comment.
Fix malformed markdown headers before code blocks.
Lines 42-43 and 54-55 contain #### ```bash which creates an empty h4 header instead of starting a code block. Remove the #### prefix so the code fence markers work correctly.
📝 Proposed fix for markdown formatting
### Step 1 — Run setup (interactive happy path)
-#### ```bash
-
```bash
relayfile setup \
--provider notion \ ### Step 2 — Verify the mount is healthy
-#### ```bash
-
```bash
relayfile status my-agentAlso applies to: 54-55
🤖 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 @.agents/skills/setting-up-relayfile/SKILL.md around lines 42 - 43, Remove
the malformed markdown headers by replacing the literal sequence "#### ```bash"
with a proper code fence "```bash" so the blocks render as code; update both
occurrences referenced by the snippet "#### ```bash" (around the relayfile setup
and relayfile status examples) to remove the "#### " prefix so each block begins
with "```bash" and ends with "```".
| without giving a repair owner command output to fix. For long rollouts, keep | ||
| the critical path evidence-based — gate on artifacts and deterministic checks, | ||
| not directly on the agent transport: | ||
| #### Interactive lead-and-worker teams are useful, but they are still process |
There was a problem hiding this comment.
Incomplete heading text.
The heading ends with "process" which is grammatically incomplete. Comparing with the corresponding section in .claude/skills/writing-agent-relay-workflows/SKILL.md (lines 1856-1857), the complete text should be "process sessions. A long-running PTY can go idle..." This appears to be a markdown formatting error where the sentence was cut off.
📝 Proposed fix
-#### Interactive lead-and-worker teams are useful, but they are still process
+#### Interactive lead-and-worker teams are useful, but they are still process sessionsOr incorporate the full explanatory text from the .claude version.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### Interactive lead-and-worker teams are useful, but they are still process | |
| #### Interactive lead-and-worker teams are useful, but they are still process sessions |
🤖 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 @.agents/skills/writing-agent-relay-workflows/SKILL.md at line 1288, The
markdown heading "Interactive lead-and-worker teams are useful, but they are
still process" is truncated; update the heading to the full sentence from the
.claude version (e.g., "Interactive lead-and-worker teams are useful, but they
are still process sessions. A long-running PTY can go idle...") so the section
reads coherently; locate the same heading text in SKILL.md and replace the
incomplete line with the full explanatory text (or copy the corresponding
paragraph from `.claude/skills/writing-agent-relay-workflows/SKILL.md`) to
restore the intended content.
| | `pattern('single')` on cloud runner | Not supported — use `dag` | | ||
| | `pattern('supervisor')` with one agent | Same agent is owner + specialist. Use `dag` | | ||
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, and `pr_url` are valid | | ||
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | |
There was a problem hiding this comment.
Inconsistent verification types list.
The Common Mistakes table states that only four verification types are valid: exit_code, output_contains, file_exists, custom. However, line 1096 in the "Verification Gates" section clearly lists five valid types, including pr_url:
"Only these five types are valid:
exit_code,output_contains,file_exists,custom,pr_url."
The pr_url verification type is also documented with usage examples at lines 1093-1099. This is an inconsistency within the same document.
📝 Proposed fix
-| Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid |
+| Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, `pr_url` are valid |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom` are valid | | |
| | Invalid verification type (`type: 'deterministic'`) | Only `exit_code`, `output_contains`, `file_exists`, `custom`, `pr_url` are valid | |
🤖 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 @.claude/skills/writing-agent-relay-workflows/SKILL.md at line 2019, The
Common Mistakes table list of valid verification types is inconsistent with the
Verification Gates section and examples that include pr_url; update the Common
Mistakes entry so it lists all five valid types: exit_code, output_contains,
file_exists, custom, pr_url (or alternatively remove pr_url from the
Verification Gates section if you intend only four types), ensuring the term
"pr_url" appears alongside exit_code, output_contains, file_exists, and custom
in the Common Mistakes text to match the documented examples and the
Verification Gates section.
1 participant
Summary
Test Plan
Screenshots