fix: clean prompt files and reduce nudge noise
This commit is contained in:
parent
b1544c32ea
commit
fa68474e6f
|
|
@ -226,13 +226,65 @@ func TestPRObservation_CIFailingNudgesAgentWithLogs(t *testing.T) {
|
|||
"Failure URL: https://ci.example/build",
|
||||
"Log tail (last 1 line):",
|
||||
"boom",
|
||||
"Failed: lint (cancelled)",
|
||||
"fetch full CI logs only if you need additional context",
|
||||
} {
|
||||
if !strings.Contains(msg.msgs[0], want) {
|
||||
t.Fatalf("CI nudge missing %q:\n%s", want, msg.msgs[0])
|
||||
}
|
||||
}
|
||||
if strings.Contains(msg.msgs[0], "lint") || strings.Contains(msg.msgs[0], "cancelled") {
|
||||
t.Fatalf("cancelled checks must not be included in CI nudge:\n%s", msg.msgs[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRObservation_CancelledChecksDoNotNudge(t *testing.T) {
|
||||
m, st, msg := newManager()
|
||||
st.sessions["mer-1"] = working("mer-1")
|
||||
o := ports.PRObservation{Fetched: true, URL: "pr1", CI: domain.CIFailing, Checks: []ports.PRCheckObservation{
|
||||
{Name: "lint", CommitHash: "c1", Status: domain.PRCheckCancelled, URL: "https://ci.example/lint"},
|
||||
}}
|
||||
if err := m.ApplyPRObservation(ctx, "mer-1", o); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(msg.msgs) != 0 {
|
||||
t.Fatalf("cancelled-only checks must not nudge, got %v", msg.msgs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReviewCommentsSignatureUsesStableIDs(t *testing.T) {
|
||||
original := []ports.PRCommentObservation{
|
||||
{ID: "c1", ThreadID: "t1", Author: "alice", File: "old.go", Line: 10, Body: "old", URL: "https://old"},
|
||||
{ID: "c2", ThreadID: "t2", Author: "bob", File: "old.go", Line: 20, Body: "old", URL: "https://old"},
|
||||
}
|
||||
editedAndReordered := []ports.PRCommentObservation{
|
||||
{ID: "c2", ThreadID: "t2", Author: "bob", File: "new.go", Line: 99, Body: "edited", URL: "https://new"},
|
||||
{ID: "c1", ThreadID: "t1", Author: "alice", File: "new.go", Line: 42, Body: "edited", URL: "https://new"},
|
||||
}
|
||||
if got, want := reviewCommentsSignature(editedAndReordered), reviewCommentsSignature(original); got != want {
|
||||
t.Fatalf("signature changed after edit/reorder\n got %q\nwant %q", got, want)
|
||||
}
|
||||
|
||||
withNewComment := append([]ports.PRCommentObservation(nil), original...)
|
||||
withNewComment = append(withNewComment, ports.PRCommentObservation{ID: "c3", ThreadID: "t2", Body: "new comment in same thread"})
|
||||
if got, old := reviewCommentsSignature(withNewComment), reviewCommentsSignature(original); got == old {
|
||||
t.Fatalf("new comment id should change signature, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatCIFailureMessageUsesNonMutatingFence(t *testing.T) {
|
||||
logTail := "start\n```\ninner\n````\nend"
|
||||
msg := formatCIFailureMessage([]ports.PRCheckObservation{{
|
||||
Name: "build", Status: domain.PRCheckFailed, LogTail: logTail,
|
||||
}})
|
||||
if !strings.Contains(msg, logTail) {
|
||||
t.Fatalf("message should preserve log text without zero-width mutation:\n%s", msg)
|
||||
}
|
||||
if strings.Contains(msg, "\u200b") {
|
||||
t.Fatalf("message must not insert zero-width characters:\n%s", msg)
|
||||
}
|
||||
if !strings.Contains(msg, "`````\n"+logTail+"\n`````") {
|
||||
t.Fatalf("message should wrap log in a fence longer than embedded runs:\n%s", msg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRObservation_ReviewCommentsNudgeAgent(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -511,7 +511,7 @@ func hasUnresolvedComments(comments []ports.PRCommentObservation) bool {
|
|||
func failedPRChecks(checks []ports.PRCheckObservation) []ports.PRCheckObservation {
|
||||
failed := make([]ports.PRCheckObservation, 0, len(checks))
|
||||
for _, ch := range checks {
|
||||
if ch.Status == domain.PRCheckFailed || ch.Status == domain.PRCheckCancelled {
|
||||
if ch.Status == domain.PRCheckFailed {
|
||||
failed = append(failed, ch)
|
||||
}
|
||||
}
|
||||
|
|
@ -545,14 +545,16 @@ func formatCIFailureMessage(checks []ports.PRCheckObservation) string {
|
|||
if ch.LogTail != "" {
|
||||
// LogTail is raw CI job output; sanitize before it reaches the
|
||||
// agent's live pane so embedded escape sequences can't drive the
|
||||
// terminal (the dedup signature stays on the raw bytes).
|
||||
tail := escapeMarkdownCodeFenceClosers(domain.SanitizeControlChars(ch.LogTail))
|
||||
// terminal (the dedup signature stays on the raw bytes). The fence
|
||||
// grows to contain embedded backtick fences without mutating logs.
|
||||
tail := domain.SanitizeControlChars(ch.LogTail)
|
||||
fence := markdownCodeFence(tail)
|
||||
lineCount := len(strings.Split(tail, "\n"))
|
||||
lineLabel := "lines"
|
||||
if lineCount == 1 {
|
||||
lineLabel = "line"
|
||||
}
|
||||
fmt.Fprintf(&msg, "\n\nLog tail (last %d %s):\n```\n%s\n```", lineCount, lineLabel, tail)
|
||||
fmt.Fprintf(&msg, "\n\nLog tail (last %d %s):\n%s\n%s\n%s", lineCount, lineLabel, fence, tail, fence)
|
||||
}
|
||||
msg.WriteString("\n")
|
||||
}
|
||||
|
|
@ -574,8 +576,14 @@ func unresolvedReviewComments(comments []ports.PRCommentObservation) []ports.PRC
|
|||
func reviewCommentsSignature(comments []ports.PRCommentObservation) string {
|
||||
parts := make([]string, 0, len(comments))
|
||||
for _, c := range comments {
|
||||
parts = append(parts, strings.Join([]string{c.ID, c.ThreadID, c.Author, c.File, fmt.Sprint(c.Line), c.Body, c.URL}, "\x00"))
|
||||
id := strings.TrimSpace(c.ID)
|
||||
threadID := strings.TrimSpace(c.ThreadID)
|
||||
if id == "" && threadID == "" {
|
||||
continue
|
||||
}
|
||||
parts = append(parts, threadID+"\x00"+id)
|
||||
}
|
||||
sort.Strings(parts)
|
||||
return strings.Join(parts, "\x01")
|
||||
}
|
||||
|
||||
|
|
@ -614,14 +622,23 @@ func formatReviewCommentsMessage(comments []ports.PRCommentObservation) string {
|
|||
return msg.String()
|
||||
}
|
||||
|
||||
func escapeMarkdownCodeFenceClosers(s string) string {
|
||||
lines := strings.Split(s, "\n")
|
||||
for i, line := range lines {
|
||||
if strings.HasPrefix(line, "```") {
|
||||
lines[i] = "\u200b" + line
|
||||
func markdownCodeFence(s string) string {
|
||||
maxRun := 0
|
||||
run := 0
|
||||
for _, r := range s {
|
||||
if r == '`' {
|
||||
run++
|
||||
if run > maxRun {
|
||||
maxRun = run
|
||||
}
|
||||
continue
|
||||
}
|
||||
run = 0
|
||||
}
|
||||
return strings.Join(lines, "\n")
|
||||
if maxRun < 3 {
|
||||
return "```"
|
||||
}
|
||||
return strings.Repeat("`", maxRun+1)
|
||||
}
|
||||
|
||||
func (m *Manager) sendOnce(ctx context.Context, id domain.SessionID, prURL, key, sig, msg string, maxAttempts int) error {
|
||||
|
|
|
|||
|
|
@ -386,6 +386,7 @@ func sessionPrefix(project domain.ProjectRecord) string {
|
|||
// rollbackSpawnSeedRow.
|
||||
func (m *Manager) markSpawnFailedTerminated(ctx context.Context, id domain.SessionID) {
|
||||
_ = m.lcm.MarkTerminated(ctx, id)
|
||||
m.cleanupSystemPromptDir(id)
|
||||
}
|
||||
|
||||
// rollbackSpawnSeedRow best-effort removes the row of a spawn that failed
|
||||
|
|
@ -395,6 +396,7 @@ func (m *Manager) markSpawnFailedTerminated(ctx context.Context, id domain.Sessi
|
|||
// fails, fall back to parking it terminated so a phantom row never looks live.
|
||||
func (m *Manager) rollbackSpawnSeedRow(ctx context.Context, id domain.SessionID) {
|
||||
if deleted, err := m.store.DeleteSession(ctx, id); err == nil && deleted {
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return
|
||||
}
|
||||
m.markSpawnFailedTerminated(ctx, id)
|
||||
|
|
@ -417,6 +419,7 @@ func (m *Manager) rollbackSpawn(ctx context.Context, id domain.SessionID) (delet
|
|||
return false, false, fmt.Errorf("rollback %s: %w", id, err)
|
||||
}
|
||||
if deleted {
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return true, false, nil
|
||||
}
|
||||
killed, err = m.Kill(ctx, id)
|
||||
|
|
@ -456,6 +459,7 @@ func (m *Manager) Kill(ctx context.Context, id domain.SessionID) (bool, error) {
|
|||
if err := m.lcm.MarkTerminated(ctx, id); err != nil {
|
||||
return false, fmt.Errorf("kill %s: %w", id, err)
|
||||
}
|
||||
defer m.cleanupSystemPromptDir(id)
|
||||
|
||||
// Clear the restore marker so the next boot's RestoreAll cannot resurrect a
|
||||
// killed session (#2319). Best-effort: teardown below still matters.
|
||||
|
|
@ -540,12 +544,14 @@ func (m *Manager) Restore(ctx context.Context, id domain.SessionID) (domain.Sess
|
|||
}
|
||||
systemPromptFile, err := m.prepareSystemPromptFile(id, rec.Harness, systemPrompt)
|
||||
if err != nil {
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return domain.SessionRecord{}, fmt.Errorf("restore %s: system prompt file: %w", id, err)
|
||||
}
|
||||
// Restore re-applies the project's resolved agent config so a configured
|
||||
// model/permissions carry across a restore, matching fresh spawn.
|
||||
argv, err := restoreArgv(ctx, agent, id, ws.Path, meta, systemPrompt, systemPromptFile, effectiveAgentConfig(rec.Kind, project.Config), rec.Kind)
|
||||
if err != nil {
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return domain.SessionRecord{}, fmt.Errorf("restore %s: %w", id, err)
|
||||
}
|
||||
handle, err := m.runtime.Create(ctx, ports.RuntimeConfig{
|
||||
|
|
@ -555,11 +561,13 @@ func (m *Manager) Restore(ctx context.Context, id domain.SessionID) (domain.Sess
|
|||
Env: m.runtimeEnv(id, rec.ProjectID, rec.IssueID, project.Config.Env),
|
||||
})
|
||||
if err != nil {
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return domain.SessionRecord{}, fmt.Errorf("restore %s: runtime: %w", id, err)
|
||||
}
|
||||
metadata := domain.SessionMetadata{Branch: ws.Branch, WorkspacePath: ws.Path, RuntimeHandleID: handle.ID, AgentSessionID: meta.AgentSessionID, Prompt: meta.Prompt}
|
||||
if err := m.lcm.MarkSpawned(ctx, id, metadata); err != nil {
|
||||
_ = m.runtime.Destroy(ctx, handle)
|
||||
m.cleanupSystemPromptDir(id)
|
||||
return domain.SessionRecord{}, fmt.Errorf("restore %s: completed: %w", id, err)
|
||||
}
|
||||
return m.getRecord(ctx, id)
|
||||
|
|
@ -906,6 +914,7 @@ func (m *Manager) Cleanup(ctx context.Context, project domain.ProjectID) (Cleanu
|
|||
}
|
||||
ws := workspaceInfo(rec)
|
||||
if ws.Path == "" {
|
||||
m.cleanupSystemPromptDir(rec.ID)
|
||||
continue
|
||||
}
|
||||
if h := runtimeHandle(rec.Metadata); h.ID != "" {
|
||||
|
|
@ -920,6 +929,7 @@ func (m *Manager) Cleanup(ctx context.Context, project domain.ProjectID) (Cleanu
|
|||
result.Skipped = append(result.Skipped, CleanupSkip{SessionID: rec.ID, Reason: cleanupSkipReason(err)})
|
||||
continue
|
||||
}
|
||||
m.cleanupSystemPromptDir(rec.ID)
|
||||
result.Cleaned = append(result.Cleaned, rec.ID)
|
||||
}
|
||||
return result, nil
|
||||
|
|
@ -1089,7 +1099,7 @@ func (m *Manager) writeSystemPromptFile(id domain.SessionID, systemPrompt string
|
|||
if systemPrompt == "" || strings.TrimSpace(m.dataDir) == "" {
|
||||
return "", nil
|
||||
}
|
||||
path := filepath.Join(m.dataDir, "prompts", string(id), "system.md")
|
||||
path := filepath.Join(m.systemPromptDir(id), "system.md")
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
|
@ -1115,6 +1125,23 @@ func systemPromptFileRequired(harness domain.AgentHarness) bool {
|
|||
return harness == domain.HarnessAider
|
||||
}
|
||||
|
||||
func (m *Manager) systemPromptDir(id domain.SessionID) string {
|
||||
if strings.TrimSpace(m.dataDir) == "" {
|
||||
return ""
|
||||
}
|
||||
return filepath.Join(m.dataDir, "prompts", string(id))
|
||||
}
|
||||
|
||||
func (m *Manager) cleanupSystemPromptDir(id domain.SessionID) {
|
||||
dir := m.systemPromptDir(id)
|
||||
if dir == "" {
|
||||
return
|
||||
}
|
||||
if err := os.RemoveAll(dir); err != nil {
|
||||
m.logger.Warn("system prompt cleanup failed", "session", id, "path", dir, "err", err)
|
||||
}
|
||||
}
|
||||
|
||||
// spawnEnv builds the runtime environment: the per-project env vars first, then
|
||||
// the AO-internal vars last so they always win (a project cannot override
|
||||
// AO_SESSION_ID and friends).
|
||||
|
|
|
|||
|
|
@ -242,6 +242,16 @@ func blockedDataDir(t *testing.T) string {
|
|||
return path
|
||||
}
|
||||
|
||||
func requireNoPromptDir(t *testing.T, dataDir string, id domain.SessionID) {
|
||||
t.Helper()
|
||||
path := filepath.Join(dataDir, "prompts", string(id))
|
||||
if _, err := os.Stat(path); err == nil {
|
||||
t.Fatalf("prompt dir %s still exists", path)
|
||||
} else if !errors.Is(err, os.ErrNotExist) {
|
||||
t.Fatalf("stat prompt dir %s: %v", path, err)
|
||||
}
|
||||
}
|
||||
|
||||
// alwaysResumeAgent mimics Claude Code: it pins a deterministic session id, so
|
||||
// GetRestoreCommand can resume any session even with no captured agentSessionId
|
||||
// and no prompt.
|
||||
|
|
@ -520,7 +530,12 @@ func TestSpawn_ParksRowTerminatedWhenSeedDeleteFails(t *testing.T) {
|
|||
}
|
||||
func TestKill_TearsDownRuntimeAndWorkspace(t *testing.T) {
|
||||
m, st, rt, ws := newManager()
|
||||
dataDir := t.TempDir()
|
||||
m.dataDir = dataDir
|
||||
st.sessions["mer-1"] = mkLive("mer-1")
|
||||
if _, err := m.writeSystemPromptFile("mer-1", "system prompt"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
freed, err := m.Kill(ctx, "mer-1")
|
||||
if err != nil || !freed {
|
||||
t.Fatalf("freed=%v err=%v", freed, err)
|
||||
|
|
@ -528,6 +543,7 @@ func TestKill_TearsDownRuntimeAndWorkspace(t *testing.T) {
|
|||
if rt.destroyed != 1 || ws.destroyed != 1 {
|
||||
t.Fatal("kill should destroy runtime and workspace")
|
||||
}
|
||||
requireNoPromptDir(t, dataDir, "mer-1")
|
||||
}
|
||||
|
||||
// TestKill_TerminatesIncompleteHandle: a session whose runtime handle or
|
||||
|
|
@ -1356,12 +1372,17 @@ func TestRestore_RefusesIncompleteHandle(t *testing.T) {
|
|||
// outright by RollbackSpawn so the user never sees an orphan terminated row.
|
||||
func TestRollbackSpawn_DeletesSeedRow(t *testing.T) {
|
||||
m, st, _, _ := newManager()
|
||||
dataDir := t.TempDir()
|
||||
m.dataDir = dataDir
|
||||
// Seed row matches what CreateSession produces — no Metadata at all.
|
||||
st.sessions["mer-1"] = domain.SessionRecord{
|
||||
ID: "mer-1",
|
||||
ProjectID: "mer",
|
||||
Activity: domain.Activity{State: domain.ActivityIdle},
|
||||
}
|
||||
if _, err := m.writeSystemPromptFile("mer-1", "system prompt"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
deleted, killed, err := m.RollbackSpawn(ctx, "mer-1")
|
||||
if err != nil {
|
||||
t.Fatalf("rollback err = %v", err)
|
||||
|
|
@ -1372,6 +1393,7 @@ func TestRollbackSpawn_DeletesSeedRow(t *testing.T) {
|
|||
if _, present := st.sessions["mer-1"]; present {
|
||||
t.Fatal("seed row must be removed from the store, not left as terminated")
|
||||
}
|
||||
requireNoPromptDir(t, dataDir, "mer-1")
|
||||
}
|
||||
|
||||
// TestRollbackSpawn_FallsBackToKillForLiveRow asserts the no-resurrection
|
||||
|
|
@ -1405,13 +1427,14 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) {
|
|||
st.projects["mer"] = domain.ProjectRecord{ID: "mer", Config: testRoleAgents()}
|
||||
rt := &fakeRuntime{}
|
||||
ws := &fakeWorkspace{}
|
||||
dataDir := t.TempDir()
|
||||
notFound := func(name string) (string, error) {
|
||||
if name == "tmux" {
|
||||
return "/bin/tmux", nil
|
||||
}
|
||||
return "", fmt.Errorf("exec: %q: not found", name)
|
||||
}
|
||||
m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, LookPath: notFound})
|
||||
m := New(Deps{Runtime: rt, Agents: fakeAgents{}, Workspace: ws, Store: st, Messenger: &fakeMessenger{}, Lifecycle: &fakeLCM{store: st}, DataDir: dataDir, LookPath: notFound})
|
||||
|
||||
_, err := m.Spawn(ctx, ports.SpawnConfig{ProjectID: "mer", Kind: domain.KindWorker})
|
||||
if !errors.Is(err, ports.ErrAgentBinaryNotFound) {
|
||||
|
|
@ -1426,6 +1449,7 @@ func TestSpawn_RejectsMissingAgentBinary(t *testing.T) {
|
|||
if rec, present := st.sessions["mer-1"]; present {
|
||||
t.Fatalf("seed row must be deleted before a runtime handle is live, got %+v", rec)
|
||||
}
|
||||
requireNoPromptDir(t, dataDir, "mer-1")
|
||||
}
|
||||
|
||||
func TestSpawn_RejectsMissingTmuxBeforeSessionRow(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue