Skip to content

Evict expired turn claims and stabilize turn listing order - #592

Merged
TheGreatAxios merged 2 commits into
mainfrom
cl-7200-evict-expired-turn-claims-and-give-turn-listings-a-stable
Sep 4, 2026
Merged

TheGreatAxios merged 2 commits into
mainfrom
cl-7200-evict-expired-turn-claims-and-give-turn-listings-a-stable

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Expired in-memory turn claims no longer linger in the Map until that workbench is claimed again. tryClaim, holds, and release delete an expired entry when they observe it. A mismatched release still cannot evict a live successor (CL-7129), but it does drop a dead expired entry instead of leaving a zombie.

Turn listing and the running-turn resolver now share the same newest-first key: startedAt descending, with occurrence as the deterministic tiebreak. Same-millisecond timestamps no longer let listing keep Map insertion order while findRunningTurn picked a different row. The Drizzle store uses the same ORDER BY started_at DESC, occurrence DESC.

Test plan

  • bun test packages/chat/src/turn-claims.test.ts packages/chat/src/agent-turns.test.ts packages/chat/src/turn-queue.test.ts — 47 pass, 0 fail (exit 0). New tests failed before the implementation (expired entry still releasable by the original token; listing order [first, second] vs running-turn second) and passed after.
  • bun run typecheck — exit 0.
  • bun run lint — prettier clean; eslint 0 errors / 10 pre-existing warnings.
  • bun run check:structural — ok (deletion, killdates, licenses, db-gate, and remaining structural checks).
  • Full bun run check reached the test step and failed in scripts/db-setup.test.ts with PostgresError: password authentication failed for user "thegreataxios" (pre-existing local DATABASE_URL, unrelated). Drizzle chat suites fail the same way.

CL-7200

Fixes CL-7200

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Fixes CL-7200

@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Self-review

Verdict: the change is correct and narrowly scoped — expired turn-claim entries are now evicted as a side effect of liveHolder in tryClaim/release/holds, and listTurns/findRunningTurn (both in-memory and Drizzle stores) now share one newest-first comparator (startedAt desc, occurrence desc tiebreak). No further fixes needed.

  • Zombie expired-claim eviction in turn-claims.ts (liveHolder): CONFIRMED-FIXED already in the PR — verified tryClaim, release, and holds all route through liveHolder, and the new tests prove eviction happens even on a mismatched-token release.
  • Shared compareTurnsNewestFirst in agent-turns.ts, applied to both listTurns and findRunningTurn, and mirrored in the Drizzle ORDER BY started_at DESC, occurrence DESC: CONFIRMED-FIXED already in the PR — in-memory and DB orderings now agree with each other on same-millisecond ties.
  • exactOptionalPropertyTypes hazards, bare catch {}, as T on untrusted input, fallbacks/legacy paths, product rules in apps/*: DISMISSED — none present in this diff; no catches, no casts, no optional-field assignment added.
  • Doc comments (release/holds JSDoc, compareTurnsNewestFirst): DISMISSED — explain the non-obvious "why" (incidental cleanup vs. a caller's own claim), not restatements of the code.

Targeted tests: bun test packages/chat/src/turn-claims.test.ts packages/chat/src/agent-turns.test.ts packages/chat/src/turn-queue.test.ts — 47 pass, 0 fail. bun run typecheck — pass.

No commits pushed; nothing to fix.

@linear-code

linear-code Bot commented Sep 4, 2026

Copy link
Copy Markdown

CL-7200

@TheGreatAxios
TheGreatAxios merged commit ae83c0b into main Sep 4, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant