feat(agent): add Gemini CLI install target#57
Open
merlinsantiago982-cmd wants to merge 3 commits into
Open
Conversation
merlinsantiago982-cmd
requested review from
ruili-testsprite and
zeshi-du
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR generalizes agent target installation to a multi-skill registry, adds gemini managed-section support, updates install/list behavior to be skill-aware, and revises related tests and documentation. ChangesMulti-skill agent registry and CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/agent.ts (1)
499-544: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport the composed managed-file byte count in dry-run output.
Line 534 computes
wouldBeBytes, but Line 543 still recordsbytesfrom only the managed section. For an append/replace into an existingAGENTS.md/GEMINI.md, the dry-run “would write” size under-reports the actual resulting file size.Proposed fix
- const bytes = Buffer.byteLength(section, 'utf8'); let wouldBeContent = section; @@ - dryRunLines.push({ abs, bytes, note: 'managed section' }); + dryRunLines.push({ abs, bytes: wouldBeBytes, note: 'managed section' });🤖 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/commands/agent.ts` around lines 499 - 544, The dry-run output in the agent command reports the wrong byte count because `dryRunLines.push` in `src/commands/agent.ts` still uses `bytes` instead of the computed `wouldBeBytes`. Update the dry-run record for the managed section so it reflects the final composed file size after `classifySection`/`composeManagedFile` logic, including append and replace cases. Keep the warning and result behavior unchanged, but make the reported “would write” size match the actual post-write content.
🧹 Nitpick comments (4)
src/commands/agent.ts (1)
325-330: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneralize the managed-section comments beyond Codex.
Gemini now also uses managed-section mode, so these comments should describe “managed-section targets” rather than Codex specifically.
Proposed wording cleanup
- * (`[skill]`); the codex managed-section target produces ONE result whose + * (`[skill]`); managed-section targets produce ONE result whose * section aggregates every installed skill (`[...skills]`).- // single AGENTS.md (so every codex row shares that path — truthful, since both + // single root instruction file (so managed-section rows share that path — truthful, since bothAlso applies to: 764-767
🤖 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/commands/agent.ts` around lines 325 - 330, The comments on the skill result shape in agent.ts are too Codex-specific and should be generalized to cover all managed-section targets. Update the documentation around the skills field and the related managed-section comment block (including the matching comment near the later section) so they refer to “managed-section targets” instead of “Codex,” while keeping the behavior description about own-file targets versus aggregated managed-section results.src/commands/agent.test.ts (1)
1418-1440: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Gemini repeat-install coverage for the managed block.
This new suite covers append, but the PR safety claim also depends on repeated Gemini installs replacing exactly one TestSprite block while preserving surrounding
GEMINI.mdcontent. Add a second run/assertion for exactly one begin/end sentinel and unchanged user text.🤖 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/commands/agent.test.ts` around lines 1418 - 1440, The new Gemini append test in runInstall only verifies the first install path, but it should also cover repeat-install behavior for the managed section. Extend the existing runInstall — gemini managed-section: append test to perform a second run with the same target and assert that GEMINI.md still preserves the user content while containing exactly one managed block bounded by TARGETS.gemini.managedSection!.begin and TARGETS.gemini.managedSection!.end. Use the existing runInstall, TARGETS.gemini.managedSection, and makeMemFs helpers to confirm repeated installs replace the TestSprite block instead of duplicating it.src/lib/agent-targets.test.ts (1)
694-716: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a Gemini onboard rendering assertion.
The new Gemini target is installed with
DEFAULT_SKILLS, but this suite only exercises Gemini withtestsprite-verify. Add one test fortestsprite-onboardso the sharedGEMINI.mdpath and managed-section contribution stay covered.Proposed test coverage
it('renderForTarget("gemini") without body arg uses the managed-section asset', () => { const result = renderForTarget('gemini', 'testsprite-verify'); expect(result.content).toContain('testsprite test run'); expect(result.content).not.toContain('name: testsprite-verify'); expect(result.content).not.toContain('alwaysApply:'); }); + + it('renderForTarget("gemini") supports the onboard managed-section contribution', () => { + const result = renderForTarget('gemini', 'testsprite-onboard'); + expect(result.path).toBe('GEMINI.md'); + expect(result.content).toBe(ONBOARD_CODEX_LINE); + expect(result.content).not.toContain('---'); + }); });🤖 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/lib/agent-targets.test.ts` around lines 694 - 716, Add a Gemini onboarding coverage test in the existing renderForTarget("gemini") suite so the shared GEMINI.md path is verified for testsprite-onboard as well as testsprite-verify. Reuse renderForTarget and assert that the onboard target resolves to GEMINI.md and includes the managed-section content expected from DEFAULT_SKILLS, keeping the assertion alongside the current gemini content-integrity tests.src/lib/agent-targets.ts (1)
136-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
pathFordocs to include Gemini’s shared path.The comment still says only Codex lands in a shared root file, but
geminialso ignoresskilland returnsGEMINI.md.Proposed JSDoc tweak
- * skills coexist; the codex target always lands at the single shared `AGENTS.md` - * (every skill's codex contribution is merged into one managed section there). + * skills coexist; managed-section targets land at shared root instruction files + * (`AGENTS.md` for codex, `GEMINI.md` for gemini), where every selected skill's + * contribution is merged into one managed section.🤖 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/lib/agent-targets.ts` around lines 136 - 140, The JSDoc for pathFor is outdated: it says only codex uses a shared root file, but gemini also ignores the skill and maps to a single shared file. Update the comment near pathFor in agent-targets.ts to mention both codex and gemini shared landing paths, and keep the target-specific behavior clear for own-file targets.
🤖 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 `@src/lib/agent-targets.ts`:
- Around line 322-326: The remaining renderForTarget call site is still passing
only the target, so the missing second argument is being treated as the skill id
and causing the unknown skill failure. Update the call in the agent install e2e
test to pass both the AgentTarget and the intended skill string, matching the
new renderForTarget(t, skill, body?) signature and other call sites.
---
Outside diff comments:
In `@src/commands/agent.ts`:
- Around line 499-544: The dry-run output in the agent command reports the wrong
byte count because `dryRunLines.push` in `src/commands/agent.ts` still uses
`bytes` instead of the computed `wouldBeBytes`. Update the dry-run record for
the managed section so it reflects the final composed file size after
`classifySection`/`composeManagedFile` logic, including append and replace
cases. Keep the warning and result behavior unchanged, but make the reported
“would write” size match the actual post-write content.
---
Nitpick comments:
In `@src/commands/agent.test.ts`:
- Around line 1418-1440: The new Gemini append test in runInstall only verifies
the first install path, but it should also cover repeat-install behavior for the
managed section. Extend the existing runInstall — gemini managed-section: append
test to perform a second run with the same target and assert that GEMINI.md
still preserves the user content while containing exactly one managed block
bounded by TARGETS.gemini.managedSection!.begin and
TARGETS.gemini.managedSection!.end. Use the existing runInstall,
TARGETS.gemini.managedSection, and makeMemFs helpers to confirm repeated
installs replace the TestSprite block instead of duplicating it.
In `@src/commands/agent.ts`:
- Around line 325-330: The comments on the skill result shape in agent.ts are
too Codex-specific and should be generalized to cover all managed-section
targets. Update the documentation around the skills field and the related
managed-section comment block (including the matching comment near the later
section) so they refer to “managed-section targets” instead of “Codex,” while
keeping the behavior description about own-file targets versus aggregated
managed-section results.
In `@src/lib/agent-targets.test.ts`:
- Around line 694-716: Add a Gemini onboarding coverage test in the existing
renderForTarget("gemini") suite so the shared GEMINI.md path is verified for
testsprite-onboard as well as testsprite-verify. Reuse renderForTarget and
assert that the onboard target resolves to GEMINI.md and includes the
managed-section content expected from DEFAULT_SKILLS, keeping the assertion
alongside the current gemini content-integrity tests.
In `@src/lib/agent-targets.ts`:
- Around line 136-140: The JSDoc for pathFor is outdated: it says only codex
uses a shared root file, but gemini also ignores the skill and maps to a single
shared file. Update the comment near pathFor in agent-targets.ts to mention both
codex and gemini shared landing paths, and keep the target-specific behavior
clear for own-file targets.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a616caba-7f61-492f-8194-260254a50514
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
DOCUMENTATION.mdREADME.mdsrc/commands/agent.test.tssrc/commands/agent.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.ts
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
geminiagent target that installs TestSprite guidance intoGEMINI.mdSafety
GEMINI.mduses sentinel-delimited managed-section writes, so existing project instructions outside the TestSprite block are preservedTests
npm run format:checknpm run test -- src/lib/agent-targets.test.ts src/commands/agent.test.ts test/help.snapshot.test.tsnpm run typechecknpm run lintnpm run buildSummary by CodeRabbit
testsprite test code get/putbehavior and example outputs for Python-based generated test sources.