[codex] fix(heartbeat): retain session retry issue lock

[kkroo]

What changed

• Keep an issue execution lock attached to scheduled session_unavailable and zero_token_session_reset retries.
• Refresh the lock owner metadata when the retry replaces its failed source run.

Why

Those two retry families were added to the execution-lock gate, but the scheduler still cleared the issue lock after creating the retry. A second run could claim the issue during the scheduled window.

Validation

• pnpm --filter @paperclipai/server typecheck
• pnpm exec vitest run server/src/tests/heartbeat-retry-scheduling.test.ts -t 'schedules and finitely exhausts|schedules .*while retaining its execution lock' --reporter=verbose --silent
• pnpm exec vitest run server/src/tests/heartbeat-workspace-branch-containment.test.ts -t 'contains mid-change branch divergence at workspace-runtime fresh worktree reuse' --reporter=verbose --silent

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 34d2325

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• server/src/services/heartbeat.ts:14683 transfers the execution lock to the scheduled session-recovery run atomically, while preserving the established release behavior for unrelated retry families.
• server/src/__tests__/heartbeat-retry-scheduling.test.ts:2615 covers both session-unavailable retry paths and verifies the scheduled run owns the retained lock.

Recommended Action

1. Merge when the required checks pass.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 34d2325

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• server/src/services/heartbeat.ts:14683 transfers the execution lock to the scheduled session-recovery run atomically, while preserving the established release behavior for unrelated retry families.
• server/src/__tests__/heartbeat-retry-scheduling.test.ts:2615 covers both session-unavailable retry paths and verifies the scheduled run owns the retained lock.

Recommended Action

1. Merge when the required checks pass.

[allyblockcast]

Hey @kkroo! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".
• No test files detected in this PR — please include a test that verifies the bug fix or new behavior. If this PR genuinely doesn't need a test (e.g. a refactor), please retitle with refactor: prefix.

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[kkroo]

Closing this — the diagnosis behind it doesn't hold, and #1298 fixes the same failure the other way. Full write-up in BLO-25610.

Why this is the wrong direction. The premise here was "the scheduler still cleared the issue lock after creating the retry ... a second run could claim the issue during the scheduled window." A second run claiming the issue during that window is handled, and deliberately:

// heartbeat.ts:1186
function issueExecutionRetryLockAvailable(currentExecutionRunId, run) {
  return (
    currentExecutionRunId == null ||
    currentExecutionRunId === run.id ||
    (run.retryOfRunId != null && currentExecutionRunId === run.retryOfRunId)
  );
}

null counts as available, so a scheduled retry with no lock is not cancelled when it comes due. And if another run really has claimed the issue, executionRunId is neither null nor ours, so the retry is correctly cancelled with issue_execution_lock_changed — the second run legitimately owns the issue and the stale retry stands down. That gate is doing its job.

The failing expectation came from 76304affd, which landed after 8446c1011 fix(heartbeat): bind issue locks only for running runs narrowed executionRunId to a running-run lock. The test encodes the pre-8446c1011 contract. The same test file already asserts the null contract a few tests earlier (the staleLock block), so master currently contains two contradictory expectations for the same field — that's the semantic merge collision, not a scheduler bug.

And the cost. This carve-out ("Other retry families retain their existing release-at-schedule behavior") would have two retry families hold an issue lock across an arbitrary delayMs window while nothing is running — precisely the stale-lock class that 8446c1011 added recovery-stale-issue-lock-sweep and issue-stale-execution-lock-routes to clean up. Special-casing shared infrastructure to satisfy a stale test is the wrong altitude for the fix.

General tests (server 3/4) was also still failing here.