Skip to content

Commit a0504b4

Browse files
Merge pull request #73 from TimeWarpEngineering/version-bump-beta.27
feat: replace NuGet CLI lookup with NuGet Protocol API and style cleanup
2 parents 05c5632 + c018da9 commit a0504b4

33 files changed

Lines changed: 898 additions & 1528 deletions

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ _pkginfo.txt
245245
# but keep track of directories ending in .cache
246246
!?*.[Cc]ache/
247247

248+
# LSP cache files
249+
*.lscache
250+
248251
# Others
249252
ClientBin/
250253
~$*

AGENTS.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ This file provides guidance to agents when working with code in this repository.
88
- **Local development**: Use `#:package TimeWarp.Amuru@*-*` and `#:property RestoreNoCache true` in scripts for fresh package downloads
99

1010
## Non-Obvious Patterns
11-
- **Build scripts avoid circular dependency**: Use raw `System.Diagnostics.Process` instead of TimeWarp.Amuru library
11+
- **Build scripts and dev-cli should use TimeWarp.Amuru**: Prefer `Shell.Builder` over raw `System.Diagnostics.Process`; only use raw process APIs in rare implementation boundaries like true TTY passthrough
1212
- **C# script execution**: `.cs` files get `--` prefix inserted before arguments to prevent dotnet interception
1313
- **Directory management**: Scripts use `[CallerFilePath]` pattern for relative path resolution from script location
1414
- **Test discovery**: Tests found via `find Integration/ -name "*.cs" -type f` command
@@ -17,6 +17,3 @@ This file provides guidance to agents when working with code in this repository.
1717

1818
## Code Style Rules
1919
See `.ai/04-csharp-coding-standards.md` and `.editorconfig` for project-specific formatting requirements.
20-
21-
## Related AI Assistant Rules
22-
- See `CLAUDE.md` for comprehensive project documentation and workflow guidance

BannedSymbols.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
T:System.Console;Prefer injecting ITerminal. TimeWarp.Terminal.Terminal static class is available for migration.
2+
T:System.Diagnostics.ProcessStartInfo;Use TimeWarp.Amuru Shell.Builder instead. See the 'amuru' skill for usage patterns.

Directory.Packages.props

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@
66
<ItemGroup>
77
<!-- External packages -->
88
<PackageVersion Include="CliWrap" Version="3.10.1" />
9-
<PackageVersion Include="ModelContextProtocol.Core" Version="1.0.0-rc.1" />
10-
<PackageVersion Include="NuGet.Versioning" Version="6.11.0" />
9+
<PackageVersion Include="ModelContextProtocol.Core" Version="1.0.0" />
10+
<PackageVersion Include="NuGet.Common" Version="7.3.0" />
11+
<PackageVersion Include="NuGet.Configuration" Version="7.3.0" />
12+
<PackageVersion Include="NuGet.Protocol" Version="7.3.0" />
13+
<PackageVersion Include="NuGet.Versioning" Version="7.3.0" />
1114
<PackageVersion Include="Shouldly" Version="4.3.0" />
1215
<PackageVersion Include="StreamJsonRpc" Version="2.24.84" />
1316
<PackageVersion Include="TimeWarp.Build.Tasks" Version="1.0.0" />

documentation/conceptual/architectural-layers.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -936,7 +936,8 @@ var files = await Fzf.Builder()
936936
### API Documentation
937937
- [README.md](../../../README.md) - Main library documentation
938938
- [Ganda README](../../../Source/TimeWarp.Ganda/README.md) - CLI tool documentation
939-
- [command-extensions.md](../../../source/timewarp-amuru/command-extensions.md) - Extension methods reference
939+
- [command-extensions.cs](../../../source/timewarp-amuru/core/command-extensions.cs) - Collocated command construction reference
940+
- [command-result.cs](../../../source/timewarp-amuru/core/command-result.cs) - Collocated command execution reference
940941

941942
### Related Concepts
942943
- [Error Handling](../ArchitecturalDecisionRecords/Approved/0002-error-handling-philosophy.md)
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# Replace NuGet package lookup CLI calls with NuGet Protocol
2+
3+
**GitHub Issue:** #72
4+
5+
## Description
6+
7+
`NuGetPackageService` currently uses `dotnet package search` CLI calls to look up package versions. This needs to be replaced with direct NuGet Protocol API usage for correctness and performance.
8+
9+
## Problems with Current Implementation
10+
11+
1. **Prerelease packages reported as "not found"** - Missing `--prerelease` flag means packages like `TimeWarp.Amuru`, `TimeWarp.Jaribu`, and `TimeWarp.Multiavatar` are not found when only prerelease versions exist
12+
2. **Slow lookups** - Each package spawns a new `dotnet` process, making `ganda nuget outdated` much slower than tools like `dotnet outdated`
13+
14+
## Checklist
15+
16+
- [x] Add NuGet.Protocol and NuGet.Configuration package references
17+
- [x] Implement `SourceRepository` creation from configured NuGet sources
18+
- [x] Implement `PackageMetadataResource` caching per source (via `NuGetSourceCache`)
19+
- [x] Replace `dotnet package search` CLI call with `FindPackageByIdResource.GetAllVersionsAsync()` API
20+
- [x] Ensure prerelease packages are correctly found
21+
- [x] Ensure authenticated feeds work correctly
22+
- [x] Maintain existing `INuGetPackageService` contract compatibility
23+
- [x] Add/update unit tests for the new implementation
24+
- [x] Verify performance improvement over CLI approach
25+
- [x] Update documentation if needed
26+
27+
## Session
28+
29+
- Created: ses_27dd18c7effe1K4rnFRhnQezjn (2026-04-12)
30+
31+
## Notes
32+
33+
- Current implementation location: `NuGetPackageService`
34+
- Current CLI command: `dotnet package search {packageId} --exact-match --format json`
35+
- The new implementation should:
36+
- correctly find prerelease packages
37+
- be much faster (avoid process startup overhead)
38+
- work with configured NuGet sources and authenticated feeds
39+
- preserve the existing `INuGetPackageService` contract
40+
- Repro observed: `ganda nuget outdated --update` shows packages as "(not found on NuGet)" even though they exist as prerelease
41+
42+
---
43+
44+
## Implementation Plan
45+
46+
### Files Modified
47+
- `Directory.Packages.props` - Added `NuGet.Protocol`, `NuGet.Configuration`, `NuGet.Common` (upgraded to 7.3.0)
48+
- `source/timewarp-amuru/timewarp-amuru.csproj` - Added package references
49+
- `source/timewarp-amuru/nu-get/nuget-package-service.cs` - Replaced CLI calls with `FindPackageByIdResource.GetAllVersionsAsync()`
50+
- `source/timewarp-amuru/nu-get/NuGetModels.cs` - Renamed to `nuget-models.cs` (kebab-case)
51+
52+
### Files Created
53+
- `source/timewarp-amuru/nu-get/nuget-source-cache.cs` - Cache `SourceRepository` instances per source URL
54+
55+
### Tests Updated
56+
- `tests/timewarp-amuru/single-file-tests/repo-services/nuget-package-service.cs` - Added prerelease package tests, all 29 tests pass
57+
58+
### Key Decisions
59+
1. Use `FindPackageByIdResource` instead of `PackageMetadataResource` - returns all versions including prerelease, simpler API
60+
2. Upgraded NuGet packages to 7.3.0 (from 6.11.0) - uses System.Text.Json instead of Newtonsoft.Json
61+
3. Cache `SourceRepository` instances in `NuGetSourceCache` to avoid repeated initialization
62+
4. Remove `ParseSearchResult` method entirely (CLI JSON parsing no longer needed)
63+
5. `INuGetPackageService` contract remains unchanged - backward compatible
64+
6. Aggregate versions from all enabled NuGet sources
65+
66+
### Additional Changes (Style/Cleanup)
67+
- All `source/timewarp-amuru/` files renamed to kebab-case (PascalCase → kebab-case)
68+
- All `TODO:` region comments replaced with proper purpose/design descriptions
69+
- `global-usings.cs` consolidated into single project-level file (removed folder-level duplicates)
70+
- `*.lscache` files added to `.gitignore`
71+
- Test files updated: replaced `ProcessStartInfo` with `Shell.Builder` (fixing RS0030 violations)
72+
- `tools/dev-cli/services/process-helpers.cs` - Replaced `ProcessStartInfo` with `Shell.Builder` + argument parser
73+
- Dev-cli: `TreatWarningsAsErrors` enabled, `global-usings.cs` added to compilation, `IL2104`/`IL3053` suppressed (StreamJsonRpc transitive Newtonsoft dep)
74+
- `Directory.Packages.props`: `ModelContextProtocol.Core` updated to 1.0.0
75+
- `AGENTS.md` updated to reflect that build scripts/dev-cli should prefer `Shell.Builder`
76+
- Command core files now use richer collocated `#region` documentation (`Purpose`, `Design`, `Responsibilities`, `Execution Modes`, `Implementation Boundaries`)
77+
- Stale sidecar docs `source/timewarp-amuru/core/command-extensions.md` and `command-result.md` deleted in favor of collocated source documentation
78+
- README and architectural docs updated to reference source files instead of deleted sidecar docs
79+
80+
### Implementation Steps
81+
1. ✅ Add NuGet.Protocol and NuGet.Configuration to Directory.Packages.props
82+
2. ✅ Add package references to timewarp-amuru.csproj
83+
3. ✅ Create NuGetSourceCache.cs helper class
84+
4. ✅ Rewrite NuGetPackageService.SearchAsync() using FindPackageByIdResource
85+
5. ✅ Remove ParseSearchResult private method
86+
6. ✅ Add NuGetVersionComparer helper
87+
7. ✅ Update tests with prerelease package test case
88+
8. ✅ Verify RepoCheckVersionService and CheckVersionCommand work unchanged
89+
9. ✅ Run full test suite (355 passed, 1 skipped)
90+
10. ✅ Update documentation
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Clean up RS0030 banned symbol audit issues
2+
3+
## Description
4+
5+
Review and resolve RS0030 banned symbol warnings throughout the repository. The project uses a banned symbol analyzer to enforce use of TimeWarp.Amuru's `Shell.Builder` instead of raw `System.Diagnostics.Process` and `ProcessStartInfo`. However, some files have received pragma exemptions that may need architectural review.
6+
7+
## Checklist
8+
9+
- [x] Audit all RS0030 pragma exemptions in the codebase
10+
- [x] Evaluate if `SshKeyHelper.cs` should use Shell.Builder instead of raw Process/ProcessStartInfo
11+
- [ ] Evaluate if `CommandResult.cs` TtyPassthroughAsync needs Process directly (TTY inheritance requirement)
12+
- [ ] Document legitimate exemptions with clear justification comments
13+
- [x] Refactor any code that should use Amuru's API but doesn't
14+
- [ ] Consider if banned symbol configuration needs adjustment for Amuru's own source
15+
16+
## Notes
17+
18+
### Current State (2026-03-27)
19+
20+
Files with RS0030 pragma exemptions:
21+
1. `source/timewarp-amuru/core/CommandResult.cs` - TtyPassthroughAsync method
22+
- Uses Process directly for TTY inheritance (no stream redirection)
23+
- May be legitimate: TTY passthrough requires Process.Start without CliWrap's stream piping
24+
25+
2. ~~`source/timewarp-amuru/native/utilities/SshKeyHelper.cs`~~ - **REFACTORED**
26+
- Now uses `Shell.Builder` for all ssh-keygen operations
27+
- Methods renamed to async versions (e.g., `GenerateKeyPair``GenerateKeyPairAsync`)
28+
- No longer needs RS0030 pragma exemptions
29+
30+
### Completed Work (2026-03-27)
31+
32+
- Refactored `SshKeyHelper.cs` to use `Shell.Builder` API
33+
- All 6 methods now use async patterns with `CaptureAsync()`
34+
- Removed all RS0030 pragmas from SshKeyHelper.cs
35+
- Added IDE0007 pragma for explicit type usage per coding standards
36+
37+
### Architectural Questions
38+
39+
- Should Amuru's own source be exempt from the banned symbol analyzer entirely?
40+
- Or should Amuru "eat its own dog food" and use Shell.Builder internally?
41+
- Is there a pattern where certain low-level operations legitimately need Process directly?
42+
43+
### Related
44+
45+
- The banned symbol analyzer enforces: `Use TimeWarp.Amuru Shell.Builder instead`
46+
- See the 'amuru' skill for usage patterns

readme.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,8 @@ See our [Architectural Decision Records](Documentation/Conceptual/ArchitecturalD
484484
## Documentation
485485

486486
- **[Migration Guide](Analysis/MigrationGuide.md)** - Guide for migrating from older versions
487-
- **[command-extensions.md](source/timewarp-amuru/command-extensions.md)** - Static API documentation
488-
- **[command-result.md](source/timewarp-amuru/command-result.md)** - Fluent interface documentation
487+
- **[command-extensions.cs](source/timewarp-amuru/core/command-extensions.cs)** - Collocated command construction design documentation
488+
- **[command-result.cs](source/timewarp-amuru/core/command-result.cs)** - Collocated command execution design documentation
489489
- **[Architectural Decisions](Documentation/Conceptual/ArchitecturalDecisionRecords/Overview.md)** - Design rationale and decisions
490490

491491
## Example Scripts
@@ -513,4 +513,4 @@ See our [Kanban board](Kanban/Overview.md) for current development tasks and pri
513513

514514
If you have an issue and don't receive a timely response, feel free to reach out on our [Discord server](https://discord.gg/A55JARGKKP).
515515

516-
[![Discord](https://img.shields.io/discord/715274085940199487?logo=discord)](https://discord.gg/7F4bS2T)
516+
[![Discord](https://img.shields.io/discord/715274085940199487?logo=discord)](https://discord.gg/7F4bS2T)

source/Directory.Build.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<!-- Default package metadata (can be overridden in individual projects) -->
66
<PropertyGroup Label="Package Metadata">
77
<IsPackable>true</IsPackable>
8-
<Version>1.0.0-beta.28</Version>
8+
<Version>1.0.0-beta.29</Version>
99
<Authors>Steven T. Cramer</Authors>
1010
<RepositoryUrl>https://github.com/TimeWarpEngineering/timewarp-amuru</RepositoryUrl>
1111
<PackageLicenseExpression>Unlicense</PackageLicenseExpression>

0 commit comments

Comments
 (0)