fix(geminicli): generate subagents to .gemini/agents/ as native (not simulated)#1409
Conversation
…rimental.enableAgents Gemini CLI expects subagent files in .gemini/agents/, not .gemini/subagents/. Also injects `experimental.enableAgents: true` into .gemini/settings.json when generating Gemini CLI subagents, which is required for the agents feature to work. Existing settings are preserved during injection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Also fix lint-staged tsx invocation to use node --import tsx/esm to avoid IPC socket creation which is unnecessary for one-off script execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Guard settings injection behind toolFiles.length > 0 to avoid touching .gemini/settings.json when no subagents are actually generated - Wrap JSON.parse in try/catch with logger.warn fallback for malformed or JSONC settings files - Strengthen E2E test to assert exact mcpServers value (not just defined) - Add E2E test asserting other experimental flags are preserved on merge - Remove stale **/.gemini/subagents/ entry from .gitignore Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add postGenerate hook to FeatureProcessor and DirFeatureProcessor base classes so tool-specific side effects can be encapsulated in the relevant processor rather than as string-switch cases in generate.ts. Move the experimental.enableAgents injection for Gemini CLI out of generateSubagentsCore() in generate.ts into SubagentsProcessor.postGenerate() where it belongs architecturally, consistent with how OpenCode handles its mode logic. Also clean up supportsSimulated meta flag — all entries are now false since the SimulatedSubagent instanceof check is the authoritative gate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lgaudez
left a comment
There was a problem hiding this comment.
Remove this postGenerate everywhere. We want the user of rulesync to be able to say where he wants his subagents folder. Note that if subagents folder was used by rulesync, we should not break any behaviour they had
… hook" This reverts commit e5f5e30.
GeminiCliSubagent extends SimulatedSubagent so --simulate-subagents is required for generation. Add simulateSubagents param to runGenerate helper and use it in all geminicli-specific E2E tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ableAgents injection - GeminiCliSubagent now extends ToolSubagent directly (native generation) instead of SimulatedSubagent, enabling generation without --simulate-subagents - Remove automatic injection of experimental.enableAgents into .gemini/settings.json; users should set this manually per the updated docs note - Update subagents-processor: supportsSimulated false for geminicli - Update README and docs: geminicli subagents column 🎮 → ✅ - Clean up E2E tests: remove simulateSubagents flag and obsolete enableAgents tests - Update unit tests to reflect native toRulesyncSubagent() support Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dyoshikawa
left a comment
There was a problem hiding this comment.
Solid refactoring overall — converting GeminiCliSubagent from SimulatedSubagent to a native ToolSubagent is clean and well-tested. The path change to .gemini/agents/ and the docs/README updates are consistent.
Two things worth addressing: fromRulesyncSubagent and fromFile both skip destructuring/forwarding the global parameter, which every other native subagent (ClaudeCode, Cursor, Copilot) does. While supportsGlobal is false for geminicli today, the inconsistency will cause a subtle bug if global support is ever added.
The .lintstagedrc.js change (tsx -> node --import tsx/esm) seems unrelated to this PR's scope — might be worth calling out in the description or splitting into its own commit.
No security concerns found.
| static fromRulesyncSubagent({ | ||
| baseDir = process.cwd(), | ||
| rulesyncSubagent, | ||
| validate = true, |
There was a problem hiding this comment.
Missing global = false destructuring here. All other native subagents (ClaudecodeSubagent, CursorSubagent, CopilotSubagent) destructure global from ToolSubagentFromRulesyncSubagentParams and forward it to getSettablePaths({ global }) and the constructor. Without it, if global support is ever added for geminicli, this method will silently ignore the flag.
| validate = true, | |
| static fromRulesyncSubagent({ | |
| baseDir = process.cwd(), | |
| rulesyncSubagent, | |
| validate = true, | |
| global = false, | |
| }: ToolSubagentFromRulesyncSubagentParams): ToolSubagent { |
| static async fromFile({ | ||
| baseDir = process.cwd(), | ||
| relativeFilePath, | ||
| validate = true, |
There was a problem hiding this comment.
Same issue as fromRulesyncSubagent — global is not destructured or forwarded to getSettablePaths(). Other native subagents pass this.getSettablePaths({ global }) here.
| validate = true, | |
| static async fromFile({ | |
| baseDir = process.cwd(), | |
| relativeFilePath, | |
| validate = true, | |
| global = false, | |
| }: ToolSubagentFromFileParams): Promise<GeminiCliSubagent> { | |
| const paths = this.getSettablePaths({ global }); |
| "*": ["npx secretlint"], | ||
| "package.json": ["npx sort-package-json"], | ||
| "docs/**/*.md": ["tsx scripts/sync-skill-docs.ts", "git add skills/rulesync/"], | ||
| "docs/**/*.md": ["node --import tsx/esm scripts/sync-skill-docs.ts", "git add skills/rulesync/"], |
There was a problem hiding this comment.
This change from tsx to node --import tsx/esm looks unrelated to the geminicli subagents fix. Consider splitting it into its own commit or at least noting it in the PR description so it does not get lost in the diff.
There was a problem hiding this comment.
Regarding the .lintstagedrc.js change: this was introduced because the tsx CLI binary creates a Unix IPC socket at /tmp/tsx-*/...pipe during execution. In my local environment (Claude Code's sandbox), access to /tmp/ is restricted, which caused the pre-commit hook to fail on every commit attempt.
Switching to node --import tsx/esm avoids the socket entirely and unblocked the hook for me. That said, this is specific to my sandbox setup — tsx works fine in a normal terminal and the change has no effect for other contributors.
Happy to revert it if you'd prefer to keep the diff clean. Alternatively, node --import tsx/esm is slightly more explicit and avoids spawning an extra subprocess, so it could be kept as a minor improvement. Your call.
…, date docs note - Add global = false destructuring in fromRulesyncSubagent and fromFile, forwarded to getSettablePaths() and the constructor for consistency with other native subagents (ClaudeCode, Cursor, Copilot) - Add date to Gemini CLI note in file-formats.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@lgaudez Thank you! |
There was a problem hiding this comment.
Pull request overview
Updates Gemini CLI subagent support to generate native agent files in the location Gemini CLI expects, and aligns documentation/E2E coverage with the new behavior.
Changes:
- Generate Gemini CLI subagents to
.gemini/agents/and treatgeminiclias a native (non-simulated) subagent target. - Add/adjust unit + E2E coverage for Gemini CLI subagent generation and conversion to Rulesync subagents.
- Update gitignore entries, docs/README support tables, and lint-staged docs sync command.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/features/subagents/subagents-processor.ts | Marks geminicli as non-simulated so it participates in native subagent generation/conversion. |
| src/features/subagents/subagents-processor.test.ts | Updates simulated-target expectations to reflect geminicli now being native. |
| src/features/subagents/geminicli-subagent.ts | Reimplements Gemini CLI subagent as a native ToolSubagent and switches output dir to .gemini/agents/. |
| src/features/subagents/geminicli-subagent.test.ts | Updates/extends tests for native behavior (including toRulesyncSubagent() and new paths). |
| src/e2e/e2e-subagents.spec.ts | Adds geminicli to the E2E matrix verifying subagent generation output paths. |
| src/e2e/e2e-helper.ts | Adds optional --simulate-subagents wiring for E2E runner (default off). |
| src/cli/commands/gitignore-entries.ts | Updates Gemini CLI subagent ignore entry to .gemini/agents/. |
| skills/rulesync/supported-tools.md | Marks Gemini CLI subagents as supported (✅) in skills docs. |
| skills/rulesync/file-formats.md | Adds Gemini CLI note about .gemini/agents/ and manual enableAgents setting. |
| README.md | Updates support table to reflect Gemini CLI subagents as supported (✅). |
| docs/reference/supported-tools.md | Updates reference support table to reflect Gemini CLI subagents as supported (✅). |
| docs/reference/file-formats.md | Adds the same Gemini CLI note as in skills docs. |
| .lintstagedrc.js | Switches docs sync hook from tsx CLI to node --import tsx/esm .... |
| .gitignore | Updates ignored Gemini CLI subagent directory to .gemini/agents/. |
| return new GeminiCliSubagent({ | ||
| baseDir, | ||
| relativeDirPath: paths.relativeDirPath, | ||
| relativeFilePath: basename(relativeFilePath), |
There was a problem hiding this comment.
fromFile() reads the subagent from relativeFilePath (which may include subdirectories), but then stores only basename(relativeFilePath) on the instance. This makes the resulting GeminiCliSubagent no longer represent the actual file path that was loaded (e.g., getFilePath()/logging will point to .gemini/agents/nested-agent.md even if the file was .gemini/agents/subdir/nested-agent.md). Consider preserving the full relativeFilePath like other native subagents (e.g. CursorSubagent.fromFile) or, if Gemini CLI truly disallows subdirectories, reject/normalize earlier and keep behavior consistent across load/write paths.
| relativeFilePath: basename(relativeFilePath), | |
| relativeFilePath, |
| const subagent = new GeminiCliSubagent({ | ||
| baseDir: testDir, | ||
| relativeDirPath: ".gemini/subagents", | ||
| relativeDirPath: ".gemini/agents", |
There was a problem hiding this comment.
Tests pass filesystem paths as hardcoded POSIX strings (e.g. ".gemini/agents"). Since the implementation uses join() for these paths (and the project aims to support Windows paths), these should ideally use join(".gemini", "agents") as well to avoid platform-dependent behavior.
| relativeDirPath: ".gemini/agents", | |
| relativeDirPath: join(".gemini", "agents"), |
Summary
.gemini/subagents/→.gemini/agents/(where Gemini CLI actually expects them)GeminiCliSubagenta native subagent (extendsToolSubagentdirectly) instead of simulated — no--simulate-subagentsflag neededexperimental.enableAgentsinto.gemini/settings.json— users should set this manually"experimental": { "enableAgents": true }in.gemini/settings.jsonmanuallyglobalparam infromRulesyncSubagentandfromFilefor consistency with other native subagents🎮→✅Unrelated change included
.lintstagedrc.js: changedtsx scripts/sync-skill-docs.ts→node --import tsx/esm scripts/sync-skill-docs.ts. This was needed because thetsxCLI creates a Unix IPC socket at/tmp/tsx-*/...pipewhich was blocked by the sandbox during development. Included here since it was required to run the pre-commit hook locally.Test plan
GeminiCliSubagentupdated to reflect native behavior (toRulesyncSubagent()now works)geminiclisubagents generate to.gemini/agents/planner.mdwithout--simulate-subagentspnpm cicheckpasses (174 test files, 4603 tests)🤖 Generated with Claude Code