Minimal agent bootstrap + native Agent Skills discovery#531
Conversation
Replace ADE's ~1k-token always-on guidance blob with a minimal bootstrap and deliver ADE's capabilities as Agent Skills that each runtime discovers natively (progressive disclosure), instead of inlining a rulebook into every agent turn. - adeCliGuidance: buildAdeCliAgentGuidance now aliases the minimal buildAdeBootstrapGuidance; the verbose socket/browser/proof/VM rulebook is removed (those rules already live in their per-domain skills). - skillReseedService: materialize bundled ade-* skills into each runtime's home skill dirs (~/.claude, ~/.agents, ~/.cursor, ~/.factory, ~/.config/opencode), version-stamped, idempotent, self-healing; never touches the user's repo or their own skills. Wired into ade-cli bootstrap/cli startup and desktop adeCliService (guarded off under tests). - ade skill list|show: tamper-proof, version-locked CLI backstop served from the bundle, for surfaces that can't natively discover (e.g. cloud agents). - CTO: drop the duplicate ADE guidance injection in ctoStateService (reclaims ~1k tokens on every CTO turn). - docs: delete stale root hand-off specs (goal.md, codexGoal.md, orchestrator-next-agent-prompt.md); fix the lanes/packs vs "inspect repo directly" contradiction; gitignore future root hand-off specs. Verified end-to-end: a real headless `claude` agent natively discovered all 11 ade-* skills from the reseeded ~/.claude/skills. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. 📝 WalkthroughWalkthroughThis PR adds local ChangesADE Skill Discovery Feature and Integration
ADE Guidance System Refactoring
Supporting Changes and Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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)
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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
lanes/packs.mdx (1)
37-45: ⚡ Quick winClarify the distinction between "not prepended" and "provided to" agents.
Line 37 states these artifacts are "not context that is prepended to agent prompts," but line 41 describes the Conflict Pack as "Provided to the resolver agent." This could confuse readers about whether these packs are actually given to agents or not.
Consider clarifying that these packs are not automatically prepended to all agent prompts as ambient context, but specific packs are explicitly provided to relevant agents when needed (e.g., Conflict Pack given to resolver when handling merge conflicts).
📝 Suggested clarification
-ADE stores a small set of compatibility artifacts under `.ade/artifacts/packs/` for audit and replay. These are records of what happened — not context that is prepended to agent prompts: +ADE stores a small set of compatibility artifacts under `.ade/artifacts/packs/` for audit and replay. These are records of what happened — they are not automatically prepended to all agent prompts as ambient context, but specific packs are explicitly provided to relevant agents when needed:🤖 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 `@lanes/packs.mdx` around lines 37 - 45, Revise the paragraph to clarify that ADE artifact files under `.ade/artifacts/packs/` are not automatically prepended as ambient context to all agent prompts, but certain packs may be explicitly passed to specific agents when required (e.g., the Conflict Pack is explicitly provided to the resolver agent when handling merge conflicts); update the sentence around "not context that is prepended to agent prompts" and reference the Conflict Pack, Plan Pack, and Worker run Pack to show which artifacts are only stored vs. which may be explicitly supplied to an agent during workflows..gitignore (1)
67-70: 💤 Low valueOptional: Clarify the comment to better communicate intent.
The comment "not read at runtime" is accurate (the orchestrator reads
goal.mdfrom the lane worktree, not the repo root), but the phrasing could more clearly convey that these are ephemeral hand-off files created during setup/orchestration that shouldn't be version-controlled.📝 Alternative comment phrasing
-# Root AI hand-off specs (orchestrator/agent hand-offs, not read at runtime) +# Root AI hand-off specs (ephemeral orchestrator/agent setup files, not version-controlled)or
-# Root AI hand-off specs (orchestrator/agent hand-offs, not read at runtime) +# Root AI hand-off specs (temporary hand-off files between humans/agents/orchestrators)🤖 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 @.gitignore around lines 67 - 70, Update the comment above the /goal.md, /codexGoal.md, and /*-next-agent-prompt.md ignores to state these are ephemeral orchestration hand-off files created at setup and not intended for version control or runtime consumption by the app; replace "not read at runtime" with a clearer phrase such as "ephemeral orchestrator/agent hand-off files created during setup — not to be versioned or relied on by runtime code" so future readers understand intent and lifetime of these files.apps/desktop/src/main/services/cto/ctoStateService.ts (1)
182-182: ⚡ Quick winPotential duplication with bootstrap guidance.
Line 182 adds guidance about using the
adeCLI with--textandade help <command>, but the bootstrap guidance (included at line 108 viaADE_CLI_INLINE_GUIDANCE) already contains similar instructions: "Ground truth for anyadeinvocation isade help <command>andade actions list --text; prefer typed commands with--text."While this might be intentional reinforcement in the tool-calling context, consider whether the cross-reference phrasing "per the ADE CLI operating guidance in your doctrine" is sufficient without repeating the specific flags, or if consolidating to avoid duplication would improve clarity.
🤖 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 `@apps/desktop/src/main/services/cto/ctoStateService.ts` at line 182, The new guidance string repeats flags already present in ADE_CLI_INLINE_GUIDANCE; to avoid duplication, either remove the specific flag text ("prefer typed commands with `--text`; `ade help <command>`") from the added string and replace it with a concise cross-reference to ADE_CLI_INLINE_GUIDANCE, or reuse the ADE_CLI_INLINE_GUIDANCE constant/variable where the guidance is assembled (e.g., in the function or array that contains the string) so the message is maintained in one place; update the string in ctoStateService.ts (the added guidance line) and ensure any consumer still displays the intended ADE CLI guidance via ADE_CLI_INLINE_GUIDANCE.apps/desktop/src/shared/adeCliGuidance.ts (1)
44-52: ⚡ Quick winConsider clarifying the "don't guess at it" guidance.
Line 46 states "it is not in your training data, so don't guess at it" — this could be made more explicit about what behavior to avoid. Consider rephrasing to something like "always consult
ade help <command>rather than guessing command syntax" to make the actionable guidance clearer.🤖 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 `@apps/desktop/src/shared/adeCliGuidance.ts` around lines 44 - 52, Update the ambiguous sentence in the returned prompt array to explicitly tell users not to invent ADE commands: replace the phrase "it is not in your training data, so don't guess at it." with a clear actionable instruction such as "always consult `ade help <command>` or `ade actions list --text` instead of guessing command syntax." Modify the string near the `adeBundledAgentSkills` entry in this function and keep the surrounding items (including `formatAdeAgentSkillRootsForPrompt(skillRoots)` and the existing ground-truth line) consistent.apps/desktop/src/main/services/skills/skillReseedService.test.ts (1)
29-93: ⚡ Quick winConsider adding test coverage for multiple target directories.
All tests currently use
targetDirs: [target](single-element array), butreseedAdeSkillsaccepts and iterates over multiple target directories. Adding a test case that seeds to multiple targets (e.g.,~/.claude/skillsand~/.agents/skills) would exercise the multi-target loop and verify behavior when one target succeeds and another fails.📝 Example test structure
it("seeds to multiple target directories independently", () => { writeSkill(bundled, "ade-browser", "# browser"); const target1 = path.join(tmp, "home", ".claude", "skills"); const target2 = path.join(tmp, "home", ".agents", "skills"); const result = reseedAdeSkills({ bundledRoot: bundled, targetDirs: [target1, target2], version: "1" }); expect(result.targetsWritten).toEqual([target1, target2]); expect(fs.existsSync(path.join(target1, "ade-browser", "SKILL.md"))).toBe(true); expect(fs.existsSync(path.join(target2, "ade-browser", "SKILL.md"))).toBe(true); });🤖 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 `@apps/desktop/src/main/services/skills/skillReseedService.test.ts` around lines 29 - 93, Add a new unit test that exercises reseedAdeSkills with multiple target directories so the multi-target loop is validated: create two distinct target dirs (e.g., target1 and target2), write a bundled ADE skill (using writeSkill), call reseedAdeSkills({ bundledRoot: bundled, targetDirs: [target1, target2], version: "1" }), then assert that result.targetsWritten contains both targets and that both target1 and target2 contain the seeding (fs.existsSync checks for path.join(targetX, "ade-browser", "SKILL.md")). Reference the reseedAdeSkills call and the writeSkill helper when adding the test to skillReseedService.test.ts.
🤖 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 `@apps/ade-cli/src/cli.ts`:
- Around line 15551-15553: The call to reseedBundledAdeSkillsForCli() in main()
forces a bundled-skill scan/hash on every CLI invocation; move this work off the
global startup path by removing the call from main() and invoking
reseedBundledAdeSkillsForCli() only from the command codepaths that actually
launch agents/runtimes or need bundled skills (e.g., the agent/runtime launch
handlers such as the functions handling "runAgent"/"startRuntime"/"launch"
commands), or alternatively implement a cheap fast-path (version-stamp check)
function that returns immediately for up-to-date installs and call that from
main() while keeping the full reseedBundledAdeSkillsForCli() behind the heavier
agent/runtime launch handlers. Ensure you update references to main(),
reseedBundledAdeSkillsForCli(), and the specific command handler functions you
change so the reseed only runs when required.
In `@apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts`:
- Line 163: The test should assert the exact ADE guidance text emitted by the
prompt builders: update the expectation in the test for
buildCodingAgentSystemPrompt (when orchestrationRole: "lead") to check for the
string produced by buildAdeCliAgentGuidance -> buildAdeBootstrapGuidance, namely
"read the matching `ade-*` skill before acting; otherwise ignore them"; also
note describeAdeOrchestratorSkill still mentions ADE_AGENT_SKILLS_DIRS so keep
that reference intact while changing the expect(...) toContain(...) to match the
new guidance text.
In `@apps/desktop/src/main/services/cli/adeCliService.ts`:
- Around line 544-556: The reseedAdeSkills call currently swallows errors and
doesn't pass a build version; update the block that calls reseedAdeSkills({
bundledRoot: bundledAgentSkillsRoot }) to (1) pass the real app/package version
(e.g., reseedAdeSkills({ bundledRoot: bundledAgentSkillsRoot, version:
args.appVersion || process.env.npm_package_version || "dev" })) so packaged
builds stamp correct skill versions, and (2) catch and log failures instead of
ignoring them by calling args.logger.warn(...) (include the caught error and
contextual message) so reseed failures remain best-effort but are visible for
debugging.
---
Nitpick comments:
In @.gitignore:
- Around line 67-70: Update the comment above the /goal.md, /codexGoal.md, and
/*-next-agent-prompt.md ignores to state these are ephemeral orchestration
hand-off files created at setup and not intended for version control or runtime
consumption by the app; replace "not read at runtime" with a clearer phrase such
as "ephemeral orchestrator/agent hand-off files created during setup — not to be
versioned or relied on by runtime code" so future readers understand intent and
lifetime of these files.
In `@apps/desktop/src/main/services/cto/ctoStateService.ts`:
- Line 182: The new guidance string repeats flags already present in
ADE_CLI_INLINE_GUIDANCE; to avoid duplication, either remove the specific flag
text ("prefer typed commands with `--text`; `ade help <command>`") from the
added string and replace it with a concise cross-reference to
ADE_CLI_INLINE_GUIDANCE, or reuse the ADE_CLI_INLINE_GUIDANCE constant/variable
where the guidance is assembled (e.g., in the function or array that contains
the string) so the message is maintained in one place; update the string in
ctoStateService.ts (the added guidance line) and ensure any consumer still
displays the intended ADE CLI guidance via ADE_CLI_INLINE_GUIDANCE.
In `@apps/desktop/src/main/services/skills/skillReseedService.test.ts`:
- Around line 29-93: Add a new unit test that exercises reseedAdeSkills with
multiple target directories so the multi-target loop is validated: create two
distinct target dirs (e.g., target1 and target2), write a bundled ADE skill
(using writeSkill), call reseedAdeSkills({ bundledRoot: bundled, targetDirs:
[target1, target2], version: "1" }), then assert that result.targetsWritten
contains both targets and that both target1 and target2 contain the seeding
(fs.existsSync checks for path.join(targetX, "ade-browser", "SKILL.md")).
Reference the reseedAdeSkills call and the writeSkill helper when adding the
test to skillReseedService.test.ts.
In `@apps/desktop/src/shared/adeCliGuidance.ts`:
- Around line 44-52: Update the ambiguous sentence in the returned prompt array
to explicitly tell users not to invent ADE commands: replace the phrase "it is
not in your training data, so don't guess at it." with a clear actionable
instruction such as "always consult `ade help <command>` or `ade actions list
--text` instead of guessing command syntax." Modify the string near the
`adeBundledAgentSkills` entry in this function and keep the surrounding items
(including `formatAdeAgentSkillRootsForPrompt(skillRoots)` and the existing
ground-truth line) consistent.
In `@lanes/packs.mdx`:
- Around line 37-45: Revise the paragraph to clarify that ADE artifact files
under `.ade/artifacts/packs/` are not automatically prepended as ambient context
to all agent prompts, but certain packs may be explicitly passed to specific
agents when required (e.g., the Conflict Pack is explicitly provided to the
resolver agent when handling merge conflicts); update the sentence around "not
context that is prepended to agent prompts" and reference the Conflict Pack,
Plan Pack, and Worker run Pack to show which artifacts are only stored vs. which
may be explicitly supplied to an agent during workflows.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a30b31a9-8ec7-46a7-a5f8-c2c92fc37444
⛔ Files ignored due to path filters (4)
codexGoal.mdis excluded by!*.mdgoal.mdis excluded by!*.mdkey-concepts.mdxis excluded by!*.mdxorchestrator-next-agent-prompt.mdis excluded by!*.md
📒 Files selected for processing (19)
.gitignoreapps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/commands/skill.test.tsapps/ade-cli/src/commands/skill.tsapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/desktop/src/main/services/ai/tools/systemPrompt.test.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/cli/adeCliService.tsapps/desktop/src/main/services/cto/ctoStateService.tsapps/desktop/src/main/services/cto/ctoWorkerLifecycle.test.tsapps/desktop/src/main/services/skills/skillReseedService.test.tsapps/desktop/src/main/services/skills/skillReseedService.tsapps/desktop/src/renderer/components/terminals/cliLaunch.test.tsapps/desktop/src/shared/adeCliGuidance.test.tsapps/desktop/src/shared/adeCliGuidance.tslanes/packs.mdx
- cli.ts: gate reseedBundledAdeSkillsForCli() to agent/skill/runtime launch plans instead of every CLI invocation (off the help/--version hot path). - adeCliService.ts: pass version to reseedAdeSkills and log reseed failures via logger.warn (still best-effort). - ctoStateService / adeCliGuidance: trim duplicated CLI-flag guidance to a cross-reference; reword bootstrap to point at `ade help`. - docs: clarify lanes/packs artifact wording; reword .gitignore comment. - tests: add multi-target reseed coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@copilot review but do not make fixes |
What & why
ADE pre-injected a ~1,000-token operating-rules blob into every agent turn (and ~9,300 tokens on CTO turns, ~1,082 of them a verified duplicate). The rules (socket/browser/proof/VM) duplicated content that already lives in the bundled
ade-*Agent Skills. This collapses the always-on text to a minimal bootstrap and delivers ADE's capabilities as Agent Skills each runtime discovers natively (progressive disclosure: name+description in the index, body loaded on demand).Changes
adeCliGuidance—buildAdeCliAgentGuidancenow aliases the new minimalbuildAdeBootstrapGuidance; the verbose rulebook is removed. ~4,300 chars → ~1,050.skillReseedService(new) — materializes bundledade-*skills into each runtime's home skill dirs (~/.claude,~/.agents,~/.cursor,~/.factory,~/.config/opencode); version-stamped, idempotent, self-healing,dereference-safe. Never writes to the user's repo and never touches their own skills. Wired into ade-clibootstrap/clistartup and desktopadeCliService(guarded off under tests).ade skill list|show(new) — tamper-proof, version-locked CLI backstop served from the bundle, for surfaces that can't natively discover (e.g. cloud agents).ctoStateService(reclaims ~1,082 tokens/turn).goal.md,codexGoal.md,orchestrator-next-agent-prompt.md); fixed thelanes/packsvs "inspect repo directly" contradiction; gitignore future root hand-off specs.How it works per runtime
All five runtimes (Claude, Codex, Cursor, OpenCode, Droid) natively implement the open
SKILL.mdAgent Skills convention and read a home-level skill dir that is not in the user's repo. Re-seeding those dirs from the app bundle is the universal mechanism; updating ADE updates the skills (no stale copies).Verification
claudeagent natively listed all 11ade-*skills from the re-seeded~/.claude/skills. Re-seed populates all 5 home dirs (verified).lint-desktopclean;validate-docspasses (158 files); ade-cli builds andnode dist/cli.cjs skill list/showworks from the artifact.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
ade skill listandade skill showcommands to discover and display ADE skills with--jsonand--textoutput modes.Documentation
ade help.Chores
.gitignorefor AI specification files.Greptile Summary
This PR replaces ADE's always-on ~1,000-token system-prompt blob with a minimal bootstrap and delivers capabilities via native Agent Skills discovery. Skills are materialized from the app bundle into the five home-level skill directories each supported runtime reads, and a new
ade skill list|showCLI backstop serves the same content for runtimes that can't discover skills natively.skillReseedService: new service that idempotently copiesade-*bundled skill dirs into~/.claude/skills,~/.agents/skills,~/.cursor/skills,~/.factory/skills, and~/.config/opencode/skillsusing a sha256 content-hash + version manifest; wired into both desktopadeCliServicestartup and ade-clibootstrap/clientry points.adeCliGuidance:buildAdeCliAgentGuidancenow delegates to the newbuildAdeBootstrapGuidance(~150 tokens); the old verbose rulebook is removed.ADE_CLI_INLINE_GUIDANCEinctoStateServiceinherits the same reduction, reclaiming ~1,082 tokens per CTO turn.ade skill list|show: new local CLI commands backed by YAML frontmatter parsing and multi-root discovery; stale docs (goal.md,codexGoal.md,orchestrator-next-agent-prompt.md) are deleted.Confidence Score: 4/5
Safe to merge with one pre-existing binary-file issue to address before the next diff-review cycle.
skillReseedService.tswas committed as a binary file (git detects null bytes in the source), making the file opaque to normal PR diff tooling — this was flagged in the previous review and remains unresolved. The rest of the changes are well-tested and the logic is sound: the reseed service is idempotent and best-effort, the CLI commands are straightforward, and the token reduction is clean. A minor inconsistency exists betweenreadSkillEntries(skips symlinked skill dirs) andlistBundledAdeSkills(includes them), but this only affects plugin-layout bundles, not the current production bundle.apps/desktop/src/main/services/skills/skillReseedService.ts— committed as a binary file; the null-byte separators in source strings should be expressed as\x00escape sequences so the file is text-diffable in future PRs.Important Files Changed
buildAdeCliAgentGuidancekept as a deprecated alias pointing to the newbuildAdeBootstrapGuidance. Clean and well-documented.reseedAdeSkillscall at service initialization; guarded behindADE_DISABLE_SKILL_RESEEDandVITESTenv vars, wrapped in try/catch so it never blocks startup.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[ADE App Bundle\nagent-skills/] -->|reseedAdeSkills| B{Hash + Version\nmatches manifest?} B -->|yes - no-op| C[Home skill dirs\nalready current] B -->|no| D[cpSync with dereference\nprune removed skills\nwrite manifest] D --> E[~/.claude/skills\n~/.agents/skills\n~/.cursor/skills\n~/.factory/skills\n~/.config/opencode/skills] E -->|native discovery| F[Claude / Codex / Cursor\nOpenCode / Droid agents] A -->|ade skill list/show| G[CLI backstop\nfor cloud agents] H[Desktop adeCliService\nstartup] -->|triggers| B I[ade-cli bootstrap\ncli startup] -->|triggers| B F -->|read matching ade-* skill| J[ADE task execution] G -->|JSON / text output| JComments Outside Diff (2)
apps/desktop/src/main/services/skills/skillReseedService.ts, line 109-124 (link)skillReseedService.tscontains literal null bytes (\0) inside its string literals (the hash delimiter` \0skill:…\0`and the file-path separator"\0"). This causes git to treat the entire file as binary, which is why the PR diff only showsBinary files /dev/null and b/…skillReseedService.ts differ— the file cannot be reviewed through normal PR diff tooling.The intent (using null-byte separators so crafted filenames can't collide in the hash) is sound, but the implementation should express them as escape sequences (
\x00) rather than embedding literalU+0000bytes in the source. As-is, editors and reviewers see invisible characters that mask what the code is doing, and any future PR touching this file will again produce an unreadable binary diff.Prompt To Fix With AI
apps/desktop/src/main/services/skills/skillReseedService.ts, line 89-107 (link)walkFilesusesentry.isDirectory()/entry.isFile()(both resolved vialstat), so symlinks to files inside a skill directory are neither traversed nor hashed.cpSyncwithdereference: truewill copy those symlinked files into the user's home correctly, but the hash fingerprint won't include their content. If a bundled skill ships file symlinks internally and their target content changes within the same ADE version, the hash comparison would declare the target up-to-date and skip re-seeding — leaving users with stale skill content until either the version string changes or a non-symlinked file in the bundle changes.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: iteration 1 — address CodeRabbit r..." | Re-trigger Greptile