feat(chat): add attachments and improve agent conversations#343
Conversation
xxx7xxxx
requested review from
GatewayJ,
RussellLuo,
jialudev and
youngbeom-shin
as code owners
| } | ||
|
|
||
| func attachmentDownloadURL(object attachmentObject) string { | ||
| downloadURL := "/api/v1/attachments/" + url.PathEscape(object.ID) |
There was a problem hiding this comment.
Could we make this URL compatible with the application base path instead of making it origin-rooted?
In production Sandbox deployments, the Web UI and API are served under a prefix such as https://ai.space.opencsg.com/v1/sandboxes/csgship-xxxxxx. With the leading / here, the browser resolves the attachment URL to:
https://ai.space.opencsg.com/api/v1/attachments/...
instead of the expected:
https://ai.space.opencsg.com/v1/sandboxes/csgship-xxxxxx/api/v1/attachments/...
Unlike regular API requests, download_url and preview_url are assigned directly to <a href> and <img src>, so the frontend’s resolveRequestPath() normalization is bypassed. Attachment previews and downloads will therefore fail in production Sandbox deployments.
Please either return a deployment-relative URL, resolve this field against the application base path in the frontend, or otherwise include the configured prefix. A subpath deployment test would also help prevent regressions.
| if strings.TrimSpace(agentID) == "" { | ||
| return append([]im.MessageAttachment(nil), attachments...) | ||
| } | ||
| workspaceRoot, err := h.svc.WorkspaceRoot(agentID) |
There was a problem hiding this comment.
Could we hide local_workspace_dir for the Manager in the UI (and ideally reject it in the backend)? The Manager is automatically managed, and persistManagerAgent resets its RuntimeOptions to nil, so exposing this setting is misleading and non-durable. If it is set through the current update path, Codex can run in a different directory from the WorkspaceRoot used here, which would make these materialized attachments unavailable to it.
There was a problem hiding this comment.
Addressed both comments. Attachment URLs now respect the application base path for sandbox deployments. The Manager no longer exposes runtime workspace options, and backend updates to those managed options are rejected, while Codex workers retain the setting.
|
LGTM |
2 participants
Summary
Screenshots