Repository navigation
Fix Vault resume for non-ASCII paths - #4683
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR introduces ChangesNon-ASCII Path Shell Quoting
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (15 passed)
✨ 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 |
Greptile SummaryFixes a mojibaking bug where Vault/restorable-agent resume startup input was built with raw UTF-8 bytes, causing
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to the shell quoting helpers used for resume startup commands, has correct octal encoding, and is covered by new regression tests that run the generated cd commands under zsh. The new TerminalStartupShellQuoting helper correctly handles every byte range: non-ASCII bytes are octal-encoded into an ASCII printf substitution, ASCII safe-chars optionally pass through bare, and everything else is single-quoted with proper embedded-single-quote escaping. The two new tests create real CJK directories, assert ASCII-only output, and verify zsh resolves the reconstructed path — giving strong evidence the fix works end-to-end. No concurrency, logging, or localization concerns are introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Build resume shell command token] --> B{Contains non-ASCII bytes?}
B -- Yes --> C[asciiPrintfCommandSubstitution: Encode all bytes as octal NNN]
C --> D[Emit ASCII-only printf substitution]
B -- No --> E{Safe bare ASCII token?}
E -- Yes --> F[Return token unquoted]
E -- No --> G[Wrap in single quotes, escape embedded quotes]
D --> H[Terminal startup input is ASCII-only]
F --> H
G --> H
H --> I[Shell expands printf substitution and reconstitutes original UTF-8 path]
Reviews (2): Last reviewed commit: "fix: mark terminal startup quoting nonis..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| enum TerminalStartupShellQuoting { | ||
| static func singleQuoted(_ value: String) -> String { | ||
| if value.utf8.contains(where: { $0 >= 0x80 }) { | ||
| return asciiPrintfCommandSubstitution(for: value) | ||
| } | ||
| return "'" + value.replacingOccurrences(of: "'", with: "'\\''") + "'" | ||
| } | ||
|
|
||
| static func shellToken(_ value: String, allowingBareASCII: Bool) -> String { | ||
| if value.utf8.contains(where: { $0 >= 0x80 }) { | ||
| return asciiPrintfCommandSubstitution(for: value) | ||
| } | ||
| if allowingBareASCII, | ||
| value.range(of: "[^A-Za-z0-9_./:=+-]", options: .regularExpression) == nil { | ||
| return value | ||
| } | ||
| return singleQuoted(value) | ||
| } | ||
|
|
||
| private static func asciiPrintfCommandSubstitution(for value: String) -> String { | ||
| let octalBytes = value.utf8 | ||
| .map { String(format: #"\%03o"#, Int($0)) } | ||
| .joined() | ||
| return #""$(printf '"# + octalBytes + #"')""# | ||
| } | ||
| } |
There was a problem hiding this comment.
TerminalStartupShellQuoting is now consumed by both RestorableAgentSession.swift and SessionIndexModels.swift, making it a cross-file dependency with its own clearly-scoped responsibility. RestorableAgentSession.swift is already 1183 lines — well past the 800-line threshold — and placing a shared utility here means its natural home is a filename that doesn't describe it. A dedicated TerminalStartupShellQuoting.swift (or similar) would give the type a discoverable location and keep both consuming files from depending on each other's internals.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Leaving this colocated with the resume-command code for this patch. The helper is only used by the two resume startup command builders, and extracting a new Swift file would add Xcode project wiring churn without changing the bug boundary; a broader shell-quoting cleanup should be separate.
— Claude Code
This branch was successfully deployed
1 participant
Summary
Fixes #4587
Reproduction
cdcommand, mojibaking its UTF-8 bytes before zsh execution, and observingcd: no such file or directory.printf.Test plan
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-4587-vault-resume-cjk --launch.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
Low risk: a small Swift concurrency annotation change that shouldn’t affect quoting behavior, but could subtly impact isolation expectations if misused.
Overview
Updates
TerminalStartupShellQuotinginRestorableAgentSession.swiftto benonisolated, allowing its static shell-quoting utilities to be called from actor-isolated/concurrent code paths (e.g., session resume command building) without Swift concurrency isolation friction.Reviewed by Cursor Bugbot for commit 78cebb2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes Vault/restorable-agent resume failures when the working directory contains non-ASCII characters by emitting ASCII-only startup input and reconstructing UTF-8 paths in the shell. Fixes #4587.
TerminalStartupShellQuotingto output ASCII-only tokens (uses"$(printf '\NNN...')"for non-ASCII, allows safe bare ASCII, otherwise single-quotes).SessionEntryand restorable agent command builders socdworks with CJK paths.zshresolves the correct directory.Written for commit 6b29125. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests