Skip to content

fix(hook): reduce hook spawn overhead#970

Open
zerone0x wants to merge 2 commits intortk-ai:developfrom
zerone0x:fix/issue-968-hook-spawn-eagain
Open

fix(hook): reduce hook spawn overhead#970
zerone0x wants to merge 2 commits intortk-ai:developfrom
zerone0x:fix/issue-968-hook-spawn-eagain

Conversation

@zerone0x
Copy link
Copy Markdown
Contributor

@zerone0x zerone0x commented Apr 2, 2026

Summary

  • cache the rtk version guard to avoid spawning multiple processes on every hook run
  • parse version in bash to remove grep/cut/head subprocesses
  • reduce jq invocations by building updated input directly from the original payload

Fixes #968

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk <command> output inspected

Important: All PRs must target the develop branch (not master).
See CONTRIBUTING.md for details.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 2, 2026

CLA assistant check
All committers have signed the CLA.

@aeppling
Copy link
Copy Markdown
Contributor

aeppling commented Apr 2, 2026

Hello,

Thanks for the contribution

Please sign the CLA above so we can merge this

@aeppling aeppling self-assigned this Apr 2, 2026
@zerone0x zerone0x force-pushed the fix/issue-968-hook-spawn-eagain branch from f7f5b93 to 74a1fd2 Compare April 2, 2026 11:00
Copy link
Copy Markdown
Collaborator

@pszymkowiak pszymkowiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zerone0x — solid optimization. The version cache, herestring instead of echo|jq, and single-pass jq are all good improvements for reducing hook overhead (#968).

Two notes (non-blocking):

  1. The cache file (~/.cache/rtk-hook-version-ok) has no expiry — after an rtk upgrade, the old cache persists. Consider adding the version to the filename (e.g., rtk-hook-version-ok-0.34) or clearing it on rtk init.
  2. This PR modifies rtk-rewrite.sh which is also modified by PR #981 (permission model). Whichever merges first will cause a conflict on the other — just a heads-up for rebase.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants