fix: split dual-purpose writtenEntry comparison, require onError in drain

- Split postContent !== writtenEntry into explicit (writtenEntry === null ||
  postContent !== writtenEntry) — makes the two intents unambiguous: write
  if no inline write happened OR if content changed
- Make onError required in drainPostHookActions — forces callers to be
  explicit about error handling (logger in production, rethrower in tests)
  instead of silently swallowing by default

Addresses greptile review: comparison clarity + silent error swallow.
This commit is contained in:
zeroaltitude 2026-03-09 22:52:13 -07:00
parent cca4fde3d2
commit b17165f982
No known key found for this signature in database
GPG Key ID: 77592FB1C703882E
2 changed files with 8 additions and 4 deletions

View File

@ -496,7 +496,9 @@ const saveSessionToMemory: HookHandler = async (event) => {
if (
event.context.blockSessionSave !== true &&
typeof postContent === "string" &&
postContent !== writtenEntry
// Two distinct intents: write if no inline write happened (writtenEntry
// is null because blockPreSet was true) OR if the content changed.
(writtenEntry === null || postContent !== writtenEntry)
) {
// Ensure memoryDir exists — the inline write may have been
// skipped (e.g. blockSessionSave was true initially) so mkdir

View File

@ -323,11 +323,13 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise<voi
* share the same drain semantics as production.
*
* @param actions - The postHookActions array to drain (mutated: cleared after snapshot).
* @param onError - Optional per-action error handler. Defaults to swallowing errors silently.
* @param onError - Per-action error handler. Required to force callers to be explicit
* about error handling pass a logger in production, a rethrower in tests, or
* `() => {}` only when intentionally swallowing errors.
*/
export async function drainPostHookActions(
actions: Array<() => Promise<void> | void>,
onError?: (err: unknown) => void,
onError: (err: unknown) => void,
): Promise<void> {
const pending = [...actions];
actions.length = 0;
@ -335,7 +337,7 @@ export async function drainPostHookActions(
try {
await action();
} catch (err) {
onError?.(err);
onError(err);
}
}
}