airkjw/claude-mem
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MAESTRO: Close PR #922 - async SessionStart hooks would break context injection

Browse Source 
SessionStart hooks must run synchronously because the context hook's stdout
is captured and injected as Claude's system-level memory context. The Windows
blocking issue was already resolved by the fail-open approach in PRs #973,
#959, and #964.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-02-05 18:27:06 -05:00
parent faa1360f33
commit 4e4ad32f97
1 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Auto Run Docs/PR-Triage/PR-Triage-04.md
+2 -1
View File
@@ -22,6 +22,7 @@ All these PRs share the goal of preventing hooks from blocking Claude Code promp
- [x] Review PR #964 (`Add fetch timeouts to Stop hook and health checks` by @rodboev). Files: `src/cli/handlers/summarize.ts`, `src/shared/worker-utils.ts`. Adds AbortController timeouts to prevent hooks from hanging on fetch calls. Steps: (1) `gh pr checkout 964` (2) Verify timeout values are reasonable (should be < hook timeout of 120s) (3) Check that AbortController usage is correct (signal passed to fetch) (4) Run `npm run build` (5) If clean: `gh pr merge 964 --rebase --delete-branch`
- **Merged** on 2026-02-05. Rebased cleanly onto main. Adds `fetchWithTimeout()` helper in `worker-utils.ts` using `Promise.race` + `setTimeout` (avoids `AbortSignal.timeout()` which causes libuv assertion crash in Bun on Windows). Applied `HEALTH_CHECK_TIMEOUT_MS` (30s / 45s on Windows) to `isWorkerHealthy()` and `getWorkerVersion()` — these previously had no timeout and would hang indefinitely when worker was unreachable. Applied `HOOK_TIMEOUTS.DEFAULT` (5min) to summarize POST request. Implementation properly clears timeout on both resolve and reject paths. Build clean.
- [ ] Review PR #922 (`fix: add async:true to SessionStart hooks` by @kamran-khalid-v9). File: `plugin/hooks/hooks.json`. This adds `async: true` to SessionStart hooks so they don't block terminal on Windows. Steps: (1) `gh pr checkout 922` (2) Verify that making SessionStart async doesn't break context injection (context must be available before Claude starts processing) (3) **IMPORTANT**: If SessionStart is async, context won't be injected in time. This may conflict with the architecture. Verify carefully. (4) If async is inappropriate for SessionStart (which injects context), close with explanation. If only certain sub-hooks should be async, request changes.
- [x] Review PR #922 (`fix: add async:true to SessionStart hooks` by @kamran-khalid-v9). File: `plugin/hooks/hooks.json`. This adds `async: true` to SessionStart hooks so they don't block terminal on Windows. Steps: (1) `gh pr checkout 922` (2) Verify that making SessionStart async doesn't break context injection (context must be available before Claude starts processing) (3) **IMPORTANT**: If SessionStart is async, context won't be injected in time. This may conflict with the architecture. Verify carefully. (4) If async is inappropriate for SessionStart (which injects context), close with explanation. If only certain sub-hooks should be async, request changes.
- **Closed** on 2026-02-05. Making SessionStart hooks async would break claude-mem's core functionality — the `context` hook's stdout must be captured synchronously for memory context injection. The three SessionStart hooks also have a strict dependency chain (install → start worker → fetch context). The Windows blocking issue was already resolved by the fail-open approach in PRs #973, #959, and #964 (graceful failures, empty context during init, fetch timeouts). PR was also stale: referenced `bun` directly instead of `bun-runner.js` wrapper and included the `user-message` hook removed in PR #960.
- [ ] Evaluate PR #530 (`fix: add retry logic with exponential backoff to hook fetch calls` by @BeamNawapat). Files: 10 files including all hook scripts and worker-utils. This is an older PR (Jan 3) that adds retry logic. Steps: (1) Check if PR is rebased on current main (2) The approach (retry with backoff) may conflict with the "fail fast" philosophy — hooks should fail quickly rather than retry. (3) If the retry approach conflicts with the hook resilience PRs above (#973, #959), close with: `gh pr close 530 --comment "The hook resilience approach has evolved — hooks now fail open rather than retry. See PRs #973 and #959 for the current approach. Thank you for the contribution!"`