fix: restore the credential-rejected branch and admit SRC absence by key

Review follow-ups that are logic rather than documentation:

- rpc.schema.ts had lost the credential-rejected branch while api-proxy.ts
  still returns that code, so a legitimate business error failed the
  client's response parse. Restore the branch and assert it.
- rpc-schemas.spec.ts had lost the workspace list, archiveSession and
  insertSessionBefore cases along with the command schemas; those routes
  still ship, so restore their coverage.
- An omitted SRC field is now recognized by an absent key instead of an
  undefined value, which makes the allowance assertExactArguments already
  granted reachable; an explicitly undefined field stays invalid input. A
  weak descriptor's undefined result rides the wire as an absent value,
  matching the envelope removal.
- The chooser unmounts an already-created backend when the surface entry
  fails to load, and no longer reverses the captured id array in place.
This commit is contained in:
imccyu
2026-08-11 23:01:20 +08:00
parent 378141bf5d
commit 9b2925e50f
6 changed files with 79 additions and 7 deletions

View File

@@ -176,6 +176,10 @@ export class TypertGatewayService extends Service implements TypertGateway {
if (request.signal?.aborted === true) throw new RemoteInvocationCancelled(endpoint, error)
throw error
}
// A weak descriptor declares no return type, so nothing returned is a void
// result and rides the wire as an absent value field. A strict descriptor
// keeps its schema: there, undefined has to be a declared result.
if (result === undefined && descriptor.result.mode !== 'strict') return result
return decode(descriptor.result, result, 'result-invalid', endpoint, 'result')
}
@@ -405,6 +409,11 @@ export class TypertGatewayService extends Service implements TypertGateway {
args: Readonly<Record<string, unknown>>,
endpoint: string,
): Promise<unknown> {
// An absent field reached assertExactArguments' allowance, so this parameter
// takes undefined; a present-but-undefined field is not JSON-safe input and
// still fails decode. Lookup ids are never omissible, so absence here only
// ever belongs to a json parameter.
if (!Object.hasOwn(args, parameter.wire)) return undefined
const value = decode(parameter.codec, args[parameter.wire], 'input-invalid', endpoint, parameter.wire)
if (parameter.source === 'json') return value
const key = parameter.lookup

View File

@@ -788,6 +788,19 @@ describe('TypertGatewayService', () => {
}), 'input-invalid')
})
it('admits an omitted SRC field and hands the Host method undefined', async () => {
const { ctx, service } = await setup()
// A weak descriptor reads parameter names from the JavaScript signature and
// cannot see which are optional, so an absent field is admitted; the case
// above keeps an explicitly undefined field rejected.
await expect(ctx.typertGateway.invoke({
namespace: 'goals',
method: 'passthrough',
args: {},
})).resolves.toBeUndefined()
expect(service.calls).toContain('passthrough')
})
it('rejects cyclic SRC input and non-JSON SRC results', async () => {
const { ctx, service } = await setup()
const cyclic: { self?: unknown } = {}

View File

@@ -61,6 +61,7 @@ export const rpcErrorSchema: z.ZodType<RpcError> = z.discriminatedUnion('code',
z.object({ code: z.literal('settings-rejected'), message: z.string(), details: z.object({ ns: z.string() }) }),
z.object({ code: z.literal('settings-not-exposed'), message: z.string(), details: z.object({ ns: z.string() }) }),
z.object({ code: z.literal('settings-conflict'), message: z.string(), details: z.object({ ns: z.string(), expected: z.number(), actual: z.number() }) }),
z.object({ code: z.literal('credential-rejected'), message: z.string(), details: z.object({ ref: z.string() }) }),
z.object({ code: z.literal('model-discovery-failed'), message: z.string(), details: z.object({ settingsNs: z.string(), baseURL: z.string().optional() }) }),
z.object({ code: z.literal('title-invalid'), message: z.string(), details: z.object({ sessionId: z.string() }) }),
z.object({ code: z.literal('fork-unavailable'), message: z.string(), details: z.object({ sessionId: z.string() }) }),

View File

@@ -20,8 +20,11 @@ import {
hostListDirectoryRequestSchema, hostListDirectoryValueSchema,
} from '../src/api/host.schema.ts'
import {
workspaceArchiveSessionRequestSchema, workspaceArchiveSessionValueSchema,
workspaceCreateRequestSchema, workspaceCreateValueSchema, workspaceIdSchema,
workspaceDeleteRequestSchema, workspaceDeleteValueSchema,
workspaceInsertSessionBeforeRequestSchema, workspaceInsertSessionBeforeValueSchema,
workspaceListRequestSchema, workspaceListValueSchema,
workspaceRenameRequestSchema, workspaceRenameValueSchema, workspaceViewSchema,
} from '../src/api/workspace.schema.ts'
import { skillEntrySchema, skillListRequestSchema, skillListValueSchema } from '../src/api/skills.schema.ts'
@@ -73,6 +76,8 @@ describe('rpcErrorSchema', () => {
expect(rpcErrorSchema.parse({ code: 'command-error', message: 'm', details: {} }).code).toBe('command-error')
expect(rpcErrorSchema.parse({ code: 'unknown-command', message: 'm', details: {} }).code).toBe('unknown-command')
expect(rpcErrorSchema.parse({ code: 'title-invalid', message: 'm', details: { sessionId: 's' } }).code).toBe('title-invalid')
// The credentials producer still emits this code, so the branch has to stay.
expect(rpcErrorSchema.parse({ code: 'credential-rejected', message: 'm', details: { ref: 'r' } }).code).toBe('credential-rejected')
expect(rpcErrorSchema.parse({ code: 'internal', message: 'm', details: {} }).code).toBe('internal')
})
@@ -344,11 +349,29 @@ describe('workspace domain schemas', () => {
createdAt: '2026-07-25T00:00:00.000Z', updatedAt: '2026-07-25T00:00:00.000Z',
}
it('validates ids and the view row', () => {
it('validates ids, the view row, and list request/value', () => {
expect(workspaceIdSchema.parse('w1')).toBe('w1')
expect(() => workspaceIdSchema.parse('')).toThrow()
expect(workspaceViewSchema.parse(view).sessionIds).toEqual(['s1'])
expect(() => workspaceViewSchema.parse({ ...view, sessionIds: 's1' })).toThrow()
expect(workspaceListRequestSchema.parse({})).toEqual({})
expect(workspaceListValueSchema.parse({ items: [view], archivedSessionIds: ['s1'] }).items).toHaveLength(1)
expect(() => workspaceListValueSchema.parse({ items: [view] })).toThrow()
})
it('archiveSession request/value carry the id and the full updated set', () => {
expect(workspaceArchiveSessionRequestSchema.parse({ sessionId: 's1' }).sessionId).toBe('s1')
expect(() => workspaceArchiveSessionRequestSchema.parse({})).toThrow()
expect(workspaceArchiveSessionValueSchema.parse({ archivedSessionIds: ['s1', 's2'] }).archivedSessionIds)
.toEqual(['s1', 's2'])
expect(() => workspaceArchiveSessionValueSchema.parse({ archivedSessionIds: 's1' })).toThrow()
})
it('insertSessionBefore accepts an anchored and an anchorless move', () => {
expect(workspaceInsertSessionBeforeRequestSchema.parse({ workspaceId: 'w1', sessionId: 's1', beforeSessionId: 's2' }).beforeSessionId).toBe('s2')
expect(workspaceInsertSessionBeforeRequestSchema.parse({ workspaceId: 'w1', sessionId: 's1' }).beforeSessionId).toBeUndefined()
expect(() => workspaceInsertSessionBeforeRequestSchema.parse({ workspaceId: 'w1' })).toThrow()
expect(workspaceInsertSessionBeforeValueSchema.parse({ workspace: view }).workspace.workspaceId).toBe('w1')
})
it('create requires a path', () => {

View File

@@ -72,11 +72,8 @@ export async function apply(ctx: Context): Promise<void> {
// backend lands first: the surface's browser half drives the capability
// the backend registers.
const ids: string[] = []
for (const name of [BACKEND_PACKAGES[backend], SURFACE_PACKAGES[backend]]) {
ids.push(await ctx.loader.create({ name }))
}
return async () => {
for (const id of ids.reverse()) {
const unmount = async () => {
for (const id of [...ids].reverse()) {
// Tree teardown (group.stop) can have removed the entry already;
// nothing is left to unmount or await then.
if (ctx.loader.store[id] === undefined) continue
@@ -85,5 +82,17 @@ export async function apply(ctx: Context): Promise<void> {
await ctx.loader.remove(id)
}
}
try {
for (const name of [BACKEND_PACKAGES[backend], SURFACE_PACKAGES[backend]]) {
ids.push(await ctx.loader.create({ name }))
}
} catch (cause) {
// Setup owns the entries it created until it returns the disposer: leaving
// the backend mounted would make a retry collide with its own
// directoryPicker registration.
await unmount()
throw cause
}
return unmount
}, 'directory-picker-auto: interaction entries')
}

View File

@@ -88,7 +88,10 @@ afterEach(async () => {
})
/** Write a two-row cordis.yml (webserver + chooser), then boot it through the real Loader. */
async function loadComposition(bindHost: '127.0.0.1' | '0.0.0.0'): Promise<{ ctx: Context; configPath: string }> {
async function loadComposition(
bindHost: '127.0.0.1' | '0.0.0.0',
options: { failSurface?: boolean } = {},
): Promise<{ ctx: Context; configPath: string }> {
root = await mkdtemp(join(tmpdir(), 'dsh-directory-picker-auto-'))
const configPath = join(root, 'cordis.yml')
await writeFile(configPath, [
@@ -115,6 +118,9 @@ async function loadComposition(bindHost: '127.0.0.1' | '0.0.0.0'): Promise<{ ctx
context.loader.internal = {
version: 'v2',
async import(specifier: string) {
if (options.failSurface === true && (specifier === NATIVE_SURFACE || specifier === BROWSE_SURFACE)) {
throw new Error(`surface import failed: ${specifier}`)
}
if (!modules.has(specifier)) throw new Error(`unexpected Loader import: ${specifier}`)
return modules.get(specifier)
},
@@ -209,6 +215,17 @@ describe('real Loader composition', () => {
expect(entryNames(ctx)).not.toContain(NATIVE_SURFACE)
})
it('unmounts the backend when the surface entry fails to load', { timeout: 60_000 }, async () => {
stubAttendedHost()
await expect(loadComposition('127.0.0.1', { failSurface: true })).rejects.toThrow(/surface import failed/)
// Setup owns both entries until it returns its disposer, so a failed surface
// must take the mounted backend with it: otherwise a retry collides with the
// directoryPicker registration this backend already made.
expect(entryNames(context!)).not.toContain(NATIVE)
expect(context!.get('directoryPicker')).toBeUndefined()
})
it('tolerates the mounted entry being removed by the tree before the chooser unloads', { timeout: 60_000 }, async () => {
stubAttendedHost()
const { ctx, configPath } = await loadComposition('127.0.0.1')