fix: end Codex sessions on Stop hook

[Rex57]

Summary

• Wire Codex Stop to run the existing session-end.mjs hook after stop.mjs.
• Preserve the current best-effort summarize behavior while closing the session lifecycle for Codex, which does not use a separate SessionEnd entry in hooks.codex.json.
• Add regression coverage that the Codex Stop hook includes both summarize and session-end commands.

Fixes #493.
Related: #308.

Verification

• npx tsdown passed in isolated fork clone.
• node -e "JSON.parse(require('fs').readFileSync('plugin/hooks/hooks.codex.json','utf8')); console.log('hooks.codex.json OK')" passed.
• Tests not run locally per local environment constraint; full npm test intentionally skipped to avoid any accidental interaction with the live AgentMemory setup.

Summary by CodeRabbit

• Chores

  • Enhanced session cleanup process configuration.

• Tests

  • Added verification tests for session cleanup functionality.

[vercel]

@Rex57 is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

The Stop hook in plugin/hooks/hooks.codex.json now invokes session-end.mjs after stop.mjs to mark sessions as ended. A test case verifies both commands are present in the Stop hook handler.

Changes

Stop Hook Session Lifecycle

Layer / File(s)	Summary
Stop hook with session-end command and verification plugin/hooks/hooks.codex.json, test/codex-plugin.test.ts	The Stop hook configuration adds a session-end.mjs command after stop.mjs, and a test verifies both stop.mjs and session-end.mjs commands are triggered when the Stop event fires.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A stop that stops is good, they say,
But sessions linger, come what may!
Now end runs after, sure and true—
The viewer knows the session's through! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: configuring the Codex Stop hook to invoke session-end behavior, which matches the primary objective of the PR.
Linked Issues check	✅ Passed	The PR implements the root-cause fix from issue #493 by wiring session-end.mjs to the Codex Stop hook and adds test coverage for this behavior, fulfilling all coding requirements.
Out of Scope Changes check	✅ Passed	All changes directly address the Stop hook lifecycle issue and test coverage; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/codex-plugin.test.ts`:
- Around line 101-105: The test currently only checks that both stop scripts
exist in stopCommands (from hooks.hooks.Stop) but not their order; update the
assertions to verify sequence explicitly by locating the two command strings
(from stopCommands) and asserting that "node
${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs" appears before "node
${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs" (e.g., compare their indices or
assert the slice/sequence matches the expected order) so regressions that invert
these handlers fail the test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5efbe79c-ac8f-47cc-9919-53c825dbede8

📥 Commits

Reviewing files that changed from the base of the PR and between 9061da5 and 5b9df38.

📒 Files selected for processing (2)

• plugin/hooks/hooks.codex.json
• test/codex-plugin.test.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert command order, not just presence.

Line 101 flattens commands and Lines 104-105 only check containment. This misses regressions where session-end.mjs runs before stop.mjs. Please assert sequence explicitly.

Suggested test tightening

-    expect(stopCommands).toContain("node ${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs");
-    expect(stopCommands).toContain("node ${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs");
+    const stopIdx = stopCommands.indexOf("node ${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs");
+    const endIdx = stopCommands.indexOf("node ${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs");
+    expect(stopIdx).toBeGreaterThanOrEqual(0);
+    expect(endIdx).toBeGreaterThanOrEqual(0);
+    expect(stopIdx).toBeLessThan(endIdx);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const stopCommands = hooks.hooks.Stop.flatMap((entry) =>
	entry.hooks.map((handler) => handler.command),
	);
	expect(stopCommands).toContain("node ${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs");
	expect(stopCommands).toContain("node ${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs");
	const stopCommands = hooks.hooks.Stop.flatMap((entry) =>
	entry.hooks.map((handler) => handler.command),
	);
	const stopIdx = stopCommands.indexOf("node ${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs");
	const endIdx = stopCommands.indexOf("node ${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs");
	expect(stopIdx).toBeGreaterThanOrEqual(0);
	expect(endIdx).toBeGreaterThanOrEqual(0);
	expect(stopIdx).toBeLessThan(endIdx);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/codex-plugin.test.ts` around lines 101 - 105, The test currently only
checks that both stop scripts exist in stopCommands (from hooks.hooks.Stop) but
not their order; update the assertions to verify sequence explicitly by locating
the two command strings (from stopCommands) and asserting that "node
${CLAUDE_PLUGIN_ROOT}/scripts/stop.mjs" appears before "node
${CLAUDE_PLUGIN_ROOT}/scripts/session-end.mjs" (e.g., compare their indices or
assert the slice/sequence matches the expected order) so regressions that invert
these handlers fail the test.

[rohitg00]

Merged + shipped in v0.9.19 — https://github.com/rohitg00/agentmemory/releases/tag/v0.9.19

Thanks @Rex57 for the surface-level fix + the regression test on the Stop chain. Closed both #493 and the related #308 thread.