fix: remove tmux-claude-teams plugin - #1330
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedFailed to post review comments 📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant Timer as Daily Timer<br/>(04:00)
participant Launcher as launchd/systemd<br/>Scheduler
participant Indexer as index.sh
participant Cass as cass CLI
Timer->>Launcher: Trigger scheduled event
Launcher->>Indexer: Execute index.sh
Indexer->>Indexer: Validate cass binary<br/>at ~/.local/bin/cass
Indexer->>Cass: cass sources sync
Cass-->>Indexer: (tolerates failures)
Indexer->>Cass: cass index --full<br/>--force-rebuild
Cass-->>Indexer: Rebuild complete
Indexer->>Cass: cass index --semantic<br/>--embedder fastembed
Cass-->>Indexer: (tolerates failures)
Indexer->>Cass: cass analytics rebuild
Cass-->>Indexer: Rollup tables regenerated
Indexer-->>Launcher: Complete with logs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
Mesa DescriptionTL;DRIntroduced cross-platform What changed?
Description generated by Mesa. Update settings |
- Use --force-rebuild to work around upstream bug where --full skips new session files (github.com/Dicklesworthstone/coding_agent_session_search/issues/153) - Add semantic vector index build with fastembed embedder - Tolerate OOM on semantic index (low-memory machines)
4b565dc to
2805de3
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new cass background service (daemon + scheduled indexer) and adds ShellSpec coverage for the new indexer script, while also adjusting cliproxyapi’s Claude keychain sync behavior to update last_refresh even when the access token is unchanged.
Changes:
- Add a
cassHome Manager service module (launchd + systemd units) plus anindex.shscript to run sync/index/analytics tasks on a schedule. - Extend ShellSpec coverage enforcement and add a new spec for the
cassindexer script. - Update
keychain-sync.shto bumplast_refreshwhen the token is unchanged; expand cass source paths insources.toml.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/coverage_spec.sh | Enforces presence of a spec for the new cass indexer script; adds cass index script to the “covered scripts” list. |
| spec/cass_indexer_spec.sh | New ShellSpec tests asserting key properties/commands exist in cass/index.sh. |
| home-manager/services/default.nix | Registers the new cass service module in the services list. |
| home-manager/services/cliproxyapi/scripts/keychain-sync.sh | Bumps Claude last_refresh even when the access token hasn’t changed. |
| home-manager/services/cass/index.sh | New bash script to run cass source sync, full index, semantic index (best-effort), and analytics rebuild. |
| home-manager/services/cass/default.nix | New launchd + systemd user units for a cass daemon and daily indexer timer. |
| home-manager/programs/cass/sources.toml | Adds multiple paths to index from a remote SSH source; switches sync schedule to manual. |
Comments suppressed due to low confidence (1)
home-manager/services/cass/index.sh:8
- The “cass binary not found” message is currently printed to stdout. Since this is an error path, it should go to stderr (and ideally include a brief hint on how to install/enable cass) so service logs and callers can distinguish failures from normal output.
if [ ! -x "$CASS" ]; then
echo "cass binary not found at $CASS"
exit 1
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request introduces the 'cass' service, including its configuration, a shell script for indexing, and associated systemd/launchd service definitions. It also updates the 'keychain-sync.sh' script to prevent unnecessary token refreshes by updating the 'last_refresh' timestamp when tokens remain unchanged. Feedback was provided regarding the use of hardcoded '/tmp' paths for logging in the 'cass' service, which may cause permission issues or log loss, and a suggestion was made to apply the 'last_refresh' update logic consistently to the 'sync_codex' function.
I am having trouble creating individual review comments. Click here to see my feedback.
home-manager/services/cass/default.nix (18-19)
Using /tmp for log files in a persistent service is problematic because logs are lost on reboot, making it difficult to debug issues that occur across restarts. Additionally, in a multi-user environment, hardcoded paths in /tmp can lead to permission conflicts if multiple users attempt to run the same service. Consider removing these lines to let launchd handle logging via the system log, or use a user-specific directory if you have access to the home directory variable.
home-manager/services/cass/default.nix (37-38)
Similar to the daemon configuration, using /tmp for the indexer logs is not ideal for persistence or multi-user safety. It is recommended to either omit these to use system logging or use a more appropriate, user-specific log location.
home-manager/services/cliproxyapi/scripts/keychain-sync.sh (90-96)
The addition of this elif block to update last_refresh when the token is unchanged is a good fix to prevent cliproxyapi from triggering its own refresh. However, for consistency, this same logic should be applied to the sync_codex function (which starts at line 100), as it likely suffers from the same issue where cliproxyapi might rotate tokens that are still valid in the local configuration.
Summary
run-shellfor tmux-claude-teams from tmux.confClaude Code has native tmux teammate support via
CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1+teammateMode: tmuxin settings. No plugin needed.Summary by cubic
Remove
tmux-claude-teamshooks and harden thecassdaily index job by forcing full rebuilds and adding a semantic index build.tmux-claude-teamsrun-shell from tmux and related hooks from settings; use Claude Code native teammate mode (CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1,teammateMode: tmux).cassindex script: run--full --force-rebuildto pick up new session files, then build the semantic vector index withfastembedand tolerate OOM (|| true).Written for commit 2805de3. Summary will update on new commits.