Skip to content

Commit 579c933

Browse files
authored
Merge pull request #2532 from dgageot/board/file-path-resolution-issue-with-sub-agen-1bc0a174
Inherit user-attached files in sub-agent sessions
2 parents adcfafb + 22e4084 commit 579c933

10 files changed

Lines changed: 243 additions & 38 deletions

File tree

e2e/testdata/cassettes/TestA2AServer_MultiAgent.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ interactions:
88
proto_minor: 1
99
content_length: 0
1010
host: api.openai.com
11-
body: '{"input":[{"content":[{"text":"You are a multi-agent system, make sure to answer the user query in the most helpful way possible. You have access to these sub-agents:\nName: web | Description: \n\nIMPORTANT: You can ONLY transfer tasks to the agents listed above using their ID. The valid agent names are: web. You MUST NOT attempt to transfer to any other agent IDs - doing so will cause system errors.\n\nIf you are the best to answer the question according to your description, you can answer it.\n\nIf another agent is better for answering the question according to its description, call `transfer_task` function to transfer the question to that agent using the agent''s ID. When transferring, do not generate any text other than the function call.\n\n","type":"input_text"}],"role":"system"},{"content":[{"text":"You are a knowledgeable assistant that helps users with various tasks.\nBe helpful, accurate, and concise in your responses.\n","type":"input_text"}],"role":"system"},{"content":"Say hello.","role":"user"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"agent":{"description":"The name of the agent to transfer the task to.","type":"string"},"expected_output":{"description":"The expected output from the member (optional).","type":"string"},"task":{"description":"A clear and concise description of the task the member should achieve.","type":"string"}},"required":["agent","expected_output","task"],"type":"object"},"name":"transfer_task","description":"Use this function to transfer a task to the selected team member.\n You must provide a clear and concise description of the task the member should achieve AND the expected output.","type":"function"}],"stream":true}'
11+
body: '{"input":[{"content":[{"text":"You are a multi-agent system, make sure to answer the user query in the most helpful way possible. You have access to these sub-agents:\nName: web | Description: \n\nIMPORTANT: You can ONLY transfer tasks to the agents listed above using their ID. The valid agent names are: web. You MUST NOT attempt to transfer to any other agent IDs - doing so will cause system errors.\n\nIf you are the best to answer the question according to your description, you can answer it.\n\nIf another agent is better for answering the question according to its description, call `transfer_task` function to transfer the question to that agent using the agent''s ID. When transferring, do not generate any text other than the function call.\n\nWhen the task involves files, always include their absolute paths in the `task` description (never just bare filenames). Sub-agents start in a fresh session and do not see the conversation history or files attached by the user, so a non-absolute path may resolve to the wrong file or force the sub-agent to scan the filesystem.\n\n","type":"input_text"}],"role":"system"},{"content":[{"text":"You are a knowledgeable assistant that helps users with various tasks.\nBe helpful, accurate, and concise in your responses.\n","type":"input_text"}],"role":"system"},{"content":"Say hello.","role":"user"}],"model":"gpt-5-mini","reasoning":{"summary":"detailed"},"tools":[{"strict":true,"parameters":{"additionalProperties":false,"properties":{"agent":{"description":"The name of the agent to transfer the task to.","type":"string"},"expected_output":{"description":"The expected output from the member (optional).","type":"string"},"task":{"description":"A clear and concise description of the task the member should achieve.","type":"string"}},"required":["agent","expected_output","task"],"type":"object"},"name":"transfer_task","description":"Use this function to transfer a task to the selected team member.\n You must provide a clear and concise description of the task the member should achieve AND the expected output.","type":"function"}],"stream":true}'
1212
url: https://api.openai.com/v1/responses
1313
method: POST
1414
response:

e2e/testdata/cassettes/TestRuntime_MultiAgent_SessionReload.yaml

Lines changed: 4 additions & 4 deletions
Large diffs are not rendered by default.

pkg/app/app.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ func (a *App) SendFirstMessage() tea.Cmd {
139139
cmds := []tea.Cmd{
140140
func() tea.Msg {
141141
// Use the shared PrepareUserMessage function for consistent attachment handling
142-
userMsg := cli.PrepareUserMessage(context.Background(), a.runtime, *a.firstMessage, a.firstMessageAttach)
142+
userMsg, attachedPath := cli.PrepareUserMessage(context.Background(), a.runtime, *a.firstMessage, a.firstMessageAttach)
143+
// Inherit the attachment in any sub-session created by this turn.
144+
a.session.AddAttachedFile(attachedPath)
143145

144146
// If the message has multi-content (attachments), we need to handle it specially
145147
if len(userMsg.Message.MultiContent) > 0 {
@@ -301,7 +303,13 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
301303
switch {
302304
case att.FilePath != "":
303305
// File-reference attachment: read and classify from disk.
304-
a.processFileAttachment(ctx, att, &textBuilder, &binaryParts)
306+
// Only remember the path on the session when the file actually
307+
// exists as a regular file — we don't want sub-agents to inherit
308+
// dangling references to directories or missing paths. The editor
309+
// resolves @-mentions to absolute paths before this point.
310+
if a.processFileAttachment(ctx, att, &textBuilder, &binaryParts) {
311+
a.session.AddAttachedFile(att.FilePath)
312+
}
305313
case att.Content != "":
306314
// Inline content attachment (e.g. pasted text).
307315
a.processInlineAttachment(att, &textBuilder)
@@ -342,7 +350,14 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
342350

343351
// processFileAttachment reads a file from disk, classifies it, and either
344352
// appends its text content to textBuilder or adds a binary part to binaryParts.
345-
func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment, textBuilder *strings.Builder, binaryParts *[]chat.MessagePart) {
353+
// Returns true when the path resolved to a real, regular file that we attempted
354+
// to surface to the model — even if the content itself was rejected (too
355+
// large, unsupported MIME, transient read error, etc.). The boolean is meant
356+
// for callers that want to record the path on the session for later reuse by
357+
// sub-agents; we don't want those references to point at directories or
358+
// missing files, but we do want them to cover "the agent has bigger tools
359+
// than us" cases.
360+
func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment, textBuilder *strings.Builder, binaryParts *[]chat.MessagePart) bool {
346361
absPath := att.FilePath
347362

348363
fi, err := os.Stat(absPath)
@@ -358,20 +373,20 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
358373
}
359374
slog.Warn("skipping attachment", "path", absPath, "reason", reason)
360375
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: %s", att.Name, reason), ""))
361-
return
376+
return false
362377
}
363378

364379
if !fi.Mode().IsRegular() {
365380
slog.Warn("skipping attachment: not a regular file", "path", absPath, "mode", fi.Mode().String())
366381
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: not a regular file", att.Name), ""))
367-
return
382+
return false
368383
}
369384

370385
const maxAttachmentSize = 100 * 1024 * 1024 // 100MB
371386
if fi.Size() > maxAttachmentSize {
372387
slog.Warn("skipping attachment: file too large", "path", absPath, "size", fi.Size(), "max", maxAttachmentSize)
373388
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: file too large (max 100MB)", att.Name), ""))
374-
return
389+
return true
375390
}
376391

377392
mimeType := chat.DetectMimeType(absPath)
@@ -381,13 +396,13 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
381396
if fi.Size() > chat.MaxInlineFileSize {
382397
slog.Warn("skipping attachment: text file too large to inline", "path", absPath, "size", fi.Size(), "max", chat.MaxInlineFileSize)
383398
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: text file too large to inline (max 5MB)", att.Name), ""))
384-
return
399+
return true
385400
}
386401
content, err := chat.ReadFileForInline(absPath)
387402
if err != nil {
388403
slog.Warn("skipping attachment: failed to read file", "path", absPath, "error", err)
389404
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: failed to read file", att.Name), ""))
390-
return
405+
return true
391406
}
392407
textBuilder.WriteString("\n\n")
393408
textBuilder.WriteString(content)
@@ -400,14 +415,14 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
400415
if readErr != nil {
401416
slog.Warn("skipping attachment: failed to read image", "path", absPath, "error", readErr)
402417
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: failed to read image", att.Name), ""))
403-
return
418+
return true
404419
}
405420
resized, resizeErr := chat.ResizeImage(imgData, mimeType)
406421
if resizeErr != nil {
407422
// Don't bypass security checks - reject the file if resize failed
408423
slog.Warn("skipping attachment: image resize failed", "path", absPath, "error", resizeErr)
409424
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: %s", att.Name, resizeErr), ""))
410-
return
425+
return true
411426
}
412427
dataURL := fmt.Sprintf("data:%s;base64,%s", resized.MimeType, base64.StdEncoding.EncodeToString(resized.Data))
413428
*binaryParts = append(*binaryParts, chat.MessagePart{
@@ -435,6 +450,8 @@ func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment
435450
slog.Warn("skipping attachment: unsupported file type", "path", absPath, "mime_type", mimeType)
436451
a.sendEvent(ctx, runtime.Warning(fmt.Sprintf("Skipped attachment %s: unsupported file type", att.Name), ""))
437452
}
453+
454+
return true
438455
}
439456

440457
// sendEvent sends an event to the TUI, respecting context cancellation to

pkg/cli/runner.go

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
104104
return nil
105105
}
106106

107-
sess.AddMessage(PrepareUserMessage(ctx, rt, userInput, cfg.AttachmentPath))
107+
userMsg, attachedPath := PrepareUserMessage(ctx, rt, userInput, cfg.AttachmentPath)
108+
sess.AddMessage(userMsg)
109+
sess.AddAttachedFile(attachedPath)
108110

109111
if cfg.OutputJSON {
110112
for event := range rt.RunStream(ctx, sess) {
@@ -320,8 +322,11 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
320322
// - userInput: the raw user input (may contain /commands and /attach directives)
321323
// - globalAttachPath: attachment path from --attach flag (can be empty)
322324
//
323-
// Returns the prepared session.Message ready to be added to the session.
324-
func PrepareUserMessage(ctx context.Context, rt runtime.Runtime, userInput, globalAttachPath string) *session.Message {
325+
// Returns the prepared session.Message ready to be added to the session, plus
326+
// the absolute path of the file that was actually attached (empty when no
327+
// attachment was used). Callers should pass that path to
328+
// session.Session.AddAttachedFile so sub-agents inherit the file context.
329+
func PrepareUserMessage(ctx context.Context, rt runtime.Runtime, userInput, globalAttachPath string) (*session.Message, string) {
325330
// Resolve any /command to its prompt text
326331
resolvedContent := runtime.ResolveCommand(ctx, rt, userInput)
327332

@@ -391,22 +396,35 @@ func ParseAttachCommand(userInput string) (messageText, attachPath string) {
391396
// CreateUserMessageWithAttachment creates a user message with optional file attachment.
392397
// Text files are inlined directly as text content for cross-provider compatibility.
393398
// Binary files (images, PDFs) are stored as file references for provider-specific upload.
394-
func CreateUserMessageWithAttachment(userContent, attachmentPath string) *session.Message {
399+
//
400+
// Returns the prepared session.Message and the absolute path of the file that
401+
// was actually attached. The returned path is empty when no attachment was
402+
// produced (no path supplied, file unreadable, type unsupported, file too
403+
// large to inline, etc.). Callers should record successful attachments via
404+
// session.Session.AddAttachedFile so sub-agents inherit the file context.
405+
func CreateUserMessageWithAttachment(userContent, attachmentPath string) (*session.Message, string) {
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) {
411+
return session.UserMessage(userContent), ""
412+
}
413+
395414
if attachmentPath == "" {
396-
return session.UserMessage(userContent)
415+
return noAttachment()
397416
}
398417

399-
// Validate file exists
400418
absPath, err := filepath.Abs(attachmentPath)
401419
if err != nil {
402420
slog.Warn("Failed to get absolute path for attachment", "path", attachmentPath, "error", err)
403-
return session.UserMessage(userContent)
421+
return noAttachment()
404422
}
405423

406424
fi, err := os.Stat(absPath)
407425
if err != nil {
408426
slog.Warn("Attachment file not accessible", "path", absPath, "error", err)
409-
return session.UserMessage(userContent)
427+
return noAttachment()
410428
}
411429

412430
// Ensure we have some text content when attaching a file
@@ -424,12 +442,12 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) *sessio
424442
// Text files are inlined directly as text content.
425443
if fi.Size() > chat.MaxInlineFileSize {
426444
slog.Warn("Attachment text file too large to inline", "path", absPath, "size", fi.Size())
427-
return session.UserMessage(userContent)
445+
return noAttachment()
428446
}
429447
content, err := chat.ReadFileForInline(absPath)
430448
if err != nil {
431449
slog.Warn("Failed to read attachment file", "path", absPath, "error", err)
432-
return session.UserMessage(userContent)
450+
return noAttachment()
433451
}
434452
multiContent = append(multiContent, chat.MessagePart{
435453
Type: chat.MessagePartTypeText,
@@ -441,7 +459,7 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) *sessio
441459
mimeType := chat.DetectMimeType(absPath)
442460
if !chat.IsSupportedMimeType(mimeType) {
443461
slog.Warn("Unsupported attachment file type", "path", absPath, "mime_type", mimeType)
444-
return session.UserMessage(userContent)
462+
return noAttachment()
445463
}
446464
if chat.IsImageMimeType(mimeType) {
447465
// Read, resize if needed, and inline as base64 data URL.
@@ -450,12 +468,12 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) *sessio
450468
imgData, readErr := os.ReadFile(absPath)
451469
if readErr != nil {
452470
slog.Warn("Failed to read image attachment", "path", absPath, "error", readErr)
453-
return session.UserMessage(userContent)
471+
return noAttachment()
454472
}
455473
resized, resizeErr := chat.ResizeImage(imgData, mimeType)
456474
if resizeErr != nil {
457475
slog.Warn("Image resize failed for attachment", "path", absPath, "error", resizeErr)
458-
return session.UserMessage(userContent)
476+
return noAttachment()
459477
}
460478
dataURL := fmt.Sprintf("data:%s;base64,%s", resized.MimeType, base64.StdEncoding.EncodeToString(resized.Data))
461479
multiContent = append(multiContent, chat.MessagePart{
@@ -477,5 +495,5 @@ func CreateUserMessageWithAttachment(userContent, attachmentPath string) *sessio
477495
}
478496
}
479497

480-
return session.UserMessage(textContent, multiContent...)
498+
return session.UserMessage(textContent, multiContent...), absPath
481499
}

pkg/runtime/agent_delegation.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,26 @@ func validateAgentInList(currentAgent, targetAgent, action, listDesc string, age
4949
}
5050

5151
// buildTaskSystemMessage constructs the system message for a delegated task.
52-
func buildTaskSystemMessage(task, expectedOutput string) string {
53-
msg := "You are a member of a team of agents. Your goal is to complete the following task:"
54-
msg += fmt.Sprintf("\n\n<task>\n%s\n</task>", task)
52+
// attachedFiles, when non-empty, lists absolute paths of files the user
53+
// attached to the parent conversation; they are surfaced to the sub-agent so
54+
// it can use them directly without scanning the workspace or guessing from a
55+
// bare filename.
56+
func buildTaskSystemMessage(task, expectedOutput string, attachedFiles []string) string {
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)
5560
if expectedOutput != "" {
56-
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)
5762
}
58-
return msg
63+
if len(attachedFiles) > 0 {
64+
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>")
65+
for _, p := range attachedFiles {
66+
fmt.Fprintf(&b, "\n- %s", p)
67+
}
68+
b.WriteString("\n</attached_files>")
69+
}
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()
5972
}
6073

6174
// SubSessionConfig describes how to build and run a child session.
@@ -95,9 +108,16 @@ type SubSessionConfig struct {
95108
// session. It consolidates the session options that were previously duplicated
96109
// across handleTaskTransfer and RunAgent.
97110
func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *agent.Agent) *session.Session {
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).
116+
attachedFiles := parent.AttachedFilesSnapshot()
117+
98118
sysMsg := cfg.SystemMessage
99119
if sysMsg == "" {
100-
sysMsg = buildTaskSystemMessage(cfg.Task, cfg.ExpectedOutput)
120+
sysMsg = buildTaskSystemMessage(cfg.Task, cfg.ExpectedOutput, attachedFiles)
101121
}
102122

103123
userMsg := cfg.ImplicitUserMessage
@@ -115,6 +135,7 @@ func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *ag
115135
session.WithToolsApproved(cfg.ToolsApproved),
116136
session.WithSendUserMessage(false),
117137
session.WithParentID(parent.ID),
138+
session.WithAttachedFiles(attachedFiles),
118139
}
119140
if cfg.PinAgent {
120141
opts = append(opts, session.WithAgentName(cfg.AgentName))

0 commit comments

Comments
 (0)