Fix issue #2180 preemptive compaction warnings#2212
Merged
Yeachan-Heo merged 2 commits intodevfrom Apr 6, 2026
Merged
Conversation
The legacy preemptive-compaction helper was never wired into hooks/hooks.json and depended on in-memory Maps that cannot survive Claude Code's one-shot shell hook processes. This change moves the warning logic into the already-registered post-tool-verifier runtime, reads transcript usage from the transcript tail, and persists cooldown by directory+session so warnings stay accurate across subprocess invocations without cross-session suppression. Constraint: PostToolUse runtime is delivered via hooks/hooks.json + scripts/post-tool-verifier.mjs Constraint: Fix must remain small and avoid registering the non-viable legacy helper directly Rejected: Register a standalone preemptive-compaction script in hooks.json | Would add another runtime surface when the existing PostToolUse path already supports additionalContext Rejected: Reuse directory-only cooldown state | Incorrectly suppresses warnings across different sessions in the same repo Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep preemptive warning persistence scoped to session-aware hook state; do not revert to process-local Maps for shell hook runtime behavior Tested: vitest --run src/__tests__/post-tool-verifier.test.mjs src/__tests__/preemptive-compaction-hook.test.ts src/__tests__/hooks-command-escaping.test.ts Tested: npx tsc --noEmit Tested: npm run lint (passes with one pre-existing warning in src/hooks/setup/__tests__/stdin-symlink.test.ts) Not-tested: Full repository test suite Related: #2180
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ommand CI runs the repository's current hooks.json, which may invoke run.cjs via either a plain node launcher or a pinned node binary path. The regression test should prove that preemptive compaction stays on the active PostToolUse runtime path without overfitting to the exact launcher prefix. Constraint: hooks.json command prefixes can vary between environments while the run.cjs -> post-tool-verifier.mjs path stays invariant Rejected: Assert the full command string starts with plain node | fails when hooks.json uses an absolute node launcher path Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep this regression focused on the active hook route invariant, not the launcher prefix Tested: vitest --run src/__tests__/preemptive-compaction-hook.test.ts src/__tests__/post-tool-verifier.test.mjs src/__tests__/hooks-command-escaping.test.ts src/__tests__/context-guard-stop.test.ts Tested: eslint src/__tests__/preemptive-compaction-hook.test.ts Not-tested: Full repository test suite Related: #2180 Related: #2212
pgagarinov
pushed a commit
to pgagarinov/oh-my-claudecode
that referenced
this pull request
Apr 12, 2026
…o#2212) * Restore preemptive context warnings on the active PostToolUse path The legacy preemptive-compaction helper was never wired into hooks/hooks.json and depended on in-memory Maps that cannot survive Claude Code's one-shot shell hook processes. This change moves the warning logic into the already-registered post-tool-verifier runtime, reads transcript usage from the transcript tail, and persists cooldown by directory+session so warnings stay accurate across subprocess invocations without cross-session suppression. Constraint: PostToolUse runtime is delivered via hooks/hooks.json + scripts/post-tool-verifier.mjs Constraint: Fix must remain small and avoid registering the non-viable legacy helper directly Rejected: Register a standalone preemptive-compaction script in hooks.json | Would add another runtime surface when the existing PostToolUse path already supports additionalContext Rejected: Reuse directory-only cooldown state | Incorrectly suppresses warnings across different sessions in the same repo Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep preemptive warning persistence scoped to session-aware hook state; do not revert to process-local Maps for shell hook runtime behavior Tested: vitest --run src/__tests__/post-tool-verifier.test.mjs src/__tests__/preemptive-compaction-hook.test.ts src/__tests__/hooks-command-escaping.test.ts Tested: npx tsc --noEmit Tested: npm run lint (passes with one pre-existing warning in src/hooks/setup/__tests__/stdin-symlink.test.ts) Not-tested: Full repository test suite Related: Yeachan-Heo#2180 * Keep preemptive hook regression aligned with the active PostToolUse command CI runs the repository's current hooks.json, which may invoke run.cjs via either a plain node launcher or a pinned node binary path. The regression test should prove that preemptive compaction stays on the active PostToolUse runtime path without overfitting to the exact launcher prefix. Constraint: hooks.json command prefixes can vary between environments while the run.cjs -> post-tool-verifier.mjs path stays invariant Rejected: Assert the full command string starts with plain node | fails when hooks.json uses an absolute node launcher path Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep this regression focused on the active hook route invariant, not the launcher prefix Tested: vitest --run src/__tests__/preemptive-compaction-hook.test.ts src/__tests__/post-tool-verifier.test.mjs src/__tests__/hooks-command-escaping.test.ts src/__tests__/context-guard-stop.test.ts Tested: eslint src/__tests__/preemptive-compaction-hook.test.ts Not-tested: Full repository test suite Related: Yeachan-Heo#2180 Related: Yeachan-Heo#2212
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
1 participant
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.
Summary\n- move preemptive context warnings onto the already-registered PostToolUse runtime\n- read transcript usage from the transcript tail and persist cooldown by directory + session\n- add regression coverage for warning thresholds, cross-process cooldown, session isolation, and escalation\n\nCloses #2180\n\n## Verification\n- npm test -- --run src/tests/post-tool-verifier.test.mjs src/tests/preemptive-compaction-hook.test.ts src/tests/hooks-command-escaping.test.ts\n- npx tsc --noEmit\n- npm run lint (passes with one pre-existing warning in src/hooks/setup/tests/stdin-symlink.test.ts)