Skip to content

feat: add session_start and session_end hook events - #50

Merged
aakashrajput merged 2 commits into
KlaatAI:mainfrom
Ayush7614:feat/session-lifecycle-hooks
Jul 27, 2026
Merged

feat: add session_start and session_end hook events#50
aakashrajput merged 2 commits into
KlaatAI:mainfrom
Ayush7614:feat/session-lifecycle-hooks

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds session_start / session_end to the hooks lifecycle (closes Add session_start/session_end hook events #9).
  • session_start fires once after boot (before the first prompt); session_end fires on graceful quit (/exit, Ctrl+D, Ctrl+C).
  • /hooks help and README document the new events.

Test plan

  • bun run typecheck
  • bun test
  • bun run build
  • Configure session_start / session_end in .klaatai/hooks.json, start and quit the TUI, confirm both fire once

Fire session_start once after REPL boot and session_end on graceful
quit so users can notify/log around a whole session lifecycle.
@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
The diff fully addresses #9. It adds "session_start" and "session_end" to the HookEvent union, fires runHooks("session_start") after boot (before the first prompt) and runHooks("session_end") inside quit() (which covers /exit, Ctrl+D, Ctrl+C). The /hooks help text and README are updated with the new events. Exact match.

Test coverage
No tests were added or changed. The PR body mentions bun test but that exercises only existing tests. Missing coverage: verifying that session_start fires exactly once at boot, that session_end fires on each graceful quit path (/exit, Ctrl+D, Ctrl+C), and that both events work with the existing hook configuration (command execution, matcher not applicable). A test should also confirm that session_end is not called twice if quit() is invoked multiple times.

Correctness concerns

  • session_end is invoked as fire-and-forget (runHooks("session_end") without await) inside quit(). The quit() function proceeds immediately to cleanup (clearInterval, disconnectAll, etc.) and then the REPL loop resolves, likely causing the process to exit. If the hook command takes any time (e.g., writing to a file or sending a network request), it may be killed before completion, leading to truncated or lost output. This is a risk for users who rely on session_end for reliable teardown.
  • The session_start call is similarly fire-and-forget, but that is less critical.
  • Both calls inherit the existing hook error-handling pattern (errors are logged but not surfaced), which is consistent but worth noting.
  • No other edge cases observed; the placement of the calls is correct.

Verdict
Needs human judgment call on whether session_end should be awaited to guarantee hook completion before process exit; tests for the new events are missing.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

@Prateekgaur

Copy link
Copy Markdown
Collaborator

session_start runs once when the app starts up
session_end runs when quitting normally, whether that's typing /exit, hitting Ctrl+D, or Ctrl+C
session_end doesn't accidentally run twice if quit gets triggered more than once

Writing these tests now also gives us a safety net if we later change how quitting works — we'd know right away if session_end stopped firing.

Extract createSessionLifecycle so we can assert session_start fires
once at boot and session_end fires once across repeated quit triggers
(/exit, Ctrl+D, Ctrl+C all call quit → end).
@Ayush7614

Copy link
Copy Markdown
Contributor Author

@Prateekgaur thanks — added tests covering exactly those cases:

  • session_start fires once on boot (start())
  • session_end fires on graceful quit (all of /exit, Ctrl+D, Ctrl+C go through quit()end())
  • session_end does not fire twice if quit is triggered multiple times

Extracted a small createSessionLifecycle helper so this stays unit-testable without booting the full TUI. runHooks already uses spawnSync, so session_end hooks finish before cleanup continues.

@aakashrajput
aakashrajput merged commit a389156 into KlaatAI:main Jul 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add session_start/session_end hook events

3 participants