fix: gateway agent method bypasses session daily/idle reset#47839
fix: gateway agent method bypasses session daily/idle reset#47839avaclaw1 wants to merge 1 commit into
Conversation
The gateway agent RPC handler pre-resolved the sessionId from the store
entry without checking session freshness (daily/idle reset policy). This
sessionId was then passed to resolveSession() which skips its own
freshness check when opts.sessionId is explicitly provided.
As a result, sessions targeted by cron announce delivery or any
callGateway({ method: "agent" }) call with a sessionKey would never
reset via the daily or idle reset mechanism. The cron delivery also
refreshes updatedAt on each run, preventing subsequent freshness checks
from detecting staleness.
The fix evaluates session freshness using the same reset policy pipeline
before deciding whether to reuse the existing sessionId or mint a new
one via randomUUID().
Greptile SummaryThis PR fixes a real session-reset bug in the gateway The fix correctly mirrors the freshness-evaluation pipeline already used by
Confidence Score: 3/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/server-methods/agent.ts
Line: 363-380
Comment:
**`systemSent` not reset on session rollover**
When `fresh` is `false` and a new `sessionId` is minted, `systemSent` is still carried over from the stale entry via `nextEntryPatch` (line ~406: `systemSent: entry?.systemSent`). If the old session had `systemSent: true`, the new session (with an empty conversation history identified by the new UUID) will skip system prompt injection — leaving the agent without its persona and instructions for the first turn.
Both reference implementations handle this explicitly:
`resolveCronSession()` (src/cron/isolated-agent/session.ts:50-54):
```typescript
} else {
// Session expired, create new
sessionId = crypto.randomUUID();
isNewSession = true;
systemSent = false; // ← reset here
}
```
`resolveSession()` (src/commands/agent/session.ts:146):
```typescript
const isNewSession = !fresh && !opts.sessionId;
```
…and `persistedThinking`/`persistedVerbose` are also only forwarded when `fresh === true`.
The fix should reset `systemSent` (and, arguably, `skillsSnapshot`) when the freshness check decides to rotate the session:
```typescript
sessionId = fresh ? entry.sessionId : randomUUID();
const isRolledOver = !fresh;
```
Then in `nextEntryPatch`:
```typescript
systemSent: isRolledOver ? undefined : entry?.systemSent,
skillsSnapshot: isRolledOver ? undefined : entry?.skillsSnapshot,
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/gateway/server-methods/agent.ts
Line: 376-380
Comment:
**Bootstrap cache not evicted on rollover**
Both `resolveCronSession()` and `resolveSession()` call `clearBootstrapSnapshotOnSessionRollover` whenever a session transitions to a new ID, ensuring the workspace bootstrap file cache is evicted. This code path skips that step entirely.
By the time `agentCommandFromIngress` runs, the store already has `updatedAt: now` with the new UUID, so `resolveSession()` will see `fresh = true` and `isNewSession = false` — meaning its own `clearBootstrapSnapshotOnSessionRollover` call is also a no-op (guarded by the `isNewSession` flag). The eviction is therefore never performed.
The fix should import `clearBootstrapSnapshotOnSessionRollover` from `../../agents/bootstrap-cache.js` and call it right after the rollover is detected (when `!fresh`), passing `canonicalKey` as the session key and the old `entry.sessionId` as the previous session ID.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 7db18a8 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let sessionId: string; | ||
| if (entry?.sessionId && entry.updatedAt != null) { | ||
| const sessionCfg = cfg.session; | ||
| const resetType = resolveSessionResetType({ sessionKey: canonicalKey }); | ||
| const resetOverride = resolveChannelResetConfig({ | ||
| sessionCfg, | ||
| channel: entry.lastChannel ?? entry.channel, | ||
| }); | ||
| const policy = resolveSessionResetPolicy({ sessionCfg, resetType, resetOverride }); | ||
| const { fresh } = evaluateSessionFreshness({ | ||
| updatedAt: entry.updatedAt, | ||
| now, | ||
| policy, | ||
| }); | ||
| sessionId = fresh ? entry.sessionId : randomUUID(); | ||
| } else { | ||
| sessionId = entry?.sessionId ?? randomUUID(); | ||
| } |
There was a problem hiding this comment.
systemSent not reset on session rollover
When fresh is false and a new sessionId is minted, systemSent is still carried over from the stale entry via nextEntryPatch (line ~406: systemSent: entry?.systemSent). If the old session had systemSent: true, the new session (with an empty conversation history identified by the new UUID) will skip system prompt injection — leaving the agent without its persona and instructions for the first turn.
Both reference implementations handle this explicitly:
resolveCronSession() (src/cron/isolated-agent/session.ts:50-54):
} else {
// Session expired, create new
sessionId = crypto.randomUUID();
isNewSession = true;
systemSent = false; // ← reset here
}resolveSession() (src/commands/agent/session.ts:146):
const isNewSession = !fresh && !opts.sessionId;…and persistedThinking/persistedVerbose are also only forwarded when fresh === true.
The fix should reset systemSent (and, arguably, skillsSnapshot) when the freshness check decides to rotate the session:
sessionId = fresh ? entry.sessionId : randomUUID();
const isRolledOver = !fresh;Then in nextEntryPatch:
systemSent: isRolledOver ? undefined : entry?.systemSent,
skillsSnapshot: isRolledOver ? undefined : entry?.skillsSnapshot,Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-methods/agent.ts
Line: 363-380
Comment:
**`systemSent` not reset on session rollover**
When `fresh` is `false` and a new `sessionId` is minted, `systemSent` is still carried over from the stale entry via `nextEntryPatch` (line ~406: `systemSent: entry?.systemSent`). If the old session had `systemSent: true`, the new session (with an empty conversation history identified by the new UUID) will skip system prompt injection — leaving the agent without its persona and instructions for the first turn.
Both reference implementations handle this explicitly:
`resolveCronSession()` (src/cron/isolated-agent/session.ts:50-54):
```typescript
} else {
// Session expired, create new
sessionId = crypto.randomUUID();
isNewSession = true;
systemSent = false; // ← reset here
}
```
`resolveSession()` (src/commands/agent/session.ts:146):
```typescript
const isNewSession = !fresh && !opts.sessionId;
```
…and `persistedThinking`/`persistedVerbose` are also only forwarded when `fresh === true`.
The fix should reset `systemSent` (and, arguably, `skillsSnapshot`) when the freshness check decides to rotate the session:
```typescript
sessionId = fresh ? entry.sessionId : randomUUID();
const isRolledOver = !fresh;
```
Then in `nextEntryPatch`:
```typescript
systemSent: isRolledOver ? undefined : entry?.systemSent,
skillsSnapshot: isRolledOver ? undefined : entry?.skillsSnapshot,
```
How can I resolve this? If you propose a fix, please make it concise.| }); | ||
| sessionId = fresh ? entry.sessionId : randomUUID(); | ||
| } else { | ||
| sessionId = entry?.sessionId ?? randomUUID(); | ||
| } |
There was a problem hiding this comment.
Bootstrap cache not evicted on rollover
Both resolveCronSession() and resolveSession() call clearBootstrapSnapshotOnSessionRollover whenever a session transitions to a new ID, ensuring the workspace bootstrap file cache is evicted. This code path skips that step entirely.
By the time agentCommandFromIngress runs, the store already has updatedAt: now with the new UUID, so resolveSession() will see fresh = true and isNewSession = false — meaning its own clearBootstrapSnapshotOnSessionRollover call is also a no-op (guarded by the isNewSession flag). The eviction is therefore never performed.
The fix should import clearBootstrapSnapshotOnSessionRollover from ../../agents/bootstrap-cache.js and call it right after the rollover is detected (when !fresh), passing canonicalKey as the session key and the old entry.sessionId as the previous session ID.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-methods/agent.ts
Line: 376-380
Comment:
**Bootstrap cache not evicted on rollover**
Both `resolveCronSession()` and `resolveSession()` call `clearBootstrapSnapshotOnSessionRollover` whenever a session transitions to a new ID, ensuring the workspace bootstrap file cache is evicted. This code path skips that step entirely.
By the time `agentCommandFromIngress` runs, the store already has `updatedAt: now` with the new UUID, so `resolveSession()` will see `fresh = true` and `isNewSession = false` — meaning its own `clearBootstrapSnapshotOnSessionRollover` call is also a no-op (guarded by the `isNewSession` flag). The eviction is therefore never performed.
The fix should import `clearBootstrapSnapshotOnSessionRollover` from `../../agents/bootstrap-cache.js` and call it right after the rollover is detected (when `!fresh`), passing `canonicalKey` as the session key and the old `entry.sessionId` as the previous session ID.
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7db18a8a28
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| now, | ||
| policy, | ||
| }); | ||
| sessionId = fresh ? entry.sessionId : randomUUID(); |
There was a problem hiding this comment.
When this branch marks a stale entry and assigns randomUUID(), the rest of the handler still merges into the existing session entry without clearing sessionFile, so the next run keeps using the prior transcript path instead of starting a fresh transcript. In the stale-session case (the exact path this patch introduces), resolveSessionTranscriptFile will keep reusing the old file from the persisted entry, so daily/idle rollover can still carry forward old history and continue growing the same transcript rather than performing a true reset.
Useful? React with 👍 / 👎.
steipete
added
duplicate
|
Thanks for tracing the gateway-agent bypass. I’m closing this as superseded by #71845. What changed in the replacement:
This PR was a real bug report/fix direction, but the replacement is broader and includes the missing lifecycle model needed for the rest of the duplicate cluster. Closing in favor of #71845. |
2 participants
Summary
The gateway
agentRPC method handler insrc/gateway/server-methods/agent.tspre-resolves thesessionIdfrom the session store entry without evaluating session freshness. ThissessionIdis then passed explicitly toresolveSession()insrc/commands/agent/session.ts, which skips its own freshness check whenopts.sessionIdis provided.Impact: Sessions that receive periodic
callGateway({ method: "agent" })calls — most commonly via cron announce delivery — will never reset through the daily or idle timeout mechanism. The cron delivery also refreshesupdatedAton each invocation, preventing any subsequent freshness check from detecting staleness.Root Cause
In
src/gateway/server-methods/agent.ts(line 354):This unconditionally reuses the stored session ID. Later,
resolvedSessionId = sessionIdpasses it toagentCommandFromIngress, whereresolveSession()has:Since
opts.sessionIdis already set, thefreshevaluation is bypassed entirely.Fix
Evaluate session freshness using the existing reset policy pipeline (
resolveSessionResetType->resolveChannelResetConfig->resolveSessionResetPolicy->evaluateSessionFreshness) before deciding whether to reuse the existingsessionIdor mint a new UUID.Reproduction
sessionTarget: "isolated"delivering via announce to a Telegram groupreset.mode: "daily",atHour: 4sessionIdindefinitely -- daily reset never triggersTesting
resolveSession()resolveDailyResetAtMscorrectly computes 4 AM local time, andevaluateSessionFreshnesscorrectly returnsfresh: falsefor sessions with overnight gaps crossing the reset hourresolveSession()anddispatchInboundMessage()AI-assisted (Claude Code)