docs(skills): teach CLI agent orchestration - #168
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe PR rewrites CLI guidance for dynamic workflows and subagent delegation. It simplifies provider-specific override references. It adds compact JSON serialization for agent targets and tests the catalog and skill terminology. ChangesCLI Guidance and Agent Catalog
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Greptile SummaryThe PR streamlines the CLI-facing subagent and dynamic-workflow skills and makes JSON target discovery return a compact, model-facing catalog.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issues identified. The compact target representation is intentional and covered by tests, configured-profile details remain available, and the revised workflow commands match the implemented CLI behavior.
|
| Filename | Overview |
|---|---|
| src/cli.ts | Routes JSON target discovery through the new compact catalog representation. |
| src/local-agent-catalog.ts | Adds an explicit target-catalog type that exposes provider names and complete profile summaries. |
| skills/subagents/SKILL.md | Rewrites direct-subagent guidance around target discovery, execution, continuation, and optional overrides. |
| skills/dynamic-workflows/SKILL.md | Replaces broad workflow documentation with concise CLI usage and representative orchestration examples. |
| src/local-agent-catalog.test.ts | Verifies that compact provider entries contain names without model metadata. |
| src/skills.test.ts | Adds content checks ensuring the revised skills describe supported CLI surfaces and omit removed guidance. |
Reviews (1): Last reviewed commit: "fix(cli): keep target discovery model-fa..." | Re-trigger Greptile
116d021 to
cb457a3
Compare
cb457a3 to
956295f
Compare
|
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/local-agent-catalog.test.ts (1)
33-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the complete compact catalog contract.
The test checks the provider name and absence of
model, but it does not check absence ofeffortor preservation ofprofiles. Assert the complete returned object once. Add a CLI-level assertion fordevspace agents targets --jsonif this is the public contract.Proposed test adjustment
+const compact = compactLocalAgentCatalog(catalog); -assert.deepEqual(compactLocalAgentCatalog(catalog).providers, [{ name: "codex" }]); -assert.equal("model" in compactLocalAgentCatalog(catalog).providers[0]!, false); +assert.deepEqual(compact.providers, [{ name: "codex" }]); +assert.equal("model" in compact.providers[0]!, false); +assert.equal("effort" in compact.providers[0]!, false); +assert.deepEqual(compact.profiles, catalog.profiles);As per coding guidelines, when changing a cross-cutting concept, trace all affected contracts through tests and CLI output.
🤖 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 `@src/local-agent-catalog.test.ts` around lines 33 - 35, Update the test around compactLocalAgentCatalog to assert the complete returned provider object in one deep-equality check, including the expected name, absence of model and effort, and preservation of profiles. Also add a CLI-level JSON assertion for “devspace agents targets --json” when that command exposes this public contract.Source: Coding 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 `@skills/dynamic-workflows/SKILL.md`:
- Around line 106-108: Update the parallel-writer guidance to explicitly require
returning the detached worktree path, and state that the calling workflow must
handle merging changes into the parent checkout and error handling. Preserve the
existing sequential pipeline guidance.
In `@skills/subagents/SKILL.md`:
- Around line 74-83: The copy-paste examples in the agent delegation section use
undeclared profile names. Update the examples around “devspace agents run” to
use targets from “devspace agents targets” or clearly mark reviewer and
implementer as placeholders requiring matching configured profiles, while
preserving the existing agt_1234abcd continuation example.
In `@src/local-agent-catalog.ts`:
- Around line 19-30: Update LocalAgentTargetCatalog and compactLocalAgentCatalog
so the JSON target catalog preserves each provider’s model and effort capability
metadata alongside its name. Ensure devspace agents targets --json exposes the
values required for --model and --effort selection, rather than emitting
name-only provider entries.
In `@src/skills.test.ts`:
- Around line 283-289: Extend the skills coverage around the existing
subagentsGuide and workflowsGuide assertions to verify the packaged skill files
and CLI path exposed through package.json, rather than only resolving
source-tree Markdown. Cover the omitted documented forms: devspace agents
targets, --model/--effort, agent-id continuation, and workflow
--arg/--resume/--follow; alternatively explicitly label the current test as a
source-tree proxy.
---
Nitpick comments:
In `@src/local-agent-catalog.test.ts`:
- Around line 33-35: Update the test around compactLocalAgentCatalog to assert
the complete returned provider object in one deep-equality check, including the
expected name, absence of model and effort, and preservation of profiles. Also
add a CLI-level JSON assertion for “devspace agents targets --json” when that
command exposes this public contract.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c01ef87-23ec-45da-80fa-e340617a88d3
📒 Files selected for processing (12)
skills/dynamic-workflows/SKILL.mdskills/subagents/SKILL.mdskills/subagents/references/claude.mdskills/subagents/references/codex.mdskills/subagents/references/copilot.mdskills/subagents/references/cursor.mdskills/subagents/references/opencode.mdskills/subagents/references/pi.mdsrc/cli.tssrc/local-agent-catalog.test.tssrc/local-agent-catalog.tssrc/skills.test.ts
| For parallel writers, request `isolation: 'worktree'` and make the prompt | ||
| describe how the result should be handed back. For sequential edits that must | ||
| see one another’s files, keep the stages in a pipeline or ordinary sequence. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 6 'isolation|worktree|merge|apply|checkout' .Repository: Waishnav/devspace
Length of output: 50374
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== target skill =="
cat -n skills/dynamic-workflows/SKILL.md | sed -n '1,150p'
echo
echo "== workflow contracts relevant =="
cat -n src/workflow-contracts.ts | sed -n '1,260p'
echo
echo "== workflow-worktrees outline/search =="
rg -n "createWorkflowWorktreeFactory|finalize|dirty|removed|checkout|worktree" src/workflow-worktrees.ts src/workflow-store.ts src/workflow-worker.ts src/workflow-script.ts src/run-workflow.ts -C 8 || true
echo
echo "== workflow-worktrees file =="
cat -n src/workflow-worktrees.ts | sed -n '1,260p'
echo
echo "== workflow scripts/execution relevant =="
rg -n "agent\\(|isolation|worktreePath|worktree_finalized|completeAgentCall|returnValueJson|dirty" src/script src/workflow-sandbox src/workflow-script.ts src/workflow-worker.ts -C 5 || trueRepository: Waishnav/devspace
Length of output: 47212
Document how dirty worktrees exit the workflow.
isolation: 'worktree' creates a detached worktree, and DevSpace only returns the worktree path without merging changes into the parent checkout. Tell workflows to return the worktree path and handle checkout merging/error handling at the call site.
🤖 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 `@skills/dynamic-workflows/SKILL.md` around lines 106 - 108, Update the
parallel-writer guidance to explicitly require returning the detached worktree
path, and state that the calling workflow must handle merging changes into the
parent checkout and error handling. Preserve the existing sequential pipeline
guidance.
Source: Coding guidelines
| ```bash | ||
| # Ask for an independent security review. | ||
| devspace agents run reviewer "Review the authentication changes for vulnerabilities. Return findings with file paths and severity." | ||
|
|
||
| # Delegate a small implementation and ask for verification. | ||
| devspace agents run implementer "Add a regression test for the parser bug. Run the focused test and report the result." | ||
|
|
||
| # Continue after the parent has inspected the first response. | ||
| devspace agents run agt_1234abcd "The test still fails on Windows. Investigate only the path handling and report a fix." | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 \
'reviewer|implementer|default.*profile|profiles' \
--glob '*.json' --glob '*.yaml' --glob '*.yml' --glob '*.md' .Repository: Waishnav/devspace
Length of output: 22297
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- focused search for reviewer/implementer in src ---\n'
rg -n '\'?reviewer\'?|\'?implementer\'?|DEVSPACE_AGENT|agent profile|agents run|targets' src docs tools bin scripts --glob '*.ts' --glob '*.js' --glob '*.mjs' --glob '*.cjs' --glob '*.json' || true
printf '\n--- package scripts/package manager files ---\n'
cat package.json | sed -n '1,80p'
git ls-files | sed -n '1,120p'
printf '\n--- subagent skill relevant lines ---\n'
cat -n skills/subagents/SKILL.md | sed -n '1,110p'Repository: Waishnav/devspace
Length of output: 246
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- focused search for reviewer/implementer in src ---'
rg -- \
'\'?reviewer\'?|\'?implementer\'?|DEVSPACE_AGEN|agent profile|agents run|targets' \
src docs tools bin scripts --glob '*.ts' --glob '*.js' --glob '*.mjs' --glob '*.cjs' --glob '*.json' || true
printf '%s\n' ''
printf '%s\n' '--- package scripts/package manager files ---'
sed -n '1,80p' package.json
git ls-files | sed -n '1,160p'
printf '%s\n' ''
printf '%s\n' '--- subagent skill relevant lines ---'
cat -n skills/subagents/SKILL.md | sed -n '1,110p'Repository: Waishnav/devspace
Length of output: 271
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- focused search for reviewer/implementer in src ---'
rg -n -S -- \
"reviewer|implementer|DEVSPACE_AGEN|agent profile|agents run|targets" \
src docs tools bin scripts --glob '*.ts' --glob '*.js' --glob '*.mjs' --glob '*.cjs' --glob '*.json' || true
printf '%s\n' ''
printf '%s\n' '--- package scripts/package manager files ---'
sed -n '1,80p' package.json
git ls-files | sed -n '1,160p'
printf '%s\n' ''
printf '%s\n' '--- subagent skill relevant lines ---'
cat -n skills/subagents/SKILL.md | sed -n '1,110p'Repository: Waishnav/devspace
Length of output: 27538
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- example agent profiles ---'
for f in examples/agents/*.md; do
echo "--- $f"
sed -n '1,8p' "$f"
done
printf '%s\n' ''
printf '%s\n' '--- subagent skill reference files names only ---'
git ls-files 'skills/subagents/references/*.md'
printf '%s\n' ''
printf '%s\n' '--- source resolution for unknown subagent ---'
sed -n '55,85p' src/local-agent-resolution.tsRepository: Waishnav/devspace
Length of output: 3746
Use configured profile names in copy-paste examples.
reviewer and implementer are user-owned profiles, not declared by this skill or provided by default configuration. Since the same guide says not to guess profile names, these examples can fail if the user has not created matching profiles. Replace them with targets from devspace agents targets, or label them as examples that require matching configured profiles.
🤖 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 `@skills/subagents/SKILL.md` around lines 74 - 83, The copy-paste examples in
the agent delegation section use undeclared profile names. Update the examples
around “devspace agents run” to use targets from “devspace agents targets” or
clearly mark reviewer and implementer as placeholders requiring matching
configured profiles, while preserving the existing agt_1234abcd continuation
example.
| export interface LocalAgentTargetCatalog { | ||
| providers: Array<{ name: string }>; | ||
| profiles: ReturnType<typeof summarizeLocalAgentProfile>[]; | ||
| } | ||
|
|
||
| /** Keep model-facing target discovery focused on selectable values. */ | ||
| export function compactLocalAgentCatalog(catalog: LocalAgentCatalog): LocalAgentTargetCatalog { | ||
| return { | ||
| providers: catalog.providers.map(({ name }) => ({ name })), | ||
| profiles: catalog.profiles, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 \
'compactLocalAgentCatalog|LocalAgentTargetCatalog|agents targets|providers|\.model|\.effort' \
--glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.mjs' --glob '*.md' .Repository: Waishnav/devspace
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Candidate files:\n'
git ls-files | rg '(^src/local-agent-(catalog|targets)\.ts|skills/subagents|devspace.*agent|agents.*targets|package\.json|README\.md|docs/(subagents|workflow|.*agent).*\.md)$' || true
printf '\nsrc/local-agent-catalog.ts outline/contents:\n'
if [ -f src/local-agent-catalog.ts ]; then
wc -l src/local-agent-catalog.ts
cat -n src/local-agent-catalog.ts
fi
printf '\nsrc/local-agent-targets.ts relevant definitions:\n'
if [ -f src/local-agent-targets.ts ]; then
ast-grep outline src/local-agent-targets.ts --view compact || true
sed -n '1,220p' src/local-agent-targets.ts
fi
printf '\nCLI targets command references:\n'
rg -n -C 4 'runAgentsTargets|agents targets|TargetsJson|targets.*json|compactLocalAgentCatalog|LocalAgentTargetCatalog' src --glob '*.ts' --glob '*.tsx' || true
printf '\nSubagents skill target catalog references:\n'
sed -n '1,240p' skills/subagents/SKILL.md
printf '\nProgrammatic shape verifier:\n'
python3 - <<'PY'
from pathlib import Path
import re
catalog = Path('src/local-agent-catalog.ts').read_text()
targets = Path('src/local-agent-targets.ts').read_text()
print('catalog interface preserves model/effort?', 'model?:' in catalog or 'model:' in catalog)
print('catalog interface preserves provider capabilities?', 'capabilities' in catalog or 'modelCapabilities' in catalog or 'supported' in catalog)
print('compact interface provider shape:', bool(re.search(r'providers:\s*Array<\{[^}]*name[^}]*\}>', catalog, re.S)))
print('resolver raw provider result:', 'kind: "provider"' in targets)
print('resolver provider model assignment:', bool(re.search(r'kind:\s*"provider",\s*\n\s*name:\s*provider,\s*\n\s*provider:\s*provider,\s*\n\s*model:\s*input\.model,\s*\n\s*effort:\s*input\.effort', targets, re.S)))
print('CLI json output uses compact?', bool(re.search(r'compactLocalAgentCatalog|LocalAgentTargetCatalog', targets)))
PYRepository: Waishnav/devspace
Length of output: 15199
Keep provider model/effort capability metadata in the JSON target catalog.
devspace agents targets --json serializes compactLocalAgentCatalog(catalog), whose provider shape now contains only name. This contradicts the Subagents skill instruction to use only values from the target catalog for --model and --effort, because raw provider entries have no capability values to select from. Keep the provider model/effort shape in the public JSON output, or add a versioned compact format and update the instructions.
🤖 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 `@src/local-agent-catalog.ts` around lines 19 - 30, Update
LocalAgentTargetCatalog and compactLocalAgentCatalog so the JSON target catalog
preserves each provider’s model and effort capability metadata alongside its
name. Ensure devspace agents targets --json exposes the values required for
--model and --effort selection, rather than emitting name-only provider entries.
Source: Coding guidelines
| const subagentsGuide = await readFile(new URL("../skills/subagents/SKILL.md", import.meta.url), "utf8"); | ||
| const workflowsGuide = await readFile(new URL("../skills/dynamic-workflows/SKILL.md", import.meta.url), "utf8"); | ||
| assert.match(subagentsGuide, /devspace agents run/); | ||
| assert.doesNotMatch(subagentsGuide, /MCP/i); | ||
| assert.match(workflowsGuide, /devspace workflow run/); | ||
| assert.doesNotMatch(workflowsGuide, /workflow tui/i); | ||
| assert.doesNotMatch(workflowsGuide, /MCP|replay walks|determinism bans/i); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -t f . |
rg '(^|/)(package.json|package-lock.json|pnpm-lock.yaml|yarn.lock|npm-shrinkwrap.json)$' |
xargs -r rg -n -C 4 'skills|files|build|package'
rg -n -C 4 \
'devspace (agents targets|agents run|workflow run)|--arg|--resume' .Repository: Waishnav/devspace
Length of output: 50375
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- package files ---\n'
for f in package.json package-lock.json; do
if [ -f "$f" ]; then
echo "$f lines"
wc -l "$f"
sed -n '1,45p' "$f" | cat -n
fi
done
printf '\n--- src/skills.test.ts relevant section ---\n'
sed -n '250,305p' src/skills.test.ts | cat -n
printf '\n--- skills directory entry ---\n'
find skills -maxdepth 2 -type f -o -maxdepth 1 -type d | sort | sed -n '1,80p'
printf '\n--- command-form assertions in src/skills.test.ts ---\n'
rg -n 'devspace agents (targets|ls|show|run)|devspace workflow (run|status|cancel|ls|calls)|--(arg|resume|follow)|MCP|workflow tui|replay walks|determinism bans' src/skills.test.tsRepository: Waishnav/devspace
Length of output: 246
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf -- '--- package files ---\n'
for f in package.json package-lock.json; do
if [ -f "$f" ]; then
echo "$f lines"
wc -l "$f"
sed -n '1,45p' "$f" | cat -n
fi
done
printf -- '\n--- src/skills.test.ts relevant section ---\n'
sed -n '250,305p' src/skills.test.ts | cat -n
printf -- '\n--- skills directory entry ---\n'
find skills -maxdepth 2 -type f -o -maxdepth 1 -type d | sort | sed -n '1,80p'
printf -- '\n--- command-form assertions in src/skills.test.ts ---\n'
rg -n 'devspace agents (targets|ls|show|run)|devspace workflow (run|status|cancel|ls|calls)|--(arg|resume|follow)|MCP|workflow tui|replay walks|determinism bans' src/skills.test.tsRepository: Waishnav/devspace
Length of output: 7651
Verify the packaged skill and CLI path, not only source Markdown.
package.json includes skills, but src/skills.test.ts resolves ../skills/... from the source-tree test file. This only covers source Markdown, not the npm/npx package. The source also includes forms not covered here: devspace agents targets, --model/--effort, agent-id continuation, and workflow --arg/--resume/--follow. Add a package smoke test, or mark this as a source-tree proxy.
🤖 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 `@src/skills.test.ts` around lines 283 - 289, Extend the skills coverage around
the existing subagentsGuide and workflowsGuide assertions to verify the packaged
skill files and CLI path exposed through package.json, rather than only
resolving source-tree Markdown. Cover the omitted documented forms: devspace
agents targets, --model/--effort, agent-id continuation, and workflow
--arg/--resume/--follow; alternatively explicitly label the current test as a
source-tree proxy.
Source: Coding guidelines
|
Closing in favor of #143. The Sol implementation gives the model-facing skills a clearer structured-output contract, explicit polling and terminal-state guidance, and better recovery semantics. This version was solid locally, but the replacement is more automation-friendly and more consistent with the CLI-only architecture. |
Coding harnesses need model-facing guidance that does not assume a particular host integration. The bundled subagent and Dynamic Workflow skills now teach the CLI commands, options, capabilities, and practical use cases, with provider references reduced to selectable defaults. Host-specific UI and backend implementation details are removed from the installed guidance.
Summary by CodeRabbit
Documentation
Improvements
Tests