Skip to content

fix(vibe): follow the v2.21.0 hook rename and three further upstream drifts - #2441

Merged
dyoshikawa merged 4 commits into
mainfrom
resolve-scrap-issue-2423-vibe-hook-types
Jul 27, 2026
Merged

fix(vibe): follow the v2.21.0 hook rename and three further upstream drifts#2441
dyoshikawa merged 4 commits into
mainfrom
resolve-scrap-issue-2423-vibe-hook-types

Conversation

@dyoshikawa

@dyoshikawa dyoshikawa commented Jul 27, 2026

Copy link
Copy Markdown
Owner

1. Every emitted hook was rejected (breaking, v2.21.0)

Vibe v2.21.0 graduated hooks from experimental and renamed every hook type:

Every hook type is renamed: post_agent_turn → post_agent, before_tool → pre_tool, after_tool → post_tool

HookType is a strict enum and HookConfig.model_validate turns a failure into a HookConfigIssue, so every .vibe/hooks.toml entry rulesync wrote was dropped at load with a TUI warning. The mapping and the tool-event gate are updated to the current names.

2. enable_experimental_hooks was removed

Same release, "Removed": the flag and its VIBE_ENABLE_EXPERIMENTAL_HOOKS env var — "No flag is required — declaring a hook is enough." The key is gone from VibeConfigSchema.

Rulesync wrote it into .vibe/config.toml as an auxiliary file on every hooks generation. That whole path is removed: the VibeConfigToml class, VibeHooks.getAuxiliaryFiles, getExtraSharedWritePaths, and the hooks entry in the file's SHARED_CONFIG_OWNERSHIP. .vibe/config.toml now has two writers (mcp, permissions) instead of three, which the shared-write derivation snapshot records.

3. Three permission tool names were wrong

Vibe's builtin names come from BaseTool.get_name() — the snake_case of each tool class:

canonical was now
webfetch fetch web_fetch
websearch search_web web_search
edit write_file edit
write write_file write_file

fetch and search_web do not exist upstream, and BaseToolConfig allows extra keys, so those blocks were silently inert rather than erroring. Worse, edit and write both collapsed onto write_file: the real edit tool never received a permission or allow/deny list, while write_file — create-only since v2.14.0 — inherited edit rules.

4. Four per-server MCP keys were dropped and clobbered

prompt, sampling_enabled, disabled and disabled_tools are on _MCPBase, and mcp_settings.py persists disabled / disabled_tools into config.toml when a user toggles a server or a single tool from the /mcp panel. Rulesync's field allowlist omitted all four, and mcp_servers is replaced as a whole array on each generate — so a user's /mcp toggle on a rulesync-managed server was wiped on the next rulesync generate. They are passthrough fields now.

5. The shared global skills root was not discovered

Vibe's user_skills_dirs returns both ~/.vibe/skills and ~/.agents/skills, and the README lists both under "Global skills directories". Rulesync registered .agents/skills as an alternative root for project scope only, so rulesync import --global never saw it. It now applies at either scope.

Not included

The issue's sixth item — commands, which Vibe expresses as skills carrying user-invocable: true — is a feature addition rather than a drift from a shipped release, and routing commands onto the skills surface is a design decision (the shape devin uses). Left on the issue.

Tests

The existing Vibe hook/permission suites are updated to the corrected names, which is where the behaviour change shows: disabled_tools for a denied edit category is now ["edit"] rather than ["write_file"], and the test that previously asserted "edit and write merge onto write_file" now asserts they stay on their own tools — that assertion was pinning the bug. The e2e hooks cases assert the new type strings and that no .vibe/config.toml is written for hooks at either scope.

pnpm cicheck passes.

Refs #2423

cm-dyoshikawa and others added 4 commits July 27, 2026 09:53
@cm-dyoshikawa @claude
fix(vibe): follow the v2.21.0 hook rename and three further upstream …
4877091 
…drifts

Vibe v2.21.0 graduated hooks from experimental and renamed every hook `type`.
`HookType` is a strict enum, so **every** `.vibe/hooks.toml` entry rulesync
emitted was rejected and dropped with a `HookConfigIssue`: `before_tool` →
`pre_tool`, `after_tool` → `post_tool`, `post_agent_turn` → `post_agent`.

The same release removed the `enable_experimental_hooks` flag and its env var —
"declaring a hook is enough" — so the auxiliary `.vibe/config.toml` write is
gone, along with the `VibeConfigToml` file class and the hooks entry in that
file's shared-config ownership. `.vibe/config.toml` is now written by the MCP
and permissions features only.

Three drifts found in the same pass:

- **Permission tool names were wrong for three categories.** Vibe's builtin
  names are the snake_case of each tool class, so `webfetch` → `web_fetch` and
  `websearch` → `web_search`; `fetch` and `search_web` do not exist, and
  `BaseToolConfig` allows extra keys, so those blocks were silently inert.
  `edit` and `write` both collapsed onto `write_file`, which meant the real
  `edit` tool never received a rule while `write_file` (create-only since
  v2.14.0) inherited edit rules. They are now distinct.
- **Four per-server MCP keys were dropped.** `prompt`, `sampling_enabled`,
  `disabled` and `disabled_tools` are what Vibe's `/mcp` panel writes back when
  a user toggles a server or one of its tools. Since `mcp_servers` is replaced
  as a whole array on each generate, leaving them out of the passthrough wiped
  the user's toggle on the next run.
- **The shared `~/.agents/skills` root was not discovered in global mode.**
  Vibe's `user_skills_dirs` returns both `~/.vibe/skills` and `~/.agents/skills`,
  so the alternative root now applies at either scope rather than project only.

The issue's sixth item — commands, which Vibe expresses as skills with
`user-invocable: true` — is a feature addition rather than a drift and is left
for a separate decision.

Refs #2423

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cm-dyoshikawa @claude
fix(vibe): repair the e2e permissions cases and make the MCP toggles …
bae3c4c 
…actually survive

Review feedback.

HIGH — three e2e permission cases (plus one in the MCP suite) still asserted the
old `write_file` mapping, so the E2E job failed on all three runners. `pnpm
cicheck` does not cover e2e, which is why the first push looked clean. Fixed.

MID — the four MCP keys were only added to the field allowlist, which reads the
rulesync source, not the file on disk. The clobbering the issue describes
therefore still happened: `mcp_servers` is rewritten as a whole array, so a
`/mcp` toggle on a rulesync-managed server disappeared on the next generate
regardless. A server rulesync writes is now seeded from the on-disk entry of the
same name for exactly those keys, with the rulesync source winning when it
states a value.

MID — `disabled_tools` maps to the canonical `disabledTools` in both directions
rather than passing through raw, the way `codexcli` already renames it.
Otherwise a canonical `disabledTools` was ignored for Vibe, and importing from
Vibe wrote a snake_case key into `.rulesync/mcp.json` that no other tool reads.

MID — the shared `.agents/skills` root is registered through
`importOnlySkillRoots`, matching `kimi-code`. `alternativeSkillRoots` is scanned
for deletion as well as import, so extending it to global scope would have made
every skill under the user's `~/.agents/skills` an orphan-deletion candidate for
`--targets vibe --delete` — and the docs sentence added in the previous commit
promised the opposite.

MID — the permissions docs now carry a migration note: rulesync only rewrites
the names it emits today, so `write_file` entries derived from the old `edit`
mapping, and inert `[tools.fetch]` / `[tools.search_web]` blocks, need removing
by hand.

LOW — dropped the comment left behind by the deleted `getExtraSharedWritePaths`,
renamed the e2e case that no longer tests an experimental flag, and corrected
the skills-processor comment about Vibe's user-scope roots.

Tests: the four MCP keys now have coverage — on-disk preservation, the rulesync
source overriding a toggle, and the `disabledTools` round trip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cm-dyoshikawa @claude
fix(vibe): let disabledTools actually reach the MCP adapter
2e3969e 
Review round 2.

HIGH — the `disabledTools` mapping added in the previous commit never fired on
generate. The MCP factory declared `supportsDisabledTools: false` for `vibe`, and
`convertRulesyncFilesToToolFiles` strips the field before calling the adapter,
so the code was unreachable. Import, meanwhile, emits it — which is worse than a
no-op: importing from Vibe wrote `disabledTools` into `.rulesync/mcp.json`, and
regenerating on a machine with no `.vibe/config.toml` to seed from dropped it,
re-enabling the MCP tools the user had disabled. Upstream `_MCPBase.disabled_tools`
is exactly the canonical field, so the flag is now true; Vibe has no per-server
`enabled_tools`, so that one stays false. The supported-tools matrix picks the
change up automatically.

The new test could not have caught this: it drove the adapter directly, past the
strip. An e2e case now goes through the processor in both directions.

LOW — a malformed `.vibe/config.toml` reported smol-toml's bare message, which
names no file; under `--targets "*"` that leaves the user guessing which tool's
config is broken. Wrapped the way the shared-config gateway wraps its own.

LOW — the MCP doc's upstream reference pointed at `_settings.py`, which 404s;
those models live in `config/models.py` now.

Not changed: canonical `agent` still emits `[tools.agent]` where Vibe's subagent
tool is `task`, so an `agent` deny is silently inert. It predates this PR and is
outside the v2.21.0 drift this issue tracks — worth its own issue rather than a
quiet fix here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cm-dyoshikawa @claude
fix(vibe): map the agent category to Vibe's task tool
ee8e8d6 
Review round 3 — no mid-or-above findings; these are the lows.

Deferring `agent` → `task` did not hold up: the same PR fixes `fetch` and
`search_web`, which are equally unrelated to the v2.21.0 drift and fail the same
way — a name Vibe does not have, silently ignored. Vibe's subagent tool is
`task`, and OpenCode already carries exactly this rename. An `agent` deny was
inert, so subagent spawning stayed enabled after the user forbade it. Mapped in
both directions, with a round-trip test.

Also: the existing-config parse goes through `parseSharedConfig` rather than a
hand-copied error message, so the wording and sanitization stay in one place; a
unit test pins the `vibe` strip flags, which is what the previous round's HIGH
turned on and what only an e2e run could catch before; and the new e2e case uses
the file's own `toTable` / `toTableArray` helpers like its neighbours.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dyoshikawa
dyoshikawa merged commit 0994178 into main Jul 27, 2026
9 checks passed
@dyoshikawa
dyoshikawa deleted the resolve-scrap-issue-2423-vibe-hook-types branch July 27, 2026 20:31
@dyoshikawa

dyoshikawa commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

@dyoshikawa Thank you!

This was referenced Jul 27, 2026
Open
Closed
Closed
Merged
@dyoshikawa dyoshikawa mentioned this pull request Aug 6, 2026
Closed
rudironsoni pushed a commit to rudironsoni/rulesync that referenced this pull request Aug 8, 2026
@cm-dyoshikawa @claude
fix(vibe): skip canonical categories with no Vibe tool instead of emi…
7fc9043 
…tting inert tables

toVibeToolName fell back to the canonical category name verbatim, so
categories with no Vibe counterpart (glob, notebookedit, ...) emitted
[tools.<category>] tables for tools Vibe does not have - a deny authored
there looked applied while being silently inert, the dangerous
direction. Unlike agent -> task (dyoshikawa#2441) there is no correct name to
rename to, so the category is now skipped with a warning naming it and
the affected rule count (the grokcli adapter's pattern). The skip also
covers the enabled/disabled cleanup loop and the sensitive_patterns
override so no phantom tool names are touched. Unknown on-disk
[tools.*] tables still round-trip untouched.

Closes dyoshikawa#2485

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dyoshikawa @cm-dyoshikawa