Synchronous I/O blocks the event loop

readTranscriptTailMessages uses blocking fs.statSync, fs.openSync, fs.readSync, and fs.closeSync calls inside an otherwise-async function. The existing code in this same file (readLastNonzeroUsageFromSessionLog) uses fs.promises.open and async reads throughout — this new function is inconsistent with that established pattern.

While 64KB reads are typically fast on local SSDs (~sub-ms), using sync I/O in an async context blocks the entire Node.js event loop for the duration of the syscalls. On NFS mounts or slow disks this can become noticeable. Consider using the async API to match the rest of the file:

async function readTranscriptTailMessages(
  filePath: string,
  maxMessages: number,
): Promise<Array<{ role?: string; content?: unknown }>> {
  const TAIL_BYTES = 64 * 1024;
  const handle = await fs.promises.open(filePath, "r");
  try {
    const stat = await handle.stat();
    const start = Math.max(0, stat.size - TAIL_BYTES);
    const buf = Buffer.alloc(Math.min(stat.size, TAIL_BYTES));
    await handle.read(buf, 0, buf.length, start);
    // ... rest of parsing logic unchanged
  } finally {
    await handle.close();
  }
}

The call-site in runMemoryFlushIfNeeded would then need to await the result. Similarly, resolveSessionFilePathForFlush uses fs.existsSync, which should be await fs.promises.access(...) to remain async-consistent.

This is a comment left during a code review.
Path: src/auto-reply/reply/agent-runner-memory.ts
Line: 626-661

Comment:
**Synchronous I/O blocks the event loop**

`readTranscriptTailMessages` uses blocking `fs.statSync`, `fs.openSync`, `fs.readSync`, and `fs.closeSync` calls inside an otherwise-async function. The existing code in this same file (`readLastNonzeroUsageFromSessionLog`) uses `fs.promises.open` and async reads throughout — this new function is inconsistent with that established pattern.

While 64KB reads are typically fast on local SSDs (~sub-ms), using sync I/O in an async context blocks the entire Node.js event loop for the duration of the syscalls. On NFS mounts or slow disks this can become noticeable. Consider using the async API to match the rest of the file:

```ts
async function readTranscriptTailMessages(
  filePath: string,
  maxMessages: number,
): Promise<Array<{ role?: string; content?: unknown }>> {
  const TAIL_BYTES = 64 * 1024;
  const handle = await fs.promises.open(filePath, "r");
  try {
    const stat = await handle.stat();
    const start = Math.max(0, stat.size - TAIL_BYTES);
    const buf = Buffer.alloc(Math.min(stat.size, TAIL_BYTES));
    await handle.read(buf, 0, buf.length, start);
    // ... rest of parsing logic unchanged
  } finally {
    await handle.close();
  }
}
```

The call-site in `runMemoryFlushIfNeeded` would then need to `await` the result. Similarly, `resolveSessionFilePathForFlush` uses `fs.existsSync`, which should be `await fs.promises.access(...)` to remain async-consistent.

How can I resolve this? If you propose a fix, please make it concise.

resolveSessionFilePathForFlush misses the transcriptPath field

The existing resolveSessionLogPath helper (lines 102–132 in this same file) explicitly handles a transcriptPath field on SessionEntry via a TypeScript cast:

const transcriptPath = (
  sessionEntry as (SessionEntry & { transcriptPath?: string }) | undefined
)?.transcriptPath?.trim();
const sessionFile = sessionEntry?.sessionFile?.trim() || transcriptPath;

resolveSessionFilePathForFlush only checks entry?.sessionFile and falls through to resolveSessionFilePath(sessionId, entry, ...). If a session's transcript lives at transcriptPath but not at sessionFile, this function returns undefined, silently skipping the hash check entirely. The flush then proceeds unconditionally, defeating the dedup for those sessions.

Since resolveSessionLogPath already has this logic, the simplest fix is to mirror it here — prefer sessionFile ?? transcriptPath before falling through to the path-resolver.

This is a comment left during a code review.
Path: src/auto-reply/reply/agent-runner-memory.ts
Line: 595-620

Comment:
**`resolveSessionFilePathForFlush` misses the `transcriptPath` field**

The existing `resolveSessionLogPath` helper (lines 102–132 in this same file) explicitly handles a `transcriptPath` field on `SessionEntry` via a TypeScript cast:

```ts
const transcriptPath = (
  sessionEntry as (SessionEntry & { transcriptPath?: string }) | undefined
)?.transcriptPath?.trim();
const sessionFile = sessionEntry?.sessionFile?.trim() || transcriptPath;
```

`resolveSessionFilePathForFlush` only checks `entry?.sessionFile` and falls through to `resolveSessionFilePath(sessionId, entry, ...)`. If a session's transcript lives at `transcriptPath` but not at `sessionFile`, this function returns `undefined`, silently skipping the hash check entirely. The flush then proceeds unconditionally, defeating the dedup for those sessions.

Since `resolveSessionLogPath` already has this logic, the simplest fix is to mirror it here — prefer `sessionFile ?? transcriptPath` before falling through to the path-resolver.

How can I resolve this? If you propose a fix, please make it concise.

Misleading test name

The test is named "ignores system and tool messages", which implies the function produces the same hash regardless of whether system messages are present. However, the assertion is not.toBe (different hashes), and the inline comment explains this is because messages.length changes.

What the function actually does is: system/tool messages are excluded from the content tail (the 3-message slice) but still counted in messages.length. The hashes differ because of the length difference, not because system messages are truly "ignored."

A more accurate name would be something like "system messages affect length but not content tail" or "excludes system messages from content tail". As-is, a future developer reading the test name and the not.toBe assertion may find it contradictory.

This is a comment left during a code review.
Path: src/auto-reply/reply/memory-flush.test.ts
Line: 66-74

Comment:
**Misleading test name**

The test is named `"ignores system and tool messages"`, which implies the function produces the same hash regardless of whether system messages are present. However, the assertion is `not.toBe` (different hashes), and the inline comment explains this is because `messages.length` changes.

What the function actually does is: system/tool messages are excluded from the *content tail* (the 3-message slice) but still counted in `messages.length`. The hashes differ because of the length difference, not because system messages are truly "ignored."

A more accurate name would be something like `"system messages affect length but not content tail"` or `"excludes system messages from content tail"`. As-is, a future developer reading the test name and the `not.toBe` assertion may find it contradictory.

How can I resolve this? If you propose a fix, please make it concise.

Guard missing newline when trimming partial tail chunk

When the last 64KB read starts in the middle of a very large JSONL record, tail.indexOf("\n") returns -1, so this expression keeps the truncated fragment instead of dropping it; parsing then yields no messages and computeContextHash([]) becomes a stable value for different contexts. In sessions with large single-line messages (or a last message larger than 64KB), this can make the dedup gate think context is unchanged and skip later memory flushes that should run.

Useful? React with 👍 / 👎.

Serialize hashed tail content without delimiter collisions

Joining message contents with raw newlines makes different tail message sequences hash to the same payload (for example, ['a\nb','c'] and ['a','b\nc'] both become a\nb\nc), so the dedup check can treat changed context as unchanged and suppress a needed memory flush. Because message content frequently contains newlines, this is a practical false-dedup case rather than a theoretical SHA collision.

Useful? React with 👍 / 👎.

Include role/turn identity in flush context hash

The dedup hash is built from messages.length plus only the last three user/assistant contents, so once the tail reader has reached its fixed-size window, distinct transcript states can still hash the same whenever those last contents repeat (for example repeated ok exchanges, or the same text appearing under different roles). In runMemoryFlushIfNeeded, that false match causes the flush to be skipped even though conversation state has changed, which can suppress needed memory flushes near compaction. Hashing role and a stronger turn discriminator (instead of content-only tail) would avoid these false-dedup skips.

Useful? React with 👍 / 👎.

Normalize sessionFile before reading transcript tail

The new fast path returns entry.sessionFile as soon as fs.promises.access(sessionFile) succeeds, which skips the normalization/compatibility logic in resolveSessionFilePath. If a session entry still carries a stale absolute transcript path (a case the resolver explicitly handles), the dedup hash is computed from the wrong file and runMemoryFlushIfNeeded can incorrectly skip or rerun flushes based on unrelated transcript content.

Useful? React with 👍 / 👎.

Pass agent-aware options when resolving flush transcript path

This resolver call omits agentId, unlike resolveSessionLogPath, so when storePath is unavailable (or (multiple)) and sessionFile is relative, it resolves against the default agent sessions directory. For non-default agent sessions that means the hash check reads the wrong path (or none), so memoryFlushContextHash dedup is silently disabled and duplicate retry flushes can reappear.

Useful? React with 👍 / 👎.

Clear stale context hash across session resets

This equality check trusts entry.memoryFlushContextHash even when the session key has just been rotated to a new transcript, and the reset path currently preserves unknown fields by merging store[sessionKey] into the new entry while only clearing memoryFlushCompactionCount/memoryFlushAt (see src/auto-reply/reply/session.ts). That lets an old session’s hash survive into a new sessionId, so if the new conversation reaches flush with the same tail hash, this branch skips the first flush and suppresses a needed MEMORY.md write for the new session. Please reset/hash-scope this field on session rollover before comparing here.

Useful? React with 👍 / 👎.

Drop raw sessionFile fallback when normalized path is missing

After resolveSessionFilePath(...) fails the existence check, this fallback returns sessionFile directly if that raw path exists. In sessions that still carry a stale absolute transcript path (for example after store/root migration), the hash gate can read the wrong transcript and then skip or rerun memory flush based on unrelated content. This reintroduces the stale-path problem the normalization step is meant to prevent.

Useful? React with 👍 / 👎.

Handle >64KB single-line tails in transcript hash reader

When the tail read starts in the middle of a large JSONL record, this branch trims the chunk to "" (either nlIdx === -1 or newline only at the end), so readTranscriptTailMessages() returns no messages. That makes computeContextHash([]) a stable value for different large-message contexts, so the dedup gate can incorrectly treat changed context as unchanged and skip a needed flush.

Useful? React with 👍 / 👎.

Treat successful willRetry compactions as completed

This gate drops every compaction:end event that has willRetry=true, but the compaction subscriber explicitly treats willRetry=true as a successful compaction when result is present (see src/agents/pi-embedded-subscribe.handlers.compaction.ts, handleAutoCompactionEnd). In overflow-retry runs, this leaves memoryCompactionCompleted false, so incrementCompactionCount is skipped and flush metadata is written with a stale compaction counter, which can mis-drive subsequent memory-flush dedup decisions.

Useful? React with 👍 / 👎.

Pass an actual agentId into flush transcript resolution

This call passes params.agentId, but runMemoryFlushIfNeeded's params shape and its caller (src/auto-reply/reply/agent-runner.ts) do not provide agentId, so this value is always undefined. In non-default-agent sessions when storePath is unavailable or (multiple) and sessionFile is relative, path resolution falls back to the default agent sessions directory, so the dedup hash may be computed from the wrong transcript and incorrectly skip or repeat a flush.

Useful? React with 👍 / 👎.

Persist post-flush context hash for dedup checks

memoryFlushContextHash is written from contextHashBeforeFlush, but the flush itself is executed immediately above via runEmbeddedPiAgent(...) and appends a new turn to the same session transcript (see activeSession.prompt(...) in src/agents/pi-embedded-runner/run/attempt.ts). That means the next dedup check hashes the post-flush transcript while comparing against a pre-flush hash, so the equality branch will not match in retry scenarios and duplicate MEMORY.md flushes can still run.

Useful? React with 👍 / 👎.

Keep resolved transcript path even before file creation

This access() gate drops the resolved transcript path whenever the file does not yet exist, but runMemoryFlushIfNeeded() reuses sessionFilePath to compute and persist the post-flush context hash. In reset/new-session flows, the transcript file is typically created by the flush run itself, so this early undefined prevents writing memoryFlushContextHash on that first flush; if the same context is retried afterward, dedup has no stored hash and can run duplicate flushes again. Returning the normalized path even when it is not yet on disk (or re-resolving after flush) avoids that gap.

Useful? React with 👍 / 👎.

Clear stale flush hash on all session-reset paths

This dedup branch still trusts entry.memoryFlushContextHash, but the new field is only cleared in initSessionState; runReplyAgent's resetSession path in src/auto-reply/reply/agent-runner.ts rotates sessionId via { ...prevEntry, ... } and does not unset the hash. In compaction-failure/role-ordering resets, a new session can inherit an old hash and this check may skip the first needed memory flush when tails coincide. Fresh evidence in this commit is the new persisted hash gate here, which now depends on every reset path clearing that field.

Useful? React with 👍 / 👎.

Clear flush hash on every session-id rotation

memoryFlushContextHash is only reset in initSessionState, but the runReplyAgent reset path (resetSession in src/auto-reply/reply/agent-runner.ts) rotates to a new sessionId by spreading prevEntry, which preserves this field. Because runMemoryFlushIfNeeded now skips when the stored hash matches, a compaction-failure/role-ordering reset can inherit an old hash and incorrectly skip the first MEMORY.md flush in the new session when early turns are similar; please clear this field in all session-reset paths, not just init.

Useful? React with 👍 / 👎.

Only suppress fallback after a confirmed memory write

If the first memory-flush candidate fails before touching memory/YYYY-MM-DD.md (for example on a transient provider error inside runEmbeddedPiAgent()), this line still marks the hash as already attempted. The guard just above then makes every later fallback candidate return a synthetic success, which runWithModelFallback() accepts as a real success in src/agents/model-fallback.ts:661-690. runMemoryFlushIfNeeded() then persists memoryFlushAt and memoryFlushContextHash even though no flush actually happened, so a recoverable first-candidate failure can suppress the only pre-compaction memory flush for that cycle.

Useful? React with 👍 / 👎.

Persist the same prompt-inclusive hash you compare later

The pre-flush dedup path now appends followupRun.prompt before hashing (src/auto-reply/reply/agent-runner-memory.ts:469-472), but the value persisted here is recomputed from the post-flush transcript alone. On any replay/retry of the same inbound message after a successful flush, the next pre-check hashes transcript + prompt while memoryFlushContextHash still represents just transcript, so the equality check never matches and the same message can append another MEMORY.md entry. This is reachable because activeSession.prompt(...) writes the user prompt into the session before the model call in src/agents/pi-embedded-runner/run/attempt.ts:2841-2843.

Useful? React with 👍 / 👎.

Suppress fallback retries even when tail hashing yields no hash

This guard only suppresses later fallback candidates when contextHashBeforeFlush is truthy. If the transcript currently has no message entries or tail extraction fails/returns empty, the first candidate can still write to memory/YYYY-MM-DD.md and then throw, but every later fallback candidate will rerun the flush because fallbackFlushAttemptedForCurrentHash was never armed. That leaves duplicate memory writes in exactly the empty/unreadable-tail cases this PR is trying to fix.

Useful? React with 👍 / 👎.

Honor bytesRead when decoding the tail chunk

If FileHandle.read() returns fewer than readLen bytes here, buf.toString() will serialize the zero-filled remainder of Buffer.alloc(readLen) as \u0000 characters. That makes the trailing JSONL lines unparsable, so readTranscriptTailMessages() can return too few messages and the new memoryFlushContextHash dedup either disables itself or hashes the wrong context on short reads (for example on interrupted/networked filesystems).

Useful? React with 👍 / 👎.