From fc62be8cf0fadedc0d26ce2fd2e7c7d0ea8457ac Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 13:41:27 +0900 Subject: [PATCH 1/6] Fix runtime timestamp clock injection (#3700) --- changelog.d/unreleased/3700.changed.md | 20 +++++++++++++++++++ src/CodeIndex/Cli/GitHubIssueReporter.cs | 4 +++- src/CodeIndex/Cli/IndexCommandRunner.cs | 7 +++++-- src/CodeIndex/Cli/IndexLock.cs | 5 ++++- src/CodeIndex/Mcp/McpToolHandlers.cs | 2 +- .../GitHubIssueReporterTests.cs | 7 ++++--- .../IndexCommandRunnerTests.cs | 5 ++++- 7 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 changelog.d/unreleased/3700.changed.md diff --git a/changelog.d/unreleased/3700.changed.md b/changelog.d/unreleased/3700.changed.md new file mode 100644 index 0000000000..60818f33dc --- /dev/null +++ b/changelog.d/unreleased/3700.changed.md @@ -0,0 +1,20 @@ +--- +category: changed +issues: + - 3700 +affected: + - src/CodeIndex/Cli/IndexCommandRunner.cs + - src/CodeIndex/Cli/IndexLock.cs + - src/CodeIndex/Cli/GitHubIssueReporter.cs + - src/CodeIndex/Mcp/McpToolHandlers.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs + - tests/CodeIndex.Tests/GitHubIssueReporterTests.cs +--- + +## English + +- **Runtime timestamps now use injectable clocks across CLI, MCP, and GitHub flows (#3700)** — index metadata, lock-holder metadata, MCP index start times, and GitHub rate-limit retry diagnostics now use testable UTC clock sources instead of direct wall-clock reads. + +## 日本語 + +- **CLI / MCP / GitHub フローのランタイム時刻が注入可能な clock を使うようになりました (#3700)** — index metadata、lock-holder metadata、MCP index の開始時刻、GitHub rate-limit retry diagnostics は、直接 wall-clock を読む代わりにテスト可能な UTC clock source を使います。 diff --git a/src/CodeIndex/Cli/GitHubIssueReporter.cs b/src/CodeIndex/Cli/GitHubIssueReporter.cs index 8ebb4a733c..167316b587 100644 --- a/src/CodeIndex/Cli/GitHubIssueReporter.cs +++ b/src/CodeIndex/Cli/GitHubIssueReporter.cs @@ -46,6 +46,8 @@ namespace CodeIndex.Cli; /// internal static class GitHubIssueReporter { + internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; + internal static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(10); private static readonly TimeSpan DefaultRateLimitRetryDelay = TimeSpan.FromMinutes(1); private const string TimeoutEnvironmentVariable = "CDIDX_GITHUB_SUBMIT_TIMEOUT_SECONDS"; @@ -542,7 +544,7 @@ private static bool IsHexHash(string value) if (!response.IsSuccessStatusCode) { var errorBody = await ReadBoundedApiErrorBodyAsync(response.Content, cancellationToken); - var rateLimitRetryAt = GetRateLimitRetryAt(response, DateTime.UtcNow); + var rateLimitRetryAt = GetRateLimitRetryAt(response, TimeProvider.GetUtcNow().UtcDateTime); if (rateLimitRetryAt != null) { Console.Error.WriteLine(BuildRateLimitFailureMessage((int)response.StatusCode, errorBody, rateLimitRetryAt.Value)); diff --git a/src/CodeIndex/Cli/IndexCommandRunner.cs b/src/CodeIndex/Cli/IndexCommandRunner.cs index 1c79e1e445..ff231774e7 100644 --- a/src/CodeIndex/Cli/IndexCommandRunner.cs +++ b/src/CodeIndex/Cli/IndexCommandRunner.cs @@ -45,6 +45,9 @@ private sealed record ScanCheckpoint( internal static Action? DeleteScanCheckpointForTesting { get; set; } internal static Func IsInputRedirectedForTesting { get; set; } = () => Console.IsInputRedirected; internal static Func ReadLineForTesting { get; set; } = Console.ReadLine; + internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; + + private static DateTime GetUtcNow() => TimeProvider.GetUtcNow().UtcDateTime; public static int Run(string[] indexArgs, JsonSerializerOptions jsonOptions) => Run(indexArgs, jsonOptions, cancellationForTesting: null); @@ -98,7 +101,7 @@ internal static int Run(string[] indexArgs, JsonSerializerOptions jsonOptions, C var dbResolution = DbPathResolver.ResolveForIndex(options.ProjectPath, options.DbPath, options.DataDir); var dbPath = dbResolution.DbPath; var stopwatch = Stopwatch.StartNew(); - var runStartedAtUtc = DateTime.UtcNow; + var runStartedAtUtc = GetUtcNow(); var isUpdateMode = IsUpdateMode(options); var mode = options.Rebuild ? "rebuild" : isUpdateMode ? "update" : "incremental"; @@ -910,7 +913,7 @@ private static void StampIndexedHeadMetadata(DbWriter writer, string projectRoot var headSha = GitHelper.TryGetHeadCommit(projectRoot, cancellationToken); var headBranch = GitHelper.TryGetHeadBranch(projectRoot, cancellationToken); var timestamp = headSha != null - ? DateTime.UtcNow.ToString("o", System.Globalization.CultureInfo.InvariantCulture) + ? GetUtcNow().ToString("o", System.Globalization.CultureInfo.InvariantCulture) : null; writer.SetMeta(DbContext.IndexedHeadShaMetaKey, headSha); writer.SetMeta(DbContext.IndexedHeadBranchMetaKey, headBranch); diff --git a/src/CodeIndex/Cli/IndexLock.cs b/src/CodeIndex/Cli/IndexLock.cs index 86588c43c0..e9e9d54173 100644 --- a/src/CodeIndex/Cli/IndexLock.cs +++ b/src/CodeIndex/Cli/IndexLock.cs @@ -32,6 +32,9 @@ internal sealed class IndexLock : IDisposable internal static Action DeleteFileForTesting { get; set; } = File.Delete; internal static Action? CleanupDiagnosticSinkForTesting { get; set; } + internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; + + private static DateTime GetUtcNow() => TimeProvider.GetUtcNow().UtcDateTime; private IndexLock(FileStream stream, string lockPath, string infoPath) { @@ -102,7 +105,7 @@ public static IndexLock Acquire(string lockPath, string projectPath) { var info = new IndexLockInfo( Pid: Environment.ProcessId, - StartedAt: DateTime.UtcNow); + StartedAt: GetUtcNow()); DataDirectorySecurity.WritePrivateText(infoPath, SerializeInfo(info), Encoding.UTF8); } catch (Exception) diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 9536910f8a..d8a72465c6 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -5610,7 +5610,7 @@ private async Task ExecuteIndexAsync(JsonNode? id, JsonNode? args, Jso if (maxFileBytes is <= 0 or > int.MaxValue) return CreateToolErrorResponse(id, "maxFileBytes must be a positive integer less than or equal to 2147483647"); var projectPath = Path.GetFullPath(path); - var runStartedAtUtc = DateTime.UtcNow; + var runStartedAtUtc = GetUtcNow(); var runStopwatch = Stopwatch.StartNew(); var memorySamples = memoryTrace ? new JsonArray { CaptureMcpIndexMemorySample("start", runStopwatch) } diff --git a/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs b/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs index b699b69cf8..13bf8282f6 100644 --- a/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs +++ b/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs @@ -1506,23 +1506,24 @@ public async Task TryCreateIssueDetailedAsync_RateLimited_ReturnsRetryAfterDiagn rateLimitResponse); using var mockClient = new HttpClient(handler); GitHubIssueReporter.s_httpClientOverride = mockClient; + var fixedNow = new DateTimeOffset(2026, 6, 20, 12, 0, 0, TimeSpan.Zero); + GitHubIssueReporter.TimeProvider = new ManualTimeProvider(fixedNow); try { var record = MakeRecordWithKnownHash(); - var before = DateTime.UtcNow; var result = await GitHubIssueReporter.TryCreateIssueDetailedAsync(record, "1.0.0-test"); - var after = DateTime.UtcNow; Assert.Null(result.IssueUrl); Assert.Contains("429", result.Error); Assert.Contains("next_retry_at=", result.Error); Assert.NotNull(result.NextRetryAt); - Assert.InRange(result.NextRetryAt.Value, before.AddSeconds(60), after.AddSeconds(61)); + Assert.Equal(fixedNow.UtcDateTime.AddSeconds(60), result.NextRetryAt.Value); Assert.Equal(3, handler.RequestCount); } finally { GitHubIssueReporter.s_httpClientOverride = null; + GitHubIssueReporter.TimeProvider = TimeProvider.System; } } diff --git a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs index 44fdab716e..0c3e207602 100644 --- a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs @@ -3447,6 +3447,8 @@ public void Run_GitRepo_PersistsIndexedHeadMetadata() { var projectRoot = CreateTempProject(); var dbPath = Path.Combine(Path.GetTempPath(), $"cdidx_head_meta_{Guid.NewGuid():N}.db"); + var fixedNow = new DateTimeOffset(2026, 6, 20, 12, 34, 56, TimeSpan.Zero); + IndexCommandRunner.TimeProvider = new ManualTimeProvider(fixedNow); try { File.WriteAllText(Path.Combine(projectRoot, "app.py"), "print('hello')\n"); @@ -3466,7 +3468,7 @@ public void Run_GitRepo_PersistsIndexedHeadMetadata() Assert.Equal(expectedSha, db.GetMetaString(DbContext.IndexedHeadShaMetaKey)); Assert.Equal("main", db.GetMetaString(DbContext.IndexedHeadBranchMetaKey)); var stamp = db.GetMetaString(DbContext.IndexedHeadTimestampMetaKey); - Assert.False(string.IsNullOrWhiteSpace(stamp)); + Assert.Equal(fixedNow.UtcDateTime.ToString("o", System.Globalization.CultureInfo.InvariantCulture), stamp); Assert.True( DateTime.TryParse(stamp, System.Globalization.CultureInfo.InvariantCulture, System.Globalization.DateTimeStyles.RoundtripKind, out _), @@ -3474,6 +3476,7 @@ public void Run_GitRepo_PersistsIndexedHeadMetadata() } finally { + IndexCommandRunner.TimeProvider = TimeProvider.System; DeleteDirectory(projectRoot); SqliteConnection.ClearAllPools(); if (File.Exists(dbPath)) From 17b58f9ce2408530f57aa69243460436efe530bb Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 13:44:58 +0900 Subject: [PATCH 2/6] Unify update-check GitHub HTTP policy (#3750) --- changelog.d/unreleased/3750.changed.md | 17 ++++++++++ src/CodeIndex/Cli/GitHubHttpClientFactory.cs | 33 ++++++++++++++++---- src/CodeIndex/Cli/UpdateChecker.cs | 9 ++---- tests/CodeIndex.Tests/ProgramRunnerTests.cs | 27 +++++++++++++++- 4 files changed, 73 insertions(+), 13 deletions(-) create mode 100644 changelog.d/unreleased/3750.changed.md diff --git a/changelog.d/unreleased/3750.changed.md b/changelog.d/unreleased/3750.changed.md new file mode 100644 index 0000000000..f2a4ab8939 --- /dev/null +++ b/changelog.d/unreleased/3750.changed.md @@ -0,0 +1,17 @@ +--- +category: changed +issues: + - 3750 +affected: + - src/CodeIndex/Cli/GitHubHttpClientFactory.cs + - src/CodeIndex/Cli/UpdateChecker.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Update checks now reuse the shared GitHub HTTP policy (#3750)** — release and prerelease probes apply the same proxy-aware client defaults and GitHub API headers as other GitHub REST calls. + +## 日本語 + +- **update-check が共有 GitHub HTTP policy を再利用するようになりました (#3750)** — release / prerelease probe は、他の GitHub REST 呼び出しと同じ proxy-aware client defaults と GitHub API headers を適用します。 diff --git a/src/CodeIndex/Cli/GitHubHttpClientFactory.cs b/src/CodeIndex/Cli/GitHubHttpClientFactory.cs index b207df47b0..3f50fd2a64 100644 --- a/src/CodeIndex/Cli/GitHubHttpClientFactory.cs +++ b/src/CodeIndex/Cli/GitHubHttpClientFactory.cs @@ -1,10 +1,14 @@ using System.Net; +using System.Net.Http.Headers; namespace CodeIndex.Cli; internal static class GitHubHttpClientFactory { internal const string ProxyDefaultCredentialsEnvironmentVariable = "CDIDX_GITHUB_PROXY_USE_DEFAULT_CREDENTIALS"; + private const string GitHubApiVersionHeader = "X-GitHub-Api-Version"; + private const string GitHubApiVersion = "2022-11-28"; + private const string GitHubAcceptMediaType = "application/vnd.github+json"; internal static HttpClient CreateDefaultHttpClient(TimeSpan timeout) { @@ -19,16 +23,33 @@ internal static HttpClient CreateDefaultHttpClient(TimeSpan timeout) var client = new HttpClient(handler) { Timeout = timeout, - DefaultRequestHeaders = - { - { "User-Agent", "cdidx" }, - { "Accept", "application/vnd.github+json" }, - { "X-GitHub-Api-Version", "2022-11-28" }, - }, }; + ApplyDefaultHeaders(client.DefaultRequestHeaders); return client; } + internal static void ApplyDefaultHeaders(HttpRequestHeaders headers) + { + if (headers.UserAgent.Count == 0) + headers.UserAgent.Add(new ProductInfoHeaderValue(new ProductHeaderValue("cdidx"))); + + var hasGitHubAccept = false; + foreach (var accept in headers.Accept) + { + if (string.Equals(accept.MediaType, GitHubAcceptMediaType, StringComparison.OrdinalIgnoreCase)) + { + hasGitHubAccept = true; + break; + } + } + + if (!hasGitHubAccept) + headers.Accept.Add(new MediaTypeWithQualityHeaderValue(GitHubAcceptMediaType)); + + if (!headers.Contains(GitHubApiVersionHeader)) + headers.Add(GitHubApiVersionHeader, GitHubApiVersion); + } + internal static bool ShouldUseDefaultProxyCredentials() { var raw = Environment.GetEnvironmentVariable(ProxyDefaultCredentialsEnvironmentVariable)?.Trim(); diff --git a/src/CodeIndex/Cli/UpdateChecker.cs b/src/CodeIndex/Cli/UpdateChecker.cs index 1472b7f689..ede46d86aa 100644 --- a/src/CodeIndex/Cli/UpdateChecker.cs +++ b/src/CodeIndex/Cli/UpdateChecker.cs @@ -1,5 +1,4 @@ using System.Globalization; -using System.Net.Http.Headers; using System.Text.Json; using CodeIndex.Models; @@ -189,7 +188,7 @@ private static string FormatHint(string latestTag) private static async Task FetchLatestReleaseTagAsync(CancellationToken cancellationToken) { - using var client = new HttpClient { Timeout = Timeout.InfiniteTimeSpan }; + using var client = GitHubHttpClientFactory.CreateDefaultHttpClient(RequestTimeout); return await FetchLatestReleaseTagAsync(client, RequestTimeout, cancellationToken).ConfigureAwait(false); } @@ -201,8 +200,7 @@ private static string FormatHint(string latestTag) using var requestCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); requestCts.CancelAfter(timeout); using var request = new HttpRequestMessage(HttpMethod.Get, LatestReleaseUrl); - request.Headers.UserAgent.Add(new ProductInfoHeaderValue("cdidx", ConsoleUi.LoadVersion())); - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json")); + GitHubHttpClientFactory.ApplyDefaultHeaders(request.Headers); using var response = await client.SendAsync( request, @@ -222,8 +220,7 @@ private static string FormatHint(string latestTag) using var requestCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); requestCts.CancelAfter(timeout); using var request = new HttpRequestMessage(HttpMethod.Get, ReleasesUrl); - request.Headers.UserAgent.Add(new ProductInfoHeaderValue("cdidx", ConsoleUi.LoadVersion())); - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json")); + GitHubHttpClientFactory.ApplyDefaultHeaders(request.Headers); using var response = await client.SendAsync( request, diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index 14e8deae4d..a3e8d596bf 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -1714,6 +1714,26 @@ public async Task UpdateChecker_ReadLatestReleaseTagAsync_ParsesTagName() Assert.Equal("v1.27.0", tag); } + [Fact] + public async Task UpdateChecker_FetchLatestReleaseTagAsync_UsesSharedGitHubHeaders_Issue3750() + { + using var content = new ByteArrayContent(Encoding.UTF8.GetBytes("""{"tag_name":"v1.27.0"}""")); + var handler = new StaticResponseHandler(content); + using var client = new HttpClient(handler) + { + Timeout = Timeout.InfiniteTimeSpan, + }; + + var tag = await UpdateChecker.FetchLatestReleaseTagAsync(client, TimeSpan.FromSeconds(1), CancellationToken.None); + + Assert.Equal("v1.27.0", tag); + Assert.NotNull(handler.LastRequest); + Assert.Contains(handler.LastRequest!.Headers.UserAgent, value => value.Product?.Name == "cdidx"); + Assert.Contains(handler.LastRequest.Headers.Accept, value => value.MediaType == "application/vnd.github+json"); + Assert.True(handler.LastRequest.Headers.TryGetValues("X-GitHub-Api-Version", out var values)); + Assert.Contains("2022-11-28", values); + } + [Fact] public async Task UpdateChecker_ReadLatestReleaseTagAsync_RejectsOverLimitResponse() { @@ -3358,13 +3378,18 @@ private sealed class StaticResponseHandler : HttpMessageHandler { private readonly HttpContent _content; + internal HttpRequestMessage? LastRequest { get; private set; } + internal StaticResponseHandler(HttpContent content) { _content = content; } protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = _content }); + { + LastRequest = request; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = _content }); + } } private sealed class UpgradeAssetResponseHandler : HttpMessageHandler From db2b9f5f55914f3699f19aa0f0579370c041b8d0 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 13:49:12 +0900 Subject: [PATCH 3/6] Report update-check cache diagnostics (#3708) --- changelog.d/unreleased/3708.fixed.md | 16 ++++ src/CodeIndex/Cli/UpdateChecker.cs | 33 ++++++++- tests/CodeIndex.Tests/ProgramRunnerTests.cs | 82 +++++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 changelog.d/unreleased/3708.fixed.md diff --git a/changelog.d/unreleased/3708.fixed.md b/changelog.d/unreleased/3708.fixed.md new file mode 100644 index 0000000000..dc61291d0c --- /dev/null +++ b/changelog.d/unreleased/3708.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3708 +affected: + - src/CodeIndex/Cli/UpdateChecker.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Update-check cache failures now have gated diagnostics (#3708)** — malformed cache reads and cache write failures remain best-effort, but `CDIDX_UPDATE_CHECK_DIAGNOSTICS=1` emits low-noise stable diagnostics for troubleshooting. + +## 日本語 + +- **update-check cache failure に gated diagnostics が追加されました (#3708)** — malformed cache read と cache write failure は best-effort のまま維持しつつ、`CDIDX_UPDATE_CHECK_DIAGNOSTICS=1` で troubleshooting 向けの low-noise stable diagnostics を出力します。 diff --git a/src/CodeIndex/Cli/UpdateChecker.cs b/src/CodeIndex/Cli/UpdateChecker.cs index ede46d86aa..0a103638a4 100644 --- a/src/CodeIndex/Cli/UpdateChecker.cs +++ b/src/CodeIndex/Cli/UpdateChecker.cs @@ -7,6 +7,7 @@ namespace CodeIndex.Cli; internal static class UpdateChecker { internal const string DisableEnvVar = "CDIDX_DISABLE_UPDATE_CHECK"; + internal const string DiagnosticsEnvVar = "CDIDX_UPDATE_CHECK_DIAGNOSTICS"; private const string LatestReleaseUrl = "https://api.github.com/repos/Widthdom/CodeIndex/releases/latest"; private const string ReleasesUrl = "https://api.github.com/repos/Widthdom/CodeIndex/releases?per_page=20"; internal const long MaxLatestReleaseResponseBytes = 64 * 1024; @@ -15,6 +16,7 @@ internal static class UpdateChecker internal const int MaxUpdateCheckCacheJsonDepth = 8; private static readonly TimeSpan CacheTtl = TimeSpan.FromHours(24); private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(2); + internal static Action? CacheDiagnosticSinkForTesting { get; set; } internal static string? GetNewerReleaseHint(string currentVersion, CancellationToken cancellationToken = default) => GetNewerReleaseHint( @@ -325,8 +327,9 @@ private static string ResolveDefaultCachePath() : null; return new UpdateCheckCache(checkedAt, latestTag); } - catch + catch (Exception ex) { + ReportCacheDiagnostic("cache_read_failed", cachePath, ex); return null; } } @@ -346,11 +349,37 @@ private static void TryWriteCache(string cachePath, UpdateCheckCache cache) }; AtomicFileWriter.WriteJson(cachePath, payload, applyFileMode: DataDirectorySecurity.ApplyPrivateFileMode); } - catch + catch (Exception ex) { + ReportCacheDiagnostic("cache_write_failed", cachePath, ex); } } + private static void ReportCacheDiagnostic(string code, string cachePath, Exception ex) + { + if (!ShouldEmitCacheDiagnostics()) + return; + + var message = + $"update_check_cache_diagnostic code={code} " + + $"path={ConsoleUi.FormatBoundedValue(cachePath)} " + + $"error={CommandErrorWriter.FormatSanitizedException(ex)}"; + var sink = CacheDiagnosticSinkForTesting; + if (sink != null) + sink(message); + else + Console.Error.WriteLine(message); + } + + private static bool ShouldEmitCacheDiagnostics() + { + var value = Environment.GetEnvironmentVariable(DiagnosticsEnvVar)?.Trim(); + return value is not null + && (string.Equals(value, "1", StringComparison.OrdinalIgnoreCase) + || string.Equals(value, "true", StringComparison.OrdinalIgnoreCase) + || string.Equals(value, "yes", StringComparison.OrdinalIgnoreCase)); + } + private static bool TryParseVersion(string value, out Version version) { var trimmed = value.Trim(); diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index a3e8d596bf..4bb40c3c22 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -971,6 +971,88 @@ public void UpdateChecker_Check_IgnoresOverDepthCache() } } + [Fact] + public void UpdateChecker_Check_MalformedCacheDiagnosticsAreGated_Issue3708() + { + lock (TestConsoleLock.Gate) + { + var cachePathWithoutDiagnostics = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_{Guid.NewGuid():N}.json"); + var cachePathWithDiagnostics = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_{Guid.NewGuid():N}.json"); + using var env = EnvironmentVariableScope.Capture(UpdateChecker.DiagnosticsEnvVar); + var diagnostics = new List(); + UpdateChecker.CacheDiagnosticSinkForTesting = diagnostics.Add; + try + { + File.WriteAllText(cachePathWithoutDiagnostics, "{"); + env.Set(UpdateChecker.DiagnosticsEnvVar, null); + + var quietResult = UpdateChecker.Check( + "1.10.0", + cachePathWithoutDiagnostics, + DateTimeOffset.Parse("2026-01-01T00:00:00Z"), + _ => Task.FromResult("v1.11.0")); + + Assert.True(quietResult.UpdateAvailable); + Assert.Empty(diagnostics); + + File.WriteAllText(cachePathWithDiagnostics, "{"); + env.Set(UpdateChecker.DiagnosticsEnvVar, "1"); + + var diagnosticResult = UpdateChecker.Check( + "1.10.0", + cachePathWithDiagnostics, + DateTimeOffset.Parse("2026-01-01T00:00:00Z"), + _ => Task.FromResult("v1.11.0")); + + Assert.True(diagnosticResult.UpdateAvailable); + var diagnostic = Assert.Single(diagnostics, value => value.Contains("code=cache_read_failed", StringComparison.Ordinal)); + Assert.Contains("update_check_cache_diagnostic", diagnostic); + Assert.Contains("Json", diagnostic, StringComparison.Ordinal); + } + finally + { + UpdateChecker.CacheDiagnosticSinkForTesting = null; + if (File.Exists(cachePathWithoutDiagnostics)) + File.Delete(cachePathWithoutDiagnostics); + if (File.Exists(cachePathWithDiagnostics)) + File.Delete(cachePathWithDiagnostics); + } + } + } + + [Fact] + public void UpdateChecker_Check_UnwritableCachePathReportsWriteDiagnostic_Issue3708() + { + lock (TestConsoleLock.Gate) + { + var cachePath = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_dir_{Guid.NewGuid():N}"); + Directory.CreateDirectory(cachePath); + using var env = EnvironmentVariableScope.Capture(UpdateChecker.DiagnosticsEnvVar); + env.Set(UpdateChecker.DiagnosticsEnvVar, "1"); + var diagnostics = new List(); + UpdateChecker.CacheDiagnosticSinkForTesting = diagnostics.Add; + try + { + var result = UpdateChecker.Check( + "1.10.0", + cachePath, + DateTimeOffset.Parse("2026-01-01T00:00:00Z"), + _ => Task.FromResult("v1.11.0")); + + Assert.True(result.UpdateAvailable); + var diagnostic = Assert.Single(diagnostics); + Assert.Contains("update_check_cache_diagnostic", diagnostic); + Assert.Contains("code=cache_write_failed", diagnostic); + } + finally + { + UpdateChecker.CacheDiagnosticSinkForTesting = null; + if (Directory.Exists(cachePath)) + Directory.Delete(cachePath); + } + } + } + [Fact] public void UpdateChecker_Check_PassesCallerCancellationTokenToFetch() { From 8ff45b4d396168bff0a1ce4b21ba213f91e19a85 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 13:58:32 +0900 Subject: [PATCH 4/6] Harden update-check cache behavior (#3822) --- changelog.d/unreleased/3822.fixed.md | 16 +++ src/CodeIndex/Cli/UpdateChecker.cs | 120 ++++++++++++++-- tests/CodeIndex.Tests/ProgramRunnerTests.cs | 149 +++++++++++++++++++- 3 files changed, 269 insertions(+), 16 deletions(-) create mode 100644 changelog.d/unreleased/3822.fixed.md diff --git a/changelog.d/unreleased/3822.fixed.md b/changelog.d/unreleased/3822.fixed.md new file mode 100644 index 0000000000..669495009f --- /dev/null +++ b/changelog.d/unreleased/3822.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3822 +affected: + - src/CodeIndex/Cli/UpdateChecker.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Update-check cache hardening was tightened (#3822)** — failed or null release probes no longer refresh the cache TTL, cache roots are validated before use, and GitHub rate-limit responses now report bounded retry metadata. + +## 日本語 + +- **update-check cache hardening を強化しました (#3822)** — 失敗または null の release probe では cache TTL を更新せず、cache root を使用前に検証し、GitHub rate-limit response では bounded retry metadata を報告するようにしました。 diff --git a/src/CodeIndex/Cli/UpdateChecker.cs b/src/CodeIndex/Cli/UpdateChecker.cs index 0a103638a4..ecc6dc0344 100644 --- a/src/CodeIndex/Cli/UpdateChecker.cs +++ b/src/CodeIndex/Cli/UpdateChecker.cs @@ -14,15 +14,17 @@ internal static class UpdateChecker internal const int MaxLatestReleaseJsonDepth = 16; internal const int MaxUpdateCheckCacheBytes = 8 * 1024; internal const int MaxUpdateCheckCacheJsonDepth = 8; + internal const int MaxUpdateCheckCacheRootLength = 4096; private static readonly TimeSpan CacheTtl = TimeSpan.FromHours(24); private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(2); + internal static TimeProvider TimeProvider { get; set; } = System.TimeProvider.System; internal static Action? CacheDiagnosticSinkForTesting { get; set; } internal static string? GetNewerReleaseHint(string currentVersion, CancellationToken cancellationToken = default) => GetNewerReleaseHint( currentVersion, ResolveDefaultCachePath(), - DateTimeOffset.UtcNow, + TimeProvider.GetUtcNow(), FetchLatestReleaseTagAsync, cancellationToken); @@ -30,7 +32,7 @@ internal static UpdateCheckResult Check(string currentVersion, CancellationToken => Check( currentVersion, ResolveDefaultCachePath(), - DateTimeOffset.UtcNow, + TimeProvider.GetUtcNow(), FetchLatestReleaseTagAsync, cancellationToken); @@ -53,11 +55,15 @@ internal static UpdateCheckResult Check( if (!fromCache) { + string? fetchedLatestTag = null; try { - latestTag = fetchLatestReleaseTagAsync(cancellationToken) + fetchedLatestTag = fetchLatestReleaseTagAsync(cancellationToken) .GetAwaiter() .GetResult(); + latestTag = string.IsNullOrWhiteSpace(fetchedLatestTag) + ? cache?.LatestTag + : fetchedLatestTag; } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { @@ -72,7 +78,8 @@ internal static UpdateCheckResult Check( errorHint = failure.Hint; } - TryWriteCache(cachePath, new UpdateCheckCache(now, latestTag)); + if (!string.IsNullOrWhiteSpace(fetchedLatestTag)) + TryWriteCache(cachePath, new UpdateCheckCache(now, fetchedLatestTag)); } return new UpdateCheckResult( @@ -97,6 +104,14 @@ internal static UpdateCheckResult CreateDisabledResult(string currentVersion) internal static UpdateCheckFailure ClassifyFailure(Exception ex) { + if (ex is UpdateCheckRateLimitException rateLimit) + { + return new( + "rate_limited", + "rate_limit", + $"GitHub release API rate limit reached; retry after {rateLimit.NextRetryAt:O}. {rateLimit.Detail}"); + } + if (ex is OperationCanceledException or TimeoutException) { return new( @@ -150,11 +165,15 @@ internal static UpdateCheckFailure ClassifyFailure(Exception ex) return null; string? latestTag = null; + string? fetchedLatestTag = null; try { - latestTag = fetchLatestReleaseTagAsync(cancellationToken) + fetchedLatestTag = fetchLatestReleaseTagAsync(cancellationToken) .GetAwaiter() .GetResult(); + latestTag = string.IsNullOrWhiteSpace(fetchedLatestTag) + ? cache?.LatestTag + : fetchedLatestTag; } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { @@ -165,7 +184,8 @@ internal static UpdateCheckFailure ClassifyFailure(Exception ex) latestTag = cache?.LatestTag; } - TryWriteCache(cachePath, new UpdateCheckCache(now, latestTag)); + if (!string.IsNullOrWhiteSpace(fetchedLatestTag)) + TryWriteCache(cachePath, new UpdateCheckCache(now, fetchedLatestTag)); return IsNewerRelease(latestTag, currentVersion) ? FormatHint(latestTag!) : null; } @@ -209,7 +229,10 @@ private static string FormatHint(string latestTag) HttpCompletionOption.ResponseHeadersRead, requestCts.Token).ConfigureAwait(false); if (!response.IsSuccessStatusCode) + { + await ThrowIfRateLimitedAsync(response, requestCts.Token).ConfigureAwait(false); return null; + } return await ReadLatestReleaseTagAsync(response.Content, requestCts.Token).ConfigureAwait(false); } @@ -229,11 +252,30 @@ private static string FormatHint(string latestTag) HttpCompletionOption.ResponseHeadersRead, requestCts.Token).ConfigureAwait(false); if (!response.IsSuccessStatusCode) + { + await ThrowIfRateLimitedAsync(response, requestCts.Token).ConfigureAwait(false); return null; + } return await ReadLatestPrereleaseTagAsync(response.Content, requestCts.Token).ConfigureAwait(false); } + private static async Task ThrowIfRateLimitedAsync(HttpResponseMessage response, CancellationToken cancellationToken) + { + var nextRetryAt = GitHubIssueReporter.GetRateLimitRetryAt( + response, + TimeProvider.GetUtcNow().UtcDateTime); + if (nextRetryAt is null) + return; + + await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + var errorBody = await GitHubIssueReporter.ReadBoundedApiErrorBodyAsync(stream, cancellationToken) + .ConfigureAwait(false); + throw new UpdateCheckRateLimitException( + nextRetryAt.Value, + GitHubIssueReporter.BuildRateLimitErrorDetail((int)response.StatusCode, errorBody, nextRetryAt.Value)); + } + internal static async Task ReadLatestReleaseTagAsync(HttpContent content, CancellationToken cancellationToken) { var payload = await BoundedHttpContentReader.ReadAsByteArrayAsync( @@ -282,21 +324,57 @@ private static string FormatHint(string latestTag) return null; } - private static string ResolveDefaultCachePath() + internal static string ResolveDefaultCachePath() { var xdgCacheHome = Environment.GetEnvironmentVariable("XDG_CACHE_HOME"); var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); var localAppData = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); - var root = !string.IsNullOrWhiteSpace(xdgCacheHome) - ? Path.Combine(xdgCacheHome, "cdidx") - : !string.IsNullOrWhiteSpace(home) - ? Path.Combine(home, ".cache", "cdidx") - : !string.IsNullOrWhiteSpace(localAppData) - ? Path.Combine(localAppData, "cdidx") - : Path.Combine(Path.GetTempPath(), "cdidx", "cache"); + + if (TryResolveCacheRoot(xdgCacheHome, "XDG_CACHE_HOME", out var xdgRoot)) + return Path.Combine(xdgRoot, "cdidx", "update-check.json"); + + if (TryResolveCacheRoot(home, "user profile", out var homeRoot)) + return Path.Combine(homeRoot, ".cache", "cdidx", "update-check.json"); + + if (TryResolveCacheRoot(localAppData, "local application data", out var localAppDataRoot)) + return Path.Combine(localAppDataRoot, "cdidx", "update-check.json"); + + var root = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "cdidx", "cache")); return Path.Combine(root, "update-check.json"); } + private static bool TryResolveCacheRoot(string? value, string source, out string root) + { + root = string.Empty; + if (string.IsNullOrWhiteSpace(value)) + return false; + + if (value.Length > MaxUpdateCheckCacheRootLength + || value.Any(char.IsControl) + || !Path.IsPathFullyQualified(value)) + { + ReportCacheDiagnostic( + "cache_root_invalid", + value, + new InvalidOperationException($"{source} must be a fully qualified cache root without control characters.")); + return false; + } + + try + { + root = Path.GetFullPath(value); + return true; + } + catch (Exception ex) when (ex is ArgumentException + or IOException + or NotSupportedException + or PathTooLongException) + { + ReportCacheDiagnostic("cache_root_invalid", value, ex); + return false; + } + } + private static UpdateCheckCache? ReadCache(string cachePath) { try @@ -396,4 +474,18 @@ private static bool TryParseVersion(string value, out Version version) private sealed record UpdateCheckCache(DateTimeOffset CheckedAt, string? LatestTag); internal readonly record struct UpdateCheckFailure(string Code, string Category, string Hint); + + private sealed class UpdateCheckRateLimitException : HttpRequestException + { + internal UpdateCheckRateLimitException(DateTime nextRetryAt, string detail) + : base("GitHub release API rate limit response.") + { + NextRetryAt = nextRetryAt; + Detail = detail; + } + + internal DateTime NextRetryAt { get; } + + internal string Detail { get; } + } } diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index 4bb40c3c22..0c80803a4d 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -1053,6 +1053,133 @@ public void UpdateChecker_Check_UnwritableCachePathReportsWriteDiagnostic_Issue3 } } + [Fact] + public void UpdateChecker_Check_TransientFailureDoesNotRefreshStaleCache_Issue3822() + { + var cachePath = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_{Guid.NewGuid():N}.json"); + var checkedAt = DateTimeOffset.Parse("2025-12-31T00:00:00Z", CultureInfo.InvariantCulture); + var originalCache = + $$"""{"checked_at":"{{checkedAt.UtcDateTime.ToString("O", CultureInfo.InvariantCulture)}}","latest_tag":"v9.9.9"}"""; + try + { + File.WriteAllText(cachePath, originalCache); + + var result = UpdateChecker.Check( + "1.10.0", + cachePath, + DateTimeOffset.Parse("2026-01-02T00:00:00Z", CultureInfo.InvariantCulture), + _ => throw new HttpRequestException("secret host detail")); + + Assert.Equal("network_failure", result.Error); + Assert.Equal("v9.9.9", result.LatestVersion); + Assert.True(result.UpdateAvailable); + Assert.False(result.FromCache); + Assert.Equal(originalCache, File.ReadAllText(cachePath)); + } + finally + { + if (File.Exists(cachePath)) + File.Delete(cachePath); + } + } + + [Fact] + public void UpdateChecker_Check_NullFetchDoesNotCreateCache_Issue3822() + { + var cachePath = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_{Guid.NewGuid():N}.json"); + try + { + var result = UpdateChecker.Check( + "1.10.0", + cachePath, + DateTimeOffset.Parse("2026-01-02T00:00:00Z", CultureInfo.InvariantCulture), + _ => Task.FromResult(null)); + + Assert.Null(result.LatestVersion); + Assert.False(result.UpdateAvailable); + Assert.False(File.Exists(cachePath)); + } + finally + { + if (File.Exists(cachePath)) + File.Delete(cachePath); + } + } + + [Fact] + public void UpdateChecker_ResolveDefaultCachePath_IgnoresRelativeXdgCacheHome_Issue3822() + { + lock (TestConsoleLock.Gate) + { + using var env = EnvironmentVariableScope.Capture("XDG_CACHE_HOME", UpdateChecker.DiagnosticsEnvVar); + env.Set("XDG_CACHE_HOME", "relative-cache-root"); + env.Set(UpdateChecker.DiagnosticsEnvVar, "1"); + var diagnostics = new List(); + UpdateChecker.CacheDiagnosticSinkForTesting = diagnostics.Add; + try + { + var path = UpdateChecker.ResolveDefaultCachePath(); + + Assert.True(Path.IsPathFullyQualified(path)); + Assert.DoesNotContain("relative-cache-root", path, StringComparison.Ordinal); + var diagnostic = Assert.Single(diagnostics, value => value.Contains("code=cache_root_invalid", StringComparison.Ordinal)); + Assert.Contains("update_check_cache_diagnostic", diagnostic); + } + finally + { + UpdateChecker.CacheDiagnosticSinkForTesting = null; + } + } + } + + [Fact] + public void UpdateChecker_Check_RateLimitResponseReportsRetryMetadata_Issue3822() + { + lock (TestConsoleLock.Gate) + { + var cachePath = Path.Combine(Path.GetTempPath(), $"cdidx_update_check_{Guid.NewGuid():N}.json"); + var now = new DateTimeOffset(2026, 6, 20, 12, 0, 0, TimeSpan.Zero); + var expectedRetryAt = now.UtcDateTime.AddSeconds(90); + var previousTimeProvider = UpdateChecker.TimeProvider; + UpdateChecker.TimeProvider = new FixedTimeProvider(now); + try + { + using var content = new ByteArrayContent(Encoding.UTF8.GetBytes( + """{"message":"rate limited","token":"secret-token"}""")); + var handler = new StaticResponseHandler(content, (HttpStatusCode)429) + { + ConfigureResponse = response => + response.Headers.RetryAfter = new System.Net.Http.Headers.RetryConditionHeaderValue( + TimeSpan.FromSeconds(90)), + }; + using var client = new HttpClient(handler) + { + Timeout = Timeout.InfiniteTimeSpan, + }; + + var result = UpdateChecker.Check( + "1.10.0", + cachePath, + now, + token => UpdateChecker.FetchLatestReleaseTagAsync(client, TimeSpan.FromSeconds(1), token)); + + Assert.Equal("rate_limited", result.Error); + Assert.Equal("rate_limit", result.ErrorCategory); + Assert.Contains("429", result.ErrorHint); + Assert.Contains("next_retry_at=", result.ErrorHint); + Assert.Contains(expectedRetryAt.ToString("O", CultureInfo.InvariantCulture), result.ErrorHint); + Assert.DoesNotContain("secret-token", JsonSerializer.Serialize(result), StringComparison.Ordinal); + Assert.False(File.Exists(cachePath)); + } + finally + { + UpdateChecker.TimeProvider = previousTimeProvider; + if (File.Exists(cachePath)) + File.Delete(cachePath); + } + } + } + [Fact] public void UpdateChecker_Check_PassesCallerCancellationTokenToFetch() { @@ -3305,6 +3432,18 @@ protected override void Dispose(bool disposing) } } + private sealed class FixedTimeProvider : TimeProvider + { + private readonly DateTimeOffset _now; + + internal FixedTimeProvider(DateTimeOffset now) + { + _now = now; + } + + public override DateTimeOffset GetUtcNow() => _now; + } + // --- --audit-log flag parsing (#1562) --- [Fact] @@ -3459,18 +3598,24 @@ public void TryConsumeAuditLogFlags_DbValueLooksLikeAuditFlag_PreservedAsDbValue private sealed class StaticResponseHandler : HttpMessageHandler { private readonly HttpContent _content; + private readonly HttpStatusCode _statusCode; internal HttpRequestMessage? LastRequest { get; private set; } - internal StaticResponseHandler(HttpContent content) + internal Action? ConfigureResponse { get; init; } + + internal StaticResponseHandler(HttpContent content, HttpStatusCode statusCode = HttpStatusCode.OK) { _content = content; + _statusCode = statusCode; } protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { LastRequest = request; - return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = _content }); + var response = new HttpResponseMessage(_statusCode) { Content = _content }; + ConfigureResponse?.Invoke(response); + return Task.FromResult(response); } } From 34f1d609b9a05bd4a65f3f343aa7cad9a7142598 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 14:19:42 +0900 Subject: [PATCH 5/6] Harden GitHub duplicate preflight (#3823) --- changelog.d/unreleased/3823.fixed.md | 21 ++ src/CodeIndex/Cli/GitHubIssueReporter.cs | 64 +++- src/CodeIndex/Cli/IssueDuplicatePreflight.cs | 304 +++++++++++++++--- src/CodeIndex/Cli/ProgramRunner.Dispatch.cs | 4 +- src/CodeIndex/Cli/QueryCommandRunner.cs | 53 ++- src/CodeIndex/Cli/SuggestionsCommandRunner.cs | 41 ++- .../GitHubIssueReporterTests.cs | 100 +++++- .../IssueDuplicatePreflightTests.cs | 110 +++++++ tests/CodeIndex.Tests/ProgramCliTests.cs | 3 +- 9 files changed, 611 insertions(+), 89 deletions(-) create mode 100644 changelog.d/unreleased/3823.fixed.md diff --git a/changelog.d/unreleased/3823.fixed.md b/changelog.d/unreleased/3823.fixed.md new file mode 100644 index 0000000000..cbc31eb83d --- /dev/null +++ b/changelog.d/unreleased/3823.fixed.md @@ -0,0 +1,21 @@ +--- +category: fixed +issues: + - 3823 +affected: + - src/CodeIndex/Cli/GitHubIssueReporter.cs + - src/CodeIndex/Cli/IssueDuplicatePreflight.cs + - src/CodeIndex/Cli/ProgramRunner.Dispatch.cs + - src/CodeIndex/Cli/QueryCommandRunner.cs + - src/CodeIndex/Cli/SuggestionsCommandRunner.cs + - tests/CodeIndex.Tests/GitHubIssueReporterTests.cs + - tests/CodeIndex.Tests/IssueDuplicatePreflightTests.cs +--- + +## English + +- **GitHub duplicate preflight diagnostics were hardened (#3823)** — GitHub open-issue preflight now uses cancellable async requests, reports bounded rate-limit retry metadata, sanitizes exception diagnostics, and adds duplicate confidence/signals from safe body and evidence-path matching. + +## 日本語 + +- **GitHub duplicate preflight diagnostics を強化しました (#3823)** — GitHub open-issue preflight は cancellable async request を使い、bounded rate-limit retry metadata を報告し、exception diagnostics を sanitize し、安全な body / evidence-path matching から duplicate confidence/signals を追加します。 diff --git a/src/CodeIndex/Cli/GitHubIssueReporter.cs b/src/CodeIndex/Cli/GitHubIssueReporter.cs index 167316b587..c832c23fb6 100644 --- a/src/CodeIndex/Cli/GitHubIssueReporter.cs +++ b/src/CodeIndex/Cli/GitHubIssueReporter.cs @@ -133,7 +133,11 @@ private static HttpClient CreateDefaultHttpClient() // レスポンスが消失した場合、ローカルレコードでは SubmittedToGitHub=false の // ままになる。再試行で重複 Issue を作らないよう、新規 POST 前に // 当該提案ハッシュを含む既存 Issue を探す。 - var existingLookup = await FindExistingIssueByHashDetailedAsync(record.Hash, token, BuildIssueLabels(record), linkedCts.Token); + var existingLookup = await FindExistingIssueByHashDetailedAsync( + record.Hash, + token, + BuildExistingSuggestionLookupLabels(record), + linkedCts.Token); if (existingLookup.Error != null) { Console.Error.WriteLine(BuildSubmissionFailureMessage(existingLookup.Error)); @@ -155,8 +159,9 @@ private static HttpClient CreateDefaultHttpClient() { // Best-effort: log to stderr but do not propagate. // ベストエフォート: stderr にログ出力するが伝播しない。 - Console.Error.WriteLine(BuildSubmissionFailureMessage(ex.Message)); - return SuggestionStore.SubmitAttemptResult.Failure($"{ex.GetType().Name}: {ex.Message}"); + var detail = CommandErrorWriter.FormatSanitizedException(ex); + Console.Error.WriteLine(BuildSubmissionFailureMessage(detail)); + return SuggestionStore.SubmitAttemptResult.Failure(detail); } } @@ -252,9 +257,10 @@ private static async Task SearchExistingIssueByHashAs cancellationToken); if (!response.IsSuccessStatusCode) { - var errorBody = await ReadBoundedApiErrorBodyAsync(response.Content, cancellationToken); return ExistingIssueLookupResult.Failure( - BuildExistingSuggestionLookupFailure("search", BuildApiErrorDetail((int)response.StatusCode, errorBody))); + BuildExistingSuggestionLookupFailure( + "search", + await BuildGitHubApiErrorDetailAsync(response, cancellationToken).ConfigureAwait(false))); } JsonNode? node; @@ -265,7 +271,7 @@ private static async Task SearchExistingIssueByHashAs catch (Exception ex) when (IsRecoverableGitHubApiResponseException(ex)) { return ExistingIssueLookupResult.Failure( - BuildExistingSuggestionLookupFailure("search", $"{ex.GetType().Name}: {ex.Message}")); + BuildExistingSuggestionLookupFailure("search", CommandErrorWriter.FormatSanitizedException(ex))); } var items = node?["items"] as JsonArray; @@ -290,7 +296,7 @@ private static async Task SearchExistingIssueByHashAs catch (Exception ex) when (IsRecoverableGitHubApiResponseException(ex)) { return ExistingIssueLookupResult.Failure( - BuildExistingSuggestionLookupFailure("search", $"{ex.GetType().Name}: {ex.Message}")); + BuildExistingSuggestionLookupFailure("search", CommandErrorWriter.FormatSanitizedException(ex))); } return ExistingIssueLookupResult.NotFound; @@ -302,8 +308,11 @@ private static async Task ListExistingSuggestionIssue IReadOnlyList lookupLabels, CancellationToken cancellationToken) { - foreach (var label in lookupLabels.Distinct(StringComparer.OrdinalIgnoreCase)) + var labelsToQuery = lookupLabels.Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + for (var labelIndex = 0; labelIndex < labelsToQuery.Count; labelIndex++) { + var label = labelsToQuery[labelIndex]; + var sawCandidateIssueForLabel = false; for (var page = 1; page <= MaxExistingSuggestionLookupPagesPerLabel; page++) { var labels = Uri.EscapeDataString(label); @@ -318,11 +327,10 @@ private static async Task ListExistingSuggestionIssue cancellationToken); if (!response.IsSuccessStatusCode) { - var errorBody = await ReadBoundedApiErrorBodyAsync(response.Content, cancellationToken); return ExistingIssueLookupResult.Failure( BuildExistingSuggestionLookupFailure( $"label list '{label}' page {page}", - BuildApiErrorDetail((int)response.StatusCode, errorBody))); + await BuildGitHubApiErrorDetailAsync(response, cancellationToken).ConfigureAwait(false))); } JsonNode? node; @@ -335,7 +343,7 @@ private static async Task ListExistingSuggestionIssue return ExistingIssueLookupResult.Failure( BuildExistingSuggestionLookupFailure( $"label list '{label}' page {page}", - $"{ex.GetType().Name}: {ex.Message}")); + CommandErrorWriter.FormatSanitizedException(ex))); } var items = node as JsonArray; @@ -356,6 +364,8 @@ private static async Task ListExistingSuggestionIssue { var body = item?["body"]?.GetValue(); var itemUrl = TryGetOpenIssueUrl(item); + if (itemUrl != null) + sawCandidateIssueForLabel = true; if (itemUrl != null && body != null && body.Contains(hash, StringComparison.Ordinal)) return ExistingIssueLookupResult.Found(itemUrl); } @@ -365,15 +375,28 @@ private static async Task ListExistingSuggestionIssue return ExistingIssueLookupResult.Failure( BuildExistingSuggestionLookupFailure( $"label list '{label}' page {page}", - $"{ex.GetType().Name}: {ex.Message}")); + CommandErrorWriter.FormatSanitizedException(ex))); } if (items.Count < 100) break; if (page == MaxExistingSuggestionLookupPagesPerLabel) + { WriteExistingSuggestionLookupPageCapWarning(label); + return ExistingIssueLookupResult.NotFound; + } } + + // Fan out to supplemental labels only when the primary label returned + // plausible open issues. An empty primary list is treated as a bounded + // "no local backstop candidates" result to avoid scanning every cdidx + // label on the common no-duplicate path. + // primary label が open issue 候補を返した場合だけ補助 label に広げる。 + // 空の primary list は bounded な候補なしとして扱い、通常の重複なし経路で + // cdidx label 全体を走査しない。 + if (labelIndex == 0 && !sawCandidateIssueForLabel) + return ExistingIssueLookupResult.NotFound; } return ExistingIssueLookupResult.NotFound; @@ -391,6 +414,17 @@ private sealed record ExistingIssueLookupResult(string? IssueUrl, string? Error) private static string BuildExistingSuggestionLookupFailure(string phase, string detail) => $"GitHub existing-suggestion lookup failed during {phase}: {detail}"; + private static async Task BuildGitHubApiErrorDetailAsync( + HttpResponseMessage response, + CancellationToken cancellationToken) + { + var errorBody = await ReadBoundedApiErrorBodyAsync(response.Content, cancellationToken).ConfigureAwait(false); + var rateLimitRetryAt = GetRateLimitRetryAt(response, TimeProvider.GetUtcNow().UtcDateTime); + return rateLimitRetryAt is null + ? BuildApiErrorDetail((int)response.StatusCode, errorBody) + : BuildRateLimitErrorDetail((int)response.StatusCode, errorBody, rateLimitRetryAt.Value); + } + private static void WriteExistingSuggestionLookupPageCapWarning(string label) { var boundedLabel = SanitizeExistingSuggestionLookupLabelForWarning(label); @@ -659,6 +693,12 @@ internal static string[] BuildIssueLabels(SuggestionRecord record) : ["enhancement"]; } + private static string[] BuildExistingSuggestionLookupLabels(SuggestionRecord record) + => BuildIssueLabels(record) + .Concat(ExistingSuggestionLookupLabels) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + private static List NormalizeEvidencePaths(string[]? paths) => SuggestionEvidencePaths.Normalize(paths); diff --git a/src/CodeIndex/Cli/IssueDuplicatePreflight.cs b/src/CodeIndex/Cli/IssueDuplicatePreflight.cs index 40e1a7d916..758ad9ba0c 100644 --- a/src/CodeIndex/Cli/IssueDuplicatePreflight.cs +++ b/src/CodeIndex/Cli/IssueDuplicatePreflight.cs @@ -17,7 +17,12 @@ internal sealed class IssueDuplicatePreflight internal const int MaxOpenIssueLabelLength = 128; internal const int MaxOpenIssueNumberLength = 32; internal const int MaxTitleTokenizationInputLength = MaxOpenIssueTitleLength; + internal const int MaxOpenIssueBodyLength = 24 * 1024; + internal const int MaxBodyTokenizationInputLength = 4096; internal const int MaxGitHubRepositoryLength = 200; + internal const double TitleLabelSimilarityThreshold = 0.45; + internal const double EvidencePathSimilarityThreshold = 0.34; + internal const double BodyLabelSimilarityThreshold = 0.35; private const int GitHubOpenIssuesPerPage = 100; private const int MaxGitHubOpenIssuePages = (MaxOpenIssueCount / GitHubOpenIssuesPerPage) + 1; private const string GitHubSourceName = "github"; @@ -99,37 +104,47 @@ public static bool TryLoad(string? path, out IssueDuplicatePreflight preflight, catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or JsonException) { preflight = new IssueDuplicatePreflight(false, null, []); - error = $"could not read --open-issues file '{path}': {ex.Message}"; + error = $"could not read --open-issues file '{path}': {CommandErrorWriter.FormatSanitizedException(ex)}"; return false; } } public static bool TryLoad(string? source, string? repository, out IssueDuplicatePreflight preflight, out string? error) { - error = null; + var result = TryLoadAsync(source, repository, CancellationToken.None).GetAwaiter().GetResult(); + preflight = result.Preflight; + error = result.Error; + return result.Loaded; + } + + internal static async Task TryLoadAsync( + string? source, + string? repository, + CancellationToken cancellationToken = default) + { + cancellationToken.ThrowIfCancellationRequested(); if (!IsGitHubOpenIssuesSource(source)) { if (!string.IsNullOrWhiteSpace(repository)) - { - preflight = new IssueDuplicatePreflight(false, null, []); - error = "--repo can only be used with `--open-issues github`."; - return false; - } + return IssueDuplicatePreflightLoadResult.Failure("--repo can only be used with `--open-issues github`."); - return TryLoad(source, out preflight, out error); + return TryLoad(source, out var filePreflight, out var fileError) + ? IssueDuplicatePreflightLoadResult.Success(filePreflight) + : IssueDuplicatePreflightLoadResult.Failure(fileError!); } var requestedRepository = ExtractGitHubRepository(source, repository); - if (!TryNormalizeGitHubRepository(requestedRepository, out var normalizedRepository, out error)) - { - preflight = new IssueDuplicatePreflight(false, null, []); - return false; - } + if (!TryNormalizeGitHubRepository(requestedRepository, out var normalizedRepository, out var error)) + return IssueDuplicatePreflightLoadResult.Failure(error!); - return TryLoadFromGitHub(normalizedRepository, out preflight, out error); + return await TryLoadFromGitHubAsync(normalizedRepository, cancellationToken).ConfigureAwait(false); } - public List FindMatches(string draftTitle, IReadOnlyList draftLabels) + public List FindMatches( + string draftTitle, + IReadOnlyList draftLabels, + IReadOnlyList? draftEvidencePaths = null, + string? draftBody = null) { if (!Checked || _issues.Count == 0) return []; @@ -137,6 +152,8 @@ public List FindMatches(string dra var draftLabelSet = draftLabels.ToHashSet(StringComparer.OrdinalIgnoreCase); var normalizedDraftTitle = NormalizeTitleText(draftTitle); var draftTokens = TokenizeTitle(draftTitle); + var draftEvidencePathSet = NormalizeEvidencePaths(draftEvidencePaths); + var draftBodyTokens = TokenizeBody(draftBody); var matches = new List(); foreach (var issue in _issues) { @@ -151,17 +168,21 @@ public List FindMatches(string dra var normalizedIssueTitle = NormalizeTitleText(issue.Title); var score = 0.0; string? reason = null; + var signals = new List(); if (normalizedIssueTitle.Length > 0 && normalizedIssueTitle == normalizedDraftTitle) { reason = "title_exact"; score = 1.0; + signals.Add("title_exact"); } else if (overlappingLabels.Count > 0) { score = ScoreTitleSimilarity(draftTokens, TokenizeTitle(issue.Title)); - if (score >= 0.45) + if (score >= TitleLabelSimilarityThreshold) { reason = "title_label_similarity"; + signals.Add("title_similarity"); + signals.Add("label_overlap"); } else if (normalizedIssueTitle.Length > 16 && normalizedDraftTitle.Length > 16 @@ -169,13 +190,37 @@ public List FindMatches(string dra || normalizedDraftTitle.Contains(normalizedIssueTitle, StringComparison.Ordinal))) { reason = "title_label_contains"; - score = Math.Max(score, 0.45); + score = Math.Max(score, TitleLabelSimilarityThreshold); + signals.Add("title_contains"); + signals.Add("label_overlap"); + } + + var evidenceScore = ScoreEvidencePathSimilarity( + draftEvidencePathSet, + ExtractEvidencePathsFromBody(issue.Body)); + if (evidenceScore >= EvidencePathSimilarityThreshold) + { + reason ??= "evidence_path_overlap"; + score = Math.Max(score, 0.65 + Math.Min(0.2, evidenceScore * 0.2)); + signals.Add("evidence_path_overlap"); + signals.Add("label_overlap"); + } + + var bodyScore = ScoreTitleSimilarity(draftBodyTokens, TokenizeBody(issue.Body)); + if (bodyScore >= BodyLabelSimilarityThreshold) + { + reason ??= "body_label_similarity"; + score = Math.Max(score, 0.5 + Math.Min(0.25, bodyScore * 0.25)); + signals.Add("body_similarity"); + signals.Add("label_overlap"); } } if (reason == null) continue; + var roundedScore = Math.Round(score, 3); + matches.Add(new SuggestionIssueDraftDuplicateMatchJsonResult( issue.Number, issue.Title, @@ -183,7 +228,9 @@ public List FindMatches(string dra issueLabels, overlappingLabels, reason, - Math.Round(score, 3))); + roundedScore, + ClassifyConfidence(roundedScore), + signals.Distinct(StringComparer.OrdinalIgnoreCase).ToList())); } return matches @@ -218,7 +265,8 @@ private static List ParseOpenIssues(JsonNode? root, bool skipPullRequ TryReadInt(item?["number"]), title, TryReadString(item?["html_url"], MaxOpenIssueUrlLength) ?? TryReadString(item?["url"], MaxOpenIssueUrlLength), - ReadLabels(item?["labels"]))); + ReadLabels(item?["labels"]), + TryReadString(item?["body"], MaxOpenIssueBodyLength))); } return issues; @@ -246,51 +294,99 @@ private static List ReadLabels(JsonNode? labelsNode) return result.Distinct(StringComparer.OrdinalIgnoreCase).ToList(); } - private static bool TryLoadFromGitHub(string repository, out IssueDuplicatePreflight preflight, out string? error) + private static async Task TryLoadFromGitHubAsync( + string repository, + CancellationToken cancellationToken) { var issues = new List(); try { for (var page = 1; page <= MaxGitHubOpenIssuePages && issues.Count < MaxOpenIssueCount; page++) { - var pageIssues = FetchGitHubOpenIssuePage(repository, page, out var rawEntryCount); - issues.AddRange(pageIssues); - if (rawEntryCount == 0 || rawEntryCount < GitHubOpenIssuesPerPage) + var pageResult = await FetchGitHubOpenIssuePageAsync(repository, page, cancellationToken) + .ConfigureAwait(false); + issues.AddRange(pageResult.Issues); + if (pageResult.RawEntryCount == 0 || pageResult.RawEntryCount < GitHubOpenIssuesPerPage) break; } - preflight = new IssueDuplicatePreflight(true, $"{GitHubSourcePrefix}{repository}", issues.Take(MaxOpenIssueCount).ToList()); - error = null; - return true; + return IssueDuplicatePreflightLoadResult.Success( + new IssueDuplicatePreflight( + true, + $"{GitHubSourcePrefix}{repository}", + issues.Take(MaxOpenIssueCount).ToList())); } - catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException or JsonException or IOException or InvalidOperationException or InvalidOpenIssuesFileException) + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - preflight = new IssueDuplicatePreflight(false, null, []); - error = $"could not fetch --open-issues github for repository '{repository}': {ex.Message}"; - return false; + throw; + } + catch (Exception ex) when (IsRecoverableGitHubPreflightException(ex)) + { + return IssueDuplicatePreflightLoadResult.Failure( + $"could not fetch --open-issues github for repository '{repository}': {FormatPreflightFailureDetail(ex)}"); } } - private static List FetchGitHubOpenIssuePage(string repository, int page, out int rawEntryCount) + private static async Task FetchGitHubOpenIssuePageAsync( + string repository, + int page, + CancellationToken cancellationToken) { var slash = repository.IndexOf('/'); var owner = repository[..slash]; var name = repository[(slash + 1)..]; var url = $"{GitHubApiBase}/repos/{Uri.EscapeDataString(owner)}/{Uri.EscapeDataString(name)}/issues?state=open&per_page={GitHubOpenIssuesPerPage.ToString(CultureInfo.InvariantCulture)}&page={page.ToString(CultureInfo.InvariantCulture)}"; + using var timeoutCts = new CancellationTokenSource(GitHubIssueReporter.ResolveSubmitTimeout()); + using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCts.Token); using var request = new HttpRequestMessage(HttpMethod.Get, url); + GitHubHttpClientFactory.ApplyDefaultHeaders(request.Headers); var token = Environment.GetEnvironmentVariable(GitHubTokenEnvironmentVariable); if (!string.IsNullOrWhiteSpace(token)) request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); - using var response = HttpClient.Send(request, HttpCompletionOption.ResponseHeadersRead); - if (!response.IsSuccessStatusCode) - throw new HttpRequestException($"GitHub API responded {(int)response.StatusCode} {response.ReasonPhrase}"); + HttpResponseMessage response; + try + { + response = await HttpClient.SendAsync( + request, + HttpCompletionOption.ResponseHeadersRead, + linkedCts.Token).ConfigureAwait(false); + } + catch (OperationCanceledException ex) when (!cancellationToken.IsCancellationRequested && timeoutCts.IsCancellationRequested) + { + throw new TimeoutException( + $"GitHub open-issues preflight timed out after {GitHubIssueReporter.ResolveSubmitTimeout().TotalSeconds:0} seconds.", + ex); + } - var json = ReadContentWithinLimit(response.Content, MaxOpenIssuesJsonBytes) - ?? throw new IOException($"GitHub open-issues response exceeds maximum supported size of {MaxOpenIssuesJsonBytes} bytes."); - var root = JsonNode.Parse(json, documentOptions: new JsonDocumentOptions { MaxDepth = MaxOpenIssuesJsonDepth }); - rawEntryCount = root is JsonArray array ? array.Count : 0; - return ParseOpenIssues(root, skipPullRequests: true); + using (response) + { + if (!response.IsSuccessStatusCode) + throw new GitHubPreflightException( + await BuildGitHubApiErrorDetailAsync(response, linkedCts.Token).ConfigureAwait(false)); + + var json = await ReadContentWithinLimitAsync(response.Content, MaxOpenIssuesJsonBytes, linkedCts.Token) + .ConfigureAwait(false) + ?? throw new IOException($"GitHub open-issues response exceeds maximum supported size of {MaxOpenIssuesJsonBytes} bytes."); + var root = JsonNode.Parse(json, documentOptions: new JsonDocumentOptions { MaxDepth = MaxOpenIssuesJsonDepth }); + var rawEntryCount = root is JsonArray array ? array.Count : 0; + return new GitHubOpenIssuePageResult(ParseOpenIssues(root, skipPullRequests: true), rawEntryCount); + } + } + + private static async Task BuildGitHubApiErrorDetailAsync( + HttpResponseMessage response, + CancellationToken cancellationToken) + { + await using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + var errorBody = await GitHubIssueReporter.ReadBoundedApiErrorBodyAsync(stream, cancellationToken) + .ConfigureAwait(false); + var retryAt = GitHubIssueReporter.GetRateLimitRetryAt( + response, + GitHubIssueReporter.TimeProvider.GetUtcNow().UtcDateTime); + return retryAt is null + ? GitHubIssueReporter.BuildApiErrorDetail((int)response.StatusCode, errorBody) + : GitHubIssueReporter.BuildRateLimitErrorDetail((int)response.StatusCode, errorBody, retryAt.Value); } private static string? ExtractGitHubRepository(string? source, string? repository) @@ -342,15 +438,19 @@ private static bool TryNormalizeGitHubRepository(string? repository, out string private static bool IsValidGitHubRepositoryPart(string value) => value.Length > 0 && value.All(c => char.IsLetterOrDigit(c) || c is '.' or '_' or '-'); - private static string? ReadContentWithinLimit(HttpContent content, int maxBytes) + private static async Task ReadContentWithinLimitAsync( + HttpContent content, + int maxBytes, + CancellationToken cancellationToken) { - using var stream = content.ReadAsStream(); + await using var stream = await content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); using var buffer = new MemoryStream(Math.Min(maxBytes, 8192)); var chunk = new byte[8192]; var total = 0; while (true) { - var read = stream.Read(chunk, 0, chunk.Length); + var read = await stream.ReadAsync(chunk.AsMemory(0, chunk.Length), cancellationToken) + .ConfigureAwait(false); if (read == 0) break; total += read; @@ -362,6 +462,21 @@ private static bool IsValidGitHubRepositoryPart(string value) return Encoding.UTF8.GetString(buffer.ToArray()); } + private static bool IsRecoverableGitHubPreflightException(Exception ex) + => ex is HttpRequestException + or OperationCanceledException + or TimeoutException + or JsonException + or IOException + or InvalidOperationException + or InvalidOpenIssuesFileException + or GitHubPreflightException; + + private static string FormatPreflightFailureDetail(Exception ex) + => ex is GitHubPreflightException githubPreflight + ? githubPreflight.Message + : CommandErrorWriter.FormatSanitizedException(ex); + private static HttpClient CreateDefaultHttpClient() => GitHubHttpClientFactory.CreateDefaultHttpClient(TimeSpan.FromSeconds(10)); @@ -429,10 +544,25 @@ private static string NormalizeTitleText(string title) private static HashSet TokenizeTitle(string title) { - title = BoundTitleProcessingInput(title); + return TokenizeWords(BoundTitleProcessingInput(title)); + } + + private static HashSet TokenizeBody(string? body) + { + if (string.IsNullOrWhiteSpace(body)) + return []; + + var bounded = body.Length <= MaxBodyTokenizationInputLength + ? body + : body[..MaxBodyTokenizationInputLength]; + return TokenizeWords(bounded); + } + + private static HashSet TokenizeWords(string text) + { var tokens = new HashSet(StringComparer.OrdinalIgnoreCase); var current = new StringBuilder(); - foreach (var c in title) + foreach (var c in text) { if (char.IsLetterOrDigit(c)) { @@ -447,6 +577,59 @@ private static HashSet TokenizeTitle(string title) return tokens; } + private static HashSet NormalizeEvidencePaths(IReadOnlyList? paths) + { + if (paths is null || paths.Count == 0) + return []; + + return paths + .Select(NormalizeEvidencePath) + .Where(path => path is not null) + .Select(path => path!) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + } + + private static HashSet ExtractEvidencePathsFromBody(string? body) + { + if (string.IsNullOrWhiteSpace(body)) + return []; + + var bounded = body.Length <= MaxOpenIssueBodyLength + ? body + : body[..MaxOpenIssueBodyLength]; + var paths = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var line in bounded.Split('\n', StringSplitOptions.RemoveEmptyEntries)) + { + var candidate = line.Trim(); + if (candidate.StartsWith("- ", StringComparison.Ordinal)) + candidate = candidate[2..].Trim(); + var normalized = NormalizeEvidencePath(candidate); + if (normalized != null) + paths.Add(normalized); + } + + return paths; + } + + private static string? NormalizeEvidencePath(string? value) + { + if (string.IsNullOrWhiteSpace(value)) + return null; + + var trimmed = value.Trim().Trim('`', '*'); + if (trimmed.Length > 512 + || trimmed.Any(char.IsControl) + || trimmed.StartsWith("http://", StringComparison.OrdinalIgnoreCase) + || trimmed.StartsWith("https://", StringComparison.OrdinalIgnoreCase) + || (!trimmed.Contains('/', StringComparison.Ordinal) && !trimmed.Contains('\\', StringComparison.Ordinal)) + || !trimmed.Contains('.', StringComparison.Ordinal)) + { + return null; + } + + return trimmed.Replace('\\', '/'); + } + private static string BoundTitleProcessingInput(string title) => title.Length <= MaxTitleTokenizationInputLength ? title @@ -473,7 +656,38 @@ private static double ScoreTitleSimilarity(HashSet left, HashSet return union == 0 ? 0.0 : intersection / (double)union; } - private sealed record OpenIssue(int? Number, string Title, string? Url, List Labels); + private static double ScoreEvidencePathSimilarity(HashSet draftPaths, HashSet issuePaths) + { + if (draftPaths.Count == 0 || issuePaths.Count == 0) + return 0.0; + + var overlap = draftPaths.Count(issuePaths.Contains); + return overlap / (double)Math.Max(draftPaths.Count, issuePaths.Count); + } + + private static string ClassifyConfidence(double score) + => score >= 0.75 + ? "high" + : score >= 0.5 + ? "medium" + : "low"; + + internal sealed record IssueDuplicatePreflightLoadResult( + bool Loaded, + IssueDuplicatePreflight Preflight, + string? Error) + { + internal static IssueDuplicatePreflightLoadResult Success(IssueDuplicatePreflight preflight) => new(true, preflight, null); + + internal static IssueDuplicatePreflightLoadResult Failure(string error) => + new(false, new IssueDuplicatePreflight(false, null, []), error); + } + + private sealed record GitHubOpenIssuePageResult(List Issues, int RawEntryCount); + + private sealed record OpenIssue(int? Number, string Title, string? Url, List Labels, string? Body); private sealed class InvalidOpenIssuesFileException(string message) : Exception(message); + + private sealed class GitHubPreflightException(string message) : Exception(message); } diff --git a/src/CodeIndex/Cli/ProgramRunner.Dispatch.cs b/src/CodeIndex/Cli/ProgramRunner.Dispatch.cs index df90f7a5f1..3bde810d23 100644 --- a/src/CodeIndex/Cli/ProgramRunner.Dispatch.cs +++ b/src/CodeIndex/Cli/ProgramRunner.Dispatch.cs @@ -214,7 +214,7 @@ private static int RunDispatchedCommand( private static Func? ResolveQueryRunner(string commandName, CommandRunContext context) => commandName switch { - "search" => a => QueryCommandRunner.RunSearch(a, context.JsonOptions), + "search" => a => QueryCommandRunner.RunSearch(a, context.JsonOptions, context.CancellationToken), "definition" => a => QueryCommandRunner.RunDefinition(a, context.JsonOptions), "goto" => a => QueryCommandRunner.RunGoto(a, context.JsonOptions), "references" => a => QueryCommandRunner.RunReferences(a, context.JsonOptions), @@ -235,7 +235,7 @@ private static int RunDispatchedCommand( "unused" => a => QueryCommandRunner.RunUnused(a, context.JsonOptions), "hotspots" => a => QueryCommandRunner.RunHotspots(a, context.JsonOptions), "batch" => a => QueryCommandRunner.RunBatch(a, context.JsonOptions), - "suggestions" => a => SuggestionsCommandRunner.Run(a, context.JsonOptions), + "suggestions" => a => SuggestionsCommandRunner.Run(a, context.JsonOptions, context.CancellationToken), _ => null, }; diff --git a/src/CodeIndex/Cli/QueryCommandRunner.cs b/src/CodeIndex/Cli/QueryCommandRunner.cs index 0d5cd98fbd..a8230059f1 100644 --- a/src/CodeIndex/Cli/QueryCommandRunner.cs +++ b/src/CodeIndex/Cli/QueryCommandRunner.cs @@ -364,7 +364,10 @@ private sealed record StatusReadinessField( StringComparer.Ordinal); private const string FindUsage = "Usage: cdidx find (--path |--all) [--db ] [--json] [--format ] [--verbose] [--limit |--top ] [--lang ] [--exclude-path ] [--exclude-tests] [--before ] [--after ] [--snippet-lines ] [--focus-line ] [--focus-column ] [--max-line-width ] [--exact] [--regex] [--count]\n cdidx find --query (--path |--all) [...]\n cdidx find [options] -- "; - public static int RunSearch(string[] cmdArgs, JsonSerializerOptions jsonOptions) + public static int RunSearch( + string[] cmdArgs, + JsonSerializerOptions jsonOptions, + CancellationToken cancellationToken = default) { var previewOptionError = ValidatePreviewOptions("search", cmdArgs, allowMaxLineWidth: true, allowFocusOptions: false); if (previewOptionError != null) @@ -621,7 +624,7 @@ public static int RunSearch(string[] cmdArgs, JsonSerializerOptions jsonOptions) } if (options.OutputFormat == OutputFormatIssueDrafts) - return RunSearchRecipeIssueDrafts(options, jsonOptions, exact); + return RunSearchRecipeIssueDrafts(options, jsonOptions, exact, cancellationToken); return RunSearchRecipe(options, jsonOptions, exact); } @@ -745,7 +748,7 @@ public static int RunSearch(string[] cmdArgs, JsonSerializerOptions jsonOptions) return CommandExitCodes.UsageError; } if (options.OutputFormat == OutputFormatIssueDrafts) - return RunSearchIssueDrafts(options, jsonOptions, exact); + return RunSearchIssueDrafts(options, jsonOptions, exact, cancellationToken); var exactSubstringHint = SearchQueryAdvisor.BuildExactSubstringHint(options.Query, options.RawFts, exact, options.Prefix); var ndjsonOptions = options.JsonOutputFormat == JsonOutputFormatNdjson ? GetCompactJsonOptions(jsonOptions) : jsonOptions; @@ -1613,7 +1616,11 @@ private static int RunSearchRecipe(QueryCommandOptions options, JsonSerializerOp }); } - private static int RunSearchRecipeIssueDrafts(QueryCommandOptions options, JsonSerializerOptions jsonOptions, bool userExact) + private static int RunSearchRecipeIssueDrafts( + QueryCommandOptions options, + JsonSerializerOptions jsonOptions, + bool userExact, + CancellationToken cancellationToken) { if (!TryResolveSearchRecipeSelection(options, out var selection, out var selectionError)) { @@ -1625,14 +1632,21 @@ private static int RunSearchRecipeIssueDrafts(QueryCommandOptions options, JsonS } var recipe = selection.Recipe; var scope = BuildSearchRecipeScope(recipe, options); - if (!IssueDuplicatePreflight.TryLoad(options.OpenIssuesPath, options.OpenIssuesRepository, out var preflight, out var error)) + var preflightResult = IssueDuplicatePreflight.TryLoadAsync( + options.OpenIssuesPath, + options.OpenIssuesRepository, + cancellationToken) + .GetAwaiter() + .GetResult(); + if (!preflightResult.Loaded) { WriteUsageError( - error!, + preflightResult.Error!, GetUsageLineOrThrow("search"), "Pass a readable JSON array from `gh issue list --state open --json number,title,labels,url`, or use `--open-issues github --repo owner/name`."); return CommandExitCodes.UsageError; } + var preflight = preflightResult.Preflight; return WithDb(options, jsonOptions, reader => { @@ -1659,16 +1673,27 @@ private static int RunSearchRecipeIssueDrafts(QueryCommandOptions options, JsonS }); } - private static int RunSearchIssueDrafts(QueryCommandOptions options, JsonSerializerOptions jsonOptions, bool exact) + private static int RunSearchIssueDrafts( + QueryCommandOptions options, + JsonSerializerOptions jsonOptions, + bool exact, + CancellationToken cancellationToken) { - if (!IssueDuplicatePreflight.TryLoad(options.OpenIssuesPath, options.OpenIssuesRepository, out var preflight, out var error)) + var preflightResult = IssueDuplicatePreflight.TryLoadAsync( + options.OpenIssuesPath, + options.OpenIssuesRepository, + cancellationToken) + .GetAwaiter() + .GetResult(); + if (!preflightResult.Loaded) { WriteUsageError( - error!, + preflightResult.Error!, GetUsageLineOrThrow("search"), "Pass a readable JSON array from `gh issue list --state open --json number,title,labels,url`, or use `--open-issues github --repo owner/name`."); return CommandExitCodes.UsageError; } + var preflight = preflightResult.Preflight; return WithDb(options, jsonOptions, reader => { @@ -1987,13 +2012,14 @@ private static SearchIssueDraftJsonResult ToSearchIssueDraft( .Distinct(StringComparer.Ordinal) .Take(10) .ToList(); - var duplicateMatches = preflight.FindMatches(title, labels); + var body = BuildSearchIssueDraftBody(recipe, queryResult, evidencePaths, options); + var duplicateMatches = preflight.FindMatches(title, labels, evidencePaths, body); return new SearchIssueDraftJsonResult( $"{recipe.Name}/{queryResult.Name}", title, labels, evidencePaths, - BuildSearchIssueDraftBody(recipe, queryResult, evidencePaths, options), + body, new SearchIssueDraftSourceJsonResult( recipe.Name, queryResult.Name, @@ -2021,13 +2047,14 @@ private static SearchIssueDraftJsonResult ToAdHocSearchIssueDraft( .Distinct(StringComparer.Ordinal) .Take(10) .ToList(); - var duplicateMatches = preflight.FindMatches(title, labels); + var body = BuildAdHocSearchIssueDraftBody(queryResult, evidencePaths); + var duplicateMatches = preflight.FindMatches(title, labels, evidencePaths, body); return new SearchIssueDraftJsonResult( "search/ad-hoc", title, labels, evidencePaths, - BuildAdHocSearchIssueDraftBody(queryResult, evidencePaths), + body, new SearchIssueDraftSourceJsonResult( null, null, diff --git a/src/CodeIndex/Cli/SuggestionsCommandRunner.cs b/src/CodeIndex/Cli/SuggestionsCommandRunner.cs index e20fff5a0d..dd33818c87 100644 --- a/src/CodeIndex/Cli/SuggestionsCommandRunner.cs +++ b/src/CodeIndex/Cli/SuggestionsCommandRunner.cs @@ -16,7 +16,10 @@ internal static class SuggestionsCommandRunner internal const int MaxSuggestionIssueDraftBodyLength = 24 * 1024; private const string SuggestionOutputTruncationMarker = "\n[truncated]"; - public static int Run(string[] args, JsonSerializerOptions jsonOptions) + public static int Run( + string[] args, + JsonSerializerOptions jsonOptions, + CancellationToken cancellationToken = default) { if (args.Length == 0 || args[0] is "--help" or "-h") { @@ -52,7 +55,7 @@ public static int Run(string[] args, JsonSerializerOptions jsonOptions) { "list" => RunList(outputRecords, options, jsonOptions), "show" => RunShow(records, options, jsonOptions), - "export" => RunExport(outputRecords, options, jsonOptions), + "export" => RunExport(outputRecords, options, jsonOptions, cancellationToken), _ => WriteUsageError($"Unknown suggestions subcommand: {verb}") }; } @@ -145,7 +148,11 @@ private static int RunShow(List records, Options options, Json return CommandExitCodes.Success; } - private static int RunExport(List records, Options options, JsonSerializerOptions jsonOptions) + private static int RunExport( + List records, + Options options, + JsonSerializerOptions jsonOptions, + CancellationToken cancellationToken) { if (options.ExportFormat == "markdown") { @@ -153,7 +160,7 @@ private static int RunExport(List records, Options options, Js return CommandExitCodes.Success; } if (options.ExportFormat == "issue-drafts") - return RunIssueDraftExport(records, options, jsonOptions); + return RunIssueDraftExport(records, options, jsonOptions, cancellationToken); var payload = new SuggestionExportJsonResult( JsonOutputContract.ApiVersion, @@ -165,10 +172,21 @@ private static int RunExport(List records, Options options, Js return CommandExitCodes.Success; } - private static int RunIssueDraftExport(List records, Options options, JsonSerializerOptions jsonOptions) + private static int RunIssueDraftExport( + List records, + Options options, + JsonSerializerOptions jsonOptions, + CancellationToken cancellationToken) { - if (!IssueDuplicatePreflight.TryLoad(options.OpenIssuesPath, options.OpenIssuesRepository, out var preflight, out var error)) - return WriteUsageError(error!); + var preflightResult = IssueDuplicatePreflight.TryLoadAsync( + options.OpenIssuesPath, + options.OpenIssuesRepository, + cancellationToken) + .GetAwaiter() + .GetResult(); + if (!preflightResult.Loaded) + return WriteUsageError(preflightResult.Error!); + var preflight = preflightResult.Preflight; var drafts = records.Select(record => ToIssueDraft(record, preflight)).ToList(); var payload = new SuggestionIssueDraftExportJsonResult( @@ -352,14 +370,15 @@ private static SuggestionIssueDraftJsonResult ToIssueDraft(SuggestionRecord reco var title = BuildIssueDraftTitle(record); var labels = GitHubIssueReporter.BuildIssueLabels(record).ToList(); var evidencePaths = NormalizeEvidencePaths(record); - var duplicateMatches = preflight.FindMatches(title, labels); + var body = BuildIssueDraftBody(record, evidencePaths); + var duplicateMatches = preflight.FindMatches(title, labels, evidencePaths, body); return new SuggestionIssueDraftJsonResult( record.Hash, ShortId(record.Hash), title, labels, evidencePaths, - BuildIssueDraftBody(record, evidencePaths), + body, new SuggestionIssueDraftSourceJsonResult( record.Category, record.Language, @@ -872,4 +891,6 @@ internal sealed record SuggestionIssueDraftDuplicateMatchJsonResult( [property: JsonPropertyName("labels")] List Labels, [property: JsonPropertyName("overlapping_labels")] List OverlappingLabels, [property: JsonPropertyName("reason")] string Reason, - [property: JsonPropertyName("score")] double Score); + [property: JsonPropertyName("score")] double Score, + [property: JsonPropertyName("confidence")] string Confidence, + [property: JsonPropertyName("signals")] List Signals); diff --git a/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs b/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs index 13bf8282f6..7afe0e1a28 100644 --- a/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs +++ b/tests/CodeIndex.Tests/GitHubIssueReporterTests.cs @@ -912,8 +912,8 @@ public async Task TryCreateIssueDetailedAsync_CreateSuccessBodyOverLimit_Returns var result = await GitHubIssueReporter.TryCreateIssueDetailedAsync(record, "1.0.0-test"); Assert.Null(result.IssueUrl); - Assert.Contains("InvalidDataException", result.Error); - Assert.Contains("HTTP response body exceeded", result.Error); + Assert.Equal("InvalidDataException", result.Error); + Assert.DoesNotContain("HTTP response body exceeded", result.Error); } finally { @@ -951,7 +951,8 @@ public async Task TryCreateIssueDetailedAsync_SearchBodyOverLimit_FailsClosedWit Assert.Null(result.IssueUrl); Assert.Contains("existing-suggestion lookup failed during search", result.Error); - Assert.Contains("HTTP response body exceeded", result.Error); + Assert.Contains("InvalidDataException", result.Error); + Assert.DoesNotContain("HTTP response body exceeded", result.Error); Assert.Equal(1, handler.RequestCount); Assert.DoesNotContain(handler.Requests, r => r.Method == HttpMethod.Post); } @@ -1025,7 +1026,8 @@ public async Task TryCreateIssueDetailedAsync_LabelListJsonOverDepthLimit_FailsC Assert.Null(result.IssueUrl); Assert.Contains("existing-suggestion lookup failed during label list", result.Error); - Assert.Contains("maximum configured depth", result.Error); + Assert.Contains("JsonReaderException", result.Error); + Assert.DoesNotContain("maximum configured depth", result.Error); Assert.Equal(2, handler.RequestCount); Assert.DoesNotContain(handler.Requests, r => r.Method == HttpMethod.Post); } @@ -1171,7 +1173,7 @@ public async Task TryCreateIssueDetailedAsync_CreateSuccessJsonOverDepthLimit_Re Assert.Null(result.IssueUrl); Assert.Contains("Json", result.Error); - Assert.Contains("maximum configured depth", result.Error); + Assert.DoesNotContain("maximum configured depth", result.Error); } finally { @@ -1429,7 +1431,8 @@ public async Task TryCreateIssueDetailedAsync_TimeoutCancellation_ReturnsDiagnos var result = await GitHubIssueReporter.TryCreateIssueDetailedAsync(record, "1.0.0-test"); Assert.Null(result.IssueUrl); - Assert.Equal("TaskCanceledException: request timed out", result.Error); + Assert.Equal("TaskCanceledException", result.Error); + Assert.DoesNotContain("request timed out", result.Error); } finally { @@ -1527,6 +1530,91 @@ public async Task TryCreateIssueDetailedAsync_RateLimited_ReturnsRetryAfterDiagn } } + [Fact] + public async Task TryCreateIssueDetailedAsync_SearchRateLimitIncludesRetryMetadata_Issue3823() + { + _env.Set("CDIDX_GITHUB_TOKEN", "ghp_idempotency_test"); + + var handler = new RecordingHandler(); + var response = new HttpResponseMessage((HttpStatusCode)429) + { + Content = MakeJsonContent("""{ "message": "rate limited", "token": "secret-value" }"""), + }; + response.Headers.RetryAfter = new System.Net.Http.Headers.RetryConditionHeaderValue(TimeSpan.FromSeconds(30)); + handler.AddResponse(req => req.Method == HttpMethod.Get && req.RequestUri!.AbsolutePath == "/search/issues", response); + handler.AddResponse(req => req.Method == HttpMethod.Post && req.RequestUri!.AbsolutePath.Contains("/issues"), + new HttpResponseMessage(HttpStatusCode.Created) + { + Content = MakeJsonContent("""{ "html_url": "https://github.com/widthdom/CodeIndex/issues/9999" }"""), + }); + using var mockClient = new HttpClient(handler); + GitHubIssueReporter.s_httpClientOverride = mockClient; + var fixedNow = new DateTimeOffset(2026, 6, 20, 12, 0, 0, TimeSpan.Zero); + GitHubIssueReporter.TimeProvider = new ManualTimeProvider(fixedNow); + try + { + var record = MakeRecordWithKnownHash(); + var result = await GitHubIssueReporter.TryCreateIssueDetailedAsync(record, "1.0.0-test"); + + Assert.Null(result.IssueUrl); + Assert.Contains("existing-suggestion lookup failed during search", result.Error); + Assert.Contains("429", result.Error); + Assert.Contains("next_retry_at=", result.Error); + Assert.Contains(fixedNow.UtcDateTime.AddSeconds(30).ToString("O", CultureInfo.InvariantCulture), result.Error); + Assert.DoesNotContain("secret-value", result.Error); + Assert.Equal(1, handler.RequestCount); + Assert.DoesNotContain(handler.Requests, request => request.Method == HttpMethod.Post); + } + finally + { + GitHubIssueReporter.s_httpClientOverride = null; + GitHubIssueReporter.TimeProvider = TimeProvider.System; + } + } + + [Fact] + public void TryCreateIssueDetailedAsync_SubmissionExceptionIsSanitized_Issue3823() + { + _env.Set("CDIDX_GITHUB_TOKEN", "ghp_idempotency_test"); + + using var mockClient = new HttpClient(new ThrowingHandler(new HttpRequestException("secret host detail"))); + GitHubIssueReporter.s_httpClientOverride = mockClient; + var originalError = Console.Error; + using var capturedError = new StringWriter(CultureInfo.InvariantCulture); + try + { + SuggestionStore.SubmitAttemptResult result; + lock (TestConsoleLock.Gate) + { + Console.SetError(capturedError); + try + { +#pragma warning disable xUnit1031 + result = GitHubIssueReporter.TryCreateIssueDetailedAsync( + MakeRecordWithKnownHash(), + "1.0.0-test") + .GetAwaiter() + .GetResult(); +#pragma warning restore xUnit1031 + } + finally + { + Console.SetError(originalError); + } + } + + Assert.Null(result.IssueUrl); + Assert.Equal("HttpRequestException", result.Error); + Assert.Contains("HttpRequestException", capturedError.ToString()); + Assert.DoesNotContain("secret host detail", capturedError.ToString()); + } + finally + { + GitHubIssueReporter.s_httpClientOverride = null; + Console.SetError(originalError); + } + } + [Fact] public async Task FindExistingIssueByHashAsync_NonHexHash_ReturnsNullWithoutCallingApi() { diff --git a/tests/CodeIndex.Tests/IssueDuplicatePreflightTests.cs b/tests/CodeIndex.Tests/IssueDuplicatePreflightTests.cs index 96f138f8b2..67abefd21f 100644 --- a/tests/CodeIndex.Tests/IssueDuplicatePreflightTests.cs +++ b/tests/CodeIndex.Tests/IssueDuplicatePreflightTests.cs @@ -1,3 +1,4 @@ +using System.Net; using System.Text; using System.Text.Json; using CodeIndex.Cli; @@ -171,6 +172,103 @@ public void TryLoad_GitHubSourceRequiresRepository_Issue3449() Assert.Contains("--open-issues github requires --repo", error); } + [Fact] + public async Task TryLoadAsync_GitHubSourcePropagatesCallerCancellation_Issue3823() + { + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsAnyAsync( + () => IssueDuplicatePreflight.TryLoadAsync("github", "Widthdom/CodeIndex", cts.Token)); + } + + [Fact] + public async Task TryLoadAsync_GitHubRateLimitIncludesRetryMetadata_Issue3823() + { + var fixedNow = new DateTimeOffset(2026, 6, 20, 12, 0, 0, TimeSpan.Zero); + var previousTimeProvider = GitHubIssueReporter.TimeProvider; + GitHubIssueReporter.TimeProvider = new ManualTimeProvider(fixedNow); + IssueDuplicatePreflight.s_httpClientOverride = new HttpClient(new SingleResponseHandler(_ => + { + var response = new HttpResponseMessage((HttpStatusCode)429) + { + Content = new StringContent("""{"message":"rate limited","token":"secret-value"}""", Encoding.UTF8, "application/json"), + }; + response.Headers.RetryAfter = new System.Net.Http.Headers.RetryConditionHeaderValue(TimeSpan.FromSeconds(45)); + return response; + })); + try + { + var result = await IssueDuplicatePreflight.TryLoadAsync("github", "Widthdom/CodeIndex"); + + Assert.False(result.Loaded); + Assert.Contains("429", result.Error); + Assert.Contains("next_retry_at=", result.Error); + Assert.Contains(fixedNow.UtcDateTime.AddSeconds(45).ToString("O", System.Globalization.CultureInfo.InvariantCulture), result.Error); + Assert.DoesNotContain("secret-value", result.Error); + } + finally + { + GitHubIssueReporter.TimeProvider = previousTimeProvider; + } + } + + [Fact] + public async Task TryLoadAsync_GitHubFetchExceptionIsSanitized_Issue3823() + { + IssueDuplicatePreflight.s_httpClientOverride = new HttpClient(new ThrowingOpenIssuesHandler( + new HttpRequestException("secret host detail"))); + + var result = await IssueDuplicatePreflight.TryLoadAsync("github", "Widthdom/CodeIndex"); + + Assert.False(result.Loaded); + Assert.Contains("HttpRequestException", result.Error); + Assert.DoesNotContain("secret host detail", result.Error); + } + + [Fact] + public async Task TryLoadAsync_GitHubInternalCancellationIsSanitized_Issue3823() + { + IssueDuplicatePreflight.s_httpClientOverride = new HttpClient(new ThrowingOpenIssuesHandler( + new OperationCanceledException("secret timeout detail"))); + + var result = await IssueDuplicatePreflight.TryLoadAsync("github", "Widthdom/CodeIndex"); + + Assert.False(result.Loaded); + Assert.Contains("OperationCanceledException", result.Error); + Assert.DoesNotContain("secret timeout detail", result.Error); + } + + [Fact] + public void FindMatches_UsesEvidenceAndBodySignals_Issue3823() + { + var path = WriteOpenIssuesJson( + """ + [ + { + "number": 3823, + "title": "Different issue title", + "labels": [{"name": "enhancement"}], + "url": "https://example.test/issues/3823", + "body": "## Evidence paths\n- src/CodeIndex/Cli/IssueDuplicatePreflight.cs\n\nretry diagnostics cancellation duplicate preflight github" + } + ] + """); + + var loaded = IssueDuplicatePreflight.TryLoad(path, out var preflight, out var error); + + Assert.True(loaded, error); + var match = Assert.Single(preflight.FindMatches( + "Unrelated title", + ["enhancement"], + ["src/CodeIndex/Cli/IssueDuplicatePreflight.cs"], + "retry diagnostics cancellation duplicate preflight github")); + Assert.Equal("evidence_path_overlap", match.Reason); + Assert.Equal("high", match.Confidence); + Assert.Contains("evidence_path_overlap", match.Signals); + Assert.Contains("body_similarity", match.Signals); + } + private string WriteOpenIssuesJson(string json) { var path = Path.Combine(_tempDir, "open-issues.json"); @@ -235,6 +333,18 @@ private HttpResponseMessage BuildResponse(HttpRequestMessage request) } } + private sealed class SingleResponseHandler(Func responseFactory) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromResult(responseFactory(request)); + } + + private sealed class ThrowingOpenIssuesHandler(Exception exception) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromException(exception); + } + private sealed record RecordedOpenIssuesRequest( string Uri, string? AuthorizationScheme, diff --git a/tests/CodeIndex.Tests/ProgramCliTests.cs b/tests/CodeIndex.Tests/ProgramCliTests.cs index 6e4c7a27b5..108ca57ca1 100644 --- a/tests/CodeIndex.Tests/ProgramCliTests.cs +++ b/tests/CodeIndex.Tests/ProgramCliTests.cs @@ -1200,7 +1200,8 @@ public void Suggestions_ExportIssueDraftsRejectsTooDeepOpenIssuesPreflight() Assert.Equal(CommandExitCodes.UsageError, exitCode); Assert.Equal(string.Empty, stdout); Assert.Contains("could not read --open-issues file", stderr); - Assert.Contains("maximum configured depth", stderr); + Assert.Contains("JsonReaderException", stderr); + Assert.DoesNotContain("maximum configured depth", stderr); } private static (int ExitCode, string StdOut, string StdErr) RunCliInSubprocess(string[] args, IReadOnlyDictionary? environment = null) From 27d13d1993b3c18c3dc8bae4b8d35a2006fcad90 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 20 Jun 2026 14:25:35 +0900 Subject: [PATCH 6/6] Harden upgrade installer diagnostics (#3831) --- changelog.d/unreleased/3831.fixed.md | 17 ++ src/CodeIndex/Cli/JsonOutputContracts.cs | 8 +- src/CodeIndex/Cli/ProgramRunner.cs | 172 +++++++++++++++++--- tests/CodeIndex.Tests/ProgramRunnerTests.cs | 159 ++++++++++++++++++ 4 files changed, 336 insertions(+), 20 deletions(-) create mode 100644 changelog.d/unreleased/3831.fixed.md diff --git a/changelog.d/unreleased/3831.fixed.md b/changelog.d/unreleased/3831.fixed.md new file mode 100644 index 0000000000..112feaf213 --- /dev/null +++ b/changelog.d/unreleased/3831.fixed.md @@ -0,0 +1,17 @@ +--- +category: fixed +issues: + - 3831 +affected: + - src/CodeIndex/Cli/JsonOutputContracts.cs + - src/CodeIndex/Cli/ProgramRunner.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Upgrade hardening and diagnostics were improved (#3831)** — explicit release tags are constrained to semver-shaped tags, JSON upgrade failures include bounded suppressed installer output tails, install-directory probe failures report sanitized diagnostics, and upgrade JSON now states the installer checksum trust boundary. + +## 日本語 + +- **upgrade hardening と diagnostics を改善しました (#3831)** — explicit release tag を semver-shaped tag に制限し、JSON upgrade failure に bounded suppressed installer output tail を含め、install-directory probe failure は sanitized diagnostic を報告し、upgrade JSON で installer checksum trust boundary を明示します。 diff --git a/src/CodeIndex/Cli/JsonOutputContracts.cs b/src/CodeIndex/Cli/JsonOutputContracts.cs index 6150f85441..4c458803b0 100644 --- a/src/CodeIndex/Cli/JsonOutputContracts.cs +++ b/src/CodeIndex/Cli/JsonOutputContracts.cs @@ -57,7 +57,13 @@ internal sealed record UpgradeJsonResult( [property: JsonPropertyName("handoff_command")] string? HandoffCommand, [property: JsonPropertyName("handoff_url")] string? HandoffUrl, [property: JsonPropertyName("handoff_asset")] string? HandoffAsset, - [property: JsonPropertyName("handoff_asset_url")] string? HandoffAssetUrl); + [property: JsonPropertyName("handoff_asset_url")] string? HandoffAssetUrl, + [property: JsonPropertyName("installer_verification")] string? InstallerVerification, + [property: JsonPropertyName("installer_trust_boundary")] string? InstallerTrustBoundary, + [property: JsonPropertyName("installer_stdout_tail")] string? InstallerStdoutTail, + [property: JsonPropertyName("installer_stderr_tail")] string? InstallerStderrTail, + [property: JsonPropertyName("installer_output_truncated")] bool? InstallerOutputTruncated, + [property: JsonPropertyName("install_directory_error")] string? InstallDirectoryError); internal sealed record DbIntegrityCheckJsonResult( [property: JsonPropertyName("db_path")] string DbPath, diff --git a/src/CodeIndex/Cli/ProgramRunner.cs b/src/CodeIndex/Cli/ProgramRunner.cs index 47e0c81d1b..2bfc3599ec 100644 --- a/src/CodeIndex/Cli/ProgramRunner.cs +++ b/src/CodeIndex/Cli/ProgramRunner.cs @@ -30,6 +30,9 @@ internal static partial class ProgramRunner private const long MaxInstallerScriptBytes = 1024 * 1024; internal const long MaxReleaseChecksumBytes = 256 * 1024; private const int InstallerSuppressedOutputDrainBufferChars = 4096; + internal const int InstallerSuppressedOutputTailChars = 4096; + private const string UpgradeInstallerVerification = "same_release_sha256_manifest"; + private const string UpgradeInstallerTrustBoundary = "install.sh is verified against sha256sums.txt from the same GitHub release asset namespace; no independent signature is currently available."; internal const int WorkspaceVersionPinMaxBytes = 4096; internal const int WorkspaceVersionPinMaxSkippedBlankLines = 16; internal const int WorkspaceVersionPinMaxLineChars = 256; @@ -3027,7 +3030,7 @@ internal static int RunUpgrade( } var installDir = AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - if (!CanWriteDirectory(installDir)) + if (!TryCheckInstallDirectoryWritable(installDir, out var installDirectoryError)) { if (wantsJson) { @@ -3039,12 +3042,15 @@ internal static int RunUpgrade( includePrerelease, installAttempted: false, installExitCode: null, - error: "install_directory_not_writable"), + error: "install_directory_not_writable", + installDirectoryError: installDirectoryError), jsonOptions)); } else { Console.Error.WriteLine($"Error: install directory is not writable: {installDir}"); + if (installDirectoryError != null) + Console.Error.WriteLine($"Reason: {installDirectoryError}"); Console.Error.WriteLine("Hint: rerun with permissions that can write this directory, or reinstall cdidx into a per-user directory."); } return CommandExitCodes.UsageError; @@ -3079,11 +3085,12 @@ internal static int RunUpgrade( } var startInfo = CreateInstallerProcessStartInfo(scriptPath, selectedReleaseTag, installDir); - var installExitCode = RunInstallerProcess( + var installerResult = RunInstallerProcessDetailed( startInfo, InstallerRunTimeout, cancellationToken, suppressOutput: wantsJson); + var installExitCode = installerResult.ExitCode; if (wantsJson) { var error = installExitCode == CommandExitCodes.Success @@ -3097,7 +3104,8 @@ internal static int RunUpgrade( includePrerelease, installAttempted: true, installExitCode: installExitCode, - error: error), + error: error, + installerResult: installerResult), jsonOptions)); } return installExitCode; @@ -3186,9 +3194,39 @@ private static bool TryNormalizeReleaseTag(string rawVersion, out string? normal normalizedVersion = trimmed[0] is 'v' or 'V' ? "v" + trimmed[1..] : "v" + trimmed; + if (!IsValidUpgradeReleaseTag(normalizedVersion)) + { + error = "--version must be a release tag shaped like vX.Y.Z or vX.Y.Z-prerelease."; + normalizedVersion = null; + return false; + } + return true; } + internal static bool IsValidUpgradeReleaseTag(string releaseTag) + { + if (string.IsNullOrWhiteSpace(releaseTag) || releaseTag[0] != 'v') + return false; + + var rest = releaseTag[1..]; + var prereleaseStart = rest.IndexOf('-'); + var core = prereleaseStart >= 0 ? rest[..prereleaseStart] : rest; + var prerelease = prereleaseStart >= 0 ? rest[(prereleaseStart + 1)..] : null; + var parts = core.Split('.'); + if (parts.Length != 3 || parts.Any(part => part.Length == 0 || !part.All(char.IsDigit))) + return false; + + if (prerelease == null) + return true; + + var identifiers = prerelease.Split('.'); + return identifiers.Length > 0 + && identifiers.All(identifier => + identifier.Length > 0 + && identifier.All(ch => char.IsAsciiLetterOrDigit(ch) || ch == '-')); + } + private static bool IsPrereleaseTag(string releaseTag) => releaseTag.Contains('-', StringComparison.Ordinal); @@ -3283,6 +3321,13 @@ internal static int RunInstallerProcess( TimeSpan timeout, CancellationToken cancellationToken = default, bool suppressOutput = false) + => RunInstallerProcessDetailed(startInfo, timeout, cancellationToken, suppressOutput).ExitCode; + + internal static InstallerProcessResult RunInstallerProcessDetailed( + ProcessStartInfo startInfo, + TimeSpan timeout, + CancellationToken cancellationToken = default, + bool suppressOutput = false) { if (suppressOutput) { @@ -3295,12 +3340,12 @@ internal static int RunInstallerProcess( { if (!suppressOutput) Console.Error.WriteLine("Error: failed to start install.sh for upgrade."); - return CommandExitCodes.InstallError; + return InstallerProcessResult.Failure(CommandExitCodes.InstallError); } var outputDrainTask = suppressOutput ? DrainSuppressedInstallerOutputAsync(process) - : Task.CompletedTask; + : Task.FromResult(SuppressedInstallerOutputResult.Empty); try { @@ -3312,8 +3357,12 @@ internal static int RunInstallerProcess( { timeoutCts.Cancel(); waitTask.GetAwaiter().GetResult(); - outputDrainTask.GetAwaiter().GetResult(); - return process.ExitCode; + var output = outputDrainTask.GetAwaiter().GetResult(); + return new InstallerProcessResult( + process.ExitCode, + output.StdoutTail, + output.StderrTail, + output.Truncated); } if (cancellationToken.IsCancellationRequested) @@ -3336,8 +3385,12 @@ internal static int RunInstallerProcess( if (process.HasExited) { - outputDrainTask.GetAwaiter().GetResult(); - return process.ExitCode; + var output = outputDrainTask.GetAwaiter().GetResult(); + return new InstallerProcessResult( + process.ExitCode, + output.StdoutTail, + output.StderrTail, + output.Truncated); } TryKillProcessTree(process); @@ -3354,19 +3407,87 @@ internal static int RunInstallerProcess( } if (!suppressOutput) Console.Error.WriteLine("Hint: rerun `install.sh` manually for the desired release."); - return CommandExitCodes.InstallError; + var timeoutOutput = outputDrainTask.IsCompletedSuccessfully + ? outputDrainTask.GetAwaiter().GetResult() + : SuppressedInstallerOutputResult.Empty; + return new InstallerProcessResult( + CommandExitCodes.InstallError, + timeoutOutput.StdoutTail, + timeoutOutput.StderrTail, + timeoutOutput.Truncated); } - private static Task DrainSuppressedInstallerOutputAsync(Process process) - => Task.WhenAll( + private static async Task DrainSuppressedInstallerOutputAsync(Process process) + { + var outputs = await Task.WhenAll( DrainSuppressedInstallerOutputAsync(process.StandardOutput), - DrainSuppressedInstallerOutputAsync(process.StandardError)); + DrainSuppressedInstallerOutputAsync(process.StandardError)).ConfigureAwait(false); + return new SuppressedInstallerOutputResult( + outputs[0].Tail, + outputs[1].Tail, + outputs[0].Truncated || outputs[1].Truncated); + } - private static async Task DrainSuppressedInstallerOutputAsync(TextReader reader) + private static async Task DrainSuppressedInstallerOutputAsync(TextReader reader) { var buffer = new char[InstallerSuppressedOutputDrainBufferChars]; - while (await reader.ReadAsync(buffer.AsMemory()).ConfigureAwait(false) > 0) + var tail = new SuppressedOutputTail(InstallerSuppressedOutputTailChars); + while (true) { + var read = await reader.ReadAsync(buffer.AsMemory()).ConfigureAwait(false); + if (read == 0) + break; + + tail.Append(buffer.AsSpan(0, read)); + } + + return new SuppressedInstallerOutput(tail.Value, tail.Truncated); + } + + internal sealed record InstallerProcessResult( + int ExitCode, + string? StdoutTail, + string? StderrTail, + bool OutputTruncated) + { + internal static InstallerProcessResult Failure(int exitCode) => new(exitCode, null, null, false); + } + + private sealed record SuppressedInstallerOutputResult( + string? StdoutTail, + string? StderrTail, + bool Truncated) + { + internal static SuppressedInstallerOutputResult Empty { get; } = new(null, null, false); + } + + private sealed record SuppressedInstallerOutput(string? Tail, bool Truncated); + + private sealed class SuppressedOutputTail(int maxChars) + { + private readonly StringBuilder _builder = new(maxChars); + private long _totalChars; + + internal bool Truncated { get; private set; } + + internal string? Value => _builder.Length == 0 ? null : _builder.ToString(); + + internal void Append(ReadOnlySpan value) + { + _totalChars += value.Length; + if (_totalChars > maxChars) + Truncated = true; + + if (value.Length >= maxChars) + { + _builder.Clear(); + _builder.Append(value[^maxChars..]); + return; + } + + _builder.Append(value); + if (_builder.Length > maxChars) + _builder.Remove(0, _builder.Length - maxChars); } } @@ -3396,7 +3517,9 @@ private static UpgradeJsonResult CreateUpgradeJsonResult( bool installAttempted, int? installExitCode, string? error, - UpgradeHandoff? handoff = null) + UpgradeHandoff? handoff = null, + InstallerProcessResult? installerResult = null, + string? installDirectoryError = null) => new( result.CurrentVersion, result.LatestVersion, @@ -3415,7 +3538,13 @@ private static UpgradeJsonResult CreateUpgradeJsonResult( handoff?.Command, handoff?.Url, handoff?.Asset, - handoff?.AssetUrl); + handoff?.AssetUrl, + result.LatestVersion is null ? null : UpgradeInstallerVerification, + result.LatestVersion is null ? null : UpgradeInstallerTrustBoundary, + installerResult is { ExitCode: not CommandExitCodes.Success } ? installerResult.StdoutTail : null, + installerResult is { ExitCode: not CommandExitCodes.Success } ? installerResult.StderrTail : null, + installerResult is { ExitCode: not CommandExitCodes.Success } ? installerResult.OutputTruncated : null, + installDirectoryError); internal static string BuildReleasePageUrl(string releaseTag) => string.Format( @@ -3528,7 +3657,11 @@ await BoundedHttpContentReader.WriteToPrivateFileAsync( } internal static bool CanWriteDirectory(string directory) + => TryCheckInstallDirectoryWritable(directory, out _); + + internal static bool TryCheckInstallDirectoryWritable(string directory, out string? diagnostic) { + diagnostic = null; string? probe = null; var createdProbe = false; try @@ -3539,8 +3672,9 @@ internal static bool CanWriteDirectory(string directory) createdProbe = true; return true; } - catch + catch (Exception ex) { + diagnostic = CommandErrorWriter.FormatSanitizedException(ex); return false; } finally diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index 0c80803a4d..27619d66f3 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -1520,6 +1520,53 @@ exit 0 } } + [Fact] + public void RunInstallerProcessDetailed_SuppressedFailureCapturesBoundedTail_Issue3831() + { + if (OperatingSystem.IsWindows()) + return; + + lock (TestConsoleLock.Gate) + { + var root = Path.Combine(Path.GetTempPath(), $"cdidx_installer_tail_{Guid.NewGuid():N}"); + Directory.CreateDirectory(root); + var script = Path.Combine(root, "install.sh"); + try + { + File.WriteAllText(script, """ +#!/bin/sh +i=0 +while [ "$i" -lt 700 ]; do + printf 'stdout-tail-%04d-abcdefghijklmnopqrstuvwxyz\n' "$i" + printf 'stderr-tail-%04d-abcdefghijklmnopqrstuvwxyz\n' "$i" >&2 + i=$((i + 1)) +done +exit 7 +"""); + File.SetUnixFileMode(script, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + var startInfo = ProgramRunner.CreateInstallerProcessStartInfo(script, "v1.27.0", root); + + var result = ProgramRunner.RunInstallerProcessDetailed( + startInfo, + TimeSpan.FromSeconds(10), + suppressOutput: true); + + Assert.Equal(7, result.ExitCode); + Assert.True(result.OutputTruncated); + Assert.True(result.StdoutTail!.Length <= ProgramRunner.InstallerSuppressedOutputTailChars); + Assert.True(result.StderrTail!.Length <= ProgramRunner.InstallerSuppressedOutputTailChars); + Assert.Contains("stdout-tail-0699", result.StdoutTail); + Assert.Contains("stderr-tail-0699", result.StderrTail); + Assert.DoesNotContain("stdout-tail-0000", result.StdoutTail); + Assert.DoesNotContain("stderr-tail-0000", result.StderrTail); + } + finally + { + TestProjectHelper.DeleteDirectory(root); + } + } + } + [Fact] public void RunUpgrade_JsonPreparationFailure_UsesInstallError_Issue3373() { @@ -1695,6 +1742,69 @@ exit 0 } } + [Fact] + public void RunUpgrade_JsonInstallerFailureIncludesSuppressedOutputTail_Issue3831() + { + if (OperatingSystem.IsWindows()) + return; + + lock (TestConsoleLock.Gate) + { + using var env = EnvironmentVariableScope.Capture("XDG_CACHE_HOME", UpdateChecker.DisableEnvVar); + var cacheRoot = Path.Combine(Path.GetTempPath(), $"cdidx_update_cache_{Guid.NewGuid():N}"); + env.Set("XDG_CACHE_HOME", cacheRoot); + env.Set(UpdateChecker.DisableEnvVar, null); + WriteFreshUpdateCheckCache(cacheRoot, "v9.9.9"); + + var installerScript = """ +#!/bin/sh +i=0 +while [ "$i" -lt 700 ]; do + printf 'json-stdout-%04d-abcdefghijklmnopqrstuvwxyz\n' "$i" + printf 'json-stderr-%04d-abcdefghijklmnopqrstuvwxyz\n' "$i" >&2 + i=$((i + 1)) +done +exit 7 +"""; + var installerSha256 = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(installerScript))).ToLowerInvariant(); + var checksumManifest = $"{installerSha256} install.sh\n"; + var previousFactory = ProgramRunner.UpgradeHttpClientFactory; + ProgramRunner.UpgradeHttpClientFactory = () => new HttpClient( + new UpgradeAssetResponseHandler( + checksumManifest, + installerScript, + _ => { })) + { + Timeout = Timeout.InfiniteTimeSpan, + }; + + try + { + var (exitCode, stdout, stderr) = CaptureConsole(() => ProgramRunner.Run( + ["upgrade", "--json"], + appVersion: "1.10.0")); + + Assert.Equal(7, exitCode); + Assert.Empty(stderr); + using var doc = JsonDocument.Parse(stdout); + var root = doc.RootElement; + Assert.True(root.GetProperty("install_attempted").GetBoolean()); + Assert.False(root.GetProperty("install_succeeded").GetBoolean()); + Assert.Equal(7, root.GetProperty("install_exit_code").GetInt32()); + Assert.Equal("installer_exit_code_7", root.GetProperty("error").GetString()); + Assert.True(root.GetProperty("installer_output_truncated").GetBoolean()); + Assert.Contains("json-stdout-0699", root.GetProperty("installer_stdout_tail").GetString(), StringComparison.Ordinal); + Assert.Contains("json-stderr-0699", root.GetProperty("installer_stderr_tail").GetString(), StringComparison.Ordinal); + Assert.DoesNotContain("json-stdout-0000", root.GetProperty("installer_stdout_tail").GetString(), StringComparison.Ordinal); + } + finally + { + ProgramRunner.UpgradeHttpClientFactory = previousFactory; + TestProjectHelper.DeleteDirectory(cacheRoot); + } + } + } + [Fact] public void RunUpgrade_InstallerScriptCleanupFailure_EmitsWarning_Issue3372() { @@ -1797,6 +1907,55 @@ public void RunUpgrade_CheckOnlyJsonExplicitVersion_ReportsSelection() Assert.Equal("explicit_version", root.GetProperty("selection_source").GetString()); Assert.True(root.GetProperty("include_prerelease").GetBoolean()); Assert.False(root.GetProperty("install_attempted").GetBoolean()); + Assert.Equal("same_release_sha256_manifest", root.GetProperty("installer_verification").GetString()); + Assert.Contains("same GitHub release asset namespace", root.GetProperty("installer_trust_boundary").GetString(), StringComparison.Ordinal); + } + } + + [Theory] + [InlineData("v1.2.3", true)] + [InlineData("1.2.3", false)] + [InlineData("v1.2.3-rc.1", true)] + [InlineData("v1.2", false)] + [InlineData("v1.2.3/evil", false)] + [InlineData("v1.2.3+build", false)] + public void IsValidUpgradeReleaseTag_ConstrainShape_Issue3831(string releaseTag, bool expected) + { + Assert.Equal(expected, ProgramRunner.IsValidUpgradeReleaseTag(releaseTag)); + } + + [Fact] + public void RunUpgrade_InvalidExplicitVersion_ReturnsUsageError_Issue3831() + { + lock (TestConsoleLock.Gate) + { + var (exitCode, stdout, stderr) = CaptureConsole(() => ProgramRunner.Run( + ["upgrade", "--check-only", "--version", "release/test"], + appVersion: "1.10.0")); + + Assert.Equal(CommandExitCodes.UsageError, exitCode); + Assert.Empty(stdout); + Assert.Contains("vX.Y.Z", stderr); + } + } + + [Fact] + public void TryCheckInstallDirectoryWritable_FilePathReportsDiagnostic_Issue3831() + { + var path = Path.Combine(Path.GetTempPath(), $"cdidx_install_dir_file_{Guid.NewGuid():N}"); + try + { + File.WriteAllText(path, ""); + + var writable = ProgramRunner.TryCheckInstallDirectoryWritable(path, out var diagnostic); + + Assert.False(writable); + Assert.Equal("IOException", diagnostic); + } + finally + { + if (File.Exists(path)) + File.Delete(path); } }