Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace TelegramSearchBot.Service.Tools {
[Injectable(Microsoft.Extensions.DependencyInjection.ServiceLifetime.Singleton)]
public sealed class CodingAgentJobService : IService {
private static readonly TimeSpan StateTtl = TimeSpan.FromDays(7);
private const string EnqueueJobScript = @"
internal const string EnqueueJobScript = @"
local activeKey = KEYS[1]
local stateKey = KEYS[2]
local queueKey = KEYS[3]
Expand Down
1 change: 1 addition & 0 deletions TelegramSearchBot.LLM/TelegramSearchBot.LLM.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<ItemGroup>
<InternalsVisibleTo Include="TelegramSearchBot.LLM.Test" />
<InternalsVisibleTo Include="TelegramSearchBot.Test" />
</ItemGroup>

<ItemGroup>
Expand Down
160 changes: 160 additions & 0 deletions TelegramSearchBot.Test/AppBootstrap/GarnetLuaScriptIntegrationTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
using System.Diagnostics;
using System.Net;
using System.Net.Sockets;
using Garnet;
using Newtonsoft.Json;
using StackExchange.Redis;
using TelegramSearchBot.AppBootstrap;
using TelegramSearchBot.Model.AI;
using TelegramSearchBot.Service.AI.LLM;
using TelegramSearchBot.Service.Tools;
using Xunit;

namespace TelegramSearchBot.Test.AppBootstrap {
public sealed class GarnetLuaScriptIntegrationTests {
[Fact]
public async Task EmbeddedGarnet_RunsProjectLuaScripts() {
var port = GetAvailablePort();
using var server = new GarnetServer(SchedulerBootstrap.BuildGarnetArguments(port.ToString()));
server.Start();
Comment on lines +17 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid TOCTOU port reservation race in test server startup.

GetAvailablePort() releases the socket before GarnetServer binds, so another process can take that port and make this integration test flaky.

Suggested hardening
-            var port = GetAvailablePort();
-            using var server = new GarnetServer(SchedulerBootstrap.BuildGarnetArguments(port.ToString()));
-            server.Start();
+            GarnetServer? server = null;
+            var port = 0;
+            for (var attempt = 0; attempt < 5 && server == null; attempt++) {
+                port = GetAvailablePort();
+                try {
+                    server = new GarnetServer(SchedulerBootstrap.BuildGarnetArguments(port.ToString()));
+                    server.Start();
+                } catch {
+                    server?.Dispose();
+                    server = null;
+                }
+            }
+            Assert.NotNull(server);
+            using (server) {
+                using var redis = await ConnectWithRetryAsync(port);
+                var db = redis.GetDatabase();
 
-            using var redis = await ConnectWithRetryAsync(port);
-            var db = redis.GetDatabase();
-
-            await RunCodingAgentEnqueueScriptAsync(db);
-            await RunAgentChatLockScriptsAsync(db);
-            await RunMusicGenerationSemaphoreScriptAsync(db);
+                await RunCodingAgentEnqueueScriptAsync(db);
+                await RunAgentChatLockScriptsAsync(db);
+                await RunMusicGenerationSemaphoreScriptAsync(db);
+            }

Also applies to: 150-157

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TelegramSearchBot.Test/AppBootstrap/GarnetLuaScriptIntegrationTests.cs`
around lines 17 - 19, The test currently uses GetAvailablePort() which closes
the probe socket before GarnetServer binds, causing a TOCTOU race; change the
setup so the test either binds and holds the listener until GarnetServer takes
it (have GetAvailablePort return an already-bound TcpListener or endpoint) or
modify GarnetServer/SchedulerBootstrap to accept an ephemeral port (0) and then
query the actual bound port after server.Start(); update the test to pass the
held/socket-backed endpoint into GarnetServer (or use port 0 and read
server.BoundPort after Start) instead of releasing the port between probe and
bind.


using var redis = await ConnectWithRetryAsync(port);
var db = redis.GetDatabase();

await RunCodingAgentEnqueueScriptAsync(db);
await RunAgentChatLockScriptsAsync(db);
await RunMusicGenerationSemaphoreScriptAsync(db);
}

private static async Task RunCodingAgentEnqueueScriptAsync(IDatabase db) {
var jobId = Guid.NewGuid().ToString("N");
var activeKey = $"test:garnet:lua:active:{jobId}";
var stateKey = $"test:garnet:lua:state:{jobId}";
var queueKey = $"test:garnet:lua:queue:{jobId}";
var payload = JsonConvert.SerializeObject(new CodingAgentJobRequest {
JobId = jobId,
ChatId = 1001,
UserId = 2002,
MessageId = 3003,
WorkingDirectory = "workspace",
Prompt = "test"
});
var createdAtUtc = DateTime.UtcNow.ToString("O");
var updatedAtUtc = DateTime.UtcNow.ToString("O");

var result = await db.ScriptEvaluateAsync(
CodingAgentJobService.EnqueueJobScript,
new RedisKey[] {
activeKey,
stateKey,
queueKey
},
new RedisValue[] {
jobId,
2,
60,
payload,
CodingAgentJobStatus.Pending.ToString(),
1001,
2002,
3003,
"workspace",
createdAtUtc,
updatedAtUtc
});

Assert.Equal(1, ( int ) result);
Assert.True(await db.SetContainsAsync(activeKey, jobId));
Assert.Equal(CodingAgentJobStatus.Pending.ToString(), await db.HashGetAsync(stateKey, "status"));
Assert.Equal(payload, await db.ListLeftPopAsync(queueKey));
}

private static async Task RunAgentChatLockScriptsAsync(IDatabase db) {
var lockKey = $"test:garnet:lua:lock:{Guid.NewGuid():N}";
var lockValue = Guid.NewGuid().ToString("N");
await db.StringSetAsync(lockKey, lockValue, TimeSpan.FromSeconds(30));

var renewed = await db.ScriptEvaluateAsync(
AgentChatBatchDispatchService.RenewLockScript,
new RedisKey[] { lockKey },
new RedisValue[] { lockValue, 30_000 });
Assert.Equal(1, ( int ) renewed);

var wrongRelease = await db.ScriptEvaluateAsync(
AgentChatBatchDispatchService.ReleaseLockScript,
new RedisKey[] { lockKey },
new RedisValue[] { "not-the-owner" });
Assert.Equal(0, ( int ) wrongRelease);
Assert.True(await db.KeyExistsAsync(lockKey));

var release = await db.ScriptEvaluateAsync(
AgentChatBatchDispatchService.ReleaseLockScript,
new RedisKey[] { lockKey },
new RedisValue[] { lockValue });
Assert.Equal(1, ( int ) release);
Assert.False(await db.KeyExistsAsync(lockKey));
}

private static async Task RunMusicGenerationSemaphoreScriptAsync(IDatabase db) {
var key = $"test:garnet:lua:music:{Guid.NewGuid():N}";

var firstAcquire = await db.ScriptEvaluateAsync(
MusicGenerationToolService.AcquireChannelSemaphoreScript,
new RedisKey[] { key },
new RedisValue[] { 1 });
Assert.Equal(1, ( int ) firstAcquire);

var secondAcquire = await db.ScriptEvaluateAsync(
MusicGenerationToolService.AcquireChannelSemaphoreScript,
new RedisKey[] { key },
new RedisValue[] { 1 });
Assert.Equal(0, ( int ) secondAcquire);
Assert.Equal(1, ( int ) await db.StringGetAsync(key));

await db.StringSetAsync(key, -1);
var normalizedAcquire = await db.ScriptEvaluateAsync(
MusicGenerationToolService.AcquireChannelSemaphoreScript,
new RedisKey[] { key },
new RedisValue[] { 1 });
Assert.Equal(1, ( int ) normalizedAcquire);
Assert.Equal(1, ( int ) await db.StringGetAsync(key));
}

private static async Task<ConnectionMultiplexer> ConnectWithRetryAsync(int port) {
var options = new ConfigurationOptions {
AbortOnConnectFail = false,
ConnectRetry = 1,
ConnectTimeout = 500,
SyncTimeout = 2_000,
AsyncTimeout = 2_000
};
options.EndPoints.Add(IPAddress.Loopback, port);

var stopwatch = Stopwatch.StartNew();
Exception? lastException = null;
while (stopwatch.Elapsed < TimeSpan.FromSeconds(10)) {
try {
return await ConnectionMultiplexer.ConnectAsync(options);
} catch (RedisConnectionException ex) {
lastException = ex;
await Task.Delay(100);
} catch (SocketException ex) {
lastException = ex;
await Task.Delay(100);
}
}

throw new TimeoutException($"Timed out connecting to embedded Garnet on port {port}.", lastException);
}

private static int GetAvailablePort() {
var listener = new TcpListener(IPAddress.Loopback, 0);
listener.Start();
try {
return (( IPEndPoint ) listener.LocalEndpoint).Port;
} finally {
listener.Stop();
}
}
}
}
5 changes: 4 additions & 1 deletion TelegramSearchBot/AppBootstrap/SchedulerBootstrap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@

namespace TelegramSearchBot.AppBootstrap {
public class SchedulerBootstrap : AppBootstrap {
internal static string[] BuildGarnetArguments(string port) =>
["--bind", "127.0.0.1", "--port", port, "--lua", "--lua-transaction-mode"];

public static void Startup(string[] args) {
if (args.Length != 2 || !args[0].Equals("Scheduler")) {
return;
}
try {
using var server = new GarnetServer(["--bind", "127.0.0.1", "--port", args[1]]);
using var server = new GarnetServer(BuildGarnetArguments(args[1]));
server.Start();
Thread.Sleep(Timeout.Infinite);
} catch (Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ public sealed class AgentChatBatchDispatchService : BackgroundService {
private static readonly TimeSpan PollInterval = TimeSpan.FromSeconds(1);
private static readonly TimeSpan LockTtl = TimeSpan.FromSeconds(30);
private static readonly TimeSpan LockRenewInterval = TimeSpan.FromSeconds(10);
private const string ReleaseLockScript = @"
internal const string ReleaseLockScript = @"
if redis.call('GET', KEYS[1]) == ARGV[1] then
return redis.call('DEL', KEYS[1])
end
return 0";
private const string RenewLockScript = @"
internal const string RenewLockScript = @"
if redis.call('GET', KEYS[1]) == ARGV[1] then
redis.call('PEXPIRE', KEYS[1], ARGV[2])
return 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public class MusicGenerationToolService : IService {
private static readonly int[] AllowedSampleRates = { 16000, 24000, 32000, 44100 };
private static readonly int[] AllowedBitrates = { 32000, 64000, 128000, 256000 };
private static readonly string[] AllowedAudioFormats = { "mp3", "wav", "pcm" };
private const string AcquireChannelSemaphoreScript = @"
internal const string AcquireChannelSemaphoreScript = @"
local key = KEYS[1]
local limit = tonumber(ARGV[1])
local current = tonumber(redis.call('GET', key) or '0')
Expand Down
Loading