Skip to content

Commit 367ef4f

Browse files
Merge pull request #74 from TimeWarpEngineering/version-bump-beta.27
feat: disable StreamJsonRpc-based JSON-RPC to unblock AOT consumers
2 parents a0504b4 + 8ebe9af commit 367ef4f

20 files changed

Lines changed: 900 additions & 568 deletions

Directory.Packages.props

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<ItemGroup>
77
<!-- External packages -->
88
<PackageVersion Include="CliWrap" Version="3.10.1" />
9-
<PackageVersion Include="ModelContextProtocol.Core" Version="1.0.0" />
9+
<PackageVersion Include="ModelContextProtocol.Core" Version="1.2.0" />
1010
<PackageVersion Include="NuGet.Common" Version="7.3.0" />
1111
<PackageVersion Include="NuGet.Configuration" Version="7.3.0" />
1212
<PackageVersion Include="NuGet.Protocol" Version="7.3.0" />
@@ -15,7 +15,7 @@
1515
<PackageVersion Include="StreamJsonRpc" Version="2.24.84" />
1616
<PackageVersion Include="TimeWarp.Build.Tasks" Version="1.0.0" />
1717
<PackageVersion Include="TimeWarp.Jaribu" Version="1.0.0-beta.12" />
18-
<PackageVersion Include="TimeWarp.Nuru" Version="3.0.0-beta.67" />
18+
<PackageVersion Include="TimeWarp.Nuru" Version="3.0.0-beta.68" />
1919
<PackageVersion Include="TimeWarp.Terminal" Version="1.0.0-beta.12" />
2020

2121
<!-- Analyzers -->
@@ -24,7 +24,7 @@
2424
<PackageVersion Include="Roslynator.Formatting.Analyzers" Version="4.15.0" />
2525
<PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="10.0.201" />
2626
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="5.3.0" />
27-
<PackageVersion Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.4" />
27+
<PackageVersion Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="4.14.0" />
2828

2929
</ItemGroup>
3030
</Project>
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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`.
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+
- [x] Evaluate if `CommandResult.cs` TtyPassthroughAsync needs Process directly (TTY inheritance requirement)
12+
- [x] Document legitimate exemptions with clear justification comments
13+
- [x] Refactor any code that should use Amuru's API but doesn't
14+
- [x] Consider if banned symbol configuration needs adjustment for Amuru's own source
15+
16+
## Notes
17+
18+
### Final State
19+
20+
Only one RS0030 pragma exemption remains in the entire codebase:
21+
22+
1. `source/timewarp-amuru/core/command-result.cs``TtyPassthroughAsync`
23+
- **Legitimate**: True TTY inheritance requires `Process.Start` without stream redirection; CliWrap cannot express this
24+
- **Documented**: `#pragma warning disable RS0030` with justification comment, plus `#region Implementation Boundaries` explaining the design decision
25+
26+
All other RS0030 exemptions have been eliminated:
27+
- `SshKeyHelper.cs` — refactored to use `Shell.Builder`
28+
- Test files (`fzf-builder.select-async.cs`, `shell-builder.select-async.cs`) — replaced `ProcessStartInfo` with `Shell.Builder`
29+
- `tools/dev-cli/services/process-helpers.cs` — replaced `ProcessStartInfo` with `Shell.Builder` + argument parser
30+
31+
### Banned Symbol Configuration
32+
33+
No adjustment needed. The single remaining exemption in `command-result.cs` is properly justified and documented. Amuru correctly "eats its own dog food" — it uses `Shell.Builder` everywhere except for the one case where raw process APIs are architecturally required (TTY passthrough).
34+
35+
### Architectural Decision
36+
37+
Amuru should use `Shell.Builder` internally wherever possible. The only exception is `TtyPassthroughAsync`, which is an implementation boundary where raw process APIs are required because CliWrap's stream-piped model cannot preserve TTY characteristics. This is documented in `#region Implementation Boundaries` in the source file.

kanban/in-progress/080-clean-up-rs0030-banned-symbol-audit-issues.md

Lines changed: 0 additions & 46 deletions
This file was deleted.
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
# Clean up repository to follow kebab-case naming and C# conventions
2+
3+
## Description
4+
5+
Audit and clean up the entire repository to ensure consistent kebab-case file naming and proper C# documentation conventions per the csharp skill. This includes renaming files from PascalCase to kebab-case and replacing all "TODO: Add purpose description" region comments with proper Purpose/Design/Responsibilities documentation.
6+
7+
## Scope
8+
9+
This is a large-scale style cleanup task affecting the entire codebase. It should be done in manageable chunks to keep PRs reviewable.
10+
11+
## Checklist
12+
13+
### Phase 1: Core Files (source/timewarp-amuru/)
14+
- [ ] Rename `AppContextExtensions.cs``app-context-extensions.cs`
15+
- [ ] Rename `CliConfiguration.cs``cli-configuration.cs`
16+
- [ ] Rename `ScriptContext.cs``script-context.cs`
17+
- [ ] Rename `PathResolver.cs``path-resolver.cs`
18+
- [ ] Rename `Installer.cs``installer.cs`
19+
- [ ] Update regions in all core files with proper Purpose/Design descriptions
20+
21+
### Phase 2: DotNet Commands (source/timewarp-amuru/dot-net-commands/)
22+
- [ ] Rename `DotNet.cs``dot-net.cs`
23+
- [ ] Rename `DotNet.*.cs` files → `dot-net.*.cs` (kebab-case)
24+
- [ ] Rename tool subdirectory files
25+
- [ ] Update regions in all dotnet command files
26+
27+
### Phase 3: Git Commands (source/timewarp-amuru/git-commands/)
28+
- [ ] Rename `Git.cs``git.cs`
29+
- [ ] Rename `Git.*.cs``git.*.cs` (kebab-case)
30+
- [ ] Update regions in all git command files
31+
32+
### Phase 4: Fzf Commands (source/timewarp-amuru/fzf-command/)
33+
- [ ] Rename `Fzf.cs``fzf.cs`
34+
- [ ] Rename `Fzf.*.cs``fzf.*.cs` (kebab-case)
35+
- [ ] Update regions in all fzf command files
36+
37+
### Phase 5: Native/FileSystem (source/timewarp-amuru/native/)
38+
- [ ] Rename `Bash.cs``bash.cs`
39+
- [ ] Rename file-system commands (`Commands.*.cs``commands.*.cs`)
40+
- [ ] Rename file-system direct (`Direct.*.cs``direct.*.cs`)
41+
- [ ] Rename utilities (`Post.cs`, `GenerateColor.cs`, etc.)
42+
- [ ] Rename `PathResolver.cs`
43+
- [ ] Update regions in all native files
44+
45+
### Phase 6: JSON-RPC (source/timewarp-amuru/json-rpc/)
46+
- [ ] Rename `JsonRpcClient.cs``json-rpc-client.cs`
47+
- [ ] Rename `JsonRpcClientBuilder.cs``json-rpc-client-builder.cs`
48+
- [ ] Rename `IJsonRpcClient.cs``ijson-rpc-client.cs`
49+
- [ ] Update regions
50+
51+
### Phase 7: Testing (source/timewarp-amuru/testing/)
52+
- [ ] Rename `CommandMock.cs``command-mock.cs`
53+
- [ ] Rename `MockScope.cs``mock-scope.cs`
54+
- [ ] Rename `MockState.cs``mock-state.cs`
55+
- [ ] Rename `MockSetup.cs``mock-setup.cs`
56+
- [ ] Update regions
57+
58+
### Phase 8: Interfaces & Extensions
59+
- [ ] Rename `ICommandBuilder.cs``icommand-builder.cs`
60+
- [ ] Rename `CommandBuilderExtensions.cs``command-builder-extensions.cs`
61+
- [ ] Update regions
62+
63+
### Phase 9: Tests (tests/)
64+
- [ ] Audit test files for PascalCase naming
65+
- [ ] Rename test files to kebab-case
66+
- [ ] Update regions in test files
67+
68+
### Phase 10: Final Verification
69+
- [ ] Run `dotnet build` to verify no compilation errors
70+
- [ ] Run `dev test` to verify all tests pass
71+
- [ ] Check for any remaining PascalCase files
72+
- [ ] Check for any remaining "TODO: Add purpose" comments
73+
- [ ] Update documentation references if needed
74+
75+
## Region Documentation Template
76+
77+
All files should follow this pattern (see csharp skill):
78+
79+
```csharp
80+
#region Purpose
81+
// One-line description of what this file/class does
82+
#endregion
83+
84+
#region Design
85+
// Key design decisions and rationale
86+
// Constraints and dependencies
87+
// Why certain approaches were chosen
88+
#endregion
89+
90+
#region Responsibilities (optional)
91+
// What this class/file is responsible for
92+
// What it is NOT responsible for
93+
#endregion
94+
95+
#region Open Questions (optional)
96+
// Q1 (YYYY-MM-DD, author): question text
97+
// A1 (YYYY-MM-DD, author): answer text
98+
#endregion
99+
```
100+
101+
## Notes
102+
103+
### Current State
104+
- 96 files have "TODO: Add purpose description" in their regions
105+
- Many files still use PascalCase naming (e.g., `AppContextExtensions.cs`, `CliConfiguration.cs`)
106+
- The `core/` folder has been cleaned up (completed in tasks 080/081)
107+
- The `nu-get/` folder has been cleaned up (completed in task 081)
108+
109+
### Approach
110+
1. Work folder-by-folder to keep commits focused
111+
2. Each rename + region update should be a single commit
112+
3. Create small PRs (one folder per PR) for easier review
113+
4. Run tests after each folder cleanup
114+
115+
### Reference
116+
- See `.ai/04-csharp-coding-standards.md` for C# conventions
117+
- See `csharp` skill for region documentation patterns
118+
- Examples of good collocated docs:
119+
- `source/timewarp-amuru/core/command-extensions.cs`
120+
- `source/timewarp-amuru/core/command-result.cs`
121+
122+
### Files with TODO: comments (96 total)
123+
124+
**Core (6 files):**
125+
- AppContextExtensions.cs
126+
- CliConfiguration.cs
127+
- Installer.cs
128+
- ScriptContext.cs
129+
- core/shell-builder.cs
130+
- interfaces/ICommandBuilder.cs
131+
132+
**DotNet Commands (25 files):**
133+
- DotNet.cs, DotNet.*.cs, tool/DotNet.*.cs
134+
135+
**Git Commands (17 files):**
136+
- Git.cs, Git.*.cs
137+
138+
**Fzf Commands (10 files):**
139+
- Fzf.cs, Fzf.*.cs
140+
141+
**JSON-RPC (3 files):**
142+
- JsonRpcClient.cs, JsonRpcClientBuilder.cs, IJsonRpcClient.cs
143+
144+
**Native (14 files):**
145+
- Bash.cs, utilities/*, file-system/commands/*, file-system/direct/*, PathResolver.cs
146+
147+
**Testing (4 files):**
148+
- CommandMock.cs, MockScope.cs, MockState.cs, MockSetup.cs
149+
150+
**Extensions (1 file):**
151+
- CommandBuilderExtensions.cs
152+
153+
**Tests (spike files, may be lower priority):**
154+
- cliwrap-exit-code-tests/*.cs (9 files)
155+
156+
## Session
157+
158+
- Created: ses_27dd18c7effe1K4rnFRhnQezjn (2026-04-13)
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Unblock AOT consumers by disabling StreamJsonRpc-based JSON-RPC support
2+
3+
## Description
4+
5+
Temporarily disable Amuru's current `StreamJsonRpc`-based JSON-RPC feature slice so downstream AOT consumers no longer inherit the `Newtonsoft.Json` dependency and related trimming/native AOT issues.
6+
7+
This is an intentional interim step. The JSON-RPC feature should preserve design intent in commented/disabled code where useful, while a replacement is developed separately in task 083.
8+
9+
## Checklist
10+
11+
- [x] Remove or comment out the `StreamJsonRpc` package reference from `source/timewarp-amuru/timewarp-amuru.csproj`
12+
- [x] Remove or comment out `global using StreamJsonRpc` from `source/timewarp-amuru/global-usings.cs`
13+
- [x] Disable `source/timewarp-amuru/json-rpc/IJsonRpcClient.cs`
14+
- [x] Disable `source/timewarp-amuru/json-rpc/JsonRpcClient.cs`
15+
- [x] Disable `source/timewarp-amuru/json-rpc/JsonRpcClientBuilder.cs`
16+
- [x] Disable `ShellBuilder.AsJsonRpcClient()` in `source/timewarp-amuru/core/shell-builder.cs`
17+
- [ ] Add clear temporary comments explaining JSON-RPC is disabled pending task 083
18+
- [x] Ensure the rest of Amuru builds without `StreamJsonRpc`
19+
- [ ] Verify AOT consumers no longer inherit `Newtonsoft.Json` through Amuru
20+
- [ ] Update any docs/samples that incorrectly imply JSON-RPC support is active
21+
22+
## Session
23+
24+
- Created: ses_27dd18c7effe1K4rnFRhnQezjn (2026-04-14)
25+
26+
## Notes
27+
28+
### Why this task exists
29+
30+
`StreamJsonRpc` still carries a direct `Newtonsoft.Json` dependency in current releases and prereleases. This dependency pollutes downstream consumers of Amuru and creates NativeAOT/trimming pain, even when Amuru does not use the legacy Newtonsoft formatter path at runtime.
31+
32+
### Scope
33+
34+
This task is about **temporarily disabling** the current JSON-RPC feature slice so Amuru's core package remains AOT-friendly.
35+
36+
It is **not** the task to implement the replacement. That work belongs in task 083:
37+
38+
- `083-replace-streamjsonrpc-with-amuru-native-minimal-json-rpc-client`
39+
40+
### What the spike proved
41+
42+
A local build experiment showed that Amuru builds successfully without `StreamJsonRpc` once the following are disabled:
43+
44+
- `global-usings.cs` reference to `StreamJsonRpc`
45+
- `json-rpc/` source files
46+
- `ShellBuilder.AsJsonRpcClient()`
47+
- `timewarp-amuru.csproj` package reference
48+
49+
This means the dependency is isolated to the JSON-RPC feature slice and can be temporarily turned off without breaking the rest of the package.
50+
51+
### Current progress
52+
53+
- The package reference and global using are commented out
54+
- The three `json-rpc/` source files are commented out
55+
- `ShellBuilder.AsJsonRpcClient()` is commented out
56+
- `dotnet build source/timewarp-amuru/timewarp-amuru.csproj` succeeds without `StreamJsonRpc`
57+
- Remaining follow-up for this task is mainly cleanup/documentation and downstream AOT validation
58+
59+
### Intent
60+
61+
Because this is still beta and AOT matters more than preserving unfinished API surface, temporarily disabled code is acceptable here as a staging move. The goal is to unblock consumers now, while preserving enough intent to guide the native replacement.

0 commit comments

Comments
 (0)