fix(axi-sdk-js): make session hook setup explicit#42
Merged
Conversation
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.
Intent
The developer wanted to address a GitHub issue about the JavaScript SDK silently auto-installing agent session hooks into user-wide Claude, Codex, and OpenCode configuration during normal CLI execution. They asked to investigate the issue, consider downstream SDK usage such as gh-axi, and propose then implement an opt-in model. They wanted normal CLI commands to be side-effect free, with hook installation moved to an explicit setup command, while preserving a simple installer API where installSessionStartHooks() can infer the current CLI defaults when called without options. They also expected tests and documentation to reflect the new behavior and downstream consumers to migrate away from implicit hooks.
What Changed
runAxiCliexecution so ordinary CLI commands no longer write user-wide agent configuration.installSessionStartHooks()API, with safeguards for development entrypoints and preserved default inference when invoked from setup flows.Risk Assessment
✅ Low: The change is narrowly scoped to removing implicit hook setup and adding guarded explicit installation inference, with tests covering the main safety paths and no material risks found in the reviewed diff.
Testing
I ran the relevant axi-sdk-js automated tests, handled the unsupported inlinevite-noderunner by building the package, then exercised the compiled end-user path in an isolated home directory: normalrunAxiCli()produced a home view without creating hook files, while explicitinstallSessionStartHooks()installed Claude, Codex, and OpenCode hook configuration successfully.Evidence: End-to-end CLI no-op vs explicit setup transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
Round 1 - passed ✅
✅ **Rebase** - passed
Round 1 - passed ✅
🔧 **Review** - 1 issue found → auto-fixed
Round 1 - found 1 warning
packages/axi-sdk-js/src/hooks.ts:505- When callers pass an explicit marker but run the setup command from a development TypeScript entrypoint, inference returns undefined, so no default shouldInstall policy is applied and installSessionStartHooks will write user hooks pointing at the .ts file. That recreates the bad persistent hook state the old runAxiCli inference path avoided for development entrypoints.Round 2 (auto-fix) - passed ✅
✅ **Test** - passed
Round 1 - passed ✅
pnpm --dir packages/axi-sdk-js test -- --runInBandpnpm --dir packages/axi-sdk-js exec vite-node -e ...(attempted e2e runner; this vite-node version requires script files, so no product behavior was exercised)pnpm --dir packages/axi-sdk-js run buildnode -e '...'usingpackages/axi-sdk-js/dist/index.jsto simulate normal CLI execution followed by explicitinstallSessionStartHooks({ homeDir })🔧 **Document** - 1 issue found → auto-fixed (2)
Round 1 - found 1 warning
packages/axi-sdk-js/README.md:79- The new behavior requires hook installation to happen only from an explicit user-invoked setup command, but the Session Hook Setup example shows a bare top-levelawait installSessionStartHooks();immediately after that guidance. Copied as-is, this can be placed at module startup and recreate the normal-command side effect the change is removing. Update the example to showinstallSessionStartHooks()inside a concretesetup hookscommand handler wired throughrunAxiCli(), ideally with a short success/no-op output example.Round 2 (auto-fix) - found 2 warnings
README.md:77- The top-level principle table still describes ambient context as "Self-install session integrations", but the change explicitly removes automatic hook installation from normal CLI execution and updates the AXI skill to require explicit user-invoked setup. Update this summary to say integrations are opt-in/installed via an explicit setup command so the public project overview does not continue recommending the removed behavior.docs/index.html:452- The website copy for principle 7 still says "Self-install session integrations" in the principles list and "Self-install into the agent's session hooks" in the detailed Ambient context section. This is stale relative to the new explicit-hook-setup behavior and the updated skill guidance. Update both website occurrences to describe explicit opt-in setup/install/repair instead of self-installing during ordinary CLI use.Round 3 (auto-fix) - passed ✅
✅ **Lint** - passed
Round 1 - passed ✅
✅ **Push** - passed
Round 1 - passed ✅