Ver Fonte

Merge pull request #3315 from deepseek-harness/worktree/issue-3310-verification-a39d1a

fix(web): publish the drill claim before the descent edit
Yichen Jiang há 3 semanas atrás
pai
commit
cf50cfa8ac

+ 6 - 0
.agents/notes/implemented/bug-fix/2026-08-29-drill-claim-precedes-the-drill-edit.i18n.yaml

@@ -0,0 +1,6 @@
+# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
+# side as of the last confirmed-consistent state. Both languages carry equal authority;
+# after editing either side, bring the other along and re-record with:
+#   pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-29-drill-claim-precedes-the-drill-edit.md
+2026-08-29-drill-claim-precedes-the-drill-edit.md: 35ca60360c2c8647c44ce9a164cff71fa112f942
+2026-08-29-drill-claim-precedes-the-drill-edit.zh.md: 58f7ca84d201726c0630f7cb6e9bf9614de76c20

+ 37 - 0
.agents/notes/implemented/bug-fix/2026-08-29-drill-claim-precedes-the-drill-edit.md

@@ -0,0 +1,37 @@
+# Agent Note: The drill claim is published before the edit that re-enters tracking
+
+Status: implemented
+
+English | [中文](2026-08-29-drill-claim-precedes-the-drill-edit.zh.md)
+
+## Problem
+
+A pointer descent in the `@` menu produced no breadcrumb, while the keyboard descent into the same directory produced one (#3310). Clicking a crumb — the gesture the breadcrumb exists for — dropped the header entirely instead of re-listing the step it named. Rows in a pointer-drilled listing also repeated the parent directory the header was supposed to carry.
+
+The three faults are one ordering defect in `InputTriggerController.settle`. The drill claim (`drilled`) was assigned after `execute()` returned, on the assumption that the input applies a descent edit and re-tracks later. That holds only for the keyboard: `KEY_TAB_COMMAND` handlers run inside a Lexical update, so `SessionInputShell.applyEdit` joins the enclosing update and the commit — with the `track()` call its update listener drives — lands after `settle` has returned. A pointer `mousedown` handler is outside any update, so `applyEdit` runs `editor.update(fn, { discrete: true })`, which sets `_flushSync` and commits synchronously; `track()` therefore re-enters the controller *during* `execute()`, and both readers of the claim — `refreshHeaders` and `fetchCandidates` — saw it still clear. Every existing test modeled the keyboard ordering: the fake insert listener returned `true` and the spec re-tracked afterwards by hand, so the pointer ordering was never exercised.
+
+## Decision
+
+`settle` claims the drill before dispatching the edit, and withdraws the claim only when the edit is refused:
+
+```ts ignore-check
+this.reduce({ type: 'close' })
+this.drilled = action === 'drill'
+if (!this.execute(outcome, hit.span)) this.drilled = false
+```
+
+The claim still follows `reduce({ type: 'close' })`, whose teardown clears it. Withdrawal remains exact because a refused edit mutates nothing and so drives no re-entrant `track()`: `insertText` fails its `draftRev` CAS before touching the editor, and `$replaceDetectSpanWithText` returns `false` from `selectSpan` ahead of `$setSelection`. The observable guarantee the [breadcrumb decision](../feature/2026-08-27-web-at-mention-discovery-and-row-content.md) states is unchanged — a header never names a directory nobody descended into — and both descent gestures now reach `header` and `candidates` as a drill.
+
+## Alternatives considered
+
+**Re-publish the header after `execute` returns.** Rejected: it treats the visible half of one defect. `fetchCandidates` reads the same claim, so the candidate request would still report `drilled: false` and `ui-reference` would keep repeating the parent directory on every row of a pointer-drilled listing.
+
+**Defer `execute` to a microtask so the re-entrant track always lands after `settle`.** Rejected: the edit carries `hit.span` for revision CAS, and postponing it past the current task lets an intervening keystroke invalidate the span, turning a working descent into a silently refused one.
+
+**Make `applyEdit` never flush synchronously.** Rejected: `discrete` is what keeps a programmatic edit and the detect coordinates computed from it in one task; relaxing it to fix a menu flag would loosen the whole input machine's ordering for every caller.
+
+## Consequences
+
+- Tab, the row chevron, and a crumb reach one behavior, so the breadcrumb no longer depends on which gesture opened the listing.
+- Any future state a source reads through `header` or `candidates` must be published before `execute`, because the input can re-enter `track()` inside it. The claim is instance state on the controller, so the ordering is the only thing enforcing it.
+- Coverage: a controller spec whose insert listener re-tracks synchronously — the pointer ordering — asserts both readers, and `reference-composer.e2e.ts` asserts the breadcrumb and the trimmed rows after a chevron drill and walks a two-level trail back through a crumb click. The keyboard ordering keeps its existing spec, so a regression that fixes one gesture by breaking the other fails.

+ 37 - 0
.agents/notes/implemented/bug-fix/2026-08-29-drill-claim-precedes-the-drill-edit.zh.md

@@ -0,0 +1,37 @@
+# Agent Note: The drill claim is published before the edit that re-enters tracking
+
+Status: implemented
+
+[English](2026-08-29-drill-claim-precedes-the-drill-edit.md) | 中文
+
+## Problem
+
+在 `@` 菜单里用指针进入目录不产生 breadcrumb,而用键盘进入同一个目录则会产生(#3310)。点击 crumb——breadcrumb 存在的意义所在——不但没有重新列出它所指的那一层,反而让整个 header 消失。指针进入的列表里,每一行还会重复 header 本应承担的父目录。
+
+这三处故障是 `InputTriggerController.settle` 中的同一个顺序缺陷。drill 声明(`drilled`)过去在 `execute()` 返回之后才赋值,前提是输入层稍后才应用下钻编辑并重新 track。该前提只对键盘成立:`KEY_TAB_COMMAND` 的处理器运行在 Lexical update 内部,`SessionInputShell.applyEdit` 因此并入外层 update,提交——以及其 update listener 驱动的 `track()` 调用——落在 `settle` 返回之后。指针的 `mousedown` 处理器不在任何 update 内,`applyEdit` 于是执行 `editor.update(fn, { discrete: true })`,该选项置起 `_flushSync` 并同步提交;`track()` 因此在 `execute()` **执行期间**重入控制器,而声明的两个读取方——`refreshHeaders` 与 `fetchCandidates`——看到的仍是未置位的值。既有测试全部按键盘顺序建模:伪造的 insert 监听器只返回 `true`,由用例事后手工重新 track,指针顺序从未被覆盖。
+
+## Decision
+
+`settle` 在派发编辑之前声明 drill,并且只在编辑被拒绝时撤回:
+
+```ts ignore-check
+this.reduce({ type: 'close' })
+this.drilled = action === 'drill'
+if (!this.execute(outcome, hit.span)) this.drilled = false
+```
+
+声明仍然排在 `reduce({ type: 'close' })` 之后,因为后者的清理会把它清掉。撤回依然精确,原因是被拒绝的编辑不做任何变更,因而不会驱动重入的 `track()`:`insertText` 在碰到编辑器之前就没通过 `draftRev` CAS,`$replaceDetectSpanWithText` 也在 `$setSelection` 之前就从 `selectSpan` 返回 `false`。[breadcrumb 决策](../feature/2026-08-27-web-at-mention-discovery-and-row-content.zh.md)所声明的可观察保证不变——header 绝不会指向一个无人进入过的目录——而两种下钻手势现在都以 drill 的身份抵达 `header` 与 `candidates`。
+
+## Alternatives considered
+
+**在 `execute` 返回后重新发布 header。** 否决:这只处理了缺陷中看得见的那一半。`fetchCandidates` 读取同一个声明,候选请求仍会报告 `drilled: false`,`ui-reference` 也就仍会在指针进入的列表中逐行重复父目录。
+
+**把 `execute` 推迟到 microtask,使重入的 track 必定落在 `settle` 之后。** 否决:该编辑携带 `hit.span` 用于版本 CAS,把它推迟到当前任务之外,会让插入其间的按键作废该 span,把一次本可成功的下钻变成静默失败。
+
+**让 `applyEdit` 永不同步 flush。** 否决:`discrete` 正是让一次程序化编辑与由它算出的 detect 坐标留在同一个任务内的机制;为了修一个菜单标志而放宽它,会为所有调用方松开整个输入机的顺序保证。
+
+## Consequences
+
+- Tab、行内 chevron 与 crumb 收敛到同一种行为,breadcrumb 不再取决于是哪种手势打开了列表。
+- 今后凡是 source 通过 `header` 或 `candidates` 读取的状态,都必须在 `execute` 之前发布,因为输入层可能在其内部重入 `track()`。该声明是控制器上的实例状态,顺序是唯一的约束手段。
+- 覆盖:一个 insert 监听器同步重新 track 的控制器用例——即指针顺序——断言两个读取方;`reference-composer.e2e.ts` 断言 chevron 下钻后的 breadcrumb 与精简后的行,并通过 crumb 点击走完两层路径的回退。键盘顺序保留原有用例,因此「修好一种手势却弄坏另一种」的回归会失败。

+ 27 - 1
apps/web/tests/reference-composer.e2e.ts

@@ -130,6 +130,11 @@ describe.skipIf(MODE === 'record')('web e2e: file and session references through
     await writeFile(join(scaffold.workspaceCwd, 'workspace', 'reference.txt'), 'reference fixture\n')
     await mkdir(join(scaffold.workspaceCwd, 'workspace', 'folderx'), { recursive: true })
     await writeFile(join(scaffold.workspaceCwd, 'workspace', 'folderx', 'child.txt'), 'child fixture\n')
+    // Two levels down: the breadcrumb needs a step above the current one to
+    // return to, and a bare '@' lists only the top level, so the deeper tree
+    // stays out of the menu golden.
+    await mkdir(join(scaffold.workspaceCwd, 'workspace', 'folderx', 'nested'), { recursive: true })
+    await writeFile(join(scaffold.workspaceCwd, 'workspace', 'folderx', 'nested', 'leaf.txt'), 'leaf fixture\n')
     await page.goto(scaffold.authenticatedUrl, { waitUntil: 'load' })
     await page.waitForSelector('[class*="frame"]', { timeout: 30_000 })
     await connectFreshWorkspace(page, scaffold.workspaceCwd)
@@ -280,13 +285,20 @@ describe.skipIf(MODE === 'record')('web e2e: file and session references through
     await expect.poll(() => input.textContent()).toBe('@folderx/')
     await menu.getByRole('option', { name: /child\.txt/ }).waitFor()
 
-    // The row chevron drills the same way by pointer.
+    // The row chevron drills the same way by pointer, header included: a
+    // pointer descent reaches the same listing a Tab descent does.
     await writeComposerDraft(page, input, '@folderx')
     const row = menu.getByRole('option', { name: /^folderx\// })
     await row.waitFor()
     await row.getByRole('button', { name: 'Browse folder' }).click()
     await expect.poll(() => input.textContent()).toBe('@folderx/')
     await menu.getByRole('option', { name: /child\.txt/ }).waitFor()
+    await expect.poll(() => page.getByRole('navigation', { name: 'Folder navigation' })
+      .getByRole('button').allTextContents()).toEqual(['Workspace', 'folderx'])
+    // The listing knows it was drilled into, so its rows drop the location the
+    // header already carries.
+    await expect.poll(() => menu.getByRole('option', { name: /child\.txt/ }).textContent())
+      .toBe('child.txt')
     await page.keyboard.press('Escape')
 
     expect(tripwire.pageErrors).toEqual([])
@@ -318,6 +330,20 @@ describe.skipIf(MODE === 'record')('web e2e: file and session references through
     await expect.poll(() => menu.getByRole('option', { name: /child\.txt/ }).textContent())
       .toBe('child.txt')
 
+    // A crumb above the current step re-lists that directory and keeps the
+    // header, which now names the step it returned to.
+    await writeComposerDraft(page, input, '@folderx/nested')
+    const nested = menu.getByRole('option', { name: /^nested\// })
+    await nested.waitFor()
+    await nested.getByRole('button', { name: 'Browse folder' }).click()
+    await expect.poll(() => input.textContent()).toBe('@folderx/nested/')
+    await expect.poll(() => crumbs.getByRole('button').allTextContents())
+      .toEqual(['Workspace', 'folderx', 'nested'])
+    await crumbs.getByRole('button', { name: 'folderx' }).click()
+    await expect.poll(() => input.textContent()).toBe('@folderx/')
+    await expect.poll(() => crumbs.getByRole('button').allTextContents())
+      .toEqual(['Workspace', 'folderx'])
+
     // Clicking the root crumb rewrites the token back to a bare trigger.
     await crumbs.getByRole('button', { name: 'Workspace' }).click()
     await expect.poll(() => input.textContent()).toBe('@')

+ 10 - 7
packages/client/ui-input-trigger/src/client/controller.ts

@@ -490,13 +490,16 @@ export class InputTriggerController {
     })
     this.stopFetch()
     this.reduce({ type: 'close' })
-    const applied = this.execute(outcome, hit.span)
-    // Set after the close above, so the reducer's own teardown cannot clear
-    // it, and only when the descent text actually landed: a refused edit
-    // (stale draft revision, or no listener) leaves the draft where it was,
-    // and a header over that draft would name a directory nobody descended
-    // into while hiding the locations its rows still need.
-    this.drilled = action === 'drill' && applied
+    // Claimed before the edit, and after the close above so the reducer's own
+    // teardown cannot clear it: the input may apply the descent through a
+    // synchronous editor commit that re-enters track(), and the header and
+    // candidate requests raised there read this flag. A refused edit (stale
+    // draft revision, or an unmappable span) mutates nothing and so reaches
+    // no re-entry, which is why withdrawing the claim afterwards still keeps
+    // a header off a draft nobody descended into — one that would name a
+    // directory while hiding the locations its rows still need.
+    this.drilled = action === 'drill'
+    if (!this.execute(outcome, hit.span)) this.drilled = false
   }
 
   /** Re-poll every header-bearing source in the hit roster and publish their crumbs. */

+ 26 - 2
packages/client/ui-input-trigger/tests/service.client.spec.ts

@@ -595,11 +595,15 @@ describe('header / drilled descent', () => {
   /** A source that publishes one crumb per path segment of a drilled query. */
   function crumbSource() {
     const requests: Array<{ query: string; quoted?: boolean; drilled: boolean }> = []
+    const fetches: boolean[] = []
     const picks: InputTriggerPick[] = []
     const source: InputTriggerSource = {
       trigger: '@',
       name: 'reference',
-      candidates: () => Promise.resolve([{ name: 'src', drill: true, value: 'src' }]),
+      candidates: (_session, req) => {
+        fetches.push(req.drilled)
+        return Promise.resolve([{ name: 'src', drill: true, value: 'src' }])
+      },
       header: (_session, req) => {
         requests.push({ ...req })
         if (!req.drilled || !req.query.includes('/')) return undefined
@@ -610,7 +614,7 @@ describe('header / drilled descent', () => {
         return pick.action === 'drill' ? { text: `@${String(pick.candidate.value)}/`, continue: true } : undefined
       },
     }
-    return { source, requests, picks }
+    return { source, requests, fetches, picks }
   }
 
   it('publishes no crumbs for a typed path and asks every source how the menu was reached', async () => {
@@ -660,6 +664,26 @@ describe('header / drilled descent', () => {
     expect(picks[0]).toMatchObject({ candidate: { name: 'src', value: 'src' }, action: 'drill', via: 'menu' })
   })
 
+  it('publishes crumbs when the input re-tracks inside the drill edit', async () => {
+    const { source, fetches } = crumbSource()
+    const { controller, actx } = controllerBench([source])
+    // A pointer drill reaches the input outside any editor update, so the
+    // descent commits synchronously and re-tracks before the pick that asked
+    // for it has returned — the keyboard drill, dispatched inside an update,
+    // re-tracks only once that update commits. Both orders must reach the
+    // header and candidate requests as a drill.
+    actx.on('slash/input-insert-text', (req) => {
+      controller.track(req.text, req.text.length, { tier: 'plain' }, 2)
+      return true
+    })
+    controller.track('@sr', 3, { tier: 'plain' }, 1)
+    await tick()
+    controller.pick('reference', 0, 'drill')
+    await tick()
+    expect(controller.headers.getSnapshot().get('reference')).toEqual([{ label: 'src', value: 'src' }])
+    expect(fetches).toEqual([false, true])
+  })
+
   it('publishes no crumbs when the input refused the drill edit', async () => {
     const { source } = crumbSource()
     const { controller } = controllerBench([source])