Skip to content

refactor: address technical debt with modularization and type safety#177

Closed
laynepenney wants to merge 4 commits intomainfrom
dev
Closed

refactor: address technical debt with modularization and type safety#177
laynepenney wants to merge 4 commits intomainfrom
dev

Conversation

@laynepenney
Copy link
Copy Markdown
Collaborator

Summary

  • Extract ~400 lines from index.ts into modular src/cli/ components (history, pipeline-input, non-interactive)
  • Improve workflow type safety with discriminated unions and type guards
  • Fix symbol-index metadata types with proper type guards
  • Fix flaky E2E test with buffer flush helpers and robust patterns
  • Create plugin system investigation document with security analysis and roadmap

Changes

New Modules (src/cli/)

  • history.ts - Command history management
  • pipeline-input.ts - File/glob resolution utilities
  • non-interactive.ts - Single-prompt mode execution
  • index.ts - Module exports

Type Safety Improvements

  • Add BaseWorkflowStep interface and WorkflowStep discriminated union
  • Add type guards: isShellStep, isSwitchModelStep, isConditionalStep, etc.
  • Add hasExtendsMetadata type guard for symbol index
  • Reduces as any usage from 30+ to 9 instances

Test Fixes

  • Add waitForOutputFlush() helper to ProcessHarness
  • Use generic response patterns and clearOutput() for robustness

Documentation

  • docs/PLUGIN-INVESTIGATION.md - Security analysis, API stability, and re-enablement roadmap

Test plan

  • pnpm build passes with 0 errors
  • All 2129 tests pass
  • Verified extracted modules work correctly
  • Verified flaky test now passes consistently

🤖 Generated with Claude Code

laynepenney and others added 4 commits January 26, 2026 10:24
Create comprehensive removal plan for consolidating Ollama functionality under
the regular ollama provider. The ollama-cloud provider has proven unreliable
while the ollama provider works correctly with Ollama Cloud via the
OpenAI-compatible API.

Key changes planned:
- Remove ollama-cloud provider registration and implementation
- Update configuration validation and CLI options
- Provide migration guide for existing users
- Maintain full functionality through regular ollama provider

The regular ollama provider is more reliable and already supports Ollama Cloud
with native tool calling via OpenAI-compatible API.

Wingman: Codi <codi@layne.pro>
Consolidate Ollama functionality under the regular 'ollama' provider, which
already works correctly with Ollama Cloud via the OpenAI-compatible API.
The dedicated ollama-cloud provider had authentication and tool calling issues
while the ollama provider offers superior reliability and simpler maintenance.

Key changes:
- Remove OllamaCloudProvider implementation and factory registration
- Delete src/providers/ollama-cloud.ts (927 lines)
- Update provider validation to exclude 'ollama-cloud'
- Update CLI options and help text
- Fix model-map configuration defaults and examples
- Update RAG embeddings provider handling
- Clean up test references and comments
- Provide migration path: --provider ollama with OLLAMA_HOST=https://ollama.com

The regular ollama provider uses the OpenAI-compatible API which:
- Supports native tool calling with JSON schema
- Has proven reliability with Ollama Cloud
- Uses robust OpenAI SDK
- Simplified maintenance overhead

All existing functionality is preserved.
Build: ✅ Tests: ✅

Wingman: Codi <codi@layne.pro>
- Extract ~400 lines from index.ts into src/cli/ modules:
  - history.ts: command history management
  - pipeline-input.ts: file/glob resolution
  - non-interactive.ts: single-prompt mode execution

- Improve workflow type safety with discriminated unions:
  - Add BaseWorkflowStep interface and WorkflowStep union type
  - Add type guards (isShellStep, isSwitchModelStep, etc.)
  - Replace `as any` casts with proper type narrowing

- Fix symbol-index metadata types:
  - Add hasExtendsMetadata type guard
  - Add SymbolMetadataWithExtends interface

- Fix flaky E2E test in commands.e2e.test.ts:
  - Add waitForOutputFlush helper to ProcessHarness
  - Use generic patterns and clearOutput() for robustness

- Create plugin system investigation document:
  - Security analysis and recommendations
  - API stability assessment
  - Phased re-enablement roadmap

Reduces `as any` usage from 30+ to 9 instances.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@laynepenney
Copy link
Copy Markdown
Collaborator Author

Self-Review

  • ✅ Build passes with 0 errors
  • ✅ All 2129 tests pass (2 skipped)
  • ✅ New src/cli/ modules properly export and integrate
  • ✅ Type guards work correctly with discriminated unions
  • ✅ Flaky E2E test now uses robust patterns
  • ✅ Plugin investigation document is comprehensive
  • ✅ Generated workflow files cleaned up in follow-up commit

Verified Changes

Area Verification
src/cli/history.ts Exports loadHistory, saveToHistory correctly
src/cli/pipeline-input.ts Exports resolvePipelineInput, resolveFileList correctly
src/cli/non-interactive.ts Exports runNonInteractive correctly
Workflow type guards isShellStep, isLoopStep, etc. narrow types properly
Symbol index types hasExtendsMetadata guard eliminates as any casts
E2E test Uses clearOutput() and generic patterns for stability

as any Reduction

Before: 30+ instances
After:  9 instances (mostly external library type extensions)

No issues found. Ready to merge.

@laynepenney
Copy link
Copy Markdown
Collaborator Author

Closing this PR to split into separate PRs:

This avoids mixing unrelated changes in a single PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant