Skip to content

Commit 532ec8d

Browse files
TelliTelli
committed
fix review feedback and windows test stability
1 parent 17e679b commit 532ec8d

8 files changed

Lines changed: 235 additions & 20 deletions

File tree

‎src/SharpClaw.Code.Commands/Handlers/ApprovalsSlashCommandHandler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public Task<int> ExecuteAsync(SlashCommandParseResult command, CommandExecutionC
3333
if (string.Equals(command.Arguments[0], "reset", StringComparison.OrdinalIgnoreCase)
3434
|| string.Equals(command.Arguments[0], "clear", StringComparison.OrdinalIgnoreCase))
3535
{
36-
replState.ApprovalSettingsOverride = ApprovalSettings.Empty;
36+
replState.ApprovalSettingsOverride = null;
3737
return RenderAsync("Auto-approval reset for the next prompt.", context, cancellationToken);
3838
}
3939

‎src/SharpClaw.Code.Git/Services/GitWorkspaceService.cs‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,10 @@ private async Task<GitWorktreeList> ListWorktreesCoreAsync(string workingDirecto
162162
throw new InvalidOperationException(BuildGitFailureMessage("Failed to list git worktrees.", listResult));
163163
}
164164

165-
var mainWorktreePath = ResolveMainWorktreePath(commonDirResult, repositoryRoot);
166-
var worktrees = ParseWorktreeEntries(listResult.StandardOutput, repositoryRoot, mainWorktreePath);
165+
var mainWorktreePath = commonDirResult.ExitCode == 0
166+
? ResolveMainWorktreePath(commonDirResult, repositoryRoot)
167+
: repositoryRoot;
168+
var worktrees = ParseWorktreeEntries(listResult.StandardOutput, repositoryRoot);
167169
if (worktrees.Count == 0)
168170
{
169171
worktrees =
@@ -251,7 +253,7 @@ private static string[] BuildCreateArguments(string path, string branchName, str
251253
: ["worktree", "add", "-b", branchName, path, startPoint.Trim()];
252254
}
253255

254-
private static List<GitWorktreeEntry> ParseWorktreeEntries(string output, string repositoryRoot, string mainWorktreePath)
256+
private static List<GitWorktreeEntry> ParseWorktreeEntries(string output, string repositoryRoot)
255257
{
256258
var entries = new List<GitWorktreeEntry>();
257259
string? currentPath = null;
@@ -376,14 +378,29 @@ private static string ResolveMainWorktreePath(ProcessRunResult commonDirResult,
376378
return repositoryRoot;
377379
}
378380

379-
if (commonDir.EndsWith($"{Path.DirectorySeparatorChar}.git", OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal))
381+
var normalizedCommonDir = NormalizeDirectorySeparators(commonDir);
382+
var comparison = OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
383+
var dotGitSuffix = $"{Path.DirectorySeparatorChar}.git";
384+
if (normalizedCommonDir.EndsWith(dotGitSuffix, comparison))
380385
{
381-
return Path.GetDirectoryName(commonDir) ?? repositoryRoot;
386+
return Path.GetDirectoryName(normalizedCommonDir) ?? repositoryRoot;
387+
}
388+
389+
var linkedMarker = $"{Path.DirectorySeparatorChar}.git{Path.DirectorySeparatorChar}worktrees{Path.DirectorySeparatorChar}";
390+
var linkedMarkerIndex = normalizedCommonDir.LastIndexOf(linkedMarker, comparison);
391+
if (linkedMarkerIndex > 0)
392+
{
393+
return normalizedCommonDir[..linkedMarkerIndex];
382394
}
383395

384396
return repositoryRoot;
385397
}
386398

399+
private static string NormalizeDirectorySeparators(string path)
400+
=> Path.DirectorySeparatorChar == Path.AltDirectorySeparatorChar
401+
? path
402+
: path.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar);
403+
387404
private static string? NormalizeBranch(string? branch)
388405
{
389406
if (string.IsNullOrWhiteSpace(branch))

‎src/SharpClaw.Code.Infrastructure/Services/LocalFileSystem.cs‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,53 @@ public IEnumerable<string> EnumerateFiles(string path, string searchPattern)
6969
/// <inheritdoc />
7070
public async Task<string?> ReadAllTextIfExistsAsync(string path, CancellationToken cancellationToken)
7171
{
72-
if (!File.Exists(path))
72+
for (var attempt = 0; ; attempt++)
7373
{
74-
return null;
75-
}
74+
cancellationToken.ThrowIfCancellationRequested();
75+
if (!File.Exists(path))
76+
{
77+
return null;
78+
}
7679

77-
return await File.ReadAllTextAsync(path, cancellationToken).ConfigureAwait(false);
80+
try
81+
{
82+
return await File.ReadAllTextAsync(path, cancellationToken).ConfigureAwait(false);
83+
}
84+
catch (IOException) when (attempt < MaxLockRetries)
85+
{
86+
await Task.Delay(LockRetryDelay, cancellationToken).ConfigureAwait(false);
87+
}
88+
catch (UnauthorizedAccessException) when (attempt < MaxLockRetries)
89+
{
90+
await Task.Delay(LockRetryDelay, cancellationToken).ConfigureAwait(false);
91+
}
92+
}
7893
}
7994

8095
/// <inheritdoc />
8196
public async Task<string[]> ReadAllLinesIfExistsAsync(string path, CancellationToken cancellationToken)
8297
{
83-
if (!File.Exists(path))
98+
for (var attempt = 0; ; attempt++)
8499
{
85-
return [];
86-
}
100+
cancellationToken.ThrowIfCancellationRequested();
101+
if (!File.Exists(path))
102+
{
103+
return [];
104+
}
87105

88-
return await File.ReadAllLinesAsync(path, cancellationToken).ConfigureAwait(false);
106+
try
107+
{
108+
return await File.ReadAllLinesAsync(path, cancellationToken).ConfigureAwait(false);
109+
}
110+
catch (IOException) when (attempt < MaxLockRetries)
111+
{
112+
await Task.Delay(LockRetryDelay, cancellationToken).ConfigureAwait(false);
113+
}
114+
catch (UnauthorizedAccessException) when (attempt < MaxLockRetries)
115+
{
116+
await Task.Delay(LockRetryDelay, cancellationToken).ConfigureAwait(false);
117+
}
118+
}
89119
}
90120

91121
/// <inheritdoc />

‎src/SharpClaw.Code.Runtime/Context/InstructionRuleService.cs‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,7 @@ private async Task AddFileAsync(
171171
}
172172

173173
var maxForDocument = Math.Min(MaxDocumentCharacters, remaining);
174-
var isTruncated = normalized.Length > maxForDocument;
175-
var trimmed = isTruncated
176-
? normalized[..Math.Max(0, maxForDocument - 28)].TrimEnd() + Environment.NewLine + "[Instruction truncated]"
177-
: normalized;
174+
var trimmed = TrimToBudget(normalized, maxForDocument, out var isTruncated);
178175

179176
if (string.IsNullOrWhiteSpace(trimmed))
180177
{
@@ -230,6 +227,41 @@ private static string NormalizeContent(string content)
230227
private static bool BudgetExhausted(List<InstructionRuleDocument> documents, RuleBudget budget)
231228
=> documents.Count >= MaxDocumentCount || budget.TotalCharacters >= MaxTotalCharacters;
232229

230+
private static string? TrimToBudget(string content, int maxCharacters, out bool isTruncated)
231+
{
232+
isTruncated = false;
233+
if (maxCharacters <= 0)
234+
{
235+
return null;
236+
}
237+
238+
if (content.Length <= maxCharacters)
239+
{
240+
return content;
241+
}
242+
243+
isTruncated = true;
244+
const string marker = "[Instruction truncated]";
245+
var markerWithNewLineLength = marker.Length + Environment.NewLine.Length;
246+
if (maxCharacters > markerWithNewLineLength)
247+
{
248+
var prefixLength = Math.Max(0, maxCharacters - markerWithNewLineLength);
249+
var prefix = content[..prefixLength].TrimEnd();
250+
if (!string.IsNullOrWhiteSpace(prefix))
251+
{
252+
return prefix + Environment.NewLine + marker;
253+
}
254+
}
255+
256+
var fallback = content[..maxCharacters].TrimEnd();
257+
if (!string.IsNullOrWhiteSpace(fallback))
258+
{
259+
return fallback;
260+
}
261+
262+
return marker[..Math.Min(marker.Length, maxCharacters)];
263+
}
264+
233265
private sealed class RuleBudget
234266
{
235267
public int TotalCharacters { get; set; }

‎src/SharpClaw.Code.Runtime/Workflow/ApprovalSettingsResolver.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public static ApprovalSettings Normalize(ApprovalSettings settings)
3131
{
3232
ArgumentNullException.ThrowIfNull(settings);
3333

34-
var scopes = settings.AutoApproveScopes
34+
var scopes = (settings.AutoApproveScopes ?? [])
3535
.Where(scope => scope != ApprovalScope.None)
3636
.Distinct()
3737
.OrderBy(static scope => scope)

‎tests/SharpClaw.Code.UnitTests/Commands/ModeAndCliOptionsTests.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,33 @@ public async Task Approvals_slash_command_should_set_auto_approve_override()
136136
renderer.LastCommandResult!.Message.Should().Contain("Auto-approval override set");
137137
}
138138

139+
[Fact]
140+
public async Task Approvals_slash_command_reset_should_clear_override()
141+
{
142+
var replState = new ReplInteractionState
143+
{
144+
ApprovalSettingsOverride = new ApprovalSettings([ApprovalScope.ShellExecution], 1)
145+
};
146+
var renderer = new StubOutputRenderer();
147+
var handler = new ApprovalsSlashCommandHandler(replState, new OutputRendererDispatcher([renderer]));
148+
var context = new CommandExecutionContext(
149+
WorkingDirectory: "/workspace",
150+
Model: null,
151+
PermissionMode: PermissionMode.WorkspaceWrite,
152+
OutputFormat: OutputFormat.Text,
153+
PrimaryMode: PrimaryMode.Build,
154+
SessionId: null);
155+
156+
var exitCode = await handler.ExecuteAsync(
157+
new SlashCommandParseResult(true, "approvals", ["reset"]),
158+
context,
159+
CancellationToken.None);
160+
161+
exitCode.Should().Be(0);
162+
replState.ApprovalSettingsOverride.Should().BeNull();
163+
renderer.LastCommandResult!.Message.Should().Contain("reset");
164+
}
165+
139166
private sealed class StubOutputRenderer : IOutputRenderer
140167
{
141168
public OutputFormat Format => OutputFormat.Text;

‎tests/SharpClaw.Code.UnitTests/MemorySkillsGit/MemorySkillsGitServiceTests.cs‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,27 @@ public async Task GitWorkspaceService_should_list_create_and_prune_worktrees()
118118
var pruned = await service.PruneWorktreesAsync("/repo", CancellationToken.None);
119119

120120
initial.Worktrees.Should().HaveCount(2);
121-
created.Worktree.Path.Should().Be("/repo-new");
121+
created.Worktree.Path.Should().Be(Path.GetFullPath(Path.Combine("/repo", "../repo-new")));
122122
created.Worktree.Branch.Should().Be("feature/new");
123123
pruned.PrunedCount.Should().Be(1);
124124
pruned.Worktrees.Should().NotContain(entry => entry.IsPrunable);
125125
}
126126

127+
/// <summary>
128+
/// Ensures linked worktrees resolve the main worktree path from git-common-dir output.
129+
/// </summary>
130+
[Fact]
131+
public async Task GitWorkspaceService_should_detect_linked_worktree_main_path()
132+
{
133+
IGitWorkspaceService service = new GitWorkspaceService(new LinkedWorktreeProcessRunner());
134+
135+
var snapshot = await service.GetSnapshotAsync("/repo-linked", CancellationToken.None);
136+
137+
snapshot.IsLinkedWorktree.Should().BeTrue();
138+
snapshot.MainWorktreePath.Should().Be("/repo");
139+
snapshot.WorktreeCount.Should().Be(2);
140+
}
141+
127142
private static string CreateTemporaryWorkspace()
128143
{
129144
var workspacePath = Path.Combine(Path.GetTempPath(), "sharpclaw-memory-skill-git-tests", Guid.NewGuid().ToString("N"));
@@ -203,4 +218,34 @@ private string RenderWorktreeList()
203218

204219
private sealed record WorktreeState(string Branch, string Head, bool IsPrunable);
205220
}
221+
222+
private sealed class LinkedWorktreeProcessRunner : IProcessRunner
223+
{
224+
public Task<ProcessRunResult> RunAsync(ProcessRunRequest request, CancellationToken cancellationToken)
225+
{
226+
var output = request.Arguments switch
227+
{
228+
["rev-parse", "--show-toplevel"] => "/repo-linked\n",
229+
["rev-parse", "--path-format=absolute", "--git-common-dir"] => "/repo/.git/worktrees/repo-linked\n",
230+
["branch", "--show-current"] => "feature/worktrees\n",
231+
["rev-parse", "HEAD"] => "def456\n",
232+
["status", "--porcelain=v1", "--branch"] => "## feature/worktrees\n",
233+
["diff", "--no-ext-diff", "--stat"] => string.Empty,
234+
["rev-list", "--left-right", "--count", "@{upstream}...HEAD"] => "0\t0\n",
235+
["worktree", "list", "--porcelain"] => """
236+
worktree /repo
237+
HEAD abc123
238+
branch refs/heads/main
239+
240+
worktree /repo-linked
241+
HEAD def456
242+
branch refs/heads/feature/worktrees
243+
244+
""",
245+
_ => throw new InvalidOperationException($"Unexpected git command: {string.Join(' ', request.Arguments)}")
246+
};
247+
248+
return Task.FromResult(new ProcessRunResult(0, output, string.Empty, DateTimeOffset.UtcNow, DateTimeOffset.UtcNow));
249+
}
250+
}
206251
}

‎tests/SharpClaw.Code.UnitTests/Runtime/InstructionRuleServiceTests.cs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
using FluentAssertions;
2+
using SharpClaw.Code.Infrastructure.Abstractions;
3+
using SharpClaw.Code.Infrastructure.Services;
4+
using SharpClaw.Code.Runtime.Context;
5+
6+
namespace SharpClaw.Code.UnitTests.Runtime;
7+
8+
/// <summary>
9+
/// Verifies instruction rule loading respects document and total-size budgets.
10+
/// </summary>
11+
public sealed class InstructionRuleServiceTests
12+
{
13+
[Fact]
14+
public async Task LoadAsync_should_not_exceed_total_budget_when_remaining_space_is_tiny()
15+
{
16+
var workspacePath = CreateTemporaryWorkspace();
17+
var globalRulesPath = Path.Combine(workspacePath, ".sharpclaw-home", "rules");
18+
Directory.CreateDirectory(globalRulesPath);
19+
20+
for (var index = 0; index < 11; index++)
21+
{
22+
await File.WriteAllTextAsync(
23+
Path.Combine(globalRulesPath, $"rule-{index:D2}.md"),
24+
new string((char)('a' + index), 1_090),
25+
CancellationToken.None);
26+
}
27+
28+
await File.WriteAllTextAsync(
29+
Path.Combine(globalRulesPath, "rule-11.md"),
30+
new string('z', 200),
31+
CancellationToken.None);
32+
33+
var service = new InstructionRuleService(
34+
new LocalFileSystem(),
35+
new PathService(),
36+
new FixedUserProfilePaths(workspacePath));
37+
38+
var snapshot = await service.LoadAsync(workspacePath, CancellationToken.None);
39+
40+
snapshot.Documents.Should().HaveCount(12);
41+
snapshot.Documents.Sum(static document => document.Content.Length).Should().BeLessThanOrEqualTo(12_000);
42+
snapshot.Documents[^1].Content.Length.Should().BeLessThanOrEqualTo(10);
43+
snapshot.Documents[^1].IsTruncated.Should().BeTrue();
44+
}
45+
46+
private static string CreateTemporaryWorkspace()
47+
{
48+
var workspacePath = Path.Combine(Path.GetTempPath(), "sharpclaw-rule-tests", Guid.NewGuid().ToString("N"));
49+
Directory.CreateDirectory(workspacePath);
50+
return workspacePath;
51+
}
52+
53+
private sealed class FixedUserProfilePaths(string root) : IUserProfilePaths
54+
{
55+
public string GetUserCustomCommandsDirectory()
56+
=> Path.Combine(root, ".sharpclaw-home", "commands");
57+
58+
public string GetUserHomeDirectory()
59+
=> root;
60+
61+
public string GetUserSharpClawRoot()
62+
=> Path.Combine(root, ".sharpclaw-home");
63+
}
64+
}

0 commit comments

Comments
 (0)