Skip to content

Commit da12986

Browse files
committed
fix: avoid recall lock errors and harden core coverage
Keep concurrent agent reads from tripping SQLite open-time locks and add direct tests around output, resume, and handoff behaviour so regressions surface closer to the affected packages.
1 parent 5e87be7 commit da12986

7 files changed

Lines changed: 1701 additions & 5 deletions

File tree

docs/mvp-status.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# MVP Status
22

3-
Last updated: 2026-04-14
3+
Last updated: 2026-04-15
44

55
This file is the repo's explicit implementation tracker.
66

@@ -23,7 +23,7 @@ If you are resuming work in a new session:
2323

2424
## Current Summary
2525

26-
The core local-memory workflow is implemented end to end, and the previously listed follow-up gaps around task state control, history indexing cost, and resume / handoff reuse have now been closed.
26+
The core local-memory workflow is implemented end to end, and the previously listed follow-up gaps around task state control, history indexing cost, resume / handoff reuse, and real-repo FTS validation have now been closed.
2727

2828
Working commands:
2929

@@ -82,6 +82,8 @@ Working global flags:
8282
- SQLite migrations now use an explicit schema version.
8383
- The Go module path is now the canonical `github.com/forjd/aid`.
8484
- The repo now explicitly stays FTS-first; optional embeddings are deferred until real recall gaps justify the added complexity.
85+
- FTS-only recall has now been validated in larger real repos; exact and near-exact lexical queries perform well enough to keep embeddings deferred for now.
86+
- Concurrent `aid recall` runs against the same repo no longer trip transient SQLite `database is locked` failures during store open.
8587

8688
## Partial
8789

@@ -106,11 +108,12 @@ Working global flags:
106108

107109
## Recommended Next Work
108110

109-
1. Validate the FTS-only recall path in larger real repos before revisiting embeddings.
111+
1. Start the coverage-hardening pass in [docs/plans/code-coverage-improvement.md](./plans/code-coverage-improvement.md), beginning with direct tests for `internal/output`, `internal/resume`, and `internal/handoff`.
110112
2. Decide whether post-MVP expansion should go toward a TUI, background automation, or multi-user sync instead of keeping the tool intentionally CLI-only.
111113

112114
## Open Notes
113115

114116
- The current implementation already satisfies the practical MVP command surface.
115117
- Remaining work is now product-expansion work rather than missing core command behavior.
118+
- Recent validation in larger repos suggests the main recall misses are tokenisation and phrasing gaps such as `town house` vs `townhouse` or `optimise` vs `speed up`, rather than a broad need for embeddings.
116119
- Future sessions should treat this file as the source of truth for implementation status.

internal/cli/cli_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os/exec"
99
"path/filepath"
1010
"strings"
11+
"sync"
1112
"testing"
1213

1314
"github.com/forjd/aid/internal/app"
@@ -94,6 +95,83 @@ func TestLeafCommandAllowsHelpAsArgument(t *testing.T) {
9495
}
9596
}
9697

98+
func TestConcurrentRecallSubprocessesDoNotHitDatabaseLocked(t *testing.T) {
99+
repoDir := t.TempDir()
100+
dataDir := filepath.Join(t.TempDir(), "aid-data")
101+
102+
runGit(t, repoDir, "init", "-q")
103+
writeFile(t, filepath.Join(repoDir, "README.md"), []byte("repo\n"))
104+
runGitWithIdentity(t, repoDir, "add", "README.md")
105+
runGitWithIdentity(t, repoDir, "commit", "-m", "feat: add readme")
106+
107+
env := []string{"AID_DATA_DIR=" + dataDir}
108+
109+
for _, args := range [][]string{
110+
{"init"},
111+
{"note", "add", "Refresh retry investigation"},
112+
{"decide", "add", "Use single refresh retry"},
113+
{"handoff", "generate"},
114+
{"history", "index"},
115+
} {
116+
output, err := runCLISubprocess(t, repoDir, env, args...)
117+
if err != nil {
118+
t.Fatalf("seed command %v failed: %v\n%s", args, err, output)
119+
}
120+
}
121+
122+
type recallResult struct {
123+
output string
124+
err error
125+
args []string
126+
}
127+
128+
queries := [][]string{
129+
{"recall", "refresh retry", "--json"},
130+
{"recall", "token refresh", "--json"},
131+
}
132+
133+
for i := 0; i < 20; i++ {
134+
results := make(chan recallResult, len(queries))
135+
var wg sync.WaitGroup
136+
137+
for _, args := range queries {
138+
args := append([]string(nil), args...)
139+
wg.Add(1)
140+
go func() {
141+
defer wg.Done()
142+
output, err := runCLISubprocess(t, repoDir, env, args...)
143+
results <- recallResult{output: output, err: err, args: args}
144+
}()
145+
}
146+
147+
wg.Wait()
148+
close(results)
149+
150+
for result := range results {
151+
if result.err != nil {
152+
t.Fatalf("concurrent recall %v failed on iteration %d: %v\n%s", result.args, i+1, result.err, result.output)
153+
}
154+
155+
var payload struct {
156+
OK bool `json:"ok"`
157+
Error *struct {
158+
Message string `json:"message"`
159+
} `json:"error"`
160+
}
161+
if err := json.Unmarshal([]byte(result.output), &payload); err != nil {
162+
t.Fatalf("unmarshal recall output for %v on iteration %d: %v\n%s", result.args, i+1, err, result.output)
163+
}
164+
if !payload.OK {
165+
message := ""
166+
if payload.Error != nil {
167+
message = payload.Error.Message
168+
}
169+
t.Fatalf("concurrent recall %v returned error payload on iteration %d: %s\n%s", result.args, i+1, message, result.output)
170+
}
171+
}
172+
}
173+
}
174+
97175
func TestInitAndCrudFlow(t *testing.T) {
98176
repoDir := t.TempDir()
99177
dataDir := filepath.Join(t.TempDir(), "aid-data")
@@ -1247,6 +1325,38 @@ func runCLI(t *testing.T, args ...string) string {
12471325
return stdout.String()
12481326
}
12491327

1328+
func TestCLIHelperProcess(t *testing.T) {
1329+
if os.Getenv("AID_TEST_SUBPROCESS") != "1" {
1330+
return
1331+
}
1332+
1333+
separator := -1
1334+
for i, arg := range os.Args {
1335+
if arg == "--" {
1336+
separator = i
1337+
break
1338+
}
1339+
}
1340+
if separator == -1 {
1341+
os.Exit(2)
1342+
}
1343+
1344+
exitCode := Run(os.Args[separator+1:], os.Stdout, os.Stderr)
1345+
os.Exit(exitCode)
1346+
}
1347+
1348+
func runCLISubprocess(t *testing.T, cwd string, env []string, args ...string) (string, error) {
1349+
t.Helper()
1350+
1351+
cmdArgs := append([]string{"-test.run=TestCLIHelperProcess", "--"}, args...)
1352+
cmd := exec.Command(os.Args[0], cmdArgs...)
1353+
cmd.Dir = cwd
1354+
cmd.Env = append(append(os.Environ(), "AID_TEST_SUBPROCESS=1"), env...)
1355+
1356+
output, err := cmd.CombinedOutput()
1357+
return string(output), err
1358+
}
1359+
12501360
func runGit(t *testing.T, cwd string, args ...string) {
12511361
t.Helper()
12521362

internal/handoff/handoff_test.go

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
package handoff
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/forjd/aid/internal/git"
8+
resumepkg "github.com/forjd/aid/internal/resume"
9+
"github.com/forjd/aid/internal/store"
10+
)
11+
12+
func TestBuild(t *testing.T) {
13+
nextAction := "inspect retry middleware"
14+
snapshot := Build(
15+
"feat/auth",
16+
git.WorktreeStatus{Dirty: true, Changed: 2, Untracked: 1},
17+
resumepkg.Bundle{
18+
ActiveTask: taskPointer(handoffTask(1, "feat/auth", store.TaskInProgress, "Fix refresh retry path")),
19+
Notes: []store.Note{
20+
handoffNote(1, "Retry still fails after 401"),
21+
},
22+
Decisions: []store.Decision{
23+
handoffDecision(1, "Use a single refresh retry"),
24+
},
25+
RecentCommits: []store.Commit{
26+
handoffCommit("abc123456789", "fix: refresh retry path"),
27+
},
28+
OpenQuestions: []string{
29+
"Which retry path is still failing?",
30+
"Should the API return a clearer auth error?",
31+
},
32+
NextAction: &nextAction,
33+
},
34+
[]store.Task{
35+
handoffTask(1, "feat/auth", store.TaskInProgress, "Fix refresh retry path"),
36+
handoffTask(2, "feat/auth", store.TaskOpen, "Review retry logging"),
37+
handoffTask(3, "feat/auth", store.TaskDone, "Ignore completed work"),
38+
},
39+
)
40+
41+
want := "Branch: feat/auth\n" +
42+
"Worktree: dirty (2 changed, 1 untracked)\n" +
43+
"Active task: Fix refresh retry path\n" +
44+
"Open tasks:\n" +
45+
"- Fix refresh retry path [in_progress]\n" +
46+
"- Review retry logging [open]\n" +
47+
"Recent notes:\n" +
48+
"- Retry still fails after 401\n" +
49+
"Key decisions:\n" +
50+
"- Use a single refresh retry\n" +
51+
"Recent commits:\n" +
52+
"- abc1234 fix: refresh retry path\n" +
53+
"Open questions:\n" +
54+
"- Which retry path is still failing?\n" +
55+
"- Should the API return a clearer auth error?\n" +
56+
"- Should the current uncommitted changes be kept, finished, or discarded?\n" +
57+
"Recommended next action:\n" +
58+
"- inspect retry middleware"
59+
60+
if snapshot.Branch != "feat/auth" {
61+
t.Fatalf("unexpected branch: %#v", snapshot)
62+
}
63+
if snapshot.Summary != want {
64+
t.Fatalf("unexpected handoff summary:\n%s", snapshot.Summary)
65+
}
66+
}
67+
68+
func TestBuildWithAmbiguousTaskAndCleanWorktree(t *testing.T) {
69+
snapshot := Build(
70+
"feat/auth",
71+
git.WorktreeStatus{},
72+
resumepkg.Bundle{ActiveTaskAmbiguous: true},
73+
nil,
74+
)
75+
76+
want := "Branch: feat/auth\nWorktree: clean\nActive task: ambiguous"
77+
if snapshot.Summary != want {
78+
t.Fatalf("unexpected minimal handoff summary:\n%s", snapshot.Summary)
79+
}
80+
}
81+
82+
func TestLimitOpenTasks(t *testing.T) {
83+
tasks := []store.Task{
84+
handoffTask(1, "", store.TaskOpen, "One"),
85+
handoffTask(2, "", store.TaskDone, "Done"),
86+
handoffTask(3, "", store.TaskBlocked, "Two"),
87+
handoffTask(4, "", store.TaskOpen, "Three"),
88+
handoffTask(5, "", store.TaskOpen, "Four"),
89+
handoffTask(6, "", store.TaskOpen, "Five"),
90+
handoffTask(7, "", store.TaskOpen, "Six"),
91+
}
92+
93+
limited := limitOpenTasks(tasks, 5)
94+
if len(limited) != 5 {
95+
t.Fatalf("expected 5 open tasks, got %#v", limited)
96+
}
97+
for _, task := range limited {
98+
if task.Status == store.TaskDone {
99+
t.Fatalf("did not expect done task in open task list: %#v", limited)
100+
}
101+
}
102+
if limited[4].Text != "Five" {
103+
t.Fatalf("expected limit to preserve task order, got %#v", limited)
104+
}
105+
}
106+
107+
func TestWorktreeLine(t *testing.T) {
108+
tests := []struct {
109+
name string
110+
status git.WorktreeStatus
111+
want string
112+
}{
113+
{name: "clean", status: git.WorktreeStatus{}, want: "clean"},
114+
{name: "untracked only", status: git.WorktreeStatus{Dirty: true, Untracked: 2}, want: "dirty (2 untracked)"},
115+
{name: "changed only", status: git.WorktreeStatus{Dirty: true, Changed: 3}, want: "dirty (3 changed)"},
116+
{name: "changed and untracked", status: git.WorktreeStatus{Dirty: true, Changed: 3, Untracked: 2}, want: "dirty (3 changed, 2 untracked)"},
117+
}
118+
119+
for _, test := range tests {
120+
t.Run(test.name, func(t *testing.T) {
121+
if got := worktreeLine(test.status); got != test.want {
122+
t.Fatalf("worktreeLine(%#v) = %q, want %q", test.status, got, test.want)
123+
}
124+
})
125+
}
126+
}
127+
128+
func TestShortSHA(t *testing.T) {
129+
if got := shortSHA("abc1234"); got != "abc1234" {
130+
t.Fatalf("expected unchanged short sha, got %q", got)
131+
}
132+
if got := shortSHA("abc123456789"); got != "abc1234" {
133+
t.Fatalf("expected shortened sha, got %q", got)
134+
}
135+
}
136+
137+
func TestHandoffOpenQuestions(t *testing.T) {
138+
questions := handoffOpenQuestions([]string{"One", "Two", "Three"}, git.WorktreeStatus{Dirty: true})
139+
if len(questions) != 3 || questions[2] != "Three" {
140+
t.Fatalf("expected existing questions to be kept when already at limit, got %#v", questions)
141+
}
142+
143+
questions = handoffOpenQuestions([]string{"One", "Two"}, git.WorktreeStatus{Dirty: true})
144+
want := []string{"One", "Two", "Should the current uncommitted changes be kept, finished, or discarded?"}
145+
if len(questions) != len(want) {
146+
t.Fatalf("unexpected handoff questions: %#v", questions)
147+
}
148+
for i := range want {
149+
if questions[i] != want[i] {
150+
t.Fatalf("unexpected handoff questions: %#v", questions)
151+
}
152+
}
153+
questions = handoffOpenQuestions(nil, git.WorktreeStatus{})
154+
if len(questions) != 0 {
155+
t.Fatalf("expected no questions for clean worktree, got %#v", questions)
156+
}
157+
}
158+
159+
func taskPointer(task store.Task) *store.Task {
160+
return &task
161+
}
162+
163+
func handoffTask(id int64, branch string, status store.TaskStatus, text string) store.Task {
164+
return store.Task{
165+
ID: id,
166+
Branch: branch,
167+
Text: text,
168+
Status: status,
169+
CreatedAt: handoffTime(),
170+
UpdatedAt: handoffTime(),
171+
}
172+
}
173+
174+
func handoffNote(id int64, text string) store.Note {
175+
return store.Note{ID: id, Text: text, CreatedAt: handoffTime()}
176+
}
177+
178+
func handoffDecision(id int64, text string) store.Decision {
179+
return store.Decision{ID: id, Text: text, CreatedAt: handoffTime()}
180+
}
181+
182+
func handoffCommit(sha, summary string) store.Commit {
183+
return store.Commit{SHA: sha, Summary: summary, CommittedAt: handoffTime()}
184+
}
185+
186+
func handoffTime() time.Time {
187+
return time.Date(2026, 4, 15, 10, 0, 0, 0, time.UTC)
188+
}

0 commit comments

Comments
 (0)