[Bug] One repeated PTC sub-call id makes an entire Session unloadable (data loss)

[ujji-k06]

Impact: a run_code sub-call id can repeat inside one Session. The Session v4 format
requires subCallId to be unique, so the collision is written happily and only caught when
the log is next read — and reading is all-or-nothing, so one repeat costs the whole
session. A real 3,569-event session became permanently unopenable:

stored session "session-…" is corrupt:
stored log is corrupt: SessionFormatError: PTC dispatch repeats subCallId

No error at write time, no recovery in the product. Reproduced on the released
0.2.0-rc.2; the same code is present on master (d74326738).

Root cause

packages/core/tools/src/ptc.ts numbers each sub-call from a counter scoped to one
execute():

let dispatches = 0                                   // line 420
...
const n = ++dispatches                               // line 548
const subCallId = brandString<ToolCallId>(`${String(exec.callId)}:ptc:${n}`)

Both halves of that id may repeat within one Session, and each for its own reason:

• exec.callId — a provider tool-call id. It is not guaranteed to be unique per
  session; the provider re-issues it when the assistant repeats a call. In the corrupted
  session, every collision had its callId re-advertised in a later step (4 ids, each
  advertised once per step).
• n — restarts at 1 for every new execute(), which is intended and is pinned by
  the existing test at tests/ptc.spec.ts:625, where three sub-calls of one run read
  call-1:ptc:1..3.

A call that has to be re-issued therefore restarts the numbering while reusing the parent
id, and emits a second tool/ptc-dispatch-start carrying an id whose first dispatch is
already settled.

How it happened in practice

The trigger is a sub-call that outlives its step — polling a background job is the natural
one:

// step 1 — the poll does not return in time
await tools.bash({ command: 'sleep 58; ls .some-dir | grep -c done; date +%H:%M:%S',
                   description: 'poll' })
// step 2 — the same call re-issued, same callId, counter back at 1

Every colliding id had exactly this shape: one dispatch per step, each immediately
following its own re-advertisement of the same callId.

colliding subCallId	assistant advertisements (seq)	dispatch starts (seq)
call_0_ca5abc1b57133bfb:ptc:1	2791, 2817, 2843	2795, 2821, 2847
call_0_970ffbdb73359cd:ptc:1	3196, 3212	3200, 3216
call_0_91366f045c2e7d01:ptc:1	3419, 3432	3423, 3436
call_0_b844fbaca8e0ee8:ptc:1	3458, 3471, 3484, 3497	3462, 3475, 3488, 3501

4 colliding ids across 357 dispatches — enough to lose the session. Note it needs a
single dispatch per step: ordinary multi-call usage takes :ptc:1 and :ptc:2 inside
one run and never reaches this.

Why it is not caught earlier

• packages/core/tools/src/invariant.ts already walks every session event, but checks only
  non-emptiness, root consistency, and turn enclosure — a repeated subCallId with an
  unchanged rootCallId passes both of its checks.
• The uniqueness requirement lives in the format validator,
  packages/session/session-format-v3-to-v4/src/relationships.ts:190:

  if (event.type === 'tool/ptc-dispatch-start') {
    if (existing !== undefined) throw new SessionFormatError('PTC dispatch repeats subCallId')

So the invariant the producer must satisfy is enforced by a different package at read time.
Worth deciding deliberately: either the append path should reject a repeated id (fail fast,
session stays usable) or the producer should make repeats impossible. The patch below does
the second; the first seems worth having anyway, as a separate change.

Proposed fix

Scope the counter to the Session rather than one execution. execute already holds the
session (exec.agent?.session, used by both append sites at lines 590 and 617), so no
execution-model change and no new persisted state is needed.

--- a/packages/core/tools/src/ptc.ts
+++ b/packages/core/tools/src/ptc.ts
+/**
+ * Dispatch numbering must be unique across a whole Session, not one `execute()`:
+ * a re-issued `run_code` call shares its `callId`, and the Session format refuses
+ * a repeated `subCallId` — one collision makes the whole log unloadable.
+ * Process-local on purpose: a counter that resumed with the log could reach an
+ * id already restored from it.
+ */
+const dispatchCounters = new WeakMap<Session, { value: number }>()
+const sessionlessDispatchCounter = { value: 0 }
+
       const runController = new AbortController()
       const onOuterAbort = (): void => { runController.abort(exec.signal.reason) }
       exec.signal.addEventListener('abort', onOuterAbort, { once: true })
 
-      let dispatches = 0
       ...
         const normalized = jsonNormalizeArgs(rawArgs)
-        const n = ++dispatches
+        // Without a session no PTC event is appended at all, so the shared
+        // fallback only has to keep ids distinct, never to correlate.
+        const session = exec.agent?.session
+        let counter = sessionlessDispatchCounter
+        if (session !== undefined) {
+          let existing = dispatchCounters.get(session)
+          if (existing === undefined) {
+            existing = { value: 0 }
+            dispatchCounters.set(session, existing)
+          }
+          counter = existing
+        }
+        const n = ++counter.value
         const subCallId = brandString<ToolCallId>(`${String(exec.callId)}:ptc:${n}`)

Session needs a type import for the WeakMap key. The package has no runtime dependency on
@deepseek-ai/dsh-session (it is a dev-dependency, and tests/ptc.spec.ts already imports
Session), so this must be a type-only import; invariant.ts types a WeakMap keyed by
session without any explicit import, which suggests the ambient path may suffice — worth
checking against the repo's conventions before applying.

Constraints that shape it

• In-memory, never persisted. The validator requires uniqueness across the whole stored
  log, so any counter that resumes with the log can reach a restored id and corrupt it
  again. A WeakMap starts empty each process and can only move forward.
• <parent>:ptc:<n> keeps its shape. README.md:129 states consumers treat these ids
  as opaque and correlate by exact equality, and nothing parses the suffix — so re-scoping
  n is invisible downstream.
• Existing behaviour is preserved. Three sub-calls inside one run still read
  :ptc:1..3, so the assertion at tests/ptc.spec.ts:625 keeps passing. The counter is
  session-wide, so a second run_code call in the same session continues the numbering
  rather than restarting — which is the point.
• No new field on ToolExecution. Anchoring on session avoids widening the frozen
  execution type.

Worth one sentence in README.md (and README.zh.md) beside the existing id contract:
that n is unique per Session, not per run_code execution. That is the fact the current
wording leaves open, and the one this bug fell through.

Two things to confirm at apply time

1. Whether exec.agent is always present for a run_code execution. Both append sites
   reach through exec.agent?.session, so it should be — but the fallback branch above
   should be dropped if a session-less dispatch-start cannot reach the log at all.
2. The Session type import path/syntax per the repo's conventions.

Regression test

packages/core/tools/tests/ptc.spec.ts already has everything needed (fakeAgent,
runCode). Two runCode calls share ToolCallId('call-1') deliberately, matching the
real failure:

it('keeps sub-call ids unique when one run_code call is re-issued in a session', async () => {
  const { ctx, runtime } = await setup({ mode: 'ptc' })
  ctx.tools.register(defineTool({
    name: 'echo', description: 'Echo a value.',
    parameters: { id: { type: 'string', required: true } },
    output: { schema: { type: 'string' }, render: (_args, value) => [{ type: 'text', text: value }] },
    execute: async (args) => args.id,
  }))
  const { agent, events } = fakeAgent()
  runtime.behavior = async (request) => {
    await request.bindings[0]!.functions.echo!({ id: 'x' })
    return { logs: [] }
  }

  // Same callId, as the provider delivers when the assistant repeats a call.
  await runCode(ctx, 'program', { agent })
  await runCode(ctx, 'program', { agent })

  const starts = events
    .filter(event => event.type === 'tool/ptc-dispatch-start')
    .map(event => (event.data as { subCallId: string }).subCallId)
  expect(starts).toHaveLength(2)
  expect(new Set(starts).size).toBe(2)               // the invariant that was violated
  expect(starts).toEqual(['call-1:ptc:1', 'call-1:ptc:2'])
})

Today that fails with starts === ['call-1:ptc:1', 'call-1:ptc:1'] — the exact state the
validator rejects on load. A second case could assert that the single-run numbering still
reads :ptc:1..3.

Verification done here

• The failure is reproduced from a real stored log through the same reader the harness uses
  (JsonlSessionPersistence.requireStoredLog), and the error above is the verbatim result.
• The counter logic above was run as a standalone transcription: it yields :ptc:1..4 for
  one call re-issued over four steps, where the current code yields :ptc:1 four times.
• The storage layout, the two append sites, and the validator rule were read from the
  released build and from master.

Not claimed: the diff has not been compiled or run inside the repository — it is written
against the sources above, so treat the hunk as a proposal to adapt, not a tested patch.

---

Context, if useful: I hit this on a session that had been polling a background job, and
repaired the stored log with a script that renumbers the colliding dispatch pairs
(nothing else in the log is touched — those two event types are the only ones carrying a
PTC id, and a start is always followed by its own settle). That is a workaround for
already-damaged logs, not a substitute for the fix.

[argszero]

Source-level confirmation, one correction, and where the seam actually is.

Traced on master d743267388. Both halves of your root cause are exactly right — packages/core/tools/src/ptc.ts:546 builds subCallId as ${exec.callId}:ptc:${n}, dispatches is declared per execute() at :420, and the two append sites are :590 (settle) and :616 (start). The asymmetry is real: the write path accepts the repeat, and the first enforcement point is a full-log read.

One correction: the in-tree backstop you cite is no longer on master. packages/core/tools/src/invariant.ts was deleted by f028f25667 ("refactor: remove runtime invariant plugins", 2026-09-30), which removed @deepseek-ai/dsh-invariants, every package's ./invariant companion, the invariant gates and the INVARIANT-coded rethrow paths. At its last revision that module did walk both tool/ptc-dispatch-start and tool/ptc-dispatch, and its checks were exactly the three you describe — non-emptiness of the three ids, root stability, and parent-belongs-to-root. Note how it checked root stability: dispatchRoots: Map<subCallId, rootCallId> was written on every dispatch event and only read for the "changed rootCallId" test, so a repeat carrying the same root overwrote the entry and passed. Your reading holds for that file; the difference is that today there is no write-time walker at all, not even that weak one.

The two sites, precisely. On append, Session.append runs validateSessionEventData (packages/core/session/src/index.ts:747, defined at packages/core/session/src/surface.ts:166) — that is payload-local only (request-header fields, developer roles, tool-error metadata) — plus SurfaceManager.validateNext (:748), which is stateful but scoped to the surface contract. Durably, a batch passes materializeAppendBatch (lossless-JSON snapshot) and assertContiguous (seq order) — packages/session/session-persistence/src/storage-contract.ts:131 and :145. No PTC relationship is examined anywhere on the write side. On read, JsonlSessionLogScan.finish() calls assertReleasedV4Relationships (packages/session/session-persistence-jsonl/src/format.ts:468), which reaches packages/session/session-format-v3-to-v4/src/relationships.ts:190 and throws; packages/session/session-persistence-jsonl/src/index.ts:734 is what turns it into the stored log is corrupt: … line you quoted. So one repeat costs the whole log because the only check is a whole-log check.

On your "worth deciding deliberately" note: the append path does already contain one stateful cross-event validator (SurfaceManager.validateNext), so an id-uniqueness check has a shape to sit in rather than needing a new mechanism — it would be a second per-session state map, not a widening of the surface one, whose contract is surface-only.

Where a plugin can hold this today. The gap is plugin-mountable on the producer side of the log, and the seam is ctx.sessions. It is SessionStore extends Service (packages/core/session/src/index.ts:921), create() delegates to prepare() (:982), and prepare (:1014) is the single construction path for agent sessions — the agent factory calls it with { seed: [...persisted, ...closers] } (packages/core/agent-loop/src/index.ts:857). A plugin can shadow that service instance's prepare and install an own-property append wrapper on the returned Session; one own-property write covers every caller, because a freshly created ctx.get() view hands back a proxy and the underlying instance is reached through Symbol.for('cordis.original'). Renaming a repeated subCallId there lands in the in-memory log and therefore in every durable batch, and the matching settle can be renamed with it through a FIFO map keyed by the original id (starts are strictly submission-ordered and commits drain in submission order, so the pairing is well defined). Uniqueness stays decidable without any prohibited history read: a resumed session's prefix arrives synchronously in the prepare option bag as options.seed, which matters because snapshotEvents / eventAt / ownEvents are closed to new callers (the 2026-09-09 note on synchronous session-event reads). The one path that shadow does not cover is a detached Session built directly through Session.create (e.g. packages/session-query/session-query/src/index.ts:186) — those do not carry PTC dispatches.

Interim answer to your last line: with the invariant runtime gone and the format validator reachable only on read, your option one (the append path refusing a repeated id) is the only thing that can keep the session usable at the moment of the mistake. Until something enforces the invariant before the write, producer-side uniqueness — or the same enforcement applied at the append boundary from outside the tree — is the whole distance between a re-issued run_code call and a session nobody can open again.

[argszero]

A guard in plugin form, published and installable — @argszero/dsh-ptc-subcall-guard@0.1.0.

Following the analysis above, the seam that is actually reachable from a plugin is the sessionPersistence service instance, not the session store: open and create are prototype methods on the shipped class, so one own-property write on the object behind the ctx.get view shadows both for every caller — including the agent loop, which never looks at a plugin. On the write handle that call returns, append is wrapped.

What it does with a colliding batch:

• a start whose subCallId this session has already started is renamed to ${original}::r1, ::r2, … — the producer mints ${callId}:ptc:${n} and never carries ::r, so the two namespaces cannot collide, and the candidate is checked against the used set anyway;
• the paired settle, which still arrives under the producer's id, is carried to the renamed id through a FIFO keyed by the original — so the log is not left holding a settle beside a start that no longer carries that id;
• read is observed, so the ids already in the stored prefix prime the used set: a guard mounted mid-session renames against the whole log, not only what it appended;
• rootCallId and parentCallId are never rewritten, so a nested dispatch's parent lookup still resolves.

Nothing is dropped — the batch stays complete and the log stays loadable. The collision is logged instead of being written down as a session that will not open again.

Against the real backend and the shipped validator, the unguarded shipment reproduces your failure (SessionPersistenceCorruptionError: session "…": stored log is corrupt: SessionFormatError: PTC dispatch repeats subCallId), and the same shipment with the guard mounted re-opens with 3 events and ids call-1:ptc:1, call-1:ptc:1::r1.

npm install --save @argszero/dsh-ptc-subcall-guard

- insert:
    - id: ptc-subcall-guard
      name: '@argszero/dsh-ptc-subcall-guard'
      inject:
        - sessionPersistence

The package ships its own bundle patch, so a profile that installs it by name gets that row without writing it by hand.

Configuration: enabled (default true), mode (repair, the default, or observe, which reports the collision and appends the batch unchanged — for measuring the shape before repairing it), maxReportedRepairs (20 per session) and maxTrackedSessions (128, least recently opened dropped). An unknown key is refused at mount rather than ignored, because a misspelled mode that mounts and guards nothing is how an operator comes to believe the log is protected.

Limits, stated plainly: it prevents the write and repairs nothing already on disk, so a session that already carries a duplicate stays unloadable; it cannot wrap a write handle obtained before it mounted; and the repair is a rename, so anything that reads a subCallId back from the log sees the guarded spelling. If the better fix is upstream — minting an id that is stable across re-issues, or consulting the relationship on the write path — this is a stop-gap for deployments that cannot wait, and I would be glad to see it made unnecessary.

Source, tests and a mutation harness: https://github.com/argszero/dsh-ptc-subcall-guard — probed against the 0.1.7-rc, 0.2.0-rc and 0.2.1-alpha lines, zero runtime dependencies.

[xiaoshenming]

两条实测补充，一条可能对你们的修法有用。

1）这道校验只有关系折叠在做 —— Session.fromRestore 全部放行。

我把你贴的那六种形状各变异一条到本机一份真实 v4 会话上（它正好带一对 tool/ptc-dispatch-start + tool/ptc-dispatch），逐条跑两个读端：

变异	assertReleasedV4Relationships（折叠）	Session.fromRestore（seed）
第二条 start 重复已结算的 subCallId	拒（同族）	accept
tool/ptc-dispatch 复用已结算的 id	拒（同族）	accept
dispatch 的 name 与 start 不同	PTC dispatch does not match its start	accept
rootCallId / subCallId 清空	PTC rootCallId requires a nonempty string 等	accept
parentCallId 不属于 rootCallId	PTC parentCallId does not belong to rootCallId	accept

所以：只跑 Session.fromRestore 的校验器看不见这一族，而它恰好是不少工具会用的那条路径。反过来也说明折叠是唯一会拒的闸门 —— 也就是打开一份已存日志时跑的那道，你的「一重复赔掉整条会话」因此没有任何更早的拦截点。这条我写进了我们自己的 REPAIR-SPEC，因为它是我们前三族形状里第一次出现的分工（那三族都是两道闸门一起拒）。

顺带：本机 13 份会话里只有一份真的带 PTC dispatch，六种变异它全部命中而健康的那对报 ok —— 假阳性这一侧我们核过了。

2）你报的这个具体形状，看起来是能修的 —— 我们还没做，先把判据说出来。

subCallId 在折叠里只有两种用法：作为 dispatch 自己的键，以及被后代的 parentCallId 引用。所以你那条 exec.callId 被重发、计数器回到 1 的第二次调用，唯一错的就是键本身 —— 它的 rootCallId/parentCallId/name/arguments 与已经结算的前一次逐项相同（这正是你说的每个碰撞 id 的形状）。那种情况下把第二次那一对（它自己的 tool/ptc-dispatch-start 和它的 tool/ptc-dispatch）一起改名成该 parent 下下一个未占用的 <parent>:ptc:<n>，就是把「计数器没有重启时写者本该产出的东西」还原出来，不丢任何内容。前提是可检的：没有任何其他行把这个旧 id 当 parentCallId 用（有就说明有子调度挂在上面，那就不该动），以及重复的那对能唯一定位。

我们目前只做到「报告」这一步（ptc-dispatch-invalid，带 seq 与具体违规），重编号的写法还没实现，所以我不会说「已经能修」；上面那段是我们要动手时依的判据，先放出来给讨论用。

3）给写入侧的一点观察。

你们已经把 n 的语义钉在测试里了（call-1:ptc:1..3），所以我不建议改 n —— 让 id 在会话范围内唯一更省事：把 execute() 的序号或一个会话内的运行计数并进前缀（<callId>:<run>:ptc:<n> 之类），这样父 id 被重发时两半都不会再撞。exec.callId 本身不是会话唯一的这一点，你们的分析已经写清楚了。

[argszero]

Both halves of your split check out at source, and the renumbering you describe is already implemented and published — with one difference in the scheme.

1. The fold is the only rejecting reader — confirmed, and it is reached through exactly one call site.

• The relationship fold lives in packages/session/session-format-v3-to-v4/src/relationships.ts: dispatch() rejects a repeated id at its start branch (PTC dispatch repeats subCallId) and compares the four fields (rootCallId / parentCallId / name / arguments) in the non-start branch (PTC dispatch does not match its start).
• Its only caller is packages/session/session-format-v3-to-v4/src/validation.ts:129 (assertV4LifecycleRelationships).
• Session.fromRestore (packages/core/session/src/index.ts:608) validates two things only — that each seed event is losslessly JSON-serializable and that seq runs contiguously from 0 — and never enters the relationship fold. So your table is right: create(id, { seed }) accepts the shapes the decode path rejects, and the fold is the sole gate. That is also why the damage is one-shot and total: the log is only ever rejected when someone folds it, and folding is all-or-nothing.

2. The second-pair rename exists as a published plugin.

@argszero/dsh-ptc-subcall-guard@0.1.0 (npm, repo argszero/dsh-ptc-subcall-guard) does exactly the move you describe and nothing else: it renames the duplicate occurrence at construction time, in the seed the agent factory hands ctx.sessions.create(...), and removes no event.

One scheme difference worth stating, because it affects your predicate:

• You propose recomputing the second pair as the next unoccupied <parent>:ptc:<n>. The plugin instead suffixes the duplicate occurrence: call-1:ptc:1 → call-1:ptc:1::r1. The first id — the one an existing parentCallId may already reference — is never touched, so a child row pointing at the original id still resolves.
• Its rename condition is weaker than yours: it renames an occurrence when the id repeats an already-recorded tool/ptc-dispatch-start, without requiring the two field sets to be identical. If the first of the pair is the bogus one, that is the wrong row to keep, and your "the pair is field-identical (writer counter restart)" condition plus your "no other row uses the old id as parentCallId" check are the safer predicate. I would take your condition over ours.
• It hooks the persistence entry points (SessionPersistence.create / open) rather than the format validator, because that is the boundary both writers and readers pass.

End-to-end witness, against the real dsh-session-persistence-jsonl backend: opening a stored log that carries the duplicate raises SessionPersistenceCorruptionError: ... SessionFormatError: PTC dispatch repeats subCallId; with the guard mounted, the same log opens and reports call-1:ptc:1, call-1:ptc:1::r1. So the "already damaged log becomes openable again" half is measured, not just reasoned.

3. On your write-side suggestion — agreed, and I would add where the id comes from: the parent half is exec.callId, which is a per-execute() identity and gets re-sent when a run_code call is retried, while the ptc:<n> half restarts with the counter. Making the prefix session-scoped (your <callId>:<run>:ptc:<n>) removes the collision without touching the ptc:<n> grammar that the tests pin. It does not repair logs already written, which is why both halves are worth having.

One correction to my own earlier reply in this thread in light of your point 1: I framed the write path as "unvalidated" — the sharper statement is that the write path validates JSON shape and sequence, and the relationship invariants are enforced only by the decode fold, so the asymmetry is between two different validators, not between validation and its absence.

[xiaoshenming]

谢谢，两点都接受，而且你的方案比我的好 —— 我按它实现了。

1）我原来的方案有个你没提但确实存在的洞。 我提的是「把第二次那一对重编号成该 parent 下下一个未占用的 <parent>:ptc:<n>」，这会把第一次的 id 也卷进重命名的范围（如果我想把编号排整齐），而那个 id 可能已经被某个子调度写进 parentCallId。你的加后缀方案把第一次的 id 完全隔离在外，子行继续解析得到 —— 我采用的就是它。

2）一个实现细节，我们这边踩到了，供你核对。 改名不能只改 tool/ptc-dispatch-start：它的 tool/ptc-dispatch 也带着同一个 subCallId，只改 start 的话那行会落到第一次的条目上，而那条已经 settled → 折叠报 PTC dispatch has no unique start。所以我这一侧的范围是「后一次出现的 start，加上它到下一个同名 start 之前所有点名该 id 的行」一起改。如果你的守卫只改 seed 里的 start 行而放行通过，那说明 seed 那条路径上 dispatch 行不在同一个检查里 —— 值得确认一下，因为落盘重读时它在。

3）你的守卫和文件级修复解决的是两个不同的面，所以我们两个都做了一遍。 你在 seed 上修，进程内就打开了；但磁盘上那份文件仍然是拒读的 —— readStoredLog 还是拒，会话检索、另一个没装守卫的客户端、以及任何别的读端也一样。我这轮把同一个动作实现到了已存文件上：

• 新 id = 旧 id + 最小的空闲 ::r<k>，空闲指全文件范围内既没被用作 subCallId 也没被用作 parentCallId（所以不会撞上已有的子引用）；
• 只动后一次出现，第一次的 id 一个字节都不碰；不删行、不改其它成员；
• 改完重跑一遍检测，仍不干净就整体 refuse —— 不许用改名去遮一个「因为它坏」的行（name 与 start 不符、rootCallId 清空、parentCallId 不属于 root 这些仍然只报告）。

实测（真实 4376 事件会话，带一对 dispatch）：造出 #9677 形状后折叠报 PTC dispatch repeats subCallId、seed 放行；改名后两道闸门都接受、health=ok、再跑一次修复计划不再写（幂等），4376 行里只动了 2 行；健康会话 mustWrite=false。

我把它作为子集实现在 dsh-session-surgeon 里（repairPtcSubCallIds），并按贡献写着你的名字与 dsh-ptc-subcall-guard 的来源 —— 如果你觉得判据还有该收紧的地方（尤其是那个「更弱的改名条件」，我读到那句被截断了），直接说，我改。