Skip to content

Centralize correlation-ID prefixes (apr_/act_/tc_) into a single typed module #717

@kylejryan

Description

@kylejryan

Summary

Centralize the three internal correlation-ID prefixes (apr_, act_, tc_) into a single typed module, with branded types on the id fields whose drift must be structurally prevented. The TypeScript compiler — not code review, not a lint rule — becomes the enforcement layer.

Net: ~+25 / −5 LOC across one new file and three edits. Zero runtime cost. Zero ongoing maintenance burden.

Origin

Surfaced by @jorgeraad as a review nit on src/core/operator/approvalGate.ts in #706:

Nit: is it possible to define these prefixes in a single place or by the code where they are generated, and then import them here to ensure there's no drift? Feel free to disregard.

The comment lands on a docstring that today reads:

/** Internal correlation IDs minted by this module: `apr_*`, `act_*`, `tc_*`. Never user-facing. */

That docstring is documentation patched over a structural problem: the prefixes it names are minted in literals scattered across the codebase, and one of them is already drifting.

Current state

All four generation sites, against origin/canary:

# File Line Prefix Random scheme
1 src/core/operator/approvalGate.ts 96 apr_ randomBytes(4).toString("hex") (8 hex chars)
2 src/core/operator/approvalGate.ts 211 act_ randomBytes(4).toString("hex") (8 hex chars)
3 src/core/operator/approvalGate.ts 275 tc_ randomBytes(4).toString("hex") (8 hex chars)
4 src/core/agents/offSecAgent/offensiveSecurityAgent.ts 504 tc_ Math.random().toString(36).slice(2, 8) (6 base36 chars)

The current literals (verbatim):

// src/core/operator/approvalGate.ts:96
id: `apr_${Date.now()}_${randomBytes(4).toString("hex")}`,

// src/core/operator/approvalGate.ts:211
id: `act_${Date.now()}_${randomBytes(4).toString("hex")}`,

// src/core/operator/approvalGate.ts:275
args.toolCallId || `tc_${Date.now()}_${randomBytes(4).toString("hex")}`;

// src/core/agents/offSecAgent/offensiveSecurityAgent.ts:504
`tc_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;

Drift is observable, not theoretical. Sites #3 and #4 both mint tc_ IDs that flow through gate.check(), sit side-by-side in approval-gate logs, and persist into ActionHistoryEntry.toolCallId. They use different charsets, different lengths, and different entropy sources (crypto.randomBytes vs Math.random). #4 isn't even cryptographically random.

Proposed fix — branded types on id fields

New file: src/core/operator/ids.ts (~22 LOC)

import { randomBytes } from "crypto";

declare const __brand: unique symbol;
type Branded<B extends string> = string & { readonly [__brand]: B };

export type ApprovalId = Branded<"ApprovalId">;
export type ActionId = Branded<"ActionId">;

const stamp = () => `${Date.now()}_${randomBytes(4).toString("hex")}`;

export const mintApprovalId = (): ApprovalId => `apr_${stamp()}` as ApprovalId;
export const mintActionId = (): ActionId => `act_${stamp()}` as ActionId;

/**
 * Tool-call IDs can also arrive from the AI SDK, so the return is plain
 * `string` — branding adds no value where external IDs share the field.
 * The win here is just collapsing two drifted minters (one cryptographic,
 * one `Math.random()`) into one shared shape.
 */
export const mintToolCallId = (): string => `tc_${stamp()}`;

src/core/operator/types.ts — 2 line changes

import type { ApprovalId, ActionId } from "./ids";

export interface PendingApproval {
  id: ApprovalId;     // was: string
  // ...
}
export interface ActionHistoryEntry {
  id: ActionId;       // was: string
  // ...
}

src/core/operator/approvalGate.ts — 4 line changes + 1 docstring deletion

import { mintApprovalId, mintActionId, mintToolCallId } from "./ids";
// ...
id: mintApprovalId(),                              // line 96
id: mintActionId(),                                // line 211
args.toolCallId || mintToolCallId();               // line 275

Delete the misleading /** Internal correlation IDs minted by this module: ... */ docstring at line ~25.

src/core/agents/offSecAgent/offensiveSecurityAgent.ts — 1 line change

import { mintToolCallId } from "../../operator/ids";
// ...
const toolCallId = args.toolCallId ?? mintToolCallId();   // line 504

Cascade impact

Narrowing the two id fields will surface ~5-8 typecheck errors in approvalGate.ts itself, each a 1-token edit:

  • pendingApprovals: Map<string, DeferredApproval>Map<ApprovalId, DeferredApproval>
  • approve(approvalId: string)approve(approvalId: ApprovalId)
  • deny(approvalId: string)deny(approvalId: ApprovalId)
  • batchApprove(approvalIds: string[])batchApprove(approvalIds: ApprovalId[])
  • OperatorEvent approval-resolved.idApprovalId
  • Operator dashboard handler picks it up automatically via inference

tsc tells you exactly where to go. One-time cost, surfaced at compile time. After it lands, the brand is invisible to day-to-day reading.

If ActionHistoryEntry ever gains a JSON-deserialization path (it doesn't today — the gate's history is in-memory and capped at 100), one as ActionId cast at that boundary handles it. Standard parse-don't-validate.

Desired state — externally observable, testable

  • Exactly one file in the repo defines apr_/act_/tc_ literal prefixes for ID generation. Verified by rg -n "(apr|act|tc)_\\\${" src returning zero matches outside src/core/operator/ids.ts.
  • mintApprovalId, mintActionId, mintToolCallId are the only producers of these IDs. Verified by rg -n "randomBytes\(4\)\.toString" src/core/operator src/core/agents returning only the call inside ids.ts.
  • tc_ IDs minted from approvalGate.ts and offensiveSecurityAgent.ts have byte-identical shape /^tc_\d+_[0-9a-f]{8}$/.
  • PendingApproval.id is typed ApprovalId. ActionHistoryEntry.id is typed ActionId. Neither is string.
  • The misleading docstring at the top of approvalGate.ts no longer exists.

Invariants and enforcement

Invariant Enforcement Strength
Only mintApprovalId() produces ApprovalId values Branded return type. Bypass requires explicit as ApprovalId cast — visible in code review. ✅ compiler
Only mintActionId() produces ActionId values Same. ✅ compiler
ApprovalId and ActionId cannot be confused as Map keys or event payloads Brands are nominally distinct. Map<ApprovalId, …> rejects ActionId. ✅ compiler
Both tc_ minters produce identical shape Single shared mintToolCallId() function. ✅ structural
A future inline tc_… literal in a fifth file is caught Convention + import discoverability. (Not branded because tool-call IDs share the field with external AI SDK IDs.) ⚠️ review

Why this is the least-burden version of structural prevention

  • Zero runtime cost. Brands compile to nothing. declare const __brand: unique symbol is type-level only.
  • Zero ongoing maintenance. No lint rule to maintain, no CI grep to update. tsc enforces forever.
  • Discoverability. Find References on ApprovalId lights up every place these IDs travel — better debugging than string.
  • Bypass leaves a fingerprint. Future drift requires an as ApprovalId cast, which is a review-flaggable smell.
  • Brand syntax is contained to one file. ~5 lines in ids.ts. New devs read it once.
  • Cascade is bounded. ~5 1-token edits in one file (approvalGate.ts), all surfaced by tsc. After landing, invisible.

Tests

src/core/operator/ids.test.ts (new, ~25 LOC):

  1. mintApprovalId() matches /^apr_\d+_[0-9a-f]{8}$/.
  2. mintActionId() matches /^act_\d+_[0-9a-f]{8}$/.
  3. mintToolCallId() matches /^tc_\d+_[0-9a-f]{8}$/.
  4. 1,000 calls in a tight loop produce 1,000 unique IDs per kind (collision smoke).
  5. Type-level: // @ts-expect-error proves mintApprovalId() is not assignable to ActionId.

Existing tests pass unchanged. approvalGate.test.ts uses literal tc_1tc_5 as caller-supplied toolCallIds — these are external-origin IDs, intentionally untouched.

Files touched

  • New: src/core/operator/ids.ts, src/core/operator/ids.test.ts
  • Edit: src/core/operator/types.ts (2 type narrowings)
  • Edit: src/core/operator/approvalGate.ts (4 site replacements + 1 docstring deletion + ~5 cascade edits in same file)
  • Edit: src/core/agents/offSecAgent/offensiveSecurityAgent.ts (1 import + 1 site replacement)

Net: ~+25 / −5 LOC of "real" change. Cascade is in-file in approvalGate.ts.

Non-goals (explicit, deferred)

  • ❌ Branding toolCallId. AI SDK supplies these too; the field is mixed-origin and the brand has nowhere clean to live.
  • ❌ Custom ESLint rule banning inline (apr|act|tc)_ literals. Brands cover the two cases that actually matter. Add a rule later if a fifth tc_ minter ever appears.
  • ❌ Migrating ID shape (UUID v7, ULID, etc.). Behavior change, separate concern.
  • ❌ Centralizing other correlation kinds (sessions, subagents, findings). Different shapes, different minters, different concerns.
  • ❌ Persistence-boundary cast for ActionHistoryEntry.id. The gate's history is in-memory and capped at 100. Add the cast when persistence is added.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    cleanupCleanup, refactoring, and maintenancegood first issueGood for newcomers

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions