fix: address ds-review-bot v7 findings on the merged image-input head
- gate model selection on steering-placement image carriers from enqueue until their steering/message event publishes; release the gate when an admission ends idle without publication (both behaviorally asserted) - reject session.updateQueue edits carrying non-text blocks at the RPC boundary (queue edits cannot bypass image admission) - extend the durable-directory walk past a first-created DSH_HOME to the deepest pre-existing ancestor - strip Windows-style separators from attachment display names on POSIX - verify attachment reads with a header-only probe (digest already proves the bytes decoded fully at admission); document the read path - make SessionInputShell.addImages refusal observable and keep workspace transfers/composer intake from leaking refused drafts - own ONE recursive image walk (dsh-llm contentHasImage) across apiproxy, pi-ai, compact-basic, and the DeepSeek text-only assertion - drop the redundant canonical-base64 regex and the no-op role read - move AttachmentId/AttachmentError out of types.ts (brand.ts/error.ts); document why AttachmentError does not extend HarnessError - document the hard attachments inject in both consumer READMEs
This commit is contained in:
@@ -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 packages/host/apiproxy/README.md
|
||||
README.md: 3badccd4144babc474f52fa44598ddec3c253e65
|
||||
README.zh.md: 8ccd5bf95c70c79e968b3ea8d0f6a48c62359ae3
|
||||
README.md: 7ee67009c33d9df843e1d5aa71e727c0c798d4a0
|
||||
README.zh.md: f7d1503d2947f84dbc21cb74e10678c4e14cb248
|
||||
|
||||
@@ -40,6 +40,7 @@ None; this package neither assembles nor sends a provider request.
|
||||
|
||||
## Known Limitations and Deferred Work
|
||||
|
||||
- **`attachments` is a hard inject** — the proxy will not mount until an attachment backend provides `ctx.attachments`; a composition missing one stalls silently as a cordis inject gap rather than failing loud (same gap as the connection route; a capability-degraded mount is deferred work).
|
||||
- **`respond` routing is shipped, but pending-interaction state is host-side work** — the wire shape (POST `/api/respond`, `RpcReceipt`) is final; the pending table that makes late/duplicate answers meaningful lives in `src/api-proxy.ts` and is still minimal (questions only, no approvals).
|
||||
- **Reserved seams stay out of `RpcMethodMap`** — `session.fork`, `prompt.mode: 'inject'`, `task.list`, `host.listModels`, and a describe `hostInstanceId` are documented reservations; an unknown method fails loud at envelope parse rather than getting a not-implemented code.
|
||||
- **No protocol version field** — client and host ship together; `host.describe` gains a version negotiation field only when an independently released client exists.
|
||||
|
||||
@@ -40,6 +40,7 @@ Workspace 列表与 Session 列表是相互独立的重连基线。`workspace.cr
|
||||
|
||||
## 已知限制与延期工作
|
||||
|
||||
- **`attachments` 是硬性注入依赖**:代理在附件后端提供 `ctx.attachments` 之前不会挂载;缺少后端的组合会以 cordis 注入缺口的形式静默停滞,而非响亮失败(与 connection 路由是同一缺口;降级挂载属于延期工作)。
|
||||
- **`respond` 路由已经发布,但待处理交互状态仍属宿主侧工作**:协议形状(POST `/api/respond`、`RpcReceipt`)已经定型;使延迟或重复回答具有明确语义的待处理表位于 `src/api-proxy.ts`,目前仍很精简(只支持问题,不支持审批)。
|
||||
- **预留 seam 不进入 `RpcMethodMap`**:`session.fork`、`prompt.mode: 'inject'`、`task.list`、`host.listModels` 和描述字段 `hostInstanceId` 都是已记录的预留项;未知方法会在信封解析时直接失败,而不会返回「尚未实现」错误码。
|
||||
- **没有协议版本字段**:客户端与宿主一同发布;只有出现独立发布的客户端后,`host.describe` 才会增加版本协商字段。
|
||||
|
||||
@@ -13,7 +13,7 @@ import type {
|
||||
} from '@deepseek-ai/dsh-agent'
|
||||
import { AttachmentError } from '@deepseek-ai/dsh-attachment'
|
||||
import type { ImageAttachmentRef } from '@deepseek-ai/dsh-attachment'
|
||||
import { createUserMessage, ReasoningEffortId } from '@deepseek-ai/dsh-llm'
|
||||
import { contentHasImage, createUserMessage, ReasoningEffortId } from '@deepseek-ai/dsh-llm'
|
||||
import { errorChain } from '@deepseek-ai/dsh-llm'
|
||||
import type { ContentBlock, MessageSource } from '@deepseek-ai/dsh-llm'
|
||||
import type { Session, SessionEvent, SessionHeader, SessionId, UserMessage } from '@deepseek-ai/dsh-session'
|
||||
@@ -65,11 +65,11 @@ const DEFAULT_MAX_MESSAGES = 50
|
||||
const MESSAGE_TYPES = new Set(['user/message', 'assistant/message', 'steering/message'])
|
||||
|
||||
function decodeBase64(data: string): Uint8Array {
|
||||
if (data.length === 0 || data.length % 4 !== 0 || !/^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/.test(data)) {
|
||||
throw new AttachmentError('Image upload is not canonical base64.', 'INVALID_IMAGE_BASE64')
|
||||
}
|
||||
const decoded = Buffer.from(data, 'base64')
|
||||
if (decoded.toString('base64') !== data) {
|
||||
// One canonical-form check: any non-canonical input (whitespace, url-safe
|
||||
// alphabet, bad padding, truncated groups) fails the exact round-trip, so a
|
||||
// pre-filter regex over the multi-MiB upload string would be pure overhead.
|
||||
if (data.length === 0 || decoded.toString('base64') !== data) {
|
||||
throw new AttachmentError('Image upload is not canonical base64.', 'INVALID_IMAGE_BASE64')
|
||||
}
|
||||
return new Uint8Array(decoded)
|
||||
@@ -147,12 +147,6 @@ function imageInEvent(event: SessionEvent, match: (ref: ImageAttachmentRef) => b
|
||||
return undefined
|
||||
}
|
||||
|
||||
/** True when typed model content contains an image, including nested tool results. */
|
||||
function contentHasImage(content: readonly ContentBlock[]): boolean {
|
||||
return content.some(block => block.type === 'image'
|
||||
|| (block.type === 'tool-result' && contentHasImage(block.content)))
|
||||
}
|
||||
|
||||
/** True when the current model-visible surface contains an image. */
|
||||
function messagesHaveImage(messages: readonly { content: readonly ContentBlock[] }[]): boolean {
|
||||
return messages.some(message => contentHasImage(message.content))
|
||||
@@ -627,11 +621,12 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
||||
*/
|
||||
const queuedMirror = new Map<SessionId, InboxItem[]>()
|
||||
/**
|
||||
* Claimed-but-unpublished queued occurrences: dequeue is not publication —
|
||||
* the `user/message` append follows it asynchronously — so an image carrier
|
||||
* stays a model-selection gate until its durable event lands, its discard
|
||||
* arrives, or the admission's turn settles idle. Kept apart from the mirror
|
||||
* so the mux-open snapshot never replays a claimed occurrence as queued.
|
||||
* Unpublished occurrences that must still gate model selection: a queued
|
||||
* item from dequeue (claim is not publication — the `user/message` append
|
||||
* follows asynchronously) and a steering item from enqueue (it never enters
|
||||
* the queued mirror, and its `steering/message` append is a separate outbox
|
||||
* hop). Entries retire on their durable event, discard, or idle. Kept apart
|
||||
* from the mirror so the mux-open snapshot never replays them as queued.
|
||||
*/
|
||||
const pendingPublication = new Map<SessionId, InboxItem[]>()
|
||||
type UnseenQueueEvent =
|
||||
@@ -692,7 +687,17 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
||||
}
|
||||
const disposers = [
|
||||
ctx.on('agent/inbox/enqueue', (agent: Agent, item: InboxItem) => {
|
||||
if (item.placement !== 'queued') return
|
||||
if (item.placement === 'steering') {
|
||||
// A steering carrier never enters the queued mirror, yet its image
|
||||
// must gate model selection from enqueue until its steering/message
|
||||
// event publishes (or the admission ends): the outbox hop between
|
||||
// steer() and the append is asynchronous, and a text-only switch
|
||||
// accepted inside it would strand every later turn.
|
||||
const pending = pendingPublication.get(agent.id) ?? []
|
||||
pending.push(item)
|
||||
pendingPublication.set(agent.id, pending)
|
||||
return
|
||||
}
|
||||
const unseen = takeUnseen(agent.id, item.id)
|
||||
if (unseen?.kind === 'terminal') return
|
||||
let entries = queuedMirror.get(agent.id)
|
||||
@@ -726,10 +731,16 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
||||
if (retire(agent, item)) publishQueue(agent.id)
|
||||
}),
|
||||
ctx.on('session/event', (session: Session, event: SessionEvent) => {
|
||||
if (event.type !== 'user/message') return
|
||||
const id = event.type === 'user/message'
|
||||
? event.data.id
|
||||
: event.type === 'steering/message'
|
||||
? (event.data as { message: UserMessage }).message.id
|
||||
: undefined
|
||||
if (id === undefined) return
|
||||
const pending = pendingPublication.get(session.id)
|
||||
if (pending === undefined) return
|
||||
const index = pending.findIndex(entry => entry.message.id === (event.data).id)
|
||||
const placement = event.type === 'user/message' ? 'queued' : 'steering'
|
||||
const index = pending.findIndex(entry => entry.message.id === id && entry.placement === placement)
|
||||
if (index === -1) return
|
||||
pending.splice(index, 1)
|
||||
if (pending.length === 0) pendingPublication.delete(session.id)
|
||||
@@ -1385,6 +1396,16 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
||||
|
||||
updateQueue(request) {
|
||||
const { sessionId, itemId, action } = request.payload
|
||||
// Queue edits bypass durablePromptContent (no admission, no durable
|
||||
// reference, no model-capability recheck), so only text blocks may be
|
||||
// written through this boundary; image intake is prompt-only.
|
||||
if (action.kind === 'edit' && action.content.some(block => block.type !== 'text')) {
|
||||
return Promise.resolve(err(request, {
|
||||
code: 'attachment-error',
|
||||
message: 'queue edits accept text content only',
|
||||
details: { reason: 'QUEUE_EDIT_NON_TEXT' },
|
||||
}))
|
||||
}
|
||||
const agent = ctx.agents.get(sessionId)
|
||||
if (agent === undefined || agent.updateInbox(itemId, action) === 'not-found') {
|
||||
return Promise.resolve(err(request, {
|
||||
|
||||
@@ -387,6 +387,70 @@ describe('Web session model selection', () => {
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('gates selection on a steering image from enqueue until its event publishes', async () => {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
registerTextOnly(ctx)
|
||||
const api = createApiProxy(ctx, { provider: 'deepseek', model: 'deepseek-chat', cwd: '/tmp', workspaceRoot: '/tmp' })
|
||||
const steering = {
|
||||
id: 's-1', role: 'user', source: { kind: 'user' },
|
||||
content: [{ type: 'image', attachment: { attachmentId: 'att-s', mediaType: 'image/png', bytes: 8, width: 1, height: 1 } }],
|
||||
} as never
|
||||
const steeringItem = { id: 'i-s-1', message: steering, placement: 'steering' } as never
|
||||
// A steering carrier never enters the queued mirror, yet the outbox hop
|
||||
// between steer() and its append must not open a text-only switch window.
|
||||
ctx.emit('agent/inbox/enqueue', agent, steeringItem)
|
||||
expect((await api.sessions.selectModel(request({ sessionId, provider: 'text-only', model: 'plain' }))).result.ok).toBe(false)
|
||||
|
||||
ctx.emit('agent/inbox/dequeue', agent, steeringItem)
|
||||
expect((await api.sessions.selectModel(request({ sessionId, provider: 'text-only', model: 'plain' }))).result.ok).toBe(false)
|
||||
|
||||
// Publication hands the gate over to the durable surface.
|
||||
agent.session.append('steering/message', { turn: 1, message: steering }, { surfaceOp: 'append' })
|
||||
expect((await api.sessions.selectModel(request({ sessionId, provider: 'text-only', model: 'plain' }))).result.ok).toBe(false)
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('re-opens selection when an admission ends idle without publication', async () => {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
registerTextOnly(ctx)
|
||||
const api = createApiProxy(ctx, { provider: 'deepseek', model: 'deepseek-chat', cwd: '/tmp', workspaceRoot: '/tmp' })
|
||||
const rejected = {
|
||||
id: 'r-1', role: 'user', source: { kind: 'user' },
|
||||
content: [{ type: 'image', attachment: { attachmentId: 'att-r', mediaType: 'image/png', bytes: 8, width: 1, height: 1 } }],
|
||||
} as never
|
||||
const rejectedItem = { id: 'i-r-1', message: rejected, placement: 'queued' } as never
|
||||
ctx.emit('agent/inbox/enqueue', agent, rejectedItem)
|
||||
ctx.emit('agent/inbox/dequeue', agent, rejectedItem)
|
||||
expect((await api.sessions.selectModel(request({ sessionId, provider: 'text-only', model: 'plain' }))).result.ok).toBe(false)
|
||||
|
||||
// Idle proves the admission ended without publication; nothing durable
|
||||
// requires an image route, so the text-only switch must be accepted again.
|
||||
ctx.emit('agent/status', agent, 'idle')
|
||||
expect(expectValue(await api.sessions.selectModel(request({
|
||||
sessionId, provider: 'text-only', model: 'plain',
|
||||
}))).selected).toEqual({ provider: 'text-only', model: 'plain' })
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('rejects a queue edit that injects unadmitted image content', async () => {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
const api = createApiProxy(ctx, { provider: 'deepseek', model: 'deepseek-chat', cwd: '/tmp', workspaceRoot: '/tmp' })
|
||||
Object.assign(agent, { updateInbox: () => 'applied' })
|
||||
const denied = await api.sessions.updateQueue(request({
|
||||
sessionId,
|
||||
itemId: 'i-x' as never,
|
||||
action: {
|
||||
kind: 'edit' as const,
|
||||
content: [{ type: 'image', attachment: { attachmentId: 'att-x', mediaType: 'image/png', bytes: 8, width: 1, height: 1 } }] as never,
|
||||
},
|
||||
}))
|
||||
expect(denied.result).toMatchObject({
|
||||
ok: false,
|
||||
error: { code: 'attachment-error', details: { reason: 'QUEUE_EDIT_NON_TEXT' } },
|
||||
})
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('serializes an image save with a concurrent model selection', async () => {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
registerTextOnly(ctx)
|
||||
|
||||
Reference in New Issue
Block a user