-
Notifications
You must be signed in to change notification settings - Fork 961
feat(iflow-cli): add iFlow-cli integration #268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds iFlow support: new AI tool entry, an Iflow ToolConfigurator, an Iflow SlashCommandConfigurator, registration of both configurators in their registries, README documentation update, and new tests for init/update behaviors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ToolRegistry
participant IflowConfigurator
participant TemplateManager
participant FileSystemUtils
User->>ToolRegistry: request configure 'iflow' (projectPath, openspecDir)
ToolRegistry->>IflowConfigurator: configure(...)
IflowConfigurator->>TemplateManager: getClaudeTemplate('iflow' / IFLOW)
TemplateManager-->>IflowConfigurator: template content
IflowConfigurator->>FileSystemUtils: updateFileWithMarkers(targetPath(IFLOW.md), OPENSPEC_MARKERS, content)
FileSystemUtils-->>IflowConfigurator: file created/updated
IflowConfigurator-->>ToolRegistry: done
ToolRegistry-->>User: success
sequenceDiagram
autonumber
participant User
participant SlashRegistry
participant IflowSlashConfigurator
participant FileSystemUtils
User->>SlashRegistry: create/update slash command (proposal/apply/archive)
SlashRegistry->>IflowSlashConfigurator: getRelativePath(id) / getFrontmatter(id)
IflowSlashConfigurator->>FileSystemUtils: ensure file at .iflow/commands/... with frontmatter & template body
FileSystemUtils-->>IflowSlashConfigurator: file ensured/updated
IflowSlashConfigurator-->>SlashRegistry: configured
SlashRegistry-->>User: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/core/config.ts(1 hunks)src/core/configurators/iflow.ts(1 hunks)src/core/configurators/registry.ts(2 hunks)src/core/configurators/slash/iflow.ts(1 hunks)src/core/configurators/slash/registry.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/core/configurators/iflow.ts (5)
src/core/configurators/base.ts (1)
ToolConfigurator(1-6)src/core/templates/index.ts (1)
TemplateManager(14-47)src/utils/file-system.ts (1)
FileSystemUtils(44-187)src/core/config.ts (1)
OPENSPEC_MARKERS(3-6)src/core/configurators/claude.ts (1)
ClaudeConfigurator(7-23)
src/core/configurators/registry.ts (2)
src/core/configurators/iflow.ts (1)
IflowConfigurator(7-23)src/core/configurators/base.ts (1)
ToolConfigurator(1-6)
src/core/configurators/slash/registry.ts (1)
src/core/configurators/slash/iflow.ts (1)
IflowSlashCommandConfigurator(31-42)
| const content = TemplateManager.getClaudeTemplate(); | ||
|
|
||
| await FileSystemUtils.updateFileWithMarkers( | ||
| filePath, | ||
| content, | ||
| OPENSPEC_MARKERS.start, | ||
| OPENSPEC_MARKERS.end | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use an iFlow-specific template for IFLOW.md content.
Lines 14-21 call TemplateManager.getClaudeTemplate(), which writes Claude-specific guidance into IFLOW.md. That gives users the wrong setup steps for iFlow. Please add an iFlow template (e.g., TemplateManager.getIflowTemplate()) and use it here so the configurator emits the correct instructions.
🤖 Prompt for AI Agents
In src/core/configurators/iflow.ts around lines 14 to 21, the configurator is
using TemplateManager.getClaudeTemplate() which injects Claude-specific guidance
into IFLOW.md; replace that call with TemplateManager.getIflowTemplate() so the
file contains iFlow-specific instructions, and if getIflowTemplate doesn't exist
add it to TemplateManager (or import the correct iFlow template function)
returning the proper iFlow content; keep the subsequent
FileSystemUtils.updateFileWithMarkers call unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while ClaudeTemplate also works for iflow or other cli.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ydai-hub, thank you for clarifying! I understand now that the Claude template is generic enough to work with iFlow and other CLI tools, so using getClaudeTemplate() here is appropriate.
✏️ Learnings added
Learnt from: ydai-hub
Repo: Fission-AI/OpenSpec PR: 268
File: src/core/configurators/iflow.ts:14-21
Timestamp: 2025-11-17T12:07:21.674Z
Learning: In the OpenSpec repository, `TemplateManager.getClaudeTemplate()` returns a generic template that works for iFlow and other CLI tools, not just Claude-specific content. It's acceptable to use this template across different tool configurators.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Could someone review this? |
|
This PR could update the 'Supported AI Tools' section in README.md and also add iflow to Native Slash Commands. |
…sion-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation.
done |
…sion-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation.
…sion-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation.
…sion-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(1 hunks)src/core/config.ts(1 hunks)src/core/configurators/iflow.ts(1 hunks)src/core/configurators/registry.ts(3 hunks)src/core/configurators/slash/iflow.ts(1 hunks)src/core/configurators/slash/registry.ts(3 hunks)test/core/init.test.ts(1 hunks)test/core/update.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/core/configurators/slash/registry.ts
- README.md
- src/core/configurators/slash/iflow.ts
- src/core/config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-17T12:07:21.701Z
Learnt from: ydai-hub
Repo: Fission-AI/OpenSpec PR: 268
File: src/core/configurators/iflow.ts:14-21
Timestamp: 2025-11-17T12:07:21.701Z
Learning: In the OpenSpec repository, `TemplateManager.getClaudeTemplate()` returns a generic template that works for iFlow and other CLI tools, not just Claude-specific content. It's acceptable to use this template across different tool configurators.
Applied to files:
src/core/configurators/iflow.ts
🧬 Code graph analysis (4)
src/core/configurators/registry.ts (2)
src/core/configurators/iflow.ts (1)
IflowConfigurator(7-23)src/core/configurators/base.ts (1)
ToolConfigurator(1-6)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
fileExists(76-86)
test/core/update.test.ts (1)
src/utils/file-system.ts (1)
FileSystemUtils(44-187)
src/core/configurators/iflow.ts (4)
src/core/configurators/base.ts (1)
ToolConfigurator(1-6)src/core/templates/index.ts (1)
TemplateManager(14-47)src/utils/file-system.ts (1)
FileSystemUtils(44-187)src/core/config.ts (1)
OPENSPEC_MARKERS(3-6)
🔇 Additional comments (3)
src/core/configurators/iflow.ts (1)
1-23: LGTM!The IflowConfigurator implementation is clean and consistent with other tool configurators. The unused
openspecDirparameter is acceptable since IFLOW.md is written to the project root, matching the pattern of CLAUDE.md and CLINE.md.Based on learnings.
src/core/configurators/registry.ts (1)
7-7: LGTM!The IflowConfigurator registration follows the established pattern consistently with other tool registrations.
Also applies to: 20-20, 29-29
test/core/init.test.ts (1)
456-471: LGTM!The test correctly validates that existing IFLOW.md is updated with OpenSpec markers while preserving custom content.
| it('should refresh existing IFLOW slash commands', async () => { | ||
| const iflowProposal = path.join( | ||
| testDir, | ||
| '.iflow/commands/openspec-proposal.md' | ||
| ); | ||
| await fs.mkdir(path.dirname(iflowProposal), { recursive: true }); | ||
| const initialContent = `description: Scaffold a new OpenSpec change and validate strictly." | ||
| prompt = """ | ||
| <!-- OPENSPEC:START --> | ||
| Old IFlow body | ||
| <!-- OPENSPEC:END --> | ||
| """ | ||
| `; | ||
| await fs.writeFile(iflowProposal, initialContent); | ||
|
|
||
| const consoleSpy = vi.spyOn(console, 'log'); | ||
|
|
||
| await updateCommand.execute(testDir); | ||
|
|
||
| const updated = await fs.readFile(iflowProposal, 'utf-8'); | ||
| expect(updated).toContain('description: Scaffold a new OpenSpec change and validate strictly.'); | ||
| expect(updated).toContain('<!-- OPENSPEC:START -->'); | ||
| expect(updated).toContain('**Guardrails**'); | ||
| expect(updated).toContain('<!-- OPENSPEC:END -->'); | ||
| expect(updated).not.toContain('Old IFlow body'); | ||
|
|
||
| const iflowApply = path.join( | ||
| testDir, | ||
| '.iflow/commands/openspec-apply.md' | ||
| ); | ||
| const iflowArchive = path.join( | ||
| testDir, | ||
| '.iflow/commands/openspec-archive.md' | ||
| ); | ||
|
|
||
| await expect(FileSystemUtils.fileExists(iflowApply)).resolves.toBe(false); | ||
| await expect(FileSystemUtils.fileExists(iflowArchive)).resolves.toBe(false); | ||
|
|
||
| const [logMessage] = consoleSpy.mock.calls[0]; | ||
| expect(logMessage).toContain( | ||
| 'Updated slash commands: .iflow/commands/openspec-proposal.md' | ||
| ); | ||
|
|
||
| consoleSpy.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the inconsistent file format in the test.
The initialContent (lines 673-680) mixes markdown and TOML syntax. It starts with description: (markdown frontmatter style) but then includes prompt = """ (TOML style). Based on the init test (line 442 in init.test.ts), iFlow uses markdown format with a frontmatter description, not TOML.
Update the initialContent to use consistent markdown format:
- const initialContent = `description: Scaffold a new OpenSpec change and validate strictly."
-
-prompt = """
-<!-- OPENSPEC:START -->
-Old IFlow body
-<!-- OPENSPEC:END -->
-"""
-`;
+ const initialContent = `description: Scaffold a new OpenSpec change and validate strictly.
+
+<!-- OPENSPEC:START -->
+Old IFlow body
+<!-- OPENSPEC:END -->
+`;Also remove the TOML-style prompt assertion on line 689 if it doesn't apply to the markdown format.
🤖 Prompt for AI Agents
In test/core/update.test.ts around lines 667 to 712, the test's initialContent
mixes TOML-style prompt syntax with a markdown frontmatter description; replace
initialContent with a consistent markdown file: add a YAML/markdown frontmatter
block for description (--- description: ... ---) followed by the OPENSPEC body
(the <!-- OPENSPEC:START -->...<!-- OPENSPEC:END --> segment) so the file
matches the project's markdown command format, and remove any assertion that
expects TOML-style prompt syntax (e.g., a check for `prompt = """` or similar)
around line 689 so the test only asserts markdown-appropriate content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/core/init.test.ts (1)
419-453: LGTM! Critical issue from previous review has been resolved.The problematic
testDirreassignment that was flagged in the past review comment has been fixed. Line 421 now correctly uses the sandboxedtestDirfrombeforeEach, preventing test artifacts from polluting the project repository.The test structure follows established patterns and validates:
- File creation in
.iflow/commands/directory- Template content including descriptions and OPENSPEC markers
- Key workflow instructions
Consider adding an extend mode test for iFlow.
Most other tools have a corresponding extend mode test that verifies the tool is marked as "already configured" during subsequent init runs (e.g., lines 915-925 for Qwen, lines 937-947 for Windsurf). Adding a similar test for iFlow would ensure consistency:
it('should mark IFlow as already configured during extend mode', async () => { queueSelections('iflow', DONE, 'iflow', DONE); await initCommand.execute(testDir); await initCommand.execute(testDir); const secondRunArgs = mockPrompt.mock.calls[1][0]; const iflowChoice = secondRunArgs.choices.find( (choice: any) => choice.value === 'iflow' ); expect(iflowChoice.configured).toBe(true); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/core/init.test.ts(1 hunks)test/core/update.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/core/update.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
fileExists(76-86)
🔇 Additional comments (1)
test/core/init.test.ts (1)
455-470: LGTM!The test correctly validates that the init command:
- Injects OPENSPEC markers into an existing IFLOW.md file
- Preserves custom user content
- Adds the reference to
@/openspec/AGENTS.mdand update instructionsThe test structure follows the established pattern used for other tools (CLAUDE.md, CLINE.md, QWEN.md) and properly uses the sandboxed test directory.
fix conflicts and add UTs. could you help review again? @TabishB |
TabishB
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for the updates @dkmos2016!!
* support iflow-cli * docs: add iFlow to supported AI tools in README ([Fission-AI#268](Fission-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation. * add UTs for iflow-cli
* support iflow-cli * docs: add iFlow to supported AI tools in README ([Fission-AI#268](Fission-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation. * add UTs for iflow-cli
* support iflow-cli * docs: add iFlow to supported AI tools in README ([Fission-AI#268](Fission-AI#268)) Add iFlow to the Native Slash Commands table in the README. iFlow support was implemented but was missing from the documentation. * add UTs for iflow-cli
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.