feat(workspace): remove a pending attachment from the composer with an × on its chip (#3738)

[vybe]

Fixes #3738 — second of two Workspace composer/tab fixes in chain-3737 (epic #3471). Stacked on #3742 — base is feature/3737-overflow-starred-row; the stack merges together in order on the operator's call (#3742 first, then this PR retargeted to dev). Draft until the chain is in.

What

• src/frontend/src/components/portal/PortalAttachmentChip.vue — new shared chip for a pending attachment (paperclip, name, upload spinner / failure text) with a real <button type="button"> × that removes exactly that file (aria-label="Remove <name>", @click.stop, chip ink, hover + focus-visible, touch-sized hit area, chip height unchanged). Both composers' chip markup was identical, so both now use it.
• src/frontend/src/composables/usePortalFileDrop.js — remove(entry): a removed flag the batch loop honours (a queued file never uploads once removed), an in-flight file is dropped from the chip row at once and its inbox copy deleted after the upload lands, a sent file's copy is deleted at once, a failed file deletes nothing. The delete is best effort: never awaited by the UI path, never writes a chip error, a failure is one console.warn with the filename and status. Another live chip with the same stored name skips the delete (the server overwrites on a repeated name).
• src/frontend/src/stores/clientPortal.js — discardComposerUpload: forgets the escalation carry-log entry first (so carryableUploadsFor cannot re-attach a withdrawn file to a room), then deletes, then refreshes the rail's Files list.
• src/frontend/src/components/portal/PortalConversation.vue — chip row on the shared chip; the × is locked during the escalation hand-off.
• src/frontend/src/components/portal/PortalRoom.vue — chip row on the shared chip; the room's upload function now returns { filename, stored: [{agent, filename}] } (plus a partial receipt when only some agents took the file), so removing a chip deletes every agent's copy — and only those.
• src/frontend/raw-color-baseline.json — the two composers' raw-gray counts go down (90→85, 47→42) because the chip markup moved into the new component; edited by hand because the regenerate script clobbers the refrozen notes block.
• src/frontend/tests/unit/portalAttachmentChipRemove.spec.js — 24 tests: composable state rules, same-name skip (live siblings only), failing delete (warn spied), double-click, the store's carry-log forget, the chip mounted inside a <form> (× removes exactly that entry, zero submits), a real PortalRoom mount with two agents, partial-landing then failure, and a template check over both composers. portalRailFiles.spec.js re-pinned to the new upload function.

Rulings carried (orchestrator, on the operator's behalf — plan file)

• TD1 — × on a file still uploading: the chip goes at once, the delete runs after the upload lands; a queued file never uploads.
• TD2 — a failed delete is a console warning only; the issue's AC forbids a chip error, and silence would leave no trace.
• TD3 — extract the chip into a shared component so it can actually be mounted in jsdom (the two composers have no mount harness); baseline counts only shrink.
• TD4 — a filename an earlier message already sent, re-dropped and then removed: the earlier message's attachment will 404 afterwards. It already pointed at the overwritten bytes, so this is accepted and documented here rather than guarded.

Review + security

/review (Fable, report-only): MERGEABLE — 0 critical; all ten rider checks hold with file:line cites (no submit path from the ×, the batch loop honours removed, carry log forgotten by File identity before the DELETE, no window before the hand-off lock, every consumer of the room upload's return value read, baseline matches the scanner). Informational I1 (same-name skip counted a never-uploaded chip as live) and I3 (a partly-landed room chip removed after its failure deleted nothing) fixed in 099c59c6b with red-before/green-after test rows; I2 closed in a4a22dab0: /validate-pr round 1 (REQUEST CHANGES) found that the component does have a mount harness, so portalAttachmentChipHandoff.mount.spec.js now mounts the real conversation composer with the escalation setup and proves the × is disabled and deletes nothing during the hand-off, then is enabled and deletes once after the hand-back; dropping the lock's set, its clear, or its binding each turns it red. /cso --diff: zero findings; the one security surface is the existing DELETE route, whose ownership check lives in the client-portal router and service (roster gate, caller-scoped inbox path, traversal refusal).

Tests

• 15 spec files / 301 tests green in the review's evidence run; 10 files / 167 after the fix; 9 files / 143 after the hand-off lock test, including the three ratchets. Fix-reverted mutation: 3 red / 19 green.
• Not run here: the full npm run test:unit island and e2e — CI's. Stacked PR: backend-unit-test does not run on a feature-branch base, so the first full run comes when this retargets dev after fix(workspace): keep a starred or pinned chat's title beside its glyph in the tab overflow menu (#3737) #3742 merges.

Before merge

• Merge order: fix(workspace): keep a starred or pinned chat's title beside its glyph in the tab overflow menu (#3737) #3742 first; then retarget this PR to dev. Merge-tree is clean against the base (3c84390) and against live dev (a32a9c8). No migration.
• Sibling feat(workspace): the autonomy dial — what a companion may do unprompted (ent#641) #2984 (autonomy dial, draft) also edits clientPortal.js: its five conflicts against live dev are entirely its own; this branch adds none.
• Known edge (TD4): re-dropping a filename an earlier message already sent and then removing it makes that earlier attachment 404.

Handoffs

• None.

🤖 Generated with Claude Code

[trinity-ability]

PR Validation Report — #3743 (stacked)

Branch: feature/3738-attachment-chip-remove @ 099c59c6b → feature/3737-overflow-starred-row (#3742, @ 3c843907b). I validated this against that stacked base, not dev. · Lane B (frontend only; no schema) · 8 files, +664/−38 · Fixes #3738 (P2 bug, closing keyword present)

Summary

Category	Status	Notes
Commit messages	✅	3 conventional commits, each says what changed and why
Base branch	⚠️	Stacked on #3742 by design. This PR retargets dev after #3742 merges. It is still a draft
Status auto-promotion	✅	Fixes #3738 (same repo)
Requirements / Architecture / Feature flows	➖	Bug fix: tiered docs say a commit message is enough
Security (§4.1–4.10)	✅	No keys, emails, IPs, env or credential files, symlinks, mode changes or host paths. The one reachable surface is the existing roster-gated DELETE …/uploads/{filename} (see the /cso --diff in the review file)
Build / config packaging	➖	No backend file in range
Structural review	✅	/review → MERGEABLE (plan file 3738-review.md). It is recorded in the PR body, not posted as a review
Code quality	✅	Scoped to the issue. The chip extraction de-duplicates two byte-identical blocks (#2662). Baseline only shrinks (90→85, 47→42); rawColorRatchet (including its exactness test) is green
Execution coverage (§5.4)	❌	The hand-off lock has no executing test. Details below
Merge gate	⚠️	Tier 1 is only partly evaluated on a feature-branch base. Details below

Evidence (from the PR's own worktree)

npx vitest run portalAttachmentChipRemove roomEscalationAttachments roomComposerChain portalComposerWiring portalRailFiles roomFileReach portalSendWhileUploading.mount portalMessageAttachments.mount loadingGateRatchet rawColorRatchet sourceTextRatchet → 11 files, 180/180 passed.

Mutation run: one change at a time, 6 related spec files (114 tests), each file restored after its run:

mutation	result
M1 loop ignores entry.removed (queued file uploads anyway)	red (1)
M2 same-name skip counts a never-uploaded chip as live (I1 fix reverted)	red (1)
M3 remove() ignores a partial receipt (I3 fix reverted)	red (1)
M4 no discard after an in-flight upload lands	red (2)
M5 store does not forget the carry-log entry	red (5)
M8 × loses type="button"	red (1)
M10 room upload drops err.receipt on a partial failure	red (1)
M6 delete handedOff.value = true after the escalation emit	green (114/114)
M7 delete handedOff.value = false in the prefill watcher	green (114/114)
M11 :locked="escalatingNow || settlingUploads" (drop handedOff)	green (114/114)
M9 conversation discard ignores the receipt's agent	green; moot in production, because convKey includes the agent name, so an agent switch remounts the composer and its chips

The composable, store, chip and room are properly executed and mutation-proven.

Execution coverage (§5.4)

changed symbol	executed by	live consumer	verdict
usePortalFileDrop.remove / discardQuietly / removed skip	spec §1 (10 cases)	chip @remove, both composers	✅
clientPortal.discardComposerUpload	spec §2	both composers' discard	✅
PortalAttachmentChip.vue ×	spec §3, mounted in a <form @submit>	both composers	✅
PortalRoom upload receipt + discard	spec §4, a real PortalRoom mount	—	✅
template call-site set (@source-text-pin)	spec §5 AST walk	—	🛡 guard (accepted: the reason names the call-site set, which a mount cannot prove)
handedOff lock in PortalConversation.vue (set after the emit at :2641, cleared at :1709, bound at :627)	—	:locked → :disabled on a × that deletes	❌

Fix-PR mutation: M1–M4 go red with the fix reverted ✅. The lock does not (M6, M7, M11).

Critical (blocks merge)

• The hand-off lock gates a destructive verb and nothing executes it. It exists because a × in that window would delete the origin agent's copy while the shell is carrying it into the room (R2). Under §5.4 / trinity#2918, a predicate that gates a delete with no executing coverage is ❌ whatever the PR type. The recorded reason (I2: "the conversation component has no mount harness") is not accurate. PortalConversation.vue is mounted by about 11 existing specs. portalSendWhileUploading.mount.spec.js mounts it with the file-drop composable, and portalInChatReply.mount.spec.js:341 already sets store.multiAgentChatAvailable = true with a two-agent roster, which is the escalation precondition. One mount test in that harness closes it:

  1. Drop a file, let it land, then type @sage … and send.
  2. Assert escalate-to-room was emitted and the chip's -remove button is disabled.
  3. setProps({ prefill: 'text' }) and assert the button is enabled again.

  M6, M7 and M11 should then all go red.

Warnings

• Merge gate on a stacked base. CI on the tip is green for everything that ran (frontend-build, frontend-e2e, gitleaks, schema-parity, verify-non-root, container-security). But the dev-filtered Tier 1 jobs did not run on a feature-branch base: the required Analyze (python) / Analyze (javascript-typescript), pytest (head) + regression diff (backend-unit-test), and the lints. That gap closes when this PR is retargeted to dev after #3742 merges, and Tier 1 must be green then, before merge. The backend suite is formally out of scope here (no backend file changed), but it still has to report. dev-ci is currently success, and no dev-ci red issue is open.
• TD4 (a re-dropped and then removed name makes an earlier message's attachment 404) is accepted and documented, per the operator ruling.

Suggestions (optional)

• The × hit box (after:-inset-2) reaches about 2px into the next chip across the 6px gap. This is a design call; flagged only.

Recommendation: REQUEST CHANGES

The core fix is correct, well scoped and mutation-proven. One test is missing: the hand-off lock that guards a delete, which the existing mount harness can execute. After that test lands and #3742 merges, retarget to dev and check that Tier 1 is green.

This PR requires the following before merge:

• Add a PortalConversation mount test that sets and clears handedOff (M6/M7/M11 red)
• After #3742 merges, retarget to dev and confirm Tier 1 (Analyze ×2, pytest (head) + regression diff) is green

[trinity-ability]

PR Validation Report — #3743, round 2 (stacked)

Branch: feature/3738-attachment-chip-remove @ a4a22dab0 → feature/3737-overflow-starred-row (#3742, @ 3c843907b, which is also the merge-base). I validated this against that stacked base, not dev. · Lane B (frontend only; no schema) · 9 files, +803/−38 · Fixes #3738 (closing keyword present)

What changed since round 1

Commits 7b6f207, 493725a and 099c59c are unchanged. The only new commit is a4a22dab0, which adds one file: src/frontend/tests/unit/portalAttachmentChipHandoff.mount.spec.js (+139, test only).

Round-1 blocker: hand-off lock coverage (§5.4) is now ✅

The new spec mounts the real PortalConversation with the real PortalAttachmentChip (// @vitest-environment jsdom, no readFileSync). It runs the escalation path a user hits: paste a file, type @sage …, press Enter. It then checks three things:

• During the hand-off: the × is disabled. A click deletes nothing, whether sent through VTU trigger or native element.click(), and the chip stays.
• After the hand-back (setProps({ prefill }), which is what Portal.vue does after a failed escalation): the × is enabled. One click removes the chip and calls deleteUpload('scout','stored_report.pdf') exactly once.
• The live consumers are PortalConversation.vue:629 (:locked), :2641 (the set) and :1709 (the clear in the prefill watcher).

Mutations I re-ran myself in the PR worktree, against portalAttachmentChipHandoff.mount (tree restored after each run):

mutation	result
M1: drop the set (:2641 handedOff.value = true)	2 red / 1 green
M2: drop the clear (:1709 handedOff.value = false)	1 red / 2 green
M3: drop `
M4 (extra): `

All three round-1 mutations, which stayed green last round, now fail. So does the extra operator-swap mutation.

Executable evidence

npx vitest run portalAttachmentChipHandoff.mount portalAttachmentChipRemove portalInChatReply.mount roomEscalationAttachments roomComposerChain portalComposerWiring portalRailFiles roomFileReach portalSendWhileUploading.mount portalMessageAttachments.mount loadingGateRatchet rawColorRatchet sourceTextRatchet → 13 files passed, 203 tests passed. The new spec reads no source text, so sourceTextRatchet stays green.

Summary

Category	Status	Notes
Commit messages	✅	4 conventional commits
Base branch	⚠️	Stacked on #3742 by design. Retarget to dev once #3742 merges
Status auto-promotion	✅	Fixes #3738
Requirements / Architecture / Flows	➖	Bug fix: a commit message is enough
Security (§4.1–4.10)	✅	0 hits for keys, emails, IPs, symlinks, mode changes or host paths over the full diff. The round-1 /cso --diff still applies: the delta is test-only
Structural review	✅	/review → MERGEABLE (round 1). The delta is a test file
Execution coverage (§5.4)	✅	Hand-off lock executed and mutation-pinned (above). The rest of the coverage is unchanged from round 1
Merge gate (Tier 1, stacked)	✅ / ➖	All 11 check runs on a4a22dab0 are success or skipped (report, auto-merge), with no cancelled rows. The dev-only contexts (Analyze (python), Analyze (javascript-typescript), the backend unit test / regression diff) only run after the retarget to dev. They have to be green on the retargeted PR before merge

Carry-overs (non-blocking, as in round 1)

I1 (same-name skip vs a never-uploaded chip) and I3 (partial receipt ignored on × after a failure) are still informational.

Recommendation

APPROVE. The single round-1 blocker is fixed, and I have mutation evidence for it. Merge order: #3742 first, then retarget this PR to dev and let the dev-only checks go green.