feat(agent): add Gemini CLI as an install target#236
Conversation
Add --skip-if-configured to testsprite setup (and the deprecated init alias). When the active profile already has a saved API key and no explicit key source (--api-key or --from-env) is given, the interactive prompt is skipped and the existing key is reused. Motivation: re-running setup to refresh the agent skill -- a common pattern in dotfiles, onboarding scripts, and CI bootstraps -- currently always re-prompts for the key, even when credentials were already configured. The flag makes setup idempotent for the credential step. Behaviour: - runConfigure short-circuits with status:already_configured when skipIfConfigured is true and existingProfile.apiKey is present. - runInit relaxes the non-interactive guard and the --output json guard when skipWillApply is true, so the flag works in CI (isTTY=false) and JSON mode without requiring a separate --api-key. - --api-key always overwrites, regardless of --skip-if-configured. - --from-env always overwrites, regardless of --skip-if-configured. - --dry-run is unaffected (no network, no writes; preview only). New tests (8 cases across auth.test.ts and init.test.ts): - skips prompt and returns early when credentials exist (text + JSON) - proceeds to prompt when no credentials exist - allows isTTY=false CI runs when skip applies - --api-key overwrites despite skip flag - --from-env overwrites despite skip flag Closes TestSprite#206
WalkthroughGemini is added as a managed-section installation target using ChangesGemini agent target
Credential reuse during setup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant AgentTargets
participant GEMINImd
CLI->>AgentTargets: install target gemini
AgentTargets->>GEMINImd: resolve GEMINI.md
AgentTargets->>GEMINImd: write sentinel-delimited Markdown section
GEMINImd-->>CLI: updated installation content
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/commands/agent.ts`:
- Around line 1073-1075: Update the --force option description in the agent
command to document that both Gemini and Codex use managed sections, while
explicitly preserving the guarantee that user content outside the
managed-section sentinels is never overwritten.
In `@src/lib/agent-targets.ts`:
- Around line 254-276: Update the shared managed-section sentinel used by the
managed-section writer to a target-neutral label instead of “testsprite agent
install codex,” while preserving recognition of the existing Codex marker for
backward compatibility. Locate the sentinel constants and managed-section
parsing/writing logic, and ensure new GEMINI.md and Codex files use the neutral
marker without breaking cleanup or updates of legacy Codex files.
In `@test/e2e/agent-install.e2e.test.ts`:
- Around line 275-304: Add deterministic offline coverage around the existing
Gemini installation test by pre-populating GEMINI.md with user content before
installing both skills, then rerun installation and assert the user content
remains unchanged, exactly one MANAGED_SECTION_BEGIN/END pair exists, and only
the content between those sentinels is replaced. Reuse the existing runCli,
freshTmpDir, TARGETS, and sentinel constants, and compare the preserved
prefix/suffix or expected managed section across runs.
- Around line 275-280: The test `gemini GEMINI.md contains ONE managed section
with verify and onboard content` ignores the CLI result. Capture the `runCli`
return value, assert a zero exit status, parse `stdout` as JSON and validate the
expected response, and assert diagnostics are emitted only on `stderr` before
reading the generated file.
🪄 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 Plus
Run ID: 4af5ce26-4c1b-4c2a-88ec-1d663051e8a7
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (6)
src/commands/agent.test.tssrc/commands/agent.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.ts
| /** | ||
| * gemini target -- managed-section mode (GEMINI.md). | ||
| * | ||
| * Gemini CLI reads project-level instructions from a `GEMINI.md` file in | ||
| * the repo root. The file is plain Markdown, loaded at the start of every | ||
| * session (always-on). Because all installed skills share this single file | ||
| * we use managed-section mode -- the same approach as codex/AGENTS.md -- | ||
| * writing only a sentinel-delimited section so any existing user content | ||
| * in GEMINI.md is never clobbered. | ||
| * | ||
| * The compact body is used (same reasoning as windsurf/copilot): GEMINI.md | ||
| * is always-injected, so keeping it small reduces context cost. | ||
| * | ||
| * --force replaces the managed section unconditionally but never touches | ||
| * content outside the sentinels. | ||
| */ | ||
| gemini: { | ||
| status: 'experimental', | ||
| path: pathFor('gemini', SKILL_NAME), | ||
| mode: 'managed-section', | ||
| // GEMINI.md is plain Markdown; wrap is a no-op (no frontmatter). | ||
| wrap: (_name, _description, body) => body, | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a target-neutral managed-section sentinel.
Gemini uses the shared managed-section writer, but MANAGED_SECTION_BEGIN currently says testsprite agent install codex. A generated GEMINI.md will therefore label its managed content as Codex, which is misleading for users and future cleanup tooling. Rename the shared marker to be target-neutral while preserving compatibility with existing Codex files.
🤖 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 254 - 276, Update the shared
managed-section sentinel used by the managed-section writer to a target-neutral
label instead of “testsprite agent install codex,” while preserving recognition
of the existing Codex marker for backward compatibility. Locate the sentinel
constants and managed-section parsing/writing logic, and ensure new GEMINI.md
and Codex files use the neutral marker without breaking cleanup or updates of
legacy Codex files.
| it('gemini GEMINI.md contains ONE managed section with verify and onboard content', () => { | ||
| const tmpDir = freshTmpDir(); | ||
| runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']); | ||
|
|
||
| const filePath = join(tmpDir, TARGETS.gemini.path); | ||
| const content = readFileSync(filePath, 'utf8'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Assert the CLI exit code and JSON output.
The runCli result is ignored, so a failed install that leaves a readable file could still let this test pass. Assert a zero exit status, parse stdout as JSON, and verify diagnostics remain on stderr.
As per path instructions, tests must cover exit-code paths and preserve machine-readable JSON output discipline.
🤖 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 `@test/e2e/agent-install.e2e.test.ts` around lines 275 - 280, The test `gemini
GEMINI.md contains ONE managed section with verify and onboard content` ignores
the CLI result. Capture the `runCli` return value, assert a zero exit status,
parse `stdout` as JSON and validate the expected response, and assert
diagnostics are emitted only on `stderr` before reading the generated file.
Source: Path instructions
| it('gemini GEMINI.md contains ONE managed section with verify and onboard content', () => { | ||
| const tmpDir = freshTmpDir(); | ||
| runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']); | ||
|
|
||
| const filePath = join(tmpDir, TARGETS.gemini.path); | ||
| const content = readFileSync(filePath, 'utf8'); | ||
|
|
||
| // (a) Exactly ONE pair of sentinels | ||
| const escRe = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| const beginCount = (content.match(new RegExp(escRe(MANAGED_SECTION_BEGIN), 'g')) ?? []).length; | ||
| const endCount = (content.match(new RegExp(escRe(MANAGED_SECTION_END), 'g')) ?? []).length; | ||
| expect(beginCount, 'exactly one BEGIN sentinel').toBe(1); | ||
| expect(endCount, 'exactly one END sentinel').toBe(1); | ||
|
|
||
| // BEGIN must come before END | ||
| expect(content.indexOf(MANAGED_SECTION_BEGIN)).toBeLessThan( | ||
| content.indexOf(MANAGED_SECTION_END), | ||
| ); | ||
|
|
||
| // (b) Load-bearing command strings from the compact verify body | ||
| expect(content).toContain('testsprite test run'); | ||
| expect(content).toContain('--wait'); | ||
| expect(content).toContain('test artifact get'); | ||
|
|
||
| // (c) Onboard one-liner must be present | ||
| expect(content).toContain('First-time setup'); | ||
|
|
||
| // (d) No frontmatter fence -- GEMINI.md is plain prose | ||
| expect(content.startsWith('---'), 'gemini: must NOT start with ---').toBe(false); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cover managed-section preservation and reruns.
This test starts from an empty GEMINI.md and verifies only a single install. Add coverage that pre-populates user content outside the sentinels, installs both skills, then reruns installation and asserts that user content remains intact, exactly one section exists, and only the managed section is replaced.
As per path instructions, tests must cover the new behavior deterministically and offline.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 283-283: Do not use variable for regular expressions
Context: new RegExp(escRe(MANAGED_SECTION_BEGIN), 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 284-284: Do not use variable for regular expressions
Context: new RegExp(escRe(MANAGED_SECTION_END), 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🤖 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 `@test/e2e/agent-install.e2e.test.ts` around lines 275 - 304, Add deterministic
offline coverage around the existing Gemini installation test by pre-populating
GEMINI.md with user content before installing both skills, then rerun
installation and assert the user content remains unchanged, exactly one
MANAGED_SECTION_BEGIN/END pair exists, and only the content between those
sentinels is replaced. Reuse the existing runCli, freshTmpDir, TARGETS, and
sentinel constants, and compare the preserved prefix/suffix or expected managed
section across runs.
Source: Path instructions
zeshi-du
left a comment
zeshi-du
left a comment
There was a problem hiding this comment.
Gemini target accepted — and the managed-section choice (single root GEMINI.md, mirroring the codex/AGENTS.md pattern) is the correct call for Gemini CLI conventions; the PR body's rationale for deviating from the copilot own-file shape is exactly right. Thin diff following the TARGETS pattern, full matrix coverage in unit + e2e + help snapshots. This supersedes the recovered #247 (closed with credit to its original author). Thanks @Tasfia-17!
|
One-step rebase needed: your other PR #234 merged first and they overlap in the help-snapshot / e2e suites, so this went conflicting. Approval stands — rebase onto current main (re-running the snapshot update if needed) and it merges straight away. |
Adds `gemini` to the supported agent-install targets. Gemini CLI reads project-level context from a GEMINI.md file in the repo root, which is loaded at the start of every session (always-on, no on-demand mode). Because all installed skills share a single GEMINI.md file in the project root, the target uses managed-section mode -- the same approach as the codex/AGENTS.md target -- writing only a sentinel-delimited section so any existing user content in GEMINI.md is never clobbered. The landing path is GEMINI.md (repo root). Both testsprite-verify and testsprite-onboard are aggregated into one managed section, consistent with how codex aggregates skills into AGENTS.md. Slots into the existing TARGETS machinery: agent list, setup --agent, and skill-nudge install-detection all pick it up automatically. Updates the AgentTarget union, pathFor, TARGETS, agent command description, help text, unit tests, e2e matrix guards, and the help snapshot. Contributes to CONTRIBUTING.md accepted target: gemini
54df468 to
6eeae99
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
test/e2e/agent-install.e2e.test.ts (2)
277-277: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert the CLI exit code and JSON output.
The
runCliresult is still ignored, meaning a failed install could pass the test if a readable file is left behind. Capture therunClireturn value, assert a zero exit status, and parsestdoutas JSON to validate the expected response. As per path instructions, tests must cover exit-code paths and maintain JSON output discipline.🐛 Proposed fix to assert CLI results
- runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']); + const result = runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']); + expect(result.status).toBe(0); + expect(() => JSON.parse(result.stdout)).not.toThrow();🤖 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 `@test/e2e/agent-install.e2e.test.ts` at line 277, Update the agent install test around runCli to capture its result, assert that the CLI exits successfully, and parse stdout as JSON to validate the expected response. Keep the existing file verification, but ensure the test covers both exit-code handling and the command’s JSON output.Source: Path instructions
275-304: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftCover managed-section preservation and reruns.
The current test validates a fresh installation but does not verify the managed-section behavior. As per path instructions,
DOCUMENTATION.mdspecifies that in managed-section mode,agent installwrites a sentinel-delimited section while always preserving user content outside the sentinels, and re-running replaces that section in-place. Please add a test or expand this one to pre-populateGEMINI.mdwith user content, run the installation, and assert that the user content remains unchanged outside the sentinels.🤖 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 `@test/e2e/agent-install.e2e.test.ts` around lines 275 - 304, Expand the Gemini installation test around the existing `runCli` and sentinel assertions to pre-populate GEMINI.md with user content, run `agent install`, and verify that content outside `MANAGED_SECTION_BEGIN` and `MANAGED_SECTION_END` remains unchanged. Also rerun installation and assert the managed section is replaced in place without duplicating sentinels, while preserving the existing content and command assertions.Source: Path instructions
🧹 Nitpick comments (2)
test/e2e/agent-install.e2e.test.ts (1)
283-285: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify string counting.
Using dynamically constructed regular expressions triggers static analysis warnings for potentially inefficient regex complexity. You can simplify this and avoid escaping logic entirely by using
String.prototype.split.♻️ Proposed refactor
- const escRe = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const beginCount = (content.match(new RegExp(escRe(MANAGED_SECTION_BEGIN), 'g')) ?? []).length; - const endCount = (content.match(new RegExp(escRe(MANAGED_SECTION_END), 'g')) ?? []).length; + const beginCount = content.split(MANAGED_SECTION_BEGIN).length - 1; + const endCount = content.split(MANAGED_SECTION_END).length - 1;🤖 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 `@test/e2e/agent-install.e2e.test.ts` around lines 283 - 285, Replace the dynamic RegExp-based counting in the managed-section test with String.prototype.split using MANAGED_SECTION_BEGIN and MANAGED_SECTION_END directly. Remove the escRe helper and preserve the existing beginCount and endCount values by deriving each count from the resulting split-array length.Source: Linters/SAST tools
src/commands/init.ts (1)
207-210: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify profile reading.
You can eliminate the ternary by passing
deps.credentialsPathdirectly toreadProfile, asoptions.pathnatively falls back to the default when it isundefined.♻️ Proposed refactor
- const credentialsPath = deps.credentialsPath; - const savedKey = credentialsPath - ? readProfile(opts.profile, { path: credentialsPath })?.apiKey - : readProfile(opts.profile)?.apiKey; + const savedKey = readProfile(opts.profile, { path: deps.credentialsPath })?.apiKey;🤖 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/init.ts` around lines 207 - 210, Update the savedKey initialization in the profile-reading flow to call readProfile once with deps.credentialsPath passed as the options.path value, relying on readProfile’s native default when it is undefined; remove the intermediate credentialsPath variable and ternary while preserving opts.profile and .apiKey access.
🤖 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/commands/auth.ts`:
- Around line 145-151: Update the early-return branch in the skipIfConfigured
flow to report the endpoint actually persisted in existingProfile rather than
the override-resolved apiUrl. Fall back to the configured default only when the
saved profile has no endpoint, while preserving the existing already_configured
output and avoiding any profile write.
In `@src/commands/init.ts`:
- Around line 239-246: Update the key-source reporting expression in the init
command to use the computed skipWillApply boolean instead of
opts.skipIfConfigured. Preserve the existing precedence for apiKey, fromEnv, and
prompt so dry-run output matches the runtime behavior when no saved key exists.
---
Duplicate comments:
In `@test/e2e/agent-install.e2e.test.ts`:
- Line 277: Update the agent install test around runCli to capture its result,
assert that the CLI exits successfully, and parse stdout as JSON to validate the
expected response. Keep the existing file verification, but ensure the test
covers both exit-code handling and the command’s JSON output.
- Around line 275-304: Expand the Gemini installation test around the existing
`runCli` and sentinel assertions to pre-populate GEMINI.md with user content,
run `agent install`, and verify that content outside `MANAGED_SECTION_BEGIN` and
`MANAGED_SECTION_END` remains unchanged. Also rerun installation and assert the
managed section is replaced in place without duplicating sentinels, while
preserving the existing content and command assertions.
---
Nitpick comments:
In `@src/commands/init.ts`:
- Around line 207-210: Update the savedKey initialization in the profile-reading
flow to call readProfile once with deps.credentialsPath passed as the
options.path value, relying on readProfile’s native default when it is
undefined; remove the intermediate credentialsPath variable and ternary while
preserving opts.profile and .apiKey access.
In `@test/e2e/agent-install.e2e.test.ts`:
- Around line 283-285: Replace the dynamic RegExp-based counting in the
managed-section test with String.prototype.split using MANAGED_SECTION_BEGIN and
MANAGED_SECTION_END directly. Remove the escRe helper and preserve the existing
beginCount and endCount values by deriving each count from the resulting
split-array length.
🪄 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 Plus
Run ID: 93ca362b-59a1-4072-aadc-765d3fa6e55c
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (10)
src/commands/agent.test.tssrc/commands/agent.tssrc/commands/auth.test.tssrc/commands/auth.tssrc/commands/init.test.tssrc/commands/init.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/commands/agent.ts
- src/commands/agent.test.ts
- src/lib/agent-targets.test.ts
- src/lib/agent-targets.ts
| if (opts.skipIfConfigured && existingProfile?.apiKey) { | ||
| out.print({ profile: opts.profile, apiUrl, status: 'already_configured' }, data => { | ||
| const d = data as { profile: string; apiUrl: string }; | ||
| return `Profile "${d.profile}" already configured. Endpoint: ${d.apiUrl}`; | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report the actual saved endpoint when skipping configuration.
When returning early due to --skip-if-configured, the apiUrl variable includes overrides from opts.endpointUrl and envApiUrl. Because writeProfile is intentionally skipped on this path, those overrides are never persisted to disk. Passing them to out.print creates misleading output (e.g., claiming the profile is already configured with a new endpoint that hasn't actually been saved).
Consider reporting the endpoint that is actually stored in the profile (or the default) to accurately reflect the preserved state.
💚 Proposed fix
- if (opts.skipIfConfigured && existingProfile?.apiKey) {
- out.print({ profile: opts.profile, apiUrl, status: 'already_configured' }, data => {
- const d = data as { profile: string; apiUrl: string };
- return `Profile "${d.profile}" already configured. Endpoint: ${d.apiUrl}`;
- });
- return;
- }
+ if (opts.skipIfConfigured && existingProfile?.apiKey) {
+ const activeApiUrl = existingProfile.apiUrl ?? DEFAULT_API_URL;
+ out.print({ profile: opts.profile, apiUrl: activeApiUrl, status: 'already_configured' }, data => {
+ const d = data as { profile: string; apiUrl: string };
+ return `Profile "${d.profile}" already configured. Endpoint: ${d.apiUrl}`;
+ });
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (opts.skipIfConfigured && existingProfile?.apiKey) { | |
| out.print({ profile: opts.profile, apiUrl, status: 'already_configured' }, data => { | |
| const d = data as { profile: string; apiUrl: string }; | |
| return `Profile "${d.profile}" already configured. Endpoint: ${d.apiUrl}`; | |
| }); | |
| return; | |
| } | |
| if (opts.skipIfConfigured && existingProfile?.apiKey) { | |
| const activeApiUrl = existingProfile.apiUrl ?? DEFAULT_API_URL; | |
| out.print({ profile: opts.profile, apiUrl: activeApiUrl, status: 'already_configured' }, data => { | |
| const d = data as { profile: string; apiUrl: string }; | |
| return `Profile "${d.profile}" already configured. Endpoint: ${d.apiUrl}`; | |
| }); | |
| return; | |
| } |
🤖 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/auth.ts` around lines 145 - 151, Update the early-return branch
in the skipIfConfigured flow to report the endpoint actually persisted in
existingProfile rather than the override-resolved apiUrl. Fall back to the
configured default only when the saved profile has no endpoint, while preserving
the existing already_configured output and avoiding any profile write.
| opts.apiKey | ||
| ? 'flag' | ||
| : opts.fromEnv | ||
| ? 'env' | ||
| : opts.skipIfConfigured | ||
| ? 'skip-if-configured' | ||
| : 'prompt' | ||
| })`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use skipWillApply for accurate dry-run reporting.
In --dry-run mode, the reported key source uses opts.skipIfConfigured directly. However, if the flag is passed but no saved key exists, the actual run will fall back to prompting. Using the already computed skipWillApply boolean ensures the dry-run preview accurately reflects the runtime behavior.
💚 Proposed fix
`[dry-run] would configure profile="${opts.profile}" (key source: ${
opts.apiKey
? 'flag'
: opts.fromEnv
? 'env'
- : opts.skipIfConfigured
+ : skipWillApply
? 'skip-if-configured'
: 'prompt'
})`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| opts.apiKey | |
| ? 'flag' | |
| : opts.fromEnv | |
| ? 'env' | |
| : opts.skipIfConfigured | |
| ? 'skip-if-configured' | |
| : 'prompt' | |
| })`, | |
| opts.apiKey | |
| ? 'flag' | |
| : opts.fromEnv | |
| ? 'env' | |
| : skipWillApply | |
| ? 'skip-if-configured' | |
| : 'prompt' | |
| })`, |
🤖 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/init.ts` around lines 239 - 246, Update the key-source reporting
expression in the init command to use the computed skipWillApply boolean instead
of opts.skipIfConfigured. Preserve the existing precedence for apiKey, fromEnv,
and prompt so dry-run output matches the runtime behavior when no saved key
exists.
2 participants
Summary
CONTRIBUTING.md explicitly lists
geminias an accepted, in-progresstarget with no existing PR. This adds it.
Gemini CLI reads project-level instructions from a
GEMINI.mdfile inthe repo root, loaded at the start of every session (always-on, no
on-demand mode). Because all installed skills share a single root file,
the target uses managed-section mode -- the same approach as the
codex/AGENTS.md target -- writing only a sentinel-delimited section so
existing user content in GEMINI.md is never clobbered.
Changes
src/lib/agent-targets.ts-- AddedgeminitoAgentTargetunion,GEMINI.mdcase inpathFor, and ageminientry inTARGETSusingmanaged-sectionmode.src/lib/agent-targets.test.ts-- Updated key count, experimentalstatus, managed-section assertions, and a dedicated
renderForTarget("gemini")describe block.src/commands/agent.ts-- Updated command description and--targethelp text to include gemini.
src/commands/agent.test.ts-- UpdatedrunListrow count to reflectthe new target.
test/e2e/agent-install.e2e.test.ts-- Updated matrix guard, addedgemini content assertions, added dedicated GEMINI.md managed-section
test.
test/e2e/setup.e2e.test.ts-- Updated matrix guard.test/__snapshots__/help.snapshot.test.ts.snap-- Updated 4 snapshots.Design decisions
testsprite-verifyandtestsprite-onboardresolve to the sameGEMINI.mdpath. Usingown-file mode causes a conflict error on the second skill write.
managed-section aggregates all skills into one sentinel-delimited
block, consistent with how codex handles AGENTS.md.
is a no-op, matching codex.
Tests
All 1865 tests pass. Coverage stays above 80%.
Closes #235
Summary by CodeRabbit
New Features
GEMINI.mdas plain Markdown within managed sections.--skip-if-configuredsupport toinit/setupflows to avoid prompting when an API key is already saved.Tests
skip-if-configuredbehavior in interactive and non-interactive scenarios.