fix(pty): self-heal a reaped shell-integration directory - #57
Conversation
The integration scripts are materialized once into $TMPDIR/noa-shell-integration-<pid> and the path was then cached in a OnceLock and handed out unchecked forever. When the OS temp reaper removes that tree under a running noa, every later pane still gets the stale path — and since ZDOTDIR (zsh) and --rcfile (bash) suppress the shell's normal startup lookup, the shell comes up with no configuration at all, not even the user's own. It reads as "zsh stopped loading and I got sh". Re-verify the scripts on every handout and rewrite them when any went missing, so an unusable directory degrades to "no integration" (user config intact) rather than to a dangling path. Materialization failure is no longer cached either: one transient write error used to disable integration for the rest of the process's life.
One directory accumulates per launch and nothing but the OS temp reaper ever removes them (112 had piled up locally). Sweep the siblings left by earlier processes when this process picks its own directory, touching only trees idle for a week; a still-running process whose directory is swept re-materializes it on its next spawn.
![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: 8af627520e
ℹ️ 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".
| EMBEDDED_SCRIPTS | ||
| .iter() | ||
| .all(|(rel, _)| base.join(rel).is_file()) |
There was a problem hiding this comment.
When a previous process's directory survives until its PID is reused, this check accepts that process's potentially older embedded scripts without rewriting them. It also accepts a truncated script left behind when std::fs::write created the file but returned an error during a prior repair. In either case zsh or bash can load stale or incomplete startup code and omit the user's configuration; compare each file with its embedded contents or unconditionally refresh the tree on first use.
Useful? React with 👍 / 👎.
| .file_name() | ||
| .and_then(|name| name.to_str()) | ||
| .is_some_and(|name| name.starts_with(prefix)) |
There was a problem hiding this comment.
When $TMPDIR contains any week-old directory whose name merely begins with noa-shell-integration-—for example noa-shell-integration-backup—this predicate classifies it as a prior generation and the code recursively deletes it. Since actual generated names have a nonempty numeric PID suffix, validate that complete suffix before allowing remove_dir_all to avoid deleting unrelated data.
Useful? React with 👍 / 👎.
| if materialize(base) { | ||
| return Some(base.as_path()); |
There was a problem hiding this comment.
When two threads call Pty::spawn after the tree has been reaped, both can observe missing scripts and enter materialize concurrently because only initialization of the path is synchronized. Each std::fs::write truncates the shared file, so one caller can return and start zsh or bash while the other caller is still truncating and rewriting its bootstrap scripts, causing that shell to read partial startup code; protect repair with a lock or publish atomically written replacements.
Useful? React with 👍 / 👎.
1 participant
Summary
The shell-integration scripts are materialized once into
$TMPDIR/noa-shell-integration-<pid>and the path was then cached in aOnceLockand handed out unchecked forever. When the OS temp reaper removesthat tree under a running noa, every later pane still gets the stale path —
and because
ZDOTDIR(zsh) and--rcfile(bash) suppress the shell's normalstartup lookup, the shell comes up with no configuration at all, not even the
user's own. It reads as "zsh stopped loading and I got sh".
The scripts are now re-verified on every handout and rewritten when any went
missing, so an unusable directory degrades to "no integration" (user config
intact) rather than to a dangling path. Materialization failure is no longer
cached either: one transient write error used to disable integration for the
rest of the process's life. A second commit sweeps the per-launch directories
earlier runs left behind (112 had accumulated locally); a still-running process
whose directory is swept simply re-materializes it.
Test plan
cargo test -p noa-pty— 39 passed (sandbox off; openpty needs real devices)cargo clippy -p noa-pty --all-targetsandcargo fmt --all -- --checkcleanZDOTDIR=/nonexistent zsh -lloads no user configNot reproduced live in the reaped state — the root-cause link rests on that
shell-behavior check plus the leaked-directory evidence.
https://claude.ai/code/session_01CYRbmrCgjVLcVNasPyHYKX