Skip to content

Commit 86641e4

Browse files
committed
Tidy up the AttachedFiles plumbing
Pure cleanup, no behaviour change: * buildTaskSystemMessage now uses a single strings.Builder throughout instead of mixing string concatenation with a one-off builder for the attached_files block. * CreateUserMessageWithAttachment introduces a noAttachment closure so the seven best-effort fallback paths share one return statement instead of repeating session.UserMessage(userContent), "" everywhere. * branch.go drops a redundant cloneStringSlice on the result of AttachedFilesSnapshot, which already returns an independent copy. * The over-eager three-line comments at the AddAttachedFile call sites and on top of newSubSession are replaced with a single line each that states the contract. Assisted-By: docker-agent
1 parent 359b8f6 commit 86641e4

4 files changed

Lines changed: 33 additions & 30 deletions

File tree

pkg/app/app.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,7 @@ func (a *App) SendFirstMessage() tea.Cmd {
140140
func() tea.Msg {
141141
// Use the shared PrepareUserMessage function for consistent attachment handling
142142
userMsg, attachedPath := cli.PrepareUserMessage(context.Background(), a.runtime, *a.firstMessage, a.firstMessageAttach)
143-
// Record the attached file on the session so sub-agents created via
144-
// task transfer can reference it through their inherited
145-
// AttachedFiles list.
143+
// Inherit the attachment in any sub-session created by this turn.
146144
a.session.AddAttachedFile(attachedPath)
147145

148146
// If the message has multi-content (attachments), we need to handle it specially
@@ -304,9 +302,9 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
304302
for _, att := range attachments {
305303
switch {
306304
case att.FilePath != "":
307-
// Record the attached path on the session so that sub-agents
308-
// created via task transfer inherit it. The editor already
309-
// resolves @-mentions to absolute paths before this point.
305+
// The editor resolves @-mentions to absolute paths before this
306+
// point; remember them so sub-agents created via task transfer
307+
// inherit the same file references.
310308
a.session.AddAttachedFile(att.FilePath)
311309
// File-reference attachment: read and classify from disk.
312310
a.processFileAttachment(ctx, att, &textBuilder, &binaryParts)

pkg/cli/runner.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -403,21 +403,28 @@ func ParseAttachCommand(userInput string) (messageText, attachPath string) {
403403
// large to inline, etc.). Callers should record successful attachments via
404404
// session.Session.AddAttachedFile so sub-agents inherit the file context.
405405
func CreateUserMessageWithAttachment(userContent, attachmentPath string) (*session.Message, string) {
406-
if attachmentPath == "" {
406+
// noAttachment returns the message without any attachment. It's used both
407+
// when the caller didn't supply a path and as the fallback for every
408+
// best-effort failure below (unreadable file, unsupported type, etc.) so
409+
// the user always gets at least a plain text message.
410+
noAttachment := func() (*session.Message, string) {
407411
return session.UserMessage(userContent), ""
408412
}
409413

410-
// Validate file exists
414+
if attachmentPath == "" {
415+
return noAttachment()
416+
}
417+
411418
absPath, err := filepath.Abs(attachmentPath)
412419
if err != nil {
413420
slog.Warn("Failed to get absolute path for attachment", "path", attachmentPath, "error", err)
414-
return session.UserMessage(userContent), ""
421+
return noAttachment()
415422
}
416423

417424
fi, err := os.Stat(absPath)
418425
if err != nil {
419426
slog.Warn("Attachment file not accessible", "path", absPath, "error", err)
420-
return session.UserMessage(userContent), ""
427+
return noAttachment()
421428
}
422429

423430
// Ensure we have some text content when attaching a file
@@ -435,12 +442,12 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) (*sessi
435442
// Text files are inlined directly as text content.
436443
if fi.Size() > chat.MaxInlineFileSize {
437444
slog.Warn("Attachment text file too large to inline", "path", absPath, "size", fi.Size())
438-
return session.UserMessage(userContent), ""
445+
return noAttachment()
439446
}
440447
content, err := chat.ReadFileForInline(absPath)
441448
if err != nil {
442449
slog.Warn("Failed to read attachment file", "path", absPath, "error", err)
443-
return session.UserMessage(userContent), ""
450+
return noAttachment()
444451
}
445452
multiContent = append(multiContent, chat.MessagePart{
446453
Type: chat.MessagePartTypeText,
@@ -452,7 +459,7 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) (*sessi
452459
mimeType := chat.DetectMimeType(absPath)
453460
if !chat.IsSupportedMimeType(mimeType) {
454461
slog.Warn("Unsupported attachment file type", "path", absPath, "mime_type", mimeType)
455-
return session.UserMessage(userContent), ""
462+
return noAttachment()
456463
}
457464
if chat.IsImageMimeType(mimeType) {
458465
// Read, resize if needed, and inline as base64 data URL.
@@ -461,12 +468,12 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) (*sessi
461468
imgData, readErr := os.ReadFile(absPath)
462469
if readErr != nil {
463470
slog.Warn("Failed to read image attachment", "path", absPath, "error", readErr)
464-
return session.UserMessage(userContent), ""
471+
return noAttachment()
465472
}
466473
resized, resizeErr := chat.ResizeImage(imgData, mimeType)
467474
if resizeErr != nil {
468475
slog.Warn("Image resize failed for attachment", "path", absPath, "error", resizeErr)
469-
return session.UserMessage(userContent), ""
476+
return noAttachment()
470477
}
471478
dataURL := fmt.Sprintf("data:%s;base64,%s", resized.MimeType, base64.StdEncoding.EncodeToString(resized.Data))
472479
multiContent = append(multiContent, chat.MessagePart{

pkg/runtime/agent_delegation.go

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,23 +54,21 @@ func validateAgentInList(currentAgent, targetAgent, action, listDesc string, age
5454
// it can use them directly without scanning the workspace or guessing from a
5555
// bare filename.
5656
func buildTaskSystemMessage(task, expectedOutput string, attachedFiles []string) string {
57-
msg := "You are a member of a team of agents. Your goal is to complete the following task:"
58-
msg += fmt.Sprintf("\n\n<task>\n%s\n</task>", task)
57+
var b strings.Builder
58+
b.WriteString("You are a member of a team of agents. Your goal is to complete the following task:")
59+
fmt.Fprintf(&b, "\n\n<task>\n%s\n</task>", task)
5960
if expectedOutput != "" {
60-
msg += fmt.Sprintf("\n\n<expected_output>\n%s\n</expected_output>", expectedOutput)
61+
fmt.Fprintf(&b, "\n\n<expected_output>\n%s\n</expected_output>", expectedOutput)
6162
}
6263
if len(attachedFiles) > 0 {
63-
var b strings.Builder
6464
b.WriteString("\n\nThe user attached these files in the original conversation. They are available for you to read at these absolute paths; prefer them over any bare filenames mentioned in <task>:\n<attached_files>")
6565
for _, p := range attachedFiles {
66-
b.WriteString("\n- ")
67-
b.WriteString(p)
66+
fmt.Fprintf(&b, "\n- %s", p)
6867
}
6968
b.WriteString("\n</attached_files>")
70-
msg += b.String()
7169
}
72-
msg += "\n\nIf the task references files, treat any absolute paths in <task> as authoritative and use them as-is. If a referenced file is given by name only (e.g. \"foo.go\"), do not guess: search the workspace or ask the calling agent for the absolute path before reading or modifying the file."
73-
return msg
70+
b.WriteString("\n\nIf the task references files, treat any absolute paths in <task> as authoritative and use them as-is. If a referenced file is given by name only (e.g. \"foo.go\"), do not guess: search the workspace or ask the calling agent for the absolute path before reading or modifying the file.")
71+
return b.String()
7472
}
7573

7674
// SubSessionConfig describes how to build and run a child session.
@@ -110,11 +108,11 @@ type SubSessionConfig struct {
110108
// session. It consolidates the session options that were previously duplicated
111109
// across handleTaskTransfer and RunAgent.
112110
func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *agent.Agent) *session.Session {
113-
// Inherit the parent's attached files so that delegated agents can read
114-
// the same files the user attached to the original conversation, without
115-
// having to scan the workspace or guess from a bare filename. This works
116-
// recursively: sub-sub-sessions also inherit because we copy the parent
117-
// session's AttachedFiles, and the child session is itself populated.
111+
// Sub-agents start in a fresh session, so they don't see the user's
112+
// original messages or attached files. Snapshot the parent's attached
113+
// files once and propagate them both to the system prompt (so the agent
114+
// is told about them) and to the child session (so further nested
115+
// transfers keep inheriting them).
118116
attachedFiles := parent.AttachedFilesSnapshot()
119117

120118
sysMsg := cfg.SystemMessage

pkg/session/branch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func copySessionMetadata(dst, src *Session, title string) {
9090
dst.Permissions = clonePermissionsConfig(src.Permissions)
9191
dst.AgentModelOverrides = cloneStringMap(src.AgentModelOverrides)
9292
dst.CustomModelsUsed = cloneStringSlice(src.CustomModelsUsed)
93-
dst.AttachedFiles = cloneStringSlice(src.AttachedFilesSnapshot())
93+
dst.AttachedFiles = src.AttachedFilesSnapshot()
9494
}
9595

9696
// generateBranchTitle creates a title for a branched session based on the parent title.

0 commit comments

Comments
 (0)