fix(routing): scope guidance throttle by session, not process.ppid (#298)#313
Merged
Conversation
…ksglu#298) On Windows + Git Bash each hook invocation spawns a fresh bash.exe with a different PID, so the legacy ppid-based marker directory changed every call and guidance was injected on every tool use instead of once per session. The PreToolUse hooks already have a stable session identifier in the hook payload; this change plumbs it into `routePreToolUse` and names the marker directory `context-mode-guidance-s-<sessionId>`. The ppid path stays as a backward-compatible fallback for callers that don't pass one. - core/routing.mjs: guidanceOnce(type, content, sessionId?), marker dir resolved per-call instead of pinned at module load. - All six adapter hooks (claude-code, codex, cursor, gemini-cli, kiro, vscode-copilot) now pass getSessionId(input, <PLATFORM_OPTS>). - In-process plugins (openclaw, opencode) are unchanged — they share a Node process across hook invocations, so process.ppid was never unstable for them. Tests: 5 new regression cases in tests/guidance-throttle.test.ts prove the throttle is now immune to ppid churn, that distinct sessionIds get independent throttles, and that the legacy no-sessionId path still works. Subprocess-based integration tests (integration.test.ts, vscode-hooks.test.ts) updated to clean the new session-scoped dir. - npx vitest run tests/guidance-throttle.test.ts → 13/13 pass - npm test → 1600/1600 pass, 23 skipped, 0 failures - npm run typecheck → clean Co-Authored-By: Claude <noreply@anthropic.com>
Owner
|
Thanks @ousamabenyounes — threading the stable sessionId through routePreToolUse is the right approach. Clean backward-compat fallback, all 6 adapter hooks updated, solid test coverage. Merged! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
guidanceOnce()inhooks/core/routing.mjsscoped its on-disk marker directory byprocess.ppid. On Windows + Git Bash each hook invocation spawns a freshbash.exewith a different PID, so the marker directory name changed every call, the throttle never fired, and Read/Bash/Grep guidance was injected on every tool use — 716 stale marker dirs in/tmpand ~10 KB of repetitive context noise per session, per #298.This PR threads the stable
sessionIdfrom the hook payload throughroutePreToolUseand names the marker directorycontext-mode-guidance-s-<sessionId>instead. The legacy ppid-based path stays as a backward-compatible fallback for callers that don't pass a session identifier.Fixes #298.
Why sessionId, not just a patched ppid
claude-code,codex,cursor,gemini-cli,kiro,vscode-copilot) already reads hook input and hasgetSessionId(input, <PLATFORM_OPTS>)available viasession-helpers.mjs. It's derived from the hook payload'stranscript_path/conversation_id/sessionId/session_id/ env var, with a finalpid-${ppid}fallback. That identifier is stable across hook invocations within the same logical session.routePreToolUseis additive (optional 5th arg) — no behavior change for callers that don't pass one, and no need to rely on platform-specific env vars or extra syscalls.sessionstart.mjs) all required more moving parts to reach the same fix.Changes
hooks/core/routing.mjs_guidanceIdis no longer pinned at module load —guidanceDirFor(sessionId)resolves the marker dir per call, usings-<sessionId>when one is passed, falling back to the ppid suffix otherwise.guidanceOnce(type, content, sessionId?)andresetGuidanceThrottle(sessionId?)accept the optional session id.routePreToolUse(toolName, toolInput, projectDir, platform, sessionId?)threads it into the threeguidanceOncecall sites (Bash/Read/Grep).Six adapter hooks now import the matching
*_OPTS(or none for Ora Studio default) and passgetSessionId(input, ...)at the routing call site:hooks/pretooluse.mjs(claude-code)hooks/codex/pretooluse.mjshooks/cursor/pretooluse.mjshooks/gemini-cli/beforetool.mjshooks/kiro/pretooluse.mjshooks/vscode-copilot/pretooluse.mjsScope note:
src/openclaw-plugin.tsandsrc/opencode-plugin.tsare intentionally not touched. Both run in-process across hook invocations, soprocess.ppidwas never unstable for them and the ppid fallback stays correct.Coverage added (added to
tests/guidance-throttle.test.ts, per CONTRIBUTING's "no new test files" rule)5 regression cases in a new
describe("sessionId scoping (#298 — stable across shifting ppids)"):second call with same sessionId is throttled even when in-memory Set is cleared— simulates the Windows Git Bash case (fresh process per hook) and asserts the on-disk marker still blocks the second call.different sessionIds get independent throttles— two sessions, same process, each fires its own guidance exactly once.sessionId routing is immune to process.ppid changes— plants a marker in the legacy ppid dir; the sessionId-scoped call ignores it.resetGuidanceThrottle(sessionId)clears the session-scoped dir.no sessionId passed → falls back to ppid-based behavior (backward compat)— the previously existing contract is preserved for in-process callers.Subprocess-based tests (
tests/hooks/integration.test.ts,tests/hooks/vscode-hooks.test.ts) now also cleancontext-mode-guidance-s-pid-<testpid>in theirbeforeEachso the new marker location doesn't leak between tests.Test plan
npx vitest run tests/guidance-throttle.test.ts→ 13/13 pass (5 new + 8 existing)npx vitest run tests/hooks/integration.test.ts tests/hooks/vscode-hooks.test.ts tests/guidance-throttle.test.ts→ 67/67 passnpm test→ 1600 pass, 23 skipped, 0 failures — including the subprocess JSON-parse cases inintegration.test.tsthat were flaking onnextnpm run typecheck→ cleanNo runtime behavior changes for macOS/Linux users. The fix activates when a caller passes a session id — which all six adapter hooks now do.
Generated by Ora Studio
Vibe coded by ousamabenyounes