Skip to content

feat(cli): extract TUI hooks into typed createTUIHooks factory#591

Open
aaight wants to merge 1 commit intodevfrom
feat/tui-hooks-factory
Open

feat(cli): extract TUI hooks into typed createTUIHooks factory#591
aaight wants to merge 1 commit intodevfrom
feat/tui-hooks-factory

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Apr 24, 2026

Summary

  • Extracts the ~120-line inline tuiHooks object from agent-command.ts into a new packages/cli/src/tui/tui-hooks.ts module
  • Creates a createTUIHooks(options): AgentHooks factory function following the pattern in packages/llmist/src/agent/hook-presets.ts
  • Replaces all any-typed hook contexts with proper types (ObserveLLMCallContext, ObserveChunkContext, ObserveLLMCompleteContext, ObserveRateLimitThrottleContext, ObserveRetryAttemptContext, GadgetExecutionControllerContext)
  • Adds 30 unit tests in packages/cli/src/tui/tui-hooks.test.ts covering all observer hooks and the beforeGadgetExecution controller

Closes: https://linear.app/mongrel/issue/MNG-294

Key Decisions

  • Mutable ref containers (iterationsRef, usageRef) instead of raw variables so the factory can update state the caller reads after the agent run, without returning values from the hook callbacks
  • Mutable gadgetApprovals object passed by reference so "always allow"/"always deny" choices persist in the caller's map across REPL turns
  • StatusBar imported directly from ./status-bar.js (not ./index.js) to avoid potential circular import issues since tui-hooks.ts lives inside the tui/ directory
  • agent-command.ts retains the tuiHooks: AgentHooks local variable type annotation so the surrounding code is minimally disturbed

Test plan

  • npm run typecheck passes (0 errors)
  • npm run test passes (220 test files, 6569 tests)
  • npm run lint (no new errors introduced; pre-existing warnings unchanged)
  • All existing agent-command-tui.test.ts and agent-command-throttle.test.ts tests continue passing
  • New tui-hooks.test.ts covers onLLMCallStart, onStreamChunk, onLLMCallComplete, onRateLimitThrottle, onRetryAttempt, and beforeGadgetExecution

🤖 Generated with Claude Code

🕵️ claude-code · claude-sonnet-4-6 · run details

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.61538% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/cli/src/tui/tui-hooks.ts 94.06% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

LGTM — Clean extraction of ~120 lines of inline hook logic into a typed, testable factory function. The refactoring is behavior-preserving, follows the existing HookPresets pattern, replaces all any-typed contexts with proper types, and comes with thorough test coverage (30 tests covering all hooks).

The mutable ref pattern (iterationsRef, usageRef) is a reasonable approach for sharing state between the factory's hooks and the caller. CI is green across all checks.

Nitpicks

  • tui-hooks.test.ts:12beforeEach is imported from vitest but never used
  • tui-hooks.test.ts:46createDefaultApprovalConfig() helper is defined but never called — dead code that can be removed

🕵️ claude-code · claude-opus-4-6 · run details

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.

3 participants