-
Notifications
You must be signed in to change notification settings - Fork 397
Add CLI application: 6 commands, 3 output formats, 260 tests #1093
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added missing copyright headers to 60 files - Fixed IDE0073 violations - Removed Main.Tests reference from solution (was deleted)
- Created Main.Tests.csproj with xUnit and Moq - Added Moq to Directory.Packages.props - Added placeholder test (1 passing) - Ready to add coverage-driven tests
- Integration tests: 18 end-to-end CLI workflows - Service tests: 10 ContentService tests - Formatter tests: 25 JSON/YAML/Factory tests - Settings tests: 48 validation tests - HumanOutputFormatter tests: 29 tests - DTO tests: 12 model tests - BaseCommand tests: 5 error handling tests Current coverage: 42.5% (need to reach 80%)
- Extract CliApplicationBuilder (configures CLI commands) - Extract ModeRouter (handles mode detection and routing) - Keep Program.cs thin (just entry point delegation) - Makes previously untestable code now testable This will increase coverage significantly.
- 11 tests for ModeRouter (mode detection and routing) - 2 tests for CliApplicationBuilder - Coverage improved from 42.5% to 69.30% (+26.8 points) - Still need 10.7 points to reach 80% target Total: 166 tests passing
- Tests error paths in all commands - Tests edge cases (non-existent IDs, empty DB) - Tests all ConfigCommand flags - Additional coverage for command execution paths Total: 176 tests (was 166)
- Fix GlobalOptions import - Fix NodesCommandSettings type mismatch Ready to measure coverage
- Added EmptyRemainingArguments helper for CommandContext - Fixed all CommandContext constructor calls - All 175 tests now passing Ready to measure coverage
Code quality fixes: - CA1861: Convert inline arrays to static readonly fields (15 fixes) - RCS1118: Mark local variables as const (51 fixes) - IDE0073: Add copyright headers to test files (8 fixes) - RCS1141: Add param XML documentation (4 fixes) - IDE0130: Fix namespace mismatches (4 fixes) - CA1869: Cache JsonSerializerOptions (2 fixes) - CA1031: Specific exception handling (1 fix) - CA2000: Dispose IDisposable objects (1 fix) - CA2201: Don't throw generic exceptions (2 fixes) - CS8625: Null literal fixes (3 fixes) - RCS1037: Remove trailing whitespace (8 fixes) - IDE0005: Remove unnecessary usings (4 fixes) Build result: 0 warnings, 0 errors Coverage: Main 92.79% (exceeds 80% target) Tests: 175 passing, 0 failed, 0 skipped
Implemented config injection architecture to prevent tests from accidentally writing to personal ~/.km directory. Config is now loaded once in Main before any command logic runs, then injected via dependency injection. Changes: - Add DI infrastructure (TypeRegistrar/TypeResolver) for Spectre.Console.Cli - Refactor CliApplicationBuilder to load config early and setup DI container - Update all 6 commands to receive AppConfig via constructor injection - Change BaseCommand.InitializeAsync() to synchronous Initialize() (no file I/O) - Create TestCliApplicationBuilder for easy test config injection - Add UserDataProtectionTests to verify tests never touch ~/.km - Update all test files to inject configs explicitly Architecture: Main → CliApplicationBuilder.Build(args) → Load config → Register in DI → Commands receive via constructor → Tests inject mock configs (no disk I/O) Results: ✅ Build: 0 warnings, 0 errors ✅ Tests: 260 tests passed (100% success rate) ✅ Protection: Tests can NO LONGER touch ~/.km accidentally 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
- Add coverage.sh script for generating test coverage reports - Update km.sh wrapper script - Improve HumanOutputFormatter for better list display - Add comprehensive ContentStorageService tests - Enhance ContentService test coverage 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
When users first install KM and run 'km list' or 'km get', they now see welcoming, helpful messages instead of technical errors. Changes: - Add DatabaseNotFoundException for expected first-run state - Update BaseCommand to throw specific exception for missing DB - Handle DatabaseNotFoundException gracefully in ListCommand and GetCommand - Show friendly welcome message in human format with next-step guidance - Return valid empty JSON/YAML for machine-readable formats - Exit code 0 (success) for missing DB - it's not an error on first run - Update tests to expect new friendly behavior UX improvements: - Human format: "Welcome to Kernel Memory! 🚀" with examples - JSON format: Valid empty array/null (parseable output) - Clear distinction: missing DB (first run) vs ID not found (user error) Exit codes: - 0 = Success (includes first-run "no data yet" state) - 1 = User error (invalid arguments, ID not found in existing DB) - 2 = System error (DB corruption, permission denied)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a complete command-line interface for Kernel Memory, enabling users to manage content via terminal commands.
Overview
Complete CLI implementation with 6 core commands, multiple output formats, comprehensive test coverage, and a friendly first-run experience.
Stats:
Features
Commands
km upsert- Create or update content with optional metadatakm get <id>- Retrieve content by IDkm delete <id>- Delete content by IDkm list- List all content with paginationkm nodes- List configured nodeskm config- Query configuration settingsOutput Formats
Global Options
--config <path>- Custom config file path--node <name>- Target specific node--format <human|json|yaml>- Output format--verbosity <normal|quiet|silent>- Control output detailExamples
Architecture
Config Injection
Configuration is loaded once at startup and injected via dependency injection:
~/.km/)Command Structure
Exit Codes
First-Run UX
When the database doesn't exist yet (expected on first run):
Components Added
CLI Layer (
src/Main/CLI/)Services Layer (
src/Main/Services/)Test Suite (
tests/Main.Tests/)Infrastructure
Quality Metrics
Test Coverage
Build Quality
dotnet formatDesign Principles
Testing Strategy
Integration Tests
Unit Tests
Critical Tests
~/.km/Breaking Changes
None - this is a new CLI, doesn't affect existing APIs or libraries.
Migration Guide
Not applicable - this is a new feature addition.