Skip to content

Commit e5c5b35

Browse files
committed
fix pr-76 review followups and startup probing
1 parent d2fa9aa commit e5c5b35

14 files changed

+383
-120
lines changed

AGENTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ For this app:
152152
- architecture work must keep a vertical-slice shape: each feature owns its contracts, orchestration, and tests behind clear boundaries instead of growing a shared horizontal service layer
153153
- keep the Uno app project presentation-only; domain, runtime host, orchestration, integrations, and persistence code must live in separate class-library projects so UI composition does not mix with feature implementation
154154
- structure both `DotPilot.Tests` and `DotPilot.UITests` by vertical slice and explicit harness boundaries; do not keep test files in one flat project-root pile
155+
- GitHub is the backlog, not the product: use issues and PRs only to drive task scope and traceability, and never copy GitHub issue text, labels, workflow language, or tracker metadata into production code, runtime snapshots, or user-facing UI
156+
- Desktop responsiveness is a product requirement: avoid synchronous probe, filesystem, network, or process work on UI-facing construction and navigation paths so the app stays fast and immediately reactive
157+
- Do not invent a repo-specific product framing such as "workbench" unless the active issue or feature spec explicitly uses it; implement the app features described in the backlog instead of turning internal implementation language into the product narrative
155158
- GitHub Actions workflows must use descriptive names and filenames that reflect their purpose; do not use a generic `ci.yml` catch-all because build validation and release automation are separate operator flows
156159
- GitHub Actions must be split into at least one validation workflow for normal builds/tests and one release workflow for CI-driven version resolution, release-note generation, desktop publishing, and GitHub Release publication
157160
- meaningful GitHub review comments must be evaluated and fixed when they still apply even if the original PR was closed; closed review threads are not a reason to ignore valid engineering feedback

DotPilot.Runtime/Features/RuntimeFoundation/RuntimeFoundationCatalog.cs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,44 @@
11
using DotPilot.Core.Features.ControlPlaneDomain;
22
using DotPilot.Core.Features.RuntimeFoundation;
3-
using DotPilot.Runtime.Features.ToolchainCenter;
4-
53
namespace DotPilot.Runtime.Features.RuntimeFoundation;
64

75
public sealed class RuntimeFoundationCatalog : IRuntimeFoundationCatalog
86
{
97
private const string EpicSummary =
10-
"Issue #12 is staged into isolated contracts, communication, host, and orchestration slices so the Uno workbench can stay presentation-only.";
8+
"Runtime contracts, host sequencing, and orchestration seams stay isolated so the Uno app can remain presentation-only.";
9+
private const string EpicLabelValue = "LOCAL RUNTIME READINESS";
1110
private const string DeterministicProbePrompt =
1211
"Summarize the runtime foundation readiness for a local-first session that may require approval.";
1312
private const string DeterministicClientStatusSummary = "Always available for in-repo and CI validation.";
13+
private const string DomainModelLabel = "DOMAIN";
1414
private const string DomainModelName = "Domain contracts";
1515
private const string DomainModelSummary =
1616
"Typed identifiers and durable agent, session, fleet, provider, and runtime contracts live outside the Uno app.";
17+
private const string CommunicationLabel = "CONTRACTS";
1718
private const string CommunicationName = "Communication contracts";
1819
private const string CommunicationSummary =
1920
"Public result and problem boundaries are isolated so later provider and orchestration slices share one contract language.";
21+
private const string HostLabel = "HOST";
2022
private const string HostName = "Embedded host";
2123
private const string HostSummary =
2224
"The Orleans host integration point is sequenced behind dedicated runtime contracts instead of being baked into page code.";
25+
private const string OrchestrationLabel = "ORCHESTRATION";
2326
private const string OrchestrationName = "Orchestration runtime";
2427
private const string OrchestrationSummary =
2528
"Agent Framework integration is prepared as a separate slice that can plug into the embedded host without reshaping the UI layer.";
29+
private readonly IReadOnlyList<ProviderDescriptor> _providers;
30+
31+
public RuntimeFoundationCatalog() => _providers = CreateProviders();
2632

2733
public RuntimeFoundationSnapshot GetSnapshot()
2834
{
2935
return new(
30-
RuntimeFoundationIssues.FormatIssueLabel(RuntimeFoundationIssues.EmbeddedAgentRuntimeHostEpic),
36+
EpicLabelValue,
3137
EpicSummary,
3238
ProviderToolchainNames.DeterministicClientDisplayName,
3339
DeterministicProbePrompt,
3440
CreateSlices(),
35-
CreateProviders());
41+
_providers);
3642
}
3743

3844
private static IReadOnlyList<RuntimeSliceDescriptor> CreateSlices()
@@ -41,25 +47,25 @@ private static IReadOnlyList<RuntimeSliceDescriptor> CreateSlices()
4147
[
4248
new(
4349
RuntimeFoundationIssues.DomainModel,
44-
RuntimeFoundationIssues.FormatIssueLabel(RuntimeFoundationIssues.DomainModel),
50+
DomainModelLabel,
4551
DomainModelName,
4652
DomainModelSummary,
4753
RuntimeSliceState.ReadyForImplementation),
4854
new(
4955
RuntimeFoundationIssues.CommunicationContracts,
50-
RuntimeFoundationIssues.FormatIssueLabel(RuntimeFoundationIssues.CommunicationContracts),
56+
CommunicationLabel,
5157
CommunicationName,
5258
CommunicationSummary,
5359
RuntimeSliceState.Sequenced),
5460
new(
5561
RuntimeFoundationIssues.EmbeddedOrleansHost,
56-
RuntimeFoundationIssues.FormatIssueLabel(RuntimeFoundationIssues.EmbeddedOrleansHost),
62+
HostLabel,
5763
HostName,
5864
HostSummary,
5965
RuntimeSliceState.Sequenced),
6066
new(
6167
RuntimeFoundationIssues.AgentFrameworkRuntime,
62-
RuntimeFoundationIssues.FormatIssueLabel(RuntimeFoundationIssues.AgentFrameworkRuntime),
68+
OrchestrationLabel,
6369
OrchestrationName,
6470
OrchestrationSummary,
6571
RuntimeSliceState.Sequenced),
@@ -79,8 +85,6 @@ private static IReadOnlyList<ProviderDescriptor> CreateProviders()
7985
StatusSummary = DeterministicClientStatusSummary,
8086
RequiresExternalToolchain = false,
8187
},
82-
.. ToolchainProviderSnapshotFactory.Create(TimeProvider.System.GetUtcNow())
83-
.Select(snapshot => snapshot.Provider),
8488
];
8589
}
8690
}

DotPilot.Runtime/Features/ToolchainCenter/ToolchainCenterCatalog.cs

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,31 @@ namespace DotPilot.Runtime.Features.ToolchainCenter;
44

55
public sealed class ToolchainCenterCatalog : IToolchainCenterCatalog, IDisposable
66
{
7+
private const string EpicLabelValue = "PRESESSION READINESS";
78
private const string EpicSummary =
8-
"Issue #14 keeps provider installation, auth, diagnostics, configuration, and polling visible before the first live session.";
9+
"Provider installation, launch checks, authentication, configuration, and refresh state stay visible before the first live session.";
10+
private const string UiWorkstreamLabel = "SURFACE";
911
private const string UiWorkstreamName = "Toolchain Center UI";
1012
private const string UiWorkstreamSummary =
1113
"The settings shell exposes a first-class desktop Toolchain Center with provider cards, detail panes, and operator actions.";
14+
private const string DiagnosticsWorkstreamLabel = "DIAGNOSTICS";
1215
private const string DiagnosticsWorkstreamName = "Connection diagnostics";
1316
private const string DiagnosticsWorkstreamSummary =
1417
"Launch, connection, resume, tool access, and auth diagnostics stay attributable before live work starts.";
18+
private const string ConfigurationWorkstreamLabel = "CONFIGURATION";
1519
private const string ConfigurationWorkstreamName = "Secrets and environment";
1620
private const string ConfigurationWorkstreamSummary =
1721
"Provider secrets, local overrides, and non-secret environment configuration stay visible without leaking values.";
22+
private const string PollingWorkstreamLabel = "POLLING";
1823
private const string PollingWorkstreamName = "Background polling";
1924
private const string PollingWorkstreamSummary =
20-
"Version and auth readiness refresh in the background so the workbench can surface stale state early.";
25+
"Version and auth readiness refresh in the background so the app can surface stale state early.";
2126
private readonly TimeProvider _timeProvider;
2227
private readonly CancellationTokenSource _disposeTokenSource = new();
2328
private readonly PeriodicTimer? _pollingTimer;
2429
private readonly Task _pollingTask;
2530
private ToolchainCenterSnapshot _snapshot;
31+
private int _disposeState;
2632

2733
public ToolchainCenterCatalog()
2834
: this(TimeProvider.System, startBackgroundPolling: true)
@@ -46,13 +52,29 @@ public ToolchainCenterCatalog(TimeProvider timeProvider, bool startBackgroundPol
4652
}
4753
}
4854

49-
public ToolchainCenterSnapshot GetSnapshot() => _snapshot;
55+
public ToolchainCenterSnapshot GetSnapshot() => Volatile.Read(ref _snapshot);
5056

5157
public void Dispose()
5258
{
59+
if (Interlocked.Exchange(ref _disposeState, 1) != 0)
60+
{
61+
return;
62+
}
63+
5364
_disposeTokenSource.Cancel();
5465
_pollingTimer?.Dispose();
66+
67+
try
68+
{
69+
_pollingTask.GetAwaiter().GetResult();
70+
}
71+
catch (OperationCanceledException)
72+
{
73+
// Expected during shutdown.
74+
}
75+
5576
_disposeTokenSource.Dispose();
77+
GC.SuppressFinalize(this);
5678
}
5779

5880
private async Task PollAsync()
@@ -66,21 +88,25 @@ private async Task PollAsync()
6688
{
6789
while (await _pollingTimer.WaitForNextTickAsync(_disposeTokenSource.Token))
6890
{
69-
_snapshot = EvaluateSnapshot();
91+
Volatile.Write(ref _snapshot, EvaluateSnapshot());
7092
}
7193
}
7294
catch (OperationCanceledException)
7395
{
7496
// Expected during app shutdown.
7597
}
98+
catch (ObjectDisposedException) when (_disposeTokenSource.IsCancellationRequested)
99+
{
100+
// Expected when the timer is disposed during shutdown.
101+
}
76102
}
77103

78104
private ToolchainCenterSnapshot EvaluateSnapshot()
79105
{
80106
var evaluatedAt = _timeProvider.GetUtcNow();
81107
var providers = ToolchainProviderSnapshotFactory.Create(evaluatedAt);
82108
return new(
83-
ToolchainCenterIssues.FormatIssueLabel(ToolchainCenterIssues.ToolchainCenterEpic),
109+
EpicLabelValue,
84110
EpicSummary,
85111
CreateWorkstreams(),
86112
providers,
@@ -95,22 +121,22 @@ private static IReadOnlyList<ToolchainCenterWorkstreamDescriptor> CreateWorkstre
95121
[
96122
new(
97123
ToolchainCenterIssues.ToolchainCenterUi,
98-
ToolchainCenterIssues.FormatIssueLabel(ToolchainCenterIssues.ToolchainCenterUi),
124+
UiWorkstreamLabel,
99125
UiWorkstreamName,
100126
UiWorkstreamSummary),
101127
new(
102128
ToolchainCenterIssues.ConnectionDiagnostics,
103-
ToolchainCenterIssues.FormatIssueLabel(ToolchainCenterIssues.ConnectionDiagnostics),
129+
DiagnosticsWorkstreamLabel,
104130
DiagnosticsWorkstreamName,
105131
DiagnosticsWorkstreamSummary),
106132
new(
107133
ToolchainCenterIssues.ProviderConfiguration,
108-
ToolchainCenterIssues.FormatIssueLabel(ToolchainCenterIssues.ProviderConfiguration),
134+
ConfigurationWorkstreamLabel,
109135
ConfigurationWorkstreamName,
110136
ConfigurationWorkstreamSummary),
111137
new(
112138
ToolchainCenterIssues.BackgroundPolling,
113-
ToolchainCenterIssues.FormatIssueLabel(ToolchainCenterIssues.BackgroundPolling),
139+
PollingWorkstreamLabel,
114140
PollingWorkstreamName,
115141
PollingWorkstreamSummary),
116142
];

DotPilot.Runtime/Features/ToolchainCenter/ToolchainCommandProbe.cs

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,23 @@ internal static class ToolchainCommandProbe
66
{
77
private static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(2);
88
private const string VersionSeparator = "version";
9+
private const string EmptyOutput = "";
910

1011
public static string? ResolveExecutablePath(string commandName) =>
1112
RuntimeFoundation.ProviderToolchainProbe.ResolveExecutablePath(commandName);
1213

1314
public static string ReadVersion(string executablePath, IReadOnlyList<string> arguments)
15+
=> ProbeVersion(executablePath, arguments).Version;
16+
17+
public static ToolchainVersionProbeResult ProbeVersion(string executablePath, IReadOnlyList<string> arguments)
1418
{
1519
ArgumentException.ThrowIfNullOrWhiteSpace(executablePath);
1620
ArgumentNullException.ThrowIfNull(arguments);
1721

1822
var execution = Execute(executablePath, arguments);
1923
if (!execution.Succeeded)
2024
{
21-
return string.Empty;
25+
return ToolchainVersionProbeResult.Missing with { Launched = execution.Launched };
2226
}
2327

2428
var output = string.IsNullOrWhiteSpace(execution.StandardOutput)
@@ -31,13 +35,15 @@ public static string ReadVersion(string executablePath, IReadOnlyList<string> ar
3135

3236
if (string.IsNullOrWhiteSpace(firstLine))
3337
{
34-
return string.Empty;
38+
return ToolchainVersionProbeResult.Missing with { Launched = execution.Launched };
3539
}
3640

3741
var separatorIndex = firstLine.IndexOf(VersionSeparator, StringComparison.OrdinalIgnoreCase);
38-
return separatorIndex >= 0
42+
var version = separatorIndex >= 0
3943
? firstLine[(separatorIndex + VersionSeparator.Length)..].Trim(' ', ':')
4044
: firstLine.Trim();
45+
46+
return new(execution.Launched, version);
4147
}
4248

4349
public static bool CanExecute(string executablePath, IReadOnlyList<string> arguments)
@@ -64,22 +70,57 @@ private static ToolchainCommandExecution Execute(string executablePath, IReadOnl
6470
startInfo.ArgumentList.Add(argument);
6571
}
6672

67-
using var process = Process.Start(startInfo);
73+
Process? process;
74+
try
75+
{
76+
process = Process.Start(startInfo);
77+
}
78+
catch
79+
{
80+
return ToolchainCommandExecution.LaunchFailed;
81+
}
82+
6883
if (process is null)
6984
{
70-
return ToolchainCommandExecution.Failed;
85+
return ToolchainCommandExecution.LaunchFailed;
7186
}
7287

73-
if (!process.WaitForExit((int)CommandTimeout.TotalMilliseconds))
88+
using (process)
7489
{
75-
TryTerminate(process);
76-
return ToolchainCommandExecution.Failed;
90+
var standardOutputTask = process.StandardOutput.ReadToEndAsync();
91+
var standardErrorTask = process.StandardError.ReadToEndAsync();
92+
93+
if (!process.WaitForExit((int)CommandTimeout.TotalMilliseconds))
94+
{
95+
TryTerminate(process);
96+
ObserveRedirectedStreamFaults(standardOutputTask, standardErrorTask);
97+
return new(true, false, EmptyOutput, EmptyOutput);
98+
}
99+
100+
return new(
101+
true,
102+
process.ExitCode == 0,
103+
AwaitStreamRead(standardOutputTask),
104+
AwaitStreamRead(standardErrorTask));
77105
}
106+
}
78107

79-
return new(
80-
process.ExitCode == 0,
81-
process.StandardOutput.ReadToEnd(),
82-
process.StandardError.ReadToEnd());
108+
private static string AwaitStreamRead(Task<string> readTask)
109+
{
110+
try
111+
{
112+
return readTask.GetAwaiter().GetResult();
113+
}
114+
catch
115+
{
116+
return EmptyOutput;
117+
}
118+
}
119+
120+
private static void ObserveRedirectedStreamFaults(Task<string> standardOutputTask, Task<string> standardErrorTask)
121+
{
122+
_ = standardOutputTask.Exception;
123+
_ = standardErrorTask.Exception;
83124
}
84125

85126
private static void TryTerminate(Process process)
@@ -97,8 +138,13 @@ private static void TryTerminate(Process process)
97138
}
98139
}
99140

100-
private readonly record struct ToolchainCommandExecution(bool Succeeded, string StandardOutput, string StandardError)
141+
public readonly record struct ToolchainVersionProbeResult(bool Launched, string Version)
142+
{
143+
public static ToolchainVersionProbeResult Missing => new(false, EmptyOutput);
144+
}
145+
146+
private readonly record struct ToolchainCommandExecution(bool Launched, bool Succeeded, string StandardOutput, string StandardError)
101147
{
102-
public static ToolchainCommandExecution Failed => new(false, string.Empty, string.Empty);
148+
public static ToolchainCommandExecution LaunchFailed => new(false, false, EmptyOutput, EmptyOutput);
103149
}
104150
}

DotPilot.Runtime/Features/ToolchainCenter/ToolchainProviderProfile.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ namespace DotPilot.Runtime.Features.ToolchainCenter;
22

33
internal sealed record ToolchainProviderProfile(
44
int IssueNumber,
5+
string SectionLabel,
56
string DisplayName,
67
string CommandName,
78
IReadOnlyList<string> VersionArguments,

DotPilot.Runtime/Features/ToolchainCenter/ToolchainProviderProfiles.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@ internal static class ToolchainProviderProfiles
1919
private const string GitHubTokenSummary = "GitHub token for Copilot and GitHub CLI authenticated flows.";
2020
private const string GitHubHostTokenSummary = "Alternative GitHub host token for CLI-authenticated Copilot flows.";
2121
private const string GitHubModelsApiKeySummary = "Optional BYOK key for GitHub Models-backed Copilot routing.";
22+
private const string CodexSectionLabel = "CODEX";
23+
private const string ClaudeSectionLabel = "CLAUDE";
24+
private const string GitHubSectionLabel = "GITHUB";
2225
private static readonly string[] VersionArguments = ["--version"];
2326

2427
public static IReadOnlyList<ToolchainProviderProfile> All { get; } =
2528
[
2629
new(
2730
ToolchainCenterIssues.CodexReadiness,
31+
CodexSectionLabel,
2832
ProviderToolchainNames.CodexDisplayName,
2933
ProviderToolchainNames.CodexCommandName,
3034
VersionArguments,
@@ -40,6 +44,7 @@ internal static class ToolchainProviderProfiles
4044
]),
4145
new(
4246
ToolchainCenterIssues.ClaudeCodeReadiness,
47+
ClaudeSectionLabel,
4348
ProviderToolchainNames.ClaudeCodeDisplayName,
4449
ProviderToolchainNames.ClaudeCodeCommandName,
4550
VersionArguments,
@@ -55,6 +60,7 @@ internal static class ToolchainProviderProfiles
5560
]),
5661
new(
5762
ToolchainCenterIssues.GitHubCopilotReadiness,
63+
GitHubSectionLabel,
5864
ProviderToolchainNames.GitHubCopilotDisplayName,
5965
ProviderToolchainNames.GitHubCopilotCommandName,
6066
VersionArguments,

0 commit comments

Comments
 (0)