jobizzness / jobizzness/bench

entry.turnsTaken goes stale for any specialist stopped and revived within one daemon uptime

Open Beginner friendly
#39 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
1
Forks
0
Avg merge
1h 21m
Merged PRs (30d)
41

Description

## Evidence

Found while building #38 (persisting a report at clearContext).

`entry.turnsTaken` (`src/daemon/registry.ts:48`) is documented at its one read site as always correct:

```ts
// Pick up the numbering where it stopped, or this turn writes over
// the last one's report.
startTurn: entry.turnsTaken,
```
(`registry.ts`, `revive()`, ~line 1074)

But it is only ever *written* in two places:

- `registry.ts:598` — `turnsTaken: await latestTurn(rec.reportsDir)`, when the registry restores entries from disk at daemon boot
- `registry.ts:973` — `turnsTaken: 0`, when a brand-new specialist is created

Nothing updates it while a specialist is live. `ClaudeSession` tracks the true count itself (`session.turn`), and the comment at `send()` (~line 1091) says as much: *"`entry.turnsTaken` is only live right after create()/restore(); once a session is running, its own counter is the true one."* That's fine as long as `entry.session` stays alive. But the `exit` handler (`registry.ts:756-769`) drops the session reference on *every* exit — including an asked-for one (`stop()`, `setRole()`, or, before #38, `clearContext()`) — without ever copying `session.turn` back into `entry.turnsTaken`:

```ts
session.on("exit", (code, stderr) => {
const entry = this.entries.get(id);
if (entry) {
this.remember(this.billInterrupted(entry, session.runningTurn));
entry.alive = false;
entry.session = null; // <- the true counter goes with it
}
...
```

So: create a specialist, let it take a few real turns (`entry.turnsTaken` stays `0` the whole time, since it's never bumped), then `stop()` or `setRole()` it. The next prompt calls `revive()` with `startTurn: entry.turnsTaken` = `0`, and the fresh `ClaudeSession`'s next turn is numbered `1` — silently overwriting `reportsDir/1`'s existing report from the specialist's real first turn.

`clearContext` itself no longer hits this after #38, because it now reads `entry.session?.turn` directly (while the reference is still live, before calling `.stop()`) to reserve the next slot, and writes the correct value back into `entry.turnsTaken` itself. But `stop()` and `setRole()` still go through the exit handler with no such reservation, and remain exposed.

## Acceptance criteria

- [ ] `entry.turnsTaken` reflects the true turn count after any exit, asked-for or not — most simply, set `entry.turnsTaken = session.turn` in the `exit` handler before dropping the reference
- [ ] A specialist that takes N real turns, then is stopped (`stop()`) or re-roled (`setRole()`) without a daemon restart in between, revives with its next turn numbered N+1, not 1
- [ ] A regression test: take several turns, call `stop()`, revive with a new prompt, assert the turn directory used is N+1

## Out of scope

- The clearContext path itself — already fixed by #38, which reserves the slot directly rather than relying on this general mechanism
- Any change to how `entry.turnsTaken` is computed at boot (`latestTurn`) — that path is already correct

## Verification commands

```
npx vitest run tests/registry.test.ts
npx tsc --noEmit
```
Manual check a green build wouldn't catch: without the fix, a specialist that takes 2+ turns and is then stopped and revived will overwrite `reportsDir/1/report.html` on its next turn - diff the file before/after to see it happen.

## Related

Found while building #38.

Contributor guide

Open the contributing guide

Research direction

Start in src/daemon/registry.ts at the exit handler around lines 756-769 and inspect the existing turn tracking in ClaudeSession. Add a regression case in tests/registry.test.ts that takes several turns, stops and revives a specialist, then verifies the next report uses N+1. Run npx vitest run tests/registry.test.ts and npx tsc --noEmit.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend, devtools
Issue type
Bug
Difficulty
2/5
Estimated time
Half a day
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.