mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix: classify session fork errors
This commit is contained in:
@@ -402,7 +402,7 @@ snapshot(source: SessionForkSource): SessionForkSeed
|
||||
fork(options: ForkSessionOptions): Session
|
||||
```
|
||||
|
||||
Source: [`packages/session-fork/session-fork/src/index.ts:63`](../../packages/session-fork/session-fork/src/index.ts)
|
||||
Source: [`packages/session-fork/session-fork/src/index.ts:65`](../../packages/session-fork/session-fork/src/index.ts)
|
||||
|
||||
### `ctx.sessionPersistence` — `SessionPersistence` (abstract seam)
|
||||
|
||||
|
||||
@@ -44,7 +44,7 @@ class SessionForkService extends Service {
|
||||
|
||||
`fork()` is the convenience half. It calls `snapshot()`, then creates a live child session via `ctx.sessions.create(sessionId, { seed, meta })`. The child inherits the source session's `cwd`, stamps `parentSession` to the source id, and sets `seedLength` to the seeded prefix length. When `sessionId` is omitted, `SessionStore` generates one using its existing id policy.
|
||||
|
||||
The boundary rule is structural: an empty source log is forkable, and any source whose last event is `turn/end` is forkable regardless of the turn-end reason (`completed`, `aborted`, `error`, `disposed`, `max-tokens`, `interrupted`, or a future merge-extensible reason). Any non-empty source whose last event is not `turn/end` is inside a turn or otherwise not at the boundary and is rejected with a typed `SessionForkError` code. This is intentionally stricter than `dsh-subagent-fork`, whose completed-prefix clipping remains unchanged because it serves tool-time delegation rather than user/session branching.
|
||||
The boundary rule is structural: an empty source log is forkable, and any source whose last event is `turn/end` is forkable regardless of the turn-end reason (`completed`, `aborted`, `error`, `disposed`, `max-tokens`, `interrupted`, or a future merge-extensible reason). Any non-empty source whose last event is not `turn/end` is inside a turn or otherwise not at the boundary and is rejected with a typed `SessionForkError` code. The service also classifies non-live source ids (`SESSION_NOT_FOUND`), stale `Session` object references whose id is live on a different instance (`SESSION_NOT_LIVE`), and duplicate requested child ids (`SESSION_ALREADY_EXISTS`) instead of leaking raw store errors. This is intentionally stricter than `dsh-subagent-fork`, whose completed-prefix clipping remains unchanged because it serves tool-time delegation rather than user/session branching.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
@@ -21,7 +21,9 @@ Forking inside a turn is rejected with `SessionForkError` code `OPEN_TURN`. The
|
||||
|
||||
| Code | Meaning |
|
||||
|---|---|
|
||||
| `SESSION_NOT_FOUND` | A source id is not live in `ctx.sessions`, or a passed `Session` object is not the live store object for its id. |
|
||||
| `SESSION_NOT_FOUND` | A source id is not live in `ctx.sessions`, or a passed `Session` object's id is not live in the store. |
|
||||
| `SESSION_NOT_LIVE` | A passed `Session` object has a live id in the store, but it is not that live store instance. |
|
||||
| `SESSION_ALREADY_EXISTS` | The requested child `sessionId` is already live in `ctx.sessions`. |
|
||||
| `OPEN_TURN` | The source log is non-empty and does not end at `turn/end`. |
|
||||
|
||||
## Persistence
|
||||
|
||||
@@ -46,6 +46,8 @@ export interface ForkSessionOptions {
|
||||
|
||||
export type SessionForkErrorCode =
|
||||
| 'SESSION_NOT_FOUND'
|
||||
| 'SESSION_NOT_LIVE'
|
||||
| 'SESSION_ALREADY_EXISTS'
|
||||
| 'OPEN_TURN'
|
||||
|
||||
/** Typed error for service-level fork rejections. */
|
||||
@@ -94,6 +96,9 @@ export class SessionForkService extends Service {
|
||||
*/
|
||||
fork(options: ForkSessionOptions): Session {
|
||||
const snapshot = this.snapshot(options.source)
|
||||
if (options.sessionId !== undefined && this.ctx.sessions.get(options.sessionId) !== undefined) {
|
||||
throw new SessionForkError(`session "${options.sessionId}" already exists`, 'SESSION_ALREADY_EXISTS')
|
||||
}
|
||||
return this.ctx.sessions.create(options.sessionId, {
|
||||
seed: snapshot.seed,
|
||||
meta: snapshot.meta,
|
||||
@@ -108,9 +113,10 @@ export class SessionForkService extends Service {
|
||||
}
|
||||
|
||||
const live = this.ctx.sessions.get(source.id)
|
||||
if (live !== source) {
|
||||
if (live === undefined) {
|
||||
throw new SessionForkError(`session "${source.id}" not found`, 'SESSION_NOT_FOUND')
|
||||
}
|
||||
if (live !== source) throw new SessionForkError(`session "${source.id}" is not the live store instance`, 'SESSION_NOT_LIVE')
|
||||
return source
|
||||
}
|
||||
|
||||
|
||||
@@ -127,6 +127,15 @@ describe('SessionForkService', () => {
|
||||
.toThrow(new SessionForkError('session "detached" not found', 'SESSION_NOT_FOUND'))
|
||||
})
|
||||
|
||||
it('rejects a stale Session object whose id is live on a different instance', async () => {
|
||||
const { ctx, fork } = await setup()
|
||||
ctx.sessions.create(SessionId('same-id'))
|
||||
const stale = new Session(SessionId('same-id'))
|
||||
|
||||
expect(() => fork.snapshot(stale))
|
||||
.toThrow(new SessionForkError('session "same-id" is not the live store instance', 'SESSION_NOT_LIVE'))
|
||||
})
|
||||
|
||||
it('rejects non-empty logs whose last event is not turn/end', async () => {
|
||||
const { ctx, fork } = await setup()
|
||||
const cases: [string, (session: Session) => void][] = [
|
||||
@@ -184,6 +193,16 @@ describe('SessionForkService', () => {
|
||||
expect(firstUserMessage(source.events).data.content).toEqual([{ type: 'text', text: 'hello' }])
|
||||
})
|
||||
|
||||
it('rejects a child session id that is already live with a typed fork error', async () => {
|
||||
const { ctx, fork } = await setup()
|
||||
const source = ctx.sessions.create(SessionId('parent'))
|
||||
appendClosedTurn(source)
|
||||
ctx.sessions.create(SessionId('child'))
|
||||
|
||||
expect(() => fork.fork({ source, sessionId: SessionId('child') }))
|
||||
.toThrow(new SessionForkError('session "child" already exists', 'SESSION_ALREADY_EXISTS'))
|
||||
})
|
||||
|
||||
it('persists a forked child seed through the existing session write path', async () => {
|
||||
const root = await tempRoot()
|
||||
const ctx = new Context()
|
||||
|
||||
Reference in New Issue
Block a user