feat(synapse): SYN Wave 2 — Hook Entry Point + Registration [Story SYN-7]#127
feat(synapse): SYN Wave 2 — Hook Entry Point + Registration [Story SYN-7]#127Pedrovaleriolopez merged 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new Node.js hook script (synapse-engine.js) has been added under .claude/hooks that serves as an entry point for Synapse Engine prompt processing. The implementation reads JSON from stdin, manages sessions, delegates to the engine, and writes structured output to stdout with a 5-second safety timeout. Supporting .gitignore rules and comprehensive test coverage are included. Changes
Sequence Diagram(s)sequenceDiagram
participant stdin as Stdin/Caller
participant hook as synapse-engine.js
participant reader as readStdin()
participant engine as SynapseEngine
participant session as Session Manager
participant stdout as Stdout
stdin->>hook: Process invoked with JSON payload
hook->>reader: Call readStdin()
reader->>stdin: Read JSON from stream
stdin-->>reader: JSON data
reader-->>hook: Parsed payload object
hook->>hook: Validate .synapse directory exists
alt .synapse missing
hook->>stdout: Exit silently (code 0)
else Directory exists
hook->>session: loadSession(cwd)
session-->>hook: Session object (or null)
hook->>engine: Instantiate SynapseEngine
hook->>engine: process(prompt, session)
engine-->>hook: Result with optional xml
hook->>hook: Extract additionalContext from result.xml
hook->>stdout: Write { hookSpecificOutput: { additionalContext } }
end
hook->>hook: Enforce 5s timeout, then exit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
bot
added
type: docs
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.claude/hooks/synapse-engine.js:
- Around line 41-45: The main function reads input and destructures sessionId,
cwd, and prompt but does not validate cwd before using it to build synapsePath;
add a guard at the top of main to verify cwd is a non-empty string (and
optionally that sessionId/prompt exist if required) and return early (or log a
clear error) when cwd is missing/invalid so path.join is never called with
undefined; update the validation near the readStdin() call and before computing
synapsePath in function main to use the cwd check.
🧹 Nitpick comments (5)
.claude/hooks/synapse-engine.js (1)
28-38: Missingerrorevent handler onprocess.stdincould leave the promise hanging.If stdin emits an
errorevent (e.g., broken pipe), the promise is never settled. Whilerun()'s 5-second timeout provides a backstop, explicitly handling the error event is more robust.Proposed fix
function readStdin() { return new Promise((resolve, reject) => { let data = ''; process.stdin.setEncoding('utf8'); + process.stdin.on('error', (e) => reject(e)); process.stdin.on('data', (chunk) => { data += chunk; }); process.stdin.on('end', () => { try { resolve(JSON.parse(data)); } catch (e) { reject(e); } }); }); }tests/synapse/hook-entry.test.js (4)
469-501: Monkey-patchedprocess.stdinis not restored if the test throws.In multiple tests (here and in the
main()in-process tests below),process.stdinandprocess.stdout.writeare replaced viaObject.definePropertybut only restored in the happy path. If an assertion or the promise rejects unexpectedly, the originals are never restored, which could cascade failures to subsequent tests.Wrapping in
try/finallyis the standard safeguard:Example pattern
const originalStdin = process.stdin; + try { const mockStdin = new Readable({ read() {} }); Object.defineProperty(process, 'stdin', { value: mockStdin, writable: true }); const promise = hookModule.readStdin(); mockStdin.push('not json'); mockStdin.push(null); await expect(promise).rejects.toThrow(); + } finally { Object.defineProperty(process, 'stdin', { value: originalStdin, writable: true }); + }This applies to all tests in sections 8 and 9 that patch
process.stdin/process.stdout.write/process.exit(lines 469–620).
503-525: Same restoration concern forprocess.stdout.write.The
originalWriteis only restored after the assertions. IfJSON.parse(captured)or any expect throws,process.stdout.writestays patched. Apply the sametry/finallypattern here and in the othermain()in-process tests (lines 527–587).
597-620:process.exitspy not guarded bytry/finally.If the
expecton line 612 or 613 fails,exitSpy.mockRestore()anderrorSpy.mockRestore()on lines 617-618 are skipped. A leakedprocess.exitmock is particularly dangerous — it silently swallows exits for all remaining tests.Safer pattern
const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { // ... test body ... expect(exitSpy).toHaveBeenCalledWith(0); expect(errorSpy).toHaveBeenCalledWith( expect.stringContaining('[synapse-hook]') ); + } finally { exitSpy.mockRestore(); errorSpy.mockRestore(); Object.defineProperty(process, 'stdin', { value: originalStdin, writable: true }); + }
385-408: Timing-based assertions can be flaky in resource-constrained CI environments.The 2000ms and 1500ms thresholds are generous, but under heavy load (shared CI runners, Windows containers), spawn overhead alone can spike. Consider bumping the thresholds slightly or marking these with a retry/flaky annotation if you start seeing intermittent failures.
- Add stdin error event handler in readStdin() to prevent unhandled rejections - Add cwd validation guard before path.join() to handle missing input gracefully - Wrap process.stdin/stdout/exit mocks in try/finally blocks for reliable test cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pedrovaleriolopez
force-pushed
the
feat/syn-7-hook-entry
branch
from
6bf8a4d to
8191e37
Compare
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Summary
UserPromptSubmithook (.claude/hooks/synapse-engine.js) that integrates SynapseEngine for per-prompt context injection via stdin/stdout JSON protocolSynapseEngine.process(), writes<synapse-rules>context to stdoutHOOK_TIMEOUT_MS=5000) withtimer.unref()as defense-in-depth alongside Claude Code's external timeout.gitignoreupdated with SYNAPSE runtime entries (.synapse/sessions/,.synapse/cache/) and!.claude/hooks/synapse-engine.jsexceptionKey Design Decisions
path,fs)require.main === moduleguard — enables both direct script execution and Jest import for coverage tracking.synapse/existence check for fast-exit pathTest Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores