docs(clave): document the PATH leak that broke v0.1.1 daily driving (#43, #44) - #46
Conversation
, #44) The justfile asserted "Stable sessions never run this binary" — an invariant the code does not enforce. clave-bar shells out to bare `clave` (PATH-resolved) for snapshot/open/bind/focus/touch/prune-tabs, so a working-tree `cargo install` hijacks a live fleet: a version-skewed `clave open` composes tab layouts pointing at the wrong wasm, zellij keys plugin identity on location, and the tab loads a SECOND bar with no shared beacon state — duplicate sidebar, dead navigation. Hit in prod today after an otherwise-correct v0.1.1 release. CONTRIBUTING gains the mechanism, the hard rule until #44 lands, the restore one-liner, and the one-grep diagnosis (every 'clave-bar: loaded' line must report the same version). README notes it for source builders. The dev-install recipe carries a DANGER block instead of the false claim. Also tracks this session's status handoff (#22 ruling), including the incident write-up. Claude-Session: https://claude.ai/code/session_019MM6DirpYLjscK1jUFbwkR
📝 WalkthroughWalkthroughThe changes add documentation about ChangesPATH resolution guidance
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 76-82: Update the diagnosis command in the “Diagnosis is one grep”
section to use ${TMPDIR:-/tmp} when resolving the temporary directory, and
restrict the log search to the current Zellij session/log rather than globbing
across all historical zellij-* directories. Preserve the existing
version-filtering and tail behavior after selecting that session-specific log.
- Around line 67-74: The stable-binary restoration guidance must identify which
version to restore instead of using the literal vX.Y.Z placeholder. In
CONTRIBUTING.md lines 67-74, explain how to determine the active stable
session’s version and require substituting it in the cp command; mirror the same
actionable lookup and substitution guidance in justfile lines 37-43.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07f41ca5-1b4f-4051-a270-af5d3a84c380
⛔ Files ignored due to path filters (1)
docs/status/2026-07-22-1606-clave-orchestrator.mdis excluded by!docs/status/**
📒 Files selected for processing (3)
CONTRIBUTING.mdREADME.mdjustfile
| **Until #44 lands, treat this as a hard rule: never `cargo install` or | ||
| `just dev-install` while a stable session is running** — including from a | ||
| worktree, and including agent-driven builds. When a dev round ends, restore the | ||
| stable binary before daily driving: | ||
|
|
||
| ```sh | ||
| cp ~/.local/share/clave/bin/clave-vX.Y.Z ~/.cargo/bin/clave | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the stable-binary restoration command directly actionable.
Both locations present vX.Y.Z as though the command were exact, but users must know which version the active stable session uses. Require substitution with that version or provide a deterministic lookup command.
CONTRIBUTING.md#L67-L74: update the primary restoration instructions and clarify how to select the active stable binary.justfile#L37-L43: mirror the same executable remediation guidance in the danger warning.
📍 Affects 2 files
CONTRIBUTING.md#L67-L74(this comment)justfile#L37-L43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` around lines 67 - 74, The stable-binary restoration guidance
must identify which version to restore instead of using the literal vX.Y.Z
placeholder. In CONTRIBUTING.md lines 67-74, explain how to determine the active
stable session’s version and require substituting it in the cp command; mirror
the same actionable lookup and substitution guidance in justfile lines 37-43.
| **Diagnosis is one grep**, because the bar logs its version at every load | ||
| (zellij's log lives under the OS temp dir, e.g. | ||
| `$TMPDIR/zellij-$UID/zellij-log/zellij.log` on macOS): | ||
|
|
||
| ```sh | ||
| grep 'clave-bar: loaded' "$TMPDIR"/zellij-*/zellij-log/zellij.log | tail | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the log diagnosis portable and session-specific.
If TMPDIR is unset, this expands to /zellij-*; the broad glob can also combine historical logs from multiple Zellij directories. Use a ${TMPDIR:-/tmp} fallback and target the current session/log before declaring versions mixed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CONTRIBUTING.md` around lines 76 - 82, Update the diagnosis command in the
“Diagnosis is one grep” section to use ${TMPDIR:-/tmp} when resolving the
temporary directory, and restrict the log search to the current Zellij
session/log rather than globbing across all historical zellij-* directories.
Preserve the existing version-filtering and tail behavior after selecting that
session-specific log.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bffe8a383
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `clave-bar` also shells out to the CLI on its own (`snapshot`, `open`, `bind`, | ||
| `focus`, `touch`, `prune-tabs`, `add`), and today it invokes plain **`clave`**, |
There was a problem hiding this comment.
The plugin shellout inventory is inaccurate: clave-bar currently also invokes run_command(&["clave", "collapse", ...]) for Effect::PersistCollapse in crates/clave-bar/src/main.rs:205, while add is generated as the Alt+a keybind Run "{binary}" "add" in crates/clave/src/setup.rs:81 rather than a plugin-internal PATH lookup. If #44 is implemented from this runbook/list, the Alt+c collapse persistence path can remain PATH-resolved and still be served by a skewed ~/.cargo/bin/clave, leaving one of the documented production leaks unfixed.
Useful? React with 👍 / 👎.
1 participant
Refs #43, #44
The justfile asserted "Stable sessions never run this binary — they bake the versioned copy". The code does not enforce that:
clave-barshells out to bareclave(PATH-resolved) forsnapshot/open/bind/focus/touch/prune-tabs/add. So a working-treecargo installcan drive a live stable fleet — which is exactly what broke daily driving after an otherwise-correct v0.1.1 release: a stale 0.1.0 binary servedclave open, composed tab layouts pointing at the old wasm, and since zellij keys plugin identity on file location, each opened tab loaded a second bar. Two populations, no shared beacon state → duplicate sidebar, dead navigation.claveonPATH" subsection under the two-environments table: mechanism, worked example, the hard rule until CRITICAL: clave-bar shells out to bareclavefrom PATH — any stale binary hijacks the running fleet #44, the restore one-liner, and the one-grep diagnosis (everyclave-bar: loadedline must report the same version).dev-install.Alt+t/Alt+w/Alt+↑↓).Docs only; no code paths touched.
just --liststill parses. The structural fix is tracked on #44 (pass the absolute binary path into the plugin at config-generation time + a version-skew refusal), with a proposed KDL guardrail test asserting the generated config's plugin binary equals the versioned artifact.https://claude.ai/code/session_019MM6DirpYLjscK1jUFbwkR
Summary by CodeRabbit