Skip to content

Commit 1934b88

Browse files
committed
Fix PR #79 review follow-ups
1 parent 9c5481c commit 1934b88

File tree

12 files changed

+131
-28
lines changed

12 files changed

+131
-28
lines changed

DotPilot.Core/Features/ToolchainCenter/ToolchainCenterContracts.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace DotPilot.Core.Features.ToolchainCenter;
44

55
public sealed record ToolchainCenterWorkstreamDescriptor(
66
int IssueNumber,
7-
string IssueLabel,
7+
string SectionLabel,
88
string Name,
99
string Summary);
1010

@@ -37,7 +37,7 @@ public sealed record ToolchainPollingDescriptor(
3737

3838
public sealed record ToolchainProviderSnapshot(
3939
int IssueNumber,
40-
string IssueLabel,
40+
string SectionLabel,
4141
ProviderDescriptor Provider,
4242
string ExecutablePath,
4343
string InstalledVersion,

DotPilot.Runtime/Features/RuntimeFoundation/DeterministicAgentRuntimeClient.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System.Diagnostics;
21
using DotPilot.Core.Features.ControlPlaneDomain;
32
using DotPilot.Core.Features.RuntimeCommunication;
43
using DotPilot.Core.Features.RuntimeFoundation;
@@ -64,7 +63,7 @@ AgentExecutionMode.Execute when RequiresApproval(request.Prompt) => Result<Agent
6463
SessionPhase.Review,
6564
ApprovalState.Approved,
6665
[CreateArtifact(request.SessionId, ReviewArtifact, ArtifactKind.Report)])),
67-
_ => throw new UnreachableException(),
66+
_ => Result<AgentTurnResult>.Fail(RuntimeCommunicationProblems.OrchestrationUnavailable()),
6867
});
6968
}
7069

DotPilot.Runtime/Features/RuntimeFoundation/RuntimeFoundationCatalog.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public sealed class RuntimeFoundationCatalog : IRuntimeFoundationCatalog
2828
"Agent Framework integration is prepared as a separate slice that can plug into the embedded host without reshaping the UI layer.";
2929
private readonly IReadOnlyList<ProviderDescriptor> _providers;
3030

31-
public RuntimeFoundationCatalog() => _providers = CreateProviders();
31+
public RuntimeFoundationCatalog() => _providers = Array.AsReadOnly(CreateProviders());
3232

3333
public RuntimeFoundationSnapshot GetSnapshot()
3434
{
@@ -72,7 +72,7 @@ private static IReadOnlyList<RuntimeSliceDescriptor> CreateSlices()
7272
];
7373
}
7474

75-
private static IReadOnlyList<ProviderDescriptor> CreateProviders()
75+
private static ProviderDescriptor[] CreateProviders()
7676
{
7777
return
7878
[

DotPilot.Runtime/Features/ToolchainCenter/ToolchainCenterCatalog.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace DotPilot.Runtime.Features.ToolchainCenter;
44

55
public sealed class ToolchainCenterCatalog : IToolchainCenterCatalog, IDisposable
66
{
7-
private const string EpicLabelValue = "PRESESSION READINESS";
7+
private const string EpicLabelValue = "PRE-SESSION READINESS";
88
private const string EpicSummary =
99
"Provider installation, launch checks, authentication, configuration, and refresh state stay visible before the first live session.";
1010
private const string UiWorkstreamLabel = "SURFACE";

DotPilot.Runtime/Features/ToolchainCenter/ToolchainCommandProbe.cs

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace DotPilot.Runtime.Features.ToolchainCenter;
55
internal static class ToolchainCommandProbe
66
{
77
private static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(2);
8+
private static readonly TimeSpan RedirectDrainTimeout = TimeSpan.FromSeconds(1);
89
private const string VersionSeparator = "version";
910
private const string EmptyOutput = "";
1011

@@ -87,14 +88,19 @@ private static ToolchainCommandExecution Execute(string executablePath, IReadOnl
8788

8889
using (process)
8990
{
90-
var standardOutputTask = process.StandardOutput.ReadToEndAsync();
91-
var standardErrorTask = process.StandardError.ReadToEndAsync();
91+
var standardOutputTask = ObserveRedirectedStream(process.StandardOutput.ReadToEndAsync());
92+
var standardErrorTask = ObserveRedirectedStream(process.StandardError.ReadToEndAsync());
9293

9394
if (!process.WaitForExit((int)CommandTimeout.TotalMilliseconds))
9495
{
9596
TryTerminate(process);
96-
ObserveRedirectedStreamFaults(standardOutputTask, standardErrorTask);
97-
return new(true, false, EmptyOutput, EmptyOutput);
97+
WaitForTermination(process);
98+
99+
return new(
100+
true,
101+
false,
102+
AwaitStreamRead(standardOutputTask),
103+
AwaitStreamRead(standardErrorTask));
98104
}
99105

100106
return new(
@@ -105,10 +111,26 @@ private static ToolchainCommandExecution Execute(string executablePath, IReadOnl
105111
}
106112
}
107113

114+
private static Task<string> ObserveRedirectedStream(Task<string> readTask)
115+
{
116+
_ = readTask.ContinueWith(
117+
static task => _ = task.Exception,
118+
CancellationToken.None,
119+
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
120+
TaskScheduler.Default);
121+
122+
return readTask;
123+
}
124+
108125
private static string AwaitStreamRead(Task<string> readTask)
109126
{
110127
try
111128
{
129+
if (!readTask.Wait(RedirectDrainTimeout))
130+
{
131+
return EmptyOutput;
132+
}
133+
112134
return readTask.GetAwaiter().GetResult();
113135
}
114136
catch
@@ -117,19 +139,28 @@ private static string AwaitStreamRead(Task<string> readTask)
117139
}
118140
}
119141

120-
private static void ObserveRedirectedStreamFaults(Task<string> standardOutputTask, Task<string> standardErrorTask)
142+
private static void TryTerminate(Process process)
121143
{
122-
_ = standardOutputTask.Exception;
123-
_ = standardErrorTask.Exception;
144+
try
145+
{
146+
if (!process.HasExited)
147+
{
148+
process.Kill(entireProcessTree: true);
149+
}
150+
}
151+
catch
152+
{
153+
// Best-effort cleanup only.
154+
}
124155
}
125156

126-
private static void TryTerminate(Process process)
157+
private static void WaitForTermination(Process process)
127158
{
128159
try
129160
{
130161
if (!process.HasExited)
131162
{
132-
process.Kill(entireProcessTree: true);
163+
process.WaitForExit((int)RedirectDrainTimeout.TotalMilliseconds);
133164
}
134165
}
135166
catch

DotPilot.Tests/Features/RuntimeFoundation/RuntimeFoundationCatalogTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,11 @@ public void DeterministicClientRejectsUnexpectedExecutionModes()
174174
var client = new DeterministicAgentRuntimeClient();
175175
var invalidRequest = CreateRequest("Plan the runtime foundation rollout.", (AgentExecutionMode)int.MaxValue);
176176

177-
var action = () => client.ExecuteAsync(invalidRequest, CancellationToken.None);
177+
var result = client.ExecuteAsync(invalidRequest, CancellationToken.None).AsTask().GetAwaiter().GetResult();
178178

179-
action.Should().Throw<System.Diagnostics.UnreachableException>();
179+
result.IsFailed.Should().BeTrue();
180+
result.HasProblem.Should().BeTrue();
181+
result.Problem!.HasErrorCode(RuntimeCommunicationProblemCode.OrchestrationUnavailable).Should().BeTrue();
180182
}
181183

182184
[Test]
@@ -221,6 +223,7 @@ public void CatalogCachesProviderListAcrossSnapshotReads()
221223
var secondSnapshot = catalog.GetSnapshot();
222224

223225
ReferenceEquals(firstSnapshot.Providers, secondSnapshot.Providers).Should().BeTrue();
226+
firstSnapshot.Providers.Should().NotBeAssignableTo<ProviderDescriptor[]>();
224227
}
225228

226229
private static RuntimeFoundationCatalog CreateCatalog()

DotPilot.Tests/Features/ToolchainCenter/ToolchainCenterCatalogTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ namespace DotPilot.Tests.Features.ToolchainCenter;
22

33
public class ToolchainCenterCatalogTests
44
{
5-
private const string ToolchainEpicLabel = "PRESESSION READINESS";
5+
private const string ToolchainEpicLabel = "PRE-SESSION READINESS";
66

77
[Test]
88
public void CatalogIncludesEpicIssueCoverageAndAllExternalProviders()
@@ -18,7 +18,7 @@ public void CatalogIncludesEpicIssueCoverageAndAllExternalProviders()
1818

1919
snapshot.EpicLabel.Should().Be(ToolchainEpicLabel);
2020
snapshot.Summary.Should().NotContain("Issue #");
21-
snapshot.Workstreams.Select(workstream => workstream.IssueLabel).Should().Equal("SURFACE", "DIAGNOSTICS", "CONFIGURATION", "POLLING");
21+
snapshot.Workstreams.Select(workstream => workstream.SectionLabel).Should().Equal("SURFACE", "DIAGNOSTICS", "CONFIGURATION", "POLLING");
2222
coveredIssues.Should().Equal(
2323
ToolchainCenterIssues.ToolchainCenterUi,
2424
ToolchainCenterIssues.CodexReadiness,

DotPilot.UITests/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ Stack: `.NET 10`, `NUnit`, `Uno.UITest`, browser-driven UI tests
1313
- `DotPilot.UITests.csproj`
1414
- `Harness/Constants.cs`
1515
- `Harness/TestBase.cs`
16-
- `Features/ApplicationShell/Given_MainPage.cs`
16+
- `Features/ApplicationShell/Given_MainPage.cs` (`GivenWorkbenchShell` in `DotPilot.UITests.Features.Workbench`)
1717

1818
## Boundaries
1919

DotPilot/Presentation/Controls/ToolchainCenterPanel.xaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
FontSize="11"
2020
FontWeight="Medium"
2121
Foreground="{StaticResource AppMutedTextBrush}"
22-
Text="{x:Bind Workstream.IssueLabel}" />
22+
Text="{x:Bind Workstream.SectionLabel}" />
2323
<TextBlock FontSize="13"
2424
FontWeight="SemiBold"
2525
Text="{x:Bind Workstream.Name}" />
@@ -44,7 +44,7 @@
4444
FontSize="11"
4545
FontWeight="Medium"
4646
Foreground="{StaticResource AppMutedTextBrush}"
47-
Text="{x:Bind IssueLabel}" />
47+
Text="{x:Bind SectionLabel}" />
4848
<TextBlock FontSize="13"
4949
FontWeight="SemiBold"
5050
AutomationProperties.AutomationId="{x:Bind AutomationId}"
@@ -256,7 +256,7 @@
256256
FontSize="11"
257257
FontWeight="Medium"
258258
Foreground="{StaticResource AppMutedTextBrush}"
259-
Text="{Binding SelectedToolchainProviderSnapshot.IssueLabel}" />
259+
Text="{Binding SelectedToolchainProviderSnapshot.SectionLabel}" />
260260
<TextBlock AutomationProperties.AutomationId="SelectedToolchainProviderTitle"
261261
FontSize="20"
262262
FontWeight="SemiBold"

DotPilot/Presentation/WorkbenchPresentationModels.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public sealed record ToolchainProviderItem(
2626
{
2727
public string DisplayName => Snapshot.Provider.DisplayName;
2828

29-
public string IssueLabel => Snapshot.IssueLabel;
29+
public string SectionLabel => Snapshot.SectionLabel;
3030

3131
public string ReadinessLabel => Snapshot.ReadinessState.ToString();
3232

0 commit comments

Comments
 (0)