diff --git a/apps/bootstrap-installer/src-tauri/src/paths.rs b/apps/bootstrap-installer/src-tauri/src/paths.rs index 0eec8ccd319db..3a7b1b0dbf5fe 100644 --- a/apps/bootstrap-installer/src-tauri/src/paths.rs +++ b/apps/bootstrap-installer/src-tauri/src/paths.rs @@ -98,6 +98,12 @@ pub fn update_in_progress_marker() -> PathBuf { /// that path), where copying onto ourselves would be a Windows sharing /// violation. Best-effort: a failure here must not fail the install, so the /// caller logs and continues. +/// +/// NOTE: because of that no-op, a user's staged installer is only ever written +/// by a full install/repair. Every later `--update` runs the ORIGINAL binary, +/// so an installer-protocol change can strand the whole installed base on a +/// binary that predates it (see `restage_from_checkout`, which repairs this +/// from the freshly-updated checkout). pub fn copy_self_to_hermes_home() -> std::io::Result<()> { let src = std::env::current_exe()?; let dest = installer_dest(); diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 72e6960c96261..834a67ece3664 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -201,7 +201,11 @@ import { sandboxPreflight } from './update-relaunch' import { isOfficialSshRemote, OFFICIAL_REPO_HTTPS_URL } from './update-remote' -import { resolveStagedUpdaterBinary, spawnUpdaterProcess } from './updater-process' +import { + resolveStagedUpdaterBinary, + spawnUpdaterProcess, + stagedUpdaterSupportsPrewrittenMarker +} from './updater-process' import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers } from './venv-blocker-scan' import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace' import { createWakeIndicatorWindowController } from './wake-indicator-window' @@ -3003,8 +3007,20 @@ async function applyUpdates(opts = {}) { // the venv. By writing the marker ourselves the renderer's // waitForUpdateToFinish() gate sees a live update and parks instead. // The updater overwrites this with its own PID later; same format. - if (Number.isInteger(child.pid)) { + // + // SKIPPED for pre-#74782 staged updaters: those have no self-PID + // exclusion, so they read this very marker as a foreign live owner and + // abort with "Another Hermes update is already running (PID )" — + // an unbreakable loop, because the update that would replace the stale + // binary is the one being refused. Losing the anti-respawn hardening is + // strictly better than never updating again, and the updater still writes + // its own marker moments later. + if (Number.isInteger(child.pid) && stagedUpdaterSupportsPrewrittenMarker(updater)) { writeUpdateMarker(HERMES_HOME, child.pid) + } else if (Number.isInteger(child.pid)) { + rememberLog( + `[updates] skipping marker pre-write: staged updater predates self-adopt (${updater}); it would refuse its own claim` + ) } rememberLog(`[updates] launched updater: ${updater} ${updaterArgs.join(' ')}; exiting desktop to release venv shim`) @@ -3091,9 +3107,15 @@ async function handOffWindowsBootstrapRecovery(reason) { // Same marker pre-write as applyUpdates — see comment there. The recovery // hand-off has the same window where the renderer can respawn a backend - // before the updater writes its own marker. - if (Number.isInteger(child.pid)) { + // before the updater writes its own marker, and the same stale-updater + // exclusion: a pre-#74782 binary would refuse its own pre-written claim and + // strand the very recovery meant to heal the install. + if (Number.isInteger(child.pid) && stagedUpdaterSupportsPrewrittenMarker(updater)) { writeUpdateMarker(HERMES_HOME, child.pid) + } else if (Number.isInteger(child.pid)) { + rememberLog( + `[bootstrap] skipping marker pre-write: staged updater predates self-adopt (${updater}); it would refuse its own claim` + ) } rememberLog( diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts index fd0f6ecea0503..3855cdab22739 100644 --- a/apps/desktop/electron/remote-lifecycle.test.ts +++ b/apps/desktop/electron/remote-lifecycle.test.ts @@ -510,7 +510,10 @@ test('connect() respawns when the requested remote profile differs from the lock ) assert.equal(result.reused, false) - assert.ok(ssh.calls.some(c => /setsid/.test(c)), 'profile mismatch must spawn a fresh dashboard') + assert.ok( + ssh.calls.some(c => /setsid/.test(c)), + 'profile mismatch must spawn a fresh dashboard' + ) }) test('connect() respawns when the lockfile hermesPath differs from the resolved path', async () => { diff --git a/apps/desktop/electron/updater-process.test.ts b/apps/desktop/electron/updater-process.test.ts index 96f48eb9ff016..e781fe3efa3ec 100644 --- a/apps/desktop/electron/updater-process.test.ts +++ b/apps/desktop/electron/updater-process.test.ts @@ -4,7 +4,65 @@ import path from 'node:path' import { test } from 'vitest' -import { resolveStagedUpdaterBinary, spawnUpdaterProcess } from './updater-process' +import { + MARKER_SELF_ADOPT_EPOCH_MS, + resolveStagedUpdaterBinary, + spawnUpdaterProcess, + stagedUpdaterSupportsPrewrittenMarker +} from './updater-process' + +const DAY_MS = 24 * 60 * 60 * 1000 + +test('stagedUpdaterSupportsPrewrittenMarker rejects installers predating the self-adopt fix', () => { + // The real-world trap: an installer staged at first install months ago, never + // refreshed because copy_self_to_hermes_home no-ops during --update. + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS - 60 * DAY_MS + }), + false + ) +}) + +test('stagedUpdaterSupportsPrewrittenMarker accepts installers from the fix onward', () => { + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS + }), + true + ) + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS + 30 * DAY_MS + }), + true + ) +}) + +test('stagedUpdaterSupportsPrewrittenMarker treats an unreadable mtime as unsupported', () => { + // Bias toward the path that can always make progress: a skipped pre-write + // loses anti-respawn hardening, a wedged updater can never update again. + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => null + }), + false + ) +}) + +test('resolveStagedUpdaterBinary still returns a stale staged updater on Windows', () => { + // Staleness gates only the marker PRE-WRITE, never the hand-off itself: + // the stale binary is the only updater these users have, and it works fine + // once it is allowed to write its own claim. + assert.equal( + resolveStagedUpdaterBinary('C:\\Hermes', { + fileExists: () => true, + isWindows: true, + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS - 60 * DAY_MS + }), + path.join('C:\\Hermes', 'hermes-setup.exe') + ) +}) test('spawnUpdaterProcess hides the updater console and detaches the child on Windows', () => { const calls: Array<{ args: string[]; command: string; options: SpawnOptions }> = [] diff --git a/apps/desktop/electron/updater-process.ts b/apps/desktop/electron/updater-process.ts index 56352da3e8534..97b1d9651afa8 100644 --- a/apps/desktop/electron/updater-process.ts +++ b/apps/desktop/electron/updater-process.ts @@ -12,8 +12,20 @@ export interface UpdaterChild { export interface ResolveStagedUpdaterBinaryDeps { isWindows?: boolean fileExists?: (candidate: string) => boolean + stagedMtimeMs?: (candidate: string) => number | null } +/** + * Staged installers older than this have no self-PID exclusion in + * `UpdateMarkerGuard::acquire` and will refuse an update whose marker was + * pre-written on their behalf. + * + * The self-adopt fix landed in #74782 / 160586ff8 (2026-07-30 17:57 +0700). + * We compare against the start of 2026-07-31 UTC so the boundary is + * unambiguous for binaries staged that same day. + */ +export const MARKER_SELF_ADOPT_EPOCH_MS = Date.UTC(2026, 6, 31) + function stagedFileExists(candidate: string): boolean { try { return statSync(candidate).isFile() @@ -22,6 +34,14 @@ function stagedFileExists(candidate: string): boolean { } } +function stagedFileMtimeMs(candidate: string): number | null { + try { + return statSync(candidate).mtimeMs + } catch { + return null + } +} + /** * Decide which staged installer binary — if any — may be handed an update. * @@ -61,6 +81,37 @@ export function resolveStagedUpdaterBinary( return fileExists(candidate) ? candidate : null } +/** + * True when the staged installer is new enough to survive a pre-written marker. + * + * `copy_self_to_hermes_home` deliberately no-ops during `--update` + * (apps/bootstrap-installer/src-tauri/src/paths.rs), so the binary staged by a + * user's ORIGINAL install orchestrates every later update — forever. Installers + * predating #74782 have no self-PID exclusion in `UpdateMarkerGuard::acquire`, + * so when the desktop pre-writes the marker naming that very updater, the + * updater reads its own claim as a foreign live owner and aborts with + * "Another Hermes update is already running (PID , started 1s ago)" — + * the observed infinite "Install didn't finish" loop. Skipping the pre-write + * for those binaries lets them acquire cleanly and run `hermes update`, which + * pulls the permanent fixes. See shouldPrewriteUpdateMarker. + * + * We cannot ask the binary its version without executing it, so use its mtime: + * the installer is written to HERMES_HOME at install/repair time, making mtime + * a faithful stamp of which installer generation produced it. + * + * Unreadable mtime counts as UNSUPPORTED — the pre-write is a best-effort + * hardening, while a wedged updater is unrecoverable, so we bias toward the + * path that can always make progress. + */ +export function stagedUpdaterSupportsPrewrittenMarker( + candidate: string, + deps: ResolveStagedUpdaterBinaryDeps = {} +): boolean { + const mtimeMs = (deps.stagedMtimeMs ?? stagedFileMtimeMs)(candidate) + + return typeof mtimeMs === 'number' && Number.isFinite(mtimeMs) && mtimeMs >= MARKER_SELF_ADOPT_EPOCH_MS +} + export interface SpawnUpdaterProcessDeps { isWindows?: boolean spawnProcess?: (command: string, args: string[], options: SpawnOptions) => UpdaterChild diff --git a/apps/desktop/src/app/chat/composer/directive-actions.test.tsx b/apps/desktop/src/app/chat/composer/directive-actions.test.tsx new file mode 100644 index 0000000000000..4c827fe9a164a --- /dev/null +++ b/apps/desktop/src/app/chat/composer/directive-actions.test.tsx @@ -0,0 +1,150 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { I18nProvider } from '@/i18n' + +import { ComposerDirectiveActions } from './directive-actions' +import { refChipElement } from './rich-editor' + +const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] } + +const openSession = vi.fn() + +vi.mock('@/app/open-session', () => ({ openSession: (...args: unknown[]) => openSession(...args) })) + +/** A live contenteditable holding real chips, with the watcher bound to it — + * the same pair both composers mount. */ +function mountEditor(chips: { kind: string; value: string }[]) { + const editor = document.createElement('div') + + editor.contentEditable = 'true' + editor.append(...chips.map(chip => refChipElement(chip.kind, `\`${chip.value}\``))) + document.body.append(editor) + + render( + + + + ) + + return editor +} + +function chips(editor: HTMLElement, kind: string) { + return Array.from(editor.querySelectorAll(`[data-ref-kind="${kind}"]`)) +} + +function hover(node: Element) { + fireEvent.pointerOver(node, { bubbles: true }) +} + +/** The reference the visible action pill points at, or null when there is none. */ +function pillValue() { + return document.querySelector('[data-slot="composer-directive-action"]')?.getAttribute('data-value') ?? null +} + +afterEach(() => { + cleanup() + document.body.replaceChildren() + delete desktopWindow.hermesDesktop + openSession.mockReset() + vi.useRealTimers() +}) + +describe('ComposerDirectiveActions', () => { + it('offers an action for a hovered actionable chip', () => { + const editor = mountEditor([{ kind: 'url', value: 'https://example.com/docs' }]) + + expect(pillValue()).toBeNull() + + hover(chips(editor, 'url')[0]!) + + expect(pillValue()).toBe('https://example.com/docs') + }) + + it('opens a url externally rather than navigating the app', () => { + const openExternal = vi.fn().mockResolvedValue(undefined) + + desktopWindow.hermesDesktop = { openExternal } as unknown as Window['hermesDesktop'] + + const editor = mountEditor([{ kind: 'url', value: 'https://example.com/docs' }]) + + hover(chips(editor, 'url')[0]!) + fireEvent.click(screen.getByRole('button')) + + expect(openExternal).toHaveBeenCalledWith('https://example.com/docs') + expect(pillValue()).toBeNull() + }) + + it('runs the kind-specific action — a session chip opens the session', async () => { + const editor = mountEditor([{ kind: 'session', value: 'default/20260722_204335_d62c16' }]) + + hover(chips(editor, 'session')[0]!) + fireEvent.click(screen.getByRole('button')) + // openSessionRef lazy-imports the navigator, so the call lands a tick later. + await vi.waitFor(() => + expect(openSession).toHaveBeenCalledWith('20260722_204335_d62c16', expect.any(Function), 'tab') + ) + }) + + it('leaves kinds with no action alone', () => { + const editor = mountEditor([{ kind: 'file', value: 'src/main.tsx' }]) + + hover(chips(editor, 'file')[0]!) + + expect(pillValue()).toBeNull() + }) + + it('follows the pointer from one chip to the next', () => { + const editor = mountEditor([ + { kind: 'url', value: 'https://one.example' }, + { kind: 'url', value: 'https://two.example' } + ]) + + const [first, second] = chips(editor, 'url') + + hover(first!) + + expect(pillValue()).toBe('https://one.example') + + hover(second!) + + expect(pillValue()).toBe('https://two.example') + }) + + it('keeps the pill up while the pointer crosses onto it', () => { + vi.useFakeTimers() + + const editor = mountEditor([{ kind: 'url', value: 'https://example.com' }]) + const chip = chips(editor, 'url')[0]! + + hover(chip) + fireEvent.pointerOut(chip, { relatedTarget: document.body }) + fireEvent.mouseEnter(screen.getByRole('button').parentElement!) + vi.advanceTimersByTime(500) + + expect(pillValue()).toBe('https://example.com') + }) + + it('binds to the document so a late-attached editor still gets the affordance', () => { + // The edit composer's editor isn't reliably in the DOM when the effect + // first runs; a document listener that reads the editor lazily works + // regardless — this is the whole reason it binds to document, not editor. + const editor = document.createElement('div') + + editor.contentEditable = 'true' + editor.append(refChipElement('url', '`https://late.example`')) + + render( + + + + ) + + // Editor attached AFTER mount. + document.body.append(editor) + hover(editor.querySelector('[data-ref-kind="url"]')!) + + expect(pillValue()).toBe('https://late.example') + }) +}) diff --git a/apps/desktop/src/app/chat/composer/directive-actions.tsx b/apps/desktop/src/app/chat/composer/directive-actions.tsx new file mode 100644 index 0000000000000..b18ade2aac039 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/directive-actions.tsx @@ -0,0 +1,154 @@ +/** + * Hover actions for directive chips in a composer. + * + * A directive chip (`@url:`, `@session:`, …) reads as the thing it points at + * and is coloured like one, but a composer is an editor — a click inside the + * contenteditable only places the caret, so there's no way to *act* on the + * reference. Instead, hovering a chip whose kind has an action floats a small + * pill above it that runs it. + * + * The kind → action table (`DIRECTIVE_ACTIONS`) lives in `directive-text`, so + * it is shared with the sent-message chip: one entry lights up both surfaces. + */ +import { type RefObject, useCallback, useEffect, useRef, useState } from 'react' +import { createPortal } from 'react-dom' + +import { DIRECTIVE_ACTIONS, type DirectiveAction } from '@/components/assistant-ui/directive-text' +import { composerFloatingPill } from '@/components/chat/composer-dock' +import { Codicon } from '@/components/ui/codicon' +import { useI18n } from '@/i18n' +import { cn } from '@/lib/utils' + +/** Moving between the chip and the pill crosses a gap where neither is hovered. + * Short enough that it still reads as instant on the way out. */ +const HIDE_DELAY_MS = 120 + +/** The actionable directive chip under `target` that also belongs to `editor`, + * if there is one. */ +function actionableChipAt(target: EventTarget | null, editor: HTMLElement): HTMLElement | null { + const chip = target instanceof Element ? target.closest('[data-ref-kind]') : null + const kind = chip?.dataset.refKind + + return chip && kind && chip.dataset.refId && editor.contains(chip) && DIRECTIVE_ACTIONS[kind] ? chip : null +} + +interface Anchor { + action: DirectiveAction + chip: HTMLElement + left: number + top: number + value: string +} + +function anchorFor(chip: HTMLElement): Anchor | null { + const value = chip.dataset.refId + const action = chip.dataset.refKind ? DIRECTIVE_ACTIONS[chip.dataset.refKind] : undefined + + if (!value || !action || !chip.isConnected) { + return null + } + + const rect = chip.getBoundingClientRect() + + return { action, chip, left: rect.left, top: rect.top, value } +} + +/** + * Renders the action pill for whichever actionable chip in `editorRef` is + * hovered. + * + * Listeners bind to `document`, not the editor, so mount timing can't strand + * them: the edit composer's contenteditable isn't reliably attached when this + * effect first runs, and a document listener that reads the editor lazily works + * regardless. Each instance filters to its own editor, so the docked and edit + * composers never show two pills for one chip. + */ +export function ComposerDirectiveActions({ editorRef }: { editorRef: RefObject }) { + const { t } = useI18n() + const [anchor, setAnchor] = useState(null) + const hideTimerRef = useRef(undefined) + + const cancelHide = useCallback(() => { + window.clearTimeout(hideTimerRef.current) + }, []) + + const hideSoon = useCallback(() => { + cancelHide() + hideTimerRef.current = window.setTimeout(() => setAnchor(null), HIDE_DELAY_MS) + }, [cancelHide]) + + useEffect(() => { + const onPointerOver = (event: PointerEvent) => { + const editor = editorRef.current + const chip = editor && actionableChipAt(event.target, editor) + + if (!chip) { + return + } + + cancelHide() + setAnchor(current => (current?.chip === chip ? current : anchorFor(chip))) + } + + const onPointerOut = (event: PointerEvent) => { + const editor = editorRef.current + const chip = editor && actionableChipAt(event.target, editor) + + // A move within the same chip (its icon → its label) is not a leave. + if (chip && editor && chip === actionableChipAt(event.relatedTarget, editor)) { + return + } + + hideSoon() + } + + // The chip can move or vanish under a parked pointer: the editor scrolls, + // the window resizes, or the user deletes the reference the pill points at. + const reanchor = () => setAnchor(current => (current ? anchorFor(current.chip) : null)) + + document.addEventListener('pointerover', onPointerOver) + document.addEventListener('pointerout', onPointerOut) + window.addEventListener('scroll', reanchor, true) + window.addEventListener('resize', reanchor) + + return () => { + document.removeEventListener('pointerover', onPointerOver) + document.removeEventListener('pointerout', onPointerOut) + window.removeEventListener('scroll', reanchor, true) + window.removeEventListener('resize', reanchor) + window.clearTimeout(hideTimerRef.current) + } + }, [cancelHide, editorRef, hideSoon]) + + if (!anchor) { + return null + } + + return createPortal( +
+ +
, + document.body + ) +} diff --git a/apps/desktop/src/app/chat/composer/empty-composer.test.ts b/apps/desktop/src/app/chat/composer/empty-composer.test.ts index 60498efcd0b1f..dcf3398e6bcb5 100644 --- a/apps/desktop/src/app/chat/composer/empty-composer.test.ts +++ b/apps/desktop/src/app/chat/composer/empty-composer.test.ts @@ -1,6 +1,13 @@ import { describe, expect, it } from 'vitest' -import { composerPlainText, normalizeComposerEditorDom, renderComposerContents, RICH_INPUT_SLOT } from './rich-editor' +import { + beginComposerComposition, + composerPlainText, + deleteChipBeforeCaret, + normalizeComposerEditorDom, + renderComposerContents, + RICH_INPUT_SLOT +} from './rich-editor' function editor(): HTMLDivElement { const el = document.createElement('div') @@ -131,4 +138,133 @@ describe('an emptied composer shows its placeholder again', () => { expect(el.matches(PLACEHOLDER_SHOWS)).toBe(true) }) + + // Chromium leaves zero-length text nodes behind whenever an edit lands next + // to a contenteditable=false chip. They render as nothing, so an editor + // holding only those is empty to the user — counting them as contents left + // the placeholder hidden under a composer that looked blank. + it('advertises emptiness for an editor holding only zero-length text nodes', () => { + const el = editor() + + el.append(document.createTextNode(''), document.createTextNode('')) + normalizeComposerEditorDom(el) + + expect(el.matches(PLACEHOLDER_SHOWS)).toBe(true) + }) + + it('does not advertise emptiness while real text sits beside that litter', () => { + const el = editor() + + el.append(document.createTextNode(''), document.createTextNode('one'), document.createTextNode('')) + normalizeComposerEditorDom(el) + + expect(el.matches(PLACEHOLDER_SHOWS)).toBe(false) + }) + + // Input events are skipped for the duration of an IME composition, so nothing + // else clears the marker until it ends — the hint would sit behind the + // hiragana the user is composing (#75960). + it('hides the placeholder before IME preedit text starts', () => { + const el = emptied() + + beginComposerComposition(el) + + expect(el.matches(PLACEHOLDER_SHOWS)).toBe(false) + }) + + it('brings the placeholder back when composition ends with nothing committed', () => { + const el = emptied() + + beginComposerComposition(el) + normalizeComposerEditorDom(el) + + expect(el.matches(PLACEHOLDER_SHOWS)).toBe(true) + }) +}) + +/** A directive chip, as `refChipElement` builds it. */ +function chip(): HTMLSpanElement { + const el = document.createElement('span') + + el.contentEditable = 'false' + el.dataset.refText = '@folder:`apps/desktop/`' + el.append(document.createTextNode('apps/desktop/')) + + return el +} + +function caretAt(node: Node, offset: number) { + const range = document.createRange() + + range.setStart(node, offset) + range.collapse(true) + + const selection = window.getSelection() + + selection?.removeAllRanges() + selection?.addRange(range) +} + +/** Committing a completion empties the typed token's text node instead of + * removing it, and `Range.insertNode` splits the line around the caret — so a + * freshly-chipped directive sits between zero-length text nodes. Backspace has + * to see past them or the chip can't be deleted at all. */ +describe('backspace deletes a chip surrounded by Chromium litter', () => { + it('deletes the chip when the caret sits in a zero-length text node after it', () => { + const el = editor() + + el.append(document.createTextNode(''), chip(), document.createTextNode('')) + caretAt(el.childNodes[2] as Node, 0) + + expect(deleteChipBeforeCaret(el)).toBe(true) + expect(el.querySelector('[data-ref-text]')).toBeNull() + }) + + it('deletes the chip when the caret is past a zero-length text node at editor level', () => { + const el = editor() + + el.append(chip(), document.createTextNode('')) + caretAt(el, 2) + + expect(deleteChipBeforeCaret(el)).toBe(true) + expect(el.querySelector('[data-ref-text]')).toBeNull() + }) + + it('still swallows the auto-inserted trailing space through that litter', () => { + const el = editor() + + el.append(chip(), document.createTextNode(''), document.createTextNode(' ')) + caretAt(el, 2) + + expect(deleteChipBeforeCaret(el)).toBe(true) + expect(composerPlainText(el)).toBe('') + }) + + it('keeps real following text when it deletes the chip', () => { + const el = editor() + + el.append(chip(), document.createTextNode(''), document.createTextNode(' and this')) + caretAt(el, 2) + + expect(deleteChipBeforeCaret(el)).toBe(true) + expect(composerPlainText(el)).toBe('and this') + }) + + it('leaves plain text to the native backspace', () => { + const el = editor() + + el.append(document.createTextNode('hello')) + caretAt(el.firstChild as Node, 5) + + expect(deleteChipBeforeCaret(el)).toBe(false) + }) + + it('sweeps the litter out of the editor when it normalizes', () => { + const el = editor() + + el.append(document.createTextNode(''), chip(), document.createTextNode('')) + normalizeComposerEditorDom(el) + + expect(Array.from(el.childNodes).map(node => node.nodeName)).toEqual(['SPAN']) + }) }) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 747177592288a..3767a4ad4e4e1 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -32,6 +32,7 @@ import { import { ContextMenu } from './context-menu' import { COMPOSER_AREAS, runComposerMiddleware } from './contrib' import { ComposerControls } from './controls' +import { ComposerDirectiveActions } from './directive-actions' import { COMPOSER_DROP_ACTIVE_CLASS, COMPOSER_DROP_FADE_CLASS } from './drop-affordance' import { markActiveComposer } from './focus' import { HelpHint } from './help-hint' @@ -57,7 +58,7 @@ import { ActionBadges } from './micro-actions' import { chipTypedPathOnSpace, pathifyRefs } from './path-refs' import { QueuePanel } from './queue-panel' import { - COMPOSER_PLACEHOLDER_CLASS, + beginComposerComposition, composerPlainText, deleteChipBeforeCaret, deleteSelectionInEditor, @@ -947,7 +948,6 @@ export function ChatBar({ autoCorrect="off" className={cn( 'min-h-[1.625rem] min-h-(--composer-input-min-height) max-h-(--composer-input-max-height) cursor-text overflow-y-auto whitespace-pre-wrap break-words [overflow-wrap:anywhere] bg-transparent pb-1 pr-1 pt-1 leading-normal text-foreground outline-none disabled:cursor-not-allowed', - COMPOSER_PLACEHOLDER_CLASS, '**:data-ref-text:cursor-default', stacked && 'pl-3', stacked ? 'w-full' : 'min-w-(--composer-input-inline-min-width) flex-1' @@ -969,8 +969,13 @@ export function ChatBar({ // until an unrelated edit forces a sync (#39614). flushEditorToDraft(event.currentTarget) }} - onCompositionStart={() => { + onCompositionStart={event => { composingRef.current = true + + // Input events are skipped for the rest of the composition, so + // nothing else would clear the empty marker until it ends — and the + // hint would sit behind the preedit text the whole time (#75960). + beginComposerComposition(event.currentTarget) }} onDragOver={handleInputDragOver} onDrop={handleInputDrop} @@ -985,6 +990,7 @@ export function ChatBar({ spellCheck={false} suppressContentEditableWarning /> + {/* assistant-ui requires ComposerPrimitive.Input somewhere in the tree so the composer-state binding (text + IME + paste + form-submit hookup) wires up. We render the real input UI ourselves above via the diff --git a/apps/desktop/src/app/chat/composer/micro-actions.tsx b/apps/desktop/src/app/chat/composer/micro-actions.tsx index 6f8cc77ad566c..65e3729eadad4 100644 --- a/apps/desktop/src/app/chat/composer/micro-actions.tsx +++ b/apps/desktop/src/app/chat/composer/micro-actions.tsx @@ -1,5 +1,6 @@ import { memo, useState } from 'react' +import { composerFloatingPill } from '@/components/chat/composer-dock' import { Codicon } from '@/components/ui/codicon' import { useSessionSlice } from '@/lib/use-session-slice' import { cn } from '@/lib/utils' @@ -7,11 +8,9 @@ import { $composerActionsBySession, type ComposerAction } from '@/store/composer import { notifyError } from '@/store/notifications' /** - * Floating pill — the treatment the thread's jump/approval button uses for a - * control that sits over scrolling content: full radius, hairline border, the - * shared composer fill behind a blur so thread text never bleeds through. - * Sized against the composer's own control height so a row of pills lines up - * with the chrome it floats above. + * Floating pill — the shared treatment for a control that sits over the + * composer (`composerFloatingPill`), plus this strip's own width cap and + * disabled state. * * NEVER `pointer-events-none`, not even when disabled. The pop-out drag region * is an `absolute` sibling behind these pills, so a pill that stops taking @@ -19,10 +18,8 @@ import { notifyError } from '@/store/notifications' * becomes a grab handle that floats the composer. */ const PILL = cn( - 'inline-flex h-(--composer-control-size) max-w-56 shrink-0 cursor-pointer items-center gap-1.5 rounded-full px-2.5', - 'border border-border/65 bg-(--composer-fill) backdrop-blur-[0.75rem] [-webkit-backdrop-filter:blur(0.75rem)]', - 'text-xs font-normal text-(--ui-text-secondary) transition-colors', - 'hover:bg-(--chrome-action-hover) hover:text-foreground', + composerFloatingPill, + 'max-w-56', 'disabled:cursor-default disabled:opacity-50 disabled:hover:bg-(--composer-fill)', 'focus-visible:outline-none focus-visible:ring-[0.1875rem] focus-visible:ring-ring/50' ) diff --git a/apps/desktop/src/app/chat/composer/rich-editor.ts b/apps/desktop/src/app/chat/composer/rich-editor.ts index 0c9c7098870c8..a7d8e0406759b 100644 --- a/apps/desktop/src/app/chat/composer/rich-editor.ts +++ b/apps/desktop/src/app/chat/composer/rich-editor.ts @@ -21,28 +21,67 @@ import { slashCommandMatches, type SlashCommandScanOptions } from './slash-refs' export const RICH_INPUT_SLOT = 'composer-rich-input' -/** Paints `data-placeholder` while the editor is empty. +/** Chromium's litter: editing beside a `contenteditable=false` chip splits the + * line and leaves zero-length text nodes behind. They render as nothing and + * serialize as nothing, so no reader of the editor should count them. */ +function isEmptyTextNode(node: ChildNode | null): boolean { + return node?.nodeType === Node.TEXT_NODE && !node.textContent +} + +/** The node before `node`, stepping over that litter. */ +function meaningfulPreviousSibling(node: ChildNode | null): ChildNode | null { + let prev = node?.previousSibling ?? null + + while (isEmptyTextNode(prev)) { + prev = prev?.previousSibling ?? null + } + + return prev +} + +/** The node after `node`, stepping over that litter. */ +function meaningfulNextSibling(node: ChildNode | null): ChildNode | null { + let next = node?.nextSibling ?? null + + while (isEmptyTextNode(next)) { + next = next?.nextSibling ?? null + } + + return next +} + +/** Keep the `data-empty` marker the placeholder paints on in step with the + * editor root's contents. * * `:empty` can't be the whole test: a cleared editor keeps a scaffolding
* so the contenteditable doesn't collapse, and that break makes `:empty` * false. Nor can CSS infer it on its own — a text node is invisible to * selectors, so `one
` and a lone `
` are the same shape, and - * `:has(> br:only-child)` would paint the placeholder straight over the - * user's text. The code that empties the editor is what knows, so it marks it. + * `:has(> br:only-child)` would paint the placeholder over the user's text. + * The code that empties the editor is what knows, so it marks it. * - * @see markEditorEmptiness */ -export const COMPOSER_PLACEHOLDER_CLASS = - '[&:is(:empty,[data-empty])]:before:content-[attr(data-placeholder)] [&:is(:empty,[data-empty])]:before:text-muted-foreground/60' - -/** Keep that marker in step with the editor root's contents. */ + * Zero-length text nodes don't count as contents. Chromium leaves them behind + * whenever an edit lands next to a `contenteditable=false` chip, and counting + * them left an editor the user had emptied looking occupied. */ export function markEditorEmptiness(editor: HTMLElement) { - if (editor.childNodes.length === 0) { + if (Array.from(editor.childNodes).every(isEmptyTextNode)) { editor.dataset.empty = '' } else { delete editor.dataset.empty } } +/** Drop the marker as IME composition starts, before any preedit text lands. + * + * Input events during composition are deliberately skipped (they carry + * uncommitted preedit text), so nothing else clears the marker until + * `compositionend` — and the hint would otherwise sit behind the hiragana the + * user is composing. `normalizeComposerEditorDom` restores it if composition + * ends with nothing committed. */ +export function beginComposerComposition(editor: HTMLElement) { + delete editor.dataset.empty +} + /** @see referenceRe — the shared pattern every surface recognises a reference * with. Module-level `/g` regexes carry `lastIndex`, so call sites reset it. */ export const REF_RE = referenceRe() @@ -407,7 +446,14 @@ export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment /** Backspace at a collapsed caret immediately after a chip: delete the chip AND * the single trailing space we auto-insert after it, atomically — so removing a * directive never strands an orphaned space (the contenteditable-driven cleanup - * was unreliable). Returns whether it ran. */ + * was unreliable). Returns whether it ran. + * + * "Immediately after" has to be read through Chromium's litter. Committing a + * completion empties the typed token's text node rather than removing it, and + * `Range.insertNode` splits around the caret, so the chip routinely sits + * between zero-length text nodes. Reading those as content made the caret look + * like it was after plain text; the delete declined and Chromium's own + * backspace bounced between the leftovers instead of removing the chip. */ export function deleteChipBeforeCaret(editor: HTMLElement): boolean { const hit = composerSelectionRange(editor) @@ -419,16 +465,20 @@ export function deleteChipBeforeCaret(editor: HTMLElement): boolean { let chip: ChildNode | null = null if (startContainer === editor) { - chip = startOffset > 0 ? editor.childNodes[startOffset - 1] : null + chip = startOffset > 0 ? (editor.childNodes[startOffset - 1] ?? null) : null + + if (isEmptyTextNode(chip)) { + chip = meaningfulPreviousSibling(chip) + } } else if (startContainer.nodeType === Node.TEXT_NODE && startOffset === 0) { - chip = startContainer.previousSibling + chip = meaningfulPreviousSibling(startContainer as ChildNode) } if (chip?.nodeType !== Node.ELEMENT_NODE || !(chip as HTMLElement).dataset.refText) { return false } - const after = chip.nextSibling + const after = meaningfulNextSibling(chip) chip.remove() // Drop the auto-inserted trailing space; keep any real following text. @@ -666,6 +716,14 @@ function isBlankNode(node: ChildNode | null): boolean { * rendering emits (we use text nodes +
+ chips). Real
line breaks * (Shift+Enter, which sit after actual text) are preserved. */ export function normalizeComposerEditorDom(editor: HTMLElement) { + // Chromium's zero-length text nodes first: every check below reads siblings, + // and litter between them makes a chip look like it has text either side. + for (const child of Array.from(editor.childNodes)) { + if (isEmptyTextNode(child)) { + child.remove() + } + } + // A trailing block wrapper holding only a break/whitespace is the phantom // "new line" Chromium adds after a chip on backspace — drop it. const tailBlock = editor.lastChild as HTMLElement | null diff --git a/apps/desktop/src/app/settings/gateway-settings.test.tsx b/apps/desktop/src/app/settings/gateway-settings.test.tsx index 06f3a99d71468..b81f14c8ea144 100644 --- a/apps/desktop/src/app/settings/gateway-settings.test.tsx +++ b/apps/desktop/src/app/settings/gateway-settings.test.tsx @@ -110,10 +110,12 @@ describe('GatewaySettings', () => { fireEvent.click(screen.getByRole('button', { name: 'Save for next restart' })) await waitFor(() => - expect(saveConnectionConfig).toHaveBeenCalledWith(expect.objectContaining({ - profile: 'work', - sshRemoteProfile: '' - })) + expect(saveConnectionConfig).toHaveBeenCalledWith( + expect.objectContaining({ + profile: 'work', + sshRemoteProfile: '' + }) + ) ) }) }) diff --git a/apps/desktop/src/app/settings/terminal-font-setting.tsx b/apps/desktop/src/app/settings/terminal-font-setting.tsx index f19a554865b42..a1efc8e7d8c51 100644 --- a/apps/desktop/src/app/settings/terminal-font-setting.tsx +++ b/apps/desktop/src/app/settings/terminal-font-setting.tsx @@ -157,7 +157,7 @@ export function TerminalFontSetting() { {copy.terminalFontPreview} - ~/project git:main ❯ + ~/project git:main ❯ } diff --git a/apps/desktop/src/app/shell/model-edit-submenu.test.tsx b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx index 52513c4d00805..3685f8e44aaa1 100644 --- a/apps/desktop/src/app/shell/model-edit-submenu.test.tsx +++ b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx @@ -81,7 +81,13 @@ describe('ModelEditSubmenu reports edits without performing them', () => { it('thinking: toggling back on restores the row level, not the hardcoded default', () => { const onSetOptions = vi.fn() - renderSubmenu({ defaultEffort: 'high', effort: 'none', fastControl: { kind: 'none' }, onSetOptions, reasoning: true }) + renderSubmenu({ + defaultEffort: 'high', + effort: 'none', + fastControl: { kind: 'none' }, + onSetOptions, + reasoning: true + }) fireEvent.click(screen.getByRole('switch')) diff --git a/apps/desktop/src/components/assistant-ui/directive-text.tsx b/apps/desktop/src/components/assistant-ui/directive-text.tsx index 6d149e2dd2d16..538f7770d734d 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.tsx +++ b/apps/desktop/src/components/assistant-ui/directive-text.tsx @@ -6,7 +6,9 @@ import type { FC } from 'react' import { Fragment, useEffect, useMemo, useState } from 'react' import { ZoomableImage } from '@/components/chat/zoomable-image' +import type { I18nContextValue } from '@/i18n' import { extractEmbeddedImages } from '@/lib/embedded-images' +import { openExternalLink } from '@/lib/external-link' import { triggerHaptic } from '@/lib/haptics' import { gatewayMediaDataUrl, isRemoteGateway } from '@/lib/media' import { useSessionLinkTitle } from '@/lib/session-link-title' @@ -442,7 +444,7 @@ const DirectiveImage: FC<{ id: string; label: string }> = ({ id, label }) => { * it's already a tile/main, otherwise open a stacked tab (never steals main * from under the chat you're reading). Lazy-imports so the composer's rich * editor can pull this module in without booting the profile/REST stack. */ -function openSessionRef(value: string) { +export function openSessionRef(value: string) { const { sessionId } = parseSessionRefValue(value) if (!sessionId) { @@ -454,6 +456,33 @@ function openSessionRef(value: string) { void import('@/app/open-session').then(({ openSession }) => openSession(sessionId, () => undefined, 'tab')) } +/** What activating a directive of a given kind does. The single source of truth + * for "you can act on this reference," shared by every surface that renders a + * chip: the composer's hover pill (`ComposerDirectiveActions`) and the sent + * message's clickable chip below. A kind with no entry is inert everywhere. + * + * Add a kind here and both surfaces light up — that's the whole point of one + * table. `icon`/`label` are for the pill; the transcript chip carries its own + * glyph and only reads `run`. */ +export interface DirectiveAction { + icon: string + label: (t: I18nContextValue['t']) => string + run: (value: string) => void +} + +export const DIRECTIVE_ACTIONS: Record = { + session: { + icon: 'link-external', + label: t => t.composer.openDirective, + run: openSessionRef + }, + url: { + icon: 'link-external', + label: t => t.composer.openDirective, + run: openExternalLink + } +} + /** A `@session:/` reference in the user transcript (directive * segments), rendered as a chip like the other composer refs. Clicking it * opens the session as a tab. */ @@ -501,14 +530,18 @@ const SlashChip: FC<{ kind: SlashChipKind; label: string; value: string }> = ({ ) -/** Inert by default; `onClick` promotes the chip to a real button (session - * refs, which open the session they name). */ +/** A directive reference in a sent message. A kind with a `DIRECTIVE_ACTIONS` + * entry (a url, …) renders as a real button that runs it on click; everything + * else is inert text. `onClick` overrides for chips that resolve their target + * themselves (session, which needs the async navigator). */ const DirectiveChip: FC<{ type: string label: string id: string onClick?: () => void }> = ({ type, label, id, onClick }) => { + const activate = onClick ?? (DIRECTIVE_ACTIONS[type] ? () => DIRECTIVE_ACTIONS[type]!.run(id) : undefined) + const body = ( <> @@ -517,14 +550,14 @@ const DirectiveChip: FC<{ ) const props = { - ...refAttrs(type, cn('wrap-anywhere', onClick && 'cursor-pointer')), + ...refAttrs(type, cn('wrap-anywhere', activate && 'cursor-pointer')), 'data-directive-id': id, 'data-slot': 'aui_directive-chip', title: id } - return onClick ? ( - ) : ( diff --git a/apps/desktop/src/components/assistant-ui/session-ref-open.test.tsx b/apps/desktop/src/components/assistant-ui/session-ref-open.test.tsx index d8fe7136aacc4..f10d729d88f67 100644 --- a/apps/desktop/src/components/assistant-ui/session-ref-open.test.tsx +++ b/apps/desktop/src/components/assistant-ui/session-ref-open.test.tsx @@ -12,9 +12,12 @@ vi.mock('@/app/open-session', () => ({ openSession: (...args: unknown[]) => openSession(...args) })) +const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] } + afterEach(() => { cleanup() openSession.mockClear() + delete desktopWindow.hermesDesktop __resetSessionLinkTitleCache() }) @@ -41,3 +44,23 @@ describe('session refs open the session', () => { await vi.waitFor(() => expect(openSession).toHaveBeenCalledWith('20260101_abc123', expect.any(Function), 'tab')) }) }) + +// A url the user sent renders as a chip too, and it opens in the browser — the +// same door the composer's hover pill uses, so a link behaves the same before +// and after send. +describe('url refs open externally', () => { + it('opens a url chip in the user transcript', () => { + const openExternal = vi.fn().mockResolvedValue(undefined) + + desktopWindow.hermesDesktop = { openExternal } as unknown as Window['hermesDesktop'] + + render() + + const chip = screen.getByTitle('https://example.com/docs') + + expect(chip.tagName).toBe('BUTTON') + fireEvent.click(chip) + + expect(openExternal).toHaveBeenCalledWith('https://example.com/docs') + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx index 1e6ec792d016a..dbb90459ccee8 100644 --- a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx @@ -13,6 +13,7 @@ import { useState } from 'react' +import { ComposerDirectiveActions } from '@/app/chat/composer/directive-actions' import { COMPOSER_DROP_ACTIVE_CLASS, COMPOSER_DROP_FADE_CLASS } from '@/app/chat/composer/drop-affordance' import { type ComposerInsertMode, @@ -35,7 +36,6 @@ import { } from '@/app/chat/composer/inline-refs' import { chipTypedPathOnSpace, pathifyRefs } from '@/app/chat/composer/path-refs' import { - COMPOSER_PLACEHOLDER_CLASS, composerPlainText, insertComposerContentsAtCaret, placeCaretEnd, @@ -773,7 +773,6 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess autoCorrect="off" className={cn( 'ui-prompt-input-editor__input max-h-48 w-full resize-none bg-transparent p-0 pr-7 text-[length:var(--conversation-text-font-size)] text-foreground/95 outline-none', - COMPOSER_PLACEHOLDER_CLASS, '**:data-ref-text:cursor-default', expanded ? 'min-h-16' : 'min-h-[1.25rem]' )} @@ -795,6 +794,7 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess spellCheck={false} suppressContentEditableWarning /> + . See markEditorEmptiness in rich-editor.ts. + ───────────────────────────────────────────────────────────────────────── */ +[data-slot='composer-rich-input'] { + position: relative; +} + +[data-slot='composer-rich-input']:is(:empty, [data-empty])::before { + content: attr(data-placeholder); + position: absolute; + inset: 0; + padding: inherit; + overflow: hidden; + color: color-mix(in srgb, var(--muted-foreground) 60%, transparent); + /* One line, clipped — the hint states the field's purpose; it never reflows + the box it sits in. A narrow composer or a long locale string would + otherwise wrap and double the empty composer's height. */ + white-space: nowrap; + text-overflow: ellipsis; + pointer-events: none; + user-select: none; +} + /* Primitive-level pointer cursor for every interactive control (buttons, selects, menu items, switches, tabs, summaries). Keeps individual components from having to hardcode `cursor-pointer`; explicit cursor diff --git a/gateway/relay/ws_transport.py b/gateway/relay/ws_transport.py index f19b39405c00c..a4bfc4f6b2607 100644 --- a/gateway/relay/ws_transport.py +++ b/gateway/relay/ws_transport.py @@ -219,6 +219,12 @@ def _event_from_wire(raw: Dict[str, Any]) -> MessageEvent: # (_is_discord_auto_thread_lane's relay-aware sibling reads these). auto_thread_created=bool(src.get("auto_thread_created", False)), auto_thread_initial_name=src.get("auto_thread_initial_name"), + # Discord auto-thread session continuity: the connector stamps the + # thread id this channel message's reply WILL be auto-threaded into + # (== the message id) so the gateway keys the initiating channel message + # and its later in-thread follow-ups to ONE session. See + # build_session_key / SessionSource.prospective_thread_id. + prospective_thread_id=src.get("prospective_thread_id"), # Authentic upstream-trust signal: this event arrived over the # per-instance-authenticated relay WS, so the connector already resolved # it to this instance's owner-bound author. ``platform`` is the diff --git a/gateway/session.py b/gateway/session.py index cdddace0751a4..ff086fe5badd3 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -191,6 +191,18 @@ class SessionSource: auto_thread_created: bool = False auto_thread_initial_name: Optional[str] = None + # Discord auto-thread session-continuity signal. Set by the connector on an + # inbound CHANNEL message (no thread_id yet) that its auto-thread policy WILL + # deliver into a newly-created thread. A Discord thread created from a message + # reuses that message's id as the thread id, so the connector knows the id + # before the thread exists. The gateway keys the session on this so a + # channel message and its thread follow-ups share ONE session: the channel + # message INITIATES it (keyed on the prospective thread id), and later + # messages arriving in that thread (real thread_id == this value) CONTINUE + # it. Without this, every channel message collapses into one parent-channel + # session and only the first auto-thread ever gets an auto-title/rename. + prospective_thread_id: Optional[str] = None + # Internal, wire-INVISIBLE trust signal: True when this event was delivered # to the gateway over the per-instance-authenticated relay WebSocket (the # Team Gateway connector). The connector authenticates the gateway's socket @@ -268,6 +280,8 @@ class SessionSource: d["auto_thread_created"] = True if self.auto_thread_initial_name: d["auto_thread_initial_name"] = self.auto_thread_initial_name + if self.prospective_thread_id: + d["prospective_thread_id"] = self.prospective_thread_id return d @classmethod @@ -291,6 +305,7 @@ class SessionSource: profile=data.get("profile"), auto_thread_created=bool(data.get("auto_thread_created", False)), auto_thread_initial_name=data.get("auto_thread_initial_name"), + prospective_thread_id=data.get("prospective_thread_id"), ) @@ -1111,20 +1126,37 @@ def build_session_key( # single group member gets two isolated per-user sessions when the # bridge reshuffles alias forms. participant_id = canonical_whatsapp_identifier(str(participant_id)) or participant_id - key_parts = [ns, platform, source.chat_type] + # Discord auto-thread continuity: a channel-initiating message carries no + # thread_id yet, but the connector tells us the thread its reply WILL be + # auto-threaded into (prospective_thread_id == the message id, which becomes + # the thread id). Key the session on that so the initiating channel message + # and every follow-up that later arrives IN that thread (real thread_id == + # prospective_thread_id) resolve to the SAME session — "initiate in channel, + # continue in thread". A real thread_id always wins when present. + # + # The follow-up arrives with chat_type="thread" while the initiating message + # has chat_type="group"/"channel"; normalize the chat_type slot to "thread" + # when keying on a prospective id so the two byte-match. (Real-thread events + # already carry chat_type="thread", so this only rewrites the initiating + # channel message's slot.) + effective_thread_id = source.thread_id or source.prospective_thread_id + chat_type_slot = source.chat_type + if source.prospective_thread_id and not source.thread_id: + chat_type_slot = "thread" + key_parts = [ns, platform, chat_type_slot] if slack_scope_id: key_parts.append(slack_scope_id) if source.chat_id: key_parts.append(source.chat_id) - if source.thread_id: - key_parts.append(source.thread_id) + if effective_thread_id: + key_parts.append(effective_thread_id) # In threads, default to shared sessions (all participants see the same # conversation). Per-user isolation only applies when explicitly enabled # via thread_sessions_per_user, or when there is no thread (regular group). isolate_user = group_sessions_per_user - if source.thread_id and not thread_sessions_per_user: + if effective_thread_id and not thread_sessions_per_user: isolate_user = False if isolate_user and participant_id: diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index 60f2cc9d95252..85256e880a229 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -750,6 +750,10 @@ def _stage_candidate_venv( logger.warning("candidate dependency sync refused: uv.lock is missing") _remove_tree(candidate, boundary=runtime_root) return None + # Locked sync must see project [tool.uv] exclude-newer; --no-config / + # UV_NO_CONFIG drops it and uv 0.12+ refuses --locked. + sync_env = dict(env) + sync_env.pop("UV_NO_CONFIG", None) synced = subprocess.run( [ uv_bin, @@ -759,10 +763,9 @@ def _stage_candidate_venv( "--locked", "--python", str(_venv_python(candidate)), - "--no-config", ], cwd=project_root, - env=env, + env=sync_env, check=False, ) if synced.returncode != 0: diff --git a/hermes_cli/npm_engine.py b/hermes_cli/npm_engine.py index 12c25d64b0e58..c33572894e769 100644 --- a/hermes_cli/npm_engine.py +++ b/hermes_cli/npm_engine.py @@ -5,7 +5,7 @@ pins an ``engines.npm`` range, so an npm outside that range aborts every ``npm ci`` / ``npm install`` we run inside the checkout:: npm error code EBADENGINE - npm error notsup Required: {"node":">=20.0.0","npm":"<11.10.0 || >=12.0.0"} + npm error notsup Required: {"node":">=26.0.0","npm":">=12.0.0"} npm error notsup Actual: {"npm":"10.9.8","node":"v22.23.1"} Rather than predicting the failure (which would mean a semver range matcher and diff --git a/tests/gateway/test_session.py b/tests/gateway/test_session.py index 4e598aa53f4ba..5b48ddd22da52 100644 --- a/tests/gateway/test_session.py +++ b/tests/gateway/test_session.py @@ -742,6 +742,90 @@ class TestWhatsAppSessionKeyConsistency: assert build_session_key(alice) == build_session_key(bob) + def test_discord_prospective_thread_initiates_and_continues_one_session(self): + """Discord auto-thread continuity: a channel-initiating message (no + thread_id, but a connector-supplied prospective_thread_id) and the later + follow-ups that arrive IN that thread (real thread_id == the prospective + id) must resolve to ONE session — "initiate in channel, continue in + thread". This is the fix for every-thread-after-the-first never getting + an auto-title/rename (staging 2026-08-02).""" + # The channel-initiating message: no thread yet, connector says it will + # be threaded into thread id "msg-100" (== the message id). + initiating = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="group", + user_id="cthulhu", + prospective_thread_id="msg-100", + ) + # A follow-up that actually arrives inside that thread. + follow_up = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="thread", + thread_id="msg-100", + user_id="cthulhu", + ) + key_init = build_session_key(initiating) + key_follow = build_session_key(follow_up) + assert key_init.endswith(":msg-100") + assert key_init == key_follow + + def test_discord_distinct_prospective_threads_are_distinct_sessions(self): + """Two different channel messages each initiate their OWN thread/session, + so each gets its own auto-title/rename (the reported bug: only the first + thread per channel was ever named).""" + first = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="group", + user_id="cthulhu", + prospective_thread_id="msg-100", + ) + second = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="group", + user_id="cthulhu", + prospective_thread_id="msg-200", + ) + assert build_session_key(first) != build_session_key(second) + assert build_session_key(first).endswith(":msg-100") + assert build_session_key(second).endswith(":msg-200") + + def test_real_thread_id_wins_over_prospective(self): + """A real thread_id always takes precedence over prospective_thread_id + (they normally match; if both are somehow set, the real one wins).""" + source = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="thread", + thread_id="real-thread", + prospective_thread_id="ignored", + user_id="cthulhu", + ) + assert build_session_key(source).endswith(":real-thread") + + def test_prospective_thread_shares_across_participants(self): + """A prospective-thread session is shared across participants, same as a + real thread (thread sessions are not per-user by default).""" + alice = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="group", + user_id="alice", + prospective_thread_id="msg-100", + ) + bob = SessionSource( + platform=Platform.DISCORD, + chat_id="channel-1", + chat_type="group", + user_id="bob", + prospective_thread_id="msg-100", + ) + assert build_session_key(alice) == build_session_key(bob) + + def test_non_thread_group_sessions_still_isolated_per_user(self): """Regular group messages (no thread_id) remain per-user by default.""" alice = SessionSource( diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 86f9456d7cefa..8cc34c40c88a6 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -389,6 +389,48 @@ class TestRuntimeRepair: assert not (root / ".hermes-runtime").exists() mock_install.assert_not_called() + def test_stage_candidate_sync_keeps_uv_project_config(self, tmp_path): + from hermes_cli.managed_uv import _stage_candidate_venv + + root = tmp_path / "checkout" + root.mkdir() + (root / "uv.lock").write_text("# lock\n", encoding="utf-8") + generation = root / ".hermes-runtime" / "python" / "gen" + python = generation / "bin" / "python" + python.parent.mkdir(parents=True) + python.write_text("py", encoding="utf-8") + + calls = [] + + def fake_run(argv, **kwargs): + calls.append((list(argv), kwargs.get("env"))) + return MagicMock(returncode=0) + + with patch("hermes_cli.managed_uv.subprocess.run", side_effect=fake_run), \ + patch( + "hermes_cli.managed_uv._smoke_candidate_venv", + return_value=(True, "", None), + ), \ + patch("hermes_cli.managed_uv.platform.system", return_value="Linux"): + candidate = _stage_candidate_venv( + "uv", + project_root=root, + generation=generation, + python=python, + ) + + assert candidate is not None + assert len(calls) == 2 + venv_argv, venv_env = calls[0] + sync_argv, sync_env = calls[1] + assert venv_argv[:2] == ["uv", "venv"] + assert "--no-config" in venv_argv + assert venv_env.get("UV_NO_CONFIG") == "1" + assert sync_argv[:2] == ["uv", "sync"] + assert "--locked" in sync_argv + assert "--no-config" not in sync_argv + assert "UV_NO_CONFIG" not in sync_env + def test_failed_candidate_preserves_live_venv(self, tmp_path): from hermes_cli.managed_uv import ( _acquire_repair_lock, diff --git a/website/package.json b/website/package.json index bb752328ad8cb..e3b8477835031 100644 --- a/website/package.json +++ b/website/package.json @@ -54,7 +54,7 @@ }, "engines": { "node": ">=20.0", - "npm": "<11.10.0 || >=12.0.0" + "npm": ">=11.17.0" }, "allowScripts": { "core-js@3.49.0": true,