Agent Note: Branded IDs everywhere they belong

Status: implemented

English | 中文

Problem

The harness brands CallId (packages/llm/llm/src/brand.ts) and the shared agent/session SessionId (packages/core/session/src/types.ts) using the Branded<B> = string & { readonly [BRAND]: B } machinery (owned by the type-only @deepseek-ai/dsh-brand package at packages/util/brand/ — see its README) and a zero-cost cast factory per type. dsh-brand also states the governing policy: "Branding is for ids that cross package boundaries and could plausibly be confused; not every string needs a brand." That policy is right; the problem is that it is only half-applied. Two gaps let a structurally-identical-but-semantically-wrong string slip through the type checker today.

Gap 1 — unbranded cross-boundary IDs in the bash seam. The background-job id is a plain string: BashTask.id: string (packages/shell/shell/src/types.ts), carried as string through the whole executor seam (ShellExecutor.get/ownerOf/readOutput/kill(id: string) in packages/shell/shell/src/index.ts) and validated/passed as string by the model-facing tools (validateJobId, assertTaskAccess, the job_id schema arg in packages/shell/tool-bash/src/index.ts). It is generated by a per-executor counter — `bash-${this.nextTaskId++}` in packages/shell/bash-local/src/index.ts — which gives it exactly the same name-N shape as SessionId's default (`session-${++counter}` in packages/core/session/src/index.ts). A bash job id and a session id are trivially swappable at a call site and the compiler says nothing. It is a model-facing id (the model passes job_id back to bash_output/bash_kill), so a confusion here is reachable from untrusted input.

The bash owner token is the related sub-case: ShellExecRequest.owner?: string and ShellExecSpec.owner: string | undefined (packages/shell/shell/src/types.ts) are documented as a deliberately opaque isolation key, but in every live caller the value IS the owning agent's shared Agent.id/SessionId (callerToken = (exec) => exec.agent?.id in packages/shell/tool-bash/src/index.ts) wearing a different seam-local name. It is compared for access control (owner !== callerToken(exec)), so a mismatched-but-well-typed string here is a cross-session isolation bug the type system currently cannot catch. This is the shared id alias covered by the unified agent/session identity decision.

Gap 2 — brand erosion at the boundaries of the already-branded IDs. Even CallId and SessionId decay back to bare string at exactly the places confusion is most likely: registry/store key types and public method params. Representative sites include the session store, the agent registry (both keyed by the shared SessionId), tool-presentation call-id maps, ACP's session records, and the persistence coordinator. A brand that is dropped at a collection key buys nothing on lookups — the value of the existing brands is partly unrealized.

Decision

A type-only change. Brands are zero-cost casts; nothing about runtime behavior, serialization, comparison, or the wire format changes. The decision has three parts, all honoring the existing "not every string" policy.

Brand the bash job id. Add BashTaskId = Branded<'BashTaskId'> plus its same-named factory in packages/shell/shell/src/types.ts (the package that owns the id), importing Branded from @deepseek-ai/dsh-brand exactly as SessionId does. The brand primitive lives in the dependency-free dsh-brand utility package precisely so dsh-shell can brand its ids by depending on it alone — it never pulls in dsh-llm (or dsh-session) just to reach Branded. Thread it through BashTask.id, the ShellExecutor Service Definition methods (get/ownerOf/readOutput/kill), the generation site in dsh-bash-local (brand the counter output once, at creation), and the dsh-tool-bash validate/access surface (validateJobId returns a BashTaskId; job_id is branded at the tool boundary where the model's string arrives).
Mint a distinct OwnerToken brand. Add OwnerToken = Branded<'OwnerToken'> in packages/shell/shell/src/types.ts; type ShellExecRequest.owner / ShellExecSpec.owner / ShellExecutor.ownerOf as OwnerToken | undefined. The dsh-tool-bash consumer casts the agent's shared id (SessionId) into an OwnerToken at the boundary — the one place the two vocabularies meet. The bash Service Definition never imports dsh-session. (Rationale in the next section.)
Stop the brand erosion. Propagate the existing brands to the Map key types and public method params listed under Gap 2 — Map<SessionId, Session>, Map<SessionId, Agent>, get(id: SessionId), Map<CallId, …>, ACP's SessionId surface, and the coordinator's Map<SessionId, …>. This is the larger mechanical share of the change and the part that makes the existing brands actually load-bearing on lookups, not just on struct fields.

Illustrative shape (the factory pattern is identical to the three existing brands):

import type { Branded } from '@deepseek-ai/dsh-brand'

/** A background bash task handle (generated `bash-N` by the local executor). */
export type BashTaskId = Branded<'BashTaskId'>
export function BashTaskId(id: string): BashTaskId {
  return id as BashTaskId
}

/** A bash task's opaque isolation key — the consumer's owner identity, NOT the bash seam's. */
export type OwnerToken = Branded<'OwnerToken'>
export function OwnerToken(id: string): OwnerToken {
  return id as OwnerToken
}

Alternatives considered

Why not typing `owner` as `SessionId`?

The obvious shortcut is to type owner as SessionId directly — it always is one. We reject that. The bash executor seam is a capability seam (Service Definition dsh-shell, Service Provider dsh-bash-local, Consumer dsh-tool-bash) and its owner token is documented as deliberately opaque: the executor "never interprets it (no access policy lives in the seam — that is the consumer's job)" (packages/shell/shell/src/types.ts). Typing the Service Definition's field as SessionId would import dsh-session's vocabulary into a package that must not know what an owner token means — it would couple a generic execution backend to the session model and contradict the opaque-token design. A sandboxed or remote executor that replaces dsh-bash-local should not inherit a session dependency. The distinct OwnerToken brand keeps the seam decoupled: dsh-shell knows only "an owner is some opaque branded token," and the dsh-tool-bash consumer — which already decides the access policy — is the single boundary that casts its SessionId into an OwnerToken. The brand still delivers the safety win (you cannot pass a BashTaskId or a raw string where an owner is expected) without the coupling.

Out of scope / possible extensions

Kept deliberately narrow per the "not every string needs a brand" policy. Each of these is a plausible future brand, deferred with a reason, not a commitment:

ModelId (GenerateOptions.model, the LlmRuntime adapter-registry key) — a real cross-package lookup key (config → agent → llm → adapter); a reasonable next brand, left out only to keep this decision's blast radius focused.
ToolName (the ToolRuntime key) — author-defined, human-readable, and rarely confused with another id; the weakest candidate, likely not worth a brand.
ErrorCode (HarnessError.code) — a closed vocabulary (ABORTED, NO_ADAPTER, …), not a per-instance id; better served by a string-literal union than a brand, if anything.
Numeric ordinals — turn number, step number, and the event seq are number, not string, so Branded<string> does not apply; a parallel number & { readonly [BRAND]: B } variant could brand them, but they are positional ordinals rarely passed across boundaries, so the payoff is low.
Validated construction — the brand factories are pure casts with no runtime check, and every boundary (ACP sessionId, provider-issued call.id, the empty-string fallback in dsh-llm-deepseek) trusts the raw string today. A SessionId.parse() / isValid() companion that throws on malformed input at boundaries is a genuine gap, but it is a runtime-behavior change with its own design (what is "malformed"? what happens on failure?) and belongs in its own decision, not bundled into this type-only change.

Verification

The landed invariants: BashTaskId and OwnerToken are defined in dsh-shell and threaded end-to-end (Service Definition, the dsh-bash-local generation site, the dsh-tool-bash model-facing tool) with no dsh-shell dependency on dsh-session; no collection keyed by an in-scope branded id (CallId/SessionId/BashTaskId) is keyed by bare string; public method params and exported signatures keep the brand; and brands are constructed via the cast factory at each boundary where a raw string enters (provider call id, ACP session id, model-supplied job_id), never as scattered as casts.

Consequences

Mechanical churn across two surfaces. Propagating brands touches the bash seam (Service Definition + Service Provider + Consumer) and the ACP session-id surface plus the persistence coordinator. The churn is broad but low-severity: a missed site is a compile error, not a silent bug. The change is observably type-only — no snapshot or e2e behavioral diff. It sits next to the unified agent/session identity decision because both touch the session-id / owner-token boundary; OwnerToken stays distinct from the unified id for the decoupling reason above.
Brands do not validate. A brand is a confusability guard, not a correctness proof: a wrong session id that is still a well-formed string passes the type checker exactly as before. This decision does not close that gap (see Out of scope) — it only stops the category error of passing the wrong kind of id.
The "where to stop" line stays a judgment call. Branding BashTaskId but not ToolName, OwnerToken but not ModelId, is a taste call about which strings "could plausibly be confused." Reasonable reviewers may want more or fewer; the policy in brand.ts is the tie-breaker, and this decision errs toward the ids that are model-facing or used for access control.

10 KiB Raw Blame History