Compare commits

..
Author SHA1 Message Date
BaaRai dfa0e8a566 ci: run ./server under the race detector
The server suite now passes go test -race against postgres, and its test teardown no longer leaks live servers (#227, #228). Add ./server to the go test -race step; the DSN and -p 1 are unchanged.
2026-11-14 19:11:10 +01:00
BaaRai 9b20a46eba Merge pull request #228 from BaaRai/fix/flaky-server-tests
test(server): fix flaky SkillGaps, notify digest and TempDir cleanup tests
2026-11-14 19:03:16 +01:00
BaaRai 8c8ae97400 test(server): join every live test server before its TempDir cleanup
New restores every unfinished task in the shared database and runs its planner, worker and deadline loops on the server context. Tests that passed context.Background() never stopped or joined them, so they kept writing under the manager TempDir after the test returned and the TempDir cleanup failed with directory not empty. Add stopTestServer, which cancels the context, waits for each task to go quiescent, joins its loops, the archive worker and the side-question writer, and defer it right after New in every test that started a live server.
2026-11-14 18:21:52 +01:00
BaaRai 149151919f test(server): join restored task loops before TestTaskDetailReturnsListMetrics cleanup
New restores every unfinished task in the shared database and runs its planner and worker loops on the server context. They kept writing transcripts under the TempDir after the test returned, so the TempDir cleanup failed with directory not empty. Cancel the context, wait for each task to go quiescent, and join its loops before m.Close and the TempDir cleanup.
2026-11-14 16:21:31 +01:00
BaaRai 5cdb720122 test(server): stop leaked fixtures from polluting later tests
TestMgmtAPI never closed its manager, so the notifier and task-archive workers started by New(context.Background()) kept claiming other tests digest batches and archive jobs. TestTaskOutcomeCard left a skill_usage miss row behind (the table has no foreign keys), which outranked TestSkillGapsReport fixture from the fourth -count iteration on.
2026-11-14 15:44:52 +01:00
BaaRai ae792126dc Merge pull request #227 from BaaRai/fix/agent-hook-race
agent: synchronise package-level hooks (data races under -race)
2026-11-14 15:01:30 +01:00
14 changed files with 87 additions and 19 deletions
+5 -3
View File
@@ -45,8 +45,9 @@ jobs:
# keeps the database state deterministic.
run: go test -p 1 ./...
- name: go test -race
# Race detector on the packages that pass under -race today. ./server is
# left out until it has been checked with -race against postgres.
# Race detector on the packages that pass under -race today. ./server
# joined once its suite was checked with -race against postgres and its
# test teardown fixed (#227, #228).
# Its own database: the plain run above leaves rows behind and the db tests
# are not idempotent across runs (duplicate llm_profiles names).
env:
@@ -54,7 +55,8 @@ jobs:
run: >-
go test -race -p 1 ./agent ./cmd/artex ./config ./db ./evidence ./guard
./intercept ./llmpool ./llmrec ./mcphttp ./notify ./sidequestion ./traffic
./intercept ./llmpool ./llmrec ./mcphttp ./notify ./server ./sidequestion
./traffic
shell:
runs-on: ubuntu-latest
+6 -2
View File
@@ -18,7 +18,9 @@ func TestChatUnavailableReasonDistinguishesStates(t *testing.T) {
}
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
// Start from a clean profile table; other tests in the shared DB may have left rows.
existing, _ := m.pg.ListProfiles()
@@ -68,7 +70,9 @@ func TestResolveChatAgentHonoursConversationProfile(t *testing.T) {
}
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
// Force the global fallback to nil so a non-nil result can ONLY come from the
// conversation's own profile — this is exactly the situation the user hit
+3 -1
View File
@@ -24,7 +24,9 @@ func TestCoreTaskLifecyclePG(t *testing.T) {
}
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
h := s.Handler()
tok, err := signJWT(s.jwtKey)
if err != nil {
+3 -1
View File
@@ -67,7 +67,9 @@ func TestInheritedActivityDetailAndRelationDeletion(t *testing.T) {
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
+6 -2
View File
@@ -50,7 +50,9 @@ SELECT id,'completed','fixed','{}'::jsonb FROM findings WHERE task_id=$1 AND nam
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
@@ -144,7 +146,9 @@ VALUES ($1,'xss','Reflected XSS','reflected in search','high','confirmed')`, tas
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
+7 -1
View File
@@ -18,8 +18,14 @@ func TestMgmtAPI(t *testing.T) {
if m.pg == nil {
t.Skip("postgres unavailable — skipping management API test")
}
// New starts the notifier and task-archive workers on context.Background();
// closing the pool stops them from claiming later tests' digest batches
// and archive jobs.
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
h := s.Handler()
tok, err := signJWT(s.jwtKey)
if err != nil {
+3 -1
View File
@@ -33,7 +33,9 @@ INSERT INTO skill_usage(skill,agent_key,task_id,found) VALUES
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
+3 -1
View File
@@ -21,7 +21,9 @@ func TestTaskCategoryBatchRoute(t *testing.T) {
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
h := s.Handler()
token, err := signJWT(s.jwtKey)
if err != nil {
+3 -1
View File
@@ -16,7 +16,9 @@ func TestWorkerControlRoutes(t *testing.T) {
defer m.Close()
td := t.TempDir()
s := New(context.Background(), m, td, td, td)
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, td, td, td)
defer stopTestServer(t, s, cancel)
h := s.Handler()
token, err := signJWT(s.jwtKey)
if err != nil {
+3 -1
View File
@@ -38,7 +38,9 @@ VALUES ($1,'goal','{}','met'), ($1,'goal','{}','open')`, task.ExpID); err != nil
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
+6 -2
View File
@@ -24,7 +24,9 @@ func TestTaskMetadataPatchReturnsRenameAndPin(t *testing.T) {
}
taskID, _ := strconv.ParseInt(task.ID, 10, 64)
defer func() { _ = m.pg.DeleteTask(taskID) }()
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
@@ -66,7 +68,9 @@ func TestConversationBatchDeleteReportsMissing(t *testing.T) {
t.Skipf("postgres unavailable (%v) — skipping", err)
}
defer m.Close()
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
first, err := m.pg.CreateConversation("mainagent", "batch-http-first", nil)
if err != nil {
t.Fatal(err)
+6 -1
View File
@@ -23,6 +23,9 @@ func TestTaskOutcomeCard(t *testing.T) {
}
taskID, _ := strconv.ParseInt(task.ID, 10, 64)
defer func() { _ = m.pg.DeleteTask(taskID) }()
// skill_usage has no foreign keys, so DeleteTask leaves these rows behind,
// and their misses would outrank TestSkillGapsReport's fixture in later runs.
defer func() { _, _ = m.pg.Exec(`DELETE FROM skill_usage WHERE task_id=$1`, taskID) }()
if _, err := m.pg.Exec(`
INSERT INTO findings(task_id,vulnclass,name,severity,status) VALUES
@@ -57,7 +60,9 @@ INSERT INTO skill_usage(skill,task_id,found) VALUES ('api-recon',$1,true),('grap
t.Fatal(err)
}
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
token, err := signJWT(s.jwtKey)
if err != nil {
t.Fatal(err)
+6 -2
View File
@@ -20,7 +20,9 @@ func TestTaskTemplateHTTPCRUD(t *testing.T) {
t.Skipf("postgres unavailable (%v) — skipping", err)
}
defer m.Close()
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
h := s.Handler()
token, err := signJWT(s.jwtKey)
if err != nil {
@@ -90,7 +92,9 @@ func TestConversationPatchReturnsPinState(t *testing.T) {
t.Skipf("postgres unavailable (%v) — skipping", err)
}
defer m.Close()
s := New(context.Background(), m, t.TempDir(), t.TempDir(), t.TempDir())
ctx, cancel := context.WithCancel(context.Background())
s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
defer stopTestServer(t, s, cancel)
conversation, err := m.pg.CreateConversation("mainagent", "pin through http", nil)
if err != nil {
t.Fatal(err)
+27
View File
@@ -0,0 +1,27 @@
package server
import (
"context"
"testing"
"time"
)
// stopTestServer cancels a test server's context and joins what New started.
// New restores every unfinished task in the shared database and runs its
// planner, worker and deadline loops, which write under m.dir. Call it with
// defer right after New, so it runs before m.Close and the TempDir cleanups.
func stopTestServer(t *testing.T, s *Server, cancel context.CancelFunc) {
t.Helper()
cancel()
for _, task := range s.m.List() {
s.engine.BeginDelete(task.ID)
waitCtx, waitCancel := context.WithTimeout(context.Background(), 30*time.Second)
if err := s.waitTaskQuiescent(waitCtx, task.ID); err != nil {
t.Errorf("task %s still running at teardown: %v", task.ID, err)
}
waitCancel()
s.engine.StopTask(task.ID)
}
s.archiveWG.Wait()
<-s.side.done
}