diff --git a/changelog.d/unreleased/1713.fixed.md b/changelog.d/unreleased/1713.fixed.md new file mode 100644 index 0000000000..5ba5efd07a --- /dev/null +++ b/changelog.d/unreleased/1713.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 1713 +affected: + - src/CodeIndex/Cli/GlobalToolLog.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Global tool logging now disposes the log writer when startup fails (#1713)** — if lifecycle-log startup fails after opening the writer, `cdidx` now closes that writer before falling back without persistent logging. + +## 日本語 + +- **global tool logging の起動失敗時に log writer を破棄するようになりました (#1713)** — lifecycle log の起動処理が writer を開いた後に失敗した場合、persistent logging なしで続行する前に writer を閉じるようになりました。 diff --git a/changelog.d/unreleased/1719.fixed.md b/changelog.d/unreleased/1719.fixed.md new file mode 100644 index 0000000000..a2c228d79c --- /dev/null +++ b/changelog.d/unreleased/1719.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 1719 +affected: + - src/CodeIndex/Cli/GlobalToolLog.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs +--- + +## English + +- **Global tool logging now detects development executions with canonicalized paths (#1719)** — build-output path checks now normalize separators and compare directory segments so mixed-separator paths do not fall through to persistent install logging. + +## 日本語 + +- **global tool logging が正規化した path で開発実行を判定するようになりました (#1719)** — build output path の確認で separator を正規化し、directory segment 単位で照合するため、separator が混在した path が persistent install logging に流れにくくなりました。 diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index 576a859d0c..048a415ea2 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -33,7 +33,22 @@ internal static class GlobalToolLog RegexOptions.CultureInvariant | RegexOptions.Compiled); internal static IDisposable? TryStart(string[] args, string appVersion) + => TryStart(args, appVersion, createWriter: null, afterWriterCreated: null); + + internal static IDisposable? TryStartForTesting( + string[] args, + string appVersion, + Func? createWriter = null, + Action? afterWriterCreated = null) + => TryStart(args, appVersion, createWriter, afterWriterCreated); + + private static IDisposable? TryStart( + string[] args, + string appVersion, + Func? createWriter, + Action? afterWriterCreated) { + StreamWriter? writer = null; try { if (!ShouldEnable()) @@ -44,14 +59,13 @@ internal static class GlobalToolLog HardenLogFiles(logDirectory); var options = LogOptions.FromEnvironment(); var logPath = ResolveLogPath(logDirectory, options); - var writer = new StreamWriter(new FileStream(logPath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite), new UTF8Encoding(false)) - { - AutoFlush = true, - }; + writer = createWriter?.Invoke(logPath) ?? CreateLogWriter(logPath); + afterWriterCreated?.Invoke(); SetLogFilePermissions(logPath); PruneOldLogs(logDirectory, options.RetainCount); var session = new Session(writer, logPath, options.Format); + writer = null; CurrentSession.Value = session; session.AttachErrorMirror(); session.Write("INFO", $"session_start pid={Environment.ProcessId} version={appVersion}"); @@ -63,11 +77,18 @@ internal static class GlobalToolLog } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + writer?.Dispose(); CurrentSession.Value = null; return null; } } + private static StreamWriter CreateLogWriter(string logPath) => + new(new FileStream(logPath, FileMode.Append, FileAccess.Write, FileShare.ReadWrite), new UTF8Encoding(false)) + { + AutoFlush = true, + }; + internal static void Info(string message) => CurrentSession.Value?.Write("INFO", message); internal static void Error(string message) => CurrentSession.Value?.Write("ERROR", message); @@ -166,14 +187,60 @@ internal static bool TryParseEnvBool(string? raw, out bool value) } } + internal static bool LooksLikeDevelopmentExecutionForTesting(string? path) => LooksLikeDevelopmentExecution(path); + private static bool LooksLikeDevelopmentExecution(string? path) { if (string.IsNullOrWhiteSpace(path)) return false; - var normalized = path.Replace('\\', '/'); - return normalized.Contains("/src/CodeIndex/bin/", StringComparison.OrdinalIgnoreCase) - || normalized.Contains("/tests/CodeIndex.Tests/bin/", StringComparison.OrdinalIgnoreCase); + var normalized = NormalizePathForDevelopmentDetection(path); + var comparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + return ContainsPathSegments(normalized, ["src", "CodeIndex", "bin"], comparison) + || ContainsPathSegments(normalized, ["tests", "CodeIndex.Tests", "bin"], comparison); + } + + private static string NormalizePathForDevelopmentDetection(string path) + { + try + { + path = Path.GetFullPath(path); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + } + + return path + .Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar) + .Replace('\\', Path.DirectorySeparatorChar) + .Replace('/', Path.DirectorySeparatorChar); + } + + private static bool ContainsPathSegments(string path, string[] expectedSegments, StringComparison comparison) + { + var segments = path.Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries); + if (segments.Length < expectedSegments.Length) + return false; + + for (var start = 0; start <= segments.Length - expectedSegments.Length; start++) + { + var matched = true; + for (var offset = 0; offset < expectedSegments.Length; offset++) + { + if (!string.Equals(segments[start + offset], expectedSegments[offset], comparison)) + { + matched = false; + break; + } + } + + if (matched) + return true; + } + + return false; } /// diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index 7b3756c49f..ce87b1e7f2 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -322,6 +322,56 @@ public void GlobalToolLog_XdgDirectory_HonorsDocumentedPrecedence() } } + [Fact] + public void GlobalToolLog_TryStart_DisposesWriterWhenStartupAfterWriterCreationFails() + { + using var env = EnvironmentVariableScope.Capture( + "CDIDX_FORCE_GLOBAL_TOOL_LOG", + "CDIDX_DISABLE_PERSISTENT_LOG", + "CDIDX_GLOBAL_TOOL_LOG_DIR"); + var logDir = Path.Combine(Path.GetTempPath(), $"cdidx_global_tool_log_fault_{Guid.NewGuid():N}"); + Directory.CreateDirectory(logDir); + var writer = new TrackingStreamWriter(); + + try + { + env.Set("CDIDX_FORCE_GLOBAL_TOOL_LOG", "1"); + env.Set("CDIDX_DISABLE_PERSISTENT_LOG", null); + env.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", logDir); + + var session = GlobalToolLog.TryStartForTesting( + ["status"], + "1.10.0", + _ => writer, + () => throw new UnauthorizedAccessException("prune failed")); + + Assert.Null(session); + Assert.True(writer.WasDisposed); + } + finally + { + TestProjectHelper.DeleteDirectory(logDir); + } + } + + [Theory] + [InlineData("/repo/src/CodeIndex/bin/Debug/net8.0/")] + [InlineData("/repo/src/CodeIndex/bin/Debug/net8.0/cdidx.dll")] + [InlineData("/repo/tests/CodeIndex.Tests/bin/Debug/net8.0/CodeIndex.Tests.dll")] + [InlineData(@"C:\repo\src\CodeIndex\bin\Debug\net8.0\cdidx.exe")] + [InlineData(@"C:/repo/src\CodeIndex/bin\Debug/net8.0/cdidx.exe")] + public void GlobalToolLog_DevelopmentExecutionDetection_RecognizesCanonicalAndMixedSeparators(string path) + { + Assert.True(GlobalToolLog.LooksLikeDevelopmentExecutionForTesting(path)); + } + + [Fact] + public void GlobalToolLog_DevelopmentExecutionDetection_DoesNotMatchPartialDirectoryNames() + { + Assert.False(GlobalToolLog.LooksLikeDevelopmentExecutionForTesting("/repo/not-src/CodeIndex/bin/Debug/net8.0/")); + Assert.False(GlobalToolLog.LooksLikeDevelopmentExecutionForTesting("/repo/src/CodeIndex.Binary/bin/Debug/net8.0/")); + } + [Fact] public void Run_ForcedGlobalToolLogging_WritesLifecycleAndMirrorsStderr() { @@ -1197,6 +1247,22 @@ private sealed class ThrowingResolver : IJsonTypeInfoResolver throw new InvalidOperationException(JsonOutputFailure.ReflectionDisabledMessage); } + private sealed class TrackingStreamWriter : StreamWriter + { + public TrackingStreamWriter() + : base(new MemoryStream()) + { + } + + public bool WasDisposed { get; private set; } + + protected override void Dispose(bool disposing) + { + WasDisposed = true; + base.Dispose(disposing); + } + } + // --- --audit-log flag parsing (#1562) --- [Fact]