fix(ui): improve picker deletion, ESC cancel, and SHIFT+ENTER path handling

[VooDisss]

Summary

Fixes three issues discovered during manual testing of the directory mentions & navigation feature (Issue #136):

1. Ghost state in dropdown - Picker now resets selection index when query changes
2. Incomplete file attachment deletion - Backspace now properly removes attachments by matching full path
3. ESC key not canceling - ESC now removes partial @mention text from prompt

Also improves SHIFT+ENTER behavior to keep @ in prompt for easy deletion while stripping it when sending to API.

Changes

unified-picker.tsx

• Reset selectedIndex to 0 when query changes to avoid ghost highlighting

usePromptKeyDown.ts

• Enhanced attachment matching to find attachments by full path (not just filename)
• Close picker when @ mentions are deleted via BACKSPACE
• Clear ignoredAtPositions when deleting last @ to allow fresh picker

usePromptPicker.ts

• ESC now removes partial @mention text (acts as cancel)
• SHIFT+ENTER keeps @path in prompt (BACKSPACE deletes as unit), removes @ when sending

prompt-placeholders.ts

• Strip @ from paths without file attachments when sending to API

Manual Testing Protocol - ALL PASSED ✓

Section 1: TAB Key Behavior

• 1.1 TAB on directory → autocomplete + show contents
• 1.2 TAB on file → autocomplete path only (no attach)

Section 2: ENTER Key Behavior

• 2.1 ENTER on directory → attach with contents + add text
• 2.2 ENTER on file → attach file + add text

Section 3: SHIFT+ENTER Key Behavior

• 3.1 SHIFT+ENTER on directory → path as text (no contents)
• 3.2 SHIFT+ENTER on file → path as text (no contents)

Section 4: ESC Key Behavior

• 4.1 ESC removes partial @mention text
• 4.2 ESC closes picker

Section 5: Deletion of Attached Tokens

• 5.1 BACKSPACE on directory token → removes token + detaches
• 5.2 BACKSPACE on file token → removes token + detaches
• 5.3 Picker closes after deletion
• 5.4 Can type @ again to open fresh picker

Section 6: Ghost State / Picker Reset

• 6.1 Navigate into directory, delete back to @ → first item highlighted
• 6.2 Query changes reset picker properly

Section 7: Multiple Attachments

• 7.1 Multiple different attachments work
• 7.2 Duplicate attachments prevented

Section 8: Click Behavior

• 8.1 Click on directory → attaches
• 8.2 Click on file → attaches

---

All manual tests passed. Ready for review.

[shantur]

@CodeNomadBot

Code Review please

[codenomadbot]

Code review for PR #171 (picker deletion + ESC cancel + Shift+Enter path behavior)

I pulled the diff locally and focused on:

• packages/ui/src/components/unified-picker.tsx
• packages/ui/src/components/prompt-input/usePromptKeyDown.ts
• packages/ui/src/components/prompt-input/usePromptPicker.ts
• packages/ui/src/lib/prompt-placeholders.ts

What looks solid

• packages/ui/src/components/unified-picker.tsx: adding a PickerSelectAction and threading it through onSelect(...) is a clean way to differentiate Tab vs Enter vs Shift+Enter vs click.
• packages/ui/src/components/unified-picker.tsx: resetting selectedIndex on query change should eliminate the “ghost highlight” state.
• packages/ui/src/components/prompt-input/usePromptPicker.ts: the replaceMentionToken(...) vs replaceMentionQueryAfterAt(...) split makes the intent (replace whole token vs navigate directory) easier to reason about.
• packages/ui/src/components/prompt-input/usePromptKeyDown.ts: matching attachments by full path (not only basename) is the right direction; it fixes the “same filename in different dirs” class of bugs.

Potential issues / regressions to watch

• packages/ui/src/lib/prompt-placeholders.ts: the new prompt.replace(/@([^\s@]+)/g, ...) is very broad and will rewrite any @token that isn’t a file attachment.

  • Examples that would be affected when sending: GitHub handles (@shantur -> shantur), emails (a@b.com -> ab.com), any literal mentions in normal text.
  • This is the highest-risk change in the PR because it alters user text in unrelated contexts.
  • Suggestion: narrow the rewrite to “path-like” mentions only (ideally something the picker can mark explicitly), instead of a global @... regex.

• packages/ui/src/components/prompt-input/usePromptPicker.ts: addPathOnlyAttachment(...) is currently unused.

  • If tsconfig ever enables noUnusedLocals, this will become a build error.
  • Also: the PR description mentions “Shift+Enter keeps @path in prompt for easy deletion”, but with the current implementation Shift+Enter does not create any attachment, so packages/ui/src/components/prompt-input/usePromptKeyDown.ts won’t treat @path as a deletable unit (it only intercepts when an attachment match is found). If your manual test passes today, it might be relying on behavior that isn’t in this diff.

• packages/ui/src/components/prompt-input/usePromptPicker.ts: the “replace parent/child directory attachment” logic removes overlapping directory attachments but does not remove/update the corresponding @docs/ token text already present in the prompt.

  • That can lead to orphaned tokens in the textarea (token remains, attachment is gone), and then BACKSPACE deletion won’t detach anything because there is nothing to match.
  • If the intent is “only one directory attachment at a time”, consider also cleaning up the old token(s) in the prompt, or avoid auto-removing attachments and instead prompt the user.

Verification note

• I attempted npm -w packages/ui run typecheck, but tsc isn’t available in this environment (deps not installed), so I couldn’t run a real typecheck/build here.

If you want one concrete change recommendation: I’d tighten packages/ui/src/lib/prompt-placeholders.ts so it only strips @ for the Shift+Enter path-only case (without touching general @mentions / emails), because that’s the biggest potential user-visible regression.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Raise PR with fixes

[codenomadbot]

I understand “Raise PR with fixes” as: apply the review fixes (primarily tightening packages/ui/src/lib/prompt-placeholders.ts so we don’t strip @ from non-path tokens like @user / emails / agent mentions; plus optionally removing the unused addPathOnlyAttachment(...) helper).

One clarification before I publish anything (to avoid targeting the wrong base):

• Do you want me to push commits directly onto the existing PR branch codenomad/pr-171 (this updates PR fix(ui): improve picker deletion, ESC cancel, and SHIFT+ENTER path handling #171 in-place) — recommended — or do you want a separate follow-up PR? If separate, should its base be dev or codenomad/pr-171?

Once you confirm the target, I’ll implement the minimal change and open/push accordingly.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Review changes between this PR branch and dev.
Commit changes to this branch

[codenomadbot]

Done.

What I understood from the latest request

• Review origin/dev...PR and apply the minimal fix(es) from the review, then commit to an org-owned branch.

What I did

• Implemented the fix in packages/ui/src/lib/prompt-placeholders.ts so we no longer strip @ from non-path tokens (e.g. @user mentions / email addresses). Stripping now only applies to path-like tokens and still preserves @ when there’s a backing file attachment.
• Removed an unused helper/import in packages/ui/src/components/prompt-input/usePromptPicker.ts.
• Committed: 4e0f064 fix(ui): avoid stripping non-path @mentions

Important note

• The PR head (pull/171/head) does not include this commit (likely a fork PR), so I can’t push the commit onto the contributor’s branch.

Fix PR raised

• fix(ui): avoid stripping non-path @mentions #172

If you’d rather not take a follow-up PR, you can cherry-pick 4e0f064 onto the original PR branch.

--
Yours,
CodeNomadBot

[VooDisss]

I have fixed the issue that bot noticed in regards to attaching emails and have fixed the non-displaying of chips when attaching files/directories via SHIFT+ENTER and also other issues. All tested manually using protocol below:

---

Summary

Fixes multiple issues discovered during manual testing of the directory mentions & navigation feature (Issue #136):

1. ESC key not canceling - ESC now clears ignored positions so typing @ again works immediately
2. Nested directory attachments - Both parent and child directories can be attached separately
3. Email preservation - Emails like test@email.com preserve the @ symbol when sending
4. SHIFT+ENTER path handling - Paths like @docs/file.txt correctly strip @ when sending while preserving emails
5. Previous fixes - Ghost state, attachment deletion, picker closing all preserved

Changes

unified-picker.tsx

• Reset selectedIndex to 0 when query changes to avoid ghost highlighting

usePromptKeyDown.ts

• Enhanced attachment matching to find attachments by full path (not just filename)
• Close picker when @ mentions are deleted via BACKSPACE
• Clear ignoredAtPositions when deleting last @ to allow fresh picker

usePromptPicker.ts

• ESC now removes partial @mention text and clears ignored positions
• SHIFT+ENTER keeps @path in prompt (BACKSPACE works), adds text attachment chip, strips @ when sending

prompt-placeholders.ts

• Only strip @ from path-like tokens (containing / or ending with /)
• Preserve @ in emails (containing . without /)
• Use text attachment set to determine which paths to strip @ from

session-actions.ts

• Skip path: text attachments from being sent as duplicate content

Manual Testing Protocol - ALL PASSED ✓

Section 1: TAB Key Behavior

• 1.1 TAB on directory → autocomplete + show contents
• 1.2 TAB on file → autocomplete path only (no attach)

Section 2: ENTER Key Behavior

• 2.1 ENTER on directory → attach with contents + add text
• 2.2 ENTER on file → attach file + add text

Section 3: SHIFT+ENTER Key Behavior

• 3.1 SHIFT+ENTER on directory → chip appears, @path in prompt, sends as path
• 3.2 SHIFT+ENTER on file → same as 3.1

Section 4: ESC Key Behavior

• 4.1 ESC removes partial @mention text
• 4.2 ESC closes picker + typing @ again works immediately

Section 5: Deletion of Attached Tokens

• 5.1 BACKSPACE on directory token → removes token + detaches
• 5.2 BACKSPACE on file token → removes token + detaches
• 5.3 Picker closes after deletion
• 5.4 Can type @ again to open fresh picker

Section 6: Ghost State / Picker Reset

• 6.1 Navigate into directory, delete back to @ → first item highlighted
• 6.2 Query changes reset picker properly

Section 7: Multiple Attachments

• 7.1 Multiple different attachments work
• 7.2 Duplicate attachments prevented

Section 8: Click Behavior

• 8.1 Click on directory → attaches
• 8.2 Click on file → attaches

Section 9: Directory Hierarchy

• 9.1 Both @docs/ and @docs/plans/ can be attached (not replaced)

Section 10: Non-Path @ Mentions

• 10.1 @user123 → keeps @
• 10.2 test@email.com → keeps @
• 10.3 @docs/ via SHIFT+ENTER → chip appears, sends as docs/

All manual tests passed. Ready for review.

---

@shantur please be biased towards merging this one, as bot can't test the UI manually like I did.

Unless you want to try finding more edge cases or bot finds them (I can test them).

[shantur]

@CodeNomadBot

Can you re-review the PR with latest changes.

[codenomadbot]

Re-reviewing PR #171 with latest head 0111a3e ("preserve emails" + SHIFT+ENTER chips + session-actions tweak).

What improved since the earlier version

• packages/ui/src/components/prompt-input/usePromptPicker.ts: SHIFT+ENTER now adds a text attachment (display: "path: ...") so chips can render and BACKSPACE can treat it as a unit.
• packages/ui/src/stores/session-actions.ts: skipping display: "path: ..." text attachments avoids sending duplicate text parts (prompt already includes the path).
• packages/ui/src/components/prompt-input/usePromptPicker.ts: ESC now clears ignored positions, which matches the “type @ again immediately” expectation.

High-risk / likely incorrect behavior still present

• packages/ui/src/lib/prompt-placeholders.ts: the current regex
  • result = prompt.replace(/@([^\s@/]+(?:\/[^ \s@/]+)*\/?)/g, ...)
    matches plain mentions like @user123 (because the /(...)* part can be empty), and in that case the code falls through to return normalizedPath, stripping the @.
  • That contradicts the stated manual test (Section 10.1) and will also affect agent mentions like @someAgent.

Edge cases to double-check (these look broken with the current implementation)

• Root directory mention @/:
  • The regex does not match / as the first char in the capture group, so @/ will not be stripped on send (it will be sent as @/).
• Root-level files like @README.md:
  • The “email heuristic” (hasDotAfterAt && !includes("/")) will treat these as “email-like” and preserve @, so SHIFT+ENTER would not strip @ for root-level files.
• Directory paths lose trailing slash on send:
  • normalizedPath = path.replace(/\/$/, "") means @docs/ becomes docs (no /). If downstream expects the slash to disambiguate directories, this will be problematic.

Suggestion (minimal + safe)

• Make stripping depend on the SHIFT+ENTER marker only (the path: text attachments set), and do exact replacements for those values (including / and README.md) rather than a broad @... regex + heuristics.
  • This avoids touching normal @mentions / agent mentions / emails entirely.

If you want, I can propose a concrete replacement strategy (regex escaping + longest-match-first) but I’m not changing code unless asked.

--
Yours,
CodeNomadBot

[VooDisss]

Last Commit: 4b967d1

What Was Fixed:

1. Exact Path Matching - Replaced regex-based @ stripping with exact matching from text attachments. This prevents:

   • @user123 being stripped to user123
   • @someAgent being stripped

2. Email Preservation - Emails like test@email.com now correctly keep the @

3. Root Directory @/ Support - Added root directory option in picker when typing @/

4. Trailing Slash Preservation - @docs/ now correctly sends as docs/ (with slash), not docs

---

Full Testing Protocol

Section 1: TAB Key Behavior

• 1.1 TAB on directory → autocomplete + show contents
• 1.2 TAB on file → autocomplete path only (no attach)

Section 2: ENTER Key Behavior

• 2.1 ENTER on directory → attach with contents + add text
• 2.2 ENTER on file → attach file + add text

Section 3: SHIFT+ENTER Key Behavior

• 3.1 SHIFT+ENTER on directory → chip appears, @path in prompt, sends as path
• 3.2 SHIFT+ENTER on file → same as 3.1

Section 4: ESC Key Behavior

• 4.1 ESC removes partial @mention text
• 4.2 ESC closes picker + typing @ again works immediately

Section 5: Deletion of Attached Tokens

• 5.1 BACKSPACE on directory token → removes token + detaches
• 5.2 BACKSPACE on file token → removes token + detaches
• 5.3 Picker closes after deletion
• 5.4 Can type @ again to open fresh picker

Section 6: Ghost State / Picker Reset

• 6.1 Navigate into directory, delete back to @ → first item highlighted
• 6.2 Query changes reset picker properly

Section 7: Multiple Attachments

• 7.1 Multiple different attachments work
• 7.2 Duplicate attachments prevented

Section 8: Click Behavior

• 8.1 Click on directory → attaches
• 8.2 Click on file → attaches

Section 9: Directory Hierarchy

• 9.1 Both @docs/ and @docs/plans/ can be attached

Section 10: Non-Path @ Mentions & Edge Cases

• 10.1 @user123 → keeps @
• 10.2 test@email.com → keeps @
• 10.3 @docs/ via SHIFT+ENTER → chip appears, sends as docs/
• 10.4 @README.md via SHIFT+ENTER → sends as README.md
• 10.5 @/ root directory → shows in picker, can attach

---

All manual tests passed ✓

[shantur]

@CodeNomadBot

Please code review with latest changes.

[codenomadbot]

Re-review (latest PR head 4b967d1).

What I understood the request to be

• Re-review PR fix(ui): improve picker deletion, ESC cancel, and SHIFT+ENTER path handling #171 after the newest changes landed.

What looks fixed (vs my previous review)

• packages/ui/src/lib/prompt-placeholders.ts: the risky global regex-based stripping is gone; stripping is now driven by the SHIFT+ENTER marker (the display: "path: ..." text attachments) and uses exact @${path} replacements. This should prevent @user123 / @someAgent / emails from being rewritten.
• packages/ui/src/components/unified-picker.tsx: @/ support is implemented by adding a root directory option when the query is /.
• packages/ui/src/lib/prompt-placeholders.ts: trailing slash preservation is effectively handled (directory values like docs/ remain docs/ when @ is stripped).

Things I’d still sanity-check / minor edge cases

• packages/ui/src/lib/prompt-placeholders.ts: exact split/join replacement won’t strip if the token is edited to include trailing punctuation (e.g. @docs/,). Not necessarily a blocker, just a behavior to be aware of.
• packages/ui/src/lib/prompt-placeholders.ts: if the same path exists as both a file attachment and a path: text attachment, the current “skip if file attachment exists” branch will keep @ (may or may not be what you want, but it’s at least deterministic).

Net: the biggest correctness concern from the previous review (stripping @ from plain mentions / agents) appears resolved in 4b967d1.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Review complete changes of the PR.

Focus on

• Code Quality
• Any issues
• Any high / critical changes to be done

[codenomadbot]

Full re-review of PR #171 head 4b967d1 (diff vs dev: 5 files, ~250 LOC added).

What I reviewed

• packages/ui/src/components/unified-picker.tsx
• packages/ui/src/components/prompt-input/usePromptKeyDown.ts
• packages/ui/src/components/prompt-input/usePromptPicker.ts
• packages/ui/src/lib/prompt-placeholders.ts
• packages/ui/src/stores/session-actions.ts

Code Quality (overall)

• The split between picker UI (unified-picker.tsx) and prompt behavior (usePromptPicker.ts) is clean; adding PickerSelectAction is a good way to keep Tab/Enter/Shift+Enter semantics explicit.
• SHIFT+ENTER now creates a tagged text attachment (display: "path: ...") which makes chips render and makes deletion logic tractable.
• session-actions.ts correctly avoids sending these path-only text attachments as duplicate parts (prompt already contains the path).
• prompt-placeholders.ts moving away from broad regex rewrites to “only strip @ for values we explicitly attached via SHIFT+ENTER” is the right safety model.

High / critical issues to address before merge

• i18n regression in packages/ui/src/components/unified-picker.tsx: t("unifiedPicker.sections.directories") is a new key but it does not exist in the message bundles (at least in this base). This will render the raw key to users.
  • Fix: add unifiedPicker.sections.directories to packages/ui/src/lib/i18n/messages/*/commands.ts (or reuse unifiedPicker.sections.files if you don’t want a new key).
• Non-i18n user-visible strings:
  • packages/ui/src/components/unified-picker.tsx renders / (root) as a hardcoded English label.
  • SHIFT+ENTER chips use display: "path: ${value}" (also English) and is visible in the UI.
  • Fix: either localize these strings or switch to a non-user-visible marker (e.g. keep display as @${value} and tag via a separate field / convention).

Notable functional edge cases (medium)

• packages/ui/src/components/unified-picker.tsx: root item rendering is gated behind fileCount() > 0.
  • If files() is empty (failed snapshot/search, empty workspace, etc.), the root option can still exist in allItems() (keyboard selection) but the UI will show the empty state and not render the root row. This is confusing.
  • Fix: treat “root row exists” as part of the visible file section condition (e.g. show file section when fileCount()>0 || props.searchQuery === "/").

Minor / code cleanliness

• packages/ui/src/lib/prompt-placeholders.ts: variants = [path, path + "/"] will generate a "//" variant when path already ends with /. Harmless, but easy to tidy.
• packages/ui/src/components/unified-picker.tsx: root FileItem object is duplicated (created in allItems() and again in the click handler). Not a blocker.

Net assessment

• The core correctness/safety issue from earlier (accidentally stripping @ from normal mentions/emails/agents) is resolved in 4b967d1.
• The remaining blockers are mostly i18n/user-visible string hygiene in unified-picker.tsx + the path: chip display convention.

--
Yours,
CodeNomadBot

[VooDisss]

Done! All tests passed.

---

Commit: 32113ea

Testing Protocol - ALL PASSED ✓

Root Path Handling

• 1.1 @./ via ENTER → Send → ./ ✓
• 1.2 @. via ENTER → Send → ./ ✓
• 1.3 Send both @. and @./ → Send → ./ ./ ✓

Path Sending (ENTER)

• 2.1 @docs/ → sends as ./docs/ ✓
• 2.2 @file.txt → sends as ./file.txt ✓

Path Sending (SHIFT+ENTER)

• 3.1 @docs/ → chip + sends as ./docs/ ✓

Email & Mention Preservation

• 4.1 @user123 → keeps @user123 ✓
• 4.2 test@email.com → keeps @ ✓

---

All manual tests passed. Ready for PR merge.

[VooDisss]

Minor Fixes Applied - Commit b7f638f

Changes:

1. Added i18n translation key for workspace root display
   • Added "unifiedPicker.sections.workspaceRoot": "WORKSPACE ROOT" to English messages
   • Updated unified-picker.tsx to use i18n instead of hardcoded text

[shantur]

@CodeNomadBot

Review complete changes of the PR.

Focus on

Code Quality
Any issues
Any high / critical changes to be done

Please provide your approval

[codenomadbot]

Full PR review (latest head b7f638f).

What I understood the request to be

• Review the complete PR changes (quality + issues + high/critical), and state whether I approve.

Code Quality

• The overall architecture is reasonable: picker emits an explicit PickerSelectAction, and prompt logic in usePromptPicker.ts branches cleanly on it.
• Using display: "path: ..." text attachments as a SHIFT+ENTER marker is pragmatic (enables chips + deterministic send-time behavior).

High / critical issues (blocking approval)

• packages/ui/src/lib/prompt-placeholders.ts: several replacements are not global, which can leave placeholders/mentions unconverted when the same token appears multiple times.
  • Step 4 uses result.replace("___ROOT___", "./") and result.replace("___ROOT_NOSLASH___", "./") (string replace = first occurrence only). If the prompt contains multiple @./ or @. tokens, later occurrences will remain as ___ROOT___ / ___ROOT_NOSLASH___ and get sent to the API.
  • Step 3 uses result = result.replace("@" + withoutPrefix, withPrefix) (also first occurrence only). Multiple instances of the same attached path won’t all be rewritten.
  • This is user-visible and can corrupt the message text being sent.
  • Fix: use replaceAll(...) (or split/join) for string patterns, and use /g for placeholders on the way back.

Non-blocking issues / follow-ups

• i18n: unifiedPicker.sections.workspaceRoot was added only for English (packages/ui/src/lib/i18n/messages/en/commands.ts). Other locales will fall back to English per our i18n layer, but ideally add the key to other locales too.
• UI text: SHIFT+ENTER chips still display path: ... (English) to users; consider localizing or using a non-user-visible marker.

Approval

• Not approved yet due to the non-global replacement bug in packages/ui/src/lib/prompt-placeholders.ts (above). Once that is fixed, I’m comfortable approving.

--
Yours,
CodeNomadBot

[shantur]

Tested manually and it works as expected.

Thanks @VooDisss