feat(hooks): wire sessionStart and sessionEnd lifecycle hooks

[raykao]

Wires sessionStart and sessionEnd into session-manager.ts — they were defined, tested, and silently never called. See #157 for full context, rationale, and validation results.

Changes

src/core/session-manager.ts

• sessionStart: fired non-blocking after session ready in createNewSession() and attachSession()
• sessionEnd: awaited before destroySession() in newSession() and reloadSession()
• Hooks cache invalidated on /new and /reload so hooks.json changes are picked up

src/core/hooks-loader.ts

• Use /bin/bash (absolute path) on non-Windows — prevents ENOENT in nvm environments where subprocess PATH may not include bash

Why sessionEnd is awaited, sessionStart is not

sessionEnd must complete before destroySession() — hook may be writing a backup. sessionStart fires after the session is live — awaiting it would delay the first response.

Tests

All 578 existing tests pass. Validated end-to-end: hooks fired correctly, no warnings, bd backup export-git committed to git branch on session end.

Relates to: #157

[Copilot]

Pull request overview

Wires previously-defined sessionStart/sessionEnd hook types into SessionManager so workspace hook automation can run at session lifecycle boundaries.

Changes:

• Fire sessionStart hooks after session creation (createNewSession) and resume (attachSession) in a non-blocking, best-effort manner.
• Fire sessionEnd hooks before teardown in /new (newSession) and /reload (reloadSession), awaiting completion and logging failures as warnings.

Comments suppressed due to low confidence (2)

src/core/session-manager.ts:705

• In reloadSession(), the session event listener is unsubscribed before awaiting the sessionEnd hook. Because channelSessions still points at the existing session until destroySession runs, a concurrent sendMessage() during this window will reuse the cached session (ensureSession() sees bridge.getSession()) but no longer has attachSessionEvents wired, so responses/usage events can be dropped. Consider firing sessionEnd before unsubscribing, or otherwise marking the session as “reloading” so ensureSession/sendMessage will wait or re-attach events during teardown.

    const unsub = this.sessionUnsubscribes.get(existingId);
    if (unsub) { unsub(); this.sessionUnsubscribes.delete(existingId); }

    // Fire sessionEnd hook before teardown (best-effort)
    const reloadWorkingDirectory = this.resolveWorkingDirectory(channelId);
    const reloadRawHooks = await this.resolveHooks(reloadWorkingDirectory);
    const reloadHooks = this.wrapHooksWithAsk(reloadRawHooks, channelId);

src/core/session-manager.ts:706

• resolveHooks() caches hooks per workingDirectory, so reloadSession() (and newSession()) will use potentially stale hooks.json contents when firing sessionEnd (and later sessionStart) after a hooks.json edit. Since /reload is intended to pick up workspace config changes, consider bypassing/invalidating the workspaceHooks cache for this workingDirectory on /reload (and possibly /new) before resolving hooks for these lifecycle events.

    // Fire sessionEnd hook before teardown (best-effort)
    const reloadWorkingDirectory = this.resolveWorkingDirectory(channelId);
    const reloadRawHooks = await this.resolveHooks(reloadWorkingDirectory);
    const reloadHooks = this.wrapHooksWithAsk(reloadRawHooks, channelId);
    if (reloadHooks?.onSessionEnd) {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The sessionEnd hook firing logic is duplicated between newSession() and reloadSession() (resolveWorkingDirectory → resolveHooks → wrapHooksWithAsk → try/await hook). Consider extracting a small helper (e.g., fireSessionEndHook(channelId, sessionId)) so future changes (timeouts, logging context, cache invalidation) stay consistent across call sites.

This issue also appears in the following locations of the same file:

• line 699
• line 702

See below for a potential fix:

  private async fireSessionEndHook(channelId: string, sessionId: string): Promise<void> {
    // Fire sessionEnd hook before teardown (best-effort)
    const workingDirectory = this.resolveWorkingDirectory(channelId);
    const rawHooks = await this.resolveHooks(workingDirectory);
    const hooks = this.wrapHooksWithAsk(rawHooks, channelId);
    if (hooks?.onSessionEnd) {
      try {
        await hooks.onSessionEnd({ sessionId, channelId }, { sessionId });
      } catch (err: any) {
        log.warn(`sessionEnd hook failed: ${err?.message ?? err}`);
      }
    }
  }

  async newSession(channelId: string): Promise<string> {
    // Clean up existing session
    const existingId = this.channelSessions.get(channelId);
    if (existingId) {
      await this.fireSessionEndHook(channelId, existingId);

[raykao]

Addressed both suggestions from the code review in commit 1e69de5:

1. Event ordering in reloadSession() — sessionEnd now fires before unsub() so event listeners remain wired during the hook await, preventing concurrent sendMessage() calls from dropping response/usage events.

2. Stale hooks cache — workspaceHooks.delete(workingDirectory) is called before resolveHooks() in both newSession() and reloadSession(), ensuring /new and /reload always read the latest hooks.json from disk.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/core/session-manager.ts:706

• Same issue as /new: sessionEnd hook resolution/cache invalidation uses resolveWorkingDirectory(channelId) at call time. If the channel’s working directory changes between session start and /reload, this can run the wrong workspace’s sessionEnd hooks. Consider using the workingDirectory associated with existingId (tracked at create/resume time) when firing sessionEnd.

    const reloadWorkingDirectory = this.resolveWorkingDirectory(channelId);
    this.workspaceHooks.delete(reloadWorkingDirectory);
    const reloadRawHooks = await this.resolveHooks(reloadWorkingDirectory);
    const reloadHooks = this.wrapHooksWithAsk(reloadRawHooks, channelId);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

sessionEnd hook loading is based on resolveWorkingDirectory(channelId) at the time /new is invoked. If the user changed the workspace override/config and then runs /new (which is suggested elsewhere in this file), this will fire sessionEnd using the new workspace’s hooks.json rather than the workspace the ending session actually ran in. Consider tracking the workingDirectory used when the session was created/resumed (e.g., map sessionId→workingDirectory) and using that for hook cache invalidation + resolveHooks() here.

This issue also appears on line 703 of the same file.

[Copilot]

Hardcoding the bash executable to /bin/bash on non-Windows can be a portability regression: it bypasses the user’s PATH and can force an older bash (notably macOS ships bash 3.2 at /bin/bash), breaking hooks that rely on bash 4+ features. Prefer resolving bash from PATH (or using /usr/bin/env bash) so the configured environment determines which bash runs.

Suggested change

	const shell = cmd.bash ? (process.platform === 'win32' ? 'bash' : '/bin/bash') : 'powershell';
	const shell = cmd.bash ? 'bash' : 'powershell';

[raykao]

Additional fix discovered during local end-to-end validation (commit 85ed66c):

fix(hooks): use absolute path /bin/bash to avoid ENOENT on spawn

In nvm-managed Node.js environments, the subprocess PATH may not include directories where bash lives, causing spawn bash ENOENT. Fixed by using /bin/bash (absolute path) on non-Windows platforms.

[raykao]

Addressed review feedback (commit 8c29c9e):

Added sessionWorkingDirectories map (sessionId → workingDirectory) populated at session create (createNewSession) and resume (attachSession) time. newSession() and reloadSession() now read from this map with a fallback to resolveWorkingDirectory(channelId). Map is cleaned up on destroy. This ensures sessionEnd always fires against the workspace the session was originally created in, regardless of live channel config changes.

[ChrisRomp]

Thanks for this, @raykao! The lifecycle hooks are a solid addition. A few things before this can merge:

CCR threads (3 unresolved):

Copilot code review left 3 comments. Two are marked outdated (looks like you may have addressed them with the sessionWorkingDirectories map), and one is current. If you've addressed them, please resolve the threads or reply so we can track status.

1. Duplicate sessionEnd logic (outdated) - CCR suggests extracting a fireSessionEndHook() helper. The hook-firing block is repeated in both newSession() and reloadSession(). Even with the workspace-tracking fix, the duplication remains. A small private method would keep this DRY.

2. Workspace tracking for sessionEnd (outdated) - CCR flagged that resolveWorkingDirectory(channelId) at teardown time might not match the session's original workspace. Your sessionWorkingDirectories map addresses this. If so, please resolve the thread.

3. /bin/bash hardcode (current) - The change from 'bash' to '/bin/bash' bypasses PATH and forces macOS's ancient bash 3.2 at that path. The original 'bash' is better since it lets the user's PATH determine which bash runs. Academically: on Windows, hooks use the powershell branch, so the bash path is moot there. But there's no guard if bash isn't installed at all (spawn would just fail). Something to consider but not a blocker for this PR.

On beads.agent.md in templates/:

The template lives in templates/agents/ which could imply it ships as part of the default agent set. Since it only activates if manually copied to a workspace and invoked via /agent beads, it's not a runtime risk. But we're considering adding a docs/recipes/ folder for opt-in workflow guides. This might be a better home for it alongside the docs/beads.md guide, rather than in templates/agents/. Thoughts?

Additional:

• No tests for the new session manager changes. At minimum, a test verifying sessionEnd fires on /new and that sessionWorkingDirectories cleans up would be good to have.