Fix timer heartbeats resuming sessions without restoring project workspace#1907
Fix timer heartbeats resuming sessions without restoring project workspace#1907konbluesky wants to merge 2 commits intopaperclipai:masterfrom
Conversation
Greptile SummaryThis PR fixes timer heartbeat runs that could resume an OpenCode session while using the agent-home fallback directory as the working directory — causing Confidence Score: 4/5Safe to merge for the timer-heartbeat path, but the One P1 finding: server/src/services/heartbeat.ts — specifically the Important Files Changed
Prompt To Fix All With AIThis is a comment left during a code review.
Path: server/src/services/heartbeat.ts
Line: 2166-2170
Comment:
**Runtime snapshot used even when session reset is requested**
`runtimeSessionSnapshot.params` is now the last fallback in the `previousSessionParams` chain. This means it is also evaluated for wake events that intentionally request a fresh session — such as `wakeReason: "issue_assigned"`, where `shouldResetTaskSessionForWake` returns `true`.
When a new issue is assigned, `taskSessionForRun` is explicitly set to `null` (line 2152) to prevent session reuse. But because `runtimeSessionSnapshot.params` can be non-null (carrying the session ID and workspace from the previous run), it bypasses that guard and populates `previousSessionParams` with the stale session. This in turn populates `previousSessionDisplayId` and `runtimeSessionParamsForAdapter`, causing the adapter to resume the old session — which is the opposite of the intended "fresh session" behavior.
Note that `runtimeSessionFallback` on line 2435 was deliberately set to `null` when `resetTaskSession` is true for exactly this reason.
The fix is to gate the fallback on `!resetTaskSession`:
```suggestion
const previousSessionParams =
explicitResumeSessionParams ??
(explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ??
normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)) ??
(!resetTaskSession ? runtimeSessionSnapshot.params : null);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: server/src/services/heartbeat.ts
Line: 2153-2157
Comment:
**Unconditional DB queries even when snapshot won't be used**
`resolveRuntimeStateSessionSnapshotForRun` always runs up to two extra DB queries — one against `agentTaskSessions` and one via `getLatestRunForSession` — regardless of whether the result will actually be needed. Since `runtimeSessionSnapshot.params` is the last item in the `??` chain, it is skipped whenever any earlier source (`explicitResumeSessionParams`, `explicitResumeSessionDisplayId`, or `taskSessionForRun`) is non-null.
Consider moving the call inside the `??` chain or lazily resolving it only when the earlier sources are all null, to avoid the overhead on the common (task-session-present) path.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix heartbeat timer session workspace re..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
server/src/services/heartbeat.ts
Outdated
| const previousSessionParams = | ||
| explicitResumeSessionParams ?? | ||
| (explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ?? | ||
| normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)); | ||
| normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)) ?? | ||
| runtimeSessionSnapshot.params; |
There was a problem hiding this comment.
runtimeSessionSnapshot.params is now the last fallback in the previousSessionParams chain. This means it is also evaluated for wake events that intentionally request a fresh session — such as wakeReason: "issue_assigned", where shouldResetTaskSessionForWake returns true.
When a new issue is assigned, taskSessionForRun is explicitly set to null (line 2152) to prevent session reuse. But because runtimeSessionSnapshot.params can be non-null (carrying the session ID and workspace from the previous run), it bypasses that guard and populates previousSessionParams with the stale session. This in turn populates previousSessionDisplayId and runtimeSessionParamsForAdapter, causing the adapter to resume the old session — which is the opposite of the intended "fresh session" behavior.
Note that runtimeSessionFallback on line 2435 was deliberately set to null when resetTaskSession is true for exactly this reason.
The fix is to gate the fallback on !resetTaskSession:
| const previousSessionParams = | |
| explicitResumeSessionParams ?? | |
| (explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ?? | |
| normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)); | |
| normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)) ?? | |
| runtimeSessionSnapshot.params; | |
| const previousSessionParams = | |
| explicitResumeSessionParams ?? | |
| (explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ?? | |
| normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)) ?? | |
| (!resetTaskSession ? runtimeSessionSnapshot.params : null); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/services/heartbeat.ts
Line: 2166-2170
Comment:
**Runtime snapshot used even when session reset is requested**
`runtimeSessionSnapshot.params` is now the last fallback in the `previousSessionParams` chain. This means it is also evaluated for wake events that intentionally request a fresh session — such as `wakeReason: "issue_assigned"`, where `shouldResetTaskSessionForWake` returns `true`.
When a new issue is assigned, `taskSessionForRun` is explicitly set to `null` (line 2152) to prevent session reuse. But because `runtimeSessionSnapshot.params` can be non-null (carrying the session ID and workspace from the previous run), it bypasses that guard and populates `previousSessionParams` with the stale session. This in turn populates `previousSessionDisplayId` and `runtimeSessionParamsForAdapter`, causing the adapter to resume the old session — which is the opposite of the intended "fresh session" behavior.
Note that `runtimeSessionFallback` on line 2435 was deliberately set to `null` when `resetTaskSession` is true for exactly this reason.
The fix is to gate the fallback on `!resetTaskSession`:
```suggestion
const previousSessionParams =
explicitResumeSessionParams ??
(explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ??
normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)) ??
(!resetTaskSession ? runtimeSessionSnapshot.params : null);
```
How can I resolve this? If you propose a fix, please make it concise.
server/src/services/heartbeat.ts
Outdated
| const runtimeSessionSnapshot = await resolveRuntimeStateSessionSnapshotForRun({ | ||
| agent, | ||
| runtime, | ||
| codec: sessionCodec, | ||
| }); |
There was a problem hiding this comment.
resolveRuntimeStateSessionSnapshotForRun always runs up to two extra DB queries — one against agentTaskSessions and one via getLatestRunForSession — regardless of whether the result will actually be needed. Since runtimeSessionSnapshot.params is the last item in the ?? chain, it is skipped whenever any earlier source (explicitResumeSessionParams, explicitResumeSessionDisplayId, or taskSessionForRun) is non-null.
Consider moving the call inside the ?? chain or lazily resolving it only when the earlier sources are all null, to avoid the overhead on the common (task-session-present) path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/services/heartbeat.ts
Line: 2153-2157
Comment:
**Unconditional DB queries even when snapshot won't be used**
`resolveRuntimeStateSessionSnapshotForRun` always runs up to two extra DB queries — one against `agentTaskSessions` and one via `getLatestRunForSession` — regardless of whether the result will actually be needed. Since `runtimeSessionSnapshot.params` is the last item in the `??` chain, it is skipped whenever any earlier source (`explicitResumeSessionParams`, `explicitResumeSessionDisplayId`, or `taskSessionForRun`) is non-null.
Consider moving the call inside the `??` chain or lazily resolving it only when the earlier sources are all null, to avoid the overhead on the common (task-session-present) path.
How can I resolve this? If you propose a fix, please make it concise.
|
Thanks — I followed up on both points.\n\nWhat changed in the latest push:\n- gated the runtime-state fallback so it is not used when is true, preserving the intended fresh-session behavior for wakes\n- made runtime-state session recovery lazy by resolving it only when explicit/task-session sources are absent\n\nI also kept the server-side validation loop running against the Paperclip deployment that originally reproduced the bug. The timer/on-demand no-task path now restores the managed project workspace instead of resuming a project session inside . |
|
Thanks — I pushed a follow-up fix for both review points. Changes in the latest commit:
I also revalidated the fix against the Paperclip deployment that originally reproduced the bug. After rebuilding the server with the patch, a no-task wakeup was able to restore the managed project workspace instead of resuming a project session inside |
2 participants
Thinking path
Paperclip's heartbeat model is supposed to let agents continue useful work across short wake/sleep cycles. For code-focused agents, that only works if session reuse and workspace reuse stay aligned. In the reported failure mode, timer heartbeats correctly reused an OpenCode session but incorrectly fell back to
agent_homeas the current workspace. That split caused OpenCode to treat the managed project checkout asexternal_directory, so even basic repo commands were denied. The fix therefore targets the control-plane workspace/session handoff itself: persist enough session metadata to reconstruct the original project workspace, and only apply that recovery when a fresh wake is not explicitly required.Summary
agent_homefallbackagent_hometimer runs, which currently triggersexternal_directorypermission failuresProblem
Timer heartbeats can reuse an existing OpenCode session while falling back to
agent_homeas the current workspace. When the reused session was created in a managed project workspace, OpenCode treats the project directory asexternal_directoryrelative to the fallback workspace and rejects basic commands likegit status.Fix
This patch teaches heartbeat runtime recovery to carry forward workspace metadata with the resumed session and to recover project workspace context from runtime/task session history when task context is missing.
Validation
paperclipWorkspace.cwdto the managed project workspace instead ofagent_homeglobandreadfiles inside the managed checkout without triggeringexternal_directoryNotes
pnpm test/pnpm -r typecheckwere not runnable before dependency installation in the local clone