chore: remove todos from git tracking
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,25 +0,0 @@
|
||||
---
|
||||
status: pending
|
||||
priority: p2
|
||||
issue_id: "001"
|
||||
tags: [code-review, typescript, types]
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Extract GeminiMcpServer as named type
|
||||
|
||||
## Problem Statement
|
||||
The `GeminiBundle` inlines the MCP server type definition, and the converter uses `NonNullable<GeminiBundle["mcpServers"]>[string]` which is hard to read. Other targets (Cursor) define a named type.
|
||||
|
||||
## Findings
|
||||
- `src/types/gemini.ts` lines 20-26: inline type in GeminiBundle
|
||||
- `src/converters/claude-to-gemini.ts` line 117: `NonNullable<GeminiBundle["mcpServers"]>[string]`
|
||||
|
||||
## Proposed Solution
|
||||
Extract a named `GeminiMcpServer` type in `src/types/gemini.ts` and use it in both the bundle type and converter.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] `GeminiMcpServer` type exists in `src/types/gemini.ts`
|
||||
- [ ] `GeminiBundle.mcpServers` uses `Record<string, GeminiMcpServer>`
|
||||
- [ ] Converter uses `GeminiMcpServer` instead of indexed access type
|
||||
- [ ] Tests still pass
|
||||
@@ -1,24 +0,0 @@
|
||||
---
|
||||
status: pending
|
||||
priority: p2
|
||||
issue_id: "002"
|
||||
tags: [code-review, testing]
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Add test for uniqueName dedup when agent collides with skill
|
||||
|
||||
## Problem Statement
|
||||
The `uniqueName` function handles name collisions by appending `-2`, but there is no test covering the scenario where an agent name collides with a pass-through skill name.
|
||||
|
||||
## Findings
|
||||
- `src/converters/claude-to-gemini.ts` lines 181-193: uniqueName function
|
||||
- `tests/gemini-converter.test.ts`: no dedup test
|
||||
|
||||
## Proposed Solution
|
||||
Add a test where a plugin has both a skill named "security-reviewer" and an agent named "Security Reviewer". The generated skill should get name "security-reviewer-2".
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] Test added for agent/skill name collision
|
||||
- [ ] Test verifies the deduped name is `security-reviewer-2`
|
||||
- [ ] All tests pass
|
||||
@@ -1,24 +0,0 @@
|
||||
---
|
||||
status: pending
|
||||
priority: p2
|
||||
issue_id: "003"
|
||||
tags: [code-review, testing, security]
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Add test for TOML triple-quote escaping in prompt
|
||||
|
||||
## Problem Statement
|
||||
The `toToml` function escapes `"""` in prompts, but there is no test verifying this works correctly. This is a potential TOML injection vector.
|
||||
|
||||
## Findings
|
||||
- `src/converters/claude-to-gemini.ts` line 150: `prompt.replace(/"""/g, '\\"\\"\\"')`
|
||||
- `tests/gemini-converter.test.ts`: no triple-quote test in `toToml` describe block
|
||||
|
||||
## Proposed Solution
|
||||
Add a test in the `toToml` describe block that passes a prompt containing `"""` and verifies the output escapes it correctly.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] Test added for prompt containing `"""`
|
||||
- [ ] Escaped output does not prematurely close the TOML multi-line string
|
||||
- [ ] All tests pass
|
||||
@@ -1,25 +0,0 @@
|
||||
---
|
||||
status: pending
|
||||
priority: p2
|
||||
issue_id: "004"
|
||||
tags: [code-review, security, data-loss]
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
# Deep-merge mcpServers in settings.json instead of replacing
|
||||
|
||||
## Problem Statement
|
||||
The Gemini writer replaces the entire `mcpServers` key in `settings.json`, silently destroying any existing user MCP servers. The test name says "merges" but it actually replaces.
|
||||
|
||||
## Findings
|
||||
- `src/targets/gemini.ts` line 44: `{ ...existingSettings, mcpServers: bundle.mcpServers }`
|
||||
- `tests/gemini-writer.test.ts` line 150: test name says "merges" but asserts replacement
|
||||
|
||||
## Proposed Solution
|
||||
Deep-merge `mcpServers` entries: `{ ...existingMcp, ...bundle.mcpServers }`. Update the test to verify existing servers are preserved alongside new ones.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] Existing mcpServers entries are preserved when new ones are added
|
||||
- [ ] New entries with same name override existing (not merged at field level)
|
||||
- [ ] Test verifies both old and new servers exist after merge
|
||||
- [ ] All tests pass
|
||||
@@ -1,23 +0,0 @@
|
||||
---
|
||||
status: pending
|
||||
priority: p2
|
||||
issue_id: "005"
|
||||
tags: [code-review, error-handling]
|
||||
dependencies: ["004"]
|
||||
---
|
||||
|
||||
# Warn when existing settings.json is invalid JSON
|
||||
|
||||
## Problem Statement
|
||||
When an existing `settings.json` cannot be parsed, the error is silently swallowed and the file is overwritten. Users get no warning that their settings were discarded.
|
||||
|
||||
## Findings
|
||||
- `src/targets/gemini.ts` lines 37-41: empty catch block
|
||||
|
||||
## Proposed Solution
|
||||
Add a `console.warn` in the catch block to inform the user that their existing settings.json could not be parsed and will be replaced.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] `console.warn` emitted when settings.json parse fails
|
||||
- [ ] File is still replaced (behavior unchanged)
|
||||
- [ ] All tests pass
|
||||
Reference in New Issue
Block a user