Skip to content

Add Factory's Droid CLI support#130

Closed
galangryandana wants to merge 4 commits intoobra:mainfrom
galangryandana:add-factory-droid-support
Closed

Add Factory's Droid CLI support#130
galangryandana wants to merge 4 commits intoobra:mainfrom
galangryandana:add-factory-droid-support

Conversation

@galangryandana
Copy link
Copy Markdown

@galangryandana galangryandana commented Nov 30, 2025

Summary
Add support for Factory's Droid CLI (https://docs.factory.ai/)

  • Add .factory/ directory with Droid CLI specific implementation
  • Add skills adjusted for Droid CLI (terminology, paths)
  • Add droids (general-purpose, code-reviewer, explore, plan)
  • Add commands (brainstorm, write-plan, execute-plan)
  • Add AGENTS.md protocol for context injection
  • Add install.sh for one-line installation
  • Add docs/README.factory.md with complete documentation

Motivation and Context

Factory's Droid CLI is an AI coding assistant similar to Claude Code, but
with some architectural differences. This PR enables Droid CLI users to
benefit from the Superpowers workflow system.

Why this is needed:

  • Droid CLI has a growing user base that could benefit from Superpowers
  • Droid CLI has native Skill and Task tools that map well to Superpowers
    concepts
  • The implementation required minimal changes - mostly terminology and path
    adjustments

Key differences from Claude Code:

Feature Claude Code Droid CLI
Skill loading Built-in /skill command Native Skill("name") tool
Subagents Built-in Task tool Native Task tool with droids
Context injection CLAUDE.md + hooks AGENTS.md (auto-loaded)
Config directory ~/.claude/ ~/.factory/

How Has This Been Tested?

Extensively tested on Droid CLI with multiple test scenarios:

Unit Tests (Individual Components)

Commands:

  • /brainstorm - triggers Skill("brainstorming") tool correctly
  • /write-plan - triggers Skill("writing-plans") tool correctly
  • /execute-plan - triggers Skill("executing-plans") tool correctly

Automatic Skill Detection:

  • "I want to build a todo app" → AI uses brainstorming skill
    automatically
  • "Help debug this error" → AI uses systematic-debugging skill
  • Protocol prevents AI from jumping directly to code

Droids:

  • general-purpose droid dispatches via Task tool
  • code-reviewer droid dispatches for code review
  • explore and plan droids available and functional

Installation:

  • One-line install script works: curl -fsSL .../install.sh | bash
  • Manual installation steps verified
  • Existing AGENTS.md preserved (appends instead of overwrites)
  • Installation with hooks disabled - system still works (verified hooks
    are not essential)

Full Workflow Test

Tested complete end-to-end workflow with prompt: "I want to create a simple
todo list application"

Step Skill/Action Status
1. Check existing docs AI searches docs/designs/, docs/plans/ first
✅ Pass
2. Brainstorming AI uses Skill("brainstorming"), asks questions ONE at
a time ✅ Pass
3. Design approval AI presents design in sections, waits for approval
✅ Pass
4. Writing plans AI uses Skill("writing-plans") after design approved
✅ Pass
5. Execution choice AI offers: Subagent-driven vs Parallel session
Pass
6. Task execution AI dispatches general-purpose droid per task
Pass
7. Code review AI dispatches code-reviewer droid between tasks
Pass
8. TDD enforcement AI follows RED-GREEN-REFACTOR during implementation
✅ Pass
9. Verification AI uses verification-before-completion before claiming
done ✅ Pass

Protocol Enforcement Tests

Scenario Expected Behavior Status
User asks to "just code it quickly" AI refuses, insists on brainstorming
first ✅ Pass
User asks to "add tests after" AI flags TDD violation, offers
alternatives ✅ Pass
AI tempted to skip skill Anti-rationalization rules prevent skipping
Pass
User asks for code review AI dispatches code-reviewer droid (doesn't
self-review) ✅ Pass

Edge Cases Tested

  • Fresh project (no existing docs) → starts brainstorming
  • Existing design doc found → AI asks: continue/fresh/update?
  • Multiple independent failures → AI considers
    dispatching-parallel-agents
  • Flaky tests reported → AI uses condition-based-waiting skill
  • Session restart → AGENTS.md reloads, protocol still active

Negative Tests (What Should NOT Work)

  • Hooks disabled (enableHooks: false) → System still works (confirmed
    hooks not essential)
  • Without AGENTS.md → Protocol not enforced (as expected)
  • Without skills folder → Skills not available (as expected)

Breaking Changes

None. This PR only adds new files in .factory/ directory and
docs/README.factory.md. No existing files are modified.

Users of other platforms (Claude Code, Codex, OpenCode) are not affected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality
    to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Skills Adjustments Made

The skills in .factory/skills/ are copies of the original skills with these
adjustments for Droid CLI compatibility:

  1. Reference format changes:

    • superpowers:skill-name`skill-name` skill
    • superpowers:code-reviewer`code-reviewer` droid
  2. Path changes:

    • ~/.claude/~/.factory/
    • CLAUDE.mdAGENTS.md
  3. Terminology:

    • "subagent" retained for methodology concepts (e.g.,
      subagent-driven-development skill name)
    • "droid" used for dispatched workers via Task tool

Architecture Notes

Droid CLI's native tool support made integration straightforward:

  • No CLI scripts needed - Unlike Codex which requires superpowers-codex
    script, Droid CLI's native Skill tool handles skill loading directly
  • No hooks needed - Droid CLI auto-loads AGENTS.md into context each
    session (hooks were tested but output doesn't inject into AI context)
  • Native Task tool - Maps directly to subagent dispatching with custom
    droids

File Structure

.factory/
├── AGENTS.md # Protocol (auto-loaded by Droid CLI)
├── INSTALL.md # Installation guide
├── install.sh # One-line installer
├── commands/ # Slash commands
│ ├── brainstorm.md
│ ├── write-plan.md
│ └── execute-plan.md
├── droids/ # Droid definitions for Task tool
│ ├── general-purpose.md
│ ├── code-reviewer.md
│ ├── explore.md
│ └── plan.md
└── skills/ # 20 skills (adjusted for Droid CLI)
├── brainstorming/
├── test-driven-development/
├── systematic-debugging/
└── ... (17 more)

docs/
└── README.factory.md # Complete documentation

Installation

Users can install with one command:

curl -fsSL
https://raw.githubusercontent.com/obra/superpowers/main/.factory/install.sh |
 bash
Or via Droid CLI:

Fetch and follow instructions from
https://raw.githubusercontent.com/obra/superpowers/main/.factory/INSTALL.md


---
Summary by CodeRabbit
Release Notes
New Features

Introduced Superpowers protocol for Factory Droid CLI with automated agent workflows
Added intelligent agent droids for code review, exploration, planning, and general-purpose development tasks
New one-line installation script and comprehensive setup guide
Automated commands for brainstorming, planning, and executing development tasks
Documentation

Extensive guides for development workflows, debugging patterns, and testing best practices
Complete integration documentation for Factory and Droid CLI
Troubleshooting and verification resources

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **New Features**
 * Superpowers protocol, coordinated agent roles, and many reusable process skills (planning, TDD, debugging, code-review, subagent workflows).
 * Installer/automation to set up the Superpowers integration and assets.

* **Documentation**
 * Extensive guides for skills, droids, commands, usage, integration, and skill-authoring best practices.

* **Tests**
 * New pressure-test scenarios and testing templates for validating skills and workflows.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

galang ryandana added 2 commits November 30, 2025 09:59
     - Add .factory/ directory with Droid CLI specific implementation
     - Add skills adjusted for Droid CLI (terminology, paths)
     - Add droids (general-purpose, code-reviewer, explore, plan)
     - Add commands (brainstorm, write-plan, execute-plan)
     - Add AGENTS.md protocol for context injection
     - Add install.sh for one-line installation
     - Add docs/README.factory.md with complete documentation
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 30, 2025

Walkthrough

Adds a comprehensive "Superpowers" protocol and installer to the Droid CLI Factory: many new skill, droid, and command documents, a Bash installer, a docs integration guide, a TypeScript wait helper, and a bash bisector script; primarily documentation and workflow artifacts with three executable utilities.

Changes

Cohort / File(s) Summary
Protocol & Installer
.factory/AGENTS.md, .factory/INSTALL.md, .factory/install.sh
New Superpowers protocol (tool/skill mappings, pre-response and subagent rules), installation guide, and installer script that clones/updates the Superpowers repo and installs skills/droids/commands and AGENTS.md (with backup/append behavior).
Commands
.factory/commands/brainstorm.md, .factory/commands/execute-plan.md, .factory/commands/write-plan.md
New command documentation for brainstorming, executing plans in batches with review checkpoints, and writing plans (how to load/use corresponding skills).
Droid Profiles
.factory/droids/general-purpose.md, .factory/droids/explore.md, .factory/droids/code-reviewer.md, .factory/droids/plan.md
New droid role templates: general-purpose workflow, read-only explorer, code-reviewer rubric, and planning/research droid definitions and protocols.
Core Skills & Policies
.factory/skills/using-superpowers/SKILL.md, .factory/skills/writing-skills/*, .factory/skills/writing-plans/SKILL.md, .factory/skills/verification-before-completion/SKILL.md, .factory/skills/test-driven-development/SKILL.md, .factory/skills/using-git-worktrees/SKILL.md, .factory/skills/defense-in-depth/SKILL.md
Numerous SKILL.md docs formalizing mandatory skill-discovery/use, skill authoring best practices, plan-writing, verification gates, TDD-first rules, git worktree workflows, and layered validation patterns.
Execution & Coordination Skills
.factory/skills/executing-plans/SKILL.md, .factory/skills/finishing-a-development-branch/SKILL.md, .factory/skills/subagent-driven-development/SKILL.md, .factory/skills/dispatching-parallel-agents/SKILL.md
New skills for batched plan execution with verification, finishing development branches (merge/PR/cleanup options), subagent-driven development workflows, and parallel-agent dispatch patterns.
Code Review & Feedback
.factory/skills/requesting-code-review/*, .factory/skills/receiving-code-review/SKILL.md, .factory/skills/requesting-code-review/code-reviewer.md
Templates and workflows for requesting, dispatching, and receiving structured code reviews, including a code-reviewer agent template and response protocol.
Testing & Debugging
.factory/skills/systematic-debugging/*, .factory/skills/root-cause-tracing/*, .factory/skills/testing-skills-with-subagents/*, .factory/skills/condition-based-waiting/*
Systematic debugging framework, root-cause tracing docs (plus bisector script), testing-with-subagents guides and pressure tests, and condition-based waiting documentation.
Utilities / Executables
.factory/skills/condition-based-waiting/example.ts, .factory/skills/root-cause-tracing/find-polluter.sh
Added TypeScript condition-based wait helper exports (three polling helpers) and a bash bisector script to find polluting tests.
Meta & Integration Docs
docs/README.factory.md, .factory/INSTALL.md
Integration/usage guide describing installation paths, verification, skill loading, tool mappings, droid dispatch, and troubleshooting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Mixed-volume docs with a few executable items requiring focused checks.
  • Files to inspect closely:
    • .factory/install.sh — idempotency, path handling, permission and backup logic.
    • .factory/AGENTS.md — accuracy of tool mappings and mandatory protocol language.
    • .factory/skills/condition-based-waiting/example.ts — polling interval, timeout and rejection semantics.
    • .factory/skills/root-cause-tracing/find-polluter.sh — argument validation, test invocation safety, and environment assumptions.

Possibly related issues

Poem

🐇
I hopped through folders, stitched each plan,
Left skills and scripts with a careful hand.
Brainstormed, tested, dispatched with cheer,
TDD and reviews — the path is clear.
Hop, commit, merge — a branch held dear. 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Factory's Droid CLI support' clearly and concisely summarizes the main change: introducing support for Factory's Droid CLI through new .factory/ directory, documentation, and related integrations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (26)
.factory/skills/test-driven-development/SKILL.md (1)

254-254: Clarify grammar at line 254.

The phrase "lose proof tests work" is unclear. Consider: "lose proof that tests work" or "lose evidence that tests are effective."

.factory/skills/subagent-driven-development/SKILL.md (1)

41-57: Add language specifiers to all fenced code blocks.

The skill documentation is well-structured and clear, but multiple code blocks lack language identifiers for markdown linting compliance and better rendering (especially bash and YAML examples).

Apply this diff to lines 41-57 (add yaml language specifier):

-```
+```yaml

Apply this diff to lines 64-73 (add yaml language specifier):

-```
+```yaml

Apply this diff to lines 85-87 (add yaml language specifier):

-```
+```yaml

Apply this diff to lines 111-146 (add text language specifier for the example workflow):

-```
+```text

The same pattern applies to other files in this review (code-reviewer.md templates, using-git-worktrees/SKILL.md, and receiving-code-review/SKILL.md). Consider adopting a linting step or pre-commit hook to enforce MD040 consistency across the repository.

Also applies to: 64-73, 85-87, 111-146

.factory/skills/requesting-code-review/code-reviewer.md (1)

25-28: Add language specifiers to fenced code blocks.

The template is comprehensive and the example output is well-structured. However, lines 25-28 (bash commands) and the example output block (lines 112-146) lack language identifiers.

Apply this diff to lines 25-28 (add bash language specifier):

-```
+```bash

Apply this diff to line 112 (add text or markdown language specifier for the example):

-```
+```text

Also applies to: 112-146

.factory/droids/code-reviewer.md (1)

37-54: Add language specifier to fenced code block output format.

The output format example (lines 37-54) lacks a language identifier for markdown linting compliance.

Apply this diff:

-```
+```text
.factory/skills/brainstorming/SKILL.md (1)

19-19: Consider hyphenating "multiple choice" as an adjective.

Line 19 uses "multiple choice questions" where the hyphenated form "multiple-choice questions" is more formally correct when the term acts as a compound adjective before the noun.

Apply this diff:

-- Prefer multiple choice questions when possible, but open-ended is fine too
+- Prefer multiple-choice questions when possible, but open-ended is fine too

(This is a minor grammatical improvement; either form is widely understood.)

.factory/skills/using-git-worktrees/SKILL.md (1)

22-26: Add language specifiers to all bash code blocks.

The skill documentation is thorough and well-structured, but multiple bash code blocks lack language identifiers for proper markdown rendering.

Apply this diff to all bash code blocks (lines 22, 32, 42, 79, 87, 106, 125, 138, 176). Example for line 22:

-```
+```bash

Repeat this pattern for all bash code blocks in the file. The overall code block formatting consistency issue spans multiple files in this PR (subagent-driven-development/SKILL.md, requesting-code-review/code-reviewer.md, droids/code-reviewer.md, receiving-code-review/SKILL.md) and could benefit from a pre-commit hook or linting step to enforce MD040 compliance.

Also applies to: 32-36, 42-49, 79-99, 106-118, 125-130, 138-143, 176-188

.factory/skills/receiving-code-review/SKILL.md (1)

112-112: Add language specifier to fenced code block.

Line 112 contains a code block without a language identifier.

Apply this diff:

-```
+```text
.factory/skills/root-cause-tracing/SKILL.md (1)

40-42: Add language identifier to error snippet fence for markdownlint compliance

markdownlint’s MD040 warning at this block is valid; you can keep behavior identical and satisfy the linter by tagging the fence as plain text:

-```
+```text
 Error: git init failed in /Users/galangryandana/project/packages/core
-```
+```
.factory/skills/sharing-skills/SKILL.md (1)

164-180: Optional: Align troubleshooting section with markdownlint (headings + links)

If markdownlint is enforced, the troubleshooting bullets can be tweaked to satisfy MD036/MD034 without changing meaning. For example:

-**"gh: command not found"**
-- Install GitHub CLI: https://cli.github.com/
-- Authenticate: `gh auth login`
+### gh: command not found
+- Install GitHub CLI: <https://cli.github.com/>
+- Authenticate: `gh auth login`

You can apply the same pattern to the other emphasized error labels and bare URLs in this section.

.factory/skills/using-superpowers/SKILL.md (1)

94-99: Renumber “Starting any task” steps for readability

The list under “Starting any task” currently shows 1., 3., 4. which can be mildly confusing even though Markdown renders it fine. Consider renumbering for clarity:

-1. If relevant skill exists → Use the skill
-3. Announce you're using it
-4. Follow what it says
+1. If relevant skill exists → Use the skill
+2. Announce you're using it
+3. Follow what it says
.factory/skills/writing-skills/SKILL.md (1)

74-80: Label plain-text fenced blocks to satisfy markdownlint MD040

Several plain-text fences (directory trees and the iron-law motto) have no language, which triggers MD040. You can keep semantics identical by marking them as text, e.g.:

-```
+```text
 skills/
   skill-name/
     SKILL.md              # Main reference (required)
     supporting-file.*     # Only if needed

And similarly for the later directory-layout examples and:

```diff
-```
-NO SKILL WITHOUT A FAILING TEST FIRST
-```
+```text
+NO SKILL WITHOUT A FAILING TEST FIRST
+```

Also applies to: 317-345

.factory/skills/condition-based-waiting/example.ts (1)

20-135: Condition-based waiting helpers look correct; consider optional tunable poll interval

The three wait helpers implement a straightforward polling loop with clear timeout behavior and no obvious race/double-resolve issues. This should be a solid foundation for de-flaking tests.

If these start getting used outside tight test contexts, you might consider an optional pollIntervalMs parameter (defaulting to 10ms) so callers can trade off responsiveness vs. overhead, but that’s not required for this PR.

.factory/skills/systematic-debugging/SKILL.md (2)

18-18: Add language specifier to fenced code blocks.

Markdown lint flags these code blocks as missing language specifiers. Line 18 should be ```text or similar, and line 77 should be ```bash.

- ```
+ ```text
  NO FIXES WITHOUT ROOT CAUSE INVESTIGATION FIRST

And:

```diff
- ```
+ ```bash
  # Layer 1: Workflow
  echo "=== Secrets available in workflow: ==="

Also applies to: 77-77


234-234: Clarify awkward possessive phrasing.

Line 234 reads "your human partner's Signals" which is awkward. Consider: "your partner's signals" or "signals from your partner."

.factory/skills/systematic-debugging/test-pressure-2.md (1)

12-15: Add language specifier to fenced code block.

Line 12 has a code block without language specified. Should be ```javascript or ```text.

- ```
+ ```javascript
  Expected: { status: 'completed', amount: 100 }
  Received: { status: 'pending', amount: 100 }

</blockquote></details>
<details>
<summary>.factory/skills/writing-plans/SKILL.md (2)</summary><blockquote>

`88-88`: **Add language specifier to fenced code block.**

Line 88 has a code block without language specified. Should be `` ```bash `` since it shows bash commands.



```diff
- ```
+ ```bash
  git add tests/path/test.py src/path/file.py
  git commit -m "feat: add specific feature"

---

`65-65`: **Consider using heading syntax for steps instead of bold emphasis.**

Markdown lint prefers headings (` ### `) over bold emphasis (`**...**`) for section headers. Lines 65, 70, 77, 82 all use bold for step headers. While this works in a template example, converting to proper heading syntax would improve markdown compliance:

```diff
- **Step 2: Run test to verify it fails**
+ ### Step 2: Run test to verify it fails

This makes the template more semantically correct and passes markdown linting.

Also applies to: 70-70, 77-77, 82-82

docs/README.factory.md (2)

251-253: Wrap bare URLs in markdown link syntax.

Bare URLs should be converted to markdown links for better formatting and accessibility.

Apply this diff:

-## Getting Help
-
-- Report issues: https://github.com/obra/superpowers/issues
-- Factory docs: https://docs.factory.ai/
-- Superpowers blog: https://blog.fsck.com/2025/10/09/superpowers/
+## Getting Help
+
+- Report issues: [GitHub Issues](https://github.com/obra/superpowers/issues)
+- Factory docs: [Factory documentation](https://docs.factory.ai/)
+- Superpowers blog: [Superpowers blog post](https://blog.fsck.com/2025/10/09/superpowers/)

26-26: Specify language identifiers for code blocks.

Add language identifiers to all fenced code blocks for syntax highlighting and clarity.

Apply these diffs:

 ## One-Line Install (Recommended)
 
-```bash
+```bash

All flagged blocks need language specs: bash, text, or markdown as appropriate. For example:

-```
+```bash
 curl -fsSL https://raw.githubusercontent.com/obra/superpowers/main/.factory/install.sh | bash
-```
+```

Lines affected: 26 (bash), 67 (text), 75 (bash), 89 (text), 110 (text), 143 (text).

Also applies to: 67-67, 75-75, 89-89, 110-110, 143-143

.factory/skills/testing-skills-with-subagents/examples/CLAUDE_MD_TESTING.md (1)

8-8: Specify language identifiers for code blocks.

Add language identifiers to fenced code blocks (lines 8, 26, 70, 89). These are markdown or text examples and should be labeled accordingly.

Example:

-```
+```markdown
 IMPORTANT: This is a real scenario. Choose and act.
-```
+```

Also applies to: 26-26, 70-70, 89-89

.factory/skills/writing-skills/anthropic-best-practices.md (2)

149-150: Fix list indentation consistency.

Adjust indentation for nested list items to align with markdownlint standards (0-space indent for top-level items).

Example on lines 149-150:

-* The [context window](/en/docs/build-with-claude/context-windows) is a public good. Your Skill shares the context window with everything else Claude needs to know, including:
+* The [context window](/en/docs/build-with-claude/context-windows) is a public good. Your Skill shares the context window with everything else Claude needs to know, including:

-  * The system prompt
-  * Conversation history
+* The system prompt
+* Conversation history

(Remove the 2-space indent from nested items.)

Also applies to: 192-194


257-257: Specify language identifiers for code blocks.

Add language identifiers to fenced code blocks on lines 257, 301, and 1040 for consistency and proper syntax highlighting.

Examples:

-```
+```
 pdf/
 ├── SKILL.md              # Main instructions (loaded when triggered)
-```
+```

(Line 257: specify text or remove fence; lines 301 and 1040: specify text for directory trees or bash for commands as appropriate.)

Also applies to: 301-301, 1040-1040

.factory/AGENTS.md (2)

3-6: Clarify the XML-like <SYSTEM_OVERRIDE> tag format.

This tag uses XML syntax within a Markdown document, which is non-standard. Clarify whether:

  1. Droid CLI parses this specially as a directive override, or
  2. It should be commented out or formatted differently (e.g., as a Markdown code block or blockquote)

If parsed by Droid CLI, document this parsing expectation; otherwise, use standard Markdown formatting.


52-52: Add language specifications to fenced code blocks.

Lines 52 and 66 contain fenced code blocks without language identifiers, which violates the MD040 rule. Add language tags for syntax highlighting and clarity.

Apply this diff to add language specifications:

-```
+```bash

Example for line 52:

 | Shell commands | `Execute` tool |

-```
+```bash
 Task(subagent_type: "general-purpose", description: "Implement Task 1", prompt: "...")
-```
+```

And similarly for line 66 (the workflow chain diagram).

Also applies to: 66-66

.factory/skills/finishing-a-development-branch/SKILL.md (2)

40-47: Consider enhancing base branch detection fallback.

The current logic tries main then master. For repositories using other naming conventions (e.g., develop, staging, or custom names), this may fail. Consider:

  1. Adding a fallback prompt if neither is detected, or
  2. Using git config --get init.defaultBranch as an additional check, or
  3. Asking the user to confirm the base branch explicitly.

This would make the skill more robust across diverse workflows.


96-104: PR creation via gh CLI assumes GitHub environment.

The gh pr create command assumes:

  1. GitHub CLI is installed and authenticated
  2. The repository is hosted on GitHub
  3. Appropriate permissions exist to create PRs

For teams using GitLab, Gitea, or other platforms, this step will fail silently or with an unclear error. Consider:

  1. Adding a note about the GitHub dependency, or
  2. Wrapping in a platform check (e.g., git config --get remote.origin.url | grep github), or
  3. Deferring to Droid CLI's native PR creation tool if available.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 515c86f and 1b755a2.

⛔ Files ignored due to path filters (2)
  • .DS_Store is excluded by !**/.DS_Store
  • .factory/skills/writing-skills/graphviz-conventions.dot is excluded by !**/*.dot
📒 Files selected for processing (42)
  • .factory/AGENTS.md (1 hunks)
  • .factory/INSTALL.md (1 hunks)
  • .factory/commands/brainstorm.md (1 hunks)
  • .factory/commands/execute-plan.md (1 hunks)
  • .factory/commands/write-plan.md (1 hunks)
  • .factory/droids/code-reviewer.md (1 hunks)
  • .factory/droids/explore.md (1 hunks)
  • .factory/droids/general-purpose.md (1 hunks)
  • .factory/droids/plan.md (1 hunks)
  • .factory/install.sh (1 hunks)
  • .factory/skills/brainstorming/SKILL.md (1 hunks)
  • .factory/skills/condition-based-waiting/SKILL.md (1 hunks)
  • .factory/skills/condition-based-waiting/example.ts (1 hunks)
  • .factory/skills/defense-in-depth/SKILL.md (1 hunks)
  • .factory/skills/dispatching-parallel-agents/SKILL.md (1 hunks)
  • .factory/skills/executing-plans/SKILL.md (1 hunks)
  • .factory/skills/finishing-a-development-branch/SKILL.md (1 hunks)
  • .factory/skills/receiving-code-review/SKILL.md (1 hunks)
  • .factory/skills/requesting-code-review/SKILL.md (1 hunks)
  • .factory/skills/requesting-code-review/code-reviewer.md (1 hunks)
  • .factory/skills/root-cause-tracing/SKILL.md (1 hunks)
  • .factory/skills/root-cause-tracing/find-polluter.sh (1 hunks)
  • .factory/skills/sharing-skills/SKILL.md (1 hunks)
  • .factory/skills/subagent-driven-development/SKILL.md (1 hunks)
  • .factory/skills/systematic-debugging/CREATION-LOG.md (1 hunks)
  • .factory/skills/systematic-debugging/SKILL.md (1 hunks)
  • .factory/skills/systematic-debugging/test-academic.md (1 hunks)
  • .factory/skills/systematic-debugging/test-pressure-1.md (1 hunks)
  • .factory/skills/systematic-debugging/test-pressure-2.md (1 hunks)
  • .factory/skills/systematic-debugging/test-pressure-3.md (1 hunks)
  • .factory/skills/test-driven-development/SKILL.md (1 hunks)
  • .factory/skills/testing-anti-patterns/SKILL.md (1 hunks)
  • .factory/skills/testing-skills-with-subagents/SKILL.md (1 hunks)
  • .factory/skills/testing-skills-with-subagents/examples/CLAUDE_MD_TESTING.md (1 hunks)
  • .factory/skills/using-git-worktrees/SKILL.md (1 hunks)
  • .factory/skills/using-superpowers/SKILL.md (1 hunks)
  • .factory/skills/verification-before-completion/SKILL.md (1 hunks)
  • .factory/skills/writing-plans/SKILL.md (1 hunks)
  • .factory/skills/writing-skills/SKILL.md (1 hunks)
  • .factory/skills/writing-skills/anthropic-best-practices.md (1 hunks)
  • .factory/skills/writing-skills/persuasion-principles.md (1 hunks)
  • docs/README.factory.md (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.factory/skills/condition-based-waiting/example.ts (1)
lib/skills-core.js (1)
  • description (23-23)
🪛 LanguageTool
.factory/skills/systematic-debugging/SKILL.md

[style] ~43-~43: ‘in a hurry’ might be wordy. Consider a shorter alternative.
Context: ...ple bugs have root causes too) - You're in a hurry (rushing guarantees rework) - Manager w...

(EN_WORDINESS_PREMIUM_IN_A_HURRY)

.factory/skills/brainstorming/SKILL.md

[grammar] ~19-~19: Use a hyphen to join words.
Context: ...ime to refine the idea - Prefer multiple choice questions when possible, but open...

(QB_NEW_EN_HYPHEN)

.factory/skills/requesting-code-review/code-reviewer.md

[style] ~106-~106: Try using a synonym here to strengthen your writing.
Context: ... checking - Mark nitpicks as Critical - Give feedback on code you didn't review - Be...

(GIVE_PROVIDE)

.factory/skills/test-driven-development/SKILL.md

[grammar] ~254-~254: Use a hyphen to join words.
Context: ...ests after ≠ TDD. You get coverage, lose proof tests work. ## Common Rationaliza...

(QB_NEW_EN_HYPHEN)


[style] ~267-~267: To elevate your writing, try using a synonym here.
Context: ...clear" | Listen to test. Hard to test = hard to use. | | "TDD will slow me down" | T...

(HARD_TO)

.factory/skills/writing-skills/SKILL.md

[grammar] ~287-~287: Ensure spelling is correct
Context: ...ists - Labels without semantic meaning (step1, helper2) See @graphviz-conventions.do...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~287-~287: Ensure spelling is correct
Context: ...Labels without semantic meaning (step1, helper2) See @graphviz-conventions.dot for gra...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[style] ~372-~372: ‘under stress’ might be wordy. Consider a shorter alternative.
Context: ...s? - Pressure scenarios: Do they comply under stress? - Multiple pressures combined: time + ...

(EN_WORDINESS_PREMIUM_UNDER_STRESS)


[style] ~544-~544: To elevate your writing, try using a synonym here.
Context: ...e"]; ``` Why bad: Can't copy-paste, hard to read ### ❌ Generic Labels helper1, ...

(HARD_TO)

.factory/skills/writing-skills/anthropic-best-practices.md

[style] ~22-~22: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...Default assumption**: Claude is already very smart Only add context Claude doesn't alread...

(EN_WEAK_ADJECTIVE)


[style] ~140-~140: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...: Is the Skill clear and efficient? * Claude Opus (powerful reasoning): Does the S...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~802-~802: Try replacing commonly used words with synonyms to make your writing more engaging.
Context: ...encounter new scenarios. Each iteration improves the Skill based on real agent behavior,...

(IMPROVE_STRENGTHEN)

.factory/skills/systematic-debugging/CREATION-LOG.md

[style] ~17-~17: ‘in a hurry’ might be wordy. Consider a shorter alternative.
Context: ...uage ("even if faster", "even if I seem in a hurry") - Concrete steps for each phase **Wh...

(EN_WORDINESS_PREMIUM_IN_A_HURRY)


[style] ~40-~40: ‘in a hurry’ might be wordy. Consider a shorter alternative.
Context: ...") - "even if faster" / "even if I seem in a hurry" - "STOP and re-analyze" (explicit paus...

(EN_WORDINESS_PREMIUM_IN_A_HURRY)


[style] ~89-~89: This phrase is redundant. Consider writing “Outcome”.
Context: ...nts confusion between methodologies ## Final Outcome Bulletproof skill that: - ✅ Clearly ma...

(FINAL_END)

.factory/skills/condition-based-waiting/SKILL.md

[grammar] ~32-~32: Ensure spelling is correct
Context: ...ass sometimes, fail under load) - Tests timeout when run in parallel - Waiting for asyn...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)
.factory/skills/testing-anti-patterns/SKILL.md

33-33: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


90-90: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


208-208: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


218-218: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


228-228: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


236-236: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


246-246: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


294-294: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


302-302: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

.factory/skills/dispatching-parallel-agents/SKILL.md

143-143: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/systematic-debugging/SKILL.md

18-18: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


77-77: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/droids/code-reviewer.md

37-37: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/verification-before-completion/SKILL.md

18-18: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


26-26: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


79-79: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


85-85: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


91-91: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


97-97: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


103-103: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/receiving-code-review/SKILL.md

112-112: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/sharing-skills/SKILL.md

164-164: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


165-165: Bare URL used

(MD034, no-bare-urls)


168-168: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


170-170: Bare URL used

(MD034, no-bare-urls)


172-172: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


176-176: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

.factory/skills/writing-plans/SKILL.md

65-65: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


70-70: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


77-77: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


82-82: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


88-88: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/systematic-debugging/test-pressure-3.md

46-46: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


54-54: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


61-61: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

.factory/skills/test-driven-development/SKILL.md

33-33: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


90-90: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


208-208: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


218-218: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


228-228: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


236-236: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


246-246: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


294-294: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


302-302: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


308-308: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


318-318: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


359-359: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/systematic-debugging/test-pressure-2.md

18-18: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/writing-skills/SKILL.md

74-74: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


293-293: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


317-317: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


324-324: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


332-332: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


343-343: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


451-451: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/writing-skills/anthropic-best-practices.md

149-149: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


150-150: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


192-192: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


193-193: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


194-194: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


257-257: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


301-301: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


1040-1040: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/root-cause-tracing/SKILL.md

40-40: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/skills/subagent-driven-development/SKILL.md

41-41: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


64-64: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


85-85: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


111-111: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

.factory/AGENTS.md

52-52: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


66-66: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

docs/README.factory.md

26-26: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


67-67: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


75-75: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


89-89: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


110-110: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


143-143: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


251-251: Bare URL used

(MD034, no-bare-urls)


252-252: Bare URL used

(MD034, no-bare-urls)


253-253: Bare URL used

(MD034, no-bare-urls)

🔇 Additional comments (35)
.factory/droids/explore.md (1)

1-34: Clean droid definition.

The explore droid is well-structured with appropriate read-only tools (Read, LS, Grep, Glob) and clear capabilities, constraints, and use cases. No concerns.

.factory/droids/plan.md (1)

1-35: Well-structured planning droid.

The plan droid definition is clear and appropriate, with research-focused tools (Read, LS, Grep, Glob, WebSearch, FetchUrl) and a practical process for gathering context. No concerns.

.factory/skills/writing-skills/persuasion-principles.md (1)

1-187: Excellent governance guidance on persuasion principles.

This file uniquely addresses the psychology behind effective skill design, grounded in research (Cialdini 2021; Meincke et al. 2025). The ethical framework, principle combinations by skill type, and quick reference checklist are practical. The formatting is clean with no violations. Well done.

.factory/skills/condition-based-waiting/SKILL.md (1)

87-87: I'm unable to complete the verification due to infrastructure limitations - the repository clone operation is failing, which prevents me from checking whether example.ts exists in the codebase or examining the PR files.

Since I cannot gather the necessary information through automated verification methods, here is the rewritten review comment:


Verify that example.ts file exists at the expected location.

Line 87 references @example.ts with domain-specific helpers (waitForEvent, waitForEventCount, waitForEventMatch) for condition-based waiting. Please confirm that this file exists in the PR at .factory/skills/condition-based-waiting/example.ts and contains the referenced helpers.

.factory/skills/subagent-driven-development/SKILL.md (1)

1-147: Skill workflow structure and integration are well-designed.

The step-by-step process (load plan → dispatch → review → apply feedback → complete) is clear, the red flags section provides strong guard rails, and the integration notes accurately document required skill dependencies. The examples effectively illustrate expected behavior.

.factory/droids/general-purpose.md (1)

1-36: Droid profile is clear and actionable.

The general-purpose droid definition provides concrete direction for task dispatch, with a reasonable tool set and explicit reporting expectations. The TDD-first emphasis in step 4 aligns well with the broader workflow patterns.

.factory/skills/requesting-code-review/code-reviewer.md (1)

1-146: Code review template is well-structured with strong guidance.

The Critical/Important/Minor categorization is clear, the example output provides concrete formatting expectations, and the DO/DON'T rules (lines 94-109) effectively prevent common review anti-patterns. The template integrates well with the broader code-review workflow.

.factory/droids/code-reviewer.md (1)

1-56: Code-reviewer droid profile is well-aligned with the code review template.

The three-step review process is sound, the issue categories are clear, and the read-focused tool set is appropriate. The profile effectively complements the .factory/skills/requesting-code-review/ templates.

.factory/skills/brainstorming/SKILL.md (1)

1-54: Brainstorming skill provides clear, practical guidance.

The one-question-at-a-time discipline and section-by-section validation approach are well-designed for iterative design discovery. The Key Principles section is strong, and the integration references to related skills (writing-plans, using-git-worktrees) are appropriate.

.factory/commands/brainstorm.md (1)

1-7: Brainstorm command dispatch is simple and clear.

The file correctly routes to the brainstorming skill and provides minimal but sufficient guidance for invocation.

.factory/skills/using-git-worktrees/SKILL.md (3)

51-73: Safety verification for .gitignore is well-designed.

The emphasis on verifying .gitignore before creating project-local worktrees (lines 55-69) is a critical guard rail. The "Fix broken things immediately" principle is properly applied, and the rationale (lines 69) is clear.


156-173: Common Mistakes and Red Flags sections provide strong practical guidance.

The Common Mistakes table (lines 156-173) is actionable, and the Red Flags section (lines 190-204) with explicit "Never" and "Always" guidance provides clear boundaries for the workflow. The Example Workflow (lines 174-188) effectively illustrates expected behavior.

Also applies to: 190-204


1-214: Overall skill structure is thorough and safety-conscious.

The systematic directory selection (priority order, AGENTS.md check), mandatory .gitignore verification, auto-detection of project setup, and baseline test verification create a robust workflow. Integration notes accurately reflect calling and called skills. The repetition of key principles (safety, priority, verification) reinforces expectations.

.factory/skills/receiving-code-review/SKILL.md (3)

27-39: Strong, principle-driven guidance on feedback handling.

The Forbidden Responses section explicitly prevents performative agreement, the guidance for external reviewers appropriately emphasizes verification and skepticism, and the YAGNI check ensures unnecessary features aren't blindly implemented. These guard rails reinforce technical rigor over social performance.

Also applies to: 67-84, 88-99


165-175: Common Mistakes and Real Examples are practical and concrete.

The Common Mistakes table (lines 165-175) is actionable, and the Real Examples section (lines 176-201) contrasts bad vs. good patterns effectively, providing clear behavioral guidance. The distinction between performative agreement, technical verification, and YAGNI reasoning is well-illustrated.

Also applies to: 176-201


1-208: Skill provides well-articulated principles for technical code review reception.

The six-step response pattern (verify before implementing), the emphasis on technical rigor over social comfort, and the pragmatic guidance for external reviewers create a strong, principled framework. The prohibition on performative language and the emphasis on "actions speak" align well with the broader agent workflow ethos.

.factory/skills/defense-in-depth/SKILL.md (1)

27-90: Defense-in-depth layering doc looks solid

The four-layer pattern and examples are consistent and actionable; nothing blocking from a correctness or clarity standpoint.

.factory/commands/execute-plan.md (1)

1-7: Command wrapper is minimal and consistent

Frontmatter and the pointer to Skill("executing-plans") are clean and match the pattern for command wrappers; no changes needed.

.factory/commands/write-plan.md (1)

1-7: LGTM!

Clear and concise command documentation that properly delegates to the writing-plans skill. Frontmatter and load instructions are consistent with the skill definition.

.factory/skills/requesting-code-review/SKILL.md (1)

1-105: Well-structured skill documentation with clear guidance.

The skill document provides clear mandatory/optional guidance, concrete examples, and integration patterns. The walkthrough example is particularly helpful for understanding when and how to request reviews. Red flags section reinforces best practices effectively.

.factory/skills/systematic-debugging/test-academic.md (1)

1-14: Well-designed assessment questions.

The academic test effectively covers the key concepts and phases of the systematic debugging skill through six targeted questions. Requiring direct quotes from the skill ensures precise understanding verification.

.factory/INSTALL.md (2)

71-118: Comprehensive verification tests ensure proper installation.

The five verification tests cover the main integration points (commands, skill detection, droids, skills list, protocol). The checklist format in lines 120-128 provides a quick summary. Tests are well-designed with clear pass criteria.


1-40: I'll help you verify the Factory documentation link referenced in the INSTALL.md file. Let me search the repository for any references to Factory documentation and then verify the URL.
<function_calls>

#!/bin/bash

Search for references to Factory documentation or docs.factory

rg -i "factory" .factory/ -A2 -B2 | head -50


</function_calls>

Let me also check for any other documentation links in the repository:
<function_calls>

#!/bin/bash

Search for all URLs and documentation references in markdown files

rg "https?://" -t md | grep -i "factory|docs" | head -20


</function_calls>

Now let me search the web for the official Factory Droid CLI documentation:
<function_calls>

Factory Droid CLI official documentation URL

</function_calls>

Let me also check if docs.factory.ai is a valid domain:
<function_calls>

docs.factory.ai Droid CLI

</function_calls>

.factory/skills/systematic-debugging/test-pressure-1.md (1)

1-58: Effective emergency scenario test.

The scenario realistically presents the tension between immediate action and systematic investigation. The financial impact framing and prior success with a similar fix create realistic psychological pressure. Options A, B, C clearly delineate different approaches with their tradeoffs.

.factory/skills/executing-plans/SKILL.md (1)

1-76: Skill structure is clear and well-organized.

The executing-plans skill provides a practical workflow for batch execution with review checkpoints. The process steps are concrete, stop conditions are explicit, and the required sub-skill reference is properly noted. The skill follows appropriate SKILL.md conventions.

.factory/skills/systematic-debugging/test-pressure-3.md (1)

1-68: Pressure test scenario is realistic and well-constructed.

The test scenario effectively combines multiple pressures (authority, social, time, expertise gap) that would realistically encourage the agent to skip systematic debugging. Options are clearly defined with appropriate trade-offs. The format is appropriate for a scenario-based pressure test.

.factory/install.sh (1)

1-125: Installer script is well-structured and robust.

The script follows bash best practices: proper error handling with set -euo pipefail, explicit prerequisite checks, safe file operations with || true, and user-friendly output with color-coded feedback. The backup of existing AGENTS.md before appending is a good defensive practice.

.factory/skills/testing-skills-with-subagents/SKILL.md (1)

1-380: TDD-based skill testing framework is comprehensive and well-structured.

The skill effectively maps the RED-GREEN-REFACTOR cycle to skill testing, with clear phases, practical pressure scenario design, and explicit bulletproofing criteria. The meta-testing section provides good troubleshooting guidance. The framework is actionable and appropriately rigorous for ensuring skills resist rationalization under pressure.

.factory/skills/finishing-a-development-branch/SKILL.md (7)

1-14: Excellent skill documentation structure.

The YAML frontmatter, clear step breakdown, and integration notes follow best practices. The announcement instruction on line 14 aligns well with the AGENTS.md protocol requirement to announce skill usage.


18-38: Strong test verification step with explicit failure handling.

The halt condition (line 36) prevents proceeding with failing tests, which is a critical safeguard. The placeholder <test command> appropriately allows per-project customization.


49-64: Effective option presentation.

Exactly 4 options, concise formatting without over-explanation. Aligns well with the AGENTS.md directive to keep instructions clear and actionable.


66-134: Comprehensive option execution logic with proper guardrails.

Each option includes:

  • Concrete bash commands for reproducibility
  • Conditional flows (e.g., test verification after merge on line 80)
  • Confirmation requirement for destructive action (Option 4, lines 116-126)
  • Clear sequencing to Step 5 for options that need cleanup

The confirmation pattern ("Type 'discard' to confirm") is good defensive practice for irreversible actions.


136-150: Correct conditional worktree cleanup logic.

Step 5 properly handles the four options:

  • Options 1, 2, 4: Clean up worktree (lines 138-148)
  • Option 3: Preserve worktree (line 150)

The detection via git worktree list | grep is appropriate for typical worktree names, though it may miss edge cases with special characters in branch names. For standard naming conventions, this is acceptable.


152-191: Excellent defensive documentation.

The Quick Reference table, Common Mistakes, and Red Flags sections reinforce the skill's critical requirements:

  • Always verify tests before offering options
  • Require explicit confirmation for destructive actions
  • Conditional cleanup based on chosen option

This defensive posture reduces the risk of accidentally merging broken code or losing work.


193-200: Clear integration points.

The Integration section effectively maps this skill to callers (subagent-driven-development, executing-plans) and paired skills (using-git-worktrees), establishing clear context boundaries.

Comment on lines +66 to +85
```typescript
async function waitFor<T>(
condition: () => T | undefined | null | false,
description: string,
timeoutMs = 5000
): Promise<T> {
const startTime = Date.now();

while (true) {
const result = condition();
if (result) return result;

if (Date.now() - startTime > timeoutMs) {
throw new Error(`Timeout waiting for ${description} after ${timeoutMs}ms`);
}

await new Promise(r => setTimeout(r, 10)); // Poll every 10ms
}
}
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add language specifier to code block.

Line 66 has a TypeScript code block without language identifier. Add typescript for syntax highlighting.

-```
+```typescript
async function waitFor<T>(
🤖 Prompt for AI Agents
In .factory/skills/condition-based-waiting/SKILL.md around lines 66 to 85 the
fenced code block is missing a language specifier; update the opening fence from
``` to ```typescript so the block begins with ```typescript (keep the existing
TypeScript code and the closing ``` intact) to enable syntax highlighting.

Comment on lines +143 to +147
```
Agent 1 → Fix agent-tool-abort.test.ts
Agent 2 → Fix batch-completion-behavior.test.ts
Agent 3 → Fix tool-approval-race-conditions.test.ts
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add language specifier to code block.

Line 143 has a code block (TypeScript-like syntax showing Task calls) without language identifier. Add typescript for consistency and highlighting.

-```
+```typescript
 // In Droid CLI / AI environment
 Task("Fix agent-tool-abort.test.ts failures")
-```
+```
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

143-143: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
.factory/skills/dispatching-parallel-agents/SKILL.md around lines 143 to 147:
the fenced code block is missing a language specifier; update the opening
triple-backtick to include "typescript" (i.e. ```typescript) so the block
becomes highlighted consistently, then keep the closing triple-backticks as-is.

Comment thread .factory/skills/root-cause-tracing/find-polluter.sh Outdated
Comment thread .factory/skills/test-driven-development/SKILL.md Outdated
Comment thread .factory/skills/test-driven-development/SKILL.md Outdated
render(<Page />);
expect(screen.getByTestId('sidebar-mock')).toBeInTheDocument();
});
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add language specifiers to TypeScript code blocks.

Lines 33 and 90 have code blocks without language specifiers. Add typescript for proper syntax highlighting.

-```
+```typescript
 // ❌ BAD
-```
+```

Also applies to: 90-90

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

33-33: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In .factory/skills/testing-anti-patterns/SKILL.md around lines 33 and 90, the
fenced code blocks lack a language specifier; update those backticks to use
"```typescript" for each code block start so TypeScript syntax highlighting is
applied, leaving the code contents and closing backticks unchanged.

```

**Why this is wrong:**
- **Partial mocks hide structural assumptions** - You only mocked fields you know about
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Convert emphasis to proper headings.

Lines 195, 200, and 208 use bold emphasis for section introductions (**Partial mocks...**, **The Iron Rule:**, **When mocks become...**) instead of proper heading syntax. Use ### Heading format for better document structure and Markdown compliance (MD036).

Also applies to: 200-200, 208-208

🤖 Prompt for AI Agents
.factory/skills/testing-anti-patterns/SKILL.md lines 195, 200, 208: three
section introductions currently use bold emphasis (e.g., **Partial mocks hide
structural assumptions**) — replace each bolded phrase with a level-3 Markdown
heading using "### " followed by the same text (no surrounding asterisks) so the
sections become proper headings (MD036 compliant) and maintain existing wording
and punctuation.

Comment thread .factory/skills/verification-before-completion/SKILL.md Outdated
- Add language specifiers to code blocks (text for pseudo-code)
- Convert bold emphasis to proper headings (MD036)
- Fix find-polluter.sh zero-match handling bug

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.factory/skills/testing-anti-patterns/SKILL.md (1)

56-64: Add language specifiers to all fenced code blocks for proper syntax highlighting.

Lines 56, 107, 156, 237, and 249 contain fenced code blocks without language specifiers. These appear to be pseudocode or plain-text gate functions/scenarios. Add text or an appropriate language identifier to each block for consistency with other code blocks in the file.

Apply this diff to add language specifiers:

  ### Gate Function
  
-```
+```text
  BEFORE asserting on any mock element:
    Ask: "Am I testing real component behavior or just mock existence?"
  
    IF testing mock existence:
      STOP - Delete the assertion or unmock the component
  
    Test real behavior instead
-```
+```

  ## Anti-Pattern 2: Test-Only Methods in Production
  ### Gate Function
  
-```
+```text
  BEFORE adding any method to production class:
    Ask: "Is this only used by tests?"
  
    IF yes:
      STOP - Don't add it
      Put it in test utilities instead
  
    Ask: "Does this class own this resource's lifecycle?"
  
    IF no:
      STOP - Wrong class for this method
-```
+```

  ## Anti-Pattern 3: Mocking Without Understanding
  ### Gate Function
  
-```
+```text
  BEFORE mocking any method:
    STOP - Don't mock yet
  
    1. Ask: "What side effects does the real method have?"
    2. Ask: "Does this test depend on any of those side effects?"
    3. Ask: "Do I fully understand what this test needs?"
  
    IF depends on side effects:
      Mock at lower level (the actual slow/external operation)
      OR use test doubles that preserve necessary behavior
      NOT the high-level method the test depends on
  
    IF unsure what test depends on:
      Run test with real implementation FIRST
      Observe what actually needs to happen
      THEN add minimal mocking at the right level
  
    Red flags:
      - "I'll mock this to be safe"
      - "This might be slow, better mock it"
      - Mocking without understanding the dependency chain
-```
+```

  ## Anti-Pattern 4: Incomplete Mocks
  **The violation:**
-```
+```text
  ✅ Implementation complete
  ❌ No tests written
  "Ready for testing"
-```
+```

  **Why this is wrong:**
  **The fix:**
-```
+```text
  TDD cycle:
  1. Write failing test
  2. Implement to pass
  3. Refactor
  4. THEN claim complete
-```
+```

  ## When Mocks Become Too Complex

Also applies to: 107-119, 156-178, 237-241, 249-255

🧹 Nitpick comments (2)
.factory/skills/test-driven-development/SKILL.md (2)

254-254: Clarify awkward phrasing in the sunk-cost rationalization section.

Line 254 reads "30 minutes of tests after ≠ TDD. You get coverage, lose proof tests work." The phrase "lose proof tests work" is unclear. Consider: "You get coverage but lose the proof that tests actually work" or similar, to clarify the distinction between test coverage and validated test-first behavior.

-30 minutes of tests after ≠ TDD. You get coverage, lose proof tests work.
+30 minutes of tests after ≠ TDD. You get coverage, lose proof that tests work.

267-267: Reduce redundant use of "Hard" in the "Test hard = design unclear" rationalization.

Line 267's table entry uses "Hard to test = hard to use" with "Hard" appearing twice in close succession, which reads awkwardly. Consider rephrasing for clarity: "Difficult to test = difficult to use" or restructure to avoid the repetition.

-| "Test hard = design unclear" | Listen to test. Hard to test = hard to use. |
+| "Test hard = design unclear" | Listen to test. If it's hard to test, the design is hard to use. |
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b755a2 and 8d97833.

📒 Files selected for processing (5)
  • .factory/skills/dispatching-parallel-agents/SKILL.md (1 hunks)
  • .factory/skills/root-cause-tracing/find-polluter.sh (1 hunks)
  • .factory/skills/test-driven-development/SKILL.md (1 hunks)
  • .factory/skills/testing-anti-patterns/SKILL.md (1 hunks)
  • .factory/skills/verification-before-completion/SKILL.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .factory/skills/root-cause-tracing/find-polluter.sh
  • .factory/skills/dispatching-parallel-agents/SKILL.md
  • .factory/skills/verification-before-completion/SKILL.md
🧰 Additional context used
🪛 LanguageTool
.factory/skills/test-driven-development/SKILL.md

[grammar] ~254-~254: Use a hyphen to join words.
Context: ...ests after ≠ TDD. You get coverage, lose proof tests work. ## Common Rationaliza...

(QB_NEW_EN_HYPHEN)


[style] ~267-~267: To elevate your writing, try using a synonym here.
Context: ...clear" | Listen to test. Hard to test = hard to use. | | "TDD will slow me down" | T...

(HARD_TO)

🪛 markdownlint-cli2 (0.18.1)
.factory/skills/test-driven-development/SKILL.md

90-90: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


294-294: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


302-302: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


308-308: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


318-318: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

.factory/skills/testing-anti-patterns/SKILL.md

56-56: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


107-107: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


156-156: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


237-237: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


249-249: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Comment thread .factory/skills/test-driven-development/SKILL.md Outdated
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.factory/skills/test-driven-development/SKILL.md (1)

254-254: Minor: Clarify phrasing on line 254.

The phrase "lose proof tests work" reads incomplete. Consider restructuring to "lose proof that tests work" or similar for clarity.

-30 minutes of tests after ≠ TDD. You get coverage, lose proof tests work.
+30 minutes of tests after ≠ TDD. You get coverage, lose proof that tests work.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d97833 and cf5eb24.

📒 Files selected for processing (1)
  • .factory/skills/test-driven-development/SKILL.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
.factory/skills/test-driven-development/SKILL.md

[grammar] ~254-~254: Use a hyphen to join words.
Context: ...ests after ≠ TDD. You get coverage, lose proof tests work. ## Common Rationaliza...

(QB_NEW_EN_HYPHEN)


[style] ~267-~267: To elevate your writing, try using a synonym here.
Context: ...clear" | Listen to test. Hard to test = hard to use. | | "TDD will slow me down" | T...

(HARD_TO)

🪛 markdownlint-cli2 (0.18.1)
.factory/skills/test-driven-development/SKILL.md

90-90: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (1)
.factory/skills/test-driven-development/SKILL.md (1)

1-367: Comprehensive TDD skill document—well structured and previous major issues resolved.

The document provides excellent guidance on Test-Driven Development principles with clear examples, verification steps, and a thorough handling of common objections. Previous review concerns (missing language specifiers on code blocks and improper heading structure) have been fully addressed. All code blocks are properly annotated with language identifiers, and section organization uses consistent ### heading syntax.

@obra
Copy link
Copy Markdown
Owner

obra commented Dec 1, 2025

this looks autogenerated and not human reviewed. I'd be delighted to have support for droid cli, but not if it means full copies of all the skills

@obra obra closed this Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants