Skip to content

expand and validate working-dir, honor it on fork path - #1747

Open
raphamorim wants to merge 1 commit into
mainfrom
expand-working-dir
Open

expand and validate working-dir, honor it on fork path#1747
raphamorim wants to merge 1 commit into
mainfrom
expand-working-dir

Conversation

@raphamorim

Copy link
Copy Markdown
Owner

Three related working-dir problems (#1088):

  • working-dir = "~" (or ~/projects) was passed to the pty literally and failed, since nothing expanded the tilde.
  • A nonexistent directory made the spawn fail or land somewhere unexpected with no feedback.
  • The fork pty path, the default on Linux/BSD via use-fork = true, never received the working directory at all, so the option was silently ignored there (same family as the shell.args bug fixed in pass shell args through the fork pty path #1721).

resolve_working_dir now expands a leading tilde, validates the path is a directory, and logs a warning while falling back to inheriting the current directory when it is not. The spawn path resolves through it, and create_pty_with_fork takes the directory and chdirs in the forked child before exec (resolved in the parent so the failure path can log). Six unit tests cover the resolution rules.

Closes #1088.

@raphamorim

Copy link
Copy Markdown
Owner Author

Review notes on this one: the fork-side chdir is async-signal-safe with the CString built pre-fork, defaults are unaffected (default working-dir is None), and all three unix spawn entry points are covered. One regression the first revision would have introduced: the flatpak branch forwards --directory to flatpak-spawn --host, where the host may see directories the sandbox cannot, so validating inside the sandbox could silently drop a working host path. Reworked so tilde expansion applies everywhere but existence validation happens only at the local use sites (the builder cwd and the fork child); the flatpak host keeps receiving the expanded path untouched. The failing Build check is the repo-wide nix/FlakeHub credential issue, same as every sibling PR.

@raphamorim
raphamorim force-pushed the expand-working-dir branch from 00329a2 to e41c27c Compare July 22, 2026 21:55
@mirsella

mirsella commented Aug 7, 2026

Copy link
Copy Markdown

hello ! LGTM, anything blocking ? can adopt it if anything is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report invalid working-dir paths

2 participants