Agent Note: Branded IDs everywhere they belong

Status: implemented

English | 中文

Problem

The harness brands ToolCallId (packages/llm/llm/src/brand.ts) and the shared agent/session SessionId (packages/core/session/src/types.ts) using Branded<B> = string & { readonly [BRAND]: B } and the stateless brandString<T>() constructor from @deepseek-ai/dsh-brand at packages/util/brand/ — see its README. 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.

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 ToolCallId 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

Brands remain ordinary strings; brandString<T>() returns its input unchanged, so serialization, comparison, and wire formats do not change. The decision has three parts, all honoring the existing "not every string" policy.

Brand the bash job id. Add BashTaskId = Branded<'BashTaskId'> in packages/shell/shell/src/types.ts (the package that owns the id), importing Branded and constructing values with brandString<BashTaskId>() from @deepseek-ai/dsh-brand. The brand utility exists so dsh-shell can brand its ids by depending on it alone — it never pulls in dsh-llm or dsh-session just to reach the primitive. Thread the type through BashTask.id, the ShellExecutor Service Definition methods (get/ownerOf/readOutput/kill), the generation site in dsh-bash-local, and the dsh-tool-bash validation/access surface.
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 applies brandString<OwnerToken>() to the agent's shared id (SessionId) at 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<ToolCallId, …>, 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:

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

/** A background bash task handle (generated `bash-N` by the local executor). */
export type BashTaskId = Branded<'BashTaskId'>
const taskId = brandString<BashTaskId>('bash-1')

/** A bash task's opaque isolation key — the consumer's owner identity, NOT the bash seam's. */
export type OwnerToken = Branded<'OwnerToken'>
const owner = brandString<OwnerToken>('session-1')

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 applies brandString<OwnerToken>() to its SessionId. 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 — brandString<T>() performs no runtime check, and every boundary (ACP sessionId, provider-issued call.id, the empty-string fallback in dsh-llm-deepseek) trusts the raw string. 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.

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 (ToolCallId/SessionId/BashTaskId) is keyed by bare string; public method params and exported signatures keep the brand; and boundaries where raw strings enter use brandString<T>() rather than 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. Construction returns the same runtime string, so there is 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