feat(core): gate tool use with hooks by taltas · Pull Request #1155 · Zoo-Code-Org/Zoo-Code

taltas · 2026-08-05T06:44:03Z

Stack

This is 4 of 4 in the Zoo Code hooks MVP stack. Review and merge from the bottom upward. Do not merge this PR until #1156 is merged.

Position	Pull request	Base	Scope
1	#1153	`main`	Hook contracts and policies
2	#1154	`fm/zoo-hooks-contracts`	Global settings and Hooks panel
3	#1156	`fm/zoo-hooks-settings`	Runner and `sessionStart` integration
4 (this PR)	#1155	`fm/zoo-hooks-session-start`	`preToolUse`, E2E, and documentation

Summary

Complete the hooks MVP by applying fail-closed preToolUse gates at every supported static, custom, and native MCP execution boundary, then document and exercise the full workflow in a real VS Code host.

Scope

Run exact-name preToolUse hooks after tool and argument validation but before approval, checkpointing, or execution.
Cover static tools, registered custom tools, and native MCP tool dispatch without double-running hooks.
Block execution on an explicit block decision and fail closed on hook errors, timeouts, or cancellation.
Preserve the exactly-one-real-tool-result invariant when a call is blocked.
Ensure invalid or mode-disallowed calls never invoke pre-execution hooks.
Add deterministic VS Code E2E coverage for session-start context, history reopen, pre-tool blocking, and process-tree cancellation.
Document supported phases, configuration, security boundaries, remote behavior, output limits, troubleshooting, and lifecycle diagrams in docs/hooks.md.
Tighten all new hook test doubles without increasing ESLint suppression counts.

Tool Gate

flowchart TD
    A[Model requests tool] --> B[Resolve tool and validate arguments]
    B -->|invalid or disallowed| C[Existing validation result]
    B -->|valid| D[Find exact preToolUse hooks]
    D --> E[Run hooks sequentially]
    E -->|all allow| F[Approval and checkpoint flow]
    F --> G[Execute static, custom, or MCP tool]
    E -->|block, error, timeout, cancel| H[Skip approval and execution]
    H --> I[Emit exactly one real tool result]
    G --> I

Tests

src/core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts
src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts
src/core/task/__tests__/Task.persistence.spec.ts
src/core/tools/__tests__/useMcpToolTool.spec.ts
src/core/webview/__tests__/webviewMessageHandler.spec.ts
webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx
packages/types/src/__tests__/hooks.test.ts
apps/vscode-e2e/src/suite/hooks.test.ts
Focused core validation: 5 files passed, 128 tests passed.
Full repository validation: pnpm test passed (437 files, 7,356 tests; 39 skipped).
pnpm check-types, pnpm lint, and pnpm build passed.
Real-host smoke validation: USE_MOCK=true TEST_FILE=hooks.test pnpm --filter @roo-code/vscode-e2e test:run passed.

Risks

Fail-closed behavior can prevent tool execution when a configured hook is broken. This is intentional for a security gate and is surfaced through structured hook status plus the tool result.
Tool dispatch has multiple implementations. Focused tests cover static, custom, and native MCP boundaries, including invalid and mode-disallowed calls.
Hooks run local executables with extension-host permissions and are not a sandbox. The documentation calls out trust, remote execution context, and import/export exclusion.
The exactly-one-result invariant is sensitive to early returns; regression tests count real tool results for blocked paths.

Review Notes

The unique diff is against fm/zoo-hooks-session-start, not main.
The original feature and test-typing commits remain reachable; additive merge da0d545b6 carries the intermediate-branch CI fixes forward without rewriting published history.
Additive merge e484eeae8 carries the Windows runner portability fix from #1156 into this top branch.
Additive merge 72fb1097f carries the explicit Windows system-binary path fix forward.
Additive merge dffdf7390 carries deterministic Windows-path coverage forward.
Additive merge 2e98360d6 carries the final taskkill fallback cleanup forward.
No changeset is included, per repository guidance.
Merge order is #1153 -> #1154 -> #1156 -> #1155.

coderabbitai · 2026-08-05T06:44:11Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 17083f09-f4b9-4f5c-826b-e08536ba4e15

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-08-05T06:54:57Z

Codecov Report

❌ Patch coverage is 89.87342% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../core/assistant-message/presentAssistantMessage.ts	86.20%	2 Missing and 2 partials ⚠️
src/core/task/Task.ts	92.50%	2 Missing and 1 partial ⚠️
...bview-ui/src/components/settings/HooksSettings.tsx	85.71%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

# Conflicts: # src/core/task/__tests__/Task.persistence.spec.ts # webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx

edelauna

Nice! Cool feature. Had a couple comments.

edelauna · 2026-08-08T12:16:15Z

+						beforeMcpExecution: async () =>
+							(await checkToolRepetition(cline, block, pushToolResult)) &&
+							(await runPreToolHooks(cline, "use_mcp_tool", pushToolResult)),
 					})


Is there a test covering the use_mcp_tool path through beforeMcpExecution? The static-tool path has spec coverage, but removing this branch wouldn't fail any test in the suite.

edelauna · 2026-08-08T12:16:15Z

+		return true
+	}
+	if (!cline.abort && result.status !== "cancelled") {
+		pushToolResult(formatResponse.toolError(result.reason))


What happens to the tool result when result.status === "cancelled"? The caller skips pushToolResult here, but is there anything upstream (e.g. validateAndFixToolResultIds) that synthesises a fallback, and is that path tested?

edelauna · 2026-08-08T12:16:15Z

+				return Object.freeze(snapshot)
+			})
+			.catch(() => Object.freeze([]))
 		this.globalStoragePath = provider.context.globalStorageUri.fsPath


If getState() rejects, the snapshot silently falls back to [], disabling all hooks with no log. Should there be a console.error here so operators know hooks are inactive?

Suggested change

this.globalStoragePath = provider.context.globalStorageUri.fsPath

.catch((err) => {

console.error("[hooks] Failed to load hook definitions; hooks disabled:", err)

return Object.freeze([])

})

edelauna · 2026-08-08T12:16:15Z

+	.min(1)
+	.max(256)
+	.refine((value) => !value.includes("\0"), "Tool name must not contain NUL characters")
+export type HookToolName = z.infer<typeof hookToolNameSchema>


The .refine() predicate here duplicates the nulFreeString factory defined a few lines above (line 11 uses the same pattern). Could this use nulFreeString(256, "Tool name") directly?

edelauna · 2026-08-08T12:16:15Z

+				context: [],
+				status: "cancelled",
+				reason: "The task was cancelled.",
+			})


Is there a test for the taskLifetimeAbortController.signal.aborted branch inside runPreToolUseHooks? The instance-replacement path is covered, but the abort-signal cancellation path doesn't appear to have its own case.

edelauna · 2026-08-08T12:21:56Z

edelauna · 2026-08-08T12:23:20Z

+
+## Maintaining this file
+
+Keep this file for knowledge useful to almost every future agent session in this project.


is this useful to include? This feels rather generic and pretty standard, and possibly communicated throughout the repo already.

taltas added 2 commits August 5, 2026 01:43

feat(core): gate tool use with hooks

61e658d

test(core): tighten hook test types

117759e

taltas requested review from JamesRobert20, edelauna, hannesrudolph and navedmerchant as code owners August 5, 2026 06:44

This was referenced Aug 5, 2026

feat(core): run session start hooks #1156

Open

feat(settings): add global hooks panel #1154

Open

feat(types): add hook contracts and policies #1153

Open

taltas added 5 commits August 5, 2026 03:08

merge: propagate hook stack CI fixes

da0d545

# Conflicts: # src/core/task/__tests__/Task.persistence.spec.ts # webview-ui/src/components/settings/__tests__/HooksSettings.spec.tsx

merge: propagate Windows hook runner fix

e484eea

merge: propagate taskkill path fix

72fb109

merge: propagate Windows coverage

dffdf73

merge: propagate taskkill fallback cleanup

2e98360

github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Aug 5, 2026

edelauna requested changes Aug 8, 2026

View reviewed changes

github-actions Bot added awaiting-author PR is waiting for the author to address requested changes and removed awaiting-review PR changes are ready and waiting for maintainer re-review labels Aug 8, 2026

Navigation Menu

Uh oh!

feat(core): gate tool use with hooks - #1155

feat(core): gate tool use with hooks#1155
taltas wants to merge 7 commits into
fm/zoo-hooks-session-startfrom
fm/zoo-hooks-pre-tool

taltas commented Aug 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 5, 2026 •

edited

Loading

Review skipped

Uh oh!

codecov Bot commented Aug 5, 2026

Uh oh!

edelauna left a comment

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

edelauna Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		this.globalStoragePath = provider.context.globalStorageUri.fsPath
+			.catch((err) => {
+				console.error("[hooks] Failed to load hook definitions; hooks disabled:", err)
+				return Object.freeze([])
+			})


		## Maintaining this file

		Keep this file for knowledge useful to almost every future agent session in this project.

Uh oh!

Conversation

taltas commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack

Summary

Scope

Tool Gate

Tests

Risks

Review Notes

Uh oh!

coderabbitai Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

codecov Bot commented Aug 5, 2026

Codecov Report

Uh oh!

edelauna left a comment

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

edelauna Aug 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

taltas commented Aug 5, 2026 •

edited

Loading

coderabbitai Bot commented Aug 5, 2026 •

edited

Loading