Repository navigation
Support file attachments in Sessions - #1630
Merged
Merged
Conversation
github-actions
Bot
added
kind/feature
gjkim42
marked this pull request as ready for review
August 10, 2026 10:34
Collaborator
Author
|
/kelos claude-review |
Contributor
|
🤖 Kelos Task Status Task |
Contributor
|
🤖 Kelos Claude Reviewer Agent @gjkim42 Review SummaryVerdict: APPROVE Findings Overview
FindingsCorrectness
Tests
Suggestions (optional)
Key takeaways
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/e2e/session_test.go">
<violation number="1" location="test/e2e/session_test.go:1261">
P3: The command context is only 3m, but this helper can poll for ~6.5m (30s connect + two 3m Eventually blocks). If the "Attached" matcher uses much of its 3m budget the context expires and kills the child process while the reply matcher is still polling, yielding a confusing failure even when the turn succeeds. Extend the command context to cover the whole wait or shrink per-matcher budgets so the poll budget fits inside the command lifetime.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| func runTerminalAttachmentTurn(namespace, name, path, prompt string, outputMatcher gomegatypes.GomegaMatcher) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) |
There was a problem hiding this comment.
P3: The command context is only 3m, but this helper can poll for ~6.5m (30s connect + two 3m Eventually blocks). If the "Attached" matcher uses much of its 3m budget the context expires and kills the child process while the reply matcher is still polling, yielding a confusing failure even when the turn succeeds. Extend the command context to cover the whole wait or shrink per-matcher budgets so the poll budget fits inside the command lifetime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/e2e/session_test.go, line 1261:
<comment>The command context is only 3m, but this helper can poll for ~6.5m (30s connect + two 3m Eventually blocks). If the "Attached" matcher uses much of its 3m budget the context expires and kills the child process while the reply matcher is still polling, yielding a confusing failure even when the turn succeeds. Extend the command context to cover the whole wait or shrink per-matcher budgets so the poll budget fits inside the command lifetime.</comment>
<file context>
@@ -1212,6 +1257,29 @@ func runTerminalTurn(namespace, name, prompt string, outputMatcher gomegatypes.G
}
+func runTerminalAttachmentTurn(namespace, name, path, prompt string, outputMatcher gomegatypes.GomegaMatcher) {
+ ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+ defer cancel()
+ command := exec.CommandContext(ctx, framework.KelosBin(), "session", "connect", name, "-n", namespace)
</file context>
Suggested change
| ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) | |
| ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) |
gjkim42
force-pushed
the
agent/session-attachments
branch
from
August 10, 2026 10:52
d68c69b to
aec44d0
Compare
gjkim42
enabled auto-merge
August 10, 2026 10:54
There was a problem hiding this comment.
13 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/cli/session_terminal_tui.go">
<violation number="1" location="internal/cli/session_terminal_tui.go:475">
P3: Typing `/send` in the interactive TUI with no staged files sends an empty message to the agent. `submitInput` handles the `message` request type by attaching `AttachmentIDs` and encoding it, but — unlike the plain-terminal path in `session_terminal.go`, which guards with `if request.Text == "" && len(pendingAttachments) == 0 { continue }` — the TUI has no such guard. Since `/send` resolves to `{Type: "message"}` with empty text, a session connected via the TUI will transmit a blank user message when nothing is attached. Consider adding the same empty-message guard used by the plain terminal before encoding. (Minor, since `/send` is documented only for the plain terminal.)</violation>
</file>
<file name="cmd/kelos-session-runtime/main.go">
<violation number="1" location="cmd/kelos-session-runtime/main.go:78">
P3: New attachment CLI protocol has no automated coverage; add put/get tests covering metadata framing, binary payloads, argument validation, and store failures so API downloads cannot silently regress.
(Based on your team's feedback about adding tests for new behavior.)</violation>
</file>
<file name="internal/sessionserver/web/styles.css">
<violation number="1" location="internal/sessionserver/web/styles.css:277">
P3: The new .attach-button (30×36px) sits beside .send-button in the composer, but the mobile media query bumps .send-button to 48×48px and leaves .attach-button at its small size, so the two are misaligned and the attach control becomes a sub-44px touch target on touch devices. Add a matching rule inside that media query (e.g. width/height 48px) alongside the existing .send-button resize.</violation>
</file>
<file name="internal/sessionattachment/client.go">
<violation number="1" location="internal/sessionattachment/client.go:26">
P2: Add direct client tests around the exec transport or an injectable stream seam; current attachment endpoint tests use a fake, leaving this new runtime wire protocol unverified against regressions.
(Based on your team's feedback about adding automated tests for new behavior.)</violation>
</file>
<file name="internal/sessionserver/web/app.js">
<violation number="1" location="internal/sessionserver/web/app.js:3294">
P2: Pressing Enter while an attachment upload is in flight can send the same message twice. `submitComposer` is now async (multipart uploads via `await Promise.all(...)`), but the Enter keydown handler calls `elements.composer.requestSubmit()` unconditionally, which re-invokes `submitComposer`. During the upload window `state.sendingMessage` is true (and only the attach button + send button are disabled), so a second Enter starts another concurrent upload and emits a duplicate `message` over the socket, producing duplicate agent turns and duplicate stored attachments. Add an early return when `state.sendingMessage` is already true (or guard the Enter handler) so re-submission is blocked while a send is in progress.</violation>
<violation number="2" location="internal/sessionserver/web/app.js:3298">
P2: Failed or disconnected sends leave already-uploaded files permanently consuming this Session’s attachment quota, so retries can eventually make valid attachments impossible. Make upload-and-message submission transactional, or add cleanup for uploaded IDs whenever handoff cannot complete.</violation>
</file>
<file name="internal/sessionruntime/server.go">
<violation number="1" location="internal/sessionruntime/server.go:165">
P2: Whitespace `StateDir` lets runtime start with nil attachment storage, so every attachment submission is rejected instead of failing invalid configuration at startup. Propagate or handle `NewAttachmentStore`'s error before serving requests.</violation>
<violation number="2" location="internal/sessionruntime/server.go:717">
P2: The same attachment IDs are resolved twice per turn: once eagerly in submitMessage (only to validate, then the resolved paths are discarded, keeping just Attachment metadata) and again in runTurn, which re-opens and re-validates every attachment file from disk. This duplicates I/O and introduces a window where the EventUserMessage is durably recorded in submitMessage but runTurn's re-resolution can still fail (e.g. if an attachment is removed between the two), producing a failed turn with no direct rejection feedback to the submitter. Consider carrying the already-resolved attachments on the turnRequest for freshly submitted turns, and resolving from IDs only for journal-recovered turns (where the resolved path is not persisted), so validation happens once.</violation>
</file>
<file name="internal/cli/session.go">
<violation number="1" location="internal/cli/session.go:567">
P2: Plain `kelos session connect` accepts more than 8 `/attach` commands, then `/send` is rejected by runtime and clears all pending attachment IDs, so users cannot send the allowed subset without reattaching. Keep the plain-terminal pending list capped at `MaxAttachmentsPerMessage`, matching the TUI path.</violation>
</file>
<file name="internal/sessionserver/server.go">
<violation number="1" location="internal/sessionserver/server.go:189">
P3: Oversized multipart uploads return 400/502 instead of 413 because `http.MaxBytesError` is not handled after `MaxBytesReader`; map that error (including the transfer-wrapped form) to `StatusRequestEntityTooLarge`.</violation>
<violation number="2" location="internal/sessionserver/server.go:842">
P2: The `attachmentUploadStatus` catch-all maps every upload failure whose message contains "storing session attachment failed" to 400 Bad Request. Because the runtime always prefixes `Storing Session attachment failed:` on *any* `store.Put` error, this branch catches genuine server-side storage failures too (e.g. "creating Session attachment directory", "writing Session attachment data", "locking Session attachment store", media detection) and reports them to the client as 400. Clients treating 4xx as "retry/fix your request" will be misled when the real cause is a server-side disk/I/O problem, and the loss of the underlying status is confusing for ops debug. Prefer mapping the server-side failures to 5xx (e.g. 500/502) and reserving 4xx for the explicitly client-caused cases (invalid name, too-large, quota).</violation>
</file>
<file name="internal/cli/session_terminal.go">
<violation number="1" location="internal/cli/session_terminal.go:313">
P3: Plain-terminal users cannot discover how to submit an attachment without text because this command list omits `/send`; include it alongside `/attach`.</violation>
</file>
<file name="internal/sessionserver/server_test.go">
<violation number="1" location="internal/sessionserver/server_test.go:49">
P3: The fake Download discards the requested attachment id (4th parameter), and Upload returns a fixed attachment regardless of the file route, so the download flow's id routing/parameterization is never actually exercised by the test. A bug where downloadSessionAttachment passes the wrong identifier (e.g. the session/pod name instead of the requested id) would pass this green. Capture the id (and pod/namespace) on the fake and assert it matches the request path so the download path is truly covered.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,115 @@ | |||
| package sessionattachment | |||
There was a problem hiding this comment.
P2: Add direct client tests around the exec transport or an injectable stream seam; current attachment endpoint tests use a fake, leaving this new runtime wire protocol unverified against regressions.
(Based on your team's feedback about adding automated tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionattachment/client.go, line 26:
<comment>Add direct client tests around the exec transport or an injectable stream seam; current attachment endpoint tests use a fake, leaving this new runtime wire protocol unverified against regressions.
(Based on your team's feedback about adding automated tests for new behavior.) </comment>
<file context>
@@ -0,0 +1,115 @@
+const runtimeExecutable = "/kelos/bin/kelos-session-runtime"
+
+// Client transfers attachments through the Session Pod exec subresource.
+type Client struct {
+ clientset kubernetes.Interface
+ restConfig *rest.Config
</file context>
| updateComposerAction(); | ||
| elements.attachFiles.disabled = true; | ||
| try { | ||
| const attachments = await Promise.all(files.map(file => uploadAttachment(session, file))); |
There was a problem hiding this comment.
P2: Failed or disconnected sends leave already-uploaded files permanently consuming this Session’s attachment quota, so retries can eventually make valid attachments impossible. Make upload-and-message submission transactional, or add cleanup for uploaded IDs whenever handoff cannot complete.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionserver/web/app.js, line 3297:
<comment>Failed or disconnected sends leave already-uploaded files permanently consuming this Session’s attachment quota, so retries can eventually make valid attachments impossible. Make upload-and-message submission transactional, or add cleanup for uploaded IDs whenever handoff cannot complete.</comment>
<file context>
@@ -3168,21 +3213,131 @@ function scheduleBottomAnchor() {
+ updateComposerAction();
+ elements.attachFiles.disabled = true;
+ try {
+ const attachments = await Promise.all(files.map(file => uploadAttachment(session, file)));
+ if (!state.selected || sessionViewKey(state.selected) !== sessionViewKey(session) || !state.socket || state.socket.readyState !== WebSocket.OPEN) {
+ throw new Error('Session disconnected before the message could be sent');
</file context>
| } | ||
| if config.StateDir != "" { | ||
| server.activityMarkerPath = filepath.Join(config.StateDir, activityPublishedFile) | ||
| server.attachmentStore, _ = NewAttachmentStore(config.StateDir) |
There was a problem hiding this comment.
P2: Whitespace StateDir lets runtime start with nil attachment storage, so every attachment submission is rejected instead of failing invalid configuration at startup. Propagate or handle NewAttachmentStore's error before serving requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionruntime/server.go, line 165:
<comment>Whitespace `StateDir` lets runtime start with nil attachment storage, so every attachment submission is rejected instead of failing invalid configuration at startup. Propagate or handle `NewAttachmentStore`'s error before serving requests.</comment>
<file context>
@@ -160,6 +162,7 @@ func NewServer(config Config, journal *Journal, provider Provider) *Server {
}
if config.StateDir != "" {
server.activityMarkerPath = filepath.Join(config.StateDir, activityPublishedFile)
+ server.attachmentStore, _ = NewAttachmentStore(config.StateDir)
}
if provider, ok := provider.(runtimeStatusProvider); ok {
</file context>
| if request.Type == "subscribe" { | ||
| continue | ||
| } | ||
| if request.Type == sessionTerminalRequestAttachment { |
There was a problem hiding this comment.
P2: Plain kelos session connect accepts more than 8 /attach commands, then /send is rejected by runtime and clears all pending attachment IDs, so users cannot send the allowed subset without reattaching. Keep the plain-terminal pending list capped at MaxAttachmentsPerMessage, matching the TUI path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cli/session.go, line 567:
<comment>Plain `kelos session connect` accepts more than 8 `/attach` commands, then `/send` is rejected by runtime and clears all pending attachment IDs, so users cannot send the allowed subset without reattaching. Keep the plain-terminal pending list capped at `MaxAttachmentsPerMessage`, matching the TUI path.</comment>
<file context>
@@ -540,6 +564,24 @@ func connectSessionWithDependencies(
if request.Type == "subscribe" {
continue
}
+ if request.Type == sessionTerminalRequestAttachment {
+ var event sessionruntime.Event
+ if dependencies.uploadAttachment == nil {
</file context>
| const session = state.selected; | ||
| const files = [...currentAttachmentFiles()]; | ||
| if (!text && files.length === 0) return; | ||
| state.sendingMessage = true; |
There was a problem hiding this comment.
P2: Pressing Enter while an attachment upload is in flight can send the same message twice. submitComposer is now async (multipart uploads via await Promise.all(...)), but the Enter keydown handler calls elements.composer.requestSubmit() unconditionally, which re-invokes submitComposer. During the upload window state.sendingMessage is true (and only the attach button + send button are disabled), so a second Enter starts another concurrent upload and emits a duplicate message over the socket, producing duplicate agent turns and duplicate stored attachments. Add an early return when state.sendingMessage is already true (or guard the Enter handler) so re-submission is blocked while a send is in progress.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionserver/web/app.js, line 3293:
<comment>Pressing Enter while an attachment upload is in flight can send the same message twice. `submitComposer` is now async (multipart uploads via `await Promise.all(...)`), but the Enter keydown handler calls `elements.composer.requestSubmit()` unconditionally, which re-invokes `submitComposer`. During the upload window `state.sendingMessage` is true (and only the attach button + send button are disabled), so a second Enter starts another concurrent upload and emits a duplicate `message` over the socket, producing duplicate agent turns and duplicate stored attachments. Add an early return when `state.sendingMessage` is already true (or guard the Enter handler) so re-submission is blocked while a send is in progress.</comment>
<file context>
@@ -3168,21 +3213,131 @@ function scheduleBottomAnchor() {
+ const session = state.selected;
+ const files = [...currentAttachmentFiles()];
+ if (!text && files.length === 0) return;
+ state.sendingMessage = true;
+ updateComposerAction();
+ elements.attachFiles.disabled = true;
</file context>
| } | ||
| } | ||
|
|
||
| func runAttachment() { |
There was a problem hiding this comment.
P3: New attachment CLI protocol has no automated coverage; add put/get tests covering metadata framing, binary payloads, argument validation, and store failures so API downloads cannot silently regress.
(Based on your team's feedback about adding tests for new behavior.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmd/kelos-session-runtime/main.go, line 78:
<comment>New attachment CLI protocol has no automated coverage; add put/get tests covering metadata framing, binary payloads, argument validation, and store failures so API downloads cannot silently regress.
(Based on your team's feedback about adding tests for new behavior.) </comment>
<file context>
@@ -66,12 +67,67 @@ func main() {
}
}
+func runAttachment() {
+ if len(os.Args) < 3 {
+ fmt.Fprintln(os.Stderr, "Usage: kelos-session-runtime attachment <put|get>")
</file context>
| .composer:focus-within { border-color: #85a493; box-shadow: 0 12px 38px rgba(28,45,36,.1), 0 0 0 3px rgba(57,112,84,.08); } | ||
| .composer textarea { flex: 1; max-height: 160px; resize: none; border: 0; outline: 0; padding: 8px 0 6px; background: transparent; color: var(--ink); font-size: 14px; line-height: 1.45; } | ||
| .composer textarea::placeholder { color: #99a19c; } | ||
| .attach-button { flex: none; width: 30px; height: 36px; padding: 0; border: 0; background: transparent; color: var(--accent-2); font-size: 22px; cursor: pointer; } |
There was a problem hiding this comment.
P3: The new .attach-button (30×36px) sits beside .send-button in the composer, but the mobile media query bumps .send-button to 48×48px and leaves .attach-button at its small size, so the two are misaligned and the attach control becomes a sub-44px touch target on touch devices. Add a matching rule inside that media query (e.g. width/height 48px) alongside the existing .send-button resize.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionserver/web/styles.css, line 277:
<comment>The new .attach-button (30×36px) sits beside .send-button in the composer, but the mobile media query bumps .send-button to 48×48px and leaves .attach-button at its small size, so the two are misaligned and the attach control becomes a sub-44px touch target on touch devices. Add a matching rule inside that media query (e.g. width/height 48px) alongside the existing .send-button resize.</comment>
<file context>
@@ -262,10 +266,16 @@ button { color: inherit; }
.composer:focus-within { border-color: #85a493; box-shadow: 0 12px 38px rgba(28,45,36,.1), 0 0 0 3px rgba(57,112,84,.08); }
.composer textarea { flex: 1; max-height: 160px; resize: none; border: 0; outline: 0; padding: 8px 0 6px; background: transparent; color: var(--ink); font-size: 14px; line-height: 1.45; }
.composer textarea::placeholder { color: #99a19c; }
+.attach-button { flex: none; width: 30px; height: 36px; padding: 0; border: 0; background: transparent; color: var(--accent-2); font-size: 22px; cursor: pointer; }
+.attach-button:disabled { color: var(--faint); cursor: default; }
.send-button { flex: 0 0 auto; width: 36px; height: 36px; border: 0; border-radius: 11px; background: var(--accent); color: white; font-size: 20px; font-weight: 500; cursor: pointer; }
</file context>
| return nil, errors.New("Kubernetes clients and REST config are required") | ||
| } | ||
| attachmentClient, err := sessionattachment.New(config.RESTConfig) | ||
| if err != nil { |
There was a problem hiding this comment.
P3: Oversized multipart uploads return 400/502 instead of 413 because http.MaxBytesError is not handled after MaxBytesReader; map that error (including the transfer-wrapped form) to StatusRequestEntityTooLarge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionserver/server.go, line 189:
<comment>Oversized multipart uploads return 400/502 instead of 413 because `http.MaxBytesError` is not handled after `MaxBytesReader`; map that error (including the transfer-wrapped form) to `StatusRequestEntityTooLarge`.</comment>
<file context>
@@ -176,6 +185,10 @@ func New(config Config) (*Server, error) {
return nil, errors.New("Kubernetes clients and REST config are required")
}
+ attachmentClient, err := sessionattachment.New(config.RESTConfig)
+ if err != nil {
+ return nil, err
+ }
</file context>
| write("\n%s\n", formatter.muted("Earlier Session history is available. Use /history to load the previous page.")) | ||
| } | ||
| write("\n%s\n\n", formatter.muted("Connected. Type a message, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit.")) | ||
| write("\n%s\n\n", formatter.muted("Connected. Type a message, /attach PATH, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit.")) |
There was a problem hiding this comment.
P3: Plain-terminal users cannot discover how to submit an attachment without text because this command list omits /send; include it alongside /attach.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cli/session_terminal.go, line 313:
<comment>Plain-terminal users cannot discover how to submit an attachment without text because this command list omits `/send`; include it alongside `/attach`.</comment>
<file context>
@@ -306,7 +310,7 @@ func runSessionPlainTerminalWithWidth(ctx context.Context, input io.Reader, outp
write("\n%s\n", formatter.muted("Earlier Session history is available. Use /history to load the previous page."))
}
- write("\n%s\n\n", formatter.muted("Connected. Type a message, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit."))
+ write("\n%s\n\n", formatter.muted("Connected. Type a message, /attach PATH, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit."))
case sessionruntime.EventRuntimeRecovered:
if !pageEvent {
</file context>
Suggested change
| write("\n%s\n\n", formatter.muted("Connected. Type a message, /attach PATH, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit.")) | |
| write("\n%s\n\n", formatter.muted("Connected. Type a message, /attach PATH, /send, /history, /interrupt, /answer INPUT QUESTION VALUE, or /quit.")) |
| return f.attachment, nil | ||
| } | ||
|
|
||
| func (f *fakeSessionAttachmentTransfer) Download(_ context.Context, _, _, _ string) (sessionruntime.Attachment, []byte, error) { |
There was a problem hiding this comment.
P3: The fake Download discards the requested attachment id (4th parameter), and Upload returns a fixed attachment regardless of the file route, so the download flow's id routing/parameterization is never actually exercised by the test. A bug where downloadSessionAttachment passes the wrong identifier (e.g. the session/pod name instead of the requested id) would pass this green. Capture the id (and pod/namespace) on the fake and assert it matches the request path so the download path is truly covered.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/sessionserver/server_test.go, line 49:
<comment>The fake Download discards the requested attachment id (4th parameter), and Upload returns a fixed attachment regardless of the file route, so the download flow's id routing/parameterization is never actually exercised by the test. A bug where downloadSessionAttachment passes the wrong identifier (e.g. the session/pod name instead of the requested id) would pass this green. Capture the id (and pod/namespace) on the fake and assert it matches the request path so the download path is truly covered.</comment>
<file context>
@@ -27,9 +29,27 @@ import (
+ return f.attachment, nil
+}
+
+func (f *fakeSessionAttachmentTransfer) Download(_ context.Context, _, _, _ string) (sessionruntime.Attachment, []byte, error) {
+ return f.attachment, f.downloadData, nil
+}
</file context>
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 10, 2026
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 10, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 12, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 14, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 15, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 19, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 20, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 21, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 22, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 23, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 24, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Aug 26, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
kelos-bot Bot
pushed a commit
that referenced
this pull request
Sep 24, 2026
PR #1629 reviews showed that bounding a controller-owned StatefulSet name without accounting for ControllerRevision suffixes could either remain invalid or break workload and PVC identity. PR #1630 reviews found inconsistent attachment limits and empty-input behavior across web, TUI, and plain terminal clients, plus server failures classified as client errors by matching message text. Encode the durable-name, shared-client-contract, and typed error-classification expectations in the project and shared Kelos agent instructions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds persisted input attachments to interactive Sessions so users can provide images and other local files from either client.
/attach PATHto terminal chat and recognizes bracketed-paste file drops in the interactive terminal UIWhich issue(s) this PR is related to:
N/A
Special notes for your reviewer:
Attachments use the existing Session state volume. They survive Pod replacement only for Sessions configured with
spec.volumeClaimTemplate; reset and deletion follow the existing workspace lifecycle. The current limits are 10 MiB per file, 8 files per message, and 100 MiB or 128 files per Session.Verification completed with:
make verifymake testmake buildDoes this PR introduce a user-facing change?
Summary by cubic
Add file attachments to Sessions across web and terminal. Users can attach images and other files; attachments are stored in Session state, included in turns and history, and persist across Pod replacement when using
spec.volumeClaimTemplate.New Features
/attach PATH, bracketed-paste drag-and-drop in the interactive UI, and/sendto submit files without text.POST /api/sessions/{ns}/{name}/attachmentsand downloadGET /api/sessions/{ns}/{name}/attachments/{id};kelos-session-runtime attachment put|get; newinternal/sessionattachmentclient using Pod exec.file://URLs; all providers receive a prompt that includes an attachment summary.Migration
Provider.RunTurnnow acceptsTurnInput(text + attachments) instead of a string prompt. Update custom providers accordingly.attachmentIdsonmessagerequests if sending attachments.Written for commit aec44d0. Summary will update on new commits.