fix keychain - #1479
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis pull request introduces a new Fish tmux session bootstrapping system that replaces tmuxinator-based management, adds shell aliases for clipboard operations, updates the Makefile's host resolution logic for Darwin/NixOS builds, and bumps multiple npm dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Fish as Fish Function
participant Bootstrap as __tmux_bootstrap_<br/>default_session
participant Tmux
participant Session as Tmux Session
User->>Fish: Call _tpo_function (primary)
alt primary session exists
Fish->>Tmux: has-session -t primary
Tmux-->>Fish: success
else primary session missing
Fish->>Bootstrap: __tmux_bootstrap_default_session primary
Bootstrap->>Tmux: new-session -d -s primary -n btop
Tmux->>Session: Create detached primary session
Session-->>Tmux: success
Bootstrap->>Tmux: new-window -t primary:1 -n dotfiles
Tmux->>Session: Add dotfiles window
Bootstrap->>Tmux: select-layout -t primary:1 even-horizontal
Bootstrap-->>Fish: Bootstrap complete
end
alt Inside tmux ($TMUX set)
Fish->>Tmux: switch-client -t primary
else Outside tmux
Fish->>Tmux: attach-session -t primary
end
Tmux-->>User: Session ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Mesa DescriptionTL;DRFixed an issue related to the keychain and included several chore updates. What changed?Specific file changes are not available, but the PR addresses a keychain fix and contains multiple chore updates. Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors tmux session management by replacing tmuxinator with a custom Fish bootstrap function and updates the Makefile to improve host resolution during Nix builds on Darwin. It also standardizes copy and paste aliases across shell environments and bumps several package dependencies. Feedback was provided regarding _two_function.fish, where the tmux-resurrect restore script is detected before the tmux server is active, which may cause the restoration process to be skipped.
| set -l restore (tmux list-keys 2>/dev/null | string match -rg '(/\S+/resurrect/scripts/restore\.sh)') | ||
| set -l restore $restore[1] | ||
| set -l bootstrapped 0 | ||
|
|
||
| # Start a server if needed so we can invoke the restore script. | ||
| if not tmux list-sessions 2>/dev/null | grep -q . | ||
| # No server running — start one; continuum will auto-restore work session | ||
| tmux new-session -d -s _bootstrap | ||
| # Give continuum a moment to restore | ||
| sleep 3 | ||
| if tmux has-session -t work 2>/dev/null | ||
| tmux kill-session -t _bootstrap 2>/dev/null | ||
| if test -n "$TMUX" | ||
| tmux switch-client -t work | ||
| else | ||
| tmux attach-session -t work | ||
| set bootstrapped 1 | ||
| end | ||
|
|
||
| if test -n "$restore" |
There was a problem hiding this comment.
The detection of the tmux-resurrect restore script happens before ensuring the tmux server is running. If the server is not already active, tmux list-keys will fail, and the restore variable will be empty. Consequently, the subsequent attempt to restore the session (lines 21-38) will be skipped even if a restore script is configured. The detection should be moved after the bootstrap session is created to ensure the server is reachable.
set -l bootstrapped 0
# Start a server if needed so we can invoke the restore script.
if not tmux list-sessions 2>/dev/null | grep -q .
tmux new-session -d -s _bootstrap
set bootstrapped 1
end
set -l restore (tmux list-keys 2>/dev/null | string match -rg '(/\S+/resurrect/scripts/restore\.sh)')
set -l restore $restore[1]
if test -n "$restore"
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 20 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="spec/make_build_host_resolution_spec.sh">
<violation number="1" location="spec/make_build_host_resolution_spec.sh:18">
P2: This test can report success even when `make build` fails because `;` makes the shell return the status of `cat` instead of `make`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| After 'cleanup' | ||
|
|
||
| It 'uses an explicit named NixOS host on Darwin builds' | ||
| When run bash -c 'make build HOST=matic OS=Darwin ARCH=arm64 DETECTED_HOST=galactica NIX_EXEC=nix NIX_ENV=ok NIX_FLAGS= NIX_USER_TRUSTED=yes 2>/dev/null; cat "$MOCK_LOG"' |
There was a problem hiding this comment.
P2: This test can report success even when make build fails because ; makes the shell return the status of cat instead of make.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/make_build_host_resolution_spec.sh, line 18:
<comment>This test can report success even when `make build` fails because `;` makes the shell return the status of `cat` instead of `make`.</comment>
<file context>
@@ -0,0 +1,31 @@
+After 'cleanup'
+
+It 'uses an explicit named NixOS host on Darwin builds'
+When run bash -c 'make build HOST=matic OS=Darwin ARCH=arm64 DETECTED_HOST=galactica NIX_EXEC=nix NIX_ENV=ok NIX_FLAGS= NIX_USER_TRUSTED=yes 2>/dev/null; cat "$MOCK_LOG"'
+The status should be success
+The output should include '.#nixosConfigurations.matic.config.system.build.toplevel'
</file context>
* fix: keychain * chore: update * chore: update * chore: update * chore: update * chore: update * chore: update
fishtape wraps the remaining arguments of @test with `test`, so @test "unknown session returns failure" test \$unknown_status -ne 0 expanded to `test test 1 -ne 0` (5 args) which fish's test rejects, causing shell-test to fail since PR #1479 introduced this spec. Drop the literal `test` so fishtape produces `test 1 -ne 0`.
1 participant
Summary by cubic
Replaced
tmuxinatorwith a built-in tmux bootstrap for default sessions and improvedmake buildhost resolution on macOS. Added clipboard aliases across shells, expanded tests, and refreshed dependencies.New Features
__tmux_bootstrap_default_sessionand wired it into_tpo,_tmo,_tdo,_two, and_tss, with a tmux-resurrect restore path; included tests for bootstrap and helpers.MakefileDarwin builds now useHOSTto select named NixOS/Darwin configs and fall back toDETECTED_HOST; added a spec for host resolution.copy/pastealiases to bash, fish, and zsh.Dependencies
package.jsonand refreshedbun.lock.Written for commit 6a06968. Summary will update on new commits.