Add --dry-run flag to agentworkforce agent#57
Merged
willwashburn merged 1 commit intomainfrom May 8, 2026
Merged
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a new ChangesDry-run CLI validation
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)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/cli/src/cli.ts`:
- Around line 925-945: The current dry-run uses one shared tempDir for all
installs (tempDir, loop over plan.installs), causing state from one skill to
affect others; change the loop in which you destructure inst.installCommand and
call spawnSync/subprocessExitCode so that for each inst you create a unique
subdirectory (e.g., join(tempDir, inst.skillId) or a per-install mkdtemp) and
run spawnSync with cwd set to that per-skill dir, ensuring you still keep the
top-level preserved dry-run root but isolate each installer to avoid
cross-contamination and preserve per-skill artifacts for inspection.
In `@personas/persona-maker.json`:
- Line 29: Update the dry-run validation rules so authors must run
agentworkforce agent <id>@<tier> --dry-run from a resolvable source directory
(either TARGET_DIR that matches the persona cascade or by adding the persona to
the cascade first) and must run the dry-run for every tier that has any
tier-specific runtime config (not just differing skills[])—specifically detect
per-tier overrides of claudeMd/agentsMd sidecars, permissions, mcpServers, or
harnessSettings and require a dry-run for each such tier; if CREATE_MODE is
built-in, require the author to append the intent to PERSONA_INTENTS and add the
persona to the workspace before running the tiered dry-runs so the agent CLI can
resolve <id>@<tier>.
🪄 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: 812bc13c-d4ec-4519-b0cf-8154611e575a
⛔ Files ignored due to path filters (1)
packages/workload-router/src/generated/personas.tsis excluded by!**/generated/**
📒 Files selected for processing (3)
packages/cli/src/cli.test.tspackages/cli/src/cli.tspersonas/persona-maker.json
Comment on lines
+925
to
+945
| const tempDir = mkdtempSync(join(tmpdir(), `agentworkforce-dryrun-${personaId}-`)); | ||
| process.stderr.write(`• temp dir: ${tempDir}\n`); | ||
| process.stderr.write(`• installing ${plan.installs.length} skill(s)\n`); | ||
|
|
||
| const failures: { skillId: string; exitCode: number }[] = []; | ||
| for (const inst of plan.installs) { | ||
| const [bin, ...args] = inst.installCommand; | ||
| if (!bin) { | ||
| process.stderr.write(` skipped ${inst.skillId}: empty install command\n`); | ||
| continue; | ||
| } | ||
| process.stderr.write(`• ${inst.skillId} (${inst.sourceKind}) → ${inst.packageRef}\n`); | ||
| const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd: tempDir }); | ||
| const code = subprocessExitCode(res); | ||
| if (code === 0) { | ||
| process.stderr.write(` ✓ ${inst.skillId}\n`); | ||
| } else { | ||
| process.stderr.write(` ✗ ${inst.skillId} (exit ${code})\n`); | ||
| failures.push({ skillId: inst.skillId, exitCode: code }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Isolate each skill install instead of reusing one temp workspace.
All installers run in the same tempDir, so a partial failure can leave behind skills-lock.json, .skills/, .agents/, or other state that poisons later installs. That makes the per-skill verdicts unreliable: one bad skill can cascade into multiple false failures.
A fresh subdir per skill under the preserved dry-run root avoids cross-contamination while still leaving artifacts behind for inspection.
Suggested change
const tempDir = mkdtempSync(join(tmpdir(), `agentworkforce-dryrun-${personaId}-`));
process.stderr.write(`• temp dir: ${tempDir}\n`);
process.stderr.write(`• installing ${plan.installs.length} skill(s)\n`);
const failures: { skillId: string; exitCode: number }[] = [];
for (const inst of plan.installs) {
+ const skillDir = mkdtempSync(
+ join(tempDir, `${inst.skillId.replace(/[^a-zA-Z0-9._-]/g, '_')}-`)
+ );
const [bin, ...args] = inst.installCommand;
if (!bin) {
process.stderr.write(` skipped ${inst.skillId}: empty install command\n`);
continue;
}
process.stderr.write(`• ${inst.skillId} (${inst.sourceKind}) → ${inst.packageRef}\n`);
- const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd: tempDir });
+ const res = spawnSync(bin, args, { stdio: 'inherit', shell: false, cwd: skillDir });
const code = subprocessExitCode(res);🤖 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 `@packages/cli/src/cli.ts` around lines 925 - 945, The current dry-run uses one
shared tempDir for all installs (tempDir, loop over plan.installs), causing
state from one skill to affect others; change the loop in which you destructure
inst.installCommand and call spawnSync/subprocessExitCode so that for each inst
you create a unique subdirectory (e.g., join(tempDir, inst.skillId) or a
per-install mkdtemp) and run spawnSync with cwd set to that per-skill dir,
ensuring you still keep the top-level preserved dry-run root but isolate each
installer to avoid cross-contamination and preserve per-skill artifacts for
inspection.
Validates a persona without spawning the harness or burning tier-model tokens. Three checks, fast-fail in order: 1. Sidecar resolution — `claudeMd` / `agentsMd` filename refs are readable. 2. Harness-spec build — `buildInteractiveSpec` rejects malformed `permissions` patterns, `mcpServers` shape errors, and missing required harness fields. 3. Skill install — runs every `skills[].source` through its real installer (`npx -y skills add` / `npx -y prpm install`) inside a fresh temp dir, with per-skill pass/fail. Temp dir is removed on success and kept on failure for inspection. Motivation: a persona authored with a hallucinated skill name (source repo exists but the named skill is not in it) bricks every launch with "failed (exit 1). Aborting." The author has no way to catch this short of attempting a launch. Dry-run gives them a deterministic signal. Persona-maker's `agentsMdContent` now requires running the dry-run before handoff and lists its outcome in the output contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
willwashburn
force-pushed
the
worktree-persona-validation
branch
from
ca4c7aa to
dfef3a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agentworkforce agent <persona>@<tier> --dry-runflag validates a persona without spawning the harness or burning tier-model tokens. Three checks, fast-fail in order: (1) sidecar resolution; (2)buildInteractiveSpecshape; (3) real skill install in a temp dir.agentsMdContentnow requires running the dry-run before handoff. The output contract gained a line for the dry-run outcome.… failed (exit 1). Aborting.; the author has no way to catch it short of attempting a real launch.Behavior
On failure, exit is non-zero, the temp dir is kept and printed for inspection, and each failing skill is listed by id + exit code. On success the temp dir is removed.
What's deliberately out of scope (Tier 2/3 — not in this PR)
claude/codex/opencodeon PATH) — environment-dependent; author's machine doesn't prove the consumer's. Could land behind a--check-harnessflag later.resolveMcpServersLenientalready drops servers whose${VAR}refs are unset and warns. Same author/consumer-environment caveat. Could land behind--strict.Test plan
parseAgentArgsaccepts--dry-run(new tests; 126/126 CLI tests pass)corepack pnpm run checkgreen across the workspaceagent debugger@minimum --dry-run→✓ dry-run ok(no skills)agent capability-discoverer@minimum --dry-run→ installs skill.sh + prpm skills cleanly in a temp dir, exits 0nextjs-anti-patternsskill →✗ dry-run failed, exit 1, temp dir preservedpersona-makeragent runs the dry-run during a real authoring session and surfaces failures back to the userRelated
@agent-relay/sdk— orthogonal to this PR but the same authoring-time-validation goal.🤖 Generated with Claude Code