cleanup: address greptile review nits for 5/5

- Remove redundant length guard on postHookActions drain loop
- Add per-action error isolation to drainPostHookActions test helper,
  matching actual triggerInternalHook behaviour
- Remove double-catch in session-memory post-hook callback; let errors
  propagate to the framework's per-action catch for consistent logging
This commit is contained in:
zeroaltitude 2026-03-07 10:31:08 -07:00
parent 51fdf17867
commit edcc079583
No known key found for this signature in database
GPG Key ID: 77592FB1C703882E
3 changed files with 49 additions and 48 deletions

View File

@ -573,12 +573,17 @@ describe("session-memory hook", () => {
expect(memoryContent).toContain("assistant: Only message 2");
});
// Helper to drain postHookActions (simulates what triggerInternalHook does)
// Helper to drain postHookActions with per-action error isolation,
// matching triggerInternalHook's actual drain behaviour.
async function drainPostHookActions(event: {
postHookActions: Array<() => Promise<void> | void>;
}) {
for (const action of event.postHookActions) {
await action();
try {
await action();
} catch {
// Per-action isolation — one failure doesn't block others.
}
}
}

View File

@ -389,48 +389,46 @@ const saveSessionToMemory: HookHandler = async (event) => {
// registered after this handler can set blockSessionSave or
// sessionSaveContent and still have them honored.
const writtenEntry = context.blockSessionSave === true ? null : entry;
// Post-hook callback — errors propagate to the framework's per-action
// catch in triggerInternalHook, which provides consistent log formatting
// and per-action isolation.
event.postHookActions.push(async () => {
try {
// If a later hook blocked the save, retract the file we just wrote.
if (event.context.blockSessionSave === true && writtenEntry !== null) {
try {
await fs.unlink(memoryFilePath);
log.debug("Session save retracted by post-hook (blockSessionSave)");
} catch (err) {
// File may not exist if inline write also didn't happen — that's fine.
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
throw err;
}
// If a later hook blocked the save, retract the file we just wrote.
if (event.context.blockSessionSave === true && writtenEntry !== null) {
try {
await fs.unlink(memoryFilePath);
log.debug("Session save retracted by post-hook (blockSessionSave)");
} catch (err) {
// File may not exist if inline write also didn't happen — that's fine.
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
throw err;
}
return;
}
return;
}
// If a later hook set sessionSaveContent, overwrite with new content.
// blockSessionSave takes precedence — never create/overwrite a file that
// was blocked, even if sessionSaveContent is also set.
const postContent = event.context.sessionSaveContent;
if (
event.context.blockSessionSave !== true &&
typeof postContent === "string" &&
postContent !== writtenEntry
) {
// Ensure memoryDir exists — the inline write may have been
// skipped (e.g. blockSessionSave was true initially) so mkdir
// might never have run.
await fs.mkdir(memoryDir, { recursive: true });
await writeFileWithinRoot({
rootDir: memoryDir,
relativePath: filename,
data: postContent,
encoding: "utf-8",
});
log.debug("Session save content replaced by post-hook (sessionSaveContent)", {
length: postContent.length,
});
}
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
log.error(`Post-hook session-memory action failed: ${message}`);
// If a later hook set sessionSaveContent, overwrite with new content.
// blockSessionSave takes precedence — never create/overwrite a file that
// was blocked, even if sessionSaveContent is also set.
const postContent = event.context.sessionSaveContent;
if (
event.context.blockSessionSave !== true &&
typeof postContent === "string" &&
postContent !== writtenEntry
) {
// Ensure memoryDir exists — the inline write may have been
// skipped (e.g. blockSessionSave was true initially) so mkdir
// might never have run.
await fs.mkdir(memoryDir, { recursive: true });
await writeFileWithinRoot({
rootDir: memoryDir,
relativePath: filename,
data: postContent,
encoding: "utf-8",
});
log.debug("Session save content replaced by post-hook (sessionSaveContent)", {
length: postContent.length,
});
}
});
} catch (err) {

View File

@ -292,14 +292,12 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise<voi
// a chance to mutate event.context, eliminating FIFO ordering issues.
// Actions execute in push order; errors are caught per-action so one
// failure doesn't block others.
if (event.postHookActions.length > 0) {
for (const action of event.postHookActions) {
try {
await action();
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`);
}
for (const action of event.postHookActions) {
try {
await action();
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`);
}
}
}