From a708a3b62e0dcd06a8d8eee92cea497dd2e7ec1b Mon Sep 17 00:00:00 2001 From: Hoang Snowy Date: Thu, 22 Oct 2026 20:37:48 +0700 Subject: [PATCH] fix(codeql): clear the 11 code-scanning alerts from the last two days - cs/dereferenced-value-may-be-null: ProjectOrchestrator reads the ticket number once via a pattern. - cs/empty-catch-block: PreviewHost deletes the pid file through a filtered TryDeleteFile. - cs/linq/missed-where: TicketDependencies decrements before testing; the cycle tail uses Where. - cs/nested-if-statements: PreviewRecipe folds the timeout check into one condition. - cs/xmldoc/missing-summary: AutopilotPlanner.Decide documented. - cs/dispose-not-called-on-throw / cs/local-not-disposed: test disposables under using. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_018WAdQ7Rm24xss5QDSvXtJB --- src/AgentOs.Domain/Preview/PreviewRecipe.cs | 10 ++++----- .../Workspaces/TicketDependencies.cs | 5 +++-- src/AgentOs.RemoteAgent/PreviewHost.cs | 11 +++++++++- .../Orchestration/AutopilotPlanner.cs | 5 +++++ .../Orchestration/ProjectOrchestrator.cs | 5 +++-- .../AgentOs.Tests/Sessions/KeepAwakeTests.cs | 22 ++++++++++--------- .../AgentOs.Tests/Sessions/TryTheAppTests.cs | 4 ++-- 7 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/AgentOs.Domain/Preview/PreviewRecipe.cs b/src/AgentOs.Domain/Preview/PreviewRecipe.cs index 745f7c5..ee1b966 100644 --- a/src/AgentOs.Domain/Preview/PreviewRecipe.cs +++ b/src/AgentOs.Domain/Preview/PreviewRecipe.cs @@ -51,13 +51,11 @@ public sealed record PreviewRecipe(string Command, Uri Url, int ReadyTimeoutSeco if (!IsLocal(url)) { error = "The url must be on this computer (localhost)."; return null; } var timeout = DefaultReadyTimeoutSeconds; - if (root.TryGetProperty("readyTimeoutSeconds", out var t)) + if (root.TryGetProperty("readyTimeoutSeconds", out var t) + && (t.ValueKind != JsonValueKind.Number || !t.TryGetInt32(out timeout) || timeout is < 10 or > 3600)) { - if (t.ValueKind != JsonValueKind.Number || !t.TryGetInt32(out timeout) || timeout is < 10 or > 3600) - { - error = "\"readyTimeoutSeconds\" must be a whole number from 10 to 3600."; - return null; - } + error = "\"readyTimeoutSeconds\" must be a whole number from 10 to 3600."; + return null; } return new PreviewRecipe(command, url, timeout); } diff --git a/src/AgentOs.Domain/Workspaces/TicketDependencies.cs b/src/AgentOs.Domain/Workspaces/TicketDependencies.cs index b586801..d0fddd8 100644 --- a/src/AgentOs.Domain/Workspaces/TicketDependencies.cs +++ b/src/AgentOs.Domain/Workspaces/TicketDependencies.cs @@ -53,11 +53,12 @@ public static IReadOnlyList CreationOrder(int count, Func !placed[i])); return order; } diff --git a/src/AgentOs.RemoteAgent/PreviewHost.cs b/src/AgentOs.RemoteAgent/PreviewHost.cs index 1f8d84f..ea774e2 100644 --- a/src/AgentOs.RemoteAgent/PreviewHost.cs +++ b/src/AgentOs.RemoteAgent/PreviewHost.cs @@ -115,7 +115,7 @@ public static void KillLeftovers(string root) } finally { - try { File.Delete(pidFile); } catch (IOException) { } catch (UnauthorizedAccessException) { } + TryDeleteFile(pidFile); } } } @@ -313,6 +313,15 @@ private static async Task ConnectsAsync(System.Net.IPAddress address, int } } + private static void TryDeleteFile(string path) + { + try { File.Delete(path); } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + _ = ex; // still held by a process — the next sweep retries + } + } + private static void WritePidFile(string dir, Process process) { try diff --git a/src/AgentOs.Web/Services/Orchestration/AutopilotPlanner.cs b/src/AgentOs.Web/Services/Orchestration/AutopilotPlanner.cs index 96efeac..beb3da1 100644 --- a/src/AgentOs.Web/Services/Orchestration/AutopilotPlanner.cs +++ b/src/AgentOs.Web/Services/Orchestration/AutopilotPlanner.cs @@ -59,6 +59,11 @@ public static class AutopilotPlanner /// (seen on the lab: the second one found the ports taken and could not check its end-to-end criteria). public const int MaxParallelTests = 1; + /// What Autopilot does next on a board, given its tickets and tasks — a pure decision, executed by the + /// orchestrator. + /// Off, Build, or BuildAndMerge. + /// The board's tickets. + /// The board's open tasks and their proposed changes. /// A computer of the member is connected, so the Testing agent can run. Without one nothing /// is tested — and nothing untested is merged. public static IReadOnlyList Decide(AutopilotMode mode, IReadOnlyList tickets, IReadOnlyList tasks, diff --git a/src/AgentOs.Web/Services/Orchestration/ProjectOrchestrator.cs b/src/AgentOs.Web/Services/Orchestration/ProjectOrchestrator.cs index acd4c09..1b7c07b 100644 --- a/src/AgentOs.Web/Services/Orchestration/ProjectOrchestrator.cs +++ b/src/AgentOs.Web/Services/Orchestration/ProjectOrchestrator.cs @@ -355,13 +355,14 @@ private async Task RecoverOrphansAsync(WorkspaceEntity board, IReadOnlyLis private async Task> DependenciesAsync(ISourceProvider? provider, WorkspaceEntity board, string tenant, string pat, BoardTicket t, CancellationToken ct) { - var key = (board.Id, t.Number!.Value); + if (t.Number is not { } number) { return []; } + var key = (board.Id, number); if (_deps.TryGetValue(key, out var cached) && DateTimeOffset.UtcNow - cached.At < TimeSpan.FromMinutes(10)) { return cached.Deps; } if (provider is null) { return []; } string? body; try { - body = await provider.ReadTicketBodyAsync(BoardTaskService.RepoDescriptor(board, tenant, t.RepoOwner!, t.RepoName!, "main", pat), t.Number.Value, ct).ConfigureAwait(false); + body = await provider.ReadTicketBodyAsync(BoardTaskService.RepoDescriptor(board, tenant, t.RepoOwner!, t.RepoName!, "main", pat), number, ct).ConfigureAwait(false); } catch (Exception ex) when (ex is System.Net.Http.HttpRequestException or Octokit.ApiException or InvalidOperationException or TaskCanceledException) { diff --git a/tests/AgentOs.Tests/Sessions/KeepAwakeTests.cs b/tests/AgentOs.Tests/Sessions/KeepAwakeTests.cs index ac4b506..c0c81f4 100644 --- a/tests/AgentOs.Tests/Sessions/KeepAwakeTests.cs +++ b/tests/AgentOs.Tests/Sessions/KeepAwakeTests.cs @@ -20,15 +20,17 @@ public class KeepAwakeTests public void Hold_CountsWorkInFlight_AndReleasesOnce() { using var awake = new KeepAwake(enabled: true); - var first = awake.Hold(); - var second = awake.Hold(); - awake.Active.ShouldBe(2); - - first.Dispose(); - first.Dispose(); // a double release must not under-count - awake.Active.ShouldBe(1); - - second.Dispose(); - awake.Active.ShouldBe(0); + using (var first = awake.Hold()) + { + using (awake.Hold()) + { + awake.Active.ShouldBe(2); + first.Dispose(); + first.Dispose(); // a double release must not under-count + awake.Active.ShouldBe(1); + } + awake.Active.ShouldBe(0); + } + awake.Active.ShouldBe(0); // the outer using's second release is a no-op too } } diff --git a/tests/AgentOs.Tests/Sessions/TryTheAppTests.cs b/tests/AgentOs.Tests/Sessions/TryTheAppTests.cs index 32856c6..536b3a0 100644 --- a/tests/AgentOs.Tests/Sessions/TryTheAppTests.cs +++ b/tests/AgentOs.Tests/Sessions/TryTheAppTests.cs @@ -164,7 +164,7 @@ public void StopPreview_PushesStopToTheRunningConnection() public void RunnerDisconnect_ReportsItsPreviewsStopped() { var broker = new InProcessRemoteAgentBroker(); - var registration = broker.RegisterRunner("conn-a", new RunnerConnection(Guid.NewGuid(), "t1", "u1")); + using var registration = broker.RegisterRunner("conn-a", new RunnerConnection(Guid.NewGuid(), "t1", "u1")); var reports = new List(); broker.PreviewReported += reports.Add; broker.StartPreview(Start(), new RunnerTarget("t1", "u1")); @@ -218,7 +218,7 @@ public void ShellFor_RunsTheLineInTheCheckout() [Fact] public async Task IsListening_TrueOnlyWhileSomethingListens() { - var listener = new TcpListener(IPAddress.Loopback, 0); + using var listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); var port = ((IPEndPoint)listener.LocalEndpoint).Port; try