Repository navigation
feat(vibe): add first-class Mistral Vibe support - #1480
Conversation
|
Thanks for the patience on this one - we gave it a full adversarial review including sandboxed installer runs, live gate smoke tests, and a read of current Vibe source. The shape of the contribution is genuinely strong: the origin gating discipline, three-state installer reporting, managed-marker idempotency, and honest fail-open all mirror our house conventions closely. And the answer to our main question is clean: every non-Vibe path is provably untouched - the merged tree passes our full suite 4446/0 and the only shared-line edit is an inert What blocks merge is that the Vibe integration itself has gone stale against your own upstream, plus a few installer defects we reproduced: 1. Vibe upstream moved two days after you filed. Hooks graduated ( 2. The config.toml merge writes the key inside the last TOML table (reproduced: appended after 3. Uninstaller coverage: 4. The mtime gate - we read your side's source and the timing assumption is right (the PLAN profile allowlists writes to PLANS_DIR during planning), but the plans dir is global, so two concurrent sessions can cross-serve plans, and with no freshness window a stale yesterday-plan reviews as if current when a write failed. The Smaller items: the annotate-last index-absent fallback picks the newest session across all projects (each session's meta.json carries cwd - filter on it, matching our Droid/Codex correct-session guarantees); Full details on any of these on request. It merges cleanly into current main and nothing shipped since needs extra wiring, so once the installer/hook shape is current with Vibe 2.25+, this lands fast. Looking forward to it - first-class Vibe support is something we want. |
…install coverage Addresses the backnotprop#1480 review against current Vibe upstream: - Installer: drop the enable_experimental_hooks config.toml leg (hooks are stable in Vibe 2.25+, and the old flag+pre_tool combo was never valid on any single version). Hook command is argv-only with the absolute binary path (unquoted heredoc now interpolates PLANNOTATOR_BIN), so it runs under shell and shell-free executors. Detection requires an existing VIBE home, so a stray 'vibe' binary never creates ~/.vibe. - Gate: detect the Vibe origin from the hook payload (pre_tool + exit_plan_mode), no baked env prefix; PLANNOTATOR_ORIGIN stays as the manual override. Pin the plan by scanning transcript_path for the last write_file/edit targeting the plans dir (kills cross-session cross-serve on the global dir), fall back to newest-by-mtime within a 10-minute freshness window, else fail open. The awk managed-marker comment now tells the truth about a missing end marker. - annotate-last: the index-absent fallback filters candidate sessions by meta.json working_directory instead of picking the newest across all projects. - Uninstaller: strip the managed hooks.toml block by markers and remove the Vibe skill dirs, so plannotator uninstall leaves no hook invoking a deleted binary. - Tests: vibe-plan.test.ts saves/restores VIBE_HOME in beforeEach/afterEach; new resolver and uninstall coverage; install.test.ts asserts the argv-only command and the absence of the config leg. - Docs: PLANNOTATOR_SKIP_VIBE_INSTALL row in the marketing env-vars doc; AGENTS.md row updated for the new hook shape.
|
Thanks for the detailed review. All items are addressed in e7b6d92. Before writing any of it, I checked your upstream claims against a live Vibe 2.25.3 install by reading Upstream drift (items 1 and 2)You were right that the config leg is dead: One correction from my side: the 2.25.3 build I inspected runs hooks through Origin detection now comes from the payload: The mtime gate (item 4)The resolver now pins the plan from annotate-last (smaller item)The index-absent fallback now reads each candidate session's Uninstaller (item 3)
Remaining smaller items
Test plan
|
Register the mistral-vibe origin and wire plan-review gate, annotate-last, and installer surfacing. Origin in AGENT_CONFIG/PromptRuntime/PLAN_TOOL_NAMES; gate resolves newest plan from $VIBE_HOME/plans by mtime and fails open; annotate-last session discovery; installer three-layer skip + managed hook block (sh) / manual instructions (ps1/cmd); apps/vibe artifacts + skills; docs + freshness guard.
…install coverage Addresses the backnotprop#1480 review against current Vibe upstream: - Installer: drop the enable_experimental_hooks config.toml leg (hooks are stable in Vibe 2.25+, and the old flag+pre_tool combo was never valid on any single version). Hook command is argv-only with the absolute binary path (unquoted heredoc now interpolates PLANNOTATOR_BIN), so it runs under shell and shell-free executors. Detection requires an existing VIBE home, so a stray 'vibe' binary never creates ~/.vibe. - Gate: detect the Vibe origin from the hook payload (pre_tool + exit_plan_mode), no baked env prefix; PLANNOTATOR_ORIGIN stays as the manual override. Pin the plan by scanning transcript_path for the last write_file/edit targeting the plans dir (kills cross-session cross-serve on the global dir), fall back to newest-by-mtime within a 10-minute freshness window, else fail open. The awk managed-marker comment now tells the truth about a missing end marker. - annotate-last: the index-absent fallback filters candidate sessions by meta.json working_directory instead of picking the newest across all projects. - Uninstaller: strip the managed hooks.toml block by markers and remove the Vibe skill dirs, so plannotator uninstall leaves no hook invoking a deleted binary. - Tests: vibe-plan.test.ts saves/restores VIBE_HOME in beforeEach/afterEach; new resolver and uninstall coverage; install.test.ts asserts the argv-only command and the absence of the config leg. - Docs: PLANNOTATOR_SKIP_VIBE_INSTALL row in the marketing env-vars doc; AGENTS.md row updated for the new hook shape.
1b58b16 to
8da0a64
Compare
|
Hey @backnotprop , I just fixed some conflicts. I'd appreciate a review on the recent changes when you get a chance, thank you 🙏 |
cleanupVibeHooks rewrote hooks.toml even under --dry-run and never reported the change. It now records the planned rewrite on dry-run, reports the removal on a real run, and keeps the file's CRLF line endings. The installer's hooks.toml.bak is left alone: it is a copy of the user's pre-install file, not something Plannotator owns.
…e skills install.ps1 and install.cmd counted a vibe binary on PATH as Vibe detected and then created the Vibe skills directory. Detection now requires the Vibe home to exist, matching install.sh. The apps/vibe skills run PLANNOTATOR_ORIGIN=mistral-vibe plannotator ..., a POSIX env-prefix. On Windows, Vibe's shell tool runs it through Git Bash when it can find one and falls back to PowerShell otherwise, where that syntax fails. The Windows installers therefore no longer copy the Vibe skills and write nothing under the Vibe home; Vibe still loads the shell-neutral core skills from ~/.agents/skills.
…s/vibe to project structure
… the transcript isInsidePlansDir compared dirname strings exactly, so a symlinked VIBE_HOME (or a symlinked segment in the transcript path) or a non-normalized path such as plans//x.md missed the pin and fell back to newest-by-mtime. Both sides are now resolved and realpath'd, falling back to the resolved path when realpath fails.
… yet The Windows notes claimed every shared command works for Vibe through ~/.agents/skills. Review and annotate do, without a Vibe origin label, but /plannotator-last without PLANNOTATOR_ORIGIN takes the Claude Code transcript path, so it fails or could read a Claude session in the same project.
|
Merged, thank you @Djiit, and thanks for your patience through two rounds of review. Before merging we added a few follow-ups on top of your commits: |
TL;DR
Makes Mistral Vibe a detected origin in Plannotator. The plan-review gate, annotate-last, and installer wiring that were hand-configured now happen automatically. Vibe's
exit_plan_modetool sends no plan in its payload, so the gate reads the newest plan from$VIBE_HOME/plansby mtime.What changed
Origin registration
mistral-vibeadded toAGENT_CONFIG(rose badge), thePromptRuntimeunion, andPLAN_TOOL_NAMES(mapped to Vibe's lowercaseexit_plan_mode). No Ask AI provider — Vibe falls back to the saved/server default, same asamp/droid/copilot-cli/gemini-cli.Plan-review gate (
apps/hook/server/index.ts,packages/shared/vibe-plans.ts)resolveLatestVibePlanreads the newest*.mdin$VIBE_HOME/plansby mtime. Vibe'sexit_plan_modetakes no args, so the plan isn't in the hook payload.{"decision":"allow"}on stdout, exit 0. Deny →{"decision":"deny","reason":...}. No plan found → empty stdout + exit 0 soexit_plan_modeproceeds and the user isn't locked out of plan mode. Hook block usesstrict: false(Vibe's default), so a crash is fail-open too.improve-contextanalog — Vibe has noEnterPlanModemoment. Documented in the install banner, not wired.annotate-last (
apps/hook/server/session-log.ts)resolveVibeSessionLogForCwdwalks$VIBE_HOME/logs/session/.session_index.jsonfor the session whosecwdmatches, newestmtime_nswins. The index is authoritative: a present-but-no-match returns null rather than falling through to a cwd-blind mtime scan. Ancestor-walk fallback only when the index is absent.getLastVibeRenderedMessageparsesmessages.jsonlbackward for the last assistant turn with non-empty string content (skips reasoning-only and tool-call-only turns).Installer (
install.sh/install.ps1/install.cmd)--skip-vibeflag >PLANNOTATOR_SKIP_VIBE_INSTALLenv var >skipInstall.vibeconfig key. Each skip remembers its source for honest reporting.VIBE_HOMEdetected viacommand -v vibeor~/.vibeexisting, honors aVIBE_HOMEoverride (mirrorsCODEX_HOME).hooks.toml(# >>> plannotator-managed-vibe-hooks >>>…<<<) so it coexists with existing hooks;enable_experimental_hooks = truemerged intoconfig.toml. Atomic rewrite with a.bak./bin/sh; Windows usescmd.exeand a.shlauncher won't run). These installers print manual setup instructions instead of writinghooks.toml— same pattern as Codex-on-Windows. Skills install cross-platform on all three.plannotator-review/-annotate/-last(origin baked in, fromapps/vibe/skills) + the single-sourcedplannotatorknowledge skill fromapps/skills/core. Stale-skill cleanup and three-state "VIBE USERS" banners in each script.Repo artifacts (
apps/vibe/)hooks/hooks.toml(managed block template),hooks/config-snippet.toml, three Vibe SKILL.md files. Addedapps/vibeto the installer sparse-checkout.Docs + freshness guard
AGENTS.mdenv table:mistral-vibeinPLANNOTATOR_ORIGINvalues, newPLANNOTATOR_SKIP_VIBE_INSTALLrow.packages/server/index.tssupported-values comment.apps/skills/core/plannotator/SKILL.mdPLANNOTATOR_ORIGINrow. Marketing env-vars doc (also fixed a pre-existing missingoh-my-pi).Out of scope
packages/ai/. Vibe falls back to the saved/server default. Adding a Mistral provider is a separate, larger effort.plannotator), soapps/pi-extension/server/doesn't need a Vibe branch for the gate. annotate-last discovery lives Bun-side; vendoring to Pi only matters if Pi gains a Vibe annotate-last path.cmd.exe; a.shlauncher isn't executable). Windows installers print manual instructions.Test plan
bun test scripts/install.test.ts— 124 pass (updated hardcoded assertions + new Vibe detection/integration test mirroring the Kiro block)bun test apps/hook/server/plannotator-skill-reference.test.ts— freshness guard, 6 passbun test apps/hook/server/session-log.test.ts— 98 pass (includes Vibe session discovery, cwd-filter fix)bun test apps/hook/server/vibe-plan.test.ts apps/hook/server/vibe-session.test.ts— 16 pass (resolver newest-by-mtime, empty-dir fail-open, gate allow/deny)bun run --cwd apps/review build && bun run build:hook— both succeed~/.vibe/plans/, pipe a Vibepre_tool/exit_plan_modeJSON toPLANNOTATOR_ORIGIN=mistral-vibe plannotator— assert allow/deny on stdoutPLANNOTATOR_ORIGIN=mistral-vibe plannotator lastopens the last assistant message frommessages.jsonl