fix(cmux-rebuild): resolve code review findings for PR #114 - #115
Open
DJRHails wants to merge 1 commit into
Open
fix(cmux-rebuild): resolve code review findings for PR #114#115DJRHails wants to merge 1 commit into
DJRHails wants to merge 1 commit into
Conversation
Two P3 findings from the post-merge review of the SKILL.md rewrite:
- F1: the pane-title cache snippet referenced $CACHE without defining it
(the rewrite dropped the assignment line). Restored the macOS assignment
inside the block with the linux path as a comment, and dropped the now-
redundant paths from the prose.
- F2: restored the enabling config for zellij session-serialization
('session_serialization true' + viewport in config.kdl) — without it no
EXITED skeletons exist after a reboot and the doc gave no debugging handle.
P4 noted, not restored: the picker Esc-fallback binding gap (covered by the
general '--bind backfills' line).
Verified: prek hooks pass (gitleaks, trufflehog, glassine, test suites).
Owner
Author
Review SummaryDirect review (docs-only follow-up delta, +6/−5 in one file) — no agent fan-out. FindingsNo P1–P3 findings. Both changes verified against the pre-rewrite text (
Verification
CommitNo fix commit needed — the PR is correct as submitted (head Verdict: approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the post-merge review of #114 (the SKILL.md rewrite). Fixes the two P3 findings posted inline there:
$CACHEundefined: the pane-title cache snippet referenced$CACHEbut the rewrite dropped the assignment line the original had. The assignment is back inside the code block (macOS value, linux path as a comment), and the now-redundant path listing is out of the prose.(session_serialization true + viewport in config.kdl)to the macOS-reboot bullet — it is the config that makes EXITED skeletons exist at all, so dropping it left the "no skeletons after reboot" failure mode undebuggable from this doc.P4 noted but not restored (respecting the rewrite's compression intent): the picker Esc-fallback binding gap — recovery is already covered by the "
--bindbackfills" line.Verified: both pi facts in the rewrite were checked against pi 0.84.2 source (renames append
session_infoentries carryingname; latest wins) and the linux cache path against zellij'sconsts.rs— both correct as written. prek hooks pass (gitleaks, trufflehog, glassine checks, all local test suites).via gantry