Add composer attachments and harden ACP session handling by haxudev · Pull Request #8 · formulahendry/acp-ui

haxudev · 2026-04-12T12:36:30Z

Summary

add generic composer attachment support so users can attach local files from the chat composer for any command, not just ingest
send attachments as ACP-native
esource_link content blocks while persisting metadata-only attachment history in chat
add validation, regression coverage, and desktop file-size metadata support via Tauri s:allow-stat

Details

add shared AttachmentRef model, validation helpers, MIME detection, and ile:// URI serialization
add Tauri-backed file picker integration with multi-file selection and metadata normalization
extend session prompt dispatch to include attachments alongside text prompts
render pending attachment chips in the composer and read-only attachment chips in history
tolerate streamed session/prompt completion edge cases without surfacing false-positive timeout errors
add regression and integration tests for command-agnostic attachment flows, session resets, and history rendering

Validation

npm test
npm run build

Notes

attachments are sent as references, not inlined file contents
history stores attachment metadata only, not document body content

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds end-to-end “composer attachments” support (picker → validation → UI chips/history → ACP resource_link serialization) and improves ACP session robustness around streamed updates/timeouts, with Vitest + CI integration for regression coverage.

Changes:

Introduce attachment helpers (attachments.ts), a Tauri-backed file picker seam, and message metadata wiring through the session store + ChatView UI.
Harden ACP session handling (defensive parsing/logging and tolerance for streamed prompt completion edge cases).
Add Vitest configuration, test setup mocks, and CI step to run the new test suite.

Reviewed changes

Copilot reviewed 42 out of 44 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
vite.config.ts	Adds Vitest config (happy-dom, globals, setup file).
src/test-setup.ts	Global mocks for Tauri/ACP modules in tests.
src/stores/session.ts	Adds attachment-aware `sendPrompt`, session-update hardening, and streamed-timeout tolerance.
src/lib/types.ts	Extends `ChatMessage` with `attachments` and defines `AttachmentRef`.
src/lib/file-picker.ts	Implements attachment picking via Tauri dialog + fs stat.
src/lib/attachments.ts	Adds attachment validation, MIME detection, file:// URI conversion, ACP content block serialization.
src/lib/acp-bridge.ts	Adds defensive logging for parse errors and session/update handler errors.
src/components/ChatView.vue	Adds attachment picker UI, chips, rejection messaging, and history rendering.
src/tests/smoke.test.ts	Basic test to ensure types import.
src/tests/session-store.test.ts	Covers `sendPrompt` attachment behavior + timeout edge case.
src/tests/session-reset-regression.test.ts	Regression tests for composer lifecycle across send/session switch/cancel picker.
src/tests/message-attachments.test.ts	Ensures metadata-only persistence and attachment retention rules.
src/tests/integration-command-agnostic.test.ts	Ensures attachment flow is identical for slash commands and plain text.
src/tests/file-picker.test.ts	Tests file picker normalization and stat failure behavior.
src/tests/cross-platform-contract.test.ts	Validates cross-platform contract + serializer behavior.
src/tests/chatview-validation.test.ts	Validates ChatView rejection UX and attachment limits/duplicates.
src/tests/chatview-history-attachments.test.ts	Ensures history chips render + size formatting.
src/tests/chatview-composer.test.ts	Covers attach UI presence, chip rendering/removal, send behavior.
src/tests/attachments.test.ts	Unit tests for validation + MIME mapping.
src/tests/attachments-serializer.test.ts	Unit tests for `toFileUri` and content block serialization.
src-tauri/capabilities/default.json	Enables `fs:allow-stat` for desktop size display.
package.json	Adds `vitest`, `happy-dom`, `@vue/test-utils`, and `npm test` script.
.sisyphus/notepads/acp-ui-composer-attachments/learnings.md	Notes on composer/session watcher behavior.
.sisyphus/evidence/task-9-no-content-preview.txt	Captures a test run transcript (evidence).
.sisyphus/evidence/task-8-metadata-only.txt	Evidence note for metadata-only storage.
.sisyphus/evidence/task-8-history-persist.txt	Evidence note for attachment persistence.
.sisyphus/evidence/task-7-state-preservation.txt	Evidence note for invalid selection preservation.
.sisyphus/evidence/task-7-invalid-selection.txt	Evidence note for rejection handling.
.sisyphus/evidence/task-5-text-only-regression.txt	Evidence note for text-only regression.
.sisyphus/evidence/task-5-store-dispatch.txt	Evidence note for store dispatch behavior.
.sisyphus/evidence/task-4-serializer-pass.txt	Evidence note for serializer tests passing.
.sisyphus/evidence/task-4-serializer-edge.txt	Evidence note for serializer edge cases.
.sisyphus/evidence/task-2-validation-pass.txt	Evidence note for validation tests passing.
.sisyphus/evidence/task-2-validation-fail.txt	Evidence note for a failing run.
.sisyphus/evidence/task-12-text-only-regression.txt	Evidence transcript showing a failing assertion (size BigInt leak).
.sisyphus/evidence/task-12-command-agnostic.txt	Evidence note for command-agnostic behavior.
.sisyphus/evidence/task-11-session-switch.txt	Evidence note for session switch regression.
.sisyphus/evidence/task-11-picker-cancel.txt	Evidence transcript for picker cancel regression.
.sisyphus/evidence/task-10-wire-format.txt	Evidence transcript for contract tests.
.sisyphus/evidence/task-10-cross-platform.txt	Evidence note for cross-platform contract.
.sisyphus/evidence/task-1-ci-step.txt	Evidence note for CI step ordering.
.github/workflows/ci.yml	Adds `npm test` step before type-check/build.

Comments suppressed due to low confidence (4)

src/lib/file-picker.ts:1

stat() may return size as a bigint (this is consistent with the failing evidence showing 12n/34n). Assigning that directly to AttachmentRef.size: number can later break comparisons (> against MAX_FILE_SIZE) and formatting (Math.log) at runtime. Coerce fileInfo.size to a number (and consider clamping / handling values beyond Number.MAX_SAFE_INTEGER) before storing it in AttachmentRef.size.
src/stores/session.ts:1
When the prompt times out but streaming already produced assistant content, the early return skips the session lastUpdated update and saveSessionsToStore() call that normally happens on successful prompt completion. That can lead to losing the streamed messages/metadata on app restart. Consider persisting the session/messages (and bumping currentSession.value.lastUpdated) before returning, or refactor so the persistence logic runs for both the normal success path and this streamed-timeout path.
src/components/ChatView.vue:1
The rejectionTimer isn’t cleared on component unmount. If the user navigates away quickly, the timeout callback can still fire and mutate state after teardown (and can leak timers in test runs). Add an onBeforeUnmount hook that clears the timer when it exists (and optionally reset it when session changes).
src/components/ChatView.vue:1
The remove-chip control is a symbol-only button (×). For screen readers, add an explicit accessible name (e.g., aria-label=\"Remove attachment: ${att.name}\") and set type=\"button\" to avoid unintended form-submit behavior if this component is ever placed inside a form.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-12T12:38:09Z

+ [31m鉂?[39m src/__tests__/integration-command-agnostic.test.ts [2m([22m[2m1 test[22m[2m | [22m[31m1 failed[39m[2m)[22m[32m 106[2mms[22m[39m
+[31m   [31m脳[31m attachment flow stays command agnostic[2m > [22muses the same attachment prompt structure for slash commands and plain text[39m[32m 104[2mms[22m[39m
+[31m     鈫?expected [ { type: 'text', 鈥?1) }, 鈥?2) ] to deeply equal [ { type: 'text', 鈥?1) }, 鈥?2) ][39m


PR description says npm test passed, but this committed evidence file shows a failing Vitest run for integration-command-agnostic.test.ts. If .sisyphus/evidence is intended to reflect current validation, update or remove this failing transcript (or replace it with the passing output) to avoid confusion during reviews/triage.

Copilot

Pull request overview

Copilot reviewed 42 out of 44 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T00:58:29Z

+    } catch (e) {
+      const message = e instanceof Error ? e.message : String(e);
+      const isPromptTimeout = message.includes('Request timeout: session/prompt');
+
+      if (isPromptTimeout && lastAgentActivityAt.value >= promptStartedAt) {
+        console.warn('Ignoring session/prompt timeout because agent activity already streamed for this turn.');
+        return;
+      }


In the Request timeout: session/prompt ignore path, the function returns early after streaming activity is detected. This skips the post-prompt side effects (e.g., first-turn session title update / lastUpdated persistence / telemetry) even though the user-visible turn succeeded via streaming. Consider handling the ignored-timeout case by still applying the same completion bookkeeping (and saving) based on the messages already received, rather than returning immediately.

Copilot · 2026-04-13T00:58:29Z

+  for (const candidate of candidates) {
+    const candidatePath = normalizePath(candidate.path);
+
+    if (!isAllowedExtension(candidate.name)) {
+      rejected.push({ name: candidate.name, reason: 'disallowed extension' });
+      continue;
+    }


validateAttachments checks allowed extensions using candidate.name, but duplicates are tracked using candidate.path. If a caller ever constructs AttachmentRef with a mismatched name (e.g., report.md) and path (e.g., /tmp/report.exe), the extension allowlist can be bypassed. To keep validation robust, derive the extension from candidate.path (or validate both name and path extensions match / are allowed).

Copilot · 2026-04-13T00:58:29Z

+// Pending attachments state
+const pendingAttachments = ref<AttachmentRef[]>([]);
+const rejectionMessages = ref<string[]>([]);
+let rejectionTimer: ReturnType<typeof setTimeout> | null = null;
+
+function setRejectionMessages(messages: string[]): void {
+  rejectionMessages.value = messages;
+
+  if (rejectionTimer) {
+    clearTimeout(rejectionTimer);
+  }
+
+  rejectionTimer = setTimeout(() => {
+    rejectionMessages.value = [];
+    rejectionTimer = null;
+  }, 5000);
+}


setRejectionMessages schedules a setTimeout but the component never clears it on unmount. This can leave a dangling timer and attempt to update state after the component is destroyed. Consider adding an onBeforeUnmount hook to clear rejectionTimer (and null it) when the view unmounts.

Copilot · 2026-04-13T00:58:30Z

+        <div v-if="pendingAttachments.length > 0" class="pending-attachments">
+          <div v-for="att in pendingAttachments" :key="att.id" class="attachment-chip">
+            <span class="attachment-chip-icon">📎</span>
+            <span class="attachment-chip-name">{{ att.name }}</span>
+            <button 
+              class="attachment-chip-remove" 
+              @click="removeAttachment(att.id)"
+              title="Remove attachment"
+            >×</button>
+          </div>
+        </div>
+
+        <div class="input-row">
+          <textarea
+            v-model="inputText"
+            :placeholder="availableCommands.length > 0 ? 'Type your message... (/ for commands)' : 'Type your message...'"
+            :disabled="isLoading"
+            @keydown="handleKeyDown"
+            rows="3"
+          />
+          <button
+            class="attach-btn"
+            :disabled="isLoading"
+            @click="handleAttach"
+            title="Attach files"
+          >
+            📎
+          </button>


The attachment UI buttons are icon-only (📎 / ×) and currently rely on title for labeling. title is not a reliable accessible name for screen readers, and the remove button has no aria-label. Please add explicit accessible labeling (e.g., aria-label/visually-hidden text) for the attach button and each attachment remove button.

Copilot · 2026-04-13T00:58:30Z

+export function toFileUri(filePath: string): string {
+  let normalized = filePath.replace(/\\/g, '/');
+
+  if (!normalized.startsWith('/')) {
+    normalized = '/' + normalized;
+  }
+
+  const encoded = normalized.split('/').map((segment) => encodeURIComponent(segment)).join('/');
+  return 'file://' + encoded;
+}


toFileUri unconditionally prefixes a leading / and then prepends file://, which produces non-standard URIs for Windows UNC paths. For example, \\server\share\file.txt becomes file://////server/share/file.txt (extra slashes) instead of the expected file://server/share/file.txt. Consider detecting UNC paths (/^\\\\/) and serializing them as file://<host>/<share>/... per file URI rules.

OpenCode and others added 16 commits April 12, 2026 17:10

test(ui): add vitest infrastructure for tauri vue app

e712c73

feat(attachments): add shared attachment model and validation helpers

98649af

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

feat(attachments): add acp resource link serialization helper

ec3c1f1

feat(attachments): add platform attachment normalization helper

c5db864

fix(attachments): align resource_link shape with ACP SDK types

db1c0db

feat(chat): add composer attachment ui

7afe60b

feat(session): support attachment-aware prompt dispatch

b09cd4a

feat(chat): persist attachment metadata in messages

fd0ffa0

feat(chat): add attachment validation feedback

adede8f

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

test(attachments): add cross-platform contract coverage

4410873

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

feat(chat): render attachment metadata in history

d141475

test(chat): add composer attachment reset regressions

c0c9b8d

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

feat(chat): finalize generic attachment send flow

7474efb

fix(chat): collapse dead branch in command select

c08d46b

fix(chat): collapse dead branch in command select

dcaa686

fix(session): tolerate streamed prompt completion edge cases

1a7ec57

Copilot AI review requested due to automatic review settings April 12, 2026 12:36

Copilot AI reviewed Apr 12, 2026

View reviewed changes

formulahendry requested a review from Copilot April 13, 2026 00:55

Copilot started reviewing on behalf of formulahendry April 13, 2026 00:56 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add composer attachments and harden ACP session handling#8

Add composer attachments and harden ACP session handling#8
haxudev wants to merge 16 commits intoformulahendry:mainfrom
haxudev:feat/composer-attachments

haxudev commented Apr 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

haxudev commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Validation

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haxudev commented Apr 12, 2026 •

edited

Loading