Răsfoiți Sursa

fix(commands): address review-bot round on the attachment envelope

- Honor a cancellation that lands during image admission before the
  handler runs, settling command/done with the abort reason (executor
  re-check after admitEncodedImages; the committed objects stay
  unreferenced, deferred-GC territory, now recorded in the Agent Note).
- Never let a pending draft-image serialization reach claim.submit after
  the attempt died (dispose/session teardown race).
- Refuse image removal while a command submit is in flight so the rail
  cannot diverge from the serialized snapshot mid-transaction.
- Declare dsh-llm as a runtime peer dependency of command-goal.
- Mirror the host executor's ordering and the producer grammar
  rejections in the fixture command plane: image checks run after
  command resolution (unknown names stay lifecycle-free), bare /goal and
  /plan//plan off with images answer the producers' error texts.
- Explain the deliberate serialize/release asymmetry in the hub's
  commandImages plumbing.
creatixchu 1 lună în urmă
părinte
comite
4ed283a2ba

+ 2 - 2
.agents/notes/implemented/feature/2026-08-17-command-image-attachment-envelope.i18n.yaml

@@ -2,5 +2,5 @@
 # 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/feature/2026-08-17-command-image-attachment-envelope.md
-2026-08-17-command-image-attachment-envelope.md: 89a8d8a047005d8267e3cb5e368d9ed938865494
-2026-08-17-command-image-attachment-envelope.zh.md: 27fe48fcaa80ea47fc1598f3242deaf00bff83a6
+2026-08-17-command-image-attachment-envelope.md: 64897b25f989210d5fa73f3a4f8124f6cbac73fb
+2026-08-17-command-image-attachment-envelope.zh.md: d2741e294ac1e30fb6e0bd3ffd0112799e39c17e

+ 1 - 0
.agents/notes/implemented/feature/2026-08-17-command-image-attachment-envelope.md

@@ -40,3 +40,4 @@ Registry executor enforcement, admission failure settlement, and frozen invocati
 - The commands package now depends on `dsh-attachment` and `dsh-llm`, and `commands/execute` carries a required `images` wire parameter — every caller states its envelope explicitly.
 - `/goal` and `/plan` gain reference-image input at the cost of one extra logged user message (goal) and image blocks in the steered message (plan), billed like any image prompt.
 - Menu-pick popup flows do not consult the envelope: picking a popup command from the menu while images are attached leaves the images visibly in the rail rather than refusing the interaction. Enter-submission is the enforced envelope boundary.
+- "A rejected batch publishes no durable object" covers exactly the pre-admission settlements (declaration, missing store, batch limit). A handler-level grammar rejection (`/goal pause` with images) and a post-admission cancellation settle AFTER the batch committed, leaving content-addressed objects without a referencing session event — harmless under sha256 dedup and the attachment store's deferred reference-aware GC, but not "no object was written".

+ 1 - 0
.agents/notes/implemented/feature/2026-08-17-command-image-attachment-envelope.zh.md

@@ -40,3 +40,4 @@ Web composer 的一次提交是一个信封——草稿文本、已附加图片
 - commands 包新增对 `dsh-attachment` 与 `dsh-llm` 的依赖,`commands/execute` 携带必填的 `images` wire 参数——每个调用方都显式陈述其信封。
 - `/goal` 与 `/plan` 获得参考图输入,代价是一条额外的已记录用户消息(goal)与 steer 消息中的图片块(plan),计费与任何图片提示词相同。
 - 菜单点选的弹窗流程不查询信封:附有图片时从菜单点选弹窗命令,图片会可见地留在附件栏,而不是拒绝该交互。回车提交是被强制执行的信封边界。
+- 「被拒绝的批量不发布任何持久化对象」只覆盖准入前的三种结算(声明、存储缺失、批量超限)。handler 级语法拒绝(如 `/goal pause` 带图)与准入后取消发生在批量已提交之后,会留下没有会话事件引用的内容寻址对象——在 sha256 去重与附件存储延后的引用感知 GC 下无害,但并非「未写入任何对象」。

+ 22 - 10
packages/client/connection/src/client/fixture.ts

@@ -1747,16 +1747,28 @@ function createFixtureWorld(options: FixtureOptions): FixtureWorld {
       const match = /^\/(\S+)((?:\s.*)?)$/.exec(line.trim())
       const name = match?.[1]
       const args = match?.[2] ?? ''
-      // Mirror the Host executor's declaration enforcement: only the
-      // descriptors listed with `input.images` accept an image-carrying
-      // submission; the fixture stores no bytes, so accepted images are
-      // acknowledged and dropped.
-      if (images.length > 0 && name !== 'goal' && name !== 'plan') {
-        const commandId = `fx-cmd-${logOf(id).length}` as CommandId
-        append(id, { type: 'command/run', data: { commandId, name: name ?? '', args, source: { kind: 'user' } } })
-        const result: CommandResult = { kind: 'error', text: `/${name} does not accept image attachments` }
-        append(id, { type: 'command/done', data: { commandId, ...result } })
-        return { ok: true, value: { commandId, result } }
+      // Mirror the Host image policy AFTER command resolution, matching the
+      // executor's order (an unknown name answers undefined and logs no
+      // lifecycle): the declaration rejection covers every known command
+      // without `input.images`, and the two producer grammar rejections cover
+      // the declaring commands' carrier-less lines. The fixture stores no
+      // bytes, so an accepted batch is acknowledged and dropped.
+      const known = ['permission', 'goal', 'compact', 'echo', 'plan']
+      if (images.length > 0 && name !== undefined && known.includes(name)) {
+        const rejection = name !== 'goal' && name !== 'plan'
+          ? `/${name} does not accept image attachments`
+          : name === 'goal' && args.trim() === ''
+            ? 'Image attachments only accompany a goal objective: /goal <objective> or /goal edit <objective>.'
+            : name === 'plan' && (args.trim() === '' || args.trim() === 'off')
+              ? 'Image attachments require a plan message: /plan <message>.'
+              : undefined
+        if (rejection !== undefined) {
+          const commandId = `fx-cmd-${logOf(id).length}` as CommandId
+          append(id, { type: 'command/run', data: { commandId, name, args, source: { kind: 'user' } } })
+          const result: CommandResult = { kind: 'error', text: rejection }
+          append(id, { type: 'command/done', data: { commandId, ...result } })
+          return { ok: true, value: { commandId, result } }
+        }
       }
       if (name === 'permission') {
         const preset = args.trim()

+ 29 - 0
packages/client/connection/tests/fixture-commands.client.spec.ts

@@ -114,6 +114,35 @@ describe('createFixtureApi commands/skills', () => {
     const accepted = await callRemote<{ result: { kind: string } } | undefined>(
       rpc, 'commands/execute', { agentId: sid('fx-alpha'), line: '/goal ship it', images: [png] })
     expect(accepted?.result.kind).toBe('success')
+    const planMessage = await callRemote<{ result: { kind: string } } | undefined>(
+      rpc, 'commands/execute', { agentId: sid('fx-alpha'), line: '/plan sketch the layout', images: [png] })
+    expect(planMessage?.result.kind).toBe('success')
+  })
+
+  it('mirrors the producer grammar rejections for carrier-less declaring lines', async () => {
+    const { rpc } = createFixtureFaces()
+    const png = { mediaType: 'image/png', data: 'AA==' }
+    const bareGoal = await callRemote<{ result: { kind: string; text?: string } } | undefined>(
+      rpc, 'commands/execute', { agentId: sid('fx-alpha'), line: '/goal', images: [png] })
+    expect(bareGoal?.result).toEqual({
+      kind: 'error',
+      text: 'Image attachments only accompany a goal objective: /goal <objective> or /goal edit <objective>.',
+    })
+    for (const line of ['/plan', '/plan off']) {
+      const refused = await callRemote<{ result: { kind: string; text?: string } } | undefined>(
+        rpc, 'commands/execute', { agentId: sid('fx-alpha'), line, images: [png] })
+      expect(refused?.result).toEqual({
+        kind: 'error',
+        text: 'Image attachments require a plan message: /plan <message>.',
+      })
+    }
+  })
+
+  it('answers no execution for an unknown name even when images accompany it', async () => {
+    const { rpc } = createFixtureFaces()
+    const png = { mediaType: 'image/png', data: 'AA==' }
+    expect(await callRemote(rpc, 'commands/execute', { agentId: sid('fx-alpha'), line: '/nope', images: [png] }))
+      .toBeUndefined()
   })
 
   it('answers no execution for unknown names and non-command lines', async () => {

+ 2 - 2
packages/client/ui-conversation/src/client/input/contract.ts

@@ -35,7 +35,7 @@ export interface SessionInput extends InputTarget {
   setDraft(text: string): void
   /** Append ordered browser-owned image ids; busy admission phases refuse. */
   addImages(ids: readonly DraftAttachmentId[]): boolean
-  /** Remove one browser-owned image id. */
+  /** Remove one browser-owned image id; busy admission phases refuse. */
   removeImage(id: DraftAttachmentId): void
   /** Drop ids whose browser-owned objects no longer exist. */
   pruneImages(ids: readonly DraftAttachmentId[]): void
@@ -75,7 +75,7 @@ export interface InputActions {
   setDraft(text: string): void
   /** Append ordered browser-owned image ids; busy admission phases refuse. */
   addImages(ids: readonly DraftAttachmentId[]): boolean
-  /** Remove one browser-owned image id. */
+  /** Remove one browser-owned image id; busy admission phases refuse. */
   removeImage(id: DraftAttachmentId): void
   /** Drop ids whose browser-owned objects no longer exist. */
   pruneImages(ids: readonly DraftAttachmentId[]): void

+ 14 - 4
packages/client/ui-conversation/src/client/input/facade.ts

@@ -125,8 +125,13 @@ export class SessionInputShell implements SessionInput {
     return true
   }
 
-  /** Remove one image id from this draft. */
+  /**
+   * Remove one image id from this draft. Busy admission phases refuse, like
+   * {@link addImages}: a removal landing while a command submit serializes
+   * would otherwise vanish from the rail yet still ride the in-flight send.
+   */
   removeImage(id: DraftAttachmentId): void {
+    if (this.snapshot.phase === 'adjudicating' || this.snapshot.phase === 'submitting') return
     const next = this.imageIds.filter(candidate => candidate !== id)
     if (next.length === this.imageIds.length) return
     this.imageIds = next
@@ -497,11 +502,16 @@ export class SessionInputShell implements SessionInput {
   private beginSubmit(attempt: SubmitAttempt, claim: CommandClaim, args: string): void {
     const imageIds = claim.images === true ? [...this.imageIds] : []
     Promise.resolve()
-      .then(() => imageIds.length > 0 ? this.deps.commandImages.serialize(imageIds) : [])
-      .then(images => claim.submit(args, this.deps.actx, images))
+      .then(async () => {
+        const images = imageIds.length > 0 ? await this.deps.commandImages.serialize(imageIds) : []
+        // Serialization may outlive the attempt (large files, session
+        // teardown); a dead attempt must not reach the Host executor.
+        if (this.dead(attempt)) return undefined
+        return claim.submit(args, this.deps.actx, images)
+      })
       .then(
         (outcome) => {
-          if (this.dead(attempt)) return
+          if (outcome === undefined || this.dead(attempt)) return
           if (outcome.kind === 'success' && imageIds.length > 0) {
             const submitted = new Set(imageIds)
             this.imageIds = this.imageIds.filter(id => !submitted.has(id))

+ 4 - 0
packages/client/ui-conversation/src/client/input/hub.ts

@@ -80,6 +80,10 @@ export class InputHub implements SessionInputResolver {
       steerQueue: () => { void this.steerQueue(session, shell) },
       commandImages: {
         serialize: ids => this.conversation().serializeDraftImages(ids),
+        // Asymmetric with serialize on purpose: release settles AFTER the
+        // submit RPC, where session teardown may already have unloaded the
+        // conversation service (the same tolerance as the scope disposer
+        // above); leaked preview URLs then die with the document.
         release: (ids) => {
           const conversation = this.rootCtx.get('conversation') as ConversationAttachmentFace | undefined
           for (const imageId of ids) conversation?.releaseDraftImage(imageId)

+ 29 - 0
packages/client/ui-conversation/tests/input-matrix.client.spec.tsx

@@ -212,6 +212,35 @@ describe('matrix row: claimed with images', () => {
     expect(release).not.toHaveBeenCalled()
     expect(shell.snapshot.phase).toBe('claimed')
   })
+
+  it('a disposed shell never lets a pending serialization reach claim.submit', async () => {
+    const submit = vi.fn(() => Promise.resolve({ kind: 'success' as const }))
+    let resolveSerialize!: (images: readonly SubmitImageAttachment[]) => void
+    const { shell, textarea, claim } = bench({
+      submit,
+      serialize: () => new Promise((resolve) => { resolveSerialize = resolve }),
+    })
+    claim('/goal ', '目标', true)
+    act(() => { shell.addImages([img]) })
+    fireEvent.keyDown(textarea, { key: 'Enter' })
+    await vi.waitFor(() => { expect(resolveSerialize).toBeDefined() })
+    shell.dispose()
+    resolveSerialize([{ mediaType: 'image/png', data: 'AA==' }])
+    await Promise.resolve()
+    await Promise.resolve()
+    expect(submit).not.toHaveBeenCalled()
+  })
+
+  it('image removal is refused while a command submit is in flight', async () => {
+    const submit = vi.fn(() => new Promise<SubmitOutcome>(() => {})) // never settles
+    const { shell, textarea, claim } = bench({ submit, serialize: () => Promise.resolve([]) })
+    claim('/goal ', '目标', true)
+    act(() => { shell.addImages([img]) })
+    fireEvent.keyDown(textarea, { key: 'Enter' })
+    expect(shell.snapshot.phase).toBe('submitting')
+    act(() => { shell.removeImage(img) })
+    expect(shell.snapshot.imageIds).toEqual([img])
+  })
 })
 
 describe('matrix row: submitting', () => {

+ 1 - 0
packages/goal/command-goal/package.json

@@ -35,6 +35,7 @@
     "@deepseek-ai/dsh-commands": "workspace:^",
     "@deepseek-ai/dsh-goal": "workspace:^",
     "@deepseek-ai/dsh-invariants": "workspace:^",
+    "@deepseek-ai/dsh-llm": "workspace:^",
     "@deepseek-ai/cordis": "workspace:^"
   },
   "devDependencies": {

+ 14 - 0
packages/interaction/commands/src/index.ts

@@ -128,6 +128,11 @@ function abortError(signal: AbortSignal): Error {
   return new Error(typeof signal.reason === 'string' ? signal.reason : 'command aborted')
 }
 
+/** The signal's normalized abort error when it is already aborted. */
+function cancellationOf(signal: AbortSignal): Error | undefined {
+  return signal.aborted ? abortError(signal) : undefined
+}
+
 /** Render arbitrary thrown values without trusting their string coercion. */
 function renderThrown(value: unknown): string {
   try {
@@ -368,6 +373,15 @@ export class CommandRuntime extends TypertRemoteService {
         this.settleThrown(agent.session, parsed.name, commandId, error)
         throw error
       }
+      // Cancellation must be honored BEFORE the handler runs: admission may
+      // await slow storage, and a handler entered after the caller cancelled
+      // would mutate state the retrying caller then duplicates. (The committed
+      // image objects stay unreferenced and are deferred-GC territory.)
+      const cancelledDuringAdmission = cancellationOf(signal)
+      if (cancelledDuringAdmission !== undefined) {
+        this.settleThrown(agent.session, parsed.name, commandId, cancelledDuringAdmission)
+        throw cancelledDuringAdmission
+      }
     }
     const invocation = Object.freeze({ commandId, agent, rawInput: parsed.rawInput, attachments, signal })
     let result: CommandResult

+ 22 - 0
packages/interaction/commands/tests/commands.spec.ts

@@ -574,6 +574,28 @@ describe('image attachments', () => {
     expect(lifecycleOf(agent).at(-1)).toMatchObject({ type: 'command/done', data: { kind: 'error' } })
   })
 
+  it('honors a cancellation that lands during admission before entering the handler', async () => {
+    const ctx = await mount()
+    const controller = new AbortController()
+    const store = storeOf()
+    store.saveImage.mockImplementationOnce((input: { mediaType: string }) => {
+      controller.abort('operator cancelled during admission')
+      return Promise.resolve({ attachmentId: 'att-late', mediaType: input.mediaType, bytes: 3, width: 1, height: 1 })
+    })
+    ctx.provide('attachments', store)
+    const { agent } = await mintAgentScope(ctx, 'a')
+    const handler = vi.fn(() => ({ kind: 'success' as const }))
+    ctx.commands.register(accepting(handler))
+    await expect(ctx.commands.execute(
+      agent, '/vision x', [{ mediaType: 'image/png', data: PNG }], controller.signal,
+    )).rejects.toThrow('operator cancelled during admission')
+    expect(handler).not.toHaveBeenCalled()
+    expect(lifecycleOf(agent).at(-1)).toMatchObject({
+      type: 'command/done',
+      data: { kind: 'error', text: 'operator cancelled during admission' },
+    })
+  })
+
   it('logs and rethrows a non-attachment admission failure', async () => {
     const ctx = await mount()
     const store = storeOf()